i2c: make creation of 8 bit addresses consistent
Subsystems (15)
cover i2c: make creation of 8 bit addresses consistent
This series addresses two things which have come up in the past:
* document that I2C_M_RD is bit 0. It is not good coding style but some drivers (and userspace) silently assume so anyhow. To avoid regressions, let's make that explicit.
* use one function to generate an 8 bit address (7 bit address + RW bit) from a struct msg. People did this in various ways, sometimes overly brave and sometimes overly anxious with the operator precedence. That was partly not nice to read and partly triggering code checkers.
The new function was tested using a Renesas Lager board with the i2c-sh_mobile driver. Buildbot was happy with the series, too. A branch can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/8bit_macro
I'd really appreciate more tests on real HW. Also happy for other comments, reviews...
Thanks,
Wolfram
Wolfram Sang (18):
i2c: guarantee that I2C_M_RD will be 0x0001 forever
i2c: introduce helper function to get 8 bit address from a message
i2c: core: use new 8 bit address helper function
i2c: i2c-bcm-iproc: use new 8 bit address helper function
i2c: i2c-bcm-kona: use new 8 bit address helper function
i2c: i2c-brcmstb: use new 8 bit address helper function
i2c: i2c-cpm: use new 8 bit address helper function
i2c: i2c-ibm_iic: use new 8 bit address helper function
i2c: i2c-img-scb: use new 8 bit address helper function
i2c: i2c-iop3xx: use new 8 bit address helper function
i2c: i2c-lpc2k: use new 8 bit address helper function
i2c: i2c-mt65xx: use new 8 bit address helper function
i2c: i2c-ocores: use new 8 bit address helper function
i2c: i2c-powermac: use new 8 bit address helper function
i2c: i2c-qup: use new 8 bit address helper function
i2c: i2c-sh_mobile: use new 8 bit address helper function
i2c: i2c-sirf: use new 8 bit address helper function
i2c: i2c-st: use new 8 bit address helper function
drivers/i2c/busses/i2c-bcm-iproc.c | 2 +- drivers/i2c/busses/i2c-bcm-kona.c | 5 +---- drivers/i2c/busses/i2c-brcmstb.c | 4 +--- drivers/i2c/busses/i2c-cpm.c | 4 +--- drivers/i2c/busses/i2c-ibm_iic.c | 2 +- drivers/i2c/busses/i2c-img-scb.c | 4 +--- drivers/i2c/busses/i2c-iop3xx.c | 5 +---- drivers/i2c/busses/i2c-lpc2k.c | 4 +--- drivers/i2c/busses/i2c-mt65xx.c | 5 +---- drivers/i2c/busses/i2c-ocores.c | 5 +---- drivers/i2c/busses/i2c-powermac.c | 4 +--- drivers/i2c/busses/i2c-qup.c | 2 +- drivers/i2c/busses/i2c-sh_mobile.c | 3 +-- drivers/i2c/busses/i2c-sirf.c | 4 +--- drivers/i2c/busses/i2c-st.c | 3 +-- drivers/i2c/i2c-core.c | 2 +- include/linux/i2c.h | 5 +++++ include/uapi/linux/i2c.h | 13 +++++++------ 18 files changed, 28 insertions(+), 48 deletions(-)
Signature · 1 line
2.7.0
PATCH 1/18 i2c: guarantee that I2C_M_RD will be 0x0001 forever
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.
--- include/uapi/linux/i2c.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
include/uapi/linux/i2c.h +7 -6
@@ -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 */ };
Signature · 1 line
2.7.0
Re: [PATCH 01/18] i2c: guarantee that I2C_M_RD will be 0x0001 forever
Quoted 7 lines · Wolfram Sang
There is code out there in user space and kernel space which relies on
I2C_M_RDbeing 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.
Quoted 50 lines
Signed-off-by Wolfram Sang <[email protected]>---
include/uapi/linux/i2c.h | 13 +++++++------1 file changed, 7 insertions(+), 6 deletions(-)include/uapi/linux/i2c.h +0
-0@@ -68,14 +68,15 @@ struct i2c_msg {
__u16addr; /* slave address *
/
__u16flags;
-#defineI2C_M_TEN0x0010/* this is a ten bit
chip address */
#defineI2C_M_RD0x0001/* read data, from
slave to master */
-#defineI2C_M_STOP0x8000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
-#defineI2C_M_NOSTART0x4000/* ifI2C_FUNC_NOSTART*/
-#defineI2C_M_REV_DIR_ADDR0x2000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
-#defineI2C_M_IGNORE_NAK0x1000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
-#defineI2C_M_NO_RD_ACK0x0800/* ifI2C_FUNC_PROTOCOL_MANGLING*/include/uapi/linux/i2c.h +1
-0+/* I2C_M_RD is guaranteed to be
0x0001! */
+#defineI2C_M_TEN0x0010/* this is a ten bit
chip address */
#defineI2C_M_RECV_LEN0x0400/* length will
be first received byte */
+#defineI2C_M_NO_RD_ACK0x0800/* ifI2C_FUNC_PROTOCOL_MANGLING*/
+#defineI2C_M_IGNORE_NAK0x1000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
+#defineI2C_M_REV_DIR_ADDR0x2000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
+#defineI2C_M_NOSTART0x4000/* ifI2C_FUNC_NOSTART*/
+#defineI2C_M_STOP0x8000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
__u16len; /* msg length*/ __u8 *buf; /* pointer to msg data */};
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Re: [PATCH 01/18] i2c: guarantee that I2C_M_RD will be 0x0001 forever
Quoted 11 lines · Andy Shevchenko
Quoted 9 lines · Wolfram Sang
There is code out there in user space and kernel space which relies on
I2C_M_RDbeing 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?
Quoted 56 lines
Quoted 56 lines
Signed-off-by Wolfram Sang <[email protected]>---
include/uapi/linux/i2c.h | 13 +++++++------1 file changed, 7 insertions(+), 6 deletions(-)include/uapi/linux/i2c.h +0
-0@@ -68,14 +68,15 @@ struct i2c_msg {
__u16addr; /* slave address
*
/
__u16flags;
-#defineI2C_M_TEN0x0010/* this is a ten
bit
chip address */
#defineI2C_M_RD0x0001/* read data, from
slave to master */
-#defineI2C_M_STOP0x8000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
-#defineI2C_M_NOSTART0x4000/* ifI2C_FUNC_NOSTART*/
-#defineI2C_M_REV_DIR_ADDR0x2000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
-#defineI2C_M_IGNORE_NAK0x1000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
-#defineI2C_M_NO_RD_ACK0x0800/* ifI2C_FUNC_PROTOCOL_MANGLING*/include/uapi/linux/i2c.h +1
-0+/* I2C_M_RD is guaranteed to
be0x0001! */
+#defineI2C_M_TEN0x0010/* this is a ten
bit
chip address */
#defineI2C_M_RECV_LEN0x0400/* length will
be first received byte */
+#defineI2C_M_NO_RD_ACK0x0800/* ifI2C_FUNC_PROTOCOL_MANGLING*/
+#defineI2C_M_IGNORE_NAK0x1000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
+#defineI2C_M_REV_DIR_ADDR0x2000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
+#defineI2C_M_NOSTART0x4000/* ifI2C_FUNC_NOSTART*/
+#defineI2C_M_STOP0x8000/* ifI2C_FUNC_PROTOCOL_MANGLING*/
__u16len; /* msg length*/ __u8 *buf; /* pointer to msg data */};
Signature · 12 lines
Andy Shevchenko <[email protected]> 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.
Re: [PATCH 01/18] i2c: guarantee that I2C_M_RD will be 0x0001 forever
Quoted 3 lines
Ah, overlooked it. Anyway, perhaps worth to mention under Documentation. What do you think?
Unsure. We can add it later, I'd say.
Re: [PATCH 01/18] i2c: guarantee that I2C_M_RD will be 0x0001 forever
In reply to Andy ShevchenkoQuoted 1 line
Didn't see a comment as stated though.
Yeah, it is hidden in the refactoring :/
include/uapi/linux/i2c.h line 72
Quoted 1 line
include/uapi/linux/i2c.h +1
-0+/* I2C_M_RD is guaranteed to be 0x0001! */
Wolfram
PATCH 2/18 i2c: introduce helper function to get 8 bit address from a message
In reply to Wolfram SangDrivers do this in various ways, let's use one standard way of doing it.
Note: I2C_M_RD is bit 0, so the code could be simplified. To be extremly robust and to advertise good coding practices, I still use the ternary operator and let the compilers do the optimizing job.
--- include/linux/i2c.h | 5 +++++ 1 file changed, 5 insertions(+)
include/linux/i2c.h +5 -0
@@ -654,6 +654,11 @@ static inline int i2c_adapter_id(struct i2c_adapter *adap)return adap->nr; } +static inline u8 i2c_8bit_addr_from_msg (const struct i2c_msg *msg) +{ +return (msg->addr <<1 ) | (msg->flags &I2C_M_RD ?1 :0 ); +} +/** * module_i2c_driver() - Helper macro for registering a modular I2C driver * @__i2c_driver: i2c_driver struct
Signature · 1 line
2.7.0
PATCH 3/18 i2c: core: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/i2c-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/i2c-core.c +1 -1
@@ -2652,7 +2652,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t count)static u8 i2c_smbus_msg_pec (u8 pec,struct i2c_msg *msg) {/* The address will be sent first */ -u8 addr = (msg->addr <<1 ) | !!(msg->flags &I2C_M_RD ); +u8 addr =i2c_8bit_addr_from_msg (msg); pec =i2c_smbus_pec (pec, &addr,1 );/* The data buffer follows */
Signature · 1 line
2.7.0
Re: [PATCH 03/18] i2c: core: use new 8 bit address helper function
On Sun, 2016-04-03 at 20:44 +0200, Wolfram Sang wrote:
Even simple short commit message? Same to the rest.
Quoted 19 lines
Signed-off-by Wolfram Sang <[email protected]>---
drivers/i2c/i2c-core.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/i2c/i2c-core.c +1
-1@@ -2652,7 +2652,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t count) static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg) { /* The address will be sent first */ -u8 addr = (msg->addr <<1 ) | !!(msg->flags &I2C_M_RD ); +u8 addr =i2c_8bit_addr_from_msg (msg); pec = i2c_smbus_pec(pec, &addr, 1);
/* The data buffer follows */
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
PATCH 4/18 i2c: i2c-bcm-iproc: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-bcm-iproc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-bcm-iproc.c +1 -1
@@ -267,7 +267,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c, iproc_i2c->msg = msg;/* format and load slave address into the TX FIFO */ - addr = msg->addr <<1 | (msg->flags &I2C_M_RD ?1 :0 ); + addr =i2c_8bit_addr_from_msg (msg);writel (addr, iproc_i2c->base +M_TX_OFFSET );/*
Signature · 1 line
2.7.0
PATCH 5/18 i2c: i2c-bcm-kona: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-bcm-kona.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/i2c/busses/i2c-bcm-kona.c +1 -4
@@ -501,10 +501,7 @@ static int bcm_kona_i2c_do_addr(struct bcm_kona_i2c_dev *dev,return -EREMOTEIO ; } }else { - addr = msg->addr <<1 ; - -if (msg->flags &I2C_M_RD ) - addr |=1 ; + addr =i2c_8bit_addr_from_msg (msg);if (bcm_kona_i2c_write_byte (dev, addr,0 ) <0 )return -EREMOTEIO ;
Signature · 1 line
2.7.0
PATCH 6/18 i2c: i2c-brcmstb: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-brcmstb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/i2c/busses/i2c-brcmstb.c +1 -3
@@ -446,9 +446,7 @@ static int brcmstb_i2c_do_addr(struct brcmstb_i2c_dev *dev, } }else { - addr = msg->addr <<1 ; -if (msg->flags &I2C_M_RD ) - addr |=1 ; + addr =i2c_8bit_addr_from_msg (msg);bsc_writel (dev, addr, chip_address); }
Signature · 1 line
2.7.0
PATCH 7/18 i2c: i2c-cpm: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-cpm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/i2c/busses/i2c-cpm.c +1 -3
@@ -197,9 +197,7 @@ static void cpm_i2c_parse_message(struct i2c_adapter *adap, tbdf = cpm->tbase + tx; rbdf = cpm->rbase + rx; - addr = pmsg->addr <<1 ; -if (pmsg->flags &I2C_M_RD ) - addr |=1 ; + addr =i2c_8bit_addr_from_msg (pmsg); tb = cpm->txbuf[tx]; rb = cpm->rxbuf[rx];
Signature · 1 line
2.7.0
PATCH 8/18 i2c: i2c-ibm_iic: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-ibm_iic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-ibm_iic.c +1 -1
@@ -269,7 +269,7 @@ static int iic_smbus_quick(struct ibm_iic_private* dev, const struct i2c_msg* p)ndelay (t->hd_sta);/* Send address */ - v = (u8)((p->addr <<1 ) | ((p->flags &I2C_M_RD ) ?1 :0 )); + v =i2c_8bit_addr_from_msg (p);for (i =0 , mask =0x80 ; i <8 ; ++i, mask >>=1 ){out_8 (&iic->directcntl, sda);ndelay (t->low /2 );
Signature · 1 line
2.7.0
PATCH 9/18 i2c: i2c-img-scb: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-img-scb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/i2c/busses/i2c-img-scb.c +1 -3
@@ -751,9 +751,7 @@ static unsigned int img_i2c_atomic(struct img_i2c *i2c,switch (i2c->at_cur_cmd) {case CMD_GEN_START : next_cmd =CMD_GEN_DATA ; - next_data = (i2c->msg.addr <<1 ); -if (i2c->msg.flags &I2C_M_RD ) - next_data |=0x1 ; + next_data =i2c_8bit_addr_from_msg (&i2c->msg);break ;case CMD_GEN_DATA :if (i2c->line_status &LINESTAT_INPUT_HELD_V )
Signature · 1 line
2.7.0
PATCH 10/18 i2c: i2c-iop3xx: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-iop3xx.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/i2c/busses/i2c-iop3xx.c +1 -4
@@ -50,10 +50,7 @@ iic_cook_addr(struct i2c_msg *msg) {unsigned char addr; - addr = (msg->addr <<1 ); - -if (msg->flags &I2C_M_RD ) - addr |=1 ; + addr =i2c_8bit_addr_from_msg (msg);return addr; }
Signature · 1 line
2.7.0
PATCH 11/18 i2c: i2c-lpc2k: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-lpc2k.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/i2c/busses/i2c-lpc2k.c +1 -3
@@ -133,9 +133,7 @@ static void i2c_lpc2k_pump_msg(struct lpc2k_i2c *i2c)case M_START :case M_REPSTART :/* Start bit was just sent out, send out addr and dir */ - data = i2c->msg->addr <<1 ; -if (i2c->msg->flags &I2C_M_RD ) - data |=1 ; + data =i2c_8bit_addr_from_msg (i2c->msg);writel (data, i2c->base +LPC24XX_I2DAT );writel (LPC24XX_STA , i2c->base +LPC24XX_I2CONCLR );
Signature · 1 line
2.7.0
PATCH 12/18 i2c: i2c-mt65xx: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-mt65xx.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/i2c/busses/i2c-mt65xx.c +1 -4
@@ -413,10 +413,7 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,else writew (I2C_FS_START_CON , i2c->base +OFFSET_EXT_CONF ); - addr_reg = msgs->addr <<1 ; -if (i2c->op ==I2C_MASTER_RD ) - addr_reg |=0x1 ; - + addr_reg =i2c_8bit_addr_from_msg (msgs);writew (addr_reg, i2c->base +OFFSET_SLAVE_ADDR );/* Clear interrupt status */
Signature · 1 line
2.7.0
PATCH 13/18 i2c: i2c-ocores: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-ocores.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/i2c/busses/i2c-ocores.c +1 -4
@@ -178,10 +178,7 @@ static void ocores_process(struct ocores_i2c *i2c)if (i2c->nmsgs) {/* end? */ /* send start? */ if (!(msg->flags &I2C_M_NOSTART )) { -u8 addr = (msg->addr <<1 ); - -if (msg->flags &I2C_M_RD ) - addr |=1 ; +u8 addr =i2c_8bit_addr_from_msg (msg); i2c->state =STATE_START ;
Signature · 1 line
2.7.0
PATCH 14/18 i2c: i2c-powermac: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-powermac.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/i2c/busses/i2c-powermac.c +1 -3
@@ -150,13 +150,11 @@ static int i2c_powermac_master_xfer( struct i2c_adapter *adap, {struct pmac_i2c_bus *bus =i2c_get_adapdata (adap);int rc =0 ; -int read;int addrdir;if (msgs->flags &I2C_M_TEN )return -EINVAL ; - read = (msgs->flags &I2C_M_RD ) !=0 ; - addrdir = (msgs->addr <<1 ) | read; + addrdir =i2c_8bit_addr_from_msg (msgs); rc =pmac_i2c_open (bus,0 );if (rc) {
Signature · 1 line
2.7.0
PATCH 15/18 i2c: i2c-qup: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-qup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-qup.c +1 -1
@@ -515,7 +515,7 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)static int qup_i2c_set_tags (u8 *tags,struct qup_i2c_dev *qup,struct i2c_msg *msg,int is_dma) { -u16 addr = (msg->addr <<1 ) | ((msg->flags &I2C_M_RD ) ==I2C_M_RD ); +u16 addr =i2c_8bit_addr_from_msg (msg);int len =0 ;int data_len;
Signature · 1 line
2.7.0
Re: [15/18] i2c: i2c-qup: use new 8 bit address helper function
Quoted 10 lines · Wolfram Sang
Signed-off-by Wolfram Sang <[email protected]>--- drivers/i2c/busses/i2c-qup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/i2c/busses/i2c-qup.c +0
-0
drivers/i2c/busses/i2c-qup.c static int qup_i2c_get_data_len(struct qup_i2c_dev *qup) · line 520
Quoted 7 lines
drivers/i2c/busses/i2c-qup.c +1
-1@@ -515,7 +515,7 @@ static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)static int qup_i2c_set_tags (u8 *tags,struct qup_i2c_dev *qup,struct i2c_msg *msg,int is_dma) { -u16 addr = (msg->addr <<1 ) | ((msg->flags &I2C_M_RD ) ==I2C_M_RD ); +u16 addr =i2c_8bit_addr_from_msg (msg);int len =0 ;int data_len;
Looks good to me. Tested ok on QDF2432.
There is another place in issue_read that we can use this helper function. I will send a follow up patch.
Reviewed by: Naveen Kaje <[email protected]>
Tested by: Naveen Kaje <[email protected]>
Quoted 1 line
PATCH 16/18 i2c: i2c-sh_mobile: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-sh_mobile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/i2c/busses/i2c-sh_mobile.c +1 -2
@@ -398,8 +398,7 @@ static void sh_mobile_i2c_get_data(struct sh_mobile_i2c_data *pd, {switch (pd->pos) {case -1 : - *buf = (pd->msg->addr &0x7f ) <<1 ; - *buf |= (pd->msg->flags &I2C_M_RD ) ?1 :0 ; + *buf =i2c_8bit_addr_from_msg (pd->msg);break ;default : *buf = pd->msg->buf[pd->pos];
Signature · 1 line
2.7.0
PATCH 17/18 i2c: i2c-sirf: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-sirf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/i2c/busses/i2c-sirf.c +1 -3
@@ -190,9 +190,7 @@ static void i2c_sirfsoc_set_address(struct sirfsoc_i2c *siic,writel (regval, siic->base +SIRFSOC_I2C_CMD (siic->cmd_ptr++)); - addr = msg->addr <<1 ;/* Generate address */ -if (msg->flags &I2C_M_RD ) - addr |=1 ; + addr =i2c_8bit_addr_from_msg (msg);/* Reverse direction bit */ if (msg->flags &I2C_M_REV_DIR_ADDR )
Signature · 1 line
2.7.0
PATCH 18/18 i2c: i2c-st: use new 8 bit address helper function
In reply to Wolfram Sang--- drivers/i2c/busses/i2c-st.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/i2c/busses/i2c-st.c +1 -2
@@ -614,8 +614,7 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,unsigned long timeout;int ret; - c->addr = (u8)(msg->addr <<1 ); - c->addr |= (msg->flags &I2C_M_RD ); + c->addr =i2c_8bit_addr_from_msg (msg); c->buf = msg->buf; c->count = msg->len; c->xfered =0 ;
Signature · 1 line
2.7.0
Quoted 21 lines · Wolfram Sang
This series addresses two things which have come up in the past:
* document that
I2C_M_RDis bit 0. It is not good coding style but some
drivers (and userspace) silently assume so anyhow. To avoid regressions,
let's make that explicit.* use one function to generate an 8 bit address (7 bit address + RW bit)
from a struct msg. People did this in various ways, sometimes overly
brave and sometimes overly anxious with the operator precedence. That
was partly not nice to read and partly triggering code checkers.The new function was tested using a Renesas Lager board with the i2c-
sh_mobile
driver. Buildbot was happy with the series, too. A branch can be found here:
Quoted 6 lines
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
i2c/8bit_macroI'd really appreciate more tests on real HW. Also happy for other comments,
reviews...
FWIW:
P.S. Couple of minor question, though I'm okay with either option.
Quoted 46 lines
Thanks,
Wolfram
Wolfram Sang (18):
i2c: guarantee thatI2C_M_RDwill be0x0001forever
i2c: introduce helper function to get 8 bit address from a message
i2c: core: use new 8 bit address helper function
i2c: i2c-bcm-iproc: use new 8 bit address helper function
i2c: i2c-bcm-kona: use new 8 bit address helper function
i2c: i2c-brcmstb: use new 8 bit address helper function
i2c: i2c-cpm: use new 8 bit address helper function
i2c: i2c-ibm_iic: use new 8 bit address helper function
i2c: i2c-img-scb: use new 8 bit address helper function
i2c: i2c-iop3xx: use new 8 bit address helper function
i2c: i2c-lpc2k: use new 8 bit address helper function
i2c: i2c-mt65xx: use new 8 bit address helper function
i2c: i2c-ocores: use new 8 bit address helper function
i2c: i2c-powermac: use new 8 bit address helper function
i2c: i2c-qup: use new 8 bit address helper function
i2c: i2c-sh_mobile: use new 8 bit address helper function
i2c: i2c-sirf: use new 8 bit address helper function
i2c: i2c-st: use new 8 bit address helper functiondrivers/i2c/busses/i2c-bcm-iproc.c | 2 +- drivers/i2c/busses/i2c-bcm-kona.c | 5 +---- drivers/i2c/busses/i2c-brcmstb.c | 4 +--- drivers/i2c/busses/i2c-cpm.c | 4 +--- drivers/i2c/busses/i2c-ibm_iic.c | 2 +- drivers/i2c/busses/i2c-img-scb.c | 4 +--- drivers/i2c/busses/i2c-iop3xx.c | 5 +---- drivers/i2c/busses/i2c-lpc2k.c | 4 +--- drivers/i2c/busses/i2c-mt65xx.c | 5 +---- drivers/i2c/busses/i2c-ocores.c | 5 +---- drivers/i2c/busses/i2c-powermac.c | 4 +--- drivers/i2c/busses/i2c-qup.c | 2 +-
drivers/i2c/busses/i2c-sh_mobile.c| 3 +-- drivers/i2c/busses/i2c-sirf.c | 4 +--- drivers/i2c/busses/i2c-st.c | 3 +-- drivers/i2c/i2c-core.c | 2 +- include/linux/i2c.h | 5 +++++ include/uapi/linux/i2c.h | 13 +++++++------18 files changed, 28 insertions(+), 48 deletions(-)
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Wolfram Sang
AppliedQuoted 11 lines · Wolfram Sang
This series addresses two things which have come up in the past:
* document that
I2C_M_RDis bit 0. It is not good coding style but some drivers (and userspace) silently assume so anyhow. To avoid regressions, let's make that explicit.* use one function to generate an 8 bit address (7 bit address + RW bit) from a struct msg. People did this in various ways, sometimes overly brave and sometimes overly anxious with the operator precedence. That was partly not nice to read and partly triggering code checkers.
Quoted 7 lines
The new function was tested using a Renesas Lager board with the i2c-sh_mobile driver. Buildbot was happy with the series, too. A branch can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.giti2c/8bit_macroI'd really appreciate more tests on real HW. Also happy for other comments, reviews...
Applied to for-next, thanks!