| Message ID | 20220218215720.26241-1-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: imx: gpcv2: Fix clock disabling imbalance in error path | expand |
Hi Laurent, Thanks for catching this! Am Freitag, dem 18.02.2022 um 23:57 +0200 schrieb Laurent Pinchart: > The imx_pgc_power_down() starts by enabling the domain clocks, and thus > disables them in the error path. Commit 18c98573a4cf ("soc: imx: gpcv2: > add domain option to keep domain clocks enabled") made the clock enable > conditional, but forgot to add the same condition to the error path. > This can result in a clock enable/disable imbalance. Fix it. > > Fixes: 18c98573a4cf ("soc: imx: gpcv2: add domain option to keep domain clocks enabled") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/soc/imx/gpcv2.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c > index a7c92bdfc53b..295e465197df 100644 > --- a/drivers/soc/imx/gpcv2.c > +++ b/drivers/soc/imx/gpcv2.c > @@ -477,7 +477,8 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd) > return 0; > > out_clk_disable: > - clk_bulk_disable_unprepare(domain->num_clks, domain->clks); > + if (!domain->keep_clocks) > + clk_bulk_disable_unprepare(domain->num_clks, domain->clks); > > return ret; > }
On Fri, Feb 18, 2022 at 11:57:20PM +0200, Laurent Pinchart wrote: > The imx_pgc_power_down() starts by enabling the domain clocks, and thus > disables them in the error path. Commit 18c98573a4cf ("soc: imx: gpcv2: > add domain option to keep domain clocks enabled") made the clock enable > conditional, but forgot to add the same condition to the error path. > This can result in a clock enable/disable imbalance. Fix it. > > Fixes: 18c98573a4cf ("soc: imx: gpcv2: add domain option to keep domain clocks enabled") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Applied, thanks!
Hello Shawn, On Mon, Feb 21, 2022 at 01:59:29PM +0800, Shawn Guo wrote: > On Fri, Feb 18, 2022 at 11:57:20PM +0200, Laurent Pinchart wrote: > > The imx_pgc_power_down() starts by enabling the domain clocks, and thus > > disables them in the error path. Commit 18c98573a4cf ("soc: imx: gpcv2: > > add domain option to keep domain clocks enabled") made the clock enable > > conditional, but forgot to add the same condition to the error path. > > This can result in a clock enable/disable imbalance. Fix it. > > > > Fixes: 18c98573a4cf ("soc: imx: gpcv2: add domain option to keep domain clocks enabled") > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Applied, thanks! Thank you. I've noticed that Lucas' reviewed-by tag didn't end up in the commit in your imx/fixes branch. Depending on your rebase policy with that branch, you may want to ammend the commit.
On Mon, Feb 21, 2022 at 08:29:24AM +0200, Laurent Pinchart wrote: > Hello Shawn, > > On Mon, Feb 21, 2022 at 01:59:29PM +0800, Shawn Guo wrote: > > On Fri, Feb 18, 2022 at 11:57:20PM +0200, Laurent Pinchart wrote: > > > The imx_pgc_power_down() starts by enabling the domain clocks, and thus > > > disables them in the error path. Commit 18c98573a4cf ("soc: imx: gpcv2: > > > add domain option to keep domain clocks enabled") made the clock enable > > > conditional, but forgot to add the same condition to the error path. > > > This can result in a clock enable/disable imbalance. Fix it. > > > > > > Fixes: 18c98573a4cf ("soc: imx: gpcv2: add domain option to keep domain clocks enabled") > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Applied, thanks! > > Thank you. > > I've noticed that Lucas' reviewed-by tag didn't end up in the commit in > your imx/fixes branch. Depending on your rebase policy with that branch, > you may want to ammend the commit. Oops! Thanks for reminding, Laurent! Fixed. Shawn
diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index a7c92bdfc53b..295e465197df 100644 --- a/drivers/soc/imx/gpcv2.c +++ b/drivers/soc/imx/gpcv2.c @@ -477,7 +477,8 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd) return 0; out_clk_disable: - clk_bulk_disable_unprepare(domain->num_clks, domain->clks); + if (!domain->keep_clocks) + clk_bulk_disable_unprepare(domain->num_clks, domain->clks); return ret; }
The imx_pgc_power_down() starts by enabling the domain clocks, and thus disables them in the error path. Commit 18c98573a4cf ("soc: imx: gpcv2: add domain option to keep domain clocks enabled") made the clock enable conditional, but forgot to add the same condition to the error path. This can result in a clock enable/disable imbalance. Fix it. Fixes: 18c98573a4cf ("soc: imx: gpcv2: add domain option to keep domain clocks enabled") Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/soc/imx/gpcv2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)