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

[Konrad Dybcio]

On 10/16/26 10:00 AM, 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.
> 
> A command can only carry as many data bytes as fit in its words, so
> validate the length before building one, and do so where an error can
> still be returned. The I2C core already rejects a transfer longer than
> the limit advertised through the adapter quirks, so this is only a
> guard against building a command that would corrupt the queue.
> 
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> ---


> +static void cci_i2c_load_write(u8 *buf, u16 len, void __iomem *loadq)
> +{
> +	u32 cmd[CCI_I2C_WRITE_WC] = { };
> +
> +	cmd[0] = CCI_I2C_WRITE | len << 4;
> +	memcpy((u8 *)cmd + 1, buf, len);

LLM says this is no longer big-endian friendly, but BE arm64
is going away anyway so I woouldn't really change it

> +	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;
>  
> +	dev_dbg(cci->dev, "master %d write: len=%u\n", master, len);
> +
> +	/* This condition should be guaranteed by the caller (i2c core) */
> +	if (WARN_ON(len > cci->data->quirks.max_write_len))
> +		return -EOPNOTSUPP;

Likewise LLM suggests this is an odd error to return and I agree EINVAL
could be better

with that:

Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

Konrad