On 9/18/19 11:17 PM, Oleh Kravchenko wrote: > 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 ? > Yes, we can cover led-hw-pattern property later. -- Best regards, Jacek Anaszewski
