diff mbox series

[-next] clk: tegra: tegra124-emc: fix missing clk_disable_unprepare() on error in emc_set_timing()

Message ID 20210518044247.605370-1-yangyingliang@huawei.com
State Accepted
Headers show
Series [-next] clk: tegra: tegra124-emc: fix missing clk_disable_unprepare() on error in emc_set_timing() | expand

Commit Message

Yang Yingliang May 18, 2021, 4:42 a.m. UTC
After calling clk_prepare_enable(), clk_disable_unprepare() need
be called when prepare_timing_change() failed.

Fixes: 2db04f16b589 ("clk: tegra: Add EMC clock driver")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/clk/tegra/clk-tegra124-emc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Stephen Boyd June 2, 2021, 7:58 a.m. UTC | #1
Why does the subject have -next in it?

Quoting Yang Yingliang (2021-05-17 21:42:47)
> After calling clk_prepare_enable(), clk_disable_unprepare() need
> be called when prepare_timing_change() failed.
> 
> Fixes: 2db04f16b589 ("clk: tegra: Add EMC clock driver")

And then the Fixes tag is for a patch that was merged in v4.10?

> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/clk/tegra/clk-tegra124-emc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
> index bdf6f4a51617..74c1d894cca8 100644
> --- a/drivers/clk/tegra/clk-tegra124-emc.c
> +++ b/drivers/clk/tegra/clk-tegra124-emc.c
> @@ -249,8 +249,10 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
>         div = timing->parent_rate / (timing->rate / 2) - 2;
>  
>         err = tegra->prepare_timing_change(emc, timing->rate);
> -       if (err)
> +       if (err) {
> +               clk_disable_unprepare(timing->parent);
>                 return err;
> +       }
>  
>         spin_lock_irqsave(tegra->lock, flags);
>  

Looks correct to me. I assume Thierry will pick it up for the next merge
window.
Thierry Reding June 2, 2021, 9:06 a.m. UTC | #2
On Wed, Jun 02, 2021 at 12:58:07AM -0700, Stephen Boyd wrote:
> Why does the subject have -next in it?
> 
> Quoting Yang Yingliang (2021-05-17 21:42:47)
> > After calling clk_prepare_enable(), clk_disable_unprepare() need
> > be called when prepare_timing_change() failed.
> > 
> > Fixes: 2db04f16b589 ("clk: tegra: Add EMC clock driver")
> 
> And then the Fixes tag is for a patch that was merged in v4.10?

To be honest, I'm not sure it's worth backporting this. The probability
of this happening is very small and the worst that will happen is that
the parent clock may stay enabled even if it doesn't have to. However,
the parent clock for these is one of pll_p, pll_c or pll_m, all of which
are usually sourced from other peripherals.

Yes, it's nice to fix this, but I don't think it warrants a backport, so
I think I'll just drop the Fixes tag.

> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> > ---
> >  drivers/clk/tegra/clk-tegra124-emc.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
> > index bdf6f4a51617..74c1d894cca8 100644
> > --- a/drivers/clk/tegra/clk-tegra124-emc.c
> > +++ b/drivers/clk/tegra/clk-tegra124-emc.c
> > @@ -249,8 +249,10 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
> >         div = timing->parent_rate / (timing->rate / 2) - 2;
> >  
> >         err = tegra->prepare_timing_change(emc, timing->rate);
> > -       if (err)
> > +       if (err) {
> > +               clk_disable_unprepare(timing->parent);
> >                 return err;
> > +       }
> >  
> >         spin_lock_irqsave(tegra->lock, flags);
> >  
> 
> Looks correct to me. I assume Thierry will pick it up for the next merge
> window.

Yes, I can do that. I've got a couple of minor fixes queued already, so
this will fit right in.

Thierry
diff mbox series

Patch

diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
index bdf6f4a51617..74c1d894cca8 100644
--- a/drivers/clk/tegra/clk-tegra124-emc.c
+++ b/drivers/clk/tegra/clk-tegra124-emc.c
@@ -249,8 +249,10 @@  static int emc_set_timing(struct tegra_clk_emc *tegra,
 	div = timing->parent_rate / (timing->rate / 2) - 2;
 
 	err = tegra->prepare_timing_change(emc, timing->rate);
-	if (err)
+	if (err) {
+		clk_disable_unprepare(timing->parent);
 		return err;
+	}
 
 	spin_lock_irqsave(tegra->lock, flags);