Hello all,

The following patch addresses a V4L2 specification violation where the
G_TUNER call would return complete garbage in the event that the tuner
is asleep.  Per Hans' suggestion, I have split out the tuner
operational mode from whether it is in standby, and fixed the G_TUNER
call to return as much as possible when the tuner is in standby.

Without this change, products that have tuners which support standby
mode cannot be tuned from within VLC.

I recognize that changes to tuner-core tend to be pretty hairy, so I
welcome suggestions/feedback on this patch.

Regards,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
tuner-core: fix broken G_TUNER call when tuner is in standby

From: Devin Heitmueller <dheitmuel...@kernellabs.com>

The logic for determining the supported device modes was combined with the
logic which dictates whether the tuner was asleep.  This resulted in calls
such as G_TUNER returning complete garbage in the event that the tuner was
in standby mode (a violation of the V4L2 specification, and causing VLC to
be broken for such tuners).

This patch reworks the logic so the current tuner mode is maintained separately
from whether it is in standby (per Hans Verkuil's suggestion).  It also
restructures the G_TUNER call such that all the staticly defined information
related to the tuner is returned regardless of whether it is in standby (e.g.
the supported frequency range, etc).

Priority: normal

Signed-off-by: Devin Heitmueller <dheitmuel...@kernellabs.com> 
Cc: Hans Verkuil <hverk...@xs4all.nl>

--- media_build/linux/drivers/media/video/tuner-core.c	2010-10-24 19:34:59.000000000 -0400
+++ media_build_950qfixes//linux/drivers/media/video/tuner-core.c	2011-01-23 17:18:22.381107568 -0500
@@ -90,6 +90,7 @@
 
 	unsigned int        mode;
 	unsigned int        mode_mask; /* Combination of allowable modes */
+	unsigned int        in_standby:1;
 
 	unsigned int        type; /* chip type id */
 	unsigned int        config;
@@ -700,6 +701,7 @@
 static inline int set_mode(struct i2c_client *client, struct tuner *t, int mode, char *cmd)
 {
 	struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
+	unsigned int orig_mode = t->mode;
 
 	if (mode == t->mode)
 		return 0;
@@ -709,7 +711,8 @@
 	if (check_mode(t, cmd) == -EINVAL) {
 		tuner_dbg("Tuner doesn't support this mode. "
 			  "Putting tuner to sleep\n");
-		t->mode = T_STANDBY;
+		t->mode = orig_mode;
+		t->in_standby = 1;
 		if (analog_ops->standby)
 			analog_ops->standby(&t->fe);
 		return -EINVAL;
@@ -769,7 +772,7 @@
 
 	if (check_mode(t, "s_power") == -EINVAL)
 		return 0;
-	t->mode = T_STANDBY;
+	t->in_standby = 1;
 	if (analog_ops->standby)
 		analog_ops->standby(&t->fe);
 	return 0;
@@ -854,47 +857,54 @@
 	struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
 	struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
 
-	if (check_mode(t, "g_tuner") == -EINVAL)
-		return 0;
 	switch_v4l2();
 
+	/* First populate everything that doesn't require talking to the 
+	   actual hardware */
 	vt->type = t->mode;
-	if (analog_ops->get_afc)
-		vt->afc = analog_ops->get_afc(&t->fe);
 	if (t->mode == V4L2_TUNER_ANALOG_TV)
+	{
 		vt->capability |= V4L2_TUNER_CAP_NORM;
-	if (t->mode != V4L2_TUNER_RADIO) {
 		vt->rangelow = tv_range[0] * 16;
 		vt->rangehigh = tv_range[1] * 16;
-		return 0;
+	} else {
+		/* radio mode */
+		vt->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO;
+		vt->capability |= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO;
+		vt->audmode = t->audmode;
+		vt->rangelow = radio_range[0] * 16000;
+		vt->rangehigh = radio_range[1] * 16000;
 	}
 
-	/* radio mode */
-	vt->rxsubchans =
-		V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO;
-	if (fe_tuner_ops->get_status) {
-		u32 tuner_status;
+	/* If the hardware is in sleep mode, bail out at this point */
+	if (t->in_standby)
+		return 0;
 
-		fe_tuner_ops->get_status(&t->fe, &tuner_status);
-		vt->rxsubchans =
-			(tuner_status & TUNER_STATUS_STEREO) ?
-			V4L2_TUNER_SUB_STEREO :
-			V4L2_TUNER_SUB_MONO;
+	/* Now populate the fields that requires the hardware to be alive */
+	if (t->mode == V4L2_TUNER_ANALOG_TV) {
+		if (analog_ops->get_afc)
+			vt->afc = analog_ops->get_afc(&t->fe);
 	} else {
-		if (analog_ops->is_stereo) {
+		if (fe_tuner_ops->get_status) {
+			u32 tuner_status;
+
+			fe_tuner_ops->get_status(&t->fe, &tuner_status);
 			vt->rxsubchans =
-				analog_ops->is_stereo(&t->fe) ?
+				(tuner_status & TUNER_STATUS_STEREO) ?
 				V4L2_TUNER_SUB_STEREO :
 				V4L2_TUNER_SUB_MONO;
+		} else {
+			if (analog_ops->is_stereo) {
+				vt->rxsubchans =
+					analog_ops->is_stereo(&t->fe) ?
+					V4L2_TUNER_SUB_STEREO :
+					V4L2_TUNER_SUB_MONO;
+			}
 		}
+		if (analog_ops->has_signal)
+			vt->signal = analog_ops->has_signal(&t->fe);
 	}
-	if (analog_ops->has_signal)
-		vt->signal = analog_ops->has_signal(&t->fe);
-	vt->capability |=
-		V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO;
-	vt->audmode = t->audmode;
-	vt->rangelow = radio_range[0] * 16000;
-	vt->rangehigh = radio_range[1] * 16000;
+
 	return 0;
 }
 
@@ -911,6 +921,11 @@
 	/* do nothing unless we're a radio tuner */
 	if (t->mode != V4L2_TUNER_RADIO)
 		return 0;
+
+	/* Should this really fail silently if the device is asleep? */
+	if (t->in_standby == 1)
+		return 0;
+
 	t->audmode = vt->audmode;
 	set_radio_freq(client, t->radio_freq);
 	return 0;
@@ -1004,14 +1019,11 @@
 	*tv = NULL;
 
 	list_for_each_entry(pos, &tuner_list, list) {
-		int mode_mask;
-
 		if (pos->i2c->adapter != adap ||
 		    strcmp(pos->i2c->driver->driver.name, "tuner"))
 			continue;
 
-		mode_mask = pos->mode_mask & ~T_STANDBY;
-		if (*radio == NULL && mode_mask == T_RADIO)
+		if (*radio == NULL && pos->mode_mask == T_RADIO)
 			*radio = pos;
 		/* Note: currently TDA9887 is the only demod-only
 		   device. If other devices appear then we need to
@@ -1063,7 +1075,8 @@
 					       t->i2c->addr) >= 0) {
 				t->type = TUNER_TEA5761;
 				t->mode_mask = T_RADIO;
-				t->mode = T_STANDBY;
+				t->mode = T_RADIO;
+				t->in_standby = 1;
 				/* Sets freq to FM range */
 				t->radio_freq = 87.5 * 16000;
 				tuner_lookup(t->i2c->adapter, &radio, &tv);
@@ -1088,7 +1101,8 @@
 				t->type = TUNER_TDA9887;
 				t->mode_mask = T_RADIO | T_ANALOG_TV |
 					       T_DIGITAL_TV;
-				t->mode = T_STANDBY;
+				t->mode = T_ANALOG_TV;
+				t->in_standby = 1;
 				goto register_client;
 			}
 			break;
@@ -1098,7 +1112,8 @@
 					>= 0) {
 				t->type = TUNER_TEA5767;
 				t->mode_mask = T_RADIO;
-				t->mode = T_STANDBY;
+				t->mode = T_RADIO;
+				t->in_standby = 1;
 				/* Sets freq to FM range */
 				t->radio_freq = 87.5 * 16000;
 				tuner_lookup(t->i2c->adapter, &radio, &tv);

Reply via email to