mbox series

[v3,0/5] ChromeOS Embedded Controller LED driver

Message ID 20240613-cros_ec-led-v3-0-500b50f41e0f@weissschuh.net
Headers show
Series ChromeOS Embedded Controller LED driver | expand

Message

Thomas Weißschuh June 13, 2024, 2:48 p.m. UTC
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 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/20240531-cros_ec-led-v2-0-6cc34408b40d@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         | 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,

Comments

Lee Jones June 14, 2024, 9:12 a.m. UTC | #1
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 <linux@weissschuh.net>
> ---
> 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/20240531-cros_ec-led-v2-0-6cc34408b40d@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         | 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
Thomas Weißschuh June 14, 2024, 9:31 a.m. UTC | #2
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 <linux@weissschuh.net>
> > ---
> > 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/20240531-cros_ec-led-v2-0-6cc34408b40d@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         | 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
Lee Jones June 14, 2024, 9:34 a.m. UTC | #3
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 <linux@weissschuh.net>
> > > ---
> > > 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/20240531-cros_ec-led-v2-0-6cc34408b40d@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         | 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 June 20, 2024, 5:13 p.m. UTC | #4
On Thu, 13 Jun 2024 16:48:35 +0200, 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.
> 
> [...]

Applied, thanks!

[1/5] leds: core: Introduce led_get_color_name() function
      commit: ab52674e673ae3ecf457ee6036c24b2c5e0a2cca
[2/5] leds: multicolor: Use led_get_color_name() function
      commit: f5e22810770af92f69d266d4a7db505679264eab
[3/5] leds: core: Unexport led_colors[] array
      commit: e056183d2852d6e8cfd0da2a1e3eaf9c08801963
[4/5] leds: Add ChromeOS EC driver
      commit: 583745b475ace58b6223aee6d174ee46f04bce95
[5/5] mfd: cros_ec: Register LED subdevice
      commit: 2796819735adfc3cdf388696ab20a6780effb525

--
Lee Jones [李琼斯]