Re: [PATCH v2 05/13] dt-bindings: soc: synaptics: add SL2610 system controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] dt-bindings: soc: synaptics: split dissimilar SL2610 syscon
  blocks into separate schemas
--

commit bc8ce74277532ebd4eaa1fb7ce6d4be07d8dfe52
Author: Jisheng Zhang <jszhang@kernel.org>

dt-bindings: soc: synaptics: add SL2610 system controllers

This commit adds a Device Tree binding schema and clock/reset header
definitions for the Synaptics SL2610 SoC, MCU, AVIO, and CPU system
control register blocks.

> diff --git a/Documentation/devicetree/bindings/soc/synaptics/syna,sl2610-soc-syscon.yaml b/Documentation/devicetree/bindings/soc/synaptics/syna,sl2610-soc-syscon.yaml
> new file mode 100644
> index 0000000000000..cdf947cb3e0f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/synaptics/syna,sl2610-soc-syscon.yaml
> @@ -0,0 +1,178 @@

[ ... ]

> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - syna,sl2610-soc-syscon
> +          - syna,sl2610-mcu-syscon
> +          - syna,sl2610-avio-syscon
> +          - syna,sl2610-cpu-syscon
> +      - const: syscon

[ ... ]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - syna,sl2610-soc-syscon
> +              - syna,sl2610-mcu-syscon
> +    then:
> +      required:
> +        - '#clock-cells'
> +        - '#reset-cells'
> +    else:
> +      properties:
> +        '#reset-cells': false
> +      patternProperties:
> +        '-(pmux|pins)$': false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: syna,sl2610-cpu-syscon
> +    then:
> +      required:
> +        - '#clock-cells'
> +        - syna,gbl-syscon
> +    else:
> +      properties:
> +        syna,gbl-syscon: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: syna,sl2610-avio-syscon
> +    then:
> +      properties:
> +        '#clock-cells': false

[Severity: Low]
Should these dissimilar system controller blocks be split into separate
binding schemas instead of combining them with three per-compatible
if-then-else blocks in allOf?

Only the SoC and MCU blocks provide resets and pin mux subnodes, only the
CPU block uses syna,gbl-syscon, and the AVIO block exposes none of these
providers.  Splitting the schema by block role would avoid toggling most
properties and patternProperties per compatible.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115102401.3520-1-jszhang@kernel.org?part=5