linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] leds: led-core: Get rid of enum led_brightness
@ 2020-12-11  1:48 Abanoub Sameh
  2020-12-11 12:56 ` Marek Behun
  0 siblings, 1 reply; 9+ messages in thread
From: Abanoub Sameh @ 2020-12-11  1:48 UTC (permalink / raw)
  To: pavel; +Cc: linux-leds, linux-kernel, Abanoub Sameh, kernel test robot

This gets rid of enum led_brightness in the main led files,
because it is deprecated, and an int can be used instead,
or maybe even a uint8_t since it only goes up to 255.
Next we can also patch the other files to get rid of it completely.

Signed-off-by: Abanoub Sameh <abanoubsameh@protonmail.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/leds/led-class.c |  3 +--
 drivers/leds/led-core.c  | 20 +++++++-------------
 drivers/leds/leds.h      |  6 ++----
 include/linux/leds.h     | 12 +++++-------
 4 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 131ca83f5fb3..51497c187967 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -145,8 +145,7 @@ static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
 	device_remove_file(led_cdev->dev, &dev_attr_brightness_hw_changed);
 }
 
-void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev,
-					       enum led_brightness brightness)
+void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev, int brightness)
 {
 	if (WARN_ON(!led_cdev->brightness_hw_changed_kn))
 		return;
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index c4e780bdb385..f024efb31853 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -39,8 +39,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
 };
 EXPORT_SYMBOL_GPL(led_colors);
 
-static int __led_set_brightness(struct led_classdev *led_cdev,
-				enum led_brightness value)
+static int __led_set_brightness(struct led_classdev *led_cdev, int value)
 {
 	if (!led_cdev->brightness_set)
 		return -ENOTSUPP;
@@ -50,8 +49,7 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
 	return 0;
 }
 
-static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
-					 enum led_brightness value)
+static int __led_set_brightness_blocking(struct led_classdev *led_cdev, int value)
 {
 	if (!led_cdev->brightness_set_blocking)
 		return -ENOTSUPP;
@@ -240,8 +238,7 @@ void led_stop_software_blink(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_stop_software_blink);
 
-void led_set_brightness(struct led_classdev *led_cdev,
-			enum led_brightness brightness)
+void led_set_brightness(struct led_classdev *led_cdev, int brightness)
 {
 	/*
 	 * If software blink is active, delay brightness setting
@@ -253,7 +250,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 		 * work queue task to avoid problems in case we are called
 		 * from hard irq context.
 		 */
-		if (brightness == LED_OFF) {
+		if (!brightness) {
 			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
 			schedule_work(&led_cdev->set_brightness_work);
 		} else {
@@ -268,8 +265,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness);
 
-void led_set_brightness_nopm(struct led_classdev *led_cdev,
-			      enum led_brightness value)
+void led_set_brightness_nopm(struct led_classdev *led_cdev, int value)
 {
 	/* Use brightness_set op if available, it is guaranteed not to sleep */
 	if (!__led_set_brightness(led_cdev, value))
@@ -281,8 +277,7 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
 
-void led_set_brightness_nosleep(struct led_classdev *led_cdev,
-				enum led_brightness value)
+void led_set_brightness_nosleep(struct led_classdev *led_cdev, int value)
 {
 	led_cdev->brightness = min(value, led_cdev->max_brightness);
 
@@ -293,8 +288,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
 
-int led_set_brightness_sync(struct led_classdev *led_cdev,
-			    enum led_brightness value)
+int led_set_brightness_sync(struct led_classdev *led_cdev, int value)
 {
 	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
 		return -EBUSY;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 2d9eb48bbed9..08925d8adcb0 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -19,10 +19,8 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
 
 void led_init_core(struct led_classdev *led_cdev);
 void led_stop_software_blink(struct led_classdev *led_cdev);
-void led_set_brightness_nopm(struct led_classdev *led_cdev,
-				enum led_brightness value);
-void led_set_brightness_nosleep(struct led_classdev *led_cdev,
-				enum led_brightness value);
+void led_set_brightness_nopm(struct led_classdev *led_cdev, int value);
+void led_set_brightness_nosleep(struct led_classdev *led_cdev, int value);
 ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
 			struct bin_attribute *attr, char *buf,
 			loff_t pos, size_t count);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6a8d6409c993..392d43ff793b 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -63,8 +63,8 @@ struct led_hw_trigger_type {
 
 struct led_classdev {
 	const char		*name;
-	enum led_brightness	 brightness;
-	enum led_brightness	 max_brightness;
+    int brightness;
+    int max_brightness;
 	int			 flags;
 
 	/* Lower 16 bits reflect status */
@@ -253,8 +253,7 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
  * software blink timer that implements blinking when the
  * hardware doesn't. This function is guaranteed not to sleep.
  */
-void led_set_brightness(struct led_classdev *led_cdev,
-			enum led_brightness brightness);
+void led_set_brightness(struct led_classdev *led_cdev, int brightness);
 
 /**
  * led_set_brightness_sync - set LED brightness synchronously
@@ -267,8 +266,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
  *
  * Returns: 0 on success or negative error value on failure
  */
-int led_set_brightness_sync(struct led_classdev *led_cdev,
-			    enum led_brightness value);
+int led_set_brightness_sync(struct led_classdev *led_cdev, int value);
 
 /**
  * led_update_brightness - update LED brightness
@@ -565,7 +563,7 @@ static inline void ledtrig_cpu(enum cpu_led_event evt)
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
 void led_classdev_notify_brightness_hw_changed(
-	struct led_classdev *led_cdev, enum led_brightness brightness);
+	struct led_classdev *led_cdev, int brightness);
 #else
 static inline void led_classdev_notify_brightness_hw_changed(
 	struct led_classdev *led_cdev, enum led_brightness brightness) { }

base-commit: 9fca90cf28920c6d0723d7efd1eae0b0fb90309c
-- 
2.28.0.rc0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] leds: led-core: Get rid of enum led_brightness
  2020-12-11  1:48 [PATCH] leds: led-core: Get rid of enum led_brightness Abanoub Sameh
@ 2020-12-11 12:56 ` Marek Behun
  2020-12-11 14:08   ` David Laight
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Behun @ 2020-12-11 12:56 UTC (permalink / raw)
  To: Abanoub Sameh
  Cc: pavel, linux-leds, linux-kernel, Abanoub Sameh, kernel test robot

On Fri, 11 Dec 2020 03:48:40 +0200
Abanoub Sameh <abanoubsameh8@gmail.com> wrote:

> This gets rid of enum led_brightness in the main led files,
> because it is deprecated, and an int can be used instead,
> or maybe even a uint8_t since it only goes up to 255.
> Next we can also patch the other files to get rid of it completely.

1. unsigned int should be used IMO
  - using int may force all implementers to check for negative value
    and return -EINVAL, which is stupid
  - some LED controllers may offer more than 8bit brightness value, so
    no uint8_t
2. I think we should remove all usages with one commit

Marek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] leds: led-core: Get rid of enum led_brightness
  2020-12-11 12:56 ` Marek Behun
@ 2020-12-11 14:08   ` David Laight
  2020-12-11 18:31     ` Marek Behun
  2020-12-17 22:59     ` Pavel Machek
  0 siblings, 2 replies; 9+ messages in thread
From: David Laight @ 2020-12-11 14:08 UTC (permalink / raw)
  To: 'Marek Behun', Abanoub Sameh
  Cc: pavel, linux-leds, linux-kernel, Abanoub Sameh, kernel test robot

From: Marek Behun
> Sent: 11 December 2020 12:56
> 
> On Fri, 11 Dec 2020 03:48:40 +0200
> Abanoub Sameh <abanoubsameh8@gmail.com> wrote:
> 
> > This gets rid of enum led_brightness in the main led files,
> > because it is deprecated, and an int can be used instead,
> > or maybe even a uint8_t since it only goes up to 255.
> > Next we can also patch the other files to get rid of it completely.
> 
> 1. unsigned int should be used IMO
>   - using int may force all implementers to check for negative value
>     and return -EINVAL, which is stupid
>   - some LED controllers may offer more than 8bit brightness value, so
>     no uint8_t

More than 8 bits would be good.
While not really relevant for actual 'brightness' it allows
for 'strange' things be encoded in the brightness field.

For instance we have some hardware that has RGB leds on it.
They are a single device so it really needs a colour property.
But it is more complex than that, between the driver and LED
there is an FPGA - so it can modulate the LED output in many ways.
As well as using PWM to change the brightness and (eg) 1/2HZ flashing
it is possible to alternate between red and green to get a reasonable
orange (works better than driving both at the same time!).

There is also the option of making the led follow some internal
signal rather then be directly driven by the driver.

While extra parameters could be added, they are only really usable
by code that knows they are present.
So encoding in the 'brightness' sort of makes sense.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] leds: led-core: Get rid of enum led_brightness
  2020-12-11 14:08   ` David Laight
@ 2020-12-11 18:31     ` Marek Behun
  2020-12-17 22:59     ` Pavel Machek
  1 sibling, 0 replies; 9+ messages in thread
From: Marek Behun @ 2020-12-11 18:31 UTC (permalink / raw)
  To: David Laight
  Cc: Abanoub Sameh, pavel, linux-leds, linux-kernel, Abanoub Sameh,
	kernel test robot

On Fri, 11 Dec 2020 14:08:43 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> 
> More than 8 bits would be good.
> While not really relevant for actual 'brightness' it allows
> for 'strange' things be encoded in the brightness field.
>
> For instance we have some hardware that has RGB leds on it.
> They are a single device so it really needs a colour property.
> But it is more complex than that, between the driver and LED
> there is an FPGA - so it can modulate the LED output in many ways.
> As well as using PWM to change the brightness and (eg) 1/2HZ flashing
> it is possible to alternate between red and green to get a reasonable
> orange (works better than driving both at the same time!).

Please don't do that. Don't misuse brightness for other settings.
Instead try to implement the settings in other sysfs files, maybe even
make it generic. Document the new sysfs ABI. But to not encode
non-brightness properties into brightness.

For you multicolor example there is multicolor LED framework now in
kernel.

Marek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] leds: led-core: Get rid of enum led_brightness
  2020-12-11 14:08   ` David Laight
  2020-12-11 18:31     ` Marek Behun
@ 2020-12-17 22:59     ` Pavel Machek
  1 sibling, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2020-12-17 22:59 UTC (permalink / raw)
  To: David Laight
  Cc: 'Marek Behun',
	Abanoub Sameh, linux-leds, linux-kernel, Abanoub Sameh,
	kernel test robot

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

Hi!

> > On Fri, 11 Dec 2020 03:48:40 +0200
> > Abanoub Sameh <abanoubsameh8@gmail.com> wrote:
> > 
> > > This gets rid of enum led_brightness in the main led files,
> > > because it is deprecated, and an int can be used instead,
> > > or maybe even a uint8_t since it only goes up to 255.
> > > Next we can also patch the other files to get rid of it completely.
> > 
> > 1. unsigned int should be used IMO
> >   - using int may force all implementers to check for negative value
> >     and return -EINVAL, which is stupid
> >   - some LED controllers may offer more than 8bit brightness value, so
> >     no uint8_t
> 
> More than 8 bits would be good.
> While not really relevant for actual 'brightness' it allows
> for 'strange' things be encoded in the brightness field.

I have headlamp which can do 0.1lumens to 750lumens. Some go down to
0.01 lumens. More than 8 bits definitely makes sense.

Display backlights also need great ranges.

> While extra parameters could be added, they are only really usable
> by code that knows they are present.
> So encoding in the 'brightness' sort of makes sense.

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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] leds: led-core: Get rid of enum led_brightness
  2020-12-11 20:42 Abanoub Sameh
@ 2021-02-19 10:36 ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2021-02-19 10:36 UTC (permalink / raw)
  To: Abanoub Sameh; +Cc: linux-leds, linux-kernel, Abanoub Sameh

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

Hi!

> This gets rid of enum led_brightness in the main led files,
> because it is deprecated, and an unsigned int can be used instead.
> 
> We can get rid of led_brightness completely and
> patches can also be supplied for the other drivers' files.
> 
> Signed-off-by: Abanoub Sameh <abanoubsameh@protonmail.com>

Ok, thanks for doing this!

I applied the patch... and let's see what happens.

Best regards,
									Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] leds: led-core: Get rid of enum led_brightness
@ 2020-12-11 20:42 Abanoub Sameh
  2021-02-19 10:36 ` Pavel Machek
  0 siblings, 1 reply; 9+ messages in thread
From: Abanoub Sameh @ 2020-12-11 20:42 UTC (permalink / raw)
  To: pavel; +Cc: linux-leds, linux-kernel, Abanoub Sameh

This gets rid of enum led_brightness in the main led files,
because it is deprecated, and an unsigned int can be used instead.

We can get rid of led_brightness completely and
patches can also be supplied for the other drivers' files.

Signed-off-by: Abanoub Sameh <abanoubsameh@protonmail.com>
---
 drivers/leds/led-class.c |  3 +--
 drivers/leds/led-core.c  | 20 +++++++-------------
 drivers/leds/leds.h      |  6 ++----
 include/linux/leds.h     | 12 +++++-------
 4 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 131ca83f5fb3..2e495ff67856 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -145,8 +145,7 @@ static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
 	device_remove_file(led_cdev->dev, &dev_attr_brightness_hw_changed);
 }
 
-void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev,
-					       enum led_brightness brightness)
+void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev, unsigned int brightness)
 {
 	if (WARN_ON(!led_cdev->brightness_hw_changed_kn))
 		return;
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index c4e780bdb385..8eb8054ef9c6 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -39,8 +39,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
 };
 EXPORT_SYMBOL_GPL(led_colors);
 
-static int __led_set_brightness(struct led_classdev *led_cdev,
-				enum led_brightness value)
+static int __led_set_brightness(struct led_classdev *led_cdev, unsigned int value)
 {
 	if (!led_cdev->brightness_set)
 		return -ENOTSUPP;
@@ -50,8 +49,7 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
 	return 0;
 }
 
-static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
-					 enum led_brightness value)
+static int __led_set_brightness_blocking(struct led_classdev *led_cdev, unsigned int value)
 {
 	if (!led_cdev->brightness_set_blocking)
 		return -ENOTSUPP;
@@ -240,8 +238,7 @@ void led_stop_software_blink(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_stop_software_blink);
 
-void led_set_brightness(struct led_classdev *led_cdev,
-			enum led_brightness brightness)
+void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
 {
 	/*
 	 * If software blink is active, delay brightness setting
@@ -253,7 +250,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 		 * work queue task to avoid problems in case we are called
 		 * from hard irq context.
 		 */
-		if (brightness == LED_OFF) {
+		if (!brightness) {
 			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
 			schedule_work(&led_cdev->set_brightness_work);
 		} else {
@@ -268,8 +265,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness);
 
-void led_set_brightness_nopm(struct led_classdev *led_cdev,
-			      enum led_brightness value)
+void led_set_brightness_nopm(struct led_classdev *led_cdev, unsigned int value)
 {
 	/* Use brightness_set op if available, it is guaranteed not to sleep */
 	if (!__led_set_brightness(led_cdev, value))
@@ -281,8 +277,7 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
 
-void led_set_brightness_nosleep(struct led_classdev *led_cdev,
-				enum led_brightness value)
+void led_set_brightness_nosleep(struct led_classdev *led_cdev, unsigned int value)
 {
 	led_cdev->brightness = min(value, led_cdev->max_brightness);
 
@@ -293,8 +288,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
 
-int led_set_brightness_sync(struct led_classdev *led_cdev,
-			    enum led_brightness value)
+int led_set_brightness_sync(struct led_classdev *led_cdev, unsigned int value)
 {
 	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
 		return -EBUSY;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 2d9eb48bbed9..345062ccabda 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -19,10 +19,8 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
 
 void led_init_core(struct led_classdev *led_cdev);
 void led_stop_software_blink(struct led_classdev *led_cdev);
-void led_set_brightness_nopm(struct led_classdev *led_cdev,
-				enum led_brightness value);
-void led_set_brightness_nosleep(struct led_classdev *led_cdev,
-				enum led_brightness value);
+void led_set_brightness_nopm(struct led_classdev *led_cdev, unsigned int value);
+void led_set_brightness_nosleep(struct led_classdev *led_cdev, unsigned int value);
 ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
 			struct bin_attribute *attr, char *buf,
 			loff_t pos, size_t count);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6a8d6409c993..329fd914cf24 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -63,8 +63,8 @@ struct led_hw_trigger_type {
 
 struct led_classdev {
 	const char		*name;
-	enum led_brightness	 brightness;
-	enum led_brightness	 max_brightness;
+	unsigned int brightness;
+	unsigned int max_brightness;
 	int			 flags;
 
 	/* Lower 16 bits reflect status */
@@ -253,8 +253,7 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
  * software blink timer that implements blinking when the
  * hardware doesn't. This function is guaranteed not to sleep.
  */
-void led_set_brightness(struct led_classdev *led_cdev,
-			enum led_brightness brightness);
+void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness);
 
 /**
  * led_set_brightness_sync - set LED brightness synchronously
@@ -267,8 +266,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
  *
  * Returns: 0 on success or negative error value on failure
  */
-int led_set_brightness_sync(struct led_classdev *led_cdev,
-			    enum led_brightness value);
+int led_set_brightness_sync(struct led_classdev *led_cdev, unsigned int value);
 
 /**
  * led_update_brightness - update LED brightness
@@ -565,7 +563,7 @@ static inline void ledtrig_cpu(enum cpu_led_event evt)
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
 void led_classdev_notify_brightness_hw_changed(
-	struct led_classdev *led_cdev, enum led_brightness brightness);
+	struct led_classdev *led_cdev, unsigned int brightness);
 #else
 static inline void led_classdev_notify_brightness_hw_changed(
 	struct led_classdev *led_cdev, enum led_brightness brightness) { }

base-commit: 33dc9614dc208291d0c4bcdeb5d30d481dcd2c4c
-- 
2.28.0.rc0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] leds: led-core: Get rid of enum led_brightness
  2020-12-10 13:49 Abanoub Sameh
@ 2020-12-10 20:03 ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-12-10 20:03 UTC (permalink / raw)
  To: Abanoub Sameh, pavel
  Cc: kbuild-all, clang-built-linux, linux-leds, linux-kernel, Abanoub Sameh

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

Hi Abanoub,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pavel-linux-leds/for-next]
[also build test WARNING on linux/master linus/master j.anaszewski-leds/for-next v5.10-rc7 next-20201210]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Abanoub-Sameh/leds-led-core-Get-rid-of-enum-led_brightness/20201210-215536
base:   git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
config: x86_64-randconfig-a004-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1968804ac726e7674d5de22bc2204b45857da344)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/172073778888c1b26342cb54099eb1f54a482c1b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Abanoub-Sameh/leds-led-core-Get-rid-of-enum-led_brightness/20201210-215536
        git checkout 172073778888c1b26342cb54099eb1f54a482c1b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/leds/led-core.c:282:25: warning: comparison of distinct pointer types ('typeof (value) *' (aka 'int *') and 'typeof (led_cdev->max_brightness) *' (aka 'enum led_brightness *')) [-Wcompare-distinct-pointer-types]
           led_cdev->brightness = min(value, led_cdev->max_brightness);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:51:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   drivers/leds/led-core.c:296:25: warning: comparison of distinct pointer types ('typeof (value) *' (aka 'int *') and 'typeof (led_cdev->max_brightness) *' (aka 'enum led_brightness *')) [-Wcompare-distinct-pointer-types]
           led_cdev->brightness = min(value, led_cdev->max_brightness);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:51:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   2 warnings generated.

vim +282 drivers/leds/led-core.c

81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  279  
172073778888c1 Abanoub Sameh    2020-12-10  280  void led_set_brightness_nosleep(struct led_classdev *led_cdev, int value)
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  281  {
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07 @282  	led_cdev->brightness = min(value, led_cdev->max_brightness);
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  283  
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  284  	if (led_cdev->flags & LED_SUSPENDED)
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  285  		return;
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  286  
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  287  	led_set_brightness_nopm(led_cdev, led_cdev->brightness);
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  288  }
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  289  EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
81fe8e5b73e3f4 Jacek Anaszewski 2015-10-07  290  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40531 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] leds: led-core: Get rid of enum led_brightness
@ 2020-12-10 13:49 Abanoub Sameh
  2020-12-10 20:03 ` kernel test robot
  0 siblings, 1 reply; 9+ messages in thread
From: Abanoub Sameh @ 2020-12-10 13:49 UTC (permalink / raw)
  To: pavel; +Cc: linux-leds, linux-kernel, Abanoub Sameh

This gets rid of enum led_brightness in the main led files,
because it is deprecated, and an int can be used instead,
or maybe even a uint8_t since it only goes up to 255.
Next we can also patch the other files to get rid of it completely.

Signed-off-by: Abanoub Sameh <abanoubsameh@protonmail.com>
---
 drivers/leds/led-class.c |  3 +--
 drivers/leds/led-core.c  | 20 +++++++-------------
 drivers/leds/leds.h      |  6 ++----
 include/linux/leds.h     |  8 +++-----
 4 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 131ca83f5fb3..51497c187967 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -145,8 +145,7 @@ static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
 	device_remove_file(led_cdev->dev, &dev_attr_brightness_hw_changed);
 }
 
-void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev,
-					       enum led_brightness brightness)
+void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev, int brightness)
 {
 	if (WARN_ON(!led_cdev->brightness_hw_changed_kn))
 		return;
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index c4e780bdb385..f024efb31853 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -39,8 +39,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
 };
 EXPORT_SYMBOL_GPL(led_colors);
 
-static int __led_set_brightness(struct led_classdev *led_cdev,
-				enum led_brightness value)
+static int __led_set_brightness(struct led_classdev *led_cdev, int value)
 {
 	if (!led_cdev->brightness_set)
 		return -ENOTSUPP;
@@ -50,8 +49,7 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
 	return 0;
 }
 
-static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
-					 enum led_brightness value)
+static int __led_set_brightness_blocking(struct led_classdev *led_cdev, int value)
 {
 	if (!led_cdev->brightness_set_blocking)
 		return -ENOTSUPP;
@@ -240,8 +238,7 @@ void led_stop_software_blink(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_stop_software_blink);
 
-void led_set_brightness(struct led_classdev *led_cdev,
-			enum led_brightness brightness)
+void led_set_brightness(struct led_classdev *led_cdev, int brightness)
 {
 	/*
 	 * If software blink is active, delay brightness setting
@@ -253,7 +250,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 		 * work queue task to avoid problems in case we are called
 		 * from hard irq context.
 		 */
-		if (brightness == LED_OFF) {
+		if (!brightness) {
 			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
 			schedule_work(&led_cdev->set_brightness_work);
 		} else {
@@ -268,8 +265,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness);
 
-void led_set_brightness_nopm(struct led_classdev *led_cdev,
-			      enum led_brightness value)
+void led_set_brightness_nopm(struct led_classdev *led_cdev, int value)
 {
 	/* Use brightness_set op if available, it is guaranteed not to sleep */
 	if (!__led_set_brightness(led_cdev, value))
@@ -281,8 +277,7 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
 
-void led_set_brightness_nosleep(struct led_classdev *led_cdev,
-				enum led_brightness value)
+void led_set_brightness_nosleep(struct led_classdev *led_cdev, int value)
 {
 	led_cdev->brightness = min(value, led_cdev->max_brightness);
 
@@ -293,8 +288,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
 
-int led_set_brightness_sync(struct led_classdev *led_cdev,
-			    enum led_brightness value)
+int led_set_brightness_sync(struct led_classdev *led_cdev, int value)
 {
 	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
 		return -EBUSY;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 2d9eb48bbed9..08925d8adcb0 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -19,10 +19,8 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
 
 void led_init_core(struct led_classdev *led_cdev);
 void led_stop_software_blink(struct led_classdev *led_cdev);
-void led_set_brightness_nopm(struct led_classdev *led_cdev,
-				enum led_brightness value);
-void led_set_brightness_nosleep(struct led_classdev *led_cdev,
-				enum led_brightness value);
+void led_set_brightness_nopm(struct led_classdev *led_cdev, int value);
+void led_set_brightness_nosleep(struct led_classdev *led_cdev, int value);
 ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
 			struct bin_attribute *attr, char *buf,
 			loff_t pos, size_t count);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6a8d6409c993..b4f8889232de 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -253,8 +253,7 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
  * software blink timer that implements blinking when the
  * hardware doesn't. This function is guaranteed not to sleep.
  */
-void led_set_brightness(struct led_classdev *led_cdev,
-			enum led_brightness brightness);
+void led_set_brightness(struct led_classdev *led_cdev, int brightness);
 
 /**
  * led_set_brightness_sync - set LED brightness synchronously
@@ -267,8 +266,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
  *
  * Returns: 0 on success or negative error value on failure
  */
-int led_set_brightness_sync(struct led_classdev *led_cdev,
-			    enum led_brightness value);
+int led_set_brightness_sync(struct led_classdev *led_cdev, int value);
 
 /**
  * led_update_brightness - update LED brightness
@@ -565,7 +563,7 @@ static inline void ledtrig_cpu(enum cpu_led_event evt)
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
 void led_classdev_notify_brightness_hw_changed(
-	struct led_classdev *led_cdev, enum led_brightness brightness);
+	struct led_classdev *led_cdev, int brightness);
 #else
 static inline void led_classdev_notify_brightness_hw_changed(
 	struct led_classdev *led_cdev, enum led_brightness brightness) { }
-- 
2.28.0.rc0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-02-19 10:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11  1:48 [PATCH] leds: led-core: Get rid of enum led_brightness Abanoub Sameh
2020-12-11 12:56 ` Marek Behun
2020-12-11 14:08   ` David Laight
2020-12-11 18:31     ` Marek Behun
2020-12-17 22:59     ` Pavel Machek
  -- strict thread matches above, loose matches on Subject: below --
2020-12-11 20:42 Abanoub Sameh
2021-02-19 10:36 ` Pavel Machek
2020-12-10 13:49 Abanoub Sameh
2020-12-10 20:03 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).