[PATCH v2 0/5] ChromeOS Embedded Controller LED driver

Thomas Weißschuh posted 5 patches 1 year, 8 months ago
There is a newer version of this series
MAINTAINERS                         |   5 +
drivers/leds/Kconfig                |  15 ++
drivers/leds/Makefile               |   1 +
drivers/leds/led-class-multicolor.c |   2 +-
drivers/leds/led-core.c             |  12 +-
drivers/leds/leds-cros_ec.c         | 297 ++++++++++++++++++++++++++++++++++++
drivers/leds/leds.h                 |   1 -
drivers/mfd/cros_ec_dev.c           |   9 ++
include/linux/leds.h                |  10 ++
9 files changed, 348 insertions(+), 4 deletions(-)
[PATCH v2 0/5] ChromeOS Embedded Controller LED driver
Posted by Thomas Weißschuh 1 year, 8 months ago
Add a LED driver that supports the LED devices exposed by the
ChromeOS Embedded Controller.

Patch 1-3 add a utility function to the led subsystem.
Patch 4 introduces the actual driver.
Patch 5 registers the driver through the cros_ec mfd devices.

Currently the driver introduces some non-standard LED functions.
(See "cros_ec_led_functions")

Tested on a Framework 13 AMD, Firmware 3.05.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v2:
- Cosmetic cleanups (Tzung-Bi)
- Add trailing comma to MFD cell array
- Rename LEDs and trigger to "chromeos" prefix, to align with kbd
  backlight driver
- Don't use type "rgb" anymore, they are only "multicolor"
- Align commit messages and subject to subsystem standards (Lee)
- Rename led_color_name() to led_get_color_name()
- The same for cros_ec_led_color_name()
- Link to v1: https://lore.kernel.org/r/20240520-cros_ec-led-v1-0-4068fc5c051a@weissschuh.net

---
Thomas Weißschuh (5):
      leds: core: Introduce led_get_color_name() function
      leds: multicolor: Use led_get_color_name() function
      leds: core: Unexport led_colors[] array
      leds: Add ChromeOS EC driver
      mfd: cros_ec: Register LED subdevice

 MAINTAINERS                         |   5 +
 drivers/leds/Kconfig                |  15 ++
 drivers/leds/Makefile               |   1 +
 drivers/leds/led-class-multicolor.c |   2 +-
 drivers/leds/led-core.c             |  12 +-
 drivers/leds/leds-cros_ec.c         | 297 ++++++++++++++++++++++++++++++++++++
 drivers/leds/leds.h                 |   1 -
 drivers/mfd/cros_ec_dev.c           |   9 ++
 include/linux/leds.h                |  10 ++
 9 files changed, 348 insertions(+), 4 deletions(-)
---
base-commit: 4a4be1ad3a6efea16c56615f31117590fd881358
change-id: 20240519-cros_ec-led-3efa24e3991e

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

Re: [PATCH v2 0/5] ChromeOS Embedded Controller LED driver
Posted by Dustin Howett 1 year, 8 months ago
On Fri, May 31, 2024 at 11:33 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> Add a LED driver that supports the LED devices exposed by the
> ChromeOS Embedded Controller.

I've tested this out on the Framework Laptop 13, 11th gen intel core
and AMD Ryzen 7040 editions.

It works fairly well! I found a couple minor issues in day-to-day use:

- Restoring the trigger to chromeos-auto does not always put the EC
back in control, e.g. the side lights no longer return to reporting
charge status.
  I believe this happens when you move from any trigger except "none"
to chromeos-auto, without first setting "none".
- The multicolor intensities report 6x 100 by default; if you set the
brightness with the intensities set as such, it becomes only red. I
would have
  expected intensities of 100 0 0 0 0 0 if that were the case

Thomas, I apologize for the duplicate message; my mail client config
here defaults to "reply" rather than "reply all."

Thanks,
Dustin

>
> Best regards,
> --
> Thomas Weißschuh <linux@weissschuh.net>
>
Re: [PATCH v2 0/5] ChromeOS Embedded Controller LED driver
Posted by Thomas Weißschuh 1 year, 8 months ago
On 2024-06-02 18:30:06+0000, Dustin Howett wrote:
> On Fri, May 31, 2024 at 11:33 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
> >
> > Add a LED driver that supports the LED devices exposed by the
> > ChromeOS Embedded Controller.
> 
> I've tested this out on the Framework Laptop 13, 11th gen intel core
> and AMD Ryzen 7040 editions.
> 
> It works fairly well! I found a couple minor issues in day-to-day use:

Thanks!

> - Restoring the trigger to chromeos-auto does not always put the EC
> back in control, e.g. the side lights no longer return to reporting
> charge status.
>   I believe this happens when you move from any trigger except "none"
> to chromeos-auto, without first setting "none".

Thanks for the report, I'll investigate that.

> - The multicolor intensities report 6x 100 by default; if you set the
> brightness with the intensities set as such, it becomes only red. I
> would have
>   expected intensities of 100 0 0 0 0 0 if that were the case

The EC will always use the first nonzero intensity for the color
channel. It isn't a real PWM color mix.
I don't think there are possibilities in the multicolor API to enforce
this. For the next revision I'll need to document this properly.
Also the default intensities could be better indeed.

> Thomas, I apologize for the duplicate message; my mail client config
> here defaults to "reply" rather than "reply all."

No worries!
Re: [PATCH v2 0/5] ChromeOS Embedded Controller LED driver
Posted by Lee Jones 1 year, 8 months ago
On Mon, 03 Jun 2024, Thomas Weißschuh wrote:

> On 2024-06-02 18:30:06+0000, Dustin Howett wrote:
> > On Fri, May 31, 2024 at 11:33 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
> > >
> > > Add a LED driver that supports the LED devices exposed by the
> > > ChromeOS Embedded Controller.
> > 
> > I've tested this out on the Framework Laptop 13, 11th gen intel core
> > and AMD Ryzen 7040 editions.
> > 
> > It works fairly well! I found a couple minor issues in day-to-day use:
> 
> Thanks!
> 
> > - Restoring the trigger to chromeos-auto does not always put the EC
> > back in control, e.g. the side lights no longer return to reporting
> > charge status.
> >   I believe this happens when you move from any trigger except "none"
> > to chromeos-auto, without first setting "none".
> 
> Thanks for the report, I'll investigate that.

So am I reviewing this set or waiting for the next version?

-- 
Lee Jones [李琼斯]
Re: [PATCH v2 0/5] ChromeOS Embedded Controller LED driver
Posted by Thomas Weißschuh 1 year, 8 months ago
On 2024-06-13 15:41:37+0000, Lee Jones wrote:
> On Mon, 03 Jun 2024, Thomas Weißschuh wrote:
> 
> > On 2024-06-02 18:30:06+0000, Dustin Howett wrote:
> > > On Fri, May 31, 2024 at 11:33 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
> > > >
> > > > Add a LED driver that supports the LED devices exposed by the
> > > > ChromeOS Embedded Controller.
> > > 
> > > I've tested this out on the Framework Laptop 13, 11th gen intel core
> > > and AMD Ryzen 7040 editions.
> > > 
> > > It works fairly well! I found a couple minor issues in day-to-day use:
> > 
> > Thanks!
> > 
> > > - Restoring the trigger to chromeos-auto does not always put the EC
> > > back in control, e.g. the side lights no longer return to reporting
> > > charge status.
> > >   I believe this happens when you move from any trigger except "none"
> > > to chromeos-auto, without first setting "none".
> > 
> > Thanks for the report, I'll investigate that.
> 
> So am I reviewing this set or waiting for the next version?

This specific bug is fixed by [0], which should be in your inbox.

I just sent v3 of the series, with only two tiny changes.
One more cosmetic and one for the coming revert to avoid the LED
hardware trigger deadlock.

Thanks for the review!

[0] https://lore.kernel.org/lkml/20240603-led-trigger-flush-v1-1-c904c6e2fb34@weissschuh.net/