Re: [PATCH] clk: qcom: negcc-nord: Add frequencies for ne_gcc_sdcc4_apps_clk_src

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: qcom: bitwise truncation of clock divider due to hid_width limit
--

commit 2477f758daf039725b34bfc8b46b211691bae410
Author: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>

clk: qcom: negcc-nord: Add frequencies for ne_gcc_sdcc4_apps_clk_src
    
Add the frequency table entries for ne_gcc_sdcc4_apps_clk_src to support
the additional clock rates required on the Nord platform.

> diff --git a/drivers/clk/qcom/negcc-nord.c b/drivers/clk/qcom/negcc-nord.c
> index 68ce4d6278baa..a72fce70de45c 100644
> --- a/drivers/clk/qcom/negcc-nord.c
> +++ b/drivers/clk/qcom/negcc-nord.c
> @@ -436,6 +436,11 @@ static struct clk_rcg2 ne_gcc_qupv3_wrap2_s6_clk_src = {
>  };
>  
>  static const struct freq_tbl ftbl_ne_gcc_sdcc4_apps_clk_src[] = {
> +	F(144000, P_BI_TCXO, 16, 3, 25),
> +	F(400000, P_BI_TCXO, 12, 1, 4),
> +	F(19200000, P_BI_TCXO, 1, 0, 0),
> +	F(20000000, P_NE_GCC_GPLL0_OUT_MAIN, 30, 0, 0),
> +	F(25000000, P_NE_GCC_GPLL0_OUT_MAIN, 24, 0, 0),

[Severity: High]
Does this configuration overflow the hardware divider field?

In the F() macro, the half-integer divider (h) evaluates to a hardware
pre_div value of 2 * h - 1. For the new 20 MHz and 25 MHz frequencies
in ftbl_ne_gcc_sdcc4_apps_clk_src, using h values of 30 and 24 results
in hardware pre_div values of 59 and 47.

If the ne_gcc_sdcc4_apps_clk_src clock has a 5-bit hid_width, its maximum
supported pre_div value would be 31.

When __clk_rcg2_configure() applies this configuration, would these values
exceed the clear mask (0x1f), resulting in hardware truncation and
potential corruption of adjacent register bits?

Even if adjacent bits are ignored by the hardware, a truncated pre_div
value of 27 (from 59 & 31) means the effective divider becomes 14 instead
of 30, causing the clock to run at 42.85 MHz instead of 20 MHz.

Could this violate timing limits for the SD Card/eMMC controller and
lead to bus instability?

Could the M/N dividers be used for these rates instead, since .mnd_width
is available?

>  	F(37500000, P_NE_GCC_GPLL0_OUT_MAIN, 16, 0, 0),
>  	F(50000000, P_NE_GCC_GPLL0_OUT_MAIN, 12, 0, 0),
>  	F(100000000, P_NE_GCC_GPLL0_OUT_MAIN, 6, 0, 0),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-nord-sdcc-clk-v1-1-63351f25a165@oss.qualcomm.com?part=1