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

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

Hi,

On 04-10-2019 18:22, Benjamin Tissoires wrote:
> On Fri, Oct 4, 2019 at 11:01 AM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>>
>> On Fri, Oct 4, 2019 at 8:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> 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.
>>
>> Yeah, it seems it's my script that generates the config that is bugged...
>>
>>>
>>> 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.
>>
>> I'll try to fix this and will push the depnds preferably or the
>> selects if no other options is possible.
> 
> I just fixed my config script and pushed your patch on top of for-5.5/logitech.

Great, thank you.

Regards,

Hans