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.
> +
> +Required properties:
> +- compatible : "crane,el15203000"
> +- #address-cells : must be 1
> +- #size-cells : must be 0
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply. In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Optional LED sub-node properties:
> +- function:
> + see Documentation/devicetree/bindings/leds/common.txt
> +- color:
> + see Documentation/devicetree/bindings/leds/common.txt
> +- label:
> + see Documentation/devicetree/bindings/leds/common.txt (deprecated)
> +
> +Example
> +-------
> +
> +#include <dt-bindings/leds/common.h>
> +
> +led-controller@0 {
> + compatible = "crane,el15203000";
> + reg = <0>;
> + spi-max-frequency = <50000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* water pipe */
> + led@50 {
> + reg = <0x50>;
> + function = "pipe";
> + color = <LED_COLOR_ID_RED>;
> + };
> +
> + /* screen frame */
> + led@53 {
> + reg = <0x53>;
> + function = "screen";
> + color = <LED_COLOR_ID_RED>;
> + };
> +
> + /* vending area */
> + led@56 {
> + reg = <0x56>;
> + function = "vend";
> + color = <LED_COLOR_ID_RED>;
> + };
> +};
>
--
Best regards,
Jacek Anaszewski
