Re: [PATCH v8 6/9] clk: eyeq: Introduce a generic clock type

[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 in divider clock registration allows read-modify-write races on shared registers.
--

commit 64cecba2d1878b01e3dd2fecb67b0bb43fd51ec6
Author: Benoît Monin <benoit.monin@bootlin.com>

clk: eyeq: Introduce a generic clock type

This commit introduces a generic eqc_clock structure to represent PLLs,
dividers, and fixed factors for the Mobileye EyeQ OLB. It includes specific
probe and unregister functions for each clock type to support complex clock
interdependencies.

> diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> index e4e690a12a238..7b8de08388d3b 100644
> --- a/drivers/clk/clk-eyeq.c
> +++ b/drivers/clk/clk-eyeq.c

[ ... ]

> @@ -336,6 +376,102 @@ static void eqc_auxdev_create_optional(struct device *dev, void __iomem *base,

[ ... ]

> +static int eqc_probe_divider(struct device *dev, struct device_node *np,
> +			     const struct eqc_clock *clk, void __iomem *base,
> +			     struct clk_hw_onecell_data *cells)
> +{
> +	struct clk_parent_data parent_data;
> +	struct clk_hw *hw;
> +	int ret;
> +
> +	ret = eqc_fill_parent_data(clk, cells, &parent_data);
> +	if (ret)
> +		return ret;
> +
> +	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);

[Severity: High]
This isn't a bug introduced by this patch, but since multiple writable
clock dividers share the same physical register (for example, div-ospi-ref
and div-ospi-sys share the register at 0x090), can concurrent clk_set_rate()
calls cause read-modify-write races here?

Because NULL is passed as the lock parameter, the clock divider core will
perform a lockless read-modify-write sequence. Would it be safer to provide
a shared spinlock to protect the register state?

> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	cells->hws[clk->index] = hw;
> +	return 0;
> +}

[ ... ]

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