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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZMFtw0LNozhNjRGF@shell.armlinux.org.uk>
Date: Wed, 26 Jul 2023 20:02:27 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Shenwei Wang <shenwei.wang@....com>
Cc: Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Shawn Guo <shawnguo@...nel.org>, dl-linux-imx <linux-imx@....com>,
	Giuseppe Cavallaro <peppe.cavallaro@...com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-stm32@...md-mailman.stormreply.com" <linux-stm32@...md-mailman.stormreply.com>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	Frank Li <frank.li@....com>
Subject: Re: [EXT] Re: [PATCH] net: stmmac: dwmac-imx: pause the TXC clock in
 fixed-link

On Wed, Jul 26, 2023 at 06:47:15PM +0000, Shenwei Wang wrote:
> 
> 
> > -----Original Message-----
> > From: Russell King <linux@...linux.org.uk>
> > Sent: Wednesday, July 26, 2023 12:01 PM
> > To: Shenwei Wang <shenwei.wang@....com>
> > Cc: Vladimir Oltean <olteanv@...il.com>; David S. Miller
> > <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>; Jakub
> > Kicinski <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Maxime
> > Coquelin <mcoquelin.stm32@...il.com>; Shawn Guo <shawnguo@...nel.org>;
> > dl-linux-imx <linux-imx@....com>; Giuseppe Cavallaro
> > <peppe.cavallaro@...com>; Alexandre Torgue <alexandre.torgue@...s.st.com>;
> > Jose Abreu <joabreu@...opsys.com>; Sascha Hauer <s.hauer@...gutronix.de>;
> > Pengutronix Kernel Team <kernel@...gutronix.de>; Fabio Estevam
> > <festevam@...il.com>; netdev@...r.kernel.org; linux-stm32@...md-
> > mailman.stormreply.com; linux-arm-kernel@...ts.infradead.org;
> > imx@...ts.linux.dev; Frank Li <frank.li@....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:59:38PM +0000, Shenwei Wang wrote:
> > > > -----Original Message-----
> > > > From: Russell King <linux@...linux.org.uk>
> > > > Sent: Wednesday, July 26, 2023 10:29 AM
> > > > To: Shenwei Wang <shenwei.wang@....com>
> > > > Cc: Vladimir Oltean <olteanv@...il.com>; David S. Miller
> > > > <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>; Jakub
> > > > Kicinski <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Maxime
> > > > Coquelin <mcoquelin.stm32@...il.com>; Shawn Guo
> > > > <shawnguo@...nel.org>; dl-linux-imx <linux-imx@....com>; Giuseppe
> > > > Cavallaro <peppe.cavallaro@...com>; Alexandre Torgue
> > > > <alexandre.torgue@...s.st.com>; Jose Abreu <joabreu@...opsys.com>;
> > > > Sascha Hauer <s.hauer@...gutronix.de>; Pengutronix Kernel Team
> > > > <kernel@...gutronix.de>; Fabio Estevam <festevam@...il.com>;
> > > > netdev@...r.kernel.org; linux-stm32@...md- mailman.stormreply.com;
> > > > linux-arm-kernel@...ts.infradead.org;
> > > > imx@...ts.linux.dev; Frank Li <frank.li@....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?
> >
> > Yes, because that is how all network drivers have behaved. If you look at the
> > function that Vladimir pointed out, then you will notice that the mere presence
> > of a fixed-link node makes it a "fixed link".
> >
> 
> Then, the way this phylink driver behaves makes the rest of the discussion kind of pointless
> for now, because I don't actually need fix_mac_speed to give me any interface info now.
> The basic of_phy_is_fixed_link check does the job for me.
> 
> Not sure why you think it's inefficient - could you explain that part?

Because of_phy_is_fixed_link() has to chase various pointers, walk
the child nodes and do a string compare on each, whereas you could
just be testing an integer!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!