diff mbox

[RFC,1/3] i2c: add 'actual' field to struct i2c_msg

Message ID 1340967927-27354-2-git-send-email-shubhrajyoti@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shubhrajyoti Datta June 29, 2012, 11:05 a.m. UTC
In case of a NACK, it's wise to tell our clients
drivers about how many bytes were actually transferred.

Support this by adding an extra field to the struct
i2c_msg which gets incremented the amount of bytes
actually transferred.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 include/linux/i2c.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Comments

Jean Delvare June 29, 2012, 12:33 p.m. UTC | #1
On Fri, 29 Jun 2012 16:35:25 +0530, Shubhrajyoti D wrote:
> In case of a NACK, it's wise to tell our clients
> drivers about how many bytes were actually transferred.
> 
> Support this by adding an extra field to the struct
> i2c_msg which gets incremented the amount of bytes
> actually transferred.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  include/linux/i2c.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index ddfa041..0cb6b83 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -547,6 +547,7 @@ struct i2c_msg {
>  #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
>  #define I2C_M_RECV_LEN		0x0400	/* length will be first received byte */
>  	__u16 len;		/* msg length				*/
> +	__u16 actual;		/* actual bytes transferred             */
>  	__u8 *buf;		/* pointer to msg data			*/
>  };
>  

David Brownell discussed this a long time ago:
http://marc.info/?l=linux-i2c&m=121103523020494&w=2

This structure is exported to user-space through i2c-dev, so changing
it is tough if possible at all. You first have to ensure that the
change doesn't alter the structure size. In other words, the field you
add must fit in a padding hole the structure had previously. This
appears to be the case here, but I'm not sure how to prove it for all
supported architectures.

Then, as you are inserting a field in the middle of the structure, you
must ensure that every initialization follows C99. And you have to do
that _before_ the above change, not after as you did in patch 3/3. This
needs to be done in both the whole kernel tree and as many user-space
applications as possible. At least i2c-tools is clean, but there must
be other (possibly unpublished) tools using this API, which your change
may break. We can't fix them all, but at least we will have to document
the change clearly and explain how to fix in case of breakage. The i2c
wiki might be the right place for that.
Jean Delvare July 2, 2012, 1:27 p.m. UTC | #2
On Fri, 29 Jun 2012 16:35:25 +0530, Shubhrajyoti D wrote:
> In case of a NACK, it's wise to tell our clients
> drivers about how many bytes were actually transferred.
> 
> Support this by adding an extra field to the struct
> i2c_msg which gets incremented the amount of bytes
> actually transferred.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  include/linux/i2c.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index ddfa041..0cb6b83 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -547,6 +547,7 @@ struct i2c_msg {
>  #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
>  #define I2C_M_RECV_LEN		0x0400	/* length will be first received byte */
>  	__u16 len;		/* msg length				*/
> +	__u16 actual;		/* actual bytes transferred             */
>  	__u8 *buf;		/* pointer to msg data			*/
>  };
>  

Oh, as a side note: I don't much like the name "actual". It's not
really clear what it means. I don't have a perfect name to propose as a
replacement, but maybe "transferred" or even just "done" would do.
diff mbox

Patch

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index ddfa041..0cb6b83 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -547,6 +547,7 @@  struct i2c_msg {
 #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_RECV_LEN		0x0400	/* length will be first received byte */
 	__u16 len;		/* msg length				*/
+	__u16 actual;		/* actual bytes transferred             */
 	__u8 *buf;		/* pointer to msg data			*/
 };