[PATCH v2 1/2] i3c: master: fixes i3c bus driver probe failure if no i3c device attached

[PATCH v2 1/2] i3c: master: fixes i3c bus driver probe failure if no i3c device attached

[Frank Li]

In i3c_master_bus_init()
{	...
	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
	if (ret && ret != I3C_ERROR_M2)
			  ^^^ // it is enum i3c_error_code
	...
}

In dw-i3c-master.c implementation:
dw_i3c_ccc_set()
{	...
	ret = xfer->ret;
	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
		ccc->err = I3C_ERROR_M2;

	dw_i3c_master_free_xfer(xfer);

	return ret;
}

Return enum i3c_error_code when error happen in i3c_master_rstdaa_locked().

Cc: stable@vger.kernel.org
Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v1 to v2:
    - cc stable

 drivers/i3c/master.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 08aeb69a78003..00a82f3ab9ac0 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -783,6 +783,9 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
 	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 	i3c_ccc_cmd_dest_cleanup(&dest);
 
+	if (ret)
+		ret = cmd.err;
+
 	return ret;
 }
 
-- 
2.34.1

[PATCH v2 2/2] i3c: master: svc: fix probe failure when no i3c device exist

[Frank Li]

I3C masters are expected to support hot-join. This means at initialization
time we might not yet discover any device and this should not be treated
as a fatal error.

During the DAA procedure which happens at probe time, if no device has
joined, all CCC will be NACKed (from a bus perspective). This leads to an
early return with an error code which fails the probe of the master.

Let's avoid this by just telling the core through an I3C_ERROR_M2
return command code that no device was discovered, which is a valid
situation. This way the master will no longer bail out and fail to probe
for a wrong reason.

Cc: stable@vger.kernel.org
Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v1 to v2:
    - rewrite commit message
    - cc stable
    - add empty line in daa()

 drivers/i3c/master/svc-i3c-master.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 770b40e28015e..cf932ee056ef9 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -789,6 +789,10 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
 				 */
 				break;
 			} else if (SVC_I3C_MSTATUS_NACKED(reg)) {
+				/* No I3C devices attached */
+				if (dev_nb == 0)
+					break;
+
 				/*
 				 * A slave device nacked the address, this is
 				 * allowed only once, DAA will be stopped and
@@ -1263,11 +1267,17 @@ static int svc_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
 {
 	struct svc_i3c_master *master = to_svc_i3c_master(m);
 	bool broadcast = cmd->id < 0x80;
+	int ret;
 
 	if (broadcast)
-		return svc_i3c_master_send_bdcast_ccc_cmd(master, cmd);
+		ret = svc_i3c_master_send_bdcast_ccc_cmd(master, cmd);
 	else
-		return svc_i3c_master_send_direct_ccc_cmd(master, cmd);
+		ret = svc_i3c_master_send_direct_ccc_cmd(master, cmd);
+
+	if (ret)
+		cmd->err = I3C_ERROR_M2;
+
+	return ret;
 }
 
 static int svc_i3c_master_priv_xfers(struct i3c_dev_desc *dev,
-- 
2.34.1

Re: [PATCH v2 1/2] i3c: master: fixes i3c bus driver probe failure if no i3c device attached

[Miquel Raynal]

Hi Frank,

Frank.Li@nxp.com wrote on Wed, 30 Aug 2023 10:17:26 -0400:

> In i3c_master_bus_init()
> {	...
> 	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> 	if (ret && ret != I3C_ERROR_M2)
> 			  ^^^ // it is enum i3c_error_code
> 	...
> }
> 
> In dw-i3c-master.c implementation:
> dw_i3c_ccc_set()
> {	...
> 	ret = xfer->ret;
> 	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> 		ccc->err = I3C_ERROR_M2;
> 
> 	dw_i3c_master_free_xfer(xfer);
> 
> 	return ret;
> }
> 
> Return enum i3c_error_code when error happen in i3c_master_rstdaa_locked().

I am sorry but the commit log needs to be worked on.

> 
> Cc: stable@vger.kernel.org
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 
> Notes:
>     Change from v1 to v2:
>     - cc stable
> 
>  drivers/i3c/master.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 08aeb69a78003..00a82f3ab9ac0 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -783,6 +783,9 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	i3c_ccc_cmd_dest_cleanup(&dest);
>  
> +	if (ret)
> +		ret = cmd.err;

Shouldn't this happen in i3c_master_send_ccc_cmd_locked()?

> +
>  	return ret;
>  }
>  


Thanks,
Miquèl

Re: [PATCH v2 1/2] i3c: master: fixes i3c bus driver probe failure if no i3c device attached

[Frank Li]

On Wed, Aug 30, 2023 at 05:53:55PM +0200, Miquel Raynal wrote:
> Hi Frank,
> 
> Frank.Li@nxp.com wrote on Wed, 30 Aug 2023 10:17:26 -0400:
> 
> > In i3c_master_bus_init()
> > {	...
> > 	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> > 	if (ret && ret != I3C_ERROR_M2)
> > 			  ^^^ // it is enum i3c_error_code
> > 	...
> > }
> > 
> > In dw-i3c-master.c implementation:
> > dw_i3c_ccc_set()
> > {	...
> > 	ret = xfer->ret;
> > 	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> > 		ccc->err = I3C_ERROR_M2;
> > 
> > 	dw_i3c_master_free_xfer(xfer);
> > 
> > 	return ret;
> > }
> > 
> > Return enum i3c_error_code when error happen in i3c_master_rstdaa_locked().
> 
> I am sorry but the commit log needs to be worked on.
> 
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 
> > Notes:
> >     Change from v1 to v2:
> >     - cc stable
> > 
> >  drivers/i3c/master.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 08aeb69a78003..00a82f3ab9ac0 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -783,6 +783,9 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
> >  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> >  	i3c_ccc_cmd_dest_cleanup(&dest);
> >  
> > +	if (ret)
> > +		ret = cmd.err;
> 
> Shouldn't this happen in i3c_master_send_ccc_cmd_locked()?

Sorry, This patch is not necesary at all.
i3c_master_send_ccc_cmd_locked() already handled it.
Only need second patch. You can discard this patch.

Do you need me send 2nd patch only? Or you can just pick 2nd one?

> 
> > +
> >  	return ret;
> >  }
> >  
> 
> 
> Thanks,
> Miquèl

Re: [PATCH v2 1/2] i3c: master: fixes i3c bus driver probe failure if no i3c device attached

[Miquel Raynal]

Hi Frank,

Frank.li@nxp.com wrote on Wed, 30 Aug 2023 12:32:28 -0400:

> On Wed, Aug 30, 2023 at 05:53:55PM +0200, Miquel Raynal wrote:
> > Hi Frank,
> > 
> > Frank.Li@nxp.com wrote on Wed, 30 Aug 2023 10:17:26 -0400:
> >   
> > > In i3c_master_bus_init()
> > > {	...
> > > 	ret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
> > > 	if (ret && ret != I3C_ERROR_M2)
> > > 			  ^^^ // it is enum i3c_error_code
> > > 	...
> > > }
> > > 
> > > In dw-i3c-master.c implementation:
> > > dw_i3c_ccc_set()
> > > {	...
> > > 	ret = xfer->ret;
> > > 	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> > > 		ccc->err = I3C_ERROR_M2;
> > > 
> > > 	dw_i3c_master_free_xfer(xfer);
> > > 
> > > 	return ret;
> > > }
> > > 
> > > Return enum i3c_error_code when error happen in i3c_master_rstdaa_locked().  
> > 
> > I am sorry but the commit log needs to be worked on.
> >   
> > > 
> > > Cc: stable@vger.kernel.org
> > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > 
> > > Notes:
> > >     Change from v1 to v2:
> > >     - cc stable
> > > 
> > >  drivers/i3c/master.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > > index 08aeb69a78003..00a82f3ab9ac0 100644
> > > --- a/drivers/i3c/master.c
> > > +++ b/drivers/i3c/master.c
> > > @@ -783,6 +783,9 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
> > >  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> > >  	i3c_ccc_cmd_dest_cleanup(&dest);
> > >  
> > > +	if (ret)
> > > +		ret = cmd.err;  
> > 
> > Shouldn't this happen in i3c_master_send_ccc_cmd_locked()?  
> 
> Sorry, This patch is not necesary at all.
> i3c_master_send_ccc_cmd_locked() already handled it.
> Only need second patch. You can discard this patch.
> 
> Do you need me send 2nd patch only? Or you can just pick 2nd one?

It is quite unclear what you are finally trying to do. Please send a
proper v3, with a changelog.

> 
> >   
> > > +
> > >  	return ret;
> > >  }
> > >    
> > 
> > 
> > Thanks,
> > Miquèl  


Thanks,
Miquèl