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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: missing #reset-cells required constraint for an7583-scu
- [Low] dt-bindings: semantic mismatch in airoha,chip-scu property description
--

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

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

Document support for the Airoha AN7583 clock. This is based on the EN7523
clock schema with the new requirement of the "airoha,chip-scu" phandle.
The patch also adds additional bindings for clock and reset lines.

> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index fc37786ac7ed8..8b94113d00ab7 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
[ ... ]
> @@ -57,12 +58,34 @@ 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]
Is this description accurate regarding PCIe configuration?

The commit message states that the Chip SCU provides access to some clock
gates and rates, while the SCU provides registers for resets and PCIe clocks.

This description attributes PCIe clocks and resets to the Chip SCU rather
than the main SCU. Could this description be updated to match the hardware
architecture outlined in the commit message?

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

[Severity: Low]
Does this new compatible string also need to require #reset-cells?

The patch introduces dedicated reset line definitions in
include/dt-bindings/reset/airoha,an7583-reset.h, indicating this variant acts
as a reset provider.

When a hardware variant acts as a provider, the corresponding standard
cell-count property should be explicitly added to the required list.

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