mbox series

[v5,0/5] Renesas RZ/G2L IRQC support

Message ID 20220523174238.28942-1-prabhakar.mahadev-lad.rj@bp.renesas.com
Headers show
Series Renesas RZ/G2L IRQC support | expand

Message

Prabhakar Mahadev Lad May 23, 2022, 5:42 p.m. UTC
Hi All,

The RZ/G2L Interrupt Controller is a front-end for the GIC found on
Renesas RZ/G2L SoC's with below pins:
- IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI
  interrupts
- GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
  maximum of only 32 can be mapped to 32 GIC SPI interrupts,
- NMI edge select.

                                                             _____________
                                                             |    GIC     |
                                                             |  ________  |
                                      ____________           | |        | |
NMI --------------------------------->|          |  SPI0-479 | | GIC-600| |
             _______                  |          |------------>|        | |
             |      |                 |          |  PPI16-31 | |        | |
             |      | IRQ0-IRQ7       |   IRQC   |------------>|        | |
P0_P48_4 --->| GPIO |---------------->|          |           | |________| |
             |      |GPIOINT0-122     |          |           |            |
             |      |---------------->| TINT0-31 |           |            |
             |______|                 |__________|           |____________|

The proposed patches add hierarchical IRQ domain, one in IRQC driver and
another in pinctrl driver. Upon interrupt requests map the interrupt to
GIC. Out of GPIOINT0-122 only 32 can be mapped to GIC SPI, this mapping is
handled by the pinctrl and IRQC driver.

Cheers,
Prabhakar

Changes for v4->v5:
* Updated commit message for patch 3/5
* Dropped interrupt-parent from and included RB tag from Geert for patch 4/5
* Implemented init_valid_mask() callback
* Dropped ngirq patch from previous series
* Dropped patches 4/7 and 5/7 from previous patch series will handle it separately.

Changes for v3->v4:
* Updated description for interrupts-cells property in patch #1
* Dropped the patch which overriding free callback in gpiolib
* Used devm helpers in patch#2
* Patch #4, #5 and #6 are newly added
* In patch #7 dropped using gpio offset as hwirq
* Implemented immutable GPIO in patch #7
* Implemented child_offset_to_irq() callback in patch #7

Changes for v2->v3:
* Updated description for interrupts-cells property in patch #1
* Included RB tag from Geert for binding patch
* Fixed review comments pointed by Geert, Biju and Sergei.

Changes for v1->v2:
* Included RB tag from Rob
* Fixed review comments pointed by Geert
* included GPIO driver changes

Changes for RFCV4 -> V1:
* Used unevaluatedProperties.
* Altered the sequence of reg property
* Set the parent type
* Used raw_spin_lock() instead of raw_spin_lock_irqsave()
* Simplified parsing IRQ map.
* Will send the GPIO and pinctrl changes as part of separate series

Changes for RFC v4:
* Used locking while RMW
* Now using interrupts property instead of interrupt-map
* Patch series depends on [0]
* Updated binding doc
* Fixed comments pointed by Andy

[0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/
20220316200633.28974-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/

Changes for RFC v3:
-> Re-structured the driver as a hierarchical irq domain instead of chained
-> made use of IRQCHIP_* macros
-> dropped locking
-> Added support for IRQ0-7 interrupts
-> Introduced 2 new patches for GPIOLIB
-> Switched to using GPIOLIB for irqdomains in pinctrl

RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20210921193028.13099-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/

RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20210803175109.1729-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/

Lad Prabhakar (5):
  dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt
    Controller
  irqchip: Add RZ/G2L IA55 Interrupt Controller driver
  gpio: gpiolib: Allow free() callback to be overridden
  dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Document the properties
    to handle GPIO IRQ
  pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO
    interrupt

 .../renesas,rzg2l-irqc.yaml                   | 133 ++++++
 .../pinctrl/renesas,rzg2l-pinctrl.yaml        |  15 +
 drivers/gpio/gpiolib.c                        |   9 +-
 drivers/irqchip/Kconfig                       |   8 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-renesas-rzg2l.c           | 425 ++++++++++++++++++
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 236 ++++++++++
 7 files changed, 824 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
 create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

Comments

Linus Walleij May 24, 2022, 8:54 a.m. UTC | #1
On Mon, May 23, 2022 at 7:43 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:

> Allow free() callback to be overridden from irq_domain_ops for
> hierarchical chips.
>
> This allows drivers to free up resources which are allocated during
> child_to_parent_hwirq()/populate_parent_alloc_arg() callbacks.
>
> On Renesas RZ/G2L platform a bitmap is maintained for TINT slots, a slot
> is allocated in child_to_parent_hwirq() callback which is freed up in free
> callback hence this override.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

So that function today looks like this:

static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
{
        ops->activate = gpiochip_irq_domain_activate;
        ops->deactivate = gpiochip_irq_domain_deactivate;
        ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
        ops->free = irq_domain_free_irqs_common;

        /*
         * We only allow overriding the translate() function for
         * hierarchical chips, and this should only be done if the user
         * really need something other than 1:1 translation.
         */
        if (!ops->translate)
                ops->translate = gpiochip_hierarchy_irq_domain_translate;
}

(...)
-       ops->free = irq_domain_free_irqs_common;
(...)
> +       if (!ops->free)
> +               ops->free = irq_domain_free_irqs_common;

Marc Z is working on cleaning up the way that gpiolib is (ab)using
irqchips. We definitely need his ACK if we do things like this.
This doesn't look like one of the big offenders to me, but I want
to make sure we don't create new problems while Marc is trying
to solve the old ones.

Yours,
Linus Walleij
Prabhakar May 24, 2022, 9:06 a.m. UTC | #2
Hi Linus,

Thank you for the feedback.

On Tue, May 24, 2022 at 9:54 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, May 23, 2022 at 7:43 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> > Allow free() callback to be overridden from irq_domain_ops for
> > hierarchical chips.
> >
> > This allows drivers to free up resources which are allocated during
> > child_to_parent_hwirq()/populate_parent_alloc_arg() callbacks.
> >
> > On Renesas RZ/G2L platform a bitmap is maintained for TINT slots, a slot
> > is allocated in child_to_parent_hwirq() callback which is freed up in free
> > callback hence this override.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> So that function today looks like this:
>
> static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
> {
>         ops->activate = gpiochip_irq_domain_activate;
>         ops->deactivate = gpiochip_irq_domain_deactivate;
>         ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
>         ops->free = irq_domain_free_irqs_common;
>
>         /*
>          * We only allow overriding the translate() function for
>          * hierarchical chips, and this should only be done if the user
>          * really need something other than 1:1 translation.
>          */
>         if (!ops->translate)
>                 ops->translate = gpiochip_hierarchy_irq_domain_translate;
> }
>
> (...)
> -       ops->free = irq_domain_free_irqs_common;
> (...)
> > +       if (!ops->free)
> > +               ops->free = irq_domain_free_irqs_common;
>
> Marc Z is working on cleaning up the way that gpiolib is (ab)using
> irqchips. We definitely need his ACK if we do things like this.
> This doesn't look like one of the big offenders to me, but I want
> to make sure we don't create new problems while Marc is trying
> to solve the old ones.
>
Agreed, I had a discussion with Marc on v3 series [0].

[0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220511183210.5248-4-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar
Linus Walleij May 24, 2022, 9:29 a.m. UTC | #3
On Tue, May 24, 2022 at 11:07 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Tue, May 24, 2022 at 9:54 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Mon, May 23, 2022 at 7:43 PM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >
> > > Allow free() callback to be overridden from irq_domain_ops for
> > > hierarchical chips.
> > >
> > > This allows drivers to free up resources which are allocated during
> > > child_to_parent_hwirq()/populate_parent_alloc_arg() callbacks.
> > >
> > > On Renesas RZ/G2L platform a bitmap is maintained for TINT slots, a slot
> > > is allocated in child_to_parent_hwirq() callback which is freed up in free
> > > callback hence this override.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > So that function today looks like this:
> >
> > static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
> > {
> >         ops->activate = gpiochip_irq_domain_activate;
> >         ops->deactivate = gpiochip_irq_domain_deactivate;
> >         ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
> >         ops->free = irq_domain_free_irqs_common;
> >
> >         /*
> >          * We only allow overriding the translate() function for
> >          * hierarchical chips, and this should only be done if the user
> >          * really need something other than 1:1 translation.
> >          */
> >         if (!ops->translate)
> >                 ops->translate = gpiochip_hierarchy_irq_domain_translate;
> > }
> >
> > (...)
> > -       ops->free = irq_domain_free_irqs_common;
> > (...)
> > > +       if (!ops->free)
> > > +               ops->free = irq_domain_free_irqs_common;
> >
> > Marc Z is working on cleaning up the way that gpiolib is (ab)using
> > irqchips. We definitely need his ACK if we do things like this.
> > This doesn't look like one of the big offenders to me, but I want
> > to make sure we don't create new problems while Marc is trying
> > to solve the old ones.
> >
> Agreed, I had a discussion with Marc on v3 series [0].

Hm yeah I guess I am just stepping on Marc's toes with all my mails :(

I'll try to just wait for Marc's Reviewed-by instead and not add to the noise,
I'm probably just wrong.

Yours,
Linus Walleij
Prabhakar June 19, 2022, 7:29 p.m. UTC | #4
Hi Marc

On Mon, May 23, 2022 at 6:42 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> Hi All,
>
> The RZ/G2L Interrupt Controller is a front-end for the GIC found on
> Renesas RZ/G2L SoC's with below pins:
> - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI
>   interrupts
> - GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
>   maximum of only 32 can be mapped to 32 GIC SPI interrupts,
> - NMI edge select.
>
>                                                              _____________
>                                                              |    GIC     |
>                                                              |  ________  |
>                                       ____________           | |        | |
> NMI --------------------------------->|          |  SPI0-479 | | GIC-600| |
>              _______                  |          |------------>|        | |
>              |      |                 |          |  PPI16-31 | |        | |
>              |      | IRQ0-IRQ7       |   IRQC   |------------>|        | |
> P0_P48_4 --->| GPIO |---------------->|          |           | |________| |
>              |      |GPIOINT0-122     |          |           |            |
>              |      |---------------->| TINT0-31 |           |            |
>              |______|                 |__________|           |____________|
>
> The proposed patches add hierarchical IRQ domain, one in IRQC driver and
> another in pinctrl driver. Upon interrupt requests map the interrupt to
> GIC. Out of GPIOINT0-122 only 32 can be mapped to GIC SPI, this mapping is
> handled by the pinctrl and IRQC driver.
>
> Cheers,
> Prabhakar
>
> Changes for v4->v5:
> * Updated commit message for patch 3/5
> * Dropped interrupt-parent from and included RB tag from Geert for patch 4/5
> * Implemented init_valid_mask() callback
> * Dropped ngirq patch from previous series
> * Dropped patches 4/7 and 5/7 from previous patch series will handle it separately.
>
> Changes for v3->v4:
> * Updated description for interrupts-cells property in patch #1
> * Dropped the patch which overriding free callback in gpiolib
> * Used devm helpers in patch#2
> * Patch #4, #5 and #6 are newly added
> * In patch #7 dropped using gpio offset as hwirq
> * Implemented immutable GPIO in patch #7
> * Implemented child_offset_to_irq() callback in patch #7
>
> Changes for v2->v3:
> * Updated description for interrupts-cells property in patch #1
> * Included RB tag from Geert for binding patch
> * Fixed review comments pointed by Geert, Biju and Sergei.
>
> Changes for v1->v2:
> * Included RB tag from Rob
> * Fixed review comments pointed by Geert
> * included GPIO driver changes
>
> Changes for RFCV4 -> V1:
> * Used unevaluatedProperties.
> * Altered the sequence of reg property
> * Set the parent type
> * Used raw_spin_lock() instead of raw_spin_lock_irqsave()
> * Simplified parsing IRQ map.
> * Will send the GPIO and pinctrl changes as part of separate series
>
> Changes for RFC v4:
> * Used locking while RMW
> * Now using interrupts property instead of interrupt-map
> * Patch series depends on [0]
> * Updated binding doc
> * Fixed comments pointed by Andy
>
> [0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/
> 20220316200633.28974-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/
>
> Changes for RFC v3:
> -> Re-structured the driver as a hierarchical irq domain instead of chained
> -> made use of IRQCHIP_* macros
> -> dropped locking
> -> Added support for IRQ0-7 interrupts
> -> Introduced 2 new patches for GPIOLIB
> -> Switched to using GPIOLIB for irqdomains in pinctrl
>
> RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
> 20210921193028.13099-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/
>
> RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
> 20210803175109.1729-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/
>
> Lad Prabhakar (5):
>   dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt
>     Controller
>   irqchip: Add RZ/G2L IA55 Interrupt Controller driver
>   gpio: gpiolib: Allow free() callback to be overridden
>   dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Document the properties
>     to handle GPIO IRQ
>   pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO
>     interrupt
>
>  .../renesas,rzg2l-irqc.yaml                   | 133 ++++++
>  .../pinctrl/renesas,rzg2l-pinctrl.yaml        |  15 +
>  drivers/gpio/gpiolib.c                        |   9 +-
>  drivers/irqchip/Kconfig                       |   8 +
>  drivers/irqchip/Makefile                      |   1 +
>  drivers/irqchip/irq-renesas-rzg2l.c           | 425 ++++++++++++++++++
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 236 ++++++++++
>  7 files changed, 824 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
>  create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
>
Gentle ping.

Are you happy with this series?

Cheers,
Prabhakar