| Message ID | 20230928151512.322016-9-christophe.roullier@foss.st.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Series to deliver Ethernets for STM32MP13 | expand |
Hello, On Thu, Sep 28, 2023 at 05:15:08PM +0200, Christophe Roullier wrote: > From: Christophe Roullier <christophe.roullier@st.com> > > Configure the phy regulator if defined by the "phy-supply" DT phandle. > > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> > --- > .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 51 ++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > index 72dda71850d75..31e3abd2caeaa 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c ... snip ... > static int stm32mp1_suspend(struct stm32_dwmac *dwmac) > @@ -455,12 +496,20 @@ static int stm32mp1_suspend(struct stm32_dwmac *dwmac) > if (dwmac->enable_eth_ck) > clk_disable_unprepare(dwmac->clk_eth_ck); > > + /* Keep the PHY up if we use Wake-on-Lan. */ > + if (!device_may_wakeup(dwmac->dev)) > + phy_power_on(dwmac, false); > + > return ret; > } > > static void stm32mp1_resume(struct stm32_dwmac *dwmac) > { > clk_disable_unprepare(dwmac->clk_ethstp); > + > + /* The PHY was up for Wake-on-Lan. */ > + if (!device_may_wakeup(dwmac->dev)) > + phy_power_on(dwmac, true); > } > > static int stm32mcu_suspend(struct stm32_dwmac *dwmac) Why only turn off the regulator in suspend on the STM32MP1 and not STM32 MCUs? It seems like this could just go in stm32_dwmac_suspend/resume(). Selfishly, I have a use case for this on an STM32F746 platform, so I would like to see support for it and would test an updated version. > -- > 2.25.1 > Thanks, Ben
On 9/28/23 17:45, Ben Wolsieffer wrote: > Hello, > > On Thu, Sep 28, 2023 at 05:15:08PM +0200, Christophe Roullier wrote: >> From: Christophe Roullier <christophe.roullier@st.com> >> >> Configure the phy regulator if defined by the "phy-supply" DT phandle. >> >> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >> --- >> .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 51 ++++++++++++++++++- >> 1 file changed, 50 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> index 72dda71850d75..31e3abd2caeaa 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > ... snip ... >> static int stm32mp1_suspend(struct stm32_dwmac *dwmac) >> @@ -455,12 +496,20 @@ static int stm32mp1_suspend(struct stm32_dwmac *dwmac) >> if (dwmac->enable_eth_ck) >> clk_disable_unprepare(dwmac->clk_eth_ck); >> >> + /* Keep the PHY up if we use Wake-on-Lan. */ >> + if (!device_may_wakeup(dwmac->dev)) >> + phy_power_on(dwmac, false); >> + >> return ret; >> } >> >> static void stm32mp1_resume(struct stm32_dwmac *dwmac) >> { >> clk_disable_unprepare(dwmac->clk_ethstp); >> + >> + /* The PHY was up for Wake-on-Lan. */ >> + if (!device_may_wakeup(dwmac->dev)) >> + phy_power_on(dwmac, true); >> } >> >> static int stm32mcu_suspend(struct stm32_dwmac *dwmac) > Why only turn off the regulator in suspend on the STM32MP1 and not STM32 > MCUs? It seems like this could just go in stm32_dwmac_suspend/resume(). > > Selfishly, I have a use case for this on an STM32F746 platform, so I > would like to see support for it and would test an updated version. > Hi, I'm working on MPU boards, I do not have MCU board, so feel free to contribute on MCU part ;-) Thanks Christophe >> -- >> 2.25.1 >> > Thanks, Ben
On 10/5/23 13:27, Christophe ROULLIER wrote: > > On 9/28/23 17:45, Ben Wolsieffer wrote: >> Hello, >> >> On Thu, Sep 28, 2023 at 05:15:08PM +0200, Christophe Roullier wrote: >>> From: Christophe Roullier <christophe.roullier@st.com> >>> >>> Configure the phy regulator if defined by the "phy-supply" DT phandle. >>> >>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >>> --- >>> .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 51 ++++++++++++++++++- >>> 1 file changed, 50 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> index 72dda71850d75..31e3abd2caeaa 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> ... snip ... >>> static int stm32mp1_suspend(struct stm32_dwmac *dwmac) >>> @@ -455,12 +496,20 @@ static int stm32mp1_suspend(struct stm32_dwmac >>> *dwmac) >>> if (dwmac->enable_eth_ck) >>> clk_disable_unprepare(dwmac->clk_eth_ck); >>> + /* Keep the PHY up if we use Wake-on-Lan. */ >>> + if (!device_may_wakeup(dwmac->dev)) >>> + phy_power_on(dwmac, false); >>> + >>> return ret; >>> } >>> static void stm32mp1_resume(struct stm32_dwmac *dwmac) >>> { >>> clk_disable_unprepare(dwmac->clk_ethstp); >>> + >>> + /* The PHY was up for Wake-on-Lan. */ >>> + if (!device_may_wakeup(dwmac->dev)) >>> + phy_power_on(dwmac, true); >>> } >>> static int stm32mcu_suspend(struct stm32_dwmac *dwmac) >> Why only turn off the regulator in suspend on the STM32MP1 and not STM32 >> MCUs? It seems like this could just go in stm32_dwmac_suspend/resume(). >> >> Selfishly, I have a use case for this on an STM32F746 platform, so I >> would like to see support for it and would test an updated version. >> > Hi, > > I'm working on MPU boards, I do not have MCU board, so feel free to > contribute on MCU part ;-) Christophe, The point here is to manage regulator for MPU and MCU. If you don't have MCU board it doesn't seem to be an issue as Ben proposed to test the patch for you. > > Thanks > > Christophe > >>> -- >>> 2.25.1 >>> >> Thanks, Ben
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c index 72dda71850d75..31e3abd2caeaa 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c @@ -14,6 +14,7 @@ #include <linux/of_net.h> #include <linux/phy.h> #include <linux/platform_device.h> +#include <linux/regulator/consumer.h> #include <linux/pm_wakeirq.h> #include <linux/regmap.h> #include <linux/slab.h> @@ -84,6 +85,7 @@ struct stm32_dwmac { u32 mode_reg; /* MAC glue-logic mode register */ u32 mode_mask; struct regmap *regmap; + struct regulator *regulator; u32 speed; const struct stm32_ops *ops; struct device *dev; @@ -309,6 +311,16 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac, if (err) pr_debug("Warning sysconfig register mask not set\n"); + dwmac->regulator = devm_regulator_get_optional(dev, "phy"); + if (IS_ERR(dwmac->regulator)) { + if (PTR_ERR(dwmac->regulator) == -EPROBE_DEFER) { + dev_dbg(dev, "phy regulator is not available yet, deferred probing\n"); + return -EPROBE_DEFER; + } + dev_dbg(dev, "no regulator found\n"); + dwmac->regulator = NULL; + } + return 0; } @@ -367,6 +379,27 @@ static int stm32_dwmac_wake_init(struct device *dev, return 0; } +static int phy_power_on(struct stm32_dwmac *bsp_priv, bool enable) +{ + int ret; + struct device *dev = bsp_priv->dev; + + if (!bsp_priv->regulator) + return 0; + + if (enable) { + ret = regulator_enable(bsp_priv->regulator); + if (ret) + dev_err(dev, "fail to enable phy-supply\n"); + } else { + ret = regulator_disable(bsp_priv->regulator); + if (ret) + dev_err(dev, "fail to disable phy-supply\n"); + } + + return 0; +} + static int stm32_dwmac_probe(struct platform_device *pdev) { struct plat_stmmacenet_data *plat_dat; @@ -414,12 +447,18 @@ static int stm32_dwmac_probe(struct platform_device *pdev) if (ret) goto err_wake_init_disable; - ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); + ret = phy_power_on(plat_dat->bsp_priv, true); if (ret) goto err_clk_disable; + ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); + if (ret) + goto err_gmac_powerdown; + return 0; +err_gmac_powerdown: + phy_power_on(plat_dat->bsp_priv, false); err_clk_disable: stm32_dwmac_clk_disable(dwmac); err_wake_init_disable: @@ -440,6 +479,8 @@ static void stm32_dwmac_remove(struct platform_device *pdev) dev_pm_clear_wake_irq(&pdev->dev); device_init_wakeup(&pdev->dev, false); + + phy_power_on(priv->plat->bsp_priv, false); } static int stm32mp1_suspend(struct stm32_dwmac *dwmac) @@ -455,12 +496,20 @@ static int stm32mp1_suspend(struct stm32_dwmac *dwmac) if (dwmac->enable_eth_ck) clk_disable_unprepare(dwmac->clk_eth_ck); + /* Keep the PHY up if we use Wake-on-Lan. */ + if (!device_may_wakeup(dwmac->dev)) + phy_power_on(dwmac, false); + return ret; } static void stm32mp1_resume(struct stm32_dwmac *dwmac) { clk_disable_unprepare(dwmac->clk_ethstp); + + /* The PHY was up for Wake-on-Lan. */ + if (!device_may_wakeup(dwmac->dev)) + phy_power_on(dwmac, true); } static int stm32mcu_suspend(struct stm32_dwmac *dwmac)