[PATCH 0/4] soc: renesas: rcar-sysc: Miscellaneous fixes and cleanups

[PATCH 0/4] soc: renesas: rcar-sysc: Miscellaneous fixes and cleanups

[Geert Uytterhoeven]

	Hi Simon, Magnus,

This series (against renesas-devel-20181204-v4.20-rc5) contains
miscellaneous fixes and cleanups for the R-Car SYSC driver.

This has been tested on R-Car Gen2 (H2 and M2-W) and R-Car Gen3 (H3
ES1.0, H3 ES2.0, M3-W, M3-N, D3, E3, and V3M) (without 3DG).

This not been tested on R-Car H1 and R-Car V3H.

Thanks!

Geert Uytterhoeven (4):
  soc: renesas: r8a77990-sysc: Fix initialization order of 3DG-{A,B}
  soc: renesas: rcar-sysc: Remove rcar_sysc_power_{down,up}() helpers
  soc: renesas: rcar-sysc: Merge PM Domain registration and linking
  soc: renesas: rcar-sysc: Fix power domain control after system resume

 drivers/soc/renesas/r8a77990-sysc.c | 23 ++--------
 drivers/soc/renesas/rcar-sysc.c     | 65 ++++++++---------------------
 2 files changed, 22 insertions(+), 66 deletions(-)

-- 
2.17.1

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

[PATCH 1/4] soc: renesas: r8a77990-sysc: Fix initialization order of 3DG-{A,B}

[Geert Uytterhoeven]

The workaround for the wrong hierarchy of the 3DG-{A,B} power
domains on R-Car E3 ES1.0 corrected the parent domains.
However, the 3DG-{A,B} power domains were still initialized and powered
in the wrong order, causing 3DG operation to fail.

Fix this by changing the order in the table at runtime, when running on
an affected SoC.

Fixes: 086b399965a7ee7e ("soc: renesas: r8a77990-sysc: Add workaround for 3DG-{A,B}")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/soc/renesas/r8a77990-sysc.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/renesas/r8a77990-sysc.c b/drivers/soc/renesas/r8a77990-sysc.c
index 15579ebc5ed2059d..664b244eb1dd9d95 100644
--- a/drivers/soc/renesas/r8a77990-sysc.c
+++ b/drivers/soc/renesas/r8a77990-sysc.c
@@ -28,19 +28,6 @@ static struct rcar_sysc_area r8a77990_areas[] __initdata = {
 	{ "3dg-b",	0x100, 1, R8A77990_PD_3DG_B,	R8A77990_PD_3DG_A },
 };
 
-static void __init rcar_sysc_fix_parent(struct rcar_sysc_area *areas,
-					unsigned int num_areas, u8 id,
-					int new_parent)
-{
-	unsigned int i;
-
-	for (i = 0; i < num_areas; i++)
-		if (areas[i].isr_bit == id) {
-			areas[i].parent = new_parent;
-			return;
-		}
-}
-
 /* Fixups for R-Car E3 ES1.0 revision */
 static const struct soc_device_attribute r8a77990[] __initconst = {
 	{ .soc_id = "r8a77990", .revision = "ES1.0" },
@@ -50,12 +37,10 @@ static const struct soc_device_attribute r8a77990[] __initconst = {
 static int __init r8a77990_sysc_init(void)
 {
 	if (soc_device_match(r8a77990)) {
-		rcar_sysc_fix_parent(r8a77990_areas,
-				     ARRAY_SIZE(r8a77990_areas),
-				     R8A77990_PD_3DG_A, R8A77990_PD_3DG_B);
-		rcar_sysc_fix_parent(r8a77990_areas,
-				     ARRAY_SIZE(r8a77990_areas),
-				     R8A77990_PD_3DG_B, R8A77990_PD_ALWAYS_ON);
+		/* Fix incorrect 3DG hierarchy */
+		swap(r8a77990_areas[7], r8a77990_areas[8]);
+		r8a77990_areas[7].parent = R8A77990_PD_ALWAYS_ON;
+		r8a77990_areas[8].parent = R8A77990_PD_3DG_B;
 	}
 
 	return 0;
-- 
2.17.1

[PATCH 2/4] soc: renesas: rcar-sysc: Remove rcar_sysc_power_{down,up}() helpers

[Geert Uytterhoeven]

Until commit 7e8a50df26f4e700 ("soc: renesas: rcar-sysc: Drop legacy
handling"), the rcar_sysc_power_{down,up}() helpers were public, as they
were called by the legacy (pre-DT) CPU power management code on R-Car H1
and R-Car Gen2 before.

As they are just one-line wrappers around rcar_sysc_power(), it makes
sense to just remove them.

This also avoids a bool/helper/bool conversion in rcar_sysc_power_cpu(),
where a bool is checked to call one of two helper functions, which
just call rcar_sysc_power() with hardcoded boolean values again.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/soc/renesas/rcar-sysc.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
index af53363eda0374ce..73fae6a9728df9ae 100644
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@ -146,16 +146,6 @@ static int rcar_sysc_power(const struct rcar_sysc_ch *sysc_ch, bool on)
 	return ret;
 }
 
-static int rcar_sysc_power_down(const struct rcar_sysc_ch *sysc_ch)
-{
-	return rcar_sysc_power(sysc_ch, false);
-}
-
-static int rcar_sysc_power_up(const struct rcar_sysc_ch *sysc_ch)
-{
-	return rcar_sysc_power(sysc_ch, true);
-}
-
 static bool rcar_sysc_power_is_off(const struct rcar_sysc_ch *sysc_ch)
 {
 	unsigned int st;
@@ -184,7 +174,7 @@ static int rcar_sysc_pd_power_off(struct generic_pm_domain *genpd)
 	struct rcar_sysc_pd *pd = to_rcar_pd(genpd);
 
 	pr_debug("%s: %s\n", __func__, genpd->name);
-	return rcar_sysc_power_down(&pd->ch);
+	return rcar_sysc_power(&pd->ch, false);
 }
 
 static int rcar_sysc_pd_power_on(struct generic_pm_domain *genpd)
@@ -192,7 +182,7 @@ static int rcar_sysc_pd_power_on(struct generic_pm_domain *genpd)
 	struct rcar_sysc_pd *pd = to_rcar_pd(genpd);
 
 	pr_debug("%s: %s\n", __func__, genpd->name);
-	return rcar_sysc_power_up(&pd->ch);
+	return rcar_sysc_power(&pd->ch, true);
 }
 
 static bool has_cpg_mstp;
@@ -252,7 +242,7 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
 		goto finalize;
 	}
 
-	rcar_sysc_power_up(&pd->ch);
+	rcar_sysc_power(&pd->ch, true);
 
 finalize:
 	error = pm_genpd_init(genpd, gov, false);
@@ -478,8 +468,7 @@ static int rcar_sysc_power_cpu(unsigned int idx, bool on)
 		if (!(pd->flags & PD_CPU) || pd->ch.chan_bit != idx)
 			continue;
 
-		return on ? rcar_sysc_power_up(&pd->ch)
-			  : rcar_sysc_power_down(&pd->ch);
+		return rcar_sysc_power(&pd->ch, on);
 	}
 
 	return -ENOENT;
-- 
2.17.1

[PATCH 3/4] soc: renesas: rcar-sysc: Merge PM Domain registration and linking

[Geert Uytterhoeven]

Commit 977d5ba4507dfe5b ("soc: renesas: rcar-sysc: Make PM domain
initialization more robust") split PM Domain registration and the
linking of children to their parents, to accommodate PM Domain tables
that list child domains before their parents.

However, this failed to realize that parent power domains must be
powered up before their children anyway, and that this thus must be
reflected by the order in the PM Domain tables.

Revert the split, as it did not help anyway.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/soc/renesas/rcar-sysc.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
index 73fae6a9728df9ae..123e553510e826f5 100644
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@ -381,9 +381,6 @@ 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;
@@ -411,22 +408,17 @@ static int __init rcar_sysc_pd_init(void)
 			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)
+		if (area->parent < 0)
 			continue;
 
 		error = pm_genpd_add_subdomain(domains->domains[area->parent],
-					       domains->domains[area->isr_bit]);
-		if (error)
+					       &pd->genpd);
+		if (error) {
 			pr_warn("Failed to add PM subdomain %s to parent %u\n",
 				area->name, area->parent);
+			goto out_put;
+		}
 	}
 
 	error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
-- 
2.17.1

[PATCH 4/4] soc: renesas: rcar-sysc: Fix power domain control after system resume

[Geert Uytterhoeven]

To control power to a power domain, the System Controller (SYSC) needs
the corresponding interrupt source to be enabled, but masked, to prevent
the CPU from receiving it.

Currently this is handled in the driver's probe() routine, and set up
for every domain present, even if it will not be controlled directly by
SYSC (CPU domains are powered through the APMU on R-Car Gen2 and later).

On R-Car Gen3, PSCI powers down the SoC during system suspend, thus
loosing any configured interrupt state.  Hence after system resume, power
domains not controlled through the APMU (e.g. A3IR, A3VC, A3VP) fail to
power up.

Fix this by replacing the global interrupt setup in the probe() routine
by a domain-specific interrupt setup in rcar_sysc_power(), where the
domain's power is actually controlled.  This brings the code more in
line with the flowchart in the Hardware User's Manual.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/soc/renesas/rcar-sysc.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
index 123e553510e826f5..0c80fab4f8de6bc8 100644
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@ -105,6 +105,15 @@ static int rcar_sysc_power(const struct rcar_sysc_ch *sysc_ch, bool on)
 
 	spin_lock_irqsave(&rcar_sysc_lock, flags);
 
+	/*
+	 * The interrupt source needs to be enabled, but masked, to prevent the
+	 * CPU from receiving it.
+	 */
+	iowrite32(ioread32(rcar_sysc_base + SYSCIMR) | isr_mask,
+		  rcar_sysc_base + SYSCIMR);
+	iowrite32(ioread32(rcar_sysc_base + SYSCIER) | isr_mask,
+		  rcar_sysc_base + SYSCIER);
+
 	iowrite32(isr_mask, rcar_sysc_base + SYSCISCR);
 
 	/* Submit power shutoff or resume request until it was accepted */
@@ -324,7 +333,6 @@ static int __init rcar_sysc_pd_init(void)
 	const struct of_device_id *match;
 	struct rcar_pm_domains *domains;
 	struct device_node *np;
-	u32 syscier, syscimr;
 	void __iomem *base;
 	unsigned int i;
 	int error;
@@ -363,24 +371,6 @@ static int __init rcar_sysc_pd_init(void)
 	domains->onecell_data.num_domains = ARRAY_SIZE(domains->domains);
 	rcar_sysc_onecell_data = &domains->onecell_data;
 
-	for (i = 0, syscier = 0; i < info->num_areas; i++)
-		syscier |= BIT(info->areas[i].isr_bit);
-
-	/*
-	 * Mask all interrupt sources to prevent the CPU from receiving them.
-	 * Make sure not to clear reserved bits that were set before.
-	 */
-	syscimr = ioread32(base + SYSCIMR);
-	syscimr |= syscier;
-	pr_debug("%pOF: syscimr = 0x%08x\n", np, syscimr);
-	iowrite32(syscimr, base + SYSCIMR);
-
-	/*
-	 * SYSC needs all interrupt sources enabled to control power.
-	 */
-	pr_debug("%pOF: syscier = 0x%08x\n", np, syscier);
-	iowrite32(syscier, base + SYSCIER);
-
 	for (i = 0; i < info->num_areas; i++) {
 		const struct rcar_sysc_area *area = &info->areas[i];
 		struct rcar_sysc_pd *pd;
-- 
2.17.1

Re: [PATCH 4/4] soc: renesas: rcar-sysc: Fix power domain control after system resume

[Simon Horman]

On Wed, Dec 05, 2018 at 04:39:45PM +0100, Geert Uytterhoeven wrote:
> To control power to a power domain, the System Controller (SYSC) needs
> the corresponding interrupt source to be enabled, but masked, to prevent
> the CPU from receiving it.
> 
> Currently this is handled in the driver's probe() routine, and set up
> for every domain present, even if it will not be controlled directly by
> SYSC (CPU domains are powered through the APMU on R-Car Gen2 and later).
> 
> On R-Car Gen3, PSCI powers down the SoC during system suspend, thus
> loosing any configured interrupt state.  Hence after system resume, power
> domains not controlled through the APMU (e.g. A3IR, A3VC, A3VP) fail to
> power up.

I corrected the spelling of losing when applying this patch.

> 
> Fix this by replacing the global interrupt setup in the probe() routine
> by a domain-specific interrupt setup in rcar_sysc_power(), where the
> domain's power is actually controlled.  This brings the code more in
> line with the flowchart in the Hardware User's Manual.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/soc/renesas/rcar-sysc.c | 28 +++++++++-------------------
>  1 file changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
> index 123e553510e826f5..0c80fab4f8de6bc8 100644
> --- a/drivers/soc/renesas/rcar-sysc.c
> +++ b/drivers/soc/renesas/rcar-sysc.c
> @@ -105,6 +105,15 @@ static int rcar_sysc_power(const struct rcar_sysc_ch *sysc_ch, bool on)
>  
>  	spin_lock_irqsave(&rcar_sysc_lock, flags);
>  
> +	/*
> +	 * The interrupt source needs to be enabled, but masked, to prevent the
> +	 * CPU from receiving it.
> +	 */
> +	iowrite32(ioread32(rcar_sysc_base + SYSCIMR) | isr_mask,
> +		  rcar_sysc_base + SYSCIMR);
> +	iowrite32(ioread32(rcar_sysc_base + SYSCIER) | isr_mask,
> +		  rcar_sysc_base + SYSCIER);
> +
>  	iowrite32(isr_mask, rcar_sysc_base + SYSCISCR);
>  
>  	/* Submit power shutoff or resume request until it was accepted */
> @@ -324,7 +333,6 @@ static int __init rcar_sysc_pd_init(void)
>  	const struct of_device_id *match;
>  	struct rcar_pm_domains *domains;
>  	struct device_node *np;
> -	u32 syscier, syscimr;
>  	void __iomem *base;
>  	unsigned int i;
>  	int error;
> @@ -363,24 +371,6 @@ static int __init rcar_sysc_pd_init(void)
>  	domains->onecell_data.num_domains = ARRAY_SIZE(domains->domains);
>  	rcar_sysc_onecell_data = &domains->onecell_data;
>  
> -	for (i = 0, syscier = 0; i < info->num_areas; i++)
> -		syscier |= BIT(info->areas[i].isr_bit);
> -
> -	/*
> -	 * Mask all interrupt sources to prevent the CPU from receiving them.
> -	 * Make sure not to clear reserved bits that were set before.
> -	 */
> -	syscimr = ioread32(base + SYSCIMR);
> -	syscimr |= syscier;
> -	pr_debug("%pOF: syscimr = 0x%08x\n", np, syscimr);
> -	iowrite32(syscimr, base + SYSCIMR);
> -
> -	/*
> -	 * SYSC needs all interrupt sources enabled to control power.
> -	 */
> -	pr_debug("%pOF: syscier = 0x%08x\n", np, syscier);
> -	iowrite32(syscier, base + SYSCIER);
> -
>  	for (i = 0; i < info->num_areas; i++) {
>  		const struct rcar_sysc_area *area = &info->areas[i];
>  		struct rcar_sysc_pd *pd;
> -- 
> 2.17.1
> 

Re: [PATCH 0/4] soc: renesas: rcar-sysc: Miscellaneous fixes and cleanups

[Simon Horman]

On Wed, Dec 05, 2018 at 04:39:41PM +0100, Geert Uytterhoeven wrote:
> 	Hi Simon, Magnus,
> 
> This series (against renesas-devel-20181204-v4.20-rc5) contains
> miscellaneous fixes and cleanups for the R-Car SYSC driver.
> 
> This has been tested on R-Car Gen2 (H2 and M2-W) and R-Car Gen3 (H3
> ES1.0, H3 ES2.0, M3-W, M3-N, D3, E3, and V3M) (without 3DG).
> 
> This not been tested on R-Car H1 and R-Car V3H.

Thanks, applied for v4.21.

Re: [PATCH 4/4] soc: renesas: rcar-sysc: Fix power domain control after system resume

[Geert Uytterhoeven]

Hi Simon,

On Wed, Dec 5, 2018 at 10:21 PM Simon Horman <horms@verge.net.au> wrote:
> On Wed, Dec 05, 2018 at 04:39:45PM +0100, Geert Uytterhoeven wrote:
> > To control power to a power domain, the System Controller (SYSC) needs
> > the corresponding interrupt source to be enabled, but masked, to prevent
> > the CPU from receiving it.
> >
> > Currently this is handled in the driver's probe() routine, and set up
> > for every domain present, even if it will not be controlled directly by
> > SYSC (CPU domains are powered through the APMU on R-Car Gen2 and later).
> >
> > On R-Car Gen3, PSCI powers down the SoC during system suspend, thus
> > loosing any configured interrupt state.  Hence after system resume, power
> > domains not controlled through the APMU (e.g. A3IR, A3VC, A3VP) fail to
> > power up.
>
> I corrected the spelling of losing when applying this patch.

Checkpatch complained about that as well, so I did some investigation, and
decided to keep it... Was I wrong?

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

Re: [PATCH 0/4] soc: renesas: rcar-sysc: Miscellaneous fixes and cleanups

[Geert Uytterhoeven]

On Wed, Dec 5, 2018 at 4:39 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> This series (against renesas-devel-20181204-v4.20-rc5) contains
> miscellaneous fixes and cleanups for the R-Car SYSC driver.
>
> This has been tested on R-Car Gen2 (H2 and M2-W) and R-Car Gen3 (H3
> ES1.0, H3 ES2.0, M3-W, M3-N, D3, E3, and V3M) (without 3DG).
>
> This not been tested on R-Car H1 and R-Car V3H.

Now tested on R-Car H1 (Marzen), with no ill effects on secondary CPU
boot and CPU hotplug.

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

Re: [PATCH 4/4] soc: renesas: rcar-sysc: Fix power domain control after system resume

[Simon Horman]

On Thu, Dec 06, 2018 at 09:22:24AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Wed, Dec 5, 2018 at 10:21 PM Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Dec 05, 2018 at 04:39:45PM +0100, Geert Uytterhoeven wrote:
> > > To control power to a power domain, the System Controller (SYSC) needs
> > > the corresponding interrupt source to be enabled, but masked, to prevent
> > > the CPU from receiving it.
> > >
> > > Currently this is handled in the driver's probe() routine, and set up
> > > for every domain present, even if it will not be controlled directly by
> > > SYSC (CPU domains are powered through the APMU on R-Car Gen2 and later).
> > >
> > > On R-Car Gen3, PSCI powers down the SoC during system suspend, thus
> > > loosing any configured interrupt state.  Hence after system resume, power
> > > domains not controlled through the APMU (e.g. A3IR, A3VC, A3VP) fail to
> > > power up.
> >
> > I corrected the spelling of losing when applying this patch.
> 
> Checkpatch complained about that as well, so I did some investigation, and
> decided to keep it... Was I wrong?

I will not claim to be an expert but  at the time I thought
checkpatch was right.

Re: [PATCH 0/4] soc: renesas: rcar-sysc: Miscellaneous fixes and cleanups

[Simon Horman]

On Thu, Dec 06, 2018 at 10:02:07AM +0100, Geert Uytterhoeven wrote:
> On Wed, Dec 5, 2018 at 4:39 PM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > This series (against renesas-devel-20181204-v4.20-rc5) contains
> > miscellaneous fixes and cleanups for the R-Car SYSC driver.
> >
> > This has been tested on R-Car Gen2 (H2 and M2-W) and R-Car Gen3 (H3
> > ES1.0, H3 ES2.0, M3-W, M3-N, D3, E3, and V3M) (without 3DG).
> >
> > This not been tested on R-Car H1 and R-Car V3H.
> 
> Now tested on R-Car H1 (Marzen), with no ill effects on secondary CPU
> boot and CPU hotplug.

Thanks for following-up, good to know.