| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f69062aa-e9e8-44c9-ad84-d9263747528c@wanadoo.fr>
Date: Sat, 6 Dec 2025 12:18:14 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: tpearson@...torengineering.com
Cc: Georgy.Yakovlev@...y.com, conor+dt@...nel.org,
devicetree@...r.kernel.org, krzysztof.kozlowski+dt@...aro.org,
lee@...nel.org, linux-kernel@...r.kernel.org, robh+dt@...nel.org,
sanastasio@...torengineering.com
Subject: Re: [PATCH v5 3/4] led: sony-cronos-smc: Add RGB LED driver for Sony
Cronos SMC
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@...lic.gmane.org>
> ---
> 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
Powered by blists - more mailing lists