* [PATCH v2 0/4] i3c: renesas: Add suspend/resume support
@ 2025-12-30 10:39 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
Dear All,
This series adds suspend/resume support to the Renesas I3C controller driver.
Thanks & Regards,
Tommaso
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 | 257 ++++++++++++++++++++++---------
1 file changed, 184 insertions(+), 73 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 10:39 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
Replace individual devm_clk_get_enabled() calls with the clk_bulk API
and store the clock handles in the driver's private data structure.
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 | 42 +++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 426a418f29b6..8ef6ff06df90 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -259,7 +259,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[3];
+ u8 num_clks;
};
struct renesas_i3c_i2c_dev_data {
@@ -276,6 +277,10 @@ struct renesas_i3c_config {
unsigned int has_pclkrw:1;
};
+static const char * const renesas_i3c_clks[] = {
+ "pclk", "tclk", "pclkrw"
+};
+
static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
{
u32 data = readl(reg);
@@ -489,7 +494,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[1].clk);
if (!rate)
return -EINVAL;
@@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
{ .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
};
+static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
+{
+ struct renesas_i3c *i3c = data;
+
+ clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
+}
+
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;
@@ -1317,19 +1328,22 @@ 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);
+ i3c->num_clks = config->has_pclkrw ? 3 : 2;
- if (config->has_pclkrw) {
- clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
- if (IS_ERR(clk))
- return PTR_ERR(clk);
- }
+ for (i = 0; i < i3c->num_clks; i++)
+ i3c->clks[i].id = renesas_i3c_clks[i];
- i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
- if (IS_ERR(i3c->tclk))
- return PTR_ERR(i3c->tclk);
+ ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
+ if (ret)
+ return ret;
+
+ ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
+ if (ret)
+ return ret;
reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
if (IS_ERR(reset))
--
2.43.0
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
@ 2025-12-30 10:39 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
Replace individual devm_clk_get_enabled() calls with the clk_bulk API
and store the clock handles in the driver's private data structure.
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 | 42 +++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 426a418f29b6..8ef6ff06df90 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -259,7 +259,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[3];
+ u8 num_clks;
};
struct renesas_i3c_i2c_dev_data {
@@ -276,6 +277,10 @@ struct renesas_i3c_config {
unsigned int has_pclkrw:1;
};
+static const char * const renesas_i3c_clks[] = {
+ "pclk", "tclk", "pclkrw"
+};
+
static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
{
u32 data = readl(reg);
@@ -489,7 +494,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[1].clk);
if (!rate)
return -EINVAL;
@@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
{ .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
};
+static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
+{
+ struct renesas_i3c *i3c = data;
+
+ clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
+}
+
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;
@@ -1317,19 +1328,22 @@ 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);
+ i3c->num_clks = config->has_pclkrw ? 3 : 2;
- if (config->has_pclkrw) {
- clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
- if (IS_ERR(clk))
- return PTR_ERR(clk);
- }
+ for (i = 0; i < i3c->num_clks; i++)
+ i3c->clks[i].id = renesas_i3c_clks[i];
- i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
- if (IS_ERR(i3c->tclk))
- return PTR_ERR(i3c->tclk);
+ ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
+ if (ret)
+ return ret;
+
+ ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
+ if (ret)
+ return ret;
reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
if (IS_ERR(reset))
--
2.43.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 15:44 ` Frank Li
-1 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:44 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
On Tue, Dec 30, 2025 at 11:39:36AM +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.
>
> 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 | 42 +++++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 426a418f29b6..8ef6ff06df90 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -259,7 +259,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[3];
> + u8 num_clks;
> };
>
> struct renesas_i3c_i2c_dev_data {
> @@ -276,6 +277,10 @@ struct renesas_i3c_config {
> unsigned int has_pclkrw:1;
> };
>
> +static const char * const renesas_i3c_clks[] = {
> + "pclk", "tclk", "pclkrw"
> +};
> +
> static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
> {
> u32 data = readl(reg);
> @@ -489,7 +494,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[1].clk);
Can you use macro of variable replace hardcode "1"
> if (!rate)
> return -EINVAL;
>
> @@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> { .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
> };
>
> +static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> +{
> + struct renesas_i3c *i3c = data;
> +
> + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> +}
> +
> 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;
>
> @@ -1317,19 +1328,22 @@ 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);
> + i3c->num_clks = config->has_pclkrw ? 3 : 2;
>
> - if (config->has_pclkrw) {
> - clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> - if (IS_ERR(clk))
> - return PTR_ERR(clk);
> - }
> + for (i = 0; i < i3c->num_clks; i++)
> + i3c->clks[i].id = renesas_i3c_clks[i];
>
> - i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> - if (IS_ERR(i3c->tclk))
> - return PTR_ERR(i3c->tclk);
> + ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
> + if (ret)
> + return ret;
> +
> + ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
> + if (ret)
> + return ret;
Can you use devm_clk_bulk_get_all_enabled()? all dts already verified
by dt-schema.
Frank
>
> reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> if (IS_ERR(reset))
> --
> 2.43.0
>
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
@ 2025-12-30 15:44 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:44 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
On Tue, Dec 30, 2025 at 11:39:36AM +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.
>
> 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 | 42 +++++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 426a418f29b6..8ef6ff06df90 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -259,7 +259,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[3];
> + u8 num_clks;
> };
>
> struct renesas_i3c_i2c_dev_data {
> @@ -276,6 +277,10 @@ struct renesas_i3c_config {
> unsigned int has_pclkrw:1;
> };
>
> +static const char * const renesas_i3c_clks[] = {
> + "pclk", "tclk", "pclkrw"
> +};
> +
> static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
> {
> u32 data = readl(reg);
> @@ -489,7 +494,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[1].clk);
Can you use macro of variable replace hardcode "1"
> if (!rate)
> return -EINVAL;
>
> @@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> { .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
> };
>
> +static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> +{
> + struct renesas_i3c *i3c = data;
> +
> + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> +}
> +
> 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;
>
> @@ -1317,19 +1328,22 @@ 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);
> + i3c->num_clks = config->has_pclkrw ? 3 : 2;
>
> - if (config->has_pclkrw) {
> - clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> - if (IS_ERR(clk))
> - return PTR_ERR(clk);
> - }
> + for (i = 0; i < i3c->num_clks; i++)
> + i3c->clks[i].id = renesas_i3c_clks[i];
>
> - i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> - if (IS_ERR(i3c->tclk))
> - return PTR_ERR(i3c->tclk);
> + ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
> + if (ret)
> + return ret;
> +
> + ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
> + if (ret)
> + return ret;
Can you use devm_clk_bulk_get_all_enabled()? all dts already verified
by dt-schema.
Frank
>
> reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> if (IS_ERR(reset))
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
2025-12-30 15:44 ` Frank Li
@ 2025-12-30 16:24 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 16:24 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
Hi Frank,
Thanks for your review!
On Tue, Dec 30, 2025 at 10:44:02AM -0500, Frank Li wrote:
> On Tue, Dec 30, 2025 at 11:39:36AM +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.
> >
> > 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 | 42 +++++++++++++++++++++-----------
> > 1 file changed, 28 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > index 426a418f29b6..8ef6ff06df90 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -259,7 +259,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[3];
> > + u8 num_clks;
> > };
> >
> > struct renesas_i3c_i2c_dev_data {
> > @@ -276,6 +277,10 @@ struct renesas_i3c_config {
> > unsigned int has_pclkrw:1;
> > };
> >
> > +static const char * const renesas_i3c_clks[] = {
> > + "pclk", "tclk", "pclkrw"
> > +};
> > +
> > static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
> > {
> > u32 data = readl(reg);
> > @@ -489,7 +494,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[1].clk);
>
> Can you use macro of variable replace hardcode "1"
Ack! I'd go for:
i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
in v3, thanks.
>
> > if (!rate)
> > return -EINVAL;
> >
> > @@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> > { .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
> > };
> >
> > +static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> > +{
> > + struct renesas_i3c *i3c = data;
> > +
> > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> > +}
> > +
> > 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;
> >
> > @@ -1317,19 +1328,22 @@ 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);
> > + i3c->num_clks = config->has_pclkrw ? 3 : 2;
> >
> > - if (config->has_pclkrw) {
> > - clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > - if (IS_ERR(clk))
> > - return PTR_ERR(clk);
> > - }
> > + for (i = 0; i < i3c->num_clks; i++)
> > + i3c->clks[i].id = renesas_i3c_clks[i];
> >
> > - i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > - if (IS_ERR(i3c->tclk))
> > - return PTR_ERR(i3c->tclk);
> > + ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
> > + if (ret)
> > + return ret;
> > +
> > + ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
> > + if (ret)
> > + return ret;
>
> Can you use devm_clk_bulk_get_all_enabled()? all dts already verified
> by dt-schema.
Ack! I'd got for:
i3c->num_clks = config->has_pclkrw ? 3 : 2;
for (i = 0; i < i3c->num_clks; i++)
i3c->clks[i].id = renesas_i3c_clks[i];
ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
if (ret < 0)
return ret;
In v3, Thanks.
>
> Frank
>
> >
> > reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > if (IS_ERR(reset))
> > --
> > 2.43.0
> >
Kind Regards,
Tommaso
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
@ 2025-12-30 16:24 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 16:24 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
Hi Frank,
Thanks for your review!
On Tue, Dec 30, 2025 at 10:44:02AM -0500, Frank Li wrote:
> On Tue, Dec 30, 2025 at 11:39:36AM +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.
> >
> > 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 | 42 +++++++++++++++++++++-----------
> > 1 file changed, 28 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > index 426a418f29b6..8ef6ff06df90 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -259,7 +259,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[3];
> > + u8 num_clks;
> > };
> >
> > struct renesas_i3c_i2c_dev_data {
> > @@ -276,6 +277,10 @@ struct renesas_i3c_config {
> > unsigned int has_pclkrw:1;
> > };
> >
> > +static const char * const renesas_i3c_clks[] = {
> > + "pclk", "tclk", "pclkrw"
> > +};
> > +
> > static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
> > {
> > u32 data = readl(reg);
> > @@ -489,7 +494,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[1].clk);
>
> Can you use macro of variable replace hardcode "1"
Ack! I'd go for:
i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
in v3, thanks.
>
> > if (!rate)
> > return -EINVAL;
> >
> > @@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> > { .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
> > };
> >
> > +static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> > +{
> > + struct renesas_i3c *i3c = data;
> > +
> > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> > +}
> > +
> > 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;
> >
> > @@ -1317,19 +1328,22 @@ 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);
> > + i3c->num_clks = config->has_pclkrw ? 3 : 2;
> >
> > - if (config->has_pclkrw) {
> > - clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > - if (IS_ERR(clk))
> > - return PTR_ERR(clk);
> > - }
> > + for (i = 0; i < i3c->num_clks; i++)
> > + i3c->clks[i].id = renesas_i3c_clks[i];
> >
> > - i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > - if (IS_ERR(i3c->tclk))
> > - return PTR_ERR(i3c->tclk);
> > + ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
> > + if (ret)
> > + return ret;
> > +
> > + ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
> > + if (ret)
> > + return ret;
>
> Can you use devm_clk_bulk_get_all_enabled()? all dts already verified
> by dt-schema.
Ack! I'd got for:
i3c->num_clks = config->has_pclkrw ? 3 : 2;
for (i = 0; i < i3c->num_clks; i++)
i3c->clks[i].id = renesas_i3c_clks[i];
ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
if (ret < 0)
return ret;
In v3, Thanks.
>
> Frank
>
> >
> > reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > if (IS_ERR(reset))
> > --
> > 2.43.0
> >
Kind Regards,
Tommaso
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
2025-12-30 16:24 ` Tommaso Merciai
@ 2025-12-30 18:39 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 18:39 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
Hi Frank,
On Tue, Dec 30, 2025 at 05:24:54PM +0100, Tommaso Merciai wrote:
> Hi Frank,
> Thanks for your review!
>
>
> On Tue, Dec 30, 2025 at 10:44:02AM -0500, Frank Li wrote:
> > On Tue, Dec 30, 2025 at 11:39:36AM +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.
> > >
> > > 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 | 42 +++++++++++++++++++++-----------
> > > 1 file changed, 28 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > > index 426a418f29b6..8ef6ff06df90 100644
> > > --- a/drivers/i3c/master/renesas-i3c.c
> > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > @@ -259,7 +259,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[3];
> > > + u8 num_clks;
> > > };
> > >
> > > struct renesas_i3c_i2c_dev_data {
> > > @@ -276,6 +277,10 @@ struct renesas_i3c_config {
> > > unsigned int has_pclkrw:1;
> > > };
> > >
> > > +static const char * const renesas_i3c_clks[] = {
> > > + "pclk", "tclk", "pclkrw"
> > > +};
> > > +
> > > static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
> > > {
> > > u32 data = readl(reg);
> > > @@ -489,7 +494,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[1].clk);
> >
> > Can you use macro of variable replace hardcode "1"
>
> Ack! I'd go for:
>
> i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
>
> in v3, thanks.
>
> >
> > > if (!rate)
> > > return -EINVAL;
> > >
> > > @@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> > > { .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
> > > };
> > >
> > > +static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> > > +{
> > > + struct renesas_i3c *i3c = data;
> > > +
> > > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> > > +}
> > > +
> > > 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;
> > >
> > > @@ -1317,19 +1328,22 @@ 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);
> > > + i3c->num_clks = config->has_pclkrw ? 3 : 2;
> > >
> > > - if (config->has_pclkrw) {
> > > - clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > > - if (IS_ERR(clk))
> > > - return PTR_ERR(clk);
> > > - }
> > > + for (i = 0; i < i3c->num_clks; i++)
> > > + i3c->clks[i].id = renesas_i3c_clks[i];
> > >
> > > - i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > > - if (IS_ERR(i3c->tclk))
> > > - return PTR_ERR(i3c->tclk);
> > > + ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
> > > + if (ret)
> > > + return ret;
> >
> > Can you use devm_clk_bulk_get_all_enabled()? all dts already verified
> > by dt-schema.
>
> Ack! I'd got for:
>
> i3c->num_clks = config->has_pclkrw ? 3 : 2;
>
> for (i = 0; i < i3c->num_clks; i++)
> i3c->clks[i].id = renesas_i3c_clks[i];
>
> ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> if (ret < 0)
> return ret;
Checking better I think we can directly use:
ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
if (ret < 0)
return ret;
i3c->num_clks = ret;
In this way we can drop also empty_i3c_config and r9a09g047_i3c_config
that are no more required.
Thanks & Regards,
Tommaso
>
> In v3, Thanks.
>
> >
> > Frank
> >
> > >
> > > reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > > if (IS_ERR(reset))
> > > --
> > > 2.43.0
> > >
>
> Kind Regards,
> Tommaso
>
>
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/4] i3c: renesas: Switch to clk_bulk API and store clocks in private data
@ 2025-12-30 18:39 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 18:39 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
Hi Frank,
On Tue, Dec 30, 2025 at 05:24:54PM +0100, Tommaso Merciai wrote:
> Hi Frank,
> Thanks for your review!
>
>
> On Tue, Dec 30, 2025 at 10:44:02AM -0500, Frank Li wrote:
> > On Tue, Dec 30, 2025 at 11:39:36AM +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.
> > >
> > > 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 | 42 +++++++++++++++++++++-----------
> > > 1 file changed, 28 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > > index 426a418f29b6..8ef6ff06df90 100644
> > > --- a/drivers/i3c/master/renesas-i3c.c
> > > +++ b/drivers/i3c/master/renesas-i3c.c
> > > @@ -259,7 +259,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[3];
> > > + u8 num_clks;
> > > };
> > >
> > > struct renesas_i3c_i2c_dev_data {
> > > @@ -276,6 +277,10 @@ struct renesas_i3c_config {
> > > unsigned int has_pclkrw:1;
> > > };
> > >
> > > +static const char * const renesas_i3c_clks[] = {
> > > + "pclk", "tclk", "pclkrw"
> > > +};
> > > +
> > > static inline void renesas_i3c_reg_update(void __iomem *reg, u32 mask, u32 val)
> > > {
> > > u32 data = readl(reg);
> > > @@ -489,7 +494,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[1].clk);
> >
> > Can you use macro of variable replace hardcode "1"
>
> Ack! I'd go for:
>
> i3c->rate = clk_get_rate(i3c->clks[RENESAS_I3C_TCLK_IDX].clk);
>
> in v3, thanks.
>
> >
> > > if (!rate)
> > > return -EINVAL;
> > >
> > > @@ -1298,11 +1303,17 @@ static const struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> > > { .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
> > > };
> > >
> > > +static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> > > +{
> > > + struct renesas_i3c *i3c = data;
> > > +
> > > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> > > +}
> > > +
> > > 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;
> > >
> > > @@ -1317,19 +1328,22 @@ 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);
> > > + i3c->num_clks = config->has_pclkrw ? 3 : 2;
> > >
> > > - if (config->has_pclkrw) {
> > > - clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> > > - if (IS_ERR(clk))
> > > - return PTR_ERR(clk);
> > > - }
> > > + for (i = 0; i < i3c->num_clks; i++)
> > > + i3c->clks[i].id = renesas_i3c_clks[i];
> > >
> > > - i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> > > - if (IS_ERR(i3c->tclk))
> > > - return PTR_ERR(i3c->tclk);
> > > + ret = devm_clk_bulk_get(&pdev->dev, i3c->num_clks, i3c->clks);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = clk_bulk_prepare_enable(i3c->num_clks, i3c->clks);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_clk_bulk_disable_unprepare, i3c);
> > > + if (ret)
> > > + return ret;
> >
> > Can you use devm_clk_bulk_get_all_enabled()? all dts already verified
> > by dt-schema.
>
> Ack! I'd got for:
>
> i3c->num_clks = config->has_pclkrw ? 3 : 2;
>
> for (i = 0; i < i3c->num_clks; i++)
> i3c->clks[i].id = renesas_i3c_clks[i];
>
> ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
> if (ret < 0)
> return ret;
Checking better I think we can directly use:
ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
if (ret < 0)
return ret;
i3c->num_clks = ret;
In this way we can drop also empty_i3c_config and r9a09g047_i3c_config
that are no more required.
Thanks & Regards,
Tommaso
>
> In v3, Thanks.
>
> >
> > Frank
> >
> > >
> > > reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> > > if (IS_ERR(reset))
> > > --
> > > 2.43.0
> > >
>
> Kind Regards,
> Tommaso
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 10:39 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
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.
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 | 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 8ef6ff06df90..2736363c9074 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -256,11 +256,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[3];
u8 num_clks;
+ struct reset_control *presetn;
+ struct reset_control *tresetn;
};
struct renesas_i3c_i2c_dev_data {
@@ -488,22 +491,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[1].clk);
- if (!rate)
+ i3c->rate = clk_get_rate(i3c->clks[1].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);
@@ -516,7 +518,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)
@@ -524,7 +526,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 */
@@ -545,8 +547,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) |
@@ -597,13 +599,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);
@@ -1313,7 +1315,6 @@ static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
static int renesas_i3c_probe(struct platform_device *pdev)
{
struct renesas_i3c *i3c;
- struct reset_control *reset;
const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
int ret, i;
@@ -1345,14 +1346,14 @@ static int renesas_i3c_probe(struct platform_device *pdev)
if (ret)
return 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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c
@ 2025-12-30 10:39 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
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.
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 | 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 8ef6ff06df90..2736363c9074 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -256,11 +256,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[3];
u8 num_clks;
+ struct reset_control *presetn;
+ struct reset_control *tresetn;
};
struct renesas_i3c_i2c_dev_data {
@@ -488,22 +491,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[1].clk);
- if (!rate)
+ i3c->rate = clk_get_rate(i3c->clks[1].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);
@@ -516,7 +518,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)
@@ -524,7 +526,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 */
@@ -545,8 +547,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) |
@@ -597,13 +599,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);
@@ -1313,7 +1315,6 @@ static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
static int renesas_i3c_probe(struct platform_device *pdev)
{
struct renesas_i3c *i3c;
- struct reset_control *reset;
const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
int ret, i;
@@ -1345,14 +1346,14 @@ static int renesas_i3c_probe(struct platform_device *pdev)
if (ret)
return 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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 15:45 ` Frank Li
-1 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:45 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
On Tue, Dec 30, 2025 at 11:39:37AM +0100, Tommaso Merciai wrote:
> 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.
>
> 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.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> 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 8ef6ff06df90..2736363c9074 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -256,11 +256,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[3];
> u8 num_clks;
> + struct reset_control *presetn;
> + struct reset_control *tresetn;
> };
>
> struct renesas_i3c_i2c_dev_data {
> @@ -488,22 +491,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[1].clk);
> - if (!rate)
> + i3c->rate = clk_get_rate(i3c->clks[1].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);
>
> @@ -516,7 +518,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)
> @@ -524,7 +526,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 */
> @@ -545,8 +547,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) |
> @@ -597,13 +599,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);
> @@ -1313,7 +1315,6 @@ static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> static int renesas_i3c_probe(struct platform_device *pdev)
> {
> struct renesas_i3c *i3c;
> - struct reset_control *reset;
> const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> int ret, i;
>
> @@ -1345,14 +1346,14 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> if (ret)
> return 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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 2/4] i3c: renesas: Store clock rate and reset controls in struct renesas_i3c
@ 2025-12-30 15:45 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:45 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
On Tue, Dec 30, 2025 at 11:39:37AM +0100, Tommaso Merciai wrote:
> 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.
>
> 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.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> 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 8ef6ff06df90..2736363c9074 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -256,11 +256,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[3];
> u8 num_clks;
> + struct reset_control *presetn;
> + struct reset_control *tresetn;
> };
>
> struct renesas_i3c_i2c_dev_data {
> @@ -488,22 +491,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[1].clk);
> - if (!rate)
> + i3c->rate = clk_get_rate(i3c->clks[1].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);
>
> @@ -516,7 +518,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)
> @@ -524,7 +526,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 */
> @@ -545,8 +547,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) |
> @@ -597,13 +599,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);
> @@ -1313,7 +1315,6 @@ static void renesas_i3c_clk_bulk_disable_unprepare(void *data)
> static int renesas_i3c_probe(struct platform_device *pdev)
> {
> struct renesas_i3c *i3c;
> - struct reset_control *reset;
> const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> int ret, i;
>
> @@ -1345,14 +1346,14 @@ static int renesas_i3c_probe(struct platform_device *pdev)
> if (ret)
> return 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
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 10:39 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
@ 2025-12-30 10:39 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 15:47 ` Frank Li
-1 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:47 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
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
>
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
@ 2025-12-30 15:47 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:47 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
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
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
2025-12-30 15:47 ` Frank Li
@ 2025-12-30 16:27 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 16:27 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
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
> >
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/4] i3c: renesas: Factor out hardware initialization to separate function
@ 2025-12-30 16:27 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 16:27 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
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
> >
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 4/4] i3c: renesas: Add suspend/resume support
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 10:39 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
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>
---
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().
- Updated commit body accordingly.
drivers/i3c/master/renesas-i3c.c | 85 ++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 4f076c372b36..f089bbf19fa5 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -254,16 +254,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[3];
u8 num_clks;
struct reset_control *presetn;
struct reset_control *tresetn;
+ u8 refclk_div;
};
struct renesas_i3c_i2c_dev_data {
@@ -615,6 +618,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);
@@ -623,6 +627,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));
@@ -1390,6 +1395,13 @@ 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);
}
@@ -1400,6 +1412,78 @@ 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)
+ return ret;
+
+ ret = reset_control_assert(i3c->tresetn);
+ if (ret) {
+ reset_control_deassert(i3c->presetn);
+ return ret;
+ }
+
+ clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
+
+ return 0;
+}
+
+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_prepare_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:
+ clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
+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 renesas_i3c_config empty_i3c_config = {
};
@@ -1420,6 +1504,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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 4/4] i3c: renesas: Add suspend/resume support
@ 2025-12-30 10:39 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 10:39 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Wolfram Sang,
Alexandre Belloni, Frank Li, Philipp Zabel, linux-i3c,
linux-kernel
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>
---
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().
- Updated commit body accordingly.
drivers/i3c/master/renesas-i3c.c | 85 ++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 4f076c372b36..f089bbf19fa5 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -254,16 +254,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[3];
u8 num_clks;
struct reset_control *presetn;
struct reset_control *tresetn;
+ u8 refclk_div;
};
struct renesas_i3c_i2c_dev_data {
@@ -615,6 +618,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);
@@ -623,6 +627,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));
@@ -1390,6 +1395,13 @@ 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);
}
@@ -1400,6 +1412,78 @@ 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)
+ return ret;
+
+ ret = reset_control_assert(i3c->tresetn);
+ if (ret) {
+ reset_control_deassert(i3c->presetn);
+ return ret;
+ }
+
+ clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
+
+ return 0;
+}
+
+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_prepare_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:
+ clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
+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 renesas_i3c_config empty_i3c_config = {
};
@@ -1420,6 +1504,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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 4/4] i3c: renesas: Add suspend/resume support
2025-12-30 10:39 ` Tommaso Merciai
@ 2025-12-30 15:52 ` Frank Li
-1 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:52 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
On Tue, Dec 30, 2025 at 11:39:39AM +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>
> ---
> 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().
> - Updated commit body accordingly.
>
> drivers/i3c/master/renesas-i3c.c | 85 ++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 4f076c372b36..f089bbf19fa5 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -254,16 +254,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[3];
> u8 num_clks;
> struct reset_control *presetn;
> struct reset_control *tresetn;
> + u8 refclk_div;
> };
>
> struct renesas_i3c_i2c_dev_data {
> @@ -615,6 +618,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);
> @@ -623,6 +627,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));
> @@ -1390,6 +1395,13 @@ 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);
nit: can you move it to previous line to reduce line number
> + if (!i3c->DATBASn)
> + return -ENOMEM;
> +
> return i3c_master_register(&i3c->base, &pdev->dev, &renesas_i3c_ops, false);
> }
>
> @@ -1400,6 +1412,78 @@ 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)
> + return ret;
> +
> + ret = reset_control_assert(i3c->tresetn);
> + if (ret) {
> + reset_control_deassert(i3c->presetn);
> + return ret;
> + }
> +
> + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> +
> + return 0;
> +}
> +
> +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_prepare_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:
> + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks)
look like it should be
reset_control_assert(i3c->tresetn);
Frank
> +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 renesas_i3c_config empty_i3c_config = {
> };
>
> @@ -1420,6 +1504,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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 4/4] i3c: renesas: Add suspend/resume support
@ 2025-12-30 15:52 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-12-30 15:52 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
On Tue, Dec 30, 2025 at 11:39:39AM +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>
> ---
> 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().
> - Updated commit body accordingly.
>
> drivers/i3c/master/renesas-i3c.c | 85 ++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 4f076c372b36..f089bbf19fa5 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -254,16 +254,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[3];
> u8 num_clks;
> struct reset_control *presetn;
> struct reset_control *tresetn;
> + u8 refclk_div;
> };
>
> struct renesas_i3c_i2c_dev_data {
> @@ -615,6 +618,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);
> @@ -623,6 +627,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));
> @@ -1390,6 +1395,13 @@ 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);
nit: can you move it to previous line to reduce line number
> + if (!i3c->DATBASn)
> + return -ENOMEM;
> +
> return i3c_master_register(&i3c->base, &pdev->dev, &renesas_i3c_ops, false);
> }
>
> @@ -1400,6 +1412,78 @@ 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)
> + return ret;
> +
> + ret = reset_control_assert(i3c->tresetn);
> + if (ret) {
> + reset_control_deassert(i3c->presetn);
> + return ret;
> + }
> +
> + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> +
> + return 0;
> +}
> +
> +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_prepare_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:
> + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks)
look like it should be
reset_control_assert(i3c->tresetn);
Frank
> +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 renesas_i3c_config empty_i3c_config = {
> };
>
> @@ -1420,6 +1504,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
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 4/4] i3c: renesas: Add suspend/resume support
2025-12-30 15:52 ` Frank Li
@ 2025-12-30 16:31 ` Tommaso Merciai
-1 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 16:31 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
Hi Frank,
Thanks for your review!
On Tue, Dec 30, 2025 at 10:52:03AM -0500, Frank Li wrote:
> On Tue, Dec 30, 2025 at 11:39:39AM +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>
> > ---
> > 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().
> > - Updated commit body accordingly.
> >
> > drivers/i3c/master/renesas-i3c.c | 85 ++++++++++++++++++++++++++++++++
> > 1 file changed, 85 insertions(+)
> >
> > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > index 4f076c372b36..f089bbf19fa5 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -254,16 +254,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[3];
> > u8 num_clks;
> > struct reset_control *presetn;
> > struct reset_control *tresetn;
> > + u8 refclk_div;
> > };
> >
> > struct renesas_i3c_i2c_dev_data {
> > @@ -615,6 +618,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);
> > @@ -623,6 +627,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));
> > @@ -1390,6 +1395,13 @@ 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);
>
> nit: can you move it to previous line to reduce line number
Ack.
>
> > + if (!i3c->DATBASn)
> > + return -ENOMEM;
> > +
> > return i3c_master_register(&i3c->base, &pdev->dev, &renesas_i3c_ops, false);
> > }
> >
> > @@ -1400,6 +1412,78 @@ 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)
> > + return ret;
> > +
> > + ret = reset_control_assert(i3c->tresetn);
> > + if (ret) {
> > + reset_control_deassert(i3c->presetn);
> > + return ret;
> > + }
> > +
> > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> > +
> > + return 0;
> > +}
> > +
> > +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_prepare_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:
> > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks)
>
> look like it should be
> reset_control_assert(i3c->tresetn);
You are completely right.
Will fix this in v3.
Kind Regards,
Tommaso
>
> Frank
>
> > +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 renesas_i3c_config empty_i3c_config = {
> > };
> >
> > @@ -1420,6 +1504,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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 4/4] i3c: renesas: Add suspend/resume support
@ 2025-12-30 16:31 ` Tommaso Merciai
0 siblings, 0 replies; 26+ messages in thread
From: Tommaso Merciai @ 2025-12-30 16:31 UTC (permalink / raw)
To: Frank Li
Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Wolfram Sang,
Alexandre Belloni, Philipp Zabel, linux-i3c, linux-kernel
Hi Frank,
Thanks for your review!
On Tue, Dec 30, 2025 at 10:52:03AM -0500, Frank Li wrote:
> On Tue, Dec 30, 2025 at 11:39:39AM +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>
> > ---
> > 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().
> > - Updated commit body accordingly.
> >
> > drivers/i3c/master/renesas-i3c.c | 85 ++++++++++++++++++++++++++++++++
> > 1 file changed, 85 insertions(+)
> >
> > diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> > index 4f076c372b36..f089bbf19fa5 100644
> > --- a/drivers/i3c/master/renesas-i3c.c
> > +++ b/drivers/i3c/master/renesas-i3c.c
> > @@ -254,16 +254,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[3];
> > u8 num_clks;
> > struct reset_control *presetn;
> > struct reset_control *tresetn;
> > + u8 refclk_div;
> > };
> >
> > struct renesas_i3c_i2c_dev_data {
> > @@ -615,6 +618,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);
> > @@ -623,6 +627,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));
> > @@ -1390,6 +1395,13 @@ 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);
>
> nit: can you move it to previous line to reduce line number
Ack.
>
> > + if (!i3c->DATBASn)
> > + return -ENOMEM;
> > +
> > return i3c_master_register(&i3c->base, &pdev->dev, &renesas_i3c_ops, false);
> > }
> >
> > @@ -1400,6 +1412,78 @@ 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)
> > + return ret;
> > +
> > + ret = reset_control_assert(i3c->tresetn);
> > + if (ret) {
> > + reset_control_deassert(i3c->presetn);
> > + return ret;
> > + }
> > +
> > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks);
> > +
> > + return 0;
> > +}
> > +
> > +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_prepare_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:
> > + clk_bulk_disable_unprepare(i3c->num_clks, i3c->clks)
>
> look like it should be
> reset_control_assert(i3c->tresetn);
You are completely right.
Will fix this in v3.
Kind Regards,
Tommaso
>
> Frank
>
> > +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 renesas_i3c_config empty_i3c_config = {
> > };
> >
> > @@ -1420,6 +1504,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
> >
^ permalink raw reply [flat|nested] 26+ messages in thread