Re: RFC, GIC based smp_cross_call cleanup suggestion

Re: RFC, GIC based smp_cross_call cleanup suggestion

[Santosh Shilimkar]

+ Thomas G,

On 4/2/2011 2:40 PM, Colin Cross wrote:
> On Sat, Apr 2, 2011 at 1:51 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk>  wrote:
>> On Fri, Apr 01, 2011 at 04:55:02PM -0600, Grant Likely wrote:
>>> On Fri, Apr 1, 2011 at 4:26 PM, John Linn<John.Linn@xilinx.com>  wrote:
>>>> I’m getting ready to submit a patch to add SMP to Xilinx code. I notice that
>>>> smp_cross_call for all GIC based platforms is duplicated across each
>>>> platform in smp.h.
>>>>
>>>>
>>>>
>>>> I thought I’d try to jump in to help with some cleanup, although I realize
>>>> it’s minimal, I have to start somewhere.
>>>>
>>>>
>>>>
>>>> What about moving the smp_cross_call for GIC based designs into gic.h?
>>>
>>> Go for it.  It's an obvious cleanup.
>>
>> That assumes that all SMP implementations will always have a GIC.  It
>> looks to me like this is conditional on shmobile, and so I don't think
>> its that trivial - maybe Paul or Magnus can first indicate why this is.
>
> OMAP4 may also require a custom smp_cross_call implementation if CPU
> idle is going to be supported in SMP - in CPU off idle modes, a GIC
> SGI will not wake the CPU, and a write directly to the CPU's power
> management controller or an external interrupt source would be
> required.

This can be done without making smp_cross_call() platform
specific.
While working on broad-cast notifiers for ARM with Thomas G, this
point was discussed.

Where the TWD can't wakeup its own local CPU from C3 mode, how do we
provide a platform specific method to perform this wakeup ?

Thomas Quoted....
"It would not complicate the OMAP code that much. All it needs is
extending the clock event device callbacks by an broadcast_affinity()
function which would be called from the broadcast code when the
broadcast device is armed. The argument would be a cpumask which would
tell you which core(s) to wake up when the broadcast timer fires next.

So OMAP would fill in that hook and implement the wakeup redirector
setup, which I guess would be a couple of lines."

 From above it's should be trivial once the broad-cast notifiers
are extended to have "broadcast_affinity()" supported

Regards
Santosh



--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC, GIC based smp_cross_call cleanup suggestion

[Colin Cross]

On Sat, Apr 2, 2011 at 11:23 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> + Thomas G,
>
> On 4/2/2011 2:40 PM, Colin Cross wrote:
>>
>> On Sat, Apr 2, 2011 at 1:51 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk>  wrote:
>>>
>>> On Fri, Apr 01, 2011 at 04:55:02PM -0600, Grant Likely wrote:
>>>>
>>>> On Fri, Apr 1, 2011 at 4:26 PM, John Linn<John.Linn@xilinx.com>  wrote:
>>>>>
>>>>> I’m getting ready to submit a patch to add SMP to Xilinx code. I notice
>>>>> that
>>>>> smp_cross_call for all GIC based platforms is duplicated across each
>>>>> platform in smp.h.
>>>>>
>>>>>
>>>>>
>>>>> I thought I’d try to jump in to help with some cleanup, although I
>>>>> realize
>>>>> it’s minimal, I have to start somewhere.
>>>>>
>>>>>
>>>>>
>>>>> What about moving the smp_cross_call for GIC based designs into gic.h?
>>>>
>>>> Go for it.  It's an obvious cleanup.
>>>
>>> That assumes that all SMP implementations will always have a GIC.  It
>>> looks to me like this is conditional on shmobile, and so I don't think
>>> its that trivial - maybe Paul or Magnus can first indicate why this is.
>>
>> OMAP4 may also require a custom smp_cross_call implementation if CPU
>> idle is going to be supported in SMP - in CPU off idle modes, a GIC
>> SGI will not wake the CPU, and a write directly to the CPU's power
>> management controller or an external interrupt source would be
>> required.
>
> This can be done without making smp_cross_call() platform
> specific.
> While working on broad-cast notifiers for ARM with Thomas G, this
> point was discussed.
>
> Where the TWD can't wakeup its own local CPU from C3 mode, how do we
> provide a platform specific method to perform this wakeup ?
>
> Thomas Quoted....
> "It would not complicate the OMAP code that much. All it needs is
> extending the clock event device callbacks by an broadcast_affinity()
> function which would be called from the broadcast code when the
> broadcast device is armed. The argument would be a cpumask which would
> tell you which core(s) to wake up when the broadcast timer fires next.
>
> So OMAP would fill in that hook and implement the wakeup redirector
> setup, which I guess would be a couple of lines."
>
> From above it's should be trivial once the broad-cast notifiers
> are extended to have "broadcast_affinity()" supported

That fixes the localtimer, but not calls to generic_exec_single, which
also calls smp_cross_call.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC, GIC based smp_cross_call cleanup suggestion

[Santosh Shilimkar]

[-- Attachment #1: Type: text/plain, Size: 2760 bytes --]

On 4/3/2011 7:18 PM, Russell King - ARM Linux wrote:
> On Sun, Apr 03, 2011 at 04:23:41PM +0530, Santosh Shilimkar wrote:
>> If you plan to commit this change then I can move these to
>> other OMAP4 header file.
>
> Can you send me a patch to do that please?
>
Here it is.. Generated against latest mainline.
Also attached just in case for any mailer issues.

 From 62563f7687d8b19f1c9c3966105d141d91e69b55 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Mon, 4 Apr 2011 12:48:37 +0530
Subject: [PATCH] OMAP4: Move the privately used SMP boot functions to 
OMAP specific header.

Header files in arch/arm/*/include/mach included from
arch/arm/include/asm/*.h are there to provide necessary definitions for
either the rest of the kernel or the ARM specific parts.  They shouldn't
be polluted with *any* platform private stuff which is not absolutely
necessary to satisfy the rest of the kernel.

Hence move the OMAP specific SMP boot functions to different header
instead of keeping them in 'plat/smp.h' which gets included indirectly
by linux/smp.h

The patch is outcome of the discussion in below thread:
	http://www.spinics.net/lists/arm-kernel/msg120363.html

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
  arch/arm/mach-omap2/include/mach/omap4-common.h |    7 +++++++
  arch/arm/plat-omap/include/plat/smp.h           |    6 ------
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h 
b/arch/arm/mach-omap2/include/mach/omap4-common.h
index de441c0..e4bd876 100644
--- a/arch/arm/mach-omap2/include/mach/omap4-common.h
+++ b/arch/arm/mach-omap2/include/mach/omap4-common.h
@@ -33,4 +33,11 @@ extern void __iomem *gic_dist_base_addr;
  extern void __init gic_init_irq(void);
  extern void omap_smc1(u32 fn, u32 arg);

+#ifdef CONFIG_SMP
+/* Needed for secondary core boot */
+extern void omap_secondary_startup(void);
+extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
+extern void omap_auxcoreboot_addr(u32 cpu_addr);
+extern u32 omap_read_auxcoreboot0(void);
+#endif
  #endif
diff --git a/arch/arm/plat-omap/include/plat/smp.h 
b/arch/arm/plat-omap/include/plat/smp.h
index 7a10257..416e9d5 100644
--- a/arch/arm/plat-omap/include/plat/smp.h
+++ b/arch/arm/plat-omap/include/plat/smp.h
@@ -19,12 +19,6 @@

  #include <asm/hardware/gic.h>

-/* Needed for secondary core boot */
-extern void omap_secondary_startup(void);
-extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
-extern void omap_auxcoreboot_addr(u32 cpu_addr);
-extern u32 omap_read_auxcoreboot0(void);
-
  /*
   * We use Soft IRQ1 as the IPI
   */
-- 
1.6.0.4


[-- Attachment #2: 0001-OMAP4-Move-the-privately-used-SMP-boot-functions-to.patch --]
[-- Type: text/plain, Size: 2390 bytes --]

>From 62563f7687d8b19f1c9c3966105d141d91e69b55 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Mon, 4 Apr 2011 12:48:37 +0530
Subject: [PATCH] OMAP4: Move the privately used SMP boot functions to OMAP specific header.

Header files in arch/arm/*/include/mach included from
arch/arm/include/asm/*.h are there to provide necessary definitions for
either the rest of the kernel or the ARM specific parts.  They shouldn't
be polluted with *any* platform private stuff which is not absolutely
necessary to satisfy the rest of the kernel.

Hence move the OMAP specific SMP boot functions to different header
instead of keeping them in 'plat/smp.h' which gets included indirectly
by linux/smp.h

The patch is outcome of the discussion in below thread:
	http://www.spinics.net/lists/arm-kernel/msg120363.html

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mach-omap2/include/mach/omap4-common.h |    7 +++++++
 arch/arm/plat-omap/include/plat/smp.h           |    6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h b/arch/arm/mach-omap2/include/mach/omap4-common.h
index de441c0..e4bd876 100644
--- a/arch/arm/mach-omap2/include/mach/omap4-common.h
+++ b/arch/arm/mach-omap2/include/mach/omap4-common.h
@@ -33,4 +33,11 @@ extern void __iomem *gic_dist_base_addr;
 extern void __init gic_init_irq(void);
 extern void omap_smc1(u32 fn, u32 arg);
 
+#ifdef CONFIG_SMP
+/* Needed for secondary core boot */
+extern void omap_secondary_startup(void);
+extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
+extern void omap_auxcoreboot_addr(u32 cpu_addr);
+extern u32 omap_read_auxcoreboot0(void);
+#endif
 #endif
diff --git a/arch/arm/plat-omap/include/plat/smp.h b/arch/arm/plat-omap/include/plat/smp.h
index 7a10257..416e9d5 100644
--- a/arch/arm/plat-omap/include/plat/smp.h
+++ b/arch/arm/plat-omap/include/plat/smp.h
@@ -19,12 +19,6 @@
 
 #include <asm/hardware/gic.h>
 
-/* Needed for secondary core boot */
-extern void omap_secondary_startup(void);
-extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
-extern void omap_auxcoreboot_addr(u32 cpu_addr);
-extern u32 omap_read_auxcoreboot0(void);
-
 /*
  * We use Soft IRQ1 as the IPI
  */
-- 
1.6.0.4

Re: RFC, GIC based smp_cross_call cleanup suggestion

[Russell King - ARM Linux]

On Mon, Apr 04, 2011 at 01:50:35PM +0530, Santosh Shilimkar wrote:
> On 4/3/2011 7:18 PM, Russell King - ARM Linux wrote:
>> On Sun, Apr 03, 2011 at 04:23:41PM +0530, Santosh Shilimkar wrote:
>>> If you plan to commit this change then I can move these to
>>> other OMAP4 header file.
>>
>> Can you send me a patch to do that please?
>>
> Here it is.. Generated against latest mainline.
> Also attached just in case for any mailer issues.

Looks good, please send to the patch system.

Re: RFC, GIC based smp_cross_call cleanup suggestion

[Santosh Shilimkar]

On 4/4/2011 1:55 PM, Russell King - ARM Linux wrote:
> On Mon, Apr 04, 2011 at 01:50:35PM +0530, Santosh Shilimkar wrote:
>> On 4/3/2011 7:18 PM, Russell King - ARM Linux wrote:
>>> On Sun, Apr 03, 2011 at 04:23:41PM +0530, Santosh Shilimkar wrote:
>>>> If you plan to commit this change then I can move these to
>>>> other OMAP4 header file.
>>>
>>> Can you send me a patch to do that please?
>>>
>> Here it is.. Generated against latest mainline.
>> Also attached just in case for any mailer issues.
>
> Looks good, please send to the patch system.
patch 6860/1

Regards
Santosh