2020-03-16 12:54:34

by Denis OSTERLAND-HEIM

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

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 <[email protected]>
---
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/


2020-03-16 18:36:58

by Jacek Anaszewski

[permalink] [raw]
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 <[email protected]>
> ---
> 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

2020-03-16 20:25:46

by Denis OSTERLAND-HEIM

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

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/

2020-03-17 20:44:58

by Jacek Anaszewski

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

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 <[email protected]>

--
Best regards,
Jacek Anaszewski

2020-03-19 16:02:25

by Denis OSTERLAND-HEIM

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

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 <[email protected]>
>


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/

2020-06-08 06:44:17

by Denis OSTERLAND-HEIM

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

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 <[email protected]>
>


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/

2020-06-08 19:29:20

by Jacek Anaszewski

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

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