| Message ID | 5023A11C.50502@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, My bad, sorry about this. Mauro's patch looks good. An alternative fix would be to #ifdefify the led code in the drivers themselves. Regards, Hans On 08/09/2012 01:38 PM, Mauro Carvalho Chehab wrote: > Em 08-08-2012 19:28, David Rientjes escreveu: >> On Tue, 31 Jul 2012, Mauro Carvalho Chehab wrote: >> >>> [media] radio-shark: New driver for the Griffin radioSHARK USB radio receiver >> >> This one gives me a build warning if CONFIG_LEDS_CLASS is disabled: >> >> ERROR: "led_classdev_register" [drivers/media/radio/shark2.ko] undefined! >> ERROR: "led_classdev_unregister" [drivers/media/radio/shark2.ko] undefined! >> > > Could you please test the enclosed patch? > > Thanks! > Mauro > > - > > [media] radio-shark: make sure LEDS_CLASS is selected > > As reported by David: > > ERROR: "led_classdev_register" [drivers/media/radio/shark2.ko] undefined! > > ERROR: "led_classdev_unregister" [drivers/media/radio/shark2.ko] undefined! > > Reported-by: Dadiv Rientjes <rientjes@google.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > > diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig > index 8090b87..be68ec2 100644 > --- a/drivers/media/radio/Kconfig > +++ b/drivers/media/radio/Kconfig > @@ -60,6 +60,7 @@ config RADIO_MAXIRADIO > config RADIO_SHARK > tristate "Griffin radioSHARK USB radio receiver" > depends on USB && SND > + select LEDS_CLASS > ---help--- > Choose Y here if you have this radio receiver. > > @@ -77,6 +78,7 @@ config RADIO_SHARK > config RADIO_SHARK2 > tristate "Griffin radioSHARK2 USB radio receiver" > depends on USB > + select LEDS_CLASS > ---help--- > Choose Y here if you have this radio receiver. > > -- 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 09-08-2012 09:00, Hans de Goede escreveu: > Hi, > > My bad, sorry about this. Mauro's patch looks good. Hmm... menuconfig NEW_LEDS bool "LED Support" help Say Y to enable Linux LED support. This allows control of supported LEDs from both userspace and optionally, by kernel events (triggers). This is not related to standard keyboard LEDs which are controlled via the input system. if NEW_LEDS config LEDS_CLASS ... It seems that the patch also need to select or depend on NEW_LEDS. > An alternative fix > would be to #ifdefify the led code in the drivers themselves. Yeah, that would work as well, although the code would look uglier. IMHO, using select/depend is better. Regards, Mauro > > Regards, > > Hans > > > On 08/09/2012 01:38 PM, Mauro Carvalho Chehab wrote: >> Em 08-08-2012 19:28, David Rientjes escreveu: >>> On Tue, 31 Jul 2012, Mauro Carvalho Chehab wrote: >>> >>>> [media] radio-shark: New driver for the Griffin radioSHARK USB radio receiver >>> >>> This one gives me a build warning if CONFIG_LEDS_CLASS is disabled: >>> >>> ERROR: "led_classdev_register" [drivers/media/radio/shark2.ko] undefined! >>> ERROR: "led_classdev_unregister" [drivers/media/radio/shark2.ko] undefined! >>> >> >> Could you please test the enclosed patch? >> >> Thanks! >> Mauro >> >> - >> >> [media] radio-shark: make sure LEDS_CLASS is selected >> >> As reported by David: >> > ERROR: "led_classdev_register" [drivers/media/radio/shark2.ko] undefined! >> > ERROR: "led_classdev_unregister" [drivers/media/radio/shark2.ko] undefined! >> >> Reported-by: Dadiv Rientjes <rientjes@google.com> >> Cc: Hans de Goede <hdegoede@redhat.com> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> >> >> >> diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig >> index 8090b87..be68ec2 100644 >> --- a/drivers/media/radio/Kconfig >> +++ b/drivers/media/radio/Kconfig >> @@ -60,6 +60,7 @@ config RADIO_MAXIRADIO >> config RADIO_SHARK >> tristate "Griffin radioSHARK USB radio receiver" >> depends on USB && SND >> + select LEDS_CLASS >> ---help--- >> Choose Y here if you have this radio receiver. >> >> @@ -77,6 +78,7 @@ config RADIO_SHARK >> config RADIO_SHARK2 >> tristate "Griffin radioSHARK2 USB radio receiver" >> depends on USB >> + select LEDS_CLASS >> ---help--- >> Choose Y here if you have this radio receiver. >> >> -- 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 Thu, 9 Aug 2012, Mauro Carvalho Chehab wrote: > Yeah, that would work as well, although the code would look uglier. > IMHO, using select/depend is better. > Agreed, I think it should be "depends on LEDS_CLASS" rather than select it if there is a hard dependency that cannot be fixed with extracting the led support in the driver to #ifdef CONFIG_LEDS_CLASS code. -- 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
Hi, On 08/09/2012 10:03 PM, David Rientjes wrote: > On Thu, 9 Aug 2012, Mauro Carvalho Chehab wrote: > >> Yeah, that would work as well, although the code would look uglier. >> IMHO, using select/depend is better. >> > > Agreed, I think it should be "depends on LEDS_CLASS" rather than select > it if there is a hard dependency that cannot be fixed with extracting the > led support in the driver to #ifdef CONFIG_LEDS_CLASS code. The led support could be #ifdef CONFIG_LEDS_CLASS, the problem with that approach is the whole module versus build-in thing: led-class shark enable-led-support build-in build-in yes build-in module yes module build-in no module module yes Now this can be coded into #ifdef magic, but it won't be pretty, of course we only need the non pretty version once at the top to set a SHARK_USE_LEDS define, but still. I'm fine with either solution (depends or ifdef magic), although I think that people will get unpleasantly surprised if they want to use the shark driver and they don't get to see it in the menu because they don't have leds enabled. Regards, Hans -- 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/Kconfig b/drivers/media/radio/Kconfig index 8090b87..be68ec2 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -60,6 +60,7 @@ config RADIO_MAXIRADIO config RADIO_SHARK tristate "Griffin radioSHARK USB radio receiver" depends on USB && SND + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver. @@ -77,6 +78,7 @@ config RADIO_SHARK config RADIO_SHARK2 tristate "Griffin radioSHARK2 USB radio receiver" depends on USB + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver.