[RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Andrzej Hajda]

This RFC proposes generic API for exposing flash subdevices via LED framework.

Rationale

Currently there are two frameworks which are used for exposing LED flash to
user space:
- V4L2 flash controls,
- LED framework(with custom sysfs attributes).

The list below shows flash drivers in mainline kernel with initial commit date
and typical chip application (according to producer):

LED API:
    lm3642: 2012-09-12, Cameras
    lm355x: 2012-09-05, Cameras
    max8997: 2011-12-14, Cameras (?)
    lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
    pca955x: 2008-07-16, Cameras, Indicators (?)
V4L2 API:
    as3645a:  2011-05-05, Cameras
    adp1653: 2011-05-05, Cameras

V4L2 provides richest functionality, but there is often demand from application
developers to provide already established LED API.
We would like to have an unified user interface for flash devices. Some of
devices already have the LED API driver exposing limited set of a Flash IC
functionality. In order to support all required features the LED API would
have to be extended or the V4L2 API would need to be used. However when
switching from a LED to a V4L2 Flash driver existing LED API interface would
need to be retained.

Proposed solution

This patch adds V4L2 helper functions to register existing V4L2 flash subdev
as LED class device.
After registration via v4l2_leddev_register appropriate entry in
/sys/class/leds/ is created.
During registration all V4L2 flash controls are enumerated and corresponding
attributes are added.

I have attached also patch with new max77693-led driver using v4l2_leddev.
This patch requires presence of the patch "max77693: added device tree support":
https://patchwork.kernel.org/patch/2414351/ .

Additional features

- simple API to access all V4L2 flash controls via sysfs,
- V4L2 subdevice should not be registered by V4L2 device to use it,
- LED triggers API can be used to control the device,
- LED device is optional - it will be created only if V4L2_LEDDEV configuration
  option is enabled and the subdev driver calls v4l2_leddev_register.

Doubts

This RFC is a result of a uncertainty which API developers should expose by
their flash drivers. It is a try to gluing together both APIs.
I am not sure if it is the best solution, but I hope there will be some
discussion and hopefully some decisions will be taken which way we should follow.

Regards
Andrzej Hajda

Andrzej Hajda (2):
  v4l2-leddev: added LED class support for V4L2 flash subdevices
  media: added max77693-led driver

 drivers/media/i2c/max77693-led.c      |  650 +++++++++++++++++++++++++++++++++
 drivers/media/v4l2-core/Kconfig       |    7 +
 drivers/media/v4l2-core/Makefile      |    1 +
 drivers/media/v4l2-core/v4l2-leddev.c |  269 ++++++++++++++
 include/media/v4l2-leddev.h           |   49 +++
 5 files changed, 976 insertions(+)
 create mode 100644 drivers/media/i2c/max77693-led.c
 create mode 100644 drivers/media/v4l2-core/v4l2-leddev.c
 create mode 100644 include/media/v4l2-leddev.h

-- 
1.7.10.4

[PATCH RFC 1/2] v4l2-leddev: added LED class support for V4L2 flash subdevices

[Andrzej Hajda]

This patch adds helper functions for exposing V4L2 flash
subdevice as LED class device. For such device there will be
created appropriate entry in <sysfs>/class/leds/ directory with
standard LED class attributes and attributes corresponding
to V4L2 flash controls exposed by the subdevice.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/v4l2-core/Kconfig       |    7 +
 drivers/media/v4l2-core/Makefile      |    1 +
 drivers/media/v4l2-core/v4l2-leddev.c |  269 +++++++++++++++++++++++++++++++++
 include/media/v4l2-leddev.h           |   49 ++++++
 4 files changed, 326 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-leddev.c
 create mode 100644 include/media/v4l2-leddev.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 8c05565..c32b7f2 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -35,6 +35,13 @@ config V4L2_MEM2MEM_DEV
         tristate
         depends on VIDEOBUF2_CORE
 
+# Used by drivers that need v4l2-leddev.ko
+config V4L2_LEDDEV
+	tristate "LED support for V4L2 flash subdevices"
+	depends on VIDEO_V4L2 && LEDS_CLASS
+	---help---
+	  This option enables LED class support for V4L2 flash devices.
+
 # Used by drivers that need Videobuf modules
 config VIDEOBUF_GEN
 	tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index aa50c46..2906c7c 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
 obj-$(CONFIG_VIDEO_TUNER) += tuner.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
+obj-$(CONFIG_V4L2_LEDDEV) += v4l2-leddev.o
 
 obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
 obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
diff --git a/drivers/media/v4l2-core/v4l2-leddev.c b/drivers/media/v4l2-core/v4l2-leddev.c
new file mode 100644
index 0000000..f41885e
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-leddev.c
@@ -0,0 +1,269 @@
+/*
+ * V4L2 API for exposing flash subdevs as LED class devices
+ *
+ * Copyright (C) 2013, Samsung Electronics Co., Ltd.
+ * Andrzej Hajda <a.hajda@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.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/sysfs.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-leddev.h>
+
+struct v4l2_leddev_attr {
+	u32 ctrl_id;
+	u8 name[32];
+	struct device_attribute dattr;
+};
+
+struct v4l2_leddev_attr *to_v4l2_leddev_attr(struct device_attribute *da)
+{
+	return container_of(da, struct v4l2_leddev_attr, dattr);
+}
+
+struct v4l2_leddev *to_v4l2_leddev(struct led_classdev *cdev)
+{
+	return container_of(cdev, struct v4l2_leddev, cdev);
+}
+
+static void v4l2_leddev_brightness_set(struct led_classdev *cdev,
+				       enum led_brightness value)
+{
+	struct v4l2_ext_control ctrls[2] = {
+		{
+			.id = V4L2_CID_FLASH_LED_MODE,
+			.value = value ? V4L2_FLASH_LED_MODE_TORCH
+				       : V4L2_FLASH_LED_MODE_NONE,
+		},
+		{
+			.id = V4L2_CID_FLASH_TORCH_INTENSITY,
+			.value = value
+		}
+
+	};
+	struct v4l2_ext_controls ext_ctrls = {
+		.ctrl_class = V4L2_CTRL_CLASS_FLASH,
+		.controls = ctrls,
+		.count = value ? 2 : 1,
+	};
+	struct v4l2_leddev *ld = to_v4l2_leddev(cdev);
+
+	v4l2_subdev_s_ext_ctrls(ld->sd, &ext_ctrls);
+}
+
+static enum led_brightness v4l2_leddev_brightness_get(struct led_classdev *cdev)
+{
+	struct v4l2_ext_control ctrls[2] = {
+		{
+			.id = V4L2_CID_FLASH_LED_MODE,
+		},
+		{
+			.id = V4L2_CID_FLASH_TORCH_INTENSITY,
+		},
+
+	};
+	struct v4l2_ext_controls ext_ctrls = {
+		.ctrl_class = V4L2_CTRL_CLASS_FLASH,
+		.controls = ctrls,
+		.count = 2,
+	};
+	struct v4l2_leddev *ld = to_v4l2_leddev(cdev);
+	int ret;
+
+	ret = v4l2_subdev_g_ext_ctrls(ld->sd, &ext_ctrls);
+
+	if (ret || ctrls[0].value != V4L2_FLASH_LED_MODE_TORCH)
+		return 0;
+
+	return ctrls[1].value;
+}
+
+static ssize_t v4l2_leddev_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct v4l2_leddev *ld = to_v4l2_leddev(cdev);
+	struct v4l2_leddev_attr *ldattr = to_v4l2_leddev_attr(attr);
+	struct v4l2_ext_control ctrl = {
+		.id = ldattr->ctrl_id,
+	};
+	struct v4l2_ext_controls ext_ctrls = {
+		.ctrl_class = V4L2_CTRL_CLASS_FLASH,
+		.controls = &ctrl,
+		.count = 1,
+	};
+	int ret;
+
+	ret = v4l2_subdev_g_ext_ctrls(ld->sd, &ext_ctrls);
+	if (ret)
+		return 0;
+
+	return sprintf(buf, "%d\n", ctrl.value);
+}
+
+static ssize_t v4l2_leddev_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct v4l2_leddev *ld = to_v4l2_leddev(cdev);
+	struct v4l2_leddev_attr *ldattr = to_v4l2_leddev_attr(attr);
+	struct v4l2_ext_control ctrl = {
+		.id = ldattr->ctrl_id,
+	};
+	struct v4l2_ext_controls ext_ctrls = {
+		.ctrl_class = V4L2_CTRL_CLASS_FLASH,
+		.controls = &ctrl,
+		.count = 1,
+	};
+	int ret;
+
+	ret = kstrtos32(buf, 0, &ctrl.value);
+	if (!ret)
+		ret = v4l2_subdev_s_ext_ctrls(ld->sd, &ext_ctrls);
+
+	return ret ? 0 : size;
+}
+
+static int v4l2_leddev_ctrls_count(struct v4l2_leddev *ld)
+{
+	struct v4l2_queryctrl qc = {
+		.id = V4L2_CID_FLASH_CLASS | V4L2_CTRL_FLAG_NEXT_CTRL,
+	};
+	int n = 0;
+
+	while (v4l2_queryctrl(ld->sd->ctrl_handler, &qc) == 0) {
+		if (V4L2_CTRL_ID2CLASS (qc.id) != V4L2_CTRL_CLASS_FLASH)
+			break;
+		++n;
+		qc.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
+	}
+	return n;
+}
+
+static int v4l2_leddev_create_attrs(struct v4l2_leddev *ld)
+{
+	struct v4l2_queryctrl qc = {
+		.id = V4L2_CID_FLASH_CLASS | V4L2_CTRL_FLAG_NEXT_CTRL,
+	};
+	int i = 0;
+	int ret;
+
+	ld->attr_count = v4l2_leddev_ctrls_count(ld);
+	ld->attrs = kzalloc(sizeof(*ld->attrs) * ld->attr_count, GFP_KERNEL);
+	if (!ld->attrs)
+		return -ENOMEM;
+
+	while (v4l2_queryctrl(ld->sd->ctrl_handler, &qc) == 0) {
+		struct v4l2_leddev_attr *attr = &ld->attrs[i];
+		if (V4L2_CTRL_ID2CLASS (qc.id) != V4L2_CTRL_CLASS_FLASH)
+			break;
+
+		attr->ctrl_id = qc.id;
+		strncpy(attr->name, qc.name, sizeof(attr->name));
+		attr->dattr.attr.name = attr->name;
+		if (!(qc.flags & V4L2_CTRL_FLAG_READ_ONLY)) {
+			attr->dattr.store = v4l2_leddev_store;
+			attr->dattr.attr.mode |= 0220;
+		}
+		if (!(qc.flags & V4L2_CTRL_FLAG_WRITE_ONLY)) {
+			attr->dattr.show = v4l2_leddev_show;
+			attr->dattr.attr.mode |= 0444;
+		}
+		v4l2_info(ld->sd, "creating attr %s(%d/%d)\n", attr->name, i,
+			  ld->attr_count);
+		ret = device_create_file(ld->cdev.dev, &attr->dattr);
+		if (!ret) {
+			if (++i == ld->attr_count)
+				break;
+		} else {
+			v4l2_err(ld->sd, "error creating attr %s (%d)\n",
+				 attr->name, ret);
+		}
+
+		qc.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
+	}
+	ld->attr_count = i;
+	return 0;
+}
+
+static void v4l2_leddev_remove_attrs(struct v4l2_leddev *ld)
+{
+	int i;
+
+	for (i = ld->attr_count - 1; i >= 0; --i)
+		device_remove_file(ld->cdev.dev, &ld->attrs[i].dattr);
+
+	kfree(ld->attrs);
+	ld->attrs = NULL;
+	ld->attr_count = 0;
+}
+
+static int v4l2_leddev_get_max_brightness(struct v4l2_leddev *ld)
+{
+	struct v4l2_queryctrl qc = {
+		.id = V4L2_CID_FLASH_TORCH_INTENSITY,
+	};
+	int ret;
+
+	ret = v4l2_queryctrl(ld->sd->ctrl_handler, &qc);
+	if (ret) {
+		v4l2_err(ld->sd, "cannot query torch intensity (%d)\n", ret);
+		return 0;
+	}
+	return qc.maximum;
+}
+
+int v4l2_leddev_register(struct device *dev, struct v4l2_leddev *ld)
+{
+	int ret;
+
+	if (!ld->sd) {
+		dev_err(dev, "cannot register leddev without subdev provided\n");
+		return -EINVAL;
+	}
+	if (!ld->cdev.name)
+		ld->cdev.name = ld->sd->name;
+	if (!ld->cdev.max_brightness)
+		ld->cdev.max_brightness = v4l2_leddev_get_max_brightness(ld);
+	if (!ld->cdev.brightness_set)
+		ld->cdev.brightness_set = v4l2_leddev_brightness_set;
+	if (!ld->cdev.brightness_get)
+		ld->cdev.brightness_get = v4l2_leddev_brightness_get;
+
+	ret = led_classdev_register(dev, &ld->cdev);
+	if (ret < 0)
+		return ret;
+
+	ret = v4l2_leddev_create_attrs(ld);
+	if (ret < 0)
+		led_classdev_unregister(&ld->cdev);
+
+	return ret;
+}
+EXPORT_SYMBOL(v4l2_leddev_register);
+
+void v4l2_leddev_unregister(struct v4l2_leddev *ld)
+{
+	v4l2_leddev_remove_attrs(ld);
+	led_classdev_unregister(&ld->cdev);
+}
+EXPORT_SYMBOL(v4l2_leddev_unregister);
+
+MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
+MODULE_DESCRIPTION("V4L2 API for exposing flash subdevs as LED class devices");
+MODULE_LICENSE("GPL");
diff --git a/include/media/v4l2-leddev.h b/include/media/v4l2-leddev.h
new file mode 100644
index 0000000..384b71f
--- /dev/null
+++ b/include/media/v4l2-leddev.h
@@ -0,0 +1,49 @@
+/*
+ * V4L2 API for exposing flash subdevs as led class devices
+ *
+ * Copyright (C) 2013, Samsung Electronics Co., Ltd.
+ * Andrzej Hajda <a.hajda@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.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _V4L2_LEDDEV_H
+#define _V4L2_LEDDEV_H
+
+#include <linux/device.h>
+#include <linux/leds.h>
+#include <media/v4l2-subdev.h>
+
+struct v4l2_leddev_attr;
+
+struct v4l2_leddev {
+	struct v4l2_subdev *sd;
+	struct led_classdev cdev;
+	int attr_count;
+	struct v4l2_leddev_attr *attrs;
+};
+
+#ifdef CONFIG_V4L2_LEDDEV
+
+int v4l2_leddev_register(struct device *dev, struct v4l2_leddev *ld);
+void v4l2_leddev_unregister(struct v4l2_leddev *ld);
+
+#else
+
+#define v4l2_leddev_register(dev, ld) (0)
+#define v4l2_leddev_unregister(ld) (void)
+
+#endif
+
+#endif /* _V4L2_LEDDEV_H */
-- 
1.7.10.4

[PATCH RFC 2/2] media: added max77693-led driver

[Andrzej Hajda]

The patch adds led-flash support to Maxim max77693 chipset.
Device is exposed to user space as a V4L2 subdevice.
It can be accessed via V4L2 controls interface.
Device supports up to two leds which can work in
flash and torch mode. Leds can be triggered externally or
by software.
Device is also exposed via LED API using v4l2-leddev helper.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
v3:
- added v4l2-leddev support

v2:
- kzalloc replaced by devm_kzalloc
- corrected cleanup code on probe fail
- simplified clamp routine
- cosmetic changes
---
 drivers/media/i2c/max77693-led.c |  650 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 650 insertions(+)
 create mode 100644 drivers/media/i2c/max77693-led.c

diff --git a/drivers/media/i2c/max77693-led.c b/drivers/media/i2c/max77693-led.c
new file mode 100644
index 0000000..a597611
--- /dev/null
+++ b/drivers/media/i2c/max77693-led.c
@@ -0,0 +1,650 @@
+/*
+ * Copyright (C) 2013, Samsung Electronics Co., Ltd.
+ * Andrzej Hajda <a.hajda@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.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/div64.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/max77693.h>
+#include <linux/mfd/max77693-private.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-leddev.h>
+
+#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)
+
+enum {
+	FLASH1,
+	FLASH2,
+	TORCH1,
+	TORCH2
+};
+
+enum {
+	FLASH,
+	TORCH
+};
+
+struct max77693_led {
+	struct regmap *regmap;
+	struct v4l2_subdev subdev;
+	struct platform_device *pdev;
+	struct max77693_led_platform_data *pdata;
+	struct v4l2_leddev leddev;
+
+	struct v4l2_ctrl_handler hdl;
+	struct {
+		struct v4l2_ctrl *led_mode;
+		struct v4l2_ctrl *source;
+	} ctrl;
+};
+
+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_led_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 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;
+}
+
+enum max77693_led_mode {
+	MODE_OFF,
+	MODE_FLASH,
+	MODE_TORCH,
+	MODE_EXTERNAL
+};
+
+static int max77693_led_set_mode(struct max77693_led *flash,
+			    enum max77693_led_mode mode)
+{
+	struct max77693_led_platform_data *p = flash->pdata;
+	struct regmap *rmap = flash->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_EXTERNAL:
+		v |= (p->trigger[FLASH1] & MAX77693_LED_TRIG_EXT) <<
+						MAX77693_FLASH_EN1_SHIFT;
+		v |= (p->trigger[FLASH2] & MAX77693_LED_TRIG_EXT) <<
+						MAX77693_FLASH_EN2_SHIFT;
+		v |= (p->trigger[TORCH1] & MAX77693_LED_TRIG_EXT) <<
+						MAX77693_TORCH_EN1_SHIFT;
+		v |= (p->trigger[TORCH2] & MAX77693_LED_TRIG_EXT) <<
+						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_led_setup(struct max77693_led *flash)
+{
+	struct max77693_led_platform_data *p = flash->pdata;
+	struct regmap *rmap = flash->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_led_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;
+
+	ret = max77693_led_set_mode(flash, MODE_OFF);
+
+	return ret;
+}
+
+static struct max77693_led *ctrl_to_flash(struct v4l2_ctrl *c)
+{
+	return container_of(c->handler, struct max77693_led, hdl);
+}
+
+static struct max77693_led *subdev_to_flash(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct max77693_led, subdev);
+}
+
+static int max77693_led_get_ctrl(struct v4l2_ctrl *c)
+{
+	struct max77693_led *flash = ctrl_to_flash(c);
+	struct regmap *rmap = flash->regmap;
+	int ret;
+	u8 v;
+
+	switch (c->id) {
+	case V4L2_CID_FLASH_STROBE_STATUS:
+		ret = max77693_read_reg(rmap, MAX77693_LED_REG_FLASH_INT_STATUS,
+									&v);
+		c->val = !!(v & MAX77693_LED_STATUS_FLASH_ON);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+/* split composite current @i into two @iout according to @imax weights */
+static void max77693_led_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_led_set_ctrl(struct v4l2_ctrl *c)
+{
+	struct max77693_led *flash = ctrl_to_flash(c);
+	struct max77693_led_platform_data *p = flash->pdata;
+	struct regmap *rmap = flash->regmap;
+	int v, ret = 0;
+	u32 iout[2];
+
+	switch (c->id) {
+	case V4L2_CID_FLASH_LED_MODE:
+		switch (c->val) {
+		case V4L2_FLASH_LED_MODE_NONE:
+			ret = max77693_led_set_mode(flash, MODE_OFF);
+			break;
+		case V4L2_FLASH_LED_MODE_FLASH:
+			if (flash->ctrl.source->val ==
+					V4L2_FLASH_STROBE_SOURCE_EXTERNAL)
+				ret = max77693_led_set_mode(flash,
+								MODE_EXTERNAL);
+			else
+				ret = max77693_led_set_mode(flash, MODE_OFF);
+			break;
+		case V4L2_FLASH_LED_MODE_TORCH:
+			ret = max77693_led_set_mode(flash, MODE_TORCH);
+			break;
+		}
+		break;
+	case V4L2_CID_FLASH_STROBE_SOURCE:
+		if (c->val == V4L2_FLASH_STROBE_SOURCE_EXTERNAL)
+			ret = max77693_led_set_mode(flash, MODE_EXTERNAL);
+		else
+			ret = max77693_led_set_mode(flash, MODE_OFF);
+		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;
+		ret = max77693_led_set_mode(flash, MODE_FLASH);
+		break;
+	case V4L2_CID_FLASH_STROBE_STOP:
+		ret = max77693_led_set_mode(flash, MODE_OFF);
+		break;
+	case V4L2_CID_FLASH_TIMEOUT:
+		v = max77693_led_timeout_to_reg(c->val);
+		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);
+		break;
+	case V4L2_CID_FLASH_INTENSITY:
+		max77693_led_calc_iout(iout, 1000 * c->val, &p->iout[FLASH1]);
+		v = max77693_led_iout_to_reg(iout[0]);
+		ret = max77693_write_reg(rmap, MAX77693_LED_REG_IFLASH1, v);
+		if (ret < 0)
+			break;
+		v = max77693_led_iout_to_reg(iout[1]);
+		ret = max77693_write_reg(rmap, MAX77693_LED_REG_IFLASH2, v);
+		break;
+	case V4L2_CID_FLASH_TORCH_INTENSITY:
+		max77693_led_calc_iout(iout, 1000 * c->val, &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);
+	}
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops max77693_led_ctrl_ops = {
+	.g_volatile_ctrl = max77693_led_get_ctrl,
+	.s_ctrl = max77693_led_set_ctrl,
+};
+
+static int max77693_init_controls(struct max77693_led *flash)
+{
+	struct max77693_led_platform_data *p = flash->pdata;
+	struct v4l2_ctrl *ctrl;
+	int mask, max;
+
+	v4l2_ctrl_handler_init(&flash->hdl, 8);
+
+	mask = 1 << V4L2_FLASH_LED_MODE_NONE;
+	max = V4L2_FLASH_LED_MODE_NONE;
+
+	if (p->trigger[FLASH1] | p->trigger[FLASH2] |
+	    ((p->trigger[TORCH1] | p->trigger[TORCH2]) &
+	     ~MAX77693_LED_TRIG_SOFT)) {
+		mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
+		max = V4L2_FLASH_LED_MODE_FLASH;
+	}
+	if (p->trigger[TORCH1] | p->trigger[TORCH2]) {
+		mask |= 1 << V4L2_FLASH_LED_MODE_TORCH;
+		max = V4L2_FLASH_LED_MODE_TORCH;
+	}
+
+	flash->ctrl.led_mode = v4l2_ctrl_new_std_menu(&flash->hdl,
+			&max77693_led_ctrl_ops, V4L2_CID_FLASH_LED_MODE,
+			max, ~mask, V4L2_FLASH_LED_MODE_NONE);
+
+	mask = 1 << V4L2_FLASH_STROBE_SOURCE_SOFTWARE;
+	max = V4L2_FLASH_STROBE_SOURCE_SOFTWARE;
+
+	if ((p->trigger[FLASH1] | p->trigger[FLASH2] | p->trigger[TORCH1] |
+	     p->trigger[TORCH2]) & ~MAX77693_LED_TRIG_SOFT) {
+		mask |= 1 << V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
+		max = V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
+	}
+
+	flash->ctrl.source = v4l2_ctrl_new_std_menu(&flash->hdl,
+			&max77693_led_ctrl_ops, V4L2_CID_FLASH_STROBE_SOURCE,
+			max, ~mask, V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
+
+
+	v4l2_ctrl_new_std(&flash->hdl, &max77693_led_ctrl_ops,
+			  V4L2_CID_FLASH_STROBE, 0, 0, 0, 0);
+
+
+	v4l2_ctrl_new_std(&flash->hdl, &max77693_led_ctrl_ops,
+			  V4L2_CID_FLASH_STROBE_STOP, 0, 0, 0, 0);
+
+
+	ctrl = v4l2_ctrl_new_std(&flash->hdl, &max77693_led_ctrl_ops,
+				 V4L2_CID_FLASH_STROBE_STATUS, 0, 1, 1, 1);
+	if (ctrl != NULL)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+
+	v4l2_ctrl_new_std(&flash->hdl, &max77693_led_ctrl_ops,
+			  V4L2_CID_FLASH_TIMEOUT, MAX77693_FLASH_TIMEOUT_MIN,
+			  MAX77693_FLASH_TIMEOUT_MAX,
+			  MAX77693_FLASH_TIMEOUT_STEP, p->timeout[FLASH]);
+
+	max = (p->iout[FLASH1] + p->iout[FLASH2]) / 1000;
+	v4l2_ctrl_new_std(&flash->hdl, &max77693_led_ctrl_ops,
+			  V4L2_CID_FLASH_INTENSITY,
+			  MAX77693_FLASH_IOUT_MIN / 1000, max, 1, max);
+
+	max = (p->iout[TORCH1] + p->iout[TORCH2]) / 1000;
+	v4l2_ctrl_new_std(&flash->hdl, &max77693_led_ctrl_ops,
+			  V4L2_CID_FLASH_TORCH_INTENSITY,
+			  MAX77693_TORCH_IOUT_MIN / 1000, max, 1, max);
+
+	flash->subdev.ctrl_handler = &flash->hdl;
+
+	return flash->hdl.error;
+}
+
+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,low-vsys", &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 *flash)
+{
+	struct max77693_led_platform_data *p;
+	struct device *dev = &flash->pdev->dev;
+
+	if (dev->of_node) {
+		p = devm_kzalloc(dev, sizeof(*flash->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;
+	}
+	flash->pdata = p;
+
+	max77693_led_validate_platform_data(p);
+
+	dev_info(dev, "Iout[uA]=%d,%d,%d,%d; Triggers=%d,%d,%d,%d;"
+		 "TriggerTypes=%d,%d;Timeouts[us]=%d,%d;BM=%d,%d;"
+		 "Vout[mV]=%d;LowVsys[mV]=%d\n",
+		 p->iout[0], p->iout[1], p->iout[2], p->iout[3],
+		 p->trigger[0], p->trigger[1], p->trigger[2], p->trigger[3],
+		 p->trigger_type[0], p->trigger_type[1],
+		 p->timeout[0], p->timeout[1],
+		 p->boost_mode[0], p->boost_mode[1],
+		 p->boost_vout, p->low_vsys);
+
+	return 0;
+}
+
+/* v4l2_subdev_init requires this structure */
+static struct v4l2_subdev_ops max77693_led_subdev_ops = {
+};
+
+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 *flash;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	flash = devm_kzalloc(dev, sizeof(*flash), GFP_KERNEL);
+	if (!flash)
+		return -ENOMEM;
+
+	flash->pdev = pdev;
+	flash->regmap = iodev->regmap;
+	ret = max77693_led_get_platform_data(flash);
+	if (ret < 0)
+		return ret;
+
+	sd = &flash->subdev;
+	v4l2_subdev_init(sd, &max77693_led_subdev_ops);
+	sd->owner = pdev->dev.driver->owner;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	strlcpy(sd->name, "max77693-led", sizeof(sd->name));
+	platform_set_drvdata(pdev, sd);
+
+	ret = max77693_init_controls(flash);
+	if (ret < 0)
+		goto err;
+
+	ret = media_entity_init(&sd->entity, 0, NULL, 0);
+	if (ret < 0)
+		goto err;
+
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
+
+	ret = max77693_led_setup(flash);
+	if (!ret) {
+		flash->leddev.sd = sd;
+		ret = v4l2_leddev_register(dev, &flash->leddev);
+	}
+	if (!ret)
+		return 0;
+
+	media_entity_cleanup(&sd->entity);
+err:
+	v4l2_ctrl_handler_free(&flash->hdl);
+	return ret;
+}
+
+static int max77693_led_remove(struct platform_device *pdev)
+{
+	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
+	struct max77693_led *flash = subdev_to_flash(sd);
+
+	v4l2_leddev_unregister(&flash->leddev);
+	v4l2_device_unregister_subdev(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(&flash->hdl);
+
+	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_DESCRIPTION("Maxim MAX77693 led flash driver");
+MODULE_LICENSE("GPL");
-- 
1.7.10.4

RE: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Kim, Milo]

Hi Andrzej,

> -----Original Message-----
> From: linux-leds-owner@vger.kernel.org [mailto:linux-leds-
> owner@vger.kernel.org] On Behalf Of Andrzej Hajda
> Sent: Monday, May 06, 2013 6:34 PM
> To: linux-media@vger.kernel.org; linux-leds@vger.kernel.org;
> devicetree-discuss@lists.ozlabs.org
> Cc: Laurent Pinchart; Sylwester Nawrocki; Sakari Ailus; Kyungmin Park;
> hj210.choi@samsung.com; sw0312.kim@samsung.com; Bryan Wu; Richard
> Purdie; Andrzej Hajda
> Subject: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class
> device
> 
> This RFC proposes generic API for exposing flash subdevices via LED
> framework.
> 
> Rationale
> 
> Currently there are two frameworks which are used for exposing LED
> flash to
> user space:
> - V4L2 flash controls,
> - LED framework(with custom sysfs attributes).
> 
> The list below shows flash drivers in mainline kernel with initial
> commit date
> and typical chip application (according to producer):
> 
> LED API:
>     lm3642: 2012-09-12, Cameras
>     lm355x: 2012-09-05, Cameras
>     max8997: 2011-12-14, Cameras (?)
>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
>     pca955x: 2008-07-16, Cameras, Indicators (?)
> V4L2 API:
>     as3645a:  2011-05-05, Cameras
>     adp1653: 2011-05-05, Cameras
> 
> V4L2 provides richest functionality, but there is often demand from
> application
> developers to provide already established LED API.
> We would like to have an unified user interface for flash devices. Some
> of
> devices already have the LED API driver exposing limited set of a Flash
> IC
> functionality. In order to support all required features the LED API
> would
> have to be extended or the V4L2 API would need to be used. However when
> switching from a LED to a V4L2 Flash driver existing LED API interface
> would
> need to be retained.
> 
> Proposed solution
> 
> This patch adds V4L2 helper functions to register existing V4L2 flash
> subdev
> as LED class device.
> After registration via v4l2_leddev_register appropriate entry in
> /sys/class/leds/ is created.
> During registration all V4L2 flash controls are enumerated and
> corresponding
> attributes are added.
> 
> I have attached also patch with new max77693-led driver using
> v4l2_leddev.
> This patch requires presence of the patch "max77693: added device tree
> support":
> https://patchwork.kernel.org/patch/2414351/ .
> 
> Additional features
> 
> - simple API to access all V4L2 flash controls via sysfs,
> - V4L2 subdevice should not be registered by V4L2 device to use it,
> - LED triggers API can be used to control the device,
> - LED device is optional - it will be created only if V4L2_LEDDEV
> configuration
>   option is enabled and the subdev driver calls v4l2_leddev_register.
> 
> Doubts
> 
> This RFC is a result of a uncertainty which API developers should
> expose by
> their flash drivers. It is a try to gluing together both APIs.
> I am not sure if it is the best solution, but I hope there will be some
> discussion and hopefully some decisions will be taken which way we
> should follow.

The LED subsystem provides similar APIs for the Camera driver.
With LED trigger event, flash and torch are enabled/disabled.
I'm not sure this is applicable for you.
Could you take a look at LED camera trigger feature?

For the camera LED trigger,
https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/?h=f
or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757

Example of camera flash driver,
https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/?h=f
or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a

Thanks,
Milo

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Laurent Pinchart]

On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
> > 
> > This RFC proposes generic API for exposing flash subdevices via LED
> > framework.
> > 
> > Rationale
> > 
> > Currently there are two frameworks which are used for exposing LED
> > flash to user space:
> > - V4L2 flash controls,
> > - LED framework(with custom sysfs attributes).
> > 
> > The list below shows flash drivers in mainline kernel with initial
> > commit date and typical chip application (according to producer):
> > 
> > LED API:
> >     lm3642: 2012-09-12, Cameras
> >     lm355x: 2012-09-05, Cameras
> >     max8997: 2011-12-14, Cameras (?)
> >     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
> >     pca955x: 2008-07-16, Cameras, Indicators (?)
> > 
> > V4L2 API:
> >     as3645a:  2011-05-05, Cameras
> >     adp1653: 2011-05-05, Cameras
> > 
> > V4L2 provides richest functionality, but there is often demand from
> > application developers to provide already established LED API. We would
> > like to have an unified user interface for flash devices. Some of devices
> > already have the LED API driver exposing limited set of a Flash IC
> > functionality. In order to support all required features the LED API
> > would have to be extended or the V4L2 API would need to be used. However
> > when switching from a LED to a V4L2 Flash driver existing LED API
> > interface would need to be retained.
> > 
> > Proposed solution
> > 
> > This patch adds V4L2 helper functions to register existing V4L2 flash
> > subdev as LED class device. After registration via v4l2_leddev_register
> > appropriate entry in /sys/class/leds/ is created. During registration all
> > V4L2 flash controls are enumerated and corresponding attributes are added.
> > 
> > I have attached also patch with new max77693-led driver using v4l2_leddev.
> > This patch requires presence of the patch "max77693: added device tree
> > support": https://patchwork.kernel.org/patch/2414351/ .
> > 
> > Additional features
> > 
> > - simple API to access all V4L2 flash controls via sysfs,
> > - V4L2 subdevice should not be registered by V4L2 device to use it,
> > - LED triggers API can be used to control the device,
> > - LED device is optional - it will be created only if V4L2_LEDDEV
> >   configuration option is enabled and the subdev driver calls
> >   v4l2_leddev_register.
> > 
> > Doubts
> > 
> > This RFC is a result of a uncertainty which API developers should expose
> > by their flash drivers. It is a try to gluing together both APIs. I am not
> > sure if it is the best solution, but I hope there will be some discussion
> > and hopefully some decisions will be taken which way we should follow.
> 
> The LED subsystem provides similar APIs for the Camera driver.
> With LED trigger event, flash and torch are enabled/disabled.
> I'm not sure this is applicable for you.
> Could you take a look at LED camera trigger feature?
> 
> For the camera LED trigger,
> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
> 
> Example of camera flash driver,
> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a

I think we should decide on one API. Implementing two APIs for a single device 
is usually messy, and will result in different feature sets (and different 
bugs) being implemented through each API, depending on the driver. 
Interactions between the APIs are also a pain point on the kernel side to 
properly synchronize calls.

The LED API is too limited for torch and flash usage, but I'm definitely open 
to moving flash devices to the LED API is we can extend it in a way that it 
covers all the use cases.

-- 
Regards,

Laurent Pinchart

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Andrzej Hajda]

On 07.05.2013 17:07, Laurent Pinchart wrote:
> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
>>> This RFC proposes generic API for exposing flash subdevices via LED
>>> framework.
>>>
>>> Rationale
>>>
>>> Currently there are two frameworks which are used for exposing LED
>>> flash to user space:
>>> - V4L2 flash controls,
>>> - LED framework(with custom sysfs attributes).
>>>
>>> The list below shows flash drivers in mainline kernel with initial
>>> commit date and typical chip application (according to producer):
>>>
>>> LED API:
>>>     lm3642: 2012-09-12, Cameras
>>>     lm355x: 2012-09-05, Cameras
>>>     max8997: 2011-12-14, Cameras (?)
>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
>>>
>>> V4L2 API:
>>>     as3645a:  2011-05-05, Cameras
>>>     adp1653: 2011-05-05, Cameras
>>>
>>> V4L2 provides richest functionality, but there is often demand from
>>> application developers to provide already established LED API. We would
>>> like to have an unified user interface for flash devices. Some of devices
>>> already have the LED API driver exposing limited set of a Flash IC
>>> functionality. In order to support all required features the LED API
>>> would have to be extended or the V4L2 API would need to be used. However
>>> when switching from a LED to a V4L2 Flash driver existing LED API
>>> interface would need to be retained.
>>>
>>> Proposed solution
>>>
>>> This patch adds V4L2 helper functions to register existing V4L2 flash
>>> subdev as LED class device. After registration via v4l2_leddev_register
>>> appropriate entry in /sys/class/leds/ is created. During registration all
>>> V4L2 flash controls are enumerated and corresponding attributes are added.
>>>
>>> I have attached also patch with new max77693-led driver using v4l2_leddev.
>>> This patch requires presence of the patch "max77693: added device tree
>>> support": https://patchwork.kernel.org/patch/2414351/ .
>>>
>>> Additional features
>>>
>>> - simple API to access all V4L2 flash controls via sysfs,
>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
>>> - LED triggers API can be used to control the device,
>>> - LED device is optional - it will be created only if V4L2_LEDDEV
>>>   configuration option is enabled and the subdev driver calls
>>>   v4l2_leddev_register.
>>>
>>> Doubts
>>>
>>> This RFC is a result of a uncertainty which API developers should expose
>>> by their flash drivers. It is a try to gluing together both APIs. I am not
>>> sure if it is the best solution, but I hope there will be some discussion
>>> and hopefully some decisions will be taken which way we should follow.
>> The LED subsystem provides similar APIs for the Camera driver.
>> With LED trigger event, flash and torch are enabled/disabled.
>> I'm not sure this is applicable for you.
>> Could you take a look at LED camera trigger feature?
>>
>> For the camera LED trigger,
>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
>>
>> Example of camera flash driver,
>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
> I think we should decide on one API. Implementing two APIs for a single device 
> is usually messy, and will result in different feature sets (and different 
> bugs) being implemented through each API, depending on the driver. 
> Interactions between the APIs are also a pain point on the kernel side to 
> properly synchronize calls.
>
> The LED API is too limited for torch and flash usage, but I'm definitely open 
> to moving flash devices to the LED API is we can extend it in a way that it 
> covers all the use cases.
>
Extending LED API IMHO seems to be quite straightforward - by adding
attributes for supported functionalities. We just need a specification for
standard flash/torch attributes.
I could prepare an RFC about it if there is a will to explore this
direction.

--
Regards
Andrzej Hajda <a.hajda@samsung.com>

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Sakari Ailus]

Hi all,

On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
> On 07.05.2013 17:07, Laurent Pinchart wrote:
> > On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
> >> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
> >>> This RFC proposes generic API for exposing flash subdevices via LED
> >>> framework.
> >>>
> >>> Rationale
> >>>
> >>> Currently there are two frameworks which are used for exposing LED
> >>> flash to user space:
> >>> - V4L2 flash controls,
> >>> - LED framework(with custom sysfs attributes).
> >>>
> >>> The list below shows flash drivers in mainline kernel with initial
> >>> commit date and typical chip application (according to producer):
> >>>
> >>> LED API:
> >>>     lm3642: 2012-09-12, Cameras
> >>>     lm355x: 2012-09-05, Cameras
> >>>     max8997: 2011-12-14, Cameras (?)
> >>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
> >>>     pca955x: 2008-07-16, Cameras, Indicators (?)
> >>>
> >>> V4L2 API:
> >>>     as3645a:  2011-05-05, Cameras
> >>>     adp1653: 2011-05-05, Cameras
> >>>
> >>> V4L2 provides richest functionality, but there is often demand from
> >>> application developers to provide already established LED API. We would
> >>> like to have an unified user interface for flash devices. Some of devices
> >>> already have the LED API driver exposing limited set of a Flash IC
> >>> functionality. In order to support all required features the LED API
> >>> would have to be extended or the V4L2 API would need to be used. However
> >>> when switching from a LED to a V4L2 Flash driver existing LED API
> >>> interface would need to be retained.
> >>>
> >>> Proposed solution
> >>>
> >>> This patch adds V4L2 helper functions to register existing V4L2 flash
> >>> subdev as LED class device. After registration via v4l2_leddev_register
> >>> appropriate entry in /sys/class/leds/ is created. During registration all
> >>> V4L2 flash controls are enumerated and corresponding attributes are added.
> >>>
> >>> I have attached also patch with new max77693-led driver using v4l2_leddev.
> >>> This patch requires presence of the patch "max77693: added device tree
> >>> support": https://patchwork.kernel.org/patch/2414351/ .
> >>>
> >>> Additional features
> >>>
> >>> - simple API to access all V4L2 flash controls via sysfs,
> >>> - V4L2 subdevice should not be registered by V4L2 device to use it,
> >>> - LED triggers API can be used to control the device,
> >>> - LED device is optional - it will be created only if V4L2_LEDDEV
> >>>   configuration option is enabled and the subdev driver calls
> >>>   v4l2_leddev_register.
> >>>
> >>> Doubts
> >>>
> >>> This RFC is a result of a uncertainty which API developers should expose
> >>> by their flash drivers. It is a try to gluing together both APIs. I am not
> >>> sure if it is the best solution, but I hope there will be some discussion
> >>> and hopefully some decisions will be taken which way we should follow.
> >> The LED subsystem provides similar APIs for the Camera driver.
> >> With LED trigger event, flash and torch are enabled/disabled.
> >> I'm not sure this is applicable for you.
> >> Could you take a look at LED camera trigger feature?
> >>
> >> For the camera LED trigger,
> >> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
> >>
> >> Example of camera flash driver,
> >> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
> > I think we should decide on one API. Implementing two APIs for a single device 
> > is usually messy, and will result in different feature sets (and different 
> > bugs) being implemented through each API, depending on the driver. 
> > Interactions between the APIs are also a pain point on the kernel side to 
> > properly synchronize calls.

I don't like having two APIs either. Especially we shouldn't have multiple
drivers implementing different APIs for the same device.

That said, I wonder if it's possible to support camera-related use cases
using the LED API: it's originally designed for quite different devices.
Even if you could handle flash strobing using the LED API, the functionality
provided by the Media controller and subdev APIs will always be missing:
device enumeration and association with the right camera.

The strobe functionality is connected to the sensor (hardware strobe pin)
which also is why it's natural for the flash to be accessible over the V4L2
API.

This was briefly discussed some time ago:

<URL:http://www.spinics.net/lists/linux-media/msg53645.html>

> > The LED API is too limited for torch and flash usage, but I'm definitely open 
> > to moving flash devices to the LED API is we can extend it in a way that it 
> > covers all the use cases.
> >
> Extending LED API IMHO seems to be quite straightforward - by adding
> attributes for supported functionalities. We just need a specification for
> standard flash/torch attributes.
> I could prepare an RFC about it if there is a will to explore this
> direction.

I'm leaning towards providing a wrapper that provides torch functionality
using V4L2 flash API unless it's really proven to be insane. ;-) The code
supporting that in an individual flash driver should be minimal --- which is
what the patchset essentially already does.

There are caveats though, such as if powering the flash controller requires
a regulator, then power management also must be done. Albeit I still
wouldn't expect this to be a major issue.

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

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Andrzej Hajda]

Hi All,

Thanks for comments and sorry for late response.

On 12.05.2013 23:12, Sakari Ailus wrote:
> Hi all,
>
> On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
>> On 07.05.2013 17:07, Laurent Pinchart wrote:
>>> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
>>>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
>>>>> This RFC proposes generic API for exposing flash subdevices via LED
>>>>> framework.
>>>>>
>>>>> Rationale
>>>>>
>>>>> Currently there are two frameworks which are used for exposing LED
>>>>> flash to user space:
>>>>> - V4L2 flash controls,
>>>>> - LED framework(with custom sysfs attributes).
>>>>>
>>>>> The list below shows flash drivers in mainline kernel with initial
>>>>> commit date and typical chip application (according to producer):
>>>>>
>>>>> LED API:
>>>>>     lm3642: 2012-09-12, Cameras
>>>>>     lm355x: 2012-09-05, Cameras
>>>>>     max8997: 2011-12-14, Cameras (?)
>>>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
>>>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
>>>>>
>>>>> V4L2 API:
>>>>>     as3645a:  2011-05-05, Cameras
>>>>>     adp1653: 2011-05-05, Cameras
>>>>>
>>>>> V4L2 provides richest functionality, but there is often demand from
>>>>> application developers to provide already established LED API. We would
>>>>> like to have an unified user interface for flash devices. Some of devices
>>>>> already have the LED API driver exposing limited set of a Flash IC
>>>>> functionality. In order to support all required features the LED API
>>>>> would have to be extended or the V4L2 API would need to be used. However
>>>>> when switching from a LED to a V4L2 Flash driver existing LED API
>>>>> interface would need to be retained.
>>>>>
>>>>> Proposed solution
>>>>>
>>>>> This patch adds V4L2 helper functions to register existing V4L2 flash
>>>>> subdev as LED class device. After registration via v4l2_leddev_register
>>>>> appropriate entry in /sys/class/leds/ is created. During registration all
>>>>> V4L2 flash controls are enumerated and corresponding attributes are added.
>>>>>
>>>>> I have attached also patch with new max77693-led driver using v4l2_leddev.
>>>>> This patch requires presence of the patch "max77693: added device tree
>>>>> support": https://patchwork.kernel.org/patch/2414351/ .
>>>>>
>>>>> Additional features
>>>>>
>>>>> - simple API to access all V4L2 flash controls via sysfs,
>>>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
>>>>> - LED triggers API can be used to control the device,
>>>>> - LED device is optional - it will be created only if V4L2_LEDDEV
>>>>>   configuration option is enabled and the subdev driver calls
>>>>>   v4l2_leddev_register.
>>>>>
>>>>> Doubts
>>>>>
>>>>> This RFC is a result of a uncertainty which API developers should expose
>>>>> by their flash drivers. It is a try to gluing together both APIs. I am not
>>>>> sure if it is the best solution, but I hope there will be some discussion
>>>>> and hopefully some decisions will be taken which way we should follow.
>>>> The LED subsystem provides similar APIs for the Camera driver.
>>>> With LED trigger event, flash and torch are enabled/disabled.
>>>> I'm not sure this is applicable for you.
>>>> Could you take a look at LED camera trigger feature?
>>>>
>>>> For the camera LED trigger,
>>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>>>> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
>>>>
>>>> Example of camera flash driver,
>>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>>>> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
>>> I think we should decide on one API. Implementing two APIs for a single device 
>>> is usually messy, and will result in different feature sets (and different 
>>> bugs) being implemented through each API, depending on the driver. 
>>> Interactions between the APIs are also a pain point on the kernel side to 
>>> properly synchronize calls.
> I don't like having two APIs either. Especially we shouldn't have multiple
> drivers implementing different APIs for the same device.
>
> That said, I wonder if it's possible to support camera-related use cases
> using the LED API: it's originally designed for quite different devices.
> Even if you could handle flash strobing using the LED API, the functionality
> provided by the Media controller and subdev APIs will always be missing:
> device enumeration and association with the right camera.
Is there a generic way to associate flash and camera subdevs in
current V4L2 API? The only ways I see now are:
- both belongs to the same media controller, but this is not enough if there
is more than one camera subdev in that controller,
- using media links/pads - at first sight it seems to be overkill/abuse...

On the other hand creating association between v4l2 subdev and led_classdev
should be possible using sysfs links and/or v4l2 controls or even using
led trigger API.
Of course it seems much less consistent than performing everything in V4L2.
>
> The strobe functionality is connected to the sensor (hardware strobe pin)
> which also is why it's natural for the flash to be accessible over the V4L2
> API.
>
> This was briefly discussed some time ago:
>
> <URL:http://www.spinics.net/lists/linux-media/msg53645.html>
>
>>> The LED API is too limited for torch and flash usage, but I'm definitely open 
>>> to moving flash devices to the LED API is we can extend it in a way that it 
>>> covers all the use cases.
>>>
>> Extending LED API IMHO seems to be quite straightforward - by adding
>> attributes for supported functionalities. We just need a specification for
>> standard flash/torch attributes.
>> I could prepare an RFC about it if there is a will to explore this
>> direction.
> I'm leaning towards providing a wrapper that provides torch functionality
> using V4L2 flash API unless it's really proven to be insane. ;-) The code
> supporting that in an individual flash driver should be minimal --- which is
> what the patchset essentially already does.
Providing only torch functionality do not require adding new attributes
(besides the ones already present in the led_classdev), so the patch will
be much simpler.

Regards
Andrzej
>
> There are caveats though, such as if powering the flash controller requires
> a regulator, then power management also must be done. Albeit I still
> wouldn't expect this to be a major issue.
>

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Sakari Ailus]

Hi Andrzej,

On Tue, May 21, 2013 at 10:34:53AM +0200, Andrzej Hajda wrote:
> On 12.05.2013 23:12, Sakari Ailus wrote:
> > On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
> >> On 07.05.2013 17:07, Laurent Pinchart wrote:
> >>> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
> >>>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
> >>>>> This RFC proposes generic API for exposing flash subdevices via LED
> >>>>> framework.
> >>>>>
> >>>>> Rationale
> >>>>>
> >>>>> Currently there are two frameworks which are used for exposing LED
> >>>>> flash to user space:
> >>>>> - V4L2 flash controls,
> >>>>> - LED framework(with custom sysfs attributes).
> >>>>>
> >>>>> The list below shows flash drivers in mainline kernel with initial
> >>>>> commit date and typical chip application (according to producer):
> >>>>>
> >>>>> LED API:
> >>>>>     lm3642: 2012-09-12, Cameras
> >>>>>     lm355x: 2012-09-05, Cameras
> >>>>>     max8997: 2011-12-14, Cameras (?)
> >>>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
> >>>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
> >>>>>
> >>>>> V4L2 API:
> >>>>>     as3645a:  2011-05-05, Cameras
> >>>>>     adp1653: 2011-05-05, Cameras
> >>>>>
> >>>>> V4L2 provides richest functionality, but there is often demand from
> >>>>> application developers to provide already established LED API. We would
> >>>>> like to have an unified user interface for flash devices. Some of devices
> >>>>> already have the LED API driver exposing limited set of a Flash IC
> >>>>> functionality. In order to support all required features the LED API
> >>>>> would have to be extended or the V4L2 API would need to be used. However
> >>>>> when switching from a LED to a V4L2 Flash driver existing LED API
> >>>>> interface would need to be retained.
> >>>>>
> >>>>> Proposed solution
> >>>>>
> >>>>> This patch adds V4L2 helper functions to register existing V4L2 flash
> >>>>> subdev as LED class device. After registration via v4l2_leddev_register
> >>>>> appropriate entry in /sys/class/leds/ is created. During registration all
> >>>>> V4L2 flash controls are enumerated and corresponding attributes are added.
> >>>>>
> >>>>> I have attached also patch with new max77693-led driver using v4l2_leddev.
> >>>>> This patch requires presence of the patch "max77693: added device tree
> >>>>> support": https://patchwork.kernel.org/patch/2414351/ .
> >>>>>
> >>>>> Additional features
> >>>>>
> >>>>> - simple API to access all V4L2 flash controls via sysfs,
> >>>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
> >>>>> - LED triggers API can be used to control the device,
> >>>>> - LED device is optional - it will be created only if V4L2_LEDDEV
> >>>>>   configuration option is enabled and the subdev driver calls
> >>>>>   v4l2_leddev_register.
> >>>>>
> >>>>> Doubts
> >>>>>
> >>>>> This RFC is a result of a uncertainty which API developers should expose
> >>>>> by their flash drivers. It is a try to gluing together both APIs. I am not
> >>>>> sure if it is the best solution, but I hope there will be some discussion
> >>>>> and hopefully some decisions will be taken which way we should follow.
> >>>> The LED subsystem provides similar APIs for the Camera driver.
> >>>> With LED trigger event, flash and torch are enabled/disabled.
> >>>> I'm not sure this is applicable for you.
> >>>> Could you take a look at LED camera trigger feature?
> >>>>
> >>>> For the camera LED trigger,
> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >>>> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
> >>>>
> >>>> Example of camera flash driver,
> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >>>> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
> >>> I think we should decide on one API. Implementing two APIs for a single device 
> >>> is usually messy, and will result in different feature sets (and different 
> >>> bugs) being implemented through each API, depending on the driver. 
> >>> Interactions between the APIs are also a pain point on the kernel side to 
> >>> properly synchronize calls.
> > I don't like having two APIs either. Especially we shouldn't have multiple
> > drivers implementing different APIs for the same device.
> >
> > That said, I wonder if it's possible to support camera-related use cases
> > using the LED API: it's originally designed for quite different devices.
> > Even if you could handle flash strobing using the LED API, the functionality
> > provided by the Media controller and subdev APIs will always be missing:
> > device enumeration and association with the right camera.
> Is there a generic way to associate flash and camera subdevs in
> current V4L2 API? The only ways I see now are:
> - both belongs to the same media controller, but this is not enough if there
> is more than one camera subdev in that controller,

Yes, there is. That's the group_id field in struct media_entity_desc. The
lens subdev is associated to the rest of the devices the same way.

> - using media links/pads - at first sight it seems to be overkill/abuse...

No. Links describe the flow of data, not relations between entities.

...

> >>> The LED API is too limited for torch and flash usage, but I'm definitely open 
> >>> to moving flash devices to the LED API is we can extend it in a way that it 
> >>> covers all the use cases.
> >>>
> >> Extending LED API IMHO seems to be quite straightforward - by adding
> >> attributes for supported functionalities. We just need a specification for
> >> standard flash/torch attributes.
> >> I could prepare an RFC about it if there is a will to explore this
> >> direction.
> > I'm leaning towards providing a wrapper that provides torch functionality
> > using V4L2 flash API unless it's really proven to be insane. ;-) The code
> > supporting that in an individual flash driver should be minimal --- which is
> > what the patchset essentially already does.
> Providing only torch functionality do not require adding new attributes
> (besides the ones already present in the led_classdev), so the patch will
> be much simpler.

Yes. Attributes could be added later on to the LED API to support flash and
the wrapper could be extended accordingly. My thinking is however that the
main use case is torch, not strobing flash, so it would be fulfilled already
without extensions to the LED API.

-- 
Kind regards,

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

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Bryan Wu]

On Tue, May 21, 2013 at 3:54 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Andrzej,
>
> On Tue, May 21, 2013 at 10:34:53AM +0200, Andrzej Hajda wrote:
>> On 12.05.2013 23:12, Sakari Ailus wrote:
>> > On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
>> >> On 07.05.2013 17:07, Laurent Pinchart wrote:
>> >>> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
>> >>>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
>> >>>>> This RFC proposes generic API for exposing flash subdevices via LED
>> >>>>> framework.
>> >>>>>
>> >>>>> Rationale
>> >>>>>
>> >>>>> Currently there are two frameworks which are used for exposing LED
>> >>>>> flash to user space:
>> >>>>> - V4L2 flash controls,
>> >>>>> - LED framework(with custom sysfs attributes).
>> >>>>>
>> >>>>> The list below shows flash drivers in mainline kernel with initial
>> >>>>> commit date and typical chip application (according to producer):
>> >>>>>
>> >>>>> LED API:
>> >>>>>     lm3642: 2012-09-12, Cameras
>> >>>>>     lm355x: 2012-09-05, Cameras
>> >>>>>     max8997: 2011-12-14, Cameras (?)
>> >>>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
>> >>>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
>> >>>>>
>> >>>>> V4L2 API:
>> >>>>>     as3645a:  2011-05-05, Cameras
>> >>>>>     adp1653: 2011-05-05, Cameras
>> >>>>>
>> >>>>> V4L2 provides richest functionality, but there is often demand from
>> >>>>> application developers to provide already established LED API. We would
>> >>>>> like to have an unified user interface for flash devices. Some of devices
>> >>>>> already have the LED API driver exposing limited set of a Flash IC
>> >>>>> functionality. In order to support all required features the LED API
>> >>>>> would have to be extended or the V4L2 API would need to be used. However
>> >>>>> when switching from a LED to a V4L2 Flash driver existing LED API
>> >>>>> interface would need to be retained.
>> >>>>>
>> >>>>> Proposed solution
>> >>>>>
>> >>>>> This patch adds V4L2 helper functions to register existing V4L2 flash
>> >>>>> subdev as LED class device. After registration via v4l2_leddev_register
>> >>>>> appropriate entry in /sys/class/leds/ is created. During registration all
>> >>>>> V4L2 flash controls are enumerated and corresponding attributes are added.
>> >>>>>
>> >>>>> I have attached also patch with new max77693-led driver using v4l2_leddev.
>> >>>>> This patch requires presence of the patch "max77693: added device tree
>> >>>>> support": https://patchwork.kernel.org/patch/2414351/ .
>> >>>>>
>> >>>>> Additional features
>> >>>>>
>> >>>>> - simple API to access all V4L2 flash controls via sysfs,
>> >>>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
>> >>>>> - LED triggers API can be used to control the device,
>> >>>>> - LED device is optional - it will be created only if V4L2_LEDDEV
>> >>>>>   configuration option is enabled and the subdev driver calls
>> >>>>>   v4l2_leddev_register.
>> >>>>>
>> >>>>> Doubts
>> >>>>>
>> >>>>> This RFC is a result of a uncertainty which API developers should expose
>> >>>>> by their flash drivers. It is a try to gluing together both APIs. I am not
>> >>>>> sure if it is the best solution, but I hope there will be some discussion
>> >>>>> and hopefully some decisions will be taken which way we should follow.
>> >>>> The LED subsystem provides similar APIs for the Camera driver.
>> >>>> With LED trigger event, flash and torch are enabled/disabled.
>> >>>> I'm not sure this is applicable for you.
>> >>>> Could you take a look at LED camera trigger feature?
>> >>>>
>> >>>> For the camera LED trigger,
>> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>> >>>> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
>> >>>>
>> >>>> Example of camera flash driver,
>> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>> >>>> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
>> >>> I think we should decide on one API. Implementing two APIs for a single device
>> >>> is usually messy, and will result in different feature sets (and different
>> >>> bugs) being implemented through each API, depending on the driver.
>> >>> Interactions between the APIs are also a pain point on the kernel side to
>> >>> properly synchronize calls.
>> > I don't like having two APIs either. Especially we shouldn't have multiple
>> > drivers implementing different APIs for the same device.
>> >
>> > That said, I wonder if it's possible to support camera-related use cases
>> > using the LED API: it's originally designed for quite different devices.
>> > Even if you could handle flash strobing using the LED API, the functionality
>> > provided by the Media controller and subdev APIs will always be missing:
>> > device enumeration and association with the right camera.
>> Is there a generic way to associate flash and camera subdevs in
>> current V4L2 API? The only ways I see now are:
>> - both belongs to the same media controller, but this is not enough if there
>> is more than one camera subdev in that controller,
>
> Yes, there is. That's the group_id field in struct media_entity_desc. The
> lens subdev is associated to the rest of the devices the same way.
>
>> - using media links/pads - at first sight it seems to be overkill/abuse...
>
> No. Links describe the flow of data, not relations between entities.
>
> ...
>
>> >>> The LED API is too limited for torch and flash usage, but I'm definitely open
>> >>> to moving flash devices to the LED API is we can extend it in a way that it
>> >>> covers all the use cases.
>> >>>
>> >> Extending LED API IMHO seems to be quite straightforward - by adding
>> >> attributes for supported functionalities. We just need a specification for
>> >> standard flash/torch attributes.
>> >> I could prepare an RFC about it if there is a will to explore this
>> >> direction.
>> > I'm leaning towards providing a wrapper that provides torch functionality
>> > using V4L2 flash API unless it's really proven to be insane. ;-) The code
>> > supporting that in an individual flash driver should be minimal --- which is
>> > what the patchset essentially already does.
>> Providing only torch functionality do not require adding new attributes
>> (besides the ones already present in the led_classdev), so the patch will
>> be much simpler.
>
> Yes. Attributes could be added later on to the LED API to support flash and
> the wrapper could be extended accordingly. My thinking is however that the
> main use case is torch, not strobing flash, so it would be fulfilled already
> without extensions to the LED API.
>

Sorry for replying so late.

I think Milo Kim did some work in our LED subsystem by add LED Flash
trigger for camera device. I agree it doesn't satisfy the usage of
V4L2 Flash API and what I'm thinking about is expanding the LED Flash
trigger driver to export a well defined sysfs interface, so user space
libv4l2 can wrap it for applications.

Thanks,
-Bryan

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Laurent Pinchart]

Hi Bryan,

On Thursday 10 October 2013 17:07:22 Bryan Wu wrote:
> On Tue, May 21, 2013 at 3:54 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > On Tue, May 21, 2013 at 10:34:53AM +0200, Andrzej Hajda wrote:
> >> On 12.05.2013 23:12, Sakari Ailus wrote:
> >> > On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
> >> >> On 07.05.2013 17:07, Laurent Pinchart wrote:
> >> >>> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
> >> >>>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
> >> >>>>> This RFC proposes generic API for exposing flash subdevices via LED
> >> >>>>> framework.
> >> >>>>> 
> >> >>>>> Rationale
> >> >>>>> 
> >> >>>>> Currently there are two frameworks which are used for exposing LED
> >> >>>>> flash to user space:
> >> >>>>> - V4L2 flash controls,
> >> >>>>> - LED framework(with custom sysfs attributes).
> >> >>>>> 
> >> >>>>> The list below shows flash drivers in mainline kernel with initial
> >> >>>>> commit date and typical chip application (according to producer):
> >> >>>>> 
> >> >>>>> LED API:
> >> >>>>>     lm3642: 2012-09-12, Cameras
> >> >>>>>     lm355x: 2012-09-05, Cameras
> >> >>>>>     max8997: 2011-12-14, Cameras (?)
> >> >>>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
> >> >>>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
> >> >>>>> 
> >> >>>>> V4L2 API:
> >> >>>>>     as3645a:  2011-05-05, Cameras
> >> >>>>>     adp1653: 2011-05-05, Cameras
> >> >>>>> 
> >> >>>>> V4L2 provides richest functionality, but there is often demand from
> >> >>>>> application developers to provide already established LED API. We
> >> >>>>> would like to have an unified user interface for flash devices.
> >> >>>>> Some of devices already have the LED API driver exposing limited
> >> >>>>> set of a Flash IC functionality. In order to support all required
> >> >>>>> features the LED API would have to be extended or the V4L2 API
> >> >>>>> would need to be used. However when switching from a LED to a V4L2
> >> >>>>> Flash driver existing LED API interface would need to be retained.
> >> >>>>> 
> >> >>>>> Proposed solution
> >> >>>>> 
> >> >>>>> This patch adds V4L2 helper functions to register existing V4L2
> >> >>>>> flash subdev as LED class device. After registration via
> >> >>>>> v4l2_leddev_register appropriate entry in /sys/class/leds/ is
> >> >>>>> created. During registration all V4L2 flash controls are enumerated
> >> >>>>> and corresponding attributes are added.
> >> >>>>> 
> >> >>>>> I have attached also patch with new max77693-led driver using
> >> >>>>> v4l2_leddev. This patch requires presence of the patch "max77693:
> >> >>>>> added device tree support":
> >> >>>>> https://patchwork.kernel.org/patch/2414351/ .
> >> >>>>> 
> >> >>>>> Additional features
> >> >>>>> 
> >> >>>>> - simple API to access all V4L2 flash controls via sysfs,
> >> >>>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
> >> >>>>> - LED triggers API can be used to control the device,
> >> >>>>> - LED device is optional - it will be created only if V4L2_LEDDEV
> >> >>>>>   configuration option is enabled and the subdev driver calls
> >> >>>>>   v4l2_leddev_register.
> >> >>>>> 
> >> >>>>> Doubts
> >> >>>>> 
> >> >>>>> This RFC is a result of a uncertainty which API developers should
> >> >>>>> expose by their flash drivers. It is a try to gluing together both
> >> >>>>> APIs. I am not sure if it is the best solution, but I hope there
> >> >>>>> will be some discussion and hopefully some decisions will be taken
> >> >>>>> which way we should follow.
> >> >>>> 
> >> >>>> The LED subsystem provides similar APIs for the Camera driver.
> >> >>>> With LED trigger event, flash and torch are enabled/disabled.
> >> >>>> I'm not sure this is applicable for you.
> >> >>>> Could you take a look at LED camera trigger feature?
> >> >>>> 
> >> >>>> For the camera LED trigger,
> >> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git
> >> >>>> /commit/ ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
> >> >>>> 
> >> >>>> Example of camera flash driver,
> >> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git
> >> >>>> /commit/ ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
> >> >>> 
> >> >>> I think we should decide on one API. Implementing two APIs for a
> >> >>> single device is usually messy, and will result in different feature
> >> >>> sets (and different bugs) being implemented through each API,
> >> >>> depending on the driver. Interactions between the APIs are also a
> >> >>> pain point on the kernel side to properly synchronize calls.
> >> > 
> >> > I don't like having two APIs either. Especially we shouldn't have
> >> > multiple drivers implementing different APIs for the same device.
> >> > 
> >> > That said, I wonder if it's possible to support camera-related use
> >> > cases using the LED API: it's originally designed for quite different
> >> > devices. Even if you could handle flash strobing using the LED API, the
> >> > functionality provided by the Media controller and subdev APIs will
> >> > always be missing: device enumeration and association with the right
> >> > camera.
> >> 
> >> Is there a generic way to associate flash and camera subdevs in
> >> current V4L2 API? The only ways I see now are:
> >> - both belongs to the same media controller, but this is not enough if
> >> there is more than one camera subdev in that controller,
> > 
> > Yes, there is. That's the group_id field in struct media_entity_desc. The
> > lens subdev is associated to the rest of the devices the same way.
> > 
> >> - using media links/pads - at first sight it seems to be
> >> overkill/abuse...
> > 
> > No. Links describe the flow of data, not relations between entities.
> > 
> > ...
> > 
> >> >>> The LED API is too limited for torch and flash usage, but I'm
> >> >>> definitely open to moving flash devices to the LED API is we can
> >> >>> extend it in a way that it covers all the use cases.
> >> >> 
> >> >> Extending LED API IMHO seems to be quite straightforward - by adding
> >> >> attributes for supported functionalities. We just need a specification
> >> >> for standard flash/torch attributes.
> >> >> I could prepare an RFC about it if there is a will to explore this
> >> >> direction.
> >> > 
> >> > I'm leaning towards providing a wrapper that provides torch
> >> > functionality using V4L2 flash API unless it's really proven to be
> >> > insane. ;-) The code supporting that in an individual flash driver
> >> > should be minimal --- which is what the patchset essentially already
> >> > does.
> >> 
> >> Providing only torch functionality do not require adding new attributes
> >> (besides the ones already present in the led_classdev), so the patch will
> >> be much simpler.
> > 
> > Yes. Attributes could be added later on to the LED API to support flash
> > and the wrapper could be extended accordingly. My thinking is however that
> > the main use case is torch, not strobing flash, so it would be fulfilled
> > already without extensions to the LED API.
> 
> Sorry for replying so late.
> 
> I think Milo Kim did some work in our LED subsystem by add LED Flash
> trigger for camera device. I agree it doesn't satisfy the usage of
> V4L2 Flash API and what I'm thinking about is expanding the LED Flash
> trigger driver to export a well defined sysfs interface, so user space
> libv4l2 can wrap it for applications.

I've replied to your comment in the media-summit agenda mail thread, but the 
discussion belongs here, so here's a repost. Let's try to keep replies in this 
mail thread.

The biggest reason why we're not fond of sysfs-based APIs for media devices is 
that they can't provide atomicity. There's no way to set multiple parameters 
in a single operation.

We can't get rid of the sysfs LEDs API, but maybe we could have a unified 
kernel LED/flash subsystem that would provide both a sysfs-based API to ensure 
compatibility with current userspace software and an ioctl-based API (possibly 
through V4L2 controls). That way LED/flash devices would be registered with a 
single subsystem, and the corresponding drivers won't have to care about the 
API exposed to userspace. That would require a major refactoring of the in-
kernel APIs though.

-- 
Regards,

Laurent Pinchart

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Sakari Ailus]

Hi Bryan,

On Thu, Oct 10, 2013 at 05:07:22PM -0700, Bryan Wu wrote:
> On Tue, May 21, 2013 at 3:54 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > Hi Andrzej,
> >
> > On Tue, May 21, 2013 at 10:34:53AM +0200, Andrzej Hajda wrote:
> >> On 12.05.2013 23:12, Sakari Ailus wrote:
> >> > On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
> >> >> On 07.05.2013 17:07, Laurent Pinchart wrote:
> >> >>> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
> >> >>>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
> >> >>>>> This RFC proposes generic API for exposing flash subdevices via LED
> >> >>>>> framework.
> >> >>>>>
> >> >>>>> Rationale
> >> >>>>>
> >> >>>>> Currently there are two frameworks which are used for exposing LED
> >> >>>>> flash to user space:
> >> >>>>> - V4L2 flash controls,
> >> >>>>> - LED framework(with custom sysfs attributes).
> >> >>>>>
> >> >>>>> The list below shows flash drivers in mainline kernel with initial
> >> >>>>> commit date and typical chip application (according to producer):
> >> >>>>>
> >> >>>>> LED API:
> >> >>>>>     lm3642: 2012-09-12, Cameras
> >> >>>>>     lm355x: 2012-09-05, Cameras
> >> >>>>>     max8997: 2011-12-14, Cameras (?)
> >> >>>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
> >> >>>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
> >> >>>>>
> >> >>>>> V4L2 API:
> >> >>>>>     as3645a:  2011-05-05, Cameras
> >> >>>>>     adp1653: 2011-05-05, Cameras
> >> >>>>>
> >> >>>>> V4L2 provides richest functionality, but there is often demand from
> >> >>>>> application developers to provide already established LED API. We would
> >> >>>>> like to have an unified user interface for flash devices. Some of devices
> >> >>>>> already have the LED API driver exposing limited set of a Flash IC
> >> >>>>> functionality. In order to support all required features the LED API
> >> >>>>> would have to be extended or the V4L2 API would need to be used. However
> >> >>>>> when switching from a LED to a V4L2 Flash driver existing LED API
> >> >>>>> interface would need to be retained.
> >> >>>>>
> >> >>>>> Proposed solution
> >> >>>>>
> >> >>>>> This patch adds V4L2 helper functions to register existing V4L2 flash
> >> >>>>> subdev as LED class device. After registration via v4l2_leddev_register
> >> >>>>> appropriate entry in /sys/class/leds/ is created. During registration all
> >> >>>>> V4L2 flash controls are enumerated and corresponding attributes are added.
> >> >>>>>
> >> >>>>> I have attached also patch with new max77693-led driver using v4l2_leddev.
> >> >>>>> This patch requires presence of the patch "max77693: added device tree
> >> >>>>> support": https://patchwork.kernel.org/patch/2414351/ .
> >> >>>>>
> >> >>>>> Additional features
> >> >>>>>
> >> >>>>> - simple API to access all V4L2 flash controls via sysfs,
> >> >>>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
> >> >>>>> - LED triggers API can be used to control the device,
> >> >>>>> - LED device is optional - it will be created only if V4L2_LEDDEV
> >> >>>>>   configuration option is enabled and the subdev driver calls
> >> >>>>>   v4l2_leddev_register.
> >> >>>>>
> >> >>>>> Doubts
> >> >>>>>
> >> >>>>> This RFC is a result of a uncertainty which API developers should expose
> >> >>>>> by their flash drivers. It is a try to gluing together both APIs. I am not
> >> >>>>> sure if it is the best solution, but I hope there will be some discussion
> >> >>>>> and hopefully some decisions will be taken which way we should follow.
> >> >>>> The LED subsystem provides similar APIs for the Camera driver.
> >> >>>> With LED trigger event, flash and torch are enabled/disabled.
> >> >>>> I'm not sure this is applicable for you.
> >> >>>> Could you take a look at LED camera trigger feature?
> >> >>>>
> >> >>>> For the camera LED trigger,
> >> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >> >>>> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
> >> >>>>
> >> >>>> Example of camera flash driver,
> >> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >> >>>> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
> >> >>> I think we should decide on one API. Implementing two APIs for a single device
> >> >>> is usually messy, and will result in different feature sets (and different
> >> >>> bugs) being implemented through each API, depending on the driver.
> >> >>> Interactions between the APIs are also a pain point on the kernel side to
> >> >>> properly synchronize calls.
> >> > I don't like having two APIs either. Especially we shouldn't have multiple
> >> > drivers implementing different APIs for the same device.
> >> >
> >> > That said, I wonder if it's possible to support camera-related use cases
> >> > using the LED API: it's originally designed for quite different devices.
> >> > Even if you could handle flash strobing using the LED API, the functionality
> >> > provided by the Media controller and subdev APIs will always be missing:
> >> > device enumeration and association with the right camera.
> >> Is there a generic way to associate flash and camera subdevs in
> >> current V4L2 API? The only ways I see now are:
> >> - both belongs to the same media controller, but this is not enough if there
> >> is more than one camera subdev in that controller,
> >
> > Yes, there is. That's the group_id field in struct media_entity_desc. The
> > lens subdev is associated to the rest of the devices the same way.
> >
> >> - using media links/pads - at first sight it seems to be overkill/abuse...
> >
> > No. Links describe the flow of data, not relations between entities.
> >
> > ...
> >
> >> >>> The LED API is too limited for torch and flash usage, but I'm definitely open
> >> >>> to moving flash devices to the LED API is we can extend it in a way that it
> >> >>> covers all the use cases.
> >> >>>
> >> >> Extending LED API IMHO seems to be quite straightforward - by adding
> >> >> attributes for supported functionalities. We just need a specification for
> >> >> standard flash/torch attributes.
> >> >> I could prepare an RFC about it if there is a will to explore this
> >> >> direction.
> >> > I'm leaning towards providing a wrapper that provides torch functionality
> >> > using V4L2 flash API unless it's really proven to be insane. ;-) The code
> >> > supporting that in an individual flash driver should be minimal --- which is
> >> > what the patchset essentially already does.
> >> Providing only torch functionality do not require adding new attributes
> >> (besides the ones already present in the led_classdev), so the patch will
> >> be much simpler.
> >
> > Yes. Attributes could be added later on to the LED API to support flash and
> > the wrapper could be extended accordingly. My thinking is however that the
> > main use case is torch, not strobing flash, so it would be fulfilled already
> > without extensions to the LED API.
> >
> 
> Sorry for replying so late.
> 
> I think Milo Kim did some work in our LED subsystem by add LED Flash
> trigger for camera device. I agree it doesn't satisfy the usage of
> V4L2 Flash API and what I'm thinking about is expanding the LED Flash
> trigger driver to export a well defined sysfs interface, so user space
> libv4l2 can wrap it for applications.

libv4l2 currently provides a V4L2 API only, not V4L2 subdev API which is
implemented by the existing flash drivers which in turn are used in embedded
systems where the user space accesses V4L2 subdev API directly.

So using libv4l2 could be workable but only in a subset of use cases for
those chips (where regular V4L2 API is sufficient). I think we'd need to
cover all to avoid writing double set of drivers.

-- 
Kind regards,

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

Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Bryan Wu]

On Sun, Oct 13, 2013 at 1:56 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Bryan,
>
> On Thu, Oct 10, 2013 at 05:07:22PM -0700, Bryan Wu wrote:
>> On Tue, May 21, 2013 at 3:54 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>> > Hi Andrzej,
>> >
>> > On Tue, May 21, 2013 at 10:34:53AM +0200, Andrzej Hajda wrote:
>> >> On 12.05.2013 23:12, Sakari Ailus wrote:
>> >> > On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
>> >> >> On 07.05.2013 17:07, Laurent Pinchart wrote:
>> >> >>> On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
>> >> >>>> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
>> >> >>>>> This RFC proposes generic API for exposing flash subdevices via LED
>> >> >>>>> framework.
>> >> >>>>>
>> >> >>>>> Rationale
>> >> >>>>>
>> >> >>>>> Currently there are two frameworks which are used for exposing LED
>> >> >>>>> flash to user space:
>> >> >>>>> - V4L2 flash controls,
>> >> >>>>> - LED framework(with custom sysfs attributes).
>> >> >>>>>
>> >> >>>>> The list below shows flash drivers in mainline kernel with initial
>> >> >>>>> commit date and typical chip application (according to producer):
>> >> >>>>>
>> >> >>>>> LED API:
>> >> >>>>>     lm3642: 2012-09-12, Cameras
>> >> >>>>>     lm355x: 2012-09-05, Cameras
>> >> >>>>>     max8997: 2011-12-14, Cameras (?)
>> >> >>>>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
>> >> >>>>>     pca955x: 2008-07-16, Cameras, Indicators (?)
>> >> >>>>>
>> >> >>>>> V4L2 API:
>> >> >>>>>     as3645a:  2011-05-05, Cameras
>> >> >>>>>     adp1653: 2011-05-05, Cameras
>> >> >>>>>
>> >> >>>>> V4L2 provides richest functionality, but there is often demand from
>> >> >>>>> application developers to provide already established LED API. We would
>> >> >>>>> like to have an unified user interface for flash devices. Some of devices
>> >> >>>>> already have the LED API driver exposing limited set of a Flash IC
>> >> >>>>> functionality. In order to support all required features the LED API
>> >> >>>>> would have to be extended or the V4L2 API would need to be used. However
>> >> >>>>> when switching from a LED to a V4L2 Flash driver existing LED API
>> >> >>>>> interface would need to be retained.
>> >> >>>>>
>> >> >>>>> Proposed solution
>> >> >>>>>
>> >> >>>>> This patch adds V4L2 helper functions to register existing V4L2 flash
>> >> >>>>> subdev as LED class device. After registration via v4l2_leddev_register
>> >> >>>>> appropriate entry in /sys/class/leds/ is created. During registration all
>> >> >>>>> V4L2 flash controls are enumerated and corresponding attributes are added.
>> >> >>>>>
>> >> >>>>> I have attached also patch with new max77693-led driver using v4l2_leddev.
>> >> >>>>> This patch requires presence of the patch "max77693: added device tree
>> >> >>>>> support": https://patchwork.kernel.org/patch/2414351/ .
>> >> >>>>>
>> >> >>>>> Additional features
>> >> >>>>>
>> >> >>>>> - simple API to access all V4L2 flash controls via sysfs,
>> >> >>>>> - V4L2 subdevice should not be registered by V4L2 device to use it,
>> >> >>>>> - LED triggers API can be used to control the device,
>> >> >>>>> - LED device is optional - it will be created only if V4L2_LEDDEV
>> >> >>>>>   configuration option is enabled and the subdev driver calls
>> >> >>>>>   v4l2_leddev_register.
>> >> >>>>>
>> >> >>>>> Doubts
>> >> >>>>>
>> >> >>>>> This RFC is a result of a uncertainty which API developers should expose
>> >> >>>>> by their flash drivers. It is a try to gluing together both APIs. I am not
>> >> >>>>> sure if it is the best solution, but I hope there will be some discussion
>> >> >>>>> and hopefully some decisions will be taken which way we should follow.
>> >> >>>> The LED subsystem provides similar APIs for the Camera driver.
>> >> >>>> With LED trigger event, flash and torch are enabled/disabled.
>> >> >>>> I'm not sure this is applicable for you.
>> >> >>>> Could you take a look at LED camera trigger feature?
>> >> >>>>
>> >> >>>> For the camera LED trigger,
>> >> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>> >> >>>> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
>> >> >>>>
>> >> >>>> Example of camera flash driver,
>> >> >>>> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
>> >> >>>> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
>> >> >>> I think we should decide on one API. Implementing two APIs for a single device
>> >> >>> is usually messy, and will result in different feature sets (and different
>> >> >>> bugs) being implemented through each API, depending on the driver.
>> >> >>> Interactions between the APIs are also a pain point on the kernel side to
>> >> >>> properly synchronize calls.
>> >> > I don't like having two APIs either. Especially we shouldn't have multiple
>> >> > drivers implementing different APIs for the same device.
>> >> >
>> >> > That said, I wonder if it's possible to support camera-related use cases
>> >> > using the LED API: it's originally designed for quite different devices.
>> >> > Even if you could handle flash strobing using the LED API, the functionality
>> >> > provided by the Media controller and subdev APIs will always be missing:
>> >> > device enumeration and association with the right camera.
>> >> Is there a generic way to associate flash and camera subdevs in
>> >> current V4L2 API? The only ways I see now are:
>> >> - both belongs to the same media controller, but this is not enough if there
>> >> is more than one camera subdev in that controller,
>> >
>> > Yes, there is. That's the group_id field in struct media_entity_desc. The
>> > lens subdev is associated to the rest of the devices the same way.
>> >
>> >> - using media links/pads - at first sight it seems to be overkill/abuse...
>> >
>> > No. Links describe the flow of data, not relations between entities.
>> >
>> > ...
>> >
>> >> >>> The LED API is too limited for torch and flash usage, but I'm definitely open
>> >> >>> to moving flash devices to the LED API is we can extend it in a way that it
>> >> >>> covers all the use cases.
>> >> >>>
>> >> >> Extending LED API IMHO seems to be quite straightforward - by adding
>> >> >> attributes for supported functionalities. We just need a specification for
>> >> >> standard flash/torch attributes.
>> >> >> I could prepare an RFC about it if there is a will to explore this
>> >> >> direction.
>> >> > I'm leaning towards providing a wrapper that provides torch functionality
>> >> > using V4L2 flash API unless it's really proven to be insane. ;-) The code
>> >> > supporting that in an individual flash driver should be minimal --- which is
>> >> > what the patchset essentially already does.
>> >> Providing only torch functionality do not require adding new attributes
>> >> (besides the ones already present in the led_classdev), so the patch will
>> >> be much simpler.
>> >
>> > Yes. Attributes could be added later on to the LED API to support flash and
>> > the wrapper could be extended accordingly. My thinking is however that the
>> > main use case is torch, not strobing flash, so it would be fulfilled already
>> > without extensions to the LED API.
>> >
>>
>> Sorry for replying so late.
>>
>> I think Milo Kim did some work in our LED subsystem by add LED Flash
>> trigger for camera device. I agree it doesn't satisfy the usage of
>> V4L2 Flash API and what I'm thinking about is expanding the LED Flash
>> trigger driver to export a well defined sysfs interface, so user space
>> libv4l2 can wrap it for applications.
>
> libv4l2 currently provides a V4L2 API only, not V4L2 subdev API which is
> implemented by the existing flash drivers which in turn are used in embedded
> systems where the user space accesses V4L2 subdev API directly.
>
> So using libv4l2 could be workable but only in a subset of use cases for
> those chips (where regular V4L2 API is sufficient). I think we'd need to
> cover all to avoid writing double set of drivers.
>

I replied in another email thread, please move to there for discussion.

Thanks,
-Bryan