linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH/RFC v7 0/3] LED / flash API integration - LED Flash Class
@ 2014-11-12 16:09 Jacek Anaszewski
  2014-11-12 16:09 ` [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way Jacek Anaszewski
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-12 16:09 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-media, sakari.ailus, kyungmin.park, b.zolnierkie, Jacek Anaszewski

This patch set is the follow-up of the LED / flash API integration
series [1].

========================
Changes since version 6:
========================

- removed addition of public LED subsystem API for setting
  torch brightness in favour of internal API for
  synchronous and asynchronous led brightness level setting
- fixed possible race condition upon creating LED Flash class
  related sysfs attributes

========================
Changes since version 5:
========================

- removed flash manager framework - its implementation needs
  further thorough discussion.
- removed external strobe facilities from the LED Flash Class
  and provided external_strobe_set op in v4l2-flash. LED subsystem
  should be strobe provider agnostic.

Thanks,
Jacek Anaszewski

[1] https://lkml.org/lkml/2014/7/11/914


Jacek Anaszewski (3):
  leds: Add support for setting brightness in a synchronous way
  leds: Add LED Flash Class wrapper to LED subsystem
  Documentation: leds: Add description of LED Flash Class extension

 Documentation/leds/leds-class-flash.txt   |   39 +++
 drivers/leds/Kconfig                      |   11 +
 drivers/leds/Makefile                     |    1 +
 drivers/leds/led-class-flash.c            |  511 +++++++++++++++++++++++++++++
 drivers/leds/led-class.c                  |   14 +-
 drivers/leds/led-core.c                   |   19 +-
 drivers/leds/leds.h                       |   20 +-
 drivers/leds/trigger/ledtrig-backlight.c  |    8 +-
 drivers/leds/trigger/ledtrig-default-on.c |    2 +-
 drivers/leds/trigger/ledtrig-gpio.c       |    6 +-
 drivers/leds/trigger/ledtrig-heartbeat.c  |    2 +-
 drivers/leds/trigger/ledtrig-oneshot.c    |    4 +-
 drivers/leds/trigger/ledtrig-transient.c  |   10 +-
 include/linux/led-class-flash.h           |  229 +++++++++++++
 include/linux/leds.h                      |   11 +
 15 files changed, 861 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/leds/leds-class-flash.txt
 create mode 100644 drivers/leds/led-class-flash.c
 create mode 100644 include/linux/led-class-flash.h

-- 
1.7.9.5


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

* [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way
  2014-11-12 16:09 [PATCH/RFC v7 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
@ 2014-11-12 16:09 ` Jacek Anaszewski
  2014-11-13 18:43   ` Bryan Wu
  2014-11-12 16:09 ` [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
  2014-11-12 16:09 ` [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension Jacek Anaszewski
  2 siblings, 1 reply; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-12 16:09 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-media, sakari.ailus, kyungmin.park, b.zolnierkie,
	Jacek Anaszewski, Bryan Wu, Richard Purdie

There are use cases when setting a LED brightness has to
have immediate effect (e.g. setting a torch LED brightness).
This patch extends LED subsystem to support such operations.
The LED subsystem internal API __led_set_brightness is changed
to led_set_brightness_async and new led_set_brightness_sync API
is added.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
---
 drivers/leds/led-class.c                  |   10 ++++++----
 drivers/leds/led-core.c                   |   19 ++++++++++++++++---
 drivers/leds/leds.h                       |   20 ++++++++++++++++----
 drivers/leds/trigger/ledtrig-backlight.c  |    8 ++++----
 drivers/leds/trigger/ledtrig-default-on.c |    2 +-
 drivers/leds/trigger/ledtrig-gpio.c       |    6 +++---
 drivers/leds/trigger/ledtrig-heartbeat.c  |    2 +-
 drivers/leds/trigger/ledtrig-oneshot.c    |    4 ++--
 drivers/leds/trigger/ledtrig-transient.c  |   10 ++++++----
 include/linux/leds.h                      |    8 ++++++++
 10 files changed, 63 insertions(+), 26 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 65722de..dbeebac 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -55,7 +55,7 @@ static ssize_t brightness_store(struct device *dev,
 
 	if (state == LED_OFF)
 		led_trigger_remove(led_cdev);
-	__led_set_brightness(led_cdev, state);
+	led_set_brightness(led_cdev, state);
 
 	ret = size;
 unlock:
@@ -109,7 +109,7 @@ static void led_timer_function(unsigned long data)
 	unsigned long delay;
 
 	if (!led_cdev->blink_delay_on || !led_cdev->blink_delay_off) {
-		__led_set_brightness(led_cdev, LED_OFF);
+		led_set_brightness_async(led_cdev, LED_OFF);
 		return;
 	}
 
@@ -132,7 +132,7 @@ static void led_timer_function(unsigned long data)
 		delay = led_cdev->blink_delay_off;
 	}
 
-	__led_set_brightness(led_cdev, brightness);
+	led_set_brightness_async(led_cdev, brightness);
 
 	/* Return in next iteration if led is in one-shot mode and we are in
 	 * the final blink state so that the led is toggled each delay_on +
@@ -158,7 +158,7 @@ static void set_brightness_delayed(struct work_struct *ws)
 
 	led_stop_software_blink(led_cdev);
 
-	__led_set_brightness(led_cdev, led_cdev->delayed_set_value);
+	led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
 }
 
 /**
@@ -233,6 +233,8 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 	if (!led_cdev->max_brightness)
 		led_cdev->max_brightness = LED_FULL;
 
+	led_cdev->flags |= SET_BRIGHTNESS_ASYNC;
+
 	led_update_brightness(led_cdev);
 
 	INIT_WORK(&led_cdev->set_brightness_work, set_brightness_delayed);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index be6d9fa..a745e32 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -42,13 +42,13 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
 
 	/* never on - just set to off */
 	if (!delay_on) {
-		__led_set_brightness(led_cdev, LED_OFF);
+		led_set_brightness_async(led_cdev, LED_OFF);
 		return;
 	}
 
 	/* never off - just set to brightness */
 	if (!delay_off) {
-		__led_set_brightness(led_cdev, led_cdev->blink_brightness);
+		led_set_brightness_async(led_cdev, led_cdev->blink_brightness);
 		return;
 	}
 
@@ -117,6 +117,8 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
 void led_set_brightness(struct led_classdev *led_cdev,
 			enum led_brightness brightness)
 {
+	int ret = 0;
+
 	/* delay brightness setting if need to stop soft-blink timer */
 	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
 		led_cdev->delayed_set_value = brightness;
@@ -124,7 +126,18 @@ void led_set_brightness(struct led_classdev *led_cdev,
 		return;
 	}
 
-	__led_set_brightness(led_cdev, brightness);
+	if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
+		led_set_brightness_async(led_cdev, brightness);
+		return;
+	} else if (led_cdev->flags & SET_BRIGHTNESS_SYNC) {
+		ret = led_set_brightness_sync(led_cdev, brightness);
+	} else {
+		ret = -EINVAL;
+	}
+
+	if (ret < 0)
+		dev_dbg(led_cdev->dev, "Setting LED brightness failed (%d)\n",
+			ret);
 }
 EXPORT_SYMBOL(led_set_brightness);
 
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 4c50365..2348dbd 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -17,16 +17,28 @@
 #include <linux/rwsem.h>
 #include <linux/leds.h>
 
-static inline void __led_set_brightness(struct led_classdev *led_cdev,
+static inline void led_set_brightness_async(struct led_classdev *led_cdev,
 					enum led_brightness value)
 {
-	if (value > led_cdev->max_brightness)
-		value = led_cdev->max_brightness;
-	led_cdev->brightness = value;
+	led_cdev->brightness = min(value, led_cdev->max_brightness);
+
 	if (!(led_cdev->flags & LED_SUSPENDED))
 		led_cdev->brightness_set(led_cdev, value);
 }
 
+static inline int led_set_brightness_sync(struct led_classdev *led_cdev,
+					enum led_brightness value)
+{
+	int ret = 0;
+
+	led_cdev->brightness = min(value, led_cdev->max_brightness);
+
+	if (!(led_cdev->flags & LED_SUSPENDED))
+		ret = led_cdev->brightness_set_sync(led_cdev,
+						led_cdev->brightness);
+	return ret;
+}
+
 static inline int led_get_brightness(struct led_classdev *led_cdev)
 {
 	return led_cdev->brightness;
diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index 47e55aa..59eca17 100644
--- a/drivers/leds/trigger/ledtrig-backlight.c
+++ b/drivers/leds/trigger/ledtrig-backlight.c
@@ -51,9 +51,9 @@ static int fb_notifier_callback(struct notifier_block *p,
 
 	if ((n->old_status == UNBLANK) ^ n->invert) {
 		n->brightness = led->brightness;
-		__led_set_brightness(led, LED_OFF);
+		led_set_brightness_async(led, LED_OFF);
 	} else {
-		__led_set_brightness(led, n->brightness);
+		led_set_brightness_async(led, n->brightness);
 	}
 
 	n->old_status = new_status;
@@ -89,9 +89,9 @@ static ssize_t bl_trig_invert_store(struct device *dev,
 
 	/* After inverting, we need to update the LED. */
 	if ((n->old_status == BLANK) ^ n->invert)
-		__led_set_brightness(led, LED_OFF);
+		led_set_brightness_async(led, LED_OFF);
 	else
-		__led_set_brightness(led, n->brightness);
+		led_set_brightness_async(led, n->brightness);
 
 	return num;
 }
diff --git a/drivers/leds/trigger/ledtrig-default-on.c b/drivers/leds/trigger/ledtrig-default-on.c
index 81a91be..6f38f88 100644
--- a/drivers/leds/trigger/ledtrig-default-on.c
+++ b/drivers/leds/trigger/ledtrig-default-on.c
@@ -19,7 +19,7 @@
 
 static void defon_trig_activate(struct led_classdev *led_cdev)
 {
-	__led_set_brightness(led_cdev, led_cdev->max_brightness);
+	led_set_brightness_async(led_cdev, led_cdev->max_brightness);
 }
 
 static struct led_trigger defon_led_trigger = {
diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
index c86c418..4cc7040 100644
--- a/drivers/leds/trigger/ledtrig-gpio.c
+++ b/drivers/leds/trigger/ledtrig-gpio.c
@@ -54,12 +54,12 @@ static void gpio_trig_work(struct work_struct *work)
 
 	if (tmp) {
 		if (gpio_data->desired_brightness)
-			__led_set_brightness(gpio_data->led,
+			led_set_brightness_async(gpio_data->led,
 					   gpio_data->desired_brightness);
 		else
-			__led_set_brightness(gpio_data->led, LED_FULL);
+			led_set_brightness_async(gpio_data->led, LED_FULL);
 	} else {
-		__led_set_brightness(gpio_data->led, LED_OFF);
+		led_set_brightness_async(gpio_data->led, LED_OFF);
 	}
 }
 
diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c
index 5c8464a..fea6871 100644
--- a/drivers/leds/trigger/ledtrig-heartbeat.c
+++ b/drivers/leds/trigger/ledtrig-heartbeat.c
@@ -74,7 +74,7 @@ static void led_heartbeat_function(unsigned long data)
 		break;
 	}
 
-	__led_set_brightness(led_cdev, brightness);
+	led_set_brightness_async(led_cdev, brightness);
 	mod_timer(&heartbeat_data->timer, jiffies + delay);
 }
 
diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
index cb4c746..fbd02cd 100644
--- a/drivers/leds/trigger/ledtrig-oneshot.c
+++ b/drivers/leds/trigger/ledtrig-oneshot.c
@@ -63,9 +63,9 @@ static ssize_t led_invert_store(struct device *dev,
 	oneshot_data->invert = !!state;
 
 	if (oneshot_data->invert)
-		__led_set_brightness(led_cdev, LED_FULL);
+		led_set_brightness_async(led_cdev, LED_FULL);
 	else
-		__led_set_brightness(led_cdev, LED_OFF);
+		led_set_brightness_async(led_cdev, LED_OFF);
 
 	return size;
 }
diff --git a/drivers/leds/trigger/ledtrig-transient.c b/drivers/leds/trigger/ledtrig-transient.c
index e5abc00..3c34de4 100644
--- a/drivers/leds/trigger/ledtrig-transient.c
+++ b/drivers/leds/trigger/ledtrig-transient.c
@@ -41,7 +41,7 @@ static void transient_timer_function(unsigned long data)
 	struct transient_trig_data *transient_data = led_cdev->trigger_data;
 
 	transient_data->activate = 0;
-	__led_set_brightness(led_cdev, transient_data->restore_state);
+	led_set_brightness_async(led_cdev, transient_data->restore_state);
 }
 
 static ssize_t transient_activate_show(struct device *dev,
@@ -72,7 +72,8 @@ static ssize_t transient_activate_store(struct device *dev,
 	if (state == 0 && transient_data->activate == 1) {
 		del_timer(&transient_data->timer);
 		transient_data->activate = state;
-		__led_set_brightness(led_cdev, transient_data->restore_state);
+		led_set_brightness_async(led_cdev,
+					transient_data->restore_state);
 		return size;
 	}
 
@@ -80,7 +81,7 @@ static ssize_t transient_activate_store(struct device *dev,
 	if (state == 1 && transient_data->activate == 0 &&
 	    transient_data->duration != 0) {
 		transient_data->activate = state;
-		__led_set_brightness(led_cdev, transient_data->state);
+		led_set_brightness_async(led_cdev, transient_data->state);
 		transient_data->restore_state =
 		    (transient_data->state == LED_FULL) ? LED_OFF : LED_FULL;
 		mod_timer(&transient_data->timer,
@@ -203,7 +204,8 @@ static void transient_trig_deactivate(struct led_classdev *led_cdev)
 
 	if (led_cdev->activated) {
 		del_timer_sync(&transient_data->timer);
-		__led_set_brightness(led_cdev, transient_data->restore_state);
+		led_set_brightness_async(led_cdev,
+					transient_data->restore_state);
 		device_remove_file(led_cdev->dev, &dev_attr_activate);
 		device_remove_file(led_cdev->dev, &dev_attr_duration);
 		device_remove_file(led_cdev->dev, &dev_attr_state);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 3737a5a..cfceef3 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -44,11 +44,19 @@ struct led_classdev {
 #define LED_BLINK_ONESHOT_STOP	(1 << 18)
 #define LED_BLINK_INVERT	(1 << 19)
 #define LED_SYSFS_DISABLE	(1 << 20)
+#define SET_BRIGHTNESS_ASYNC	(1 << 21)
+#define SET_BRIGHTNESS_SYNC	(1 << 22)
 
 	/* Set LED brightness level */
 	/* Must not sleep, use a workqueue if needed */
 	void		(*brightness_set)(struct led_classdev *led_cdev,
 					  enum led_brightness brightness);
+	/*
+	 * Set LED brightness level immediately - it can block the caller for
+	 * the time required for accessing a LED device register.
+	 */
+	int		(*brightness_set_sync)(struct led_classdev *led_cdev,
+					enum led_brightness brightness);
 	/* Get LED brightness level */
 	enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
 
-- 
1.7.9.5


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

* [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem
  2014-11-12 16:09 [PATCH/RFC v7 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
  2014-11-12 16:09 ` [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way Jacek Anaszewski
@ 2014-11-12 16:09 ` Jacek Anaszewski
  2014-11-27  8:43   ` Sakari Ailus
  2014-11-12 16:09 ` [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension Jacek Anaszewski
  2 siblings, 1 reply; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-12 16:09 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-media, sakari.ailus, kyungmin.park, b.zolnierkie,
	Jacek Anaszewski, Bryan Wu, Richard Purdie

Some LED devices support two operation modes - torch and flash.
This patch provides support for flash LED devices in the LED subsystem
by introducing new sysfs attributes and kernel internal interface.
The attributes being introduced are: flash_brightness, flash_strobe,
flash_timeout, max_flash_timeout, max_flash_brightness, flash_fault,
indicator_brightness and  max_indicator_brightness. All the flash
related features are placed in a separate module.

The modifications aim to be compatible with V4L2 framework requirements
related to the flash devices management. The design assumes that V4L2
sub-device can take of the LED class device control and communicate
with it through the kernel internal interface. When V4L2 Flash sub-device
file is opened, the LED class device sysfs interface is made
unavailable.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
---
 drivers/leds/Kconfig            |   11 +
 drivers/leds/Makefile           |    1 +
 drivers/leds/led-class-flash.c  |  511 +++++++++++++++++++++++++++++++++++++++
 drivers/leds/led-class.c        |    4 +
 include/linux/led-class-flash.h |  229 ++++++++++++++++++
 include/linux/leds.h            |    3 +
 6 files changed, 759 insertions(+)
 create mode 100644 drivers/leds/led-class-flash.c
 create mode 100644 include/linux/led-class-flash.h

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 808095b..50c4370 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -19,6 +19,17 @@ config LEDS_CLASS
 	  This option enables the led sysfs class in /sys/class/leds.  You'll
 	  need this to do anything useful with LEDs.  If unsure, say N.
 
+config LEDS_CLASS_FLASH
+	tristate "LED Flash Class Support"
+	depends on LEDS_CLASS
+	depends on OF
+	help
+	  This option enables the flash led sysfs class in /sys/class/leds.
+	  It wrapps LED Class and adds flash LEDs specific sysfs attributes
+	  and kernel internal API to it. You'll need this to provide support
+	  for the flash related features of a LED device. It can be built
+	  as a module.
+
 comment "LED drivers"
 
 config LEDS_88PM860X
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index a2b1647..73c0cec 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -2,6 +2,7 @@
 # LED Core
 obj-$(CONFIG_NEW_LEDS)			+= led-core.o
 obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
+obj-$(CONFIG_LEDS_CLASS_FLASH)		+= led-class-flash.o
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
new file mode 100644
index 0000000..bec99f6
--- /dev/null
+++ b/drivers/leds/led-class-flash.c
@@ -0,0 +1,511 @@
+/*
+ * LED Flash Class interface
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd.
+ * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/led-class-flash.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include "leds.h"
+
+#define has_flash_op(flash, op)				\
+	(flash && flash->ops->op)
+
+#define call_flash_op(flash, op, args...)		\
+	((has_flash_op(flash, op)) ?			\
+			(flash->ops->op(flash, args)) :	\
+			-EINVAL)
+
+static ssize_t flash_brightness_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+	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, 10, &state);
+	if (ret)
+		goto unlock;
+
+	ret = led_set_flash_brightness(flash, state);
+	if (ret < 0)
+		goto unlock;
+
+	ret = size;
+unlock:
+	mutex_unlock(&led_cdev->led_access);
+	return ret;
+}
+
+static ssize_t flash_brightness_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	/* no lock needed for this */
+	led_update_flash_brightness(flash);
+
+	return sprintf(buf, "%u\n", flash->brightness.val);
+}
+static DEVICE_ATTR_RW(flash_brightness);
+
+static ssize_t max_flash_brightness_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	return sprintf(buf, "%u\n", flash->brightness.max);
+}
+static DEVICE_ATTR_RO(max_flash_brightness);
+
+static ssize_t indicator_brightness_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+	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, 10, &state);
+	if (ret)
+		goto unlock;
+
+	ret = led_set_indicator_brightness(flash, state);
+	if (ret < 0)
+		goto unlock;
+
+	ret = size;
+unlock:
+	mutex_unlock(&led_cdev->led_access);
+	return ret;
+}
+
+static ssize_t indicator_brightness_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	/* no lock needed for this */
+	led_update_indicator_brightness(flash);
+
+	return sprintf(buf, "%u\n", flash->indicator_brightness->val);
+}
+static DEVICE_ATTR_RW(indicator_brightness);
+
+static ssize_t max_indicator_brightness_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	return sprintf(buf, "%u\n", flash->indicator_brightness->max);
+}
+static DEVICE_ATTR_RO(max_indicator_brightness);
+
+static ssize_t flash_strobe_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+	unsigned long state;
+	ssize_t ret = -EINVAL;
+
+	mutex_lock(&led_cdev->led_access);
+
+	if (led_sysfs_is_disabled(led_cdev)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = kstrtoul(buf, 10, &state);
+	if (ret)
+		goto unlock;
+
+	if (state < 0 || state > 1) {
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	ret = led_set_flash_strobe(flash, state);
+	if (ret < 0)
+		goto unlock;
+	ret = size;
+unlock:
+	mutex_unlock(&led_cdev->led_access);
+	return ret;
+}
+
+static ssize_t flash_strobe_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+	bool state;
+	int ret;
+
+	/* no lock needed for this */
+	ret = led_get_flash_strobe(flash, &state);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%u\n", state);
+}
+static DEVICE_ATTR_RW(flash_strobe);
+
+static ssize_t flash_timeout_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+	unsigned long flash_timeout;
+	ssize_t ret;
+
+	mutex_lock(&led_cdev->led_access);
+
+	if (led_sysfs_is_disabled(led_cdev)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = kstrtoul(buf, 10, &flash_timeout);
+	if (ret)
+		goto unlock;
+
+	ret = led_set_flash_timeout(flash, flash_timeout);
+	if (ret < 0)
+		goto unlock;
+
+	ret = size;
+unlock:
+	mutex_unlock(&led_cdev->led_access);
+	return ret;
+}
+
+static ssize_t flash_timeout_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	return sprintf(buf, "%u\n", flash->timeout.val);
+}
+static DEVICE_ATTR_RW(flash_timeout);
+
+static ssize_t max_flash_timeout_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	return sprintf(buf, "%u\n", flash->timeout.max);
+}
+static DEVICE_ATTR_RO(max_flash_timeout);
+
+static ssize_t flash_fault_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+	u32 fault;
+	int ret;
+
+	ret = led_get_flash_fault(flash, &fault);
+	if (ret < 0)
+		return -EINVAL;
+
+	return sprintf(buf, "0x%8.8x\n", fault);
+}
+static DEVICE_ATTR_RO(flash_fault);
+
+static struct attribute *led_flash_strobe_attrs[] = {
+	&dev_attr_flash_strobe.attr,
+	NULL,
+};
+
+static struct attribute *led_flash_indicator_attrs[] = {
+	&dev_attr_indicator_brightness.attr,
+	&dev_attr_max_indicator_brightness.attr,
+	NULL,
+};
+
+static struct attribute *led_flash_timeout_attrs[] = {
+	&dev_attr_flash_timeout.attr,
+	&dev_attr_max_flash_timeout.attr,
+	NULL,
+};
+
+static struct attribute *led_flash_brightness_attrs[] = {
+	&dev_attr_flash_brightness.attr,
+	&dev_attr_max_flash_brightness.attr,
+	NULL,
+};
+
+static struct attribute *led_flash_fault_attrs[] = {
+	&dev_attr_flash_fault.attr,
+	NULL,
+};
+
+static const struct attribute_group led_flash_strobe_group = {
+	.attrs = led_flash_strobe_attrs,
+};
+
+static const struct attribute_group led_flash_indicator_group = {
+	.attrs = led_flash_indicator_attrs,
+};
+
+static const struct attribute_group led_flash_timeout_group = {
+	.attrs = led_flash_timeout_attrs,
+};
+
+static const struct attribute_group led_flash_brightness_group = {
+	.attrs = led_flash_brightness_attrs,
+};
+
+static const struct attribute_group led_flash_fault_group = {
+	.attrs = led_flash_fault_attrs,
+};
+
+static const struct attribute_group *flash_groups[] = {
+	&led_flash_strobe_group,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+};
+
+static void led_flash_resume(struct led_classdev *led_cdev)
+{
+	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+	call_flash_op(flash, flash_brightness_set, flash->brightness.val);
+	call_flash_op(flash, timeout_set, flash->timeout.val);
+	call_flash_op(flash, indicator_brightness_set,
+				flash->indicator_brightness->val);
+}
+
+static void led_flash_init_sysfs_groups(struct led_classdev_flash *flash)
+{
+	struct led_classdev *led_cdev = &flash->led_cdev;
+	const struct led_flash_ops *ops = flash->ops;
+	int num_sysfs_groups = 1;
+
+	if (led_cdev->flags & LED_DEV_CAP_INDICATOR)
+		flash_groups[num_sysfs_groups++] = &led_flash_indicator_group;
+
+	if (ops->flash_brightness_set)
+		flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
+
+	if (ops->timeout_set)
+		flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
+
+	if (ops->fault_get)
+		flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
+
+	led_cdev->groups = flash_groups;
+}
+
+int led_classdev_flash_register(struct device *parent,
+				struct led_classdev_flash *flash)
+{
+	struct led_classdev *led_cdev;
+	const struct led_flash_ops *ops;
+	int ret;
+
+	if (!flash)
+		return -EINVAL;
+
+	led_cdev = &flash->led_cdev;
+
+	if (led_cdev->flags & LED_DEV_CAP_FLASH) {
+		if (!led_cdev->brightness_set_sync)
+			return -EINVAL;
+
+		ops = flash->ops;
+		if (!ops || !ops->strobe_set)
+			return -EINVAL;
+
+		if ((led_cdev->flags & LED_DEV_CAP_INDICATOR) &&
+		    (!flash->indicator_brightness ||
+		     !ops->indicator_brightness_set))
+			return -EINVAL;
+
+		led_cdev->flash_resume = led_flash_resume;
+	}
+
+	/* Select the sysfs attributes to be created for the device */
+	led_flash_init_sysfs_groups(flash);
+
+	/* Register led class device */
+	ret = led_classdev_register(parent, led_cdev);
+	if (ret < 0)
+		return ret;
+
+	/* Setting a torch brightness needs to have immediate effect */
+	led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
+	led_cdev->flags |= SET_BRIGHTNESS_SYNC;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(led_classdev_flash_register);
+
+void led_classdev_flash_unregister(struct led_classdev_flash *flash)
+{
+	if (!flash)
+		return;
+
+	led_classdev_unregister(&flash->led_cdev);
+}
+EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
+
+int led_set_flash_strobe(struct led_classdev_flash *flash, bool state)
+{
+	return call_flash_op(flash, strobe_set, state);
+}
+EXPORT_SYMBOL_GPL(led_set_flash_strobe);
+
+int led_get_flash_strobe(struct led_classdev_flash *flash, bool *state)
+{
+	return call_flash_op(flash, strobe_get, state);
+}
+EXPORT_SYMBOL_GPL(led_get_flash_strobe);
+
+static void led_clamp_align(struct led_flash_setting *s)
+{
+	u32 v, offset;
+
+	v = s->val + s->step / 2;
+	v = clamp(v, s->min, s->max);
+	offset = v - s->min;
+	offset = s->step * (offset / s->step);
+	s->val = s->min + offset;
+}
+
+int led_set_flash_timeout(struct led_classdev_flash *flash, u32 timeout)
+{
+	struct led_classdev *led_cdev = &flash->led_cdev;
+	struct led_flash_setting *s = &flash->timeout;
+	int ret = 0;
+
+	s->val = timeout;
+	led_clamp_align(s);
+
+	if (!(led_cdev->flags & LED_SUSPENDED))
+		ret = call_flash_op(flash, timeout_set, s->val);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(led_set_flash_timeout);
+
+int led_get_flash_fault(struct led_classdev_flash *flash, u32 *fault)
+{
+	return call_flash_op(flash, fault_get, fault);
+}
+EXPORT_SYMBOL_GPL(led_get_flash_fault);
+
+int led_set_flash_brightness(struct led_classdev_flash *flash,
+				u32 brightness)
+{
+	struct led_classdev *led_cdev = &flash->led_cdev;
+	struct led_flash_setting *s = &flash->brightness;
+	int ret = 0;
+
+	s->val = brightness;
+	led_clamp_align(s);
+
+	if (!(led_cdev->flags & LED_SUSPENDED))
+		ret = call_flash_op(flash, flash_brightness_set, s->val);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(led_set_flash_brightness);
+
+int led_update_flash_brightness(struct led_classdev_flash *flash)
+{
+	struct led_flash_setting *s = &flash->brightness;
+	u32 brightness;
+	int ret = 0;
+
+	if (has_flash_op(flash, flash_brightness_get)) {
+		ret = call_flash_op(flash, flash_brightness_get,
+						&brightness);
+		if (ret < 0)
+			return ret;
+		s->val = brightness;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(led_update_flash_brightness);
+
+int led_set_indicator_brightness(struct led_classdev_flash *flash,
+					u32 brightness)
+{
+	struct led_classdev *led_cdev = &flash->led_cdev;
+	struct led_flash_setting *s = flash->indicator_brightness;
+	int ret = 0;
+
+	if (!s)
+		return -EINVAL;
+
+	s->val = brightness;
+	led_clamp_align(s);
+
+	if (!(led_cdev->flags & LED_SUSPENDED))
+		ret = call_flash_op(flash, indicator_brightness_set, s->val);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(led_set_indicator_brightness);
+
+int led_update_indicator_brightness(struct led_classdev_flash *flash)
+{
+	struct led_flash_setting *s = flash->indicator_brightness;
+	u32 brightness;
+	int ret = 0;
+
+	if (!s)
+		return -EINVAL;
+
+	if (has_flash_op(flash, indicator_brightness_get)) {
+		ret = call_flash_op(flash, indicator_brightness_get,
+							&brightness);
+		if (ret < 0)
+			return ret;
+		s->val = brightness;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(led_update_indicator_brightness);
+
+MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LED Flash Class Interface");
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index dbeebac..02564c5 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -179,6 +179,10 @@ EXPORT_SYMBOL_GPL(led_classdev_suspend);
 void led_classdev_resume(struct led_classdev *led_cdev)
 {
 	led_cdev->brightness_set(led_cdev, led_cdev->brightness);
+
+	if (led_cdev->flash_resume)
+		led_cdev->flash_resume(led_cdev);
+
 	led_cdev->flags &= ~LED_SUSPENDED;
 }
 EXPORT_SYMBOL_GPL(led_classdev_resume);
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
new file mode 100644
index 0000000..df95b8a
--- /dev/null
+++ b/include/linux/led-class-flash.h
@@ -0,0 +1,229 @@
+/*
+ * LED Flash Class interface
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd.
+ * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#ifndef __LINUX_FLASH_LEDS_H_INCLUDED
+#define __LINUX_FLASH_LEDS_H_INCLUDED
+
+#include <linux/leds.h>
+
+struct led_classdev_flash;
+struct device_node;
+
+/*
+ * Supported led fault bits - must be kept in synch
+ * with V4L2_FLASH_FAULT bits.
+ */
+#define LED_FAULT_OVER_VOLTAGE		 V4L2_FLASH_FAULT_OVER_VOLTAGE
+#define LED_FAULT_TIMEOUT		 V4L2_FLASH_FAULT_TIMEOUT
+#define LED_FAULT_OVER_TEMPERATURE	 V4L2_FLASH_FAULT_OVER_TEMPERATURE
+#define LED_FAULT_SHORT_CIRCUIT		 V4L2_FLASH_FAULT_SHORT_CIRCUIT
+#define LED_FAULT_OVER_CURRENT		 V4L2_FLASH_FAULT_OVER_CURRENT
+#define LED_FAULT_INDICATOR		 V4L2_FLASH_FAULT_INDICATOR
+#define LED_FAULT_UNDER_VOLTAGE		 V4L2_FLASH_FAULT_UNDER_VOLTAGE
+#define LED_FAULT_INPUT_VOLTAGE		 V4L2_FLASH_FAULT_INPUT_VOLTAGE
+#define LED_FAULT_LED_OVER_TEMPERATURE	 V4L2_FLASH_OVER_TEMPERATURE
+
+struct led_flash_ops {
+	/* set flash brightness */
+	int (*flash_brightness_set)(struct led_classdev_flash *flash,
+					u32 brightness);
+	/* get flash brightness */
+	int (*flash_brightness_get)(struct led_classdev_flash *flash,
+					u32 *brightness);
+	/* set flash indicator brightness */
+	int (*indicator_brightness_set)(struct led_classdev_flash *flash,
+					u32 brightness);
+	/* get flash indicator brightness */
+	int (*indicator_brightness_get)(struct led_classdev_flash *flash,
+					u32 *brightness);
+	/* set flash strobe state */
+	int (*strobe_set)(struct led_classdev_flash *flash, bool state);
+	/* get flash strobe state */
+	int (*strobe_get)(struct led_classdev_flash *flash, bool *state);
+	/* set flash timeout */
+	int (*timeout_set)(struct led_classdev_flash *flash, u32 timeout);
+	/* get the flash LED fault */
+	int (*fault_get)(struct led_classdev_flash *flash, u32 *fault);
+};
+
+/*
+ * Current value of a flash setting along
+ * with its constraints.
+ */
+struct led_flash_setting {
+	/* maximum allowed value */
+	u32 min;
+	/* maximum allowed value */
+	u32 max;
+	/* step value */
+	u32 step;
+	/* current value */
+	u32 val;
+};
+
+/*
+ * Aggregated flash settings - designed for ease
+ * of passing initialization data to the clients
+ * wrapping a LED Flash class device.
+ */
+struct led_flash_config {
+	struct led_flash_setting torch_brightness;
+	struct led_flash_setting flash_brightness;
+	struct led_flash_setting indicator_brightness;
+	struct led_flash_setting flash_timeout;
+	u32 flash_faults;
+};
+
+struct led_classdev_flash {
+	/* led class device */
+	struct led_classdev led_cdev;
+
+	/* flash led specific ops */
+	const struct led_flash_ops *ops;
+
+	/* flash brightness value in microamperes along with its constraints */
+	struct led_flash_setting brightness;
+
+	/* timeout value in microseconds along with its constraints */
+	struct led_flash_setting timeout;
+
+	/*
+	 * Indicator brightness value in microamperes along with
+	 * its constraints - this is an optional setting and must
+	 * be allocated by the driver if the device supports privacy
+	 * indicator led.
+	 */
+	struct led_flash_setting *indicator_brightness;
+};
+
+static inline struct led_classdev_flash *lcdev_to_flash(
+						struct led_classdev *lcdev)
+{
+	return container_of(lcdev, struct led_classdev_flash, led_cdev);
+}
+
+/**
+ * led_classdev_flash_register - register a new object of led_classdev class
+				 with support for flash LEDs
+ * @parent: the flash LED to register
+ * @flash: the led_classdev_flash structure for this device
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+int led_classdev_flash_register(struct device *parent,
+				struct led_classdev_flash *flash);
+
+/**
+ * led_classdev_flash_unregister - unregisters an object of led_classdev class
+				   with support for flash LEDs
+ * @flash: the flash LED to unregister
+ *
+ * Unregister a previously registered via led_classdev_flash_register object
+ */
+void led_classdev_flash_unregister(struct led_classdev_flash *flash);
+
+/**
+ * led_set_flash_strobe - setup flash strobe
+ * @flash: the flash LED to set strobe on
+ * @state: 1 - strobe flash, 0 - stop flash strobe
+ *
+ * Strobe the flash LED.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_set_flash_strobe(struct led_classdev_flash *flash,
+				bool state);
+
+/**
+ * led_get_flash_strobe - get flash strobe status
+ * @flash: the flash LED to query
+ * @state: 1 - flash is strobing, 0 - flash is off
+ *
+ * Check whether the flash is strobing at the moment.
+ *
+u* Returns: 0 on success or negative error value on failure
+ */
+extern int led_get_flash_strobe(struct led_classdev_flash *flash,
+				bool *state);
+/**
+ * led_set_flash_brightness - set flash LED brightness
+ * @flash: the flash LED to set
+ * @brightness: the brightness to set it to
+ *
+ * Set a flash LED's brightness.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_set_flash_brightness(struct led_classdev_flash *flash,
+					u32 brightness);
+
+/**
+ * led_update_flash_brightness - update flash LED brightness
+ * @flash: the flash LED to query
+ *
+ * Get a flash LED's current brightness and update led_flash->brightness
+ * member with the obtained value.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_update_flash_brightness(struct led_classdev_flash *flash);
+
+/**
+ * led_set_flash_timeout - set flash LED timeout
+ * @flash: the flash LED to set
+ * @timeout: the flash timeout to set it to
+ *
+ * Set the flash strobe duration. The duration set by the driver
+ * is returned in the timeout argument and may differ from the
+ * one that was originally passed.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_set_flash_timeout(struct led_classdev_flash *flash,
+					u32 timeout);
+
+/**
+ * led_get_flash_fault - get the flash LED fault
+ * @flash: the flash LED to query
+ * @fault: bitmask containing flash faults
+ *
+ * Get the flash LED fault.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_get_flash_fault(struct led_classdev_flash *flash,
+					u32 *fault);
+
+/**
+ * led_set_indicator_brightness - set indicator LED brightness
+ * @flash: the flash LED to set
+ * @brightness: the brightness to set it to
+ *
+ * Set an indicator LED's brightness.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_set_indicator_brightness(struct led_classdev_flash *flash,
+					u32 led_brightness);
+
+/**
+ * led_update_indicator_brightness - update flash indicator LED brightness
+ * @flash: the flash LED to query
+ *
+ * Get a flash indicator LED's current brightness and update
+ * led_flash->indicator_brightness member with the obtained value.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_update_indicator_brightness(struct led_classdev_flash *flash);
+
+
+#endif	/* __LINUX_FLASH_LEDS_H_INCLUDED */
diff --git a/include/linux/leds.h b/include/linux/leds.h
index cfceef3..b9a2797 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -46,6 +46,8 @@ struct led_classdev {
 #define LED_SYSFS_DISABLE	(1 << 20)
 #define SET_BRIGHTNESS_ASYNC	(1 << 21)
 #define SET_BRIGHTNESS_SYNC	(1 << 22)
+#define LED_DEV_CAP_FLASH	(1 << 23)
+#define LED_DEV_CAP_INDICATOR	(1 << 24)
 
 	/* Set LED brightness level */
 	/* Must not sleep, use a workqueue if needed */
@@ -81,6 +83,7 @@ struct led_classdev {
 	unsigned long		 blink_delay_on, blink_delay_off;
 	struct timer_list	 blink_timer;
 	int			 blink_brightness;
+	void			(*flash_resume)(struct led_classdev *led_cdev);
 
 	struct work_struct	set_brightness_work;
 	int			delayed_set_value;
-- 
1.7.9.5


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

* [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension
  2014-11-12 16:09 [PATCH/RFC v7 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
  2014-11-12 16:09 ` [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way Jacek Anaszewski
  2014-11-12 16:09 ` [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
@ 2014-11-12 16:09 ` Jacek Anaszewski
  2014-11-13 18:58   ` Bryan Wu
  2 siblings, 1 reply; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-12 16:09 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-media, sakari.ailus, kyungmin.park, b.zolnierkie,
	Jacek Anaszewski, Bryan Wu, Richard Purdie

The documentation being added contains overall description of the
LED Flash Class and the related sysfs attributes.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
---
 Documentation/leds/leds-class-flash.txt |   39 +++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/leds/leds-class-flash.txt

diff --git a/Documentation/leds/leds-class-flash.txt b/Documentation/leds/leds-class-flash.txt
new file mode 100644
index 0000000..0164329
--- /dev/null
+++ b/Documentation/leds/leds-class-flash.txt
@@ -0,0 +1,39 @@
+
+Flash LED handling under Linux
+==============================
+
+Some LED devices support two modes - torch and flash. In order to enable
+support for flash LEDs CONFIG_LEDS_CLASS_FLASH symbol must be defined
+in the kernel config. A flash LED driver must register in the LED subsystem
+with led_classdev_flash_register to gain flash capabilities.
+
+Following sysfs attributes are exposed for controlling flash led devices:
+
+	- flash_brightness - flash LED brightness in microamperes (RW)
+	- max_flash_brightness - maximum available flash LED brightness (RO)
+	- indicator_brightness - privacy LED brightness in microamperes (RW)
+	- max_indicator_brightness - maximum privacy LED brightness in
+				     microamperes (RO)
+	- flash_timeout - flash strobe duration in microseconds (RW)
+	- max_flash_timeout - maximum available flash strobe duration (RO)
+	- flash_strobe - flash strobe state (RW)
+	- flash_fault - bitmask of flash faults that may have occurred,
+			possible flags are:
+		* 0x01 - flash controller voltage to the flash LED has exceeded
+			 the limit specific to the flash controller
+		* 0x02 - the flash strobe was still on when the timeout set by
+			 the user has expired; not all flash controllers may
+			 set this in all such conditions
+		* 0x04 - the flash controller has overheated
+		* 0x08 - the short circuit protection of the flash controller
+			 has been triggered
+		* 0x10 - current in the LED power supply has exceeded the limit
+			 specific to the flash controller
+		* 0x40 - flash controller voltage to the flash LED has been
+			 below the minimum limit specific to the flash
+		* 0x80 - the input voltage of the flash controller is below
+			 the limit under which strobing the flash at full
+			 current will not be possible. The condition persists
+			 until this flag is no longer set
+		* 0x100 - the temperature of the LED has exceeded its allowed
+			  upper limit
-- 
1.7.9.5


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

* Re: [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way
  2014-11-12 16:09 ` [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way Jacek Anaszewski
@ 2014-11-13 18:43   ` Bryan Wu
  0 siblings, 0 replies; 9+ messages in thread
From: Bryan Wu @ 2014-11-13 18:43 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Linux LED Subsystem, linux-media, sakari.ailus, Kyungmin Park,
	b.zolnierkie, Richard Purdie

On Wed, Nov 12, 2014 at 8:09 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> There are use cases when setting a LED brightness has to
> have immediate effect (e.g. setting a torch LED brightness).
> This patch extends LED subsystem to support such operations.
> The LED subsystem internal API __led_set_brightness is changed
> to led_set_brightness_async and new led_set_brightness_sync API
> is added.
>

Great, I do like this patch and just one pick coding style as below.

Thanks,
-Bryan

> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> ---
>  drivers/leds/led-class.c                  |   10 ++++++----
>  drivers/leds/led-core.c                   |   19 ++++++++++++++++---
>  drivers/leds/leds.h                       |   20 ++++++++++++++++----
>  drivers/leds/trigger/ledtrig-backlight.c  |    8 ++++----
>  drivers/leds/trigger/ledtrig-default-on.c |    2 +-
>  drivers/leds/trigger/ledtrig-gpio.c       |    6 +++---
>  drivers/leds/trigger/ledtrig-heartbeat.c  |    2 +-
>  drivers/leds/trigger/ledtrig-oneshot.c    |    4 ++--
>  drivers/leds/trigger/ledtrig-transient.c  |   10 ++++++----
>  include/linux/leds.h                      |    8 ++++++++
>  10 files changed, 63 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 65722de..dbeebac 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -55,7 +55,7 @@ static ssize_t brightness_store(struct device *dev,
>
>         if (state == LED_OFF)
>                 led_trigger_remove(led_cdev);
> -       __led_set_brightness(led_cdev, state);
> +       led_set_brightness(led_cdev, state);
>
>         ret = size;
>  unlock:
> @@ -109,7 +109,7 @@ static void led_timer_function(unsigned long data)
>         unsigned long delay;
>
>         if (!led_cdev->blink_delay_on || !led_cdev->blink_delay_off) {
> -               __led_set_brightness(led_cdev, LED_OFF);
> +               led_set_brightness_async(led_cdev, LED_OFF);
>                 return;
>         }
>
> @@ -132,7 +132,7 @@ static void led_timer_function(unsigned long data)
>                 delay = led_cdev->blink_delay_off;
>         }
>
> -       __led_set_brightness(led_cdev, brightness);
> +       led_set_brightness_async(led_cdev, brightness);
>
>         /* Return in next iteration if led is in one-shot mode and we are in
>          * the final blink state so that the led is toggled each delay_on +
> @@ -158,7 +158,7 @@ static void set_brightness_delayed(struct work_struct *ws)
>
>         led_stop_software_blink(led_cdev);
>
> -       __led_set_brightness(led_cdev, led_cdev->delayed_set_value);
> +       led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
>  }
>
>  /**
> @@ -233,6 +233,8 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
>         if (!led_cdev->max_brightness)
>                 led_cdev->max_brightness = LED_FULL;
>
> +       led_cdev->flags |= SET_BRIGHTNESS_ASYNC;
> +
>         led_update_brightness(led_cdev);
>
>         INIT_WORK(&led_cdev->set_brightness_work, set_brightness_delayed);
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index be6d9fa..a745e32 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -42,13 +42,13 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
>
>         /* never on - just set to off */
>         if (!delay_on) {
> -               __led_set_brightness(led_cdev, LED_OFF);
> +               led_set_brightness_async(led_cdev, LED_OFF);
>                 return;
>         }
>
>         /* never off - just set to brightness */
>         if (!delay_off) {
> -               __led_set_brightness(led_cdev, led_cdev->blink_brightness);
> +               led_set_brightness_async(led_cdev, led_cdev->blink_brightness);
>                 return;
>         }
>
> @@ -117,6 +117,8 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
>  void led_set_brightness(struct led_classdev *led_cdev,
>                         enum led_brightness brightness)
>  {
> +       int ret = 0;
> +
>         /* delay brightness setting if need to stop soft-blink timer */
>         if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
>                 led_cdev->delayed_set_value = brightness;
> @@ -124,7 +126,18 @@ void led_set_brightness(struct led_classdev *led_cdev,
>                 return;
>         }
>
> -       __led_set_brightness(led_cdev, brightness);
> +       if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
> +               led_set_brightness_async(led_cdev, brightness);
> +               return;
> +       } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC) {
> +               ret = led_set_brightness_sync(led_cdev, brightness);
> +       } else {
> +               ret = -EINVAL;
> +       }
> +

Since there is only one line in the {}, we don't need {} here:

+       if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
+               led_set_brightness_async(led_cdev, brightness);
+               return;
+       } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC)
+               ret = led_set_brightness_sync(led_cdev, brightness);
+       else
+               ret = -EINVAL;
+



> +       if (ret < 0)
> +               dev_dbg(led_cdev->dev, "Setting LED brightness failed (%d)\n",
> +                       ret);
>  }
>  EXPORT_SYMBOL(led_set_brightness);
>
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index 4c50365..2348dbd 100644
> --- a/drivers/leds/leds.h
> +++ b/drivers/leds/leds.h
> @@ -17,16 +17,28 @@
>  #include <linux/rwsem.h>
>  #include <linux/leds.h>
>
> -static inline void __led_set_brightness(struct led_classdev *led_cdev,
> +static inline void led_set_brightness_async(struct led_classdev *led_cdev,
>                                         enum led_brightness value)
>  {
> -       if (value > led_cdev->max_brightness)
> -               value = led_cdev->max_brightness;
> -       led_cdev->brightness = value;
> +       led_cdev->brightness = min(value, led_cdev->max_brightness);
> +
>         if (!(led_cdev->flags & LED_SUSPENDED))
>                 led_cdev->brightness_set(led_cdev, value);
>  }
>
> +static inline int led_set_brightness_sync(struct led_classdev *led_cdev,
> +                                       enum led_brightness value)
> +{
> +       int ret = 0;
> +
> +       led_cdev->brightness = min(value, led_cdev->max_brightness);
> +
> +       if (!(led_cdev->flags & LED_SUSPENDED))
> +               ret = led_cdev->brightness_set_sync(led_cdev,
> +                                               led_cdev->brightness);
> +       return ret;
> +}
> +
>  static inline int led_get_brightness(struct led_classdev *led_cdev)
>  {
>         return led_cdev->brightness;
> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
> index 47e55aa..59eca17 100644
> --- a/drivers/leds/trigger/ledtrig-backlight.c
> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> @@ -51,9 +51,9 @@ static int fb_notifier_callback(struct notifier_block *p,
>
>         if ((n->old_status == UNBLANK) ^ n->invert) {
>                 n->brightness = led->brightness;
> -               __led_set_brightness(led, LED_OFF);
> +               led_set_brightness_async(led, LED_OFF);
>         } else {
> -               __led_set_brightness(led, n->brightness);
> +               led_set_brightness_async(led, n->brightness);
>         }
>
>         n->old_status = new_status;
> @@ -89,9 +89,9 @@ static ssize_t bl_trig_invert_store(struct device *dev,
>
>         /* After inverting, we need to update the LED. */
>         if ((n->old_status == BLANK) ^ n->invert)
> -               __led_set_brightness(led, LED_OFF);
> +               led_set_brightness_async(led, LED_OFF);
>         else
> -               __led_set_brightness(led, n->brightness);
> +               led_set_brightness_async(led, n->brightness);
>
>         return num;
>  }
> diff --git a/drivers/leds/trigger/ledtrig-default-on.c b/drivers/leds/trigger/ledtrig-default-on.c
> index 81a91be..6f38f88 100644
> --- a/drivers/leds/trigger/ledtrig-default-on.c
> +++ b/drivers/leds/trigger/ledtrig-default-on.c
> @@ -19,7 +19,7 @@
>
>  static void defon_trig_activate(struct led_classdev *led_cdev)
>  {
> -       __led_set_brightness(led_cdev, led_cdev->max_brightness);
> +       led_set_brightness_async(led_cdev, led_cdev->max_brightness);
>  }
>
>  static struct led_trigger defon_led_trigger = {
> diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
> index c86c418..4cc7040 100644
> --- a/drivers/leds/trigger/ledtrig-gpio.c
> +++ b/drivers/leds/trigger/ledtrig-gpio.c
> @@ -54,12 +54,12 @@ static void gpio_trig_work(struct work_struct *work)
>
>         if (tmp) {
>                 if (gpio_data->desired_brightness)
> -                       __led_set_brightness(gpio_data->led,
> +                       led_set_brightness_async(gpio_data->led,
>                                            gpio_data->desired_brightness);
>                 else
> -                       __led_set_brightness(gpio_data->led, LED_FULL);
> +                       led_set_brightness_async(gpio_data->led, LED_FULL);
>         } else {
> -               __led_set_brightness(gpio_data->led, LED_OFF);
> +               led_set_brightness_async(gpio_data->led, LED_OFF);
>         }
>  }
>
> diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c
> index 5c8464a..fea6871 100644
> --- a/drivers/leds/trigger/ledtrig-heartbeat.c
> +++ b/drivers/leds/trigger/ledtrig-heartbeat.c
> @@ -74,7 +74,7 @@ static void led_heartbeat_function(unsigned long data)
>                 break;
>         }
>
> -       __led_set_brightness(led_cdev, brightness);
> +       led_set_brightness_async(led_cdev, brightness);
>         mod_timer(&heartbeat_data->timer, jiffies + delay);
>  }
>
> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
> index cb4c746..fbd02cd 100644
> --- a/drivers/leds/trigger/ledtrig-oneshot.c
> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
> @@ -63,9 +63,9 @@ static ssize_t led_invert_store(struct device *dev,
>         oneshot_data->invert = !!state;
>
>         if (oneshot_data->invert)
> -               __led_set_brightness(led_cdev, LED_FULL);
> +               led_set_brightness_async(led_cdev, LED_FULL);
>         else
> -               __led_set_brightness(led_cdev, LED_OFF);
> +               led_set_brightness_async(led_cdev, LED_OFF);
>
>         return size;
>  }
> diff --git a/drivers/leds/trigger/ledtrig-transient.c b/drivers/leds/trigger/ledtrig-transient.c
> index e5abc00..3c34de4 100644
> --- a/drivers/leds/trigger/ledtrig-transient.c
> +++ b/drivers/leds/trigger/ledtrig-transient.c
> @@ -41,7 +41,7 @@ static void transient_timer_function(unsigned long data)
>         struct transient_trig_data *transient_data = led_cdev->trigger_data;
>
>         transient_data->activate = 0;
> -       __led_set_brightness(led_cdev, transient_data->restore_state);
> +       led_set_brightness_async(led_cdev, transient_data->restore_state);
>  }
>
>  static ssize_t transient_activate_show(struct device *dev,
> @@ -72,7 +72,8 @@ static ssize_t transient_activate_store(struct device *dev,
>         if (state == 0 && transient_data->activate == 1) {
>                 del_timer(&transient_data->timer);
>                 transient_data->activate = state;
> -               __led_set_brightness(led_cdev, transient_data->restore_state);
> +               led_set_brightness_async(led_cdev,
> +                                       transient_data->restore_state);
>                 return size;
>         }
>
> @@ -80,7 +81,7 @@ static ssize_t transient_activate_store(struct device *dev,
>         if (state == 1 && transient_data->activate == 0 &&
>             transient_data->duration != 0) {
>                 transient_data->activate = state;
> -               __led_set_brightness(led_cdev, transient_data->state);
> +               led_set_brightness_async(led_cdev, transient_data->state);
>                 transient_data->restore_state =
>                     (transient_data->state == LED_FULL) ? LED_OFF : LED_FULL;
>                 mod_timer(&transient_data->timer,
> @@ -203,7 +204,8 @@ static void transient_trig_deactivate(struct led_classdev *led_cdev)
>
>         if (led_cdev->activated) {
>                 del_timer_sync(&transient_data->timer);
> -               __led_set_brightness(led_cdev, transient_data->restore_state);
> +               led_set_brightness_async(led_cdev,
> +                                       transient_data->restore_state);
>                 device_remove_file(led_cdev->dev, &dev_attr_activate);
>                 device_remove_file(led_cdev->dev, &dev_attr_duration);
>                 device_remove_file(led_cdev->dev, &dev_attr_state);
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 3737a5a..cfceef3 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -44,11 +44,19 @@ struct led_classdev {
>  #define LED_BLINK_ONESHOT_STOP (1 << 18)
>  #define LED_BLINK_INVERT       (1 << 19)
>  #define LED_SYSFS_DISABLE      (1 << 20)
> +#define SET_BRIGHTNESS_ASYNC   (1 << 21)
> +#define SET_BRIGHTNESS_SYNC    (1 << 22)
>
>         /* Set LED brightness level */
>         /* Must not sleep, use a workqueue if needed */
>         void            (*brightness_set)(struct led_classdev *led_cdev,
>                                           enum led_brightness brightness);
> +       /*
> +        * Set LED brightness level immediately - it can block the caller for
> +        * the time required for accessing a LED device register.
> +        */
> +       int             (*brightness_set_sync)(struct led_classdev *led_cdev,
> +                                       enum led_brightness brightness);
>         /* Get LED brightness level */
>         enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
>
> --
> 1.7.9.5
>

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

* Re: [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension
  2014-11-12 16:09 ` [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension Jacek Anaszewski
@ 2014-11-13 18:58   ` Bryan Wu
  2014-11-14 10:21     ` Jacek Anaszewski
  0 siblings, 1 reply; 9+ messages in thread
From: Bryan Wu @ 2014-11-13 18:58 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Linux LED Subsystem, linux-media, sakari.ailus, Kyungmin Park,
	b.zolnierkie, Richard Purdie

On Wed, Nov 12, 2014 at 8:09 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> The documentation being added contains overall description of the
> LED Flash Class and the related sysfs attributes.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> ---
>  Documentation/leds/leds-class-flash.txt |   39 +++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/leds/leds-class-flash.txt
>
> diff --git a/Documentation/leds/leds-class-flash.txt b/Documentation/leds/leds-class-flash.txt
> new file mode 100644
> index 0000000..0164329
> --- /dev/null
> +++ b/Documentation/leds/leds-class-flash.txt
> @@ -0,0 +1,39 @@
> +
> +Flash LED handling under Linux
> +==============================
> +
> +Some LED devices support two modes - torch and flash. In order to enable

I think I asked this question before, Torch, Flash and Indicator. As
you answered torch is implemented by sync led brightness set operation
in our LEDS_CLASS and Flash is implemented in this LEDS_CLASS_FLASH.

I suggest put this information in document or code comments. Then
people know how to use torch and flash.

For indicator I still don't know why we need this since indicator is
like blinking and it should be support by LEDS_CLASS right?

Flash is for some camera capture, right?

> +support for flash LEDs CONFIG_LEDS_CLASS_FLASH symbol must be defined
> +in the kernel config. A flash LED driver must register in the LED subsystem
> +with led_classdev_flash_register to gain flash capabilities.
> +
> +Following sysfs attributes are exposed for controlling flash led devices:
> +
> +       - flash_brightness - flash LED brightness in microamperes (RW)
> +       - max_flash_brightness - maximum available flash LED brightness (RO)
> +       - indicator_brightness - privacy LED brightness in microamperes (RW)
> +       - max_indicator_brightness - maximum privacy LED brightness in
> +                                    microamperes (RO)

What's the privacy mean here?

> +       - flash_timeout - flash strobe duration in microseconds (RW)
> +       - max_flash_timeout - maximum available flash strobe duration (RO)
> +       - flash_strobe - flash strobe state (RW)
> +       - flash_fault - bitmask of flash faults that may have occurred,
> +                       possible flags are:
> +               * 0x01 - flash controller voltage to the flash LED has exceeded
> +                        the limit specific to the flash controller
> +               * 0x02 - the flash strobe was still on when the timeout set by
> +                        the user has expired; not all flash controllers may
> +                        set this in all such conditions
> +               * 0x04 - the flash controller has overheated
> +               * 0x08 - the short circuit protection of the flash controller
> +                        has been triggered
> +               * 0x10 - current in the LED power supply has exceeded the limit
> +                        specific to the flash controller
> +               * 0x40 - flash controller voltage to the flash LED has been
> +                        below the minimum limit specific to the flash
> +               * 0x80 - the input voltage of the flash controller is below
> +                        the limit under which strobing the flash at full
> +                        current will not be possible. The condition persists
> +                        until this flag is no longer set
> +               * 0x100 - the temperature of the LED has exceeded its allowed
> +                         upper limit

Are these error code the same for all the LED controller? Or just for
some specific chip?


> --
> 1.7.9.5
>

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

* Re: [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension
  2014-11-13 18:58   ` Bryan Wu
@ 2014-11-14 10:21     ` Jacek Anaszewski
  0 siblings, 0 replies; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-14 10:21 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Linux LED Subsystem, linux-media, sakari.ailus, Kyungmin Park,
	b.zolnierkie, Richard Purdie

Hi Bryan,

Thanks for a review.

On 11/13/2014 07:58 PM, Bryan Wu wrote:
> On Wed, Nov 12, 2014 at 8:09 AM, Jacek Anaszewski
> <j.anaszewski@samsung.com> wrote:
>> The documentation being added contains overall description of the
>> LED Flash Class and the related sysfs attributes.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Bryan Wu <cooloney@gmail.com>
>> Cc: Richard Purdie <rpurdie@rpsys.net>
>> ---
>>   Documentation/leds/leds-class-flash.txt |   39 +++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>   create mode 100644 Documentation/leds/leds-class-flash.txt
>>
>> diff --git a/Documentation/leds/leds-class-flash.txt b/Documentation/leds/leds-class-flash.txt
>> new file mode 100644
>> index 0000000..0164329
>> --- /dev/null
>> +++ b/Documentation/leds/leds-class-flash.txt
>> @@ -0,0 +1,39 @@
>> +
>> +Flash LED handling under Linux
>> +==============================
>> +
>> +Some LED devices support two modes - torch and flash. In order to enable
>
> I think I asked this question before, Torch, Flash and Indicator. As
> you answered torch is implemented by sync led brightness set operation
> in our LEDS_CLASS and Flash is implemented in this LEDS_CLASS_FLASH.
>
> I suggest put this information in document or code comments. Then
> people know how to use torch and flash.

Good point.

> For indicator I still don't know why we need this since indicator is
> like blinking and it should be support by LEDS_CLASS right?

Indicator led is strictly related to flash devices. It is also called a
"privacy led" because of its purpose - protecting a privacy of a person
being recorded by providing a light signal signifying that a camera is
on. It is a low current led, but some devices use the same led as
for torch and flash and only apply reduced current in the indicator
mode.

In the V4L2 subsystem I see only one driver supporting indicator
leds: /drivers/media/i2c/as3645a.c. It looks like indicator intensity
can be set only when flash mode is V4L2_FLASH_LED_MODE_NONE, i.e. torch
and flash leds cannot be active simultaneously with indicator led.
It is reasonable, as active torch led is a sufficient signalization
of recording.

In the LED subsystem I also see indicators in some drivers,
e.g. leds-lm355x.c, but they are registered as a separate LED class
devices. Moreover the driver adds also a "pattern" sysfs attribute
for choosing indicator blinking pattern so that is something to be
added to the LED Flash class.
I think that similar improvement to the V4L2 Flash API should be made.

Sakari, what is your opinion?

> Flash is for some camera capture, right?
>
>> +support for flash LEDs CONFIG_LEDS_CLASS_FLASH symbol must be defined
>> +in the kernel config. A flash LED driver must register in the LED subsystem
>> +with led_classdev_flash_register to gain flash capabilities.
>> +
>> +Following sysfs attributes are exposed for controlling flash led devices:
>> +
>> +       - flash_brightness - flash LED brightness in microamperes (RW)
>> +       - max_flash_brightness - maximum available flash LED brightness (RO)
>> +       - indicator_brightness - privacy LED brightness in microamperes (RW)
>> +       - max_indicator_brightness - maximum privacy LED brightness in
>> +                                    microamperes (RO)
>
> What's the privacy mean here?

Indeed, consistent naming should be applied, so I will modify it to:

"maximum indicator LED brightness in microaperes (RO)"

>> +       - flash_timeout - flash strobe duration in microseconds (RW)
>> +       - max_flash_timeout - maximum available flash strobe duration (RO)
>> +       - flash_strobe - flash strobe state (RW)
>> +       - flash_fault - bitmask of flash faults that may have occurred,
>> +                       possible flags are:
>> +               * 0x01 - flash controller voltage to the flash LED has exceeded
>> +                        the limit specific to the flash controller
>> +               * 0x02 - the flash strobe was still on when the timeout set by
>> +                        the user has expired; not all flash controllers may
>> +                        set this in all such conditions
>> +               * 0x04 - the flash controller has overheated
>> +               * 0x08 - the short circuit protection of the flash controller
>> +                        has been triggered
>> +               * 0x10 - current in the LED power supply has exceeded the limit
>> +                        specific to the flash controller
>> +               * 0x40 - flash controller voltage to the flash LED has been
>> +                        below the minimum limit specific to the flash
>> +               * 0x80 - the input voltage of the flash controller is below
>> +                        the limit under which strobing the flash at full
>> +                        current will not be possible. The condition persists
>> +                        until this flag is no longer set
>> +               * 0x100 - the temperature of the LED has exceeded its allowed
>> +                         upper limit
>
> Are these error code the same for all the LED controller? Or just for
> some specific chip?

They are generic error codes, and map directly to V4L2 Flash errors.
Error descriptions were copied from the V4L2 Flash documentation.

Best Regards,
Jacek Anaszewski

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

* Re: [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem
  2014-11-12 16:09 ` [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
@ 2014-11-27  8:43   ` Sakari Ailus
  2014-11-28  9:34     ` Jacek Anaszewski
  0 siblings, 1 reply; 9+ messages in thread
From: Sakari Ailus @ 2014-11-27  8:43 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, sakari.ailus, kyungmin.park,
	b.zolnierkie, Bryan Wu, Richard Purdie

Hi Jacek,

Thank you for the patch.

One generic question: how do you support flash controllers with multiple
flash LEDs? I thought that once was a part of the patchset?

On Wed, Nov 12, 2014 at 05:09:16PM +0100, Jacek Anaszewski wrote:
> Some LED devices support two operation modes - torch and flash.
> This patch provides support for flash LED devices in the LED subsystem
> by introducing new sysfs attributes and kernel internal interface.
> The attributes being introduced are: flash_brightness, flash_strobe,
> flash_timeout, max_flash_timeout, max_flash_brightness, flash_fault,
> indicator_brightness and  max_indicator_brightness. All the flash
> related features are placed in a separate module.
> 
> The modifications aim to be compatible with V4L2 framework requirements
> related to the flash devices management. The design assumes that V4L2
> sub-device can take of the LED class device control and communicate
> with it through the kernel internal interface. When V4L2 Flash sub-device
> file is opened, the LED class device sysfs interface is made
> unavailable.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> ---
>  drivers/leds/Kconfig            |   11 +
>  drivers/leds/Makefile           |    1 +
>  drivers/leds/led-class-flash.c  |  511 +++++++++++++++++++++++++++++++++++++++
>  drivers/leds/led-class.c        |    4 +
>  include/linux/led-class-flash.h |  229 ++++++++++++++++++
>  include/linux/leds.h            |    3 +
>  6 files changed, 759 insertions(+)
>  create mode 100644 drivers/leds/led-class-flash.c
>  create mode 100644 include/linux/led-class-flash.h
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 808095b..50c4370 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -19,6 +19,17 @@ config LEDS_CLASS
>  	  This option enables the led sysfs class in /sys/class/leds.  You'll
>  	  need this to do anything useful with LEDs.  If unsure, say N.
>  
> +config LEDS_CLASS_FLASH
> +	tristate "LED Flash Class Support"
> +	depends on LEDS_CLASS
> +	depends on OF

Why the dependency? I don't see any references to Open Firmware.

> +	help
> +	  This option enables the flash led sysfs class in /sys/class/leds.
> +	  It wrapps LED Class and adds flash LEDs specific sysfs attributes
> +	  and kernel internal API to it. You'll need this to provide support
> +	  for the flash related features of a LED device. It can be built
> +	  as a module.
> +
>  comment "LED drivers"
>  
>  config LEDS_88PM860X
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index a2b1647..73c0cec 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -2,6 +2,7 @@
>  # LED Core
>  obj-$(CONFIG_NEW_LEDS)			+= led-core.o
>  obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
> +obj-$(CONFIG_LEDS_CLASS_FLASH)		+= led-class-flash.o
>  obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
>  
>  # LED Platform Drivers
> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> new file mode 100644
> index 0000000..bec99f6
> --- /dev/null
> +++ b/drivers/leds/led-class-flash.c
> @@ -0,0 +1,511 @@
> +/*
> + * LED Flash Class interface
> + *
> + * Copyright (C) 2014 Samsung Electronics Co., Ltd.
> + * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/led-class-flash.h>

Nitpicking... "-" comes before "s".

> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include "leds.h"
> +
> +#define has_flash_op(flash, op)				\
> +	(flash && flash->ops->op)
> +
> +#define call_flash_op(flash, op, args...)		\
> +	((has_flash_op(flash, op)) ?			\
> +			(flash->ops->op(flash, args)) :	\
> +			-EINVAL)
> +
> +static ssize_t flash_brightness_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +	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, 10, &state);
> +	if (ret)
> +		goto unlock;
> +
> +	ret = led_set_flash_brightness(flash, state);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = size;
> +unlock:
> +	mutex_unlock(&led_cdev->led_access);
> +	return ret;
> +}
> +
> +static ssize_t flash_brightness_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	/* no lock needed for this */
> +	led_update_flash_brightness(flash);
> +
> +	return sprintf(buf, "%u\n", flash->brightness.val);
> +}
> +static DEVICE_ATTR_RW(flash_brightness);
> +
> +static ssize_t max_flash_brightness_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	return sprintf(buf, "%u\n", flash->brightness.max);
> +}
> +static DEVICE_ATTR_RO(max_flash_brightness);
> +
> +static ssize_t indicator_brightness_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +	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, 10, &state);
> +	if (ret)
> +		goto unlock;
> +
> +	ret = led_set_indicator_brightness(flash, state);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = size;
> +unlock:
> +	mutex_unlock(&led_cdev->led_access);
> +	return ret;
> +}
> +
> +static ssize_t indicator_brightness_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	/* no lock needed for this */
> +	led_update_indicator_brightness(flash);
> +
> +	return sprintf(buf, "%u\n", flash->indicator_brightness->val);
> +}
> +static DEVICE_ATTR_RW(indicator_brightness);
> +
> +static ssize_t max_indicator_brightness_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	return sprintf(buf, "%u\n", flash->indicator_brightness->max);
> +}
> +static DEVICE_ATTR_RO(max_indicator_brightness);
> +
> +static ssize_t flash_strobe_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +	unsigned long state;
> +	ssize_t ret = -EINVAL;
> +
> +	mutex_lock(&led_cdev->led_access);
> +
> +	if (led_sysfs_is_disabled(led_cdev)) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	ret = kstrtoul(buf, 10, &state);
> +	if (ret)
> +		goto unlock;
> +
> +	if (state < 0 || state > 1) {
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	ret = led_set_flash_strobe(flash, state);
> +	if (ret < 0)
> +		goto unlock;
> +	ret = size;
> +unlock:
> +	mutex_unlock(&led_cdev->led_access);
> +	return ret;
> +}
> +
> +static ssize_t flash_strobe_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +	bool state;
> +	int ret;
> +
> +	/* no lock needed for this */
> +	ret = led_get_flash_strobe(flash, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%u\n", state);
> +}
> +static DEVICE_ATTR_RW(flash_strobe);
> +
> +static ssize_t flash_timeout_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +	unsigned long flash_timeout;
> +	ssize_t ret;
> +
> +	mutex_lock(&led_cdev->led_access);
> +
> +	if (led_sysfs_is_disabled(led_cdev)) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	ret = kstrtoul(buf, 10, &flash_timeout);
> +	if (ret)
> +		goto unlock;
> +
> +	ret = led_set_flash_timeout(flash, flash_timeout);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = size;
> +unlock:
> +	mutex_unlock(&led_cdev->led_access);
> +	return ret;
> +}
> +
> +static ssize_t flash_timeout_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	return sprintf(buf, "%u\n", flash->timeout.val);
> +}
> +static DEVICE_ATTR_RW(flash_timeout);
> +
> +static ssize_t max_flash_timeout_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	return sprintf(buf, "%u\n", flash->timeout.max);
> +}
> +static DEVICE_ATTR_RO(max_flash_timeout);
> +
> +static ssize_t flash_fault_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +	u32 fault;
> +	int ret;
> +
> +	ret = led_get_flash_fault(flash, &fault);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	return sprintf(buf, "0x%8.8x\n", fault);
> +}
> +static DEVICE_ATTR_RO(flash_fault);
> +
> +static struct attribute *led_flash_strobe_attrs[] = {
> +	&dev_attr_flash_strobe.attr,
> +	NULL,
> +};
> +
> +static struct attribute *led_flash_indicator_attrs[] = {
> +	&dev_attr_indicator_brightness.attr,
> +	&dev_attr_max_indicator_brightness.attr,
> +	NULL,
> +};
> +
> +static struct attribute *led_flash_timeout_attrs[] = {
> +	&dev_attr_flash_timeout.attr,
> +	&dev_attr_max_flash_timeout.attr,
> +	NULL,
> +};
> +
> +static struct attribute *led_flash_brightness_attrs[] = {
> +	&dev_attr_flash_brightness.attr,
> +	&dev_attr_max_flash_brightness.attr,
> +	NULL,
> +};
> +
> +static struct attribute *led_flash_fault_attrs[] = {
> +	&dev_attr_flash_fault.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group led_flash_strobe_group = {
> +	.attrs = led_flash_strobe_attrs,
> +};
> +
> +static const struct attribute_group led_flash_indicator_group = {
> +	.attrs = led_flash_indicator_attrs,
> +};
> +
> +static const struct attribute_group led_flash_timeout_group = {
> +	.attrs = led_flash_timeout_attrs,
> +};
> +
> +static const struct attribute_group led_flash_brightness_group = {
> +	.attrs = led_flash_brightness_attrs,
> +};
> +
> +static const struct attribute_group led_flash_fault_group = {
> +	.attrs = led_flash_fault_attrs,
> +};
> +
> +static const struct attribute_group *flash_groups[] = {
> +	&led_flash_strobe_group,
> +	NULL,
> +	NULL,
> +	NULL,
> +	NULL,
> +	NULL,
> +};
> +
> +static void led_flash_resume(struct led_classdev *led_cdev)
> +{
> +	struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> +	call_flash_op(flash, flash_brightness_set, flash->brightness.val);
> +	call_flash_op(flash, timeout_set, flash->timeout.val);
> +	call_flash_op(flash, indicator_brightness_set,
> +				flash->indicator_brightness->val);
> +}
> +
> +static void led_flash_init_sysfs_groups(struct led_classdev_flash *flash)
> +{
> +	struct led_classdev *led_cdev = &flash->led_cdev;
> +	const struct led_flash_ops *ops = flash->ops;
> +	int num_sysfs_groups = 1;
> +
> +	if (led_cdev->flags & LED_DEV_CAP_INDICATOR)
> +		flash_groups[num_sysfs_groups++] = &led_flash_indicator_group;
> +
> +	if (ops->flash_brightness_set)
> +		flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
> +
> +	if (ops->timeout_set)
> +		flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
> +
> +	if (ops->fault_get)
> +		flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
> +
> +	led_cdev->groups = flash_groups;
> +}
> +
> +int led_classdev_flash_register(struct device *parent,
> +				struct led_classdev_flash *flash)
> +{
> +	struct led_classdev *led_cdev;
> +	const struct led_flash_ops *ops;
> +	int ret;
> +
> +	if (!flash)
> +		return -EINVAL;
> +
> +	led_cdev = &flash->led_cdev;
> +
> +	if (led_cdev->flags & LED_DEV_CAP_FLASH) {
> +		if (!led_cdev->brightness_set_sync)
> +			return -EINVAL;
> +
> +		ops = flash->ops;
> +		if (!ops || !ops->strobe_set)
> +			return -EINVAL;
> +
> +		if ((led_cdev->flags & LED_DEV_CAP_INDICATOR) &&
> +		    (!flash->indicator_brightness ||
> +		     !ops->indicator_brightness_set))
> +			return -EINVAL;
> +
> +		led_cdev->flash_resume = led_flash_resume;
> +	}
> +
> +	/* Select the sysfs attributes to be created for the device */
> +	led_flash_init_sysfs_groups(flash);
> +
> +	/* Register led class device */
> +	ret = led_classdev_register(parent, led_cdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Setting a torch brightness needs to have immediate effect */
> +	led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
> +	led_cdev->flags |= SET_BRIGHTNESS_SYNC;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(led_classdev_flash_register);
> +
> +void led_classdev_flash_unregister(struct led_classdev_flash *flash)
> +{
> +	if (!flash)
> +		return;
> +
> +	led_classdev_unregister(&flash->led_cdev);
> +}
> +EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
> +
> +int led_set_flash_strobe(struct led_classdev_flash *flash, bool state)
> +{
> +	return call_flash_op(flash, strobe_set, state);
> +}
> +EXPORT_SYMBOL_GPL(led_set_flash_strobe);
> +
> +int led_get_flash_strobe(struct led_classdev_flash *flash, bool *state)
> +{
> +	return call_flash_op(flash, strobe_get, state);
> +}
> +EXPORT_SYMBOL_GPL(led_get_flash_strobe);
> +
> +static void led_clamp_align(struct led_flash_setting *s)
> +{
> +	u32 v, offset;
> +
> +	v = s->val + s->step / 2;
> +	v = clamp(v, s->min, s->max);
> +	offset = v - s->min;
> +	offset = s->step * (offset / s->step);

You could use the rounddown() macro. I.e.

rounddown(v - s->min, s->step) + s->min;

> +	s->val = s->min + offset;
> +}
> +
> +int led_set_flash_timeout(struct led_classdev_flash *flash, u32 timeout)
> +{
> +	struct led_classdev *led_cdev = &flash->led_cdev;
> +	struct led_flash_setting *s = &flash->timeout;
> +	int ret = 0;
> +
> +	s->val = timeout;
> +	led_clamp_align(s);
> +
> +	if (!(led_cdev->flags & LED_SUSPENDED))
> +		ret = call_flash_op(flash, timeout_set, s->val);
> +
> +	return ret;

ret is pretty much redundant here.

> +}
> +EXPORT_SYMBOL_GPL(led_set_flash_timeout);
> +
> +int led_get_flash_fault(struct led_classdev_flash *flash, u32 *fault)
> +{
> +	return call_flash_op(flash, fault_get, fault);
> +}
> +EXPORT_SYMBOL_GPL(led_get_flash_fault);
> +
> +int led_set_flash_brightness(struct led_classdev_flash *flash,
> +				u32 brightness)
> +{
> +	struct led_classdev *led_cdev = &flash->led_cdev;
> +	struct led_flash_setting *s = &flash->brightness;
> +	int ret = 0;
> +
> +	s->val = brightness;
> +	led_clamp_align(s);
> +
> +	if (!(led_cdev->flags & LED_SUSPENDED))
> +		ret = call_flash_op(flash, flash_brightness_set, s->val);

Same here.

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(led_set_flash_brightness);
> +
> +int led_update_flash_brightness(struct led_classdev_flash *flash)
> +{
> +	struct led_flash_setting *s = &flash->brightness;
> +	u32 brightness;
> +	int ret = 0;
> +
> +	if (has_flash_op(flash, flash_brightness_get)) {
> +		ret = call_flash_op(flash, flash_brightness_get,
> +						&brightness);
> +		if (ret < 0)
> +			return ret;
> +		s->val = brightness;
> +	}
> +
> +	return ret;

Shouldn't ret be always zero here? You can declare ret inside if () { }.
Same for brightness.

> +}
> +EXPORT_SYMBOL_GPL(led_update_flash_brightness);
> +
> +int led_set_indicator_brightness(struct led_classdev_flash *flash,
> +					u32 brightness)
> +{
> +	struct led_classdev *led_cdev = &flash->led_cdev;
> +	struct led_flash_setting *s = flash->indicator_brightness;
> +	int ret = 0;
> +
> +	if (!s)
> +		return -EINVAL;
> +
> +	s->val = brightness;
> +	led_clamp_align(s);
> +
> +	if (!(led_cdev->flags & LED_SUSPENDED))
> +		ret = call_flash_op(flash, indicator_brightness_set, s->val);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(led_set_indicator_brightness);
> +
> +int led_update_indicator_brightness(struct led_classdev_flash *flash)
> +{
> +	struct led_flash_setting *s = flash->indicator_brightness;
> +	u32 brightness;
> +	int ret = 0;
> +
> +	if (!s)
> +		return -EINVAL;
> +
> +	if (has_flash_op(flash, indicator_brightness_get)) {
> +		ret = call_flash_op(flash, indicator_brightness_get,
> +							&brightness);
> +		if (ret < 0)
> +			return ret;
> +		s->val = brightness;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(led_update_indicator_brightness);
> +
> +MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("LED Flash Class Interface");
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index dbeebac..02564c5 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -179,6 +179,10 @@ EXPORT_SYMBOL_GPL(led_classdev_suspend);
>  void led_classdev_resume(struct led_classdev *led_cdev)
>  {
>  	led_cdev->brightness_set(led_cdev, led_cdev->brightness);
> +
> +	if (led_cdev->flash_resume)
> +		led_cdev->flash_resume(led_cdev);
> +
>  	led_cdev->flags &= ~LED_SUSPENDED;
>  }
>  EXPORT_SYMBOL_GPL(led_classdev_resume);
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> new file mode 100644
> index 0000000..df95b8a
> --- /dev/null
> +++ b/include/linux/led-class-flash.h
> @@ -0,0 +1,229 @@
> +/*
> + * LED Flash Class interface
> + *
> + * Copyright (C) 2014 Samsung Electronics Co., Ltd.
> + * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +#ifndef __LINUX_FLASH_LEDS_H_INCLUDED
> +#define __LINUX_FLASH_LEDS_H_INCLUDED
> +
> +#include <linux/leds.h>

If you use the V4L2 flash fault definitions, you should include
uapi/linux/v4l2-controls.h .

> +struct led_classdev_flash;
> +struct device_node;
> +
> +/*
> + * Supported led fault bits - must be kept in synch
> + * with V4L2_FLASH_FAULT bits.
> + */
> +#define LED_FAULT_OVER_VOLTAGE		 V4L2_FLASH_FAULT_OVER_VOLTAGE
> +#define LED_FAULT_TIMEOUT		 V4L2_FLASH_FAULT_TIMEOUT
> +#define LED_FAULT_OVER_TEMPERATURE	 V4L2_FLASH_FAULT_OVER_TEMPERATURE
> +#define LED_FAULT_SHORT_CIRCUIT		 V4L2_FLASH_FAULT_SHORT_CIRCUIT
> +#define LED_FAULT_OVER_CURRENT		 V4L2_FLASH_FAULT_OVER_CURRENT
> +#define LED_FAULT_INDICATOR		 V4L2_FLASH_FAULT_INDICATOR
> +#define LED_FAULT_UNDER_VOLTAGE		 V4L2_FLASH_FAULT_UNDER_VOLTAGE
> +#define LED_FAULT_INPUT_VOLTAGE		 V4L2_FLASH_FAULT_INPUT_VOLTAGE
> +#define LED_FAULT_LED_OVER_TEMPERATURE	 V4L2_FLASH_OVER_TEMPERATURE
> +
> +struct led_flash_ops {
> +	/* set flash brightness */
> +	int (*flash_brightness_set)(struct led_classdev_flash *flash,
> +					u32 brightness);
> +	/* get flash brightness */
> +	int (*flash_brightness_get)(struct led_classdev_flash *flash,
> +					u32 *brightness);
> +	/* set flash indicator brightness */
> +	int (*indicator_brightness_set)(struct led_classdev_flash *flash,
> +					u32 brightness);
> +	/* get flash indicator brightness */
> +	int (*indicator_brightness_get)(struct led_classdev_flash *flash,
> +					u32 *brightness);
> +	/* set flash strobe state */
> +	int (*strobe_set)(struct led_classdev_flash *flash, bool state);
> +	/* get flash strobe state */
> +	int (*strobe_get)(struct led_classdev_flash *flash, bool *state);
> +	/* set flash timeout */
> +	int (*timeout_set)(struct led_classdev_flash *flash, u32 timeout);
> +	/* get the flash LED fault */
> +	int (*fault_get)(struct led_classdev_flash *flash, u32 *fault);
> +};
> +
> +/*
> + * Current value of a flash setting along
> + * with its constraints.
> + */
> +struct led_flash_setting {
> +	/* maximum allowed value */
> +	u32 min;
> +	/* maximum allowed value */
> +	u32 max;
> +	/* step value */
> +	u32 step;
> +	/* current value */
> +	u32 val;
> +};
> +
> +/*
> + * Aggregated flash settings - designed for ease
> + * of passing initialization data to the clients
> + * wrapping a LED Flash class device.
> + */
> +struct led_flash_config {
> +	struct led_flash_setting torch_brightness;
> +	struct led_flash_setting flash_brightness;
> +	struct led_flash_setting indicator_brightness;
> +	struct led_flash_setting flash_timeout;
> +	u32 flash_faults;
> +};
> +
> +struct led_classdev_flash {
> +	/* led class device */
> +	struct led_classdev led_cdev;
> +
> +	/* flash led specific ops */
> +	const struct led_flash_ops *ops;
> +
> +	/* flash brightness value in microamperes along with its constraints */
> +	struct led_flash_setting brightness;
> +
> +	/* timeout value in microseconds along with its constraints */
> +	struct led_flash_setting timeout;
> +
> +	/*
> +	 * Indicator brightness value in microamperes along with
> +	 * its constraints - this is an optional setting and must
> +	 * be allocated by the driver if the device supports privacy
> +	 * indicator led.
> +	 */
> +	struct led_flash_setting *indicator_brightness;
> +};
> +
> +static inline struct led_classdev_flash *lcdev_to_flash(
> +						struct led_classdev *lcdev)
> +{
> +	return container_of(lcdev, struct led_classdev_flash, led_cdev);
> +}
> +
> +/**
> + * led_classdev_flash_register - register a new object of led_classdev class
> +				 with support for flash LEDs

" *" ...

> + * @parent: the flash LED to register
> + * @flash: the led_classdev_flash structure for this device
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +int led_classdev_flash_register(struct device *parent,
> +				struct led_classdev_flash *flash);
> +
> +/**
> + * led_classdev_flash_unregister - unregisters an object of led_classdev class
> +				   with support for flash LEDs
> + * @flash: the flash LED to unregister
> + *
> + * Unregister a previously registered via led_classdev_flash_register object
> + */
> +void led_classdev_flash_unregister(struct led_classdev_flash *flash);
> +
> +/**
> + * led_set_flash_strobe - setup flash strobe
> + * @flash: the flash LED to set strobe on
> + * @state: 1 - strobe flash, 0 - stop flash strobe
> + *
> + * Strobe the flash LED.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_set_flash_strobe(struct led_classdev_flash *flash,
> +				bool state);
> +
> +/**
> + * led_get_flash_strobe - get flash strobe status
> + * @flash: the flash LED to query
> + * @state: 1 - flash is strobing, 0 - flash is off
> + *
> + * Check whether the flash is strobing at the moment.
> + *
> +u* Returns: 0 on success or negative error value on failure
> + */
> +extern int led_get_flash_strobe(struct led_classdev_flash *flash,
> +				bool *state);

I'd put a newline here.

> +/**
> + * led_set_flash_brightness - set flash LED brightness
> + * @flash: the flash LED to set
> + * @brightness: the brightness to set it to
> + *
> + * Set a flash LED's brightness.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_set_flash_brightness(struct led_classdev_flash *flash,
> +					u32 brightness);
> +
> +/**
> + * led_update_flash_brightness - update flash LED brightness
> + * @flash: the flash LED to query
> + *
> + * Get a flash LED's current brightness and update led_flash->brightness
> + * member with the obtained value.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_update_flash_brightness(struct led_classdev_flash *flash);
> +
> +/**
> + * led_set_flash_timeout - set flash LED timeout
> + * @flash: the flash LED to set
> + * @timeout: the flash timeout to set it to
> + *
> + * Set the flash strobe duration. The duration set by the driver
> + * is returned in the timeout argument and may differ from the
> + * one that was originally passed.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_set_flash_timeout(struct led_classdev_flash *flash,
> +					u32 timeout);
> +
> +/**
> + * led_get_flash_fault - get the flash LED fault
> + * @flash: the flash LED to query
> + * @fault: bitmask containing flash faults
> + *
> + * Get the flash LED fault.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_get_flash_fault(struct led_classdev_flash *flash,
> +					u32 *fault);
> +
> +/**
> + * led_set_indicator_brightness - set indicator LED brightness
> + * @flash: the flash LED to set
> + * @brightness: the brightness to set it to
> + *
> + * Set an indicator LED's brightness.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_set_indicator_brightness(struct led_classdev_flash *flash,
> +					u32 led_brightness);
> +
> +/**
> + * led_update_indicator_brightness - update flash indicator LED brightness
> + * @flash: the flash LED to query
> + *
> + * Get a flash indicator LED's current brightness and update
> + * led_flash->indicator_brightness member with the obtained value.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_update_indicator_brightness(struct led_classdev_flash *flash);
> +
> +
> +#endif	/* __LINUX_FLASH_LEDS_H_INCLUDED */
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index cfceef3..b9a2797 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -46,6 +46,8 @@ struct led_classdev {
>  #define LED_SYSFS_DISABLE	(1 << 20)
>  #define SET_BRIGHTNESS_ASYNC	(1 << 21)
>  #define SET_BRIGHTNESS_SYNC	(1 << 22)
> +#define LED_DEV_CAP_FLASH	(1 << 23)
> +#define LED_DEV_CAP_INDICATOR	(1 << 24)
>  
>  	/* Set LED brightness level */
>  	/* Must not sleep, use a workqueue if needed */
> @@ -81,6 +83,7 @@ struct led_classdev {
>  	unsigned long		 blink_delay_on, blink_delay_off;
>  	struct timer_list	 blink_timer;
>  	int			 blink_brightness;
> +	void			(*flash_resume)(struct led_classdev *led_cdev);
>  
>  	struct work_struct	set_brightness_work;
>  	int			delayed_set_value;

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

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

* Re: [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem
  2014-11-27  8:43   ` Sakari Ailus
@ 2014-11-28  9:34     ` Jacek Anaszewski
  0 siblings, 0 replies; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-28  9:34 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-leds, linux-media, sakari.ailus, kyungmin.park,
	b.zolnierkie, Bryan Wu, Richard Purdie

Hi Sakari,

Thanks for the review.
[...]
>> +static void led_clamp_align(struct led_flash_setting *s)
>> +{
>> +	u32 v, offset;
>> +
>> +	v = s->val + s->step / 2;
>> +	v = clamp(v, s->min, s->max);
>> +	offset = v - s->min;
>> +	offset = s->step * (offset / s->step);
>
> You could use the rounddown() macro. I.e.
>
> rounddown(v - s->min, s->step) + s->min;

I took this code snippet from v4l2-ctrls.c.
It allows for aligning the control value to the nearest
step - top or bottom, whereas rounddown only to the
bottom one.

Best Regards,
Jacek Anaszewski

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

end of thread, other threads:[~2014-11-28  9:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-12 16:09 [PATCH/RFC v7 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
2014-11-12 16:09 ` [PATCH/RFC v7 1/3] leds: Add support for setting brightness in a synchronous way Jacek Anaszewski
2014-11-13 18:43   ` Bryan Wu
2014-11-12 16:09 ` [PATCH/RFC v7 2/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
2014-11-27  8:43   ` Sakari Ailus
2014-11-28  9:34     ` Jacek Anaszewski
2014-11-12 16:09 ` [PATCH/RFC v7 3/3] Documentation: leds: Add description of LED Flash Class extension Jacek Anaszewski
2014-11-13 18:58   ` Bryan Wu
2014-11-14 10:21     ` Jacek Anaszewski

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).