diff mbox

[01/18] i2c: guarantee that I2C_M_RD will be 0x0001 forever

Message ID 1459709101-24462-2-git-send-email-wsa@the-dreams.de
State Accepted
Headers show

Commit Message

Wolfram Sang April 3, 2016, 6:44 p.m. UTC
There is code out there in user space and kernel space which relies on
I2C_M_RD being bit 0 to simplify their bit operations. Add a comment to
make sure this will never break. Do proper sorting of the defines while
we are here.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 include/uapi/linux/i2c.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Andy Shevchenko April 4, 2016, 10:26 a.m. UTC | #1
On Sun, 2016-04-03 at 20:44 +0200, Wolfram Sang wrote:
> There is code out there in user space and kernel space which relies
> on
> I2C_M_RD being bit 0 to simplify their bit operations. Add a comment
> to
> make sure this will never break. Do proper sorting of the defines
> while
> we are here.

Didn't see a comment as stated though.

> 
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
>  include/uapi/linux/i2c.h | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
> index b0a7dd61eb353a..adcbef4bff610a 100644
> --- a/include/uapi/linux/i2c.h
> +++ b/include/uapi/linux/i2c.h
> @@ -68,14 +68,15 @@
>  struct i2c_msg {
>  	__u16 addr;	/* slave address			*
> /
>  	__u16 flags;
> -#define I2C_M_TEN		0x0010	/* this is a ten bit
> chip address */
>  #define I2C_M_RD		0x0001	/* read data, from
> slave to master */
> -#define I2C_M_STOP		0x8000	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> -#define I2C_M_NOSTART		0x4000	/* if
> I2C_FUNC_NOSTART */
> -#define I2C_M_REV_DIR_ADDR	0x2000	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> -#define I2C_M_IGNORE_NAK	0x1000	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> -#define I2C_M_NO_RD_ACK		0x0800	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> +					/* I2C_M_RD is guaranteed to
> be 0x0001! */
> +#define I2C_M_TEN		0x0010	/* this is a ten bit
> chip address */
>  #define I2C_M_RECV_LEN		0x0400	/* length will
> be first received byte */
> +#define I2C_M_NO_RD_ACK		0x0800	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_IGNORE_NAK	0x1000	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_REV_DIR_ADDR	0x2000	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NOSTART		0x4000	/* if
> I2C_FUNC_NOSTART */
> +#define I2C_M_STOP		0x8000	/* if
> I2C_FUNC_PROTOCOL_MANGLING */
>  	__u16 len;		/* msg length			
> 	*/
>  	__u8 *buf;		/* pointer to msg data		
> 	*/
>  };
Andy Shevchenko April 4, 2016, 10:29 a.m. UTC | #2
On Mon, 2016-04-04 at 13:26 +0300, Andy Shevchenko wrote:
> On Sun, 2016-04-03 at 20:44 +0200, Wolfram Sang wrote:

> > 

> > There is code out there in user space and kernel space which relies

> > on

> > I2C_M_RD being bit 0 to simplify their bit operations. Add a

> > comment

> > to

> > make sure this will never break. Do proper sorting of the defines

> > while

> > we are here.

> Didn't see a comment as stated though.


Ah, overlooked it. Anyway, perhaps worth to mention under
Documentation. What do you think?

> 

> > 

> > 

> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

> > ---

> >  include/uapi/linux/i2c.h | 13 +++++++------

> >  1 file changed, 7 insertions(+), 6 deletions(-)

> > 

> > diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h

> > index b0a7dd61eb353a..adcbef4bff610a 100644

> > --- a/include/uapi/linux/i2c.h

> > +++ b/include/uapi/linux/i2c.h

> > @@ -68,14 +68,15 @@

> >  struct i2c_msg {

> >  	__u16 addr;	/* slave address			

> > *

> > /

> >  	__u16 flags;

> > -#define I2C_M_TEN		0x0010	/* this is a ten

> > bit

> > chip address */

> >  #define I2C_M_RD		0x0001	/* read data, from

> > slave to master */

> > -#define I2C_M_STOP		0x8000	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > -#define I2C_M_NOSTART		0x4000	/* if

> > I2C_FUNC_NOSTART */

> > -#define I2C_M_REV_DIR_ADDR	0x2000	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > -#define I2C_M_IGNORE_NAK	0x1000	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > -#define I2C_M_NO_RD_ACK		0x0800	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > +					/* I2C_M_RD is guaranteed

> > to

> > be 0x0001! */

> > +#define I2C_M_TEN		0x0010	/* this is a ten

> > bit

> > chip address */

> >  #define I2C_M_RECV_LEN		0x0400	/* length will

> > be first received byte */

> > +#define I2C_M_NO_RD_ACK		0x0800	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > +#define I2C_M_IGNORE_NAK	0x1000	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > +#define I2C_M_REV_DIR_ADDR	0x2000	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> > +#define I2C_M_NOSTART		0x4000	/* if

> > I2C_FUNC_NOSTART */

> > +#define I2C_M_STOP		0x8000	/* if

> > I2C_FUNC_PROTOCOL_MANGLING */

> >  	__u16 len;		/* msg length			

> > 	*/

> >  	__u8 *buf;		/* pointer to msg data		

> > 	*/

> >  };


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Wolfram Sang April 4, 2016, 10:47 a.m. UTC | #3
> Didn't see a comment as stated though.

Yeah, it is hidden in the refactoring :/

 > +					/* I2C_M_RD is guaranteed to  be 0x0001! */

   Wolfram
Wolfram Sang April 11, 2016, 4:54 p.m. UTC | #4
> Ah, overlooked it. Anyway, perhaps worth to mention under
> Documentation. What do you think?

Unsure. We can add it later, I'd say.
diff mbox

Patch

diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index b0a7dd61eb353a..adcbef4bff610a 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -68,14 +68,15 @@ 
 struct i2c_msg {
 	__u16 addr;	/* slave address			*/
 	__u16 flags;
-#define I2C_M_TEN		0x0010	/* this is a ten bit chip address */
 #define I2C_M_RD		0x0001	/* read data, from slave to master */
-#define I2C_M_STOP		0x8000	/* if I2C_FUNC_PROTOCOL_MANGLING */
-#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
-#define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
-#define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
-#define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
+					/* I2C_M_RD is guaranteed to be 0x0001! */
+#define I2C_M_TEN		0x0010	/* this is a ten bit chip address */
 #define I2C_M_RECV_LEN		0x0400	/* length will be first received byte */
+#define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
+#define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
+#define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
+#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
+#define I2C_M_STOP		0x8000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 	__u16 len;		/* msg length				*/
 	__u8 *buf;		/* pointer to msg data			*/
 };