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 <[email protected]>
---
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/[email protected]
---
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 <[email protected]>
This is similar to the existing led_colors[] array but is safer to use and
usable by everyone.
Getting string representations of color ids is useful for drivers
which are handling color IDs anyways, for example for the multicolor API.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/leds/led-core.c | 9 +++++++++
include/linux/leds.h | 10 ++++++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 89c9806cc97f..e0dd2284e84a 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -534,6 +534,15 @@ int led_compose_name(struct device *dev, struct led_init_data *init_data,
}
EXPORT_SYMBOL_GPL(led_compose_name);
+const char *led_get_color_name(u8 color_id)
+{
+ if (color_id >= ARRAY_SIZE(led_colors))
+ return NULL;
+
+ return led_colors[color_id];
+}
+EXPORT_SYMBOL_GPL(led_get_color_name);
+
enum led_default_state led_init_default_state_get(struct fwnode_handle *fwnode)
{
const char *state = NULL;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6300313c46b7..dedea965afbf 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -427,6 +427,16 @@ void led_sysfs_enable(struct led_classdev *led_cdev);
int led_compose_name(struct device *dev, struct led_init_data *init_data,
char *led_classdev_name);
+/**
+ * led_get_color_name - get string representation of color ID
+ * @color_id: The LED_COLOR_ID_* constant
+ *
+ * Get the string name of a LED_COLOR_ID_* constant.
+ *
+ * Returns: A string constant or NULL on an invalid ID.
+ */
+const char *led_get_color_name(u8 color_id);
+
/**
* led_sysfs_is_disabled - check if LED sysfs interface is disabled
* @led_cdev: the LED to query
--
2.45.1
On Fri, May 31, 2024 at 11:33 AM Thomas Weißschuh <[email protected]> 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 <[email protected]>
>
On 2024-06-02 18:30:06+0000, Dustin Howett wrote:
> On Fri, May 31, 2024 at 11:33 AM Thomas Weißschuh <[email protected]> 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 <[email protected]> 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 [李琼斯]
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 <[email protected]> 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/[email protected]/