* [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver
@ 2025-05-14 10:36 Steffen Trumtrar
2025-05-14 10:36 ` [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller Steffen Trumtrar
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Steffen Trumtrar @ 2025-05-14 10:36 UTC (permalink / raw)
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Steffen Trumtrar, Pavel Machek
Cc: linux-leds, devicetree, linux-kernel, kernel, Steffen Trumtrar
The lp5860 is a LED matrix driver with 18 constant current sinks and 11
scan switches which allows controlling up to 198 LED dots.
This series adds just the basic support for the device on the SPI bus.
It is also possible to use it on I2C. The interface can be
switched/selected via an interface select pin.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
Changes in v2:
- add open and short detection
- add ABI documentation
- fix devicetree binding (maxItems/minItems)
- fix commit message to imperative mood
- minor cleanup
- Link to v1: https://lore.kernel.org/r/20250220-v6-14-topic-ti-lp5860-v1-0-42874bdc7513@pengutronix.de
---
Steffen Trumtrar (4):
dt-bindings: leds: add lp5860 LED controller
leds: add support for TI LP5860 LED driver chip
leds: lp5860: save count of multi_leds
leds: lp5860: detect and report fault state
Documentation/ABI/testing/sysfs-class-spi-lp5860 | 73 ++++
.../devicetree/bindings/leds/leds-lp5860.yaml | 112 ++++++
drivers/leds/Kconfig | 23 ++
drivers/leds/Makefile | 2 +
drivers/leds/leds-lp5860-core.c | 413 +++++++++++++++++++++
drivers/leds/leds-lp5860-spi.c | 101 +++++
drivers/leds/leds-lp5860.h | 326 ++++++++++++++++
7 files changed, 1050 insertions(+)
---
base-commit: 63ee0d00c8fc276d8653e71fb2c914cf257db2b2
change-id: 20250219-v6-14-topic-ti-lp5860-f25a48b62c79
Best regards,
--
Steffen Trumtrar <s.trumtrar@pengutronix.de>
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller 2025-05-14 10:36 [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Steffen Trumtrar @ 2025-05-14 10:36 ` Steffen Trumtrar 2025-05-21 9:02 ` Krzysztof Kozlowski 2025-05-14 10:36 ` [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip Steffen Trumtrar ` (3 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Steffen Trumtrar @ 2025-05-14 10:36 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek Cc: linux-leds, devicetree, linux-kernel, kernel, Steffen Trumtrar The lp5860 is a LED matrix driver with 18 constant current sinks and 11 scan switches for 198 LED dots: * Supply range from 2.7 V to 5.5 V * 0.1mA - 50mA per current sink * 1MHz I2C and 12MHz SPI control interface * 8-bit analog dimming * 8/16-bit PWM dimming * individual ON and OFF control for each LED dot * globat 3-bit Maximum Current setting for all LED dots * individual LED dot open/short detection Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> --- .../devicetree/bindings/leds/leds-lp5860.yaml | 112 +++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/leds-lp5860.yaml b/Documentation/devicetree/bindings/leds/leds-lp5860.yaml new file mode 100644 index 0000000000000000000000000000000000000000..330ff42b27fb19395e64f813da4b0ab1fa22fe69 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-lp5860.yaml @@ -0,0 +1,112 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-lp5860.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LED driver for LP5860 RGB LED from Texas Instruments. + +maintainers: + - Steffen Trumtrar <kernel@pengutronix.de> + +description: | + The LP5860 is multi-channel, I2C and SPI RGB LED Driver that can group RGB LEDs + into a LED group or control them individually. + + For more product information please see the link below: + https://www.ti.com/lit/ds/symlink/lp5860.pdf + +properties: + compatible: + enum: + - ti,lp5860 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 12000000 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + +patternProperties: + '^multi-led@[0-9a-f]+$': + type: object + $ref: leds-class-multicolor.yaml# + unevaluatedProperties: false + + properties: + reg: + minimum: 0 + maximum: 198 + description: + This property denotes the LED module number that is used + for the child node. + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + patternProperties: + "^led@[0-9a-f]+$": + type: object + $ref: common.yaml# + unevaluatedProperties: false + + properties: + reg: + maxItems: 1 + + required: + - reg + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/leds/common.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@0 { + compatible = "ti,lp5860"; + reg = <0x0>; + #address-cells = <1>; + #size-cells = <0>; + + multi-led@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x0>; + color = <LED_COLOR_ID_RGB>; + + led@0 { + reg = <0x0>; + color = <LED_COLOR_ID_RED>; + }; + + led@1 { + reg = <0x1>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@2 { + reg = <0x2>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + }; + }; -- 2.47.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller 2025-05-14 10:36 ` [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller Steffen Trumtrar @ 2025-05-21 9:02 ` Krzysztof Kozlowski 0 siblings, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2025-05-21 9:02 UTC (permalink / raw) To: Steffen Trumtrar Cc: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek, linux-leds, devicetree, linux-kernel On Wed, May 14, 2025 at 12:36:28PM GMT, Steffen Trumtrar wrote: > The lp5860 is a LED matrix driver with 18 constant current sinks and 11 > scan switches for 198 LED dots: > * Supply range from 2.7 V to 5.5 V > * 0.1mA - 50mA per current sink > * 1MHz I2C and 12MHz SPI control interface > * 8-bit analog dimming > * 8/16-bit PWM dimming > * individual ON and OFF control for each LED dot > * globat 3-bit Maximum Current setting for all LED dots > * individual LED dot open/short detection > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > --- > .../devicetree/bindings/leds/leds-lp5860.yaml | 112 +++++++++++++++++++++ > 1 file changed, 112 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/leds-lp5860.yaml b/Documentation/devicetree/bindings/leds/leds-lp5860.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..330ff42b27fb19395e64f813da4b0ab1fa22fe69 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lp5860.yaml > @@ -0,0 +1,112 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-lp5860.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LED driver for LP5860 RGB LED from Texas Instruments. > + > +maintainers: > + - Steffen Trumtrar <kernel@pengutronix.de> > + > +description: | > + The LP5860 is multi-channel, I2C and SPI RGB LED Driver that can group RGB LEDs > + into a LED group or control them individually. > + > + For more product information please see the link below: > + https://www.ti.com/lit/ds/symlink/lp5860.pdf > + > +properties: > + compatible: > + enum: > + - ti,lp5860 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 12000000 So that's a SPI device? Where is the definition of this field? You miss ref to peripheral props. ... > +required: > + - compatible > + - reg > + > +additionalProperties: false And this is unevaluated. See other examples. > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> Where do you use this header? > + #include <dt-bindings/leds/common.h> > + > + spi { > + #address-cells = <1>; Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip 2025-05-14 10:36 [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Steffen Trumtrar 2025-05-14 10:36 ` [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller Steffen Trumtrar @ 2025-05-14 10:36 ` Steffen Trumtrar 2025-05-23 10:28 ` Lee Jones 2025-05-14 10:36 ` [PATCH v2 3/4] leds: lp5860: save count of multi_leds Steffen Trumtrar ` (2 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Steffen Trumtrar @ 2025-05-14 10:36 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek Cc: linux-leds, devicetree, linux-kernel, kernel, Steffen Trumtrar Add support for the Texas Instruments LP5860 LED driver chip via SPI interfaces. The LP5860 is an LED matrix driver for up to 196 LEDs, which supports short and open detection of the individual channel select lines. The original driver is from an unknown author at Texas Instruments. Only the cryptic handle 'zlzx' is known. Co-developed-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> --- Documentation/ABI/testing/sysfs-class-spi-lp5860 | 23 ++ drivers/leds/Kconfig | 23 ++ drivers/leds/Makefile | 2 + drivers/leds/leds-lp5860-core.c | 276 ++++++++++++++++++++ drivers/leds/leds-lp5860-spi.c | 99 +++++++ drivers/leds/leds-lp5860.h | 315 +++++++++++++++++++++++ 6 files changed, 738 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-class-spi-lp5860 b/Documentation/ABI/testing/sysfs-class-spi-lp5860 new file mode 100644 index 0000000000000000000000000000000000000000..d24b49d38ecae55f1a1a4e465fbe71d30eff497e --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-spi-lp5860 @@ -0,0 +1,23 @@ +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/b_current_set +Date: May 2025 +KernelVersion: 6.15 +Contact: Steffen Trumtrar <kernel@pengutronix.de> +Description: + Contains and sets the current for the B color group. + Can be adjusted in 128 steps from 0% to 100% of the maximum output current. + +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/g_current_set +Date: May 2025 +KernelVersion: 6.15 +Contact: Steffen Trumtrar <kernel@pengutronix.de> +Description: + Contains and sets the current for the G color group. + Can be adjusted in 128 steps from 0% to 100% of the maximum output current. + +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/r_current_set +Date: May 2025 +KernelVersion: 6.15 +Contact: Steffen Trumtrar <kernel@pengutronix.de> +Description: + Contains and sets the current for the R color group. + Can be adjusted in 128 steps from 0% to 100% of the maximum output current. diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a104cbb0a0019a0510f7e33b4e70e13083efb390..d2b6a53f2ea2cef751cd19774e05f966e66a0a5f 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -483,6 +483,29 @@ config LEDS_LP5569 Driver provides direct control via LED class and interface for programming the engines. +config LEDS_LP5860_CORE + tristate "Core Driver for TI LP5860" + depends on LEDS_CLASS + depends on LEDS_CLASS_MULTICOLOR + depends on OF + select FW_LOADER + select FW_LOADER_USER_HELPER + select REGMAP + help + This option supports common operations for LP5860 devices. + The device can be used with SPI or I2C bus. + +config LEDS_LP5860_SPI + tristate "LED Support for TI LP5860 SPI LED driver chip" + depends on SPI + depends on LEDS_LP5860_CORE + help + If you say yes here you get support for the Texas Instruments + LP5860 LED driver for SPI bus connections. + + To compile this driver as a module, choose M here: the + module will be called lp5860_spi. + config LEDS_LP8501 tristate "LED Support for TI LP8501 LED driver chip" depends on LEDS_CLASS && I2C diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 2f170d69dcbf517cf7df98f25c49e3b445da3470..714d37a8361b757b71d37e95211ea04aed5d5d59 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -54,6 +54,8 @@ obj-$(CONFIG_LEDS_LP5523) += leds-lp5523.o obj-$(CONFIG_LEDS_LP5562) += leds-lp5562.o obj-$(CONFIG_LEDS_LP5569) += leds-lp5569.o obj-$(CONFIG_LEDS_LP55XX_COMMON) += leds-lp55xx-common.o +obj-$(CONFIG_LEDS_LP5860_CORE) += leds-lp5860-core.o +obj-$(CONFIG_LEDS_LP5860_SPI) += leds-lp5860-spi.o obj-$(CONFIG_LEDS_LP8501) += leds-lp8501.o obj-$(CONFIG_LEDS_LP8788) += leds-lp8788.o obj-$(CONFIG_LEDS_LP8860) += leds-lp8860.o diff --git a/drivers/leds/leds-lp5860-core.c b/drivers/leds/leds-lp5860-core.c new file mode 100644 index 0000000000000000000000000000000000000000..c3ce2528783a1910a267c1b35a65ad27a63083db --- /dev/null +++ b/drivers/leds/leds-lp5860-core.c @@ -0,0 +1,276 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2025 Pengutronix, + * Steffen Trumtrar <kernel@pengutronix.de + * + * Based on: + * + * LP5860 Driver + * + * Copyright 2012 Texas Instruments + * + * Author: Texas Instruments, zlzx + * + */ + +#include <linux/gpio.h> +#include <linux/led-class-multicolor.h> +#include <linux/module.h> +#include <linux/of_gpio.h> +#include <linux/of_platform.h> +#include <linux/regmap.h> + +#include "leds-lp5860.h" + +static struct lp5860_led *mcled_cdev_to_led(struct led_classdev_mc *mc_cdev) +{ + return container_of(mc_cdev, struct lp5860_led, mc_cdev); +} + +LP5860_SHOW_MODE(r_current_set, LP5860_R_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) +LP5860_STORE_MODE(r_current_set, LP5860_R_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) +static LP5860_DEV_ATTR_RW(r_current_set); + +LP5860_SHOW_MODE(g_current_set, LP5860_G_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) +LP5860_STORE_MODE(g_current_set, LP5860_G_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) +static LP5860_DEV_ATTR_RW(g_current_set); + +LP5860_SHOW_MODE(b_current_set, LP5860_B_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) +LP5860_STORE_MODE(b_current_set, LP5860_B_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) +static LP5860_DEV_ATTR_RW(b_current_set); + +static struct attribute *lp5860_attributes[] = { + &dev_attr_r_current_set.attr, + &dev_attr_g_current_set.attr, + &dev_attr_b_current_set.attr, + NULL, +}; + +static const struct attribute_group lp5860_group = { + .attrs = lp5860_attributes, +}; + +static int lp5860_dotonoff(struct lp5860_led *led, unsigned int dot, + bool enable) +{ + unsigned int offset = dot / LP5860_MAX_DOT_ONOFF_GROUP_NUM; + unsigned int mask = BIT(dot % LP5860_MAX_DOT_ONOFF_GROUP_NUM); + + return regmap_update_bits(led->priv->regmap, + LP5860_DOT_ONOFF_START + offset, mask, + enable ? 0xff : 0x0); +} + +static int lp5860_set_mc_brightness(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); + struct lp5860_led *led = mcled_cdev_to_led(mc_cdev); + int i; + + led_mc_calc_color_components(mc_cdev, brightness); + + for (i = 0; i < led->mc_cdev.num_colors; i++) { + unsigned int channel = mc_cdev->subled_info[i].channel; + unsigned int led_brightness = mc_cdev->subled_info[i].brightness; + int ret; + + ret = lp5860_dotonoff(led, channel, led_brightness); + if (ret) + return ret; + + ret = regmap_write(led->priv->regmap, + LP5860_PWM_BRI_START + channel, + led_brightness); + if (ret) + return ret; + } + + return 0; +} + +static int lp5860_enable_disable(struct lp5860 *led, int enable_disable) +{ + return regmap_write(led->regmap, LP5860_CHIP_EN, enable_disable); +} + +static int lp5860_led_init_default_state(struct lp5860_led *led, + unsigned int channel) +{ + unsigned int level; + int ret; + + ret = regmap_read(led->priv->regmap, LP5860_PWM_BRI_START + channel, + &level); + + switch (led->default_state) { + case LEDS_DEFSTATE_ON: + led->brightness = LP5860_MAX_BRIGHTNESS; + break; + case LEDS_DEFSTATE_KEEP: + led->brightness = min(level, LP5860_MAX_BRIGHTNESS); + break; + default: + led->brightness = 0; + break; + } + + return lp5860_set_mc_brightness(&led->mc_cdev.led_cdev, led->brightness); +} + +static int lp5860_probe_leds(struct fwnode_handle *child, struct lp5860 *lp, + struct lp5860_led *led, int num_leds) +{ + u32 led_banks[LP5860_MAX_LED] = {0}; + int led_number; + int ret; + + if (num_leds > 1) { + if (num_leds > LP5860_MAX_LED) { + dev_err(lp->dev, "too many leds specified\n"); + return -EINVAL; + } + + ret = fwnode_property_read_u32_array(child, "reg", led_banks, + num_leds); + if (ret) { + dev_err(lp->dev, "reg property is missing\n"); + return ret; + } + } else { + ret = fwnode_property_read_u32(child, "reg", &led_number); + if (ret) { + dev_err(lp->dev, "led reg property missing\n"); + return ret; + } + + if (led_number > LP5860_MAX_LED) { + dev_err(lp->dev, "led-sources property is invalid\n"); + return -EINVAL; + } + + led->chan_nr = led_number; + } + + return 0; +} + +static int lp5860_probe_dt(struct lp5860 *lp) +{ + struct fwnode_handle *led_node = NULL; + struct fwnode_handle *subchild; + struct led_init_data init_data = {}; + struct led_classdev *led_cdev; + struct mc_subled *mc_led_info; + struct lp5860_led *led; + int num_channels; + u32 color_id; + u32 channel; + int i = 0; + int ret; + + device_for_each_child_node_scoped(lp->dev, child) { + + led = &lp->leds[i]; + ret = fwnode_property_count_u32(child, "reg"); + if (ret < 0) { + dev_err(lp->dev, "reg property is invalid\n"); + return ret; + } + + ret = lp5860_probe_leds(child, lp, led, ret); + if (ret) + return ret; + + init_data.fwnode = child; + + num_channels = 0; + fwnode_for_each_available_child_node(child, subchild) + num_channels++; + + if (!num_channels || num_channels > LP5860_MAX_LED_CHANNELS) + return -EINVAL; + + mc_led_info = devm_kcalloc(lp->dev, num_channels, + sizeof(*mc_led_info), GFP_KERNEL); + if (!mc_led_info) + return -ENOMEM; + + num_channels = 0; + led->priv = lp; + fwnode_for_each_child_node(child, led_node) { + ret = fwnode_property_read_u32(led_node, "color", + &color_id); + if (ret) { + fwnode_handle_put(led_node); + dev_err(lp->dev, "Cannot read color\n"); + return ret; + } + ret = fwnode_property_read_u32(led_node, "reg", + &channel); + if (ret < 0) { + dev_err(lp->dev, "reg property is invalid\n"); + return ret; + } + + mc_led_info[num_channels].color_index = color_id; + mc_led_info[num_channels].channel = channel; + lp5860_led_init_default_state(led, num_channels); + + num_channels++; + } + + led->mc_cdev.num_colors = num_channels; + led->mc_cdev.subled_info = mc_led_info; + led->default_state = led_init_default_state_get(init_data.fwnode); + led_cdev = &led->mc_cdev.led_cdev; + led_cdev->max_brightness = LP5860_MAX_BRIGHTNESS; + led_cdev->brightness_set_blocking = lp5860_set_mc_brightness; + + ret = devm_led_classdev_multicolor_register_ext(lp->dev, + &led->mc_cdev, + &init_data); + if (ret) { + dev_err(lp->dev, "led register err: %d\n", ret); + return ret; + } + i++; + } + + return 0; +} + +int lp5860_device_init(struct lp5860 *lp) +{ + int ret; + + dev_set_drvdata(lp->dev, lp); + + ret = lp5860_enable_disable(lp, 1); + if (ret) + return ret; + + ret = regmap_update_bits(lp->regmap, LP5860_DEV_INITIAL, + LP5860_MODE_MASK, + LP5860_MODE_1 << LP5860_MODE_OFFSET); + if (ret < 0) + return ret; + + ret = sysfs_create_group(&lp->dev->kobj, &lp5860_group); + if (ret) + return ret; + + return lp5860_probe_dt(lp); +} + +void lp5860_device_remove(struct device *dev) +{ + struct lp5860 *lp = dev_get_drvdata(dev); + int err; + + err = lp5860_enable_disable(lp, 0); + if (err) + dev_err(lp->dev, "Failed to disable chip\n"); + + sysfs_remove_group(&dev->kobj, &lp5860_group); +} diff --git a/drivers/leds/leds-lp5860-spi.c b/drivers/leds/leds-lp5860-spi.c new file mode 100644 index 0000000000000000000000000000000000000000..751cc4184037c3c0e14d3493d0a43f0885786523 --- /dev/null +++ b/drivers/leds/leds-lp5860-spi.c @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2025 Pengutronix, + * Steffen Trumtrar <kernel@pengutronix.de + * + * Based on: + * + * Copyright 2012 Texas Instruments + * + * Author: Texas Instruments, zlzx + * + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/spi/spi.h> + +#include "leds-lp5860.h" + +#define LP5860_SPI_WRITE_FLAG BIT(5) + +static const struct regmap_config lp5860_regmap_config = { + .name = "lp5860-spi", + .reg_bits = 10, + .pad_bits = 6, + .val_bits = 8, + .write_flag_mask = (__force unsigned long)cpu_to_be16(LP5860_SPI_WRITE_FLAG), + .reg_format_endian = REGMAP_ENDIAN_BIG, + .max_register = LP5860_MAX_REG, +}; + +static int lp5860_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct lp5860 *lp5860; + unsigned int multi_leds; + int ret; + + multi_leds = device_get_child_node_count(dev); + if (!multi_leds) { + dev_err(dev, "LEDs are not defined in device tree!"); + return -ENODEV; + } + + lp5860 = devm_kzalloc(dev, struct_size(lp5860, leds, multi_leds), + GFP_KERNEL); + if (!lp5860) + return -ENOMEM; + + spi_set_drvdata(spi, lp5860); + + spi->mode = SPI_MODE_0; + spi->bits_per_word = 8; + + ret = spi_setup(spi); + if (ret) + return ret; + + lp5860->regmap = devm_regmap_init_spi(spi, &lp5860_regmap_config); + if (IS_ERR(lp5860->regmap)) { + dev_err(&spi->dev, "regmap init failed.\n"); + return PTR_ERR(lp5860->regmap); + } + + lp5860->dev = dev; + + return lp5860_device_init(lp5860); +} + +static void lp5860_remove(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + + lp5860_device_remove(dev); +} + +static const struct of_device_id lp5860_of_match[] = { + { .compatible = "ti,lp5860" }, + {} +}; +MODULE_DEVICE_TABLE(of, lp5860_of_match); + +static struct spi_driver lp5860_driver = { + .driver = { + .name = "lp5860", + .of_match_table = lp5860_of_match, + }, + .probe = lp5860_probe, + .remove = lp5860_remove, +}; + +module_spi_driver(lp5860_driver); + +MODULE_DESCRIPTION("TI leds lp5860"); +MODULE_AUTHOR("Steffen Trumtrar <kernel@pengutronix.de>"); +MODULE_LICENSE("GPL"); diff --git a/drivers/leds/leds-lp5860.h b/drivers/leds/leds-lp5860.h new file mode 100644 index 0000000000000000000000000000000000000000..b4255fb48372814c7fda86ada96f504c2036f534 --- /dev/null +++ b/drivers/leds/leds-lp5860.h @@ -0,0 +1,315 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2025 Pengutronix, + * Steffen Trumtrar <kernel@pengutronix.de + * + * Based on: + * + * LP5860 Driver Header + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Texas Instruments + * + * Derived from leds-lp5860.c + */ + +#ifndef _LEDS_LP5860_COMMON_H +#define _LEDS_LP5860_COMMON_H + +#include <linux/regmap.h> +#include <linux/led-class-multicolor.h> + +/* lp586x Register */ +#define LP5860_CHIP_EN 0x00 +#define LP5860_DEV_INITIAL 0x01 +#define LP5860_DEV_CONFIG1 0x02 +#define LP5860_DEV_CONFIG2 0x03 +#define LP5860_DEV_CONFIG3 0x04 +#define LP5860_GLOBAL_BRI 0x05 +#define LP5860_GROUP0_BRI 0x06 +#define LP5860_GROUP1_BRI 0x07 +#define LP5860_GROUP2_BRI 0x08 +#define LP5860_R_CURRENT_SET 0x09 +#define LP5860_G_CURRENT_SET 0x0A +#define LP5860_B_CURRENT_SET 0x0B +#define LP5860_GRP_SEL_START 0x0C +#define LP5860_DOT_ONOFF_START 0x43 +#define LP5860_DOT_ONOFF_MAX 0x63 +#define LP5860_FAULT_STATE 0x64 +#define LP5860_DOT_LOD_START 0x65 +#define LP5860_DOT_LSD_START 0x86 +#define LP5860_LOD_CLEAR 0xA7 +#define LP5860_LSD_CLEAR 0xA8 +#define LP5860_RESET 0xA9 +#define LP5860_DC_START 0x0100 +#define LP5860_PWM_BRI_START 0x0200 +#define LP5860_MAX_REG 0x038B + +/* Register chip_enable Value */ +#define LP5860_CHIP_OFFSET 0 +#define LP5860_CHIP_MASK BIT(0) +#define LP5860_CHIP_DISABLE 0x00 +#define LP5860_CHIP_ENABLE 0x01 + +/* Register Dev_initial Value */ +#define LP5860_MAX_LINE_OFFSET 3 +#define LP5860_MAX_LINE_MASK GENMASK(6, 3) +#define LP5860_MAX_LINE_11 0x0B +#define LP5860_MAX_LINE_10 0x0A +#define LP5860_MAX_LINE_9 0x09 +#define LP5860_MAX_LINE_8 0x08 +#define LP5860_MAX_LINE_7 0x07 +#define LP5860_MAX_LINE_6 0x06 +#define LP5860_MAX_LINE_5 0x05 +#define LP5860_MAX_LINE_4 0x04 +#define LP5860_MAX_LINE_3 0x03 +#define LP5860_MAX_LINE_2 0x02 +#define LP5860_MAX_LINE_1 0x01 + +#define LP5860_MODE_OFFSET 1 +#define LP5860_MODE_MASK GENMASK(2, 1) +#define LP5860_MODE_3_1 0x03 +#define LP5860_MODE_3 0x02 +#define LP5860_MODE_2 0x01 +#define LP5860_MODE_1 0x00 + +#define LP5860_PWM_FREQUENCY_OFFSET 0 +#define LP5860_PWM_FREQUENCY_MASK BIT(0) +#define LP5860_PWM_FREQUENCY_62_5K 0x01 +#define LP5860_PWM_FREQUENCY_125K 0x00 + +/* *Register Dev_config1 Valu */ +#define LP5860_SW_BLK_OFFSET 3 +#define LP5860_SW_BLK_MASK BIT(3) +#define LP5860_SW_BLK_05US 0x01 +#define LP5860_SW_BLK_1US 0x00 + +#define LP5860_PWM_SCALE_MODE_OFFSET 2 +#define LP5860_PWM_SCALE_MODE_MASK BIT(2) +#define LP5860_PWM_SCALE_EXPONENTIAL 0x01 +#define LP5860_PWM_SCALE_LINEAR 0x00 + +#define LP5860_PWM_PHASESHIFT_OFFSET 1 +#define LP5860_PWM_PHASESHIFT_MASK BIT(1) +#define LP5860_PWM_PHASESHIFT_ON 0x01 +#define LP5860_PWM_PHASESHIFT_OFF 0x00 + +#define LP5860_CS_ON_SHIFT_OFFSET 0 +#define LP5860_CS_ON_SHIFT_MASK BIT(0) +#define LP5860_CS_DELAY_ON 0x01 +#define LP5860_CS_DELAY_OFF 0x00 + +/* Register Dev_config2 Value */ +#define LP5860_COMP_GROUP3_OFFSET 6 +#define LP5860_COMP_GROUP3_MASK GENMASK(7, 6) +#define LP5860_COMP_GROUP3_3CLOCK 0x03 +#define LP5860_COMP_GROUP3_2CLOCK 0x02 +#define LP5860_COMP_GROUP3_1CLOCK 0x01 +#define LP5860_COMP_GROUP3_OFF 0x00 + +#define LP5860_COMP_GROUP2_OFFSET 4 +#define LP5860_COMP_GROUP2_MASK GENMASK(5, 4) +#define LP5860_COMP_GROUP2_3CLOCK 0x03 +#define LP5860_COMP_GROUP2_2CLOCK 0x02 +#define LP5860_COMP_GROUP2_1CLOCK 0x01 +#define LP5860_COMP_GROUP2_OFF 0x00 + +#define LP5860_COMP_GROUP1_OFFSET 2 +#define LP5860_COMP_GROUP1_MASK GENMASK(3, 2) +#define LP5860_COMP_GROUP1_3CLOCK 0x03 +#define LP5860_COMP_GROUP1_2CLOCK 0x02 +#define LP5860_COMP_GROUP1_1CLOCK 0x01 +#define LP5860_COMP_GROUP1_OFF 0x00 + +#define LP5860_LOD_REMOVAL_OFFSET 1 +#define LP5860_LOD_REMOVAL_MASK BIT(1) +#define LP5860_LOD_REMOVAL_EN 0x01 +#define LP5860_LOD_REMOVAL_OFF 0x00 + +#define LP5860_LSD_REMOVAL_OFFSET 0 +#define LP5860_LSD_REMOVAL_MASK BIT(0) +#define LP5860_LSD_REMOVAL_EN 0x01 +#define LP5860_LSD_REMOVAL_OFF 0x00 + +/* Register Dev_config3 Value */ +#define LP5860_DOWN_DEGHOST_OFFSET 6 +#define LP5860_DOWN_DEGHOST_MASK GENMASK(7, 6) +#define LP5860_DOWN_DEGHOST_STRONG 0x03 +#define LP5860_DOWN_DEGHOST_MEDIUM 0x02 +#define LP5860_DOWN_DEGHOST_WEAK 0x01 +#define LP5860_DOWN_DEGHOST_OFF 0x00 + +#define LP5860_UP_DEGHOST_OFFSET 4 +#define LP5860_UP_DEGHOST_MASK GENMASK(5, 4) +#define LP5860_UP_DEGHOST_GND 0x03 +#define LP5860_UP_DEGHOST_3 0x02 +#define LP5860_UP_DEGHOST_2_5 0x01 +#define LP5860_UP_DEGHOST_2 0x00 + +#define LP5860_MAXIMUM_CURRENT_OFFSET 1 +#define LP5860_MAXIMUM_CURRENT_MASK GENMASK(3, 1) +#define LP5860_MAXIMUM_CURRENT_50 0x07 +#define LP5860_MAXIMUM_CURRENT_40 0x06 +#define LP5860_MAXIMUM_CURRENT_30 0x05 +#define LP5860_MAXIMUM_CURRENT_20 0x04 +#define LP5860_MAXIMUM_CURRENT_15 0x03 +#define LP5860_MAXIMUM_CURRENT_10 0x02 +#define LP5860_MAXIMUM_CURRENT_5 0x01 +#define LP5860_MAXIMUM_CURRENT_3 0x00 + +#define LP5860_UP_DEGHOST_ENABLE_OFFSET 0 +#define LP5860_UP_DEGHOST_ENABLE_MASK BIT(0) +#define LP5860_UP_DEGHOST_ENABLE_EN 0x01 +#define LP5860_UP_DEGHOST_ENABLE_OFF 0x00 + +/* Register PWM */ +#define LP5860_PWM_GLOBAL_MAX 0xff +#define LP5860_PWM_GROUP_MAX 0xff + +/* Register CC GROUP select */ +#define LP5860_CC_GROUP_MASK GENMASK(7, 0) +#define LP5860_CC_GROUP_MAX 0x7F + +/* Register dot group select */ +#define LP5860_DOT_0_OFFSET 0 +#define LP5860_DOT_1_OFFSET 2 +#define LP5860_DOT_2_OFFSET 4 +#define LP5860_DOT_3_OFFSET 6 + +#define LP5860_DOT_GROUP3 0x03 +#define LP5860_DOT_GROUP2 0x02 +#define LP5860_DOT_GROUP1 0x01 +#define LP5860_DOT_GROUP_NONE 0x00 + +#define LP5860_PWM_DOT_MAX 0xff +/* dot onoff Value */ +#define LP5860_DOT_CS0_OFFSET 0 +#define LP5860_DOT_CS1_OFFSET 1 +#define LP5860_DOT_CS2_OFFSET 2 +#define LP5860_DOT_CS3_OFFSET 3 +#define LP5860_DOT_CS4_OFFSET 4 +#define LP5860_DOT_CS5_OFFSET 5 +#define LP5860_DOT_CS6_OFFSET 6 +#define LP5860_DOT_CS7_OFFSET 7 + +#define LP5860_DOT_CS_ON 0x01 +#define LP5860_DOT_CS_OFF 0x00 + +/* dot lod Value */ +#define LP5860_DOT_LOD0_OFFSET 0 +#define LP5860_DOT_LOD1_OFFSET 1 +#define LP5860_DOT_LOD2_OFFSET 2 +#define LP5860_DOT_LOD3_OFFSET 3 +#define LP5860_DOT_LOD4_OFFSET 4 +#define LP5860_DOT_LOD5_OFFSET 5 +#define LP5860_DOT_LOD6_OFFSET 6 +#define LP5860_DOT_LOD7_OFFSET 7 + +#define LP5860_DOT_LOD_ON 0x01 +#define LP5860_DOT_LOD_OFF 0x00 + +/* dot lsd Value */ +#define LP5860_DOT_LSD0_OFFSET 0 +#define LP5860_DOT_LSD1_OFFSET 1 +#define LP5860_DOT_LSD2_OFFSET 2 +#define LP5860_DOT_LSD3_OFFSET 3 +#define LP5860_DOT_LSD4_OFFSET 4 +#define LP5860_DOT_LSD5_OFFSET 5 +#define LP5860_DOT_LSD6_OFFSET 6 +#define LP5860_DOT_LSD7_OFFSET 7 + +#define LP5860_DOT_LSD_ON 0x01 +#define LP5860_DOT_LSD_OFF 0x00 + +/* REG FAULT_STATE */ +#define LP5860_GLOBAL_LOD_OFFSET 1 +#define LP5860_GLOBAL_LOD_STATE BIT(1) +#define LP5860_GLOBAL_LSD_OFFSET 0 +#define LP5860_GLOBAL_LSD_STATE BIT(0) + +#define LP5860_FAULT_STATE_ON 0x01 +#define LP5860_FAULT_STATE_OFF 0x00 + +/* REG FAULT_STATE */ +#define LP5860_GLOBAL_LOD_CELAR 0x00 +#define LP5860_GLOBAL_LSD_CELAR 0x00 + + +#define LP5860_LOD_CLEAR_EN 0xff +#define LP5860_LSD_CLEAR_EN 0xff +#define LP5860_RESET_EN 0xff + +#define LP5860_MAX_BRIGHTNESS 255 +#define LP5860_REG_R_PWM 0x0 +#define LP5860_REG_G_PWM 0x1 +#define LP5860_REG_B_PWM 0x2 + +#define LP5860_MAX_LED_CONSTANT 18 +#define LP5860_MAX_LED_SCAN 11 +#define LP5860_MAX_LED (LP5860_MAX_LED_CONSTANT * LP5860_MAX_LED_SCAN) + +#define LP5860_MAX_DOT_ONOFF_GROUP_NUM 8 + +/* + * Theoretically, there is no max channel per LED, + * limit this to a reasonable value for RGBW LEDs + */ +#define LP5860_MAX_LED_CHANNELS 4 + +#define LP5860_DEV_ATTR_RW(name) \ + DEVICE_ATTR(name, 0644, lp5860_##name##_show, lp5860_##name##_store) + +#define LP5860_SHOW_MODE(nr, reg, mask, offset) \ + static ssize_t lp5860_##nr##_show(struct device *dev, \ + struct device_attribute *attr,\ + char *buf) \ + { \ + struct lp5860 *led = dev_get_drvdata(dev); \ + unsigned int value = 0; \ + int ret; \ + \ + ret = regmap_read(led->regmap, reg, &value); \ + if (ret) \ + return ret; \ + return sysfs_emit(buf, "%d\n", (value & (int)mask) >> (int)offset);\ + } + +#define LP5860_STORE_MODE(nr, reg, mask, offset) \ + static ssize_t lp5860_##nr##_store(struct device *dev, \ + struct device_attribute *attr,\ + const char *buf, size_t len) \ + { \ + struct lp5860 *led = dev_get_drvdata(dev); \ + unsigned int value = 0; \ + int ret; \ + \ + if (kstrtoint(buf, 0, &value)) \ + return -EINVAL; \ + ret = regmap_update_bits(led->regmap, reg, (int)mask, \ + value << (int)offset); \ + if (ret < 0) \ + return ret; \ + return len; \ + } + +struct lp5860_led { + struct lp5860 *priv; + struct led_classdev_mc mc_cdev; + u8 chan_nr; + u8 led_current; + u8 brightness; + enum led_default_state default_state; +}; + +struct lp5860 { + struct device *dev; + struct regmap *regmap; + + DECLARE_FLEX_ARRAY(struct lp5860_led, leds); +}; + +int lp5860_device_init(struct lp5860 *lp5860); +void lp5860_device_remove(struct device *dev); + +#endif /* _LEDS_LP5860_H */ -- 2.47.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip 2025-05-14 10:36 ` [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip Steffen Trumtrar @ 2025-05-23 10:28 ` Lee Jones 2025-07-14 8:26 ` Steffen Trumtrar 0 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2025-05-23 10:28 UTC (permalink / raw) To: Steffen Trumtrar Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek, linux-leds, devicetree, linux-kernel On Wed, 14 May 2025, Steffen Trumtrar wrote: > Add support for the Texas Instruments LP5860 LED driver chip > via SPI interfaces. > > The LP5860 is an LED matrix driver for up to 196 LEDs, which supports > short and open detection of the individual channel select lines. > > The original driver is from an unknown author at Texas Instruments. Only > the cryptic handle 'zlzx' is known. > > Co-developed-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > --- > Documentation/ABI/testing/sysfs-class-spi-lp5860 | 23 ++ > drivers/leds/Kconfig | 23 ++ > drivers/leds/Makefile | 2 + > drivers/leds/leds-lp5860-core.c | 276 ++++++++++++++++++++ > drivers/leds/leds-lp5860-spi.c | 99 +++++++ Are you going to follow-up with another option? Say I2C? > drivers/leds/leds-lp5860.h | 315 +++++++++++++++++++++++ > 6 files changed, 738 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-class-spi-lp5860 b/Documentation/ABI/testing/sysfs-class-spi-lp5860 This doesn't belong here. > new file mode 100644 > index 0000000000000000000000000000000000000000..d24b49d38ecae55f1a1a4e465fbe71d30eff497e > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-spi-lp5860 > @@ -0,0 +1,23 @@ > +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/b_current_set Why would you want to change the current of the SPI bus? > +Date: May 2025 > +KernelVersion: 6.15 > +Contact: Steffen Trumtrar <kernel@pengutronix.de> > +Description: > + Contains and sets the current for the B color group. What does the current set? Brightness? If so, the user shouldn't be expected to know what current set in order to obtain a specific brightness. Instead, shouldn't you use /sys/class/leds/<led>/multi_intensity and let the driver deal with the particulars of setting that brightness? > + Can be adjusted in 128 steps from 0% to 100% of the maximum output current. > + > +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/g_current_set > +Date: May 2025 > +KernelVersion: 6.15 > +Contact: Steffen Trumtrar <kernel@pengutronix.de> > +Description: > + Contains and sets the current for the G color group. > + Can be adjusted in 128 steps from 0% to 100% of the maximum output current. > + > +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/r_current_set > +Date: May 2025 > +KernelVersion: 6.15 > +Contact: Steffen Trumtrar <kernel@pengutronix.de> > +Description: > + Contains and sets the current for the R color group. > + Can be adjusted in 128 steps from 0% to 100% of the maximum output current. > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a104cbb0a0019a0510f7e33b4e70e13083efb390..d2b6a53f2ea2cef751cd19774e05f966e66a0a5f 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -483,6 +483,29 @@ config LEDS_LP5569 > Driver provides direct control via LED class and interface for > programming the engines. > > +config LEDS_LP5860_CORE > + tristate "Core Driver for TI LP5860" > + depends on LEDS_CLASS > + depends on LEDS_CLASS_MULTICOLOR > + depends on OF > + select FW_LOADER > + select FW_LOADER_USER_HELPER > + select REGMAP > + help > + This option supports common operations for LP5860 devices. > + The device can be used with SPI or I2C bus. This is not sufficient. What is the device? What are its capabilities? What does the LED array look like? More input ... > +config LEDS_LP5860_SPI > + tristate "LED Support for TI LP5860 SPI LED driver chip" > + depends on SPI > + depends on LEDS_LP5860_CORE > + help > + If you say yes here you get support for the Texas Instruments > + LP5860 LED driver for SPI bus connections. > + > + To compile this driver as a module, choose M here: the > + module will be called lp5860_spi. > + > config LEDS_LP8501 > tristate "LED Support for TI LP8501 LED driver chip" > depends on LEDS_CLASS && I2C > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 2f170d69dcbf517cf7df98f25c49e3b445da3470..714d37a8361b757b71d37e95211ea04aed5d5d59 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -54,6 +54,8 @@ obj-$(CONFIG_LEDS_LP5523) += leds-lp5523.o > obj-$(CONFIG_LEDS_LP5562) += leds-lp5562.o > obj-$(CONFIG_LEDS_LP5569) += leds-lp5569.o > obj-$(CONFIG_LEDS_LP55XX_COMMON) += leds-lp55xx-common.o > +obj-$(CONFIG_LEDS_LP5860_CORE) += leds-lp5860-core.o > +obj-$(CONFIG_LEDS_LP5860_SPI) += leds-lp5860-spi.o > obj-$(CONFIG_LEDS_LP8501) += leds-lp8501.o > obj-$(CONFIG_LEDS_LP8788) += leds-lp8788.o > obj-$(CONFIG_LEDS_LP8860) += leds-lp8860.o > diff --git a/drivers/leds/leds-lp5860-core.c b/drivers/leds/leds-lp5860-core.c > new file mode 100644 > index 0000000000000000000000000000000000000000..c3ce2528783a1910a267c1b35a65ad27a63083db > --- /dev/null > +++ b/drivers/leds/leds-lp5860-core.c > @@ -0,0 +1,276 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2025 Pengutronix, Author: ... > + * Steffen Trumtrar <kernel@pengutronix.de > + * > + * Based on: > + * > + * LP5860 Driver > + * > + * Copyright 2012 Texas Instruments > + * > + * Author: Texas Instruments, zlzx Remove this. Most drivers take inspiration from others. > + */ > + > +#include <linux/gpio.h> > +#include <linux/led-class-multicolor.h> > +#include <linux/module.h> > +#include <linux/of_gpio.h> > +#include <linux/of_platform.h> > +#include <linux/regmap.h> > + > +#include "leds-lp5860.h" How many header files do you see in drivers/leds? > +static struct lp5860_led *mcled_cdev_to_led(struct led_classdev_mc *mc_cdev) > +{ > + return container_of(mc_cdev, struct lp5860_led, mc_cdev); > +} > + > +LP5860_SHOW_MODE(r_current_set, LP5860_R_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) > +LP5860_STORE_MODE(r_current_set, LP5860_R_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) > +static LP5860_DEV_ATTR_RW(r_current_set); This is opaque. The macro should be readable here. > +LP5860_SHOW_MODE(g_current_set, LP5860_G_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) > +LP5860_STORE_MODE(g_current_set, LP5860_G_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) > +static LP5860_DEV_ATTR_RW(g_current_set); > + > +LP5860_SHOW_MODE(b_current_set, LP5860_B_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) > +LP5860_STORE_MODE(b_current_set, LP5860_B_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) > +static LP5860_DEV_ATTR_RW(b_current_set); > + > +static struct attribute *lp5860_attributes[] = { > + &dev_attr_r_current_set.attr, > + &dev_attr_g_current_set.attr, > + &dev_attr_b_current_set.attr, > + NULL, > +}; > + > +static const struct attribute_group lp5860_group = { > + .attrs = lp5860_attributes, > +}; > + > +static int lp5860_dotonoff(struct lp5860_led *led, unsigned int dot, What does the datasheet call this? How many 'dots' are there? > + bool enable) Unwrap this. You can use 100-chars to prevent silly line feeds. > +{ > + unsigned int offset = dot / LP5860_MAX_DOT_ONOFF_GROUP_NUM; > + unsigned int mask = BIT(dot % LP5860_MAX_DOT_ONOFF_GROUP_NUM); > + > + return regmap_update_bits(led->priv->regmap, > + LP5860_DOT_ONOFF_START + offset, mask, > + enable ? 0xff : 0x0); Either 0 or 0x00. The former looks more sensible. > +} > + > +static int lp5860_set_mc_brightness(struct led_classdev *cdev, > + enum led_brightness brightness) > +{ > + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); > + struct lp5860_led *led = mcled_cdev_to_led(mc_cdev); > + int i; > + > + led_mc_calc_color_components(mc_cdev, brightness); > + > + for (i = 0; i < led->mc_cdev.num_colors; i++) { > + unsigned int channel = mc_cdev->subled_info[i].channel; > + unsigned int led_brightness = mc_cdev->subled_info[i].brightness; > + int ret; > + > + ret = lp5860_dotonoff(led, channel, led_brightness); > + if (ret) > + return ret; > + > + ret = regmap_write(led->priv->regmap, > + LP5860_PWM_BRI_START + channel, > + led_brightness); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static int lp5860_enable_disable(struct lp5860 *led, int enable_disable) _enable_toggle()? > +{ > + return regmap_write(led->regmap, LP5860_CHIP_EN, enable_disable); 'enable' > +} > + > +static int lp5860_led_init_default_state(struct lp5860_led *led, > + unsigned int channel) > +{ > + unsigned int level; Voltage level? Brightness level? Sea level? > + int ret; > + > + ret = regmap_read(led->priv->regmap, LP5860_PWM_BRI_START + channel, > + &level); > + > + switch (led->default_state) { > + case LEDS_DEFSTATE_ON: > + led->brightness = LP5860_MAX_BRIGHTNESS; > + break; > + case LEDS_DEFSTATE_KEEP: > + led->brightness = min(level, LP5860_MAX_BRIGHTNESS); > + break; > + default: > + led->brightness = 0; > + break; > + } > + > + return lp5860_set_mc_brightness(&led->mc_cdev.led_cdev, led->brightness); > +} > + > +static int lp5860_probe_leds(struct fwnode_handle *child, struct lp5860 *lp, > + struct lp5860_led *led, int num_leds) > +{ > + u32 led_banks[LP5860_MAX_LED] = {0}; > + int led_number; > + int ret; > + > + if (num_leds > 1) { > + if (num_leds > LP5860_MAX_LED) { > + dev_err(lp->dev, "too many leds specified\n"); LEDs > + return -EINVAL; > + } > + > + ret = fwnode_property_read_u32_array(child, "reg", led_banks, Why don't you do anything with the value? > + num_leds); Remove the line break. > + if (ret) { > + dev_err(lp->dev, "reg property is missing\n"); > + return ret; > + } > + } else { > + ret = fwnode_property_read_u32(child, "reg", &led_number); > + if (ret) { > + dev_err(lp->dev, "led reg property missing\n"); "reg" or 'reg'. > + return ret; > + } > + > + if (led_number > LP5860_MAX_LED) { > + dev_err(lp->dev, "led-sources property is invalid\n"); > + return -EINVAL; > + } > + > + led->chan_nr = led_number; What's the difference between a led_number and a chan_nr? Keep nomenclature consistent (nr vs number). Why not call it 'chan_nr' (or more commonly 'chan') from the start? > + } > + > + return 0; > +} > + > +static int lp5860_probe_dt(struct lp5860 *lp) > +{ > + struct fwnode_handle *led_node = NULL; > + struct fwnode_handle *subchild; > + struct led_init_data init_data = {}; > + struct led_classdev *led_cdev; > + struct mc_subled *mc_led_info; > + struct lp5860_led *led; > + int num_channels; > + u32 color_id; > + u32 channel; > + int i = 0; > + int ret; A bunch of these could be define in the scope below. > + device_for_each_child_node_scoped(lp->dev, child) { > + Remove this line. > + led = &lp->leds[i]; > + ret = fwnode_property_count_u32(child, "reg"); > + if (ret < 0) { > + dev_err(lp->dev, "reg property is invalid\n"); > + return ret; > + } > + > + ret = lp5860_probe_leds(child, lp, led, ret); Definitely drop the _probe() terminology at this point. Describe the function properly. What is it doing? > + if (ret) > + return ret; > + > + init_data.fwnode = child; > + > + num_channels = 0; It's num_channels now? Consistency please. > + fwnode_for_each_available_child_node(child, subchild) > + num_channels++; Why don't we already have this from lp5860_probe_leds()? I'm getting confused between channels and LEDs at this point! > + if (!num_channels || num_channels > LP5860_MAX_LED_CHANNELS) > + return -EINVAL; > + > + mc_led_info = devm_kcalloc(lp->dev, num_channels, > + sizeof(*mc_led_info), GFP_KERNEL); > + if (!mc_led_info) > + return -ENOMEM; > + > + num_channels = 0; Wait, what? We're going to count LEDs/channels for a 3rd time now? > + led->priv = lp; > + fwnode_for_each_child_node(child, led_node) { > + ret = fwnode_property_read_u32(led_node, "color", > + &color_id); > + if (ret) { > + fwnode_handle_put(led_node); > + dev_err(lp->dev, "Cannot read color\n"); Highlight the property names 'color'. > + return ret; > + } Don't squish these unrelated calls together: '\n' here. > + ret = fwnode_property_read_u32(led_node, "reg", > + &channel); > + if (ret < 0) { > + dev_err(lp->dev, "reg property is invalid\n"); > + return ret; > + } > + > + mc_led_info[num_channels].color_index = color_id; > + mc_led_info[num_channels].channel = channel; > + lp5860_led_init_default_state(led, num_channels); > + > + num_channels++; > + } > + > + led->mc_cdev.num_colors = num_channels; > + led->mc_cdev.subled_info = mc_led_info; > + led->default_state = led_init_default_state_get(init_data.fwnode); > + led_cdev = &led->mc_cdev.led_cdev; > + led_cdev->max_brightness = LP5860_MAX_BRIGHTNESS; > + led_cdev->brightness_set_blocking = lp5860_set_mc_brightness; > + > + ret = devm_led_classdev_multicolor_register_ext(lp->dev, > + &led->mc_cdev, > + &init_data); > + if (ret) { > + dev_err(lp->dev, "led register err: %d\n", ret); > + return ret; > + } > + i++; > + } > + > + return 0; > +} > + > +int lp5860_device_init(struct lp5860 *lp) > +{ > + int ret; > + > + dev_set_drvdata(lp->dev, lp); You already did this in the SPI part. > + ret = lp5860_enable_disable(lp, 1); 1 is meaningless. Shouldn't this be LP5860_CHIP_ENABLE? > + if (ret) > + return ret; > + > + ret = regmap_update_bits(lp->regmap, LP5860_DEV_INITIAL, > + LP5860_MODE_MASK, > + LP5860_MODE_1 << LP5860_MODE_OFFSET); > + if (ret < 0) > + return ret; > + > + ret = sysfs_create_group(&lp->dev->kobj, &lp5860_group); > + if (ret) > + return ret; > + > + return lp5860_probe_dt(lp); Don't go back to 'probe' nomenclature. Or, call the SPI part, lp5860_spi_probe(), then call this function lp5860_probe(), then you can keep lp5860_probe_dt(). > +} > + > +void lp5860_device_remove(struct device *dev) > +{ > + struct lp5860 *lp = dev_get_drvdata(dev); > + int err; > + > + err = lp5860_enable_disable(lp, 0); Use the defines you created. > + if (err) > + dev_err(lp->dev, "Failed to disable chip\n"); > + > + sysfs_remove_group(&dev->kobj, &lp5860_group); > +} > diff --git a/drivers/leds/leds-lp5860-spi.c b/drivers/leds/leds-lp5860-spi.c > new file mode 100644 > index 0000000000000000000000000000000000000000..751cc4184037c3c0e14d3493d0a43f0885786523 > --- /dev/null > +++ b/drivers/leds/leds-lp5860-spi.c > @@ -0,0 +1,99 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2025 Pengutronix, > + * Steffen Trumtrar <kernel@pengutronix.de > + * > + * Based on: > + * > + * Copyright 2012 Texas Instruments > + * > + * Author: Texas Instruments, zlzx > + * > + */ > + > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/spi/spi.h> > + > +#include "leds-lp5860.h" > + > +#define LP5860_SPI_WRITE_FLAG BIT(5) > + > +static const struct regmap_config lp5860_regmap_config = { > + .name = "lp5860-spi", > + .reg_bits = 10, > + .pad_bits = 6, > + .val_bits = 8, > + .write_flag_mask = (__force unsigned long)cpu_to_be16(LP5860_SPI_WRITE_FLAG), > + .reg_format_endian = REGMAP_ENDIAN_BIG, > + .max_register = LP5860_MAX_REG, > +}; > + > +static int lp5860_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct lp5860 *lp5860; > + unsigned int multi_leds; > + int ret; > + > + multi_leds = device_get_child_node_count(dev); > + if (!multi_leds) { > + dev_err(dev, "LEDs are not defined in device tree!"); > + return -ENODEV; > + } > + > + lp5860 = devm_kzalloc(dev, struct_size(lp5860, leds, multi_leds), > + GFP_KERNEL); > + if (!lp5860) > + return -ENOMEM; > + > + spi_set_drvdata(spi, lp5860); > + > + spi->mode = SPI_MODE_0; > + spi->bits_per_word = 8; > + > + ret = spi_setup(spi); > + if (ret) > + return ret; > + > + lp5860->regmap = devm_regmap_init_spi(spi, &lp5860_regmap_config); > + if (IS_ERR(lp5860->regmap)) { > + dev_err(&spi->dev, "regmap init failed.\n"); > + return PTR_ERR(lp5860->regmap); > + } > + > + lp5860->dev = dev; > + > + return lp5860_device_init(lp5860); > +} > + > +static void lp5860_remove(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; You only use this one. Just use the full dereference in _remove(). > + lp5860_device_remove(dev); > +} > + > +static const struct of_device_id lp5860_of_match[] = { > + { .compatible = "ti,lp5860" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, lp5860_of_match); > + > +static struct spi_driver lp5860_driver = { > + .driver = { > + .name = "lp5860", > + .of_match_table = lp5860_of_match, > + }, > + .probe = lp5860_probe, > + .remove = lp5860_remove, > +}; > + Remove this line. > +module_spi_driver(lp5860_driver); > + > +MODULE_DESCRIPTION("TI leds lp5860"); More info. "LP5860" > +MODULE_AUTHOR("Steffen Trumtrar <kernel@pengutronix.de>"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/leds/leds-lp5860.h b/drivers/leds/leds-lp5860.h > new file mode 100644 > index 0000000000000000000000000000000000000000..b4255fb48372814c7fda86ada96f504c2036f534 > --- /dev/null > +++ b/drivers/leds/leds-lp5860.h > @@ -0,0 +1,315 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2025 Pengutronix, > + * Steffen Trumtrar <kernel@pengutronix.de > + * > + * Based on: > + * > + * LP5860 Driver Header > + * > + * Copyright (C) 2012 Texas Instruments > + * > + * Author: Texas Instruments > + * > + * Derived from leds-lp5860.c As before. > + */ > + > +#ifndef _LEDS_LP5860_COMMON_H > +#define _LEDS_LP5860_COMMON_H > + > +#include <linux/regmap.h> > +#include <linux/led-class-multicolor.h> > + > +/* lp586x Register */ LP5860_REG_... then omit the comment. > +#define LP5860_CHIP_EN 0x00 > +#define LP5860_DEV_INITIAL 0x01 > +#define LP5860_DEV_CONFIG1 0x02 > +#define LP5860_DEV_CONFIG2 0x03 > +#define LP5860_DEV_CONFIG3 0x04 > +#define LP5860_GLOBAL_BRI 0x05 > +#define LP5860_GROUP0_BRI 0x06 > +#define LP5860_GROUP1_BRI 0x07 > +#define LP5860_GROUP2_BRI 0x08 > +#define LP5860_R_CURRENT_SET 0x09 > +#define LP5860_G_CURRENT_SET 0x0A > +#define LP5860_B_CURRENT_SET 0x0B > +#define LP5860_GRP_SEL_START 0x0C > +#define LP5860_DOT_ONOFF_START 0x43 > +#define LP5860_DOT_ONOFF_MAX 0x63 > +#define LP5860_FAULT_STATE 0x64 > +#define LP5860_DOT_LOD_START 0x65 > +#define LP5860_DOT_LSD_START 0x86 > +#define LP5860_LOD_CLEAR 0xA7 > +#define LP5860_LSD_CLEAR 0xA8 > +#define LP5860_RESET 0xA9 > +#define LP5860_DC_START 0x0100 > +#define LP5860_PWM_BRI_START 0x0200 > +#define LP5860_MAX_REG 0x038B > + > +/* Register chip_enable Value */ > +#define LP5860_CHIP_OFFSET 0 > +#define LP5860_CHIP_MASK BIT(0) > +#define LP5860_CHIP_DISABLE 0x00 > +#define LP5860_CHIP_ENABLE 0x01 > + > +/* Register Dev_initial Value */ > +#define LP5860_MAX_LINE_OFFSET 3 > +#define LP5860_MAX_LINE_MASK GENMASK(6, 3) > +#define LP5860_MAX_LINE_11 0x0B > +#define LP5860_MAX_LINE_10 0x0A > +#define LP5860_MAX_LINE_9 0x09 > +#define LP5860_MAX_LINE_8 0x08 > +#define LP5860_MAX_LINE_7 0x07 > +#define LP5860_MAX_LINE_6 0x06 > +#define LP5860_MAX_LINE_5 0x05 > +#define LP5860_MAX_LINE_4 0x04 > +#define LP5860_MAX_LINE_3 0x03 > +#define LP5860_MAX_LINE_2 0x02 > +#define LP5860_MAX_LINE_1 0x01 > + > +#define LP5860_MODE_OFFSET 1 > +#define LP5860_MODE_MASK GENMASK(2, 1) > +#define LP5860_MODE_3_1 0x03 > +#define LP5860_MODE_3 0x02 > +#define LP5860_MODE_2 0x01 > +#define LP5860_MODE_1 0x00 > + > +#define LP5860_PWM_FREQUENCY_OFFSET 0 > +#define LP5860_PWM_FREQUENCY_MASK BIT(0) > +#define LP5860_PWM_FREQUENCY_62_5K 0x01 > +#define LP5860_PWM_FREQUENCY_125K 0x00 > + > +/* *Register Dev_config1 Valu */ > +#define LP5860_SW_BLK_OFFSET 3 > +#define LP5860_SW_BLK_MASK BIT(3) > +#define LP5860_SW_BLK_05US 0x01 > +#define LP5860_SW_BLK_1US 0x00 > + > +#define LP5860_PWM_SCALE_MODE_OFFSET 2 > +#define LP5860_PWM_SCALE_MODE_MASK BIT(2) > +#define LP5860_PWM_SCALE_EXPONENTIAL 0x01 > +#define LP5860_PWM_SCALE_LINEAR 0x00 > + > +#define LP5860_PWM_PHASESHIFT_OFFSET 1 > +#define LP5860_PWM_PHASESHIFT_MASK BIT(1) > +#define LP5860_PWM_PHASESHIFT_ON 0x01 > +#define LP5860_PWM_PHASESHIFT_OFF 0x00 > + > +#define LP5860_CS_ON_SHIFT_OFFSET 0 > +#define LP5860_CS_ON_SHIFT_MASK BIT(0) > +#define LP5860_CS_DELAY_ON 0x01 > +#define LP5860_CS_DELAY_OFF 0x00 > + > +/* Register Dev_config2 Value */ > +#define LP5860_COMP_GROUP3_OFFSET 6 > +#define LP5860_COMP_GROUP3_MASK GENMASK(7, 6) > +#define LP5860_COMP_GROUP3_3CLOCK 0x03 > +#define LP5860_COMP_GROUP3_2CLOCK 0x02 > +#define LP5860_COMP_GROUP3_1CLOCK 0x01 > +#define LP5860_COMP_GROUP3_OFF 0x00 > + > +#define LP5860_COMP_GROUP2_OFFSET 4 > +#define LP5860_COMP_GROUP2_MASK GENMASK(5, 4) > +#define LP5860_COMP_GROUP2_3CLOCK 0x03 > +#define LP5860_COMP_GROUP2_2CLOCK 0x02 > +#define LP5860_COMP_GROUP2_1CLOCK 0x01 > +#define LP5860_COMP_GROUP2_OFF 0x00 > + > +#define LP5860_COMP_GROUP1_OFFSET 2 > +#define LP5860_COMP_GROUP1_MASK GENMASK(3, 2) > +#define LP5860_COMP_GROUP1_3CLOCK 0x03 > +#define LP5860_COMP_GROUP1_2CLOCK 0x02 > +#define LP5860_COMP_GROUP1_1CLOCK 0x01 > +#define LP5860_COMP_GROUP1_OFF 0x00 > + > +#define LP5860_LOD_REMOVAL_OFFSET 1 > +#define LP5860_LOD_REMOVAL_MASK BIT(1) > +#define LP5860_LOD_REMOVAL_EN 0x01 > +#define LP5860_LOD_REMOVAL_OFF 0x00 > + > +#define LP5860_LSD_REMOVAL_OFFSET 0 > +#define LP5860_LSD_REMOVAL_MASK BIT(0) > +#define LP5860_LSD_REMOVAL_EN 0x01 > +#define LP5860_LSD_REMOVAL_OFF 0x00 > + > +/* Register Dev_config3 Value */ > +#define LP5860_DOWN_DEGHOST_OFFSET 6 > +#define LP5860_DOWN_DEGHOST_MASK GENMASK(7, 6) > +#define LP5860_DOWN_DEGHOST_STRONG 0x03 > +#define LP5860_DOWN_DEGHOST_MEDIUM 0x02 > +#define LP5860_DOWN_DEGHOST_WEAK 0x01 > +#define LP5860_DOWN_DEGHOST_OFF 0x00 > + > +#define LP5860_UP_DEGHOST_OFFSET 4 > +#define LP5860_UP_DEGHOST_MASK GENMASK(5, 4) > +#define LP5860_UP_DEGHOST_GND 0x03 > +#define LP5860_UP_DEGHOST_3 0x02 > +#define LP5860_UP_DEGHOST_2_5 0x01 > +#define LP5860_UP_DEGHOST_2 0x00 > + > +#define LP5860_MAXIMUM_CURRENT_OFFSET 1 > +#define LP5860_MAXIMUM_CURRENT_MASK GENMASK(3, 1) > +#define LP5860_MAXIMUM_CURRENT_50 0x07 > +#define LP5860_MAXIMUM_CURRENT_40 0x06 > +#define LP5860_MAXIMUM_CURRENT_30 0x05 > +#define LP5860_MAXIMUM_CURRENT_20 0x04 > +#define LP5860_MAXIMUM_CURRENT_15 0x03 > +#define LP5860_MAXIMUM_CURRENT_10 0x02 > +#define LP5860_MAXIMUM_CURRENT_5 0x01 > +#define LP5860_MAXIMUM_CURRENT_3 0x00 > + > +#define LP5860_UP_DEGHOST_ENABLE_OFFSET 0 > +#define LP5860_UP_DEGHOST_ENABLE_MASK BIT(0) > +#define LP5860_UP_DEGHOST_ENABLE_EN 0x01 > +#define LP5860_UP_DEGHOST_ENABLE_OFF 0x00 > + > +/* Register PWM */ > +#define LP5860_PWM_GLOBAL_MAX 0xff > +#define LP5860_PWM_GROUP_MAX 0xff > + > +/* Register CC GROUP select */ > +#define LP5860_CC_GROUP_MASK GENMASK(7, 0) > +#define LP5860_CC_GROUP_MAX 0x7F > + > +/* Register dot group select */ > +#define LP5860_DOT_0_OFFSET 0 > +#define LP5860_DOT_1_OFFSET 2 > +#define LP5860_DOT_2_OFFSET 4 > +#define LP5860_DOT_3_OFFSET 6 > + > +#define LP5860_DOT_GROUP3 0x03 > +#define LP5860_DOT_GROUP2 0x02 > +#define LP5860_DOT_GROUP1 0x01 > +#define LP5860_DOT_GROUP_NONE 0x00 > + > +#define LP5860_PWM_DOT_MAX 0xff > +/* dot onoff Value */ > +#define LP5860_DOT_CS0_OFFSET 0 > +#define LP5860_DOT_CS1_OFFSET 1 > +#define LP5860_DOT_CS2_OFFSET 2 > +#define LP5860_DOT_CS3_OFFSET 3 > +#define LP5860_DOT_CS4_OFFSET 4 > +#define LP5860_DOT_CS5_OFFSET 5 > +#define LP5860_DOT_CS6_OFFSET 6 > +#define LP5860_DOT_CS7_OFFSET 7 > + > +#define LP5860_DOT_CS_ON 0x01 > +#define LP5860_DOT_CS_OFF 0x00 > + > +/* dot lod Value */ > +#define LP5860_DOT_LOD0_OFFSET 0 > +#define LP5860_DOT_LOD1_OFFSET 1 > +#define LP5860_DOT_LOD2_OFFSET 2 > +#define LP5860_DOT_LOD3_OFFSET 3 > +#define LP5860_DOT_LOD4_OFFSET 4 > +#define LP5860_DOT_LOD5_OFFSET 5 > +#define LP5860_DOT_LOD6_OFFSET 6 > +#define LP5860_DOT_LOD7_OFFSET 7 > + > +#define LP5860_DOT_LOD_ON 0x01 > +#define LP5860_DOT_LOD_OFF 0x00 > + > +/* dot lsd Value */ > +#define LP5860_DOT_LSD0_OFFSET 0 > +#define LP5860_DOT_LSD1_OFFSET 1 > +#define LP5860_DOT_LSD2_OFFSET 2 > +#define LP5860_DOT_LSD3_OFFSET 3 > +#define LP5860_DOT_LSD4_OFFSET 4 > +#define LP5860_DOT_LSD5_OFFSET 5 > +#define LP5860_DOT_LSD6_OFFSET 6 > +#define LP5860_DOT_LSD7_OFFSET 7 > + > +#define LP5860_DOT_LSD_ON 0x01 > +#define LP5860_DOT_LSD_OFF 0x00 > + > +/* REG FAULT_STATE */ > +#define LP5860_GLOBAL_LOD_OFFSET 1 > +#define LP5860_GLOBAL_LOD_STATE BIT(1) > +#define LP5860_GLOBAL_LSD_OFFSET 0 > +#define LP5860_GLOBAL_LSD_STATE BIT(0) > + > +#define LP5860_FAULT_STATE_ON 0x01 > +#define LP5860_FAULT_STATE_OFF 0x00 > + > +/* REG FAULT_STATE */ > +#define LP5860_GLOBAL_LOD_CELAR 0x00 > +#define LP5860_GLOBAL_LSD_CELAR 0x00 > + > + > +#define LP5860_LOD_CLEAR_EN 0xff > +#define LP5860_LSD_CLEAR_EN 0xff > +#define LP5860_RESET_EN 0xff > + > +#define LP5860_MAX_BRIGHTNESS 255 > +#define LP5860_REG_R_PWM 0x0 > +#define LP5860_REG_G_PWM 0x1 > +#define LP5860_REG_B_PWM 0x2 > + > +#define LP5860_MAX_LED_CONSTANT 18 > +#define LP5860_MAX_LED_SCAN 11 > +#define LP5860_MAX_LED (LP5860_MAX_LED_CONSTANT * LP5860_MAX_LED_SCAN) > + > +#define LP5860_MAX_DOT_ONOFF_GROUP_NUM 8 > + > +/* > + * Theoretically, there is no max channel per LED, > + * limit this to a reasonable value for RGBW LEDs > + */ > +#define LP5860_MAX_LED_CHANNELS 4 > + > +#define LP5860_DEV_ATTR_RW(name) \ > + DEVICE_ATTR(name, 0644, lp5860_##name##_show, lp5860_##name##_store) > + > +#define LP5860_SHOW_MODE(nr, reg, mask, offset) \ > + static ssize_t lp5860_##nr##_show(struct device *dev, \ > + struct device_attribute *attr,\ > + char *buf) \ > + { \ > + struct lp5860 *led = dev_get_drvdata(dev); \ > + unsigned int value = 0; \ > + int ret; \ > + \ > + ret = regmap_read(led->regmap, reg, &value); \ > + if (ret) \ > + return ret; \ > + return sysfs_emit(buf, "%d\n", (value & (int)mask) >> (int)offset);\ > + } > + > +#define LP5860_STORE_MODE(nr, reg, mask, offset) \ > + static ssize_t lp5860_##nr##_store(struct device *dev, \ > + struct device_attribute *attr,\ > + const char *buf, size_t len) \ > + { \ > + struct lp5860 *led = dev_get_drvdata(dev); \ > + unsigned int value = 0; \ > + int ret; \ > + \ > + if (kstrtoint(buf, 0, &value)) \ > + return -EINVAL; \ > + ret = regmap_update_bits(led->regmap, reg, (int)mask, \ > + value << (int)offset); \ > + if (ret < 0) \ > + return ret; \ > + return len; \ > + } > + > +struct lp5860_led { > + struct lp5860 *priv; > + struct led_classdev_mc mc_cdev; > + u8 chan_nr; > + u8 led_current; > + u8 brightness; > + enum led_default_state default_state; > +}; > + > +struct lp5860 { > + struct device *dev; > + struct regmap *regmap; > + > + DECLARE_FLEX_ARRAY(struct lp5860_led, leds); > +}; > + > +int lp5860_device_init(struct lp5860 *lp5860); > +void lp5860_device_remove(struct device *dev); > + > +#endif /* _LEDS_LP5860_H */ > > -- > 2.47.1 > > -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip 2025-05-23 10:28 ` Lee Jones @ 2025-07-14 8:26 ` Steffen Trumtrar 2025-07-18 13:18 ` Lee Jones 0 siblings, 1 reply; 11+ messages in thread From: Steffen Trumtrar @ 2025-07-14 8:26 UTC (permalink / raw) To: Lee Jones Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek, linux-leds, devicetree, linux-kernel Hi, On 2025-05-23 at 11:28 +01, Lee Jones <lee@kernel.org> wrote: > On Wed, 14 May 2025, Steffen Trumtrar wrote: > > > Add support for the Texas Instruments LP5860 LED driver chip > > via SPI interfaces. > > > > The LP5860 is an LED matrix driver for up to 196 LEDs, which supports > > short and open detection of the individual channel select lines. > > > > The original driver is from an unknown author at Texas Instruments. Only > > the cryptic handle 'zlzx' is known. > > > > Co-developed-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > > --- > > Documentation/ABI/testing/sysfs-class-spi-lp5860 | 23 ++ > > drivers/leds/Kconfig | 23 ++ > > drivers/leds/Makefile | 2 + > > drivers/leds/leds-lp5860-core.c | 276 ++++++++++++++++++++ > > drivers/leds/leds-lp5860-spi.c | 99 +++++++ > > Are you going to follow-up with another option? Say I2C? the chip also supports connection via I2C, but it's unlikely that I will add that myself. > > drivers/leds/leds-lp5860.h | 315 +++++++++++++++++++++++ > > 6 files changed, 738 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-class-spi-lp5860 b/Documentation/ABI/testing/sysfs-class-spi-lp5860 > > This doesn't belong here. > > > new file mode 100644 > > index 0000000000000000000000000000000000000000..d24b49d38ecae55f1a1a4e465fbe71d30eff497e > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-class-spi-lp5860 > > @@ -0,0 +1,23 @@ > > +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/b_current_set > > Why would you want to change the current of the SPI bus? > Where does it belong? I grepped and followed the eeprom (Documentation/ABI/testing/sysfs-class-spi-eeprom) which uses sernum in the same way. Directions welcome. > > > +Date: May 2025 > > +KernelVersion: 6.15 > > +Contact: Steffen Trumtrar <kernel@pengutronix.de> > > +Description: > > + Contains and sets the current for the B color group. > > What does the current set? Brightness? > > If so, the user shouldn't be expected to know what current set in order > to obtain a specific brightness. Instead, shouldn't you use > /sys/class/leds/<led>/multi_intensity and let the driver deal with the > particulars of setting that brightness? > The chip has a global setting for the current of the three color groups. And an indiviual setting for every LED itself. The multi_intensity is for one LED as far as I understand. And the brightness of the whole matrix can be controlled via this global current setting. (...) Thanks for the rest of the feedback. I already addressed all of that in my patches, but I'm not sure what is the correct way to proceed with the sysfs ABI entries. Thanks, Steffen -- Pengutronix e.K. | Dipl.-Inform. Steffen Trumtrar | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip 2025-07-14 8:26 ` Steffen Trumtrar @ 2025-07-18 13:18 ` Lee Jones 0 siblings, 0 replies; 11+ messages in thread From: Lee Jones @ 2025-07-18 13:18 UTC (permalink / raw) To: Steffen Trumtrar Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek, linux-leds, devicetree, linux-kernel On Mon, 14 Jul 2025, Steffen Trumtrar wrote: > > Hi, > > On 2025-05-23 at 11:28 +01, Lee Jones <lee@kernel.org> wrote: > > > On Wed, 14 May 2025, Steffen Trumtrar wrote: > > > > > Add support for the Texas Instruments LP5860 LED driver chip > > > via SPI interfaces. > > > > The LP5860 is an LED matrix driver for up to 196 LEDs, which > > supports > > > short and open detection of the individual channel select lines. > > > > The original driver is from an unknown author at Texas Instruments. > > Only > > > the cryptic handle 'zlzx' is known. > > > > Co-developed-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > > > --- > > > Documentation/ABI/testing/sysfs-class-spi-lp5860 | 23 ++ > > > drivers/leds/Kconfig | 23 ++ > > > drivers/leds/Makefile | 2 + > > > drivers/leds/leds-lp5860-core.c | 276 ++++++++++++++++++++ > > > drivers/leds/leds-lp5860-spi.c | 99 +++++++ > > > > Are you going to follow-up with another option? Say I2C? > > the chip also supports connection via I2C, but it's unlikely that I will add that myself. > > > > drivers/leds/leds-lp5860.h | 315 +++++++++++++++++++++++ > > > 6 files changed, 738 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-class-spi-lp5860 > > b/Documentation/ABI/testing/sysfs-class-spi-lp5860 > > > > This doesn't belong here. > > > > > new file mode 100644 > > > index 0000000000000000000000000000000000000000..d24b49d38ecae55f1a1a4e465fbe71d30eff497e > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-class-spi-lp5860 > > > @@ -0,0 +1,23 @@ > > > +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/b_current_set > > > > Why would you want to change the current of the SPI bus? > > > > Where does it belong? I grepped and followed the eeprom > (Documentation/ABI/testing/sysfs-class-spi-eeprom) which uses sernum > in the same way. Directions welcome. I mean, in this patch. Submit this part to the SPI subsystem maintainer. > > > +Date: May 2025 > > > +KernelVersion: 6.15 > > > +Contact: Steffen Trumtrar <kernel@pengutronix.de> > > > +Description: > > > + Contains and sets the current for the B color group. > > > > What does the current set? Brightness? > > > > If so, the user shouldn't be expected to know what current set in order > > to obtain a specific brightness. Instead, shouldn't you use > > /sys/class/leds/<led>/multi_intensity and let the driver deal with the > > particulars of setting that brightness? > > > > The chip has a global setting for the current of the three color > groups. And an indiviual setting for every LED itself. The > multi_intensity is for one LED as far as I understand. And the > brightness of the whole matrix can be controlled via this global > current setting. What I mean is - don't call it current. Use what it does when you change the current. > (...) > > Thanks for the rest of the feedback. I already addressed all of that > in my patches, but I'm not sure what is the correct way to proceed > with the sysfs ABI entries. > > > Thanks, > Steffen > > -- > Pengutronix e.K. | Dipl.-Inform. Steffen Trumtrar | > Steuerwalder Str. 21 | https://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 | > -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] leds: lp5860: save count of multi_leds 2025-05-14 10:36 [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Steffen Trumtrar 2025-05-14 10:36 ` [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller Steffen Trumtrar 2025-05-14 10:36 ` [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip Steffen Trumtrar @ 2025-05-14 10:36 ` Steffen Trumtrar 2025-06-12 13:52 ` Lee Jones 2025-05-14 10:36 ` [PATCH v2 4/4] leds: lp5860: detect and report fault state Steffen Trumtrar 2025-05-23 11:53 ` [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Pavel Machek 4 siblings, 1 reply; 11+ messages in thread From: Steffen Trumtrar @ 2025-05-14 10:36 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek Cc: linux-leds, devicetree, linux-kernel, kernel, Steffen Trumtrar Save the count of multi_leds child nodes for later use. As the leds are added to a flex array, the size needs to be saved at runtime. Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> --- drivers/leds/leds-lp5860-spi.c | 2 ++ drivers/leds/leds-lp5860.h | 1 + 2 files changed, 3 insertions(+) diff --git a/drivers/leds/leds-lp5860-spi.c b/drivers/leds/leds-lp5860-spi.c index 751cc4184037c3c0e14d3493d0a43f0885786523..eee41ee8a8c226db6a68413998642624fabffe7c 100644 --- a/drivers/leds/leds-lp5860-spi.c +++ b/drivers/leds/leds-lp5860-spi.c @@ -50,6 +50,8 @@ static int lp5860_probe(struct spi_device *spi) if (!lp5860) return -ENOMEM; + lp5860->leds_size = multi_leds; + spi_set_drvdata(spi, lp5860); spi->mode = SPI_MODE_0; diff --git a/drivers/leds/leds-lp5860.h b/drivers/leds/leds-lp5860.h index b4255fb48372814c7fda86ada96f504c2036f534..3b8342a832bc75afdf2318fd4ee1ee9ce105cbe3 100644 --- a/drivers/leds/leds-lp5860.h +++ b/drivers/leds/leds-lp5860.h @@ -305,6 +305,7 @@ struct lp5860_led { struct lp5860 { struct device *dev; struct regmap *regmap; + unsigned int leds_size; DECLARE_FLEX_ARRAY(struct lp5860_led, leds); }; -- 2.47.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] leds: lp5860: save count of multi_leds 2025-05-14 10:36 ` [PATCH v2 3/4] leds: lp5860: save count of multi_leds Steffen Trumtrar @ 2025-06-12 13:52 ` Lee Jones 0 siblings, 0 replies; 11+ messages in thread From: Lee Jones @ 2025-06-12 13:52 UTC (permalink / raw) To: Steffen Trumtrar Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek, linux-leds, devicetree, linux-kernel On Wed, 14 May 2025, Steffen Trumtrar wrote: > Save the count of multi_leds child nodes for later use. > As the leds are added to a flex array, the size needs to be saved at > runtime. Size of the LEDs? Length in millimeters? > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > --- > drivers/leds/leds-lp5860-spi.c | 2 ++ > drivers/leds/leds-lp5860.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/drivers/leds/leds-lp5860-spi.c b/drivers/leds/leds-lp5860-spi.c > index 751cc4184037c3c0e14d3493d0a43f0885786523..eee41ee8a8c226db6a68413998642624fabffe7c 100644 > --- a/drivers/leds/leds-lp5860-spi.c > +++ b/drivers/leds/leds-lp5860-spi.c > @@ -50,6 +50,8 @@ static int lp5860_probe(struct spi_device *spi) > if (!lp5860) > return -ENOMEM; > > + lp5860->leds_size = multi_leds; > + > spi_set_drvdata(spi, lp5860); > > spi->mode = SPI_MODE_0; > diff --git a/drivers/leds/leds-lp5860.h b/drivers/leds/leds-lp5860.h > index b4255fb48372814c7fda86ada96f504c2036f534..3b8342a832bc75afdf2318fd4ee1ee9ce105cbe3 100644 > --- a/drivers/leds/leds-lp5860.h > +++ b/drivers/leds/leds-lp5860.h > @@ -305,6 +305,7 @@ struct lp5860_led { > struct lp5860 { > struct device *dev; > struct regmap *regmap; > + unsigned int leds_size; > > DECLARE_FLEX_ARRAY(struct lp5860_led, leds); > }; > > -- > 2.47.1 > > -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] leds: lp5860: detect and report fault state 2025-05-14 10:36 [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Steffen Trumtrar ` (2 preceding siblings ...) 2025-05-14 10:36 ` [PATCH v2 3/4] leds: lp5860: save count of multi_leds Steffen Trumtrar @ 2025-05-14 10:36 ` Steffen Trumtrar 2025-05-23 11:53 ` [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Pavel Machek 4 siblings, 0 replies; 11+ messages in thread From: Steffen Trumtrar @ 2025-05-14 10:36 UTC (permalink / raw) To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, Pavel Machek Cc: linux-leds, devicetree, linux-kernel, kernel, Steffen Trumtrar The lp5860 can detect shorts and open circuits. If an open (LOD) or short (LSD) is detected, the global bit in LP5860_FAULT_STATE is set. The channel that caused this can be read from the corresponding Dot_lsdX and Dot_lodX register and bit offset. The global bits can be cleared by writing 0xf to the LOD/LSD_clear register. Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> --- Documentation/ABI/testing/sysfs-class-spi-lp5860 | 50 +++++++++ drivers/leds/leds-lp5860-core.c | 137 +++++++++++++++++++++++ drivers/leds/leds-lp5860.h | 10 ++ 3 files changed, 197 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-class-spi-lp5860 b/Documentation/ABI/testing/sysfs-class-spi-lp5860 index d24b49d38ecae55f1a1a4e465fbe71d30eff497e..292e5988ef5c6c1b353446f362dbd5b82d185c54 100644 --- a/Documentation/ABI/testing/sysfs-class-spi-lp5860 +++ b/Documentation/ABI/testing/sysfs-class-spi-lp5860 @@ -21,3 +21,53 @@ Contact: Steffen Trumtrar <kernel@pengutronix.de> Description: Contains and sets the current for the R color group. Can be adjusted in 128 steps from 0% to 100% of the maximum output current. + +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state +Date: May 2025 +KernelVersion: 6.15 +Contact: Steffen Trumtrar <kernel@pengutronix.de> +Description: + Contains and sets the global fault state: + + * 3: Open and short detected + * 2: Open detected + * 1: Short detected + + Can be cleared by writing the corresponding value back to fault_state. + + Example usage:: + + ## Read + # cat /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state + 2 + + ## Write + # echo 2 > /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state + +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state_open +Date: May 2025 +KernelVersion: 6.15 +Contact: Steffen Trumtrar <kernel@pengutronix.de> +Description: + Contains all LEDs and channels where an open condition was detected. + The format is ledname:channel. + + Example usage:: + + ## Read + # cat /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state_open + rgb1:0 rgb2:4 + +What: /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state_short +Date: May 2025 +KernelVersion: 6.15 +Contact: Steffen Trumtrar <kernel@pengutronix.de> +Description: + Contains all LEDs and channels where a short condition was detected. + The format is ledname:channel. + + Example usage:: + + ## Read + # cat /sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fault_state_short + rgb1:0 rgb2:4 diff --git a/drivers/leds/leds-lp5860-core.c b/drivers/leds/leds-lp5860-core.c index c3ce2528783a1910a267c1b35a65ad27a63083db..0a802bc7a6aca25dab5cf012a1a0fcb2edcf9432 100644 --- a/drivers/leds/leds-lp5860-core.c +++ b/drivers/leds/leds-lp5860-core.c @@ -27,6 +27,140 @@ static struct lp5860_led *mcled_cdev_to_led(struct led_classdev_mc *mc_cdev) return container_of(mc_cdev, struct lp5860_led, mc_cdev); } +static const char *lp5860_find_led(struct lp5860 *lp, unsigned int idx) +{ + struct mc_subled *mc_led_info; + struct lp5860_led *led; + int i, j; + + for (i = lp->leds_size - 1; i >= 0; i--) { + led = &lp->leds[i]; + + mc_led_info = led->mc_cdev.subled_info; + + for (j = 0; j < led->mc_cdev.num_colors; j++) { + if (mc_led_info[j].channel == idx) + goto out; + } + } + +out: + return led->mc_cdev.led_cdev.dev->kobj.name; +} + +static ssize_t lp5860_fault_state_lod_lsd(struct lp5860 *led, char *buf, + unsigned int reg, unsigned int length) +{ + unsigned int value = 0; + unsigned int i, j; + unsigned int max_bits; + unsigned int offset = 0; + int len = 0; + bool match = false; + int ret; + + for (i = 0; i < length; i++) { + match = false; + ret = regmap_read(led->regmap, reg + i, &value); + if (ret) + return ret; + + max_bits = BITS_PER_BYTE; + // every 3rd Dot_x register only has 2 bits + if (i%3 == 2) + max_bits = 2; + + for (j = 0; j < max_bits; j++) { + offset++; + if (value & BIT(j)) { + len += sprintf(buf + len, "%s:%d", + lp5860_find_led(led, offset), + offset-1); + match = true; + len += sprintf(buf + len, " "); + } + } + } + + buf[len++] = '\n'; + + return len; +} + +static ssize_t lp5860_fault_state_open_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct lp5860 *led = dev_get_drvdata(dev); + unsigned int value = 0; + int ret; + + ret = regmap_read(led->regmap, LP5860_FAULT_STATE, &value); + if (ret) + return ret; + + if (!(value & LP5860_FAULT_STATE_LOD)) + return 0; + + return lp5860_fault_state_lod_lsd(led, buf, LP5860_DOT_LOD_START, + LP5860_DOT_LOD_LENGTH); +} +static LP5860_DEV_ATTR_R(fault_state_open); + +static ssize_t lp5860_fault_state_short_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct lp5860 *led = dev_get_drvdata(dev); + unsigned int value = 0; + int ret; + + ret = regmap_read(led->regmap, LP5860_FAULT_STATE, &value); + if (ret) + return ret; + + if (!(value & LP5860_FAULT_STATE_LSD)) + return 0; + + return lp5860_fault_state_lod_lsd(led, buf, LP5860_DOT_LSD_START, + LP5860_DOT_LSD_LENGTH); +} +static LP5860_DEV_ATTR_R(fault_state_short); + +static ssize_t lp5860_fault_state_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct lp5860 *led = dev_get_drvdata(dev); + unsigned int value = 0; + int ret; + + ret = regmap_read(led->regmap, LP5860_FAULT_STATE, &value); + if (ret) + return ret; + return sysfs_emit(buf, "%d\n", (value & 0x3)); +} + +static ssize_t lp5860_fault_state_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct lp5860 *led = dev_get_drvdata(dev); + unsigned int value = 0; + int ret; + + if (kstrtoint(buf, 0, &value)) + return -EINVAL; + + if (value & LP5860_FAULT_STATE_LOD) + ret = regmap_write(led->regmap, LP5860_LOD_CLEAR, 0xf); + if (value & LP5860_FAULT_STATE_LSD) + ret = regmap_write(led->regmap, LP5860_LSD_CLEAR, 0xf); + + if (ret < 0) + return ret; + return len; +} +static LP5860_DEV_ATTR_RW(fault_state); + LP5860_SHOW_MODE(r_current_set, LP5860_R_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) LP5860_STORE_MODE(r_current_set, LP5860_R_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) static LP5860_DEV_ATTR_RW(r_current_set); @@ -40,6 +174,9 @@ LP5860_STORE_MODE(b_current_set, LP5860_B_CURRENT_SET, LP5860_CC_GROUP_MASK, 0) static LP5860_DEV_ATTR_RW(b_current_set); static struct attribute *lp5860_attributes[] = { + &dev_attr_fault_state_open.attr, + &dev_attr_fault_state_short.attr, + &dev_attr_fault_state.attr, &dev_attr_r_current_set.attr, &dev_attr_g_current_set.attr, &dev_attr_b_current_set.attr, diff --git a/drivers/leds/leds-lp5860.h b/drivers/leds/leds-lp5860.h index 3b8342a832bc75afdf2318fd4ee1ee9ce105cbe3..4b1f9e2f85b9bbd16d4082521a25cdb217a5a315 100644 --- a/drivers/leds/leds-lp5860.h +++ b/drivers/leds/leds-lp5860.h @@ -196,6 +196,9 @@ #define LP5860_DOT_CS_ON 0x01 #define LP5860_DOT_CS_OFF 0x00 +#define LP5860_FAULT_STATE_LOD BIT(1) +#define LP5860_FAULT_STATE_LSD BIT(0) + /* dot lod Value */ #define LP5860_DOT_LOD0_OFFSET 0 #define LP5860_DOT_LOD1_OFFSET 1 @@ -209,6 +212,8 @@ #define LP5860_DOT_LOD_ON 0x01 #define LP5860_DOT_LOD_OFF 0x00 +#define LP5860_DOT_LOD_LENGTH 0x20 + /* dot lsd Value */ #define LP5860_DOT_LSD0_OFFSET 0 #define LP5860_DOT_LSD1_OFFSET 1 @@ -222,6 +227,8 @@ #define LP5860_DOT_LSD_ON 0x01 #define LP5860_DOT_LSD_OFF 0x00 +#define LP5860_DOT_LSD_LENGTH 0x20 + /* REG FAULT_STATE */ #define LP5860_GLOBAL_LOD_OFFSET 1 #define LP5860_GLOBAL_LOD_STATE BIT(1) @@ -257,6 +264,9 @@ */ #define LP5860_MAX_LED_CHANNELS 4 +#define LP5860_DEV_ATTR_R(name) \ + DEVICE_ATTR(name, 0444, lp5860_##name##_show, NULL) + #define LP5860_DEV_ATTR_RW(name) \ DEVICE_ATTR(name, 0644, lp5860_##name##_show, lp5860_##name##_store) -- 2.47.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver 2025-05-14 10:36 [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Steffen Trumtrar ` (3 preceding siblings ...) 2025-05-14 10:36 ` [PATCH v2 4/4] leds: lp5860: detect and report fault state Steffen Trumtrar @ 2025-05-23 11:53 ` Pavel Machek 4 siblings, 0 replies; 11+ messages in thread From: Pavel Machek @ 2025-05-23 11:53 UTC (permalink / raw) To: Steffen Trumtrar Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Steffen Trumtrar, linux-leds, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 514 bytes --] Hi! > The lp5860 is a LED matrix driver with 18 constant current sinks and 11 > scan switches which allows controlling up to 198 LED dots. 200 points. Is it likely to be used as individual LEDs? > Documentation/ABI/testing/sysfs-class-spi-lp5860 | 73 ++++ I'm not big fan of new ABI. Should currents / per channel dimming be configured from dts (and not sysfs)? Best regards, Pavel -- I don't work for Nazis and criminals, and neither should you. Boycott Putin, Trump, and Musk! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-07-18 13:19 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-14 10:36 [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Steffen Trumtrar 2025-05-14 10:36 ` [PATCH v2 1/4] dt-bindings: leds: add lp5860 LED controller Steffen Trumtrar 2025-05-21 9:02 ` Krzysztof Kozlowski 2025-05-14 10:36 ` [PATCH v2 2/4] leds: add support for TI LP5860 LED driver chip Steffen Trumtrar 2025-05-23 10:28 ` Lee Jones 2025-07-14 8:26 ` Steffen Trumtrar 2025-07-18 13:18 ` Lee Jones 2025-05-14 10:36 ` [PATCH v2 3/4] leds: lp5860: save count of multi_leds Steffen Trumtrar 2025-06-12 13:52 ` Lee Jones 2025-05-14 10:36 ` [PATCH v2 4/4] leds: lp5860: detect and report fault state Steffen Trumtrar 2025-05-23 11:53 ` [PATCH v2 0/4] LED: Add basic LP5860 LED matrix driver Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).