[v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

  Convert the Atmel AIC binding document to DT schema format using
json-schema.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Changelog
v1 -> v2
- Drop the '|' as there is no formatting to preserve.
- Remove unnecessary marketing statement from description.
- Drop the description for interrupts and reg, it's obvious.
- Put reg after compatible.
- Drop comment in example.
- Drop the example of device that is wired to an AIC as it's(dma) binding is
  not yet available.
---
 .../interrupt-controller/atmel,aic.txt        | 43 -----------
 .../interrupt-controller/atmel,aic.yaml       | 74 +++++++++++++++++++
 2 files changed, 74 insertions(+), 43 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml


base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
  

On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:

> +examples:
> +  - |
> +    interrupt-controller@fffff000 {
> +      compatible = "atmel,at91rm9200-aic";
> +      interrupt-controller;
> +      #interrupt-cells = <3>;
> +      reg = <0xfffff000 0x200>;

One nit if there's some other reason for a respin, reg before
interrupt-controller.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.
  

On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:
> Convert the Atmel AIC binding document to DT schema format using
> json-schema.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
> Changelog
> v1 -> v2
> - Drop the '|' as there is no formatting to preserve.
> - Remove unnecessary marketing statement from description.
> - Drop the description for interrupts and reg, it's obvious.
> - Put reg after compatible.
> - Drop comment in example.
> - Drop the example of device that is wired to an AIC as it's(dma) binding is
>   not yet available.
> ---
>  .../interrupt-controller/atmel,aic.txt        | 43 -----------
>  .../interrupt-controller/atmel,aic.yaml       | 74 +++++++++++++++++++
>  2 files changed, 74 insertions(+), 43 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
> deleted file mode 100644
> index 7079d44bf3ba..000000000000
> --- a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -* Advanced Interrupt Controller (AIC)
> -
> -Required properties:
> -- compatible: Should be:
> -    - "atmel,<chip>-aic" where  <chip> can be "at91rm9200", "sama5d2",
> -      "sama5d3" or "sama5d4"
> -    - "microchip,<chip>-aic" where <chip> can be "sam9x60"
> -
> -- interrupt-controller: Identifies the node as an interrupt controller.
> -- #interrupt-cells: The number of cells to define the interrupts. It should be 3.
> -  The first cell is the IRQ number (aka "Peripheral IDentifier" on datasheet).
> -  The second cell is used to specify flags:
> -    bits[3:0] trigger type and level flags:
> -      1 = low-to-high edge triggered.
> -      2 = high-to-low edge triggered.
> -      4 = active high level-sensitive.
> -      8 = active low level-sensitive.
> -      Valid combinations are 1, 2, 3, 4, 8.
> -      Default flag for internal sources should be set to 4 (active high).
> -  The third cell is used to specify the irq priority from 0 (lowest) to 7
> -  (highest).
> -- reg: Should contain AIC registers location and length
> -- atmel,external-irqs: u32 array of external irqs.
> -
> -Examples:
> -	/*
> -	 * AIC
> -	 */
> -	aic: interrupt-controller@fffff000 {
> -		compatible = "atmel,at91rm9200-aic";
> -		interrupt-controller;
> -		#interrupt-cells = <3>;
> -		reg = <0xfffff000 0x200>;
> -	};
> -
> -	/*
> -	 * An interrupt generating device that is wired to an AIC.
> -	 */
> -	dma: dma-controller@ffffec00 {
> -		compatible = "atmel,at91sam9g45-dma";
> -		reg = <0xffffec00 0x200>;
> -		interrupts = <21 4 5>;
> -	};
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
> new file mode 100644
> index 000000000000..df81115a8b7f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/atmel,aic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Advanced Interrupt Controller (AIC)
> +
> +maintainers:
> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
> +  - Dharma balasubiramani <dharma.b@microchip.com>
> +
> +description:
> +  The Advanced Interrupt Controller (AIC) is an 8-level priority, individually
> +  maskable, vectored interrupt controller providing handling of up to one
> +  hundred and twenty-eight interrupt sources.
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91rm9200-aic
> +      - atmel,sama5d2-aic
> +      - atmel,sama5d3-aic
> +      - atmel,sama5d4-aic
> +      - microchip,sam9x60-aic
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 3
> +    description: |
> +      The 1st cell is the IRQ number (Peripheral IDentifier on datasheet).
> +      The 2nd cell specifies flags:
> +        bits[3:0] trigger type and level flags:
> +          1 = low-to-high edge triggered.
> +          2 = high-to-low edge triggered.
> +          4 = active high level-sensitive.
> +          8 = active low level-sensitive.
> +        Valid combinations: 1, 2, 3, 4, 8.
> +        Default for internal sources: 4 (active high).
> +      The 3rd cell specifies irq priority from 0 (lowest) to 7 (highest).
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  atmel,external-irqs:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: u32 array of external irqs.

Constraints on the array size and/or entry values?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - atmel,external-irqs
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    interrupt-controller@fffff000 {
> +      compatible = "atmel,at91rm9200-aic";
> +      interrupt-controller;
> +      #interrupt-cells = <3>;
> +      reg = <0xfffff000 0x200>;
> +      atmel,external-irqs = <31>;
> +    };
> +...
> 
> base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
> -- 
> 2.25.1
>
  

Hi Rob,

On 12/02/24 7:38 pm, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:
>> Convert the Atmel AIC binding document to DT schema format using
>> json-schema.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>> ---
>> Changelog
>> v1 -> v2
>> - Drop the '|' as there is no formatting to preserve.
>> - Remove unnecessary marketing statement from description.
>> - Drop the description for interrupts and reg, it's obvious.
>> - Put reg after compatible.
>> - Drop comment in example.
>> - Drop the example of device that is wired to an AIC as it's(dma) binding is
>>    not yet available.
>> ---
>>   .../interrupt-controller/atmel,aic.txt        | 43 -----------
>>   .../interrupt-controller/atmel,aic.yaml       | 74 +++++++++++++++++++
>>   2 files changed, 74 insertions(+), 43 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
>>   create mode 100644 Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
>> deleted file mode 100644
>> index 7079d44bf3ba..000000000000
>> --- a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -* Advanced Interrupt Controller (AIC)
>> -
>> -Required properties:
>> -- compatible: Should be:
>> -    - "atmel,<chip>-aic" where  <chip> can be "at91rm9200", "sama5d2",
>> -      "sama5d3" or "sama5d4"
>> -    - "microchip,<chip>-aic" where <chip> can be "sam9x60"
>> -
>> -- interrupt-controller: Identifies the node as an interrupt controller.
>> -- #interrupt-cells: The number of cells to define the interrupts. It should be 3.
>> -  The first cell is the IRQ number (aka "Peripheral IDentifier" on datasheet).
>> -  The second cell is used to specify flags:
>> -    bits[3:0] trigger type and level flags:
>> -      1 = low-to-high edge triggered.
>> -      2 = high-to-low edge triggered.
>> -      4 = active high level-sensitive.
>> -      8 = active low level-sensitive.
>> -      Valid combinations are 1, 2, 3, 4, 8.
>> -      Default flag for internal sources should be set to 4 (active high).
>> -  The third cell is used to specify the irq priority from 0 (lowest) to 7
>> -  (highest).
>> -- reg: Should contain AIC registers location and length
>> -- atmel,external-irqs: u32 array of external irqs.
>> -
>> -Examples:
>> -     /*
>> -      * AIC
>> -      */
>> -     aic: interrupt-controller@fffff000 {
>> -             compatible = "atmel,at91rm9200-aic";
>> -             interrupt-controller;
>> -             #interrupt-cells = <3>;
>> -             reg = <0xfffff000 0x200>;
>> -     };
>> -
>> -     /*
>> -      * An interrupt generating device that is wired to an AIC.
>> -      */
>> -     dma: dma-controller@ffffec00 {
>> -             compatible = "atmel,at91sam9g45-dma";
>> -             reg = <0xffffec00 0x200>;
>> -             interrupts = <21 4 5>;
>> -     };
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
>> new file mode 100644
>> index 000000000000..df81115a8b7f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
>> @@ -0,0 +1,74 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/interrupt-controller/atmel,aic.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Advanced Interrupt Controller (AIC)
>> +
>> +maintainers:
>> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
>> +  - Dharma balasubiramani <dharma.b@microchip.com>
>> +
>> +description:
>> +  The Advanced Interrupt Controller (AIC) is an 8-level priority, individually
>> +  maskable, vectored interrupt controller providing handling of up to one
>> +  hundred and twenty-eight interrupt sources.
>> +
>> +allOf:
>> +  - $ref: /schemas/interrupt-controller.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - atmel,at91rm9200-aic
>> +      - atmel,sama5d2-aic
>> +      - atmel,sama5d3-aic
>> +      - atmel,sama5d4-aic
>> +      - microchip,sam9x60-aic
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 3
>> +    description: |
>> +      The 1st cell is the IRQ number (Peripheral IDentifier on datasheet).
>> +      The 2nd cell specifies flags:
>> +        bits[3:0] trigger type and level flags:
>> +          1 = low-to-high edge triggered.
>> +          2 = high-to-low edge triggered.
>> +          4 = active high level-sensitive.
>> +          8 = active low level-sensitive.
>> +        Valid combinations: 1, 2, 3, 4, 8.
>> +        Default for internal sources: 4 (active high).
>> +      The 3rd cell specifies irq priority from 0 (lowest) to 7 (highest).
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  atmel,external-irqs:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description: u32 array of external irqs.
> 
> Constraints on the array size and/or entry values?

The hardware's support for external IRQs may differ, which is why a u32 
array is utilized. This choice is based on the fact that IRQ numbers are 
commonly expressed as integers, and a 32-bit unsigned integer provides a 
standardized size capable of representing a broad range of numbers. This 
size is more than adequate for accommodating IRQ numbering.


> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupt-controller
>> +  - "#interrupt-cells"
>> +  - atmel,external-irqs
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    interrupt-controller@fffff000 {
>> +      compatible = "atmel,at91rm9200-aic";
>> +      interrupt-controller;
>> +      #interrupt-cells = <3>;
>> +      reg = <0xfffff000 0x200>;
>> +      atmel,external-irqs = <31>;
>> +    };
>> +...
>>
>> base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
>> --
>> 2.25.1
>>

-- 
With Best Regards,
Dharma B.
  

On Tue, Feb 13, 2024 at 04:23:36AM +0000, Dharma.B@microchip.com wrote:
> On 12/02/24 7:38 pm, Rob Herring wrote:
> > On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:
> >> +  atmel,external-irqs:
> >> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >> +    description: u32 array of external irqs.
> > 
> > Constraints on the array size and/or entry values?
> 
> The hardware's support for external IRQs may differ, which is why a u32 
> array is utilized. This choice is based on the fact that IRQ numbers are 
> commonly expressed as integers, and a 32-bit unsigned integer provides a 
> standardized size capable of representing a broad range of numbers. This 
> size is more than adequate for accommodating IRQ numbering.

I don't think Rob was questioning your use of u32s, but rather the fact
that you do not limit the values at all nor the number of values.
  

Hi Rob and Conor,

On 14/02/24 12:43 am, Conor Dooley wrote:
> On Tue, Feb 13, 2024 at 04:23:36AM +0000,Dharma.B@microchip.com  wrote:
>> On 12/02/24 7:38 pm, Rob Herring wrote:
>>> On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:
>>>> +  atmel,external-irqs:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +    description: u32 array of external irqs.
>>> Constraints on the array size and/or entry values?
>> The hardware's support for external IRQs may differ, which is why a u32
>> array is utilized. This choice is based on the fact that IRQ numbers are
>> commonly expressed as integers, and a 32-bit unsigned integer provides a
>> standardized size capable of representing a broad range of numbers. This
>> size is more than adequate for accommodating IRQ numbering.
> I don't think Rob was questioning your use of u32s, but rather the fact
> that you do not limit the values at all nor the number of values.

The peripheral identification defined at the product level corresponds 
to the interrupt source number.

SoC           External Interrupts    Peripheral ID
AT91RM9200    - IRQ0–IRQ6            25 - 31
SAMA5D2       - IRQ0–IRQn            49
SAMA5D3       - IRQ0–IRQn            47
SAMA5D4       - IRQ0–IRQn            56
SAM9x60       - IRQ0–IRQn            31

To reflect these constraints in bindings, I intend to make the following 
changes.

   atmel,external-irqs:
     $ref: /schemas/types.yaml#/definitions/uint32-array
     description: u32 array of external irqs.
     if:
       properties:
         compatible:
           contains:
             const: atmel,at91rm9200-aic
     then:
       minItems: 1
       maxItems: 7
     else:
       minItems: 1
       maxItems: 1


-- 
With Best Regards,
Dharma B.
  

On Mon, Feb 19, 2024 at 03:30:52PM +0000, Dharma.B@microchip.com wrote:
> Hi Rob and Conor,
> 
> On 14/02/24 12:43 am, Conor Dooley wrote:
> > On Tue, Feb 13, 2024 at 04:23:36AM +0000,Dharma.B@microchip.com  wrote:
> >> On 12/02/24 7:38 pm, Rob Herring wrote:
> >>> On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:
> >>>> +  atmel,external-irqs:
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >>>> +    description: u32 array of external irqs.
> >>> Constraints on the array size and/or entry values?
> >> The hardware's support for external IRQs may differ, which is why a u32
> >> array is utilized. This choice is based on the fact that IRQ numbers are
> >> commonly expressed as integers, and a 32-bit unsigned integer provides a
> >> standardized size capable of representing a broad range of numbers. This
> >> size is more than adequate for accommodating IRQ numbering.
> > I don't think Rob was questioning your use of u32s, but rather the fact
> > that you do not limit the values at all nor the number of values.
> 
> The peripheral identification defined at the product level corresponds 
> to the interrupt source number.
> 
> SoC           External Interrupts    Peripheral ID
> AT91RM9200    - IRQ0–IRQ6            25 - 31
> SAMA5D2       - IRQ0–IRQn            49
> SAMA5D3       - IRQ0–IRQn            47
> SAMA5D4       - IRQ0–IRQn            56
> SAM9x60       - IRQ0–IRQn            31
> 
> To reflect these constraints in bindings, I intend to make the following 
> changes.
> 
>    atmel,external-irqs:
>      $ref: /schemas/types.yaml#/definitions/uint32-array
>      description: u32 array of external irqs.

>      if:
>        properties:
>          compatible:
>            contains:
>              const: atmel,at91rm9200-aic
>      then:
>        minItems: 1
>        maxItems: 7
>      else:
>        minItems: 1
>        maxItems: 1

Just to point out, that if this is not psuedocode, the syntax here is
not quite right. It should be:

allOf:
- if:
    properties:
      compatible:
        contains:
          const: atmel,at91rm9200-aic
  then:
    properties:
      atmel,external-irqs:
        minItems: 1
        maxItems: 7
  else:
    properties:
      atmel,external-irqs:
        minItems: 1
        maxItems: 1

But you can simply this further by applying minItems: 1 & maxitems: 7
to the property directly and just setting maxItems: 1 when the
compatible is not atmel,at91rm9200-aic.

There should be plenty of examples in-tree for you to copy this sort of
thing from - clock-names is a property you often see this used for.

Cheers,
Conor.
  

On 21/02/24 1:02 am, Conor Dooley wrote:
> On Mon, Feb 19, 2024 at 03:30:52PM +0000,Dharma.B@microchip.com  wrote:
>> Hi Rob and Conor,
>>
>> On 14/02/24 12:43 am, Conor Dooley wrote:
>>> On Tue, Feb 13, 2024 at 04:23:36AM +0000,Dharma.B@microchip.com   wrote:
>>>> On 12/02/24 7:38 pm, Rob Herring wrote:
>>>>> On Fri, Feb 09, 2024 at 03:31:22PM +0530, Dharma Balasubiramani wrote:
>>>>>> +  atmel,external-irqs:
>>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>>> +    description: u32 array of external irqs.
>>>>> Constraints on the array size and/or entry values?
>>>> The hardware's support for external IRQs may differ, which is why a u32
>>>> array is utilized. This choice is based on the fact that IRQ numbers are
>>>> commonly expressed as integers, and a 32-bit unsigned integer provides a
>>>> standardized size capable of representing a broad range of numbers. This
>>>> size is more than adequate for accommodating IRQ numbering.
>>> I don't think Rob was questioning your use of u32s, but rather the fact
>>> that you do not limit the values at all nor the number of values.
>> The peripheral identification defined at the product level corresponds
>> to the interrupt source number.
>>
>> SoC           External Interrupts    Peripheral ID
>> AT91RM9200    - IRQ0–IRQ6            25 - 31
>> SAMA5D2       - IRQ0–IRQn            49
>> SAMA5D3       - IRQ0–IRQn            47
>> SAMA5D4       - IRQ0–IRQn            56
>> SAM9x60       - IRQ0–IRQn            31
>>
>> To reflect these constraints in bindings, I intend to make the following
>> changes.
>>
>>     atmel,external-irqs:
>>       $ref: /schemas/types.yaml#/definitions/uint32-array
>>       description: u32 array of external irqs.
>>       if:
>>         properties:
>>           compatible:
>>             contains:
>>               const: atmel,at91rm9200-aic
>>       then:
>>         minItems: 1
>>         maxItems: 7
>>       else:
>>         minItems: 1
>>         maxItems: 1
> Just to point out, that if this is not psuedocode, the syntax here is
> not quite right. It should be:
> 
> allOf:
> - if:
>      properties:
>        compatible:
>          contains:
>            const: atmel,at91rm9200-aic
>    then:
>      properties:
>        atmel,external-irqs:
>          minItems: 1
>          maxItems: 7
>    else:
>      properties:
>        atmel,external-irqs:
>          minItems: 1
>          maxItems: 1
> 
> But you can simply this further by applying minItems: 1 & maxitems: 7
> to the property directly and just setting maxItems: 1 when the
> compatible is not atmel,at91rm9200-aic.
> 
> There should be plenty of examples in-tree for you to copy this sort of
> thing from - clock-names is a property you often see this used for.

Sure, Thanks for pointing it out.

I will address your previous suggestion "reg before
interrupt-controller." as well in v3.

> 
> Cheers,
> Conor.

-- 
With Best Regards,
Dharma B.