Re: [PATCH AUTOSEL 6.18] i3c: master: svc: Prevent IRQ storm from false SLVSTART on NPCM845

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Inconsistent use of I/O accessors for FIFO accesses, mixing stream and register accessors, which causes data corruption on big-endian systems.
--

commit c282e92f4b93fba4c854d5e9c34c1ac10212811f
Author: Stanley Chu <yschu@nuvoton.com>

    i3c: master: svc: Prevent IRQ storm from false SLVSTART on NPCM845

This commit adds a hardware quirk workaround for the NPCM845 controller.
It explicitly clears the SLVSTART status bit after emitting a STOP
condition in the Master Request IBI handler to prevent an IRQ storm caused
by a spurious SLVSTART.

Link: https://patch.msgid.link/20260413005040.1211107-3-yschu@nuvoton.com

> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 9641e66a4e5f2..e74b22c6f808c 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing
issue in this file regarding I/O accessors for FIFO registers.

In svc_i3c_master_xfer(), does this code mix a stream accessor (writesb)
for bulk data with a register accessor (writel) for the final byte on the
same FIFO?

drivers/i3c/master/svc-i3c-master.c:svc_i3c_master_xfer() {
    ...
    writesb(master->regs + SVC_I3C_MWDATAB1, out, len - 1);
    /* Mark END bit if this is the last byte */
    writel(out[len - 1] | end, master->regs + SVC_I3C_MWDATAB);
    ...
}

Using writel() for the remainder byte performs unwanted byte-swapping on
big-endian platforms, which can corrupt the data sent on the I3C bus.

[Severity: High]
This is also a pre-existing issue, but do the readl() calls in
svc_i3c_master_read() and svc_i3c_master_readb() perform incorrect
byteswapping when reading from the MRDATAB FIFO?

drivers/i3c/master/svc-i3c-master.c:svc_i3c_master_handle_ibi() {
    ...
    readsb(master->regs + SVC_I3C_MRDATAB, buf, count);
    ...
}

drivers/i3c/master/svc-i3c-master.c:svc_i3c_master_read() {
    ...
    in[offset + i] = readl(master->regs + SVC_I3C_MRDATAB);
    ...
}

drivers/i3c/master/svc-i3c-master.c:svc_i3c_master_readb() {
    ...
    dst[i] = readl(master->regs + SVC_I3C_MRDATAB);
    ...
}

Since readsb() is used in svc_i3c_master_handle_ibi() for the same
MRDATAB FIFO, using readl() to read individual bytes into a u8 buffer in
other functions could result in corrupted data on big-endian systems.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-647-sashal@kernel.org?part=1