| Message ID | 20240828121805.3696631-1-lihongbo22@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9023fda2f186168e45bb8e7397ad15ff17fd7bf8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: realtek: make use of dev_err_cast_probe() | expand |
On Wed, Aug 28, 2024 at 08:18:05PM +0800, Hongbo Li wrote: > Using dev_err_cast_probe() to simplify the code. > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Wed, Aug 28, 2024 at 08:18:05PM GMT, Hongbo Li wrote: > Using dev_err_cast_probe() to simplify the code. > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com> > --- > drivers/net/dsa/realtek/rtl83xx.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c > index 35709a1756ae..3c5018d5e1f9 100644 > --- a/drivers/net/dsa/realtek/rtl83xx.c > +++ b/drivers/net/dsa/realtek/rtl83xx.c > @@ -185,11 +185,9 @@ rtl83xx_probe(struct device *dev, > > /* TODO: if power is software controlled, set up any regulators here */ > priv->reset_ctl = devm_reset_control_get_optional(dev, NULL); > - if (IS_ERR(priv->reset_ctl)) { > - ret = PTR_ERR(priv->reset_ctl); > - dev_err_probe(dev, ret, "failed to get reset control\n"); > - return ERR_CAST(priv->reset_ctl); > - } > + if (IS_ERR(priv->reset_ctl)) > + return dev_err_cast_probe(dev, priv->reset_ctl, > + "failed to get reset control\n"); The change is fine, but maybe it would be nice to fix up the other two similar cases as well? The errors would be stringified but that's OK. 159 rc.lock_arg = priv; 160 priv->map = devm_regmap_init(dev, NULL, priv, &rc); 161 if (IS_ERR(priv->map)) { 162 ret = PTR_ERR(priv->map); 163 dev_err(dev, "regmap init failed: %d\n", ret); 164 return ERR_PTR(ret); 165 } 166 167 rc.disable_locking = true; 168 priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc); 169 if (IS_ERR(priv->map_nolock)) { 170 ret = PTR_ERR(priv->map_nolock); 171 dev_err(dev, "regmap init failed: %d\n", ret); 172 return ERR_PTR(ret); 173 } Then you can remove the ret variable altogether. Either way, Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
On 2024/8/29 0:14, Alvin Šipraga wrote: > On Wed, Aug 28, 2024 at 08:18:05PM GMT, Hongbo Li wrote: >> Using dev_err_cast_probe() to simplify the code. >> >> Signed-off-by: Hongbo Li <lihongbo22@huawei.com> >> --- >> drivers/net/dsa/realtek/rtl83xx.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c >> index 35709a1756ae..3c5018d5e1f9 100644 >> --- a/drivers/net/dsa/realtek/rtl83xx.c >> +++ b/drivers/net/dsa/realtek/rtl83xx.c >> @@ -185,11 +185,9 @@ rtl83xx_probe(struct device *dev, >> >> /* TODO: if power is software controlled, set up any regulators here */ >> priv->reset_ctl = devm_reset_control_get_optional(dev, NULL); >> - if (IS_ERR(priv->reset_ctl)) { >> - ret = PTR_ERR(priv->reset_ctl); >> - dev_err_probe(dev, ret, "failed to get reset control\n"); >> - return ERR_CAST(priv->reset_ctl); >> - } >> + if (IS_ERR(priv->reset_ctl)) >> + return dev_err_cast_probe(dev, priv->reset_ctl, >> + "failed to get reset control\n"); > > The change is fine, but maybe it would be nice to fix up the other two > similar cases as well? The errors would be stringified but that's OK. > > 159 rc.lock_arg = priv; > 160 priv->map = devm_regmap_init(dev, NULL, priv, &rc); > 161 if (IS_ERR(priv->map)) { > 162 ret = PTR_ERR(priv->map); > 163 dev_err(dev, "regmap init failed: %d\n", ret); > 164 return ERR_PTR(ret); It's similar, but not the same. Now we just have the dev_err_cast_probe which wraps the dev_err_probe. The dev_err_probe is not completely equivalent to dev_err. Thanks, Hongbo > 165 } > 166 > 167 rc.disable_locking = true; > 168 priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc); > 169 if (IS_ERR(priv->map_nolock)) { > 170 ret = PTR_ERR(priv->map_nolock); > 171 dev_err(dev, "regmap init failed: %d\n", ret); > 172 return ERR_PTR(ret); > 173 } > > Then you can remove the ret variable altogether. > > Either way, > > Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 28 Aug 2024 20:18:05 +0800 you wrote: > Using dev_err_cast_probe() to simplify the code. > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com> > --- > drivers/net/dsa/realtek/rtl83xx.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) Here is the summary with links: - [net-next] net: dsa: realtek: make use of dev_err_cast_probe() https://git.kernel.org/netdev/net-next/c/9023fda2f186 You are awesome, thank you!
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c index 35709a1756ae..3c5018d5e1f9 100644 --- a/drivers/net/dsa/realtek/rtl83xx.c +++ b/drivers/net/dsa/realtek/rtl83xx.c @@ -185,11 +185,9 @@ rtl83xx_probe(struct device *dev, /* TODO: if power is software controlled, set up any regulators here */ priv->reset_ctl = devm_reset_control_get_optional(dev, NULL); - if (IS_ERR(priv->reset_ctl)) { - ret = PTR_ERR(priv->reset_ctl); - dev_err_probe(dev, ret, "failed to get reset control\n"); - return ERR_CAST(priv->reset_ctl); - } + if (IS_ERR(priv->reset_ctl)) + return dev_err_cast_probe(dev, priv->reset_ctl, + "failed to get reset control\n"); priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); if (IS_ERR(priv->reset)) {
Using dev_err_cast_probe() to simplify the code. Signed-off-by: Hongbo Li <lihongbo22@huawei.com> --- drivers/net/dsa/realtek/rtl83xx.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)