[PATCH v4 0/4] i3c: renesas: Add suspend/resume support

[PATCH v4 0/4] i3c: renesas: Add suspend/resume support

[Tommaso Merciai]

Dear All,
This series adds suspend/resume support to the Renesas I3C controller driver.

Thanks & Regards,
Tommaso

v3->v4:
 - Rebased on top of next-20260105
 - PATCH 1/4:
     - Collected tag.
 - PATCH 4/4:
     - Use clk_bulk_disable/enable() into renesas_i3c_suspend/resume_noirq()
       instead of clk_bulk_prepare_enable()/clk_bulk_disable_unprepare()

v2->v3:
 - PATCH 1/4:
     - Added define for TCLK index.
     - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
     - Improved commit body.
     - Dropped unnecessary static const char * const renesas_i3c_clks[].
     - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
 - PATCH 2/4:
     - Collected FLi tag.
     - Improved commit body.
 - PATCH 3/4:
     - Collected FLi tag.
     - Improved commit body.
 - PATCH 4/4:
     Fixed error path into renesas_i3c_resume_noirq() and
     renesas_i3c_suspend_noirq() function.

v1->v2:
 - Split patches into smaller chunks.
 - Use clock bulk API.
 - Update patch 4/4 accordingly.

Tommaso Merciai (4):
  i3c: renesas: Switch to clk_bulk API and store clocks in private data
  i3c: renesas: Store clock rate and reset controls in struct
    renesas_i3c
  i3c: renesas: Factor out hardware initialization to separate function
  i3c: renesas: Add suspend/resume support

 drivers/i3c/master/renesas-i3c.c | 256 ++++++++++++++++++++-----------
 1 file changed, 168 insertions(+), 88 deletions(-)

-- 
2.43.0


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

[PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Tommaso Merciai]

Replace individual devm_clk_get_enabled() calls with the clk_bulk API
and store the clock handles in the driver's private data structure.

All clocks required by the controller are now acquired and enabled using
devm_clk_bulk_get_all_enabled(), removing the need for per-SoC clock
handling and the renesas_i3c_config data.
The TCLK is accessed via a fixed index in the bulk clock array.

Simplify the code and prepare the driver for upcoming suspend/resume
support.

No functional change intended.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v3->v4:
 - Collected Biju Das tag.

v2->v3:
 - Added define for TCLK index.
 - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
 - Improved commit body.
 - Dropped unnecessary static const char * const renesas_i3c_clks[].
 - Removed the need for per-SoC clock handling and the renesas_i3c_config data.

v1->v2:
 - New patch.

 drivers/i3c/master/renesas-i3c.c | 43 ++++++++------------------------
 1 file changed, 11 insertions(+), 32 deletions(-)

diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 426a418f29b6..1b8f4be9ad67 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -198,6 +198,8 @@
 #define RENESAS_I3C_MAX_DEVS	8
 #define I2C_INIT_MSG		-1
 
+#define RENESAS_I3C_TCLK_IDX	1
+
 enum i3c_internal_state {
 	I3C_INTERNAL_STATE_DISABLED,
 	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
@@ -259,7 +261,8 @@ struct renesas_i3c {
 	u8 addrs[RENESAS_I3C_MAX_DEVS];
 	struct renesas_i3c_xferqueue xferqueue;
 	void __iomem *regs;
-	struct clk *tclk;
+	struct clk_bulk_data *clks;
+	u8 num_clks;
 };
 
 struct renesas_i3c_i2c_dev_data {
@@ -272,10 +275,6 @@ struct renesas_i3c_irq_desc {
 	const char *desc;
 };
 
-struct renesas_i3c_config {
-	unsigned int has_pclkrw:1;
-};
-
 static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
 {
 	u32 data = readl(reg);
@@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 	int od_high_ticks, od_low_ticks, i2c_total_ticks;
 	int ret;
 
-	rate = clk_get_rate(i3c->tclk);
+	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
 	if (!rate)
 		return -EINVAL;
 
@@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct platform_device *pdev)
 {
 	struct renesas_i3c *i3c;
 	struct reset_control *reset;
-	struct clk *clk;
-	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
 	int ret, i;
 
-	if (!config)
-		return -ENODATA;
-
 	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
 	if (!i3c)
 		return -ENOMEM;
@@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
 	if (IS_ERR(i3c->regs))
 		return PTR_ERR(i3c->regs);
 
-	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
-	if (config->has_pclkrw) {
-		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
-		if (IS_ERR(clk))
-			return PTR_ERR(clk);
-	}
+	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
+	if (ret < 0)
+		return ret;
 
-	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
-	if (IS_ERR(i3c->tclk))
-		return PTR_ERR(i3c->tclk);
+	i3c->num_clks = ret;
 
 	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
 	if (IS_ERR(reset))
@@ -1374,16 +1360,9 @@ static void renesas_i3c_remove(struct platform_device *pdev)
 	i3c_master_unregister(&i3c->base);
 }
 
-static const struct renesas_i3c_config empty_i3c_config = {
-};
-
-static const struct renesas_i3c_config r9a09g047_i3c_config = {
-	.has_pclkrw = 1,
-};
-
 static const struct of_device_id renesas_i3c_of_ids[] = {
-	{ .compatible = "renesas,r9a08g045-i3c", .data = &empty_i3c_config },
-	{ .compatible = "renesas,r9a09g047-i3c", .data = &r9a09g047_i3c_config },
+	{ .compatible = "renesas,r9a08g045-i3c" },
+	{ .compatible = "renesas,r9a09g047-i3c" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, renesas_i3c_of_ids);
-- 
2.43.0


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

[PATCH v4 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c

[Tommaso Merciai]

Update the struct renesas_i3c to store the clock rate, presetn and
tresetn handlers. Replace local usage of the clock rate and reset
controls with these structure fields.

Simplify the code and prepare the driver for upcoming suspend/resume
support.

No functional change intended.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v3->v4:
 - No changes.

v2->v3:
 - Collected FLi tag.
 - Improved commit body.

v1->v2:
 - New patch.

 drivers/i3c/master/renesas-i3c.c | 39 ++++++++++++++++----------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 1b8f4be9ad67..7359f71f78dd 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -258,11 +258,14 @@ struct renesas_i3c {
 	u32 free_pos;
 	u32 i2c_STDBR;
 	u32 i3c_STDBR;
+	unsigned long rate;
 	u8 addrs[RENESAS_I3C_MAX_DEVS];
 	struct renesas_i3c_xferqueue xferqueue;
 	void __iomem *regs;
 	struct clk_bulk_data *clks;
 	u8 num_clks;
+	struct reset_control *presetn;
+	struct reset_control *tresetn;
 };
 
 struct renesas_i3c_i2c_dev_data {
@@ -482,22 +485,21 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 	struct i3c_bus *bus = i3c_master_get_bus(m);
 	struct i3c_device_info info = {};
 	struct i2c_timings t;
-	unsigned long rate;
 	u32 double_SBR, val;
 	int cks, pp_high_ticks, pp_low_ticks, i3c_total_ticks;
 	int od_high_ticks, od_low_ticks, i2c_total_ticks;
 	int ret;
 
-	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
-	if (!rate)
+	i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
+	if (!i3c->rate)
 		return -EINVAL;
 
 	ret = renesas_i3c_reset(i3c);
 	if (ret)
 		return ret;
 
-	i2c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i2c);
-	i3c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i3c);
+	i2c_total_ticks = DIV_ROUND_UP(i3c->rate, bus->scl_rate.i2c);
+	i3c_total_ticks = DIV_ROUND_UP(i3c->rate, bus->scl_rate.i3c);
 
 	i2c_parse_fw_timings(&m->dev, &t, true);
 
@@ -510,7 +512,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 			pp_high_ticks = ((i3c_total_ticks * 5) / 10);
 		else
 			pp_high_ticks = DIV_ROUND_UP(I3C_BUS_THIGH_MIXED_MAX_NS,
-						     NSEC_PER_SEC / rate);
+						     NSEC_PER_SEC / i3c->rate);
 		pp_low_ticks = i3c_total_ticks - pp_high_ticks;
 
 		if ((od_low_ticks / 2) <= 0xFF && pp_low_ticks < 0x3F)
@@ -518,7 +520,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 
 		i2c_total_ticks /= 2;
 		i3c_total_ticks /= 2;
-		rate /= 2;
+		i3c->rate /= 2;
 	}
 
 	/* SCL clock period calculation in Open-drain mode */
@@ -539,8 +541,8 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 			STDBR_SBRLP(pp_low_ticks) |
 			STDBR_SBRHP(pp_high_ticks);
 
-	od_low_ticks -= t.scl_fall_ns / (NSEC_PER_SEC / rate) + 1;
-	od_high_ticks -= t.scl_rise_ns / (NSEC_PER_SEC / rate) + 1;
+	od_low_ticks -= t.scl_fall_ns / (NSEC_PER_SEC / i3c->rate) + 1;
+	od_high_ticks -= t.scl_rise_ns / (NSEC_PER_SEC / i3c->rate) + 1;
 	i3c->i2c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
 			STDBR_SBRLO(double_SBR, od_low_ticks) |
 			STDBR_SBRHO(double_SBR, od_high_ticks) |
@@ -591,13 +593,13 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 	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 / rate);
+	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 / rate);
+	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 / rate);
+	val = DIV_ROUND_UP(I3C_BUS_TIDLE_MIN_NS, NSEC_PER_SEC / i3c->rate);
 	renesas_writel(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
 
 	ret = i3c_master_get_free_addr(m, 0);
@@ -1300,7 +1302,6 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
 static int renesas_i3c_probe(struct platform_device *pdev)
 {
 	struct renesas_i3c *i3c;
-	struct reset_control *reset;
 	int ret, i;
 
 	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
@@ -1317,14 +1318,14 @@ static int renesas_i3c_probe(struct platform_device *pdev)
 
 	i3c->num_clks = ret;
 
-	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
-	if (IS_ERR(reset))
-		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
+	i3c->tresetn = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
+	if (IS_ERR(i3c->tresetn))
+		return dev_err_probe(&pdev->dev, PTR_ERR(i3c->tresetn),
 				     "Error: missing tresetn ctrl\n");
 
-	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
-	if (IS_ERR(reset))
-		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
+	i3c->presetn = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
+	if (IS_ERR(i3c->presetn))
+		return dev_err_probe(&pdev->dev, PTR_ERR(i3c->presetn),
 				     "Error: missing presetn ctrl\n");
 
 	spin_lock_init(&i3c->xferqueue.lock);
-- 
2.43.0


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

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

[Tommaso Merciai]

Move the hardware initialization sequence in renesas_i3c_bus_init()
into a dedicated renesas_i3c_hw_init() helper.

Simplify the code and prepare the driver for upcoming suspend/resume
support.

No functional change intended.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v3->v4:
 - No changes.

v2->v3:
 - Collected FLi tag.
 - Improved commit body.

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 7359f71f78dd..b065b8d4b138 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -479,13 +479,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;
@@ -558,49 +610,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


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

[PATCH v4 4/4] i3c: renesas: Add suspend/resume support

[Tommaso Merciai]

The Renesas I3C controller does not retain its register state across system
suspend, requiring the driver to explicitly save and restore hardware
configuration.

Add suspend and resume NOIRQ callbacks to handle system sleep transitions.

During suspend, save the Device Address Table (DAT) entries, assert reset
lines, and disable all related clocks to allow the controller to enter a
low-power state.

On resume, re-enable clocks and reset lines in the proper order. Restore
the REFCKCTL register, master dynamic address, and all DAT entries, then
reinitialize the controller.

Store the REFCLK divider value, and the master dynamic address to restore
timing and addressing configuration after resume.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v3->v4:
 - Use clk_bulk_disable/enable() into renesas_i3c_suspend/resume_noirq()
   instead of clk_bulk_prepare_enable()/clk_bulk_disable_unprepare()

v2->v3:
 - Fixed error path into renesas_i3c_resume_noirq() and
   renesas_i3c_suspend_noirq() function.
 - Moved up one line sizeof(u32) * i3c->maxdevs into devm_kzalloc() call.

v1->v2:
 - Updated commit as v1 has been split into smaller patches.
 - Use clock bulk API into renesas_i3c_suspend_noirq() and
   renesas_i3c_resume_noirq().

 drivers/i3c/master/renesas-i3c.c | 89 ++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index b065b8d4b138..4123593fc16b 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -256,16 +256,19 @@ struct renesas_i3c {
 	enum i3c_internal_state internal_state;
 	u16 maxdevs;
 	u32 free_pos;
+	u32 dyn_addr;
 	u32 i2c_STDBR;
 	u32 i3c_STDBR;
 	unsigned long rate;
 	u8 addrs[RENESAS_I3C_MAX_DEVS];
 	struct renesas_i3c_xferqueue xferqueue;
 	void __iomem *regs;
+	u32 *DATBASn;
 	struct clk_bulk_data *clks;
 	u8 num_clks;
 	struct reset_control *presetn;
 	struct reset_control *tresetn;
+	u8 refclk_div;
 };
 
 struct renesas_i3c_i2c_dev_data {
@@ -609,6 +612,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 					   EXTBR_EBRHP(pp_high_ticks));
 
 	renesas_writel(i3c->regs, REFCKCTL, REFCKCTL_IREFCKS(cks));
+	i3c->refclk_div = cks;
 
 	/* I3C hw init*/
 	renesas_i3c_hw_init(i3c);
@@ -617,6 +621,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
 	if (ret < 0)
 		return ret;
 
+	i3c->dyn_addr = ret;
 	renesas_writel(i3c->regs, MSDVAD, MSDVAD_MDYAD(ret) | MSDVAD_MDYADV);
 
 	memset(&info, 0, sizeof(info));
@@ -1362,6 +1367,12 @@ static int renesas_i3c_probe(struct platform_device *pdev)
 	i3c->maxdevs = RENESAS_I3C_MAX_DEVS;
 	i3c->free_pos = GENMASK(i3c->maxdevs - 1, 0);
 
+	/* Allocate dynamic Device Address Table backup. */
+	i3c->DATBASn = devm_kzalloc(&pdev->dev, sizeof(u32) * i3c->maxdevs,
+				    GFP_KERNEL);
+	if (!i3c->DATBASn)
+		return -ENOMEM;
+
 	return i3c_master_register(&i3c->base, &pdev->dev, &renesas_i3c_ops, false);
 }
 
@@ -1372,6 +1383,83 @@ static void renesas_i3c_remove(struct platform_device *pdev)
 	i3c_master_unregister(&i3c->base);
 }
 
+static int renesas_i3c_suspend_noirq(struct device *dev)
+{
+	struct renesas_i3c *i3c = dev_get_drvdata(dev);
+	int i, ret;
+
+	i2c_mark_adapter_suspended(&i3c->base.i2c);
+
+	/* Store Device Address Table values. */
+	for (i = 0; i < i3c->maxdevs; i++)
+		i3c->DATBASn[i] = renesas_readl(i3c->regs, DATBAS(i));
+
+	ret = reset_control_assert(i3c->presetn);
+	if (ret)
+		goto err_mark_resumed;
+
+	ret = reset_control_assert(i3c->tresetn);
+	if (ret)
+		goto err_presetn;
+
+	clk_bulk_disable(i3c->num_clks, i3c->clks);
+
+	return 0;
+
+err_presetn:
+	reset_control_deassert(i3c->presetn);
+err_mark_resumed:
+	i2c_mark_adapter_resumed(&i3c->base.i2c);
+
+	return ret;
+}
+
+static int renesas_i3c_resume_noirq(struct device *dev)
+{
+	struct renesas_i3c *i3c = dev_get_drvdata(dev);
+	int i, ret;
+
+	ret = reset_control_deassert(i3c->presetn);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(i3c->tresetn);
+	if (ret)
+		goto err_presetn;
+
+	ret = clk_bulk_enable(i3c->num_clks, i3c->clks);
+	if (ret)
+		goto err_tresetn;
+
+	/* Re-store I3C registers value. */
+	renesas_writel(i3c->regs, REFCKCTL,
+		       REFCKCTL_IREFCKS(i3c->refclk_div));
+	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);
+
+	i2c_mark_adapter_resumed(&i3c->base.i2c);
+
+	return 0;
+
+err_tresetn:
+	reset_control_assert(i3c->tresetn);
+err_presetn:
+	reset_control_assert(i3c->presetn);
+	return ret;
+}
+
+static const struct dev_pm_ops renesas_i3c_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(renesas_i3c_suspend_noirq,
+				  renesas_i3c_resume_noirq)
+};
+
 static const struct of_device_id renesas_i3c_of_ids[] = {
 	{ .compatible = "renesas,r9a08g045-i3c" },
 	{ .compatible = "renesas,r9a09g047-i3c" },
@@ -1385,6 +1473,7 @@ static struct platform_driver renesas_i3c = {
 	.driver = {
 		.name = "renesas-i3c",
 		.of_match_table = renesas_i3c_of_ids,
+		.pm = pm_sleep_ptr(&renesas_i3c_pm_ops),
 	},
 };
 module_platform_driver(renesas_i3c);
-- 
2.43.0


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

RE: [PATCH v4 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c

[Biju Das]

Hi Tommaso Merciai,

> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Sent: 05 January 2026 10:50
> Subject: [PATCH v4 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c
> 
> Update the struct renesas_i3c to store the clock rate, presetn and tresetn handlers. Replace local
> usage of the clock rate and reset controls with these structure fields.
> 
> Simplify the code and prepare the driver for upcoming suspend/resume support.
> 
> No functional change intended.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v3->v4:
>  - No changes.
> 
> v2->v3:
>  - Collected FLi tag.
>  - Improved commit body.
> 
> v1->v2:
>  - New patch.
> 
>  drivers/i3c/master/renesas-i3c.c | 39 ++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 1b8f4be9ad67..7359f71f78dd 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -258,11 +258,14 @@ struct renesas_i3c {
>  	u32 free_pos;
>  	u32 i2c_STDBR;
>  	u32 i3c_STDBR;
> +	unsigned long rate;
>  	u8 addrs[RENESAS_I3C_MAX_DEVS];
>  	struct renesas_i3c_xferqueue xferqueue;
>  	void __iomem *regs;
>  	struct clk_bulk_data *clks;
>  	u8 num_clks;
> +	struct reset_control *presetn;
> +	struct reset_control *tresetn;

Can this be above num_clks to avoid padding?

Cheers,
Biju

>  };
> 
>  struct renesas_i3c_i2c_dev_data {
> @@ -482,22 +485,21 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
>  	struct i3c_bus *bus = i3c_master_get_bus(m);
>  	struct i3c_device_info info = {};
>  	struct i2c_timings t;
> -	unsigned long rate;
>  	u32 double_SBR, val;
>  	int cks, pp_high_ticks, pp_low_ticks, i3c_total_ticks;
>  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
>  	int ret;
> 
> -	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> -	if (!rate)
> +	i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> +	if (!i3c->rate)
>  		return -EINVAL;
> 
>  	ret = renesas_i3c_reset(i3c);
>  	if (ret)
>  		return ret;
> 
> -	i2c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i2c);
> -	i3c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i3c);
> +	i2c_total_ticks = DIV_ROUND_UP(i3c->rate, bus->scl_rate.i2c);
> +	i3c_total_ticks = DIV_ROUND_UP(i3c->rate, bus->scl_rate.i3c);
> 
>  	i2c_parse_fw_timings(&m->dev, &t, true);
> 
> @@ -510,7 +512,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
>  			pp_high_ticks = ((i3c_total_ticks * 5) / 10);
>  		else
>  			pp_high_ticks = DIV_ROUND_UP(I3C_BUS_THIGH_MIXED_MAX_NS,
> -						     NSEC_PER_SEC / rate);
> +						     NSEC_PER_SEC / i3c->rate);
>  		pp_low_ticks = i3c_total_ticks - pp_high_ticks;
> 
>  		if ((od_low_ticks / 2) <= 0xFF && pp_low_ticks < 0x3F) @@ -518,7 +520,7 @@ static int
> renesas_i3c_bus_init(struct i3c_master_controller *m)
> 
>  		i2c_total_ticks /= 2;
>  		i3c_total_ticks /= 2;
> -		rate /= 2;
> +		i3c->rate /= 2;
>  	}
> 
>  	/* SCL clock period calculation in Open-drain mode */ @@ -539,8 +541,8 @@ static int
> renesas_i3c_bus_init(struct i3c_master_controller *m)
>  			STDBR_SBRLP(pp_low_ticks) |
>  			STDBR_SBRHP(pp_high_ticks);
> 
> -	od_low_ticks -= t.scl_fall_ns / (NSEC_PER_SEC / rate) + 1;
> -	od_high_ticks -= t.scl_rise_ns / (NSEC_PER_SEC / rate) + 1;
> +	od_low_ticks -= t.scl_fall_ns / (NSEC_PER_SEC / i3c->rate) + 1;
> +	od_high_ticks -= t.scl_rise_ns / (NSEC_PER_SEC / i3c->rate) + 1;
>  	i3c->i2c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
>  			STDBR_SBRLO(double_SBR, od_low_ticks) |
>  			STDBR_SBRHO(double_SBR, od_high_ticks) | @@ -591,13 +593,13 @@ static int
> renesas_i3c_bus_init(struct i3c_master_controller *m)
>  	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 / rate);
> +	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 / rate);
> +	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 / rate);
> +	val = DIV_ROUND_UP(I3C_BUS_TIDLE_MIN_NS, NSEC_PER_SEC / i3c->rate);
>  	renesas_writel(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
> 
>  	ret = i3c_master_get_free_addr(m, 0);
> @@ -1300,7 +1302,6 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {  static int
> renesas_i3c_probe(struct platform_device *pdev)  {
>  	struct renesas_i3c *i3c;
> -	struct reset_control *reset;
>  	int ret, i;
> 
>  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL); @@ -1317,14 +1318,14 @@ static int
> renesas_i3c_probe(struct platform_device *pdev)
> 
>  	i3c->num_clks = ret;
> 
> -	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> -	if (IS_ERR(reset))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
> +	i3c->tresetn = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> +	if (IS_ERR(i3c->tresetn))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(i3c->tresetn),
>  				     "Error: missing tresetn ctrl\n");
> 
> -	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
> -	if (IS_ERR(reset))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
> +	i3c->presetn = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
> +	if (IS_ERR(i3c->presetn))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(i3c->presetn),
>  				     "Error: missing presetn ctrl\n");
> 
>  	spin_lock_init(&i3c->xferqueue.lock);
> --
> 2.43.0


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

Re: [PATCH v4 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c

[Tommaso Merciai]

Hi Biju,
Thanks for your review.

On Mon, Jan 05, 2026 at 10:53:29AM +0000, Biju Das wrote:
> Hi Tommaso Merciai,
> 
> > -----Original Message-----
> > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > Sent: 05 January 2026 10:50
> > Subject: [PATCH v4 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c
> > 
> > Update the struct renesas_i3c to store the clock rate, presetn and tresetn handlers. Replace local
> > usage of the clock rate and reset controls with these structure fields.
> > 
> > Simplify the code and prepare the driver for upcoming suspend/resume support.
> > 
> > No functional change intended.
> > 
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v3->v4:
> >  - No changes.
> > 
> > v2->v3:
> >  - Collected FLi tag.
> >  - Improved commit body.
> > 
> > v1->v2:
> >  - New patch.
> > 
> >  drivers/i3c/master/renesas-i3c.c | 39 ++++++++++++++++----------------
> >  1 file changed, 20 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > index 1b8f4be9ad67..7359f71f78dd 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -258,11 +258,14 @@ struct renesas_i3c {
> >  	u32 free_pos;
> >  	u32 i2c_STDBR;
> >  	u32 i3c_STDBR;
> > +	unsigned long rate;
> >  	u8 addrs[RENESAS_I3C_MAX_DEVS];
> >  	struct renesas_i3c_xferqueue xferqueue;
> >  	void __iomem *regs;
> >  	struct clk_bulk_data *clks;
> >  	u8 num_clks;
> > +	struct reset_control *presetn;
> > +	struct reset_control *tresetn;
> 
> Can this be above num_clks to avoid padding?

Ack. Thanks.

Kind Regards,
Tommaso

> 
> Cheers,
> Biju
> 
> >  };
> > 
> >  struct renesas_i3c_i2c_dev_data {
> > @@ -482,22 +485,21 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> >  	struct i3c_bus *bus = i3c_master_get_bus(m);
> >  	struct i3c_device_info info = {};
> >  	struct i2c_timings t;
> > -	unsigned long rate;
> >  	u32 double_SBR, val;
> >  	int cks, pp_high_ticks, pp_low_ticks, i3c_total_ticks;
> >  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> >  	int ret;
> > 
> > -	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> > -	if (!rate)
> > +	i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> > +	if (!i3c->rate)
> >  		return -EINVAL;
> > 
> >  	ret = renesas_i3c_reset(i3c);
> >  	if (ret)
> >  		return ret;
> > 
> > -	i2c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i2c);
> > -	i3c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i3c);
> > +	i2c_total_ticks = DIV_ROUND_UP(i3c->rate, bus->scl_rate.i2c);
> > +	i3c_total_ticks = DIV_ROUND_UP(i3c->rate, bus->scl_rate.i3c);
> > 
> >  	i2c_parse_fw_timings(&m->dev, &t, true);
> > 
> > @@ -510,7 +512,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> >  			pp_high_ticks = ((i3c_total_ticks * 5) / 10);
> >  		else
> >  			pp_high_ticks = DIV_ROUND_UP(I3C_BUS_THIGH_MIXED_MAX_NS,
> > -						     NSEC_PER_SEC / rate);
> > +						     NSEC_PER_SEC / i3c->rate);
> >  		pp_low_ticks = i3c_total_ticks - pp_high_ticks;
> > 
> >  		if ((od_low_ticks / 2) <= 0xFF && pp_low_ticks < 0x3F) @@ -518,7 +520,7 @@ static int
> > renesas_i3c_bus_init(struct i3c_master_controller *m)
> > 
> >  		i2c_total_ticks /= 2;
> >  		i3c_total_ticks /= 2;
> > -		rate /= 2;
> > +		i3c->rate /= 2;
> >  	}
> > 
> >  	/* SCL clock period calculation in Open-drain mode */ @@ -539,8 +541,8 @@ static int
> > renesas_i3c_bus_init(struct i3c_master_controller *m)
> >  			STDBR_SBRLP(pp_low_ticks) |
> >  			STDBR_SBRHP(pp_high_ticks);
> > 
> > -	od_low_ticks -= t.scl_fall_ns / (NSEC_PER_SEC / rate) + 1;
> > -	od_high_ticks -= t.scl_rise_ns / (NSEC_PER_SEC / rate) + 1;
> > +	od_low_ticks -= t.scl_fall_ns / (NSEC_PER_SEC / i3c->rate) + 1;
> > +	od_high_ticks -= t.scl_rise_ns / (NSEC_PER_SEC / i3c->rate) + 1;
> >  	i3c->i2c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
> >  			STDBR_SBRLO(double_SBR, od_low_ticks) |
> >  			STDBR_SBRHO(double_SBR, od_high_ticks) | @@ -591,13 +593,13 @@ static int
> > renesas_i3c_bus_init(struct i3c_master_controller *m)
> >  	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 / rate);
> > +	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 / rate);
> > +	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 / rate);
> > +	val = DIV_ROUND_UP(I3C_BUS_TIDLE_MIN_NS, NSEC_PER_SEC / i3c->rate);
> >  	renesas_writel(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
> > 
> >  	ret = i3c_master_get_free_addr(m, 0);
> > @@ -1300,7 +1302,6 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {  static int
> > renesas_i3c_probe(struct platform_device *pdev)  {
> >  	struct renesas_i3c *i3c;
> > -	struct reset_control *reset;
> >  	int ret, i;
> > 
> >  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL); @@ -1317,14 +1318,14 @@ static int
> > renesas_i3c_probe(struct platform_device *pdev)
> > 
> >  	i3c->num_clks = ret;
> > 
> > -	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > -	if (IS_ERR(reset))
> > -		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
> > +	i3c->tresetn = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > +	if (IS_ERR(i3c->tresetn))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(i3c->tresetn),
> >  				     "Error: missing tresetn ctrl\n");
> > 
> > -	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
> > -	if (IS_ERR(reset))
> > -		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
> > +	i3c->presetn = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
> > +	if (IS_ERR(i3c->presetn))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(i3c->presetn),
> >  				     "Error: missing presetn ctrl\n");
> > 
> >  	spin_lock_init(&i3c->xferqueue.lock);
> > --
> > 2.43.0
> 

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

Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Frank Li]

On Mon, Jan 05, 2026 at 11:49:59AM +0100, Tommaso Merciai wrote:
> Replace individual devm_clk_get_enabled() calls with the clk_bulk API
> and store the clock handles in the driver's private data structure.
>
> All clocks required by the controller are now acquired and enabled using
> devm_clk_bulk_get_all_enabled(), removing the need for per-SoC clock
> handling and the renesas_i3c_config data.
> The TCLK is accessed via a fixed index in the bulk clock array.
>
> Simplify the code and prepare the driver for upcoming suspend/resume
> support.
>
> No functional change intended.
>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v3->v4:
>  - Collected Biju Das tag.
>
> v2->v3:
>  - Added define for TCLK index.
>  - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
>  - Improved commit body.
>  - Dropped unnecessary static const char * const renesas_i3c_clks[].
>  - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
>
> v1->v2:
>  - New patch.
>
>  drivers/i3c/master/renesas-i3c.c | 43 ++++++++------------------------
>  1 file changed, 11 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 426a418f29b6..1b8f4be9ad67 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -198,6 +198,8 @@
>  #define RENESAS_I3C_MAX_DEVS	8
>  #define I2C_INIT_MSG		-1
>
> +#define RENESAS_I3C_TCLK_IDX	1
> +
>  enum i3c_internal_state {
>  	I3C_INTERNAL_STATE_DISABLED,
>  	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> @@ -259,7 +261,8 @@ struct renesas_i3c {
>  	u8 addrs[RENESAS_I3C_MAX_DEVS];
>  	struct renesas_i3c_xferqueue xferqueue;
>  	void __iomem *regs;
> -	struct clk *tclk;
> +	struct clk_bulk_data *clks;
> +	u8 num_clks;
>  };
>
>  struct renesas_i3c_i2c_dev_data {
> @@ -272,10 +275,6 @@ struct renesas_i3c_irq_desc {
>  	const char *desc;
>  };
>
> -struct renesas_i3c_config {
> -	unsigned int has_pclkrw:1;
> -};
> -
>  static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
>  {
>  	u32 data = readl(reg);
> @@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
>  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
>  	int ret;
>
> -	rate = clk_get_rate(i3c->tclk);
> +	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
>  	if (!rate)
>  		return -EINVAL;
>
> @@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct platform_device *pdev)
>  {
>  	struct renesas_i3c *i3c;
>  	struct reset_control *reset;
> -	struct clk *clk;
> -	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
>  	int ret, i;
>
> -	if (!config)
> -		return -ENODATA;
> -
>  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
>  	if (!i3c)
>  		return -ENOMEM;
> @@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
>  	if (IS_ERR(i3c->regs))
>  		return PTR_ERR(i3c->regs);
>
> -	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> -	if (IS_ERR(clk))
> -		return PTR_ERR(clk);
> -
> -	if (config->has_pclkrw) {
> -		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> -		if (IS_ERR(clk))
> -			return PTR_ERR(clk);
> -	}
> +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> +	if (ret < 0)
> +		return ret;
>
> -	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> -	if (IS_ERR(i3c->tclk))
> -		return PTR_ERR(i3c->tclk);
> +	i3c->num_clks = ret;

Need check num_clks > RENESAS_I3C_TCLK_IDX to avoid outbound access at
i3c->clks[RENESAS_I3C_TCLK_IDX].clk

Frank

>
>  	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
>  	if (IS_ERR(reset))
> @@ -1374,16 +1360,9 @@ static void renesas_i3c_remove(struct platform_device *pdev)
>  	i3c_master_unregister(&i3c->base);
>  }
>
> -static const struct renesas_i3c_config empty_i3c_config = {
> -};
> -
> -static const struct renesas_i3c_config r9a09g047_i3c_config = {
> -	.has_pclkrw = 1,
> -};
> -
>  static const struct of_device_id renesas_i3c_of_ids[] = {
> -	{ .compatible = "renesas,r9a08g045-i3c", .data = &empty_i3c_config },
> -	{ .compatible = "renesas,r9a09g047-i3c", .data = &r9a09g047_i3c_config },
> +	{ .compatible = "renesas,r9a08g045-i3c" },
> +	{ .compatible = "renesas,r9a09g047-i3c" },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, renesas_i3c_of_ids);
> --
> 2.43.0
>

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

Re: [PATCH v4 4/4] i3c: renesas: Add suspend/resume support

[Frank Li]

On Mon, Jan 05, 2026 at 11:50:02AM +0100, Tommaso Merciai wrote:
> The Renesas I3C controller does not retain its register state across system
> suspend, requiring the driver to explicitly save and restore hardware
> configuration.
>
> Add suspend and resume NOIRQ callbacks to handle system sleep transitions.
>
> During suspend, save the Device Address Table (DAT) entries, assert reset
> lines, and disable all related clocks to allow the controller to enter a
> low-power state.
>
> On resume, re-enable clocks and reset lines in the proper order. Restore
> the REFCKCTL register, master dynamic address, and all DAT entries, then
> reinitialize the controller.
>
> Store the REFCLK divider value, and the master dynamic address to restore
> timing and addressing configuration after resume.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---

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

> v3->v4:
>  - Use clk_bulk_disable/enable() into renesas_i3c_suspend/resume_noirq()
>    instead of clk_bulk_prepare_enable()/clk_bulk_disable_unprepare()
>
> v2->v3:
>  - Fixed error path into renesas_i3c_resume_noirq() and
>    renesas_i3c_suspend_noirq() function.
>  - Moved up one line sizeof(u32) * i3c->maxdevs into devm_kzalloc() call.
>
> v1->v2:
>  - Updated commit as v1 has been split into smaller patches.
>  - Use clock bulk API into renesas_i3c_suspend_noirq() and
>    renesas_i3c_resume_noirq().
>
>  drivers/i3c/master/renesas-i3c.c | 89 ++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index b065b8d4b138..4123593fc16b 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -256,16 +256,19 @@ struct renesas_i3c {
>  	enum i3c_internal_state internal_state;
>  	u16 maxdevs;
>  	u32 free_pos;
> +	u32 dyn_addr;
>  	u32 i2c_STDBR;
>  	u32 i3c_STDBR;
>  	unsigned long rate;
>  	u8 addrs[RENESAS_I3C_MAX_DEVS];
>  	struct renesas_i3c_xferqueue xferqueue;
>  	void __iomem *regs;
> +	u32 *DATBASn;
>  	struct clk_bulk_data *clks;
>  	u8 num_clks;
>  	struct reset_control *presetn;
>  	struct reset_control *tresetn;
> +	u8 refclk_div;
>  };
>
>  struct renesas_i3c_i2c_dev_data {
> @@ -609,6 +612,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
>  					   EXTBR_EBRHP(pp_high_ticks));
>
>  	renesas_writel(i3c->regs, REFCKCTL, REFCKCTL_IREFCKS(cks));
> +	i3c->refclk_div = cks;
>
>  	/* I3C hw init*/
>  	renesas_i3c_hw_init(i3c);
> @@ -617,6 +621,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
>  	if (ret < 0)
>  		return ret;
>
> +	i3c->dyn_addr = ret;
>  	renesas_writel(i3c->regs, MSDVAD, MSDVAD_MDYAD(ret) | MSDVAD_MDYADV);
>
>  	memset(&info, 0, sizeof(info));
> @@ -1362,6 +1367,12 @@ static int renesas_i3c_probe(struct platform_device *pdev)
>  	i3c->maxdevs = RENESAS_I3C_MAX_DEVS;
>  	i3c->free_pos = GENMASK(i3c->maxdevs - 1, 0);
>
> +	/* Allocate dynamic Device Address Table backup. */
> +	i3c->DATBASn = devm_kzalloc(&pdev->dev, sizeof(u32) * i3c->maxdevs,
> +				    GFP_KERNEL);
> +	if (!i3c->DATBASn)
> +		return -ENOMEM;
> +
>  	return i3c_master_register(&i3c->base, &pdev->dev, &renesas_i3c_ops, false);
>  }
>
> @@ -1372,6 +1383,83 @@ static void renesas_i3c_remove(struct platform_device *pdev)
>  	i3c_master_unregister(&i3c->base);
>  }
>
> +static int renesas_i3c_suspend_noirq(struct device *dev)
> +{
> +	struct renesas_i3c *i3c = dev_get_drvdata(dev);
> +	int i, ret;
> +
> +	i2c_mark_adapter_suspended(&i3c->base.i2c);
> +
> +	/* Store Device Address Table values. */
> +	for (i = 0; i < i3c->maxdevs; i++)
> +		i3c->DATBASn[i] = renesas_readl(i3c->regs, DATBAS(i));
> +
> +	ret = reset_control_assert(i3c->presetn);
> +	if (ret)
> +		goto err_mark_resumed;
> +
> +	ret = reset_control_assert(i3c->tresetn);
> +	if (ret)
> +		goto err_presetn;
> +
> +	clk_bulk_disable(i3c->num_clks, i3c->clks);
> +
> +	return 0;
> +
> +err_presetn:
> +	reset_control_deassert(i3c->presetn);
> +err_mark_resumed:
> +	i2c_mark_adapter_resumed(&i3c->base.i2c);
> +
> +	return ret;
> +}
> +
> +static int renesas_i3c_resume_noirq(struct device *dev)
> +{
> +	struct renesas_i3c *i3c = dev_get_drvdata(dev);
> +	int i, ret;
> +
> +	ret = reset_control_deassert(i3c->presetn);
> +	if (ret)
> +		return ret;
> +
> +	ret = reset_control_deassert(i3c->tresetn);
> +	if (ret)
> +		goto err_presetn;
> +
> +	ret = clk_bulk_enable(i3c->num_clks, i3c->clks);
> +	if (ret)
> +		goto err_tresetn;
> +
> +	/* Re-store I3C registers value. */
> +	renesas_writel(i3c->regs, REFCKCTL,
> +		       REFCKCTL_IREFCKS(i3c->refclk_div));
> +	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);
> +
> +	i2c_mark_adapter_resumed(&i3c->base.i2c);
> +
> +	return 0;
> +
> +err_tresetn:
> +	reset_control_assert(i3c->tresetn);
> +err_presetn:
> +	reset_control_assert(i3c->presetn);
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops renesas_i3c_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(renesas_i3c_suspend_noirq,
> +				  renesas_i3c_resume_noirq)
> +};
> +
>  static const struct of_device_id renesas_i3c_of_ids[] = {
>  	{ .compatible = "renesas,r9a08g045-i3c" },
>  	{ .compatible = "renesas,r9a09g047-i3c" },
> @@ -1385,6 +1473,7 @@ static struct platform_driver renesas_i3c = {
>  	.driver = {
>  		.name = "renesas-i3c",
>  		.of_match_table = renesas_i3c_of_ids,
> +		.pm = pm_sleep_ptr(&renesas_i3c_pm_ops),
>  	},
>  };
>  module_platform_driver(renesas_i3c);
> --
> 2.43.0
>

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

RE: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Biju Das]

Hi Frank Li,

> -----Original Message-----
> From: Frank Li <Frank.li@nxp.com>
> Sent: 05 January 2026 17:12
> Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
> 
> On Mon, Jan 05, 2026 at 11:49:59AM +0100, Tommaso Merciai wrote:
> > Replace individual devm_clk_get_enabled() calls with the clk_bulk API
> > and store the clock handles in the driver's private data structure.
> >
> > All clocks required by the controller are now acquired and enabled
> > using devm_clk_bulk_get_all_enabled(), removing the need for per-SoC
> > clock handling and the renesas_i3c_config data.
> > The TCLK is accessed via a fixed index in the bulk clock array.
> >
> > Simplify the code and prepare the driver for upcoming suspend/resume
> > support.
> >
> > No functional change intended.
> >
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v3->v4:
> >  - Collected Biju Das tag.
> >
> > v2->v3:
> >  - Added define for TCLK index.
> >  - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
> >  - Improved commit body.
> >  - Dropped unnecessary static const char * const renesas_i3c_clks[].
> >  - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
> >
> > v1->v2:
> >  - New patch.
> >
> >  drivers/i3c/master/renesas-i3c.c | 43
> > ++++++++------------------------
> >  1 file changed, 11 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/i3c/master/renesas-i3c.c
> > b/drivers/i3c/master/renesas-i3c.c
> > index 426a418f29b6..1b8f4be9ad67 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -198,6 +198,8 @@
> >  #define RENESAS_I3C_MAX_DEVS	8
> >  #define I2C_INIT_MSG		-1
> >
> > +#define RENESAS_I3C_TCLK_IDX	1
> > +
> >  enum i3c_internal_state {
> >  	I3C_INTERNAL_STATE_DISABLED,
> >  	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> > @@ -259,7 +261,8 @@ struct renesas_i3c {
> >  	u8 addrs[RENESAS_I3C_MAX_DEVS];
> >  	struct renesas_i3c_xferqueue xferqueue;
> >  	void __iomem *regs;
> > -	struct clk *tclk;
> > +	struct clk_bulk_data *clks;
> > +	u8 num_clks;
> >  };
> >
> >  struct renesas_i3c_i2c_dev_data {
> > @@ -272,10 +275,6 @@ struct renesas_i3c_irq_desc {
> >  	const char *desc;
> >  };
> >
> > -struct renesas_i3c_config {
> > -	unsigned int has_pclkrw:1;
> > -};
> > -
> >  static inline void renesas_i3c_reg_update(void __iomem *reg, u32
> > mask, u32 val)  {
> >  	u32 data = readl(reg);
> > @@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> >  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> >  	int ret;
> >
> > -	rate = clk_get_rate(i3c->tclk);
> > +	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> >  	if (!rate)
> >  		return -EINVAL;
> >
> > @@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct
> > platform_device *pdev)  {
> >  	struct renesas_i3c *i3c;
> >  	struct reset_control *reset;
> > -	struct clk *clk;
> > -	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> >  	int ret, i;
> >
> > -	if (!config)
> > -		return -ENODATA;
> > -
> >  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> >  	if (!i3c)
> >  		return -ENOMEM;
> > @@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> >  	if (IS_ERR(i3c->regs))
> >  		return PTR_ERR(i3c->regs);
> >
> > -	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> > -	if (IS_ERR(clk))
> > -		return PTR_ERR(clk);
> > -
> > -	if (config->has_pclkrw) {
> > -		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > -		if (IS_ERR(clk))
> > -			return PTR_ERR(clk);
> > -	}
> > +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> > +	if (ret < 0)
> > +		return ret;
> >
> > -	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > -	if (IS_ERR(i3c->tclk))
> > -		return PTR_ERR(i3c->tclk);
> > +	i3c->num_clks = ret;
> 
> Need check num_clks > RENESAS_I3C_TCLK_IDX to avoid outbound access at
> i3c->clks[RENESAS_I3C_TCLK_IDX].clk

I guess dt binding check validate this as well. Eg: a single clk defined
in the DT instead of minimum 2, will give DT warnings.

Do you expect additional check in C code as well?

Cheers,
Biju


> >
> >  	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> >  	if (IS_ERR(reset))
> > @@ -1374,16 +1360,9 @@ static void renesas_i3c_remove(struct platform_device *pdev)
> >  	i3c_master_unregister(&i3c->base);
> >  }
> >
> > -static const struct renesas_i3c_config empty_i3c_config = { -};
> > -
> > -static const struct renesas_i3c_config r9a09g047_i3c_config = {
> > -	.has_pclkrw = 1,
> > -};
> > -
> >  static const struct of_device_id renesas_i3c_of_ids[] = {
> > -	{ .compatible = "renesas,r9a08g045-i3c", .data = &empty_i3c_config },
> > -	{ .compatible = "renesas,r9a09g047-i3c", .data = &r9a09g047_i3c_config },
> > +	{ .compatible = "renesas,r9a08g045-i3c" },
> > +	{ .compatible = "renesas,r9a09g047-i3c" },
> >  	{ /* sentinel */ },
> >  };
> >  MODULE_DEVICE_TABLE(of, renesas_i3c_of_ids);
> > --
> > 2.43.0
> >

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

Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Frank Li]

On Mon, Jan 05, 2026 at 06:06:19PM +0000, Biju Das wrote:
> Hi Frank Li,
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@nxp.com>
> > Sent: 05 January 2026 17:12
> > Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
> >
> > On Mon, Jan 05, 2026 at 11:49:59AM +0100, Tommaso Merciai wrote:
> > > Replace individual devm_clk_get_enabled() calls with the clk_bulk API
> > > and store the clock handles in the driver's private data structure.
> > >
> > > All clocks required by the controller are now acquired and enabled
> > > using devm_clk_bulk_get_all_enabled(), removing the need for per-SoC
> > > clock handling and the renesas_i3c_config data.
> > > The TCLK is accessed via a fixed index in the bulk clock array.
> > >
> > > Simplify the code and prepare the driver for upcoming suspend/resume
> > > support.
> > >
> > > No functional change intended.
> > >
> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > v3->v4:
> > >  - Collected Biju Das tag.
> > >
> > > v2->v3:
> > >  - Added define for TCLK index.
> > >  - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
> > >  - Improved commit body.
> > >  - Dropped unnecessary static const char * const renesas_i3c_clks[].
> > >  - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
> > >
> > > v1->v2:
> > >  - New patch.
> > >
> > >  drivers/i3c/master/renesas-i3c.c | 43
> > > ++++++++------------------------
> > >  1 file changed, 11 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/renesas-i3c.c
> > > b/drivers/i3c/master/renesas-i3c.c
> > > index 426a418f29b6..1b8f4be9ad67 100644
> > > --- a/drivers/i3c/master/renesas-i3c.c
> > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > @@ -198,6 +198,8 @@
> > >  #define RENESAS_I3C_MAX_DEVS	8
> > >  #define I2C_INIT_MSG		-1
> > >
> > > +#define RENESAS_I3C_TCLK_IDX	1
> > > +
> > >  enum i3c_internal_state {
> > >  	I3C_INTERNAL_STATE_DISABLED,
> > >  	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> > > @@ -259,7 +261,8 @@ struct renesas_i3c {
> > >  	u8 addrs[RENESAS_I3C_MAX_DEVS];
> > >  	struct renesas_i3c_xferqueue xferqueue;
> > >  	void __iomem *regs;
> > > -	struct clk *tclk;
> > > +	struct clk_bulk_data *clks;
> > > +	u8 num_clks;
> > >  };
> > >
> > >  struct renesas_i3c_i2c_dev_data {
> > > @@ -272,10 +275,6 @@ struct renesas_i3c_irq_desc {
> > >  	const char *desc;
> > >  };
> > >
> > > -struct renesas_i3c_config {
> > > -	unsigned int has_pclkrw:1;
> > > -};
> > > -
> > >  static inline void renesas_i3c_reg_update(void __iomem *reg, u32
> > > mask, u32 val)  {
> > >  	u32 data = readl(reg);
> > > @@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> > >  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> > >  	int ret;
> > >
> > > -	rate = clk_get_rate(i3c->tclk);
> > > +	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> > >  	if (!rate)
> > >  		return -EINVAL;
> > >
> > > @@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct
> > > platform_device *pdev)  {
> > >  	struct renesas_i3c *i3c;
> > >  	struct reset_control *reset;
> > > -	struct clk *clk;
> > > -	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> > >  	int ret, i;
> > >
> > > -	if (!config)
> > > -		return -ENODATA;
> > > -
> > >  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> > >  	if (!i3c)
> > >  		return -ENOMEM;
> > > @@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(i3c->regs))
> > >  		return PTR_ERR(i3c->regs);
> > >
> > > -	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> > > -	if (IS_ERR(clk))
> > > -		return PTR_ERR(clk);
> > > -
> > > -	if (config->has_pclkrw) {
> > > -		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > > -		if (IS_ERR(clk))
> > > -			return PTR_ERR(clk);
> > > -	}
> > > +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> > > +	if (ret < 0)
> > > +		return ret;
> > >
> > > -	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > > -	if (IS_ERR(i3c->tclk))
> > > -		return PTR_ERR(i3c->tclk);
> > > +	i3c->num_clks = ret;
> >
> > Need check num_clks > RENESAS_I3C_TCLK_IDX to avoid outbound access at
> > i3c->clks[RENESAS_I3C_TCLK_IDX].clk
>
> I guess dt binding check validate this as well. Eg: a single clk defined
> in the DT instead of minimum 2, will give DT warnings.
>
> Do you expect additional check in C code as well?

Yes, worry about a wrong dtb cause kernel crash. Direct access
i3c->clks[RENESAS_I3C_TCLK_IDX] without check is risk.  if clock wrong in
dtb,generally, it just impact function. but this may cause crash. So I
suggest add addtional check here.

Or search 'tclk' in array i3c->clks.
for (i = 0; i < i3c->num_clks; i++)
	if (!strcmp("tclk", i3c->clks[i]->id))
		break;

Frank

>
> Cheers,
> Biju
>
>
> > >
> > >  	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > >  	if (IS_ERR(reset))
> > > @@ -1374,16 +1360,9 @@ static void renesas_i3c_remove(struct platform_device *pdev)
> > >  	i3c_master_unregister(&i3c->base);
> > >  }
> > >
> > > -static const struct renesas_i3c_config empty_i3c_config = { -};
> > > -
> > > -static const struct renesas_i3c_config r9a09g047_i3c_config = {
> > > -	.has_pclkrw = 1,
> > > -};
> > > -
> > >  static const struct of_device_id renesas_i3c_of_ids[] = {
> > > -	{ .compatible = "renesas,r9a08g045-i3c", .data = &empty_i3c_config },
> > > -	{ .compatible = "renesas,r9a09g047-i3c", .data = &r9a09g047_i3c_config },
> > > +	{ .compatible = "renesas,r9a08g045-i3c" },
> > > +	{ .compatible = "renesas,r9a09g047-i3c" },
> > >  	{ /* sentinel */ },
> > >  };
> > >  MODULE_DEVICE_TABLE(of, renesas_i3c_of_ids);
> > > --
> > > 2.43.0
> > >

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

RE: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Biju Das]

Hi Frank Li,

Thanks for the feedback.

> -----Original Message-----
> From: Frank Li <Frank.li@nxp.com>
> Sent: 06 January 2026 14:49
> Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
> 
> On Mon, Jan 05, 2026 at 06:06:19PM +0000, Biju Das wrote:
> > Hi Frank Li,
> >
> > > -----Original Message-----
> > > From: Frank Li <Frank.li@nxp.com>
> > > Sent: 05 January 2026 17:12
> > > Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and
> > > store clocks in private data
> > >
> > > On Mon, Jan 05, 2026 at 11:49:59AM +0100, Tommaso Merciai wrote:
> > > > Replace individual devm_clk_get_enabled() calls with the clk_bulk
> > > > API and store the clock handles in the driver's private data structure.
> > > >
> > > > All clocks required by the controller are now acquired and enabled
> > > > using devm_clk_bulk_get_all_enabled(), removing the need for
> > > > per-SoC clock handling and the renesas_i3c_config data.
> > > > The TCLK is accessed via a fixed index in the bulk clock array.
> > > >
> > > > Simplify the code and prepare the driver for upcoming
> > > > suspend/resume support.
> > > >
> > > > No functional change intended.
> > > >
> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > v3->v4:
> > > >  - Collected Biju Das tag.
> > > >
> > > > v2->v3:
> > > >  - Added define for TCLK index.
> > > >  - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
> > > >  - Improved commit body.
> > > >  - Dropped unnecessary static const char * const renesas_i3c_clks[].
> > > >  - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
> > > >
> > > > v1->v2:
> > > >  - New patch.
> > > >
> > > >  drivers/i3c/master/renesas-i3c.c | 43
> > > > ++++++++------------------------
> > > >  1 file changed, 11 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/drivers/i3c/master/renesas-i3c.c
> > > > b/drivers/i3c/master/renesas-i3c.c
> > > > index 426a418f29b6..1b8f4be9ad67 100644
> > > > --- a/drivers/i3c/master/renesas-i3c.c
> > > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > > @@ -198,6 +198,8 @@
> > > >  #define RENESAS_I3C_MAX_DEVS	8
> > > >  #define I2C_INIT_MSG		-1
> > > >
> > > > +#define RENESAS_I3C_TCLK_IDX	1
> > > > +
> > > >  enum i3c_internal_state {
> > > >  	I3C_INTERNAL_STATE_DISABLED,
> > > >  	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> > > > @@ -259,7 +261,8 @@ struct renesas_i3c {
> > > >  	u8 addrs[RENESAS_I3C_MAX_DEVS];
> > > >  	struct renesas_i3c_xferqueue xferqueue;
> > > >  	void __iomem *regs;
> > > > -	struct clk *tclk;
> > > > +	struct clk_bulk_data *clks;
> > > > +	u8 num_clks;
> > > >  };
> > > >
> > > >  struct renesas_i3c_i2c_dev_data { @@ -272,10 +275,6 @@ struct
> > > > renesas_i3c_irq_desc {
> > > >  	const char *desc;
> > > >  };
> > > >
> > > > -struct renesas_i3c_config {
> > > > -	unsigned int has_pclkrw:1;
> > > > -};
> > > > -
> > > >  static inline void renesas_i3c_reg_update(void __iomem *reg, u32
> > > > mask, u32 val)  {
> > > >  	u32 data = readl(reg);
> > > > @@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> > > >  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> > > >  	int ret;
> > > >
> > > > -	rate = clk_get_rate(i3c->tclk);
> > > > +	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> > > >  	if (!rate)
> > > >  		return -EINVAL;
> > > >
> > > > @@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct
> > > > platform_device *pdev)  {
> > > >  	struct renesas_i3c *i3c;
> > > >  	struct reset_control *reset;
> > > > -	struct clk *clk;
> > > > -	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> > > >  	int ret, i;
> > > >
> > > > -	if (!config)
> > > > -		return -ENODATA;
> > > > -
> > > >  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> > > >  	if (!i3c)
> > > >  		return -ENOMEM;
> > > > @@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> > > >  	if (IS_ERR(i3c->regs))
> > > >  		return PTR_ERR(i3c->regs);
> > > >
> > > > -	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> > > > -	if (IS_ERR(clk))
> > > > -		return PTR_ERR(clk);
> > > > -
> > > > -	if (config->has_pclkrw) {
> > > > -		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > > > -		if (IS_ERR(clk))
> > > > -			return PTR_ERR(clk);
> > > > -	}
> > > > +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> > > > +	if (ret < 0)
> > > > +		return ret;
> > > >
> > > > -	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > > > -	if (IS_ERR(i3c->tclk))
> > > > -		return PTR_ERR(i3c->tclk);
> > > > +	i3c->num_clks = ret;
> > >
> > > Need check num_clks > RENESAS_I3C_TCLK_IDX to avoid outbound access
> > > at
> > > i3c->clks[RENESAS_I3C_TCLK_IDX].clk
> >
> > I guess dt binding check validate this as well. Eg: a single clk
> > defined in the DT instead of minimum 2, will give DT warnings.
> >
> > Do you expect additional check in C code as well?
> 
> Yes, worry about a wrong dtb cause kernel crash. Direct access
> i3c->clks[RENESAS_I3C_TCLK_IDX] without check is risk.  if clock wrong
> i3c->in
> dtb,generally, it just impact function. but this may cause crash. So I suggest add addtional check
> here.

OK, to avoid a crash num_clks > RENESAS_I3C_TCLK_IDX check is sufficient
But it does not check whether tclk in 0th entry or first entry.

> 
> Or search 'tclk' in array i3c->clks.
> for (i = 0; i < i3c->num_clks; i++)
> 	if (!strcmp("tclk", i3c->clks[i]->id))
> 		break;

But this returns correct index of the "tclk"

Cheers,
Biju

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

Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Frank Li]

On Tue, Jan 06, 2026 at 03:01:49PM +0000, Biju Das wrote:
> Hi Frank Li,
>
> Thanks for the feedback.
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@nxp.com>
> > Sent: 06 January 2026 14:49
> > Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
> >
> > On Mon, Jan 05, 2026 at 06:06:19PM +0000, Biju Das wrote:
> > > Hi Frank Li,
> > >
> > > > -----Original Message-----
> > > > From: Frank Li <Frank.li@nxp.com>
> > > > Sent: 05 January 2026 17:12
> > > > Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and
> > > > store clocks in private data
> > > >
> > > > On Mon, Jan 05, 2026 at 11:49:59AM +0100, Tommaso Merciai wrote:
> > > > > Replace individual devm_clk_get_enabled() calls with the clk_bulk
> > > > > API and store the clock handles in the driver's private data structure.
> > > > >
> > > > > All clocks required by the controller are now acquired and enabled
> > > > > using devm_clk_bulk_get_all_enabled(), removing the need for
> > > > > per-SoC clock handling and the renesas_i3c_config data.
> > > > > The TCLK is accessed via a fixed index in the bulk clock array.
> > > > >
> > > > > Simplify the code and prepare the driver for upcoming
> > > > > suspend/resume support.
> > > > >
> > > > > No functional change intended.
> > > > >
> > > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > ---
> > > > > v3->v4:
> > > > >  - Collected Biju Das tag.
> > > > >
> > > > > v2->v3:
> > > > >  - Added define for TCLK index.
> > > > >  - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
> > > > >  - Improved commit body.
> > > > >  - Dropped unnecessary static const char * const renesas_i3c_clks[].
> > > > >  - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
> > > > >
> > > > > v1->v2:
> > > > >  - New patch.
> > > > >
> > > > >  drivers/i3c/master/renesas-i3c.c | 43
> > > > > ++++++++------------------------
> > > > >  1 file changed, 11 insertions(+), 32 deletions(-)
> > > > >
> > > > > diff --git a/drivers/i3c/master/renesas-i3c.c
> > > > > b/drivers/i3c/master/renesas-i3c.c
> > > > > index 426a418f29b6..1b8f4be9ad67 100644
> > > > > --- a/drivers/i3c/master/renesas-i3c.c
> > > > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > > > @@ -198,6 +198,8 @@
> > > > >  #define RENESAS_I3C_MAX_DEVS	8
> > > > >  #define I2C_INIT_MSG		-1
> > > > >
> > > > > +#define RENESAS_I3C_TCLK_IDX	1
> > > > > +
> > > > >  enum i3c_internal_state {
> > > > >  	I3C_INTERNAL_STATE_DISABLED,
> > > > >  	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> > > > > @@ -259,7 +261,8 @@ struct renesas_i3c {
> > > > >  	u8 addrs[RENESAS_I3C_MAX_DEVS];
> > > > >  	struct renesas_i3c_xferqueue xferqueue;
> > > > >  	void __iomem *regs;
> > > > > -	struct clk *tclk;
> > > > > +	struct clk_bulk_data *clks;
> > > > > +	u8 num_clks;
> > > > >  };
> > > > >
> > > > >  struct renesas_i3c_i2c_dev_data { @@ -272,10 +275,6 @@ struct
> > > > > renesas_i3c_irq_desc {
> > > > >  	const char *desc;
> > > > >  };
> > > > >
> > > > > -struct renesas_i3c_config {
> > > > > -	unsigned int has_pclkrw:1;
> > > > > -};
> > > > > -
> > > > >  static inline void renesas_i3c_reg_update(void __iomem *reg, u32
> > > > > mask, u32 val)  {
> > > > >  	u32 data = readl(reg);
> > > > > @@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> > > > >  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> > > > >  	int ret;
> > > > >
> > > > > -	rate = clk_get_rate(i3c->tclk);
> > > > > +	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> > > > >  	if (!rate)
> > > > >  		return -EINVAL;
> > > > >
> > > > > @@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct
> > > > > platform_device *pdev)  {
> > > > >  	struct renesas_i3c *i3c;
> > > > >  	struct reset_control *reset;
> > > > > -	struct clk *clk;
> > > > > -	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> > > > >  	int ret, i;
> > > > >
> > > > > -	if (!config)
> > > > > -		return -ENODATA;
> > > > > -
> > > > >  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> > > > >  	if (!i3c)
> > > > >  		return -ENOMEM;
> > > > > @@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> > > > >  	if (IS_ERR(i3c->regs))
> > > > >  		return PTR_ERR(i3c->regs);
> > > > >
> > > > > -	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> > > > > -	if (IS_ERR(clk))
> > > > > -		return PTR_ERR(clk);
> > > > > -
> > > > > -	if (config->has_pclkrw) {
> > > > > -		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > > > > -		if (IS_ERR(clk))
> > > > > -			return PTR_ERR(clk);
> > > > > -	}
> > > > > +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> > > > > +	if (ret < 0)
> > > > > +		return ret;
> > > > >
> > > > > -	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > > > > -	if (IS_ERR(i3c->tclk))
> > > > > -		return PTR_ERR(i3c->tclk);
> > > > > +	i3c->num_clks = ret;
> > > >
> > > > Need check num_clks > RENESAS_I3C_TCLK_IDX to avoid outbound access
> > > > at
> > > > i3c->clks[RENESAS_I3C_TCLK_IDX].clk
> > >
> > > I guess dt binding check validate this as well. Eg: a single clk
> > > defined in the DT instead of minimum 2, will give DT warnings.
> > >
> > > Do you expect additional check in C code as well?
> >
> > Yes, worry about a wrong dtb cause kernel crash. Direct access
> > i3c->clks[RENESAS_I3C_TCLK_IDX] without check is risk.  if clock wrong
> > i3c->in
> > dtb,generally, it just impact function. but this may cause crash. So I suggest add addtional check
> > here.
>
> OK, to avoid a crash num_clks > RENESAS_I3C_TCLK_IDX check is sufficient

Yes,

> But it does not check whether tclk in 0th entry or first entry.
>
> >
> > Or search 'tclk' in array i3c->clks.
> > for (i = 0; i < i3c->num_clks; i++)
> > 	if (!strcmp("tclk", i3c->clks[i]->id))
> > 		break;
>
> But this returns correct index of the "tclk"

This is just more flexiable for clk schema. If your schema is simple enough
check num_clks > RENESAS_I3C_TCLK_IDX should be enough.

Frank

>
> Cheers,
> Biju

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

RE: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Biju Das]

Hi Frank Li,

> -----Original Message-----
> From: Frank Li <Frank.li@nxp.com>
> Sent: 06 January 2026 15:17
> Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
> 
> On Tue, Jan 06, 2026 at 03:01:49PM +0000, Biju Das wrote:
> > Hi Frank Li,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: Frank Li <Frank.li@nxp.com>
> > > Sent: 06 January 2026 14:49
> > > Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and
> > > store clocks in private data
> > >
> > > On Mon, Jan 05, 2026 at 06:06:19PM +0000, Biju Das wrote:
> > > > Hi Frank Li,
> > > >
> > > > > -----Original Message-----
> > > > > From: Frank Li <Frank.li@nxp.com>
> > > > > Sent: 05 January 2026 17:12
> > > > > Subject: Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API
> > > > > and store clocks in private data
> > > > >
> > > > > On Mon, Jan 05, 2026 at 11:49:59AM +0100, Tommaso Merciai wrote:
> > > > > > Replace individual devm_clk_get_enabled() calls with the
> > > > > > clk_bulk API and store the clock handles in the driver's private data structure.
> > > > > >
> > > > > > All clocks required by the controller are now acquired and
> > > > > > enabled using devm_clk_bulk_get_all_enabled(), removing the
> > > > > > need for per-SoC clock handling and the renesas_i3c_config data.
> > > > > > The TCLK is accessed via a fixed index in the bulk clock array.
> > > > > >
> > > > > > Simplify the code and prepare the driver for upcoming
> > > > > > suspend/resume support.
> > > > > >
> > > > > > No functional change intended.
> > > > > >
> > > > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > Signed-off-by: Tommaso Merciai
> > > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > > ---
> > > > > > v3->v4:
> > > > > >  - Collected Biju Das tag.
> > > > > >
> > > > > > v2->v3:
> > > > > >  - Added define for TCLK index.
> > > > > >  - Use devm_clk_bulk_get_all_enabled() into renesas_i3c_probe().
> > > > > >  - Improved commit body.
> > > > > >  - Dropped unnecessary static const char * const renesas_i3c_clks[].
> > > > > >  - Removed the need for per-SoC clock handling and the renesas_i3c_config data.
> > > > > >
> > > > > > v1->v2:
> > > > > >  - New patch.
> > > > > >
> > > > > >  drivers/i3c/master/renesas-i3c.c | 43
> > > > > > ++++++++------------------------
> > > > > >  1 file changed, 11 insertions(+), 32 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/i3c/master/renesas-i3c.c
> > > > > > b/drivers/i3c/master/renesas-i3c.c
> > > > > > index 426a418f29b6..1b8f4be9ad67 100644
> > > > > > --- a/drivers/i3c/master/renesas-i3c.c
> > > > > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > > > > @@ -198,6 +198,8 @@
> > > > > >  #define RENESAS_I3C_MAX_DEVS	8
> > > > > >  #define I2C_INIT_MSG		-1
> > > > > >
> > > > > > +#define RENESAS_I3C_TCLK_IDX	1
> > > > > > +
> > > > > >  enum i3c_internal_state {
> > > > > >  	I3C_INTERNAL_STATE_DISABLED,
> > > > > >  	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> > > > > > @@ -259,7 +261,8 @@ struct renesas_i3c {
> > > > > >  	u8 addrs[RENESAS_I3C_MAX_DEVS];
> > > > > >  	struct renesas_i3c_xferqueue xferqueue;
> > > > > >  	void __iomem *regs;
> > > > > > -	struct clk *tclk;
> > > > > > +	struct clk_bulk_data *clks;
> > > > > > +	u8 num_clks;
> > > > > >  };
> > > > > >
> > > > > >  struct renesas_i3c_i2c_dev_data { @@ -272,10 +275,6 @@ struct
> > > > > > renesas_i3c_irq_desc {
> > > > > >  	const char *desc;
> > > > > >  };
> > > > > >
> > > > > > -struct renesas_i3c_config {
> > > > > > -	unsigned int has_pclkrw:1;
> > > > > > -};
> > > > > > -
> > > > > >  static inline void renesas_i3c_reg_update(void __iomem *reg,
> > > > > > u32 mask, u32 val)  {
> > > > > >  	u32 data = readl(reg);
> > > > > > @@ -489,7 +488,7 @@ static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> > > > > >  	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> > > > > >  	int ret;
> > > > > >
> > > > > > -	rate = clk_get_rate(i3c->tclk);
> > > > > > +	rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
> > > > > >  	if (!rate)
> > > > > >  		return -EINVAL;
> > > > > >
> > > > > > @@ -1302,13 +1301,8 @@ static int renesas_i3c_probe(struct
> > > > > > platform_device *pdev)  {
> > > > > >  	struct renesas_i3c *i3c;
> > > > > >  	struct reset_control *reset;
> > > > > > -	struct clk *clk;
> > > > > > -	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> > > > > >  	int ret, i;
> > > > > >
> > > > > > -	if (!config)
> > > > > > -		return -ENODATA;
> > > > > > -
> > > > > >  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> > > > > >  	if (!i3c)
> > > > > >  		return -ENOMEM;
> > > > > > @@ -1317,19 +1311,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> > > > > >  	if (IS_ERR(i3c->regs))
> > > > > >  		return PTR_ERR(i3c->regs);
> > > > > >
> > > > > > -	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> > > > > > -	if (IS_ERR(clk))
> > > > > > -		return PTR_ERR(clk);
> > > > > > -
> > > > > > -	if (config->has_pclkrw) {
> > > > > > -		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > > > > > -		if (IS_ERR(clk))
> > > > > > -			return PTR_ERR(clk);
> > > > > > -	}
> > > > > > +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> > > > > > +	if (ret < 0)
> > > > > > +		return ret;
> > > > > >
> > > > > > -	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > > > > > -	if (IS_ERR(i3c->tclk))
> > > > > > -		return PTR_ERR(i3c->tclk);
> > > > > > +	i3c->num_clks = ret;
> > > > >
> > > > > Need check num_clks > RENESAS_I3C_TCLK_IDX to avoid outbound
> > > > > access at
> > > > > i3c->clks[RENESAS_I3C_TCLK_IDX].clk
> > > >
> > > > I guess dt binding check validate this as well. Eg: a single clk
> > > > defined in the DT instead of minimum 2, will give DT warnings.
> > > >
> > > > Do you expect additional check in C code as well?
> > >
> > > Yes, worry about a wrong dtb cause kernel crash. Direct access
> > > i3c->clks[RENESAS_I3C_TCLK_IDX] without check is risk.  if clock
> > > i3c->wrong in
> > > dtb,generally, it just impact function. but this may cause crash. So
> > > I suggest add addtional check here.
> >
> > OK, to avoid a crash num_clks > RENESAS_I3C_TCLK_IDX check is
> > sufficient
> 
> Yes,
> 
> > But it does not check whether tclk in 0th entry or first entry.
> >
> > >
> > > Or search 'tclk' in array i3c->clks.
> > > for (i = 0; i < i3c->num_clks; i++)
> > > 	if (!strcmp("tclk", i3c->clks[i]->id))
> > > 		break;
> >
> > But this returns correct index of the "tclk"
> 
> This is just more flexiable for clk schema. If your schema is simple enough check num_clks >
> RENESAS_I3C_TCLK_IDX should be enough.

For avoiding crash that check is sufficient, but as you said wrong dtb like crash
can also lead to non-functional i3c device

Eg:

From bindings, we expect the below entries in DTS for RZ/G3S

"pclk", "tclk" for RZ/G3S

But user mistakenly just added "pclk", that will lead to kernel crash

Or

Swap the clocks

"tclk", "pclk" this will lead to non-functional i3c device

On both cases, user ignored DT binding check warnings.

As you said crash is fatal, maybe stick with just num_clks > RENESAS_I3C_TCLK_IDX

Cheers,
Biju

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

Re: [PATCH v4 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data

[Alexandre Belloni]

On 06/01/2026 15:41:10+0000, Biju Das wrote:
> > This is just more flexiable for clk schema. If your schema is simple enough check num_clks >
> > RENESAS_I3C_TCLK_IDX should be enough.
> 
> For avoiding crash that check is sufficient, but as you said wrong dtb like crash
> can also lead to non-functional i3c device
> 
> Eg:
> 
> From bindings, we expect the below entries in DTS for RZ/G3S
> 
> "pclk", "tclk" for RZ/G3S
> 
> But user mistakenly just added "pclk", that will lead to kernel crash
> 
> Or
> 
> Swap the clocks
> 
> "tclk", "pclk" this will lead to non-functional i3c device
> 
> On both cases, user ignored DT binding check warnings.
> 
> As you said crash is fatal, maybe stick with just num_clks > RENESAS_I3C_TCLK_IDX

Yes, this is enough


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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