Re: [PATCH 03/17] i3c: renesas: Restore STDBR and EXTBR registers on resume

[Frank Li]

On Thu, May 28, 2026 at 11:29:46AM +0300, Claudiu Beznea wrote:
> Hi, Frank,
>
> On 5/22/26 22:10, Frank Li wrote:
> > On Fri, May 22, 2026 at 01:18:01PM +0300, Claudiu Beznea wrote:
> > > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> > >
> > > The Renesas RZ/G3S supports a power saving state where power to the most
> > > SoC componentes (including I3C) is lost.
> > >
> > > The STDBR and EXTBR are configured in initialization phase though the
> > > struct i3c_master_controller_ops::bus_init. Set them on resume function
> > > as well to keep the same state of the controller after a suspend with
> > > power loss and a similar initialization sequence as in bus_init.
> > >
> > > Fixes: e7218986319b ("i3c: renesas: Add suspend/resume support")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> > > ---
> > >   drivers/i3c/master/renesas-i3c.c | 10 ++++++----
> > >   1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > > index 1917549cf6d5..6c23f956ad2a 100644
> > > --- a/drivers/i3c/master/renesas-i3c.c
> > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > @@ -260,6 +260,7 @@ struct renesas_i3c {
> > >   	u32 dyn_addr;
> > >   	u32 i2c_STDBR;
> > >   	u32 i3c_STDBR;
> > > +	u32 extbr;
> >
> > can you keep consisent with above 2 register, use upcase EXTBR,
>
> Using upper case for this will mislead the compiler. There is already a
> macro defined for EXTBR:

That's fine.

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>
> #define EXTBR                   0x78
>
> Defining this member as:
>
> u32 EXTBR;
>
> will make the compiler try to replace the EXTBR in "i3c->EXTBR" at
> preprocessing time:
>
>   CC [M]  drivers/i3c/master/renesas-i3c.o
> ../drivers/i3c/master/renesas-i3c.c:65:17: error: expected identifier or ‘(’
> before numeric constant
>    65 | #define EXTBR   0x78
>       |                 ^~~~
> ../drivers/i3c/master/renesas-i3c.c:263:6: note: in expansion of macro ‘EXTBR’
>   263 |  u32 EXTBR;
>       |      ^~~~~
> ../drivers/i3c/master/renesas-i3c.c: In function ‘renesas_i3c_bus_init’:
> ../drivers/i3c/master/renesas-i3c.c:65:17: error: expected identifier before
> numeric constant
>    65 | #define EXTBR   0x78
>       |                 ^~~~
> ../drivers/i3c/master/renesas-i3c.c:611:7: note: in expansion of macro ‘EXTBR’
>   611 |  i3c->EXTBR = EXTBR_EBRLO(od_low_ticks) | EXTBR_EBRHO(od_high_ticks) |
>       |       ^~~~~
> ../drivers/i3c/master/renesas-i3c.c:65:17: error: expected identifier before
> numeric constant
>    65 | #define EXTBR   0x78
>       |                 ^~~~
> ../drivers/i3c/master/renesas-i3c.c:613:40: note: in expansion of macro ‘EXTBR’
>   613 |  renesas_writel(i3c->regs, EXTBR, i3c->EXTBR);
>       |                                        ^~~~~
> ../drivers/i3c/master/renesas-i3c.c: In function ‘renesas_i3c_resume_noirq’:
> ../drivers/i3c/master/renesas-i3c.c:65:17: error: expected identifier before
> numeric constant
>    65 | #define EXTBR   0x78
>       |                 ^~~~
> ../drivers/i3c/master/renesas-i3c.c:1451:40: note: in expansion of macro ‘EXTBR’
>  1451 |  renesas_writel(i3c->regs, EXTBR, i3c->EXTBR);
>       |                                        ^~~~~
>
> The register contains both i3c and i2c specific fields. I'm not sure using
> i2c_i3c_EXTBR is the best way to go forward for this or just keeping it as
> is.
>
> --
> Thank you,
> Claudiu
>