Discussion:
[PATCH v3 1/3] leds: pwm: remove header
(too old to reply)
Denis Osterland-Heim
2020-03-16 12:53:58 UTC
Permalink
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 <***@ucw.cz>
Signed-off-by: Denis Osterland-Heim <***@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/
Denis Osterland-Heim
2020-03-16 12:53:59 UTC
Permalink
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 <***@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/
Jacek Anaszewski
2020-03-16 18:36:12 UTC
Permalink
Hi Denis,
Post by 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.
---
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
Jacek Anaszewski
2020-03-17 20:43:57 UTC
Permalink
Hi Denis,
Hi Jacek,
Post by Jacek Anaszewski
Hi Denis,
...
Post by Jacek Anaszewski
Post by Denis Osterland-Heim
@@ -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 <***@gmail.com>
--
Best regards,
Jacek Anaszewski
Denis Osterland-Heim
2020-03-16 12:54:00 UTC
Permalink
The default-state is now supported for PWM leds.

Signed-off-by: Denis Osterland-Heim <***@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/
Loading...