Messages in this thread | | | Subject | Re: [PATCH v6 6/9] leds: multicolor: Introduce a multicolor class definition | From | Dan Murphy <> | Date | Fri, 20 Sep 2019 07:31:44 -0500 |
| |
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
| |