[PATCH] pwm: lpc32xx: remove handling of PWM channels

[Uwe Kleine-König]

Hello,

On Mon, Jul 17, 2023 at 05:52:57PM +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.
> 
> Signed-off-by: Vladimir Zapolskiy <vz at mleia.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> ---
> Hello,
> 
> this is a patch that was submitted before by Vladimir in 2016[1]. I
> stumbled over hwpwm always being 0 while doing some restructuring of all
> pwm drivers. I thought this wasn't the first time I diagnosed this and
> while searching for such a patch by me, I found Vladimir's :-)
> 
> I added "only a" to the commit log and rebased to v6.5-rc1. I considered
> adding
> [ukl: improved wording of commit log and rebase to v6.5-rc1]
> to the commit log, but the adaptions seemed too trivial to me.
> 
> Best regards
> Uwe
> 
> [1] https://lore.kernel.org/linux-pwm/20161205014308.1741-2-vz@mleia.com
> 
>  drivers/pwm/pwm-lpc32xx.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> 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);

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 at linaro.org>
	Fixes: 3d2813fb17e5 ("pwm: lpc32xx: Don't modify HW state in .probe() after the PWM chip was registered")

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20230724/b3dfb076/attachment.sig>