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

[Jacek Anaszewski <j.anaszewski@samsung.com>]

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