| Message ID | 1406294628-16079-3-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 25, 2014 at 8:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > This adds the device tree bindings used by syscon-based LEDs. > > Cc: devicetree@vger.kernel.org > Cc: Bryan Wu <cooloney@gmail.com> > Cc: Richard Purdie <rpurdie@rpsys.net> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > .../devicetree/bindings/leds/leds-syscon.txt | 83 ++++++++++++++++++++++ > 1 file changed, 83 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-syscon.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-syscon.txt b/Documentation/devicetree/bindings/leds/leds-syscon.txt > new file mode 100644 > index 000000000000..460b9c3d1bd3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-syscon.txt > @@ -0,0 +1,83 @@ > +Device Tree Bindings for Syscon LEDs > + > +Required properties: > +- compatible : must be "syscon-leds". Not really happy with the name... More below. > +- regmap : a phandle to a syscon node containing a regmap > + > +Each LED is represented as a sub-node of the syscon-leds device. Each > +node's name represents the name of the corresponding LED. > + > +LED sub-node properties: > +- offset : register offset to the register controlling this LED > +- mask : bit mask for the bit controlling this LED in the register > + typically 0x01, 0x02, 0x04 ... This would be a single bit, right? What about inverted bits (i.e. 0 is on or 1 is on)? > +- label : (optional) Please group all required and optional properties under those headings. > + see Documentation/devicetree/bindings/leds/common.txt > +- linux,default-trigger : (optional) > + see Documentation/devicetree/bindings/leds/common.txt > +- default-state: (optional) The initial state of the LED. Valid > + values are "on", "off", and "keep". If the LED is already on or off > + and the default-state property is set the to same value, then no > + glitch should be produced where the LED momentarily turns off (or > + on). The "keep" setting will keep the LED at whatever its current > + state is, without producing a glitch. The default is off if this > + property is not present. > + > +Example: > + > +leds: leds@08 { > + compatible = "syscon-leds"; > + regmap = <&syscon>; > + > + led@bit0 { Perhaps we can define a way to express unit address as offset+bit like <offset>_<bit> or <offset>.<bit>. I think we should get rid of the leds node and put this within the syscon device node and each node here should have a compatible property. I think the compatible should be something like "register-bit-led" (perhaps someone has a better name) as syscon is somewhat linux specific term and you could use this binding for any LEDs that have a single register bit control. Rob > + offset = <0x08>; > + mask = <0x01>; > + label = "versatile:0"; > + linux,default-trigger = "heartbeat"; > + default-state = "on"; > + }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 25, 2014 at 4:07 PM, Rob Herring <robh@kernel.org> wrote: > On Fri, Jul 25, 2014 at 8:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> This adds the device tree bindings used by syscon-based LEDs. (...) >> +Device Tree Bindings for Syscon LEDs >> + >> +Required properties: >> +- compatible : must be "syscon-leds". > > Not really happy with the name... More below. [copy paste] > I think the compatible should be something like > "register-bit-led" (perhaps someone has a better name) as syscon is > somewhat linux specific term and you could use this binding for any > LEDs that have a single register bit control. Hm... this driver: drivers/gpio/gpio-syscon.c Has some bindings specific to a certain controller here: Documentation/devicetree/bindings/gpio/cirrus,clps711x-mctrl-gpio.txt Maybe I should think of something generic for that one along the same lines then, so we get some consensus on this. Documentation/devicetree/bindings/mfd/syscon.txt Specifies the string "syscon" for such regmap ranges, maybe it should rather use the compatible string "misc-register-range" or something. >> +LED sub-node properties: >> +- offset : register offset to the register controlling this LED >> +- mask : bit mask for the bit controlling this LED in the register >> + typically 0x01, 0x02, 0x04 ... > > This would be a single bit, right? What about inverted bits (i.e. 0 is > on or 1 is on)? I can of course add inversion bindings, but cannot add it to the driver as I can't test it. Just an "active-low;" string would do I guess. >> +- label : (optional) > > Please group all required and optional properties under those headings. OK. >> +Example: >> + >> +leds: leds@08 { >> + compatible = "syscon-leds"; >> + regmap = <&syscon>; >> + >> + led@bit0 { > > Perhaps we can define a way to express unit address as offset+bit like > <offset>_<bit> or <offset>.<bit>. I'll come up with something. We don't really have a place to document such schemas do we? > I think we should get rid of the leds node and put this within the > syscon device node and each node here should have a compatible > property. OK. But then I guess you also expect to get rid of the phandle <&syscon> from the node, so that it implicitly uses the syscon it's embedded in? Or is it OK to keep? Or should it be optional, so the (syscon) driver will look upward when locating the syscon for a node? Like I need to add a new function syscon_regmap_lookup_by_hierarchy() to syscon.c... It already has like three ways to look up syscons, now we add another one. I know that is a Linux implementation detail but it reflects the fact that similar code will be needed in all operating systems using syscon-type things, if it's a case-by-case question whether a phandle or hierarchy traversal is to be used for getting hold of the interface. Your, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/leds/leds-syscon.txt b/Documentation/devicetree/bindings/leds/leds-syscon.txt new file mode 100644 index 000000000000..460b9c3d1bd3 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-syscon.txt @@ -0,0 +1,83 @@ +Device Tree Bindings for Syscon LEDs + +Required properties: +- compatible : must be "syscon-leds". +- regmap : a phandle to a syscon node containing a regmap + +Each LED is represented as a sub-node of the syscon-leds device. Each +node's name represents the name of the corresponding LED. + +LED sub-node properties: +- offset : register offset to the register controlling this LED +- mask : bit mask for the bit controlling this LED in the register + typically 0x01, 0x02, 0x04 ... +- label : (optional) + see Documentation/devicetree/bindings/leds/common.txt +- linux,default-trigger : (optional) + see Documentation/devicetree/bindings/leds/common.txt +- default-state: (optional) The initial state of the LED. Valid + values are "on", "off", and "keep". If the LED is already on or off + and the default-state property is set the to same value, then no + glitch should be produced where the LED momentarily turns off (or + on). The "keep" setting will keep the LED at whatever its current + state is, without producing a glitch. The default is off if this + property is not present. + +Example: + +leds: leds@08 { + compatible = "syscon-leds"; + regmap = <&syscon>; + + led@bit0 { + offset = <0x08>; + mask = <0x01>; + label = "versatile:0"; + linux,default-trigger = "heartbeat"; + default-state = "on"; + }; + led@bit1 { + offset = <0x08>; + mask = <0x02>; + label = "versatile:1"; + linux,default-trigger = "mmc0"; + default-state = "off"; + }; + led@bit2 { + offset = <0x08>; + mask = <0x04>; + label = "versatile:2"; + linux,default-trigger = "cpu0"; + default-state = "off"; + }; + led@bit3 { + offset = <0x08>; + mask = <0x08>; + label = "versatile:3"; + default-state = "off"; + }; + led@bit4 { + offset = <0x08>; + mask = <0x10>; + label = "versatile:4"; + default-state = "off"; + }; + led@bit5 { + offset = <0x08>; + mask = <0x20>; + label = "versatile:5"; + default-state = "off"; + }; + led@bit6 { + offset = <0x08>; + mask = <0x40>; + label = "versatile:6"; + default-state = "off"; + }; + led@bit7 { + offset = <0x08>; + mask = <0x80>; + label = "versatile:7"; + default-state = "off"; + }; +};
This adds the device tree bindings used by syscon-based LEDs. Cc: devicetree@vger.kernel.org Cc: Bryan Wu <cooloney@gmail.com> Cc: Richard Purdie <rpurdie@rpsys.net> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- .../devicetree/bindings/leds/leds-syscon.txt | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-syscon.txt