[EXT] Re: [PATCH v3 net 2/2] net: stmmac: dwmac-imx: pause the TXC clock in fixed-link
Johannes Zink
j.zink at pengutronix.de
Tue Aug 1 23:25:46 PDT 2023
Hi Shenwei,
On 8/1/23 19:10, Shenwei Wang wrote:
>
>
>> -----Original Message-----
>> From: Johannes Zink <j.zink at pengutronix.de>
>> Sent: Tuesday, August 1, 2023 7:48 AM
>> To: Shenwei Wang <shenwei.wang at nxp.com>; Russell King
>> <linux at armlinux.org.uk>; David S. Miller <davem at davemloft.net>; Eric
>> Dumazet <edumazet at google.com>; Jakub Kicinski <kuba at kernel.org>; Paolo
>> Abeni <pabeni at redhat.com>; Maxime Coquelin
>> <mcoquelin.stm32 at gmail.com>; Shawn Guo <shawnguo at kernel.org>; Sascha
>> Hauer <s.hauer at pengutronix.de>; Neil Armstrong <neil.armstrong at linaro.org>;
>> Kevin Hilman <khilman at baylibre.com>; Vinod Koul <vkoul at kernel.org>; Chen-
>> Yu Tsai <wens at csie.org>; Jernej Skrabec <jernej.skrabec at gmail.com>; Samuel
>> Holland <samuel at sholland.org>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro at st.com>; Alexandre Torgue
>> <alexandre.torgue at foss.st.com>; Jose Abreu <joabreu at synopsys.com>;
>> Pengutronix Kernel Team <kernel at pengutronix.de>; Fabio Estevam
>> <festevam at gmail.com>; dl-linux-imx <linux-imx at nxp.com>; Jerome Brunet
>> <jbrunet at baylibre.com>; Martin Blumenstingl
>> <martin.blumenstingl at googlemail.com>; Bhupesh Sharma
>> <bhupesh.sharma at linaro.org>; Nobuhiro Iwamatsu
>> <nobuhiro1.iwamatsu at toshiba.co.jp>; Simon Horman
>> <simon.horman at corigine.com>; Andrew Halaney <ahalaney at redhat.com>;
>> Bartosz Golaszewski <bartosz.golaszewski at linaro.org>; Wong Vee Khee
>> <veekhee at apple.com>; Revanth Kumar Uppala <ruppala at nvidia.com>; Jochen
>> Henneberg <jh at henneberg-systemdesign.com>; netdev at vger.kernel.org; linux-
>> stm32 at st-md-mailman.stormreply.com; linux-arm-kernel at lists.infradead.org;
>> linux-kernel at vger.kernel.org; linux-amlogic at lists.infradead.org;
>> imx at lists.linux.dev; Frank Li <frank.li at nxp.com>
>> Subject: [EXT] Re: [PATCH v3 net 2/2] net: stmmac: dwmac-imx: pause the TXC
>> clock in fixed-link
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report this
>> email' button
>>
>>
>> Hi Shenwei,
>>
>> thanks for your patch.
>>
>> On 7/31/23 18:19, Shenwei Wang wrote:
>>> When using a fixed-link setup, certain devices like the SJA1105
>>> require a small pause in the TXC clock line to enable their internal
>>> tunable delay line (TDL).
>>
>> If this is only required for some devices, is it safe to enforce this behaviour
>> unconditionally for any kind of fixed link devices connected to the MX93 EQOS
>> or could this possibly break for other devices?
>>
>
> It won't impact normal devices. The link layer hasn't built up yet.
>
As Russel suggested in [1] - maybe you could rephrase your commit message for
your v4 to point this out to future reviewers (apparently multiple people have
had questions about this...) and have this fact also recorded in the git log
later on.
Also: does this only apply to i.MX93, or would we have to test and enable it on
e.g. i.MX8MP as well?
Thanks
Johannes
[1] ZMk/xqRP67zXHNrf at shell.armlinux.org.uk
> Thanks,
> Shenwei
>
>> Best regards
>> Johannes
>>
>>>
>>> To satisfy this requirement, this patch temporarily disables the TX
>>> clock, and restarts it after a required period. This provides the
>>> required silent interval on the clock line for SJA1105 to complete the
>>> frequency transition and enable the internal TDLs.
>>>
>>> So far we have only enabled this feature on the i.MX93 platform.
>>>
>>> Signed-off-by: Shenwei Wang <shenwei.wang at nxp.com>
>>> Reviewed-by: Frank Li <frank.li at nxp.com>
>>> ---
>>> .../net/ethernet/stmicro/stmmac/dwmac-imx.c | 42 +++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
>>> index 53ee5a42c071..2e4173d099f3 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
>>> @@ -32,6 +32,7 @@
>>> #define GPR_ENET_QOS_RGMII_EN (0x1 << 21)
>>>
>>> #define MX93_GPR_ENET_QOS_INTF_MODE_MASK GENMASK(3, 0)
>>> +#define MX93_GPR_ENET_QOS_INTF_MASK GENMASK(3, 1)
>>> #define MX93_GPR_ENET_QOS_INTF_SEL_MII (0x0 << 1)
>>> #define MX93_GPR_ENET_QOS_INTF_SEL_RMII (0x4 << 1)
>>> #define MX93_GPR_ENET_QOS_INTF_SEL_RGMII (0x1 << 1)
>>> @@ -40,6 +41,7 @@
>>> #define DMA_BUS_MODE 0x00001000
>>> #define DMA_BUS_MODE_SFT_RESET (0x1 << 0)
>>> #define RMII_RESET_SPEED (0x3 << 14)
>>> +#define CTRL_SPEED_MASK GENMASK(15, 14)
>>>
>>> struct imx_dwmac_ops {
>>> u32 addr_width;
>>> @@ -56,6 +58,7 @@ struct imx_priv_data {
>>> struct regmap *intf_regmap;
>>> u32 intf_reg_off;
>>> bool rmii_refclk_ext;
>>> + void __iomem *base_addr;
>>>
>>> const struct imx_dwmac_ops *ops;
>>> struct plat_stmmacenet_data *plat_dat; @@ -212,6 +215,42 @@
>>> static void imx_dwmac_fix_speed(void *priv, uint speed, uint mode)
>>> dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>>> }
>>>
>>> +static void imx_dwmac_fix_speed_mx93(void *priv, uint speed, uint
>>> +mode) {
>>> + struct imx_priv_data *dwmac = priv;
>>> + int ctrl, old_ctrl, iface;
>>> +
>>> + imx_dwmac_fix_speed(priv, speed, mode);
>>> +
>>> + if (!dwmac || mode != MLO_AN_FIXED)
>>> + return;
>>> +
>>> + if (regmap_read(dwmac->intf_regmap, dwmac->intf_reg_off, &iface))
>>> + return;
>>> +
>>> + iface &= MX93_GPR_ENET_QOS_INTF_MASK;
>>> + if (iface != MX93_GPR_ENET_QOS_INTF_SEL_RGMII)
>>> + return;
>>> +
>>> + old_ctrl = readl(dwmac->base_addr + MAC_CTRL_REG);
>>> + ctrl = old_ctrl & ~CTRL_SPEED_MASK;
>>> + regmap_update_bits(dwmac->intf_regmap, dwmac->intf_reg_off,
>>> + MX93_GPR_ENET_QOS_INTF_MODE_MASK, 0);
>>> + writel(ctrl, dwmac->base_addr + MAC_CTRL_REG);
>>> +
>>> + /* Ensure the settings for CTRL are applied and avoid CPU/Compiler
>>> + * reordering.
>>> + */
>>> + wmb();
>>> +
>>> + usleep_range(10, 20);
>>> + iface |= MX93_GPR_ENET_QOS_CLK_GEN_EN;
>>> + regmap_update_bits(dwmac->intf_regmap, dwmac->intf_reg_off,
>>> + MX93_GPR_ENET_QOS_INTF_MODE_MASK, iface);
>>> +
>>> + writel(old_ctrl, dwmac->base_addr + MAC_CTRL_REG); }
>>> +
>>> static int imx_dwmac_mx93_reset(void *priv, void __iomem *ioaddr)
>>> {
>>> struct plat_stmmacenet_data *plat_dat = priv; @@ -317,8 +356,11
>>> @@ static int imx_dwmac_probe(struct platform_device *pdev)
>>> plat_dat->exit = imx_dwmac_exit;
>>> plat_dat->clks_config = imx_dwmac_clks_config;
>>> plat_dat->fix_mac_speed = imx_dwmac_fix_speed;
>>> + if (of_machine_is_compatible("fsl,imx93"))
>>> + plat_dat->fix_mac_speed = imx_dwmac_fix_speed_mx93;
>>> plat_dat->bsp_priv = dwmac;
>>> dwmac->plat_dat = plat_dat;
>>> + dwmac->base_addr = stmmac_res.addr;
>>>
>>> ret = imx_dwmac_clks_config(dwmac, true);
>>> if (ret)
>>
>> --
>> Pengutronix e.K. | Johannes Zink |
>> Steuerwalder Str. 21 |
>> https://www.pe/
>> ngutronix.de%2F&data=05%7C01%7Cshenwei.wang%40nxp.com%7C761fbb75c
>> 1c24cfe091508db928d8ade%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
>> 0%7C638264908852977732%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
>> 7C&sdata=2l2zNfIaNnRJENmERehNae8g%2F%2BQqlxD2YRx7ksY2X%2BE%3D&r
>> eserved=0 |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>> Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |
>
>
--
Pengutronix e.K. | Johannes Zink |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |
More information about the linux-amlogic
mailing list