Re: [PATCH v2 02/10] dt-bindings: interrupt-controller: apple,aic2: Invert #interrupt-cells condition

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The inverted conditional block intended to enforce `#interrupt-cells: 4` for multi-die SoCs omits the new M3 multi-die compatible (`apple,t6031-aic3`), causing it to incorrectly receive a `const: 3` constraint.
- [High] The new `apple,t6031-aic3` compatible string is completely missing from the top-level `compatible` property definition.
--

--- Patch [2]: [PATCH v2 02/10] dt-bindings: interrupt-controller: apple,aic2: Invert #interrupt-cells condition ---
commit c6c471e4280adaa7e66dba52b531c34594c8ccfa
Author: Janne Grunau <[email protected]>

dt-bindings: interrupt-controller: apple,aic2: Invert #interrupt-cells condition

The commit inverts the condition for #interrupt-cells in the apple,aic2
binding. It lists compatibles used for multi-die SoCs explicitly and uses
4 #interrupt-cells for them, while defaulting to 3 for others. The commit
message mentions this accommodates M3 Max and Ultra based devices.

>     The condition for 3 or 4 #interrupt-cells list compatibles for the 3
>     cells variant (not used in SoCs consisting out of multiple dies). This
>     includes the AICv3 base compatible "apple,t8122-aic3". This will match
>         compatible = "apple,t6031-aic3", "apple,t8122-aic3";
>     shared for M3 Max and Ultra based devices which require the additional

[Severity: High]
Is apple,t6031-aic3 missing from the top-level compatible definition?

Looking at the top-level definition in this file, apple,t6031-aic3 is not
listed as a valid compatible string:

properties:
  compatible:
    oneOf:
      - items:
          - enum:
              - apple,t6000-aic
              - apple,t6020-aic
              - apple,t8112-aic
...
      - items:
          - enum:
              - apple,t6030-aic3
          - const: apple,t8122-aic3

Will this cause dt_binding_check to fail for any device tree node using
the apple,t6031-aic3 compatible string, as the string is not recognized as
a valid value?

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
> index d0d9a90e96e7b..a94787939d345 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
> @@ -128,16 +128,16 @@ allOf:
>          compatible:
>            contains:
>              enum:
> -              - apple,t8112-aic
> -              - apple,t8122-aic3
> +              - apple,t6000-aic
> +              - apple,t6020-aic

[Severity: High]
Should apple,t6031-aic3 be included in this enumerated list of multi-die
SoCs?

Because it is omitted here, won't it fall into the else block which enforces
const: 3 instead of the intended const: 4?

This seems like it will cause device tree nodes for M3 Max and Ultra to fail
schema validation when they specify the correct 4 interrupt cells.

>      then:
>        properties:
>          '#interrupt-cells':
> -          const: 3
> +          const: 4
>      else:
>        properties:
>          '#interrupt-cells':
> -          const: 4
> +          const: 3
>  
>  examples:
>    - |

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-apple-t603x-initial-devices-v2-0-df65b2485710@jannau.net?part=2