[PATCH v2.6.38 resend 0/7] cx88 deadlock and data races

[PATCH v2.6.38 resend 0/7] cx88 deadlock and data races

[Jonathan Nieder]

Hi,

Since v2.6.37 (BKL removal), trying to open a cx88-blackbird driven
MPEG device would hang.  Ben Hutchings provided an initial patch[1] to
fix that, and Andi Huber helped a lot in finding races that patch had
missed.  Ben requested that I take authorship of the series, so I've
done so.

These patches have visited the list twice before[2], towards the
beginning of this month.  No feedback from maintainers or reviewers
--- I'm beginning to wonder if I have the right list.  Luckily two
people[3][4] experiencing this problem on bugzilla were kind enough to
test the series and found it worked okay.

Patches are against v2.6.38 (but they do not conflict with anything in
media-tree/staging/for_v2.6.40).  The intent is to include them in
v2.6.40 if possible.  A copy of these patches is also available for
convenient fetching from

  git://repo.or.cz/linux-2.6/jrn.git cx88-locking

Thoughts?

Jonathan Nieder (7):
  [media] cx88: protect per-device driver list with device lock
  [media] cx88: fix locking of sub-driver operations
  [media] cx88: hold device lock during sub-driver initialization
  [media] cx88: protect cx8802_devlist with a mutex
  [media] cx88: gracefully reject attempts to use unregistered
    cx88-blackbird driver
  [media] cx88: don't use atomic_t for core->mpeg_users
  [media] cx88: don't use atomic_t for core->users

 drivers/media/video/cx88/cx88-blackbird.c |   41 +++++++++++++++-------------
 drivers/media/video/cx88/cx88-dvb.c       |    2 +
 drivers/media/video/cx88/cx88-mpeg.c      |   40 ++++++++++++++++++----------
 drivers/media/video/cx88/cx88-video.c     |    5 ++-
 drivers/media/video/cx88/cx88.h           |   11 +++++--
 5 files changed, 61 insertions(+), 38 deletions(-)

[1] http://bugs.debian.org/619827
[2] http://thread.gmane.org/gmane.linux.kernel/1118815
[3] https://bugzilla.kernel.org/show_bug.cgi?id=31792
[4] https://bugzilla.kernel.org/show_bug.cgi?id=31962

[PATCH 1/7] [media] cx88: protect per-device driver list with device lock

[Jonathan Nieder]

The BKL conversion of this driver seems to have gone wrong.  Various
uses of the sub-device and driver lists appear to be subject to race
conditions.

In particular, some functions access drvlist without a relevant lock
held, which will race against removal of drivers.  Let's start with
that --- clean up by consistently protecting dev->drvlist with
dev->core->lock, noting driver functions that require the device lock
to be held or not to be held.

After this patch, there are still some races --- e.g.,
cx8802_blackbird_remove can run between the time the blackbird driver
is acquired and the time it is used in mpeg_release, and there's a
similar race in cx88_dvb_bus_ctrl.  Later patches will address the
remaining known races and the deadlock noticed by Andi.  This patch
just makes the semantics clearer in preparation for those later
changes.

Based on work by Ben Hutchings <ben@decadent.org.uk>.

Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Cc: stable@kernel.org
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |    3 ++-
 drivers/media/video/cx88/cx88-dvb.c       |    3 +++
 drivers/media/video/cx88/cx88-mpeg.c      |   11 +++++++----
 drivers/media/video/cx88/cx88.h           |    9 ++++++++-
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index bca307e..b93fbd3 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1122,10 +1122,11 @@ static int mpeg_release(struct file *file)
 	mutex_lock(&dev->core->lock);
 	file->private_data = NULL;
 	kfree(fh);
-	mutex_unlock(&dev->core->lock);
 
 	/* Make sure we release the hardware */
 	drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
+	mutex_unlock(&dev->core->lock);
+
 	if (drv)
 		drv->request_release(drv);
 
diff --git a/drivers/media/video/cx88/cx88-dvb.c b/drivers/media/video/cx88/cx88-dvb.c
index 90717ee..88a1507 100644
--- a/drivers/media/video/cx88/cx88-dvb.c
+++ b/drivers/media/video/cx88/cx88-dvb.c
@@ -132,7 +132,10 @@ static int cx88_dvb_bus_ctrl(struct dvb_frontend* fe, int acquire)
 		return -EINVAL;
 	}
 
+	mutex_lock(&dev->core->lock);
 	drv = cx8802_get_driver(dev, CX88_MPEG_DVB);
+	mutex_unlock(&dev->core->lock);
+
 	if (drv) {
 		if (acquire){
 			dev->frontends.active_fe_id = fe_id;
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index addf954..918172b 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -748,6 +748,8 @@ int cx8802_unregister_driver(struct cx8802_driver *drv)
 		       dev->pci->subsystem_device, dev->core->board.name,
 		       dev->core->boardnr);
 
+		mutex_lock(&dev->core->lock);
+
 		list_for_each_entry_safe(d, dtmp, &dev->drvlist, drvlist) {
 			/* only unregister the correct driver type */
 			if (d->type_id != drv->type_id)
@@ -755,15 +757,14 @@ int cx8802_unregister_driver(struct cx8802_driver *drv)
 
 			err = d->remove(d);
 			if (err == 0) {
-				mutex_lock(&drv->core->lock);
 				list_del(&d->drvlist);
-				mutex_unlock(&drv->core->lock);
 				kfree(d);
 			} else
 				printk(KERN_ERR "%s/2: cx8802 driver remove "
 				       "failed (%d)\n", dev->core->name, err);
 		}
 
+		mutex_unlock(&dev->core->lock);
 	}
 
 	return err;
@@ -827,6 +828,8 @@ static void __devexit cx8802_remove(struct pci_dev *pci_dev)
 
 	flush_request_modules(dev);
 
+	mutex_lock(&dev->core->lock);
+
 	if (!list_empty(&dev->drvlist)) {
 		struct cx8802_driver *drv, *tmp;
 		int err;
@@ -838,9 +841,7 @@ static void __devexit cx8802_remove(struct pci_dev *pci_dev)
 		list_for_each_entry_safe(drv, tmp, &dev->drvlist, drvlist) {
 			err = drv->remove(drv);
 			if (err == 0) {
-				mutex_lock(&drv->core->lock);
 				list_del(&drv->drvlist);
-				mutex_unlock(&drv->core->lock);
 			} else
 				printk(KERN_ERR "%s/2: cx8802 driver remove "
 				       "failed (%d)\n", dev->core->name, err);
@@ -848,6 +849,8 @@ static void __devexit cx8802_remove(struct pci_dev *pci_dev)
 		}
 	}
 
+	mutex_unlock(&dev->core->lock);
+
 	/* Destroy any 8802 reference. */
 	dev->core->dvbdev = NULL;
 
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index c9981e7..6ff34c7 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -496,7 +496,11 @@ struct cx8802_driver {
 	int (*resume)(struct pci_dev *pci_dev);
 
 	/* MPEG 8802 -> mini driver - Driver probe and configuration */
+
+	/* Caller must _not_ hold core->lock */
 	int (*probe)(struct cx8802_driver *drv);
+
+	/* Caller must hold core->lock */
 	int (*remove)(struct cx8802_driver *drv);
 
 	/* MPEG 8802 -> mini driver - Access for hardware control */
@@ -551,8 +555,9 @@ struct cx8802_dev {
 	/* for switching modulation types */
 	unsigned char              ts_gen_cntrl;
 
-	/* List of attached drivers */
+	/* List of attached drivers; must hold core->lock to access */
 	struct list_head	   drvlist;
+
 	struct work_struct	   request_module_wk;
 };
 
@@ -675,6 +680,8 @@ int cx88_audio_thread(void *data);
 
 int cx8802_register_driver(struct cx8802_driver *drv);
 int cx8802_unregister_driver(struct cx8802_driver *drv);
+
+/* Caller must hold core->lock */
 struct cx8802_driver * cx8802_get_driver(struct cx8802_dev *dev, enum cx88_board_type btype);
 
 /* ----------------------------------------------------------- */
-- 
1.7.5

[PATCH 2/7] [media] cx88: fix locking of sub-driver operations

[Jonathan Nieder]

The BKL conversion of this driver seems to have gone wrong.
Loading the cx88-blackbird driver deadlocks.

The cause: mpeg_ops::open in the cx2388x blackbird driver acquires the
device lock and calls the sub-driver's request_acquire, which tries to
acquire the lock again.  Fix it by clarifying the semantics of
request_acquire, request_release, advise_acquire, and advise_release:
now all will rely on the caller to acquire the device lock.

Based on work by Ben Hutchings <ben@decadent.org.uk>.

Reported-by: Andi Huber <hobrom@gmx.at>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=31962
Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Cc: stable@kernel.org
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |    4 ++--
 drivers/media/video/cx88/cx88-dvb.c       |    3 +--
 drivers/media/video/cx88/cx88-mpeg.c      |    4 ----
 drivers/media/video/cx88/cx88.h           |    3 ++-
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index b93fbd3..a6f7d53 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1125,13 +1125,13 @@ static int mpeg_release(struct file *file)
 
 	/* Make sure we release the hardware */
 	drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
-	mutex_unlock(&dev->core->lock);
-
 	if (drv)
 		drv->request_release(drv);
 
 	atomic_dec(&dev->core->mpeg_users);
 
+	mutex_unlock(&dev->core->lock);
+
 	return 0;
 }
 
diff --git a/drivers/media/video/cx88/cx88-dvb.c b/drivers/media/video/cx88/cx88-dvb.c
index 88a1507..5eccd02 100644
--- a/drivers/media/video/cx88/cx88-dvb.c
+++ b/drivers/media/video/cx88/cx88-dvb.c
@@ -134,8 +134,6 @@ static int cx88_dvb_bus_ctrl(struct dvb_frontend* fe, int acquire)
 
 	mutex_lock(&dev->core->lock);
 	drv = cx8802_get_driver(dev, CX88_MPEG_DVB);
-	mutex_unlock(&dev->core->lock);
-
 	if (drv) {
 		if (acquire){
 			dev->frontends.active_fe_id = fe_id;
@@ -145,6 +143,7 @@ static int cx88_dvb_bus_ctrl(struct dvb_frontend* fe, int acquire)
 			dev->frontends.active_fe_id = 0;
 		}
 	}
+	mutex_unlock(&dev->core->lock);
 
 	return ret;
 }
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index 918172b..9147c16 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -624,13 +624,11 @@ static int cx8802_request_acquire(struct cx8802_driver *drv)
 
 	if (drv->advise_acquire)
 	{
-		mutex_lock(&drv->core->lock);
 		core->active_ref++;
 		if (core->active_type_id == CX88_BOARD_NONE) {
 			core->active_type_id = drv->type_id;
 			drv->advise_acquire(drv);
 		}
-		mutex_unlock(&drv->core->lock);
 
 		mpeg_dbg(1,"%s() Post acquire GPIO=%x\n", __func__, cx_read(MO_GP0_IO));
 	}
@@ -643,14 +641,12 @@ static int cx8802_request_release(struct cx8802_driver *drv)
 {
 	struct cx88_core *core = drv->core;
 
-	mutex_lock(&drv->core->lock);
 	if (drv->advise_release && --core->active_ref == 0)
 	{
 		drv->advise_release(drv);
 		core->active_type_id = CX88_BOARD_NONE;
 		mpeg_dbg(1,"%s() Post release GPIO=%x\n", __func__, cx_read(MO_GP0_IO));
 	}
-	mutex_unlock(&drv->core->lock);
 
 	return 0;
 }
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index 6ff34c7..e912919 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -500,7 +500,8 @@ struct cx8802_driver {
 	/* Caller must _not_ hold core->lock */
 	int (*probe)(struct cx8802_driver *drv);
 
-	/* Caller must hold core->lock */
+	/* Callers to the following functions must hold core->lock */
+
 	int (*remove)(struct cx8802_driver *drv);
 
 	/* MPEG 8802 -> mini driver - Access for hardware control */
-- 
1.7.5

[PATCH 3/7] [media] cx88: hold device lock during sub-driver initialization

[Jonathan Nieder]

cx8802_blackbird_probe makes a device node for the mpeg sub-device
before it has been added to dev->drvlist.  If the device is opened
during that time, the open succeeds but request_acquire cannot be
called, so the reference count remains zero.  Later, when the device
is closed, the reference count becomes negative --- uh oh.

Close the race by holding core->lock during probe and not releasing
until the device is in drvlist and initialization finished.
Previously the BKL prevented this race.

Reported-by: Andreas Huber <hobrom@gmx.at>
Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Cc: stable@kernel.org
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |    2 --
 drivers/media/video/cx88/cx88-mpeg.c      |    5 ++---
 drivers/media/video/cx88/cx88.h           |    7 ++-----
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index a6f7d53..f637d34 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1335,11 +1335,9 @@ static int cx8802_blackbird_probe(struct cx8802_driver *drv)
 	blackbird_register_video(dev);
 
 	/* initial device configuration: needed ? */
-	mutex_lock(&dev->core->lock);
 //	init_controls(core);
 	cx88_set_tvnorm(core,core->tvnorm);
 	cx88_video_mux(core,0);
-	mutex_unlock(&dev->core->lock);
 
 	return 0;
 
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index 9147c16..497f26f 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -709,18 +709,17 @@ int cx8802_register_driver(struct cx8802_driver *drv)
 		drv->request_release = cx8802_request_release;
 		memcpy(driver, drv, sizeof(*driver));
 
+		mutex_lock(&drv->core->lock);
 		err = drv->probe(driver);
 		if (err == 0) {
 			i++;
-			mutex_lock(&drv->core->lock);
 			list_add_tail(&driver->drvlist, &dev->drvlist);
-			mutex_unlock(&drv->core->lock);
 		} else {
 			printk(KERN_ERR
 			       "%s/2: cx8802 probe failed, err = %d\n",
 			       dev->core->name, err);
 		}
-
+		mutex_unlock(&drv->core->lock);
 	}
 
 	return i ? 0 : -ENODEV;
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index e912919..93a94bf 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -495,13 +495,10 @@ struct cx8802_driver {
 	int (*suspend)(struct pci_dev *pci_dev, pm_message_t state);
 	int (*resume)(struct pci_dev *pci_dev);
 
+	/* Callers to the following functions must hold core->lock */
+
 	/* MPEG 8802 -> mini driver - Driver probe and configuration */
-
-	/* Caller must _not_ hold core->lock */
 	int (*probe)(struct cx8802_driver *drv);
-
-	/* Callers to the following functions must hold core->lock */
-
 	int (*remove)(struct cx8802_driver *drv);
 
 	/* MPEG 8802 -> mini driver - Access for hardware control */
-- 
1.7.5

[PATCH 4/7] [media] cx88: protect cx8802_devlist with a mutex

[Jonathan Nieder]

Add and use a mutex to protect the cx88-mpeg device list.  Previously
the BKL prevented races.

Based on work by Ben Hutchings <ben@decadent.org.uk>.

Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-mpeg.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index 497f26f..b18f9fe 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -78,6 +78,7 @@ static void flush_request_modules(struct cx8802_dev *dev)
 
 
 static LIST_HEAD(cx8802_devlist);
+static DEFINE_MUTEX(cx8802_mutex);
 /* ------------------------------------------------------------------ */
 
 static int cx8802_start_dma(struct cx8802_dev    *dev,
@@ -689,6 +690,8 @@ int cx8802_register_driver(struct cx8802_driver *drv)
 		return err;
 	}
 
+	mutex_lock(&cx8802_mutex);
+
 	list_for_each_entry(dev, &cx8802_devlist, devlist) {
 		printk(KERN_INFO
 		       "%s/2: subsystem: %04x:%04x, board: %s [card=%d]\n",
@@ -698,8 +701,10 @@ int cx8802_register_driver(struct cx8802_driver *drv)
 
 		/* Bring up a new struct for each driver instance */
 		driver = kzalloc(sizeof(*drv),GFP_KERNEL);
-		if (driver == NULL)
-			return -ENOMEM;
+		if (driver == NULL) {
+			err = -ENOMEM;
+			goto out;
+		}
 
 		/* Snapshot of the driver registration data */
 		drv->core = dev->core;
@@ -722,7 +727,10 @@ int cx8802_register_driver(struct cx8802_driver *drv)
 		mutex_unlock(&drv->core->lock);
 	}
 
-	return i ? 0 : -ENODEV;
+	err = i ? 0 : -ENODEV;
+out:
+	mutex_unlock(&cx8802_mutex);
+	return err;
 }
 
 int cx8802_unregister_driver(struct cx8802_driver *drv)
@@ -736,6 +744,8 @@ int cx8802_unregister_driver(struct cx8802_driver *drv)
 	       drv->type_id == CX88_MPEG_DVB ? "dvb" : "blackbird",
 	       drv->hw_access == CX8802_DRVCTL_SHARED ? "shared" : "exclusive");
 
+	mutex_lock(&cx8802_mutex);
+
 	list_for_each_entry(dev, &cx8802_devlist, devlist) {
 		printk(KERN_INFO
 		       "%s/2: subsystem: %04x:%04x, board: %s [card=%d]\n",
@@ -762,6 +772,8 @@ int cx8802_unregister_driver(struct cx8802_driver *drv)
 		mutex_unlock(&dev->core->lock);
 	}
 
+	mutex_unlock(&cx8802_mutex);
+
 	return err;
 }
 
@@ -799,7 +811,9 @@ static int __devinit cx8802_probe(struct pci_dev *pci_dev,
 		goto fail_free;
 
 	INIT_LIST_HEAD(&dev->drvlist);
+	mutex_lock(&cx8802_mutex);
 	list_add_tail(&dev->devlist,&cx8802_devlist);
+	mutex_unlock(&cx8802_mutex);
 
 	/* now autoload cx88-dvb or cx88-blackbird */
 	request_modules(dev);
-- 
1.7.5

[PATCH 5/7] [media] cx88: gracefully reject attempts to use unregistered cx88-blackbird driver

[Jonathan Nieder]

It should not be possible to enter mpeg_open and acquire core->lock
without the blackbird driver being registered, so just error out if it
is not.  This makes the code more readable and should prevent the bug
fixed by the patch "hold device lock during sub-driver initialization"
from resurfacing again.

Similarly, if we enter mpeg_release and acquire core->lock then either
the blackbird driver is registered (since open files keep it loaded)
or the sysadmin forced the driver's removal.  In the latter case the
state will be inconsistent and this is worth a loud warning.

Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index f637d34..fa8e347 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1060,18 +1060,21 @@ static int mpeg_open(struct file *file)
 
 	/* Make sure we can acquire the hardware */
 	drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
-	if (drv) {
-		err = drv->request_acquire(drv);
-		if(err != 0) {
-			dprintk(1,"%s: Unable to acquire hardware, %d\n", __func__, err);
-			mutex_unlock(&dev->core->lock);
-			return err;
-		}
+	if (!drv) {
+		dprintk(1, "%s: blackbird driver is not loaded\n", __func__);
+		mutex_unlock(&dev->core->lock);
+		return -ENODEV;
+	}
+
+	err = drv->request_acquire(drv);
+	if (err != 0) {
+		dprintk(1,"%s: Unable to acquire hardware, %d\n", __func__, err);
+		mutex_unlock(&dev->core->lock);
+		return err;
 	}
 
 	if (!atomic_read(&dev->core->mpeg_users) && blackbird_initialize_codec(dev) < 0) {
-		if (drv)
-			drv->request_release(drv);
+		drv->request_release(drv);
 		mutex_unlock(&dev->core->lock);
 		return -EINVAL;
 	}
@@ -1080,8 +1083,7 @@ static int mpeg_open(struct file *file)
 	/* allocate + initialize per filehandle data */
 	fh = kzalloc(sizeof(*fh),GFP_KERNEL);
 	if (NULL == fh) {
-		if (drv)
-			drv->request_release(drv);
+		drv->request_release(drv);
 		mutex_unlock(&dev->core->lock);
 		return -ENOMEM;
 	}
@@ -1125,6 +1127,7 @@ static int mpeg_release(struct file *file)
 
 	/* Make sure we release the hardware */
 	drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
+	WARN_ON(!drv);
 	if (drv)
 		drv->request_release(drv);
 
-- 
1.7.5

[PATCH 6/7] [media] cx88: don't use atomic_t for core->mpeg_users

[Jonathan Nieder]

mpeg_users is always read or written with core->lock held except
in mpeg_release (where it looks like a bug).  A plain int is simpler
and faster.

Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |   11 ++++++-----
 drivers/media/video/cx88/cx88.h           |    2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index fa8e347..11e49bb 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1073,7 +1073,7 @@ static int mpeg_open(struct file *file)
 		return err;
 	}
 
-	if (!atomic_read(&dev->core->mpeg_users) && blackbird_initialize_codec(dev) < 0) {
+	if (!dev->core->mpeg_users && blackbird_initialize_codec(dev) < 0) {
 		drv->request_release(drv);
 		mutex_unlock(&dev->core->lock);
 		return -EINVAL;
@@ -1101,7 +1101,7 @@ static int mpeg_open(struct file *file)
 	cx88_set_scale(dev->core, dev->width, dev->height,
 			fh->mpegq.field);
 
-	atomic_inc(&dev->core->mpeg_users);
+	dev->core->mpeg_users++;
 	mutex_unlock(&dev->core->lock);
 	return 0;
 }
@@ -1112,7 +1112,9 @@ static int mpeg_release(struct file *file)
 	struct cx8802_dev *dev = fh->dev;
 	struct cx8802_driver *drv = NULL;
 
-	if (dev->mpeg_active && atomic_read(&dev->core->mpeg_users) == 1)
+	mutex_lock(&dev->core->lock);
+
+	if (dev->mpeg_active && dev->core->mpeg_users == 1)
 		blackbird_stop_codec(dev);
 
 	cx8802_cancel_buffers(fh->dev);
@@ -1121,7 +1123,6 @@ static int mpeg_release(struct file *file)
 
 	videobuf_mmap_free(&fh->mpegq);
 
-	mutex_lock(&dev->core->lock);
 	file->private_data = NULL;
 	kfree(fh);
 
@@ -1131,7 +1132,7 @@ static int mpeg_release(struct file *file)
 	if (drv)
 		drv->request_release(drv);
 
-	atomic_dec(&dev->core->mpeg_users);
+	dev->core->mpeg_users--;
 
 	mutex_unlock(&dev->core->lock);
 
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index 93a94bf..09e329f 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -384,7 +384,7 @@ struct cx88_core {
 	/* various v4l controls */
 	u32                        freq;
 	atomic_t		   users;
-	atomic_t                   mpeg_users;
+	int                        mpeg_users;
 
 	/* cx88-video needs to access cx8802 for hybrid tuner pll access. */
 	struct cx8802_dev          *dvbdev;
-- 
1.7.5

[PATCH 7/7] [media] cx88: don't use atomic_t for core->users

[Jonathan Nieder]

users is always read or written with core->lock held.  A plain int is
simpler and faster.

Tested-by: Andi Huber <hobrom@gmx.at>
Tested-by: Marlon de Boer <marlon@hyves.nl>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That's the end of the series.  Thanks for reading.

 drivers/media/video/cx88/cx88-video.c |    5 +++--
 drivers/media/video/cx88/cx88.h       |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-video.c b/drivers/media/video/cx88/cx88-video.c
index 508dabb..51d1b09 100644
--- a/drivers/media/video/cx88/cx88-video.c
+++ b/drivers/media/video/cx88/cx88-video.c
@@ -823,7 +823,7 @@ static int video_open(struct file *file)
 		call_all(core, tuner, s_radio);
 	}
 
-	atomic_inc(&core->users);
+	core->users++;
 	mutex_unlock(&core->lock);
 
 	return 0;
@@ -921,7 +921,8 @@ static int video_release(struct file *file)
 	file->private_data = NULL;
 	kfree(fh);
 
-	if(atomic_dec_and_test(&dev->core->users))
+	dev->core->users--;
+	if (!dev->core->users)
 		call_all(dev->core, core, s_power, 0);
 	mutex_unlock(&dev->core->lock);
 
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index 09e329f..887a978 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -383,7 +383,7 @@ struct cx88_core {
 	struct mutex               lock;
 	/* various v4l controls */
 	u32                        freq;
-	atomic_t		   users;
+	int                        users;
 	int                        mpeg_users;
 
 	/* cx88-video needs to access cx8802 for hybrid tuner pll access. */
-- 
1.7.5

Re: [PATCH v2.6.38 resend 0/7] cx88 deadlock and data races

[linuxtv]

FYI I too experienced the problem of hanging and used the patch dated
6th April to get it working.
However I do have the problem that sound does not always work/come on.
Once it is started it stays, getting it started is not reliable.

On 01/05/11 10:17, Jonathan Nieder wrote:
> Hi,
>
> Since v2.6.37 (BKL removal), trying to open a cx88-blackbird driven
> MPEG device would hang.  Ben Hutchings provided an initial patch[1] to
> fix that, and Andi Huber helped a lot in finding races that patch had
> missed.  Ben requested that I take authorship of the series, so I've
> done so.
>
> These patches have visited the list twice before[2], towards the
> beginning of this month.  No feedback from maintainers or reviewers
> --- I'm beginning to wonder if I have the right list.  Luckily two
> people[3][4] experiencing this problem on bugzilla were kind enough to
> test the series and found it worked okay.
>
> Patches are against v2.6.38 (but they do not conflict with anything in
> media-tree/staging/for_v2.6.40).  The intent is to include them in
> v2.6.40 if possible.  A copy of these patches is also available for
> convenient fetching from
>
>   git://repo.or.cz/linux-2.6/jrn.git cx88-locking
>
> Thoughts?
>
> Jonathan Nieder (7):
>   [media] cx88: protect per-device driver list with device lock
>   [media] cx88: fix locking of sub-driver operations
>   [media] cx88: hold device lock during sub-driver initialization
>   [media] cx88: protect cx8802_devlist with a mutex
>   [media] cx88: gracefully reject attempts to use unregistered
>     cx88-blackbird driver
>   [media] cx88: don't use atomic_t for core->mpeg_users
>   [media] cx88: don't use atomic_t for core->users
>
>  drivers/media/video/cx88/cx88-blackbird.c |   41 +++++++++++++++-------------
>  drivers/media/video/cx88/cx88-dvb.c       |    2 +
>  drivers/media/video/cx88/cx88-mpeg.c      |   40 ++++++++++++++++++----------
>  drivers/media/video/cx88/cx88-video.c     |    5 ++-
>  drivers/media/video/cx88/cx88.h           |   11 +++++--
>  5 files changed, 61 insertions(+), 38 deletions(-)
>
> [1] http://bugs.debian.org/619827
> [2] http://thread.gmane.org/gmane.linux.kernel/1118815
> [3] https://bugzilla.kernel.org/show_bug.cgi?id=31792
> [4] https://bugzilla.kernel.org/show_bug.cgi?id=31962
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

cx88 sound does not always work (Re: [PATCH v2.6.38 resend 0/7] cx88 deadlock and data races)

[Jonathan Nieder]

Hi,

linuxtv wrote:

> FYI I too experienced the problem of hanging and used the patch dated
> 6th April to get it working.
> However I do have the problem that sound does not always work/come on.
> Once it is started it stays, getting it started is not reliable.

Could you give details?  What card do you use?  Does it show up in
lspci -vvnn output (and if so, could you show us)?  What kernel
version?  Could you attach your .config and dmesg?  Was this reported
on bugzilla before?  How does sound not working manifest itself?  How
do you go about getting it to work?

See the REPORTING-BUGS file for hints.

Thanks and hope that helps,
Jonathan

Re: cx88 sound does not always work (Re: [PATCH v2.6.38 resend 0/7] cx88 deadlock and data races)

[linuxtv]

Card Hauppage HVR-1300
Does it show up - yes
Does it work - yes.

However when testing for audio, either via mythbackend or smplayer
/dev/video1 I get no sound 75% of the time on a first run.

Of that, 75% of the time if I run smplayer /dev/video1 a few times sound
reappears and will stay there until a power off reboot. (Soft reboot
will keep the sound on).
25% of the time I cannot get sound started at all. Either via smplayer,
mplayer, mythbackend or v4lctl changes.

Have I seen this reported ever? I saw something mentioned on a mailing
list dated Aug 2010. But no resolution.

Is it hardware ? I don't believe so, same hardware I have linux Suse
11.1 kernel 2.6.27 with custom built drivers from v4l (July 2009). This
works 100%.

Drivers I was using was the default from the kernel with 11.4 (below). I
then switched to try the v4l media_build repository (plus your patch).
Unfortunately I can't build the 2009 drivers to try that level out (too
much has changed).

Hope the information below is of use.

Drivers used from the default SuSE build and also from the v4l media build.

Linux pvr1 2.6.37.1-1.2-desktop #1 SMP PREEMPT 2011-02-21 10:34:10 +0100
x86_64 x86_64 x86_64 GNU/Linux (SuSE 11.4)


04:01.0 Multimedia video controller [0400]: Conexant Systems, Inc.
CX23880/1/2/3 PCI Video and Audio Decoder [14f1:8800] (re$
        Subsystem: Hauppauge computer works Inc. WinTV 88x Video [0070:9600]
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 32 (5000ns min, 13750ns max), Cache Line Size: 32 bytes
        Interrupt: pin A routed to IRQ 19
        Region 0: Memory at e4000000 (32-bit, non-prefetchable) [size=16M]
        Capabilities: [44] Vital Product Data
                Unknown large resource type 04, will not decode more.
        Capabilities: [4c] Power Management version 2
                Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Kernel driver in use: cx8800

04:01.1 Multimedia controller [0480]: Conexant Systems, Inc.
CX23880/1/2/3 PCI Video and Audio Decoder [Audio Port] [14f1:88$
        Subsystem: Hauppauge computer works Inc. WinTV 88x Audio [0070:9600]
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 32 (1000ns min, 63750ns max), Cache Line Size: 32 bytes
        Interrupt: pin A routed to IRQ 19
        Region 0: Memory at e5000000 (32-bit, non-prefetchable) [size=16M]
        Capabilities: [4c] Power Management version 2
                Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Kernel driver in use: cx88_audio


04:01.2 Multimedia controller [0480]: Conexant Systems, Inc.
CX23880/1/2/3 PCI Video and Audio Decoder [MPEG Port] [14f1:880$
        Subsystem: Hauppauge computer works Inc. WinTV 88x MPEG Encoder
[0070:9600]
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 32 (1500ns min, 22000ns max), Cache Line Size: 32 bytes
        Interrupt: pin A routed to IRQ 19
        Region 0: Memory at e6000000 (32-bit, non-prefetchable) [size=16M]
        Capabilities: [4c] Power Management version 2
                Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Kernel driver in use: cx88-mpeg driver manager



dmesg extract
[    6.341606] tda9887 1-0043: creating new
instance                                                                        

[    6.341607] tda9887 1-0043: tda988[5/6/7]
found                                                                          

[    6.342842] tuner 1-0043: Tuner 74 found with type(s) Radio
TV.                                                          
[    6.346330] tuner 1-0061: Tuner -1 found with type(s) Radio
TV.                                                          
[    6.386123] tveeprom 1-0050: Hauppauge model 96559, rev C5A0, serial#
825267                                             
[    6.386125] tveeprom 1-0050: MAC address is
00:0d:fe:0c:97:b3                                                            

[    6.386127] tveeprom 1-0050: tuner model is Philips FMD1216ME (idx
100, type 63)                                         
[    6.386129] tveeprom 1-0050: TV standards PAL(B/G) PAL(I) SECAM(L/L')
PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)         
[    6.386131] tveeprom 1-0050: audio processor is CX882 (idx
33)                                                           
[    6.386133] tveeprom 1-0050: decoder processor is CX882 (idx
25)                                                         
[    6.386134] tveeprom 1-0050: has
radio                                                                                   

[    6.386136] cx88[0]: hauppauge eeprom:
model=96559                                                                       

[    6.407438] tuner-simple 1-0061: creating new
instance                                                                   
[    6.407440] tuner-simple 1-0061: type set to 63 (Philips FMD1216ME
MK3 Hybrid Tuner)                                     
[    6.413602] cx88[0]/1: CX88x/0: ALSA support for cx2388x
boards                                                          
[    6.413710] cx88[0]/2: cx2388x 8802 Driver
Manager                                                                       

[    6.413720] cx88-mpeg driver manager 0000:04:01.2: PCI INT A -> GSI
19 (level, low) -> IRQ 19                            
[    6.413725] cx88[0]/2: found at 0000:04:01.2, rev: 5, irq: 19,
latency: 32, mmio: 0xe6000000                             
[    6.413769] cx8800 0000:04:01.0: PCI INT A -> GSI 19 (level, low) ->
IRQ 19                                              
[    6.413773] cx88[0]/0: found at 0000:04:01.0, rev: 5, irq: 19,
latency: 32, mmio: 0xe4000000                             
[    6.463568] WARNING: You are using an experimental version of the
media stack.                                           
[    6.463569]  As the driver is backported to an older kernel, it
doesn't offer                                            
[    6.463570]  enough quality for its usage in
production.                                                                 

[    6.463571]  Use it with
care.                                                                                           

[    6.463571] Latest git patches (needed if you report a bug to
linux-media@vger.kernel.org):                              
[    6.463572]  847aae409344e3c2efcc58e0639e659427447388 [media]
lmedm04: get rid of on-stack dma buffers                   
[    6.463573]  d71d07543c9bd2ea6779af91a3dc185bc8710d7c [media] au6610:
get rid of on-stack dma buffer                     
[    6.463573]  036d3f3f98f8b4c513bbe0bc8ccf932e5c8a72b6 [media] ce6230:
get rid of on-stack dma buffer                     
[    6.473441] wm8775 1-001b: chip found @ 0x36
(cx88[0])                                                                   

[    6.479106] cx88/2: cx2388x dvb driver version 0.0.8
loaded                                                              
[    6.479108] cx88/2: registering cx8802 driver, type: dvb access:
shared                                                  
[    6.479110] cx88[0]/2: subsystem: 0070:9600, board: Hauppauge
WinTV-HVR1300 DVB-T/Hybrid MPEG Encoder [card=56]          
[    6.479112] cx88[0]/2: cx2388x based DVB/ATSC
card                                                                       
[    6.479113] cx8802_alloc_frontends() allocating 1
frontend(s)                                                            
[    6.518120] tuner-simple 1-0061: attaching existing
instance                                                             
[    6.518123] tuner-simple 1-0061: type set to 63 (Philips FMD1216ME
MK3 Hybrid Tuner)                                     
[    6.521942] DVB: registering new adapter
(cx88[0])                                                                       

[    6.521944] DVB: registering adapter 0 frontend 0 (Conexant CX22702
DVB-T)...                                            
[    6.544641] cx88[0]/0: registered device video0
[v4l2]                                                                   
[    6.544665] cx88[0]/0: registered device
vbi0                                                                            

[    6.544686] cx88[0]/0: registered device
radio0                                                                          

[    6.667451] cx2388x blackbird driver version 0.0.8
loaded                                                                
[    6.667453] cx88/2: registering cx8802 driver, type: blackbird
access: shared                                            
[    6.667456] cx88[0]/2: subsystem: 0070:9600, board: Hauppauge
WinTV-HVR1300 DVB-T/Hybrid MPEG Encoder [card=56]          
[    6.667458] cx88[0]/2: cx23416 based mpeg encoder (blackbird
reference design)                                           
[    6.667678] cx88[0]/2-bb: Firmware and/or mailbox pointer not
initialized or corrupted                                   
[    9.436018] cx88[0]/2-bb: Firmware upload
successful.                                                                    

[    9.439795] cx88[0]/2-bb: Firmware version is
0x02060039                                                                 
[    9.446678] cx88[0]/2: registered device video1 [mpeg]


On 02/05/11 09:19, Jonathan Nieder wrote:
> Hi,
>
> linuxtv wrote:
>
>   
>> FYI I too experienced the problem of hanging and used the patch dated
>> 6th April to get it working.
>> However I do have the problem that sound does not always work/come on.
>> Once it is started it stays, getting it started is not reliable.
>>     
> Could you give details?  What card do you use?  Does it show up in
> lspci -vvnn output (and if so, could you show us)?  What kernel
> version?  Could you attach your .config and dmesg?  Was this reported
> on bugzilla before?  How does sound not working manifest itself?  How
> do you go about getting it to work?
>
> See the REPORTING-BUGS file for hints.
>
> Thanks and hope that helps,
> Jonathan
>   

Re: cx88 sound does not always work (Re: [PATCH v2.6.38 resend 0/7] cx88 deadlock and data races)

[hubstar]

Update:

I noticed I was using the desktop kernel this time, and reinstalled the
default kernel. The audio now works fine (once this patch is installed).
Works every time.

This rings a bell, like I've had to do this to a system in the past.

I don't really understand what desktop vs default kernel would have an
effect on the drivers - from what I can read there doesn't seem to be
anything in there.




On 02/05/11 19:40, linuxtv wrote:
> Card Hauppage HVR-1300
> Does it show up - yes
> Does it work - yes.
>
> However when testing for audio, either via mythbackend or smplayer
> /dev/video1 I get no sound 75% of the time on a first run.
>
> Of that, 75% of the time if I run smplayer /dev/video1 a few times sound
> reappears and will stay there until a power off reboot. (Soft reboot
> will keep the sound on).
> 25% of the time I cannot get sound started at all. Either via smplayer,
> mplayer, mythbackend or v4lctl changes.
>
> Have I seen this reported ever? I saw something mentioned on a mailing
> list dated Aug 2010. But no resolution.
>
> Is it hardware ? I don't believe so, same hardware I have linux Suse
> 11.1 kernel 2.6.27 with custom built drivers from v4l (July 2009). This
> works 100%.
>
> Drivers I was using was the default from the kernel with 11.4 (below). I
> then switched to try the v4l media_build repository (plus your patch).
> Unfortunately I can't build the 2009 drivers to try that level out (too
> much has changed).
>
> Hope the information below is of use.
>
> Drivers used from the default SuSE build and also from the v4l media build.
>
> Linux pvr1 2.6.37.1-1.2-desktop #1 SMP PREEMPT 2011-02-21 10:34:10 +0100
> x86_64 x86_64 x86_64 GNU/Linux (SuSE 11.4)
>
>
> 04:01.0 Multimedia video controller [0400]: Conexant Systems, Inc.
> CX23880/1/2/3 PCI Video and Audio Decoder [14f1:8800] (re$
>         Subsystem: Hauppauge computer works Inc. WinTV 88x Video [0070:9600]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 32 (5000ns min, 13750ns max), Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 19
>         Region 0: Memory at e4000000 (32-bit, non-prefetchable) [size=16M]
>         Capabilities: [44] Vital Product Data
>                 Unknown large resource type 04, will not decode more.
>         Capabilities: [4c] Power Management version 2
>                 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>         Kernel driver in use: cx8800
>
> 04:01.1 Multimedia controller [0480]: Conexant Systems, Inc.
> CX23880/1/2/3 PCI Video and Audio Decoder [Audio Port] [14f1:88$
>         Subsystem: Hauppauge computer works Inc. WinTV 88x Audio [0070:9600]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 32 (1000ns min, 63750ns max), Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 19
>         Region 0: Memory at e5000000 (32-bit, non-prefetchable) [size=16M]
>         Capabilities: [4c] Power Management version 2
>                 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>         Kernel driver in use: cx88_audio
>
>
> 04:01.2 Multimedia controller [0480]: Conexant Systems, Inc.
> CX23880/1/2/3 PCI Video and Audio Decoder [MPEG Port] [14f1:880$
>         Subsystem: Hauppauge computer works Inc. WinTV 88x MPEG Encoder
> [0070:9600]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 32 (1500ns min, 22000ns max), Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 19
>         Region 0: Memory at e6000000 (32-bit, non-prefetchable) [size=16M]
>         Capabilities: [4c] Power Management version 2
>                 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>         Kernel driver in use: cx88-mpeg driver manager
>
>
>
> dmesg extract
> [    6.341606] tda9887 1-0043: creating new
> instance                                                                        
>
> [    6.341607] tda9887 1-0043: tda988[5/6/7]
> found                                                                          
>
> [    6.342842] tuner 1-0043: Tuner 74 found with type(s) Radio
> TV.                                                          
> [    6.346330] tuner 1-0061: Tuner -1 found with type(s) Radio
> TV.                                                          
> [    6.386123] tveeprom 1-0050: Hauppauge model 96559, rev C5A0, serial#
> 825267                                             
> [    6.386125] tveeprom 1-0050: MAC address is
> 00:0d:fe:0c:97:b3                                                            
>
> [    6.386127] tveeprom 1-0050: tuner model is Philips FMD1216ME (idx
> 100, type 63)                                         
> [    6.386129] tveeprom 1-0050: TV standards PAL(B/G) PAL(I) SECAM(L/L')
> PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)         
> [    6.386131] tveeprom 1-0050: audio processor is CX882 (idx
> 33)                                                           
> [    6.386133] tveeprom 1-0050: decoder processor is CX882 (idx
> 25)                                                         
> [    6.386134] tveeprom 1-0050: has
> radio                                                                                   
>
> [    6.386136] cx88[0]: hauppauge eeprom:
> model=96559                                                                       
>
> [    6.407438] tuner-simple 1-0061: creating new
> instance                                                                   
> [    6.407440] tuner-simple 1-0061: type set to 63 (Philips FMD1216ME
> MK3 Hybrid Tuner)                                     
> [    6.413602] cx88[0]/1: CX88x/0: ALSA support for cx2388x
> boards                                                          
> [    6.413710] cx88[0]/2: cx2388x 8802 Driver
> Manager                                                                       
>
> [    6.413720] cx88-mpeg driver manager 0000:04:01.2: PCI INT A -> GSI
> 19 (level, low) -> IRQ 19                            
> [    6.413725] cx88[0]/2: found at 0000:04:01.2, rev: 5, irq: 19,
> latency: 32, mmio: 0xe6000000                             
> [    6.413769] cx8800 0000:04:01.0: PCI INT A -> GSI 19 (level, low) ->
> IRQ 19                                              
> [    6.413773] cx88[0]/0: found at 0000:04:01.0, rev: 5, irq: 19,
> latency: 32, mmio: 0xe4000000                             
> [    6.463568] WARNING: You are using an experimental version of the
> media stack.                                           
> [    6.463569]  As the driver is backported to an older kernel, it
> doesn't offer                                            
> [    6.463570]  enough quality for its usage in
> production.                                                                 
>
> [    6.463571]  Use it with
> care.                                                                                           
>
> [    6.463571] Latest git patches (needed if you report a bug to
> linux-media@vger.kernel.org):                              
> [    6.463572]  847aae409344e3c2efcc58e0639e659427447388 [media]
> lmedm04: get rid of on-stack dma buffers                   
> [    6.463573]  d71d07543c9bd2ea6779af91a3dc185bc8710d7c [media] au6610:
> get rid of on-stack dma buffer                     
> [    6.463573]  036d3f3f98f8b4c513bbe0bc8ccf932e5c8a72b6 [media] ce6230:
> get rid of on-stack dma buffer                     
> [    6.473441] wm8775 1-001b: chip found @ 0x36
> (cx88[0])                                                                   
>
> [    6.479106] cx88/2: cx2388x dvb driver version 0.0.8
> loaded                                                              
> [    6.479108] cx88/2: registering cx8802 driver, type: dvb access:
> shared                                                  
> [    6.479110] cx88[0]/2: subsystem: 0070:9600, board: Hauppauge
> WinTV-HVR1300 DVB-T/Hybrid MPEG Encoder [card=56]          
> [    6.479112] cx88[0]/2: cx2388x based DVB/ATSC
> card                                                                       
> [    6.479113] cx8802_alloc_frontends() allocating 1
> frontend(s)                                                            
> [    6.518120] tuner-simple 1-0061: attaching existing
> instance                                                             
> [    6.518123] tuner-simple 1-0061: type set to 63 (Philips FMD1216ME
> MK3 Hybrid Tuner)                                     
> [    6.521942] DVB: registering new adapter
> (cx88[0])                                                                       
>
> [    6.521944] DVB: registering adapter 0 frontend 0 (Conexant CX22702
> DVB-T)...                                            
> [    6.544641] cx88[0]/0: registered device video0
> [v4l2]                                                                   
> [    6.544665] cx88[0]/0: registered device
> vbi0                                                                            
>
> [    6.544686] cx88[0]/0: registered device
> radio0                                                                          
>
> [    6.667451] cx2388x blackbird driver version 0.0.8
> loaded                                                                
> [    6.667453] cx88/2: registering cx8802 driver, type: blackbird
> access: shared                                            
> [    6.667456] cx88[0]/2: subsystem: 0070:9600, board: Hauppauge
> WinTV-HVR1300 DVB-T/Hybrid MPEG Encoder [card=56]          
> [    6.667458] cx88[0]/2: cx23416 based mpeg encoder (blackbird
> reference design)                                           
> [    6.667678] cx88[0]/2-bb: Firmware and/or mailbox pointer not
> initialized or corrupted                                   
> [    9.436018] cx88[0]/2-bb: Firmware upload
> successful.                                                                    
>
> [    9.439795] cx88[0]/2-bb: Firmware version is
> 0x02060039                                                                 
> [    9.446678] cx88[0]/2: registered device video1 [mpeg]
>
>
> On 02/05/11 09:19, Jonathan Nieder wrote:
>   
>> Hi,
>>
>> linuxtv wrote:
>>
>>   
>>     
>>> FYI I too experienced the problem of hanging and used the patch dated
>>> 6th April to get it working.
>>> However I do have the problem that sound does not always work/come on.
>>> Once it is started it stays, getting it started is not reliable.
>>>     
>>>       
>> Could you give details?  What card do you use?  Does it show up in
>> lspci -vvnn output (and if so, could you show us)?  What kernel
>> version?  Could you attach your .config and dmesg?  Was this reported
>> on bugzilla before?  How does sound not working manifest itself?  How
>> do you go about getting it to work?
>>
>> See the REPORTING-BUGS file for hints.
>>
>> Thanks and hope that helps,
>> Jonathan
>>   
>>     
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: cx88 sound does not always work

[Jonathan Nieder]

(culling cc list)
hubstar wrote:

> Update:
>
> I noticed I was using the desktop kernel this time, and reinstalled the
> default kernel. The audio now works fine (once this patch is installed).
> Works every time.
>
> This rings a bell, like I've had to do this to a system in the past.
>
> I don't really understand what desktop vs default kernel would have an
> effect on the drivers - from what I can read there doesn't seem to be
> anything in there.

Alas, I don't even know who these desktop and default kernel fellows
are. :)  I'd suggest reporting to whoever supplied your kernel.

Thanks for working to make Linux support hardware well.

Regards,
Jonathan

Re: cx88 sound does not always work

[hubstar]

Will do , just for limited into

Distro OpenSuSE though I think I've seen Ubuntu has it as well.

desktop -- supposed to be more responsive to user (timer set to 1000hz + other tunings)
default -- supposed to be better for server and others






On 04/05/11 10:23, Jonathan Nieder wrote:
> (culling cc list)
> hubstar wrote:
>
>   
>> Update:
>>
>> I noticed I was using the desktop kernel this time, and reinstalled the
>> default kernel. The audio now works fine (once this patch is installed).
>> Works every time.
>>
>> This rings a bell, like I've had to do this to a system in the past.
>>
>> I don't really understand what desktop vs default kernel would have an
>> effect on the drivers - from what I can read there doesn't seem to be
>> anything in there.
>>     
> Alas, I don't even know who these desktop and default kernel fellows
> are. :)  I'd suggest reporting to whoever supplied your kernel.
>
> Thanks for working to make Linux support hardware well.
>
> Regards,
> Jonathan
>   

[PATCH 5/7] [media] cx88: gracefully reject attempts to use unregistered cx88-blackbird driver

[Jonathan Nieder]

It should not be possible to enter mpeg_open and acquire core->lock
without the blackbird driver being registered, so just error out if it
is not.  This makes the code more readable and should prevent the bug
fixed by the patch "hold device lock during sub-driver initialization"
from resurfacing again.

Similarly, if we enter mpeg_release and acquire core->lock then either
the blackbird driver is registered (since open files keep it loaded)
or the sysadmin forced the driver's removal.  In the latter case the
state will be inconsistent and this is worth a loud warning.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index f637d34..fa8e347 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1060,18 +1060,21 @@ static int mpeg_open(struct file *file)
 
 	/* Make sure we can acquire the hardware */
 	drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
-	if (drv) {
-		err = drv->request_acquire(drv);
-		if(err != 0) {
-			dprintk(1,"%s: Unable to acquire hardware, %d\n", __func__, err);
-			mutex_unlock(&dev->core->lock);
-			return err;
-		}
+	if (!drv) {
+		dprintk(1, "%s: blackbird driver is not loaded\n", __func__);
+		mutex_unlock(&dev->core->lock);
+		return -ENODEV;
+	}
+
+	err = drv->request_acquire(drv);
+	if (err != 0) {
+		dprintk(1,"%s: Unable to acquire hardware, %d\n", __func__, err);
+		mutex_unlock(&dev->core->lock);
+		return err;
 	}
 
 	if (!atomic_read(&dev->core->mpeg_users) && blackbird_initialize_codec(dev) < 0) {
-		if (drv)
-			drv->request_release(drv);
+		drv->request_release(drv);
 		mutex_unlock(&dev->core->lock);
 		return -EINVAL;
 	}
@@ -1080,8 +1083,7 @@ static int mpeg_open(struct file *file)
 	/* allocate + initialize per filehandle data */
 	fh = kzalloc(sizeof(*fh),GFP_KERNEL);
 	if (NULL == fh) {
-		if (drv)
-			drv->request_release(drv);
+		drv->request_release(drv);
 		mutex_unlock(&dev->core->lock);
 		return -ENOMEM;
 	}
@@ -1125,6 +1127,7 @@ static int mpeg_release(struct file *file)
 
 	/* Make sure we release the hardware */
 	drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
+	WARN_ON(!drv);
 	if (drv)
 		drv->request_release(drv);
 
-- 
1.7.5.rc0