Re: [PATCH v2 2/3] i3c: dw: Track the transmit and receive buffer depths separately

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] i3c: dw: u8 truncation of rxfifodepth in dw_i3c_common_probe()
--

commit c93cc6843b57993d053db03cc3f7cee25dbd4371
Author: Shubham Patil <shubhamsanjay.patil@amd.com>

i3c: dw: Track the transmit and receive buffer depths separately

This commit splits caps.datafifodepth into caps.txfifodepth and
caps.rxfifodepth, populates caps.rxfifodepth from the QUEUE_SIZE_CAPABILITY
register, and exposes the FIFO capacities via base.tx_fifo_bytes and
base.rx_fifo_bytes.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index d1119f203d1b6..dd5462b129514 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -203,7 +203,10 @@
>  #define BUS_IDLE_TIMING			0xd8
>  #define I3C_VER_ID			0xe0
>  #define I3C_VER_TYPE			0xe4
> -#define EXTENDED_CAPABILITY		0xe8
> +#define QUEUE_SIZE_CAPABILITY		0xe8
> +#define QUEUE_SIZE_CAPABILITY_RX_BUF(x)	(((x) & GENMASK(7, 4)) >> 4)
> +/* Sizes are encoded, 0 meaning 2 DWORDs and every further step doubling it. */
> +#define QUEUE_SIZE_TO_DWORDS(x)		(2U << (x))
>  #define SLAVE_CONFIG			0xec
>  
>  #define DYN_ADDR_LO_MASK GENMASK(4, 0)
> @@ -978,8 +981,8 @@ static int dw_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
>  			ntxwords += DIV_ROUND_UP(i3c_xfers[i].len, 4);
>  	}
>  
> -	if (ntxwords > master->caps.datafifodepth ||
> -	    nrxwords > master->caps.datafifodepth)
> +	if (ntxwords > master->caps.txfifodepth ||
> +	    nrxwords > master->caps.rxfifodepth)
>  		return -EOPNOTSUPP;
>  
>  	struct dw_i3c_xfer *xfer __free(kfree) = dw_i3c_master_alloc_xfer(master, i3c_nxfers);
> @@ -1136,8 +1139,8 @@ static int dw_i3c_master_i2c_xfers(struct i2c_dev_desc *dev,
>  			ntxwords += DIV_ROUND_UP(i2c_xfers[i].len, 4);
>  	}
>  
> -	if (ntxwords > master->caps.datafifodepth ||
> -	    nrxwords > master->caps.datafifodepth)
> +	if (ntxwords > master->caps.txfifodepth ||
> +	    nrxwords > master->caps.rxfifodepth)
>  		return -EOPNOTSUPP;
>  
>  	struct dw_i3c_xfer *xfer __free(kfree) = dw_i3c_master_alloc_xfer(master, i2c_nxfers);
> @@ -1738,7 +1741,14 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>  	master->caps.cmdfifodepth = QUEUE_STATUS_LEVEL_CMD(ret);
>  
>  	ret = readl(master->regs + DATA_BUFFER_STATUS_LEVEL);
> -	master->caps.datafifodepth = DATA_BUFFER_STATUS_LEVEL_TX(ret);
> +	master->caps.txfifodepth = DATA_BUFFER_STATUS_LEVEL_TX(ret);
> +
> +	ret = readl(master->regs + QUEUE_SIZE_CAPABILITY);
> +	master->caps.rxfifodepth =
> +		QUEUE_SIZE_TO_DWORDS(QUEUE_SIZE_CAPABILITY_RX_BUF(ret));
> +
> +	master->base.tx_fifo_bytes = master->caps.txfifodepth * 4;
> +	master->base.rx_fifo_bytes = master->caps.rxfifodepth * 4;
>  
>  	ret = readl(master->regs + DEVICE_ADDR_TABLE_POINTER);
>  	master->datstartaddr = ret;
> diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
> index f5b4491b247a0..e23554bfc5e49 100644
> --- a/drivers/i3c/master/dw-i3c-master.h
> +++ b/drivers/i3c/master/dw-i3c-master.h
> @@ -14,7 +14,8 @@
>  
>  struct dw_i3c_master_caps {
>  	u8 cmdfifodepth;
> -	u8 datafifodepth;
> +	u8 txfifodepth;
> +	u8 rxfifodepth;

[Severity: High]
Can master->caps.rxfifodepth overflow u8 and truncate to 0 in
dw_i3c_common_probe()?

In the DesignWare I3C controller, QUEUE_SIZE_CAPABILITY_RX_BUF(ret) can
return up to 0x7 (256 32-bit words, or 1024 bytes). When that happens,
QUEUE_SIZE_TO_DWORDS(7) evaluates to 256 (0x100), which truncates to 0 when
stored into the u8 rxfifodepth field in struct dw_i3c_master_caps.

With master->caps.rxfifodepth set to 0, master->base.rx_fifo_bytes also
becomes 0 in dw_i3c_common_probe(), and the nrxwords >
master->caps.rxfifodepth check in both dw_i3c_master_i3c_xfers() and
dw_i3c_master_i2c_xfers() will reject every non-empty read transfer with
-EOPNOTSUPP.

Should rxfifodepth (and txfifodepth) in struct dw_i3c_master_caps be u16
instead of u8?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105085425.3418230-1-shubhamsanjay.patil@amd.com?part=2