Re: [PATCH v2 2/2] leds: add LED driver for EL15203000 board

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

Hi!

> >> +    u8            cmd[2];
> >
> > I am wondering if you should #define this as well.
> It's used only in one place, should I really wrap it to define?
> >
> > Then the #define can be used here and then in the for loop.
> >
> > There is no reason to do ARRAY_SIZE if it will always be 2.
> 
> Result of ARRAY_SIZE() will be always constant and actually it  can be avoided by define.
> 
> But I prefer ARRAY_SIZE() :-)

Me too.

> > As pointed out your max_brightness is a binary and the 0x32 is an effect you technically don't even need max_brightness if you
> >
> > expose the effects as a file.
> >
> > Does 0x32 turn the LED on or off?  Or does it blink the LED?
> 
> It depends on LED board,
> 
> it can blink.
> It can play scene on LED array.
> It can blink smoothly.
> 
> But it depend on the board, not a protocol.

But we expect drivers to control the boards, not protocols.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]