Re: [PATCH] dt-bindings: clock: qcom,gcc-msm8660: collision fixes + per-clock annotations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Critical ABI Breakage: The patch destroys Device Tree ABI backwards compatibility by shifting numeric IDs of over 150 existing clock bindings. DT binding headers define the stable hardware-software ABI; changing these IDs invalidates existing compiled Device Tree Blobs (DTBs). The commit claims to fix ID collisions, but the baseline values were already distinct (e.g., `USB_HS1_H_CLK` and `USB_HS1_XCVR_SRC` were 110 and 111). By inserting new clocks (like `CE2_P_CLK` at ID 78) and renumbering perceived "collisions", the patch shifts all subsequent clocks. For example, `SDC1_H_CLK` shifted from 95 to 96, and `GSBI1_H_CLK` from 125 to 126.
--

commit 41cfc5d6ae5f24f8a2e508765d49af3a6921e1a7
Author: Herman van Hazendonk <github.com@herrie.org>

dt-bindings: clock: qcom,gcc-msm8660: collision fixes + per-clock annotations

This commit adds trailing documentation comments to per-clock definitions and
attempts to fix perceived ID collisions. However, it alters the numeric
values of numerous existing clock definitions.

> diff --git a/include/dt-bindings/clock/qcom,gcc-msm8660.h b/include/dt-bindings/clock/qcom,gcc-msm8660.h
> index 4777c002711a1..354e9d8657b92 100644
> --- a/include/dt-bindings/clock/qcom,gcc-msm8660.h
> +++ b/include/dt-bindings/clock/qcom,gcc-msm8660.h

[ ... ]

> -#define CE1_H_CLK				76
> -#define CE2_H_CLK				77
> -#define SFPB_H_CLK_SRC				78
> -#define SFPB_H_CLK				79
> -#define SFAB_SFPB_M_H_CLK			80

[ ... ]

> +#define CE1_H_CLK                           76    /* header-only - CE1 (crypto engine 1) AHB */
> +#define CE2_H_CLK                           77    /* drv+dt - CE2 AHB */
> +#define CE2_P_CLK                           78    /* drv+dt legacy:HTC,Sam,TP - CE2 APB */
> +#define SFPB_H_CLK_SRC                      79    /* header-only - SFPB AHB src */
> +/* header-only - SFPB (system peripheral bridge) AHB */
> +#define SFPB_H_CLK                          80

[Severity: Critical]
Does this cause a regression in DT ABI backwards compatibility by shifting
the numeric IDs of existing clock bindings?

It looks like inserting CE2_P_CLK at ID 78 shifts the values of subsequent
clocks. Device Tree bindings represent a stable ABI, and changing these IDs
means older compiled Device Tree Blobs (DTBs) running on this kernel will
request incorrect clocks.

[ ... ]

> -#define PMIC_SSBI2_CLK				94
> -#define SDC1_H_CLK				95
> -#define SDC2_H_CLK				96

[ ... ]

> +#define PMIC_SSBI2_CLK                      95    /* drv legacy:HTC,Sam,TP,Pre3 - PMIC SSBI2 */
> +#define SDC1_H_CLK                          96    /* drv+dt - eMMC1 AHB */
> +#define SDC2_H_CLK                          97    /* drv+dt - eMMC2 AHB */

[Severity: Critical]
Will this shift result in boot failures for older DTBs?

Because SDC1_H_CLK shifted from 95 to 96, a DTB requesting ID 95 will now
receive PMIC_SSBI2_CLK instead of SDC1_H_CLK.

When drivers attempt to access hardware registers without the correct clocks
enabled, it can trigger synchronous external aborts and system hangs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530135731.185313-1-github.com@herrie.org?part=1