Re: [PATCH v3 02/17] 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] The EXTBR configuration truncates `od_low_ticks` and `od_high_ticks` if they exceed 255, resulting in incorrect open-drain timing settings being cached and applied.
--

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

i3c: renesas: Restore STDBR and EXTBR registers on resume

This patch stores the Extended Bit Rate (EXTBR) settings during bus
initialization and explicitly restores both STDBR and EXTBR on resume
to preserve controller state across a power-loss suspend.

Fixes: e7218986319b ("i3c: renesas: Add suspend/resume support")
Cc: stable@vger.kernel.org

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 6e7ece2e0b4ee..88a16efe096de 100644
> --- 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 calculation silently truncate
the open-drain ticks?

Looking earlier in renesas_i3c_bus_init(), the while-loop termination
condition allows od_low_ticks and od_high_ticks to exceed 255:

        if ((od_low_ticks / 2) <= 0xFF && pp_low_ticks < 0x3F)
            break;

The Standard Bit Rate (STDBR) configuration safely handles these larger
values by right-shifting them when double_SBR is set. However, the EXTBR
macros use FIELD_PREP(GENMASK(7, 0), x), which silently truncates any
values over 255.

Since this patch now explicitly caches the truncated EXTBR calculation to
persist it across system resumes, would it be better to address this
truncation so we don't restore broken timing configurations?

> +	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/20260608201543.804902-1-claudiu.beznea@kernel.org?part=2

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