i2c_intel_mid: Improve error reporting
PATCH 1/2 i2c_intel_mid: Improve error reporting
From: Catalin Popescu <[email protected]>
The error messages printed from mrst_i2c_abort() didn't give slave address info.
But I2C device driver developers always need this to check which slave device has the problem.
This patch enhances the error message format by adding slave address info to each error message.
Signed-off-by Bin Yang <[email protected]>
[Ported to upstream driver branch and tidied a spot]
Signed-off-by Alan Cox <alan-VuQAYsv1563Yd54FQh9/[email protected]>
---
drivers/i2c/busses/i2c-intel-mid.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
drivers/i2c/busses/i2c-intel-mid.c +12 -4
@@ -78,7 +78,7 @@ struct intel_mid_i2c_private {void __iomem *base;int speed;struct completion complete; -int abort; +u32 abort;u8 *rx_buf;int rx_buf_len;enum mid_i2c_status status;
@@ -464,9 +464,15 @@ static inline bool intel_mid_i2c_address_neq(const struct i2c_msg *p1,static void intel_mid_i2c_abort (struct intel_mid_i2c_private *i2c) {/* Read about source register */ -int abort = i2c->abort; +u32 abort = i2c->abort;struct i2c_adapter *adap = &i2c->adap; +if (abort & (ABRT_MASTER_DIS |ABRT_10B_RD_NORSTRT | +ABRT_SBYTE_NORSTRT |ABRT_SBYTE_ACKDET |ABRT_TXDATA_NOACK | +ABRT_10ADDR2_NOACK |ABRT_10ADDR1_NOACK |ABRT_7B_ADDR_NOACK )) +dev_err (&adap->dev,"i2c abort on address 0x%x\n" , + i2c->msg->addr); +/* Single transfer error check: * According to databook, TX/RX FIFOs would be flushed when * the abort interrupt occured.
@@ -550,7 +556,8 @@ static int xfer_read(struct i2c_adapter *adap, unsigned char *buf, int length) i2c->status =STATUS_READ_START ; err =wait_for_completion_interruptible_timeout (&i2c->complete,HZ );if (!err) { -dev_err (&adap->dev,"Timeout for ACK from I2C slave device\n" ); +dev_err (&adap->dev,"Timeout for ACK from I2C slave 0x%x\n" , + i2c->msg->addr);intel_mid_i2c_hwinit (i2c);return -ETIMEDOUT ; }
@@ -602,7 +609,8 @@ static int xfer_write(struct i2c_adapter *adap, i2c->status =STATUS_WRITE_START ; err =wait_for_completion_interruptible_timeout (&i2c->complete,HZ );if (!err) { -dev_err (&adap->dev,"Timeout for ACK from I2C slave device\n" ); +dev_err (&adap->dev,"Timeout for ACK from I2C slave 0x%x\n" , + i2c->msg->addr);intel_mid_i2c_hwinit (i2c);return -ETIMEDOUT ; }else {
A message that is not in the archive (sent off-list or not yet received): 20101124144904.10210.90314.stgit-Z/[email protected]
PATCH 2/2 i2c-intel-mid: improve timeout handling
From: Bin Yang <[email protected]>
The old solution for i2c xfer timeout was to set timeout value to one second for all i2c xfers. That's not reasonable for all of the various speed modes and data lengths.
This patch sets the xfer_read timeout value based on both bus speed and data length.
Signed-off-by Bin Yang <[email protected]>
[Ported to the upstream branch and extracted as a helper function]
Signed-off-by Alan Cox <alan-VuQAYsv1563Yd54FQh9/[email protected]>
---
drivers/i2c/busses/i2c-intel-mid.c | 31 +++++++++++++++++++++++++++---- 1 files changed, 27 insertions(+), 4 deletions(-)
drivers/i2c/busses/i2c-intel-mid.c +27 -4
@@ -303,6 +303,29 @@ static int ctl_num = 6;module_param_array (speed_mode, int, &ctl_num,S_IRUGO );MODULE_PARM_DESC (speed_mode,"Set the speed of the i2c interface (0-2)" ); +/* Timeout and delay bases for the speed modes */ +static const u16 xfer_timeout[NUM_SPEEDS ] = {100 ,25 ,3 }; + +/** +* xfer_wait - wait for a transfer to complete +* @adapt: i2c interface +* @len: length of message +* +* Wait for completion or timeout of a message. The timeout depends upon +* the speed of the interface and the message length +* +* Returns the result of the wait for completion +*/ + +static int xfer_wait (struct intel_mid_i2c_private *i2c,int len) +{ +unsigned long timeout; + + timeout =msecs_to_jiffies (len * xfer_timeout[i2c->speed]); +return wait_for_completion_interruptible_timeout (&i2c->complete, + timeout); +} +/** * intel_mid_i2c_disable - Disable I2C controller * @adap: struct pointer to i2c_adapter
@@ -322,7 +345,6 @@ static int intel_mid_i2c_disable(struct i2c_adapter *adap)int err =0 ;int count =0 ;int ret1, ret2; -static const u16 delay[NUM_SPEEDS ] = {100 ,25 ,3 };/* Set IC_ENABLE to 0 */ writel (0 , i2c->base +IC_ENABLE );
@@ -331,7 +353,7 @@ static int intel_mid_i2c_disable(struct i2c_adapter *adap)dev_dbg (&adap->dev,"mrst i2c disable\n" );while ((ret1 =readl (i2c->base +IC_ENABLE_STATUS ) &0x1 ) || (ret2 =readl (i2c->base +IC_STATUS ) &0x1 )) { -udelay (delay[i2c->speed]); +udelay (xfer_timeout[i2c->speed]);writel (0 , i2c->base +IC_ENABLE );dev_dbg (&adap->dev,"i2c is busy, count is %d speed %d\n" , count, i2c->speed);
@@ -513,6 +535,7 @@ static void intel_mid_i2c_abort(struct intel_mid_i2c_private *i2c) i2c->status =STATUS_XFER_ABORT ; } +/** * xfer_read - Internal function to implement master read transfer. * @adap: i2c_adapter struct pointer
@@ -554,7 +577,7 @@ static int xfer_read(struct i2c_adapter *adap, unsigned char *buf, int length)writel (IC_RD , i2c->base +IC_DATA_CMD ); i2c->status =STATUS_READ_START ; - err =wait_for_completion_interruptible_timeout (&i2c->complete,HZ ); + err =xfer_wait (i2c, length);if (!err) {dev_err (&adap->dev,"Timeout for ACK from I2C slave 0x%x\n" , i2c->msg->addr);
@@ -607,7 +630,7 @@ static int xfer_write(struct i2c_adapter *adap,writel ((u16)(*(buf + i)), i2c->base +IC_DATA_CMD ); i2c->status =STATUS_WRITE_START ; - err =wait_for_completion_interruptible_timeout (&i2c->complete,HZ ); + err =xfer_wait (i2c, length);if (!err) {dev_err (&adap->dev,"Timeout for ACK from I2C slave 0x%x\n" , i2c->msg->addr);
A message that is not in the archive (sent off-list or not yet received): 20101124144920.10210.36601.stgit-Z/[email protected]
Re: [PATCH 2/2] i2c-intel-mid: improve timeout handling
Quoted 23 lines · Alan Cox
From: Bin Yang <[email protected]>
The old solution for i2c xfer timeout was to set timeout value to one second for all i2c xfers. That's not reasonable for all of the various speed modes and data lengths.
This patch sets the
xfer_readtimeout value based on both bus speed and data length.Signed-off-by Bin Yang <[email protected]>[Ported to the upstream branch and extracted as a helper function]Signed-off-by Alan Cox <alan-VuQAYsv1563Yd54FQh9/[email protected]>---
drivers/i2c/busses/i2c-intel-mid.c | 31 +++++++++++++++++++++++++++---- 1 files changed, 27 insertions(+), 4 deletions(-)drivers/i2c/busses/i2c-intel-mid.c +0
-0
drivers/i2c/busses/i2c-intel-mid.c static int ctl_num = 6; · line 304
Quoted 2 lines
drivers/i2c/busses/i2c-intel-mid.c +0
-0@@ -303,6 +303,29 @@ static int ctl_num = 6;module_param_array (speed_mode, int, &ctl_num,S_IRUGO );MODULE_PARM_DESC (speed_mode,"Set the speed of the i2c interface (0-2)" );
ok, will queue for next merge window
Quoted 61 lines · Alan Cox
From: Catalin Popescu <[email protected]>
The error messages printed from
mrst_i2c_abort()didn't give slave address info.But I2C device driver developers always need this to check which slave device has the problem.
This patch enhances the error message format by adding slave address info to each error message.
Signed-off-by Bin Yang <[email protected]>[Ported to upstream driver branch and tidied a spot]Signed-off-by Alan Cox <alan-VuQAYsv1563Yd54FQh9/[email protected]>---
drivers/i2c/busses/i2c-intel-mid.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)drivers/i2c/busses/i2c-intel-mid.c +10
-3@@ -78,7 +78,7 @@ struct intel_mid_i2c_private {void __iomem *base;int speed;struct completion complete; -int abort; +u32 abort;u8 *rx_buf;int rx_buf_len;enum mid_i2c_status status;@@ -464,9 +464,15 @@ static inline bool intel_mid_i2c_address_neq(const struct i2c_msg *p1,static void intel_mid_i2c_abort (struct intel_mid_i2c_private *i2c) {/* Read about source register */ -int abort = i2c->abort; +u32 abort = i2c->abort;struct i2c_adapter *adap = &i2c->adap; +if (abort & (ABRT_MASTER_DIS |ABRT_10B_RD_NORSTRT | +ABRT_SBYTE_NORSTRT |ABRT_SBYTE_ACKDET |ABRT_TXDATA_NOACK | +ABRT_10ADDR2_NOACK |ABRT_10ADDR1_NOACK |ABRT_7B_ADDR_NOACK )) +dev_err (&adap->dev,"i2c abort on address 0x%x\n" , + i2c->msg->addr); +/* Single transfer error check: * According to databook, TX/RX FIFOs would be flushed when * the abort interrupt occured. @@ -550,7 +556,8 @@ static int xfer_read(struct i2c_adapter *adap, unsigned char *buf, int length) i2c->status =STATUS_READ_START ; err =wait_for_completion_interruptible_timeout (&i2c->complete,HZ );if (!err) { -dev_err (&adap->dev,"Timeout for ACK from I2C slave device\n" ); +dev_err (&adap->dev,"Timeout for ACK from I2C slave 0x%x\n" , + i2c->msg->addr);intel_mid_i2c_hwinit (i2c);return -ETIMEDOUT ; }
drivers/i2c/busses/i2c-intel-mid.c static int xfer_write(struct i2c_adapter *adap, · line 616
Quoted 9 lines
drivers/i2c/busses/i2c-intel-mid.c +2
-1@@ -602,7 +609,8 @@ static int xfer_write(struct i2c_adapter *adap, i2c->status =STATUS_WRITE_START ; err =wait_for_completion_interruptible_timeout (&i2c->complete,HZ );if (!err) { -dev_err (&adap->dev,"Timeout for ACK from I2C slave device\n" ); +dev_err (&adap->dev,"Timeout for ACK from I2C slave 0x%x\n" , + i2c->msg->addr);intel_mid_i2c_hwinit (i2c);return -ETIMEDOUT ; }else {
ok, will queue for next merge window.