Skip to contentSkip to conversation

Support for RTL930x/RTL931x GPIOs

PATCH Applied Sander Vanheule 5 participants 18 messages Apr 9, 2022 – Apr 22, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Support for RTL930x/RTL931x GPIOs

This patch series adds support for the GPIO controllers as found on the RTL930x and RTL931x SoC families of MIPS CPUs, used in managed NBase-T ethernet switches.

The RTL931x's GPIO controller behaves the same as the one in the older RTL838x and RTL839x series. This controller is trivially supported.

The RTL930x's controller has a reversed port order, and supports CPU affinity settings for individual GPIO line IRQs, thus requiring two additional changes to implement these features.

Sander Vanheule (6):
dt-bindings: gpio: realtek-otto: Add rtl9300 compatible
gpio: realtek-otto: Support reversed port layouts
gpio: realtek-otto: Support per-cpu interrupts
gpio: realtek-otto: Add RTL930x support
dt-bindings: gpio: realtek-otto: Add rtl9310 compatible
gpio: realtek-otto: Add RTL931x support

 .../bindings/gpio/realtek,otto-gpio.yaml      |  34 ++++-
 drivers/gpio/gpio-realtek-otto.c              | 137 +++++++++++++++++-
 2 files changed, 164 insertions(+), 7 deletions(-)
Signature · 1 line
2.35.1
#1 lore raw

PATCH 1/6 dt-bindings: gpio: realtek-otto: Add rtl9300 compatible

Add the "realtek,rlt9300-gpio", "realtek,otto-gpio" compatible for GPIO nodes on the RTL930x SoC series. This SoC requires an extra register range, defining the interrupt CPU mapping registers.

---
 .../bindings/gpio/realtek,otto-gpio.yaml      | 33 +++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml +31 -2
index 100f20cebd76..3c511e9af377 100644
@@ -28,10 +28,10 @@ properties:
       - enum:
           - realtek,rtl8380-gpio
           - realtek,rtl8390-gpio
+          - realtek,rtl9300-gpio
       - const: realtek,otto-gpio
 
-  reg:
-    maxItems: 1
+  reg: true
 
   "#gpio-cells":
     const: 2
@@ -50,6 +50,23 @@ properties:
   interrupts:
     maxItems: 1
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: realtek,rtl9300-gpio
+then:
+  properties:
+    reg:
+      items:
+        - description: GPIO and interrupt control
+        - description: interrupt CPU map
+else:
+  properties:
+    reg:
+      items:
+        - description: GPIO and interrupt control
+
 required:
   - compatible
   - reg
@@ -74,5 +91,17 @@ examples:
         interrupt-parent = <&rtlintc>;
         interrupts = <23>;
       };
+  - |
+      gpio@3300 {
+        compatible = "realtek,rtl9300-gpio", "realtek,otto-gpio";
+        reg = <0x3300 0x1c>, <0x3338 0x8>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        ngpios = <24>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        interrupt-parent = <&rtlintc>;
+        interrupts = <13>;
+      };
 
 ...
Signature · 1 line
2.35.1
Reviewed-by on v1 1/6
#2 lore raw

Re: [PATCH v1 1/6] dt-bindings: gpio: realtek-otto: Add rtl9300 compatible

Quoted 27 lines · Sander Vanheule

Add the "realtek,rlt9300-gpio", "realtek,otto-gpio" compatible for GPIO nodes on the RTL930x SoC series. This SoC requires an extra register range, defining the interrupt CPU mapping registers.

---
 .../bindings/gpio/realtek,otto-gpio.yaml      | 33 +++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml +2 -2
index 100f20cebd76..3c511e9af377 100644
@@ -28,10 +28,10 @@ properties:
       - enum:
           - realtek,rtl8380-gpio
           - realtek,rtl8390-gpio
+          - realtek,rtl9300-gpio
       - const: realtek,otto-gpio
 
-  reg:
-    maxItems: 1
+  reg: true
 
   "#gpio-cells":
     const: 2

Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml properties: · line 53

Quoted 4 lines
Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml +1 -0
@@ -50,6 +50,23 @@ properties:
   interrupts:
     maxItems: 1
 
+if:

You could put it under allOf:, so if you ever need to extend it, you don't mess with the indentation, but it's fine as it is.

Best regards,
Krzysztof

#3 lore raw

Re: [PATCH v1 1/6] dt-bindings: gpio: realtek-otto: Add rtl9300 compatible

Hi Krzysztof,

Quoted 38 lines · Krzysztof Kozlowski
Quoted 28 lines · Sander Vanheule

Add the "realtek,rlt9300-gpio", "realtek,otto-gpio" compatible for GPIO nodes on the RTL930x SoC series. This SoC requires an extra register range, defining the interrupt CPU mapping registers.

---
 .../bindings/gpio/realtek,otto-gpio.yaml      | 33 +++++++++++++++++--

 1 file changed, 31 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml +0 -0
index 100f20cebd76..3c511e9af377 100644
@@ -28,10 +28,10 @@ properties:

       - enum:
           - realtek,rtl8380-gpio
           - realtek,rtl8390-gpio
+          - realtek,rtl9300-gpio
       - const: realtek,otto-gpio

-  reg:
-    maxItems: 1
+  reg: true

   "#gpio-cells":
     const: 2

Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml +0 -0
@@ -50,6 +50,23 @@ properties:
Quoted 4 lines

   interrupts:
     maxItems: 1

+if:

You could put it under allOf:, so if you ever need to extend it, you don't mess with the indentation, but it's fine as it is.

Quoted 1 line

Thanks for the review. I'll probably need to send a v2 of the series, so I'll update the indentation too. The commit messages have a typo ('rlt9300' and 'rlt9310') that needs to be fixed anyway.

Best,
Sander

#4 lore raw

PATCH 2/6 gpio: realtek-otto: Support reversed port layouts

In reply to Sander Vanheule

The GPIO port layout on the RTL930x SoC series is reversed compared to the RTL838x and RTL839x SoC series. Add new port offset calculator functions to ensure the correct order is used when reading port IRQ data, and ensure bgpio uses the right byte ordering.

---
 drivers/gpio/gpio-realtek-otto.c | 55 +++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 4 deletions(-)
drivers/gpio/gpio-realtek-otto.c +51 -4
index bd75401b549d..c838ad8ce55f 100644
@@ -58,6 +58,8 @@ struct realtek_gpio_ctrl {
 	raw_spinlock_t lock;
 	u16 intr_mask[REALTEK_GPIO_PORTS_PER_BANK];
 	u16 intr_type[REALTEK_GPIO_PORTS_PER_BANK];
+	unsigned int (*port_offset_u8)(unsigned int port);
+	unsigned int (*port_offset_u16)(unsigned int port);
 };
 
 /* Expand with more flags as devices with other quirks are added */
@@ -69,6 +71,11 @@ enum realtek_gpio_flags {
 	 * line the IRQ handler was assigned to, causing uncaught interrupts.
 	 */
 	GPIO_INTERRUPTS_DISABLED = BIT(0),
+	/*
+	 * Port order is reversed, meaning DCBA register layout for 1-bit
+	 * fields, and [BA, DC] for 2-bit fields.
+	 */
+	GPIO_PORTS_REVERSED = BIT(1),
 };
 
 static struct realtek_gpio_ctrl *irq_data_to_ctrl(struct irq_data *data)
@@ -86,21 +93,50 @@ static struct realtek_gpio_ctrl *irq_data_to_ctrl(struct irq_data *data)
  * port. The two interrupt mask registers store two bits per GPIO, so use u16
  * values.
  */
+static unsigned int realtek_gpio_port_offset_u8(unsigned int port)
+{
+	return port;
+}
+
+static unsigned int realtek_gpio_port_offset_u16(unsigned int port)
+{
+	return 2 * port;
+}
+
+/*
+ * Reversed port order register access
+ *
+ * For registers with one bit per GPIO, all ports are stored as u8-s in one
+ * register in reversed order. The two interrupt mask registers store two bits
+ * per GPIO, so use u16 values. The first register contains ports 1 and 0, the
+ * second ports 3 and 2.
+ */
+static unsigned int realtek_gpio_port_offset_u8_rev(unsigned int port)
+{
+	return 3 - port;
+}
+
+static unsigned int realtek_gpio_port_offset_u16_rev(unsigned int port)
+{
+	return 2 * (port ^ 1);
+}
+
 static void realtek_gpio_write_imr(struct realtek_gpio_ctrl *ctrl,
 	unsigned int port, u16 irq_type, u16 irq_mask)
 {
-	iowrite16(irq_type & irq_mask, ctrl->base + REALTEK_GPIO_REG_IMR + 2 * port);
+	iowrite16(irq_type & irq_mask,
+		ctrl->base + REALTEK_GPIO_REG_IMR + ctrl->port_offset_u16(port));
 }
 
 static void realtek_gpio_clear_isr(struct realtek_gpio_ctrl *ctrl,
 	unsigned int port, u8 mask)
 {
-	iowrite8(mask, ctrl->base + REALTEK_GPIO_REG_ISR + port);
+	iowrite8(mask, ctrl->base + REALTEK_GPIO_REG_ISR + ctrl->port_offset_u8(port));
 }
 
 static u8 realtek_gpio_read_isr(struct realtek_gpio_ctrl *ctrl, unsigned int port)
 {
-	return ioread8(ctrl->base + REALTEK_GPIO_REG_ISR + port);
+	return ioread8(ctrl->base + REALTEK_GPIO_REG_ISR + ctrl->port_offset_u8(port));
 }
 
 /* Set the rising and falling edge mask bits for a GPIO port pin */
@@ -250,6 +286,7 @@ MODULE_DEVICE_TABLE(of, realtek_gpio_of_match);
 static int realtek_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	unsigned long bgpio_flags;
 	unsigned int dev_flags;
 	struct gpio_irq_chip *girq;
 	struct realtek_gpio_ctrl *ctrl;
@@ -277,10 +314,20 @@ static int realtek_gpio_probe(struct platform_device *pdev)
 
 	raw_spin_lock_init(&ctrl->lock);
 
+	if (dev_flags & GPIO_PORTS_REVERSED) {
+		bgpio_flags = 0;
+		ctrl->port_offset_u8 = realtek_gpio_port_offset_u8_rev;
+		ctrl->port_offset_u16 = realtek_gpio_port_offset_u16_rev;
+	} else {
+		bgpio_flags = BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+		ctrl->port_offset_u8 = realtek_gpio_port_offset_u8;
+		ctrl->port_offset_u16 = realtek_gpio_port_offset_u16;
+	}
+
 	err = bgpio_init(&ctrl->gc, dev, 4,
 		ctrl->base + REALTEK_GPIO_REG_DATA, NULL, NULL,
 		ctrl->base + REALTEK_GPIO_REG_DIR, NULL,
-		BGPIOF_BIG_ENDIAN_BYTE_ORDER);
+		bgpio_flags);
 	if (err) {
 		dev_err(dev, "unable to init generic GPIO");
 		return err;
Signature · 1 line
2.35.1
#5 lore raw

Re: [PATCH v1 2/6] gpio: realtek-otto: Support reversed port layouts

drivers/gpio/gpio-realtek-otto.c static int realtek_gpio_probe(struct platform_device *pdev) · line 325

Quoted 9 lines · Sander Vanheule
drivers/gpio/gpio-realtek-otto.c +9 -0
+       if (dev_flags & GPIO_PORTS_REVERSED) {
+               bgpio_flags = 0;
+               ctrl->port_offset_u8 = realtek_gpio_port_offset_u8_rev;
+               ctrl->port_offset_u16 = realtek_gpio_port_offset_u16_rev;
+       } else {
+               bgpio_flags = BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+               ctrl->port_offset_u8 = realtek_gpio_port_offset_u8;
+               ctrl->port_offset_u16 = realtek_gpio_port_offset_u16;
+       }

Just checking: is this really a different silicon block, or is this GPIO_PORTS_REVERSED flag passed around just a way of saying:

if (!IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) ...?

Yours,
Linus Walleij

#6 lore raw

Re: [PATCH v1 2/6] gpio: realtek-otto: Support reversed port layouts

Hi Linus,

Quoted 12 lines · Linus Walleij
Quoted 9 lines · Sander Vanheule

+       if (dev_flags & GPIO_PORTS_REVERSED) {
+               bgpio_flags = 0;
+               ctrl->port_offset_u8 = realtek_gpio_port_offset_u8_rev; +               ctrl->port_offset_u16 = realtek_gpio_port_offset_u16_rev; +       } else {
+               bgpio_flags = BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+               ctrl->port_offset_u8 = realtek_gpio_port_offset_u8; +               ctrl->port_offset_u16 = realtek_gpio_port_offset_u16; +       }

Quoted 4 lines

Just checking: is this really a different silicon block, or is this GPIO_PORTS_REVERSED flag passed around just a way of saying:

if (!IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) ...?

The kernel for RTL930x SoC is built with CONFIG_CPU_BIG_ENDIAN=y, just like the older SoCs that were previously supported. The SoC's IRQ controller is also the same across RTL930x/RTL839x/RTL838x, even though 32-bit registers are used there.

On RTL838x/RTL839x the GPIO IRQ control registers have byte layout:

	[H1] [L1] [H2] [L2]
	[H3] [L3] [H4] [L4]

On RTL930x, the GPIO IRQ control registers are:

	[H2] [L2] [H1] [L1]
	[H4] [L4] [H3] [L3]
which is the reverse of:
	[L1] [H1] [L2] [H2]
	[L3] [H3] [L4] [H4]

Same for the GPIO registers:

	On RTL83xx: [P1] [P2] [P3] [P4] (four 8b ports)
	On RTL930x: [P4] [P3] [P2] [P1] (one BE32 port)

It looks like the RTL930x could use a little-endian interpretation of the 32b registers, followed by a little-endian interpretation of the contained port values. That would mean two reorderings for every 16b read or write operation, and manual manipulation of the register values. Although I have to say that the current offset calculation is not too pretty either.

We also discussed this with Andy with the original submission of the driver:
https://lore.kernel.org/linux-gpio/CAHp75VdrqE0kBwzK9Jk7pZGjyfFnhatfa8UY0z-3T1w1PrbAbw@mail.gmail.com/

Best,
Sander

Reviewed-by
#7 lore raw

Re: [PATCH v1 2/6] gpio: realtek-otto: Support reversed port layouts

Quoted 18 lines · Sander Vanheule

The kernel for RTL930x SoC is built with CONFIG_CPU_BIG_ENDIAN=y, just like the older SoCs that were previously supported. The SoC's IRQ controller is also the same across RTL930x/RTL839x/RTL838x, even though 32-bit registers are used there.

On RTL838x/RTL839x the GPIO IRQ control registers have byte layout:

        [H1] [L1] [H2] [L2]
        [H3] [L3] [H4] [L4]

On RTL930x, the GPIO IRQ control registers are:

        [H2] [L2] [H1] [L1]
        [H4] [L4] [H3] [L3]
which is the reverse of:
        [L1] [H1] [L2] [H2]
        [L3] [H3] [L4] [H4]

Same for the GPIO registers:

Quoted 8 lines
        On RTL83xx: [P1] [P2] [P3] [P4] (four 8b ports)
        On RTL930x: [P4] [P3] [P2] [P1] (one BE32 port)

It looks like the RTL930x could use a little-endian interpretation of the 32b registers, followed by a little-endian interpretation of the contained port values. That would mean two reorderings for every 16b read or write operation, and manual manipulation of the register values. Although I have to say that the current offset calculation is not too pretty either.

I'm happy.

It's not very invasive and the bulk of the problem is addressed by simply using the GPIO MMIO library, so:

If someone knows a more elegant way, they can send a patch, this works so we should merge it.

Yours,
Linus Walleij

#8 lore raw

PATCH 3/6 gpio: realtek-otto: Support per-cpu interrupts

In reply to Sander Vanheule

On SoCs with multiple cores, it is possible that the GPIO interrupt controller supports assigning specific pins to one or more cores.

IRQ balancing can be performed on a line-by-line basis if the parent interrupt is routed to all available cores, which is the default upon initialisation.

---
 drivers/gpio/gpio-realtek-otto.c | 75 +++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)
drivers/gpio/gpio-realtek-otto.c +74 -1
index c838ad8ce55f..dd1b7656d23a 100644
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <linux/gpio/driver.h>
+#include <linux/cpumask.h>
 #include <linux/irq.h>
 #include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
@@ -55,6 +56,8 @@
 struct realtek_gpio_ctrl {
 	struct gpio_chip gc;
 	void __iomem *base;
+	void __iomem *cpumask_base;
+	struct cpumask cpu_irq_maskable;
 	raw_spinlock_t lock;
 	u16 intr_mask[REALTEK_GPIO_PORTS_PER_BANK];
 	u16 intr_type[REALTEK_GPIO_PORTS_PER_BANK];
@@ -76,6 +79,11 @@ enum realtek_gpio_flags {
 	 * fields, and [BA, DC] for 2-bit fields.
 	 */
 	GPIO_PORTS_REVERSED = BIT(1),
+	/*
+	 * Interrupts can be enabled per cpu. This requires a secondary IO
+	 * range, where the per-cpu enable masks are located.
+	 */
+	GPIO_INTERRUPTS_PER_CPU = BIT(2),
 };
 
 static struct realtek_gpio_ctrl *irq_data_to_ctrl(struct irq_data *data)
@@ -247,14 +255,61 @@ static void realtek_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(irq_chip, desc);
 }
 
+static inline void __iomem *realtek_gpio_irq_cpu_mask(struct realtek_gpio_ctrl *ctrl,
+	unsigned int port, int cpu)
+{
+	return ctrl->cpumask_base + ctrl->port_offset_u8(port) +
+		REALTEK_GPIO_PORTS_PER_BANK * cpu;
+}
+
+static int realtek_gpio_irq_set_affinity(struct irq_data *data,
+	const struct cpumask *dest, bool force)
+{
+	struct realtek_gpio_ctrl *ctrl = irq_data_to_ctrl(data);
+	unsigned int line = irqd_to_hwirq(data);
+	unsigned int port = line / 8;
+	unsigned int port_pin = line % 8;
+	void __iomem *irq_cpu_mask;
+	unsigned long flags;
+	int cpu;
+	u8 v;
+
+	if (!ctrl->cpumask_base)
+		return -ENXIO;
+
+	raw_spin_lock_irqsave(&ctrl->lock, flags);
+
+	for_each_cpu(cpu, &ctrl->cpu_irq_maskable) {
+		irq_cpu_mask = realtek_gpio_irq_cpu_mask(ctrl, port, cpu);
+		v = ioread8(irq_cpu_mask);
+
+		if (cpumask_test_cpu(cpu, dest))
+			v |= BIT(port_pin);
+		else
+			v &= ~BIT(port_pin);
+
+		iowrite8(v, irq_cpu_mask);
+	}
+
+	raw_spin_unlock_irqrestore(&ctrl->lock, flags);
+
+	irq_data_update_effective_affinity(data, dest);
+
+	return 0;
+}
+
 static int realtek_gpio_irq_init(struct gpio_chip *gc)
 {
 	struct realtek_gpio_ctrl *ctrl = gpiochip_get_data(gc);
 	unsigned int port;
+	int cpu;
 
 	for (port = 0; (port * 8) < gc->ngpio; port++) {
 		realtek_gpio_write_imr(ctrl, port, 0, 0);
 		realtek_gpio_clear_isr(ctrl, port, GENMASK(7, 0));
+
+		for_each_cpu(cpu, &ctrl->cpu_irq_maskable)
+			iowrite8(GENMASK(7, 0), realtek_gpio_irq_cpu_mask(ctrl, port, cpu));
 	}
 
 	return 0;
@@ -266,6 +321,7 @@ static struct irq_chip realtek_gpio_irq_chip = {
 	.irq_mask = realtek_gpio_irq_mask,
 	.irq_unmask = realtek_gpio_irq_unmask,
 	.irq_set_type = realtek_gpio_irq_set_type,
+	.irq_set_affinity = realtek_gpio_irq_set_affinity,
 };
 
 static const struct of_device_id realtek_gpio_of_match[] = {
@@ -290,8 +346,10 @@ static int realtek_gpio_probe(struct platform_device *pdev)
 	unsigned int dev_flags;
 	struct gpio_irq_chip *girq;
 	struct realtek_gpio_ctrl *ctrl;
+	struct resource *res;
 	u32 ngpios;
-	int err, irq;
+	unsigned int nr_cpus;
+	int cpu, err, irq;
 
 	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
 	if (!ctrl)
@@ -352,6 +410,21 @@ static int realtek_gpio_probe(struct platform_device *pdev)
 		girq->init_hw = realtek_gpio_irq_init;
 	}
 
+	cpumask_clear(&ctrl->cpu_irq_maskable);
+
+	if ((dev_flags & GPIO_INTERRUPTS_PER_CPU) && irq > 0) {
+		ctrl->cpumask_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
+		if (IS_ERR(ctrl->cpumask_base))
+			return dev_err_probe(dev, PTR_ERR(ctrl->cpumask_base),
+				"missing CPU IRQ mask registers");
+
+		nr_cpus = resource_size(res) / REALTEK_GPIO_PORTS_PER_BANK;
+		nr_cpus = min(nr_cpus, num_present_cpus());
+
+		for (cpu = 0; cpu < nr_cpus; cpu++)
+			cpumask_set_cpu(cpu, &ctrl->cpu_irq_maskable);
+	}
+
 	return devm_gpiochip_add_data(dev, &ctrl->gc, ctrl);
 }
 
Signature · 1 line
2.35.1
#9 lore raw

Re: [PATCH v1 3/6] gpio: realtek-otto: Support per-cpu interrupts

Quoted 8 lines · Sander Vanheule

On SoCs with multiple cores, it is possible that the GPIO interrupt controller supports assigning specific pins to one or more cores.

IRQ balancing can be performed on a line-by-line basis if the parent interrupt is routed to all available cores, which is the default upon initialisation.

That sounds complicated.

Sounds like something the IRQ maintainer (Marc Z) should have a quick look at.

Yours,
Linus Walleij

#10 lore raw

Re: [PATCH v1 3/6] gpio: realtek-otto: Support per-cpu interrupts

Quoted 10 lines · Linus Walleij
Quoted 7 lines · Sander Vanheule

On SoCs with multiple cores, it is possible that the GPIO interrupt controller supports assigning specific pins to one or more cores.

IRQ balancing can be performed on a line-by-line basis if the parent interrupt is routed to all available cores, which is the default upon initialisation.

Quoted 6 lines
Quoted 1 line

That sounds complicated.

Sounds like something the IRQ maintainer (Marc Z) should have a quick look at.

This is pretty odd indeed. There seem to be a direct mapping between the GPIOs and the CPU it interrupts (or at least that's what the code seem to express). However, I don't see a direct relation between the CPUs and the chained interrupt. It isn't even clear if this interrupt itself is per-CPU.

So this begs a few questions:

- is the affinity actually affecting the target CPU? or is it affecting the target mux?

- how is the affinity of the mux interrupt actually enforced?

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#11 lore raw

Re: [PATCH v1 3/6] gpio: realtek-otto: Support per-cpu interrupts

Hi Linus, Marc,

Quoted 25 lines · Marc Zyngier
Quoted 10 lines · Linus Walleij
Quoted 7 lines · Sander Vanheule

On SoCs with multiple cores, it is possible that the GPIO interrupt controller supports assigning specific pins to one or more cores.

IRQ balancing can be performed on a line-by-line basis if the parent interrupt is routed to all available cores, which is the default upon initialisation.

Quoted 6 lines
Quoted 1 line

That sounds complicated.

Sounds like something the IRQ maintainer (Marc Z) should have a quick look at.

This is pretty odd indeed. There seem to be a direct mapping between the GPIOs and the CPU it interrupts (or at least that's what the code seem to express). However, I don't see a direct relation between the CPUs and the chained interrupt. It isn't even clear if this interrupt itself is per-CPU.

Quoted 6 lines

So this begs a few questions:

- is the affinity actually affecting the target CPU? or is it affecting the target mux?

- how is the affinity of the mux interrupt actually enforced?

There are three interrupt controllers at play here:
1. MIPS CPU interrupt controller: drivers/irqchip/irq-mips-cpu.c One interrupt controller per VPE, so in this case there are two. Provides per-CPU interrupts.
2. SoC interrupt controller: drivers/irqchip/irq-realtek-rtl.c Also one interrupt controller per VPE. I suppose these will also be per- CPU, although this isn't implemented in the driver yet, and I don't think I yet fully understand how should work in the kernel.
3. GPIO interrupt controller: drivers/gpio/gpio-realtek-otto.c One interrupt controller for the entire GPIO bank, with optional configurable affinity (this patch) for the different VPEs.

For the RTL839x series of SoCs, this results in the following:

GPIO LINES SOC IRQ MIPS

+--------+ +-----------+ HW IRQ +--------+
--->| GPIO | | SOC IRQ | LINES | IRQ |
--->| BANK |-----o-->| VPE0 CTRL |=========>| VPE0 |
. | | | +-----------+ +--------+
. +--------+ | 
. |
| +-----------+ +--------+
\-->| SOC IRQ | | IRQ |
| VPE1 CTRL |=========>| VPE1 |
+-----------+ +--------+

For RTL930x, where GPIO IRQ affinity is configurable:

GPIO LINES SOC IRQ MIPS

+--------+ +-----------+ HW IRQ +--------+
--->| GPIO |-------->| SOC IRQ | LINES | IRQ |
--->| BANK | | VPE0 CTRL |=========>| VPE0 |
. | |-----\ +-----------+ +--------+
. +--------+ | 
. |
| +-----------+ +--------+
\-->| SOC IRQ | | IRQ |
| VPE1 CTRL |=========>| VPE1 |
+-----------+ +--------+

The interrupt for the GPIO controller can be muxed to any of the MIPS HW interrupts on any (or all) of the VPEs, and these muxes (SoC IRQ controllers) can be configured independently per CPU. The SoC IRQ line index is fixed, and consistent for both VPEs.
Only in the second diagram can individual GPIO interrupts be muxed to any of the VPEs, but there is still only one IRQ line per VPE for all selected GPIO lines.

I hopes this helps to clarify the situation. We don't have any real documentation, so this is basically derived from registers descriptions in SDK headers and testing the interrupt behaviour.

Best,
Sander

#12 lore raw

Re: [PATCH v1 3/6] gpio: realtek-otto: Support per-cpu interrupts

Quoted 47 lines · Sander Vanheule

Hi Linus, Marc,

Quoted 25 lines · Marc Zyngier
Quoted 10 lines · Linus Walleij
Quoted 7 lines · Sander Vanheule

On SoCs with multiple cores, it is possible that the GPIO interrupt controller supports assigning specific pins to one or more cores.

IRQ balancing can be performed on a line-by-line basis if the parent interrupt is routed to all available cores, which is the default upon initialisation.

Quoted 6 lines
Quoted 1 line

That sounds complicated.

Sounds like something the IRQ maintainer (Marc Z) should have a quick look at.

This is pretty odd indeed. There seem to be a direct mapping between the GPIOs and the CPU it interrupts (or at least that's what the code seem to express). However, I don't see a direct relation between the CPUs and the chained interrupt. It isn't even clear if this interrupt itself is per-CPU.

Quoted 6 lines

So this begs a few questions:

- is the affinity actually affecting the target CPU? or is it affecting the target mux?

- how is the affinity of the mux interrupt actually enforced?

There are three interrupt controllers at play here:
   1. MIPS CPU interrupt controller: drivers/irqchip/irq-mips-cpu.c One interrupt controller per VPE, so in this case there are two. Provides per-CPU interrupts.
   2. SoC interrupt controller: drivers/irqchip/irq-realtek-rtl.c Also one interrupt controller per VPE. I suppose these will also be per- CPU, although this isn't implemented in the driver yet, and I don't think I yet fully understand how should work in the kernel.
   3. GPIO interrupt controller: drivers/gpio/gpio-realtek-otto.c One interrupt controller for the entire GPIO bank, with optional configurable affinity (this patch) for the different VPEs.

Quoted 1 line

For the RTL839x series of SoCs, this results in the following:

Sorry for the messed up formattng, let me try that again.

GPIO LINES        SOC IRQ              MIPS
    +--------+    LINE +-----------+   HW IRQ +--------+
--->| GPIO   |         | SOC IRQ   |   LINES  | IRQ    |
--->| BANK   |-----o-->| VPE0 CTRL |=========>| VPE0   |
 .  |        |     |   +-----------+          +--------+
 .  +--------+     | 
 .                 |
                   |   +-----------+          +--------+
                   \-->| SOC IRQ   |          | IRQ    |
                       | VPE1 CTRL |=========>| VPE1   |
                       +-----------+          +--------+
Quoted 1 line

For RTL930x, where GPIO IRQ affinity is configurable:

GPIO LINES        SOC IRQ              MIPS
    +--------+    LINE +-----------+   HW IRQ +--------+
--->| GPIO   |-------->| SOC IRQ   |   LINES  | IRQ    |
--->| BANK   |         | VPE0 CTRL |=========>| VPE0   |
 .  |        |-----\   +-----------+          +--------+
 .  +--------+     | 
 .                 |
                   |   +-----------+          +--------+
                   \-->| SOC IRQ   |          | IRQ    |
                       | VPE1 CTRL |=========>| VPE1   |
                       +-----------+          +--------+
Quoted 13 lines

The interrupt for the GPIO controller can be muxed to any of the MIPS HW interrupts on any (or all) of the VPEs, and these muxes (SoC IRQ controllers) can be configured independently per CPU. The SoC IRQ line index is fixed, and consistent for both VPEs.
Only in the second diagram can individual GPIO interrupts be muxed to any of the VPEs, but there is still only one IRQ line per VPE for all selected GPIO lines.

I hopes this helps to clarify the situation. We don't have any real documentation, so this is basically derived from registers descriptions in SDK headers and testing the interrupt behaviour.

Best,
Sander

#13 lore raw

PATCH 4/6 gpio: realtek-otto: Add RTL930x support

In reply to Sander Vanheule

The RTL930x SoC series has support for 24 GPIOs, with the port order reversed compared to RTL838x and RTL839x. The RTL930x series also has two CPUs (VPEs) and can distribute individual GPIO interrupts between them.

---
 drivers/gpio/gpio-realtek-otto.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/gpio/gpio-realtek-otto.c +4 -0
index dd1b7656d23a..3ddaa17accff 100644
@@ -335,6 +335,10 @@ static const struct of_device_id realtek_gpio_of_match[] = {
 	{
 		.compatible = "realtek,rtl8390-gpio",
 	},
+	{
+		.compatible = "realtek,rtl9300-gpio",
+		.data = (void *)(GPIO_PORTS_REVERSED | GPIO_INTERRUPTS_PER_CPU)
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, realtek_gpio_of_match);
Signature · 1 line
2.35.1
#14 lore raw

PATCH 5/6 dt-bindings: gpio: realtek-otto: Add rtl9310 compatible

In reply to Sander Vanheule

Add the "realtek,rlt9310-gpio", "realtek,otto-gpio" compatible for GPIO nodes on the RTL931x SoC series.

---
 Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml +1 -0
index 3c511e9af377..39fd959c45d2 100644
@@ -29,6 +29,7 @@ properties:
           - realtek,rtl8380-gpio
           - realtek,rtl8390-gpio
           - realtek,rtl9300-gpio
+          - realtek,rtl9310-gpio
       - const: realtek,otto-gpio
 
   reg: true
Signature · 1 line
2.35.1
Acked-by on v1 5/6
#15 lore raw

Re: [PATCH v1 5/6] dt-bindings: gpio: realtek-otto: Add rtl9310 compatible

Quoted 8 lines · Sander Vanheule

Add the "realtek,rlt9310-gpio", "realtek,otto-gpio" compatible for GPIO nodes on the RTL931x SoC series.

---
 Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml | 1 +
 1 file changed, 1 insertion(+)

Best regards,
Krzysztof

#16 lore raw

PATCH 6/6 gpio: realtek-otto: Add RTL931x support

In reply to Sander Vanheule

The RTL931x SoC series has support for 32 GPIOs, although not all lines may be broken out to a physical pad.

The GPIO bank's parent interrupt can be routed to either or both of the SoC's CPU cores by the GIC. Line-by-line IRQ balancing is not possible on these SoCs.

---
 drivers/gpio/gpio-realtek-otto.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/gpio/gpio-realtek-otto.c +3 -0
index 3ddaa17accff..c52b2cb1acae 100644
@@ -339,6 +339,9 @@ static const struct of_device_id realtek_gpio_of_match[] = {
 		.compatible = "realtek,rtl9300-gpio",
 		.data = (void *)(GPIO_PORTS_REVERSED | GPIO_INTERRUPTS_PER_CPU)
 	},
+	{
+		.compatible = "realtek,rtl9310-gpio",
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, realtek_gpio_of_match);
Signature · 1 line
2.35.1
#17 lore raw
In reply to Sander Vanheule
Quoted 20 lines · Sander Vanheule

This patch series adds support for the GPIO controllers as found on the RTL930x and RTL931x SoC families of MIPS CPUs, used in managed NBase-T ethernet switches.

The RTL931x's GPIO controller behaves the same as the one in the older RTL838x and RTL839x series. This controller is trivially supported.

The RTL930x's controller has a reversed port order, and supports CPU affinity settings for individual GPIO line IRQs, thus requiring two additional changes to implement these features.

Sander Vanheule (6):
dt-bindings: gpio: realtek-otto: Add rtl9300 compatible
gpio: realtek-otto: Support reversed port layouts
gpio: realtek-otto: Support per-cpu interrupts
gpio: realtek-otto: Add RTL930x support
dt-bindings: gpio: realtek-otto: Add rtl9310 compatible
gpio: realtek-otto: Add RTL931x support

Quoted 7 lines
 .../bindings/gpio/realtek,otto-gpio.yaml      |  34 ++++-
 drivers/gpio/gpio-realtek-otto.c              | 137 +++++++++++++++++-
 2 files changed, 164 insertions(+), 7 deletions(-)

--
2.35.1

Queued the entire series, thanks!

Bart