On 03/18/2018 01:49 PM, Rob Herring wrote: > On Tue, Mar 13, 2018 at 02:45:48PM +0200, Oleh Kravchenko wrote: >> This patch adds a LED class driver for the RGB LEDs found on >> the Crane Merchandising System CR0014114 LEDs board. >> >> Signed-off-by: Oleh Kravchenko <oleg@xxxxxxxxxx> >> --- >> .../devicetree/bindings/leds/leds-cr0014114.txt | 46 ++++ >> .../devicetree/bindings/vendor-prefixes.txt | 1 + > > Please split to separate patch. > >> drivers/leds/Kconfig | 13 + >> drivers/leds/Makefile | 1 + >> drivers/leds/leds-cr0014114.c | 292 +++++++++++++++++++++ >> 5 files changed, 353 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-cr0014114.txt >> create mode 100644 drivers/leds/leds-cr0014114.c >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-cr0014114.txt b/Documentation/devicetree/bindings/leds/leds-cr0014114.txt >> new file mode 100644 >> index 000000000000..977a9929b04f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-cr0014114.txt >> @@ -0,0 +1,46 @@ >> +Crane Merchandising System - cr0014114 LED driver >> +------------------------------------------------- >> + >> +This LED Board is widely used in vending machines produced >> +by Crane Merchandising Systems. >> + >> +Required properties: >> +- compatible: "cms,cr0014114" >> +- reg: chip select address for the device >> +- spi-cpha: shifted clock phase mode is required >> + >> +LED sub-node properties: >> +- label : (optional) >> + see Documentation/devicetree/bindings/leds/common.txt >> +- linux,default-trigger : (optional) >> + see Documentation/devicetree/bindings/leds/common.txt >> + >> +Example >> +------- >> + >> +cr0014114@0 { > > leds@... In [0] you required it to be led-controller and the rest like below: led-controller@12 { reg = <12>; led@0 { reg = <0>; }; led@1 { reg = <1>; }; }; Since that time I started to require adhering to this naming pattern for LED controller node and led@address for child nodes, where applicable. I plan on submitting a patch for common LED bindings, that will switch device names to led-controller in DT examples. With this scheme another problem arises, because now we have: - label: The label for this LED. If omitted, the label is taken from the node name (excluding the unit address). With that DT child node name is useless, because it is "led" for each sub-led. Therefore I propose to make label property required, and avoid using child DT node name as a fallback. >> + compatible = "crane,cr0014114"; >> + reg = <0>; >> + spi-max-frequency = <50000>; >> + spi-cpha; >> + >> + led0 { > > Is '0' meaningful to the controller as an address/channel. If so, use > reg. > >> + label = "cr0:red:"; >> + }; >> + led1 { >> + label = "cr0:green:"; >> + }; >> + led2 { >> + label = "cr0:blue:"; >> + }; >> + led3 { >> + label = "cr1:red:"; >> + }; >> + led4 { >> + label = "cr1:green:"; >> + }; >> + led5 { >> + label = "cr1:blue:"; >> + }; >> + ... >> +}; >> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt >> index ae850d6c0ad3..f17949c365f5 100644 >> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt >> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt >> @@ -75,6 +75,7 @@ cnxt Conexant Systems, Inc. >> compulab CompuLab Ltd. >> cortina Cortina Systems, Inc. >> cosmic Cosmic Circuits >> +crane Crane Connectivity Solutions >> creative Creative Technology Ltd >> crystalfontz Crystalfontz America, Inc. >> cubietech Cubietech, Ltd. > [0] https://patchwork.kernel.org/patch/10093757/ -- Best regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html