linux-kernel - Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver

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] [day] [month] [year] [list]

Message-id: <5770D114.70909@samsung.com>
Date:	Mon, 27 Jun 2016 09:09:08 +0200
From:	Jacek Anaszewski <j.anaszewski@...sung.com>
To:	Florian Vaussard <florian.vaussard@...g-vd.ch>
Cc:	Pavel Machek <pavel@....cz>,
	Florian Vaussard <florian.vaussard@...il.com>,
	devicetree@...r.kernel.org, Richard Purdie <rpurdie@...ys.net>,
	Rob Herring <robh+dt@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver

Hi Florian and Pavel,

On 06/27/2016 07:46 AM, Florian Vaussard wrote:
> Hi Pavel,
>
> Le 26. 06. 16 à 23:49, Pavel Machek a écrit :
>> Hi!
>>
>>>> +struct ncp5623_led {
>>>> +	bool active;
>>>> +	unsigned int led_no;
>>>> +	struct led_classdev ldev;
>>>> +	struct work_struct work;
>>>> +	struct ncp5623_priv *priv;
>>>> +};
>>>> +
>>>> +struct ncp5623_priv {
>>>> +	struct ncp5623_led leds[NCP5623_MAX_LEDS];
>>>
>>> Please allocate memory dynamically, depending on the number
>>> of LEDs defined in a Device Tree.
>>
>> MAX_LEDs is three. Are you sure overhead of dynamic allocation is
>> worth it?
>>
>> And if this is for RGB leds... very probably device will want to use
>> all 3 channels.
>>
>
> I was about to raise the same question during the v2 of this patch. In addition
> to your arguments, this also changes the way this array is indexed.
>
> Currently the LED number is used as index, but with dynamic allocation I have to
> use an abstract index. This makes some logic a bit harder, especially to check
> if the same LED is declared twice in the device tree (duplicated 'reg' property).

Fair enough. Please ignore my remark then.

-- 
Best regards,
Jacek Anaszewski