| Message ID | 20171115071345.24331-4-bjorn.andersson@linaro.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Qualcomm Light Pulse Generator | expand |
On Tue, Nov 14, 2017 at 11:13:45PM -0800, Bjorn Andersson wrote: > This adds the binding document describing the three hardware blocks > related to the Light Pulse Generator found in a wide range of Qualcomm > PMICs. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v2: > - Squashed all things into one node > - Removed quirks from the binding, compatible implies number of channels, their > configuration etc. > - Binding describes LEDs connected as child nodes > - Support describing multi-channel LEDs > - Change style of the binding document, to match other LED bindings > > Changes since v1: > - Dropped custom pattern properties > - Renamed cell-index to qcom,lpg-channel to clarify its purpose > > .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt Acked-by: Rob Herring <robh@kernel.org> -- 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 Bjorn, Thanks for the update. On 11/15/2017 08:13 AM, Bjorn Andersson wrote: > This adds the binding document describing the three hardware blocks > related to the Light Pulse Generator found in a wide range of Qualcomm > PMICs. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v2: > - Squashed all things into one node > - Removed quirks from the binding, compatible implies number of channels, their > configuration etc. > - Binding describes LEDs connected as child nodes > - Support describing multi-channel LEDs > - Change style of the binding document, to match other LED bindings > > Changes since v1: > - Dropped custom pattern properties > - Renamed cell-index to qcom,lpg-channel to clarify its purpose > > .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > new file mode 100644 > index 000000000000..9cee6f9f543c > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > @@ -0,0 +1,66 @@ > +Binding for Qualcomm Light Pulse Generator > + > +The Qualcomm Light Pulse Generator consists of three different hardware blocks; > +a ramp generator with lookup table, the light pulse generator and a three > +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. > + > +Required properties: > +- compatible: one of: > + "qcom,pm8916-pwm", > + "qcom,pm8941-lpg", > + "qcom,pm8994-lpg", > + "qcom,pmi8994-lpg", > + "qcom,pmi8998-lpg", > + > +Optional properties: > +- qcom,power-source: power-source used to drive the output, as defined in the > + datasheet. Should be specified if the TRILED block is > + present Range of possible values is missing here. > +- qcom,dtest: configures the output into an internal test line of the > + pmic. Specified by a list of u32 pairs, one pair per channel, > + where each pair denotes the test line to drive and the second > + configures how the value should be outputed, as defined in the > + datasheet > +- #pwm-cells: should be 2, see ../pwm/pwm.txt > + > +LED subnodes: > +A set of subnodes can be used to specify LEDs connected to the LPG. Channels > +not associated with a LED are available as pwm channels, see ../pwm/pwm.txt. > + > +Required properties: > +- led-sources: list of channels associated with this LED, starting at 1 for the > + first LPG channel > + > +Optional properties: > +- label: see Documentation/devicetree/bindings/leds/common.txt > +- default-state: see Documentation/devicetree/bindings/leds/common.txt > +- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt > + > +Example: > +The following example defines a RGB LED attached to the PM8941. > + > +&spmi_bus { > + pm8941@1 { > + lpg { > + compatible = "qcom,pm8941-lpg"; > + qcom,power-source = <1>; > + > + rgb { > + led-sources = <7 6 5>; > + }; > + }; > + }; > +}; > + > +The following example defines the single PWM channel of the PM8916, which can > +be muxed by the MPP4 as a current sink. > + > +&spmi_bus { > + pm8916@1 { > + pm8916_pwm: pwm { > + compatible = "qcom,pm8916-pwm"; > + > + #pwm-cells = <2>; LED has to be represented as a child node - see Documentation/devicetree/bindings/leds/common.txt > + }; > + }; > +}; > Could you please also provide an example of the arrangement on the board DragonBoard820c, you were describing in the discussions under the previous version of the patch set. i.e. three green LEDs connected to TRILED and one to the GPIO sink? Also any other non-trivial board configurations supported by the driver would allow to increase our comprehensions of the device capabilities.
On Sun 19 Nov 13:35 PST 2017, Jacek Anaszewski wrote: > Hi Bjorn, > > Thanks for the update. > > On 11/15/2017 08:13 AM, Bjorn Andersson wrote: > > This adds the binding document describing the three hardware blocks > > related to the Light Pulse Generator found in a wide range of Qualcomm > > PMICs. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > > > Changes since v2: > > - Squashed all things into one node > > - Removed quirks from the binding, compatible implies number of channels, their > > configuration etc. > > - Binding describes LEDs connected as child nodes > > - Support describing multi-channel LEDs > > - Change style of the binding document, to match other LED bindings > > > > Changes since v1: > > - Dropped custom pattern properties > > - Renamed cell-index to qcom,lpg-channel to clarify its purpose > > > > .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ > > 1 file changed, 66 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > > > > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > > new file mode 100644 > > index 000000000000..9cee6f9f543c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > > @@ -0,0 +1,66 @@ > > +Binding for Qualcomm Light Pulse Generator > > + > > +The Qualcomm Light Pulse Generator consists of three different hardware blocks; > > +a ramp generator with lookup table, the light pulse generator and a three > > +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. > > + > > +Required properties: > > +- compatible: one of: > > + "qcom,pm8916-pwm", > > + "qcom,pm8941-lpg", > > + "qcom,pm8994-lpg", > > + "qcom,pmi8994-lpg", > > + "qcom,pmi8998-lpg", > > + > > +Optional properties: > > +- qcom,power-source: power-source used to drive the output, as defined in the > > + datasheet. Should be specified if the TRILED block is > > + present > > Range of possible values is missing here. > There seems to be a 4-way mux in all variants, but the wiring is different in the different products. E.g. in pm8941 1 represents VPH_PWR while in pmi8994 this is pulled from a dedicated pin named VIN_RGB. Would you like me to list the 4 options for each compatible? > > +- qcom,dtest: configures the output into an internal test line of the > > + pmic. Specified by a list of u32 pairs, one pair per channel, > > + where each pair denotes the test line to drive and the second > > + configures how the value should be outputed, as defined in the > > + datasheet > > +- #pwm-cells: should be 2, see ../pwm/pwm.txt > > + > > +LED subnodes: > > +A set of subnodes can be used to specify LEDs connected to the LPG. Channels > > +not associated with a LED are available as pwm channels, see ../pwm/pwm.txt. > > + > > +Required properties: > > +- led-sources: list of channels associated with this LED, starting at 1 for the > > + first LPG channel > > + > > +Optional properties: > > +- label: see Documentation/devicetree/bindings/leds/common.txt > > +- default-state: see Documentation/devicetree/bindings/leds/common.txt > > +- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt > > + > > +Example: > > +The following example defines a RGB LED attached to the PM8941. > > + > > +&spmi_bus { > > + pm8941@1 { > > + lpg { > > + compatible = "qcom,pm8941-lpg"; > > + qcom,power-source = <1>; > > + > > + rgb { > > + led-sources = <7 6 5>; > > + }; > > + }; > > + }; > > +}; > > + > > +The following example defines the single PWM channel of the PM8916, which can > > +be muxed by the MPP4 as a current sink. > > + > > +&spmi_bus { > > + pm8916@1 { > > + pm8916_pwm: pwm { > > + compatible = "qcom,pm8916-pwm"; > > + > > + #pwm-cells = <2>; > > LED has to be represented as a child node - > see Documentation/devicetree/bindings/leds/common.txt > Some use cases for this hardware block is to use it as a "traditional" PWM source, so any channels not allocated to a LED are available as pwm channels. So this is from the Dragonboard410c, where the example application is to wire this pwm signal as control signal to a backlight controller; i.e. using pwm-backlight associated with the display panel. For testing purposes I did route the signal to another gpio with a LED on the board, added a LED node here and saw that I can control that as well. I did include this example here, even though there's no LED, just to show how this could be done. > > + }; > > + }; > > +}; > > > > Could you please also provide an example of the arrangement on the > board DragonBoard820c, you were describing in the discussions under > the previous version of the patch set. i.e. three green LEDs connected > to TRILED and one to the GPIO sink? > Of course. > Also any other non-trivial board configurations supported by the > driver would allow to increase our comprehensions of the device > capabilities. > There is a few other hardware components that can be fed the output of the LPG as control signal, but I think the GPIO sink case above does showcase the power. Regards, Bjorn -- 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 11/20/2017 08:58 PM, Bjorn Andersson wrote: > On Sun 19 Nov 13:35 PST 2017, Jacek Anaszewski wrote: > >> Hi Bjorn, >> >> Thanks for the update. >> >> On 11/15/2017 08:13 AM, Bjorn Andersson wrote: >>> This adds the binding document describing the three hardware blocks >>> related to the Light Pulse Generator found in a wide range of Qualcomm >>> PMICs. >>> >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>> --- >>> >>> Changes since v2: >>> - Squashed all things into one node >>> - Removed quirks from the binding, compatible implies number of channels, their >>> configuration etc. >>> - Binding describes LEDs connected as child nodes >>> - Support describing multi-channel LEDs >>> - Change style of the binding document, to match other LED bindings >>> >>> Changes since v1: >>> - Dropped custom pattern properties >>> - Renamed cell-index to qcom,lpg-channel to clarify its purpose >>> >>> .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ >>> 1 file changed, 66 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>> >>> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>> new file mode 100644 >>> index 000000000000..9cee6f9f543c >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>> @@ -0,0 +1,66 @@ >>> +Binding for Qualcomm Light Pulse Generator >>> + >>> +The Qualcomm Light Pulse Generator consists of three different hardware blocks; >>> +a ramp generator with lookup table, the light pulse generator and a three >>> +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. >>> + >>> +Required properties: >>> +- compatible: one of: >>> + "qcom,pm8916-pwm", >>> + "qcom,pm8941-lpg", >>> + "qcom,pm8994-lpg", >>> + "qcom,pmi8994-lpg", >>> + "qcom,pmi8998-lpg", >>> + >>> +Optional properties: >>> +- qcom,power-source: power-source used to drive the output, as defined in the >>> + datasheet. Should be specified if the TRILED block is >>> + present >> >> Range of possible values is missing here. >> > > There seems to be a 4-way mux in all variants, but the wiring is > different in the different products. E.g. in pm8941 1 represents VPH_PWR > while in pmi8994 this is pulled from a dedicated pin named VIN_RGB. > > Would you like me to list the 4 options for each compatible? Could you please explain why user would prefer one power source over the other? Is it that they have different max current limit? >>> +- qcom,dtest: configures the output into an internal test line of the >>> + pmic. Specified by a list of u32 pairs, one pair per channel, >>> + where each pair denotes the test line to drive and the second >>> + configures how the value should be outputed, as defined in the >>> + datasheet >>> +- #pwm-cells: should be 2, see ../pwm/pwm.txt >>> + >>> +LED subnodes: >>> +A set of subnodes can be used to specify LEDs connected to the LPG. Channels >>> +not associated with a LED are available as pwm channels, see ../pwm/pwm.txt. >>> + >>> +Required properties: >>> +- led-sources: list of channels associated with this LED, starting at 1 for the >>> + first LPG channel >>> + >>> +Optional properties: >>> +- label: see Documentation/devicetree/bindings/leds/common.txt >>> +- default-state: see Documentation/devicetree/bindings/leds/common.txt >>> +- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt >>> + >>> +Example: >>> +The following example defines a RGB LED attached to the PM8941. >>> + >>> +&spmi_bus { >>> + pm8941@1 { >>> + lpg { >>> + compatible = "qcom,pm8941-lpg"; >>> + qcom,power-source = <1>; >>> + >>> + rgb { >>> + led-sources = <7 6 5>; >>> + }; >>> + }; >>> + }; >>> +}; >>> + >>> +The following example defines the single PWM channel of the PM8916, which can >>> +be muxed by the MPP4 as a current sink. >>> + >>> +&spmi_bus { >>> + pm8916@1 { >>> + pm8916_pwm: pwm { >>> + compatible = "qcom,pm8916-pwm"; >>> + >>> + #pwm-cells = <2>; >> >> LED has to be represented as a child node - >> see Documentation/devicetree/bindings/leds/common.txt >> > > Some use cases for this hardware block is to use it as a "traditional" > PWM source, so any channels not allocated to a LED are available as pwm > channels. > > So this is from the Dragonboard410c, where the example application is to > wire this pwm signal as control signal to a backlight controller; i.e. > using pwm-backlight associated with the display panel. Ack. > > For testing purposes I did route the signal to another gpio with a LED > on the board, added a LED node here and saw that I can control that as > well. > > I did include this example here, even though there's no LED, just to > show how this could be done. > >>> + }; >>> + }; >>> +}; >>> >> >> Could you please also provide an example of the arrangement on the >> board DragonBoard820c, you were describing in the discussions under >> the previous version of the patch set. i.e. three green LEDs connected >> to TRILED and one to the GPIO sink? >> > > Of course. One more question regarding TRILED - in your design it will be exposed as a single LED class device with one brightness file, right? Does it mean that all three LEDs will be applied the same brightness after writing it to the sysfs file? >> Also any other non-trivial board configurations supported by the >> driver would allow to increase our comprehensions of the device >> capabilities. >> > > There is a few other hardware components that can be fed the output of > the LPG as control signal, but I think the GPIO sink case above does > showcase the power. > > Regards, > Bjorn >
On Mon 20 Nov 12:35 PST 2017, Jacek Anaszewski wrote: > On 11/20/2017 08:58 PM, Bjorn Andersson wrote: > > On Sun 19 Nov 13:35 PST 2017, Jacek Anaszewski wrote: > > > >> Hi Bjorn, > >> > >> Thanks for the update. > >> > >> On 11/15/2017 08:13 AM, Bjorn Andersson wrote: > >>> This adds the binding document describing the three hardware blocks > >>> related to the Light Pulse Generator found in a wide range of Qualcomm > >>> PMICs. > >>> > >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > >>> --- > >>> > >>> Changes since v2: > >>> - Squashed all things into one node > >>> - Removed quirks from the binding, compatible implies number of channels, their > >>> configuration etc. > >>> - Binding describes LEDs connected as child nodes > >>> - Support describing multi-channel LEDs > >>> - Change style of the binding document, to match other LED bindings > >>> > >>> Changes since v1: > >>> - Dropped custom pattern properties > >>> - Renamed cell-index to qcom,lpg-channel to clarify its purpose > >>> > >>> .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ > >>> 1 file changed, 66 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > >>> new file mode 100644 > >>> index 000000000000..9cee6f9f543c > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > >>> @@ -0,0 +1,66 @@ > >>> +Binding for Qualcomm Light Pulse Generator > >>> + > >>> +The Qualcomm Light Pulse Generator consists of three different hardware blocks; > >>> +a ramp generator with lookup table, the light pulse generator and a three > >>> +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. > >>> + > >>> +Required properties: > >>> +- compatible: one of: > >>> + "qcom,pm8916-pwm", > >>> + "qcom,pm8941-lpg", > >>> + "qcom,pm8994-lpg", > >>> + "qcom,pmi8994-lpg", > >>> + "qcom,pmi8998-lpg", > >>> + > >>> +Optional properties: > >>> +- qcom,power-source: power-source used to drive the output, as defined in the > >>> + datasheet. Should be specified if the TRILED block is > >>> + present > >> > >> Range of possible values is missing here. > >> > > > > There seems to be a 4-way mux in all variants, but the wiring is > > different in the different products. E.g. in pm8941 1 represents VPH_PWR > > while in pmi8994 this is pulled from a dedicated pin named VIN_RGB. > > > > Would you like me to list the 4 options for each compatible? > > Could you please explain why user would prefer one power source > over the other? Is it that they have different max current limit? > The mux in pm8941 is connected to ground (0V), vph_pwr (3.6V), internal 5V and min(5V, charger). In pmi8994 it's ground, vdd_rgb (a dedicated pin) and 4.2V. PMI8998 is a slight variation of PMI8994 and I expect there to be more variants. So it's different voltage level and, potentially, current limit. [..] > One more question regarding TRILED - in your design it will be > exposed as a single LED class device with one brightness file, > right? Does it mean that all three LEDs will be applied the > same brightness after writing it to the sysfs file? > Correct, each LED described in DT will become one LED and can have more than one (any number of) physical channel associated. The current implementation applies the same brightness (and pattern) to all channels associated with a LED. The open question is still how to pass a color from user space, the brightness_set and pattern_set would need to be modified to map a list of brightnesses to the individual channels or to adapt the brightness by some color-modifier(?). I've tested the driver on single-LEDs and on RGB-leds and it supports both, the latter with pulse pattern synchronized over the 3 channels. So I'm hoping that we can move forward with merging the driver with this limitation and then discuss the RGB interface in LED-class separately. Regards, Bjorn -- 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 11/20/2017 10:45 PM, Bjorn Andersson wrote: > On Mon 20 Nov 12:35 PST 2017, Jacek Anaszewski wrote: > >> On 11/20/2017 08:58 PM, Bjorn Andersson wrote: >>> On Sun 19 Nov 13:35 PST 2017, Jacek Anaszewski wrote: >>> >>>> Hi Bjorn, >>>> >>>> Thanks for the update. >>>> >>>> On 11/15/2017 08:13 AM, Bjorn Andersson wrote: >>>>> This adds the binding document describing the three hardware blocks >>>>> related to the Light Pulse Generator found in a wide range of Qualcomm >>>>> PMICs. >>>>> >>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>>>> --- >>>>> >>>>> Changes since v2: >>>>> - Squashed all things into one node >>>>> - Removed quirks from the binding, compatible implies number of channels, their >>>>> configuration etc. >>>>> - Binding describes LEDs connected as child nodes >>>>> - Support describing multi-channel LEDs >>>>> - Change style of the binding document, to match other LED bindings >>>>> >>>>> Changes since v1: >>>>> - Dropped custom pattern properties >>>>> - Renamed cell-index to qcom,lpg-channel to clarify its purpose >>>>> >>>>> .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ >>>>> 1 file changed, 66 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>>>> new file mode 100644 >>>>> index 000000000000..9cee6f9f543c >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>>>> @@ -0,0 +1,66 @@ >>>>> +Binding for Qualcomm Light Pulse Generator >>>>> + >>>>> +The Qualcomm Light Pulse Generator consists of three different hardware blocks; >>>>> +a ramp generator with lookup table, the light pulse generator and a three >>>>> +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. >>>>> + >>>>> +Required properties: >>>>> +- compatible: one of: >>>>> + "qcom,pm8916-pwm", >>>>> + "qcom,pm8941-lpg", >>>>> + "qcom,pm8994-lpg", >>>>> + "qcom,pmi8994-lpg", >>>>> + "qcom,pmi8998-lpg", >>>>> + >>>>> +Optional properties: >>>>> +- qcom,power-source: power-source used to drive the output, as defined in the >>>>> + datasheet. Should be specified if the TRILED block is >>>>> + present >>>> >>>> Range of possible values is missing here. >>>> >>> >>> There seems to be a 4-way mux in all variants, but the wiring is >>> different in the different products. E.g. in pm8941 1 represents VPH_PWR >>> while in pmi8994 this is pulled from a dedicated pin named VIN_RGB. >>> >>> Would you like me to list the 4 options for each compatible? >> >> Could you please explain why user would prefer one power source >> over the other? Is it that they have different max current limit? >> > > The mux in pm8941 is connected to ground (0V), vph_pwr (3.6V), internal > 5V and min(5V, charger). In pmi8994 it's ground, vdd_rgb (a dedicated > pin) and 4.2V. PMI8998 is a slight variation of PMI8994 and I expect > there to be more variants. > > So it's different voltage level and, potentially, current limit. I'd replace this property with led-max-microamp (see Documentation/devicetree/bindings/leds/common.txt) and let the driver to decide which power source to choose, basing on that limit. > [..] >> One more question regarding TRILED - in your design it will be >> exposed as a single LED class device with one brightness file, >> right? Does it mean that all three LEDs will be applied the >> same brightness after writing it to the sysfs file? >> > > Correct, each LED described in DT will become one LED and can have more > than one (any number of) physical channel associated. The current > implementation applies the same brightness (and pattern) to all channels > associated with a LED. The rgb DT node name would be a bit misleading in this case, since RGB usually implies the possibility of having different intensity of each color. > The open question is still how to pass a color from user space, the > brightness_set and pattern_set would need to be modified to map a list > of brightnesses to the individual channels or to adapt the brightness by > some color-modifier(?). Pavel made and attempt of reworking Heiner Kallweit's HSV approach few months ago [0]. You can take a look and share your opinion or even continue this effort. > I've tested the driver on single-LEDs and on RGB-leds and it supports > both, the latter with pulse pattern synchronized over the 3 channels. > So I'm hoping that we can move forward with merging the driver with this > limitation and then discuss the RGB interface in LED-class separately. Agreed. [0] https://lkml.org/lkml/2017/8/30/423
On Wed 22 Nov 12:42 PST 2017, Jacek Anaszewski wrote: > On 11/20/2017 10:45 PM, Bjorn Andersson wrote: > > On Mon 20 Nov 12:35 PST 2017, Jacek Anaszewski wrote: > > > >> On 11/20/2017 08:58 PM, Bjorn Andersson wrote: > >>> On Sun 19 Nov 13:35 PST 2017, Jacek Anaszewski wrote: > >>> > >>>> Hi Bjorn, > >>>> > >>>> Thanks for the update. > >>>> > >>>> On 11/15/2017 08:13 AM, Bjorn Andersson wrote: > >>>>> This adds the binding document describing the three hardware blocks > >>>>> related to the Light Pulse Generator found in a wide range of Qualcomm > >>>>> PMICs. > >>>>> > >>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > >>>>> --- > >>>>> > >>>>> Changes since v2: > >>>>> - Squashed all things into one node > >>>>> - Removed quirks from the binding, compatible implies number of channels, their > >>>>> configuration etc. > >>>>> - Binding describes LEDs connected as child nodes > >>>>> - Support describing multi-channel LEDs > >>>>> - Change style of the binding document, to match other LED bindings > >>>>> > >>>>> Changes since v1: > >>>>> - Dropped custom pattern properties > >>>>> - Renamed cell-index to qcom,lpg-channel to clarify its purpose > >>>>> > >>>>> .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ > >>>>> 1 file changed, 66 insertions(+) > >>>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > >>>>> new file mode 100644 > >>>>> index 000000000000..9cee6f9f543c > >>>>> --- /dev/null > >>>>> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > >>>>> @@ -0,0 +1,66 @@ > >>>>> +Binding for Qualcomm Light Pulse Generator > >>>>> + > >>>>> +The Qualcomm Light Pulse Generator consists of three different hardware blocks; > >>>>> +a ramp generator with lookup table, the light pulse generator and a three > >>>>> +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. > >>>>> + > >>>>> +Required properties: > >>>>> +- compatible: one of: > >>>>> + "qcom,pm8916-pwm", > >>>>> + "qcom,pm8941-lpg", > >>>>> + "qcom,pm8994-lpg", > >>>>> + "qcom,pmi8994-lpg", > >>>>> + "qcom,pmi8998-lpg", > >>>>> + > >>>>> +Optional properties: > >>>>> +- qcom,power-source: power-source used to drive the output, as defined in the > >>>>> + datasheet. Should be specified if the TRILED block is > >>>>> + present > >>>> > >>>> Range of possible values is missing here. > >>>> > >>> > >>> There seems to be a 4-way mux in all variants, but the wiring is > >>> different in the different products. E.g. in pm8941 1 represents VPH_PWR > >>> while in pmi8994 this is pulled from a dedicated pin named VIN_RGB. > >>> > >>> Would you like me to list the 4 options for each compatible? > >> > >> Could you please explain why user would prefer one power source > >> over the other? Is it that they have different max current limit? > >> > > > > The mux in pm8941 is connected to ground (0V), vph_pwr (3.6V), internal > > 5V and min(5V, charger). In pmi8994 it's ground, vdd_rgb (a dedicated > > pin) and 4.2V. PMI8998 is a slight variation of PMI8994 and I expect > > there to be more variants. > > > > So it's different voltage level and, potentially, current limit. > > I'd replace this property with led-max-microamp > (see Documentation/devicetree/bindings/leds/common.txt) and let > the driver to decide which power source to choose, basing on that limit. > Unfortunately I don't think specifying this in uA is possible, at least some of these inputs does not have a knows (at least not platform defined) current limit. Like in the case on PM8941, the knobs this property tweaks is: "should we output 3.6V or 5V" and both values depend on external power-supplies. For most cases specifying this as led-microvolt would be possible, but then there are the levels that are min(5V, charger). One thing we've done in some other drivers with similar "enum" like types is to provide an dt-bindings include file with these constants. This makes the dts self documented and doesn't require additional translation of the values. > > [..] > >> One more question regarding TRILED - in your design it will be > >> exposed as a single LED class device with one brightness file, > >> right? Does it mean that all three LEDs will be applied the > >> same brightness after writing it to the sysfs file? > >> > > > > Correct, each LED described in DT will become one LED and can have more > > than one (any number of) physical channel associated. The current > > implementation applies the same brightness (and pattern) to all channels > > associated with a LED. > > The rgb DT node name would be a bit misleading in this case, since > RGB usually implies the possibility of having different intensity > of each color. > In the sense of the devicetree this is exactly what it describes, the fact that we haven't figured out a way to implement this is, in my view, a separate topic. > > The open question is still how to pass a color from user space, the > > brightness_set and pattern_set would need to be modified to map a list > > of brightnesses to the individual channels or to adapt the brightness by > > some color-modifier(?). > > Pavel made and attempt of reworking Heiner Kallweit's HSV approach > few months ago [0]. You can take a look and share your opinion > or even continue this effort. > I did not consider using HSV to get around the problem of everything operating on "brightness", but this seems like a quite nice solution. In the case of lpg_brightness_set() this would map nicely into the case where a LED is either a single channel or three channels, and until we land those patches the driver would just implement H = S = 0. And for the pattern setting, we can retain the proposed interface of pattern being a sequence of brightness/delay values and then map this in the driver as we apply the patterns. Regards, Bjorn -- 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 12/18/2017 09:49 PM, Bjorn Andersson wrote: > On Wed 22 Nov 12:42 PST 2017, Jacek Anaszewski wrote: > >> On 11/20/2017 10:45 PM, Bjorn Andersson wrote: >>> On Mon 20 Nov 12:35 PST 2017, Jacek Anaszewski wrote: >>> >>>> On 11/20/2017 08:58 PM, Bjorn Andersson wrote: >>>>> On Sun 19 Nov 13:35 PST 2017, Jacek Anaszewski wrote: >>>>> >>>>>> Hi Bjorn, >>>>>> >>>>>> Thanks for the update. >>>>>> >>>>>> On 11/15/2017 08:13 AM, Bjorn Andersson wrote: >>>>>>> This adds the binding document describing the three hardware blocks >>>>>>> related to the Light Pulse Generator found in a wide range of Qualcomm >>>>>>> PMICs. >>>>>>> >>>>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>>>>>> --- >>>>>>> >>>>>>> Changes since v2: >>>>>>> - Squashed all things into one node >>>>>>> - Removed quirks from the binding, compatible implies number of channels, their >>>>>>> configuration etc. >>>>>>> - Binding describes LEDs connected as child nodes >>>>>>> - Support describing multi-channel LEDs >>>>>>> - Change style of the binding document, to match other LED bindings >>>>>>> >>>>>>> Changes since v1: >>>>>>> - Dropped custom pattern properties >>>>>>> - Renamed cell-index to qcom,lpg-channel to clarify its purpose >>>>>>> >>>>>>> .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ >>>>>>> 1 file changed, 66 insertions(+) >>>>>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>>>>>> new file mode 100644 >>>>>>> index 000000000000..9cee6f9f543c >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt >>>>>>> @@ -0,0 +1,66 @@ >>>>>>> +Binding for Qualcomm Light Pulse Generator >>>>>>> + >>>>>>> +The Qualcomm Light Pulse Generator consists of three different hardware blocks; >>>>>>> +a ramp generator with lookup table, the light pulse generator and a three >>>>>>> +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. >>>>>>> + >>>>>>> +Required properties: >>>>>>> +- compatible: one of: >>>>>>> + "qcom,pm8916-pwm", >>>>>>> + "qcom,pm8941-lpg", >>>>>>> + "qcom,pm8994-lpg", >>>>>>> + "qcom,pmi8994-lpg", >>>>>>> + "qcom,pmi8998-lpg", >>>>>>> + >>>>>>> +Optional properties: >>>>>>> +- qcom,power-source: power-source used to drive the output, as defined in the >>>>>>> + datasheet. Should be specified if the TRILED block is >>>>>>> + present >>>>>> >>>>>> Range of possible values is missing here. >>>>>> >>>>> >>>>> There seems to be a 4-way mux in all variants, but the wiring is >>>>> different in the different products. E.g. in pm8941 1 represents VPH_PWR >>>>> while in pmi8994 this is pulled from a dedicated pin named VIN_RGB. >>>>> >>>>> Would you like me to list the 4 options for each compatible? >>>> >>>> Could you please explain why user would prefer one power source >>>> over the other? Is it that they have different max current limit? >>>> >>> >>> The mux in pm8941 is connected to ground (0V), vph_pwr (3.6V), internal >>> 5V and min(5V, charger). In pmi8994 it's ground, vdd_rgb (a dedicated >>> pin) and 4.2V. PMI8998 is a slight variation of PMI8994 and I expect >>> there to be more variants. >>> >>> So it's different voltage level and, potentially, current limit. >> >> I'd replace this property with led-max-microamp >> (see Documentation/devicetree/bindings/leds/common.txt) and let >> the driver to decide which power source to choose, basing on that limit. >> > > Unfortunately I don't think specifying this in uA is possible, at least > some of these inputs does not have a knows (at least not platform > defined) current limit. > > Like in the case on PM8941, the knobs this property tweaks is: "should > we output 3.6V or 5V" and both values depend on external power-supplies. > > For most cases specifying this as led-microvolt would be possible, but > then there are the levels that are min(5V, charger). > > > One thing we've done in some other drivers with similar "enum" like > types is to provide an dt-bindings include file with these constants. > This makes the dts self documented and doesn't require additional > translation of the values. Sounds good. >>> [..] >>>> One more question regarding TRILED - in your design it will be >>>> exposed as a single LED class device with one brightness file, >>>> right? Does it mean that all three LEDs will be applied the >>>> same brightness after writing it to the sysfs file? >>>> >>> >>> Correct, each LED described in DT will become one LED and can have more >>> than one (any number of) physical channel associated. The current >>> implementation applies the same brightness (and pattern) to all channels >>> associated with a LED. >> >> The rgb DT node name would be a bit misleading in this case, since >> RGB usually implies the possibility of having different intensity >> of each color. >> > > In the sense of the devicetree this is exactly what it describes, the > fact that we haven't figured out a way to implement this is, in my view, > a separate topic. Indeed, I was just looking at it from the LED ABI POV. >>> The open question is still how to pass a color from user space, the >>> brightness_set and pattern_set would need to be modified to map a list >>> of brightnesses to the individual channels or to adapt the brightness by >>> some color-modifier(?). >> >> Pavel made and attempt of reworking Heiner Kallweit's HSV approach >> few months ago [0]. You can take a look and share your opinion >> or even continue this effort. >> > > I did not consider using HSV to get around the problem of everything > operating on "brightness", but this seems like a quite nice solution. > > In the case of lpg_brightness_set() this would map nicely into the case > where a LED is either a single channel or three channels, and until we > land those patches the driver would just implement H = S = 0. > > And for the pattern setting, we can retain the proposed interface of > pattern being a sequence of brightness/delay values and then map this in > the driver as we apply the patterns. Yes, the HSV approach would be very nice especially due to its compatibility with monochrome LEDs. We will however need to allow for defining suitable coefficients for adjusting HSV values, so that the perceived color matched the expected one.
diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt new file mode 100644 index 000000000000..9cee6f9f543c --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt @@ -0,0 +1,66 @@ +Binding for Qualcomm Light Pulse Generator + +The Qualcomm Light Pulse Generator consists of three different hardware blocks; +a ramp generator with lookup table, the light pulse generator and a three +channel current sink. These blocks are found in a wide range of Qualcomm PMICs. + +Required properties: +- compatible: one of: + "qcom,pm8916-pwm", + "qcom,pm8941-lpg", + "qcom,pm8994-lpg", + "qcom,pmi8994-lpg", + "qcom,pmi8998-lpg", + +Optional properties: +- qcom,power-source: power-source used to drive the output, as defined in the + datasheet. Should be specified if the TRILED block is + present +- qcom,dtest: configures the output into an internal test line of the + pmic. Specified by a list of u32 pairs, one pair per channel, + where each pair denotes the test line to drive and the second + configures how the value should be outputed, as defined in the + datasheet +- #pwm-cells: should be 2, see ../pwm/pwm.txt + +LED subnodes: +A set of subnodes can be used to specify LEDs connected to the LPG. Channels +not associated with a LED are available as pwm channels, see ../pwm/pwm.txt. + +Required properties: +- led-sources: list of channels associated with this LED, starting at 1 for the + first LPG channel + +Optional properties: +- label: see Documentation/devicetree/bindings/leds/common.txt +- default-state: see Documentation/devicetree/bindings/leds/common.txt +- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt + +Example: +The following example defines a RGB LED attached to the PM8941. + +&spmi_bus { + pm8941@1 { + lpg { + compatible = "qcom,pm8941-lpg"; + qcom,power-source = <1>; + + rgb { + led-sources = <7 6 5>; + }; + }; + }; +}; + +The following example defines the single PWM channel of the PM8916, which can +be muxed by the MPP4 as a current sink. + +&spmi_bus { + pm8916@1 { + pm8916_pwm: pwm { + compatible = "qcom,pm8916-pwm"; + + #pwm-cells = <2>; + }; + }; +};
This adds the binding document describing the three hardware blocks related to the Light Pulse Generator found in a wide range of Qualcomm PMICs. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- Changes since v2: - Squashed all things into one node - Removed quirks from the binding, compatible implies number of channels, their configuration etc. - Binding describes LEDs connected as child nodes - Support describing multi-channel LEDs - Change style of the binding document, to match other LED bindings Changes since v1: - Dropped custom pattern properties - Renamed cell-index to qcom,lpg-channel to clarify its purpose .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt