* [RFC PATCH 0/5] MultiColor LED framework Documentation @ 2019-04-01 17:33 Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 1/5] leds: multicolor: Add sysfs interface definition Dan Murphy ` (4 more replies) 0 siblings, 5 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-01 17:33 UTC (permalink / raw) To: robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds, Dan Murphy I am RFCing these patches to start this Multicolor framework as it appears we have a few devices that need to leverage a framework like this. I have attempted to create as much documentation as possible and have no issue in combining documents into a single document but I think this patchset is logical. I have also added the dt-bindings patch that include the COLOR_ID and COLOR_NAME and documented the existing patch on the mail list. I have added this only for demonstration purposes. Finally the code itself is a work in progress that at the very least will create a colors directory as well as the associated colors. The code works in principle but I need to scrub it for issues. The framework code also does not contain the available brightness support as this seems to be a sticky point. Dan Dan Murphy (5): leds: multicolor: Add sysfs interface definition dt: bindings: Add multicolor class dt bindings documention documention: leds: Add multicolor class documentation dt-bindings: leds: Add LED_COLOR_ID and COLOR_NAME definitions leds: multicolor: Introduce a multicolor class definition .../ABI/testing/sysfs-class-led-multicolor | 91 ++++ .../bindings/leds/leds-class-multicolor.txt | 140 ++++++ Documentation/leds/leds-class-multicolor.txt | 99 +++++ drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 411 ++++++++++++++++++ include/dt-bindings/leds/common.h | 19 + include/linux/led-class-multicolor.h | 69 +++ 8 files changed, 840 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt create mode 100644 Documentation/leds/leds-class-multicolor.txt create mode 100644 drivers/leds/led-class-multicolor.c create mode 100644 include/linux/led-class-multicolor.h -- 2.19.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 1/5] leds: multicolor: Add sysfs interface definition 2019-04-01 17:33 [RFC PATCH 0/5] MultiColor LED framework Documentation Dan Murphy @ 2019-04-01 17:33 ` Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention Dan Murphy ` (3 subsequent siblings) 4 siblings, 0 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-01 17:33 UTC (permalink / raw) To: robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds, Dan Murphy Add a documentation of LED Multicolor LED class specific sysfs attributes. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- .../ABI/testing/sysfs-class-led-multicolor | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor new file mode 100644 index 000000000000..5583ecaa3170 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor @@ -0,0 +1,91 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <dmurphy@ti.com> +Description: read/write + Writing a 1 to this file will enable the synchronization of all + the defined color LEDs within the LED node. Brightness values + for each LED will be stored and written when sync is set to 1. + Writing a 0 to this file will disable syncing and allow + individual control of the LEDs brightness settings. + +What: /sys/class/leds/<led>/colors/sync +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <dmurphy@ti.com> +Description: write only + Writing a 1 to this file while sync_enable is set to 1 will + write the current brightness values to all defined LEDs within + the LED node. All LEDs defined will be configured based + on the brightness that has been requested. + + If sync_enable is set to 0 then writing a 1 to sync has no + affect on the LEDs. + +What: /sys/class/leds/<led>/colors/<led_color>/brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <dmurphy@ti.com> +Description: read/write + The led_color directory is dynamically created based on the + colors defined by the registrar of the class. + The led_color can be but not limited to red, green, blue, + white, amber, yellow and violet. Drivers can also declare a + LED color for presentation. There is one directory per color + presented. The brightness file is created under each + led_color directory and controls the individual LED color + setting. + + If sync is enabled then writing the brightness value of the LED + is deferred until a 1 is written to + /sys/class/leds/<led>/color/sync. If syncing is + disabled then the LED brightness value will be written + immediately to the LED driver. + + The value of the color is from 0 to + /sys/class/leds/<led>/colors/<led_color>/max_brightness. + +What: /sys/class/leds/<led>/colors/<led_color>/max_brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <dmurphy@ti.com> +Description: read only + Maximum brightness level for the LED color, default is + 255 (LED_FULL). + + If the LED does not support different brightness levels, this + should be 1. + +What: /sys/class/leds/<led>/brightness_model/<model_color> +Date: April 2019 +KernelVersion: 5.2 +Contact: Jacek Anaszewski <j.anaszewski@samsung.com> +Description: read/write + This directory contains the defined color models that are read + from the firmware node. The model color will be the read from + the firmware node. The model names should be a color that the + LED cluster can produce. + +What: /sys/class/leds/<led>/brightness_model/<model_color>/brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Jacek Anaszewski <j.anaszewski@samsung.com> +Description: read/write + Writing this file will initiate a synchronized write to the LED + cluster to produce the color output defined in the firmware node. + + This value cannot exceed <model_color>/max_brightness. + Writing a 0 to this file will cause the LED cluster to turn off. + + The sync and sync_enable files do not have an affect on this as + this is a real time update of the LED cluster. + +What: /sys/class/leds/<led>/brightness_model/<model_color>/max_brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Jacek Anaszewski <j.anaszewski@samsung.com> +Description: read + Reading this file will return the maximum number of brightness + levels that are defined in the firmware node for the model color. + It should be assumed that the levels are from 1 -> max_brightness + and the LED cluster brightness is increasing. -- 2.19.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-01 17:33 [RFC PATCH 0/5] MultiColor LED framework Documentation Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 1/5] leds: multicolor: Add sysfs interface definition Dan Murphy @ 2019-04-01 17:33 ` Dan Murphy 2019-04-01 21:29 ` Pavel Machek 2019-04-01 17:33 ` [RFC PATCH 3/5] documention: leds: Add multicolor class documentation Dan Murphy ` (2 subsequent siblings) 4 siblings, 1 reply; 21+ messages in thread From: Dan Murphy @ 2019-04-01 17:33 UTC (permalink / raw) To: robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds, Dan Murphy Add DT bindings for the LEDs multicolor class framework. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- .../bindings/leds/leds-class-multicolor.txt | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file mode 100644 index 000000000000..4b1a26104c79 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt @@ -0,0 +1,140 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : This is the color ID of the LED. Definitions can be found + in include/linux/leds/common.txt + +Optional LED Child properties: + - available-brightness-models : This is the phandle to the brightness-model + node(s) that this LED cluster can support. + +Required Brightness model properties + - led-brightness-model : This flag alerts the device driver and class + code that this node is a brightness model node + and to process the properties differently. + +Required Brightness model child properties + - model_name : This is the name of the model presented to the user. This + should be a color that the LED cluster can produce for + the device it is attached to. + - layout : This is the LED layout for the levels. This layout will + determine the color order of the levels. The layout and + level-x properties array should be the same size. + - level-x : These are the values for the LEDs to produce the color that + is defined. These values are placed in the array according + to the layout property. + +led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + lp5024_model_yellow: brightness-models { + led-brightness-model; + model@0 { + model_name = "yellow"; + layout = <LED_COLOR_ID_RED + LED_COLOR_ID_GREEN + LED_COLOR_ID_BLUE>; + level-1 = <255 227 40>; + level-2 = <255 240 136>; + level-3 = <255 247 196>; + }; + }; + + lp5024_model_orange: brightness-models { + led-brightness-model; + model@1 { + model_name = "orange"; + layout = <LED_COLOR_ID_RED + LED_COLOR_ID_GREEN + LED_COLOR_ID_BLUE>; + level-1 = <236 140 16>; + level-2 = <236 157 55>; + level-3 = <236 183 115>; + }; + }; + + multi-led@4 { + #address-cells = <1>; + #size-cells = <0>; + reg = <4>; + label = "rgb:led_mod4"; + available-brightness-models = <&lp5024_model_yellow>; + + led@12 { + reg = <12>; + color = <LED_COLOR_ID_RED>; + }; + + led@13 { + reg = <13>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@14 { + reg = <14>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + /* Only support RGB no model defined */ + multi-led@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + label = "rgb:led_mod1"; + + led@3 { + reg = <3>; + color = <LED_COLOR_ID_RED>; + }; + + led@4 { + reg = <4>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@5 { + reg = <5>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + multi-led@2 { + #address-cells = <1>; + #size-cells = <0>; + label = "rgb:banks"; + reg = <2>; + ti,led-bank = <2 3 5>; + available-brightness-models = <&lp5024_model_orange>; + + led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RED>; + led-sources = <6 9 15>; + }; + + led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_GREEN>; + led-sources = <7 10 16>; + }; + + led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_BLUE>; + led-sources = <8 11 17>; + }; + }; + +}; -- 2.19.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-01 17:33 ` [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention Dan Murphy @ 2019-04-01 21:29 ` Pavel Machek 2019-04-02 11:40 ` Dan Murphy 2019-04-02 19:51 ` Jacek Anaszewski 0 siblings, 2 replies; 21+ messages in thread From: Pavel Machek @ 2019-04-01 21:29 UTC (permalink / raw) To: Dan Murphy Cc: robh+dt, jacek.anaszewski, marek.behun, linux-kernel, linux-leds [-- Attachment #1: Type: text/plain, Size: 2937 bytes --] Hi! > .../bindings/leds/leds-class-multicolor.txt | 140 ++++++++++++++++++ > 1 file changed, 140 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt > new file mode 100644 > index 000000000000..4b1a26104c79 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt > @@ -0,0 +1,140 @@ > +* Multicolor LED properties > + > +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices > +can be grouped together and also provide a modeling mechanism so that the > +cluster LEDs can vary in hue and intensity to produce a wide range of colors. > + > +The nodes and properties defined in this document are unique to the multicolor > +LED class. Common LED nodes and properties are inherited from the common.txt > +within this documentation directory. > + > +Required LED Child properties: > + - color : This is the color ID of the LED. Definitions can be found > + in include/linux/leds/common.txt > + > +Optional LED Child properties: > + - available-brightness-models : This is the phandle to the brightness-model > + node(s) that this LED cluster can support. > + > +Required Brightness model properties > + - led-brightness-model : This flag alerts the device driver and class > + code that this node is a brightness model node > + and to process the properties differently. > + > +Required Brightness model child properties > + - model_name : This is the name of the model presented to the user. This > + should be a color that the LED cluster can produce for > + the device it is attached to. > + - layout : This is the LED layout for the levels. This layout will > + determine the color order of the levels. The layout and > + level-x properties array should be the same size. > + - level-x : These are the values for the LEDs to produce the color that > + is defined. These values are placed in the array according > + to the layout property. > +led-controller@30 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,lp5024"; > + reg = <0x29>; > + > + lp5024_model_yellow: brightness-models { > + led-brightness-model; > + model@0 { > + model_name = "yellow"; > + layout = <LED_COLOR_ID_RED > + LED_COLOR_ID_GREEN > + LED_COLOR_ID_BLUE>; > + level-1 = <255 227 40>; > + level-2 = <255 240 136>; > + level-3 = <255 247 196>; > + }; > + }; I don't think this works. RGB LED can show millions of colors. Do you propose to have millions of entries in dts? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-01 21:29 ` Pavel Machek @ 2019-04-02 11:40 ` Dan Murphy 2019-04-02 16:17 ` Marek Behun 2019-04-02 20:40 ` Jacek Anaszewski 2019-04-02 19:51 ` Jacek Anaszewski 1 sibling, 2 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-02 11:40 UTC (permalink / raw) To: Pavel Machek Cc: robh+dt, jacek.anaszewski, marek.behun, linux-kernel, linux-leds Pavel On 4/1/19 4:29 PM, Pavel Machek wrote: > Hi! > > >> .../bindings/leds/leds-class-multicolor.txt | 140 ++++++++++++++++++ >> 1 file changed, 140 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >> new file mode 100644 >> index 000000000000..4b1a26104c79 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >> @@ -0,0 +1,140 @@ >> +* Multicolor LED properties >> + >> +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices >> +can be grouped together and also provide a modeling mechanism so that the >> +cluster LEDs can vary in hue and intensity to produce a wide range of colors. >> + >> +The nodes and properties defined in this document are unique to the multicolor >> +LED class. Common LED nodes and properties are inherited from the common.txt >> +within this documentation directory. >> + >> +Required LED Child properties: >> + - color : This is the color ID of the LED. Definitions can be found >> + in include/linux/leds/common.txt >> + >> +Optional LED Child properties: >> + - available-brightness-models : This is the phandle to the brightness-model >> + node(s) that this LED cluster can support. >> + >> +Required Brightness model properties >> + - led-brightness-model : This flag alerts the device driver and class >> + code that this node is a brightness model node >> + and to process the properties differently. >> + >> +Required Brightness model child properties >> + - model_name : This is the name of the model presented to the user. This >> + should be a color that the LED cluster can produce for >> + the device it is attached to. >> + - layout : This is the LED layout for the levels. This layout will >> + determine the color order of the levels. The layout and >> + level-x properties array should be the same size. >> + - level-x : These are the values for the LEDs to produce the color that >> + is defined. These values are placed in the array according >> + to the layout property. > >> +led-controller@30 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "ti,lp5024"; >> + reg = <0x29>; >> + >> + lp5024_model_yellow: brightness-models { >> + led-brightness-model; >> + model@0 { >> + model_name = "yellow"; >> + layout = <LED_COLOR_ID_RED >> + LED_COLOR_ID_GREEN >> + LED_COLOR_ID_BLUE>; >> + level-1 = <255 227 40>; >> + level-2 = <255 240 136>; >> + level-3 = <255 247 196>; >> + }; >> + }; > > I don't think this works. RGB LED can show millions of colors. Do you > propose to have millions of entries in dts? > Pavel > I have had off line conversations with Jacek about this brightness model node. Your concern was actually one of my concerns as well. Not only millions of entries but also having a huge DT binary. We wanted to RFC this to get feedback. And this is why I have not added any support for this in the framework code. Dan -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-02 11:40 ` Dan Murphy @ 2019-04-02 16:17 ` Marek Behun 2019-04-02 17:06 ` Dan Murphy 2019-04-02 20:40 ` Jacek Anaszewski 1 sibling, 1 reply; 21+ messages in thread From: Marek Behun @ 2019-04-02 16:17 UTC (permalink / raw) To: Dan Murphy Cc: Pavel Machek, robh+dt, jacek.anaszewski, linux-kernel, linux-leds On Tue, 2 Apr 2019 06:40:53 -0500 Dan Murphy <dmurphy@ti.com> wrote: > I have had off line conversations with Jacek about this brightness model node. > > Your concern was actually one of my concerns as well. Not only millions of entries but also having a huge > DT binary. > > We wanted to RFC this to get feedback. And this is why I have not added any support for this in the framework code. > > Dan > What if we just stuck to color spaces and defined channel curves in the device tree? The curve could then be defined via an array of integer pairs, which would be interpreted as points via which the curve passes and the curve would be approximated by linear segemts... led@0 { colorspace = <COLOR_SPACE_ID_RGB>; red-curve = <0 0>, <255 255>; green-curve = <0 0>, <255 128>; blue-curve = <0 0>, <255 128>; }; ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-02 16:17 ` Marek Behun @ 2019-04-02 17:06 ` Dan Murphy 0 siblings, 0 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-02 17:06 UTC (permalink / raw) To: Marek Behun Cc: Pavel Machek, robh+dt, jacek.anaszewski, linux-kernel, linux-leds, Vesa Jääskeläinen Marek On 4/2/19 11:17 AM, Marek Behun wrote: > On Tue, 2 Apr 2019 06:40:53 -0500 > Dan Murphy <dmurphy@ti.com> wrote: > >> I have had off line conversations with Jacek about this brightness model node. >> >> Your concern was actually one of my concerns as well. Not only millions of entries but also having a huge >> DT binary. >> >> We wanted to RFC this to get feedback. And this is why I have not added any support for this in the framework code. >> >> Dan >> > > What if we just stuck to color spaces and defined channel curves in the > device tree? > Vesa did a good summary of the current MC framework implementations that are being proposed The thread also documents Pavel's requirements https://lore.kernel.org/patchwork/patch/1037147/ And here is the series with a single brightness file and passing in multiple integers. I did not document this code as I was pretty certain it was not the way to go. https://patches.linaro.org/project/linux-leds/list/?series=18022 Dan > The curve could then be defined via an array of integer pairs, which > would be interpreted as points via which the curve passes and the curve > would be approximated by linear segemts... > > led@0 { > colorspace = <COLOR_SPACE_ID_RGB>; > red-curve = <0 0>, <255 255>; > green-curve = <0 0>, <255 128>; > blue-curve = <0 0>, <255 128>; > }; > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-02 11:40 ` Dan Murphy 2019-04-02 16:17 ` Marek Behun @ 2019-04-02 20:40 ` Jacek Anaszewski 1 sibling, 0 replies; 21+ messages in thread From: Jacek Anaszewski @ 2019-04-02 20:40 UTC (permalink / raw) To: Dan Murphy, Pavel Machek, devicetree Cc: robh+dt, marek.behun, linux-kernel, linux-leds On 4/2/19 1:40 PM, Dan Murphy wrote: > Pavel > > On 4/1/19 4:29 PM, Pavel Machek wrote: >> Hi! >> >> >>> .../bindings/leds/leds-class-multicolor.txt | 140 ++++++++++++++++++ >>> 1 file changed, 140 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >>> >>> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >>> new file mode 100644 >>> index 000000000000..4b1a26104c79 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >>> @@ -0,0 +1,140 @@ >>> +* Multicolor LED properties >>> + >>> +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices >>> +can be grouped together and also provide a modeling mechanism so that the >>> +cluster LEDs can vary in hue and intensity to produce a wide range of colors. >>> + >>> +The nodes and properties defined in this document are unique to the multicolor >>> +LED class. Common LED nodes and properties are inherited from the common.txt >>> +within this documentation directory. >>> + >>> +Required LED Child properties: >>> + - color : This is the color ID of the LED. Definitions can be found >>> + in include/linux/leds/common.txt >>> + >>> +Optional LED Child properties: >>> + - available-brightness-models : This is the phandle to the brightness-model >>> + node(s) that this LED cluster can support. >>> + >>> +Required Brightness model properties >>> + - led-brightness-model : This flag alerts the device driver and class >>> + code that this node is a brightness model node >>> + and to process the properties differently. >>> + >>> +Required Brightness model child properties >>> + - model_name : This is the name of the model presented to the user. This >>> + should be a color that the LED cluster can produce for >>> + the device it is attached to. >>> + - layout : This is the LED layout for the levels. This layout will >>> + determine the color order of the levels. The layout and >>> + level-x properties array should be the same size. >>> + - level-x : These are the values for the LEDs to produce the color that >>> + is defined. These values are placed in the array according >>> + to the layout property. >> >>> +led-controller@30 { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + compatible = "ti,lp5024"; >>> + reg = <0x29>; >>> + >>> + lp5024_model_yellow: brightness-models { >>> + led-brightness-model; >>> + model@0 { >>> + model_name = "yellow"; >>> + layout = <LED_COLOR_ID_RED >>> + LED_COLOR_ID_GREEN >>> + LED_COLOR_ID_BLUE>; >>> + level-1 = <255 227 40>; >>> + level-2 = <255 240 136>; >>> + level-3 = <255 247 196>; >>> + }; >>> + }; >> >> I don't think this works. RGB LED can show millions of colors. Do you >> propose to have millions of entries in dts? >> Pavel >> > > I have had off line conversations with Jacek about this brightness model node. > > Your concern was actually one of my concerns as well. Not only millions of entries but also having a huge > DT binary. > > We wanted to RFC this to get feedback. And this is why I have not added any support for this in the framework code. We all know that nobody sane is going to add millions of color entries. It is just a holistic solution to the problem, without the need to worry about different colors being produced by different LED elements, either due to their inherent properties, due to different parameters of LED controllers and board designs, or due to aging. Of course the last case will be possible to fix only by update of DTB blob. Regarding the possible size of DTB blob, I made some calculations and even compiled sample dts. Here is my reasoning from one of the replies to Dan during our private discussion: ------------------------------------------------------------------------- Brightness models will be reusable between modules. There could be arbitrary number of brightness models, limited only by maximum dtb blob size. In uboot it can be configured with "setenv fdt_high". For the dtbs appended to zImage for ARM the limit is 2MB AFAIK. Brightness models will be what consumes the most of memory with this proposed DT design. Let's calculate. 4 bytes for single brightness value (we allow for values > 255). I think quite exaggerated exemplary quantities: number of colors in the mcled_dev: 10 number of levels per model: 256 number of models: 100 bytes required = 4 * 10 * 256 * 100 = 1024000, i.e. less than 1MB And average mainline dtbs usually don't exceed 100kB, so I think we are safe. I've even just tried it out by adding such 10 models of 255 levels * 10 values * 4-byte to versatile-pb.dts and I got dtb size ~150kB. But to be sure it would be good to get early ack from DT guys. ------------------------------------------------------------------------- And now is this moment - we'd like to hear the feedback from DT guys. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention 2019-04-01 21:29 ` Pavel Machek 2019-04-02 11:40 ` Dan Murphy @ 2019-04-02 19:51 ` Jacek Anaszewski 1 sibling, 0 replies; 21+ messages in thread From: Jacek Anaszewski @ 2019-04-02 19:51 UTC (permalink / raw) To: Pavel Machek, Dan Murphy Cc: robh+dt, marek.behun, linux-kernel, linux-leds, Vesa Jääskeläinen On 4/1/19 11:29 PM, Pavel Machek wrote: > Hi! > > >> .../bindings/leds/leds-class-multicolor.txt | 140 ++++++++++++++++++ >> 1 file changed, 140 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >> new file mode 100644 >> index 000000000000..4b1a26104c79 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt >> @@ -0,0 +1,140 @@ >> +* Multicolor LED properties >> + >> +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices >> +can be grouped together and also provide a modeling mechanism so that the >> +cluster LEDs can vary in hue and intensity to produce a wide range of colors. >> + >> +The nodes and properties defined in this document are unique to the multicolor >> +LED class. Common LED nodes and properties are inherited from the common.txt >> +within this documentation directory. >> + >> +Required LED Child properties: >> + - color : This is the color ID of the LED. Definitions can be found >> + in include/linux/leds/common.txt >> + >> +Optional LED Child properties: >> + - available-brightness-models : This is the phandle to the brightness-model >> + node(s) that this LED cluster can support. >> + >> +Required Brightness model properties >> + - led-brightness-model : This flag alerts the device driver and class >> + code that this node is a brightness model node >> + and to process the properties differently. >> + >> +Required Brightness model child properties >> + - model_name : This is the name of the model presented to the user. This >> + should be a color that the LED cluster can produce for >> + the device it is attached to. >> + - layout : This is the LED layout for the levels. This layout will >> + determine the color order of the levels. The layout and >> + level-x properties array should be the same size. >> + - level-x : These are the values for the LEDs to produce the color that >> + is defined. These values are placed in the array according >> + to the layout property. > >> +led-controller@30 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "ti,lp5024"; >> + reg = <0x29>; >> + >> + lp5024_model_yellow: brightness-models { >> + led-brightness-model; >> + model@0 { >> + model_name = "yellow"; >> + layout = <LED_COLOR_ID_RED >> + LED_COLOR_ID_GREEN >> + LED_COLOR_ID_BLUE>; >> + level-1 = <255 227 40>; >> + level-2 = <255 240 136>; >> + level-3 = <255 247 196>; >> + }; >> + }; > > I don't think this works. RGB LED can show millions of colors. Do you > propose to have millions of entries in dts? Pavel, you know very well what is going on here and what is the background of this design. I suppose you've just entered your trolling mode - you lately admitted that it sometimes happens :-) [0] This aims at solving the problem with multi color LED support in a backward compatible way, which is especially important in relation to existing triggers. This is our attempt of addressing that problem in a color space agnostic way. It was inspired by out-of-tree approach presented by Vesa few months ago. [0] https://www.spinics.net/lists/linux-leds/msg11388.html -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 3/5] documention: leds: Add multicolor class documentation 2019-04-01 17:33 [RFC PATCH 0/5] MultiColor LED framework Documentation Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 1/5] leds: multicolor: Add sysfs interface definition Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention Dan Murphy @ 2019-04-01 17:33 ` Dan Murphy 2019-04-01 21:18 ` Randy Dunlap 2019-04-01 21:27 ` Pavel Machek 2019-04-01 17:33 ` [RFC PATCH 4/5] dt-bindings: leds: Add LED_COLOR_ID and COLOR_NAME definitions Dan Murphy 2019-04-01 17:34 ` [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition Dan Murphy 4 siblings, 2 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-01 17:33 UTC (permalink / raw) To: robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds, Dan Murphy Add the support documentation on the multicolor LED framework. This document defines the directores and file generated by the multicolor framework. It also documents usage. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- Documentation/leds/leds-class-multicolor.txt | 99 ++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 Documentation/leds/leds-class-multicolor.txt diff --git a/Documentation/leds/leds-class-multicolor.txt b/Documentation/leds/leds-class-multicolor.txt new file mode 100644 index 000000000000..8112b99a7668 --- /dev/null +++ b/Documentation/leds/leds-class-multicolor.txt @@ -0,0 +1,99 @@ + +Multi Color LED handling under Linux +================================================= + +Authors: Dan Murphy <dmurphy@ti.com> + +Description +----------- +There are varying monochrome LED colors available for application. These +LEDs can be used as a single use case LED or can be mixed with other color +LEDs to produce the full spectrum of color. Color LEDs that are grouped +can be presented under a single LED node with individual color control. +The multicolor class groups these LEDs and allows dynamically setting the value +of a single LED or setting the brightness values of the LEDs in the group and +updating the LEDs virtually simultaneously. + +Multicolor Class Control +------------------------- +The multicolor class presents the LED groups under a directory called "colors". +This directory is a child under the LED parent node created but the led_class +framework. The led_class framework is documented in led-class.txt within this +documentation directory. + +Each colored LED is given it's own directory. These directories can be but not +limited to red, green, blue, white, amber, yellow and violet. Under these +directories the brightness and max_brightness files are presented for each LED. + +Under the "colors" directory there are two files created "sync" and +"sync_enable". The sync_enable file controls whether the LED brightness +value is set real time or if the LED brightness value setting is deferred until +the "sync" file is written. If sync_enable is set then writing to each LED +"brightness" file will store the brightness value. Once the "sync" file is +written then each LED color defined in the node will write the brightness of +the LED in the device driver. + +If "sync_enable" is not set then writing the brightness value of the LED to the +device driver is done immediately. Writing the "sync" file has no affect. + +Directory Layout Example +------------------------ +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/ +colors/: +drwxr-xr-x 2 root root 0 Jun 28 20:21 blue +drwxr-xr-x 2 root root 0 Jun 28 20:21 green +drwxr-xr-x 2 root root 0 Jun 28 20:21 red +--w------- 1 root root 4096 Jun 28 20:21 sync +-rw------- 1 root root 4096 Jun 28 20:22 sync_enable + +colors/blue: +-rw------- 1 root root 4096 Jun 28 20:21 brightness +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness + +colors/green: +-rw------- 1 root root 4096 Jun 28 20:22 brightness +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness + +colors/red: +-rw------- 1 root root 4096 Jun 28 20:21 brightness +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness + +Example of Writing LEDs with Sync Enabled +----------------------------------------- +Below the red, green and blue LEDs are set to corresponding values. These +values are stored and not written until the sync file is written. + +cd /sys/class/leds/rgb:grouped_leds/colors + +echo 1 > sync_enable + +echo 100 > red/brightness +echo 80 > green/brightness +echo 180 > blue/brightness + +* LED device driver has not been updated and the LED states have not changed. +* Writing the LED brightness files again will only change the stored value and +* not the device driver value. + +echo 1 > sync + +* LED device driver has been updated the LEDs should present the brightness +* levels that have been set. Since sync_enable is still enabled writing to the +* LED brightness files will not change the current brightnesses. + +Example of Writing LEDs with Sync Disabled +------------------------------------------ +Below the values of each LED are written to the device driver immediately upon +request. + +cd /sys/class/leds/rgb:grouped_leds/colors + +echo 0 > sync_enable + +echo 100 > red/brightness // Red LED should be on with the current brightness +echo 80 > green/brightness // Green LED should be on with the current brightness +echo 180 > blue/brightness // Blue LED should be on with the current brightness +. +. +. +echo 0 > green/brightness // Green LED should be off -- 2.19.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] documention: leds: Add multicolor class documentation 2019-04-01 17:33 ` [RFC PATCH 3/5] documention: leds: Add multicolor class documentation Dan Murphy @ 2019-04-01 21:18 ` Randy Dunlap 2019-04-01 21:27 ` Pavel Machek 1 sibling, 0 replies; 21+ messages in thread From: Randy Dunlap @ 2019-04-01 21:18 UTC (permalink / raw) To: Dan Murphy, robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds Hi, On 4/1/19 10:33 AM, Dan Murphy wrote: > Add the support documentation on the multicolor LED framework. > This document defines the directores and file generated by the directories and files > multicolor framework. It also documents usage. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > Documentation/leds/leds-class-multicolor.txt | 99 ++++++++++++++++++++ > 1 file changed, 99 insertions(+) > create mode 100644 Documentation/leds/leds-class-multicolor.txt > > diff --git a/Documentation/leds/leds-class-multicolor.txt b/Documentation/leds/leds-class-multicolor.txt > new file mode 100644 > index 000000000000..8112b99a7668 > --- /dev/null > +++ b/Documentation/leds/leds-class-multicolor.txt > @@ -0,0 +1,99 @@ > + > +Multi Color LED handling under Linux > +================================================= > + > +Authors: Dan Murphy <dmurphy@ti.com> or Author: > + > +Description > +----------- > +There are varying monochrome LED colors available for application. These > +LEDs can be used as a single use case LED or can be mixed with other color > +LEDs to produce the full spectrum of color. Color LEDs that are grouped > +can be presented under a single LED node with individual color control. > +The multicolor class groups these LEDs and allows dynamically setting the value > +of a single LED or setting the brightness values of the LEDs in the group and > +updating the LEDs virtually simultaneously. > + > +Multicolor Class Control > +------------------------- > +The multicolor class presents the LED groups under a directory called "colors". > +This directory is a child under the LED parent node created but the led_class created by > +framework. The led_class framework is documented in led-class.txt within this > +documentation directory. > + > +Each colored LED is given it's own directory. These directories can be but not its huh??????????? > +limited to red, green, blue, white, amber, yellow and violet. Under these > +directories the brightness and max_brightness files are presented for each LED. > + > +Under the "colors" directory there are two files created "sync" and created: > +"sync_enable". The sync_enable file controls whether the LED brightness > +value is set real time or if the LED brightness value setting is deferred until > +the "sync" file is written. If sync_enable is set then writing to each LED > +"brightness" file will store the brightness value. Once the "sync" file is > +written then each LED color defined in the node will write the brightness of > +the LED in the device driver. > + > +If "sync_enable" is not set then writing the brightness value of the LED to the > +device driver is done immediately. Writing the "sync" file has no affect. > + > +Directory Layout Example > +------------------------ > +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/ > +colors/: > +drwxr-xr-x 2 root root 0 Jun 28 20:21 blue > +drwxr-xr-x 2 root root 0 Jun 28 20:21 green > +drwxr-xr-x 2 root root 0 Jun 28 20:21 red > +--w------- 1 root root 4096 Jun 28 20:21 sync > +-rw------- 1 root root 4096 Jun 28 20:22 sync_enable > + > +colors/blue: > +-rw------- 1 root root 4096 Jun 28 20:21 brightness > +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness > + > +colors/green: > +-rw------- 1 root root 4096 Jun 28 20:22 brightness > +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness > + > +colors/red: > +-rw------- 1 root root 4096 Jun 28 20:21 brightness > +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness > + > +Example of Writing LEDs with Sync Enabled > +----------------------------------------- > +Below the red, green and blue LEDs are set to corresponding values. These > +values are stored and not written until the sync file is written. > + > +cd /sys/class/leds/rgb:grouped_leds/colors > + > +echo 1 > sync_enable > + > +echo 100 > red/brightness > +echo 80 > green/brightness > +echo 180 > blue/brightness > + > +* LED device driver has not been updated and the LED states have not changed. > +* Writing the LED brightness files again will only change the stored value and > +* not the device driver value. > + > +echo 1 > sync > + > +* LED device driver has been updated the LEDs should present the brightness updated; the LEDs > +* levels that have been set. Since sync_enable is still enabled writing to the > +* LED brightness files will not change the current brightnesses. > + > +Example of Writing LEDs with Sync Disabled > +------------------------------------------ > +Below the values of each LED are written to the device driver immediately upon > +request. > + > +cd /sys/class/leds/rgb:grouped_leds/colors > + > +echo 0 > sync_enable > + > +echo 100 > red/brightness // Red LED should be on with the current brightness > +echo 80 > green/brightness // Green LED should be on with the current brightness > +echo 180 > blue/brightness // Blue LED should be on with the current brightness > +. > +. > +. > +echo 0 > green/brightness // Green LED should be off > cheers. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] documention: leds: Add multicolor class documentation 2019-04-01 17:33 ` [RFC PATCH 3/5] documention: leds: Add multicolor class documentation Dan Murphy 2019-04-01 21:18 ` Randy Dunlap @ 2019-04-01 21:27 ` Pavel Machek 2019-04-02 5:55 ` Marek Behun 2019-04-02 11:53 ` Dan Murphy 1 sibling, 2 replies; 21+ messages in thread From: Pavel Machek @ 2019-04-01 21:27 UTC (permalink / raw) To: Dan Murphy Cc: robh+dt, jacek.anaszewski, marek.behun, linux-kernel, linux-leds [-- Attachment #1: Type: text/plain, Size: 1094 bytes --] Hi! > +Under the "colors" directory there are two files created "sync" and > +"sync_enable". The sync_enable file controls whether the LED brightness > +value is set real time or if the LED brightness value setting is deferred until > +the "sync" file is written. If sync_enable is set then writing to each LED > +"brightness" file will store the brightness value. Once the "sync" file is > +written then each LED color defined in the node will write the brightness of > +the LED in the device driver. > + > +If "sync_enable" is not set then writing the brightness value of the LED to the > +device driver is done immediately. Writing the "sync" file has no > affect. I believe better solutions exists for this problem. One was discussed before -- have single file which contains coefficients for r/g/b channels. Or at least... you should not really need separate sync and sync_enable files. One should do. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] documention: leds: Add multicolor class documentation 2019-04-01 21:27 ` Pavel Machek @ 2019-04-02 5:55 ` Marek Behun 2019-04-02 11:53 ` Dan Murphy 1 sibling, 0 replies; 21+ messages in thread From: Marek Behun @ 2019-04-02 5:55 UTC (permalink / raw) To: Pavel Machek Cc: Dan Murphy, robh+dt, jacek.anaszewski, linux-kernel, linux-leds On Mon, 1 Apr 2019 23:27:10 +0200 Pavel Machek <pavel@ucw.cz> wrote: > One was discussed before -- have single file which contains > coefficients for r/g/b channels. That would somehow break the rule one file/one value. Although you could consider the whole color one value, that way it would not be broken... ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] documention: leds: Add multicolor class documentation 2019-04-01 21:27 ` Pavel Machek 2019-04-02 5:55 ` Marek Behun @ 2019-04-02 11:53 ` Dan Murphy 2019-04-02 15:56 ` Marek Behun 1 sibling, 1 reply; 21+ messages in thread From: Dan Murphy @ 2019-04-02 11:53 UTC (permalink / raw) To: Pavel Machek Cc: robh+dt, jacek.anaszewski, marek.behun, linux-kernel, linux-leds Pavel Thanks for the comments On 4/1/19 4:27 PM, Pavel Machek wrote: > Hi! > >> +Under the "colors" directory there are two files created "sync" and >> +"sync_enable". The sync_enable file controls whether the LED brightness >> +value is set real time or if the LED brightness value setting is deferred until >> +the "sync" file is written. If sync_enable is set then writing to each LED >> +"brightness" file will store the brightness value. Once the "sync" file is >> +written then each LED color defined in the node will write the brightness of >> +the LED in the device driver. >> + >> +If "sync_enable" is not set then writing the brightness value of the LED to the >> +device driver is done immediately. Writing the "sync" file has no >> affect. > > I believe better solutions exists for this problem. > I agree there are other solutions. Better not so sure. We keep kicking a solution down the road we have been talking about adding something to the kernel for almost 5 months now, but no one can decide what to do. How do we move a solution forward? Should we just forget the whole idea and keep developing against the current framework? > One was discussed before -- have single file which contains > coefficients for r/g/b channels. > That has been presented as well and the concept was not well received either. > Or at least... you should not really need separate sync and > sync_enable files. One should do. > The idea here was to be able to set the LED brightness immediately on a single LED with a single brightness write or setup the color brightness on all the LEDs and then sync the LED brightnesses. If you wanted to control a single LED for notifications the user space may have to do echo 0 > blue/brightness echo 0 > green/brightness echo 255 > red/brightness echo 1 > sync As opposed to just echo 255 > red/brightness But if that is not a concern then removing the sync_enable is fine with me from a code standpoint but not from a user space side. Dan > Pavel > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] documention: leds: Add multicolor class documentation 2019-04-02 11:53 ` Dan Murphy @ 2019-04-02 15:56 ` Marek Behun 0 siblings, 0 replies; 21+ messages in thread From: Marek Behun @ 2019-04-02 15:56 UTC (permalink / raw) To: Dan Murphy Cc: Pavel Machek, robh+dt, jacek.anaszewski, linux-kernel, linux-leds Hi On Tue, 2 Apr 2019 06:53:30 -0500 Dan Murphy <dmurphy@ti.com> wrote: > I agree there are other solutions. Better not so sure. > We keep kicking a solution down the road we have been talking about adding something to the kernel > for almost 5 months now, but no one can decide what to do. > > How do we move a solution forward? > Should we just forget the whole idea and keep developing against the current framework? > > > One was discussed before -- have single file which contains > > coefficients for r/g/b channels. > > > > That has been presented as well and the concept was not well received either. > > > Or at least... you should not really need separate sync and > > sync_enable files. One should do. > > > > The idea here was to be able to set the LED brightness immediately on a single LED > with a single brightness write or setup the color brightness on all the LEDs and then > sync the LED brightnesses. > > If you wanted to control a single LED for notifications the user space may have to do > > echo 0 > blue/brightness > echo 0 > green/brightness > echo 255 > red/brightness > echo 1 > sync > > As opposed to just > echo 255 > red/brightness > Maybe other kernel developers, or sysfs maintainers like Greg, schould also have a word about this. Another solution, although not usable from shell, could be this: if a process opens all brightness files of a singled multicolor LED and then does a special SYNC_ON_THIS ioctl on one of them, then the sync is done when this color is written. But that would also require ioctl for sysfs files, and I don't think Greg would like that. Dan's sync_enable API looks right to me from the one file/one value sysfs rule as well - you can enable this setting (sync_enable) via one file. What is the main reason you guys are concerned about this? That users will complain that they are writing to red/brightness and the LED does not change color (because they did not disable sync_enable)? Marek ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 4/5] dt-bindings: leds: Add LED_COLOR_ID and COLOR_NAME definitions 2019-04-01 17:33 [RFC PATCH 0/5] MultiColor LED framework Documentation Dan Murphy ` (2 preceding siblings ...) 2019-04-01 17:33 ` [RFC PATCH 3/5] documention: leds: Add multicolor class documentation Dan Murphy @ 2019-04-01 17:33 ` Dan Murphy 2019-04-01 17:34 ` [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition Dan Murphy 4 siblings, 0 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-01 17:33 UTC (permalink / raw) To: robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds, Dan Murphy This patch is being added to demonstrate reuse of Jacek's patch https://lore.kernel.org/patchwork/patch/1056726/ I have added color name definitions to the above patch. I do not claim authorship and the intent of this patch to strictly for demonstration. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- include/dt-bindings/leds/common.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h index e171d0a6beb2..7e1279d761e4 100644 --- a/include/dt-bindings/leds/common.h +++ b/include/dt-bindings/leds/common.h @@ -19,4 +19,23 @@ #define LEDS_BOOST_ADAPTIVE 1 #define LEDS_BOOST_FIXED 2 +#define LED_COLOR_ID_WHITE 0 +#define LED_COLOR_ID_RED 1 +#define LED_COLOR_ID_GREEN 2 +#define LED_COLOR_ID_BLUE 3 +#define LED_COLOR_ID_AMBER 4 +#define LED_COLOR_ID_VIOLET 5 +#define LED_COLOR_ID_YELLOW 6 +#define LED_COLOR_ID_MAX LED_COLOR_ID_YELLOW + +#define LED_COLOR_NAME_WHITE "white" +#define LED_COLOR_NAME_RED "red" +#define LED_COLOR_NAME_GREEN "green" +#define LED_COLOR_NAME_BLUE "blue" +#define LED_COLOR_NAME_AMBER "amber" +#define LED_COLOR_NAME_VIOLET "violet" +#define LED_COLOR_NAME_YELLOW "yellow" + +#define LED_COLOR_NAME_MAX_SZ 6 + #endif /* __DT_BINDINGS_LEDS_H */ -- 2.19.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition 2019-04-01 17:33 [RFC PATCH 0/5] MultiColor LED framework Documentation Dan Murphy ` (3 preceding siblings ...) 2019-04-01 17:33 ` [RFC PATCH 4/5] dt-bindings: leds: Add LED_COLOR_ID and COLOR_NAME definitions Dan Murphy @ 2019-04-01 17:34 ` Dan Murphy 2019-04-01 21:10 ` Randy Dunlap 2019-04-02 5:44 ` Marek Behun 4 siblings, 2 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-01 17:34 UTC (permalink / raw) To: robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds, Dan Murphy Introduce a multicolor class that groups colored LEDs within a LED node. The framework allows for dynamically setting individual LEDs or setting brightness levels of LEDs and updating them virtually simultaneously. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 411 +++++++++++++++++++++++++++ include/linux/led-class-multicolor.h | 69 +++++ 4 files changed, 491 insertions(+) create mode 100644 drivers/leds/led-class-multicolor.c create mode 100644 include/linux/led-class-multicolor.h diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a72f97fca57b..477e108fd2a7 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -29,6 +29,16 @@ config LEDS_CLASS_FLASH for the flash related features of a LED device. It can be built as a module. +config LEDS_CLASS_MULTI_COLOR + tristate "LED Mulit Color LED Class Support" + depends on LEDS_CLASS + help + This option enables the flash led sysfs class in /sys/class/leds. + It wrapps LED Class and adds flash LEDs specific sysfs attributes + and kernel internal API to it. You'll need this to provide support + for the flash related features of a LED device. It can be built + as a module. + config LEDS_BRIGHTNESS_HW_CHANGED bool "LED Class brightness_hw_changed attribute support" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 4c1b0054f379..c57c2aec9510 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -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_MULTI_COLOR) += led-class-multicolor.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c new file mode 100644 index 000000000000..d54a884eaf09 --- /dev/null +++ b/drivers/leds/led-class-multicolor.c @@ -0,0 +1,411 @@ +// SPDX-License-Identifier: GPL-2.0 +// LED Multi Color class interface +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + +#include <linux/device.h> +#include <linux/init.h> +#include <linux/led-class-multicolor.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/slab.h> +#include "leds.h" + +const char *led_colors[LED_COLOR_ID_MAX + 1] = { + [LED_COLOR_ID_WHITE] = LED_COLOR_NAME_WHITE, + [LED_COLOR_ID_RED] = LED_COLOR_NAME_RED, + [LED_COLOR_ID_GREEN] = LED_COLOR_NAME_GREEN, + [LED_COLOR_ID_BLUE] = LED_COLOR_NAME_BLUE, + [LED_COLOR_ID_AMBER] = LED_COLOR_NAME_AMBER, + [LED_COLOR_ID_VIOLET] = LED_COLOR_NAME_VIOLET, + [LED_COLOR_ID_YELLOW] = LED_COLOR_NAME_YELLOW, +}; + +struct led_classdev_mc_data { + struct led_classdev_mc *mcled_cdev; + struct kobject *color_kobj; + struct kobject *led_kobj; + + struct device_attribute sync_attr; + struct device_attribute sync_enable_attr; + + struct list_head color_list; +}; + +struct led_classdev_mc_priv { + struct led_classdev_mc *mcled_cdev; + + struct device_attribute max_brightness_attr; + struct device_attribute brightness_attr; + + enum led_brightness max_brightness; + enum led_brightness brightness; + struct list_head list; + + int color_id; +}; + +static ssize_t sync_store(struct device *dev, + struct device_attribute *sync_attr, + const char *buf, size_t size) +{ + struct led_classdev_mc_data *data = container_of(sync_attr, + struct led_classdev_mc_data, + sync_attr); + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; + struct led_classdev *led_cdev = &mcled_cdev->led_cdev; + const struct led_multicolor_ops *ops = mcled_cdev->ops; + struct led_classdev_mc_priv *priv; + unsigned long sync_value; + ssize_t ret = -EINVAL; + + mutex_lock(&led_cdev->led_access); + + if (!mcled_cdev->sync_enabled) + goto unlock; + + ret = kstrtoul(buf, 0, &sync_value); + if (ret) + goto unlock; + + if (!sync_value) { + ret = size; + goto unlock; + } + + list_for_each_entry(priv, &data->color_list, list) { + ret = ops->set_color_brightness(priv->mcled_cdev, + priv->color_id, + priv->brightness); + if (ret < 0) + goto unlock; + } + + ret = size; +unlock: + mutex_unlock(&led_cdev->led_access); + return ret; +} + +static ssize_t sync_enable_store(struct device *dev, + struct device_attribute *sync_enable_attr, + const char *buf, size_t size) +{ + struct led_classdev_mc_data *data = container_of(sync_enable_attr, + struct led_classdev_mc_data, + sync_enable_attr); + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; + struct led_classdev *led_cdev = &mcled_cdev->led_cdev; + unsigned long sync_value; + ssize_t ret = -EINVAL; + + mutex_lock(&led_cdev->led_access); + + ret = kstrtoul(buf, 0, &sync_value); + if (ret) + goto unlock; + + mcled_cdev->sync_enabled = sync_value; + + ret = size; +unlock: + mutex_unlock(&led_cdev->led_access); + return ret; +} + +static ssize_t sync_enable_show(struct device *dev, + struct device_attribute *sync_enable_attr, + char *buf) +{ + struct led_classdev_mc_data *data = container_of(sync_enable_attr, + struct led_classdev_mc_data, + sync_enable_attr); + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; + + return sprintf(buf, "%d\n", mcled_cdev->sync_enabled); +} + +static ssize_t brightness_store(struct device *dev, + struct device_attribute *brightness_attr, + const char *buf, size_t size) +{ + struct led_classdev_mc_priv *priv = container_of(brightness_attr, + struct led_classdev_mc_priv, + brightness_attr); + struct led_multicolor_ops *ops = priv->mcled_cdev->ops; + struct led_classdev *led_cdev = &priv->mcled_cdev->led_cdev; + + int old_brightness; + unsigned long value; + ssize_t ret = -EINVAL; + + mutex_lock(&led_cdev->led_access); + + ret = kstrtoul(buf, 10, &value); + if (ret) + goto unlock; + + if (value > priv->max_brightness) { + ret = -EINVAL; + goto unlock; + } + + /* Retain the current brightness in case writing the LED fails */ + old_brightness = priv->brightness; + priv->brightness = value; + + if (priv->mcled_cdev->sync_enabled) { + ret = size; + goto unlock; + } + + ret = ops->set_color_brightness(priv->mcled_cdev, + priv->color_id, value); + if (ret < 0) { + priv->brightness = old_brightness; + goto unlock; + } + + ret = size; +unlock: + mutex_unlock(&led_cdev->led_access); + return ret; +} + +static ssize_t brightness_show(struct device *dev, + struct device_attribute *brightness_attr, char *buf) +{ + struct led_classdev_mc_priv *priv = container_of(brightness_attr, + struct led_classdev_mc_priv, + brightness_attr); + const struct led_multicolor_ops *ops = priv->mcled_cdev->ops; + int value = 0; + + if (priv->mcled_cdev->sync_enabled) { + value = priv->brightness; + goto sync_enabled; + } + + if (ops->get_color_brightness) { + value = ops->get_color_brightness(priv->mcled_cdev, + priv->color_id); + priv->brightness = value; + } else { + value = priv->brightness; + } + +sync_enabled: + return sprintf(buf, "%d\n", value); +} + +static ssize_t max_brightness_show(struct device *dev, + struct device_attribute *max_brightness_attr, + char *buf) +{ + struct led_classdev_mc_priv *priv = container_of(max_brightness_attr, + struct led_classdev_mc_priv, + max_brightness_attr); + + return sprintf(buf, "%d\n", priv->max_brightness); +} + +static int led_multicolor_init_color(struct led_classdev_mc_data *data, + struct led_classdev_mc *mcled_cdev, + int color_id) +{ + struct led_classdev *led_cdev = &mcled_cdev->led_cdev; + struct led_classdev_mc_priv *mc_priv; + int ret; + + mc_priv = devm_kzalloc(led_cdev->dev, sizeof(*mc_priv), GFP_KERNEL); + if (!mc_priv) + return -ENOMEM; + + mc_priv->color_id = color_id; + mc_priv->mcled_cdev = mcled_cdev; + + data->led_kobj = kobject_create_and_add(led_colors[color_id], + data->color_kobj); + if (!data->led_kobj) + return -EINVAL; + + sysfs_attr_init(&mc_priv->brightness_attr.attr); + mc_priv->brightness_attr.attr.name = "brightness"; + mc_priv->brightness_attr.attr.mode = S_IRUSR | S_IWUSR; + mc_priv->brightness_attr.show = brightness_show; + mc_priv->brightness_attr.store = brightness_store; + ret = sysfs_create_file(data->led_kobj, + &mc_priv->brightness_attr.attr); + if (ret) + goto err_out; + + sysfs_attr_init(&mc_priv->max_brightness_attr.attr); + mc_priv->max_brightness_attr.attr.name = "max_brightness"; + mc_priv->max_brightness_attr.attr.mode = S_IRUSR; + mc_priv->max_brightness_attr.show = max_brightness_show; + ret = sysfs_create_file(data->led_kobj, + &mc_priv->max_brightness_attr.attr); + if (ret) + goto err_out; + + mc_priv->max_brightness = LED_FULL; + list_add_tail(&mc_priv->list, &data->color_list); + +err_out: + return ret; +} + +static int led_multicolor_init_color_dir(struct led_classdev_mc_data *data, + struct led_classdev_mc *mcled_cdev) +{ + struct led_classdev *led_cdev = &mcled_cdev->led_cdev; + int ret; + + data->color_kobj = kobject_create_and_add("colors", + &led_cdev->dev->kobj); + if (!data->color_kobj) + return -EINVAL; + + sysfs_attr_init(&data->sync_enable_attr.attr); + data->sync_enable_attr.attr.name = "sync_enable"; + data->sync_enable_attr.attr.mode = S_IRUSR | S_IWUSR; + data->sync_enable_attr.show = sync_enable_show; + data->sync_enable_attr.store = sync_enable_store; + ret = sysfs_create_file(data->color_kobj, + &data->sync_enable_attr.attr); + if (ret) + goto err_out; + + sysfs_attr_init(&data->sync_attr.attr); + data->sync_attr.attr.name = "sync"; + data->sync_attr.attr.mode = S_IWUSR; + data->sync_attr.store = sync_store; + ret = sysfs_create_file(data->color_kobj, + &data->sync_attr.attr); + if (ret) + goto err_out; + + data->mcled_cdev = mcled_cdev; + +err_out: + return ret; +} + +int led_classdev_multicolor_register(struct device *parent, + struct led_classdev_mc *mcled_cdev) +{ + struct led_classdev *led_cdev; + struct led_multicolor_ops *ops; + struct led_classdev_mc_data *data; + int ret; + int i; + + if (!mcled_cdev) + return -EINVAL; + + ops = mcled_cdev->ops; + if (!ops || !ops->set_color_brightness) + return -EINVAL; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + led_cdev = &mcled_cdev->led_cdev; + + INIT_LIST_HEAD(&data->color_list); + + /* Register led class device */ + ret = led_classdev_register(parent, led_cdev); + if (ret) + return ret; + + ret = led_multicolor_init_color_dir(data, mcled_cdev); + if (ret) + return ret; + + + /* Select the sysfs attributes to be created for the device */ + for (i = 0; i < mcled_cdev->num_of_leds; i++) { + ret = led_multicolor_init_color(data, mcled_cdev, + mcled_cdev->available_colors[i]); + if (ret) + break; + } + + return ret; +} +EXPORT_SYMBOL_GPL(led_classdev_multicolor_register); + +void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev) +{ + if (!mcled_cdev) + return; + + led_classdev_unregister(&mcled_cdev->led_cdev); +} +EXPORT_SYMBOL_GPL(led_classdev_multicolor_unregister); + +static void devm_led_classdev_multicolor_release(struct device *dev, void *res) +{ + led_classdev_multicolor_unregister(*(struct led_classdev_mc **)res); +} + +/** + * devm_of_led_classdev_register - resource managed led_classdev_register() + * + * @parent: parent of LED device + * @led_cdev: the led_classdev structure for this device. + */ +int devm_led_classdev_multicolor_register(struct device *parent, + struct led_classdev_mc *mcled_cdev) +{ + struct led_classdev_mc **dr; + int ret; + + dr = devres_alloc(devm_led_classdev_multicolor_release, + sizeof(*dr), GFP_KERNEL); + if (!dr) + return -ENOMEM; + + ret = led_classdev_multicolor_register(parent, mcled_cdev); + if (ret) { + devres_free(dr); + return ret; + } + + *dr = mcled_cdev; + devres_add(parent, dr); + + return 0; +} +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_register); + +static int devm_led_classdev_multicolor_match(struct device *dev, + void *res, void *data) +{ + struct mcled_cdev **p = res; + + if (WARN_ON(!p || !*p)) + return 0; + + return *p == data; +} + +/** + * devm_led_classdev_multicolor_unregister() - resource managed + * led_classdev_multicolor_unregister() + * @parent: The device to unregister. + * @mcled_cdev: the led_classdev_mc structure for this device. + */ +void devm_led_classdev_multicolor_unregister(struct device *dev, + struct led_classdev_mc *mcled_cdev) +{ + WARN_ON(devres_release(dev, + devm_led_classdev_multicolor_release, + devm_led_classdev_multicolor_match, mcled_cdev)); +} +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_unregister); + +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>"); +MODULE_DESCRIPTION("Multi Color LED class interface"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/led-class-multicolor.h b/include/linux/led-class-multicolor.h new file mode 100644 index 000000000000..8ca33ffe4493 --- /dev/null +++ b/include/linux/led-class-multicolor.h @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-2.0 +/* LED Multicolor class interface + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + */ + +#ifndef __LINUX_MULTICOLOR_LEDS_H_INCLUDED +#define __LINUX_MULTICOLOR_LEDS_H_INCLUDED + +#include <linux/leds.h> +#include <dt-bindings/leds/common.h> + +struct led_classdev_mc; + +struct led_multicolor_ops { + /* Set brightness for a specific color id */ + int (*set_color_brightness)(struct led_classdev_mc *mcled_cdev, + int color_id, int value); + /* Read current color setting */ + int (*get_color_brightness)(struct led_classdev_mc *mcled_cdev, + int color_id); +}; + +struct led_classdev_mc { + /* led class device */ + struct led_classdev led_cdev; + + /* multicolor led specific ops */ + struct led_multicolor_ops *ops; + + u32 available_colors[LED_COLOR_ID_MAX]; + int num_of_leds; + + bool sync_enabled; +}; + +static inline struct led_classdev_mc *lcdev_to_mccdev( + struct led_classdev *lcdev) +{ + return container_of(lcdev, struct led_classdev_mc, led_cdev); +} + +/** + * led_classdev_multicolor_register - register a new object of led_classdev + * class with support for multicolor LEDs + * @parent: the multicolor LED to register + * @mcled_cdev: the led_classdev_mc structure for this device + * + * Returns: 0 on success or negative error value on failure + */ +extern int led_classdev_multicolor_register(struct device *parent, + struct led_classdev_mc *mcled_cdev); + +/** + * led_classdev_multicolor_unregister - unregisters an object of led_classdev + * class with support for multicolor LEDs + * @mcled_cdev: the multicolor LED to unregister + * + * Unregister a previously registered via led_classdev_multicolor_register + * object + */ +extern void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev); + +extern int devm_led_classdev_multicolor_register(struct device *parent, + struct led_classdev_mc *mcled_cdev); + +extern void devm_led_classdev_multicolor_unregister(struct device *parent, + struct led_classdev_mc *mcled_cdev); + +#endif /* __LINUX_MULTICOLOR_LEDS_H_INCLUDED */ -- 2.19.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition 2019-04-01 17:34 ` [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition Dan Murphy @ 2019-04-01 21:10 ` Randy Dunlap 2019-04-02 19:07 ` Dan Murphy 2019-04-02 5:44 ` Marek Behun 1 sibling, 1 reply; 21+ messages in thread From: Randy Dunlap @ 2019-04-01 21:10 UTC (permalink / raw) To: Dan Murphy, robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds Hi, On 4/1/19 10:34 AM, Dan Murphy wrote: > Introduce a multicolor class that groups colored LEDs > within a LED node. > > The framework allows for dynamically setting individual LEDs > or setting brightness levels of LEDs and updating them virtually > simultaneously. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > drivers/leds/Kconfig | 10 + > drivers/leds/Makefile | 1 + > drivers/leds/led-class-multicolor.c | 411 +++++++++++++++++++++++++++ > include/linux/led-class-multicolor.h | 69 +++++ > 4 files changed, 491 insertions(+) > create mode 100644 drivers/leds/led-class-multicolor.c > create mode 100644 include/linux/led-class-multicolor.h > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a72f97fca57b..477e108fd2a7 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -29,6 +29,16 @@ config LEDS_CLASS_FLASH > for the flash related features of a LED device. It can be built > as a module. > > +config LEDS_CLASS_MULTI_COLOR > + tristate "LED Mulit Color LED Class Support" Multi > + depends on LEDS_CLASS > + help > + This option enables the flash led sysfs class in /sys/class/leds. LED (?) > + It wrapps LED Class and adds flash LEDs specific sysfs attributes wraps > + and kernel internal API to it. You'll need this to provide support > + for the flash related features of a LED device. It can be built of an LED device. > + as a module. > + > config LEDS_BRIGHTNESS_HW_CHANGED > bool "LED Class brightness_hw_changed attribute support" > depends on LEDS_CLASS ciao. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition 2019-04-01 21:10 ` Randy Dunlap @ 2019-04-02 19:07 ` Dan Murphy 0 siblings, 0 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-02 19:07 UTC (permalink / raw) To: Randy Dunlap, robh+dt, jacek.anaszewski, pavel Cc: marek.behun, linux-kernel, linux-leds Randy Thank you. On 4/1/19 4:10 PM, Randy Dunlap wrote: > Hi, > > On 4/1/19 10:34 AM, Dan Murphy wrote: >> Introduce a multicolor class that groups colored LEDs >> within a LED node. >> >> The framework allows for dynamically setting individual LEDs >> or setting brightness levels of LEDs and updating them virtually >> simultaneously. >> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- >> drivers/leds/Kconfig | 10 + >> drivers/leds/Makefile | 1 + >> drivers/leds/led-class-multicolor.c | 411 +++++++++++++++++++++++++++ >> include/linux/led-class-multicolor.h | 69 +++++ >> 4 files changed, 491 insertions(+) >> create mode 100644 drivers/leds/led-class-multicolor.c >> create mode 100644 include/linux/led-class-multicolor.h >> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig >> index a72f97fca57b..477e108fd2a7 100644 >> --- a/drivers/leds/Kconfig >> +++ b/drivers/leds/Kconfig >> @@ -29,6 +29,16 @@ config LEDS_CLASS_FLASH >> for the flash related features of a LED device. It can be built >> as a module. >> >> +config LEDS_CLASS_MULTI_COLOR >> + tristate "LED Mulit Color LED Class Support" > > Multi Ack > >> + depends on LEDS_CLASS >> + help >> + This option enables the flash led sysfs class in /sys/class/leds. > > LED (?) > HA! This was a copy paste from the Flash class kconfig. I need to re-write the description anyway. So your comments actually will turn into a bug fix for the Kconfig on that flag. Dan <snip> -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition 2019-04-01 17:34 ` [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition Dan Murphy 2019-04-01 21:10 ` Randy Dunlap @ 2019-04-02 5:44 ` Marek Behun 2019-04-02 11:55 ` Dan Murphy 1 sibling, 1 reply; 21+ messages in thread From: Marek Behun @ 2019-04-02 5:44 UTC (permalink / raw) To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds Hi Dan, On Mon, 1 Apr 2019 12:34:00 -0500 Dan Murphy <dmurphy@ti.com> wrote: > +static ssize_t sync_store(struct device *dev, > + struct device_attribute *sync_attr, > + const char *buf, size_t size) > +{ > + struct led_classdev_mc_data *data = container_of(sync_attr, > + struct led_classdev_mc_data, > + sync_attr); > + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; > + struct led_classdev *led_cdev = &mcled_cdev->led_cdev; > + const struct led_multicolor_ops *ops = mcled_cdev->ops; > + struct led_classdev_mc_priv *priv; > + unsigned long sync_value; > + ssize_t ret = -EINVAL; > + > + mutex_lock(&led_cdev->led_access); > + > + if (!mcled_cdev->sync_enabled) > + goto unlock; This lock is redundant, you could just put if (mcled_cdev->sync_enabled) return ret; before the lock. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition 2019-04-02 5:44 ` Marek Behun @ 2019-04-02 11:55 ` Dan Murphy 0 siblings, 0 replies; 21+ messages in thread From: Dan Murphy @ 2019-04-02 11:55 UTC (permalink / raw) To: Marek Behun; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds Marek On 4/2/19 12:44 AM, Marek Behun wrote: > Hi Dan, > > On Mon, 1 Apr 2019 12:34:00 -0500 > Dan Murphy <dmurphy@ti.com> wrote: > >> +static ssize_t sync_store(struct device *dev, >> + struct device_attribute *sync_attr, >> + const char *buf, size_t size) >> +{ >> + struct led_classdev_mc_data *data = container_of(sync_attr, >> + struct led_classdev_mc_data, >> + sync_attr); >> + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; >> + struct led_classdev *led_cdev = &mcled_cdev->led_cdev; >> + const struct led_multicolor_ops *ops = mcled_cdev->ops; >> + struct led_classdev_mc_priv *priv; >> + unsigned long sync_value; >> + ssize_t ret = -EINVAL; >> + >> + mutex_lock(&led_cdev->led_access); >> + >> + if (!mcled_cdev->sync_enabled) >> + goto unlock; > > This lock is redundant, you could just put > if (mcled_cdev->sync_enabled) > return ret; > before the lock. > Ack Thanks the code is just proof of concept code. But I will make this change as well. Dan -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2019-04-02 20:40 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-01 17:33 [RFC PATCH 0/5] MultiColor LED framework Documentation Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 1/5] leds: multicolor: Add sysfs interface definition Dan Murphy 2019-04-01 17:33 ` [RFC PATCH 2/5] dt: bindings: Add multicolor class dt bindings documention Dan Murphy 2019-04-01 21:29 ` Pavel Machek 2019-04-02 11:40 ` Dan Murphy 2019-04-02 16:17 ` Marek Behun 2019-04-02 17:06 ` Dan Murphy 2019-04-02 20:40 ` Jacek Anaszewski 2019-04-02 19:51 ` Jacek Anaszewski 2019-04-01 17:33 ` [RFC PATCH 3/5] documention: leds: Add multicolor class documentation Dan Murphy 2019-04-01 21:18 ` Randy Dunlap 2019-04-01 21:27 ` Pavel Machek 2019-04-02 5:55 ` Marek Behun 2019-04-02 11:53 ` Dan Murphy 2019-04-02 15:56 ` Marek Behun 2019-04-01 17:33 ` [RFC PATCH 4/5] dt-bindings: leds: Add LED_COLOR_ID and COLOR_NAME definitions Dan Murphy 2019-04-01 17:34 ` [RFC PATCH 5/5] leds: multicolor: Introduce a multicolor class definition Dan Murphy 2019-04-01 21:10 ` Randy Dunlap 2019-04-02 19:07 ` Dan Murphy 2019-04-02 5:44 ` Marek Behun 2019-04-02 11:55 ` Dan Murphy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).