Re: [PATCH v2 05/24] dt-bindings: reset: Add ZTE ZX279128S reset controller bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: reset: unstable ABI via _COUNT macros
- [Low] dt-bindings: clock/reset: patch ordering breaks git bisect
--

commit 964ebb4388d7c67dbfa57a9fa7d74c6b1cd7fe12
Author: Navid Ghahremani <ghahramani.navid@gmail.com>

dt-bindings: reset: Add ZTE ZX279128S reset controller bindings

Define logical reset indices (0, 1, 2...) for the topcrm and crm2
reset controllers on ZTE ZX279128S SoCs.

[Severity: Low]
Does this patch ordering break git bisect? 

The preceding commit in the series introduces
Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml, which
explicitly includes <dt-bindings/reset/zte,zx279128s-reset.h> in its
examples section. However, that header file is not introduced until this
current patch. 

This dependency violation causes a missing header error when building the DT
bindings at the previous commit. Should the patch defining the header be
ordered before the patch that references it?

> diff --git a/include/dt-bindings/reset/zte,zx279128s-reset.h b/include/dt-bindings/reset/zte,zx279128s-reset.h
> new file mode 100644
> index 0000000000000..5b580bcda865d
> --- /dev/null
> +++ b/include/dt-bindings/reset/zte,zx279128s-reset.h
> @@ -0,0 +1,26 @@
[ ... ]
> +/*
> + * Top CRM logical reset lines. Mapped by drivers/clk/zte/clk-zx279128s.c
> + * to hardware register offsets and bit shifts.
> + */
> +#define ZX279128S_RESET_USB_B9		0
> +#define ZX279128S_RESET_USB_B10		1
> +#define ZX279128S_RESET_USB_B11		2
> +#define ZX279128S_RESET_USB_B14		3
> +#define ZX279128S_RESET_COUNT		4

[Severity: Low]
Does exporting this upper bound macro to the stable ABI header create an
unstable ABI?

Device tree binding headers define the stable ABI between the device tree
and the kernel. Defining macros like ZX279128S_RESET_COUNT encourages
downstream or future code to use them for bounding. If new reset lines are
added in future hardware revisions, these bounds change, breaking backward
compatibility.

Since the driver correctly infers the reset count using ARRAY_SIZE() on its
internal mapping arrays later in the series, is it possible to drop this
macro and keep array sizes and upper bounds strictly inside the driver code?

> +
> +/*
> + * Top CRM 2 logical reset lines.
> + */
> +#define ZX279128S_CRM2_RESET_PCIE0	0
> +#define ZX279128S_CRM2_RESET_PCIE1	1
> +#define ZX279128S_CRM2_RESET_COUNT	2

[Severity: Low]
Similarly, could ZX279128S_CRM2_RESET_COUNT also be removed from the stable
binding header to preserve backward compatibility if new resets are
introduced later?

> +
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014072109.1759502-1-ghahramani.navid@gmail.com?part=5