[PATCH 0/3] pwm: Renesas R-Car and TPU fixes

[PATCH 0/3] pwm: Renesas R-Car and TPU fixes

[Geert Uytterhoeven]

	Hi all,

This patch series contains two Runtime PM-related fixes for the Renesas
R-Car and Timer Pulse Unit PWM drivers, and a small fix for the TPU
driver.

This has been tested on r8a7791/koelsch.

Thanks for your comments!

Geert Uytterhoeven (3):
  pwm: rcar: Fix late Runtime PM enablement
  pwm: renesas-tpu: Fix late Runtime PM enablement
  pwm: renesas-tpu: Drop confusing registered message

 drivers/pwm/pwm-rcar.c        | 10 +++++++---
 drivers/pwm/pwm-renesas-tpu.c | 11 ++++-------
 2 files changed, 11 insertions(+), 10 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 0/3] pwm: Renesas R-Car and TPU fixes

[Geert Uytterhoeven]

	Hi all,

This patch series contains two Runtime PM-related fixes for the Renesas
R-Car and Timer Pulse Unit PWM drivers, and a small fix for the TPU
driver.

This has been tested on r8a7791/koelsch.

Thanks for your comments!

Geert Uytterhoeven (3):
  pwm: rcar: Fix late Runtime PM enablement
  pwm: renesas-tpu: Fix late Runtime PM enablement
  pwm: renesas-tpu: Drop confusing registered message

 drivers/pwm/pwm-rcar.c        | 10 +++++++---
 drivers/pwm/pwm-renesas-tpu.c | 11 ++++-------
 2 files changed, 11 insertions(+), 10 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

[PATCH 1/3] pwm: rcar: Fix late Runtime PM enablement

[Geert Uytterhoeven]

Runtime PM should be enabled before calling pwmchip_add(), as PWM users
can appear immediately after the PWM chip has been added.
Likewise, Runtime PM should be disabled after the removal of the PWM
chip.

Fixes: ed6c1476bf7f16d5 ("pwm: Add support for R-Car PWM Timer")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pwm/pwm-rcar.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index 2685577b6dd45be7..7ab9eb6616d950cb 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -229,24 +229,28 @@ static int rcar_pwm_probe(struct platform_device *pdev)
 	rcar_pwm->chip.base = -1;
 	rcar_pwm->chip.npwm = 1;
 
+	pm_runtime_enable(&pdev->dev);
+
 	ret = pwmchip_add(&rcar_pwm->chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register PWM chip: %d\n", ret);
+		pm_runtime_disable(&pdev->dev);
 		return ret;
 	}
 
-	pm_runtime_enable(&pdev->dev);
-
 	return 0;
 }
 
 static int rcar_pwm_remove(struct platform_device *pdev)
 {
 	struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = pwmchip_remove(&rcar_pwm->chip);
 
 	pm_runtime_disable(&pdev->dev);
 
-	return pwmchip_remove(&rcar_pwm->chip);
+	return ret;
 }
 
 static const struct of_device_id rcar_pwm_of_table[] = {
-- 
2.17.1

[PATCH 2/3] pwm: renesas-tpu: Fix late Runtime PM enablement

[Geert Uytterhoeven]

Runtime PM should be enabled before calling pwmchip_add(), as PWM users
can appear immediately after the PWM chip has been added.
Likewise, Runtime PM should always be disabled after the removal of the
PWM chip, even if the latter failed.

Fixes: 99b82abb0a35b073 ("pwm: Add Renesas TPU PWM driver")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pwm/pwm-renesas-tpu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 4a855a21b782dea3..8032acc84161a9dd 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -415,16 +415,17 @@ static int tpu_probe(struct platform_device *pdev)
 	tpu->chip.base = -1;
 	tpu->chip.npwm = TPU_CHANNEL_MAX;
 
+	pm_runtime_enable(&pdev->dev);
+
 	ret = pwmchip_add(&tpu->chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register PWM chip\n");
+		pm_runtime_disable(&pdev->dev);
 		return ret;
 	}
 
 	dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
 
-	pm_runtime_enable(&pdev->dev);
-
 	return 0;
 }
 
@@ -434,12 +435,10 @@ static int tpu_remove(struct platform_device *pdev)
 	int ret;
 
 	ret = pwmchip_remove(&tpu->chip);
-	if (ret)
-		return ret;
 
 	pm_runtime_disable(&pdev->dev);
 
-	return 0;
+	return ret;
 }
 
 #ifdef CONFIG_OF
-- 
2.17.1

[PATCH 3/3] pwm: renesas-tpu: Drop confusing registered message

[Geert Uytterhoeven]

During device probe, the message

    TPU PWM -1 registered

is printed.

While the "-1" looks suspicious, it is perfectly normal for a device
instantiated from DT.

Remove the message, as there are no non-DT users left, and other drivers
don't print such messages neither.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pwm/pwm-renesas-tpu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 8032acc84161a9dd..81ad5a551455e4b8 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -424,8 +424,6 @@ static int tpu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
-
 	return 0;
 }
 
-- 
2.17.1

Re: [PATCH 1/3] pwm: rcar: Fix late Runtime PM enablement

[Uwe Kleine-König]

On Mon, Mar 16, 2020 at 11:32:14AM +0100, Geert Uytterhoeven wrote:
> Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> can appear immediately after the PWM chip has been added.
> Likewise, Runtime PM should be disabled after the removal of the PWM
> chip.
> 
> Fixes: ed6c1476bf7f16d5 ("pwm: Add support for R-Car PWM Timer")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pwm/pwm-rcar.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> index 2685577b6dd45be7..7ab9eb6616d950cb 100644
> --- a/drivers/pwm/pwm-rcar.c
> +++ b/drivers/pwm/pwm-rcar.c
> @@ -229,24 +229,28 @@ static int rcar_pwm_probe(struct platform_device *pdev)
>  	rcar_pwm->chip.base = -1;
>  	rcar_pwm->chip.npwm = 1;
>  
> +	pm_runtime_enable(&pdev->dev);
> +
>  	ret = pwmchip_add(&rcar_pwm->chip);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to register PWM chip: %d\n", ret);
> +		pm_runtime_disable(&pdev->dev);
>  		return ret;
>  	}
>  
> -	pm_runtime_enable(&pdev->dev);
> -

Wouldn't it be wiser to do the pm_runtime_enable in .request, or even in
.apply when enabled=true?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH 1/3] pwm: rcar: Fix late Runtime PM enablement

[Geert Uytterhoeven]

Hi Uwe,

On Mon, Mar 16, 2020 at 4:40 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Mar 16, 2020 at 11:32:14AM +0100, Geert Uytterhoeven wrote:
> > Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> > can appear immediately after the PWM chip has been added.
> > Likewise, Runtime PM should be disabled after the removal of the PWM
> > chip.
> >
> > Fixes: ed6c1476bf7f16d5 ("pwm: Add support for R-Car PWM Timer")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/pwm/pwm-rcar.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> > index 2685577b6dd45be7..7ab9eb6616d950cb 100644
> > --- a/drivers/pwm/pwm-rcar.c
> > +++ b/drivers/pwm/pwm-rcar.c
> > @@ -229,24 +229,28 @@ static int rcar_pwm_probe(struct platform_device *pdev)
> >       rcar_pwm->chip.base = -1;
> >       rcar_pwm->chip.npwm = 1;
> >
> > +     pm_runtime_enable(&pdev->dev);
> > +
> >       ret = pwmchip_add(&rcar_pwm->chip);
> >       if (ret < 0) {
> >               dev_err(&pdev->dev, "failed to register PWM chip: %d\n", ret);
> > +             pm_runtime_disable(&pdev->dev);
> >               return ret;
> >       }
> >
> > -     pm_runtime_enable(&pdev->dev);
> > -
>
> Wouldn't it be wiser to do the pm_runtime_enable in .request, or even in
> .apply when enabled=true?

Wouldn't that mean that the device cannot be powered down until the first
time a PWM is used?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] pwm: rcar: Fix late Runtime PM enablement

[Uwe Kleine-König]

Hello Geert,

On Mon, Mar 16, 2020 at 04:42:31PM +0100, Geert Uytterhoeven wrote:
> On Mon, Mar 16, 2020 at 4:40 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Mon, Mar 16, 2020 at 11:32:14AM +0100, Geert Uytterhoeven wrote:
> > > Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> > > can appear immediately after the PWM chip has been added.
> > > Likewise, Runtime PM should be disabled after the removal of the PWM
> > > chip.
> > >
> > > Fixes: ed6c1476bf7f16d5 ("pwm: Add support for R-Car PWM Timer")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  drivers/pwm/pwm-rcar.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> > > index 2685577b6dd45be7..7ab9eb6616d950cb 100644
> > > --- a/drivers/pwm/pwm-rcar.c
> > > +++ b/drivers/pwm/pwm-rcar.c
> > > @@ -229,24 +229,28 @@ static int rcar_pwm_probe(struct platform_device *pdev)
> > >       rcar_pwm->chip.base = -1;
> > >       rcar_pwm->chip.npwm = 1;
> > >
> > > +     pm_runtime_enable(&pdev->dev);
> > > +
> > >       ret = pwmchip_add(&rcar_pwm->chip);
> > >       if (ret < 0) {
> > >               dev_err(&pdev->dev, "failed to register PWM chip: %d\n", ret);
> > > +             pm_runtime_disable(&pdev->dev);
> > >               return ret;
> > >       }
> > >
> > > -     pm_runtime_enable(&pdev->dev);
> > > -
> >
> > Wouldn't it be wiser to do the pm_runtime_enable in .request, or even in
> > .apply when enabled=true?
> 
> Wouldn't that mean that the device cannot be powered down until the first
> time a PWM is used?

Ah, it seems I got the semantic of pm_runtime_enable() wrong. I confused
it with pm_runtime_get(). Now with that corrected your fix is obviously
right:

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH 2/3] pwm: renesas-tpu: Fix late Runtime PM enablement

[Uwe Kleine-König]

Hello Geert,

On Mon, Mar 16, 2020 at 11:32:15AM +0100, Geert Uytterhoeven wrote:
> Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> can appear immediately after the PWM chip has been added.
> Likewise, Runtime PM should always be disabled after the removal of the
> PWM chip, even if the latter failed.
> 
> Fixes: 99b82abb0a35b073 ("pwm: Add Renesas TPU PWM driver")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pwm/pwm-renesas-tpu.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> index 4a855a21b782dea3..8032acc84161a9dd 100644
> --- a/drivers/pwm/pwm-renesas-tpu.c
> +++ b/drivers/pwm/pwm-renesas-tpu.c
> @@ -415,16 +415,17 @@ static int tpu_probe(struct platform_device *pdev)
>  	tpu->chip.base = -1;
>  	tpu->chip.npwm = TPU_CHANNEL_MAX;
>  
> +	pm_runtime_enable(&pdev->dev);
> +
>  	ret = pwmchip_add(&tpu->chip);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to register PWM chip\n");
> +		pm_runtime_disable(&pdev->dev);
>  		return ret;
>  	}
>  
>  	dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
>  
> -	pm_runtime_enable(&pdev->dev);
> -
>  	return 0;
>  }
>  
> @@ -434,12 +435,10 @@ static int tpu_remove(struct platform_device *pdev)
>  	int ret;
>  
>  	ret = pwmchip_remove(&tpu->chip);
> -	if (ret)
> -		return ret;
>  
>  	pm_runtime_disable(&pdev->dev);
>  
> -	return 0;
> +	return ret;
>  }

Maybe I was a bit quick with my reply to the previous patch. I wonder if
it is right to call pm_runtime_disable if pwmchip_remove failed?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH 1/3] pwm: rcar: Fix late Runtime PM enablement

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Mon, Mar 16, 2020 at 11:32:14AM +0100, Geert Uytterhoeven wrote:
> Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> can appear immediately after the PWM chip has been added.
> Likewise, Runtime PM should be disabled after the removal of the PWM
> chip.
> 
> Fixes: ed6c1476bf7f16d5 ("pwm: Add support for R-Car PWM Timer")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/pwm/pwm-rcar.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> index 2685577b6dd45be7..7ab9eb6616d950cb 100644
> --- a/drivers/pwm/pwm-rcar.c
> +++ b/drivers/pwm/pwm-rcar.c
> @@ -229,24 +229,28 @@ static int rcar_pwm_probe(struct platform_device *pdev)
>  	rcar_pwm->chip.base = -1;
>  	rcar_pwm->chip.npwm = 1;
>  
> +	pm_runtime_enable(&pdev->dev);
> +
>  	ret = pwmchip_add(&rcar_pwm->chip);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to register PWM chip: %d\n", ret);
> +		pm_runtime_disable(&pdev->dev);
>  		return ret;
>  	}
>  
> -	pm_runtime_enable(&pdev->dev);
> -
>  	return 0;
>  }
>  
>  static int rcar_pwm_remove(struct platform_device *pdev)
>  {
>  	struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = pwmchip_remove(&rcar_pwm->chip);
>  
>  	pm_runtime_disable(&pdev->dev);
>  
> -	return pwmchip_remove(&rcar_pwm->chip);
> +	return ret;
>  }
>  
>  static const struct of_device_id rcar_pwm_of_table[] = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] pwm: renesas-tpu: Fix late Runtime PM enablement

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Mon, Mar 16, 2020 at 05:01:08PM +0100, Uwe Kleine-König wrote:
> On Mon, Mar 16, 2020 at 11:32:15AM +0100, Geert Uytterhoeven wrote:
> > Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> > can appear immediately after the PWM chip has been added.
> > Likewise, Runtime PM should always be disabled after the removal of the
> > PWM chip, even if the latter failed.
> > Fixes: 99b82abb0a35b073 ("pwm: Add Renesas TPU PWM driver")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/pwm/pwm-renesas-tpu.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> > index 4a855a21b782dea3..8032acc84161a9dd 100644
> > --- a/drivers/pwm/pwm-renesas-tpu.c
> > +++ b/drivers/pwm/pwm-renesas-tpu.c
> > @@ -415,16 +415,17 @@ static int tpu_probe(struct platform_device *pdev)
> >  	tpu->chip.base = -1;
> >  	tpu->chip.npwm = TPU_CHANNEL_MAX;
> >  
> > +	pm_runtime_enable(&pdev->dev);
> > +
> >  	ret = pwmchip_add(&tpu->chip);
> >  	if (ret < 0) {
> >  		dev_err(&pdev->dev, "failed to register PWM chip\n");
> > +		pm_runtime_disable(&pdev->dev);
> >  		return ret;
> >  	}
> >  
> >  	dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
> >  
> > -	pm_runtime_enable(&pdev->dev);
> > -
> >  	return 0;
> >  }

This part looks good to me.

> >  
> > @@ -434,12 +435,10 @@ static int tpu_remove(struct platform_device *pdev)
> >  	int ret;
> >  
> >  	ret = pwmchip_remove(&tpu->chip);
> > -	if (ret)
> > -		return ret;
> >  
> >  	pm_runtime_disable(&pdev->dev);
> >  
> > -	return 0;
> > +	return ret;
> >  }
> 
> Maybe I was a bit quick with my reply to the previous patch. I wonder if
> it is right to call pm_runtime_disable if pwmchip_remove failed?

It should at least be explained in the commit message why this is the
right thing to do.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] pwm: renesas-tpu: Drop confusing registered message

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Mon, Mar 16, 2020 at 11:32:16AM +0100, Geert Uytterhoeven wrote:
> During device probe, the message
> 
>     TPU PWM -1 registered
> 
> is printed.
> 
> While the "-1" looks suspicious, it is perfectly normal for a device
> instantiated from DT.
> 
> Remove the message, as there are no non-DT users left, and other drivers
> don't print such messages neither.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/pwm/pwm-renesas-tpu.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> index 8032acc84161a9dd..81ad5a551455e4b8 100644
> --- a/drivers/pwm/pwm-renesas-tpu.c
> +++ b/drivers/pwm/pwm-renesas-tpu.c
> @@ -424,8 +424,6 @@ static int tpu_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
> -
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] pwm: renesas-tpu: Fix late Runtime PM enablement

[Geert Uytterhoeven]

Hi Uwe,

On Mon, Mar 16, 2020 at 5:01 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Mar 16, 2020 at 11:32:15AM +0100, Geert Uytterhoeven wrote:
> > Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> > can appear immediately after the PWM chip has been added.
> > Likewise, Runtime PM should always be disabled after the removal of the
> > PWM chip, even if the latter failed.
> >
> > Fixes: 99b82abb0a35b073 ("pwm: Add Renesas TPU PWM driver")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/pwm/pwm-renesas-tpu.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> > index 4a855a21b782dea3..8032acc84161a9dd 100644
> > --- a/drivers/pwm/pwm-renesas-tpu.c
> > +++ b/drivers/pwm/pwm-renesas-tpu.c
> > @@ -415,16 +415,17 @@ static int tpu_probe(struct platform_device *pdev)
> >       tpu->chip.base = -1;
> >       tpu->chip.npwm = TPU_CHANNEL_MAX;
> >
> > +     pm_runtime_enable(&pdev->dev);
> > +
> >       ret = pwmchip_add(&tpu->chip);
> >       if (ret < 0) {
> >               dev_err(&pdev->dev, "failed to register PWM chip\n");
> > +             pm_runtime_disable(&pdev->dev);
> >               return ret;
> >       }
> >
> >       dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
> >
> > -     pm_runtime_enable(&pdev->dev);
> > -
> >       return 0;
> >  }
> >
> > @@ -434,12 +435,10 @@ static int tpu_remove(struct platform_device *pdev)
> >       int ret;
> >
> >       ret = pwmchip_remove(&tpu->chip);
> > -     if (ret)
> > -             return ret;
> >
> >       pm_runtime_disable(&pdev->dev);
> >
> > -     return 0;
> > +     return ret;
> >  }
>
> Maybe I was a bit quick with my reply to the previous patch. I wonder if
> it is right to call pm_runtime_disable if pwmchip_remove failed?

While the pwmchip may still exist, the hardware is unmapped and no
longer accessible.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] pwm: renesas-tpu: Fix late Runtime PM enablement

[Laurent Pinchart]

Hi Geert,

On Mon, Mar 16, 2020 at 05:06:12PM +0100, Geert Uytterhoeven wrote:
> On Mon, Mar 16, 2020 at 5:01 PM Uwe Kleine-König wrote:
> > On Mon, Mar 16, 2020 at 11:32:15AM +0100, Geert Uytterhoeven wrote:
> > > Runtime PM should be enabled before calling pwmchip_add(), as PWM users
> > > can appear immediately after the PWM chip has been added.
> > > Likewise, Runtime PM should always be disabled after the removal of the
> > > PWM chip, even if the latter failed.
> > >
> > > Fixes: 99b82abb0a35b073 ("pwm: Add Renesas TPU PWM driver")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  drivers/pwm/pwm-renesas-tpu.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> > > index 4a855a21b782dea3..8032acc84161a9dd 100644
> > > --- a/drivers/pwm/pwm-renesas-tpu.c
> > > +++ b/drivers/pwm/pwm-renesas-tpu.c
> > > @@ -415,16 +415,17 @@ static int tpu_probe(struct platform_device *pdev)
> > >       tpu->chip.base = -1;
> > >       tpu->chip.npwm = TPU_CHANNEL_MAX;
> > >
> > > +     pm_runtime_enable(&pdev->dev);
> > > +
> > >       ret = pwmchip_add(&tpu->chip);
> > >       if (ret < 0) {
> > >               dev_err(&pdev->dev, "failed to register PWM chip\n");
> > > +             pm_runtime_disable(&pdev->dev);
> > >               return ret;
> > >       }
> > >
> > >       dev_info(&pdev->dev, "TPU PWM %d registered\n", tpu->pdev->id);
> > >
> > > -     pm_runtime_enable(&pdev->dev);
> > > -
> > >       return 0;
> > >  }
> > >
> > > @@ -434,12 +435,10 @@ static int tpu_remove(struct platform_device *pdev)
> > >       int ret;
> > >
> > >       ret = pwmchip_remove(&tpu->chip);
> > > -     if (ret)
> > > -             return ret;
> > >
> > >       pm_runtime_disable(&pdev->dev);
> > >
> > > -     return 0;
> > > +     return ret;
> > >  }
> >
> > Maybe I was a bit quick with my reply to the previous patch. I wonder if
> > it is right to call pm_runtime_disable if pwmchip_remove failed?
> 
> While the pwmchip may still exist, the hardware is unmapped and no
> longer accessible.

Is it the case on module unload, doesn't a .remove() failure prevent the
module from being unloaded, and keeps the device associated with the
driver ? I haven't actually checked myself.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] pwm: renesas-tpu: Drop confusing registered message

[Uwe Kleine-König]

On Mon, Mar 16, 2020 at 11:32:16AM +0100, Geert Uytterhoeven wrote:
> During device probe, the message
> 
>     TPU PWM -1 registered
> 
> is printed.
> 
> While the "-1" looks suspicious, it is perfectly normal for a device
> instantiated from DT.
> 
> Remove the message, as there are no non-DT users left, and other drivers
> don't print such messages neither.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |