| Message ID | 20230405161043.46190-3-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix RK3588 error prints | expand |
On Wed, Apr 05, 2023 at 06:10:43PM +0200, Sebastian Reichel wrote: > The usual devm_regulator_get() call already handles "optional" > regulators by returning a valid dummy and printing a warning > that the dummy regulator should be described properly. This > code open coded the same behaviour, but masked any errors that > are not -EPROBE_DEFER and is quite noisy. > > This change effectively unmasks and propagates regulators errors > not involving -ENODEV, downgrades the error print to warning level > if no regulator is specified and captures the probe defer message > for /sys/kernel/debug/devices_deferred. > > Fixes: 2e12f536635f8 ("net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator") > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > index 6fdad0f10d6f..d9deba110d4b 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > @@ -1656,14 +1656,11 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev, > } > } > > - bsp_priv->regulator = devm_regulator_get_optional(dev, "phy"); > + bsp_priv->regulator = devm_regulator_get(dev, "phy"); > if (IS_ERR(bsp_priv->regulator)) { > - if (PTR_ERR(bsp_priv->regulator) == -EPROBE_DEFER) { > - dev_err(dev, "phy regulator is not available yet, deferred probing\n"); > - return ERR_PTR(-EPROBE_DEFER); > - } > - dev_err(dev, "no regulator found\n"); > - bsp_priv->regulator = NULL; Does phy_power_on() need to be updated for this change? F.e. Does the bsp_priv->regulator == NULL still make sense? > + ret = PTR_ERR(bsp_priv->regulator); > + dev_err_probe(dev, ret, "failed to get phy regulator\n"); > + return ERR_PTR(ret); > } > > ret = of_property_read_string(dev->of_node, "clock_in_out", &strings); > -- > 2.39.2 >
Hi Simon, On Wed, Apr 05, 2023 at 07:43:15PM +0200, Simon Horman wrote: > On Wed, Apr 05, 2023 at 06:10:43PM +0200, Sebastian Reichel wrote: > > The usual devm_regulator_get() call already handles "optional" > > regulators by returning a valid dummy and printing a warning > > that the dummy regulator should be described properly. This > > code open coded the same behaviour, but masked any errors that > > are not -EPROBE_DEFER and is quite noisy. > > > > This change effectively unmasks and propagates regulators errors > > not involving -ENODEV, downgrades the error print to warning level > > if no regulator is specified and captures the probe defer message > > for /sys/kernel/debug/devices_deferred. > > > > Fixes: 2e12f536635f8 ("net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator") > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > --- > > drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > > index 6fdad0f10d6f..d9deba110d4b 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > > @@ -1656,14 +1656,11 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev, > > } > > } > > > > - bsp_priv->regulator = devm_regulator_get_optional(dev, "phy"); > > + bsp_priv->regulator = devm_regulator_get(dev, "phy"); > > if (IS_ERR(bsp_priv->regulator)) { > > - if (PTR_ERR(bsp_priv->regulator) == -EPROBE_DEFER) { > > - dev_err(dev, "phy regulator is not available yet, deferred probing\n"); > > - return ERR_PTR(-EPROBE_DEFER); > > - } > > - dev_err(dev, "no regulator found\n"); > > - bsp_priv->regulator = NULL; > > Does phy_power_on() need to be updated for this change? > F.e. Does the bsp_priv->regulator == NULL still make sense? Yes, it can be removed (but does not hurt). The regulator API returns NULL for devm_regulator_get when CONFIG_REGULATOR is not enabled. But regulator_enable/regulator_disable are just 'return 0;' stubs for that case anyways. -- Sebastian > > + ret = PTR_ERR(bsp_priv->regulator); > > + dev_err_probe(dev, ret, "failed to get phy regulator\n"); > > + return ERR_PTR(ret); > > } > > > > ret = of_property_read_string(dev->of_node, "clock_in_out", &strings); > > -- > > 2.39.2 > > > > -- > To unsubscribe, send mail to kernel-unsubscribe@lists.collabora.co.uk.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 6fdad0f10d6f..d9deba110d4b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -1656,14 +1656,11 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev, } } - bsp_priv->regulator = devm_regulator_get_optional(dev, "phy"); + bsp_priv->regulator = devm_regulator_get(dev, "phy"); if (IS_ERR(bsp_priv->regulator)) { - if (PTR_ERR(bsp_priv->regulator) == -EPROBE_DEFER) { - dev_err(dev, "phy regulator is not available yet, deferred probing\n"); - return ERR_PTR(-EPROBE_DEFER); - } - dev_err(dev, "no regulator found\n"); - bsp_priv->regulator = NULL; + ret = PTR_ERR(bsp_priv->regulator); + dev_err_probe(dev, ret, "failed to get phy regulator\n"); + return ERR_PTR(ret); } ret = of_property_read_string(dev->of_node, "clock_in_out", &strings);
The usual devm_regulator_get() call already handles "optional" regulators by returning a valid dummy and printing a warning that the dummy regulator should be described properly. This code open coded the same behaviour, but masked any errors that are not -EPROBE_DEFER and is quite noisy. This change effectively unmasks and propagates regulators errors not involving -ENODEV, downgrades the error print to warning level if no regulator is specified and captures the probe defer message for /sys/kernel/debug/devices_deferred. Fixes: 2e12f536635f8 ("net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator") Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)