| Message ID | 20190808203204.8614-1-oleg@kaa.org.ua |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v4,1/2] dt-bindings: Add docs for EL15203000 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
Hi Oleh, Thank you for the patch set. On 8/8/19 10:32 PM, Oleh Kravchenko wrote: > This patch adds a LED class driver for the RGB LEDs found on > the Crane Merchandising System EL15203000 LEDs board > (aka RED LEDs board). > > Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua> > --- > .../testing/sysfs-class-led-driver-el15203000 | 22 + > drivers/leds/Kconfig | 13 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-el15203000.c | 478 ++++++++++++++++++ > 4 files changed, 514 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-el15203000 > create mode 100644 drivers/leds/leds-el15203000.c > > diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 > new file mode 100644 > index 000000000000..91a483e493d9 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 > @@ -0,0 +1,22 @@ > +What: /sys/class/leds/<led>/hw_pattern > +Date: August 2019 > +KernelVersion: 5.3 > +Description: > + Specify a hardware pattern for the EL15203000 LED. > + The LEDs board supports only predefined patterns by firmware > + for specific LEDs. > + > + Breathing mode for Screen frame light tube: > + "0 4000 1 4000" > + > + Cascade mode for Pipe LED: > + "1 800 2 800 4 800 8 800 16 800 1 800 2 800 4 800 8 800 16 800" Why the sequence "1 800 2 800 4 800 8 800 16 800" is duplicated here? It seems redundant. But aside of that - aren't the timings modifiable? In other words - are these all patterns somehow configurable or they are pre-programmed? > + > + Inverted cascade mode for Pipe LED: > + "30 800 29 800 27 800 23 800 15 800 30 800 29 800 27 800 23 800 15 800" Similar duplication here. > + > + Bounce mode for Pipe LED: > + "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" Instead of two repeating "16 800" you could provide "16 1600". But here again is the question whether these values are configurable. From what I can see in your driver implementation you're expecting exactly the values provided in these examples to enable given hardware pattern (led_pattern_cmp()). > + > + Inverted bounce mode for Pipe LED: > + "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800"
Jacek Need your input below On 8/8/19 3:32 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@kaa.org.ua> > --- > .../bindings/leds/leds-el15203000.txt | 47 +++++++++++++++++++ > 1 file changed, 47 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..4c2245babfdc > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt > @@ -0,0 +1,47 @@ > +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. > + > +Required properties: > +- compatible : "crane,el15203000" > +- reg : > + see Documentation/devicetree/bindings/spi/spi-bus.txt > +- spi-max-frequency : (optional) > + see Documentation/devicetree/bindings/spi/spi-bus.txt > + > +Optional LED sub-node properties: > +- label : > + see Documentation/devicetree/bindings/leds/common.txt > +- linux,default-trigger : > + see Documentation/devicetree/bindings/leds/common.txt > + > +Example > +------- > + > +led-controller@0 { > + compatible = "crane,el15203000"; > + reg = <0>; > + spi-max-frequency = <50000>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + /* water pipe */ > + pipe@50 { > + reg = <0x50>; > + label = "red:pipe"; Should we use the color and function property here? Not sure what function would be for pipe, screen or vending but there may be comparable functions that may fit. Dan <snip>
Hello Jacek, Thank you for your useful review, 13.08.19 23:28, Jacek Anaszewski пише: > Hi Oleh, > > Thank you for the patch set. > > On 8/8/19 10:32 PM, Oleh Kravchenko wrote: >> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 >> @@ -0,0 +1,22 @@ >> +What: /sys/class/leds/<led>/hw_pattern >> +Date: August 2019 >> +KernelVersion: 5.3 >> +Description: >> + Specify a hardware pattern for the EL15203000 LED. >> + The LEDs board supports only predefined patterns by firmware >> + for specific LEDs. >> + >> + Breathing mode for Screen frame light tube: >> + "0 4000 1 4000" >> + >> + Cascade mode for Pipe LED: >> + "1 800 2 800 4 800 8 800 16 800 1 800 2 800 4 800 8 800 16 800" > > Why the sequence "1 800 2 800 4 800 8 800 16 800" is duplicated here? > It seems redundant. But aside of that - aren't the timings modifiable? > In other words - are these all patterns somehow configurable or they are > pre-programmed? > All pattern is predefined, you can't change them at all. I just tried to describe real things what happened in LED board. It's ticks every 800 milliseconds for Pipe LEDs. >> + >> + Inverted cascade mode for Pipe LED: >> + "30 800 29 800 27 800 23 800 15 800 30 800 29 800 27 800 23 800 15 800" > > Similar duplication here. > >> + >> + Bounce mode for Pipe LED: >> + "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" > > Instead of two repeating "16 800" you could provide "16 1600". > But here again is the question whether these values are configurable. > From what I can see in your driver implementation you're expecting > exactly the values provided in these examples to enable given hardware > pattern (led_pattern_cmp()). > >> + >> + Inverted bounce mode for Pipe LED: >> + "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800" > Should I cut this patterns to smaller? Or let keep it?
Hi Oleh, On 8/13/19 10:37 PM, Oleh Kravchenko wrote: > Hello Jacek, > > Thank you for your useful review, > > 13.08.19 23:28, Jacek Anaszewski пише: >> Hi Oleh, >> >> Thank you for the patch set. >> >> On 8/8/19 10:32 PM, Oleh Kravchenko wrote: >>> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 >>> @@ -0,0 +1,22 @@ >>> +What: /sys/class/leds/<led>/hw_pattern >>> +Date: August 2019 >>> +KernelVersion: 5.3 >>> +Description: >>> + Specify a hardware pattern for the EL15203000 LED. >>> + The LEDs board supports only predefined patterns by firmware >>> + for specific LEDs. >>> + >>> + Breathing mode for Screen frame light tube: >>> + "0 4000 1 4000" >>> + >>> + Cascade mode for Pipe LED: >>> + "1 800 2 800 4 800 8 800 16 800 1 800 2 800 4 800 8 800 16 800" >> >> Why the sequence "1 800 2 800 4 800 8 800 16 800" is duplicated here? >> It seems redundant. But aside of that - aren't the timings modifiable? >> In other words - are these all patterns somehow configurable or they are >> pre-programmed? >> > > All pattern is predefined, you can't change them at all. > I just tried to describe real things what happened in LED board. > It's ticks every 800 milliseconds for Pipe LEDs. It makes me wonder how you figured out the values? If you have a documentation for this controller, could you share how the pattern settings are documented? >>> + >>> + Inverted cascade mode for Pipe LED: >>> + "30 800 29 800 27 800 23 800 15 800 30 800 29 800 27 800 23 800 15 800" >> >> Similar duplication here. >> >>> + >>> + Bounce mode for Pipe LED: >>> + "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" >> >> Instead of two repeating "16 800" you could provide "16 1600". >> But here again is the question whether these values are configurable. >> From what I can see in your driver implementation you're expecting >> exactly the values provided in these examples to enable given hardware >> pattern (led_pattern_cmp()). >> >>> + >>> + Inverted bounce mode for Pipe LED: >>> + "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800" >> > > Should I cut this patterns to smaller? Or let keep it? > For the first two we could do without sequence duplication.
Hello Jacek, 14.08.19 21:18, Jacek Anaszewski пише: >>> Why the sequence "1 800 2 800 4 800 8 800 16 800" is duplicated here? >>> It seems redundant. But aside of that - aren't the timings modifiable? >>> In other words - are these all patterns somehow configurable or they are >>> pre-programmed? >>> >> >> All pattern is predefined, you can't change them at all. >> I just tried to describe real things what happened in LED board. >> It's ticks every 800 milliseconds for Pipe LEDs. > > It makes me wonder how you figured out the values? If you have > a documentation for this controller, could you share how the pattern > settings are documented? I saw the code of firmware. Not sure if I can find any documentation for it right now. >>>> + >>>> + Inverted cascade mode for Pipe LED: >>>> + "30 800 29 800 27 800 23 800 15 800 30 800 29 800 27 800 23 800 15 800" >>> >>> Similar duplication here. >>> >>>> + >>>> + Bounce mode for Pipe LED: >>>> + "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" >>> >>> Instead of two repeating "16 800" you could provide "16 1600". >>> But here again is the question whether these values are configurable. >>> From what I can see in your driver implementation you're expecting >>> exactly the values provided in these examples to enable given hardware >>> pattern (led_pattern_cmp()). >>> >>>> + >>>> + Inverted bounce mode for Pipe LED: >>>> + "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800" >>> >> >> Should I cut this patterns to smaller? Or let keep it? >> > > For the first two we could do without sequence duplication. Ok, I will reduce it.
Dan, On 8/13/19 10:31 PM, Dan Murphy wrote: > Jacek > > Need your input below > > On 8/8/19 3:32 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@kaa.org.ua> >> --- >> .../bindings/leds/leds-el15203000.txt | 47 +++++++++++++++++++ >> 1 file changed, 47 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..4c2245babfdc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt >> @@ -0,0 +1,47 @@ >> +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. >> + >> +Required properties: >> +- compatible : "crane,el15203000" >> +- reg : >> + see Documentation/devicetree/bindings/spi/spi-bus.txt >> +- spi-max-frequency : (optional) >> + see Documentation/devicetree/bindings/spi/spi-bus.txt >> + >> +Optional LED sub-node properties: >> +- label : >> + see Documentation/devicetree/bindings/leds/common.txt >> +- linux,default-trigger : >> + see Documentation/devicetree/bindings/leds/common.txt >> + >> +Example >> +------- >> + >> +led-controller@0 { >> + compatible = "crane,el15203000"; >> + reg = <0>; >> + spi-max-frequency = <50000>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + /* water pipe */ >> + pipe@50 { >> + reg = <0x50>; >> + label = "red:pipe"; > > Should we use the color and function property here? Yes, label is already deprecated in the common LED bindings in linux-next. We need separate color and function here. > Not sure what function would be for pipe, screen or vending but there > may be > > comparable functions that may fit. These are functions specific to this board and it is not something that could have some vast system-wide use. Common LED functions aim rather to unify the naming for LEDs on standard system devices and peripherals. I'd not strive for creating common LED_FUNCTION macros for these.
On 8/14/19 9:46 PM, Oleh Kravchenko wrote: > Hello Jacek, > > 14.08.19 21:18, Jacek Anaszewski пише: >>>> Why the sequence "1 800 2 800 4 800 8 800 16 800" is duplicated here? >>>> It seems redundant. But aside of that - aren't the timings modifiable? >>>> In other words - are these all patterns somehow configurable or they are >>>> pre-programmed? >>>> >>> >>> All pattern is predefined, you can't change them at all. >>> I just tried to describe real things what happened in LED board. >>> It's ticks every 800 milliseconds for Pipe LEDs. >> >> It makes me wonder how you figured out the values? If you have >> a documentation for this controller, could you share how the pattern >> settings are documented? > > I saw the code of firmware. > Not sure if I can find any documentation for it right now. Have you tried to alter the values? Or check what happens when the duplication is removed? >>>>> + >>>>> + Inverted cascade mode for Pipe LED: >>>>> + "30 800 29 800 27 800 23 800 15 800 30 800 29 800 27 800 23 800 15 800" >>>> >>>> Similar duplication here. >>>> >>>>> + >>>>> + Bounce mode for Pipe LED: >>>>> + "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" >>>> >>>> Instead of two repeating "16 800" you could provide "16 1600". >>>> But here again is the question whether these values are configurable. >>>> From what I can see in your driver implementation you're expecting >>>> exactly the values provided in these examples to enable given hardware >>>> pattern (led_pattern_cmp()). >>>> >>>>> + >>>>> + Inverted bounce mode for Pipe LED: >>>>> + "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800" >>>> >>> >>> Should I cut this patterns to smaller? Or let keep it? >>> >> >> For the first two we could do without sequence duplication. > > Ok, I will reduce it. Please hold on for a while. I will have some more remarks to the driver, just collecting missing info for now to gain more complete view on this device.
Hello Jacek, 14.08.19 22:57, Jacek Anaszewski пише: >>>> >>>> All pattern is predefined, you can't change them at all. >>>> I just tried to describe real things what happened in LED board. >>>> It's ticks every 800 milliseconds for Pipe LEDs. >>> >>> It makes me wonder how you figured out the values? If you have >>> a documentation for this controller, could you share how the pattern >>> settings are documented? >> >> I saw the code of firmware. >> Not sure if I can find any documentation for it right now. > > Have you tried to alter the values? Or check what happens when > the duplication is removed? What do you mean alter? It doesn't make any sense. Board is accepts only brightness level from '0' to '5'. I'm really confused :-) >>> >>> For the first two we could do without sequence duplication. >> >> Ok, I will reduce it. > > Please hold on for a while. I will have some more remarks to the driver, > just collecting missing info for now to gain more complete view on this > device. Here is the full story: EL15203000 LEDs board (aka RED LEDs board, because it has only RED LEDs). It's provide access to 3 LEDs: - First LED (Screen) is a light tube around big 21" screen It's have 3 brightness levels: * OFF * ON * Breathing mode (8 seconds full cycle) - Second LED (Vending area) is highlight coffee cap * OFF * ON - Third LED (Pipe) is actually virtual, because consists from 5 LEDs * OFF for all 5 LEDs * ON for all 5 LEDs * Cascade * Inverses cascade * Bounce * Inverses bounce
On 8/14/19 10:16 PM, Oleh Kravchenko wrote: > Hello Jacek, > > 14.08.19 22:57, Jacek Anaszewski пише: >>>>> >>>>> All pattern is predefined, you can't change them at all. >>>>> I just tried to describe real things what happened in LED board. >>>>> It's ticks every 800 milliseconds for Pipe LEDs. >>>> >>>> It makes me wonder how you figured out the values? If you have >>>> a documentation for this controller, could you share how the pattern >>>> settings are documented? >>> >>> I saw the code of firmware. >>> Not sure if I can find any documentation for it right now. >> >> Have you tried to alter the values? Or check what happens when >> the duplication is removed? > > What do you mean alter? It doesn't make any sense. > Board is accepts only brightness level from '0' to '5'. > I'm really confused :-) OK, I didn't analyze the driver thoroughly enough. Now everything is clear to me. Patterns are triggered just by writing two-byte command. >>>> >>>> For the first two we could do without sequence duplication. >>> >>> Ok, I will reduce it. >> >> Please hold on for a while. I will have some more remarks to the driver, >> just collecting missing info for now to gain more complete view on this >> device. > > Here is the full story: > > EL15203000 LEDs board (aka RED LEDs board, because it has only RED LEDs). > It's provide access to 3 LEDs: > > - First LED (Screen) is a light tube around big 21" screen > It's have 3 brightness levels: > * OFF > * ON > * Breathing mode (8 seconds full cycle) OK, so this is LED string. We would need to bend rules to make it appearing as a single LED class device, but allowing non-synchronous blinking of LEDs in the string. > - Second LED (Vending area) is highlight coffee cap > * OFF > * ON > - Third LED (Pipe) is actually virtual, because consists from 5 LEDs > * OFF for all 5 LEDs > * ON for all 5 LEDs > * Cascade > * Inverses cascade > * Bounce > * Inverses bounce Similarly in case of this one. I will give you a feedback on how to define the patterns within few days.
By the way, 14.08.19 23:53, Jacek Anaszewski пише: > On 8/14/19 10:16 PM, Oleh Kravchenko wrote: >> Hello Jacek, >> >> 14.08.19 22:57, Jacek Anaszewski пише: >>>>>> >>>>>> All pattern is predefined, you can't change them at all. >>>>>> I just tried to describe real things what happened in LED board. >>>>>> It's ticks every 800 milliseconds for Pipe LEDs. >>>>> >>>>> It makes me wonder how you figured out the values? If you have >>>>> a documentation for this controller, could you share how the pattern >>>>> settings are documented? >>>> >>>> I saw the code of firmware. >>>> Not sure if I can find any documentation for it right now. >>> >>> Have you tried to alter the values? Or check what happens when >>> the duplication is removed? >> >> What do you mean alter? It doesn't make any sense. >> Board is accepts only brightness level from '0' to '5'. >> I'm really confused :-) > > OK, I didn't analyze the driver thoroughly enough. > Now everything is clear to me. Patterns are triggered just > by writing two-byte command. > >>>>> >>>>> For the first two we could do without sequence duplication. >>>> >>>> Ok, I will reduce it. >>> >>> Please hold on for a while. I will have some more remarks to the driver, >>> just collecting missing info for now to gain more complete view on this >>> device. >> >> Here is the full story: >> >> EL15203000 LEDs board (aka RED LEDs board, because it has only RED LEDs). >> It's provide access to 3 LEDs: >> >> - First LED (Screen) is a light tube around big 21" screen >> It's have 3 brightness levels: >> * OFF >> * ON >> * Breathing mode (8 seconds full cycle) > > OK, so this is LED string. We would need to bend rules to make it > appearing as a single LED class device, but allowing non-synchronous > blinking of LEDs in the string. > >> - Second LED (Vending area) is highlight coffee cap >> * OFF >> * ON >> - Third LED (Pipe) is actually virtual, because consists from 5 LEDs >> * OFF for all 5 LEDs >> * ON for all 5 LEDs >> * Cascade >> * Inverses cascade >> * Bounce >> * Inverses bounce Full cycle takes 8 seconds too. So actually it's splitted into 10 stages, each takes 800ms. > Similarly in case of this one. I will give you a feedback on how > to define the patterns within few days. > Take you time.
Hi! > >>>> All pattern is predefined, you can't change them at all. > >>>> I just tried to describe real things what happened in LED board. > >>>> It's ticks every 800 milliseconds for Pipe LEDs. > >>> > >>> It makes me wonder how you figured out the values? If you have > >>> a documentation for this controller, could you share how the pattern > >>> settings are documented? > >> > >> I saw the code of firmware. > >> Not sure if I can find any documentation for it right now. > > > > Have you tried to alter the values? Or check what happens when > > the duplication is removed? > > What do you mean alter? It doesn't make any sense. > Board is accepts only brightness level from '0' to '5'. > I'm really confused :-) For the record, what you did seems ok to me. > >> Ok, I will reduce it. > > > > Please hold on for a while. I will have some more remarks to the driver, > > just collecting missing info for now to gain more complete view on this > > device. > > Here is the full story: > > EL15203000 LEDs board (aka RED LEDs board, because it has only RED LEDs). > It's provide access to 3 LEDs: > > - First LED (Screen) is a light tube around big 21" screen > It's have 3 brightness levels: > * OFF > * ON > * Breathing mode (8 seconds full cycle) > - Second LED (Vending area) is highlight coffee cap > * OFF > * ON Again, this is ok. > - Third LED (Pipe) is actually virtual, because consists from 5 LEDs > * OFF for all 5 LEDs > * ON for all 5 LEDs > * Cascade > * Inverses cascade > * Bounce > * Inverses bounce But having one virtual LED that is really five LEDs scares me a tiny bit. But I guess its ok for your case... Best regards, Pavel
Hi Oleh, On 8/8/19 10:32 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@kaa.org.ua> > --- > .../bindings/leds/leds-el15203000.txt | 47 +++++++++++++++++++ > 1 file changed, 47 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..4c2245babfdc > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt > @@ -0,0 +1,47 @@ > +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. > + > +Required properties: > +- compatible : "crane,el15203000" > +- reg : > + see Documentation/devicetree/bindings/spi/spi-bus.txt > +- spi-max-frequency : (optional) > + see Documentation/devicetree/bindings/spi/spi-bus.txt > + > +Optional LED sub-node properties: > +- label : > + see Documentation/devicetree/bindings/leds/common.txt Please change this label description to the below: - 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). > +- linux,default-trigger : > + see Documentation/devicetree/bindings/leds/common.txt > + > +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 */ > + pipe@50 { s/pipe/led/ > + reg = <0x50>; > + label = "red:pipe"; label is now deprecated. Please use function and color: function = "pipe"; color = <LED_COLOR_ID_RED>; > + }; > + > + /* screen frame */ > + screen@53 { s/screen/led/ > + reg = <0x53>; > + label = "red:screen"; function = "screen"; color = <LED_COLOR_ID_RED>; > + }; > + > + /* vending area */ > + vend@56 { s/vend/led/ > + reg = <0x56>; > + label = "red:vend"; function = "vend"; color = <LED_COLOR_ID_RED>; > + }; > +}; >
Hi Oleh, Please find my review remarks below. On 8/8/19 10:32 PM, Oleh Kravchenko wrote: > This patch adds a LED class driver for the RGB LEDs found on > the Crane Merchandising System EL15203000 LEDs board > (aka RED LEDs board). > > Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua> > --- > .../testing/sysfs-class-led-driver-el15203000 | 22 + > drivers/leds/Kconfig | 13 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-el15203000.c | 478 ++++++++++++++++++ > 4 files changed, 514 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-el15203000 > create mode 100644 drivers/leds/leds-el15203000.c > > diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 > new file mode 100644 > index 000000000000..91a483e493d9 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 > @@ -0,0 +1,22 @@ > +What: /sys/class/leds/<led>/hw_pattern > +Date: August 2019 > +KernelVersion: 5.3 > +Description: > + Specify a hardware pattern for the EL15203000 LED. > + The LEDs board supports only predefined patterns by firmware > + for specific LEDs. > + > + Breathing mode for Screen frame light tube: > + "0 4000 1 4000" > + > + Cascade mode for Pipe LED: > + "1 800 2 800 4 800 8 800 16 800 1 800 2 800 4 800 8 800 16 800" There's no point in repeating same sequence. Just require to provide it once: "1 800 2 800 4 800 8 800 16 800" > + > + Inverted cascade mode for Pipe LED: > + "30 800 29 800 27 800 23 800 15 800 30 800 29 800 27 800 23 800 15 800" The same story: "30 800 29 800 27 800 23 800 15 800" > + > + Bounce mode for Pipe LED: > + "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" > + > + Inverted bounce mode for Pipe LED: > + "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800" These can be left as they are now. > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 760f73a49c9f..0cbdb9ba5213 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -129,6 +129,19 @@ config LEDS_CR0014114 > To compile this driver as a module, choose M here: the module > will be called leds-cr0014114. > > +config LEDS_EL15203000 > + tristate "LED Support for Crane EL15203000" > + depends on LEDS_CLASS > + depends on SPI > + depends on OF > + help > + This option enables support for EL15203000 LED Board > + (aka RED LED board) which is widely used in coffee vending > + machines produced by Crane Merchandising Systems. > + > + To compile this driver as a module, choose M here: the module > + will be called leds-el15203000. > + > config LEDS_LM3530 > tristate "LCD Backlight driver for LM3530" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 1e9702ebffee..1f193ffc2feb 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -85,6 +85,7 @@ obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o > # LED SPI Drivers > obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o > obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o > +obj-$(CONFIG_LEDS_EL15203000) += leds-el15203000.o > > # LED Userspace Drivers > obj-$(CONFIG_LEDS_USER) += uleds.o > diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c > new file mode 100644 > index 000000000000..c62da5ec6630 > --- /dev/null > +++ b/drivers/leds/leds-el15203000.c > @@ -0,0 +1,478 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2019 Crane Merchandising Systems. All rights reserved. > +// Copyright (C) 2019 Oleh Kravchenko <oleg@kaa.org.ua> > + > +#include <linux/delay.h> > +#include <linux/leds.h> > +#include <linux/limits.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/spi/spi.h> > +#include <uapi/linux/uleds.h> > + > +/* > + * EL15203000 SPI protocol description: > + * +-----+------------+ > + * | LED | BRIGHTNESS | > + * +-----+------------+ > + * | 1 | 1 | > + * +-----+------------+ > + * (*) LEDs MCU board expects 20 msec delay per byte. > + * > + * LEDs: > + * +----------+--------------+-------------------------------------------+ > + * | ID | NAME | DESCRIPTION | > + * +----------+--------------+-------------------------------------------+ > + * | 'P' 0x50 | Pipe | Consists from 5 LEDs, controlled by board | > + * +----------+--------------+-------------------------------------------+ > + * | 'S' 0x53 | Screen frame | Light tube around the screen | > + * +----------+--------------+-------------------------------------------+ > + * | 'V' 0x56 | Vending area | Highlights a cup of coffee | > + * +----------+--------------+-------------------------------------------+ > + * > + * BRIGHTNESS: > + * +----------+-----------------+--------------+--------------+ > + * | VALUES | PIPE | SCREEN FRAME | VENDING AREA | > + * +----------+-----------------+--------------+--------------+ > + * | '0' 0x30 | Off | > + * +----------+-----------------------------------------------+ > + * | '1' 0x31 | On | > + * +----------+-----------------+--------------+--------------+ > + * | '2' 0x32 | Cascade | Breathing | > + * +----------+-----------------+--------------+ > + * | '3' 0x33 | Inverse cascade | > + * +----------+-----------------+ > + * | '4' 0x34 | Bounce | > + * +----------+-----------------+ > + * | '5' 0x35 | Inverse bounce | > + * +----------+-----------------+ > + */ > + > +/* EL15203000 default settings */ > +#define EL_FW_DELAY_USEC 20000 > + > +enum el15203000_brightness { Please change 'brightness' to 'command' or so. Brightness is misleading since this device supports in fact only one level of brightness for non-pattern use case. > + /* for all LEDs */ > + EL_OFF = '0', > + EL_ON = '1', > + > + /* for Screen LED */ > + EL_SCREEN_BREATHING = '2', > + > + /* for Pipe LED */ > + EL_PIPE_CASCADE = '2', > + EL_PIPE_INV_CASCADE = '3', > + EL_PIPE_BOUNCE = '4', > + EL_PIPE_INV_BOUNCE = '5', > +}; > + > +struct el15203000_led { > + struct el15203000 *priv; > + struct led_classdev ldev; > + char name[LED_MAX_NAME_SIZE]; This field will not be needed if we switch to devm_led_classdev_register_ext(). > + u8 reg; > +}; > + > +struct el15203000 { > + struct device *dev; > + struct mutex lock; > + struct spi_device *spi; > + unsigned long delay; > + size_t count; > + struct el15203000_led leds[]; > +}; > + > +static int el15203000_cmd(struct el15203000_led *led, u8 brightness) > +{ > + int ret; > + u8 cmd[2]; > + size_t i; > + unsigned long jdelay, udelay, now; > + > + mutex_lock(&led->priv->lock); > + > + dev_dbg(led->priv->dev, "Set brightness of %s to %d", > + led->name, brightness); > + > + /* to avoid SPI mistiming with firmware we should wait some time */ > + now = jiffies; > + if (time_after(led->priv->delay, now)) { > + jdelay = led->priv->delay - now; > + udelay = jiffies_to_usecs(jdelay); > + > + dev_dbg(led->priv->dev, "Wait %luus (%lu jiffies) to sync", > + udelay, jdelay); > + > + usleep_range(udelay, udelay + 1); > + } > + > + cmd[0] = led->reg; > + cmd[1] = brightness; > + > + for (i = 0; i < ARRAY_SIZE(cmd); i++) { > + if (i) > + usleep_range(EL_FW_DELAY_USEC, > + EL_FW_DELAY_USEC + 1); > + > + ret = spi_write(led->priv->spi, &cmd[i], sizeof(cmd[i])); > + if (ret) { > + dev_err(led->priv->dev, > + "spi_write() error %d", ret); > + break; > + } > + } > + > + led->priv->delay = jiffies + usecs_to_jiffies(EL_FW_DELAY_USEC); > + > + mutex_unlock(&led->priv->lock); > + > + return ret; > +} > + > +static int el15203000_set_blocking(struct led_classdev *ldev, > + enum led_brightness brightness) > +{ > + struct el15203000_led *led = container_of(ldev, > + struct el15203000_led, > + ldev); > + > + return el15203000_cmd(led, brightness == LED_OFF ? EL_OFF : EL_ON); > +} > + > +static int led_pattern_cmp(const struct led_pattern *p1, u32 p1_len, > + const struct led_pattern *p2, u32 p2_len) > +{ > + u32 i; > + > + if (p1_len != p2_len) > + return -1; > + > + for (i = 0; i < p1_len; i ++) > + if (p1[i].delta_t != p2[i].delta_t || > + p1[i].brightness != p2[i].brightness) > + return -1; > + > + return 0; > +} > + > +int el15203000_pattern_set_S(struct led_classdev *ldev, > + struct led_pattern *pattern, > + u32 len, int repeat) > +{ > + struct el15203000_led *led = container_of(ldev, > + struct el15203000_led, > + ldev); > + struct led_pattern scr_pattern[] = {{ > + .delta_t = 4000, > + .brightness = 0, > + }, { > + .delta_t = 4000, > + .brightness = 1, > + }}; > + > + if (repeat > 0) > + return -EINVAL; > + > + if (led_pattern_cmp(scr_pattern, ARRAY_SIZE(scr_pattern), pattern, len)) > + return -EINVAL; > + > + dev_dbg(led->priv->dev, "Breathing mode for '%s'", led->name); > + > + return el15203000_cmd(led, EL_SCREEN_BREATHING); > +} > + > +int el15203000_pattern_set_P(struct led_classdev *ldev, > + struct led_pattern *pattern, > + u32 len, int repeat) > +{ > + struct el15203000_led *led = container_of(ldev, > + struct el15203000_led, > + ldev); > + > + struct led_pattern cascade[] = {{ > + .delta_t = 800, > + .brightness = 1, > + }, { > + .delta_t = 800, > + .brightness = 2, > + }, { > + .delta_t = 800, > + .brightness = 4, > + }, { > + .delta_t = 800, > + .brightness = 8, > + }, { > + .delta_t = 800, > + .brightness = 16, > + }, { > + .delta_t = 800, > + .brightness = 1, > + }, { > + .delta_t = 800, > + .brightness = 2, > + }, { > + .delta_t = 800, > + .brightness = 4, > + }, { > + .delta_t = 800, > + .brightness = 8, > + }, { > + .delta_t = 800, > + .brightness = 16, > + }}; > + > + struct led_pattern inv_cascade[] = {{ > + .delta_t = 800, > + .brightness = 30, > + }, { > + .delta_t = 800, > + .brightness = 29, > + }, { > + .delta_t = 800, > + .brightness = 27, > + }, { > + .delta_t = 800, > + .brightness = 23, > + }, { > + .delta_t = 800, > + .brightness = 15, > + }, { > + .delta_t = 800, > + .brightness = 30, > + }, { > + .delta_t = 800, > + .brightness = 29, > + }, { > + .delta_t = 800, > + .brightness = 27, > + }, { > + .delta_t = 800, > + .brightness = 23, > + }, { > + .delta_t = 800, > + .brightness = 15, > + }}; > + > + struct led_pattern bounce[] = {{ > + .delta_t = 800, > + .brightness = 1, > + }, { > + .delta_t = 800, > + .brightness = 2, > + }, { > + .delta_t = 800, > + .brightness = 4, > + }, { > + .delta_t = 800, > + .brightness = 8, > + }, { > + .delta_t = 800, > + .brightness = 16, > + }, { > + .delta_t = 800, > + .brightness = 16, > + }, { > + .delta_t = 800, > + .brightness = 8, > + }, { > + .delta_t = 800, > + .brightness = 4, > + }, { > + .delta_t = 800, > + .brightness = 2, > + }, { > + .delta_t = 800, > + .brightness = 1, > + }}; > + > + struct led_pattern inv_bounce[] = {{ > + .delta_t = 800, > + .brightness = 30, > + }, { > + .delta_t = 800, > + .brightness = 29, > + }, { > + .delta_t = 800, > + .brightness = 27, > + }, { > + .delta_t = 800, > + .brightness = 23, > + }, { > + .delta_t = 800, > + .brightness = 15, > + }, { > + .delta_t = 800, > + .brightness = 15, > + }, { > + .delta_t = 800, > + .brightness = 23, > + }, { > + .delta_t = 800, > + .brightness = 27, > + }, { > + .delta_t = 800, > + .brightness = 29, > + }, { > + .delta_t = 800, > + .brightness = 30, > + }}; There is no need for these arrays. We can come up with formulas for validating the pattern tuples. Basically all of them involve differences in brightness being powers of two: "1 800 2 800 4 800 8 800 16 800" Here each next step should be equal to (1 << i) staring from i = 0. "30 800 29 800 27 800 23 800 15 800" brightness = 30; for (i = 0; i < pattern_len; i++) { br_diff = (i > 0) ? (1 << (i - 1)) : 0; brightness -= br_diff; } "1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800" "30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800" Above two can be handled by slightly modifying what I proposed above. And for delta_t just require by default always 800. > + if (repeat > 0) > + return -EINVAL; > + > + if (!led_pattern_cmp(cascade, ARRAY_SIZE(cascade), pattern, len)) { > + dev_dbg(led->priv->dev, "Cascade mode for '%s'", led->name); > + return el15203000_cmd(led, EL_PIPE_CASCADE); > + } else if (!led_pattern_cmp(inv_cascade, ARRAY_SIZE(inv_cascade), pattern, len)) { > + dev_dbg(led->priv->dev, "Inverse cascade mode for '%s'", led->name); > + return el15203000_cmd(led, EL_PIPE_INV_CASCADE); > + } else if (!led_pattern_cmp(bounce, ARRAY_SIZE(bounce), pattern, len)) { > + dev_dbg(led->priv->dev, "Bounce mode for '%s'", led->name); > + return el15203000_cmd(led, EL_PIPE_BOUNCE); > + } else if (!led_pattern_cmp(inv_bounce, ARRAY_SIZE(inv_bounce), pattern, len)) { > + dev_dbg(led->priv->dev, "Inverse bounce mode for '%s'", led->name); > + return el15203000_cmd(led, EL_PIPE_INV_BOUNCE); > + } > + > + return -EINVAL; > +} > + > +int el15203000_pattern_clear(struct led_classdev *ldev) > +{ > + struct el15203000_led *led = container_of(ldev, > + struct el15203000_led, > + ldev); > + > + return el15203000_cmd(led, EL_OFF); > +} > + > +static int el15203000_probe_dt(struct el15203000 *priv) > +{ > + size_t i = 0; > + struct el15203000_led *led; > + struct fwnode_handle *child; > + int ret; > + const char *str; > + u32 reg; > + > + device_for_each_child_node(priv->dev, child) { struct led_init_data init_data = {}; Please add above declaration here. > + led = &priv->leds[i]; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret) { > + dev_err(priv->dev, "LED without ID number"); > + fwnode_handle_put(child); > + > + return ret; > + } > + > + if (reg > U8_MAX) { > + dev_err(priv->dev, "LED value %d is invalid", reg); > + fwnode_handle_put(child); > + > + return -EINVAL; > + } > + > + led->reg = reg; > + > + ret = fwnode_property_read_string(child, "label", &str); > + if (ret) > + str = ":"; > + snprintf(led->name, sizeof(led->name), "el15203000:%s", str); And instead of the four above lines please just do the below assignment: init_data.fwnode = child; but let's do it right before calling devm_led_classdev_register_ext(). > + > + fwnode_property_read_string(child, "linux,default-trigger", > + &led->ldev.default_trigger); > + > + led->ldev.max_brightness = LED_ON; > + led->priv = priv; > + led->ldev.name = led->name; Please also remove the above line. > + led->ldev.brightness_set_blocking = el15203000_set_blocking; > + > + if (reg == 'S') { > + led->ldev.pattern_set = el15203000_pattern_set_S; > + led->ldev.pattern_clear = el15203000_pattern_clear; > + } else if (reg == 'P') { > + led->ldev.pattern_set = el15203000_pattern_set_P; > + led->ldev.pattern_clear = el15203000_pattern_clear; > + } > + > + ret = devm_led_classdev_register(priv->dev, &led->ldev); And here let's switch to the new LED registration API: ret = devm_led_classdev_register_ext(priv->dev, &led->ldev, &init_data); > + if (ret) { > + dev_err(priv->dev, > + "failed to register LED device %s, err %d", > + led->name, ret); > + fwnode_handle_put(child); > + return ret; > + } > + > + i++; > + } > + > + return ret; > +} > + > +static int el15203000_probe(struct spi_device *spi) > +{ > + struct el15203000 *priv; > + size_t count; > + int ret; > + > + count = device_get_child_node_count(&spi->dev); > + if (!count) { > + dev_err(&spi->dev, "LEDs are not defined in device tree!"); > + return -ENODEV; > + } > + > + priv = devm_kzalloc(&spi->dev, struct_size(priv, leds, count), > + GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + mutex_init(&priv->lock); > + priv->count = count; > + priv->dev = &spi->dev; > + priv->spi = spi; > + priv->delay = jiffies - > + usecs_to_jiffies(EL_FW_DELAY_USEC); > + > + ret = el15203000_probe_dt(priv); > + if (ret) > + return ret; > + > + spi_set_drvdata(spi, priv); > + dev_dbg(priv->dev, "%zd LEDs registered", priv->count); Please remove this debug log. > + > + return 0; > +} > + > +static int el15203000_remove(struct spi_device *spi) > +{ > + struct el15203000 *priv = spi_get_drvdata(spi); > + > + mutex_destroy(&priv->lock); > + > + return 0; > +} > + > +static const struct of_device_id el15203000_dt_ids[] = { > + { .compatible = "crane,el15203000", }, > + {}, > +}; > + > +MODULE_DEVICE_TABLE(of, el15203000_dt_ids); > + > +static struct spi_driver el15203000_driver = { > + .probe = el15203000_probe, > + .remove = el15203000_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = el15203000_dt_ids, > + }, > +}; > + > +module_spi_driver(el15203000_driver); > + > +MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>"); > +MODULE_DESCRIPTION("el15203000 LED driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("spi:el15203000"); > Please also fix the issues checkpatch.pl complains about.
Hello Pavel, 17.08.19 18:02, Pavel Machek пише: > Hi! > >>>>>> All pattern is predefined, you can't change them at all. >>>>>> I just tried to describe real things what happened in LED board. >>>>>> It's ticks every 800 milliseconds for Pipe LEDs. >>>>> >>>>> It makes me wonder how you figured out the values? If you have >>>>> a documentation for this controller, could you share how the pattern >>>>> settings are documented? >>>> >>>> I saw the code of firmware. >>>> Not sure if I can find any documentation for it right now. >>> >>> Have you tried to alter the values? Or check what happens when >>> the duplication is removed? >> >> What do you mean alter? It doesn't make any sense. >> Board is accepts only brightness level from '0' to '5'. >> I'm really confused :-) > > For the record, what you did seems ok to me. Thanks :) >>>> Ok, I will reduce it. >>> >>> Please hold on for a while. I will have some more remarks to the driver, >>> just collecting missing info for now to gain more complete view on this >>> device. >> >> Here is the full story: >> >> EL15203000 LEDs board (aka RED LEDs board, because it has only RED LEDs). >> It's provide access to 3 LEDs: >> >> - First LED (Screen) is a light tube around big 21" screen >> It's have 3 brightness levels: >> * OFF >> * ON >> * Breathing mode (8 seconds full cycle) >> - Second LED (Vending area) is highlight coffee cap >> * OFF >> * ON > > Again, this is ok. > >> - Third LED (Pipe) is actually virtual, because consists from 5 LEDs >> * OFF for all 5 LEDs >> * ON for all 5 LEDs >> * Cascade >> * Inverses cascade >> * Bounce >> * Inverses bounce > > But having one virtual LED that is really five LEDs scares me a tiny > bit. But I guess its ok for your case... > > Best regards, > Pavel > > > >
Hello Jacek, thanks for your review. 18.08.19 16:41, Jacek Anaszewski пише: > Hi Oleh, > > On 8/8/19 10:32 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@kaa.org.ua> >> --- >> .../bindings/leds/leds-el15203000.txt | 47 +++++++++++++++++++ >> 1 file changed, 47 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..4c2245babfdc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt >> @@ -0,0 +1,47 @@ >> +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. >> + >> +Required properties: >> +- compatible : "crane,el15203000" >> +- reg : >> + see Documentation/devicetree/bindings/spi/spi-bus.txt >> +- spi-max-frequency : (optional) >> + see Documentation/devicetree/bindings/spi/spi-bus.txt >> + >> +Optional LED sub-node properties: >> +- label : >> + see Documentation/devicetree/bindings/leds/common.txt > > Please change this label description to the below: > > - 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). > >> +- linux,default-trigger : >> + see Documentation/devicetree/bindings/leds/common.txt >> + >> +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 */ >> + pipe@50 { > > s/pipe/led/ > >> + reg = <0x50>; >> + label = "red:pipe"; > > label is now deprecated. > > Please use function and color: > > function = "pipe"; > color = <LED_COLOR_ID_RED>; > > >> + }; >> + >> + /* screen frame */ >> + screen@53 { > > s/screen/led/ > >> + reg = <0x53>; >> + label = "red:screen"; > > function = "screen"; > color = <LED_COLOR_ID_RED>; > >> + }; >> + >> + /* vending area */ >> + vend@56 { > > s/vend/led/ > >> + reg = <0x56>; >> + label = "red:vend"; > > function = "vend"; > color = <LED_COLOR_ID_RED>; > >> + }; >> +}; >> >
diff --git a/Documentation/devicetree/bindings/leds/leds-el15203000.txt b/Documentation/devicetree/bindings/leds/leds-el15203000.txt new file mode 100644 index 000000000000..4c2245babfdc --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt @@ -0,0 +1,47 @@ +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. + +Required properties: +- compatible : "crane,el15203000" +- reg : + see Documentation/devicetree/bindings/spi/spi-bus.txt +- spi-max-frequency : (optional) + see Documentation/devicetree/bindings/spi/spi-bus.txt + +Optional LED sub-node properties: +- label : + see Documentation/devicetree/bindings/leds/common.txt +- linux,default-trigger : + see Documentation/devicetree/bindings/leds/common.txt + +Example +------- + +led-controller@0 { + compatible = "crane,el15203000"; + reg = <0>; + spi-max-frequency = <50000>; + #address-cells = <1>; + #size-cells = <0>; + + /* water pipe */ + pipe@50 { + reg = <0x50>; + label = "red:pipe"; + }; + + /* screen frame */ + screen@53 { + reg = <0x53>; + label = "red:screen"; + }; + + /* vending area */ + vend@56 { + reg = <0x56>; + label = "red:vend"; + }; +};
Add documentation and example for dt-bindings EL15203000. LED board (aka RED LED board) from Crane Merchandising Systems. Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua> --- .../bindings/leds/leds-el15203000.txt | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-el15203000.txt