| Message ID | 20241024103540.3482216-1-dario.binacchi@amarulasolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pmdomain: imx: gpcv2: replace dev_err() with dev_err_probe() | expand |
On 24/10/2024 12:35, Dario Binacchi wrote: > The patch standardizes the probe() code by replacing the two occurrences > of dev_err() with dev_err_probe(). Indeed, dev_err_probe() was used in all > other error paths of the probe() function. But why? Does not simplify the code and called function cannot defer. It prints error, but your commit does not mention this as benefit. Best regards, Krzysztof
Hi Dario, thanks for the patch. On 24-10-24, Dario Binacchi wrote: > The patch standardizes the probe() code by replacing the two occurrences > of dev_err() with dev_err_probe(). Indeed, dev_err_probe() was used in all > other error paths of the probe() function. I assume that this paths aren't using dev_err_probe because these paths can't return EPROBE_DEFER and therefore dev_err_probe() would use dev_err() anyway. Regards, Marco > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > --- > > drivers/pmdomain/imx/gpcv2.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pmdomain/imx/gpcv2.c b/drivers/pmdomain/imx/gpcv2.c > index 963d61c5af6d..6e6ecbf2e152 100644 > --- a/drivers/pmdomain/imx/gpcv2.c > +++ b/drivers/pmdomain/imx/gpcv2.c > @@ -1356,7 +1356,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev) > > ret = pm_genpd_init(&domain->genpd, NULL, true); > if (ret) { > - dev_err(domain->dev, "Failed to init power domain\n"); > + dev_err_probe(domain->dev, ret, "Failed to init power domain\n"); > goto out_domain_unmap; > } > > @@ -1367,7 +1367,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev) > ret = of_genpd_add_provider_simple(domain->dev->of_node, > &domain->genpd); > if (ret) { > - dev_err(domain->dev, "Failed to add genpd provider\n"); > + dev_err_probe(domain->dev, ret, "Failed to add genpd provider\n"); > goto out_genpd_remove; > } > > -- > 2.43.0 > > >
Hallo Marco, On Thu, Oct 24, 2024 at 01:01:23PM +0200, Marco Felsch wrote: > On 24-10-24, Dario Binacchi wrote: > > The patch standardizes the probe() code by replacing the two occurrences > > of dev_err() with dev_err_probe(). Indeed, dev_err_probe() was used in all > > other error paths of the probe() function. > > I assume that this paths aren't using dev_err_probe because these paths > can't return EPROBE_DEFER and therefore dev_err_probe() would use > dev_err() anyway. Note that dev_err_probe() has advantages even if the error code isn't EPROBE_DEFER. In this case it's mentioning the error code. See also commits 7065f92255bb ("driver core: Clarify that dev_err_probe() is OK even w/out -EPROBE_DEFER") 532888a59505 ("driver core: Better advertise dev_err_probe()") Best regards Uwe
Hi Uwe, On 24-10-24, Uwe Kleine-König wrote: > Hallo Marco, > > On Thu, Oct 24, 2024 at 01:01:23PM +0200, Marco Felsch wrote: > > On 24-10-24, Dario Binacchi wrote: > > > The patch standardizes the probe() code by replacing the two occurrences > > > of dev_err() with dev_err_probe(). Indeed, dev_err_probe() was used in all > > > other error paths of the probe() function. > > > > I assume that this paths aren't using dev_err_probe because these paths > > can't return EPROBE_DEFER and therefore dev_err_probe() would use > > dev_err() anyway. > > Note that dev_err_probe() has advantages even if the error code isn't > EPROBE_DEFER. In this case it's mentioning the error code. > > See also commits > 7065f92255bb ("driver core: Clarify that dev_err_probe() is OK even w/out -EPROBE_DEFER") > 532888a59505 ("driver core: Better advertise dev_err_probe()") thanks for the pointers. With that in mind it make sense to me to convert it. Feel free to add my: Reviewed-by: Marco Felsch <m.felsch@pengutronix.de> Regards, Marco
Hi Dario, On 24-10-24, Marco Felsch wrote: > Hi Uwe, > > On 24-10-24, Uwe Kleine-König wrote: > > Hallo Marco, > > > > On Thu, Oct 24, 2024 at 01:01:23PM +0200, Marco Felsch wrote: > > > On 24-10-24, Dario Binacchi wrote: > > > > The patch standardizes the probe() code by replacing the two occurrences > > > > of dev_err() with dev_err_probe(). Indeed, dev_err_probe() was used in all > > > > other error paths of the probe() function. > > > > > > I assume that this paths aren't using dev_err_probe because these paths > > > can't return EPROBE_DEFER and therefore dev_err_probe() would use > > > dev_err() anyway. > > > > Note that dev_err_probe() has advantages even if the error code isn't > > EPROBE_DEFER. In this case it's mentioning the error code. > > > > See also commits > > 7065f92255bb ("driver core: Clarify that dev_err_probe() is OK even w/out -EPROBE_DEFER") > > 532888a59505 ("driver core: Better advertise dev_err_probe()") > > thanks for the pointers. With that in mind it make sense to me to > convert it. Feel free to add my: that being said, I would like to ask you if you could addapt the commit message, to point out the advantage of using dev_err_probe(). > Reviewed-by: Marco Felsch <m.felsch@pengutronix.de> > > Regards, > Marco > >
diff --git a/drivers/pmdomain/imx/gpcv2.c b/drivers/pmdomain/imx/gpcv2.c index 963d61c5af6d..6e6ecbf2e152 100644 --- a/drivers/pmdomain/imx/gpcv2.c +++ b/drivers/pmdomain/imx/gpcv2.c @@ -1356,7 +1356,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev) ret = pm_genpd_init(&domain->genpd, NULL, true); if (ret) { - dev_err(domain->dev, "Failed to init power domain\n"); + dev_err_probe(domain->dev, ret, "Failed to init power domain\n"); goto out_domain_unmap; } @@ -1367,7 +1367,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev) ret = of_genpd_add_provider_simple(domain->dev->of_node, &domain->genpd); if (ret) { - dev_err(domain->dev, "Failed to add genpd provider\n"); + dev_err_probe(domain->dev, ret, "Failed to add genpd provider\n"); goto out_genpd_remove; }