2022-09-29 16:11:44

by Johan Hovold

Subject: [PATCH] clk: qcom: gdsc: add missing error handling

Since commit 7eb231c337e0 ("PM / Domains: Convert pm_genpd_init() to
return an error code") pm_genpd_init() can return an error which the
caller must handle.

The current error handling was also incomplete as the runtime PM and
regulator use counts were not balanced in all error paths.

Add the missing error handling to the GDSC initialisation to avoid
continuing as if nothing happened on errors.

Signed-off-by: Johan Hovold <[email protected]>
---
drivers/clk/qcom/gdsc.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index e1d6574d2797..0b67bd72e217 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -451,11 +451,8 @@ static int gdsc_init(struct gdsc *sc)

/* ...and the power-domain */
ret = gdsc_pm_runtime_get(sc);
- if (ret) {
- if (sc->rsupply)
- regulator_disable(sc->rsupply);
- return ret;
- }
+ if (ret)
+ goto err_disable_supply;

/*
* Votable GDSCs can be ON due to Vote from other masters.
@@ -464,14 +461,14 @@ static int gdsc_init(struct gdsc *sc)
if (sc->flags & VOTABLE) {
ret = gdsc_update_collapse_bit(sc, false);
if (ret)
- return ret;
+ goto err_put_rpm;
}

/* Turn on HW trigger mode if supported */
if (sc->flags & HW_CTRL) {
ret = gdsc_hwctrl(sc, true);
if (ret < 0)
- return ret;
+ goto err_put_rpm;
}

/*
@@ -498,9 +495,21 @@ static int gdsc_init(struct gdsc *sc)
sc->pd.power_off = gdsc_disable;
if (!sc->pd.power_on)
sc->pd.power_on = gdsc_enable;
- pm_genpd_init(&sc->pd, NULL, !on);
+
+ ret = pm_genpd_init(&sc->pd, NULL, !on);
+ if (ret)
+ goto err_put_rpm;

return 0;
+
+err_put_rpm:
+ if (on)
+ gdsc_pm_runtime_put(sc);
+err_disable_supply:
+ if (on && sc->rsupply)
+ regulator_disable(sc->rsupply);
+
+ return ret;
}

int gdsc_register(struct gdsc_desc *desc,
--
2.35.1

2022-09-29 17:41:16

by Bjorn Andersson

Subject: Re: [PATCH] clk: qcom: gdsc: add missing error handling

On Thu, 29 Sep 2022 17:58:16 +0200, Johan Hovold wrote:
> Since commit 7eb231c337e0 ("PM / Domains: Convert pm_genpd_init() to
> return an error code") pm_genpd_init() can return an error which the
> caller must handle.
>
> The current error handling was also incomplete as the runtime PM and
> regulator use counts were not balanced in all error paths.
>
> [...]

Applied, thanks!

[1/1] clk: qcom: gdsc: add missing error handling
commit: eab4c1ebdd657957bf7ae66ffb8849b462db78b3

Best regards,
--
Bjorn Andersson <[email protected]>

2022-09-29 17:44:17

by Bjorn Andersson

Subject: Re: [PATCH] clk: qcom: gdsc: add missing error handling

On Thu, Sep 29, 2022 at 05:58:16PM +0200, Johan Hovold wrote:
> Since commit 7eb231c337e0 ("PM / Domains: Convert pm_genpd_init() to
> return an error code") pm_genpd_init() can return an error which the
> caller must handle.
>
> The current error handling was also incomplete as the runtime PM and
> regulator use counts were not balanced in all error paths.
>
> Add the missing error handling to the GDSC initialisation to avoid
> continuing as if nothing happened on errors.
>
> Signed-off-by: Johan Hovold <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/clk/qcom/gdsc.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index e1d6574d2797..0b67bd72e217 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -451,11 +451,8 @@ static int gdsc_init(struct gdsc *sc)
>
> /* ...and the power-domain */
> ret = gdsc_pm_runtime_get(sc);
> - if (ret) {
> - if (sc->rsupply)
> - regulator_disable(sc->rsupply);
> - return ret;
> - }
> + if (ret)
> + goto err_disable_supply;
>
> /*
> * Votable GDSCs can be ON due to Vote from other masters.
> @@ -464,14 +461,14 @@ static int gdsc_init(struct gdsc *sc)
> if (sc->flags & VOTABLE) {
> ret = gdsc_update_collapse_bit(sc, false);
> if (ret)
> - return ret;
> + goto err_put_rpm;
> }
>
> /* Turn on HW trigger mode if supported */
> if (sc->flags & HW_CTRL) {
> ret = gdsc_hwctrl(sc, true);
> if (ret < 0)
> - return ret;
> + goto err_put_rpm;
> }
>
> /*
> @@ -498,9 +495,21 @@ static int gdsc_init(struct gdsc *sc)
> sc->pd.power_off = gdsc_disable;
> if (!sc->pd.power_on)
> sc->pd.power_on = gdsc_enable;
> - pm_genpd_init(&sc->pd, NULL, !on);
> +
> + ret = pm_genpd_init(&sc->pd, NULL, !on);
> + if (ret)
> + goto err_put_rpm;
>
> return 0;
> +
> +err_put_rpm:
> + if (on)
> + gdsc_pm_runtime_put(sc);
> +err_disable_supply:
> + if (on && sc->rsupply)
> + regulator_disable(sc->rsupply);
> +
> + return ret;
> }
>
> int gdsc_register(struct gdsc_desc *desc,
> --
> 2.35.1
>