| Message ID | 20220523174238.28942-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| Headers | show |
| Series | Renesas RZ/G2L IRQC support | expand |
Hi Linus, Thank you for the review. On Tue, May 24, 2022 at 9:57 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: > > > Add IRQ domain to RZ/G2L pinctrl driver to handle GPIO interrupt. > > > > GPIO0-GPIO122 pins can be used as IRQ lines but only 32 pins can be > > used as IRQ lines at a given time. Selection of pins as IRQ lines > > is handled by IA55 (which is the IRQC block) which sits in between the > > GPIO and GIC. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > I don't know if I'm too tired or reading it wrong, but it seems you > went through the trouble of making it possible to override .free() in > the irqdomain in patch 3/5 and yet not using it in this patch 5/5? > I think you missed it, free callback is overridden with rzg2l_gpio_irq_domain_free(). [0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220523174238.28942-6-prabhakar.mahadev-lad.rj@bp.renesas.com/ Cheers, Prabhakar
On Tue, May 24, 2022 at 11:01 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Tue, May 24, 2022 at 9:57 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: > > > > > Add IRQ domain to RZ/G2L pinctrl driver to handle GPIO interrupt. > > > > > > GPIO0-GPIO122 pins can be used as IRQ lines but only 32 pins can be > > > used as IRQ lines at a given time. Selection of pins as IRQ lines > > > is handled by IA55 (which is the IRQC block) which sits in between the > > > GPIO and GIC. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > I don't know if I'm too tired or reading it wrong, but it seems you > > went through the trouble of making it possible to override .free() in > > the irqdomain in patch 3/5 and yet not using it in this patch 5/5? > > > I think you missed it, free callback is overridden with > rzg2l_gpio_irq_domain_free(). > > [0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220523174238.28942-6-prabhakar.mahadev-lad.rj@bp.renesas.com/ Yeah my bad, can't read properly today :/ Why is it necessary to do this stuff in the irqdomain rather than in the irqchip? Especially this: + bitmap_release_region(pctrl->tint_slot, i, get_order(1)); Since the idea with irq_domain is to translate physical (hardware) IRQs to Linux IRQ numbers, I don't see how this is related to that. To me it seems you have taken the usecase that is normally in irqchip and moved it to irqdomain. To me this seems much more like a job that needs to happen in the irqchip .irq_enable()/.irq_disable() pair, and which we have done before in Hans Verkuils patch series: 461c1a7d4733 gpiolib: override irq_enable/disable 4e9439ddacea gpiolib: add flag to indicate if the irq is disabled ca620f2de153 gliolib: set hooks in gpiochip_set_irq_hooks() This gets used by drivers such as: drivers/media/cec/platform/cec-gpio/cec-gpio.c Where you can see these dynamic calls: static bool cec_gpio_enable_irq(struct cec_adapter *adap) { struct cec_gpio *cec = cec_get_drvdata(adap); enable_irq(cec->cec_irq); return true; } static void cec_gpio_disable_irq(struct cec_adapter *adap) { struct cec_gpio *cec = cec_get_drvdata(adap); disable_irq(cec->cec_irq); } Which end up calling .irq_enable()/.irq_disable() on the irq_chip dynamically enabling/disabling the irq. If you prefer to have this done in process context up front when the irq is requested/released then irq_chip also have these callbacks: int (*irq_request_resources)(struct irq_data *data); void (*irq_release_resources)(struct irq_data *data); So I would think over the usecase here a bit. Why does this have to be in the irqdomain? Yours, Linus Walleij
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
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