Hello Jacek, 19.09.19 00:02, Jacek Anaszewski пише: > Hi Oleh, > > Thank you for the update. > > I have some comments below. Please take a look. > > On 9/18/19 12:52 PM, Oleh Kravchenko wrote: >> Add documentation and example for dt-bindings EL15203000. >> LED board (aka RED LED board) from Crane Merchandising Systems. >> >> Signed-off-by: Oleh Kravchenko <oleg@xxxxxxxxxx> >> --- >> .../bindings/leds/leds-el15203000.txt | 147 ++++++++++++++++++ >> 1 file changed, 147 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-el15203000.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-el15203000.txt b/Documentation/devicetree/bindings/leds/leds-el15203000.txt >> new file mode 100644 >> index 000000000000..4a9b29cc9f46 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt >> @@ -0,0 +1,147 @@ >> +Crane Merchandising System - EL15203000 LED driver >> +-------------------------------------------------- >> + >> +This LED Board (aka RED LEDs board) is widely used in >> +coffee vending machines produced by Crane Merchandising Systems. >> +The board manages 3 LEDs and supports predefined blinking patterns >> +for specific leds. >> + >> +Vending area LED encoded with symbol 'V' (hex code 0x56). >> +Doesn't have any hardware blinking pattern. >> + >> +Screen light tube LED which surrounds vending machine screen and >> +encoded with symbol 'S' (hex code 0x53). Supports blinking breathing pattern: >> + >> + ^ >> + | >> +Max >_____***___________** >> + | * * * >> + | * * * >> + | * * * >> + | * * * >> +Min >*___________***______ >> + | >> + +------^------^------> time (sec) >> + 0 4 8 >> + >> + >> +Water Pipe LED actually consists from 5 LEDs > > "(hex code 0x50)" is here missing if you want to be consistent. > >> +that exposed by protocol like one LED. Supports next patterns: >> + >> +- cascade pattern >> + >> + ^ >> + | >> +LED0 >*****____________________*****____________________***** >> + | >> +LED1 >_____*****____________________*****____________________ >> + | >> +LED2 >__________*****____________________*****_______________ >> + | >> +LED3 >_______________*****____________________*****__________ >> + | >> +LED4 >____________________*****____________________*****_____ >> + | >> + +----^----^----^----^----^----^----^----^----^----^----> time (sec) >> + 0 0.8 1.6 2.4 3.2 4 4.8 5.6 6.4 7.2 8 >> + >> +- inversed cascade pattern >> + >> + ^ >> + | >> +LED0 >_____********************_____********************_____ >> + | >> +LED1 >*****_____********************_____******************** >> + | >> +LED2 >**********_____********************_____*************** >> + | >> +LED3 >***************_____********************_____********** >> + | >> +LED4 >********************_____********************_____***** >> + | >> + +----^----^----^----^----^----^----^----^----^----^----> time (sec) >> + 0 0.8 1.6 2.4 3.2 4 4.8 5.6 6.4 7.2 8 >> + >> +- bounce pattern >> + >> + ^ >> + | >> +LED0 >*****________________________________________*****_____ >> + | >> +LED1 >_____*****______________________________*****_____***** >> + | >> +LED2 >__________*****____________________*****_______________ >> + | >> +LED3 >_______________*****__________*****____________________ >> + | >> +LED4 >____________________**********_________________________ >> + | >> + +----^----^----^----^----^----^----^----^----^----^----> time (sec) >> + 0 0.8 1.6 2.4 3.2 4 4.8 5.6 6.4 7.2 8 >> + >> +- inversed bounce pattern >> + >> + ^ >> + | >> +LED0 >_____****************************************_____***** >> + | >> +LED1 >*****_____******************************_____*****_____ >> + | >> +LED2 >**********_____********************_____*************** >> + | >> +LED3 >***************_____**********_____******************** >> + | >> +LED4 >********************__________************************* >> + | >> + +----^----^----^----^----^----^----^----^----^----^----> time (sec) >> + 0 0.8 1.6 2.4 3.2 4 4.8 5.6 6.4 7.2 8 > > Regarding this ASCII art - I presume you wanted to follow > Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt. > > It was added to bindings because we support 'pattern' value > for linux,default-trigger, which in turn looks for 'led-pattern' > property, whose format needs to be documented in the LED bindings. > > leds-trigger-pattern.txt documents only common syntax for software > pattern engine. Currently we don't have a means to setup hw_pattern > for the pattern trigger from DT, which is obvious omission and your > patch just brings it to light. > > That said, I propose to fix it alongside and introduce led-hw-pattern > property. When present, ledtrig-pattern would setup the pattern > using pattern_set op, similarly as if it was set via sysfs hw_pattern > file. > > Only in this case placing the pattern depiction here would be justified. > Otherwise, it would have to land in the ABI documentation. You are okay, if I move it to Documentation/ABI/testing/sysfs-class-led-driver-el15203000 ? -- Best regards, Oleh Kravchenko