Re: [PATCH v9 02/12] dt-bindings: clk: zte: Add zx297520v3 matrix clock and reset controller

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Overlapping macro definitions in shared dt-bindings headers create a naming collision risk.
--

--- Patch [2]: [PATCH v9 02/12] dt-bindings: clk: zte: Add zx297520v3 matrix clock and reset controller ---
commit 504f3c5393f4a8144508453d38c306eb0f25c3f4
Author: Stefan Dösinger <[email protected]>

dt-bindings: clk: zte: Add zx297520v3 matrix clock and reset controller

This commit adds device tree bindings for the zx297520v3 matrix clock and
reset controller. It introduces new macros for clocks and resets in the
existing zte,zx297520v3 headers to support the matrixcrm block.

> diff --git a/include/dt-bindings/clock/zte,zx297520v3-clk.h b/include/dt-bindings/clock/zte,zx297520v3-clk.h
> index 3aed94ccc26fe..a8477e9b2a149 100644
> --- a/include/dt-bindings/clock/zte,zx297520v3-clk.h
> +++ b/include/dt-bindings/clock/zte,zx297520v3-clk.h
> @@ -63,4 +63,45 @@
>  #define ZX297520V3_HSIC_WCLK			54
>  #define ZX297520V3_HSIC_PCLK			55
>  
> +#define ZX297520V3_SYS_TIMER_WCLK		0
> +#define ZX297520V3_CPU_WCLK			1

[Severity: Medium]
Could this naming convention lead to silent misconfigurations in device trees?

These new matrixcrm macros share the exact same ZX297520V3_ prefix as the
existing topcrm macros in this header, and their indices start over at 0.

Since both sets of macros resolve to valid integers within the same namespace,
accidentally referencing a topcrm macro instead of a matrixcrm macro in a
device tree will not trigger a compiler error, making hardware misconfigurations
difficult to debug.

[ ... ]

> diff --git a/include/dt-bindings/reset/zte,zx297520v3-reset.h b/include/dt-bindings/reset/zte,zx297520v3-reset.h
> index f60aaccf0f4d7..d79fa601afa68 100644
> --- a/include/dt-bindings/reset/zte,zx297520v3-reset.h
> +++ b/include/dt-bindings/reset/zte,zx297520v3-reset.h
> @@ -27,4 +27,16 @@
>  #define ZX297520V3_USB_RESET			18
>  #define ZX297520V3_HSIC_RESET			19
>  
> +#define ZX297520V3_CPU_RESET			0
> +#define ZX297520V3_DDR_CTRL_RESET		1

[Severity: Medium]
Does this carry the same namespace collision risk as the clock macros above?

Since the matrixcrm reset indices also start at 0 and use the identical
ZX297520V3_ prefix as the existing topcrm reset definitions, there is no built-in
protection against mixing them up when writing the device tree.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2