linux-kernel - Re: [PATCH v24 02/16] leds: multicolor: Introduce a multicolor class definition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b33c83a2-4cf1-7137-74d9-7e1cb8b00737@gmail.com>
Date:   Mon, 4 May 2020 22:31:23 +0200
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Dan Murphy <dmurphy@...com>, pavel@....cz
Cc:     linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v24 02/16] leds: multicolor: Introduce a multicolor class
 definition

Dan,

On 5/3/20 2:32 PM, Dan Murphy wrote:
> 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@...il.com>
> Signed-off-by: Dan Murphy <dmurphy@...com>
> ---
[...]
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -30,6 +30,17 @@ 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"
> +	depends on LEDS_CLASS
> +	depends on LEDS_CLASS_MULTI_COLOR || !LEDS_CLASS_MULTI_COLOR

I was saying about adding this dependency to the drivers based on
LED mc class. This way it does not make any sense. Moreover it is
erroneous:

$ make menuconfig
drivers/leds/Kconfig:33:error: recursive dependency detected!

Instead you should add it to the Kconfig entries of all drivers
that depend on LED mc class, i.e.:

- config LEDS_LP50XX
- config LEDS_LP5521
- config LEDS_LP5523

Moreover there are still some checkpatch.pl problems:

---------------------------------------------------------------
0003-leds-multicolor-Introduce-a-multicolor-class-definit.patch
---------------------------------------------------------------
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#89: FILE: Documentation/leds/leds-class-multicolor.rst:1:
+====================================

ERROR: spaces required around that '=' (ctx:WxO)
#294: FILE: drivers/leds/led-class-multicolor.c:62:
+		ret =-EINVAL;
  		    ^

ERROR: space required before that '-' (ctx:OxV)
#294: FILE: drivers/leds/led-class-multicolor.c:62:
+		ret =-EINVAL;

WARNING: DT binding documents should be licensed (GPL-2.0-only OR 
BSD-2-Clause)
#31: FILE: Documentation/devicetree/bindings/leds/leds-lp50xx.yaml:1:
+# SPDX-License-Identifier: GPL-2.0

WARNING: Block comments use * on subsequent lines
#705: FILE: drivers/leds/leds-lp50xx.c:636:
+		/* There are only 3 LEDs per module otherwise they should be
+		   banked which also is presented as 3 LEDs*/

WARNING: Block comments use a trailing */ on a separate line
#705: FILE: drivers/leds/leds-lp50xx.c:636:
+		   banked which also is presented as 3 LEDs*/


---------------------------------------------------------------
0008-ARM-dts-n900-Add-reg-property-to-the-LP5523-channel-.patch
---------------------------------------------------------------
WARNING: 'accomodate' may be misspelled - perhaps 'accommodate'?

---------------------------------------------------------------
0009-ARM-dts-imx6dl-yapp4-Add-reg-property-to-the-lp5562-.patch
---------------------------------------------------------------
WARNING: 'accomodate' may be misspelled - perhaps 'accommodate'?

---------------------------------------------------------------
0010-ARM-dts-ste-href-Add-reg-property-to-the-LP5521-chan.patch
---------------------------------------------------------------
WARNING: 'accomodate' may be misspelled - perhaps 'accommodate'?


> +	help
> +	  This option enables the multicolor LED sysfs class in /sys/class/leds.
> +	  It wraps LED class and adds multicolor LED specific sysfs attributes
> +	  and kernel internal API to it. You'll need this to provide support
> +	  for multicolor LEDs that are grouped together. This class is not
> +	  intended for single color LEDs. It can be built as a module.
> +


-- 
Best regards,
Jacek Anaszewski