Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

[Jacek Anaszewski]

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?


> But I could rename it to led-max-microamp and figure out an algo to convert to max brightness
>
> Dan

--
Best regards,
Jacek Anaszewski