lkml.org 
[lkml]   [2019]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 6/9] leds: multicolor: Introduce a multicolor class definition
From
Date
Jacek

On 9/19/19 4:32 PM, Jacek Anaszewski wrote:
> Dan,
>
> On 9/19/19 3:07 AM, Dan Murphy wrote:
>> Jacek
>>
>> On 9/18/19 4:27 PM, Jacek Anaszewski wrote:
>>> Hi Dan,
>>>
>>> I think Greg's guidance clarified everything nicely -
>>> we will avoid <color> sub-dirs in favour of prefixes
>>> to *intensity and *max_intensity.
>> Yes I will make the change accordingly.  It will simplify the code.
>>> Before you will send an update I have some improvement
>>> ideas regarding the remnants after the previous approach,
>>> where single color intensity update resulted in updating
>>> hardware state. Now the update will happen only on write to
>>> brightness file, so we will not need color_set/color_get ops
>>> anymore.
>> I left those call backs in specifically for the LP50xx. Otherwise the
>> LEDs are only updated when the brightness file is written.
>> The LP50xx has an engine that performs the intensity computation for the
>> specific LED.  So there is no call back to the MC FW for calculating the
>> intensity.
>>
>> The brightness and intensity are written directly to the device and the
>> MCU in the device does all the computations so you have real time update.
> You can still handle that in brightness_set op. You need to compare
> which color channels have changed and update them in hardware in
> addition to setting LEDn_BRIGHTNESS register.
>
> And yes - even updating a single color will need two operations:

If we kept the ops then the LP50xx device would only need one operation
to the led intensity file to update the color.

> echo 231 > colors/red_intensity // only cache the color in MC core
> echo 100 > brightness // do the actual hw update
And this is the way the LP55xx device works now.
>
> Note that brightness value doesn't have to be necessarily different
> from the previous one here, but writing brightness file will be needed
> to trigger the hw update.
>
>> For the LP55xx device the LEDs are only updated when the brightness file
>> is written.
>>
>> I think we can leave those call backs in if device driver or product
>> development teams would like to use them.
> I'd not do that - it will be confusing. We can accomplish everything
> in brightness_set{_blocking} op. It will have also the advantage of
> same ABI semantics across all devices. Otherwise we would need separate
> documentation for devices like LP50xx.

OK I am not going to argue this I will just remove the ops even though I
don't agree.

Removing the ops will just make the LP50xx driver more complex then what
it needs to be.

I will post v8 later today.

>
> I have also another question - what with linear vs logarithmic
> LP50xx brightness scale? I think we should make both options available
> to the userspace.

I have no requirements from customers to provide this scaling.

It can be an enhancement to the driver later if we get the request.

Dan

\
 
 \ /
  Last update: 2019-09-20 14:29    [W:1.383 / U:0.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site