From 78f420acc4231f7db99291d846bc73d5f8a8df72 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sat, 22 May 2021 00:00:28 +0200 Subject: i2c: i801: Remove unneeded warning after wait_event_timeout timeout When passing -ETIMEDOUT to i801_check_post() it will emit a timeout error message. I don't see much benefit in an additional warning stating more or less the same. Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Tested-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 99d446763530..bfea94d02775 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -509,11 +509,9 @@ static int i801_transaction(struct i801_priv *priv, int xact) result = wait_event_timeout(priv->waitq, (status = priv->status), adap->timeout); - if (!result) { + if (!result) status = -ETIMEDOUT; - dev_warn(&priv->pci_dev->dev, - "Timeout waiting for interrupt!\n"); - } + priv->status = 0; return i801_check_post(priv, status); } @@ -732,11 +730,9 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv, result = wait_event_timeout(priv->waitq, (status = priv->status), adap->timeout); - if (!result) { + if (!result) status = -ETIMEDOUT; - dev_warn(&priv->pci_dev->dev, - "Timeout waiting for interrupt!\n"); - } + priv->status = 0; return i801_check_post(priv, status); } -- cgit v1.2.3-59-g8ed1b From 1de93d5d521717cbb77cc9796a4df141d800d608 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sat, 22 May 2021 00:02:43 +0200 Subject: i2c: i801: Replace waitqueue with completion API Using the completion API is more intuitive and it allows to simplify the code. Note that we don't have to set priv->status = 0 any longer with the completion API. Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Tested-by: Jean Delvare Reviewed-by: Daniel Kurtz Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 48 +++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index bfea94d02775..738204d7726d 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -103,7 +103,7 @@ #include #include #include -#include +#include #include #include #include @@ -270,7 +270,7 @@ struct i801_priv { unsigned int features; /* isr processing */ - wait_queue_head_t waitq; + struct completion done; u8 status; /* Command state used by isr for byte-by-byte block transactions */ @@ -496,24 +496,19 @@ static int i801_wait_byte_done(struct i801_priv *priv) static int i801_transaction(struct i801_priv *priv, int xact) { int status; - int result; + unsigned long result; const struct i2c_adapter *adap = &priv->adapter; - result = i801_check_pre(priv); - if (result < 0) - return result; + 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, SMBHSTCNT(priv)); - result = wait_event_timeout(priv->waitq, - (status = priv->status), - adap->timeout); - if (!result) - status = -ETIMEDOUT; - - priv->status = 0; - return i801_check_post(priv, status); + result = wait_for_completion_timeout(&priv->done, adap->timeout); + return i801_check_post(priv, result ? priv->status : -ETIMEDOUT); } /* the current contents of SMBHSTCNT can be overwritten, since PEC, @@ -638,7 +633,7 @@ static irqreturn_t i801_host_notify_isr(struct i801_priv *priv) * DEV_ERR - Invalid command, NAK or communication timeout * BUS_ERR - SMI# transaction collision * FAILED - transaction was canceled due to a KILL request - * When any of these occur, update ->status and wake up the waitq. + * When any of these occur, update ->status and signal completion. * ->status must be cleared before kicking off the next transaction. * * 2) For byte-by-byte (I2C read/write) transactions, one BYTE_DONE interrupt @@ -675,7 +670,7 @@ static irqreturn_t i801_isr(int irq, void *dev_id) if (status) { outb_p(status, SMBHSTSTS(priv)); priv->status = status; - wake_up(&priv->waitq); + complete(&priv->done); } return IRQ_HANDLED; @@ -694,15 +689,15 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv, int i, len; int smbcmd; int status; - int result; + unsigned long result; const struct i2c_adapter *adap = &priv->adapter; if (command == I2C_SMBUS_BLOCK_PROC_CALL) return -EOPNOTSUPP; - result = i801_check_pre(priv); - if (result < 0) - return result; + status = i801_check_pre(priv); + if (status < 0) + return status; len = data->block[0]; @@ -726,15 +721,10 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv, priv->count = 0; priv->data = &data->block[1]; + reinit_completion(&priv->done); outb_p(priv->cmd | SMBHSTCNT_START, SMBHSTCNT(priv)); - result = wait_event_timeout(priv->waitq, - (status = priv->status), - adap->timeout); - if (!result) - status = -ETIMEDOUT; - - priv->status = 0; - return i801_check_post(priv, status); + result = wait_for_completion_timeout(&priv->done, adap->timeout); + return i801_check_post(priv, result ? priv->status : -ETIMEDOUT); } for (i = 1; i <= len; i++) { @@ -1889,7 +1879,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) } if (priv->features & FEATURE_IRQ) { - init_waitqueue_head(&priv->waitq); + init_completion(&priv->done); err = devm_request_irq(&dev->dev, dev->irq, i801_isr, IRQF_SHARED, -- cgit v1.2.3-59-g8ed1b From 0d3f1e4524bb70f528b7002803fc0737c83ddd5e Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sat, 22 May 2021 23:46:20 +0200 Subject: i2c: i801: Use standard PCI constants instead of own ones Layout of these registers is part of the PCI standard. Therefore use the constants defined by the PCI subsystem. Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Tested-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 738204d7726d..f6d7866f11c3 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -131,8 +131,6 @@ /* PCI Address Constants */ #define SMBBAR 4 -#define SMBPCICTL 0x004 -#define SMBPCISTS 0x006 #define SMBHSTCFG 0x040 #define TCOBASE 0x050 #define TCOCTL 0x054 @@ -141,12 +139,6 @@ #define SBREG_SMBCTRL 0xc6000c #define SBREG_SMBCTRL_DNV 0xcf000c -/* Host status bits for SMBPCISTS */ -#define SMBPCISTS_INTS BIT(3) - -/* Control bits for SMBPCICTL */ -#define SMBPCICTL_INTDIS BIT(10) - /* Host configuration bits for SMBHSTCFG */ #define SMBHSTCFG_HST_EN BIT(0) #define SMBHSTCFG_SMB_SMI_EN BIT(1) @@ -648,8 +640,8 @@ static irqreturn_t i801_isr(int irq, void *dev_id) u8 status; /* Confirm this is our interrupt */ - pci_read_config_word(priv->pci_dev, SMBPCISTS, &pcists); - if (!(pcists & SMBPCISTS_INTS)) + pci_read_config_word(priv->pci_dev, PCI_STATUS, &pcists); + if (!(pcists & PCI_STATUS_INTERRUPT)) return IRQ_NONE; if (priv->features & FEATURE_HOST_NOTIFY) { @@ -1866,13 +1858,13 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) u16 pcictl, pcists; /* Complain if an interrupt is already pending */ - pci_read_config_word(priv->pci_dev, SMBPCISTS, &pcists); - if (pcists & SMBPCISTS_INTS) + pci_read_config_word(priv->pci_dev, PCI_STATUS, &pcists); + if (pcists & PCI_STATUS_INTERRUPT) dev_warn(&dev->dev, "An interrupt is pending!\n"); /* Check if interrupts have been disabled */ - pci_read_config_word(priv->pci_dev, SMBPCICTL, &pcictl); - if (pcictl & SMBPCICTL_INTDIS) { + pci_read_config_word(priv->pci_dev, PCI_COMMAND, &pcictl); + if (pcictl & PCI_COMMAND_INTX_DISABLE) { dev_info(&dev->dev, "Interrupts are disabled\n"); priv->features &= ~FEATURE_IRQ; } -- cgit v1.2.3-59-g8ed1b From 44c54c4ec391412c7f529e53d27844dadc6d536a Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Tue, 25 May 2021 21:59:05 +0200 Subject: i2c: i801: Improve status polling Polling uses the same timeout as irq mode: 400 * 500us = 200ms = HZ / 5. So let's use the adapter->timeout value also for polling. This has the advantage that userspace can control the timeout value for polling as well. In addition change the code to make it better readable. Last but not least remove the timeout debug messages. Calls to both functions are followed by a call to i801_check_post() that will print an error message in case of timeout. Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Tested-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index f6d7866f11c3..2c6e84108013 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -156,9 +156,6 @@ #define SMBAUXCTL_CRC BIT(0) #define SMBAUXCTL_E32B BIT(1) -/* Other settings */ -#define MAX_RETRIES 400 - /* I801 command constants */ #define I801_QUICK 0x00 #define I801_BYTE 0x04 @@ -447,42 +444,35 @@ static int i801_check_post(struct i801_priv *priv, int status) /* Wait for BUSY being cleared and either INTR or an error flag being set */ static int i801_wait_intr(struct i801_priv *priv) { - int timeout = 0; - int status; + unsigned long timeout = jiffies + priv->adapter.timeout; + int status, busy; - /* We will always wait for a fraction of a second! */ do { usleep_range(250, 500); status = inb_p(SMBHSTSTS(priv)); - } while (((status & SMBHSTSTS_HOST_BUSY) || - !(status & (STATUS_ERROR_FLAGS | SMBHSTSTS_INTR))) && - (timeout++ < MAX_RETRIES)); + busy = status & SMBHSTSTS_HOST_BUSY; + status &= STATUS_ERROR_FLAGS | SMBHSTSTS_INTR; + if (!busy && status) + return status; + } while (time_is_after_eq_jiffies(timeout)); - if (timeout > MAX_RETRIES) { - dev_dbg(&priv->pci_dev->dev, "INTR Timeout!\n"); - return -ETIMEDOUT; - } - return status & (STATUS_ERROR_FLAGS | SMBHSTSTS_INTR); + return -ETIMEDOUT; } /* Wait for either BYTE_DONE or an error flag being set */ static int i801_wait_byte_done(struct i801_priv *priv) { - int timeout = 0; + unsigned long timeout = jiffies + priv->adapter.timeout; int status; - /* We will always wait for a fraction of a second! */ do { usleep_range(250, 500); status = inb_p(SMBHSTSTS(priv)); - } while (!(status & (STATUS_ERROR_FLAGS | SMBHSTSTS_BYTE_DONE)) && - (timeout++ < MAX_RETRIES)); + if (status & (STATUS_ERROR_FLAGS | SMBHSTSTS_BYTE_DONE)) + return status & STATUS_ERROR_FLAGS; + } while (time_is_after_eq_jiffies(timeout)); - if (timeout > MAX_RETRIES) { - dev_dbg(&priv->pci_dev->dev, "BYTE_DONE Timeout!\n"); - return -ETIMEDOUT; - } - return status & STATUS_ERROR_FLAGS; + return -ETIMEDOUT; } static int i801_transaction(struct i801_priv *priv, int xact) -- cgit v1.2.3-59-g8ed1b From 8d83973e7a85b2fab168894ea327dfd4e6ef596e Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Tue, 25 May 2021 22:01:31 +0200 Subject: i2c: i801: Simplify initialization of i2c_board_info in i801_probe_optional_slaves Why shall we bother to open-code something that the compiler can do for us. Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 2c6e84108013..9b40ea74e325 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1289,11 +1289,11 @@ static void i801_probe_optional_slaves(struct i801_priv *priv) return; if (apanel_addr) { - struct i2c_board_info info; + struct i2c_board_info info = { + .addr = apanel_addr, + .type = "fujitsu_apanel", + }; - memset(&info, 0, sizeof(struct i2c_board_info)); - info.addr = apanel_addr; - strlcpy(info.type, "fujitsu_apanel", I2C_NAME_SIZE); i2c_new_client_device(&priv->adapter, &info); } -- cgit v1.2.3-59-g8ed1b From d4a994f69f0bed0ba49db12d7bae2c891dc4b51f Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Tue, 25 May 2021 22:04:23 +0200 Subject: i2c: i801: Use driver name constant instead of function dev_driver_string We are the driver, so we can use the driver name directly instead of retrieving it by calling dev_driver_string(). Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Tested-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 9b40ea74e325..6f056ef62151 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -88,6 +88,8 @@ * See the file Documentation/i2c/busses/i2c-i801.rst for details. */ +#define DRV_NAME "i801_smbus" + #include #include #include @@ -1805,8 +1807,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) if (i801_acpi_probe(priv)) return -ENODEV; - err = pcim_iomap_regions(dev, 1 << SMBBAR, - dev_driver_string(&dev->dev)); + err = pcim_iomap_regions(dev, 1 << SMBBAR, DRV_NAME); if (err) { dev_err(&dev->dev, "Failed to request SMBus region 0x%lx-0x%Lx\n", @@ -1864,8 +1865,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) init_completion(&priv->done); err = devm_request_irq(&dev->dev, dev->irq, i801_isr, - IRQF_SHARED, - dev_driver_string(&dev->dev), priv); + IRQF_SHARED, DRV_NAME, priv); if (err) { dev_err(&dev->dev, "Failed to allocate irq %d: %d\n", dev->irq, err); @@ -1955,7 +1955,7 @@ static int i801_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(i801_pm_ops, i801_suspend, i801_resume); static struct pci_driver i801_driver = { - .name = "i801_smbus", + .name = DRV_NAME, .id_table = i801_ids, .probe = i801_probe, .remove = i801_remove, -- cgit v1.2.3-59-g8ed1b From c601610cd73d4cfc2dcbae185c134deb7c4c52cc Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Tue, 25 May 2021 22:07:17 +0200 Subject: i2c: i801: Improve i801_setup_hstcfg i801_setup_hstcfg() leaves the bits in priv->original_hstcfg that we're interested in intact. Therefore we can remove the return value from the function and use priv->original_hstcfg directly. Signed-off-by: Heiner Kallweit Reviewed-by: Jean Delvare Tested-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-i801.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/i2c/busses/i2c-i801.c') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 6f056ef62151..75037afe7c0a 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1684,19 +1684,17 @@ static inline int i801_acpi_probe(struct i801_priv *priv) { return 0; } static inline void i801_acpi_remove(struct i801_priv *priv) { } #endif -static unsigned char i801_setup_hstcfg(struct i801_priv *priv) +static void i801_setup_hstcfg(struct i801_priv *priv) { unsigned char hstcfg = priv->original_hstcfg; hstcfg &= ~SMBHSTCFG_I2C_EN; /* SMBus timing */ hstcfg |= SMBHSTCFG_HST_EN; pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hstcfg); - return hstcfg; } static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) { - unsigned char temp; int err, i; struct i801_priv *priv; @@ -1818,16 +1816,16 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) } pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &priv->original_hstcfg); - temp = i801_setup_hstcfg(priv); + i801_setup_hstcfg(priv); if (!(priv->original_hstcfg & SMBHSTCFG_HST_EN)) dev_info(&dev->dev, "Enabling SMBus device\n"); - if (temp & SMBHSTCFG_SMB_SMI_EN) { + if (priv->original_hstcfg & SMBHSTCFG_SMB_SMI_EN) { dev_dbg(&dev->dev, "SMBus using interrupt SMI#\n"); /* Disable SMBus interrupt feature if SMBus using SMI# */ priv->features &= ~FEATURE_IRQ; } - if (temp & SMBHSTCFG_SPD_WD) + if (priv->original_hstcfg & SMBHSTCFG_SPD_WD) dev_info(&dev->dev, "SPD Write Disable is set\n"); /* Clear special mode bits */ -- cgit v1.2.3-59-g8ed1b