On Sun 16 Jul 11:49 PDT 2017, Jacek Anaszewski wrote: > On 07/15/2017 12:45 AM, Bjorn Andersson wrote: > > 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..cc9ffee6586b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt > > @@ -0,0 +1,145 @@ > > +Binding for Qualcomm Light Pulse Generator > > + > > +The Qualcomm Light Pulse Generator consists of three different hardware blocks; > > Is there a freely available documentation thereof? > The only publicly available Qualcomm PMIC documentation that I'm aware of only have the PWM hardware block, so it will be possible to use this driver but with limited functionality. [..] > > += Light Pulse Generator (LPG) > > +The Light Pulse Generator can operate either as a standard PWM controller or in > > +a more advanced lookup-table based mode. These are described separately below. > > Why a user would prefer one option over the other? I assume that both > controllers offer at least slightly different capabilities. > If so, then it could be the driver which would decide which one fits > better for the requested LED class device configuration. > I have never seen this hardware block been used as a PWM, but I imagine it to be used when someone has another driver that expects to be able to use the PWM API to control an output. In this case the node would need a #pwm-cells property, which it doesn't when it's acting as a LED and it wouldn't make much sense to expose the pin as a LED at the same time. Perhaps I overthought this? Maybe I should just leave the PWM mode out for now and it could be added in the future? [..] > > +&spmi_bus { > > + pmic@3 { > > + compatible = "qcom,pmi8994", "qcom,spmi-pmic"; > > + reg = <0x3 SPMI_USID>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pmi8994_lpg_lut: lpg-lut@b000 { > > + compatible = "qcom,spmi-lpg-lut"; > > + reg = <0xb000>; > > + > > + qcom,lut-size = <24>; > > + }; > > + > > + lpg@b200 { > > + compatible = "qcom,spmi-lpg"; > > + reg = <0xb200>; > > + > > + cell-index = <2>; > > + > > + label = "lpg:green:user0"; > > + > > + qcom,tri-led = <&pmi8994_tri_led 1>; > > + qcom,lut = <&pmi8994_lpg_lut>; > > + > > + default-state = "on"; > > + }; > > + > > + pmi8994_tri_led: tri-led@d000 { > > + compatible = "qcom,spmi-tri-led"; > > + reg = <0xd000>; > > + > > + qcom,power-source = <1>; > > + }; > > Such a design is uncommon for LED class DT bindings. It should > suffice to have a single DT LED node per LED. I have an impression > that you're exposing too many hardware details here. > You can use led-sources property (see Documentation/devicetree/bindings > /leds/common.txt and drivers/leds/leds-max77693.c where it is used). > The LUT is shared among the (up to) 8 LPG blocks, so while I did consider just including the LUT in each LPG block it didn't look nice and I had to implement the LUT as a singleton in the driver itself. The TRILED is only one of the available current sinks in the PMIC that can be driven by the LPG; the other one I use so far is a special GPIO pin acting as a current sink. Also the power-source configuration is shared among the three channels of the TRILED, so it doesn't really make sense to put this configuration in the LPG blocks themselves. And note that these are different blocks within the Qualcomm PMIC, with my design only one of them is actually representing the LED instance. > It is also not clear to me why single green color LED presented here > would have to use tri-led sink? I suppose that the sink is predestined > for three-color LEDs. > The board I'm working on (DragonBoard820c) has 4 green LEDs, the first 3 are connected to the 3 channels of the TRILED and the fourth is connected to a special GPIO in current sink mode. But I choose to shorted the example to one channel. So I end up having one LUT node, four LPG nodes, one TRILED and one GPIO node and the user space is presented with a unified interface to all four. Regards, Bjorn -- 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