Re: [PATCH] pwm: rockchip: Simplify using devm_pwmchip_add()

From: Krzysztof Kozlowski
Date: Wed Sep 01 2021 - 04:58:19 EST


On Tue, 31 Aug 2021 at 14:52, zhaoxiao <zhaoxiao@xxxxxxxxxxxxx> wrote:
>
> With devm_pwmchip_add() we can drop pwmchip_remove() from the device
> remove callback. The latter can then go away, too and as this is the
> only user of platform_get_drvdata(), the respective call to
> platform_set_drvdata() can go, too.
>
> Signed-off-by: zhaoxiao <zhaoxiao@xxxxxxxxxxxxx>
> ---
> drivers/pwm/pwm-rockchip.c | 29 +----------------------------
> 1 file changed, 1 insertion(+), 28 deletions(-)
>
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index cbe900877724..c22856916e63 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -347,8 +347,6 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> goto err_clk;
> }
>
> - platform_set_drvdata(pdev, pc);
> -
> pc->data = id->data;
> pc->chip.dev = &pdev->dev;
> pc->chip.ops = &rockchip_pwm_ops;
> @@ -358,7 +356,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
> enabled = (ctrl & enable_conf) == enable_conf;
>
> - ret = pwmchip_add(&pc->chip);
> + ret = devm_pwmchip_add(&pdev->dev, &pc->chip);
> if (ret < 0) {
> dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
> goto err_pclk;
> @@ -380,37 +378,12 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> return ret;
> }
>
> -static int rockchip_pwm_remove(struct platform_device *pdev)
> -{
> - struct rockchip_pwm_chip *pc = platform_get_drvdata(pdev);
> -
> - /*
> - * Disable the PWM clk before unpreparing it if the PWM device is still
> - * running. This should only happen when the last PWM user left it
> - * enabled, or when nobody requested a PWM that was previously enabled
> - * by the bootloader.
> - *
> - * FIXME: Maybe the core should disable all PWM devices in
> - * pwmchip_remove(). In this case we'd only have to call
> - * clk_unprepare() after pwmchip_remove().
> - *
> - */
> - if (pwm_is_enabled(pc->chip.pwms))
> - clk_disable(pc->clk);
> -
> - clk_unprepare(pc->pclk);
> - clk_unprepare(pc->clk);

NAK, for the same reason as pwm_samsung.

Please test your patches.

Best regards,
Krzysztof