lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 8 Apr 2019 00:17:40 +0200
From:   Pavel Machek <pavel@....cz>
To:     Jacek Anaszewski <jacek.anaszewski@...il.com>, robh@...nel.org,
        dmurphy@...com
Cc:     lee.jones@...aro.org, tony@...mide.com, linux-leds@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL v2] LM3532 backlight support improvements and
 relocation

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

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)

Powered by blists - more mailing lists