OpenRISC de0 nano single and multicore boards

Since v2:
- Fixup gpio-mmio patch to update driver compatible list and just add opencores,gpio to mmio-gpio bindings. Discussed with Geert and Linus W [1].
- Update new device trees to use proper ordering, remove debug options, remove unneeded "status" properties. Suggested by Geert.
Since v1:
- Use proper schema in gpio-mmio suggsted by Conor Dooley
- Remove 0 clock-frequency definitions in dtsi file

The patches add support for OpenRISC systems running on the De0 Nano FPGA development board. We have two SoCs which are available here:

- https://github.com/olofk/de0_nano - Single core
- https://github.com/stffrdhrn/de0_nano-multicore - Multicore

As I work on tutorials [2] to help other get started with OpenRISC I would like to have these defconfig and devicetree definitions in the upstream kernel to avoid losing them.

When I was working on resurrecting these old setup's I found a major bug in OpenRISC SMP which is fixed in this series as well.

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://openrisc.io/tutorials/

Stafford Horne (6):
gpio: mmio: Add compatible for opencores GPIO
dt-bindings: gpio-mmio: Add opencores GPIO
openrisc: dts: Add de0 nano config and devicetree
openrisc: Fix IPIs on simple multicore systems
openrisc: dts: Split simple smp dts to dts and dtsi
openrisc: dts: Add de0 nano multicore config and devicetree

 .../devicetree/bindings/gpio/gpio-mmio.yaml   |  9 ++
 arch/openrisc/boot/dts/de0-nano-common.dtsi   | 42 +++++++++
 arch/openrisc/boot/dts/de0-nano-multicore.dts | 25 +++++
 arch/openrisc/boot/dts/de0-nano.dts           | 54 +++++++++++
 arch/openrisc/boot/dts/simple-smp.dts         | 25 +++++
 .../dts/{simple_smp.dts => simple-smp.dtsi}   | 11 +--
 arch/openrisc/configs/de0_nano_defconfig      | 79 ++++++++++++++++
 .../configs/de0_nano_multicore_defconfig      | 92 +++++++++++++++++++
 arch/openrisc/configs/simple_smp_defconfig    |  2 +-
 arch/openrisc/include/asm/smp.h               |  3 +-
 arch/openrisc/kernel/smp.c                    | 22 ++++-
 drivers/gpio/gpio-mmio.c                      |  1 +
 drivers/irqchip/irq-ompic.c                   | 15 ++-
 drivers/irqchip/irq-or1k-pic.c                | 27 +++++-
 14 files changed, 393 insertions(+), 14 deletions(-)
 create mode 100644 arch/openrisc/boot/dts/de0-nano-common.dtsi
 create mode 100644 arch/openrisc/boot/dts/de0-nano-multicore.dts
 create mode 100644 arch/openrisc/boot/dts/de0-nano.dts
 create mode 100644 arch/openrisc/boot/dts/simple-smp.dts
 rename arch/openrisc/boot/dts/{simple_smp.dts => simple-smp.dtsi} (90%)
 create mode 100644 arch/openrisc/configs/de0_nano_defconfig
 create mode 100644 arch/openrisc/configs/de0_nano_multicore_defconfig

2.51.0

On FPGA Development boards with GPIOs the OpenRISC architecture uses the opencores gpio verilog rtl. This is compatible with the gpio-mmio. Add the compatible string to allow probing this driver from the devicetree.

---
Since v2:
- New patch

 drivers/gpio/gpio-mmio.c | 1 +
 1 file changed, 1 insertion(+)

index 5daf962b0323..edbcaad57d00 100644
@@ -724,6 +724,7 @@ static const struct of_device_id gpio_mmio_of_match[] = {
 	{ .compatible = "wd,mbl-gpio" },
 	{ .compatible = "ni,169445-nand-gpio" },
 	{ .compatible = "intel,ixp4xx-expansion-bus-mmio-gpio" },
+	{ .compatible = "opencores,gpio" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, gpio_mmio_of_match);

2.51.0

Yours,
Linus Walleij

Gr{oetje,eeting}s,

Geert

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Add a device tree binding for the opencores GPIO controller.

On FPGA Development boards with GPIOs the OpenRISC architecture uses the opencores gpio verilog rtl which is compatible with the MMIO GPIO driver.

---
Since v2:
- Fixup patch to simply add opencores,gpio and add an example.
Since v1:
- Fix schema to actually match the example.

 Documentation/devicetree/bindings/gpio/gpio-mmio.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

index ee5d5d25ae82..d44edc181e0a 100644
@@ -23,6 +23,7 @@ properties:
       - ni,169445-nand-gpio
       - wd,mbl-gpio # Western Digital MyBook Live memory-mapped GPIO controller
       - intel,ixp4xx-expansion-bus-mmio-gpio
+      - opencores,gpio
 
   big-endian: true
 

@@ -160,3 +161,11 @@ examples:
             intel,ixp4xx-eb-write-enable = <1>;
         };
     };
+
+    gpio@91000000 {
+        compatible = "opencores,gpio";
+        reg = <0x91000000 0x1>, <0x91000001 0x1>;
+        reg-names = "dat", "dirout";
+        gpio-controller;
+        #gpio-cells = <2>;
+    };

2.51.0

Yours,
Linus Walleij

Gr{oetje,eeting}s,

Geert

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Simplify? You completely changed the meaning of binding here - now device is not compatible.

I don't know which one is correct, but your changelog must explain why now devices are not compatible but they were before.

Hello,

Did you miss the 1/6 patch in this series? We add the compatible string to the driver there before we add it here.

Sorry, I thought the series and the over letter would be enough to understand what I meant by the "Fixup" description here.

I figured this is a new compatible string with a different set of 8 bit registers so having this example would be beneficial.

There is no 1/6!

How does it matter? How can you add something to the driver before you document the ABI? Did you read the submitting patches doc?

You still did not answer to my comments.

Best regards,
Krzysztof

Trying to answer this better this time:

As per our discussion with Geert and Linus W. It was pointed out that the original patch, which added openrisc,gpio to be allowed along with the broadcom chip e.g. ( compatible = "opencores,gpio", "brcm,bcm6345-gpio"; ), was wrong.

The opencores,gpio is compatible with the gpio-mmio driver, but it is not a hardware clone with the broadcomm chip. It has 8-bit registers vs 32-bit registers and the register map is different. Instead of allowing opencores,gpio to be specified along with the broadcom chip, opencores,gpio should be specified on its own.

So we agreed to resend the patch with to parts:

1. A commit to add the opencores,gpio to the driver compatibility list. (new

    1/6)

2. A commit to add opencores,gpio to the binding (replacement of the original patch 2/6)

(now I understand this order is bad, I can resend)

This is a "simplification" as we are now just adding the opencores,gpio string to the list rather than changing the schema with oneOf and items.

I wanted top get it out quickly so it can be fixed up before the merge window opens.

It seems you are not on it, but it is on lore here, if you missed it.

https://lore.kernel.org/lkml/[email protected]/

Reading the bindings submitting patches doc's it seems I need to send the whole series to the bindings list. Which may explain.

Sorry, I didn't read, or realize there was a device tree bindings specific patch document. I see it now, and I see point 5 makes it clear that we should document the binding before the code change. I got the order swapped.

https://docs.kernel.org/devicetree/bindings/submitting-patches.html

If necessary I can resend the 2 patches in the right order as a series to the devicetree list. [email protected]

OK, I tried again above.

-stafford

This does not even apply now. Your previous version was applied almost one month before and you WERE notified about it.

Why did you ignore Bartosz's reply?

Best regards,
Krzysztof

Hi Krzysztof,

Sorry for the confusion, we discussed [1] this patch in the last Series with Geert and Linus W. We decided to Fixup (replace) the original patch with this and a patch to the GPIO MMIO driver instead. I pointed it out in the cover letter but it may have not been clear.

I will reply to Bartosz's original mail too so he can understand the intent.

[1] https://lore.kernel.org/lkml/CAD++jLm1u9ChqsftwvbOptiG3Qo2KWxPjqN2snOVuZDYuVST5Q@mail.gmail.com/

-Stafford

But you did not send any fixup, anything which would change what was applied almost one month ago!

Best regards,
Krzysztof

The de0 nano from Terasic is an FPGA board that we use in the OpenRISC community to test OpenRISC configurations. Add a base configuration for the board that runs an OpenRISC CPU at 50Mhz with 32MB ram, UART for console and some GPIOs for LEDs and switches.

There is an older version of this floating around that defines all of the hardware on the board including SPI's, flash devices, sram, ADCs etc. Eventually it would be good to get the full version upstream but for now I think a minimal board is good to start with.

---
Since v2:
- Move leds block up to the top.
- Remove unneeded "status" from gpio0.
- Removed earlycon from de0-nano.dts.

 arch/openrisc/boot/dts/de0-nano-common.dtsi | 42 +++++++++++
 arch/openrisc/boot/dts/de0-nano.dts         | 54 ++++++++++++++
 arch/openrisc/configs/de0_nano_defconfig    | 79 +++++++++++++++++++++
 3 files changed, 175 insertions(+)
 create mode 100644 arch/openrisc/boot/dts/de0-nano-common.dtsi
 create mode 100644 arch/openrisc/boot/dts/de0-nano.dts
 create mode 100644 arch/openrisc/configs/de0_nano_defconfig

new file mode 100644
index 000000000000..02e329e28e33
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	leds0: leds {
+		compatible = "gpio-leds";
+
+		led-heartbeat {
+			gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_HEARTBEAT;
+			linux,default-trigger = "heartbeat";
+			label = "heartbeat";
+		};
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x00000000 0x02000000>;
+	};
+
+	/* 8 Green LEDs */
+	gpio0: gpio@91000000 {
+		compatible = "opencores,gpio";
+		reg = <0x91000000 0x1>, <0x91000001 0x1>;
+		reg-names = "dat", "dirout";
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	/* 4 DIP Switches */
+	gpio1: gpio@92000000 {
+		compatible = "opencores,gpio";
+		reg = <0x92000000 0x1>, <0x92000001 0x1>;
+		reg-names = "dat", "dirout";
+		gpio-controller;
+		#gpio-cells = <2>;
+		status = "disabled";
+	};
+};

2.51.0

Commit c05671846451 ("openrisc: sleep instead of spin on secondary wait") fixed OpenRISC SMP Linux for QEMU. However, stability was never achieved on FPGA development boards. This is because the above patch has a step to unmask IPIs on non-boot cpu's but on hardware without power management, IPIs remain masked.

This meant that IPI's were never actually working on the simple SMP systems we run on development boards. The systems booted but stability was very suspect.

Add the ability to unmask IPI's on the non-boot cores. This is done by making the OMPIC IRQs proper percpu IRQs. We can then use the enabled_percpu_irq() to unmask IRQ on the non-boot cpus.

Update the or1k PIC driver to use a flow handler that can switch between percpu and the configured level or edge flow handlers at runtime. This mechanism is inspired by that done in the J-Core AIC driver.

---
 arch/openrisc/include/asm/smp.h |  3 ++-
 arch/openrisc/kernel/smp.c      | 22 +++++++++++++++++++++-
 drivers/irqchip/irq-ompic.c     | 15 +++++++++++----
 drivers/irqchip/irq-or1k-pic.c  | 27 ++++++++++++++++++++++++++-
 4 files changed, 60 insertions(+), 7 deletions(-)

index e21d2f12b5b6..007296f160ef 100644
@@ -20,7 +20,8 @@ extern void smp_init_cpus(void);
 extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 
-extern void set_smp_cross_call(void (*)(const struct cpumask *, unsigned int));
+extern void set_smp_cross_call(void (*)(const struct cpumask *, unsigned int),
+			       unsigned int irq);
 extern void handle_IPI(unsigned int ipi_msg);
 
 #endif /* __ASM_OPENRISC_SMP_H */

index 86da4bc5ee0b..040ca201b692 100644
@@ -13,6 +13,7 @@
 
 #include <linux/smp.h>
 #include <linux/cpu.h>
+#include <linux/interrupt.h>
 #include <linux/sched.h>
 #include <linux/sched/mm.h>
 #include <linux/irq.h>

@@ -25,6 +26,7 @@
 
 asmlinkage __init void secondary_start_kernel(void);
 
+static unsigned int ipi_irq __ro_after_init;
 static void (*smp_cross_call)(const struct cpumask *, unsigned int);
 
 unsigned long secondary_release = -1;

@@ -39,6 +41,14 @@ enum ipi_msg_type {
 
 static DEFINE_SPINLOCK(boot_lock);
 
+static void or1k_ipi_enable(void)
+{
+	if (WARN_ON_ONCE(!ipi_irq))
+		return;
+
+	enable_percpu_irq(ipi_irq, 0);
+}
+
 static void boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	/*

@@ -136,6 +146,7 @@ asmlinkage __init void secondary_start_kernel(void)
 	complete(&cpu_running);
 
 	synchronise_count_slave(cpu);
+	or1k_ipi_enable();
 	set_cpu_online(cpu, true);
 
 	local_irq_enable();

@@ -195,9 +206,18 @@ void smp_send_stop(void)
 	smp_call_function(stop_this_cpu, NULL, 0);
 }
 
-void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
+void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int),
+			       unsigned int irq)
 {
+	if (WARN_ON(ipi_irq))
+		return;
+
 	smp_cross_call = fn;
+
+	ipi_irq = irq;
+
+	/* Enabled IPIs for boot CPU immediately */
+	or1k_ipi_enable();
 }
 
 void arch_send_call_function_single_ipi(int cpu)

index e66ef4373b1e..f0e0b435bb1d 100644
@@ -84,6 +84,8 @@ DEFINE_PER_CPU(unsigned long, ops);
 
 static void __iomem *ompic_base;
 
+static DEFINE_PER_CPU_READ_MOSTLY(int, ipi_dummy_dev);
+
 static inline u32 ompic_readreg(void __iomem *base, loff_t offset)
 {
 	return ioread32be(base + offset);

@@ -183,12 +185,17 @@ static int __init ompic_of_init(struct device_node *node,
 		goto out_unmap;
 	}
 
-	ret = request_irq(irq, ompic_ipi_handler, IRQF_PERCPU,
-				"ompic_ipi", NULL);
-	if (ret)
+	irq_set_percpu_devid(irq);
+	ret = request_percpu_irq(irq, ompic_ipi_handler, "ompic_ipi",
+				 &ipi_dummy_dev);
+
+	if (ret) {
+		pr_err("ompic: failed to request irq %d, error: %d",
+		       irq, ret);
 		goto out_irq_disp;
+	}
 
-	set_smp_cross_call(ompic_raise_softirq);
+	set_smp_cross_call(ompic_raise_softirq, irq);
 
 	return 0;
 

index 48126067c54b..73dc99c71d40 100644
@@ -118,11 +118,36 @@ static void or1k_pic_handle_irq(struct pt_regs *regs)
 		generic_handle_domain_irq(root_domain, irq);
 }
 
+/*
+ * The OR1K PIC is a cpu-local interrupt controller and does not distinguish or
+ * use distinct irq number ranges for per-cpu event interrupts (IPI). Since
+ * information to determine whether a particular irq number should be treated as
+ * per-cpu is not available at mapping time, we use a wrapper handler function
+ * which chooses the right handler at runtime based on whether IRQF_PERCPU was
+ * used when requesting the irq.  Borrowed from J-Core AIC.
+ */
+static void or1k_irq_flow_handler(struct irq_desc *desc)
+{
+#ifdef CONFIG_SMP
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct or1k_pic_dev *pic = data->domain->host_data;
+
+	if (irqd_is_per_cpu(data))
+		handle_percpu_devid_irq(desc);
+	else
+		pic->handle(desc);
+#endif
+}
+
 static int or1k_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 {
 	struct or1k_pic_dev *pic = d->host_data;
 
-	irq_set_chip_and_handler(irq, &pic->chip, pic->handle);
+	if (IS_ENABLED(CONFIG_SMP))
+		irq_set_chip_and_handler(irq, &pic->chip, or1k_irq_flow_handler);
+	else
+		irq_set_chip_and_handler(irq, &pic->chip, pic->handle);
+
 	irq_set_status_flags(irq, pic->flags);
 
 	return 0;

2.51.0

Split out the common memory, CPU and PIC definitions of the simple SMP system to a DTSI file which we will later use for our De0 Nano multicore board device tree. We also take this opportunity to swich underscores to dashes as that seems to be the more common convention for DTS files.

---
Since v2:
- Sort blocks alphabetically.

 arch/openrisc/boot/dts/simple-smp.dts         | 25 +++++++++++++++++++
 .../dts/{simple_smp.dts => simple-smp.dtsi}   | 11 ++++----
 arch/openrisc/configs/simple_smp_defconfig    |  2 +-
 3 files changed, 31 insertions(+), 7 deletions(-)
 create mode 100644 arch/openrisc/boot/dts/simple-smp.dts
 rename arch/openrisc/boot/dts/{simple_smp.dts => simple-smp.dtsi} (90%)

new file mode 100644
index 000000000000..01cf219e6aac
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/dts-v1/;
+
+#include "simple-smp.dtsi"
+
+/ {
+	model = "Simple SMP Board";
+};
+
+&cpu0 {
+	clock-frequency = <20000000>;
+};
+
+&cpu1 {
+	clock-frequency = <20000000>;
+};
+
+&enet0 {
+	status = "okay";
+};
+
+&serial0 {
+	clock-frequency = <20000000>;
+};

similarity index 90%
rename from arch/openrisc/boot/dts/simple_smp.dts
rename to arch/openrisc/boot/dts/simple-smp.dtsi
index 71af0e117bfe..42d6eda33b71 100644
@@ -1,4 +1,3 @@
-/dts-v1/;
 / {
 	compatible = "opencores,or1ksim";
 	#address-cells = <1>;

@@ -22,15 +21,15 @@ memory@0 {
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		cpu@0 {
+
+		cpu0: cpu@0 {
 			compatible = "opencores,or1200-rtlsvn481";
 			reg = <0>;
-			clock-frequency = <20000000>;
 		};
-		cpu@1 {
+
+		cpu1: cpu@1 {
 			compatible = "opencores,or1200-rtlsvn481";
 			reg = <1>;
-			clock-frequency = <20000000>;
 		};
 	};
 

@@ -57,7 +56,6 @@ serial0: serial@90000000 {
 		compatible = "opencores,uart16550-rtlsvn105", "ns16550a";
 		reg = <0x90000000 0x100>;
 		interrupts = <2>;
-		clock-frequency = <20000000>;
 	};
 
 	enet0: ethoc@92000000 {

@@ -65,5 +63,6 @@ enet0: ethoc@92000000 {
 		reg = <0x92000000 0x800>;
 		interrupts = <4>;
 		big-endian;
+		status = "disabled";
 	};
 };

index 6008e824d31c..db77c795225e 100644
@@ -20,7 +20,7 @@ CONFIG_SLUB=y
 CONFIG_SLUB_TINY=y
 CONFIG_MODULES=y
 # CONFIG_BLOCK is not set
-CONFIG_BUILTIN_DTB_NAME="simple_smp"
+CONFIG_BUILTIN_DTB_NAME="simple-smp"
 CONFIG_SMP=y
 CONFIG_HZ_100=y
 CONFIG_OPENRISC_HAVE_SHADOW_GPRS=y

2.51.0

Add a multicore configuration for the Terasic de0 nano FPGA development board. This SoC runs 2 OpenRISC CPUs at 50Mhz with 32MB ram, UART for console and GPIOs for LEDs.

This FPGA SoC is based on the simple-smp reference board and brings in devices from the de0 nano common DTSI file.

A default config is added that brings together the device tree and driver setup.

---
 arch/openrisc/boot/dts/de0-nano-multicore.dts | 25 +++++
 .../configs/de0_nano_multicore_defconfig      | 92 +++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 arch/openrisc/boot/dts/de0-nano-multicore.dts
 create mode 100644 arch/openrisc/configs/de0_nano_multicore_defconfig

new file mode 100644
index 000000000000..b6cf286afaa4
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+/dts-v1/;
+
+#include "simple-smp.dtsi"
+#include "de0-nano-common.dtsi"
+
+/ {
+	model = "Terasic DE0 Nano - Multicore";
+};
+
+&cpu0 {
+	clock-frequency = <50000000>;
+};
+
+&cpu1 {
+	clock-frequency = <50000000>;
+};
+
+&serial0 {
+	clock-frequency = <50000000>;
+};

2.51.0