led: core: fix misleading comment after workqueue removal from drivers
PATCH 1/1 led: core: fix misleading comment after workqueue removal from drivers
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.
--- include/linux/leds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
include/linux/leds.h +4 -1
@@ -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. +*/ void (*brightness_set)(struct led_classdev *led_cdev,enum led_brightness brightness);/*
Signature · 1 line
2.7.0
Hi Heiner,
Quoted 14 lines · Heiner Kallweit
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 <[email protected]>--- include/linux/leds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)include/linux/leds.h +0
-0
include/linux/leds.h struct led_classdev { · line 56
Quoted 8 lines
include/linux/leds.h +4
-1@@ -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."
Quoted 4 lines
include/linux/leds.h +0
-0void (*brightness_set)(struct led_classdev *led_cdev,enum led_brightness brightness);/*
Signature · 2 lines
Best Regards, Jacek Anaszewski
Quoted 30 lines · Jacek Anaszewski
Hi Heiner,
Quoted 15 lines · Heiner Kallweit
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 <[email protected]>--- include/linux/leds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)include/linux/leds.h +0
-0include/linux/leds.h struct led_classdev { · line 56
Quoted 8 lines
include/linux/leds.h +4
-1@@ -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.
Quoted 5 lines
How about:
"Must not sleep. Use
brightness_set_blockingfor 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?
Regards, Heiner
Quoted 9 lines
include/linux/leds.h struct led_classdev { · line 59
Quoted 4 lines
include/linux/leds.h +0
-0void (*brightness_set)(struct led_classdev *led_cdev,enum led_brightness brightness);/* --
Best Regards,
Jacek Anaszewski
Quoted 32 lines · Heiner Kallweit
Quoted 30 lines · Jacek Anaszewski
Hi Heiner,
Quoted 15 lines · Heiner Kallweit
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 <[email protected]>--- include/linux/leds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)include/linux/leds.h +0
-0include/linux/leds.h struct led_classdev { · line 56
Quoted 8 lines
include/linux/leds.h +4
-1@@ -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.
Quoted 8 lines
Quoted 5 lines
How about:
"Must not sleep. Use
brightness_set_blockingfor 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.
Quoted 11 lines
Quoted 9 lines
include/linux/leds.h struct led_classdev { · line 59
Quoted 4 lines
include/linux/leds.h +0
-0void (*brightness_set)(struct led_classdev *led_cdev,enum led_brightness brightness);/* --
Best Regards,
Jacek Anaszewski
Signature · 2 lines
Best Regards, Jacek Anaszewski