| Message ID | 145383feecbe43f3bbd3e128143f7890f0314b3b.1649658220.git.baruch@tkos.co.il |
|---|---|
| State | Accepted |
| Headers | show |
| Series | gpio: mvebu: drop pwm base assignment | expand |
Hello Baruch, 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> Thanks for catching this. Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> There is another one in drivers/staging/greybus/pwm.c, will send a patch shortly. Best regards Uwe
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 >
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
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
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
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
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
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 --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);
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(-)