[PATCH v2] pwm: rockchip: Keep enabled PWMs running while probing

[Uwe Kleine-König]

Hello,

On Sat, Sep 19, 2020 at 03:33:06PM -0400, Simon South wrote:
> Following commit cfc4c189bc70 ("pwm: Read initial hardware state at
> request time") the Rockchip PWM driver can no longer assume a device's
> pwm_state structure has been populated after a call to pwmchip_add().
> Consequently, the test in rockchip_pwm_probe() intended to prevent the
> driver from stopping PWM devices already enabled by the bootloader no
> longer functions reliably and this can lead to the kernel hanging
> during startup, particularly on devices like the Pinebook Pro that use
> a PWM-controlled backlight for their display.
> 
> Avoid this by querying the device directly at probe time to determine
> whether or not it is enabled.
> 
> Fixes: cfc4c189bc70 ("pwm: Read initial hardware state at request time")
> Signed-off-by: Simon South <simon at simonsouth.net>
> ---
>  drivers/pwm/pwm-rockchip.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index eb8c9cb645a6..098e94335cb5 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -288,6 +288,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>  	const struct of_device_id *id;
>  	struct rockchip_pwm_chip *pc;
>  	struct resource *r;
> +	u32 enable_conf, ctrl;
>  	int ret, count;
>  
>  	id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
> @@ -362,7 +363,9 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Keep the PWM clk enabled if the PWM appears to be up and running. */
> -	if (!pwm_is_enabled(pc->chip.pwms))
> +	enable_conf = pc->data->enable_conf;
> +	ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
> +	if ((ctrl & enable_conf) != enable_conf)
>  		clk_disable(pc->clk);

In my book a pwm driver should never call pwm_get_state() (or its
wrappers).

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

I looked at the other drivers for similar problems. I found a few
issues, but they are all independently of cfc4c189bc70:

 - pwm-lpc18xx-sct.c does some allocation that the .request
   callback depends on. pwm-sti.c does allocation that the irq handler
   depends on quite late.
 - some drivers modify the hardware after pwmchip_add()
   [pwm-fsl-ftm.c, pwm-hibvt.c, pwm-lpc18xx-sct.c, pwm-lpc32xx.c,
   pwm-mtk-disp.c, pwm-mxs.c]
 - pwm-lpss.c, pwm-pca9685.c, pwm-tiehrpwm.c and cpwm-tiecap.c do some
   pm_runtime stuff which should better be done before pwmchip_add()?

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-rockchip/attachments/20200921/8edb0ece/attachment-0001.sig>