Multicolor Framework v31
Subsystems (5)
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
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
@@ -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
@@ -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
@@ -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
@@ -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
Pavel Machek
AppliedRe: [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.
Acked-by Jacek Anaszewski <[email protected]>Signed-off-by Dan Murphy <[email protected]>
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
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.
Acked-by Jacek Anaszewski <[email protected]>Signed-off-by Dan Murphy <[email protected]>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
PATCH v31 2/12 dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
In reply to Dan MurphyIntroduce 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
@@ -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
PATCH v31 3/12 leds: lp50xx: Add the LP50XX family of the RGB LED driver
In reply to Dan MurphyIntroduce 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
@@ -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
@@ -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
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
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_putso 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
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
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
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
Re: [PATCH v31 03/12] leds: lp50xx: Add the LP50XX family of the RGB LED driver
In reply to Dan MurphyQuoted 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 LEDsThe 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.
Acked-by Jacek Anaszewski <[email protected]>Signed-off-by Dan Murphy <[email protected]>
Quoted 9 lines
+/*
+ * structlp50xx_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
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 LEDsThe 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.
Acked-by Jacek Anaszewski <[email protected]>Signed-off-by Dan Murphy <[email protected]>Quoted 9 lines
+/*
+ * structlp50xx_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
PATCH v31 4/12 dt-bindings: leds: Convert leds-lp55xx to yaml
In reply to Dan MurphyConvert 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
@@ -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
PATCH v31 5/12 leds: lp55xx: Convert LED class registration to devm_*
In reply to Dan MurphyConvert 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH v31 6/12 leds: lp55xx: Add multicolor framework support to lp55xx
In reply to Dan MurphyAdd 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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.
Acked-by Pavel Machek <[email protected]>Acked-by Jacek Anaszewski <[email protected]>Signed-off-by Dan Murphy <[email protected]>
Applied 4,5,6 and 8,9.
drivers/leds/Kconfig config LEDS_LP50XX · line 403
Quoted 7 lines
config
LEDS_LP55XX_COMMONdrivers/leds/Kconfig +2
-1tristate "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
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.
Acked-by Pavel Machek <[email protected]>Acked-by Jacek Anaszewski <[email protected]>Signed-off-by Dan Murphy <[email protected]>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_HELPERBut I have to ask: what does this do to userland interface once
LEDS_CLASS_MULTICOLORis 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
PATCH v31 7/12 ARM: defconfig: u8500: Add LP55XX_COMMON config flag
In reply to Dan MurphyThe 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
@@ -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
PATCH v31 8/12 leds: lp5523: Update the lp5523 code to add multicolor brightness function
In reply to Dan MurphyAdd 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
@@ -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
PATCH v31 9/12 leds: lp5521: Add multicolor framework multicolor brightness support
In reply to Dan MurphyAdd 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
@@ -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
PATCH v31 10/12 ARM: dts: n900: Add reg property to the LP5523 channel node
In reply to Dan MurphyAdd 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
@@ -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
PATCH v31 11/12 ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 channel node
In reply to Dan MurphyAdd 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/imx6dl-yapp4-common.dtsi | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi +10 -4
@@ -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
PATCH v31 12/12 ARM: dts: ste-href: Add reg property to the LP5521 channel nodes
In reply to Dan MurphyAdd 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
@@ -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