Re: [PATCH v8 7/9] clk: eyeq: Convert clocks declaration to eqc_clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing Spinlock for Shared Register
--

commit 8a6ca96f73accf716971f013f35f97d51b0a39dd
Author: Benoît Monin <benoit.monin@bootlin.com>

clk: eyeq: Convert clocks declaration to eqc_clock

Convert all declarations of PLLs, dividers, and fixed factors to struct
eqc_clock for the EyeQ5, EyeQ6H, EyeQ6L, and EyeQ6Lplus, both for the
match data used during probe and the early match data used in early
initialization.

> diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> index 7b8de08388d3b..8a38ad9afb5ef 100644
> --- a/drivers/clk/clk-eyeq.c
> +++ b/drivers/clk/clk-eyeq.c
[ ... ]
> @@ -776,33 +715,10 @@ static const struct eqc_match_data eqc_eyeq6h_east_match_data = {
[ ... ]
> +static const struct eqc_clock eqc_eyeq6h_south_clks[] = {
> +	PLL_FRACG(EQ6HC_SOUTH_PLL_VDI, PARENT_BY_FWNAME, "pll-vdi", "ref", 0x000),
> +	PLL_FRACG(EQ6HC_SOUTH_PLL_PCIE, PARENT_BY_FWNAME, "pll-pcie", "ref", 0x008),
> +	PLL_FRACG(EQ6HC_SOUTH_PLL_PER, PARENT_BY_FWNAME, "pll-per", "ref", 0x010),
> +	PLL_FRACG(EQ6HC_SOUTH_PLL_ISP, PARENT_BY_FWNAME, "pll-isp", "ref", 0x018),
>  
> -static const struct eqc_div eqc_eyeq6h_south_divs[] = {
> -	{
[ ... ]
> +	DIV(EQ6HC_SOUTH_DIV_EMMC, EQ6HC_SOUTH_PLL_PER, "div-emmc", NULL, 0x070, 4, 4),
> +	DIV(EQ6HC_SOUTH_DIV_OSPI_REF, EQ6HC_SOUTH_PLL_PER, "div-ospi-ref", NULL, 0x090, 4, 4),
> +	DIV(EQ6HC_SOUTH_DIV_OSPI_SYS, EQ6HC_SOUTH_PLL_PER, "div-ospi-sys", NULL, 0x090, 8, 1),

[Severity: High]
This isn't a bug introduced by this patch, but since both "div-ospi-ref" and
"div-ospi-sys" share the same hardware register at 0x090, does this need a
spinlock?

Looking at eqc_probe_divider() in drivers/clk/clk-eyeq.c, it passes a NULL
spinlock argument to clk_hw_register_divider_table_parent_data():

drivers/clk/clk-eyeq.c:eqc_probe_divider() {
	...
	hw = clk_hw_register_divider_table_parent_data(dev, clk->name,
			&parent_data, 0, base + clk->div.reg, clk->div.shift,
			clk->div.width, clk->div.flags, clk->div.table, NULL);
	...
}

Will concurrent rate changes via clk_set_rate() execute unprotected
read-modify-write cycles on this shared register and cause register corruption?

> +	DIV(EQ6HC_SOUTH_DIV_TSU, EQ6HC_SOUTH_PLL_PCIE, "div-tsu", NULL, 0x098, 4, 8),
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-clk-eyeq7-v8-0-53f618174cf6@bootlin.com?part=7