Re: [PATCH v8 06/10] clk: realtek: Add support for mux clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk_regmap_mux_get_parent() returns 0 instead of an error indicator when regmap_read() fails, silently masking the I/O error and confusing the Common Clock Framework.
--

commit 6f252a1d2352da3ab0338149dff431cfd1082f04
Author: Cheng-Yu Lee <cylee12@realtek.com>

clk: realtek: Add support for mux clock

Add a simple regmap-based clk_ops implementation for Realtek mux clocks.
The implementation supports parent selection and rate determination through
regmap-backed register access.

> diff --git a/drivers/clk/realtek/clk-regmap-mux.c b/drivers/clk/realtek/clk-regmap-mux.c
> new file mode 100644
> index 0000000000000..8bb66a9e244a1
> --- /dev/null
> +++ b/drivers/clk/realtek/clk-regmap-mux.c

[ ... ]

> +static u8 clk_regmap_mux_get_parent(struct clk_hw *hw)
> +{
> +	struct clk_regmap_mux *clkm = to_clk_regmap_mux(hw);
> +	int num_parents = clk_hw_get_num_parents(hw);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(clkm->clkr.regmap, clkm->mux_ofs, &val);
> +	if (ret)
> +		return 0;

[Severity: High]
When regmap_read() fails here, it returns 0. Since 0 is a valid parent
index in the Common Clock Framework, will this mask the I/O error and falsely
report that parent 0 is active?

If the hardware was actually using a different parent, this could lead the
CCF to incorrectly assume the actual parent has no active children and
potentially gate it during clk_disable_unused(). Would it be safer to
return an out-of-bounds value like 0xff (which is used later in this
function) or an error code when the read fails, to prevent the CCF from
making incorrect assumptions about the hardware state?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610080824.255063-1-eleanor.lin@realtek.com?part=6