[PATCH v8 0/8] LED / flash API integration

[PATCH v8 0/8] LED / flash API integration

[Jacek Anaszewski]

This is eighth non-RFC version of LED / flash API integration
series [1]. It is based on linux_next-20150519 with patch [2].

================
Changes since v7
================

- Merged patches from Sakari for v4l2-flash-led-class and V4L2 related
  patches for leds-max77693 and leds-aat1290 drivers
- applied minor modifications to the both led drivers related patches
- modified exynos4-is media device to parse new samsung,flash-led
  property, instead of 'flashes' array
- added DT documentation for samsung,flash-led property

================
Changes since v5
================
- renamed v4l2-flash module to v4l2-flash-led-class and applied
  other related modifications spotted by Sakari
- fixed not released of_node reference in max77693-led driver

================
Changes since v4
================
- adapted leds-max77693 and leds-aat1290 drivers to the recent
  modifications in leds/common.txt bindings documentation and
  changed the behaviour when properties are missing
- modified DT bindings documenation for the aforementioned
  drivers
- removed unjustified use of goto in the leds-aat1290 driver
- fixed lack of of_node_put in leds-aat1290 driver, after parsing
  DT child node 
- removed patch adding 'skyworks' vendor prefix, as the entry
  has been recently added

================
Changes since v2
================
- improved leds/common DT bindings documentation
- improved max77693-led DT documentation
- fixed validation of DT confguration for leds-max77693 by
  minimal values in joint iouts case
- removed trigger-type property from leds-max77693 bindings
  and adjusted the driver accordingly
- improved LED Flash class documentation related to v4l2-flash sub-device
  initialization
- excluded from leds-aat1290 DT bindings documentation the part
  related to handling external strobe sources

================
Changes since v1
================

- excluded exynos4-is media device related patches, as there is
  consenus required related to flash devices handling in media device
  DT bindings
- modifications around LED Flash class settings and v4l2 flash config
  initialization in LED Flash class drivers and v4l2-flash wrapper
- switched to using DT node name as a device name for leds-max77693
  and leds-aat1290 drivers, in case DT 'label' property is absent
- dropped OF dependecy for v4l2-flash wrapper
- moved LED_FAULTS definitions from led-class-flash.h to uapi/linux/leds.h
- allowed for multiple clients of v4l2-flash sub-device

======================
Changes since RFC v13:
======================

- reduced number of patches - some of them have been merged
- slightly modified max77693-led device naming
- fixed issues in v4l2-flash helpers detected with yavta
- cleaned up AAT1290 device tree documentation
- added GPIOLIB dependecy to AAT1290 related entry in Kconfig

Thanks,
Jacek Anaszewski

[1] http://www.spinics.net/lists/kernel/msg1944538.html
[2] http://www.spinics.net/lists/linux-media/msg89839.html

Jacek Anaszewski (8):
  media: Add registration helpers for V4L2 flash sub-devices
  leds: max77693: add support for V4L2 Flash sub-device
  DT: aat1290: Document handling external strobe sources
  leds: aat1290: add support for V4L2 Flash sub-device
  exynos4-is: Add support for v4l2-flash subdevs
  DT: s5c73m3: Add documentation for samsung,flash-led property
  DT: s5k6a3: Add documentation for samsung,flash-led property
  DT: samsung-fimc: Add examples for samsung,flash-led property

 .../devicetree/bindings/leds/leds-aat1290.txt      |   36 +-
 .../devicetree/bindings/media/samsung-fimc.txt     |    4 +
 .../devicetree/bindings/media/samsung-s5c73m3.txt  |    5 +
 .../devicetree/bindings/media/samsung-s5k6a3.txt   |    4 +
 drivers/leds/Kconfig                               |    1 +
 drivers/leds/leds-aat1290.c                        |  137 +++-
 drivers/leds/leds-max77693.c                       |  129 +++-
 drivers/media/platform/exynos4-is/media-dev.c      |   39 +-
 drivers/media/platform/exynos4-is/media-dev.h      |   13 +-
 drivers/media/v4l2-core/Kconfig                    |   11 +
 drivers/media/v4l2-core/Makefile                   |    2 +
 drivers/media/v4l2-core/v4l2-flash-led-class.c     |  655 ++++++++++++++++++++
 include/media/v4l2-flash-led-class.h               |  139 +++++
 13 files changed, 1158 insertions(+), 17 deletions(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-flash-led-class.c
 create mode 100644 include/media/v4l2-flash-led-class.h

-- 
1.7.9.5

[PATCH v8 1/8] media: Add registration helpers for V4L2 flash sub-devices

[Jacek Anaszewski]

This patch adds helper functions for registering/unregistering
LED Flash class devices as V4L2 sub-devices. The functions should
be called from the LED subsystem device driver. In case the
support for V4L2 Flash sub-devices is disabled in the kernel
config the functions' empty versions will be used.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/Kconfig                |   11 +
 drivers/media/v4l2-core/Makefile               |    2 +
 drivers/media/v4l2-core/v4l2-flash-led-class.c |  655 ++++++++++++++++++++++++
 include/media/v4l2-flash-led-class.h           |  139 +++++
 4 files changed, 807 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-flash-led-class.c
 create mode 100644 include/media/v4l2-flash-led-class.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index f7a01a7..b4b0229 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -44,6 +44,17 @@ config V4L2_MEM2MEM_DEV
         tristate
         depends on VIDEOBUF2_CORE
 
+# Used by LED subsystem flash drivers
+config V4L2_FLASH_LED_CLASS
+	tristate "V4L2 flash API for LED flash class devices"
+	depends on VIDEO_V4L2_SUBDEV_API
+	depends on LEDS_CLASS_FLASH
+	---help---
+	  Say Y here to enable V4L2 flash API support for LED flash
+	  class drivers.
+
+	  When in doubt, say N.
+
 # 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 63d29f2..dc3de00 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -22,6 +22,8 @@ obj-$(CONFIG_VIDEO_TUNER) += tuner.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
 
+obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o
+
 obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
 obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
 obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c
new file mode 100644
index 0000000..17d7498
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
@@ -0,0 +1,655 @@
+/*
+ * V4L2 flash LED sub-device registration helpers.
+ *
+ *	Copyright (C) 2015 Samsung Electronics Co., Ltd
+ *	Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/led-class-flash.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <media/v4l2-flash-led-class.h>
+
+#define has_flash_op(v4l2_flash, op)				\
+	(v4l2_flash && v4l2_flash->ops->op)
+
+#define call_flash_op(v4l2_flash, op, arg)			\
+		(has_flash_op(v4l2_flash, op) ?			\
+			v4l2_flash->ops->op(v4l2_flash, arg) :	\
+			-EINVAL)
+
+enum ctrl_init_data_id {
+	LED_MODE,
+	TORCH_INTENSITY,
+	FLASH_INTENSITY,
+	INDICATOR_INTENSITY,
+	FLASH_TIMEOUT,
+	STROBE_SOURCE,
+	/*
+	 * Only above values are applicable to
+	 * the 'ctrls' array in the struct v4l2_flash.
+	 */
+	FLASH_STROBE,
+	STROBE_STOP,
+	STROBE_STATUS,
+	FLASH_FAULT,
+	NUM_FLASH_CTRLS,
+};
+
+static enum led_brightness __intensity_to_led_brightness(
+					struct v4l2_ctrl *ctrl, s32 intensity)
+{
+	intensity -= ctrl->minimum;
+	intensity /= (u32) ctrl->step;
+
+	/*
+	 * Indicator LEDs, unlike torch LEDs, are turned on/off basing on
+	 * the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
+	 * Therefore it must be possible to set it to 0 level which in
+	 * the LED subsystem reflects LED_OFF state.
+	 */
+	if (ctrl->minimum)
+		++intensity;
+
+	return intensity;
+}
+
+static s32 __led_brightness_to_intensity(struct v4l2_ctrl *ctrl,
+					 enum led_brightness brightness)
+{
+	/*
+	 * Indicator LEDs, unlike torch LEDs, are turned on/off basing on
+	 * the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
+	 * Do not decrement brightness read from the LED subsystem for
+	 * indicator LED as it may equal 0. For torch LEDs this function
+	 * is called only when V4L2_FLASH_LED_MODE_TORCH is set and the
+	 * brightness read is guaranteed to be greater than 0. In the mode
+	 * V4L2_FLASH_LED_MODE_NONE the cached torch intensity value is used.
+	 */
+	if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
+		--brightness;
+
+	return (brightness * ctrl->step) + ctrl->minimum;
+}
+
+static void v4l2_flash_set_led_brightness(struct v4l2_flash *v4l2_flash,
+					struct v4l2_ctrl *ctrl)
+{
+	struct v4l2_ctrl **ctrls = v4l2_flash->ctrls;
+	enum led_brightness brightness;
+
+	if (has_flash_op(v4l2_flash, intensity_to_led_brightness))
+		brightness = call_flash_op(v4l2_flash,
+					intensity_to_led_brightness,
+					ctrl->val);
+	else
+		brightness = __intensity_to_led_brightness(ctrl, ctrl->val);
+	/*
+	 * In case a LED Flash class driver provides ops for custom
+	 * brightness <-> intensity conversion, it also must have defined
+	 * related v4l2 control step == 1. In such a case a backward conversion
+	 * from led brightness to v4l2 intensity is required to find out the
+	 * the aligned intensity value.
+	 */
+	if (has_flash_op(v4l2_flash, led_brightness_to_intensity))
+		ctrl->val = call_flash_op(v4l2_flash,
+					led_brightness_to_intensity,
+					brightness);
+
+	if (ctrl == ctrls[TORCH_INTENSITY] &&
+	    ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_TORCH)
+		return;
+
+	led_set_brightness(&v4l2_flash->fled_cdev->led_cdev, brightness);
+}
+
+static int v4l2_flash_update_led_brightness(struct v4l2_flash *v4l2_flash,
+					struct v4l2_ctrl *ctrl)
+{
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	struct v4l2_ctrl **ctrls = v4l2_flash->ctrls;
+	int ret;
+
+	/*
+	 * Update torch brightness only if in TORCH_MODE. In other modes torch
+	 * led is turned off, which would spuriously inform the user space that
+	 * V4L2_CID_FLASH_TORCH_INTENSITY control value has changed to 0.
+	 */
+	if (ctrl == ctrls[TORCH_INTENSITY] &&
+	    ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_TORCH)
+		return 0;
+
+	ret = led_update_brightness(led_cdev);
+	if (ret < 0)
+		return ret;
+
+	if (has_flash_op(v4l2_flash, led_brightness_to_intensity))
+		ctrl->val = call_flash_op(v4l2_flash,
+						led_brightness_to_intensity,
+						led_cdev->brightness);
+	else
+		ctrl->val = __led_brightness_to_intensity(ctrl,
+						led_cdev->brightness);
+
+	return 0;
+}
+
+static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
+{
+	struct v4l2_flash *v4l2_flash = v4l2_ctrl_to_v4l2_flash(c);
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	bool is_strobing;
+	int ret;
+
+	switch (c->id) {
+	case V4L2_CID_FLASH_TORCH_INTENSITY:
+	case V4L2_CID_FLASH_INDICATOR_INTENSITY:
+		return v4l2_flash_update_led_brightness(v4l2_flash, c);
+	case V4L2_CID_FLASH_INTENSITY:
+		ret = led_update_flash_brightness(fled_cdev);
+		if (ret < 0)
+			return ret;
+		/*
+		 * No conversion is needed as LED Flash class also uses
+		 * microamperes for flash intensity units.
+		 */
+		c->val = fled_cdev->brightness.val;
+		return 0;
+	case V4L2_CID_FLASH_STROBE_STATUS:
+		ret = led_get_flash_strobe(fled_cdev, &is_strobing);
+		if (ret < 0)
+			return ret;
+		c->val = is_strobing;
+		return 0;
+	case V4L2_CID_FLASH_FAULT:
+		/* LED faults map directly to V4L2 flash faults */
+		return led_get_flash_fault(fled_cdev, &c->val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static bool __software_strobe_mode_inactive(struct v4l2_ctrl **ctrls)
+{
+	return ((ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_FLASH) ||
+		(ctrls[STROBE_SOURCE] && (ctrls[STROBE_SOURCE]->val !=
+				V4L2_FLASH_STROBE_SOURCE_SOFTWARE)));
+}
+
+static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
+{
+	struct v4l2_flash *v4l2_flash = v4l2_ctrl_to_v4l2_flash(c);
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	struct v4l2_ctrl **ctrls = v4l2_flash->ctrls;
+	bool external_strobe;
+	int ret = 0;
+
+	switch (c->id) {
+	case V4L2_CID_FLASH_LED_MODE:
+		switch (c->val) {
+		case V4L2_FLASH_LED_MODE_NONE:
+			led_set_brightness(led_cdev, LED_OFF);
+			return led_set_flash_strobe(fled_cdev, false);
+		case V4L2_FLASH_LED_MODE_FLASH:
+			/* Turn the torch LED off */
+			led_set_brightness(led_cdev, LED_OFF);
+			if (ctrls[STROBE_SOURCE]) {
+				external_strobe = (ctrls[STROBE_SOURCE]->val ==
+					V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
+
+				ret = call_flash_op(v4l2_flash,
+						external_strobe_set,
+						external_strobe);
+			}
+			return ret;
+		case V4L2_FLASH_LED_MODE_TORCH:
+			if (ctrls[STROBE_SOURCE]) {
+				ret = call_flash_op(v4l2_flash,
+						external_strobe_set,
+						false);
+				if (ret < 0)
+					return ret;
+			}
+			/* Stop flash strobing */
+			ret = led_set_flash_strobe(fled_cdev, false);
+			if (ret < 0)
+				return ret;
+
+			v4l2_flash_set_led_brightness(v4l2_flash,
+							ctrls[TORCH_INTENSITY]);
+			return 0;
+		}
+		break;
+	case V4L2_CID_FLASH_STROBE_SOURCE:
+		external_strobe = (c->val == V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
+		/*
+		 * For some hardware arrangements setting strobe source may
+		 * affect torch mode. Therefore, if not in the flash mode,
+		 * cache only this setting. It will be applied upon switching
+		 * to flash mode.
+		 */
+		if (ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_FLASH)
+			return 0;
+
+		return call_flash_op(v4l2_flash, external_strobe_set,
+					external_strobe);
+	case V4L2_CID_FLASH_STROBE:
+		if (__software_strobe_mode_inactive(ctrls))
+			return -EBUSY;
+		return led_set_flash_strobe(fled_cdev, true);
+	case V4L2_CID_FLASH_STROBE_STOP:
+		if (__software_strobe_mode_inactive(ctrls))
+			return -EBUSY;
+		return led_set_flash_strobe(fled_cdev, false);
+	case V4L2_CID_FLASH_TIMEOUT:
+		/*
+		 * No conversion is needed as LED Flash class also uses
+		 * microseconds for flash timeout units.
+		 */
+		return led_set_flash_timeout(fled_cdev, c->val);
+	case V4L2_CID_FLASH_INTENSITY:
+		/*
+		 * No conversion is needed as LED Flash class also uses
+		 * microamperes for flash intensity units.
+		 */
+		return led_set_flash_brightness(fled_cdev, c->val);
+	case V4L2_CID_FLASH_TORCH_INTENSITY:
+	case V4L2_CID_FLASH_INDICATOR_INTENSITY:
+		v4l2_flash_set_led_brightness(v4l2_flash, c);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops v4l2_flash_ctrl_ops = {
+	.g_volatile_ctrl = v4l2_flash_g_volatile_ctrl,
+	.s_ctrl = v4l2_flash_s_ctrl,
+};
+
+static void __lfs_to_v4l2_ctrl_config(struct led_flash_setting *s,
+				struct v4l2_ctrl_config *c)
+{
+	c->min = s->min;
+	c->max = s->max;
+	c->step = s->step;
+	c->def = s->val;
+}
+
+static void __fill_ctrl_init_data(struct v4l2_flash *v4l2_flash,
+			  struct v4l2_flash_config *flash_cfg,
+			  struct v4l2_flash_ctrl_data *ctrl_init_data)
+{
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	const struct led_flash_ops *fled_cdev_ops = fled_cdev->ops;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	struct v4l2_ctrl_config *ctrl_cfg;
+	u32 mask;
+
+	/* Init FLASH_FAULT ctrl data */
+	if (flash_cfg->flash_faults) {
+		ctrl_init_data[FLASH_FAULT].cid = V4L2_CID_FLASH_FAULT;
+		ctrl_cfg = &ctrl_init_data[FLASH_FAULT].config;
+		ctrl_cfg->id = V4L2_CID_FLASH_FAULT;
+		ctrl_cfg->max = flash_cfg->flash_faults;
+		ctrl_cfg->flags = V4L2_CTRL_FLAG_VOLATILE |
+				  V4L2_CTRL_FLAG_READ_ONLY;
+	}
+
+	/* Init INDICATOR_INTENSITY ctrl data */
+	if (flash_cfg->indicator_led) {
+		ctrl_init_data[INDICATOR_INTENSITY].cid =
+					V4L2_CID_FLASH_INDICATOR_INTENSITY;
+		ctrl_cfg = &ctrl_init_data[INDICATOR_INTENSITY].config;
+		__lfs_to_v4l2_ctrl_config(&flash_cfg->intensity, ctrl_cfg);
+		ctrl_cfg->id = V4L2_CID_FLASH_INDICATOR_INTENSITY;
+		ctrl_cfg->min = 0;
+		ctrl_cfg->flags = V4L2_CTRL_FLAG_VOLATILE |
+				  V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
+
+		/* Indicator LED can have only faults and intensity controls. */
+		return;
+	}
+
+	/* Init FLASH_LED_MODE ctrl data */
+	mask = 1 << V4L2_FLASH_LED_MODE_NONE |
+	       1 << V4L2_FLASH_LED_MODE_TORCH;
+	if (led_cdev->flags & LED_DEV_CAP_FLASH)
+		mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
+
+	ctrl_init_data[LED_MODE].cid = V4L2_CID_FLASH_LED_MODE;
+	ctrl_cfg = &ctrl_init_data[LED_MODE].config;
+	ctrl_cfg->id = V4L2_CID_FLASH_LED_MODE;
+	ctrl_cfg->max = V4L2_FLASH_LED_MODE_TORCH;
+	ctrl_cfg->menu_skip_mask = ~mask;
+	ctrl_cfg->def = V4L2_FLASH_LED_MODE_NONE;
+	ctrl_cfg->flags = 0;
+
+	/* Init TORCH_INTENSITY ctrl data */
+	ctrl_init_data[TORCH_INTENSITY].cid = V4L2_CID_FLASH_TORCH_INTENSITY;
+	ctrl_cfg = &ctrl_init_data[TORCH_INTENSITY].config;
+	__lfs_to_v4l2_ctrl_config(&flash_cfg->intensity, ctrl_cfg);
+	ctrl_cfg->id = V4L2_CID_FLASH_TORCH_INTENSITY;
+	ctrl_cfg->flags = V4L2_CTRL_FLAG_VOLATILE |
+			  V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
+
+	if (!(led_cdev->flags & LED_DEV_CAP_FLASH))
+		return;
+
+	/* Init FLASH_STROBE ctrl data */
+	ctrl_init_data[FLASH_STROBE].cid = V4L2_CID_FLASH_STROBE;
+	ctrl_cfg = &ctrl_init_data[FLASH_STROBE].config;
+	ctrl_cfg->id = V4L2_CID_FLASH_STROBE;
+
+	/* Init STROBE_STOP ctrl data */
+	ctrl_init_data[STROBE_STOP].cid = V4L2_CID_FLASH_STROBE_STOP;
+	ctrl_cfg = &ctrl_init_data[STROBE_STOP].config;
+	ctrl_cfg->id = V4L2_CID_FLASH_STROBE_STOP;
+
+	/* Init FLASH_STROBE_SOURCE ctrl data */
+	if (flash_cfg->has_external_strobe) {
+		mask = (1 << V4L2_FLASH_STROBE_SOURCE_SOFTWARE) |
+		       (1 << V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
+		ctrl_init_data[STROBE_SOURCE].cid =
+					V4L2_CID_FLASH_STROBE_SOURCE;
+		ctrl_cfg = &ctrl_init_data[STROBE_SOURCE].config;
+		ctrl_cfg->id = V4L2_CID_FLASH_STROBE_SOURCE;
+		ctrl_cfg->max = V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
+		ctrl_cfg->menu_skip_mask = ~mask;
+		ctrl_cfg->def = V4L2_FLASH_STROBE_SOURCE_SOFTWARE;
+	}
+
+	/* Init STROBE_STATUS ctrl data */
+	if (fled_cdev_ops->strobe_get) {
+		ctrl_init_data[STROBE_STATUS].cid =
+					V4L2_CID_FLASH_STROBE_STATUS;
+		ctrl_cfg = &ctrl_init_data[STROBE_STATUS].config;
+		ctrl_cfg->id = V4L2_CID_FLASH_STROBE_STATUS;
+		ctrl_cfg->flags = V4L2_CTRL_FLAG_VOLATILE |
+				  V4L2_CTRL_FLAG_READ_ONLY;
+	}
+
+	/* Init FLASH_TIMEOUT ctrl data */
+	if (fled_cdev_ops->timeout_set) {
+		ctrl_init_data[FLASH_TIMEOUT].cid = V4L2_CID_FLASH_TIMEOUT;
+		ctrl_cfg = &ctrl_init_data[FLASH_TIMEOUT].config;
+		__lfs_to_v4l2_ctrl_config(&fled_cdev->timeout, ctrl_cfg);
+		ctrl_cfg->id = V4L2_CID_FLASH_TIMEOUT;
+	}
+
+	/* Init FLASH_INTENSITY ctrl data */
+	if (fled_cdev_ops->flash_brightness_set) {
+		ctrl_init_data[FLASH_INTENSITY].cid = V4L2_CID_FLASH_INTENSITY;
+		ctrl_cfg = &ctrl_init_data[FLASH_INTENSITY].config;
+		__lfs_to_v4l2_ctrl_config(&fled_cdev->brightness, ctrl_cfg);
+		ctrl_cfg->id = V4L2_CID_FLASH_INTENSITY;
+		ctrl_cfg->flags = V4L2_CTRL_FLAG_VOLATILE |
+				  V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
+	}
+}
+
+static int v4l2_flash_init_controls(struct v4l2_flash *v4l2_flash,
+				struct v4l2_flash_config *flash_cfg)
+
+{
+	struct v4l2_flash_ctrl_data *ctrl_init_data;
+	struct v4l2_ctrl *ctrl;
+	struct v4l2_ctrl_config *ctrl_cfg;
+	int i, ret, num_ctrls = 0;
+
+	v4l2_flash->ctrls = devm_kzalloc(v4l2_flash->sd.dev,
+					sizeof(*v4l2_flash->ctrls) *
+					(STROBE_SOURCE + 1), GFP_KERNEL);
+	if (!v4l2_flash->ctrls)
+		return -ENOMEM;
+
+	/* allocate memory dynamically so as not to exceed stack frame size */
+	ctrl_init_data = kcalloc(NUM_FLASH_CTRLS, sizeof(*ctrl_init_data),
+					GFP_KERNEL);
+	if (!ctrl_init_data)
+		return -ENOMEM;
+
+	__fill_ctrl_init_data(v4l2_flash, flash_cfg, ctrl_init_data);
+
+	for (i = 0; i < NUM_FLASH_CTRLS; ++i)
+		if (ctrl_init_data[i].cid)
+			++num_ctrls;
+
+	v4l2_ctrl_handler_init(&v4l2_flash->hdl, num_ctrls);
+
+	for (i = 0; i < NUM_FLASH_CTRLS; ++i) {
+		ctrl_cfg = &ctrl_init_data[i].config;
+		if (!ctrl_init_data[i].cid)
+			continue;
+
+		if (ctrl_cfg->id == V4L2_CID_FLASH_LED_MODE ||
+		    ctrl_cfg->id == V4L2_CID_FLASH_STROBE_SOURCE)
+			ctrl = v4l2_ctrl_new_std_menu(&v4l2_flash->hdl,
+						&v4l2_flash_ctrl_ops,
+						ctrl_cfg->id,
+						ctrl_cfg->max,
+						ctrl_cfg->menu_skip_mask,
+						ctrl_cfg->def);
+		else
+			ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl,
+						&v4l2_flash_ctrl_ops,
+						ctrl_cfg->id,
+						ctrl_cfg->min,
+						ctrl_cfg->max,
+						ctrl_cfg->step,
+						ctrl_cfg->def);
+
+		if (ctrl)
+			ctrl->flags |= ctrl_cfg->flags;
+
+		if (i <= STROBE_SOURCE)
+			v4l2_flash->ctrls[i] = ctrl;
+	}
+
+	kfree(ctrl_init_data);
+
+	if (v4l2_flash->hdl.error) {
+		ret = v4l2_flash->hdl.error;
+		goto error_free_handler;
+	}
+
+	v4l2_ctrl_handler_setup(&v4l2_flash->hdl);
+
+	v4l2_flash->sd.ctrl_handler = &v4l2_flash->hdl;
+
+	return 0;
+
+error_free_handler:
+	v4l2_ctrl_handler_free(&v4l2_flash->hdl);
+	return ret;
+}
+
+static int __sync_device_with_v4l2_controls(struct v4l2_flash *v4l2_flash)
+{
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct v4l2_ctrl **ctrls = v4l2_flash->ctrls;
+	int ret = 0;
+
+	if (ctrls[INDICATOR_INTENSITY]) {
+		v4l2_flash_set_led_brightness(v4l2_flash,
+						ctrls[INDICATOR_INTENSITY]);
+		return 0;
+	}
+
+	v4l2_flash_set_led_brightness(v4l2_flash, ctrls[TORCH_INTENSITY]);
+
+	if (ctrls[FLASH_TIMEOUT]) {
+		ret = led_set_flash_timeout(fled_cdev,
+					ctrls[FLASH_TIMEOUT]->val);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (ctrls[FLASH_INTENSITY]) {
+		ret = led_set_flash_brightness(fled_cdev,
+					ctrls[FLASH_INTENSITY]->val);
+		if (ret < 0)
+			return ret;
+	}
+
+	/*
+	 * For some hardware arrangements setting strobe source may affect
+	 * torch mode. Synchronize strobe source setting only if not in torch
+	 * mode. For torch mode case it will get synchronized upon switching
+	 * to flash mode.
+	 */
+	if (ctrls[STROBE_SOURCE] &&
+	    ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_TORCH)
+		ret = call_flash_op(v4l2_flash, external_strobe_set,
+					ctrls[STROBE_SOURCE]->val);
+
+	return ret;
+}
+
+/*
+ * V4L2 subdev internal operations
+ */
+
+static int v4l2_flash_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd);
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	int ret = 0;
+
+	mutex_lock(&led_cdev->led_access);
+
+	if (!v4l2_fh_is_singular(&fh->vfh))
+		goto unlock;
+
+	led_sysfs_disable(led_cdev);
+	led_trigger_remove(led_cdev);
+
+	ret = __sync_device_with_v4l2_controls(v4l2_flash);
+
+unlock:
+	mutex_unlock(&led_cdev->led_access);
+	return ret;
+}
+
+static int v4l2_flash_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd);
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	int ret = 0;
+
+	mutex_lock(&led_cdev->led_access);
+
+	if (v4l2_fh_is_singular(&fh->vfh)) {
+		if (v4l2_flash->ctrls[STROBE_SOURCE])
+			ret = v4l2_ctrl_s_ctrl(v4l2_flash->ctrls[STROBE_SOURCE],
+					V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
+		led_sysfs_enable(led_cdev);
+	}
+
+	mutex_unlock(&led_cdev->led_access);
+
+	return ret;
+}
+
+static const struct v4l2_subdev_internal_ops v4l2_flash_subdev_internal_ops = {
+	.open = v4l2_flash_open,
+	.close = v4l2_flash_close,
+};
+
+static const struct v4l2_subdev_core_ops v4l2_flash_core_ops = {
+	.queryctrl = v4l2_subdev_queryctrl,
+	.querymenu = v4l2_subdev_querymenu,
+};
+
+static const struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
+	.core = &v4l2_flash_core_ops,
+};
+
+struct v4l2_flash *v4l2_flash_init(
+	struct device *dev, struct device_node *of_node,
+	struct led_classdev_flash *fled_cdev, const struct v4l2_flash_ops *ops,
+	struct v4l2_flash_config *config)
+{
+	struct v4l2_flash *v4l2_flash;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	if (!fled_cdev || !ops || !config)
+		return ERR_PTR(-EINVAL);
+
+	v4l2_flash = devm_kzalloc(led_cdev->dev, sizeof(*v4l2_flash),
+					GFP_KERNEL);
+	if (!v4l2_flash)
+		return ERR_PTR(-ENOMEM);
+
+	sd = &v4l2_flash->sd;
+	v4l2_flash->fled_cdev = fled_cdev;
+	v4l2_flash->ops = ops;
+	sd->dev = dev;
+	sd->of_node = of_node;
+	v4l2_subdev_init(sd, &v4l2_flash_subdev_ops);
+	sd->internal_ops = &v4l2_flash_subdev_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	strlcpy(sd->name, config->dev_name, sizeof(sd->name));
+
+	ret = media_entity_init(&sd->entity, 0, NULL, 0);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
+
+	ret = v4l2_flash_init_controls(v4l2_flash, config);
+	if (ret < 0)
+		goto err_init_controls;
+
+	of_node_get(led_cdev->dev->of_node);
+
+	ret = v4l2_async_register_subdev(sd);
+	if (ret < 0)
+		goto err_async_register_sd;
+
+	return v4l2_flash;
+
+err_async_register_sd:
+	of_node_put(led_cdev->dev->of_node);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+err_init_controls:
+	media_entity_cleanup(&sd->entity);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(v4l2_flash_init);
+
+void v4l2_flash_release(struct v4l2_flash *v4l2_flash)
+{
+	struct v4l2_subdev *sd;
+	struct led_classdev *led_cdev;
+
+	if (IS_ERR(v4l2_flash))
+		return;
+
+	sd = &v4l2_flash->sd;
+	led_cdev = &v4l2_flash->fled_cdev->led_cdev;
+
+	v4l2_async_unregister_subdev(sd);
+	of_node_put(led_cdev->dev->of_node);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+	media_entity_cleanup(&sd->entity);
+}
+EXPORT_SYMBOL_GPL(v4l2_flash_release);
+
+MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
+MODULE_DESCRIPTION("V4L2 Flash sub-device helpers");
+MODULE_LICENSE("GPL v2");
diff --git a/include/media/v4l2-flash-led-class.h b/include/media/v4l2-flash-led-class.h
new file mode 100644
index 0000000..5c8389a
--- /dev/null
+++ b/include/media/v4l2-flash-led-class.h
@@ -0,0 +1,139 @@
+/*
+ * V4L2 flash LED sub-device registration helpers.
+ *
+ *	Copyright (C) 2015 Samsung Electronics Co., Ltd
+ *	Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _V4L2_FLASH_H
+#define _V4L2_FLASH_H
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-subdev.h>
+
+struct led_classdev_flash;
+struct led_classdev;
+struct v4l2_flash;
+enum led_brightness;
+
+/*
+ * struct v4l2_flash_ctrl_data - flash control initialization data, filled
+ *				basing on the features declared by the LED Flash
+ *				class driver in the v4l2_flash_config
+ * @config:	initialization data for a control
+ * @cid:	contains v4l2 flash control id if the config
+ *		field was initialized, 0 otherwise
+ */
+struct v4l2_flash_ctrl_data {
+	struct v4l2_ctrl_config config;
+	u32 cid;
+};
+
+struct v4l2_flash_ops {
+	/* setup strobing the flash by hardware pin state assertion */
+	int (*external_strobe_set)(struct v4l2_flash *v4l2_flash,
+					bool enable);
+	/* convert intensity to brightness in a device specific manner */
+	enum led_brightness (*intensity_to_led_brightness)
+		(struct v4l2_flash *v4l2_flash, s32 intensity);
+	/* convert brightness to intensity in a device specific manner */
+	s32 (*led_brightness_to_intensity)
+		(struct v4l2_flash *v4l2_flash, enum led_brightness);
+};
+
+/**
+ * struct v4l2_flash_config - V4L2 Flash sub-device initialization data
+ * @dev_name:			the name of the media entity,
+				unique in the system
+ * @intensity:			constraints for the LED in a non-flash mode
+ * @flash_faults:		bitmask of flash faults that the LED Flash class
+				device can report; corresponding LED_FAULT* bit
+				definitions are available in the header file
+				<linux/led-class-flash.h>
+ * @has_external_strobe:	external strobe capability
+ * @indicator_led:		signifies that a led is of indicator type
+ */
+struct v4l2_flash_config {
+	char dev_name[32];
+	struct led_flash_setting intensity;
+	u32 flash_faults;
+	unsigned int has_external_strobe:1;
+	unsigned int indicator_led:1;
+};
+
+/**
+ * struct v4l2_flash - Flash sub-device context
+ * @fled_cdev:		LED Flash class device controlled by this sub-device
+ * @ops:		V4L2 specific flash ops
+ * @sd:			V4L2 sub-device
+ * @hdl:		flash controls handler
+ * @ctrls:		array of pointers to controls, whose values define
+ *			the sub-device state
+ */
+struct v4l2_flash {
+	struct led_classdev_flash *fled_cdev;
+	const struct v4l2_flash_ops *ops;
+
+	struct v4l2_subdev sd;
+	struct v4l2_ctrl_handler hdl;
+	struct v4l2_ctrl **ctrls;
+};
+
+static inline struct v4l2_flash *v4l2_subdev_to_v4l2_flash(
+							struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct v4l2_flash, sd);
+}
+
+static inline struct v4l2_flash *v4l2_ctrl_to_v4l2_flash(struct v4l2_ctrl *c)
+{
+	return container_of(c->handler, struct v4l2_flash, hdl);
+}
+
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+/**
+ * v4l2_flash_init - initialize V4L2 flash led sub-device
+ * @dev:	flash device, e.g. an I2C device
+ * @of_node:	of_node of the LED, may be NULL if the same as device's
+ * @fled_cdev:	the LED Flash class device to wrap
+ * @flash_ops:	V4L2 Flash device ops
+ * @config:	initialization data for V4L2 Flash sub-device
+ *
+ * Create V4L2 Flash sub-device wrapping given LED subsystem device.
+ *
+ * Returns: A valid pointer, or, when an error occurs, the return
+ * value is encoded using ERR_PTR(). Use IS_ERR() to check and
+ * PTR_ERR() to obtain the numeric return value.
+ */
+struct v4l2_flash *v4l2_flash_init(
+	struct device *dev, struct device_node *of_node,
+	struct led_classdev_flash *fled_cdev, const struct v4l2_flash_ops *ops,
+	struct v4l2_flash_config *config);
+
+/**
+ * v4l2_flash_release - release V4L2 Flash sub-device
+ * @flash: the V4L2 Flash sub-device to release
+ *
+ * Release V4L2 Flash sub-device.
+ */
+void v4l2_flash_release(struct v4l2_flash *v4l2_flash);
+
+#else
+static inline struct v4l2_flash *v4l2_flash_init(
+	struct device *dev, struct device_node *of_node,
+	struct led_classdev_flash *fled_cdev, const struct v4l2_flash_ops *ops,
+	struct v4l2_flash_config *config)
+{
+	return NULL;
+}
+
+static inline void v4l2_flash_release(struct v4l2_flash *v4l2_flash)
+{
+}
+#endif /* CONFIG_V4L2_FLASH_LED_CLASS */
+
+#endif /* _V4L2_FLASH_H */
-- 
1.7.9.5

[PATCH v8 2/8] leds: max77693: add support for V4L2 Flash sub-device

[Jacek Anaszewski]

Add support for V4L2 Flash sub-device to the max77693 LED Flash class
driver. The support allows for V4L2 Flash sub-device to take the control
of the LED Flash class device.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/leds/leds-max77693.c |  129 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 123 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
index eecaa92..4fcd7f0 100644
--- a/drivers/leds/leds-max77693.c
+++ b/drivers/leds/leds-max77693.c
@@ -20,6 +20,7 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <media/v4l2-flash-led-class.h>
 
 #define MODE_OFF		0
 #define MODE_FLASH(a)		(1 << (a))
@@ -62,6 +63,8 @@ struct max77693_sub_led {
 	struct led_classdev_flash fled_cdev;
 	/* assures led-triggers compatibility */
 	struct work_struct work_brightness_set;
+	/* V4L2 Flash device */
+	struct v4l2_flash *v4l2_flash;
 
 	/* brightness cache */
 	unsigned int torch_brightness;
@@ -627,7 +630,8 @@ static int max77693_led_flash_timeout_set(
 }
 
 static int max77693_led_parse_dt(struct max77693_led_device *led,
-				struct max77693_led_config_data *cfg)
+				struct max77693_led_config_data *cfg,
+				struct device_node **sub_nodes)
 {
 	struct device *dev = &led->pdev->dev;
 	struct max77693_sub_led *sub_leds = led->sub_leds;
@@ -674,6 +678,13 @@ static int max77693_led_parse_dt(struct max77693_led_device *led,
 			return -EINVAL;
 		}
 
+		if (sub_nodes[fled_id]) {
+			dev_err(dev,
+				"Conflicting \"led-sources\" DT properties\n");
+			return -EINVAL;
+		}
+
+		sub_nodes[fled_id] = child_node;
 		sub_leds[fled_id].fled_id = fled_id;
 
 		cfg->label[fled_id] =
@@ -786,11 +797,12 @@ static void max77693_led_validate_configuration(struct max77693_led_device *led,
 }
 
 static int max77693_led_get_configuration(struct max77693_led_device *led,
-				struct max77693_led_config_data *cfg)
+				struct max77693_led_config_data *cfg,
+				struct device_node **sub_nodes)
 {
 	int ret;
 
-	ret = max77693_led_parse_dt(led, cfg);
+	ret = max77693_led_parse_dt(led, cfg, sub_nodes);
 	if (ret < 0)
 		return ret;
 
@@ -838,6 +850,71 @@ static void max77693_init_flash_settings(struct max77693_sub_led *sub_led,
 	setting->val = setting->max;
 }
 
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+
+static int max77693_led_external_strobe_set(
+				struct v4l2_flash *v4l2_flash,
+				bool enable)
+{
+	struct max77693_sub_led *sub_led =
+				flcdev_to_sub_led(v4l2_flash->fled_cdev);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	int fled_id = sub_led->fled_id;
+	int ret;
+
+	mutex_lock(&led->lock);
+
+	if (enable)
+		ret = max77693_add_mode(led, MODE_FLASH_EXTERNAL(fled_id));
+	else
+		ret = max77693_clear_mode(led, MODE_FLASH_EXTERNAL(fled_id));
+
+	mutex_unlock(&led->lock);
+
+	return ret;
+}
+
+static void max77693_init_v4l2_flash_config(struct max77693_sub_led *sub_led,
+				struct max77693_led_config_data *led_cfg,
+				struct v4l2_flash_config *v4l2_sd_cfg)
+{
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	struct device *dev = &led->pdev->dev;
+	struct max77693_dev *iodev = dev_get_drvdata(dev->parent);
+	struct i2c_client *i2c = iodev->i2c;
+	struct led_flash_setting *s;
+
+	snprintf(v4l2_sd_cfg->dev_name, sizeof(v4l2_sd_cfg->dev_name),
+		 "%s %d-%04x", sub_led->fled_cdev.led_cdev.name,
+		 i2c_adapter_id(i2c->adapter), i2c->addr);
+
+	s = &v4l2_sd_cfg->intensity;
+	s->min = TORCH_IOUT_MIN;
+	s->max = sub_led->fled_cdev.led_cdev.max_brightness * TORCH_IOUT_STEP;
+	s->step = TORCH_IOUT_STEP;
+	s->val = s->max;
+
+	/* Init flash faults config */
+	v4l2_sd_cfg->flash_faults = LED_FAULT_OVER_VOLTAGE |
+				LED_FAULT_SHORT_CIRCUIT |
+				LED_FAULT_OVER_CURRENT;
+
+	v4l2_sd_cfg->has_external_strobe = true;
+}
+
+static const struct v4l2_flash_ops v4l2_flash_ops = {
+	.external_strobe_set = max77693_led_external_strobe_set,
+};
+#else
+static inline void max77693_init_v4l2_flash_config(
+				struct max77693_sub_led *sub_led,
+				struct max77693_led_config_data *led_cfg,
+				struct v4l2_flash_config *v4l2_sd_cfg)
+{
+}
+static const struct v4l2_flash_ops v4l2_flash_ops;
+#endif
+
 static void max77693_init_fled_cdev(struct max77693_sub_led *sub_led,
 				struct max77693_led_config_data *led_cfg)
 {
@@ -870,12 +947,45 @@ static void max77693_init_fled_cdev(struct max77693_sub_led *sub_led,
 	sub_led->flash_timeout = fled_cdev->timeout.val;
 }
 
+static int max77693_register_led(struct max77693_sub_led *sub_led,
+				 struct max77693_led_config_data *led_cfg,
+				 struct device_node *sub_node)
+{
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	struct led_classdev_flash *fled_cdev = &sub_led->fled_cdev;
+	struct device *dev = &led->pdev->dev;
+	struct v4l2_flash_config v4l2_sd_cfg = {};
+	int ret;
+
+	/* Register in the LED subsystem */
+	ret = led_classdev_flash_register(dev, fled_cdev);
+	if (ret < 0)
+		return ret;
+
+	max77693_init_v4l2_flash_config(sub_led, led_cfg, &v4l2_sd_cfg);
+
+	/* Register in the V4L2 subsystem. */
+	sub_led->v4l2_flash = v4l2_flash_init(dev, sub_node, fled_cdev,
+					      &v4l2_flash_ops, &v4l2_sd_cfg);
+	if (IS_ERR(sub_led->v4l2_flash)) {
+		ret = PTR_ERR(sub_led->v4l2_flash);
+		goto err_v4l2_flash_init;
+	}
+
+	return 0;
+
+err_v4l2_flash_init:
+	led_classdev_flash_unregister(fled_cdev);
+	return ret;
+}
+
 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_device *led;
 	struct max77693_sub_led *sub_leds;
+	struct device_node *sub_nodes[2] = {};
 	struct max77693_led_config_data led_cfg = {};
 	int init_fled_cdev[2], i, ret;
 
@@ -889,7 +999,7 @@ static int max77693_led_probe(struct platform_device *pdev)
 	sub_leds = led->sub_leds;
 
 	platform_set_drvdata(pdev, led);
-	ret = max77693_led_get_configuration(led, &led_cfg);
+	ret = max77693_led_get_configuration(led, &led_cfg, sub_nodes);
 	if (ret < 0)
 		return ret;
 
@@ -911,8 +1021,12 @@ static int max77693_led_probe(struct platform_device *pdev)
 		/* Initialize LED Flash class device */
 		max77693_init_fled_cdev(&sub_leds[i], &led_cfg);
 
-		/* Register LED Flash class device */
-		ret = led_classdev_flash_register(dev, &sub_leds[i].fled_cdev);
+		/*
+		 * Register LED Flash class device and corresponding
+		 * V4L2 Flash device.
+		 */
+		ret = max77693_register_led(&sub_leds[i], &led_cfg,
+						sub_nodes[i]);
 		if (ret < 0) {
 			/*
 			 * At this moment FLED1 might have been already
@@ -931,6 +1045,7 @@ err_register_led2:
 	/* It is possible than only FLED2 was to be registered */
 	if (!init_fled_cdev[FLED1])
 		goto err_register_led1;
+	v4l2_flash_release(sub_leds[FLED1].v4l2_flash);
 	led_classdev_flash_unregister(&sub_leds[FLED1].fled_cdev);
 err_register_led1:
 	mutex_destroy(&led->lock);
@@ -944,11 +1059,13 @@ static int max77693_led_remove(struct platform_device *pdev)
 	struct max77693_sub_led *sub_leds = led->sub_leds;
 
 	if (led->iout_joint || max77693_fled_used(led, FLED1)) {
+		v4l2_flash_release(sub_leds[FLED1].v4l2_flash);
 		led_classdev_flash_unregister(&sub_leds[FLED1].fled_cdev);
 		cancel_work_sync(&sub_leds[FLED1].work_brightness_set);
 	}
 
 	if (!led->iout_joint && max77693_fled_used(led, FLED2)) {
+		v4l2_flash_release(sub_leds[FLED2].v4l2_flash);
 		led_classdev_flash_unregister(&sub_leds[FLED2].fled_cdev);
 		cancel_work_sync(&sub_leds[FLED2].work_brightness_set);
 	}
-- 
1.7.9.5

[PATCH v8 3/8] DT: aat1290: Document handling external strobe sources

[Jacek Anaszewski]

This patch adds documentation for a pinctrl-names property.
The property, when present, is used for switching the source
of the strobe signal for the device.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/leds/leds-aat1290.txt      |   36 ++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-aat1290.txt b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
index ef88b9c..c05ed91 100644
--- a/Documentation/devicetree/bindings/leds/leds-aat1290.txt
+++ b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
@@ -2,7 +2,9 @@
 
 The device is controlled through two pins: FL_EN and EN_SET. The pins when,
 asserted high, enable flash strobe and movie mode (max 1/2 of flash current)
-respectively.
+respectively. In order to add a capability of selecting the strobe signal source
+(e.g. CPU or camera sensor) there is an additional switch required, independent
+of the flash chip. The switch is controlled with pin control.
 
 Required properties:
 
@@ -10,6 +12,13 @@ Required properties:
 - flen-gpios : Must be device tree identifier of the flash device FL_EN pin.
 - enset-gpios : Must be device tree identifier of the flash device EN_SET pin.
 
+Optional properties:
+- pinctrl-names : Must contain entries: "default", "host", "isp". Entries
+		"default" and "host" must refer to the same pin configuration
+		node, which sets the host as a strobe signal provider. Entry
+		"isp" must refer to the pin configuration node, which sets the
+		ISP as a strobe signal provider.
+
 A discrete LED element connected to the device must be represented by a child
 node - see Documentation/devicetree/bindings/leds/common.txt.
 
@@ -25,13 +34,22 @@ Required properties of the LED child node:
 Optional properties of the LED child node:
 - label : see Documentation/devicetree/bindings/leds/common.txt
 
-Example (by Ct = 220nF, Rset = 160kohm):
+Example (by Ct = 220nF, Rset = 160kohm and exynos4412-trats2 board with
+a switch that allows for routing strobe signal either from the host or from
+the camera sensor):
+
+#include "exynos4412.dtsi"
 
 aat1290 {
 	compatible = "skyworks,aat1290";
 	flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
 	enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
 
+	pinctrl-names = "default", "host", "isp";
+	pinctrl-0 = <&camera_flash_host>;
+	pinctrl-1 = <&camera_flash_host>;
+	pinctrl-2 = <&camera_flash_isp>;
+
 	camera_flash: flash-led {
 		label = "aat1290-flash";
 		led-max-microamp = <520833>;
@@ -39,3 +57,17 @@ aat1290 {
 		flash-timeout-us = <1940000>;
 	};
 };
+
+&pinctrl_0 {
+	camera_flash_host: camera-flash-host {
+		samsung,pins = "gpj1-0";
+		samsung,pin-function = <1>;
+		samsung,pin-val = <0>;
+	};
+
+	camera_flash_isp: camera-flash-isp {
+		samsung,pins = "gpj1-0";
+		samsung,pin-function = <1>;
+		samsung,pin-val = <1>;
+	};
+};
-- 
1.7.9.5

[PATCH v8 4/8] leds: aat1290: add support for V4L2 Flash sub-device

[Jacek Anaszewski]

Add support for V4L2 Flash sub-device to the aat1290 LED Flash class
driver. The support allows for V4L2 Flash sub-device to take the control
of the LED Flash class device.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/leds/Kconfig        |    1 +
 drivers/leds/leds-aat1290.c |  137 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 132 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index ec26dd1..244815e 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -47,6 +47,7 @@ config LEDS_AAT1290
 	depends on LEDS_CLASS_FLASH
 	depends on GPIOLIB
 	depends on OF
+	depends on PINCTRL
 	help
 	 This option enables support for the LEDs on the AAT1290.
 
diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index 6ea1d54..7ea676e 100644
--- a/drivers/leds/leds-aat1290.c
+++ b/drivers/leds/leds-aat1290.c
@@ -17,9 +17,11 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <media/v4l2-flash-led-class.h>
 
 #define AAT1290_MOVIE_MODE_CURRENT_ADDR	17
 #define AAT1290_MAX_MM_CURR_PERCENT_0	16
@@ -52,6 +54,8 @@ struct aat1290_led_config_data {
 	u32 max_flash_current;
 	/* maximum flash timeout */
 	u32 max_flash_tm;
+	/* external strobe capability */
+	bool has_external_strobe;
 	/* max LED brightness level */
 	enum led_brightness max_brightness;
 };
@@ -64,6 +68,8 @@ struct aat1290_led {
 
 	/* corresponding LED Flash class device */
 	struct led_classdev_flash fled_cdev;
+	/* V4L2 Flash device */
+	struct v4l2_flash *v4l2_flash;
 
 	/* FLEN pin */
 	struct gpio_desc *gpio_fl_en;
@@ -230,11 +236,15 @@ static int aat1290_led_flash_timeout_set(struct led_classdev_flash *fled_cdev,
 }
 
 static int aat1290_led_parse_dt(struct aat1290_led *led,
-			struct aat1290_led_config_data *cfg)
+			struct aat1290_led_config_data *cfg,
+			struct device_node **sub_node)
 {
 	struct led_classdev *led_cdev = &led->fled_cdev.led_cdev;
 	struct device *dev = &led->pdev->dev;
 	struct device_node *child_node;
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+	struct pinctrl *pinctrl;
+#endif
 	int ret = 0;
 
 	led->gpio_fl_en = devm_gpiod_get(dev, "flen");
@@ -251,6 +261,17 @@ static int aat1290_led_parse_dt(struct aat1290_led *led,
 		return ret;
 	}
 
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+	pinctrl = devm_pinctrl_get_select_default(&led->pdev->dev);
+	if (IS_ERR(pinctrl)) {
+		cfg->has_external_strobe = false;
+		dev_info(dev,
+			 "No support for external strobe detected.\n");
+	} else {
+		cfg->has_external_strobe = true;
+	}
+#endif
+
 	child_node = of_get_next_available_child(dev->of_node, NULL);
 	if (!child_node) {
 		dev_err(dev, "No DT child node found for connected LED.\n");
@@ -288,6 +309,8 @@ static int aat1290_led_parse_dt(struct aat1290_led *led,
 
 	of_node_put(child_node);
 
+	*sub_node = child_node;
+
 	return ret;
 }
 
@@ -316,7 +339,8 @@ int init_mm_current_scale(struct aat1290_led *led,
 	int i, max_mm_current =
 			AAT1290_MAX_MM_CURRENT(cfg->max_flash_current);
 
-	led->mm_current_scale = kzalloc(sizeof(max_mm_current_percent),
+	led->mm_current_scale = devm_kzalloc(&led->pdev->dev,
+					sizeof(max_mm_current_percent),
 					GFP_KERNEL);
 	if (!led->mm_current_scale)
 		return -ENOMEM;
@@ -329,11 +353,12 @@ int init_mm_current_scale(struct aat1290_led *led,
 }
 
 static int aat1290_led_get_configuration(struct aat1290_led *led,
-					struct aat1290_led_config_data *cfg)
+					struct aat1290_led_config_data *cfg,
+					struct device_node **sub_node)
 {
 	int ret;
 
-	ret = aat1290_led_parse_dt(led, cfg);
+	ret = aat1290_led_parse_dt(led, cfg, sub_node);
 	if (ret < 0)
 		return ret;
 	/*
@@ -346,7 +371,10 @@ static int aat1290_led_get_configuration(struct aat1290_led *led,
 
 	aat1290_led_validate_mm_current(led, cfg);
 
-	kfree(led->mm_current_scale);
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+#else
+	devm_kfree(&led->pdev->dev, led->mm_current_scale);
+#endif
 
 	return 0;
 }
@@ -365,6 +393,88 @@ static void aat1290_init_flash_timeout(struct aat1290_led *led,
 	setting->val = setting->max;
 }
 
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+enum led_brightness aat1290_intensity_to_brightness(
+					struct v4l2_flash *v4l2_flash,
+					s32 intensity)
+{
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
+	int i;
+
+	for (i = AAT1290_MM_CURRENT_SCALE_SIZE - 1; i >= 0; --i)
+		if (intensity >= led->mm_current_scale[i])
+			return i + 1;
+
+	return 1;
+}
+
+s32 aat1290_brightness_to_intensity(struct v4l2_flash *v4l2_flash,
+					enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
+
+	return led->mm_current_scale[brightness - 1];
+}
+
+static int aat1290_led_external_strobe_set(struct v4l2_flash *v4l2_flash,
+						bool enable)
+{
+	struct aat1290_led *led = fled_cdev_to_led(v4l2_flash->fled_cdev);
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	struct pinctrl *pinctrl;
+
+	gpiod_direction_output(led->gpio_fl_en, 0);
+	gpiod_direction_output(led->gpio_en_set, 0);
+
+	led->movie_mode = false;
+	led_cdev->brightness = 0;
+
+	pinctrl = devm_pinctrl_get_select(&led->pdev->dev,
+						enable ? "isp" : "host");
+	if (IS_ERR(pinctrl)) {
+		dev_warn(&led->pdev->dev, "Unable to switch strobe source.\n");
+		return PTR_ERR(pinctrl);
+	}
+
+	return 0;
+}
+
+static void aat1290_init_v4l2_flash_config(struct aat1290_led *led,
+					struct aat1290_led_config_data *led_cfg,
+					struct v4l2_flash_config *v4l2_sd_cfg)
+{
+	struct led_classdev *led_cdev = &led->fled_cdev.led_cdev;
+	struct led_flash_setting *s;
+
+	strlcpy(v4l2_sd_cfg->dev_name, led_cdev->name,
+		sizeof(v4l2_sd_cfg->dev_name));
+
+	s = &v4l2_sd_cfg->intensity;
+	s->min = led->mm_current_scale[0];
+	s->max = led_cfg->max_mm_current;
+	s->step = 1;
+	s->val = s->max;
+
+	v4l2_sd_cfg->has_external_strobe = led_cfg->has_external_strobe;
+}
+
+static const struct v4l2_flash_ops v4l2_flash_ops = {
+	.external_strobe_set = aat1290_led_external_strobe_set,
+	.intensity_to_led_brightness = aat1290_intensity_to_brightness,
+	.led_brightness_to_intensity = aat1290_brightness_to_intensity,
+};
+#else
+static inline void aat1290_init_v4l2_flash_config(struct aat1290_led *led,
+				struct aat1290_led_config_data *led_cfg,
+				struct v4l2_flash_config *v4l2_sd_cfg)
+{
+}
+static const struct v4l2_flash_ops v4l2_flash_ops;
+#endif
+
 static const struct led_flash_ops flash_ops = {
 	.strobe_set = aat1290_led_flash_strobe_set,
 	.timeout_set = aat1290_led_flash_timeout_set,
@@ -373,10 +483,12 @@ static const struct led_flash_ops flash_ops = {
 static int aat1290_led_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *sub_node = NULL;
 	struct aat1290_led *led;
 	struct led_classdev *led_cdev;
 	struct led_classdev_flash *fled_cdev;
 	struct aat1290_led_config_data led_cfg = {};
+	struct v4l2_flash_config v4l2_sd_cfg = {};
 	int ret;
 
 	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
@@ -390,7 +502,7 @@ static int aat1290_led_probe(struct platform_device *pdev)
 	fled_cdev->ops = &flash_ops;
 	led_cdev = &fled_cdev->led_cdev;
 
-	ret = aat1290_led_get_configuration(led, &led_cfg);
+	ret = aat1290_led_get_configuration(led, &led_cfg, &sub_node);
 	if (ret < 0)
 		return ret;
 
@@ -410,8 +522,20 @@ static int aat1290_led_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_flash_register;
 
+	aat1290_init_v4l2_flash_config(led, &led_cfg, &v4l2_sd_cfg);
+
+	/* Create V4L2 Flash subdev. */
+	led->v4l2_flash = v4l2_flash_init(dev, sub_node, fled_cdev,
+					  &v4l2_flash_ops, &v4l2_sd_cfg);
+	if (IS_ERR(led->v4l2_flash)) {
+		ret = PTR_ERR(led->v4l2_flash);
+		goto error_v4l2_flash_init;
+	}
+
 	return 0;
 
+error_v4l2_flash_init:
+	led_classdev_flash_unregister(fled_cdev);
 err_flash_register:
 	mutex_destroy(&led->lock);
 
@@ -422,6 +546,7 @@ static int aat1290_led_remove(struct platform_device *pdev)
 {
 	struct aat1290_led *led = platform_get_drvdata(pdev);
 
+	v4l2_flash_release(led->v4l2_flash);
 	led_classdev_flash_unregister(&led->fled_cdev);
 	cancel_work_sync(&led->work_brightness_set);
 
-- 
1.7.9.5

[PATCH v8 5/8] exynos4-is: Add support for v4l2-flash subdevs

[Jacek Anaszewski]

This patch adds support for external v4l2-flash devices.
The support includes parsing "samsung,flash-led" DT property
and asynchronous subdevice registration.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c |   39 +++++++++++++++++++++++--
 drivers/media/platform/exynos4-is/media-dev.h |   13 ++++++++-
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index f315ef9..80cecd8 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -451,6 +451,28 @@ rpm_put:
 	return ret;
 }
 
+static void fimc_md_register_flash_entities(struct fimc_md *fmd)
+{
+	int i;
+
+	fmd->num_flashes = 0;
+
+	for (i = 0; i < fmd->num_sensors; i++) {
+		const struct device_node *np =
+					fmd->sensor[i].asd.match.of.node;
+		const int nf = fmd->num_flashes;
+
+		np = of_parse_phandle(np, "samsung,flash-led", 0);
+		if (!np)
+			continue;
+
+		fmd->flash[nf].asd.match_type = V4L2_ASYNC_MATCH_OF;
+		fmd->flash[nf].asd.match.of.node = np;
+		fmd->async_subdevs[fmd->num_sensors + nf] = &fmd->flash[nf].asd;
+		fmd->num_flashes++;
+	}
+}
+
 static int __of_get_csis_id(struct device_node *np)
 {
 	u32 reg = 0;
@@ -1275,6 +1297,15 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
 	struct fimc_sensor_info *si = NULL;
 	int i;
 
+	/* Register flash subdev if detected any */
+	for (i = 0; i < ARRAY_SIZE(fmd->flash); i++) {
+		if (fmd->flash[i].asd.match.of.node == subdev->of_node) {
+			fmd->flash[i].subdev = subdev;
+			fmd->num_flashes++;
+			return 0;
+		}
+	}
+
 	/* Find platform data for this sensor subdev */
 	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
 		if (fmd->sensor[i].asd.match.of.node == subdev->dev->of_node)
@@ -1385,6 +1416,8 @@ static int fimc_md_probe(struct platform_device *pdev)
 		goto err_m_ent;
 	}
 
+	fimc_md_register_flash_entities(fmd);
+
 	mutex_unlock(&fmd->media_dev.graph_mutex);
 
 	ret = device_create_file(&pdev->dev, &dev_attr_subdev_conf_mode);
@@ -1401,12 +1434,14 @@ static int fimc_md_probe(struct platform_device *pdev)
 		goto err_attr;
 	}
 
-	if (fmd->num_sensors > 0) {
+	if (fmd->num_sensors > 0 || fmd->num_flashes > 0) {
 		fmd->subdev_notifier.subdevs = fmd->async_subdevs;
-		fmd->subdev_notifier.num_subdevs = fmd->num_sensors;
+		fmd->subdev_notifier.num_subdevs = fmd->num_sensors +
+							fmd->num_flashes;
 		fmd->subdev_notifier.bound = subdev_notifier_bound;
 		fmd->subdev_notifier.complete = subdev_notifier_complete;
 		fmd->num_sensors = 0;
+		fmd->num_flashes = 0;
 
 		ret = v4l2_async_notifier_register(&fmd->v4l2_dev,
 						&fmd->subdev_notifier);
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 0321454..d4eed34 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -34,6 +34,8 @@
 
 #define FIMC_MAX_SENSORS	4
 #define FIMC_MAX_CAMCLKS	2
+#define FIMC_MAX_FLASHES	4
+#define FIMC_MAX_ASYNC_SUBDEVS (FIMC_MAX_SENSORS + FIMC_MAX_FLASHES)
 #define DEFAULT_SENSOR_CLK_FREQ	24000000U
 
 /* LCD/ISP Writeback clocks (PIXELASYNCMx) */
@@ -93,6 +95,11 @@ struct fimc_sensor_info {
 	struct fimc_dev *host;
 };
 
+struct fimc_flash_info {
+	struct v4l2_subdev *subdev;
+	struct v4l2_async_subdev asd;
+};
+
 struct cam_clk {
 	struct clk_hw hw;
 	struct fimc_md *fmd;
@@ -104,6 +111,8 @@ struct cam_clk {
  * @csis: MIPI CSIS subdevs data
  * @sensor: array of registered sensor subdevs
  * @num_sensors: actual number of registered sensors
+ * @flash: array of registered flash subdevs
+ * @num_flashes: actual number of registered flashes
  * @camclk: external sensor clock information
  * @fimc: array of registered fimc devices
  * @fimc_is: fimc-is data structure
@@ -123,6 +132,8 @@ struct fimc_md {
 	struct fimc_csis_info csis[CSIS_MAX_ENTITIES];
 	struct fimc_sensor_info sensor[FIMC_MAX_SENSORS];
 	int num_sensors;
+	struct fimc_flash_info flash[FIMC_MAX_FLASHES];
+	int num_flashes;
 	struct fimc_camclk_info camclk[FIMC_MAX_CAMCLKS];
 	struct clk *wbclk[FIMC_MAX_WBCLKS];
 	struct fimc_lite *fimc_lite[FIMC_LITE_MAX_DEVS];
@@ -149,7 +160,7 @@ struct fimc_md {
 	} clk_provider;
 
 	struct v4l2_async_notifier subdev_notifier;
-	struct v4l2_async_subdev *async_subdevs[FIMC_MAX_SENSORS];
+	struct v4l2_async_subdev *async_subdevs[FIMC_MAX_ASYNC_SUBDEVS];
 
 	bool user_subdev_api;
 	spinlock_t slock;
-- 
1.7.9.5

[PATCH v8 6/8] DT: s5c73m3: Add documentation for samsung,flash-led property

[Jacek Anaszewski]

This patch adds a description of 'samsung,flash-led' property
to the samsung-s5c73m3.txt.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/samsung-s5c73m3.txt  |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
index 2c85c45..8a413c0 100644
--- a/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
+++ b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
@@ -32,6 +32,10 @@ Optional properties:
 - clock-frequency   : the frequency at which the "cis_extclk" clock should be
 		      configured to operate, in Hz; if this property is not
 		      specified default 24 MHz value will be used.
+- samsung,flash-led : phandle to the flash LED associated with this sensor.
+		      Flash LED is represented by the child node of a flash LED
+		      device
+		      (see Documentation/devicetree/bindings/leds/common.txt).
 
 The common video interfaces bindings (see video-interfaces.txt) should be used
 to specify link from the S5C73M3 to an external image data receiver. The S5C73M3
@@ -78,6 +82,7 @@ i2c@138A000000 {
 		clock-names = "cis_extclk";
 		reset-gpios = <&gpf1 3 1>;
 		standby-gpios = <&gpm0 1 1>;
+		samsung,flash-led = <&camera1_flash>;
 		port {
 			s5c73m3_ep: endpoint {
 				remote-endpoint = <&csis0_ep>;
-- 
1.7.9.5

[PATCH v8 7/8] DT: s5k6a3: Add documentation for samsung,flash-led property

[Jacek Anaszewski]

This patch adds a description of 'samsung,flash-led' property
to the samsung-s5k6a3.txt.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/samsung-s5k6a3.txt   |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
index cce01e8..25f2e82 100644
--- a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
+++ b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
@@ -22,6 +22,10 @@ Optional properties:
 - clock-frequency : the frequency at which the "extclk" clock should be
 		    configured to operate, in Hz; if this property is not
 		    specified default 24 MHz value will be used.
+- samsung,flash-led : phandle to the flash LED associated with this sensor.
+		      Flash LED is represented by the child node of a flash LED
+		      device
+		      (see Documentation/devicetree/bindings/leds/common.txt).
 
 The common video interfaces bindings (see video-interfaces.txt) should be
 used to specify link to the image data receiver. The S5K6A3(YX) device
-- 
1.7.9.5

[PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

This patch adds examples for samsung,flash-led property to the
samsung-fimc.txt.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..57edffa 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -126,6 +126,8 @@ Example:
 			clocks = <&camera 1>;
 			clock-names = "mclk";
 
+			samsung,flash-led = <&front_cam_flash>;
+
 			port {
 				s5k6aa_ep: endpoint {
 					remote-endpoint = <&fimc0_ep>;
@@ -147,6 +149,8 @@ Example:
 			clocks = <&camera 0>;
 			clock-names = "mclk";
 
+			samsung,flash-led = <&rear_cam_flash>;
+
 			port {
 				s5c73m3_1: endpoint {
 					data-lanes = <1 2 3 4>;
-- 
1.7.9.5

Re: [PATCH v8 1/8] media: Add registration helpers for V4L2 flash sub-devices

[Sakari Ailus]

Hi Jacek,

On Wed, May 20, 2015 at 04:10:08PM +0200, Jacek Anaszewski wrote:
...
> +struct v4l2_flash *v4l2_flash_init(
> +	struct device *dev, struct device_node *of_node,
> +	struct led_classdev_flash *fled_cdev, const struct v4l2_flash_ops *ops,
> +	struct v4l2_flash_config *config)
> +{
> +	struct v4l2_flash *v4l2_flash;
> +	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> +	struct v4l2_subdev *sd;
> +	int ret;
> +
> +	if (!fled_cdev || !ops || !config)
> +		return ERR_PTR(-EINVAL);
> +
> +	v4l2_flash = devm_kzalloc(led_cdev->dev, sizeof(*v4l2_flash),
> +					GFP_KERNEL);
> +	if (!v4l2_flash)
> +		return ERR_PTR(-ENOMEM);
> +
> +	sd = &v4l2_flash->sd;
> +	v4l2_flash->fled_cdev = fled_cdev;
> +	v4l2_flash->ops = ops;
> +	sd->dev = dev;
> +	sd->of_node = of_node;
> +	v4l2_subdev_init(sd, &v4l2_flash_subdev_ops);
> +	sd->internal_ops = &v4l2_flash_subdev_internal_ops;
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	strlcpy(sd->name, config->dev_name, sizeof(sd->name));
> +
> +	ret = media_entity_init(&sd->entity, 0, NULL, 0);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +
> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +
> +	ret = v4l2_flash_init_controls(v4l2_flash, config);
> +	if (ret < 0)
> +		goto err_init_controls;
> +
> +	of_node_get(led_cdev->dev->of_node);
> +
> +	ret = v4l2_async_register_subdev(sd);
> +	if (ret < 0)
> +		goto err_async_register_sd;
> +
> +	return v4l2_flash;
> +
> +err_async_register_sd:
> +	of_node_put(led_cdev->dev->of_node);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +err_init_controls:
> +	media_entity_cleanup(&sd->entity);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_flash_init);
> +
> +void v4l2_flash_release(struct v4l2_flash *v4l2_flash)
> +{
> +	struct v4l2_subdev *sd;
> +	struct led_classdev *led_cdev;
> +
> +	if (IS_ERR(v4l2_flash))

I propose to use IS_ERR_OR_NULL() instead. Then this can be safely called
without calling v4l2_flash_init() first, making error handling easier.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> +		return;
> +
> +	sd = &v4l2_flash->sd;
> +	led_cdev = &v4l2_flash->fled_cdev->led_cdev;
> +
> +	v4l2_async_unregister_subdev(sd);
> +	of_node_put(led_cdev->dev->of_node);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +	media_entity_cleanup(&sd->entity);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_flash_release);

...

-- 
Kind regards,

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

Re: [PATCH v8 2/8] leds: max77693: add support for V4L2 Flash sub-device

[Sakari Ailus]

On Wed, May 20, 2015 at 04:10:09PM +0200, Jacek Anaszewski wrote:
> Add support for V4L2 Flash sub-device to the max77693 LED Flash class
> driver. The support allows for V4L2 Flash sub-device to take the control
> of the LED Flash class device.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Cc: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

Re: [PATCH v8 2/8] leds: max77693: add support for V4L2 Flash sub-device

[Sakari Ailus]

On Wed, May 20, 2015 at 04:10:09PM +0200, Jacek Anaszewski wrote:
> Add support for V4L2 Flash sub-device to the max77693 LED Flash class
> driver. The support allows for V4L2 Flash sub-device to take the control
> of the LED Flash class device.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Cc: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Jacek,

On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
> This patch adds examples for samsung,flash-led property to the
> samsung-fimc.txt.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> index 922d6f8..57edffa 100644
> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> @@ -126,6 +126,8 @@ Example:
>  			clocks = <&camera 1>;
>  			clock-names = "mclk";
>  
> +			samsung,flash-led = <&front_cam_flash>;
> +
>  			port {
>  				s5k6aa_ep: endpoint {
>  					remote-endpoint = <&fimc0_ep>;
> @@ -147,6 +149,8 @@ Example:
>  			clocks = <&camera 0>;
>  			clock-names = "mclk";
>  
> +			samsung,flash-led = <&rear_cam_flash>;
> +
>  			port {
>  				s5c73m3_1: endpoint {
>  					data-lanes = <1 2 3 4>;

Oops. I missed this property would have ended to the sensor's DT node. I
don't think we should have properties here that are parsed by another
driver --- let's discuss this tomorrow.

There are two main options that I can think of --- either put the property
under the bridge (ISP) driver's device node as a temporary solution that
works on a few ISP drivers, or think how sensor modules should be modelled,
in which case we'd have some idea how lens device would be taken into
account.

Cc Sebastian.

-- 
Kind regards,

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

Re: [PATCH v8 5/8] exynos4-is: Add support for v4l2-flash subdevs

[Sakari Ailus]

Hi Jacek,

On Wed, May 20, 2015 at 04:10:12PM +0200, Jacek Anaszewski wrote:
> This patch adds support for external v4l2-flash devices.
> The support includes parsing "samsung,flash-led" DT property
> and asynchronous subdevice registration.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/media/platform/exynos4-is/media-dev.c |   39 +++++++++++++++++++++++--
>  drivers/media/platform/exynos4-is/media-dev.h |   13 ++++++++-
>  2 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index f315ef9..80cecd8 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -451,6 +451,28 @@ rpm_put:
>  	return ret;
>  }
>  
> +static void fimc_md_register_flash_entities(struct fimc_md *fmd)
> +{
> +	int i;
> +
> +	fmd->num_flashes = 0;
> +
> +	for (i = 0; i < fmd->num_sensors; i++) {
> +		const struct device_node *np =
> +					fmd->sensor[i].asd.match.of.node;
> +		const int nf = fmd->num_flashes;
> +
> +		np = of_parse_phandle(np, "samsung,flash-led", 0);
> +		if (!np)
> +			continue;
> +
> +		fmd->flash[nf].asd.match_type = V4L2_ASYNC_MATCH_OF;
> +		fmd->flash[nf].asd.match.of.node = np;
> +		fmd->async_subdevs[fmd->num_sensors + nf] = &fmd->flash[nf].asd;
> +		fmd->num_flashes++;
> +	}
> +}
> +
>  static int __of_get_csis_id(struct device_node *np)
>  {
>  	u32 reg = 0;
> @@ -1275,6 +1297,15 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>  	struct fimc_sensor_info *si = NULL;
>  	int i;
>  
> +	/* Register flash subdev if detected any */
> +	for (i = 0; i < ARRAY_SIZE(fmd->flash); i++) {
> +		if (fmd->flash[i].asd.match.of.node == subdev->of_node) {

Does the index of a particular sub-device index matter? Could you just use
the next available one?

There would be no need to for the check anything here, please see how the
omap3isp driver does it --- it's in isp_subdev_notifier_bound()
drivers/media/platform/omap3isp/isp.c .

> +			fmd->flash[i].subdev = subdev;
> +			fmd->num_flashes++;
> +			return 0;
> +		}
> +	}
> +
>  	/* Find platform data for this sensor subdev */
>  	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
>  		if (fmd->sensor[i].asd.match.of.node == subdev->dev->of_node)

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

Hi Sakari,

On 05/21/2015 12:00 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
>> This patch adds examples for samsung,flash-led property to the
>> samsung-fimc.txt.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> index 922d6f8..57edffa 100644
>> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> @@ -126,6 +126,8 @@ Example:
>>   			clocks = <&camera 1>;
>>   			clock-names = "mclk";
>>
>> +			samsung,flash-led = <&front_cam_flash>;
>> +
>>   			port {
>>   				s5k6aa_ep: endpoint {
>>   					remote-endpoint = <&fimc0_ep>;
>> @@ -147,6 +149,8 @@ Example:
>>   			clocks = <&camera 0>;
>>   			clock-names = "mclk";
>>
>> +			samsung,flash-led = <&rear_cam_flash>;
>> +
>>   			port {
>>   				s5c73m3_1: endpoint {
>>   					data-lanes = <1 2 3 4>;
>
> Oops. I missed this property would have ended to the sensor's DT node. I
> don't think we should have properties here that are parsed by another
> driver --- let's discuss this tomorrow.

exynos4-is driver already parses sensor nodes (at least their 'port'
sub-nodes).

> There are two main options that I can think of --- either put the property
> under the bridge (ISP) driver's device node as a temporary solution that
> works on a few ISP drivers, or think how sensor modules should be modelled,
> in which case we'd have some idea how lens device would be taken into
> account.
>
> Cc Sebastian.
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 5/8] exynos4-is: Add support for v4l2-flash subdevs

[Jacek Anaszewski]

On 05/21/2015 09:46 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Wed, May 20, 2015 at 04:10:12PM +0200, Jacek Anaszewski wrote:
>> This patch adds support for external v4l2-flash devices.
>> The support includes parsing "samsung,flash-led" DT property
>> and asynchronous subdevice registration.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>>   drivers/media/platform/exynos4-is/media-dev.c |   39 +++++++++++++++++++++++--
>>   drivers/media/platform/exynos4-is/media-dev.h |   13 ++++++++-
>>   2 files changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
>> index f315ef9..80cecd8 100644
>> --- a/drivers/media/platform/exynos4-is/media-dev.c
>> +++ b/drivers/media/platform/exynos4-is/media-dev.c
>> @@ -451,6 +451,28 @@ rpm_put:
>>   	return ret;
>>   }
>>
>> +static void fimc_md_register_flash_entities(struct fimc_md *fmd)
>> +{
>> +	int i;
>> +
>> +	fmd->num_flashes = 0;
>> +
>> +	for (i = 0; i < fmd->num_sensors; i++) {
>> +		const struct device_node *np =
>> +					fmd->sensor[i].asd.match.of.node;
>> +		const int nf = fmd->num_flashes;
>> +
>> +		np = of_parse_phandle(np, "samsung,flash-led", 0);
>> +		if (!np)
>> +			continue;
>> +
>> +		fmd->flash[nf].asd.match_type = V4L2_ASYNC_MATCH_OF;
>> +		fmd->flash[nf].asd.match.of.node = np;
>> +		fmd->async_subdevs[fmd->num_sensors + nf] = &fmd->flash[nf].asd;
>> +		fmd->num_flashes++;
>> +	}
>> +}
>> +
>>   static int __of_get_csis_id(struct device_node *np)
>>   {
>>   	u32 reg = 0;
>> @@ -1275,6 +1297,15 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>>   	struct fimc_sensor_info *si = NULL;
>>   	int i;
>>
>> +	/* Register flash subdev if detected any */
>> +	for (i = 0; i < ARRAY_SIZE(fmd->flash); i++) {
>> +		if (fmd->flash[i].asd.match.of.node == subdev->of_node) {
>
> Does the index of a particular sub-device index matter? Could you just use
> the next available one?

Having the positions of sensor sub-devices kept in synch with
the positions of associated flash sub-devices in the 'flash' array
allows to avoid the addition o a condition for checking if the element
of the flash array is available.

> There would be no need to for the check anything here, please see how the
> omap3isp driver does it --- it's in isp_subdev_notifier_bound()
> drivers/media/platform/omap3isp/isp.c .

This 'for' loop allows to detect that the sub-device being bound
is of flash type and basing on that we can exit the function
at this point. Otherwise some guards would have to be added in
the remaining part of the function.

>> +			fmd->flash[i].subdev = subdev;
>> +			fmd->num_flashes++;
>> +			return 0;
>> +		}
>> +	}
>> +
>>   	/* Find platform data for this sensor subdev */
>>   	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
>>   		if (fmd->sensor[i].asd.match.of.node == subdev->dev->of_node)
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 5/8] exynos4-is: Add support for v4l2-flash subdevs

[Sakari Ailus]

Hi Jacek,

On Thu, May 21, 2015 at 11:39:35AM +0200, Jacek Anaszewski wrote:
> 
> 
> On 05/21/2015 09:46 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, May 20, 2015 at 04:10:12PM +0200, Jacek Anaszewski wrote:
> >>This patch adds support for external v4l2-flash devices.
> >>The support includes parsing "samsung,flash-led" DT property
> >>and asynchronous subdevice registration.
> >>
> >>Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> >>Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >>---
> >>  drivers/media/platform/exynos4-is/media-dev.c |   39 +++++++++++++++++++++++--
> >>  drivers/media/platform/exynos4-is/media-dev.h |   13 ++++++++-
> >>  2 files changed, 49 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> >>index f315ef9..80cecd8 100644
> >>--- a/drivers/media/platform/exynos4-is/media-dev.c
> >>+++ b/drivers/media/platform/exynos4-is/media-dev.c
> >>@@ -451,6 +451,28 @@ rpm_put:
> >>  	return ret;
> >>  }
> >>
> >>+static void fimc_md_register_flash_entities(struct fimc_md *fmd)
> >>+{
> >>+	int i;
> >>+
> >>+	fmd->num_flashes = 0;
> >>+
> >>+	for (i = 0; i < fmd->num_sensors; i++) {
> >>+		const struct device_node *np =
> >>+					fmd->sensor[i].asd.match.of.node;
> >>+		const int nf = fmd->num_flashes;
> >>+
> >>+		np = of_parse_phandle(np, "samsung,flash-led", 0);
> >>+		if (!np)
> >>+			continue;
> >>+
> >>+		fmd->flash[nf].asd.match_type = V4L2_ASYNC_MATCH_OF;
> >>+		fmd->flash[nf].asd.match.of.node = np;
> >>+		fmd->async_subdevs[fmd->num_sensors + nf] = &fmd->flash[nf].asd;
> >>+		fmd->num_flashes++;
> >>+	}
> >>+}
> >>+
> >>  static int __of_get_csis_id(struct device_node *np)
> >>  {
> >>  	u32 reg = 0;
> >>@@ -1275,6 +1297,15 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
> >>  	struct fimc_sensor_info *si = NULL;
> >>  	int i;
> >>
> >>+	/* Register flash subdev if detected any */
> >>+	for (i = 0; i < ARRAY_SIZE(fmd->flash); i++) {
> >>+		if (fmd->flash[i].asd.match.of.node == subdev->of_node) {
> >
> >Does the index of a particular sub-device index matter? Could you just use
> >the next available one?
> 
> Having the positions of sensor sub-devices kept in synch with
> the positions of associated flash sub-devices in the 'flash' array
> allows to avoid the addition o a condition for checking if the element
> of the flash array is available.
> 
> >There would be no need to for the check anything here, please see how the
> >omap3isp driver does it --- it's in isp_subdev_notifier_bound()
> >drivers/media/platform/omap3isp/isp.c .
> 
> This 'for' loop allows to detect that the sub-device being bound
> is of flash type and basing on that we can exit the function
> at this point. Otherwise some guards would have to be added in
> the remaining part of the function.

You have that information in fimc_md_register_flash_entities() but you're
not storing that. A common struct for all async sub-device would help. I
think you could associate sensors to flashes the same way.

-- 
Regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Jacek,

On Thu, May 21, 2015 at 11:10:49AM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 05/21/2015 12:00 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
> >>This patch adds examples for samsung,flash-led property to the
> >>samsung-fimc.txt.
> >>
> >>Signed-off-by: Jacek Anaszewski <j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >>Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >>Cc: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >>Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>---
> >>  .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>index 922d6f8..57edffa 100644
> >>--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>@@ -126,6 +126,8 @@ Example:
> >>  			clocks = <&camera 1>;
> >>  			clock-names = "mclk";
> >>
> >>+			samsung,flash-led = <&front_cam_flash>;
> >>+
> >>  			port {
> >>  				s5k6aa_ep: endpoint {
> >>  					remote-endpoint = <&fimc0_ep>;
> >>@@ -147,6 +149,8 @@ Example:
> >>  			clocks = <&camera 0>;
> >>  			clock-names = "mclk";
> >>
> >>+			samsung,flash-led = <&rear_cam_flash>;
> >>+
> >>  			port {
> >>  				s5c73m3_1: endpoint {
> >>  					data-lanes = <1 2 3 4>;
> >
> >Oops. I missed this property would have ended to the sensor's DT node. I
> >don't think we should have properties here that are parsed by another
> >driver --- let's discuss this tomorrow.
> 
> exynos4-is driver already parses sensor nodes (at least their 'port'
> sub-nodes).

If you read the code and the comment, it looks like something that should be
done better but hasn't been done yet. :-) That's something we should avoid.
Also, flash devices are by far more common than external ISPs I presume.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Jacek,

On Thu, May 21, 2015 at 11:10:49AM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 05/21/2015 12:00 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
> >>This patch adds examples for samsung,flash-led property to the
> >>samsung-fimc.txt.
> >>
> >>Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> >>Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >>Cc: devicetree@vger.kernel.org
> >>---
> >>  .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>index 922d6f8..57edffa 100644
> >>--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>@@ -126,6 +126,8 @@ Example:
> >>  			clocks = <&camera 1>;
> >>  			clock-names = "mclk";
> >>
> >>+			samsung,flash-led = <&front_cam_flash>;
> >>+
> >>  			port {
> >>  				s5k6aa_ep: endpoint {
> >>  					remote-endpoint = <&fimc0_ep>;
> >>@@ -147,6 +149,8 @@ Example:
> >>  			clocks = <&camera 0>;
> >>  			clock-names = "mclk";
> >>
> >>+			samsung,flash-led = <&rear_cam_flash>;
> >>+
> >>  			port {
> >>  				s5c73m3_1: endpoint {
> >>  					data-lanes = <1 2 3 4>;
> >
> >Oops. I missed this property would have ended to the sensor's DT node. I
> >don't think we should have properties here that are parsed by another
> >driver --- let's discuss this tomorrow.
> 
> exynos4-is driver already parses sensor nodes (at least their 'port'
> sub-nodes).

If you read the code and the comment, it looks like something that should be
done better but hasn't been done yet. :-) That's something we should avoid.
Also, flash devices are by far more common than external ISPs I presume.

-- 
Regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

On 21/05/15 13:32, Sakari Ailus wrote:
>>>> @@ -147,6 +149,8 @@ Example:
>>>> > >>  			clocks = <&camera 0>;
>>>> > >>  			clock-names = "mclk";
>>>> > >>
>>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
>>>> > >>+
>>>> > >>  			port {
>>>> > >>  				s5c73m3_1: endpoint {
>>>> > >>  					data-lanes = <1 2 3 4>;
>>> > >
>>> > >Oops. I missed this property would have ended to the sensor's DT node. I
>>> > >don't think we should have properties here that are parsed by another
>>> > >driver --- let's discuss this tomorrow.
>> > 
>> > exynos4-is driver already parses sensor nodes (at least their 'port'
>> > sub-nodes).
>
> If you read the code and the comment, it looks like something that should be
> done better but hasn't been done yet. :-) That's something we should avoid.
> Also, flash devices are by far more common than external ISPs I presume.

Yes, especially let's not require any samsung specific properties in
other vendors' sensor bindings.

One way of modelling [flash led]/[image sensor] association I imagine
would be to put, e.g. 'flash-leds' property in the SoC camera host
interface/ISP DT node. This property would then contain pairs of phandles,
first to the led node and the second to the sensor node, e.g.

i2c_controller {
	...
	flash_xx@NN {
		...
		led_a {
			...		
		}
	};

	image_sensor_x@NN {
		...
	};
};

flash-leds = <&flash_xx &image_sensor_x>, <...>;

For the purpose of this patch set presumably just samsung specific
property name could be used (i.e. samsung,flash-leds).

--
Thanks,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> On 21/05/15 13:32, Sakari Ailus wrote:
> >>>> @@ -147,6 +149,8 @@ Example:
> >>>> > >>  			clocks = <&camera 0>;
> >>>> > >>  			clock-names = "mclk";
> >>>> > >>
> >>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
> >>>> > >>+
> >>>> > >>  			port {
> >>>> > >>  				s5c73m3_1: endpoint {
> >>>> > >>  					data-lanes = <1 2 3 4>;
> >>> > >
> >>> > >Oops. I missed this property would have ended to the sensor's DT node. I
> >>> > >don't think we should have properties here that are parsed by another
> >>> > >driver --- let's discuss this tomorrow.
> >> > 
> >> > exynos4-is driver already parses sensor nodes (at least their 'port'
> >> > sub-nodes).
> >
> > If you read the code and the comment, it looks like something that should be
> > done better but hasn't been done yet. :-) That's something we should avoid.
> > Also, flash devices are by far more common than external ISPs I presume.
> 
> Yes, especially let's not require any samsung specific properties in
> other vendors' sensor bindings.
> 
> One way of modelling [flash led]/[image sensor] association I imagine
> would be to put, e.g. 'flash-leds' property in the SoC camera host
> interface/ISP DT node. This property would then contain pairs of phandles,
> first to the led node and the second to the sensor node, e.g.
> 
> i2c_controller {
> 	...
> 	flash_xx@NN {
> 		...
> 		led_a {
> 			...		
> 		}
> 	};
> 
> 	image_sensor_x@NN {
> 		...
> 	};
> };
> 
> flash-leds = <&flash_xx &image_sensor_x>, <...>;

Maybe a stupid question, but how do you access this in a driver? I have to
admit I'm no DT expert.

> 
> For the purpose of this patch set presumably just samsung specific
> property name could be used (i.e. samsung,flash-leds).

I agree. I'll add similar support for the omap3isp driver in the near future
though. Let's see how the camera modules will get modelled, if they will,
and if this property still fits to the picture by that time, then we make it
more generic.

What do you think?

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

Hi Sakari,

On 21/05/15 16:20, Sakari Ailus wrote:
> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>> > On 21/05/15 13:32, Sakari Ailus wrote:
>>>>>> > >>>> @@ -147,6 +149,8 @@ Example:
>>>>>>>>> > >>>> > >>  			clocks = <&camera 0>;
>>>>>>>>> > >>>> > >>  			clock-names = "mclk";
>>>>>>>>> > >>>> > >>
>>>>>>>>> > >>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
>>>>>>>>> > >>>> > >>+
>>>>>>>>> > >>>> > >>  			port {
>>>>>>>>> > >>>> > >>  				s5c73m3_1: endpoint {
>>>>>>>>> > >>>> > >>  					data-lanes = <1 2 3 4>;
>>>>>>> > >>> > >
>>>>>>> > >>> > >Oops. I missed this property would have ended to the sensor's DT node. I
>>>>>>> > >>> > >don't think we should have properties here that are parsed by another
>>>>>>> > >>> > >driver --- let's discuss this tomorrow.
>>>>> > >> > 
>>>>> > >> > exynos4-is driver already parses sensor nodes (at least their 'port'
>>>>> > >> > sub-nodes).
>>> > >
>>> > > If you read the code and the comment, it looks like something that should be
>>> > > done better but hasn't been done yet. :-) That's something we should avoid.
>>> > > Also, flash devices are by far more common than external ISPs I presume.
>> > 
>> > Yes, especially let's not require any samsung specific properties in
>> > other vendors' sensor bindings.
>> > 
>> > One way of modelling [flash led]/[image sensor] association I imagine
>> > would be to put, e.g. 'flash-leds' property in the SoC camera host
>> > interface/ISP DT node. This property would then contain pairs of phandles,
>> > first to the led node and the second to the sensor node, e.g.
>> > 
>> > i2c_controller {
>> > 	...
>> > 	flash_xx@NN {
>> > 		...
>> > 		led_a {
>> > 			...		
>> > 		}
>> > 	};
>> > 
>> > 	image_sensor_x@NN {
>> > 		...
>> > 	};
>> > };
>> > 
>> > flash-leds = <&flash_xx &image_sensor_x>, <...>;
>
> Maybe a stupid question, but how do you access this in a driver? I have to
> admit I'm no DT expert.

You could get of_node pointers with of_parse_phandle() call and then
lookup related flash and sensor devices based on that.

>> > For the purpose of this patch set presumably just samsung specific
>> > property name could be used (i.e. samsung,flash-leds).
>
> I agree. I'll add similar support for the omap3isp driver in the near future
> though. Let's see how the camera modules will get modelled, if they will,
> and if this property still fits to the picture by that time, then we make it
> more generic.
> 
> What do you think?

I think we could do that, perhaps we could get some more opinions and
use generic name already in this series? I'm not sure what are exact
plans for this series, I guess it is targeted for 4.2?

-- 
Regards,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Thu, May 21, 2015 at 06:58:59PM +0200, Sylwester Nawrocki wrote:
> Hi Sakari,
> 
> On 21/05/15 16:20, Sakari Ailus wrote:
> > On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >> > On 21/05/15 13:32, Sakari Ailus wrote:
> >>>>>> > >>>> @@ -147,6 +149,8 @@ Example:
> >>>>>>>>> > >>>> > >>  			clocks = <&camera 0>;
> >>>>>>>>> > >>>> > >>  			clock-names = "mclk";
> >>>>>>>>> > >>>> > >>
> >>>>>>>>> > >>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
> >>>>>>>>> > >>>> > >>+
> >>>>>>>>> > >>>> > >>  			port {
> >>>>>>>>> > >>>> > >>  				s5c73m3_1: endpoint {
> >>>>>>>>> > >>>> > >>  					data-lanes = <1 2 3 4>;
> >>>>>>> > >>> > >
> >>>>>>> > >>> > >Oops. I missed this property would have ended to the sensor's DT node. I
> >>>>>>> > >>> > >don't think we should have properties here that are parsed by another
> >>>>>>> > >>> > >driver --- let's discuss this tomorrow.
> >>>>> > >> > 
> >>>>> > >> > exynos4-is driver already parses sensor nodes (at least their 'port'
> >>>>> > >> > sub-nodes).
> >>> > >
> >>> > > If you read the code and the comment, it looks like something that should be
> >>> > > done better but hasn't been done yet. :-) That's something we should avoid.
> >>> > > Also, flash devices are by far more common than external ISPs I presume.
> >> > 
> >> > Yes, especially let's not require any samsung specific properties in
> >> > other vendors' sensor bindings.
> >> > 
> >> > One way of modelling [flash led]/[image sensor] association I imagine
> >> > would be to put, e.g. 'flash-leds' property in the SoC camera host
> >> > interface/ISP DT node. This property would then contain pairs of phandles,
> >> > first to the led node and the second to the sensor node, e.g.
> >> > 
> >> > i2c_controller {
> >> > 	...
> >> > 	flash_xx@NN {
> >> > 		...
> >> > 		led_a {
> >> > 			...		
> >> > 		}
> >> > 	};
> >> > 
> >> > 	image_sensor_x@NN {
> >> > 		...
> >> > 	};
> >> > };
> >> > 
> >> > flash-leds = <&flash_xx &image_sensor_x>, <...>;
> >
> > Maybe a stupid question, but how do you access this in a driver? I have to
> > admit I'm no DT expert.
> 
> You could get of_node pointers with of_parse_phandle() call and then
> lookup related flash and sensor devices based on that.

Ack. Looks good to me.

> >> > For the purpose of this patch set presumably just samsung specific
> >> > property name could be used (i.e. samsung,flash-leds).
> >
> > I agree. I'll add similar support for the omap3isp driver in the near future
> > though. Let's see how the camera modules will get modelled, if they will,
> > and if this property still fits to the picture by that time, then we make it
> > more generic.
> > 
> > What do you think?
> 
> I think we could do that, perhaps we could get some more opinions and
> use generic name already in this series? I'm not sure what are exact
> plans for this series, I guess it is targeted for 4.2?

There have been very few opinions expressed besides yours, mine and Jacek's,
unfortunately. I'm also not very certain on the future-proofness of this
solution until we have better understanding of how modules would best be
expressed in DT.

v4.2 would be nice target for these, yes.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester and Jacek,

On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> flash-leds = <&flash_xx &image_sensor_x>, <...>;

One more matter to consider: xenon flash devices.

How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
this awhile, I'm ok with removing the vendor prefix as well.

Let me know what you think.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

Hi,

On 23/05/15 14:03, Sakari Ailus wrote:
> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> 
> One more matter to consider: xenon flash devices.
> 
> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> this awhile, I'm ok with removing the vendor prefix as well.
> 
> Let me know what you think.

I thought about it a bit more and I have some doubts about semantics
as above. I'm fine with 'camera-flashes' as far as name is concerned.

Perhaps we should put only phandles to leds or xenon flash devices
in the 'camera-flashes' property. I think it would be more future
proof in case there is more nodes needed to describe the camera flash
(or a camera module) than the above two. And phandles to corresponding
image sensor device nodes would be put in a separate property.

camera-flashes = <&flash_xx>, ...
camera-flash-masters = <&image_sensor_x>, ...

Then pairs at same index would describe a single flash, 0 would indicate
a null entry if needed.
Similarly we could create properties for other sub-devices of a camera
module, like lenses, etc.

--
Thanks,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

Hi,

On 05/25/2015 02:00 PM, Sylwester Nawrocki wrote:
> Hi,
>
> On 23/05/15 14:03, Sakari Ailus wrote:
>> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
>>
>> One more matter to consider: xenon flash devices.
>>
>> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
>> this awhile, I'm ok with removing the vendor prefix as well.
>>
>> Let me know what you think.
>
> I thought about it a bit more and I have some doubts about semantics
> as above. I'm fine with 'camera-flashes' as far as name is concerned.
>
> Perhaps we should put only phandles to leds or xenon flash devices
> in the 'camera-flashes' property. I think it would be more future
> proof in case there is more nodes needed to describe the camera flash
> (or a camera module) than the above two. And phandles to corresponding
> image sensor device nodes would be put in a separate property.

Could you give examples of the cases you are thinking of?

> camera-flashes = <&flash_xx>, ...
> camera-flash-masters = <&image_sensor_x>, ...
>
> Then pairs at same index would describe a single flash, 0 would indicate
> a null entry if needed.

When it should be needed?

> Similarly we could create properties for other sub-devices of a camera
> module, like lenses, etc.


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

On 25/05/15 14:50, Jacek Anaszewski wrote:
>> On 23/05/15 14:03, Sakari Ailus wrote:
>>> >> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>>>> >>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
>>> >>
>>> >> One more matter to consider: xenon flash devices.
>>> >>
>>> >> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
>>> >> this awhile, I'm ok with removing the vendor prefix as well.
>>> >>
>>> >> Let me know what you think.
>> >
>> > I thought about it a bit more and I have some doubts about semantics
>> > as above. I'm fine with 'camera-flashes' as far as name is concerned.
>> >
>> > Perhaps we should put only phandles to leds or xenon flash devices
>> > in the 'camera-flashes' property. I think it would be more future
>> > proof in case there is more nodes needed to describe the camera flash
>> > (or a camera module) than the above two. And phandles to corresponding
>> > image sensor device nodes would be put in a separate property.
>
> Could you give examples of the cases you are thinking of?

I don't have any examples in mind ATM, I just wanted to point out
the above convention might not be flexible enough. Especially since
we already know there is more sub-devices within the camera module
than just flashes and image sensors.

>> > camera-flashes = <&flash_xx>, ...
>> > camera-flash-masters = <&image_sensor_x>, ...
>> >
>> > Then pairs at same index would describe a single flash, 0 would indicate
>> > a null entry if needed.
>
> When it should be needed?

Not sure if there is a real use case for null entries, it was just to
note we can skip any entry if needed - probably an irrelevant comment.
I could imagine 2 LEDs of which one is only triggered in software, so
it wouldn't have a 'camera-flash-masters' entry.

>> > Similarly we could create properties for other sub-devices of a camera
>> > module, like lenses, etc.

--
Regards,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

On 05/25/2015 04:28 PM, Sylwester Nawrocki wrote:
> On 25/05/15 14:50, Jacek Anaszewski wrote:
>>> On 23/05/15 14:03, Sakari Ailus wrote:
>>>>>> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>>>>>>>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
>>>>>>
>>>>>> One more matter to consider: xenon flash devices.
>>>>>>
>>>>>> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
>>>>>> this awhile, I'm ok with removing the vendor prefix as well.
>>>>>>
>>>>>> Let me know what you think.
>>>>
>>>> I thought about it a bit more and I have some doubts about semantics
>>>> as above. I'm fine with 'camera-flashes' as far as name is concerned.
>>>>
>>>> Perhaps we should put only phandles to leds or xenon flash devices
>>>> in the 'camera-flashes' property. I think it would be more future
>>>> proof in case there is more nodes needed to describe the camera flash
>>>> (or a camera module) than the above two. And phandles to corresponding
>>>> image sensor device nodes would be put in a separate property.
>>
>> Could you give examples of the cases you are thinking of?
>
> I don't have any examples in mind ATM, I just wanted to point out
> the above convention might not be flexible enough. Especially since
> we already know there is more sub-devices within the camera module
> than just flashes and image sensors.
>
>>>> camera-flashes = <&flash_xx>, ...
>>>> camera-flash-masters = <&image_sensor_x>, ...
>>>>
>>>> Then pairs at same index would describe a single flash, 0 would indicate
>>>> a null entry if needed.
>>
>> When it should be needed?
>
> Not sure if there is a real use case for null entries, it was just to
> note we can skip any entry if needed - probably an irrelevant comment.
> I could imagine 2 LEDs of which one is only triggered in software, so
> it wouldn't have a 'camera-flash-masters' entry.
>
>>>> Similarly we could create properties for other sub-devices of a camera
>>>> module, like lenses, etc.

I have had the ninth version of the patch set ready to send today,
so in view of your doubts I made the property samsung specific
so as not to prevent us from going further while we will be
discussing the implementation of the generic property.
The 9th version of the patch set has been just sent.

-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Mon, May 25, 2015 at 04:28:22PM +0200, Sylwester Nawrocki wrote:
> On 25/05/15 14:50, Jacek Anaszewski wrote:
> >> On 23/05/15 14:03, Sakari Ailus wrote:
> >>> >> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >>>> >>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> >>> >>
> >>> >> One more matter to consider: xenon flash devices.
> >>> >>
> >>> >> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> >>> >> this awhile, I'm ok with removing the vendor prefix as well.
> >>> >>
> >>> >> Let me know what you think.
> >> >
> >> > I thought about it a bit more and I have some doubts about semantics
> >> > as above. I'm fine with 'camera-flashes' as far as name is concerned.
> >> >
> >> > Perhaps we should put only phandles to leds or xenon flash devices
> >> > in the 'camera-flashes' property. I think it would be more future
> >> > proof in case there is more nodes needed to describe the camera flash
> >> > (or a camera module) than the above two. And phandles to corresponding
> >> > image sensor device nodes would be put in a separate property.
> >
> > Could you give examples of the cases you are thinking of?
> 
> I don't have any examples in mind ATM, I just wanted to point out
> the above convention might not be flexible enough. Especially since
> we already know there is more sub-devices within the camera module
> than just flashes and image sensors.

I have to admit I've never seen a camera module with an integrated flash.
The lens is part of the module but typically flash is not. That doesn't say
there aren't such devices though.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Mon, May 25, 2015 at 02:00:33PM +0200, Sylwester Nawrocki wrote:
> Hi,
> 
> On 23/05/15 14:03, Sakari Ailus wrote:
> > On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> > 
> > One more matter to consider: xenon flash devices.
> > 
> > How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> > this awhile, I'm ok with removing the vendor prefix as well.
> > 
> > Let me know what you think.
> 
> I thought about it a bit more and I have some doubts about semantics
> as above. I'm fine with 'camera-flashes' as far as name is concerned.
> 
> Perhaps we should put only phandles to leds or xenon flash devices
> in the 'camera-flashes' property. I think it would be more future
> proof in case there is more nodes needed to describe the camera flash
> (or a camera module) than the above two. And phandles to corresponding
> image sensor device nodes would be put in a separate property.
> 
> camera-flashes = <&flash_xx>, ...
> camera-flash-masters = <&image_sensor_x>, ...
> 
> Then pairs at same index would describe a single flash, 0 would indicate
> a null entry if needed.
> Similarly we could create properties for other sub-devices of a camera
> module, like lenses, etc.

This arrangement would be advantageous compared to a single property when
adding modules (or lenses) to the equation, and probably more future proof
than "samsung,camera-flashes" / "ti,camera-flashes" I believe.

I'm also ok with keeping it as-is though.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Mon, May 25, 2015 at 02:00:33PM +0200, Sylwester Nawrocki wrote:
> Hi,
> 
> On 23/05/15 14:03, Sakari Ailus wrote:
> > On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> > 
> > One more matter to consider: xenon flash devices.
> > 
> > How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> > this awhile, I'm ok with removing the vendor prefix as well.
> > 
> > Let me know what you think.
> 
> I thought about it a bit more and I have some doubts about semantics
> as above. I'm fine with 'camera-flashes' as far as name is concerned.
> 
> Perhaps we should put only phandles to leds or xenon flash devices
> in the 'camera-flashes' property. I think it would be more future
> proof in case there is more nodes needed to describe the camera flash
> (or a camera module) than the above two. And phandles to corresponding
> image sensor device nodes would be put in a separate property.
> 
> camera-flashes = <&flash_xx>, ...
> camera-flash-masters = <&image_sensor_x>, ...
> 
> Then pairs at same index would describe a single flash, 0 would indicate
> a null entry if needed.
> Similarly we could create properties for other sub-devices of a camera
> module, like lenses, etc.

This arrangement would be advantageous compared to a single property when
adding modules (or lenses) to the equation, and probably more future proof
than "samsung,camera-flashes" / "ti,camera-flashes" I believe.

I'm also ok with keeping it as-is though.

-- 
Kind regards,

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