[PATCH 1/4] [media] tuner-core: remove usage of DIGITAL_TV

[PATCH 1/4] [media] tuner-core: remove usage of DIGITAL_TV

[Mauro Carvalho Chehab]

tuner-core has no business to do with digital TV. So, don't use
T_DIGITAL_TV on it, as it has no code to distinguish between
them, and nobody fills T_DIGITAL_TV right.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/au0828/au0828-cards.c b/drivers/media/video/au0828/au0828-cards.c
index 01be89f..39fc923 100644
--- a/drivers/media/video/au0828/au0828-cards.c
+++ b/drivers/media/video/au0828/au0828-cards.c
@@ -185,8 +185,7 @@ void au0828_card_setup(struct au0828_dev *dev)
 	static u8 eeprom[256];
 	struct tuner_setup tun_setup;
 	struct v4l2_subdev *sd;
-	unsigned int mode_mask = T_ANALOG_TV |
-				 T_DIGITAL_TV;
+	unsigned int mode_mask = T_ANALOG_TV;
 
 	dprintk(1, "%s()\n", __func__);
 
diff --git a/drivers/media/video/bt8xx/bttv-cards.c b/drivers/media/video/bt8xx/bttv-cards.c
index 7f58756..242f0d5 100644
--- a/drivers/media/video/bt8xx/bttv-cards.c
+++ b/drivers/media/video/bt8xx/bttv-cards.c
@@ -3616,7 +3616,7 @@ void __devinit bttv_init_tuner(struct bttv *btv)
 				&btv->c.i2c_adap, "tuner",
 				0, v4l2_i2c_tuner_addrs(ADDRS_TV_WITH_DEMOD));
 
-		tun_setup.mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
+		tun_setup.mode_mask = T_ANALOG_TV;
 		tun_setup.type = btv->tuner_type;
 		tun_setup.addr = addr;
 
diff --git a/drivers/media/video/cx88/cx88-cards.c b/drivers/media/video/cx88/cx88-cards.c
index 4e6ee55..8128b93 100644
--- a/drivers/media/video/cx88/cx88-cards.c
+++ b/drivers/media/video/cx88/cx88-cards.c
@@ -3165,9 +3165,7 @@ static void cx88_card_setup(struct cx88_core *core)
 {
 	static u8 eeprom[256];
 	struct tuner_setup tun_setup;
-	unsigned int mode_mask = T_RADIO     |
-				 T_ANALOG_TV |
-				 T_DIGITAL_TV;
+	unsigned int mode_mask = T_RADIO | T_ANALOG_TV;
 
 	memset(&tun_setup, 0, sizeof(tun_setup));
 
diff --git a/drivers/media/video/saa7134/saa7134-cards.c b/drivers/media/video/saa7134/saa7134-cards.c
index 74467c1..61c6007 100644
--- a/drivers/media/video/saa7134/saa7134-cards.c
+++ b/drivers/media/video/saa7134/saa7134-cards.c
@@ -7333,9 +7333,7 @@ int saa7134_board_init1(struct saa7134_dev *dev)
 static void saa7134_tuner_setup(struct saa7134_dev *dev)
 {
 	struct tuner_setup tun_setup;
-	unsigned int mode_mask = T_RADIO     |
-				 T_ANALOG_TV |
-				 T_DIGITAL_TV;
+	unsigned int mode_mask = T_RADIO | T_ANALOG_TV;
 
 	memset(&tun_setup, 0, sizeof(tun_setup));
 	tun_setup.tuner_callback = saa7134_tuner_callback;
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index dcf03fa..5e1437c 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -497,7 +497,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
 		   device. If other devices appear then we need to
 		   make this test more general. */
 		else if (*tv == NULL && pos->type != TUNER_TDA9887 &&
-			 (pos->mode_mask & (T_ANALOG_TV | T_DIGITAL_TV)))
+			 (pos->mode_mask & T_ANALOG_TV))
 			*tv = pos;
 	}
 }
@@ -565,8 +565,7 @@ static int tuner_probe(struct i2c_client *client,
 			} else {
 				/* Default is being tda9887 */
 				t->type = TUNER_TDA9887;
-				t->mode_mask = T_RADIO | T_ANALOG_TV |
-					       T_DIGITAL_TV;
+				t->mode_mask = T_RADIO | T_ANALOG_TV;
 				goto register_client;
 			}
 			break;
@@ -596,7 +595,7 @@ static int tuner_probe(struct i2c_client *client,
 	   first found TV tuner. */
 	tuner_lookup(t->i2c->adapter, &radio, &tv);
 	if (tv == NULL) {
-		t->mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
+		t->mode_mask = T_ANALOG_TV;
 		if (radio == NULL)
 			t->mode_mask |= T_RADIO;
 		tuner_dbg("Setting mode_mask to 0x%02x\n", t->mode_mask);
@@ -607,18 +606,15 @@ register_client:
 	/* Sets a default mode */
 	if (t->mode_mask & T_ANALOG_TV)
 		t->mode = V4L2_TUNER_ANALOG_TV;
-	else if (t->mode_mask & T_RADIO)
-		t->mode = V4L2_TUNER_RADIO;
 	else
-		t->mode = V4L2_TUNER_DIGITAL_TV;
+		t->mode = V4L2_TUNER_RADIO;
 	set_type(client, t->type, t->mode_mask, t->config, t->fe.callback);
 	list_add_tail(&t->list, &tuner_list);
 
-	tuner_info("Tuner %d found with type(s)%s%s%s.\n",
+	tuner_info("Tuner %d found with type(s)%s%s.\n",
 		   t->type,
-		   t->mode_mask & T_RADIO ? " radio" : "",
-		   t->mode_mask & T_ANALOG_TV ? " TV" : "",
-		   t->mode_mask & T_ANALOG_TV ? " DTV" : "");
+		   t->mode_mask & T_RADIO ? " Radio" : "",
+		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
 	return 0;
 }
 
-- 
1.7.1

Re: [PATCH 1/4] [media] tuner-core: remove usage of DIGITAL_TV

[Andy Walls]

Mauro Carvalho Chehab <mchehab@redhat.com> wrote:

>tuner-core has no business to do with digital TV. So, don't use
>T_DIGITAL_TV on it, as it has no code to distinguish between
>them, and nobody fills T_DIGITAL_TV right.
>
>Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>
>diff --git a/drivers/media/video/au0828/au0828-cards.c
>b/drivers/media/video/au0828/au0828-cards.c
>index 01be89f..39fc923 100644
>--- a/drivers/media/video/au0828/au0828-cards.c
>+++ b/drivers/media/video/au0828/au0828-cards.c
>@@ -185,8 +185,7 @@ void au0828_card_setup(struct au0828_dev *dev)
> 	static u8 eeprom[256];
> 	struct tuner_setup tun_setup;
> 	struct v4l2_subdev *sd;
>-	unsigned int mode_mask = T_ANALOG_TV |
>-				 T_DIGITAL_TV;
>+	unsigned int mode_mask = T_ANALOG_TV;
> 
> 	dprintk(1, "%s()\n", __func__);
> 
>diff --git a/drivers/media/video/bt8xx/bttv-cards.c
>b/drivers/media/video/bt8xx/bttv-cards.c
>index 7f58756..242f0d5 100644
>--- a/drivers/media/video/bt8xx/bttv-cards.c
>+++ b/drivers/media/video/bt8xx/bttv-cards.c
>@@ -3616,7 +3616,7 @@ void __devinit bttv_init_tuner(struct bttv *btv)
> 				&btv->c.i2c_adap, "tuner",
> 				0, v4l2_i2c_tuner_addrs(ADDRS_TV_WITH_DEMOD));
> 
>-		tun_setup.mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
>+		tun_setup.mode_mask = T_ANALOG_TV;
> 		tun_setup.type = btv->tuner_type;
> 		tun_setup.addr = addr;
> 
>diff --git a/drivers/media/video/cx88/cx88-cards.c
>b/drivers/media/video/cx88/cx88-cards.c
>index 4e6ee55..8128b93 100644
>--- a/drivers/media/video/cx88/cx88-cards.c
>+++ b/drivers/media/video/cx88/cx88-cards.c
>@@ -3165,9 +3165,7 @@ static void cx88_card_setup(struct cx88_core
>*core)
> {
> 	static u8 eeprom[256];
> 	struct tuner_setup tun_setup;
>-	unsigned int mode_mask = T_RADIO     |
>-				 T_ANALOG_TV |
>-				 T_DIGITAL_TV;
>+	unsigned int mode_mask = T_RADIO | T_ANALOG_TV;
> 
> 	memset(&tun_setup, 0, sizeof(tun_setup));
> 
>diff --git a/drivers/media/video/saa7134/saa7134-cards.c
>b/drivers/media/video/saa7134/saa7134-cards.c
>index 74467c1..61c6007 100644
>--- a/drivers/media/video/saa7134/saa7134-cards.c
>+++ b/drivers/media/video/saa7134/saa7134-cards.c
>@@ -7333,9 +7333,7 @@ int saa7134_board_init1(struct saa7134_dev *dev)
> static void saa7134_tuner_setup(struct saa7134_dev *dev)
> {
> 	struct tuner_setup tun_setup;
>-	unsigned int mode_mask = T_RADIO     |
>-				 T_ANALOG_TV |
>-				 T_DIGITAL_TV;
>+	unsigned int mode_mask = T_RADIO | T_ANALOG_TV;
> 
> 	memset(&tun_setup, 0, sizeof(tun_setup));
> 	tun_setup.tuner_callback = saa7134_tuner_callback;
>diff --git a/drivers/media/video/tuner-core.c
>b/drivers/media/video/tuner-core.c
>index dcf03fa..5e1437c 100644
>--- a/drivers/media/video/tuner-core.c
>+++ b/drivers/media/video/tuner-core.c
>@@ -497,7 +497,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
> 		   device. If other devices appear then we need to
> 		   make this test more general. */
> 		else if (*tv == NULL && pos->type != TUNER_TDA9887 &&
>-			 (pos->mode_mask & (T_ANALOG_TV | T_DIGITAL_TV)))
>+			 (pos->mode_mask & T_ANALOG_TV))
> 			*tv = pos;
> 	}
> }
>@@ -565,8 +565,7 @@ static int tuner_probe(struct i2c_client *client,
> 			} else {
> 				/* Default is being tda9887 */
> 				t->type = TUNER_TDA9887;
>-				t->mode_mask = T_RADIO | T_ANALOG_TV |
>-					       T_DIGITAL_TV;
>+				t->mode_mask = T_RADIO | T_ANALOG_TV;
> 				goto register_client;
> 			}
> 			break;
>@@ -596,7 +595,7 @@ static int tuner_probe(struct i2c_client *client,
> 	   first found TV tuner. */
> 	tuner_lookup(t->i2c->adapter, &radio, &tv);
> 	if (tv == NULL) {
>-		t->mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
>+		t->mode_mask = T_ANALOG_TV;
> 		if (radio == NULL)
> 			t->mode_mask |= T_RADIO;
> 		tuner_dbg("Setting mode_mask to 0x%02x\n", t->mode_mask);
>@@ -607,18 +606,15 @@ register_client:
> 	/* Sets a default mode */
> 	if (t->mode_mask & T_ANALOG_TV)
> 		t->mode = V4L2_TUNER_ANALOG_TV;
>-	else if (t->mode_mask & T_RADIO)
>-		t->mode = V4L2_TUNER_RADIO;
> 	else
>-		t->mode = V4L2_TUNER_DIGITAL_TV;
>+		t->mode = V4L2_TUNER_RADIO;
> 	set_type(client, t->type, t->mode_mask, t->config, t->fe.callback);
> 	list_add_tail(&t->list, &tuner_list);
> 
>-	tuner_info("Tuner %d found with type(s)%s%s%s.\n",
>+	tuner_info("Tuner %d found with type(s)%s%s.\n",
> 		   t->type,
>-		   t->mode_mask & T_RADIO ? " radio" : "",
>-		   t->mode_mask & T_ANALOG_TV ? " TV" : "",
>-		   t->mode_mask & T_ANALOG_TV ? " DTV" : "");
>+		   t->mode_mask & T_RADIO ? " Radio" : "",
>+		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
> 	return 0;
> }
> 
>-- 
>1.7.1
>
>
>--
>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

Hmm.  I thought tuner-cards.c or tuner-simple.c had entries for hybrid tuner assemblies.  You are changing the default mode from digital to radio; does that affect the use of the hybrid tuner assemblies.  

Regards,
Andy
(Not near a decent computer screen at the moment.)

Re: [PATCH 1/4] [media] tuner-core: remove usage of DIGITAL_TV

[Mauro Carvalho Chehab]

Em 15-02-2011 15:25, Andy Walls escreveu:
> Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
> 
>> tuner-core has no business to do with digital TV. So, don't use
>> T_DIGITAL_TV on it, as it has no code to distinguish between
>> them, and nobody fills T_DIGITAL_TV right.
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>>
>> diff --git a/drivers/media/video/au0828/au0828-cards.c
>> b/drivers/media/video/au0828/au0828-cards.c
>> index 01be89f..39fc923 100644
>> --- a/drivers/media/video/au0828/au0828-cards.c
>> +++ b/drivers/media/video/au0828/au0828-cards.c
>> @@ -185,8 +185,7 @@ void au0828_card_setup(struct au0828_dev *dev)
>> 	static u8 eeprom[256];
>> 	struct tuner_setup tun_setup;
>> 	struct v4l2_subdev *sd;
>> -	unsigned int mode_mask = T_ANALOG_TV |
>> -				 T_DIGITAL_TV;
>> +	unsigned int mode_mask = T_ANALOG_TV;
>>
>> 	dprintk(1, "%s()\n", __func__);
>>
>> diff --git a/drivers/media/video/bt8xx/bttv-cards.c
>> b/drivers/media/video/bt8xx/bttv-cards.c
>> index 7f58756..242f0d5 100644
>> --- a/drivers/media/video/bt8xx/bttv-cards.c
>> +++ b/drivers/media/video/bt8xx/bttv-cards.c
>> @@ -3616,7 +3616,7 @@ void __devinit bttv_init_tuner(struct bttv *btv)
>> 				&btv->c.i2c_adap, "tuner",
>> 				0, v4l2_i2c_tuner_addrs(ADDRS_TV_WITH_DEMOD));
>>
>> -		tun_setup.mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
>> +		tun_setup.mode_mask = T_ANALOG_TV;
>> 		tun_setup.type = btv->tuner_type;
>> 		tun_setup.addr = addr;
>>
>> diff --git a/drivers/media/video/cx88/cx88-cards.c
>> b/drivers/media/video/cx88/cx88-cards.c
>> index 4e6ee55..8128b93 100644
>> --- a/drivers/media/video/cx88/cx88-cards.c
>> +++ b/drivers/media/video/cx88/cx88-cards.c
>> @@ -3165,9 +3165,7 @@ static void cx88_card_setup(struct cx88_core
>> *core)
>> {
>> 	static u8 eeprom[256];
>> 	struct tuner_setup tun_setup;
>> -	unsigned int mode_mask = T_RADIO     |
>> -				 T_ANALOG_TV |
>> -				 T_DIGITAL_TV;
>> +	unsigned int mode_mask = T_RADIO | T_ANALOG_TV;
>>
>> 	memset(&tun_setup, 0, sizeof(tun_setup));
>>
>> diff --git a/drivers/media/video/saa7134/saa7134-cards.c
>> b/drivers/media/video/saa7134/saa7134-cards.c
>> index 74467c1..61c6007 100644
>> --- a/drivers/media/video/saa7134/saa7134-cards.c
>> +++ b/drivers/media/video/saa7134/saa7134-cards.c
>> @@ -7333,9 +7333,7 @@ int saa7134_board_init1(struct saa7134_dev *dev)
>> static void saa7134_tuner_setup(struct saa7134_dev *dev)
>> {
>> 	struct tuner_setup tun_setup;
>> -	unsigned int mode_mask = T_RADIO     |
>> -				 T_ANALOG_TV |
>> -				 T_DIGITAL_TV;
>> +	unsigned int mode_mask = T_RADIO | T_ANALOG_TV;
>>
>> 	memset(&tun_setup, 0, sizeof(tun_setup));
>> 	tun_setup.tuner_callback = saa7134_tuner_callback;
>> diff --git a/drivers/media/video/tuner-core.c
>> b/drivers/media/video/tuner-core.c
>> index dcf03fa..5e1437c 100644
>> --- a/drivers/media/video/tuner-core.c
>> +++ b/drivers/media/video/tuner-core.c
>> @@ -497,7 +497,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
>> 		   device. If other devices appear then we need to
>> 		   make this test more general. */
>> 		else if (*tv == NULL && pos->type != TUNER_TDA9887 &&
>> -			 (pos->mode_mask & (T_ANALOG_TV | T_DIGITAL_TV)))
>> +			 (pos->mode_mask & T_ANALOG_TV))
>> 			*tv = pos;
>> 	}
>> }
>> @@ -565,8 +565,7 @@ static int tuner_probe(struct i2c_client *client,
>> 			} else {
>> 				/* Default is being tda9887 */
>> 				t->type = TUNER_TDA9887;
>> -				t->mode_mask = T_RADIO | T_ANALOG_TV |
>> -					       T_DIGITAL_TV;
>> +				t->mode_mask = T_RADIO | T_ANALOG_TV;
>> 				goto register_client;
>> 			}
>> 			break;
>> @@ -596,7 +595,7 @@ static int tuner_probe(struct i2c_client *client,
>> 	   first found TV tuner. */
>> 	tuner_lookup(t->i2c->adapter, &radio, &tv);
>> 	if (tv == NULL) {
>> -		t->mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
>> +		t->mode_mask = T_ANALOG_TV;
>> 		if (radio == NULL)
>> 			t->mode_mask |= T_RADIO;
>> 		tuner_dbg("Setting mode_mask to 0x%02x\n", t->mode_mask);
>> @@ -607,18 +606,15 @@ register_client:
>> 	/* Sets a default mode */
>> 	if (t->mode_mask & T_ANALOG_TV)
>> 		t->mode = V4L2_TUNER_ANALOG_TV;
>> -	else if (t->mode_mask & T_RADIO)
>> -		t->mode = V4L2_TUNER_RADIO;
>> 	else
>> -		t->mode = V4L2_TUNER_DIGITAL_TV;
>> +		t->mode = V4L2_TUNER_RADIO;
>> 	set_type(client, t->type, t->mode_mask, t->config, t->fe.callback);
>> 	list_add_tail(&t->list, &tuner_list);
>>
>> -	tuner_info("Tuner %d found with type(s)%s%s%s.\n",
>> +	tuner_info("Tuner %d found with type(s)%s%s.\n",
>> 		   t->type,
>> -		   t->mode_mask & T_RADIO ? " radio" : "",
>> -		   t->mode_mask & T_ANALOG_TV ? " TV" : "",
>> -		   t->mode_mask & T_ANALOG_TV ? " DTV" : "");
>> +		   t->mode_mask & T_RADIO ? " Radio" : "",
>> +		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
>> 	return 0;
>> }
>>
>> -- 
>> 1.7.1
>>
>>
>> --
>> 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
> 
> Hmm.  I thought tuner-cards.c or tuner-simple.c had entries for hybrid tuner assemblies.  

They have, but tuner-core takes care only for V4L2 API calls.

> You are changing the default mode from digital to radio; does that affect the use of the hybrid tuner assemblies.  

Where are you seeing such change? I just removed T_DIGITAL_TV mode mask, as this is
unused. On all places at boards, they use a mask with (T_ANALOG_TV | T_DIGITAL_TV).
The same mask is used at tuner-core. This patch is basically:
	s/"T_ANALOG_TV | T_DIGITAL_TV"/T_ANALOG_TV/g

Also, the default mode is almost meaningless. On all VIDIOC calls that touch at tuner
(get/set frequency, get/set tuner), the type of the tuner is passed as a parameter.
So, no default mode is assumed. At digital mode, on all cases, the set_params callback
will pass the bandwidth, digital tv standard and the frequency to set. The digital TV
logic inside the tuner will handle it directly, via a direct I2C attach function, not
using tuner-core.

So, this patch should cause no functional change.

Cheers,
Mauro

Re: [PATCH 1/4] [media] tuner-core: remove usage of DIGITAL_TV

[Andy Walls]

On Tue, 2011-02-15 at 17:48 -0200, Mauro Carvalho Chehab wrote:
> Em 15-02-2011 15:25, Andy Walls escreveu:
> > Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
> > 
> >> tuner-core has no business to do with digital TV. So, don't use
> >> T_DIGITAL_TV on it, as it has no code to distinguish between
> >> them, and nobody fills T_DIGITAL_TV right.
> >>
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> >>

> >> diff --git a/drivers/media/video/tuner-core.c
> >> b/drivers/media/video/tuner-core.c
> >> index dcf03fa..5e1437c 100644
> >> --- a/drivers/media/video/tuner-core.c
> >> +++ b/drivers/media/video/tuner-core.c

> >> @@ -596,7 +595,7 @@ static int tuner_probe(struct i2c_client *client,
> >> 	   first found TV tuner. */
> >> 	tuner_lookup(t->i2c->adapter, &radio, &tv);
> >> 	if (tv == NULL) {
> >> -		t->mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
> >> +		t->mode_mask = T_ANALOG_TV;
> >> 		if (radio == NULL)
> >> 			t->mode_mask |= T_RADIO;
> >> 		tuner_dbg("Setting mode_mask to 0x%02x\n", t->mode_mask);
> >> @@ -607,18 +606,15 @@ register_client:
> >> 	/* Sets a default mode */
> >> 	if (t->mode_mask & T_ANALOG_TV)
> >> 		t->mode = V4L2_TUNER_ANALOG_TV;
> >> -	else if (t->mode_mask & T_RADIO)
> >> -		t->mode = V4L2_TUNER_RADIO;
> >> 	else
> >> -		t->mode = V4L2_TUNER_DIGITAL_TV;
> >> +		t->mode = V4L2_TUNER_RADIO;
                            ^^^^^^^^^^^^^^^^^^^^^
Mauro,

Here's where I saw a default being changed from DIGITAL_TV to RADIO.
Maybe it doesn't matter?

> > Hmm.  I thought tuner-cards.c or tuner-simple.c had entries for hybrid tuner assemblies.  
> 
> They have, but tuner-core takes care only for V4L2 API calls.
> 
> > You are changing the default mode from digital to radio; does that affect the use of the hybrid tuner assemblies.  
> 
> Where are you seeing such change? I just removed T_DIGITAL_TV mode mask, as this is
> unused. On all places at boards, they use a mask with (T_ANALOG_TV | T_DIGITAL_TV).
> The same mask is used at tuner-core. This patch is basically:
> 	s/"T_ANALOG_TV | T_DIGITAL_TV"/T_ANALOG_TV/g
> 
> Also, the default mode is almost meaningless. On all VIDIOC calls that touch at tuner
> (get/set frequency, get/set tuner), the type of the tuner is passed as a parameter.
> So, no default mode is assumed. At digital mode, on all cases, the set_params callback
> will pass the bandwidth, digital tv standard and the frequency to set. The digital TV
> logic inside the tuner will handle it directly, via a direct I2C attach function, not
> using tuner-core.

OK.  

Regards,
Andy

> So, this patch should cause no functional change.
> 
> Cheers,
> Mauro

Re: [PATCH 1/4] [media] tuner-core: remove usage of DIGITAL_TV

[Mauro Carvalho Chehab]

Em 16-02-2011 10:55, Andy Walls escreveu:
> On Tue, 2011-02-15 at 17:48 -0200, Mauro Carvalho Chehab wrote:
>> Em 15-02-2011 15:25, Andy Walls escreveu:
>>> Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
>>>
>>>> tuner-core has no business to do with digital TV. So, don't use
>>>> T_DIGITAL_TV on it, as it has no code to distinguish between
>>>> them, and nobody fills T_DIGITAL_TV right.
>>>>
>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>>>>
> 
>>>> diff --git a/drivers/media/video/tuner-core.c
>>>> b/drivers/media/video/tuner-core.c
>>>> index dcf03fa..5e1437c 100644
>>>> --- a/drivers/media/video/tuner-core.c
>>>> +++ b/drivers/media/video/tuner-core.c
> 
>>>> @@ -596,7 +595,7 @@ static int tuner_probe(struct i2c_client *client,
>>>> 	   first found TV tuner. */
>>>> 	tuner_lookup(t->i2c->adapter, &radio, &tv);
>>>> 	if (tv == NULL) {
>>>> -		t->mode_mask = T_ANALOG_TV | T_DIGITAL_TV;
>>>> +		t->mode_mask = T_ANALOG_TV;
>>>> 		if (radio == NULL)
>>>> 			t->mode_mask |= T_RADIO;
>>>> 		tuner_dbg("Setting mode_mask to 0x%02x\n", t->mode_mask);
>>>> @@ -607,18 +606,15 @@ register_client:
>>>> 	/* Sets a default mode */
>>>> 	if (t->mode_mask & T_ANALOG_TV)
>>>> 		t->mode = V4L2_TUNER_ANALOG_TV;
>>>> -	else if (t->mode_mask & T_RADIO)
>>>> -		t->mode = V4L2_TUNER_RADIO;
>>>> 	else
>>>> -		t->mode = V4L2_TUNER_DIGITAL_TV;
>>>> +		t->mode = V4L2_TUNER_RADIO;
>                             ^^^^^^^^^^^^^^^^^^^^^
> Mauro,
> 
> Here's where I saw a default being changed from DIGITAL_TV to RADIO.
> Maybe it doesn't matter?

Currently, there are just two mode_mask's: T_ANALOG_TV and T_RADIO. If it is not one, it is
the other ;)

Well, in a matter of fact, I didn't drop T_DIGITAL_TV yet, just because it is used internally
inside one driver, for its internal usage only:

$ git grep T_DIGITAL_TV include/media
include/media/tuner.h:    T_DIGITAL_TV    = 1 << V4L2_TUNER_DIGITAL_TV,

$ git grep -B1 T_DIGITAL_TV drivers/media/ 
drivers/media/common/tuners/tuner-xc2028.c-       return generic_set_freq(fe, p->frequency,
drivers/media/common/tuners/tuner-xc2028.c:                               T_DIGITAL_TV, type, 0, demod);

As you see, the changes are simple, and the usage is pure internally:

drivers/media/common/tuners/tuner-xc2028.c:static int generic_set_freq(struct dvb_frontend *fe, u32 freq /* in HZ */,
drivers/media/common/tuners/tuner-xc2028.c-                           enum tuner_mode new_mode,
--
drivers/media/common/tuners/tuner-xc2028.c:               return generic_set_freq(fe, (625l * p->frequency) / 10,
drivers/media/common/tuners/tuner-xc2028.c-                               T_RADIO, type, 0, 0);
--
drivers/media/common/tuners/tuner-xc2028.c:       return generic_set_freq(fe, 62500l * p->frequency,
drivers/media/common/tuners/tuner-xc2028.c-                               T_ANALOG_TV, type, p->std, 0);
--
drivers/media/common/tuners/tuner-xc2028.c:       return generic_set_freq(fe, p->frequency,
drivers/media/common/tuners/tuner-xc2028.c-                               T_DIGITAL_TV, type, 0, demod);

I'll write a patch removing the latest usage of it (basically, replacing them by V4L2_TUNER_*).

Of course, tests are more than welcome to be sure that those changes didn't cause any regression.

Cheers,
Mauro

[PATCH 3/4] [media] tuner-core: Rearrange some functions to better document

[Mauro Carvalho Chehab]

Group a few functions together and add/fix comments for each
block of the driver.

This is just a cleanup patch meant to improve driver readability.
No functional changes in this patch.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 2c7fe18..a91a299 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -251,7 +251,7 @@ static struct analog_demod_ops tuner_analog_ops = {
 };
 
 /*
- * Functions to select between radio and TV and tuner probe functions
+ * Functions to select between radio and TV and tuner probe/remove functions
  */
 
 /**
@@ -698,6 +698,75 @@ static int tuner_remove(struct i2c_client *client)
 }
 
 /*
+ * Functions to switch between Radio and TV
+ *
+ * A few cards have a separate I2C tuner for radio. Those routines
+ * take care of switching between TV/Radio mode, filtering only the
+ * commands that apply to the Radio or TV tuner.
+ */
+
+/**
+ * check_mode - Verify if tuner supports the requested mode
+ * @t: a pointer to the module's internal struct_tuner
+ *
+ * This function checks if the tuner is capable of tuning analog TV,
+ * digital TV or radio, depending on what the caller wants. If the
+ * tuner can't support that mode, it returns -EINVAL. Otherwise, it
+ * returns 0.
+ * This function is needed for boards that have a separate tuner for
+ * radio (like devices with tea5767).
+ */
+static inline int check_mode(struct tuner *t, enum v4l2_tuner_type mode)
+{
+	if ((1 << mode & t->mode_mask) == 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * set_mode_freq - Switch tuner to other mode.
+ * @client:	struct i2c_client pointer
+ * @t:		a pointer to the module's internal struct_tuner
+ * @mode:	enum v4l2_type (radio or TV)
+ * @freq:	frequency to set (0 means to use the previous one)
+ *
+ * If tuner doesn't support the needed mode (radio or TV), prints a
+ * debug message and returns -EINVAL, changing its state to standby.
+ * Otherwise, changes the state and sets frequency to the last value, if
+ * the tuner can sleep or if it supports both Radio and TV.
+ */
+static int set_mode_freq(struct i2c_client *client, struct tuner *t,
+			 enum v4l2_tuner_type mode, unsigned int freq)
+{
+	struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
+
+	if (mode != t->mode) {
+		if (check_mode(t, mode) == -EINVAL) {
+			tuner_dbg("Tuner doesn't support mode %d. "
+				  "Putting tuner to sleep\n", mode);
+			t->standby = true;
+			if (analog_ops->standby)
+				analog_ops->standby(&t->fe);
+			return -EINVAL;
+		}
+		t->mode = mode;
+		tuner_dbg("Changing to mode %d\n", mode);
+	}
+	if (t->mode == V4L2_TUNER_RADIO) {
+		if (freq)
+			t->radio_freq = freq;
+		set_radio_freq(client, t->radio_freq);
+	} else {
+		if (freq)
+			t->tv_freq = freq;
+		set_tv_freq(client, t->tv_freq);
+	}
+
+	return 0;
+}
+
+/*
  * Functions that are specific for TV mode
  */
 
@@ -925,66 +994,9 @@ static void set_radio_freq(struct i2c_client *c, unsigned int freq)
 	analog_ops->set_params(&t->fe, &params);
 }
 
-/**
- * check_mode - Verify if tuner supports the requested mode
- * @t: a pointer to the module's internal struct_tuner
- *
- * This function checks if the tuner is capable of tuning analog TV,
- * digital TV or radio, depending on what the caller wants. If the
- * tuner can't support that mode, it returns -EINVAL. Otherwise, it
- * returns 0.
- * This function is needed for boards that have a separate tuner for
- * radio (like devices with tea5767).
+/*
+ * Debug function for reporting tuner status to userspace
  */
-static inline int check_mode(struct tuner *t, enum v4l2_tuner_type mode)
-{
-	if ((1 << mode & t->mode_mask) == 0)
-		return -EINVAL;
-
-	return 0;
-}
-
-/**
- * set_mode_freq - Switch tuner to other mode.
- * @client:	struct i2c_client pointer
- * @t:		a pointer to the module's internal struct_tuner
- * @mode:	enum v4l2_type (radio or TV)
- * @freq:	frequency to set (0 means to use the previous one)
- *
- * If tuner doesn't support the needed mode (radio or TV), prints a
- * debug message and returns -EINVAL, changing internal state to T_STANDBY.
- * Otherwise, changes the state and sets frequency to the last value, if
- * the tuner can sleep or if it supports both Radio and TV.
- */
-static int set_mode_freq(struct i2c_client *client, struct tuner *t,
-			 enum v4l2_tuner_type mode, unsigned int freq)
-{
-	struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
-
-	if (mode != t->mode) {
-		if (check_mode(t, mode) == -EINVAL) {
-			tuner_dbg("Tuner doesn't support mode %d. "
-				  "Putting tuner to sleep\n", mode);
-			t->standby = true;
-			if (analog_ops->standby)
-				analog_ops->standby(&t->fe);
-			return -EINVAL;
-		}
-		t->mode = mode;
-		tuner_dbg("Changing to mode %d\n", mode);
-	}
-	if (t->mode == V4L2_TUNER_RADIO) {
-		if (freq)
-			t->radio_freq = freq;
-		set_radio_freq(client, t->radio_freq);
-	} else {
-		if (freq)
-			t->tv_freq = freq;
-		set_tv_freq(client, t->tv_freq);
-	}
-
-	return 0;
-}
 
 /**
  * tuner_status - Dumps the current tuner status at dmesg
@@ -1040,6 +1052,24 @@ static void tuner_status(struct dvb_frontend *fe)
 			   analog_ops->has_signal(fe));
 }
 
+/*
+ * Function to splicitly change mode to radio. Probably not needed anymore
+ */
+
+static int tuner_s_radio(struct v4l2_subdev *sd)
+{
+	struct tuner *t = to_tuner(sd);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	if (set_mode_freq(client, t, V4L2_TUNER_RADIO, 0) == -EINVAL)
+		return 0;
+	return 0;
+}
+
+/*
+ * Tuner callbacks to handle userspace ioctl's
+ */
+
 /**
  * tuner_s_power - controls the power state of the tuner
  * @sd: pointer to struct v4l2_subdev
@@ -1061,24 +1091,6 @@ static int tuner_s_power(struct v4l2_subdev *sd, int on)
 	return 0;
 }
 
-/*
- * Function to splicitly change mode to radio. Probably not needed anymore
- */
-
-static int tuner_s_radio(struct v4l2_subdev *sd)
-{
-	struct tuner *t = to_tuner(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-	if (set_mode_freq(client, t, V4L2_TUNER_RADIO, 0) == -EINVAL)
-		return 0;
-	return 0;
-}
-
-/*
- * Tuner callbacks to handle userspace ioctl's
- */
-
 static int tuner_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
 	struct tuner *t = to_tuner(sd);
-- 
1.7.1

[PATCH 2/4] [media] tuner-core: Improve function documentation

[Mauro Carvalho Chehab]

This driver is complex, and used by everyone. Better to have it
properly documented.

No functional changes are done in this patch.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 5e1437c..2c7fe18 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -1,7 +1,17 @@
 /*
- *
  * i2c tv tuner chip device driver
  * core core, i.e. kernel interfaces, registering and so on
+ *
+ * Copyright(c) by Ralph Metzler, Gerd Knorr, Gunther Mayer
+ *
+ * Copyright(c) 2005-2011 by Mauro Carvalho Chehab
+ *	- Added support for a separate Radio tuner
+ *	- Major rework and cleanups at the code
+ *
+ * This driver supports many devices and the idea is to let the driver
+ * detect which device is present. So rather than listing all supported
+ * devices here, we pretend to support a single, fake device type that will
+ * handle both radio and analog TV tuning.
  */
 
 #include <linux/module.h>
@@ -67,6 +77,7 @@ module_param_string(ntsc, ntsc, sizeof(ntsc), 0644);
  */
 
 static LIST_HEAD(tuner_list);
+static const struct v4l2_subdev_ops tuner_ops;
 
 /*
  * Debug macros
@@ -125,6 +136,13 @@ struct tuner {
 };
 
 /*
+ * Function prototypes
+ */
+
+static void set_tv_freq(struct i2c_client *c, unsigned int freq);
+static void set_radio_freq(struct i2c_client *c, unsigned int freq);
+
+/*
  * tuner attach/detach logic
  */
 
@@ -233,13 +251,24 @@ static struct analog_demod_ops tuner_analog_ops = {
 };
 
 /*
- * Functions that are common to both TV and radio
+ * Functions to select between radio and TV and tuner probe functions
  */
 
-static void set_tv_freq(struct i2c_client *c, unsigned int freq);
-static void set_radio_freq(struct i2c_client *c, unsigned int freq);
-static const struct v4l2_subdev_ops tuner_ops;
-
+/**
+ * set_type - Sets the tuner type for a given device
+ *
+ * @c:			i2c_client descriptoy
+ * @type:		type of the tuner (e. g. tuner number)
+ * @new_mode_mask:	Indicates if tuner supports TV and/or Radio
+ * @new_config:		an optional parameter ranging from 0-255 used by
+			a few tuners to adjust an internal parameter,
+			like LNA mode
+ * @tuner_callback:	an optional function to be called when switching
+ *			to analog mode
+ *
+ * This function applys the tuner config to tuner specified
+ * by tun_setup structure. It contains several per-tuner initialization "magic"
+ */
 static void set_type(struct i2c_client *c, unsigned int type,
 		     unsigned int new_mode_mask, unsigned int new_config,
 		     int (*tuner_callback) (void *dev, int component, int cmd, int arg))
@@ -452,6 +481,15 @@ static int tuner_s_type_addr(struct v4l2_subdev *sd,
 	return 0;
 }
 
+/**
+ * tuner_s_config - Sets tuner configuration
+ *
+ * @sd:		subdev descriptor
+ * @cfg:	tuner configuration
+ *
+ * Calls tuner set_config() private function to set some tuner-internal
+ * parameters
+ */
 static int tuner_s_config(struct v4l2_subdev *sd,
 			  const struct v4l2_priv_tun_config *cfg)
 {
@@ -470,10 +508,20 @@ static int tuner_s_config(struct v4l2_subdev *sd,
 	return 0;
 }
 
-/* Search for existing radio and/or TV tuners on the given I2C adapter.
-   Note that when this function is called from tuner_probe you can be
-   certain no other devices will be added/deleted at the same time, I2C
-   core protects against that. */
+/**
+ * tuner_lookup - Seek for tuner adapters
+ *
+ * @adap:	i2c_adapter struct
+ * @radio:	pointer to be filled if the adapter is radio
+ * @tv:		pointer to be filled if the adapter is TV
+ *
+ * Search for existing radio and/or TV tuners on the given I2C adapter,
+ * discarding demod-only adapters (tda9887).
+ *
+ * Note that when this function is called from tuner_probe you can be
+ * certain no other devices will be added/deleted at the same time, I2C
+ * core protects against that.
+ */
 static void tuner_lookup(struct i2c_adapter *adap,
 		struct tuner **radio, struct tuner **tv)
 {
@@ -502,8 +550,20 @@ static void tuner_lookup(struct i2c_adapter *adap,
 	}
 }
 
-/* During client attach, set_type is called by adapter's attach_inform callback.
-   set_type must then be completed by tuner_probe.
+/**
+ *tuner_probe - Probes the existing tuners on an I2C bus
+ *
+ * @client:	i2c_client descriptor
+ * @id:		not used
+ *
+ * This routine probes for tuners at the expected I2C addresses. On most
+ * cases, if a device answers to a given I2C address, it assumes that the
+ * device is a tuner. On a few cases, however, an additional logic is needed
+ * to double check if the device is really a tuner, or to identify the tuner
+ * type, like on tea5767/5761 devices.
+ *
+ * During client attach, set_type is called by adapter's attach_inform callback.
+ * set_type must then be completed by tuner_probe.
  */
 static int tuner_probe(struct i2c_client *client,
 		       const struct i2c_device_id *id)
@@ -618,6 +678,12 @@ register_client:
 	return 0;
 }
 
+/**
+ * tuner_remove - detaches a tuner
+ *
+ * @client:	i2c_client descriptor
+ */
+
 static int tuner_remove(struct i2c_client *client)
 {
 	struct tuner *t = to_tuner(i2c_get_clientdata(client));
@@ -635,7 +701,12 @@ static int tuner_remove(struct i2c_client *client)
  * Functions that are specific for TV mode
  */
 
-/* Set tuner frequency,  freq in Units of 62.5kHz = 1/16MHz */
+/**
+ * set_tv_freq - Set tuner frequency,  freq in Units of 62.5 kHz = 1/16MHz
+ *
+ * @c:	i2c_client descriptor
+ * @freq: frequency
+ */
 static void set_tv_freq(struct i2c_client *c, unsigned int freq)
 {
 	struct tuner *t = to_tuner(i2c_get_clientdata(c));
@@ -675,7 +746,19 @@ static void set_tv_freq(struct i2c_client *c, unsigned int freq)
 	analog_ops->set_params(&t->fe, &params);
 }
 
-/* get more precise norm info from insmod option */
+/**
+ * tuner_fixup_std - force a given video standard variant
+ *
+ * @t:	tuner internal struct
+ *
+ * A few devices or drivers have problem to detect some standard variations.
+ * On other operational systems, the drivers generally have a per-country
+ * code, and some logic to apply per-country hacks. V4L2 API doesn't provide
+ * such hacks. Instead, it relies on a proper video standard selection from
+ * the userspace application. However, as some apps are buggy, not allowing
+ * to distinguish all video standard variations, a modprobe parameter can
+ * be used to force a video standard match.
+ */
 static int tuner_fixup_std(struct tuner *t)
 {
 	if ((t->std & V4L2_STD_PAL) == V4L2_STD_PAL) {
@@ -797,6 +880,12 @@ static int tuner_fixup_std(struct tuner *t)
  * Functions that are specific for Radio mode
  */
 
+/**
+ * set_radio_freq - Set tuner frequency,  freq in Units of 62.5 Hz  = 1/16kHz
+ *
+ * @c:	i2c_client descriptor
+ * @freq: frequency
+ */
 static void set_radio_freq(struct i2c_client *c, unsigned int freq)
 {
 	struct tuner *t = to_tuner(i2c_get_clientdata(c));
@@ -972,7 +1061,9 @@ static int tuner_s_power(struct v4l2_subdev *sd, int on)
 	return 0;
 }
 
-/* ---------------------------------------------------------------------- */
+/*
+ * Function to splicitly change mode to radio. Probably not needed anymore
+ */
 
 static int tuner_s_radio(struct v4l2_subdev *sd)
 {
@@ -984,9 +1075,10 @@ static int tuner_s_radio(struct v4l2_subdev *sd)
 	return 0;
 }
 
-/* --- v4l ioctls --- */
-/* take care: bttv does userspace copying, we'll get a
-   kernel pointer here... */
+/*
+ * Tuner callbacks to handle userspace ioctl's
+ */
+
 static int tuner_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
 	struct tuner *t = to_tuner(sd);
@@ -1137,7 +1229,9 @@ static int tuner_command(struct i2c_client *client, unsigned cmd, void *arg)
 	return -ENOIOCTLCMD;
 }
 
-/* ----------------------------------------------------------------------- */
+/*
+ * Callback structs
+ */
 
 static const struct v4l2_subdev_core_ops tuner_core_ops = {
 	.log_status = tuner_log_status,
@@ -1160,11 +1254,10 @@ static const struct v4l2_subdev_ops tuner_ops = {
 	.tuner = &tuner_tuner_ops,
 };
 
-/* ----------------------------------------------------------------------- */
+/*
+ * I2C structs and module init functions
+ */
 
-/* This driver supports many devices and the idea is to let the driver
-   detect which device is present. So rather than listing all supported
-   devices here, we pretend to support a single, fake device type. */
 static const struct i2c_device_id tuner_id[] = {
 	{ "tuner", }, /* autodetect */
 	{ }
-- 
1.7.1

[PATCH 4/4] [media] tuner-core: Don't touch at standby during tuner_lookup

[Mauro Carvalho Chehab]

It makes no sense that tuner_lookup would touch at the standby
state. Remove it.

Thanks-to: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index a91a299..9363ed9 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -538,7 +538,6 @@ static void tuner_lookup(struct i2c_adapter *adap,
 			continue;
 
 		mode_mask = pos->mode_mask;
-		pos->standby = 1;
 		if (*radio == NULL && mode_mask == T_RADIO)
 			*radio = pos;
 		/* Note: currently TDA9887 is the only demod-only
-- 
1.7.1