* [PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction()
2023-02-16 16:08 [PATCH v3 0/6] i2c: i801: Series with minor improvements (second half) Jean Delvare
@ 2023-02-16 16:10 ` Jean Delvare
2023-02-17 21:40 ` Wolfram Sang
2023-02-16 16:11 ` [PATCH v3 2/6] i2c: i801: Handle SMBAUXCTL_E32B in i801_block_transaction_by_block only Jean Delvare
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2023-02-16 16:10 UTC (permalink / raw)
To: Linux I2C; +Cc: Heiner Kallweit
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction()
2023-02-16 16:10 ` [PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction() Jean Delvare
@ 2023-02-17 21:40 ` Wolfram Sang
0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2023-02-17 21:40 UTC (permalink / raw)
To: Jean Delvare; +Cc: Linux I2C, Heiner Kallweit
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/6] i2c: i801: Handle SMBAUXCTL_E32B in i801_block_transaction_by_block only
2023-02-16 16:08 [PATCH v3 0/6] i2c: i801: Series with minor improvements (second half) Jean Delvare
2023-02-16 16:10 ` [PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction() Jean Delvare
@ 2023-02-16 16:11 ` Jean Delvare
2023-02-17 21:40 ` Wolfram Sang
2023-02-16 16:12 ` [PATCH v3 3/6] i2c: i801: Centralize configuring non-block commands in i801_simple_transaction Jean Delvare
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2023-02-16 16:11 UTC (permalink / raw)
To: Linux I2C; +Cc: Heiner Kallweit
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 3/6] i2c: i801: Centralize configuring non-block commands in i801_simple_transaction
2023-02-16 16:08 [PATCH v3 0/6] i2c: i801: Series with minor improvements (second half) Jean Delvare
2023-02-16 16:10 ` [PATCH v3 1/6] i2c: i801: Add i801_simple_transaction(), complementing i801_block_transaction() Jean Delvare
2023-02-16 16:11 ` [PATCH v3 2/6] i2c: i801: Handle SMBAUXCTL_E32B in i801_block_transaction_by_block only Jean Delvare
@ 2023-02-16 16:12 ` Jean Delvare
2023-02-17 21:40 ` Wolfram Sang
2023-02-16 16:14 ` [PATCH v3 4/6] i2c: i801: Centralize configuring block commands in i801_block_transaction Jean Delvare
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2023-02-16 16:12 UTC (permalink / raw)
To: Linux I2C; +Cc: Heiner Kallweit
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 4/6] i2c: i801: Centralize configuring block commands in i801_block_transaction
2023-02-16 16:08 [PATCH v3 0/6] i2c: i801: Series with minor improvements (second half) Jean Delvare
` (2 preceding siblings ...)
2023-02-16 16:12 ` [PATCH v3 3/6] i2c: i801: Centralize configuring non-block commands in i801_simple_transaction Jean Delvare
@ 2023-02-16 16:14 ` Jean Delvare
2023-02-17 21:41 ` Wolfram Sang
2023-02-16 16:14 ` [PATCH v3 5/6] i2c: i801: Call i801_check_pre() from i801_access() Jean Delvare
2023-02-16 16:15 ` [PATCH v3 6/6] i2c: i801: Call i801_check_post() " Jean Delvare
5 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2023-02-16 16:14 UTC (permalink / raw)
To: Linux I2C; +Cc: Heiner Kallweit
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3 5/6] i2c: i801: Call i801_check_pre() from i801_access()
2023-02-16 16:08 [PATCH v3 0/6] i2c: i801: Series with minor improvements (second half) Jean Delvare
` (3 preceding siblings ...)
2023-02-16 16:14 ` [PATCH v3 4/6] i2c: i801: Centralize configuring block commands in i801_block_transaction Jean Delvare
@ 2023-02-16 16:14 ` Jean Delvare
2023-02-17 21:41 ` Wolfram Sang
2023-02-16 16:15 ` [PATCH v3 6/6] i2c: i801: Call i801_check_post() " Jean Delvare
5 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2023-02-16 16:14 UTC (permalink / raw)
To: Linux I2C; +Cc: Heiner Kallweit
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3 6/6] i2c: i801: Call i801_check_post() from i801_access()
2023-02-16 16:08 [PATCH v3 0/6] i2c: i801: Series with minor improvements (second half) Jean Delvare
` (4 preceding siblings ...)
2023-02-16 16:14 ` [PATCH v3 5/6] i2c: i801: Call i801_check_pre() from i801_access() Jean Delvare
@ 2023-02-16 16:15 ` Jean Delvare
2023-02-17 21:42 ` Wolfram Sang
5 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2023-02-16 16:15 UTC (permalink / raw)
To: Linux I2C; +Cc: Heiner Kallweit
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
^ permalink raw reply [flat|nested] 13+ messages in thread