On Thu, 20 May 2021 21:16:15 +0200 Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote: > So, assuming that we will have one trigger per each hardware > state, it could have something like (names subject to change): > > - hw:powerstate > - hw:disk_activity > - hw:ethernet_activity > - hw:wifi_active > - hw:power_limit > > Right? Yes, but we should really try to map ethernet_activity to netdev and disk_activity to a potential blkdev trigger :-) That's my opinion. > It still needs to indicate two other possible states: > > - software controlled led; > - led is disabled. > > Setting led's brightness to zero is different than disabling > it. > > Disabling can be done via BIOS, but BIOS config doesn't allow > setting the brightness. There are other difference on BIOS settings: > it allow disabling each/all LED controls and/or to disable software > control of each LED. > > So, we need a way at the API to uniquely identify when the LED > is software-controlled and when it is disabled. > Would it be something like: > > - hw:disable > > trigger? or better to implement it on a different way? What is the functional difference (visible to the user) between zero brightness and disabled LED? IMO if user says echo 0 >brightness you can just disable the LED. Or is this impossible? > > Is the speed of breathing/strobing also adjustable? Or only when > > pulsing? > > Yes, speed is also adjustable, from 0.1 to 1.0 HZ, in 0.1 Hz > (NUC 8 and above). > > The NUC6 API is more limited than NUC8+: it has just two > blink patterns (blink, fade), and only 3 frequencies are allowed > (0.25 Hz, 0.50 Hz and 1.0 Hz). > > > When this "hw:powerstate" trigger is enabled for this LED, > > only then another sysfs files should appear in this LED's sysfs > > directory. > > OK, makes sense. > > Out of curiosity: is it reliable to make sysfs nodes appear and > disappear dynamically? Does inotify (or something similar) can > be used to identify when such nodes appear/disappear? > > I remember a long time ago I wanted to use something like that > at the media (or edac?) subsystem, but someone (Greg, I think) > recommended otherwise due to some potential racing issues. No idea, but I would guess yes. > > I'd rather use one file for frequencies and one for intervals, and map > > in to an array, but that is just my preference... > > By intervals are you meaning 1/frequency? So, basically exposing > the frequency as two fields? If so, it sounds overkill to me to have both. Sorry, I meant one file for frequencies and one for patterns. > > Btw, maybe instead of "blink_behavior" it could use "blink_pattern". > > This would diverge from the datahseet name, but it probably describes > better what will be controlled when blink is enabled: > > - frequency (or inverval) > - pattern > > > Regarding the enum with 8 colors: are these > > colors red, yellow, green, cyan, blue, magenta? Because if so, then > > this is RGB with each channel being binary :) So you can again use > > multicolor framework. > > The dual-colored ones aren't RGB. Two types are supported: > - Blue/Amber > - Blue/White These would need a new API, ignore these for now. Marek