* [PATCH/RFC v6 0/3] LED / flash API integration - LED Flash Class
@ 2014-09-22 15:21 Jacek Anaszewski
2014-09-22 15:21 ` [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism Jacek Anaszewski
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jacek Anaszewski @ 2014-09-22 15:21 UTC (permalink / raw)
To: linux-leds, linux-media; +Cc: kyungmin.park, b.zolnierkie, Jacek Anaszewski
This patch set is the follow-up of the LED / flash API integration
series [1]. For clarity reasons the patchset has been split into
four subsets:
- LED Flash Class
- V4L2 Flash
- LED Flash Class drivers
- Documentation
========================
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: implement sysfs interface locking mechanism
leds: add API for setting torch brightness
leds: Add LED Flash Class wrapper to LED subsystem
drivers/leds/Kconfig | 11 +
drivers/leds/Makefile | 1 +
drivers/leds/led-class-flash.c | 557 +++++++++++++++++++++++++++++++++++++++
drivers/leds/led-class.c | 30 ++-
drivers/leds/led-core.c | 32 +++
drivers/leds/led-triggers.c | 16 +-
include/linux/led-class-flash.h | 238 +++++++++++++++++
include/linux/leds.h | 56 ++++
8 files changed, 934 insertions(+), 7 deletions(-)
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 v6 1/3] leds: implement sysfs interface locking mechanism
2014-09-22 15:21 [PATCH/RFC v6 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
@ 2014-09-22 15:21 ` Jacek Anaszewski
2014-10-21 1:01 ` Bryan Wu
2014-09-22 15:21 ` [PATCH/RFC v6 2/3] leds: add API for setting torch brightness Jacek Anaszewski
2014-09-22 15:21 ` [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
2 siblings, 1 reply; 9+ messages in thread
From: Jacek Anaszewski @ 2014-09-22 15:21 UTC (permalink / raw)
To: linux-leds, linux-media
Cc: kyungmin.park, b.zolnierkie, Jacek Anaszewski, Bryan Wu,
Richard Purdie
Add a mechanism for locking LED subsystem sysfs interface.
This patch prepares ground for addition of LED Flash Class
extension, whose API will be integrated with V4L2 Flash API.
Such a fusion enforces introducing a locking scheme, which
will secure consistent access to the LED Flash Class device.
The mechanism being introduced allows for disabling LED
subsystem sysfs interface by calling led_sysfs_disable function
and enabling it by calling led_sysfs_enable. The functions
alter the LED_SYSFS_DISABLE flag state and must be called
under mutex lock. The state of the lock is checked with use
of led_sysfs_is_disabled function. Such a design allows for
providing immediate feedback to the user space on whether
the LED Flash Class device is available or is under V4L2 Flash
sub-device control.
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 | 19 ++++++++++++++++---
drivers/leds/led-core.c | 18 ++++++++++++++++++
drivers/leds/led-triggers.c | 16 +++++++++++++---
include/linux/leds.h | 32 ++++++++++++++++++++++++++++++++
4 files changed, 79 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 2e124aa2..a39ca8f 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -39,17 +39,27 @@ static ssize_t brightness_store(struct device *dev,
{
struct led_classdev *led_cdev = dev_get_drvdata(dev);
unsigned long state;
- ssize_t ret = -EINVAL;
+ 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)
- return ret;
+ goto unlock;
if (state == LED_OFF)
led_trigger_remove(led_cdev);
__led_set_brightness(led_cdev, state);
- return size;
+ ret = size;
+unlock:
+ mutex_unlock(&led_cdev->led_access);
+ return ret;
}
static DEVICE_ATTR_RW(brightness);
@@ -213,6 +223,7 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
#ifdef CONFIG_LEDS_TRIGGERS
init_rwsem(&led_cdev->trigger_lock);
#endif
+ mutex_init(&led_cdev->led_access);
/* add to the list of leds */
down_write(&leds_list_lock);
list_add_tail(&led_cdev->node, &leds_list);
@@ -266,6 +277,8 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
down_write(&leds_list_lock);
list_del(&led_cdev->node);
up_write(&leds_list_lock);
+
+ mutex_destroy(&led_cdev->led_access);
}
EXPORT_SYMBOL_GPL(led_classdev_unregister);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 0d15aa9..cca86ab 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -142,3 +142,21 @@ int led_update_brightness(struct led_classdev *led_cdev)
return ret;
}
EXPORT_SYMBOL(led_update_brightness);
+
+/* Caller must ensure led_cdev->led_access held */
+void led_sysfs_disable(struct led_classdev *led_cdev)
+{
+ lockdep_assert_held(&led_cdev->led_access);
+
+ led_cdev->flags |= LED_SYSFS_DISABLE;
+}
+EXPORT_SYMBOL_GPL(led_sysfs_disable);
+
+/* Caller must ensure led_cdev->led_access held */
+void led_sysfs_enable(struct led_classdev *led_cdev)
+{
+ lockdep_assert_held(&led_cdev->led_access);
+
+ led_cdev->flags &= ~LED_SYSFS_DISABLE;
+}
+EXPORT_SYMBOL_GPL(led_sysfs_enable);
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index c3734f1..e8b1120 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -37,6 +37,14 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
char trigger_name[TRIG_NAME_MAX];
struct led_trigger *trig;
size_t len;
+ int ret = count;
+
+ mutex_lock(&led_cdev->led_access);
+
+ if (led_sysfs_is_disabled(led_cdev)) {
+ ret = -EBUSY;
+ goto unlock;
+ }
trigger_name[sizeof(trigger_name) - 1] = '\0';
strncpy(trigger_name, buf, sizeof(trigger_name) - 1);
@@ -47,7 +55,7 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
if (!strcmp(trigger_name, "none")) {
led_trigger_remove(led_cdev);
- return count;
+ goto unlock;
}
down_read(&triggers_list_lock);
@@ -58,12 +66,14 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
up_write(&led_cdev->trigger_lock);
up_read(&triggers_list_lock);
- return count;
+ goto unlock;
}
}
up_read(&triggers_list_lock);
- return -EINVAL;
+unlock:
+ mutex_unlock(&led_cdev->led_access);
+ return ret;
}
EXPORT_SYMBOL_GPL(led_trigger_store);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index f8b2f58..44c8a98 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -13,6 +13,7 @@
#define __LINUX_LEDS_H_INCLUDED
#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/rwsem.h>
#include <linux/spinlock.h>
#include <linux/timer.h>
@@ -42,6 +43,7 @@ struct led_classdev {
#define LED_BLINK_ONESHOT (1 << 17)
#define LED_BLINK_ONESHOT_STOP (1 << 18)
#define LED_BLINK_INVERT (1 << 19)
+#define LED_SYSFS_DISABLE (1 << 20)
/* Set LED brightness level */
/* Must not sleep, use a workqueue if needed */
@@ -85,6 +87,9 @@ struct led_classdev {
/* true if activated - deactivate routine uses it to do cleanup */
bool activated;
#endif
+
+ /* Ensures consistent access to the LED Flash Class device */
+ struct mutex led_access;
};
extern int led_classdev_register(struct device *parent,
@@ -151,6 +156,33 @@ extern void led_set_brightness(struct led_classdev *led_cdev,
*/
extern int led_update_brightness(struct led_classdev *led_cdev);
+/**
+ * led_sysfs_disable - disable LED sysfs interface
+ * @led_cdev: the LED to set
+ *
+ * Disable the led_cdev's sysfs interface.
+ */
+extern void led_sysfs_disable(struct led_classdev *led_cdev);
+
+/**
+ * led_sysfs_enable - enable LED sysfs interface
+ * @led_cdev: the LED to set
+ *
+ * Enable the led_cdev's sysfs interface.
+ */
+extern void led_sysfs_enable(struct led_classdev *led_cdev);
+
+/**
+ * led_sysfs_is_disabled - check if LED sysfs interface is disabled
+ * @led_cdev: the LED to query
+ *
+ * Returns: true if the led_cdev's sysfs interface is disabled.
+ */
+static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
+{
+ return led_cdev->flags & LED_SYSFS_DISABLE;
+}
+
/*
* LED Triggers
*/
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH/RFC v6 2/3] leds: add API for setting torch brightness
2014-09-22 15:21 [PATCH/RFC v6 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
2014-09-22 15:21 ` [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism Jacek Anaszewski
@ 2014-09-22 15:21 ` Jacek Anaszewski
2014-10-21 1:05 ` Bryan Wu
2014-09-22 15:21 ` [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
2 siblings, 1 reply; 9+ messages in thread
From: Jacek Anaszewski @ 2014-09-22 15:21 UTC (permalink / raw)
To: linux-leds, linux-media
Cc: kyungmin.park, b.zolnierkie, Jacek Anaszewski, Bryan Wu,
Richard Purdie
This patch prepares ground for addition of LED Flash Class extension to
the LED subsystem. Since turning the torch on must have guaranteed
immediate effect the brightness_set op can't be used for it. Drivers must
schedule a work queue task in this op to be compatible with led-triggers,
which call brightess_set from timer irqs. In order to address this
limitation a torch_brightness_set op and led_set_torch_brightness API
is introduced. Setting brightness sysfs attribute will result in calling
brightness_set op for LED Class devices and torch_brightness_set op for
LED Flash Class devices, whereas triggers will still call brightness
op in both cases.
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 | 9 +++++++--
drivers/leds/led-core.c | 14 ++++++++++++++
include/linux/leds.h | 21 +++++++++++++++++++++
3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index a39ca8f..5a11a07 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -54,9 +54,14 @@ static ssize_t brightness_store(struct device *dev,
if (state == LED_OFF)
led_trigger_remove(led_cdev);
- __led_set_brightness(led_cdev, state);
- ret = size;
+ if (led_cdev->flags & LED_DEV_CAP_TORCH)
+ ret = led_set_torch_brightness(led_cdev, state);
+ else
+ __led_set_brightness(led_cdev, state);
+
+ if (!ret)
+ ret = size;
unlock:
mutex_unlock(&led_cdev->led_access);
return ret;
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index cca86ab..c6d8288 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -143,6 +143,20 @@ int led_update_brightness(struct led_classdev *led_cdev)
}
EXPORT_SYMBOL(led_update_brightness);
+int led_set_torch_brightness(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ int ret = 0;
+
+ led_cdev->brightness = min(brightness, led_cdev->max_brightness);
+
+ if (!(led_cdev->flags & LED_SUSPENDED))
+ ret = led_cdev->torch_brightness_set(led_cdev,
+ led_cdev->brightness);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(led_set_torch_brightness);
+
/* Caller must ensure led_cdev->led_access held */
void led_sysfs_disable(struct led_classdev *led_cdev)
{
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 44c8a98..bc2a570 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -44,11 +44,21 @@ struct led_classdev {
#define LED_BLINK_ONESHOT_STOP (1 << 18)
#define LED_BLINK_INVERT (1 << 19)
#define LED_SYSFS_DISABLE (1 << 20)
+#define LED_DEV_CAP_TORCH (1 << 21)
/* 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 immediately - it is required for flash led
+ * devices as they require setting torch brightness to have immediate
+ * effect. brightness_set op cannot be used for this purpose because
+ * the led drivers schedule a work queue task in it to allow for
+ * being called from led-triggers, i.e. from the timer irq context.
+ */
+ int (*torch_brightness_set)(struct led_classdev *led_cdev,
+ enum led_brightness brightness);
/* Get LED brightness level */
enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
@@ -157,6 +167,17 @@ extern void led_set_brightness(struct led_classdev *led_cdev,
extern int led_update_brightness(struct led_classdev *led_cdev);
/**
+ * led_set_torch_brightness - set torch LED brightness
+ * @led_cdev: the LED to set
+ * @brightness: the brightness to set it to
+ *
+ * Returns: 0 on success or negative error value on failure
+ *
+ * Set a torch LED's brightness.
+ */
+extern int led_set_torch_brightness(struct led_classdev *led_cdev,
+ enum led_brightness brightness);
+/**
* led_sysfs_disable - disable LED sysfs interface
* @led_cdev: the LED to set
*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem
2014-09-22 15:21 [PATCH/RFC v6 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
2014-09-22 15:21 ` [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism Jacek Anaszewski
2014-09-22 15:21 ` [PATCH/RFC v6 2/3] leds: add API for setting torch brightness Jacek Anaszewski
@ 2014-09-22 15:21 ` Jacek Anaszewski
2014-11-04 1:34 ` Bryan Wu
2 siblings, 1 reply; 9+ messages in thread
From: Jacek Anaszewski @ 2014-09-22 15:21 UTC (permalink / raw)
To: linux-leds, linux-media
Cc: 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 | 557 +++++++++++++++++++++++++++++++++++++++
drivers/leds/led-class.c | 4 +
include/linux/led-class-flash.h | 238 +++++++++++++++++
include/linux/leds.h | 3 +
6 files changed, 814 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 8c96e2d..3c58021 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -22,6 +22,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 d8cc5f2..9238b8a 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..f1ba539
--- /dev/null
+++ b/drivers/leds/led-class-flash.c
@@ -0,0 +1,557 @@
+/*
+ * 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 struct attribute_group led_flash_strobe_group = {
+ .attrs = led_flash_strobe_attrs,
+};
+
+static struct attribute_group led_flash_brightness_group = {
+ .attrs = led_flash_brightness_attrs,
+};
+
+static struct attribute_group led_flash_timeout_group = {
+ .attrs = led_flash_timeout_attrs,
+};
+
+static struct attribute_group led_flash_indicator_group = {
+ .attrs = led_flash_indicator_attrs,
+};
+
+static struct attribute_group led_flash_fault_group = {
+ .attrs = led_flash_fault_attrs,
+};
+
+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_remove_sysfs_groups(struct led_classdev_flash *flash)
+{
+ struct led_classdev *led_cdev = &flash->led_cdev;
+ int i;
+
+ for (i = 0; i < LED_FLASH_MAX_SYSFS_GROUPS; ++i)
+ if (flash->sysfs_groups[i])
+ sysfs_remove_group(&led_cdev->dev->kobj,
+ flash->sysfs_groups[i]);
+}
+
+static int led_flash_create_sysfs_groups(struct led_classdev_flash *flash)
+{
+ struct led_classdev *led_cdev = &flash->led_cdev;
+ const struct led_flash_ops *ops = flash->ops;
+ int ret, num_sysfs_groups = 0;
+
+ memset(flash->sysfs_groups, 0, sizeof(*flash->sysfs_groups) *
+ LED_FLASH_MAX_SYSFS_GROUPS);
+
+ ret = sysfs_create_group(&led_cdev->dev->kobj, &led_flash_strobe_group);
+ if (ret < 0)
+ goto err_create_group;
+ flash->sysfs_groups[num_sysfs_groups++] = &led_flash_strobe_group;
+
+ if (flash->indicator_brightness) {
+ ret = sysfs_create_group(&led_cdev->dev->kobj,
+ &led_flash_indicator_group);
+ if (ret < 0)
+ goto err_create_group;
+ flash->sysfs_groups[num_sysfs_groups++] =
+ &led_flash_indicator_group;
+ }
+
+ if (ops->flash_brightness_set) {
+ ret = sysfs_create_group(&led_cdev->dev->kobj,
+ &led_flash_brightness_group);
+ if (ret < 0)
+ goto err_create_group;
+ flash->sysfs_groups[num_sysfs_groups++] =
+ &led_flash_brightness_group;
+ }
+
+ if (ops->timeout_set) {
+ ret = sysfs_create_group(&led_cdev->dev->kobj,
+ &led_flash_timeout_group);
+ if (ret < 0)
+ goto err_create_group;
+ flash->sysfs_groups[num_sysfs_groups++] =
+ &led_flash_timeout_group;
+ }
+
+ if (ops->fault_get) {
+ ret = sysfs_create_group(&led_cdev->dev->kobj,
+ &led_flash_fault_group);
+ if (ret < 0)
+ goto err_create_group;
+ flash->sysfs_groups[num_sysfs_groups++] =
+ &led_flash_fault_group;
+ }
+
+ return 0;
+
+err_create_group:
+ led_flash_remove_sysfs_groups(flash);
+ return ret;
+}
+
+int led_classdev_flash_register(struct device *parent,
+ struct led_classdev_flash *flash,
+ struct device_node *node)
+{
+ struct led_classdev *led_cdev;
+ const struct led_flash_ops *ops;
+ int ret = -EINVAL;
+
+ if (!flash)
+ return -EINVAL;
+
+ led_cdev = &flash->led_cdev;
+
+ /* Torch capability is default for every LED Flash Class device */
+ led_cdev->flags |= LED_DEV_CAP_TORCH;
+
+ if (led_cdev->flags & LED_DEV_CAP_FLASH) {
+ if (!led_cdev->torch_brightness_set)
+ 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;
+ }
+
+ /* Register led class device */
+ ret = led_classdev_register(parent, led_cdev);
+ if (ret < 0)
+ return -EINVAL;
+
+ /* Create flash led specific sysfs attributes */
+ ret = led_flash_create_sysfs_groups(flash);
+ if (ret < 0)
+ goto err_create_sysfs_groups;
+
+ return 0;
+
+err_create_sysfs_groups:
+ led_classdev_unregister(led_cdev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(led_classdev_flash_register);
+
+void led_classdev_flash_unregister(struct led_classdev_flash *flash)
+{
+ if (!flash)
+ return;
+
+ led_flash_remove_sysfs_groups(flash);
+ 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 5a11a07..d398b87 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -183,6 +183,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..fcc15ed
--- /dev/null
+++ b/include/linux/led-class-flash.h
@@ -0,0 +1,238 @@
+/*
+ * 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
+
+#define LED_FLASH_MAX_SYSFS_GROUPS 5
+
+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-flash-manager uses it to link flashes */
+ struct list_head list;
+ /* led class device */
+ struct led_classdev led_cdev;
+ /* flash led specific ops */
+ const struct led_flash_ops *ops;
+
+ /* flash sysfs groups */
+ struct attribute_group *sysfs_groups[LED_FLASH_MAX_SYSFS_GROUPS];
+
+ /* 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
+ * @node: device tree node of the LED Flash Class device - it must be
+ initialized if the device is to be registered in the flash manager
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+int led_classdev_flash_register(struct device *parent,
+ struct led_classdev_flash *flash,
+ struct device_node *node);
+
+/**
+ * led_classdev_flash_unregister - unregisters an object of led_classdev class
+ with support for flash LEDs
+ * @flash: the flash LED to unregister
+ *
+ * Unregisters 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
+ *
+ * Setup flash strobe - trigger flash strobe
+ *
+ * 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 or not.
+ *
+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
+ *
+ * Returns: 0 on success or negative error value on failure
+ *
+ * Set a flash LED's brightness.
+ */
+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 bc2a570..70fa0ad 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -45,6 +45,8 @@ struct led_classdev {
#define LED_BLINK_INVERT (1 << 19)
#define LED_SYSFS_DISABLE (1 << 20)
#define LED_DEV_CAP_TORCH (1 << 21)
+#define LED_DEV_CAP_FLASH (1 << 22)
+#define LED_DEV_CAP_INDICATOR (1 << 23)
/* Set LED brightness level */
/* Must not sleep, use a workqueue if needed */
@@ -83,6 +85,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
* Re: [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism
2014-09-22 15:21 ` [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism Jacek Anaszewski
@ 2014-10-21 1:01 ` Bryan Wu
2014-11-04 1:37 ` Bryan Wu
0 siblings, 1 reply; 9+ messages in thread
From: Bryan Wu @ 2014-10-21 1:01 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Linux LED Subsystem, linux-media@vger.kernel.org, Kyungmin Park,
b.zolnierkie, Richard Purdie
On Mon, Sep 22, 2014 at 8:21 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> Add a mechanism for locking LED subsystem sysfs interface.
> This patch prepares ground for addition of LED Flash Class
> extension, whose API will be integrated with V4L2 Flash API.
> Such a fusion enforces introducing a locking scheme, which
> will secure consistent access to the LED Flash Class device.
>
> The mechanism being introduced allows for disabling LED
> subsystem sysfs interface by calling led_sysfs_disable function
> and enabling it by calling led_sysfs_enable. The functions
> alter the LED_SYSFS_DISABLE flag state and must be called
> under mutex lock. The state of the lock is checked with use
> of led_sysfs_is_disabled function. Such a design allows for
> providing immediate feedback to the user space on whether
> the LED Flash Class device is available or is under V4L2 Flash
> sub-device control.
>
I'm good with this and will merge it soon.
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 | 19 ++++++++++++++++---
> drivers/leds/led-core.c | 18 ++++++++++++++++++
> drivers/leds/led-triggers.c | 16 +++++++++++++---
> include/linux/leds.h | 32 ++++++++++++++++++++++++++++++++
> 4 files changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 2e124aa2..a39ca8f 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -39,17 +39,27 @@ static ssize_t brightness_store(struct device *dev,
> {
> struct led_classdev *led_cdev = dev_get_drvdata(dev);
> unsigned long state;
> - ssize_t ret = -EINVAL;
> + 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)
> - return ret;
> + goto unlock;
>
> if (state == LED_OFF)
> led_trigger_remove(led_cdev);
> __led_set_brightness(led_cdev, state);
>
> - return size;
> + ret = size;
> +unlock:
> + mutex_unlock(&led_cdev->led_access);
> + return ret;
> }
> static DEVICE_ATTR_RW(brightness);
>
> @@ -213,6 +223,7 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
> #ifdef CONFIG_LEDS_TRIGGERS
> init_rwsem(&led_cdev->trigger_lock);
> #endif
> + mutex_init(&led_cdev->led_access);
> /* add to the list of leds */
> down_write(&leds_list_lock);
> list_add_tail(&led_cdev->node, &leds_list);
> @@ -266,6 +277,8 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
> down_write(&leds_list_lock);
> list_del(&led_cdev->node);
> up_write(&leds_list_lock);
> +
> + mutex_destroy(&led_cdev->led_access);
> }
> EXPORT_SYMBOL_GPL(led_classdev_unregister);
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 0d15aa9..cca86ab 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -142,3 +142,21 @@ int led_update_brightness(struct led_classdev *led_cdev)
> return ret;
> }
> EXPORT_SYMBOL(led_update_brightness);
> +
> +/* Caller must ensure led_cdev->led_access held */
> +void led_sysfs_disable(struct led_classdev *led_cdev)
> +{
> + lockdep_assert_held(&led_cdev->led_access);
> +
> + led_cdev->flags |= LED_SYSFS_DISABLE;
> +}
> +EXPORT_SYMBOL_GPL(led_sysfs_disable);
> +
> +/* Caller must ensure led_cdev->led_access held */
> +void led_sysfs_enable(struct led_classdev *led_cdev)
> +{
> + lockdep_assert_held(&led_cdev->led_access);
> +
> + led_cdev->flags &= ~LED_SYSFS_DISABLE;
> +}
> +EXPORT_SYMBOL_GPL(led_sysfs_enable);
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index c3734f1..e8b1120 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -37,6 +37,14 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
> char trigger_name[TRIG_NAME_MAX];
> struct led_trigger *trig;
> size_t len;
> + int ret = count;
> +
> + mutex_lock(&led_cdev->led_access);
> +
> + if (led_sysfs_is_disabled(led_cdev)) {
> + ret = -EBUSY;
> + goto unlock;
> + }
>
> trigger_name[sizeof(trigger_name) - 1] = '\0';
> strncpy(trigger_name, buf, sizeof(trigger_name) - 1);
> @@ -47,7 +55,7 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
>
> if (!strcmp(trigger_name, "none")) {
> led_trigger_remove(led_cdev);
> - return count;
> + goto unlock;
> }
>
> down_read(&triggers_list_lock);
> @@ -58,12 +66,14 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
> up_write(&led_cdev->trigger_lock);
>
> up_read(&triggers_list_lock);
> - return count;
> + goto unlock;
> }
> }
> up_read(&triggers_list_lock);
>
> - return -EINVAL;
> +unlock:
> + mutex_unlock(&led_cdev->led_access);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(led_trigger_store);
>
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index f8b2f58..44c8a98 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -13,6 +13,7 @@
> #define __LINUX_LEDS_H_INCLUDED
>
> #include <linux/list.h>
> +#include <linux/mutex.h>
> #include <linux/rwsem.h>
> #include <linux/spinlock.h>
> #include <linux/timer.h>
> @@ -42,6 +43,7 @@ struct led_classdev {
> #define LED_BLINK_ONESHOT (1 << 17)
> #define LED_BLINK_ONESHOT_STOP (1 << 18)
> #define LED_BLINK_INVERT (1 << 19)
> +#define LED_SYSFS_DISABLE (1 << 20)
>
> /* Set LED brightness level */
> /* Must not sleep, use a workqueue if needed */
> @@ -85,6 +87,9 @@ struct led_classdev {
> /* true if activated - deactivate routine uses it to do cleanup */
> bool activated;
> #endif
> +
> + /* Ensures consistent access to the LED Flash Class device */
> + struct mutex led_access;
> };
>
> extern int led_classdev_register(struct device *parent,
> @@ -151,6 +156,33 @@ extern void led_set_brightness(struct led_classdev *led_cdev,
> */
> extern int led_update_brightness(struct led_classdev *led_cdev);
>
> +/**
> + * led_sysfs_disable - disable LED sysfs interface
> + * @led_cdev: the LED to set
> + *
> + * Disable the led_cdev's sysfs interface.
> + */
> +extern void led_sysfs_disable(struct led_classdev *led_cdev);
> +
> +/**
> + * led_sysfs_enable - enable LED sysfs interface
> + * @led_cdev: the LED to set
> + *
> + * Enable the led_cdev's sysfs interface.
> + */
> +extern void led_sysfs_enable(struct led_classdev *led_cdev);
> +
> +/**
> + * led_sysfs_is_disabled - check if LED sysfs interface is disabled
> + * @led_cdev: the LED to query
> + *
> + * Returns: true if the led_cdev's sysfs interface is disabled.
> + */
> +static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
> +{
> + return led_cdev->flags & LED_SYSFS_DISABLE;
> +}
> +
> /*
> * LED Triggers
> */
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH/RFC v6 2/3] leds: add API for setting torch brightness
2014-09-22 15:21 ` [PATCH/RFC v6 2/3] leds: add API for setting torch brightness Jacek Anaszewski
@ 2014-10-21 1:05 ` Bryan Wu
0 siblings, 0 replies; 9+ messages in thread
From: Bryan Wu @ 2014-10-21 1:05 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Linux LED Subsystem, linux-media@vger.kernel.org, Kyungmin Park,
b.zolnierkie, Richard Purdie
On Mon, Sep 22, 2014 at 8:21 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> This patch prepares ground for addition of LED Flash Class extension to
> the LED subsystem. Since turning the torch on must have guaranteed
> immediate effect the brightness_set op can't be used for it. Drivers must
> schedule a work queue task in this op to be compatible with led-triggers,
> which call brightess_set from timer irqs. In order to address this
> limitation a torch_brightness_set op and led_set_torch_brightness API
> is introduced. Setting brightness sysfs attribute will result in calling
> brightness_set op for LED Class devices and torch_brightness_set op for
> LED Flash Class devices, whereas triggers will still call brightness
> op in both cases.
>
Although this torch API is for torch, but I think we can make it more
generic and other use case can benefit from this change.
What about this?
Add 2 flags:
1. SET_BRIGHTNESS_ASYNC
2. SET_BRIGHTNESS_SYNC
Then move old API to
__led_set_brightness(led_cdev, state);
led_set_brightness_async();
And add a new API like
led_set_bightness_sync();
Then for torch use case, we use led_set_brightness_sync() API.
-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 | 9 +++++++--
> drivers/leds/led-core.c | 14 ++++++++++++++
> include/linux/leds.h | 21 +++++++++++++++++++++
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index a39ca8f..5a11a07 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -54,9 +54,14 @@ static ssize_t brightness_store(struct device *dev,
>
> if (state == LED_OFF)
> led_trigger_remove(led_cdev);
> - __led_set_brightness(led_cdev, state);
>
> - ret = size;
> + if (led_cdev->flags & LED_DEV_CAP_TORCH)
> + ret = led_set_torch_brightness(led_cdev, state);
> + else
> + __led_set_brightness(led_cdev, state);
> +
> + if (!ret)
> + ret = size;
> unlock:
> mutex_unlock(&led_cdev->led_access);
> return ret;
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index cca86ab..c6d8288 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -143,6 +143,20 @@ int led_update_brightness(struct led_classdev *led_cdev)
> }
> EXPORT_SYMBOL(led_update_brightness);
>
> +int led_set_torch_brightness(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + int ret = 0;
> +
> + led_cdev->brightness = min(brightness, led_cdev->max_brightness);
> +
> + if (!(led_cdev->flags & LED_SUSPENDED))
> + ret = led_cdev->torch_brightness_set(led_cdev,
> + led_cdev->brightness);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(led_set_torch_brightness);
> +
> /* Caller must ensure led_cdev->led_access held */
> void led_sysfs_disable(struct led_classdev *led_cdev)
> {
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 44c8a98..bc2a570 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -44,11 +44,21 @@ struct led_classdev {
> #define LED_BLINK_ONESHOT_STOP (1 << 18)
> #define LED_BLINK_INVERT (1 << 19)
> #define LED_SYSFS_DISABLE (1 << 20)
> +#define LED_DEV_CAP_TORCH (1 << 21)
>
> /* 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 immediately - it is required for flash led
> + * devices as they require setting torch brightness to have immediate
> + * effect. brightness_set op cannot be used for this purpose because
> + * the led drivers schedule a work queue task in it to allow for
> + * being called from led-triggers, i.e. from the timer irq context.
> + */
> + int (*torch_brightness_set)(struct led_classdev *led_cdev,
> + enum led_brightness brightness);
> /* Get LED brightness level */
> enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
>
> @@ -157,6 +167,17 @@ extern void led_set_brightness(struct led_classdev *led_cdev,
> extern int led_update_brightness(struct led_classdev *led_cdev);
>
> /**
> + * led_set_torch_brightness - set torch LED brightness
> + * @led_cdev: the LED to set
> + * @brightness: the brightness to set it to
> + *
> + * Returns: 0 on success or negative error value on failure
> + *
> + * Set a torch LED's brightness.
> + */
> +extern int led_set_torch_brightness(struct led_classdev *led_cdev,
> + enum led_brightness brightness);
> +/**
> * led_sysfs_disable - disable LED sysfs interface
> * @led_cdev: the LED to set
> *
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem
2014-09-22 15:21 ` [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
@ 2014-11-04 1:34 ` Bryan Wu
2014-11-04 9:21 ` Jacek Anaszewski
0 siblings, 1 reply; 9+ messages in thread
From: Bryan Wu @ 2014-11-04 1:34 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Linux LED Subsystem, linux-media@vger.kernel.org, Kyungmin Park,
b.zolnierkie, Richard Purdie
On Mon, Sep 22, 2014 at 8:21 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> Some LED devices support two operation modes - torch and flash.
I got several terms here:
flash, torch and indicator.
And we have 3 CAPs
CAP_TORCH
CAP_FLASH
CAP_INDICATOR
I assume flash == indicator but it doesn't from the code. So what's
the difference between flash and indicator.
> 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.
>
There is no torch interface? only flash and indicator.
> 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 | 557 +++++++++++++++++++++++++++++++++++++++
> drivers/leds/led-class.c | 4 +
> include/linux/led-class-flash.h | 238 +++++++++++++++++
> include/linux/leds.h | 3 +
> 6 files changed, 814 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 8c96e2d..3c58021 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -22,6 +22,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 d8cc5f2..9238b8a 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..f1ba539
> --- /dev/null
> +++ b/drivers/leds/led-class-flash.c
> @@ -0,0 +1,557 @@
> +/*
> + * 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 struct attribute_group led_flash_strobe_group = {
> + .attrs = led_flash_strobe_attrs,
> +};
> +
> +static struct attribute_group led_flash_brightness_group = {
> + .attrs = led_flash_brightness_attrs,
> +};
> +
> +static struct attribute_group led_flash_timeout_group = {
> + .attrs = led_flash_timeout_attrs,
> +};
> +
> +static struct attribute_group led_flash_indicator_group = {
> + .attrs = led_flash_indicator_attrs,
> +};
> +
> +static struct attribute_group led_flash_fault_group = {
> + .attrs = led_flash_fault_attrs,
> +};
> +
> +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_remove_sysfs_groups(struct led_classdev_flash *flash)
> +{
> + struct led_classdev *led_cdev = &flash->led_cdev;
> + int i;
> +
> + for (i = 0; i < LED_FLASH_MAX_SYSFS_GROUPS; ++i)
> + if (flash->sysfs_groups[i])
> + sysfs_remove_group(&led_cdev->dev->kobj,
> + flash->sysfs_groups[i]);
> +}
> +
> +static int led_flash_create_sysfs_groups(struct led_classdev_flash *flash)
> +{
> + struct led_classdev *led_cdev = &flash->led_cdev;
> + const struct led_flash_ops *ops = flash->ops;
> + int ret, num_sysfs_groups = 0;
> +
> + memset(flash->sysfs_groups, 0, sizeof(*flash->sysfs_groups) *
> + LED_FLASH_MAX_SYSFS_GROUPS);
> +
> + ret = sysfs_create_group(&led_cdev->dev->kobj, &led_flash_strobe_group);
> + if (ret < 0)
> + goto err_create_group;
> + flash->sysfs_groups[num_sysfs_groups++] = &led_flash_strobe_group;
> +
> + if (flash->indicator_brightness) {
> + ret = sysfs_create_group(&led_cdev->dev->kobj,
> + &led_flash_indicator_group);
> + if (ret < 0)
> + goto err_create_group;
> + flash->sysfs_groups[num_sysfs_groups++] =
> + &led_flash_indicator_group;
> + }
> +
> + if (ops->flash_brightness_set) {
> + ret = sysfs_create_group(&led_cdev->dev->kobj,
> + &led_flash_brightness_group);
> + if (ret < 0)
> + goto err_create_group;
> + flash->sysfs_groups[num_sysfs_groups++] =
> + &led_flash_brightness_group;
> + }
> +
> + if (ops->timeout_set) {
> + ret = sysfs_create_group(&led_cdev->dev->kobj,
> + &led_flash_timeout_group);
> + if (ret < 0)
> + goto err_create_group;
> + flash->sysfs_groups[num_sysfs_groups++] =
> + &led_flash_timeout_group;
> + }
> +
> + if (ops->fault_get) {
> + ret = sysfs_create_group(&led_cdev->dev->kobj,
> + &led_flash_fault_group);
> + if (ret < 0)
> + goto err_create_group;
> + flash->sysfs_groups[num_sysfs_groups++] =
> + &led_flash_fault_group;
> + }
> +
> + return 0;
> +
> +err_create_group:
> + led_flash_remove_sysfs_groups(flash);
> + return ret;
> +}
> +
> +int led_classdev_flash_register(struct device *parent,
> + struct led_classdev_flash *flash,
> + struct device_node *node)
> +{
> + struct led_classdev *led_cdev;
> + const struct led_flash_ops *ops;
> + int ret = -EINVAL;
> +
> + if (!flash)
> + return -EINVAL;
> +
> + led_cdev = &flash->led_cdev;
> +
> + /* Torch capability is default for every LED Flash Class device */
> + led_cdev->flags |= LED_DEV_CAP_TORCH;
> +
> + if (led_cdev->flags & LED_DEV_CAP_FLASH) {
> + if (!led_cdev->torch_brightness_set)
> + 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;
> + }
> +
> + /* Register led class device */
> + ret = led_classdev_register(parent, led_cdev);
> + if (ret < 0)
> + return -EINVAL;
> +
> + /* Create flash led specific sysfs attributes */
> + ret = led_flash_create_sysfs_groups(flash);
> + if (ret < 0)
> + goto err_create_sysfs_groups;
> +
Here might cause some race condition. Please check the commits
commit d0d480cce8f522b37c2c1de38230fc9ad15fa506
Author: Johan Hovold <johan@kernel.org>
Date: Wed Jun 25 10:08:44 2014 -0700
leds: add led-class attribute-group support
Allow led-class devices to be created with optional attribute groups.
This is needed in order to allow led drivers to create custom device
attributes in a race-free manner.
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Bryan Wu <cooloney@gmail.com>
And related fixes in drivers/leds/ recently.
> + return 0;
> +
> +err_create_sysfs_groups:
> + led_classdev_unregister(led_cdev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(led_classdev_flash_register);
> +
> +void led_classdev_flash_unregister(struct led_classdev_flash *flash)
> +{
> + if (!flash)
> + return;
> +
> + led_flash_remove_sysfs_groups(flash);
> + 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)) {
I think call_flash_op() will call has_flash_op() in your previous definitions.
> + 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 5a11a07..d398b87 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -183,6 +183,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..fcc15ed
> --- /dev/null
> +++ b/include/linux/led-class-flash.h
> @@ -0,0 +1,238 @@
> +/*
> + * 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
> +
> +#define LED_FLASH_MAX_SYSFS_GROUPS 5
> +
> +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-flash-manager uses it to link flashes */
I guess no more led-flash-manager, right?
> + struct list_head list;
> + /* led class device */
> + struct led_classdev led_cdev;
> + /* flash led specific ops */
> + const struct led_flash_ops *ops;
> +
> + /* flash sysfs groups */
> + struct attribute_group *sysfs_groups[LED_FLASH_MAX_SYSFS_GROUPS];
> +
> + /* 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
> + * @node: device tree node of the LED Flash Class device - it must be
> + initialized if the device is to be registered in the flash manager
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +int led_classdev_flash_register(struct device *parent,
> + struct led_classdev_flash *flash,
> + struct device_node *node);
> +
> +/**
> + * led_classdev_flash_unregister - unregisters an object of led_classdev class
> + with support for flash LEDs
> + * @flash: the flash LED to unregister
> + *
> + * Unregisters 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
> + *
> + * Setup flash strobe - trigger flash strobe
> + *
> + * 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 or not.
> + *
> +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
> + *
> + * Returns: 0 on success or negative error value on failure
> + *
> + * Set a flash LED's brightness.
> + */
> +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 bc2a570..70fa0ad 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -45,6 +45,8 @@ struct led_classdev {
> #define LED_BLINK_INVERT (1 << 19)
> #define LED_SYSFS_DISABLE (1 << 20)
> #define LED_DEV_CAP_TORCH (1 << 21)
> +#define LED_DEV_CAP_FLASH (1 << 22)
> +#define LED_DEV_CAP_INDICATOR (1 << 23)
>
> /* Set LED brightness level */
> /* Must not sleep, use a workqueue if needed */
> @@ -83,6 +85,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 [flat|nested] 9+ messages in thread
* Re: [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism
2014-10-21 1:01 ` Bryan Wu
@ 2014-11-04 1:37 ` Bryan Wu
0 siblings, 0 replies; 9+ messages in thread
From: Bryan Wu @ 2014-11-04 1:37 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Linux LED Subsystem, linux-media@vger.kernel.org, Kyungmin Park,
b.zolnierkie, Richard Purdie
On Mon, Oct 20, 2014 at 6:01 PM, Bryan Wu <cooloney@gmail.com> wrote:
> On Mon, Sep 22, 2014 at 8:21 AM, Jacek Anaszewski
> <j.anaszewski@samsung.com> wrote:
>> Add a mechanism for locking LED subsystem sysfs interface.
>> This patch prepares ground for addition of LED Flash Class
>> extension, whose API will be integrated with V4L2 Flash API.
>> Such a fusion enforces introducing a locking scheme, which
>> will secure consistent access to the LED Flash Class device.
>>
>> The mechanism being introduced allows for disabling LED
>> subsystem sysfs interface by calling led_sysfs_disable function
>> and enabling it by calling led_sysfs_enable. The functions
>> alter the LED_SYSFS_DISABLE flag state and must be called
>> under mutex lock. The state of the lock is checked with use
>> of led_sysfs_is_disabled function. Such a design allows for
>> providing immediate feedback to the user space on whether
>> the LED Flash Class device is available or is under V4L2 Flash
>> sub-device control.
>>
>
> I'm good with this and will merge it soon.
>
Just merged it into my tree.
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 | 19 ++++++++++++++++---
>> drivers/leds/led-core.c | 18 ++++++++++++++++++
>> drivers/leds/led-triggers.c | 16 +++++++++++++---
>> include/linux/leds.h | 32 ++++++++++++++++++++++++++++++++
>> 4 files changed, 79 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index 2e124aa2..a39ca8f 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -39,17 +39,27 @@ static ssize_t brightness_store(struct device *dev,
>> {
>> struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> unsigned long state;
>> - ssize_t ret = -EINVAL;
>> + 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)
>> - return ret;
>> + goto unlock;
>>
>> if (state == LED_OFF)
>> led_trigger_remove(led_cdev);
>> __led_set_brightness(led_cdev, state);
>>
>> - return size;
>> + ret = size;
>> +unlock:
>> + mutex_unlock(&led_cdev->led_access);
>> + return ret;
>> }
>> static DEVICE_ATTR_RW(brightness);
>>
>> @@ -213,6 +223,7 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
>> #ifdef CONFIG_LEDS_TRIGGERS
>> init_rwsem(&led_cdev->trigger_lock);
>> #endif
>> + mutex_init(&led_cdev->led_access);
>> /* add to the list of leds */
>> down_write(&leds_list_lock);
>> list_add_tail(&led_cdev->node, &leds_list);
>> @@ -266,6 +277,8 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
>> down_write(&leds_list_lock);
>> list_del(&led_cdev->node);
>> up_write(&leds_list_lock);
>> +
>> + mutex_destroy(&led_cdev->led_access);
>> }
>> EXPORT_SYMBOL_GPL(led_classdev_unregister);
>>
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index 0d15aa9..cca86ab 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -142,3 +142,21 @@ int led_update_brightness(struct led_classdev *led_cdev)
>> return ret;
>> }
>> EXPORT_SYMBOL(led_update_brightness);
>> +
>> +/* Caller must ensure led_cdev->led_access held */
>> +void led_sysfs_disable(struct led_classdev *led_cdev)
>> +{
>> + lockdep_assert_held(&led_cdev->led_access);
>> +
>> + led_cdev->flags |= LED_SYSFS_DISABLE;
>> +}
>> +EXPORT_SYMBOL_GPL(led_sysfs_disable);
>> +
>> +/* Caller must ensure led_cdev->led_access held */
>> +void led_sysfs_enable(struct led_classdev *led_cdev)
>> +{
>> + lockdep_assert_held(&led_cdev->led_access);
>> +
>> + led_cdev->flags &= ~LED_SYSFS_DISABLE;
>> +}
>> +EXPORT_SYMBOL_GPL(led_sysfs_enable);
>> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
>> index c3734f1..e8b1120 100644
>> --- a/drivers/leds/led-triggers.c
>> +++ b/drivers/leds/led-triggers.c
>> @@ -37,6 +37,14 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
>> char trigger_name[TRIG_NAME_MAX];
>> struct led_trigger *trig;
>> size_t len;
>> + int ret = count;
>> +
>> + mutex_lock(&led_cdev->led_access);
>> +
>> + if (led_sysfs_is_disabled(led_cdev)) {
>> + ret = -EBUSY;
>> + goto unlock;
>> + }
>>
>> trigger_name[sizeof(trigger_name) - 1] = '\0';
>> strncpy(trigger_name, buf, sizeof(trigger_name) - 1);
>> @@ -47,7 +55,7 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
>>
>> if (!strcmp(trigger_name, "none")) {
>> led_trigger_remove(led_cdev);
>> - return count;
>> + goto unlock;
>> }
>>
>> down_read(&triggers_list_lock);
>> @@ -58,12 +66,14 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
>> up_write(&led_cdev->trigger_lock);
>>
>> up_read(&triggers_list_lock);
>> - return count;
>> + goto unlock;
>> }
>> }
>> up_read(&triggers_list_lock);
>>
>> - return -EINVAL;
>> +unlock:
>> + mutex_unlock(&led_cdev->led_access);
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(led_trigger_store);
>>
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index f8b2f58..44c8a98 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -13,6 +13,7 @@
>> #define __LINUX_LEDS_H_INCLUDED
>>
>> #include <linux/list.h>
>> +#include <linux/mutex.h>
>> #include <linux/rwsem.h>
>> #include <linux/spinlock.h>
>> #include <linux/timer.h>
>> @@ -42,6 +43,7 @@ struct led_classdev {
>> #define LED_BLINK_ONESHOT (1 << 17)
>> #define LED_BLINK_ONESHOT_STOP (1 << 18)
>> #define LED_BLINK_INVERT (1 << 19)
>> +#define LED_SYSFS_DISABLE (1 << 20)
>>
>> /* Set LED brightness level */
>> /* Must not sleep, use a workqueue if needed */
>> @@ -85,6 +87,9 @@ struct led_classdev {
>> /* true if activated - deactivate routine uses it to do cleanup */
>> bool activated;
>> #endif
>> +
>> + /* Ensures consistent access to the LED Flash Class device */
>> + struct mutex led_access;
>> };
>>
>> extern int led_classdev_register(struct device *parent,
>> @@ -151,6 +156,33 @@ extern void led_set_brightness(struct led_classdev *led_cdev,
>> */
>> extern int led_update_brightness(struct led_classdev *led_cdev);
>>
>> +/**
>> + * led_sysfs_disable - disable LED sysfs interface
>> + * @led_cdev: the LED to set
>> + *
>> + * Disable the led_cdev's sysfs interface.
>> + */
>> +extern void led_sysfs_disable(struct led_classdev *led_cdev);
>> +
>> +/**
>> + * led_sysfs_enable - enable LED sysfs interface
>> + * @led_cdev: the LED to set
>> + *
>> + * Enable the led_cdev's sysfs interface.
>> + */
>> +extern void led_sysfs_enable(struct led_classdev *led_cdev);
>> +
>> +/**
>> + * led_sysfs_is_disabled - check if LED sysfs interface is disabled
>> + * @led_cdev: the LED to query
>> + *
>> + * Returns: true if the led_cdev's sysfs interface is disabled.
>> + */
>> +static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
>> +{
>> + return led_cdev->flags & LED_SYSFS_DISABLE;
>> +}
>> +
>> /*
>> * LED Triggers
>> */
>> --
>> 1.7.9.5
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem
2014-11-04 1:34 ` Bryan Wu
@ 2014-11-04 9:21 ` Jacek Anaszewski
0 siblings, 0 replies; 9+ messages in thread
From: Jacek Anaszewski @ 2014-11-04 9:21 UTC (permalink / raw)
To: Bryan Wu
Cc: Linux LED Subsystem, linux-media@vger.kernel.org, Kyungmin Park,
b.zolnierkie, Richard Purdie, Sakari Ailus
Hi Bryan,
Thanks for a review.
On 11/04/2014 02:34 AM, Bryan Wu wrote:
> On Mon, Sep 22, 2014 at 8:21 AM, Jacek Anaszewski
> <j.anaszewski@samsung.com> wrote:
>> Some LED devices support two operation modes - torch and flash.
>
> I got several terms here:
> flash, torch and indicator.
>
> And we have 3 CAPs
> CAP_TORCH
> CAP_FLASH
> CAP_INDICATOR
>
> I assume flash == indicator but it doesn't from the code. So what's
> the difference between flash and indicator.
Indicator is a so-called "privacy led", that can be used to indicate
when a person is being photographed or filmed. V4L2 API defines
only an API for setting its intensity.
Adding Sakari.
>> 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.
>>
>
> There is no torch interface? only flash and indicator.
LED Class interface is used for torch.
>> 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 | 557 +++++++++++++++++++++++++++++++++++++++
>> drivers/leds/led-class.c | 4 +
>> include/linux/led-class-flash.h | 238 +++++++++++++++++
>> include/linux/leds.h | 3 +
>> 6 files changed, 814 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 8c96e2d..3c58021 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -22,6 +22,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 d8cc5f2..9238b8a 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..f1ba539
>> --- /dev/null
>> +++ b/drivers/leds/led-class-flash.c
>> @@ -0,0 +1,557 @@
>> +/*
>> + * 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 struct attribute_group led_flash_strobe_group = {
>> + .attrs = led_flash_strobe_attrs,
>> +};
>> +
>> +static struct attribute_group led_flash_brightness_group = {
>> + .attrs = led_flash_brightness_attrs,
>> +};
>> +
>> +static struct attribute_group led_flash_timeout_group = {
>> + .attrs = led_flash_timeout_attrs,
>> +};
>> +
>> +static struct attribute_group led_flash_indicator_group = {
>> + .attrs = led_flash_indicator_attrs,
>> +};
>> +
>> +static struct attribute_group led_flash_fault_group = {
>> + .attrs = led_flash_fault_attrs,
>> +};
>> +
>> +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_remove_sysfs_groups(struct led_classdev_flash *flash)
>> +{
>> + struct led_classdev *led_cdev = &flash->led_cdev;
>> + int i;
>> +
>> + for (i = 0; i < LED_FLASH_MAX_SYSFS_GROUPS; ++i)
>> + if (flash->sysfs_groups[i])
>> + sysfs_remove_group(&led_cdev->dev->kobj,
>> + flash->sysfs_groups[i]);
>> +}
>> +
>> +static int led_flash_create_sysfs_groups(struct led_classdev_flash *flash)
>> +{
>> + struct led_classdev *led_cdev = &flash->led_cdev;
>> + const struct led_flash_ops *ops = flash->ops;
>> + int ret, num_sysfs_groups = 0;
>> +
>> + memset(flash->sysfs_groups, 0, sizeof(*flash->sysfs_groups) *
>> + LED_FLASH_MAX_SYSFS_GROUPS);
>> +
>> + ret = sysfs_create_group(&led_cdev->dev->kobj, &led_flash_strobe_group);
>> + if (ret < 0)
>> + goto err_create_group;
>> + flash->sysfs_groups[num_sysfs_groups++] = &led_flash_strobe_group;
>> +
>> + if (flash->indicator_brightness) {
>> + ret = sysfs_create_group(&led_cdev->dev->kobj,
>> + &led_flash_indicator_group);
>> + if (ret < 0)
>> + goto err_create_group;
>> + flash->sysfs_groups[num_sysfs_groups++] =
>> + &led_flash_indicator_group;
>> + }
>> +
>> + if (ops->flash_brightness_set) {
>> + ret = sysfs_create_group(&led_cdev->dev->kobj,
>> + &led_flash_brightness_group);
>> + if (ret < 0)
>> + goto err_create_group;
>> + flash->sysfs_groups[num_sysfs_groups++] =
>> + &led_flash_brightness_group;
>> + }
>> +
>> + if (ops->timeout_set) {
>> + ret = sysfs_create_group(&led_cdev->dev->kobj,
>> + &led_flash_timeout_group);
>> + if (ret < 0)
>> + goto err_create_group;
>> + flash->sysfs_groups[num_sysfs_groups++] =
>> + &led_flash_timeout_group;
>> + }
>> +
>> + if (ops->fault_get) {
>> + ret = sysfs_create_group(&led_cdev->dev->kobj,
>> + &led_flash_fault_group);
>> + if (ret < 0)
>> + goto err_create_group;
>> + flash->sysfs_groups[num_sysfs_groups++] =
>> + &led_flash_fault_group;
>> + }
>> +
>> + return 0;
>> +
>> +err_create_group:
>> + led_flash_remove_sysfs_groups(flash);
>> + return ret;
>> +}
>> +
>> +int led_classdev_flash_register(struct device *parent,
>> + struct led_classdev_flash *flash,
>> + struct device_node *node)
>> +{
>> + struct led_classdev *led_cdev;
>> + const struct led_flash_ops *ops;
>> + int ret = -EINVAL;
>> +
>> + if (!flash)
>> + return -EINVAL;
>> +
>> + led_cdev = &flash->led_cdev;
>> +
>> + /* Torch capability is default for every LED Flash Class device */
>> + led_cdev->flags |= LED_DEV_CAP_TORCH;
>> +
>> + if (led_cdev->flags & LED_DEV_CAP_FLASH) {
>> + if (!led_cdev->torch_brightness_set)
>> + 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;
>> + }
>> +
>> + /* Register led class device */
>> + ret = led_classdev_register(parent, led_cdev);
>> + if (ret < 0)
>> + return -EINVAL;
>> +
>> + /* Create flash led specific sysfs attributes */
>> + ret = led_flash_create_sysfs_groups(flash);
>> + if (ret < 0)
>> + goto err_create_sysfs_groups;
>> +
>
> Here might cause some race condition. Please check the commits
> commit d0d480cce8f522b37c2c1de38230fc9ad15fa506
> Author: Johan Hovold <johan@kernel.org>
> Date: Wed Jun 25 10:08:44 2014 -0700
>
> leds: add led-class attribute-group support
>
> Allow led-class devices to be created with optional attribute groups.
>
> This is needed in order to allow led drivers to create custom device
> attributes in a race-free manner.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> Signed-off-by: Bryan Wu <cooloney@gmail.com>
>
> And related fixes in drivers/leds/ recently.
OK, I will modify the code appropriately.
>> + return 0;
>> +
>> +err_create_sysfs_groups:
>> + led_classdev_unregister(led_cdev);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(led_classdev_flash_register);
>> +
>> +void led_classdev_flash_unregister(struct led_classdev_flash *flash)
>> +{
>> + if (!flash)
>> + return;
>> +
>> + led_flash_remove_sysfs_groups(flash);
>> + 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)) {
> I think call_flash_op() will call has_flash_op() in your previous definitions.
call_flash_op fails when the op is not initialized. Since the op is
optional we don't want the led_update_flash_brightness to fail when
it is not defined. This is consistent with the way how
led_update_brightness works. The op presence is checked prior calling
it and success is reported even if it doesn't exist.
Relevant comment should probably be added here.
>> + 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 5a11a07..d398b87 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -183,6 +183,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..fcc15ed
>> --- /dev/null
>> +++ b/include/linux/led-class-flash.h
>> @@ -0,0 +1,238 @@
>> +/*
>> + * 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
>> +
>> +#define LED_FLASH_MAX_SYSFS_GROUPS 5
>> +
>> +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-flash-manager uses it to link flashes */
> I guess no more led-flash-manager, right?
Yes, this is stray code, I will remove it.
>> + struct list_head list;
>> + /* led class device */
>> + struct led_classdev led_cdev;
>> + /* flash led specific ops */
>> + const struct led_flash_ops *ops;
>> +
>> + /* flash sysfs groups */
>> + struct attribute_group *sysfs_groups[LED_FLASH_MAX_SYSFS_GROUPS];
>> +
>> + /* 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
>> + * @node: device tree node of the LED Flash Class device - it must be
>> + initialized if the device is to be registered in the flash manager
>> + *
>> + * Returns: 0 on success or negative error value on failure
>> + */
>> +int led_classdev_flash_register(struct device *parent,
>> + struct led_classdev_flash *flash,
>> + struct device_node *node);
>> +
>> +/**
>> + * led_classdev_flash_unregister - unregisters an object of led_classdev class
>> + with support for flash LEDs
>> + * @flash: the flash LED to unregister
>> + *
>> + * Unregisters 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
>> + *
>> + * Setup flash strobe - trigger flash strobe
>> + *
>> + * 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 or not.
>> + *
>> +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
>> + *
>> + * Returns: 0 on success or negative error value on failure
>> + *
>> + * Set a flash LED's brightness.
>> + */
>> +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 bc2a570..70fa0ad 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -45,6 +45,8 @@ struct led_classdev {
>> #define LED_BLINK_INVERT (1 << 19)
>> #define LED_SYSFS_DISABLE (1 << 20)
>> #define LED_DEV_CAP_TORCH (1 << 21)
>> +#define LED_DEV_CAP_FLASH (1 << 22)
>> +#define LED_DEV_CAP_INDICATOR (1 << 23)
>>
>> /* Set LED brightness level */
>> /* Must not sleep, use a workqueue if needed */
>> @@ -83,6 +85,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 [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-11-04 9:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-22 15:21 [PATCH/RFC v6 0/3] LED / flash API integration - LED Flash Class Jacek Anaszewski
2014-09-22 15:21 ` [PATCH/RFC v6 1/3] leds: implement sysfs interface locking mechanism Jacek Anaszewski
2014-10-21 1:01 ` Bryan Wu
2014-11-04 1:37 ` Bryan Wu
2014-09-22 15:21 ` [PATCH/RFC v6 2/3] leds: add API for setting torch brightness Jacek Anaszewski
2014-10-21 1:05 ` Bryan Wu
2014-09-22 15:21 ` [PATCH/RFC v6 3/3] leds: Add LED Flash Class wrapper to LED subsystem Jacek Anaszewski
2014-11-04 1:34 ` Bryan Wu
2014-11-04 9: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).