[PATCH 1/2] i3c: master: svc: change ENXIO to EAGAIN when IBI occurs during start frame

[PATCH 1/2] i3c: master: svc: change ENXIO to EAGAIN when IBI occurs during start frame

[Frank Li]

svc_i3c_master_xfer() returns error ENXIO if an In-Band Interrupt (IBI)
occurs when the host starts the frame.

Change error code to EAGAIN to inform the client driver that this
situation has occurred and to try again sometime later.

Fixes: 5e5e3c92e748 ("i3c: master: svc: fix wrong data return when IBI happen during start frame")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/i3c/master/svc-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 5ee4db68988e2..a2298ab460a37 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1080,7 +1080,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
 	 * and yield the above events handler.
 	 */
 	if (SVC_I3C_MSTATUS_IBIWON(reg)) {
-		ret = -ENXIO;
+		ret = -EAGAIN;
 		*actual_len = 0;
 		goto emit_stop;
 	}
-- 
2.34.1

[PATCH 2/2] i3c: master: svc: fix invalidate IBI type and miss call client IBI handler

[Frank Li]

In an In-Band Interrupt (IBI) handle, the code logic is as follows:

1: writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI | SVC_I3C_MCTRL_IBIRESP_AUTO,
	  master->regs + SVC_I3C_MCTRL);

2: ret = readl_relaxed_poll_timeout(master->regs + SVC_I3C_MSTATUS, val,
                                    SVC_I3C_MSTATUS_IBIWON(val), 0, 1000);
	...
3: ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
   ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);

SVC_I3C_MSTATUS_IBIWON may be set before step 1. Thus, step 2 will return
immediately, and the I3C controller has not sent out the 9th SCL yet.
Consequently, ibitype and ibiaddr are 0, resulting in an unknown IBI type
occurrence and missing call I3C client driver's IBI handler.

A typical case is that SVC_I3C_MSTATUS_IBIWON is set when an IBI occurs
during the controller send start frame in svc_i3c_master_xfer().

Clear SVC_I3C_MSTATUS_IBIWON before issue SVC_I3C_MCTRL_REQUEST_AUTO_IBI
to fix this issue.

Cc: stable@vger.kernel.org
Fixes: 5e5e3c92e748 ("i3c: master: svc: fix wrong data return when IBI happen during start frame")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/i3c/master/svc-i3c-master.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index a2298ab460a37..3bfe8e694f840 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -415,6 +415,9 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
 	int ret;
 
 	mutex_lock(&master->lock);
+	/* Clear the interrupt status */
+	writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS);
+
 	/* Acknowledge the incoming interrupt with the AUTOIBI mechanism */
 	writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI |
 	       SVC_I3C_MCTRL_IBIRESP_AUTO,
@@ -429,9 +432,6 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
 		goto reenable_ibis;
 	}
 
-	/* Clear the interrupt status */
-	writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS);
-
 	status = readl(master->regs + SVC_I3C_MSTATUS);
 	ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
 	ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);
-- 
2.34.1

Re: [PATCH 1/2] i3c: master: svc: change ENXIO to EAGAIN when IBI occurs during start frame

[Miquel Raynal]

Hi Frank,

Frank.Li@nxp.com wrote on Wed, 24 Apr 2024 15:00:29 -0400:

> svc_i3c_master_xfer() returns error ENXIO if an In-Band Interrupt (IBI)
> occurs when the host starts the frame.
> 
> Change error code to EAGAIN to inform the client driver that this
> situation has occurred and to try again sometime later.

This changes slightly the user API, but feels legitimate at the same
time. Maybe a comment somewhere in the i3c headers to clarify this
possibility might be welcome?

> Fixes: 5e5e3c92e748 ("i3c: master: svc: fix wrong data return when IBI happen during start frame")
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/i3c/master/svc-i3c-master.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 5ee4db68988e2..a2298ab460a37 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -1080,7 +1080,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
>  	 * and yield the above events handler.
>  	 */
>  	if (SVC_I3C_MSTATUS_IBIWON(reg)) {
> -		ret = -ENXIO;
> +		ret = -EAGAIN;
>  		*actual_len = 0;
>  		goto emit_stop;
>  	}


Thanks,
Miquèl

Re: [PATCH 2/2] i3c: master: svc: fix invalidate IBI type and miss call client IBI handler

[Miquel Raynal]

Hi Frank,

Frank.Li@nxp.com wrote on Wed, 24 Apr 2024 15:00:30 -0400:

> In an In-Band Interrupt (IBI) handle, the code logic is as follows:
> 
> 1: writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI | SVC_I3C_MCTRL_IBIRESP_AUTO,
> 	  master->regs + SVC_I3C_MCTRL);
> 
> 2: ret = readl_relaxed_poll_timeout(master->regs + SVC_I3C_MSTATUS, val,
>                                     SVC_I3C_MSTATUS_IBIWON(val), 0, 1000);
> 	...
> 3: ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
>    ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);
> 
> SVC_I3C_MSTATUS_IBIWON may be set before step 1. Thus, step 2 will return
> immediately, and the I3C controller has not sent out the 9th SCL yet.
> Consequently, ibitype and ibiaddr are 0, resulting in an unknown IBI type
> occurrence and missing call I3C client driver's IBI handler.
> 
> A typical case is that SVC_I3C_MSTATUS_IBIWON is set when an IBI occurs
> during the controller send start frame in svc_i3c_master_xfer().
> 
> Clear SVC_I3C_MSTATUS_IBIWON before issue SVC_I3C_MCTRL_REQUEST_AUTO_IBI
> to fix this issue.
> 
> Cc: stable@vger.kernel.org
> Fixes: 5e5e3c92e748 ("i3c: master: svc: fix wrong data return when IBI happen during start frame")
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/i3c/master/svc-i3c-master.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index a2298ab460a37..3bfe8e694f840 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -415,6 +415,9 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
>  	int ret;
>  
>  	mutex_lock(&master->lock);
> +	/* Clear the interrupt status */

Could you improve the comment to explain why this is needed here?

Otherwise seems okay, but I don't have any strong feelings here.

> +	writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS);
> +
>  	/* Acknowledge the incoming interrupt with the AUTOIBI mechanism */
>  	writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI |
>  	       SVC_I3C_MCTRL_IBIRESP_AUTO,
> @@ -429,9 +432,6 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
>  		goto reenable_ibis;
>  	}
>  
> -	/* Clear the interrupt status */
> -	writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS);
> -
>  	status = readl(master->regs + SVC_I3C_MSTATUS);
>  	ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
>  	ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);

Thanks,
Miquèl