* [PATCH 0/4 v2] i2c-i801: Make interrupt mode more robust
@ 2014-11-12 9:19 Jean Delvare
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jean Delvare @ 2014-11-12 9:19 UTC (permalink / raw)
To: Linux I2C; +Cc: Wolfram Sang
Hi all,
This is a patch series aiming at making the interrupt mode of the
i2c-i801 driver more robust. A number of driver lock-ups have been
reported on a few systems over the past few years, all related to
interrupt mode. So let's make it more tolerant to unreliable hardware
or whatever the actual problem is.
i2c-i801: Use wait_event_timeout to wait for interrupts
i2c-i801: Fallback to polling if request_irq() fails
i2c-i801: Check if interrupts are disabled
i2c-i801: Drop useless debug message
Changes since v1:
* Updated according to Wolfram's review. Thanks!
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4 v2] i2c-i801: Use wait_event_timeout to wait for interrupts
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
@ 2014-11-12 9:20 ` Jean Delvare
2014-11-12 9:24 ` [PATCH 2/4 v2] i2c-i801: Fallback to polling if request_irq() fails Jean Delvare
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-11-12 9:20 UTC (permalink / raw)
To: Linux I2C; +Cc: Wolfram Sang
Some systems have been reported to have trouble with interrupts. Use
wait_event_timeout() instead of wait_event() so we don't get stuck in
that case, and log the problem.
Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
No change since v1.
drivers/i2c/busses/i2c-i801.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
--- linux-3.18-rc4.orig/drivers/i2c/busses/i2c-i801.c 2014-11-10 22:26:47.212135115 +0100
+++ linux-3.18-rc4/drivers/i2c/busses/i2c-i801.c 2014-11-10 22:29:18.040417276 +0100
@@ -2,7 +2,7 @@
Copyright (c) 1998 - 2002 Frodo Looijaard <frodol-B0qZmFHriGg@public.gmane.org>,
Philip Edelbrock <phil-KXOFo5pg7o1l57MIdRCFDg@public.gmane.org>, and Mark D. Studebaker
<mdsxyz123-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
- Copyright (C) 2007 - 2012 Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
+ Copyright (C) 2007 - 2014 Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Copyright (C) 2010 Intel Corporation,
David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@@ -373,6 +373,7 @@ static int i801_transaction(struct i801_
{
int status;
int result;
+ const struct i2c_adapter *adap = &priv->adapter;
result = i801_check_pre(priv);
if (result < 0)
@@ -381,7 +382,14 @@ static int i801_transaction(struct i801_
if (priv->features & FEATURE_IRQ) {
outb_p(xact | SMBHSTCNT_INTREN | SMBHSTCNT_START,
SMBHSTCNT(priv));
- wait_event(priv->waitq, (status = priv->status));
+ result = wait_event_timeout(priv->waitq,
+ (status = priv->status),
+ adap->timeout);
+ if (!result) {
+ status = -ETIMEDOUT;
+ dev_warn(&priv->pci_dev->dev,
+ "Timeout waiting for interrupt!\n");
+ }
priv->status = 0;
return i801_check_post(priv, status);
}
@@ -529,6 +537,7 @@ static int i801_block_transaction_byte_b
int smbcmd;
int status;
int result;
+ const struct i2c_adapter *adap = &priv->adapter;
result = i801_check_pre(priv);
if (result < 0)
@@ -557,7 +566,14 @@ static int i801_block_transaction_byte_b
priv->data = &data->block[1];
outb_p(priv->cmd | SMBHSTCNT_START, SMBHSTCNT(priv));
- wait_event(priv->waitq, (status = priv->status));
+ result = wait_event_timeout(priv->waitq,
+ (status = priv->status),
+ adap->timeout);
+ if (!result) {
+ status = -ETIMEDOUT;
+ dev_warn(&priv->pci_dev->dev,
+ "Timeout waiting for interrupt!\n");
+ }
priv->status = 0;
return i801_check_post(priv, status);
}
@@ -1215,6 +1231,9 @@ static int i801_probe(struct pci_dev *de
outb_p(inb_p(SMBAUXCTL(priv)) &
~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
+ /* Default timeout in interrupt mode: 200 ms */
+ priv->adapter.timeout = HZ / 5;
+
if (priv->features & FEATURE_IRQ) {
init_waitqueue_head(&priv->waitq);
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/4 v2] i2c-i801: Fallback to polling if request_irq() fails
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-11-12 9:20 ` [PATCH 1/4 v2] i2c-i801: Use wait_event_timeout to wait for interrupts Jean Delvare
@ 2014-11-12 9:24 ` Jean Delvare
2014-11-12 9:25 ` [PATCH 3/4 v2] i2c-i801: Check if interrupts are disabled Jean Delvare
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-11-12 9:24 UTC (permalink / raw)
To: Linux I2C; +Cc: Wolfram Sang
The i2c-i801 driver can work without interrupts, so there is no reason
to make a request_irq failure fatal. Instead we can simply fallback
to polling.
Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
Changes since v1:
* Always log whether the driver is using polling or PCI interrupt.
drivers/i2c/busses/i2c-i801.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- linux-3.18-rc4.orig/drivers/i2c/busses/i2c-i801.c 2014-11-10 22:29:42.788955868 +0100
+++ linux-3.18-rc4/drivers/i2c/busses/i2c-i801.c 2014-11-11 14:56:35.777941563 +0100
@@ -1242,10 +1242,11 @@ static int i801_probe(struct pci_dev *de
if (err) {
dev_err(&dev->dev, "Failed to allocate irq %d: %d\n",
dev->irq, err);
- goto exit_release;
+ priv->features &= ~FEATURE_IRQ;
}
- dev_info(&dev->dev, "SMBus using PCI Interrupt\n");
}
+ dev_info(&dev->dev, "SMBus using %s\n",
+ priv->features & FEATURE_IRQ ? "PCI interrupt" : "polling");
/* set up the sysfs linkage to our parent device */
priv->adapter.dev.parent = &dev->dev;
@@ -1272,7 +1273,6 @@ static int i801_probe(struct pci_dev *de
exit_free_irq:
if (priv->features & FEATURE_IRQ)
free_irq(dev->irq, priv);
-exit_release:
pci_release_region(dev, SMBBAR);
exit:
kfree(priv);
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/4 v2] i2c-i801: Check if interrupts are disabled
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-11-12 9:20 ` [PATCH 1/4 v2] i2c-i801: Use wait_event_timeout to wait for interrupts Jean Delvare
2014-11-12 9:24 ` [PATCH 2/4 v2] i2c-i801: Fallback to polling if request_irq() fails Jean Delvare
@ 2014-11-12 9:25 ` Jean Delvare
2014-11-12 9:26 ` [PATCH 4/4 v2] i2c-i801: Drop useless debug message Jean Delvare
2014-11-12 15:28 ` [PATCH 0/4 v2] i2c-i801: Make interrupt mode more robust Wolfram Sang
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-11-12 9:25 UTC (permalink / raw)
To: Linux I2C; +Cc: Wolfram Sang
There is a control bit in the PCI configuration space which disables
interrupts. If this bit is set, the driver should not try to make use
of interrupts, it won't receive any.
Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
Changes since v1:
* Turned warning message into info message and made it shorter.
drivers/i2c/busses/i2c-i801.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
--- linux-3.18-rc4.orig/drivers/i2c/busses/i2c-i801.c 2014-11-11 14:56:35.777941563 +0100
+++ linux-3.18-rc4/drivers/i2c/busses/i2c-i801.c 2014-11-11 15:32:30.128418755 +0100
@@ -110,12 +110,16 @@
/* PCI Address Constants */
#define SMBBAR 4
+#define SMBPCICTL 0x004
#define SMBPCISTS 0x006
#define SMBHSTCFG 0x040
/* Host status bits for SMBPCISTS */
#define SMBPCISTS_INTS 0x08
+/* Control bits for SMBPCICTL */
+#define SMBPCICTL_INTDIS 0x0400
+
/* Host configuration bits for SMBHSTCFG */
#define SMBHSTCFG_HST_EN 1
#define SMBHSTCFG_SMB_SMI_EN 2
@@ -1235,6 +1239,22 @@ static int i801_probe(struct pci_dev *de
priv->adapter.timeout = HZ / 5;
if (priv->features & FEATURE_IRQ) {
+ u16 pcictl, pcists;
+
+ /* Complain if an interrupt is already pending */
+ pci_read_config_word(priv->pci_dev, SMBPCISTS, &pcists);
+ if (pcists & SMBPCISTS_INTS)
+ 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) {
+ dev_info(&dev->dev, "Interrupts are disabled\n");
+ priv->features &= ~FEATURE_IRQ;
+ }
+ }
+
+ if (priv->features & FEATURE_IRQ) {
init_waitqueue_head(&priv->waitq);
err = request_irq(dev->irq, i801_isr, IRQF_SHARED,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4 v2] i2c-i801: Drop useless debug message
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
` (2 preceding siblings ...)
2014-11-12 9:25 ` [PATCH 3/4 v2] i2c-i801: Check if interrupts are disabled Jean Delvare
@ 2014-11-12 9:26 ` Jean Delvare
2014-11-12 15:28 ` [PATCH 0/4 v2] i2c-i801: Make interrupt mode more robust Wolfram Sang
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-11-12 9:26 UTC (permalink / raw)
To: Linux I2C; +Cc: Wolfram Sang
Don't log the host status register value in i801_isr(), it has very
little value and fills up the log when debugging is enabled.
Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
No change since v1.
drivers/i2c/busses/i2c-i801.c | 3 ---
1 file changed, 3 deletions(-)
--- linux-3.18-rc4.orig/drivers/i2c/busses/i2c-i801.c 2014-11-10 19:54:05.042326020 +0100
+++ linux-3.18-rc4/drivers/i2c/busses/i2c-i801.c 2014-11-10 19:56:51.562907696 +0100
@@ -507,9 +507,6 @@ static irqreturn_t i801_isr(int irq, voi
return IRQ_NONE;
status = inb_p(SMBHSTSTS(priv));
- if (status != 0x42)
- dev_dbg(&priv->pci_dev->dev, "irq: status = %02x\n", status);
-
if (status & SMBHSTSTS_BYTE_DONE)
i801_isr_byte_done(priv);
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/4 v2] i2c-i801: Make interrupt mode more robust
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
` (3 preceding siblings ...)
2014-11-12 9:26 ` [PATCH 4/4 v2] i2c-i801: Drop useless debug message Jean Delvare
@ 2014-11-12 15:28 ` Wolfram Sang
4 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2014-11-12 15:28 UTC (permalink / raw)
To: Jean Delvare; +Cc: Linux I2C
[-- Attachment #1: Type: text/plain, Size: 636 bytes --]
On Wed, Nov 12, 2014 at 10:19:31AM +0100, Jean Delvare wrote:
> Hi all,
>
> This is a patch series aiming at making the interrupt mode of the
> i2c-i801 driver more robust. A number of driver lock-ups have been
> reported on a few systems over the past few years, all related to
> interrupt mode. So let's make it more tolerant to unreliable hardware
> or whatever the actual problem is.
>
> i2c-i801: Use wait_event_timeout to wait for interrupts
> i2c-i801: Fallback to polling if request_irq() fails
> i2c-i801: Check if interrupts are disabled
> i2c-i801: Drop useless debug message
Applied to for-next, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-12 15:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-12 9:19 [PATCH 0/4 v2] i2c-i801: Make interrupt mode more robust Jean Delvare
[not found] ` <20141112101931.5f8c196a-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-11-12 9:20 ` [PATCH 1/4 v2] i2c-i801: Use wait_event_timeout to wait for interrupts Jean Delvare
2014-11-12 9:24 ` [PATCH 2/4 v2] i2c-i801: Fallback to polling if request_irq() fails Jean Delvare
2014-11-12 9:25 ` [PATCH 3/4 v2] i2c-i801: Check if interrupts are disabled Jean Delvare
2014-11-12 9:26 ` [PATCH 4/4 v2] i2c-i801: Drop useless debug message Jean Delvare
2014-11-12 15:28 ` [PATCH 0/4 v2] i2c-i801: Make interrupt mode more robust Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox