[RFC 1/2] rtc: max77686: use symbolic error messages

[RFC 1/2] rtc: max77686: use symbolic error messages

[Edmundo Carmona Antoranz]

Modify some error messages so that the symbolic error value be
printed instead of a numeric value.
---
 drivers/rtc/rtc-max77686.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index ce089ed934ad..470260478752 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -711,7 +711,8 @@ static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
 						info->drv_data->regmap_config);
 	if (IS_ERR(info->rtc_regmap)) {
 		ret = PTR_ERR(info->rtc_regmap);
-		dev_err(info->dev, "Failed to allocate RTC regmap: %d\n", ret);
+		dev_err(info->dev, "Failed to allocate RTC regmap: %pe\n",
+			info->rtc_regmap);
 		return ret;
 	}
 
@@ -763,7 +764,8 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 
 	if (IS_ERR(info->rtc_dev)) {
 		ret = PTR_ERR(info->rtc_dev);
-		dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);
+		dev_err(&pdev->dev, "Failed to register RTC device: %pe\n",
+			info->rtc_dev);
 		goto err_rtc;
 	}
 
-- 
2.30.2

[RFC 2/2] staging: fbtft: use symbolic error messages

[Edmundo Carmona Antoranz]

Modify some error messages so that the symbolic error value be
printed instead of a numeric value.
---
 drivers/staging/fbtft/fb_ssd1351.c   | 3 +--
 drivers/staging/fbtft/fb_watterott.c | 3 +--
 drivers/staging/fbtft/fbtft-core.c   | 5 ++---
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
index 900b28d826b2..adf486b36289 100644
--- a/drivers/staging/fbtft/fb_ssd1351.c
+++ b/drivers/staging/fbtft/fb_ssd1351.c
@@ -223,8 +223,7 @@ static void register_onboard_backlight(struct fbtft_par *par)
 				       &bl_props);
 	if (IS_ERR(bd)) {
 		dev_err(par->info->device,
-			"cannot register backlight device (%ld)\n",
-			PTR_ERR(bd));
+			"cannot register backlight device (%pe)\n", bd);
 		return;
 	}
 	par->info->bl_dev = bd;
diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index 76b25df376b8..f9ecc5202c47 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -263,8 +263,7 @@ static void register_chip_backlight(struct fbtft_par *par)
 				       &bl_props);
 	if (IS_ERR(bd)) {
 		dev_err(par->info->device,
-			"cannot register backlight device (%ld)\n",
-			PTR_ERR(bd));
+			"cannot register backlight device (%pe)\n", bd);
 		return;
 	}
 	par->info->bl_dev = bd;
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 4f362dad4436..4d96ab2ea71c 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -83,7 +83,7 @@ static int fbtft_request_one_gpio(struct fbtft_par *par,
 	if (IS_ERR(*gpiop)) {
 		ret = PTR_ERR(*gpiop);
 		dev_err(dev,
-			"Failed to request %s GPIO: %d\n", name, ret);
+			"Failed to request %s GPIO: %pe\n", name, *gpiop);
 		return ret;
 	}
 	fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n",
@@ -194,8 +194,7 @@ void fbtft_register_backlight(struct fbtft_par *par)
 				       &fbtft_bl_ops, &bl_props);
 	if (IS_ERR(bd)) {
 		dev_err(par->info->device,
-			"cannot register backlight device (%ld)\n",
-			PTR_ERR(bd));
+			"cannot register backlight device (%pe)\n", bd);
 		return;
 	}
 	par->info->bl_dev = bd;
-- 
2.30.2

Re: [RFC 2/2] staging: fbtft: use symbolic error messages

[Edmundo Carmona Antoranz]

On Sat, May 8, 2021 at 6:42 PM Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
>
> Modify some error messages so that the symbolic error value be
> printed instead of a numeric value.
> ---

I wonder if this is more or less what Dan and Christophe were talking about.

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Christophe JAILLET]

Le 09/05/2021 à 02:41, Edmundo Carmona Antoranz a écrit :
> Modify some error messages so that the symbolic error value be
> printed instead of a numeric value.
> ---
>   drivers/rtc/rtc-max77686.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index ce089ed934ad..470260478752 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -711,7 +711,8 @@ static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
>   						info->drv_data->regmap_config);
>   	if (IS_ERR(info->rtc_regmap)) {
>   		ret = PTR_ERR(info->rtc_regmap);
> -		dev_err(info->dev, "Failed to allocate RTC regmap: %d\n", ret);
> +		dev_err(info->dev, "Failed to allocate RTC regmap: %pe\n",
> +			info->rtc_regmap);
>   		return ret;
>   	}
>   
> @@ -763,7 +764,8 @@ static int max77686_rtc_probe(struct platform_device *pdev)
>   
>   	if (IS_ERR(info->rtc_dev)) {
>   		ret = PTR_ERR(info->rtc_dev);
> -		dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);
> +		dev_err(&pdev->dev, "Failed to register RTC device: %pe\n",
> +			info->rtc_dev);
>   		goto err_rtc;
>   	}
>   
> 
Hi,
both patches LGTM.

CJ

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Dan Carpenter]

On Sat, May 08, 2021 at 06:41:11PM -0600, Edmundo Carmona Antoranz wrote:
> Modify some error messages so that the symbolic error value be
> printed instead of a numeric value.
> ---

No Signed-off-by.  I feel like we should hold off on this for existing
code until the %pe patches get backported to more -stable kernels.
Otherwise if we backport a driver which uses %pe and the kernel doesn't
support it then that's a headache.  I think it gets treated like vanilla
%p.

regards,
dan carpenter

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Julia Lawall]

On Mon, 10 May 2021, Dan Carpenter wrote:

> On Sat, May 08, 2021 at 06:41:11PM -0600, Edmundo Carmona Antoranz wrote:
> > Modify some error messages so that the symbolic error value be
> > printed instead of a numeric value.
> > ---
>
> No Signed-off-by.

It wasn't meant as a patch to be applied.  Just to show the opportunities.

> I feel like we should hold off on this for existing
> code until the %pe patches get backported to more -stable kernels.
> Otherwise if we backport a driver which uses %pe and the kernel doesn't
> support it then that's a headache.  I think it gets treated like vanilla
> %p.

OK, that seems like a reasonable strategy.

Any idea what %i is?  Does it actually exist?

julia

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Marion & Christophe JAILLET]

Le 10/05/2021 à 07:06, Julia Lawall a écrit :
> Any idea what %i is?  Does it actually exist?

%i = %d
(https://elixir.bootlin.com/linux/v5.13-rc1/source/lib/vsprintf.c#L2592)

CJ


> julia

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Dan Carpenter]

On Sun, May 09, 2021 at 11:17:42PM +0200, Julia Lawall wrote:
> ---------------------------
> 
> Patch of a recent linux next:
> 
> diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
> index 2e35354b61f5..ff75e928772a 100644
> --- a/arch/arm/mach-pxa/cm-x300.c
> +++ b/arch/arm/mach-pxa/cm-x300.c
> @@ -531,7 +531,7 @@ static int cm_x300_u2d_init(struct device *dev)
>  		pout_clk = clk_get(NULL, "CLK_POUT");
>  		if (IS_ERR(pout_clk)) {
>  			err = PTR_ERR(pout_clk);
> -			pr_err("failed to get CLK_POUT: %d\n", err);
> +			pr_err("failed to get CLK_POUT: %pe\n", err);

It has to print the pointer and not the err.   So it should be:

			pr_err("failed to get CLK_POUT: %pe\n", pout_clk);

But really someone needs to introduce an %e (and everyone can see the
need for %e but it's just a matter of finding someone who is feeling
motivated).

regards,
dan carpenter

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Julia Lawall]

On Mon, 10 May 2021, Dan Carpenter wrote:

> On Sun, May 09, 2021 at 11:17:42PM +0200, Julia Lawall wrote:
> > ---------------------------
> >
> > Patch of a recent linux next:
> >
> > diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
> > index 2e35354b61f5..ff75e928772a 100644
> > --- a/arch/arm/mach-pxa/cm-x300.c
> > +++ b/arch/arm/mach-pxa/cm-x300.c
> > @@ -531,7 +531,7 @@ static int cm_x300_u2d_init(struct device *dev)
> >  		pout_clk = clk_get(NULL, "CLK_POUT");
> >  		if (IS_ERR(pout_clk)) {
> >  			err = PTR_ERR(pout_clk);
> > -			pr_err("failed to get CLK_POUT: %d\n", err);
> > +			pr_err("failed to get CLK_POUT: %pe\n", err);
>
> It has to print the pointer and not the err.   So it should be:
>
> 			pr_err("failed to get CLK_POUT: %pe\n", pout_clk);

Oops!  Thanks.  That is easy to fix.

julia

>
> But really someone needs to introduce an %e (and everyone can see the
> need for %e but it's just a matter of finding someone who is feeling
> motivated).
>
> regards,
> dan carpenter
>
>

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Dan Carpenter]

On Mon, May 10, 2021 at 07:03:59AM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 10 May 2021, Dan Carpenter wrote:
> 
> > On Sun, May 09, 2021 at 11:17:42PM +0200, Julia Lawall wrote:
> > > ---------------------------
> > >
> > > Patch of a recent linux next:
> > >
> > > diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
> > > index 2e35354b61f5..ff75e928772a 100644
> > > --- a/arch/arm/mach-pxa/cm-x300.c
> > > +++ b/arch/arm/mach-pxa/cm-x300.c
> > > @@ -531,7 +531,7 @@ static int cm_x300_u2d_init(struct device *dev)
> > >  		pout_clk = clk_get(NULL, "CLK_POUT");
> > >  		if (IS_ERR(pout_clk)) {
> > >  			err = PTR_ERR(pout_clk);
> > > -			pr_err("failed to get CLK_POUT: %d\n", err);
> > > +			pr_err("failed to get CLK_POUT: %pe\n", err);
> >
> > It has to print the pointer and not the err.   So it should be:
> >
> > 			pr_err("failed to get CLK_POUT: %pe\n", pout_clk);
> 
> Oops!  Thanks.  That is easy to fix.

Yeah, but I would prefer to wait until %e exists and use that instead.

regards,
dan carpenter

Re: [RFC 1/2] rtc: max77686: use symbolic error messages

[Julia Lawall]

On Mon, 10 May 2021, Dan Carpenter wrote:

> On Mon, May 10, 2021 at 07:03:59AM +0200, Julia Lawall wrote:
> >
> >
> > On Mon, 10 May 2021, Dan Carpenter wrote:
> >
> > > On Sun, May 09, 2021 at 11:17:42PM +0200, Julia Lawall wrote:
> > > > ---------------------------
> > > >
> > > > Patch of a recent linux next:
> > > >
> > > > diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
> > > > index 2e35354b61f5..ff75e928772a 100644
> > > > --- a/arch/arm/mach-pxa/cm-x300.c
> > > > +++ b/arch/arm/mach-pxa/cm-x300.c
> > > > @@ -531,7 +531,7 @@ static int cm_x300_u2d_init(struct device *dev)
> > > >  		pout_clk = clk_get(NULL, "CLK_POUT");
> > > >  		if (IS_ERR(pout_clk)) {
> > > >  			err = PTR_ERR(pout_clk);
> > > > -			pr_err("failed to get CLK_POUT: %d\n", err);
> > > > +			pr_err("failed to get CLK_POUT: %pe\n", err);
> > >
> > > It has to print the pointer and not the err.   So it should be:
> > >
> > > 			pr_err("failed to get CLK_POUT: %pe\n", pout_clk);
> >
> > Oops!  Thanks.  That is easy to fix.
>
> Yeah, but I would prefer to wait until %e exists and use that instead.

OK.

thanks,
julia