Re: [PATCH v3 1/6] dt-bindings: interrupt-controller: Refine size/interrupt-cell usage.

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/12/2024 16:52, Kevin Chen wrote:
> 1. Because size-cells is no need to use 2, modify to 1 for use.

???

> 2. Add minItems to 1 for interrupts for intc1.

???

> 3. Add 1 interrupt of intc1 example into yaml file.

> 4. Add intc1 sub-module of uart12 as example using the intc0 and intc1.

What is all this?

BTW, there was no such patch in previous version and your changelog is
silent about it.

Subject: drop all full stops. Subject never ends with full stop.

> ---
>  .../aspeed,ast2700-intc.yaml                  | 60 +++++++++++++++----
>  1 file changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
> index 55636d06a674..eadfbc45326b 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
> @@ -31,6 +31,7 @@ properties:
>        type as defined in interrupt.txt in this directory.
>  
>    interrupts:
> +    minItems: 1

Nope, not explained, not constrained. Your schema is supposed to be
constrained.


>      maxItems: 6
>      description: |
>        Depend to which INTC0 or INTC1 used.
> @@ -68,19 +69,52 @@ examples:
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
>      bus {
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +
> +      intc0: interrupt-controller@12100000 {
> +        compatible = "simple-mfd";
> +        reg = <0 0x12100000 0x4000>;
> +        ranges = <0x0 0x0 0x0 0x12100000 0x4000>;
>          #address-cells = <2>;
> -        #size-cells = <2>;
> -
> -        interrupt-controller@12101b00 {
> -            compatible = "aspeed,ast2700-intc-ic";
> -            reg = <0 0x12101b00 0 0x10>;
> -            #interrupt-cells = <2>;
> -            interrupt-controller;
> -            interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
> -                         <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>,
> -                         <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>,
> -                         <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>,
> -                         <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> -                         <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;


I don't understand what is all this.

> +        #size-cells = <1>;
> +
> +        intc0_11: interrupt-controller@1b00 {
> +          compatible = "aspeed,ast2700-intc-ic";
> +          reg = <0 0x12101b00 0x10>;
> +          #interrupt-cells = <2>;
> +          interrupt-controller;
> +          interrupts = <GIC_SPI 192 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_SPI 193 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_SPI 194 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_SPI 195 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_SPI 196 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_SPI 197 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>          };
> +      };
> +
> +      intc1: interrupt-controller@14c18000 {
> +        compatible = "simple-mfd";
> +        reg = <0 0x14c18000 0x400>;
> +        ranges = <0x0 0x0 0x0 0x14c18000 0x400>;
> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +
> +        intc1_4: interrupt-controller@140 {
> +          compatible = "aspeed,ast2700-intc-ic";
> +          reg = <0x0 0x140 0x10>;
> +          #interrupt-cells = <2>;
> +          interrupt-controller;
> +          interrupts-extended = <&intc0_11 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +        };
> +      };
> +
> +      uart12: serial@14c33b00 {
> +        compatible = "ns16550a";
> +        reg = <0x0 0x14c33b00 0x100>;
> +        interrupts-extended = <&intc1_4 18 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +        reg-shift = <2>;
> +        reg-io-width = <4>;
> +        no-loopback-test;
> +      };

And above is not related at all. Don't add entirely unrelated changes. Drop.


Best regards,
Krzysztof