Message ID | 1459709101-24462-2-git-send-email-wsa@the-dreams.de |
---|---|
State | Accepted |
Headers | show |
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 > */ > };
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.
> Didn't see a comment as stated though. Yeah, it is hidden in the refactoring :/ > + /* I2C_M_RD is guaranteed to be 0x0001! */ Wolfram
> Ah, overlooked it. Anyway, perhaps worth to mention under > Documentation. What do you think? Unsure. We can add it later, I'd say.
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 */ };
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(-)