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

"NG, TZE YEE" <[email protected]>
Newsgroups org.infradead.lists.linux-i3c,dev.linux.lists.imx,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
On 1/7/2026 1:06 am, Frank Li wrote:
> On Tue, Jun 30, 2026 at 01:32:38PM +0000, [email protected] 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 <[email protected]>
>>
>> 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/[email protected]?part=2

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.