[PATCH v4 16/16] i3c: renesas: Add runtime PM support

[Claudiu Beznea]

On 6/12/26 19:34, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Timeout handler fails to abort hardware transfers because it clears the BCTL_ABT bit instead of setting it.
> 
> Pre-existing issues:
> - [High] Transfer loops do not break on timeout and unconditionally return success, leading to silent failures, race conditions, and memory corruption.
> --
> 
> commit d8976043a71981cb4634aa1e9eafca1113e44ec6
> Author: Claudiu Beznea <claudiu.beznea.uj at bp.renesas.com>
> 
> i3c: renesas: Add runtime PM support
> 
> This commit adds runtime PM support to the Renesas I3C driver, guarding
> subsystem operations with pm_runtime APIs. It also drops explicit clock
> handling, modifies wait_xfer to return unsigned long, and introduces a
> function to clear pending IRQs on transfer aborts.
> 
>> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
>> --- a/drivers/i3c/master/renesas-i3c.c
>> +++ b/drivers/i3c/master/renesas-i3c.c
> 
> [ ... ]
> 
>> @@ -443,7 +442,24 @@ static void renesas_i3c_enqueue_xfer(struct renesas_i3c *i3c, struct renesas_i3c
>>   	}
>>   }
>>   
>> -static void renesas_i3c_wait_xfer(struct renesas_i3c *i3c, struct renesas_i3c_xfer *xfer)
>> +static void renesas_i3c_irqs_mask_and_clear(struct renesas_i3c *i3c)
>> +{
>> +	guard(spinlock_irqsave)(&i3c->xferqueue.lock);
>> +
>> +	/* Disable all the interrupts */
>> +	renesas_writel(i3c->regs, BIE, 0);
>> +	renesas_writel(i3c->regs, NTIE, 0);
>> +
>> +	/* Clear normal transfer status flags. */
>> +	renesas_clear_bit(i3c->regs, NTST, NTST_TDBEF0 | NTST_RDBFF0 | NTST_RSPQFF |
>> +					   NTST_TEF | NTST_TABTF);
>> +	/* Clear bus status flags. */
>> +	renesas_clear_bit(i3c->regs, BST, BST_NACKDF | BST_TENDF | BST_SPCNDDF);
>> +	/* Clear error flags. */
>> +	renesas_clear_bit(i3c->regs, BCTL, BCTL_ABT);
>> +}
> 
> [Severity: High]
> Does this actually abort the hardware transfer? The commit message mentions
> disabling interrupts and clearing pending IRQ status bits when a completion
> timeout occurs.
> 
> However, clearing the BCTL_ABT bit is usually how an abort is acknowledged.
> To actually initiate a hardware abort, the host typically must set the
> BCTL_ABT bit, or explicitly reset the controller.
> 
> By only clearing an already-zero bit, does the controller remain in a busy
> or hung state, potentially corrupting subsequent transfers?

The ABT bit of BCTL has the following meaning according to documentation:

  ABT
0: I3C is running.
1: I3C has aborted a transfer.

And the following explanation:

When set to 1, this bit allows I3C to relinquish control of the I3C Bus before 
completing the currently issued transfer.
In response to an ABORT request, I3C issues the STOP condition on the I3C Bus 
after the complete data byte is transferred or received.

The Driver shall clear the ABT bit to allow operation on the Bus.

Thus, the drive is clearing this bit to allow next I3C transfers to go on.

Thank you,
Claudiu