Re: [PATCH 02/11] dt-bindings: interrupt-controller: apple,aic2: Invert #interrupt-cells condition
[email protected] Thu, 09 Jul 2026 07:42:02 +0000
| Newsgroups | org.kernel.vger.linux-watchdog,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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