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

From: Jacek Anaszewski
Date: Mon Mar 16 2020 - 14:36:19 EST


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@xxxxxxxxx>
> ---
> 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