Re: [PATCH v2 1/6] i2c: qcom-cci: Factor out write command queue programming

[Bryan O'Donoghue]

On 13/10/2026 11:30, Loic Poulain wrote:
> Factor out write-command queueing into a dedicated helper and replace
> the manual byte-to-word packing code with iowrite32_rep().
>   
> The resulting code is easier to follow and will be reused by
> upcoming burst-write support.
> 
> Also add a sanity check on the write buffer length, which should
> already be validated by the caller/I2C-core.
> 
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> ---
>   drivers/i2c/busses/i2c-qcom-cci.c | 41 +++++++++++++++++++++------------------
>   1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index d509dc1c14700d9c199d67df19082bae4f1ae470..3304760411082b72e10e529a2bd66361e100fa06 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -77,6 +77,9 @@
>   #define CCI_I2C_WRITE		9
>   #define CCI_I2C_READ		10
> 
> +#define CCI_I2C_WRITE_WC	3
> +#define CCI_I2C_WRITE_NWORDS(len) DIV_ROUND_UP((len) + 1, sizeof(u32))
> +
>   #define CCI_I2C_REPORT_IRQ_EN	BIT(8)
> 
>   enum {
> @@ -485,42 +488,42 @@ static int cci_i2c_read(struct cci *cci, u16 master,
>   	return 0;
>   }
> 
> +static void cci_i2c_load_write(u8 *buf, u16 len, void __iomem *loadq)
> +{
> +	u32 cmd[CCI_I2C_WRITE_WC] = { };
> +
> +	if (WARN_ON(len >= CCI_I2C_WRITE_WC * sizeof(u32)))
> +		return;

Why not trap this and return the error - cci_i2c_write() has a result code.

> +
> +	cmd[0] = CCI_I2C_WRITE | len << 4;
> +	memcpy((u8 *)cmd + 1, buf, len);
> +	iowrite32_rep(loadq, cmd, CCI_I2C_WRITE_NWORDS(len));
> +}
> +
>   static int cci_i2c_write(struct cci *cci, u16 master,
>   			 u16 addr, u8 *buf, u16 len)
>   {
> -	u8 queue = QUEUE_0;
> -	u8 load[12] = { 0 };
> -	int i = 0, j, ret;
> +	void __iomem *loadq = cci->base + CCI_I2C_Mm_Qn_LOAD_DATA(master, QUEUE_0);
> +	int ret;
>   	u32 val;
> 
>   	/*
>   	 * Call validate queue to make sure queue is empty before starting.
>   	 * This is to avoid overflow / underflow of queue.
>   	 */
> -	ret = cci_validate_queue(cci, master, queue);
> +	ret = cci_validate_queue(cci, master, QUEUE_0);
>   	if (ret < 0)
>   		return ret;
> 
>   	val = CCI_I2C_SET_PARAM | (addr & 0x7f) << 4;
> -	writel(val, cci->base + CCI_I2C_Mm_Qn_LOAD_DATA(master, queue));
> +	writel(val, loadq);
> 
> -	load[i++] = CCI_I2C_WRITE | len << 4;
> -
> -	for (j = 0; j < len; j++)
> -		load[i++] = buf[j];
> -
> -	for (j = 0; j < i; j += 4) {
> -		val = load[j];
> -		val |= load[j + 1] << 8;
> -		val |= load[j + 2] << 16;
> -		val |= load[j + 3] << 24;
> -		writel(val, cci->base + CCI_I2C_Mm_Qn_LOAD_DATA(master, queue));
> -	}
> +	cci_i2c_load_write(buf, len, loadq);
> 
>   	val = CCI_I2C_REPORT | CCI_I2C_REPORT_IRQ_EN;
> -	writel(val, cci->base + CCI_I2C_Mm_Qn_LOAD_DATA(master, queue));
> +	writel(val, loadq);
> 
> -	return cci_run_queue(cci, master, queue);
> +	return cci_run_queue(cci, master, QUEUE_0);
>   }
> 
>   static int cci_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> 
> --
> 2.34.1