mbox series

[v26,00/15] Multicolor Framework v26

Message ID 20200604120504.32425-1-dmurphy@ti.com
Headers show
Series Multicolor Framework v26 | expand

Message

Dan Murphy June 4, 2020, 12:04 p.m. UTC
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

Dan


Dan Murphy (15):
  dt: bindings: Add multicolor class dt bindings documention
  leds: Add multicolor ID to the color ID list
  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
  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
  leds: lp55xx: Convert LED class registration to devm_*
  leds: lp55xx: Add multicolor framework support to lp55xx
  leds: lp5523: Update the lp5523 code to add multicolor brightness
    function
  leds: lp5521: Add multicolor framework multicolor brightness support
  leds: lp55xx: Fix checkpatch file permissions issues
  leds: lp5523: Fix checkpatch issues in the code

 .../ABI/testing/sysfs-class-led-multicolor    |  34 +
 .../bindings/leds/leds-class-multicolor.yaml  |  39 +
 .../devicetree/bindings/leds/leds-lp50xx.yaml | 136 +++
 .../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  |  88 ++
 MAINTAINERS                                   |   8 +
 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 +-
 drivers/leds/Kconfig                          |  24 +
 drivers/leds/Makefile                         |   2 +
 drivers/leds/led-class-multicolor.c           | 210 +++++
 drivers/leds/led-core.c                       |   1 +
 drivers/leds/leds-lp50xx.c                    | 778 ++++++++++++++++++
 drivers/leds/leds-lp5521.c                    |  43 +-
 drivers/leds/leds-lp5523.c                    |  62 +-
 drivers/leds/leds-lp5562.c                    |  22 +-
 drivers/leds/leds-lp55xx-common.c             | 212 +++--
 drivers/leds/leds-lp55xx-common.h             |  16 +-
 drivers/leds/leds-lp8501.c                    |  23 +-
 include/dt-bindings/leds/common.h             |   3 +-
 include/linux/led-class-multicolor.h          | 121 +++
 include/linux/platform_data/leds-lp55xx.h     |   8 +
 25 files changed, 1989 insertions(+), 355 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor
 create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
 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

Comments

Pavel Machek June 6, 2020, 3:53 p.m. UTC | #1
Hi!

> Introduce a multicolor class that groups colored LEDs
> within a LED node.
> 
> The multi color 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 <jacek.anaszewski@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor
> new file mode 100644
> index 000000000000..7d33a82a4b07
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor
> @@ -0,0 +1,34 @@
> +What:		/sys/class/leds/<led>/brightness
> +Date:		March 2020
> +KernelVersion:	5.8
> +Contact:	Dan Murphy <dmurphy@ti.com>
> +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 color is from 0 to
> +		/sys/class/leds/<led>/max_brightness.

It is not too clear to me what "color" means here.

It would be worth mentioning that this is single integer.

> +What:		/sys/class/leds/<led>/multi_index
> +Date:		March 2020
> +KernelVersion:	5.8
> +Contact:	Dan Murphy <dmurphy@ti.com>
> +Description:	read
> +		The multi_index array, when read, will output the LED colors
> +		by name as they are indexed in the multi_intensity file.

This should specify that it is array of strings.

> +What:		/sys/class/leds/<led>/multi_intensity
> +Date:		March 2020
> +KernelVersion:	5.8
> +Contact:	Dan Murphy <dmurphy@ti.com>
> +Description:	read/write
> +		Intensity level for the LED color within the array.
> +		The intensities for each color must be entered based on the
> +		multi_index array.

I'd mention here that it is array of integers, and what the maximum
values are.

> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9533,6 +9533,14 @@ F:	Documentation/devicetree/bindings/leds/
>  F:	drivers/leds/
>  F:	include/linux/leds.h
>  
> +LED MULTICOLOR FRAMEWORK
> +M:	Dan Murphy <dmurphy@ti.com>
> +L:	linux-leds@vger.kernel.org

I'd like to be mentioned here, too. "M: Pavel Machek
<pavel@ucw.cz>". And I'm not sure if I should be taking MAINTAINER
file update through a LED tree. Should definitely go to separate
patch.

> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9cdc4cfc5d11..fe7d90d4fa23 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -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_MULTI_COLOR
> +	tristate "LED MultiColor LED Class Support"

"LED MultiColor Class Support"

Best regards,
									Pavel
Pavel Machek June 6, 2020, 3:57 p.m. UTC | #2
Hi!

> 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

Can you re-order the patches so that stuff that can be applied now
goes first? Bugfixes, cleanups, devm conversion, yaml conversions that
are already acked...

Best regards,
								Pavel
Dan Murphy June 6, 2020, 4:39 p.m. UTC | #3
Pavek

Thanks for the review

On 6/6/20 10:53 AM, Pavel Machek wrote:
> Hi!
>
>> Introduce a multicolor class that groups colored LEDs
>> within a LED node.
>>
>> The multi color 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 <jacek.anaszewski@gmail.com>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor
>> new file mode 100644
>> index 000000000000..7d33a82a4b07
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor
>> @@ -0,0 +1,34 @@
>> +What:		/sys/class/leds/<led>/brightness
>> +Date:		March 2020
>> +KernelVersion:	5.8
>> +Contact:	Dan Murphy <dmurphy@ti.com>
>> +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 color is from 0 to
>> +		/sys/class/leds/<led>/max_brightness.
> It is not too clear to me what "color" means here.
>
> It would be worth mentioning that this is single integer.

OK I will update this


>> +What:		/sys/class/leds/<led>/multi_index
>> +Date:		March 2020
>> +KernelVersion:	5.8
>> +Contact:	Dan Murphy <dmurphy@ti.com>
>> +Description:	read
>> +		The multi_index array, when read, will output the LED colors
>> +		by name as they are indexed in the multi_intensity file.
> This should specify that it is array of strings.

Yeah this sounds better


>> +What:		/sys/class/leds/<led>/multi_intensity
>> +Date:		March 2020
>> +KernelVersion:	5.8
>> +Contact:	Dan Murphy <dmurphy@ti.com>
>> +Description:	read/write
>> +		Intensity level for the LED color within the array.
>> +		The intensities for each color must be entered based on the
>> +		multi_index array.
> I'd mention here that it is array of integers, and what the maximum
> values are.

Same here.  I will indicate max value cannot exceed max_brightness

But that was what the max_intensity file was for in prior patchsets.

>
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9533,6 +9533,14 @@ F:	Documentation/devicetree/bindings/leds/
>>   F:	drivers/leds/
>>   F:	include/linux/leds.h
>>   
>> +LED MULTICOLOR FRAMEWORK
>> +M:	Dan Murphy <dmurphy@ti.com>
>> +L:	linux-leds@vger.kernel.org
> I'd like to be mentioned here, too. "M: Pavel Machek
> <pavel@ucw.cz>". And I'm not sure if I should be taking MAINTAINER
> file update through a LED tree. Should definitely go to separate
> patch.

Oh definitely.  I thought it was implied that you and Jacek are both 
Maintainers as well.

I will add you but will wait to see if Jacek wants to be added.

I will separate this out and make it a separate patch


>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 9cdc4cfc5d11..fe7d90d4fa23 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -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_MULTI_COLOR
>> +	tristate "LED MultiColor LED Class Support"
> "LED MultiColor Class Support"

OK.

Dan

>
> Best regards,
> 									Pavel
Jacek Anaszewski June 6, 2020, 7:59 p.m. UTC | #4
Dan,

On 6/6/20 6:39 PM, Dan Murphy wrote:
> Pavek
> 
> Thanks for the review
> 
> On 6/6/20 10:53 AM, Pavel Machek wrote:
>> Hi!
>>
>>> Introduce a multicolor class that groups colored LEDs
>>> within a LED node.
>>>
>>> The multi color 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 <jacek.anaszewski@gmail.com>
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor 
>>> b/Documentation/ABI/testing/sysfs-class-led-multicolor
>>> new file mode 100644
[...]
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -9533,6 +9533,14 @@ F:    Documentation/devicetree/bindings/leds/
>>>   F:    drivers/leds/
>>>   F:    include/linux/leds.h
>>> +LED MULTICOLOR FRAMEWORK
>>> +M:    Dan Murphy <dmurphy@ti.com>
>>> +L:    linux-leds@vger.kernel.org
>> I'd like to be mentioned here, too. "M: Pavel Machek
>> <pavel@ucw.cz>". And I'm not sure if I should be taking MAINTAINER
>> file update through a LED tree. Should definitely go to separate
>> patch.
> 
> Oh definitely.  I thought it was implied that you and Jacek are both 
> Maintainers as well.
> 
> I will add you but will wait to see if Jacek wants to be added.

Actually I don't think that we need to add this separate entry
for LED multicolor class. This is still under LED subsystem,
and I didn't add anything for LED class flash.

> I will separate this out and make it a separate patch
Dan Murphy June 8, 2020, 2:34 p.m. UTC | #5
Jacek

On 6/6/20 2:59 PM, Jacek Anaszewski wrote:
> Dan,
>
> On 6/6/20 6:39 PM, Dan Murphy wrote:
>> Pavek
>>
>> Thanks for the review
>>
>> On 6/6/20 10:53 AM, Pavel Machek wrote:
>>> Hi!
>>>
>>>> Introduce a multicolor class that groups colored LEDs
>>>> within a LED node.
>>>>
>>>> The multi color 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 <jacek.anaszewski@gmail.com>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor 
>>>> b/Documentation/ABI/testing/sysfs-class-led-multicolor
>>>> new file mode 100644
> [...]
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -9533,6 +9533,14 @@ F: Documentation/devicetree/bindings/leds/
>>>>   F:    drivers/leds/
>>>>   F:    include/linux/leds.h
>>>> +LED MULTICOLOR FRAMEWORK
>>>> +M:    Dan Murphy <dmurphy@ti.com>
>>>> +L:    linux-leds@vger.kernel.org
>>> I'd like to be mentioned here, too. "M: Pavel Machek
>>> <pavel@ucw.cz>". And I'm not sure if I should be taking MAINTAINER
>>> file update through a LED tree. Should definitely go to separate
>>> patch.
>>
>> Oh definitely.  I thought it was implied that you and Jacek are both 
>> Maintainers as well.
>>
>> I will add you but will wait to see if Jacek wants to be added.
>
> Actually I don't think that we need to add this separate entry
> for LED multicolor class. This is still under LED subsystem,
> and I didn't add anything for LED class flash.

We only need this because I am not a maintainer of the LED flash class 
or the LED class.

But since I authored the code it only made sense to add me as a 
maintainer for this specific class.

You are one of the maintainers of the LED subsystem and wrote the Flash 
class so your maintainer ship is implied and you will be CC'd for all 
patches.

This will not be the case for the multi color class

Dan
Dan Murphy June 8, 2020, 6:35 p.m. UTC | #6
Pavel

On 6/6/20 10:57 AM, Pavel Machek wrote:
> Hi!
>
>> 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
> Can you re-order the patches so that stuff that can be applied now
> goes first? Bugfixes, cleanups, devm conversion, yaml conversions that
> are already acked...

This series should be close to being applied. I am almost done making 
v26 changes.

I don't want to re-order this series.

Dan
Jacek Anaszewski June 8, 2020, 7:41 p.m. UTC | #7
Dan,

On 6/8/20 4:34 PM, Dan Murphy wrote:
> Jacek
> 
> On 6/6/20 2:59 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 6/6/20 6:39 PM, Dan Murphy wrote:
>>> Pavek
>>>
>>> Thanks for the review
>>>
>>> On 6/6/20 10:53 AM, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>> Introduce a multicolor class that groups colored LEDs
>>>>> within a LED node.
>>>>>
>>>>> The multi color 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 <jacek.anaszewski@gmail.com>
>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor 
>>>>> b/Documentation/ABI/testing/sysfs-class-led-multicolor
>>>>> new file mode 100644
>> [...]
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -9533,6 +9533,14 @@ F: Documentation/devicetree/bindings/leds/
>>>>>   F:    drivers/leds/
>>>>>   F:    include/linux/leds.h
>>>>> +LED MULTICOLOR FRAMEWORK
>>>>> +M:    Dan Murphy <dmurphy@ti.com>
>>>>> +L:    linux-leds@vger.kernel.org
>>>> I'd like to be mentioned here, too. "M: Pavel Machek
>>>> <pavel@ucw.cz>". And I'm not sure if I should be taking MAINTAINER
>>>> file update through a LED tree. Should definitely go to separate
>>>> patch.
>>>
>>> Oh definitely.  I thought it was implied that you and Jacek are both 
>>> Maintainers as well.
>>>
>>> I will add you but will wait to see if Jacek wants to be added.
>>
>> Actually I don't think that we need to add this separate entry
>> for LED multicolor class. This is still under LED subsystem,
>> and I didn't add anything for LED class flash.
> 
> We only need this because I am not a maintainer of the LED flash class 
> or the LED class.
> 
> But since I authored the code it only made sense to add me as a 
> maintainer for this specific class.
> 
> You are one of the maintainers of the LED subsystem and wrote the Flash 
> class so your maintainer ship is implied and you will be CC'd for all 
> patches.
> 
> This will not be the case for the multi color class

scripts/get_maintainer.pl returns yourself as well for LED drivers.
But it's up to you.
Dan Murphy June 9, 2020, 11:45 a.m. UTC | #8
Jacek

On 6/8/20 2:41 PM, Jacek Anaszewski wrote:
> Dan,
>
> On 6/8/20 4:34 PM, Dan Murphy wrote:
>> Jacek
>>
>> On 6/6/20 2:59 PM, Jacek Anaszewski wrote:
>>> Dan,
>>>
>>> On 6/6/20 6:39 PM, Dan Murphy wrote:
>>>> Pavek
>>>>
>>>> Thanks for the review
>>>>
>>>> On 6/6/20 10:53 AM, Pavel Machek wrote:
>>>>> Hi!
>>>>>
>>>>>> Introduce a multicolor class that groups colored LEDs
>>>>>> within a LED node.
>>>>>>
>>>>>> The multi color 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 <jacek.anaszewski@gmail.com>
>>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor 
>>>>>> b/Documentation/ABI/testing/sysfs-class-led-multicolor
>>>>>> new file mode 100644
>>> [...]
>>>>>> --- a/MAINTAINERS
>>>>>> +++ b/MAINTAINERS
>>>>>> @@ -9533,6 +9533,14 @@ F: Documentation/devicetree/bindings/leds/
>>>>>>   F:    drivers/leds/
>>>>>>   F:    include/linux/leds.h
>>>>>> +LED MULTICOLOR FRAMEWORK
>>>>>> +M:    Dan Murphy <dmurphy@ti.com>
>>>>>> +L:    linux-leds@vger.kernel.org
>>>>> I'd like to be mentioned here, too. "M: Pavel Machek
>>>>> <pavel@ucw.cz>". And I'm not sure if I should be taking MAINTAINER
>>>>> file update through a LED tree. Should definitely go to separate
>>>>> patch.
>>>>
>>>> Oh definitely.  I thought it was implied that you and Jacek are 
>>>> both Maintainers as well.
>>>>
>>>> I will add you but will wait to see if Jacek wants to be added.
>>>
>>> Actually I don't think that we need to add this separate entry
>>> for LED multicolor class. This is still under LED subsystem,
>>> and I didn't add anything for LED class flash.
>>
>> We only need this because I am not a maintainer of the LED flash 
>> class or the LED class.
>>
>> But since I authored the code it only made sense to add me as a 
>> maintainer for this specific class.
>>
>> You are one of the maintainers of the LED subsystem and wrote the 
>> Flash class so your maintainer ship is implied and you will be CC'd 
>> for all patches.
>>
>> This will not be the case for the multi color class
>
> scripts/get_maintainer.pl returns yourself as well for LED drivers.
> But it's up to you.
>
I dropped it.

Dan