2007-10-22 02:47:45

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] fix CONFIG_TUNER_TEA5761=m

This patch fixes CONFIG_TUNER_TEA5761=m broken by
commit ca805d57cf5ea7482ed3da28653f30621249ee45.

Signed-off-by: Adrian Bunk <[email protected]>

---

drivers/media/video/tuner-core.c | 6 ------
1 file changed, 6 deletions(-)

26336c30e76c37bda368a24b8b12978388a18cf3
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 9484308..1795b78 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -30,9 +30,7 @@

/* standard i2c insmod options */
static unsigned short normal_i2c[] = {
-#ifdef CONFIG_TUNER_TEA5761
0x10,
-#endif
0x42, 0x43, 0x4a, 0x4b, /* tda8290 */
0x60, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67,
0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f,
@@ -292,7 +290,6 @@ static void set_type(struct i2c_client *c, unsigned int type,
}
t->mode_mask = T_RADIO;
break;
-#ifdef CONFIG_TUNER_TEA5761
case TUNER_TEA5761:
if (tea5761_attach(&t->fe, t->i2c.adapter, t->i2c.addr) == NULL) {
t->type = TUNER_ABSENT;
@@ -301,7 +298,6 @@ static void set_type(struct i2c_client *c, unsigned int type,
}
t->mode_mask = T_RADIO;
break;
-#endif
case TUNER_PHILIPS_FMD1216ME_MK3:
buffer[0] = 0x0b;
buffer[1] = 0xdc;
@@ -594,7 +590,6 @@ static int tuner_attach(struct i2c_adapter *adap, int addr, int kind)
/* autodetection code based on the i2c addr */
if (!no_autodetect) {
switch (addr) {
-#ifdef CONFIG_TUNER_TEA5761
case 0x10:
if (tea5761_autodetection(t->i2c.adapter, t->i2c.addr) != EINVAL) {
t->type = TUNER_TEA5761;
@@ -606,7 +601,6 @@ static int tuner_attach(struct i2c_adapter *adap, int addr, int kind)
goto register_client;
}
break;
-#endif
case 0x42:
case 0x43:
case 0x4a:


2007-10-22 03:33:30

by Michael Ira Krufky

[permalink] [raw]
Subject: Re: [2.6 patch] fix CONFIG_TUNER_TEA5761=m

Adrian Bunk wrote:
> This patch fixes CONFIG_TUNER_TEA5761=m broken by
> commit ca805d57cf5ea7482ed3da28653f30621249ee45.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> drivers/media/video/tuner-core.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> 26336c30e76c37bda368a24b8b12978388a18cf3
> diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
> index 9484308..1795b78 100644
> --- a/drivers/media/video/tuner-core.c
> +++ b/drivers/media/video/tuner-core.c
> @@ -30,9 +30,7 @@
>
> /* standard i2c insmod options */
> static unsigned short normal_i2c[] = {
> -#ifdef CONFIG_TUNER_TEA5761
> 0x10,
> -#endif
>
^^^ I believe that we want to keep these #ifdef's above... The removals
below are fine.


> 0x42, 0x43, 0x4a, 0x4b, /* tda8290 */
> 0x60, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67,
> 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f,
> @@ -292,7 +290,6 @@ static void set_type(struct i2c_client *c, unsigned int type,
> }
> t->mode_mask = T_RADIO;
> break;
> -#ifdef CONFIG_TUNER_TEA5761
> case TUNER_TEA5761:
> if (tea5761_attach(&t->fe, t->i2c.adapter, t->i2c.addr) == NULL) {
> t->type = TUNER_ABSENT;
> @@ -301,7 +298,6 @@ static void set_type(struct i2c_client *c, unsigned int type,
> }
> t->mode_mask = T_RADIO;
> break;
> -#endif
> case TUNER_PHILIPS_FMD1216ME_MK3:
> buffer[0] = 0x0b;
> buffer[1] = 0xdc;
> @@ -594,7 +590,6 @@ static int tuner_attach(struct i2c_adapter *adap, int addr, int kind)
> /* autodetection code based on the i2c addr */
> if (!no_autodetect) {
> switch (addr) {
> -#ifdef CONFIG_TUNER_TEA5761
> case 0x10:
> if (tea5761_autodetection(t->i2c.adapter, t->i2c.addr) != EINVAL) {
> t->type = TUNER_TEA5761;
> @@ -606,7 +601,6 @@ static int tuner_attach(struct i2c_adapter *adap, int addr, int kind)
> goto register_client;
> }
> break;
> -#endif
> case 0x42:
> case 0x43:
> case 0x4a:
>
>
Regards,

Mike Krufky

2007-10-22 05:30:00

by Trent Piepho

[permalink] [raw]
Subject: Re: [v4l-dvb-maintainer] [2.6 patch] fix CONFIG_TUNER_TEA5761=m

On Sun, 21 Oct 2007, Michael Krufky wrote:
> Adrian Bunk wrote:
> > This patch fixes CONFIG_TUNER_TEA5761=m broken by
> > commit ca805d57cf5ea7482ed3da28653f30621249ee45.
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > ---
> >
> > drivers/media/video/tuner-core.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > 26336c30e76c37bda368a24b8b12978388a18cf3
> > diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
> > index 9484308..1795b78 100644
> > --- a/drivers/media/video/tuner-core.c
> > +++ b/drivers/media/video/tuner-core.c
> > @@ -30,9 +30,7 @@
> >
> > /* standard i2c insmod options */
> > static unsigned short normal_i2c[] = {
> > -#ifdef CONFIG_TUNER_TEA5761
> > 0x10,
> > -#endif
> >
> ^^^ I believe that we want to keep these #ifdef's above... The removals
> below are fine.

Won't leaving in the call to tea5761_autodetection() when tea5761 is not
compiled in result in an undefined symbol error?

I think what we want to do put in the typical if(TEA5761 >= TUNER) include tea5761
code in tuner logic that the rest of the selectable sub-driver use.

#if defined(CONFIG_TUNER_TEA5761) || (defined(CONFIG_TUNER_TEA5761_MODULE) && defined(MODULE))

Of course compiling TEA5761 lower than TUNER doesn't make any sense now
since the only thing that uses tea5761 is the tuner. But it's still
possible, you just end up with a tuner that doesn't support tea5761 and a
tea5761 module that isn't needed by anything.

>
>
> > 0x42, 0x43, 0x4a, 0x4b, /* tda8290 */
> > 0x60, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67,
> > 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f,
> > @@ -292,7 +290,6 @@ static void set_type(struct i2c_client *c, unsigned int type,
> > }
> > t->mode_mask = T_RADIO;
> > break;
> > -#ifdef CONFIG_TUNER_TEA5761
> > case TUNER_TEA5761:
> > if (tea5761_attach(&t->fe, t->i2c.adapter, t->i2c.addr) == NULL) {
> > t->type = TUNER_ABSENT;
> > @@ -301,7 +298,6 @@ static void set_type(struct i2c_client *c, unsigned int type,
> > }
> > t->mode_mask = T_RADIO;
> > break;
> > -#endif
> > case TUNER_PHILIPS_FMD1216ME_MK3:
> > buffer[0] = 0x0b;
> > buffer[1] = 0xdc;
> > @@ -594,7 +590,6 @@ static int tuner_attach(struct i2c_adapter *adap, int addr, int kind)
> > /* autodetection code based on the i2c addr */
> > if (!no_autodetect) {
> > switch (addr) {
> > -#ifdef CONFIG_TUNER_TEA5761
> > case 0x10:
> > if (tea5761_autodetection(t->i2c.adapter, t->i2c.addr) != EINVAL) {
> > t->type = TUNER_TEA5761;
> > @@ -606,7 +601,6 @@ static int tuner_attach(struct i2c_adapter *adap, int addr, int kind)
> > goto register_client;
> > }
> > break;
> > -#endif
> > case 0x42:
> > case 0x43:
> > case 0x4a:
> >
> >
> Regards,
>
> Mike Krufky
>
> _______________________________________________
> v4l-dvb-maintainer mailing list
> [email protected]
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
>

2007-10-22 11:41:32

by Michael Ira Krufky

[permalink] [raw]
Subject: Re: [v4l-dvb-maintainer] [2.6 patch] fix CONFIG_TUNER_TEA5761=m

Trent Piepho wrote:
> On Sun, 21 Oct 2007, Michael Krufky wrote:
>
>> Adrian Bunk wrote:
>>
>>> This patch fixes CONFIG_TUNER_TEA5761=m broken by
>>> commit ca805d57cf5ea7482ed3da28653f30621249ee45.
>>>
>>> Signed-off-by: Adrian Bunk <[email protected]>
>>>
>>> ---
>>>
>>> drivers/media/video/tuner-core.c | 6 ------
>>> 1 file changed, 6 deletions(-)
>>>
>>> 26336c30e76c37bda368a24b8b12978388a18cf3
>>> diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
>>> index 9484308..1795b78 100644
>>> --- a/drivers/media/video/tuner-core.c
>>> +++ b/drivers/media/video/tuner-core.c
>>> @@ -30,9 +30,7 @@
>>>
>>> /* standard i2c insmod options */
>>> static unsigned short normal_i2c[] = {
>>> -#ifdef CONFIG_TUNER_TEA5761
>>> 0x10,
>>> -#endif
>>>
>>>
>> ^^^ I believe that we want to keep these #ifdef's above... The removals
>> below are fine.
>>
>
> Won't leaving in the call to tea5761_autodetection() when tea5761 is not
> compiled in result in an undefined symbol error?
>
It shouldn't -- See tea5761.h ... If the module is not selected, then a
stub will report that the driver was disabled by Kconfig.
> I think what we want to do put in the typical if(TEA5761 >= TUNER) include tea5761
> code in tuner logic that the rest of the selectable sub-driver use.
>
> #if defined(CONFIG_TUNER_TEA5761) || (defined(CONFIG_TUNER_TEA5761_MODULE) && defined(MODULE))
>
This is what I have in my tree here... ^^^ Isn't this already upstream
by now?

-Mike

2007-10-22 12:15:55

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] fix CONFIG_TUNER_TEA5761=m

On Sun, Oct 21, 2007 at 11:33:05PM -0400, Michael Krufky wrote:
> Adrian Bunk wrote:
> > This patch fixes CONFIG_TUNER_TEA5761=m broken by
> > commit ca805d57cf5ea7482ed3da28653f30621249ee45.
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > ---
> >
> > drivers/media/video/tuner-core.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > 26336c30e76c37bda368a24b8b12978388a18cf3
> > diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
> > index 9484308..1795b78 100644
> > --- a/drivers/media/video/tuner-core.c
> > +++ b/drivers/media/video/tuner-core.c
> > @@ -30,9 +30,7 @@
> >
> > /* standard i2c insmod options */
> > static unsigned short normal_i2c[] = {
> > -#ifdef CONFIG_TUNER_TEA5761
> > 0x10,
> > -#endif
> >
> ^^^ I believe that we want to keep these #ifdef's above... The removals
> below are fine.
>...

If it's required, then we need
#if defined(CONFIG_TUNER_TEA5761) || (defined(CONFIG_TUNER_TEA5761_MODULE) && defined(MODULE))
here for getting the modular case working.

But at least at first glance it seems the code already tries to handle
the cases when tea5761_{attach,autodetection}() fail?

> Regards,
>
> Mike Krufky

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-10-22 20:31:47

by Trent Piepho

[permalink] [raw]
Subject: Re: [v4l-dvb-maintainer] [2.6 patch] fix CONFIG_TUNER_TEA5761=m

On Mon, 22 Oct 2007, Michael Krufky wrote:
> Trent Piepho wrote:
> >>> +++ b/drivers/media/video/tuner-core.c
> >>> @@ -30,9 +30,7 @@
> >>>
> >>> /* standard i2c insmod options */
> >>> static unsigned short normal_i2c[] = {
> >>> -#ifdef CONFIG_TUNER_TEA5761
> >>> 0x10,
> >>> -#endif
> >>>
> >>>
> >> ^^^ I believe that we want to keep these #ifdef's above... The removals
> >> below are fine.
> >>
> >
> > Won't leaving in the call to tea5761_autodetection() when tea5761 is not
> > compiled in result in an undefined symbol error?
> >
> It shouldn't -- See tea5761.h ... If the module is not selected, then a
> stub will report that the driver was disabled by Kconfig.

You're right, there's no need to have the tuner-core code remove the tea5761
function calls as the stubs will take care of it.

We could delete the #if/#endif around the 0x10 address to scan, or keep it.
What's better, to not scan the address if it's not needed, or to scan it and
then produce an error message if something is found there?

But if it stays, it should look like this:

> > #if defined(CONFIG_TUNER_TEA5761) || (defined(CONFIG_TUNER_TEA5761_MODULE) && defined(MODULE))

i.e., only scan 0x10 if there is callable tea5761 code to handle it.