Re: [PATCH 2/4] ARM: dts: omap4-droid4: Update backlight dt properties

[Dan Murphy]

Pavel

Thanks for the review.

On 3/8/19 9:14 AM, Pavel Machek wrote:
> Hi!
> 
>> Update the properties for the lm3532 device node for droid4.
>> With this change the backlight LED string and the keypad
>> LED strings will be controlled separately.
>>
>> Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
>> ---
>>  arch/arm/boot/dts/omap4-droid4-xt894.dts | 27 +++++++++++++++++-------
>>  1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
>> index e21ec929f096..94e3d53dbcf3 100644
>> --- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
>> +++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
>> @@ -6,6 +6,7 @@
>>  /dts-v1/;
>>  
>>  #include <dt-bindings/input/input.h>
>> +#include <dt-bindings/leds/leds-lm3532.h>
>>  #include "omap443x.dtsi"
>>  #include "motorola-cpcap-mapphone.dtsi"
>>  
>> @@ -383,20 +384,30 @@
>>  };
>>  
>>  &i2c1 {
>> -	lm3532@38 {
>> +	led-controller@38 {
>>  		compatible = "ti,lm3532";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>>  		reg = <0x38>;
>>  
>>  		enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
>>  
>> -		lcd_backlight: backlight {
>> -			compatible = "ti,lm3532-backlight";
>> +		ramp-up-ms = <LM3532_RAMP_1024us>;
>> +		ramp-down-ms = <LM3532_RAMP_65536us>;
> 
> I guess dt people will have some comments here. I'd expect
> 
> ramp-up-us = <1024> would be more natural.
> 

Actually ramp-up-us/ramp-down-us is more correct this is an error in this dt definition
and will be fixed in v2


>> +		lcd_backlight: led@0 {
>> +			reg = <0>;
>> +			led-sources = <2>;
>> +			ti,led-mode = <LM3532_BL_MODE_MANUAL>;
>> +			label = "backlight";
> 
> Ok, so we'll have lm3532::backlight. That is not too useful, as it
> does not tell userland what kaclight it is.
> 
> main_display::backlight ?
> 
> OTOH this one is not too important as backlight subsystem should
> handle this.
> 

For Droid4 I am not particular to any specific label.

And if the series in https://lkml.org/lkml/2018/11/7/144
is ever implemented then the label may change as well.

The driver forces the lm3532 string to be part of the label.

This was a discussion a while back with Jacek when I submitted other drivers.

>> +		led@1 {
>> +			reg = <1>;
>> +			led-sources = <1>;
>> +			ti,led-mode = <LM3532_BL_MODE_MANUAL>;
>> +			label = "keypad";
> 
> I guess best variant would be inputX::backlight here, but that might
> be tricky to implement.
> 

Yeah because we don't know what input the keyboard would be on.
Unless there are APIs to retrieve that info.  I have not looked at the input
framework in a while.

Dan

> 									Pavel
> 


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