Re: [RESEND LEDs] leds: remove led_brightness

[Lee Jones <lee@kernel.org>]

On Sun, 21 Jul 2024, Guilherme Giácomo Simões wrote:

> Hi community, I hope this email finds you well
> I maked a change in kernel linux, for fulfill a TODO in
> drivers/leds/TODO that say:
> >* On/off LEDs should have max_brightness of 1
> >* Get rid of enum led_brightness
> >
> >It is really an integer, as maximum is configurable. Get rid of it, or
> >make it into typedef or something.
> 
> Then I removed the led_brightness. And in the files that enum
> led_brightness was used i replace to "int" ... And in the file
> "include/linux/leds.h" I created constants like:
> +#define LED_OFF  0
> +#define LED_ON   1
> +#define LED_HALF 127
> +#define LED_FULL 255
> 
> because in some files when has a compare like "brightness == LED_OFF"
> it will work yet.
> 
> The includes/linux/leds.h diff:
> -/* 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,
> -};

I'm not aware of the history of this, however I'm even less sure how
converting these from an enum to #defines makes this any better.

> +// default values for leds brightness
> +#define LED_OFF  0
> +#define LED_ON   1
> +#define LED_HALF 127
> +#define LED_FULL 255
> 
>  enum led_default_state {
>         LEDS_DEFSTATE_OFF       = 0,
> @@ -127,15 +125,15 @@ struct led_classdev {
>          * that can sleep while setting brightness.
>          */
>         void            (*brightness_set)(struct led_classdev *led_cdev,
> -                                         enum led_brightness brightness);
> +                                         int brightness);
>         /*
>          * Set LED brightness level immediately - it can block the caller for
>          * the time required for accessing a LED device register.
>          */
>         int (*brightness_set_blocking)(struct led_classdev *led_cdev,
> -                                      enum led_brightness brightness);
> +                                      int brightness);
>         /* Get LED brightness level */
> -       enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
> +       int (*brightness_get)(struct led_classdev *led_cdev);
> 
>         /*
>          * Activate hardware accelerated blink, delays are in milliseconds
> @@ -486,7 +484,7 @@ int devm_led_trigger_register(struct device *dev,
>  void led_trigger_register_simple(const char *name,
>                                 struct led_trigger **trigger);
>  void led_trigger_unregister_simple(struct led_trigger *trigger);
> -void led_trigger_event(struct led_trigger *trigger,  enum
> led_brightness event);
> +void led_trigger_event(struct led_trigger *trigger,  int event);
>  void led_trigger_blink(struct led_trigger *trigger, unsigned long delay_on,
>                        unsigned long delay_off);
> 
> 
> 
> How the kernel has a lot of files that use this led_brightness, the
> change is very very big.
> I have some questions:
> 
> Does my change make sense?
> 
> How can I split the change into several patches for sending to
> different email lists and still have the split change make sense in
> the email lists I'm going to send it to? can I reference the commit in
> which I delete the enum?
> 

-- 
Lee Jones [李琼斯]