Re: [PATCH v4 2/3] i3c: master: dw: Map CCC hardware errors to I3C M0/M2

From: "NG, TZE YEE" <tze.yee.ng@altera.com>
To: Frank Li <Frank.li@oss.nxp.com>,
	"sashiko-reviews@lists.linux.dev"
	<sashiko-reviews@lists.linux.dev>
Cc: "imx@lists.linux.dev" <imx@lists.linux.dev>,
	"Frank.Li@kernel.org" <Frank.Li@kernel.org>,
	"linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>
Subject: Re: [PATCH v4 2/3] i3c: master: dw: Map CCC hardware errors to I3C M0/M2
Date: Wed, 1 Jul 2026 02:45:48 +0000	[thread overview]
Message-ID: <3df7989e-cfdf-499c-a8fc-f8826659bcb7@altera.com> (raw)
In-Reply-To: <akP3mEJMkWHycvZr@lizhi-Precision-Tower-5810>

On 1/7/2026 1:06 am, Frank Li wrote:
> On Tue, Jun 30, 2026 at 01:32:38PM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] Undocumented behavioral change and silent bug fix
>> --
>>
>> commit 5afd5fbee8dc926110284bae98310b5970134131
>> Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>>
>> i3c: master: dw: Map CCC hardware errors to I3C M0/M2
>>
>> This commit updates the DesignWare I3C master driver to map specific
>> response queue errors to standard I3C error codes M0 and M2. It also returns
>> -EIO for address NACKs and introduces dw_i3c_ccc_map_err() to centralize
>> error mapping.
>>
>>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>>> index e659ede79fc03..27d2025fcf041 100644
>>> --- a/drivers/i3c/master/dw-i3c-master.c
>>> +++ b/drivers/i3c/master/dw-i3c-master.c
>>
>> [ ... ]
>>
>>> @@ -743,10 +760,7 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>>>
>>>     ret = xfer->ret;
>>>     cmd = &xfer->cmds[0];
>>> -   if (!ret)
>>> -           ccc->dests[0].payload.actual_len = cmd->rx_len;
>>> -   if (cmd->error == RESPONSE_ERROR_IBA_NACK)
>>> -           ccc->err = I3C_ERROR_M2;
>>> +   ccc->err = dw_i3c_ccc_map_err(cmd->error);
>>
>> [Severity: Medium]
>> Is this an unrelated bug fix? The commit message mentions mapping hardware
>> errors to I3C M0/M2, but it does not mention removing the assignment of
>> actual_len from the transmit operation here.
>
> Yes, don't touch .payload.actual_len in this patch
>
>
>>
>>>
>>>     return ret;
>>>   }
>>
>> [ ... ]
>>
>>> @@ -783,8 +799,10 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>>>             dw_i3c_master_dequeue_xfer(master, xfer);
>>>
>>>     ret = xfer->ret;
>>> -   if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
>>> -           ccc->err = I3C_ERROR_M2;
>>> +   cmd = &xfer->cmds[0];
>>> +   ccc->err = dw_i3c_ccc_map_err(cmd->error);
>>> +   if (!ret)
>>> +           ccc->dests[0].payload.actual_len = cmd->rx_len;
>>
>> [Severity: Medium]
>> Does this silently add the actual_len assignment to the receive operation?
>
> The same here.
>
> update actual_len should be new patch.
>
> Frank
>

Hi Frank,

Thanks for review. I will move all changes to update actual_len into a
new patch.

Tze Yee

>>
>> Since moving this assignment from dw_i3c_ccc_set() to dw_i3c_ccc_get() is
>> not documented in the commit message, should it be split into a separate
>> fix or explicitly explained?
>>
>>>
>>>     return ret;
>>>   }
>>
>> --
>> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824628.git.tze.yee.ng@altera.com?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c