Re: [PATCH v11 1/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] dt-bindings: clock: airoha: unwieldy per-compatible conditionals
  in airoha,en7523-scu.yaml
- [Low] dt-bindings: clock: airoha: inaccurate description of
  airoha,chip-scu in airoha,en7523-scu.yaml
--

commit 94419d913db80e12747b18f5e97b56dbea634264
Author: Christian Marangi <ansuelsmth@gmail.com>

dt-bindings: clock: airoha: Document support for AN7583 clock

This commit documents Device Tree bindings for the Airoha AN7583 clock and
reset controller in airoha,en7523-scu.yaml, and adds clock and reset ID
definitions for AN7583.

> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index fc37786ac7ed8..05cd20ce3ab91 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml

[ ... ]

> @@ -57,12 +58,36 @@ properties:
>        available serdes port.
>      const: 1
>  
> +  airoha,chip-scu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the Chip SCU providing the registers required
> +      for configuring the PCIe related clocks and resets.

[Severity: Low]
As reported by Brian Masney on v9 [1], is the description of
airoha,chip-scu accurate?  As noted in the commit message, the Chip SCU
provides registers for clock gates, base clock rates, and dividers, whereas
resets and PCIe clocks live in the main SCU register block.

> +
>  required:
>    - compatible
>    - reg
>    - '#clock-cells'
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: airoha,an7583-scu
> +
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1
> +
> +      required:
> +        - airoha,chip-scu
> +        - '#reset-cells'
> +        - '#phy-cells'
> +
> +    else:
> +      properties:
> +        airoha,chip-scu: false

[Severity: Low]
This isn't a bug, but adding airoha,an7583-scu gives every compatible in
this binding its own if-then block under allOf, with differing rules for
reg, #phy-cells, #reset-cells, and airoha,chip-scu.  Would it be cleaner to
split the binding into separate schema documents rather than accumulating
per-compatible conditionals here?

[1] https://lore.kernel.org/r/179383990045.753844.10400171550739497565.b4-review@b4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106115852.189098-1-ansuelsmth@gmail.com?part=1