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:10:11 +0000
From: Shenwei Wang <shenwei.wang@....com>
To: Russell King <linux@...linux.org.uk>
CC: "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



> -----Original Message-----
> From: Russell King <linux@...linux.org.uk>
> Sent: Wednesday, July 26, 2023 10:09 AM
> To: Shenwei Wang <shenwei.wang@....com>
> Cc: 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:00:49PM +0000, Shenwei Wang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Russell King <linux@...linux.org.uk>
> > > Sent: Tuesday, July 25, 2023 4:05 PM
> > > To: Shenwei Wang <shenwei.wang@....com>
> > > Cc: 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: [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;
>
> So, plat->phy_node will never ever be equal to your "dn" above.
> plat->phylink_node is the same as dwmac->dev->of_node above, and
> so plat->phylink_node will never be your "dn" above either.
>
> Those two together means that imx_dwmac_is_fixed_link() will _always_ return
> false, and thus most of the code you're adding is rather useless.
>
> It also means the code you're submitting probably hasn't been properly tested.
>
> Have you confirmed that imx_dwmac_is_fixed_link() will actually return true in
> your testing? Under what conditions did your testing reveal a true return value
> from this function?
>

We can't make that assumption. In my testing code, I had trace statements in that
section to indicate the code was actually executed. You may get some clues from the following DTS:

+&eqos {
+       pinctrl-names = "default";
+       pinctrl-0 = <&pinctrl_eqos_rgmii>;
+       phy-mode = "rgmii-rxid";
+       phy-handle = <&fixed0>;
+       status = "okay";
+
+       fixed0: fixed-link {
+               speed = <1000>;
+               full-duplex;
+       };
+
+       mdio {
+               compatible = "snps,dwmac-mdio";
+               #address-cells = <1>;
+               #size-cells = <0>;
+               clock-frequency = <2500000>;
+
+               phy0: ethernet-phy@8 {
+                       reg = <0x8>;
+                       max-speed = <100>;
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+
+                       phy1: ethernet-phy@9 {
+                               reg = <0x9>;
+                               max-speed = <100>;
+                       };
+               };
...


Thanks,
Shenwei



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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ