| Message ID | 1448521025-2796-4-git-send-email-milo.kim@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
Hi Milo, On 11/26/2015 07:56 AM, Milo Kim wrote: > LM3633 LED device is one of TI LMU device list. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: devicetree@vger.kernel.org > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Jacek Anaszewski <j.anaszewski@samsung.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: linux-leds@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Milo Kim <milo.kim@ti.com> > --- > .../devicetree/bindings/leds/leds-lm3633.txt | 24 ++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt > new file mode 100644 > index 0000000..a553894 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt > @@ -0,0 +1,24 @@ > +TI LMU LM3633 LED device tree bindings > + > +Required properties: > + - compatible: "ti,lm3633-leds" > + > +Child nodes: > + Each node matches with LED control bank. > + Please refer to the datasheet [1]. leds/common.txt documentation says that child nodes represent discrete LED elements. > + Required properties of a child node: > + - led-sources: List of enabled channels from 0 to 5. > + Please refer to LED binding [2]. led-sources property should contain always one element - a control bank identifier that the iout is to be associated with. For example, if there are three LEDs and they should be controlled with control bank A, then the bindings should look as follows (assuming that control bank identifiers start from 0 [A:0, B:1, C:2, etc. - it has to be also explicitly stated in the documentation]: lvled1 { led-sources = <2>; led-max-microamp = <1000>; } lvled2 { led-sources = <2>; led-max-microamp = <29000>; } lvled3 { led-sources = <2>; led-max-microamp = <7000>; } The driver should expose singled LED class device for this arrangement. Since all of these LEDs will be controlled by the common bank, the lowest led-max-micromp constraint value from the three should be taken as a base for calculating max_brightness property of the LED class device. Of course in the sane case all these properties should have the same values. If the LEDs are to be connected to separate banks, then DT child nodes should look e.g. as follows (led-max-microamp values are set arbitrarily here): lvled4 { led-sources = <6>; led-max-microamp = <20000>; } lvled5 { led-sources = <7>; led-max-microamp = <17000>; } lvled6 { led-sources = <8>; led-max-microamp = <11000>; } You can of course add label properties to the child nodes. LED class device name could be composed out of LED names separated with colon, so here it would be: lvled4:levled5:lvled6 > + Optional properties of a child node: > + - label: LED channel identification. Please refer to LED binding [2]. > + - led-max-microamp: Max current setting. Type is <u32>. > + Unit is microampere. Range is from 5000 to 30000. > + Step is 1000. Please refer to LED binding [2]. > + > +Example: Please refer to ti-lmu dt-bindings [3]. > + > +[1] http://www.ti.com/product/LM3633/datasheet > +[2] ../leds/common.txt > +[2] ../mfd/ti-lmu.txt Last index should be [3] and all references above should be corrected accordingly.
Hi Jacek, On 11/27/2015 8:19 PM, Jacek Anaszewski wrote: > Hi Milo, > > On 11/26/2015 07:56 AM, Milo Kim wrote: >> LM3633 LED device is one of TI LMU device list. >> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: devicetree@vger.kernel.org >> Cc: Lee Jones <lee.jones@linaro.org> >> Cc: Jacek Anaszewski <j.anaszewski@samsung.com> >> Cc: Mark Brown <broonie@kernel.org> >> Cc: linux-leds@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Milo Kim <milo.kim@ti.com> >> --- >> .../devicetree/bindings/leds/leds-lm3633.txt | 24 ++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt >> new file mode 100644 >> index 0000000..a553894 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt >> @@ -0,0 +1,24 @@ >> +TI LMU LM3633 LED device tree bindings >> + >> +Required properties: >> + - compatible: "ti,lm3633-leds" >> + >> +Child nodes: >> + Each node matches with LED control bank. >> + Please refer to the datasheet [1]. > > leds/common.txt documentation says that child nodes represent discrete > LED elements. > >> + Required properties of a child node: >> + - led-sources: List of enabled channels from 0 to 5. >> + Please refer to LED binding [2]. > > led-sources property should contain always one element - > a control bank identifier that the iout is to be associated with. > For example, if there are three LEDs and they should be controlled > with control bank A, then the bindings should look as follows > (assuming that control bank identifiers start from 0 [A:0, B:1, > C:2, etc. - it has to be also explicitly stated in the documentation]: My understanding is 'led-sources' means output channel rather than control bank. Output channel is visible and intuitive - just output LED. On the other hand, control banks works inside the silicon, LM3633. I'm not sure other LED devices have control bank or not, but output channel is common concept. > > lvled1 { > led-sources = <2>; > led-max-microamp = <1000>; > } > > lvled2 { > led-sources = <2>; > led-max-microamp = <29000>; > } > > lvled3 { > led-sources = <2>; > led-max-microamp = <7000>; > } For this reason, I don't understand this LED configuration - one output channel with multiple LED devices. LED child node should be matched with led class device. If three output channels are controlled by one control bank, then it should be represented as below. lvled-group-A { led-sources = <0>, <1>, <2>; led-max-microamp = <some value>; } Please let me know if I misunderstand. Best regards, Milo -- 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
Hi Milo, On 11/30/2015 09:19 AM, Kim, Milo wrote: > Hi Jacek, > > On 11/27/2015 8:19 PM, Jacek Anaszewski wrote: >> Hi Milo, >> >> On 11/26/2015 07:56 AM, Milo Kim wrote: >>> LM3633 LED device is one of TI LMU device list. >>> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Cc: devicetree@vger.kernel.org >>> Cc: Lee Jones <lee.jones@linaro.org> >>> Cc: Jacek Anaszewski <j.anaszewski@samsung.com> >>> Cc: Mark Brown <broonie@kernel.org> >>> Cc: linux-leds@vger.kernel.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Milo Kim <milo.kim@ti.com> >>> --- >>> .../devicetree/bindings/leds/leds-lm3633.txt | 24 >>> ++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/leds/leds-lm3633.txt >>> >>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt >>> b/Documentation/devicetree/bindings/leds/leds-lm3633.txt >>> new file mode 100644 >>> index 0000000..a553894 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt >>> @@ -0,0 +1,24 @@ >>> +TI LMU LM3633 LED device tree bindings >>> + >>> +Required properties: >>> + - compatible: "ti,lm3633-leds" >>> + >>> +Child nodes: >>> + Each node matches with LED control bank. >>> + Please refer to the datasheet [1]. >> >> leds/common.txt documentation says that child nodes represent discrete >> LED elements. >> >>> + Required properties of a child node: >>> + - led-sources: List of enabled channels from 0 to 5. >>> + Please refer to LED binding [2]. >> >> led-sources property should contain always one element - Here I should have added that this would have been the correct approach for this device. For led flash controllers where we have one LED connected to two outputs there are two element in the led-sources array. >> a control bank identifier that the iout is to be associated with. >> For example, if there are three LEDs and they should be controlled >> with control bank A, then the bindings should look as follows >> (assuming that control bank identifiers start from 0 [A:0, B:1, >> C:2, etc. - it has to be also explicitly stated in the documentation]: > > My understanding is 'led-sources' means output channel rather than > control bank. Output channel is visible and intuitive - just output LED. > On the other hand, control banks works inside the silicon, LM3633. > I'm not sure other LED devices have control bank or not, but output > channel is common concept. There is no "channel" notion present in the leds/common.txt documentation. led-sources property is documented as "list of device current outputs the LED is connected to." In case of your device the LVLEDn pins can be configured so that they are routed to the common current output. led-sources property has been initially designed for representing one-LED-to many-iouts relation, but it can be equally well exploited for representing many-LEDs-to-one-iout relation, if used in conjunction with DT child nodes. >> >> lvled1 { >> led-sources = <2>; >> led-max-microamp = <1000>; >> } >> >> lvled2 { >> led-sources = <2>; >> led-max-microamp = <29000>; >> } >> >> lvled3 { >> led-sources = <2>; >> led-max-microamp = <7000>; >> } > > For this reason, I don't understand this LED configuration - one output > channel with multiple LED devices. LED child node should be matched with > led class device. I can't find any claim saying that child node should represent LED class device. Instead, there is a statement saying that discrete LED components "are represented by child nodes of the parent LED device binding". What is more e.g. leds-bcm6328.txt bindings don't expose LED class device for the LEDs marked as hardware controlled. Device tree describes hardware, and above bindings properly reflect hardware arrangement - there are three LEDs and each of them is connected to the same current output. Please note that common/leds.txt documentation doesn't make any reservation saying that current output necessarily reflects a hardware pin. In case of LM3633 the real current outputs are banks and multiplexing that occurs between banks and LVLEDn pins can be conveniently expressed with the bindings I proposed above. > If three output channels are controlled by one control bank, then it > should be represented as below. > > lvled-group-A { > led-sources = <0>, <1>, <2>; > led-max-microamp = <some value>; > } > > Please let me know if I misunderstand. You silently assumed some definition of a "channel". led-sources means in fact current sources, and when device is configured so that three output pins are routed to the same current source, then in fact they share common current source.
Hi Jacek, On 11/30/2015 9:26 PM, Jacek Anaszewski wrote: > In case of LM3633 the real current outputs are banks and multiplexing > that occurs between banks and LVLEDn pins can be conveniently expressed > with the bindings I proposed above. > >> >If three output channels are controlled by one control bank, then it >> >should be represented as below. >> > >> >lvled-group-A { >> > led-sources = <0>, <1>, <2>; >> > led-max-microamp = <some value>; >> >} >> > >> >Please let me know if I misunderstand. > You silently assumed some definition of a "channel". led-sources means > in fact current sources, and when device is configured so that three > output pins are routed to the same current source, then in fact > they share common current source. Thanks for comments. I've modified 'led-sources' based on your feedback. Could you check updated dt-bindings below? * Updates - led-sources: List of current sources from 0 to 5. - ti,led-outputs: Output channel specifier The 'ti,led-outputs' property is exactly matched with datasheet description, so it's easy to understand which current source is used for output channels. --- TI LMU LM3633 LED device tree bindings Required properties: - compatible: "ti,lm3633-leds" Child nodes: Each node matches with LED control bank. Please refer to the datasheet [1]. Required properties of a child node: - led-sources: List of current sources from 0 to 5. 0: control bank C for output LED 1 control bank C for output LED 1 and 2 control bank C for output LED 1, 2 and 3 1: control bank D for output LED 2 2: control bank E for output LED 3 3: control bank F for output LED 4 control bank F for output LED 4 and 5 control bank F for output LED 4, 5 and 6 4: control bank G for output LED 5 5: control bank H for output LED 6 Please refer to LED binding [2]. - ti,led-outputs: Output channel specifier from 0 to 5. 0: LED 1 1: LED 2 2: LED 3 3: LED 4 4: LED 5 5: LED 6 Optional properties of a child node: - label: LED channel identification. Please refer to LED binding [2]. - led-max-microamp: Max current setting. Type is <u32>. Unit is microampere. Range is from 5000 to 29800. Step is 800. Please refer to LED binding [2]. Examples: LED 1 is assigned for status LED. LED 4,5 and 6 are used for RGB indicator. RGB channels are controlled by bank F internally. leds { compatible = "ti,lm3633-leds"; status { led-sources = <0>; led-max-microamp = <5000>; ti,led-outputs = <0>; }; rgb { led-sources = <3>; led-max-microamp = <6600>; ti,led-outputs = <3 4 5>; }; }; LED 2 is power LED. LED 5 is notification LED. leds { compatible = "ti,lm3633-leds"; lvled2 { label = "power"; led-sources = <1>; led-max-microamp = <29000>; ti,led-outputs = <1>; }; lvled5 { label = "noti"; led-sources = <4>; led-max-microamp = <6600>; ti,led-outputs = <4>; }; }; [1] http://www.ti.com/product/LM3633/datasheet [2] ../leds/common.txt Best regards, Milo -- 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
Hi Milo, On 12/07/2015 09:46 AM, Kim, Milo wrote: > Hi Jacek, > > On 11/30/2015 9:26 PM, Jacek Anaszewski wrote: >> In case of LM3633 the real current outputs are banks and multiplexing >> that occurs between banks and LVLEDn pins can be conveniently expressed >> with the bindings I proposed above. >> >>> >If three output channels are controlled by one control bank, then it >>> >should be represented as below. >>> > >>> >lvled-group-A { >>> > led-sources = <0>, <1>, <2>; >>> > led-max-microamp = <some value>; >>> >} >>> > >>> >Please let me know if I misunderstand. >> You silently assumed some definition of a "channel". led-sources means >> in fact current sources, and when device is configured so that three >> output pins are routed to the same current source, then in fact >> they share common current source. > > Thanks for comments. I've modified 'led-sources' based on your feedback. > Could you check updated dt-bindings below? > > * Updates > - led-sources: List of current sources from 0 to 5. > - ti,led-outputs: Output channel specifier > > The 'ti,led-outputs' property is exactly matched with datasheet > description, so it's easy to understand which current source is used for > output channels. > --- > TI LMU LM3633 LED device tree bindings > > Required properties: > - compatible: "ti,lm3633-leds" > > Child nodes: > Each node matches with LED control bank. > Please refer to the datasheet [1]. > > Required properties of a child node: > - led-sources: List of current sources from 0 to 5. > 0: control bank C for output LED 1 > control bank C for output LED 1 and 2 > control bank C for output LED 1, 2 and 3 > 1: control bank D for output LED 2 > 2: control bank E for output LED 3 > 3: control bank F for output LED 4 > control bank F for output LED 4 and 5 > control bank F for output LED 4, 5 and 6 > 4: control bank G for output LED 5 > 5: control bank H for output LED 6 > Please refer to LED binding [2]. > > - ti,led-outputs: Output channel specifier from 0 to 5. > 0: LED 1 > 1: LED 2 > 2: LED 3 > 3: LED 4 > 4: LED 5 > 5: LED 6 This property is redundant. LED class driver can infer this information by analyzing the led-sources property from each child node. Below arrangement: led1 { led-sources = <0>; }; led2 { led-sources = <0>; }; led3 { led-sources = <2>; }; led4 { led-sources = <3>; }; led5 { led-sources = <3>; }; led6 { led-sources = <3>; }; suffices to discover the following: BANKC controls LVLED1, LVLED2 BANKE controls LVLED3 BANKF controls LVLED4, LVLED5, LVLED6 > Optional properties of a child node: > - label: LED channel identification. Please refer to LED binding [2]. > - led-max-microamp: Max current setting. Type is <u32>. > Unit is microampere. Range is from 5000 to 29800. > Step is 800. Please refer to LED binding [2]. > > Examples: > > LED 1 is assigned for status LED. LED 4,5 and 6 are used for RGB > indicator. RGB channels are controlled by bank F internally. > > leds { > compatible = "ti,lm3633-leds"; > > status { > led-sources = <0>; > led-max-microamp = <5000>; > ti,led-outputs = <0>; > }; > > rgb { > led-sources = <3>; > led-max-microamp = <6600>; > ti,led-outputs = <3 4 5>; > }; > }; > > LED 2 is power LED. LED 5 is notification LED. > > leds { > compatible = "ti,lm3633-leds"; > > lvled2 { > label = "power"; > led-sources = <1>; > led-max-microamp = <29000>; > ti,led-outputs = <1>; > }; > > lvled5 { > label = "noti"; > led-sources = <4>; > led-max-microamp = <6600>; > ti,led-outputs = <4>; > }; > }; > > > [1] http://www.ti.com/product/LM3633/datasheet > [2] ../leds/common.txt > > Best regards, > Milo > >
diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt new file mode 100644 index 0000000..a553894 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt @@ -0,0 +1,24 @@ +TI LMU LM3633 LED device tree bindings + +Required properties: + - compatible: "ti,lm3633-leds" + +Child nodes: + Each node matches with LED control bank. + Please refer to the datasheet [1]. + + Required properties of a child node: + - led-sources: List of enabled channels from 0 to 5. + Please refer to LED binding [2]. + + Optional properties of a child node: + - label: LED channel identification. Please refer to LED binding [2]. + - led-max-microamp: Max current setting. Type is <u32>. + Unit is microampere. Range is from 5000 to 30000. + Step is 1000. Please refer to LED binding [2]. + +Example: Please refer to ti-lmu dt-bindings [3]. + +[1] http://www.ti.com/product/LM3633/datasheet +[2] ../leds/common.txt +[2] ../mfd/ti-lmu.txt
LM3633 LED device is one of TI LMU device list. Cc: Rob Herring <robh+dt@kernel.org> Cc: devicetree@vger.kernel.org Cc: Lee Jones <lee.jones@linaro.org> Cc: Jacek Anaszewski <j.anaszewski@samsung.com> Cc: Mark Brown <broonie@kernel.org> Cc: linux-leds@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Milo Kim <milo.kim@ti.com> --- .../devicetree/bindings/leds/leds-lm3633.txt | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt