diff mbox series

gpio: mvebu: drop pwm base assignment

Message ID 145383feecbe43f3bbd3e128143f7890f0314b3b.1649658220.git.baruch@tkos.co.il
State Accepted
Commit eaaad16a9b3f16b411c33404e17d4dc61cf69b16
Headers show
Series gpio: mvebu: drop pwm base assignment | expand

Commit Message

Baruch Siach April 11, 2022, 6:23 a.m. UTC
pwmchip_add() unconditionally assigns the base ID dynamically. Commit
f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
dropped all base assignment from drivers under drivers/pwm/. It missed
this driver. Fix that.

Fixes: f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/gpio/gpio-mvebu.c | 7 -------
 1 file changed, 7 deletions(-)

Comments

Thierry Reding April 22, 2022, 4:48 p.m. UTC | #1
On Mon, Apr 11, 2022 at 09:23:40AM +0300, Baruch Siach wrote:
> pwmchip_add() unconditionally assigns the base ID dynamically. Commit
> f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
> dropped all base assignment from drivers under drivers/pwm/. It missed
> this driver. Fix that.
> 
> Fixes: f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/gpio/gpio-mvebu.c | 7 -------
>  1 file changed, 7 deletions(-)

Linus, Bartosz,

I see that this was Cc'ed to linux-gpio but not to you guys, so I'm not
sure if you're aware of this. Given that this touches the PWM-specific
bits of this driver I could also pick this up into the PWM tree if you
don't mind.

Quoting in full in case you don't have this in your inboxes.

Thierry

> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 4c1f9e1091b7..a2c8dd329b31 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -871,13 +871,6 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>  	mvpwm->chip.dev = dev;
>  	mvpwm->chip.ops = &mvebu_pwm_ops;
>  	mvpwm->chip.npwm = mvchip->chip.ngpio;
> -	/*
> -	 * There may already be some PWM allocated, so we can't force
> -	 * mvpwm->chip.base to a fixed point like mvchip->chip.base.
> -	 * So, we let pwmchip_add() do the numbering and take the next free
> -	 * region.
> -	 */
> -	mvpwm->chip.base = -1;
>  
>  	spin_lock_init(&mvpwm->lock);
>  
> -- 
> 2.35.1
>
Linus Walleij April 22, 2022, 10:18 p.m. UTC | #2
On Fri, Apr 22, 2022 at 6:48 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> > -     /*
> > -      * There may already be some PWM allocated, so we can't force
> > -      * mvpwm->chip.base to a fixed point like mvchip->chip.base.
> > -      * So, we let pwmchip_add() do the numbering and take the next free
> > -      * region.
> > -      */
> > -     mvpwm->chip.base = -1;

I don't see why this is removed. I understand why the comment is removed
but all contemporary GPIO chips should use dynamic assignment of numbers
i.e. base = -1.

Yours,
Linus Walleij
Uwe Kleine-König April 23, 2022, 4:18 p.m. UTC | #3
On Sat, Apr 23, 2022 at 12:18:20AM +0200, Linus Walleij wrote:
> On Fri, Apr 22, 2022 at 6:48 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > > -     /*
> > > -      * There may already be some PWM allocated, so we can't force
> > > -      * mvpwm->chip.base to a fixed point like mvchip->chip.base.
> > > -      * So, we let pwmchip_add() do the numbering and take the next free
> > > -      * region.
> > > -      */
> > > -     mvpwm->chip.base = -1;
> 
> I don't see why this is removed. I understand why the comment is removed
> but all contemporary GPIO chips should use dynamic assignment of numbers
> i.e. base = -1.

This is an assignment to struct pwm_chip::base, not struct gpio_chip::base.

Best regards
Uwe
Linus Walleij April 24, 2022, 1:30 p.m. UTC | #4
On Sat, Apr 23, 2022 at 6:18 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Sat, Apr 23, 2022 at 12:18:20AM +0200, Linus Walleij wrote:
> > On Fri, Apr 22, 2022 at 6:48 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > > > -     /*
> > > > -      * There may already be some PWM allocated, so we can't force
> > > > -      * mvpwm->chip.base to a fixed point like mvchip->chip.base.
> > > > -      * So, we let pwmchip_add() do the numbering and take the next free
> > > > -      * region.
> > > > -      */
> > > > -     mvpwm->chip.base = -1;
> >
> > I don't see why this is removed. I understand why the comment is removed
> > but all contemporary GPIO chips should use dynamic assignment of numbers
> > i.e. base = -1.
>
> This is an assignment to struct pwm_chip::base, not struct gpio_chip::base.

Ah, how confusing.
If this is OK with Uwe:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Bartosz Golaszewski May 2, 2022, 8:47 a.m. UTC | #5
On Mon, Apr 11, 2022 at 8:25 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> pwmchip_add() unconditionally assigns the base ID dynamically. Commit
> f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
> dropped all base assignment from drivers under drivers/pwm/. It missed
> this driver. Fix that.
>
> Fixes: f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/gpio/gpio-mvebu.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 4c1f9e1091b7..a2c8dd329b31 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -871,13 +871,6 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>         mvpwm->chip.dev = dev;
>         mvpwm->chip.ops = &mvebu_pwm_ops;
>         mvpwm->chip.npwm = mvchip->chip.ngpio;
> -       /*
> -        * There may already be some PWM allocated, so we can't force
> -        * mvpwm->chip.base to a fixed point like mvchip->chip.base.
> -        * So, we let pwmchip_add() do the numbering and take the next free
> -        * region.
> -        */
> -       mvpwm->chip.base = -1;
>
>         spin_lock_init(&mvpwm->lock);
>
> --
> 2.35.1
>

Queued for fixes, thanks!

Bart
Baruch Siach May 2, 2022, 9:10 a.m. UTC | #6
Hi Bartosz,

On Mon, May 02 2022, Bartosz Golaszewski wrote:
> On Mon, Apr 11, 2022 at 8:25 AM Baruch Siach <baruch@tkos.co.il> wrote:
>>
>> pwmchip_add() unconditionally assigns the base ID dynamically. Commit
>> f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
>> dropped all base assignment from drivers under drivers/pwm/. It missed
>> this driver. Fix that.
>>
>> Fixes: f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

[...]

> Queued for fixes, thanks!

Thanks. I see it's in your tree (brgl/linux). Is that the main GPIO
development tree now? The MAINTAINERS entry for GPIO SUBSYSTEM currently
lists linusw/linux-gpio.

baruch
Bartosz Golaszewski May 2, 2022, 9:30 a.m. UTC | #7
On Mon, May 2, 2022 at 11:14 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Bartosz,
>
> On Mon, May 02 2022, Bartosz Golaszewski wrote:
> > On Mon, Apr 11, 2022 at 8:25 AM Baruch Siach <baruch@tkos.co.il> wrote:
> >>
> >> pwmchip_add() unconditionally assigns the base ID dynamically. Commit
> >> f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
> >> dropped all base assignment from drivers under drivers/pwm/. It missed
> >> this driver. Fix that.
> >>
> >> Fixes: f9a8ee8c8bcd1 ("pwm: Always allocate PWM chip base ID dynamically")
> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>
> [...]
>
> > Queued for fixes, thanks!
>
> Thanks. I see it's in your tree (brgl/linux). Is that the main GPIO
> development tree now? The MAINTAINERS entry for GPIO SUBSYSTEM currently
> lists linusw/linux-gpio.
>
> baruch

Thanks for spotting that! Yes, that's the main tree, I will send a fix.

Bart
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 4c1f9e1091b7..a2c8dd329b31 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -871,13 +871,6 @@  static int mvebu_pwm_probe(struct platform_device *pdev,
 	mvpwm->chip.dev = dev;
 	mvpwm->chip.ops = &mvebu_pwm_ops;
 	mvpwm->chip.npwm = mvchip->chip.ngpio;
-	/*
-	 * There may already be some PWM allocated, so we can't force
-	 * mvpwm->chip.base to a fixed point like mvchip->chip.base.
-	 * So, we let pwmchip_add() do the numbering and take the next free
-	 * region.
-	 */
-	mvpwm->chip.base = -1;
 
 	spin_lock_init(&mvpwm->lock);