Re: [PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

From: Tony Lindgren <tony@atomide.com>
To: Rob Herring <robh@kernel.org>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>,
	jacek.anaszewski@gmail.com, pavel@ucw.cz, sre@kernel.org,
	mark.rutland@arm.com, lee.jones@linaro.org,
	daniel.thompson@linaro.org, dmurphy@ti.com,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, tomi.valkeinen@ti.com,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding
Date: Thu, 21 Nov 2019 10:22:28 -0800	[thread overview]
Message-ID: <20191121182228.GO43123@atomide.com> (raw)
In-Reply-To: <20191009193523.GA7094@bogus>

Hi,

* Rob Herring <robh@kernel.org> [700101 00:00]:
> On Wed, Oct 09, 2019 at 10:51:26AM +0200, Jean-Jacques Hiblot wrote:
> > Add DT binding for led-backlight.
...
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
...

> > +  default-brightness:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Default brightness level on boot.
> 
> It's not clear that this is an index when 'brightness-levels' is present 
> and absolute level when not. I wonder if we've been consistent on that?

Yeah.. And should we use "default-brightness-level" here like we do
in the kernel now?

Sorry if I've missed some discussion on this before..

> > +      brightness-levels = <0 4 8 16 32 64 128 255>;

What we're using for droid4 with an earlier version of this
patch set for the brightness-levels is generated backwards
with:

$ for i in 0 1 2 3 4 5 6 7; do echo "255 - ${i} * (256 / 8)" | bc; done

This produces the following range that seem to behave nicely:

brightness-levels = <31 63 95 127 159 191 223 255>;

Of course depends on the backing hardware, this is with
leds_lm3532 on droid4. But I think also the current example
in the binding might be from Pavel also for droid4?

If so, you might want to update the range :)

Regards,

Tony