diff mbox series

[v2,1/5] dt-bindings: interrupt-controller: Update STM32 EXTI interrupt controller

Message ID 20211215105847.2328-2-alexandre.torgue@foss.st.com (mailing list archive)
State New, archived
Headers show
Series Add STM32MP13 EXTI support | expand

Commit Message

Alexandre TORGUE Dec. 15, 2021, 10:58 a.m. UTC
Document new entry "st,exti-mapping" which links EXTI lines with GIC
interrupt lines and add an include file to define EXTI interrupt type.

Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com>

Comments

Rob Herring Dec. 16, 2021, 8:15 p.m. UTC | #1
On Wed, Dec 15, 2021 at 11:58:43AM +0100, Alexandre Torgue wrote:
> Document new entry "st,exti-mapping" which links EXTI lines with GIC
> interrupt lines and add an include file to define EXTI interrupt type.
> 
> Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com>
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
> index d19c881b4abc..e08bb51e97a8 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
> @@ -41,6 +41,17 @@ properties:
>      description:
>        Interrupts references to primary interrupt controller
>  
> +  st,exti-mapping:
> +    $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
> +    description: |
> +            Define mapping between EXTI lines and GIC irq lines. Should be:
> +            st,exti-mapping = <EXTI_LINE GIC_IRQ EXTI_TYPE>, ...;
> +            With:
> +            - EXTI_LINE: EXTI line number.
> +            - GIC_IRQ: GIC IRQ associated to the EXTI line.
> +            - EXTI_TYPE: STM32_EXTI_TYPE_CONFIGURABLE or STM32_EXTI_TYPE_DIRECT.
> +              Defined in include/dt-bindings/interrupt-controller/stm32-exti.h

No custom properties for this. See[1][2][3].

Rob


[1] https://lore.kernel.org/all/20211122103032.517923-1-maz@kernel.org/
[2] https://lore.kernel.org/all/87k0g8jlmg.wl-maz@kernel.org/
[3] https://lore.kernel.org/all/CAL_JsqK2Shj6smam7HgNAmy3UG+vVQPkU3Q0OjyEHOEJB45n0A@mail.gmail.com/
Alexandre TORGUE Dec. 17, 2021, 1:39 p.m. UTC | #2
On 12/16/21 9:15 PM, Rob Herring wrote:
> On Wed, Dec 15, 2021 at 11:58:43AM +0100, Alexandre Torgue wrote:
>> Document new entry "st,exti-mapping" which links EXTI lines with GIC
>> interrupt lines and add an include file to define EXTI interrupt type.
>>
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com>
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
>> index d19c881b4abc..e08bb51e97a8 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
>> @@ -41,6 +41,17 @@ properties:
>>       description:
>>         Interrupts references to primary interrupt controller
>>   
>> +  st,exti-mapping:
>> +    $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
>> +    description: |
>> +            Define mapping between EXTI lines and GIC irq lines. Should be:
>> +            st,exti-mapping = <EXTI_LINE GIC_IRQ EXTI_TYPE>, ...;
>> +            With:
>> +            - EXTI_LINE: EXTI line number.
>> +            - GIC_IRQ: GIC IRQ associated to the EXTI line.
>> +            - EXTI_TYPE: STM32_EXTI_TYPE_CONFIGURABLE or STM32_EXTI_TYPE_DIRECT.
>> +              Defined in include/dt-bindings/interrupt-controller/stm32-exti.h
> 
> No custom properties for this. See[1][2][3].
> 

Thanks for inputs. In my case the mapping consists to map an EXTI line 
with a GIC irq line which could be done using interrupt-map (avoiding to 
parse it in my driver). But for each EXTI/GIC association I would like 
also to describe the EXTI_TYPE (which actually describe the well irqchip 
to use inside my exti driver) . This property is not generic and so I 
assume I can't use a generic binding such "interrupt-map".

If the solution consists to use a common binding (i.e. interrupt-map) 
plus a conversion table in exti driver to affect the well irq_chip to 
the well EXTI line then we could envisage to keep the whole mapping 
inside the driver (even if it's not the best solution).

Another solution could be to define two exti irq-controllers (one for 
configurable type and another one for direct type) and use interrupt-map 
inside each. It means to have 2 child node inside (one per irq 
controller) EXTI node.

Alex


> Rob
> 
> 
> [1] https://lore.kernel.org/all/20211122103032.517923-1-maz@kernel.org/
> [2] https://lore.kernel.org/all/87k0g8jlmg.wl-maz@kernel.org/
> [3] https://lore.kernel.org/all/CAL_JsqK2Shj6smam7HgNAmy3UG+vVQPkU3Q0OjyEHOEJB45n0A@mail.gmail.com/
>
Marc Zyngier Dec. 20, 2021, 12:34 p.m. UTC | #3
On Fri, 17 Dec 2021 13:39:11 +0000,
Alexandre TORGUE <alexandre.torgue@foss.st.com> wrote:
> 
> On 12/16/21 9:15 PM, Rob Herring wrote:
> > On Wed, Dec 15, 2021 at 11:58:43AM +0100, Alexandre Torgue wrote:
> >> Document new entry "st,exti-mapping" which links EXTI lines with GIC
> >> interrupt lines and add an include file to define EXTI interrupt type.
> >> 
> >> Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com>
> >> 
> >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
> >> index d19c881b4abc..e08bb51e97a8 100644
> >> --- a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
> >> +++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
> >> @@ -41,6 +41,17 @@ properties:
> >>       description:
> >>         Interrupts references to primary interrupt controller
> >>   +  st,exti-mapping:
> >> +    $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
> >> +    description: |
> >> +            Define mapping between EXTI lines and GIC irq lines. Should be:
> >> +            st,exti-mapping = <EXTI_LINE GIC_IRQ EXTI_TYPE>, ...;
> >> +            With:
> >> +            - EXTI_LINE: EXTI line number.
> >> +            - GIC_IRQ: GIC IRQ associated to the EXTI line.
> >> +            - EXTI_TYPE: STM32_EXTI_TYPE_CONFIGURABLE or STM32_EXTI_TYPE_DIRECT.
> >> +              Defined in include/dt-bindings/interrupt-controller/stm32-exti.h
> > 
> > No custom properties for this. See[1][2][3].
> > 
> 
> Thanks for inputs. In my case the mapping consists to map an EXTI line
> with a GIC irq line which could be done using interrupt-map (avoiding
> to parse it in my driver).

The problem is that 'interrupt-map' defines an interrupt mapping
between an input and an output, and that mentioning the GIC in such a
table will only result in your EXTI to be bypassed.

'interrupt-map' really is a dispatch table for targeting an interrupt
controller (or multiple controllers, even), but really isn't the
correct tool to carry configuration informations to an interrupt
controller driver.

> But for each EXTI/GIC association I would
> like also to describe the EXTI_TYPE (which actually describe the well
> irqchip to use inside my exti driver) . This property is not generic
> and so I assume I can't use a generic binding such "interrupt-map".
> 
> If the solution consists to use a common binding (i.e. interrupt-map)
> plus a conversion table in exti driver to affect the well irq_chip to
> the well EXTI line then we could envisage to keep the whole mapping
> inside the driver (even if it's not the best solution).

A possible solution would be to have:

- A set of standard 'interrupts' properties describing the output
signals

- A set of properties describing the input to output mapping (if
relevant) and additional configuration information that for the
interrupt controller driver.

	M.
Alexandre TORGUE Jan. 4, 2022, 5:14 p.m. UTC | #4
Hi Marc

On 12/20/21 1:34 PM, Marc Zyngier wrote:
> On Fri, 17 Dec 2021 13:39:11 +0000,
> Alexandre TORGUE <alexandre.torgue@foss.st.com> wrote:
>>
>> On 12/16/21 9:15 PM, Rob Herring wrote:
>>> On Wed, Dec 15, 2021 at 11:58:43AM +0100, Alexandre Torgue wrote:
>>>> Document new entry "st,exti-mapping" which links EXTI lines with GIC
>>>> interrupt lines and add an include file to define EXTI interrupt type.
>>>>
>>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com>
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
>>>> index d19c881b4abc..e08bb51e97a8 100644
>>>> --- a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
>>>> @@ -41,6 +41,17 @@ properties:
>>>>        description:
>>>>          Interrupts references to primary interrupt controller
>>>>    +  st,exti-mapping:
>>>> +    $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
>>>> +    description: |
>>>> +            Define mapping between EXTI lines and GIC irq lines. Should be:
>>>> +            st,exti-mapping = <EXTI_LINE GIC_IRQ EXTI_TYPE>, ...;
>>>> +            With:
>>>> +            - EXTI_LINE: EXTI line number.
>>>> +            - GIC_IRQ: GIC IRQ associated to the EXTI line.
>>>> +            - EXTI_TYPE: STM32_EXTI_TYPE_CONFIGURABLE or STM32_EXTI_TYPE_DIRECT.
>>>> +              Defined in include/dt-bindings/interrupt-controller/stm32-exti.h
>>>
>>> No custom properties for this. See[1][2][3].
>>>
>>
>> Thanks for inputs. In my case the mapping consists to map an EXTI line
>> with a GIC irq line which could be done using interrupt-map (avoiding
>> to parse it in my driver).
> 
> The problem is that 'interrupt-map' defines an interrupt mapping
> between an input and an output, and that mentioning the GIC in such a
> table will only result in your EXTI to be bypassed.
> 
> 'interrupt-map' really is a dispatch table for targeting an interrupt
> controller (or multiple controllers, even), but really isn't the
> correct tool to carry configuration informations to an interrupt
> controller driver.

Ok so let's forget "interrupt-map"

>> But for each EXTI/GIC association I would
>> like also to describe the EXTI_TYPE (which actually describe the well
>> irqchip to use inside my exti driver) . This property is not generic
>> and so I assume I can't use a generic binding such "interrupt-map".
>>
>> If the solution consists to use a common binding (i.e. interrupt-map)
>> plus a conversion table in exti driver to affect the well irq_chip to
>> the well EXTI line then we could envisage to keep the whole mapping
>> inside the driver (even if it's not the best solution).
> 
> A possible solution would be to have:
> 
> - A set of standard 'interrupts' properties describing the output
> signals
> 
> - A set of properties describing the input to output mapping (if
> relevant) and additional configuration information that for the
> interrupt controller driver.

Does it means to have my own description of "interrupt" property using 
xlate function in EXTI driver ?

something like that:

interrupt = <GIC_SPI 6 EXTI_LINE EXTI_TYPE>, ...

regards
Alex
> 
> 	M.
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
index d19c881b4abc..e08bb51e97a8 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.yaml
@@ -41,6 +41,17 @@  properties:
     description:
       Interrupts references to primary interrupt controller
 
+  st,exti-mapping:
+    $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
+    description: |
+            Define mapping between EXTI lines and GIC irq lines. Should be:
+            st,exti-mapping = <EXTI_LINE GIC_IRQ EXTI_TYPE>, ...;
+            With:
+            - EXTI_LINE: EXTI line number.
+            - GIC_IRQ: GIC IRQ associated to the EXTI line.
+            - EXTI_TYPE: STM32_EXTI_TYPE_CONFIGURABLE or STM32_EXTI_TYPE_DIRECT.
+              Defined in include/dt-bindings/interrupt-controller/stm32-exti.h
+
 required:
   - "#interrupt-cells"
   - compatible
diff --git a/include/dt-bindings/interrupt-controller/stm32-exti.h b/include/dt-bindings/interrupt-controller/stm32-exti.h
new file mode 100644
index 000000000000..02b7e0e30cf7
--- /dev/null
+++ b/include/dt-bindings/interrupt-controller/stm32-exti.h
@@ -0,0 +1,10 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_STM32_EXTI_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_STM32_EXTI_H
+
+#define STM32_EXTI_TYPE_CONFIGURABLE	0
+#define STM32_EXTI_TYPE_DIRECT		1
+
+#define STM32_EXTI_MAPPING_CELL_NB	3
+
+#endif