Re: [PATCHv3] [media] saa7164: use an MSI interrupt when available

2015-03-03 Thread catchall

On 02/28/2015 04:14 PM, Brendan McGrath wrote:

Enhances driver to use an MSI interrupt when available.

Adds the module option 'enable_msi' (type bool) which by default is
enabled. Can be set to 'N' to disable.

Fixes (or can reduce the occurrence of) a crash which is most commonly
reported when multiple saa7164 chips are in use. A reported example can
be found here:
http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/83948

Reviewed-by: Steven Toth 
Signed-off-by: Brendan McGrath 

I wanted to report that I have been running this patch for about a week 
now and I have had no instances of the zero free sequences issue.


Thank you very much!

David

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3] [media] saa7164: use an MSI interrupt when available

2015-03-03 Thread catchall

On 02/28/2015 04:14 PM, Brendan McGrath wrote:

Enhances driver to use an MSI interrupt when available.

Adds the module option 'enable_msi' (type bool) which by default is
enabled. Can be set to 'N' to disable.

Fixes (or can reduce the occurrence of) a crash which is most commonly
reported when multiple saa7164 chips are in use. A reported example can
be found here:
http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/83948

Reviewed-by: Steven Toth st...@kernellabs.com
Signed-off-by: Brendan McGrath red...@redmandi.dyndns.org

I wanted to report that I have been running this patch for about a week 
now and I have had no instances of the zero free sequences issue.


Thank you very much!

David

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCHv3] [media] saa7164: use an MSI interrupt when available

2015-02-28 Thread Brendan McGrath
Enhances driver to use an MSI interrupt when available.

Adds the module option 'enable_msi' (type bool) which by default is
enabled. Can be set to 'N' to disable.

Fixes (or can reduce the occurrence of) a crash which is most commonly
reported when multiple saa7164 chips are in use. A reported example can
be found here:
http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/83948

Reviewed-by: Steven Toth 
Signed-off-by: Brendan McGrath 
---

Changes since v3:
 * Added link to reported incident in comments
 * Added Reviewed-by tag from Steven Toth
 * Used git send-mail (as my patch got mangled using Thunderbird)
 * Added the linux-kernel mailing list to the recipient list

Note I have not included the Tested-by tag from Kyle Sanderson as his test was 
on v1 (which did not include the module option 'enable_msi') - although much of 
the code in v3 was there is v1.

 drivers/media/pci/saa7164/saa7164-core.c | 40 ++--
 drivers/media/pci/saa7164/saa7164.h  |  1 +
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-core.c 
b/drivers/media/pci/saa7164/saa7164-core.c
index 4b0bec3..c9a6447 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -85,6 +85,11 @@ module_param(guard_checking, int, 0644);
 MODULE_PARM_DESC(guard_checking,
"enable dma sanity checking for buffer overruns");
 
+static bool enable_msi = true;
+module_param(enable_msi, bool, 0444);
+MODULE_PARM_DESC(enable_msi,
+   "enable the use of an msi interrupt if available");
+
 static unsigned int saa7164_devcount;
 
 static DEFINE_MUTEX(devlist);
@@ -1230,8 +1235,34 @@ static int saa7164_initdev(struct pci_dev *pci_dev,
goto fail_irq;
}
 
-   err = request_irq(pci_dev->irq, saa7164_irq,
-   IRQF_SHARED, dev->name, dev);
+   /* irq bit */
+   if (enable_msi)
+   err = pci_enable_msi(pci_dev);
+
+   if (!err && enable_msi) {
+   /* no error - so request an msi interrupt */
+   err = request_irq(pci_dev->irq, saa7164_irq, 0,
+ dev->name, dev);
+
+   if (err) {
+   /* fall back to legacy interrupt */
+   printk(KERN_ERR "%s() Failed to get an MSI interrupt."
+  " Falling back to a shared IRQ\n", __func__);
+   pci_disable_msi(pci_dev);
+   } else {
+   dev->msi = true;
+   }
+   }
+
+   if ((!enable_msi) || err) {
+   dev->msi = false;
+   /* if we have an error (i.e. we don't have an interrupt)
+or msi is not enabled - fallback to shared interrupt */
+
+   err = request_irq(pci_dev->irq, saa7164_irq,
+ IRQF_SHARED, dev->name, dev);
+   }
+
if (err < 0) {
printk(KERN_ERR "%s: can't get IRQ %d\n", dev->name,
pci_dev->irq);
@@ -1441,6 +1472,11 @@ static void saa7164_finidev(struct pci_dev *pci_dev)
/* unregister stuff */
free_irq(pci_dev->irq, dev);
 
+   if (dev->msi) {
+   pci_disable_msi(pci_dev);
+   dev->msi = false;
+   }
+
mutex_lock();
list_del(>devlist);
mutex_unlock();
diff --git a/drivers/media/pci/saa7164/saa7164.h 
b/drivers/media/pci/saa7164/saa7164.h
index cd1a07c..6df4b252 100644
--- a/drivers/media/pci/saa7164/saa7164.h
+++ b/drivers/media/pci/saa7164/saa7164.h
@@ -459,6 +459,7 @@ struct saa7164_dev {
/* Interrupt status and ack registers */
u32 int_status;
u32 int_ack;
+   u32 msi;
 
struct cmd  cmds[SAA_CMD_MAX_MSG_UNITS];
struct mutexlock;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCHv3] [media] saa7164: use an MSI interrupt when available

2015-02-28 Thread Brendan McGrath
Enhances driver to use an MSI interrupt when available.

Adds the module option 'enable_msi' (type bool) which by default is
enabled. Can be set to 'N' to disable.

Fixes (or can reduce the occurrence of) a crash which is most commonly
reported when multiple saa7164 chips are in use. A reported example can
be found here:
http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/83948

Reviewed-by: Steven Toth st...@kernellabs.com
Signed-off-by: Brendan McGrath red...@redmandi.dyndns.org
---

Changes since v3:
 * Added link to reported incident in comments
 * Added Reviewed-by tag from Steven Toth
 * Used git send-mail (as my patch got mangled using Thunderbird)
 * Added the linux-kernel mailing list to the recipient list

Note I have not included the Tested-by tag from Kyle Sanderson as his test was 
on v1 (which did not include the module option 'enable_msi') - although much of 
the code in v3 was there is v1.

 drivers/media/pci/saa7164/saa7164-core.c | 40 ++--
 drivers/media/pci/saa7164/saa7164.h  |  1 +
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-core.c 
b/drivers/media/pci/saa7164/saa7164-core.c
index 4b0bec3..c9a6447 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -85,6 +85,11 @@ module_param(guard_checking, int, 0644);
 MODULE_PARM_DESC(guard_checking,
enable dma sanity checking for buffer overruns);
 
+static bool enable_msi = true;
+module_param(enable_msi, bool, 0444);
+MODULE_PARM_DESC(enable_msi,
+   enable the use of an msi interrupt if available);
+
 static unsigned int saa7164_devcount;
 
 static DEFINE_MUTEX(devlist);
@@ -1230,8 +1235,34 @@ static int saa7164_initdev(struct pci_dev *pci_dev,
goto fail_irq;
}
 
-   err = request_irq(pci_dev-irq, saa7164_irq,
-   IRQF_SHARED, dev-name, dev);
+   /* irq bit */
+   if (enable_msi)
+   err = pci_enable_msi(pci_dev);
+
+   if (!err  enable_msi) {
+   /* no error - so request an msi interrupt */
+   err = request_irq(pci_dev-irq, saa7164_irq, 0,
+ dev-name, dev);
+
+   if (err) {
+   /* fall back to legacy interrupt */
+   printk(KERN_ERR %s() Failed to get an MSI interrupt.
+   Falling back to a shared IRQ\n, __func__);
+   pci_disable_msi(pci_dev);
+   } else {
+   dev-msi = true;
+   }
+   }
+
+   if ((!enable_msi) || err) {
+   dev-msi = false;
+   /* if we have an error (i.e. we don't have an interrupt)
+or msi is not enabled - fallback to shared interrupt */
+
+   err = request_irq(pci_dev-irq, saa7164_irq,
+ IRQF_SHARED, dev-name, dev);
+   }
+
if (err  0) {
printk(KERN_ERR %s: can't get IRQ %d\n, dev-name,
pci_dev-irq);
@@ -1441,6 +1472,11 @@ static void saa7164_finidev(struct pci_dev *pci_dev)
/* unregister stuff */
free_irq(pci_dev-irq, dev);
 
+   if (dev-msi) {
+   pci_disable_msi(pci_dev);
+   dev-msi = false;
+   }
+
mutex_lock(devlist);
list_del(dev-devlist);
mutex_unlock(devlist);
diff --git a/drivers/media/pci/saa7164/saa7164.h 
b/drivers/media/pci/saa7164/saa7164.h
index cd1a07c..6df4b252 100644
--- a/drivers/media/pci/saa7164/saa7164.h
+++ b/drivers/media/pci/saa7164/saa7164.h
@@ -459,6 +459,7 @@ struct saa7164_dev {
/* Interrupt status and ack registers */
u32 int_status;
u32 int_ack;
+   u32 msi;
 
struct cmd  cmds[SAA_CMD_MAX_MSG_UNITS];
struct mutexlock;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/