[PATCH 06/17] i3c: renesas: Reset the controller on resume
Claudiu Beznea
claudiu.beznea at kernel.org
Sat May 23 03:24:15 PDT 2026
On 5/22/26 22:15, Frank Li wrote:
> On Fri, May 22, 2026 at 01:18:04PM +0300, Claudiu Beznea wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj at bp.renesas.com>
>>
>> Reset the controller on resume after enabling the clocks to follow the
>> same sequence as in probe and avoid potential ordering related failures.
>>
>> Fixes: e7218986319b ("i3c: renesas: Add suspend/resume support")
>> Cc: stable at vger.kernel.org
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj at bp.renesas.com>
>> ---
>
> Can you move these similar stuff to one helper function to avoid duplicate
> efforts later?
If you are talking about moving also the control of the reset signals in the
renesas_i3c_reset() I can do that, but, FMPOV, it will complicate the code,
especially the initialization and failure paths (see the above diff built on top
of this series).
Moving the reset de-assert in the renesas_i3c_reset() will involve calling
functions to assert back the resets in case of failure. FMPOV, that is a bit
unbalanced (wrt the way the code looks) because we are calling deassert in one
function and assert in another function. It is a bit difficult to follow.
Please see the above diff and let me know your thoughts.
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 3b9807a89b54..5f45a024aa54 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -255,8 +255,7 @@ struct renesas_i3c_xferqueue {
struct renesas_i3c {
void __iomem *regs;
struct clk *tclk;
- struct reset_control *presetn;
- struct reset_control *tresetn;
+ struct reset_control_bulk_data *resets;
struct device *dev;
int *irqs;
struct renesas_i3c_xferqueue xferqueue;
@@ -264,6 +263,7 @@ struct renesas_i3c {
unsigned long rate;
unsigned int num_irqs;
enum i3c_internal_state internal_state;
+ u32 num_resets;
u32 free_pos;
u32 dyn_addr;
u32 i2c_STDBR;
@@ -492,16 +492,28 @@ static int renesas_i3c_reset(struct renesas_i3c *i3c)
u32 val;
int ret;
+ ret = reset_control_bulk_deassert(i3c->num_resets, i3c->resets);
+ if (ret)
+ return ret;
+
PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(i3c->dev, pm);
ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
if (ret)
- return ret;
+ goto assert;
renesas_writel(i3c->regs, BCTL, 0);
renesas_set_bit(i3c->regs, RSTCTL, RSTCTL_RI3CRST);
- return read_poll_timeout(renesas_readl, val, !(val & RSTCTL_RI3CRST),
- 0, 1000, false, i3c->regs, RSTCTL);
+ ret = read_poll_timeout(renesas_readl, val, !(val & RSTCTL_RI3CRST),
+ 0, 1000, false, i3c->regs, RSTCTL);
+ if (ret)
+ goto assert;
+
+ return 0;
+
+assert:
+ reset_control_bulk_assert(i3c->num_resets, i3c->resets);
+ return ret;
}
static void renesas_i3c_hw_init(struct renesas_i3c *i3c)
@@ -1430,11 +1442,20 @@ static const struct renesas_i3c_irq_desc
renesas_i3c_irqs[] = {
{ .name = "nack", .isr = renesas_i3c_tend_isr, .desc = "i3c-nack" },
};
+static const char * const renesas_i3c_resets[] = { "tresetn", "presetn" };
+
static void renesas_i3c_dont_use_autosuspend(void *data)
{
pm_runtime_dont_use_autosuspend(data);
}
+static void renesas_i3c_resets_assert(void *data)
+{
+ struct renesas_i3c *i3c = data;
+
+ reset_control_bulk_assert(i3c->num_resets, i3c->resets);
+}
+
static int renesas_i3c_probe(struct platform_device *pdev)
{
struct renesas_i3c *i3c;
@@ -1464,15 +1485,20 @@ static int renesas_i3c_probe(struct platform_device *pdev)
if (ret)
return ret;
- 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");
+ i3c->num_resets = ARRAY_SIZE(renesas_i3c_resets);
+ i3c->resets = devm_kmalloc_array(&pdev->dev, i3c->num_resets,
+ sizeof(*i3c->resets), GFP_KERNEL);
+ if (!i3c->resets)
+ return -ENOMEM;
- 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");
+ for (unsigned int i = 0; i < i3c->num_resets; i++)
+ i3c->resets[i].id = renesas_i3c_resets[i];
+
+ ret = devm_reset_control_bulk_get_optional_exclusive(&pdev->dev,
+ i3c->num_resets,
+ i3c->resets);
+ if (ret)
+ return ret;
spin_lock_init(&i3c->xferqueue.lock);
INIT_LIST_HEAD(&i3c->xferqueue.list);
@@ -1481,6 +1507,11 @@ static int renesas_i3c_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /* Add devm action for resets deasserted in renesas_i3c_reset(). */
+ ret = devm_add_action_or_reset(&pdev->dev, renesas_i3c_resets_assert, NULL);
+ if (ret)
+ return ret;
+
i3c->num_irqs = ARRAY_SIZE(renesas_i3c_irqs);
i3c->irqs = devm_kcalloc(&pdev->dev, i3c->num_irqs, sizeof(*i3c->irqs),
GFP_KERNEL);
if (!i3c->irqs)
@@ -1523,15 +1554,11 @@ static void renesas_i3c_remove(struct platform_device *pdev)
static int renesas_i3c_suspend(struct device *dev)
{
struct renesas_i3c *i3c = dev_get_drvdata(dev);
- struct reset_control_bulk_data resets[] = {
- { .rstc = i3c->presetn },
- { .rstc = i3c->tresetn },
- };
int ret;
i2c_mark_adapter_suspended(&i3c->base.i2c);
- ret = reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
+ ret = reset_control_bulk_assert(i3c->num_resets, i3c->resets);
if (ret)
goto err_mark_resumed;
@@ -1542,7 +1569,7 @@ static int renesas_i3c_suspend(struct device *dev)
return 0;
err_resets_deassert:
- reset_control_bulk_deassert(ARRAY_SIZE(resets), resets);
+ reset_control_bulk_deassert(i3c->num_resets, i3c->resets);
err_mark_resumed:
i2c_mark_adapter_resumed(&i3c->base.i2c);
@@ -1552,23 +1579,15 @@ static int renesas_i3c_suspend(struct device *dev)
static int renesas_i3c_resume(struct device *dev)
{
struct renesas_i3c *i3c = dev_get_drvdata(dev);
- struct reset_control_bulk_data resets[] = {
- { .rstc = i3c->presetn },
- { .rstc = i3c->tresetn },
- };
int ret;
ret = pm_runtime_force_resume(dev);
if (ret)
return ret;
- ret = reset_control_bulk_deassert(ARRAY_SIZE(resets), resets);
- if (ret)
- return ret;
-
ret = renesas_i3c_reset(i3c);
if (ret)
- goto err_resets_asserted;
+ return ret;
PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(i3c->dev, pm);
ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
@@ -1607,7 +1626,7 @@ static int renesas_i3c_resume(struct device *dev)
* if a runtime path is triggered after a failed resume). Checked on
* RZ/G3S.
*/
- reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
+ reset_control_bulk_assert(i3c->num_resets, i3c->resets);
return ret;
}
--
Thank you,
Claudiu
More information about the linux-i3c
mailing list