Re: [PATCH RFC v3 07/14] i2c: core: implement i2c_transfer_v2() - transfer with detailed reporting

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Nov 05, 2026 at 07:15:28PM +0200, Dmitry Guzman wrote:
> In __i2c_transfer() and i2c_transfer(), add one more parameter - pointer
> to transfer report. In __i2c_transfer(), if the pointer is not NULL,
> use algo->xfer_v2() callback instead of algo->xfer() in order to get the
> detailed transfer report. If the adapter does not have xfer_v2(), return
> -EOPNOTSUPP.
> 
> If the adapter has any quirks that make impossible to transfer requested
> message or messages, put the index of the first such message to the
> transfer report structure.
> 
> If the report is not NULL, algo->xfer_v2() returned an error and there
> are any read messages, trace the read data using `trace_i2c_reply(). If
> the last read message is incomplete (transfer was interrupted in the
> middle of a read message), trace the read portion of data also.
> 
> Rename __i2c_transfer() to __i2c_transfer_v2() and i2c_transfer() to
> i2c_transfer_v2().
> 
> For backward compatibility, add __i2c_transfer() and i2c_transfer() that
> call __i2c_transfer_v2() and i2c_transfer_v2() respectively with null
> report pointer.
> 
> Remove comment "REVISIT the fault reporting model here is weak".

...

>  	if (static_branch_unlikely(&i2c_trace_msg_key)) {
> -		int i;
> -		for (i = 0; i < ret; i++)
> +		int i, n;
> +
> +		if (report)
> +			n = report->msgs_cplt;
> +		else
> +			n = ret;

> +		for (i = 0; i < n; i++)
>  			if (msgs[i].flags & I2C_M_RD)
>  				trace_i2c_reply(adap, &msgs[i], msgs[i].len, i);

Besides missing {}. the iterator is local to the loop, hence

		for (int i = 0; i < n; i++) {
			if (msgs[i].flags & I2C_M_RD)
				trace_i2c_reply(adap, &msgs[i], msgs[i].len, i);
		}

> +		if (report && report->bytes_cplt > 0 && msgs[n].flags & I2C_M_RD)
> +			trace_i2c_reply(adap, &msgs[n], report->bytes_cplt, n);
>  		trace_i2c_result(adap, num, ret);
>  	}

...

> +/**
> + * __i2c_transfer - unlocked flavor of i2c_transfer
> + * @adap: Handle to I2C bus
> + * @msgs: One or more messages to execute before STOP is issued to
> + *	terminate the operation; each message begins with a START.
> + * @num: Number of messages to be executed.

> + * Returns negative errno, else the number of messages executed.

Return section must be last in the kernel-doc (it's documented) and actually be
a such. Always validate with kernel-doc script.

> + * Adapter lock must be held when calling this function. No debug logging
> + * takes place.

This is Context section in kernel-doc for.
And more effectively is to add a lockdep annotation.

> + */
> +int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> +	return __i2c_transfer_v2(adap, msgs, num, NULL);
> +}

-- 
With Best Regards,
Andy Shevchenko