Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex

["Rafał Miłecki" <zajec5@gmail.com>]

On 12.03.2021 09:23, Marek Behun wrote:
> On Fri, 12 Mar 2021 08:52:16 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
> 
>> On 12.03.2021 08:44, Marek Behun wrote:
>>> On Wed, 10 Mar 2021 08:00:25 +0100
>>> Rafał Miłecki <zajec5@gmail.com> wrote:
>>>    
>>>> 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.
> 
>> 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 ;)


> 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>;
	};
};