| Message ID | 20211117115101.28281-4-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | clk: renesas: trivial fixes | expand |
On Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Make sure we check the return value of pm_genpd_init() which might fail. > Also add a devres action to remove the power-domain in-case the probe > callback fails further down in the code flow. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-clk-for-v5.17. > @@ -574,7 +580,13 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev, > GENPD_FLAG_ACTIVE_WAKEUP; > genpd->attach_dev = cpg_mssr_attach_dev; > genpd->detach_dev = cpg_mssr_detach_dev; > - pm_genpd_init(genpd, &pm_domain_always_on_gov, false); > + ret = pm_genpd_init(genpd, &pm_domain_always_on_gov, false); > + if (ret) > + return ret; > + ret = devm_add_action_or_reset(dev, cpg_mssr_genpd_remove, genpd); Will insert a blank line here. > + if (ret) > + return ret; > + > cpg_mssr_clk_domain = pd; > > of_genpd_add_provider_simple(np, genpd); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 11/18/21 6:29 PM, Geert Uytterhoeven wrote: [...] >> Make sure we check the return value of pm_genpd_init() which might fail. >> Also add a devres action to remove the power-domain in-case the probe >> callback fails further down in the code flow. >> >> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > i.e. will queue in renesas-clk-for-v5.17. > >> @@ -574,7 +580,13 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev, >> GENPD_FLAG_ACTIVE_WAKEUP; >> genpd->attach_dev = cpg_mssr_attach_dev; >> genpd->detach_dev = cpg_mssr_detach_dev; >> - pm_genpd_init(genpd, &pm_domain_always_on_gov, false); >> + ret = pm_genpd_init(genpd, &pm_domain_always_on_gov, false); >> + if (ret) >> + return ret; >> + ret = devm_add_action_or_reset(dev, cpg_mssr_genpd_remove, genpd); > > Will insert a blank line here. You mean after *return*? Else I don't think we need an empty line. :-) > >> + if (ret) >> + return ret; >> + >> cpg_mssr_clk_domain = pd; >> >> of_genpd_add_provider_simple(np, genpd); > > Gr{oetje,eeting}s, > > Geert MBR, Sergey
Hi Sergey, On Thu, Nov 18, 2021 at 6:44 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > On 11/18/21 6:29 PM, Geert Uytterhoeven wrote: > > [...] > >> Make sure we check the return value of pm_genpd_init() which might fail. > >> Also add a devres action to remove the power-domain in-case the probe > >> callback fails further down in the code flow. > >> > >> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > i.e. will queue in renesas-clk-for-v5.17. > > > >> @@ -574,7 +580,13 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev, > >> GENPD_FLAG_ACTIVE_WAKEUP; > >> genpd->attach_dev = cpg_mssr_attach_dev; > >> genpd->detach_dev = cpg_mssr_detach_dev; > >> - pm_genpd_init(genpd, &pm_domain_always_on_gov, false); > >> + ret = pm_genpd_init(genpd, &pm_domain_always_on_gov, false); > >> + if (ret) > >> + return ret; > >> + ret = devm_add_action_or_reset(dev, cpg_mssr_genpd_remove, genpd); > > > > Will insert a blank line here. > > You mean after *return*? Else I don't think we need an empty line. :-) Meh, fortunately I'm better at making the actual code change ;-) > > > > >> + if (ret) > >> + return ret; > >> + > >> cpg_mssr_clk_domain = pd; > >> > >> of_genpd_add_provider_simple(np, genpd); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c index 21f762aa2131..f596a6438ac4 100644 --- a/drivers/clk/renesas/renesas-cpg-mssr.c +++ b/drivers/clk/renesas/renesas-cpg-mssr.c @@ -552,6 +552,11 @@ void cpg_mssr_detach_dev(struct generic_pm_domain *unused, struct device *dev) pm_clk_destroy(dev); } +static void cpg_mssr_genpd_remove(void *data) +{ + pm_genpd_remove(data); +} + static int __init cpg_mssr_add_clk_domain(struct device *dev, const unsigned int *core_pm_clks, unsigned int num_core_pm_clks) @@ -560,6 +565,7 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev, struct generic_pm_domain *genpd; struct cpg_mssr_clk_domain *pd; size_t pm_size = num_core_pm_clks * sizeof(core_pm_clks[0]); + int ret; pd = devm_kzalloc(dev, sizeof(*pd) + pm_size, GFP_KERNEL); if (!pd) @@ -574,7 +580,13 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev, GENPD_FLAG_ACTIVE_WAKEUP; genpd->attach_dev = cpg_mssr_attach_dev; genpd->detach_dev = cpg_mssr_detach_dev; - pm_genpd_init(genpd, &pm_domain_always_on_gov, false); + ret = pm_genpd_init(genpd, &pm_domain_always_on_gov, false); + if (ret) + return ret; + ret = devm_add_action_or_reset(dev, cpg_mssr_genpd_remove, genpd); + if (ret) + return ret; + cpg_mssr_clk_domain = pd; of_genpd_add_provider_simple(np, genpd);
Make sure we check the return value of pm_genpd_init() which might fail. Also add a devres action to remove the power-domain in-case the probe callback fails further down in the code flow. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- drivers/clk/renesas/renesas-cpg-mssr.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)