Messages in this thread | | | Subject | Re: [PATCH v2 1/2] leds: Add Intel Cherry Trail Whiskey Cove PMIC LEDs | From | Jacek Anaszewski <> | Date | Sat, 16 Feb 2019 18:02:34 +0100 |
| |
Hi Hans,
On 2/16/19 12:14 AM, Hans de Goede wrote: > Hi, > > On 2/15/19 11:31 PM, Jacek Anaszewski wrote: >> On 2/15/19 11:26 PM, Hans de Goede wrote: > > <snip> > >>> I think that should work fine, which means that we can use the timer and >>> pattern trigger support for the blinking and breathing modes. >>> >>> That still leaves the switching between user and hw-control modes, >>> as discussed the hw-controlled mode could be modelled as a new >>> "hardware" >>> trigger, but then we cannot choose between on/blink/breathing when >>> in hw-controlled mode. As Pavel mentioned, that would require some >>> sort of composed trigger, where we have both the hardware and >>> timer triggers active for example. >>> >>> I think it might be easier to just allow turning on/off the hardware >>> control mode through a special "hardware_control" sysfs attribute and >>> then use the existing timer and pattern triggers for blinking / >>> breathing. >> >> Pattern trigger exposes pattern file by default and hw_pattern if >> pattern_set/get ops are provided. Writing them enables software and >> hardware pattern respectively. > > This is not about software vs hardware pattern. > > There are 2 *orthogonal*, separate problems/challenges with this LED > controller: > > 1) It has hardware blinking and breathing, as discussed this can be > controlled through the timer and pattern triggers, so this problem > is solved. > > 2) It has 2 operating modes: > > a) Automatic/hardware controlled, in this mode the LED is turned > off or on (where on can be continues on, blinking or breathing) > by the hardware itself, when in this mode we / userspace is not > in control of the LED > > b) Manual/user controlled mode, in this mode we / userspace can > control of the LED. > > Currently there is no API in the ledclass to switch a LED from > automatic controlled to user controlled and back, This is what > the proposed hardware trigger was for, to switch to automatic > mode. A problem with this is that we still want to be able > to chose between continues on, blinking or breathing (when on), > configure the max brightness, etc.
Yes, we do have the API to switch a LED from automatic (hardware accelerated) control to software control and back. This is pattern trigger, which exposes two files for setting pattern: pattern and hw_pattern. Writing pattern file switches the device to software control mode and writing hw_pattern switches it to the hardware control, with the possibility of defining device specific ABI syntax to enable particular pattern (blinking, breathing or event permanently on in case of this device).
The hw_pattern is device specific, so the device specific ABI documentation can state that when the pattern trigger is enabled for this device in hw accelerated mode, then it will blink the LED only when specific conditions are met - in this case it will be activation of charging.
Since timer trigger does not allow to explicitly choose between software and hardware timer engine, then I propose to not take it into account for supporting hardware blinking offered by this device. Enabling pattern trigger will just periodically set brightness of the LED.
Since pattern trigger is a superset of timer, then it can handle ordinary blinking as well.
> Since there cannot be more then 1 trigger active, with the > hardware trigger solution we cannot say we want automatic > control (hardware trigger) and blinking when the automatic > mode says the LED should be on (timer trigger) as that > would mean having 2 triggers active with the hardware > trigger solution. > > As such I think the switch between automatic and manual > control would be best exported through a special "hardware_control" > sysfs attribute and then use the existing timer and pattern > triggers for blinking / breathing. > > Regards, > > Hans >
-- Best regards, Jacek Anaszewski
| |