[PATCH v5 06/17] i3c: renesas: Perform Dynamic Address Assignment on resume
Frank Li
Frank.li at oss.nxp.com
Mon Jul 13 09:59:40 PDT 2026
On Mon, Jul 13, 2026 at 01:50:32PM +0000, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removal of the DATBASn save/restore mechanism completely breaks the I3C bus upon resume for previously attached devices, and fails to resume if the bus was fully occupied.
> --
>
> --- Patch [6]: [PATCH v5 06/17] i3c: renesas: Perform Dynamic Address Assignment on resume ---
> commit e4ac838c055dc33beb2ff4e8968a032da48a2a47
> Author: Claudiu Beznea <claudiu.beznea.uj at bp.renesas.com>
>
> i3c: renesas: Perform Dynamic Address Assignment on resume
>
> This commit updates the suspend/resume operations by moving them out of
> the noirq phase and attempting to perform Dynamic Address Assignment on
> resume. It also removes the save and restore of the DATBASn registers.
>
> > 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
>
> [ ... ]
>
> > @@ -1495,15 +1484,19 @@ static int renesas_i3c_resume_noirq(struct device *dev)
> > renesas_writel(i3c->regs, MSDVAD, MSDVAD_MDYADV |
> > MSDVAD_MDYAD(i3c->dyn_addr));
> >
> > - /* Restore Device Address Table values. */
> > - for (i = 0; i < i3c->maxdevs; i++)
> > - renesas_writel(i3c->regs, DATBAS(i), i3c->DATBASn[i]);
> > -
> > /* I3C hw init. */
> > renesas_i3c_hw_init(i3c);
> >
> > + ret = i3c_master_do_daa_ext(&i3c->base, true);
>
> [Severity: High]
> Does removing the DATBASn save and restore mechanism break communication
> with previously attached devices upon resume?
>
> Looking at renesas_i3c_daa(), it explicitly skips hardware register
> initialization for previously attached devices:
>
> renesas_i3c_daa() {
> ...
> if (olddevs & BIT(pos))
> continue;
> ...
> }
>
> Since the controller was reset during suspend, won't these hardware
> registers remain uninitialized (0) if DATBASn isn't manually restored here?
>
> Additionally, if the bus was fully occupied before suspend, will this cause
> the DAA process to abort entirely?
>
> renesas_i3c_daa() {
> ...
> ret = renesas_i3c_get_free_pos(i3c);
> if (ret < 0)
> return ret;
> ...
> }
>
> If the bus is full, free_pos would be 0, causing this to return -ENOSPC
> immediately. This aborts before the ENTDAA command is sent, which would
> leave the previously attached devices permanently stranded without dynamic
> addresses.
Please check this sashiko review result?
Frank
>
> > + if (ret)
> > + dev_err(dev, "DAA failed on resume, ret=%d", ret);
> > +
> > i2c_mark_adapter_resumed(&i3c->base.i2c);
> >
> > + /*
> > + * I3C devices may have retained their dynamic address anyway. Do not
> > + * fail the resume because of DAA error.
> > + */
> > return 0;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260713130545.568657-1-claudiu.beznea+renesas@tuxon.dev?part=6
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list