linux-kernel - RE: [PATCH v3 net] net: phy: micrel: Dynamically control external clock of KSZ PHY

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:
 <PAXPR04MB8510008EBDA6EB1CF89246F8883E2@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Wed, 11 Dec 2024 01:49:50 +0000
From: Wei Fang <wei.fang@....com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "andrew@...n.ch" <andrew@...n.ch>, "hkallweit1@...il.com"
	<hkallweit1@...il.com>, "linux@...linux.org.uk" <linux@...linux.org.uk>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"florian.fainelli@...adcom.com" <florian.fainelli@...adcom.com>,
	"heiko.stuebner@...rry.de" <heiko.stuebner@...rry.de>, "fank.li@....com"
	<fank.li@....com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>
Subject: RE: [PATCH v3 net] net: phy: micrel: Dynamically control external
 clock of KSZ PHY

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: 2024年12月11日 8:43
> To: Wei Fang <wei.fang@....com>
> Cc: andrew@...n.ch; hkallweit1@...il.com; linux@...linux.org.uk;
> davem@...emloft.net; edumazet@...gle.com; pabeni@...hat.com;
> florian.fainelli@...adcom.com; heiko.stuebner@...rry.de; fank.li@....com;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; imx@...ts.linux.dev
> Subject: Re: [PATCH v3 net] net: phy: micrel: Dynamically control external clock
> of KSZ PHY
> 
> On Tue, 10 Dec 2024 02:45:57 +0000 Wei Fang wrote:
> > The simple fix could only fix the commit 985329462723 ("net: phy: micrel: use
> > devm_clk_get_optional_enabled for the rmii-ref clock"), because as the
> commit
> > message said some clock suppliers need to be enabled so that the driver can
> get
> > the correct clock rate.
> >
> > But the problem is that the simple fix cannot fix the 99ac4cbcc2a5 ("net: phy:
> > micrel: allow usage of generic ethernet-phy clock"). The change is as follows,
> > this change just enables the clock when the PHY driver probes. There are no
> > other operations on the clock, such as obtaining the clock rate. So you still
> think
> > a simple fix is good enough for net tree?
> 
> I may be missing something but if you don't need to disable the generic
> clock you can put the disable into the if () block for rmii-ref ?

For my case, it's fine to disable rmii-ref because this clock source is always
enabled in FEC driver. But the commit 99ac4cbcc2a5 ("net: phy: micrel: allow
usage of generic ethernet-phy clock") was applied a year ago, so I raised a
concern in V2 [1], if a new platform only enables rmii-ref in the PHY driver,
disabling rmii-ref after getting the clock rate will cause problem, which will
cause RMII to not work. I'm not sure if any platform actually does this, if so
the following changes will be a more serious problem.

[1] https://lore.kernel.org/all/PAXPR04MB8510D36DDA1B9E98B2FB77B488362@PAXPR04MB8510.eurprd04.prod.outlook.com/

> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 3ef508840674..8bbd2018f2a6 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -2214,6 +2214,8 @@ static int kszphy_probe(struct phy_device *phydev)
>                                    rate);
>                         return -EINVAL;
>                 }
> +
> +               clk_disable_unprepare(clk);
>         } else if (!clk) {
>                 /* unnamed clock from the generic ethernet-phy binding */
>                 clk = devm_clk_get_optional_enabled(&phydev->mdio.dev,
> NULL);