question about bt8xx/bttv-audio-hook.c, tvaudio.c

question about bt8xx/bttv-audio-hook.c, tvaudio.c

[Julia Lawall]

The files drivers/media/video/bt8xx/bttv-audio-hook.c and 
drivers/media/video/tvaudio.c contain a number of occurrences of eg:

mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;

and

if (mode & V4L2_TUNER_MODE_MONO)

(both from tvaudio.c)

V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2 is suspicious because 
V4L2_TUNER_MODE_LANG1 is 3 and V4L2_TUNER_MODE_LANG2 is 2, so the result 
is just the same as V4L2_TUNER_MODE_LANG1.  Maybe 
V4L2_TUNER_MODE_LANG1_LANG2 was intended?

mode & V4L2_TUNER_MODE_MONO is suspicious because V4L2_TUNER_MODE_MONO is 
0.  Maybe & should be ==?

If & is to be changed to == everywhere, then some new code may need to be 
constructed to account for V4L2_TUNER_MODE_LANG1_LANG2.  For example, the 
function tda8425_setmode has ifs for the other values, but not for this 
one.  On the other hand, the function ta8874z_setmode already uses == (or 
rather switch), and does not take V4L2_TUNER_MODE_LANG1_LANG2 into 
account, so perhaps it is not appropriate in this context?

julia

Re: question about bt8xx/bttv-audio-hook.c, tvaudio.c

[Hans Verkuil]

On Wed June 6 2012 09:06:23 Julia Lawall wrote:
> The files drivers/media/video/bt8xx/bttv-audio-hook.c and 
> drivers/media/video/tvaudio.c contain a number of occurrences of eg:
> 
> mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
> 
> and
> 
> if (mode & V4L2_TUNER_MODE_MONO)
> 
> (both from tvaudio.c)
> 
> V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2 is suspicious because 
> V4L2_TUNER_MODE_LANG1 is 3 and V4L2_TUNER_MODE_LANG2 is 2, so the result 
> is just the same as V4L2_TUNER_MODE_LANG1.  Maybe 
> V4L2_TUNER_MODE_LANG1_LANG2 was intended?
> 
> mode & V4L2_TUNER_MODE_MONO is suspicious because V4L2_TUNER_MODE_MONO is 
> 0.  Maybe & should be ==?
> 
> If & is to be changed to == everywhere, then some new code may need to be 
> constructed to account for V4L2_TUNER_MODE_LANG1_LANG2.  For example, the 
> function tda8425_setmode has ifs for the other values, but not for this 
> one.  On the other hand, the function ta8874z_setmode already uses == (or 
> rather switch), and does not take V4L2_TUNER_MODE_LANG1_LANG2 into 
> account, so perhaps it is not appropriate in this context?

I would have to analyse this more carefully, but the core issue here is that
these drivers mixup the tuner audio reception bitmask flags (V4L2_TUNER_SUB_*)
and the tuner audio modes (V4L2_TUNER_MODE_*, not a bitmask). This happened
regularly in older drivers, and apparently these two are still not fixed.

More info is here:

http://hverkuil.home.xs4all.nl/spec/media.html#vidioc-g-tuner

I can't just replace one define with another, I would need to look carefully
at the code to see what was intended.

If you find more places where this happens, then please let us know. Otherwise
this is something for us to fix.

Regards,

	Hans

Re: question about bt8xx/bttv-audio-hook.c, tvaudio.c

[Daniel Glöckner]

On Sat, Jun 09, 2012 at 10:05:16AM +0200, Hans Verkuil wrote:
> On Wed June 6 2012 09:06:23 Julia Lawall wrote:
> > The files drivers/media/video/bt8xx/bttv-audio-hook.c and 
> > drivers/media/video/tvaudio.c contain a number of occurrences of eg:
> > 
> > mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
> > 
> > and
> > 
> > if (mode & V4L2_TUNER_MODE_MONO)
> > 
> > (both from tvaudio.c)
> 
> I would have to analyse this more carefully, but the core issue here is that
> these drivers mixup the tuner audio reception bitmask flags (V4L2_TUNER_SUB_*)
> and the tuner audio modes (V4L2_TUNER_MODE_*, not a bitmask). This happened
> regularly in older drivers, and apparently these two are still not fixed.
> 
> More info is here:
> 
> http://hverkuil.home.xs4all.nl/spec/media.html#vidioc-g-tuner
> 
> I can't just replace one define with another, I would need to look carefully
> at the code to see what was intended.

I have an old patch on one of my other machines that should fix this
in tvaudio.c. I'll try to clean it up.

  Daniel

Some tvaudio fixes

[Daniel Glöckner]

This patchset is made up of changes I did to the tvaudio driver
back in 2009. IIRC I started these to get automatic mono/stereo
swiching working again in mplayer. These changes have been tested
with a TDA9873H only and most of the time there was stereo. The
last patch is just a few hours old and has received no testing at
all.

  Daniel
      
 [PATCH 1/9] tvaudio: fix TDA9873 constants
 [PATCH 2/9] tvaudio: fix tda8425_setmode
 [PATCH 3/9] tvaudio: use V4L2_TUNER_MODE_SAP for TDA985x SAP
 [PATCH 4/9] tvaudio: remove watch_stereo
 [PATCH 5/9] tvaudio: don't use thread for TA8874Z
 [PATCH 6/9] tvaudio: use V4L2_TUNER_SUB_* for bitfields
 [PATCH 7/9] tvaudio: obey V4L2 tuner audio matrix
 [PATCH 8/9] tvaudio: support V4L2_TUNER_MODE_LANG1_LANG2
 [PATCH 9/9] tvaudio: don't report mono when stereo is received

 drivers/media/video/tvaudio.c |  189 +++++++++++++++++++++++------------------
 1 files changed, 107 insertions(+), 82 deletions(-)

Re: Some tvaudio fixes

[Hans Verkuil]

On Sun June 10 2012 03:43:49 Daniel Glöckner wrote:
> This patchset is made up of changes I did to the tvaudio driver
> back in 2009. IIRC I started these to get automatic mono/stereo
> swiching working again in mplayer. These changes have been tested
> with a TDA9873H only and most of the time there was stereo. The
> last patch is just a few hours old and has received no testing at
> all.
> 
>   Daniel
>       
>  [PATCH 1/9] tvaudio: fix TDA9873 constants
>  [PATCH 2/9] tvaudio: fix tda8425_setmode
>  [PATCH 3/9] tvaudio: use V4L2_TUNER_MODE_SAP for TDA985x SAP
>  [PATCH 4/9] tvaudio: remove watch_stereo
>  [PATCH 5/9] tvaudio: don't use thread for TA8874Z
>  [PATCH 6/9] tvaudio: use V4L2_TUNER_SUB_* for bitfields
>  [PATCH 7/9] tvaudio: obey V4L2 tuner audio matrix
>  [PATCH 8/9] tvaudio: support V4L2_TUNER_MODE_LANG1_LANG2
>  [PATCH 9/9] tvaudio: don't report mono when stereo is received
> 
>  drivers/media/video/tvaudio.c |  189 +++++++++++++++++++++++------------------
>  1 files changed, 107 insertions(+), 82 deletions(-)
> 

Looks good, but I'd like to see one final change: rename setmode to setaudmode
and getmode to getrxsubchans. That would make the code so much more understandable :-)

It's OK to do that as a final 10th patch.

Good work!

Regards,

	Hans

[PATCH] tvaudio: rename getmode and setmode

[Daniel Glöckner]

This is basically s/getmode/getrxsubchans/ and s/setmode/setaudmode/
with some whitespace adjustment in affected lines to please the eye.
The rename is done to point out their relation to the rxsubchans and
audmode fields of struct v4l2_tuner.

I also corrected a commented out call to v4l_dbg in one of the lines.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |  108 +++++++++++++++++++++++------------------
 1 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 1e61cbf..321b315 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -59,8 +59,8 @@ struct CHIPSTATE;
 typedef int  (*getvalue)(int);
 typedef int  (*checkit)(struct CHIPSTATE*);
 typedef int  (*initialize)(struct CHIPSTATE*);
-typedef int  (*getmode)(struct CHIPSTATE*);
-typedef void (*setmode)(struct CHIPSTATE*, int mode);
+typedef int  (*getrxsubchans)(struct CHIPSTATE *);
+typedef void (*setaudmode)(struct CHIPSTATE*, int mode);
 
 /* i2c command */
 typedef struct AUDIOCMD {
@@ -96,8 +96,8 @@ struct CHIPDESC {
 	getvalue volfunc,treblefunc,bassfunc;
 
 	/* get/set mode */
-	getmode  getmode;
-	setmode  setmode;
+	getrxsubchans	getrxsubchans;
+	setaudmode	setaudmode;
 
 	/* input switch register + values for v4l inputs */
 	int  inputreg;
@@ -306,7 +306,7 @@ static int chip_thread(void *data)
 			continue;
 
 		/* have a look what's going on */
-		mode = desc->getmode(chip);
+		mode = desc->getrxsubchans(chip);
 		if (mode == chip->prevmode)
 			continue;
 
@@ -340,7 +340,7 @@ static int chip_thread(void *data)
 			else if (mode & V4L2_TUNER_SUB_STEREO)
 				selected = V4L2_TUNER_MODE_STEREO;
 		}
-		desc->setmode(chip, selected);
+		desc->setaudmode(chip, selected);
 
 		/* schedule next check */
 		mod_timer(&chip->wt, jiffies+msecs_to_jiffies(2000));
@@ -373,7 +373,7 @@ static int chip_thread(void *data)
 #define TDA9840_TEST_INT1SN 0x1 /* Integration time 0.5s when set */
 #define TDA9840_TEST_INTFU 0x02 /* Disables integrator function */
 
-static int tda9840_getmode(struct CHIPSTATE *chip)
+static int tda9840_getrxsubchans(struct CHIPSTATE *chip)
 {
 	struct v4l2_subdev *sd = &chip->sd;
 	int val, mode;
@@ -385,12 +385,13 @@ static int tda9840_getmode(struct CHIPSTATE *chip)
 	if (val & TDA9840_ST_STEREO)
 		mode = V4L2_TUNER_SUB_STEREO;
 
-	v4l2_dbg(1, debug, sd, "tda9840_getmode(): raw chip read: %d, return: %d\n",
+	v4l2_dbg(1, debug, sd,
+		"tda9840_getrxsubchans(): raw chip read: %d, return: %d\n",
 		val, mode);
 	return mode;
 }
 
-static void tda9840_setmode(struct CHIPSTATE *chip, int mode)
+static void tda9840_setaudmode(struct CHIPSTATE *chip, int mode)
 {
 	int update = 1;
 	int t = chip->shadow.bytes[TDA9840_SW + 1] & ~0x7e;
@@ -532,7 +533,7 @@ static int tda9855_volume(int val) { return val/0x2e8+0x27; }
 static int tda9855_bass(int val)   { return val/0xccc+0x06; }
 static int tda9855_treble(int val) { return (val/0x1c71+0x3)<<1; }
 
-static int  tda985x_getmode(struct CHIPSTATE *chip)
+static int  tda985x_getrxsubchans(struct CHIPSTATE *chip)
 {
 	int mode, val;
 
@@ -547,7 +548,7 @@ static int  tda985x_getmode(struct CHIPSTATE *chip)
 	return mode;
 }
 
-static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
+static void tda985x_setaudmode(struct CHIPSTATE *chip, int mode)
 {
 	int update = 1;
 	int c6 = chip->shadow.bytes[TDA985x_C6+1] & 0x3f;
@@ -692,7 +693,7 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
 #define TDA9873_STEREO      2 /* Stereo sound is identified     */
 #define TDA9873_DUAL        4 /* Dual sound is identified       */
 
-static int tda9873_getmode(struct CHIPSTATE *chip)
+static int tda9873_getrxsubchans(struct CHIPSTATE *chip)
 {
 	struct v4l2_subdev *sd = &chip->sd;
 	int val,mode;
@@ -703,24 +704,29 @@ static int tda9873_getmode(struct CHIPSTATE *chip)
 		mode = V4L2_TUNER_SUB_STEREO;
 	if (val & TDA9873_DUAL)
 		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
-	v4l2_dbg(1, debug, sd, "tda9873_getmode(): raw chip read: %d, return: %d\n",
+	v4l2_dbg(1, debug, sd,
+		"tda9873_getrxsubchans(): raw chip read: %d, return: %d\n",
 		val, mode);
 	return mode;
 }
 
-static void tda9873_setmode(struct CHIPSTATE *chip, int mode)
+static void tda9873_setaudmode(struct CHIPSTATE *chip, int mode)
 {
 	struct v4l2_subdev *sd = &chip->sd;
 	int sw_data  = chip->shadow.bytes[TDA9873_SW+1] & ~ TDA9873_TR_MASK;
 	/*	int adj_data = chip->shadow.bytes[TDA9873_AD+1] ; */
 
 	if ((sw_data & TDA9873_INP_MASK) != TDA9873_INTERNAL) {
-		v4l2_dbg(1, debug, sd, "tda9873_setmode(): external input\n");
+		v4l2_dbg(1, debug, sd,
+			 "tda9873_setaudmode(): external input\n");
 		return;
 	}
 
-	v4l2_dbg(1, debug, sd, "tda9873_setmode(): chip->shadow.bytes[%d] = %d\n", TDA9873_SW+1, chip->shadow.bytes[TDA9873_SW+1]);
-	v4l2_dbg(1, debug, sd, "tda9873_setmode(): sw_data  = %d\n", sw_data);
+	v4l2_dbg(1, debug, sd,
+		 "tda9873_setaudmode(): chip->shadow.bytes[%d] = %d\n",
+		 TDA9873_SW+1, chip->shadow.bytes[TDA9873_SW+1]);
+	v4l2_dbg(1, debug, sd, "tda9873_setaudmode(): sw_data  = %d\n",
+		 sw_data);
 
 	switch (mode) {
 	case V4L2_TUNER_MODE_MONO:
@@ -743,7 +749,8 @@ static void tda9873_setmode(struct CHIPSTATE *chip, int mode)
 	}
 
 	chip_write(chip, TDA9873_SW, sw_data);
-	v4l2_dbg(1, debug, sd, "tda9873_setmode(): req. mode %d; chip_write: %d\n",
+	v4l2_dbg(1, debug, sd,
+		"tda9873_setaudmode(): req. mode %d; chip_write: %d\n",
 		mode, sw_data);
 }
 
@@ -889,7 +896,7 @@ static int tda9874a_setup(struct CHIPSTATE *chip)
 	return 1;
 }
 
-static int tda9874a_getmode(struct CHIPSTATE *chip)
+static int tda9874a_getrxsubchans(struct CHIPSTATE *chip)
 {
 	struct v4l2_subdev *sd = &chip->sd;
 	int dsr,nsr,mode;
@@ -928,12 +935,13 @@ static int tda9874a_getmode(struct CHIPSTATE *chip)
 			mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	}
 
-	v4l2_dbg(1, debug, sd, "tda9874a_getmode(): DSR=0x%X, NSR=0x%X, NECR=0x%X, return: %d.\n",
+	v4l2_dbg(1, debug, sd,
+		 "tda9874a_getrxsubchans(): DSR=0x%X, NSR=0x%X, NECR=0x%X, return: %d.\n",
 		 dsr, nsr, necr, mode);
 	return mode;
 }
 
-static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
+static void tda9874a_setaudmode(struct CHIPSTATE *chip, int mode)
 {
 	struct v4l2_subdev *sd = &chip->sd;
 
@@ -979,7 +987,8 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
 		chip_write(chip, TDA9874A_AOSR, aosr);
 		chip_write(chip, TDA9874A_MDACOSR, mdacosr);
 
-		v4l2_dbg(1, debug, sd, "tda9874a_setmode(): req. mode %d; AOSR=0x%X, MDACOSR=0x%X.\n",
+		v4l2_dbg(1, debug, sd,
+			"tda9874a_setaudmode(): req. mode %d; AOSR=0x%X, MDACOSR=0x%X.\n",
 			mode, aosr, mdacosr);
 
 	} else { /* dic == 0x07 */
@@ -1017,7 +1026,8 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
 		chip_write(chip, TDA9874A_FMMR, fmmr);
 		chip_write(chip, TDA9874A_AOSR, aosr);
 
-		v4l2_dbg(1, debug, sd, "tda9874a_setmode(): req. mode %d; FMMR=0x%X, AOSR=0x%X.\n",
+		v4l2_dbg(1, debug, sd,
+			"tda9874a_setaudmode(): req. mode %d; FMMR=0x%X, AOSR=0x%X.\n",
 			mode, fmmr, aosr);
 	}
 }
@@ -1262,7 +1272,7 @@ static int tea6320_initialize(struct CHIPSTATE * chip)
 static int tda8425_shift10(int val) { return (val >> 10) | 0xc0; }
 static int tda8425_shift12(int val) { return (val >> 12) | 0xf0; }
 
-static void tda8425_setmode(struct CHIPSTATE *chip, int mode)
+static void tda8425_setaudmode(struct CHIPSTATE *chip, int mode)
 {
 	int s1 = chip->shadow.bytes[TDA8425_S1+1] & 0xe1;
 
@@ -1341,7 +1351,7 @@ static void tda8425_setmode(struct CHIPSTATE *chip, int mode)
  * stereo  L  L
  * BIL     H  L
  */
-static int ta8874z_getmode(struct CHIPSTATE *chip)
+static int ta8874z_getrxsubchans(struct CHIPSTATE *chip)
 {
 	int val, mode;
 
@@ -1352,7 +1362,9 @@ static int ta8874z_getmode(struct CHIPSTATE *chip)
 	}else if (!(val & TA8874Z_B0)){
 		mode = V4L2_TUNER_SUB_STEREO;
 	}
-	/* v4l_dbg(1, debug, chip->c, "ta8874z_getmode(): raw chip read: 0x%02x, return: 0x%02x\n", val, mode); */
+	/* v4l2_dbg(1, debug, &chip->sd,
+		 "ta8874z_getrxsubchans(): raw chip read: 0x%02x, return: 0x%02x\n",
+		 val, mode); */
 	return mode;
 }
 
@@ -1362,13 +1374,13 @@ static audiocmd ta8874z_main = {2, { 0, TA8874Z_SEPARATION_DEFAULT}};
 static audiocmd ta8874z_sub = {2, { TA8874Z_MODE_SUB, TA8874Z_SEPARATION_DEFAULT}};
 static audiocmd ta8874z_both = {2, { TA8874Z_MODE_MAIN | TA8874Z_MODE_SUB, TA8874Z_SEPARATION_DEFAULT}};
 
-static void ta8874z_setmode(struct CHIPSTATE *chip, int mode)
+static void ta8874z_setaudmode(struct CHIPSTATE *chip, int mode)
 {
 	struct v4l2_subdev *sd = &chip->sd;
 	int update = 1;
 	audiocmd *t = NULL;
 
-	v4l2_dbg(1, debug, sd, "ta8874z_setmode(): mode: 0x%02x\n", mode);
+	v4l2_dbg(1, debug, sd, "ta8874z_setaudmode(): mode: 0x%02x\n", mode);
 
 	switch(mode){
 	case V4L2_TUNER_MODE_MONO:
@@ -1442,8 +1454,8 @@ static struct CHIPDESC chiplist[] = {
 
 		/* callbacks */
 		.checkit    = tda9840_checkit,
-		.getmode    = tda9840_getmode,
-		.setmode    = tda9840_setmode,
+		.getrxsubchans = tda9840_getrxsubchans,
+		.setaudmode = tda9840_setaudmode,
 
 		.init       = { 2, { TDA9840_TEST, TDA9840_TEST_INT1SN
 				/* ,TDA9840_SW, TDA9840_MONO */} }
@@ -1458,8 +1470,8 @@ static struct CHIPDESC chiplist[] = {
 
 		/* callbacks */
 		.checkit    = tda9873_checkit,
-		.getmode    = tda9873_getmode,
-		.setmode    = tda9873_setmode,
+		.getrxsubchans = tda9873_getrxsubchans,
+		.setaudmode = tda9873_setaudmode,
 
 		.init       = { 4, { TDA9873_SW, 0xa4, 0x06, 0x03 } },
 		.inputreg   = TDA9873_SW,
@@ -1478,8 +1490,8 @@ static struct CHIPDESC chiplist[] = {
 		/* callbacks */
 		.initialize = tda9874a_initialize,
 		.checkit    = tda9874a_checkit,
-		.getmode    = tda9874a_getmode,
-		.setmode    = tda9874a_setmode,
+		.getrxsubchans = tda9874a_getrxsubchans,
+		.setaudmode = tda9874a_setaudmode,
 	},
 	{
 		.name       = "tda9875",
@@ -1508,8 +1520,8 @@ static struct CHIPDESC chiplist[] = {
 		.addr_hi    = I2C_ADDR_TDA985x_H >> 1,
 		.registers  = 11,
 
-		.getmode    = tda985x_getmode,
-		.setmode    = tda985x_setmode,
+		.getrxsubchans = tda985x_getrxsubchans,
+		.setaudmode = tda985x_setaudmode,
 
 		.init       = { 8, { TDA9850_C4, 0x08, 0x08, TDA985x_STEREO, 0x07, 0x10, 0x10, 0x03 } }
 	},
@@ -1530,8 +1542,8 @@ static struct CHIPDESC chiplist[] = {
 		.volfunc    = tda9855_volume,
 		.bassfunc   = tda9855_bass,
 		.treblefunc = tda9855_treble,
-		.getmode    = tda985x_getmode,
-		.setmode    = tda985x_setmode,
+		.getrxsubchans = tda985x_getrxsubchans,
+		.setaudmode = tda985x_setaudmode,
 
 		.init       = { 12, { 0, 0x6f, 0x6f, 0x0e, 0x07<<1, 0x8<<2,
 				    TDA9855_MUTE | TDA9855_AVL | TDA9855_LOUD | TDA9855_INT,
@@ -1612,7 +1624,7 @@ static struct CHIPDESC chiplist[] = {
 		.volfunc    = tda8425_shift10,
 		.bassfunc   = tda8425_shift12,
 		.treblefunc = tda8425_shift12,
-		.setmode    = tda8425_setmode,
+		.setaudmode = tda8425_setaudmode,
 
 		.inputreg   = TDA8425_S1,
 		.inputmap   = { TDA8425_S1_CH1, TDA8425_S1_CH1, TDA8425_S1_CH1 },
@@ -1643,8 +1655,8 @@ static struct CHIPDESC chiplist[] = {
 		.registers  = 2,
 
 		/* callbacks */
-		.getmode    = ta8874z_getmode,
-		.setmode    = ta8874z_setmode,
+		.getrxsubchans = ta8874z_getrxsubchans,
+		.setaudmode = ta8874z_setaudmode,
 
 		.init       = {2, { TA8874Z_MONO_SET, TA8874Z_SEPARATION_DEFAULT}},
 	},
@@ -1840,7 +1852,7 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 	struct CHIPSTATE *chip = to_state(sd);
 	struct CHIPDESC *desc = chip->desc;
 
-	if (!desc->setmode)
+	if (!desc->setaudmode)
 		return 0;
 	if (chip->radio)
 		return 0;
@@ -1860,7 +1872,7 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 	if (chip->thread)
 		wake_up_process(chip->thread);
 	else
-		desc->setmode(chip, vt->audmode);
+		desc->setaudmode(chip, vt->audmode);
 
 	return 0;
 }
@@ -1870,13 +1882,13 @@ static int tvaudio_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 	struct CHIPSTATE *chip = to_state(sd);
 	struct CHIPDESC *desc = chip->desc;
 
-	if (!desc->getmode)
+	if (!desc->getrxsubchans)
 		return 0;
 	if (chip->radio)
 		return 0;
 
 	vt->audmode = chip->audmode;
-	vt->rxsubchans = desc->getmode(chip);
+	vt->rxsubchans = desc->getrxsubchans(chip);
 	vt->capability = V4L2_TUNER_CAP_STEREO |
 		V4L2_TUNER_CAP_LANG1 | V4L2_TUNER_CAP_LANG2;
 
@@ -1896,7 +1908,7 @@ static int tvaudio_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *fr
 	struct CHIPSTATE *chip = to_state(sd);
 	struct CHIPDESC *desc = chip->desc;
 
-	/* For chips that provide getmode and setmode, and doesn't
+	/* For chips that provide getrxsubchans and setaudmode, and doesn't
 	   automatically follows the stereo carrier, a kthread is
 	   created to set the audio standard. In this case, when then
 	   the video channel is changed, tvaudio starts on MONO mode.
@@ -1905,7 +1917,7 @@ static int tvaudio_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *fr
 	   audio carrier.
 	 */
 	if (chip->thread) {
-		desc->setmode(chip, V4L2_TUNER_MODE_MONO);
+		desc->setaudmode(chip, V4L2_TUNER_MODE_MONO);
 		chip->prevmode = -1; /* reset previous mode */
 		mod_timer(&chip->wt, jiffies+msecs_to_jiffies(2000));
 	}
@@ -2048,7 +2060,7 @@ static int tvaudio_probe(struct i2c_client *client, const struct i2c_device_id *
 	chip->thread = NULL;
 	init_timer(&chip->wt);
 	if (desc->flags & CHIP_NEED_CHECKMODE) {
-		if (!desc->getmode || !desc->setmode) {
+		if (!desc->getrxsubchans || !desc->setaudmode) {
 			/* This shouldn't be happen. Warn user, but keep working
 			   without kthread
 			 */
-- 
1.7.0.5

Re: [PATCH] tvaudio: rename getmode and setmode

[Hans Verkuil]

On Sun June 17 2012 13:53:42 Daniel Glöckner wrote:
> This is basically s/getmode/getrxsubchans/ and s/setmode/setaudmode/
> with some whitespace adjustment in affected lines to please the eye.
> The rename is done to point out their relation to the rxsubchans and
> audmode fields of struct v4l2_tuner.
> 
> I also corrected a commented out call to v4l_dbg in one of the lines.

Looks much better!

For the whole patch series:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

You could even make it a little bit better by removing all typedefs
(they are totally bogus) and lower case those ugly uppercase struct
names (CHIPSTATE, AUDIOCMD and CHIPDESC).

Regards,

	Hans

> 
> Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
> ---
>  drivers/media/video/tvaudio.c |  108 +++++++++++++++++++++++------------------
>  1 files changed, 60 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
> index 1e61cbf..321b315 100644
> --- a/drivers/media/video/tvaudio.c
> +++ b/drivers/media/video/tvaudio.c
> @@ -59,8 +59,8 @@ struct CHIPSTATE;
>  typedef int  (*getvalue)(int);
>  typedef int  (*checkit)(struct CHIPSTATE*);
>  typedef int  (*initialize)(struct CHIPSTATE*);
> -typedef int  (*getmode)(struct CHIPSTATE*);
> -typedef void (*setmode)(struct CHIPSTATE*, int mode);
> +typedef int  (*getrxsubchans)(struct CHIPSTATE *);
> +typedef void (*setaudmode)(struct CHIPSTATE*, int mode);
>  
>  /* i2c command */
>  typedef struct AUDIOCMD {
> @@ -96,8 +96,8 @@ struct CHIPDESC {
>  	getvalue volfunc,treblefunc,bassfunc;
>  
>  	/* get/set mode */
> -	getmode  getmode;
> -	setmode  setmode;
> +	getrxsubchans	getrxsubchans;
> +	setaudmode	setaudmode;
>  
>  	/* input switch register + values for v4l inputs */
>  	int  inputreg;
> @@ -306,7 +306,7 @@ static int chip_thread(void *data)
>  			continue;
>  
>  		/* have a look what's going on */
> -		mode = desc->getmode(chip);
> +		mode = desc->getrxsubchans(chip);
>  		if (mode == chip->prevmode)
>  			continue;
>  
> @@ -340,7 +340,7 @@ static int chip_thread(void *data)
>  			else if (mode & V4L2_TUNER_SUB_STEREO)
>  				selected = V4L2_TUNER_MODE_STEREO;
>  		}
> -		desc->setmode(chip, selected);
> +		desc->setaudmode(chip, selected);
>  
>  		/* schedule next check */
>  		mod_timer(&chip->wt, jiffies+msecs_to_jiffies(2000));
> @@ -373,7 +373,7 @@ static int chip_thread(void *data)
>  #define TDA9840_TEST_INT1SN 0x1 /* Integration time 0.5s when set */
>  #define TDA9840_TEST_INTFU 0x02 /* Disables integrator function */
>  
> -static int tda9840_getmode(struct CHIPSTATE *chip)
> +static int tda9840_getrxsubchans(struct CHIPSTATE *chip)
>  {
>  	struct v4l2_subdev *sd = &chip->sd;
>  	int val, mode;
> @@ -385,12 +385,13 @@ static int tda9840_getmode(struct CHIPSTATE *chip)
>  	if (val & TDA9840_ST_STEREO)
>  		mode = V4L2_TUNER_SUB_STEREO;
>  
> -	v4l2_dbg(1, debug, sd, "tda9840_getmode(): raw chip read: %d, return: %d\n",
> +	v4l2_dbg(1, debug, sd,
> +		"tda9840_getrxsubchans(): raw chip read: %d, return: %d\n",
>  		val, mode);
>  	return mode;
>  }
>  
> -static void tda9840_setmode(struct CHIPSTATE *chip, int mode)
> +static void tda9840_setaudmode(struct CHIPSTATE *chip, int mode)
>  {
>  	int update = 1;
>  	int t = chip->shadow.bytes[TDA9840_SW + 1] & ~0x7e;
> @@ -532,7 +533,7 @@ static int tda9855_volume(int val) { return val/0x2e8+0x27; }
>  static int tda9855_bass(int val)   { return val/0xccc+0x06; }
>  static int tda9855_treble(int val) { return (val/0x1c71+0x3)<<1; }
>  
> -static int  tda985x_getmode(struct CHIPSTATE *chip)
> +static int  tda985x_getrxsubchans(struct CHIPSTATE *chip)
>  {
>  	int mode, val;
>  
> @@ -547,7 +548,7 @@ static int  tda985x_getmode(struct CHIPSTATE *chip)
>  	return mode;
>  }
>  
> -static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
> +static void tda985x_setaudmode(struct CHIPSTATE *chip, int mode)
>  {
>  	int update = 1;
>  	int c6 = chip->shadow.bytes[TDA985x_C6+1] & 0x3f;
> @@ -692,7 +693,7 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
>  #define TDA9873_STEREO      2 /* Stereo sound is identified     */
>  #define TDA9873_DUAL        4 /* Dual sound is identified       */
>  
> -static int tda9873_getmode(struct CHIPSTATE *chip)
> +static int tda9873_getrxsubchans(struct CHIPSTATE *chip)
>  {
>  	struct v4l2_subdev *sd = &chip->sd;
>  	int val,mode;
> @@ -703,24 +704,29 @@ static int tda9873_getmode(struct CHIPSTATE *chip)
>  		mode = V4L2_TUNER_SUB_STEREO;
>  	if (val & TDA9873_DUAL)
>  		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
> -	v4l2_dbg(1, debug, sd, "tda9873_getmode(): raw chip read: %d, return: %d\n",
> +	v4l2_dbg(1, debug, sd,
> +		"tda9873_getrxsubchans(): raw chip read: %d, return: %d\n",
>  		val, mode);
>  	return mode;
>  }
>  
> -static void tda9873_setmode(struct CHIPSTATE *chip, int mode)
> +static void tda9873_setaudmode(struct CHIPSTATE *chip, int mode)
>  {
>  	struct v4l2_subdev *sd = &chip->sd;
>  	int sw_data  = chip->shadow.bytes[TDA9873_SW+1] & ~ TDA9873_TR_MASK;
>  	/*	int adj_data = chip->shadow.bytes[TDA9873_AD+1] ; */
>  
>  	if ((sw_data & TDA9873_INP_MASK) != TDA9873_INTERNAL) {
> -		v4l2_dbg(1, debug, sd, "tda9873_setmode(): external input\n");
> +		v4l2_dbg(1, debug, sd,
> +			 "tda9873_setaudmode(): external input\n");
>  		return;
>  	}
>  
> -	v4l2_dbg(1, debug, sd, "tda9873_setmode(): chip->shadow.bytes[%d] = %d\n", TDA9873_SW+1, chip->shadow.bytes[TDA9873_SW+1]);
> -	v4l2_dbg(1, debug, sd, "tda9873_setmode(): sw_data  = %d\n", sw_data);
> +	v4l2_dbg(1, debug, sd,
> +		 "tda9873_setaudmode(): chip->shadow.bytes[%d] = %d\n",
> +		 TDA9873_SW+1, chip->shadow.bytes[TDA9873_SW+1]);
> +	v4l2_dbg(1, debug, sd, "tda9873_setaudmode(): sw_data  = %d\n",
> +		 sw_data);
>  
>  	switch (mode) {
>  	case V4L2_TUNER_MODE_MONO:
> @@ -743,7 +749,8 @@ static void tda9873_setmode(struct CHIPSTATE *chip, int mode)
>  	}
>  
>  	chip_write(chip, TDA9873_SW, sw_data);
> -	v4l2_dbg(1, debug, sd, "tda9873_setmode(): req. mode %d; chip_write: %d\n",
> +	v4l2_dbg(1, debug, sd,
> +		"tda9873_setaudmode(): req. mode %d; chip_write: %d\n",
>  		mode, sw_data);
>  }
>  
> @@ -889,7 +896,7 @@ static int tda9874a_setup(struct CHIPSTATE *chip)
>  	return 1;
>  }
>  
> -static int tda9874a_getmode(struct CHIPSTATE *chip)
> +static int tda9874a_getrxsubchans(struct CHIPSTATE *chip)
>  {
>  	struct v4l2_subdev *sd = &chip->sd;
>  	int dsr,nsr,mode;
> @@ -928,12 +935,13 @@ static int tda9874a_getmode(struct CHIPSTATE *chip)
>  			mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
>  	}
>  
> -	v4l2_dbg(1, debug, sd, "tda9874a_getmode(): DSR=0x%X, NSR=0x%X, NECR=0x%X, return: %d.\n",
> +	v4l2_dbg(1, debug, sd,
> +		 "tda9874a_getrxsubchans(): DSR=0x%X, NSR=0x%X, NECR=0x%X, return: %d.\n",
>  		 dsr, nsr, necr, mode);
>  	return mode;
>  }
>  
> -static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
> +static void tda9874a_setaudmode(struct CHIPSTATE *chip, int mode)
>  {
>  	struct v4l2_subdev *sd = &chip->sd;
>  
> @@ -979,7 +987,8 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
>  		chip_write(chip, TDA9874A_AOSR, aosr);
>  		chip_write(chip, TDA9874A_MDACOSR, mdacosr);
>  
> -		v4l2_dbg(1, debug, sd, "tda9874a_setmode(): req. mode %d; AOSR=0x%X, MDACOSR=0x%X.\n",
> +		v4l2_dbg(1, debug, sd,
> +			"tda9874a_setaudmode(): req. mode %d; AOSR=0x%X, MDACOSR=0x%X.\n",
>  			mode, aosr, mdacosr);
>  
>  	} else { /* dic == 0x07 */
> @@ -1017,7 +1026,8 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
>  		chip_write(chip, TDA9874A_FMMR, fmmr);
>  		chip_write(chip, TDA9874A_AOSR, aosr);
>  
> -		v4l2_dbg(1, debug, sd, "tda9874a_setmode(): req. mode %d; FMMR=0x%X, AOSR=0x%X.\n",
> +		v4l2_dbg(1, debug, sd,
> +			"tda9874a_setaudmode(): req. mode %d; FMMR=0x%X, AOSR=0x%X.\n",
>  			mode, fmmr, aosr);
>  	}
>  }
> @@ -1262,7 +1272,7 @@ static int tea6320_initialize(struct CHIPSTATE * chip)
>  static int tda8425_shift10(int val) { return (val >> 10) | 0xc0; }
>  static int tda8425_shift12(int val) { return (val >> 12) | 0xf0; }
>  
> -static void tda8425_setmode(struct CHIPSTATE *chip, int mode)
> +static void tda8425_setaudmode(struct CHIPSTATE *chip, int mode)
>  {
>  	int s1 = chip->shadow.bytes[TDA8425_S1+1] & 0xe1;
>  
> @@ -1341,7 +1351,7 @@ static void tda8425_setmode(struct CHIPSTATE *chip, int mode)
>   * stereo  L  L
>   * BIL     H  L
>   */
> -static int ta8874z_getmode(struct CHIPSTATE *chip)
> +static int ta8874z_getrxsubchans(struct CHIPSTATE *chip)
>  {
>  	int val, mode;
>  
> @@ -1352,7 +1362,9 @@ static int ta8874z_getmode(struct CHIPSTATE *chip)
>  	}else if (!(val & TA8874Z_B0)){
>  		mode = V4L2_TUNER_SUB_STEREO;
>  	}
> -	/* v4l_dbg(1, debug, chip->c, "ta8874z_getmode(): raw chip read: 0x%02x, return: 0x%02x\n", val, mode); */
> +	/* v4l2_dbg(1, debug, &chip->sd,
> +		 "ta8874z_getrxsubchans(): raw chip read: 0x%02x, return: 0x%02x\n",
> +		 val, mode); */
>  	return mode;
>  }
>  
> @@ -1362,13 +1374,13 @@ static audiocmd ta8874z_main = {2, { 0, TA8874Z_SEPARATION_DEFAULT}};
>  static audiocmd ta8874z_sub = {2, { TA8874Z_MODE_SUB, TA8874Z_SEPARATION_DEFAULT}};
>  static audiocmd ta8874z_both = {2, { TA8874Z_MODE_MAIN | TA8874Z_MODE_SUB, TA8874Z_SEPARATION_DEFAULT}};
>  
> -static void ta8874z_setmode(struct CHIPSTATE *chip, int mode)
> +static void ta8874z_setaudmode(struct CHIPSTATE *chip, int mode)
>  {
>  	struct v4l2_subdev *sd = &chip->sd;
>  	int update = 1;
>  	audiocmd *t = NULL;
>  
> -	v4l2_dbg(1, debug, sd, "ta8874z_setmode(): mode: 0x%02x\n", mode);
> +	v4l2_dbg(1, debug, sd, "ta8874z_setaudmode(): mode: 0x%02x\n", mode);
>  
>  	switch(mode){
>  	case V4L2_TUNER_MODE_MONO:
> @@ -1442,8 +1454,8 @@ static struct CHIPDESC chiplist[] = {
>  
>  		/* callbacks */
>  		.checkit    = tda9840_checkit,
> -		.getmode    = tda9840_getmode,
> -		.setmode    = tda9840_setmode,
> +		.getrxsubchans = tda9840_getrxsubchans,
> +		.setaudmode = tda9840_setaudmode,
>  
>  		.init       = { 2, { TDA9840_TEST, TDA9840_TEST_INT1SN
>  				/* ,TDA9840_SW, TDA9840_MONO */} }
> @@ -1458,8 +1470,8 @@ static struct CHIPDESC chiplist[] = {
>  
>  		/* callbacks */
>  		.checkit    = tda9873_checkit,
> -		.getmode    = tda9873_getmode,
> -		.setmode    = tda9873_setmode,
> +		.getrxsubchans = tda9873_getrxsubchans,
> +		.setaudmode = tda9873_setaudmode,
>  
>  		.init       = { 4, { TDA9873_SW, 0xa4, 0x06, 0x03 } },
>  		.inputreg   = TDA9873_SW,
> @@ -1478,8 +1490,8 @@ static struct CHIPDESC chiplist[] = {
>  		/* callbacks */
>  		.initialize = tda9874a_initialize,
>  		.checkit    = tda9874a_checkit,
> -		.getmode    = tda9874a_getmode,
> -		.setmode    = tda9874a_setmode,
> +		.getrxsubchans = tda9874a_getrxsubchans,
> +		.setaudmode = tda9874a_setaudmode,
>  	},
>  	{
>  		.name       = "tda9875",
> @@ -1508,8 +1520,8 @@ static struct CHIPDESC chiplist[] = {
>  		.addr_hi    = I2C_ADDR_TDA985x_H >> 1,
>  		.registers  = 11,
>  
> -		.getmode    = tda985x_getmode,
> -		.setmode    = tda985x_setmode,
> +		.getrxsubchans = tda985x_getrxsubchans,
> +		.setaudmode = tda985x_setaudmode,
>  
>  		.init       = { 8, { TDA9850_C4, 0x08, 0x08, TDA985x_STEREO, 0x07, 0x10, 0x10, 0x03 } }
>  	},
> @@ -1530,8 +1542,8 @@ static struct CHIPDESC chiplist[] = {
>  		.volfunc    = tda9855_volume,
>  		.bassfunc   = tda9855_bass,
>  		.treblefunc = tda9855_treble,
> -		.getmode    = tda985x_getmode,
> -		.setmode    = tda985x_setmode,
> +		.getrxsubchans = tda985x_getrxsubchans,
> +		.setaudmode = tda985x_setaudmode,
>  
>  		.init       = { 12, { 0, 0x6f, 0x6f, 0x0e, 0x07<<1, 0x8<<2,
>  				    TDA9855_MUTE | TDA9855_AVL | TDA9855_LOUD | TDA9855_INT,
> @@ -1612,7 +1624,7 @@ static struct CHIPDESC chiplist[] = {
>  		.volfunc    = tda8425_shift10,
>  		.bassfunc   = tda8425_shift12,
>  		.treblefunc = tda8425_shift12,
> -		.setmode    = tda8425_setmode,
> +		.setaudmode = tda8425_setaudmode,
>  
>  		.inputreg   = TDA8425_S1,
>  		.inputmap   = { TDA8425_S1_CH1, TDA8425_S1_CH1, TDA8425_S1_CH1 },
> @@ -1643,8 +1655,8 @@ static struct CHIPDESC chiplist[] = {
>  		.registers  = 2,
>  
>  		/* callbacks */
> -		.getmode    = ta8874z_getmode,
> -		.setmode    = ta8874z_setmode,
> +		.getrxsubchans = ta8874z_getrxsubchans,
> +		.setaudmode = ta8874z_setaudmode,
>  
>  		.init       = {2, { TA8874Z_MONO_SET, TA8874Z_SEPARATION_DEFAULT}},
>  	},
> @@ -1840,7 +1852,7 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
>  	struct CHIPSTATE *chip = to_state(sd);
>  	struct CHIPDESC *desc = chip->desc;
>  
> -	if (!desc->setmode)
> +	if (!desc->setaudmode)
>  		return 0;
>  	if (chip->radio)
>  		return 0;
> @@ -1860,7 +1872,7 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
>  	if (chip->thread)
>  		wake_up_process(chip->thread);
>  	else
> -		desc->setmode(chip, vt->audmode);
> +		desc->setaudmode(chip, vt->audmode);
>  
>  	return 0;
>  }
> @@ -1870,13 +1882,13 @@ static int tvaudio_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
>  	struct CHIPSTATE *chip = to_state(sd);
>  	struct CHIPDESC *desc = chip->desc;
>  
> -	if (!desc->getmode)
> +	if (!desc->getrxsubchans)
>  		return 0;
>  	if (chip->radio)
>  		return 0;
>  
>  	vt->audmode = chip->audmode;
> -	vt->rxsubchans = desc->getmode(chip);
> +	vt->rxsubchans = desc->getrxsubchans(chip);
>  	vt->capability = V4L2_TUNER_CAP_STEREO |
>  		V4L2_TUNER_CAP_LANG1 | V4L2_TUNER_CAP_LANG2;
>  
> @@ -1896,7 +1908,7 @@ static int tvaudio_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *fr
>  	struct CHIPSTATE *chip = to_state(sd);
>  	struct CHIPDESC *desc = chip->desc;
>  
> -	/* For chips that provide getmode and setmode, and doesn't
> +	/* For chips that provide getrxsubchans and setaudmode, and doesn't
>  	   automatically follows the stereo carrier, a kthread is
>  	   created to set the audio standard. In this case, when then
>  	   the video channel is changed, tvaudio starts on MONO mode.
> @@ -1905,7 +1917,7 @@ static int tvaudio_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *fr
>  	   audio carrier.
>  	 */
>  	if (chip->thread) {
> -		desc->setmode(chip, V4L2_TUNER_MODE_MONO);
> +		desc->setaudmode(chip, V4L2_TUNER_MODE_MONO);
>  		chip->prevmode = -1; /* reset previous mode */
>  		mod_timer(&chip->wt, jiffies+msecs_to_jiffies(2000));
>  	}
> @@ -2048,7 +2060,7 @@ static int tvaudio_probe(struct i2c_client *client, const struct i2c_device_id *
>  	chip->thread = NULL;
>  	init_timer(&chip->wt);
>  	if (desc->flags & CHIP_NEED_CHECKMODE) {
> -		if (!desc->getmode || !desc->setmode) {
> +		if (!desc->getrxsubchans || !desc->setaudmode) {
>  			/* This shouldn't be happen. Warn user, but keep working
>  			   without kthread
>  			 */
> 

[PATCH 1/9] tvaudio: fix TDA9873 constants

[Daniel Glöckner]

These constants were unused so far but need | instead of &.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index c5b1a73..9b85e2a 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -583,7 +583,7 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
 #define TDA9873_TR_MASK     (7 << 2)
 #define TDA9873_TR_MONO     4
 #define TDA9873_TR_STEREO   1 << 4
-#define TDA9873_TR_REVERSE  (1 << 3) & (1 << 2)
+#define TDA9873_TR_REVERSE  ((1 << 3) | (1 << 2))
 #define TDA9873_TR_DUALA    1 << 2
 #define TDA9873_TR_DUALB    1 << 3
 
@@ -653,11 +653,11 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
 #define TDA9873_MOUT_DUALA  0
 #define TDA9873_MOUT_DUALB  1 << 3
 #define TDA9873_MOUT_ST     1 << 4
-#define TDA9873_MOUT_EXTM   (1 << 4 ) & (1 << 3)
+#define TDA9873_MOUT_EXTM   ((1 << 4) | (1 << 3))
 #define TDA9873_MOUT_EXTL   1 << 5
-#define TDA9873_MOUT_EXTR   (1 << 5 ) & (1 << 3)
-#define TDA9873_MOUT_EXTLR  (1 << 5 ) & (1 << 4)
-#define TDA9873_MOUT_MUTE   (1 << 5 ) & (1 << 4) & (1 << 3)
+#define TDA9873_MOUT_EXTR   ((1 << 5) | (1 << 3))
+#define TDA9873_MOUT_EXTLR  ((1 << 5) | (1 << 4))
+#define TDA9873_MOUT_MUTE   ((1 << 5) | (1 << 4) | (1 << 3))
 
 /* Status bits: (chip read) */
 #define TDA9873_PONR        0 /* Power-on reset detected if = 1 */
-- 
1.7.0.5

[PATCH 2/9] tvaudio: fix tda8425_setmode

[Daniel Glöckner]

The passed audio mode is not a bitfield.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 9b85e2a..76a8cbe 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -1230,21 +1230,25 @@ static void tda8425_setmode(struct CHIPSTATE *chip, int mode)
 {
 	int s1 = chip->shadow.bytes[TDA8425_S1+1] & 0xe1;
 
-	if (mode & V4L2_TUNER_MODE_LANG1) {
+	switch (mode) {
+	case V4L2_TUNER_MODE_LANG1:
 		s1 |= TDA8425_S1_ML_SOUND_A;
 		s1 |= TDA8425_S1_STEREO_PSEUDO;
-
-	} else if (mode & V4L2_TUNER_MODE_LANG2) {
+		break;
+	case V4L2_TUNER_MODE_LANG2:
 		s1 |= TDA8425_S1_ML_SOUND_B;
 		s1 |= TDA8425_S1_STEREO_PSEUDO;
-
-	} else {
+		break;
+	case V4L2_TUNER_MODE_MONO:
 		s1 |= TDA8425_S1_ML_STEREO;
-
-		if (mode & V4L2_TUNER_MODE_MONO)
-			s1 |= TDA8425_S1_STEREO_MONO;
-		if (mode & V4L2_TUNER_MODE_STEREO)
-			s1 |= TDA8425_S1_STEREO_SPATIAL;
+		s1 |= TDA8425_S1_STEREO_MONO;
+		break;
+	case V4L2_TUNER_MODE_STEREO:
+		s1 |= TDA8425_S1_ML_STEREO;
+		s1 |= TDA8425_S1_STEREO_SPATIAL;
+		break;
+	default:
+		return;
 	}
 	chip_write(chip,TDA8425_S1,s1);
 }
-- 
1.7.0.5

[PATCH 3/9] tvaudio: use V4L2_TUNER_MODE_SAP for TDA985x SAP

[Daniel Glöckner]

As V4L2_TUNER_MODE_SAP == V4L2_TUNER_MODE_LANG2, we make
V4L2_TUNER_MODE_LANG1 equal to V4L2_TUNER_MODE_STEREO.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 76a8cbe..3fbaaa0 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -534,9 +534,10 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
 		c6 |= TDA985x_MONO;
 		break;
 	case V4L2_TUNER_MODE_STEREO:
+	case V4L2_TUNER_MODE_LANG1:
 		c6 |= TDA985x_STEREO;
 		break;
-	case V4L2_TUNER_MODE_LANG1:
+	case V4L2_TUNER_MODE_SAP:
 		c6 |= TDA985x_SAP;
 		break;
 	default:
-- 
1.7.0.5

[PATCH 4/9] tvaudio: remove watch_stereo

[Daniel Glöckner]

It is never read and only assigned 0.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 3fbaaa0..fc37587 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -126,7 +126,6 @@ struct CHIPSTATE {
 	/* thread */
 	struct task_struct   *thread;
 	struct timer_list    wt;
-	int                  watch_stereo;
 	int 		     audmode;
 };
 
@@ -1741,7 +1740,6 @@ static int tvaudio_s_radio(struct v4l2_subdev *sd)
 	struct CHIPSTATE *chip = to_state(sd);
 
 	chip->radio = 1;
-	chip->watch_stereo = 0;
 	/* del_timer(&chip->wt); */
 	return 0;
 }
@@ -1821,7 +1819,6 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 	chip->audmode = vt->audmode;
 
 	if (mode) {
-		chip->watch_stereo = 0;
 		/* del_timer(&chip->wt); */
 		chip->mode = mode;
 		desc->setmode(chip, mode);
-- 
1.7.0.5

[PATCH 5/9] tvaudio: don't use thread for TA8874Z

[Daniel Glöckner]

Judging from the data sheet it will automatically switch to the next
best audio mode in accordance with the V4L2 tuner audio matrix.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index fc37587..0e77d49 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -1597,7 +1597,6 @@ static struct CHIPDESC chiplist[] = {
 		.addr_lo    = I2C_ADDR_TDA9840 >> 1,
 		.addr_hi    = I2C_ADDR_TDA9840 >> 1,
 		.registers  = 2,
-		.flags      = CHIP_NEED_CHECKMODE,
 
 		/* callbacks */
 		.getmode    = ta8874z_getmode,
-- 
1.7.0.5

[PATCH 6/9] tvaudio: use V4L2_TUNER_SUB_* for bitfields

[Daniel Glöckner]

The V4L2_TUNER_MODE_* constants are not suited for use in bitfields.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |   63 +++++++++++++++++-----------------------
 1 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 0e77d49..58a0e9c 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -315,13 +315,13 @@ static int chip_thread(void *data)
 
 		chip->prevmode = mode;
 
-		if (mode & V4L2_TUNER_MODE_STEREO)
+		if (mode & V4L2_TUNER_SUB_STEREO)
 			desc->setmode(chip, V4L2_TUNER_MODE_STEREO);
-		if (mode & V4L2_TUNER_MODE_LANG1_LANG2)
+		if (mode & V4L2_TUNER_SUB_LANG1_LANG2)
 			desc->setmode(chip, V4L2_TUNER_MODE_STEREO);
-		else if (mode & V4L2_TUNER_MODE_LANG1)
+		else if (mode & V4L2_SUB_MODE_LANG1)
 			desc->setmode(chip, V4L2_TUNER_MODE_LANG1);
-		else if (mode & V4L2_TUNER_MODE_LANG2)
+		else if (mode & V4L2_SUB_MODE_LANG2)
 			desc->setmode(chip, V4L2_TUNER_MODE_LANG2);
 		else
 			desc->setmode(chip, V4L2_TUNER_MODE_MONO);
@@ -363,11 +363,11 @@ static int tda9840_getmode(struct CHIPSTATE *chip)
 	int val, mode;
 
 	val = chip_read(chip);
-	mode = V4L2_TUNER_MODE_MONO;
+	mode = V4L2_TUNER_SUB_MONO;
 	if (val & TDA9840_DS_DUAL)
-		mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
+		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	if (val & TDA9840_ST_STEREO)
-		mode |= V4L2_TUNER_MODE_STEREO;
+		mode |= V4L2_TUNER_SUB_STEREO;
 
 	v4l2_dbg(1, debug, sd, "tda9840_getmode(): raw chip read: %d, return: %d\n",
 		val, mode);
@@ -514,13 +514,17 @@ static int tda9855_treble(int val) { return (val/0x1c71+0x3)<<1; }
 
 static int  tda985x_getmode(struct CHIPSTATE *chip)
 {
-	int mode;
+	int mode, val;
 
-	mode = ((TDA985x_STP | TDA985x_SAPP) &
-		chip_read(chip)) >> 4;
 	/* Add mono mode regardless of SAP and stereo */
 	/* Allows forced mono */
-	return mode | V4L2_TUNER_MODE_MONO;
+	mode = V4L2_TUNER_SUB_MONO;
+	val = chip_read(chip);
+	if (val & TDA985x_STP)
+		mode |= V4L2_TUNER_SUB_STEREO;
+	if (val & TDA985x_SAPP)
+		mode |= V4L2_TUNER_SUB_SAP;
+	return mode;
 }
 
 static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
@@ -670,11 +674,11 @@ static int tda9873_getmode(struct CHIPSTATE *chip)
 	int val,mode;
 
 	val = chip_read(chip);
-	mode = V4L2_TUNER_MODE_MONO;
+	mode = V4L2_TUNER_SUB_MONO;
 	if (val & TDA9873_STEREO)
-		mode |= V4L2_TUNER_MODE_STEREO;
+		mode |= V4L2_TUNER_SUB_STEREO;
 	if (val & TDA9873_DUAL)
-		mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
+		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	v4l2_dbg(1, debug, sd, "tda9873_getmode(): raw chip read: %d, return: %d\n",
 		val, mode);
 	return mode;
@@ -865,7 +869,7 @@ static int tda9874a_getmode(struct CHIPSTATE *chip)
 	int dsr,nsr,mode;
 	int necr; /* just for debugging */
 
-	mode = V4L2_TUNER_MODE_MONO;
+	mode = V4L2_TUNER_SUB_MONO;
 
 	if(-1 == (dsr = chip_read2(chip,TDA9874A_DSR)))
 		return mode;
@@ -888,14 +892,14 @@ static int tda9874a_getmode(struct CHIPSTATE *chip)
 		 * external 4052 multiplexer in audio_hook().
 		 */
 		if(nsr & 0x02) /* NSR.S/MB=1 */
-			mode |= V4L2_TUNER_MODE_STEREO;
+			mode |= V4L2_TUNER_SUB_STEREO;
 		if(nsr & 0x01) /* NSR.D/SB=1 */
-			mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
+			mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	} else {
 		if(dsr & 0x02) /* DSR.IDSTE=1 */
-			mode |= V4L2_TUNER_MODE_STEREO;
+			mode |= V4L2_TUNER_SUB_STEREO;
 		if(dsr & 0x04) /* DSR.IDDUA=1 */
-			mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
+			mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	}
 
 	v4l2_dbg(1, debug, sd, "tda9874a_getmode(): DSR=0x%X, NSR=0x%X, NECR=0x%X, return: %d.\n",
@@ -1306,11 +1310,11 @@ static int ta8874z_getmode(struct CHIPSTATE *chip)
 	int val, mode;
 
 	val = chip_read(chip);
-	mode = V4L2_TUNER_MODE_MONO;
+	mode = V4L2_TUNER_SUB_MONO;
 	if (val & TA8874Z_B1){
-		mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
+		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	}else if (!(val & TA8874Z_B0)){
-		mode |= V4L2_TUNER_MODE_STEREO;
+		mode |= V4L2_TUNER_SUB_STEREO;
 	}
 	/* v4l_dbg(1, debug, chip->c, "ta8874z_getmode(): raw chip read: 0x%02x, return: 0x%02x\n", val, mode); */
 	return mode;
@@ -1829,7 +1833,6 @@ static int tvaudio_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 {
 	struct CHIPSTATE *chip = to_state(sd);
 	struct CHIPDESC *desc = chip->desc;
-	int mode = V4L2_TUNER_MODE_MONO;
 
 	if (!desc->getmode)
 		return 0;
@@ -1837,22 +1840,10 @@ static int tvaudio_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 		return 0;
 
 	vt->audmode = chip->audmode;
-	vt->rxsubchans = 0;
+	vt->rxsubchans = desc->getmode(chip);
 	vt->capability = V4L2_TUNER_CAP_STEREO |
 		V4L2_TUNER_CAP_LANG1 | V4L2_TUNER_CAP_LANG2;
 
-	mode = desc->getmode(chip);
-
-	if (mode & V4L2_TUNER_MODE_MONO)
-		vt->rxsubchans |= V4L2_TUNER_SUB_MONO;
-	if (mode & V4L2_TUNER_MODE_STEREO)
-		vt->rxsubchans |= V4L2_TUNER_SUB_STEREO;
-	/* Note: for SAP it should be mono/lang2 or stereo/lang2.
-	   When this module is converted fully to v4l2, then this
-	   should change for those chips that can detect SAP. */
-	if (mode & V4L2_TUNER_MODE_LANG1)
-		vt->rxsubchans = V4L2_TUNER_SUB_LANG1 |
-			V4L2_TUNER_SUB_LANG2;
 	return 0;
 }
 
-- 
1.7.0.5

[PATCH 7/9] tvaudio: obey V4L2 tuner audio matrix

[Daniel Glöckner]

V4L2 specifies the audio mode to use for combinations of possible
(rxsubchans) and requested (audmode) audio modes. Up to now tvaudio
has made these decisions automatically based on the possible audio
modes from setting of the frequency until VIDIOC_S_TUNER was called.
It then forced the hardware to use the mode requested by the user.
With this patch it continues to adjust the audio mode while taking
the requested mode into account.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |   61 +++++++++++++++++++++-------------------
 1 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 58a0e9c..04ebdfe 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -118,7 +118,7 @@ struct CHIPSTATE {
 	audiocmd   shadow;
 
 	/* current settings */
-	__u16 left,right,treble,bass,muted,mode;
+	__u16 left, right, treble, bass, muted;
 	int prevmode;
 	int radio;
 	int input;
@@ -287,7 +287,7 @@ static int chip_thread(void *data)
 	struct CHIPSTATE *chip = data;
 	struct CHIPDESC  *desc = chip->desc;
 	struct v4l2_subdev *sd = &chip->sd;
-	int mode;
+	int mode, selected;
 
 	v4l2_dbg(1, debug, sd, "thread started\n");
 	set_freezable();
@@ -301,8 +301,8 @@ static int chip_thread(void *data)
 			break;
 		v4l2_dbg(1, debug, sd, "thread wakeup\n");
 
-		/* don't do anything for radio or if mode != auto */
-		if (chip->radio || chip->mode != 0)
+		/* don't do anything for radio */
+		if (chip->radio)
 			continue;
 
 		/* have a look what's going on */
@@ -315,16 +315,27 @@ static int chip_thread(void *data)
 
 		chip->prevmode = mode;
 
-		if (mode & V4L2_TUNER_SUB_STEREO)
-			desc->setmode(chip, V4L2_TUNER_MODE_STEREO);
-		if (mode & V4L2_TUNER_SUB_LANG1_LANG2)
-			desc->setmode(chip, V4L2_TUNER_MODE_STEREO);
-		else if (mode & V4L2_SUB_MODE_LANG1)
-			desc->setmode(chip, V4L2_TUNER_MODE_LANG1);
-		else if (mode & V4L2_SUB_MODE_LANG2)
-			desc->setmode(chip, V4L2_TUNER_MODE_LANG2);
-		else
-			desc->setmode(chip, V4L2_TUNER_MODE_MONO);
+		selected = V4L2_TUNER_MODE_MONO;
+		switch (chip->audmode) {
+		case V4L2_TUNER_MODE_MONO:
+			if (mode & V4L2_TUNER_SUB_LANG1)
+				selected = V4L2_TUNER_MODE_LANG1;
+			break;
+		case V4L2_TUNER_MODE_STEREO:
+		case V4L2_TUNER_MODE_LANG1:
+			if (mode & V4L2_TUNER_SUB_LANG1)
+				selected = V4L2_TUNER_MODE_LANG1;
+			else if (mode & V4L2_TUNER_SUB_STEREO)
+				selected = V4L2_TUNER_MODE_STEREO;
+			break;
+		case V4L2_TUNER_MODE_LANG2:
+			if (mode & V4L2_TUNER_SUB_LANG2)
+				selected = V4L2_TUNER_MODE_LANG2;
+			else if (mode & V4L2_TUNER_SUB_STEREO)
+				selected = V4L2_TUNER_MODE_STEREO;
+			break;
+		}
+		desc->setmode(chip, selected);
 
 		/* schedule next check */
 		mod_timer(&chip->wt, jiffies+msecs_to_jiffies(2000));
@@ -712,7 +723,6 @@ static void tda9873_setmode(struct CHIPSTATE *chip, int mode)
 		sw_data |= TDA9873_TR_DUALB;
 		break;
 	default:
-		chip->mode = 0;
 		return;
 	}
 
@@ -944,7 +954,6 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
 			mdacosr = (tda9874a_mode) ? 0x83:0x81;
 			break;
 		default:
-			chip->mode = 0;
 			return;
 		}
 		chip_write(chip, TDA9874A_AOSR, aosr);
@@ -979,7 +988,6 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
 			aosr = 0x20; /* dual B/B */
 			break;
 		default:
-			chip->mode = 0;
 			return;
 		}
 		chip_write(chip, TDA9874A_FMMR, fmmr);
@@ -1799,7 +1807,6 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 {
 	struct CHIPSTATE *chip = to_state(sd);
 	struct CHIPDESC *desc = chip->desc;
-	int mode = 0;
 
 	if (!desc->setmode)
 		return 0;
@@ -1811,21 +1818,20 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 	case V4L2_TUNER_MODE_STEREO:
 	case V4L2_TUNER_MODE_LANG1:
 	case V4L2_TUNER_MODE_LANG2:
-		mode = vt->audmode;
 		break;
 	case V4L2_TUNER_MODE_LANG1_LANG2:
-		mode = V4L2_TUNER_MODE_STEREO;
+		vt->audmode = V4L2_TUNER_MODE_STEREO;
 		break;
 	default:
 		return -EINVAL;
 	}
 	chip->audmode = vt->audmode;
 
-	if (mode) {
-		/* del_timer(&chip->wt); */
-		chip->mode = mode;
-		desc->setmode(chip, mode);
-	}
+	if (chip->thread)
+		wake_up_process(chip->thread);
+	else
+		desc->setmode(chip, vt->audmode);
+
 	return 0;
 }
 
@@ -1860,8 +1866,6 @@ static int tvaudio_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *fr
 	struct CHIPSTATE *chip = to_state(sd);
 	struct CHIPDESC *desc = chip->desc;
 
-	chip->mode = 0; /* automatic */
-
 	/* For chips that provide getmode and setmode, and doesn't
 	   automatically follows the stereo carrier, a kthread is
 	   created to set the audio standard. In this case, when then
@@ -1872,8 +1876,7 @@ static int tvaudio_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *fr
 	 */
 	if (chip->thread) {
 		desc->setmode(chip, V4L2_TUNER_MODE_MONO);
-		if (chip->prevmode != V4L2_TUNER_MODE_MONO)
-			chip->prevmode = -1; /* reset previous mode */
+		chip->prevmode = -1; /* reset previous mode */
 		mod_timer(&chip->wt, jiffies+msecs_to_jiffies(2000));
 	}
 	return 0;
-- 
1.7.0.5

[PATCH 8/9] tvaudio: support V4L2_TUNER_MODE_LANG1_LANG2

[Daniel Glöckner]

Many of the audio decoders handled by the driver support this mode,
so the driver should support it as well.

Coding style errors are done to blend into the surrounding code.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index 04ebdfe..f3ce93a 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -334,6 +334,11 @@ static int chip_thread(void *data)
 			else if (mode & V4L2_TUNER_SUB_STEREO)
 				selected = V4L2_TUNER_MODE_STEREO;
 			break;
+		case V4L2_TUNER_MODE_LANG1_LANG2:
+			if (mode & V4L2_TUNER_SUB_LANG2)
+				selected = V4L2_TUNER_MODE_LANG1_LANG2;
+			else if (mode & V4L2_TUNER_SUB_STEREO)
+				selected = V4L2_TUNER_MODE_STEREO;
 		}
 		desc->setmode(chip, selected);
 
@@ -403,6 +408,9 @@ static void tda9840_setmode(struct CHIPSTATE *chip, int mode)
 	case V4L2_TUNER_MODE_LANG2:
 		t |= TDA9840_DUALB;
 		break;
+	case V4L2_TUNER_MODE_LANG1_LANG2:
+		t |= TDA9840_DUALAB;
+		break;
 	default:
 		update = 0;
 	}
@@ -487,6 +495,7 @@ static int tda9840_checkit(struct CHIPSTATE *chip)
 /* 0x06 - C6 - Control 2 in TDA9855, Control 3 in TDA9850 */
 /* Common to TDA9855 and TDA9850: */
 #define TDA985x_SAP	3<<6 /* Selects SAP output, mute if not received */
+#define TDA985x_MONOSAP	2<<6 /* Selects Mono on left, SAP on right */
 #define TDA985x_STEREO	1<<6 /* Selects Stereo ouput, mono if not received */
 #define TDA985x_MONO	0    /* Forces Mono output */
 #define TDA985x_LMU	1<<3 /* Mute (LOR/LOL for 9855, OUTL/OUTR for 9850) */
@@ -554,6 +563,9 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
 	case V4L2_TUNER_MODE_SAP:
 		c6 |= TDA985x_SAP;
 		break;
+	case V4L2_TUNER_MODE_LANG1_LANG2:
+		c6 |= TDA985x_MONOSAP;
+		break;
 	default:
 		update = 0;
 	}
@@ -601,6 +613,7 @@ static void tda985x_setmode(struct CHIPSTATE *chip, int mode)
 #define TDA9873_TR_REVERSE  ((1 << 3) | (1 << 2))
 #define TDA9873_TR_DUALA    1 << 2
 #define TDA9873_TR_DUALB    1 << 3
+#define TDA9873_TR_DUALAB   0
 
 /* output level controls
  * B5:  output level switch (0 = reduced gain, 1 = normal gain)
@@ -722,6 +735,9 @@ static void tda9873_setmode(struct CHIPSTATE *chip, int mode)
 	case V4L2_TUNER_MODE_LANG2:
 		sw_data |= TDA9873_TR_DUALB;
 		break;
+	case V4L2_TUNER_MODE_LANG1_LANG2:
+		sw_data |= TDA9873_TR_DUALAB;
+		break;
 	default:
 		return;
 	}
@@ -953,6 +969,10 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
 			aosr = 0xa0; /* auto-select, dual B/B */
 			mdacosr = (tda9874a_mode) ? 0x83:0x81;
 			break;
+		case V4L2_TUNER_MODE_LANG1_LANG2:
+			aosr = 0x00; /* always route L to L and R to R */
+			mdacosr = (tda9874a_mode) ? 0x82:0x80;
+			break;
 		default:
 			return;
 		}
@@ -987,6 +1007,10 @@ static void tda9874a_setmode(struct CHIPSTATE *chip, int mode)
 			fmmr = 0x02; /* dual */
 			aosr = 0x20; /* dual B/B */
 			break;
+		case V4L2_TUNER_MODE_LANG1_LANG2:
+			fmmr = 0x02; /* dual */
+			aosr = 0x00; /* dual A/B */
+			break;
 		default:
 			return;
 		}
@@ -1251,6 +1275,10 @@ static void tda8425_setmode(struct CHIPSTATE *chip, int mode)
 		s1 |= TDA8425_S1_ML_SOUND_B;
 		s1 |= TDA8425_S1_STEREO_PSEUDO;
 		break;
+	case V4L2_TUNER_MODE_LANG1_LANG2:
+		s1 |= TDA8425_S1_ML_STEREO;
+		s1 |= TDA8425_S1_STEREO_LINEAR;
+		break;
 	case V4L2_TUNER_MODE_MONO:
 		s1 |= TDA8425_S1_ML_STEREO;
 		s1 |= TDA8425_S1_STEREO_MONO;
@@ -1332,6 +1360,7 @@ static audiocmd ta8874z_stereo = { 2, {0, TA8874Z_SEPARATION_DEFAULT}};
 static audiocmd ta8874z_mono = {2, { TA8874Z_MONO_SET, TA8874Z_SEPARATION_DEFAULT}};
 static audiocmd ta8874z_main = {2, { 0, TA8874Z_SEPARATION_DEFAULT}};
 static audiocmd ta8874z_sub = {2, { TA8874Z_MODE_SUB, TA8874Z_SEPARATION_DEFAULT}};
+static audiocmd ta8874z_both = {2, { TA8874Z_MODE_MAIN | TA8874Z_MODE_SUB, TA8874Z_SEPARATION_DEFAULT}};
 
 static void ta8874z_setmode(struct CHIPSTATE *chip, int mode)
 {
@@ -1354,6 +1383,9 @@ static void ta8874z_setmode(struct CHIPSTATE *chip, int mode)
 	case V4L2_TUNER_MODE_LANG2:
 		t = &ta8874z_sub;
 		break;
+	case V4L2_TUNER_MODE_LANG1_LANG2:
+		t = &ta8874z_both;
+		break;
 	default:
 		update = 0;
 	}
@@ -1818,9 +1850,7 @@ static int tvaudio_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
 	case V4L2_TUNER_MODE_STEREO:
 	case V4L2_TUNER_MODE_LANG1:
 	case V4L2_TUNER_MODE_LANG2:
-		break;
 	case V4L2_TUNER_MODE_LANG1_LANG2:
-		vt->audmode = V4L2_TUNER_MODE_STEREO;
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.0.5

[PATCH 9/9] tvaudio: don't report mono when stereo is received

[Daniel Glöckner]

The V4L2 spec says reporting mono and stereo at the same time means
the hardware can not distinguish between the two. So when we can,
we should report only one of them.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 drivers/media/video/tvaudio.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c
index f3ce93a..1e61cbf 100644
--- a/drivers/media/video/tvaudio.c
+++ b/drivers/media/video/tvaudio.c
@@ -383,7 +383,7 @@ static int tda9840_getmode(struct CHIPSTATE *chip)
 	if (val & TDA9840_DS_DUAL)
 		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	if (val & TDA9840_ST_STEREO)
-		mode |= V4L2_TUNER_SUB_STEREO;
+		mode = V4L2_TUNER_SUB_STEREO;
 
 	v4l2_dbg(1, debug, sd, "tda9840_getmode(): raw chip read: %d, return: %d\n",
 		val, mode);
@@ -541,7 +541,7 @@ static int  tda985x_getmode(struct CHIPSTATE *chip)
 	mode = V4L2_TUNER_SUB_MONO;
 	val = chip_read(chip);
 	if (val & TDA985x_STP)
-		mode |= V4L2_TUNER_SUB_STEREO;
+		mode = V4L2_TUNER_SUB_STEREO;
 	if (val & TDA985x_SAPP)
 		mode |= V4L2_TUNER_SUB_SAP;
 	return mode;
@@ -700,7 +700,7 @@ static int tda9873_getmode(struct CHIPSTATE *chip)
 	val = chip_read(chip);
 	mode = V4L2_TUNER_SUB_MONO;
 	if (val & TDA9873_STEREO)
-		mode |= V4L2_TUNER_SUB_STEREO;
+		mode = V4L2_TUNER_SUB_STEREO;
 	if (val & TDA9873_DUAL)
 		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	v4l2_dbg(1, debug, sd, "tda9873_getmode(): raw chip read: %d, return: %d\n",
@@ -918,12 +918,12 @@ static int tda9874a_getmode(struct CHIPSTATE *chip)
 		 * external 4052 multiplexer in audio_hook().
 		 */
 		if(nsr & 0x02) /* NSR.S/MB=1 */
-			mode |= V4L2_TUNER_SUB_STEREO;
+			mode = V4L2_TUNER_SUB_STEREO;
 		if(nsr & 0x01) /* NSR.D/SB=1 */
 			mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	} else {
 		if(dsr & 0x02) /* DSR.IDSTE=1 */
-			mode |= V4L2_TUNER_SUB_STEREO;
+			mode = V4L2_TUNER_SUB_STEREO;
 		if(dsr & 0x04) /* DSR.IDDUA=1 */
 			mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	}
@@ -1350,7 +1350,7 @@ static int ta8874z_getmode(struct CHIPSTATE *chip)
 	if (val & TA8874Z_B1){
 		mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
 	}else if (!(val & TA8874Z_B0)){
-		mode |= V4L2_TUNER_SUB_STEREO;
+		mode = V4L2_TUNER_SUB_STEREO;
 	}
 	/* v4l_dbg(1, debug, chip->c, "ta8874z_getmode(): raw chip read: 0x%02x, return: 0x%02x\n", val, mode); */
 	return mode;
-- 
1.7.0.5

Re: question about bt8xx/bttv-audio-hook.c, tvaudio.c

[Julia Lawall]

On Sat, 9 Jun 2012, Hans Verkuil wrote:

> On Wed June 6 2012 09:06:23 Julia Lawall wrote:
>> The files drivers/media/video/bt8xx/bttv-audio-hook.c and
>> drivers/media/video/tvaudio.c contain a number of occurrences of eg:
>>
>> mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
>>
>> and
>>
>> if (mode & V4L2_TUNER_MODE_MONO)
>>
>> (both from tvaudio.c)
>>
>> V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2 is suspicious because
>> V4L2_TUNER_MODE_LANG1 is 3 and V4L2_TUNER_MODE_LANG2 is 2, so the result
>> is just the same as V4L2_TUNER_MODE_LANG1.  Maybe
>> V4L2_TUNER_MODE_LANG1_LANG2 was intended?
>>
>> mode & V4L2_TUNER_MODE_MONO is suspicious because V4L2_TUNER_MODE_MONO is
>> 0.  Maybe & should be ==?
>>
>> If & is to be changed to == everywhere, then some new code may need to be
>> constructed to account for V4L2_TUNER_MODE_LANG1_LANG2.  For example, the
>> function tda8425_setmode has ifs for the other values, but not for this
>> one.  On the other hand, the function ta8874z_setmode already uses == (or
>> rather switch), and does not take V4L2_TUNER_MODE_LANG1_LANG2 into
>> account, so perhaps it is not appropriate in this context?
>
> I would have to analyse this more carefully, but the core issue here is that
> these drivers mixup the tuner audio reception bitmask flags (V4L2_TUNER_SUB_*)
> and the tuner audio modes (V4L2_TUNER_MODE_*, not a bitmask). This happened
> regularly in older drivers, and apparently these two are still not fixed.
>
> More info is here:
>
> http://hverkuil.home.xs4all.nl/spec/media.html#vidioc-g-tuner
>
> I can't just replace one define with another, I would need to look carefully
> at the code to see what was intended.
>
> If you find more places where this happens, then please let us know. Otherwise
> this is something for us to fix.

No, I explicitly looked, but only found the problem in these two files.

julia