Re: [PATCH v2 06/17] leds: leds-nuc: allow changing the LED colors

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Wed, 19 May 2021 14:04:51 +0200
Marek Behún <kabel@kernel.org> escreveu:

> On Wed, 19 May 2021 14:00:40 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> 
> > Em Wed, 19 May 2021 13:07:25 +0200
> > Pavel Machek <pavel@ucw.cz> escreveu:
> > 
> > > Hi!
> > >   
> > > > > We have multicolor LED framerwork in Linux. This should be implemented
> > > > > via that framework. Please do not implement your own way for RGB leds.
> > > > > 
> > > > > Marek    
> > > > 
> > > > I saw the multicolor LED framework, but IMO it won't fit here.
> > > > 
> > > > See, Linux doesn't have direct access to the LED. The access is
> > > > provided via ACPI WMI.    
> > > 
> > > So?
> > >   
> > > > The way BIOS reports the type of the led is via a bitmap flag.
> > > > So, the same LED can be represented with either single-color
> > > > or multi-color one. See:
> > > > 	https://www.intel.com/content/dam/support/us/en/documents/intel-nuc/WMI-Spec-Intel-NUC-NUC10ixFNx.pdf
> > > > 
> > > > 	Table 2.2 LED Color Type
> > > > 	Bit Number	Type
> > > > 	0		Dual-color Blue / Amber
> > > > 	1		Dual-color Blue / White
> > > > 	2		RGB-color
> > > > 	3		Single-color LED
> > > > 
> > > > Also as each NUC can support up to 7, and they may have a mix of
> > > > single color, dual color and RGB LEDs, if we would use the
> > > > multicolor class for the colored ones, that would mean that the
> > > > code will need to be duplicated, as, depending on what the BIOS
> > > > reports, the LED would need to be exposed either as via
> > > > led-multicolor-class or as leds-class.    
> > > 
> > > So?
> > >   
> > > > Worse than that, there's even a WMI command that would allow
> > > > to "switch LED type" (see page 8). On other words, the BIOS can
> > > > expose a "virtual" single-color LED, but actually implemented
> > > > in hardware using a RGB one, and this can be changed in real time.    
> > > 
> > > So you simply always use it as RGB one?  
> > 
> > Hmm... are you meaning that I should only use the multicolor led class,
> > even when the BIOS is reporting the LEDs as single color?
> > 
> > I can surely do that.
> 
> No. If the BIOS reports that the LED is single color, register a
> classic LED. If the BIOS reports a RGB LED, register a multi-color
> LED...

Ok. I'll try to change the code to work with both APIs. Let's hope
it won't result on too much code duplication.

-

I have one doubt about how to use this API. It is meant just
for the RGB LEDs, right?

NUC also have dual-color and multi-colored leds that don't
allow specifying the intensity of each component. All it lets
is to change the color (being, for instance, either blue or
white).

For those, the API should basically show the names of the
supported colors for each LED, letting the user change it
to some other color.

How should I map such leds? via led-class or via 
led-class-multicolor?

Thanks,
Mauro