Re: [PATCH v6 6/9] leds: multicolor: Introduce a multicolor class definition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux