Re: [PATCH] HID: logitech: Add depends on LEDS_CLASS to Logitech Kconfig entry

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/4/19 1:30 PM, Benjamin Tissoires wrote:
> Hi Hans,
> 
> On Fri, Oct 4, 2019 at 3:37 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Commit 97b741aba918 ("HID: lg-g15: Add keyboard and LCD backlight control")
>> makes the hid-lg15 kernel module, which gets configured through config
>> HID_LOGITECH depends on symbols from the led class. Add a depends on
>> LEDS_CLASS to HID_LOGITECH to avoid undefined reference errors on the
>> led class symbols.
>>
>> Fixes: 97b741aba918 ("HID: lg-g15: Add keyboard and LCD backlight control")
>> Reported-by: kbuild test robot <lkp@intel.com>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/hid/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 1ecb5124421c..494a39e74939 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -525,6 +525,7 @@ config HID_LENOVO
>>   config HID_LOGITECH
>>          tristate "Logitech devices"
>>          depends on HID
>> +       depends on LEDS_CLASS
> 
> My CI failed with this patch. It seems this creates a loop in the
> dependencies and I can not create the config file. Also, it seems
> weird to have a depends on LEDS_CLASS when you surely wants to enable
> it. How about using SELECT instead?

That is weird, there is a whole bunch of other HID_FOO config
symbols in drivers/hid/Kconfig which also have a depends on LEDS_CLASS,
so if this is a problem then it should be a problem for those other ones
too.

I do see that there are others which use select instead of depends
(yeah consistency) so I guess that changing to select is fine here...

Please let me know how you want to proceed with this.

Regards,

Hans