linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
@ 2021-03-10  7:00 Rafał Miłecki
  2021-03-12  7:44 ` Marek Behun
  2021-03-16 22:31 ` Rob Herring
  0 siblings, 2 replies; 11+ messages in thread
From: Rafał Miłecki @ 2021-03-10  7:00 UTC (permalink / raw)
  To: Pavel Machek, Dan Murphy, Rob Herring
  Cc: Jacek Anaszewski, linux-leds, devicetree, Florian Fainelli,
	Rafał Miłecki

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)":
     type: object
 
     $ref: common.yaml#
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-10  7:00 [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex Rafał Miłecki
@ 2021-03-12  7:44 ` Marek Behun
  2021-03-12  7:52   ` Rafał Miłecki
  2021-03-16 22:31 ` Rob Herring
  1 sibling, 1 reply; 11+ messages in thread
From: Marek Behun @ 2021-03-12  7:44 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Pavel Machek, Dan Murphy, Rob Herring, Jacek Anaszewski,
	linux-leds, devicetree, Florian Fainelli,
	Rafał Miłecki

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)"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-12  7:44 ` Marek Behun
@ 2021-03-12  7:52   ` Rafał Miłecki
  2021-03-12  8:23     ` Marek Behun
  0 siblings, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2021-03-12  7:52 UTC (permalink / raw)
  To: Marek Behun
  Cc: Pavel Machek, Dan Murphy, Rob Herring, Jacek Anaszewski,
	linux-leds, devicetree, Florian Fainelli,
	Rafał Miłecki

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.
2. If I use one range and +, that will allow unwanted names like "led--power"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-12  7:52   ` Rafał Miłecki
@ 2021-03-12  8:23     ` Marek Behun
  2021-03-12  9:12       ` Rafał Miłecki
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Behun @ 2021-03-12  8:23 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Pavel Machek, Dan Murphy, Rob Herring, Jacek Anaszewski,
	linux-leds, devicetree, Florian Fainelli,
	Rafał Miłecki

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".

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.

Marek

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-12  8:23     ` Marek Behun
@ 2021-03-12  9:12       ` Rafał Miłecki
  2021-03-12  9:26         ` Marek Behun
  0 siblings, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2021-03-12  9:12 UTC (permalink / raw)
  To: Marek Behun
  Cc: Pavel Machek, Dan Murphy, Rob Herring, Jacek Anaszewski,
	linux-leds, devicetree, Florian Fainelli,
	Rafał Miłecki

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-12  9:12       ` Rafał Miłecki
@ 2021-03-12  9:26         ` Marek Behun
  2021-03-12  9:42           ` Rafał Miłecki
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Behun @ 2021-03-12  9:26 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Pavel Machek, Dan Murphy, Rob Herring, Jacek Anaszewski,
	linux-leds, devicetree, Florian Fainelli,
	Rafał Miłecki

On Fri, 12 Mar 2021 10:12:26 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-12  9:26         ` Marek Behun
@ 2021-03-12  9:42           ` Rafał Miłecki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2021-03-12  9:42 UTC (permalink / raw)
  To: Marek Behun, Rob Herring
  Cc: Pavel Machek, Dan Murphy, Jacek Anaszewski, linux-leds,
	devicetree, Florian Fainelli, Rafał Miłecki

[Rob: please kindly comment on nodes numbering]

On 12.03.2021 10:26, Marek Behun wrote:
> On Fri, 12 Mar 2021 10:12:26 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
> 
>> 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>;
>> 	};
>> };
> 
> 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;
>   };

1. I found is a bit unnatural
2. Inserting/removing single LED may result in renaming nodes

Nothing really serious, I'm just wondering if that could be slightly improved.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-10  7:00 [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex Rafał Miłecki
  2021-03-12  7:44 ` Marek Behun
@ 2021-03-16 22:31 ` Rob Herring
  2021-03-16 22:44   ` Rafał Miłecki
  1 sibling, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-03-16 22:31 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Pavel Machek, Dan Murphy, Jacek Anaszewski, linux-leds,
	devicetree, Florian Fainelli, Rafał Miłecki

On Wed, Mar 10, 2021 at 08:00:25AM +0100, Rafał Miłecki 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. 

Indeed.

> 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

No, node names are supposed to be generic and reflect the class of 
device.

> 
> 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)":
>      type: object
>  
>      $ref: common.yaml#
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-16 22:31 ` Rob Herring
@ 2021-03-16 22:44   ` Rafał Miłecki
  2021-03-23 22:02     ` Rob Herring
  0 siblings, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2021-03-16 22:44 UTC (permalink / raw)
  To: Rob Herring, Rafał Miłecki
  Cc: Pavel Machek, Dan Murphy, Jacek Anaszewski, linux-leds,
	devicetree, Florian Fainelli

On 16.03.2021 23:31, Rob Herring wrote:
> On Wed, Mar 10, 2021 at 08:00:25AM +0100, Rafał Miłecki 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.
> 
> Indeed.
> 
>> 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
> 
> No, node names are supposed to be generic and reflect the class of
> device.

There was some extra discussion on this patch that has ended up with a question about numbering nodes.

Current binding assumes that nodes should be numbered with independent suffix numbers like:
led-0 { };
led-1 { };
led-2 { };

Do you think this could / should be improved somehow?

One option I was thinking about was using:
led@0 { };
led@5 { };
where numbers ("0", "5") should match GPIO numbers.

Is that a valid solution and does it improve things to make it worth it?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-16 22:44   ` Rafał Miłecki
@ 2021-03-23 22:02     ` Rob Herring
  2021-03-23 22:07       ` Rafał Miłecki
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-03-23 22:02 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Rafał Miłecki, Pavel Machek, Dan Murphy,
	Jacek Anaszewski, Linux LED Subsystem, devicetree,
	Florian Fainelli

On Tue, Mar 16, 2021 at 5:25 PM Rafał Miłecki <rafal@milecki.pl> wrote:
>
> On 16.03.2021 23:31, Rob Herring wrote:
> > On Wed, Mar 10, 2021 at 08:00:25AM +0100, Rafał Miłecki 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.
> >
> > Indeed.
> >
> >> 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
> >
> > No, node names are supposed to be generic and reflect the class of
> > device.
>
> There was some extra discussion on this patch that has ended up with a question about numbering nodes.
>
> Current binding assumes that nodes should be numbered with independent suffix numbers like:
> led-0 { };
> led-1 { };
> led-2 { };
>
> Do you think this could / should be improved somehow?

No, we have other ways for meaningful names (label, color, function, etc.).

> One option I was thinking about was using:
> led@0 { };
> led@5 { };
> where numbers ("0", "5") should match GPIO numbers.
>
> Is that a valid solution and does it improve things to make it worth it?

What if you have <gpioa 1> and <gpiob 1>?

The cells in a consumer for a provider are opaque to the consumer.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex
  2021-03-23 22:02     ` Rob Herring
@ 2021-03-23 22:07       ` Rafał Miłecki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2021-03-23 22:07 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rafał Miłecki, Pavel Machek, Dan Murphy,
	Jacek Anaszewski, Linux LED Subsystem, devicetree,
	Florian Fainelli

On 23.03.2021 23:02, Rob Herring wrote:
> On Tue, Mar 16, 2021 at 5:25 PM Rafał Miłecki <rafal@milecki.pl> wrote:
>>
>> On 16.03.2021 23:31, Rob Herring wrote:
>>> On Wed, Mar 10, 2021 at 08:00:25AM +0100, Rafał Miłecki 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.
>>>
>>> Indeed.
>>>
>>>> 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
>>>
>>> No, node names are supposed to be generic and reflect the class of
>>> device.
>>
>> There was some extra discussion on this patch that has ended up with a question about numbering nodes.
>>
>> Current binding assumes that nodes should be numbered with independent suffix numbers like:
>> led-0 { };
>> led-1 { };
>> led-2 { };
>>
>> Do you think this could / should be improved somehow?
> 
> No, we have other ways for meaningful names (label, color, function, etc.).
> 
>> One option I was thinking about was using:
>> led@0 { };
>> led@5 { };
>> where numbers ("0", "5") should match GPIO numbers.
>>
>> Is that a valid solution and does it improve things to make it worth it?
> 
> What if you have <gpioa 1> and <gpiob 1>?
> 
> The cells in a consumer for a provider are opaque to the consumer.

Thanks a lot for helping me understand that, it make sense ofc.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-03-23 23:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10  7:00 [PATCH] dt-bindings: leds: leds-gpio: fix & extend node regex Rafał Miłecki
2021-03-12  7:44 ` Marek Behun
2021-03-12  7:52   ` Rafał Miłecki
2021-03-12  8:23     ` Marek Behun
2021-03-12  9:12       ` Rafał Miłecki
2021-03-12  9:26         ` Marek Behun
2021-03-12  9:42           ` Rafał Miłecki
2021-03-16 22:31 ` Rob Herring
2021-03-16 22:44   ` Rafał Miłecki
2021-03-23 22:02     ` Rob Herring
2021-03-23 22:07       ` Rafał Miłecki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).