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]

Date: Wed, 26 Jul 2023 16:29:25 +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 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.

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.

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.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!