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