On 4/4/19 1:39 PM, Jacek Anaszewski wrote: > Dan, > > On 4/3/19 10:23 PM, Dan Murphy wrote: >> Jacek >> >> On 4/3/19 3:10 PM, Jacek Anaszewski wrote: >>> Hi Dan, >>> >>> Thank you for the patch. >>> >>> You need Lee Jones on CC for this series. >>> >> >> Yes I saw I missed Lee. >> >>> One more comment below. >>> >>> On 3/25/19 3:24 PM, Dan Murphy wrote: >>>> The LM3697 is a single function LED driver. The single function LED >>>> driver needs to reside in the LED directory as a dedicated LED driver >>>> and not as a MFD device. The device does have common brightness and ramp >>>> features and those can be accomodated by a TI LMU framework. >>>> >>>> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated >>>> LED dt binding needs to be added. The new LM3697 LED dt binding will then >>>> reside in the Documentation/devicetree/bindings/leds directory and follow the >>>> current LED and general bindings guidelines. >>>> >>>> Signed-off-by: Dan Murphy <dmurphy@xxxxxx> >>>> --- >>>> .../devicetree/bindings/leds/leds-lm3697.txt | 77 +++++++++++++++++++ >>>> .../devicetree/bindings/mfd/ti-lmu.txt | 26 +------ >>>> 2 files changed, 78 insertions(+), 25 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt >>>> new file mode 100644 >>>> index 000000000000..a780f11acd38 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt >>>> @@ -0,0 +1,77 @@ >>>> +* Texas Instruments - LM3697 Highly Efficient White LED Driver >>>> + >>>> +The LM3697 11-bit LED driver provides high- >>>> +performance backlight dimming for 1, 2, or 3 series >>>> +LED strings while delivering up to 90% efficiency. >>>> + >>>> +This device is suitable for display and keypad Lighting >>>> + >>>> +Required properties: >>>> + - compatible: >>>> + "ti,lm3697" >>>> + - reg : I2C slave address >>>> + - #address-cells : 1 >>>> + - #size-cells : 0 >>>> + >>>> +Optional properties: >>>> + - enable-gpios : GPIO pin to enable/disable the device >>>> + - vled-supply : LED supply >>>> + >>>> +Required child properties: >>>> + - reg : 0 - LED is Controlled by bank A >>>> + 1 - LED is Controlled by bank B >>>> + - led-sources : Indicates which HVLED string is associated to which >>>> + control bank. This is a zero based property so >>>> + HVLED1 = 0, HVLED2 = 1, HVLED3 = 2. >>>> + Additional information is contained >>>> + in Documentation/devicetree/bindings/leds/common.txt >>>> + >>>> +Optional child properties: >>>> + - max_brightness - This determines whether to use 8 bit brightness mode >>>> + or 11 bit brightness mode. If this value is not >>>> + set the device is defaulted to the preferred 8bit >>>> + brightness mode per 7.3.4.1 of the data sheet. >>>> + The values are 255 (8bit) or 2047 (11bit). >>> >>> We should use led-max-microamp for that, if possible. >>> >> >> Actually I was thinking this property could move to common.txt >> LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt > > It was considered back in 2014 when I was working on LED flash > class framework. It was then assessed inappropriate for describing > physical property of a device. It needs to be kept in mind that > it was in the context of flash LEDs, where currents are much > higher than for common LEDs. Since then we have been always using > led-max-microamp. > > With max-brightness in Device Tree there is also another problem - > we don't know what it really means - greater allowed current or > greater resolution. > > Why not allow for maximum available brightness resolution for ti-lmu > always when the amperage is not an issue? > Maybe I should rename this to ti,brightness-resolution with the same values. or ti,brightness-res for short hand. Or I could try to figure out the max-microamp but it really does not describe the hardware or the configuration. Dan > >> But I could rename it to led-max-microamp and figure out an algo to convert to max brightness >> >> Dan >> > -- ------------------ Dan Murphy