| Message ID | 20230317174243.61500-3-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix RK3588 error prints | expand |
On Fri, Mar 17, 2023 at 06:42:43PM +0100, 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. Code looks fine, however this seems like a fix, then Fixes tag would be nice. Also target tree (net?) should be specified. > > 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 126812cd17e6..01de0174fa18 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > @@ -1680,14 +1680,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); > -- > 2.39.2 >
Hi, On Fri, Mar 17, 2023 at 07:16:50PM +0100, Piotr Raczynski wrote: > On Fri, Mar 17, 2023 at 06:42:43PM +0100, 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. > > Code looks fine, however this seems like a fix, then Fixes tag would > be nice. Also target tree (net?) should be specified. Fixes: 2e12f536635f8 ("net: stmmac: dwmac-rk: Use standard devicetree property for phy regulator") net-next should be good enough. -- Sebastian > > > > > 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 126812cd17e6..01de0174fa18 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c > > @@ -1680,14 +1680,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); > > -- > > 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 126812cd17e6..01de0174fa18 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -1680,14 +1680,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. 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(-)