Re: [RFC PATCH v2] drivers: bus: add ARM CCI support
On Fri, May 03, 2013 at 04:17:07PM +0100, Javi Merino wrote: Hi Lorenzo, On Wed, May 01, 2013 at 05:18:28PM +0100, Lorenzo Pieralisi wrote: On ARM multi-cluster systems coherency between cores running on different clusters is managed by the cache-coherent interconnect (CCI). It allows broadcasting of TLB invalidates and memory barriers and it guarantees cache coherency at system level through snooping of slave interfaces connected to it. This patch enables the basic infrastructure required in Linux to handle and programme the CCI component. Non-local variables used by the CCI management functions called by power down function calls after disabling the cache must be flushed out to main memory in advance, otherwise incoherency of those values may occur if they are sitting in the cache of some other CPU when power down functions execute. Driver code ensures that relevant data structures are flushed from inner and outer caches after the driver probe is completed. CCI slave port resources are linked to set of CPUs through bus masters phandle properties that link the interface resources to masters node in the device tree. Documentation describing the CCI DT bindings is provided with the patch. Signed-off-by: Lorenzo Pieralisi lorenzo.pieral...@arm.com --- [...] diff --git a/include/linux/arm-cci.h b/include/linux/arm-cci.h new file mode 100644 index 000..0e70942 --- /dev/null +++ b/include/linux/arm-cci.h @@ -0,0 +1,59 @@ +/* + * CCI cache coherent interconnect support + * + * Copyright (C) 2013 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __LINUX_ARM_CCI_H +#define __LINUX_ARM_CCI_H + +#include linux/errno.h +#include linux/types.h + +struct device_node; + +#ifdef CONFIG_ARM_CCI +extern int cci_ace_get_port(struct device_node *dn); +extern int cci_disable_port_by_cpu(u64 mpidr); +extern int __cci_control_port_by_device(struct device_node *dn, bool enable); +extern int __cci_control_port_by_index(u32 port, bool enable); +#else +static inline int cci_ace_get_port(struct device_node *dn, bool cpu) Builds with !CONFIG_ARM_CCI fail because this definition of cci_ace_get_port() takes two arguments whereas the one for CONFIG_ARM_CCI takes only one. Yes, sorry, I missed that while refactoring the interface. Thanks, Lorenzo ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v2] drivers: bus: add ARM CCI support
On Mon, May 06, 2013 at 04:05:28PM +0100, Nicolas Pitre wrote:
Review comments below.
Thanks Nico.
On Wed, 1 May 2013, Lorenzo Pieralisi wrote:
index 000..fb9e8e0
--- /dev/null
+++ b/drivers/bus/arm-cci.c
@@ -0,0 +1,372 @@
+/*
+ * CCI cache coherent interconnect driver
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ * Author: 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 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/arm-cci.h
+#include linux/device.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of_address.h
+#include linux/of_device.h
+#include linux/platform_device.h
+#include linux/slab.h
+
+#include asm/cacheflush.h
+#include asm/dt_affinity.h
+#include asm/outercache.h
You don't need this include anymore.
True.
+#include asm/smp_plat.h
+
+#define DRIVER_NAME CCI
+
+#define CCI_PORT_CTRL0x0
+#define CCI_CTRL_STATUS 0xc
+
+#define CCI_ENABLE_SNOOP_REQ 0x1
+#define CCI_ENABLE_DVM_REQ 0x2
+#define CCI_ENABLE_REQ (CCI_ENABLE_SNOOP_REQ |
CCI_ENABLE_DVM_REQ)
+
+struct cci_nb_ports {
+ unsigned int nb_ace;
+ unsigned int nb_ace_lite;
+};
+
+enum cci_ace_port_type {
+ ACE_INVALID_PORT = 0x0,
+ ACE_PORT,
+ ACE_LITE_PORT,
+};
+
+struct cci_ace_port {
+ void __iomem *base;
+ int type;
You could use: enum cci_ace_port_type type;
Ok.
[...]
+ * Disabling a CCI port for a CPU implies disabling the CCI port
+ * controlling that CPU cluster. Code disabling CPU CCI ports
+ * must make sure that the CPU running the code is the last active CPU
+ * in the cluster ie all other CPUs are quiescent in a low power state.
+ */
+int notrace cci_disable_port_by_cpu(u64 mpidr)
+{
+ int cpu = get_logical_index(mpidr MPIDR_HWID_BITMASK);
This is dangerous. Same reasoning for not using cpu_relax() above
should apply here too. Better avoid any external calls and cache things
locally by marking cpu_port[] entries with MPIDR values directly.
Ok, I will stash them at boot.
Furthermore, get_logical_index() might not be reliable when the cache or
local coherency is off. We'd have to flush all the data it might access
to RAM just like it is done for our very own data structures in this
code, but starting doing that outside of this driver would become rather
ugly.
That's correct.
+ if (cpu 0 || cpu_port[cpu] == INVALID_PORT_IDX)
+ return -ENODEV;
+ cci_port_control(cpu_port[cpu], false);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cci_disable_port_by_cpu);
+
+/*
+ * __cci_control_port_by_device()
+ * @dn = device node pointer of the device whose CCI port should be
+ * controlled
+ * @enable = if true enables the port, if false disables it
+ * Returns:
+ * 0 on success
+ * -ENODEV on port look-up failure
+ */
+int notrace __cci_control_port_by_device(struct device_node *dn, bool
enable)
+{
+ int port;
+
+ if (!dn)
+ return -ENODEV;
+
+ port = __cci_ace_get_port(dn, ACE_LITE_PORT);
+ if (WARN_ONCE(port 0, node %s ACE lite port look-up failure\n,
+ dn-full_name))
+ return -ENODEV;
+ cci_port_control(port, enable);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__cci_control_port_by_device);
+
+/*
+ * __cci_control_port_by_index()
+ * @port = port index previously retrieved with cci_ace_get_port()
+ * @enable = if true enables the port, if false disables it
+ * Returns:
+ * 0 on success
+ * -ENODEV on port index out of range
+ * -EPERM if operation carried out on an ACE PORT
+ */
+int notrace __cci_control_port_by_index(u32 port, bool enable)
+{
+ if (port = nb_cci_ports || ports[port].type == ACE_INVALID_PORT)
+ return -ENODEV;
+ /*
+ * CCI control for ports connected to CPUS is extremely fragile
+ * and must be made to go through a specific and controlled
+ * interface (ie cci_disable_port_by_cpu(); control by general purpose
+ * indexing is therefore disabled for ACE ports.
+ */
+ if (ports[port].type == ACE_PORT)
+ return -EPERM;
+
+ cci_port_control(port, enable);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__cci_control_port_by_index);
+
+static const struct cci_nb_ports cci400_ports = {
+ .nb_ace = 2,
+ .nb_ace_lite = 3
Re: [RFC PATCH v2] drivers: bus: add ARM CCI support
On Tue, 7 May 2013, Lorenzo Pieralisi wrote: On Mon, May 06, 2013 at 04:05:28PM +0100, Nicolas Pitre wrote: When compiled in, module_init() is translated into device_initcall(). This is way too late for bringing up secondary CPUs during boot via the MCPM layer. That is not an issue as far as the code presented here is concerned since there is no integration with MCPM yet, but eventually we'd want the MCPM power_up_setup method to integrate with the port discovery performed here instead of having them hardcoded in the assembly code. This means it would have to become early_initcall() instead. And at that point the driver infrastructure isn't fully operational, meaning that driver_probe() won't be usable either (looking at what we did to the TC2 spc init code is a good example of what I mean here). Yes, I thought about that. This means that CCI driver should not rely on platform device structs, and yes I can mirror SPC probing code to take care of ordering issues. I am quite tempted to remove the platform device/driver infrastructure altogether and just rely on the DT layer (as GIC, PL310 do) to initialize CCI. What do you think ? I think that's probably the best approach. That's what we ended up doing to the spc code for the same reasons. I think I will leave the platform driver infrastructure, and at probe it will check if data structures have already been initialized by an exported function, say: cci_of_init() Please let's try to avoid propagating the of acronym further. We don't use Open Firmware here. Using dt is more appropriate IMHO. if the data structures are already initialized basically the probe function will do precious little and just succeeds otherwise it will initialize the driver as it does now. Is there really a point in having the CCI driver represented as a platform driver ? Not sure at all. I'm not sure either. Nicolas ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v2] drivers: bus: add ARM CCI support
Review comments below.
On Wed, 1 May 2013, Lorenzo Pieralisi wrote:
index 000..fb9e8e0
--- /dev/null
+++ b/drivers/bus/arm-cci.c
@@ -0,0 +1,372 @@
+/*
+ * CCI cache coherent interconnect driver
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ * Author: 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 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/arm-cci.h
+#include linux/device.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of_address.h
+#include linux/of_device.h
+#include linux/platform_device.h
+#include linux/slab.h
+
+#include asm/cacheflush.h
+#include asm/dt_affinity.h
+#include asm/outercache.h
You don't need this include anymore.
+#include asm/smp_plat.h
+
+#define DRIVER_NAME CCI
+
+#define CCI_PORT_CTRL0x0
+#define CCI_CTRL_STATUS 0xc
+
+#define CCI_ENABLE_SNOOP_REQ 0x1
+#define CCI_ENABLE_DVM_REQ 0x2
+#define CCI_ENABLE_REQ (CCI_ENABLE_SNOOP_REQ |
CCI_ENABLE_DVM_REQ)
+
+struct cci_nb_ports {
+ unsigned int nb_ace;
+ unsigned int nb_ace_lite;
+};
+
+enum cci_ace_port_type {
+ ACE_INVALID_PORT = 0x0,
+ ACE_PORT,
+ ACE_LITE_PORT,
+};
+
+struct cci_ace_port {
+ void __iomem *base;
+ int type;
You could use: enum cci_ace_port_type type;
+ struct device_node *dn;
+};
+
+static struct cci_ace_port *ports;
+static unsigned int nb_cci_ports;
+
+static void __iomem *cci_ctrl_base;
+#define INVALID_PORT_IDX -1
+static int cpu_port[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_PORT_IDX };
+
+/*
+ * __cci_ace_get_port - Function to retrieve the port index connected to
+ * a cpu or device.
+ *
+ * @dn = device node of the device to look-up
+ * @type = port type
+ *
+ * Return value:
+ * - CCI port index if success
+ * - -ENODEV if failure
+ */
+static int __cci_ace_get_port(struct device_node *dn, int type)
+{
+ int i;
+ bool ace_match;
+ struct device_node *cci_portn;
+
+ cci_portn = of_parse_phandle(dn, cci-control-port, 0);
+ for (i = 0; i nb_cci_ports; i++) {
+ ace_match = ports[i].type == type;
+ if (ace_match cci_portn == ports[i].dn)
+ return i;
+ }
+ return -ENODEV;
+}
+
+int cci_ace_get_port(struct device_node *dn)
+{
+ return __cci_ace_get_port(dn, ACE_LITE_PORT);
+}
+EXPORT_SYMBOL_GPL(cci_ace_get_port);
+
+static void __init cci_ace_init_ports(void)
+{
+ int port, ac, cpu;
+ u64 hwid;
+ const u32 *cell;
+ struct device_node *cpun, *cpus;
+
+ cpus = of_find_node_by_path(/cpus);
+ if (WARN(!cpus, Missing cpus node, bailing out\n))
+ return;
+
+ if (WARN_ON(of_property_read_u32(cpus, #address-cells, ac)))
+ ac = of_n_addr_cells(cpus);
+ /*
+ * Port index look-up speeds up the function disabling ports by CPU,
+ * since the logical to port index mapping is done once and does
+ * not change after system boot.
+ * The stashed index array is initialized for all possible CPUs
+ * at probe time.
+ */
+ for_each_child_of_node(cpus, cpun) {
+ if (of_node_cmp(cpun-type, cpu))
+ continue;
+ cell = of_get_property(cpun, reg, NULL);
+ if (WARN(!cell, %s: missing reg property\n, cpun-full_name))
+ continue;
+ hwid = of_read_number(cell, ac);
+ cpu = get_logical_index(hwid MPIDR_HWID_BITMASK);
+ if (cpu 0)
+ continue;
+ port = __cci_ace_get_port(cpun, ACE_PORT);
+ if (port 0)
+ continue;
+ cpu_port[cpu] = port;
+ }
+
+ for_each_possible_cpu(cpu) {
+ WARN(cpu_port[cpu] == INVALID_PORT_IDX, CPU %d has an invalid
CCI port index\n,
+ cpu);
+ }
+}
+
+/*
+ * Functions to enable/disable a CCI interconnect slave port
+ *
+ * They are called by low-level power management code to disable slave
+ * interfaces snoops and DVM broadcast.
+ * Since they may execute with cache data allocation disabled and
+ * after the caches have been cleaned and invalidated the functions provide
+ * no explicit locking since they may run with D-cache disabled, so normal
+ * cacheable kernel locks based on ldrex/strex may not work.
+ * Locking has to be provided by BSP implementations to ensure proper
+ * operations.
+ */
+
[RFC PATCH v2] drivers: bus: add ARM CCI support
On ARM multi-cluster systems coherency between cores running on
different clusters is managed by the cache-coherent interconnect (CCI).
It allows broadcasting of TLB invalidates and memory barriers and it
guarantees cache coherency at system level through snooping of slave
interfaces connected to it.
This patch enables the basic infrastructure required in Linux to handle and
programme the CCI component.
Non-local variables used by the CCI management functions called by power
down function calls after disabling the cache must be flushed out to main
memory in advance, otherwise incoherency of those values may occur if they
are sitting in the cache of some other CPU when power down functions
execute. Driver code ensures that relevant data structures are flushed
from inner and outer caches after the driver probe is completed.
CCI slave port resources are linked to set of CPUs through bus masters
phandle properties that link the interface resources to masters node in
the device tree.
Documentation describing the CCI DT bindings is provided with the patch.
Signed-off-by: Lorenzo Pieralisi lorenzo.pieral...@arm.com
---
Documentation/devicetree/bindings/arm/cci.txt | 161 +++
drivers/bus/Kconfig | 7 +
drivers/bus/Makefile | 2 +
drivers/bus/arm-cci.c | 372 ++
include/linux/arm-cci.h | 59
5 files changed, 601 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/cci.txt
create mode 100644 drivers/bus/arm-cci.c
create mode 100644 include/linux/arm-cci.h
diff --git a/Documentation/devicetree/bindings/arm/cci.txt
b/Documentation/devicetree/bindings/arm/cci.txt
new file mode 100644
index 000..384b460
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/cci.txt
@@ -0,0 +1,161 @@
+===
+ARM CCI cache coherent interconnect binding description
+===
+
+ARM multi-cluster systems maintain intra-cluster coherency through a
+cache coherent interconnect (CCI) that is capable of monitoring bus
+transactions and manage coherency, TLB invalidations and memory barriers.
+
+It allows snooping and distributed virtual memory message broadcast across
+clusters, through memory mapped interface, with a global control register
+space and multiple sets of interface control registers, one per slave
+interface.
+
+Bindings for the CCI node follow the ePAPR standard, available from:
+
+www.power.org/documentation/epapr-version-1-1/
+
+with the addition of the bindings described in this document which are
+specific to ARM.
+
+* CCI interconnect node
+
+ Description: Describes a CCI cache coherent Interconnect component
+
+ Node name must be cci.
+ Node's parent must be the root node /, and the address space visible
+ through the CCI interconnect is the same as the one seen from the
+ root node (ie from CPUs perspective as per DT standard).
+ Every CCI node has to define the following properties:
+
+ - compatible
+ Usage: required
+ Value type: string
+ Definition: must be set to
+ arm,cci-400
+
+ - reg
+ Usage: required
+ Value type: prop-encoded-array
+ Definition: A standard property. Specifies base physical
+ address of CCI control registers common to all
+ interfaces.
+
+ - ranges:
+ Usage: required
+ Value type: prop-encoded-array
+ Definition: A standard property. Follow rules in the ePAPR for
+ hierarchical bus addressing. CCI interfaces
+ addresses refer to the parent node addressing
+ scheme to declare their register bases.
+
+ CCI interconnect node can define the following child nodes:
+
+ - CCI control interface nodes
+
+ Node name must be slave-if.
+ Parent node must be CCI interconnect node.
+
+ A CCI interface node must contain the following properties:
+ - interface-type:
+ Usage: required
+ Value type: string
+ Definition: must be set to one of {ace, ace-lite}
+ depending on the interface type the node
+ represents.
+
+ - reg:
+ Usage: required
+ Value type: prop-encoded-array
+ Definition: the base address and size of the
+ corresponding interface programming
+ registers.
+
+* CCI interconnect bus masters
+
+ Description: masters in the device tree connected to a CCI port
+
