[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

Hello,

version v5 of VExpress SPC driver, please read on the changelog for major
changes and explanations.

The probing scheme is unchanged, since after trying the early platform
devices approach it appeared that the end result was no better than the
current one. The only clean solution relies either on changing how
secondaries are brought up in the kernel (later than now) or enable
early platform device registration through DT. Please check this
thread for the related discussion:

https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html

The interface was adapted to regmap and again reverted to old driver for
the following reasons:

- Power down registers locking is hairy and requires arch spinlocks in
  the MCPM back end to work properly, normal spinlocks cannot be used
- Regmap adds unnecessary code to manage SPC since it is just a bunch of
  registers used to control power management flags, the overhead is just
  not worth it (talking about power down registers, not the vexpress config
  interface)
- The locking scheme behind regmap requires all registers in the map
  to be protected with the same lock, which is not exactly what we want
  here
- Given the reasons above, adding a regmap interface buys us nothing from
  a driver readability and maintainability perspective (again just talking
  about the power interface, a few registers) because for the SPC it would
  simply not be used

/drivers/mfd is probably not the right place for this code as it stands (but
probably will be when the entire driver, with DVFS and config interface, is
complete).

Thank you for the review in advance,
Lorenzo

This patch is v5 of a previous posting:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/177150.html

v5 changes:

- Completely removed vexpress-config interface waiting for refactoring
  based on regmap
- Removed frequency scaling interface and operating points programming
  retrieval
- Trimmed the driver code and DT bindings

v4 changes:
- Applied review comments (trimmed function names, added comments, refactored
  some APIs)
- Added comments throughout the set
- Fixed irq handler bug in checking the transaction status
- Improved commit log to explain early init synchro scheme
- Created a single static structure for variables dynamically allocated to
  remove usage of static
- Improved Kconfig entry

v3 changes:

- added __refdata to spc_check_loaded pointer
- removed some exported symbols
- added node pointer check in vexpress_spc_init()

v2 changes:

- Dropped timeout interface patch
- Converted interfaces to non-timeout ones, integrated and retested
- Removed mutex used at init
- Refactored code to work around init sections warning
- Fixed two minor bugs

Lorenzo Pieralisi (1):
  drivers: mfd: vexpress: add Serial Power Controller (SPC) support

 Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
 drivers/mfd/Kconfig                                    |  10 +
 drivers/mfd/Makefile                                   |   1 +
 drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
 include/linux/vexpress.h                               |  17 +
 5 files changed, 317 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/vexpress-spc.txt
 create mode 100644 drivers/mfd/vexpress-spc.c

-- 
1.8.2.2

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

Hello,

version v5 of VExpress SPC driver, please read on the changelog for major
changes and explanations.

The probing scheme is unchanged, since after trying the early platform
devices approach it appeared that the end result was no better than the
current one. The only clean solution relies either on changing how
secondaries are brought up in the kernel (later than now) or enable
early platform device registration through DT. Please check this
thread for the related discussion:

https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html

The interface was adapted to regmap and again reverted to old driver for
the following reasons:

- Power down registers locking is hairy and requires arch spinlocks in
  the MCPM back end to work properly, normal spinlocks cannot be used
- Regmap adds unnecessary code to manage SPC since it is just a bunch of
  registers used to control power management flags, the overhead is just
  not worth it (talking about power down registers, not the vexpress config
  interface)
- The locking scheme behind regmap requires all registers in the map
  to be protected with the same lock, which is not exactly what we want
  here
- Given the reasons above, adding a regmap interface buys us nothing from
  a driver readability and maintainability perspective (again just talking
  about the power interface, a few registers) because for the SPC it would
  simply not be used

/drivers/mfd is probably not the right place for this code as it stands (but
probably will be when the entire driver, with DVFS and config interface, is
complete).

Thank you for the review in advance,
Lorenzo

This patch is v5 of a previous posting:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/177150.html

v5 changes:

- Completely removed vexpress-config interface waiting for refactoring
  based on regmap
- Removed frequency scaling interface and operating points programming
  retrieval
- Trimmed the driver code and DT bindings

v4 changes:
- Applied review comments (trimmed function names, added comments, refactored
  some APIs)
- Added comments throughout the set
- Fixed irq handler bug in checking the transaction status
- Improved commit log to explain early init synchro scheme
- Created a single static structure for variables dynamically allocated to
  remove usage of static
- Improved Kconfig entry

v3 changes:

- added __refdata to spc_check_loaded pointer
- removed some exported symbols
- added node pointer check in vexpress_spc_init()

v2 changes:

- Dropped timeout interface patch
- Converted interfaces to non-timeout ones, integrated and retested
- Removed mutex used at init
- Refactored code to work around init sections warning
- Fixed two minor bugs

Lorenzo Pieralisi (1):
  drivers: mfd: vexpress: add Serial Power Controller (SPC) support

 Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
 drivers/mfd/Kconfig                                    |  10 +
 drivers/mfd/Makefile                                   |   1 +
 drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
 include/linux/vexpress.h                               |  17 +
 5 files changed, 317 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/vexpress-spc.txt
 create mode 100644 drivers/mfd/vexpress-spc.c

-- 
1.8.2.2

[RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Lorenzo Pieralisi]

The TC2 versatile express core tile integrates a logic block that provides the
interface between the dual cluster test-chip and the M3 microcontroller that
carries out power management. The logic block, called Serial Power Controller
(SPC), contains several memory mapped registers to control among other things
low-power states, wake-up irqs and per-CPU jump addresses registers.

This patch provides a driver that enables run-time control of features
implemented by the SPC power management control logic.

The SPC control logic is required to be programmed very early in the boot
process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
wake-up IRQs for power management. Hence, waiting for core changes to be
made in the device core code to enable early registration of platform
devices, the driver puts in place an early init scheme that allows kernel
drivers to initialize the SPC driver directly from the components requiring
it, if their initialization routine is called before the driver init
function by the boot process.

Device tree bindings documentation for the SPC component is provided with
the patchset.

Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Amit Kucheria <amit.kucheria@linaro.org>
Cc: Jon Medhurst <tixy@linaro.org>
Signed-off-by: Achin Gupta <achin.gupta@arm.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>
---
 Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
 drivers/mfd/Kconfig                                    |  10 +
 drivers/mfd/Makefile                                   |   1 +
 drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
 include/linux/vexpress.h                               |  17 +
 5 files changed, 317 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
new file mode 100644
index 0000000..1614725
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
@@ -0,0 +1,36 @@
+* 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@7fff0000 {
+	compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
+	reg = <0x7fff0000 0x1000>;
+	interrupts = <0 95 4>;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6959b8d..ebd23f4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1149,3 +1149,13 @@ config VEXPRESS_CONFIG
 	help
 	  Platform configuration infrastructure for the ARM Ltd.
 	  Versatile Express.
+
+config VEXPRESS_SPC
+	bool "Versatile Express SPC driver support"
+	depends on ARM
+	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 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/mfd/Makefile b/drivers/mfd/Makefile
index 718e94a..3a01203 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -153,5 +153,6 @@ obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
 obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o vexpress-sysreg.o
+obj-$(CONFIG_VEXPRESS_SPC)	+= vexpress-spc.o
 obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
 obj-$(CONFIG_MFD_AS3711)	+= as3711.o
diff --git a/drivers/mfd/vexpress-spc.c b/drivers/mfd/vexpress-spc.c
new file mode 100644
index 0000000..aa8c2a4
--- /dev/null
+++ b/drivers/mfd/vexpress-spc.c
@@ -0,0 +1,253 @@
+/*
+ * Versatile Express Serial Power Controller (SPC) support
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ *
+ * Authors: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
+ *          Achin Gupta           <achin.gupta@arm.com>
+ *          Lorenzo Pieralisi     <lorenzo.pieralisi@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/vexpress.h>
+
+#include <asm/cacheflush.h>
+
+#define SPCLOG "vexpress-spc: "
+
+/* SCC conf registers */
+#define A15_CONF		0x400
+#define SYS_INFO		0x700
+
+/* SPC registers base */
+#define SPC_BASE		0xB00
+
+/* SPC wake-up IRQs status and mask */
+#define WAKE_INT_MASK		(SPC_BASE + 0x24)
+#define WAKE_INT_RAW		(SPC_BASE + 0x28)
+#define WAKE_INT_STAT		(SPC_BASE + 0x2c)
+/* SPC power down registers */
+#define A15_PWRDN_EN		(SPC_BASE + 0x30)
+#define A7_PWRDN_EN		(SPC_BASE + 0x34)
+/* SPC per-CPU mailboxes */
+#define A15_BX_ADDR0		(SPC_BASE + 0x68)
+#define A7_BX_ADDR0		(SPC_BASE + 0x78)
+
+/* wake-up interrupt masks */
+#define GBL_WAKEUP_INT_MSK	(0x3 << 10)
+
+/* TC2 static dual-cluster configuration */
+#define MAX_CLUSTERS		2
+
+struct ve_spc_drvdata {
+	void __iomem *baseaddr;
+	/*
+	 * A15s cluster identifier
+	 * It corresponds to A15 processors MPIDR[15:8] bitfield
+	 */
+	u32 a15_clusid;
+};
+
+static struct ve_spc_drvdata *info;
+
+static inline bool cluster_is_a15(u32 cluster)
+{
+	return cluster == info->a15_clusid;
+}
+
+/**
+ * ve_spc_global_wakeup_irq()
+ *
+ * Function to set/clear global wakeup IRQs. Not protected by locking since
+ * it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @set: if true, global wake-up IRQs are set, if false they are cleared
+ */
+void ve_spc_global_wakeup_irq(bool set)
+{
+	u32 reg;
+
+	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
+
+	if (set)
+		reg |= GBL_WAKEUP_INT_MSK;
+	else
+		reg &= ~GBL_WAKEUP_INT_MSK;
+
+	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
+}
+
+/**
+ * ve_spc_cpu_wakeup_irq()
+ *
+ * Function to set/clear per-CPU wake-up IRQs. Not protected by locking since
+ * it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @cpu: mpidr[7:0] bitfield describing cpu affinity level
+ * @set: if true, wake-up IRQs are set, if false they are cleared
+ */
+void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set)
+{
+	u32 mask, reg;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	mask = 1 << cpu;
+
+	if (!cluster_is_a15(cluster))
+		mask <<= 4;
+
+	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
+
+	if (set)
+		reg |= mask;
+	else
+		reg &= ~mask;
+
+	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
+}
+
+/**
+ * ve_spc_set_resume_addr() - set the jump address used for warm boot
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @cpu: mpidr[7:0] bitfield describing cpu affinity level
+ * @addr: physical resume address
+ */
+void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
+{
+	void __iomem *baseaddr;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	if (cluster_is_a15(cluster))
+		baseaddr = info->baseaddr + A15_BX_ADDR0 + (cpu << 2);
+	else
+		baseaddr = info->baseaddr + A7_BX_ADDR0 + (cpu << 2);
+
+	writel_relaxed(addr, baseaddr);
+}
+
+/**
+ * ve_spc_get_nr_cpus() - get number of cpus in a cluster
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ *
+ * Return: > 0 number of cpus in the cluster
+ *         or 0 if cluster number invalid
+ */
+u32 ve_spc_get_nr_cpus(u32 cluster)
+{
+	u32 val;
+
+	if (cluster >= MAX_CLUSTERS)
+		return 0;
+
+	val = readl_relaxed(info->baseaddr + SYS_INFO);
+	val = cluster_is_a15(cluster) ? (val >> 16) : (val >> 20);
+	return val & 0xf;
+}
+
+/**
+ * ve_spc_powerdown()
+ *
+ * Function to enable/disable cluster powerdown. Not protected by locking
+ * since it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @enable: if true enables powerdown, if false disables it
+ */
+void ve_spc_powerdown(u32 cluster, bool enable)
+{
+	u32 pwdrn_reg;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
+	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);
+
+	if (!node)
+		return -ENODEV;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info) {
+		pr_err(SPCLOG "unable to allocate mem\n");
+		return -ENOMEM;
+	}
+
+	info->baseaddr = of_iomap(node, 0);
+	if (!info->baseaddr) {
+		pr_err(SPCLOG "unable to ioremap memory\n");
+		ret = -ENXIO;
+		goto mem_free;
+	}
+
+	info->a15_clusid = readl_relaxed(info->baseaddr + A15_CONF) & 0xf;
+
+	/*
+	 * Multi-cluster systems may need this data when non-coherent, during
+	 * cluster power-up/power-down. Make sure driver info reaches main
+	 * memory.
+	 */
+	sync_cache_w(info);
+	sync_cache_w(&info);
+	pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
+	return 0;
+
+mem_free:
+	kfree(info);
+	return ret;
+}
+
+/**
+ * ve_spc_init()
+ *
+ * Function exported to manage pre early_initcall initialization.
+ * SPC code is needed very early in the boot process to bring CPUs out of
+ * reset and initialize power management back-end so an init interface is
+ * provided to platform code to allow early initialization. The init
+ * interface can be removed as soon as the DT layer and platform bus allow
+ * platform device creation and probing before SMP boot.
+ */
+int __init ve_spc_init(void)
+{
+	static int ve_spc_init_status __initdata = -EAGAIN;
+
+	if (ve_spc_init_status == -EAGAIN)
+		ve_spc_init_status = ve_spc_probe();
+
+	return ve_spc_init_status;
+}
+early_initcall(ve_spc_init);
diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
index 50368e0..d0106ef 100644
--- a/include/linux/vexpress.h
+++ b/include/linux/vexpress.h
@@ -132,4 +132,21 @@ void vexpress_clk_of_register_spc(void);
 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.2.2

[RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Lorenzo Pieralisi]

The TC2 versatile express core tile integrates a logic block that provides the
interface between the dual cluster test-chip and the M3 microcontroller that
carries out power management. The logic block, called Serial Power Controller
(SPC), contains several memory mapped registers to control among other things
low-power states, wake-up irqs and per-CPU jump addresses registers.

This patch provides a driver that enables run-time control of features
implemented by the SPC power management control logic.

The SPC control logic is required to be programmed very early in the boot
process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
wake-up IRQs for power management. Hence, waiting for core changes to be
made in the device core code to enable early registration of platform
devices, the driver puts in place an early init scheme that allows kernel
drivers to initialize the SPC driver directly from the components requiring
it, if their initialization routine is called before the driver init
function by the boot process.

Device tree bindings documentation for the SPC component is provided with
the patchset.

Cc: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Amit Kucheria <amit.kucheria-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Jon Medhurst <tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Achin Gupta <achin.gupta-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha-5wv7dgnIgG8@public.gmane.org>
---
 Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
 drivers/mfd/Kconfig                                    |  10 +
 drivers/mfd/Makefile                                   |   1 +
 drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
 include/linux/vexpress.h                               |  17 +
 5 files changed, 317 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
new file mode 100644
index 0000000..1614725
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
@@ -0,0 +1,36 @@
+* 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@7fff0000 {
+	compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
+	reg = <0x7fff0000 0x1000>;
+	interrupts = <0 95 4>;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6959b8d..ebd23f4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1149,3 +1149,13 @@ config VEXPRESS_CONFIG
 	help
 	  Platform configuration infrastructure for the ARM Ltd.
 	  Versatile Express.
+
+config VEXPRESS_SPC
+	bool "Versatile Express SPC driver support"
+	depends on ARM
+	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 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/mfd/Makefile b/drivers/mfd/Makefile
index 718e94a..3a01203 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -153,5 +153,6 @@ obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
 obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o vexpress-sysreg.o
+obj-$(CONFIG_VEXPRESS_SPC)	+= vexpress-spc.o
 obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
 obj-$(CONFIG_MFD_AS3711)	+= as3711.o
diff --git a/drivers/mfd/vexpress-spc.c b/drivers/mfd/vexpress-spc.c
new file mode 100644
index 0000000..aa8c2a4
--- /dev/null
+++ b/drivers/mfd/vexpress-spc.c
@@ -0,0 +1,253 @@
+/*
+ * Versatile Express Serial Power Controller (SPC) support
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ *
+ * Authors: Sudeep KarkadaNagesha <sudeep.karkadanagesha-5wv7dgnIgG8@public.gmane.org>
+ *          Achin Gupta           <achin.gupta-5wv7dgnIgG8@public.gmane.org>
+ *          Lorenzo Pieralisi     <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/vexpress.h>
+
+#include <asm/cacheflush.h>
+
+#define SPCLOG "vexpress-spc: "
+
+/* SCC conf registers */
+#define A15_CONF		0x400
+#define SYS_INFO		0x700
+
+/* SPC registers base */
+#define SPC_BASE		0xB00
+
+/* SPC wake-up IRQs status and mask */
+#define WAKE_INT_MASK		(SPC_BASE + 0x24)
+#define WAKE_INT_RAW		(SPC_BASE + 0x28)
+#define WAKE_INT_STAT		(SPC_BASE + 0x2c)
+/* SPC power down registers */
+#define A15_PWRDN_EN		(SPC_BASE + 0x30)
+#define A7_PWRDN_EN		(SPC_BASE + 0x34)
+/* SPC per-CPU mailboxes */
+#define A15_BX_ADDR0		(SPC_BASE + 0x68)
+#define A7_BX_ADDR0		(SPC_BASE + 0x78)
+
+/* wake-up interrupt masks */
+#define GBL_WAKEUP_INT_MSK	(0x3 << 10)
+
+/* TC2 static dual-cluster configuration */
+#define MAX_CLUSTERS		2
+
+struct ve_spc_drvdata {
+	void __iomem *baseaddr;
+	/*
+	 * A15s cluster identifier
+	 * It corresponds to A15 processors MPIDR[15:8] bitfield
+	 */
+	u32 a15_clusid;
+};
+
+static struct ve_spc_drvdata *info;
+
+static inline bool cluster_is_a15(u32 cluster)
+{
+	return cluster == info->a15_clusid;
+}
+
+/**
+ * ve_spc_global_wakeup_irq()
+ *
+ * Function to set/clear global wakeup IRQs. Not protected by locking since
+ * it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @set: if true, global wake-up IRQs are set, if false they are cleared
+ */
+void ve_spc_global_wakeup_irq(bool set)
+{
+	u32 reg;
+
+	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
+
+	if (set)
+		reg |= GBL_WAKEUP_INT_MSK;
+	else
+		reg &= ~GBL_WAKEUP_INT_MSK;
+
+	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
+}
+
+/**
+ * ve_spc_cpu_wakeup_irq()
+ *
+ * Function to set/clear per-CPU wake-up IRQs. Not protected by locking since
+ * it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @cpu: mpidr[7:0] bitfield describing cpu affinity level
+ * @set: if true, wake-up IRQs are set, if false they are cleared
+ */
+void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set)
+{
+	u32 mask, reg;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	mask = 1 << cpu;
+
+	if (!cluster_is_a15(cluster))
+		mask <<= 4;
+
+	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
+
+	if (set)
+		reg |= mask;
+	else
+		reg &= ~mask;
+
+	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
+}
+
+/**
+ * ve_spc_set_resume_addr() - set the jump address used for warm boot
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @cpu: mpidr[7:0] bitfield describing cpu affinity level
+ * @addr: physical resume address
+ */
+void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
+{
+	void __iomem *baseaddr;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	if (cluster_is_a15(cluster))
+		baseaddr = info->baseaddr + A15_BX_ADDR0 + (cpu << 2);
+	else
+		baseaddr = info->baseaddr + A7_BX_ADDR0 + (cpu << 2);
+
+	writel_relaxed(addr, baseaddr);
+}
+
+/**
+ * ve_spc_get_nr_cpus() - get number of cpus in a cluster
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ *
+ * Return: > 0 number of cpus in the cluster
+ *         or 0 if cluster number invalid
+ */
+u32 ve_spc_get_nr_cpus(u32 cluster)
+{
+	u32 val;
+
+	if (cluster >= MAX_CLUSTERS)
+		return 0;
+
+	val = readl_relaxed(info->baseaddr + SYS_INFO);
+	val = cluster_is_a15(cluster) ? (val >> 16) : (val >> 20);
+	return val & 0xf;
+}
+
+/**
+ * ve_spc_powerdown()
+ *
+ * Function to enable/disable cluster powerdown. Not protected by locking
+ * since it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @enable: if true enables powerdown, if false disables it
+ */
+void ve_spc_powerdown(u32 cluster, bool enable)
+{
+	u32 pwdrn_reg;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
+	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);
+
+	if (!node)
+		return -ENODEV;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info) {
+		pr_err(SPCLOG "unable to allocate mem\n");
+		return -ENOMEM;
+	}
+
+	info->baseaddr = of_iomap(node, 0);
+	if (!info->baseaddr) {
+		pr_err(SPCLOG "unable to ioremap memory\n");
+		ret = -ENXIO;
+		goto mem_free;
+	}
+
+	info->a15_clusid = readl_relaxed(info->baseaddr + A15_CONF) & 0xf;
+
+	/*
+	 * Multi-cluster systems may need this data when non-coherent, during
+	 * cluster power-up/power-down. Make sure driver info reaches main
+	 * memory.
+	 */
+	sync_cache_w(info);
+	sync_cache_w(&info);
+	pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
+	return 0;
+
+mem_free:
+	kfree(info);
+	return ret;
+}
+
+/**
+ * ve_spc_init()
+ *
+ * Function exported to manage pre early_initcall initialization.
+ * SPC code is needed very early in the boot process to bring CPUs out of
+ * reset and initialize power management back-end so an init interface is
+ * provided to platform code to allow early initialization. The init
+ * interface can be removed as soon as the DT layer and platform bus allow
+ * platform device creation and probing before SMP boot.
+ */
+int __init ve_spc_init(void)
+{
+	static int ve_spc_init_status __initdata = -EAGAIN;
+
+	if (ve_spc_init_status == -EAGAIN)
+		ve_spc_init_status = ve_spc_probe();
+
+	return ve_spc_init_status;
+}
+early_initcall(ve_spc_init);
diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
index 50368e0..d0106ef 100644
--- a/include/linux/vexpress.h
+++ b/include/linux/vexpress.h
@@ -132,4 +132,21 @@ void vexpress_clk_of_register_spc(void);
 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.2.2

[RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Lorenzo Pieralisi]

The TC2 versatile express core tile integrates a logic block that provides the
interface between the dual cluster test-chip and the M3 microcontroller that
carries out power management. The logic block, called Serial Power Controller
(SPC), contains several memory mapped registers to control among other things
low-power states, wake-up irqs and per-CPU jump addresses registers.

This patch provides a driver that enables run-time control of features
implemented by the SPC power management control logic.

The SPC control logic is required to be programmed very early in the boot
process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
wake-up IRQs for power management. Hence, waiting for core changes to be
made in the device core code to enable early registration of platform
devices, the driver puts in place an early init scheme that allows kernel
drivers to initialize the SPC driver directly from the components requiring
it, if their initialization routine is called before the driver init
function by the boot process.

Device tree bindings documentation for the SPC component is provided with
the patchset.

Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Amit Kucheria <amit.kucheria@linaro.org>
Cc: Jon Medhurst <tixy@linaro.org>
Signed-off-by: Achin Gupta <achin.gupta@arm.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>
---
 Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
 drivers/mfd/Kconfig                                    |  10 +
 drivers/mfd/Makefile                                   |   1 +
 drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
 include/linux/vexpress.h                               |  17 +
 5 files changed, 317 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
new file mode 100644
index 0000000..1614725
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
@@ -0,0 +1,36 @@
+* 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>;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6959b8d..ebd23f4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1149,3 +1149,13 @@ config VEXPRESS_CONFIG
 	help
 	  Platform configuration infrastructure for the ARM Ltd.
 	  Versatile Express.
+
+config VEXPRESS_SPC
+	bool "Versatile Express SPC driver support"
+	depends on ARM
+	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 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/mfd/Makefile b/drivers/mfd/Makefile
index 718e94a..3a01203 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -153,5 +153,6 @@ obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
 obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o vexpress-sysreg.o
+obj-$(CONFIG_VEXPRESS_SPC)	+= vexpress-spc.o
 obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
 obj-$(CONFIG_MFD_AS3711)	+= as3711.o
diff --git a/drivers/mfd/vexpress-spc.c b/drivers/mfd/vexpress-spc.c
new file mode 100644
index 0000000..aa8c2a4
--- /dev/null
+++ b/drivers/mfd/vexpress-spc.c
@@ -0,0 +1,253 @@
+/*
+ * Versatile Express Serial Power Controller (SPC) support
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ *
+ * Authors: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
+ *          Achin Gupta           <achin.gupta@arm.com>
+ *          Lorenzo Pieralisi     <lorenzo.pieralisi@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/vexpress.h>
+
+#include <asm/cacheflush.h>
+
+#define SPCLOG "vexpress-spc: "
+
+/* SCC conf registers */
+#define A15_CONF		0x400
+#define SYS_INFO		0x700
+
+/* SPC registers base */
+#define SPC_BASE		0xB00
+
+/* SPC wake-up IRQs status and mask */
+#define WAKE_INT_MASK		(SPC_BASE + 0x24)
+#define WAKE_INT_RAW		(SPC_BASE + 0x28)
+#define WAKE_INT_STAT		(SPC_BASE + 0x2c)
+/* SPC power down registers */
+#define A15_PWRDN_EN		(SPC_BASE + 0x30)
+#define A7_PWRDN_EN		(SPC_BASE + 0x34)
+/* SPC per-CPU mailboxes */
+#define A15_BX_ADDR0		(SPC_BASE + 0x68)
+#define A7_BX_ADDR0		(SPC_BASE + 0x78)
+
+/* wake-up interrupt masks */
+#define GBL_WAKEUP_INT_MSK	(0x3 << 10)
+
+/* TC2 static dual-cluster configuration */
+#define MAX_CLUSTERS		2
+
+struct ve_spc_drvdata {
+	void __iomem *baseaddr;
+	/*
+	 * A15s cluster identifier
+	 * It corresponds to A15 processors MPIDR[15:8] bitfield
+	 */
+	u32 a15_clusid;
+};
+
+static struct ve_spc_drvdata *info;
+
+static inline bool cluster_is_a15(u32 cluster)
+{
+	return cluster == info->a15_clusid;
+}
+
+/**
+ * ve_spc_global_wakeup_irq()
+ *
+ * Function to set/clear global wakeup IRQs. Not protected by locking since
+ * it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @set: if true, global wake-up IRQs are set, if false they are cleared
+ */
+void ve_spc_global_wakeup_irq(bool set)
+{
+	u32 reg;
+
+	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
+
+	if (set)
+		reg |= GBL_WAKEUP_INT_MSK;
+	else
+		reg &= ~GBL_WAKEUP_INT_MSK;
+
+	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
+}
+
+/**
+ * ve_spc_cpu_wakeup_irq()
+ *
+ * Function to set/clear per-CPU wake-up IRQs. Not protected by locking since
+ * it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @cpu: mpidr[7:0] bitfield describing cpu affinity level
+ * @set: if true, wake-up IRQs are set, if false they are cleared
+ */
+void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set)
+{
+	u32 mask, reg;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	mask = 1 << cpu;
+
+	if (!cluster_is_a15(cluster))
+		mask <<= 4;
+
+	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
+
+	if (set)
+		reg |= mask;
+	else
+		reg &= ~mask;
+
+	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
+}
+
+/**
+ * ve_spc_set_resume_addr() - set the jump address used for warm boot
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @cpu: mpidr[7:0] bitfield describing cpu affinity level
+ * @addr: physical resume address
+ */
+void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
+{
+	void __iomem *baseaddr;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	if (cluster_is_a15(cluster))
+		baseaddr = info->baseaddr + A15_BX_ADDR0 + (cpu << 2);
+	else
+		baseaddr = info->baseaddr + A7_BX_ADDR0 + (cpu << 2);
+
+	writel_relaxed(addr, baseaddr);
+}
+
+/**
+ * ve_spc_get_nr_cpus() - get number of cpus in a cluster
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ *
+ * Return: > 0 number of cpus in the cluster
+ *         or 0 if cluster number invalid
+ */
+u32 ve_spc_get_nr_cpus(u32 cluster)
+{
+	u32 val;
+
+	if (cluster >= MAX_CLUSTERS)
+		return 0;
+
+	val = readl_relaxed(info->baseaddr + SYS_INFO);
+	val = cluster_is_a15(cluster) ? (val >> 16) : (val >> 20);
+	return val & 0xf;
+}
+
+/**
+ * ve_spc_powerdown()
+ *
+ * Function to enable/disable cluster powerdown. Not protected by locking
+ * since it might be used in code paths where normal cacheable locks are not
+ * working. Locking must be provided by the caller to ensure atomicity.
+ *
+ * @cluster: mpidr[15:8] bitfield describing cluster affinity level
+ * @enable: if true enables powerdown, if false disables it
+ */
+void ve_spc_powerdown(u32 cluster, bool enable)
+{
+	u32 pwdrn_reg;
+
+	if (cluster >= MAX_CLUSTERS)
+		return;
+
+	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
+	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);
+
+	if (!node)
+		return -ENODEV;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info) {
+		pr_err(SPCLOG "unable to allocate mem\n");
+		return -ENOMEM;
+	}
+
+	info->baseaddr = of_iomap(node, 0);
+	if (!info->baseaddr) {
+		pr_err(SPCLOG "unable to ioremap memory\n");
+		ret = -ENXIO;
+		goto mem_free;
+	}
+
+	info->a15_clusid = readl_relaxed(info->baseaddr + A15_CONF) & 0xf;
+
+	/*
+	 * Multi-cluster systems may need this data when non-coherent, during
+	 * cluster power-up/power-down. Make sure driver info reaches main
+	 * memory.
+	 */
+	sync_cache_w(info);
+	sync_cache_w(&info);
+	pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
+	return 0;
+
+mem_free:
+	kfree(info);
+	return ret;
+}
+
+/**
+ * ve_spc_init()
+ *
+ * Function exported to manage pre early_initcall initialization.
+ * SPC code is needed very early in the boot process to bring CPUs out of
+ * reset and initialize power management back-end so an init interface is
+ * provided to platform code to allow early initialization. The init
+ * interface can be removed as soon as the DT layer and platform bus allow
+ * platform device creation and probing before SMP boot.
+ */
+int __init ve_spc_init(void)
+{
+	static int ve_spc_init_status __initdata = -EAGAIN;
+
+	if (ve_spc_init_status == -EAGAIN)
+		ve_spc_init_status = ve_spc_probe();
+
+	return ve_spc_init_status;
+}
+early_initcall(ve_spc_init);
diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
index 50368e0..d0106ef 100644
--- a/include/linux/vexpress.h
+++ b/include/linux/vexpress.h
@@ -132,4 +132,21 @@ void vexpress_clk_of_register_spc(void);
 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.2.2

Re: [RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Rob Herring]

On 07/16/2013 11:05 AM, Lorenzo Pieralisi wrote:
> The TC2 versatile express core tile integrates a logic block that provides the
> interface between the dual cluster test-chip and the M3 microcontroller that
> carries out power management. The logic block, called Serial Power Controller
> (SPC), contains several memory mapped registers to control among other things
> low-power states, wake-up irqs and per-CPU jump addresses registers.
> 
> This patch provides a driver that enables run-time control of features
> implemented by the SPC power management control logic.
> 
> The SPC control logic is required to be programmed very early in the boot
> process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
> wake-up IRQs for power management. Hence, waiting for core changes to be
> made in the device core code to enable early registration of platform
> devices, the driver puts in place an early init scheme that allows kernel
> drivers to initialize the SPC driver directly from the components requiring
> it, if their initialization routine is called before the driver init
> function by the boot process.
> 
> Device tree bindings documentation for the SPC component is provided with
> the patchset.

Just curious, wouldn't a TC2 PSCI implementation eliminate the need for
most/all of this code?

Rob

> 
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Amit Kucheria <amit.kucheria@linaro.org>
> Cc: Jon Medhurst <tixy@linaro.org>
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>
> ---
>  Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
>  drivers/mfd/Kconfig                                    |  10 +
>  drivers/mfd/Makefile                                   |   1 +
>  drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
>  include/linux/vexpress.h                               |  17 +
>  5 files changed, 317 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
> new file mode 100644
> index 0000000..1614725
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
> @@ -0,0 +1,36 @@
> +* 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@7fff0000 {
> +	compatible = "arm,vexpress-spc,v2p-ca15_a7", "arm,vexpress-spc";
> +	reg = <0x7fff0000 0x1000>;
> +	interrupts = <0 95 4>;
> +};
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 6959b8d..ebd23f4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1149,3 +1149,13 @@ config VEXPRESS_CONFIG
>  	help
>  	  Platform configuration infrastructure for the ARM Ltd.
>  	  Versatile Express.
> +
> +config VEXPRESS_SPC
> +	bool "Versatile Express SPC driver support"
> +	depends on ARM
> +	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 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/mfd/Makefile b/drivers/mfd/Makefile
> index 718e94a..3a01203 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -153,5 +153,6 @@ obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
>  obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
>  obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
>  obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o vexpress-sysreg.o
> +obj-$(CONFIG_VEXPRESS_SPC)	+= vexpress-spc.o
>  obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
>  obj-$(CONFIG_MFD_AS3711)	+= as3711.o
> diff --git a/drivers/mfd/vexpress-spc.c b/drivers/mfd/vexpress-spc.c
> new file mode 100644
> index 0000000..aa8c2a4
> --- /dev/null
> +++ b/drivers/mfd/vexpress-spc.c
> @@ -0,0 +1,253 @@
> +/*
> + * Versatile Express Serial Power Controller (SPC) support
> + *
> + * Copyright (C) 2013 ARM Ltd.
> + *
> + * Authors: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> + *          Achin Gupta           <achin.gupta@arm.com>
> + *          Lorenzo Pieralisi     <lorenzo.pieralisi@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/vexpress.h>
> +
> +#include <asm/cacheflush.h>
> +
> +#define SPCLOG "vexpress-spc: "
> +
> +/* SCC conf registers */
> +#define A15_CONF		0x400
> +#define SYS_INFO		0x700
> +
> +/* SPC registers base */
> +#define SPC_BASE		0xB00
> +
> +/* SPC wake-up IRQs status and mask */
> +#define WAKE_INT_MASK		(SPC_BASE + 0x24)
> +#define WAKE_INT_RAW		(SPC_BASE + 0x28)
> +#define WAKE_INT_STAT		(SPC_BASE + 0x2c)
> +/* SPC power down registers */
> +#define A15_PWRDN_EN		(SPC_BASE + 0x30)
> +#define A7_PWRDN_EN		(SPC_BASE + 0x34)
> +/* SPC per-CPU mailboxes */
> +#define A15_BX_ADDR0		(SPC_BASE + 0x68)
> +#define A7_BX_ADDR0		(SPC_BASE + 0x78)
> +
> +/* wake-up interrupt masks */
> +#define GBL_WAKEUP_INT_MSK	(0x3 << 10)
> +
> +/* TC2 static dual-cluster configuration */
> +#define MAX_CLUSTERS		2
> +
> +struct ve_spc_drvdata {
> +	void __iomem *baseaddr;
> +	/*
> +	 * A15s cluster identifier
> +	 * It corresponds to A15 processors MPIDR[15:8] bitfield
> +	 */
> +	u32 a15_clusid;
> +};
> +
> +static struct ve_spc_drvdata *info;
> +
> +static inline bool cluster_is_a15(u32 cluster)
> +{
> +	return cluster == info->a15_clusid;
> +}
> +
> +/**
> + * ve_spc_global_wakeup_irq()
> + *
> + * Function to set/clear global wakeup IRQs. Not protected by locking since
> + * it might be used in code paths where normal cacheable locks are not
> + * working. Locking must be provided by the caller to ensure atomicity.
> + *
> + * @set: if true, global wake-up IRQs are set, if false they are cleared
> + */
> +void ve_spc_global_wakeup_irq(bool set)
> +{
> +	u32 reg;
> +
> +	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
> +
> +	if (set)
> +		reg |= GBL_WAKEUP_INT_MSK;
> +	else
> +		reg &= ~GBL_WAKEUP_INT_MSK;
> +
> +	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
> +}
> +
> +/**
> + * ve_spc_cpu_wakeup_irq()
> + *
> + * Function to set/clear per-CPU wake-up IRQs. Not protected by locking since
> + * it might be used in code paths where normal cacheable locks are not
> + * working. Locking must be provided by the caller to ensure atomicity.
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + * @cpu: mpidr[7:0] bitfield describing cpu affinity level
> + * @set: if true, wake-up IRQs are set, if false they are cleared
> + */
> +void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set)
> +{
> +	u32 mask, reg;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return;
> +
> +	mask = 1 << cpu;
> +
> +	if (!cluster_is_a15(cluster))
> +		mask <<= 4;
> +
> +	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
> +
> +	if (set)
> +		reg |= mask;
> +	else
> +		reg &= ~mask;
> +
> +	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
> +}
> +
> +/**
> + * ve_spc_set_resume_addr() - set the jump address used for warm boot
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + * @cpu: mpidr[7:0] bitfield describing cpu affinity level
> + * @addr: physical resume address
> + */
> +void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
> +{
> +	void __iomem *baseaddr;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return;
> +
> +	if (cluster_is_a15(cluster))
> +		baseaddr = info->baseaddr + A15_BX_ADDR0 + (cpu << 2);
> +	else
> +		baseaddr = info->baseaddr + A7_BX_ADDR0 + (cpu << 2);
> +
> +	writel_relaxed(addr, baseaddr);
> +}
> +
> +/**
> + * ve_spc_get_nr_cpus() - get number of cpus in a cluster
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + *
> + * Return: > 0 number of cpus in the cluster
> + *         or 0 if cluster number invalid
> + */
> +u32 ve_spc_get_nr_cpus(u32 cluster)
> +{
> +	u32 val;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return 0;
> +
> +	val = readl_relaxed(info->baseaddr + SYS_INFO);
> +	val = cluster_is_a15(cluster) ? (val >> 16) : (val >> 20);
> +	return val & 0xf;
> +}
> +
> +/**
> + * ve_spc_powerdown()
> + *
> + * Function to enable/disable cluster powerdown. Not protected by locking
> + * since it might be used in code paths where normal cacheable locks are not
> + * working. Locking must be provided by the caller to ensure atomicity.
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + * @enable: if true enables powerdown, if false disables it
> + */
> +void ve_spc_powerdown(u32 cluster, bool enable)
> +{
> +	u32 pwdrn_reg;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return;
> +
> +	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
> +	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);
> +
> +	if (!node)
> +		return -ENODEV;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info) {
> +		pr_err(SPCLOG "unable to allocate mem\n");
> +		return -ENOMEM;
> +	}
> +
> +	info->baseaddr = of_iomap(node, 0);
> +	if (!info->baseaddr) {
> +		pr_err(SPCLOG "unable to ioremap memory\n");
> +		ret = -ENXIO;
> +		goto mem_free;
> +	}
> +
> +	info->a15_clusid = readl_relaxed(info->baseaddr + A15_CONF) & 0xf;
> +
> +	/*
> +	 * Multi-cluster systems may need this data when non-coherent, during
> +	 * cluster power-up/power-down. Make sure driver info reaches main
> +	 * memory.
> +	 */
> +	sync_cache_w(info);
> +	sync_cache_w(&info);
> +	pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
> +	return 0;
> +
> +mem_free:
> +	kfree(info);
> +	return ret;
> +}
> +
> +/**
> + * ve_spc_init()
> + *
> + * Function exported to manage pre early_initcall initialization.
> + * SPC code is needed very early in the boot process to bring CPUs out of
> + * reset and initialize power management back-end so an init interface is
> + * provided to platform code to allow early initialization. The init
> + * interface can be removed as soon as the DT layer and platform bus allow
> + * platform device creation and probing before SMP boot.
> + */
> +int __init ve_spc_init(void)
> +{
> +	static int ve_spc_init_status __initdata = -EAGAIN;
> +
> +	if (ve_spc_init_status == -EAGAIN)
> +		ve_spc_init_status = ve_spc_probe();
> +
> +	return ve_spc_init_status;
> +}
> +early_initcall(ve_spc_init);
> diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
> index 50368e0..d0106ef 100644
> --- a/include/linux/vexpress.h
> +++ b/include/linux/vexpress.h
> @@ -132,4 +132,21 @@ void vexpress_clk_of_register_spc(void);
>  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
> 

[RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Rob Herring]

On 07/16/2013 11:05 AM, Lorenzo Pieralisi wrote:
> The TC2 versatile express core tile integrates a logic block that provides the
> interface between the dual cluster test-chip and the M3 microcontroller that
> carries out power management. The logic block, called Serial Power Controller
> (SPC), contains several memory mapped registers to control among other things
> low-power states, wake-up irqs and per-CPU jump addresses registers.
> 
> This patch provides a driver that enables run-time control of features
> implemented by the SPC power management control logic.
> 
> The SPC control logic is required to be programmed very early in the boot
> process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
> wake-up IRQs for power management. Hence, waiting for core changes to be
> made in the device core code to enable early registration of platform
> devices, the driver puts in place an early init scheme that allows kernel
> drivers to initialize the SPC driver directly from the components requiring
> it, if their initialization routine is called before the driver init
> function by the boot process.
> 
> Device tree bindings documentation for the SPC component is provided with
> the patchset.

Just curious, wouldn't a TC2 PSCI implementation eliminate the need for
most/all of this code?

Rob

> 
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Amit Kucheria <amit.kucheria@linaro.org>
> Cc: Jon Medhurst <tixy@linaro.org>
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>
> ---
>  Documentation/devicetree/bindings/mfd/vexpress-spc.txt |  36 ++
>  drivers/mfd/Kconfig                                    |  10 +
>  drivers/mfd/Makefile                                   |   1 +
>  drivers/mfd/vexpress-spc.c                             | 253 ++++++++++
>  include/linux/vexpress.h                               |  17 +
>  5 files changed, 317 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
> new file mode 100644
> index 0000000..1614725
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
> @@ -0,0 +1,36 @@
> +* 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>;
> +};
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 6959b8d..ebd23f4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1149,3 +1149,13 @@ config VEXPRESS_CONFIG
>  	help
>  	  Platform configuration infrastructure for the ARM Ltd.
>  	  Versatile Express.
> +
> +config VEXPRESS_SPC
> +	bool "Versatile Express SPC driver support"
> +	depends on ARM
> +	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 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/mfd/Makefile b/drivers/mfd/Makefile
> index 718e94a..3a01203 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -153,5 +153,6 @@ obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
>  obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
>  obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
>  obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o vexpress-sysreg.o
> +obj-$(CONFIG_VEXPRESS_SPC)	+= vexpress-spc.o
>  obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
>  obj-$(CONFIG_MFD_AS3711)	+= as3711.o
> diff --git a/drivers/mfd/vexpress-spc.c b/drivers/mfd/vexpress-spc.c
> new file mode 100644
> index 0000000..aa8c2a4
> --- /dev/null
> +++ b/drivers/mfd/vexpress-spc.c
> @@ -0,0 +1,253 @@
> +/*
> + * Versatile Express Serial Power Controller (SPC) support
> + *
> + * Copyright (C) 2013 ARM Ltd.
> + *
> + * Authors: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> + *          Achin Gupta           <achin.gupta@arm.com>
> + *          Lorenzo Pieralisi     <lorenzo.pieralisi@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/vexpress.h>
> +
> +#include <asm/cacheflush.h>
> +
> +#define SPCLOG "vexpress-spc: "
> +
> +/* SCC conf registers */
> +#define A15_CONF		0x400
> +#define SYS_INFO		0x700
> +
> +/* SPC registers base */
> +#define SPC_BASE		0xB00
> +
> +/* SPC wake-up IRQs status and mask */
> +#define WAKE_INT_MASK		(SPC_BASE + 0x24)
> +#define WAKE_INT_RAW		(SPC_BASE + 0x28)
> +#define WAKE_INT_STAT		(SPC_BASE + 0x2c)
> +/* SPC power down registers */
> +#define A15_PWRDN_EN		(SPC_BASE + 0x30)
> +#define A7_PWRDN_EN		(SPC_BASE + 0x34)
> +/* SPC per-CPU mailboxes */
> +#define A15_BX_ADDR0		(SPC_BASE + 0x68)
> +#define A7_BX_ADDR0		(SPC_BASE + 0x78)
> +
> +/* wake-up interrupt masks */
> +#define GBL_WAKEUP_INT_MSK	(0x3 << 10)
> +
> +/* TC2 static dual-cluster configuration */
> +#define MAX_CLUSTERS		2
> +
> +struct ve_spc_drvdata {
> +	void __iomem *baseaddr;
> +	/*
> +	 * A15s cluster identifier
> +	 * It corresponds to A15 processors MPIDR[15:8] bitfield
> +	 */
> +	u32 a15_clusid;
> +};
> +
> +static struct ve_spc_drvdata *info;
> +
> +static inline bool cluster_is_a15(u32 cluster)
> +{
> +	return cluster == info->a15_clusid;
> +}
> +
> +/**
> + * ve_spc_global_wakeup_irq()
> + *
> + * Function to set/clear global wakeup IRQs. Not protected by locking since
> + * it might be used in code paths where normal cacheable locks are not
> + * working. Locking must be provided by the caller to ensure atomicity.
> + *
> + * @set: if true, global wake-up IRQs are set, if false they are cleared
> + */
> +void ve_spc_global_wakeup_irq(bool set)
> +{
> +	u32 reg;
> +
> +	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
> +
> +	if (set)
> +		reg |= GBL_WAKEUP_INT_MSK;
> +	else
> +		reg &= ~GBL_WAKEUP_INT_MSK;
> +
> +	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
> +}
> +
> +/**
> + * ve_spc_cpu_wakeup_irq()
> + *
> + * Function to set/clear per-CPU wake-up IRQs. Not protected by locking since
> + * it might be used in code paths where normal cacheable locks are not
> + * working. Locking must be provided by the caller to ensure atomicity.
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + * @cpu: mpidr[7:0] bitfield describing cpu affinity level
> + * @set: if true, wake-up IRQs are set, if false they are cleared
> + */
> +void ve_spc_cpu_wakeup_irq(u32 cluster, u32 cpu, bool set)
> +{
> +	u32 mask, reg;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return;
> +
> +	mask = 1 << cpu;
> +
> +	if (!cluster_is_a15(cluster))
> +		mask <<= 4;
> +
> +	reg = readl_relaxed(info->baseaddr + WAKE_INT_MASK);
> +
> +	if (set)
> +		reg |= mask;
> +	else
> +		reg &= ~mask;
> +
> +	writel_relaxed(reg, info->baseaddr + WAKE_INT_MASK);
> +}
> +
> +/**
> + * ve_spc_set_resume_addr() - set the jump address used for warm boot
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + * @cpu: mpidr[7:0] bitfield describing cpu affinity level
> + * @addr: physical resume address
> + */
> +void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
> +{
> +	void __iomem *baseaddr;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return;
> +
> +	if (cluster_is_a15(cluster))
> +		baseaddr = info->baseaddr + A15_BX_ADDR0 + (cpu << 2);
> +	else
> +		baseaddr = info->baseaddr + A7_BX_ADDR0 + (cpu << 2);
> +
> +	writel_relaxed(addr, baseaddr);
> +}
> +
> +/**
> + * ve_spc_get_nr_cpus() - get number of cpus in a cluster
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + *
> + * Return: > 0 number of cpus in the cluster
> + *         or 0 if cluster number invalid
> + */
> +u32 ve_spc_get_nr_cpus(u32 cluster)
> +{
> +	u32 val;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return 0;
> +
> +	val = readl_relaxed(info->baseaddr + SYS_INFO);
> +	val = cluster_is_a15(cluster) ? (val >> 16) : (val >> 20);
> +	return val & 0xf;
> +}
> +
> +/**
> + * ve_spc_powerdown()
> + *
> + * Function to enable/disable cluster powerdown. Not protected by locking
> + * since it might be used in code paths where normal cacheable locks are not
> + * working. Locking must be provided by the caller to ensure atomicity.
> + *
> + * @cluster: mpidr[15:8] bitfield describing cluster affinity level
> + * @enable: if true enables powerdown, if false disables it
> + */
> +void ve_spc_powerdown(u32 cluster, bool enable)
> +{
> +	u32 pwdrn_reg;
> +
> +	if (cluster >= MAX_CLUSTERS)
> +		return;
> +
> +	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
> +	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);
> +
> +	if (!node)
> +		return -ENODEV;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info) {
> +		pr_err(SPCLOG "unable to allocate mem\n");
> +		return -ENOMEM;
> +	}
> +
> +	info->baseaddr = of_iomap(node, 0);
> +	if (!info->baseaddr) {
> +		pr_err(SPCLOG "unable to ioremap memory\n");
> +		ret = -ENXIO;
> +		goto mem_free;
> +	}
> +
> +	info->a15_clusid = readl_relaxed(info->baseaddr + A15_CONF) & 0xf;
> +
> +	/*
> +	 * Multi-cluster systems may need this data when non-coherent, during
> +	 * cluster power-up/power-down. Make sure driver info reaches main
> +	 * memory.
> +	 */
> +	sync_cache_w(info);
> +	sync_cache_w(&info);
> +	pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
> +	return 0;
> +
> +mem_free:
> +	kfree(info);
> +	return ret;
> +}
> +
> +/**
> + * ve_spc_init()
> + *
> + * Function exported to manage pre early_initcall initialization.
> + * SPC code is needed very early in the boot process to bring CPUs out of
> + * reset and initialize power management back-end so an init interface is
> + * provided to platform code to allow early initialization. The init
> + * interface can be removed as soon as the DT layer and platform bus allow
> + * platform device creation and probing before SMP boot.
> + */
> +int __init ve_spc_init(void)
> +{
> +	static int ve_spc_init_status __initdata = -EAGAIN;
> +
> +	if (ve_spc_init_status == -EAGAIN)
> +		ve_spc_init_status = ve_spc_probe();
> +
> +	return ve_spc_init_status;
> +}
> +early_initcall(ve_spc_init);
> diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h
> index 50368e0..d0106ef 100644
> --- a/include/linux/vexpress.h
> +++ b/include/linux/vexpress.h
> @@ -132,4 +132,21 @@ void vexpress_clk_of_register_spc(void);
>  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
> 

Re: [RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Nicolas Pitre]

On Tue, 16 Jul 2013, Rob Herring wrote:

> On 07/16/2013 11:05 AM, Lorenzo Pieralisi wrote:
> > The TC2 versatile express core tile integrates a logic block that provides the
> > interface between the dual cluster test-chip and the M3 microcontroller that
> > carries out power management. The logic block, called Serial Power Controller
> > (SPC), contains several memory mapped registers to control among other things
> > low-power states, wake-up irqs and per-CPU jump addresses registers.
> > 
> > This patch provides a driver that enables run-time control of features
> > implemented by the SPC power management control logic.
> > 
> > The SPC control logic is required to be programmed very early in the boot
> > process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
> > wake-up IRQs for power management. Hence, waiting for core changes to be
> > made in the device core code to enable early registration of platform
> > devices, the driver puts in place an early init scheme that allows kernel
> > drivers to initialize the SPC driver directly from the components requiring
> > it, if their initialization routine is called before the driver init
> > function by the boot process.
> > 
> > Device tree bindings documentation for the SPC component is provided with
> > the patchset.
> 
> Just curious, wouldn't a TC2 PSCI implementation eliminate the need for
> most/all of this code?

There is a PSCI equivalent for the above already, in the sense that 
there is a simple MCPM backend that bypass most of the MCPM race 
avoidance code paths and simply calls into PSCI instead.  But not all 
the world is going to be PSCI, and therefore we need to ensure good 
support for non-PSCI platforms as well.

This is why TC2 supports both, and this also ensure that the PSCI 
implementation, which won't be part of the kernel and therefore unlikely 
to get the same level of scrutiny, is properly implemented and doesn't 
introduce any regression when compared to the non PSCI case.

Remember that TC2 is multi-cluster which means that the PSCI 
implementation has to carry the same amount of complexity as the whole 
in-kernel MCPM layer and that doesn't make me overly confident it is 
going to always be right.

All this to say that we do need this code despite PSCI availability.


Nicolas

[RFC PATCH v5 1/1] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

[Nicolas Pitre]

On Tue, 16 Jul 2013, Rob Herring wrote:

> On 07/16/2013 11:05 AM, Lorenzo Pieralisi wrote:
> > The TC2 versatile express core tile integrates a logic block that provides the
> > interface between the dual cluster test-chip and the M3 microcontroller that
> > carries out power management. The logic block, called Serial Power Controller
> > (SPC), contains several memory mapped registers to control among other things
> > low-power states, wake-up irqs and per-CPU jump addresses registers.
> > 
> > This patch provides a driver that enables run-time control of features
> > implemented by the SPC power management control logic.
> > 
> > The SPC control logic is required to be programmed very early in the boot
> > process to reset secondary CPUs on the TC2 testchip, set-up jump addresses and
> > wake-up IRQs for power management. Hence, waiting for core changes to be
> > made in the device core code to enable early registration of platform
> > devices, the driver puts in place an early init scheme that allows kernel
> > drivers to initialize the SPC driver directly from the components requiring
> > it, if their initialization routine is called before the driver init
> > function by the boot process.
> > 
> > Device tree bindings documentation for the SPC component is provided with
> > the patchset.
> 
> Just curious, wouldn't a TC2 PSCI implementation eliminate the need for
> most/all of this code?

There is a PSCI equivalent for the above already, in the sense that 
there is a simple MCPM backend that bypass most of the MCPM race 
avoidance code paths and simply calls into PSCI instead.  But not all 
the world is going to be PSCI, and therefore we need to ensure good 
support for non-PSCI platforms as well.

This is why TC2 supports both, and this also ensure that the PSCI 
implementation, which won't be part of the kernel and therefore unlikely 
to get the same level of scrutiny, is properly implemented and doesn't 
introduce any regression when compared to the non PSCI case.

Remember that TC2 is multi-cluster which means that the PSCI 
implementation has to carry the same amount of complexity as the whole 
in-kernel MCPM layer and that doesn't make me overly confident it is 
going to always be right.

All this to say that we do need this code despite PSCI availability.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Tue, 16 Jul 2013, Lorenzo Pieralisi wrote:

> Hello,
> 
> version v5 of VExpress SPC driver, please read on the changelog for major
> changes and explanations.
> 
> The probing scheme is unchanged, since after trying the early platform
> devices approach it appeared that the end result was no better than the
> current one. The only clean solution relies either on changing how
> secondaries are brought up in the kernel (later than now) or enable
> early platform device registration through DT. Please check this
> thread for the related discussion:
> 
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> 
> The interface was adapted to regmap and again reverted to old driver for
> the following reasons:
> 
> - Power down registers locking is hairy and requires arch spinlocks in
>   the MCPM back end to work properly, normal spinlocks cannot be used
> - Regmap adds unnecessary code to manage SPC since it is just a bunch of
>   registers used to control power management flags, the overhead is just
>   not worth it (talking about power down registers, not the vexpress config
>   interface)
> - The locking scheme behind regmap requires all registers in the map
>   to be protected with the same lock, which is not exactly what we want
>   here
> - Given the reasons above, adding a regmap interface buys us nothing from
>   a driver readability and maintainability perspective (again just talking
>   about the power interface, a few registers) because for the SPC it would
>   simply not be used
> 
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).
> 
> Thank you for the review in advance,
> Lorenzo

I've integrated this patch in my MCPM backend for TC2 patch series.

ACKs from concerned/interested people would be appreciated so I could 
send this for ARM-SOC inclusion right away.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Tue, 16 Jul 2013, Lorenzo Pieralisi wrote:

> Hello,
> 
> version v5 of VExpress SPC driver, please read on the changelog for major
> changes and explanations.
> 
> The probing scheme is unchanged, since after trying the early platform
> devices approach it appeared that the end result was no better than the
> current one. The only clean solution relies either on changing how
> secondaries are brought up in the kernel (later than now) or enable
> early platform device registration through DT. Please check this
> thread for the related discussion:
> 
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> 
> The interface was adapted to regmap and again reverted to old driver for
> the following reasons:
> 
> - Power down registers locking is hairy and requires arch spinlocks in
>   the MCPM back end to work properly, normal spinlocks cannot be used
> - Regmap adds unnecessary code to manage SPC since it is just a bunch of
>   registers used to control power management flags, the overhead is just
>   not worth it (talking about power down registers, not the vexpress config
>   interface)
> - The locking scheme behind regmap requires all registers in the map
>   to be protected with the same lock, which is not exactly what we want
>   here
> - Given the reasons above, adding a regmap interface buys us nothing from
>   a driver readability and maintainability perspective (again just talking
>   about the power interface, a few registers) because for the SPC it would
>   simply not be used
> 
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).
> 
> Thank you for the review in advance,
> Lorenzo

I've integrated this patch in my MCPM backend for TC2 patch series.

ACKs from concerned/interested people would be appreciated so I could 
send this for ARM-SOC inclusion right away.


Nicolas

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Tue, 16 Jul 2013, Lorenzo Pieralisi wrote:

> Hello,
> 
> version v5 of VExpress SPC driver, please read on the changelog for major
> changes and explanations.
> 
> The probing scheme is unchanged, since after trying the early platform
> devices approach it appeared that the end result was no better than the
> current one. The only clean solution relies either on changing how
> secondaries are brought up in the kernel (later than now) or enable
> early platform device registration through DT. Please check this
> thread for the related discussion:
> 
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> 
> The interface was adapted to regmap and again reverted to old driver for
> the following reasons:
> 
> - Power down registers locking is hairy and requires arch spinlocks in
>   the MCPM back end to work properly, normal spinlocks cannot be used
> - Regmap adds unnecessary code to manage SPC since it is just a bunch of
>   registers used to control power management flags, the overhead is just
>   not worth it (talking about power down registers, not the vexpress config
>   interface)
> - The locking scheme behind regmap requires all registers in the map
>   to be protected with the same lock, which is not exactly what we want
>   here
> - Given the reasons above, adding a regmap interface buys us nothing from
>   a driver readability and maintainability perspective (again just talking
>   about the power interface, a few registers) because for the SPC it would
>   simply not be used
> 
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).
> 
> Thank you for the review in advance,
> Lorenzo

I've integrated this patch in my MCPM backend for TC2 patch series.

ACKs from concerned/interested people would be appreciated so I could 
send this for ARM-SOC inclusion right away.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).

Not that it really matters now, but my vexpress-sysreg rework will -
most likely - leave only skeleton in the MFD (registering mfd_cells) and
other stuff is going to be spread all around. Then I'm planning to move
the remaining of the vexpress-specific initialization to
drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
this (non-existing yet) directory would be the right thing to do?

Other than that:

Acked-by: Pawel Moll <pawel.moll@arm.com>

Thanks!

Paweł

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).

Not that it really matters now, but my vexpress-sysreg rework will -
most likely - leave only skeleton in the MFD (registering mfd_cells) and
other stuff is going to be spread all around. Then I'm planning to move
the remaining of the vexpress-specific initialization to
drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
this (non-existing yet) directory would be the right thing to do?

Other than that:

Acked-by: Pawel Moll <pawel.moll@arm.com>

Thanks!

Paweł

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).

Not that it really matters now, but my vexpress-sysreg rework will -
most likely - leave only skeleton in the MFD (registering mfd_cells) and
other stuff is going to be spread all around. Then I'm planning to move
the remaining of the vexpress-specific initialization to
drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
this (non-existing yet) directory would be the right thing to do?

Other than that:

Acked-by: Pawel Moll <pawel.moll@arm.com>

Thanks!

Pawe?

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

On Wed, Jul 17, 2013 at 10:18:25AM +0100, Pawel Moll wrote:
> On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> 
> Not that it really matters now, but my vexpress-sysreg rework will -
> most likely - leave only skeleton in the MFD (registering mfd_cells) and
> other stuff is going to be spread all around. Then I'm planning to move
> the remaining of the vexpress-specific initialization to
> drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
> this (non-existing yet) directory would be the right thing to do?

Done. I do not think there is a point in splitting the patch to create
the dir and make infrastructure, so I squashed everything in the
original patch. I have not added any maintainer for that dir/file, I
guess it can wait till you finish the rework so that you can add
yourself there.

If that's all I need to change I do not even think that reposting is
necessary.

It does matter though, since it implies changes on who is in charge of
ack/nack'ing this code, if it is no more an mfd matter.

I will wait to check all interested/concerned parties opinions, that are
always welcome.

> Other than that:
> 
> Acked-by: Pawel Moll <pawel.moll@arm.com>

Thank you !!!
Lorenzo

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

On Wed, Jul 17, 2013 at 10:18:25AM +0100, Pawel Moll wrote:
> On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> 
> Not that it really matters now, but my vexpress-sysreg rework will -
> most likely - leave only skeleton in the MFD (registering mfd_cells) and
> other stuff is going to be spread all around. Then I'm planning to move
> the remaining of the vexpress-specific initialization to
> drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
> this (non-existing yet) directory would be the right thing to do?

Done. I do not think there is a point in splitting the patch to create
the dir and make infrastructure, so I squashed everything in the
original patch. I have not added any maintainer for that dir/file, I
guess it can wait till you finish the rework so that you can add
yourself there.

If that's all I need to change I do not even think that reposting is
necessary.

It does matter though, since it implies changes on who is in charge of
ack/nack'ing this code, if it is no more an mfd matter.

I will wait to check all interested/concerned parties opinions, that are
always welcome.

> Other than that:
> 
> Acked-by: Pawel Moll <pawel.moll@arm.com>

Thank you !!!
Lorenzo

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

On Wed, Jul 17, 2013 at 10:18:25AM +0100, Pawel Moll wrote:
> On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> 
> Not that it really matters now, but my vexpress-sysreg rework will -
> most likely - leave only skeleton in the MFD (registering mfd_cells) and
> other stuff is going to be spread all around. Then I'm planning to move
> the remaining of the vexpress-specific initialization to
> drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
> this (non-existing yet) directory would be the right thing to do?

Done. I do not think there is a point in splitting the patch to create
the dir and make infrastructure, so I squashed everything in the
original patch. I have not added any maintainer for that dir/file, I
guess it can wait till you finish the rework so that you can add
yourself there.

If that's all I need to change I do not even think that reposting is
necessary.

It does matter though, since it implies changes on who is in charge of
ack/nack'ing this code, if it is no more an mfd matter.

I will wait to check all interested/concerned parties opinions, that are
always welcome.

> Other than that:
> 
> Acked-by: Pawel Moll <pawel.moll@arm.com>

Thank you !!!
Lorenzo

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> 
> Not that it really matters now, but my vexpress-sysreg rework will -
> most likely - leave only skeleton in the MFD (registering mfd_cells) and
> other stuff is going to be spread all around. Then I'm planning to move
> the remaining of the vexpress-specific initialization to
> drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
> this (non-existing yet) directory would be the right thing to do?

I don't like this idea.  We worked hard to shrink platform specific 
directories such as arch/arm/mach-*/ as much as possible.  Simply moving 
stuff to drivers/platform/arm/* doesn't make the situation any much 
better.

If this is really miscelaneous code that really doesn't fit 
anywhere else, it should rather go into drivers/misc/ as a last resort.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> 
> Not that it really matters now, but my vexpress-sysreg rework will -
> most likely - leave only skeleton in the MFD (registering mfd_cells) and
> other stuff is going to be spread all around. Then I'm planning to move
> the remaining of the vexpress-specific initialization to
> drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
> this (non-existing yet) directory would be the right thing to do?

I don't like this idea.  We worked hard to shrink platform specific 
directories such as arch/arm/mach-*/ as much as possible.  Simply moving 
stuff to drivers/platform/arm/* doesn't make the situation any much 
better.

If this is really miscelaneous code that really doesn't fit 
anywhere else, it should rather go into drivers/misc/ as a last resort.


Nicolas

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Tue, 2013-07-16 at 17:05 +0100, Lorenzo Pieralisi wrote:
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> 
> Not that it really matters now, but my vexpress-sysreg rework will -
> most likely - leave only skeleton in the MFD (registering mfd_cells) and
> other stuff is going to be spread all around. Then I'm planning to move
> the remaining of the vexpress-specific initialization to
> drivers/platform/arm/vexpress.c, so maybe sticking vexpress-spc.c to
> this (non-existing yet) directory would be the right thing to do?

I don't like this idea.  We worked hard to shrink platform specific 
directories such as arch/arm/mach-*/ as much as possible.  Simply moving 
stuff to drivers/platform/arm/* doesn't make the situation any much 
better.

If this is really miscelaneous code that really doesn't fit 
anywhere else, it should rather go into drivers/misc/ as a last resort.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> If this is really miscelaneous code that really doesn't fit 
> anywhere else, it should rather go into drivers/misc/ as a last resort.

Interestingly enough drivers/misc was my first choice for all the
vexpress stuff, but it wasn't received well...

Anyway, the SPC driver as it is now seem to be a "power management
system driver". Maybe a relevant directory would be in place? Wouldn't
PSCI belong there as well? (there are two psci.c files in arch/arm and
arch/arm64, surprisingly similar ones ;-)

The bottom line is: today it is not an MFD driver.

Paweł

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> If this is really miscelaneous code that really doesn't fit 
> anywhere else, it should rather go into drivers/misc/ as a last resort.

Interestingly enough drivers/misc was my first choice for all the
vexpress stuff, but it wasn't received well...

Anyway, the SPC driver as it is now seem to be a "power management
system driver". Maybe a relevant directory would be in place? Wouldn't
PSCI belong there as well? (there are two psci.c files in arch/arm and
arch/arm64, surprisingly similar ones ;-)

The bottom line is: today it is not an MFD driver.

Paweł

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> If this is really miscelaneous code that really doesn't fit 
> anywhere else, it should rather go into drivers/misc/ as a last resort.

Interestingly enough drivers/misc was my first choice for all the
vexpress stuff, but it wasn't received well...

Anyway, the SPC driver as it is now seem to be a "power management
system driver". Maybe a relevant directory would be in place? Wouldn't
PSCI belong there as well? (there are two psci.c files in arch/arm and
arch/arm64, surprisingly similar ones ;-)

The bottom line is: today it is not an MFD driver.

Pawe?

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > If this is really miscelaneous code that really doesn't fit 
> > anywhere else, it should rather go into drivers/misc/ as a last resort.
> 
> Interestingly enough drivers/misc was my first choice for all the
> vexpress stuff, but it wasn't received well...
> 
> Anyway, the SPC driver as it is now seem to be a "power management
> system driver". Maybe a relevant directory would be in place? Wouldn't
> PSCI belong there as well? (there are two psci.c files in arch/arm and
> arch/arm64, surprisingly similar ones ;-)
> 
> The bottom line is: today it is not an MFD driver.

But we know it will, right?  So better  save some churn by storing the 
initial code where it would end up anyway once complete.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > If this is really miscelaneous code that really doesn't fit 
> > anywhere else, it should rather go into drivers/misc/ as a last resort.
> 
> Interestingly enough drivers/misc was my first choice for all the
> vexpress stuff, but it wasn't received well...
> 
> Anyway, the SPC driver as it is now seem to be a "power management
> system driver". Maybe a relevant directory would be in place? Wouldn't
> PSCI belong there as well? (there are two psci.c files in arch/arm and
> arch/arm64, surprisingly similar ones ;-)
> 
> The bottom line is: today it is not an MFD driver.

But we know it will, right?  So better  save some churn by storing the 
initial code where it would end up anyway once complete.


Nicolas

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > If this is really miscelaneous code that really doesn't fit 
> > anywhere else, it should rather go into drivers/misc/ as a last resort.
> 
> Interestingly enough drivers/misc was my first choice for all the
> vexpress stuff, but it wasn't received well...
> 
> Anyway, the SPC driver as it is now seem to be a "power management
> system driver". Maybe a relevant directory would be in place? Wouldn't
> PSCI belong there as well? (there are two psci.c files in arch/arm and
> arch/arm64, surprisingly similar ones ;-)
> 
> The bottom line is: today it is not an MFD driver.

But we know it will, right?  So better  save some churn by storing the 
initial code where it would end up anyway once complete.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Pawel Moll wrote:
> 
> > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > If this is really miscelaneous code that really doesn't fit 
> > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > 
> > Interestingly enough drivers/misc was my first choice for all the
> > vexpress stuff, but it wasn't received well...
> > 
> > Anyway, the SPC driver as it is now seem to be a "power management
> > system driver". Maybe a relevant directory would be in place? Wouldn't
> > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > arch/arm64, surprisingly similar ones ;-)
> > 
> > The bottom line is: today it is not an MFD driver.
> 
> But we know it will, right?  So better  save some churn by storing the 
> initial code where it would end up anyway once complete.

Not in that form, no. The code living in mfd will just register
mfd_cells while "functional" parts are going to live elsewhere. This is
how I understand what Samuel asked me to do and that's what is happening
to vexpress-sysreg now.

Paweł

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Pawel Moll wrote:
> 
> > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > If this is really miscelaneous code that really doesn't fit 
> > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > 
> > Interestingly enough drivers/misc was my first choice for all the
> > vexpress stuff, but it wasn't received well...
> > 
> > Anyway, the SPC driver as it is now seem to be a "power management
> > system driver". Maybe a relevant directory would be in place? Wouldn't
> > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > arch/arm64, surprisingly similar ones ;-)
> > 
> > The bottom line is: today it is not an MFD driver.
> 
> But we know it will, right?  So better  save some churn by storing the 
> initial code where it would end up anyway once complete.

Not in that form, no. The code living in mfd will just register
mfd_cells while "functional" parts are going to live elsewhere. This is
how I understand what Samuel asked me to do and that's what is happening
to vexpress-sysreg now.

Paweł

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Pawel Moll]

On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Pawel Moll wrote:
> 
> > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > If this is really miscelaneous code that really doesn't fit 
> > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > 
> > Interestingly enough drivers/misc was my first choice for all the
> > vexpress stuff, but it wasn't received well...
> > 
> > Anyway, the SPC driver as it is now seem to be a "power management
> > system driver". Maybe a relevant directory would be in place? Wouldn't
> > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > arch/arm64, surprisingly similar ones ;-)
> > 
> > The bottom line is: today it is not an MFD driver.
> 
> But we know it will, right?  So better  save some churn by storing the 
> initial code where it would end up anyway once complete.

Not in that form, no. The code living in mfd will just register
mfd_cells while "functional" parts are going to live elsewhere. This is
how I understand what Samuel asked me to do and that's what is happening
to vexpress-sysreg now.

Pawe?

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> > On Wed, 17 Jul 2013, Pawel Moll wrote:
> > 
> > > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > > If this is really miscelaneous code that really doesn't fit 
> > > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > > 
> > > Interestingly enough drivers/misc was my first choice for all the
> > > vexpress stuff, but it wasn't received well...
> > > 
> > > Anyway, the SPC driver as it is now seem to be a "power management
> > > system driver". Maybe a relevant directory would be in place? Wouldn't
> > > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > > arch/arm64, surprisingly similar ones ;-)
> > > 
> > > The bottom line is: today it is not an MFD driver.
> > 
> > But we know it will, right?  So better  save some churn by storing the 
> > initial code where it would end up anyway once complete.
> 
> Not in that form, no. The code living in mfd will just register
> mfd_cells while "functional" parts are going to live elsewhere. This is
> how I understand what Samuel asked me to do and that's what is happening
> to vexpress-sysreg now.

A drivers/pm/ or drivers/power/ could be created, but that implies sort 
of a more defined subsystem with a common API and the SPC code doesn't 
fit that as it is only providing services to actual drivers on top of 
it.

The sanest location at this point might simply be 
drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
depending on whether or not more such driver glue is expected in the 
vexpress future.  No point putting "arm" in the path, especially if this 
is later reused on arm64.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> > On Wed, 17 Jul 2013, Pawel Moll wrote:
> > 
> > > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > > If this is really miscelaneous code that really doesn't fit 
> > > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > > 
> > > Interestingly enough drivers/misc was my first choice for all the
> > > vexpress stuff, but it wasn't received well...
> > > 
> > > Anyway, the SPC driver as it is now seem to be a "power management
> > > system driver". Maybe a relevant directory would be in place? Wouldn't
> > > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > > arch/arm64, surprisingly similar ones ;-)
> > > 
> > > The bottom line is: today it is not an MFD driver.
> > 
> > But we know it will, right?  So better  save some churn by storing the 
> > initial code where it would end up anyway once complete.
> 
> Not in that form, no. The code living in mfd will just register
> mfd_cells while "functional" parts are going to live elsewhere. This is
> how I understand what Samuel asked me to do and that's what is happening
> to vexpress-sysreg now.

A drivers/pm/ or drivers/power/ could be created, but that implies sort 
of a more defined subsystem with a common API and the SPC code doesn't 
fit that as it is only providing services to actual drivers on top of 
it.

The sanest location at this point might simply be 
drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
depending on whether or not more such driver glue is expected in the 
vexpress future.  No point putting "arm" in the path, especially if this 
is later reused on arm64.


Nicolas

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Pawel Moll wrote:

> On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> > On Wed, 17 Jul 2013, Pawel Moll wrote:
> > 
> > > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > > If this is really miscelaneous code that really doesn't fit 
> > > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > > 
> > > Interestingly enough drivers/misc was my first choice for all the
> > > vexpress stuff, but it wasn't received well...
> > > 
> > > Anyway, the SPC driver as it is now seem to be a "power management
> > > system driver". Maybe a relevant directory would be in place? Wouldn't
> > > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > > arch/arm64, surprisingly similar ones ;-)
> > > 
> > > The bottom line is: today it is not an MFD driver.
> > 
> > But we know it will, right?  So better  save some churn by storing the 
> > initial code where it would end up anyway once complete.
> 
> Not in that form, no. The code living in mfd will just register
> mfd_cells while "functional" parts are going to live elsewhere. This is
> how I understand what Samuel asked me to do and that's what is happening
> to vexpress-sysreg now.

A drivers/pm/ or drivers/power/ could be created, but that implies sort 
of a more defined subsystem with a common API and the SPC code doesn't 
fit that as it is only providing services to actual drivers on top of 
it.

The sanest location at this point might simply be 
drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
depending on whether or not more such driver glue is expected in the 
vexpress future.  No point putting "arm" in the path, especially if this 
is later reused on arm64.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Russell King - ARM Linux]

On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> The sanest location at this point might simply be 
> drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> depending on whether or not more such driver glue is expected in the 
> vexpress future.  No point putting "arm" in the path, especially if this 
> is later reused on arm64.

You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
you'd probably be arguing that "arm" made perfect sense.

Don't get too hung up on names please, it's really not worth the time
and effort being soo pedantic, and being soo pedantic leads to "pointless
churn" when someone comes along and wants to pedantically change the
names because it no longer matches the users.

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Russell King - ARM Linux]

On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> The sanest location at this point might simply be 
> drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> depending on whether or not more such driver glue is expected in the 
> vexpress future.  No point putting "arm" in the path, especially if this 
> is later reused on arm64.

You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
you'd probably be arguing that "arm" made perfect sense.

Don't get too hung up on names please, it's really not worth the time
and effort being soo pedantic, and being soo pedantic leads to "pointless
churn" when someone comes along and wants to pedantically change the
names because it no longer matches the users.

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Russell King - ARM Linux]

On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> The sanest location at this point might simply be 
> drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> depending on whether or not more such driver glue is expected in the 
> vexpress future.  No point putting "arm" in the path, especially if this 
> is later reused on arm64.

You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
you'd probably be arguing that "arm" made perfect sense.

Don't get too hung up on names please, it's really not worth the time
and effort being soo pedantic, and being soo pedantic leads to "pointless
churn" when someone comes along and wants to pedantically change the
names because it no longer matches the users.

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Russell King - ARM Linux wrote:

> On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> > The sanest location at this point might simply be 
> > drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> > depending on whether or not more such driver glue is expected in the 
> > vexpress future.  No point putting "arm" in the path, especially if this 
> > is later reused on arm64.
> 
> You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
> you'd probably be arguing that "arm" made perfect sense.

Well... in a sense: yes.  But in the end, having per arch directories 
under drivers/ is silly.  We already have per arch directories under 
arch/already.

> Don't get too hung up on names please, it's really not worth the time
> and effort being soo pedantic, and being soo pedantic leads to "pointless
> churn" when someone comes along and wants to pedantically change the
> names because it no longer matches the users.

At this point I don't really care about the name.  I just want the damn 
thing merged upstream.  But after several iterations to either fit one 
or another maintainers taste, each rework ends up in that maintainer 
saying: "Now that you've reworked the code, I still don't like it since 
this no longer fits in my subsystem tree."

In fact what we'd need at this point is 
drivers/code_that_no_subsystem_maintainers_wants/.  This is becoming 
overly ridiculous.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Russell King - ARM Linux wrote:

> On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> > The sanest location at this point might simply be 
> > drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> > depending on whether or not more such driver glue is expected in the 
> > vexpress future.  No point putting "arm" in the path, especially if this 
> > is later reused on arm64.
> 
> You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
> you'd probably be arguing that "arm" made perfect sense.

Well... in a sense: yes.  But in the end, having per arch directories 
under drivers/ is silly.  We already have per arch directories under 
arch/already.

> Don't get too hung up on names please, it's really not worth the time
> and effort being soo pedantic, and being soo pedantic leads to "pointless
> churn" when someone comes along and wants to pedantically change the
> names because it no longer matches the users.

At this point I don't really care about the name.  I just want the damn 
thing merged upstream.  But after several iterations to either fit one 
or another maintainers taste, each rework ends up in that maintainer 
saying: "Now that you've reworked the code, I still don't like it since 
this no longer fits in my subsystem tree."

In fact what we'd need at this point is 
drivers/code_that_no_subsystem_maintainers_wants/.  This is becoming 
overly ridiculous.


Nicolas

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Russell King - ARM Linux wrote:

> On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> > The sanest location at this point might simply be 
> > drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> > depending on whether or not more such driver glue is expected in the 
> > vexpress future.  No point putting "arm" in the path, especially if this 
> > is later reused on arm64.
> 
> You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
> you'd probably be arguing that "arm" made perfect sense.

Well... in a sense: yes.  But in the end, having per arch directories 
under drivers/ is silly.  We already have per arch directories under 
arch/already.

> Don't get too hung up on names please, it's really not worth the time
> and effort being soo pedantic, and being soo pedantic leads to "pointless
> churn" when someone comes along and wants to pedantically change the
> names because it no longer matches the users.

At this point I don't really care about the name.  I just want the damn 
thing merged upstream.  But after several iterations to either fit one 
or another maintainers taste, each rework ends up in that maintainer 
saying: "Now that you've reworked the code, I still don't like it since 
this no longer fits in my subsystem tree."

In fact what we'd need at this point is 
drivers/code_that_no_subsystem_maintainers_wants/.  This is becoming 
overly ridiculous.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Lorenzo,

On Tue, Jul 16, 2013 at 05:05:42PM +0100, Lorenzo Pieralisi wrote:
> Hello,
> 
> version v5 of VExpress SPC driver, please read on the changelog for major
> changes and explanations.
> 
> The probing scheme is unchanged, since after trying the early platform
> devices approach it appeared that the end result was no better than the
> current one. The only clean solution relies either on changing how
> secondaries are brought up in the kernel (later than now) or enable
> early platform device registration through DT. Please check this
> thread for the related discussion:
> 
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> 
> The interface was adapted to regmap and again reverted to old driver for
> the following reasons:
> 
> - Power down registers locking is hairy and requires arch spinlocks in
>   the MCPM back end to work properly, normal spinlocks cannot be used
> - Regmap adds unnecessary code to manage SPC since it is just a bunch of
>   registers used to control power management flags, the overhead is just
>   not worth it (talking about power down registers, not the vexpress config
>   interface)
> - The locking scheme behind regmap requires all registers in the map
>   to be protected with the same lock, which is not exactly what we want
>   here
> - Given the reasons above, adding a regmap interface buys us nothing from
>   a driver readability and maintainability perspective (again just talking
>   about the power interface, a few registers) because for the SPC it would
>   simply not be used
> 
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).
Could you please elaborate on how will the SPC driver extend into an MFD
driver?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Lorenzo,

On Tue, Jul 16, 2013 at 05:05:42PM +0100, Lorenzo Pieralisi wrote:
> Hello,
> 
> version v5 of VExpress SPC driver, please read on the changelog for major
> changes and explanations.
> 
> The probing scheme is unchanged, since after trying the early platform
> devices approach it appeared that the end result was no better than the
> current one. The only clean solution relies either on changing how
> secondaries are brought up in the kernel (later than now) or enable
> early platform device registration through DT. Please check this
> thread for the related discussion:
> 
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> 
> The interface was adapted to regmap and again reverted to old driver for
> the following reasons:
> 
> - Power down registers locking is hairy and requires arch spinlocks in
>   the MCPM back end to work properly, normal spinlocks cannot be used
> - Regmap adds unnecessary code to manage SPC since it is just a bunch of
>   registers used to control power management flags, the overhead is just
>   not worth it (talking about power down registers, not the vexpress config
>   interface)
> - The locking scheme behind regmap requires all registers in the map
>   to be protected with the same lock, which is not exactly what we want
>   here
> - Given the reasons above, adding a regmap interface buys us nothing from
>   a driver readability and maintainability perspective (again just talking
>   about the power interface, a few registers) because for the SPC it would
>   simply not be used
> 
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).
Could you please elaborate on how will the SPC driver extend into an MFD
driver?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Lorenzo,

On Tue, Jul 16, 2013 at 05:05:42PM +0100, Lorenzo Pieralisi wrote:
> Hello,
> 
> version v5 of VExpress SPC driver, please read on the changelog for major
> changes and explanations.
> 
> The probing scheme is unchanged, since after trying the early platform
> devices approach it appeared that the end result was no better than the
> current one. The only clean solution relies either on changing how
> secondaries are brought up in the kernel (later than now) or enable
> early platform device registration through DT. Please check this
> thread for the related discussion:
> 
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> 
> The interface was adapted to regmap and again reverted to old driver for
> the following reasons:
> 
> - Power down registers locking is hairy and requires arch spinlocks in
>   the MCPM back end to work properly, normal spinlocks cannot be used
> - Regmap adds unnecessary code to manage SPC since it is just a bunch of
>   registers used to control power management flags, the overhead is just
>   not worth it (talking about power down registers, not the vexpress config
>   interface)
> - The locking scheme behind regmap requires all registers in the map
>   to be protected with the same lock, which is not exactly what we want
>   here
> - Given the reasons above, adding a regmap interface buys us nothing from
>   a driver readability and maintainability perspective (again just talking
>   about the power interface, a few registers) because for the SPC it would
>   simply not be used
> 
> /drivers/mfd is probably not the right place for this code as it stands (but
> probably will be when the entire driver, with DVFS and config interface, is
> complete).
Could you please elaborate on how will the SPC driver extend into an MFD
driver?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Pawel,

On Wed, Jul 17, 2013 at 03:20:11PM +0100, Pawel Moll wrote:
> On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> > On Wed, 17 Jul 2013, Pawel Moll wrote:
> > 
> > > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > > If this is really miscelaneous code that really doesn't fit 
> > > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > > 
> > > Interestingly enough drivers/misc was my first choice for all the
> > > vexpress stuff, but it wasn't received well...
> > > 
> > > Anyway, the SPC driver as it is now seem to be a "power management
> > > system driver". Maybe a relevant directory would be in place? Wouldn't
> > > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > > arch/arm64, surprisingly similar ones ;-)
> > > 
> > > The bottom line is: today it is not an MFD driver.
> > 
> > But we know it will, right?  So better  save some churn by storing the 
> > initial code where it would end up anyway once complete.
> 
> Not in that form, no. The code living in mfd will just register
> mfd_cells while "functional" parts are going to live elsewhere. This is
> how I understand what Samuel asked me to do and that's what is happening
> to vexpress-sysreg now.
Very good, I'll happily apply such changes.
If I understand the IP correctly, the SPC driver will stay as a set of
runtime APIs for controlling the SPC features. If that's the case, then
misc sounds like a more appropriate place for this driver.
drivers/power/ really is for power supplies and charging drivers.

Cheers,
Samuel. 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Pawel,

On Wed, Jul 17, 2013 at 03:20:11PM +0100, Pawel Moll wrote:
> On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> > On Wed, 17 Jul 2013, Pawel Moll wrote:
> > 
> > > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > > If this is really miscelaneous code that really doesn't fit 
> > > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > > 
> > > Interestingly enough drivers/misc was my first choice for all the
> > > vexpress stuff, but it wasn't received well...
> > > 
> > > Anyway, the SPC driver as it is now seem to be a "power management
> > > system driver". Maybe a relevant directory would be in place? Wouldn't
> > > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > > arch/arm64, surprisingly similar ones ;-)
> > > 
> > > The bottom line is: today it is not an MFD driver.
> > 
> > But we know it will, right?  So better  save some churn by storing the 
> > initial code where it would end up anyway once complete.
> 
> Not in that form, no. The code living in mfd will just register
> mfd_cells while "functional" parts are going to live elsewhere. This is
> how I understand what Samuel asked me to do and that's what is happening
> to vexpress-sysreg now.
Very good, I'll happily apply such changes.
If I understand the IP correctly, the SPC driver will stay as a set of
runtime APIs for controlling the SPC features. If that's the case, then
misc sounds like a more appropriate place for this driver.
drivers/power/ really is for power supplies and charging drivers.

Cheers,
Samuel. 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Pawel,

On Wed, Jul 17, 2013 at 03:20:11PM +0100, Pawel Moll wrote:
> On Wed, 2013-07-17 at 15:16 +0100, Nicolas Pitre wrote:
> > On Wed, 17 Jul 2013, Pawel Moll wrote:
> > 
> > > On Wed, 2013-07-17 at 13:33 +0100, Nicolas Pitre wrote:
> > > > If this is really miscelaneous code that really doesn't fit 
> > > > anywhere else, it should rather go into drivers/misc/ as a last resort.
> > > 
> > > Interestingly enough drivers/misc was my first choice for all the
> > > vexpress stuff, but it wasn't received well...
> > > 
> > > Anyway, the SPC driver as it is now seem to be a "power management
> > > system driver". Maybe a relevant directory would be in place? Wouldn't
> > > PSCI belong there as well? (there are two psci.c files in arch/arm and
> > > arch/arm64, surprisingly similar ones ;-)
> > > 
> > > The bottom line is: today it is not an MFD driver.
> > 
> > But we know it will, right?  So better  save some churn by storing the 
> > initial code where it would end up anyway once complete.
> 
> Not in that form, no. The code living in mfd will just register
> mfd_cells while "functional" parts are going to live elsewhere. This is
> how I understand what Samuel asked me to do and that's what is happening
> to vexpress-sysreg now.
Very good, I'll happily apply such changes.
If I understand the IP correctly, the SPC driver will stay as a set of
runtime APIs for controlling the SPC features. If that's the case, then
misc sounds like a more appropriate place for this driver.
drivers/power/ really is for power supplies and charging drivers.

Cheers,
Samuel. 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Nicolas,

On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Russell King - ARM Linux wrote:
> 
> > On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> > > The sanest location at this point might simply be 
> > > drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> > > depending on whether or not more such driver glue is expected in the 
> > > vexpress future.  No point putting "arm" in the path, especially if this 
> > > is later reused on arm64.
> > 
> > You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
> > you'd probably be arguing that "arm" made perfect sense.
> 
> Well... in a sense: yes.  But in the end, having per arch directories 
> under drivers/ is silly.  We already have per arch directories under 
> arch/already.
> 
> > Don't get too hung up on names please, it's really not worth the time
> > and effort being soo pedantic, and being soo pedantic leads to "pointless
> > churn" when someone comes along and wants to pedantically change the
> > names because it no longer matches the users.
> 
> At this point I don't really care about the name.  I just want the damn 
> thing merged upstream.  But after several iterations to either fit one 
> or another maintainers taste, each rework ends up in that maintainer 
> saying: "Now that you've reworked the code, I still don't like it since 
> this no longer fits in my subsystem tree."
FWIW, we asked Pawel to rework the sysreg and config parts of the
vexpress driver, make it an actual MFD driver, and spread the remaining
bits of the code into their respective subsystems. I don't think
this is an eccentric requirement.


> In fact what we'd need at this point is 
> drivers/code_that_no_subsystem_maintainers_wants/.  
Which is what some people think drivers/mfd/ is...
I don't mind merging Lorenzo's SPC driver as it is if he can explain to
me how it will eventually evolve into an actual MFD driver. If that's
not the case, I don't see how I could justify merging it through the
MFD tree.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Nicolas,

On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Russell King - ARM Linux wrote:
> 
> > On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> > > The sanest location at this point might simply be 
> > > drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> > > depending on whether or not more such driver glue is expected in the 
> > > vexpress future.  No point putting "arm" in the path, especially if this 
> > > is later reused on arm64.
> > 
> > You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
> > you'd probably be arguing that "arm" made perfect sense.
> 
> Well... in a sense: yes.  But in the end, having per arch directories 
> under drivers/ is silly.  We already have per arch directories under 
> arch/already.
> 
> > Don't get too hung up on names please, it's really not worth the time
> > and effort being soo pedantic, and being soo pedantic leads to "pointless
> > churn" when someone comes along and wants to pedantically change the
> > names because it no longer matches the users.
> 
> At this point I don't really care about the name.  I just want the damn 
> thing merged upstream.  But after several iterations to either fit one 
> or another maintainers taste, each rework ends up in that maintainer 
> saying: "Now that you've reworked the code, I still don't like it since 
> this no longer fits in my subsystem tree."
FWIW, we asked Pawel to rework the sysreg and config parts of the
vexpress driver, make it an actual MFD driver, and spread the remaining
bits of the code into their respective subsystems. I don't think
this is an eccentric requirement.


> In fact what we'd need at this point is 
> drivers/code_that_no_subsystem_maintainers_wants/.  
Which is what some people think drivers/mfd/ is...
I don't mind merging Lorenzo's SPC driver as it is if he can explain to
me how it will eventually evolve into an actual MFD driver. If that's
not the case, I don't see how I could justify merging it through the
MFD tree.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

Hi Nicolas,

On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Russell King - ARM Linux wrote:
> 
> > On Wed, Jul 17, 2013 at 11:57:55AM -0400, Nicolas Pitre wrote:
> > > The sanest location at this point might simply be 
> > > drivers/platform/vexpress_spc.c or drivers/platform/vexpress/spc.c 
> > > depending on whether or not more such driver glue is expected in the 
> > > vexpress future.  No point putting "arm" in the path, especially if this 
> > > is later reused on arm64.
> > 
> > You wouldn't be making that argument if it were arch/arm64 and arch/arm32 -
> > you'd probably be arguing that "arm" made perfect sense.
> 
> Well... in a sense: yes.  But in the end, having per arch directories 
> under drivers/ is silly.  We already have per arch directories under 
> arch/already.
> 
> > Don't get too hung up on names please, it's really not worth the time
> > and effort being soo pedantic, and being soo pedantic leads to "pointless
> > churn" when someone comes along and wants to pedantically change the
> > names because it no longer matches the users.
> 
> At this point I don't really care about the name.  I just want the damn 
> thing merged upstream.  But after several iterations to either fit one 
> or another maintainers taste, each rework ends up in that maintainer 
> saying: "Now that you've reworked the code, I still don't like it since 
> this no longer fits in my subsystem tree."
FWIW, we asked Pawel to rework the sysreg and config parts of the
vexpress driver, make it an actual MFD driver, and spread the remaining
bits of the code into their respective subsystems. I don't think
this is an eccentric requirement.


> In fact what we'd need at this point is 
> drivers/code_that_no_subsystem_maintainers_wants/.  
Which is what some people think drivers/mfd/ is...
I don't mind merging Lorenzo's SPC driver as it is if he can explain to
me how it will eventually evolve into an actual MFD driver. If that's
not the case, I don't see how I could justify merging it through the
MFD tree.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Samuel Ortiz wrote:

> On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> > At this point I don't really care about the name.  I just want the damn 
> > thing merged upstream.  But after several iterations to either fit one 
> > or another maintainers taste, each rework ends up in that maintainer 
> > saying: "Now that you've reworked the code, I still don't like it since 
> > this no longer fits in my subsystem tree."
> FWIW, we asked Pawel to rework the sysreg and config parts of the
> vexpress driver, make it an actual MFD driver, and spread the remaining
> bits of the code into their respective subsystems. I don't think
> this is an eccentric requirement.

I agree.  However the code that Lorenzo just posted can't be deprived 
of any more sysreg/config parts.  They are simply nonexistent.

Even the larger code you reviewed before is completely useless without 
_additional_ drivers to go on top which are indeed waiting after this 
code to be merged before they are submitted to their respective 
subsystems.

And those additional drivers are way more interesting than this dumb 
register access arbitrator.  Because this is fundamentally the only 
thing it does.

> > In fact what we'd need at this point is 
> > drivers/code_that_no_subsystem_maintainers_wants/.  
> Which is what some people think drivers/mfd/ is...

Does mfd still stand for "Multi Function Device"?

> I don't mind merging Lorenzo's SPC driver as it is if he can explain to
> me how it will eventually evolve into an actual MFD driver. If that's
> not the case, I don't see how I could justify merging it through the
> MFD tree.

Maybe the misunderstanding is about what actually is a MFD driver.
Given your persisting reluctance, I may only conclude that this is 
indeed not a MFD driver after all.

So I'll follow existing precedents in the kernel and move Lorenzo's code 
to drivers/platform/vexpress/.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Samuel Ortiz wrote:

> On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> > At this point I don't really care about the name.  I just want the damn 
> > thing merged upstream.  But after several iterations to either fit one 
> > or another maintainers taste, each rework ends up in that maintainer 
> > saying: "Now that you've reworked the code, I still don't like it since 
> > this no longer fits in my subsystem tree."
> FWIW, we asked Pawel to rework the sysreg and config parts of the
> vexpress driver, make it an actual MFD driver, and spread the remaining
> bits of the code into their respective subsystems. I don't think
> this is an eccentric requirement.

I agree.  However the code that Lorenzo just posted can't be deprived 
of any more sysreg/config parts.  They are simply nonexistent.

Even the larger code you reviewed before is completely useless without 
_additional_ drivers to go on top which are indeed waiting after this 
code to be merged before they are submitted to their respective 
subsystems.

And those additional drivers are way more interesting than this dumb 
register access arbitrator.  Because this is fundamentally the only 
thing it does.

> > In fact what we'd need at this point is 
> > drivers/code_that_no_subsystem_maintainers_wants/.  
> Which is what some people think drivers/mfd/ is...

Does mfd still stand for "Multi Function Device"?

> I don't mind merging Lorenzo's SPC driver as it is if he can explain to
> me how it will eventually evolve into an actual MFD driver. If that's
> not the case, I don't see how I could justify merging it through the
> MFD tree.

Maybe the misunderstanding is about what actually is a MFD driver.
Given your persisting reluctance, I may only conclude that this is 
indeed not a MFD driver after all.

So I'll follow existing precedents in the kernel and move Lorenzo's code 
to drivers/platform/vexpress/.


Nicolas

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Nicolas Pitre]

On Wed, 17 Jul 2013, Samuel Ortiz wrote:

> On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> > At this point I don't really care about the name.  I just want the damn 
> > thing merged upstream.  But after several iterations to either fit one 
> > or another maintainers taste, each rework ends up in that maintainer 
> > saying: "Now that you've reworked the code, I still don't like it since 
> > this no longer fits in my subsystem tree."
> FWIW, we asked Pawel to rework the sysreg and config parts of the
> vexpress driver, make it an actual MFD driver, and spread the remaining
> bits of the code into their respective subsystems. I don't think
> this is an eccentric requirement.

I agree.  However the code that Lorenzo just posted can't be deprived 
of any more sysreg/config parts.  They are simply nonexistent.

Even the larger code you reviewed before is completely useless without 
_additional_ drivers to go on top which are indeed waiting after this 
code to be merged before they are submitted to their respective 
subsystems.

And those additional drivers are way more interesting than this dumb 
register access arbitrator.  Because this is fundamentally the only 
thing it does.

> > In fact what we'd need at this point is 
> > drivers/code_that_no_subsystem_maintainers_wants/.  
> Which is what some people think drivers/mfd/ is...

Does mfd still stand for "Multi Function Device"?

> I don't mind merging Lorenzo's SPC driver as it is if he can explain to
> me how it will eventually evolve into an actual MFD driver. If that's
> not the case, I don't see how I could justify merging it through the
> MFD tree.

Maybe the misunderstanding is about what actually is a MFD driver.
Given your persisting reluctance, I may only conclude that this is 
indeed not a MFD driver after all.

So I'll follow existing precedents in the kernel and move Lorenzo's code 
to drivers/platform/vexpress/.


Nicolas

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

On Wed, Jul 17, 2013 at 06:22:46PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Samuel Ortiz wrote:
> 
> > On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> > > At this point I don't really care about the name.  I just want the damn 
> > > thing merged upstream.  But after several iterations to either fit one 
> > > or another maintainers taste, each rework ends up in that maintainer 
> > > saying: "Now that you've reworked the code, I still don't like it since 
> > > this no longer fits in my subsystem tree."
> > FWIW, we asked Pawel to rework the sysreg and config parts of the
> > vexpress driver, make it an actual MFD driver, and spread the remaining
> > bits of the code into their respective subsystems. I don't think
> > this is an eccentric requirement.
> 
> I agree.  However the code that Lorenzo just posted can't be deprived 
> of any more sysreg/config parts.  
Yes, and I appreciate Lorenzo's effort here.


> Even the larger code you reviewed before is completely useless without 
> _additional_ drivers to go on top which are indeed waiting after this 
> code to be merged before they are submitted to their respective 
> subsystems.
Right. And merging this code in the right place is exactly what we're
doing here.

 
> And those additional drivers are way more interesting than this dumb 
> register access arbitrator.  
Yes, they're a lot smarter hence the requirement to have them live into
their respective subsystems where the right maintainer can provide
relevant reviews on them.

> > I don't mind merging Lorenzo's SPC driver as it is if he can explain to
> > me how it will eventually evolve into an actual MFD driver. If that's
> > not the case, I don't see how I could justify merging it through the
> > MFD tree.
> 
> Maybe the misunderstanding is about what actually is a MFD driver.
That's possible. I agree it should be documented properly.

> So I'll follow existing precedents in the kernel and move Lorenzo's code 
> to drivers/platform/vexpress/.
Thanks for that.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

On Wed, Jul 17, 2013 at 06:22:46PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Samuel Ortiz wrote:
> 
> > On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> > > At this point I don't really care about the name.  I just want the damn 
> > > thing merged upstream.  But after several iterations to either fit one 
> > > or another maintainers taste, each rework ends up in that maintainer 
> > > saying: "Now that you've reworked the code, I still don't like it since 
> > > this no longer fits in my subsystem tree."
> > FWIW, we asked Pawel to rework the sysreg and config parts of the
> > vexpress driver, make it an actual MFD driver, and spread the remaining
> > bits of the code into their respective subsystems. I don't think
> > this is an eccentric requirement.
> 
> I agree.  However the code that Lorenzo just posted can't be deprived 
> of any more sysreg/config parts.  
Yes, and I appreciate Lorenzo's effort here.


> Even the larger code you reviewed before is completely useless without 
> _additional_ drivers to go on top which are indeed waiting after this 
> code to be merged before they are submitted to their respective 
> subsystems.
Right. And merging this code in the right place is exactly what we're
doing here.

 
> And those additional drivers are way more interesting than this dumb 
> register access arbitrator.  
Yes, they're a lot smarter hence the requirement to have them live into
their respective subsystems where the right maintainer can provide
relevant reviews on them.

> > I don't mind merging Lorenzo's SPC driver as it is if he can explain to
> > me how it will eventually evolve into an actual MFD driver. If that's
> > not the case, I don't see how I could justify merging it through the
> > MFD tree.
> 
> Maybe the misunderstanding is about what actually is a MFD driver.
That's possible. I agree it should be documented properly.

> So I'll follow existing precedents in the kernel and move Lorenzo's code 
> to drivers/platform/vexpress/.
Thanks for that.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Samuel Ortiz]

On Wed, Jul 17, 2013 at 06:22:46PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Jul 2013, Samuel Ortiz wrote:
> 
> > On Wed, Jul 17, 2013 at 02:29:02PM -0400, Nicolas Pitre wrote:
> > > At this point I don't really care about the name.  I just want the damn 
> > > thing merged upstream.  But after several iterations to either fit one 
> > > or another maintainers taste, each rework ends up in that maintainer 
> > > saying: "Now that you've reworked the code, I still don't like it since 
> > > this no longer fits in my subsystem tree."
> > FWIW, we asked Pawel to rework the sysreg and config parts of the
> > vexpress driver, make it an actual MFD driver, and spread the remaining
> > bits of the code into their respective subsystems. I don't think
> > this is an eccentric requirement.
> 
> I agree.  However the code that Lorenzo just posted can't be deprived 
> of any more sysreg/config parts.  
Yes, and I appreciate Lorenzo's effort here.


> Even the larger code you reviewed before is completely useless without 
> _additional_ drivers to go on top which are indeed waiting after this 
> code to be merged before they are submitted to their respective 
> subsystems.
Right. And merging this code in the right place is exactly what we're
doing here.

 
> And those additional drivers are way more interesting than this dumb 
> register access arbitrator.  
Yes, they're a lot smarter hence the requirement to have them live into
their respective subsystems where the right maintainer can provide
relevant reviews on them.

> > I don't mind merging Lorenzo's SPC driver as it is if he can explain to
> > me how it will eventually evolve into an actual MFD driver. If that's
> > not the case, I don't see how I could justify merging it through the
> > MFD tree.
> 
> Maybe the misunderstanding is about what actually is a MFD driver.
That's possible. I agree it should be documented properly.

> So I'll follow existing precedents in the kernel and move Lorenzo's code 
> to drivers/platform/vexpress/.
Thanks for that.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

Hi Samuel,

On Wed, Jul 17, 2013 at 10:07:00PM +0100, Samuel Ortiz wrote:
> Hi Lorenzo,
> 
> On Tue, Jul 16, 2013 at 05:05:42PM +0100, Lorenzo Pieralisi wrote:
> > Hello,
> > 
> > version v5 of VExpress SPC driver, please read on the changelog for major
> > changes and explanations.
> > 
> > The probing scheme is unchanged, since after trying the early platform
> > devices approach it appeared that the end result was no better than the
> > current one. The only clean solution relies either on changing how
> > secondaries are brought up in the kernel (later than now) or enable
> > early platform device registration through DT. Please check this
> > thread for the related discussion:
> > 
> > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> > 
> > The interface was adapted to regmap and again reverted to old driver for
> > the following reasons:
> > 
> > - Power down registers locking is hairy and requires arch spinlocks in
> >   the MCPM back end to work properly, normal spinlocks cannot be used
> > - Regmap adds unnecessary code to manage SPC since it is just a bunch of
> >   registers used to control power management flags, the overhead is just
> >   not worth it (talking about power down registers, not the vexpress config
> >   interface)
> > - The locking scheme behind regmap requires all registers in the map
> >   to be protected with the same lock, which is not exactly what we want
> >   here
> > - Given the reasons above, adding a regmap interface buys us nothing from
> >   a driver readability and maintainability perspective (again just talking
> >   about the power interface, a few registers) because for the SPC it would
> >   simply not be used
> > 
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> Could you please elaborate on how will the SPC driver extend into an MFD
> driver?

Reading through the thread I noticed Nico explained details properly, I
was about to mention a possible solution to the directory issue but I am
pretty sure that what he did will turn out for the best.

Usually, or better, historically, these pieces of code that program
PMICs lived in arch/arm/mach-* directories and that's something we could
have done as well (create a static mapping and write some functions to
peek and poke a few registers), but we thought that it was not the proper
way to go.

On top of that, the SPC is part of a component whose register space maps
disparate functions (config interface for voltage, clocks, energy probes,
frequency scaling and power states management) and basically that's the
reason we struggled to partition it properly (with further complexity
implied by the way requests - config and frequency scaling - have to be
serialized).

I hope the end result is reasonable, and overall I think it was a debate
that was worth having.

Thank you,
Lorenzo

Re: [RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

Hi Samuel,

On Wed, Jul 17, 2013 at 10:07:00PM +0100, Samuel Ortiz wrote:
> Hi Lorenzo,
> 
> On Tue, Jul 16, 2013 at 05:05:42PM +0100, Lorenzo Pieralisi wrote:
> > Hello,
> > 
> > version v5 of VExpress SPC driver, please read on the changelog for major
> > changes and explanations.
> > 
> > The probing scheme is unchanged, since after trying the early platform
> > devices approach it appeared that the end result was no better than the
> > current one. The only clean solution relies either on changing how
> > secondaries are brought up in the kernel (later than now) or enable
> > early platform device registration through DT. Please check this
> > thread for the related discussion:
> > 
> > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> > 
> > The interface was adapted to regmap and again reverted to old driver for
> > the following reasons:
> > 
> > - Power down registers locking is hairy and requires arch spinlocks in
> >   the MCPM back end to work properly, normal spinlocks cannot be used
> > - Regmap adds unnecessary code to manage SPC since it is just a bunch of
> >   registers used to control power management flags, the overhead is just
> >   not worth it (talking about power down registers, not the vexpress config
> >   interface)
> > - The locking scheme behind regmap requires all registers in the map
> >   to be protected with the same lock, which is not exactly what we want
> >   here
> > - Given the reasons above, adding a regmap interface buys us nothing from
> >   a driver readability and maintainability perspective (again just talking
> >   about the power interface, a few registers) because for the SPC it would
> >   simply not be used
> > 
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> Could you please elaborate on how will the SPC driver extend into an MFD
> driver?

Reading through the thread I noticed Nico explained details properly, I
was about to mention a possible solution to the directory issue but I am
pretty sure that what he did will turn out for the best.

Usually, or better, historically, these pieces of code that program
PMICs lived in arch/arm/mach-* directories and that's something we could
have done as well (create a static mapping and write some functions to
peek and poke a few registers), but we thought that it was not the proper
way to go.

On top of that, the SPC is part of a component whose register space maps
disparate functions (config interface for voltage, clocks, energy probes,
frequency scaling and power states management) and basically that's the
reason we struggled to partition it properly (with further complexity
implied by the way requests - config and frequency scaling - have to be
serialized).

I hope the end result is reasonable, and overall I think it was a debate
that was worth having.

Thank you,
Lorenzo

[RFC PATCH v5 0/1] drivers: mfd: Versatile Express SPC support

[Lorenzo Pieralisi]

Hi Samuel,

On Wed, Jul 17, 2013 at 10:07:00PM +0100, Samuel Ortiz wrote:
> Hi Lorenzo,
> 
> On Tue, Jul 16, 2013 at 05:05:42PM +0100, Lorenzo Pieralisi wrote:
> > Hello,
> > 
> > version v5 of VExpress SPC driver, please read on the changelog for major
> > changes and explanations.
> > 
> > The probing scheme is unchanged, since after trying the early platform
> > devices approach it appeared that the end result was no better than the
> > current one. The only clean solution relies either on changing how
> > secondaries are brought up in the kernel (later than now) or enable
> > early platform device registration through DT. Please check this
> > thread for the related discussion:
> > 
> > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036542.html
> > 
> > The interface was adapted to regmap and again reverted to old driver for
> > the following reasons:
> > 
> > - Power down registers locking is hairy and requires arch spinlocks in
> >   the MCPM back end to work properly, normal spinlocks cannot be used
> > - Regmap adds unnecessary code to manage SPC since it is just a bunch of
> >   registers used to control power management flags, the overhead is just
> >   not worth it (talking about power down registers, not the vexpress config
> >   interface)
> > - The locking scheme behind regmap requires all registers in the map
> >   to be protected with the same lock, which is not exactly what we want
> >   here
> > - Given the reasons above, adding a regmap interface buys us nothing from
> >   a driver readability and maintainability perspective (again just talking
> >   about the power interface, a few registers) because for the SPC it would
> >   simply not be used
> > 
> > /drivers/mfd is probably not the right place for this code as it stands (but
> > probably will be when the entire driver, with DVFS and config interface, is
> > complete).
> Could you please elaborate on how will the SPC driver extend into an MFD
> driver?

Reading through the thread I noticed Nico explained details properly, I
was about to mention a possible solution to the directory issue but I am
pretty sure that what he did will turn out for the best.

Usually, or better, historically, these pieces of code that program
PMICs lived in arch/arm/mach-* directories and that's something we could
have done as well (create a static mapping and write some functions to
peek and poke a few registers), but we thought that it was not the proper
way to go.

On top of that, the SPC is part of a component whose register space maps
disparate functions (config interface for voltage, clocks, energy probes,
frequency scaling and power states management) and basically that's the
reason we struggled to partition it properly (with further complexity
implied by the way requests - config and frequency scaling - have to be
serialized).

I hope the end result is reasonable, and overall I think it was a debate
that was worth having.

Thank you,
Lorenzo