Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1408694 > unrolled thread

[PATCH v2 00/10] i2c-i801: Various cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/10] i2c-i801: Various cleanups minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 08/10] i2c-i801: Handle a protocol error in byte-by-byte isr minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 09/10] i2c-i801: Null isr data buffer when done with it minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 04/10] i2c-i801: Consolidate calls to i801_check_pre() minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 06/10] i2c-i801: Pass around a boolean read/write variable minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 05/10] i2c-i801: Consolidate calls to i801_check_post minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 02/10] i2c-i801: Move hostcfg set/reset to i801_access() minyard@acm.org - 2016-05-30 03:20 +0200
    [PATCH v2 01/10] i2c-i801: Remove hwpec from block byte-by-byte function minyard@acm.org - 2016-05-30 03:20 +0200

#1408694 — [PATCH v2 00/10] i2c-i801: Various cleanups

This is a new set that is fairly different than the old set, though it
fixes a lot of the same issues, and some new ones I noticed.

I did not do the consolidation of the isr and non-isr byte-by-byte
handling, based on Jean's comments.

I tested this on qemu (including returning a bad number of bytes to
cause a protocol error) with all four combinations of block/byte-by-byte
and interrupts/polled.

I also tested all those combinations on an Intel Chesnee board.

I do not know aobut hwpec, so I assumed it doesn't work on byte-by-byte
transactions.  I'm not quite sure how to test that.

Feel free to say if you like or don't like certain changes, and I
will rework as necessary, of course.

-corey

[toc] | [next] | [standalone]

#1408695 — [PATCH v2 08/10] i2c-i801: Handle a protocol error in byte-by-byte isr

From: Corey Minyard <cminyard@mvista.com>

If a bad number of bytes is read on a transaction, have it
report an error and return -EPROTO for the transaction.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b415948..70da60a 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -230,7 +230,7 @@ struct i801_priv {
 
 	/* isr processing */
 	wait_queue_head_t waitq;
-	u8 status;
+	int status;
 
 	/* Command state used by isr for byte-by-byte block transactions */
 	u8 subcmd;
@@ -472,16 +472,18 @@ static void i801_isr_byte_done(struct i801_priv *priv)
 		    (priv->count == 0)) {
 			priv->len = inb_p(SMBHSTDAT0(priv));
 			if (priv->len < 1 || priv->len > I2C_SMBUS_BLOCK_MAX) {
-				dev_err(&priv->pci_dev->dev,
-					"Illegal SMBus block read size %d\n",
-					priv->len);
-				/* FIXME: Recover */
-				priv->len = I2C_SMBUS_BLOCK_MAX;
-			} else {
-				dev_dbg(&priv->pci_dev->dev,
-					"SMBus block read size is %d\n",
-					priv->len);
+				/*
+				 * Terminate the transaction here, to avoid
+				 * getting any more interrupts that might
+				 * hold us in an interrupt loop forever.
+				 */
+				outb_p(inb_p(SMBHSTCNT(priv)) | SMBHSTCNT_KILL,
+				       SMBHSTCNT(priv));
+				priv->status = -EPROTO;
+				return;
 			}
+			dev_dbg(&priv->pci_dev->dev,
+				"SMBus block read size is %d\n", priv->len);
 			priv->data[-1] = priv->len;
 		}
 
@@ -541,7 +543,10 @@ static irqreturn_t i801_isr(int irq, void *dev_id)
 	status &= SMBHSTSTS_INTR | STATUS_ERROR_FLAGS;
 	if (status) {
 		outb_p(status, SMBHSTSTS(priv));
-		priv->status |= status;
+		if (status < 0)
+			priv->status = status;
+		else
+			priv->status |= status;
 		wake_up(&priv->waitq);
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1408696 — [PATCH v2 09/10] i2c-i801: Null isr data buffer when done with it

From: Corey Minyard <cminyard@mvista.com>

Don't leave a pointer to some external buffer lying around.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 70da60a..bb15356 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -598,6 +598,7 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 			dev_warn(&priv->pci_dev->dev,
 				 "Timeout waiting for interrupt!\n");
 		}
+		priv->data = NULL;
 		priv->status = 0;
 		return status;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1408697 — [PATCH v2 04/10] i2c-i801: Consolidate calls to i801_check_pre()

From: Corey Minyard <cminyard@mvista.com>

It was always done before starting the transaction, so do it
in common code before the transaction start.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 222be9c..8794e70 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -402,10 +402,6 @@ static int i801_transaction(struct i801_priv *priv, int xact)
 	int result;
 	const struct i2c_adapter *adap = &priv->adapter;
 
-	result = i801_check_pre(priv);
-	if (result < 0)
-		return result;
-
 	if (priv->features & FEATURE_IRQ) {
 		outb_p(xact | SMBHSTCNT_INTREN | SMBHSTCNT_START,
 		       SMBHSTCNT(priv));
@@ -562,10 +558,6 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 	int result;
 	const struct i2c_adapter *adap = &priv->adapter;
 
-	result = i801_check_pre(priv);
-	if (result < 0)
-		return result;
-
 	len = data->block[0];
 
 	if (read_write == I2C_SMBUS_WRITE) {
@@ -789,6 +781,10 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		goto out;
 	}
 
+	ret = i801_check_pre(priv);
+	if (ret < 0)
+		goto out;
+
 	if (block)
 		ret = i801_block_transaction(priv, flags, data, read_write,
 					     size);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1408698 — [PATCH v2 06/10] i2c-i801: Pass around a boolean read/write variable

From: Corey Minyard <cminyard@mvista.com>

The code was passing around read_write, which required comparison
with a constant, but was effectively a bool.  Pass around an
is_read bool instead.  This also makes it consistent with the
priv->is_read used for interrupt handling.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 51 ++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 56db310..ae1e60a 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -432,7 +432,7 @@ static int i801_transaction(struct i801_priv *priv, int xact)
 
 static int i801_block_transaction_by_block(struct i801_priv *priv,
 					   union i2c_smbus_data *data,
-					   char read_write, int hwpec)
+					   bool is_read, int hwpec)
 {
 	int i, len;
 	int status;
@@ -440,7 +440,7 @@ static int i801_block_transaction_by_block(struct i801_priv *priv,
 	inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
 
 	/* Use 32-byte buffer to process this transaction */
-	if (read_write == I2C_SMBUS_WRITE) {
+	if (!is_read) {
 		len = data->block[0];
 		outb_p(len, SMBHSTDAT0(priv));
 		for (i = 0; i < len; i++)
@@ -452,7 +452,7 @@ static int i801_block_transaction_by_block(struct i801_priv *priv,
 	if (status < 0 || status & STATUS_ERROR_FLAGS)
 		return status;
 
-	if (read_write == I2C_SMBUS_READ) {
+	if (is_read) {
 		len = priv->len = inb_p(SMBHSTDAT0(priv));
 		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
 			return -EPROTO;
@@ -555,7 +555,7 @@ static irqreturn_t i801_isr(int irq, void *dev_id)
  */
 static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 					       union i2c_smbus_data *data,
-					       char read_write, int command)
+					       bool is_read, int command)
 {
 	int i, len;
 	int smbcmd;
@@ -565,20 +565,19 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 
 	len = data->block[0];
 
-	if (read_write == I2C_SMBUS_WRITE) {
+	if (!is_read) {
 		outb_p(len, SMBHSTDAT0(priv));
 		outb_p(data->block[1], SMBBLKDAT(priv));
 	}
 
-	if (command == I2C_SMBUS_I2C_BLOCK_DATA &&
-	    read_write == I2C_SMBUS_READ)
+	if (command == I2C_SMBUS_I2C_BLOCK_DATA && is_read)
 		smbcmd = I801_I2C_BLOCK_DATA;
 	else
 		smbcmd = I801_BLOCK_DATA;
 
 	if (priv->features & FEATURE_IRQ) {
-		priv->is_read = (read_write == I2C_SMBUS_READ);
-		if (len == 1 && priv->is_read)
+		priv->is_read = is_read;
+		if (len == 1 && is_read)
 			smbcmd |= SMBHSTCNT_LAST_BYTE;
 		priv->cmd = smbcmd | SMBHSTCNT_INTREN;
 		priv->len = len;
@@ -599,7 +598,7 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 	}
 
 	for (i = 1; i <= len; i++) {
-		if (i == len && read_write == I2C_SMBUS_READ)
+		if (i == len && is_read)
 			smbcmd |= SMBHSTCNT_LAST_BYTE;
 		outb_p(smbcmd, SMBHSTCNT(priv));
 
@@ -611,7 +610,7 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 		if (status)
 			return status;
 
-		if (i == 1 && read_write == I2C_SMBUS_READ
+		if (i == 1 && is_read
 		 && command != I2C_SMBUS_I2C_BLOCK_DATA) {
 			priv->len = inb_p(SMBHSTDAT0(priv));
 			if (priv->len < 1 || priv->len > I2C_SMBUS_BLOCK_MAX)
@@ -620,9 +619,9 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 		}
 
 		/* Retrieve/store value in SMBBLKDAT */
-		if (read_write == I2C_SMBUS_READ)
+		if (is_read)
 			data->block[i] = inb_p(SMBBLKDAT(priv));
-		if (read_write == I2C_SMBUS_WRITE && i+1 <= len)
+		else if (i+1 <= len)
 			outb_p(data->block[i+1], SMBBLKDAT(priv));
 
 		/* signals SMBBLKDAT ready */
@@ -642,7 +641,7 @@ static int i801_set_block_buffer_mode(struct i801_priv *priv)
 
 /* Block transaction function */
 static int i801_block_transaction(struct i801_priv *priv, unsigned short flags,
-				  union i2c_smbus_data *data, char read_write,
+				  union i2c_smbus_data *data, bool is_read,
 				  int command)
 {
 	int result = 0;
@@ -658,8 +657,7 @@ static int i801_block_transaction(struct i801_priv *priv, unsigned short flags,
 		outb_p(inb_p(SMBAUXCTL(priv)) & (~SMBAUXCTL_CRC),
 		       SMBAUXCTL(priv));
 
-	if (read_write == I2C_SMBUS_WRITE
-	 || command == I2C_SMBUS_I2C_BLOCK_DATA) {
+	if (!is_read || command == I2C_SMBUS_I2C_BLOCK_DATA) {
 		if (data->block[0] < 1)
 			data->block[0] = 1;
 		if (data->block[0] > I2C_SMBUS_BLOCK_MAX)
@@ -674,12 +672,11 @@ static int i801_block_transaction(struct i801_priv *priv, unsigned short flags,
 	if ((priv->features & FEATURE_BLOCK_BUFFER)
 	 && command != I2C_SMBUS_I2C_BLOCK_DATA
 	 && i801_set_block_buffer_mode(priv) == 0)
-		result = i801_block_transaction_by_block(priv, data,
-							 read_write, hwpec);
+		result = i801_block_transaction_by_block(priv, data, is_read,
+							 hwpec);
 	else
 		result = i801_block_transaction_byte_by_byte(priv, data,
-							     read_write,
-							     command);
+							     is_read, command);
 
 	/*
 	 * Some BIOSes don't like it when PEC is enabled at reboot or
@@ -702,6 +699,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 	int block = 0;
 	int ret = 0, xact = 0;
 	int hostc = -1;
+	bool is_read = (read_write == I2C_SMBUS_READ);
 	struct i801_priv *priv = i2c_get_adapdata(adap);
 
 	pm_runtime_get_sync(&priv->pci_dev->dev);
@@ -715,7 +713,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 	case I2C_SMBUS_BYTE:
 		outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
 		       SMBHSTADD(priv));
-		if (read_write == I2C_SMBUS_WRITE)
+		if (!is_read)
 			outb_p(command, SMBHSTCMD(priv));
 		xact = I801_BYTE;
 		break;
@@ -723,7 +721,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
 		       SMBHSTADD(priv));
 		outb_p(command, SMBHSTCMD(priv));
-		if (read_write == I2C_SMBUS_WRITE)
+		if (!is_read)
 			outb_p(data->byte, SMBHSTDAT0(priv));
 		xact = I801_BYTE_DATA;
 		break;
@@ -731,7 +729,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
 		       SMBHSTADD(priv));
 		outb_p(command, SMBHSTCMD(priv));
-		if (read_write == I2C_SMBUS_WRITE) {
+		if (!is_read) {
 			outb_p(data->word & 0xff, SMBHSTDAT0(priv));
 			outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
 		}
@@ -747,7 +745,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		/* NB: page 240 of ICH5 datasheet shows that the R/#W
 		 * bit should be cleared here, even when reading */
 		outb_p((addr & 0x7f) << 1, SMBHSTADD(priv));
-		if (read_write == I2C_SMBUS_WRITE) {
+		if (!is_read) {
 			unsigned char thostc;
 
 			outb_p(command, SMBHSTCMD(priv));
@@ -779,8 +777,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		goto out;
 
 	if (block)
-		ret = i801_block_transaction(priv, flags, data, read_write,
-					     size);
+		ret = i801_block_transaction(priv, flags, data, is_read, size);
 	else
 		ret = i801_transaction(priv, xact);
 
@@ -797,7 +794,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		goto out;
 	if (ret)
 		goto out;
-	if ((read_write == I2C_SMBUS_WRITE) || (xact == I801_QUICK))
+	if (!is_read || (xact == I801_QUICK))
 		goto out;
 
 	switch (xact & 0x7f) {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1408699 — [PATCH v2 05/10] i2c-i801: Consolidate calls to i801_check_post

From: Corey Minyard <cminyard@mvista.com>

This was almost always called at the end of the transaction.  The
only time it wasn't called was when a protocol violation occurred,
and the error reporting was inconsistent there.

So have the transaction functions return positive status or a
negative error if they detect an issue.  If a protocol violation
does occur, always do a kill operation on the bus instead of just
trying to finish the operation.  If there was a hardware issue,
the code that was there to terminate the transaction could loop
forever, and the kill seems more appropriate if something goes
grossly wrong.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 51 +++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 8794e70..56db310 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -317,7 +317,13 @@ static int i801_check_post(struct i801_priv *priv, int status)
 	 * DEV_ERR.
 	 */
 	if (unlikely(status < 0)) {
-		dev_err(&priv->pci_dev->dev, "Transaction timeout\n");
+		result = status;
+		if (status == -EPROTO)
+			dev_err(&priv->pci_dev->dev,
+				"Illegal SMBus block read size %d\n",
+				priv->len);
+		else
+			dev_err(&priv->pci_dev->dev, "Transaction timeout\n");
 		/* try to stop the current command */
 		dev_dbg(&priv->pci_dev->dev, "Terminating the current operation\n");
 		outb_p(inb_p(SMBHSTCNT(priv)) | SMBHSTCNT_KILL,
@@ -333,7 +339,7 @@ static int i801_check_post(struct i801_priv *priv, int status)
 			dev_err(&priv->pci_dev->dev,
 				"Failed terminating the transaction\n");
 		outb_p(STATUS_FLAGS, SMBHSTSTS(priv));
-		return -ETIMEDOUT;
+		return result;
 	}
 
 	if (status & SMBHSTSTS_FAILED) {
@@ -414,15 +420,14 @@ static int i801_transaction(struct i801_priv *priv, int xact)
 				 "Timeout waiting for interrupt!\n");
 		}
 		priv->status = 0;
-		return i801_check_post(priv, status);
+		return status;
 	}
 
 	/* the current contents of SMBHSTCNT can be overwritten, since PEC,
 	 * SMBSCMD are passed in xact */
 	outb_p(xact | SMBHSTCNT_START, SMBHSTCNT(priv));
 
-	status = i801_wait_intr(priv);
-	return i801_check_post(priv, status);
+	return i801_wait_intr(priv);
 }
 
 static int i801_block_transaction_by_block(struct i801_priv *priv,
@@ -444,11 +449,11 @@ static int i801_block_transaction_by_block(struct i801_priv *priv,
 
 	status = i801_transaction(priv, I801_BLOCK_DATA |
 				  (hwpec ? SMBHSTCNT_PEC_EN : 0));
-	if (status)
+	if (status < 0 || status & STATUS_ERROR_FLAGS)
 		return status;
 
 	if (read_write == I2C_SMBUS_READ) {
-		len = inb_p(SMBHSTDAT0(priv));
+		len = priv->len = inb_p(SMBHSTDAT0(priv));
 		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
 			return -EPROTO;
 
@@ -456,7 +461,7 @@ static int i801_block_transaction_by_block(struct i801_priv *priv,
 		for (i = 0; i < len; i++)
 			data->block[i + 1] = inb_p(SMBBLKDAT(priv));
 	}
-	return 0;
+	return status;
 }
 
 static void i801_isr_byte_done(struct i801_priv *priv)
@@ -590,7 +595,7 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 				 "Timeout waiting for interrupt!\n");
 		}
 		priv->status = 0;
-		return i801_check_post(priv, status);
+		return status;
 	}
 
 	for (i = 1; i <= len; i++) {
@@ -604,24 +609,14 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 
 		status = i801_wait_byte_done(priv);
 		if (status)
-			goto exit;
+			return status;
 
 		if (i == 1 && read_write == I2C_SMBUS_READ
 		 && command != I2C_SMBUS_I2C_BLOCK_DATA) {
-			len = inb_p(SMBHSTDAT0(priv));
-			if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
-				dev_err(&priv->pci_dev->dev,
-					"Illegal SMBus block read size %d\n",
-					len);
-				/* Recover */
-				while (inb_p(SMBHSTSTS(priv)) &
-				       SMBHSTSTS_HOST_BUSY)
-					outb_p(SMBHSTSTS_BYTE_DONE,
-					       SMBHSTSTS(priv));
-				outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
+			priv->len = inb_p(SMBHSTDAT0(priv));
+			if (priv->len < 1 || priv->len > I2C_SMBUS_BLOCK_MAX)
 				return -EPROTO;
-			}
-			data->block[0] = len;
+			data->block[0] = len = priv->len;
 		}
 
 		/* Retrieve/store value in SMBBLKDAT */
@@ -634,9 +629,7 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 		outb_p(SMBHSTSTS_BYTE_DONE, SMBHSTSTS(priv));
 	}
 
-	status = i801_wait_intr(priv);
-exit:
-	return i801_check_post(priv, status);
+	return i801_wait_intr(priv);
 }
 
 static int i801_set_block_buffer_mode(struct i801_priv *priv)
@@ -791,6 +784,12 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 	else
 		ret = i801_transaction(priv, xact);
 
+	/*
+	 * ret can be status if positive or an error if negative.  Let
+	 * the post check handle it.
+	 */
+	ret = i801_check_post(priv, ret);
+
 	if (hostc >= 0)
 		pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1408700 — [PATCH v2 02/10] i2c-i801: Move hostcfg set/reset to i801_access()

From: Corey Minyard <cminyard@mvista.com>

The HSTCFG register save/restore was done in i2c_block_transaction,
but all the checks were already done in i801_access, so move it into
those checks.

This results in a small savings of code, and moves some special
handing for I2C transactions into code that is already handling
special things for I2C transactions.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 818c0c8..205f9d0 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -661,20 +661,6 @@ static int i801_block_transaction(struct i801_priv *priv,
 				  int command, int hwpec)
 {
 	int result = 0;
-	unsigned char hostc;
-
-	if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
-		if (read_write == I2C_SMBUS_WRITE) {
-			/* set I2C_EN bit in configuration register */
-			pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &hostc);
-			pci_write_config_byte(priv->pci_dev, SMBHSTCFG,
-					      hostc | SMBHSTCFG_I2C_EN);
-		} else if (!(priv->features & FEATURE_I2C_BLOCK_READ)) {
-			dev_err(&priv->pci_dev->dev,
-				"I2C block read is unsupported!\n");
-			return -EOPNOTSUPP;
-		}
-	}
 
 	if (read_write == I2C_SMBUS_WRITE
 	 || command == I2C_SMBUS_I2C_BLOCK_DATA) {
@@ -699,11 +685,6 @@ static int i801_block_transaction(struct i801_priv *priv,
 							     read_write,
 							     command);
 
-	if (command == I2C_SMBUS_I2C_BLOCK_DATA
-	 && read_write == I2C_SMBUS_WRITE) {
-		/* restore saved configuration register value */
-		pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc);
-	}
 	return result;
 }
 
@@ -715,6 +696,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 	int hwpec;
 	int block = 0;
 	int ret = 0, xact = 0;
+	int hostc = -1;
 	struct i801_priv *priv = i2c_get_adapdata(adap);
 
 	pm_runtime_get_sync(&priv->pci_dev->dev);
@@ -764,12 +746,24 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		/* NB: page 240 of ICH5 datasheet shows that the R/#W
 		 * bit should be cleared here, even when reading */
 		outb_p((addr & 0x7f) << 1, SMBHSTADD(priv));
-		if (read_write == I2C_SMBUS_READ) {
+		if (read_write == I2C_SMBUS_WRITE) {
+			unsigned char thostc;
+
+			outb_p(command, SMBHSTCMD(priv));
+			/* set I2C_EN bit in configuration register */
+			pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &thostc);
+			pci_write_config_byte(priv->pci_dev, SMBHSTCFG,
+					      thostc | SMBHSTCFG_I2C_EN);
+			hostc = thostc;
+		} else if (!(priv->features & FEATURE_I2C_BLOCK_READ)) {
+			dev_err(&priv->pci_dev->dev,
+				"I2C block read is unsupported!\n");
+			ret = -EOPNOTSUPP;
+			goto out;
+		} else
 			/* NB: page 240 of ICH5 datasheet also shows
 			 * that DATA1 is the cmd field when reading */
 			outb_p(command, SMBHSTDAT1(priv));
-		} else
-			outb_p(command, SMBHSTCMD(priv));
 		block = 1;
 		break;
 	default:
@@ -798,6 +792,9 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		outb_p(inb_p(SMBAUXCTL(priv)) &
 		       ~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
 
+	if (hostc >= 0)
+		pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc);
+
 	if (block)
 		goto out;
 	if (ret)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1408701 — [PATCH v2 01/10] i2c-i801: Remove hwpec from block byte-by-byte function

From: Corey Minyard <cminyard@mvista.com>

It's not used in the function, so get rid of it.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 64b1208b..818c0c8 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -554,8 +554,7 @@ static irqreturn_t i801_isr(int irq, void *dev_id)
  */
 static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 					       union i2c_smbus_data *data,
-					       char read_write, int command,
-					       int hwpec)
+					       char read_write, int command)
 {
 	int i, len;
 	int smbcmd;
@@ -698,7 +697,7 @@ static int i801_block_transaction(struct i801_priv *priv,
 	else
 		result = i801_block_transaction_byte_by_byte(priv, data,
 							     read_write,
-							     command, hwpec);
+							     command);
 
 	if (command == I2C_SMBUS_I2C_BLOCK_DATA
 	 && read_write == I2C_SMBUS_WRITE) {
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web