| Message ID | 1528211115-12334-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, Jun 05, 2018 at 05:05:15PM +0200, Geert Uytterhoeven wrote: > The quirk for R-Car E3 ES1.0 added in commit 086b399965a7ee7e ("soc: > renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") makes the 3DG-A > PM domain a subdomain of the 3DG-B PM domain. However, registering > 3DG-A with its parent fails silently, as the 3DG-B PM domain hasn't been > registered yet, and such failures are never reported. > > Fix this by: > 1. Splitting PM Domain initialization in two steps, so all PM domains > are registered before any child-parent links are established, > 2. Reporting any failures in establishing child-parent relations. > > Check for and report pm_genpd_init() failures, too, as that function > gained a return value in commit 7eb231c337e00735 ("PM / Domains: Convert > pm_genpd_init() to return an error code"). > > Fixes: 086b399965a7ee7e ("soc: renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") That makes this a fix for v4.18, right? > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> This looks fine to me but I will wait to see if there are other reviews before applying. Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Hi Simon, On Wed, Jun 6, 2018 at 10:52 AM, Simon Horman <horms@verge.net.au> wrote: > On Tue, Jun 05, 2018 at 05:05:15PM +0200, Geert Uytterhoeven wrote: >> The quirk for R-Car E3 ES1.0 added in commit 086b399965a7ee7e ("soc: >> renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") makes the 3DG-A >> PM domain a subdomain of the 3DG-B PM domain. However, registering >> 3DG-A with its parent fails silently, as the 3DG-B PM domain hasn't been >> registered yet, and such failures are never reported. >> >> Fix this by: >> 1. Splitting PM Domain initialization in two steps, so all PM domains >> are registered before any child-parent links are established, >> 2. Reporting any failures in establishing child-parent relations. >> >> Check for and report pm_genpd_init() failures, too, as that function >> gained a return value in commit 7eb231c337e00735 ("PM / Domains: Convert >> pm_genpd_init() to return an error code"). >> >> Fixes: 086b399965a7ee7e ("soc: renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") > > That makes this a fix for v4.18, right? Correct. >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > This looks fine to me but I will wait to see if there are other reviews > before applying. Let's wait for feedback from Shimoda-san. Perpaps the quirk did rely on the buggy behavior. > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> Thanks! Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Behalf Of Geert Uytterhoeven, Sent: Wednesday, June 6, 2018 5:58 PM > > Hi Simon, > > On Wed, Jun 6, 2018 at 10:52 AM, Simon Horman <horms@verge.net.au> wrote: > > On Tue, Jun 05, 2018 at 05:05:15PM +0200, Geert Uytterhoeven wrote: <snip> > > This looks fine to me but I will wait to see if there are other reviews > > before applying. > > Let's wait for feedback from Shimoda-san. Perpaps the quirk did rely on > the buggy behavior. I checked /sys/kernel/debug/pm_genpd/pm_genpd_summary. < Before applies the patch > - "3dg-b's" status is "off-0". - "3dg-b" doesn't have any slaves. < After applied the patch > - "3dg-b's" status is "on". - "3dg-b" has "3dg-a" slave. So, Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > > Thanks! > > 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 5 June 2018 at 17:05, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > The quirk for R-Car E3 ES1.0 added in commit 086b399965a7ee7e ("soc: > renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") makes the 3DG-A > PM domain a subdomain of the 3DG-B PM domain. However, registering > 3DG-A with its parent fails silently, as the 3DG-B PM domain hasn't been > registered yet, and such failures are never reported. > > Fix this by: > 1. Splitting PM Domain initialization in two steps, so all PM domains > are registered before any child-parent links are established, > 2. Reporting any failures in establishing child-parent relations. > > Check for and report pm_genpd_init() failures, too, as that function > gained a return value in commit 7eb231c337e00735 ("PM / Domains: Convert > pm_genpd_init() to return an error code"). > > Fixes: 086b399965a7ee7e ("soc: renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe > --- > drivers/soc/renesas/rcar-sysc.c | 35 +++++++++++++++++++++++++++++------ > 1 file changed, 29 insertions(+), 6 deletions(-) > > diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c > index 4db53fa38abc5cf3..99e926f68299878d 100644 > --- a/drivers/soc/renesas/rcar-sysc.c > +++ b/drivers/soc/renesas/rcar-sysc.c > @@ -379,11 +379,12 @@ static int rcar_sysc_pd_power_on(struct generic_pm_domain *genpd) > > static bool has_cpg_mstp; > > -static void __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd) > +static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd) > { > struct generic_pm_domain *genpd = &pd->genpd; > const char *name = pd->genpd.name; > struct dev_power_governor *gov = &simple_qos_governor; > + int error; > > if (pd->flags & PD_CPU) { > /* > @@ -436,7 +437,11 @@ static void __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd) > rcar_sysc_power_up(&pd->ch); > > finalize: > - pm_genpd_init(genpd, gov, false); > + error = pm_genpd_init(genpd, gov, false); > + if (error) > + pr_err("Failed to init PM domain %s: %d\n", name, error); > + > + return error; > } > > static const struct of_device_id rcar_sysc_matches[] __initconst = { > @@ -560,6 +565,9 @@ static int __init rcar_sysc_pd_init(void) > pr_debug("%pOF: syscier = 0x%08x\n", np, syscier); > iowrite32(syscier, base + SYSCIER); > > + /* > + * First, create all PM domains > + */ > for (i = 0; i < info->num_areas; i++) { > const struct rcar_sysc_area *area = &info->areas[i]; > struct rcar_sysc_pd *pd; > @@ -582,14 +590,29 @@ static int __init rcar_sysc_pd_init(void) > pd->ch.isr_bit = area->isr_bit; > pd->flags = area->flags; > > - rcar_sysc_pd_setup(pd); > - if (area->parent >= 0) > - pm_genpd_add_subdomain(domains->domains[area->parent], > - &pd->genpd); > + error = rcar_sysc_pd_setup(pd); > + if (error) > + goto out_put; > > domains->domains[area->isr_bit] = &pd->genpd; > } > > + /* > + * Second, link all PM domains to their parents > + */ > + for (i = 0; i < info->num_areas; i++) { > + const struct rcar_sysc_area *area = &info->areas[i]; > + > + if (!area->name || area->parent < 0) > + continue; > + > + error = pm_genpd_add_subdomain(domains->domains[area->parent], > + domains->domains[area->isr_bit]); > + if (error) > + pr_warn("Failed to add PM subdomain %s to parent %u\n", > + area->name, area->parent); > + } > + > error = of_genpd_add_provider_onecell(np, &domains->onecell_data); > > out_put: > -- > 2.7.4 >
diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c index 4db53fa38abc5cf3..99e926f68299878d 100644 --- a/drivers/soc/renesas/rcar-sysc.c +++ b/drivers/soc/renesas/rcar-sysc.c @@ -379,11 +379,12 @@ static int rcar_sysc_pd_power_on(struct generic_pm_domain *genpd) static bool has_cpg_mstp; -static void __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd) +static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd) { struct generic_pm_domain *genpd = &pd->genpd; const char *name = pd->genpd.name; struct dev_power_governor *gov = &simple_qos_governor; + int error; if (pd->flags & PD_CPU) { /* @@ -436,7 +437,11 @@ static void __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd) rcar_sysc_power_up(&pd->ch); finalize: - pm_genpd_init(genpd, gov, false); + error = pm_genpd_init(genpd, gov, false); + if (error) + pr_err("Failed to init PM domain %s: %d\n", name, error); + + return error; } static const struct of_device_id rcar_sysc_matches[] __initconst = { @@ -560,6 +565,9 @@ static int __init rcar_sysc_pd_init(void) pr_debug("%pOF: syscier = 0x%08x\n", np, syscier); iowrite32(syscier, base + SYSCIER); + /* + * First, create all PM domains + */ for (i = 0; i < info->num_areas; i++) { const struct rcar_sysc_area *area = &info->areas[i]; struct rcar_sysc_pd *pd; @@ -582,14 +590,29 @@ static int __init rcar_sysc_pd_init(void) pd->ch.isr_bit = area->isr_bit; pd->flags = area->flags; - rcar_sysc_pd_setup(pd); - if (area->parent >= 0) - pm_genpd_add_subdomain(domains->domains[area->parent], - &pd->genpd); + error = rcar_sysc_pd_setup(pd); + if (error) + goto out_put; domains->domains[area->isr_bit] = &pd->genpd; } + /* + * Second, link all PM domains to their parents + */ + for (i = 0; i < info->num_areas; i++) { + const struct rcar_sysc_area *area = &info->areas[i]; + + if (!area->name || area->parent < 0) + continue; + + error = pm_genpd_add_subdomain(domains->domains[area->parent], + domains->domains[area->isr_bit]); + if (error) + pr_warn("Failed to add PM subdomain %s to parent %u\n", + area->name, area->parent); + } + error = of_genpd_add_provider_onecell(np, &domains->onecell_data); out_put:
The quirk for R-Car E3 ES1.0 added in commit 086b399965a7ee7e ("soc: renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") makes the 3DG-A PM domain a subdomain of the 3DG-B PM domain. However, registering 3DG-A with its parent fails silently, as the 3DG-B PM domain hasn't been registered yet, and such failures are never reported. Fix this by: 1. Splitting PM Domain initialization in two steps, so all PM domains are registered before any child-parent links are established, 2. Reporting any failures in establishing child-parent relations. Check for and report pm_genpd_init() failures, too, as that function gained a return value in commit 7eb231c337e00735 ("PM / Domains: Convert pm_genpd_init() to return an error code"). Fixes: 086b399965a7ee7e ("soc: renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/soc/renesas/rcar-sysc.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)