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

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

[Denis Osterland-Heim]

v2->v3:
 - s/set/sets/
 - remove leds_pwm.h
 - rebase on atomic PWM API
 - separate patch for devicetree changes
 - PWM default state defines instead of GPIO reuse
 - apply elegant if, else if schema

 .../devicetree/bindings/leds/leds-pwm.txt          |  2 +
 drivers/leds/leds-pwm.c                            | 49 ++++++++++++++++++++--
 include/linux/leds_pwm.h                           | 22 ----------
 3 files changed, 48 insertions(+), 25 deletions(-)

Message-Id: 20200310123126.4709-1-Denis.Osterland@diehl.com




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 v3 1/3] leds: pwm: remove header

[Denis Osterland-Heim]

The header is only used by leds_pwm.c, so move contents to leds_pwm.c
and remove it.
Apply minor changes suggested by checkpatch.

Suggested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Denis Osterland-Heim <Denis.Osterland@diehl.com>
---
 drivers/leds/leds-pwm.c  | 15 ++++++++++++++-
 include/linux/leds_pwm.h | 22 ----------------------
 2 files changed, 14 insertions(+), 23 deletions(-)
 delete mode 100644 include/linux/leds_pwm.h

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 9111cdede0ee..5f69b6571595 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -16,9 +16,22 @@
 #include <linux/leds.h>
 #include <linux/err.h>
 #include <linux/pwm.h>
-#include <linux/leds_pwm.h>
 #include <linux/slab.h>
 
+struct led_pwm {
+	const char	*name;
+	const char	*default_trigger;
+	unsigned int	pwm_id __deprecated;
+	u8		active_low;
+	unsigned int	max_brightness;
+	unsigned int	pwm_period_ns;
+};
+
+struct led_pwm_platform_data {
+	int		num_leds;
+	struct led_pwm	*leds;
+};
+
 struct led_pwm_data {
 	struct led_classdev	cdev;
 	struct pwm_device	*pwm;
diff --git a/include/linux/leds_pwm.h b/include/linux/leds_pwm.h
deleted file mode 100644
index 93d101d28943..000000000000
--- a/include/linux/leds_pwm.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * PWM LED driver data - see drivers/leds/leds-pwm.c
- */
-#ifndef __LINUX_LEDS_PWM_H
-#define __LINUX_LEDS_PWM_H
-
-struct led_pwm {
-	const char	*name;
-	const char	*default_trigger;
-	unsigned	pwm_id __deprecated;
-	u8 		active_low;
-	unsigned 	max_brightness;
-	unsigned	pwm_period_ns;
-};
-
-struct led_pwm_platform_data {
-	int			num_leds;
-	struct led_pwm	*leds;
-};
-
-#endif
-- 
2.25.1



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 v3 2/3] 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>
---
 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);
+		brightness = led->max_brightness;
+		brightness *= led_data->pwmstate.duty_cycle;
+		do_div(brightness, led_data->pwmstate.period);
+		led_data->cdev.brightness = (enum led_brightness)brightness;
+	}
+
 	if (!led_data->pwmstate.period)
 		led_data->pwmstate.period = led->pwm_period_ns;
 
 	ret = devm_led_classdev_register(dev, &led_data->cdev);
 	if (ret == 0) {
 		priv->num_leds++;
-		led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
+		if (led->default_state != LEDS_PWM_DEFSTATE_KEEP)
+			led_pwm_set(&led_data->cdev,
+					led_data->cdev.brightness);
 	} else {
 		dev_err(dev, "failed to register PWM led for %s: %d\n",
 			led->name, ret);
@@ -116,6 +134,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;
@@ -133,6 +153,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.25.1



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 v3 2/3] leds: pwm: add support for default-state device property

[Jacek Anaszewski]

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@diehl.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

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

[Denis Osterland-Heim]

Hi Jacek,

Am Montag, den 16.03.2020, 19:36 +0100 schrieb Jacek Anaszewski:
> Hi Denis,
> 
> On 3/16/20 1:53 PM, Denis Osterland-Heim wrote:
...
> >  
> > @@ -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?
> 

well, not you, but I missed cfc4c189bc70b1acc17e6f1abf1dc1c0ae890bd8.
Since this commit pwm_get_state() is sufficient.

Regards Denis



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 v3 2/3] leds: pwm: add support for default-state device property

[Jacek Anaszewski]

Hi Denis,

On 3/16/20 9:24 PM, Denis Osterland-Heim wrote:
> Hi Jacek,
> 
> Am Montag, den 16.03.2020, 19:36 +0100 schrieb Jacek Anaszewski:
>> Hi Denis,
>>
>> On 3/16/20 1:53 PM, Denis Osterland-Heim wrote:
> ...
>>>  
>>> @@ -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?
>>
> 
> well, not you, but I missed cfc4c189bc70b1acc17e6f1abf1dc1c0ae890bd8.
> Since this commit pwm_get_state() is sufficient.

I assume you tested it?

With that, for the whole set:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

-- 
Best regards,
Jacek Anaszewski

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

[Denis Osterland-Heim]

Hi Jacek,

I have testet with a i.MX6 custom board.
It is possible to use qemu to test it.
Using the smdkc210 machine with modified device tree:

```patch
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index 7c39dd1c4d3a..9ca10f69b3a2 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -26,8 +26,8 @@
 	};
 
 	chosen {
-		bootargs = "root=/dev/mmcblk0p5 rootwait earlyprintk panic=5";
-		stdout-path = "serial2:115200n8";
+		bootargs = "console=ttySAC0 rdinit=/sbin/init root=/dev/mmcblk0p5 rootwait earlyprintk panic=5";
+		stdout-path = "serial0:115200n8";
 	};
 
 	regulators {
@@ -124,7 +124,7 @@
 	fixed-rate-clocks {
 		xxti {
 			compatible = "samsung,clock-xxti";
-			clock-frequency = <0>;
+			clock-frequency = <24000000>;
 		};
 
 		xusbxti {
@@ -148,12 +148,27 @@
 		};
 	};
 
+	pwm-leds {
+		compatible = "pwm-leds";
+		led {
+			label = "led";
+			pwms = <&pwm 0 1000000000 0>;
+			max-brightness = <255>;
+			default-state = "keep";
+		};
+	};
+
+};
+
+&pwm {
+	status = "okay";
+	samsung,pwm-outputs = <0>;
 };
 
 &camera {
 	pinctrl-names = "default";
 	pinctrl-0 = <>;
-	status = "okay";
+	status = "disabled";
 };
 
 &cpu0 {
@@ -166,7 +181,7 @@
 	samsung,burst-clock-frequency = <500000000>;
 	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
-	status = "okay";
+	status = "disabled";
 
 	panel@0 {
 		reg = <0>;
@@ -196,15 +211,16 @@
 			};
 		};
 	};
+
 };
 
 &exynos_usbphy {
-	status = "okay";
+	status = "disabled";
 	vbus-supply = <&safe1_sreg>;
 };
 
 &fimc_0 {
-	status = "okay";
+	status = "disabled";
 	assigned-clocks = <&clock CLK_MOUT_FIMC0>,
 			  <&clock CLK_SCLK_FIMC0>;
 	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
@@ -212,7 +228,7 @@
 };
 
 &fimc_1 {
-	status = "okay";
+	status = "disabled";
 	assigned-clocks = <&clock CLK_MOUT_FIMC1>,
 			  <&clock CLK_SCLK_FIMC1>;
 	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
@@ -220,7 +236,7 @@
 };
 
 &fimc_2 {
-	status = "okay";
+	status = "disabled";
 	assigned-clocks = <&clock CLK_MOUT_FIMC2>,
 			  <&clock CLK_SCLK_FIMC2>;
 	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
@@ -228,7 +244,7 @@
 };
 
 &fimc_3 {
-	status = "okay";
+	status = "disabled";
 	assigned-clocks = <&clock CLK_MOUT_FIMC3>,
 			  <&clock CLK_SCLK_FIMC3>;
 	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
@@ -236,18 +252,18 @@
 };
 
 &fimd {
-	status = "okay";
+	status = "disabled";
 };
 
 &gpu {
-	status = "okay";
+	status = "disabled";
 };
 
 &hsotg {
 	vusb_d-supply = <&vusb_reg>;
 	vusb_a-supply = <&vusbdac_reg>;
 	dr_mode = "peripheral";
-	status = "okay";
+	status = "disabled";
 };
 
 &i2c_3 {
@@ -256,7 +272,7 @@
 	samsung,i2c-max-bus-freq = <400000>;
 	pinctrl-0 = <&i2c3_bus>;
 	pinctrl-names = "default";
-	status = "okay";
+	status = "disabled";
 
 	mms114-touchscreen@48 {
 		compatible = "melfas,mms114";
@@ -276,7 +292,7 @@
 	samsung,i2c-max-bus-freq = <100000>;
 	pinctrl-0 = <&i2c5_bus>;
 	pinctrl-names = "default";
-	status = "okay";
+	status = "disabled";
 
 	max8997_pmic@66 {
 		compatible = "maxim,max8997-pmic";
```

To actually see what happens on HW you need to patch qemu as well:

```patch
diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c
index 9bc03277852d..a1c2bc05b935 100644
--- a/hw/timer/exynos4210_pwm.c
+++ b/hw/timer/exynos4210_pwm.c
@@ -30,7 +30,7 @@
 
 #include "hw/arm/exynos4210.h"
 
-//#define DEBUG_PWM
+#define DEBUG_PWM
 
 #ifdef DEBUG_PWM
 #define DPRINTF(fmt, ...) \
@@ -199,8 +199,8 @@ static void exynos4210_pwm_tick(void *opaque)
     }
 
     if (cmp) {
-        DPRINTF("auto reload timer %d count to %x\n", id,
-                p->timer[id].reg_tcntb);
+        DPRINTF("auto reload timer %d count to 0x%08x and compare to 0x%08x\n", id,
+                p->timer[id].reg_tcntb, p->timer[id].reg_tcmpb);
         ptimer_set_count(p->timer[id].ptimer, p->timer[id].reg_tcntb);
         ptimer_run(p->timer[id].ptimer, 1);
     } else {
```

With this changes you can test on and off default-state.
The on state will print:
PWM: [     exynos4210_pwm_tick:  202] auto reload timer 0 count to 0x00b7d73f and compare to 0xffffffff
The off state only checks that it is disabled.

To test the default-state keep, more changes are needed:

```patch
diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c
index 9bc03277852d..a1c2bc05b935 100644
--- a/hw/timer/exynos4210_pwm.c
+++ b/hw/timer/exynos4210_pwm.c
@@ -370,6 +370,12 @@ static void exynos4210_pwm_reset(DeviceState *d)
         exynos4210_pwm_update_freq(s, s->timer[i].id);
         ptimer_stop(s->timer[i].ptimer);
     }
+
+    exynos4210_pwm_write(s,   TCON,          4, 4);
+    exynos4210_pwm_write(s, TCNTB0, 0x00b7d73f, 4);
+    exynos4210_pwm_write(s, TCMPB0, 0x005b8f57, 4);
+    exynos4210_pwm_write(s,   TCON,          6, 4);
+    exynos4210_pwm_write(s,   TCON,        0xd, 4);
 }
 
 static const MemoryRegionOps exynos4210_pwm_ops = {
```

```patch
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 87a886f7dc2f..cf578a235208 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -387,6 +387,23 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	return __pwm_samsung_config(chip, pwm, duty_ns, period_ns, false);
 }
 
+static void pwm_samsung_get_state(struct pwm_chip *chip,
+			struct pwm_device *pwm, struct pwm_state *state)
+{
+	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
+	u32 tin_rate = pwm_samsung_get_tin_rate(our_chip, pwm->hwpwm);
+	u32 tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
+	u32 tcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
+	u32 tcon = readl(our_chip->base + REG_TCON);
+	u64 tmp;
+
+	state->enabled = !!(tcon & TCON_AUTORELOAD(pwm->hwpwm));
+	tmp = NSEC_PER_SEC * (u64)tcmp;
+	state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, tin_rate);
+	tmp = NSEC_PER_SEC * (u64)tcnt;
+	state->period = DIV_ROUND_CLOSEST_ULL(tmp, tin_rate);
+}
+
 static void pwm_samsung_set_invert(struct samsung_pwm_chip *chip,
 				   unsigned int channel, bool invert)
 {
@@ -430,6 +447,7 @@ static const struct pwm_ops pwm_samsung_ops = {
 	.enable		= pwm_samsung_enable,
 	.disable	= pwm_samsung_disable,
 	.config		= pwm_samsung_config,
+	.get_state	= pwm_samsung_get_state,
 	.set_polarity	= pwm_samsung_set_polarity,
 	.owner		= THIS_MODULE,
 };
```

Without the get_state() a divide by zero is triggered and device is not there.
I think it is valid to use default-state keep with a disabled PWM
and this may cause a division by zero, I will add a `if (pwmstate->enabled)`
to avoid this.

Regards Denis

Am Dienstag, den 17.03.2020, 21:43 +0100 schrieb Jacek Anaszewski:
> Hi Denis,
> 
> On 3/16/20 9:24 PM, Denis Osterland-Heim wrote:
> > Hi Jacek,
> > 
> > Am Montag, den 16.03.2020, 19:36 +0100 schrieb Jacek Anaszewski:
> > > Hi Denis,
> > > 
> > > On 3/16/20 1:53 PM, Denis Osterland-Heim wrote:
> > 
> > ...
> > > >  
> > > > @@ -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?
> > > 
> > 
> > well, not you, but I missed cfc4c189bc70b1acc17e6f1abf1dc1c0ae890bd8.
> > Since this commit pwm_get_state() is sufficient.
> 
> I assume you tested it?
> 
> With that, for the whole set:
> 
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> 


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 v3 2/3] leds: pwm: add support for default-state device property

[Denis Osterland-Heim]

Hi Jacek,

is your ack still valid for the new versions of the patch-set?
Due to the changes I made, I am not sure.

Regards, Denis

Am Dienstag, den 17.03.2020, 21:43 +0100 schrieb Jacek Anaszewski:
> Hi Denis,
> 
> On 3/16/20 9:24 PM, Denis Osterland-Heim wrote:
> > Hi Jacek,
> > 
> > Am Montag, den 16.03.2020, 19:36 +0100 schrieb Jacek Anaszewski:
> > > Hi Denis,
> > > 
> > > On 3/16/20 1:53 PM, Denis Osterland-Heim wrote:
> > 
> > ...
> > > >  
> > > > @@ -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?
> > > 
> > 
> > well, not you, but I missed cfc4c189bc70b1acc17e6f1abf1dc1c0ae890bd8.
> > Since this commit pwm_get_state() is sufficient.
> 
> I assume you tested it?
> 
> With that, for the whole set:
> 
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> 


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 v3 2/3] leds: pwm: add support for default-state device property

[Jacek Anaszewski]

Hi Dennis,

On 6/8/20 8:32 AM, Denis Osterland-Heim wrote:
> Hi Jacek,
> 
> is your ack still valid for the new versions of the patch-set?
> Due to the changes I made, I am not sure.

Yes, you can keep it.

-- 
Best regards,
Jacek Anaszewski

[PATCH v3 3/3] 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>
---
 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.25.1



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/