diff mbox series

pmdomain: imx: gpcv2: replace dev_err() with dev_err_probe()

Message ID 20241024103540.3482216-1-dario.binacchi@amarulasolutions.com
State New
Headers
Series pmdomain: imx: gpcv2: replace dev_err() with dev_err_probe() |

Commit Message

Dario Binacchi Oct. 24, 2024, 10:35 a.m. UTC 
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.

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(-)

Comments

Krzysztof Kozlowski Oct. 24, 2024, 10:58 a.m. UTC | #1 
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

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Marco Felsch Oct. 24, 2024, 11:01 a.m. UTC | #2 
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
> 
> 
> 

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Uwe Kleine-König Oct. 24, 2024, 5:26 p.m. UTC | #3 
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

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Marco Felsch Oct. 24, 2024, 6:08 p.m. UTC | #4 
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

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Marco Felsch Oct. 25, 2024, 7:14 a.m. UTC | #5 
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
> 
> 

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
diff mbox series

Patch

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;
 	}