Some leds on our board are active low leds, which means these leds
are lighted when the corresponding gpio line is low, while the
original leds-gpio driver default all leds are active high leds.
This patch adds a devicetree node "light-state", whose value should
be "high" for active high leds and "low" for active low leds.
The default value is "high" for compatible for the original driver.
Signed-off-by: Song Qiang <[email protected]>
---
.../devicetree/bindings/leds/leds-gpio.txt | 15 +++++++++++
drivers/leds/leds-gpio.c | 25 +++++++++++++++++--
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.txt b/Documentation/devicetree/bindings/leds/leds-gpio.txt
index a48dda268f81..0a8fad75c704 100644
--- a/Documentation/devicetree/bindings/leds/leds-gpio.txt
+++ b/Documentation/devicetree/bindings/leds/leds-gpio.txt
@@ -23,6 +23,9 @@ LED sub-node properties:
remains up.
- panic-indicator : (optional)
see Documentation/devicetree/bindings/leds/common.txt
+- light-state: (optional) Values should be "high" or "low", which indicates
+ the state of the GPIO pin when the led is on.
+ see Documentation/devicetree/bindings/leds/common.txt
Examples:
@@ -64,3 +67,15 @@ leds {
retain-state-suspended;
};
};
+
+leds {
+ compatible = "gpio-leds";
+
+ led0 {
+ label = "led0";
+ gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ default-state = "off";
+ light-state = "low";
+ };
+}
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 764c31301f90..463ded0c71ac 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -26,9 +26,13 @@ struct gpio_led_data {
struct gpio_desc *gpiod;
u8 can_sleep;
u8 blinking;
+ u8 light_state;
gpio_blink_set_t platform_gpio_blink_set;
};
+#define LEDS_GPIO_LIGHTSTATE_HIGH 0
+#define LEDS_GPIO_LIGHTSTATE_LOW 1
+
static inline struct gpio_led_data *
cdev_to_gpio_led_data(struct led_classdev *led_cdev)
{
@@ -42,9 +46,15 @@ static void gpio_led_set(struct led_classdev *led_cdev,
int level;
if (value == LED_OFF)
- level = 0;
+ if (led_dat->light_state == LEDS_GPIO_LIGHTSTATE_HIGH)
+ level = 0;
+ else
+ level = 1;
else
- level = 1;
+ if (led_dat->light_state == LEDS_GPIO_LIGHTSTATE_HIGH)
+ level = 1;
+ else
+ level = 0;
if (led_dat->blinking) {
led_dat->platform_gpio_blink_set(led_dat->gpiod, level,
@@ -205,6 +215,17 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
led.default_state = LEDS_GPIO_DEFSTATE_OFF;
}
+ if (!fwnode_property_read_string(child, "light-state",
+ &state)) {
+ if (!strcmp(state, "low"))
+ led_dat->light_state =
+ LEDS_GPIO_LIGHTSTATE_LOW;
+ else{
+ led_dat->light_state =
+ LEDS_GPIO_LIGHTSTATE_HIGH;
+ }
+ }
+
if (fwnode_property_present(child, "retain-state-suspended"))
led.retain_state_suspended = 1;
if (fwnode_property_present(child, "retain-state-shutdown"))
--
2.17.1
Hi Song,
On 5.9.2018 04:34, Song Qiang wrote:
> Some leds on our board are active low leds, which means these leds
> are lighted when the corresponding gpio line is low, while the
> original leds-gpio driver default all leds are active high leds.
> This patch adds a devicetree node "light-state", whose value should
> be "high" for active high leds and "low" for active low leds.
> The default value is "high" for compatible for the original driver.
>
> Signed-off-by: Song Qiang <[email protected]>
> ---
> .../devicetree/bindings/leds/leds-gpio.txt | 15 +++++++++++
> drivers/leds/leds-gpio.c | 25 +++++++++++++++++--
> 2 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.txt b/Documentation/devicetree/bindings/leds/leds-gpio.txt
> index a48dda268f81..0a8fad75c704 100644
> --- a/Documentation/devicetree/bindings/leds/leds-gpio.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-gpio.txt
> @@ -23,6 +23,9 @@ LED sub-node properties:
> remains up.
> - panic-indicator : (optional)
> see Documentation/devicetree/bindings/leds/common.txt
> +- light-state: (optional) Values should be "high" or "low", which indicates
> + the state of the GPIO pin when the led is on.
> + see Documentation/devicetree/bindings/leds/common.txt
>
> Examples:
>
> @@ -64,3 +67,15 @@ leds {
> retain-state-suspended;
> };
> };
> +
> +leds {
> + compatible = "gpio-leds";
> +
> + led0 {
> + label = "led0";
> + gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
With this patch you are introducing one more way to invert the logic.
If your LED is active LOW, you should use GPIO_ACTIVE_LOW in your DT.
All should work as expected then.
Best regards,
Michal
> + linux,default-trigger = "heartbeat";
> + default-state = "off";
> + light-state = "low";
> + };
> +}
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 764c31301f90..463ded0c71ac 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -26,9 +26,13 @@ struct gpio_led_data {
> struct gpio_desc *gpiod;
> u8 can_sleep;
> u8 blinking;
> + u8 light_state;
> gpio_blink_set_t platform_gpio_blink_set;
> };
>
> +#define LEDS_GPIO_LIGHTSTATE_HIGH 0
> +#define LEDS_GPIO_LIGHTSTATE_LOW 1
> +
> static inline struct gpio_led_data *
> cdev_to_gpio_led_data(struct led_classdev *led_cdev)
> {
> @@ -42,9 +46,15 @@ static void gpio_led_set(struct led_classdev *led_cdev,
> int level;
>
> if (value == LED_OFF)
> - level = 0;
> + if (led_dat->light_state == LEDS_GPIO_LIGHTSTATE_HIGH)
> + level = 0;
> + else
> + level = 1;
> else
> - level = 1;
> + if (led_dat->light_state == LEDS_GPIO_LIGHTSTATE_HIGH)
> + level = 1;
> + else
> + level = 0;
>
> if (led_dat->blinking) {
> led_dat->platform_gpio_blink_set(led_dat->gpiod, level,
> @@ -205,6 +215,17 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
> led.default_state = LEDS_GPIO_DEFSTATE_OFF;
> }
>
> + if (!fwnode_property_read_string(child, "light-state",
> + &state)) {
> + if (!strcmp(state, "low"))
> + led_dat->light_state =
> + LEDS_GPIO_LIGHTSTATE_LOW;
> + else{
> + led_dat->light_state =
> + LEDS_GPIO_LIGHTSTATE_HIGH;
> + }
> + }
> +
> if (fwnode_property_present(child, "retain-state-suspended"))
> led.retain_state_suspended = 1;
> if (fwnode_property_present(child, "retain-state-shutdown"))
>
On Wed 2018-09-05 09:44:04, Michal Vokáč wrote:
> Hi Song,
> On 5.9.2018 04:34, Song Qiang wrote:
> >Some leds on our board are active low leds, which means these leds
> >are lighted when the corresponding gpio line is low, while the
> >original leds-gpio driver default all leds are active high leds.
> >This patch adds a devicetree node "light-state", whose value should
> >be "high" for active high leds and "low" for active low leds.
> >The default value is "high" for compatible for the original driver.
> >
> >Signed-off-by: Song Qiang <[email protected]>
> >---
> > .../devicetree/bindings/leds/leds-gpio.txt | 15 +++++++++++
> > drivers/leds/leds-gpio.c | 25 +++++++++++++++++--
> > 2 files changed, 38 insertions(+), 2 deletions(-)
> >
> >diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.txt b/Documentation/devicetree/bindings/leds/leds-gpio.txt
> >index a48dda268f81..0a8fad75c704 100644
> >--- a/Documentation/devicetree/bindings/leds/leds-gpio.txt
> >+++ b/Documentation/devicetree/bindings/leds/leds-gpio.txt
> >@@ -23,6 +23,9 @@ LED sub-node properties:
> > remains up.
> > - panic-indicator : (optional)
> > see Documentation/devicetree/bindings/leds/common.txt
> >+- light-state: (optional) Values should be "high" or "low", which indicates
> >+ the state of the GPIO pin when the led is on.
> >+ see Documentation/devicetree/bindings/leds/common.txt
> > Examples:
> >@@ -64,3 +67,15 @@ leds {
> > retain-state-suspended;
> > };
> > };
> >+
> >+leds {
> >+ compatible = "gpio-leds";
> >+
> >+ led0 {
> >+ label = "led0";
> >+ gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
>
> With this patch you are introducing one more way to invert the logic.
> If your LED is active LOW, you should use GPIO_ACTIVE_LOW in your DT.
> All should work as expected then.
Exactly, GPIO subsystem already has ways to specify active-low and
active-high.
NAK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html