All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] led: core: RfC - add RGB LED handling to the core
@ 2016-01-10 20:27 Heiner Kallweit
  2016-01-13  9:42 ` Jacek Anaszewski
  2016-01-14 12:08 ` Jacek Anaszewski
  0 siblings, 2 replies; 27+ messages in thread
From: Heiner Kallweit @ 2016-01-10 20:27 UTC (permalink / raw)
  To: linux-leds, Jacek Anaszewski

When playing with a ThingM Blink(1) USB RGB LED device I found that there
are few drivers for RGB LED's spread across the kernel and each one
implements the RGB functionality in its own way.
Some examples:
- drivers/hid/hid-thingm.c
- drivers/usb/misc/usbled.c
- drivers/leds/leds-bd2802.c
- drivers/leds/leds-blinkm.c
...
IMHO it would make sense to add generic RGB functionality to the LED core.

Things I didn't like too much in other driver implementations:
- one led_classdev per color or at least
- one sysfs attribute per color
Colors are not really independent therefore I'd prefer one led_classdev
per RGB LED. Also userspace should be able to change the color with one
call -> therefore only one sysfs attribute for the RGB values.

Also the current brightness-related functionality should not be effected
by the RGB extension.

This patch is intended to demonstrate the idea of the extension. Most likely
it's not ready yet for submission.

Main idea is to base the effective RGB value on a combination of brightness
and a scaled-to-max RGB value.
The RGB-related callbacks are basically the same as for brightness.
RGB functionally can be switched on with a new flag in the led_classdev.flags
bitmap.

Experimentally I changed the thingm driver to use this extension and it works
quite well. As one result the driver could be very much simplified.

Any feedback is appreciated.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/leds/led-class.c |  62 +++++++++++++++++++++++++++
 drivers/leds/led-core.c  | 106 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/leds/leds.h      |  12 ++++++
 include/linux/leds.h     |  13 ++++++
 4 files changed, 193 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 14139c3..5e4c2f2 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -64,6 +64,47 @@ unlock:
 }
 static DEVICE_ATTR_RW(brightness);
 
+static ssize_t rgb_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	u32 rgb_val;
+
+	mutex_lock(&led_cdev->led_access);
+	led_update_rgb_val(led_cdev);
+	rgb_val = led_get_rgb_val(led_cdev);
+	mutex_unlock(&led_cdev->led_access);
+
+	return sprintf(buf, "0x%06x\n", rgb_val);
+}
+
+static ssize_t rgb_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	unsigned long state;
+	ssize_t ret;
+
+	mutex_lock(&led_cdev->led_access);
+
+	if (led_sysfs_is_disabled(led_cdev)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = kstrtoul(buf, 0, &state);
+	if (ret)
+		goto unlock;
+
+	led_set_rgb_val(led_cdev, state);
+
+	ret = size;
+unlock:
+	mutex_unlock(&led_cdev->led_access);
+	return ret;
+}
+static DEVICE_ATTR_RW(rgb);
+
 static ssize_t max_brightness_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -190,6 +231,16 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 	char name[64];
 	int ret;
 
+	/* FLASH is not supported for RGB LEDs so far
+	 * and RGB enforces max_brightness = LED_FULL.
+	 * Initialize the color as white.
+	 */
+	if (led_isRGB(led_cdev)) {
+		led_cdev->flags &= ~LED_DEV_CAP_FLASH;
+		led_cdev->max_brightness = LED_FULL;
+		led_cdev->rgb_val = 0xffffff;
+	}
+
 	ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
 	if (ret < 0)
 		return ret;
@@ -203,6 +254,14 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 		dev_warn(parent, "Led %s renamed to %s due to name collision",
 				led_cdev->name, dev_name(led_cdev->dev));
 
+	if (led_isRGB(led_cdev)) {
+		ret = device_create_file(led_cdev->dev, &dev_attr_rgb);
+		if (ret) {
+			device_unregister(led_cdev->dev);
+			return ret;
+		}
+	}
+
 #ifdef CONFIG_LEDS_TRIGGERS
 	init_rwsem(&led_cdev->trigger_lock);
 #endif
@@ -252,6 +311,9 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 
 	flush_work(&led_cdev->set_brightness_work);
 
+	if (led_isRGB(led_cdev))
+		device_remove_file(led_cdev->dev, &dev_attr_rgb);
+
 	device_unregister(led_cdev->dev);
 
 	down_write(&leds_list_lock);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 19e1e60..6563bd3 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -25,6 +25,48 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
 LIST_HEAD(leds_list);
 EXPORT_SYMBOL_GPL(leds_list);
 
+static void led_set_rgb_raw(struct led_classdev *cdev, u32 rgb)
+{
+	u32 red_raw = (rgb >> 16) & 0xff;
+	u32 green_raw = (rgb >> 8) & 0xff;
+	u32 blue_raw = rgb & 0xff;
+	u32 max_raw, red, green, blue;
+
+	max_raw = max(red_raw, green_raw);
+	if (blue_raw > max_raw)
+		max_raw = blue_raw;
+
+	if (!max_raw) {
+		cdev->brightness = 0;
+		cdev->rgb_val = 0;
+		return;
+	}
+
+	red = DIV_ROUND_CLOSEST(red_raw * LED_FULL, max_raw);
+	green = DIV_ROUND_CLOSEST(green_raw * LED_FULL, max_raw);
+	blue = DIV_ROUND_CLOSEST(blue_raw * LED_FULL, max_raw);
+
+	cdev->brightness = max_raw;
+	cdev->rgb_val = (red << 16) + (green << 8) + blue;
+}
+
+static u32 led_get_rgb_raw(struct led_classdev *cdev, bool delayed)
+{
+	u32 rgb = delayed ? cdev->delayed_rgb_value : cdev->rgb_val;
+	u32 brightness = delayed ? cdev->delayed_set_value :
+			 cdev->brightness;
+	u32 red = (rgb >> 16) & 0xff;
+	u32 green = (rgb >> 8) & 0xff;
+	u32 blue = rgb & 0xff;
+	u32 red_raw, green_raw, blue_raw;
+
+	red_raw = DIV_ROUND_CLOSEST(red * brightness, LED_FULL);
+	green_raw = DIV_ROUND_CLOSEST(green * brightness, LED_FULL);
+	blue_raw = DIV_ROUND_CLOSEST(blue * brightness, LED_FULL);
+
+	return (red_raw << 16) + (green_raw << 8) + blue_raw;
+}
+
 static void led_timer_function(unsigned long data)
 {
 	struct led_classdev *led_cdev = (void *)data;
@@ -91,6 +133,21 @@ static void set_brightness_delayed(struct work_struct *ws)
 		led_cdev->flags &= ~LED_BLINK_DISABLE;
 	}
 
+	if (led_isRGB(led_cdev)) {
+		u32 rgb = led_get_rgb_raw(led_cdev, true);
+
+		if (led_cdev->rgb_set)
+			led_cdev->rgb_set(led_cdev, rgb);
+		else if (led_cdev->rgb_set_blocking)
+			ret = led_cdev->rgb_set_blocking(led_cdev, rgb);
+		else
+			ret = -EOPNOTSUPP;
+		if (ret < 0)
+			dev_err(led_cdev->dev,
+				"Setting LED RGB value failed (%d)\n", ret);
+		return;
+	}
+
 	if (led_cdev->brightness_set)
 		led_cdev->brightness_set(led_cdev, led_cdev->delayed_set_value);
 	else if (led_cdev->brightness_set_blocking)
@@ -229,9 +286,35 @@ void led_set_brightness(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness);
 
+void led_set_rgb_val(struct led_classdev *led_cdev, u32 rgb_val)
+{
+	if (!led_isRGB(led_cdev) || (led_cdev->flags & LED_SUSPENDED))
+		return;
+
+	led_set_rgb_raw(led_cdev, rgb_val);
+
+	if (led_cdev->rgb_set) {
+		led_cdev->rgb_set(led_cdev, rgb_val);
+		return;
+	}
+
+	/* If RGB setting can sleep, delegate it to a work queue task */
+	led_cdev->delayed_set_value = led_cdev->brightness;
+	led_cdev->delayed_rgb_value = led_cdev->rgb_val;
+	schedule_work(&led_cdev->set_brightness_work);
+}
+EXPORT_SYMBOL_GPL(led_set_rgb_val);
+
 void led_set_brightness_nopm(struct led_classdev *led_cdev,
 			      enum led_brightness value)
 {
+	if (led_isRGB(led_cdev) && led_cdev->rgb_set) {
+		u32 rgb = led_get_rgb_raw(led_cdev, false);
+
+		led_cdev->rgb_set(led_cdev, rgb);
+		return;
+	}
+
 	/* Use brightness_set op if available, it is guaranteed not to sleep */
 	if (led_cdev->brightness_set) {
 		led_cdev->brightness_set(led_cdev, value);
@@ -240,6 +323,7 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
 
 	/* If brightness setting can sleep, delegate it to a work queue task */
 	led_cdev->delayed_set_value = value;
+	led_cdev->delayed_rgb_value = led_cdev->rgb_val;
 	schedule_work(&led_cdev->set_brightness_work);
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
@@ -267,6 +351,12 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
 	if (led_cdev->flags & LED_SUSPENDED)
 		return 0;
 
+	if (led_isRGB(led_cdev) && led_cdev->rgb_set_blocking) {
+		u32 rgb = led_get_rgb_raw(led_cdev, false);
+
+		return led_cdev->rgb_set_blocking(led_cdev, rgb);
+	}
+
 	if (led_cdev->brightness_set_blocking)
 		return led_cdev->brightness_set_blocking(led_cdev,
 							 led_cdev->brightness);
@@ -290,6 +380,22 @@ int led_update_brightness(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_update_brightness);
 
+int led_update_rgb_val(struct led_classdev *led_cdev)
+{
+	s32 ret = 0;
+
+	if (led_isRGB(led_cdev) && led_cdev->rgb_get) {
+		ret = led_cdev->rgb_get(led_cdev);
+		if (ret >= 0) {
+			led_set_rgb_raw(led_cdev, ret);
+			return 0;
+		}
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(led_update_rgb_val);
+
 /* Caller must ensure led_cdev->led_access held */
 void led_sysfs_disable(struct led_classdev *led_cdev)
 {
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index db3f20d..c733777 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -16,17 +16,29 @@
 #include <linux/rwsem.h>
 #include <linux/leds.h>
 
+static inline bool led_isRGB(struct led_classdev *led_cdev)
+{
+	return (led_cdev->flags & LED_RGB) != 0;
+}
+
 static inline int led_get_brightness(struct led_classdev *led_cdev)
 {
 	return led_cdev->brightness;
 }
 
+static inline u32 led_get_rgb_val(struct led_classdev *led_cdev)
+{
+	return led_cdev->rgb_val;
+}
+
 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_rgb_val(struct led_classdev *led_cdev, u32 rgb_val);
+int led_update_rgb_val(struct led_classdev *led_cdev);
 
 extern struct rw_semaphore leds_list_lock;
 extern struct list_head leds_list;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 4429887..83d2912 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -35,6 +35,7 @@ struct led_classdev {
 	const char		*name;
 	enum led_brightness	 brightness;
 	enum led_brightness	 max_brightness;
+	u32			 rgb_val;
 	int			 flags;
 
 	/* Lower 16 bits reflect status */
@@ -48,6 +49,7 @@ struct led_classdev {
 #define LED_BLINK_DISABLE	(1 << 21)
 #define LED_SYSFS_DISABLE	(1 << 22)
 #define LED_DEV_CAP_FLASH	(1 << 23)
+#define LED_RGB			(1 << 24)
 
 	/* Set LED brightness level */
 	/* Must not sleep. If no non-blocking version can be provided
@@ -56,15 +58,25 @@ struct led_classdev {
 	 */
 	void		(*brightness_set)(struct led_classdev *led_cdev,
 					  enum led_brightness brightness);
+
+	void		(*rgb_set)(struct led_classdev *led_cdev,
+				   u32 rgb_val);
 	/*
 	 * 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 (*rgb_set_blocking)(struct led_classdev *led_cdev,
+				u32 rgb_val);
+
 	/* Get LED brightness level */
 	enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
 
+	/* Get LED RGB val */
+	s32 (*rgb_get)(struct led_classdev *led_cdev);
+
 	/*
 	 * Activate hardware accelerated blink, delays are in milliseconds
 	 * and if both are zero then a sensible default should be chosen.
@@ -90,6 +102,7 @@ struct led_classdev {
 
 	struct work_struct	set_brightness_work;
 	int			delayed_set_value;
+	u32			delayed_rgb_value;
 
 #ifdef CONFIG_LEDS_TRIGGERS
 	/* Protects the trigger data below */
-- 
2.7.0

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

end of thread, other threads:[~2016-02-07  0:20 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-10 20:27 [PATCH] led: core: RfC - add RGB LED handling to the core Heiner Kallweit
2016-01-13  9:42 ` Jacek Anaszewski
2016-01-13 19:54   ` Heiner Kallweit
2016-01-14 11:14     ` Jacek Anaszewski
2016-01-14 12:05       ` Jacek Anaszewski
2016-01-14 12:08 ` Jacek Anaszewski
2016-01-15 20:16   ` Heiner Kallweit
2016-01-17 22:31     ` Jacek Anaszewski
2016-01-24 13:38       ` Heiner Kallweit
2016-01-25  8:41         ` Jacek Anaszewski
2016-01-25  9:51           ` Heiner Kallweit
2016-01-25 10:52             ` Jacek Anaszewski
2016-01-25 19:09               ` Heiner Kallweit
2016-01-26  9:37                 ` Jacek Anaszewski
2016-01-26 20:08                   ` Heiner Kallweit
2016-01-27  8:27                     ` Jacek Anaszewski
2016-01-29  7:00                       ` Heiner Kallweit
2016-01-29  8:24                         ` Jacek Anaszewski
2016-02-01  6:43                           ` Heiner Kallweit
2016-02-01  8:26                             ` Jacek Anaszewski
2016-02-01 21:41                               ` Heiner Kallweit
2016-02-02  8:58                                 ` Jacek Anaszewski
2016-02-03  6:42                                   ` Heiner Kallweit
2016-02-03  8:28                                     ` Jacek Anaszewski
2016-02-03 22:08                                       ` Heiner Kallweit
2016-02-04  8:30                                         ` Jacek Anaszewski
2016-02-07  0:18                                           ` Heiner Kallweit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.