Re: [PATCH treewide 2/5] clk: visconti: Make sure clk_init_data is fully initialized

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

Pre-existing issues:
- [High] visconti_reset_status() incorrectly uses the bit index directly instead of a bitmask, resulting in corrupted status checks.
--

--- Patch [2]: [PATCH treewide 2/5] clk: visconti: Make sure clk_init_data is fully initialized ---
commit 61772ea3c610a3b3ebf435b199e400c1c5aaee4c
Author: Geert Uytterhoeven <[email protected]>

clk: visconti: Make sure clk_init_data is fully initialized

The clk_init_data structure contains several mutually-exclusive members for
different methods to specify the possible parents of a clock. Not
initializing all members may cause subtle issues when uninitialized members
are unexpectedly accessed. This commit ensures all members are fully
initialized to zero.

> diff --git a/drivers/clk/visconti/clkc.c b/drivers/clk/visconti/clkc.c
> index d0b193b5d0b35..4018d1298880d 100644
> --- a/drivers/clk/visconti/clkc.c
> +++ b/drivers/clk/visconti/clkc.c
> @@ -81,9 +81,9 @@ static struct clk_hw *visconti_clk_register_gate(struct device *dev,
>  						 u8	rs_idx,
>  						 spinlock_t *lock)
>  {
> +	struct clk_init_data init = {};
>  	struct visconti_clk_gate *gate;
>  	struct clk_parent_data *pdata;
> -	struct clk_init_data init;
>  	struct clk_hw *hw;
>  	int ret;
>  

[Severity: High]
This is a pre-existing issue, but while reviewing the visconti clock and
reset driver changes, I noticed a potential problem with how rs_idx is used.

In drivers/clk/visconti/reset.c:visconti_reset_status():

	spin_lock_irqsave(reset->lock, flags);
	ret = regmap_read(reset->regmap, data->rson_offset, &reg);
	spin_unlock_irqrestore(reset->lock, flags);
	if (ret)
		return ret;

	return !(reg & data->rs_idx);

Is this reset status check operating as intended? Since rs_idx is a raw bit
index (0-31), would it be more accurate to mask the register with
BIT(data->rs_idx) rather than the raw index value?

Masking with the raw index seems like it could produce incorrect status
returns. For example, if rs_idx is 0, the mask would always evaluate to 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic