Re: [PATCH v3] i3c: master: dw-i3c-master: Add HDR-DDR support to Designware Controller.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] i3c: master: dw: broken HDR-DDR direction check in
  dw_i3c_master_i3c_xfers()
--

commit e898c32aed081cceae7f567f3da1c50b825f55c6
Author: Dineshkumar Parameswaran <Dineshkumar.Parameswaran@synopsys.com>

i3c: master: dw-i3c-master: Add HDR-DDR support to Designware Controller.

This commit adds HDR-DDR private transfer support to the Synopsys
DesignWare I3C controller driver.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 34aa8f8e1a0a2..aba27abb79272 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c

[ ... ]

> @@ -995,28 +1006,36 @@ static int dw_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
>  	}
>  
>  	for (i = 0; i < i3c_nxfers; i++) {
> +		u32 rnw_cmd = (mode == I3C_SDR) ? i3c_xfers[i].rnw : i3c_xfers[i].cmd;
> +		bool rnw = dw_cmd_is_read(rnw_cmd, mode);
>  		struct dw_i3c_cmd *cmd = &xfer->cmds[i];
>  
>  		cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(i3c_xfers[i].len) |
>  			COMMAND_PORT_TRANSFER_ARG;
>  
> -		if (i3c_xfers[i].rnw) {
> +		if (rnw) {
>  			cmd->rx_buf = i3c_xfers[i].data.in;
>  			cmd->rx_len = i3c_xfers[i].len;
> -			cmd->cmd_lo = COMMAND_PORT_READ_TRANSFER |
> -				      COMMAND_PORT_SPEED(dev->info.max_read_ds);
> +			cmd->cmd_lo = COMMAND_PORT_READ_TRANSFER;
>  
>  		} else {
>  			cmd->tx_buf = i3c_xfers[i].data.out;
>  			cmd->tx_len = i3c_xfers[i].len;
> -			cmd->cmd_lo =
> -				COMMAND_PORT_SPEED(dev->info.max_write_ds);
>  		}

[Severity: High]
As reported by Frank Li on v2 [1], should the FIFO depth check loop and
the post-transfer length update loop in dw_i3c_master_i3c_xfers() also
use dw_cmd_is_read() instead of testing i3c_xfers[i].rnw directly?

In struct i3c_xfer, rnw and cmd share an anonymous union:

include/linux/i3c/device.h:
struct i3c_xfer {
	union {
		u8 rnw;
		u8 cmd;
	};

For any HDR-DDR write with a non-zero command code (0x01..0x7f),
i3c_xfers[i].rnw is non-zero even though bit 7 (HDR_DDR_CMD_READ) is
clear.  The other two loops in dw_i3c_master_i3c_xfers() still check
if (i3c_xfers[i].rnw), so HDR-DDR writes with cmd in 0x01..0x7f are
counted into nrxwords instead of ntxwords during the datafifodepth check,
and on completion their i3c_xfers[i].len is overwritten with cmd->rx_len
(0).

[1] https://lore.kernel.org/r/aukCf1reB8LWbuzq@lizhi-Precision-Tower-5810

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111021151.58380-1-parame@synopsys.com?part=1

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