Skip to contentSkip to conversation

dt-bindings: leds: register-bit-led: Add active-low property

PATCH Superseded Pali Rohár 4 participants 15 messages Aug 18, 2022 – Nov 8, 2022

Subsystems (2)
Device tree bindings LED
lore
Share & tools
lore

Pali Rohár

author
#0 lore raw

PATCH 1/2 dt-bindings: leds: register-bit-led: Add active-low property

Allow to define inverted logic (0 - enable LED, 1 - disable LED) via active-low property.

Signed-off-by Pali Rohár <[email protected]>
---
 Documentation/devicetree/bindings/leds/register-bit-led.yaml | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/leds/register-bit-led.yaml +5 -0
index 79b8fc0f9d23..5c6ef26f1a94 100644
@@ -43,6 +43,11 @@ properties:
         0x100000, 0x200000, 0x400000, 0x800000, 0x1000000, 0x2000000, 0x4000000,
         0x8000000, 0x10000000, 0x20000000, 0x40000000, 0x80000000 ]
 
+  active-low:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      LED is ON when bit in register is not set
+
   offset:
     description:
       register offset to the register controlling this LED
Signature · 1 line
2.20.1
author
#1 lore raw

PATCH 2/2 leds: syscon: Implement support for active-low property

This new active-low property specify that LED has inverted logic (0 - enable LED, 1 - disable LED).

Signed-off-by Pali Rohár <[email protected]>
---
 drivers/leds/leds-syscon.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/leds/leds-syscon.c +10 -4
index 7eddb8ecb44e..5e605d8438e9 100644
@@ -29,6 +29,7 @@ struct syscon_led {
 	struct regmap *map;
 	u32 offset;
 	u32 mask;
+	bool active_low;
 	bool state;
 };
 

@@ -41,10 +42,10 @@ static void syscon_led_set(struct led_classdev *led_cdev,
 	int ret;
 
 	if (value == LED_OFF) {
-		val = 0;
+		val = sled->active_low ? sled->mask : 0;
 		sled->state = false;
 	} else {
-		val = sled->mask;
+		val = sled->active_low ? 0 : sled->mask;
 		sled->state = true;
 	}
 

@@ -85,6 +86,8 @@ static int syscon_led_probe(struct platform_device *pdev)
 		return -EINVAL;
 	if (of_property_read_u32(np, "mask", &sled->mask))
 		return -EINVAL;
+	if (of_find_property(np, "active-low", NULL))
+		sled->active_low = true;
 
 	state = of_get_property(np, "default-state", NULL);
 	if (state) {

@@ -95,17 +98,20 @@ static int syscon_led_probe(struct platform_device *pdev)
 			if (ret < 0)
 				return ret;
 			sled->state = !!(val & sled->mask);
+			if (sled->active_low)
+				sled->state = !sled->state;
 		} else if (!strcmp(state, "on")) {
 			sled->state = true;
 			ret = regmap_update_bits(map, sled->offset,
 						 sled->mask,
-						 sled->mask);
+						 sled->active_low ? 0 : sled->mask);
 			if (ret < 0)
 				return ret;
 		} else {
 			sled->state = false;
 			ret = regmap_update_bits(map, sled->offset,
-						 sled->mask, 0);
+						 sled->mask,
+						 sled->active_low ? sled->mask : 0);
 			if (ret < 0)
 				return ret;
 		}
Signature · 1 line
2.20.1
author
#3 lore raw

Re: [PATCH 2/2] leds: syscon: Implement support for active-low property

Quoted 11 lines · On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
Quoted 4 lines · Pali Rohár

This new active-low property specify that LED has inverted logic (0 - enable LED, 1 - disable LED).

Signed-off-by Pali Rohár <[email protected]>
Acked-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Is something else needed for this patch series?

author
#4 lore raw

Re: [PATCH 2/2] leds: syscon: Implement support for active-low property

Quoted 6 lines · On Wednesday 31 August 2022 13:49:36 Pali Rohár wrote:
Quoted 5 lines · On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
Quoted 3 lines · Pali Rohár

This new active-low property specify that LED has inverted logic (0 - enable LED, 1 - disable LED).

Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Pali Rohár <[email protected]>
Acked-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Is something else needed for this patch series?

PING?

author
#5 lore raw

Re: [PATCH 2/2] leds: syscon: Implement support for active-low property

Quoted 9 lines · On Sunday 09 October 2022 13:40:43 Pali Rohár wrote:
Quoted 8 lines · On Wednesday 31 August 2022 13:49:36 Pali Rohár wrote:
Quoted 7 lines · On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
Quoted 4 lines · Pali Rohár

This new active-low property specify that LED has inverted logic (0 - enable LED, 1 - disable LED).

Signed-off-by Pali Rohár <[email protected]>
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Acked-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Is something else needed for this patch series?

PING?

PING?

#6 lore raw

Re: [PATCH 2/2] leds: syscon: Implement support for active-low property

Quoted 12 lines · Pali Rohár
Quoted 11 lines · On Sunday 09 October 2022 13:40:43 Pali Rohár wrote:
Quoted 10 lines · On Wednesday 31 August 2022 13:49:36 Pali Rohár wrote:
Quoted 9 lines · On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
Quoted 4 lines · Pali Rohár

This new active-low property specify that LED has inverted logic (0 - enable LED, 1 - disable LED).

Signed-off-by Pali Rohár <[email protected]>
Acked-by Linus Walleij <[email protected]>
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Yours,
Linus Walleij

Is something else needed for this patch series?

PING?

PING?

If the LED subsystem isn't working consider sending the patch to the SoC tree with a notice that they apply it instead and why.

Yours,
Linus Walleij

Reviewed-by on v1 1/2
#7 lore raw
In reply to Pali Rohár
Quoted 13 lines · Pali Rohár

Allow to define inverted logic (0 - enable LED, 1 - disable LED) via active-low property.

Signed-off-by Pali Rohár <[email protected]>
---
 Documentation/devicetree/bindings/leds/register-bit-led.yaml | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/leds/register-bit-led.yaml +0 -0
index 79b8fc0f9d23..5c6ef26f1a94 100644

Documentation/devicetree/bindings/leds/register-bit-led.yaml properties: · line 47

Quoted 5 lines
Documentation/devicetree/bindings/leds/register-bit-led.yaml +2 -0
@@ -43,6 +43,11 @@ properties:
         0x100000, 0x200000, 0x400000, 0x800000, 0x1000000, 0x2000000, 0x4000000,
         0x8000000, 0x10000000, 0x20000000, 0x40000000, 0x80000000 ]
 
+  active-low:
+    $ref: /schemas/types.yaml#/definitions/flag

This could be shorter: type: boolean

Reviewed-by Krzysztof Kozlowski <[email protected]>

Although the question is - where is the user of it?

Best regards,
Krzysztof

author
#8 lore raw
Quoted 1 line · On Friday 19 August 2022 09:46:56 Krzysztof Kozlowski wrote:

Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these register-bit-led definitions after accepting dt bindings.

#9 lore raw

Hi!

Quoted 4 lines
Quoted 1 line

Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these register-bit-led definitions after accepting dt bindings.

We need device tree people to ack them, first. But a note saying "this is for Turris Omnia router" would be welcome.

Best regards,
Pavel

Signature · 1 line
People of Russia, stop Putin before his war on Ukraine escalates.
#10 lore raw
Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines
Quoted 1 line

Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these register-bit-led definitions after accepting dt bindings.

We need device tree people to ack them, first. But a note saying "this is for Turris Omnia router" would be welcome.

In general the process is one of:
1. Send DT bindings with driver and DTS changes,
2. Send DT bindings with driver in one patchset, DTS in second but you mention the dependency.

You should not wait with DTS till bindings got accepted. Why? Because for example we do not want bindings for stuff which never is going to be upstreamed (with several exceptions, e.g. for other systems). Also because we want to be able to compare bindings with your DTS implementing them, so we are sure you described everything (especially that you said running one command to install dtchema and second command to make the check is not possible in your system).

Without DTS here how can anyone be sure your DTS actually follows the bindings?

Best regards,
Krzysztof

author
#11 lore raw
Quoted 24 lines · On Friday 19 August 2022 11:38:29 Krzysztof Kozlowski wrote:
Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines
Quoted 1 line

Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these register-bit-led definitions after accepting dt bindings.

We need device tree people to ack them, first. But a note saying "this is for Turris Omnia router" would be welcome.

In general the process is one of:
1. Send DT bindings with driver and DTS changes,
2. Send DT bindings with driver in one patchset, DTS in second but you mention the dependency.

You should not wait with DTS till bindings got accepted. Why? Because for example we do not want bindings for stuff which never is going to be upstreamed (with several exceptions, e.g. for other systems). Also because we want to be able to compare bindings with your DTS implementing them, so we are sure you described everything (especially that you said running one command to install dtchema and second command to make the check is not possible in your system).

Quoted 5 lines

Without DTS here how can anyone be sure your DTS actually follows the bindings?

Best regards,
Krzysztof

Well, last time I was told that first needs to be accepted bindings documentation and then device tree files. So I did it like this. And now it is again feasible and different steps and ordering is needed... Sorry I cannot known all requirements which are moreover changing every day.

#12 lore raw
Quoted 28 lines · Pali Rohár
Quoted 27 lines · On Friday 19 August 2022 11:38:29 Krzysztof Kozlowski wrote:
Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines
Quoted 1 line

Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these register-bit-led definitions after accepting dt bindings.

We need device tree people to ack them, first. But a note saying "this is for Turris Omnia router" would be welcome.

In general the process is one of:
1. Send DT bindings with driver and DTS changes,
2. Send DT bindings with driver in one patchset, DTS in second but you mention the dependency.

You should not wait with DTS till bindings got accepted. Why? Because for example we do not want bindings for stuff which never is going to be upstreamed (with several exceptions, e.g. for other systems). Also because we want to be able to compare bindings with your DTS implementing them, so we are sure you described everything (especially that you said running one command to install dtchema and second command to make the check is not possible in your system).

Without DTS here how can anyone be sure your DTS actually follows the bindings?

Quoted 8 lines
Quoted 2 lines

Best regards,
Krzysztof

Well, last time I was told that first needs to be accepted bindings documentation and then device tree files. So I did it like this. And now it is again feasible and different steps and ordering is needed... Sorry I cannot known all requirements which are moreover changing every day.

The rule is the same from years and no one was changing it. All driver submitters, all DTS developers follow this. You are literally the one of very few which is surprised by that generic rule and call it "a change". The patches should be accepted by maintainers in such order (so without bindings acceptance, the driver and DTS should not go), but not the sending order.

It's the same with sysfs ABI. Exactly the same.

Best regards,
Krzysztof

author
#13 lore raw
In reply to Pali Rohár
Quoted 28 lines · On Friday 19 August 2022 10:42:22 Pali Rohár wrote:
Quoted 27 lines · On Friday 19 August 2022 11:38:29 Krzysztof Kozlowski wrote:
Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines
Quoted 1 line

Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these register-bit-led definitions after accepting dt bindings.

We need device tree people to ack them, first. But a note saying "this is for Turris Omnia router" would be welcome.

In general the process is one of:
1. Send DT bindings with driver and DTS changes,
2. Send DT bindings with driver in one patchset, DTS in second but you mention the dependency.

You should not wait with DTS till bindings got accepted. Why? Because for example we do not want bindings for stuff which never is going to be upstreamed (with several exceptions, e.g. for other systems). Also because we want to be able to compare bindings with your DTS implementing them, so we are sure you described everything (especially that you said running one command to install dtchema and second command to make the check is not possible in your system).

Without DTS here how can anyone be sure your DTS actually follows the bindings?

Quoted 8 lines
Quoted 2 lines

Best regards,
Krzysztof

Well, last time I was told that first needs to be accepted bindings documentation and then device tree files. So I did it like this. And now it is again feasible and different steps and ordering is needed... Sorry I cannot known all requirements which are moreover changing every day.

Here it is:
https://lore.kernel.org/linuxppc-dev/[email protected]/