Re: [PATCH RFC v5 02/12] dt-bindings: soc: zte: Add zx297520v3 matrix clock and reset bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The dt-bindings headers for the new matrixcrm clocks and resets reuse the exact same indices and generic prefix (`ZX297520V3_`) as the existing topcrm clocks and resets, creating a risk of silent device tree misconfigurations.
--

commit 5a96f2683fa1c9255c9f0d3649cfb566a763da11
Author: Stefan Dösinger <stefandoesinger@gmail.com>

dt-bindings: soc: zte: Add zx297520v3 matrix clock and reset bindings

This adds the device tree binding schema for the ZTE zx297520v3 matrix clock
and reset controller. It describes the clock inputs, subdivisions, and hardware
spinlocks. It also appends the new clock and reset indices to the corresponding
dt-binding headers.

> diff --git a/include/dt-bindings/clock/zte,zx297520v3-clk.h b/include/dt-bindings/clock/zte,zx297520v3-clk.h
> index de1c08b6a5a9f..8a6aa456a7089 100644
> --- a/include/dt-bindings/clock/zte,zx297520v3-clk.h
> +++ b/include/dt-bindings/clock/zte,zx297520v3-clk.h

[ ... ]

> @@ -94,4 +94,41 @@
>  #define ZX297520V3_HSIC_WCLK			86
>  #define ZX297520V3_HSIC_PCLK			87
>  
> +#define ZX297520V3_CPU_WCLK			1
> +#define ZX297520V3_CPU_PCLK			2

[Severity: Low]
Is there a risk of silent device tree misconfigurations by reusing the exact
same generic ZX297520V3_ prefix for these new matrixcrm clocks?

Since they restart the index values at 1, they overlap with the existing topcrm
clock macros. If a device tree author mistakenly uses a phandle to topcrm with
one of these matrixcrm macros like <&topcrm ZX297520V3_CPU_WCLK>, it will
silently compile to <&topcrm 1>, which configures a completely different
hardware clock at runtime without any warning.

Would it be safer to use a distinct prefix like ZX297520V3_MATRIX_ for these
new macros to prevent mixing them up with the topcrm ones?

[ ... ]

> diff --git a/include/dt-bindings/reset/zte,zx297520v3-reset.h b/include/dt-bindings/reset/zte,zx297520v3-reset.h
> index 43db72bb59de0..81ffc8bc34c5c 100644
> --- a/include/dt-bindings/reset/zte,zx297520v3-reset.h
> +++ b/include/dt-bindings/reset/zte,zx297520v3-reset.h

[ ... ]

> @@ -29,4 +29,14 @@
>  #define ZX297520V3_HSIC_PHY_RESET		20
>  #define ZX297520V3_HSIC_RESET			21
>  
> +#define ZX297520V3_CPU_RESET			0
> +#define ZX297520V3_EDCP_RESET			1

[Severity: Low]
Does this have the same overlapping index issue as the clocks above?

These resets also use the generic ZX297520V3_ prefix while starting over at
index 0, creating a similar risk where a device tree author might accidentally
mix macros from different reset controllers without getting any compiler
warnings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-zx29clk-v5-0-79ff044e4192@gmail.com?part=2