| Message ID | 20240531-cros_ec-led-v2-0-6cc34408b40d@weissschuh.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | ChromeOS Embedded Controller LED driver | expand |
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> >
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!
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?
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/
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,