* [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-20 15:28 ` Richard Purdie
2014-03-23 23:18 ` Sakari Ailus
2014-03-20 14:51 ` [PATCH/RFC 2/8] leds: Improve and export led_update_brightness function Jacek Anaszewski
` (6 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, 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_mode, flash_timeout, max_flash_timeout,
flash_fault and hw_triggered.
The modifications aim to be compatible with V4L2 framework
requirements related to the flash devices management. The
design assumes that V4L2 driver can take of the LED class
device control and communicate with it through the kernel
internal interface. The LED sysfs interface is made
unavailable then.
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 | 216 +++++++++++++++++++++++++++++++++++++++++--
drivers/leds/led-core.c | 124 +++++++++++++++++++++++--
drivers/leds/led-triggers.c | 17 +++-
drivers/leds/leds.h | 9 ++
include/linux/leds.h | 136 +++++++++++++++++++++++++++
5 files changed, 486 insertions(+), 16 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f37d63c..0510532 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -4,6 +4,9 @@
* Copyright (C) 2005 John Lenz <lenz@cs.wisc.edu>
* Copyright (C) 2005-2007 Richard Purdie <rpurdie@openedhand.com>
*
+ * 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.
@@ -13,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/list.h>
+#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/device.h>
#include <linux/timer.h>
@@ -45,28 +49,186 @@ 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_lock);
+
+ if (led_sysfs_is_locked(led_cdev)) {
+ ret = -EBUSY;
+ goto exit_unlock;
+ }
ret = kstrtoul(buf, 10, &state);
if (ret)
- return ret;
+ goto exit_unlock;
if (state == LED_OFF)
led_trigger_remove(led_cdev);
- __led_set_brightness(led_cdev, state);
+ led_set_brightness(led_cdev, state);
+ ret = size;
- return size;
+exit_unlock:
+ mutex_unlock(&led_cdev->led_lock);
+ return ret;
}
static DEVICE_ATTR_RW(brightness);
-static ssize_t led_max_brightness_show(struct device *dev,
+static ssize_t max_brightness_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct led_classdev *led_cdev = dev_get_drvdata(dev);
return sprintf(buf, "%u\n", led_cdev->max_brightness);
}
-static DEVICE_ATTR(max_brightness, 0444, led_max_brightness_show, NULL);
+static DEVICE_ATTR_RO(max_brightness);
+
+static ssize_t flash_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t size)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ unsigned long flash_mode;
+ ssize_t ret;
+
+ mutex_lock(&led_cdev->led_lock);
+
+ if (led_sysfs_is_locked(led_cdev)) {
+ ret = -EBUSY;
+ goto exit_unlock;
+ }
+
+ ret = kstrtoul(buf, 10, &flash_mode);
+ if (ret)
+ goto exit_unlock;
+
+ if (flash_mode < 0 && flash_mode > 1)
+ return -EINVAL;
+
+ if (led_is_flash_mode(led_cdev) == flash_mode) {
+ ret = size;
+ goto exit_unlock;
+ }
+
+ led_trigger_remove(led_cdev);
+
+ led_set_flash_mode(led_cdev, flash_mode);
+ ret = size;
+
+exit_unlock:
+ mutex_unlock(&led_cdev->led_lock);
+ return ret;
+}
+
+static ssize_t flash_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", led_is_flash_mode(led_cdev));
+}
+static DEVICE_ATTR_RW(flash_mode);
+
+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);
+ unsigned long flash_timeout;
+ ssize_t ret;
+
+ mutex_lock(&led_cdev->led_lock);
+
+ if (led_sysfs_is_locked(led_cdev)) {
+ ret = -EBUSY;
+ goto exit_unlock;
+ }
+
+ ret = kstrtoul(buf, 10, &flash_timeout);
+ if (ret)
+ goto exit_unlock;
+
+ led_set_flash_timeout(led_cdev, flash_timeout);
+ ret = size;
+
+exit_unlock:
+ mutex_unlock(&led_cdev->led_lock);
+ 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_flash *flash = led_cdev->flash;
+
+ return sprintf(buf, "%lu\n", flash->timeout);
+}
+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_flash *flash = led_cdev->flash;
+
+ return sprintf(buf, "%lu\n", flash->max_timeout);
+}
+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);
+ unsigned int fault;
+ int ret;
+
+ ret = led_get_flash_fault(led_cdev, &fault);
+ if (ret < 0)
+ return -EINVAL;
+
+ return sprintf(buf, "%x\n", fault);
+}
+static DEVICE_ATTR_RO(flash_fault);
+
+static ssize_t hw_triggered_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t size)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ unsigned long hw_triggered;
+ ssize_t ret;
+
+ mutex_lock(&led_cdev->led_lock);
+
+ if (led_sysfs_is_locked(led_cdev)) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+
+ ret = kstrtoul(buf, 10, &hw_triggered);
+ if (ret)
+ goto unlock;
+
+ if (hw_triggered > 1) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ ret = led_set_hw_triggered(led_cdev, hw_triggered);
+ if (ret < 0)
+ goto unlock;
+ ret = size;
+
+unlock:
+ mutex_unlock(&led_cdev->led_lock);
+ return ret;
+}
+
+static ssize_t hw_triggered_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", led_cdev->hw_triggered);
+}
+static DEVICE_ATTR_RW(hw_triggered);
#ifdef CONFIG_LEDS_TRIGGERS
static DEVICE_ATTR(trigger, 0644, led_trigger_show, led_trigger_store);
@@ -82,6 +244,11 @@ static const struct attribute_group led_trigger_group = {
static struct attribute *led_class_attrs[] = {
&dev_attr_brightness.attr,
&dev_attr_max_brightness.attr,
+ &dev_attr_flash_mode.attr,
+ &dev_attr_flash_timeout.attr,
+ &dev_attr_max_flash_timeout.attr,
+ &dev_attr_flash_fault.attr,
+ &dev_attr_hw_triggered.attr,
NULL,
};
@@ -204,20 +371,54 @@ static const struct dev_pm_ops leds_class_dev_pm_ops = {
};
/**
+ * led_classdev_init_flash - add support for flash led
+ * @led_cdev: the device to add flash led support to
+ *
+ * Returns: 0 on success, error code on failure.
+ */
+int led_classdev_init_flash(struct led_classdev *led_cdev)
+{
+ if (led_cdev->flash)
+ return -EINVAL;
+
+ led_cdev->flash = kzalloc(sizeof(struct led_flash), GFP_KERNEL);
+ if (!led_cdev->flash)
+ return -ENOMEM;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(led_classdev_init_flash);
+
+/**
* led_classdev_register - register a new object of led_classdev class.
* @parent: The device to register.
* @led_cdev: the led_classdev structure for this device.
*/
int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
{
+ struct led_flash_ops *ops;
+
+ if (led_cdev->flash) {
+ ops = &led_cdev->flash->ops;
+ if (!ops || !ops->strobe_set || !ops->mode_set ||
+ !ops->fault_get) {
+ dev_dbg(parent,
+ "Flash LED ops validation failed for the %s\n"
+ "LED device", led_cdev->name);
+ return -EINVAL;
+ }
+ }
+
led_cdev->dev = device_create(leds_class, parent, 0, led_cdev,
"%s", led_cdev->name);
if (IS_ERR(led_cdev->dev))
return PTR_ERR(led_cdev->dev);
+
#ifdef CONFIG_LEDS_TRIGGERS
init_rwsem(&led_cdev->trigger_lock);
#endif
+ mutex_init(&led_cdev->led_lock);
/* add to the list of leds */
down_write(&leds_list_lock);
list_add_tail(&led_cdev->node, &leds_list);
@@ -271,6 +472,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);
+
+ kfree(led_cdev->flash);
}
EXPORT_SYMBOL_GPL(led_classdev_unregister);
@@ -293,5 +496,6 @@ subsys_initcall(leds_init);
module_exit(leds_exit);
MODULE_AUTHOR("John Lenz, Richard Purdie");
+MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("LED Class Interface");
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 71b40d3..093703c 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -5,6 +5,9 @@
*
* Author: Richard Purdie <rpurdie@openedhand.com>
*
+ * 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.
@@ -71,10 +74,27 @@ static void led_blink_setup(struct led_classdev *led_cdev,
led_set_software_blink(led_cdev, *delay_on, *delay_off);
}
+static int led_flash_strobe_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness,
+ unsigned long *timeout)
+{
+ if (!get_flash_op(led_cdev, strobe_set))
+ return -EINVAL;
+
+ if (brightness > led_cdev->max_brightness)
+ brightness = led_cdev->max_brightness;
+ call_flash_op(strobe_set, led_cdev, brightness, timeout);
+
+ return 0;
+}
+
void led_blink_set(struct led_classdev *led_cdev,
unsigned long *delay_on,
unsigned long *delay_off)
{
+ if (led_is_flash_mode(led_cdev))
+ return;
+
del_timer_sync(&led_cdev->blink_timer);
led_cdev->flags &= ~LED_BLINK_ONESHOT;
@@ -89,6 +109,9 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
unsigned long *delay_off,
int invert)
{
+ if (led_is_flash_mode(led_cdev))
+ return;
+
if ((led_cdev->flags & LED_BLINK_ONESHOT) &&
timer_pending(&led_cdev->blink_timer))
return;
@@ -116,13 +139,100 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
void led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
- /* delay brightness setting if need to stop soft-blink timer */
- if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
- led_cdev->delayed_set_value = brightness;
- schedule_work(&led_cdev->set_brightness_work);
- return;
+ struct led_flash *flash = led_cdev->flash;
+ int ret;
+
+ if (led_is_flash_mode(led_cdev)) {
+ if (brightness > 0) {
+ ret = led_flash_strobe_set(led_cdev,
+ brightness,
+ &flash->timeout);
+ if (ret < 0)
+ dev_err(led_cdev->dev,
+ "Failed to setup flash strobe (%d)",
+ ret);
+ } else {
+ __led_set_brightness(led_cdev, 0);
+ }
+ } else {
+ /* delay brightness setting if need to stop soft-blink timer */
+ if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
+ led_cdev->delayed_set_value = brightness;
+ schedule_work(&led_cdev->set_brightness_work);
+ return;
+ }
+ __led_set_brightness(led_cdev, brightness);
}
-
- __led_set_brightness(led_cdev, brightness);
}
EXPORT_SYMBOL(led_set_brightness);
+
+int led_set_flash_mode(struct led_classdev *led_cdev,
+ bool flash_mode)
+{
+ if (!get_flash_op(led_cdev, mode_set))
+ return -EINVAL;
+
+ call_flash_op(mode_set, led_cdev, flash_mode);
+
+ if (flash_mode)
+ led_cdev->flags |= LED_MODE_FLASH;
+ else
+ led_cdev->flags &= ~LED_MODE_FLASH;
+
+ led_set_brightness(led_cdev, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(led_set_flash_mode);
+
+/* Caller must ensure led_cdev->led_lock held */
+void led_sysfs_lock(struct led_classdev *led_cdev)
+{
+ led_cdev->flags |= LED_SYSFS_LOCK;
+}
+EXPORT_SYMBOL(led_sysfs_lock);
+
+/* Caller must ensure led_cdev->led_lock held */
+void led_sysfs_unlock(struct led_classdev *led_cdev)
+{
+ led_cdev->flags &= ~LED_SYSFS_LOCK;
+}
+EXPORT_SYMBOL(led_sysfs_unlock);
+
+void led_set_flash_timeout(struct led_classdev *led_cdev, unsigned long timeout)
+{
+ struct led_flash *flash = led_cdev->flash;
+
+ /*
+ * Flash timeout is passed to a flash LED driver
+ * through the strobe_set callback - here we only
+ * cache the value.
+ */
+ if (timeout > flash->max_timeout)
+ flash->timeout = flash->max_timeout;
+ else
+ flash->timeout = timeout;
+}
+EXPORT_SYMBOL(led_set_flash_timeout);
+
+int led_get_flash_fault(struct led_classdev *led_cdev, unsigned int *fault)
+{
+ if (!get_flash_op(led_cdev, fault_get))
+ return -EINVAL;
+
+ return call_flash_op(fault_get, led_cdev, fault);
+}
+EXPORT_SYMBOL(led_get_flash_fault);
+
+int led_set_hw_triggered(struct led_classdev *led_cdev, bool enable)
+{
+ if (led_cdev->has_hw_trig) {
+ __led_set_brightness(led_cdev, 0);
+ led_cdev->hw_triggered = enable;
+ } else if (enable) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(led_set_hw_triggered);
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index df1a7c1..448e7c8 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -37,6 +37,15 @@ 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_lock);
+
+ if (led_sysfs_is_locked(led_cdev) ||
+ led_is_flash_mode(led_cdev)) {
+ ret = -EBUSY;
+ goto exit_unlock;
+ }
trigger_name[sizeof(trigger_name) - 1] = '\0';
strncpy(trigger_name, buf, sizeof(trigger_name) - 1);
@@ -47,7 +56,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 exit_unlock;
}
down_read(&triggers_list_lock);
@@ -58,12 +67,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 exit_unlock;
}
}
up_read(&triggers_list_lock);
- return -EINVAL;
+exit_unlock:
+ mutex_unlock(&led_cdev->led_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(led_trigger_store);
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 4c50365..44cc384 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -17,6 +17,15 @@
#include <linux/rwsem.h>
#include <linux/leds.h>
+
+#define call_flash_op(op, args...) \
+ ((led_cdev)->flash->ops.op(args))
+
+#define get_flash_op(led_cdev, op) \
+ ((led_cdev)->flash ? \
+ (led_cdev)->flash->ops.op : \
+ 0)
+
static inline void __led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness value)
{
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 0287ab2..1bf0ab3 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -17,6 +17,14 @@
#include <linux/rwsem.h>
#include <linux/timer.h>
#include <linux/workqueue.h>
+#include <linux/mutex.h>
+#include <media/v4l2-device.h>
+
+#define LED_FAULT_OVER_VOLTAGE (1 << 0)
+#define LED_FAULT_TIMEOUT (1 << 1)
+#define LED_FAULT_OVER_TEMPERATURE (1 << 2)
+#define LED_FAULT_SHORT_CIRCUIT (1 << 3)
+#define LED_FAULT_OVER_CURRENT (1 << 4)
struct device;
/*
@@ -29,6 +37,33 @@ enum led_brightness {
LED_FULL = 255,
};
+struct led_classdev;
+
+struct led_flash_ops {
+ /* Set torch/flash LED mode */
+ void (*mode_set)(struct led_classdev *led_cdev,
+ bool flash_mode);
+ /* Setup flash strobe */
+ void (*strobe_set)(struct led_classdev *led_cdev,
+ enum led_brightness brightness,
+ unsigned long *timeout);
+ /* Get the flash LED fault */
+ int (*fault_get)(struct led_classdev *led_cdev,
+ unsigned int *fault);
+};
+
+struct led_flash {
+ /* flash led specific ops */
+ struct led_flash_ops ops;
+ /*
+ * maximum allowed flash timeout - it is read only and
+ * should be initialized by the driver
+ */
+ unsigned long max_timeout;
+ /* current flash timeout */
+ unsigned long timeout;
+};
+
struct led_classdev {
const char *name;
int brightness;
@@ -42,6 +77,8 @@ struct led_classdev {
#define LED_BLINK_ONESHOT (1 << 17)
#define LED_BLINK_ONESHOT_STOP (1 << 18)
#define LED_BLINK_INVERT (1 << 19)
+#define LED_MODE_FLASH (1 << 20)
+#define LED_SYSFS_LOCK (1 << 21)
/* Set LED brightness level */
/* Must not sleep, use a workqueue if needed */
@@ -69,6 +106,19 @@ struct led_classdev {
unsigned long blink_delay_on, blink_delay_off;
struct timer_list blink_timer;
int blink_brightness;
+ struct led_flash *flash;
+ /*
+ * Determines whether a device supports triggering a flash led
+ * with use of a dedicated hardware pin
+ */
+ bool has_hw_trig;
+ /* If true then hardware pin triggers flash strobe */
+ bool hw_triggered;
+ /*
+ * Ensures consistent LED sysfs access and protects
+ * LED sysfs locking mechanism
+ */
+ struct mutex led_lock;
struct work_struct set_brightness_work;
int delayed_set_value;
@@ -90,6 +140,7 @@ extern int led_classdev_register(struct device *parent,
extern void led_classdev_unregister(struct led_classdev *led_cdev);
extern void led_classdev_suspend(struct led_classdev *led_cdev);
extern void led_classdev_resume(struct led_classdev *led_cdev);
+extern int led_classdev_init_flash(struct led_classdev *led_cdev);
/**
* led_blink_set - set blinking with software fallback
@@ -138,6 +189,91 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
*/
extern void led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness brightness);
+/**
+/**
+ * led_set_flash_mode - set LED flash mode
+ * @led_cdev: the LED to set
+ * @flash_mode: true - flash mode, false - torch mode
+ *
+ * Returns: 0 on success, -EINVAL on failure
+ *
+ * Switch an LED's flash mode and set brightness to 0.
+ */
+extern int led_set_flash_mode(struct led_classdev *led_cdev,
+ bool flash_mode);
+
+/**
+ * led_set_flash_timeout - set flash LED timeout
+ * @led_cdev: the LED to set
+ * @timeout: the flash timeout to be set
+ *
+ * Set the flash strobe duration.
+ */
+extern void led_set_flash_timeout(struct led_classdev *led_cdev,
+ unsigned long timeout);
+
+/**
+ * led_get_flash_fault - get the flash LED fault
+ * @led_cdev: the LED to query
+ * @fault: bitmask containing flash faults
+ *
+ * Returns: 0 on success, -EINVAL on failure
+ *
+ * Get the flash LED fault.
+ */
+extern int led_get_flash_fault(struct led_classdev *led_cdev,
+ unsigned int *fault);
+
+/**
+ * led_set_hw_triggered - set the flash LED hw_triggered mode
+ * @led_cdev: the LED to set
+ * @enable: the state to set
+ *
+ * Returns: 0 on success, -EINVAL on failure
+ *
+ * Enable/disable triggering the flash LED via hardware pin
+ */
+extern int led_set_hw_triggered(struct led_classdev *led_cdev, bool enable);
+
+/**
+ * led_sysfs_lock - lock LED sysfs interface
+ * @led_cdev: the LED to set
+ *
+ * Lock the LED's sysfs interface
+ */
+extern void led_sysfs_lock(struct led_classdev *led_cdev);
+
+/**
+ * led_sysfs_unlock - unlock LED sysfs interface
+ * @led_cdev: the LED to set
+ *
+ * Unlock the LED's sysfs interface
+ */
+extern void led_sysfs_unlock(struct led_classdev *led_cdev);
+
+/**
+ * led_is_flash_mode
+ * @led_cdev: the LED query
+ *
+ * Returns: true if a led device is in the flash mode, false if it is
+ is in the torch mode
+ */
+static inline bool led_is_flash_mode(struct led_classdev *led_cdev)
+{
+ return !!(led_cdev->flags & LED_MODE_FLASH);
+}
+
+/**
+ * led_sysfs_is_locked
+ * @led_cdev: the LED query about
+ *
+ * Returns: true if the sysfs interface of the led is disabled,
+ * false otherwise
+ */
+static inline bool led_sysfs_is_locked(struct led_classdev *led_cdev)
+{
+ return !!(led_cdev->flags & LED_SYSFS_LOCK);
+}
/*
* LED Triggers
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
2014-03-20 14:51 ` [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs Jacek Anaszewski
@ 2014-03-20 15:28 ` Richard Purdie
2014-03-21 8:27 ` Jacek Anaszewski
2014-03-23 23:18 ` Sakari Ailus
1 sibling, 1 reply; 25+ messages in thread
From: Richard Purdie @ 2014-03-20 15:28 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu
On Thu, 2014-03-20 at 15:51 +0100, Jacek Anaszewski wrote:
> Some LED devices support two operation modes - torch and
> flash. This patch provides support for flash LED devices
> in the LED subsystem by introducing new sysfs attributes
> and kernel internal interface. The attributes being
> introduced are: flash_mode, flash_timeout, max_flash_timeout,
> flash_fault and hw_triggered.
> The modifications aim to be compatible with V4L2 framework
> requirements related to the flash devices management. The
> design assumes that V4L2 driver can take of the LED class
> device control and communicate with it through the kernel
> internal interface. The LED sysfs interface is made
> unavailable then.
>
> 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 | 216 +++++++++++++++++++++++++++++++++++++++++--
> drivers/leds/led-core.c | 124 +++++++++++++++++++++++--
> drivers/leds/led-triggers.c | 17 +++-
> drivers/leds/leds.h | 9 ++
> include/linux/leds.h | 136 +++++++++++++++++++++++++++
> 5 files changed, 486 insertions(+), 16 deletions(-)
It seems rather sad to have to insert that amount of code into the core
LED files for something which only a small number of LEDs actually use.
This will increase the footprint of the core LED code significantly.
Is it not possible to add this as a module/extension to the LED core
rather than completely entangling them?
Cheers,
Richard
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
2014-03-20 15:28 ` Richard Purdie
@ 2014-03-21 8:27 ` Jacek Anaszewski
0 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-21 8:27 UTC (permalink / raw)
To: Richard Purdie
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, Bryan Wu
On 03/20/2014 04:28 PM, Richard Purdie wrote:
> On Thu, 2014-03-20 at 15:51 +0100, Jacek Anaszewski wrote:
>> Some LED devices support two operation modes - torch and
>> flash. This patch provides support for flash LED devices
>> in the LED subsystem by introducing new sysfs attributes
>> and kernel internal interface. The attributes being
>> introduced are: flash_mode, flash_timeout, max_flash_timeout,
>> flash_fault and hw_triggered.
>> The modifications aim to be compatible with V4L2 framework
>> requirements related to the flash devices management. The
>> design assumes that V4L2 driver can take of the LED class
>> device control and communicate with it through the kernel
>> internal interface. The LED sysfs interface is made
>> unavailable then.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Cc: Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Richard Purdie <rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org>
>> ---
>> drivers/leds/led-class.c | 216 +++++++++++++++++++++++++++++++++++++++++--
>> drivers/leds/led-core.c | 124 +++++++++++++++++++++++--
>> drivers/leds/led-triggers.c | 17 +++-
>> drivers/leds/leds.h | 9 ++
>> include/linux/leds.h | 136 +++++++++++++++++++++++++++
>> 5 files changed, 486 insertions(+), 16 deletions(-)
>
> It seems rather sad to have to insert that amount of code into the core
> LED files for something which only a small number of LEDs actually use.
> This will increase the footprint of the core LED code significantly.
>
> Is it not possible to add this as a module/extension to the LED core
> rather than completely entangling them?
OK, I'll try to decouple it.
Thanks,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
@ 2014-03-21 8:27 ` Jacek Anaszewski
0 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-21 8:27 UTC (permalink / raw)
To: Richard Purdie
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu
On 03/20/2014 04:28 PM, Richard Purdie wrote:
> On Thu, 2014-03-20 at 15:51 +0100, Jacek Anaszewski wrote:
>> Some LED devices support two operation modes - torch and
>> flash. This patch provides support for flash LED devices
>> in the LED subsystem by introducing new sysfs attributes
>> and kernel internal interface. The attributes being
>> introduced are: flash_mode, flash_timeout, max_flash_timeout,
>> flash_fault and hw_triggered.
>> The modifications aim to be compatible with V4L2 framework
>> requirements related to the flash devices management. The
>> design assumes that V4L2 driver can take of the LED class
>> device control and communicate with it through the kernel
>> internal interface. The LED sysfs interface is made
>> unavailable then.
>>
>> 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 | 216 +++++++++++++++++++++++++++++++++++++++++--
>> drivers/leds/led-core.c | 124 +++++++++++++++++++++++--
>> drivers/leds/led-triggers.c | 17 +++-
>> drivers/leds/leds.h | 9 ++
>> include/linux/leds.h | 136 +++++++++++++++++++++++++++
>> 5 files changed, 486 insertions(+), 16 deletions(-)
>
> It seems rather sad to have to insert that amount of code into the core
> LED files for something which only a small number of LEDs actually use.
> This will increase the footprint of the core LED code significantly.
>
> Is it not possible to add this as a module/extension to the LED core
> rather than completely entangling them?
OK, I'll try to decouple it.
Thanks,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
2014-03-20 14:51 ` [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs Jacek Anaszewski
2014-03-20 15:28 ` Richard Purdie
@ 2014-03-23 23:18 ` Sakari Ailus
2014-03-28 15:30 ` Jacek Anaszewski
1 sibling, 1 reply; 25+ messages in thread
From: Sakari Ailus @ 2014-03-23 23:18 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie
Hi Jacek,
Thanks for the patchset. It's very nice in general. I have a few comments
below.
On Thu, Mar 20, 2014 at 03:51:03PM +0100, Jacek Anaszewski wrote:
> Some LED devices support two operation modes - torch and
> flash. This patch provides support for flash LED devices
> in the LED subsystem by introducing new sysfs attributes
> and kernel internal interface. The attributes being
> introduced are: flash_mode, flash_timeout, max_flash_timeout,
> flash_fault and hw_triggered.
> The modifications aim to be compatible with V4L2 framework
> requirements related to the flash devices management. The
> design assumes that V4L2 driver can take of the LED class
> device control and communicate with it through the kernel
> internal interface. The LED sysfs interface is made
> unavailable then.
>
> 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 | 216 +++++++++++++++++++++++++++++++++++++++++--
> drivers/leds/led-core.c | 124 +++++++++++++++++++++++--
> drivers/leds/led-triggers.c | 17 +++-
> drivers/leds/leds.h | 9 ++
> include/linux/leds.h | 136 +++++++++++++++++++++++++++
> 5 files changed, 486 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index f37d63c..0510532 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -4,6 +4,9 @@
> * Copyright (C) 2005 John Lenz <lenz@cs.wisc.edu>
> * Copyright (C) 2005-2007 Richard Purdie <rpurdie@openedhand.com>
> *
> + * 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.
> @@ -13,6 +16,7 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/list.h>
> +#include <linux/slab.h>
The list seems to be in quite a disarray. Could you order it as you're
adding a new header to it?
> #include <linux/spinlock.h>
> #include <linux/device.h>
> #include <linux/timer.h>
> @@ -45,28 +49,186 @@ 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_lock);
> +
> + if (led_sysfs_is_locked(led_cdev)) {
> + ret = -EBUSY;
> + goto exit_unlock;
> + }
>
> ret = kstrtoul(buf, 10, &state);
> if (ret)
> - return ret;
> + goto exit_unlock;
>
> if (state == LED_OFF)
> led_trigger_remove(led_cdev);
> - __led_set_brightness(led_cdev, state);
> + led_set_brightness(led_cdev, state);
> + ret = size;
>
> - return size;
> +exit_unlock:
> + mutex_unlock(&led_cdev->led_lock);
> + return ret;
> }
> static DEVICE_ATTR_RW(brightness);
>
> -static ssize_t led_max_brightness_show(struct device *dev,
> +static ssize_t max_brightness_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> struct led_classdev *led_cdev = dev_get_drvdata(dev);
>
> return sprintf(buf, "%u\n", led_cdev->max_brightness);
> }
> -static DEVICE_ATTR(max_brightness, 0444, led_max_brightness_show, NULL);
> +static DEVICE_ATTR_RO(max_brightness);
> +
> +static ssize_t flash_mode_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + unsigned long flash_mode;
> + ssize_t ret;
> +
> + mutex_lock(&led_cdev->led_lock);
> +
> + if (led_sysfs_is_locked(led_cdev)) {
> + ret = -EBUSY;
> + goto exit_unlock;
> + }
> +
> + ret = kstrtoul(buf, 10, &flash_mode);
> + if (ret)
> + goto exit_unlock;
> +
> + if (flash_mode < 0 && flash_mode > 1)
> + return -EINVAL;
> +
> + if (led_is_flash_mode(led_cdev) == flash_mode) {
> + ret = size;
> + goto exit_unlock;
> + }
> +
> + led_trigger_remove(led_cdev);
> +
> + led_set_flash_mode(led_cdev, flash_mode);
> + ret = size;
> +
> +exit_unlock:
> + mutex_unlock(&led_cdev->led_lock);
> + return ret;
> +}
> +
> +static ssize_t flash_mode_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", led_is_flash_mode(led_cdev));
> +}
> +static DEVICE_ATTR_RW(flash_mode);
> +
> +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);
> + unsigned long flash_timeout;
> + ssize_t ret;
> +
> + mutex_lock(&led_cdev->led_lock);
> +
> + if (led_sysfs_is_locked(led_cdev)) {
> + ret = -EBUSY;
> + goto exit_unlock;
> + }
> +
> + ret = kstrtoul(buf, 10, &flash_timeout);
> + if (ret)
> + goto exit_unlock;
> +
> + led_set_flash_timeout(led_cdev, flash_timeout);
> + ret = size;
> +
> +exit_unlock:
> + mutex_unlock(&led_cdev->led_lock);
> + 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_flash *flash = led_cdev->flash;
> +
> + return sprintf(buf, "%lu\n", flash->timeout);
> +}
> +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_flash *flash = led_cdev->flash;
> +
> + return sprintf(buf, "%lu\n", flash->max_timeout);
> +}
> +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);
> + unsigned int fault;
> + int ret;
> +
> + ret = led_get_flash_fault(led_cdev, &fault);
> + if (ret < 0)
> + return -EINVAL;
> +
> + return sprintf(buf, "%x\n", fault);
How about ...0x%8.8x... or such?
> +}
> +static DEVICE_ATTR_RO(flash_fault);
> +
> +static ssize_t hw_triggered_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + unsigned long hw_triggered;
> + ssize_t ret;
> +
> + mutex_lock(&led_cdev->led_lock);
> +
> + if (led_sysfs_is_locked(led_cdev)) {
> + ret = -EBUSY;
> + goto unlock;
> + }
> +
> + ret = kstrtoul(buf, 10, &hw_triggered);
> + if (ret)
> + goto unlock;
> +
> + if (hw_triggered > 1) {
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + ret = led_set_hw_triggered(led_cdev, hw_triggered);
> + if (ret < 0)
> + goto unlock;
> + ret = size;
> +
> +unlock:
> + mutex_unlock(&led_cdev->led_lock);
> + return ret;
> +}
> +
> +static ssize_t hw_triggered_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", led_cdev->hw_triggered);
> +}
> +static DEVICE_ATTR_RW(hw_triggered);
>
> #ifdef CONFIG_LEDS_TRIGGERS
> static DEVICE_ATTR(trigger, 0644, led_trigger_show, led_trigger_store);
> @@ -82,6 +244,11 @@ static const struct attribute_group led_trigger_group = {
> static struct attribute *led_class_attrs[] = {
> &dev_attr_brightness.attr,
> &dev_attr_max_brightness.attr,
> + &dev_attr_flash_mode.attr,
> + &dev_attr_flash_timeout.attr,
> + &dev_attr_max_flash_timeout.attr,
> + &dev_attr_flash_fault.attr,
> + &dev_attr_hw_triggered.attr,
> NULL,
> };
>
> @@ -204,20 +371,54 @@ static const struct dev_pm_ops leds_class_dev_pm_ops = {
> };
>
> /**
> + * led_classdev_init_flash - add support for flash led
> + * @led_cdev: the device to add flash led support to
> + *
> + * Returns: 0 on success, error code on failure.
> + */
> +int led_classdev_init_flash(struct led_classdev *led_cdev)
> +{
> + if (led_cdev->flash)
> + return -EINVAL;
> +
> + led_cdev->flash = kzalloc(sizeof(struct led_flash), GFP_KERNEL);
> + if (!led_cdev->flash)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(led_classdev_init_flash);
> +
> +/**
> * led_classdev_register - register a new object of led_classdev class.
> * @parent: The device to register.
> * @led_cdev: the led_classdev structure for this device.
> */
> int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
> {
> + struct led_flash_ops *ops;
> +
> + if (led_cdev->flash) {
> + ops = &led_cdev->flash->ops;
> + if (!ops || !ops->strobe_set || !ops->mode_set ||
> + !ops->fault_get) {
> + dev_dbg(parent,
> + "Flash LED ops validation failed for the %s\n"
> + "LED device", led_cdev->name);
Splitting strings breaks grep... I think the 80 character limit rule is
a lesser problem in this case.
> + return -EINVAL;
> + }
> + }
> +
> led_cdev->dev = device_create(leds_class, parent, 0, led_cdev,
> "%s", led_cdev->name);
> if (IS_ERR(led_cdev->dev))
> return PTR_ERR(led_cdev->dev);
>
> +
> #ifdef CONFIG_LEDS_TRIGGERS
> init_rwsem(&led_cdev->trigger_lock);
> #endif
> + mutex_init(&led_cdev->led_lock);
> /* add to the list of leds */
> down_write(&leds_list_lock);
> list_add_tail(&led_cdev->node, &leds_list);
> @@ -271,6 +472,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);
> +
> + kfree(led_cdev->flash);
mutex_destroy() here as well?
> }
> EXPORT_SYMBOL_GPL(led_classdev_unregister);
>
> @@ -293,5 +496,6 @@ subsys_initcall(leds_init);
> module_exit(leds_exit);
>
> MODULE_AUTHOR("John Lenz, Richard Purdie");
> +MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("LED Class Interface");
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 71b40d3..093703c 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -5,6 +5,9 @@
> *
> * Author: Richard Purdie <rpurdie@openedhand.com>
> *
> + * 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.
> @@ -71,10 +74,27 @@ static void led_blink_setup(struct led_classdev *led_cdev,
> led_set_software_blink(led_cdev, *delay_on, *delay_off);
> }
>
> +static int led_flash_strobe_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness,
> + unsigned long *timeout)
> +{
> + if (!get_flash_op(led_cdev, strobe_set))
> + return -EINVAL;
> +
> + if (brightness > led_cdev->max_brightness)
> + brightness = led_cdev->max_brightness;
> + call_flash_op(strobe_set, led_cdev, brightness, timeout);
> +
> + return 0;
> +}
> +
> void led_blink_set(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off)
> {
> + if (led_is_flash_mode(led_cdev))
> + return;
> +
> del_timer_sync(&led_cdev->blink_timer);
>
> led_cdev->flags &= ~LED_BLINK_ONESHOT;
> @@ -89,6 +109,9 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
> unsigned long *delay_off,
> int invert)
> {
> + if (led_is_flash_mode(led_cdev))
> + return;
> +
> if ((led_cdev->flags & LED_BLINK_ONESHOT) &&
> timer_pending(&led_cdev->blink_timer))
> return;
> @@ -116,13 +139,100 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
> void led_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness brightness)
> {
> - /* delay brightness setting if need to stop soft-blink timer */
> - if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
> - led_cdev->delayed_set_value = brightness;
> - schedule_work(&led_cdev->set_brightness_work);
> - return;
> + struct led_flash *flash = led_cdev->flash;
> + int ret;
> +
> + if (led_is_flash_mode(led_cdev)) {
> + if (brightness > 0) {
> + ret = led_flash_strobe_set(led_cdev,
> + brightness,
> + &flash->timeout);
Indentation. Could you align the rest of the arguments to the opening
parenthesis?
> + if (ret < 0)
> + dev_err(led_cdev->dev,
> + "Failed to setup flash strobe (%d)",
> + ret);
> + } else {
> + __led_set_brightness(led_cdev, 0);
> + }
> + } else {
> + /* delay brightness setting if need to stop soft-blink timer */
> + if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
> + led_cdev->delayed_set_value = brightness;
> + schedule_work(&led_cdev->set_brightness_work);
> + return;
> + }
> + __led_set_brightness(led_cdev, brightness);
> }
> -
> - __led_set_brightness(led_cdev, brightness);
> }
> EXPORT_SYMBOL(led_set_brightness);
> +
> +int led_set_flash_mode(struct led_classdev *led_cdev,
> + bool flash_mode)
> +{
> + if (!get_flash_op(led_cdev, mode_set))
> + return -EINVAL;
> +
> + call_flash_op(mode_set, led_cdev, flash_mode);
> +
> + if (flash_mode)
> + led_cdev->flags |= LED_MODE_FLASH;
> + else
> + led_cdev->flags &= ~LED_MODE_FLASH;
> +
> + led_set_brightness(led_cdev, 0);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(led_set_flash_mode);
> +
> +/* Caller must ensure led_cdev->led_lock held */
> +void led_sysfs_lock(struct led_classdev *led_cdev)
> +{
> + led_cdev->flags |= LED_SYSFS_LOCK;
> +}
> +EXPORT_SYMBOL(led_sysfs_lock);
> +
> +/* Caller must ensure led_cdev->led_lock held */
> +void led_sysfs_unlock(struct led_classdev *led_cdev)
> +{
> + led_cdev->flags &= ~LED_SYSFS_LOCK;
> +}
> +EXPORT_SYMBOL(led_sysfs_unlock);
> +
> +void led_set_flash_timeout(struct led_classdev *led_cdev, unsigned long timeout)
> +{
> + struct led_flash *flash = led_cdev->flash;
> +
> + /*
> + * Flash timeout is passed to a flash LED driver
> + * through the strobe_set callback - here we only
> + * cache the value.
> + */
> + if (timeout > flash->max_timeout)
You could use the min() macro here.
> + flash->timeout = flash->max_timeout;
> + else
> + flash->timeout = timeout;
> +}
> +EXPORT_SYMBOL(led_set_flash_timeout);
> +
> +int led_get_flash_fault(struct led_classdev *led_cdev, unsigned int *fault)
> +{
> + if (!get_flash_op(led_cdev, fault_get))
> + return -EINVAL;
> +
> + return call_flash_op(fault_get, led_cdev, fault);
> +}
> +EXPORT_SYMBOL(led_get_flash_fault);
> +
> +int led_set_hw_triggered(struct led_classdev *led_cdev, bool enable)
> +{
> + if (led_cdev->has_hw_trig) {
> + __led_set_brightness(led_cdev, 0);
...why?
> + led_cdev->hw_triggered = enable;
> + } else if (enable) {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(led_set_hw_triggered);
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index df1a7c1..448e7c8 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -37,6 +37,15 @@ 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_lock);
> +
> + if (led_sysfs_is_locked(led_cdev) ||
> + led_is_flash_mode(led_cdev)) {
Fits on the previous line.
> + ret = -EBUSY;
> + goto exit_unlock;
> + }
>
> trigger_name[sizeof(trigger_name) - 1] = '\0';
> strncpy(trigger_name, buf, sizeof(trigger_name) - 1);
> @@ -47,7 +56,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 exit_unlock;
> }
>
> down_read(&triggers_list_lock);
> @@ -58,12 +67,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 exit_unlock;
> }
> }
> up_read(&triggers_list_lock);
>
> - return -EINVAL;
> +exit_unlock:
> + mutex_unlock(&led_cdev->led_lock);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(led_trigger_store);
>
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index 4c50365..44cc384 100644
> --- a/drivers/leds/leds.h
> +++ b/drivers/leds/leds.h
> @@ -17,6 +17,15 @@
> #include <linux/rwsem.h>
> #include <linux/leds.h>
>
> +
I'd probably spare a newline here.
> +#define call_flash_op(op, args...) \
> + ((led_cdev)->flash->ops.op(args))
> +
> +#define get_flash_op(led_cdev, op) \
> + ((led_cdev)->flash ? \
> + (led_cdev)->flash->ops.op : \
> + 0)
> +
> static inline void __led_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 0287ab2..1bf0ab3 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -17,6 +17,14 @@
> #include <linux/rwsem.h>
> #include <linux/timer.h>
> #include <linux/workqueue.h>
> +#include <linux/mutex.h>
mutex.h should be earlier in the list of included files.
> +#include <media/v4l2-device.h>
> +
> +#define LED_FAULT_OVER_VOLTAGE (1 << 0)
> +#define LED_FAULT_TIMEOUT (1 << 1)
> +#define LED_FAULT_OVER_TEMPERATURE (1 << 2)
> +#define LED_FAULT_SHORT_CIRCUIT (1 << 3)
> +#define LED_FAULT_OVER_CURRENT (1 << 4)
This patch went in to the media-tree some time ago. I wonder if the relevant
bits should be added here now as well.
commit 935aa6b2e8a911e81baecec0537dd7e478dc8c91
Author: Daniel Jeong <gshark.jeong@gmail.com>
Date: Mon Mar 3 06:52:08 2014 -0300
[media] v4l2-controls.h: Add addtional Flash fault bits
Three Flash fault are added. V4L2_FLASH_FAULT_UNDER_VOLTAGE for the case low
voltage below the min. limit. V4L2_FLASH_FAULT_INPUT_VOLTAGE for the case
falling input voltage and chip adjust flash current not occur under voltage
event. V4L2_FLASH_FAULT_LED_OVER_TEMPERATURE for the case the temperature
exceed the maximun limit
Signed-off-by: Daniel Jeong <gshark.jeong@gmail.com>
Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> struct device;
> /*
> @@ -29,6 +37,33 @@ enum led_brightness {
> LED_FULL = 255,
> };
>
> +struct led_classdev;
> +
> +struct led_flash_ops {
> + /* Set torch/flash LED mode */
> + void (*mode_set)(struct led_classdev *led_cdev,
> + bool flash_mode);
> + /* Setup flash strobe */
> + void (*strobe_set)(struct led_classdev *led_cdev,
> + enum led_brightness brightness,
> + unsigned long *timeout);
Can't the above operations fail?
Does this also assume that strobing the flash will always configure the
brightness and timeout as well? It'd be rather nice if it didn't: strobing
the flash is time critical, and as the I2C bus is quite slow, additional
device access should be avoided especially when timing is critical.
> + /* Get the flash LED fault */
> + int (*fault_get)(struct led_classdev *led_cdev,
> + unsigned int *fault);
> +};
> +
> +struct led_flash {
> + /* flash led specific ops */
> + struct led_flash_ops ops;
const?
> + /*
> + * maximum allowed flash timeout - it is read only and
> + * should be initialized by the driver
s/should/must/ ?
> + */
> + unsigned long max_timeout;
> + /* current flash timeout */
> + unsigned long timeout;
> +};
> +
> struct led_classdev {
> const char *name;
> int brightness;
> @@ -42,6 +77,8 @@ struct led_classdev {
> #define LED_BLINK_ONESHOT (1 << 17)
> #define LED_BLINK_ONESHOT_STOP (1 << 18)
> #define LED_BLINK_INVERT (1 << 19)
> +#define LED_MODE_FLASH (1 << 20)
> +#define LED_SYSFS_LOCK (1 << 21)
>
> /* Set LED brightness level */
> /* Must not sleep, use a workqueue if needed */
> @@ -69,6 +106,19 @@ struct led_classdev {
> unsigned long blink_delay_on, blink_delay_off;
> struct timer_list blink_timer;
> int blink_brightness;
> + struct led_flash *flash;
> + /*
> + * Determines whether a device supports triggering a flash led
> + * with use of a dedicated hardware pin
> + */
> + bool has_hw_trig;
> + /* If true then hardware pin triggers flash strobe */
> + bool hw_triggered;
> + /*
> + * Ensures consistent LED sysfs access and protects
> + * LED sysfs locking mechanism
> + */
> + struct mutex led_lock;
>
> struct work_struct set_brightness_work;
> int delayed_set_value;
> @@ -90,6 +140,7 @@ extern int led_classdev_register(struct device *parent,
> extern void led_classdev_unregister(struct led_classdev *led_cdev);
> extern void led_classdev_suspend(struct led_classdev *led_cdev);
> extern void led_classdev_resume(struct led_classdev *led_cdev);
> +extern int led_classdev_init_flash(struct led_classdev *led_cdev);
>
> /**
> * led_blink_set - set blinking with software fallback
> @@ -138,6 +189,91 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
> */
> extern void led_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness brightness);
> +/**
I think the above line should be moved to the surplus comment stash or
something? :-)
> +/**
> + * led_set_flash_mode - set LED flash mode
> + * @led_cdev: the LED to set
> + * @flash_mode: true - flash mode, false - torch mode
> + *
> + * Returns: 0 on success, -EINVAL on failure
> + *
> + * Switch an LED's flash mode and set brightness to 0.
> + */
> +extern int led_set_flash_mode(struct led_classdev *led_cdev,
> + bool flash_mode);
> +
> +/**
> + * led_set_flash_timeout - set flash LED timeout
> + * @led_cdev: the LED to set
> + * @timeout: the flash timeout to be set
> + *
> + * Set the flash strobe duration.
> + */
> +extern void led_set_flash_timeout(struct led_classdev *led_cdev,
> + unsigned long timeout);
> +
> +/**
> + * led_get_flash_fault - get the flash LED fault
> + * @led_cdev: the LED to query
> + * @fault: bitmask containing flash faults
> + *
> + * Returns: 0 on success, -EINVAL on failure
> + *
> + * Get the flash LED fault.
> + */
> +extern int led_get_flash_fault(struct led_classdev *led_cdev,
> + unsigned int *fault);
> +
> +/**
> + * led_set_hw_triggered - set the flash LED hw_triggered mode
> + * @led_cdev: the LED to set
> + * @enable: the state to set
> + *
> + * Returns: 0 on success, -EINVAL on failure
> + *
> + * Enable/disable triggering the flash LED via hardware pin
> + */
> +extern int led_set_hw_triggered(struct led_classdev *led_cdev, bool enable);
> +
> +/**
> + * led_sysfs_lock - lock LED sysfs interface
> + * @led_cdev: the LED to set
> + *
> + * Lock the LED's sysfs interface
> + */
> +extern void led_sysfs_lock(struct led_classdev *led_cdev);
> +
> +/**
> + * led_sysfs_unlock - unlock LED sysfs interface
> + * @led_cdev: the LED to set
> + *
> + * Unlock the LED's sysfs interface
> + */
> +extern void led_sysfs_unlock(struct led_classdev *led_cdev);
> +
> +/**
> + * led_is_flash_mode
> + * @led_cdev: the LED query
> + *
> + * Returns: true if a led device is in the flash mode, false if it is
> + is in the torch mode
> + */
> +static inline bool led_is_flash_mode(struct led_classdev *led_cdev)
> +{
> + return !!(led_cdev->flags & LED_MODE_FLASH);
I don't think you necessarily need !!() --- converting any non-zero integer
to bool is true. Same below.
> +}
> +
> +/**
> + * led_sysfs_is_locked
> + * @led_cdev: the LED query about
> + *
> + * Returns: true if the sysfs interface of the led is disabled,
> + * false otherwise
> + */
> +static inline bool led_sysfs_is_locked(struct led_classdev *led_cdev)
> +{
> + return !!(led_cdev->flags & LED_SYSFS_LOCK);
> +}
>
> /*
> * LED Triggers
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
2014-03-23 23:18 ` Sakari Ailus
@ 2014-03-28 15:30 ` Jacek Anaszewski
2014-03-31 9:26 ` Sakari Ailus
0 siblings, 1 reply; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-28 15:30 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie
Hi Sakari,
Thanks for the review.
On 03/24/2014 12:18 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> Thanks for the patchset. It's very nice in general. I have a few comments
> below.
[...]
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index 0287ab2..1bf0ab3 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -17,6 +17,14 @@
>> #include <linux/rwsem.h>
>> #include <linux/timer.h>
>> #include <linux/workqueue.h>
>> +#include <linux/mutex.h>
>
> mutex.h should be earlier in the list of included files.
>
>> +#include <media/v4l2-device.h>
>> +
>> +#define LED_FAULT_OVER_VOLTAGE (1 << 0)
>> +#define LED_FAULT_TIMEOUT (1 << 1)
>> +#define LED_FAULT_OVER_TEMPERATURE (1 << 2)
>> +#define LED_FAULT_SHORT_CIRCUIT (1 << 3)
>> +#define LED_FAULT_OVER_CURRENT (1 << 4)
>
> This patch went in to the media-tree some time ago. I wonder if the relevant
> bits should be added here now as well.
>
> commit 935aa6b2e8a911e81baecec0537dd7e478dc8c91
> Author: Daniel Jeong <gshark.jeong@gmail.com>
> Date: Mon Mar 3 06:52:08 2014 -0300
>
> [media] v4l2-controls.h: Add addtional Flash fault bits
>
> Three Flash fault are added. V4L2_FLASH_FAULT_UNDER_VOLTAGE for the case low
> voltage below the min. limit. V4L2_FLASH_FAULT_INPUT_VOLTAGE for the case
> falling input voltage and chip adjust flash current not occur under voltage
> event. V4L2_FLASH_FAULT_LED_OVER_TEMPERATURE for the case the temperature
> exceed the maximun limit
>
> Signed-off-by: Daniel Jeong <gshark.jeong@gmail.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
As it will not cause a build break and any runtime problems, even if
the patch is not merged, I added these bits to my implementation.
BTW I have doubts about V4L2_FLASH_FAULT_INDICATOR and
V4L2_CID_FLASH_INDICATOR_INTENSITY control. I did not take them
into account in my implementation because it is not clear for
me how an indicator led is related to a torch led. There is
a control for setting indicator intensity but there is not
one for enabling it. Could you shed some light on this issue?
Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs
2014-03-28 15:30 ` Jacek Anaszewski
@ 2014-03-31 9:26 ` Sakari Ailus
0 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2014-03-31 9:26 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie
Hi Jacek,
On Fri, Mar 28, 2014 at 04:30:19PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
>
> Thanks for the review.
>
> On 03/24/2014 12:18 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >Thanks for the patchset. It's very nice in general. I have a few comments
> >below.
>
> [...]
>
> >>diff --git a/include/linux/leds.h b/include/linux/leds.h
> >>index 0287ab2..1bf0ab3 100644
> >>--- a/include/linux/leds.h
> >>+++ b/include/linux/leds.h
> >>@@ -17,6 +17,14 @@
> >> #include <linux/rwsem.h>
> >> #include <linux/timer.h>
> >> #include <linux/workqueue.h>
> >>+#include <linux/mutex.h>
> >
> >mutex.h should be earlier in the list of included files.
> >
> >>+#include <media/v4l2-device.h>
> >>+
> >>+#define LED_FAULT_OVER_VOLTAGE (1 << 0)
> >>+#define LED_FAULT_TIMEOUT (1 << 1)
> >>+#define LED_FAULT_OVER_TEMPERATURE (1 << 2)
> >>+#define LED_FAULT_SHORT_CIRCUIT (1 << 3)
> >>+#define LED_FAULT_OVER_CURRENT (1 << 4)
> >
> >This patch went in to the media-tree some time ago. I wonder if the relevant
> >bits should be added here now as well.
> >
> >commit 935aa6b2e8a911e81baecec0537dd7e478dc8c91
> >Author: Daniel Jeong <gshark.jeong@gmail.com>
> >Date: Mon Mar 3 06:52:08 2014 -0300
> >
> > [media] v4l2-controls.h: Add addtional Flash fault bits
> >
> > Three Flash fault are added. V4L2_FLASH_FAULT_UNDER_VOLTAGE for the case low
> > voltage below the min. limit. V4L2_FLASH_FAULT_INPUT_VOLTAGE for the case
> > falling input voltage and chip adjust flash current not occur under voltage
> > event. V4L2_FLASH_FAULT_LED_OVER_TEMPERATURE for the case the temperature
> > exceed the maximun limit
> >
> > Signed-off-by: Daniel Jeong <gshark.jeong@gmail.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
>
> As it will not cause a build break and any runtime problems, even if
> the patch is not merged, I added these bits to my implementation.
>
> BTW I have doubts about V4L2_FLASH_FAULT_INDICATOR and
> V4L2_CID_FLASH_INDICATOR_INTENSITY control. I did not take them
> into account in my implementation because it is not clear for
> me how an indicator led is related to a torch led. There is
> a control for setting indicator intensity but there is not
> one for enabling it. Could you shed some light on this issue?
The indicator isn't related to the torch mode in any way. Some flash
controllers contain an additional output for indicator, also called privacy
led. These faults are related to that.
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH/RFC 2/8] leds: Improve and export led_update_brightness function
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
2014-03-20 14:51 ` [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-23 23:20 ` Sakari Ailus
2014-03-20 14:51 ` [PATCH/RFC 3/8] Documentation: leds: Add description of flash mode Jacek Anaszewski
` (5 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski, Bryan Wu,
Richard Purdie
led_update_brightness helper function used to be exploited
only locally in the led-class.c module, where its result was
being passed to the brightness_show sysfs callback. With the
introduction of v4l2-flash subdevice the same functionality
became required for checking the current flash strobe status.
This patch adds checking brightness_get callback error code
and adds the function to the LED subsystem public API.
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 | 6 ------
drivers/leds/led-core.c | 17 +++++++++++++++++
include/linux/leds.h | 10 ++++++++++
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 0510532..04de352 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -27,12 +27,6 @@
static struct class *leds_class;
-static void led_update_brightness(struct led_classdev *led_cdev)
-{
- if (led_cdev->brightness_get)
- led_cdev->brightness = led_cdev->brightness_get(led_cdev);
-}
-
static ssize_t brightness_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 093703c..21ceda1 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -166,6 +166,23 @@ void led_set_brightness(struct led_classdev *led_cdev,
}
EXPORT_SYMBOL(led_set_brightness);
+int led_update_brightness(struct led_classdev *led_cdev)
+{
+ int ret;
+
+ if (led_cdev->brightness_get == NULL)
+ return -EINVAL;
+
+ ret = led_cdev->brightness_get(led_cdev);
+ if (ret >= 0)
+ led_cdev->brightness = ret;
+ else
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(led_update_brightness);
+
int led_set_flash_mode(struct led_classdev *led_cdev,
bool flash_mode)
{
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 1bf0ab3..5bf05cc 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -190,6 +190,16 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
extern void led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness brightness);
/**
+ * led_update_brightness - update LED brightness
+ * @led_cdev: the LED to query about
+ *
+ * Get an LED's current brightness and update led_cdev->brightness
+ * member with the obtained value.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_update_brightness(struct led_classdev *led_cdev);
+
/**
* led_set_flash_mode - set LED flash mode
* @led_cdev: the LED to set
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 2/8] leds: Improve and export led_update_brightness function
2014-03-20 14:51 ` [PATCH/RFC 2/8] leds: Improve and export led_update_brightness function Jacek Anaszewski
@ 2014-03-23 23:20 ` Sakari Ailus
0 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2014-03-23 23:20 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie
Hi Jacek,
On Thu, Mar 20, 2014 at 03:51:04PM +0100, Jacek Anaszewski wrote:
> led_update_brightness helper function used to be exploited
> only locally in the led-class.c module, where its result was
> being passed to the brightness_show sysfs callback. With the
> introduction of v4l2-flash subdevice the same functionality
> became required for checking the current flash strobe status.
> This patch adds checking brightness_get callback error code
> and adds the function to the LED subsystem public API.
>
> 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 | 6 ------
> drivers/leds/led-core.c | 17 +++++++++++++++++
> include/linux/leds.h | 10 ++++++++++
> 3 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 0510532..04de352 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -27,12 +27,6 @@
>
> static struct class *leds_class;
>
> -static void led_update_brightness(struct led_classdev *led_cdev)
> -{
> - if (led_cdev->brightness_get)
> - led_cdev->brightness = led_cdev->brightness_get(led_cdev);
> -}
> -
> static ssize_t brightness_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 093703c..21ceda1 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -166,6 +166,23 @@ void led_set_brightness(struct led_classdev *led_cdev,
> }
> EXPORT_SYMBOL(led_set_brightness);
>
> +int led_update_brightness(struct led_classdev *led_cdev)
> +{
> + int ret;
> +
> + if (led_cdev->brightness_get == NULL)
> + return -EINVAL;
> +
> + ret = led_cdev->brightness_get(led_cdev);
> + if (ret >= 0)
> + led_cdev->brightness = ret;
> + else
> + return ret;
You don't really need the else branch here. It'd be cleaner without, I
think.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(led_update_brightness);
> +
> int led_set_flash_mode(struct led_classdev *led_cdev,
> bool flash_mode)
> {
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 1bf0ab3..5bf05cc 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -190,6 +190,16 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
> extern void led_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness brightness);
> /**
> + * led_update_brightness - update LED brightness
> + * @led_cdev: the LED to query about
> + *
> + * Get an LED's current brightness and update led_cdev->brightness
> + * member with the obtained value.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_update_brightness(struct led_classdev *led_cdev);
> +
> /**
> * led_set_flash_mode - set LED flash mode
> * @led_cdev: the LED to set
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH/RFC 3/8] Documentation: leds: Add description of flash mode
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
2014-03-20 14:51 ` [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs Jacek Anaszewski
2014-03-20 14:51 ` [PATCH/RFC 2/8] leds: Improve and export led_update_brightness function Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-20 14:51 ` [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices Jacek Anaszewski
` (4 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski, Bryan Wu,
Richard Purdie
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
---
Documentation/leds/leds-class.txt | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
index 62261c0..d34d990 100644
--- a/Documentation/leds/leds-class.txt
+++ b/Documentation/leds/leds-class.txt
@@ -8,6 +8,31 @@ LED is defined in max_brightness file. The brightness file will set the brightne
of the LED (taking a value 0-max_brightness). Most LEDs don't have hardware
brightness support so will just be turned on for non-zero brightness settings.
+Some LED devices support two modes - torch and flash. A LED subsystem device
+driver can declare this by calling led_classdev_init_flash function and
+initializing flash field of the led_classdev structure (see <linux/leds.h>).
+There are five sysfs attributes dedicated specifically to the flash LED devices:
+
+ - flash_mode - sets/unsets the flash mode
+ - flash_timeout - determines duration of the flash blink in milliseconds
+ - max_flash_timeout - maximum flash blink duration that can be set (RO)
+ - flash_fault - bitmask of flash faults that may have occured, possible
+ flags are:
+ * 0x01 - Flash controller voltage to the flash LED has exceeded
+ the limit specific to the flash controller.
+ * 0x02 - The flash strobe was still on when the timeout set by
+ the user has expired. Not all flash controllers may set
+ this in all such conditions.
+ * 0x04 - The flash controller has overheated.
+ * 0x08 - The short circuit protection of the flash controller
+ has been triggered.
+ * 0x10 - Current in the LED power supply has exceeded the limit
+ specific to the flash controller.
+ - hw_triggered - Some devices expose dedicated hardware pins for
+ triggering a flash LED. The attribute allows to set
+ this mode. After writting 1 the brightness has to be set
+ to the desired value to arm a led.
+
The class also introduces the optional concept of an LED trigger. A trigger
is a kernel based source of led events. Triggers can either be simple or
complex. A simple trigger isn't configurable and is designed to slot into
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
` (2 preceding siblings ...)
2014-03-20 14:51 ` [PATCH/RFC 3/8] Documentation: leds: Add description of flash mode Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-24 0:08 ` Sakari Ailus
2014-03-20 14:51 ` [PATCH/RFC 5/8] media: exynos4-is: Add support for v4l2-flash subdevs Jacek Anaszewski
` (3 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski
This patch adds helper functions for registering/unregistering
LED class flash devices as V4L2 subdevs. The functions should
be called from the LED subsystem device driver. In case the
Multimedia Framework support is disabled in the kernel config
the functions' empty versions will be used.
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/media/v4l2-core/Makefile | 2 +-
drivers/media/v4l2-core/v4l2-flash.c | 320 ++++++++++++++++++++++++++++++++++
include/media/v4l2-flash.h | 102 +++++++++++
3 files changed, 423 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
create mode 100644 include/media/v4l2-flash.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c6ae7ba..63e8f03 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -6,7 +6,7 @@ tuner-objs := tuner-core.o
videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
- v4l2-async.o
+ v4l2-async.o v4l2-flash.o
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
diff --git a/drivers/media/v4l2-core/v4l2-flash.c b/drivers/media/v4l2-core/v4l2-flash.c
new file mode 100644
index 0000000..6be0ba9
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,320 @@
+/*
+ * V4L2 flash LED subdevice registration helpers.
+ *
+ * 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 <media/v4l2-ioctl.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-flash.h>
+#include <media/v4l2-dev.h>
+
+static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
+
+{
+ struct v4l2_flash *flash = ctrl_to_flash(c);
+ struct led_classdev *led_cdev = flash->led_cdev;
+ unsigned int fault;
+ int ret;
+
+ switch (c->id) {
+ case V4L2_CID_FLASH_STROBE_STATUS:
+ ret = led_update_brightness(led_cdev);
+ if (ret < 0)
+ return ret;
+ c->val = !!ret;
+ return 0;
+ case V4L2_CID_FLASH_FAULT:
+ /* led faults map directly to V4L2 flash faults */
+ ret = led_get_flash_fault(led_cdev, &fault);
+ if (!ret)
+ c->val = fault;
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int v4l2_flash_set_intensity(struct v4l2_flash *flash,
+ unsigned int intensity)
+{
+ struct led_classdev *led_cdev = flash->led_cdev;
+ unsigned int fault;
+ int ret;
+
+ ret = led_get_flash_fault(led_cdev, &fault);
+ if (ret < 0 || fault)
+ return -EINVAL;
+
+ led_set_brightness(led_cdev, intensity);
+
+ return ret;
+}
+
+static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
+{
+ struct v4l2_flash *flash = ctrl_to_flash(c);
+ struct led_classdev *led_cdev = flash->led_cdev;
+ int ret = 0;
+
+ switch (c->id) {
+ case V4L2_CID_FLASH_LED_MODE:
+ switch (c->val) {
+ case V4L2_FLASH_LED_MODE_NONE:
+ /* clear flash mode on releae */
+ ret = led_set_flash_mode(led_cdev, false);
+ if (ret < 0)
+ return ret;
+ mutex_lock(&led_cdev->led_lock);
+ led_sysfs_unlock(led_cdev);
+ mutex_unlock(&led_cdev->led_lock);
+ break;
+ case V4L2_FLASH_LED_MODE_FLASH:
+ mutex_lock(&led_cdev->led_lock);
+ led_sysfs_lock(led_cdev);
+ mutex_unlock(&led_cdev->led_lock);
+
+ ret = led_set_flash_mode(led_cdev, true);
+ if (ret < 0)
+ return ret;
+ if (flash->ctrl.source->val ==
+ V4L2_FLASH_STROBE_SOURCE_EXTERNAL) {
+ ret = led_set_hw_triggered(led_cdev, true);
+ if (ret < 0)
+ return ret;
+ ret = v4l2_flash_set_intensity(flash,
+ flash->flash_intensity);
+ } else {
+ ret = led_set_hw_triggered(led_cdev, false);
+ if (ret < 0)
+ return ret;
+ }
+ break;
+ case V4L2_FLASH_LED_MODE_TORCH:
+ mutex_lock(&led_cdev->led_lock);
+ led_sysfs_lock(led_cdev);
+ mutex_unlock(&led_cdev->led_lock);
+
+ ret = led_set_flash_mode(led_cdev, false);
+ if (ret < 0)
+ return ret;
+ /* torch is always triggered by software */
+ ret = led_set_hw_triggered(led_cdev, false);
+ if (ret)
+ return -EINVAL;
+ ret = v4l2_flash_set_intensity(flash,
+ flash->torch_intensity);
+ break;
+ }
+ break;
+ case V4L2_CID_FLASH_STROBE_SOURCE:
+ ret = led_set_hw_triggered(led_cdev,
+ c->val == V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
+ break;
+ case V4L2_CID_FLASH_STROBE:
+ if (flash->ctrl.led_mode->val != V4L2_FLASH_LED_MODE_FLASH ||
+ flash->ctrl.source->val != V4L2_FLASH_STROBE_SOURCE_SOFTWARE)
+ return -EINVAL;
+ led_set_flash_timeout(led_cdev, flash->flash_timeout);
+ ret = v4l2_flash_set_intensity(flash,
+ flash->flash_intensity);
+ break;
+ case V4L2_CID_FLASH_STROBE_STOP:
+ led_set_brightness(led_cdev, 0);
+ break;
+ case V4L2_CID_FLASH_TIMEOUT:
+ flash->flash_timeout = c->val;
+ break;
+ case V4L2_CID_FLASH_INTENSITY:
+ flash->flash_intensity = c->val;
+ break;
+ case V4L2_CID_FLASH_TORCH_INTENSITY:
+ flash->torch_intensity = c->val;
+ if (flash->ctrl.led_mode->val == V4L2_FLASH_LED_MODE_TORCH)
+ ret = v4l2_flash_set_intensity(flash,
+ flash->torch_intensity);
+ break;
+ }
+
+ return ret;
+}
+
+static const struct v4l2_ctrl_ops v4l2_flash_ctrl_ops = {
+ .g_volatile_ctrl = v4l2_flash_g_volatile_ctrl,
+ .s_ctrl = v4l2_flash_s_ctrl,
+};
+
+static int v4l2_flash_init_controls(struct v4l2_flash *flash,
+ struct v4l2_flash_ctrl_config *config)
+
+{
+ unsigned int mask;
+ struct v4l2_ctrl *ctrl;
+ struct v4l2_ctrl_config *ctrl_cfg;
+ bool has_flash = config->flags & V4L2_FLASH_CFG_LED_FLASH;
+ bool has_torch = config->flags & V4L2_FLASH_CFG_LED_TORCH;
+ int ret, num_ctrls;
+
+ if (!has_flash && !has_torch)
+ return -EINVAL;
+
+ num_ctrls = has_flash ? 8 : 2;
+ if (config->flags & V4L2_FLASH_CFG_FAULTS_MASK)
+ ++num_ctrls;
+
+ v4l2_ctrl_handler_init(&flash->hdl, num_ctrls);
+
+ mask = 1 << V4L2_FLASH_LED_MODE_NONE;
+ if (has_flash)
+ mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
+ if (has_torch)
+ mask |= 1 << V4L2_FLASH_LED_MODE_TORCH;
+
+ /* Configure TORCH_INTENSITY ctrl */
+ ctrl_cfg = &config->torch_intensity;
+ ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_TORCH_INTENSITY,
+ ctrl_cfg->min, ctrl_cfg->max,
+ ctrl_cfg->step, ctrl_cfg->def);
+
+ if (has_flash) {
+ /* Configure FLASH_LED_MODE ctrl */
+ flash->ctrl.led_mode = v4l2_ctrl_new_std_menu(&flash->hdl,
+ &v4l2_flash_ctrl_ops, V4L2_CID_FLASH_LED_MODE,
+ V4L2_FLASH_LED_MODE_TORCH, ~mask,
+ V4L2_FLASH_LED_MODE_NONE);
+
+ /* Configure FLASH_STROBE_SOURCE ctrl */
+ mask = 1 << V4L2_FLASH_STROBE_SOURCE_SOFTWARE |
+ 1 << V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
+
+ flash->ctrl.source = v4l2_ctrl_new_std_menu(&flash->hdl,
+ &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_STROBE_SOURCE,
+ V4L2_FLASH_STROBE_SOURCE_EXTERNAL,
+ ~mask,
+ V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
+
+ /* Configure FLASH_STROBE ctrl */
+ ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_STROBE, 0, 1, 1, 0);
+ if (ctrl)
+ ctrl->type = V4L2_CTRL_TYPE_BUTTON;
+
+ /* Configure FLASH_STROBE_STOP ctrl */
+ ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_STROBE_STOP,
+ 0, 1, 1, 0);
+ if (ctrl)
+ ctrl->type = V4L2_CTRL_TYPE_BUTTON;
+
+ /* Configure FLASH_STROBE_STATUS ctrl */
+ ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_STROBE_STATUS,
+ 0, 1, 1, 1);
+ if (ctrl)
+ ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
+ V4L2_CTRL_FLAG_READ_ONLY;
+
+ /* Configure FLASH_TIMEOUT ctrl */
+ ctrl_cfg = &config->flash_timeout;
+ ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_TIMEOUT, ctrl_cfg->min,
+ ctrl_cfg->max, ctrl_cfg->step,
+ ctrl_cfg->def);
+
+ /* Configure FLASH_INTENSITY ctrl */
+ ctrl_cfg = &config->flash_intensity;
+ ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_INTENSITY,
+ ctrl_cfg->min, ctrl_cfg->max,
+ ctrl_cfg->step, ctrl_cfg->def);
+
+ if (config->flags & V4L2_FLASH_CFG_FAULTS_MASK) {
+ /* Configure FLASH_FAULT ctrl */
+ ctrl = v4l2_ctrl_new_std(&flash->hdl,
+ &v4l2_flash_ctrl_ops,
+ V4L2_CID_FLASH_FAULT, 0,
+ config->flags &
+ V4L2_FLASH_CFG_FAULTS_MASK,
+ 0, 0);
+ if (ctrl) {
+ ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
+ V4L2_CTRL_FLAG_READ_ONLY;
+ ctrl->type = V4L2_CTRL_TYPE_BITMASK;
+ }
+ }
+ }
+
+ if (flash->hdl.error) {
+ ret = flash->hdl.error;
+ goto error_free;
+ }
+
+ ret = v4l2_ctrl_handler_setup(&flash->hdl);
+ if (ret < 0)
+ goto error_free;
+
+ flash->subdev.ctrl_handler = &flash->hdl;
+
+ return 0;
+
+error_free:
+ v4l2_ctrl_handler_free(&flash->hdl);
+ return ret;
+}
+
+/* v4l2_subdev_init requires this structure */
+static struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
+};
+
+int v4l2_flash_init(struct led_classdev *led_cdev, struct v4l2_flash *flash,
+ struct v4l2_flash_ctrl_config *config)
+{
+ struct v4l2_subdev *sd = &flash->subdev;
+ int ret;
+
+ flash->led_cdev = led_cdev;
+ sd->dev = led_cdev->dev->parent;
+ v4l2_subdev_init(sd, &v4l2_flash_subdev_ops);
+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ snprintf(sd->name, sizeof(sd->name), led_cdev->name);
+
+ ret = v4l2_flash_init_controls(flash, config);
+ if (ret < 0)
+ goto err_init_controls;
+
+ ret = media_entity_init(&sd->entity, 0, NULL, 0);
+ if (ret < 0)
+ goto err_init_entity;
+
+ sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
+
+ ret = v4l2_async_register_subdev(sd);
+ if (ret < 0)
+ goto err_init_entity;
+
+ return 0;
+
+err_init_entity:
+ media_entity_cleanup(&sd->entity);
+err_init_controls:
+ v4l2_ctrl_handler_free(sd->ctrl_handler);
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(v4l2_flash_init);
+
+void v4l2_flash_release(struct v4l2_flash *flash)
+{
+ media_entity_cleanup(&flash->subdev.entity);
+ v4l2_ctrl_handler_free(flash->subdev.ctrl_handler);
+ v4l2_async_unregister_subdev(&flash->subdev);
+}
+EXPORT_SYMBOL_GPL(v4l2_flash_release);
diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
new file mode 100644
index 0000000..138edae
--- /dev/null
+++ b/include/media/v4l2-flash.h
@@ -0,0 +1,102 @@
+/*
+ * V4L2 flash LED subdevice registration helpers.
+ *
+ * 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 _V4L2_FLASH_H
+#define _V4L2_FLASH_H
+
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-dev.h>
+#include <linux/leds.h>
+
+/*
+ * Supported led fault and mode bits -
+ * must be kept in synch with V4L2_FLASH_FAULT bits
+ */
+#define V4L2_FLASH_CFG_FAULT_OVER_VOLTAGE (1 << 0)
+#define V4L2_FLASH_CFG_FAULT_TIMEOUT (1 << 1)
+#define V4L2_FLASH_CFG_FAULT_OVER_TEMPERATURE (1 << 2)
+#define V4L2_FLASH_CFG_FAULT_SHORT_CIRCUIT (1 << 3)
+#define V4L2_FLASH_CFG_FAULT_OVER_CURRENT (1 << 4)
+#define V4L2_FLASH_CFG_FAULT_INDICATOR (1 << 5)
+#define V4L2_FLASH_CFG_FAULTS_MASK 0x3f
+#define V4L2_FLASH_CFG_LED_FLASH (1 << 6)
+#define V4L2_FLASH_CFG_LED_TORCH (1 << 7)
+
+/* Flash control config data initializer */
+
+struct v4l2_flash {
+ struct led_classdev *led_cdev;
+ struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
+
+ struct {
+ struct v4l2_ctrl *source;
+ struct v4l2_ctrl *led_mode;
+ } ctrl;
+
+ unsigned int flash_intensity;
+ unsigned int torch_intensity;
+ unsigned int flash_timeout;
+};
+
+struct v4l2_flash_ctrl_config {
+ struct v4l2_ctrl_config flash_timeout;
+ struct v4l2_ctrl_config flash_intensity;
+ struct v4l2_ctrl_config torch_intensity;
+ unsigned int flags;
+};
+
+static inline struct v4l2_flash *subdev_to_flash(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct v4l2_flash, subdev);
+}
+
+static inline struct v4l2_flash *ctrl_to_flash(struct v4l2_ctrl *c)
+{
+ return container_of(c->handler, struct v4l2_flash, hdl);
+}
+
+#ifdef CONFIG_VIDEO_V4L2
+/**
+ * v4l2_flash_init - initialize V4L2 flash led sub-device
+ * @led_cdev: the LED to create subdev upon
+ * @flash: a structure representing V4L2 flash led device
+ * @config: initial data for the flash led subdev controls
+ *
+ * Create V4L2 subdev wrapping given LED subsystem device.
+ */
+int v4l2_flash_init(struct led_classdev *led_cdev, struct v4l2_flash *flash,
+ struct v4l2_flash_ctrl_config *config);
+
+/**
+ * v4l2_flash_release - release V4L2 flash led sub-device
+ * @flash: a structure representing V4L2 flash led device
+ *
+ * Release V4L2 flash led subdev.
+ */
+void v4l2_flash_release(struct v4l2_flash *flash);
+#else
+static inline int v4l2_flash_init(struct led_classdev *led_cdev,
+ struct v4l2_flash *flash,
+ struct v4l2_flash_ctrl_config *config)
+{
+ return 0;
+}
+
+static inline void v4l2_flash_release(struct v4l2_flash *flash)
+{
+}
+#endif
+
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices
2014-03-20 14:51 ` [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices Jacek Anaszewski
@ 2014-03-24 0:08 ` Sakari Ailus
2014-03-28 15:30 ` Jacek Anaszewski
0 siblings, 1 reply; 25+ messages in thread
From: Sakari Ailus @ 2014-03-24 0:08 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park
Hi Jacek,
On Thu, Mar 20, 2014 at 03:51:06PM +0100, Jacek Anaszewski wrote:
> This patch adds helper functions for registering/unregistering
> LED class flash devices as V4L2 subdevs. The functions should
> be called from the LED subsystem device driver. In case the
> Multimedia Framework support is disabled in the kernel config
> the functions' empty versions will be used.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/media/v4l2-core/Makefile | 2 +-
> drivers/media/v4l2-core/v4l2-flash.c | 320 ++++++++++++++++++++++++++++++++++
> include/media/v4l2-flash.h | 102 +++++++++++
> 3 files changed, 423 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
> create mode 100644 include/media/v4l2-flash.h
>
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index c6ae7ba..63e8f03 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -6,7 +6,7 @@ tuner-objs := tuner-core.o
>
> videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> - v4l2-async.o
> + v4l2-async.o v4l2-flash.o
> ifeq ($(CONFIG_COMPAT),y)
> videodev-objs += v4l2-compat-ioctl32.o
> endif
> diff --git a/drivers/media/v4l2-core/v4l2-flash.c b/drivers/media/v4l2-core/v4l2-flash.c
> new file mode 100644
> index 0000000..6be0ba9
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-flash.c
> @@ -0,0 +1,320 @@
> +/*
> + * V4L2 flash LED subdevice registration helpers.
> + *
> + * 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 <media/v4l2-ioctl.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-flash.h>
> +#include <media/v4l2-dev.h>
Alphabetical order, please.
> +
> +static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
> +
> +{
> + struct v4l2_flash *flash = ctrl_to_flash(c);
> + struct led_classdev *led_cdev = flash->led_cdev;
> + unsigned int fault;
> + int ret;
> +
> + switch (c->id) {
> + case V4L2_CID_FLASH_STROBE_STATUS:
> + ret = led_update_brightness(led_cdev);
> + if (ret < 0)
> + return ret;
> + c->val = !!ret;
> + return 0;
> + case V4L2_CID_FLASH_FAULT:
> + /* led faults map directly to V4L2 flash faults */
> + ret = led_get_flash_fault(led_cdev, &fault);
> + if (!ret)
> + c->val = fault;
> + return ret;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int v4l2_flash_set_intensity(struct v4l2_flash *flash,
> + unsigned int intensity)
> +{
> + struct led_classdev *led_cdev = flash->led_cdev;
> + unsigned int fault;
> + int ret;
> +
> + ret = led_get_flash_fault(led_cdev, &fault);
> + if (ret < 0 || fault)
> + return -EINVAL;
Is it meaningful to check the faults here?
The existing flash controller drivers mostly do not. The responsibility is
left to the user --- something the user should probably do after the strobe
has expectedly finished. This isn't particularly very well documented in the
spec, though.
Also, the presence of every fault does not prevent using the flash.
> + led_set_brightness(led_cdev, intensity);
Where do you convert between the LED framework brightness and the value used
by the V4L2 controls?
> +
> + return ret;
> +}
> +
> +static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
> +{
> + struct v4l2_flash *flash = ctrl_to_flash(c);
> + struct led_classdev *led_cdev = flash->led_cdev;
> + int ret = 0;
> +
> + switch (c->id) {
> + case V4L2_CID_FLASH_LED_MODE:
> + switch (c->val) {
> + case V4L2_FLASH_LED_MODE_NONE:
> + /* clear flash mode on releae */
It's not uncommon for the user to leave the mode to something else than none
when the user goes away. Could there be other ways to mediate access?
> + ret = led_set_flash_mode(led_cdev, false);
> + if (ret < 0)
> + return ret;
> + mutex_lock(&led_cdev->led_lock);
> + led_sysfs_unlock(led_cdev);
> + mutex_unlock(&led_cdev->led_lock);
> + break;
> + case V4L2_FLASH_LED_MODE_FLASH:
> + mutex_lock(&led_cdev->led_lock);
> + led_sysfs_lock(led_cdev);
> + mutex_unlock(&led_cdev->led_lock);
> +
> + ret = led_set_flash_mode(led_cdev, true);
> + if (ret < 0)
> + return ret;
> + if (flash->ctrl.source->val ==
> + V4L2_FLASH_STROBE_SOURCE_EXTERNAL) {
> + ret = led_set_hw_triggered(led_cdev, true);
> + if (ret < 0)
> + return ret;
> + ret = v4l2_flash_set_intensity(flash,
> + flash->flash_intensity);
> + } else {
> + ret = led_set_hw_triggered(led_cdev, false);
> + if (ret < 0)
> + return ret;
As the switch() is the last thing you'll do anyway, you could as well just
return here. Same below. Up to you.
> + }
> + break;
> + case V4L2_FLASH_LED_MODE_TORCH:
> + mutex_lock(&led_cdev->led_lock);
> + led_sysfs_lock(led_cdev);
> + mutex_unlock(&led_cdev->led_lock);
> +
> + ret = led_set_flash_mode(led_cdev, false);
> + if (ret < 0)
> + return ret;
> + /* torch is always triggered by software */
> + ret = led_set_hw_triggered(led_cdev, false);
> + if (ret)
> + return -EINVAL;
> + ret = v4l2_flash_set_intensity(flash,
> + flash->torch_intensity);
> + break;
> + }
> + break;
> + case V4L2_CID_FLASH_STROBE_SOURCE:
> + ret = led_set_hw_triggered(led_cdev,
> + c->val == V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
> + break;
> + case V4L2_CID_FLASH_STROBE:
> + if (flash->ctrl.led_mode->val != V4L2_FLASH_LED_MODE_FLASH ||
> + flash->ctrl.source->val != V4L2_FLASH_STROBE_SOURCE_SOFTWARE)
> + return -EINVAL;
> + led_set_flash_timeout(led_cdev, flash->flash_timeout);
> + ret = v4l2_flash_set_intensity(flash,
> + flash->flash_intensity);
Same comment as on the other patch: strobe should be just strobe, nothing
else.
> + break;
> + case V4L2_CID_FLASH_STROBE_STOP:
> + led_set_brightness(led_cdev, 0);
> + break;
> + case V4L2_CID_FLASH_TIMEOUT:
> + flash->flash_timeout = c->val;
> + break;
> + case V4L2_CID_FLASH_INTENSITY:
> + flash->flash_intensity = c->val;
> + break;
> + case V4L2_CID_FLASH_TORCH_INTENSITY:
> + flash->torch_intensity = c->val;
> + if (flash->ctrl.led_mode->val == V4L2_FLASH_LED_MODE_TORCH)
> + ret = v4l2_flash_set_intensity(flash,
> + flash->torch_intensity);
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops v4l2_flash_ctrl_ops = {
> + .g_volatile_ctrl = v4l2_flash_g_volatile_ctrl,
> + .s_ctrl = v4l2_flash_s_ctrl,
> +};
> +
> +static int v4l2_flash_init_controls(struct v4l2_flash *flash,
> + struct v4l2_flash_ctrl_config *config)
> +
> +{
> + unsigned int mask;
> + struct v4l2_ctrl *ctrl;
> + struct v4l2_ctrl_config *ctrl_cfg;
> + bool has_flash = config->flags & V4L2_FLASH_CFG_LED_FLASH;
> + bool has_torch = config->flags & V4L2_FLASH_CFG_LED_TORCH;
> + int ret, num_ctrls;
> +
> + if (!has_flash && !has_torch)
> + return -EINVAL;
> +
> + num_ctrls = has_flash ? 8 : 2;
> + if (config->flags & V4L2_FLASH_CFG_FAULTS_MASK)
> + ++num_ctrls;
> +
> + v4l2_ctrl_handler_init(&flash->hdl, num_ctrls);
> +
> + mask = 1 << V4L2_FLASH_LED_MODE_NONE;
> + if (has_flash)
> + mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
> + if (has_torch)
> + mask |= 1 << V4L2_FLASH_LED_MODE_TORCH;
I don't expect to see this on LED flash devices. :-)
> + /* Configure TORCH_INTENSITY ctrl */
> + ctrl_cfg = &config->torch_intensity;
> + ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_TORCH_INTENSITY,
> + ctrl_cfg->min, ctrl_cfg->max,
> + ctrl_cfg->step, ctrl_cfg->def);
> +
> + if (has_flash) {
> + /* Configure FLASH_LED_MODE ctrl */
> + flash->ctrl.led_mode = v4l2_ctrl_new_std_menu(&flash->hdl,
> + &v4l2_flash_ctrl_ops, V4L2_CID_FLASH_LED_MODE,
> + V4L2_FLASH_LED_MODE_TORCH, ~mask,
> + V4L2_FLASH_LED_MODE_NONE);
> +
> + /* Configure FLASH_STROBE_SOURCE ctrl */
> + mask = 1 << V4L2_FLASH_STROBE_SOURCE_SOFTWARE |
> + 1 << V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
Not every implementation supports hardware strobe. It could be that the
flash chip does but the pin isn't connected. That's the case on e.g. the
Nokia N900. The hardware strobe source is typically the sensor but not all
sensors support it. So you just have to cope with what you have. :-)
> + flash->ctrl.source = v4l2_ctrl_new_std_menu(&flash->hdl,
> + &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_STROBE_SOURCE,
> + V4L2_FLASH_STROBE_SOURCE_EXTERNAL,
> + ~mask,
> + V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
> +
> + /* Configure FLASH_STROBE ctrl */
> + ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_STROBE, 0, 1, 1, 0);
> + if (ctrl)
> + ctrl->type = V4L2_CTRL_TYPE_BUTTON;
I think this is already handled by the control framework.
> + /* Configure FLASH_STROBE_STOP ctrl */
> + ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_STROBE_STOP,
> + 0, 1, 1, 0);
> + if (ctrl)
> + ctrl->type = V4L2_CTRL_TYPE_BUTTON;
Same here.
> + /* Configure FLASH_STROBE_STATUS ctrl */
> + ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_STROBE_STATUS,
> + 0, 1, 1, 1);
> + if (ctrl)
> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
> + V4L2_CTRL_FLAG_READ_ONLY;
> +
> + /* Configure FLASH_TIMEOUT ctrl */
> + ctrl_cfg = &config->flash_timeout;
> + ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_TIMEOUT, ctrl_cfg->min,
> + ctrl_cfg->max, ctrl_cfg->step,
> + ctrl_cfg->def);
> +
> + /* Configure FLASH_INTENSITY ctrl */
> + ctrl_cfg = &config->flash_intensity;
> + ctrl = v4l2_ctrl_new_std(&flash->hdl, &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_INTENSITY,
> + ctrl_cfg->min, ctrl_cfg->max,
> + ctrl_cfg->step, ctrl_cfg->def);
> +
> + if (config->flags & V4L2_FLASH_CFG_FAULTS_MASK) {
> + /* Configure FLASH_FAULT ctrl */
> + ctrl = v4l2_ctrl_new_std(&flash->hdl,
> + &v4l2_flash_ctrl_ops,
> + V4L2_CID_FLASH_FAULT, 0,
> + config->flags &
> + V4L2_FLASH_CFG_FAULTS_MASK,
> + 0, 0);
> + if (ctrl) {
> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
> + V4L2_CTRL_FLAG_READ_ONLY;
> + ctrl->type = V4L2_CTRL_TYPE_BITMASK;
And here.
> + }
> + }
> + }
> +
> + if (flash->hdl.error) {
> + ret = flash->hdl.error;
> + goto error_free;
> + }
> +
> + ret = v4l2_ctrl_handler_setup(&flash->hdl);
> + if (ret < 0)
> + goto error_free;
> +
> + flash->subdev.ctrl_handler = &flash->hdl;
> +
> + return 0;
> +
> +error_free:
> + v4l2_ctrl_handler_free(&flash->hdl);
> + return ret;
> +}
> +
> +/* v4l2_subdev_init requires this structure */
...because v4l2_subdev_call() requires it to be set. I might leave the
comment out. Up to you.
> +static struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
> +};
> +
> +int v4l2_flash_init(struct led_classdev *led_cdev, struct v4l2_flash *flash,
> + struct v4l2_flash_ctrl_config *config)
> +{
> + struct v4l2_subdev *sd = &flash->subdev;
> + int ret;
> +
> + flash->led_cdev = led_cdev;
> + sd->dev = led_cdev->dev->parent;
> + v4l2_subdev_init(sd, &v4l2_flash_subdev_ops);
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + snprintf(sd->name, sizeof(sd->name), led_cdev->name);
> +
> + ret = v4l2_flash_init_controls(flash, config);
> + if (ret < 0)
> + goto err_init_controls;
> +
> + ret = media_entity_init(&sd->entity, 0, NULL, 0);
> + if (ret < 0)
> + goto err_init_entity;
> +
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +
> + ret = v4l2_async_register_subdev(sd);
> + if (ret < 0)
> + goto err_init_entity;
> +
> + return 0;
> +
> +err_init_entity:
> + media_entity_cleanup(&sd->entity);
> +err_init_controls:
> + v4l2_ctrl_handler_free(sd->ctrl_handler);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_flash_init);
> +
> +void v4l2_flash_release(struct v4l2_flash *flash)
> +{
> + media_entity_cleanup(&flash->subdev.entity);
media_entity_cleanup() has to be called after
v4l2_async_unregister_subdev(). v4l2_device_unregister_subdev() assumes the
entity to be there.
> + v4l2_ctrl_handler_free(flash->subdev.ctrl_handler);
> + v4l2_async_unregister_subdev(&flash->subdev);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_flash_release);
> diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
> new file mode 100644
> index 0000000..138edae
> --- /dev/null
> +++ b/include/media/v4l2-flash.h
> @@ -0,0 +1,102 @@
> +/*
> + * V4L2 flash LED subdevice registration helpers.
> + *
> + * 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 _V4L2_FLASH_H
> +#define _V4L2_FLASH_H
> +
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-dev.h>
> +#include <linux/leds.h>
Order, please.
> +
> +/*
> + * Supported led fault and mode bits -
> + * must be kept in synch with V4L2_FLASH_FAULT bits
> + */
> +#define V4L2_FLASH_CFG_FAULT_OVER_VOLTAGE (1 << 0)
> +#define V4L2_FLASH_CFG_FAULT_TIMEOUT (1 << 1)
> +#define V4L2_FLASH_CFG_FAULT_OVER_TEMPERATURE (1 << 2)
> +#define V4L2_FLASH_CFG_FAULT_SHORT_CIRCUIT (1 << 3)
> +#define V4L2_FLASH_CFG_FAULT_OVER_CURRENT (1 << 4)
> +#define V4L2_FLASH_CFG_FAULT_INDICATOR (1 << 5)
> +#define V4L2_FLASH_CFG_FAULTS_MASK 0x3f
> +#define V4L2_FLASH_CFG_LED_FLASH (1 << 6)
> +#define V4L2_FLASH_CFG_LED_TORCH (1 << 7)
> +
> +/* Flash control config data initializer */
> +
> +struct v4l2_flash {
> + struct led_classdev *led_cdev;
> + struct v4l2_subdev subdev;
> + struct v4l2_ctrl_handler hdl;
> +
> + struct {
> + struct v4l2_ctrl *source;
> + struct v4l2_ctrl *led_mode;
> + } ctrl;
> +
> + unsigned int flash_intensity;
> + unsigned int torch_intensity;
> + unsigned int flash_timeout;
> +};
> +
> +struct v4l2_flash_ctrl_config {
> + struct v4l2_ctrl_config flash_timeout;
> + struct v4l2_ctrl_config flash_intensity;
> + struct v4l2_ctrl_config torch_intensity;
> + unsigned int flags;
> +};
> +
> +static inline struct v4l2_flash *subdev_to_flash(struct v4l2_subdev *sd)
I'd prefer to have at least the v4l2_ prefix in public V4L2 sub-device
framework functions.
> +{
> + return container_of(sd, struct v4l2_flash, subdev);
> +}
> +
> +static inline struct v4l2_flash *ctrl_to_flash(struct v4l2_ctrl *c)
How about e.g. v4l2_ctrl_to_v4l2_flash?
> +{
> + return container_of(c->handler, struct v4l2_flash, hdl);
> +}
> +
> +#ifdef CONFIG_VIDEO_V4L2
> +/**
> + * v4l2_flash_init - initialize V4L2 flash led sub-device
> + * @led_cdev: the LED to create subdev upon
> + * @flash: a structure representing V4L2 flash led device
> + * @config: initial data for the flash led subdev controls
> + *
> + * Create V4L2 subdev wrapping given LED subsystem device.
> + */
> +int v4l2_flash_init(struct led_classdev *led_cdev, struct v4l2_flash *flash,
> + struct v4l2_flash_ctrl_config *config);
> +
> +/**
> + * v4l2_flash_release - release V4L2 flash led sub-device
> + * @flash: a structure representing V4L2 flash led device
> + *
> + * Release V4L2 flash led subdev.
> + */
> +void v4l2_flash_release(struct v4l2_flash *flash);
> +#else
> +static inline int v4l2_flash_init(struct led_classdev *led_cdev,
> + struct v4l2_flash *flash,
> + struct v4l2_flash_ctrl_config *config)
> +{
> + return 0;
> +}
> +
> +static inline void v4l2_flash_release(struct v4l2_flash *flash)
> +{
> +}
> +#endif
> +
> +#endif
#endif /* WHATWASIT */ would be nice.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices
2014-03-24 0:08 ` Sakari Ailus
@ 2014-03-28 15:30 ` Jacek Anaszewski
2014-03-31 9:37 ` Sakari Ailus
0 siblings, 1 reply; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-28 15:30 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park
Hi Sakari,
On 03/24/2014 01:08 AM, Sakari Ailus wrote:
> Hi Jacek,
>
[...]
>> +static int v4l2_flash_set_intensity(struct v4l2_flash *flash,
>> + unsigned int intensity)
>> +{
>> + struct led_classdev *led_cdev = flash->led_cdev;
>> + unsigned int fault;
>> + int ret;
>> +
>> + ret = led_get_flash_fault(led_cdev, &fault);
>> + if (ret < 0 || fault)
>> + return -EINVAL;
>
> Is it meaningful to check the faults here?
>
> The existing flash controller drivers mostly do not. The responsibility is
> left to the user --- something the user should probably do after the strobe
> has expectedly finished. This isn't particularly very well documented in the
> spec, though.
I was influenced by the documentation which says that sometimes strobing
the flash may not be possible due to faults. But I agree that checking
the faults should be user's responsibility.
> Also, the presence of every fault does not prevent using the flash.
>
>> + led_set_brightness(led_cdev, intensity);
>
> Where do you convert between the LED framework brightness and the value used
> by the V4L2 controls?
I think that there is no need for conversion. AFAIK the LED subsystem
doesn't specify units of the brightness. It specifies LED_FULL enum
value but not all LED drivers stick to it. Moreover it limits
brightness resolution to 256 levels.
>> +
>> + return ret;
>> +}
>> +
>> +static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
>> +{
>> + struct v4l2_flash *flash = ctrl_to_flash(c);
>> + struct led_classdev *led_cdev = flash->led_cdev;
>> + int ret = 0;
>> +
>> + switch (c->id) {
>> + case V4L2_CID_FLASH_LED_MODE:
>> + switch (c->val) {
>> + case V4L2_FLASH_LED_MODE_NONE:
>> + /* clear flash mode on releae */
>
> It's not uncommon for the user to leave the mode to something else than none
> when the user goes away. Could there be other ways to mediate access?
IMHO user space application should release the mode on exit as any other
resource it acquires. However if it is terminated unexpectedly
the sysfs will remain locked forever. Maybe a dedicated sysfs
attribute should be provided in the LED subsystem for controlling the
sysfs lock state? It would have to be always available for the user
though.
>> +static int v4l2_flash_init_controls(struct v4l2_flash *flash,
>> + struct v4l2_flash_ctrl_config *config)
>> +
>> +{
>> + unsigned int mask;
>> + struct v4l2_ctrl *ctrl;
>> + struct v4l2_ctrl_config *ctrl_cfg;
>> + bool has_flash = config->flags & V4L2_FLASH_CFG_LED_FLASH;
>> + bool has_torch = config->flags & V4L2_FLASH_CFG_LED_TORCH;
>> + int ret, num_ctrls;
>> +
>> + if (!has_flash && !has_torch)
>> + return -EINVAL;
>> +
>> + num_ctrls = has_flash ? 8 : 2;
>> + if (config->flags & V4L2_FLASH_CFG_FAULTS_MASK)
>> + ++num_ctrls;
>> +
>> + v4l2_ctrl_handler_init(&flash->hdl, num_ctrls);
>> +
>> + mask = 1 << V4L2_FLASH_LED_MODE_NONE;
>> + if (has_flash)
>> + mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
>> + if (has_torch)
>> + mask |= 1 << V4L2_FLASH_LED_MODE_TORCH;
>
> I don't expect to see this on LED flash devices. :-)
I don't get your point here. Could you be more specific? :)
Torch only mode is supported by V4L2_CID_FLASH_LED_MODE control,
isn't it?
Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices
2014-03-28 15:30 ` Jacek Anaszewski
@ 2014-03-31 9:37 ` Sakari Ailus
0 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2014-03-31 9:37 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park
Hi jacek,
On Fri, Mar 28, 2014 at 04:30:49PM +0100, Jacek Anaszewski wrote:
...
> >>+static int v4l2_flash_set_intensity(struct v4l2_flash *flash,
> >>+ unsigned int intensity)
> >>+{
> >>+ struct led_classdev *led_cdev = flash->led_cdev;
> >>+ unsigned int fault;
> >>+ int ret;
> >>+
> >>+ ret = led_get_flash_fault(led_cdev, &fault);
> >>+ if (ret < 0 || fault)
> >>+ return -EINVAL;
> >
> >Is it meaningful to check the faults here?
> >
> >The existing flash controller drivers mostly do not. The responsibility is
> >left to the user --- something the user should probably do after the strobe
> >has expectedly finished. This isn't particularly very well documented in the
> >spec, though.
>
> I was influenced by the documentation which says that sometimes strobing
> the flash may not be possible due to faults. But I agree that checking
> the faults should be user's responsibility.
What that means is that the presence of the fault *on the chip* may limit
the device's willingness to actually strobe. The driver does not need to
enforce it; the chip does.
> >Also, the presence of every fault does not prevent using the flash.
> >
> >>+ led_set_brightness(led_cdev, intensity);
> >
> >Where do you convert between the LED framework brightness and the value used
> >by the V4L2 controls?
>
> I think that there is no need for conversion. AFAIK the LED subsystem
> doesn't specify units of the brightness. It specifies LED_FULL enum
> value but not all LED drivers stick to it. Moreover it limits
> brightness resolution to 256 levels.
The V4L2 control's unit is mA. Does this mean that the unit in the LED API
also becomes mA then?
> >>+
> >>+ return ret;
> >>+}
> >>+
> >>+static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
> >>+{
> >>+ struct v4l2_flash *flash = ctrl_to_flash(c);
> >>+ struct led_classdev *led_cdev = flash->led_cdev;
> >>+ int ret = 0;
> >>+
> >>+ switch (c->id) {
> >>+ case V4L2_CID_FLASH_LED_MODE:
> >>+ switch (c->val) {
> >>+ case V4L2_FLASH_LED_MODE_NONE:
> >>+ /* clear flash mode on releae */
> >
> >It's not uncommon for the user to leave the mode to something else than none
> >when the user goes away. Could there be other ways to mediate access?
>
> IMHO user space application should release the mode on exit as any other
> resource it acquires. However if it is terminated unexpectedly
The application's resources are released by the kernel when the application
exists but V4L2 control's don't behave that way.
> the sysfs will remain locked forever. Maybe a dedicated sysfs
> attribute should be provided in the LED subsystem for controlling the
> sysfs lock state? It would have to be always available for the user
> though.
How about using the information on open file handles to the flash sub-device
instead? If someone holds a file handle there, sure then the device is in
use by that program? The advantage compared to the control value is that the
file handles are released when the application quits whether or not any
cleanup is performed by the application. I don't immediately see other
reasons to keep a file handle to a V4L2 flash sub-device open, except, well,
to find about its capabilities.
> >>+static int v4l2_flash_init_controls(struct v4l2_flash *flash,
> >>+ struct v4l2_flash_ctrl_config *config)
> >>+
> >>+{
> >>+ unsigned int mask;
> >>+ struct v4l2_ctrl *ctrl;
> >>+ struct v4l2_ctrl_config *ctrl_cfg;
> >>+ bool has_flash = config->flags & V4L2_FLASH_CFG_LED_FLASH;
> >>+ bool has_torch = config->flags & V4L2_FLASH_CFG_LED_TORCH;
> >>+ int ret, num_ctrls;
> >>+
> >>+ if (!has_flash && !has_torch)
> >>+ return -EINVAL;
> >>+
> >>+ num_ctrls = has_flash ? 8 : 2;
> >>+ if (config->flags & V4L2_FLASH_CFG_FAULTS_MASK)
> >>+ ++num_ctrls;
> >>+
> >>+ v4l2_ctrl_handler_init(&flash->hdl, num_ctrls);
> >>+
> >>+ mask = 1 << V4L2_FLASH_LED_MODE_NONE;
> >>+ if (has_flash)
> >>+ mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
> >>+ if (has_torch)
> >>+ mask |= 1 << V4L2_FLASH_LED_MODE_TORCH;
> >
> >I don't expect to see this on LED flash devices. :-)
>
> I don't get your point here. Could you be more specific? :)
> Torch only mode is supported by V4L2_CID_FLASH_LED_MODE control,
> isn't it?
Have you seen a LED flash controller which does not implement the torch
mode?
I have no objections to keeping the check though, but there were more
important things missing at the time.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH/RFC 5/8] media: exynos4-is: Add support for v4l2-flash subdevs
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
` (3 preceding siblings ...)
2014-03-20 14:51 ` [PATCH/RFC 4/8] media: Add registration helpers for V4L2 flash sub-devices Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-20 14:51 ` [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell Jacek Anaszewski
` (2 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski
This patch adds suppport for external v4l2-flash devices.
The support includes parsing camera-flash DT property
and asynchronous subdevice registration.
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/media/platform/exynos4-is/media-dev.c | 34 ++++++++++++++++++++++---
drivers/media/platform/exynos4-is/media-dev.h | 14 +++++++++-
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index e62211a..0f80210 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -28,6 +28,7 @@
#include <linux/types.h>
#include <linux/slab.h>
#include <media/v4l2-async.h>
+#include <media/v4l2-flash.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-of.h>
#include <media/media-device.h>
@@ -400,7 +401,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
struct device_node *port,
unsigned int index)
{
- struct device_node *rem, *ep, *np;
+ struct device_node *rem, *ep, *np, *fn;
struct fimc_source_info *pd;
struct v4l2_of_endpoint endpoint;
u32 val;
@@ -440,6 +441,14 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
return -EINVAL;
}
+ fn = of_parse_phandle(rem, "camera-flash", 0);
+ if (fn) {
+ fmd->flash[fmd->num_flashes].asd.match_type =
+ V4L2_ASYNC_MATCH_OF;
+ fmd->flash[fmd->num_flashes].asd.match.of.node = fn;
+ fmd->num_flashes++;
+ }
+
if (fimc_input_is_parallel(endpoint.base.port)) {
if (endpoint.bus_type == V4L2_MBUS_PARALLEL)
pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601;
@@ -1531,6 +1540,15 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
struct fimc_sensor_info *si = NULL;
int i;
+ /* Register flash subdev if detected any */
+ for (i = 0; i < ARRAY_SIZE(fmd->flash); i++) {
+ if (fmd->flash[i].asd.match.of.node == subdev->dev->of_node) {
+ fmd->flash[i].v4l2_flash = subdev_to_flash(subdev);
+ fmd->num_flashes++;
+ return 0;
+ }
+ }
+
/* Find platform data for this sensor subdev */
for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
if (fmd->sensor[i].asd.match.of.node == subdev->dev->of_node)
@@ -1578,7 +1596,7 @@ static int fimc_md_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct v4l2_device *v4l2_dev;
struct fimc_md *fmd;
- int ret;
+ int i, ret;
fmd = devm_kzalloc(dev, sizeof(*fmd), GFP_KERNEL);
if (!fmd)
@@ -1646,6 +1664,12 @@ static int fimc_md_probe(struct platform_device *pdev)
mutex_unlock(&fmd->media_dev.graph_mutex);
goto err_m_ent;
}
+
+ if (dev->of_node) {
+ for (i = 0; i < fmd->num_flashes; ++i)
+ fmd->async_subdevs[fmd->num_sensors + i] =
+ &fmd->flash[i].asd;
+ }
}
mutex_unlock(&fmd->media_dev.graph_mutex);
@@ -1664,12 +1688,14 @@ static int fimc_md_probe(struct platform_device *pdev)
goto err_attr;
}
- if (fmd->num_sensors > 0) {
+ if (fmd->num_sensors > 0 || fmd->num_flashes > 0) {
fmd->subdev_notifier.subdevs = fmd->async_subdevs;
- fmd->subdev_notifier.num_subdevs = fmd->num_sensors;
+ fmd->subdev_notifier.num_subdevs = fmd->num_sensors +
+ fmd->num_flashes;
fmd->subdev_notifier.bound = subdev_notifier_bound;
fmd->subdev_notifier.complete = subdev_notifier_complete;
fmd->num_sensors = 0;
+ fmd->num_flashes = 0;
ret = v4l2_async_notifier_register(&fmd->v4l2_dev,
&fmd->subdev_notifier);
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index ee1e251..9af9de8 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -18,6 +18,7 @@
#include <media/media-device.h>
#include <media/media-entity.h>
#include <media/v4l2-device.h>
+#include <media/v4l2-flash.h>
#include <media/v4l2-subdev.h>
#include <media/s5p_fimc.h>
@@ -33,6 +34,8 @@
#define PINCTRL_STATE_IDLE "idle"
#define FIMC_MAX_SENSORS 4
+#define FIMC_MAX_FLASHES 1
+#define FIMC_MAX_ASYNC_SUBDEVS (FIMC_MAX_SENSORS + FIMC_MAX_FLASHES)
#define FIMC_MAX_CAMCLKS 2
#define DEFAULT_SENSOR_CLK_FREQ 24000000U
@@ -93,6 +96,11 @@ struct fimc_sensor_info {
struct fimc_dev *host;
};
+struct fimc_flash_info {
+ struct v4l2_flash *v4l2_flash;
+ struct v4l2_async_subdev asd;
+};
+
struct cam_clk {
struct clk_hw hw;
struct fimc_md *fmd;
@@ -104,6 +112,8 @@ struct cam_clk {
* @csis: MIPI CSIS subdevs data
* @sensor: array of registered sensor subdevs
* @num_sensors: actual number of registered sensors
+ * @flash: array of registered flash subdevs
+ * @num_flashes: actual number of registered flashes
* @camclk: external sensor clock information
* @fimc: array of registered fimc devices
* @fimc_is: fimc-is data structure
@@ -123,6 +133,8 @@ struct fimc_md {
struct fimc_csis_info csis[CSIS_MAX_ENTITIES];
struct fimc_sensor_info sensor[FIMC_MAX_SENSORS];
int num_sensors;
+ struct fimc_flash_info flash[FIMC_MAX_FLASHES];
+ int num_flashes;
struct fimc_camclk_info camclk[FIMC_MAX_CAMCLKS];
struct clk *wbclk[FIMC_MAX_WBCLKS];
struct fimc_lite *fimc_lite[FIMC_LITE_MAX_DEVS];
@@ -149,7 +161,7 @@ struct fimc_md {
} clk_provider;
struct v4l2_async_notifier subdev_notifier;
- struct v4l2_async_subdev *async_subdevs[FIMC_MAX_SENSORS];
+ struct v4l2_async_subdev *async_subdevs[FIMC_MAX_ASYNC_SUBDEVS];
bool user_subdev_api;
spinlock_t slock;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
` (4 preceding siblings ...)
2014-03-20 14:51 ` [PATCH/RFC 5/8] media: exynos4-is: Add support for v4l2-flash subdevs Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-20 15:34 ` Lee Jones
2014-03-20 14:51 ` [PATCH/RFC 7/8] DT: Add documentation for the mfd Maxim max77693 " Jacek Anaszewski
[not found] ` <1395327070-20215-1-git-send-email-j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
7 siblings, 1 reply; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski, Bryan Wu,
Richard Purdie, SangYoung Son, Samuel Ortiz, Lee Jones
This patch adds led-flash support to Maxim max77693 chipset.
Device can be exposed to user space through LED subsystem
sysfs interface or through V4L2 subdevice when the support
for Multimedia Framework is enabled. Device supports up to
two leds which can work in flash and torch mode. Leds can
be triggered externally or by software.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
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>
Cc: SangYoung Son <hello.son@smasung.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
drivers/leds/Kconfig | 9 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-max77693.c | 768 ++++++++++++++++++++++++++++++++++++++++++
drivers/mfd/max77693.c | 21 +-
include/linux/mfd/max77693.h | 32 ++
5 files changed, 825 insertions(+), 6 deletions(-)
create mode 100644 drivers/leds/leds-max77693.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 2062682..f2d0e2c 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -454,6 +454,15 @@ config LEDS_TCA6507
LED driver chips accessed via the I2C bus.
Driver support brightness control and hardware-assisted blinking.
+config LEDS_MAX77693
+ tristate "LED support for MAX77693 Flash"
+ depends on MFD_MAX77693
+ depends on OF
+ help
+ This option enables support for the flash part of the MAX77693
+ multifunction device. It has build in control for two leds in flash
+ and torch mode.
+
config LEDS_MAX8997
tristate "LED support for MAX8997 PMIC"
depends on LEDS_CLASS && MFD_MAX8997
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 3cd76db..597585f 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o
+obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o
obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o
diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
new file mode 100644
index 0000000..8b5daa1
--- /dev/null
+++ b/drivers/leds/leds-max77693.c
@@ -0,0 +1,768 @@
+/*
+ * Copyright (C) 2014, Samsung Electronics Co., Ltd.
+ *
+ * Authors: Andrzej Hajda <a.hajda@samsung.com>
+ * 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/slab.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/leds.h>
+#include <linux/mfd/max77693.h>
+#include <linux/mfd/max77693-private.h>
+#include <linux/mutex.h>
+#include <linux/workqueue.h>
+#include <asm/div64.h>
+
+#include <media/v4l2-flash.h>
+
+#define MAX77693_LED_NAME "max77693-led"
+
+#define MAX77693_TORCH_IOUT_BITS 4
+
+#define MAX77693_TORCH_NO_TIMER 0x40
+#define MAX77693_FLASH_TIMER_LEVEL 0x80
+
+#define MAX77693_FLASH_EN_OFF 0
+#define MAX77693_FLASH_EN_FLASH 1
+#define MAX77693_FLASH_EN_TORCH 2
+#define MAX77693_FLASH_EN_ON 3
+
+#define MAX77693_FLASH_EN1_SHIFT 6
+#define MAX77693_FLASH_EN2_SHIFT 4
+#define MAX77693_TORCH_EN1_SHIFT 2
+#define MAX77693_TORCH_EN2_SHIFT 0
+
+#define MAX77693_FLASH_LOW_BATTERY_EN 0x80
+
+#define MAX77693_FLASH_BOOST_FIXED 0x04
+#define MAX77693_FLASH_BOOST_LEDNUM_2 0x80
+
+#define MAX77693_FLASH_TIMEOUT_MIN 62500
+#define MAX77693_FLASH_TIMEOUT_MAX 1000000
+#define MAX77693_FLASH_TIMEOUT_STEP 62500
+
+#define MAX77693_TORCH_TIMEOUT_MIN 262000
+#define MAX77693_TORCH_TIMEOUT_MAX 15728000
+
+#define MAX77693_FLASH_IOUT_MIN 15625
+#define MAX77693_FLASH_IOUT_MAX_1LED 1000000
+#define MAX77693_FLASH_IOUT_MAX_2LEDS 625000
+#define MAX77693_FLASH_IOUT_STEP 15625
+
+#define MAX77693_TORCH_IOUT_MIN 15625
+#define MAX77693_TORCH_IOUT_MAX 250000
+#define MAX77693_TORCH_IOUT_STEP 15625
+
+#define MAX77693_FLASH_VSYS_MIN 2400
+#define MAX77693_FLASH_VSYS_MAX 3400
+#define MAX77693_FLASH_VSYS_STEP 33
+
+#define MAX77693_FLASH_VOUT_MIN 3300
+#define MAX77693_FLASH_VOUT_MAX 5500
+#define MAX77693_FLASH_VOUT_STEP 25
+#define MAX77693_FLASH_VOUT_RMIN 0x0c
+
+#define MAX77693_LED_STATUS_FLASH_ON (1 << 3)
+#define MAX77693_LED_STATUS_TORCH_ON (1 << 2)
+
+#define MAX77693_LED_FLASH_INT_FLED2_OPEN (1 << 0)
+#define MAX77693_LED_FLASH_INT_FLED2_SHORT (1 << 1)
+#define MAX77693_LED_FLASH_INT_FLED1_OPEN (1 << 2)
+#define MAX77693_LED_FLASH_INT_FLED1_SHORT (1 << 3)
+#define MAX77693_LED_FLASH_INT_OVER_CURRENT (1 << 4)
+
+enum {
+ FLASH1,
+ FLASH2,
+ TORCH1,
+ TORCH2
+};
+
+enum {
+ FLASH,
+ TORCH
+};
+
+enum max77693_led_mode {
+ MODE_OFF,
+ MODE_FLASH,
+ MODE_TORCH,
+ MODE_FLASH_EXTERNAL,
+};
+
+struct max77693_led {
+ struct regmap *regmap;
+ struct platform_device *pdev;
+ struct max77693_led_platform_data *pdata;
+ struct mutex lock;
+
+ struct led_classdev ldev;
+ struct v4l2_flash v4l2_flash;
+
+ unsigned int max_flash_intensity;
+ unsigned int max_torch_intensity;
+ unsigned int flash_brightness;
+ unsigned int flash_timeout;
+ unsigned int torch_brightness;
+
+ struct work_struct work_brightness_set;
+ struct work_struct work_strobe_set;
+};
+
+static u8 max77693_led_iout_to_reg(u32 ua)
+{
+ if (ua < MAX77693_FLASH_IOUT_MIN)
+ ua = MAX77693_FLASH_IOUT_MIN;
+ return (ua - MAX77693_FLASH_IOUT_MIN) / MAX77693_FLASH_IOUT_STEP;
+}
+
+static u8 max77693_flash_timeout_to_reg(u32 us)
+{
+ return (us - MAX77693_FLASH_TIMEOUT_MIN) / MAX77693_FLASH_TIMEOUT_STEP;
+}
+
+static const u32 max77693_torch_timeouts[] = {
+ 262000, 524000, 786000, 1048000,
+ 1572000, 2096000, 2620000, 3144000,
+ 4193000, 5242000, 6291000, 7340000,
+ 9437000, 11534000, 13631000, 1572800
+};
+
+static u8 max77693_torch_timeout_to_reg(u32 us)
+{
+ int i, b = 0, e = ARRAY_SIZE(max77693_torch_timeouts);
+
+ while (e - b > 1) {
+ i = b + (e - b) / 2;
+ if (us < max77693_torch_timeouts[i])
+ e = i;
+ else
+ b = i;
+ }
+ return b;
+}
+
+static struct max77693_led *ldev_to_led(struct led_classdev *ldev)
+{
+ return container_of(ldev, struct max77693_led, ldev);
+}
+
+static u32 max77693_torch_timeout_from_reg(u8 reg)
+{
+ return max77693_torch_timeouts[reg];
+}
+
+static u8 max77693_led_vsys_to_reg(u32 mv)
+{
+ return ((mv - MAX77693_FLASH_VSYS_MIN) / MAX77693_FLASH_VSYS_STEP) << 2;
+}
+
+static u8 max77693_led_vout_to_reg(u32 mv)
+{
+ return (mv - MAX77693_FLASH_VOUT_MIN) / MAX77693_FLASH_VOUT_STEP +
+ MAX77693_FLASH_VOUT_RMIN;
+}
+
+/* split composite current @i into two @iout according to @imax weights */
+static void max77693_calc_iout(u32 iout[2], u32 i, u32 imax[2])
+{
+ u64 t = i;
+
+ t *= imax[1];
+ do_div(t, imax[0] + imax[1]);
+
+ iout[1] = (u32)t / MAX77693_FLASH_IOUT_STEP * MAX77693_FLASH_IOUT_STEP;
+ iout[0] = i - iout[1];
+}
+
+static int max77693_set_mode(struct max77693_led *led,
+ enum max77693_led_mode mode)
+{
+ struct max77693_led_platform_data *p = led->pdata;
+ struct regmap *rmap = led->regmap;
+ int ret, v = 0;
+
+ switch (mode) {
+ case MODE_OFF:
+ break;
+ case MODE_FLASH:
+ if (p->trigger[FLASH1] & MAX77693_LED_TRIG_SOFT)
+ v |= MAX77693_FLASH_EN_ON << MAX77693_FLASH_EN1_SHIFT;
+ if (p->trigger[FLASH2] & MAX77693_LED_TRIG_SOFT)
+ v |= MAX77693_FLASH_EN_ON << MAX77693_FLASH_EN2_SHIFT;
+ break;
+ case MODE_TORCH:
+ if (p->trigger[TORCH1] & MAX77693_LED_TRIG_SOFT)
+ v |= MAX77693_FLASH_EN_ON << MAX77693_TORCH_EN1_SHIFT;
+ if (p->trigger[TORCH2] & MAX77693_LED_TRIG_SOFT)
+ v |= MAX77693_FLASH_EN_ON << MAX77693_TORCH_EN2_SHIFT;
+ break;
+ case MODE_FLASH_EXTERNAL:
+ if (p->trigger[FLASH1] & MAX77693_LED_TRIG_EXT)
+ v |= MAX77693_FLASH_EN_FLASH << MAX77693_FLASH_EN1_SHIFT;
+ if (p->trigger[FLASH2] & MAX77693_LED_TRIG_EXT)
+ v |= MAX77693_FLASH_EN_FLASH << MAX77693_FLASH_EN2_SHIFT;
+ /*
+ * Enable hw triggering also for torch mode, as some camera
+ * sensors use torch led to fathom ambient light conditions
+ * before strobing the flash.
+ */
+ if (p->trigger[TORCH1] & MAX77693_LED_TRIG_EXT)
+ v |= MAX77693_FLASH_EN_TORCH << MAX77693_TORCH_EN1_SHIFT;
+ if (p->trigger[TORCH2] & MAX77693_LED_TRIG_EXT)
+ v |= MAX77693_FLASH_EN_TORCH << MAX77693_TORCH_EN2_SHIFT;
+ break;
+ }
+ if (v != 0)
+ max77693_write_reg(rmap, MAX77693_LED_REG_FLASH_EN, 0);
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_FLASH_EN, v);
+
+ return ret;
+}
+
+static int max77693_set_current(struct max77693_led *led,
+ unsigned int milli_amp)
+{
+ struct max77693_led_platform_data *p = led->pdata;
+ struct regmap *rmap = led->regmap;
+ u32 iout[2];
+ u8 v;
+ int ret;
+
+ if (led_is_flash_mode(&led->ldev)) {
+ max77693_calc_iout(iout, 1000 * milli_amp, &p->iout[FLASH1]);
+ v = max77693_led_iout_to_reg(iout[0]);
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_IFLASH1, v);
+ if (ret < 0)
+ goto error_ret;
+ v = max77693_led_iout_to_reg(iout[1]);
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_IFLASH2, v);
+ if (ret < 0)
+ goto error_ret;
+ } else {
+ max77693_calc_iout(iout, 1000 * milli_amp, &p->iout[TORCH1]);
+ v = max77693_led_iout_to_reg(iout[0]);
+ v |= max77693_led_iout_to_reg(iout[1]) <<
+ MAX77693_TORCH_IOUT_BITS;
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_ITORCH, v);
+ if (ret < 0)
+ goto error_ret;
+ }
+
+error_ret:
+ return ret;
+}
+
+static int max77693_set_timeout(struct max77693_led *led,
+ unsigned int timeout)
+{
+ struct max77693_led_platform_data *p = led->pdata;
+ struct regmap *rmap = led->regmap;
+ u8 v;
+
+ v = max77693_flash_timeout_to_reg(timeout);
+
+ if (p->trigger_type[FLASH] == MAX77693_LED_TRIG_TYPE_LEVEL)
+ v |= MAX77693_FLASH_TIMER_LEVEL;
+
+ return max77693_write_reg(rmap, MAX77693_LED_REG_FLASH_TIMER, v);
+}
+
+static int max77693_strobe_status_get(struct max77693_led *led)
+{
+ struct regmap *rmap = led->regmap;
+ u8 v;
+ int ret;
+
+ ret = max77693_read_reg(rmap, MAX77693_LED_REG_FLASH_INT_STATUS, &v);
+ if (ret < 0)
+ return ret;
+
+ return !!(v & MAX77693_LED_STATUS_FLASH_ON);
+}
+
+static int max77693_int_flag_get(struct max77693_led *led, u8 *v)
+{
+ struct regmap *rmap = led->regmap;
+
+ return max77693_read_reg(rmap, MAX77693_LED_REG_FLASH_INT, v);
+}
+
+static int max77693_setup(struct max77693_led *led)
+{
+ struct max77693_led_platform_data *p = led->pdata;
+ struct regmap *rmap = led->regmap;
+ int ret;
+ u8 v;
+
+ v = max77693_led_iout_to_reg(p->iout[FLASH1]);
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_IFLASH1, v);
+ if (ret < 0)
+ return ret;
+
+ v = max77693_led_iout_to_reg(p->iout[FLASH2]);
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_IFLASH2, v);
+ if (ret < 0)
+ return ret;
+
+ v = max77693_led_iout_to_reg(p->iout[TORCH1]);
+ v |= max77693_led_iout_to_reg(p->iout[TORCH2]) <<
+ MAX77693_TORCH_IOUT_BITS;
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_ITORCH, v);
+ if (ret < 0)
+ return ret;
+
+ if (p->timeout[TORCH] > 0)
+ v = max77693_torch_timeout_to_reg(p->timeout[TORCH]);
+ else
+ v = MAX77693_TORCH_NO_TIMER;
+ if (p->trigger_type[TORCH] == MAX77693_LED_TRIG_TYPE_LEVEL)
+ v |= MAX77693_FLASH_TIMER_LEVEL;
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_ITORCHTIMER, v);
+ if (ret < 0)
+ return ret;
+
+ v = max77693_flash_timeout_to_reg(p->timeout[FLASH]);
+ if (p->trigger_type[FLASH] == MAX77693_LED_TRIG_TYPE_LEVEL)
+ v |= MAX77693_FLASH_TIMER_LEVEL;
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_FLASH_TIMER, v);
+ if (ret < 0)
+ return ret;
+
+ if (p->low_vsys > 0)
+ v = max77693_led_vsys_to_reg(p->low_vsys) |
+ MAX77693_FLASH_LOW_BATTERY_EN;
+ else
+ v = 0;
+
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_MAX_FLASH1, v);
+ if (ret < 0)
+ return ret;
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_MAX_FLASH2, 0);
+ if (ret < 0)
+ return ret;
+
+ if (p->boost_mode[FLASH1] == MAX77693_LED_BOOST_FIXED ||
+ p->boost_mode[FLASH2] == MAX77693_LED_BOOST_FIXED)
+ v = MAX77693_FLASH_BOOST_FIXED;
+ else
+ v = p->boost_mode[FLASH1] | (p->boost_mode[FLASH2] << 1);
+ if (p->boost_mode[FLASH1] && p->boost_mode[FLASH2])
+ v |= MAX77693_FLASH_BOOST_LEDNUM_2;
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_VOUT_CNTL, v);
+ if (ret < 0)
+ return ret;
+
+ v = max77693_led_vout_to_reg(p->boost_vout);
+ ret = max77693_write_reg(rmap, MAX77693_LED_REG_VOUT_FLASH1, v);
+ if (ret < 0)
+ return ret;
+
+ return max77693_set_mode(led, MODE_OFF);
+}
+
+/* LED subsystem callbacks */
+
+static void max77693_led_flash_mode_set(struct led_classdev *led_cdev,
+ bool flash_mode)
+{
+ struct max77693_led *led = ldev_to_led(led_cdev);
+
+ mutex_lock(&led->lock);
+
+ if (flash_mode)
+ led_cdev->max_brightness =
+ led->max_flash_intensity;
+ else
+ led_cdev->max_brightness =
+ led->max_torch_intensity;
+
+ mutex_unlock(&led->lock);
+}
+
+static void max77693_brightness_set_work(struct work_struct *work)
+{
+ struct max77693_led *led =
+ container_of(work, struct max77693_led, work_brightness_set);
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ if (led->torch_brightness == 0) {
+ ret = max77693_set_mode(led, MODE_OFF);
+ goto unlock;
+ }
+
+ ret = max77693_set_current(led, led->torch_brightness);
+ if (ret < 0)
+ goto unlock;
+
+ /*
+ * Activate FLED output here only in torch mode - in flash
+ * mode the activation occurs in the strobe_set callback.
+ */
+ if (!led_is_flash_mode(&led->ldev))
+ ret = max77693_set_mode(led, MODE_TORCH);
+
+unlock:
+ mutex_unlock(&led->lock);
+}
+
+static void max77693_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct max77693_led *led = ldev_to_led(led_cdev);
+
+ led->torch_brightness = value;
+ schedule_work(&led->work_brightness_set);
+}
+
+static enum led_brightness max77693_led_brightness_get(
+ struct led_classdev *led_cdev)
+{
+ struct max77693_led *led = ldev_to_led(led_cdev);
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ if (led_is_flash_mode(led_cdev)) {
+ ret = max77693_strobe_status_get(led);
+ if (ret < 0)
+ goto unlock;
+ ret = ret ? led_cdev->brightness : 0;
+ } else {
+ ret = led_cdev->brightness;
+ }
+
+unlock:
+ mutex_unlock(&led->lock);
+ return ret;
+}
+
+static int max77693_led_flash_fault_get(struct led_classdev *led_cdev,
+ unsigned int *fault)
+{
+ struct max77693_led *led = ldev_to_led(led_cdev);
+ u8 v;
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ ret = max77693_int_flag_get(led, &v);
+ if (ret < 0)
+ goto unlock;
+
+ *fault = 0;
+
+ if (v & MAX77693_LED_FLASH_INT_FLED2_OPEN ||
+ v & MAX77693_LED_FLASH_INT_FLED1_OPEN)
+ *fault |= LED_FAULT_OVER_VOLTAGE;
+ if (v & MAX77693_LED_FLASH_INT_FLED2_SHORT ||
+ v & MAX77693_LED_FLASH_INT_FLED1_SHORT)
+ *fault |= LED_FAULT_SHORT_CIRCUIT;
+ if (v & MAX77693_LED_FLASH_INT_OVER_CURRENT)
+ *fault |= LED_FAULT_OVER_CURRENT;
+
+unlock:
+ mutex_unlock(&led->lock);
+ return ret;
+}
+
+static void max77693_flash_strobe_set_work(struct work_struct *work)
+{
+ struct max77693_led *led =
+ container_of(work, struct max77693_led, work_strobe_set);
+ unsigned long delay_us;
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ if (led->flash_brightness == 0) {
+ ret = max77693_set_mode(led, MODE_OFF);
+ goto unlock;
+ }
+
+ delay_us = led->flash_timeout * 1000;
+ if (delay_us < MAX77693_FLASH_TIMEOUT_MIN)
+ delay_us = MAX77693_FLASH_TIMEOUT_MIN;
+
+ ret = max77693_set_mode(led, MODE_OFF);
+ if (ret < 0)
+ goto unlock;
+
+ ret = max77693_set_current(led, led->flash_brightness);
+ if (ret < 0)
+ goto unlock;
+
+ ret = max77693_set_timeout(led, delay_us);
+ if (ret < 0)
+ goto unlock;
+
+ if (led->ldev.hw_triggered)
+ ret = max77693_set_mode(led, MODE_FLASH_EXTERNAL);
+ else
+ ret = max77693_set_mode(led, MODE_FLASH);
+
+ if (ret < 0)
+ goto unlock;
+
+unlock:
+ mutex_unlock(&led->lock);
+}
+
+static void max77693_led_flash_strobe_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness,
+ unsigned long *timeout)
+{
+ struct max77693_led *led = ldev_to_led(led_cdev);
+
+ led->flash_brightness = brightness;
+ led->flash_timeout = *timeout;
+ schedule_work(&led->work_strobe_set);
+}
+
+static void max77693_led_parse_dt(struct max77693_led_platform_data *p,
+ struct device_node *node)
+{
+ of_property_read_u32_array(node, "maxim,iout", p->iout, 4);
+ of_property_read_u32_array(node, "maxim,trigger", p->trigger, 4);
+ of_property_read_u32_array(node, "maxim,trigger-type", p->trigger_type,
+ 2);
+ of_property_read_u32_array(node, "maxim,timeout", p->timeout, 2);
+ of_property_read_u32_array(node, "maxim,boost-mode", p->boost_mode, 2);
+ of_property_read_u32(node, "maxim,boost-vout", &p->boost_vout);
+ of_property_read_u32(node, "maxim,vsys-min", &p->low_vsys);
+}
+
+static void clamp_align(u32 *v, u32 min, u32 max, u32 step)
+{
+ *v = clamp_val(*v, min, max);
+ if (step > 1)
+ *v = (*v - min) / step * step + min;
+}
+
+static void max77693_led_validate_platform_data(
+ struct max77693_led_platform_data *p)
+{
+ u32 max;
+ int i;
+
+ for (i = 0; i < 2; ++i)
+ clamp_align(&p->boost_mode[i], MAX77693_LED_BOOST_NONE,
+ MAX77693_LED_BOOST_FIXED, 1);
+ /* boost, if enabled, should be the same on both leds */
+ if (p->boost_mode[0] != MAX77693_LED_BOOST_NONE &&
+ p->boost_mode[1] != MAX77693_LED_BOOST_NONE)
+ p->boost_mode[1] = p->boost_mode[0];
+
+ max = (p->boost_mode[FLASH1] && p->boost_mode[FLASH2]) ?
+ MAX77693_FLASH_IOUT_MAX_2LEDS : MAX77693_FLASH_IOUT_MAX_1LED;
+
+ clamp_align(&p->iout[FLASH1], MAX77693_FLASH_IOUT_MIN,
+ max, MAX77693_FLASH_IOUT_STEP);
+ clamp_align(&p->iout[FLASH2], MAX77693_FLASH_IOUT_MIN,
+ max, MAX77693_FLASH_IOUT_STEP);
+ clamp_align(&p->iout[TORCH1], MAX77693_TORCH_IOUT_MIN,
+ MAX77693_TORCH_IOUT_MAX, MAX77693_TORCH_IOUT_STEP);
+ clamp_align(&p->iout[TORCH2], MAX77693_TORCH_IOUT_MIN,
+ MAX77693_TORCH_IOUT_MAX, MAX77693_TORCH_IOUT_STEP);
+
+ for (i = 0; i < 4; ++i)
+ clamp_align(&p->trigger[i], 0, 7, 1);
+ for (i = 0; i < 2; ++i)
+ clamp_align(&p->trigger_type[i], MAX77693_LED_TRIG_TYPE_EDGE,
+ MAX77693_LED_TRIG_TYPE_LEVEL, 1);
+
+ clamp_align(&p->timeout[FLASH], MAX77693_FLASH_TIMEOUT_MIN,
+ MAX77693_FLASH_TIMEOUT_MAX, MAX77693_FLASH_TIMEOUT_STEP);
+
+ if (p->timeout[TORCH]) {
+ clamp_align(&p->timeout[TORCH], MAX77693_TORCH_TIMEOUT_MIN,
+ MAX77693_TORCH_TIMEOUT_MAX, 1);
+ p->timeout[TORCH] = max77693_torch_timeout_from_reg(
+ max77693_torch_timeout_to_reg(p->timeout[TORCH]));
+ }
+
+ clamp_align(&p->boost_vout, MAX77693_FLASH_VOUT_MIN,
+ MAX77693_FLASH_VOUT_MAX, MAX77693_FLASH_VOUT_STEP);
+
+ if (p->low_vsys) {
+ clamp_align(&p->low_vsys, MAX77693_FLASH_VSYS_MIN,
+ MAX77693_FLASH_VSYS_MAX, MAX77693_FLASH_VSYS_STEP);
+ }
+}
+
+static int max77693_led_get_platform_data(struct max77693_led *led)
+{
+ struct max77693_led_platform_data *p;
+ struct device *dev = &led->pdev->dev;
+
+ if (dev->of_node) {
+ p = devm_kzalloc(dev, sizeof(*led->pdata), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+ max77693_led_parse_dt(p, dev->of_node);
+ } else {
+ p = dev_get_platdata(dev);
+ if (!p)
+ return -ENODEV;
+ }
+ led->pdata = p;
+
+ max77693_led_validate_platform_data(p);
+
+ return 0;
+}
+
+#ifdef CONFIG_VIDEO_V4L2
+static void max77693_init_ctrl_config(struct v4l2_flash_ctrl_config *config,
+ void *pdata)
+{
+ struct max77693_led_platform_data *p = pdata;
+ int max;
+
+ config->flags = V4L2_FLASH_CFG_LED_FLASH |
+ V4L2_FLASH_CFG_LED_TORCH |
+ V4L2_FLASH_CFG_FAULT_OVER_VOLTAGE |
+ V4L2_FLASH_CFG_FAULT_SHORT_CIRCUIT |
+ V4L2_FLASH_CFG_FAULT_OVER_CURRENT;
+
+ config->flash_timeout.min = MAX77693_FLASH_TIMEOUT_MIN / 1000;
+ config->flash_timeout.max = MAX77693_FLASH_TIMEOUT_MAX / 1000;
+ config->flash_timeout.step = 1;
+ config->flash_timeout.def = p->timeout[FLASH] / 1000;
+
+ max = (p->iout[FLASH1] + p->iout[FLASH2]) / 1000;
+ config->flash_intensity.min = MAX77693_FLASH_IOUT_MIN / 1000;
+ config->flash_intensity.max = max;
+ config->flash_intensity.step = 1;
+ config->flash_intensity.def = max;
+
+ max = (p->iout[TORCH1] + p->iout[TORCH2]) / 1000;
+ config->torch_intensity.min = 0;
+ config->torch_intensity.max = max;
+ config->torch_intensity.step = 1;
+ config->torch_intensity.def = max;
+}
+#else
+#define max77693_init_ctrl_config(config, pdata)
+#endif
+
+static struct led_flash_ops flash_ops = {
+ .mode_set = max77693_led_flash_mode_set,
+ .strobe_set = max77693_led_flash_strobe_set,
+ .fault_get = max77693_led_flash_fault_get,
+};
+
+static int max77693_led_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct max77693_dev *iodev = dev_get_drvdata(dev->parent);
+ struct max77693_led *led;
+ struct max77693_led_platform_data *p;
+ struct led_classdev *led_dev;
+ struct v4l2_flash_ctrl_config config;
+ int ret;
+
+ led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ led->pdev = pdev;
+ led->regmap = iodev->regmap;
+ platform_set_drvdata(pdev, led);
+ ret = max77693_led_get_platform_data(led);
+ if (ret < 0)
+ return -EINVAL;
+ p = led->pdata;
+
+ if (p->boost_mode[0] != MAX77693_LED_BOOST_NONE &&
+ p->boost_mode[1] != MAX77693_LED_BOOST_NONE) {
+ led->max_flash_intensity =
+ MAX77693_FLASH_IOUT_MAX_2LEDS * 2 / 1000;
+ led->max_torch_intensity = MAX77693_TORCH_IOUT_MAX * 2 / 1000;
+ } else {
+ led->max_flash_intensity = MAX77693_FLASH_IOUT_MAX_1LED / 1000;
+ led->max_torch_intensity = MAX77693_TORCH_IOUT_MAX / 1000;
+ }
+
+ INIT_WORK(&led->work_brightness_set, max77693_brightness_set_work);
+ INIT_WORK(&led->work_strobe_set, max77693_flash_strobe_set_work);
+
+ mutex_init(&led->lock);
+
+ /* register in the LED subsystem */
+ led_dev = &led->ldev;
+
+ led_dev->name = MAX77693_LED_NAME;
+ led->ldev.brightness_set = max77693_led_brightness_set;
+ led->ldev.brightness_get = max77693_led_brightness_get;
+ led->ldev.max_brightness = led->max_torch_intensity;
+
+ if ((p->trigger[FLASH1] & MAX77693_LED_TRIG_FLASH) ||
+ (p->trigger[FLASH2] & MAX77693_LED_TRIG_FLASH))
+ led_dev->has_hw_trig = true;
+
+ led_classdev_init_flash(led_dev);
+
+ led_dev->flash->ops = flash_ops;
+ led_dev->flash->max_timeout = MAX77693_FLASH_TIMEOUT_MAX / 1000;
+
+ ret = led_classdev_register(&pdev->dev, led_dev);
+ if (ret < 0)
+ return -EINVAL;
+
+ /* Initialize V4L2 Flash control configuration data. */
+ max77693_init_ctrl_config(&config, p);
+
+ /* Create V4L2 Flash subdev. */
+ ret = v4l2_flash_init(&led->ldev, &led->v4l2_flash, &config);
+ if (ret < 0)
+ goto error_flash_init;
+
+ ret = max77693_setup(led);
+
+ return ret;
+
+error_flash_init:
+ led_classdev_unregister(&led->ldev);
+ return ret;
+}
+
+static int max77693_led_remove(struct platform_device *pdev)
+{
+ struct max77693_led *led = platform_get_drvdata(pdev);
+
+ v4l2_flash_release(&led->v4l2_flash);
+ led_classdev_unregister(&led->ldev);
+
+ return 0;
+}
+
+static struct of_device_id max77693_led_dt_match[] = {
+ {.compatible = "maxim,max77693-led"},
+ {},
+};
+
+static struct platform_driver max77693_led_driver = {
+ .probe = max77693_led_probe,
+ .remove = max77693_led_remove,
+ .driver = {
+ .name = "max77693-led",
+ .owner = THIS_MODULE,
+ .of_match_table = max77693_led_dt_match,
+ },
+};
+
+module_platform_driver(max77693_led_driver);
+
+MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
+MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
+MODULE_DESCRIPTION("Maxim MAX77693 led flash driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index c5535f0..6fa92d3 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -41,12 +41,21 @@
#define I2C_ADDR_MUIC (0x4A >> 1)
#define I2C_ADDR_HAPTIC (0x90 >> 1)
-static const struct mfd_cell max77693_devs[] = {
- { .name = "max77693-pmic", },
- { .name = "max77693-charger", },
- { .name = "max77693-flash", },
- { .name = "max77693-muic", },
- { .name = "max77693-haptic", },
+enum mfd_devs_idx {
+ IDX_PMIC,
+ IDX_CHARGER,
+ IDX_LED,
+ IDX_MUIC,
+ IDX_HAPTIC,
+};
+
+static struct mfd_cell max77693_devs[] = {
+ [IDX_PMIC] = { .name = "max77693-pmic", },
+ [IDX_CHARGER] = { .name = "max77693-charger", },
+ [IDX_LED] = { .name = "max77693-led",
+ .of_compatible = "maxim,max77693-led"},
+ [IDX_MUIC] = { .name = "max77693-muic", },
+ [IDX_HAPTIC] = { .name = "max77693-haptic", },
};
int max77693_read_reg(struct regmap *map, u8 reg, u8 *dest)
diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
index 3f3dc45..5859698 100644
--- a/include/linux/mfd/max77693.h
+++ b/include/linux/mfd/max77693.h
@@ -63,6 +63,37 @@ struct max77693_muic_platform_data {
int path_uart;
};
+/* MAX77693 led flash */
+
+/* triggers */
+#define MAX77693_LED_TRIG_OFF 0
+#define MAX77693_LED_TRIG_FLASH 1
+#define MAX77693_LED_TRIG_TORCH 2
+#define MAX77693_LED_TRIG_EXT (MAX77693_LED_TRIG_FLASH |\
+ MAX77693_LED_TRIG_TORCH)
+#define MAX77693_LED_TRIG_SOFT 4
+
+/* trigger types */
+#define MAX77693_LED_TRIG_TYPE_EDGE 0
+#define MAX77693_LED_TRIG_TYPE_LEVEL 1
+
+/* boost modes */
+#define MAX77693_LED_BOOST_NONE 0
+#define MAX77693_LED_BOOST_ADAPTIVE 1
+#define MAX77693_LED_BOOST_FIXED 2
+
+struct max77693_led_platform_data {
+ u32 iout[4];
+ u32 trigger[4];
+ u32 trigger_type[2];
+ u32 timeout[2];
+ u32 boost_mode[2];
+ u32 boost_vout;
+ u32 low_vsys;
+};
+
+/* MAX77693 */
+
struct max77693_platform_data {
/* regulator data */
struct max77693_regulator_data *regulators;
@@ -70,5 +101,6 @@ struct max77693_platform_data {
/* muic data */
struct max77693_muic_platform_data *muic_data;
+ struct max77693_led_platform_data *led_data;
};
#endif /* __LINUX_MFD_MAX77693_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell
2014-03-20 14:51 ` [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell Jacek Anaszewski
@ 2014-03-20 15:34 ` Lee Jones
2014-03-21 8:22 ` Jacek Anaszewski
0 siblings, 1 reply; 25+ messages in thread
From: Lee Jones @ 2014-03-20 15:34 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie, SangYoung Son,
Samuel Ortiz
On Thu, 20 Mar 2014, Jacek Anaszewski wrote:
> This patch adds led-flash support to Maxim max77693 chipset.
> Device can be exposed to user space through LED subsystem
> sysfs interface or through V4L2 subdevice when the support
> for Multimedia Framework is enabled. Device supports up to
> two leds which can work in flash and torch mode. Leds can
> be triggered externally or by software.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> 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>
> Cc: SangYoung Son <hello.son@smasung.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/leds/Kconfig | 9 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-max77693.c | 768 ++++++++++++++++++++++++++++++++++++++++++
> drivers/mfd/max77693.c | 21 +-
> include/linux/mfd/max77693.h | 32 ++
> 5 files changed, 825 insertions(+), 6 deletions(-)
> create mode 100644 drivers/leds/leds-max77693.c
[...]
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index c5535f0..6fa92d3 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -41,12 +41,21 @@
> #define I2C_ADDR_MUIC (0x4A >> 1)
> #define I2C_ADDR_HAPTIC (0x90 >> 1)
>
> -static const struct mfd_cell max77693_devs[] = {
> - { .name = "max77693-pmic", },
> - { .name = "max77693-charger", },
> - { .name = "max77693-flash", },
> - { .name = "max77693-muic", },
> - { .name = "max77693-haptic", },
> +enum mfd_devs_idx {
> + IDX_PMIC,
> + IDX_CHARGER,
> + IDX_LED,
> + IDX_MUIC,
> + IDX_HAPTIC,
> +};
> +
> +static struct mfd_cell max77693_devs[] = {
> + [IDX_PMIC] = { .name = "max77693-pmic", },
> + [IDX_CHARGER] = { .name = "max77693-charger", },
> + [IDX_LED] = { .name = "max77693-led",
> + .of_compatible = "maxim,max77693-led"},
> + [IDX_MUIC] = { .name = "max77693-muic", },
> + [IDX_HAPTIC] = { .name = "max77693-haptic", },
> };
What is the purpose of this change?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell
2014-03-20 15:34 ` Lee Jones
@ 2014-03-21 8:22 ` Jacek Anaszewski
2014-03-21 9:36 ` Lee Jones
0 siblings, 1 reply; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-21 8:22 UTC (permalink / raw)
To: Lee Jones
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie, SangYoung Son,
Samuel Ortiz
On 03/20/2014 04:34 PM, Lee Jones wrote:
> On Thu, 20 Mar 2014, Jacek Anaszewski wrote:
>
>> This patch adds led-flash support to Maxim max77693 chipset.
>> Device can be exposed to user space through LED subsystem
>> sysfs interface or through V4L2 subdevice when the support
>> for Multimedia Framework is enabled. Device supports up to
>> two leds which can work in flash and torch mode. Leds can
>> be triggered externally or by software.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> 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>
>> Cc: SangYoung Son <hello.son@smasung.com>
>> Cc: Samuel Ortiz <sameo@linux.intel.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> ---
>> drivers/leds/Kconfig | 9 +
>> drivers/leds/Makefile | 1 +
>> drivers/leds/leds-max77693.c | 768 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/mfd/max77693.c | 21 +-
>> include/linux/mfd/max77693.h | 32 ++
>> 5 files changed, 825 insertions(+), 6 deletions(-)
>> create mode 100644 drivers/leds/leds-max77693.c
>
> [...]
>
>> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
>> index c5535f0..6fa92d3 100644
>> --- a/drivers/mfd/max77693.c
>> +++ b/drivers/mfd/max77693.c
>> @@ -41,12 +41,21 @@
>> #define I2C_ADDR_MUIC (0x4A >> 1)
>> #define I2C_ADDR_HAPTIC (0x90 >> 1)
>>
>> -static const struct mfd_cell max77693_devs[] = {
>> - { .name = "max77693-pmic", },
>> - { .name = "max77693-charger", },
>> - { .name = "max77693-flash", },
>> - { .name = "max77693-muic", },
>> - { .name = "max77693-haptic", },
>> +enum mfd_devs_idx {
>> + IDX_PMIC,
>> + IDX_CHARGER,
>> + IDX_LED,
>> + IDX_MUIC,
>> + IDX_HAPTIC,
>> +};
>> +
>> +static struct mfd_cell max77693_devs[] = {
>> + [IDX_PMIC] = { .name = "max77693-pmic", },
>> + [IDX_CHARGER] = { .name = "max77693-charger", },
>> + [IDX_LED] = { .name = "max77693-led",
>> + .of_compatible = "maxim,max77693-led"},
>> + [IDX_MUIC] = { .name = "max77693-muic", },
>> + [IDX_HAPTIC] = { .name = "max77693-haptic", },
>> };
>
> What is the purpose of this change?
>
Introducing mfd_devs_idx itself is a cosmetic change, which
actually could be avoided. Initialization of the of_compatible field
is required for the led driver to get matched properly. And as I've
just realized also max77693-flash name should be preserved.
I will fix this in the next version of the patch.
Thanks,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell
2014-03-21 8:22 ` Jacek Anaszewski
@ 2014-03-21 9:36 ` Lee Jones
0 siblings, 0 replies; 25+ messages in thread
From: Lee Jones @ 2014-03-21 9:36 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Bryan Wu, Richard Purdie, SangYoung Son,
Samuel Ortiz
> >>This patch adds led-flash support to Maxim max77693 chipset.
> >>Device can be exposed to user space through LED subsystem
> >>sysfs interface or through V4L2 subdevice when the support
> >>for Multimedia Framework is enabled. Device supports up to
> >>two leds which can work in flash and torch mode. Leds can
> >>be triggered externally or by software.
> >>
> >>Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> >>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>
> >>Cc: SangYoung Son <hello.son@smasung.com>
> >>Cc: Samuel Ortiz <sameo@linux.intel.com>
> >>Cc: Lee Jones <lee.jones@linaro.org>
> >>---
> >> drivers/leds/Kconfig | 9 +
> >> drivers/leds/Makefile | 1 +
> >> drivers/leds/leds-max77693.c | 768 ++++++++++++++++++++++++++++++++++++++++++
> >> drivers/mfd/max77693.c | 21 +-
> >> include/linux/mfd/max77693.h | 32 ++
> >> 5 files changed, 825 insertions(+), 6 deletions(-)
> >> create mode 100644 drivers/leds/leds-max77693.c
> >
> >[...]
> >>-static const struct mfd_cell max77693_devs[] = {
> >>- { .name = "max77693-pmic", },
> >>- { .name = "max77693-charger", },
> >>- { .name = "max77693-flash", },
> >>- { .name = "max77693-muic", },
> >>- { .name = "max77693-haptic", },
> >>+enum mfd_devs_idx {
> >>+ IDX_PMIC,
> >>+ IDX_CHARGER,
> >>+ IDX_LED,
> >>+ IDX_MUIC,
> >>+ IDX_HAPTIC,
> >>+};
> >>+
> >>+static struct mfd_cell max77693_devs[] = {
> >>+ [IDX_PMIC] = { .name = "max77693-pmic", },
> >>+ [IDX_CHARGER] = { .name = "max77693-charger", },
> >>+ [IDX_LED] = { .name = "max77693-led",
> >>+ .of_compatible = "maxim,max77693-led"},
> >>+ [IDX_MUIC] = { .name = "max77693-muic", },
> >>+ [IDX_HAPTIC] = { .name = "max77693-haptic", },
> >> };
> >
> >What is the purpose of this change?
> >
> Introducing mfd_devs_idx itself is a cosmetic change, which
> actually could be avoided. Initialization of the of_compatible field
> is required for the led driver to get matched properly. And as I've
> just realized also max77693-flash name should be preserved.
> I will fix this in the next version of the patch.
I'm happy with the addition of any .of_compatible strings, however
please leave out the IDXs in your next version(s).
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH/RFC 7/8] DT: Add documentation for the mfd Maxim max77693 flash cell
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
` (5 preceding siblings ...)
2014-03-20 14:51 ` [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
[not found] ` <1395327070-20215-1-git-send-email-j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
7 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala
This patch adds device tree binding documentation for
the flash cell of the Maxim max77693 multifunctional device.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
Documentation/devicetree/bindings/mfd/max77693.txt | 47 ++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
index 11921cc..9e1aa97 100644
--- a/Documentation/devicetree/bindings/mfd/max77693.txt
+++ b/Documentation/devicetree/bindings/mfd/max77693.txt
@@ -27,6 +27,43 @@ Optional properties:
[*] refer Documentation/devicetree/bindings/regulator/regulator.txt
+Optional node:
+- led-flash : the LED submodule device node
+
+Required properties of "led-flash" node:
+- compatible : must be "maxim,max77693-led"
+
+Optional properties of "led-flash" node:
+- maxim,iout : Array of four intensities in microampheres of the current
+ in order: flash1, flash2, torch1, torch2.
+ Range:
+ flash - 15625 - 1000000,
+ torch - 15625 - 250000.
+- maxim,trigger : Array of flags indicating which trigger can activate given led
+ in order: flash1, flash2, torch1, torch2.
+ Possible flag values (can be combined):
+ 1 - FLASH pin of the chip,
+ 2 - TORCH pin of the chip,
+ 4 - software via I2C command.
+- maxim,trigger-type : Array of trigger types in order: flash, torch.
+ Possible trigger types:
+ 0 - Rising edge of the signal triggers the flash/torch,
+ 1 - Signal level controls duration of the flash/torch.
+- maxim,timeout : Array of timeouts in microseconds after which leds are
+ turned off in order: flash, torch.
+ Range:
+ flash: 62500 - 1000000,
+ torch: 0 (no timeout) - 15728000.
+- maxim,boost-mode : Array of the flash boost modes in order: flash1, flash2.
+ Possible values:
+ 0 - no boost,
+ 1 - adaptive mode,
+ 2 - fixed mode.
+- maxim,boost-vout : Output voltage of the boost module in millivolts.
+- maxim,vsys-min : Low input voltage level in millivolts. Flash is not fired
+ if chip estimates that system voltage could drop below this level due
+ to flash power consumption.
+
Example:
max77693@66 {
compatible = "maxim,max77693";
@@ -52,4 +89,14 @@ Example:
regulator-boot-on;
};
};
+ led_flash: led-flash {
+ compatible = "maxim,max77693-led";
+ maxim,iout = <625000 625000 250000 250000>;
+ maxim,trigger = <5 5 6 0>;
+ maxim,trigger-type = <0 1>;
+ maxim,timeout = <500000 0>;
+ maxim,boost-mode = <1 1>;
+ maxim,boost-vout = <5000>;
+ maxim,vsys-min = <2400>;
+ };
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread[parent not found: <1395327070-20215-1-git-send-email-j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* [PATCH/RFC 8/8] DT: Add documentation for exynos4-is camera-flash property
2014-03-20 14:51 [PATCH/RFC 0/8] LED / flash API integration Jacek Anaszewski
@ 2014-03-20 14:51 ` Jacek Anaszewski
2014-03-20 14:51 ` [PATCH/RFC 2/8] leds: Improve and export led_update_brightness function Jacek Anaszewski
` (6 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, Jacek Anaszewski,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala
Signed-off-by: Jacek Anaszewski <j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
.../devicetree/bindings/media/samsung-fimc.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..88f9287 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -108,6 +108,8 @@ Image sensor nodes
The sensor device nodes should be added to their control bus controller (e.g.
I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
using the common video interfaces bindings, defined in video-interfaces.txt.
+If the sensor device has a led flash device associated with it then its phandle
+should be assigned to the camera-flash property.
Example:
@@ -125,6 +127,7 @@ Example:
clock-frequency = <24000000>;
clocks = <&camera 1>;
clock-names = "mclk";
+ camera-flash = <&led_flash>;
port {
s5k6aa_ep: endpoint {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH/RFC 8/8] DT: Add documentation for exynos4-is camera-flash property
@ 2014-03-20 14:51 ` Jacek Anaszewski
0 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-20 14:51 UTC (permalink / raw)
To: linux-media, linux-leds, devicetree, linux-kernel
Cc: s.nawrocki, a.hajda, kyungmin.park, Jacek Anaszewski,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
.../devicetree/bindings/media/samsung-fimc.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..88f9287 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -108,6 +108,8 @@ Image sensor nodes
The sensor device nodes should be added to their control bus controller (e.g.
I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
using the common video interfaces bindings, defined in video-interfaces.txt.
+If the sensor device has a led flash device associated with it then its phandle
+should be assigned to the camera-flash property.
Example:
@@ -125,6 +127,7 @@ Example:
clock-frequency = <24000000>;
clocks = <&camera 1>;
clock-names = "mclk";
+ camera-flash = <&led_flash>;
port {
s5k6aa_ep: endpoint {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 8/8] DT: Add documentation for exynos4-is camera-flash property
2014-03-20 14:51 ` Jacek Anaszewski
(?)
@ 2014-03-24 1:05 ` Sakari Ailus
2014-03-28 15:31 ` Jacek Anaszewski
-1 siblings, 1 reply; 25+ messages in thread
From: Sakari Ailus @ 2014-03-24 1:05 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala
Hi Jacek,
On Thu, Mar 20, 2014 at 03:51:10PM +0100, Jacek Anaszewski wrote:
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> ---
> .../devicetree/bindings/media/samsung-fimc.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> index 922d6f8..88f9287 100644
> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> @@ -108,6 +108,8 @@ Image sensor nodes
> The sensor device nodes should be added to their control bus controller (e.g.
> I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
> using the common video interfaces bindings, defined in video-interfaces.txt.
> +If the sensor device has a led flash device associated with it then its phandle
> +should be assigned to the camera-flash property.
>
> Example:
>
> @@ -125,6 +127,7 @@ Example:
> clock-frequency = <24000000>;
> clocks = <&camera 1>;
> clock-names = "mclk";
> + camera-flash = <&led_flash>;
>
> port {
> s5k6aa_ep: endpoint {
It's indeed an interesting idea to declare the flash controller in the
sensor's properties rather than those of the ISP. The obvious upside is that
this way it's easy to figure out which subdev group the flash controller
belongs to.
There are a few other things to consider as well:
- You can't have a flash without a sensor. I can't think of why this would
be a real issue, though.
- Relations other than one-to-one become difficult. One flash but two
cameras --- think of stereo cameras.
- One camera and two flashes. I haven't seen any but I don't think
that's unthinkable.
- It's not very nice of the ISP driver to just go and parse the
sensor's properties.
- As the property is FIMC specific, the sensor DT node now carries FIMC
related information.
A generic solution would be preferrable as this is not a FIMC related
problem.
I have to admit that I can't think of a better solution right now than just
putting a list of the flash device phandles to the ISP device's DT node, and
then adding information on which sensor (numeric ID) the flash is related to
as an array. Better ideas would be welcome.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH/RFC 8/8] DT: Add documentation for exynos4-is camera-flash property
2014-03-24 1:05 ` Sakari Ailus
@ 2014-03-28 15:31 ` Jacek Anaszewski
0 siblings, 0 replies; 25+ messages in thread
From: Jacek Anaszewski @ 2014-03-28 15:31 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, linux-leds, devicetree, linux-kernel, s.nawrocki,
a.hajda, kyungmin.park, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala
Hi Sakari,
On 03/24/2014 02:05 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Thu, Mar 20, 2014 at 03:51:10PM +0100, Jacek Anaszewski wrote:
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>> Cc: Kumar Gala <galak@codeaurora.org>
>> ---
>> .../devicetree/bindings/media/samsung-fimc.txt | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> index 922d6f8..88f9287 100644
>> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> @@ -108,6 +108,8 @@ Image sensor nodes
>> The sensor device nodes should be added to their control bus controller (e.g.
>> I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
>> using the common video interfaces bindings, defined in video-interfaces.txt.
>> +If the sensor device has a led flash device associated with it then its phandle
>> +should be assigned to the camera-flash property.
>>
>> Example:
>>
>> @@ -125,6 +127,7 @@ Example:
>> clock-frequency = <24000000>;
>> clocks = <&camera 1>;
>> clock-names = "mclk";
>> + camera-flash = <&led_flash>;
>>
>> port {
>> s5k6aa_ep: endpoint {
>
> It's indeed an interesting idea to declare the flash controller in the
> sensor's properties rather than those of the ISP. The obvious upside is that
> this way it's easy to figure out which subdev group the flash controller
> belongs to.
>
> There are a few other things to consider as well:
>
> - You can't have a flash without a sensor. I can't think of why this would
> be a real issue, though.
>
> - Relations other than one-to-one become difficult. One flash but two
> cameras --- think of stereo cameras.
>
> - One camera and two flashes. I haven't seen any but I don't think
> that's unthinkable.
>
> - It's not very nice of the ISP driver to just go and parse the
> sensor's properties.
>
> - As the property is FIMC specific, the sensor DT node now carries FIMC
> related information.
>
> A generic solution would be preferrable as this is not a FIMC related
> problem.
>
> I have to admit that I can't think of a better solution right now than just
> putting a list of the flash device phandles to the ISP device's DT node, and
> then adding information on which sensor (numeric ID) the flash is related to
> as an array. Better ideas would be welcome.
>
One reason why the flash sub-dev is registered by the sensor is the
fact that a subdev has to be registered to make it available for use.
The second reason is that it is physically connected with the sensor on
the board via torchen/flashen traces. However it would be nice if the
flash could be available for use even if its parent sensor driver isn't
probed.
There are also possible configurations where traces are routed through
multiplexers and in such cases the sensor-flash relation is not fixed.
I propose to introduce a "flash manager" which would maintain
all the available flashes. V4L2 Flash sub-devices could register
with it asynchronously. The flash manager could expose controls
for configuring flash-sensor relations. In specific case a flash
manager could be built upon a multiplexer device. This would suit
me very well as I am currently facing such a configuration
on another board. For time being I put gpios of a multiplexer
to the flash DT node, but it doesn't reflect board configuration,
where multiplexer is a separate device. I could try to implement
the flash manager and submit an RFC. What is your opinion, does
it make a sense?
Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 25+ messages in thread