Skip to contentSkip to conversation

ARC: Remove ISS_SMP_EXTN entirely

PATCH In review Paul Bolle 2 participants 7 messages Apr 8, 2014 Synopsys ARC architecture

lore
Share & tools
lore

Paul Bolle

author
#0 lore raw

PATCH 1/1 ARC: Remove ISS_SMP_EXTN entirely

The Kconfig symbol ARC_HAS_COH_RTSC was removed in v3.13. The Kconfig entry for ISS_SMP_EXTN still had a select for that symbol. That select was just removed in commit d345ea2892ae ("ARC: Remove ARC_HAS_COH_RTSC").

But selecting ARC_HAS_COH_RTSC was the only thing ISS_SMP_EXTN ever did. That means that since v3.13 setting ISS_SMP_EXTN was effectively a nop anyhow. So ISS_SMP_EXTN can as well be removed entirely.

---
Tested wit "git grep" only.

 arch/arc/plat-arcfpga/Kconfig | 13 -------------
 1 file changed, 13 deletions(-)
arch/arc/plat-arcfpga/Kconfig +0 -13
index 33058aa40e77..fde2a3fb604d 100644
@@ -8,7 +8,6 @@
 
 menuconfig ARC_PLAT_FPGA_LEGACY
 	bool "\"Legacy\" ARC FPGA dev Boards"
-	select ISS_SMP_EXTN if SMP
 	help
 	  Support for ARC development boards, provided by Synopsys.
 	  These are based on FPGA or ISS. e.g.
@@ -29,18 +28,6 @@ config ARC_BOARD_ML509
 	help
 	  ARC ML509 FPGA Ref Platform (Xilinx Virtex-5 Based)
 
-config ISS_SMP_EXTN
-	bool "ARC SMP Extensions (ISS Models only)"
-	default n
-	depends on SMP
-	help
-	  SMP Extensions to ARC700, in a "simulation only" Model, supported in
-	  ARC ISS (Instruction Set Simulator).
-	  The SMP extensions include:
-	  -IDU (Interrupt Distribution Unit)
-	  -XTL (To enable CPU start/stop/set-PC for another CPU)
-	  It doesn't provide coherent Caches and/or Atomic Ops (LLOCK/SCOND)
-
 config ARC_SERIAL_BAUD
 	int "UART Baud rate"
 	default "115200"
Signature · 1 line
1.9.0
#1 lore raw

Hi Paul,

Quoted 8 lines · Paul Bolle

The Kconfig symbol ARC_HAS_COH_RTSC was removed in v3.13. The Kconfig entry for ISS_SMP_EXTN still had a select for that symbol. That select was just removed in commit d345ea2892ae ("ARC: Remove ARC_HAS_COH_RTSC").

But selecting ARC_HAS_COH_RTSC was the only thing ISS_SMP_EXTN ever did. That means that since v3.13 setting ISS_SMP_EXTN was effectively a nop anyhow. So ISS_SMP_EXTN can as well be removed entirely.

There's obvious bitrot here - however this code is not dead yet ! I have a unpushed fix which uses this Kconfig item.

arch/arc/plat-arcfpga/Makefile +1 -1
index a44e22ebc1b7..4d1bddc34b5b 100644
@@ -9,4 +9,4 @@
 KBUILD_CFLAGS  += -Iarch/arc/plat-arcfpga/include
 
 obj-y := platform.o irq.o
-obj-$(CONFIG_SMP)              += smp.o
+obj-$(CONFIG_ISS_SMP_EXTN)             += smp.o

-Vineet

author
#2 lore raw

Vineet,

Quoted 2 lines · Vineet Gupta

There's obvious bitrot here - however this code is not dead yet ! I have a unpushed fix which uses this Kconfig item.

I see. Would it bother you if I send you a reminder if that fix (or something equivalent) hasn't been added in some future release?

Thanks,

Paul Bolle

author
#3 lore raw
In reply to Vineet Gupta

Vineet,

arch/arc/plat-arcfpga/Makefile line 12

Quoted 13 lines · Vineet Gupta

There's obvious bitrot here - however this code is not dead yet ! I have a unpushed fix which uses this Kconfig item.

arch/arc/plat-arcfpga/Makefile +1 -1
index a44e22ebc1b7..4d1bddc34b5b 100644
@@ -9,4 +9,4 @@
 KBUILD_CFLAGS  += -Iarch/arc/plat-arcfpga/include
 
 obj-y := platform.o irq.o
-obj-$(CONFIG_SMP)              += smp.o
+obj-$(CONFIG_ISS_SMP_EXTN)             += smp.o

I just realized that this means you're only using ISS_SMP_EXTN as an alias for SMP, aren't you? Ie, smp.o will now be built if both ARC_PLAT_FPGA_LEGACY and SMP are set. That will not change by this patch.

So unless you're planning to do something else too if ISS_SMP_EXTN is set there seems to be little value in using this symbol like that.

Paul Bolle

#4 lore raw
Quoted 21 lines · Paul Bolle

Vineet,

arch/arc/plat-arcfpga/Makefile line 12

Quoted 13 lines · Vineet Gupta

There's obvious bitrot here - however this code is not dead yet ! I have a unpushed fix which uses this Kconfig item.

arch/arc/plat-arcfpga/Makefile +1 -1
index a44e22ebc1b7..4d1bddc34b5b 100644
@@ -9,4 +9,4 @@
 KBUILD_CFLAGS  += -Iarch/arc/plat-arcfpga/include
 
 obj-y := platform.o irq.o
-obj-$(CONFIG_SMP)              += smp.o
+obj-$(CONFIG_ISS_SMP_EXTN)             += smp.o

I just realized that this means you're only using ISS_SMP_EXTN as an alias for SMP, aren't you? Ie, smp.o will now be built if both ARC_PLAT_FPGA_LEGACY and SMP are set. That will not change by this patch.

Quoted 2 lines

So unless you're planning to do something else too if ISS_SMP_EXTN is set there seems to be little value in using this symbol like that.

Indeed so - I just sent u one hunk to give an idea. There's other code too where CONFIG_SMP will be replaced by CONFIG_ISS_SMP_EXTN as I'm decoupling SMP support from this SMP extension.

I'll push a patch later today - which will hopefully hit mainline within this merge window - if not in one of the rcs.

-Vineet

#5 lore raw
Quoted 3 lines · Vineet Gupta

I'll push a patch later today - which will hopefully hit mainline within this merge window - if not in one of the rcs.

Here's the patch I want to push - care to take a look.

Note that it might still seem that ISS_SMP_EXTN symbol is uselss (and alias for SMP) because the code for SMP_EXTN_2 is still in works and will take some more time to hit upstream. So you see the motivation of this change now ?

--------------------->

Quoted 1 line

From c7f4c1084d2563def0f5bc5dc2b1b857bc4b50f0 Mon Sep 17 00:00:00 2001

From: Vineet Gupta <[email protected]>
Date: Fri, 28 Feb 2014 16:13:44 +0800
Subject: [PATCH] ARC: [SMP] ISS SMP extension bitrot

* Move extension specific code out of common SMP code
* Don't enable it by default for SMP

---
 arch/arc/include/asm/irq.h       |  2 --
 arch/arc/kernel/irq.c            | 18 ------------------
 arch/arc/plat-arcfpga/Makefile   |  2 +-
 arch/arc/plat-arcfpga/platform.c |  4 ++--
 arch/arc/plat-arcfpga/smp.c      | 18 ++++++++++++++++++
 5 files changed, 21 insertions(+), 23 deletions(-)
arch/arc/include/asm/irq.h +0 -2
index 291a70db68b8..781f57f25bd8 100644
@@ -19,8 +19,6 @@
 #include <asm-generic/irq.h>
 
 extern void arc_init_IRQ(void);
-extern int get_hw_config_num_irq(void);
-
 void arc_local_timer_setup(unsigned int cpu);
 
 #endif
arch/arc/kernel/irq.c +0 -18
index a4b141ee9a6a..7d653c0d0773 100644
@@ -150,24 +150,6 @@ void arch_do_IRQ(unsigned int irq, struct pt_regs *regs)
 	set_irq_regs(old_regs);
 }
 
-int get_hw_config_num_irq(void)
-{
-	uint32_t val = read_aux_reg(ARC_REG_VECBASE_BCR);
-
-	switch (val & 0x03) {
-	case 0:
-		return 16;
-	case 1:
-		return 32;
-	case 2:
-		return 8;
-	default:
-		return 0;
-	}
-
-	return 0;
-}
-
 /*
  * arch_local_irq_enable - Enable interrupts.
  *
arch/arc/plat-arcfpga/Makefile +1 -1
index a44e22ebc1b7..4d1bddc34b5b 100644
@@ -9,4 +9,4 @@
 KBUILD_CFLAGS	+= -Iarch/arc/plat-arcfpga/include
 
 obj-y := platform.o irq.o
-obj-$(CONFIG_SMP)		+= smp.o
+obj-$(CONFIG_ISS_SMP_EXTN)		+= smp.o
arch/arc/plat-arcfpga/platform.c +2 -2
index 19b76b61f44b..213394a15cd2 100644
@@ -169,7 +169,7 @@ static void __init plat_fpga_early_init(void)
 
 	arc_fpga_serial_init();
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_ISS_SMP_EXTN
 	iss_model_init_early_smp();
 #endif
 }
@@ -211,7 +211,7 @@ MACHINE_START(ANGEL4, "angel4")
 	.init_early	= plat_fpga_early_init,
 	.init_machine	= plat_fpga_populate_dev,
 	.init_irq	= plat_fpga_init_IRQ,
-#ifdef CONFIG_SMP
+#ifdef CONFIG_ISS_SMP_EXTN
 	.init_smp	= iss_model_init_smp,
 #endif
 MACHINE_END
arch/arc/plat-arcfpga/smp.c +18 -0
index 8a12741f5f7a..92bad9122077 100644
@@ -42,6 +42,24 @@ static void iss_model_smp_wakeup_cpu(int cpu, unsigned long pc)
 
 }
 
+static inline int get_hw_config_num_irq(void)
+{
+	uint32_t val = read_aux_reg(ARC_REG_VECBASE_BCR);
+
+	switch (val & 0x03) {
+	case 0:
+		return 16;
+	case 1:
+		return 32;
+	case 2:
+		return 8;
+	default:
+		return 0;
+	}
+
+	return 0;
+}
+
 /*
  * Any SMP specific init any CPU does when it comes up.
  * Here we setup the CPU to enable Inter-Processor-Interrupts
Signature · 1 line
1.8.3.2
author
#6 lore raw
In reply to Paul Bolle
Quoted 4 lines · Paul Bolle

I just realized that this means you're only using ISS_SMP_EXTN as an alias for SMP, aren't you? Ie, smp.o will now be built if both ARC_PLAT_FPGA_LEGACY and SMP are set. That will not change by this patch.

Of course, the patch does add the ability to set both ARC_PLAT_FPGA_LEGACY and SMP but still disable building smp.o. Would that be a useful ability?

Quoted 2 lines

So unless you're planning to do something else too if ISS_SMP_EXTN is set there seems to be little value in using this symbol like that.

Paul Bolle