linux-kernel - Re: [PATCH net-next 2/2] net: stmmac: platform: add support for phy-supply

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:   Tue, 18 Jul 2023 15:08:46 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Marco Felsch <m.felsch@...gutronix.de>
Cc:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        peppe.cavallaro@...com, alexandre.torgue@...s.st.com,
        joabreu@...opsys.com, mcoquelin.stm32@...il.com,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, kernel@...gutronix.de
Subject: Re: [PATCH net-next 2/2] net: stmmac: platform: add support for
 phy-supply

On Tue, Jul 18, 2023 at 10:35:04AM +0200, Marco Felsch wrote:
> On 23-07-18, Andrew Lunn wrote:
> > On Mon, Jul 17, 2023 at 06:43:07PM +0200, Marco Felsch wrote:
> > > Add generic phy-supply handling support to control the phy regulator.
> > > Use the common stmmac_platform code path so all drivers using
> > > stmmac_probe_config_dt() and stmmac_pltfr_pm_ops can use it.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@...gutronix.de>
> > > ---
> > >  .../ethernet/stmicro/stmmac/stmmac_platform.c | 51 +++++++++++++++++++
> > >  include/linux/stmmac.h                        |  1 +
> > >  2 files changed, 52 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > > index eb0b2898daa3d..6193d42b53fb7 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > > @@ -10,6 +10,7 @@
> > >  
> > >  #include <linux/platform_device.h>
> > >  #include <linux/pm_runtime.h>
> > > +#include <linux/regulator/consumer.h>
> > >  #include <linux/module.h>
> > >  #include <linux/io.h>
> > >  #include <linux/of.h>
> > > @@ -423,6 +424,15 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
> > >  	if (plat->interface < 0)
> > >  		plat->interface = plat->phy_interface;
> > >  
> > > +	/* Optional regulator for PHY */
> > > +	plat->phy_regulator = devm_regulator_get_optional(&pdev->dev, "phy");
> > > +	if (IS_ERR(plat->phy_regulator)) {
> > > +		if (PTR_ERR(plat->phy_regulator) == -EPROBE_DEFER)
> > > +			return ERR_CAST(plat->phy_regulator);
> > > +		dev_info(&pdev->dev, "No regulator found\n");
> > > +		plat->phy_regulator = NULL;
> > > +	}
> > > +
> > 
> > So this gets the regulator. When do you actually turn it on?
> 
> During the suspend/resume logic like the rockchip, sun8i platform
> integrations did.

So you are assuming the boot loader has turned it on?

You also might have a difference between the actual state, and what
kernel thinks the state is, depending on how the regulator is
implemented.

It would be better to explicitly turn it on before registering the
MDIO bus.

     Andrew