linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
@ 2013-08-05  4:09 Olof Johansson
  2013-08-05  4:09 ` [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress Olof Johansson
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Olof Johansson @ 2013-08-05  4:09 UTC (permalink / raw)
  To: linux-arm-kernel

Here's a short series on top of Nico's TC2 MCPM series to move the SPC driver
back under mach-vexpress where it should be, and a couple of tiny cleanups
to the driver.

I'll pick this up together with the base branch in arm-soc for-next, but I
haven't staged it in a next/* branch so it can be rebased/changed/dropped
if needed.

There's still one thing left to iron out, and that's the actual binding
for the "SPC" node. Since the spc code needs a non-zero SPC_BASE, it
implies that there's more than just SPC described by the device node. So
either the naming and description needs to be changed, or there's need
for more than one device node to describe the other register ranges. The
former might be easier in this case...

Pawel, I'd appreciate a quick test from you to make sure I didn't break
anything since I don't have a way to boot this myself.


-Olof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress
  2013-08-05  4:09 [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Olof Johansson
@ 2013-08-05  4:09 ` Olof Johansson
  2013-08-05  9:52   ` Lorenzo Pieralisi
  2013-08-05  4:09 ` [PATCH 2/3] ARM: vexpress: make spc code TC2-only Olof Johansson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Olof Johansson @ 2013-08-05  4:09 UTC (permalink / raw)
  To: linux-arm-kernel

Otherwise we have tc2_pm platform code calling into exported driver functions,
which doesn't make sense at this level.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm/mach-vexpress/Makefile                         |  2 +-
 .../platform/vexpress => arch/arm/mach-vexpress}/spc.c  |  1 -
 arch/arm/mach-vexpress/tc2_pm.c                         |  3 ++-
 drivers/platform/Kconfig                                |  3 ---
 drivers/platform/Makefile                               |  1 -
 drivers/platform/vexpress/Kconfig                       |  9 ---------
 drivers/platform/vexpress/Makefile                      |  1 -
 include/linux/vexpress.h                                | 17 -----------------
 8 files changed, 3 insertions(+), 34 deletions(-)
 rename {drivers/platform/vexpress => arch/arm/mach-vexpress}/spc.c (99%)
 delete mode 100644 drivers/platform/vexpress/Kconfig
 delete mode 100644 drivers/platform/vexpress/Makefile

diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
index 0853da6..36ea824 100644
--- a/arch/arm/mach-vexpress/Makefile
+++ b/arch/arm/mach-vexpress/Makefile
@@ -7,6 +7,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \
 obj-y					:= v2m.o
 obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
 obj-$(CONFIG_ARCH_VEXPRESS_DCSCB)	+= dcscb.o	dcscb_setup.o
-obj-$(CONFIG_ARCH_VEXPRESS_TC2_PM)	+= tc2_pm.o
+obj-$(CONFIG_ARCH_VEXPRESS_TC2_PM)	+= tc2_pm.o spc.o
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o
diff --git a/drivers/platform/vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
similarity index 99%
rename from drivers/platform/vexpress/spc.c
rename to arch/arm/mach-vexpress/spc.c
index aa8c2a4..34e4bf4 100644
--- a/drivers/platform/vexpress/spc.c
+++ b/arch/arm/mach-vexpress/spc.c
@@ -250,4 +250,3 @@ int __init ve_spc_init(void)
 
 	return ve_spc_init_status;
 }
-early_initcall(ve_spc_init);
diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
index dfb55d4..79cf4bc 100644
--- a/arch/arm/mach-vexpress/tc2_pm.c
+++ b/arch/arm/mach-vexpress/tc2_pm.c
@@ -23,9 +23,10 @@
 #include <asm/cputype.h>
 #include <asm/cp15.h>
 
-#include <linux/vexpress.h>
 #include <linux/arm-cci.h>
 
+#include "spc.h"
+
 /*
  * We can't use regular spinlocks. In the switcher case, it is possible
  * for an outbound CPU to call power_down() after its inbound counterpart
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 449010e..57811ea 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -4,6 +4,3 @@ endif
 if GOLDFISH
 source "drivers/platform/goldfish/Kconfig"
 endif
-if ARCH_VEXPRESS
-source "drivers/platform/vexpress/Kconfig"
-endif
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index c1173d6..8a44a4c 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -5,4 +5,3 @@
 obj-$(CONFIG_X86)		+= x86/
 obj-$(CONFIG_OLPC)		+= olpc/
 obj-$(CONFIG_GOLDFISH)		+= goldfish/
-obj-$(CONFIG_ARCH_VEXPRESS)	+= vexpress/
diff --git a/drivers/platform/vexpress/Kconfig b/drivers/platform/vexpress/Kconfig
deleted file mode 100644
index d09a7c4..0000000
--- a/drivers/platform/vexpress/Kconfig
+++ /dev/null
@@ -1,9 +0,0 @@
-config VEXPRESS_SPC
-	bool "Versatile Express SPC driver support"
-	help
-	  The Serial Power Controller (SPC) for ARM Ltd. test chips, is
-	  an IP that provides a memory mapped interface to power controller
-	  HW. The driver provides an API abstraction meant to be used by
-	  subsystem drivers allowing to program registers controlling
-	  low-level power management features like power down flags,
-	  global and per-cpu wake-up IRQs.
diff --git a/drivers/platform/vexpress/Makefile b/drivers/platform/vexpress/Makefile
deleted file mode 100644
index d31eca2..0000000
--- a/drivers/platform/vexpress/Makefile
+++ /dev/null
@@ -1 +0,0 @@
-obj-$(CONFIG_VEXPRESS_SPC)	+= spc.o
diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
index 3e35556..617c01b 100644
--- a/include/linux/vexpress.h
+++ b/include/linux/vexpress.h
@@ -124,21 +124,4 @@ void vexpress_osc_of_setup(struct device_node *node);
 void vexpress_clk_init(void __iomem *sp810_base);
 void vexpress_clk_of_init(void);
 
-/* SPC */
-
-#ifdef CONFIG_VEXPRESS_SPC
-int __init ve_spc_init(void);
-void ve_spc_global_wakeup_irq(bool set);
-void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set);
-void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr);
-u32 ve_spc_get_nr_cpus(u32 cluster);
-void ve_spc_powerdown(u32 cluster, bool enable);
-#else
-static inline int ve_spc_init(void) { return -ENODEV; }
-static inline void ve_spc_global_wakeup_irq(bool set) { }
-static inline void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set) { }
-static inline void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr) { }
-static inline u32 ve_spc_get_nr_cpus(u32 cluster) { return 0; }
-static inline void ve_spc_powerdown(u32 cluster, bool enable) { }
-#endif
 #endif
-- 
1.8.1.192.gc4361b8

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] ARM: vexpress: make spc code TC2-only
  2013-08-05  4:09 [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Olof Johansson
  2013-08-05  4:09 ` [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress Olof Johansson
@ 2013-08-05  4:09 ` Olof Johansson
  2013-08-05  9:55   ` Lorenzo Pieralisi
  2013-08-05  4:09 ` [PATCH 3/3] ARM: vexpress: don't print virtual address in dmesg Olof Johansson
  2013-08-05  9:47 ` [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Lorenzo Pieralisi
  3 siblings, 1 reply; 20+ messages in thread
From: Olof Johansson @ 2013-08-05  4:09 UTC (permalink / raw)
  To: linux-arm-kernel

It doesn't make sense to match this to the generic compatible value,
since the code is quite specific to TC2 as it is.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm/mach-vexpress/spc.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
index 34e4bf4..30cfa9f 100644
--- a/arch/arm/mach-vexpress/spc.c
+++ b/arch/arm/mach-vexpress/spc.c
@@ -187,18 +187,14 @@ void ve_spc_powerdown(u32 cluster, bool enable)
 	writel_relaxed(enable, info->baseaddr + pwdrn_reg);
 }
 
-static const struct of_device_id ve_spc_ids[] __initconst = {
-	{ .compatible = "arm,vexpress-spc,v2p-ca15_a7" },
-	{ .compatible = "arm,vexpress-spc" },
-	{},
-};
-
 static int __init ve_spc_probe(void)
 {
 	int ret;
-	struct device_node *node = of_find_matching_node(NULL, ve_spc_ids);
+	struct device_node *dn;
+
+	dn = of_find_compatible_node(NULL, "arm,vexpress-spc,v2p-ca15_a7");
 
-	if (!node)
+	if (!dn)
 		return -ENODEV;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
@@ -207,7 +203,7 @@ static int __init ve_spc_probe(void)
 		return -ENOMEM;
 	}
 
-	info->baseaddr = of_iomap(node, 0);
+	info->baseaddr = of_iomap(dn, 0);
 	if (!info->baseaddr) {
 		pr_err(SPCLOG "unable to ioremap memory\n");
 		ret = -ENXIO;
-- 
1.8.1.192.gc4361b8

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] ARM: vexpress: don't print virtual address in dmesg
  2013-08-05  4:09 [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Olof Johansson
  2013-08-05  4:09 ` [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress Olof Johansson
  2013-08-05  4:09 ` [PATCH 2/3] ARM: vexpress: make spc code TC2-only Olof Johansson
@ 2013-08-05  4:09 ` Olof Johansson
  2013-08-05  9:47 ` [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Lorenzo Pieralisi
  3 siblings, 0 replies; 20+ messages in thread
From: Olof Johansson @ 2013-08-05  4:09 UTC (permalink / raw)
  To: linux-arm-kernel

Printing out the virtual address of an ioremap in dmesg makes no real
sense. Just drop it.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm/mach-vexpress/spc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
index 30cfa9f..1001c6c 100644
--- a/arch/arm/mach-vexpress/spc.c
+++ b/arch/arm/mach-vexpress/spc.c
@@ -219,7 +219,7 @@ static int __init ve_spc_probe(void)
 	 */
 	sync_cache_w(info);
 	sync_cache_w(&info);
-	pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
+
 	return 0;
 
 mem_free:
-- 
1.8.1.192.gc4361b8

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-05  4:09 [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Olof Johansson
                   ` (2 preceding siblings ...)
  2013-08-05  4:09 ` [PATCH 3/3] ARM: vexpress: don't print virtual address in dmesg Olof Johansson
@ 2013-08-05  9:47 ` Lorenzo Pieralisi
  2013-08-05 16:45   ` Olof Johansson
  2013-08-06 16:19   ` Pawel Moll
  3 siblings, 2 replies; 20+ messages in thread
From: Lorenzo Pieralisi @ 2013-08-05  9:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 05, 2013 at 05:09:50AM +0100, Olof Johansson wrote:
> Here's a short series on top of Nico's TC2 MCPM series to move the SPC driver
> back under mach-vexpress where it should be, and a couple of tiny cleanups
> to the driver.
> 
> I'll pick this up together with the base branch in arm-soc for-next, but I
> haven't staged it in a next/* branch so it can be rebased/changed/dropped
> if needed.
> 
> There's still one thing left to iron out, and that's the actual binding
> for the "SPC" node. Since the spc code needs a non-zero SPC_BASE, it
> implies that there's more than just SPC described by the device node. So
> either the naming and description needs to be changed, or there's need
> for more than one device node to describe the other register ranges. The
> former might be easier in this case...
> 
> Pawel, I'd appreciate a quick test from you to make sure I didn't break
> anything since I don't have a way to boot this myself.

Thanks Olof.

Tested, a couple of fixes needed to compile your series, comments in the
relative patches.

For the bindings, you are right, how about this (commit log written just
for the sake of it, it should be squashed in Nico's original series) ?

-- >8 --
Subject: [PATCH] Documentation: devicetree: arm: rename SPC device tree
 bindings to SCC

The Serial Power Controller (SPC) is part of an HW IP called Serial
Configuration Controller (SCC), hence it must not define an independent
devicetree node and relative bindings. This patch replaces the existing
SPC documentation with the more generic SCC bindings.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 .../devicetree/bindings/arm/vexpress-scc.txt       | 37 ++++++++++++++++++++++
 .../devicetree/bindings/arm/vexpress-spc.txt       | 36 ---------------------
 2 files changed, 37 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress-scc.txt
 delete mode 100644 Documentation/devicetree/bindings/arm/vexpress-spc.txt

diff --git a/Documentation/devicetree/bindings/arm/vexpress-scc.txt b/Documentation/devicetree/bindings/arm/vexpress-scc.txt
new file mode 100644
index 0000000..f25849d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/vexpress-scc.txt
@@ -0,0 +1,37 @@
+* ARM Versatile Express Serial Configuration Controller device tree bindings
+
+Latest ARM development boards implement a serial configuration interface (SCC)
+that can be used to retrieve temperature sensors, energy/voltage/current
+probes and oscillators values through the SYS configuration protocol defined
+for versatile express motherboards.
+It also provides a power management interface (serial power controller - SPC)
+that is capable of managing power/voltage and operating points transitions,
+wake-up IRQs and resume addresses for ARM multiprocessor testchips.
+
+The SCC DT bindings are defined as follows:
+
+- scc node
+
+	- compatible:
+		Usage: required
+		Value type: <stringlist>
+		Definition: must be
+			    "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc"
+	- reg:
+		Usage: required
+		Value type: <prop-encode-array>
+		Definition: A standard property that specifies the base address
+			    and the size of the SCC address space
+	- interrupts:
+		Usage: required
+		Value type: <prop-encoded-array>
+		Definition:  SCC interrupt configuration. A standard property
+			     that follows ePAPR interrupts specifications
+
+Example:
+
+scc: scc at 7fff0000 {
+	compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
+	reg = <0x7fff0000 0x1000>;
+	interrupts = <0 95 4>;
+};
diff --git a/Documentation/devicetree/bindings/arm/vexpress-spc.txt b/Documentation/devicetree/bindings/arm/vexpress-spc.txt
deleted file mode 100644
index 1614725..0000000
--- a/Documentation/devicetree/bindings/arm/vexpress-spc.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-* ARM Versatile Express Serial Power Controller device tree bindings
-
-Latest ARM development boards implement a power management interface (serial
-power controller - SPC) that is capable of managing power states transitions,
-wake-up IRQs and resume addresses for ARM multiprocessor testchips.
-The serial controller can be programmed through a memory mapped interface
-that enables communication between firmware running on the microcontroller
-managing power states and the application processors.
-
-The SPC DT bindings are defined as follows:
-
-- spc node
-
-	- compatible:
-		Usage: required
-		Value type: <stringlist>
-		Definition: must be
-			    "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc"
-	- reg:
-		Usage: required
-		Value type: <prop-encode-array>
-		Definition: A standard property that specifies the base address
-			    and the size of the SPC address space
-	- interrupts:
-		Usage: required
-		Value type: <prop-encoded-array>
-		Definition:  SPC interrupt configuration. A standard property
-			     that follows ePAPR interrupts specifications
-
-Example:
-
-spc: spc at 7fff0000 {
-	compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
-	reg = <0x7fff0000 0x1000>;
-	interrupts = <0 95 4>;
-};
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress
  2013-08-05  4:09 ` [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress Olof Johansson
@ 2013-08-05  9:52   ` Lorenzo Pieralisi
  2013-08-05 23:26     ` Olof Johansson
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Pieralisi @ 2013-08-05  9:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 05, 2013 at 05:09:51AM +0100, Olof Johansson wrote:
> Otherwise we have tc2_pm platform code calling into exported driver functions,
> which doesn't make sense at this level.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  arch/arm/mach-vexpress/Makefile                         |  2 +-
>  .../platform/vexpress => arch/arm/mach-vexpress}/spc.c  |  1 -
>  arch/arm/mach-vexpress/tc2_pm.c                         |  3 ++-
>  drivers/platform/Kconfig                                |  3 ---
>  drivers/platform/Makefile                               |  1 -
>  drivers/platform/vexpress/Kconfig                       |  9 ---------
>  drivers/platform/vexpress/Makefile                      |  1 -
>  include/linux/vexpress.h                                | 17 -----------------
>  8 files changed, 3 insertions(+), 34 deletions(-)
>  rename {drivers/platform/vexpress => arch/arm/mach-vexpress}/spc.c (99%)
>  delete mode 100644 drivers/platform/vexpress/Kconfig
>  delete mode 100644 drivers/platform/vexpress/Makefile
> 
> diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> index 0853da6..36ea824 100644
> --- a/arch/arm/mach-vexpress/Makefile
> +++ b/arch/arm/mach-vexpress/Makefile
> @@ -7,6 +7,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \
>  obj-y					:= v2m.o
>  obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
>  obj-$(CONFIG_ARCH_VEXPRESS_DCSCB)	+= dcscb.o	dcscb_setup.o
> -obj-$(CONFIG_ARCH_VEXPRESS_TC2_PM)	+= tc2_pm.o
> +obj-$(CONFIG_ARCH_VEXPRESS_TC2_PM)	+= tc2_pm.o spc.o
>  obj-$(CONFIG_SMP)			+= platsmp.o
>  obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o
> diff --git a/drivers/platform/vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
> similarity index 99%
> rename from drivers/platform/vexpress/spc.c
> rename to arch/arm/mach-vexpress/spc.c
> index aa8c2a4..34e4bf4 100644
> --- a/drivers/platform/vexpress/spc.c
> +++ b/arch/arm/mach-vexpress/spc.c
> @@ -250,4 +250,3 @@ int __init ve_spc_init(void)
>  
>  	return ve_spc_init_status;
>  }
> -early_initcall(ve_spc_init);
> diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
> index dfb55d4..79cf4bc 100644
> --- a/arch/arm/mach-vexpress/tc2_pm.c
> +++ b/arch/arm/mach-vexpress/tc2_pm.c
> @@ -23,9 +23,10 @@
>  #include <asm/cputype.h>
>  #include <asm/cp15.h>
>  
> -#include <linux/vexpress.h>
>  #include <linux/arm-cci.h>
>  
> +#include "spc.h"

Missing. Should we add it to mach-vexpress/include/mach ?

I have not sent a patch to check what we should do first and if Pawel is
ok with these changes.

Lorenzo

> +
>  /*
>   * We can't use regular spinlocks. In the switcher case, it is possible
>   * for an outbound CPU to call power_down() after its inbound counterpart
> diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> index 449010e..57811ea 100644
> --- a/drivers/platform/Kconfig
> +++ b/drivers/platform/Kconfig
> @@ -4,6 +4,3 @@ endif
>  if GOLDFISH
>  source "drivers/platform/goldfish/Kconfig"
>  endif
> -if ARCH_VEXPRESS
> -source "drivers/platform/vexpress/Kconfig"
> -endif
> diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> index c1173d6..8a44a4c 100644
> --- a/drivers/platform/Makefile
> +++ b/drivers/platform/Makefile
> @@ -5,4 +5,3 @@
>  obj-$(CONFIG_X86)		+= x86/
>  obj-$(CONFIG_OLPC)		+= olpc/
>  obj-$(CONFIG_GOLDFISH)		+= goldfish/
> -obj-$(CONFIG_ARCH_VEXPRESS)	+= vexpress/
> diff --git a/drivers/platform/vexpress/Kconfig b/drivers/platform/vexpress/Kconfig
> deleted file mode 100644
> index d09a7c4..0000000
> --- a/drivers/platform/vexpress/Kconfig
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -config VEXPRESS_SPC
> -	bool "Versatile Express SPC driver support"
> -	help
> -	  The Serial Power Controller (SPC) for ARM Ltd. test chips, is
> -	  an IP that provides a memory mapped interface to power controller
> -	  HW. The driver provides an API abstraction meant to be used by
> -	  subsystem drivers allowing to program registers controlling
> -	  low-level power management features like power down flags,
> -	  global and per-cpu wake-up IRQs.
> diff --git a/drivers/platform/vexpress/Makefile b/drivers/platform/vexpress/Makefile
> deleted file mode 100644
> index d31eca2..0000000
> --- a/drivers/platform/vexpress/Makefile
> +++ /dev/null
> @@ -1 +0,0 @@
> -obj-$(CONFIG_VEXPRESS_SPC)	+= spc.o
> diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
> index 3e35556..617c01b 100644
> --- a/include/linux/vexpress.h
> +++ b/include/linux/vexpress.h
> @@ -124,21 +124,4 @@ void vexpress_osc_of_setup(struct device_node *node);
>  void vexpress_clk_init(void __iomem *sp810_base);
>  void vexpress_clk_of_init(void);
>  
> -/* SPC */
> -
> -#ifdef CONFIG_VEXPRESS_SPC
> -int __init ve_spc_init(void);
> -void ve_spc_global_wakeup_irq(bool set);
> -void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set);
> -void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr);
> -u32 ve_spc_get_nr_cpus(u32 cluster);
> -void ve_spc_powerdown(u32 cluster, bool enable);
> -#else
> -static inline int ve_spc_init(void) { return -ENODEV; }
> -static inline void ve_spc_global_wakeup_irq(bool set) { }
> -static inline void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set) { }
> -static inline void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr) { }
> -static inline u32 ve_spc_get_nr_cpus(u32 cluster) { return 0; }
> -static inline void ve_spc_powerdown(u32 cluster, bool enable) { }
> -#endif
>  #endif
> -- 
> 1.8.1.192.gc4361b8
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3] ARM: vexpress: make spc code TC2-only
  2013-08-05  4:09 ` [PATCH 2/3] ARM: vexpress: make spc code TC2-only Olof Johansson
@ 2013-08-05  9:55   ` Lorenzo Pieralisi
  2013-08-05 23:28     ` Olof Johansson
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Pieralisi @ 2013-08-05  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 05, 2013 at 05:09:52AM +0100, Olof Johansson wrote:
> It doesn't make sense to match this to the generic compatible value,
> since the code is quite specific to TC2 as it is.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  arch/arm/mach-vexpress/spc.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
> index 34e4bf4..30cfa9f 100644
> --- a/arch/arm/mach-vexpress/spc.c
> +++ b/arch/arm/mach-vexpress/spc.c
> @@ -187,18 +187,14 @@ void ve_spc_powerdown(u32 cluster, bool enable)
>  	writel_relaxed(enable, info->baseaddr + pwdrn_reg);
>  }
>  
> -static const struct of_device_id ve_spc_ids[] __initconst = {
> -	{ .compatible = "arm,vexpress-spc,v2p-ca15_a7" },
> -	{ .compatible = "arm,vexpress-spc" },
> -	{},
> -};
> -
>  static int __init ve_spc_probe(void)
>  {
>  	int ret;
> -	struct device_node *node = of_find_matching_node(NULL, ve_spc_ids);
> +	struct device_node *dn;
> +
> +	dn = of_find_compatible_node(NULL, "arm,vexpress-spc,v2p-ca15_a7");

Missing a parameter, should be:

of_find_compatible_node(NULL, NULL, "arm,vexpress-spc,v2p-ca15_a7");

If my changes to the bindings are acceptable we must match the new string.

Thanks,
Lorenzo

>  
> -	if (!node)
> +	if (!dn)
>  		return -ENODEV;
>  
>  	info = kzalloc(sizeof(*info), GFP_KERNEL);
> @@ -207,7 +203,7 @@ static int __init ve_spc_probe(void)
>  		return -ENOMEM;
>  	}
>  
> -	info->baseaddr = of_iomap(node, 0);
> +	info->baseaddr = of_iomap(dn, 0);
>  	if (!info->baseaddr) {
>  		pr_err(SPCLOG "unable to ioremap memory\n");
>  		ret = -ENXIO;
> -- 
> 1.8.1.192.gc4361b8
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-05  9:47 ` [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Lorenzo Pieralisi
@ 2013-08-05 16:45   ` Olof Johansson
  2013-08-06 16:19   ` Pawel Moll
  1 sibling, 0 replies; 20+ messages in thread
From: Olof Johansson @ 2013-08-05 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 5, 2013 at 2:47 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Mon, Aug 05, 2013 at 05:09:50AM +0100, Olof Johansson wrote:
>> Here's a short series on top of Nico's TC2 MCPM series to move the SPC driver
>> back under mach-vexpress where it should be, and a couple of tiny cleanups
>> to the driver.
>>
>> I'll pick this up together with the base branch in arm-soc for-next, but I
>> haven't staged it in a next/* branch so it can be rebased/changed/dropped
>> if needed.
>>
>> There's still one thing left to iron out, and that's the actual binding
>> for the "SPC" node. Since the spc code needs a non-zero SPC_BASE, it
>> implies that there's more than just SPC described by the device node. So
>> either the naming and description needs to be changed, or there's need
>> for more than one device node to describe the other register ranges. The
>> former might be easier in this case...
>>
>> Pawel, I'd appreciate a quick test from you to make sure I didn't break
>> anything since I don't have a way to boot this myself.
>
> Thanks Olof.
>
> Tested, a couple of fixes needed to compile your series, comments in the
> relative patches.

Thanks. For some reason I figured vexpress_defconfig would provide
build coverage, so it obviously missed build coverage by me. I also
missed a git add.

> For the bindings, you are right, how about this (commit log written just
> for the sake of it, it should be squashed in Nico's original series) ?

There's no harm in keeping the change history by having two patches.
I'll add this below my series of three when I revise them.



-Olof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress
  2013-08-05  9:52   ` Lorenzo Pieralisi
@ 2013-08-05 23:26     ` Olof Johansson
  2013-08-06  8:55       ` Lorenzo Pieralisi
  0 siblings, 1 reply; 20+ messages in thread
From: Olof Johansson @ 2013-08-05 23:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 05, 2013 at 10:52:35AM +0100, Lorenzo Pieralisi wrote:
> > +#include "spc.h"
> 
> Missing. Should we add it to mach-vexpress/include/mach ?

Oops, missed the git add on it.

No, include/mach is going away on all multiplatform platforms. Since the
exposed functions are only used in the other file in the same directory, local
include file is the way to go.

> I have not sent a patch to check what we should do first and if Pawel is
> ok with these changes.

Not sure I'm following, but see above.


-Olof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3] ARM: vexpress: make spc code TC2-only
  2013-08-05  9:55   ` Lorenzo Pieralisi
@ 2013-08-05 23:28     ` Olof Johansson
  0 siblings, 0 replies; 20+ messages in thread
From: Olof Johansson @ 2013-08-05 23:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 05, 2013 at 10:55:32AM +0100, Lorenzo Pieralisi wrote:
> On Mon, Aug 05, 2013 at 05:09:52AM +0100, Olof Johansson wrote:
> > It doesn't make sense to match this to the generic compatible value,
> > since the code is quite specific to TC2 as it is.
> > 
> > Signed-off-by: Olof Johansson <olof@lixom.net>
> > ---
> >  arch/arm/mach-vexpress/spc.c | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
> > index 34e4bf4..30cfa9f 100644
> > --- a/arch/arm/mach-vexpress/spc.c
> > +++ b/arch/arm/mach-vexpress/spc.c
> > @@ -187,18 +187,14 @@ void ve_spc_powerdown(u32 cluster, bool enable)
> >  	writel_relaxed(enable, info->baseaddr + pwdrn_reg);
> >  }
> >  
> > -static const struct of_device_id ve_spc_ids[] __initconst = {
> > -	{ .compatible = "arm,vexpress-spc,v2p-ca15_a7" },
> > -	{ .compatible = "arm,vexpress-spc" },
> > -	{},
> > -};
> > -
> >  static int __init ve_spc_probe(void)
> >  {
> >  	int ret;
> > -	struct device_node *node = of_find_matching_node(NULL, ve_spc_ids);
> > +	struct device_node *dn;
> > +
> > +	dn = of_find_compatible_node(NULL, "arm,vexpress-spc,v2p-ca15_a7");
> 
> Missing a parameter, should be:
> 
> of_find_compatible_node(NULL, NULL, "arm,vexpress-spc,v2p-ca15_a7");

My bad, thinking vexpress_defconfig enabled this so I didn't make sure I had
build coverage.

> If my changes to the bindings are acceptable we must match the new string.

Done.


-Olof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress
  2013-08-05 23:26     ` Olof Johansson
@ 2013-08-06  8:55       ` Lorenzo Pieralisi
  2013-08-06 12:28         ` Pawel Moll
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Pieralisi @ 2013-08-06  8:55 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 06, 2013 at 12:26:05AM +0100, Olof Johansson wrote:
> On Mon, Aug 05, 2013 at 10:52:35AM +0100, Lorenzo Pieralisi wrote:
> > > +#include "spc.h"
> > 
> > Missing. Should we add it to mach-vexpress/include/mach ?
> 
> Oops, missed the git add on it.
> 
> No, include/mach is going away on all multiplatform platforms. Since the
> exposed functions are only used in the other file in the same directory, local
> include file is the way to go.
> 
> > I have not sent a patch to check what we should do first and if Pawel is
> > ok with these changes.
> 
> Not sure I'm following, but see above.

I just wanted to say that sending a patch to fix the issue was inappropriate
because it depended on where the spc.h has to be created and did not
know if Pawel was ok with the idea of moving spc back to mach-vexpress.

I am ok with your changes, thank you very much.

Lorenzo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress
  2013-08-06  8:55       ` Lorenzo Pieralisi
@ 2013-08-06 12:28         ` Pawel Moll
  0 siblings, 0 replies; 20+ messages in thread
From: Pawel Moll @ 2013-08-06 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2013-08-06 at 09:55 +0100, Lorenzo Pieralisi wrote:
> I just wanted to say that sending a patch to fix the issue was inappropriate
> because it depended on where the spc.h has to be created and did not
> know if Pawel was ok with the idea of moving spc back to mach-vexpress.
> 
> I am ok with your changes, thank you very much.

Although it works against the goal of removing arch/arm/mach-vexpress
completely ;-) I appreciate the fact that there's more than that on the
way, so yeah, it's fine with me.

Cheers!

Pawel

PS. Apologies about the delay and thanks for testing it out, Lorenzo!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-05  9:47 ` [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Lorenzo Pieralisi
  2013-08-05 16:45   ` Olof Johansson
@ 2013-08-06 16:19   ` Pawel Moll
  2013-08-06 16:44     ` Olof Johansson
  2013-08-06 16:55     ` Lorenzo Pieralisi
  1 sibling, 2 replies; 20+ messages in thread
From: Pawel Moll @ 2013-08-06 16:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2013-08-05 at 10:47 +0100, Lorenzo Pieralisi wrote:
> For the bindings, you are right, how about this (commit log written just
> for the sake of it, it should be squashed in Nico's original series) ?

So I finally sat down and did what I should have done long time ago...
Read the spec ;-)

SCC is (was?) mainly used to set initial conditions for the cores,
interconnect and all the other bits and pieces inside the test chip. It
could be considered an equivalent of the classic "boot mode" pins, but
as test chips have many more of them comparing to the normal SOCs, the
data is being "injected" to the chip in a serial fashion *before* the
main reset is being de-asserted. This is done by an external entity,
namely the DCC (daughterboard configuration controller, so simply a
microcontroller living next to the test chip). Why is this detail
important at all? Because traditionally the SCC was *not* available in
the normal memory map, otherwise it would be a perfect candidate for
the . In order to change anything one had to go through the usual
Versatile Express config infrastructure. Fortunately there was no need
to do this at all...

And here comes the V2P-CA15_A7, also known as TC2 ;-) where the
interface was re-(or ab-?)used as a "convenient" communication channel
between the test chip and the microcontroller. And the SPC is even
described as "being merged" with the SCC. Uh...

Now, the bottom line. How about keeping the driver look for
"arm,vexpress-spc,v2p-ca15_a7" because it's a driver for the SPC bit
after all and doing the following in the tree:

scc at 7fff0000 {
	compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
	reg = <0x7fff0000 0x1000>;
	interrupts = <0 95 4>;

	spc at b00 {
		compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
		reg = <0xb00 0x100>;
	};
};

This, I believe, would represent the actual situation, require no change
in the driver (except for the retirement of SPC_BASE which is good :-)
and allowed as, if and when necessary, to drive the SCC as a MFD/syscon
device.

Does it make some sense?

Pawel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 16:19   ` Pawel Moll
@ 2013-08-06 16:44     ` Olof Johansson
  2013-08-06 17:03       ` Pawel Moll
  2013-08-06 17:08       ` Lorenzo Pieralisi
  2013-08-06 16:55     ` Lorenzo Pieralisi
  1 sibling, 2 replies; 20+ messages in thread
From: Olof Johansson @ 2013-08-06 16:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 6, 2013 at 9:19 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> On Mon, 2013-08-05 at 10:47 +0100, Lorenzo Pieralisi wrote:
>> For the bindings, you are right, how about this (commit log written just
>> for the sake of it, it should be squashed in Nico's original series) ?
>
> So I finally sat down and did what I should have done long time ago...
> Read the spec ;-)

Uh... Ok. Biting my tongue. :)

Is the spec public, per chance?

> SCC is (was?) mainly used to set initial conditions for the cores,
> interconnect and all the other bits and pieces inside the test chip. It
> could be considered an equivalent of the classic "boot mode" pins, but
> as test chips have many more of them comparing to the normal SOCs, the
> data is being "injected" to the chip in a serial fashion *before* the
> main reset is being de-asserted. This is done by an external entity,
> namely the DCC (daughterboard configuration controller, so simply a
> microcontroller living next to the test chip). Why is this detail
> important at all? Because traditionally the SCC was *not* available in
> the normal memory map, otherwise it would be a perfect candidate for
> the . In order to change anything one had to go through the usual
> Versatile Express config infrastructure. Fortunately there was no need
> to do this at all...

Vexpress is such a hack. :) It's unfortunate that it ends up being the
lead platform on these kind of technologies, in some ways.

I think it makes sense to hide some of this ick in mach-vexpress, I
don't think we should ever aim to empty it out. Not based on how messy
the platform architecture is. I'd rather hide it in there than pollute
other parts of the kernel. Or at least, we should keep it as an option
in some cases such as these -- there might be other parts that do make
sense to move out to generic kernel.

> And here comes the V2P-CA15_A7, also known as TC2 ;-) where the
> interface was re-(or ab-?)used as a "convenient" communication channel
> between the test chip and the microcontroller. And the SPC is even
> described as "being merged" with the SCC. Uh...
>
> Now, the bottom line. How about keeping the driver look for
> "arm,vexpress-spc,v2p-ca15_a7" because it's a driver for the SPC bit
> after all and doing the following in the tree:
>
> scc at 7fff0000 {
>         compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
>         reg = <0x7fff0000 0x1000>;
>         interrupts = <0 95 4>;
>
>         spc at b00 {
>                 compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
>                 reg = <0xb00 0x100>;
>         };
> };
>
> This, I believe, would represent the actual situation, require no change
> in the driver (except for the retirement of SPC_BASE which is good :-)
> and allowed as, if and when necessary, to drive the SCC as a MFD/syscon
> device.
>
> Does it make some sense?

It does, but you need to setup the scc as a bus (with ranges, etc) for
that binding to work.

I pushed the fixed-up set of patches to the branch in arm-soc, so you
can find them there (olof/vexpress). Sounds like this isn't quite
ready to merge if these things still need to be worked out. So feel
free to take my patches, change them up as needed and send a fresh
copy.

Or, if you want to rebase the whole series and squash things in with
Nico's patches, that's ok too. But sounds like it needs a little more
work still. 3.12 is still a realistic target though.


-Olof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 16:19   ` Pawel Moll
  2013-08-06 16:44     ` Olof Johansson
@ 2013-08-06 16:55     ` Lorenzo Pieralisi
  2013-08-06 17:06       ` Pawel Moll
  1 sibling, 1 reply; 20+ messages in thread
From: Lorenzo Pieralisi @ 2013-08-06 16:55 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 06, 2013 at 05:19:53PM +0100, Pawel Moll wrote:
> On Mon, 2013-08-05 at 10:47 +0100, Lorenzo Pieralisi wrote:
> > For the bindings, you are right, how about this (commit log written just
> > for the sake of it, it should be squashed in Nico's original series) ?
> 
> So I finally sat down and did what I should have done long time ago...
> Read the spec ;-)
> 
> SCC is (was?) mainly used to set initial conditions for the cores,
> interconnect and all the other bits and pieces inside the test chip. It
> could be considered an equivalent of the classic "boot mode" pins, but
> as test chips have many more of them comparing to the normal SOCs, the
> data is being "injected" to the chip in a serial fashion *before* the
> main reset is being de-asserted. This is done by an external entity,
> namely the DCC (daughterboard configuration controller, so simply a
> microcontroller living next to the test chip). Why is this detail
> important at all? Because traditionally the SCC was *not* available in
> the normal memory map, otherwise it would be a perfect candidate for
> the . In order to change anything one had to go through the usual
> Versatile Express config infrastructure. Fortunately there was no need
> to do this at all...
> 
> And here comes the V2P-CA15_A7, also known as TC2 ;-) where the
> interface was re-(or ab-?)used as a "convenient" communication channel
> between the test chip and the microcontroller. And the SPC is even
> described as "being merged" with the SCC. Uh...

Yes, the "merging" of SCC and SPC is the root cause of this hopefully happy
ended story, they lumped together pieces of HW that should have been kept
separate.

> Now, the bottom line. How about keeping the driver look for
> "arm,vexpress-spc,v2p-ca15_a7" because it's a driver for the SPC bit
> after all and doing the following in the tree:
> 
> scc at 7fff0000 {
> 	compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
> 	reg = <0x7fff0000 0x1000>;
> 	interrupts = <0 95 4>;
> 
> 	spc at b00 {
> 		compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
> 		reg = <0xb00 0x100>;
> 	};
> };
> 
> This, I believe, would represent the actual situation, require no change
> in the driver (except for the retirement of SPC_BASE which is good :-)
> and allowed as, if and when necessary, to drive the SCC as a MFD/syscon
> device.

I do need some SCC registers to check the cluster ID against the MPIDR in
order to carry out powerdown operations. So if we do what you suggest we have
to change the driver, I would avoid doing that at this stage.

> Does it make some sense?

Well, it does but it complicates our lives for not much IMHO (and Olof
mentioned that in the cover letter). I still need to map the SCC as a
whole to read some IDs registers as I mentioned. We can find a way to make
SPC part of a MFD device later on.

Lorenzo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 16:44     ` Olof Johansson
@ 2013-08-06 17:03       ` Pawel Moll
  2013-08-06 17:25         ` Olof Johansson
  2013-08-06 17:08       ` Lorenzo Pieralisi
  1 sibling, 1 reply; 20+ messages in thread
From: Pawel Moll @ 2013-08-06 17:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2013-08-06 at 17:44 +0100, Olof Johansson wrote:
> On Tue, Aug 6, 2013 at 9:19 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> > On Mon, 2013-08-05 at 10:47 +0100, Lorenzo Pieralisi wrote:
> >> For the bindings, you are right, how about this (commit log written just
> >> for the sake of it, it should be squashed in Nico's original series) ?
> >
> > So I finally sat down and did what I should have done long time ago...
> > Read the spec ;-)
> 
> Uh... Ok. Biting my tongue. :)
> 
> Is the spec public, per chance?

Not the one I had to go to understand the implementation details, no.

> > SCC is (was?) mainly used to set initial conditions for the cores,
> > interconnect and all the other bits and pieces inside the test chip. It
> > could be considered an equivalent of the classic "boot mode" pins, but
> > as test chips have many more of them comparing to the normal SOCs, the
> > data is being "injected" to the chip in a serial fashion *before* the
> > main reset is being de-asserted. This is done by an external entity,
> > namely the DCC (daughterboard configuration controller, so simply a
> > microcontroller living next to the test chip). Why is this detail
> > important at all? Because traditionally the SCC was *not* available in
> > the normal memory map, otherwise it would be a perfect candidate for
> > the . In order to change anything one had to go through the usual
> > Versatile Express config infrastructure. Fortunately there was no need
> > to do this at all...
> 
> Vexpress is such a hack. :) 

I think I can say "thank you" on behalf of our board designers :-P :-)

Now, seriously speaking. It is a complex design, no question about that.
It has flaws, and I'm the first one to name the cock-ups (at least
internally). And some of the solutions may seem "over engineered" at
first. But the ultimate goal here was flexibility. You can change pretty
much everything here - most of the motherboard peripherals live in a
FPGA, we have more than one SMM (FPGA-based core implementation). And it
came at the cost.

> It's unfortunate that it ends up being the
> lead platform on these kind of technologies, in some ways.

You've just named the reason. It delivers technology before the
technology is mature. So I don't think it's fair to compare it with
final platforms.

> I think it makes sense to hide some of this ick in mach-vexpress, I
> don't think we should ever aim to empty it out.

I dare to disagree. And the reason is simple - arch/arm64. It's vexpress
as well, and we'll be in the same position. That's why I like the
"drivers/platform/vexpress" idea a lot. It's tucked away enough not to
offend usual by-passers while still being out of arch.

> Not based on how messy the platform architecture is. 

De gustibus non est disputandum. But anyway, it's my job to hide the
details behind as many of the generic interfaces as possible. There are
two main bones of contention now. The config infrastructure, which I am
working on (it will be hidden behind standard regmap API, so you won't
even notice it exists ;-). The other is the Power Management, which has
the unfortunate problem that we're not sure what are we getting into
yet.

> I'd rather hide it in there than pollute
> other parts of the kernel. Or at least, we should keep it as an option
> in some cases such as these -- there might be other parts that do make
> sense to move out to generic kernel.

And this is exactly what we're trying to do. Keep it all in one place.

> > And here comes the V2P-CA15_A7, also known as TC2 ;-) where the
> > interface was re-(or ab-?)used as a "convenient" communication channel
> > between the test chip and the microcontroller. And the SPC is even
> > described as "being merged" with the SCC. Uh...
> >
> > Now, the bottom line. How about keeping the driver look for
> > "arm,vexpress-spc,v2p-ca15_a7" because it's a driver for the SPC bit
> > after all and doing the following in the tree:
> >
> > scc at 7fff0000 {
> >         compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
> >         reg = <0x7fff0000 0x1000>;
> >         interrupts = <0 95 4>;
> >
> >         spc at b00 {
> >                 compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
> >                 reg = <0xb00 0x100>;
> >         };
> > };
> >
> > This, I believe, would represent the actual situation, require no change
> > in the driver (except for the retirement of SPC_BASE which is good :-)
> > and allowed as, if and when necessary, to drive the SCC as a MFD/syscon
> > device.
> >
> > Does it make some sense?
> 
> It does, but you need to setup the scc as a bus (with ranges, etc) for
> that binding to work.

Sure, it was just an example.

> I pushed the fixed-up set of patches to the branch in arm-soc, so you
> can find them there (olof/vexpress). Sounds like this isn't quite
> ready to merge if these things still need to be worked out. So feel
> free to take my patches, change them up as needed and send a fresh
> copy.

Ok, will do.

Pawe?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 16:55     ` Lorenzo Pieralisi
@ 2013-08-06 17:06       ` Pawel Moll
  0 siblings, 0 replies; 20+ messages in thread
From: Pawel Moll @ 2013-08-06 17:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2013-08-06 at 17:55 +0100, Lorenzo Pieralisi wrote:
> > And here comes the V2P-CA15_A7, also known as TC2 ;-) where the
> > interface was re-(or ab-?)used as a "convenient" communication channel
> > between the test chip and the microcontroller. And the SPC is even
> > described as "being merged" with the SCC. Uh...
> 
> Yes, the "merging" of SCC and SPC is the root cause of this hopefully happy
> ended story, they lumped together pieces of HW that should have been kept
> separate.

Show me an SOC when the silicon people didn't try to save silicon area
and pads, for the cost of "solutions" like this...

> > Now, the bottom line. How about keeping the driver look for
> > "arm,vexpress-spc,v2p-ca15_a7" because it's a driver for the SPC bit
> > after all and doing the following in the tree:
> > 
> > scc at 7fff0000 {
> > 	compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
> > 	reg = <0x7fff0000 0x1000>;
> > 	interrupts = <0 95 4>;
> > 
> > 	spc at b00 {
> > 		compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
> > 		reg = <0xb00 0x100>;
> > 	};
> > };
> > 
> > This, I believe, would represent the actual situation, require no change
> > in the driver (except for the retirement of SPC_BASE which is good :-)
> > and allowed as, if and when necessary, to drive the SCC as a MFD/syscon
> > device.
> 
> I do need some SCC registers to check the cluster ID against the MPIDR in
> order to carry out powerdown operations. So if we do what you suggest we have
> to change the driver, I would avoid doing that at this stage.

That's what I'm saying - if we do the binding right, you won't have to
change the driver at all. It will do the same 
	of_find_compatible_node(NULL, NULL, "arm,vexpress-spc,v2p-ca15_a7");
as previously.

Pawe?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 16:44     ` Olof Johansson
  2013-08-06 17:03       ` Pawel Moll
@ 2013-08-06 17:08       ` Lorenzo Pieralisi
  1 sibling, 0 replies; 20+ messages in thread
From: Lorenzo Pieralisi @ 2013-08-06 17:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 06, 2013 at 05:44:55PM +0100, Olof Johansson wrote:
> On Tue, Aug 6, 2013 at 9:19 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> > On Mon, 2013-08-05 at 10:47 +0100, Lorenzo Pieralisi wrote:
> >> For the bindings, you are right, how about this (commit log written just
> >> for the sake of it, it should be squashed in Nico's original series) ?
> >
> > So I finally sat down and did what I should have done long time ago...
> > Read the spec ;-)
> 
> Uh... Ok. Biting my tongue. :)
> 
> Is the spec public, per chance?

I will check.

> > SCC is (was?) mainly used to set initial conditions for the cores,
> > interconnect and all the other bits and pieces inside the test chip. It
> > could be considered an equivalent of the classic "boot mode" pins, but
> > as test chips have many more of them comparing to the normal SOCs, the
> > data is being "injected" to the chip in a serial fashion *before* the
> > main reset is being de-asserted. This is done by an external entity,
> > namely the DCC (daughterboard configuration controller, so simply a
> > microcontroller living next to the test chip). Why is this detail
> > important at all? Because traditionally the SCC was *not* available in
> > the normal memory map, otherwise it would be a perfect candidate for
> > the . In order to change anything one had to go through the usual
> > Versatile Express config infrastructure. Fortunately there was no need
> > to do this at all...
> 
> Vexpress is such a hack. :) It's unfortunate that it ends up being the
> lead platform on these kind of technologies, in some ways.
> 
> I think it makes sense to hide some of this ick in mach-vexpress, I
> don't think we should ever aim to empty it out. Not based on how messy
> the platform architecture is. I'd rather hide it in there than pollute
> other parts of the kernel. Or at least, we should keep it as an option
> in some cases such as these -- there might be other parts that do make
> sense to move out to generic kernel.
> 
> > And here comes the V2P-CA15_A7, also known as TC2 ;-) where the
> > interface was re-(or ab-?)used as a "convenient" communication channel
> > between the test chip and the microcontroller. And the SPC is even
> > described as "being merged" with the SCC. Uh...
> >
> > Now, the bottom line. How about keeping the driver look for
> > "arm,vexpress-spc,v2p-ca15_a7" because it's a driver for the SPC bit
> > after all and doing the following in the tree:
> >
> > scc at 7fff0000 {
> >         compatible = "arm,vexpress-scc,v2p-ca15_a7", "arm,vexpress-scc";
> >         reg = <0x7fff0000 0x1000>;
> >         interrupts = <0 95 4>;
> >
> >         spc at b00 {
> >                 compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
> >                 reg = <0xb00 0x100>;
> >         };
> > };
> >
> > This, I believe, would represent the actual situation, require no change
> > in the driver (except for the retirement of SPC_BASE which is good :-)
> > and allowed as, if and when necessary, to drive the SCC as a MFD/syscon
> > device.
> >
> > Does it make some sense?
> 
> It does, but you need to setup the scc as a bus (with ranges, etc) for
> that binding to work.
> 
> I pushed the fixed-up set of patches to the branch in arm-soc, so you
> can find them there (olof/vexpress). Sounds like this isn't quite
> ready to merge if these things still need to be worked out. So feel
> free to take my patches, change them up as needed and send a fresh
> copy.

No, please, no. By no means MCPM for TC2 should be delayed for longer by a
piece of HW that is giving me (and unfortunately you too) the time of my
life. We moved code to mach-vexpress, trimmed it to just a bunch of regs
reads and writes, and put it where it belongs, in platform specific code.

There is still room for making the SCC a proper MFD/syscon device, but not now,
it makes no sense to delay this code for something that we might not even
come to implement.

Let's get it through as it is, please.

Lorenzo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 17:03       ` Pawel Moll
@ 2013-08-06 17:25         ` Olof Johansson
  2013-08-06 17:32           ` Pawel Moll
  0 siblings, 1 reply; 20+ messages in thread
From: Olof Johansson @ 2013-08-06 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 6, 2013 at 10:03 AM, Pawel Moll <pawel.moll@arm.com> wrote:

> I dare to disagree. And the reason is simple - arch/arm64. It's vexpress
> as well, and we'll be in the same position.

Actually, no -- PSCI will be mandatory there so there will be no need
to have these kernel components.


-Olof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups
  2013-08-06 17:25         ` Olof Johansson
@ 2013-08-06 17:32           ` Pawel Moll
  0 siblings, 0 replies; 20+ messages in thread
From: Pawel Moll @ 2013-08-06 17:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2013-08-06 at 18:25 +0100, Olof Johansson wrote:
> > I dare to disagree. And the reason is simple - arch/arm64. It's vexpress
> > as well, and we'll be in the same position.
> 
> Actually, no -- PSCI will be mandatory there so there will be no need
> to have these kernel components.

Oh, SPC must die with TC2, no argument from me here. That's why I don't
really mind it being in arch/arm/mach-vexpress today. If it turns out
that it's the last and only thing there, we can discuss it again.

Pawe?

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-08-06 17:32 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-05  4:09 [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Olof Johansson
2013-08-05  4:09 ` [PATCH 1/3] ARM: vexpress: move spc driver back under mach-vexpress Olof Johansson
2013-08-05  9:52   ` Lorenzo Pieralisi
2013-08-05 23:26     ` Olof Johansson
2013-08-06  8:55       ` Lorenzo Pieralisi
2013-08-06 12:28         ` Pawel Moll
2013-08-05  4:09 ` [PATCH 2/3] ARM: vexpress: make spc code TC2-only Olof Johansson
2013-08-05  9:55   ` Lorenzo Pieralisi
2013-08-05 23:28     ` Olof Johansson
2013-08-05  4:09 ` [PATCH 3/3] ARM: vexpress: don't print virtual address in dmesg Olof Johansson
2013-08-05  9:47 ` [PATCH 0/3] ARM: vexpress: TC2 MCPM/SPC cleanups Lorenzo Pieralisi
2013-08-05 16:45   ` Olof Johansson
2013-08-06 16:19   ` Pawel Moll
2013-08-06 16:44     ` Olof Johansson
2013-08-06 17:03       ` Pawel Moll
2013-08-06 17:25         ` Olof Johansson
2013-08-06 17:32           ` Pawel Moll
2013-08-06 17:08       ` Lorenzo Pieralisi
2013-08-06 16:55     ` Lorenzo Pieralisi
2013-08-06 17:06       ` Pawel Moll

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).