1. Patchew
  2. linux
  3. [PATCH v2 0/4] i3c: renesas: Add suspend/resume support
  4. View patch

[PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function

Tommaso Merciai posted 4 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
Posted by Tommaso Merciai 1 month, 1 week ago 
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.

No functional change intended.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.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
Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
Posted by Frank Li 1 month, 1 week ago 
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 ...

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> No functional change intended.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.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
>
Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
Posted by Tommaso Merciai 1 month, 1 week ago 
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@nxp.com>

Kind Regards,
Tommaso

> 
> >
> > No functional change intended.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.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
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.