[v1] gpio: dwapb: Don't print error on -EPROBE_DEFER

[PATCH] gpio: dwapb: Don't print error on -EPROBE_DEFER

Posted by Serge Semin 3 years, 10 months ago

Currently if the APB or Debounce clocks aren't yet ready to be requested
the DW GPIO driver will correctly handle that by deferring the probe
procedure, but the error is still printed to the system log. It needlessly
pollutes the log since there was no real error but a request to postpone
the clock request procedure since the clocks subsystem hasn't been fully
initialized yet. Let's fix that by using the dev_err_probe method to print
the APB/clock request error status. It will correctly handle the deferred
probe situation and print the error if it actually happens.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/gpio/gpio-dwapb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index b0f3aca61974..a51458be34a9 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -653,7 +653,7 @@ static int dwapb_get_clks(struct dwapb_gpio *gpio)
 	err = devm_clk_bulk_get_optional(gpio->dev, DWAPB_NR_CLOCKS,
 					 gpio->clks);
 	if (err) {
-		dev_err(gpio->dev, "Cannot get APB/Debounce clocks\n");
+		dev_err_probe(gpio->dev, err, "Cannot get APB/Debounce clocks\n");
 		return err;
 	}
 
-- 
2.35.1

Re: [PATCH] gpio: dwapb: Don't print error on -EPROBE_DEFER

Posted by Serge Semin 3 years, 10 months ago

On Fri, Jun 10, 2022 at 10:51:52AM +0300, Serge Semin wrote:
> Currently if the APB or Debounce clocks aren't yet ready to be requested
> the DW GPIO driver will correctly handle that by deferring the probe
> procedure, but the error is still printed to the system log. It needlessly
> pollutes the log since there was no real error but a request to postpone
> the clock request procedure since the clocks subsystem hasn't been fully
> initialized yet. Let's fix that by using the dev_err_probe method to print
> the APB/clock request error status. It will correctly handle the deferred
> probe situation and print the error if it actually happens.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  drivers/gpio/gpio-dwapb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index b0f3aca61974..a51458be34a9 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -653,7 +653,7 @@ static int dwapb_get_clks(struct dwapb_gpio *gpio)
>  	err = devm_clk_bulk_get_optional(gpio->dev, DWAPB_NR_CLOCKS,
>  					 gpio->clks);
>  	if (err) {

> -		dev_err(gpio->dev, "Cannot get APB/Debounce clocks\n");
> +		dev_err_probe(gpio->dev, err, "Cannot get APB/Debounce clocks\n");

As Philipp correctly pointed out here:
https://lore.kernel.org/lkml/20220610080103.10689-1-Sergey.Semin@baikalelectronics.ru/
This can be shortened out by directly returning a value returned by
the dev_err_probe() method. I'll fix that in v2.

-Sergey

>  		return err;
>  	}
>  
> -- 
> 2.35.1
>

[PATCH v2] gpio: dwapb: Don't print error on -EPROBE_DEFER

Posted by Serge Semin 3 years, 10 months ago

Currently if the APB or Debounce clocks aren't yet ready to be requested
the DW GPIO driver will correctly handle that by deferring the probe
procedure, but the error is still printed to the system log. It needlessly
pollutes the log since there was no real error but a request to postpone
the clock request procedure since the clocks subsystem hasn't been fully
initialized yet. Let's fix that by using the dev_err_probe method to print
the APB/clock request error status. It will correctly handle the deferred
probe situation and print the error if it actually happens.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Link: https://lore.kernel.org/linux-gpio/20220610075152.10214-1-Sergey.Semin@baikalelectronics.ru/
Changelog v2:
- Use the dev_err_probe() return value as the return status of the
  corresponding method. (@Philipp Zabel)
---
 drivers/gpio/gpio-dwapb.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index b0f3aca61974..9467d695a33e 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -652,10 +652,9 @@ static int dwapb_get_clks(struct dwapb_gpio *gpio)
 	gpio->clks[1].id = "db";
 	err = devm_clk_bulk_get_optional(gpio->dev, DWAPB_NR_CLOCKS,
 					 gpio->clks);
-	if (err) {
-		dev_err(gpio->dev, "Cannot get APB/Debounce clocks\n");
-		return err;
-	}
+	if (err)
+		return dev_err_probe(gpio->dev, err,
+				     "Cannot get APB/Debounce clocks\n");
 
 	err = clk_bulk_prepare_enable(DWAPB_NR_CLOCKS, gpio->clks);
 	if (err) {
-- 
2.35.1

Re: [PATCH v2] gpio: dwapb: Don't print error on -EPROBE_DEFER

Posted by Bartosz Golaszewski 3 years, 10 months ago

On Fri, Jun 10, 2022 at 12:45 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> Currently if the APB or Debounce clocks aren't yet ready to be requested
> the DW GPIO driver will correctly handle that by deferring the probe
> procedure, but the error is still printed to the system log. It needlessly
> pollutes the log since there was no real error but a request to postpone
> the clock request procedure since the clocks subsystem hasn't been fully
> initialized yet. Let's fix that by using the dev_err_probe method to print
> the APB/clock request error status. It will correctly handle the deferred
> probe situation and print the error if it actually happens.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> ---
>
> Link: https://lore.kernel.org/linux-gpio/20220610075152.10214-1-Sergey.Semin@baikalelectronics.ru/
> Changelog v2:
> - Use the dev_err_probe() return value as the return status of the
>   corresponding method. (@Philipp Zabel)
> ---
>  drivers/gpio/gpio-dwapb.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index b0f3aca61974..9467d695a33e 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -652,10 +652,9 @@ static int dwapb_get_clks(struct dwapb_gpio *gpio)
>         gpio->clks[1].id = "db";
>         err = devm_clk_bulk_get_optional(gpio->dev, DWAPB_NR_CLOCKS,
>                                          gpio->clks);
> -       if (err) {
> -               dev_err(gpio->dev, "Cannot get APB/Debounce clocks\n");
> -               return err;
> -       }
> +       if (err)
> +               return dev_err_probe(gpio->dev, err,
> +                                    "Cannot get APB/Debounce clocks\n");
>
>         err = clk_bulk_prepare_enable(DWAPB_NR_CLOCKS, gpio->clks);
>         if (err) {
> --
> 2.35.1
>

Applied, thanks!

Bart

Re: [PATCH v2] gpio: dwapb: Don't print error on -EPROBE_DEFER

Posted by Andy Shevchenko 3 years, 10 months ago

On Fri, Jun 10, 2022 at 01:45:00PM +0300, Serge Semin wrote:
> Currently if the APB or Debounce clocks aren't yet ready to be requested
> the DW GPIO driver will correctly handle that by deferring the probe
> procedure, but the error is still printed to the system log. It needlessly
> pollutes the log since there was no real error but a request to postpone
> the clock request procedure since the clocks subsystem hasn't been fully
> initialized yet. Let's fix that by using the dev_err_probe method to print
> the APB/clock request error status. It will correctly handle the deferred
> probe situation and print the error if it actually happens.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

With or without below nit-pick
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> ---
> 
> Link: https://lore.kernel.org/linux-gpio/20220610075152.10214-1-Sergey.Semin@baikalelectronics.ru/
> Changelog v2:
> - Use the dev_err_probe() return value as the return status of the
>   corresponding method. (@Philipp Zabel)
> ---
>  drivers/gpio/gpio-dwapb.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index b0f3aca61974..9467d695a33e 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -652,10 +652,9 @@ static int dwapb_get_clks(struct dwapb_gpio *gpio)
>  	gpio->clks[1].id = "db";
>  	err = devm_clk_bulk_get_optional(gpio->dev, DWAPB_NR_CLOCKS,
>  					 gpio->clks);
> -	if (err) {
> -		dev_err(gpio->dev, "Cannot get APB/Debounce clocks\n");
> -		return err;
> -	}
> +	if (err)
> +		return dev_err_probe(gpio->dev, err,
> +				     "Cannot get APB/Debounce clocks\n");

I would leave it on one line, checkpatch wouldn't complain even before 100
characters era.

>  
>  	err = clk_bulk_prepare_enable(DWAPB_NR_CLOCKS, gpio->clks);
>  	if (err) {
> -- 
> 2.35.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: dwapb: Don't print error on -EPROBE_DEFER

Posted by Serge Semin 3 years, 10 months ago

On Fri, Jun 10, 2022 at 02:13:51PM +0300, Andy Shevchenko wrote:
> On Fri, Jun 10, 2022 at 01:45:00PM +0300, Serge Semin wrote:
> > Currently if the APB or Debounce clocks aren't yet ready to be requested
> > the DW GPIO driver will correctly handle that by deferring the probe
> > procedure, but the error is still printed to the system log. It needlessly
> > pollutes the log since there was no real error but a request to postpone
> > the clock request procedure since the clocks subsystem hasn't been fully
> > initialized yet. Let's fix that by using the dev_err_probe method to print
> > the APB/clock request error status. It will correctly handle the deferred
> > probe situation and print the error if it actually happens.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 

> With or without below nit-pick
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thanks.

> 
> > ---
> > 
> > Link: https://lore.kernel.org/linux-gpio/20220610075152.10214-1-Sergey.Semin@baikalelectronics.ru/
> > Changelog v2:
> > - Use the dev_err_probe() return value as the return status of the
> >   corresponding method. (@Philipp Zabel)
> > ---
> >  drivers/gpio/gpio-dwapb.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> > index b0f3aca61974..9467d695a33e 100644
> > --- a/drivers/gpio/gpio-dwapb.c
> > +++ b/drivers/gpio/gpio-dwapb.c
> > @@ -652,10 +652,9 @@ static int dwapb_get_clks(struct dwapb_gpio *gpio)
> >  	gpio->clks[1].id = "db";
> >  	err = devm_clk_bulk_get_optional(gpio->dev, DWAPB_NR_CLOCKS,
> >  					 gpio->clks);
> > -	if (err) {
> > -		dev_err(gpio->dev, "Cannot get APB/Debounce clocks\n");
> > -		return err;
> > -	}
> > +	if (err)
> > +		return dev_err_probe(gpio->dev, err,
> > +				     "Cannot get APB/Debounce clocks\n");
>
 
> I would leave it on one line, checkpatch wouldn't complain even before 100
> characters era.

You are right, checkpatch won't complain but the 80-chars limit is
still preferred. In this case we can fulfill that requirement without
harm to the code readability.

-Sergey

> 
> >  
> >  	err = clk_bulk_prepare_enable(DWAPB_NR_CLOCKS, gpio->clks);
> >  	if (err) {
> > -- 
> > 2.35.1
> > 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>