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

[Jacek Anaszewski]

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