Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
From: Marek Behun <hidden>
Date: 2021-03-12 09:27:23
Also in:
linux-leds
On Fri, 12 Mar 2021 10:12:26 +0100 Rafał Miłecki [off-list ref] wrote:
On 12.03.2021 09:23, Marek Behun wrote:quoted
On Fri, 12 Mar 2021 08:52:16 +0100 Rafał Miłecki [off-list ref] wrote:quoted
On 12.03.2021 08:44, Marek Behun wrote:quoted
On Wed, 10 Mar 2021 08:00:25 +0100 Rafał Miłecki [off-list ref] wrote:quoted
From: Rafał Miłecki <rafal@milecki.pl> The old regex allowed only 1 character to follow the "led-" prefix which was most likely just an overlook. Fix it and while at it allow dashes in node names. It allows more meaningful names and it helpful e.g. when having the same function name with 2 different colors. For example: 1. led-power-white 2. led-power-blue Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- Documentation/devicetree/bindings/leds/leds-gpio.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.yaml b/Documentation/devicetree/bindings/leds/leds-gpio.yaml index 7ad2baeda0b0..ae46a43e480f 100644 --- a/Documentation/devicetree/bindings/leds/leds-gpio.yaml +++ b/Documentation/devicetree/bindings/leds/leds-gpio.yaml@@ -21,7 +21,7 @@ properties: patternProperties: # The first form is preferred, but fall back to just 'led' anywhere in the # node name to at least catch some child nodes. - "(^led-[0-9a-f]$|led)": + "(^led-[0-9a-f][0-9a-f-]*$|led)":Why not use +, like everywhere else? "(^led-[0-9a-f]+$|led)"1. Your regex doesn't allow dashes. I described that in commit message.Ah, I confess I did not read the commit message. My fault.quoted
2. If I use one range and +, that will allow unwanted names like "led--power"But this can happen anyway. Your regex will match for example "led-deaf------beef".You're right. I probably was overthinking that ;)quoted
Moreover you give as an example names led-power-white led-power-blue but the regex only allows hexadecimal characters, ie led-dead-beef led-1f-3 The idea is that the string after "led-" is a hexadecimal address. Names like led-power-white shouldn't be used, as far as I understand.Oops! 1. My regex was meant to be [0-9][a-z-][0-9][a-z-]+ 2. I totally missed that nodename should contain hex num and not a name My patch is based on bad binding understanding. So as I understand it now, the point of led hex number is to enumerate nodes. That way we avoid: ERROR (duplicate_node_names): /leds/led: Duplicate node name I'm just wondering if there is some cleaner solution than using those led-0, led-1, led-2, led-3, led-4 (...) names. Would that be acceptable to use address with GPIO number? Example: leds { compatible = "gpio-leds"; led@6 { gpios = <&mpc8572 6 GPIO_ACTIVE_HIGH>; color = <LED_COLOR_ID_RED>; }; led@7 { gpios = <&mpc8572 7 GPIO_ACTIVE_HIGH>; color = <LED_COLOR_ID_GREEN>; }; };
I don't know. This is a question for Rob Herring.
But why is led-0, led-1, led-2 not good enough?
You can still define function via the function property:
led-7 {
gpios = <&mpc8572 7 GPIO_ACTIVE_HIGH>;
color = <LED_COLOR_ID_GREEN>;
function = LED_FUNCTION_POWER;
};