[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aVP9j0c5lbfgRWlD@tom-desktop>
Date: Tue, 30 Dec 2025 17:27:59 +0100
From: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
To: Frank Li <Frank.li@....com>
Cc: tomm.merciai@...il.com, linux-renesas-soc@...r.kernel.org,
biju.das.jz@...renesas.com,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization
to separate function
Hi Frank!
Thanks for your review!
On Tue, Dec 30, 2025 at 10:47:57AM -0500, Frank Li wrote:
> On Tue, Dec 30, 2025 at 11:39:38AM +0100, Tommaso Merciai wrote:
> > Move the hardware initialization sequence in renesas_i3c_bus_init()
> > into a dedicated renesas_i3c_hw_init() helper.
> >
> > This simplifies the code, and prepares the driver for upcoming
> > suspend/resume support.
>
> Nit: Simplify the code and prepare the driver for ...
Will fix that in v3, Thanks!
Will fix also Patch 1/4, 2/4.
>
> Reviewed-by: Frank Li <Frank.Li@....com>
Kind Regards,
Tommaso
>
> >
> > No functional change intended.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
> > ---
> > v1->v2:
> > - New patch.
> >
> > drivers/i3c/master/renesas-i3c.c | 99 ++++++++++++++++++--------------
> > 1 file changed, 55 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > index 2736363c9074..4f076c372b36 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -485,13 +485,65 @@ static int renesas_i3c_reset(struct renesas_i3c *i3c)
> > 0, 1000, false, i3c->regs, RSTCTL);
> > }
> >
> > +static void renesas_i3c_hw_init(struct renesas_i3c *i3c)
> > +{
> > + u32 val;
> > +
> > + /* Disable Slave Mode */
> > + renesas_writel(i3c->regs, SVCTL, 0);
> > +
> > + /* Initialize Queue/Buffer threshold */
> > + renesas_writel(i3c->regs, NQTHCTL, NQTHCTL_IBIDSSZ(6) |
> > + NQTHCTL_CMDQTH(1));
> > +
> > + /* The only supported configuration is two entries*/
> > + renesas_writel(i3c->regs, NTBTHCTL0, 0);
> > + /* Interrupt when there is one entry in the queue */
> > + renesas_writel(i3c->regs, NRQTHCTL, 0);
> > +
> > + /* Enable all Bus/Transfer Status Flags */
> > + renesas_writel(i3c->regs, BSTE, BSTE_ALL_FLAG);
> > + renesas_writel(i3c->regs, NTSTE, NTSTE_ALL_FLAG);
> > +
> > + /* Interrupt enable settings */
> > + renesas_writel(i3c->regs, BIE, BIE_NACKDIE | BIE_TENDIE);
> > + renesas_writel(i3c->regs, NTIE, 0);
> > +
> > + /* Clear Status register */
> > + renesas_writel(i3c->regs, NTST, 0);
> > + renesas_writel(i3c->regs, INST, 0);
> > + renesas_writel(i3c->regs, BST, 0);
> > +
> > + /* Hot-Join Acknowlege setting. */
> > + renesas_set_bit(i3c->regs, BCTL, BCTL_HJACKCTL);
> > +
> > + renesas_writel(i3c->regs, IBINCTL, IBINCTL_NRHJCTL | IBINCTL_NRMRCTL |
> > + IBINCTL_NRSIRCTL);
> > +
> > + renesas_writel(i3c->regs, SCSTLCTL, 0);
> > + renesas_set_bit(i3c->regs, SCSTRCTL, SCSTRCTL_ACKTWE);
> > +
> > + /* Bus condition timing */
> > + val = DIV_ROUND_UP(I3C_BUS_TBUF_MIXED_FM_MIN_NS,
> > + NSEC_PER_SEC / i3c->rate);
> > + renesas_writel(i3c->regs, BFRECDT, BFRECDT_FRECYC(val));
> > +
> > + val = DIV_ROUND_UP(I3C_BUS_TAVAL_MIN_NS,
> > + NSEC_PER_SEC / i3c->rate);
> > + renesas_writel(i3c->regs, BAVLCDT, BAVLCDT_AVLCYC(val));
> > +
> > + val = DIV_ROUND_UP(I3C_BUS_TIDLE_MIN_NS,
> > + NSEC_PER_SEC / i3c->rate);
> > + renesas_writel(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
> > +}
> > +
> > static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> > {
> > struct renesas_i3c *i3c = to_renesas_i3c(m);
> > struct i3c_bus *bus = i3c_master_get_bus(m);
> > struct i3c_device_info info = {};
> > struct i2c_timings t;
> > - u32 double_SBR, val;
> > + u32 double_SBR;
> > int cks, pp_high_ticks, pp_low_ticks, i3c_total_ticks;
> > int od_high_ticks, od_low_ticks, i2c_total_ticks;
> > int ret;
> > @@ -564,49 +616,8 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> >
> > renesas_writel(i3c->regs, REFCKCTL, REFCKCTL_IREFCKS(cks));
> >
> > - /* Disable Slave Mode */
> > - renesas_writel(i3c->regs, SVCTL, 0);
> > -
> > - /* Initialize Queue/Buffer threshold */
> > - renesas_writel(i3c->regs, NQTHCTL, NQTHCTL_IBIDSSZ(6) |
> > - NQTHCTL_CMDQTH(1));
> > -
> > - /* The only supported configuration is two entries*/
> > - renesas_writel(i3c->regs, NTBTHCTL0, 0);
> > - /* Interrupt when there is one entry in the queue */
> > - renesas_writel(i3c->regs, NRQTHCTL, 0);
> > -
> > - /* Enable all Bus/Transfer Status Flags */
> > - renesas_writel(i3c->regs, BSTE, BSTE_ALL_FLAG);
> > - renesas_writel(i3c->regs, NTSTE, NTSTE_ALL_FLAG);
> > -
> > - /* Interrupt enable settings */
> > - renesas_writel(i3c->regs, BIE, BIE_NACKDIE | BIE_TENDIE);
> > - renesas_writel(i3c->regs, NTIE, 0);
> > -
> > - /* Clear Status register */
> > - renesas_writel(i3c->regs, NTST, 0);
> > - renesas_writel(i3c->regs, INST, 0);
> > - renesas_writel(i3c->regs, BST, 0);
> > -
> > - /* Hot-Join Acknowlege setting. */
> > - renesas_set_bit(i3c->regs, BCTL, BCTL_HJACKCTL);
> > -
> > - renesas_writel(i3c->regs, IBINCTL, IBINCTL_NRHJCTL | IBINCTL_NRMRCTL |
> > - IBINCTL_NRSIRCTL);
> > -
> > - renesas_writel(i3c->regs, SCSTLCTL, 0);
> > - renesas_set_bit(i3c->regs, SCSTRCTL, SCSTRCTL_ACKTWE);
> > -
> > - /* Bus condition timing */
> > - val = DIV_ROUND_UP(I3C_BUS_TBUF_MIXED_FM_MIN_NS, NSEC_PER_SEC / i3c->rate);
> > - renesas_writel(i3c->regs, BFRECDT, BFRECDT_FRECYC(val));
> > -
> > - val = DIV_ROUND_UP(I3C_BUS_TAVAL_MIN_NS, NSEC_PER_SEC / i3c->rate);
> > - renesas_writel(i3c->regs, BAVLCDT, BAVLCDT_AVLCYC(val));
> > -
> > - val = DIV_ROUND_UP(I3C_BUS_TIDLE_MIN_NS, NSEC_PER_SEC / i3c->rate);
> > - renesas_writel(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
> > + /* I3C hw init*/
> > + renesas_i3c_hw_init(i3c);
> >
> > ret = i3c_master_get_free_addr(m, 0);
> > if (ret < 0)
> > --
> > 2.43.0
> >
Powered by blists - more mailing lists