diff mbox series

[v2,6/6] i2c: mpc: Interrupt driven transfer

Message ID 20210329015206.17437-7-chris.packham@alliedtelesis.co.nz
State Superseded
Headers show
Series i2c: mpc: Refactor to improve responsiveness | expand

Commit Message

Chris Packham March 29, 2021, 1:52 a.m. UTC 
The fsl-i2c controller will generate an interrupt after every byte
transferred. Make use of this interrupt to drive a state machine which
allows the next part of a transfer to happen as soon as the interrupt is
received. This is particularly helpful with SMBUS devices like the LM81
which will timeout if we take too long between bytes in a transfer.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v2:
    - add static_assert for state debug strings
    - remove superfluous space

 drivers/i2c/busses/i2c-mpc.c | 434 +++++++++++++++++++----------------
 1 file changed, 241 insertions(+), 193 deletions(-)

Comments

Wolfram Sang April 10, 2021, 8:13 p.m. UTC | #1 
On Mon, Mar 29, 2021 at 02:52:06PM +1300, Chris Packham wrote:
> The fsl-i2c controller will generate an interrupt after every byte
> transferred. Make use of this interrupt to drive a state machine which
> allows the next part of a transfer to happen as soon as the interrupt is
> received. This is particularly helpful with SMBUS devices like the LM81
> which will timeout if we take too long between bytes in a transfer.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Okay, this change is too large and HW specific for a detailed review.
But I trust you and hope you will be around to fix regressions if I
apply it for 5.13? That kind of leads to the question if you want to
step up as the maintainer for this driver?

Only thing I noticed was a "BUG" and "BUG_ON" and wonder if we really
need to halt the kernel in that case. Maybe WARN is enough?

I'll apply the first five patches now, they look good to me.
Chris Packham April 11, 2021, 9:31 p.m. UTC | #2 
On 11/04/21 8:13 am, Wolfram Sang wrote:
> On Mon, Mar 29, 2021 at 02:52:06PM +1300, Chris Packham wrote:
>> The fsl-i2c controller will generate an interrupt after every byte
>> transferred. Make use of this interrupt to drive a state machine which
>> allows the next part of a transfer to happen as soon as the interrupt is
>> received. This is particularly helpful with SMBUS devices like the LM81
>> which will timeout if we take too long between bytes in a transfer.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Okay, this change is too large and HW specific for a detailed review.
> But I trust you and hope you will be around to fix regressions if I
> apply it for 5.13?
Yep I plan on being around. I've got access to a couple of designs with 
P2040 and T2081 so hopefully that's sufficient to deal with any 
regressions. One issue is a lack of different i2c devices (the systems 
we have tend to use the same devices) but hopefully any reports of 
regression will be from people with access to such devices.
> That kind of leads to the question if you want to
> step up as the maintainer for this driver?
Sure can do. It'd be nice if it was someone from NXP but I think they've 
lost interest in the PowerPC based SoCs. Should I send a patch for 
MAINTAINERS? If so does that go through the i2c tree?
> Only thing I noticed was a "BUG" and "BUG_ON" and wonder if we really
> need to halt the kernel in that case. Maybe WARN is enough?

Yeah I think they can both be WARN variants. The one in mpc_xfer() can 
happily continue. It's a little less clear what I should do in 
mpc_i2c_do_action() if the WARN is ever hit but in theory it should be 
an unreachable case anyway so the only thing that could get there is 
some kind of memory corruption which would likely cause a crash elsewhere.

Do you want me to send a V3 of just that patch?

> I'll apply the first five patches now, they look good to me.
>
Wolfram Sang April 12, 2021, 6:17 p.m. UTC | #3 
Hi Chris,

> Yep I plan on being around. I've got access to a couple of designs with 

> P2040 and T2081 so hopefully that's sufficient to deal with any 

> regressions. One issue is a lack of different i2c devices (the systems 

> we have tend to use the same devices) but hopefully any reports of 

> regression will be from people with access to such devices.


Sounds very good to me.

> > That kind of leads to the question if you want to

> > step up as the maintainer for this driver?

> Sure can do. It'd be nice if it was someone from NXP but I think they've 

> lost interest in the PowerPC based SoCs. Should I send a patch for 

> MAINTAINERS? If so does that go through the i2c tree?


Yes, please send a patch and I will merge it via I2C. I don't have hope
for someone from NXP because it was difficult enough to get maintainers
for the actively sold SoCs.

> > Only thing I noticed was a "BUG" and "BUG_ON" and wonder if we really

> > need to halt the kernel in that case. Maybe WARN is enough?

> 

> Yeah I think they can both be WARN variants. The one in mpc_xfer() can 

> happily continue. It's a little less clear what I should do in 

> mpc_i2c_do_action() if the WARN is ever hit but in theory it should be 

> an unreachable case anyway so the only thing that could get there is 

> some kind of memory corruption which would likely cause a crash elsewhere.


Yeah, please change to WARN.

> Do you want me to send a V3 of just that patch?


Yes, plus the MAINTAINERS patch, please.

Happy hacking,

   Wolfram
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 46cdb36e2f9b..f5c155125de9 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -1,16 +1,11 @@ 
+// SPDX-License-Identifier: GPL-2.0
 /*
- * (C) Copyright 2003-2004
- * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
-
  * This is a combined i2c adapter and algorithm driver for the
  * MPC107/Tsi107 PowerPC northbridge and processors that include
  * the same I2C unit (8240, 8245, 85xx).
  *
- * Release 0.8
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
+ * Copyright (C) 2003-2004 Humboldt Solutions Ltd, adrian@humboldt.co.uk
+ * Copyright (C) 2021 Allied Telesis Labs
  */
 
 #include <linux/kernel.h>
@@ -58,11 +53,36 @@ 
 #define CSR_MIF  0x02
 #define CSR_RXAK 0x01
 
+enum mpc_i2c_action {
+	MPC_I2C_ACTION_INVALID = 0,
+	MPC_I2C_ACTION_START,
+	MPC_I2C_ACTION_RESTART,
+	MPC_I2C_ACTION_READ_BEGIN,
+	MPC_I2C_ACTION_READ_BYTE,
+	MPC_I2C_ACTION_WRITE,
+	MPC_I2C_ACTION_STOP,
+
+	__MPC_I2C_ACTION_CNT
+};
+
+static char *action_str[] = {
+	"invalid",
+	"start",
+	"restart",
+	"read begin",
+	"read",
+	"write",
+	"stop",
+};
+
+static_assert(ARRAY_SIZE(action_str) == __MPC_I2C_ACTION_CNT);
+
 struct mpc_i2c {
 	struct device *dev;
 	void __iomem *base;
 	u32 interrupt;
-	wait_queue_head_t queue;
+	wait_queue_head_t waitq;
+	spinlock_t lock;
 	struct i2c_adapter adap;
 	int irq;
 	u32 real_clk;
@@ -70,6 +90,16 @@  struct mpc_i2c {
 	u8 fdr, dfsrr;
 #endif
 	struct clk *clk_per;
+	u32 cntl_bits;
+	enum mpc_i2c_action action;
+	struct i2c_msg *msgs;
+	int num_msgs;
+	int curr_msg;
+	u32 byte_posn;
+	u32 block;
+	int rc;
+	int expect_rxack;
+
 };
 
 struct mpc_i2c_divider {
@@ -86,19 +116,6 @@  static inline void writeccr(struct mpc_i2c *i2c, u32 x)
 	writeb(x, i2c->base + MPC_I2C_CR);
 }
 
-static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
-{
-	struct mpc_i2c *i2c = dev_id;
-	if (readb(i2c->base + MPC_I2C_SR) & CSR_MIF) {
-		/* Read again to allow register to stabilise */
-		i2c->interrupt = readb(i2c->base + MPC_I2C_SR);
-		writeb(0, i2c->base + MPC_I2C_SR);
-		wake_up(&i2c->queue);
-		return IRQ_HANDLED;
-	}
-	return IRQ_NONE;
-}
-
 /* Sometimes 9th clock pulse isn't generated, and slave doesn't release
  * the bus, because it wants to send ACK.
  * Following sequence of enabling/disabling and sending start/stop generates
@@ -121,45 +138,6 @@  static void mpc_i2c_fixup(struct mpc_i2c *i2c)
 	}
 }
 
-static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
-{
-	u32 cmd_err;
-	int result;
-
-	result = wait_event_timeout(i2c->queue,
-			(i2c->interrupt & CSR_MIF), timeout);
-
-	if (unlikely(!(i2c->interrupt & CSR_MIF))) {
-		dev_dbg(i2c->dev, "wait timeout\n");
-		writeccr(i2c, 0);
-		result = -ETIMEDOUT;
-	}
-
-	cmd_err = i2c->interrupt;
-	i2c->interrupt = 0;
-
-	if (result < 0)
-		return result;
-
-	if (!(cmd_err & CSR_MCF)) {
-		dev_dbg(i2c->dev, "unfinished\n");
-		return -EIO;
-	}
-
-	if (cmd_err & CSR_MAL) {
-		dev_dbg(i2c->dev, "MAL\n");
-		return -EAGAIN;
-	}
-
-	if (writing && (cmd_err & CSR_RXAK)) {
-		dev_dbg(i2c->dev, "No RXAK\n");
-		/* generate stop */
-		writeccr(i2c, CCR_MEN);
-		return -ENXIO;
-	}
-	return 0;
-}
-
 #if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x)
 static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
@@ -434,168 +412,209 @@  static void mpc_i2c_setup_8xxx(struct device_node *node,
 }
 #endif /* CONFIG_FSL_SOC */
 
-static void mpc_i2c_start(struct mpc_i2c *i2c)
+static void mpc_i2c_finish(struct mpc_i2c *i2c, int rc)
 {
-	/* Clear arbitration */
-	writeb(0, i2c->base + MPC_I2C_SR);
-	/* Start with MEN */
-	writeccr(i2c, CCR_MEN);
+	i2c->rc = rc;
+	i2c->block = 0;
+	i2c->cntl_bits = CCR_MEN;
+	writeccr(i2c, i2c->cntl_bits);
+	wake_up(&i2c->waitq);
 }
 
-static void mpc_i2c_stop(struct mpc_i2c *i2c)
+static void mpc_i2c_do_action(struct mpc_i2c *i2c)
 {
-	writeccr(i2c, CCR_MEN);
-}
+	struct i2c_msg *msg = &i2c->msgs[i2c->curr_msg];
+	int dir = 0;
+	int recv_len = 0;
+	u8 byte;
+
+	dev_dbg(i2c->dev, "%s: action = %s\n", __func__,
+		action_str[i2c->action]);
+
+	i2c->cntl_bits &= ~(CCR_RSTA | CCR_MTX | CCR_TXAK);
+
+	if (msg->flags & I2C_M_RD)
+		dir = 1;
+	if (msg->flags & I2C_M_RECV_LEN)
+		recv_len = 1;
+
+	switch (i2c->action) {
+	case MPC_I2C_ACTION_RESTART:
+		i2c->cntl_bits |= CCR_RSTA;
+		fallthrough;
+
+	case MPC_I2C_ACTION_START:
+		i2c->cntl_bits |= CCR_MSTA | CCR_MTX;
+		writeccr(i2c, i2c->cntl_bits);
+		writeb((msg->addr << 1) | dir, i2c->base + MPC_I2C_DR);
+		i2c->expect_rxack = 1;
+		i2c->action = dir ? MPC_I2C_ACTION_READ_BEGIN : MPC_I2C_ACTION_WRITE;
+		break;
+
+	case MPC_I2C_ACTION_READ_BEGIN:
+		if (msg->len) {
+			if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
+				i2c->cntl_bits |= CCR_TXAK;
+
+			writeccr(i2c, i2c->cntl_bits);
+			/* Dummy read */
+			readb(i2c->base + MPC_I2C_DR);
+		}
+		i2c->action = MPC_I2C_ACTION_READ_BYTE;
+		break;
 
-static int mpc_write(struct mpc_i2c *i2c, int target,
-		     const u8 *data, int length, int restart)
-{
-	int i, result;
-	unsigned timeout = i2c->adap.timeout;
-	u32 flags = restart ? CCR_RSTA : 0;
+	case MPC_I2C_ACTION_READ_BYTE:
+		if (i2c->byte_posn || !recv_len) {
+			/* Generate txack on next to last byte */
+			if (i2c->byte_posn == msg->len - 2)
+				i2c->cntl_bits |= CCR_TXAK;
+			/* Do not generate stop on last byte */
+			if (i2c->byte_posn == msg->len - 1)
+				i2c->cntl_bits |= CCR_MTX;
 
-	/* Start as master */
-	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags);
-	/* Write target byte */
-	writeb((target << 1), i2c->base + MPC_I2C_DR);
+			writeccr(i2c, i2c->cntl_bits);
+		}
 
-	result = i2c_wait(i2c, timeout, 1);
-	if (result < 0)
-		return result;
+		byte = readb(i2c->base + MPC_I2C_DR);
 
-	for (i = 0; i < length; i++) {
-		/* Write data byte */
-		writeb(data[i], i2c->base + MPC_I2C_DR);
+		if (i2c->byte_posn == 0 && recv_len) {
+			if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX) {
+				mpc_i2c_finish(i2c, -EPROTO);
+				return;
+			}
+			msg->len += byte;
+			/*
+			 * For block reads, generate txack here if data length
+			 * is 1 byte (total length is 2 bytes).
+			 */
+			if (msg->len == 2) {
+				i2c->cntl_bits |= CCR_TXAK;
+				writeccr(i2c, i2c->cntl_bits);
+			}
+		}
 
-		result = i2c_wait(i2c, timeout, 1);
-		if (result < 0)
-			return result;
+		dev_dbg(i2c->dev, "%s: %s %02x\n", __func__,
+			action_str[i2c->action], byte);
+		msg->buf[i2c->byte_posn++] = byte;
+		break;
+
+	case MPC_I2C_ACTION_WRITE:
+		dev_dbg(i2c->dev, "%s: %s %02x\n", __func__,
+			action_str[i2c->action], msg->buf[i2c->byte_posn]);
+		writeb(msg->buf[i2c->byte_posn++], i2c->base + MPC_I2C_DR);
+		i2c->expect_rxack = 1;
+		break;
+
+	case MPC_I2C_ACTION_STOP:
+		mpc_i2c_finish(i2c, 0);
+		break;
+
+	case MPC_I2C_ACTION_INVALID:
+	default:
+		BUG();
+		break;
 	}
 
-	return 0;
+	if (msg->len == i2c->byte_posn) {
+		i2c->curr_msg++;
+		i2c->byte_posn = 0;
+
+		if (i2c->curr_msg == i2c->num_msgs) {
+			i2c->action = MPC_I2C_ACTION_STOP;
+			/*
+			 * We don't get another interrupt on read so
+			 * finish the transfer now
+			 */
+			if (dir)
+				mpc_i2c_finish(i2c, 0);
+		} else {
+			i2c->action = MPC_I2C_ACTION_RESTART;
+		}
+	}
 }
 
-static int mpc_read(struct mpc_i2c *i2c, int target,
-		    u8 *data, int length, int restart, bool recv_len)
+static void mpc_i2c_do_intr(struct mpc_i2c *i2c, u8 status)
 {
-	unsigned timeout = i2c->adap.timeout;
-	int i, result;
-	u32 flags = restart ? CCR_RSTA : 0;
+	unsigned long flags;
 
-	/* Switch to read - restart */
-	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags);
-	/* Write target address byte - this time with the read flag set */
-	writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);
+	spin_lock_irqsave(&i2c->lock, flags);
 
-	result = i2c_wait(i2c, timeout, 1);
-	if (result < 0)
-		return result;
+	if (!(status & CSR_MCF)) {
+		dev_dbg(i2c->dev, "unfinished\n");
+		mpc_i2c_finish(i2c, -EIO);
+		goto out;
+	}
 
-	if (length) {
-		if (length == 1 && !recv_len)
-			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK);
-		else
-			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA);
-		/* Dummy read */
-		readb(i2c->base + MPC_I2C_DR);
+	if (status & CSR_MAL) {
+		dev_dbg(i2c->dev, "arbiritration lost\n");
+		mpc_i2c_finish(i2c, -EAGAIN);
+		goto out;
 	}
 
-	for (i = 0; i < length; i++) {
-		u8 byte;
+	if (i2c->expect_rxack && (status & CSR_RXAK)) {
+		dev_dbg(i2c->dev, "no RXAK\n");
+		mpc_i2c_finish(i2c, -ENXIO);
+		goto out;
+	}
+	i2c->expect_rxack = 0;
 
-		result = i2c_wait(i2c, timeout, 0);
-		if (result < 0)
-			return result;
+	mpc_i2c_do_action(i2c);
 
-		/*
-		 * For block reads, we have to know the total length (1st byte)
-		 * before we can determine if we are done.
-		 */
-		if (i || !recv_len) {
-			/* Generate txack on next to last byte */
-			if (i == length - 2)
-				writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA
-					 | CCR_TXAK);
-			/* Do not generate stop on last byte */
-			if (i == length - 1)
-				writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA
-					 | CCR_MTX);
-		}
+out:
+	spin_unlock_irqrestore(&i2c->lock, flags);
+}
 
-		byte = readb(i2c->base + MPC_I2C_DR);
+static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
+{
+	struct mpc_i2c *i2c = dev_id;
+	u8 status = readb(i2c->base + MPC_I2C_SR);
 
-		/*
-		 * Adjust length if first received byte is length.
-		 * The length is 1 length byte plus actually data length
-		 */
-		if (i == 0 && recv_len) {
-			if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX)
-				return -EPROTO;
-			length += byte;
-			/*
-			 * For block reads, generate txack here if data length
-			 * is 1 byte (total length is 2 bytes).
-			 */
-			if (length == 2)
-				writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA
-					 | CCR_TXAK);
-		}
-		data[i] = byte;
+	if (status & CSR_MIF) {
+		writeb(0, i2c->base + MPC_I2C_SR);
+		mpc_i2c_do_intr(i2c, status);
+		return IRQ_HANDLED;
 	}
+	return IRQ_NONE;
+}
+
+static void mpc_i2c_wait_for_completion(struct mpc_i2c *i2c)
+{
+	long time_left;
 
-	return length;
+	time_left = wait_event_timeout(i2c->waitq, !i2c->block, i2c->adap.timeout);
+
+	if (!time_left)
+		i2c->rc = -ETIMEDOUT;
+	else if (time_left < 0)
+		i2c->rc = time_left;
 }
 
-static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+static int mpc_i2c_execute_msg(struct mpc_i2c *i2c)
 {
-	struct i2c_msg *pmsg;
-	int i;
-	int ret = 0;
-	unsigned long orig_jiffies = jiffies;
-	struct mpc_i2c *i2c = i2c_get_adapdata(adap);
+	unsigned long orig_jiffies;
+	unsigned long flags;
 
-	mpc_i2c_start(i2c);
+	spin_lock_irqsave(&i2c->lock, flags);
 
-	/* Allow bus up to 1s to become not busy */
-	while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
-		if (signal_pending(current)) {
-			dev_dbg(i2c->dev, "Interrupted\n");
-			writeccr(i2c, 0);
-			return -EINTR;
-		}
-		if (time_after(jiffies, orig_jiffies + HZ)) {
-			u8 status = readb(i2c->base + MPC_I2C_SR);
+	i2c->curr_msg = 0;
+	i2c->rc = 0;
+	i2c->byte_posn = 0;
+	i2c->block = 1;
+	i2c->action = MPC_I2C_ACTION_START;
 
-			dev_dbg(i2c->dev, "timeout\n");
-			if ((status & (CSR_MCF | CSR_MBB | CSR_RXAK)) != 0) {
-				writeb(status & ~CSR_MAL,
-				       i2c->base + MPC_I2C_SR);
-				i2c_recover_bus(&i2c->adap);
-			}
-			return -EIO;
-		}
-		schedule();
-	}
+	i2c->cntl_bits = CCR_MEN | CCR_MIEN;
+	writeb(0, i2c->base + MPC_I2C_SR);
+	writeccr(i2c, i2c->cntl_bits);
+
+	mpc_i2c_do_action(i2c);
+
+	spin_unlock_irqrestore(&i2c->lock, flags);
+
+	mpc_i2c_wait_for_completion(i2c);
+
+	if (i2c->rc == -EIO || i2c->rc == -EAGAIN || i2c->rc == -ETIMEDOUT)
+		i2c_recover_bus(&i2c->adap);
 
-	for (i = 0; ret >= 0 && i < num; i++) {
-		pmsg = &msgs[i];
-		dev_dbg(i2c->dev,
-			"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
-			pmsg->flags & I2C_M_RD ? "read" : "write",
-			pmsg->len, pmsg->addr, i + 1, num);
-		if (pmsg->flags & I2C_M_RD) {
-			bool recv_len = pmsg->flags & I2C_M_RECV_LEN;
-
-			ret = mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i,
-				       recv_len);
-			if (recv_len && ret > 0)
-				pmsg->len = ret;
-		} else {
-			ret =
-			    mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
-		}
-	}
-	mpc_i2c_stop(i2c); /* Initiate STOP */
 	orig_jiffies = jiffies;
 	/* Wait until STOP is seen, allow up to 1 s */
 	while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
@@ -612,7 +631,35 @@  static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 		}
 		cond_resched();
 	}
-	return (ret < 0) ? ret : num;
+
+	return i2c->rc;
+}
+
+static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	int rc, ret = num;
+	struct mpc_i2c *i2c = i2c_get_adapdata(adap);
+	int i;
+
+	dev_dbg(i2c->dev, "%s: num = %d\n", __func__, num);
+	for (i = 0; i < num; i++)
+		dev_dbg(i2c->dev, "  addr = %02x, flags = %02x, len = %d, %*ph\n",
+			msgs[i].addr, msgs[i].flags, msgs[i].len,
+			msgs[i].flags & I2C_M_RD ? 0 : msgs[i].len,
+			msgs[i].buf);
+
+	BUG_ON(i2c->msgs != NULL);
+	i2c->msgs = msgs;
+	i2c->num_msgs = num;
+
+	rc = mpc_i2c_execute_msg(i2c);
+	if (rc < 0)
+		ret = rc;
+
+	i2c->num_msgs = 0;
+	i2c->msgs = NULL;
+
+	return ret;
 }
 
 static u32 mpc_functionality(struct i2c_adapter *adap)
@@ -667,7 +714,8 @@  static int fsl_i2c_probe(struct platform_device *op)
 
 	i2c->dev = &op->dev; /* for debug and error output */
 
-	init_waitqueue_head(&i2c->queue);
+	init_waitqueue_head(&i2c->waitq);
+	spin_lock_init(&i2c->lock);
 
 	i2c->base = devm_platform_ioremap_resource(op, 0);
 	if (IS_ERR(i2c->base)) {