Re: [PATCH v4 02/16] i3c: renesas: Restore STDBR and EXTBR registers on resume

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Silent data truncation in EXTBR register configuration for slow clock speeds.
--

commit decdc939209046d74f5ae66361c115e1c225a379
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

i3c: renesas: Restore STDBR and EXTBR registers on resume

This commit restores the STDBR and EXTBR registers during the resume process 
for the Renesas RZ/G3S. This ensures the controller maintains the same state 
after a suspend with power loss.

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c

[ ... ]

> @@ -607,10 +608,9 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
>  	renesas_writel(i3c->regs, STDBR, i3c->i3c_STDBR);
>  
>  	/* Extended Bit Rate setting */
> -	renesas_writel(i3c->regs, EXTBR, EXTBR_EBRLO(od_low_ticks) |
> -					   EXTBR_EBRHO(od_high_ticks) |
> -					   EXTBR_EBRLP(pp_low_ticks) |
> -					   EXTBR_EBRHP(pp_high_ticks));
> +	i3c->extbr = EXTBR_EBRLO(od_low_ticks) | EXTBR_EBRHO(od_high_ticks) |
> +		     EXTBR_EBRLP(pp_low_ticks) | EXTBR_EBRHP(pp_high_ticks);

[Severity: High]
This is a pre-existing issue, but does this code silently truncate 
od_low_ticks and od_high_ticks when the clock is slow?

For slow I2C speeds, od_low_ticks and od_high_ticks can reach up to 510 
because the clock divider loop only ensures (od_low_ticks / 2) <= 0xFF.

The STDBR register configuration applies a right shift when double_SBR is 
set to avoid overflow.

Since the EXTBR_EBRLO and EXTBR_EBRHO macros use 8-bit masks, and this line 
doesn't apply the double_SBR right shift, won't any tick value greater than 
255 get truncated by the bitwise AND and result in incorrect extended bit 
rate timings?

> +	renesas_writel(i3c->regs, EXTBR, i3c->extbr);
>  
>  	renesas_writel(i3c->regs, REFCKCTL, REFCKCTL_IREFCKS(cks));
>  	i3c->refclk_div = cks;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612160458.3102106-1-claudiu.beznea@kernel.org?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c