Skip to contentSkip to conversation

Multicolor Framework v31

PATCH Applied Dan Murphy 2 participants 24 messages Jul 16, 2020 – Jul 22, 2020

Subsystems (5)

v31 of 33 Applied Latest is v33 →

lore
Share & tools
lore

Dan Murphy

author
#0 lore raw

v31 cover Multicolor Framework v31

Hello

This is the multi color LED framework. This framework presents clustered
colored LEDs into an array and allows the user space to adjust the brightness of the cluster using a single file write. The individual colored LEDs intensities are controlled via a single file that is an array of LEDs

Globally changed multi color->multicolor. Simplified adding a new line. Updated testing doc to reflect 5.9 kernel. Rebased on LEDs for-next branch.

Dan

Dan Murphy (12):
leds: multicolor: Introduce a multicolor class definition
dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
leds: lp50xx: Add the LP50XX family of the RGB LED driver
dt-bindings: leds: Convert leds-lp55xx to yaml
leds: lp55xx: Convert LED class registration to devm_*
leds: lp55xx: Add multicolor framework support to lp55xx
ARM: defconfig: u8500: Add LP55XX_COMMON config flag
leds: lp5523: Update the lp5523 code to add multicolor brightness
function
leds: lp5521: Add multicolor framework multicolor brightness support
ARM: dts: n900: Add reg property to the LP5523 channel node
ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 channel node
ARM: dts: ste-href: Add reg property to the LP5521 channel nodes

 .../ABI/testing/sysfs-class-led-multicolor    |  35 +
 .../devicetree/bindings/leds/leds-lp50xx.yaml | 130 +++
 .../devicetree/bindings/leds/leds-lp55xx.txt  | 228 -----
 .../devicetree/bindings/leds/leds-lp55xx.yaml | 220 +++++
 Documentation/leds/index.rst                  |   1 +
 Documentation/leds/leds-class-multicolor.rst  |  86 ++
 arch/arm/boot/dts/imx6dl-yapp4-common.dtsi    |  14 +-
 arch/arm/boot/dts/omap3-n900.dts              |  29 +-
 arch/arm/boot/dts/ste-href.dtsi               |  22 +-
 arch/arm/configs/u8500_defconfig              |   1 +
 drivers/leds/Kconfig                          |  32 +-
 drivers/leds/Makefile                         |   2 +
 drivers/leds/led-class-multicolor.c           | 203 +++++
 drivers/leds/leds-lp50xx.c                    | 784 ++++++++++++++++++
 drivers/leds/leds-lp5521.c                    |  43 +-
 drivers/leds/leds-lp5523.c                    |  43 +-
 drivers/leds/leds-lp5562.c                    |  22 +-
 drivers/leds/leds-lp55xx-common.c             | 190 ++++-
 drivers/leds/leds-lp55xx-common.h             |  16 +-
 drivers/leds/leds-lp8501.c                    |  23 +-
 include/linux/led-class-multicolor.h          | 121 +++
 include/linux/platform_data/leds-lp55xx.h     |   7 +
 22 files changed, 1914 insertions(+), 338 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-lp55xx.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
 create mode 100644 Documentation/leds/leds-class-multicolor.rst
 create mode 100644 drivers/leds/led-class-multicolor.c
 create mode 100644 drivers/leds/leds-lp50xx.c
 create mode 100644 include/linux/led-class-multicolor.h
Signature · 1 line
2.27.0
author
#1 lore raw

PATCH v31 1/12 leds: multicolor: Introduce a multicolor class definition

Introduce a multicolor class that groups colored LEDs within a LED node.

The multicolor class groups monochrome LEDs and allows controlling two aspects of the final combined color: hue and lightness. The former is controlled via the intensity file and the latter is controlled via brightness file.

---
 .../ABI/testing/sysfs-class-led-multicolor    |  35 +++
 Documentation/leds/index.rst                  |   1 +
 Documentation/leds/leds-class-multicolor.rst  |  86 ++++++++
 drivers/leds/Kconfig                          |  10 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/led-class-multicolor.c           | 203 ++++++++++++++++++
 include/linux/led-class-multicolor.h          | 121 +++++++++++
 7 files changed, 457 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor
 create mode 100644 Documentation/leds/leds-class-multicolor.rst
 create mode 100644 drivers/leds/led-class-multicolor.c
 create mode 100644 include/linux/led-class-multicolor.h
Documentation/ABI/testing/sysfs-class-led-multicolor new file +35 -0
new file mode 100644
index 000000000000..eeeddcbdbbe3
@@ -0,0 +1,35 @@
+What:		/sys/class/leds/<led>/brightness
+Date:		March 2020
+KernelVersion:	5.9
+Contact:	Dan Murphy <[email protected]>
+Description:	read/write
+		Writing to this file will update all LEDs within the group to a
+		calculated percentage of what each color LED intensity is set
+		to. The percentage is calculated for each grouped LED via the
+		equation below:
+
+		led_brightness = brightness * multi_intensity/max_brightness
+
+		For additional details please refer to
+		Documentation/leds/leds-class-multicolor.rst.
+
+		The value of the LED is from 0 to
+		/sys/class/leds/<led>/max_brightness.
+
+What:		/sys/class/leds/<led>/multi_index
+Date:		March 2020
+KernelVersion:	5.9
+Contact:	Dan Murphy <[email protected]>
+Description:	read
+		The multi_index array, when read, will output the LED colors
+		as an array of strings as they are indexed in the
+		multi_intensity file.
+
+What:		/sys/class/leds/<led>/multi_intensity
+Date:		March 2020
+KernelVersion:	5.9
+Contact:	Dan Murphy <[email protected]>
+Description:	read/write
+		This file contains array of integers. Order of components is
+		described by the multi_index array. The maximum intensity should
+		not exceed /sys/class/leds/<led>/max_brightness.
Documentation/leds/index.rst +1 -0
index 060f4e485897..bc70c6aa7138 100644
@@ -9,6 +9,7 @@ LEDs
 
    leds-class
    leds-class-flash
+   leds-class-multicolor
    ledtrig-oneshot
    ledtrig-transient
    ledtrig-usbport
Documentation/leds/leds-class-multicolor.rst new file +86 -0

86 lines not on this page · raw message

drivers/leds/Kconfig +10 -0
index ed943140e1fd..a2c72212ddfc 100644
@@ -30,6 +30,16 @@ config LEDS_CLASS_FLASH
 	  for the flash related features of a LED device. It can be built
 	  as a module.
 
+config LEDS_CLASS_MULTICOLOR
+	tristate "LED MultiColor Class Support"
+	depends on LEDS_CLASS
+	help
+	  This option enables the multicolor LED sysfs class in /sys/class/leds.
+	  It wraps LED class and adds multicolor LED specific sysfs attributes
+	  and kernel internal API to it. You'll need this to provide support
+	  for multicolor LEDs that are grouped together. This class is not
+	  intended for single color LEDs. It can be built as a module.
+
 config LEDS_BRIGHTNESS_HW_CHANGED
 	bool "LED Class brightness_hw_changed attribute support"
 	depends on LEDS_CLASS
drivers/leds/Makefile +1 -0
index d6b8a792c936..d684bc76d2b2 100644
@@ -4,6 +4,7 @@
 obj-$(CONFIG_NEW_LEDS)			+= led-core.o
 obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
 obj-$(CONFIG_LEDS_CLASS_FLASH)		+= led-class-flash.o
+obj-$(CONFIG_LEDS_CLASS_MULTICOLOR)	+= led-class-multicolor.o
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers (keep this sorted, M-| sort)
drivers/leds/led-class-multicolor.c new file +203 -0

203 lines not on this page · raw message

include/linux/led-class-multicolor.h new file +121 -0

121 lines not on this page · raw message

Signature · 1 line
2.27.0
Applied
#2 lore raw

Re: [PATCH v31 01/12] leds: multicolor: Introduce a multicolor class definition

Hi!

Quoted 10 lines

Introduce a multicolor class that groups colored LEDs within a LED node.

The multicolor class groups monochrome LEDs and allows controlling two aspects of the final combined color: hue and lightness. The former is controlled via the intensity file and the latter is controlled via brightness file.

Thanks, applied and pushed out.

Quoted 3 lines

+====================================
+MultiColor LED handling under Linux
+====================================

...

Quoted 2 lines

+Multicolor Class Control
+========================

AFAICT The first one should be "Multicolor" for consistency.

drivers/leds/Kconfig config LEDS_CLASS_FLASH · line 34

Quoted 2 lines
drivers/leds/Kconfig +2 -0
+config LEDS_CLASS_MULTICOLOR
+	tristate "LED MultiColor Class Support"

Here too.

Can you send a followup patch to fix it up?

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#3 lore raw

Re: [PATCH v31 01/12] leds: multicolor: Introduce a multicolor class definition

Pavel

Quoted 5 lines · Pavel Machek

Hi!

Quoted 3 lines

Introduce a multicolor class that groups colored LEDs within a LED node.

Quoted 8 lines
Quoted 7 lines

The multicolor class groups monochrome LEDs and allows controlling two aspects of the final combined color: hue and lightness. The former is controlled via the intensity file and the latter is controlled via brightness file.

Thanks, applied and pushed out.

Thank you. What about the patches for the users?

Quoted 6 lines
Quoted 3 lines

+====================================
+MultiColor LED handling under Linux
+====================================

...

Quoted 2 lines

+Multicolor Class Control
+========================

Quoted 7 lines

AFAICT The first one should be "Multicolor" for consistency.

drivers/leds/Kconfig config LEDS_CLASS_FLASH · line 34

Quoted 2 lines
drivers/leds/Kconfig +2 -0
+config LEDS_CLASS_MULTICOLOR
+	tristate "LED MultiColor Class Support"

Here too.

Can you send a followup patch to fix it up?

Will send a patch to fix it up but not sure if I should send as part of this series or separately?

Because I am not sure if you are going to apply the remaining patches up to the DTs

Dan

Quoted 2 lines

Best regards,
Pavel

author
#4 lore raw

PATCH v31 2/12 dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers

In reply to Dan Murphy

Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024, LP5018, LP5012 and LP5009 RGB LED device driver. The LP5036/30/24/18/12/9 can control RGB LEDs individually or as part of a control bank group. These devices have the ability to adjust the mixing control for the RGB LEDs to obtain different colors independent of the overall brightness of the LED grouping.

Datasheet:
http://www.ti.com/lit/ds/symlink/lp5012.pdf
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdf

---
 .../devicetree/bindings/leds/leds-lp50xx.yaml | 130 ++++++++++++++++++
 1 file changed, 130 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
Documentation/devicetree/bindings/leds/leds-lp50xx.yaml new file +130 -0
new file mode 100644
index 000000000000..de5225b54a45
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-lp50xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LED driver for LP50XX RGB LED from Texas Instruments.
+
+maintainers:
+  - Dan Murphy <[email protected]>
+
+description: |
+  The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into
+  a LED group or control them individually.
+
+  The difference in these RGB LED drivers is the number of supported RGB
+  modules.
+
+  For more product information please see the link below:
+  http://www.ti.com/lit/ds/symlink/lp5012.pdf
+  http://www.ti.com/lit/ds/symlink/lp5024.pdf
+  http://www.ti.com/lit/ds/symlink/lp5036.pdf
+
+properties:
+  compatible:
+    enum:
+      - ti,lp5009
+      - ti,lp5012
+      - ti,lp5018
+      - ti,lp5024
+      - ti,lp5030
+      - ti,lp5036
+
+  reg:
+    maxItems: 1
+    description:
+      I2C slave address
+      lp5009/12 - 0x14, 0x15, 0x16, 0x17
+      lp5018/24 - 0x28, 0x29, 0x2a, 0x2b
+      lp5030/36 - 0x30, 0x31, 0x32, 0x33
+
+  enable-gpios:
+    maxItems: 1
+    description: GPIO pin to enable/disable the device.
+
+  vled-supply:
+    description: LED supply.
+
+patternProperties:
+  '^multi-led@[0-9a-f]$':
+    type: object
+    allOf:
+      - $ref: leds-class-multicolor.yaml#
+    properties:
+      reg:
+        minItems: 1
+        maxItems: 12
+        description:
+          This property denotes the LED module number(s) that is used on the
+          for the child node.  The LED modules can either be used stand alone
+          or grouped into a module bank.
+
+    patternProperties:
+      "(^led-[0-9a-f]$|led)":
+        type: object
+        $ref: common.yaml#
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+   #include <dt-bindings/gpio/gpio.h>
+   #include <dt-bindings/leds/common.h>
+
+   i2c {
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       led-controller@14 {
+           compatible = "ti,lp5009";
+           reg = <0x14>;
+           #address-cells = <1>;
+           #size-cells = <0>;
+           enable-gpios = <&gpio1 16>;
+
+           multi-led@1 {
+               #address-cells = <1>;
+               #size-cells = <0>;
+               reg = <0x1>;
+               color = <LED_COLOR_ID_MULTI>;
+               function = LED_FUNCTION_CHARGING;
+
+               led-0 {
+                   color = <LED_COLOR_ID_RED>;
+               };
+
+               led-1 {
+                   color = <LED_COLOR_ID_GREEN>;
+               };
+
+               led-2 {
+                   color = <LED_COLOR_ID_BLUE>;
+               };
+          };
+
+          multi-led@2 {
+            #address-cells = <1>;
+            #size-cells = <2>;
+            reg = <0x2 0x3 0x5>;
+            color = <LED_COLOR_ID_MULTI>;
+            function = LED_FUNCTION_STANDBY;
+
+            led-6 {
+              color = <LED_COLOR_ID_RED>;
+            };
+
+            led-7 {
+              color = <LED_COLOR_ID_GREEN>;
+            };
+
+            led-8 {
+              color = <LED_COLOR_ID_BLUE>;
+            };
+         };
+       };
+    };
+
+...
Signature · 1 line
2.27.0
author
#5 lore raw

PATCH v31 3/12 leds: lp50xx: Add the LP50XX family of the RGB LED driver

In reply to Dan Murphy

Introduce the LP5036/30/24/18/12/9 RGB LED driver. The difference in these parts are the number of LED outputs where the:

LP5036 can control 36 LEDs
LP5030 can control 30 LEDs
LP5024 can control 24 LEDs
LP5018 can control 18 LEDs
LP5012 can control 12 LEDs
LP5009 can control 9 LEDs

The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently.

---
 drivers/leds/Kconfig       |  11 +
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lp50xx.c | 784 +++++++++++++++++++++++++++++++++++++
 3 files changed, 796 insertions(+)
 create mode 100644 drivers/leds/leds-lp50xx.c
drivers/leds/Kconfig +11 -0
index a2c72212ddfc..0af209d44c9e 100644
@@ -384,6 +384,17 @@ config LEDS_LP3952
 	  To compile this driver as a module, choose M here: the
 	  module will be called leds-lp3952.
 
+config LEDS_LP50XX
+	tristate "LED Support for TI LP5036/30/24/18/12/9 LED driver chip"
+	depends on LEDS_CLASS && REGMAP_I2C
+	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
+	help
+	  If you say yes here you get support for the Texas Instruments
+	  LP5036, LP5030, LP5024, LP5018, LP5012 and LP5009 LED driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called leds-lp50xx.
+
 config LEDS_LP55XX_COMMON
 	tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"
 	depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
drivers/leds/Makefile +1 -0
index d684bc76d2b2..0231070602a6 100644
@@ -49,6 +49,7 @@ obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LP3944)		+= leds-lp3944.o
 obj-$(CONFIG_LEDS_LP3952)		+= leds-lp3952.o
+obj-$(CONFIG_LEDS_LP50XX)		+= leds-lp50xx.o
 obj-$(CONFIG_LEDS_LP5521)		+= leds-lp5521.o
 obj-$(CONFIG_LEDS_LP5523)		+= leds-lp5523.o
 obj-$(CONFIG_LEDS_LP5562)		+= leds-lp5562.o
drivers/leds/leds-lp50xx.c new file +784 -0

784 lines not on this page · raw message

Signature · 1 line
2.27.0
#6 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Hi!

Quoted 3 lines

The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently.

Inversely?

drivers/leds/leds-lp50xx.c line 4

Quoted 7 lines
drivers/leds/leds-lp50xx.c new file +4 -0
@@ -0,0 +1,784 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LP50XX LED chip family driver
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+

Can we get https here and in the binding document?

Please run this through checkpatch -- I believe it will have some comments.

drivers/leds/leds-lp50xx.c line 580

Quoted 8 lines
drivers/leds/leds-lp50xx.c new file +8 -0
+
+	device_for_each_child_node(priv->dev, child) {
+		led = &priv->leds[i];
+		ret = fwnode_property_count_u32(child, "reg");
+		if (ret < 0) {
+			dev_err(&priv->client->dev,
+					"reg property is invalid\n");
+			return -EINVAL;

is handle_put(child) needed here?

Quoted 6 lines
drivers/leds/leds-lp50xx.c new file +6 -0
+		}
+		if (ret > 1) {
+			priv->num_of_banked_leds = ret;
+			if (priv->num_of_banked_leds >
+			    priv->chip_info->max_modules) {
+				dev_err(&priv->client->dev,

drivers/leds/leds-lp50xx.c line 596

Quoted 10 lines
drivers/leds/leds-lp50xx.c new file +10 -0
+					"reg property is invalid\n");
+				ret = -EINVAL;
+				fwnode_handle_put(child);
+				goto child_out;
+			}
+
+			ret = fwnode_property_read_u32_array(child,
+							     "reg",
+							     led_banks,
+							     ret);

Move this to subfunction to reduce the indentation? (Or, just refactor it somehow).

drivers/leds/leds-lp50xx.c line 602

Quoted 6 lines
drivers/leds/leds-lp50xx.c new file +6 -0
+			if (ret) {
+				dev_err(&priv->client->dev,
+					"reg property is missing\n");
+				fwnode_handle_put(child);
+				goto child_out;
+			}

Create label that does the handle_put so you don't need to repeat it quite so often?

drivers/leds/leds-lp50xx.c line 651

Quoted 7 lines
drivers/leds/leds-lp50xx.c new file +7 -0
+		fwnode_for_each_child_node(child, led_node) {
+			ret = fwnode_property_read_u32(led_node, "color",
+						       &color_id);
+			if (ret)
+				dev_err(priv->dev, "Cannot read color\n");
+
+			mc_led_info[num_colors].color_index = color_id;

This uses undefined value.

drivers/leds/leds-lp50xx.c line 712

Quoted 1 line
drivers/leds/leds-lp50xx.c new file +1 -0
+	ret = lp50xx_reset(led);

Does the GPIO need to be disabled before enabling it for reset?

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#7 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Pavel

Quoted 6 lines · Pavel Machek

Hi!

Quoted 3 lines

The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently.

Inversely?

I will revise it.

Quoted 10 lines

drivers/leds/leds-lp50xx.c line 4

Quoted 8 lines
drivers/leds/leds-lp50xx.c new file +4 -0
@@ -0,0 +1,784 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LP50XX LED chip family driver
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+

Can we get https here and in the binding document?

Quoted 2 lines

Please run this through checkpatch -- I believe it will have some comments.

OK.

Quoted 10 lines

drivers/leds/leds-lp50xx.c line 580

Quoted 9 lines
drivers/leds/leds-lp50xx.c new file +8 -0
+
+	device_for_each_child_node(priv->dev, child) {
+		led = &priv->leds[i];
+		ret = fwnode_property_count_u32(child, "reg");
+		if (ret < 0) {
+			dev_err(&priv->client->dev,
+					"reg property is invalid\n");
+			return -EINVAL;

is handle_put(child) needed here?

It will be after I refactor the label

Quoted 10 lines
Quoted 10 lines
drivers/leds/leds-lp50xx.c new file +10 -0
+		}
+		if (ret > 1) {
+			priv->num_of_banked_leds = ret;
+			if (priv->num_of_banked_leds >
+			    priv->chip_info->max_modules) {
+				dev_err(&priv->client->dev,
+					"reg property is invalid\n");
+				ret = -EINVAL;
+				fwnode_handle_put(child);
+				goto child_out;
Quoted 8 lines

drivers/leds/leds-lp50xx.c line 596

Quoted 6 lines
drivers/leds/leds-lp50xx.c new file +6 -0
+			}
+
+			ret = fwnode_property_read_u32_array(child,
+							     "reg",
+							     led_banks,
+							     ret);

Move this to subfunction to reduce the indentation? (Or, just refactor it somehow).

Actually I can just put it all on the same line since the 80 character requirement is relaxed.

Quoted 8 lines

drivers/leds/leds-lp50xx.c line 602

Quoted 6 lines
drivers/leds/leds-lp50xx.c new file +6 -0
+			if (ret) {
+				dev_err(&priv->client->dev,
+					"reg property is missing\n");
+				fwnode_handle_put(child);
+				goto child_out;
+			}

Create label that does the handle_put so you don't need to repeat it quite so often?

I will rework it for  all

Quoted 9 lines

drivers/leds/leds-lp50xx.c line 651

Quoted 8 lines
drivers/leds/leds-lp50xx.c new file +7 -0
+		fwnode_for_each_child_node(child, led_node) {
+			ret = fwnode_property_read_u32(led_node, "color",
+						       &color_id);
+			if (ret)
+				dev_err(priv->dev, "Cannot read color\n");
+
+			mc_led_info[num_colors].color_index = color_id;

This uses undefined value.

OK needs to goto to out.

Quoted 3 lines

drivers/leds/leds-lp50xx.c line 712

Quoted 2 lines
drivers/leds/leds-lp50xx.c new file +1 -0
+	ret = lp50xx_reset(led);

Does the GPIO need to be disabled before enabling it for reset?

You mean toggle the GPIO?  Yes it should be toggled I will update it.

Dan

Quoted 3 lines

Best regards,
Pavel

#8 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Hi!

Quoted 9 lines
Quoted 6 lines

drivers/leds/leds-lp50xx.c line 596

Quoted 4 lines
drivers/leds/leds-lp50xx.c new file +4 -0
+			ret = fwnode_property_read_u32_array(child,
+							     "reg",
+							     led_banks,
+							     ret);

Move this to subfunction to reduce the indentation? (Or, just refactor it somehow).

Actually I can just put it all on the same line since the 80 character requirement is relaxed.

No.

You have too long and too complex function, with too many blocks inside each other. Please fix it.

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#9 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Pavel

Quoted 8 lines · Pavel Machek

Hi!

Quoted 6 lines
Quoted 6 lines

drivers/leds/leds-lp50xx.c line 596

Quoted 4 lines
drivers/leds/leds-lp50xx.c new file +4 -0
+			ret = fwnode_property_read_u32_array(child,
+							     "reg",
+							     led_banks,
+							     ret);

Move this to subfunction to reduce the indentation? (Or, just refactor it somehow).

Quoted 6 lines
Quoted 2 lines

Actually I can just put it all on the same line since the 80 character requirement is relaxed.

No.

You have too long and too complex function, with too many blocks inside each other. Please fix it.

I will refactor

Dan

#10 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Hi!

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

drivers/leds/leds-lp50xx.c line 596

Quoted 4 lines
drivers/leds/leds-lp50xx.c new file +4 -0
+			ret = fwnode_property_read_u32_array(child,
+							     "reg",
+							     led_banks,
+							     ret);

Move this to subfunction to reduce the indentation? (Or, just refactor it somehow).

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Actually I can just put it all on the same line since the 80 character requirement is relaxed.

No.

You have too long and too complex function, with too many blocks inside each other. Please fix it.

I will refactor

Thank you.
Pavel

#11 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

In reply to Dan Murphy
Quoted 18 lines · Dan Murphy

Introduce the LP5036/30/24/18/12/9 RGB LED driver. The difference in these parts are the number of LED outputs where the:

LP5036 can control 36 LEDs
LP5030 can control 30 LEDs
LP5024 can control 24 LEDs
LP5018 can control 18 LEDs
LP5012 can control 12 LEDs
LP5009 can control 9 LEDs

The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently.

Quoted 9 lines

+/*
+ * struct lp50xx_chip_info -
+ * @num_leds: number of LED outputs available on the device
+ * @led_brightness0_reg: first brightness register of the device
+ * @mix_out0_reg: first color mix register of the device
+ * @bank_brt_reg: bank brightness register
+ * @bank_mix_reg: color mix register
+ * @reset_reg: device reset register
+ */

Should have /** if this is kerneldoc.

drivers/leds/leds-lp50xx.c line 639

Quoted 6 lines
drivers/leds/leds-lp50xx.c new file +6 -0
+		init_data.fwnode = child;
+		num_colors = 0;
+
+		/* There are only 3 LEDs per module otherwise they should be
+		 * banked which also is presented as 3 LEDs
+		 */

This is not usual comment style for kernel. (And add . at end of sentence).

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#12 lore raw

Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Pavel

Quoted 29 lines · Pavel Machek
Quoted 17 lines · Dan Murphy

Introduce the LP5036/30/24/18/12/9 RGB LED driver. The difference in these parts are the number of LED outputs where the:

LP5036 can control 36 LEDs
LP5030 can control 30 LEDs
LP5024 can control 24 LEDs
LP5018 can control 18 LEDs
LP5012 can control 12 LEDs
LP5009 can control 9 LEDs

The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently.

Quoted 9 lines

+/*
+ * struct lp50xx_chip_info -
+ * @num_leds: number of LED outputs available on the device
+ * @led_brightness0_reg: first brightness register of the device
+ * @mix_out0_reg: first color mix register of the device
+ * @bank_brt_reg: bank brightness register
+ * @bank_mix_reg: color mix register
+ * @reset_reg: device reset register
+ */

Should have /** if this is kerneldoc.

Quoted 8 lines

drivers/leds/leds-lp50xx.c line 639

Quoted 6 lines
drivers/leds/leds-lp50xx.c new file +6 -0
+		init_data.fwnode = child;
+		num_colors = 0;
+
+		/* There are only 3 LEDs per module otherwise they should be
+		 * banked which also is presented as 3 LEDs
+		 */

This is not usual comment style for kernel. (And add . at end of sentence).

I will fix both.

Dan

Quoted 2 lines

Best regards,
Pavel

author
#13 lore raw

PATCH v31 4/12 dt-bindings: leds: Convert leds-lp55xx to yaml

In reply to Dan Murphy

Convert the leds-lp55xx.txt to yaml binding.

---
 .../devicetree/bindings/leds/leds-lp55xx.txt  | 228 ------------------
 .../devicetree/bindings/leds/leds-lp55xx.yaml | 220 +++++++++++++++++
 2 files changed, 220 insertions(+), 228 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-lp55xx.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
Documentation/devicetree/bindings/leds/leds-lp55xx.txt deleted +0 -228
deleted file mode 100644
index 1b66a413fb9d..000000000000
@@ -1,228 +0,0 @@
-Binding for TI/National Semiconductor LP55xx Led Drivers
-
-Required properties:
-- compatible: one of
-	national,lp5521
-	national,lp5523
-	ti,lp55231
-	ti,lp5562
-	ti,lp8501
-
-- reg: I2C slave address
-- clock-mode: Input clock mode, (0: automode, 1: internal, 2: external)
-
-Each child has own specific current settings
-- led-cur: Current setting at each led channel (mA x10, 0 if led is not connected)
-- max-cur: Maximun current at each led channel.
-
-Optional properties:
-- enable-gpio: GPIO attached to the chip's enable pin
-- label: Used for naming LEDs
-- pwr-sel: LP8501 specific property. Power selection for output channels.
-         0: D1~9 are connected to VDD
-         1: D1~6 with VDD, D7~9 with VOUT
-         2: D1~6 with VOUT, D7~9 with VDD
-         3: D1~9 are connected to VOUT
-
-Alternatively, each child can have a specific channel name and trigger:
-- chan-name (optional): name of channel
-- linux,default-trigger (optional): see
-  Documentation/devicetree/bindings/leds/common.txt
-
-example 1) LP5521
-3 LED channels, external clock used. Channel names are 'lp5521_pri:channel0',
-'lp5521_pri:channel1' and 'lp5521_pri:channel2', with a heartbeat trigger
-on channel 0.
-
-lp5521@32 {
-	compatible = "national,lp5521";
-	reg = <0x32>;
-	label = "lp5521_pri";
-	clock-mode = /bits/ 8 <2>;
-
-	chan0 {
-		led-cur = /bits/ 8 <0x2f>;
-		max-cur = /bits/ 8 <0x5f>;
-		linux,default-trigger = "heartbeat";
-	};
-
-	chan1 {
-		led-cur = /bits/ 8 <0x2f>;
-		max-cur = /bits/ 8 <0x5f>;
-	};
-
-	chan2 {
-		led-cur = /bits/ 8 <0x2f>;
-		max-cur = /bits/ 8 <0x5f>;
-	};
-};
-
-example 2) LP5523
-9 LED channels with specific name. Internal clock used.
-The I2C slave address is configurable with ASEL1 and ASEL0 pins.
-Available addresses are 32/33/34/35h.
-
-ASEL1    ASEL0    Address
--------------------------
- GND      GND       32h
- GND      VEN       33h
- VEN      GND       34h
- VEN      VEN       35h
-
-lp5523@32 {
-	compatible = "national,lp5523";
-	reg = <0x32>;
-	clock-mode = /bits/ 8 <1>;
-
-	chan0 {
-		chan-name = "d1";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan1 {
-		chan-name = "d2";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan2 {
-		chan-name = "d3";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan3 {
-		chan-name = "d4";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan4 {
-		chan-name = "d5";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan5 {
-		chan-name = "d6";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan6 {
-		chan-name = "d7";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan7 {
-		chan-name = "d8";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan8 {
-		chan-name = "d9";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-};
-
-example 3) LP5562
-4 channels are defined.
-
-lp5562@30 {
-	compatible = "ti,lp5562";
-	reg = <0x30>;
-	clock-mode = /bits/8 <2>;
-
-	chan0 {
-		chan-name = "R";
-		led-cur = /bits/ 8 <0x20>;
-		max-cur = /bits/ 8 <0x60>;
-	};
-
-	chan1 {
-		chan-name = "G";
-		led-cur = /bits/ 8 <0x20>;
-		max-cur = /bits/ 8 <0x60>;
-	};
-
-	chan2 {
-		chan-name = "B";
-		led-cur = /bits/ 8 <0x20>;
-		max-cur = /bits/ 8 <0x60>;
-	};
-
-	chan3 {
-		chan-name = "W";
-		led-cur = /bits/ 8 <0x20>;
-		max-cur = /bits/ 8 <0x60>;
-	};
-};
-
-example 4) LP8501
-9 channels are defined. The 'pwr-sel' is LP8501 specific property.
-Others are same as LP5523.
-
-lp8501@32 {
-	compatible = "ti,lp8501";
-	reg = <0x32>;
-	clock-mode = /bits/ 8 <2>;
-	pwr-sel = /bits/ 8 <3>;	/* D1~9 connected to VOUT */
-
-	chan0 {
-		chan-name = "d1";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan1 {
-		chan-name = "d2";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan2 {
-		chan-name = "d3";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan3 {
-		chan-name = "d4";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan4 {
-		chan-name = "d5";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan5 {
-		chan-name = "d6";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan6 {
-		chan-name = "d7";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan7 {
-		chan-name = "d8";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-
-	chan8 {
-		chan-name = "d9";
-		led-cur = /bits/ 8 <0x14>;
-		max-cur = /bits/ 8 <0x20>;
-	};
-};
Documentation/devicetree/bindings/leds/leds-lp55xx.yaml new file +220 -0

220 lines not on this page · raw message

Signature · 1 line
2.27.0
author
#14 lore raw

PATCH v31 5/12 leds: lp55xx: Convert LED class registration to devm_*

In reply to Dan Murphy

Convert the LED class registration calls to the LED devm_* registration calls.

---
 drivers/leds/leds-lp5521.c        |  9 +++------
 drivers/leds/leds-lp5523.c        |  9 +++------
 drivers/leds/leds-lp5562.c        |  9 +++------
 drivers/leds/leds-lp55xx-common.c | 15 +--------------
 drivers/leds/leds-lp55xx-common.h |  2 --
 drivers/leds/leds-lp8501.c        |  9 +++------
 6 files changed, 13 insertions(+), 40 deletions(-)
drivers/leds/leds-lp5521.c +3 -6
index 6f0272249dc8..6d2163c0f625 100644
@@ -541,19 +541,17 @@ static int lp5521_probe(struct i2c_client *client,
 
 	ret = lp55xx_register_leds(led, chip);
 	if (ret)
-		goto err_register_leds;
+		goto err_out;
 
 	ret = lp55xx_register_sysfs(chip);
 	if (ret) {
 		dev_err(&client->dev, "registering sysfs failed\n");
-		goto err_register_sysfs;
+		goto err_out;
 	}
 
 	return 0;
 
-err_register_sysfs:
-	lp55xx_unregister_leds(led, chip);
-err_register_leds:
+err_out:
 	lp55xx_deinit_device(chip);
 err_init:
 	return ret;
@@ -566,7 +564,6 @@ static int lp5521_remove(struct i2c_client *client)
 
 	lp5521_stop_all_engines(chip);
 	lp55xx_unregister_sysfs(chip);
-	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
 	return 0;
drivers/leds/leds-lp5523.c +3 -6
index 8518de957b48..cb550cf19e14 100644
@@ -909,19 +909,17 @@ static int lp5523_probe(struct i2c_client *client,
 
 	ret = lp55xx_register_leds(led, chip);
 	if (ret)
-		goto err_register_leds;
+		goto err_out;
 
 	ret = lp55xx_register_sysfs(chip);
 	if (ret) {
 		dev_err(&client->dev, "registering sysfs failed\n");
-		goto err_register_sysfs;
+		goto err_out;
 	}
 
 	return 0;
 
-err_register_sysfs:
-	lp55xx_unregister_leds(led, chip);
-err_register_leds:
+err_out:
 	lp55xx_deinit_device(chip);
 err_init:
 	return ret;
@@ -934,7 +932,6 @@ static int lp5523_remove(struct i2c_client *client)
 
 	lp5523_stop_all_engines(chip);
 	lp55xx_unregister_sysfs(chip);
-	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
 	return 0;
drivers/leds/leds-lp5562.c +3 -6
index edb57c42e8b1..1c94422408b0 100644
@@ -554,19 +554,17 @@ static int lp5562_probe(struct i2c_client *client,
 
 	ret = lp55xx_register_leds(led, chip);
 	if (ret)
-		goto err_register_leds;
+		goto err_out;
 
 	ret = lp55xx_register_sysfs(chip);
 	if (ret) {
 		dev_err(&client->dev, "registering sysfs failed\n");
-		goto err_register_sysfs;
+		goto err_out;
 	}
 
 	return 0;
 
-err_register_sysfs:
-	lp55xx_unregister_leds(led, chip);
-err_register_leds:
+err_out:
 	lp55xx_deinit_device(chip);
 err_init:
 	return ret;
@@ -580,7 +578,6 @@ static int lp5562_remove(struct i2c_client *client)
 	lp5562_stop_engine(chip);
 
 	lp55xx_unregister_sysfs(chip);
-	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
 	return 0;
drivers/leds/leds-lp55xx-common.c +1 -14
index 3d0bc4dcd9f2..243c749ebda5 100644
@@ -179,7 +179,7 @@ static int lp55xx_init_led(struct lp55xx_led *led,
 		led->cdev.name = name;
 	}
 
-	ret = led_classdev_register(dev, &led->cdev);
+	ret = devm_led_classdev_register(dev, &led->cdev);
 	if (ret) {
 		dev_err(dev, "led register err: %d\n", ret);
 		return ret;
@@ -480,23 +480,10 @@ int lp55xx_register_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
 	return 0;
 
 err_init_led:
-	lp55xx_unregister_leds(led, chip);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(lp55xx_register_leds);
 
-void lp55xx_unregister_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
-{
-	int i;
-	struct lp55xx_led *each;
-
-	for (i = 0; i < chip->num_leds; i++) {
-		each = led + i;
-		led_classdev_unregister(&each->cdev);
-	}
-}
-EXPORT_SYMBOL_GPL(lp55xx_unregister_leds);
-
 int lp55xx_register_sysfs(struct lp55xx_chip *chip)
 {
 	struct device *dev = &chip->cl->dev;
drivers/leds/leds-lp55xx-common.h +0 -2
index 783ed5103ce5..b9b1041e8143 100644
@@ -189,8 +189,6 @@ extern void lp55xx_deinit_device(struct lp55xx_chip *chip);
 /* common LED class device functions */
 extern int lp55xx_register_leds(struct lp55xx_led *led,
 				struct lp55xx_chip *chip);
-extern void lp55xx_unregister_leds(struct lp55xx_led *led,
-				struct lp55xx_chip *chip);
 
 /* common device attributes functions */
 extern int lp55xx_register_sysfs(struct lp55xx_chip *chip);
drivers/leds/leds-lp8501.c +3 -6
index 2638dbf0e8ac..a58019cdb8c3 100644
@@ -344,19 +344,17 @@ static int lp8501_probe(struct i2c_client *client,
 
 	ret = lp55xx_register_leds(led, chip);
 	if (ret)
-		goto err_register_leds;
+		goto err_out;
 
 	ret = lp55xx_register_sysfs(chip);
 	if (ret) {
 		dev_err(&client->dev, "registering sysfs failed\n");
-		goto err_register_sysfs;
+		goto err_out;
 	}
 
 	return 0;
 
-err_register_sysfs:
-	lp55xx_unregister_leds(led, chip);
-err_register_leds:
+err_out:
 	lp55xx_deinit_device(chip);
 err_init:
 	return ret;
@@ -369,7 +367,6 @@ static int lp8501_remove(struct i2c_client *client)
 
 	lp8501_stop_engine(chip);
 	lp55xx_unregister_sysfs(chip);
-	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
 	return 0;
Signature · 1 line
2.27.0
author
#15 lore raw

PATCH v31 6/12 leds: lp55xx: Add multicolor framework support to lp55xx

In reply to Dan Murphy

Add multicolor framework support for the lp55xx family.

---
 drivers/leds/Kconfig                      |  11 +-
 drivers/leds/leds-lp5521.c                |  14 +-
 drivers/leds/leds-lp5523.c                |  14 +-
 drivers/leds/leds-lp5562.c                |  13 +-
 drivers/leds/leds-lp55xx-common.c         | 177 +++++++++++++++++++---
 drivers/leds/leds-lp55xx-common.h         |  14 +-
 drivers/leds/leds-lp8501.c                |  14 +-
 include/linux/platform_data/leds-lp55xx.h |   7 +
 8 files changed, 212 insertions(+), 52 deletions(-)
drivers/leds/Kconfig +6 -5
index 0af209d44c9e..4a642e5d6419 100644
@@ -397,7 +397,8 @@ config LEDS_LP50XX
 
 config LEDS_LP55XX_COMMON
 	tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"
-	depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
+	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
+	depends on OF
 	select FW_LOADER
 	select FW_LOADER_USER_HELPER
 	help
@@ -407,7 +408,7 @@ config LEDS_LP55XX_COMMON
 config LEDS_LP5521
 	tristate "LED Support for N.S. LP5521 LED driver chip"
 	depends on LEDS_CLASS && I2C
-	select LEDS_LP55XX_COMMON
+	depends on LEDS_LP55XX_COMMON
 	help
 	  If you say yes here you get support for the National Semiconductor
 	  LP5521 LED driver. It is 3 channel chip with programmable engines.
@@ -417,7 +418,7 @@ config LEDS_LP5521
 config LEDS_LP5523
 	tristate "LED Support for TI/National LP5523/55231 LED driver chip"
 	depends on LEDS_CLASS && I2C
-	select LEDS_LP55XX_COMMON
+	depends on LEDS_LP55XX_COMMON
 	help
 	  If you say yes here you get support for TI/National Semiconductor
 	  LP5523/55231 LED driver.
@@ -428,7 +429,7 @@ config LEDS_LP5523
 config LEDS_LP5562
 	tristate "LED Support for TI LP5562 LED driver chip"
 	depends on LEDS_CLASS && I2C
-	select LEDS_LP55XX_COMMON
+	depends on LEDS_LP55XX_COMMON
 	help
 	  If you say yes here you get support for TI LP5562 LED driver.
 	  It is 4 channels chip with programmable engines.
@@ -438,7 +439,7 @@ config LEDS_LP5562
 config LEDS_LP8501
 	tristate "LED Support for TI LP8501 LED driver chip"
 	depends on LEDS_CLASS && I2C
-	select LEDS_LP55XX_COMMON
+	depends on LEDS_LP55XX_COMMON
 	help
 	  If you say yes here you get support for TI LP8501 LED driver.
 	  It is 9 channel chip with programmable engines.
drivers/leds/leds-lp5521.c +8 -6
index 6d2163c0f625..6ff81d6be789 100644
@@ -505,9 +505,16 @@ static int lp5521_probe(struct i2c_client *client,
 	struct lp55xx_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device_node *np = client->dev.of_node;
 
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->cfg = &lp5521_cfg;
+
 	if (!pdata) {
 		if (np) {
-			pdata = lp55xx_of_populate_pdata(&client->dev, np);
+			pdata = lp55xx_of_populate_pdata(&client->dev, np,
+							 chip);
 			if (IS_ERR(pdata))
 				return PTR_ERR(pdata);
 		} else {
@@ -516,10 +523,6 @@ static int lp5521_probe(struct i2c_client *client,
 		}
 	}
 
-	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
-	if (!chip)
-		return -ENOMEM;
-
 	led = devm_kcalloc(&client->dev,
 			pdata->num_channels, sizeof(*led), GFP_KERNEL);
 	if (!led)
@@ -527,7 +530,6 @@ static int lp5521_probe(struct i2c_client *client,
 
 	chip->cl = client;
 	chip->pdata = pdata;
-	chip->cfg = &lp5521_cfg;
 
 	mutex_init(&chip->lock);
 
drivers/leds/leds-lp5523.c +8 -6
index cb550cf19e14..bb97549007d7 100644
@@ -873,9 +873,16 @@ static int lp5523_probe(struct i2c_client *client,
 	struct lp55xx_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device_node *np = client->dev.of_node;
 
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->cfg = &lp5523_cfg;
+
 	if (!pdata) {
 		if (np) {
-			pdata = lp55xx_of_populate_pdata(&client->dev, np);
+			pdata = lp55xx_of_populate_pdata(&client->dev, np,
+							 chip);
 			if (IS_ERR(pdata))
 				return PTR_ERR(pdata);
 		} else {
@@ -884,10 +891,6 @@ static int lp5523_probe(struct i2c_client *client,
 		}
 	}
 
-	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
-	if (!chip)
-		return -ENOMEM;
-
 	led = devm_kcalloc(&client->dev,
 			pdata->num_channels, sizeof(*led), GFP_KERNEL);
 	if (!led)
@@ -895,7 +898,6 @@ static int lp5523_probe(struct i2c_client *client,
 
 	chip->cl = client;
 	chip->pdata = pdata;
-	chip->cfg = &lp5523_cfg;
 
 	mutex_init(&chip->lock);
 
drivers/leds/leds-lp5562.c +8 -5
index 1c94422408b0..7ecdd199d7ef 100644
@@ -520,9 +520,16 @@ static int lp5562_probe(struct i2c_client *client,
 	struct lp55xx_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device_node *np = client->dev.of_node;
 
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->cfg = &lp5562_cfg;
+
 	if (!pdata) {
 		if (np) {
-			pdata = lp55xx_of_populate_pdata(&client->dev, np);
+			pdata = lp55xx_of_populate_pdata(&client->dev, np,
+							 chip);
 			if (IS_ERR(pdata))
 				return PTR_ERR(pdata);
 		} else {
@@ -531,9 +538,6 @@ static int lp5562_probe(struct i2c_client *client,
 		}
 	}
 
-	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
-	if (!chip)
-		return -ENOMEM;
 
 	led = devm_kcalloc(&client->dev,
 			pdata->num_channels, sizeof(*led), GFP_KERNEL);
@@ -542,7 +546,6 @@ static int lp5562_probe(struct i2c_client *client,
 
 	chip->cl = client;
 	chip->pdata = pdata;
-	chip->cfg = &lp5562_cfg;
 
 	mutex_init(&chip->lock);
 
drivers/leds/leds-lp55xx-common.c +155 -22

234 lines not on this page · raw message

drivers/leds/leds-lp55xx-common.h +12 -2
index b9b1041e8143..2f38c5b33830 100644
@@ -12,6 +12,8 @@
 #ifndef _LEDS_LP55XX_COMMON_H
 #define _LEDS_LP55XX_COMMON_H
 
+#include <linux/led-class-multicolor.h>
+
 enum lp55xx_engine_index {
 	LP55XX_ENGINE_INVALID,
 	LP55XX_ENGINE_1,
@@ -93,6 +95,7 @@ struct lp55xx_reg {
  * @max_channel        : Maximum number of channels
  * @post_init_device   : Chip specific initialization code
  * @brightness_fn      : Brightness function
+ * @multicolor_brightness_fn : Multicolor brightness function
  * @set_led_current    : LED current set function
  * @firmware_cb        : Call function when the firmware is loaded
  * @run_engine         : Run internal engine for pattern
@@ -106,9 +109,12 @@ struct lp55xx_device_config {
 	/* define if the device has specific initialization process */
 	int (*post_init_device) (struct lp55xx_chip *chip);
 
-	/* access brightness register */
+	/* set LED brightness */
 	int (*brightness_fn)(struct lp55xx_led *led);
 
+	/* set multicolor LED brightness */
+	int (*multicolor_brightness_fn)(struct lp55xx_led *led);
+
 	/* current setting function */
 	void (*set_led_current) (struct lp55xx_led *led, u8 led_current);
 
@@ -159,6 +165,8 @@ struct lp55xx_chip {
  * struct lp55xx_led
  * @chan_nr         : Channel number
  * @cdev            : LED class device
+ * @mc_cdev         : Multi color class device
+ * @color_components: Multi color LED map information
  * @led_current     : Current setting at each led channel
  * @max_current     : Maximun current at each led channel
  * @brightness      : Brightness value
@@ -167,6 +175,7 @@ struct lp55xx_chip {
 struct lp55xx_led {
 	int chan_nr;
 	struct led_classdev cdev;
+	struct led_classdev_mc mc_cdev;
 	u8 led_current;
 	u8 max_current;
 	u8 brightness;
@@ -196,6 +205,7 @@ extern void lp55xx_unregister_sysfs(struct lp55xx_chip *chip);
 
 /* common device tree population function */
 extern struct lp55xx_platform_data
-*lp55xx_of_populate_pdata(struct device *dev, struct device_node *np);
+*lp55xx_of_populate_pdata(struct device *dev, struct device_node *np,
+			  struct lp55xx_chip *chip);
 
 #endif /* _LEDS_LP55XX_COMMON_H */
drivers/leds/leds-lp8501.c +8 -6
index a58019cdb8c3..ac2c31db4a65 100644
@@ -308,9 +308,16 @@ static int lp8501_probe(struct i2c_client *client,
 	struct lp55xx_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device_node *np = client->dev.of_node;
 
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->cfg = &lp8501_cfg;
+
 	if (!pdata) {
 		if (np) {
-			pdata = lp55xx_of_populate_pdata(&client->dev, np);
+			pdata = lp55xx_of_populate_pdata(&client->dev, np,
+							 chip);
 			if (IS_ERR(pdata))
 				return PTR_ERR(pdata);
 		} else {
@@ -319,10 +326,6 @@ static int lp8501_probe(struct i2c_client *client,
 		}
 	}
 
-	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
-	if (!chip)
-		return -ENOMEM;
-
 	led = devm_kcalloc(&client->dev,
 			pdata->num_channels, sizeof(*led), GFP_KERNEL);
 	if (!led)
@@ -330,7 +333,6 @@ static int lp8501_probe(struct i2c_client *client,
 
 	chip->cl = client;
 	chip->pdata = pdata;
-	chip->cfg = &lp8501_cfg;
 
 	mutex_init(&chip->lock);
 
include/linux/platform_data/leds-lp55xx.h +7 -0
index 00492d6ff018..3441064713a3 100644
@@ -13,18 +13,25 @@
 #define _LEDS_LP55XX_H
 
 #include <linux/gpio/consumer.h>
+#include <linux/led-class-multicolor.h>
 
 /* Clock configuration */
 #define LP55XX_CLOCK_AUTO	0
 #define LP55XX_CLOCK_INT	1
 #define LP55XX_CLOCK_EXT	2
 
+#define LP55XX_MAX_GROUPED_CHAN	4
+
 struct lp55xx_led_config {
 	const char *name;
 	const char *default_trigger;
 	u8 chan_nr;
 	u8 led_current; /* mA x10, 0 if led is not connected */
 	u8 max_current;
+	int num_colors;
+	unsigned int max_channel;
+	int color_id[LED_COLOR_ID_MAX];
+	int output_num[LED_COLOR_ID_MAX];
 };
 
 struct lp55xx_predef_pattern {
Signature · 1 line
2.27.0
#16 lore raw

Re: [PATCH v31 06/12] leds: lp55xx: Add multicolor framework support to lp55xx

Hi!

Quoted 5 lines

Add multicolor framework support for the lp55xx family.

Applied 4,5,6 and 8,9.

drivers/leds/Kconfig config LEDS_LP50XX · line 403

Quoted 7 lines

config LEDS_LP55XX_COMMON

drivers/leds/Kconfig +2 -1
 	tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"
-	depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
+	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
+	depends on OF
 	select FW_LOADER
 	select FW_LOADER_USER_HELPER

But I have to ask: what does this do to userland interface once LEDS_CLASS_MULTICOLOR is enabled?

Will users see some changes? Will they see some changes after dts parts are applied?

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#17 lore raw

Re: [PATCH v31 06/12] leds: lp55xx: Add multicolor framework support to lp55xx

Pavel

Quoted 9 lines · Pavel Machek

Hi!

Quoted 5 lines

Add multicolor framework support for the lp55xx family.

Applied 4,5,6 and 8,9.

Quoted 9 lines

drivers/leds/Kconfig config LEDS_LP50XX · line 403

Quoted 7 lines

config LEDS_LP55XX_COMMON
tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"

drivers/leds/Kconfig +2 -1
-	depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
+	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
+	depends on OF
  	select FW_LOADER
  	select FW_LOADER_USER_HELPER

But I have to ask: what does this do to userland interface once LEDS_CLASS_MULTICOLOR is enabled?

If the DT is instrumented with MC FW properties there will be a change to the user land interface.  If the properties follow the LED properties then there should be no change to the userland interface.  See the DT and user interface examples below.  So the n900 should see no delta in the lighting since it has the LED properties and not the MC FW properties.  I hope this answers your question.

Quoted 2 lines

Will users see some changes? Will they see some changes after dts parts are applied?

In my testing I did not see any delta in the user interface.  I tested with both MC FW and non-MC FW properties on the LP5523 evm.

So the legacy DT's should not be affected and should work as is. The DT patches were to make the DT's compliant with the updated bindings.

Only the u8500 defconfig patch needs to be added or the u8500_defconfig will break in compilation so we need that patch as well.

DTS changes not applied

With multicolor framework properties in the DT

ls
beaglebone:green:heartbeat  beaglebone:green:usr2 lp5523:channel1 beaglebone:green:mmc0       beaglebone:green:usr3 lp5523:channel5

        chan5 {
            color = <LED_COLOR_ID_WHITE>;
            chan-name = "lp5523:channel5";
            reg = <0x5>;
            led-cur = /bits/ 8 <50>;
            max-cur = /bits/ 8 <100>;
        };

        multi-led@0 {
            #address-cells = <1>;
            #size-cells = <0>;
            reg = <0>;
            color = <LED_COLOR_ID_MULTI>;
            function = LED_FUNCTION_STANDBY;
            linux,default-trigger = "heartbeat";

            led@0 {
                led-cur = /bits/ 8 <50>;
                max-cur = /bits/ 8 <100>;
                reg = <0x0>;
                color = <LED_COLOR_ID_GREEN>;
            };

            led@1 {
                led-cur = /bits/ 8 <50>;
                max-cur = /bits/ 8 <100>;
                reg = <0x1>;
                color = <LED_COLOR_ID_BLUE>;
            };

            led@6 {
                led-cur = /bits/ 8 <50>;
                max-cur = /bits/ 8 <100>;
                reg = <0x6>;
                color = <LED_COLOR_ID_RED>;
            };
        };

lp5523:channel5# ls

brightness      device          led_current     max_brightness max_current     power           subsystem       trigger uevent

ls lp5523\:channel1
brightness       device           max_brightness multi_index      multi_intensity  power subsystem        trigger          uevent

Without MC FW DT properties

And as individual LEDs as the DTs are populated today.

ls
beaglebone:green:heartbeat  beaglebone:green:usr2 lp5523:channel0 lp5523:channel5 beaglebone:green:mmc0       beaglebone:green:usr3 lp5523:channel2             lp5523:channel6

        chan0 {
            color = <LED_COLOR_ID_GREEN>;
            chan-name = "lp5523:channel0";
            reg = <0x0>;
            led-cur = /bits/ 8 <50>;
            max-cur = /bits/ 8 <100>;
        };
        chan7 {
            color = <LED_COLOR_ID_RED>;
            chan-name = "lp5523:channel6";
            reg = <0x6>;
            led-cur = /bits/ 8 <50>;
            max-cur = /bits/ 8 <100>;
        };
        chan2 {
            color = <LED_COLOR_ID_BLUE>;
            chan-name = "lp5523:channel2";
            reg = <0x1>;
            led-cur = /bits/ 8 <50>;
            max-cur = /bits/ 8 <100>;
        };

        chan5 {
            color = <LED_COLOR_ID_WHITE>;
            chan-name = "lp5523:channel5";
            reg = <0x5>;
            led-cur = /bits/ 8 <50>;
            max-cur = /bits/ 8 <100>;
        };

Dan

author
#18 lore raw

PATCH v31 7/12 ARM: defconfig: u8500: Add LP55XX_COMMON config flag

In reply to Dan Murphy

The dependencies for the LP55XX LED drivers have been updated so that the child devices (i.e. LP5521) now depends on the LP55XX_COMMON config flag as opposed to the device selecting the LP55XX_COMMON flag. The LP55XX_COMMON needs to be set in the defconfig.

---
 arch/arm/configs/u8500_defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm/configs/u8500_defconfig +1 -0
index 28dd7cf56048..dea599e1c17e 100644
@@ -113,6 +113,7 @@ CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_LM3530=y
 CONFIG_LEDS_GPIO=y
+CONFIG_LEDS_LP55XX_COMMON=y
 CONFIG_LEDS_LP5521=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_RTC_CLASS=y
Signature · 1 line
2.27.0
author
#19 lore raw

PATCH v31 8/12 leds: lp5523: Update the lp5523 code to add multicolor brightness function

In reply to Dan Murphy

Add the multicolor brightness call back to support the multicolor framework. This call back allows setting brightness on grouped channels in a single function.

---
 drivers/leds/leds-lp5523.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/leds/leds-lp5523.c +20 -0
index bb97549007d7..f55d97258d5e 100644
@@ -792,6 +792,25 @@ static ssize_t store_master_fader_leds(struct device *dev,
 	return ret;
 }
 
+static int lp5523_multicolor_brightness(struct lp55xx_led *led)
+{
+	struct lp55xx_chip *chip = led->chip;
+	int ret;
+	int i;
+
+	mutex_lock(&chip->lock);
+	for (i = 0; i < led->mc_cdev.num_colors; i++) {
+		ret = lp55xx_write(chip,
+				   LP5523_REG_LED_PWM_BASE +
+				   led->mc_cdev.subled_info[i].channel,
+				   led->mc_cdev.subled_info[i].brightness);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
 static int lp5523_led_brightness(struct lp55xx_led *led)
 {
 	struct lp55xx_chip *chip = led->chip;
@@ -858,6 +877,7 @@ static struct lp55xx_device_config lp5523_cfg = {
 	.max_channel  = LP5523_MAX_LEDS,
 	.post_init_device   = lp5523_post_init_device,
 	.brightness_fn      = lp5523_led_brightness,
+	.multicolor_brightness_fn = lp5523_multicolor_brightness,
 	.set_led_current    = lp5523_set_led_current,
 	.firmware_cb        = lp5523_firmware_loaded,
 	.run_engine         = lp5523_run_engine,
Signature · 1 line
2.27.0
author
#20 lore raw

PATCH v31 9/12 leds: lp5521: Add multicolor framework multicolor brightness support

In reply to Dan Murphy

Add the multicolor brightness call back to support the multicolor framework. This function allows setting the brightness across grouped LED channels in a single call.

---
 drivers/leds/leds-lp5521.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/leds/leds-lp5521.c +20 -0
index 6ff81d6be789..ef8c3bfa8f3c 100644
@@ -349,6 +349,25 @@ static int lp5521_run_selftest(struct lp55xx_chip *chip, char *buf)
 	return 0;
 }
 
+static int lp5521_multicolor_brightness(struct lp55xx_led *led)
+{
+	struct lp55xx_chip *chip = led->chip;
+	int ret;
+	int i;
+
+	mutex_lock(&chip->lock);
+	for (i = 0; i < led->mc_cdev.num_colors; i++) {
+		ret = lp55xx_write(chip,
+				   LP5521_REG_LED_PWM_BASE +
+				   led->mc_cdev.subled_info[i].channel,
+				   led->mc_cdev.subled_info[i].brightness);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
 static int lp5521_led_brightness(struct lp55xx_led *led)
 {
 	struct lp55xx_chip *chip = led->chip;
@@ -490,6 +509,7 @@ static struct lp55xx_device_config lp5521_cfg = {
 	.max_channel  = LP5521_MAX_LEDS,
 	.post_init_device   = lp5521_post_init_device,
 	.brightness_fn      = lp5521_led_brightness,
+	.multicolor_brightness_fn = lp5521_multicolor_brightness,
 	.set_led_current    = lp5521_set_led_current,
 	.firmware_cb        = lp5521_firmware_loaded,
 	.run_engine         = lp5521_run_engine,
Signature · 1 line
2.27.0
author
#21 lore raw

PATCH v31 10/12 ARM: dts: n900: Add reg property to the LP5523 channel node

In reply to Dan Murphy

Add the reg property to each channel node. This update is to accommodate the multicolor framework. In addition to the accommodation this allows the LEDs to be placed on any channel and allow designs to skip channels as opposed to requiring sequential order.

---
 arch/arm/boot/dts/omap3-n900.dts | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)
arch/arm/boot/dts/omap3-n900.dts +20 -9
index 4089d97405c9..ebe93b06b4f7 100644
@@ -618,63 +618,74 @@ indicator {
 	};
 
 	lp5523: lp5523@32 {
+		#address-cells = <1>;
+		#size-cells = <0>;
 		compatible = "national,lp5523";
 		reg = <0x32>;
 		clock-mode = /bits/ 8 <0>; /* LP55XX_CLOCK_AUTO */
 		enable-gpio = <&gpio2 9 GPIO_ACTIVE_HIGH>; /* 41 */
 
-		chan0 {
+		chan@0 {
 			chan-name = "lp5523:kb1";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <0>;
 		};
 
-		chan1 {
+		chan@1 {
 			chan-name = "lp5523:kb2";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <1>;
 		};
 
-		chan2 {
+		chan@2 {
 			chan-name = "lp5523:kb3";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <2>;
 		};
 
-		chan3 {
+		chan@3 {
 			chan-name = "lp5523:kb4";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <3>;
 		};
 
-		chan4 {
+		chan@4 {
 			chan-name = "lp5523:b";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <4>;
 		};
 
-		chan5 {
+		chan@5 {
 			chan-name = "lp5523:g";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <5>;
 		};
 
-		chan6 {
+		chan@6 {
 			chan-name = "lp5523:r";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <6>;
 		};
 
-		chan7 {
+		chan@7 {
 			chan-name = "lp5523:kb5";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <7>;
 		};
 
-		chan8 {
+		chan@8 {
 			chan-name = "lp5523:kb6";
 			led-cur = /bits/ 8 <50>;
 			max-cur = /bits/ 8 <100>;
+			reg = <8>;
 		};
 	};
 
Signature · 1 line
2.27.0
author
#22 lore raw

PATCH v31 11/12 ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 channel node

In reply to Dan Murphy

Add the reg property to each channel node. This update is to accommodate the multicolor framework. In addition to the accommodation this allows the LEDs to be placed on any channel and allow designs to skip channels as opposed to requiring sequential order.

CC Shawn Guo <[email protected]>
CC Sascha Hauer <[email protected]>
CC Pengutronix Kernel Team <[email protected]>
CC Fabio Estevam <[email protected]>
CC NXP Linux Team <[email protected]>
---
 arch/arm/boot/dts/imx6dl-yapp4-common.dtsi | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi +10 -4
index 2b9423d55c37..77aa90640052 100644
@@ -258,29 +258,35 @@ leds: led-controller@30 {
 		reg = <0x30>;
 		clock-mode = /bits/ 8 <1>;
 		status = "disabled";
+		#address-cells = <1>;
+		#size-cells = <0>;
 
-		chan0 {
+		chan@0 {
 			chan-name = "R";
 			led-cur = /bits/ 8 <0x20>;
 			max-cur = /bits/ 8 <0x60>;
+			reg = <0>;
 		};
 
-		chan1 {
+		chan@1 {
 			chan-name = "G";
 			led-cur = /bits/ 8 <0x20>;
 			max-cur = /bits/ 8 <0x60>;
+			reg = <1>;
 		};
 
-		chan2 {
+		chan@2 {
 			chan-name = "B";
 			led-cur = /bits/ 8 <0x20>;
 			max-cur = /bits/ 8 <0x60>;
+			reg = <2>;
 		};
 
-		chan3 {
+		chan@3 {
 			chan-name = "W";
 			led-cur = /bits/ 8 <0x0>;
 			max-cur = /bits/ 8 <0x0>;
+			reg = <3>;
 		};
 	};
 
Signature · 1 line
2.27.0
author
#23 lore raw

PATCH v31 12/12 ARM: dts: ste-href: Add reg property to the LP5521 channel nodes

In reply to Dan Murphy

Add the reg property to each channel node. This update is to accommodate the multicolor framework. In addition to the accommodation this allows the LEDs to be placed on any channel and allow designs to skip channels as opposed to requiring sequential order.

---
 arch/arm/boot/dts/ste-href.dtsi | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
arch/arm/boot/dts/ste-href.dtsi +16 -6
index 33e3b0b3c53d..ff47cbf6ed3b 100644
@@ -58,16 +58,21 @@ lp5521@33 {
 				reg = <0x33>;
 				label = "lp5521_pri";
 				clock-mode = /bits/ 8 <2>;
-				chan0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				chan@0 {
+					reg = <0>;
 					led-cur = /bits/ 8 <0x2f>;
 					max-cur = /bits/ 8 <0x5f>;
 					linux,default-trigger = "heartbeat";
 				};
-				chan1 {
+				chan@1 {
+					reg = <1>;
 					led-cur = /bits/ 8 <0x2f>;
 					max-cur = /bits/ 8 <0x5f>;
 				};
-				chan2 {
+				chan@2 {
+					reg = <2>;
 					led-cur = /bits/ 8 <0x2f>;
 					max-cur = /bits/ 8 <0x5f>;
 				};
@@ -77,15 +82,20 @@ lp5521@34 {
 				reg = <0x34>;
 				label = "lp5521_sec";
 				clock-mode = /bits/ 8 <2>;
-				chan0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				chan@0 {
+					reg = <0>;
 					led-cur = /bits/ 8 <0x2f>;
 					max-cur = /bits/ 8 <0x5f>;
 				};
-				chan1 {
+				chan@1 {
+					reg = <1>;
 					led-cur = /bits/ 8 <0x2f>;
 					max-cur = /bits/ 8 <0x5f>;
 				};
-				chan2 {
+				chan@2 {
+					reg = <2>;
 					led-cur = /bits/ 8 <0x2f>;
 					max-cur = /bits/ 8 <0x5f>;
 				};
Signature · 1 line
2.27.0