| Message ID | 1319976530.14409.17.camel@x61.thuisdomein (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1RKUCW-0008Iy-6v; Sun, 30 Oct 2011 13:09:44 +0100 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-1) with esmtp id 1RKUCV-0000R5-KL; Sun, 30 Oct 2011 13:09:19 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933988Ab1J3MI5 (ORCPT <rfc822; patchwork@linuxtv.org> + 3 others); Sun, 30 Oct 2011 08:08:57 -0400 Received: from smtp-out0.tiscali.nl ([195.241.79.175]:37006 "EHLO smtp-out0.tiscali.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933965Ab1J3MI5 (ORCPT <rfc822;linux-media@vger.kernel.org>); Sun, 30 Oct 2011 08:08:57 -0400 Received: from [212.123.169.34] (helo=[192.168.1.103]) by smtp-out0.tiscali.nl with esmtp (Exim) (envelope-from <pebolle@tiscali.nl>) id 1RKUC8-0004YI-Br; Sun, 30 Oct 2011 13:08:56 +0100 Subject: [PATCH] [RESEND] media: tea5764: reconcile Kconfig symbol and macro From: Paul Bolle <pebolle@tiscali.nl> To: Mauro Carvalho Chehab <mchehab@infradead.org> Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Date: Sun, 30 Oct 2011 13:08:50 +0100 Message-ID: <1319976530.14409.17.camel@x61.thuisdomein> Mime-Version: 1.0 X-Mailer: Evolution 2.32.3 (2.32.3-1.fc14) Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2011.10.30.115717 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, MSGID_ADDED_BY_MTA 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1200_1299 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __URI_NO_WWW 0, __URI_NS ' X-LSpam-Score: -4.2 (----) X-LSpam-Report: No, score=-4.2 required=5.0 tests=BAYES_00=-1.9, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_MED=-2.3 autolearn=ham |
Commit Message
Paul Bolle
Oct. 30, 2011, 12:08 p.m. UTC
The Kconfig symbol RADIO_TEA5764_XTAL is unused. The code does use a
RADIO_TEA5764_XTAL macro, but does that rather peculiar. But there seems
to be a way to keep both. (The easiest way out would be to rip out both
the Kconfig symbol and the macro.)
Note there's also a module parameter 'use_xtal' to influence all this.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
I didn't dare to submit this a trivial patch. This is still untested. By
the way, is xtal a common abbreviation of crystal?
drivers/media/radio/radio-tea5764.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
Comments
On 10/30/11 05:08, Paul Bolle wrote: > The Kconfig symbol RADIO_TEA5764_XTAL is unused. The code does use a > RADIO_TEA5764_XTAL macro, but does that rather peculiar. But there seems > to be a way to keep both. (The easiest way out would be to rip out both > the Kconfig symbol and the macro.) > > Note there's also a module parameter 'use_xtal' to influence all this. It's curious that the module parameter is only available when the driver is builtin (=y) but not built as a loadable module (=m): config RADIO_TEA5764_XTAL bool "TEA5764 crystal reference" depends on RADIO_TEA5764=y default y > > Signed-off-by: Paul Bolle <pebolle@tiscali.nl> > --- > I didn't dare to submit this a trivial patch. This is still untested. By > the way, is xtal a common abbreviation of crystal? Yes, it is. Acked-by: Randy Dunlap <rdunlap@xenotime.net> Thanks. > drivers/media/radio/radio-tea5764.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/radio/radio-tea5764.c b/drivers/media/radio/radio-tea5764.c > index 95ddcc4..db20904 100644 > --- a/drivers/media/radio/radio-tea5764.c > +++ b/drivers/media/radio/radio-tea5764.c > @@ -128,8 +128,10 @@ struct tea5764_write_regs { > u16 rdsbbl; /* PAUSEDET & RDSBBL */ > } __attribute__ ((packed)); > > -#ifndef RADIO_TEA5764_XTAL > +#ifdef CONFIG_RADIO_TEA5764_XTAL > #define RADIO_TEA5764_XTAL 1 > +#else > +#define RADIO_TEA5764_XTAL 0 > #endif > > static int radio_nr = -1;
On Sun, 2011-10-30 at 09:43 -0700, Randy Dunlap wrote: > On 10/30/11 05:08, Paul Bolle wrote: > > The Kconfig symbol RADIO_TEA5764_XTAL is unused. The code does use a > > RADIO_TEA5764_XTAL macro, but does that rather peculiar. But there seems > > to be a way to keep both. (The easiest way out would be to rip out both > > the Kconfig symbol and the macro.) > > > > Note there's also a module parameter 'use_xtal' to influence all this. > > It's curious that the module parameter is only available when the driver > is builtin (=y) but not built as a loadable module (=m): As far as I can see the module parameter is available in both cases but defaults to different values when builtin and when loadable. > config RADIO_TEA5764_XTAL > bool "TEA5764 crystal reference" > depends on RADIO_TEA5764=y > default y 0) I've noticed similar dependencies (while doing some other Kconfig related clean up) with a number of other config entries in that same Kconfig file: $ git grep -n "depends on.*=y" drivers/media/radio/ drivers/media/radio/Kconfig:60: depends on RADIO_RTRACK=y drivers/media/radio/Kconfig:83: depends on RADIO_RTRACK2=y drivers/media/radio/Kconfig:106: depends on RADIO_AZTECH=y drivers/media/radio/Kconfig:135: depends on RADIO_GEMTEK=y drivers/media/radio/Kconfig:147: depends on RADIO_GEMTEK=y drivers/media/radio/Kconfig:239: depends on RADIO_TERRATEC=y drivers/media/radio/Kconfig:257: depends on RADIO_TRUST=y drivers/media/radio/Kconfig:280: depends on RADIO_TYPHOON=y drivers/media/radio/Kconfig:287: depends on RADIO_TYPHOON=y drivers/media/radio/Kconfig:314: depends on RADIO_ZOLTRIX=y drivers/media/radio/Kconfig:385: depends on RADIO_TEA5764=y 1) It seems the logic behind those config symbols is mostly like this: - if the driver for a radio is builtin: default some setting for that radio to a sane value, but allow overriding of that setting on the kernel commandline (through a module parameter) - if the driver for a radio is a module: default that same setting to something invalid and _force_ the use of module parameters to get a sane value This logic isn't implemented flawless but it does look to me that this is intentional. 2) I'm not sure why things are done that way. Why can't builtin drivers and loadable drivers default to identical values? But perhaps I'm just misunderstanding the code. Paul Bolle -- 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
On 10/30/11 10:12, Paul Bolle wrote: > On Sun, 2011-10-30 at 09:43 -0700, Randy Dunlap wrote: >> On 10/30/11 05:08, Paul Bolle wrote: >>> The Kconfig symbol RADIO_TEA5764_XTAL is unused. The code does use a >>> RADIO_TEA5764_XTAL macro, but does that rather peculiar. But there seems >>> to be a way to keep both. (The easiest way out would be to rip out both >>> the Kconfig symbol and the macro.) >>> >>> Note there's also a module parameter 'use_xtal' to influence all this. >> >> It's curious that the module parameter is only available when the driver >> is builtin (=y) but not built as a loadable module (=m): > > As far as I can see the module parameter is available in both cases but > defaults to different values when builtin and when loadable. Ah, you are correct. Thanks. >> config RADIO_TEA5764_XTAL >> bool "TEA5764 crystal reference" >> depends on RADIO_TEA5764=y >> default y > > 0) I've noticed similar dependencies (while doing some other Kconfig > related clean up) with a number of other config entries in that same > Kconfig file: > $ git grep -n "depends on.*=y" drivers/media/radio/ > drivers/media/radio/Kconfig:60: depends on RADIO_RTRACK=y > drivers/media/radio/Kconfig:83: depends on RADIO_RTRACK2=y > drivers/media/radio/Kconfig:106: depends on RADIO_AZTECH=y > drivers/media/radio/Kconfig:135: depends on RADIO_GEMTEK=y > drivers/media/radio/Kconfig:147: depends on RADIO_GEMTEK=y > drivers/media/radio/Kconfig:239: depends on RADIO_TERRATEC=y > drivers/media/radio/Kconfig:257: depends on RADIO_TRUST=y > drivers/media/radio/Kconfig:280: depends on RADIO_TYPHOON=y > drivers/media/radio/Kconfig:287: depends on RADIO_TYPHOON=y > drivers/media/radio/Kconfig:314: depends on RADIO_ZOLTRIX=y > drivers/media/radio/Kconfig:385: depends on RADIO_TEA5764=y > > 1) It seems the logic behind those config symbols is mostly like this: > - if the driver for a radio is builtin: default some setting for that > radio to a sane value, but allow overriding of that setting on the > kernel commandline (through a module parameter) > - if the driver for a radio is a module: default that same setting to > something invalid and _force_ the use of module parameters to get a > sane value > > This logic isn't implemented flawless but it does look to me that this > is intentional. > > 2) I'm not sure why things are done that way. Why can't builtin drivers > and loadable drivers default to identical values? But perhaps I'm just > misunderstanding the code. They could default to identical values. Maybe someone thinks that it's more difficult to pass parameters to builtin drivers so they just try to use some sane defaults for them instead, whereas it's easy (easier) to pass parameters to loadable modules. ??
On Sun, 2011-10-30 at 10:24 -0700, Randy Dunlap wrote: > On 10/30/11 10:12, Paul Bolle wrote: > > 2) I'm not sure why things are done that way. Why can't builtin drivers > > and loadable drivers default to identical values? But perhaps I'm just > > misunderstanding the code. > > They could default to identical values. That would make the cleaning up I'm trying to do now somewhat easier. It would allow to simplify the drivers a bit too. > Maybe someone thinks that > it's more difficult to pass parameters to builtin drivers so they > just try to use some sane defaults for them instead, whereas it's > easy (easier) to pass parameters to loadable modules. ?? Perhaps Mauro or the people at linux-media know the reasoning here. Or they can show us that I didn't parse the code correctly, of course. Paul Bolle -- 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
Em 30-10-2011 15:31, Paul Bolle escreveu: > On Sun, 2011-10-30 at 10:24 -0700, Randy Dunlap wrote: >> On 10/30/11 10:12, Paul Bolle wrote: >>> 2) I'm not sure why things are done that way. Why can't builtin drivers >>> and loadable drivers default to identical values? But perhaps I'm just >>> misunderstanding the code. >> >> They could default to identical values. > > That would make the cleaning up I'm trying to do now somewhat easier. It > would allow to simplify the drivers a bit too. > >> Maybe someone thinks that >> it's more difficult to pass parameters to builtin drivers so they >> just try to use some sane defaults for them instead, whereas it's >> easy (easier) to pass parameters to loadable modules. ?? > > Perhaps Mauro or the people at linux-media know the reasoning here. Or > they can show us that I didn't parse the code correctly, of course. I can't remember the dirty details about this driver, sorry. The first patch on it might shed some light: commit 46a60cfef581307d8273919182ae939d44ff7cca Author: Fabio Belavenuto <belavenuto@gmail.com> Date: Tue Dec 30 19:27:09 2008 -0300 V4L/DVB (10155): Add TEA5764 radio driver Add support for radio driver TEA5764 from NXP. This chip is connected in pxa I2C bus in EZX phones from Motorola, the chip is used in phone model A1200. This driver is for OpenEZX project (www.openezx.org) Tested with A1200 phone, openezx kernel and fm-tools [mchehab@redhat.com: Fixed CodingStyle and solved some merge conflicts] Signed-off-by: Fabio Belavenuto <belavenuto@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> From the above, I _suspect_ that the default (whatever it is) is due to the Motorola A1200 phone. Not sure if it is compiled as module or as builtin at OpenEZX. > > > Paul Bolle > > -- > 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 -- 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
diff --git a/drivers/media/radio/radio-tea5764.c b/drivers/media/radio/radio-tea5764.c index 95ddcc4..db20904 100644 --- a/drivers/media/radio/radio-tea5764.c +++ b/drivers/media/radio/radio-tea5764.c @@ -128,8 +128,10 @@ struct tea5764_write_regs { u16 rdsbbl; /* PAUSEDET & RDSBBL */ } __attribute__ ((packed)); -#ifndef RADIO_TEA5764_XTAL +#ifdef CONFIG_RADIO_TEA5764_XTAL #define RADIO_TEA5764_XTAL 1 +#else +#define RADIO_TEA5764_XTAL 0 #endif static int radio_nr = -1;