Re: [PATCH 04/12] dt-bindings: soc: fsl: qe: Add support of IRQ in QE GPIO

["Paul Louvel" <paul.louvel@bootlin.com>]

On Mon Jul 6, 2026 at 8:52 AM CEST, Krzysztof Kozlowski wrote:
> On Fri, Jul 03, 2026 at 03:30:12PM +0200, Paul Louvel wrote:
>> Some QE GPIO pins have an associated interrupt line in the QE PIC to
>> signal state changes on the pin.  Add the corresponding
>> interrupt-controller / nexus properties to the QE GPIO binding.
>> 
>> Because the GPIO controller does not perform any interrupt handling
>> itself, a nexus node (interrupt-map) is used to map each GPIO line
>> supporting IRQ to the parent QE PIC interrupt domain.
>> 
>> As the QE PIC can be configured to generate an interrupt on either a
>> high-to-low transition or any change in signal state, three
>> interrupt-map entries are needed per GPIO pin that can yield an
>> interrupt (falling, both, and the "none" case which defaults to both in
>> QE PIC).  This overhead is necessary because the interrupt-map-pass-thru
>> property is not part of the DT specification.
>> 
>> The interrupt-map property is optional: it is not required for GPIO
>> banks that have no interrupt capable GPIO line (e.g. port D on MPC8323),
>> or when interrupt functionality is not used.
>> 
>> Update the example to show a scenario where each bank supports a
>> different numbers of IRQs, or no IRQs at all.
>> 
>> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
>> ---
>>  .../bindings/gpio/fsl,mpc8323-qe-pario-bank.yaml   | 69 +++++++++++++++++++++-
>>  1 file changed, 66 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/gpio/fsl,mpc8323-qe-pario-bank.yaml b/Documentation/devicetree/bindings/gpio/fsl,mpc8323-qe-pario-bank.yaml
>> index 1af99339ff40..0c849a5698f4 100644
>> --- a/Documentation/devicetree/bindings/gpio/fsl,mpc8323-qe-pario-bank.yaml
>> +++ b/Documentation/devicetree/bindings/gpio/fsl,mpc8323-qe-pario-bank.yaml
>> @@ -27,6 +27,17 @@ properties:
>>    "#gpio-cells":
>>      const: 2
>>  
>> +  "#address-cells":
>> +    const: 0
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +
>
> If this has interrupt-cells, then it is a nexus, thus why isn't this
> also a "interrupt-controller"?

Because these these banks are not interrupt controllers.
Interrupts are handled by the QE PIC, and the GPIO controller does not do any
interrupt handling itself.
In this setup, does it really needs an "interrupt-controller" property?

>
>> +  interrupt-map:
>> +    description: |
>> +      Specifies the mapping of GPIO lines to the parent interrupt controller, as the
>> +      GPIO controller does not do interrupt handling itself.
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -37,9 +48,61 @@ additionalProperties: false
>>  
>>  examples:
>>    - |
>> -    gpio-controller@1400 {
>> -        compatible = "fsl,mpc8360-qe-pario-bank", "fsl,mpc8323-qe-pario-bank";
>> -        reg = <0x1400 0x18>;
>
> I don't get why you rewrite existing example instead of adding new one.

I could yes.

>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    pic: interrupt-controller {
>> +      interrupt-controller;
>> +      #address-cells = <0>;
>> +      #interrupt-cells = <2>;
>> +    };
>
> Drop node, irrelevant.
>
>> +
>> +    gpio-controller@1418 {
>> +        #gpio-cells = <2>;
>> +        #address-cells = <0>;
>> +        #interrupt-cells = <2>;
>> +        compatible = "fsl,mpc8323-qe-pario-bank";
>> +        reg = <0x1418 0x18>;
>
> And now you are not following DTS coding style.
>
>>          gpio-controller;
>> +        interrupt-map = <
>> +          7 IRQ_TYPE_EDGE_FALLING  &pic 4 IRQ_TYPE_EDGE_FALLING
>> +          7 IRQ_TYPE_EDGE_BOTH     &pic 4 IRQ_TYPE_EDGE_BOTH
>> +          7 0                      &pic 4 IRQ_TYPE_NONE
>> +
>> +          9 IRQ_TYPE_EDGE_FALLING  &pic 5 IRQ_TYPE_EDGE_FALLING
>> +          9 IRQ_TYPE_EDGE_BOTH     &pic 5 IRQ_TYPE_EDGE_BOTH
>> +          9 0                      &pic 5 IRQ_TYPE_NONE
>> +
>> +          25 IRQ_TYPE_EDGE_FALLING &pic 6 IRQ_TYPE_EDGE_FALLING
>> +          25 IRQ_TYPE_EDGE_BOTH    &pic 6 IRQ_TYPE_EDGE_BOTH
>> +          25 0                     &pic 6 IRQ_TYPE_NONE
>> +
>> +          27 IRQ_TYPE_EDGE_FALLING &pic 7 IRQ_TYPE_EDGE_FALLING
>> +          27 IRQ_TYPE_EDGE_BOTH    &pic 7 IRQ_TYPE_EDGE_BOTH
>> +          27 0                     &pic 7 IRQ_TYPE_NONE
>> +        >;
>> +    };
>> +
>> +    gpio-controller@1430 {
>>          #gpio-cells = <2>;
>
> So two new examples? But old one was wrong?
>
>> +        #address-cells = <0>;
>> +        #interrupt-cells = <2>;
>> +        compatible = "fsl,mpc8323-qe-pario-bank";
>> +        reg = <0x1430 0x18>;
>> +        gpio-controller;
>> +        interrupt-map = <
>> +          24 IRQ_TYPE_EDGE_FALLING &pic 8 IRQ_TYPE_EDGE_FALLING
>> +          24 IRQ_TYPE_EDGE_BOTH    &pic 8 IRQ_TYPE_EDGE_BOTH
>> +          24 0                     &pic 8 IRQ_TYPE_NONE
>> +
>> +          29 IRQ_TYPE_EDGE_FALLING &pic 9 IRQ_TYPE_EDGE_FALLING
>> +          29 IRQ_TYPE_EDGE_BOTH    &pic 9 IRQ_TYPE_EDGE_BOTH
>> +          29 0                     &pic 9 IRQ_TYPE_NONE
>> +        >;
>> +    };
>> +
>> +    gpio-controller@1448 {
>> +        #gpio-cells = <2>;
>> +        compatible = "fsl,mpc8323-qe-pario-bank";
>
> Wait, three examples? But isn't this the same as previous one?

I wanted to outline that each bank do not have the same number of GPIO lines
that support interrupts.

>
>> +        reg = <0x1448 0x18>;
>> +        gpio-controller;
>>      };
>> 
>> -- 
>> 2.55.0
>> 

Thanks,
Paul.




-- 
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com