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

2013-06-05 Thread Lorenzo Pieralisi 
On Wed, Jun 05, 2013 at 07:08:33PM +0100, Jon Medhurst (Tixy) wrote:
> On Wed, 2013-06-05 at 12:46 +0100, Lorenzo Pieralisi wrote:
> [...]
> > +static const struct of_device_id vexpress_spc_ids[] __initconst = {
> > +   { .compatible = "arm,vexpress-spc,v2p-ca15_a7" },
> > +   { .compatible = "arm,vexpress-spc" },
> > +   {},
> > +};
> > +
> > +static int __init vexpress_spc_init(void)
> > +{
> > +   int ret;
> > +   struct device_node *node = of_find_matching_node(NULL,
> > +vexpress_spc_ids);
> 
> To allow for devices without an SPC we should check for !node here and
> bail out, otherwise we get an ugly message from the WARN_ON further
> down. I see this on RTSM, and multiplatform kernels would suffer this as
> well.
> 
> Even if the ugly warning wasn't there, it still seems cleaner to me to
> have a proper check for an absent spc node.

Absolutely, I will apply both fixes, thanks a lot for the review.

Lorenzo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread Jon Medhurst (Tixy) 
On Wed, 2013-06-05 at 12:46 +0100, Lorenzo Pieralisi wrote:
[...]
> +static const struct of_device_id vexpress_spc_ids[] __initconst = {
> + { .compatible = "arm,vexpress-spc,v2p-ca15_a7" },
> + { .compatible = "arm,vexpress-spc" },
> + {},
> +};
> +
> +static int __init vexpress_spc_init(void)
> +{
> + int ret;
> + struct device_node *node = of_find_matching_node(NULL,
> +  vexpress_spc_ids);

To allow for devices without an SPC we should check for !node here and
bail out, otherwise we get an ugly message from the WARN_ON further
down. I see this on RTSM, and multiplatform kernels would suffer this as
well.

Even if the ugly warning wasn't there, it still seems cleaner to me to
have a proper check for an absent spc node.

> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info) {
> + pr_err("%s: unable to allocate mem\n", __func__);
> + return -ENOMEM;
> + }
> + info->cur_req_type = INVALID_TYPE;
> +
> + info->baseaddr = of_iomap(node, 0);
> + if (WARN_ON(!info->baseaddr)) {
> + ret = -ENXIO;
> + goto mem_free;
> + }
> +
> + info->irq = irq_of_parse_and_map(node, 0);
> +
> + if (WARN_ON(!info->irq)) {
> + ret = -ENXIO;
> + goto unmap;
> + }
> +
> + readl_relaxed(info->baseaddr + PWC_STATUS);
> +
> + ret = request_irq(info->irq, vexpress_spc_irq_handler,
> + IRQF_DISABLED | IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "arm-spc", info);
> +
> + if (ret) {
> + pr_err("IRQ %d request failed\n", info->irq);
> + ret = -ENODEV;
> + goto unmap;
> + }
> +
> + info->a15_clusid = readl_relaxed(info->baseaddr + A15_CONF) & 0xf;
> +
> + vexpress_spc_config_bridge = vexpress_config_bridge_register(
> + node, _spc_config_bridge_info);
> +
> + if (WARN_ON(!vexpress_spc_config_bridge)) {
> + ret = -ENODEV;
> + goto unmap;
> + }
> +
> + opp_func = vexpress_config_func_get(vexpress_spc_config_bridge, "opp");
> + perf_func =
> + vexpress_config_func_get(vexpress_spc_config_bridge, "perf");
> +
> + if (!opp_func || !perf_func) {
> + ret = -ENODEV;
> + goto unmap;
> + }
> +
> + if (vexpress_spc_populate_opps(0) || vexpress_spc_populate_opps(1)) {
> + if (info->irq)
> + free_irq(info->irq, info);
> + pr_err("failed to build OPP table\n");
> + ret = -ENODEV;
> + goto unmap;
> + }
> + /*
> +  * Multi-cluster systems may need this data when non-coherent, during
> +  * cluster power-up/power-down. Make sure it reaches main memory:
> +  */
> + sync_cache_w(info);
> + sync_cache_w();
> + pr_info("vexpress-spc loaded at %p\n", info->baseaddr);
> + return 0;
> +
> +unmap:
> + iounmap(info->baseaddr);
> +
> +mem_free:
> + kfree(info);
> + return ret;
> +}
> +
> +static bool __init __vexpress_spc_check_loaded(void);
> +static bool (*spc_check_loaded)(void) = &__vexpress_spc_check_loaded;
> +
> +static bool __init __vexpress_spc_check_loaded(void)
> +{
> + if (vexpress_spc_load_result == -EAGAIN)
> + vexpress_spc_load_result = vexpress_spc_init();
> + spc_check_loaded = _spc_initialized;
> + return vexpress_spc_initialized();
> +}
> +
> +/*
> + * Function exported to manage early_initcall ordering.
> + * SPC code is needed very early in the boot process
> + * to bring CPUs out of reset and initialize power
> + * management back-end. After boot swap pointers to
> + * make the functionality check available to loadable
> + * modules, when early boot init functions have been
> + * already freed from kernel address space.
> + */
> +bool vexpress_spc_check_loaded(void)
> +{
> + return spc_check_loaded();
> +}
> +EXPORT_SYMBOL_GPL(vexpress_spc_check_loaded);
> +
> +static int __init vexpress_spc_early_init(void)
> +{
> + __vexpress_spc_check_loaded();
> + return vexpress_spc_load_result;
> +}
> +early_initcall(vexpress_spc_early_init);
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Serial Power Controller (SPC) support");
[...]

-- 
Tixy


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread Jon Medhurst (Tixy) 
On Wed, 2013-06-05 at 12:46 +0100, Lorenzo Pieralisi wrote:
[...]
> +
> +static bool __init __vexpress_spc_check_loaded(void);
> +static bool (*spc_check_loaded)(void) = &__vexpress_spc_check_loaded;

We get a section mismatch warning from the above because
__vexpress_spc_check_loaded is marked __init.

Now, we know the code is safe because we replace the reference in the
early_initcall below, before the init section is discarded (which is the
whole point of the construct) so we can add __refdata to the declaration
of spc_check_loaded to silence the warning.

Other than that I've not noticed any issues when testing the patches.

> +
> +static bool __init __vexpress_spc_check_loaded(void)
> +{
> + if (vexpress_spc_load_result == -EAGAIN)
> + vexpress_spc_load_result = vexpress_spc_init();
> + spc_check_loaded = _spc_initialized;
> + return vexpress_spc_initialized();
> +}
> +
> +/*
> + * Function exported to manage early_initcall ordering.
> + * SPC code is needed very early in the boot process
> + * to bring CPUs out of reset and initialize power
> + * management back-end. After boot swap pointers to
> + * make the functionality check available to loadable
> + * modules, when early boot init functions have been
> + * already freed from kernel address space.
> + */
> +bool vexpress_spc_check_loaded(void)
> +{
> + return spc_check_loaded();
> +}
> +EXPORT_SYMBOL_GPL(vexpress_spc_check_loaded);
> +
> +static int __init vexpress_spc_early_init(void)
> +{
> + __vexpress_spc_check_loaded();
> + return vexpress_spc_load_result;
> +}
> +early_initcall(vexpress_spc_early_init);
[...]

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread Nicolas Pitre 
On Wed, 5 Jun 2013, 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, operating points and reset control.
> 
> This patch provides a driver that enables run-time control of features
> implemented by the SPC control logic.
> 
> The driver also provides a bridge interface through the vexpress config
> infrastructure. Operations allowing to read/write operating points are
> made to go via the same interface as configuration transactions so that
> all requests to M3 are serialized.
> 
> Device tree bindings documentation for the SPC component is provided with
> the patchset.
> 

[...]

> +/*
> + * Versatile Express Serial Power Controller (SPC) support
> + *
> + * Copyright (C) 2013 ARM Ltd.
> + *
> + * Author(s): Sudeep KarkadaNagesha 
> + *Achin Gupta   
> + *Lorenzo Pieralisi 

I imagine "author(s)" can be written as "authors" without a doubt.  :-)

> +EXPORT_SYMBOL_GPL(vexpress_spc_write_resume_reg);
> +EXPORT_SYMBOL_GPL(vexpress_spc_set_global_wakeup_intr);
> +EXPORT_SYMBOL_GPL(vexpress_spc_set_cpu_wakeup_irq);
> +EXPORT_SYMBOL_GPL(vexpress_spc_powerdown_enable);

I don't think anything that could possibly be made into modules should 
ever have a need for those particular calls.  So I'd suggest not 
exporting them.

Other than that, you may add...

Reviewed-by: Nicolas Pitre 


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread 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, operating points and reset control.

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

The driver also provides a bridge interface through the vexpress config
infrastructure. Operations allowing to read/write operating points are
made to go via the same interface as configuration transactions so that
all requests to M3 are serialized.

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

Cc: Samuel Ortiz 
Cc: Pawel Moll 
Cc: Nicolas Pitre 
Cc: Amit Kucheria 
Cc: Jon Medhurst 
Signed-off-by: Achin Gupta 
Signed-off-by: Lorenzo Pieralisi 
Signed-off-by: Sudeep KarkadaNagesha 
---
 .../devicetree/bindings/mfd/vexpress-spc.txt   |  35 ++
 drivers/mfd/Kconfig|   7 +
 drivers/mfd/Makefile   |   1 +
 drivers/mfd/vexpress-spc.c | 630 +
 include/linux/vexpress.h   |  43 ++
 5 files changed, 716 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/vexpress-spc.txt
 create mode 100644 drivers/mfd/vexpress-spc.c

diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt 
b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
new file mode 100644
index 000..1d71dc2
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
@@ -0,0 +1,35 @@
+* 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/voltage and
+operating point transitions, through memory mapped registers interface.
+
+On testchips like TC2 it also provides a configuration interface that can
+be used to read/write values which cannot be read/written through simple
+memory mapped reads/writes.
+
+- spc node
+
+   - compatible:
+   Usage: required
+   Value type: 
+   Definition: must be
+   "arm,vexpress-spc,v2p-ca15_a7","arm,vexpress-spc"
+   - reg:
+   Usage: required
+   Value type: 
+   Definition: A standard property that specifies the base address
+   and the size of the SPC address space
+   - interrupts:
+   Usage: required
+   Value type: 
+   Definition:  SPC interrupt configuration. A standard property
+that follows ePAPR interrupts specifications
+
+Example:
+
+spc: spc@7fff {
+   compatible = "arm,vexpress-spc,v2p-ca15_a7","arm,vexpress-spc";
+   reg = <0 0x7FFF 0 0x1000>;
+   interrupts = <0 95 4>;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d54e985..391eda1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1148,3 +1148,10 @@ 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
+   depends on VEXPRESS_CONFIG
+   help
+ Serial Power Controller driver for ARM Ltd. test chips.
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 000..1aaa673
--- /dev/null
+++ b/drivers/mfd/vexpress-spc.c
@@ -0,0 +1,630 @@
+/*
+ * Versatile Express Serial Power Controller (SPC) support
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ *
+ * Author(s): Sudeep KarkadaNagesha 
+ *Achin Gupta   
+ *Lorenzo Pieralisi 
+ *
+ * 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 
+#include

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

2013-06-05 Thread 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, operating points and reset control.

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

The driver also provides a bridge interface through the vexpress config
infrastructure. Operations allowing to read/write operating points are
made to go via the same interface as configuration transactions so that
all requests to M3 are serialized.

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

Cc: Samuel Ortiz sa...@linux.intel.com
Cc: Pawel Moll pawel.m...@arm.com
Cc: Nicolas Pitre nicolas.pi...@linaro.org
Cc: Amit Kucheria amit.kuche...@linaro.org
Cc: Jon Medhurst t...@linaro.org
Signed-off-by: Achin Gupta achin.gu...@arm.com
Signed-off-by: Lorenzo Pieralisi lorenzo.pieral...@arm.com
Signed-off-by: Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com
---
 .../devicetree/bindings/mfd/vexpress-spc.txt   |  35 ++
 drivers/mfd/Kconfig|   7 +
 drivers/mfd/Makefile   |   1 +
 drivers/mfd/vexpress-spc.c | 630 +
 include/linux/vexpress.h   |  43 ++
 5 files changed, 716 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/vexpress-spc.txt
 create mode 100644 drivers/mfd/vexpress-spc.c

diff --git a/Documentation/devicetree/bindings/mfd/vexpress-spc.txt 
b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
new file mode 100644
index 000..1d71dc2
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/vexpress-spc.txt
@@ -0,0 +1,35 @@
+* 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/voltage and
+operating point transitions, through memory mapped registers interface.
+
+On testchips like TC2 it also provides a configuration interface that can
+be used to read/write values which cannot be read/written through simple
+memory mapped reads/writes.
+
+- 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@7fff {
+   compatible = arm,vexpress-spc,v2p-ca15_a7,arm,vexpress-spc;
+   reg = 0 0x7FFF 0 0x1000;
+   interrupts = 0 95 4;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d54e985..391eda1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1148,3 +1148,10 @@ 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
+   depends on VEXPRESS_CONFIG
+   help
+ Serial Power Controller driver for ARM Ltd. test chips.
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 000..1aaa673
--- /dev/null
+++ b/drivers/mfd/vexpress-spc.c
@@ -0,0 +1,630 @@
+/*
+ * Versatile Express Serial Power Controller (SPC) support
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ *
+ * Author(s): Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com
+ *Achin Gupta   achin.gu...@arm.com
+ *Lorenzo Pieralisi lorenzo.pieral...@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

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

2013-06-05 Thread Nicolas Pitre 
On Wed, 5 Jun 2013, 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, operating points and reset control.
 
 This patch provides a driver that enables run-time control of features
 implemented by the SPC control logic.
 
 The driver also provides a bridge interface through the vexpress config
 infrastructure. Operations allowing to read/write operating points are
 made to go via the same interface as configuration transactions so that
 all requests to M3 are serialized.
 
 Device tree bindings documentation for the SPC component is provided with
 the patchset.
 

[...]

 +/*
 + * Versatile Express Serial Power Controller (SPC) support
 + *
 + * Copyright (C) 2013 ARM Ltd.
 + *
 + * Author(s): Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com
 + *Achin Gupta   achin.gu...@arm.com
 + *Lorenzo Pieralisi lorenzo.pieral...@arm.com

I imagine author(s) can be written as authors without a doubt.  :-)

 +EXPORT_SYMBOL_GPL(vexpress_spc_write_resume_reg);
 +EXPORT_SYMBOL_GPL(vexpress_spc_set_global_wakeup_intr);
 +EXPORT_SYMBOL_GPL(vexpress_spc_set_cpu_wakeup_irq);
 +EXPORT_SYMBOL_GPL(vexpress_spc_powerdown_enable);

I don't think anything that could possibly be made into modules should 
ever have a need for those particular calls.  So I'd suggest not 
exporting them.

Other than that, you may add...

Reviewed-by: Nicolas Pitre n...@linaro.org


Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread Jon Medhurst (Tixy) 
On Wed, 2013-06-05 at 12:46 +0100, Lorenzo Pieralisi wrote:
[...]
 +
 +static bool __init __vexpress_spc_check_loaded(void);
 +static bool (*spc_check_loaded)(void) = __vexpress_spc_check_loaded;

We get a section mismatch warning from the above because
__vexpress_spc_check_loaded is marked __init.

Now, we know the code is safe because we replace the reference in the
early_initcall below, before the init section is discarded (which is the
whole point of the construct) so we can add __refdata to the declaration
of spc_check_loaded to silence the warning.

Other than that I've not noticed any issues when testing the patches.

 +
 +static bool __init __vexpress_spc_check_loaded(void)
 +{
 + if (vexpress_spc_load_result == -EAGAIN)
 + vexpress_spc_load_result = vexpress_spc_init();
 + spc_check_loaded = vexpress_spc_initialized;
 + return vexpress_spc_initialized();
 +}
 +
 +/*
 + * Function exported to manage early_initcall ordering.
 + * SPC code is needed very early in the boot process
 + * to bring CPUs out of reset and initialize power
 + * management back-end. After boot swap pointers to
 + * make the functionality check available to loadable
 + * modules, when early boot init functions have been
 + * already freed from kernel address space.
 + */
 +bool vexpress_spc_check_loaded(void)
 +{
 + return spc_check_loaded();
 +}
 +EXPORT_SYMBOL_GPL(vexpress_spc_check_loaded);
 +
 +static int __init vexpress_spc_early_init(void)
 +{
 + __vexpress_spc_check_loaded();
 + return vexpress_spc_load_result;
 +}
 +early_initcall(vexpress_spc_early_init);
[...]

-- 
Tixy

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread Jon Medhurst (Tixy) 
On Wed, 2013-06-05 at 12:46 +0100, Lorenzo Pieralisi wrote:
[...]
 +static const struct of_device_id vexpress_spc_ids[] __initconst = {
 + { .compatible = arm,vexpress-spc,v2p-ca15_a7 },
 + { .compatible = arm,vexpress-spc },
 + {},
 +};
 +
 +static int __init vexpress_spc_init(void)
 +{
 + int ret;
 + struct device_node *node = of_find_matching_node(NULL,
 +  vexpress_spc_ids);

To allow for devices without an SPC we should check for !node here and
bail out, otherwise we get an ugly message from the WARN_ON further
down. I see this on RTSM, and multiplatform kernels would suffer this as
well.

Even if the ugly warning wasn't there, it still seems cleaner to me to
have a proper check for an absent spc node.

 + info = kzalloc(sizeof(*info), GFP_KERNEL);
 + if (!info) {
 + pr_err(%s: unable to allocate mem\n, __func__);
 + return -ENOMEM;
 + }
 + info-cur_req_type = INVALID_TYPE;
 +
 + info-baseaddr = of_iomap(node, 0);
 + if (WARN_ON(!info-baseaddr)) {
 + ret = -ENXIO;
 + goto mem_free;
 + }
 +
 + info-irq = irq_of_parse_and_map(node, 0);
 +
 + if (WARN_ON(!info-irq)) {
 + ret = -ENXIO;
 + goto unmap;
 + }
 +
 + readl_relaxed(info-baseaddr + PWC_STATUS);
 +
 + ret = request_irq(info-irq, vexpress_spc_irq_handler,
 + IRQF_DISABLED | IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
 + arm-spc, info);
 +
 + if (ret) {
 + pr_err(IRQ %d request failed\n, info-irq);
 + ret = -ENODEV;
 + goto unmap;
 + }
 +
 + info-a15_clusid = readl_relaxed(info-baseaddr + A15_CONF)  0xf;
 +
 + vexpress_spc_config_bridge = vexpress_config_bridge_register(
 + node, vexpress_spc_config_bridge_info);
 +
 + if (WARN_ON(!vexpress_spc_config_bridge)) {
 + ret = -ENODEV;
 + goto unmap;
 + }
 +
 + opp_func = vexpress_config_func_get(vexpress_spc_config_bridge, opp);
 + perf_func =
 + vexpress_config_func_get(vexpress_spc_config_bridge, perf);
 +
 + if (!opp_func || !perf_func) {
 + ret = -ENODEV;
 + goto unmap;
 + }
 +
 + if (vexpress_spc_populate_opps(0) || vexpress_spc_populate_opps(1)) {
 + if (info-irq)
 + free_irq(info-irq, info);
 + pr_err(failed to build OPP table\n);
 + ret = -ENODEV;
 + goto unmap;
 + }
 + /*
 +  * Multi-cluster systems may need this data when non-coherent, during
 +  * cluster power-up/power-down. Make sure it reaches main memory:
 +  */
 + sync_cache_w(info);
 + sync_cache_w(info);
 + pr_info(vexpress-spc loaded at %p\n, info-baseaddr);
 + return 0;
 +
 +unmap:
 + iounmap(info-baseaddr);
 +
 +mem_free:
 + kfree(info);
 + return ret;
 +}
 +
 +static bool __init __vexpress_spc_check_loaded(void);
 +static bool (*spc_check_loaded)(void) = __vexpress_spc_check_loaded;
 +
 +static bool __init __vexpress_spc_check_loaded(void)
 +{
 + if (vexpress_spc_load_result == -EAGAIN)
 + vexpress_spc_load_result = vexpress_spc_init();
 + spc_check_loaded = vexpress_spc_initialized;
 + return vexpress_spc_initialized();
 +}
 +
 +/*
 + * Function exported to manage early_initcall ordering.
 + * SPC code is needed very early in the boot process
 + * to bring CPUs out of reset and initialize power
 + * management back-end. After boot swap pointers to
 + * make the functionality check available to loadable
 + * modules, when early boot init functions have been
 + * already freed from kernel address space.
 + */
 +bool vexpress_spc_check_loaded(void)
 +{
 + return spc_check_loaded();
 +}
 +EXPORT_SYMBOL_GPL(vexpress_spc_check_loaded);
 +
 +static int __init vexpress_spc_early_init(void)
 +{
 + __vexpress_spc_check_loaded();
 + return vexpress_spc_load_result;
 +}
 +early_initcall(vexpress_spc_early_init);
 +MODULE_LICENSE(GPL);
 +MODULE_DESCRIPTION(Serial Power Controller (SPC) support);
[...]

-- 
Tixy


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-06-05 Thread Lorenzo Pieralisi 
On Wed, Jun 05, 2013 at 07:08:33PM +0100, Jon Medhurst (Tixy) wrote:
 On Wed, 2013-06-05 at 12:46 +0100, Lorenzo Pieralisi wrote:
 [...]
  +static const struct of_device_id vexpress_spc_ids[] __initconst = {
  +   { .compatible = arm,vexpress-spc,v2p-ca15_a7 },
  +   { .compatible = arm,vexpress-spc },
  +   {},
  +};
  +
  +static int __init vexpress_spc_init(void)
  +{
  +   int ret;
  +   struct device_node *node = of_find_matching_node(NULL,
  +vexpress_spc_ids);
 
 To allow for devices without an SPC we should check for !node here and
 bail out, otherwise we get an ugly message from the WARN_ON further
 down. I see this on RTSM, and multiplatform kernels would suffer this as
 well.
 
 Even if the ugly warning wasn't there, it still seems cleaner to me to
 have a proper check for an absent spc node.

Absolutely, I will apply both fixes, thanks a lot for the review.

Lorenzo

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/