V4L/DVB (7321): pvrusb2: Rework context handling and initialization
This change significantly rearranges pvr2_context level initialization
and operation:
1. A new kernel thread is set up for management of the context.
2. Destruction of the pvr2_context instance is moved into the kernel
thread. No other context is able to remove the instance; doing
this simplifies lock handling.
3. The callback into pvrusb2-main, which is used to trigger
initialization of each interface, is now issued from this kernel
thread. Previously it had been indirectly issued out of the work
queue thread in pvr2_hdw, which led to deadlock issues if the
interface needed to change a control setting (which in turn
requires dispatch of another work queue entry).
4. Callbacks into the interfaces (via the pvr2_channel structure) are
now issued strictly from this thread. The net result of this is
that such callback functions can now also safely operate driver
controls without deadlocking the work queue. (At the moment this
is not actually a problem, but I'm anticipating issues with this in
the future).
5. There is no longer any need for anyone to enter / exit the
pvr2_context structure. Implementation of the kernel thread here
allows this all to be internal now, simplifying other logic.
6. A very very longstanding issue involving a mutex deadlock between
the pvrusb2 driver and v4l should now be solved. The deadlock
involved the pvr2_context mutex and a globals-protecting mutex in
v4l. During initialization the driver would take the pvr2_context
mutex first then the v4l2 interface would register with v4l and
implicitly take the v4l mutex. Later when v4l would call back into
the driver, the two mutexes could possibly be taken in the opposite
order, a situation that can lead to deadlock. In practice this
really wasn't an issue unless a v4l app tried to start VERY early
after the driver appeared. However it still needed to be solved,
and with the use of the kernel thread relieving need for
pvr2_context mutex, the problem should be finally solved.
Signed-off-by: Mike Isely <isely@pobox.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
diff --git a/drivers/media/video/pvrusb2/pvrusb2-context.c b/drivers/media/video/pvrusb2/pvrusb2-context.c
index 953784c..22bd830 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-context.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-context.c
@@ -23,6 +23,7 @@
#include "pvrusb2-ioread.h"
#include "pvrusb2-hdw.h"
#include "pvrusb2-debug.h"
+#include <linux/kthread.h>
#include <linux/errno.h>
#include <linux/string.h>
#include <linux/slab.h>
@@ -30,32 +31,65 @@
static void pvr2_context_destroy(struct pvr2_context *mp)
{
- pvr2_trace(PVR2_TRACE_STRUCT,"Destroying pvr_main id=%p",mp);
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (destroy)",mp);
if (mp->hdw) pvr2_hdw_destroy(mp->hdw);
kfree(mp);
}
-static void pvr2_context_state_check(struct pvr2_context *mp)
+static void pvr2_context_notify(struct pvr2_context *mp)
{
- if (mp->init_flag) return;
+ mp->notify_flag = !0;
+ wake_up(&mp->wait_data);
+}
- switch (pvr2_hdw_get_state(mp->hdw)) {
- case PVR2_STATE_WARM: break;
- case PVR2_STATE_ERROR: break;
- case PVR2_STATE_READY: break;
- case PVR2_STATE_RUN: break;
- default: return;
+
+static int pvr2_context_thread(void *_mp)
+{
+ struct pvr2_channel *ch1,*ch2;
+ struct pvr2_context *mp = _mp;
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread start)",mp);
+
+ /* Finish hardware initialization */
+ if (pvr2_hdw_initialize(mp->hdw,
+ (void (*)(void *))pvr2_context_notify,mp)) {
+ mp->video_stream.stream =
+ pvr2_hdw_get_video_stream(mp->hdw);
+ /* Trigger interface initialization. By doing this here
+ initialization runs in our own safe and cozy thread
+ context. */
+ if (mp->setup_func) mp->setup_func(mp);
+ } else {
+ pvr2_trace(PVR2_TRACE_CTXT,
+ "pvr2_context %p (thread skipping setup)",mp);
+ /* Even though initialization did not succeed, we're still
+ going to enter the wait loop anyway. We need to do this
+ in order to await the expected disconnect (which we will
+ detect in the normal course of operation). */
}
- pvr2_context_enter(mp); do {
- mp->init_flag = !0;
- mp->video_stream.stream = pvr2_hdw_get_video_stream(mp->hdw);
- if (mp->setup_func) {
- mp->setup_func(mp);
+ /* Now just issue callbacks whenever hardware state changes or if
+ there is a disconnect. If there is a disconnect and there are
+ no channels left, then there's no reason to stick around anymore
+ so we'll self-destruct - tearing down the rest of this driver
+ instance along the way. */
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread enter loop)",mp);
+ while (!mp->disconnect_flag || mp->mc_first) {
+ if (mp->notify_flag) {
+ mp->notify_flag = 0;
+ pvr2_trace(PVR2_TRACE_CTXT,
+ "pvr2_context %p (thread notify)",mp);
+ for (ch1 = mp->mc_first; ch1; ch1 = ch2) {
+ ch2 = ch1->mc_next;
+ if (ch1->check_func) ch1->check_func(ch1);
+ }
}
- } while (0); pvr2_context_exit(mp);
- }
+ wait_event_interruptible(mp->wait_data, mp->notify_flag);
+ }
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread end)",mp);
+ pvr2_context_destroy(mp);
+ return 0;
+}
struct pvr2_context *pvr2_context_create(
@@ -63,10 +97,12 @@
const struct usb_device_id *devid,
void (*setup_func)(struct pvr2_context *))
{
+ struct task_struct *thread;
struct pvr2_context *mp = NULL;
mp = kzalloc(sizeof(*mp),GFP_KERNEL);
if (!mp) goto done;
- pvr2_trace(PVR2_TRACE_STRUCT,"Creating pvr_main id=%p",mp);
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (create)",mp);
+ init_waitqueue_head(&mp->wait_data);
mp->setup_func = setup_func;
mutex_init(&mp->mutex);
mp->hdw = pvr2_hdw_create(intf,devid);
@@ -75,57 +111,45 @@
mp = NULL;
goto done;
}
- pvr2_hdw_initialize(mp->hdw,
- (void (*)(void *))pvr2_context_state_check,
- mp);
+ thread = kthread_run(pvr2_context_thread, mp, "pvrusb2-context");
+ if (!thread) {
+ pvr2_context_destroy(mp);
+ mp = NULL;
+ goto done;
+ }
done:
return mp;
}
-void pvr2_context_enter(struct pvr2_context *mp)
+static void pvr2_context_enter(struct pvr2_context *mp)
{
mutex_lock(&mp->mutex);
- pvr2_trace(PVR2_TRACE_CREG,"pvr2_context_enter(id=%p)",mp);
}
-void pvr2_context_exit(struct pvr2_context *mp)
+static void pvr2_context_exit(struct pvr2_context *mp)
{
int destroy_flag = 0;
if (!(mp->mc_first || !mp->disconnect_flag)) {
destroy_flag = !0;
}
- pvr2_trace(PVR2_TRACE_CREG,"pvr2_context_exit(id=%p) outside",mp);
mutex_unlock(&mp->mutex);
- if (destroy_flag) pvr2_context_destroy(mp);
-}
-
-
-static void pvr2_context_run_checks(struct pvr2_context *mp)
-{
- struct pvr2_channel *ch1,*ch2;
- for (ch1 = mp->mc_first; ch1; ch1 = ch2) {
- ch2 = ch1->mc_next;
- if (ch1->check_func) {
- ch1->check_func(ch1);
- }
- }
+ if (destroy_flag) pvr2_context_notify(mp);
}
void pvr2_context_disconnect(struct pvr2_context *mp)
{
- pvr2_context_enter(mp); do {
- pvr2_hdw_disconnect(mp->hdw);
- mp->disconnect_flag = !0;
- pvr2_context_run_checks(mp);
- } while (0); pvr2_context_exit(mp);
+ pvr2_hdw_disconnect(mp->hdw);
+ mp->disconnect_flag = !0;
+ pvr2_context_notify(mp);
}
void pvr2_channel_init(struct pvr2_channel *cp,struct pvr2_context *mp)
{
+ pvr2_context_enter(mp);
cp->hdw = mp->hdw;
cp->mc_head = mp;
cp->mc_next = NULL;
@@ -136,6 +160,7 @@
mp->mc_first = cp;
}
mp->mc_last = cp;
+ pvr2_context_exit(mp);
}
@@ -151,6 +176,7 @@
void pvr2_channel_done(struct pvr2_channel *cp)
{
struct pvr2_context *mp = cp->mc_head;
+ pvr2_context_enter(mp);
pvr2_channel_disclaim_stream(cp);
if (cp->mc_next) {
cp->mc_next->mc_prev = cp->mc_prev;
@@ -163,6 +189,7 @@
mp->mc_first = cp->mc_next;
}
cp->hdw = NULL;
+ pvr2_context_exit(mp);
}
diff --git a/drivers/media/video/pvrusb2/pvrusb2-context.h b/drivers/media/video/pvrusb2/pvrusb2-context.h
index 96b255f..127ec53 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-context.h
+++ b/drivers/media/video/pvrusb2/pvrusb2-context.h
@@ -43,8 +43,10 @@
struct pvr2_hdw *hdw;
struct pvr2_context_stream video_stream;
struct mutex mutex;
+ int notify_flag;
int disconnect_flag;
- int init_flag;
+
+ wait_queue_head_t wait_data;
/* Called after pvr2_context initialization is complete */
void (*setup_func)(struct pvr2_context *);
@@ -60,9 +62,6 @@
void (*check_func)(struct pvr2_channel *);
};
-void pvr2_context_enter(struct pvr2_context *);
-void pvr2_context_exit(struct pvr2_context *);
-
struct pvr2_context *pvr2_context_create(struct usb_interface *intf,
const struct usb_device_id *devid,
void (*setup_func)(struct pvr2_context *));
diff --git a/drivers/media/video/pvrusb2/pvrusb2-debug.h b/drivers/media/video/pvrusb2/pvrusb2-debug.h
index fca49d8..11537dd 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-debug.h
+++ b/drivers/media/video/pvrusb2/pvrusb2-debug.h
@@ -39,7 +39,7 @@
#define PVR2_TRACE_EEPROM (1 << 10) /* eeprom parsing / report */
#define PVR2_TRACE_STRUCT (1 << 11) /* internal struct creation */
#define PVR2_TRACE_OPEN_CLOSE (1 << 12) /* application open / close */
-#define PVR2_TRACE_CREG (1 << 13) /* Main critical region entry / exit */
+#define PVR2_TRACE_CTXT (1 << 13) /* Main context tracking */
#define PVR2_TRACE_SYSFS (1 << 14) /* Sysfs driven I/O */
#define PVR2_TRACE_FIRMWARE (1 << 15) /* firmware upload actions */
#define PVR2_TRACE_CHIPS (1 << 16) /* chip broadcast operation */
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h b/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
index bc341ae..c725495 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
@@ -187,7 +187,6 @@
struct workqueue_struct *workqueue;
struct work_struct workpoll; /* Update driver state */
struct work_struct worki2csync; /* Update i2c clients */
- struct work_struct workinit; /* Driver initialization sequence */
/* Video spigot */
struct pvr2_stream *vid_stream;
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
index 1dff2d0..7e7c570 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
@@ -221,7 +221,6 @@
static void pvr2_hdw_set_cur_freq(struct pvr2_hdw *,unsigned long);
static void pvr2_hdw_worker_i2c(struct work_struct *work);
static void pvr2_hdw_worker_poll(struct work_struct *work);
-static void pvr2_hdw_worker_init(struct work_struct *work);
static int pvr2_hdw_wait(struct pvr2_hdw *,int state);
static int pvr2_hdw_untrip_unlocked(struct pvr2_hdw *);
static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
@@ -1816,15 +1815,16 @@
/* Perform second stage initialization. Set callback pointer first so that
we can avoid a possible initialization race (if the kernel thread runs
before the callback has been set). */
-void pvr2_hdw_initialize(struct pvr2_hdw *hdw,
- void (*callback_func)(void *),
- void *callback_data)
+int pvr2_hdw_initialize(struct pvr2_hdw *hdw,
+ void (*callback_func)(void *),
+ void *callback_data)
{
LOCK_TAKE(hdw->big_lock); do {
hdw->state_data = callback_data;
hdw->state_func = callback_func;
} while (0); LOCK_GIVE(hdw->big_lock);
- queue_work(hdw->workqueue,&hdw->workinit);
+ pvr2_hdw_setup(hdw);
+ return hdw->flag_init_ok;
}
@@ -2032,7 +2032,6 @@
hdw->workqueue = create_singlethread_workqueue(hdw->name);
INIT_WORK(&hdw->workpoll,pvr2_hdw_worker_poll);
INIT_WORK(&hdw->worki2csync,pvr2_hdw_worker_i2c);
- INIT_WORK(&hdw->workinit,pvr2_hdw_worker_init);
pvr2_trace(PVR2_TRACE_INIT,"Driver unit number is %d, name is %s",
hdw->unit_number,hdw->name);
@@ -2517,15 +2516,6 @@
}
-static void pvr2_hdw_worker_init(struct work_struct *work)
-{
- struct pvr2_hdw *hdw = container_of(work,struct pvr2_hdw,workinit);
- LOCK_TAKE(hdw->big_lock); do {
- pvr2_hdw_setup(hdw);
- } while (0); LOCK_GIVE(hdw->big_lock);
-}
-
-
static int pvr2_hdw_wait(struct pvr2_hdw *hdw,int state)
{
return wait_event_interruptible(
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.h b/drivers/media/video/pvrusb2/pvrusb2-hdw.h
index 597ee50..a868e52 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.h
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.h
@@ -103,9 +103,9 @@
/* Perform second stage initialization, passing in a notification callback
for when the master state changes. */
-void pvr2_hdw_initialize(struct pvr2_hdw *,
- void (*callback_func)(void *),
- void *callback_data);
+int pvr2_hdw_initialize(struct pvr2_hdw *,
+ void (*callback_func)(void *),
+ void *callback_data);
/* Destroy hardware interaction structure */
void pvr2_hdw_destroy(struct pvr2_hdw *);
diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
index e878c64..249d748 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
@@ -884,7 +884,6 @@
{
struct pvr2_v4l2_fh *fhp = file->private_data;
struct pvr2_v4l2 *vp = fhp->vhead;
- struct pvr2_context *mp = fhp->vhead->channel.mc_head;
struct pvr2_hdw *hdw = fhp->channel.mc_head->hdw;
pvr2_trace(PVR2_TRACE_OPEN_CLOSE,"pvr2_v4l2_release");
@@ -901,42 +900,40 @@
v4l2_prio_close(&vp->prio, &fhp->prio);
file->private_data = NULL;
- pvr2_context_enter(mp); do {
- /* Restore the previous input selection, if it makes sense
- to do so. */
- if (fhp->dev_info->v4l_type == VFL_TYPE_RADIO) {
- struct pvr2_ctrl *cp;
- int pval;
- cp = pvr2_hdw_get_ctrl_by_id(hdw,PVR2_CID_INPUT);
- pvr2_ctrl_get_value(cp,&pval);
- /* Only restore if we're still selecting the radio */
- if (pval == PVR2_CVAL_INPUT_RADIO) {
- pvr2_ctrl_set_value(cp,fhp->prev_input_val);
- pvr2_hdw_commit_ctl(hdw);
- }
+ /* Restore the previous input selection, if it makes sense
+ to do so. */
+ if (fhp->dev_info->v4l_type == VFL_TYPE_RADIO) {
+ struct pvr2_ctrl *cp;
+ int pval;
+ cp = pvr2_hdw_get_ctrl_by_id(hdw,PVR2_CID_INPUT);
+ pvr2_ctrl_get_value(cp,&pval);
+ /* Only restore if we're still selecting the radio */
+ if (pval == PVR2_CVAL_INPUT_RADIO) {
+ pvr2_ctrl_set_value(cp,fhp->prev_input_val);
+ pvr2_hdw_commit_ctl(hdw);
}
+ }
- if (fhp->vnext) {
- fhp->vnext->vprev = fhp->vprev;
- } else {
- vp->vlast = fhp->vprev;
- }
- if (fhp->vprev) {
- fhp->vprev->vnext = fhp->vnext;
- } else {
- vp->vfirst = fhp->vnext;
- }
- fhp->vnext = NULL;
- fhp->vprev = NULL;
- fhp->vhead = NULL;
- pvr2_channel_done(&fhp->channel);
- pvr2_trace(PVR2_TRACE_STRUCT,
- "Destroying pvr_v4l2_fh id=%p",fhp);
- kfree(fhp);
- if (vp->channel.mc_head->disconnect_flag && !vp->vfirst) {
- pvr2_v4l2_destroy_no_lock(vp);
- }
- } while (0); pvr2_context_exit(mp);
+ if (fhp->vnext) {
+ fhp->vnext->vprev = fhp->vprev;
+ } else {
+ vp->vlast = fhp->vprev;
+ }
+ if (fhp->vprev) {
+ fhp->vprev->vnext = fhp->vnext;
+ } else {
+ vp->vfirst = fhp->vnext;
+ }
+ fhp->vnext = NULL;
+ fhp->vprev = NULL;
+ fhp->vhead = NULL;
+ pvr2_channel_done(&fhp->channel);
+ pvr2_trace(PVR2_TRACE_STRUCT,
+ "Destroying pvr_v4l2_fh id=%p",fhp);
+ kfree(fhp);
+ if (vp->channel.mc_head->disconnect_flag && !vp->vfirst) {
+ pvr2_v4l2_destroy_no_lock(vp);
+ }
return 0;
}
@@ -969,32 +966,30 @@
init_waitqueue_head(&fhp->wait_data);
fhp->dev_info = dip;
- pvr2_context_enter(vp->channel.mc_head); do {
- pvr2_trace(PVR2_TRACE_STRUCT,"Creating pvr_v4l2_fh id=%p",fhp);
- pvr2_channel_init(&fhp->channel,vp->channel.mc_head);
+ pvr2_trace(PVR2_TRACE_STRUCT,"Creating pvr_v4l2_fh id=%p",fhp);
+ pvr2_channel_init(&fhp->channel,vp->channel.mc_head);
- fhp->vnext = NULL;
- fhp->vprev = vp->vlast;
- if (vp->vlast) {
- vp->vlast->vnext = fhp;
- } else {
- vp->vfirst = fhp;
- }
- vp->vlast = fhp;
- fhp->vhead = vp;
+ fhp->vnext = NULL;
+ fhp->vprev = vp->vlast;
+ if (vp->vlast) {
+ vp->vlast->vnext = fhp;
+ } else {
+ vp->vfirst = fhp;
+ }
+ vp->vlast = fhp;
+ fhp->vhead = vp;
- /* Opening the /dev/radioX device implies a mode switch.
- So execute that here. Note that you can get the
- IDENTICAL effect merely by opening the normal video
- device and setting the input appropriately. */
- if (dip->v4l_type == VFL_TYPE_RADIO) {
- struct pvr2_ctrl *cp;
- cp = pvr2_hdw_get_ctrl_by_id(hdw,PVR2_CID_INPUT);
- pvr2_ctrl_get_value(cp,&fhp->prev_input_val);
- pvr2_ctrl_set_value(cp,PVR2_CVAL_INPUT_RADIO);
- pvr2_hdw_commit_ctl(hdw);
- }
- } while (0); pvr2_context_exit(vp->channel.mc_head);
+ /* Opening the /dev/radioX device implies a mode switch.
+ So execute that here. Note that you can get the
+ IDENTICAL effect merely by opening the normal video
+ device and setting the input appropriately. */
+ if (dip->v4l_type == VFL_TYPE_RADIO) {
+ struct pvr2_ctrl *cp;
+ cp = pvr2_hdw_get_ctrl_by_id(hdw,PVR2_CID_INPUT);
+ pvr2_ctrl_get_value(cp,&fhp->prev_input_val);
+ pvr2_ctrl_set_value(cp,PVR2_CVAL_INPUT_RADIO);
+ pvr2_hdw_commit_ctl(hdw);
+ }
fhp->file = file;
file->private_data = fhp;