diff mbox series

[v2,1/3] pwm: mtk_disp: clear the clock operations

Message ID 20210130141226.25357-2-jitao.shi@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Convert the mtk_disp driver to aotmic API | expand

Commit Message

Jitao Shi Jan. 30, 2021, 2:12 p.m. UTC
Remove the clk_prepare from mtk_disp_pwm_probe.
Remove the clk_unprepare from mtk_disp_pwm_remove.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
 drivers/pwm/pwm-mtk-disp.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

Comments

Thierry Reding Feb. 22, 2021, 2:29 p.m. UTC | #1
On Sat, Jan 30, 2021 at 10:12:24PM +0800, Jitao Shi wrote:
> Remove the clk_prepare from mtk_disp_pwm_probe.
> Remove the clk_unprepare from mtk_disp_pwm_remove.
> 
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/pwm/pwm-mtk-disp.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)

It's not clear *why* you're doing this change. It's already obvious from
the changes in this patch that you're removing the calls to
clk_prepare() and clk_unprepare(), so instead of duplicating that
information in the commit message, take this opportunity to describe why
this change is needed. Without any further context, this would seem to
just break operation of this chip because now these clocks are never
enabled in the first place.

Thierry
Jitao Shi May 26, 2021, 1:54 a.m. UTC | #2
On Mon, 2021-02-22 at 22:29 +0800, Thierry Reding wrote:
> On Sat, Jan 30, 2021 at 10:12:24PM +0800, Jitao Shi wrote:
> > Remove the clk_prepare from mtk_disp_pwm_probe.
> > Remove the clk_unprepare from mtk_disp_pwm_remove.
> > 
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > ---
> >  drivers/pwm/pwm-mtk-disp.c | 23 ++---------------------
> >  1 file changed, 2 insertions(+), 21 deletions(-)
> 
> It's not clear *why* you're doing this change. It's already obvious from
> the changes in this patch that you're removing the calls to
> clk_prepare() and clk_unprepare(), so instead of duplicating that
> information in the commit message, take this opportunity to describe why
> this change is needed. Without any further context, this would seem to
> just break operation of this chip because now these clocks are never
> enabled in the first place.
> 
> Thierry

Thanks for your review.

I'll update the commit message next version.

The original issue is the clocks doesn't be disabled when system enter
standby.

Then I adjust the the clock sequence. and Uwe subject convert thelegacy
APIs of .apply().
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 87c6b4bc5d43..21416a8b6b47 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -192,14 +192,6 @@  static int mtk_disp_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(mdp->clk_mm))
 		return PTR_ERR(mdp->clk_mm);
 
-	ret = clk_prepare(mdp->clk_main);
-	if (ret < 0)
-		return ret;
-
-	ret = clk_prepare(mdp->clk_mm);
-	if (ret < 0)
-		goto disable_clk_main;
-
 	mdp->chip.dev = &pdev->dev;
 	mdp->chip.ops = &mtk_disp_pwm_ops;
 	mdp->chip.base = -1;
@@ -208,7 +200,7 @@  static int mtk_disp_pwm_probe(struct platform_device *pdev)
 	ret = pwmchip_add(&mdp->chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
-		goto disable_clk_mm;
+		return ret;
 	}
 
 	platform_set_drvdata(pdev, mdp);
@@ -227,24 +219,13 @@  static int mtk_disp_pwm_probe(struct platform_device *pdev)
 	}
 
 	return 0;
-
-disable_clk_mm:
-	clk_unprepare(mdp->clk_mm);
-disable_clk_main:
-	clk_unprepare(mdp->clk_main);
-	return ret;
 }
 
 static int mtk_disp_pwm_remove(struct platform_device *pdev)
 {
 	struct mtk_disp_pwm *mdp = platform_get_drvdata(pdev);
-	int ret;
-
-	ret = pwmchip_remove(&mdp->chip);
-	clk_unprepare(mdp->clk_mm);
-	clk_unprepare(mdp->clk_main);
 
-	return ret;
+	return pwmchip_remove(&mdp->chip);
 }
 
 static const struct mtk_pwm_data mt2701_pwm_data = {