| Message ID | 20240820125840.9032-1-rongqianfeng@vivo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gpu: drm: Use devm_clk_get_enabled() helpers | expand |
在 2024/8/20 21:51, Chen-Yu Tsai 写道: > On Tue, Aug 20, 2024 at 8:59 PM Rong Qianfeng <rongqianfeng@vivo.com> wrote: >> Replace devm_clk_get() and clk_prepare_enable() with >> devm_clk_get_enabled() that also disables and unprepares it on >> driver detach. >> >> Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> >> --- >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 +++---------- >> drivers/gpu/drm/sun4i/sun6i_drc.c | 15 ++++----------- >> drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +++---------- >> 3 files changed, 10 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c >> index ab6c0c6cd0e2..057dceaf079e 100644 >> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c >> @@ -284,16 +284,11 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) >> return PTR_ERR(fsl_dev->regmap); >> } >> >> - fsl_dev->clk = devm_clk_get(dev, "dcu"); >> + fsl_dev->clk = devm_clk_get_enabled(dev, "dcu"); >> if (IS_ERR(fsl_dev->clk)) { >> dev_err(dev, "failed to get dcu clock\n"); >> return PTR_ERR(fsl_dev->clk); >> } >> - ret = clk_prepare_enable(fsl_dev->clk); >> - if (ret < 0) { >> - dev_err(dev, "failed to enable dcu clk\n"); >> - return ret; >> - } >> >> pix_clk_in = devm_clk_get(dev, "pix"); >> if (IS_ERR(pix_clk_in)) { >> @@ -311,8 +306,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) >> div_ratio_shift, 8, CLK_DIVIDER_ROUND_CLOSEST, NULL); >> if (IS_ERR(fsl_dev->pix_clk)) { >> dev_err(dev, "failed to register pix clk\n"); >> - ret = PTR_ERR(fsl_dev->pix_clk); >> - goto disable_clk; >> + return PTR_ERR(fsl_dev->pix_clk); >> } >> >> fsl_dev->tcon = fsl_tcon_init(dev); >> @@ -341,8 +335,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) >> drm_dev_put(drm); >> unregister_pix_clk: >> clk_unregister(fsl_dev->pix_clk); >> -disable_clk: >> - clk_disable_unprepare(fsl_dev->clk); >> + >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c >> index 0d342f43fa93..f263ad282828 100644 >> --- a/drivers/gpu/drm/sun4i/sun6i_drc.c >> +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c >> @@ -42,33 +42,28 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, >> return ret; >> } >> >> - drc->bus_clk = devm_clk_get(dev, "ahb"); >> + drc->bus_clk = devm_clk_get_enabled(dev, "ahb"); > devm_* is actually not the correct thing to use in this case, as this > is the component bind function, not the probe function. The lifetimes > are not the same. Thanks for your reminder, I didn't notice such a difference. > > We get away with devm_*_get because it's just a reference count, > but devm_*_get_enabled is going to be worse, because the clock > or whatever will not get disabled upon unbind. Same for sun8i_mixer. > > I just got bitten by this in an ASoC component driver, but the > problem is similar. Could you please post the log of the problem? > > > ChenYu > >> if (IS_ERR(drc->bus_clk)) { >> dev_err(dev, "Couldn't get our bus clock\n"); >> ret = PTR_ERR(drc->bus_clk); >> goto err_assert_reset; >> } >> - clk_prepare_enable(drc->bus_clk); >> >> - drc->mod_clk = devm_clk_get(dev, "mod"); >> + drc->mod_clk = devm_clk_get_enabled(dev, "mod"); >> if (IS_ERR(drc->mod_clk)) { >> dev_err(dev, "Couldn't get our mod clock\n"); >> ret = PTR_ERR(drc->mod_clk); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> ret = clk_set_rate_exclusive(drc->mod_clk, 300000000); >> if (ret) { >> dev_err(dev, "Couldn't set the module clock frequency\n"); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> - clk_prepare_enable(drc->mod_clk); >> - >> return 0; >> >> -err_disable_bus_clk: >> - clk_disable_unprepare(drc->bus_clk); >> err_assert_reset: >> reset_control_assert(drc->reset); >> return ret; >> @@ -80,8 +75,6 @@ static void sun6i_drc_unbind(struct device *dev, struct device *master, >> struct sun6i_drc *drc = dev_get_drvdata(dev); >> >> clk_rate_exclusive_put(drc->mod_clk); >> - clk_disable_unprepare(drc->mod_clk); >> - clk_disable_unprepare(drc->bus_clk); >> reset_control_assert(drc->reset); >> } >> >> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c >> index bd0fe2c6624e..ebf00676a76d 100644 >> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c >> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c >> @@ -507,19 +507,18 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, >> return ret; >> } >> >> - mixer->bus_clk = devm_clk_get(dev, "bus"); >> + mixer->bus_clk = devm_clk_get_enabled(dev, "bus"); >> if (IS_ERR(mixer->bus_clk)) { >> dev_err(dev, "Couldn't get the mixer bus clock\n"); >> ret = PTR_ERR(mixer->bus_clk); >> goto err_assert_reset; >> } >> - clk_prepare_enable(mixer->bus_clk); >> >> - mixer->mod_clk = devm_clk_get(dev, "mod"); >> + mixer->mod_clk = devm_clk_get_enabled(dev, "mod"); >> if (IS_ERR(mixer->mod_clk)) { >> dev_err(dev, "Couldn't get the mixer module clock\n"); >> ret = PTR_ERR(mixer->mod_clk); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> /* >> @@ -530,8 +529,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, >> if (mixer->cfg->mod_rate) >> clk_set_rate(mixer->mod_clk, mixer->cfg->mod_rate); >> >> - clk_prepare_enable(mixer->mod_clk); >> - >> list_add_tail(&mixer->engine.list, &drv->engine_list); >> >> base = sun8i_blender_base(mixer); >> @@ -592,8 +589,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, >> >> return 0; >> >> -err_disable_bus_clk: >> - clk_disable_unprepare(mixer->bus_clk); >> err_assert_reset: >> reset_control_assert(mixer->reset); >> return ret; >> @@ -606,8 +601,6 @@ static void sun8i_mixer_unbind(struct device *dev, struct device *master, >> >> list_del(&mixer->engine.list); >> >> - clk_disable_unprepare(mixer->mod_clk); >> - clk_disable_unprepare(mixer->bus_clk); >> reset_control_assert(mixer->reset); >> } >> >> -- >> 2.39.0 >>
Hi Rong, On Tue, Aug 20, 2024 at 10:59 PM Rong Qianfeng <rongqianfeng@vivo.com> wrote: > > Replace devm_clk_get() and clk_prepare_enable() with > devm_clk_get_enabled() that also disables and unprepares it on > driver detach. > > Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> > --- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 +++---------- > drivers/gpu/drm/sun4i/sun6i_drc.c | 15 ++++----------- > drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +++---------- > 3 files changed, 10 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c > index 0d342f43fa93..f263ad282828 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_drc.c > +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c > @@ -42,33 +42,28 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, > return ret; > } > > - drc->bus_clk = devm_clk_get(dev, "ahb"); > + drc->bus_clk = devm_clk_get_enabled(dev, "ahb"); > if (IS_ERR(drc->bus_clk)) { > dev_err(dev, "Couldn't get our bus clock\n"); > ret = PTR_ERR(drc->bus_clk); > goto err_assert_reset; > } > - clk_prepare_enable(drc->bus_clk); > > - drc->mod_clk = devm_clk_get(dev, "mod"); > + drc->mod_clk = devm_clk_get_enabled(dev, "mod"); > if (IS_ERR(drc->mod_clk)) { > dev_err(dev, "Couldn't get our mod clock\n"); > ret = PTR_ERR(drc->mod_clk); > - goto err_disable_bus_clk; > + goto err_assert_reset; > } > > ret = clk_set_rate_exclusive(drc->mod_clk, 300000000); > if (ret) { > dev_err(dev, "Couldn't set the module clock frequency\n"); > - goto err_disable_bus_clk; > + goto err_assert_reset; > } > > - clk_prepare_enable(drc->mod_clk); Am I reading this right: is this changing the init sequence so that the clock is enabled before setting a rate? This is the sort of thing that might cause glitches and issues in some hardware, so it'd be polite to test such a change on the actual hardware before posting it. Thanks,
在 2024/8/22 8:35, Julian Calaby 写道: > [Some people who received this message don't often get email from julian.calaby@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Hi Rong, > > On Tue, Aug 20, 2024 at 10:59 PM Rong Qianfeng <rongqianfeng@vivo.com> wrote: >> Replace devm_clk_get() and clk_prepare_enable() with >> devm_clk_get_enabled() that also disables and unprepares it on >> driver detach. >> >> Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> >> --- >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 +++---------- >> drivers/gpu/drm/sun4i/sun6i_drc.c | 15 ++++----------- >> drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +++---------- >> 3 files changed, 10 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c >> index 0d342f43fa93..f263ad282828 100644 >> --- a/drivers/gpu/drm/sun4i/sun6i_drc.c >> +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c >> @@ -42,33 +42,28 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, >> return ret; >> } >> >> - drc->bus_clk = devm_clk_get(dev, "ahb"); >> + drc->bus_clk = devm_clk_get_enabled(dev, "ahb"); >> if (IS_ERR(drc->bus_clk)) { >> dev_err(dev, "Couldn't get our bus clock\n"); >> ret = PTR_ERR(drc->bus_clk); >> goto err_assert_reset; >> } >> - clk_prepare_enable(drc->bus_clk); >> >> - drc->mod_clk = devm_clk_get(dev, "mod"); >> + drc->mod_clk = devm_clk_get_enabled(dev, "mod"); >> if (IS_ERR(drc->mod_clk)) { >> dev_err(dev, "Couldn't get our mod clock\n"); >> ret = PTR_ERR(drc->mod_clk); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> ret = clk_set_rate_exclusive(drc->mod_clk, 300000000); >> if (ret) { >> dev_err(dev, "Couldn't set the module clock frequency\n"); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> - clk_prepare_enable(drc->mod_clk); > Am I reading this right: is this changing the init sequence so that > the clock is enabled before setting a rate? > > This is the sort of thing that might cause glitches and issues in some > hardware, so it'd be polite to test such a change on the actual > hardware before posting it. Hi Julian , I have seen this used in other places, but the problem you raised may be different on different hardware. I wonder if anyone can clarify this? Best regards, Qianfeng > > Thanks, > > -- > Julian Calaby > > Email: julian.calaby@gmail.com > Profile: http://www.google.com/profiles/julian.calaby/
Hi Rong, On Tue, 20 Aug 2024 at 18:30, Rong Qianfeng <rongqianfeng@vivo.com> wrote: > > Replace devm_clk_get() and clk_prepare_enable() with > devm_clk_get_enabled() that also disables and unprepares it on > driver detach. > > Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> > --- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 +++---------- > drivers/gpu/drm/sun4i/sun6i_drc.c | 15 ++++----------- > drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +++---------- > 3 files changed, 10 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > index ab6c0c6cd0e2..057dceaf079e 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > @@ -284,16 +284,11 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) > return PTR_ERR(fsl_dev->regmap); > } > > - fsl_dev->clk = devm_clk_get(dev, "dcu"); > + fsl_dev->clk = devm_clk_get_enabled(dev, "dcu"); > if (IS_ERR(fsl_dev->clk)) { > dev_err(dev, "failed to get dcu clock\n"); > return PTR_ERR(fsl_dev->clk); You can use dev_err_probe it will be fine in all cases for clocks.to get enabled. return dev_err_probe(dev, PTR_ERR(fsl_dev->clk), "failed to get duc clock\n"); Thanks -Anand > } > - ret = clk_prepare_enable(fsl_dev->clk); > - if (ret < 0) { > - dev_err(dev, "failed to enable dcu clk\n"); > - return ret; > - } > > pix_clk_in = devm_clk_get(dev, "pix"); > if (IS_ERR(pix_clk_in)) { > @@ -311,8 +306,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) > div_ratio_shift, 8, CLK_DIVIDER_ROUND_CLOSEST, NULL); > if (IS_ERR(fsl_dev->pix_clk)) { > dev_err(dev, "failed to register pix clk\n"); > - ret = PTR_ERR(fsl_dev->pix_clk); > - goto disable_clk; > + return PTR_ERR(fsl_dev->pix_clk); > } > > fsl_dev->tcon = fsl_tcon_init(dev); > @@ -341,8 +335,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) > drm_dev_put(drm); > unregister_pix_clk: > clk_unregister(fsl_dev->pix_clk); > -disable_clk: > - clk_disable_unprepare(fsl_dev->clk); > + > return ret; > } > > diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c > index 0d342f43fa93..f263ad282828 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_drc.c > +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c > @@ -42,33 +42,28 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, > return ret; > } > > - drc->bus_clk = devm_clk_get(dev, "ahb"); > + drc->bus_clk = devm_clk_get_enabled(dev, "ahb"); > if (IS_ERR(drc->bus_clk)) { > dev_err(dev, "Couldn't get our bus clock\n"); > ret = PTR_ERR(drc->bus_clk); > goto err_assert_reset; > } > - clk_prepare_enable(drc->bus_clk); > > - drc->mod_clk = devm_clk_get(dev, "mod"); > + drc->mod_clk = devm_clk_get_enabled(dev, "mod"); > if (IS_ERR(drc->mod_clk)) { > dev_err(dev, "Couldn't get our mod clock\n"); > ret = PTR_ERR(drc->mod_clk); > - goto err_disable_bus_clk; > + goto err_assert_reset; > } > > ret = clk_set_rate_exclusive(drc->mod_clk, 300000000); > if (ret) { > dev_err(dev, "Couldn't set the module clock frequency\n"); > - goto err_disable_bus_clk; > + goto err_assert_reset; > } > > - clk_prepare_enable(drc->mod_clk); > - > return 0; > > -err_disable_bus_clk: > - clk_disable_unprepare(drc->bus_clk); > err_assert_reset: > reset_control_assert(drc->reset); > return ret; > @@ -80,8 +75,6 @@ static void sun6i_drc_unbind(struct device *dev, struct device *master, > struct sun6i_drc *drc = dev_get_drvdata(dev); > > clk_rate_exclusive_put(drc->mod_clk); > - clk_disable_unprepare(drc->mod_clk); > - clk_disable_unprepare(drc->bus_clk); > reset_control_assert(drc->reset); > } > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c > index bd0fe2c6624e..ebf00676a76d 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -507,19 +507,18 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, > return ret; > } > > - mixer->bus_clk = devm_clk_get(dev, "bus"); > + mixer->bus_clk = devm_clk_get_enabled(dev, "bus"); > if (IS_ERR(mixer->bus_clk)) { > dev_err(dev, "Couldn't get the mixer bus clock\n"); > ret = PTR_ERR(mixer->bus_clk); > goto err_assert_reset; > } > - clk_prepare_enable(mixer->bus_clk); > > - mixer->mod_clk = devm_clk_get(dev, "mod"); > + mixer->mod_clk = devm_clk_get_enabled(dev, "mod"); > if (IS_ERR(mixer->mod_clk)) { > dev_err(dev, "Couldn't get the mixer module clock\n"); > ret = PTR_ERR(mixer->mod_clk); > - goto err_disable_bus_clk; > + goto err_assert_reset; > } > > /* > @@ -530,8 +529,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, > if (mixer->cfg->mod_rate) > clk_set_rate(mixer->mod_clk, mixer->cfg->mod_rate); > > - clk_prepare_enable(mixer->mod_clk); > - > list_add_tail(&mixer->engine.list, &drv->engine_list); > > base = sun8i_blender_base(mixer); > @@ -592,8 +589,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, > > return 0; > > -err_disable_bus_clk: > - clk_disable_unprepare(mixer->bus_clk); > err_assert_reset: > reset_control_assert(mixer->reset); > return ret; > @@ -606,8 +601,6 @@ static void sun8i_mixer_unbind(struct device *dev, struct device *master, > > list_del(&mixer->engine.list); > > - clk_disable_unprepare(mixer->mod_clk); > - clk_disable_unprepare(mixer->bus_clk); > reset_control_assert(mixer->reset); > } > > -- > 2.39.0 > >
在 2024/8/22 13:32, Anand Moon 写道: > [Some people who received this message don't often get email from linux.amoon@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Hi Rong, > > On Tue, 20 Aug 2024 at 18:30, Rong Qianfeng <rongqianfeng@vivo.com> wrote: >> Replace devm_clk_get() and clk_prepare_enable() with >> devm_clk_get_enabled() that also disables and unprepares it on >> driver detach. >> >> Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> >> --- >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 +++---------- >> drivers/gpu/drm/sun4i/sun6i_drc.c | 15 ++++----------- >> drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +++---------- >> 3 files changed, 10 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c >> index ab6c0c6cd0e2..057dceaf079e 100644 >> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c >> @@ -284,16 +284,11 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) >> return PTR_ERR(fsl_dev->regmap); >> } >> >> - fsl_dev->clk = devm_clk_get(dev, "dcu"); >> + fsl_dev->clk = devm_clk_get_enabled(dev, "dcu"); >> if (IS_ERR(fsl_dev->clk)) { >> dev_err(dev, "failed to get dcu clock\n"); >> return PTR_ERR(fsl_dev->clk); > You can use dev_err_probe it will be fine in all cases for clocks.to > get enabled. > return dev_err_probe(dev, PTR_ERR(fsl_dev->clk), > "failed to get duc clock\n"); Good point, Thanks, I'll send v2 shortly. Best regards, Qianfeng > > Thanks > -Anand >> } >> - ret = clk_prepare_enable(fsl_dev->clk); >> - if (ret < 0) { >> - dev_err(dev, "failed to enable dcu clk\n"); >> - return ret; >> - } >> >> pix_clk_in = devm_clk_get(dev, "pix"); >> if (IS_ERR(pix_clk_in)) { >> @@ -311,8 +306,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) >> div_ratio_shift, 8, CLK_DIVIDER_ROUND_CLOSEST, NULL); >> if (IS_ERR(fsl_dev->pix_clk)) { >> dev_err(dev, "failed to register pix clk\n"); >> - ret = PTR_ERR(fsl_dev->pix_clk); >> - goto disable_clk; >> + return PTR_ERR(fsl_dev->pix_clk); >> } >> >> fsl_dev->tcon = fsl_tcon_init(dev); >> @@ -341,8 +335,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) >> drm_dev_put(drm); >> unregister_pix_clk: >> clk_unregister(fsl_dev->pix_clk); >> -disable_clk: >> - clk_disable_unprepare(fsl_dev->clk); >> + >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c >> index 0d342f43fa93..f263ad282828 100644 >> --- a/drivers/gpu/drm/sun4i/sun6i_drc.c >> +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c >> @@ -42,33 +42,28 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, >> return ret; >> } >> >> - drc->bus_clk = devm_clk_get(dev, "ahb"); >> + drc->bus_clk = devm_clk_get_enabled(dev, "ahb"); >> if (IS_ERR(drc->bus_clk)) { >> dev_err(dev, "Couldn't get our bus clock\n"); >> ret = PTR_ERR(drc->bus_clk); >> goto err_assert_reset; >> } >> - clk_prepare_enable(drc->bus_clk); >> >> - drc->mod_clk = devm_clk_get(dev, "mod"); >> + drc->mod_clk = devm_clk_get_enabled(dev, "mod"); >> if (IS_ERR(drc->mod_clk)) { >> dev_err(dev, "Couldn't get our mod clock\n"); >> ret = PTR_ERR(drc->mod_clk); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> ret = clk_set_rate_exclusive(drc->mod_clk, 300000000); >> if (ret) { >> dev_err(dev, "Couldn't set the module clock frequency\n"); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> - clk_prepare_enable(drc->mod_clk); >> - >> return 0; >> >> -err_disable_bus_clk: >> - clk_disable_unprepare(drc->bus_clk); >> err_assert_reset: >> reset_control_assert(drc->reset); >> return ret; >> @@ -80,8 +75,6 @@ static void sun6i_drc_unbind(struct device *dev, struct device *master, >> struct sun6i_drc *drc = dev_get_drvdata(dev); >> >> clk_rate_exclusive_put(drc->mod_clk); >> - clk_disable_unprepare(drc->mod_clk); >> - clk_disable_unprepare(drc->bus_clk); >> reset_control_assert(drc->reset); >> } >> >> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c >> index bd0fe2c6624e..ebf00676a76d 100644 >> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c >> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c >> @@ -507,19 +507,18 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, >> return ret; >> } >> >> - mixer->bus_clk = devm_clk_get(dev, "bus"); >> + mixer->bus_clk = devm_clk_get_enabled(dev, "bus"); >> if (IS_ERR(mixer->bus_clk)) { >> dev_err(dev, "Couldn't get the mixer bus clock\n"); >> ret = PTR_ERR(mixer->bus_clk); >> goto err_assert_reset; >> } >> - clk_prepare_enable(mixer->bus_clk); >> >> - mixer->mod_clk = devm_clk_get(dev, "mod"); >> + mixer->mod_clk = devm_clk_get_enabled(dev, "mod"); >> if (IS_ERR(mixer->mod_clk)) { >> dev_err(dev, "Couldn't get the mixer module clock\n"); >> ret = PTR_ERR(mixer->mod_clk); >> - goto err_disable_bus_clk; >> + goto err_assert_reset; >> } >> >> /* >> @@ -530,8 +529,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, >> if (mixer->cfg->mod_rate) >> clk_set_rate(mixer->mod_clk, mixer->cfg->mod_rate); >> >> - clk_prepare_enable(mixer->mod_clk); >> - >> list_add_tail(&mixer->engine.list, &drv->engine_list); >> >> base = sun8i_blender_base(mixer); >> @@ -592,8 +589,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, >> >> return 0; >> >> -err_disable_bus_clk: >> - clk_disable_unprepare(mixer->bus_clk); >> err_assert_reset: >> reset_control_assert(mixer->reset); >> return ret; >> @@ -606,8 +601,6 @@ static void sun8i_mixer_unbind(struct device *dev, struct device *master, >> >> list_del(&mixer->engine.list); >> >> - clk_disable_unprepare(mixer->mod_clk); >> - clk_disable_unprepare(mixer->bus_clk); >> reset_control_assert(mixer->reset); >> } >> >> -- >> 2.39.0 >> >>
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index ab6c0c6cd0e2..057dceaf079e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -284,16 +284,11 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) return PTR_ERR(fsl_dev->regmap); } - fsl_dev->clk = devm_clk_get(dev, "dcu"); + fsl_dev->clk = devm_clk_get_enabled(dev, "dcu"); if (IS_ERR(fsl_dev->clk)) { dev_err(dev, "failed to get dcu clock\n"); return PTR_ERR(fsl_dev->clk); } - ret = clk_prepare_enable(fsl_dev->clk); - if (ret < 0) { - dev_err(dev, "failed to enable dcu clk\n"); - return ret; - } pix_clk_in = devm_clk_get(dev, "pix"); if (IS_ERR(pix_clk_in)) { @@ -311,8 +306,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) div_ratio_shift, 8, CLK_DIVIDER_ROUND_CLOSEST, NULL); if (IS_ERR(fsl_dev->pix_clk)) { dev_err(dev, "failed to register pix clk\n"); - ret = PTR_ERR(fsl_dev->pix_clk); - goto disable_clk; + return PTR_ERR(fsl_dev->pix_clk); } fsl_dev->tcon = fsl_tcon_init(dev); @@ -341,8 +335,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) drm_dev_put(drm); unregister_pix_clk: clk_unregister(fsl_dev->pix_clk); -disable_clk: - clk_disable_unprepare(fsl_dev->clk); + return ret; } diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c index 0d342f43fa93..f263ad282828 100644 --- a/drivers/gpu/drm/sun4i/sun6i_drc.c +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c @@ -42,33 +42,28 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, return ret; } - drc->bus_clk = devm_clk_get(dev, "ahb"); + drc->bus_clk = devm_clk_get_enabled(dev, "ahb"); if (IS_ERR(drc->bus_clk)) { dev_err(dev, "Couldn't get our bus clock\n"); ret = PTR_ERR(drc->bus_clk); goto err_assert_reset; } - clk_prepare_enable(drc->bus_clk); - drc->mod_clk = devm_clk_get(dev, "mod"); + drc->mod_clk = devm_clk_get_enabled(dev, "mod"); if (IS_ERR(drc->mod_clk)) { dev_err(dev, "Couldn't get our mod clock\n"); ret = PTR_ERR(drc->mod_clk); - goto err_disable_bus_clk; + goto err_assert_reset; } ret = clk_set_rate_exclusive(drc->mod_clk, 300000000); if (ret) { dev_err(dev, "Couldn't set the module clock frequency\n"); - goto err_disable_bus_clk; + goto err_assert_reset; } - clk_prepare_enable(drc->mod_clk); - return 0; -err_disable_bus_clk: - clk_disable_unprepare(drc->bus_clk); err_assert_reset: reset_control_assert(drc->reset); return ret; @@ -80,8 +75,6 @@ static void sun6i_drc_unbind(struct device *dev, struct device *master, struct sun6i_drc *drc = dev_get_drvdata(dev); clk_rate_exclusive_put(drc->mod_clk); - clk_disable_unprepare(drc->mod_clk); - clk_disable_unprepare(drc->bus_clk); reset_control_assert(drc->reset); } diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bd0fe2c6624e..ebf00676a76d 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -507,19 +507,18 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, return ret; } - mixer->bus_clk = devm_clk_get(dev, "bus"); + mixer->bus_clk = devm_clk_get_enabled(dev, "bus"); if (IS_ERR(mixer->bus_clk)) { dev_err(dev, "Couldn't get the mixer bus clock\n"); ret = PTR_ERR(mixer->bus_clk); goto err_assert_reset; } - clk_prepare_enable(mixer->bus_clk); - mixer->mod_clk = devm_clk_get(dev, "mod"); + mixer->mod_clk = devm_clk_get_enabled(dev, "mod"); if (IS_ERR(mixer->mod_clk)) { dev_err(dev, "Couldn't get the mixer module clock\n"); ret = PTR_ERR(mixer->mod_clk); - goto err_disable_bus_clk; + goto err_assert_reset; } /* @@ -530,8 +529,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, if (mixer->cfg->mod_rate) clk_set_rate(mixer->mod_clk, mixer->cfg->mod_rate); - clk_prepare_enable(mixer->mod_clk); - list_add_tail(&mixer->engine.list, &drv->engine_list); base = sun8i_blender_base(mixer); @@ -592,8 +589,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, return 0; -err_disable_bus_clk: - clk_disable_unprepare(mixer->bus_clk); err_assert_reset: reset_control_assert(mixer->reset); return ret; @@ -606,8 +601,6 @@ static void sun8i_mixer_unbind(struct device *dev, struct device *master, list_del(&mixer->engine.list); - clk_disable_unprepare(mixer->mod_clk); - clk_disable_unprepare(mixer->bus_clk); reset_control_assert(mixer->reset); }
Replace devm_clk_get() and clk_prepare_enable() with devm_clk_get_enabled() that also disables and unprepares it on driver detach. Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 +++---------- drivers/gpu/drm/sun4i/sun6i_drc.c | 15 ++++----------- drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 +++---------- 3 files changed, 10 insertions(+), 31 deletions(-)