Re: [PATCH v2] dt-bindings: clock: qcom,gcc-msm8660: add CE2_P_CLK, PLL4_VOTE + per-clock annotations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The commit adds new clock IDs `CE2_P_CLK` (258) and `PLL4_VOTE` (259) to the device tree bindings header, but fails to update or pad the corresponding clock provider array (`gcc_msm8660_clks`) in the driver. Because the array is sized automatically by its designated initializers (with the maximum explicitly initialized index being `PLL8_VOTE` = 253), `ARRAY_SIZE(gcc_msm8660_clks)` evaluates to 254. When consumers request the newly added clocks via the device tree, the request will be deterministically rejected with `-EINVAL` by `qcom_cc_clk_hw_get()` because the requested index (258 or 259) is out of bounds (>= 254), leading to probe failures for dependent drivers like `qce`.
--

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

dt-bindings: clock: qcom,gcc-msm8660: add CE2_P_CLK, PLL4_VOTE + per-clock annotations

This commit annotates each clock with provenance and consumer information
cross-referenced against various sources. It also appends two clocks,
CE2_P_CLK and PLL4_VOTE, which are needed by downstream consumers but were
missing from the header.

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

[ ... ]

> -#define PLL11					256
> -#define PLL12					257
> +#define PLL11                               256     /* header-only - PLL11 (Scorpion L2) */
> +#define PLL12                               257     /* header-only - PLL12 (DDR) */
> +
> +/* New clock IDs added in this cleanup. Append-only — never insert in the
> + * middle of the existing numbering, since that would shift IDs of
> + * earlier entries and break DT ABI for boards already using them. */
> +#define CE2_P_CLK                           258     /* drv+dt legacy:HTC,Sam,TP - CE2 APB (newly assigned, was missing) */
> +#define PLL4_VOTE                           259     /* drv legacy - PLL4 software vote */

[Severity: High]
Does adding these new clock IDs at 258 and 259 exceed the array bounds of
the driver's clock provider array?

In drivers/clk/qcom/gcc-msm8660.c, the gcc_msm8660_clks[] array is populated
using designated initializers, with the highest explicitly initialized index
being PLL8_VOTE (253).

Since the array size isn't padded to cover the new indices,
ARRAY_SIZE(gcc_msm8660_clks) evaluates to 254. 

When consumers request these newly added clocks via the device tree, will
qcom_cc_clk_hw_get() reject them with -EINVAL because the requested index
(258 or 259) is out of bounds (>= 254)?

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