[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: Tuesday, July 25, 2023 4:05 PM
> To: Shenwei Wang <shenwei.wang at nxp.com>
> Cc: 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: [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 Tue, Jul 25, 2023 at 02:49:31PM -0500, Shenwei Wang wrote:
> > +static bool imx_dwmac_is_fixed_link(struct imx_priv_data *dwmac) {
> > +     struct plat_stmmacenet_data *plat_dat;
> > +     struct device_node *dn;
> > +
> > +     if (!dwmac || !dwmac->plat_dat)
> > +             return false;
> > +
> > +     plat_dat = dwmac->plat_dat;
> > +     dn = of_get_child_by_name(dwmac->dev->of_node, "fixed-link");
> > +     if (!dn)
> > +             return false;
> > +
> > +     if (plat_dat->phy_node == dn || plat_dat->phylink_node == dn)
> > +             return true;
>
> Why would the phy_node or the phylink_node ever be pointing at the fixed-link
> node?
>

The logic was learned from the function of stmmac_probe_config_dt, and it normally
save the phy handle to those two members: phy_node and phylink_node. But seems
checking phy_node is enough here, right?

        plat->phy_node = of_parse_phandle(np, "phy-handle", 0);

        /* PHYLINK automatically parses the phy-handle property */
        plat->phylink_node = np;

> For one, phylink expects the fwnode being passed to it to be pointing at the
> _parent_ node of the fixed-link node, since it looks up from the parent for
> "fixed-link" node.
>

Yes,  the above line of code passes the parent node to phylink_node.

Thanks,
Shenwei

> --
> RMK's Patch system:
> https://www.ar/
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=05%7C01%7Cshenwei.wang
> %40nxp.com%7Cd5a4b8372a4a4e5092b008db8d52c6ce%7C686ea1d3bc2b4c6f
> a92cd99c5c301635%7C0%7C0%7C638259158876867949%7CUnknown%7CTWF
> pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%3D%7C3000%7C%7C%7C&sdata=XPNpTlv7jbmOfeiQL5w0A6M2c3p5AOiT
> UOGg73ijFb8%3D&reserved=0
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!