Hi Shenwei,

On 8/2/23 16:27, Shenwei Wang wrote:

quoted

-----Original Message-----
From: Johannes Zink <redacted>
Sent: Wednesday, August 2, 2023 1:26 AM
To: Shenwei Wang <shenwei.wang@nxp.com>; Russell King
[off-list ref]; David S. Miller [off-list ref]; Eric
Dumazet [off-list ref]; Jakub Kicinski [off-list ref]; Paolo
Abeni [off-list ref]; Maxime Coquelin
[off-list ref]; Shawn Guo [off-list ref]; Sascha
Hauer [off-list ref]; Neil Armstrong [off-list ref];
Kevin Hilman [off-list ref]; Vinod Koul [off-list ref]; Chen-
Yu Tsai [off-list ref]; Jernej Skrabec [off-list ref]; Samuel
Holland [off-list ref]
Cc: Giuseppe Cavallaro <redacted>; Alexandre Torgue
[off-list ref]; Jose Abreu [off-list ref];
Pengutronix Kernel Team [off-list ref]; Fabio Estevam
[off-list ref]; dl-linux-imx [off-list ref]; Jerome Brunet
[off-list ref]; Martin Blumenstingl
[off-list ref]; Bhupesh Sharma
[off-list ref]; Nobuhiro Iwamatsu
[off-list ref]; Simon Horman
[off-list ref]; Andrew Halaney [off-list ref];
Bartosz Golaszewski [off-list ref]; Wong Vee Khee
[off-list ref]; Revanth Kumar Uppala [off-list ref]; Jochen
Henneberg [off-list ref]; netdev@vger.kernel.org; linux-
stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org; linux-amlogic@lists.infradead.org;
imx@lists.linux.dev; Frank Li [off-list ref]
Subject: Re: [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,

On 8/1/23 19:10, Shenwei Wang wrote:

quoted

quoted

-----Original Message-----
From: Johannes Zink <redacted>
Sent: Tuesday, August 1, 2023 7:48 AM
To: Shenwei Wang <shenwei.wang@nxp.com>; Russell King
[off-list ref]; David S. Miller [off-list ref]; Eric
Dumazet [off-list ref]; Jakub Kicinski [off-list ref];
Paolo Abeni [off-list ref]; Maxime Coquelin
[off-list ref]; Shawn Guo [off-list ref];

Sascha

quoted

quoted

Hauer [off-list ref]; Neil Armstrong
[off-list ref]; Kevin Hilman [off-list ref];
Vinod Koul [off-list ref]; Chen- Yu Tsai [off-list ref]; Jernej
Skrabec [off-list ref]; Samuel Holland
[off-list ref]
Cc: Giuseppe Cavallaro <redacted>; Alexandre Torgue
[off-list ref]; Jose Abreu [off-list ref];
Pengutronix Kernel Team [off-list ref]; Fabio Estevam
[off-list ref]; dl-linux-imx [off-list ref]; Jerome Brunet
[off-list ref]; Martin Blumenstingl
[off-list ref]; Bhupesh Sharma
[off-list ref]; Nobuhiro Iwamatsu
[off-list ref]; Simon Horman
[off-list ref]; Andrew Halaney [off-list ref];
Bartosz Golaszewski [off-list ref]; Wong Vee Khee
[off-list ref]; Revanth Kumar Uppala [off-list ref];
Jochen Henneberg [off-list ref];
netdev@vger.kernel.org; linux- stm32@st-md-mailman.stormreply.com;
linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org; linux-amlogic@lists.infradead.org;
imx@lists.linux.dev; Frank Li [off-list ref]
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:

quoted

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.

Okay.

quoted

Also: does this only apply to i.MX93, or would we have to test and enable it on
e.g. i.MX8MP as well?

Yes, it is required when the EQOS MAC is selected. However, this patch just enables
The feature on i.MX93.

If this behaviour is required on all EQOS, I think the name 
imx_dwmac_fix_speed_mx93() is misleading. It should either be 
imx_dwmac_fix_speed() if applicable to all imx implementations, or 
dwmac_fix_speed() (and moved to a non-gluecode file) if applicable for all 
implementations in general.

You can then add a second patch for enabling it for the i.mx93 in the gluecode 
driver.

Johannes

Thanks,
Shenwei

quoted

Thanks
Johannes

[1] ZMk/xqRP67zXHNrf@shell.armlinux.org.uk

quoted

Thanks,
Shenwei

quoted

Best regards
Johannes

quoted

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@nxp.com>
Reviewed-by: Frank Li <frank.li@nxp.com>
---
    .../net/ethernet/stmicro/stmmac/dwmac-imx.c   | 42

+++++++++++++++++++

quoted

quoted

quoted

    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%2F&data=05%7C01%7Cshenwei.wang%40nxp.com%7Ccfd142f0d60a461

ee01408

quoted

quoted

db9321578d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63826554
36335

quoted

quoted

61986%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
zIiLCJ

quoted

quoted

BTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CV10o1M%2BOj
DPOaH5C

quoted

quoted

y%2Fka%2B0aOMs0IaVapMH7aa3RnTI%3D&reserved=0

ngutronix.de%2F&data=05%7C01%7Cshenwei.wang%40nxp.com%7C761fbb75c

quoted

quoted

1c24cfe091508db928d8ade%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C

quoted

quoted

0%7C638264908852977732%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA

quoted

quoted

wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%

quoted

quoted

7C&sdata=2l2zNfIaNnRJENmERehNae8g%2F%2BQqlxD2YRx7ksY2X%2BE%3D&r

quoted

quoted

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.pe/
ngutronix.de%2F&data=05%7C01%7Cshenwei.wang%40nxp.com%7Ccfd142f0d
60a461ee01408db9321578d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
C0%7C638265543633561986%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
%7C&sdata=yKzNPsHqD%2FxU%2FRmzLn4JSQjmuT9tU8SabLxHyGTTmms%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    |