linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/12] LED / flash API integration
@ 2015-03-31 13:52 Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
                   ` (8 more replies)
  0 siblings, 9 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski

This is a fourth non-RFC version of LED / flash API integration
series [1]. It is based on linux-next_20150330.

================
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

Jacek Anaszewski (12):
  DT: leds: Improve description of flash LEDs related properties
  leds: unify the location of led-trigger API
  leds: Add support for max77693 mfd flash cell
  DT: Add documentation for the mfd Maxim max77693
  leds: Add driver for AAT1290 flash LED controller
  of: Add Skyworks Solutions, Inc. vendor prefix
  DT: Add documentation for the Skyworks AAT1290
  media: Add registration helpers for V4L2 flash sub-devices
  Documentation: leds: Add description of v4l2-flash sub-device
  leds: max77693: add support for V4L2 Flash sub-device
  leds: aat1290: add support for V4L2 Flash sub-device
  DT: aat1290: Document handling external strobe sources

 Documentation/devicetree/bindings/leds/common.txt  |   16 +-
 .../devicetree/bindings/leds/leds-aat1290.txt      |   70 ++
 Documentation/devicetree/bindings/mfd/max77693.txt |   61 ++
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 Documentation/leds/leds-class-flash.txt            |   47 +
 drivers/leds/Kconfig                               |   19 +
 drivers/leds/Makefile                              |    2 +
 drivers/leds/leds-aat1290.c                        |  518 +++++++++
 drivers/leds/leds-max77693.c                       | 1094 ++++++++++++++++++++
 drivers/leds/leds.h                                |   24 -
 drivers/media/v4l2-core/Kconfig                    |   11 +
 drivers/media/v4l2-core/Makefile                   |    2 +
 drivers/media/v4l2-core/v4l2-flash.c               |  619 +++++++++++
 include/linux/leds.h                               |   24 +
 include/media/v4l2-flash.h                         |  145 +++
 15 files changed, 2622 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-aat1290.txt
 create mode 100644 drivers/leds/leds-aat1290.c
 create mode 100644 drivers/leds/leds-max77693.c
 create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
 create mode 100644 include/media/v4l2-flash.h

-- 
1.7.9.5


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

* [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-04-02 14:41   ` Pavel Machek
                     ` (2 more replies)
  2015-03-31 13:52 ` [PATCH v4 02/12] leds: unify the location of led-trigger API Jacek Anaszewski
                   ` (7 subsequent siblings)
  8 siblings, 3 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski, Sakari Ailus, devicetree

Description of flash LEDs related properties was not precise regarding
the state of corresponding settings in case a property is missing.
Add relevant statements.
Removed is also the requirement making the flash-max-microamp
property obligatory for flash LEDs. It was inconsistent as the property
is defined as optional. Devices which require the property will have
to assert this in their DT bindings.

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: Pavel Machek <pavel@ucw.cz>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 747c538..21a25e4 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -29,13 +29,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
 
-- max-microamp : maximum intensity in microamperes of the LED
-		 (torch LED for flash devices)
-- flash-max-microamp : maximum intensity in microamperes of the
-                       flash LED; it is mandatory if the LED should
-		       support the flash mode
-- flash-timeout-us : timeout in microseconds after which the flash
-                     LED is turned off
+- max-microamp : Maximum intensity in microamperes of the LED
+		 (torch LED for flash devices). If omitted this will default
+		 to the maximum current allowed by the device.
+- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
+		       If omitted this will default to the maximum
+		       current allowed by the device.
+- flash-timeout-us : Timeout in microseconds after which the flash
+                     LED is turned off. If omitted this will default to the
+		     maximum timeout allowed by the device.
 
 
 Examples:
-- 
1.7.9.5


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

* [PATCH v4 02/12] leds: unify the location of led-trigger API
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-04-08  1:25   ` Bryan Wu
  2015-03-31 13:52 ` [PATCH v4 03/12] leds: Add support for max77693 mfd flash cell Jacek Anaszewski
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski

Part of led-trigger API was in the private drivers/leds/leds.h header.
Move it to the include/linux/leds.h header to unify the API location
and announce it as public. It has been already exported from
led-triggers.c with EXPORT_SYMBOL_GPL macro.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
---
 drivers/leds/leds.h  |   24 ------------------------
 include/linux/leds.h |   24 ++++++++++++++++++++++++
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 79efe57..bc89d7a 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -13,7 +13,6 @@
 #ifndef __LEDS_H_INCLUDED
 #define __LEDS_H_INCLUDED
 
-#include <linux/device.h>
 #include <linux/rwsem.h>
 #include <linux/leds.h>
 
@@ -50,27 +49,4 @@ void led_stop_software_blink(struct led_classdev *led_cdev);
 extern struct rw_semaphore leds_list_lock;
 extern struct list_head leds_list;
 
-#ifdef CONFIG_LEDS_TRIGGERS
-void led_trigger_set_default(struct led_classdev *led_cdev);
-void led_trigger_set(struct led_classdev *led_cdev,
-			struct led_trigger *trigger);
-void led_trigger_remove(struct led_classdev *led_cdev);
-
-static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
-{
-	return led_cdev->trigger_data;
-}
-
-#else
-#define led_trigger_set_default(x) do {} while (0)
-#define led_trigger_set(x, y) do {} while (0)
-#define led_trigger_remove(x) do {} while (0)
-#define led_get_trigger_data(x) (NULL)
-#endif
-
-ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
-			const char *buf, size_t count);
-ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr,
-			char *buf);
-
 #endif	/* __LEDS_H_INCLUDED */
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 9a2b000..0579708 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -12,6 +12,7 @@
 #ifndef __LINUX_LEDS_H_INCLUDED
 #define __LINUX_LEDS_H_INCLUDED
 
+#include <linux/device.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/rwsem.h>
@@ -222,6 +223,29 @@ struct led_trigger {
 	struct list_head  next_trig;
 };
 
+#ifdef CONFIG_LEDS_TRIGGERS
+void led_trigger_set_default(struct led_classdev *led_cdev);
+void led_trigger_set(struct led_classdev *led_cdev,
+			struct led_trigger *trigger);
+void led_trigger_remove(struct led_classdev *led_cdev);
+
+static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
+{
+	return led_cdev->trigger_data;
+}
+
+#else
+#define led_trigger_set_default(x) do {} while (0)
+#define led_trigger_set(x, y) do {} while (0)
+#define led_trigger_remove(x) do {} while (0)
+#define led_get_trigger_data(x) (NULL)
+#endif
+
+ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t count);
+ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr,
+			char *buf);
+
 /* Registration functions for complex triggers */
 extern int led_trigger_register(struct led_trigger *trigger);
 extern void led_trigger_unregister(struct led_trigger *trigger);
-- 
1.7.9.5


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

* [PATCH v4 03/12] leds: Add support for max77693 mfd flash cell
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 02/12] leds: unify the location of led-trigger API Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 04/12] DT: Add documentation for the mfd Maxim max77693 Jacek Anaszewski
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski, Andrzej Hajda, Lee Jones,
	Chanwoo Choi

This patch adds led-flash support to Maxim max77693 chipset.
A device can be exposed to user space through LED subsystem
sysfs interface. Device supports up to two leds which can
work in flash and torch mode. The leds can be triggered
externally or by software.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/leds/Kconfig         |   10 +
 drivers/leds/Makefile        |    1 +
 drivers/leds/leds-max77693.c |  981 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 992 insertions(+)
 create mode 100644 drivers/leds/leds-max77693.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 966b960..f9fbeb5 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -467,6 +467,16 @@ config LEDS_TCA6507
 	  LED driver chips accessed via the I2C bus.
 	  Driver support brightness control and hardware-assisted blinking.
 
+config LEDS_MAX77693
+	tristate "LED support for MAX77693 Flash"
+	depends on LEDS_CLASS_FLASH
+	depends on MFD_MAX77693
+	depends on OF
+	help
+	  This option enables support for the flash part of the MAX77693
+	  multifunction device. It has build in control for two leds in flash
+	  and torch mode.
+
 config LEDS_MAX8997
 	tristate "LED support for MAX8997 PMIC"
 	depends on LEDS_CLASS && MFD_MAX8997
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index bf46093..9413fdb 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_LEDS_MC13783)		+= leds-mc13783.o
 obj-$(CONFIG_LEDS_NS2)			+= leds-ns2.o
 obj-$(CONFIG_LEDS_NETXBIG)		+= leds-netxbig.o
 obj-$(CONFIG_LEDS_ASIC3)		+= leds-asic3.o
+obj-$(CONFIG_LEDS_MAX77693)		+= leds-max77693.o
 obj-$(CONFIG_LEDS_MAX8997)		+= leds-max8997.o
 obj-$(CONFIG_LEDS_LM355x)		+= leds-lm355x.o
 obj-$(CONFIG_LEDS_BLINKM)		+= leds-blinkm.o
diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
new file mode 100644
index 0000000..aa974be
--- /dev/null
+++ b/drivers/leds/leds-max77693.c
@@ -0,0 +1,981 @@
+/*
+ * LED Flash class driver for the flash cell of max77693 mfd.
+ *
+ *	Copyright (C) 2015, Samsung Electronics Co., Ltd.
+ *
+ *	Authors: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *		 Andrzej Hajda <a.hajda@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/led-class-flash.h>
+#include <linux/mfd/max77693.h>
+#include <linux/mfd/max77693-private.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#define MODE_OFF		0
+#define MODE_FLASH(a)		(1 << (a))
+#define MODE_TORCH(a)		(1 << (2 + (a)))
+#define MODE_FLASH_EXTERNAL(a)	(1 << (4 + (a)))
+
+#define MODE_FLASH_MASK		(MODE_FLASH(FLED1) | MODE_FLASH(FLED2) | \
+				 MODE_FLASH_EXTERNAL(FLED1) | \
+				 MODE_FLASH_EXTERNAL(FLED2))
+#define MODE_TORCH_MASK		(MODE_TORCH(FLED1) | MODE_TORCH(FLED2))
+
+#define FLED1_IOUT		(1 << 0)
+#define FLED2_IOUT		(1 << 1)
+
+enum max77693_fled {
+	FLED1,
+	FLED2,
+};
+
+enum max77693_led_mode {
+	FLASH,
+	TORCH,
+};
+
+struct max77693_led_config_data {
+	const char *label[2];
+	u32 iout_torch_max[2];
+	u32 iout_flash_max[2];
+	u32 flash_timeout[2];
+	u32 num_leds;
+	u32 boost_mode;
+	u32 boost_vout;
+	u32 low_vsys;
+};
+
+struct max77693_sub_led {
+	/* corresponding FLED output identifier */
+	int fled_id;
+	/* corresponding LED Flash class device */
+	struct led_classdev_flash fled_cdev;
+	/* assures led-triggers compatibility */
+	struct work_struct work_brightness_set;
+
+	/* brightness cache */
+	unsigned int torch_brightness;
+	/* flash timeout cache */
+	unsigned int flash_timeout;
+	/* flash faults that may have occurred */
+	u32 flash_faults;
+};
+
+struct max77693_led_device {
+	/* parent mfd regmap */
+	struct regmap *regmap;
+	/* platform device data */
+	struct platform_device *pdev;
+	/* secures access to the device */
+	struct mutex lock;
+
+	/* sub led data */
+	struct max77693_sub_led sub_leds[2];
+
+	/* maximum torch current values for FLED outputs */
+	u32 iout_torch_max[2];
+	/* maximum flash current values for FLED outputs */
+	u32 iout_flash_max[2];
+
+	/* current flash timeout cache */
+	unsigned int current_flash_timeout;
+	/* ITORCH register cache */
+	u8 torch_iout_reg;
+	/* mode of fled outputs */
+	unsigned int mode_flags;
+	/* recently strobed fled */
+	int strobing_sub_led_id;
+	/* bitmask of FLED outputs use state (bit 0. - FLED1, bit 1. - FLED2) */
+	u8 fled_mask;
+	/* FLED modes that can be set */
+	u8 allowed_modes;
+
+	/* arrangement of current outputs */
+	bool iout_joint;
+};
+
+static u8 max77693_led_iout_to_reg(u32 ua)
+{
+	if (ua < FLASH_IOUT_MIN)
+		ua = FLASH_IOUT_MIN;
+	return (ua - FLASH_IOUT_MIN) / FLASH_IOUT_STEP;
+}
+
+static u8 max77693_flash_timeout_to_reg(u32 us)
+{
+	return (us - FLASH_TIMEOUT_MIN) / FLASH_TIMEOUT_STEP;
+}
+
+static inline struct max77693_sub_led *flcdev_to_sub_led(
+					struct led_classdev_flash *fled_cdev)
+{
+	return container_of(fled_cdev, struct max77693_sub_led, fled_cdev);
+}
+
+static inline struct max77693_led_device *sub_led_to_led(
+					struct max77693_sub_led *sub_led)
+{
+	return container_of(sub_led, struct max77693_led_device,
+				sub_leds[sub_led->fled_id]);
+}
+
+static inline u8 max77693_led_vsys_to_reg(u32 mv)
+{
+	return ((mv - MAX_FLASH1_VSYS_MIN) / MAX_FLASH1_VSYS_STEP) << 2;
+}
+
+static inline u8 max77693_led_vout_to_reg(u32 mv)
+{
+	return (mv - FLASH_VOUT_MIN) / FLASH_VOUT_STEP + FLASH_VOUT_RMIN;
+}
+
+static inline bool max77693_fled_used(struct max77693_led_device *led,
+					 int fled_id)
+{
+	u8 fled_bit = (fled_id == FLED1) ? FLED1_IOUT : FLED2_IOUT;
+
+	return led->fled_mask & fled_bit;
+}
+
+static int max77693_set_mode_reg(struct max77693_led_device *led, u8 mode)
+{
+	struct regmap *rmap = led->regmap;
+	int ret, v = 0, i;
+
+	for (i = FLED1; i <= FLED2; ++i) {
+		if (mode & MODE_TORCH(i))
+			v |= FLASH_EN_ON << TORCH_EN_SHIFT(i);
+
+		if (mode & MODE_FLASH(i)) {
+			v |= FLASH_EN_ON << FLASH_EN_SHIFT(i);
+		} else if (mode & MODE_FLASH_EXTERNAL(i)) {
+			v |= FLASH_EN_FLASH << FLASH_EN_SHIFT(i);
+			/*
+			 * Enable hw triggering also for torch mode, as some
+			 * camera sensors use torch led to fathom ambient light
+			 * conditions before strobing the flash.
+			 */
+			v |= FLASH_EN_TORCH << TORCH_EN_SHIFT(i);
+		}
+	}
+
+	/* Reset the register only prior setting flash modes */
+	if (mode & ~(MODE_TORCH(FLED1) | MODE_TORCH(FLED2))) {
+		ret = regmap_write(rmap, MAX77693_LED_REG_FLASH_EN, 0);
+		if (ret < 0)
+			return ret;
+	}
+
+	return regmap_write(rmap, MAX77693_LED_REG_FLASH_EN, v);
+}
+
+static int max77693_add_mode(struct max77693_led_device *led, u8 mode)
+{
+	int i, ret;
+
+	mode &= led->allowed_modes;
+
+	/*
+	 * Torch mode once enabled remains active until turned off. If the FLED2
+	 * output isn't to be disabled check if the torch mode to be set isn't
+	 * already activated and avoid re-setting it.
+	 */
+	if ((!(mode ^ led->mode_flags)) & MODE_TORCH(FLED2)) {
+		for (i = FLED1; i <= FLED2; ++i)
+			if ((mode & led->mode_flags & MODE_TORCH(i)))
+				return 0;
+	}
+
+	if (led->iout_joint)
+		/* Span the mode on FLED2 for joint iouts case */
+		mode |= (mode << 1);
+
+	/*
+	 * FLASH_EXTERNAL mode activates FLASHEN and TORCHEN pins in the device.
+	 * Corresponding register bit fields interfere with SW triggered modes,
+	 * thus clear them to ensure proper device configuration.
+	 */
+	for (i = FLED1; i <= FLED2; ++i)
+		if (mode & MODE_FLASH_EXTERNAL(i))
+			led->mode_flags &= (~MODE_TORCH(i) & ~MODE_FLASH(i));
+
+	led->mode_flags |= mode;
+	led->mode_flags &= led->allowed_modes;
+
+	ret = max77693_set_mode_reg(led, led->mode_flags);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Clear flash mode flag after setting the mode to avoid spurious flash
+	 * strobing on each subsequent torch mode setting.
+	 */
+	if (mode & MODE_FLASH_MASK)
+		led->mode_flags &= ~mode;
+
+	return ret;
+}
+
+static int max77693_clear_mode(struct max77693_led_device *led,
+				u8 mode)
+{
+	if (led->iout_joint)
+		/* Clear mode also on FLED2 for joint iouts case */
+		mode |= (mode << 1);
+
+	led->mode_flags &= ~mode;
+
+	return max77693_set_mode_reg(led, led->mode_flags);
+}
+
+static void max77693_add_allowed_modes(struct max77693_led_device *led,
+				int fled_id, enum max77693_led_mode mode)
+{
+	if (mode == FLASH)
+		led->allowed_modes |= (MODE_FLASH(fled_id) |
+				       MODE_FLASH_EXTERNAL(fled_id));
+	else
+		led->allowed_modes |= MODE_TORCH(fled_id);
+}
+
+static void max77693_distribute_currents(struct max77693_led_device *led,
+				int fled_id, enum max77693_led_mode mode,
+				u32 micro_amp, u32 iout_max[2], u32 iout[2])
+{
+	if (!led->iout_joint) {
+		iout[fled_id] = micro_amp;
+		max77693_add_allowed_modes(led, fled_id, mode);
+		return;
+	}
+
+	iout[FLED1] = min(micro_amp, iout_max[FLED1]);
+	iout[FLED2] = micro_amp - iout[FLED1];
+
+	if (mode == FLASH)
+		led->allowed_modes &= ~MODE_FLASH_MASK;
+	else
+		led->allowed_modes &= ~MODE_TORCH_MASK;
+
+	max77693_add_allowed_modes(led, FLED1, mode);
+
+	if (iout[FLED2])
+		max77693_add_allowed_modes(led, FLED2, mode);
+}
+
+static int max77693_set_torch_current(struct max77693_led_device *led,
+				int fled_id, u32 micro_amp)
+{
+	struct regmap *rmap = led->regmap;
+	u8 iout1_reg = 0, iout2_reg = 0;
+	u32 iout[2];
+
+	max77693_distribute_currents(led, fled_id, TORCH, micro_amp,
+					led->iout_torch_max, iout);
+
+	if (fled_id == FLED1 || led->iout_joint) {
+		iout1_reg = max77693_led_iout_to_reg(iout[FLED1]);
+		led->torch_iout_reg &= TORCH_IOUT_MASK(TORCH_IOUT2_SHIFT);
+	}
+	if (fled_id == FLED2 || led->iout_joint) {
+		iout2_reg = max77693_led_iout_to_reg(iout[FLED2]);
+		led->torch_iout_reg &= TORCH_IOUT_MASK(TORCH_IOUT1_SHIFT);
+	}
+
+	led->torch_iout_reg |= ((iout1_reg << TORCH_IOUT1_SHIFT) |
+				(iout2_reg << TORCH_IOUT2_SHIFT));
+
+	return regmap_write(rmap, MAX77693_LED_REG_ITORCH,
+						led->torch_iout_reg);
+}
+
+static int max77693_set_flash_current(struct max77693_led_device *led,
+					int fled_id,
+					u32 micro_amp)
+{
+	struct regmap *rmap = led->regmap;
+	u8 iout1_reg, iout2_reg;
+	u32 iout[2];
+	int ret = -EINVAL;
+
+	max77693_distribute_currents(led, fled_id, FLASH, micro_amp,
+					led->iout_flash_max, iout);
+
+	if (fled_id == FLED1 || led->iout_joint) {
+		iout1_reg = max77693_led_iout_to_reg(iout[FLED1]);
+		ret = regmap_write(rmap, MAX77693_LED_REG_IFLASH1,
+							iout1_reg);
+		if (ret < 0)
+			return ret;
+	}
+	if (fled_id == FLED2 || led->iout_joint) {
+		iout2_reg = max77693_led_iout_to_reg(iout[FLED2]);
+		ret = regmap_write(rmap, MAX77693_LED_REG_IFLASH2,
+							iout2_reg);
+	}
+
+	return ret;
+}
+
+static int max77693_set_timeout(struct max77693_led_device *led, u32 microsec)
+{
+	struct regmap *rmap = led->regmap;
+	u8 v;
+	int ret;
+
+	v = max77693_flash_timeout_to_reg(microsec) | FLASH_TMR_LEVEL;
+
+	ret = regmap_write(rmap, MAX77693_LED_REG_FLASH_TIMER, v);
+	if (ret < 0)
+		return ret;
+
+	led->current_flash_timeout = microsec;
+
+	return 0;
+}
+
+static int max77693_get_strobe_status(struct max77693_led_device *led,
+					bool *state)
+{
+	struct regmap *rmap = led->regmap;
+	unsigned int v;
+	int ret;
+
+	ret = regmap_read(rmap, MAX77693_LED_REG_FLASH_STATUS, &v);
+	if (ret < 0)
+		return ret;
+
+	*state = v & FLASH_STATUS_FLASH_ON;
+
+	return ret;
+}
+
+static int max77693_get_flash_faults(struct max77693_sub_led *sub_led)
+{
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	struct regmap *rmap = led->regmap;
+	unsigned int v;
+	u8 fault_open_mask, fault_short_mask;
+	int ret;
+
+	sub_led->flash_faults = 0;
+
+	if (led->iout_joint) {
+		fault_open_mask = FLASH_INT_FLED1_OPEN | FLASH_INT_FLED2_OPEN;
+		fault_short_mask = FLASH_INT_FLED1_SHORT |
+							FLASH_INT_FLED2_SHORT;
+	} else {
+		fault_open_mask = (sub_led->fled_id == FLED1) ?
+						FLASH_INT_FLED1_OPEN :
+						FLASH_INT_FLED2_OPEN;
+		fault_short_mask = (sub_led->fled_id == FLED1) ?
+						FLASH_INT_FLED1_SHORT :
+						FLASH_INT_FLED2_SHORT;
+	}
+
+	ret = regmap_read(rmap, MAX77693_LED_REG_FLASH_INT, &v);
+	if (ret < 0)
+		return ret;
+
+	if (v & fault_open_mask)
+		sub_led->flash_faults |= LED_FAULT_OVER_VOLTAGE;
+	if (v & fault_short_mask)
+		sub_led->flash_faults |= LED_FAULT_SHORT_CIRCUIT;
+	if (v & FLASH_INT_OVER_CURRENT)
+		sub_led->flash_faults |= LED_FAULT_OVER_CURRENT;
+
+	return 0;
+}
+
+static int max77693_setup(struct max77693_led_device *led,
+			 struct max77693_led_config_data *led_cfg)
+{
+	struct regmap *rmap = led->regmap;
+	int i, first_led, last_led, ret;
+	u32 max_flash_curr[2];
+	u8 v;
+
+	/*
+	 * Initialize only flash current. Torch current doesn't
+	 * require initialization as ITORCH register is written with
+	 * new value each time brightness_set op is called.
+	 */
+	if (led->iout_joint) {
+		first_led = FLED1;
+		last_led = FLED1;
+		max_flash_curr[FLED1] = led_cfg->iout_flash_max[FLED1] +
+					led_cfg->iout_flash_max[FLED2];
+	} else {
+		first_led = max77693_fled_used(led, FLED1) ? FLED1 : FLED2;
+		last_led = max77693_fled_used(led, FLED2) ? FLED2 : FLED1;
+		max_flash_curr[FLED1] = led_cfg->iout_flash_max[FLED1];
+		max_flash_curr[FLED2] = led_cfg->iout_flash_max[FLED2];
+	}
+
+	for (i = first_led; i <= last_led; ++i) {
+		ret = max77693_set_flash_current(led, i,
+					max_flash_curr[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	v = TORCH_TMR_NO_TIMER | MAX77693_LED_TRIG_TYPE_LEVEL;
+	ret = regmap_write(rmap, MAX77693_LED_REG_ITORCHTIMER, v);
+	if (ret < 0)
+		return ret;
+
+	if (led_cfg->low_vsys > 0)
+		v = max77693_led_vsys_to_reg(led_cfg->low_vsys) |
+						MAX_FLASH1_MAX_FL_EN;
+	else
+		v = 0;
+
+	ret = regmap_write(rmap, MAX77693_LED_REG_MAX_FLASH1, v);
+	if (ret < 0)
+		return ret;
+	ret = regmap_write(rmap, MAX77693_LED_REG_MAX_FLASH2, 0);
+	if (ret < 0)
+		return ret;
+
+	if (led_cfg->boost_mode == MAX77693_LED_BOOST_FIXED)
+		v = FLASH_BOOST_FIXED;
+	else
+		v = led_cfg->boost_mode | led_cfg->boost_mode << 1;
+
+	if (max77693_fled_used(led, FLED1) && max77693_fled_used(led, FLED2))
+		v |= FLASH_BOOST_LEDNUM_2;
+
+	ret = regmap_write(rmap, MAX77693_LED_REG_VOUT_CNTL, v);
+	if (ret < 0)
+		return ret;
+
+	v = max77693_led_vout_to_reg(led_cfg->boost_vout);
+	ret = regmap_write(rmap, MAX77693_LED_REG_VOUT_FLASH1, v);
+	if (ret < 0)
+		return ret;
+
+	return max77693_set_mode_reg(led, MODE_OFF);
+}
+
+static int __max77693_led_brightness_set(struct max77693_led_device *led,
+					int fled_id, enum led_brightness value)
+{
+	int ret;
+
+	mutex_lock(&led->lock);
+
+	if (value == 0) {
+		ret = max77693_clear_mode(led, MODE_TORCH(fled_id));
+		if (ret < 0)
+			dev_dbg(&led->pdev->dev,
+				"Failed to clear torch mode (%d)\n",
+				ret);
+		goto unlock;
+	}
+
+	ret = max77693_set_torch_current(led, fled_id, value * TORCH_IOUT_STEP);
+	if (ret < 0) {
+		dev_dbg(&led->pdev->dev,
+			"Failed to set torch current (%d)\n",
+			ret);
+		goto unlock;
+	}
+
+	ret = max77693_add_mode(led, MODE_TORCH(fled_id));
+	if (ret < 0)
+		dev_dbg(&led->pdev->dev,
+			"Failed to set torch mode (%d)\n",
+			ret);
+unlock:
+	mutex_unlock(&led->lock);
+	return ret;
+}
+
+static void max77693_led_brightness_set_work(
+					struct work_struct *work)
+{
+	struct max77693_sub_led *sub_led =
+			container_of(work, struct max77693_sub_led,
+					work_brightness_set);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+
+	__max77693_led_brightness_set(led, sub_led->fled_id,
+				sub_led->torch_brightness);
+}
+
+/* LED subsystem callbacks */
+
+static int max77693_led_brightness_set_sync(
+				struct led_classdev *led_cdev,
+				enum led_brightness value)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+
+	return __max77693_led_brightness_set(led, sub_led->fled_id, value);
+}
+
+static void max77693_led_brightness_set(
+				struct led_classdev *led_cdev,
+				enum led_brightness value)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+
+	sub_led->torch_brightness = value;
+	schedule_work(&sub_led->work_brightness_set);
+}
+
+static int max77693_led_flash_brightness_set(
+				struct led_classdev_flash *fled_cdev,
+				u32 brightness)
+{
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	int ret;
+
+	mutex_lock(&led->lock);
+	ret = max77693_set_flash_current(led, sub_led->fled_id, brightness);
+	mutex_unlock(&led->lock);
+
+	return ret;
+}
+
+static int max77693_led_flash_strobe_set(
+				struct led_classdev_flash *fled_cdev,
+				bool state)
+{
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(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 (!state) {
+		ret = max77693_clear_mode(led, MODE_FLASH(fled_id));
+		goto unlock;
+	}
+
+	if (sub_led->flash_timeout != led->current_flash_timeout) {
+		ret = max77693_set_timeout(led, sub_led->flash_timeout);
+		if (ret < 0)
+			goto unlock;
+	}
+
+	led->strobing_sub_led_id = fled_id;
+
+	ret = max77693_add_mode(led, MODE_FLASH(fled_id));
+	if (ret < 0)
+		goto unlock;
+
+	ret = max77693_get_flash_faults(sub_led);
+
+unlock:
+	mutex_unlock(&led->lock);
+	return ret;
+}
+
+static int max77693_led_flash_fault_get(
+				struct led_classdev_flash *fled_cdev,
+				u32 *fault)
+{
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+
+	*fault = sub_led->flash_faults;
+
+	return 0;
+}
+
+static int max77693_led_flash_strobe_get(
+				struct led_classdev_flash *fled_cdev,
+				bool *state)
+{
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	int ret;
+
+	if (!state)
+		return -EINVAL;
+
+	mutex_lock(&led->lock);
+
+	ret = max77693_get_strobe_status(led, state);
+
+	*state = !!(*state && (led->strobing_sub_led_id == sub_led->fled_id));
+
+	mutex_unlock(&led->lock);
+
+	return ret;
+}
+
+static int max77693_led_flash_timeout_set(
+				struct led_classdev_flash *fled_cdev,
+				u32 timeout)
+{
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+
+	mutex_lock(&led->lock);
+	sub_led->flash_timeout = timeout;
+	mutex_unlock(&led->lock);
+
+	return 0;
+}
+
+static int max77693_led_parse_dt(struct max77693_led_device *led,
+				struct max77693_led_config_data *cfg)
+{
+	struct device *dev = &led->pdev->dev;
+	struct max77693_sub_led *sub_leds = led->sub_leds;
+	struct device_node *node = dev->of_node, *child_node;
+	struct property *prop;
+	u32 led_sources[2];
+	int i, fled_id;
+
+	of_property_read_u32(node, "maxim,boost-mode", &cfg->boost_mode);
+	of_property_read_u32(node, "maxim,boost-mvout", &cfg->boost_vout);
+	of_property_read_u32(node, "maxim,mvsys-min", &cfg->low_vsys);
+
+	for_each_available_child_of_node(node, child_node) {
+		prop = of_find_property(child_node, "led-sources", NULL);
+		if (prop) {
+			const __be32 *srcs = NULL;
+
+			for (i = 0; i < ARRAY_SIZE(led_sources); ++i) {
+				srcs = of_prop_next_u32(prop, srcs,
+							&led_sources[i]);
+				if (!srcs)
+					break;
+			}
+		} else {
+			dev_err(dev,
+				"\"led-sources\" DT property not found.\n");
+			return -EINVAL;
+		}
+
+		if (i == 2) {
+			fled_id = FLED1;
+			led->fled_mask = FLED1_IOUT | FLED2_IOUT;
+		} else if (led_sources[0] == FLED1) {
+			fled_id = FLED1;
+			led->fled_mask |= FLED1_IOUT;
+		} else if (led_sources[0] == FLED2) {
+			fled_id = FLED2;
+			led->fled_mask |= FLED2_IOUT;
+		} else {
+			dev_err(dev,
+				"Wrong \"led-sources\" DT property value.\n");
+			return -EINVAL;
+		}
+
+		sub_leds[fled_id].fled_id = fled_id;
+
+		cfg->label[fled_id] =
+			of_get_property(child_node, "label", NULL) ? :
+						child_node->name;
+
+		of_property_read_u32(child_node, "max-microamp",
+						&cfg->iout_torch_max[fled_id]);
+		of_property_read_u32(child_node, "flash-max-microamp",
+						&cfg->iout_flash_max[fled_id]);
+		of_property_read_u32(child_node, "flash-timeout-us",
+						&cfg->flash_timeout[fled_id]);
+
+		if (++cfg->num_leds == 2 ||
+		    (max77693_fled_used(led, FLED1) &&
+		     max77693_fled_used(led, FLED2)))
+			break;
+	}
+
+	if (cfg->num_leds == 0) {
+		dev_err(dev, "No DT child node found for connected LED(s).\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void clamp_align(u32 *v, u32 min, u32 max, u32 step)
+{
+	*v = clamp_val(*v, min, max);
+	if (step > 1)
+		*v = (*v - min) / step * step + min;
+}
+
+static void max77693_align_iout_current(struct max77693_led_device *led,
+					u32 *iout, u32 min, u32 max, u32 step)
+{
+	int i;
+
+	/* Handle no DT property case */
+	for (i = FLED1; i <= FLED2; ++i)
+		if (iout[i] == 0)
+			iout[i] = max;
+
+	if (led->iout_joint) {
+		if (iout[FLED1] > min) {
+			iout[FLED1] /= 2;
+			iout[FLED2] = iout[FLED1];
+		} else {
+			iout[FLED1] = min;
+			iout[FLED2] = 0;
+			return;
+		}
+	}
+
+	for (i = FLED1; i <= FLED2; ++i)
+		if (max77693_fled_used(led, i))
+			clamp_align(&iout[i], min, max, step);
+		else
+			iout[i] = 0;
+}
+
+static void max77693_led_validate_configuration(struct max77693_led_device *led,
+					struct max77693_led_config_data *cfg)
+{
+	u32 flash_iout_max = cfg->boost_mode ? FLASH_IOUT_MAX_2LEDS :
+					       FLASH_IOUT_MAX_1LED;
+	int i;
+
+	if (cfg->num_leds == 1 &&
+	    max77693_fled_used(led, FLED1) && max77693_fled_used(led, FLED2))
+		led->iout_joint = true;
+
+	cfg->boost_mode = clamp_val(cfg->boost_mode, MAX77693_LED_BOOST_NONE,
+			    MAX77693_LED_BOOST_FIXED);
+
+	/* Boost must be enabled if both current outputs are used */
+	if ((cfg->boost_mode == MAX77693_LED_BOOST_NONE) && led->iout_joint)
+		cfg->boost_mode = MAX77693_LED_BOOST_FIXED;
+
+	max77693_align_iout_current(led, cfg->iout_torch_max,
+			TORCH_IOUT_MIN, TORCH_IOUT_MAX, TORCH_IOUT_STEP);
+
+	max77693_align_iout_current(led, cfg->iout_flash_max,
+			FLASH_IOUT_MIN, flash_iout_max, FLASH_IOUT_STEP);
+
+	for (i = 0; i < ARRAY_SIZE(cfg->flash_timeout); ++i) {
+		if (cfg->flash_timeout[i] == 0) {
+			cfg->flash_timeout[i] = FLASH_TIMEOUT_MAX;
+			continue;
+		}
+		clamp_align(&cfg->flash_timeout[i], FLASH_TIMEOUT_MIN,
+				FLASH_TIMEOUT_MAX, FLASH_TIMEOUT_STEP);
+	}
+
+	clamp_align(&cfg->boost_vout, FLASH_VOUT_MIN, FLASH_VOUT_MAX,
+							FLASH_VOUT_STEP);
+
+	if (cfg->low_vsys)
+		clamp_align(&cfg->low_vsys, MAX_FLASH1_VSYS_MIN,
+				MAX_FLASH1_VSYS_MAX, MAX_FLASH1_VSYS_STEP);
+}
+
+static int max77693_led_get_configuration(struct max77693_led_device *led,
+				struct max77693_led_config_data *cfg)
+{
+	int ret;
+
+	ret = max77693_led_parse_dt(led, cfg);
+	if (ret < 0)
+		return ret;
+
+	max77693_led_validate_configuration(led, cfg);
+
+	memcpy(led->iout_torch_max, cfg->iout_torch_max,
+				sizeof(led->iout_torch_max));
+	memcpy(led->iout_flash_max, cfg->iout_flash_max,
+				sizeof(led->iout_flash_max));
+
+	return 0;
+}
+
+static const struct led_flash_ops flash_ops = {
+	.flash_brightness_set	= max77693_led_flash_brightness_set,
+	.strobe_set		= max77693_led_flash_strobe_set,
+	.strobe_get		= max77693_led_flash_strobe_get,
+	.timeout_set		= max77693_led_flash_timeout_set,
+	.fault_get		= max77693_led_flash_fault_get,
+};
+
+static void max77693_init_flash_settings(struct max77693_sub_led *sub_led,
+				 struct max77693_led_config_data *led_cfg)
+{
+	struct led_classdev_flash *fled_cdev = &sub_led->fled_cdev;
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	int fled_id = sub_led->fled_id;
+	struct led_flash_setting *setting;
+
+	/* Init flash intensity setting */
+	setting = &fled_cdev->brightness;
+	setting->min = FLASH_IOUT_MIN;
+	setting->max = led->iout_joint ?
+		led_cfg->iout_flash_max[FLED1] +
+		led_cfg->iout_flash_max[FLED2] :
+		led_cfg->iout_flash_max[fled_id];
+	setting->step = FLASH_IOUT_STEP;
+	setting->val = setting->max;
+
+	/* Init flash timeout setting */
+	setting = &fled_cdev->timeout;
+	setting->min = FLASH_TIMEOUT_MIN;
+	setting->max = led_cfg->flash_timeout[fled_id];
+	setting->step = FLASH_TIMEOUT_STEP;
+	setting->val = setting->max;
+}
+
+static void max77693_init_fled_cdev(struct max77693_sub_led *sub_led,
+				struct max77693_led_config_data *led_cfg)
+{
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	int fled_id = sub_led->fled_id;
+	struct led_classdev_flash *fled_cdev;
+	struct led_classdev *led_cdev;
+
+	/* Initialize LED Flash class device */
+	fled_cdev = &sub_led->fled_cdev;
+	fled_cdev->ops = &flash_ops;
+	led_cdev = &fled_cdev->led_cdev;
+
+	led_cdev->name = led_cfg->label[fled_id];
+
+	led_cdev->brightness_set = max77693_led_brightness_set;
+	led_cdev->brightness_set_sync = max77693_led_brightness_set_sync;
+	led_cdev->max_brightness = (led->iout_joint ?
+					led_cfg->iout_torch_max[FLED1] +
+					led_cfg->iout_torch_max[FLED2] :
+					led_cfg->iout_torch_max[fled_id]) /
+				   TORCH_IOUT_STEP;
+	led_cdev->flags |= LED_DEV_CAP_FLASH;
+	INIT_WORK(&sub_led->work_brightness_set,
+			max77693_led_brightness_set_work);
+
+	max77693_init_flash_settings(sub_led, led_cfg);
+
+	/* Init flash timeout cache */
+	sub_led->flash_timeout = fled_cdev->timeout.val;
+}
+
+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 max77693_led_config_data led_cfg = {};
+	int init_fled_cdev[2], i, ret;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pdev = pdev;
+	led->regmap = iodev->regmap;
+	led->allowed_modes = MODE_FLASH_MASK;
+	sub_leds = led->sub_leds;
+
+	platform_set_drvdata(pdev, led);
+	ret = max77693_led_get_configuration(led, &led_cfg);
+	if (ret < 0)
+		return ret;
+
+	ret = max77693_setup(led, &led_cfg);
+	if (ret < 0)
+		return ret;
+
+	mutex_init(&led->lock);
+
+	init_fled_cdev[FLED1] =
+			led->iout_joint || max77693_fled_used(led, FLED1);
+	init_fled_cdev[FLED2] =
+			!led->iout_joint && max77693_fled_used(led, FLED2);
+
+	/* Initialize LED Flash class device(s) */
+	for (i = FLED1; i <= FLED2; ++i)
+		if (init_fled_cdev[i])
+			max77693_init_fled_cdev(&sub_leds[i], &led_cfg);
+
+
+	/* Register LED Flash class device(s) */
+	for (i = FLED1; i <= FLED2; ++i) {
+		if (!init_fled_cdev[i])
+			continue;
+
+		ret = led_classdev_flash_register(dev, &sub_leds[i].fled_cdev);
+		if (ret < 0) {
+			/*
+			 * At this moment FLED1 might have been already
+			 * registered and it needs to be released.
+			 */
+			if (i == FLED2)
+				goto err_register_led2;
+			else
+				goto err_register_led1;
+		}
+	}
+
+	return 0;
+
+err_register_led2:
+	/* It is possible than only FLED2 was to be registered */
+	if (!init_fled_cdev[FLED1])
+		goto err_register_led1;
+	led_classdev_flash_unregister(&sub_leds[FLED1].fled_cdev);
+err_register_led1:
+	mutex_destroy(&led->lock);
+
+	return ret;
+}
+
+static int max77693_led_remove(struct platform_device *pdev)
+{
+	struct max77693_led_device *led = platform_get_drvdata(pdev);
+	struct max77693_sub_led *sub_leds = led->sub_leds;
+
+	if (led->iout_joint || max77693_fled_used(led, FLED1)) {
+		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)) {
+		led_classdev_flash_unregister(&sub_leds[FLED2].fled_cdev);
+		cancel_work_sync(&sub_leds[FLED2].work_brightness_set);
+	}
+
+	mutex_destroy(&led->lock);
+
+	return 0;
+}
+
+static const struct of_device_id max77693_led_dt_match[] = {
+	{ .compatible = "maxim,max77693-led" },
+	{},
+};
+
+static struct platform_driver max77693_led_driver = {
+	.probe		= max77693_led_probe,
+	.remove		= max77693_led_remove,
+	.driver		= {
+		.name	= "max77693-led",
+		.owner	= THIS_MODULE,
+		.of_match_table = max77693_led_dt_match,
+	},
+};
+
+module_platform_driver(max77693_led_driver);
+
+MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
+MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
+MODULE_DESCRIPTION("Maxim MAX77693 led flash driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


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

* [PATCH v4 04/12] DT: Add documentation for the mfd Maxim max77693
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
                   ` (2 preceding siblings ...)
  2015-03-31 13:52 ` [PATCH v4 03/12] leds: Add support for max77693 mfd flash cell Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-03-31 14:34   ` Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 05/12] leds: Add driver for AAT1290 flash LED controller Jacek Anaszewski
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski, Andrzej Hajda, Lee Jones,
	Chanwoo Choi, devicetree

This patch adds device tree binding documentation for
the flash cell of the Maxim max77693 multifunctional device.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/mfd/max77693.txt |   61 ++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
index 38e6440..deb832f 100644
--- a/Documentation/devicetree/bindings/mfd/max77693.txt
+++ b/Documentation/devicetree/bindings/mfd/max77693.txt
@@ -76,7 +76,54 @@ Optional properties:
     Valid values: 4300000, 4700000, 4800000, 4900000
     Default: 4300000
 
+- led : the LED submodule device node
+
+There are two LED outputs available - FLED1 and FLED2. Each of them can
+control a separate LED or they can be connected together to double
+the maximum current for a single connected LED. One LED is represented
+by one child node.
+
+Required properties:
+- compatible : Must be "maxim,max77693-led".
+
+Optional properties:
+- maxim,boost-mode :
+	In boost mode the device can produce up to 1.2A of total current
+	on both outputs. The maximum current on each output is reduced
+	to 625mA then. If not enabled explicitly, boost setting defaults to
+	LEDS_BOOST_FIXED in case both current sources are used.
+	Possible values:
+		LEDS_BOOST_OFF (0) - no boost,
+		LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
+		LEDS_BOOST_FIXED (2) - fixed mode.
+- maxim,boost-mvout : Output voltage of the boost module in millivolts.
+	Valid values: 3300 - 5500, step by 25 (rounded down)
+	Default: 3300
+- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
+	if chip estimates that system voltage could drop below this level due
+	to flash power consumption.
+	Valid values: 2400 - 3400, step by 33 (rounded down)
+	Default: 2400
+
+Required properties of the LED child node:
+- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
+		device current output identifiers: 0 - FLED1, 1 - FLED2
+
+Optional properties of the LED child node:
+- label : see Documentation/devicetree/bindings/leds/common.txt
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+	Valid values: 15625 - 250000, step by 15625 (rounded down)
+	Default: 250000
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+	Valid values: 15625 - 1000000, step by 15625 (rounded down)
+	Default: 1000000 when boost mode is off and 625000 otherwise
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+	Valid values: 62500 - 1000000, step by 62500 (rounded down)
+	Default: 1000000
+
 Example:
+#include <dt-bindings/leds/common.h>
+
 	max77693@66 {
 		compatible = "maxim,max77693";
 		reg = <0x66>;
@@ -117,5 +164,19 @@ Example:
 			maxim,thermal-regulation-celsius = <75>;
 			maxim,battery-overcurrent-microamp = <3000000>;
 			maxim,charge-input-threshold-microvolt = <4300000>;
+
+		led {
+			compatible = "maxim,max77693-led";
+			maxim,boost-mode = <LEDS_BOOST_FIXED>;
+			maxim,boost-mvout = <5000>;
+			maxim,mvsys-min = <2400>;
+
+			camera_flash: flash-led {
+				label = "max77693-flash";
+				led-sources = <0>, <1>;
+				max-microamp = <500000>;
+				flash-max-microamp = <1250000>;
+				flash-timeout-us = <1000000>;
+			};
 		};
 	};
-- 
1.7.9.5


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

* [PATCH v4 05/12] leds: Add driver for AAT1290 flash LED controller
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
                   ` (3 preceding siblings ...)
  2015-03-31 13:52 ` [PATCH v4 04/12] DT: Add documentation for the mfd Maxim max77693 Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 06/12] of: Add Skyworks Solutions, Inc. vendor prefix Jacek Anaszewski
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski

This patch adds a driver for the 1.5A Step-Up Current Regulator
for Flash LEDs. The device is programmed through a Skyworks proprietary
AS2Cwire serial digital interface.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
---
 drivers/leds/Kconfig        |    8 +
 drivers/leds/Makefile       |    1 +
 drivers/leds/leds-aat1290.c |  372 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 381 insertions(+)
 create mode 100644 drivers/leds/leds-aat1290.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f9fbeb5..c3b5b027 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -42,6 +42,14 @@ config LEDS_88PM860X
 	  This option enables support for on-chip LED drivers found on Marvell
 	  Semiconductor 88PM8606 PMIC.
 
+config LEDS_AAT1290
+	tristate "LED support for the AAT1290"
+	depends on LEDS_CLASS_FLASH
+	depends on GPIOLIB
+	depends on OF
+	help
+	 This option enables support for the LEDs on the AAT1290.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 9413fdb..0b3fd0e 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
 obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
+obj-$(CONFIG_LEDS_AAT1290)		+= leds-aat1290.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
new file mode 100644
index 0000000..acfae35
--- /dev/null
+++ b/drivers/leds/leds-aat1290.c
@@ -0,0 +1,372 @@
+/*
+ *	LED Flash class driver for the AAT1290
+ *	1.5A Step-Up Current Regulator for Flash LEDs
+ *
+ *	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/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/led-class-flash.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#define AAT1290_MOVIE_MODE_CURRENT_ADDR	17
+#define AAT1290_MAX_MM_CURR_PERCENT_0	16
+#define AAT1290_MAX_MM_CURR_PERCENT_100	1
+
+#define AAT1290_FLASH_SAFETY_TIMER_ADDR	18
+
+#define AAT1290_MOVIE_MODE_CONFIG_ADDR	19
+#define AAT1290_MOVIE_MODE_OFF		1
+#define AAT1290_MOVIE_MODE_ON		3
+
+#define AAT1290_MM_CURRENT_RATIO_ADDR	20
+#define AAT1290_MM_TO_FL_1_92		1
+
+#define AAT1290_MM_TO_FL_RATIO		1000 / 1920
+#define AAT1290_MAX_MM_CURRENT(fl_max)	(fl_max * AAT1290_MM_TO_FL_RATIO)
+
+#define AAT1290_LATCH_TIME_MIN_US	500
+#define AAT1290_LATCH_TIME_MAX_US	1000
+#define AAT1290_EN_SET_TICK_TIME_US	1
+#define AAT1290_FLEN_OFF_DELAY_TIME_US	10
+#define AAT1290_FLASH_TM_NUM_LEVELS	16
+#define AAT1290_MM_CURRENT_SCALE_SIZE	15
+
+
+struct aat1290_led {
+	/* platform device data */
+	struct platform_device *pdev;
+	/* secures access to the device */
+	struct mutex lock;
+
+	/* corresponding LED Flash class device */
+	struct led_classdev_flash fled_cdev;
+
+	/* FLEN pin */
+	struct gpio_desc *gpio_fl_en;
+	/* EN|SET pin  */
+	struct gpio_desc *gpio_en_set;
+
+	/* maximum flash timeout */
+	u32 max_flash_tm;
+	/* maximum LED current in flash mode */
+	u32 max_flash_current;
+	/* device mode */
+	bool movie_mode;
+
+	/* brightness cache */
+	unsigned int torch_brightness;
+	/* assures led-triggers compatibility */
+	struct work_struct work_brightness_set;
+};
+
+static struct aat1290_led *fled_cdev_to_led(
+				struct led_classdev_flash *fled_cdev)
+{
+	return container_of(fled_cdev, struct aat1290_led, fled_cdev);
+}
+
+static void aat1290_as2cwire_write(struct aat1290_led *led, int addr, int value)
+{
+	int i;
+
+	gpiod_direction_output(led->gpio_fl_en, 0);
+	gpiod_direction_output(led->gpio_en_set, 0);
+
+	udelay(AAT1290_FLEN_OFF_DELAY_TIME_US);
+
+	/* write address */
+	for (i = 0; i < addr; ++i) {
+		udelay(AAT1290_EN_SET_TICK_TIME_US);
+		gpiod_direction_output(led->gpio_en_set, 0);
+		udelay(AAT1290_EN_SET_TICK_TIME_US);
+		gpiod_direction_output(led->gpio_en_set, 1);
+	}
+
+	usleep_range(AAT1290_LATCH_TIME_MIN_US, AAT1290_LATCH_TIME_MAX_US);
+
+	/* write data */
+	for (i = 0; i < value; ++i) {
+		udelay(AAT1290_EN_SET_TICK_TIME_US);
+		gpiod_direction_output(led->gpio_en_set, 0);
+		udelay(AAT1290_EN_SET_TICK_TIME_US);
+		gpiod_direction_output(led->gpio_en_set, 1);
+	}
+
+	usleep_range(AAT1290_LATCH_TIME_MIN_US, AAT1290_LATCH_TIME_MAX_US);
+}
+
+static void aat1290_set_flash_safety_timer(struct aat1290_led *led,
+					unsigned int micro_sec)
+{
+	struct led_classdev_flash *fled_cdev = &led->fled_cdev;
+	struct led_flash_setting *flash_tm = &fled_cdev->timeout;
+	int flash_tm_reg = AAT1290_FLASH_TM_NUM_LEVELS -
+				(micro_sec / flash_tm->step) + 1;
+
+	aat1290_as2cwire_write(led, AAT1290_FLASH_SAFETY_TIMER_ADDR,
+							flash_tm_reg);
+}
+
+static void aat1290_brightness_set(struct aat1290_led *led,
+					enum led_brightness brightness)
+{
+	mutex_lock(&led->lock);
+
+	if (brightness == 0) {
+		gpiod_direction_output(led->gpio_fl_en, 0);
+		gpiod_direction_output(led->gpio_en_set, 0);
+		led->movie_mode = false;
+		goto unlock;
+	}
+
+	if (!led->movie_mode) {
+		aat1290_as2cwire_write(led, AAT1290_MM_CURRENT_RATIO_ADDR,
+					AAT1290_MM_TO_FL_1_92);
+		led->movie_mode = true;
+	}
+
+	aat1290_as2cwire_write(led, AAT1290_MOVIE_MODE_CURRENT_ADDR,
+				AAT1290_MAX_MM_CURR_PERCENT_0 - brightness);
+	aat1290_as2cwire_write(led, AAT1290_MOVIE_MODE_CONFIG_ADDR,
+				AAT1290_MOVIE_MODE_ON);
+unlock:
+	mutex_unlock(&led->lock);
+}
+
+/* LED subsystem callbacks */
+
+static void aat1290_brightness_set_work(struct work_struct *work)
+{
+	struct aat1290_led *led =
+		container_of(work, struct aat1290_led, work_brightness_set);
+
+	aat1290_brightness_set(led, led->torch_brightness);
+}
+
+static void aat1290_led_brightness_set(struct led_classdev *led_cdev,
+					enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
+
+	led->torch_brightness = brightness;
+	schedule_work(&led->work_brightness_set);
+}
+
+static int aat1290_led_brightness_set_sync(struct led_classdev *led_cdev,
+					enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
+
+	aat1290_brightness_set(led, brightness);
+
+	return 0;
+}
+
+static int aat1290_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
+					 bool state)
+
+{
+	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
+	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+	struct led_flash_setting *timeout = &fled_cdev->timeout;
+
+	mutex_lock(&led->lock);
+
+	if (state == 0) {
+		gpiod_direction_output(led->gpio_fl_en, 0);
+		gpiod_direction_output(led->gpio_en_set, 0);
+		goto done;
+	}
+
+	aat1290_set_flash_safety_timer(led, timeout->val);
+
+	gpiod_direction_output(led->gpio_fl_en, 1);
+
+done:
+	/*
+	 * To reenter movie mode after a flash event the part must be cycled
+	 * off and back on to reset the movie mode and reprogrammed via the
+	 * AS2Cwire. Therefore the brightness and movie_mode properties needs
+	 * to be updated here to reflect the actual state.
+	 */
+	led_cdev->brightness = 0;
+	led->movie_mode = false;
+
+	mutex_unlock(&led->lock);
+
+	return 0;
+}
+
+static int aat1290_led_flash_timeout_set(struct led_classdev_flash *fled_cdev,
+						u32 timeout)
+{
+	/*
+	 * Don't do anything - flash timeout is cached in the led-class-flash
+	 * core and will be applied in the strobe_set op, as writing the
+	 * safety timer register spuriously turns the torch mode on.
+	 */
+
+	return 0;
+}
+
+static int aat1290_led_parse_dt(struct aat1290_led *led)
+{
+	struct led_classdev *led_cdev = &led->fled_cdev.led_cdev;
+	struct device *dev = &led->pdev->dev;
+	struct device_node *child_node;
+	int ret = 0;
+
+	led->gpio_fl_en = devm_gpiod_get(dev, "flen");
+	if (IS_ERR(led->gpio_fl_en)) {
+		ret = PTR_ERR(led->gpio_fl_en);
+		dev_err(dev, "Unable to claim gpio \"flen\".\n");
+		return ret;
+	}
+
+	led->gpio_en_set = devm_gpiod_get(dev, "enset");
+	if (IS_ERR(led->gpio_en_set)) {
+		ret = PTR_ERR(led->gpio_en_set);
+		dev_err(dev, "Unable to claim gpio \"enset\".\n");
+		return ret;
+	}
+
+	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");
+		return -EINVAL;
+	}
+
+	led_cdev->name = of_get_property(child_node, "label", NULL) ? :
+						child_node->name;
+
+	ret = of_property_read_u32(child_node, "flash-max-microamp",
+				&led->max_flash_current);
+	if (ret < 0) {
+		dev_err(dev,
+			"Error reading \"flash-max-microamp\" DT property\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(child_node, "flash-timeout-us",
+				&led->max_flash_tm);
+	if (ret < 0) {
+		dev_err(dev,
+			"Error reading \"flash-timeout-us\" DT property\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static void aat1290_init_flash_timeout(struct aat1290_led *led)
+{
+	struct led_classdev_flash *fled_cdev = &led->fled_cdev;
+	struct led_flash_setting *setting;
+
+	/* Init flash timeout setting */
+	setting = &fled_cdev->timeout;
+	setting->min = led->max_flash_tm / AAT1290_FLASH_TM_NUM_LEVELS;
+	setting->max = setting->min * AAT1290_FLASH_TM_NUM_LEVELS;
+	setting->step = setting->min;
+	setting->val = setting->max;
+}
+
+static const struct led_flash_ops flash_ops = {
+	.strobe_set = aat1290_led_flash_strobe_set,
+	.timeout_set = aat1290_led_flash_timeout_set,
+};
+
+static int aat1290_led_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct aat1290_led *led;
+	struct led_classdev *led_cdev;
+	struct led_classdev_flash *fled_cdev;
+	int ret;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pdev = pdev;
+	platform_set_drvdata(pdev, led);
+
+	fled_cdev = &led->fled_cdev;
+	fled_cdev->ops = &flash_ops;
+	led_cdev = &fled_cdev->led_cdev;
+
+	ret = aat1290_led_parse_dt(led);
+	if (ret < 0)
+		return ret;
+
+	mutex_init(&led->lock);
+
+	/* Initialize LED Flash class device */
+	led_cdev->brightness_set = aat1290_led_brightness_set;
+	led_cdev->brightness_set_sync = aat1290_led_brightness_set_sync;
+	led_cdev->max_brightness = AAT1290_MM_CURRENT_SCALE_SIZE;
+	led_cdev->flags |= LED_DEV_CAP_FLASH;
+	INIT_WORK(&led->work_brightness_set, aat1290_brightness_set_work);
+
+	aat1290_init_flash_timeout(led);
+
+	/* Register LED Flash class device */
+	ret = led_classdev_flash_register(&pdev->dev, fled_cdev);
+	if (ret < 0)
+		goto err_flash_register;
+
+	return 0;
+
+err_flash_register:
+	mutex_destroy(&led->lock);
+
+	return ret;
+}
+
+static int aat1290_led_remove(struct platform_device *pdev)
+{
+	struct aat1290_led *led = platform_get_drvdata(pdev);
+
+	led_classdev_flash_unregister(&led->fled_cdev);
+	cancel_work_sync(&led->work_brightness_set);
+
+	mutex_destroy(&led->lock);
+
+	return 0;
+}
+
+static const struct of_device_id aat1290_led_dt_match[] = {
+	{ .compatible = "skyworks,aat1290" },
+	{},
+};
+
+static struct platform_driver aat1290_led_driver = {
+	.probe		= aat1290_led_probe,
+	.remove		= aat1290_led_remove,
+	.driver		= {
+		.name	= "aat1290",
+		.owner	= THIS_MODULE,
+		.of_match_table = aat1290_led_dt_match,
+	},
+};
+
+module_platform_driver(aat1290_led_driver);
+
+MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
+MODULE_DESCRIPTION("Skyworks Current Regulator for Flash LEDs");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


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

* [PATCH v4 06/12] of: Add Skyworks Solutions, Inc. vendor prefix
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
                   ` (4 preceding siblings ...)
  2015-03-31 13:52 ` [PATCH v4 05/12] leds: Add driver for AAT1290 flash LED controller Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-04-08 11:59   ` Sakari Ailus
  2015-03-31 13:52 ` [PATCH v4 07/12] DT: Add documentation for the Skyworks AAT1290 Jacek Anaszewski
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski, devicetree

Use "skyworks" as the vendor prefix for the Skyworks Solutions, Inc.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 42b3dab..4cd18bb 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -163,6 +163,7 @@ ricoh	Ricoh Co. Ltd.
 rockchip	Fuzhou Rockchip Electronics Co., Ltd
 samsung	Samsung Semiconductor
 sandisk	Sandisk Corporation
+skyworks	Skyworks Solutions, Inc.
 sbs	Smart Battery System
 schindler	Schindler
 seagate	Seagate Technology PLC
-- 
1.7.9.5


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

* [PATCH v4 07/12] DT: Add documentation for the Skyworks AAT1290
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
                   ` (5 preceding siblings ...)
  2015-03-31 13:52 ` [PATCH v4 06/12] of: Add Skyworks Solutions, Inc. vendor prefix Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 08/12] media: Add registration helpers for V4L2 flash sub-devices Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 09/12] Documentation: leds: Add description of v4l2-flash sub-device Jacek Anaszewski
  8 siblings, 0 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski, devicetree

This patch adds device tree binding documentation for
1.5A Step-Up Current Regulator for Flash LEDs.

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: devicetree@vger.kernel.org
---
 .../devicetree/bindings/leds/leds-aat1290.txt      |   40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-aat1290.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-aat1290.txt b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
new file mode 100644
index 0000000..6893eb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
@@ -0,0 +1,40 @@
+* Skyworks Solutions, Inc. AAT1290 Current Regulator for Flash LEDs
+
+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.
+
+Required properties:
+
+- compatible : Must be "skyworks,aat1290".
+- 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.
+
+A discrete LED element connected to the device must be represented by a child
+node - see Documentation/devicetree/bindings/leds/common.txt.
+
+Required properties of the LED child node:
+- flash-max-microamp : Maximum intensity in microamperes of the flash LED -
+		       it can be calculated using following formula:
+		       I = 1A * 162kohm / Rset.
+- flash-timeout-us : Maximum flash timeout in microseconds -
+		     it can be calculated using following formula:
+		     T = 8.82 * 10^9 * Ct.
+
+Optional properties of the LED child node:
+- label : see Documentation/devicetree/bindings/leds/common.txt
+
+Example (by Ct = 220nF, Rset = 160kohm and exynos4412-trats2 board with
+a switch that allows for routing strobe signal either from host or from ISP):
+
+aat1290 {
+	compatible = "skyworks,aat1290";
+	flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
+	enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
+
+	camera_flash: flash-led {
+		label = "aat1290-flash";
+		flash-max-microamp = <1012500>;
+		flash-timeout-us = <1940000>;
+	};
+};
-- 
1.7.9.5


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

* [PATCH v4 08/12] media: Add registration helpers for V4L2 flash sub-devices
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
                   ` (6 preceding siblings ...)
  2015-03-31 13:52 ` [PATCH v4 07/12] DT: Add documentation for the Skyworks AAT1290 Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  2015-03-31 13:52 ` [PATCH v4 09/12] Documentation: leds: Add description of v4l2-flash sub-device Jacek Anaszewski
  8 siblings, 0 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski, Hans Verkuil

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.c |  619 ++++++++++++++++++++++++++++++++++
 include/media/v4l2-flash.h           |  145 ++++++++
 4 files changed, 777 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
 create mode 100644 include/media/v4l2-flash.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index ba7e21a..f034f1a 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 "Enable support for Flash sub-devices"
+	depends on VIDEO_V4L2_SUBDEV_API
+	depends on LEDS_CLASS_FLASH
+	---help---
+	  Say Y here to enable support for Flash sub-devices, which allow
+	  to control LED class devices with use of V4L2 Flash controls.
+
+	  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..44e858c 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.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.c b/drivers/media/v4l2-core/v4l2-flash.c
new file mode 100644
index 0000000..bed2036
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,619 @@
+/*
+ * 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.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)
+
+static enum led_brightness __intensity_to_led_brightness(
+					struct v4l2_ctrl *ctrl,
+					s32 intensity)
+{
+	s64 intensity64 = intensity - ctrl->minimum;
+
+	do_div(intensity64, 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->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
+		++intensity64;
+
+	return intensity64;
+}
+
+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 */
+		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 */
+		return led_set_flash_timeout(fled_cdev, c->val);
+	case V4L2_CID_FLASH_INTENSITY:
+		/* no conversion is needed */
+		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,
+};
+
+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;
+
+		/* 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;
+
+	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;
+	}
+}
+
+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;
+
+	/* 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) &&
+	    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 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 = led_cdev->dev;
+	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.h b/include/media/v4l2-flash.h
new file mode 100644
index 0000000..7272eff
--- /dev/null
+++ b/include/media/v4l2-flash.h
@@ -0,0 +1,145 @@
+/*
+ * 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;
+
+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,
+};
+
+/*
+ * 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_ctrl_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[STROBE_SOURCE + 1];
+};
+
+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
+ * @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 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
+#define v4l2_flash_init(fled_cdev, ops, config) (NULL)
+#define v4l2_flash_release(v4l2_flash)
+#endif /* CONFIG_V4L2_FLASH_LED_CLASS */
+
+#endif /* _V4L2_FLASH_H */
-- 
1.7.9.5


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

* [PATCH v4 09/12] Documentation: leds: Add description of v4l2-flash sub-device
  2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
                   ` (7 preceding siblings ...)
  2015-03-31 13:52 ` [PATCH v4 08/12] media: Add registration helpers for V4L2 flash sub-devices Jacek Anaszewski
@ 2015-03-31 13:52 ` Jacek Anaszewski
  8 siblings, 0 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 13:52 UTC (permalink / raw)
  To: linux-leds, linux-media
  Cc: kyungmin.park, pavel, cooloney, rpurdie, sakari.ailus,
	s.nawrocki, Jacek Anaszewski

This patch extends LED Flash class documention by
the description of interactions with v4l2-flash sub-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>
---
 Documentation/leds/leds-class-flash.txt |   47 +++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/leds/leds-class-flash.txt b/Documentation/leds/leds-class-flash.txt
index 19bb673..ad80eeb 100644
--- a/Documentation/leds/leds-class-flash.txt
+++ b/Documentation/leds/leds-class-flash.txt
@@ -20,3 +20,50 @@ Following sysfs attributes are exposed for controlling flash LED devices:
 	- max_flash_timeout
 	- flash_strobe
 	- flash_fault
+
+
+V4L2 Flash wrapper for flash LEDs
+=================================
+
+A LED subsystem driver can be controlled also from the level of VideoForLinux2
+subsystem. In order to enable this CONFIG_V4L2_FLASH_LED_CLASS symbol has to
+be defined in the kernel config.
+
+The driver must call the v4l2_flash_init function to get registered in the
+V4L2 subsystem. The function takes three arguments:
+- fled_cdev : the LED Flash class device to wrap
+- ops : V4L2 specific ops
+	* external_strobe_set - defines the source of the flash LED strobe -
+		V4L2_CID_FLASH_STROBE control or external source, typically
+		a sensor, which makes it possible to synchronise the flash
+		strobe start with exposure start,
+	* intensity_to_led_brightness and led_brightness_to_intensity - perform
+		enum led_brightness <-> V4L2 intensity conversion in a device
+		specific manner - they can be used for devices with non-linear
+		LED current scale.
+- config : configuration for V4L2 Flash sub-device
+	* dev_name - the name of the media entity, unique in the system,
+	* flash_faults - bitmask of flash faults that the LED Flash class
+		device can report; corresponding LED_FAULT* bit definitions are
+		available in <linux/led-class-flash.h>,
+	* intensity - constraints for the LED in the TORCH or INDICATOR mode,
+		in microamperes,
+	* has_external_strobe - determines whether the flash strobe source
+		can be switched to external,
+	* indicator_led - signifies that a led is of indicator type, which
+		implies that it can have only two V4L2 controls:
+		V4L2_CID_FLASH_INDICATOR_INTENSITY and V4L2_CID_FLASH_FAULT.
+
+On remove the v4l2_flash_release function has to be called, which takes one
+argument - struct v4l2_flash pointer returned previously by v4l2_flash_init.
+
+Please refer to drivers/leds/leds-max77693.c for an exemplary usage of the
+v4l2-flash API.
+
+Once the V4L2 sub-device is registered by the driver which created the Media
+controller device, the sub-device node acts just as a node of a native V4L2
+flash API device would. The calls are simply routed to the LED flash API.
+
+Opening the V4L2 Flash sub-device makes the LED subsystem sysfs interface
+unavailable. The interface is re-enabled after the V4L2 Flash sub-device
+is closed.
-- 
1.7.9.5


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

* Re: [PATCH v4 04/12] DT: Add documentation for the mfd Maxim max77693
  2015-03-31 13:52 ` [PATCH v4 04/12] DT: Add documentation for the mfd Maxim max77693 Jacek Anaszewski
@ 2015-03-31 14:34   ` Jacek Anaszewski
  0 siblings, 0 replies; 29+ messages in thread
From: Jacek Anaszewski @ 2015-03-31 14:34 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	sakari.ailus, s.nawrocki, Andrzej Hajda, Lee Jones, Chanwoo Choi,
	devicetree

I've just pushed PATCH v5 - it adds more precise description
of current settings constraints.

On 03/31/2015 03:52 PM, Jacek Anaszewski wrote:
> This patch adds device tree binding documentation for
> the flash cell of the Maxim max77693 multifunctional device.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Cc: devicetree@vger.kernel.org
> ---
>   Documentation/devicetree/bindings/mfd/max77693.txt |   61 ++++++++++++++++++++
>   1 file changed, 61 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
> index 38e6440..deb832f 100644
> --- a/Documentation/devicetree/bindings/mfd/max77693.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
> @@ -76,7 +76,54 @@ Optional properties:
>       Valid values: 4300000, 4700000, 4800000, 4900000
>       Default: 4300000
>
> +- led : the LED submodule device node
> +
> +There are two LED outputs available - FLED1 and FLED2. Each of them can
> +control a separate LED or they can be connected together to double
> +the maximum current for a single connected LED. One LED is represented
> +by one child node.
> +
> +Required properties:
> +- compatible : Must be "maxim,max77693-led".
> +
> +Optional properties:
> +- maxim,boost-mode :
> +	In boost mode the device can produce up to 1.2A of total current
> +	on both outputs. The maximum current on each output is reduced
> +	to 625mA then. If not enabled explicitly, boost setting defaults to
> +	LEDS_BOOST_FIXED in case both current sources are used.
> +	Possible values:
> +		LEDS_BOOST_OFF (0) - no boost,
> +		LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
> +		LEDS_BOOST_FIXED (2) - fixed mode.
> +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
> +	Valid values: 3300 - 5500, step by 25 (rounded down)
> +	Default: 3300
> +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
> +	if chip estimates that system voltage could drop below this level due
> +	to flash power consumption.
> +	Valid values: 2400 - 3400, step by 33 (rounded down)
> +	Default: 2400
> +
> +Required properties of the LED child node:
> +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
> +		device current output identifiers: 0 - FLED1, 1 - FLED2
> +
> +Optional properties of the LED child node:
> +- label : see Documentation/devicetree/bindings/leds/common.txt
> +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
> +	Valid values: 15625 - 250000, step by 15625 (rounded down)
> +	Default: 250000
> +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
> +	Valid values: 15625 - 1000000, step by 15625 (rounded down)
> +	Default: 1000000 when boost mode is off and 625000 otherwise
> +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
> +	Valid values: 62500 - 1000000, step by 62500 (rounded down)
> +	Default: 1000000
> +
>   Example:
> +#include <dt-bindings/leds/common.h>
> +
>   	max77693@66 {
>   		compatible = "maxim,max77693";
>   		reg = <0x66>;
> @@ -117,5 +164,19 @@ Example:
>   			maxim,thermal-regulation-celsius = <75>;
>   			maxim,battery-overcurrent-microamp = <3000000>;
>   			maxim,charge-input-threshold-microvolt = <4300000>;
> +
> +		led {
> +			compatible = "maxim,max77693-led";
> +			maxim,boost-mode = <LEDS_BOOST_FIXED>;
> +			maxim,boost-mvout = <5000>;
> +			maxim,mvsys-min = <2400>;
> +
> +			camera_flash: flash-led {
> +				label = "max77693-flash";
> +				led-sources = <0>, <1>;
> +				max-microamp = <500000>;
> +				flash-max-microamp = <1250000>;
> +				flash-timeout-us = <1000000>;
> +			};
>   		};
>   	};
>


-- 
Best Regards,
Jacek Anaszewski

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
@ 2015-04-02 14:41   ` Pavel Machek
  2015-04-03 12:09   ` Sakari Ailus
  2015-04-08 10:03   ` Sylwester Nawrocki
  2 siblings, 0 replies; 29+ messages in thread
From: Pavel Machek @ 2015-04-02 14:41 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, kyungmin.park, cooloney, rpurdie,
	sakari.ailus, s.nawrocki, Sakari Ailus, devicetree

On Tue 2015-03-31 15:52:37, Jacek Anaszewski wrote:
> Description of flash LEDs related properties was not precise regarding
> the state of corresponding settings in case a property is missing.
> Add relevant statements.
> Removed is also the requirement making the flash-max-microamp
> property obligatory for flash LEDs. It was inconsistent as the property
> is defined as optional. Devices which require the property will have
> to assert this in their DT bindings.
> 
> 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>

Acked-by: Pavel Machek <pavel@ucw.cz>

> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 747c538..21a25e4 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>       "ide-disk" - LED indicates disk activity
>       "timer" - LED flashes at a fixed, configurable rate
>  
> -- max-microamp : maximum intensity in microamperes of the LED
> -		 (torch LED for flash devices)
> -- flash-max-microamp : maximum intensity in microamperes of the
> -                       flash LED; it is mandatory if the LED should
> -		       support the flash mode
> -- flash-timeout-us : timeout in microseconds after which the flash
> -                     LED is turned off
> +- max-microamp : Maximum intensity in microamperes of the LED
> +		 (torch LED for flash devices). If omitted this will default
> +		 to the maximum current allowed by the device.
> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
> +		       If omitted this will default to the maximum
> +		       current allowed by the device.
> +- flash-timeout-us : Timeout in microseconds after which the flash
> +                     LED is turned off. If omitted this will default to the
> +		     maximum timeout allowed by the device.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
  2015-04-02 14:41   ` Pavel Machek
@ 2015-04-03 12:09   ` Sakari Ailus
  2015-04-03 12:56     ` Jacek Anaszewski
  2015-04-08  8:54     ` Jacek Anaszewski
  2015-04-08 10:03   ` Sylwester Nawrocki
  2 siblings, 2 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-04-03 12:09 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, Sakari Ailus, devicetree

Hi Jacek,

On Tue, Mar 31, 2015 at 03:52:37PM +0200, Jacek Anaszewski wrote:
> Description of flash LEDs related properties was not precise regarding
> the state of corresponding settings in case a property is missing.
> Add relevant statements.
> Removed is also the requirement making the flash-max-microamp
> property obligatory for flash LEDs. It was inconsistent as the property
> is defined as optional. Devices which require the property will have
> to assert this in their DT bindings.
> 
> 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: Pavel Machek <pavel@ucw.cz>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 747c538..21a25e4 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>       "ide-disk" - LED indicates disk activity
>       "timer" - LED flashes at a fixed, configurable rate
>  
> -- max-microamp : maximum intensity in microamperes of the LED
> -		 (torch LED for flash devices)
> -- flash-max-microamp : maximum intensity in microamperes of the
> -                       flash LED; it is mandatory if the LED should
> -		       support the flash mode
> -- flash-timeout-us : timeout in microseconds after which the flash
> -                     LED is turned off
> +- max-microamp : Maximum intensity in microamperes of the LED
> +		 (torch LED for flash devices). If omitted this will default
> +		 to the maximum current allowed by the device.
> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
> +		       If omitted this will default to the maximum
> +		       current allowed by the device.
> +- flash-timeout-us : Timeout in microseconds after which the flash
> +                     LED is turned off. If omitted this will default to the
> +		     maximum timeout allowed by the device.
>  
>  
>  Examples:

Pavel pointed out that the brightness between maximum current and the
maximum *allowed* another current might not be noticeable, leading a
potential spelling error to cause the LED being run at too high current.

The three drivers I've looked also require these properties, which I think
is in the line with the above.

How about either dropping the patch, or changing maximum to minimum and
will to should? The drivers could also behave this way instead of requiring
the properties, but I don't think there's anything wrong with requiring the
properties either.

I think this is worth considering now as we can't change this later without
breaking something.

-- 
Kind regards,

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

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-03 12:09   ` Sakari Ailus
@ 2015-04-03 12:56     ` Jacek Anaszewski
  2015-04-03 20:37       ` Pavel Machek
  2015-04-08  8:54     ` Jacek Anaszewski
  1 sibling, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-04-03 12:56 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, Sakari Ailus, devicetree

Hi Sakari,

On 04/03/2015 02:09 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Tue, Mar 31, 2015 at 03:52:37PM +0200, Jacek Anaszewski wrote:
>> Description of flash LEDs related properties was not precise regarding
>> the state of corresponding settings in case a property is missing.
>> Add relevant statements.
>> Removed is also the requirement making the flash-max-microamp
>> property obligatory for flash LEDs. It was inconsistent as the property
>> is defined as optional. Devices which require the property will have
>> to assert this in their DT bindings.
>>
>> 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: Pavel Machek <pavel@ucw.cz>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 747c538..21a25e4 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>>        "ide-disk" - LED indicates disk activity
>>        "timer" - LED flashes at a fixed, configurable rate
>>
>> -- max-microamp : maximum intensity in microamperes of the LED
>> -		 (torch LED for flash devices)
>> -- flash-max-microamp : maximum intensity in microamperes of the
>> -                       flash LED; it is mandatory if the LED should
>> -		       support the flash mode
>> -- flash-timeout-us : timeout in microseconds after which the flash
>> -                     LED is turned off
>> +- max-microamp : Maximum intensity in microamperes of the LED
>> +		 (torch LED for flash devices). If omitted this will default
>> +		 to the maximum current allowed by the device.
>> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
>> +		       If omitted this will default to the maximum
>> +		       current allowed by the device.
>> +- flash-timeout-us : Timeout in microseconds after which the flash
>> +                     LED is turned off. If omitted this will default to the
>> +		     maximum timeout allowed by the device.
>>
>>
>>   Examples:
>
> Pavel pointed out that the brightness between maximum current and the
> maximum *allowed* another current might not be noticeable,leading a
> potential spelling error to cause the LED being run at too high current.

Where did he point this out? Do you think about the current version
of the leds/common.txt documentation or there was some other message,
that I don't see?

Besides, I can't understand your point. Could you express it in other
words, please?

> The three drivers I've looked also require these properties, which I think
> is in the line with the above.

These properties were introduced two months ago and there is no merged
driver which require them. It means that the drivers that are currently
in the mainline assume that they can set the maximum current (please
note that max-microamp property refers to non-flash LEDs too).
The modifications I proposed just validate the current status.

> How about either dropping the patch, or changing maximum to minimum and
> will to should? The drivers could also behave this way instead of requiring
> the properties, but I don't think there's anything wrong with requiring the
> properties either.
>
> I think this is worth considering now as we can't change this later without
> breaking something.
>

-- 
Best Regards,
Jacek Anaszewski

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-03 12:56     ` Jacek Anaszewski
@ 2015-04-03 20:37       ` Pavel Machek
  2015-04-08  1:20         ` Bryan Wu
  0 siblings, 1 reply; 29+ messages in thread
From: Pavel Machek @ 2015-04-03 20:37 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Sakari Ailus, linux-leds, linux-media, kyungmin.park, cooloney,
	rpurdie, s.nawrocki, Sakari Ailus, devicetree

Hi!

> >>+- flash-timeout-us : Timeout in microseconds after which the flash
> >>+                     LED is turned off. If omitted this will default to the
> >>+		     maximum timeout allowed by the device.
> >>
> >>
> >>  Examples:
> >
> >Pavel pointed out that the brightness between maximum current and the
> >maximum *allowed* another current might not be noticeable,leading a
> >potential spelling error to cause the LED being run at too high current.
> 
> Where did he point this out? Do you think about the current version
> of the leds/common.txt documentation or there was some other message,
> that I don't see?

Date: Thu, 2 Apr 2015 22:30:44 +0200
From: Pavel Machek <pavel@ucw.cz>
To: Sakari Ailus <sakari.ailus@iki.fi>
Subject: Re: [PATCHv3] media: i2c/adp1653: devicetree support for adp1653

> Besides, I can't understand your point. Could you express it in other
> words, please?

Typo in device tree would cause hardware damage. But idea. Make the
properties mandatory.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-03 20:37       ` Pavel Machek
@ 2015-04-08  1:20         ` Bryan Wu
  2015-04-08  7:48           ` Sakari Ailus
  2015-04-08  9:13           ` Pavel Machek
  0 siblings, 2 replies; 29+ messages in thread
From: Bryan Wu @ 2015-04-08  1:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jacek Anaszewski, Sakari Ailus, Linux LED Subsystem, linux-media,
	Kyungmin Park, rpurdie, Sylwester Nawrocki, Sakari Ailus,
	devicetree

On Fri, Apr 3, 2015 at 1:37 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> >>+- flash-timeout-us : Timeout in microseconds after which the flash
>> >>+                     LED is turned off. If omitted this will default to the
>> >>+                maximum timeout allowed by the device.
>> >>
>> >>
>> >>  Examples:
>> >
>> >Pavel pointed out that the brightness between maximum current and the
>> >maximum *allowed* another current might not be noticeable,leading a
>> >potential spelling error to cause the LED being run at too high current.
>>
>> Where did he point this out? Do you think about the current version
>> of the leds/common.txt documentation or there was some other message,
>> that I don't see?
>
> Date: Thu, 2 Apr 2015 22:30:44 +0200
> From: Pavel Machek <pavel@ucw.cz>
> To: Sakari Ailus <sakari.ailus@iki.fi>
> Subject: Re: [PATCHv3] media: i2c/adp1653: devicetree support for adp1653
>
>> Besides, I can't understand your point. Could you express it in other
>> words, please?
>
> Typo in device tree would cause hardware damage. But idea. Make the
> properties mandatory.
>                                                                 Pavel

I don't quite follow there. I think Pavel acked this patch right? So
what's left to hold here?

Thanks,
-Bryan

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

* Re: [PATCH v4 02/12] leds: unify the location of led-trigger API
  2015-03-31 13:52 ` [PATCH v4 02/12] leds: unify the location of led-trigger API Jacek Anaszewski
@ 2015-04-08  1:25   ` Bryan Wu
  0 siblings, 0 replies; 29+ messages in thread
From: Bryan Wu @ 2015-04-08  1:25 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Linux LED Subsystem, linux-media, Kyungmin Park, Pavel Machek,
	rpurdie, Sakari Ailus, Sylwester Nawrocki

On Tue, Mar 31, 2015 at 6:52 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> Part of led-trigger API was in the private drivers/leds/leds.h header.
> Move it to the include/linux/leds.h header to unify the API location
> and announce it as public. It has been already exported from
> led-triggers.c with EXPORT_SYMBOL_GPL macro.
>

Applied, thanks.

-Bryan

> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> ---
>  drivers/leds/leds.h  |   24 ------------------------
>  include/linux/leds.h |   24 ++++++++++++++++++++++++
>  2 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index 79efe57..bc89d7a 100644
> --- a/drivers/leds/leds.h
> +++ b/drivers/leds/leds.h
> @@ -13,7 +13,6 @@
>  #ifndef __LEDS_H_INCLUDED
>  #define __LEDS_H_INCLUDED
>
> -#include <linux/device.h>
>  #include <linux/rwsem.h>
>  #include <linux/leds.h>
>
> @@ -50,27 +49,4 @@ void led_stop_software_blink(struct led_classdev *led_cdev);
>  extern struct rw_semaphore leds_list_lock;
>  extern struct list_head leds_list;
>
> -#ifdef CONFIG_LEDS_TRIGGERS
> -void led_trigger_set_default(struct led_classdev *led_cdev);
> -void led_trigger_set(struct led_classdev *led_cdev,
> -                       struct led_trigger *trigger);
> -void led_trigger_remove(struct led_classdev *led_cdev);
> -
> -static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
> -{
> -       return led_cdev->trigger_data;
> -}
> -
> -#else
> -#define led_trigger_set_default(x) do {} while (0)
> -#define led_trigger_set(x, y) do {} while (0)
> -#define led_trigger_remove(x) do {} while (0)
> -#define led_get_trigger_data(x) (NULL)
> -#endif
> -
> -ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
> -                       const char *buf, size_t count);
> -ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr,
> -                       char *buf);
> -
>  #endif /* __LEDS_H_INCLUDED */
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 9a2b000..0579708 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -12,6 +12,7 @@
>  #ifndef __LINUX_LEDS_H_INCLUDED
>  #define __LINUX_LEDS_H_INCLUDED
>
> +#include <linux/device.h>
>  #include <linux/list.h>
>  #include <linux/mutex.h>
>  #include <linux/rwsem.h>
> @@ -222,6 +223,29 @@ struct led_trigger {
>         struct list_head  next_trig;
>  };
>
> +#ifdef CONFIG_LEDS_TRIGGERS
> +void led_trigger_set_default(struct led_classdev *led_cdev);
> +void led_trigger_set(struct led_classdev *led_cdev,
> +                       struct led_trigger *trigger);
> +void led_trigger_remove(struct led_classdev *led_cdev);
> +
> +static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
> +{
> +       return led_cdev->trigger_data;
> +}
> +
> +#else
> +#define led_trigger_set_default(x) do {} while (0)
> +#define led_trigger_set(x, y) do {} while (0)
> +#define led_trigger_remove(x) do {} while (0)
> +#define led_get_trigger_data(x) (NULL)
> +#endif
> +
> +ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
> +                       const char *buf, size_t count);
> +ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr,
> +                       char *buf);
> +
>  /* Registration functions for complex triggers */
>  extern int led_trigger_register(struct led_trigger *trigger);
>  extern void led_trigger_unregister(struct led_trigger *trigger);
> --
> 1.7.9.5
>

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08  1:20         ` Bryan Wu
@ 2015-04-08  7:48           ` Sakari Ailus
  2015-04-08  9:13           ` Pavel Machek
  1 sibling, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-04-08  7:48 UTC (permalink / raw)
  To: Bryan Wu, Pavel Machek
  Cc: Jacek Anaszewski, Sakari Ailus, Linux LED Subsystem, linux-media,
	Kyungmin Park, rpurdie, Sylwester Nawrocki, devicetree

Hi Bryan,

Bryan Wu wrote:
> On Fri, Apr 3, 2015 at 1:37 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> Hi!
>>
>>>>> +- flash-timeout-us : Timeout in microseconds after which the flash
>>>>> +                     LED is turned off. If omitted this will default to the
>>>>> +                maximum timeout allowed by the device.
>>>>>
>>>>>
>>>>>  Examples:
>>>>
>>>> Pavel pointed out that the brightness between maximum current and the
>>>> maximum *allowed* another current might not be noticeable,leading a
>>>> potential spelling error to cause the LED being run at too high current.
>>>
>>> Where did he point this out? Do you think about the current version
>>> of the leds/common.txt documentation or there was some other message,
>>> that I don't see?
>>
>> Date: Thu, 2 Apr 2015 22:30:44 +0200
>> From: Pavel Machek <pavel@ucw.cz>
>> To: Sakari Ailus <sakari.ailus@iki.fi>
>> Subject: Re: [PATCHv3] media: i2c/adp1653: devicetree support for adp1653
>>
>>> Besides, I can't understand your point. Could you express it in other
>>> words, please?
>>
>> Typo in device tree would cause hardware damage. But idea. Make the
>> properties mandatory.
>>                                                                 Pavel
> 
> I don't quite follow there. I think Pavel acked this patch right? So
> what's left to hold here?

LED flash controllers are capable of providing more current than the
LEDs attached to them can withstand without hardware damage. This is the
reason the maximum current limits lower than the LED controller maximums
are there.

Pavel, quite rightly so in my opinion, is suggesting these properties
are made mandatory. A typo in the DT source could cause the controller
maximum current used instead of LED maximum which is often lower. That
kind of a problem would easily go unnoticed since there isn't
necessarily any perceivable change in the functionality of the board.

You'd only notice later on, when the LEDs stop working.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-03 12:09   ` Sakari Ailus
  2015-04-03 12:56     ` Jacek Anaszewski
@ 2015-04-08  8:54     ` Jacek Anaszewski
  2015-04-08  9:11       ` Sakari Ailus
  1 sibling, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-04-08  8:54 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, Sakari Ailus, devicetree

Hi Sakari,

On 04/03/2015 02:09 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Tue, Mar 31, 2015 at 03:52:37PM +0200, Jacek Anaszewski wrote:
>> Description of flash LEDs related properties was not precise regarding
>> the state of corresponding settings in case a property is missing.
>> Add relevant statements.
>> Removed is also the requirement making the flash-max-microamp
>> property obligatory for flash LEDs. It was inconsistent as the property
>> is defined as optional. Devices which require the property will have
>> to assert this in their DT bindings.
>>
>> 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: Pavel Machek <pavel@ucw.cz>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 747c538..21a25e4 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>>        "ide-disk" - LED indicates disk activity
>>        "timer" - LED flashes at a fixed, configurable rate
>>
>> -- max-microamp : maximum intensity in microamperes of the LED
>> -		 (torch LED for flash devices)
>> -- flash-max-microamp : maximum intensity in microamperes of the
>> -                       flash LED; it is mandatory if the LED should
>> -		       support the flash mode
>> -- flash-timeout-us : timeout in microseconds after which the flash
>> -                     LED is turned off
>> +- max-microamp : Maximum intensity in microamperes of the LED
>> +		 (torch LED for flash devices). If omitted this will default
>> +		 to the maximum current allowed by the device.
>> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
>> +		       If omitted this will default to the maximum
>> +		       current allowed by the device.
>> +- flash-timeout-us : Timeout in microseconds after which the flash
>> +                     LED is turned off. If omitted this will default to the
>> +		     maximum timeout allowed by the device.
>>
>>
>>   Examples:
>
> Pavel pointed out that the brightness between maximum current and the
> maximum *allowed* another current might not be noticeable, leading a
> potential spelling error to cause the LED being run at too high current.

I think that a board designed so that it can be damaged because of
software bugs should be considered not eligible for commercial use.
Any self-esteeming manufacturer will not connect a LED to the output
that can produce the current greater than the LED's absolute maximum
current.

The DT properties could be useful for devices like aat1290 device I was
writing a driver for, which has the maximum current and timeout values
depending on corresponding capacitor and resistor values respectively.
Such devices should make the properties required in their bindings.

> The three drivers I've looked also require these properties, which I think
> is in the line with the above.
>
> How about either dropping the patch, or changing maximum to minimum and
> will to should? The drivers could also behave this way instead of requiring
> the properties, but I don't think there's anything wrong with requiring the
> properties either.

As I mentioned in the previous message in this subject, the max-microamp
property refers also to non-flash LEDs. Since existing LED class devices
does not require them, then it should be left optional and default to
max. It would however be inconsistent with flash LEDs related
properties.


> I think this is worth considering now as we can't change this later without
> breaking something.
>

-- 
Best Regards,
Jacek Anaszewski

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08  8:54     ` Jacek Anaszewski
@ 2015-04-08  9:11       ` Sakari Ailus
  2015-04-08  9:17         ` Pavel Machek
  2015-04-08 10:23         ` Jacek Anaszewski
  0 siblings, 2 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-04-08  9:11 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, Sakari Ailus, devicetree

Hi Jacek,

On Wed, Apr 08, 2015 at 10:54:52AM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 04/03/2015 02:09 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Tue, Mar 31, 2015 at 03:52:37PM +0200, Jacek Anaszewski wrote:
> >>Description of flash LEDs related properties was not precise regarding
> >>the state of corresponding settings in case a property is missing.
> >>Add relevant statements.
> >>Removed is also the requirement making the flash-max-microamp
> >>property obligatory for flash LEDs. It was inconsistent as the property
> >>is defined as optional. Devices which require the property will have
> >>to assert this in their DT bindings.
> >>
> >>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: Pavel Machek <pavel@ucw.cz>
> >>Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>Cc: devicetree@vger.kernel.org
> >>---
> >>  Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
> >>  1 file changed, 9 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> >>index 747c538..21a25e4 100644
> >>--- a/Documentation/devicetree/bindings/leds/common.txt
> >>+++ b/Documentation/devicetree/bindings/leds/common.txt
> >>@@ -29,13 +29,15 @@ Optional properties for child nodes:
> >>       "ide-disk" - LED indicates disk activity
> >>       "timer" - LED flashes at a fixed, configurable rate
> >>
> >>-- max-microamp : maximum intensity in microamperes of the LED
> >>-		 (torch LED for flash devices)
> >>-- flash-max-microamp : maximum intensity in microamperes of the
> >>-                       flash LED; it is mandatory if the LED should
> >>-		       support the flash mode
> >>-- flash-timeout-us : timeout in microseconds after which the flash
> >>-                     LED is turned off
> >>+- max-microamp : Maximum intensity in microamperes of the LED
> >>+		 (torch LED for flash devices). If omitted this will default
> >>+		 to the maximum current allowed by the device.
> >>+- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
> >>+		       If omitted this will default to the maximum
> >>+		       current allowed by the device.
> >>+- flash-timeout-us : Timeout in microseconds after which the flash
> >>+                     LED is turned off. If omitted this will default to the
> >>+		     maximum timeout allowed by the device.
> >>
> >>
> >>  Examples:
> >
> >Pavel pointed out that the brightness between maximum current and the
> >maximum *allowed* another current might not be noticeable, leading a
> >potential spelling error to cause the LED being run at too high current.
> 
> I think that a board designed so that it can be damaged because of
> software bugs should be considered not eligible for commercial use.
> Any self-esteeming manufacturer will not connect a LED to the output
> that can produce the current greater than the LED's absolute maximum
> current.

The maximum current *is* used to prevent potential hardware damage. This is
how mobile phones typically are, probably also the one you're using. :-) I
don't believe there's really a difference between vendors in this respect.

We still lack a proper way to model the temperature of the flash LED, so
what we have now is a bit incomplete, but at least it prevents causing
damage unintentionally.

> The DT properties could be useful for devices like aat1290 device I was
> writing a driver for, which has the maximum current and timeout values
> depending on corresponding capacitor and resistor values respectively.
> Such devices should make the properties required in their bindings.
> 
> >The three drivers I've looked also require these properties, which I think
> >is in the line with the above.
> >
> >How about either dropping the patch, or changing maximum to minimum and
> >will to should? The drivers could also behave this way instead of requiring
> >the properties, but I don't think there's anything wrong with requiring the
> >properties either.
> 
> As I mentioned in the previous message in this subject, the max-microamp
> property refers also to non-flash LEDs. Since existing LED class devices
> does not require them, then it should be left optional and default to
> max. It would however be inconsistent with flash LEDs related
> properties.

I do agree with Pavel here, these should be mandatory (at least for new
drivers) OR default to minimum.

-- 
Regards,

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

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08  1:20         ` Bryan Wu
  2015-04-08  7:48           ` Sakari Ailus
@ 2015-04-08  9:13           ` Pavel Machek
  1 sibling, 0 replies; 29+ messages in thread
From: Pavel Machek @ 2015-04-08  9:13 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Jacek Anaszewski, Sakari Ailus, Linux LED Subsystem, linux-media,
	Kyungmin Park, rpurdie, Sylwester Nawrocki, Sakari Ailus,
	devicetree

On Tue 2015-04-07 18:20:08, Bryan Wu wrote:
> On Fri, Apr 3, 2015 at 1:37 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> >>+- flash-timeout-us : Timeout in microseconds after which the flash
> >> >>+                     LED is turned off. If omitted this will default to the
> >> >>+                maximum timeout allowed by the device.
> >> >>
> >> >>
> >> >>  Examples:
> >> >
> >> >Pavel pointed out that the brightness between maximum current and the
> >> >maximum *allowed* another current might not be noticeable,leading a
> >> >potential spelling error to cause the LED being run at too high current.
> >>
> >> Where did he point this out? Do you think about the current version
> >> of the leds/common.txt documentation or there was some other message,
> >> that I don't see?
> >
> > Date: Thu, 2 Apr 2015 22:30:44 +0200
> > From: Pavel Machek <pavel@ucw.cz>
> > To: Sakari Ailus <sakari.ailus@iki.fi>
> > Subject: Re: [PATCHv3] media: i2c/adp1653: devicetree support for adp1653
> >
> >> Besides, I can't understand your point. Could you express it in other
> >> words, please?
> >
> > Typo in device tree would cause hardware damage. But idea. Make the
> > properties mandatory.
> >                                                                 Pavel
> 
> I don't quite follow there. I think Pavel acked this patch right? So
> what's left to hold here?

Yeah. Then I realized that patch is wrong/dangerous. Sorry about
that. Try to forget about my ACK if you can ;-).
									
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08  9:11       ` Sakari Ailus
@ 2015-04-08  9:17         ` Pavel Machek
  2015-04-08 10:23         ` Jacek Anaszewski
  1 sibling, 0 replies; 29+ messages in thread
From: Pavel Machek @ 2015-04-08  9:17 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Jacek Anaszewski, linux-leds, linux-media, kyungmin.park,
	cooloney, rpurdie, s.nawrocki, Sakari Ailus, devicetree

Hi!

> > I think that a board designed so that it can be damaged because of
> > software bugs should be considered not eligible for commercial
> > use.

Hello? It is 2015. Yes, that was nice rule... in 1995 or so :-).

> > As I mentioned in the previous message in this subject, the max-microamp
> > property refers also to non-flash LEDs. Since existing LED class devices
> > does not require them, then it should be left optional and default to
> > max. It would however be inconsistent with flash LEDs related
> > properties.

For non-flash LEDs and backward compatibility, I guess you are
right. Inconsistency is fine in this case...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
  2015-04-02 14:41   ` Pavel Machek
  2015-04-03 12:09   ` Sakari Ailus
@ 2015-04-08 10:03   ` Sylwester Nawrocki
  2015-04-08 10:36     ` Pavel Machek
  2015-04-08 11:58     ` Jacek Anaszewski
  2 siblings, 2 replies; 29+ messages in thread
From: Sylwester Nawrocki @ 2015-04-08 10:03 UTC (permalink / raw)
  To: Jacek Anaszewski, pavel, sakari.ailus, Sakari Ailus
  Cc: linux-leds, linux-media, kyungmin.park, cooloney, rpurdie, devicetree

Hello,

On 31/03/15 15:52, Jacek Anaszewski wrote:
> Description of flash LEDs related properties was not precise regarding
> the state of corresponding settings in case a property is missing.
> Add relevant statements.
> Removed is also the requirement making the flash-max-microamp
> property obligatory for flash LEDs. It was inconsistent as the property
> is defined as optional. Devices which require the property will have
> to assert this in their DT bindings.
> 
> 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: Pavel Machek <pavel@ucw.cz>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 747c538..21a25e4 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>       "ide-disk" - LED indicates disk activity
>       "timer" - LED flashes at a fixed, configurable rate
>  
> -- max-microamp : maximum intensity in microamperes of the LED
> -		 (torch LED for flash devices)
> -- flash-max-microamp : maximum intensity in microamperes of the
> -                       flash LED; it is mandatory if the LED should
> -		       support the flash mode
> -- flash-timeout-us : timeout in microseconds after which the flash
> -                     LED is turned off
> +- max-microamp : Maximum intensity in microamperes of the LED
> +		 (torch LED for flash devices). If omitted this will default
> +		 to the maximum current allowed by the device.
> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
> +		       If omitted this will default to the maximum
> +		       current allowed by the device.
> +- flash-timeout-us : Timeout in microseconds after which the flash
> +                     LED is turned off. If omitted this will default to the
> +		     maximum timeout allowed by the device.

Sorry about late comments on that, but since we can still change these
properties and it seems we're going to do that, I'd like throw in my
few preferences on the colour of this bike...

IMO "max-microamp" is a poor property name, how about:

s/max-microamp/led-max-current-ua,
s/flash-max-microamp/flash-max-current-ua,

so we have more consistent set of properties like:

led-max-current-ua
flash-max-current-ua
flash-timeout-us

Also expressing light intensity in micro-amperes seems technically wrong.
I would propose to substitute word "intensity in microamperes" with "LED
supply current in microamperes".

I also think we should require the maximum current properties and
the driver should warn if they are missing and limit current to some
potentially safe value, e.g. small fraction of the maximum current.

Also from the description it should be clear whether the current
limits refer to capabilities of a LED or the desired settings we want
to be applied at the LED driver device.
We could, for example, add a sentence after the above 3 properties:

"Required properties for Flash LEDs:

 - led-max-current-ua
 - flash-max-current-ua
 - flash-timeout-us

These properties determine a LED driver IC settings required for
safe operation."

Or something along these lines.

-- 
Regards,
Sylwester

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08  9:11       ` Sakari Ailus
  2015-04-08  9:17         ` Pavel Machek
@ 2015-04-08 10:23         ` Jacek Anaszewski
  2015-04-08 10:44           ` Sakari Ailus
  1 sibling, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-04-08 10:23 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, Sakari Ailus, devicetree

Hi Sakari,

On 04/08/2015 11:11 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Wed, Apr 08, 2015 at 10:54:52AM +0200, Jacek Anaszewski wrote:
>> Hi Sakari,
>>
>> On 04/03/2015 02:09 PM, Sakari Ailus wrote:
>>> Hi Jacek,
>>>
>>> On Tue, Mar 31, 2015 at 03:52:37PM +0200, Jacek Anaszewski wrote:
>>>> Description of flash LEDs related properties was not precise regarding
>>>> the state of corresponding settings in case a property is missing.
>>>> Add relevant statements.
>>>> Removed is also the requirement making the flash-max-microamp
>>>> property obligatory for flash LEDs. It was inconsistent as the property
>>>> is defined as optional. Devices which require the property will have
>>>> to assert this in their DT bindings.
>>>>
>>>> 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: Pavel Machek <pavel@ucw.cz>
>>>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>> Cc: devicetree@vger.kernel.org
>>>> ---
>>>>   Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
>>>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>>>> index 747c538..21a25e4 100644
>>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>>> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>>>>        "ide-disk" - LED indicates disk activity
>>>>        "timer" - LED flashes at a fixed, configurable rate
>>>>
>>>> -- max-microamp : maximum intensity in microamperes of the LED
>>>> -		 (torch LED for flash devices)
>>>> -- flash-max-microamp : maximum intensity in microamperes of the
>>>> -                       flash LED; it is mandatory if the LED should
>>>> -		       support the flash mode
>>>> -- flash-timeout-us : timeout in microseconds after which the flash
>>>> -                     LED is turned off
>>>> +- max-microamp : Maximum intensity in microamperes of the LED
>>>> +		 (torch LED for flash devices). If omitted this will default
>>>> +		 to the maximum current allowed by the device.
>>>> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
>>>> +		       If omitted this will default to the maximum
>>>> +		       current allowed by the device.
>>>> +- flash-timeout-us : Timeout in microseconds after which the flash
>>>> +                     LED is turned off. If omitted this will default to the
>>>> +		     maximum timeout allowed by the device.
>>>>
>>>>
>>>>   Examples:
>>>
>>> Pavel pointed out that the brightness between maximum current and the
>>> maximum *allowed* another current might not be noticeable, leading a
>>> potential spelling error to cause the LED being run at too high current.
>>
>> I think that a board designed so that it can be damaged because of
>> software bugs should be considered not eligible for commercial use.
>> Any self-esteeming manufacturer will not connect a LED to the output
>> that can produce the current greater than the LED's absolute maximum
>> current.
>
> The maximum current *is* used to prevent potential hardware damage.

What hardware are we talking about - LED controller or the discrete
LED component attached to the LED controller's current output?

The maximum current the LED controller can produce is fixed or depends
on external components like resistors.

This is at least the case for max77693 and aat1290 device I've been
working with. If a LED is rated to max 1A and it will be connected
to the output capable of producing 1.2A then it is likely that it
will be damaged. I was thinking about this kind of hardware damage.

How vendors can protect from it if they connect incompatible LED
to the current output?

There might be boards that provide sockets for connecting external
LEDs though. Such arrangements indeed justify the need for making the
properties required.

> This is
> how mobile phones typically are, probably also the one you're using. :-) I
> don't believe there's really a difference between vendors in this respect.
>
> We still lack a proper way to model the temperature of the flash LED, so
> what we have now is a bit incomplete, but at least it prevents causing
> damage unintentionally.

Are you thinking about flash faults?

>> The DT properties could be useful for devices like aat1290 device I was
>> writing a driver for, which has the maximum current and timeout values
>> depending on corresponding capacitor and resistor values respectively.
>> Such devices should make the properties required in their bindings.
>>
>>> The three drivers I've looked also require these properties, which I think
>>> is in the line with the above.
>>>
>>> How about either dropping the patch, or changing maximum to minimum and
>>> will to should? The drivers could also behave this way instead of requiring
>>> the properties, but I don't think there's anything wrong with requiring the
>>> properties either.
>>
>> As I mentioned in the previous message in this subject, the max-microamp
>> property refers also to non-flash LEDs. Since existing LED class devices
>> does not require them, then it should be left optional and default to
>> max. It would however be inconsistent with flash LEDs related
>> properties.
>
> I do agree with Pavel here, these should be mandatory (at least for new
> drivers) OR default to minimum.
>


-- 
Best Regards,
Jacek Anaszewski

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08 10:03   ` Sylwester Nawrocki
@ 2015-04-08 10:36     ` Pavel Machek
  2015-04-08 11:58     ` Jacek Anaszewski
  1 sibling, 0 replies; 29+ messages in thread
From: Pavel Machek @ 2015-04-08 10:36 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Jacek Anaszewski, sakari.ailus, Sakari Ailus, linux-leds,
	linux-media, kyungmin.park, cooloney, rpurdie, devicetree

On Wed 2015-04-08 12:03:27, Sylwester Nawrocki wrote:
> Hello,
> 
> On 31/03/15 15:52, Jacek Anaszewski wrote:
> > Description of flash LEDs related properties was not precise regarding
> > the state of corresponding settings in case a property is missing.
> > Add relevant statements.
> > Removed is also the requirement making the flash-max-microamp
> > property obligatory for flash LEDs. It was inconsistent as the property
> > is defined as optional. Devices which require the property will have
> > to assert this in their DT bindings.
> > 
> > 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: Pavel Machek <pavel@ucw.cz>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: devicetree@vger.kernel.org
> > ---
> >  Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> > index 747c538..21a25e4 100644
> > --- a/Documentation/devicetree/bindings/leds/common.txt
> > +++ b/Documentation/devicetree/bindings/leds/common.txt
> > @@ -29,13 +29,15 @@ Optional properties for child nodes:
> >       "ide-disk" - LED indicates disk activity
> >       "timer" - LED flashes at a fixed, configurable rate
> >  
> > -- max-microamp : maximum intensity in microamperes of the LED
> > -		 (torch LED for flash devices)
> > -- flash-max-microamp : maximum intensity in microamperes of the
> > -                       flash LED; it is mandatory if the LED should
> > -		       support the flash mode
> > -- flash-timeout-us : timeout in microseconds after which the flash
> > -                     LED is turned off
> > +- max-microamp : Maximum intensity in microamperes of the LED
> > +		 (torch LED for flash devices). If omitted this will default
> > +		 to the maximum current allowed by the device.
> > +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
> > +		       If omitted this will default to the maximum
> > +		       current allowed by the device.
> > +- flash-timeout-us : Timeout in microseconds after which the flash
> > +                     LED is turned off. If omitted this will default to the
> > +		     maximum timeout allowed by the device.
> 
> Sorry about late comments on that, but since we can still change these
> properties and it seems we're going to do that, I'd like throw in my
> few preferences on the colour of this bike...

Lets not paint bikes here.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08 10:23         ` Jacek Anaszewski
@ 2015-04-08 10:44           ` Sakari Ailus
  0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-04-08 10:44 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, Sakari Ailus, devicetree

Hi Jacek,

On Wed, Apr 08, 2015 at 12:23:23PM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 04/08/2015 11:11 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, Apr 08, 2015 at 10:54:52AM +0200, Jacek Anaszewski wrote:
> >>Hi Sakari,
> >>
> >>On 04/03/2015 02:09 PM, Sakari Ailus wrote:
> >>>Hi Jacek,
> >>>
> >>>On Tue, Mar 31, 2015 at 03:52:37PM +0200, Jacek Anaszewski wrote:
> >>>>Description of flash LEDs related properties was not precise regarding
> >>>>the state of corresponding settings in case a property is missing.
> >>>>Add relevant statements.
> >>>>Removed is also the requirement making the flash-max-microamp
> >>>>property obligatory for flash LEDs. It was inconsistent as the property
> >>>>is defined as optional. Devices which require the property will have
> >>>>to assert this in their DT bindings.
> >>>>
> >>>>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: Pavel Machek <pavel@ucw.cz>
> >>>>Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>>>Cc: devicetree@vger.kernel.org
> >>>>---
> >>>>  Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
> >>>>  1 file changed, 9 insertions(+), 7 deletions(-)
> >>>>
> >>>>diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> >>>>index 747c538..21a25e4 100644
> >>>>--- a/Documentation/devicetree/bindings/leds/common.txt
> >>>>+++ b/Documentation/devicetree/bindings/leds/common.txt
> >>>>@@ -29,13 +29,15 @@ Optional properties for child nodes:
> >>>>       "ide-disk" - LED indicates disk activity
> >>>>       "timer" - LED flashes at a fixed, configurable rate
> >>>>
> >>>>-- max-microamp : maximum intensity in microamperes of the LED
> >>>>-		 (torch LED for flash devices)
> >>>>-- flash-max-microamp : maximum intensity in microamperes of the
> >>>>-                       flash LED; it is mandatory if the LED should
> >>>>-		       support the flash mode
> >>>>-- flash-timeout-us : timeout in microseconds after which the flash
> >>>>-                     LED is turned off
> >>>>+- max-microamp : Maximum intensity in microamperes of the LED
> >>>>+		 (torch LED for flash devices). If omitted this will default
> >>>>+		 to the maximum current allowed by the device.
> >>>>+- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
> >>>>+		       If omitted this will default to the maximum
> >>>>+		       current allowed by the device.
> >>>>+- flash-timeout-us : Timeout in microseconds after which the flash
> >>>>+                     LED is turned off. If omitted this will default to the
> >>>>+		     maximum timeout allowed by the device.
> >>>>
> >>>>
> >>>>  Examples:
> >>>
> >>>Pavel pointed out that the brightness between maximum current and the
> >>>maximum *allowed* another current might not be noticeable, leading a
> >>>potential spelling error to cause the LED being run at too high current.
> >>
> >>I think that a board designed so that it can be damaged because of
> >>software bugs should be considered not eligible for commercial use.
> >>Any self-esteeming manufacturer will not connect a LED to the output
> >>that can produce the current greater than the LED's absolute maximum
> >>current.
> >
> >The maximum current *is* used to prevent potential hardware damage.
> 
> What hardware are we talking about - LED controller or the discrete
> LED component attached to the LED controller's current output?

Generally the LED itself, not the controller. Most controllers have
overheating protection while the LEDs do not.

> 
> The maximum current the LED controller can produce is fixed or depends
> on external components like resistors.

On some controllers perhaps, but not on most of them.

> 
> This is at least the case for max77693 and aat1290 device I've been
> working with. If a LED is rated to max 1A and it will be connected
> to the output capable of producing 1.2A then it is likely that it
> will be damaged. I was thinking about this kind of hardware damage.

This is the very reason why the maximum current limit is there: to prevent
hardware damage. If the LED could safely be used at the controller's maximum
current, there would be no need for the maximum current property
(torch/flash).

> 
> How vendors can protect from it if they connect incompatible LED
> to the current output?
> 
> There might be boards that provide sockets for connecting external
> LEDs though. Such arrangements indeed justify the need for making the
> properties required.

Pluggable hardware is a completely different matter. If used with DT
overlays, the overlay should contain the limits as well.

> 
> >This is
> >how mobile phones typically are, probably also the one you're using. :-) I
> >don't believe there's really a difference between vendors in this respect.
> >
> >We still lack a proper way to model the temperature of the flash LED, so
> >what we have now is a bit incomplete, but at least it prevents causing
> >damage unintentionally.
> 
> Are you thinking about flash faults?

The question is: when if it safe to strobe again after a strobe has ended?

-- 
Kind regards,

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

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08 10:03   ` Sylwester Nawrocki
  2015-04-08 10:36     ` Pavel Machek
@ 2015-04-08 11:58     ` Jacek Anaszewski
  2015-04-08 14:45       ` Sylwester Nawrocki
  1 sibling, 1 reply; 29+ messages in thread
From: Jacek Anaszewski @ 2015-04-08 11:58 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: pavel, sakari.ailus, Sakari Ailus, linux-leds, linux-media,
	kyungmin.park, cooloney, rpurdie, devicetree

Hi Sylwester,

On 04/08/2015 12:03 PM, Sylwester Nawrocki wrote:
> Hello,
>
> On 31/03/15 15:52, Jacek Anaszewski wrote:
>> Description of flash LEDs related properties was not precise regarding
>> the state of corresponding settings in case a property is missing.
>> Add relevant statements.
>> Removed is also the requirement making the flash-max-microamp
>> property obligatory for flash LEDs. It was inconsistent as the property
>> is defined as optional. Devices which require the property will have
>> to assert this in their DT bindings.
>>
>> 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: Pavel Machek <pavel@ucw.cz>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt |   16 +++++++++-------
>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 747c538..21a25e4 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>>        "ide-disk" - LED indicates disk activity
>>        "timer" - LED flashes at a fixed, configurable rate
>>
>> -- max-microamp : maximum intensity in microamperes of the LED
>> -		 (torch LED for flash devices)
>> -- flash-max-microamp : maximum intensity in microamperes of the
>> -                       flash LED; it is mandatory if the LED should
>> -		       support the flash mode
>> -- flash-timeout-us : timeout in microseconds after which the flash
>> -                     LED is turned off
>> +- max-microamp : Maximum intensity in microamperes of the LED
>> +		 (torch LED for flash devices). If omitted this will default
>> +		 to the maximum current allowed by the device.
>> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
>> +		       If omitted this will default to the maximum
>> +		       current allowed by the device.
>> +- flash-timeout-us : Timeout in microseconds after which the flash
>> +                     LED is turned off. If omitted this will default to the
>> +		     maximum timeout allowed by the device.
>
> Sorry about late comments on that, but since we can still change these
> properties and it seems we're going to do that, I'd like throw in my
> few preferences on the colour of this bike...
>
> IMO "max-microamp" is a poor property name, how about:
>
> s/max-microamp/led-max-current-ua,
> s/flash-max-microamp/flash-max-current-ua,
>
> so we have more consistent set of properties like:
>
> led-max-current-ua
> flash-max-current-ua
> flash-timeout-us

The "-microamp' suffix is consistent with regulator bindings.
Please refer to [1].

> Also expressing light intensity in micro-amperes seems technically wrong.
> I would propose to substitute word "intensity in microamperes" with "LED
> supply current in microamperes".

OK, I will address this in the next version of the patch.

> I also think we should require the maximum current properties and
> the driver should warn if they are missing and limit current to some
> potentially safe value, e.g. small fraction of the maximum current.

TBD.

> Also from the description it should be clear whether the current
> limits refer to capabilities of a LED or the desired settings we want
> to be applied at the LED driver device.
> We could, for example, add a sentence after the above 3 properties:
>
> "Required properties for Flash LEDs:
>
>   - led-max-current-ua
>   - flash-max-current-ua
>   - flash-timeout-us
>
> These properties determine a LED driver IC settings required for
> safe operation."
>
> Or something along these lines.

OK.


[1] http://www.spinics.net/lists/linux-leds/msg02674.html

-- 
Best Regards,
Jacek Anaszewski

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

* Re: [PATCH v4 06/12] of: Add Skyworks Solutions, Inc. vendor prefix
  2015-03-31 13:52 ` [PATCH v4 06/12] of: Add Skyworks Solutions, Inc. vendor prefix Jacek Anaszewski
@ 2015-04-08 11:59   ` Sakari Ailus
  0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-04-08 11:59 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-media, kyungmin.park, pavel, cooloney, rpurdie,
	s.nawrocki, devicetree

On Tue, Mar 31, 2015 at 03:52:42PM +0200, Jacek Anaszewski wrote:
> Use "skyworks" as the vendor prefix for the Skyworks Solutions, Inc.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 42b3dab..4cd18bb 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -163,6 +163,7 @@ ricoh	Ricoh Co. Ltd.
>  rockchip	Fuzhou Rockchip Electronics Co., Ltd
>  samsung	Samsung Semiconductor
>  sandisk	Sandisk Corporation
> +skyworks	Skyworks Solutions, Inc.

Please maintain the alphabetic order. With that fixed,

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

>  sbs	Smart Battery System
>  schindler	Schindler
>  seagate	Seagate Technology PLC

-- 
Regards,

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

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

* Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties
  2015-04-08 11:58     ` Jacek Anaszewski
@ 2015-04-08 14:45       ` Sylwester Nawrocki
  0 siblings, 0 replies; 29+ messages in thread
From: Sylwester Nawrocki @ 2015-04-08 14:45 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: pavel, sakari.ailus, Sakari Ailus, linux-leds, linux-media,
	kyungmin.park, cooloney, rpurdie, devicetree

Hi Jacek,

On 08/04/15 13:58, Jacek Anaszewski wrote:
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> >> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> >> @@ -29,13 +29,15 @@ Optional properties for child nodes:
>>> >>        "ide-disk" - LED indicates disk activity
>>> >>        "timer" - LED flashes at a fixed, configurable rate
>>> >>
>>> >> -- max-microamp : maximum intensity in microamperes of the LED
>>> >> -		 (torch LED for flash devices)
>>> >> -- flash-max-microamp : maximum intensity in microamperes of the
>>> >> -                       flash LED; it is mandatory if the LED should
>>> >> -		       support the flash mode
>>> >> -- flash-timeout-us : timeout in microseconds after which the flash
>>> >> -                     LED is turned off
>>> >> +- max-microamp : Maximum intensity in microamperes of the LED
>>> >> +		 (torch LED for flash devices). If omitted this will default
>>> >> +		 to the maximum current allowed by the device.
>>> >> +- flash-max-microamp : Maximum intensity in microamperes of the flash LED.
>>> >> +		       If omitted this will default to the maximum
>>> >> +		       current allowed by the device.
>>> >> +- flash-timeout-us : Timeout in microseconds after which the flash
>>> >> +                     LED is turned off. If omitted this will default to the
>>> >> +		     maximum timeout allowed by the device.
>> >
>> > Sorry about late comments on that, but since we can still change these
>> > properties and it seems we're going to do that, I'd like throw in my
>> > few preferences on the colour of this bike...
>> >
>> > IMO "max-microamp" is a poor property name, how about:
>> >
>> > s/max-microamp/led-max-current-ua,
>> > s/flash-max-microamp/flash-max-current-ua,
>> >
>> > so we have more consistent set of properties like:
>> >
>> > led-max-current-ua
>> > flash-max-current-ua
>> > flash-timeout-us
>
> The "-microamp' suffix is consistent with regulator bindings.
> Please refer to [1].

OK, in a perfect world we would have clean and consistent notation of
units. If it's acked let's leave it, I didn't know it was, sorry about
that.

When I read yesterday Documentation/devicetree/bindings/leds/common.txt
the set of new properties looked rather sloppy, especially "max-microamp"
looked incomplete to me, as if the subject was missing.

Anyway, I'll just get used to it, let's complete this whole Flash/LED
integration story.

-- 
Thanks,
Sylwester

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

end of thread, other threads:[~2015-04-08 14:45 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-31 13:52 [PATCH v4 00/12] LED / flash API integration Jacek Anaszewski
2015-03-31 13:52 ` [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties Jacek Anaszewski
2015-04-02 14:41   ` Pavel Machek
2015-04-03 12:09   ` Sakari Ailus
2015-04-03 12:56     ` Jacek Anaszewski
2015-04-03 20:37       ` Pavel Machek
2015-04-08  1:20         ` Bryan Wu
2015-04-08  7:48           ` Sakari Ailus
2015-04-08  9:13           ` Pavel Machek
2015-04-08  8:54     ` Jacek Anaszewski
2015-04-08  9:11       ` Sakari Ailus
2015-04-08  9:17         ` Pavel Machek
2015-04-08 10:23         ` Jacek Anaszewski
2015-04-08 10:44           ` Sakari Ailus
2015-04-08 10:03   ` Sylwester Nawrocki
2015-04-08 10:36     ` Pavel Machek
2015-04-08 11:58     ` Jacek Anaszewski
2015-04-08 14:45       ` Sylwester Nawrocki
2015-03-31 13:52 ` [PATCH v4 02/12] leds: unify the location of led-trigger API Jacek Anaszewski
2015-04-08  1:25   ` Bryan Wu
2015-03-31 13:52 ` [PATCH v4 03/12] leds: Add support for max77693 mfd flash cell Jacek Anaszewski
2015-03-31 13:52 ` [PATCH v4 04/12] DT: Add documentation for the mfd Maxim max77693 Jacek Anaszewski
2015-03-31 14:34   ` Jacek Anaszewski
2015-03-31 13:52 ` [PATCH v4 05/12] leds: Add driver for AAT1290 flash LED controller Jacek Anaszewski
2015-03-31 13:52 ` [PATCH v4 06/12] of: Add Skyworks Solutions, Inc. vendor prefix Jacek Anaszewski
2015-04-08 11:59   ` Sakari Ailus
2015-03-31 13:52 ` [PATCH v4 07/12] DT: Add documentation for the Skyworks AAT1290 Jacek Anaszewski
2015-03-31 13:52 ` [PATCH v4 08/12] media: Add registration helpers for V4L2 flash sub-devices Jacek Anaszewski
2015-03-31 13:52 ` [PATCH v4 09/12] Documentation: leds: Add description of v4l2-flash sub-device Jacek Anaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).