2022-04-11 10:55:11

by Sander Vanheule

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

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 <[email protected]>
---
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


2022-04-21 15:41:10

by Linus Walleij

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

On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <[email protected]> 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 <[email protected]>

That sounds complicated.

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

Yours,
Linus Walleij

2022-04-22 15:58:34

by Sander Vanheule

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

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 <[email protected]> wrote:
> >
> > On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <[email protected]> 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 <[email protected]>
> >
> > 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

2022-04-22 17:43:01

by Marc Zyngier

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

On Thu, 21 Apr 2022 00:04:16 +0100,
Linus Walleij <[email protected]> wrote:
>
> On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <[email protected]> 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 <[email protected]>
>
> 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.

2022-04-22 23:02:02

by Sander Vanheule

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

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 <[email protected]> wrote:
> > >
> > > On Sat, Apr 9, 2022 at 9:56 PM Sander Vanheule <[email protected]> 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 <[email protected]>
> > >
> > > 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