linux-kernel - Re: [PATCH v3 2/3] leds: pwm: add support for default-state device property

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]

Message-ID: <bee4d31f-1f00-c621-f93c-f49207e406d6@gmail.com>
Date:   Mon, 16 Mar 2020 19:36:12 +0100
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Denis Osterland-Heim <denis.osterland@...hl.com>,
        "dmurphy@...com" <dmurphy@...com>, "pavel@....cz" <pavel@....cz>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] leds: pwm: add support for default-state device
 property

Hi Denis,

On 3/16/20 1:53 PM, Denis Osterland-Heim wrote:
> This patch adds support for "default-state" devicetree property, which
> allows to defer pwm init to first use of led.
> 
> This allows to configure the PWM early in bootloader to let the LED
> blink until an application in Linux userspace sets something different.
> 
> Signed-off-by: Denis Osterland-Heim <Denis.Osterland@...hl.com>
> ---
>  drivers/leds/leds-pwm.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 5f69b6571595..fce7969e7918 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -18,11 +18,16 @@
>  #include <linux/pwm.h>
>  #include <linux/slab.h>
>  
> +#define LEDS_PWM_DEFSTATE_OFF	0
> +#define LEDS_PWM_DEFSTATE_ON	1
> +#define LEDS_PWM_DEFSTATE_KEEP	2
> +
>  struct led_pwm {
>  	const char	*name;
>  	const char	*default_trigger;
>  	unsigned int	pwm_id __deprecated;
>  	u8		active_low;
> +	u8		default_state;
>  	unsigned int	max_brightness;
>  	unsigned int	pwm_period_ns;
>  };
> @@ -72,7 +77,6 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
>  	led_data->active_low = led->active_low;
>  	led_data->cdev.name = led->name;
>  	led_data->cdev.default_trigger = led->default_trigger;
> -	led_data->cdev.brightness = LED_OFF;
>  	led_data->cdev.max_brightness = led->max_brightness;
>  	led_data->cdev.flags = LED_CORE_SUSPENDRESUME;
>  
> @@ -92,13 +96,27 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
>  
>  	pwm_init_state(led_data->pwm, &led_data->pwmstate);
>  
> +	if (led->default_state == LEDS_PWM_DEFSTATE_ON)
> +		led_data->cdev.brightness = led->max_brightness;
> +	else if (led->default_state == LEDS_PWM_DEFSTATE_KEEP) {
> +		uint64_t brightness;
> +
> +		pwm_get_state(led_data->pwm, &led_data->pwmstate);

This seems to not be reading the device state, i.e. what you tried
to address by direct call to pwm->chip->ops->get_state() before.

Am I missing something?

-- 
Best regards,
Jacek Anaszewski