[PATCH v1 0/6] Support for RTL930x/RTL931x GPIOs

[PATCH v1 0/6] Support for RTL930x/RTL931x GPIOs

[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

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

-- 
2.35.1

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

[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.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 .../bindings/gpio/realtek,otto-gpio.yaml      | 33 +++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
index 100f20cebd76..3c511e9af377 100644
--- a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
@@ -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>;
+      };
 
 ...
-- 
2.35.1

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

[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.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/gpio/gpio-realtek-otto.c | 55 +++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c
index bd75401b549d..c838ad8ce55f 100644
--- a/drivers/gpio/gpio-realtek-otto.c
+++ b/drivers/gpio/gpio-realtek-otto.c
@@ -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;
-- 
2.35.1

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

[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.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/gpio/gpio-realtek-otto.c | 75 +++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c
index c838ad8ce55f..dd1b7656d23a 100644
--- a/drivers/gpio/gpio-realtek-otto.c
+++ b/drivers/gpio/gpio-realtek-otto.c
@@ -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);
 }
 
-- 
2.35.1

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

[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.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/gpio/gpio-realtek-otto.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c
index dd1b7656d23a..3ddaa17accff 100644
--- a/drivers/gpio/gpio-realtek-otto.c
+++ b/drivers/gpio/gpio-realtek-otto.c
@@ -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);
-- 
2.35.1

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

[Sander Vanheule]

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

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
index 3c511e9af377..39fd959c45d2 100644
--- a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
@@ -29,6 +29,7 @@ properties:
           - realtek,rtl8380-gpio
           - realtek,rtl8390-gpio
           - realtek,rtl9300-gpio
+          - realtek,rtl9310-gpio
       - const: realtek,otto-gpio
 
   reg: true
-- 
2.35.1

[PATCH v1 6/6] gpio: realtek-otto: Add RTL931x support

[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.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/gpio/gpio-realtek-otto.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c
index 3ddaa17accff..c52b2cb1acae 100644
--- a/drivers/gpio/gpio-realtek-otto.c
+++ b/drivers/gpio/gpio-realtek-otto.c
@@ -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);
-- 
2.35.1

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

[Krzysztof Kozlowski]

On 09/04/2022 21:55, Sander Vanheule wrote:
> 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.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
>  .../bindings/gpio/realtek,otto-gpio.yaml      | 33 +++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> index 100f20cebd76..3c511e9af377 100644
> --- a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> @@ -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:

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.


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 09/04/2022 21:55, Sander Vanheule wrote:
> Add the "realtek,rlt9310-gpio", "realtek,otto-gpio" compatible for GPIO
> nodes on the RTL931x SoC series.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
>  Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

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

[Sander Vanheule]

Hi Krzysztof, 

On Sun, 2022-04-10 at 16:16 +0200, Krzysztof Kozlowski wrote:
> On 09/04/2022 21:55, Sander Vanheule wrote:
> > 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.
> > 
> > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> > ---
> >  .../bindings/gpio/realtek,otto-gpio.yaml      | 33 +++++++++++++++++--
> >  1 file changed, 31 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> > b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> > index 100f20cebd76..3c511e9af377 100644
> > --- a/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> > +++ b/Documentation/devicetree/bindings/gpio/realtek,otto-gpio.yaml
> > @@ -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:
> 
> 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.
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

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

Re: [PATCH v1 0/6] Support for RTL930x/RTL931x GPIOs

[Bartosz Golaszewski]

On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:
>
> 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(-)
>
> --
> 2.35.1
>

Queued the entire series, thanks!

Bart

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

[Linus Walleij]

On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:

> +       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

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

[Linus Walleij]

On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:

> 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.
>
> Signed-off-by: Sander Vanheule <sander@svanheule.net>

That sounds complicated.

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

Yours,
Linus Walleij

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

[Sander Vanheule]

Hi Linus,

On Thu, 2022-04-21 at 01:01 +0200, Linus Walleij wrote:
> On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:
> 
> > +       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)) ...?

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

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

[Marc Zyngier]

On Thu, 21 Apr 2022 00:04:16 +0100,
Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:
> 
> > 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.
> >
> > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> 
> 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.

-- 
Without deviation from the norm, progress is not possible.

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

[Sander Vanheule]

Hi Linus, Marc,

On Thu, 2022-04-21 at 10:48 +0100, Marc Zyngier wrote:
> On Thu, 21 Apr 2022 00:04:16 +0100,
> Linus Walleij <linus.walleij@linaro.org> wrote:
> > 
> > On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:
> > 
> > > 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.
> > > 
> > > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> > 
> > 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?

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

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

[Sander Vanheule]

On Fri, 2022-04-22 at 09:04 +0200, Sander Vanheule wrote:
> Hi Linus, Marc,
> 
> On Thu, 2022-04-21 at 10:48 +0100, Marc Zyngier wrote:
> > On Thu, 21 Apr 2022 00:04:16 +0100,
> > Linus Walleij <linus.walleij@linaro.org> wrote:
> > > 
> > > On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <sander@svanheule.net> wrote:
> > > 
> > > > 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.
> > > > 
> > > > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> > > 
> > > 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?
> 
> 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:

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   |
                       +-----------+          +--------+


> 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   |
                       +-----------+          +--------+

> 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

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

[Linus Walleij]

On Thu, Apr 21, 2022 at 9:55 AM Sander Vanheule <sander@svanheule.net> wrote:

> 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.

I'm happy.

It's not very invasive and the bulk of the problem is addressed by
simply using the GPIO MMIO library, so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

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

Yours,
Linus Walleij