ARCv2: intc: untangle SMP, MCIP and IDU
PATCH 1/1 ARCv2: intc: untangle SMP, MCIP and IDU
The IDU intc is technically part of MCIP (Multi-core IP) hence historically was only available in a SMP hardware build (and thus only in a SMP kernel build). Now that hardware restriction has been lifted, so a UP kernel needs to support it.
This requires breaking mcip.c into parts which are strictly SMP (inter-core interrupts) and IDU which in reality is just another intc and thus has no bearing on SMP.
This change allows IDU in UP builds and with a suitable device tree, we can have the cascaded intc system
ARCv2 core intc <---> ARCv2 IDU intc <---> periperals
--- arch/arc/Kconfig | 17 +++++++++-------- arch/arc/include/asm/mcip.h | 16 ++++++++++++++++ arch/arc/kernel/mcip.c | 32 ++++++++++++-------------------- 3 files changed, 37 insertions(+), 28 deletions(-)
arch/arc/Kconfig +9 -8
@@ -186,14 +186,6 @@ if SMP config ARC_HAS_COH_CACHES def_bool n -config ARC_MCIP - bool "ARConnect Multicore IP (MCIP) Support " - depends on ISA_ARCV2 - help - This IP block enables SMP in ARC-HS38 cores. - It provides for cross-core interrupts, multi-core debug - hardware semaphores, shared memory,.... - config NR_CPUS int "Maximum number of CPUs (2-4096)" range 2 4096
@@ -211,6 +203,15 @@ config ARC_SMP_HALT_ON_RESET endif #SMP +config ARC_MCIP + bool "ARConnect Multicore IP (MCIP) Support " + depends on ISA_ARCV2 + default y if SMP + help + This IP block enables SMP in ARC-HS38 cores. + It provides for cross-core interrupts, multi-core debug + hardware semaphores, shared memory,.... + menuconfig ARC_CACHE bool "Enable Cache Support" default y
arch/arc/include/asm/mcip.h +16 -0
@@ -55,6 +55,22 @@ struct mcip_cmd {#define IDU_M_DISTRI_DEST 0x2 }; +struct mcip_bcr { +#ifdef CONFIG_CPU_BIG_ENDIAN +unsigned int pad3:8 , + idu:1 , llm:1 , num_cores:6 , + iocoh:1 , gfrc:1 , dbg:1 , pad2:1 , + msg:1 , sem:1 , ipi:1 , pad:1 , + ver:8 ; +#else +unsigned int ver:8 , + pad:1 , ipi:1 , sem:1 , msg:1 , + pad2:1 , dbg:1 , gfrc:1 , iocoh:1 , + num_cores:6 , llm:1 , idu:1 , + pad3:8 ; +#endif +}; +/* * MCIP programming model *
arch/arc/kernel/mcip.c +12 -20
@@ -15,11 +15,12 @@#include <asm/mcip.h> #include <asm/setup.h> -static char smp_cpuinfo_buf[128 ]; -static int idu_detected; -static DEFINE_RAW_SPINLOCK (mcip_lock); +#ifdef CONFIG_SMP + +static char smp_cpuinfo_buf[128 ]; +static void mcip_setup_per_cpu (int cpu) {smp_ipi_irq_setup (cpu,IPI_IRQ );
@@ -84,23 +85,10 @@ static void mcip_ipi_clear(int irq)raw_spin_unlock_irqrestore (&mcip_lock, flags); } +static void mcip_probe_n_setup (void ) { -struct mcip_bcr { -#ifdef CONFIG_CPU_BIG_ENDIAN -unsigned int pad3:8 , - idu:1 , llm:1 , num_cores:6 , - iocoh:1 , gfrc:1 , dbg:1 , pad2:1 , - msg:1 , sem:1 , ipi:1 , pad:1 , - ver:8 ; -#else -unsigned int ver:8 , - pad:1 , ipi:1 , sem:1 , msg:1 , - pad2:1 , dbg:1 , gfrc:1 , iocoh:1 , - num_cores:6 , llm:1 , idu:1 , - pad3:8 ; -#endif - } mp; +struct mcip_bcr mp;READ_BCR (ARC_REG_MCIP_BCR , mp);
@@ -114,7 +102,6 @@ static void mcip_probe_n_setup(void) IS_AVAIL1(mp.gfrc, "GFRC")); cpuinfo_arc700[0 ].extn.gfrc = mp.gfrc; - idu_detected = mp.idu;if (mp.dbg) {__mcip_cmd_data (CMD_DEBUG_SET_SELECT ,0 ,0xf );
@@ -130,6 +117,8 @@ struct plat_smp_ops plat_smp_ops = { .ipi_clear = mcip_ipi_clear, }; +#endif +/*************************************************************************** * ARCv2 Interrupt Distribution Unit (IDU) *
@@ -295,8 +284,11 @@ idu_of_init(struct device_node *intc, struct device_node *parent)/* Read IDU BCR to confirm nr_irqs */ int nr_irqs =of_irq_count (intc);int i, irq; +struct mcip_bcr mp; + +READ_BCR (ARC_REG_MCIP_BCR , mp); -if (!idu_detected) +if (!mp.idu)panic ("IDU not detected, but DeviceTree using it" );pr_info ("MCIP: IDU referenced from Devicetree %d irqs\n" , nr_irqs);
Signature · 1 line
2.7.4
Hi Vineet,
Quoted 9 lines · Vineet Gupta
The IDU intc is technically part of MCIP (Multi-core IP) hence historically was only available in a SMP hardware build (and thus only in a SMP kernel build). Now that hardware restriction has been lifted, so a UP kernel needs to support it.
This requires breaking
mcip.cinto parts which are strictly SMP (inter-core interrupts) and IDU which in reality is just another intc and thus has no bearing on SMP.
Quoted 7 lines
This change allows IDU in UP builds and with a suitable device tree, we can have the cascaded intc system
????ARCv2 core intc <---> ARCv2 IDU intc <---> periperals
Signed-off-by Vineet Gupta <vgupta at synopsys.com>---
[snip]
?
Quoted 5 lines
(quoted diff) +5
-0+struct mcip_bcr { +#ifdef CONFIG_CPU_BIG_ENDIAN + unsigned int pad3:8, + ?????idu:1, llm:1, num_cores:6, + ?????iocoh:1,??gfrc:1, dbg:1, pad2:1,
Review
Quoted 10 lines
(quoted diff) +10
-0+ ?????msg:1, sem:1, ipi:1, pad:1, + ?????ver:8; +#else + unsigned int ver:8, + ?????pad:1, ipi:1, sem:1, msg:1, + ?????pad2:1, dbg:1, gfrc:1, iocoh:1, + ?????num_cores:6, llm:1, idu:1, + ?????pad3:8; +#endif +};
IMHO we should stop using this kind of constructions because they are ugly and what's more important not portable.
Even though we have it now working for both LE and BE configurations it won't work for 64-bit cores. We'll need to add ifdeffed 32-bit paddings then which will make that construction even more ugly.
Probably that's not the right patch to address my complaint but just to reiterate this topic once again and think about clean-up series on that regard :)
-Alexey
Quoted 10 lines · Alexey Brodkin
Quoted 10 lines
arch/arc/include/asm/mcip.h +10
-0+struct mcip_bcr { +#ifdef CONFIG_CPU_BIG_ENDIAN +unsigned int pad3:8 , + idu:1 , llm:1 , num_cores:6 , + iocoh:1 , gfrc:1 , dbg:1 , pad2:1 , + msg:1 , sem:1 , ipi:1 , pad:1 , + ver:8 ; +#else +unsigned int ver:8 , + pad:1, ipi:1, sem:1, msg:1,
Quoted 8 lines
arch/arc/include/asm/mcip.h struct mcip_cmd { · line 72
Quoted 5 lines
arch/arc/include/asm/mcip.h +5
-0+ pad2:1, dbg:1, gfrc:1, iocoh:1, + num_cores:6, llm:1, idu:1, + pad3:8; +#endif +};IMHO we should stop using this kind of constructions because they are ugly and what's more important not portable.
They are ugly I agree - but not portable - really ? The whole point is to make this work on BE w/o changing the src code - this details remains hidden in an obscure header.
Quoted 3 lines
Even though we have it now working for both LE and BE configurations it won't work for 64-bit cores. We'll need to add ifdeffed 32-bit paddings then which will make that construction even more ugly.
When we get to 64-bit a lot things would have to change - and possibly the aux reg layout. There is no way to make this exact code 64-bit ready !
Quoted 3 lines
Probably that's not the right patch to address my complaint but just to reiterate this topic once again and think about clean-up series on that regard :)
Patches are welcome ;-)
-Vineet
Hi Vineet,
Quoted 22 lines · Vineet Gupta
Quoted 12 lines · Alexey Brodkin
Quoted 12 lines
arch/arc/include/asm/mcip.h +10
-0+struct mcip_bcr { +#ifdef CONFIG_CPU_BIG_ENDIAN +unsigned int pad3:8 , + ?????idu:1, llm:1, num_cores:6, + ?????iocoh:1,??gfrc:1, dbg:1, pad2:1, + ?????msg:1, sem:1, ipi:1, pad:1, + ?????ver:8; +#else +unsigned int ver:8 , + ?????pad:1, ipi:1, sem:1, msg:1,Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ ?????pad2:1, dbg:1, gfrc:1, iocoh:1, + ?????num_cores:6, llm:1, idu:1, + ?????pad3:8; +#endif +};IMHO we should stop using this kind of constructions because they are ugly and what's more important not portable.
Quoted 3 lines
They are ugly I agree - but not portable - really ? The whole point is to make this work on BE w/o changing the src code - this details remains hidden in an obscure header.
That's what I learned the hard way.
At least I was beaten a couple of times yet in both Linux kernel community and U-Boot
one.
Quoted 6 lines
Quoted 3 lines
Even though we have it now working for both LE and BE configurations it won't work for 64-bit cores. We'll need to add ifdeffed 32-bit paddings then which will make that construction even more ugly.
When we get to 64-bit a lot things would have to change - and possibly the aux reg layout. There is no way to make this exact code 64-bit ready !
Probably but as of now I believe use of offsets for bit-fields is the safest approach which makes code ugly as well but at least that way we reduce risk of erroneous copy-paste in "mirrored" part.
-Alexey
Quoted 7 lines · Alexey Brodkin
Quoted 3 lines
They are ugly I agree - but not portable - really ? The whole point is to make
Quoted 2 lines
this work on BE w/o changing the src code - this details remains hidden in an obscure header.
That's what I learned the hard way.
At least I was beaten a couple of times yet in both Linux kernel community and U-Boot
one.
Beaten for writing code like above - please point me to those discussions. I'd love to be educated in art of writing portable code !