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: echo 231 > colors/red_intensity // only cache the color in MC core echo 100 > brightness // do the actual hw update 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. I have also another question - what with linear vs logarithmic LP50xx brightness scale? I think we should make both options available to the userspace. -- Best regards, Jacek Anaszewski