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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux