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

[email protected]
Newsgroups org.infradead.lists.linux-i3c
Message-ID <[email protected]>
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 <[email protected]>

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/[email protected]?part=2

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.