[EXT] Re: [PATCH] net: stmmac: dwmac-imx: pause the TXC clock in fixed-link

[Shenwei Wang]

> -----Original Message-----
> From: Russell King <linux at armlinux.org.uk>
> Sent: Wednesday, July 26, 2023 10:29 AM
> To: Shenwei Wang <shenwei.wang at nxp.com>
> Cc: Vladimir Oltean <olteanv at gmail.com>; 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>;
> dl-linux-imx <linux-imx at nxp.com>; Giuseppe Cavallaro
> <peppe.cavallaro at st.com>; Alexandre Torgue <alexandre.torgue at foss.st.com>;
> Jose Abreu <joabreu at synopsys.com>; Sascha Hauer <s.hauer at pengutronix.de>;
> Pengutronix Kernel Team <kernel at pengutronix.de>; Fabio Estevam
> <festevam at gmail.com>; netdev at vger.kernel.org; linux-stm32 at st-md-
> mailman.stormreply.com; linux-arm-kernel at lists.infradead.org;
> imx at lists.linux.dev; Frank Li <frank.li at nxp.com>
> Subject: Re: [EXT] Re: [PATCH] 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
>
>
> On Wed, Jul 26, 2023 at 03:10:19PM +0000, Shenwei Wang wrote:
> > > if (of_phy_is_fixed_link(dwmac->dev->of_node)) {
> > >
> >
> > This does not help in this case. What I need to determine is if the PHY currently
> in use is a fixed-link.
> > The dwmac DTS node may have multiple PHY nodes defined, including both
> fixed-link and real PHYs.
>
> ... and this makes me wonder what DT node structure you think would describe a
> fixed-link.
>
> A valid ethernet device node would be:
>
>         dwmac-node {
>                 phy-handle = <&phy1>;
>         };
>
> In this case:
>         dwmac->dev->of_node points at "dwmac-node"
>         plat->phylink_node points at "dwmac-node"
>         plat->phy_node points at "phy1"
>         Your "dn" is NULL.
>         Therefore, your imx_dwmac_is_fixed_link() returns false.
>
>         dwmac-node {
>                 fixed-link {
>                         speed = <...>;
>                         full-duplex;
>                 };
>         };
>
> In this case:
>         dwmac->dev->of_node points at "dwmac-node"
>         plat->phylink_node points at "dwmac-node"
>         plat->phy_node is NULL
>         Your "dn" points at the "fixed-link" node.
>         Therefore, your imx_dwmac_is_fixed_link() also returns false.
>
> Now, as far as your comment "What I need to determine is if the PHY currently
> in use is a fixed-link." I'm just going "Eh? What?" at that, because it makes zero
> sense to me.
>
> stmmac uses phylink. phylink doesn't use a PHY for fixed-links, unlike the old
> phylib-based fixed-link implementation that software-emulated a clause-22 PHY.
> With phylink, when fixed-link is specified, there is _no_ PHY.

So you mean the fixed-link node will always be the highest priority to be used in the phylink
use case?  If so, I just need to check if there is a fixed-link node as Vladimir pointed out, right?

>
> There is no need to do any of this poking about to determine if the link that is
> being brought up is a fixed-link or not, because phylink's callbacks into the MAC
> driver already contain this information in the "mode" argument. However, that
> is not passed to the driver's internal
> priv->plat->fix_mac_speed() method - but this is the information you
> need.
>

Yes, you are right. The best way is to change the fix_mac_speed prototype but it
will change several other platforms. That's why I didn't go that way.

Thanks,
Shenwei

> There is no need to write code to try and second-guess this, phylink tells drivers
> what mode it is operating under.
>
> stmmac really needs to be cleaned up - and really doesn't need more complexity
> when the information is already being provided to the driver.
>
> --
> RMK's Patch system:
> https://www.ar/
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=05%7C01%7Cshenwei.wang
> %40nxp.com%7C3d20eec67cbe49ecdbf008db8ded1aab%7C686ea1d3bc2b4c6fa
> 92cd99c5c301635%7C0%7C0%7C638259821712485867%7CUnknown%7CTWFp
> bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%3D%7C3000%7C%7C%7C&sdata=pMle7Mu4ed57Qjf7DR2PPQ67F5oKq9qr
> GRG%2BNMpSMDM%3D&reserved=0
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!