| Message ID | 20241211-b4-linux-next-24-11-18-clock-multiple-power-domains-v8-1-5d93cef910a4@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: qcom: Add support for multiple power-domains for a clock controller. | expand |
On 12/11/24 18:54, Bryan O'Donoghue wrote: > Adding a new clause to this if/else I noticed the existing usage of > pm_genpd_add_subdomain() wasn't capturing and returning the result code. > > pm_genpd_add_subdomain() returns an int and can fail. Capture that result > code and throw it up the call stack if something goes wrong. > > Fixes: 1b771839de05 ("clk: qcom: gdsc: enable optional power domain support") > Cc: stable@vger.kernel.org > Reviewed-by: Bjorn Andersson <andersson@kernel.org> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/clk/qcom/gdsc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index fa5fe4c2a2ee7786c2e8858f3e41301f639e5d59..4fc6f957d0b846cc90e50ef243f23a7a27e66899 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -555,9 +555,11 @@ int gdsc_register(struct gdsc_desc *desc, > if (!scs[i]) > continue; > if (scs[i]->parent) > - pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); > + ret = pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); > else if (!IS_ERR_OR_NULL(dev->pm_domain)) > - pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); > + ret = pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); > + if (ret) > + return ret; This is wrong, on the error path you shall rollback the registered subdomains in the reverse order. > } > > return of_genpd_add_provider_onecell(dev->of_node, data); > -- Best wishes, Vladimir
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index fa5fe4c2a2ee7786c2e8858f3e41301f639e5d59..4fc6f957d0b846cc90e50ef243f23a7a27e66899 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -555,9 +555,11 @@ int gdsc_register(struct gdsc_desc *desc, if (!scs[i]) continue; if (scs[i]->parent) - pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); + ret = pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); else if (!IS_ERR_OR_NULL(dev->pm_domain)) - pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); + ret = pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); + if (ret) + return ret; } return of_genpd_add_provider_onecell(dev->of_node, data);
Adding a new clause to this if/else I noticed the existing usage of pm_genpd_add_subdomain() wasn't capturing and returning the result code. pm_genpd_add_subdomain() returns an int and can fail. Capture that result code and throw it up the call stack if something goes wrong. Fixes: 1b771839de05 ("clk: qcom: gdsc: enable optional power domain support") Cc: stable@vger.kernel.org Reviewed-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/clk/qcom/gdsc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)