[PATCH v2 05/14] dt-bindings: add openrisc to vendor prefixes list

[PATCH v2 05/14] dt-bindings: add openrisc to vendor prefixes list

[Stafford Horne]

Add OpenRISC.io to vendor prefixes.  This is reserved for softcores
developed by the OpenRISC community.  The OpenRISC community has
separated from OpenCores.org requiring a new prefix.

Signed-off-by: Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

Changes since v1
 - New patch

 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index daf465bef758..0025aa7c3745 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -241,6 +241,7 @@ onion	Onion Corporation
 onnn	ON Semiconductor Corp.
 ontat	On Tat Industrial Company
 opencores	OpenCores.org
+openrisc	OpenRISC.io
 option	Option NV
 ORCL	Oracle Corporation
 ortustech	Ortus Technology Co., Ltd.
-- 
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 05/14] dt-bindings: add openrisc to vendor prefixes list

[Andreas Färber]

Am 10.09.2017 um 08:49 schrieb Stafford Horne:
> Add OpenRISC.io to vendor prefixes.  This is reserved for softcores
> developed by the OpenRISC community.  The OpenRISC community has
> separated from OpenCores.org requiring a new prefix.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 05/14] dt-bindings: add openrisc to vendor prefixes list

[Rob Herring]

On Sun, Sep 10, 2017 at 03:49:17PM +0900, Stafford Horne wrote:
> Add OpenRISC.io to vendor prefixes.  This is reserved for softcores
> developed by the OpenRISC community.  The OpenRISC community has
> separated from OpenCores.org requiring a new prefix.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
> 
> Changes since v1
>  - New patch
> 
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v2 06/14] irqchip: add initial support for ompic

[Stafford Horne]

From: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>

IPI driver for the Open Multi-Processor Interrupt Controller (ompic) as
described in the Multicore support section of the OpenRISC 1.2
proposed architecture specification:

  https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf

Each OpenRISC core contains a full interrupt controller which is used in
the SMP architecture for interrupt balancing.  This IPI device, the
ompic, is the only external device required for enabling SMP on
OpenRISC.

Pending ops are stored in a memory bit mask which can allow multiple
pending operations to be set and serviced at a time. This is mostly
borrowed from the alpha IPI implementation.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
[shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: converted ops to bitmask, wrote commit message]
Signed-off-by: Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

Changes since v1
 - Added openrisc, prefix
 - Clarified 8 bytes per cpu
 - Removed #interrupt-cells as this will not be an irq parent
 - Changed ops to be percpu
 - Added DTS and intialization failure validations

 .../interrupt-controller/openrisc,ompic.txt        |  19 ++
 arch/openrisc/Kconfig                              |   1 +
 drivers/irqchip/Kconfig                            |   3 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-ompic.c                        | 205 +++++++++++++++++++++
 5 files changed, 229 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
 create mode 100644 drivers/irqchip/irq-ompic.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
new file mode 100644
index 000000000000..346e6042d62f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
@@ -0,0 +1,19 @@
+Open Multi-Processor Interrupt Controller
+
+Required properties:
+
+- compatible : This should be "openrisc,ompic"
+- reg : Specifies base physical address and size of the register space. The
+  size is based on the number of cores the controller has been configured
+  to handle, this should be set to 8 bytes per cpu core.
+- interrupt-controller : Identifies the node as an interrupt controller
+- interrupts : Specifies the interrupt line to which the ompic is wired.
+
+Example:
+
+ompic: ompic {
+	compatible = "openrisc,ompic";
+	reg = <0x98000000 16>;
+	interrupt-controller;
+	interrupts = <1>;
+};
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index b49acda5e8f4..34eb4e90f56c 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -30,6 +30,7 @@ config OPENRISC
 	select NO_BOOTMEM
 	select ARCH_USE_QUEUED_SPINLOCKS
 	select ARCH_USE_QUEUED_RWLOCKS
+	select OMPIC if SMP
 
 config CPU_BIG_ENDIAN
 	def_bool y
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f1fd5f44d1d4..0e4c96c90b86 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -145,6 +145,9 @@ config CLPS711X_IRQCHIP
 	select SPARSE_IRQ
 	default y
 
+config OMPIC
+	bool
+
 config OR1K_PIC
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e88d856cc09c..123047d7a20d 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
 obj-$(CONFIG_METAG)			+= irq-metag-ext.o
 obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
 obj-$(CONFIG_CLPS711X_IRQCHIP)		+= irq-clps711x.o
+obj-$(CONFIG_OMPIC)			+= irq-ompic.o
 obj-$(CONFIG_OR1K_PIC)			+= irq-or1k-pic.o
 obj-$(CONFIG_ORION_IRQCHIP)		+= irq-orion.o
 obj-$(CONFIG_OMAP_IRQCHIP)		+= irq-omap-intc.o
diff --git a/drivers/irqchip/irq-ompic.c b/drivers/irqchip/irq-ompic.c
new file mode 100644
index 000000000000..cd2616b6639b
--- /dev/null
+++ b/drivers/irqchip/irq-ompic.c
@@ -0,0 +1,205 @@
+/*
+ * Open Multi-Processor Interrupt Controller driver
+ *
+ * Copyright (C) 2014 Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
+ * Copyright (C) 2017 Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ *
+ * The ompic device handles IPI communication because cores in mulicore
+ * OpenRISC systems.
+ *
+ * Registers
+ *
+ * For each CPU the ompic has 2 registers. The control register for sending
+ * and acking IPIs and the status register for receiving IPIs. The register
+ * layouts are as follows:
+ *
+ *  Control register
+ *  +---------+---------+----------+---------+
+ *  | 31      | 30      | 29 .. 16 | 15 .. 0 |
+ *  ----------+---------+----------+----------
+ *  | IRQ ACK | IRQ GEN | DST CORE | DATA    |
+ *  +---------+---------+----------+---------+
+ *
+ *  Status register
+ *  +----------+-------------+----------+---------+
+ *  | 31       | 30          | 29 .. 16 | 15 .. 0 |
+ *  -----------+-------------+----------+---------+
+ *  | Reserved | IRQ Pending | SRC CORE | DATA    |
+ *  +----------+-------------+----------+---------+
+ *
+ * Architecture
+ *
+ * - The ompic generates a level interrupt to the CPU PIC when a message is
+ *   ready.  Messages are delivered via the memory bus.
+ * - The ompic does not have any interrupt input lines.
+ * - The ompic is wired to the same irq line on each core.
+ * - Devices are wired to the same irq line on each core.
+ *
+ *   +---------+                         +---------+
+ *   | CPU     |                         | CPU     |
+ *   |  Core 0 |<==\ (memory access) /==>|  Core 1 |
+ *   |  [ PIC ]|   |                 |   |  [ PIC ]|
+ *   +----^-^--+   |                 |   +----^-^--+
+ *        | |      v                 v        | |
+ *   <====|=|=================================|=|==> (memory bus)
+ *        | |      ^                  ^       | |
+ *  (ipi  | +------|---------+--------|-------|-+ (device irq)
+ *   irq  |        |         |        |       |
+ *  core0)| +------|---------|--------|-------+ (ipi irq core1)
+ *        | |      |         |        |
+ *   +----o-o-+    |    +--------+    |
+ *   | ompic  |<===/    | Device |<===/
+ *   |  IPI   |         +--------+
+ *   +--------+*
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/interrupt.h>
+#include <linux/smp.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+
+#include <linux/irqchip.h>
+
+#define OMPIC_CPUBYTES		8
+#define OMPIC_CTRL(cpu)		(0x0 + (cpu * OMPIC_CPUBYTES))
+#define OMPIC_STAT(cpu)		(0x4 + (cpu * OMPIC_CPUBYTES))
+
+#define OMPIC_CTRL_IRQ_ACK	(1 << 31)
+#define OMPIC_CTRL_IRQ_GEN	(1 << 30)
+#define OMPIC_CTRL_DST(cpu)	(((cpu) & 0x3fff) << 16)
+
+#define OMPIC_STAT_IRQ_PENDING	(1 << 30)
+
+#define OMPIC_DATA(x)		((x) & 0xffff)
+
+DEFINE_PER_CPU(unsigned long, ops);
+
+static void __iomem *ompic_base;
+
+static inline u32 ompic_readreg(void __iomem *base, loff_t offset)
+{
+	return ioread32be(base + offset);
+}
+
+static void ompic_writereg(void __iomem *base, loff_t offset, u32 data)
+{
+	iowrite32be(data, base + offset);
+}
+
+void ompic_raise_softirq(const struct cpumask *mask, unsigned int ipi_msg)
+{
+	unsigned int dst_cpu;
+	unsigned int src_cpu = smp_processor_id();
+
+	for_each_cpu(dst_cpu, mask) {
+		set_bit(ipi_msg, &per_cpu(ops, dst_cpu));
+
+		/*
+		 * On OpenRISC the atomic set_bit() call implies a memory
+		 * barrier.  Otherwise we would need: smp_wmb(); paired
+		 * with the read in ompic_ipi_handler.
+		 */
+
+		ompic_writereg(ompic_base, OMPIC_CTRL(src_cpu),
+			       OMPIC_CTRL_IRQ_GEN |
+			       OMPIC_CTRL_DST(dst_cpu) |
+			       OMPIC_DATA(1));
+	}
+}
+
+irqreturn_t ompic_ipi_handler(int irq, void *dev_id)
+{
+	unsigned int cpu = smp_processor_id();
+	unsigned long *pending_ops = &per_cpu(ops, cpu);
+	unsigned long ops;
+
+	ompic_writereg(ompic_base, OMPIC_CTRL(cpu), OMPIC_CTRL_IRQ_ACK);
+	while ((ops = xchg(pending_ops, 0)) != 0) {
+
+		/*
+		 * On OpenRISC the atomic xchg() call implies a memory
+		 * barrier.  Otherwise we may need an smp_rmb(); paired
+		 * with the write in ompic_raise_softirq.
+		 */
+
+		do {
+			unsigned long ipi_msg;
+
+			ipi_msg = __ffs(ops);
+			ops &= ~(1UL << ipi_msg);
+
+			handle_IPI(ipi_msg);
+		} while (ops);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction ompi_ipi_irqaction = {
+	.handler =      ompic_ipi_handler,
+	.flags =        IRQF_PERCPU,
+	.name =         "ompic_ipi",
+};
+
+int __init ompic_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct resource res;
+	int irq;
+	int ret;
+
+	/* Validate the DT */
+	if (ompic_base) {
+		pr_err("ompic: duplicate ompic's are not supported");
+		return -EEXIST;
+	}
+
+	if (of_address_to_resource(node, 0, &res)) {
+		pr_err("ompic: reg property requires an address and size");
+		return -EINVAL;
+	}
+
+	if (resource_size(&res) < (num_possible_cpus() * OMPIC_CPUBYTES)) {
+		pr_err("ompic: reg size, currently %d must be at least %d",
+			resource_size(&res),
+			(num_possible_cpus() * OMPIC_CPUBYTES));
+		return -EINVAL;
+	}
+
+	/* Setup the device */
+	ompic_base = ioremap(res.start, resource_size(&res));
+	if (IS_ERR(ompic_base)) {
+		pr_err("ompic: unable to map registers");
+		return PTR_ERR(ompic_base);
+	}
+
+	irq = irq_of_parse_and_map(node, 0);
+	if (irq <= 0) {
+		pr_err("ompic: unable to parse device irq");
+		ret = -EINVAL;
+		goto out_unmap;
+	}
+
+	ret = setup_irq(irq, &ompi_ipi_irqaction);
+	if (ret)
+		goto out_irq_disp;
+
+	set_smp_cross_call(ompic_raise_softirq);
+
+	return 0;
+
+out_irq_disp:
+	irq_dispose_mapping(irq);
+out_unmap:
+	iounmap(ompic_base);
+	ompic_base = NULL;
+	return ret;
+}
+IRQCHIP_DECLARE(ompic, "openrisc,ompic", ompic_of_init);
-- 
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Marc Zyngier]

On Sun, Sep 10 2017 at  3:49:18 pm BST, Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
>
> IPI driver for the Open Multi-Processor Interrupt Controller (ompic) as
> described in the Multicore support section of the OpenRISC 1.2
> proposed architecture specification:
>
>   https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf
>
> Each OpenRISC core contains a full interrupt controller which is used in
> the SMP architecture for interrupt balancing.  This IPI device, the
> ompic, is the only external device required for enabling SMP on
> OpenRISC.
>
> Pending ops are stored in a memory bit mask which can allow multiple
> pending operations to be set and serviced at a time. This is mostly
> borrowed from the alpha IPI implementation.
>
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> [shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: converted ops to bitmask, wrote commit message]
> Signed-off-by: Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>
> Changes since v1
>  - Added openrisc, prefix
>  - Clarified 8 bytes per cpu
>  - Removed #interrupt-cells as this will not be an irq parent
>  - Changed ops to be percpu
>  - Added DTS and intialization failure validations
>
>  .../interrupt-controller/openrisc,ompic.txt        |  19 ++
>  arch/openrisc/Kconfig                              |   1 +
>  drivers/irqchip/Kconfig                            |   3 +
>  drivers/irqchip/Makefile                           |   1 +
>  drivers/irqchip/irq-ompic.c                        | 205 +++++++++++++++++++++
>  5 files changed, 229 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
>  create mode 100644 drivers/irqchip/irq-ompic.c
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> new file mode 100644
> index 000000000000..346e6042d62f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> @@ -0,0 +1,19 @@
> +Open Multi-Processor Interrupt Controller
> +
> +Required properties:
> +
> +- compatible : This should be "openrisc,ompic"
> +- reg : Specifies base physical address and size of the register space. The
> +  size is based on the number of cores the controller has been configured
> +  to handle, this should be set to 8 bytes per cpu core.
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- interrupts : Specifies the interrupt line to which the ompic is wired.
> +
> +Example:
> +
> +ompic: ompic {
> +	compatible = "openrisc,ompic";
> +	reg = <0x98000000 16>;
> +	interrupt-controller;
> +	interrupts = <1>;
> +};
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index b49acda5e8f4..34eb4e90f56c 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -30,6 +30,7 @@ config OPENRISC
>  	select NO_BOOTMEM
>  	select ARCH_USE_QUEUED_SPINLOCKS
>  	select ARCH_USE_QUEUED_RWLOCKS
> +	select OMPIC if SMP
>  
>  config CPU_BIG_ENDIAN
>  	def_bool y
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f1fd5f44d1d4..0e4c96c90b86 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -145,6 +145,9 @@ config CLPS711X_IRQCHIP
>  	select SPARSE_IRQ
>  	default y
>  
> +config OMPIC
> +	bool
> +
>  config OR1K_PIC
>  	bool
>  	select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e88d856cc09c..123047d7a20d 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
>  obj-$(CONFIG_METAG)			+= irq-metag-ext.o
>  obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
>  obj-$(CONFIG_CLPS711X_IRQCHIP)		+= irq-clps711x.o
> +obj-$(CONFIG_OMPIC)			+= irq-ompic.o
>  obj-$(CONFIG_OR1K_PIC)			+= irq-or1k-pic.o
>  obj-$(CONFIG_ORION_IRQCHIP)		+= irq-orion.o
>  obj-$(CONFIG_OMAP_IRQCHIP)		+= irq-omap-intc.o
> diff --git a/drivers/irqchip/irq-ompic.c b/drivers/irqchip/irq-ompic.c
> new file mode 100644
> index 000000000000..cd2616b6639b
> --- /dev/null
> +++ b/drivers/irqchip/irq-ompic.c
> @@ -0,0 +1,205 @@
> +/*
> + * Open Multi-Processor Interrupt Controller driver
> + *
> + * Copyright (C) 2014 Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> + * Copyright (C) 2017 Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + *
> + * The ompic device handles IPI communication because cores in mulicore
> + * OpenRISC systems.

Should the above read "between cores"?

> + *
> + * Registers
> + *
> + * For each CPU the ompic has 2 registers. The control register for sending
> + * and acking IPIs and the status register for receiving IPIs. The register
> + * layouts are as follows:
> + *
> + *  Control register
> + *  +---------+---------+----------+---------+
> + *  | 31      | 30      | 29 .. 16 | 15 .. 0 |
> + *  ----------+---------+----------+----------
> + *  | IRQ ACK | IRQ GEN | DST CORE | DATA    |
> + *  +---------+---------+----------+---------+
> + *
> + *  Status register
> + *  +----------+-------------+----------+---------+
> + *  | 31       | 30          | 29 .. 16 | 15 .. 0 |
> + *  -----------+-------------+----------+---------+
> + *  | Reserved | IRQ Pending | SRC CORE | DATA    |
> + *  +----------+-------------+----------+---------+
> + *
> + * Architecture
> + *
> + * - The ompic generates a level interrupt to the CPU PIC when a message is
> + *   ready.  Messages are delivered via the memory bus.
> + * - The ompic does not have any interrupt input lines.
> + * - The ompic is wired to the same irq line on each core.
> + * - Devices are wired to the same irq line on each core.
> + *
> + *   +---------+                         +---------+
> + *   | CPU     |                         | CPU     |
> + *   |  Core 0 |<==\ (memory access) /==>|  Core 1 |
> + *   |  [ PIC ]|   |                 |   |  [ PIC ]|
> + *   +----^-^--+   |                 |   +----^-^--+
> + *        | |      v                 v        | |
> + *   <====|=|=================================|=|==> (memory bus)
> + *        | |      ^                  ^       | |
> + *  (ipi  | +------|---------+--------|-------|-+ (device irq)
> + *   irq  |        |         |        |       |
> + *  core0)| +------|---------|--------|-------+ (ipi irq core1)
> + *        | |      |         |        |
> + *   +----o-o-+    |    +--------+    |
> + *   | ompic  |<===/    | Device |<===/
> + *   |  IPI   |         +--------+
> + *   +--------+*
> + *
> + */
> +
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/interrupt.h>
> +#include <linux/smp.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +
> +#include <linux/irqchip.h>
> +
> +#define OMPIC_CPUBYTES		8
> +#define OMPIC_CTRL(cpu)		(0x0 + (cpu * OMPIC_CPUBYTES))
> +#define OMPIC_STAT(cpu)		(0x4 + (cpu * OMPIC_CPUBYTES))
> +
> +#define OMPIC_CTRL_IRQ_ACK	(1 << 31)
> +#define OMPIC_CTRL_IRQ_GEN	(1 << 30)
> +#define OMPIC_CTRL_DST(cpu)	(((cpu) & 0x3fff) << 16)
> +
> +#define OMPIC_STAT_IRQ_PENDING	(1 << 30)
> +
> +#define OMPIC_DATA(x)		((x) & 0xffff)
> +
> +DEFINE_PER_CPU(unsigned long, ops);
> +
> +static void __iomem *ompic_base;
> +
> +static inline u32 ompic_readreg(void __iomem *base, loff_t offset)
> +{
> +	return ioread32be(base + offset);
> +}
> +
> +static void ompic_writereg(void __iomem *base, loff_t offset, u32 data)
> +{
> +	iowrite32be(data, base + offset);
> +}
> +
> +void ompic_raise_softirq(const struct cpumask *mask, unsigned int ipi_msg)

Please make this static.

> +{
> +	unsigned int dst_cpu;
> +	unsigned int src_cpu = smp_processor_id();
> +
> +	for_each_cpu(dst_cpu, mask) {
> +		set_bit(ipi_msg, &per_cpu(ops, dst_cpu));
> +
> +		/*
> +		 * On OpenRISC the atomic set_bit() call implies a memory
> +		 * barrier.  Otherwise we would need: smp_wmb(); paired
> +		 * with the read in ompic_ipi_handler.
> +		 */

One last question on this, because the architecture document is terribly
unclear: If you have CPU0 doing an atomic operation A0, CPU1 seeing A0
and doeing another atomic A1 (the set_bit above) followed by an IPI to
CPU2, is CPU2 *guaranteed* to observe both A0 *and* A1? Because that's
required by the IPI semantics, and you wouldn't see that kind of issue
with only two CPUs.

> +
> +		ompic_writereg(ompic_base, OMPIC_CTRL(src_cpu),
> +			       OMPIC_CTRL_IRQ_GEN |
> +			       OMPIC_CTRL_DST(dst_cpu) |
> +			       OMPIC_DATA(1));
> +	}
> +}
> +
> +irqreturn_t ompic_ipi_handler(int irq, void *dev_id)

This should be static.

> +{
> +	unsigned int cpu = smp_processor_id();
> +	unsigned long *pending_ops = &per_cpu(ops, cpu);
> +	unsigned long ops;
> +
> +	ompic_writereg(ompic_base, OMPIC_CTRL(cpu), OMPIC_CTRL_IRQ_ACK);
> +	while ((ops = xchg(pending_ops, 0)) != 0) {
> +
> +		/*
> +		 * On OpenRISC the atomic xchg() call implies a memory
> +		 * barrier.  Otherwise we may need an smp_rmb(); paired
> +		 * with the write in ompic_raise_softirq.
> +		 */
> +
> +		do {
> +			unsigned long ipi_msg;
> +
> +			ipi_msg = __ffs(ops);
> +			ops &= ~(1UL << ipi_msg);
> +
> +			handle_IPI(ipi_msg);
> +		} while (ops);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct irqaction ompi_ipi_irqaction = {
> +	.handler =      ompic_ipi_handler,
> +	.flags =        IRQF_PERCPU,
> +	.name =         "ompic_ipi",
> +};
> +
> +int __init ompic_of_init(struct device_node *node, struct device_node *parent)

static again.

> +{
> +	struct resource res;
> +	int irq;
> +	int ret;
> +
> +	/* Validate the DT */
> +	if (ompic_base) {
> +		pr_err("ompic: duplicate ompic's are not supported");
> +		return -EEXIST;
> +	}
> +
> +	if (of_address_to_resource(node, 0, &res)) {
> +		pr_err("ompic: reg property requires an address and size");
> +		return -EINVAL;
> +	}
> +
> +	if (resource_size(&res) < (num_possible_cpus() * OMPIC_CPUBYTES)) {
> +		pr_err("ompic: reg size, currently %d must be at least %d",
> +			resource_size(&res),
> +			(num_possible_cpus() * OMPIC_CPUBYTES));
> +		return -EINVAL;
> +	}
> +
> +	/* Setup the device */
> +	ompic_base = ioremap(res.start, resource_size(&res));
> +	if (IS_ERR(ompic_base)) {
> +		pr_err("ompic: unable to map registers");
> +		return PTR_ERR(ompic_base);
> +	}
> +
> +	irq = irq_of_parse_and_map(node, 0);
> +	if (irq <= 0) {
> +		pr_err("ompic: unable to parse device irq");
> +		ret = -EINVAL;
> +		goto out_unmap;
> +	}
> +
> +	ret = setup_irq(irq, &ompi_ipi_irqaction);
> +	if (ret)
> +		goto out_irq_disp;
> +
> +	set_smp_cross_call(ompic_raise_softirq);
> +
> +	return 0;
> +
> +out_irq_disp:
> +	irq_dispose_mapping(irq);
> +out_unmap:
> +	iounmap(ompic_base);
> +	ompic_base = NULL;
> +	return ret;
> +}
> +IRQCHIP_DECLARE(ompic, "openrisc,ompic", ompic_of_init);

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Stafford Horne]

On Wed, Sep 13, 2017 at 06:21:39PM +0100, Marc Zyngier wrote:
> On Sun, Sep 10 2017 at  3:49:18 pm BST, Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > From: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> >
> > IPI driver for the Open Multi-Processor Interrupt Controller (ompic) as
> > described in the Multicore support section of the OpenRISC 1.2
> > proposed architecture specification:
> >
> >   https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf
> >
> > Each OpenRISC core contains a full interrupt controller which is used in
> > the SMP architecture for interrupt balancing.  This IPI device, the
> > ompic, is the only external device required for enabling SMP on
> > OpenRISC.
> >
> > Pending ops are stored in a memory bit mask which can allow multiple
> > pending operations to be set and serviced at a time. This is mostly
> > borrowed from the alpha IPI implementation.
> >
> > Signed-off-by: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > [shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: converted ops to bitmask, wrote commit message]
> > Signed-off-by: Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >
> > Changes since v1
> >  - Added openrisc, prefix
> >  - Clarified 8 bytes per cpu
> >  - Removed #interrupt-cells as this will not be an irq parent
> >  - Changed ops to be percpu
> >  - Added DTS and intialization failure validations
> >
> >  .../interrupt-controller/openrisc,ompic.txt        |  19 ++
> >  arch/openrisc/Kconfig                              |   1 +
> >  drivers/irqchip/Kconfig                            |   3 +
> >  drivers/irqchip/Makefile                           |   1 +
> >  drivers/irqchip/irq-ompic.c                        | 205 +++++++++++++++++++++
> >  5 files changed, 229 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> >  create mode 100644 drivers/irqchip/irq-ompic.c
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > new file mode 100644
> > index 000000000000..346e6042d62f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > @@ -0,0 +1,19 @@
> > +Open Multi-Processor Interrupt Controller
> > +
> > +Required properties:
> > +
> > +- compatible : This should be "openrisc,ompic"
> > +- reg : Specifies base physical address and size of the register space. The
> > +  size is based on the number of cores the controller has been configured
> > +  to handle, this should be set to 8 bytes per cpu core.
> > +- interrupt-controller : Identifies the node as an interrupt controller
> > +- interrupts : Specifies the interrupt line to which the ompic is wired.
> > +
> > +Example:
> > +
> > +ompic: ompic {
> > +	compatible = "openrisc,ompic";
> > +	reg = <0x98000000 16>;
> > +	interrupt-controller;
> > +	interrupts = <1>;
> > +};
> > diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> > index b49acda5e8f4..34eb4e90f56c 100644
> > --- a/arch/openrisc/Kconfig
> > +++ b/arch/openrisc/Kconfig
> > @@ -30,6 +30,7 @@ config OPENRISC
> >  	select NO_BOOTMEM
> >  	select ARCH_USE_QUEUED_SPINLOCKS
> >  	select ARCH_USE_QUEUED_RWLOCKS
> > +	select OMPIC if SMP
> >  
> >  config CPU_BIG_ENDIAN
> >  	def_bool y
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index f1fd5f44d1d4..0e4c96c90b86 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -145,6 +145,9 @@ config CLPS711X_IRQCHIP
> >  	select SPARSE_IRQ
> >  	default y
> >  
> > +config OMPIC
> > +	bool
> > +
> >  config OR1K_PIC
> >  	bool
> >  	select IRQ_DOMAIN
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index e88d856cc09c..123047d7a20d 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
> >  obj-$(CONFIG_METAG)			+= irq-metag-ext.o
> >  obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
> >  obj-$(CONFIG_CLPS711X_IRQCHIP)		+= irq-clps711x.o
> > +obj-$(CONFIG_OMPIC)			+= irq-ompic.o
> >  obj-$(CONFIG_OR1K_PIC)			+= irq-or1k-pic.o
> >  obj-$(CONFIG_ORION_IRQCHIP)		+= irq-orion.o
> >  obj-$(CONFIG_OMAP_IRQCHIP)		+= irq-omap-intc.o
> > diff --git a/drivers/irqchip/irq-ompic.c b/drivers/irqchip/irq-ompic.c
> > new file mode 100644
> > index 000000000000..cd2616b6639b
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-ompic.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + * Open Multi-Processor Interrupt Controller driver
> > + *
> > + * Copyright (C) 2014 Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > + * Copyright (C) 2017 Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > + *
> > + * This file is licensed under the terms of the GNU General Public License
> > + * version 2.  This program is licensed "as is" without any warranty of any
> > + * kind, whether express or implied.
> > + *
> > + * The ompic device handles IPI communication because cores in mulicore
> > + * OpenRISC systems.
> 
> Should the above read "between cores"?

Yes, it should be, I am bad with these kind of typos.

> > + *
> > + * Registers
> > + *
> > + * For each CPU the ompic has 2 registers. The control register for sending
> > + * and acking IPIs and the status register for receiving IPIs. The register
> > + * layouts are as follows:
> > + *
> > + *  Control register
> > + *  +---------+---------+----------+---------+
> > + *  | 31      | 30      | 29 .. 16 | 15 .. 0 |
> > + *  ----------+---------+----------+----------
> > + *  | IRQ ACK | IRQ GEN | DST CORE | DATA    |
> > + *  +---------+---------+----------+---------+
> > + *
> > + *  Status register
> > + *  +----------+-------------+----------+---------+
> > + *  | 31       | 30          | 29 .. 16 | 15 .. 0 |
> > + *  -----------+-------------+----------+---------+
> > + *  | Reserved | IRQ Pending | SRC CORE | DATA    |
> > + *  +----------+-------------+----------+---------+
> > + *
> > + * Architecture
> > + *
> > + * - The ompic generates a level interrupt to the CPU PIC when a message is
> > + *   ready.  Messages are delivered via the memory bus.
> > + * - The ompic does not have any interrupt input lines.
> > + * - The ompic is wired to the same irq line on each core.
> > + * - Devices are wired to the same irq line on each core.
> > + *
> > + *   +---------+                         +---------+
> > + *   | CPU     |                         | CPU     |
> > + *   |  Core 0 |<==\ (memory access) /==>|  Core 1 |
> > + *   |  [ PIC ]|   |                 |   |  [ PIC ]|
> > + *   +----^-^--+   |                 |   +----^-^--+
> > + *        | |      v                 v        | |
> > + *   <====|=|=================================|=|==> (memory bus)
> > + *        | |      ^                  ^       | |
> > + *  (ipi  | +------|---------+--------|-------|-+ (device irq)
> > + *   irq  |        |         |        |       |
> > + *  core0)| +------|---------|--------|-------+ (ipi irq core1)
> > + *        | |      |         |        |
> > + *   +----o-o-+    |    +--------+    |
> > + *   | ompic  |<===/    | Device |<===/
> > + *   |  IPI   |         +--------+
> > + *   +--------+*
> > + *
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/ioport.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/smp.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_address.h>
> > +
> > +#include <linux/irqchip.h>
> > +
> > +#define OMPIC_CPUBYTES		8
> > +#define OMPIC_CTRL(cpu)		(0x0 + (cpu * OMPIC_CPUBYTES))
> > +#define OMPIC_STAT(cpu)		(0x4 + (cpu * OMPIC_CPUBYTES))
> > +
> > +#define OMPIC_CTRL_IRQ_ACK	(1 << 31)
> > +#define OMPIC_CTRL_IRQ_GEN	(1 << 30)
> > +#define OMPIC_CTRL_DST(cpu)	(((cpu) & 0x3fff) << 16)
> > +
> > +#define OMPIC_STAT_IRQ_PENDING	(1 << 30)
> > +
> > +#define OMPIC_DATA(x)		((x) & 0xffff)
> > +
> > +DEFINE_PER_CPU(unsigned long, ops);
> > +
> > +static void __iomem *ompic_base;
> > +
> > +static inline u32 ompic_readreg(void __iomem *base, loff_t offset)
> > +{
> > +	return ioread32be(base + offset);
> > +}
> > +
> > +static void ompic_writereg(void __iomem *base, loff_t offset, u32 data)
> > +{
> > +	iowrite32be(data, base + offset);
> > +}
> > +
> > +void ompic_raise_softirq(const struct cpumask *mask, unsigned int ipi_msg)
> 
> Please make this static.

OK

> > +{
> > +	unsigned int dst_cpu;
> > +	unsigned int src_cpu = smp_processor_id();
> > +
> > +	for_each_cpu(dst_cpu, mask) {
> > +		set_bit(ipi_msg, &per_cpu(ops, dst_cpu));
> > +
> > +		/*
> > +		 * On OpenRISC the atomic set_bit() call implies a memory
> > +		 * barrier.  Otherwise we would need: smp_wmb(); paired
> > +		 * with the read in ompic_ipi_handler.
> > +		 */
> 
> One last question on this, because the architecture document is terribly
> unclear: If you have CPU0 doing an atomic operation A0, CPU1 seeing A0
> and doeing another atomic A1 (the set_bit above) followed by an IPI to
> CPU2, is CPU2 *guaranteed* to observe both A0 *and* A1? Because that's
> required by the IPI semantics, and you wouldn't see that kind of issue
> with only two CPUs.

Could you suggest an architecture document that makes this case clear?

I believe this will not be a problem, but:
  1. If this needs to be clear in the architecture document I can propose
     changes.
  2. To be clear is this the scenario you mean..

CASE1 - A0 and A1 are to different locations?
  A0 - writes to some unrelated global location?

CPU0                       CPU1                     CPU2
 A0:atomic store (global)
                           A1:set_bit (ops[CPU2])
                           IPI
                                                    read (A0,A1)


OR

CASE2 - A0 and A1 are to the same location.
  A0 - writes to the same location as A1

CPU0                       CPU1                     CPU2
 A0:set_bit (ops[CPU2])
                           A1:set_bit (ops[CPU2])
                           IPI
                                                    read (A0,A1)
 IPI


OR - something else?

In both cases CPU2 would be able to see the results of both atomic
operations.  All, cpus in the OpenRISC system snoop for memory writes to
enable cash coherency, so each CPU would see each write once it is synced
to memory (there is a single memory bus).  This is not limited to atomic
operations, but the atomic operations provide a syncrhonization point
accross all CPUs.

> > +
> > +		ompic_writereg(ompic_base, OMPIC_CTRL(src_cpu),
> > +			       OMPIC_CTRL_IRQ_GEN |
> > +			       OMPIC_CTRL_DST(dst_cpu) |
> > +			       OMPIC_DATA(1));
> > +	}
> > +}
> > +
> > +irqreturn_t ompic_ipi_handler(int irq, void *dev_id)
> 
> This should be static.

OK

> > +{
> > +	unsigned int cpu = smp_processor_id();
> > +	unsigned long *pending_ops = &per_cpu(ops, cpu);
> > +	unsigned long ops;
> > +
> > +	ompic_writereg(ompic_base, OMPIC_CTRL(cpu), OMPIC_CTRL_IRQ_ACK);
> > +	while ((ops = xchg(pending_ops, 0)) != 0) {
> > +
> > +		/*
> > +		 * On OpenRISC the atomic xchg() call implies a memory
> > +		 * barrier.  Otherwise we may need an smp_rmb(); paired
> > +		 * with the write in ompic_raise_softirq.
> > +		 */
> > +
> > +		do {
> > +			unsigned long ipi_msg;
> > +
> > +			ipi_msg = __ffs(ops);
> > +			ops &= ~(1UL << ipi_msg);
> > +
> > +			handle_IPI(ipi_msg);
> > +		} while (ops);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static struct irqaction ompi_ipi_irqaction = {
> > +	.handler =      ompic_ipi_handler,
> > +	.flags =        IRQF_PERCPU,
> > +	.name =         "ompic_ipi",
> > +};
> > +
> > +int __init ompic_of_init(struct device_node *node, struct device_node *parent)
> 
> static again.

OK

> > +{
> > +	struct resource res;
> > +	int irq;
> > +	int ret;
> > +
> > +	/* Validate the DT */
> > +	if (ompic_base) {
> > +		pr_err("ompic: duplicate ompic's are not supported");
> > +		return -EEXIST;
> > +	}
> > +
> > +	if (of_address_to_resource(node, 0, &res)) {
> > +		pr_err("ompic: reg property requires an address and size");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (resource_size(&res) < (num_possible_cpus() * OMPIC_CPUBYTES)) {
> > +		pr_err("ompic: reg size, currently %d must be at least %d",
> > +			resource_size(&res),
> > +			(num_possible_cpus() * OMPIC_CPUBYTES));
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Setup the device */
> > +	ompic_base = ioremap(res.start, resource_size(&res));
> > +	if (IS_ERR(ompic_base)) {
> > +		pr_err("ompic: unable to map registers");
> > +		return PTR_ERR(ompic_base);
> > +	}
> > +
> > +	irq = irq_of_parse_and_map(node, 0);
> > +	if (irq <= 0) {
> > +		pr_err("ompic: unable to parse device irq");
> > +		ret = -EINVAL;
> > +		goto out_unmap;
> > +	}
> > +
> > +	ret = setup_irq(irq, &ompi_ipi_irqaction);
> > +	if (ret)
> > +		goto out_irq_disp;
> > +
> > +	set_smp_cross_call(ompic_raise_softirq);
> > +
> > +	return 0;
> > +
> > +out_irq_disp:
> > +	irq_dispose_mapping(irq);
> > +out_unmap:
> > +	iounmap(ompic_base);
> > +	ompic_base = NULL;
> > +	return ret;
> > +}
> > +IRQCHIP_DECLARE(ompic, "openrisc,ompic", ompic_of_init);
>

Thanks for your time!

 -Stafford

ps: Frank Zappa rocks :)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Marc Zyngier]

On Thu, Sep 14 2017 at  3:54:02 pm BST, Stafford Horne <shorne@gmail.com> wrote:
> On Wed, Sep 13, 2017 at 06:21:39PM +0100, Marc Zyngier wrote:

[...]

>> > +{
>> > +	unsigned int dst_cpu;
>> > +	unsigned int src_cpu = smp_processor_id();
>> > +
>> > +	for_each_cpu(dst_cpu, mask) {
>> > +		set_bit(ipi_msg, &per_cpu(ops, dst_cpu));
>> > +
>> > +		/*
>> > +		 * On OpenRISC the atomic set_bit() call implies a memory
>> > +		 * barrier.  Otherwise we would need: smp_wmb(); paired
>> > +		 * with the read in ompic_ipi_handler.
>> > +		 */
>> 
>> One last question on this, because the architecture document is terribly
>> unclear: If you have CPU0 doing an atomic operation A0, CPU1 seeing A0
>> and doeing another atomic A1 (the set_bit above) followed by an IPI to
>> CPU2, is CPU2 *guaranteed* to observe both A0 *and* A1? Because that's
>> required by the IPI semantics, and you wouldn't see that kind of issue
>> with only two CPUs.
>
> Could you suggest an architecture document that makes this case clear?
>
> I believe this will not be a problem, but:
>   1. If this needs to be clear in the architecture document I can propose
>      changes.
>   2. To be clear is this the scenario you mean..
>
> CASE1 - A0 and A1 are to different locations?
>   A0 - writes to some unrelated global location?
>
> CPU0                       CPU1                     CPU2
>  A0:atomic store (global)
>                            A1:set_bit (ops[CPU2])
>                            IPI
>                                                     read (A0,A1)
>
>
> OR
>
> CASE2 - A0 and A1 are to the same location.
>   A0 - writes to the same location as A1
>
> CPU0                       CPU1                     CPU2
>  A0:set_bit (ops[CPU2])
>                            A1:set_bit (ops[CPU2])
>                            IPI
>                                                     read (A0,A1)
>  IPI

I think this covers both cases I had in mind.

>
>
> OR - something else?
>
> In both cases CPU2 would be able to see the results of both atomic
> operations.  All, cpus in the OpenRISC system snoop for memory writes to
> enable cash coherency, so each CPU would see each write once it is synced
> to memory (there is a single memory bus).  This is not limited to atomic
> operations, but the atomic operations provide a syncrhonization point
> accross all CPUs.

OK. It would be good if the architecture document had something about
transitivity of writes on SMP (maybe it has, I only went through it
pretty quickly). But overall, the above will work correctly.

> ps: Frank Zappa rocks :)

His music certainly does! ;-)

Thanks,

        M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Rob Herring]

On Thu, Sep 14, 2017 at 03:54:02PM +0900, Stafford Horne wrote:
> On Wed, Sep 13, 2017 at 06:21:39PM +0100, Marc Zyngier wrote:
> > On Sun, Sep 10 2017 at  3:49:18 pm BST, Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > From: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > >
> > > IPI driver for the Open Multi-Processor Interrupt Controller (ompic) as
> > > described in the Multicore support section of the OpenRISC 1.2
> > > proposed architecture specification:
> > >
> > >   https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf
> > >
> > > Each OpenRISC core contains a full interrupt controller which is used in
> > > the SMP architecture for interrupt balancing.  This IPI device, the
> > > ompic, is the only external device required for enabling SMP on
> > > OpenRISC.
> > >
> > > Pending ops are stored in a memory bit mask which can allow multiple
> > > pending operations to be set and serviced at a time. This is mostly
> > > borrowed from the alpha IPI implementation.
> > >
> > > Signed-off-by: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > > [shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: converted ops to bitmask, wrote commit message]
> > > Signed-off-by: Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > ---
> > >
> > > Changes since v1
> > >  - Added openrisc, prefix
> > >  - Clarified 8 bytes per cpu
> > >  - Removed #interrupt-cells as this will not be an irq parent
> > >  - Changed ops to be percpu
> > >  - Added DTS and intialization failure validations
> > >
> > >  .../interrupt-controller/openrisc,ompic.txt        |  19 ++
> > >  arch/openrisc/Kconfig                              |   1 +
> > >  drivers/irqchip/Kconfig                            |   3 +
> > >  drivers/irqchip/Makefile                           |   1 +
> > >  drivers/irqchip/irq-ompic.c                        | 205 +++++++++++++++++++++
> > >  5 files changed, 229 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > >  create mode 100644 drivers/irqchip/irq-ompic.c
> > >
> > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > > new file mode 100644
> > > index 000000000000..346e6042d62f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > > @@ -0,0 +1,19 @@
> > > +Open Multi-Processor Interrupt Controller
> > > +
> > > +Required properties:
> > > +
> > > +- compatible : This should be "openrisc,ompic"
> > > +- reg : Specifies base physical address and size of the register space. The
> > > +  size is based on the number of cores the controller has been configured
> > > +  to handle, this should be set to 8 bytes per cpu core.
> > > +- interrupt-controller : Identifies the node as an interrupt controller
> > > +- interrupts : Specifies the interrupt line to which the ompic is wired.
> > > +
> > > +Example:
> > > +
> > > +ompic: ompic {
> > > +	compatible = "openrisc,ompic";
> > > +	reg = <0x98000000 16>;
> > > +	interrupt-controller;
> > > +	interrupts = <1>;
> > > +};
> > > diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> > > index b49acda5e8f4..34eb4e90f56c 100644
> > > --- a/arch/openrisc/Kconfig
> > > +++ b/arch/openrisc/Kconfig
> > > @@ -30,6 +30,7 @@ config OPENRISC
> > >  	select NO_BOOTMEM
> > >  	select ARCH_USE_QUEUED_SPINLOCKS
> > >  	select ARCH_USE_QUEUED_RWLOCKS
> > > +	select OMPIC if SMP
> > >  
> > >  config CPU_BIG_ENDIAN
> > >  	def_bool y
> > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > > index f1fd5f44d1d4..0e4c96c90b86 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -145,6 +145,9 @@ config CLPS711X_IRQCHIP
> > >  	select SPARSE_IRQ
> > >  	default y
> > >  
> > > +config OMPIC
> > > +	bool
> > > +
> > >  config OR1K_PIC
> > >  	bool
> > >  	select IRQ_DOMAIN
> > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > > index e88d856cc09c..123047d7a20d 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -17,6 +17,7 @@ obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
> > >  obj-$(CONFIG_METAG)			+= irq-metag-ext.o
> > >  obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
> > >  obj-$(CONFIG_CLPS711X_IRQCHIP)		+= irq-clps711x.o
> > > +obj-$(CONFIG_OMPIC)			+= irq-ompic.o
> > >  obj-$(CONFIG_OR1K_PIC)			+= irq-or1k-pic.o
> > >  obj-$(CONFIG_ORION_IRQCHIP)		+= irq-orion.o
> > >  obj-$(CONFIG_OMAP_IRQCHIP)		+= irq-omap-intc.o
> > > diff --git a/drivers/irqchip/irq-ompic.c b/drivers/irqchip/irq-ompic.c
> > > new file mode 100644
> > > index 000000000000..cd2616b6639b
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-ompic.c
> > > @@ -0,0 +1,205 @@
> > > +/*
> > > + * Open Multi-Processor Interrupt Controller driver
> > > + *
> > > + * Copyright (C) 2014 Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > > + * Copyright (C) 2017 Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > + *
> > > + * This file is licensed under the terms of the GNU General Public License
> > > + * version 2.  This program is licensed "as is" without any warranty of any
> > > + * kind, whether express or implied.
> > > + *
> > > + * The ompic device handles IPI communication because cores in mulicore
> > > + * OpenRISC systems.
> > 
> > Should the above read "between cores"?
> 
> Yes, it should be, I am bad with these kind of typos.

And "multi-core"
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Stafford Horne]

On Mon, Sep 18, 2017 at 03:29:58PM -0500, Rob Herring wrote:
> On Thu, Sep 14, 2017 at 03:54:02PM +0900, Stafford Horne wrote:
> > On Wed, Sep 13, 2017 at 06:21:39PM +0100, Marc Zyngier wrote:
> > > On Sun, Sep 10 2017 at  3:49:18 pm BST, Stafford Horne <shorne@gmail.com> wrote:
> > > > From: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> > > >
[..]
> > > > diff --git a/drivers/irqchip/irq-ompic.c b/drivers/irqchip/irq-ompic.c
> > > > new file mode 100644
> > > > index 000000000000..cd2616b6639b
> > > > --- /dev/null
> > > > +++ b/drivers/irqchip/irq-ompic.c
> > > > @@ -0,0 +1,205 @@
> > > > +/*
> > > > + * Open Multi-Processor Interrupt Controller driver
> > > > + *
> > > > + * Copyright (C) 2014 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> > > > + * Copyright (C) 2017 Stafford Horne <shorne@gmail.com>
> > > > + *
> > > > + * This file is licensed under the terms of the GNU General Public License
> > > > + * version 2.  This program is licensed "as is" without any warranty of any
> > > > + * kind, whether express or implied.
> > > > + *
> > > > + * The ompic device handles IPI communication because cores in mulicore
> > > > + * OpenRISC systems.
> > > 
> > > Should the above read "between cores"?
> > 
> > Yes, it should be, I am bad with these kind of typos.
> 
> And "multi-core"

Thanks, and I have many s/multicore/multi-core/ issues throughout the patch
series as well.

-Stafford

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Rob Herring]

On Sun, Sep 10, 2017 at 03:49:18PM +0900, Stafford Horne wrote:
> From: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> 
> IPI driver for the Open Multi-Processor Interrupt Controller (ompic) as
> described in the Multicore support section of the OpenRISC 1.2
> proposed architecture specification:
> 
>   https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf
> 
> Each OpenRISC core contains a full interrupt controller which is used in
> the SMP architecture for interrupt balancing.  This IPI device, the
> ompic, is the only external device required for enabling SMP on
> OpenRISC.
> 
> Pending ops are stored in a memory bit mask which can allow multiple
> pending operations to be set and serviced at a time. This is mostly
> borrowed from the alpha IPI implementation.
> 
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> [shorne@gmail.com: converted ops to bitmask, wrote commit message]
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
> 
> Changes since v1
>  - Added openrisc, prefix
>  - Clarified 8 bytes per cpu
>  - Removed #interrupt-cells as this will not be an irq parent

You should still have #interrupt-cells as that is required with 
"interrupt-controller". It could be 0 though.

>  - Changed ops to be percpu
>  - Added DTS and intialization failure validations
> 
>  .../interrupt-controller/openrisc,ompic.txt        |  19 ++
>  arch/openrisc/Kconfig                              |   1 +
>  drivers/irqchip/Kconfig                            |   3 +
>  drivers/irqchip/Makefile                           |   1 +
>  drivers/irqchip/irq-ompic.c                        | 205 +++++++++++++++++++++
>  5 files changed, 229 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
>  create mode 100644 drivers/irqchip/irq-ompic.c
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> new file mode 100644
> index 000000000000..346e6042d62f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> @@ -0,0 +1,19 @@
> +Open Multi-Processor Interrupt Controller
> +
> +Required properties:
> +
> +- compatible : This should be "openrisc,ompic"
> +- reg : Specifies base physical address and size of the register space. The
> +  size is based on the number of cores the controller has been configured
> +  to handle, this should be set to 8 bytes per cpu core.
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- interrupts : Specifies the interrupt line to which the ompic is wired.
> +
> +Example:
> +
> +ompic: ompic {

interrupt-controller@98000000 {

> +	compatible = "openrisc,ompic";
> +	reg = <0x98000000 16>;
> +	interrupt-controller;
> +	interrupts = <1>;
> +};

Re: [PATCH v2 06/14] irqchip: add initial support for ompic

[Stafford Horne]

On Mon, Sep 18, 2017 at 03:43:39PM -0500, Rob Herring wrote:
> On Sun, Sep 10, 2017 at 03:49:18PM +0900, Stafford Horne wrote:
> > From: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > 
> > IPI driver for the Open Multi-Processor Interrupt Controller (ompic) as
> > described in the Multicore support section of the OpenRISC 1.2
> > proposed architecture specification:
> > 
> >   https://github.com/stffrdhrn/doc/raw/arch-1.2-proposal/openrisc-arch-1.2-rev0.pdf
> > 
> > Each OpenRISC core contains a full interrupt controller which is used in
> > the SMP architecture for interrupt balancing.  This IPI device, the
> > ompic, is the only external device required for enabling SMP on
> > OpenRISC.
> > 
> > Pending ops are stored in a memory bit mask which can allow multiple
> > pending operations to be set and serviced at a time. This is mostly
> > borrowed from the alpha IPI implementation.
> > 
> > Signed-off-by: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > [shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: converted ops to bitmask, wrote commit message]
> > Signed-off-by: Stafford Horne <shorne-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> > 
> > Changes since v1
> >  - Added openrisc, prefix
> >  - Clarified 8 bytes per cpu
> >  - Removed #interrupt-cells as this will not be an irq parent
> 
> You should still have #interrupt-cells as that is required with 
> "interrupt-controller". It could be 0 though.

Thanks, I didn't notice that from reading the code.  I will update it.

> >  - Changed ops to be percpu
> >  - Added DTS and intialization failure validations
> > 
> >  .../interrupt-controller/openrisc,ompic.txt        |  19 ++
> >  arch/openrisc/Kconfig                              |   1 +
> >  drivers/irqchip/Kconfig                            |   3 +
> >  drivers/irqchip/Makefile                           |   1 +
> >  drivers/irqchip/irq-ompic.c                        | 205 +++++++++++++++++++++
> >  5 files changed, 229 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> >  create mode 100644 drivers/irqchip/irq-ompic.c
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > new file mode 100644
> > index 000000000000..346e6042d62f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/openrisc,ompic.txt
> > @@ -0,0 +1,19 @@
> > +Open Multi-Processor Interrupt Controller
> > +
> > +Required properties:
> > +
> > +- compatible : This should be "openrisc,ompic"
> > +- reg : Specifies base physical address and size of the register space. The
> > +  size is based on the number of cores the controller has been configured
> > +  to handle, this should be set to 8 bytes per cpu core.
> > +- interrupt-controller : Identifies the node as an interrupt controller
> > +- interrupts : Specifies the interrupt line to which the ompic is wired.
> > +
> > +Example:
> > +
> > +ompic: ompic {
> 
> interrupt-controller@98000000 {

OK, I will change to the format. But I notice many other docs like this:

  ompic: interrupt-controller@98000000 {

> > +	compatible = "openrisc,ompic";
> > +	reg = <0x98000000 16>;
> > +	interrupt-controller;
> > +	interrupts = <1>;
> > +};

Thanks for the review.

-Stafford
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/14] openrisc: add simple_smp dts and defconfig for simulators

[Stafford Horne]

From: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>

Simple enough to be compatible with simulation environments,
such as verilated systems, QEMU and other targets supporting OpenRISC
SMP.  This also supports our base FPGA SoC's if the cpu frequency is
upped to 50Mhz.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
[shorne@gmail.com: Added defconfig]
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
Changes since v1
 - Use openrisc, prefix for ompic
 - Add stdout-path
 - Remove @interrupt cells

 arch/openrisc/boot/dts/simple_smp.dts      | 58 ++++++++++++++++++++++++++
 arch/openrisc/configs/simple_smp_defconfig | 66 ++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)
 create mode 100644 arch/openrisc/boot/dts/simple_smp.dts
 create mode 100644 arch/openrisc/configs/simple_smp_defconfig

diff --git a/arch/openrisc/boot/dts/simple_smp.dts b/arch/openrisc/boot/dts/simple_smp.dts
new file mode 100644
index 000000000000..2d367f23c976
--- /dev/null
+++ b/arch/openrisc/boot/dts/simple_smp.dts
@@ -0,0 +1,58 @@
+/dts-v1/;
+/ {
+	compatible = "opencores,or1ksim";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&pic>;
+
+	chosen {
+		bootargs = "console=uart,mmio,0x90000000,115200";
+		stdout-path = &serial0;
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x00000000 0x02000000>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu@0 {
+			compatible = "opencores,or1200-rtlsvn481";
+			reg = <0>;
+			clock-frequency = <20000000>;
+		};
+		cpu@1 {
+			compatible = "opencores,or1200-rtlsvn481";
+			reg = <1>;
+			clock-frequency = <20000000>;
+		};
+	};
+
+	ompic: ompic {
+		compatible = "openrisc,ompic";
+		reg = <0x98000000 16>;
+		interrupt-controller;
+		interrupts = <1>;
+	};
+
+	/*
+	 * OR1K PIC is built into CPU and accessed via special purpose
+	 * registers.  It is not addressable and, hence, has no 'reg'
+	 * property.
+	 */
+	pic: pic {
+		compatible = "opencores,or1k-pic-level";
+		#interrupt-cells = <1>;
+		interrupt-controller;
+	};
+
+	serial0: serial@90000000 {
+		compatible = "opencores,uart16550-rtlsvn105", "ns16550a";
+		reg = <0x90000000 0x100>;
+		interrupts = <2>;
+		clock-frequency = <20000000>;
+	};
+
+};
diff --git a/arch/openrisc/configs/simple_smp_defconfig b/arch/openrisc/configs/simple_smp_defconfig
new file mode 100644
index 000000000000..b6e3c7e158e7
--- /dev/null
+++ b/arch/openrisc/configs/simple_smp_defconfig
@@ -0,0 +1,66 @@
+CONFIG_CROSS_COMPILE="or1k-linux-"
+CONFIG_LOCALVERSION="-simple-smp"
+CONFIG_NO_HZ=y
+CONFIG_LOG_BUF_SHIFT=14
+CONFIG_BLK_DEV_INITRD=y
+# CONFIG_RD_GZIP is not set
+# CONFIG_RD_BZIP2 is not set
+# CONFIG_RD_LZMA is not set
+# CONFIG_RD_XZ is not set
+# CONFIG_RD_LZO is not set
+# CONFIG_RD_LZ4 is not set
+CONFIG_EXPERT=y
+# CONFIG_KALLSYMS is not set
+# CONFIG_EPOLL is not set
+# CONFIG_TIMERFD is not set
+# CONFIG_EVENTFD is not set
+# CONFIG_AIO is not set
+# CONFIG_VM_EVENT_COUNTERS is not set
+# CONFIG_COMPAT_BRK is not set
+CONFIG_SLOB=y
+CONFIG_MODULES=y
+# CONFIG_BLOCK is not set
+CONFIG_OPENRISC_BUILTIN_DTB="simple_smp"
+CONFIG_SMP=y
+CONFIG_HZ_100=y
+CONFIG_OPENRISC_HAVE_SHADOW_GPRS=y
+CONFIG_NET=y
+CONFIG_PACKET=y
+CONFIG_UNIX=y
+CONFIG_INET=y
+# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
+# CONFIG_INET_XFRM_MODE_TUNNEL is not set
+# CONFIG_INET_XFRM_MODE_BEET is not set
+# CONFIG_INET_DIAG is not set
+CONFIG_TCP_CONG_ADVANCED=y
+# CONFIG_TCP_CONG_BIC is not set
+# CONFIG_TCP_CONG_CUBIC is not set
+# CONFIG_TCP_CONG_WESTWOOD is not set
+# CONFIG_TCP_CONG_HTCP is not set
+# CONFIG_IPV6 is not set
+# CONFIG_WIRELESS is not set
+CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
+# CONFIG_PREVENT_FIRMWARE_BUILD is not set
+# CONFIG_FW_LOADER is not set
+CONFIG_NETDEVICES=y
+CONFIG_ETHOC=y
+CONFIG_MICREL_PHY=y
+# CONFIG_WLAN is not set
+# CONFIG_INPUT is not set
+# CONFIG_SERIO is not set
+# CONFIG_VT is not set
+# CONFIG_LEGACY_PTYS is not set
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
+# CONFIG_HW_RANDOM is not set
+# CONFIG_HWMON is not set
+# CONFIG_USB_SUPPORT is not set
+# CONFIG_DNOTIFY is not set
+CONFIG_TMPFS=y
+CONFIG_NFS_FS=y
+CONFIG_XZ_DEC=y
+# CONFIG_ENABLE_WARN_DEPRECATED is not set
+# CONFIG_ENABLE_MUST_CHECK is not set
+# CONFIG_RCU_TRACE is not set
-- 
2.13.5