* [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag
@ 2015-05-13 15:41 Stas Sergeev
2015-05-14 9:11 ` Jacek Anaszewski
0 siblings, 1 reply; 4+ messages in thread
From: Stas Sergeev @ 2015-05-13 15:41 UTC (permalink / raw)
To: linux-leds; +Cc: Linux kernel, Bryan Wu, Richard Purdie, Stas Sergeev
There is a complimentary flag called SET_BRIGHTNESS_ASYNC.
Having both is redundant. This patch removes the unneeded flag
without any functionality change.
CC: Bryan Wu <cooloney@gmail.com>
CC: Richard Purdie <rpurdie@rpsys.net>
CC: linux-leds@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
drivers/leds/led-class-flash.c | 1 -
drivers/leds/led-core.c | 4 +---
include/linux/leds.h | 2 +-
3 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 3b25734..bb67364 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -318,7 +318,6 @@ int led_classdev_flash_register(struct device *parent,
/* Setting a torch brightness needs to have immediate effect */
led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
- led_cdev->flags |= SET_BRIGHTNESS_SYNC;
return 0;
}
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 9886dac..356e851 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -129,10 +129,8 @@ void led_set_brightness(struct led_classdev *led_cdev,
if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
led_set_brightness_async(led_cdev, brightness);
return;
- } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC)
+ } else
ret = led_set_brightness_sync(led_cdev, brightness);
- else
- ret = -EINVAL;
if (ret < 0)
dev_dbg(led_cdev->dev, "Setting LED brightness failed (%d)\n",
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 9a2b000..c9e6e5d 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -45,7 +45,7 @@ struct led_classdev {
#define LED_BLINK_INVERT (1 << 19)
#define LED_SYSFS_DISABLE (1 << 20)
#define SET_BRIGHTNESS_ASYNC (1 << 21)
-#define SET_BRIGHTNESS_SYNC (1 << 22)
+/* bit 22 unused, take it */
#define LED_DEV_CAP_FLASH (1 << 23)
/* Set LED brightness level */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag
2015-05-13 15:41 [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag Stas Sergeev
@ 2015-05-14 9:11 ` Jacek Anaszewski
2015-05-14 9:19 ` Jacek Anaszewski
2015-05-14 20:50 ` Bryan Wu
0 siblings, 2 replies; 4+ messages in thread
From: Jacek Anaszewski @ 2015-05-14 9:11 UTC (permalink / raw)
To: Stas Sergeev
Cc: linux-leds, Linux kernel, Bryan Wu, Richard Purdie, Stas Sergeev
I personally can't find any strong argument against this optimization.
I added both flags on Bryan's request while implementing LED Flash class
extension. I can see a few occurrences in the include/linux directory
where both SYNC and ASYNC flags are implemented. The argument in favour
could be the fact that the flags influence the core functionality of
the subsystem.
Bryan, could you express your opinion, please?
On 05/13/2015 05:41 PM, Stas Sergeev wrote:
>
> There is a complimentary flag called SET_BRIGHTNESS_ASYNC.
> Having both is redundant. This patch removes the unneeded flag
> without any functionality change.
>
> CC: Bryan Wu <cooloney@gmail.com>
> CC: Richard Purdie <rpurdie@rpsys.net>
> CC: linux-leds@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
>
> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
> ---
> drivers/leds/led-class-flash.c | 1 -
> drivers/leds/led-core.c | 4 +---
> include/linux/leds.h | 2 +-
> 3 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> index 3b25734..bb67364 100644
> --- a/drivers/leds/led-class-flash.c
> +++ b/drivers/leds/led-class-flash.c
> @@ -318,7 +318,6 @@ int led_classdev_flash_register(struct device *parent,
>
> /* Setting a torch brightness needs to have immediate effect */
> led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
> - led_cdev->flags |= SET_BRIGHTNESS_SYNC;
>
> return 0;
> }
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 9886dac..356e851 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -129,10 +129,8 @@ void led_set_brightness(struct led_classdev *led_cdev,
> if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
> led_set_brightness_async(led_cdev, brightness);
> return;
> - } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC)
> + } else
> ret = led_set_brightness_sync(led_cdev, brightness);
> - else
> - ret = -EINVAL;
>
> if (ret < 0)
> dev_dbg(led_cdev->dev, "Setting LED brightness failed (%d)\n",
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 9a2b000..c9e6e5d 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -45,7 +45,7 @@ struct led_classdev {
> #define LED_BLINK_INVERT (1 << 19)
> #define LED_SYSFS_DISABLE (1 << 20)
> #define SET_BRIGHTNESS_ASYNC (1 << 21)
> -#define SET_BRIGHTNESS_SYNC (1 << 22)
> +/* bit 22 unused, take it */
> #define LED_DEV_CAP_FLASH (1 << 23)
>
> /* Set LED brightness level */
>
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag
2015-05-14 9:11 ` Jacek Anaszewski
@ 2015-05-14 9:19 ` Jacek Anaszewski
2015-05-14 20:50 ` Bryan Wu
1 sibling, 0 replies; 4+ messages in thread
From: Jacek Anaszewski @ 2015-05-14 9:19 UTC (permalink / raw)
To: Stas Sergeev
Cc: linux-leds, Linux kernel, Bryan Wu, Richard Purdie, Stas Sergeev
On 05/14/2015 11:11 AM, Jacek Anaszewski wrote:
> I personally can't find any strong argument against this optimization.
> I added both flags on Bryan's request while implementing LED Flash class
> extension. I can see a few occurrences in the include/linux directory
> where both SYNC and ASYNC flags are implemented. The argument in favour
I meant: "in favour of keeping that as is"
> could be the fact that the flags influence the core functionality of
> the subsystem.
>
> Bryan, could you express your opinion, please?
>
> On 05/13/2015 05:41 PM, Stas Sergeev wrote:
>>
>> There is a complimentary flag called SET_BRIGHTNESS_ASYNC.
>> Having both is redundant. This patch removes the unneeded flag
>> without any functionality change.
>>
>> CC: Bryan Wu <cooloney@gmail.com>
>> CC: Richard Purdie <rpurdie@rpsys.net>
>> CC: linux-leds@vger.kernel.org
>> CC: linux-kernel@vger.kernel.org
>>
>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>> ---
>> drivers/leds/led-class-flash.c | 1 -
>> drivers/leds/led-core.c | 4 +---
>> include/linux/leds.h | 2 +-
>> 3 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/leds/led-class-flash.c
>> b/drivers/leds/led-class-flash.c
>> index 3b25734..bb67364 100644
>> --- a/drivers/leds/led-class-flash.c
>> +++ b/drivers/leds/led-class-flash.c
>> @@ -318,7 +318,6 @@ int led_classdev_flash_register(struct device
>> *parent,
>>
>> /* Setting a torch brightness needs to have immediate effect */
>> led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
>> - led_cdev->flags |= SET_BRIGHTNESS_SYNC;
>>
>> return 0;
>> }
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index 9886dac..356e851 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -129,10 +129,8 @@ void led_set_brightness(struct led_classdev
>> *led_cdev,
>> if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
>> led_set_brightness_async(led_cdev, brightness);
>> return;
>> - } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC)
>> + } else
>> ret = led_set_brightness_sync(led_cdev, brightness);
>> - else
>> - ret = -EINVAL;
>>
>> if (ret < 0)
>> dev_dbg(led_cdev->dev, "Setting LED brightness failed (%d)\n",
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index 9a2b000..c9e6e5d 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -45,7 +45,7 @@ struct led_classdev {
>> #define LED_BLINK_INVERT (1 << 19)
>> #define LED_SYSFS_DISABLE (1 << 20)
>> #define SET_BRIGHTNESS_ASYNC (1 << 21)
>> -#define SET_BRIGHTNESS_SYNC (1 << 22)
>> +/* bit 22 unused, take it */
>> #define LED_DEV_CAP_FLASH (1 << 23)
>>
>> /* Set LED brightness level */
>>
>
>
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag
2015-05-14 9:11 ` Jacek Anaszewski
2015-05-14 9:19 ` Jacek Anaszewski
@ 2015-05-14 20:50 ` Bryan Wu
1 sibling, 0 replies; 4+ messages in thread
From: Bryan Wu @ 2015-05-14 20:50 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Stas Sergeev, Linux LED Subsystem, Linux kernel, Richard Purdie,
Stas Sergeev
On Thu, May 14, 2015 at 2:11 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> I personally can't find any strong argument against this optimization.
> I added both flags on Bryan's request while implementing LED Flash class
> extension. I can see a few occurrences in the include/linux directory
> where both SYNC and ASYNC flags are implemented. The argument in favour
> could be the fact that the flags influence the core functionality of
> the subsystem.
>
> Bryan, could you express your opinion, please?
>
Please keep this flag and although it's redundant at this point I
think it's still make code more clear and easy to understand.
Thanks,
-Bryan
>
> On 05/13/2015 05:41 PM, Stas Sergeev wrote:
>>
>>
>> There is a complimentary flag called SET_BRIGHTNESS_ASYNC.
>> Having both is redundant. This patch removes the unneeded flag
>> without any functionality change.
>>
>> CC: Bryan Wu <cooloney@gmail.com>
>> CC: Richard Purdie <rpurdie@rpsys.net>
>> CC: linux-leds@vger.kernel.org
>> CC: linux-kernel@vger.kernel.org
>>
>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>> ---
>> drivers/leds/led-class-flash.c | 1 -
>> drivers/leds/led-core.c | 4 +---
>> include/linux/leds.h | 2 +-
>> 3 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/leds/led-class-flash.c
>> b/drivers/leds/led-class-flash.c
>> index 3b25734..bb67364 100644
>> --- a/drivers/leds/led-class-flash.c
>> +++ b/drivers/leds/led-class-flash.c
>> @@ -318,7 +318,6 @@ int led_classdev_flash_register(struct device *parent,
>>
>> /* Setting a torch brightness needs to have immediate effect */
>> led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
>> - led_cdev->flags |= SET_BRIGHTNESS_SYNC;
>>
>> return 0;
>> }
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index 9886dac..356e851 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -129,10 +129,8 @@ void led_set_brightness(struct led_classdev
>> *led_cdev,
>> if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
>> led_set_brightness_async(led_cdev, brightness);
>> return;
>> - } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC)
>> + } else
>> ret = led_set_brightness_sync(led_cdev, brightness);
>> - else
>> - ret = -EINVAL;
>>
>> if (ret < 0)
>> dev_dbg(led_cdev->dev, "Setting LED brightness failed
>> (%d)\n",
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index 9a2b000..c9e6e5d 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -45,7 +45,7 @@ struct led_classdev {
>> #define LED_BLINK_INVERT (1 << 19)
>> #define LED_SYSFS_DISABLE (1 << 20)
>> #define SET_BRIGHTNESS_ASYNC (1 << 21)
>> -#define SET_BRIGHTNESS_SYNC (1 << 22)
>> +/* bit 22 unused, take it */
>> #define LED_DEV_CAP_FLASH (1 << 23)
>>
>> /* Set LED brightness level */
>>
>
>
> --
> Best Regards,
> Jacek Anaszewski
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-05-14 20:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13 15:41 [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag Stas Sergeev
2015-05-14 9:11 ` Jacek Anaszewski
2015-05-14 9:19 ` Jacek Anaszewski
2015-05-14 20:50 ` Bryan Wu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.