2024-06-13 14:49:56

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 0/5] ChromeOS Embedded Controller LED driver

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 v3:
- Set default_trigger explicitly as the LED core doesn't do this anymore
- Only set intensity for first subled by default
- Link to v2: https://lore.kernel.org/r/[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 | 299 ++++++++++++++++++++++++++++++++++++
drivers/leds/leds.h | 1 -
drivers/mfd/cros_ec_dev.c | 9 ++
include/linux/leds.h | 10 ++
9 files changed, 350 insertions(+), 4 deletions(-)
---
base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5
change-id: 20240519-cros_ec-led-3efa24e3991e

Best regards,
--
Thomas Weißschuh <[email protected]>



2024-06-13 14:49:58

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 1/5] leds: core: Introduce led_get_color_name() function

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


2024-06-13 14:50:07

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 5/5] mfd: cros_ec: Register LED subdevice

Add ChromeOS EC-based LED control as EC subdevice.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/mfd/cros_ec_dev.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index a52d59cc2b1e..d8408054ba15 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -99,6 +99,10 @@ static const struct mfd_cell cros_ec_wdt_cells[] = {
{ .name = "cros-ec-wdt", }
};

+static const struct mfd_cell cros_ec_led_cells[] = {
+ { .name = "cros-ec-led", },
+};
+
static const struct cros_feature_to_cells cros_subdevices[] = {
{
.id = EC_FEATURE_CEC,
@@ -125,6 +129,11 @@ static const struct cros_feature_to_cells cros_subdevices[] = {
.mfd_cells = cros_ec_wdt_cells,
.num_cells = ARRAY_SIZE(cros_ec_wdt_cells),
},
+ {
+ .id = EC_FEATURE_LED,
+ .mfd_cells = cros_ec_led_cells,
+ .num_cells = ARRAY_SIZE(cros_ec_led_cells),
+ },
};

static const struct mfd_cell cros_ec_platform_cells[] = {

--
2.45.2


2024-06-14 09:12:33

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ChromeOS Embedded Controller LED driver

On Thu, 13 Jun 2024, Thomas Weißschuh wrote:

> 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 v3:
> - Set default_trigger explicitly as the LED core doesn't do this anymore
> - Only set intensity for first subled by default
> - Link to v2: https://lore.kernel.org/r/[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 | 299 ++++++++++++++++++++++++++++++++++++
> drivers/leds/leds.h | 1 -
> drivers/mfd/cros_ec_dev.c | 9 ++
> include/linux/leds.h | 10 ++
> 9 files changed, 350 insertions(+), 4 deletions(-)
> ---
> base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5
> change-id: 20240519-cros_ec-led-3efa24e3991e

Applied and submitted for testing.

All being well, I'll follow-up with a cross-subsystem pull-request shortly

--
Lee Jones [李琼斯]

2024-06-14 09:31:23

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ChromeOS Embedded Controller LED driver

On 2024-06-14 10:12:20+0000, Lee Jones wrote:
> On Thu, 13 Jun 2024, Thomas Weißschuh wrote:
>
> > 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 v3:
> > - Set default_trigger explicitly as the LED core doesn't do this anymore
> > - Only set intensity for first subled by default
> > - Link to v2: https://lore.kernel.org/r/[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 | 299 ++++++++++++++++++++++++++++++++++++
> > drivers/leds/leds.h | 1 -
> > drivers/mfd/cros_ec_dev.c | 9 ++
> > include/linux/leds.h | 10 ++
> > 9 files changed, 350 insertions(+), 4 deletions(-)
> > ---
> > base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5
> > change-id: 20240519-cros_ec-led-3efa24e3991e
>
> Applied and submitted for testing.
>
> All being well, I'll follow-up with a cross-subsystem pull-request shortly

Thanks!

I'm not sure which effect this application has on the review comments
you gave to patch 4 (thanks for those, too).

After implementing your requests, should I
* resubmit the whole series
* resubmit only patch 4
* send an incremental patch on top of the series
?


Thomas

2024-06-14 09:34:58

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ChromeOS Embedded Controller LED driver

On Fri, 14 Jun 2024, Thomas Weißschuh wrote:

> On 2024-06-14 10:12:20+0000, Lee Jones wrote:
> > On Thu, 13 Jun 2024, Thomas Weißschuh wrote:
> >
> > > 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 v3:
> > > - Set default_trigger explicitly as the LED core doesn't do this anymore
> > > - Only set intensity for first subled by default
> > > - Link to v2: https://lore.kernel.org/r/[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 | 299 ++++++++++++++++++++++++++++++++++++
> > > drivers/leds/leds.h | 1 -
> > > drivers/mfd/cros_ec_dev.c | 9 ++
> > > include/linux/leds.h | 10 ++
> > > 9 files changed, 350 insertions(+), 4 deletions(-)
> > > ---
> > > base-commit: 2ccbdf43d5e758f8493a95252073cf9078a5fea5
> > > change-id: 20240519-cros_ec-led-3efa24e3991e
> >
> > Applied and submitted for testing.
> >
> > All being well, I'll follow-up with a cross-subsystem pull-request shortly
>
> Thanks!
>
> I'm not sure which effect this application has on the review comments
> you gave to patch 4 (thanks for those, too).
>
> After implementing your requests, should I
> * resubmit the whole series
> * resubmit only patch 4
> * send an incremental patch on top of the series

Incremental patch please.

--
Lee Jones [李琼斯]