Re: [PATCH v5 3/4] led: sony-cronos-smc: Add RGB LED driver for Sony Cronos SMC
From: Christophe JAILLET
Date: Sat Dec 06 2025 - 06:27:38 EST
Le 04/12/2025 à 19:50, Timothy Pearson a écrit :
The Sony Cronos Platform Controller is a multi-purpose platform controller with
an integrated multi-channel RGB LED controller. The LED controller is a
pseudo-RGB device with only two states for each of the RGB subcomponents of
each LED, but is exposed as a full RGB device for ease of integration with
userspace software. Internal thresholding is used to convert the color values
to the required on/off RGB subcomponent controls.
Signed-off-by: Timothy Pearson <tpearson-z0qzliK6Om0mgXJStvpl+vpXobYPEAuW@xxxxxxxxxxxxxxxx>
---
drivers/leds/Kconfig | 19 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-sony-cronos.c | 378 ++++++++++++++++++++++++++++++++
3 files changed, 398 insertions(+)
create mode 100644 drivers/leds/leds-sony-cronos.c
Hi,
kernel test robot has complained, so it is still time to suggest some small clean-ups, if of any interest.
...
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 9a0333ec1a86..6dbcf747cab6 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
obj-$(CONFIG_LEDS_PWM) += leds-pwm.o
obj-$(CONFIG_LEDS_QNAP_MCU) += leds-qnap-mcu.o
obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o
+obj-$(CONFIG_LEDS_SONY_CRONOS) += leds-sony-cronos.o
Keep alphabetical order?
obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
obj-$(CONFIG_LEDS_ST1202) += leds-st1202.o
obj-$(CONFIG_LEDS_SUN50I_A100) += leds-sun50i-a100.o
diff --git a/drivers/leds/leds-sony-cronos.c b/drivers/leds/leds-sony-cronos.c
new file mode 100644
index 000000000000..ce71a8b6ce94
--- /dev/null
+++ b/drivers/leds/leds-sony-cronos.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LED driver for Sony Cronos SMCs
+ * Copyright (C) 2012 Dialog Semiconductor Ltd.
+ * Copyright (C) 2023 Sony Interactive Entertainment
+ * Copyright (C) 2025 Raptor Engineering, LLC
+ */
+
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/mfd/sony-cronos.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
Sometimes, it is preferred to keep #include in alphabetic order.
+
+/* Masks and Bit shifts */
+#define CRONOS_LEDS_STATUS_FLASHING_MASK 0x40
+#define CRONOS_LEDS_STATUS_FLASHING_SHIFT 6
+#define CRONOS_LEDS_STATUS_COLOR_MASK 0x07
+#define CRONOS_LEDS_STATUS_COLOR_SHIFT 0
...
+enum sony_cronos_led_id {
+ LED_ID_CCM1_STATUS = 0x00,
+ LED_ID_CCM2_STATUS,
+ LED_ID_CCM3_STATUS,
+ LED_ID_CCM4_STATUS,
+ LED_ID_SWITCH_STATUS,
+ LED_ID_SMC_STATUS,
+
+ LED_ID_CCM1_LINK,
+ LED_ID_CCM2_LINK,
+ LED_ID_CCM3_LINK,
+ LED_ID_CCM4_LINK,
+ LED_ID_SWITCH_LINK,
+
+ LED_ID_CCM1_POWER,
+ LED_ID_CCM2_POWER,
+ LED_ID_CCM3_POWER,
+ LED_ID_CCM4_POWER,
+
+ LED_ID_COUNT,
Unneeded trailing comma after a terminator.
Moreover, LED_ID_COUNT looks unused. Is it needed?
+};
+
+enum sony_cronos_led_type {
+ LED_TYPE_STATUS,
+ LED_TYPE_LINK,
+ LED_TYPE_POWER,
+};
...
+static int cronos_led_color_store(struct sony_cronos_smc *chip, struct sony_cronos_led *led)
+{
+ u8 byte;
+ u8 color_mask;
+ u8 color_shift;
+ u8 color_key_red;
+ u8 color_key_green;
+ u8 color_key_blue;
+ int ret;
+
+ if (led->led_type == LED_TYPE_STATUS) {
+ color_mask = CRONOS_LEDS_STATUS_COLOR_MASK;
+ color_shift = CRONOS_LEDS_STATUS_COLOR_SHIFT;
+ } else if (led->led_type == LED_TYPE_LINK) {
+ color_mask = CRONOS_LEDS_LINK_COLOR_MASK;
+ color_shift = CRONOS_LEDS_LINK_COLOR_SHIFT;
+ } else if (led->led_id == LED_ID_CCM1_POWER) {
+ color_mask = CRONOS_LEDS_CCM1_POWER_COLOR_MASK;
+ color_shift = CRONOS_LEDS_CCM1_POWER_COLOR_SHIFT;
+ } else if (led->led_id == LED_ID_CCM2_POWER) {
+ color_mask = CRONOS_LEDS_CCM2_POWER_COLOR_MASK;
+ color_shift = CRONOS_LEDS_CCM2_POWER_COLOR_SHIFT;
+ } else if (led->led_id == LED_ID_CCM3_POWER) {
+ color_mask = CRONOS_LEDS_CCM3_POWER_COLOR_MASK;
+ color_shift = CRONOS_LEDS_CCM3_POWER_COLOR_SHIFT;
+ } else if (led->led_id == LED_ID_CCM4_POWER) {
+ color_mask = CRONOS_LEDS_CCM4_POWER_COLOR_MASK;
+ color_shift = CRONOS_LEDS_CCM4_POWER_COLOR_SHIFT;
+ } else
+ return ret;
+
+ switch (led->led_type) {
+ case LED_TYPE_POWER:
+ color_key_red = LED_COLOR_POWER_RED;
+ color_key_green = LED_COLOR_POWER_GREEN;
+ /* Blue channel does not exist for CCM power LEDs */
+ color_key_blue = LED_COLOR_POWER_OFF;
+ break;
+ default:
+ color_key_red = LED_COLOR_RED;
+ color_key_green = LED_COLOR_GREEN;
+ color_key_blue = LED_COLOR_BLUE;
+ }
+
+ /* Assemble SMC color command code */
+ byte = LED_COLOR_POWER_OFF;
+ if (led->subled_info[0].brightness > 128)
+ byte |= color_key_red;
+ if (led->subled_info[1].brightness > 128)
+ byte |= color_key_green;
+ if (led->subled_info[2].brightness > 128)
+ byte |= color_key_blue;
+
+ ret = regmap_update_bits(chip->regmap, led->led_register, color_mask, byte << color_shift);
+ if (ret) {
+ dev_err(chip->dev, "Failed to set color value 0x%02x to LED register 0x%02x", byte,
Missing \n at the end of the message.
+ led->led_register);
+ return ret;
+ }
+ return 0;
+}
...
+static const struct of_device_id sony_cronos_led_of_id_table[] = {
+ { .compatible = "sie,cronos-led", },
+ {},
Unneeded trailing comma after a terminator.
+};
+MODULE_DEVICE_TABLE(of, sony_cronos_led_of_id_table);
...
CJ