2021-08-31 12:53:55

by zhaoxiao

[permalink] [raw]
Subject: [PATCH] pwm: rockchip: Simplify using devm_pwmchip_add()

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 <[email protected]>
---
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);
-
- return pwmchip_remove(&pc->chip);
-}
-
static struct platform_driver rockchip_pwm_driver = {
.driver = {
.name = "rockchip-pwm",
.of_match_table = rockchip_pwm_dt_ids,
},
.probe = rockchip_pwm_probe,
- .remove = rockchip_pwm_remove,
};
module_platform_driver(rockchip_pwm_driver);

--
2.20.1




2021-09-01 09:04:08

by Krzysztof Kozlowski

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

On Tue, 31 Aug 2021 at 14:52, zhaoxiao <[email protected]> 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 <[email protected]>
> ---
> 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