[PATCH 0/3] make reinitialization of ARM core components possible

[PATCH 0/3] make reinitialization of ARM core components possible

[Shawn Guo]

When ARM core resumes from low-power mode where losing power, for my
example: CA-9 MP resumes from Dormant/Shutdown, we have to reinitialize
components like L2 Cache, GIC and SCU to bring system back to work.

The patch set basically removes __init annotation from a butch of
initialization functions, so that platform resume procedure can call
into them again to set those components up.

It's based on rmk's for-next branch, and tested on i.MX6Q.

Shawn Guo (3):
      ARM: cache-l2x0: make the reinitialization possible
      ARM: GIC: add gic_reinit() function to help ARM resume
      ARM: smp_scu: remove __init annotation from scu_enable()

 arch/arm/common/gic.c                      |   15 +++++++++++++--
 arch/arm/include/asm/hardware/cache-l2x0.h |   10 +++++++++-
 arch/arm/include/asm/hardware/gic.h        |    1 +
 arch/arm/kernel/smp_scu.c                  |    2 +-
 arch/arm/mm/cache-l2x0.c                   |   23 +++++++++++++----------
 5 files changed, 37 insertions(+), 14 deletions(-)

[PATCH 1/3] ARM: cache-l2x0: make the reinitialization possible

[Shawn Guo]

If ARM core gets powered off during suspend, L2 cache controller
has to be reinitialized by resume procedure.

The patch removes __init annotation from a few initialization
functions to make the reinitialization possible.  For example,
platform resume function can call l2x0_of_init() to get L2 cache
back to work.

It also adds the empty function for l2x0_init() and l2x0_of_init(),
so that we can keep '#ifdef CONFIG_CACHE_L2X0' check in header.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/include/asm/hardware/cache-l2x0.h |   10 +++++++++-
 arch/arm/mm/cache-l2x0.c                   |   23 +++++++++++++----------
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 4a6004a..ed946c5 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -89,8 +89,16 @@
 #define L2X0_ADDR_FILTER_EN		1
 
 #ifndef __ASSEMBLY__
-extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
+#ifdef CONFIG_CACHE_L2X0
+extern void l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
 extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
+#else
+static void l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask) {}
+static inline int l2x0_of_init(__u32 aux_val, __u32 aux_mask)
+{
+	return -ENOSYS;
+}
+#endif
 #endif
 
 #endif
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index c035b9a..3eeb025 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -280,7 +280,7 @@ static void l2x0_disable(void)
 	spin_unlock_irqrestore(&l2x0_lock, flags);
 }
 
-void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
+void l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 {
 	__u32 aux;
 	__u32 cache_id;
@@ -350,13 +350,13 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 	outer_cache.disable = l2x0_disable;
 	outer_cache.set_debug = l2x0_set_debug;
 
-	printk(KERN_INFO "%s cache controller enabled\n", type);
-	printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
+	pr_info_once("%s cache controller enabled\n", type);
+	pr_info_once("l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
 			ways, cache_id, aux, l2x0_size);
 }
 
 #ifdef CONFIG_OF
-static void __init l2x0_of_setup(const struct device_node *np,
+static void l2x0_of_setup(const struct device_node *np,
 				 __u32 *aux_val, __u32 *aux_mask)
 {
 	u32 data[2] = { 0, 0 };
@@ -390,7 +390,7 @@ static void __init l2x0_of_setup(const struct device_node *np,
 	*aux_mask &= ~mask;
 }
 
-static void __init pl310_of_setup(const struct device_node *np,
+static void pl310_of_setup(const struct device_node *np,
 				  __u32 *aux_val, __u32 *aux_mask)
 {
 	u32 data[3] = { 0, 0, 0 };
@@ -424,14 +424,14 @@ static void __init pl310_of_setup(const struct device_node *np,
 	}
 }
 
-static const struct of_device_id l2x0_ids[] __initconst = {
+static const struct of_device_id l2x0_ids[] = {
 	{ .compatible = "arm,pl310-cache", .data = pl310_of_setup },
 	{ .compatible = "arm,l220-cache", .data = l2x0_of_setup },
 	{ .compatible = "arm,l210-cache", .data = l2x0_of_setup },
 	{}
 };
 
-int __init l2x0_of_init(__u32 aux_val, __u32 aux_mask)
+int l2x0_of_init(__u32 aux_val, __u32 aux_mask)
 {
 	struct device_node *np;
 	void (*l2_setup)(const struct device_node *np,
@@ -440,9 +440,12 @@ int __init l2x0_of_init(__u32 aux_val, __u32 aux_mask)
 	np = of_find_matching_node(NULL, l2x0_ids);
 	if (!np)
 		return -ENODEV;
-	l2x0_base = of_iomap(np, 0);
-	if (!l2x0_base)
-		return -ENOMEM;
+
+	if (!l2x0_base) {
+		l2x0_base = of_iomap(np, 0);
+		if (!l2x0_base)
+			return -ENOMEM;
+	}
 
 	/* L2 configuration can only be changed if the cache is disabled */
 	if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
-- 
1.7.4.1

[PATCH 1/3] ARM: cache-l2x0: make the reinitialization possible

[Barry Song]

2011/8/30 Shawn Guo <shawn.guo@linaro.org>:
> If ARM core gets powered off during suspend, L2 cache controller
> has to be reinitialized by resume procedure.
>
> The patch removes __init annotation from a few initialization
> functions to make the reinitialization possible. ?For example,
> platform resume function can call l2x0_of_init() to get L2 cache
> back to work.

i think it is good. and i have sent a similar patch before:

http://www.spinics.net/lists/arm-kernel/msg137372.html
[PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section

people haven't give any feedback yet.

>
> It also adds the empty function for l2x0_init() and l2x0_of_init(),
> so that we can keep '#ifdef CONFIG_CACHE_L2X0' check in header.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
-barry

[PATCH 1/3] ARM: cache-l2x0: make the reinitialization possible

[Shawn Guo]

On Tue, Aug 30, 2011 at 10:08:59PM +0800, Barry Song wrote:
> 2011/8/30 Shawn Guo <shawn.guo@linaro.org>:
> > If ARM core gets powered off during suspend, L2 cache controller
> > has to be reinitialized by resume procedure.
> >
> > The patch removes __init annotation from a few initialization
> > functions to make the reinitialization possible. ?For example,
> > platform resume function can call l2x0_of_init() to get L2 cache
> > back to work.
> 
> i think it is good. and i have sent a similar patch before:
> 
> http://www.spinics.net/lists/arm-kernel/msg137372.html
> [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section
> 
Since prima2 supports DT, you may also want to call l2x0_of_init()
instead of l2x0_init().  l2x0_of_init() will parse configuration from
DT and sets up tag RAM control for you.

-- 
Regards,
Shawn

[PATCH 1/3] ARM: cache-l2x0: make the reinitialization possible

[Barry Song]

2011/8/30 Shawn Guo <shawn.guo@freescale.com>:
> On Tue, Aug 30, 2011 at 10:08:59PM +0800, Barry Song wrote:
>> 2011/8/30 Shawn Guo <shawn.guo@linaro.org>:
>> > If ARM core gets powered off during suspend, L2 cache controller
>> > has to be reinitialized by resume procedure.
>> >
>> > The patch removes __init annotation from a few initialization
>> > functions to make the reinitialization possible. ?For example,
>> > platform resume function can call l2x0_of_init() to get L2 cache
>> > back to work.
>>
>> i think it is good. and i have sent a similar patch before:
>>
>> http://www.spinics.net/lists/arm-kernel/msg137372.html
>> [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section
>>
> Since prima2 supports DT, you may also want to call l2x0_of_init()
> instead of l2x0_init(). ?l2x0_of_init() will parse configuration from
> DT and sets up tag RAM control for you.

i know that. and rob's patch did some suppport to prima2 and we were
in the loop of that patch and i have acked it if you read the mail.
i sent the patch againest the Linus's tree.

>
> --
> Regards,
> Shawn
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH 1/3] ARM: cache-l2x0: make the reinitialization possible

[Barry Song]

2011/8/31 Barry Song <21cnbao@gmail.com>:
> 2011/8/30 Shawn Guo <shawn.guo@freescale.com>:
>> On Tue, Aug 30, 2011 at 10:08:59PM +0800, Barry Song wrote:
>>> 2011/8/30 Shawn Guo <shawn.guo@linaro.org>:
>>> > If ARM core gets powered off during suspend, L2 cache controller
>>> > has to be reinitialized by resume procedure.
>>> >
>>> > The patch removes __init annotation from a few initialization
>>> > functions to make the reinitialization possible. ?For example,
>>> > platform resume function can call l2x0_of_init() to get L2 cache
>>> > back to work.
>>>
>>> i think it is good. and i have sent a similar patch before:
>>>
>>> http://www.spinics.net/lists/arm-kernel/msg137372.html
>>> [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section
>>>
>> Since prima2 supports DT, you may also want to call l2x0_of_init()
>> instead of l2x0_init(). ?l2x0_of_init() will parse configuration from
>> DT and sets up tag RAM control for you.
>
> i know that. and rob's patch did some suppport to prima2 and we were
> in the loop of that patch and i have acked it if you read the mail.
> i sent the patch againest the Linus's tree.

you might find two earlier threads about l2x0 re-init as well:

Colin Cross:
[PATCH] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

Barry Song:
[RFC] ARM: l2x0: suspend/resume entries

let's wait for some feedback from Russell.

>
>>
>> --
>> Regards,
>> Shawn
-barry

[PATCH 2/3] ARM: GIC: add gic_reinit() function to help ARM resume

[Shawn Guo]

If ARM core gets powered off during suspend, GIC controller has to be
reinitialized by resume procedure.  This patch adds a helper function
for resume procedure to reinitialize GIC.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/common/gic.c               |   15 +++++++++++++--
 arch/arm/include/asm/hardware/gic.h |    1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 666b278..bf0f6d8 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -255,7 +255,7 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
-static void __init gic_dist_init(struct gic_chip_data *gic,
+static void gic_dist_init(struct gic_chip_data *gic,
 	unsigned int irq_start)
 {
 	unsigned int gic_irqs, irq_limit, i;
@@ -326,7 +326,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
 
-static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
+static void gic_cpu_init(struct gic_chip_data *gic)
 {
 	void __iomem *dist_base = gic->dist_base;
 	void __iomem *base = gic->cpu_base;
@@ -349,6 +349,17 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
 	writel_relaxed(1, base + GIC_CPU_CTRL);
 }
 
+void gic_reinit(unsigned int gic_nr, unsigned int irq_start)
+{
+	struct gic_chip_data *gic;
+
+	BUG_ON(gic_nr >= MAX_GIC_NR);
+
+	gic = &gic_data[gic_nr];
+	gic_dist_init(gic, irq_start);
+	gic_cpu_init(gic);
+}
+
 void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
 	void __iomem *dist_base, void __iomem *cpu_base)
 {
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..9338326 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -37,6 +37,7 @@ extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
 void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
+void gic_reinit(unsigned int gic_nr, unsigned int irq_start);
 void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-- 
1.7.4.1

[PATCH 2/3] ARM: GIC: add gic_reinit() function to help ARM resume

[Rob Herring]

Shawn,

On 08/30/2011 02:40 AM, Shawn Guo wrote:
> If ARM core gets powered off during suspend, GIC controller has to be
> reinitialized by resume procedure.  This patch adds a helper function
> for resume procedure to reinitialize GIC.

Is re-initializing rather than save/restore registers the right thing to
do here? Won't you lose things like edge or level triggered settings?
It's always been fuzzy to what should be done with interrupt masks in
suspend. Is every driver expected to disable and re-enable their
interrupts or this should be maintained thru a suspend cycle?

> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm/common/gic.c               |   15 +++++++++++++--
>  arch/arm/include/asm/hardware/gic.h |    1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index 666b278..bf0f6d8 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -255,7 +255,7 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
>  	irq_set_chained_handler(irq, gic_handle_cascade_irq);
>  }
>  
> -static void __init gic_dist_init(struct gic_chip_data *gic,
> +static void gic_dist_init(struct gic_chip_data *gic,
>  	unsigned int irq_start)
>  {
>  	unsigned int gic_irqs, irq_limit, i;
> @@ -326,7 +326,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
>  	writel_relaxed(1, base + GIC_DIST_CTRL);
>  }
>  
> -static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
> +static void gic_cpu_init(struct gic_chip_data *gic)

I don't think you need to change this. With hotplug, this section will
be kept.

Rob

>  {
>  	void __iomem *dist_base = gic->dist_base;
>  	void __iomem *base = gic->cpu_base;
> @@ -349,6 +349,17 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
>  	writel_relaxed(1, base + GIC_CPU_CTRL);
>  }
>  
> +void gic_reinit(unsigned int gic_nr, unsigned int irq_start)
> +{
> +	struct gic_chip_data *gic;
> +
> +	BUG_ON(gic_nr >= MAX_GIC_NR);
> +
> +	gic = &gic_data[gic_nr];
> +	gic_dist_init(gic, irq_start);
> +	gic_cpu_init(gic);
> +}
> +
>  void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>  	void __iomem *dist_base, void __iomem *cpu_base)
>  {
> diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
> index 435d3f8..9338326 100644
> --- a/arch/arm/include/asm/hardware/gic.h
> +++ b/arch/arm/include/asm/hardware/gic.h
> @@ -37,6 +37,7 @@ extern void __iomem *gic_cpu_base_addr;
>  extern struct irq_chip gic_arch_extn;
>  
>  void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
> +void gic_reinit(unsigned int gic_nr, unsigned int irq_start);
>  void gic_secondary_init(unsigned int);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);

[PATCH 2/3] ARM: GIC: add gic_reinit() function to help ARM resume

[Shawn Guo]

On Tue, Aug 30, 2011 at 10:00:35AM -0500, Rob Herring wrote:
> Shawn,
> 
> On 08/30/2011 02:40 AM, Shawn Guo wrote:
> > If ARM core gets powered off during suspend, GIC controller has to be
> > reinitialized by resume procedure.  This patch adds a helper function
> > for resume procedure to reinitialize GIC.
> 
> Is re-initializing rather than save/restore registers the right thing to
> do here? Won't you lose things like edge or level triggered settings?
> It's always been fuzzy to what should be done with interrupt masks in
> suspend. Is every driver expected to disable and re-enable their
> interrupts or this should be maintained thru a suspend cycle?
> 
Good point.  As one of the series, I easily went for the re-initializing
approach here, plus it is pretty easy to go.  With your reminding, I
agree that for interrupt controller save/restore registers might be the
right thing to do.

> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  arch/arm/common/gic.c               |   15 +++++++++++++--
> >  arch/arm/include/asm/hardware/gic.h |    1 +
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> > index 666b278..bf0f6d8 100644
> > --- a/arch/arm/common/gic.c
> > +++ b/arch/arm/common/gic.c
> > @@ -255,7 +255,7 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
> >  	irq_set_chained_handler(irq, gic_handle_cascade_irq);
> >  }
> >  
> > -static void __init gic_dist_init(struct gic_chip_data *gic,
> > +static void gic_dist_init(struct gic_chip_data *gic,
> >  	unsigned int irq_start)
> >  {
> >  	unsigned int gic_irqs, irq_limit, i;
> > @@ -326,7 +326,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
> >  	writel_relaxed(1, base + GIC_DIST_CTRL);
> >  }
> >  
> > -static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
> > +static void gic_cpu_init(struct gic_chip_data *gic)
> 
> I don't think you need to change this. With hotplug, this section will
> be kept.
> 
You are right.  I missed that it's a __cpuinit.

Regards,
Shawn

[PATCH 2/3] ARM: GIC: add gic_reinit() function to help ARM resume

[Shawn Guo]

On Tue, Aug 30, 2011 at 11:47:18PM +0800, Shawn Guo wrote:
> On Tue, Aug 30, 2011 at 10:00:35AM -0500, Rob Herring wrote:
> > Shawn,
> > 
> > On 08/30/2011 02:40 AM, Shawn Guo wrote:
> > > If ARM core gets powered off during suspend, GIC controller has to be
> > > reinitialized by resume procedure.  This patch adds a helper function
> > > for resume procedure to reinitialize GIC.
> > 
> > Is re-initializing rather than save/restore registers the right thing to
> > do here? Won't you lose things like edge or level triggered settings?
> > It's always been fuzzy to what should be done with interrupt masks in
> > suspend. Is every driver expected to disable and re-enable their
> > interrupts or this should be maintained thru a suspend cycle?
> > 
> Good point.  As one of the series, I easily went for the re-initializing
> approach here, plus it is pretty easy to go.  With your reminding, I
> agree that for interrupt controller save/restore registers might be the
> right thing to do.
> 
Rob,

As we share the same goal between Highbank and i.MX6Q on this, I'm
wondering if you already have something for save/restore registers
approach.  Otherwise, I may give a try on that.  But I want to avoid
duplicated effort.

-- 
Regards,
Shawn

[PATCH 3/3] ARM: smp_scu: remove __init annotation from scu_enable()

[Shawn Guo]

When Cortex-A9 MPCore resumes from Dormant or Shutdown modes,
SCU needs to be re-enabled.  This patch removes __init annotation
from function scu_enable(), so that platform resume procedure can
call it to re-enable SCU.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/kernel/smp_scu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index 79ed5e7..5b6d536 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -33,7 +33,7 @@ unsigned int __init scu_get_core_count(void __iomem *scu_base)
 /*
  * Enable the SCU
  */
-void __init scu_enable(void __iomem *scu_base)
+void scu_enable(void __iomem *scu_base)
 {
 	u32 scu_ctrl;
 
-- 
1.7.4.1