| Message ID | 20230717155257.2568627-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pwm: lpc32xx: remove handling of PWM channels | expand |
On Mon, 17 Jul 2023 17:52:57 +0200, Uwe Kleine-König wrote: > Because LPC32xx PWM controllers have only a single output which is > registered as the only PWM device/channel per controller, it is known in > advance that pwm->hwpwm value is always 0. On basis of this fact > simplify the code by removing operations with pwm->hwpwm, there is no > controls which require channel number as input. > > > [...] Applied, thanks! [1/1] pwm: lpc32xx: remove handling of PWM channels commit: 6894c2373e9fc8deaf26f9571f1e183c1ac91120 Best regards,
Hello Thierry, On Mon, Jul 24, 2023 at 04:00:32PM +0200, Uwe Kleine-König wrote: > As noted by Dan Carpenter in > https://lore.kernel.org/linux-pwm/919cac5d-491e-4534-baed-bf813181192d@moroto.mountain > lpc32xx->chip.pwms is NULL before devm_pwmchip_add() is called so this > patch fixes a null pointer exception. > > Maybe add the following to the commit log: > > Even though I wasn't aware at the time when I forward ported that patch, > this fixes a null pointer dereference as lpc32xx->chip.pwms is NULL > before devm_pwmchip_add() is called. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Fixes: 3d2813fb17e5 ("pwm: lpc32xx: Don't modify HW state in .probe() after the PWM chip was registered") You (probably with b4's help) picked up the Fixes line. The description and also the Reported-by was not picked up, though. IMHO adding the missed bits would be beneficial. Best regards Uwe
Hello Thierry, On Fri, Jul 28, 2023 at 11:23:29AM +0200, Uwe Kleine-König wrote: > On Mon, Jul 24, 2023 at 04:00:32PM +0200, Uwe Kleine-König wrote: > > As noted by Dan Carpenter in > > https://lore.kernel.org/linux-pwm/919cac5d-491e-4534-baed-bf813181192d@moroto.mountain > > lpc32xx->chip.pwms is NULL before devm_pwmchip_add() is called so this > > patch fixes a null pointer exception. > > > > Maybe add the following to the commit log: > > > > Even though I wasn't aware at the time when I forward ported that patch, > > this fixes a null pointer dereference as lpc32xx->chip.pwms is NULL > > before devm_pwmchip_add() is called. > > > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > > Fixes: 3d2813fb17e5 ("pwm: lpc32xx: Don't modify HW state in .probe() after the PWM chip was registered") > > You (probably with b4's help) picked up the Fixes line. The description > and also the Reported-by was not picked up, though. > > IMHO adding the missed bits would be beneficial. I'd consider it important that this is added before you send the PR to Linus for this cycle. Is this still on your radar? Best regards Uwe
On Wed, Sep 06, 2023 at 11:47:30AM +0200, Uwe Kleine-König wrote: > Hello Thierry, > > On Fri, Jul 28, 2023 at 11:23:29AM +0200, Uwe Kleine-König wrote: > > On Mon, Jul 24, 2023 at 04:00:32PM +0200, Uwe Kleine-König wrote: > > > As noted by Dan Carpenter in > > > https://lore.kernel.org/linux-pwm/919cac5d-491e-4534-baed-bf813181192d@moroto.mountain > > > lpc32xx->chip.pwms is NULL before devm_pwmchip_add() is called so this > > > patch fixes a null pointer exception. > > > > > > Maybe add the following to the commit log: > > > > > > Even though I wasn't aware at the time when I forward ported that patch, > > > this fixes a null pointer dereference as lpc32xx->chip.pwms is NULL > > > before devm_pwmchip_add() is called. > > > > > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > > > Fixes: 3d2813fb17e5 ("pwm: lpc32xx: Don't modify HW state in .probe() after the PWM chip was registered") > > > > You (probably with b4's help) picked up the Fixes line. The description > > and also the Reported-by was not picked up, though. > > > > IMHO adding the missed bits would be beneficial. > > I'd consider it important that this is added before you send the PR to > Linus for this cycle. Is this still on your radar? This has been in the for-next branch for a few weeks now. Can you check and let me know it's what you expect? I was planning on sending the PR today or tomorrow. Thierry
diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c index 86a0ea0f6955..806f0bb3ad6d 100644 --- a/drivers/pwm/pwm-lpc32xx.c +++ b/drivers/pwm/pwm-lpc32xx.c @@ -51,10 +51,10 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, if (duty_cycles > 255) duty_cycles = 255; - val = readl(lpc32xx->base + (pwm->hwpwm << 2)); + val = readl(lpc32xx->base); val &= ~0xFFFF; val |= (period_cycles << 8) | duty_cycles; - writel(val, lpc32xx->base + (pwm->hwpwm << 2)); + writel(val, lpc32xx->base); return 0; } @@ -69,9 +69,9 @@ static int lpc32xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) if (ret) return ret; - val = readl(lpc32xx->base + (pwm->hwpwm << 2)); + val = readl(lpc32xx->base); val |= PWM_ENABLE; - writel(val, lpc32xx->base + (pwm->hwpwm << 2)); + writel(val, lpc32xx->base); return 0; } @@ -81,9 +81,9 @@ static void lpc32xx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip); u32 val; - val = readl(lpc32xx->base + (pwm->hwpwm << 2)); + val = readl(lpc32xx->base); val &= ~PWM_ENABLE; - writel(val, lpc32xx->base + (pwm->hwpwm << 2)); + writel(val, lpc32xx->base); clk_disable_unprepare(lpc32xx->clk); } @@ -141,9 +141,9 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev) lpc32xx->chip.npwm = 1; /* If PWM is disabled, configure the output to the default value */ - val = readl(lpc32xx->base + (lpc32xx->chip.pwms[0].hwpwm << 2)); + val = readl(lpc32xx->base); val &= ~PWM_PIN_LEVEL; - writel(val, lpc32xx->base + (lpc32xx->chip.pwms[0].hwpwm << 2)); + writel(val, lpc32xx->base); ret = devm_pwmchip_add(&pdev->dev, &lpc32xx->chip); if (ret < 0) {