* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.