* [PATCH] leds: core: add helpers for calling brightness_set(_blocking)
@ 2016-02-07 0:20 Heiner Kallweit
2016-02-12 16:10 ` Jacek Anaszewski
0 siblings, 1 reply; 3+ messages in thread
From: Heiner Kallweit @ 2016-02-07 0:20 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: linux-leds
Add helpers for calling brightness_set(_blocking) allowing to
simplify the code and make it better readable.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/leds/led-core.c | 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index ad684b6..c29e4c9 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -25,6 +25,26 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
LIST_HEAD(leds_list);
EXPORT_SYMBOL_GPL(leds_list);
+static int led_set_output(struct led_classdev *cdev,
+ enum led_brightness value)
+{
+ if (!cdev->brightness_set)
+ return -ENOTSUPP;
+
+ cdev->brightness_set(cdev, value);
+
+ return 0;
+}
+
+static int led_set_output_blocking(struct led_classdev *cdev,
+ enum led_brightness value)
+{
+ if (!cdev->brightness_set_blocking)
+ return -ENOTSUPP;
+
+ return cdev->brightness_set_blocking(cdev, value);
+}
+
static void led_timer_function(unsigned long data)
{
struct led_classdev *led_cdev = (void *)data;
@@ -91,13 +111,10 @@ static void set_brightness_delayed(struct work_struct *ws)
led_cdev->flags &= ~LED_BLINK_DISABLE;
}
- if (led_cdev->brightness_set)
- led_cdev->brightness_set(led_cdev, led_cdev->delayed_set_value);
- else if (led_cdev->brightness_set_blocking)
- ret = led_cdev->brightness_set_blocking(led_cdev,
- led_cdev->delayed_set_value);
- else
- ret = -ENOTSUPP;
+ ret = led_set_output(led_cdev, led_cdev->delayed_set_value);
+ if (ret == -ENOTSUPP)
+ ret = led_set_output_blocking(led_cdev,
+ led_cdev->delayed_set_value);
if (ret < 0 &&
/* LED HW might have been unplugged, therefore don't warn */
!(ret == -ENODEV && (led_cdev->flags & LED_UNREGISTERING) &&
@@ -236,10 +253,8 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
enum led_brightness value)
{
/* Use brightness_set op if available, it is guaranteed not to sleep */
- if (led_cdev->brightness_set) {
- led_cdev->brightness_set(led_cdev, value);
+ if (!led_set_output(led_cdev, value))
return;
- }
/* If brightness setting can sleep, delegate it to a work queue task */
led_cdev->delayed_set_value = value;
@@ -270,10 +285,7 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
if (led_cdev->flags & LED_SUSPENDED)
return 0;
- if (led_cdev->brightness_set_blocking)
- return led_cdev->brightness_set_blocking(led_cdev,
- led_cdev->brightness);
- return -ENOTSUPP;
+ return led_set_output_blocking(led_cdev, led_cdev->brightness);
}
EXPORT_SYMBOL_GPL(led_set_brightness_sync);
--
2.7.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] leds: core: add helpers for calling brightness_set(_blocking)
2016-02-07 0:20 [PATCH] leds: core: add helpers for calling brightness_set(_blocking) Heiner Kallweit
@ 2016-02-12 16:10 ` Jacek Anaszewski
2016-02-13 17:28 ` Heiner Kallweit
0 siblings, 1 reply; 3+ messages in thread
From: Jacek Anaszewski @ 2016-02-12 16:10 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-leds
On 02/07/2016 01:20 AM, Heiner Kallweit wrote:
> Add helpers for calling brightness_set(_blocking) allowing to
> simplify the code and make it better readable.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/leds/led-core.c | 40 ++++++++++++++++++++++++++--------------
> 1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index ad684b6..c29e4c9 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -25,6 +25,26 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
> LIST_HEAD(leds_list);
> EXPORT_SYMBOL_GPL(leds_list);
>
> +static int led_set_output(struct led_classdev *cdev,
s/cdev/led_cdev/
Please adhere to the existing naming convention.
led_set_output isn't too informative name, and can be misleading.
We already have a number of different internal functions for setting LED
brightness, so we have to be careful when adding new ones.
At the moment I don't have any good candidate.
__led_set_brightness() and __led_set_brightness_blocking() as a last
resort. We had the former at some point.
Nonetheless it would be great if you came up with some better
names:)
> + enum led_brightness value)
> +{
> + if (!cdev->brightness_set)
> + return -ENOTSUPP;
> +
> + cdev->brightness_set(cdev, value);
> +
> + return 0;
> +}
> +
> +static int led_set_output_blocking(struct led_classdev *cdev,
> + enum led_brightness value)
> +{
> + if (!cdev->brightness_set_blocking)
> + return -ENOTSUPP;
> +
> + return cdev->brightness_set_blocking(cdev, value);
> +}
> +
> static void led_timer_function(unsigned long data)
> {
> struct led_classdev *led_cdev = (void *)data;
> @@ -91,13 +111,10 @@ static void set_brightness_delayed(struct work_struct *ws)
> led_cdev->flags &= ~LED_BLINK_DISABLE;
> }
>
> - if (led_cdev->brightness_set)
> - led_cdev->brightness_set(led_cdev, led_cdev->delayed_set_value);
> - else if (led_cdev->brightness_set_blocking)
> - ret = led_cdev->brightness_set_blocking(led_cdev,
> - led_cdev->delayed_set_value);
> - else
> - ret = -ENOTSUPP;
> + ret = led_set_output(led_cdev, led_cdev->delayed_set_value);
> + if (ret == -ENOTSUPP)
> + ret = led_set_output_blocking(led_cdev,
> + led_cdev->delayed_set_value);
> if (ret < 0 &&
> /* LED HW might have been unplugged, therefore don't warn */
> !(ret == -ENODEV && (led_cdev->flags & LED_UNREGISTERING) &&
> @@ -236,10 +253,8 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> /* Use brightness_set op if available, it is guaranteed not to sleep */
> - if (led_cdev->brightness_set) {
> - led_cdev->brightness_set(led_cdev, value);
> + if (!led_set_output(led_cdev, value))
> return;
> - }
>
> /* If brightness setting can sleep, delegate it to a work queue task */
> led_cdev->delayed_set_value = value;
> @@ -270,10 +285,7 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
> if (led_cdev->flags & LED_SUSPENDED)
> return 0;
>
> - if (led_cdev->brightness_set_blocking)
> - return led_cdev->brightness_set_blocking(led_cdev,
> - led_cdev->brightness);
> - return -ENOTSUPP;
> + return led_set_output_blocking(led_cdev, led_cdev->brightness);
> }
> EXPORT_SYMBOL_GPL(led_set_brightness_sync);
>
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] leds: core: add helpers for calling brightness_set(_blocking)
2016-02-12 16:10 ` Jacek Anaszewski
@ 2016-02-13 17:28 ` Heiner Kallweit
0 siblings, 0 replies; 3+ messages in thread
From: Heiner Kallweit @ 2016-02-13 17:28 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: linux-leds
Am 12.02.2016 um 17:10 schrieb Jacek Anaszewski:
> On 02/07/2016 01:20 AM, Heiner Kallweit wrote:
>> Add helpers for calling brightness_set(_blocking) allowing to
>> simplify the code and make it better readable.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/leds/led-core.c | 40 ++++++++++++++++++++++++++--------------
>> 1 file changed, 26 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index ad684b6..c29e4c9 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -25,6 +25,26 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
>> LIST_HEAD(leds_list);
>> EXPORT_SYMBOL_GPL(leds_list);
>>
>> +static int led_set_output(struct led_classdev *cdev,
>
> s/cdev/led_cdev/
>
> Please adhere to the existing naming convention.
>
> led_set_output isn't too informative name, and can be misleading.
> We already have a number of different internal functions for setting LED brightness, so we have to be careful when adding new ones.
>
> At the moment I don't have any good candidate.
>
> __led_set_brightness() and __led_set_brightness_blocking() as a last
> resort. We had the former at some point.
>
> Nonetheless it would be great if you came up with some better
> names:)
>
I was expecting such a remark as I wasn't too happy with the names either.
But I had (and still have) no really good idea for a telling name being not
too long (as I'd like to avoid Java-like variable names spanning across more
than one line). Having said that your proposal of using a double underscore
prefix might still be the best compromise.
>> + enum led_brightness value)
>> +{
>> + if (!cdev->brightness_set)
>> + return -ENOTSUPP;
>> +
>> + cdev->brightness_set(cdev, value);
>> +
>> + return 0;
>> +}
>> +
>> +static int led_set_output_blocking(struct led_classdev *cdev,
>> + enum led_brightness value)
>> +{
>> + if (!cdev->brightness_set_blocking)
>> + return -ENOTSUPP;
>> +
>> + return cdev->brightness_set_blocking(cdev, value);
>> +}
>> +
>> static void led_timer_function(unsigned long data)
>> {
>> struct led_classdev *led_cdev = (void *)data;
>> @@ -91,13 +111,10 @@ static void set_brightness_delayed(struct work_struct *ws)
>> led_cdev->flags &= ~LED_BLINK_DISABLE;
>> }
>>
>> - if (led_cdev->brightness_set)
>> - led_cdev->brightness_set(led_cdev, led_cdev->delayed_set_value);
>> - else if (led_cdev->brightness_set_blocking)
>> - ret = led_cdev->brightness_set_blocking(led_cdev,
>> - led_cdev->delayed_set_value);
>> - else
>> - ret = -ENOTSUPP;
>> + ret = led_set_output(led_cdev, led_cdev->delayed_set_value);
>> + if (ret == -ENOTSUPP)
>> + ret = led_set_output_blocking(led_cdev,
>> + led_cdev->delayed_set_value);
>> if (ret < 0 &&
>> /* LED HW might have been unplugged, therefore don't warn */
>> !(ret == -ENODEV && (led_cdev->flags & LED_UNREGISTERING) &&
>> @@ -236,10 +253,8 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
>> enum led_brightness value)
>> {
>> /* Use brightness_set op if available, it is guaranteed not to sleep */
>> - if (led_cdev->brightness_set) {
>> - led_cdev->brightness_set(led_cdev, value);
>> + if (!led_set_output(led_cdev, value))
>> return;
>> - }
>>
>> /* If brightness setting can sleep, delegate it to a work queue task */
>> led_cdev->delayed_set_value = value;
>> @@ -270,10 +285,7 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
>> if (led_cdev->flags & LED_SUSPENDED)
>> return 0;
>>
>> - if (led_cdev->brightness_set_blocking)
>> - return led_cdev->brightness_set_blocking(led_cdev,
>> - led_cdev->brightness);
>> - return -ENOTSUPP;
>> + return led_set_output_blocking(led_cdev, led_cdev->brightness);
>> }
>> EXPORT_SYMBOL_GPL(led_set_brightness_sync);
>>
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-02-13 18:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-07 0:20 [PATCH] leds: core: add helpers for calling brightness_set(_blocking) Heiner Kallweit
2016-02-12 16:10 ` Jacek Anaszewski
2016-02-13 17:28 ` Heiner Kallweit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).