On Thu, 20 May 2021 21:16:15 +0200
Mauro Carvalho Chehab [off-list ref] 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?
 

quoted

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).

quoted

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.

quoted

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

quoted

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