2024-02-09 10:02:21

by Dharma Balasubiramani

[permalink] [raw]
Subject: [PATCH 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 <[email protected]>
---
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 <[email protected]>
+ - Dharma balasubiramani <[email protected]>
+
+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.
+
+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



2024-02-09 16:52:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

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 <[email protected]>

Cheers,
Conor.


Attachments:
(No filename) (454.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-12 14:14:12

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

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 <[email protected]>
> ---
> 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 <[email protected]>
> + - Dharma balasubiramani <[email protected]>
> +
> +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
>

2024-02-13 04:23:55

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> + - Dharma balasubiramani <[email protected]>
>> +
>> +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.

2024-02-13 19:14:22

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

On Tue, Feb 13, 2024 at 04:23:36AM +0000, [email protected] 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.


Attachments:
(No filename) (915.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-19 15:31:29

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

Hi Rob and Conor,

On 14/02/24 12:43 am, Conor Dooley wrote:
> On Tue, Feb 13, 2024 at 04:23:36AM +0000,[email protected] 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.

2024-02-20 19:33:08

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

On Mon, Feb 19, 2024 at 03:30:52PM +0000, [email protected] 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,[email protected] 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.


Attachments:
(No filename) (2.65 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-22 03:42:29

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: Convert Atmel AIC to json-schema

On 21/02/24 1:02 am, Conor Dooley wrote:
> On Mon, Feb 19, 2024 at 03:30:52PM +0000,[email protected] 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,[email protected] 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.