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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The compatible string `apple,t6031-aic3` explicitly referenced in the commit message is completely missing from both the top-level schema `properties: compatible` definition and the inverted `#interrupt-cells` 4-cell conditional block, defeating the stated purpose of the patch.
--

--- Patch [2]: [PATCH 02/11] dt-bindings: interrupt-controller: apple,aic2: Invert #interrupt-cells condition ---
commit 96c242a2f78e06c67eac2004fca37e5d7d112361
Author: Janne Grunau <j@jannau.net>

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

This commit updates the #interrupt-cells condition by explicitly listing
the compatibles used for multi-die SoCs and assigning them 4 cells, while
leaving 3 cells as the default for others.

>     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
>     interrupt cell for the die number.
>     Instead list the compatibles used for multi-die SoCs explicitly and use
>     4 #interrupt-cells for them.

[Severity: Medium]
The commit message mentions adding support for "apple,t6031-aic3", but is
this compatible string missing from the schema entirely? 

It appears that "apple,t6031-aic3" is not present in the top-level properties
compatible definition. Without being listed there, would device trees using
this string fail standard schema validation?

> 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: Medium]
Does this conditional block omit the M3 Max/Ultra compatibles? 

Since "apple,t6031-aic3" and "apple,t8122-aic3" are not included in this
explicit list of multi-die SoCs, they will fall through to the else branch.
Will they incorrectly receive a 3-cell constraint instead of the 4 cells
intended for multi-die devices?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-apple-t603x-initial-devices-v1-0-55b305833123@jannau.net?part=2