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!
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 <[email protected]>
Cc: Hans de Goede <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
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.
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 <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>
>
> 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.
>
>
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 <[email protected]>
>> Cc: Hans de Goede <[email protected]>
>> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>>
>>
>> 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.
>>
>>
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.
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