[PATCH v6 2/2] leds: pwm: add reference to common leds for default-state

[PATCH v6 2/2] leds: pwm: add reference to common leds for default-state

[Denis Osterland-Heim]

The default-state is now supported for PWM leds.

Signed-off-by: Denis Osterland-Heim <Denis.Osterland@diehl.com>
Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/leds/leds-pwm.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.txt b/Documentation/devicetree/bindings/leds/leds-pwm.txt
index 6c6583c35f2f..d0f489680594 100644
--- a/Documentation/devicetree/bindings/leds/leds-pwm.txt
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.txt
@@ -19,6 +19,8 @@ LED sub-node properties:
   see Documentation/devicetree/bindings/leds/common.txt
 - linux,default-trigger :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
+- default-state : (optional)
+  see Documentation/devicetree/bindings/leds/common.yaml
 
 Example:
 
-- 
2.27.0



Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315
___________________________________________________________________________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. 
- For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/

[PATCH v6 1/2] leds: pwm: add support for default-state device property

[Denis Osterland-Heim]

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@diehl.com>
Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
---
 drivers/leds/leds-pwm.c | 54 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index ef7b91bd2064..7b199c151768 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -18,10 +18,15 @@
 #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;
 	u8		active_low;
+	u8		default_state;
 	unsigned int	max_brightness;
 };
 
@@ -88,7 +93,30 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
 
 	led_data->cdev.brightness_set_blocking = led_pwm_set;
 
-	pwm_init_state(led_data->pwm, &led_data->pwmstate);
+	/* init PWM state */
+	if (led->default_state == LEDS_PWM_DEFSTATE_KEEP) {
+		pwm_get_state(led_data->pwm, &led_data->pwmstate);
+		if (!led_data->pwmstate.period) {
+			led->default_state = LEDS_PWM_DEFSTATE_OFF;
+			dev_warn(dev,
+				"failed to read period for %s, default to off",
+				led->name);
+		}
+	}
+	if (led->default_state != LEDS_PWM_DEFSTATE_KEEP)
+		pwm_init_state(led_data->pwm, &led_data->pwmstate);
+
+	/* set brightness */
+	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;
+
+		brightness = led->max_brightness;
+		brightness *= led_data->pwmstate.duty_cycle;
+		do_div(brightness, led_data->pwmstate.period);
+		led_data->cdev.brightness = brightness;
+	}
 
 	ret = devm_led_classdev_register(dev, &led_data->cdev);
 	if (ret) {
@@ -97,11 +125,13 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
 		return ret;
 	}
 
-	ret = led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
-	if (ret) {
-		dev_err(dev, "failed to set led PWM value for %s: %d",
-			led->name, ret);
-		return ret;
+	if (led->default_state != LEDS_PWM_DEFSTATE_KEEP) {
+		ret = led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
+		if (ret) {
+			dev_err(dev, "failed to set led PWM value for %s: %d",
+				led->name, ret);
+			return ret;
+		}
 	}
 
 	priv->num_leds++;
@@ -117,6 +147,8 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 	memset(&led, 0, sizeof(led));
 
 	device_for_each_child_node(dev, fwnode) {
+		const char *state = NULL;
+
 		ret = fwnode_property_read_string(fwnode, "label", &led.name);
 		if (ret && is_of_node(fwnode))
 			led.name = to_of_node(fwnode)->name;
@@ -134,6 +166,16 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 		fwnode_property_read_u32(fwnode, "max-brightness",
 					 &led.max_brightness);
 
+		if (!fwnode_property_read_string(fwnode, "default-state",
+						 &state)) {
+			if (!strcmp(state, "keep"))
+				led.default_state = LEDS_PWM_DEFSTATE_KEEP;
+			else if (!strcmp(state, "on"))
+				led.default_state = LEDS_PWM_DEFSTATE_ON;
+			else
+				led.default_state = LEDS_PWM_DEFSTATE_OFF;
+		}
+
 		ret = led_pwm_add(dev, priv, &led, fwnode);
 		if (ret) {
 			fwnode_handle_put(fwnode);
-- 
2.27.0



Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315
___________________________________________________________________________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. 
- For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/

[PATCH v6 0/2] leds: pwm: add support for default-state device

[Denis Osterland-Heim]

v5 -> v6: tested the rebase to v5.8-rc2 based for-next

 .../devicetree/bindings/leds/leds-pwm.txt          |  2 +
 drivers/leds/leds-pwm.c                            | 54 +++++++++++++++++++---
 2 files changed, 50 insertions(+), 6 deletions(-)

Message-Id: <20200421130644.16059-1-Denis.Osterland@diehl.com>
base-commit: cf1a1a6a7d81d73bcb5568b23572d6fd593add87





Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315
___________________________________________________________________________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. 
- For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/

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

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 3270 bytes --]


> 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@diehl.com>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

> +#define LEDS_PWM_DEFSTATE_OFF	0
> +#define LEDS_PWM_DEFSTATE_ON	1
> +#define LEDS_PWM_DEFSTATE_KEEP	2

Turn this into enum; no need for prefix as this is private to the driver.

>  struct led_pwm {
>  	const char	*name;
>  	const char	*default_trigger;
>  	u8		active_low;
> +	u8		default_state;
>  	unsigned int	max_brightness;
>  };
>  
> @@ -88,7 +93,30 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
>  
>  	led_data->cdev.brightness_set_blocking = led_pwm_set;
>  
> -	pwm_init_state(led_data->pwm, &led_data->pwmstate);
> +	/* init PWM state */
> +	if (led->default_state == LEDS_PWM_DEFSTATE_KEEP) {
> +		pwm_get_state(led_data->pwm, &led_data->pwmstate);
> +		if (!led_data->pwmstate.period) {
> +			led->default_state = LEDS_PWM_DEFSTATE_OFF;
> +			dev_warn(dev,
> +				"failed to read period for %s, default to off",
> +				led->name);
> +		}
> +	}
> +	if (led->default_state != LEDS_PWM_DEFSTATE_KEEP)
> +		pwm_init_state(led_data->pwm, &led_data->pwmstate);
> +
> +	/* set brightness */
> +	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;
> +
> +		brightness = led->max_brightness;
> +		brightness *= led_data->pwmstate.duty_cycle;
> +		do_div(brightness, led_data->pwmstate.period);
> +		led_data->cdev.brightness = brightness;
> +	}

Try to clean this up... switch() might help. Maybe two of them.

> @@ -134,6 +166,16 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
>  		fwnode_property_read_u32(fwnode, "max-brightness",
>  					 &led.max_brightness);
>  
> +		if (!fwnode_property_read_string(fwnode, "default-state",
> +						 &state)) {
> +			if (!strcmp(state, "keep"))
> +				led.default_state = LEDS_PWM_DEFSTATE_KEEP;
> +			else if (!strcmp(state, "on"))
> +				led.default_state = LEDS_PWM_DEFSTATE_ON;
> +			else
> +				led.default_state = LEDS_PWM_DEFSTATE_OFF;
> +		}

Actually... Move the enum to core, and add helper for this. We don't
want to see this duplicated.

> The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
> mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. 
> - For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/

Get rid of this.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

[Denis Osterland-Heim]

Hi Pavel,

Am Mittwoch, den 22.07.2020, 16:04 +0200 schrieb Pavel Machek:
> > 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@diehl.com>
> > Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> > +#define LEDS_PWM_DEFSTATE_OFF	0
> > +#define LEDS_PWM_DEFSTATE_ON	1
> > +#define LEDS_PWM_DEFSTATE_KEEP	2
> 
> Turn this into enum; no need for prefix as this is private to the driver.
> 
> >  struct led_pwm {
> >  	const char	*name;
> >  	const char	*default_trigger;
> >  	u8		active_low;
> > +	u8		default_state;
> >  	unsigned int	max_brightness;
> >  };
> >  
> > @@ -88,7 +93,30 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> >  
> >  	led_data->cdev.brightness_set_blocking = led_pwm_set;
> >  
> > -	pwm_init_state(led_data->pwm, &led_data->pwmstate);
> > +	/* init PWM state */
> > +	if (led->default_state == LEDS_PWM_DEFSTATE_KEEP) {
> > +		pwm_get_state(led_data->pwm, &led_data->pwmstate);
> > +		if (!led_data->pwmstate.period) {
> > +			led->default_state = LEDS_PWM_DEFSTATE_OFF;
> > +			dev_warn(dev,
> > +				"failed to read period for %s, default to off",
> > +				led->name);
> > +		}
> > +	}
> > +	if (led->default_state != LEDS_PWM_DEFSTATE_KEEP)
> > +		pwm_init_state(led_data->pwm, &led_data->pwmstate);
> > +
> > +	/* set brightness */
> > +	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;
> > +
> > +		brightness = led->max_brightness;
> > +		brightness *= led_data->pwmstate.duty_cycle;
> > +		do_div(brightness, led_data->pwmstate.period);
> > +		led_data->cdev.brightness = brightness;
> > +	}
> 
> Try to clean this up... switch() might help. Maybe two of them.
looks possible, lets see if it improves readability

> 
> > @@ -134,6 +166,16 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
> >  		fwnode_property_read_u32(fwnode, "max-brightness",
> >  					 &led.max_brightness);
> >  
> > +		if (!fwnode_property_read_string(fwnode, "default-state",
> > +						 &state)) {
> > +			if (!strcmp(state, "keep"))
> > +				led.default_state = LEDS_PWM_DEFSTATE_KEEP;
> > +			else if (!strcmp(state, "on"))
> > +				led.default_state = LEDS_PWM_DEFSTATE_ON;
> > +			else
> > +				led.default_state = LEDS_PWM_DEFSTATE_OFF;
> > +		}
> 
> Actually... Move the enum to core, and add helper for this. We don't
> want to see this duplicated.
I think I should put the refactoring into a separate patch.

This code duplicates leds-gpio, which uses the LEDS_GPIO_DEFSTATE_* defines,
defined in include/linux/leds.h.
The I idea was to let gpio its defines and add dt compatible defines for leds-pwm.
But it would be useful to have a common function, yes.

It looks like a enum leds_default_state with the define for leds-gpio changed
to the new enum in linux/leds.h and a new function declaration leds_defstate_get()
in leds/leds.h, with the implementation led-core.c. And leds-gpio and leds-pwm
will then include leds/leds.h to use leds_defstate_get().

> 
> > The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
> > mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. 
> > - For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/
> 
> Get rid of this.
my force is not strong enough...

Regards, Denis

> 									Pavel
> +----------------------------------------------------------------------+
> > Z1 SecureMail Gateway Processing Info                                |
> 
> +----------------------------------------------------------------------+
> > - The message was signed by                                          |
> >   [No Info available]                                                |
> >   Signature not verifiable                                           |
> >   - Message content not verifiable                                   |
> >   - Certificate not verifiable                                       |
> 
> +----------------------------------------------------------------------+


Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315
___________________________________________________________________________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. 
- For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/