Re: [PATCH] led: core: fix misleading comment after workqueue removal from drivers

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 01/21/2016 10:50 AM, Heiner Kallweit wrote:
> On Thu, Jan 21, 2016 at 10:03 AM, Jacek Anaszewski
> <j.anaszewski@samsung.com> wrote:
>> Hi Heiner,
>>
>> On 01/10/2016 06:16 PM, Heiner Kallweit wrote:
>>>
>>> Now that workqueues have been removed from individual drivers and
>>> were replaced with a core-internal workqueue we shouldn't
>>> encourage people to add new workqueues to drivers.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>    include/linux/leds.h | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>>> index bc1476f..4429887 100644
>>> --- a/include/linux/leds.h
>>> +++ b/include/linux/leds.h
>>> @@ -50,7 +50,10 @@ struct led_classdev {
>>>    #define LED_DEV_CAP_FLASH     (1 << 23)
>>>
>>>          /* Set LED brightness level */
>>> -       /* Must not sleep, use a workqueue if needed */
>>> +       /* Must not sleep. If no non-blocking version can be provided
>>> +        * set brightness_set_blocking only. The LED core will use an
>>> +        * internal work queue to create a non-blocking version.
>>> +        */
>>
>>
>> This comment isn't easily comprehensible at first glance.
>>
>> How about:
>>
>> "Must not sleep. Use brightness_set_blocking for drivers
>> that can sleep while setting brightness."
>>
> Yes, this is better. It's simpler and clearer.
> Are you going to fix the comment on your own or would you like me
> to submit it as a patch?

Since you are the original author of the patch, feel free to submit
the final version with the agreed comment.

>>>          void            (*brightness_set)(struct led_classdev *led_cdev,
>>>                                            enum led_brightness brightness);
>>>          /*
>>>
>>
>>
>> --
>> Best Regards,
>> Jacek Anaszewski
>
>


-- 
Best Regards,
Jacek Anaszewski