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 [李琼斯]
