[GIT PULL v2] LM3532 backlight support improvements and relocation

[GIT PULL v2] LM3532 backlight support improvements and relocation

[Jacek Anaszewski]

Changes since v1:

- synchronized DT label properties in DT bindings with what has been agreed
  for the patch "ARM: dts: omap4-droid4: Update backlight dt properties"

The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b:

  Linux 5.1-rc1 (2019-03-17 14:22:26 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git tags/lm3532-driver-improvements

for you to fetch changes up to bc1b8492c764fea940fc66206047e37a7f8d77e1:

  leds: lm3532: Introduce the lm3532 LED driver (2019-04-07 20:45:49 +0200)

Thanks,
Jacek Anaszewski

----------------------------------------------------------------
LM3532 backlight driver improvements and relocation
----------------------------------------------------------------
Dan Murphy (4):
      dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
      ARM: dts: omap4-droid4: Update backlight dt properties
      mfd: ti-lmu: Remove LM3532 backlight driver references
      leds: lm3532: Introduce the lm3532 LED driver

 .../devicetree/bindings/leds/leds-lm3532.txt       | 101 +++
 Documentation/devicetree/bindings/mfd/ti-lmu.txt   |  20 -
 arch/arm/boot/dts/omap4-droid4-xt894.dts           |  27 +-
 drivers/leds/Kconfig                               |  10 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-lm3532.c                         | 683 +++++++++++++++++++++
 drivers/mfd/ti-lmu.c                               |  11 -
 include/linux/mfd/ti-lmu-register.h                |  44 --
 include/linux/mfd/ti-lmu.h                         |   1 -
 9 files changed, 813 insertions(+), 85 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3532.txt
 create mode 100644 drivers/leds/leds-lm3532.c

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2464 bytes --]

Hi!

> Changes since v1:
> 
> - synchronized DT label properties in DT bindings with what has been agreed
>   for the patch "ARM: dts: omap4-droid4: Update backlight dt properties"
> 
> The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b:
> 
>   Linux 5.1-rc1 (2019-03-17 14:22:26 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git tags/lm3532-driver-improvements
> 
> for you to fetch changes up to bc1b8492c764fea940fc66206047e37a7f8d77e1:
> 
>   leds: lm3532: Introduce the lm3532 LED driver (2019-04-07 20:45:49 +0200)
> 
> Thanks,
> Jacek Anaszewski
> 
> ----------------------------------------------------------------
> LM3532 backlight driver improvements and relocation
> ----------------------------------------------------------------
> Dan Murphy (4):
>       dt: lm3532: Add lm3532 dt doc and update ti_lmu doc

Sorry, this one will need more work.

When did Rob acknowledge this? I do not remember that mail & quick
google does not find it.

I still don't like the fact that it is using different binding from
other similar chips. It for example replaces "ramp-up-msec" with
"ramp-up-us".

This is certainly wrong:

+Optional properties if ALS mode is used:
+	  - ti,als-vmin - Minimum ALS voltage defined in Volts
+	  - ti,als-vmax - Maximum ALS voltage defined in Volts
+	  Per the data sheet the max ALS voltage is 2V and the min is
0V

...but milivolts (or microvolts?) make sense there, and clearly
milivolts are used because 2000V is way out of range.:

+	  ti,als-vmin = <0>;
+	  ti,als-vmax = <2000>;

Plus:

+Required child properties:
...
+	- ti,led-mode : Defines if the LED strings are manually controlled or
+			if the LED strings are controlled by the ALS.
+			       0x00 - LED strings are I2C controlled via full scale brightness control register
+        		  	  0x01 - LED strings are ALS controlled

Dunno. Normally we'd have "ti,automatic-led-mode", and if it was not
present, we'd default to manual, no? (Also "led-mode" is a bit too
generic. ti,als-enabled would be better description).

Plus, I'd kind of expect ALS enabled/disabled to be runtime controled,
not from the device tree.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

Hi!

> > Changes since v1:
> > 
> > - synchronized DT label properties in DT bindings with what has been agreed
> >   for the patch "ARM: dts: omap4-droid4: Update backlight dt properties"
> > 
> > The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b:
> > 
> >   Linux 5.1-rc1 (2019-03-17 14:22:26 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git tags/lm3532-driver-improvements
> > 
> > for you to fetch changes up to bc1b8492c764fea940fc66206047e37a7f8d77e1:
> > 
> >   leds: lm3532: Introduce the lm3532 LED driver (2019-04-07 20:45:49 +0200)
> > 
> > Thanks,
> > Jacek Anaszewski
> > 
> > ----------------------------------------------------------------
> > LM3532 backlight driver improvements and relocation
> > ----------------------------------------------------------------
> > Dan Murphy (4):
> >       dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
> 
> Sorry, this one will need more work.
> 
> When did Rob acknowledge this? I do not remember that mail & quick
> google does not find it.

Aha, found it now:

Date: Fri, 15 Mar 2019 18:35:01 -0500
From: Rob Herring <robh@kernel.org>
To: Dan Murphy <dmurphy@ti.com>
Subject: Re: [PATCH v4 1/4] dt: lm3532: Add lm3532 dt doc and update ti_lmu doc

 									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Dan Murphy]

Hi

On 4/7/19 5:50 PM, Pavel Machek wrote:
> Hi!
> 
>>> Changes since v1:
>>>
>>> - synchronized DT label properties in DT bindings with what has been agreed
>>>   for the patch "ARM: dts: omap4-droid4: Update backlight dt properties"
>>>
>>> The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b:
>>>
>>>   Linux 5.1-rc1 (2019-03-17 14:22:26 -0700)
>>>
>>> are available in the git repository at:
>>>
>>>   git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git tags/lm3532-driver-improvements
>>>
>>> for you to fetch changes up to bc1b8492c764fea940fc66206047e37a7f8d77e1:
>>>
>>>   leds: lm3532: Introduce the lm3532 LED driver (2019-04-07 20:45:49 +0200)
>>>
>>> Thanks,
>>> Jacek Anaszewski
>>>
>>> ----------------------------------------------------------------
>>> LM3532 backlight driver improvements and relocation
>>> ----------------------------------------------------------------
>>> Dan Murphy (4):
>>>       dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
>>
>> Sorry, this one will need more work.
>>
>> When did Rob acknowledge this? I do not remember that mail & quick
>> google does not find it.
> 
> Aha, found it now:
> 
> Date: Fri, 15 Mar 2019 18:35:01 -0500
> From: Rob Herring <robh@kernel.org>
> To: Dan Murphy <dmurphy@ti.com>
> Subject: Re: [PATCH v4 1/4] dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
> 

For reference to all here is the Reviewed-by from Rob.

https://lore.kernel.org/patchwork/patch/1050412/

>  									Pavel
> 


-- 
------------------
Dan Murphy

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Dan Murphy]

Pavel

Thanks for the review

On 4/7/19 5:17 PM, Pavel Machek wrote:
> Hi!
> 
>> Changes since v1:
>>
>> - synchronized DT label properties in DT bindings with what has been agreed
>>   for the patch "ARM: dts: omap4-droid4: Update backlight dt properties"
>>
>> The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b:
>>
>>   Linux 5.1-rc1 (2019-03-17 14:22:26 -0700)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git tags/lm3532-driver-improvements
>>
>> for you to fetch changes up to bc1b8492c764fea940fc66206047e37a7f8d77e1:
>>
>>   leds: lm3532: Introduce the lm3532 LED driver (2019-04-07 20:45:49 +0200)
>>
>> Thanks,
>> Jacek Anaszewski
>>
>> ----------------------------------------------------------------
>> LM3532 backlight driver improvements and relocation
>> ----------------------------------------------------------------
>> Dan Murphy (4):
>>       dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
> 
> Sorry, this one will need more work.
> 
> When did Rob acknowledge this? I do not remember that mail & quick
> google does not find it.
> 
> I still don't like the fact that it is using different binding from
> other similar chips. It for example replaces "ramp-up-msec" with
> "ramp-up-us".
> 

It does not replace anything.  The LM3532 has a completely different ramping table then
the other LMU devices.  The ramp times for this part is in uS not mS and there is no alignment on
common values or deltas.

> This is certainly wrong:
> 
> +Optional properties if ALS mode is used:
> +	  - ti,als-vmin - Minimum ALS voltage defined in Volts
> +	  - ti,als-vmax - Maximum ALS voltage defined in Volts
> +	  Per the data sheet the max ALS voltage is 2V and the min is
> 0V
> 
> ...but milivolts (or microvolts?) make sense there, and clearly
> milivolts are used because 2000V is way out of range.:
> 

I will send a patch that changes "Volts" to "millivolts" in the description.

> +	  ti,als-vmin = <0>;
> +	  ti,als-vmax = <2000>;
> 
> Plus:
> 
> +Required child properties:
> ...
> +	- ti,led-mode : Defines if the LED strings are manually controlled or
> +			if the LED strings are controlled by the ALS.
> +			       0x00 - LED strings are I2C controlled via full scale brightness control register
> +        		  	  0x01 - LED strings are ALS controlled
> 
> Dunno. Normally we'd have "ti,automatic-led-mode", and if it was not
> present, we'd default to manual, no? (Also "led-mode" is a bit too
> generic. ti,als-enabled would be better description).
> 
> Plus, I'd kind of expect ALS enabled/disabled to be runtime controled,
> not from the device tree.

We can always add runtime override control to the driver.

Just need to see if there is a common interface from input or IIO we can adopt.

Dan


> 
> Best regards,
> 									Pavel
> 


-- 
------------------
Dan Murphy

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Tony Lindgren]

* Dan Murphy <dmurphy@ti.com> [190408 15:54]:
> On 4/7/19 5:17 PM, Pavel Machek wrote:
> > Plus, I'd kind of expect ALS enabled/disabled to be runtime controled,
> > not from the device tree.
> 
> We can always add runtime override control to the driver.

Yeah that sounds good to me :) Sounds like this can be done
with follow up patches though.

> Just need to see if there is a common interface from input or IIO we can adopt.

Hmm can the lm3532 registers actually show the value
for the ALS sensors?

Regards,

Tony

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Dan Murphy]

Tony

On 4/8/19 2:30 PM, Tony Lindgren wrote:
> * Dan Murphy <dmurphy@ti.com> [190408 15:54]:
>> On 4/7/19 5:17 PM, Pavel Machek wrote:
>>> Plus, I'd kind of expect ALS enabled/disabled to be runtime controled,
>>> not from the device tree.
>> We can always add runtime override control to the driver.
> Yeah that sounds good to me :) Sounds like this can be done
> with follow up patches though.
>
>> Just need to see if there is a common interface from input or IIO we can adopt.
> Hmm can the lm3532 registers actually show the value
> for the ALS sensors?


No it can only report the current configured ALS zone not the LUX value.

Dan


> Regards,
>
> Tony

Re: [GIT PULL v2] LM3532 backlight support improvements and relocation

[Tony Lindgren]

* Dan Murphy <dmurphy@ti.com> [190408 20:04]:
> Tony
> 
> On 4/8/19 2:30 PM, Tony Lindgren wrote:
> > * Dan Murphy <dmurphy@ti.com> [190408 15:54]:
> > > On 4/7/19 5:17 PM, Pavel Machek wrote:
> > > > Plus, I'd kind of expect ALS enabled/disabled to be runtime controled,
> > > > not from the device tree.
> > > We can always add runtime override control to the driver.
> > Yeah that sounds good to me :) Sounds like this can be done
> > with follow up patches though.
> > 
> > > Just need to see if there is a common interface from input or IIO we can adopt.
> > Hmm can the lm3532 registers actually show the value
> > for the ALS sensors?
> 
> 
> No it can only report the current configured ALS zone not the LUX value.

Sounds like it should be just enable/disable or auto/manual
type toggle then in /sys somewhere for the users.

Not sure if ledtrig-backlight-auto vs ledtrig-backlight would
make sense?

Regards,

Tony