Re: [PATCH v5 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:
- [Medium] The new compatible string 'airoha,an7583-scu' is missing from the conditional `if-then` blocks defining `reg` constraints.
- [Low] Inconsistent naming and formatting for the MDIO reset definitions.
--

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

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

This commit documents support for the Airoha AN7583 clock based on the
EN7523 clock schema, adding the "airoha,chip-scu" requirement. It also
adds an additional binding for extra 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 eb24a5687639e..6c3c887985153 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
[ ... ]
> @@ -50,12 +51,29 @@ properties:
>      description: ID of the controller reset line
>      const: 1
>  
> +  airoha,chip-scu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to Chip SCU
> +
>  required:
>    - compatible
>    - reg
>    - '#clock-cells'
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: airoha,an7583-scu
> +
> +    then:
> +      required:
> +        - airoha,chip-scu
> +
> +    else:
> +      properties:
> +        airoha,chip-scu: false
> +
>    - if:
>        properties:
>          compatible:

[Severity: Medium]
Since the AN7583 shares the same register layout as the EN7581, shouldn't
"airoha,an7583-scu" also be added to the if-then conditional block further
down that enforces maxItems: 1 for the reg property?

Omitting it leaves the reg property unconstrained for this new hardware
generation, which bypasses validation constraints.

> diff --git a/include/dt-bindings/reset/airoha,an7583-reset.h b/include/dt-bindings/reset/airoha,an7583-reset.h
> new file mode 100644
> index 0000000000000..7ff07986f8ba2
> --- /dev/null
> +++ b/include/dt-bindings/reset/airoha,an7583-reset.h
> @@ -0,0 +1,62 @@
[ ... ]
> +#define AN7583_TDMA_RST			19
> +#define AN7583_EMMC_RST			20
> +#define AN7583_SOE_RST			21
> +#define AN7583_XFP_MAC_RST		22
> +#define AN7583_MDIO0                    23
> +#define AN7583_MDIO1                    24

[Severity: Low]
Are the _RST suffixes intentionally omitted from AN7583_MDIO0 and
AN7583_MDIO1?

They seem to be missing the suffix used by all other reset constants in this
file, which might cause ambiguity with the matching clock macros.

Could these also be updated to use tabs instead of spaces for value alignment
to match the rest of the file?

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