Re: [PATCH v6 1/6] dt-bindings: spmi: apple,spmi: Add t603x and t8122

[email protected]
Newsgroups 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 binding introduces interrupt controller provider capabilities but fails to add the mandatory properties to the `required` list, violating subsystem guidelines.
--

--- Patch [1]: [PATCH v6 1/6] dt-bindings: spmi: apple,spmi: Add t603x and t8122 ---
commit 55496611e1596aed1ddc4ec7dccafdc5fc8fb28b
Author: Sasha Finkelstein <[email protected]>

dt-bindings: spmi: apple,spmi: Add t603x and t8122

This commit adds t603x and t8122 compatibles, interrupt support, and
support for SPMI controllers that are not always-on to the apple,spmi
binding.

> diff --git a/Documentation/devicetree/bindings/spmi/apple,spmi.yaml b/Documentation/devicetree/bindings/spmi/apple,spmi.yaml
> --- a/Documentation/devicetree/bindings/spmi/apple,spmi.yaml
> +++ b/Documentation/devicetree/bindings/spmi/apple,spmi.yaml
[ ... ]
> @@ -34,6 +37,18 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  interrupts:
> +    maxItems: 1
> +    description: Optional, operates in polled mode if not present
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  power-domains:
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - reg

[Severity: Medium]
The commit message notes that the interrupt controller functionality is kept
optional since it is only needed on 3 out of 10 controllers. However, does
leaving these properties out of the required block allow incomplete device
tree nodes to pass schema validation?

According to the subsystem guidelines for hardware variants, when new provider
capabilities are added, the corresponding properties like interrupt-controller
and "#interrupt-cells" must be included in the required list.

Should these properties be added to the required list under an if-then block
matching the specific hardware variations that support this functionality, or
should a dependency be added to ensure that "#interrupt-cells" is always present
when interrupt-controller is used?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.