linux-mips
[Top] [All Lists]

Re: [PATCH 15/16] i2c-designware: i2c_dw_xfer_msg: Mark as completed on

To: baruch@tkos.co.il, linux-i2c@vger.kernel.org
Subject: Re: [PATCH 15/16] i2c-designware: i2c_dw_xfer_msg: Mark as completed on an error
From: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
Date: Thu, 15 Oct 2009 14:29:43 +0900
Cc: ben-linux@fluff.org, linux-mips@linux-mips.org, linux-arm-kernel@lists.infradead.org
In-reply-to: <4AD3EBC4.6030705@necel.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <4AD3E974.8080200@necel.com> <4AD3EBC4.6030705@necel.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Thunderbird 2.0.0.23 (Windows/20090812)
Shinya Kuribayashi wrote:
As wait_for_completion_interruptible_timeout() will be invoked after
the first call to i2c_dw_xfer_msg() is made whether or not an error is
detected in it, we need to mark ->cmd_complete as completed to avoid a
needless HZ timeout.

diff --git a/drivers/i2c/busses/i2c-designware.c 
b/drivers/i2c/busses/i2c-designware.c
index f7ea032..6f85e28 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -376,6 +376,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
                        dev_err(dev->dev,
                                "%s: invalid message length\n", __func__);
                        dev->msg_err = -EINVAL;
+                       complete(&dev->cmd_complete);
                        return;
                }

It turned out to be incomplete, I confimred with I2C_FUNC_SMBUS_QUICK
flag for testing.

In [PATCH 10/16] (Do dw_i2c_pump_msg's jobs in the interrutp handler),
I deleted the following line from the interrupt handler,
-       writel(0, dev->base + DW_IC_INTR_MASK);      /* disable interrupts */

so some interrupt bits, namely RX_FULL and somtimes TX_EMPTY, are now
kept _opened_ by default.  In error handling procedures, we need to
disable them or shutdown DW I2C core properly, otherwise those
interrupts continue to be asserted.

In addition, I noticed that we need similar treatments in target addr
inconsistency checking path.

Here's problematic parts.  I'll sort out these bits, later.
---
drivers/i2c/busses/i2c-designware.c |   18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c 
b/drivers/i2c/busses/i2c-designware.c
index 060f2dd..70aaaec 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -370,13 +370,21 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
                 * reprogram the target address in the i2c
                 * adapter when we are done with this transfer
                 */
-               if (msgs[dev->msg_write_idx].addr != addr)
+               if (msgs[dev->msg_write_idx].addr != addr) {
+                       dev_err(dev->dev,
+                               "%s: invalid message address\n", __func__);
+                       dev->msg_err = -EINVAL;
+                       writel(0, dev->base + DW_IC_ENABLE);
+                       complete(&dev->cmd_complete);
                        return;
+               }

                if (msgs[dev->msg_write_idx].len == 0) {
                        dev_err(dev->dev,
                                "%s: invalid message length\n", __func__);
                        dev->msg_err = -EINVAL;
+                       writel(0, dev->base + DW_IC_ENABLE);
+                       complete(&dev->cmd_complete);
                        return;
                }

@@ -430,8 +438,14 @@ i2c_dw_read(struct dw_i2c_dev *dev)
                        continue;

                /* different i2c client, reprogram the i2c adapter */
-               if (msgs[dev->msg_read_idx].addr != addr)
+               if (msgs[dev->msg_read_idx].addr != addr) {
+                       dev_err(dev->dev,
+                               "%s: invalid message address\n", __func__);
+                       dev->msg_err = -EINVAL;
+                       writel(0, dev->base + DW_IC_ENABLE);
+                       complete(&dev->cmd_complete);
                        return;
+               }

                if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
                        len = msgs[dev->msg_read_idx].len;
--
Shinya Kuribayashi
NEC Electronics

<Prev in Thread] Current Thread [Next in Thread>