linux-kernel - Re: [PATCH] leds: remove redundant SET_BRIGHTNESS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 14 May 2015 11:11:37 +0200
From:	Jacek Anaszewski <j.anaszewski@...sung.com>
To:	Stas Sergeev <stsp@...t.ru>
Cc:	linux-leds@...r.kernel.org,
	Linux kernel <linux-kernel@...r.kernel.org>,
	Bryan Wu <cooloney@...il.com>,
	Richard Purdie <rpurdie@...ys.net>,
	Stas Sergeev <stsp@...rs.sourceforge.net>
Subject: Re: [PATCH] leds: remove redundant SET_BRIGHTNESS_SYNC flag

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@...il.com>
> CC: Richard Purdie <rpurdie@...ys.net>
> CC: linux-leds@...r.kernel.org
> CC: linux-kernel@...r.kernel.org
>
> Signed-off-by: Stas Sergeev <stsp@...rs.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/