Hi Geert,
These are some sanity patches to remove power domain on probe failure,
said that the system might be not in a bootable state when such case
is hit.
Note: I have just compile tested for renesas-cpg-mssr driver.
Cheers,
Prabhakar
Lad Prabhakar (4):
clk: renesas: rzg2l: Check return value of pm_genpd_init()
clk: renesas: rzg2l: propagate return value
of_genpd_add_provider_simple()
clk: renesas: cpg-mssr: Check return value of pm_genpd_init()
clk: renesas: cpg-mssr: propagate return value
of_genpd_add_provider_simple()
drivers/clk/renesas/renesas-cpg-mssr.c | 17 ++++++++++++++---
drivers/clk/renesas/rzg2l-cpg.c | 17 ++++++++++++++---
2 files changed, 28 insertions(+), 6 deletions(-)
--
2.17.1
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.
Fixes: ef3c613ccd68a ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/clk/renesas/rzg2l-cpg.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index f9dfee14a33e..b0cffc8b22c0 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -851,10 +851,16 @@ static void rzg2l_cpg_detach_dev(struct generic_pm_domain *unused, struct device
pm_clk_destroy(dev);
}
+static void rzg2l_cpg_genpd_remove(void *data)
+{
+ pm_genpd_remove(data);
+}
+
static int __init rzg2l_cpg_add_clk_domain(struct device *dev)
{
struct device_node *np = dev->of_node;
struct generic_pm_domain *genpd;
+ int ret;
genpd = devm_kzalloc(dev, sizeof(*genpd), GFP_KERNEL);
if (!genpd)
@@ -865,7 +871,13 @@ static int __init rzg2l_cpg_add_clk_domain(struct device *dev)
GENPD_FLAG_ACTIVE_WAKEUP;
genpd->attach_dev = rzg2l_cpg_attach_dev;
genpd->detach_dev = rzg2l_cpg_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, rzg2l_cpg_genpd_remove, genpd);
+ if (ret)
+ return ret;
of_genpd_add_provider_simple(np, genpd);
return 0;
--
2.17.1
of_genpd_add_provider_simple() might fail, this patch makes sure we check
the return value of of_genpd_add_provider_simple() by propagating the
return value to the caller of rzg2l_cpg_add_clk_domain().
Fixes: ef3c613ccd68a ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/clk/renesas/rzg2l-cpg.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index b0cffc8b22c0..393e5560c076 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -879,8 +879,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct device *dev)
if (ret)
return ret;
- of_genpd_add_provider_simple(np, genpd);
- return 0;
+ return of_genpd_add_provider_simple(np, genpd);
}
static int __init rzg2l_cpg_probe(struct platform_device *pdev)
--
2.17.1
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 <[email protected]>
---
drivers/clk/renesas/renesas-cpg-mssr.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
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);
--
2.17.1
of_genpd_add_provider_simple() might fail, this patch makes sure we check
the return value of of_genpd_add_provider_simple() by propagating the
return value to the caller of cpg_mssr_add_clk_domain().
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/clk/renesas/renesas-cpg-mssr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index f596a6438ac4..dde0693d2f3c 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -589,8 +589,7 @@ static int __init cpg_mssr_add_clk_domain(struct device *dev,
cpg_mssr_clk_domain = pd;
- of_genpd_add_provider_simple(np, genpd);
- return 0;
+ return of_genpd_add_provider_simple(np, genpd);
}
#ifdef CONFIG_RESET_CONTROLLER
--
2.17.1
On Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar
<[email protected]> 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.
>
> Fixes: ef3c613ccd68a ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
> Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar
<[email protected]> wrote:
> of_genpd_add_provider_simple() might fail, this patch makes sure we check
> the return value of of_genpd_add_provider_simple() by propagating the
> return value to the caller of rzg2l_cpg_add_clk_domain().
>
> Fixes: ef3c613ccd68a ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
> Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Thu, Nov 18, 2021 at 4:26 PM Geert Uytterhoeven <[email protected]> wrote:
> On Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar
> <[email protected]> wrote:
> > of_genpd_add_provider_simple() might fail, this patch makes sure we check
> > the return value of of_genpd_add_provider_simple() by propagating the
> > return value to the caller of rzg2l_cpg_add_clk_domain().
> >
> > Fixes: ef3c613ccd68a ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
i.e. will queue in renesas-clk-for-v5.17.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Thu, Nov 18, 2021 at 4:25 PM Geert Uytterhoeven <[email protected]> wrote:
> On Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar
> <[email protected]> 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.
> >
> > Fixes: ef3c613ccd68a ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
i.e. will queue in renesas-clk-for-v5.17.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar
<[email protected]> 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 <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
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 -- [email protected]
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 Wed, Nov 17, 2021 at 12:51 PM Lad Prabhakar
<[email protected]> wrote:
> of_genpd_add_provider_simple() might fail, this patch makes sure we check
> the return value of of_genpd_add_provider_simple() by propagating the
> return value to the caller of cpg_mssr_add_clk_domain().
>
> Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
i.e. will queue in renesas-clk-for-v5.17.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 <[email protected]>
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> 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 <[email protected]> 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 <[email protected]>
> >
> > Reviewed-by: Geert Uytterhoeven <[email protected]>
> > 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 -- [email protected]
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