From: Christophe Roullier <[email protected]>
Configure the phy regulator if defined by the "phy-supply" DT phandle.
Signed-off-by: Christophe Roullier <[email protected]>
---
.../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
@@ -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)
--
2.25.1
> +static int phy_power_on(struct stm32_dwmac *bsp_priv, bool enable)
I find this function name confusing, since 50% of the time it does not
actually power the PHY on. You never call it with anything other than
a static true/false value. So it might was well be two functions,
phy_power_on() and phy_power_off().
> +{
> + 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");
Not all PHYs are usable in 0 picoseconds. You probably want a delay
here. Otherwise the first few accesses to it might not work.
Andrew
On 9/28/23 19:53, Andrew Lunn wrote:
>> +static int phy_power_on(struct stm32_dwmac *bsp_priv, bool enable)
> I find this function name confusing, since 50% of the time it does not
> actually power the PHY on. You never call it with anything other than
> a static true/false value. So it might was well be two functions,
> phy_power_on() and phy_power_off().
Hi,
I wanted to keep same implementation of all others Ethernet glues
(dwmac-rk.c ...) to be consistent.
>> +{
>> + 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");
> Not all PHYs are usable in 0 picoseconds. You probably want a delay
> here. Otherwise the first few accesses to it might not work.
>
> Andrew
You're right I will add a delay.
Thanks
Christophe