[PATCH v3 0/6] i2c: i801: Series with minor improvements (second half)

[PATCH v3 0/6] i2c: i801: Series with minor improvements (second half)

[Jean Delvare]

This series includes a number of minor improvements, it's a re-send of
patches submitted in December last year already, with minor fixes.

Changes in v3:
* Series is renumbered because original patches 1-4 have been accepted
  already.
* Patch 2 (originally 6): Typo fixed in description.
* Patch 3 (originally 7): Typo fixed in description.
* Patch 4 (originally 8): Reword description, drop stray blank line.

-- 
Jean Delvare
SUSE L3 Support

[PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction()

[Jean Delvare]

From: Heiner Kallweit <hkallweit1@gmail.com>

Factor out non-block pre/post processing to a new function
i801_simple_transaction(), complementing existing function
i801_block_transaction(). This makes i801_access() better readable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-i801.c |   92 +++++++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 37 deletions(-)

--- linux-6.1.orig/drivers/i2c/busses/i2c-i801.c
+++ linux-6.1/drivers/i2c/busses/i2c-i801.c
@@ -732,6 +732,59 @@ static void i801_set_hstadd(struct i801_
 	outb_p((addr << 1) | (read_write & 0x01), SMBHSTADD(priv));
 }
 
+/* Single value transaction function */
+static int i801_simple_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
+				   char read_write, int command)
+{
+	int xact, ret;
+
+	switch (command) {
+	case I2C_SMBUS_QUICK:
+		xact = I801_QUICK;
+		break;
+	case I2C_SMBUS_BYTE:
+		xact = I801_BYTE;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		if (read_write == I2C_SMBUS_WRITE)
+			outb_p(data->byte, SMBHSTDAT0(priv));
+		xact = I801_BYTE_DATA;
+		break;
+	case I2C_SMBUS_WORD_DATA:
+		if (read_write == I2C_SMBUS_WRITE) {
+			outb_p(data->word & 0xff, SMBHSTDAT0(priv));
+			outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
+		}
+		xact = I801_WORD_DATA;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+		outb_p(data->word & 0xff, SMBHSTDAT0(priv));
+		outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
+		xact = I801_PROC_CALL;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	ret = i801_transaction(priv, xact);
+	if (ret || read_write == I2C_SMBUS_WRITE)
+		return ret;
+
+	switch (command) {
+	case I2C_SMBUS_BYTE:
+	case I2C_SMBUS_BYTE_DATA:
+		data->byte = inb_p(SMBHSTDAT0(priv));
+		break;
+	case I2C_SMBUS_WORD_DATA:
+	case I2C_SMBUS_PROC_CALL:
+		data->word = inb_p(SMBHSTDAT0(priv)) +
+			     (inb_p(SMBHSTDAT1(priv)) << 8);
+		break;
+	}
+
+	return 0;
+}
+
 /* Block transaction function */
 static int i801_block_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
 				  char read_write, int command)
@@ -783,9 +836,7 @@ static s32 i801_access(struct i2c_adapte
 		       unsigned short flags, char read_write, u8 command,
 		       int size, union i2c_smbus_data *data)
 {
-	int hwpec;
-	int block = 0;
-	int ret, xact;
+	int hwpec, ret, block = 0;
 	struct i801_priv *priv = i2c_get_adapdata(adap);
 
 	mutex_lock(&priv->acpi_lock);
@@ -803,36 +854,23 @@ static s32 i801_access(struct i2c_adapte
 	switch (size) {
 	case I2C_SMBUS_QUICK:
 		i801_set_hstadd(priv, addr, read_write);
-		xact = I801_QUICK;
 		break;
 	case I2C_SMBUS_BYTE:
 		i801_set_hstadd(priv, addr, read_write);
 		if (read_write == I2C_SMBUS_WRITE)
 			outb_p(command, SMBHSTCMD(priv));
-		xact = I801_BYTE;
 		break;
 	case I2C_SMBUS_BYTE_DATA:
 		i801_set_hstadd(priv, addr, read_write);
 		outb_p(command, SMBHSTCMD(priv));
-		if (read_write == I2C_SMBUS_WRITE)
-			outb_p(data->byte, SMBHSTDAT0(priv));
-		xact = I801_BYTE_DATA;
 		break;
 	case I2C_SMBUS_WORD_DATA:
 		i801_set_hstadd(priv, addr, read_write);
 		outb_p(command, SMBHSTCMD(priv));
-		if (read_write == I2C_SMBUS_WRITE) {
-			outb_p(data->word & 0xff, SMBHSTDAT0(priv));
-			outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
-		}
-		xact = I801_WORD_DATA;
 		break;
 	case I2C_SMBUS_PROC_CALL:
 		i801_set_hstadd(priv, addr, I2C_SMBUS_WRITE);
 		outb_p(command, SMBHSTCMD(priv));
-		outb_p(data->word & 0xff, SMBHSTDAT0(priv));
-		outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
-		xact = I801_PROC_CALL;
 		read_write = I2C_SMBUS_READ;
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
@@ -880,7 +918,7 @@ static s32 i801_access(struct i2c_adapte
 	if (block)
 		ret = i801_block_transaction(priv, data, read_write, size);
 	else
-		ret = i801_transaction(priv, xact);
+		ret = i801_simple_transaction(priv, data, read_write, size);
 
 	/* Some BIOSes don't like it when PEC is enabled at reboot or resume
 	   time, so we forcibly disable it after every transaction. Turn off
@@ -888,26 +926,6 @@ static s32 i801_access(struct i2c_adapte
 	if (hwpec || block)
 		outb_p(inb_p(SMBAUXCTL(priv)) &
 		       ~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
-
-	if (block)
-		goto out;
-	if (ret)
-		goto out;
-	if ((read_write == I2C_SMBUS_WRITE) || (xact == I801_QUICK))
-		goto out;
-
-	switch (xact) {
-	case I801_BYTE:	/* Result put in SMBHSTDAT0 */
-	case I801_BYTE_DATA:
-		data->byte = inb_p(SMBHSTDAT0(priv));
-		break;
-	case I801_WORD_DATA:
-	case I801_PROC_CALL:
-		data->word = inb_p(SMBHSTDAT0(priv)) +
-			     (inb_p(SMBHSTDAT1(priv)) << 8);
-		break;
-	}
-
 out:
 	/*
 	 * Unlock the SMBus device for use by BIOS/ACPI,

-- 
Jean Delvare
SUSE L3 Support

[PATCH v3 2/6] i2c: i801: Handle SMBAUXCTL_E32B in i801_block_transaction_by_block only

[Jean Delvare]

From: Heiner Kallweit <hkallweit1@gmail.com>

Currently we touch SMBAUXCTL even if not needed. That's the case for block
commands that don't use block buffer mode, either because block buffer
mode isn't available or because it's not supported for the respective
command (e.g. I2C block transfer). Improve this by setting/resetting
SMBAUXCTL_E32B in i801_block_transaction_by_block() only.

Small downside is that we now access SMBAUXCTL twice for transactions
that use PEC and block buffer mode. But this should a rather rare case
and the impact is negligible.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
Changes since v2:
 * Typo fixed in description

 drivers/i2c/busses/i2c-i801.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index d934d410b..d7182f7c8 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -511,19 +511,23 @@ static int i801_block_transaction_by_block(struct i801_priv *priv,
 
 	status = i801_transaction(priv, xact);
 	if (status)
-		return status;
+		goto out;
 
 	if (read_write == I2C_SMBUS_READ ||
 	    command == I2C_SMBUS_BLOCK_PROC_CALL) {
 		len = inb_p(SMBHSTDAT0(priv));
-		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
-			return -EPROTO;
+		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
+			status = -EPROTO;
+			goto out;
+		}
 
 		data->block[0] = len;
 		for (i = 0; i < len; i++)
 			data->block[i + 1] = inb_p(SMBBLKDAT(priv));
 	}
-	return 0;
+out:
+	outb_p(inb_p(SMBAUXCTL(priv)) & ~SMBAUXCTL_E32B, SMBAUXCTL(priv));
+	return status;
 }
 
 static void i801_isr_byte_done(struct i801_priv *priv)
@@ -921,11 +925,10 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 		ret = i801_simple_transaction(priv, data, read_write, size);
 
 	/* Some BIOSes don't like it when PEC is enabled at reboot or resume
-	   time, so we forcibly disable it after every transaction. Turn off
-	   E32B for the same reason. */
-	if (hwpec || block)
-		outb_p(inb_p(SMBAUXCTL(priv)) &
-		       ~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
+	 * time, so we forcibly disable it after every transaction.
+	 */
+	if (hwpec)
+		outb_p(inb_p(SMBAUXCTL(priv)) & ~SMBAUXCTL_CRC, SMBAUXCTL(priv));
 out:
 	/*
 	 * Unlock the SMBus device for use by BIOS/ACPI,
-- 
2.39.0



-- 
Jean Delvare
SUSE L3 Support

[PATCH v3 3/6] i2c: i801: Centralize configuring non-block commands in i801_simple_transaction

[Jean Delvare]

From: Heiner Kallweit <hkallweit1@gmail.com>

Currently configuring command register settings is distributed over multiple
functions. At first centralize this for non-block commands in
i801_simple_transaction().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
Changes since v2:
 * Typos fixed in description

 drivers/i2c/busses/i2c-i801.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index d7182f7c8..0d49e9587 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -738,35 +738,47 @@ static void i801_set_hstadd(struct i801_priv *priv, u8 addr, char read_write)
 
 /* Single value transaction function */
 static int i801_simple_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
-				   char read_write, int command)
+				   u8 addr, u8 hstcmd, char read_write, int command)
 {
 	int xact, ret;
 
 	switch (command) {
 	case I2C_SMBUS_QUICK:
+		i801_set_hstadd(priv, addr, read_write);
 		xact = I801_QUICK;
 		break;
 	case I2C_SMBUS_BYTE:
+		i801_set_hstadd(priv, addr, read_write);
+		if (read_write == I2C_SMBUS_WRITE)
+			outb_p(hstcmd, SMBHSTCMD(priv));
 		xact = I801_BYTE;
 		break;
 	case I2C_SMBUS_BYTE_DATA:
+		i801_set_hstadd(priv, addr, read_write);
 		if (read_write == I2C_SMBUS_WRITE)
 			outb_p(data->byte, SMBHSTDAT0(priv));
+		outb_p(hstcmd, SMBHSTCMD(priv));
 		xact = I801_BYTE_DATA;
 		break;
 	case I2C_SMBUS_WORD_DATA:
+		i801_set_hstadd(priv, addr, read_write);
 		if (read_write == I2C_SMBUS_WRITE) {
 			outb_p(data->word & 0xff, SMBHSTDAT0(priv));
 			outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
 		}
+		outb_p(hstcmd, SMBHSTCMD(priv));
 		xact = I801_WORD_DATA;
 		break;
 	case I2C_SMBUS_PROC_CALL:
+		i801_set_hstadd(priv, addr, I2C_SMBUS_WRITE);
 		outb_p(data->word & 0xff, SMBHSTDAT0(priv));
 		outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
+		outb_p(hstcmd, SMBHSTCMD(priv));
+		read_write = I2C_SMBUS_READ;
 		xact = I801_PROC_CALL;
 		break;
 	default:
+		pci_err(priv->pci_dev, "Unsupported transaction %d\n", command);
 		return -EOPNOTSUPP;
 	}
 
@@ -857,25 +869,10 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 
 	switch (size) {
 	case I2C_SMBUS_QUICK:
-		i801_set_hstadd(priv, addr, read_write);
-		break;
 	case I2C_SMBUS_BYTE:
-		i801_set_hstadd(priv, addr, read_write);
-		if (read_write == I2C_SMBUS_WRITE)
-			outb_p(command, SMBHSTCMD(priv));
-		break;
 	case I2C_SMBUS_BYTE_DATA:
-		i801_set_hstadd(priv, addr, read_write);
-		outb_p(command, SMBHSTCMD(priv));
-		break;
 	case I2C_SMBUS_WORD_DATA:
-		i801_set_hstadd(priv, addr, read_write);
-		outb_p(command, SMBHSTCMD(priv));
-		break;
 	case I2C_SMBUS_PROC_CALL:
-		i801_set_hstadd(priv, addr, I2C_SMBUS_WRITE);
-		outb_p(command, SMBHSTCMD(priv));
-		read_write = I2C_SMBUS_READ;
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
 		i801_set_hstadd(priv, addr, read_write);
@@ -922,7 +919,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 	if (block)
 		ret = i801_block_transaction(priv, data, read_write, size);
 	else
-		ret = i801_simple_transaction(priv, data, read_write, size);
+		ret = i801_simple_transaction(priv, data, addr, command, read_write, size);
 
 	/* Some BIOSes don't like it when PEC is enabled at reboot or resume
 	 * time, so we forcibly disable it after every transaction.
-- 
2.39.0



-- 
Jean Delvare
SUSE L3 Support

[PATCH v3 4/6] i2c: i801: Centralize configuring block commands in i801_block_transaction

[Jean Delvare]

From: Heiner Kallweit <hkallweit1@gmail.com>

Similar to what was done for non-block commands, centralize block
command register settings in i801_block_transaction().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
Changes since v2:
 * Reword description
 * Drop stray blank line

 drivers/i2c/busses/i2c-i801.c |   84 +++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 49 deletions(-)

--- linux-6.1.orig/drivers/i2c/busses/i2c-i801.c
+++ linux-6.1/drivers/i2c/busses/i2c-i801.c
@@ -803,7 +803,7 @@ static int i801_simple_transaction(struc
 
 /* Block transaction function */
 static int i801_block_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
-				  char read_write, int command)
+				  u8 addr, u8 hstcmd, char read_write, int command)
 {
 	int result = 0;
 	unsigned char hostc;
@@ -813,7 +813,29 @@ static int i801_block_transaction(struct
 	else if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX)
 		return -EPROTO;
 
-	if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
+	switch (command) {
+	case I2C_SMBUS_BLOCK_DATA:
+		i801_set_hstadd(priv, addr, read_write);
+		outb_p(hstcmd, SMBHSTCMD(priv));
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		/*
+		 * NB: page 240 of ICH5 datasheet shows that the R/#W
+		 * bit should be cleared here, even when reading.
+		 * However if SPD Write Disable is set (Lynx Point and later),
+		 * the read will fail if we don't set the R/#W bit.
+		 */
+		i801_set_hstadd(priv, addr,
+				priv->original_hstcfg & SMBHSTCFG_SPD_WD ?
+				read_write : I2C_SMBUS_WRITE);
+		if (read_write == I2C_SMBUS_READ) {
+			/* NB: page 240 of ICH5 datasheet also shows
+			 * that DATA1 is the cmd field when reading
+			 */
+			outb_p(hstcmd, SMBHSTDAT1(priv));
+		} else
+			outb_p(hstcmd, SMBHSTCMD(priv));
+
 		if (read_write == I2C_SMBUS_WRITE) {
 			/* set I2C_EN bit in configuration register */
 			pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &hostc);
@@ -824,6 +846,12 @@ static int i801_block_transaction(struct
 				"I2C block read is unsupported!\n");
 			return -EOPNOTSUPP;
 		}
+		break;
+	case I2C_SMBUS_BLOCK_PROC_CALL:
+		/* Needs to be flagged as write transaction */
+		i801_set_hstadd(priv, addr, I2C_SMBUS_WRITE);
+		outb_p(hstcmd, SMBHSTCMD(priv));
+		break;
 	}
 
 	/* Experience has shown that the block buffer can only be used for
@@ -852,7 +880,7 @@ static s32 i801_access(struct i2c_adapte
 		       unsigned short flags, char read_write, u8 command,
 		       int size, union i2c_smbus_data *data)
 {
-	int hwpec, ret, block = 0;
+	int hwpec, ret;
 	struct i801_priv *priv = i2c_get_adapdata(adap);
 
 	mutex_lock(&priv->acpi_lock);
@@ -867,57 +895,16 @@ static s32 i801_access(struct i2c_adapte
 		&& size != I2C_SMBUS_QUICK
 		&& size != I2C_SMBUS_I2C_BLOCK_DATA;
 
-	switch (size) {
-	case I2C_SMBUS_QUICK:
-	case I2C_SMBUS_BYTE:
-	case I2C_SMBUS_BYTE_DATA:
-	case I2C_SMBUS_WORD_DATA:
-	case I2C_SMBUS_PROC_CALL:
-		break;
-	case I2C_SMBUS_BLOCK_DATA:
-		i801_set_hstadd(priv, addr, read_write);
-		outb_p(command, SMBHSTCMD(priv));
-		block = 1;
-		break;
-	case I2C_SMBUS_I2C_BLOCK_DATA:
-		/*
-		 * NB: page 240 of ICH5 datasheet shows that the R/#W
-		 * bit should be cleared here, even when reading.
-		 * However if SPD Write Disable is set (Lynx Point and later),
-		 * the read will fail if we don't set the R/#W bit.
-		 */
-		i801_set_hstadd(priv, addr,
-				priv->original_hstcfg & SMBHSTCFG_SPD_WD ?
-				read_write : I2C_SMBUS_WRITE);
-		if (read_write == I2C_SMBUS_READ) {
-			/* 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;
-	case I2C_SMBUS_BLOCK_PROC_CALL:
-		/* Needs to be flagged as write transaction */
-		i801_set_hstadd(priv, addr, I2C_SMBUS_WRITE);
-		outb_p(command, SMBHSTCMD(priv));
-		block = 1;
-		break;
-	default:
-		dev_err(&priv->pci_dev->dev, "Unsupported transaction %d\n",
-			size);
-		ret = -EOPNOTSUPP;
-		goto out;
-	}
-
 	if (hwpec)	/* enable/disable hardware PEC */
 		outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_CRC, SMBAUXCTL(priv));
 	else
 		outb_p(inb_p(SMBAUXCTL(priv)) & (~SMBAUXCTL_CRC),
 		       SMBAUXCTL(priv));
 
-	if (block)
-		ret = i801_block_transaction(priv, data, read_write, size);
+	if (size == I2C_SMBUS_BLOCK_DATA ||
+	    size == I2C_SMBUS_I2C_BLOCK_DATA ||
+	    size == I2C_SMBUS_BLOCK_PROC_CALL)
+		ret = i801_block_transaction(priv, data, addr, command, read_write, size);
 	else
 		ret = i801_simple_transaction(priv, data, addr, command, read_write, size);
 
@@ -926,7 +913,6 @@ static s32 i801_access(struct i2c_adapte
 	 */
 	if (hwpec)
 		outb_p(inb_p(SMBAUXCTL(priv)) & ~SMBAUXCTL_CRC, SMBAUXCTL(priv));
-out:
 	/*
 	 * Unlock the SMBus device for use by BIOS/ACPI,
 	 * and clear status flags if not done already.

-- 
Jean Delvare
SUSE L3 Support

[PATCH v3 5/6] i2c: i801: Call i801_check_pre() from i801_access()

[Jean Delvare]

From: Heiner Kallweit <hkallweit1@gmail.com>

This avoids code duplication, in a next step we'll call
i801_check_post() from i801_transaction() as well.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-i801.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

--- linux-6.1.orig/drivers/i2c/busses/i2c-i801.c
+++ linux-6.1/drivers/i2c/busses/i2c-i801.c
@@ -462,10 +462,6 @@ static int i801_transaction(struct i801_
 	unsigned long result;
 	const struct i2c_adapter *adap = &priv->adapter;
 
-	status = i801_check_pre(priv);
-	if (status < 0)
-		return status;
-
 	if (priv->features & FEATURE_IRQ) {
 		reinit_completion(&priv->done);
 		outb_p(xact | SMBHSTCNT_INTREN | SMBHSTCNT_START,
@@ -653,10 +649,6 @@ static int i801_block_transaction_byte_b
 	if (command == I2C_SMBUS_BLOCK_PROC_CALL)
 		return -EOPNOTSUPP;
 
-	status = i801_check_pre(priv);
-	if (status < 0)
-		return status;
-
 	len = data->block[0];
 
 	if (read_write == I2C_SMBUS_WRITE) {
@@ -891,6 +883,10 @@ static s32 i801_access(struct i2c_adapte
 
 	pm_runtime_get_sync(&priv->pci_dev->dev);
 
+	ret = i801_check_pre(priv);
+	if (ret)
+		goto out;
+
 	hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC)
 		&& size != I2C_SMBUS_QUICK
 		&& size != I2C_SMBUS_I2C_BLOCK_DATA;
@@ -913,6 +909,7 @@ static s32 i801_access(struct i2c_adapte
 	 */
 	if (hwpec)
 		outb_p(inb_p(SMBAUXCTL(priv)) & ~SMBAUXCTL_CRC, SMBAUXCTL(priv));
+out:
 	/*
 	 * Unlock the SMBus device for use by BIOS/ACPI,
 	 * and clear status flags if not done already.

-- 
Jean Delvare
SUSE L3 Support

[PATCH v3 6/6] i2c: i801: Call i801_check_post() from i801_access()

[Jean Delvare]

From: Heiner Kallweit <hkallweit1@gmail.com>

Avoid code duplication by calling i801_check_post() from i801_access().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-i801.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

--- linux-6.1.orig/drivers/i2c/busses/i2c-i801.c
+++ linux-6.1/drivers/i2c/busses/i2c-i801.c
@@ -434,7 +434,7 @@ static int i801_wait_intr(struct i801_pr
 		busy = status & SMBHSTSTS_HOST_BUSY;
 		status &= STATUS_ERROR_FLAGS | SMBHSTSTS_INTR;
 		if (!busy && status)
-			return status;
+			return status & STATUS_ERROR_FLAGS;
 	} while (time_is_after_eq_jiffies(timeout));
 
 	return -ETIMEDOUT;
@@ -458,7 +458,6 @@ static int i801_wait_byte_done(struct i8
 
 static int i801_transaction(struct i801_priv *priv, int xact)
 {
-	int status;
 	unsigned long result;
 	const struct i2c_adapter *adap = &priv->adapter;
 
@@ -467,13 +466,12 @@ static int i801_transaction(struct i801_
 		outb_p(xact | SMBHSTCNT_INTREN | SMBHSTCNT_START,
 		       SMBHSTCNT(priv));
 		result = wait_for_completion_timeout(&priv->done, adap->timeout);
-		return i801_check_post(priv, result ? priv->status : -ETIMEDOUT);
+		return result ? priv->status : -ETIMEDOUT;
 	}
 
 	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,
@@ -624,7 +622,7 @@ static irqreturn_t i801_isr(int irq, voi
 
 	status &= STATUS_ERROR_FLAGS | SMBHSTSTS_INTR;
 	if (status) {
-		priv->status = status;
+		priv->status = status & STATUS_ERROR_FLAGS;
 		complete(&priv->done);
 	}
 
@@ -674,7 +672,7 @@ static int i801_block_transaction_byte_b
 		reinit_completion(&priv->done);
 		outb_p(priv->cmd | SMBHSTCNT_START, SMBHSTCNT(priv));
 		result = wait_for_completion_timeout(&priv->done, adap->timeout);
-		return i801_check_post(priv, result ? priv->status : -ETIMEDOUT);
+		return result ? priv->status : -ETIMEDOUT;
 	}
 
 	for (i = 1; i <= len; i++) {
@@ -688,7 +686,7 @@ static int i801_block_transaction_byte_b
 
 		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) {
@@ -718,9 +716,7 @@ static int i801_block_transaction_byte_b
 		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 void i801_set_hstadd(struct i801_priv *priv, u8 addr, char read_write)
@@ -904,6 +900,8 @@ static s32 i801_access(struct i2c_adapte
 	else
 		ret = i801_simple_transaction(priv, data, addr, command, read_write, size);
 
+	ret = i801_check_post(priv, ret);
+
 	/* Some BIOSes don't like it when PEC is enabled at reboot or resume
 	 * time, so we forcibly disable it after every transaction.
 	 */

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

On Thu, Feb 16, 2023 at 05:10:30PM +0100, Jean Delvare wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Factor out non-block pre/post processing to a new function
> i801_simple_transaction(), complementing existing function
> i801_block_transaction(). This makes i801_access() better readable.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/6] i2c: i801: Handle SMBAUXCTL_E32B in i801_block_transaction_by_block only

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On Thu, Feb 16, 2023 at 05:11:17PM +0100, Jean Delvare wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Currently we touch SMBAUXCTL even if not needed. That's the case for block
> commands that don't use block buffer mode, either because block buffer
> mode isn't available or because it's not supported for the respective
> command (e.g. I2C block transfer). Improve this by setting/resetting
> SMBAUXCTL_E32B in i801_block_transaction_by_block() only.
> 
> Small downside is that we now access SMBAUXCTL twice for transactions
> that use PEC and block buffer mode. But this should a rather rare case
> and the impact is negligible.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/6] i2c: i801: Centralize configuring non-block commands in i801_simple_transaction

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

On Thu, Feb 16, 2023 at 05:12:12PM +0100, Jean Delvare wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Currently configuring command register settings is distributed over multiple
> functions. At first centralize this for non-block commands in
> i801_simple_transaction().
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 4/6] i2c: i801: Centralize configuring block commands in i801_block_transaction

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On Thu, Feb 16, 2023 at 05:14:16PM +0100, Jean Delvare wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Similar to what was done for non-block commands, centralize block
> command register settings in i801_block_transaction().
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 5/6] i2c: i801: Call i801_check_pre() from i801_access()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

On Thu, Feb 16, 2023 at 05:14:51PM +0100, Jean Delvare wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> This avoids code duplication, in a next step we'll call
> i801_check_post() from i801_transaction() as well.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 6/6] i2c: i801: Call i801_check_post() from i801_access()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

On Thu, Feb 16, 2023 at 05:15:33PM +0100, Jean Delvare wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Avoid code duplication by calling i801_check_post() from i801_access().
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]