Re: [PATCH 1/2] swap led_brightness from enum to typedef

[Pavel Machek <pavel@ucw.cz>]

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

Hi!

> > > The TODO located at drivers/leds/TODO requests:
> > >
> > > * Get rid of led_brightness
> > >
> > > It is really an integer, as maximum is configurable. Get rid of it, or
> > > make it into typedef or something.
> > >
> > > This patch changes the declaration of led_brightness from an enum to a
> > > typedef. In order to hold the currently existing enum values, macro
> > > definitions are provided. Files which use led_brightness are updated to
> > > conform to the new types.
> > >
> > > Signed-off-by: Amy Parker <apark0006@student.cerritos.edu>
> >
> > Thanks for your patch!
> >
> > > 207 files changed, 437 insertions(+), 438 deletions(-)
> >
> > This touches a lot of files, so we better get it right.
> >
> > > --- a/include/linux/leds.h
> > > +++ b/include/linux/leds.h
> > > @@ -26,12 +26,11 @@ struct device_node;
> > >  */
> > >
> > > /* This is obsolete/useless. We now support variable maximum brightness. */
> > > -enum led_brightness {
> > > -     LED_OFF         = 0,
> > > -     LED_ON          = 1,
> > > -     LED_HALF        = 127,
> > > -     LED_FULL        = 255,
> > > -};
> > > +typedef u8 led_brightness;
> >
> > In general, typedefs are frowned upon in the kernel, but there can be a
> > good reason to use one.
> > What if the maximum brightness is larger than 255?
> > Using "unsigned int" sounds better to me, but let's wait for Pavel...
> 
> And as Dan just pointed out, "signed int" would be even better, as it
> would allow a function to return an error code.

We can not apply huge patch all at once, and changing from enum to int
directly will result in warnings in some configurations, no?

Agreed that int would be good.

Best regards,
								Pavel

-- 
http://www.livejournal.com/~pavelmachek

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