linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] AS3645A flash support
@ 2017-08-19 21:24 Sakari Ailus
  2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Sakari Ailus @ 2017-08-19 21:24 UTC (permalink / raw)
  To: linux-media; +Cc: javier, jacek.anaszewski, linux-leds, devicetree

Hi everyone,

This set adds support for the AS3645A flash driver which can be found e.g.
in Nokia N9.

The set depeds on the flash patches here so I'd prefer to merge this
through mediatree.

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=flash>

Since v1:

- Fix flash label use.

- Drop "flash" property from sensors in N9 DTS patch. This will reappear in
  a sepeate patch after the "flash" binding documentation patch.

- Remove PM support from the driver (the LED framework handles this).

- Enable runtime PM un as3645a driver.

- Add I²C device ID table for module autoloading.

- Add label to the binding examples as well as reference to common.txt.

- Document label as an optional property for both LED sub-nodes.

Since the RFC set:

- Add back the DT binding documentation I lost long ago.

- Use colon (":") in the default names instead of a space.

Sakari Ailus (3):
  dt: bindings: Document DT bindings for Analog devices as3645a
  leds: as3645a: Add LED flash class driver
  arm: dts: omap3: N9/N950: Add AS3645A camera flash

 .../devicetree/bindings/leds/ams,as3645a.txt       |  71 ++
 MAINTAINERS                                        |   6 +
 arch/arm/boot/dts/omap3-n950-n9.dtsi               |  14 +
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-as3645a.c                        | 770 +++++++++++++++++++++
 6 files changed, 870 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
 create mode 100644 drivers/leds/leds-as3645a.c

-- 
2.11.0

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

* [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-19 21:24 [PATCH v2 0/3] AS3645A flash support Sakari Ailus
@ 2017-08-19 21:24 ` Sakari Ailus
  2017-08-22 18:34   ` Jacek Anaszewski
                     ` (2 more replies)
  2017-08-19 21:24 ` [PATCH v2 2/3] leds: as3645a: Add LED flash class driver Sakari Ailus
  2017-08-19 21:24 ` [PATCH v2 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash Sakari Ailus
  2 siblings, 3 replies; 17+ messages in thread
From: Sakari Ailus @ 2017-08-19 21:24 UTC (permalink / raw)
  To: linux-media
  Cc: javier, jacek.anaszewski, linux-leds, devicetree, Sakari Ailus

From: Sakari Ailus <sakari.ailus@iki.fi>

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../devicetree/bindings/leds/ams,as3645a.txt       | 71 ++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt

diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
new file mode 100644
index 000000000000..12c5ef26ec73
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
@@ -0,0 +1,71 @@
+Analog devices AS3645A device tree bindings
+
+The AS3645A flash LED controller can drive two LEDs, one high current
+flash LED and one indicator LED. The high current flash LED can be
+used in torch mode as well.
+
+Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
+and b are included in the range.
+
+Please also see common.txt in the same directory.
+
+
+Required properties
+===================
+
+compatible	: Must be "ams,as3645a".
+reg		: The I2C address of the device. Typically 0x30.
+
+
+Required properties of the "flash" child node
+=============================================
+
+flash-timeout-us: Flash timeout in microseconds. The value must be in
+		  the range [100000, 850000] and divisible by 50000.
+flash-max-microamp: Maximum flash current in microamperes. Has to be
+		    in the range between [200000, 500000] and
+		    divisible by 20000.
+led-max-microamp: Maximum torch (assist) current in microamperes. The
+		  value must be in the range between [20000, 160000] and
+		  divisible by 20000.
+ams,input-max-microamp: Maximum flash controller input current. The
+			value must be in the range [1250000, 2000000]
+			and divisible by 50000.
+
+
+Optional properties of the "flash" child node
+=============================================
+
+label		: The label of the flash LED.
+
+
+Required properties of the "indicator" child node
+=================================================
+
+led-max-microamp: Maximum indicator current. The allowed values are
+		  2500, 5000, 7500 and 10000.
+
+Optional properties of the "indicator" child node
+=================================================
+
+label		: The label of the indicator LED.
+
+
+Example
+=======
+
+	as3645a@30 {
+		reg = <0x30>;
+		compatible = "ams,as3645a";
+		flash {
+			flash-timeout-us = <150000>;
+			flash-max-microamp = <320000>;
+			led-max-microamp = <60000>;
+			ams,input-max-microamp = <1750000>;
+			label = "as3645a:flash";
+		};
+		indicator {
+			led-max-microamp = <10000>;
+			label = "as3645a:indicator";
+		};
+	};
-- 
2.11.0

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

* [PATCH v2 2/3] leds: as3645a: Add LED flash class driver
  2017-08-19 21:24 [PATCH v2 0/3] AS3645A flash support Sakari Ailus
  2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
@ 2017-08-19 21:24 ` Sakari Ailus
  2017-08-19 21:42   ` [PATCH v2.1 " Sakari Ailus
  2017-08-19 21:24 ` [PATCH v2 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash Sakari Ailus
  2 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2017-08-19 21:24 UTC (permalink / raw)
  To: linux-media
  Cc: javier, jacek.anaszewski, linux-leds, devicetree, Sakari Ailus

From: Sakari Ailus <sakari.ailus@iki.fi>

Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
is based on that.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 MAINTAINERS                 |   6 +
 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-as3645a.c | 770 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 785 insertions(+)
 create mode 100644 drivers/leds/leds-as3645a.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 931abca006b7..8f40ba2e5303 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2124,6 +2124,12 @@ F:	arch/arm64/
 F:	Documentation/arm64/
 
 AS3645A LED FLASH CONTROLLER DRIVER
+M:	Sakari Ailus <sakari.ailus@iki.fi>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+F:	drivers/leds/leds-as3645a.c
+
+AS3645A LED FLASH CONTROLLER DRIVER
 M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 L:	linux-media@vger.kernel.org
 T:	git git://linuxtv.org/media_tree.git
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 594b24d410c3..bad3a4098104 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -58,6 +58,14 @@ config LEDS_AAT1290
 	help
 	 This option enables support for the LEDs on the AAT1290.
 
+config LEDS_AS3645A
+	tristate "AS3645A LED flash controller support"
+	depends on I2C && LEDS_CLASS_FLASH
+	help
+	  Enable LED flash class support for AS3645A LED flash
+	  controller. V4L2 flash API is provided as well if
+	  CONFIG_V4L2_FLASH_API is enabled.
+
 config LEDS_BCM6328
 	tristate "LED Support for Broadcom BCM6328"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 909dae62ba05..7d7b26552923 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -8,6 +8,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_AS3645A)		+= leds-as3645a.o
 obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
new file mode 100644
index 000000000000..9e9e10907e87
--- /dev/null
+++ b/drivers/leds/leds-as3645a.c
@@ -0,0 +1,770 @@
+/*
+ * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver
+ *
+ * Copyright (C) 2008-2011 Nokia Corporation
+ * Copyright (c) 2011, 2017 Intel Corporation.
+ *
+ * Based on drivers/media/i2c/as3645a.c.
+ *
+ * Contact: Sakari Ailus <sakari.ailus@iki.fi>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.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/pm_runtime.h>
+#include <linux/slab.h>
+
+#include <media/v4l2-flash-led-class.h>
+
+#define AS_TIMER_US_TO_CODE(t)			(((t) / 1000 - 100) / 50)
+#define AS_TIMER_CODE_TO_US(c)			((50 * (c) + 100) * 1000)
+
+/* Register definitions */
+
+/* Read-only Design info register: Reset state: xxxx 0001 */
+#define AS_DESIGN_INFO_REG			0x00
+#define AS_DESIGN_INFO_FACTORY(x)		(((x) >> 4))
+#define AS_DESIGN_INFO_MODEL(x)			((x) & 0x0f)
+
+/* Read-only Version control register: Reset state: 0000 0000
+ * for first engineering samples
+ */
+#define AS_VERSION_CONTROL_REG			0x01
+#define AS_VERSION_CONTROL_RFU(x)		(((x) >> 4))
+#define AS_VERSION_CONTROL_VERSION(x)		((x) & 0x0f)
+
+/* Read / Write	(Indicator and timer register): Reset state: 0000 1111 */
+#define AS_INDICATOR_AND_TIMER_REG		0x02
+#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT	0
+#define AS_INDICATOR_AND_TIMER_VREF_SHIFT	4
+#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT	6
+
+/* Read / Write	(Current set register): Reset state: 0110 1001 */
+#define AS_CURRENT_SET_REG			0x03
+#define AS_CURRENT_ASSIST_LIGHT_SHIFT		0
+#define AS_CURRENT_LED_DET_ON			(1 << 3)
+#define AS_CURRENT_FLASH_CURRENT_SHIFT		4
+
+/* Read / Write	(Control register): Reset state: 1011 0100 */
+#define AS_CONTROL_REG				0x04
+#define AS_CONTROL_MODE_SETTING_SHIFT		0
+#define AS_CONTROL_STROBE_ON			(1 << 2)
+#define AS_CONTROL_OUT_ON			(1 << 3)
+#define AS_CONTROL_EXT_TORCH_ON			(1 << 4)
+#define AS_CONTROL_STROBE_TYPE_EDGE		(0 << 5)
+#define AS_CONTROL_STROBE_TYPE_LEVEL		(1 << 5)
+#define AS_CONTROL_COIL_PEAK_SHIFT		6
+
+/* Read only (D3 is read / write) (Fault and info): Reset state: 0000 x000 */
+#define AS_FAULT_INFO_REG			0x05
+#define AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT	(1 << 1)
+#define AS_FAULT_INFO_INDICATOR_LED		(1 << 2)
+#define AS_FAULT_INFO_LED_AMOUNT		(1 << 3)
+#define AS_FAULT_INFO_TIMEOUT			(1 << 4)
+#define AS_FAULT_INFO_OVER_TEMPERATURE		(1 << 5)
+#define AS_FAULT_INFO_SHORT_CIRCUIT		(1 << 6)
+#define AS_FAULT_INFO_OVER_VOLTAGE		(1 << 7)
+
+/* Boost register */
+#define AS_BOOST_REG				0x0d
+#define AS_BOOST_CURRENT_DISABLE		(0 << 0)
+#define AS_BOOST_CURRENT_ENABLE			(1 << 0)
+
+/* Password register is used to unlock boost register writing */
+#define AS_PASSWORD_REG				0x0f
+#define AS_PASSWORD_UNLOCK_VALUE		0x55
+
+#define AS_NAME					"as3645a"
+#define AS_I2C_ADDR				(0x60 >> 1) /* W:0x60, R:0x61 */
+
+#define AS_FLASH_TIMEOUT_MIN			100000	/* us */
+#define AS_FLASH_TIMEOUT_MAX			850000
+#define AS_FLASH_TIMEOUT_STEP			50000
+
+#define AS_FLASH_INTENSITY_MIN			200000	/* uA */
+#define AS_FLASH_INTENSITY_MAX_1LED		500000
+#define AS_FLASH_INTENSITY_MAX_2LEDS		400000
+#define AS_FLASH_INTENSITY_STEP			20000
+
+#define AS_TORCH_INTENSITY_MIN			20000	/* uA */
+#define AS_TORCH_INTENSITY_MAX			160000
+#define AS_TORCH_INTENSITY_STEP			20000
+
+#define AS_INDICATOR_INTENSITY_MIN		0	/* uA */
+#define AS_INDICATOR_INTENSITY_MAX		10000
+#define AS_INDICATOR_INTENSITY_STEP		2500
+
+#define AS_PEAK_mA_MAX				2000
+#define AS_PEAK_mA_TO_REG(a) \
+	((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
+
+enum as_mode {
+	AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
+	AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,
+	AS_MODE_ASSIST = 2 << AS_CONTROL_MODE_SETTING_SHIFT,
+	AS_MODE_FLASH = 3 << AS_CONTROL_MODE_SETTING_SHIFT,
+};
+
+struct as3645a_config {
+	u32 flash_timeout_us;
+	u32 flash_max_ua;
+	u32 assist_max_ua;
+	u32 indicator_max_ua;
+	u32 voltage_reference;
+	u32 peak;
+};
+
+struct as3645a_names {
+	char flash[32];
+	char indicator[32];
+};
+
+struct as3645a {
+	struct i2c_client *client;
+
+	struct mutex mutex;
+
+	struct led_classdev_flash fled;
+	struct led_classdev iled_cdev;
+
+	struct v4l2_flash *vf;
+	struct v4l2_flash *vfind;
+
+	struct device_node *flash_node;
+	struct device_node *indicator_node;
+
+	struct as3645a_config cfg;
+
+	enum as_mode mode;
+	unsigned int timeout;
+	unsigned int flash_current;
+	unsigned int assist_current;
+	unsigned int indicator_current;
+	enum v4l2_flash_strobe_source strobe_source;
+};
+
+#define fled_to_as3645a(__fled) container_of(__fled, struct as3645a, fled)
+#define iled_cdev_to_as3645a(__iled_cdev) \
+	container_of(__iled_cdev, struct as3645a, iled_cdev)
+
+/* Return negative errno else zero on success */
+static int as3645a_write(struct as3645a *flash, u8 addr, u8 val)
+{
+	struct i2c_client *client = flash->client;
+	int rval;
+
+	rval = i2c_smbus_write_byte_data(client, addr, val);
+
+	dev_dbg(&client->dev, "Write Addr:%02X Val:%02X %s\n", addr, val,
+		rval < 0 ? "fail" : "ok");
+
+	return rval;
+}
+
+/* Return negative errno else a data byte received from the device. */
+static int as3645a_read(struct as3645a *flash, u8 addr)
+{
+	struct i2c_client *client = flash->client;
+	int rval;
+
+	rval = i2c_smbus_read_byte_data(client, addr);
+
+	dev_dbg(&client->dev, "Read Addr:%02X Val:%02X %s\n", addr, rval,
+		rval < 0 ? "fail" : "ok");
+
+	return rval;
+}
+
+/* -----------------------------------------------------------------------------
+ * Hardware configuration and trigger
+ */
+
+/**
+ * as3645a_set_config - Set flash configuration registers
+ * @flash: The flash
+ *
+ * Configure the hardware with flash, assist and indicator currents, as well as
+ * flash timeout.
+ *
+ * Return 0 on success, or a negative error code if an I2C communication error
+ * occurred.
+ */
+static int as3645a_set_current(struct as3645a *flash)
+{
+	u8 val;
+
+	val = (flash->flash_current << AS_CURRENT_FLASH_CURRENT_SHIFT)
+	    | (flash->assist_current << AS_CURRENT_ASSIST_LIGHT_SHIFT)
+	    | AS_CURRENT_LED_DET_ON;
+
+	return as3645a_write(flash, AS_CURRENT_SET_REG, val);
+}
+
+static int as3645a_set_timeout(struct as3645a *flash)
+{
+	u8 val;
+
+	val = flash->timeout << AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT;
+
+	val |= (flash->cfg.voltage_reference
+		<< AS_INDICATOR_AND_TIMER_VREF_SHIFT)
+	    |  ((flash->indicator_current ? flash->indicator_current - 1 : 0)
+		 << AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT);
+
+	return as3645a_write(flash, AS_INDICATOR_AND_TIMER_REG, val);
+}
+
+/**
+ * as3645a_set_control - Set flash control register
+ * @flash: The flash
+ * @mode: Desired output mode
+ * @on: Desired output state
+ *
+ * Configure the hardware with output mode and state.
+ *
+ * Return 0 on success, or a negative error code if an I2C communication error
+ * occurred.
+ */
+static int
+as3645a_set_control(struct as3645a *flash, enum as_mode mode, bool on)
+{
+	u8 reg;
+
+	/* Configure output parameters and operation mode. */
+	reg = (flash->cfg.peak << AS_CONTROL_COIL_PEAK_SHIFT)
+	    | (on ? AS_CONTROL_OUT_ON : 0)
+	    | mode;
+
+	if (mode == AS_MODE_FLASH &&
+	    flash->strobe_source == V4L2_FLASH_STROBE_SOURCE_EXTERNAL)
+		reg |= AS_CONTROL_STROBE_TYPE_LEVEL
+		    |  AS_CONTROL_STROBE_ON;
+
+	return as3645a_write(flash, AS_CONTROL_REG, reg);
+}
+
+static int as3645a_get_fault(struct led_classdev_flash *fled, u32 *fault)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+	int rval;
+
+	/* NOTE: reading register clears fault status */
+	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
+	if (rval < 0)
+		return rval;
+
+	if (rval & AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT)
+		*fault |= LED_FAULT_OVER_CURRENT;
+
+	if (rval & AS_FAULT_INFO_INDICATOR_LED)
+		*fault |= LED_FAULT_INDICATOR;
+
+	dev_dbg(&flash->client->dev, "%u connected LEDs\n",
+		rval & AS_FAULT_INFO_LED_AMOUNT ? 2 : 1);
+
+	if (rval & AS_FAULT_INFO_TIMEOUT)
+		*fault |= LED_FAULT_TIMEOUT;
+
+	if (rval & AS_FAULT_INFO_OVER_TEMPERATURE)
+		*fault |= LED_FAULT_OVER_TEMPERATURE;
+
+	if (rval & AS_FAULT_INFO_SHORT_CIRCUIT)
+		*fault |= LED_FAULT_OVER_CURRENT;
+
+	if (rval & AS_FAULT_INFO_OVER_VOLTAGE)
+		*fault |= LED_FAULT_INPUT_VOLTAGE;
+
+	return rval;
+}
+
+static unsigned int __as3645a_current_to_reg(unsigned int min, unsigned int max,
+					     unsigned int step,
+					     unsigned int val)
+{
+	if (val < min)
+		val = min;
+
+	if (val > max)
+		val = max;
+
+	return (val - min) / step;
+}
+
+static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool is_flash,
+					   unsigned int ua)
+{
+	if (is_flash)
+		return __as3645a_current_to_reg(AS_TORCH_INTENSITY_MIN,
+						flash->cfg.assist_max_ua,
+						AS_TORCH_INTENSITY_STEP, ua);
+	else
+		return __as3645a_current_to_reg(AS_FLASH_INTENSITY_MIN,
+						flash->cfg.flash_max_ua,
+						AS_FLASH_INTENSITY_STEP, ua);
+}
+
+static int as3645a_set_indicator_brightness(struct led_classdev *iled_cdev,
+					    enum led_brightness brightness)
+{
+	struct as3645a *flash = iled_cdev_to_as3645a(iled_cdev);
+	int rval;
+
+	flash->indicator_current = brightness;
+
+	rval = as3645a_set_timeout(flash);
+	if (rval)
+		return rval;
+
+	return as3645a_set_control(flash, AS_MODE_INDICATOR, brightness);
+}
+
+static int as3645a_set_assist_brightness(struct led_classdev *fled_cdev,
+					 enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled = lcdev_to_flcdev(fled_cdev);
+	struct as3645a *flash = fled_to_as3645a(fled);
+	int rval;
+
+	if (brightness) {
+		/* Register value 0 is 20 mA. */
+		flash->assist_current = brightness - 1;
+
+		rval = as3645a_set_current(flash);
+		if (rval)
+			return rval;
+	}
+
+	return as3645a_set_control(flash, AS_MODE_ASSIST, brightness);
+}
+
+static int as3645a_set_flash_brightness(struct led_classdev_flash *fled,
+					u32 brightness_ua)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+
+	flash->flash_current = as3645a_current_to_reg(flash, true, brightness_ua);
+
+	return as3645a_set_current(flash);
+}
+
+static int as3645a_set_flash_timeout(struct led_classdev_flash *fled,
+				     u32 timeout_us)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+
+	flash->timeout = AS_TIMER_US_TO_CODE(timeout_us);
+
+	return as3645a_set_timeout(flash);
+}
+
+static int as3645a_set_strobe(struct led_classdev_flash *fled, bool state)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+
+	return as3645a_set_control(flash, AS_MODE_FLASH, state);
+}
+
+static const struct led_flash_ops as3645a_led_flash_ops = {
+	.flash_brightness_set = as3645a_set_flash_brightness,
+	.timeout_set = as3645a_set_flash_timeout,
+	.strobe_set = as3645a_set_strobe,
+	.fault_get = as3645a_get_fault,
+};
+
+static int as3645a_setup(struct as3645a *flash)
+{
+	struct device *dev = &flash->client->dev;
+	u32 fault = 0;
+	int rval;
+
+	/* clear errors */
+	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
+	if (rval < 0)
+		return rval;
+
+	dev_dbg(dev, "Fault info: %02x\n", rval);
+
+	rval = as3645a_set_current(flash);
+	if (rval < 0)
+		return rval;
+
+	rval = as3645a_set_timeout(flash);
+	if (rval < 0)
+		return rval;
+
+	rval = as3645a_set_control(flash, AS_MODE_INDICATOR, false);
+	if (rval < 0)
+		return rval;
+
+	/* read status */
+	rval = as3645a_get_fault(&flash->fled, &fault);
+	if (rval < 0)
+		return rval;
+
+	dev_dbg(dev, "AS_INDICATOR_AND_TIMER_REG: %02x\n",
+		as3645a_read(flash, AS_INDICATOR_AND_TIMER_REG));
+	dev_dbg(dev, "AS_CURRENT_SET_REG: %02x\n",
+		as3645a_read(flash, AS_CURRENT_SET_REG));
+	dev_dbg(dev, "AS_CONTROL_REG: %02x\n",
+		as3645a_read(flash, AS_CONTROL_REG));
+
+	return rval & ~AS_FAULT_INFO_LED_AMOUNT ? -EIO : 0;
+}
+
+static int as3645a_detect(struct as3645a *flash)
+{
+	struct device *dev = &flash->client->dev;
+	int rval, man, model, rfu, version;
+	const char *vendor;
+
+	rval = as3645a_read(flash, AS_DESIGN_INFO_REG);
+	if (rval < 0) {
+		dev_err(dev, "can't read design info reg\n");
+		return rval;
+	}
+
+	man = AS_DESIGN_INFO_FACTORY(rval);
+	model = AS_DESIGN_INFO_MODEL(rval);
+
+	rval = as3645a_read(flash, AS_VERSION_CONTROL_REG);
+	if (rval < 0) {
+		dev_err(dev, "can't read version control reg\n");
+		return rval;
+	}
+
+	rfu = AS_VERSION_CONTROL_RFU(rval);
+	version = AS_VERSION_CONTROL_VERSION(rval);
+
+	/* Verify the chip model and version. */
+	if (model != 0x01 || rfu != 0x00) {
+		dev_err(dev, "AS3645A not detected "
+			"(model %d rfu %d)\n", model, rfu);
+		return -ENODEV;
+	}
+
+	switch (man) {
+	case 1:
+		vendor = "AMS, Austria Micro Systems";
+		break;
+	case 2:
+		vendor = "ADI, Analog Devices Inc.";
+		break;
+	case 3:
+		vendor = "NSC, National Semiconductor";
+		break;
+	case 4:
+		vendor = "NXP";
+		break;
+	case 5:
+		vendor = "TI, Texas Instrument";
+		break;
+	default:
+		vendor = "Unknown";
+	}
+
+	dev_info(dev, "Chip vendor: %s (%d) Version: %d\n", vendor,
+		 man, version);
+
+	rval = as3645a_write(flash, AS_PASSWORD_REG, AS_PASSWORD_UNLOCK_VALUE);
+	if (rval < 0)
+		return rval;
+
+	return as3645a_write(flash, AS_BOOST_REG, AS_BOOST_CURRENT_DISABLE);
+}
+
+static int as3645a_parse_node(struct as3645a *flash,
+			      struct as3645a_names *names,
+			      struct device_node *node)
+{
+	struct as3645a_config *cfg = &flash->cfg;
+	const char *name;
+	int rval;
+
+	flash->flash_node = of_get_child_by_name(node, "flash");
+	if (!flash->flash_node) {
+		dev_err(&flash->client->dev, "can't find flash node\n");
+		return -ENODEV;
+	}
+
+	rval = of_property_read_string(flash->flash_node, "label", &name);
+	if (!rval)
+		strlcpy(names->flash, name, sizeof(names->flash));
+	else
+		snprintf(names->flash, sizeof(names->flash),
+			 "%s:flash", node->name);
+
+	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
+				    &cfg->flash_timeout_us);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read flash-timeout-us property for flash\n");
+		goto out_err;
+	}
+
+	rval = of_property_read_u32(flash->flash_node, "flash-max-microamp",
+				    &cfg->flash_max_ua);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read flash-max-microamp property for flash\n");
+		goto out_err;
+	}
+
+	rval = of_property_read_u32(flash->flash_node, "led-max-microamp",
+				    &cfg->assist_max_ua);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read led-max-microamp property for flash\n");
+		goto out_err;
+	}
+
+	of_property_read_u32(flash->flash_node, "voltage-reference",
+			     &cfg->voltage_reference);
+
+	of_property_read_u32(flash->flash_node, "peak-current-limit",
+			     &cfg->peak);
+	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
+
+	flash->indicator_node = of_get_child_by_name(node, "indicator");
+	if (!flash->indicator_node) {
+		dev_warn(&flash->client->dev,
+			 "can't find indicator node\n");
+		goto out_err;
+	}
+
+	rval = of_property_read_string(flash->indicator_node, "label", &name);
+	if (!rval)
+		strlcpy(names->indicator, name, sizeof(names->indicator));
+	else
+		snprintf(names->indicator, sizeof(names->indicator),
+			 "%s:indicator", node->name);
+
+	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
+				    &cfg->indicator_max_ua);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read led-max-microamp property for indicator\n");
+		goto out_err;
+	}
+
+	return 0;
+
+out_err:
+	of_node_put(flash->flash_node);
+	of_node_put(flash->indicator_node);
+
+	return rval;
+}
+
+static int as3645a_led_class_setup(struct as3645a *flash,
+				   struct as3645a_names *names)
+{
+	struct led_classdev *fled_cdev = &flash->fled.led_cdev;
+	struct led_classdev *iled_cdev = &flash->iled_cdev;
+	struct led_flash_setting *cfg;
+	int rval;
+
+	iled_cdev->name = names->indicator;
+	iled_cdev->brightness_set_blocking = as3645a_set_indicator_brightness;
+	iled_cdev->max_brightness =
+		flash->cfg.indicator_max_ua / AS_INDICATOR_INTENSITY_STEP;
+	iled_cdev->flags = LED_CORE_SUSPENDRESUME;
+
+	rval = led_classdev_register(&flash->client->dev, iled_cdev);
+	if (rval < 0)
+		return rval;
+
+	cfg = &flash->fled.brightness;
+	cfg->min = AS_FLASH_INTENSITY_MIN;
+	cfg->max = flash->cfg.flash_max_ua;
+	cfg->step = AS_FLASH_INTENSITY_STEP;
+	cfg->val = flash->cfg.flash_max_ua;
+
+	cfg = &flash->fled.timeout;
+	cfg->min = AS_FLASH_TIMEOUT_MIN;
+	cfg->max = flash->cfg.flash_timeout_us;
+	cfg->step = AS_FLASH_TIMEOUT_STEP;
+	cfg->val = flash->cfg.flash_timeout_us;
+
+	flash->fled.ops = &as3645a_led_flash_ops;
+
+	fled_cdev->name = names->flash;
+	fled_cdev->brightness_set_blocking = as3645a_set_assist_brightness;
+	/* Value 0 is off in LED class. */
+	fled_cdev->max_brightness =
+		as3645a_current_to_reg(flash, false,
+				       flash->cfg.assist_max_ua) + 1;
+	fled_cdev->flags = LED_DEV_CAP_FLASH | LED_CORE_SUSPENDRESUME;
+
+	rval = led_classdev_flash_register(&flash->client->dev, &flash->fled);
+	if (rval) {
+		led_classdev_unregister(iled_cdev);
+		dev_err(&flash->client->dev,
+			"led_classdev_flash_register() failed, error %d\n",
+			rval);
+	}
+
+	return rval;
+}
+
+static int as3645a_v4l2_setup(struct as3645a *flash)
+{
+	struct led_classdev_flash *fled = &flash->fled;
+	struct led_classdev *led = &fled->led_cdev;
+	struct v4l2_flash_config cfg = {
+		.intensity = {
+			.min = AS_TORCH_INTENSITY_MIN,
+			.max = flash->cfg.assist_max_ua,
+			.step = AS_TORCH_INTENSITY_STEP,
+			.val = flash->cfg.assist_max_ua,
+		},
+	};
+	struct v4l2_flash_config cfgind = {
+		.intensity = {
+			.min = AS_INDICATOR_INTENSITY_MIN,
+			.max = flash->cfg.indicator_max_ua,
+			.step = AS_INDICATOR_INTENSITY_STEP,
+			.val = flash->cfg.indicator_max_ua,
+		},
+	};
+
+	strlcpy(cfg.dev_name, led->name, sizeof(cfg.dev_name));
+	strlcpy(cfgind.dev_name, flash->iled_cdev.name, sizeof(cfg.dev_name));
+
+	flash->vf = v4l2_flash_init(
+		&flash->client->dev, of_fwnode_handle(flash->flash_node),
+		&flash->fled, NULL, &cfg);
+	if (IS_ERR(flash->vf))
+		return PTR_ERR(flash->vf);
+
+	flash->vfind = v4l2_flash_indicator_init(
+		&flash->client->dev, of_fwnode_handle(flash->indicator_node),
+		&flash->iled_cdev, &cfgind);
+	if (IS_ERR(flash->vfind)) {
+		v4l2_flash_release(flash->vf);
+		return PTR_ERR(flash->vfind);
+	}
+
+	return 0;
+}
+
+static int as3645a_probe(struct i2c_client *client)
+{
+	struct as3645a_names names;
+	struct as3645a *flash;
+	int rval;
+
+	if (client->dev.of_node == NULL)
+		return -ENODEV;
+
+	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
+	if (flash == NULL)
+		return -ENOMEM;
+
+	flash->client = client;
+
+	rval = as3645a_parse_node(flash, &names, client->dev.of_node);
+	if (rval < 0)
+		return rval;
+
+	rval = as3645a_detect(flash);
+	if (rval < 0)
+		goto out_put_nodes;
+
+	mutex_init(&flash->mutex);
+	i2c_set_clientdata(client, flash);
+
+	rval = as3645a_setup(flash);
+	if (rval)
+		goto out_mutex_destroy;
+
+	rval = as3645a_led_class_setup(flash, &names);
+	if (rval)
+		goto out_mutex_destroy;
+
+	rval = as3645a_v4l2_setup(flash);
+	if (rval)
+		goto out_led_classdev_flash_unregister;
+
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+
+	return 0;
+
+out_led_classdev_flash_unregister:
+	led_classdev_flash_unregister(&flash->fled);
+
+out_mutex_destroy:
+	mutex_destroy(&flash->mutex);
+
+out_put_nodes:
+	of_node_put(flash->flash_node);
+	of_node_put(flash->indicator_node);
+
+	return rval;
+}
+
+static int as3645a_remove(struct i2c_client *client)
+{
+	struct as3645a *flash = i2c_get_clientdata(client);
+
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+
+	as3645a_set_control(flash, AS_MODE_EXT_TORCH, false);
+
+	v4l2_flash_release(flash->vf);
+
+	led_classdev_flash_unregister(&flash->fled);
+	led_classdev_unregister(&flash->iled_cdev);
+
+	mutex_destroy(&flash->mutex);
+
+	of_node_put(flash->flash_node);
+	of_node_put(flash->indicator_node);
+
+	return 0;
+}
+
+static const struct i2c_device_id as3645a_id_table[] = {
+	{ AS_NAME, 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, as3645a_id_table);
+
+static const struct of_device_id as3645a_of_table[] = {
+	{ .compatible = "ams,as3645a" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, as3645a_of_table);
+
+static struct i2c_driver as3645a_i2c_driver = {
+	.driver	= {
+		.of_match_table = as3645a_of_table,
+		.name = AS_NAME,
+	},
+	.probe_new	= as3645a_probe,
+	.remove	= as3645a_remove,
+	.id_table = as3645a_id_table,
+};
+
+module_i2c_driver(as3645a_i2c_driver);
+
+MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
+MODULE_AUTHOR("Sakari Ailus <sakari.ailus@iki.fi>");
+MODULE_DESCRIPTION("LED flash driver for AS3645A, LM3555 and their clones");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

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

* [PATCH v2 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash
  2017-08-19 21:24 [PATCH v2 0/3] AS3645A flash support Sakari Ailus
  2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
  2017-08-19 21:24 ` [PATCH v2 2/3] leds: as3645a: Add LED flash class driver Sakari Ailus
@ 2017-08-19 21:24 ` Sakari Ailus
  2017-08-21 19:57   ` [PATCH v2.1 " Sakari Ailus
  2 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2017-08-19 21:24 UTC (permalink / raw)
  To: linux-media
  Cc: javier, jacek.anaszewski, linux-leds, devicetree, Sakari Ailus

From: Sakari Ailus <sakari.ailus@iki.fi>

Add the as3645a flash controller to the DT source.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index df3366fa5409..92c1a1da28d3 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -265,6 +265,20 @@
 
 &i2c2 {
 	clock-frequency = <400000>;
+
+	as3645a@30 {
+		reg = <0x30>;
+		compatible = "ams,as3645a";
+		as3645a_flash: flash {
+			flash-timeout-us = <150000>;
+			flash-max-microamp = <320000>;
+			led-max-microamp = <60000>;
+			peak-current-limit = <1750000>;
+		};
+		as3645a_indicator: indicator {
+			led-max-microamp = <10000>;
+		};
+	};
 };
 
 &i2c3 {
-- 
2.11.0

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

* [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
  2017-08-19 21:24 ` [PATCH v2 2/3] leds: as3645a: Add LED flash class driver Sakari Ailus
@ 2017-08-19 21:42   ` Sakari Ailus
  2017-08-20 10:09     ` Jacek Anaszewski
  0 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2017-08-19 21:42 UTC (permalink / raw)
  To: linux-media
  Cc: javier, jacek.anaszewski, linux-leds, devicetree, Sakari Ailus

From: Sakari Ailus <sakari.ailus@iki.fi>

Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
is based on that.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Well. The driver does not control explicit power resources nor it used
runtime PM. Remove references to it.

 MAINTAINERS                 |   6 +
 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-as3645a.c | 763 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 778 insertions(+)
 create mode 100644 drivers/leds/leds-as3645a.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 931abca006b7..8f40ba2e5303 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2124,6 +2124,12 @@ F:	arch/arm64/
 F:	Documentation/arm64/
 
 AS3645A LED FLASH CONTROLLER DRIVER
+M:	Sakari Ailus <sakari.ailus@iki.fi>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+F:	drivers/leds/leds-as3645a.c
+
+AS3645A LED FLASH CONTROLLER DRIVER
 M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 L:	linux-media@vger.kernel.org
 T:	git git://linuxtv.org/media_tree.git
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 594b24d410c3..bad3a4098104 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -58,6 +58,14 @@ config LEDS_AAT1290
 	help
 	 This option enables support for the LEDs on the AAT1290.
 
+config LEDS_AS3645A
+	tristate "AS3645A LED flash controller support"
+	depends on I2C && LEDS_CLASS_FLASH
+	help
+	  Enable LED flash class support for AS3645A LED flash
+	  controller. V4L2 flash API is provided as well if
+	  CONFIG_V4L2_FLASH_API is enabled.
+
 config LEDS_BCM6328
 	tristate "LED Support for Broadcom BCM6328"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 909dae62ba05..7d7b26552923 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -8,6 +8,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_AS3645A)		+= leds-as3645a.o
 obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
new file mode 100644
index 000000000000..bbbbe0898233
--- /dev/null
+++ b/drivers/leds/leds-as3645a.c
@@ -0,0 +1,763 @@
+/*
+ * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver
+ *
+ * Copyright (C) 2008-2011 Nokia Corporation
+ * Copyright (c) 2011, 2017 Intel Corporation.
+ *
+ * Based on drivers/media/i2c/as3645a.c.
+ *
+ * Contact: Sakari Ailus <sakari.ailus@iki.fi>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.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/slab.h>
+
+#include <media/v4l2-flash-led-class.h>
+
+#define AS_TIMER_US_TO_CODE(t)			(((t) / 1000 - 100) / 50)
+#define AS_TIMER_CODE_TO_US(c)			((50 * (c) + 100) * 1000)
+
+/* Register definitions */
+
+/* Read-only Design info register: Reset state: xxxx 0001 */
+#define AS_DESIGN_INFO_REG			0x00
+#define AS_DESIGN_INFO_FACTORY(x)		(((x) >> 4))
+#define AS_DESIGN_INFO_MODEL(x)			((x) & 0x0f)
+
+/* Read-only Version control register: Reset state: 0000 0000
+ * for first engineering samples
+ */
+#define AS_VERSION_CONTROL_REG			0x01
+#define AS_VERSION_CONTROL_RFU(x)		(((x) >> 4))
+#define AS_VERSION_CONTROL_VERSION(x)		((x) & 0x0f)
+
+/* Read / Write	(Indicator and timer register): Reset state: 0000 1111 */
+#define AS_INDICATOR_AND_TIMER_REG		0x02
+#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT	0
+#define AS_INDICATOR_AND_TIMER_VREF_SHIFT	4
+#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT	6
+
+/* Read / Write	(Current set register): Reset state: 0110 1001 */
+#define AS_CURRENT_SET_REG			0x03
+#define AS_CURRENT_ASSIST_LIGHT_SHIFT		0
+#define AS_CURRENT_LED_DET_ON			(1 << 3)
+#define AS_CURRENT_FLASH_CURRENT_SHIFT		4
+
+/* Read / Write	(Control register): Reset state: 1011 0100 */
+#define AS_CONTROL_REG				0x04
+#define AS_CONTROL_MODE_SETTING_SHIFT		0
+#define AS_CONTROL_STROBE_ON			(1 << 2)
+#define AS_CONTROL_OUT_ON			(1 << 3)
+#define AS_CONTROL_EXT_TORCH_ON			(1 << 4)
+#define AS_CONTROL_STROBE_TYPE_EDGE		(0 << 5)
+#define AS_CONTROL_STROBE_TYPE_LEVEL		(1 << 5)
+#define AS_CONTROL_COIL_PEAK_SHIFT		6
+
+/* Read only (D3 is read / write) (Fault and info): Reset state: 0000 x000 */
+#define AS_FAULT_INFO_REG			0x05
+#define AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT	(1 << 1)
+#define AS_FAULT_INFO_INDICATOR_LED		(1 << 2)
+#define AS_FAULT_INFO_LED_AMOUNT		(1 << 3)
+#define AS_FAULT_INFO_TIMEOUT			(1 << 4)
+#define AS_FAULT_INFO_OVER_TEMPERATURE		(1 << 5)
+#define AS_FAULT_INFO_SHORT_CIRCUIT		(1 << 6)
+#define AS_FAULT_INFO_OVER_VOLTAGE		(1 << 7)
+
+/* Boost register */
+#define AS_BOOST_REG				0x0d
+#define AS_BOOST_CURRENT_DISABLE		(0 << 0)
+#define AS_BOOST_CURRENT_ENABLE			(1 << 0)
+
+/* Password register is used to unlock boost register writing */
+#define AS_PASSWORD_REG				0x0f
+#define AS_PASSWORD_UNLOCK_VALUE		0x55
+
+#define AS_NAME					"as3645a"
+#define AS_I2C_ADDR				(0x60 >> 1) /* W:0x60, R:0x61 */
+
+#define AS_FLASH_TIMEOUT_MIN			100000	/* us */
+#define AS_FLASH_TIMEOUT_MAX			850000
+#define AS_FLASH_TIMEOUT_STEP			50000
+
+#define AS_FLASH_INTENSITY_MIN			200000	/* uA */
+#define AS_FLASH_INTENSITY_MAX_1LED		500000
+#define AS_FLASH_INTENSITY_MAX_2LEDS		400000
+#define AS_FLASH_INTENSITY_STEP			20000
+
+#define AS_TORCH_INTENSITY_MIN			20000	/* uA */
+#define AS_TORCH_INTENSITY_MAX			160000
+#define AS_TORCH_INTENSITY_STEP			20000
+
+#define AS_INDICATOR_INTENSITY_MIN		0	/* uA */
+#define AS_INDICATOR_INTENSITY_MAX		10000
+#define AS_INDICATOR_INTENSITY_STEP		2500
+
+#define AS_PEAK_mA_MAX				2000
+#define AS_PEAK_mA_TO_REG(a) \
+	((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
+
+enum as_mode {
+	AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
+	AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,
+	AS_MODE_ASSIST = 2 << AS_CONTROL_MODE_SETTING_SHIFT,
+	AS_MODE_FLASH = 3 << AS_CONTROL_MODE_SETTING_SHIFT,
+};
+
+struct as3645a_config {
+	u32 flash_timeout_us;
+	u32 flash_max_ua;
+	u32 assist_max_ua;
+	u32 indicator_max_ua;
+	u32 voltage_reference;
+	u32 peak;
+};
+
+struct as3645a_names {
+	char flash[32];
+	char indicator[32];
+};
+
+struct as3645a {
+	struct i2c_client *client;
+
+	struct mutex mutex;
+
+	struct led_classdev_flash fled;
+	struct led_classdev iled_cdev;
+
+	struct v4l2_flash *vf;
+	struct v4l2_flash *vfind;
+
+	struct device_node *flash_node;
+	struct device_node *indicator_node;
+
+	struct as3645a_config cfg;
+
+	enum as_mode mode;
+	unsigned int timeout;
+	unsigned int flash_current;
+	unsigned int assist_current;
+	unsigned int indicator_current;
+	enum v4l2_flash_strobe_source strobe_source;
+};
+
+#define fled_to_as3645a(__fled) container_of(__fled, struct as3645a, fled)
+#define iled_cdev_to_as3645a(__iled_cdev) \
+	container_of(__iled_cdev, struct as3645a, iled_cdev)
+
+/* Return negative errno else zero on success */
+static int as3645a_write(struct as3645a *flash, u8 addr, u8 val)
+{
+	struct i2c_client *client = flash->client;
+	int rval;
+
+	rval = i2c_smbus_write_byte_data(client, addr, val);
+
+	dev_dbg(&client->dev, "Write Addr:%02X Val:%02X %s\n", addr, val,
+		rval < 0 ? "fail" : "ok");
+
+	return rval;
+}
+
+/* Return negative errno else a data byte received from the device. */
+static int as3645a_read(struct as3645a *flash, u8 addr)
+{
+	struct i2c_client *client = flash->client;
+	int rval;
+
+	rval = i2c_smbus_read_byte_data(client, addr);
+
+	dev_dbg(&client->dev, "Read Addr:%02X Val:%02X %s\n", addr, rval,
+		rval < 0 ? "fail" : "ok");
+
+	return rval;
+}
+
+/* -----------------------------------------------------------------------------
+ * Hardware configuration and trigger
+ */
+
+/**
+ * as3645a_set_config - Set flash configuration registers
+ * @flash: The flash
+ *
+ * Configure the hardware with flash, assist and indicator currents, as well as
+ * flash timeout.
+ *
+ * Return 0 on success, or a negative error code if an I2C communication error
+ * occurred.
+ */
+static int as3645a_set_current(struct as3645a *flash)
+{
+	u8 val;
+
+	val = (flash->flash_current << AS_CURRENT_FLASH_CURRENT_SHIFT)
+	    | (flash->assist_current << AS_CURRENT_ASSIST_LIGHT_SHIFT)
+	    | AS_CURRENT_LED_DET_ON;
+
+	return as3645a_write(flash, AS_CURRENT_SET_REG, val);
+}
+
+static int as3645a_set_timeout(struct as3645a *flash)
+{
+	u8 val;
+
+	val = flash->timeout << AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT;
+
+	val |= (flash->cfg.voltage_reference
+		<< AS_INDICATOR_AND_TIMER_VREF_SHIFT)
+	    |  ((flash->indicator_current ? flash->indicator_current - 1 : 0)
+		 << AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT);
+
+	return as3645a_write(flash, AS_INDICATOR_AND_TIMER_REG, val);
+}
+
+/**
+ * as3645a_set_control - Set flash control register
+ * @flash: The flash
+ * @mode: Desired output mode
+ * @on: Desired output state
+ *
+ * Configure the hardware with output mode and state.
+ *
+ * Return 0 on success, or a negative error code if an I2C communication error
+ * occurred.
+ */
+static int
+as3645a_set_control(struct as3645a *flash, enum as_mode mode, bool on)
+{
+	u8 reg;
+
+	/* Configure output parameters and operation mode. */
+	reg = (flash->cfg.peak << AS_CONTROL_COIL_PEAK_SHIFT)
+	    | (on ? AS_CONTROL_OUT_ON : 0)
+	    | mode;
+
+	if (mode == AS_MODE_FLASH &&
+	    flash->strobe_source == V4L2_FLASH_STROBE_SOURCE_EXTERNAL)
+		reg |= AS_CONTROL_STROBE_TYPE_LEVEL
+		    |  AS_CONTROL_STROBE_ON;
+
+	return as3645a_write(flash, AS_CONTROL_REG, reg);
+}
+
+static int as3645a_get_fault(struct led_classdev_flash *fled, u32 *fault)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+	int rval;
+
+	/* NOTE: reading register clears fault status */
+	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
+	if (rval < 0)
+		return rval;
+
+	if (rval & AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT)
+		*fault |= LED_FAULT_OVER_CURRENT;
+
+	if (rval & AS_FAULT_INFO_INDICATOR_LED)
+		*fault |= LED_FAULT_INDICATOR;
+
+	dev_dbg(&flash->client->dev, "%u connected LEDs\n",
+		rval & AS_FAULT_INFO_LED_AMOUNT ? 2 : 1);
+
+	if (rval & AS_FAULT_INFO_TIMEOUT)
+		*fault |= LED_FAULT_TIMEOUT;
+
+	if (rval & AS_FAULT_INFO_OVER_TEMPERATURE)
+		*fault |= LED_FAULT_OVER_TEMPERATURE;
+
+	if (rval & AS_FAULT_INFO_SHORT_CIRCUIT)
+		*fault |= LED_FAULT_OVER_CURRENT;
+
+	if (rval & AS_FAULT_INFO_OVER_VOLTAGE)
+		*fault |= LED_FAULT_INPUT_VOLTAGE;
+
+	return rval;
+}
+
+static unsigned int __as3645a_current_to_reg(unsigned int min, unsigned int max,
+					     unsigned int step,
+					     unsigned int val)
+{
+	if (val < min)
+		val = min;
+
+	if (val > max)
+		val = max;
+
+	return (val - min) / step;
+}
+
+static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool is_flash,
+					   unsigned int ua)
+{
+	if (is_flash)
+		return __as3645a_current_to_reg(AS_TORCH_INTENSITY_MIN,
+						flash->cfg.assist_max_ua,
+						AS_TORCH_INTENSITY_STEP, ua);
+	else
+		return __as3645a_current_to_reg(AS_FLASH_INTENSITY_MIN,
+						flash->cfg.flash_max_ua,
+						AS_FLASH_INTENSITY_STEP, ua);
+}
+
+static int as3645a_set_indicator_brightness(struct led_classdev *iled_cdev,
+					    enum led_brightness brightness)
+{
+	struct as3645a *flash = iled_cdev_to_as3645a(iled_cdev);
+	int rval;
+
+	flash->indicator_current = brightness;
+
+	rval = as3645a_set_timeout(flash);
+	if (rval)
+		return rval;
+
+	return as3645a_set_control(flash, AS_MODE_INDICATOR, brightness);
+}
+
+static int as3645a_set_assist_brightness(struct led_classdev *fled_cdev,
+					 enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled = lcdev_to_flcdev(fled_cdev);
+	struct as3645a *flash = fled_to_as3645a(fled);
+	int rval;
+
+	if (brightness) {
+		/* Register value 0 is 20 mA. */
+		flash->assist_current = brightness - 1;
+
+		rval = as3645a_set_current(flash);
+		if (rval)
+			return rval;
+	}
+
+	return as3645a_set_control(flash, AS_MODE_ASSIST, brightness);
+}
+
+static int as3645a_set_flash_brightness(struct led_classdev_flash *fled,
+					u32 brightness_ua)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+
+	flash->flash_current = as3645a_current_to_reg(flash, true, brightness_ua);
+
+	return as3645a_set_current(flash);
+}
+
+static int as3645a_set_flash_timeout(struct led_classdev_flash *fled,
+				     u32 timeout_us)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+
+	flash->timeout = AS_TIMER_US_TO_CODE(timeout_us);
+
+	return as3645a_set_timeout(flash);
+}
+
+static int as3645a_set_strobe(struct led_classdev_flash *fled, bool state)
+{
+	struct as3645a *flash = fled_to_as3645a(fled);
+
+	return as3645a_set_control(flash, AS_MODE_FLASH, state);
+}
+
+static const struct led_flash_ops as3645a_led_flash_ops = {
+	.flash_brightness_set = as3645a_set_flash_brightness,
+	.timeout_set = as3645a_set_flash_timeout,
+	.strobe_set = as3645a_set_strobe,
+	.fault_get = as3645a_get_fault,
+};
+
+static int as3645a_setup(struct as3645a *flash)
+{
+	struct device *dev = &flash->client->dev;
+	u32 fault = 0;
+	int rval;
+
+	/* clear errors */
+	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
+	if (rval < 0)
+		return rval;
+
+	dev_dbg(dev, "Fault info: %02x\n", rval);
+
+	rval = as3645a_set_current(flash);
+	if (rval < 0)
+		return rval;
+
+	rval = as3645a_set_timeout(flash);
+	if (rval < 0)
+		return rval;
+
+	rval = as3645a_set_control(flash, AS_MODE_INDICATOR, false);
+	if (rval < 0)
+		return rval;
+
+	/* read status */
+	rval = as3645a_get_fault(&flash->fled, &fault);
+	if (rval < 0)
+		return rval;
+
+	dev_dbg(dev, "AS_INDICATOR_AND_TIMER_REG: %02x\n",
+		as3645a_read(flash, AS_INDICATOR_AND_TIMER_REG));
+	dev_dbg(dev, "AS_CURRENT_SET_REG: %02x\n",
+		as3645a_read(flash, AS_CURRENT_SET_REG));
+	dev_dbg(dev, "AS_CONTROL_REG: %02x\n",
+		as3645a_read(flash, AS_CONTROL_REG));
+
+	return rval & ~AS_FAULT_INFO_LED_AMOUNT ? -EIO : 0;
+}
+
+static int as3645a_detect(struct as3645a *flash)
+{
+	struct device *dev = &flash->client->dev;
+	int rval, man, model, rfu, version;
+	const char *vendor;
+
+	rval = as3645a_read(flash, AS_DESIGN_INFO_REG);
+	if (rval < 0) {
+		dev_err(dev, "can't read design info reg\n");
+		return rval;
+	}
+
+	man = AS_DESIGN_INFO_FACTORY(rval);
+	model = AS_DESIGN_INFO_MODEL(rval);
+
+	rval = as3645a_read(flash, AS_VERSION_CONTROL_REG);
+	if (rval < 0) {
+		dev_err(dev, "can't read version control reg\n");
+		return rval;
+	}
+
+	rfu = AS_VERSION_CONTROL_RFU(rval);
+	version = AS_VERSION_CONTROL_VERSION(rval);
+
+	/* Verify the chip model and version. */
+	if (model != 0x01 || rfu != 0x00) {
+		dev_err(dev, "AS3645A not detected "
+			"(model %d rfu %d)\n", model, rfu);
+		return -ENODEV;
+	}
+
+	switch (man) {
+	case 1:
+		vendor = "AMS, Austria Micro Systems";
+		break;
+	case 2:
+		vendor = "ADI, Analog Devices Inc.";
+		break;
+	case 3:
+		vendor = "NSC, National Semiconductor";
+		break;
+	case 4:
+		vendor = "NXP";
+		break;
+	case 5:
+		vendor = "TI, Texas Instrument";
+		break;
+	default:
+		vendor = "Unknown";
+	}
+
+	dev_info(dev, "Chip vendor: %s (%d) Version: %d\n", vendor,
+		 man, version);
+
+	rval = as3645a_write(flash, AS_PASSWORD_REG, AS_PASSWORD_UNLOCK_VALUE);
+	if (rval < 0)
+		return rval;
+
+	return as3645a_write(flash, AS_BOOST_REG, AS_BOOST_CURRENT_DISABLE);
+}
+
+static int as3645a_parse_node(struct as3645a *flash,
+			      struct as3645a_names *names,
+			      struct device_node *node)
+{
+	struct as3645a_config *cfg = &flash->cfg;
+	const char *name;
+	int rval;
+
+	flash->flash_node = of_get_child_by_name(node, "flash");
+	if (!flash->flash_node) {
+		dev_err(&flash->client->dev, "can't find flash node\n");
+		return -ENODEV;
+	}
+
+	rval = of_property_read_string(flash->flash_node, "label", &name);
+	if (!rval)
+		strlcpy(names->flash, name, sizeof(names->flash));
+	else
+		snprintf(names->flash, sizeof(names->flash),
+			 "%s:flash", node->name);
+
+	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
+				    &cfg->flash_timeout_us);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read flash-timeout-us property for flash\n");
+		goto out_err;
+	}
+
+	rval = of_property_read_u32(flash->flash_node, "flash-max-microamp",
+				    &cfg->flash_max_ua);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read flash-max-microamp property for flash\n");
+		goto out_err;
+	}
+
+	rval = of_property_read_u32(flash->flash_node, "led-max-microamp",
+				    &cfg->assist_max_ua);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read led-max-microamp property for flash\n");
+		goto out_err;
+	}
+
+	of_property_read_u32(flash->flash_node, "voltage-reference",
+			     &cfg->voltage_reference);
+
+	of_property_read_u32(flash->flash_node, "peak-current-limit",
+			     &cfg->peak);
+	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
+
+	flash->indicator_node = of_get_child_by_name(node, "indicator");
+	if (!flash->indicator_node) {
+		dev_warn(&flash->client->dev,
+			 "can't find indicator node\n");
+		goto out_err;
+	}
+
+	rval = of_property_read_string(flash->indicator_node, "label", &name);
+	if (!rval)
+		strlcpy(names->indicator, name, sizeof(names->indicator));
+	else
+		snprintf(names->indicator, sizeof(names->indicator),
+			 "%s:indicator", node->name);
+
+	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
+				    &cfg->indicator_max_ua);
+	if (rval < 0) {
+		dev_err(&flash->client->dev,
+			"can't read led-max-microamp property for indicator\n");
+		goto out_err;
+	}
+
+	return 0;
+
+out_err:
+	of_node_put(flash->flash_node);
+	of_node_put(flash->indicator_node);
+
+	return rval;
+}
+
+static int as3645a_led_class_setup(struct as3645a *flash,
+				   struct as3645a_names *names)
+{
+	struct led_classdev *fled_cdev = &flash->fled.led_cdev;
+	struct led_classdev *iled_cdev = &flash->iled_cdev;
+	struct led_flash_setting *cfg;
+	int rval;
+
+	iled_cdev->name = names->indicator;
+	iled_cdev->brightness_set_blocking = as3645a_set_indicator_brightness;
+	iled_cdev->max_brightness =
+		flash->cfg.indicator_max_ua / AS_INDICATOR_INTENSITY_STEP;
+	iled_cdev->flags = LED_CORE_SUSPENDRESUME;
+
+	rval = led_classdev_register(&flash->client->dev, iled_cdev);
+	if (rval < 0)
+		return rval;
+
+	cfg = &flash->fled.brightness;
+	cfg->min = AS_FLASH_INTENSITY_MIN;
+	cfg->max = flash->cfg.flash_max_ua;
+	cfg->step = AS_FLASH_INTENSITY_STEP;
+	cfg->val = flash->cfg.flash_max_ua;
+
+	cfg = &flash->fled.timeout;
+	cfg->min = AS_FLASH_TIMEOUT_MIN;
+	cfg->max = flash->cfg.flash_timeout_us;
+	cfg->step = AS_FLASH_TIMEOUT_STEP;
+	cfg->val = flash->cfg.flash_timeout_us;
+
+	flash->fled.ops = &as3645a_led_flash_ops;
+
+	fled_cdev->name = names->flash;
+	fled_cdev->brightness_set_blocking = as3645a_set_assist_brightness;
+	/* Value 0 is off in LED class. */
+	fled_cdev->max_brightness =
+		as3645a_current_to_reg(flash, false,
+				       flash->cfg.assist_max_ua) + 1;
+	fled_cdev->flags = LED_DEV_CAP_FLASH | LED_CORE_SUSPENDRESUME;
+
+	rval = led_classdev_flash_register(&flash->client->dev, &flash->fled);
+	if (rval) {
+		led_classdev_unregister(iled_cdev);
+		dev_err(&flash->client->dev,
+			"led_classdev_flash_register() failed, error %d\n",
+			rval);
+	}
+
+	return rval;
+}
+
+static int as3645a_v4l2_setup(struct as3645a *flash)
+{
+	struct led_classdev_flash *fled = &flash->fled;
+	struct led_classdev *led = &fled->led_cdev;
+	struct v4l2_flash_config cfg = {
+		.intensity = {
+			.min = AS_TORCH_INTENSITY_MIN,
+			.max = flash->cfg.assist_max_ua,
+			.step = AS_TORCH_INTENSITY_STEP,
+			.val = flash->cfg.assist_max_ua,
+		},
+	};
+	struct v4l2_flash_config cfgind = {
+		.intensity = {
+			.min = AS_INDICATOR_INTENSITY_MIN,
+			.max = flash->cfg.indicator_max_ua,
+			.step = AS_INDICATOR_INTENSITY_STEP,
+			.val = flash->cfg.indicator_max_ua,
+		},
+	};
+
+	strlcpy(cfg.dev_name, led->name, sizeof(cfg.dev_name));
+	strlcpy(cfgind.dev_name, flash->iled_cdev.name, sizeof(cfg.dev_name));
+
+	flash->vf = v4l2_flash_init(
+		&flash->client->dev, of_fwnode_handle(flash->flash_node),
+		&flash->fled, NULL, &cfg);
+	if (IS_ERR(flash->vf))
+		return PTR_ERR(flash->vf);
+
+	flash->vfind = v4l2_flash_indicator_init(
+		&flash->client->dev, of_fwnode_handle(flash->indicator_node),
+		&flash->iled_cdev, &cfgind);
+	if (IS_ERR(flash->vfind)) {
+		v4l2_flash_release(flash->vf);
+		return PTR_ERR(flash->vfind);
+	}
+
+	return 0;
+}
+
+static int as3645a_probe(struct i2c_client *client)
+{
+	struct as3645a_names names;
+	struct as3645a *flash;
+	int rval;
+
+	if (client->dev.of_node == NULL)
+		return -ENODEV;
+
+	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
+	if (flash == NULL)
+		return -ENOMEM;
+
+	flash->client = client;
+
+	rval = as3645a_parse_node(flash, &names, client->dev.of_node);
+	if (rval < 0)
+		return rval;
+
+	rval = as3645a_detect(flash);
+	if (rval < 0)
+		goto out_put_nodes;
+
+	mutex_init(&flash->mutex);
+	i2c_set_clientdata(client, flash);
+
+	rval = as3645a_setup(flash);
+	if (rval)
+		goto out_mutex_destroy;
+
+	rval = as3645a_led_class_setup(flash, &names);
+	if (rval)
+		goto out_mutex_destroy;
+
+	rval = as3645a_v4l2_setup(flash);
+	if (rval)
+		goto out_led_classdev_flash_unregister;
+
+	return 0;
+
+out_led_classdev_flash_unregister:
+	led_classdev_flash_unregister(&flash->fled);
+
+out_mutex_destroy:
+	mutex_destroy(&flash->mutex);
+
+out_put_nodes:
+	of_node_put(flash->flash_node);
+	of_node_put(flash->indicator_node);
+
+	return rval;
+}
+
+static int as3645a_remove(struct i2c_client *client)
+{
+	struct as3645a *flash = i2c_get_clientdata(client);
+
+	as3645a_set_control(flash, AS_MODE_EXT_TORCH, false);
+
+	v4l2_flash_release(flash->vf);
+
+	led_classdev_flash_unregister(&flash->fled);
+	led_classdev_unregister(&flash->iled_cdev);
+
+	mutex_destroy(&flash->mutex);
+
+	of_node_put(flash->flash_node);
+	of_node_put(flash->indicator_node);
+
+	return 0;
+}
+
+static const struct i2c_device_id as3645a_id_table[] = {
+	{ AS_NAME, 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, as3645a_id_table);
+
+static const struct of_device_id as3645a_of_table[] = {
+	{ .compatible = "ams,as3645a" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, as3645a_of_table);
+
+static struct i2c_driver as3645a_i2c_driver = {
+	.driver	= {
+		.of_match_table = as3645a_of_table,
+		.name = AS_NAME,
+	},
+	.probe_new	= as3645a_probe,
+	.remove	= as3645a_remove,
+	.id_table = as3645a_id_table,
+};
+
+module_i2c_driver(as3645a_i2c_driver);
+
+MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
+MODULE_AUTHOR("Sakari Ailus <sakari.ailus@iki.fi>");
+MODULE_DESCRIPTION("LED flash driver for AS3645A, LM3555 and their clones");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

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

* Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
  2017-08-19 21:42   ` [PATCH v2.1 " Sakari Ailus
@ 2017-08-20 10:09     ` Jacek Anaszewski
  2017-08-20 10:12       ` Jacek Anaszewski
  2017-08-21 13:53       ` Sakari Ailus
  0 siblings, 2 replies; 17+ messages in thread
From: Jacek Anaszewski @ 2017-08-20 10:09 UTC (permalink / raw)
  To: Sakari Ailus, linux-media; +Cc: javier, linux-leds, devicetree, Sakari Ailus

Hi Sakari,

Thanks for the update.
I've noticed that you added node labels to the child device nodes
in [0]:

"as3645a_flash : flash" and "as3645a_indicator : indicator"

I am still seeing problems with this approach:

1) AFAIK these labels are only used for referencing nodes inside dts
   files and they don't affect the name property of struct device_node
2) Even if you changed the node name from flash to as3645a_flash, you
   would get weird LED class device name "as3645a_flash:flash" in case
   label property is absent. Do you have any objections against the
   approach I proposed in the previous review?:


    snprintf(names->flash, sizeof(names->flash),
	     AS_NAME":%s", node->name);

Best regards,
Jacek Anaszewski

On 08/19/2017 11:42 PM, Sakari Ailus wrote:
> From: Sakari Ailus <sakari.ailus@iki.fi>
> 
> Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
> driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
> is based on that.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Well. The driver does not control explicit power resources nor it used
> runtime PM. Remove references to it.
> 
>  MAINTAINERS                 |   6 +
>  drivers/leds/Kconfig        |   8 +
>  drivers/leds/Makefile       |   1 +
>  drivers/leds/leds-as3645a.c | 763 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 778 insertions(+)
>  create mode 100644 drivers/leds/leds-as3645a.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 931abca006b7..8f40ba2e5303 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2124,6 +2124,12 @@ F:	arch/arm64/
>  F:	Documentation/arm64/
>  
>  AS3645A LED FLASH CONTROLLER DRIVER
> +M:	Sakari Ailus <sakari.ailus@iki.fi>
> +L:	linux-leds@vger.kernel.org
> +S:	Maintained
> +F:	drivers/leds/leds-as3645a.c
> +
> +AS3645A LED FLASH CONTROLLER DRIVER
>  M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>  L:	linux-media@vger.kernel.org
>  T:	git git://linuxtv.org/media_tree.git
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 594b24d410c3..bad3a4098104 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -58,6 +58,14 @@ config LEDS_AAT1290
>  	help
>  	 This option enables support for the LEDs on the AAT1290.
>  
> +config LEDS_AS3645A
> +	tristate "AS3645A LED flash controller support"
> +	depends on I2C && LEDS_CLASS_FLASH
> +	help
> +	  Enable LED flash class support for AS3645A LED flash
> +	  controller. V4L2 flash API is provided as well if
> +	  CONFIG_V4L2_FLASH_API is enabled.
> +
>  config LEDS_BCM6328
>  	tristate "LED Support for Broadcom BCM6328"
>  	depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 909dae62ba05..7d7b26552923 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -8,6 +8,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_AS3645A)		+= leds-as3645a.o
>  obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
>  obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
>  obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
> diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
> new file mode 100644
> index 000000000000..bbbbe0898233
> --- /dev/null
> +++ b/drivers/leds/leds-as3645a.c
> @@ -0,0 +1,763 @@
> +/*
> + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver
> + *
> + * Copyright (C) 2008-2011 Nokia Corporation
> + * Copyright (c) 2011, 2017 Intel Corporation.
> + *
> + * Based on drivers/media/i2c/as3645a.c.
> + *
> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.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/slab.h>
> +
> +#include <media/v4l2-flash-led-class.h>
> +
> +#define AS_TIMER_US_TO_CODE(t)			(((t) / 1000 - 100) / 50)
> +#define AS_TIMER_CODE_TO_US(c)			((50 * (c) + 100) * 1000)
> +
> +/* Register definitions */
> +
> +/* Read-only Design info register: Reset state: xxxx 0001 */
> +#define AS_DESIGN_INFO_REG			0x00
> +#define AS_DESIGN_INFO_FACTORY(x)		(((x) >> 4))
> +#define AS_DESIGN_INFO_MODEL(x)			((x) & 0x0f)
> +
> +/* Read-only Version control register: Reset state: 0000 0000
> + * for first engineering samples
> + */
> +#define AS_VERSION_CONTROL_REG			0x01
> +#define AS_VERSION_CONTROL_RFU(x)		(((x) >> 4))
> +#define AS_VERSION_CONTROL_VERSION(x)		((x) & 0x0f)
> +
> +/* Read / Write	(Indicator and timer register): Reset state: 0000 1111 */
> +#define AS_INDICATOR_AND_TIMER_REG		0x02
> +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT	0
> +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT	4
> +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT	6
> +
> +/* Read / Write	(Current set register): Reset state: 0110 1001 */
> +#define AS_CURRENT_SET_REG			0x03
> +#define AS_CURRENT_ASSIST_LIGHT_SHIFT		0
> +#define AS_CURRENT_LED_DET_ON			(1 << 3)
> +#define AS_CURRENT_FLASH_CURRENT_SHIFT		4
> +
> +/* Read / Write	(Control register): Reset state: 1011 0100 */
> +#define AS_CONTROL_REG				0x04
> +#define AS_CONTROL_MODE_SETTING_SHIFT		0
> +#define AS_CONTROL_STROBE_ON			(1 << 2)
> +#define AS_CONTROL_OUT_ON			(1 << 3)
> +#define AS_CONTROL_EXT_TORCH_ON			(1 << 4)
> +#define AS_CONTROL_STROBE_TYPE_EDGE		(0 << 5)
> +#define AS_CONTROL_STROBE_TYPE_LEVEL		(1 << 5)
> +#define AS_CONTROL_COIL_PEAK_SHIFT		6
> +
> +/* Read only (D3 is read / write) (Fault and info): Reset state: 0000 x000 */
> +#define AS_FAULT_INFO_REG			0x05
> +#define AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT	(1 << 1)
> +#define AS_FAULT_INFO_INDICATOR_LED		(1 << 2)
> +#define AS_FAULT_INFO_LED_AMOUNT		(1 << 3)
> +#define AS_FAULT_INFO_TIMEOUT			(1 << 4)
> +#define AS_FAULT_INFO_OVER_TEMPERATURE		(1 << 5)
> +#define AS_FAULT_INFO_SHORT_CIRCUIT		(1 << 6)
> +#define AS_FAULT_INFO_OVER_VOLTAGE		(1 << 7)
> +
> +/* Boost register */
> +#define AS_BOOST_REG				0x0d
> +#define AS_BOOST_CURRENT_DISABLE		(0 << 0)
> +#define AS_BOOST_CURRENT_ENABLE			(1 << 0)
> +
> +/* Password register is used to unlock boost register writing */
> +#define AS_PASSWORD_REG				0x0f
> +#define AS_PASSWORD_UNLOCK_VALUE		0x55
> +
> +#define AS_NAME					"as3645a"
> +#define AS_I2C_ADDR				(0x60 >> 1) /* W:0x60, R:0x61 */
> +
> +#define AS_FLASH_TIMEOUT_MIN			100000	/* us */
> +#define AS_FLASH_TIMEOUT_MAX			850000
> +#define AS_FLASH_TIMEOUT_STEP			50000
> +
> +#define AS_FLASH_INTENSITY_MIN			200000	/* uA */
> +#define AS_FLASH_INTENSITY_MAX_1LED		500000
> +#define AS_FLASH_INTENSITY_MAX_2LEDS		400000
> +#define AS_FLASH_INTENSITY_STEP			20000
> +
> +#define AS_TORCH_INTENSITY_MIN			20000	/* uA */
> +#define AS_TORCH_INTENSITY_MAX			160000
> +#define AS_TORCH_INTENSITY_STEP			20000
> +
> +#define AS_INDICATOR_INTENSITY_MIN		0	/* uA */
> +#define AS_INDICATOR_INTENSITY_MAX		10000
> +#define AS_INDICATOR_INTENSITY_STEP		2500
> +
> +#define AS_PEAK_mA_MAX				2000
> +#define AS_PEAK_mA_TO_REG(a) \
> +	((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
> +
> +enum as_mode {
> +	AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
> +	AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,
> +	AS_MODE_ASSIST = 2 << AS_CONTROL_MODE_SETTING_SHIFT,
> +	AS_MODE_FLASH = 3 << AS_CONTROL_MODE_SETTING_SHIFT,
> +};
> +
> +struct as3645a_config {
> +	u32 flash_timeout_us;
> +	u32 flash_max_ua;
> +	u32 assist_max_ua;
> +	u32 indicator_max_ua;
> +	u32 voltage_reference;
> +	u32 peak;
> +};
> +
> +struct as3645a_names {
> +	char flash[32];
> +	char indicator[32];
> +};
> +
> +struct as3645a {
> +	struct i2c_client *client;
> +
> +	struct mutex mutex;
> +
> +	struct led_classdev_flash fled;
> +	struct led_classdev iled_cdev;
> +
> +	struct v4l2_flash *vf;
> +	struct v4l2_flash *vfind;
> +
> +	struct device_node *flash_node;
> +	struct device_node *indicator_node;
> +
> +	struct as3645a_config cfg;
> +
> +	enum as_mode mode;
> +	unsigned int timeout;
> +	unsigned int flash_current;
> +	unsigned int assist_current;
> +	unsigned int indicator_current;
> +	enum v4l2_flash_strobe_source strobe_source;
> +};
> +
> +#define fled_to_as3645a(__fled) container_of(__fled, struct as3645a, fled)
> +#define iled_cdev_to_as3645a(__iled_cdev) \
> +	container_of(__iled_cdev, struct as3645a, iled_cdev)
> +
> +/* Return negative errno else zero on success */
> +static int as3645a_write(struct as3645a *flash, u8 addr, u8 val)
> +{
> +	struct i2c_client *client = flash->client;
> +	int rval;
> +
> +	rval = i2c_smbus_write_byte_data(client, addr, val);
> +
> +	dev_dbg(&client->dev, "Write Addr:%02X Val:%02X %s\n", addr, val,
> +		rval < 0 ? "fail" : "ok");
> +
> +	return rval;
> +}
> +
> +/* Return negative errno else a data byte received from the device. */
> +static int as3645a_read(struct as3645a *flash, u8 addr)
> +{
> +	struct i2c_client *client = flash->client;
> +	int rval;
> +
> +	rval = i2c_smbus_read_byte_data(client, addr);
> +
> +	dev_dbg(&client->dev, "Read Addr:%02X Val:%02X %s\n", addr, rval,
> +		rval < 0 ? "fail" : "ok");
> +
> +	return rval;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Hardware configuration and trigger
> + */
> +
> +/**
> + * as3645a_set_config - Set flash configuration registers
> + * @flash: The flash
> + *
> + * Configure the hardware with flash, assist and indicator currents, as well as
> + * flash timeout.
> + *
> + * Return 0 on success, or a negative error code if an I2C communication error
> + * occurred.
> + */
> +static int as3645a_set_current(struct as3645a *flash)
> +{
> +	u8 val;
> +
> +	val = (flash->flash_current << AS_CURRENT_FLASH_CURRENT_SHIFT)
> +	    | (flash->assist_current << AS_CURRENT_ASSIST_LIGHT_SHIFT)
> +	    | AS_CURRENT_LED_DET_ON;
> +
> +	return as3645a_write(flash, AS_CURRENT_SET_REG, val);
> +}
> +
> +static int as3645a_set_timeout(struct as3645a *flash)
> +{
> +	u8 val;
> +
> +	val = flash->timeout << AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT;
> +
> +	val |= (flash->cfg.voltage_reference
> +		<< AS_INDICATOR_AND_TIMER_VREF_SHIFT)
> +	    |  ((flash->indicator_current ? flash->indicator_current - 1 : 0)
> +		 << AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT);
> +
> +	return as3645a_write(flash, AS_INDICATOR_AND_TIMER_REG, val);
> +}
> +
> +/**
> + * as3645a_set_control - Set flash control register
> + * @flash: The flash
> + * @mode: Desired output mode
> + * @on: Desired output state
> + *
> + * Configure the hardware with output mode and state.
> + *
> + * Return 0 on success, or a negative error code if an I2C communication error
> + * occurred.
> + */
> +static int
> +as3645a_set_control(struct as3645a *flash, enum as_mode mode, bool on)
> +{
> +	u8 reg;
> +
> +	/* Configure output parameters and operation mode. */
> +	reg = (flash->cfg.peak << AS_CONTROL_COIL_PEAK_SHIFT)
> +	    | (on ? AS_CONTROL_OUT_ON : 0)
> +	    | mode;
> +
> +	if (mode == AS_MODE_FLASH &&
> +	    flash->strobe_source == V4L2_FLASH_STROBE_SOURCE_EXTERNAL)
> +		reg |= AS_CONTROL_STROBE_TYPE_LEVEL
> +		    |  AS_CONTROL_STROBE_ON;
> +
> +	return as3645a_write(flash, AS_CONTROL_REG, reg);
> +}
> +
> +static int as3645a_get_fault(struct led_classdev_flash *fled, u32 *fault)
> +{
> +	struct as3645a *flash = fled_to_as3645a(fled);
> +	int rval;
> +
> +	/* NOTE: reading register clears fault status */
> +	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
> +	if (rval < 0)
> +		return rval;
> +
> +	if (rval & AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT)
> +		*fault |= LED_FAULT_OVER_CURRENT;
> +
> +	if (rval & AS_FAULT_INFO_INDICATOR_LED)
> +		*fault |= LED_FAULT_INDICATOR;
> +
> +	dev_dbg(&flash->client->dev, "%u connected LEDs\n",
> +		rval & AS_FAULT_INFO_LED_AMOUNT ? 2 : 1);
> +
> +	if (rval & AS_FAULT_INFO_TIMEOUT)
> +		*fault |= LED_FAULT_TIMEOUT;
> +
> +	if (rval & AS_FAULT_INFO_OVER_TEMPERATURE)
> +		*fault |= LED_FAULT_OVER_TEMPERATURE;
> +
> +	if (rval & AS_FAULT_INFO_SHORT_CIRCUIT)
> +		*fault |= LED_FAULT_OVER_CURRENT;
> +
> +	if (rval & AS_FAULT_INFO_OVER_VOLTAGE)
> +		*fault |= LED_FAULT_INPUT_VOLTAGE;
> +
> +	return rval;
> +}
> +
> +static unsigned int __as3645a_current_to_reg(unsigned int min, unsigned int max,
> +					     unsigned int step,
> +					     unsigned int val)
> +{
> +	if (val < min)
> +		val = min;
> +
> +	if (val > max)
> +		val = max;
> +
> +	return (val - min) / step;
> +}
> +
> +static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool is_flash,
> +					   unsigned int ua)
> +{
> +	if (is_flash)
> +		return __as3645a_current_to_reg(AS_TORCH_INTENSITY_MIN,
> +						flash->cfg.assist_max_ua,
> +						AS_TORCH_INTENSITY_STEP, ua);
> +	else
> +		return __as3645a_current_to_reg(AS_FLASH_INTENSITY_MIN,
> +						flash->cfg.flash_max_ua,
> +						AS_FLASH_INTENSITY_STEP, ua);
> +}
> +
> +static int as3645a_set_indicator_brightness(struct led_classdev *iled_cdev,
> +					    enum led_brightness brightness)
> +{
> +	struct as3645a *flash = iled_cdev_to_as3645a(iled_cdev);
> +	int rval;
> +
> +	flash->indicator_current = brightness;
> +
> +	rval = as3645a_set_timeout(flash);
> +	if (rval)
> +		return rval;
> +
> +	return as3645a_set_control(flash, AS_MODE_INDICATOR, brightness);
> +}
> +
> +static int as3645a_set_assist_brightness(struct led_classdev *fled_cdev,
> +					 enum led_brightness brightness)
> +{
> +	struct led_classdev_flash *fled = lcdev_to_flcdev(fled_cdev);
> +	struct as3645a *flash = fled_to_as3645a(fled);
> +	int rval;
> +
> +	if (brightness) {
> +		/* Register value 0 is 20 mA. */
> +		flash->assist_current = brightness - 1;
> +
> +		rval = as3645a_set_current(flash);
> +		if (rval)
> +			return rval;
> +	}
> +
> +	return as3645a_set_control(flash, AS_MODE_ASSIST, brightness);
> +}
> +
> +static int as3645a_set_flash_brightness(struct led_classdev_flash *fled,
> +					u32 brightness_ua)
> +{
> +	struct as3645a *flash = fled_to_as3645a(fled);
> +
> +	flash->flash_current = as3645a_current_to_reg(flash, true, brightness_ua);
> +
> +	return as3645a_set_current(flash);
> +}
> +
> +static int as3645a_set_flash_timeout(struct led_classdev_flash *fled,
> +				     u32 timeout_us)
> +{
> +	struct as3645a *flash = fled_to_as3645a(fled);
> +
> +	flash->timeout = AS_TIMER_US_TO_CODE(timeout_us);
> +
> +	return as3645a_set_timeout(flash);
> +}
> +
> +static int as3645a_set_strobe(struct led_classdev_flash *fled, bool state)
> +{
> +	struct as3645a *flash = fled_to_as3645a(fled);
> +
> +	return as3645a_set_control(flash, AS_MODE_FLASH, state);
> +}
> +
> +static const struct led_flash_ops as3645a_led_flash_ops = {
> +	.flash_brightness_set = as3645a_set_flash_brightness,
> +	.timeout_set = as3645a_set_flash_timeout,
> +	.strobe_set = as3645a_set_strobe,
> +	.fault_get = as3645a_get_fault,
> +};
> +
> +static int as3645a_setup(struct as3645a *flash)
> +{
> +	struct device *dev = &flash->client->dev;
> +	u32 fault = 0;
> +	int rval;
> +
> +	/* clear errors */
> +	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
> +	if (rval < 0)
> +		return rval;
> +
> +	dev_dbg(dev, "Fault info: %02x\n", rval);
> +
> +	rval = as3645a_set_current(flash);
> +	if (rval < 0)
> +		return rval;
> +
> +	rval = as3645a_set_timeout(flash);
> +	if (rval < 0)
> +		return rval;
> +
> +	rval = as3645a_set_control(flash, AS_MODE_INDICATOR, false);
> +	if (rval < 0)
> +		return rval;
> +
> +	/* read status */
> +	rval = as3645a_get_fault(&flash->fled, &fault);
> +	if (rval < 0)
> +		return rval;
> +
> +	dev_dbg(dev, "AS_INDICATOR_AND_TIMER_REG: %02x\n",
> +		as3645a_read(flash, AS_INDICATOR_AND_TIMER_REG));
> +	dev_dbg(dev, "AS_CURRENT_SET_REG: %02x\n",
> +		as3645a_read(flash, AS_CURRENT_SET_REG));
> +	dev_dbg(dev, "AS_CONTROL_REG: %02x\n",
> +		as3645a_read(flash, AS_CONTROL_REG));
> +
> +	return rval & ~AS_FAULT_INFO_LED_AMOUNT ? -EIO : 0;
> +}
> +
> +static int as3645a_detect(struct as3645a *flash)
> +{
> +	struct device *dev = &flash->client->dev;
> +	int rval, man, model, rfu, version;
> +	const char *vendor;
> +
> +	rval = as3645a_read(flash, AS_DESIGN_INFO_REG);
> +	if (rval < 0) {
> +		dev_err(dev, "can't read design info reg\n");
> +		return rval;
> +	}
> +
> +	man = AS_DESIGN_INFO_FACTORY(rval);
> +	model = AS_DESIGN_INFO_MODEL(rval);
> +
> +	rval = as3645a_read(flash, AS_VERSION_CONTROL_REG);
> +	if (rval < 0) {
> +		dev_err(dev, "can't read version control reg\n");
> +		return rval;
> +	}
> +
> +	rfu = AS_VERSION_CONTROL_RFU(rval);
> +	version = AS_VERSION_CONTROL_VERSION(rval);
> +
> +	/* Verify the chip model and version. */
> +	if (model != 0x01 || rfu != 0x00) {
> +		dev_err(dev, "AS3645A not detected "
> +			"(model %d rfu %d)\n", model, rfu);
> +		return -ENODEV;
> +	}
> +
> +	switch (man) {
> +	case 1:
> +		vendor = "AMS, Austria Micro Systems";
> +		break;
> +	case 2:
> +		vendor = "ADI, Analog Devices Inc.";
> +		break;
> +	case 3:
> +		vendor = "NSC, National Semiconductor";
> +		break;
> +	case 4:
> +		vendor = "NXP";
> +		break;
> +	case 5:
> +		vendor = "TI, Texas Instrument";
> +		break;
> +	default:
> +		vendor = "Unknown";
> +	}
> +
> +	dev_info(dev, "Chip vendor: %s (%d) Version: %d\n", vendor,
> +		 man, version);
> +
> +	rval = as3645a_write(flash, AS_PASSWORD_REG, AS_PASSWORD_UNLOCK_VALUE);
> +	if (rval < 0)
> +		return rval;
> +
> +	return as3645a_write(flash, AS_BOOST_REG, AS_BOOST_CURRENT_DISABLE);
> +}
> +
> +static int as3645a_parse_node(struct as3645a *flash,
> +			      struct as3645a_names *names,
> +			      struct device_node *node)
> +{
> +	struct as3645a_config *cfg = &flash->cfg;
> +	const char *name;
> +	int rval;
> +
> +	flash->flash_node = of_get_child_by_name(node, "flash");
> +	if (!flash->flash_node) {
> +		dev_err(&flash->client->dev, "can't find flash node\n");
> +		return -ENODEV;
> +	}
> +
> +	rval = of_property_read_string(flash->flash_node, "label", &name);
> +	if (!rval)
> +		strlcpy(names->flash, name, sizeof(names->flash));
> +	else
> +		snprintf(names->flash, sizeof(names->flash),
> +			 "%s:flash", node->name);
> +
> +	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
> +				    &cfg->flash_timeout_us);
> +	if (rval < 0) {
> +		dev_err(&flash->client->dev,
> +			"can't read flash-timeout-us property for flash\n");
> +		goto out_err;
> +	}
> +
> +	rval = of_property_read_u32(flash->flash_node, "flash-max-microamp",
> +				    &cfg->flash_max_ua);
> +	if (rval < 0) {
> +		dev_err(&flash->client->dev,
> +			"can't read flash-max-microamp property for flash\n");
> +		goto out_err;
> +	}
> +
> +	rval = of_property_read_u32(flash->flash_node, "led-max-microamp",
> +				    &cfg->assist_max_ua);
> +	if (rval < 0) {
> +		dev_err(&flash->client->dev,
> +			"can't read led-max-microamp property for flash\n");
> +		goto out_err;
> +	}
> +
> +	of_property_read_u32(flash->flash_node, "voltage-reference",
> +			     &cfg->voltage_reference);
> +
> +	of_property_read_u32(flash->flash_node, "peak-current-limit",
> +			     &cfg->peak);
> +	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
> +
> +	flash->indicator_node = of_get_child_by_name(node, "indicator");
> +	if (!flash->indicator_node) {
> +		dev_warn(&flash->client->dev,
> +			 "can't find indicator node\n");
> +		goto out_err;
> +	}
> +
> +	rval = of_property_read_string(flash->indicator_node, "label", &name);
> +	if (!rval)
> +		strlcpy(names->indicator, name, sizeof(names->indicator));
> +	else
> +		snprintf(names->indicator, sizeof(names->indicator),
> +			 "%s:indicator", node->name);
> +
> +	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
> +				    &cfg->indicator_max_ua);
> +	if (rval < 0) {
> +		dev_err(&flash->client->dev,
> +			"can't read led-max-microamp property for indicator\n");
> +		goto out_err;
> +	}
> +
> +	return 0;
> +
> +out_err:
> +	of_node_put(flash->flash_node);
> +	of_node_put(flash->indicator_node);
> +
> +	return rval;
> +}
> +
> +static int as3645a_led_class_setup(struct as3645a *flash,
> +				   struct as3645a_names *names)
> +{
> +	struct led_classdev *fled_cdev = &flash->fled.led_cdev;
> +	struct led_classdev *iled_cdev = &flash->iled_cdev;
> +	struct led_flash_setting *cfg;
> +	int rval;
> +
> +	iled_cdev->name = names->indicator;
> +	iled_cdev->brightness_set_blocking = as3645a_set_indicator_brightness;
> +	iled_cdev->max_brightness =
> +		flash->cfg.indicator_max_ua / AS_INDICATOR_INTENSITY_STEP;
> +	iled_cdev->flags = LED_CORE_SUSPENDRESUME;
> +
> +	rval = led_classdev_register(&flash->client->dev, iled_cdev);
> +	if (rval < 0)
> +		return rval;
> +
> +	cfg = &flash->fled.brightness;
> +	cfg->min = AS_FLASH_INTENSITY_MIN;
> +	cfg->max = flash->cfg.flash_max_ua;
> +	cfg->step = AS_FLASH_INTENSITY_STEP;
> +	cfg->val = flash->cfg.flash_max_ua;
> +
> +	cfg = &flash->fled.timeout;
> +	cfg->min = AS_FLASH_TIMEOUT_MIN;
> +	cfg->max = flash->cfg.flash_timeout_us;
> +	cfg->step = AS_FLASH_TIMEOUT_STEP;
> +	cfg->val = flash->cfg.flash_timeout_us;
> +
> +	flash->fled.ops = &as3645a_led_flash_ops;
> +
> +	fled_cdev->name = names->flash;
> +	fled_cdev->brightness_set_blocking = as3645a_set_assist_brightness;
> +	/* Value 0 is off in LED class. */
> +	fled_cdev->max_brightness =
> +		as3645a_current_to_reg(flash, false,
> +				       flash->cfg.assist_max_ua) + 1;
> +	fled_cdev->flags = LED_DEV_CAP_FLASH | LED_CORE_SUSPENDRESUME;
> +
> +	rval = led_classdev_flash_register(&flash->client->dev, &flash->fled);
> +	if (rval) {
> +		led_classdev_unregister(iled_cdev);
> +		dev_err(&flash->client->dev,
> +			"led_classdev_flash_register() failed, error %d\n",
> +			rval);
> +	}
> +
> +	return rval;
> +}
> +
> +static int as3645a_v4l2_setup(struct as3645a *flash)
> +{
> +	struct led_classdev_flash *fled = &flash->fled;
> +	struct led_classdev *led = &fled->led_cdev;
> +	struct v4l2_flash_config cfg = {
> +		.intensity = {
> +			.min = AS_TORCH_INTENSITY_MIN,
> +			.max = flash->cfg.assist_max_ua,
> +			.step = AS_TORCH_INTENSITY_STEP,
> +			.val = flash->cfg.assist_max_ua,
> +		},
> +	};
> +	struct v4l2_flash_config cfgind = {
> +		.intensity = {
> +			.min = AS_INDICATOR_INTENSITY_MIN,
> +			.max = flash->cfg.indicator_max_ua,
> +			.step = AS_INDICATOR_INTENSITY_STEP,
> +			.val = flash->cfg.indicator_max_ua,
> +		},
> +	};
> +
> +	strlcpy(cfg.dev_name, led->name, sizeof(cfg.dev_name));
> +	strlcpy(cfgind.dev_name, flash->iled_cdev.name, sizeof(cfg.dev_name));
> +
> +	flash->vf = v4l2_flash_init(
> +		&flash->client->dev, of_fwnode_handle(flash->flash_node),
> +		&flash->fled, NULL, &cfg);
> +	if (IS_ERR(flash->vf))
> +		return PTR_ERR(flash->vf);
> +
> +	flash->vfind = v4l2_flash_indicator_init(
> +		&flash->client->dev, of_fwnode_handle(flash->indicator_node),
> +		&flash->iled_cdev, &cfgind);
> +	if (IS_ERR(flash->vfind)) {
> +		v4l2_flash_release(flash->vf);
> +		return PTR_ERR(flash->vfind);
> +	}
> +
> +	return 0;
> +}
> +
> +static int as3645a_probe(struct i2c_client *client)
> +{
> +	struct as3645a_names names;
> +	struct as3645a *flash;
> +	int rval;
> +
> +	if (client->dev.of_node == NULL)
> +		return -ENODEV;
> +
> +	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
> +	if (flash == NULL)
> +		return -ENOMEM;
> +
> +	flash->client = client;
> +
> +	rval = as3645a_parse_node(flash, &names, client->dev.of_node);
> +	if (rval < 0)
> +		return rval;
> +
> +	rval = as3645a_detect(flash);
> +	if (rval < 0)
> +		goto out_put_nodes;
> +
> +	mutex_init(&flash->mutex);
> +	i2c_set_clientdata(client, flash);
> +
> +	rval = as3645a_setup(flash);
> +	if (rval)
> +		goto out_mutex_destroy;
> +
> +	rval = as3645a_led_class_setup(flash, &names);
> +	if (rval)
> +		goto out_mutex_destroy;
> +
> +	rval = as3645a_v4l2_setup(flash);
> +	if (rval)
> +		goto out_led_classdev_flash_unregister;
> +
> +	return 0;
> +
> +out_led_classdev_flash_unregister:
> +	led_classdev_flash_unregister(&flash->fled);
> +
> +out_mutex_destroy:
> +	mutex_destroy(&flash->mutex);
> +
> +out_put_nodes:
> +	of_node_put(flash->flash_node);
> +	of_node_put(flash->indicator_node);
> +
> +	return rval;
> +}
> +
> +static int as3645a_remove(struct i2c_client *client)
> +{
> +	struct as3645a *flash = i2c_get_clientdata(client);
> +
> +	as3645a_set_control(flash, AS_MODE_EXT_TORCH, false);
> +
> +	v4l2_flash_release(flash->vf);
> +
> +	led_classdev_flash_unregister(&flash->fled);
> +	led_classdev_unregister(&flash->iled_cdev);
> +
> +	mutex_destroy(&flash->mutex);
> +
> +	of_node_put(flash->flash_node);
> +	of_node_put(flash->indicator_node);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id as3645a_id_table[] = {
> +	{ AS_NAME, 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, as3645a_id_table);
> +
> +static const struct of_device_id as3645a_of_table[] = {
> +	{ .compatible = "ams,as3645a" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, as3645a_of_table);
> +
> +static struct i2c_driver as3645a_i2c_driver = {
> +	.driver	= {
> +		.of_match_table = as3645a_of_table,
> +		.name = AS_NAME,
> +	},
> +	.probe_new	= as3645a_probe,
> +	.remove	= as3645a_remove,
> +	.id_table = as3645a_id_table,
> +};
> +
> +module_i2c_driver(as3645a_i2c_driver);
> +
> +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
> +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@iki.fi>");
> +MODULE_DESCRIPTION("LED flash driver for AS3645A, LM3555 and their clones");
> +MODULE_LICENSE("GPL v2");
> 

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

* Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
  2017-08-20 10:09     ` Jacek Anaszewski
@ 2017-08-20 10:12       ` Jacek Anaszewski
  2017-08-21 13:53       ` Sakari Ailus
  1 sibling, 0 replies; 17+ messages in thread
From: Jacek Anaszewski @ 2017-08-20 10:12 UTC (permalink / raw)
  To: Sakari Ailus, linux-media; +Cc: javier, linux-leds, devicetree, Sakari Ailus

Forgot to add a link to the referenced patch:

[0] http://www.spinics.net/lists/devicetree/msg191273.html

On 08/20/2017 12:09 PM, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the update.
> I've noticed that you added node labels to the child device nodes
> in [0]:
> 
> "as3645a_flash : flash" and "as3645a_indicator : indicator"
> 
> I am still seeing problems with this approach:
> 
> 1) AFAIK these labels are only used for referencing nodes inside dts
>    files and they don't affect the name property of struct device_node
> 2) Even if you changed the node name from flash to as3645a_flash, you
>    would get weird LED class device name "as3645a_flash:flash" in case
>    label property is absent. Do you have any objections against the
>    approach I proposed in the previous review?:
> 
> 
>     snprintf(names->flash, sizeof(names->flash),
> 	     AS_NAME":%s", node->name);
> 
> Best regards,
> Jacek Anaszewski
> 
> On 08/19/2017 11:42 PM, Sakari Ailus wrote:
>> From: Sakari Ailus <sakari.ailus@iki.fi>
>>
>> Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
>> driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
>> is based on that.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>> Well. The driver does not control explicit power resources nor it used
>> runtime PM. Remove references to it.
>>
>>  MAINTAINERS                 |   6 +
>>  drivers/leds/Kconfig        |   8 +
>>  drivers/leds/Makefile       |   1 +
>>  drivers/leds/leds-as3645a.c | 763 ++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 778 insertions(+)
>>  create mode 100644 drivers/leds/leds-as3645a.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 931abca006b7..8f40ba2e5303 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2124,6 +2124,12 @@ F:	arch/arm64/
>>  F:	Documentation/arm64/
>>  
>>  AS3645A LED FLASH CONTROLLER DRIVER
>> +M:	Sakari Ailus <sakari.ailus@iki.fi>
>> +L:	linux-leds@vger.kernel.org
>> +S:	Maintained
>> +F:	drivers/leds/leds-as3645a.c
>> +
>> +AS3645A LED FLASH CONTROLLER DRIVER
>>  M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>  L:	linux-media@vger.kernel.org
>>  T:	git git://linuxtv.org/media_tree.git
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 594b24d410c3..bad3a4098104 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -58,6 +58,14 @@ config LEDS_AAT1290
>>  	help
>>  	 This option enables support for the LEDs on the AAT1290.
>>  
>> +config LEDS_AS3645A
>> +	tristate "AS3645A LED flash controller support"
>> +	depends on I2C && LEDS_CLASS_FLASH
>> +	help
>> +	  Enable LED flash class support for AS3645A LED flash
>> +	  controller. V4L2 flash API is provided as well if
>> +	  CONFIG_V4L2_FLASH_API is enabled.
>> +
>>  config LEDS_BCM6328
>>  	tristate "LED Support for Broadcom BCM6328"
>>  	depends on LEDS_CLASS
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index 909dae62ba05..7d7b26552923 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -8,6 +8,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_AS3645A)		+= leds-as3645a.o
>>  obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
>>  obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
>>  obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
>> diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
>> new file mode 100644
>> index 000000000000..bbbbe0898233
>> --- /dev/null
>> +++ b/drivers/leds/leds-as3645a.c
>> @@ -0,0 +1,763 @@
>> +/*
>> + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver
>> + *
>> + * Copyright (C) 2008-2011 Nokia Corporation
>> + * Copyright (c) 2011, 2017 Intel Corporation.
>> + *
>> + * Based on drivers/media/i2c/as3645a.c.
>> + *
>> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.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/slab.h>
>> +
>> +#include <media/v4l2-flash-led-class.h>
>> +
>> +#define AS_TIMER_US_TO_CODE(t)			(((t) / 1000 - 100) / 50)
>> +#define AS_TIMER_CODE_TO_US(c)			((50 * (c) + 100) * 1000)
>> +
>> +/* Register definitions */
>> +
>> +/* Read-only Design info register: Reset state: xxxx 0001 */
>> +#define AS_DESIGN_INFO_REG			0x00
>> +#define AS_DESIGN_INFO_FACTORY(x)		(((x) >> 4))
>> +#define AS_DESIGN_INFO_MODEL(x)			((x) & 0x0f)
>> +
>> +/* Read-only Version control register: Reset state: 0000 0000
>> + * for first engineering samples
>> + */
>> +#define AS_VERSION_CONTROL_REG			0x01
>> +#define AS_VERSION_CONTROL_RFU(x)		(((x) >> 4))
>> +#define AS_VERSION_CONTROL_VERSION(x)		((x) & 0x0f)
>> +
>> +/* Read / Write	(Indicator and timer register): Reset state: 0000 1111 */
>> +#define AS_INDICATOR_AND_TIMER_REG		0x02
>> +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT	0
>> +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT	4
>> +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT	6
>> +
>> +/* Read / Write	(Current set register): Reset state: 0110 1001 */
>> +#define AS_CURRENT_SET_REG			0x03
>> +#define AS_CURRENT_ASSIST_LIGHT_SHIFT		0
>> +#define AS_CURRENT_LED_DET_ON			(1 << 3)
>> +#define AS_CURRENT_FLASH_CURRENT_SHIFT		4
>> +
>> +/* Read / Write	(Control register): Reset state: 1011 0100 */
>> +#define AS_CONTROL_REG				0x04
>> +#define AS_CONTROL_MODE_SETTING_SHIFT		0
>> +#define AS_CONTROL_STROBE_ON			(1 << 2)
>> +#define AS_CONTROL_OUT_ON			(1 << 3)
>> +#define AS_CONTROL_EXT_TORCH_ON			(1 << 4)
>> +#define AS_CONTROL_STROBE_TYPE_EDGE		(0 << 5)
>> +#define AS_CONTROL_STROBE_TYPE_LEVEL		(1 << 5)
>> +#define AS_CONTROL_COIL_PEAK_SHIFT		6
>> +
>> +/* Read only (D3 is read / write) (Fault and info): Reset state: 0000 x000 */
>> +#define AS_FAULT_INFO_REG			0x05
>> +#define AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT	(1 << 1)
>> +#define AS_FAULT_INFO_INDICATOR_LED		(1 << 2)
>> +#define AS_FAULT_INFO_LED_AMOUNT		(1 << 3)
>> +#define AS_FAULT_INFO_TIMEOUT			(1 << 4)
>> +#define AS_FAULT_INFO_OVER_TEMPERATURE		(1 << 5)
>> +#define AS_FAULT_INFO_SHORT_CIRCUIT		(1 << 6)
>> +#define AS_FAULT_INFO_OVER_VOLTAGE		(1 << 7)
>> +
>> +/* Boost register */
>> +#define AS_BOOST_REG				0x0d
>> +#define AS_BOOST_CURRENT_DISABLE		(0 << 0)
>> +#define AS_BOOST_CURRENT_ENABLE			(1 << 0)
>> +
>> +/* Password register is used to unlock boost register writing */
>> +#define AS_PASSWORD_REG				0x0f
>> +#define AS_PASSWORD_UNLOCK_VALUE		0x55
>> +
>> +#define AS_NAME					"as3645a"
>> +#define AS_I2C_ADDR				(0x60 >> 1) /* W:0x60, R:0x61 */
>> +
>> +#define AS_FLASH_TIMEOUT_MIN			100000	/* us */
>> +#define AS_FLASH_TIMEOUT_MAX			850000
>> +#define AS_FLASH_TIMEOUT_STEP			50000
>> +
>> +#define AS_FLASH_INTENSITY_MIN			200000	/* uA */
>> +#define AS_FLASH_INTENSITY_MAX_1LED		500000
>> +#define AS_FLASH_INTENSITY_MAX_2LEDS		400000
>> +#define AS_FLASH_INTENSITY_STEP			20000
>> +
>> +#define AS_TORCH_INTENSITY_MIN			20000	/* uA */
>> +#define AS_TORCH_INTENSITY_MAX			160000
>> +#define AS_TORCH_INTENSITY_STEP			20000
>> +
>> +#define AS_INDICATOR_INTENSITY_MIN		0	/* uA */
>> +#define AS_INDICATOR_INTENSITY_MAX		10000
>> +#define AS_INDICATOR_INTENSITY_STEP		2500
>> +
>> +#define AS_PEAK_mA_MAX				2000
>> +#define AS_PEAK_mA_TO_REG(a) \
>> +	((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
>> +
>> +enum as_mode {
>> +	AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
>> +	AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,
>> +	AS_MODE_ASSIST = 2 << AS_CONTROL_MODE_SETTING_SHIFT,
>> +	AS_MODE_FLASH = 3 << AS_CONTROL_MODE_SETTING_SHIFT,
>> +};
>> +
>> +struct as3645a_config {
>> +	u32 flash_timeout_us;
>> +	u32 flash_max_ua;
>> +	u32 assist_max_ua;
>> +	u32 indicator_max_ua;
>> +	u32 voltage_reference;
>> +	u32 peak;
>> +};
>> +
>> +struct as3645a_names {
>> +	char flash[32];
>> +	char indicator[32];
>> +};
>> +
>> +struct as3645a {
>> +	struct i2c_client *client;
>> +
>> +	struct mutex mutex;
>> +
>> +	struct led_classdev_flash fled;
>> +	struct led_classdev iled_cdev;
>> +
>> +	struct v4l2_flash *vf;
>> +	struct v4l2_flash *vfind;
>> +
>> +	struct device_node *flash_node;
>> +	struct device_node *indicator_node;
>> +
>> +	struct as3645a_config cfg;
>> +
>> +	enum as_mode mode;
>> +	unsigned int timeout;
>> +	unsigned int flash_current;
>> +	unsigned int assist_current;
>> +	unsigned int indicator_current;
>> +	enum v4l2_flash_strobe_source strobe_source;
>> +};
>> +
>> +#define fled_to_as3645a(__fled) container_of(__fled, struct as3645a, fled)
>> +#define iled_cdev_to_as3645a(__iled_cdev) \
>> +	container_of(__iled_cdev, struct as3645a, iled_cdev)
>> +
>> +/* Return negative errno else zero on success */
>> +static int as3645a_write(struct as3645a *flash, u8 addr, u8 val)
>> +{
>> +	struct i2c_client *client = flash->client;
>> +	int rval;
>> +
>> +	rval = i2c_smbus_write_byte_data(client, addr, val);
>> +
>> +	dev_dbg(&client->dev, "Write Addr:%02X Val:%02X %s\n", addr, val,
>> +		rval < 0 ? "fail" : "ok");
>> +
>> +	return rval;
>> +}
>> +
>> +/* Return negative errno else a data byte received from the device. */
>> +static int as3645a_read(struct as3645a *flash, u8 addr)
>> +{
>> +	struct i2c_client *client = flash->client;
>> +	int rval;
>> +
>> +	rval = i2c_smbus_read_byte_data(client, addr);
>> +
>> +	dev_dbg(&client->dev, "Read Addr:%02X Val:%02X %s\n", addr, rval,
>> +		rval < 0 ? "fail" : "ok");
>> +
>> +	return rval;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Hardware configuration and trigger
>> + */
>> +
>> +/**
>> + * as3645a_set_config - Set flash configuration registers
>> + * @flash: The flash
>> + *
>> + * Configure the hardware with flash, assist and indicator currents, as well as
>> + * flash timeout.
>> + *
>> + * Return 0 on success, or a negative error code if an I2C communication error
>> + * occurred.
>> + */
>> +static int as3645a_set_current(struct as3645a *flash)
>> +{
>> +	u8 val;
>> +
>> +	val = (flash->flash_current << AS_CURRENT_FLASH_CURRENT_SHIFT)
>> +	    | (flash->assist_current << AS_CURRENT_ASSIST_LIGHT_SHIFT)
>> +	    | AS_CURRENT_LED_DET_ON;
>> +
>> +	return as3645a_write(flash, AS_CURRENT_SET_REG, val);
>> +}
>> +
>> +static int as3645a_set_timeout(struct as3645a *flash)
>> +{
>> +	u8 val;
>> +
>> +	val = flash->timeout << AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT;
>> +
>> +	val |= (flash->cfg.voltage_reference
>> +		<< AS_INDICATOR_AND_TIMER_VREF_SHIFT)
>> +	    |  ((flash->indicator_current ? flash->indicator_current - 1 : 0)
>> +		 << AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT);
>> +
>> +	return as3645a_write(flash, AS_INDICATOR_AND_TIMER_REG, val);
>> +}
>> +
>> +/**
>> + * as3645a_set_control - Set flash control register
>> + * @flash: The flash
>> + * @mode: Desired output mode
>> + * @on: Desired output state
>> + *
>> + * Configure the hardware with output mode and state.
>> + *
>> + * Return 0 on success, or a negative error code if an I2C communication error
>> + * occurred.
>> + */
>> +static int
>> +as3645a_set_control(struct as3645a *flash, enum as_mode mode, bool on)
>> +{
>> +	u8 reg;
>> +
>> +	/* Configure output parameters and operation mode. */
>> +	reg = (flash->cfg.peak << AS_CONTROL_COIL_PEAK_SHIFT)
>> +	    | (on ? AS_CONTROL_OUT_ON : 0)
>> +	    | mode;
>> +
>> +	if (mode == AS_MODE_FLASH &&
>> +	    flash->strobe_source == V4L2_FLASH_STROBE_SOURCE_EXTERNAL)
>> +		reg |= AS_CONTROL_STROBE_TYPE_LEVEL
>> +		    |  AS_CONTROL_STROBE_ON;
>> +
>> +	return as3645a_write(flash, AS_CONTROL_REG, reg);
>> +}
>> +
>> +static int as3645a_get_fault(struct led_classdev_flash *fled, u32 *fault)
>> +{
>> +	struct as3645a *flash = fled_to_as3645a(fled);
>> +	int rval;
>> +
>> +	/* NOTE: reading register clears fault status */
>> +	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	if (rval & AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT)
>> +		*fault |= LED_FAULT_OVER_CURRENT;
>> +
>> +	if (rval & AS_FAULT_INFO_INDICATOR_LED)
>> +		*fault |= LED_FAULT_INDICATOR;
>> +
>> +	dev_dbg(&flash->client->dev, "%u connected LEDs\n",
>> +		rval & AS_FAULT_INFO_LED_AMOUNT ? 2 : 1);
>> +
>> +	if (rval & AS_FAULT_INFO_TIMEOUT)
>> +		*fault |= LED_FAULT_TIMEOUT;
>> +
>> +	if (rval & AS_FAULT_INFO_OVER_TEMPERATURE)
>> +		*fault |= LED_FAULT_OVER_TEMPERATURE;
>> +
>> +	if (rval & AS_FAULT_INFO_SHORT_CIRCUIT)
>> +		*fault |= LED_FAULT_OVER_CURRENT;
>> +
>> +	if (rval & AS_FAULT_INFO_OVER_VOLTAGE)
>> +		*fault |= LED_FAULT_INPUT_VOLTAGE;
>> +
>> +	return rval;
>> +}
>> +
>> +static unsigned int __as3645a_current_to_reg(unsigned int min, unsigned int max,
>> +					     unsigned int step,
>> +					     unsigned int val)
>> +{
>> +	if (val < min)
>> +		val = min;
>> +
>> +	if (val > max)
>> +		val = max;
>> +
>> +	return (val - min) / step;
>> +}
>> +
>> +static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool is_flash,
>> +					   unsigned int ua)
>> +{
>> +	if (is_flash)
>> +		return __as3645a_current_to_reg(AS_TORCH_INTENSITY_MIN,
>> +						flash->cfg.assist_max_ua,
>> +						AS_TORCH_INTENSITY_STEP, ua);
>> +	else
>> +		return __as3645a_current_to_reg(AS_FLASH_INTENSITY_MIN,
>> +						flash->cfg.flash_max_ua,
>> +						AS_FLASH_INTENSITY_STEP, ua);
>> +}
>> +
>> +static int as3645a_set_indicator_brightness(struct led_classdev *iled_cdev,
>> +					    enum led_brightness brightness)
>> +{
>> +	struct as3645a *flash = iled_cdev_to_as3645a(iled_cdev);
>> +	int rval;
>> +
>> +	flash->indicator_current = brightness;
>> +
>> +	rval = as3645a_set_timeout(flash);
>> +	if (rval)
>> +		return rval;
>> +
>> +	return as3645a_set_control(flash, AS_MODE_INDICATOR, brightness);
>> +}
>> +
>> +static int as3645a_set_assist_brightness(struct led_classdev *fled_cdev,
>> +					 enum led_brightness brightness)
>> +{
>> +	struct led_classdev_flash *fled = lcdev_to_flcdev(fled_cdev);
>> +	struct as3645a *flash = fled_to_as3645a(fled);
>> +	int rval;
>> +
>> +	if (brightness) {
>> +		/* Register value 0 is 20 mA. */
>> +		flash->assist_current = brightness - 1;
>> +
>> +		rval = as3645a_set_current(flash);
>> +		if (rval)
>> +			return rval;
>> +	}
>> +
>> +	return as3645a_set_control(flash, AS_MODE_ASSIST, brightness);
>> +}
>> +
>> +static int as3645a_set_flash_brightness(struct led_classdev_flash *fled,
>> +					u32 brightness_ua)
>> +{
>> +	struct as3645a *flash = fled_to_as3645a(fled);
>> +
>> +	flash->flash_current = as3645a_current_to_reg(flash, true, brightness_ua);
>> +
>> +	return as3645a_set_current(flash);
>> +}
>> +
>> +static int as3645a_set_flash_timeout(struct led_classdev_flash *fled,
>> +				     u32 timeout_us)
>> +{
>> +	struct as3645a *flash = fled_to_as3645a(fled);
>> +
>> +	flash->timeout = AS_TIMER_US_TO_CODE(timeout_us);
>> +
>> +	return as3645a_set_timeout(flash);
>> +}
>> +
>> +static int as3645a_set_strobe(struct led_classdev_flash *fled, bool state)
>> +{
>> +	struct as3645a *flash = fled_to_as3645a(fled);
>> +
>> +	return as3645a_set_control(flash, AS_MODE_FLASH, state);
>> +}
>> +
>> +static const struct led_flash_ops as3645a_led_flash_ops = {
>> +	.flash_brightness_set = as3645a_set_flash_brightness,
>> +	.timeout_set = as3645a_set_flash_timeout,
>> +	.strobe_set = as3645a_set_strobe,
>> +	.fault_get = as3645a_get_fault,
>> +};
>> +
>> +static int as3645a_setup(struct as3645a *flash)
>> +{
>> +	struct device *dev = &flash->client->dev;
>> +	u32 fault = 0;
>> +	int rval;
>> +
>> +	/* clear errors */
>> +	rval = as3645a_read(flash, AS_FAULT_INFO_REG);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	dev_dbg(dev, "Fault info: %02x\n", rval);
>> +
>> +	rval = as3645a_set_current(flash);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	rval = as3645a_set_timeout(flash);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	rval = as3645a_set_control(flash, AS_MODE_INDICATOR, false);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	/* read status */
>> +	rval = as3645a_get_fault(&flash->fled, &fault);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	dev_dbg(dev, "AS_INDICATOR_AND_TIMER_REG: %02x\n",
>> +		as3645a_read(flash, AS_INDICATOR_AND_TIMER_REG));
>> +	dev_dbg(dev, "AS_CURRENT_SET_REG: %02x\n",
>> +		as3645a_read(flash, AS_CURRENT_SET_REG));
>> +	dev_dbg(dev, "AS_CONTROL_REG: %02x\n",
>> +		as3645a_read(flash, AS_CONTROL_REG));
>> +
>> +	return rval & ~AS_FAULT_INFO_LED_AMOUNT ? -EIO : 0;
>> +}
>> +
>> +static int as3645a_detect(struct as3645a *flash)
>> +{
>> +	struct device *dev = &flash->client->dev;
>> +	int rval, man, model, rfu, version;
>> +	const char *vendor;
>> +
>> +	rval = as3645a_read(flash, AS_DESIGN_INFO_REG);
>> +	if (rval < 0) {
>> +		dev_err(dev, "can't read design info reg\n");
>> +		return rval;
>> +	}
>> +
>> +	man = AS_DESIGN_INFO_FACTORY(rval);
>> +	model = AS_DESIGN_INFO_MODEL(rval);
>> +
>> +	rval = as3645a_read(flash, AS_VERSION_CONTROL_REG);
>> +	if (rval < 0) {
>> +		dev_err(dev, "can't read version control reg\n");
>> +		return rval;
>> +	}
>> +
>> +	rfu = AS_VERSION_CONTROL_RFU(rval);
>> +	version = AS_VERSION_CONTROL_VERSION(rval);
>> +
>> +	/* Verify the chip model and version. */
>> +	if (model != 0x01 || rfu != 0x00) {
>> +		dev_err(dev, "AS3645A not detected "
>> +			"(model %d rfu %d)\n", model, rfu);
>> +		return -ENODEV;
>> +	}
>> +
>> +	switch (man) {
>> +	case 1:
>> +		vendor = "AMS, Austria Micro Systems";
>> +		break;
>> +	case 2:
>> +		vendor = "ADI, Analog Devices Inc.";
>> +		break;
>> +	case 3:
>> +		vendor = "NSC, National Semiconductor";
>> +		break;
>> +	case 4:
>> +		vendor = "NXP";
>> +		break;
>> +	case 5:
>> +		vendor = "TI, Texas Instrument";
>> +		break;
>> +	default:
>> +		vendor = "Unknown";
>> +	}
>> +
>> +	dev_info(dev, "Chip vendor: %s (%d) Version: %d\n", vendor,
>> +		 man, version);
>> +
>> +	rval = as3645a_write(flash, AS_PASSWORD_REG, AS_PASSWORD_UNLOCK_VALUE);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	return as3645a_write(flash, AS_BOOST_REG, AS_BOOST_CURRENT_DISABLE);
>> +}
>> +
>> +static int as3645a_parse_node(struct as3645a *flash,
>> +			      struct as3645a_names *names,
>> +			      struct device_node *node)
>> +{
>> +	struct as3645a_config *cfg = &flash->cfg;
>> +	const char *name;
>> +	int rval;
>> +
>> +	flash->flash_node = of_get_child_by_name(node, "flash");
>> +	if (!flash->flash_node) {
>> +		dev_err(&flash->client->dev, "can't find flash node\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	rval = of_property_read_string(flash->flash_node, "label", &name);
>> +	if (!rval)
>> +		strlcpy(names->flash, name, sizeof(names->flash));
>> +	else
>> +		snprintf(names->flash, sizeof(names->flash),
>> +			 "%s:flash", node->name);
>> +
>> +	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
>> +				    &cfg->flash_timeout_us);
>> +	if (rval < 0) {
>> +		dev_err(&flash->client->dev,
>> +			"can't read flash-timeout-us property for flash\n");
>> +		goto out_err;
>> +	}
>> +
>> +	rval = of_property_read_u32(flash->flash_node, "flash-max-microamp",
>> +				    &cfg->flash_max_ua);
>> +	if (rval < 0) {
>> +		dev_err(&flash->client->dev,
>> +			"can't read flash-max-microamp property for flash\n");
>> +		goto out_err;
>> +	}
>> +
>> +	rval = of_property_read_u32(flash->flash_node, "led-max-microamp",
>> +				    &cfg->assist_max_ua);
>> +	if (rval < 0) {
>> +		dev_err(&flash->client->dev,
>> +			"can't read led-max-microamp property for flash\n");
>> +		goto out_err;
>> +	}
>> +
>> +	of_property_read_u32(flash->flash_node, "voltage-reference",
>> +			     &cfg->voltage_reference);
>> +
>> +	of_property_read_u32(flash->flash_node, "peak-current-limit",
>> +			     &cfg->peak);
>> +	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
>> +
>> +	flash->indicator_node = of_get_child_by_name(node, "indicator");
>> +	if (!flash->indicator_node) {
>> +		dev_warn(&flash->client->dev,
>> +			 "can't find indicator node\n");
>> +		goto out_err;
>> +	}
>> +
>> +	rval = of_property_read_string(flash->indicator_node, "label", &name);
>> +	if (!rval)
>> +		strlcpy(names->indicator, name, sizeof(names->indicator));
>> +	else
>> +		snprintf(names->indicator, sizeof(names->indicator),
>> +			 "%s:indicator", node->name);
>> +
>> +	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
>> +				    &cfg->indicator_max_ua);
>> +	if (rval < 0) {
>> +		dev_err(&flash->client->dev,
>> +			"can't read led-max-microamp property for indicator\n");
>> +		goto out_err;
>> +	}
>> +
>> +	return 0;
>> +
>> +out_err:
>> +	of_node_put(flash->flash_node);
>> +	of_node_put(flash->indicator_node);
>> +
>> +	return rval;
>> +}
>> +
>> +static int as3645a_led_class_setup(struct as3645a *flash,
>> +				   struct as3645a_names *names)
>> +{
>> +	struct led_classdev *fled_cdev = &flash->fled.led_cdev;
>> +	struct led_classdev *iled_cdev = &flash->iled_cdev;
>> +	struct led_flash_setting *cfg;
>> +	int rval;
>> +
>> +	iled_cdev->name = names->indicator;
>> +	iled_cdev->brightness_set_blocking = as3645a_set_indicator_brightness;
>> +	iled_cdev->max_brightness =
>> +		flash->cfg.indicator_max_ua / AS_INDICATOR_INTENSITY_STEP;
>> +	iled_cdev->flags = LED_CORE_SUSPENDRESUME;
>> +
>> +	rval = led_classdev_register(&flash->client->dev, iled_cdev);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	cfg = &flash->fled.brightness;
>> +	cfg->min = AS_FLASH_INTENSITY_MIN;
>> +	cfg->max = flash->cfg.flash_max_ua;
>> +	cfg->step = AS_FLASH_INTENSITY_STEP;
>> +	cfg->val = flash->cfg.flash_max_ua;
>> +
>> +	cfg = &flash->fled.timeout;
>> +	cfg->min = AS_FLASH_TIMEOUT_MIN;
>> +	cfg->max = flash->cfg.flash_timeout_us;
>> +	cfg->step = AS_FLASH_TIMEOUT_STEP;
>> +	cfg->val = flash->cfg.flash_timeout_us;
>> +
>> +	flash->fled.ops = &as3645a_led_flash_ops;
>> +
>> +	fled_cdev->name = names->flash;
>> +	fled_cdev->brightness_set_blocking = as3645a_set_assist_brightness;
>> +	/* Value 0 is off in LED class. */
>> +	fled_cdev->max_brightness =
>> +		as3645a_current_to_reg(flash, false,
>> +				       flash->cfg.assist_max_ua) + 1;
>> +	fled_cdev->flags = LED_DEV_CAP_FLASH | LED_CORE_SUSPENDRESUME;
>> +
>> +	rval = led_classdev_flash_register(&flash->client->dev, &flash->fled);
>> +	if (rval) {
>> +		led_classdev_unregister(iled_cdev);
>> +		dev_err(&flash->client->dev,
>> +			"led_classdev_flash_register() failed, error %d\n",
>> +			rval);
>> +	}
>> +
>> +	return rval;
>> +}
>> +
>> +static int as3645a_v4l2_setup(struct as3645a *flash)
>> +{
>> +	struct led_classdev_flash *fled = &flash->fled;
>> +	struct led_classdev *led = &fled->led_cdev;
>> +	struct v4l2_flash_config cfg = {
>> +		.intensity = {
>> +			.min = AS_TORCH_INTENSITY_MIN,
>> +			.max = flash->cfg.assist_max_ua,
>> +			.step = AS_TORCH_INTENSITY_STEP,
>> +			.val = flash->cfg.assist_max_ua,
>> +		},
>> +	};
>> +	struct v4l2_flash_config cfgind = {
>> +		.intensity = {
>> +			.min = AS_INDICATOR_INTENSITY_MIN,
>> +			.max = flash->cfg.indicator_max_ua,
>> +			.step = AS_INDICATOR_INTENSITY_STEP,
>> +			.val = flash->cfg.indicator_max_ua,
>> +		},
>> +	};
>> +
>> +	strlcpy(cfg.dev_name, led->name, sizeof(cfg.dev_name));
>> +	strlcpy(cfgind.dev_name, flash->iled_cdev.name, sizeof(cfg.dev_name));
>> +
>> +	flash->vf = v4l2_flash_init(
>> +		&flash->client->dev, of_fwnode_handle(flash->flash_node),
>> +		&flash->fled, NULL, &cfg);
>> +	if (IS_ERR(flash->vf))
>> +		return PTR_ERR(flash->vf);
>> +
>> +	flash->vfind = v4l2_flash_indicator_init(
>> +		&flash->client->dev, of_fwnode_handle(flash->indicator_node),
>> +		&flash->iled_cdev, &cfgind);
>> +	if (IS_ERR(flash->vfind)) {
>> +		v4l2_flash_release(flash->vf);
>> +		return PTR_ERR(flash->vfind);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int as3645a_probe(struct i2c_client *client)
>> +{
>> +	struct as3645a_names names;
>> +	struct as3645a *flash;
>> +	int rval;
>> +
>> +	if (client->dev.of_node == NULL)
>> +		return -ENODEV;
>> +
>> +	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
>> +	if (flash == NULL)
>> +		return -ENOMEM;
>> +
>> +	flash->client = client;
>> +
>> +	rval = as3645a_parse_node(flash, &names, client->dev.of_node);
>> +	if (rval < 0)
>> +		return rval;
>> +
>> +	rval = as3645a_detect(flash);
>> +	if (rval < 0)
>> +		goto out_put_nodes;
>> +
>> +	mutex_init(&flash->mutex);
>> +	i2c_set_clientdata(client, flash);
>> +
>> +	rval = as3645a_setup(flash);
>> +	if (rval)
>> +		goto out_mutex_destroy;
>> +
>> +	rval = as3645a_led_class_setup(flash, &names);
>> +	if (rval)
>> +		goto out_mutex_destroy;
>> +
>> +	rval = as3645a_v4l2_setup(flash);
>> +	if (rval)
>> +		goto out_led_classdev_flash_unregister;
>> +
>> +	return 0;
>> +
>> +out_led_classdev_flash_unregister:
>> +	led_classdev_flash_unregister(&flash->fled);
>> +
>> +out_mutex_destroy:
>> +	mutex_destroy(&flash->mutex);
>> +
>> +out_put_nodes:
>> +	of_node_put(flash->flash_node);
>> +	of_node_put(flash->indicator_node);
>> +
>> +	return rval;
>> +}
>> +
>> +static int as3645a_remove(struct i2c_client *client)
>> +{
>> +	struct as3645a *flash = i2c_get_clientdata(client);
>> +
>> +	as3645a_set_control(flash, AS_MODE_EXT_TORCH, false);
>> +
>> +	v4l2_flash_release(flash->vf);
>> +
>> +	led_classdev_flash_unregister(&flash->fled);
>> +	led_classdev_unregister(&flash->iled_cdev);
>> +
>> +	mutex_destroy(&flash->mutex);
>> +
>> +	of_node_put(flash->flash_node);
>> +	of_node_put(flash->indicator_node);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id as3645a_id_table[] = {
>> +	{ AS_NAME, 0 },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, as3645a_id_table);
>> +
>> +static const struct of_device_id as3645a_of_table[] = {
>> +	{ .compatible = "ams,as3645a" },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, as3645a_of_table);
>> +
>> +static struct i2c_driver as3645a_i2c_driver = {
>> +	.driver	= {
>> +		.of_match_table = as3645a_of_table,
>> +		.name = AS_NAME,
>> +	},
>> +	.probe_new	= as3645a_probe,
>> +	.remove	= as3645a_remove,
>> +	.id_table = as3645a_id_table,
>> +};
>> +
>> +module_i2c_driver(as3645a_i2c_driver);
>> +
>> +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
>> +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@iki.fi>");
>> +MODULE_DESCRIPTION("LED flash driver for AS3645A, LM3555 and their clones");
>> +MODULE_LICENSE("GPL v2");
>>

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

* Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
  2017-08-20 10:09     ` Jacek Anaszewski
  2017-08-20 10:12       ` Jacek Anaszewski
@ 2017-08-21 13:53       ` Sakari Ailus
  2017-08-21 19:03         ` Jacek Anaszewski
  1 sibling, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2017-08-21 13:53 UTC (permalink / raw)
  To: Jacek Anaszewski, Sakari Ailus, linux-media
  Cc: javier, linux-leds, devicetree

Hi Jacek,

Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the update.
> I've noticed that you added node labels to the child device nodes
> in [0]:
> 
> "as3645a_flash : flash" and "as3645a_indicator : indicator"

The phandle references (as3645a_flash and as3645a_indicator) should
actually be moved to the patch adding the flash property to the sensor
device node. It doesn't do anything here, yet.

> 
> I am still seeing problems with this approach:
> 
> 1) AFAIK these labels are only used for referencing nodes inside dts
>    files and they don't affect the name property of struct device_node

That's right.

> 2) Even if you changed the node name from flash to as3645a_flash, you
>    would get weird LED class device name "as3645a_flash:flash" in case
>    label property is absent. Do you have any objections against the
>    approach I proposed in the previous review?:
> 
> 
>     snprintf(names->flash, sizeof(names->flash),
> 	     AS_NAME":%s", node->name);

In the current patch, the device node of the flash controller is used,
postfixed with colon and the name of the LED ("flash" or "indicator") if
no label is defined. In other words, with that DT source you'll have
"as3645a:flash" and "as3645a:indicator". So if you change the name of
the device node of the I²C device, that will be reflected in the label.

If a label exists, then the label is used as such.

I don't really have objections to what you're proposing as such but my
question is: is it useful? With that, the flash and indicator labels
will not come from DT if label properties are undefined. They'll always
be "as3645a:flash" and "as3645a:indicator", independently of the names
of the device nodes.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@iki.fi

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

* Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
  2017-08-21 13:53       ` Sakari Ailus
@ 2017-08-21 19:03         ` Jacek Anaszewski
  2017-08-21 20:59           ` Sakari Ailus
  0 siblings, 1 reply; 17+ messages in thread
From: Jacek Anaszewski @ 2017-08-21 19:03 UTC (permalink / raw)
  To: Sakari Ailus, Sakari Ailus, linux-media; +Cc: javier, linux-leds, devicetree

Hi Sakari,

On 08/21/2017 03:53 PM, Sakari Ailus wrote:
> Hi Jacek,
> 
> Jacek Anaszewski wrote:
>> Hi Sakari,
>>
>> Thanks for the update.
>> I've noticed that you added node labels to the child device nodes
>> in [0]:
>>
>> "as3645a_flash : flash" and "as3645a_indicator : indicator"
> 
> The phandle references (as3645a_flash and as3645a_indicator) should
> actually be moved to the patch adding the flash property to the sensor
> device node. It doesn't do anything here, yet.
> 
>>
>> I am still seeing problems with this approach:
>>
>> 1) AFAIK these labels are only used for referencing nodes inside dts
>>    files and they don't affect the name property of struct device_node
> 
> That's right.
> 
>> 2) Even if you changed the node name from flash to as3645a_flash, you
>>    would get weird LED class device name "as3645a_flash:flash" in case
>>    label property is absent. Do you have any objections against the
>>    approach I proposed in the previous review?:
>>
>>
>>     snprintf(names->flash, sizeof(names->flash),
>> 	     AS_NAME":%s", node->name);
> 
> In the current patch, the device node of the flash controller is used,
> postfixed with colon and the name of the LED ("flash" or "indicator") if
> no label is defined. In other words, with that DT source you'll have
> "as3645a:flash" and "as3645a:indicator". So if you change the name of
> the device node of the I²C device, that will be reflected in the label.
> 
> If a label exists, then the label is used as such.
> 
> I don't really have objections to what you're proposing as such but my
> question is: is it useful? With that, the flash and indicator labels
> will not come from DT if label properties are undefined. They'll always
> be "as3645a:flash" and "as3645a:indicator", independently of the names
> of the device nodes.
> 

Ah, indeed, the node->name is put in place of devicename segment and
the node points to the LED controller node. Neat approach, likely to
be adopted as a pattern from now on for all new LED class drivers.


For the patch going through media tree:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

-- 
Best regards,
Jacek Anaszewski

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

* [PATCH v2.1 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash
  2017-08-19 21:24 ` [PATCH v2 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash Sakari Ailus
@ 2017-08-21 19:57   ` Sakari Ailus
  0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2017-08-21 19:57 UTC (permalink / raw)
  To: linux-media
  Cc: linux-leds, devicetree, javier, jacek.anaszewski, Sakari Ailus

From: Sakari Ailus <sakari.ailus@iki.fi>

Add the as3645a flash controller to the DT source.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
since v3:

- Drop reference names from flash and indicator nodes. They were unused.

 arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index df3366fa5409..cb47ae79a5f9 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -265,6 +265,20 @@
 
 &i2c2 {
 	clock-frequency = <400000>;
+
+	as3645a@30 {
+		reg = <0x30>;
+		compatible = "ams,as3645a";
+		flash {
+			flash-timeout-us = <150000>;
+			flash-max-microamp = <320000>;
+			led-max-microamp = <60000>;
+			peak-current-limit = <1750000>;
+		};
+		indicator {
+			led-max-microamp = <10000>;
+		};
+	};
 };
 
 &i2c3 {
-- 
2.11.0

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

* Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
  2017-08-21 19:03         ` Jacek Anaszewski
@ 2017-08-21 20:59           ` Sakari Ailus
  0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2017-08-21 20:59 UTC (permalink / raw)
  To: Jacek Anaszewski, Sakari Ailus, linux-media
  Cc: javier, linux-leds, devicetree

Hi Jacek,

Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 08/21/2017 03:53 PM, Sakari Ailus wrote:
>> Hi Jacek,
>>
>> Jacek Anaszewski wrote:
>>> Hi Sakari,
>>>
>>> Thanks for the update.
>>> I've noticed that you added node labels to the child device nodes
>>> in [0]:
>>>
>>> "as3645a_flash : flash" and "as3645a_indicator : indicator"
>>
>> The phandle references (as3645a_flash and as3645a_indicator) should
>> actually be moved to the patch adding the flash property to the sensor
>> device node. It doesn't do anything here, yet.
>>
>>>
>>> I am still seeing problems with this approach:
>>>
>>> 1) AFAIK these labels are only used for referencing nodes inside dts
>>>    files and they don't affect the name property of struct device_node
>>
>> That's right.
>>
>>> 2) Even if you changed the node name from flash to as3645a_flash, you
>>>    would get weird LED class device name "as3645a_flash:flash" in case
>>>    label property is absent. Do you have any objections against the
>>>    approach I proposed in the previous review?:
>>>
>>>
>>>     snprintf(names->flash, sizeof(names->flash),
>>> 	     AS_NAME":%s", node->name);
>>
>> In the current patch, the device node of the flash controller is used,
>> postfixed with colon and the name of the LED ("flash" or "indicator") if
>> no label is defined. In other words, with that DT source you'll have
>> "as3645a:flash" and "as3645a:indicator". So if you change the name of
>> the device node of the I²C device, that will be reflected in the label.
>>
>> If a label exists, then the label is used as such.
>>
>> I don't really have objections to what you're proposing as such but my
>> question is: is it useful? With that, the flash and indicator labels
>> will not come from DT if label properties are undefined. They'll always
>> be "as3645a:flash" and "as3645a:indicator", independently of the names
>> of the device nodes.
>>
> 
> Ah, indeed, the node->name is put in place of devicename segment and
> the node points to the LED controller node. Neat approach, likely to
> be adopted as a pattern from now on for all new LED class drivers.
> 
> 
> For the patch going through media tree:
> 
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

Thanks!

I sent a new version of the DT source patch for N9 / N950; I'll proceed
to send a pull request tomorrow / the day after unless there are further
comments.

-- 
Regards,

Sakari Ailus
sakari.ailus@iki.fi

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

* Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
@ 2017-08-22 18:34   ` Jacek Anaszewski
  2017-08-23  0:28   ` Rob Herring
  2017-08-28 10:33   ` Pavel Machek
  2 siblings, 0 replies; 17+ messages in thread
From: Jacek Anaszewski @ 2017-08-22 18:34 UTC (permalink / raw)
  To: Sakari Ailus, linux-media; +Cc: javier, linux-leds, devicetree, Sakari Ailus

Hi Sakari,

Thanks for the update.

On 08/19/2017 11:24 PM, Sakari Ailus wrote:
> From: Sakari Ailus <sakari.ailus@iki.fi>
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../devicetree/bindings/leds/ams,as3645a.txt       | 71 ++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> new file mode 100644
> index 000000000000..12c5ef26ec73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> @@ -0,0 +1,71 @@
> +Analog devices AS3645A device tree bindings
> +
> +The AS3645A flash LED controller can drive two LEDs, one high current
> +flash LED and one indicator LED. The high current flash LED can be
> +used in torch mode as well.
> +
> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> +and b are included in the range.
> +
> +Please also see common.txt in the same directory.
> +
> +
> +Required properties
> +===================
> +
> +compatible	: Must be "ams,as3645a".
> +reg		: The I2C address of the device. Typically 0x30.
> +
> +
> +Required properties of the "flash" child node
> +=============================================
> +
> +flash-timeout-us: Flash timeout in microseconds. The value must be in
> +		  the range [100000, 850000] and divisible by 50000.
> +flash-max-microamp: Maximum flash current in microamperes. Has to be
> +		    in the range between [200000, 500000] and
> +		    divisible by 20000.
> +led-max-microamp: Maximum torch (assist) current in microamperes. The
> +		  value must be in the range between [20000, 160000] and
> +		  divisible by 20000.
> +ams,input-max-microamp: Maximum flash controller input current. The
> +			value must be in the range [1250000, 2000000]
> +			and divisible by 50000.
> +
> +
> +Optional properties of the "flash" child node
> +=============================================
> +
> +label		: The label of the flash LED.
> +
> +
> +Required properties of the "indicator" child node
> +=================================================
> +
> +led-max-microamp: Maximum indicator current. The allowed values are
> +		  2500, 5000, 7500 and 10000.
> +
> +Optional properties of the "indicator" child node
> +=================================================
> +
> +label		: The label of the indicator LED.
> +
> +
> +Example
> +=======
> +
> +	as3645a@30 {
> +		reg = <0x30>;
> +		compatible = "ams,as3645a";
> +		flash {
> +			flash-timeout-us = <150000>;
> +			flash-max-microamp = <320000>;
> +			led-max-microamp = <60000>;
> +			ams,input-max-microamp = <1750000>;
> +			label = "as3645a:flash";
> +		};
> +		indicator {
> +			led-max-microamp = <10000>;
> +			label = "as3645a:indicator";
> +		};
> +	};
> 

For the patch going through media tree:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

-- 
Best regards,
Jacek Anaszewski

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

* Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
  2017-08-22 18:34   ` Jacek Anaszewski
@ 2017-08-23  0:28   ` Rob Herring
  2017-08-23  7:33     ` Sakari Ailus
  2017-08-28 10:33   ` Pavel Machek
  2 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2017-08-23  0:28 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, javier, jacek.anaszewski, linux-leds, devicetree,
	Sakari Ailus

On Sun, Aug 20, 2017 at 12:24:08AM +0300, Sakari Ailus wrote:
> From: Sakari Ailus <sakari.ailus@iki.fi>

Commit msg?

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

Shouldn't author and SoB be the same email?

> ---
>  .../devicetree/bindings/leds/ams,as3645a.txt       | 71 ++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-23  0:28   ` Rob Herring
@ 2017-08-23  7:33     ` Sakari Ailus
  0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2017-08-23  7:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-media, javier, jacek.anaszewski, linux-leds, devicetree,
	Sakari Ailus

Hi Rob,

On Tue, Aug 22, 2017 at 07:28:10PM -0500, Rob Herring wrote:
> On Sun, Aug 20, 2017 at 12:24:08AM +0300, Sakari Ailus wrote:
> > From: Sakari Ailus <sakari.ailus@iki.fi>
> 
> Commit msg?

I'll add:

Document DT bindings for analog devices as3645a flash LED controller which
also supports an indicator LED.

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Shouldn't author and SoB be the same email?

I'll change that.

> 
> > ---
> >  .../devicetree/bindings/leds/ams,as3645a.txt       | 71 ++++++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
> 
> Otherwise,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks!

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

* Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
  2017-08-22 18:34   ` Jacek Anaszewski
  2017-08-23  0:28   ` Rob Herring
@ 2017-08-28 10:33   ` Pavel Machek
  2017-08-28 10:48     ` Sakari Ailus
  2 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2017-08-28 10:33 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, javier, jacek.anaszewski, linux-leds, devicetree,
	Sakari Ailus

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

Hi!

> +
> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> +and b are included in the range.

Normally I've seen <a, b> for closed ranges, (a, b) for open
ranges. Is that different in your country?

Otherwise

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


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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-28 10:33   ` Pavel Machek
@ 2017-08-28 10:48     ` Sakari Ailus
  2017-08-28 11:09       ` Pavel Machek
  0 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2017-08-28 10:48 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-media, javier, jacek.anaszewski, linux-leds, devicetree,
	Sakari Ailus


[-- Attachment #1.1: Type: text/plain, Size: 1017 bytes --]

Hi Pavel,

Thanks for the review!

On 08/28/17 13:33, Pavel Machek wrote:
> Hi!
> 
>> +
>> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
>> +and b are included in the range.
> 
> Normally I've seen <a, b> for closed ranges, (a, b) for open
> ranges. Is that different in your country?

I guess there are different notations. :-) I've seen regular parentheses
being used for open ranges, too, but not < and >.

Open range is documented in a related well written Wikipedia article:

<URL:https://en.wikipedia.org/wiki/Open_range>

Are there such open ranges in Czechia? For instance, reindeer generally
roam freely in Finnish Lappland.

What comes to the patch, I guess "interval" could be a more appropriate
term to use in this case:

<URL:https://en.wikipedia.org/wiki/Interval_(mathematics)>

The patch is in line with the Wikipedia article in notation but not in
terminology. I'll send a fix.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
  2017-08-28 10:48     ` Sakari Ailus
@ 2017-08-28 11:09       ` Pavel Machek
  0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2017-08-28 11:09 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, javier, jacek.anaszewski, linux-leds, devicetree,
	Sakari Ailus

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

Hi!

> Thanks for the review!
> 
> On 08/28/17 13:33, Pavel Machek wrote:
> > Hi!
> > 
> >> +
> >> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> >> +and b are included in the range.
> > 
> > Normally I've seen <a, b> for closed ranges, (a, b) for open
> > ranges. Is that different in your country?
> 
> I guess there are different notations. :-) I've seen regular parentheses
> being used for open ranges, too, but not < and >.
> 
> Open range is documented in a related well written Wikipedia article:
> 
> <URL:https://en.wikipedia.org/wiki/Open_range>
> 
> Are there such open ranges in Czechia? For instance, reindeer generally
> roam freely in Finnish Lappland.

:-). Well, we have pigs and roes roaming freely in the woods, but
would not normally call it open range.

> What comes to the patch, I guess "interval" could be a more appropriate
> term to use in this case:
> 
> <URL:https://en.wikipedia.org/wiki/Interval_(mathematics)>
> 
> The patch is in line with the Wikipedia article in notation but not in
> terminology. I'll send a fix.

Ok, that was really nitpicking, thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

end of thread, other threads:[~2017-08-28 11:09 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-19 21:24 [PATCH v2 0/3] AS3645A flash support Sakari Ailus
2017-08-19 21:24 ` [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a Sakari Ailus
2017-08-22 18:34   ` Jacek Anaszewski
2017-08-23  0:28   ` Rob Herring
2017-08-23  7:33     ` Sakari Ailus
2017-08-28 10:33   ` Pavel Machek
2017-08-28 10:48     ` Sakari Ailus
2017-08-28 11:09       ` Pavel Machek
2017-08-19 21:24 ` [PATCH v2 2/3] leds: as3645a: Add LED flash class driver Sakari Ailus
2017-08-19 21:42   ` [PATCH v2.1 " Sakari Ailus
2017-08-20 10:09     ` Jacek Anaszewski
2017-08-20 10:12       ` Jacek Anaszewski
2017-08-21 13:53       ` Sakari Ailus
2017-08-21 19:03         ` Jacek Anaszewski
2017-08-21 20:59           ` Sakari Ailus
2017-08-19 21:24 ` [PATCH v2 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash Sakari Ailus
2017-08-21 19:57   ` [PATCH v2.1 " Sakari Ailus

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).