Re: [PATCH v6 1/2] dt: bindings: lm3601x: Introduce the lm3601x driver

[Jacek Anaszewski]

Dan,

On 05/15/2018 11:29 PM, Dan Murphy wrote:
> Jacek
>
> On 05/15/2018 04:13 PM, Jacek Anaszewski wrote:
> > Hi Dan,
> >
> > Thanks for the update.
> >
> > On 05/15/2018 05:43 PM, Dan Murphy wrote:
> > > Introduce the device tree bindings for the lm3601x
> > > family of LED torch, flash and IR drivers.
> > >
> > > Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
> > > ---
> > >
> > > v6 - Removed multiple led child nodes, fixed example to display micro ranges
> > > for corresponding child nodes and added led-sources to define the current driver -
> > > https://patchwork.kernel.org/patch/10392121/
> > >
> > > v5 - No changes - https://patchwork.kernel.org/patch/10391743/
> > > v4 - Added " " around "=", changed strobe to flash on label, removed "support and
> > > register" comment and change ir lable to ir:torch - See v2 patchworks for comments
> > > v3 - Removed wildcard compatible - https://patchwork.kernel.org/patch/10386241/
> > > v2 - No changes - https://patchwork.kernel.org/patch/10384587/
> > >
> > >  Â .../devicetree/bindings/leds/leds-lm3601x.txt | 47 +++++++++++++++++++
> > >  Â 1 file changed, 47 insertions(+)
> > >  Â create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3601x.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3601x.txt b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
> > > new file mode 100644
> > > index 000000000000..27930a89e9a5
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
> > > @@ -0,0 +1,47 @@
> > > +* Texas Instruments - lm3601x Single-LED Flash Driver
> > > +
> > > +The LM3601X are ultra-small LED flash drivers that
> > > +provide a high level of adjustability.
> > > +
> > > +Required properties:
> > > +ÂÂÂ - compatible : Can be one of the following
> > > +ÂÂÂÂÂÂÂ "ti,lm36010"
> > > +ÂÂÂÂÂÂÂ "ti,lm36011"
> > > +ÂÂÂ - reg : I2C slave address
> > > +ÂÂÂ - #address-cells : 1
> > > +ÂÂÂ - #size-cells : 0
> > > +
> > > +Required child properties:
> > > +ÂÂÂ - reg : 0
> > > +ÂÂÂ - led-sources:ÂÂÂ 0 - Indicates a IR mode
> > > +ÂÂÂÂÂÂÂÂÂÂÂ 1 - Indicates a Torch (white LED) mode
> >
> > You don't need led-sources at all. Please use reg as in
> > the previous version. led-sources is useful for describing
> > more complex arrangements. Here reg will do.
>
> OK.  Thought we would keep consistent with the max IC.
>
> > > +
> > > +Required properties for flash LED child nodes:
> > > +ÂÂÂ See Documentation/devicetree/bindings/leds/common.txt
> > > +ÂÂÂ - flash-max-microamp : Range from 11mA -> 1.5A
> > > +ÂÂÂ - flash-max-timeout-us : Range from 40ms -> 1600ms
> > > +ÂÂÂ - led-max-microamp : Range from 2.4mA -> 376mA
> >
> > Please give also a step in the format like below
> > (taken from max77693):
> >
> >  ÂÂÂÂValid values: 62500 - 1000000, step by 62500 (rounded down)
>
> I would do this but the step is not linear.  The step is 40ms from 40 -> 400.
> after that the step goes to 200ms from 400->1600.
>
> same with the current ranges.

If you're going to apply Andy's formula in the driver, then you can
present it also here. Anyway, please avoid using non-standard "->"
symbol in favor of "to" if you're using "from".

Or even better I propose to list allowed values - there are not
so many of them.

--
Best regards,
Jacek Anaszewski