[PATCH 0/3] Add ktd2692 Flash LED driver

[PATCH 0/3] Add ktd2692 Flash LED driver

[Ingi Kim]

This patch supports KTD2692 flash LED driver

Ingi Kim (3):
  of: Add vendor prefix for Kinetic technologies
  leds: ktd2692: add device tree bindings for ktd2692
  leds: Add ktd2692 flash LED driver

 .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 drivers/leds/Kconfig                               |    8 +
 drivers/leds/Makefile                              |    1 +
 drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
 5 files changed, 274 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
 create mode 100644 drivers/leds/leds-ktd2692.c

-- 
1.7.9.5

[PATCH 1/3] of: Add vendor prefix for Kinetic technologies

[Ingi Kim]

This patch adds vendor prefix for Kinetic technologies

Signed-off-by: Ingi Kim <ingi2.kim@samsung.com>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 389ca13..de9e126 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -98,6 +98,7 @@ isee	ISEE 2007 S.L.
 isil	Intersil
 karo	Ka-Ro electronics GmbH
 keymile	Keymile GmbH
+kinetic Kinetic Technologies
 lacie	LaCie
 lantiq	Lantiq Semiconductor
 lenovo	Lenovo Group Ltd.
-- 
1.7.9.5

[PATCH 2/3] leds: ktd2692: add device tree bindings for ktd2692

[Ingi Kim]

This patch adds the device tree bindings for ktd2692 flash LEDs

Signed-off-by: Ingi Kim <ingi2.kim@samsung.com>
---
 .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-ktd2692.txt b/Documentation/devicetree/bindings/leds/leds-ktd2692.txt
new file mode 100644
index 0000000..b68faa6
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-ktd2692.txt
@@ -0,0 +1,19 @@
+* Kinetic Technologies - KTD2692 Flash LED Driver
+
+KTD2692 is the ideal power solution for high-power flash LEDs.
+It uses ExpressWire single-wire programming for maximum flexibility.
+
+Required properties:
+	- compatible: "kinetic,ktd2692"
+	- strobe-gpio: gpio pin to control movie mode current level.
+	- torch-gpio: gpio pin to control ON/OFF flash mode
+			 as a higher priority over strobe-gpio.
+
+Example:
+
+flash-led {
+	compatible = "kinetic,ktd2692";
+	strobe-gpio = <&gpc0 1 0>;
+	torch-gpio = <&gpc0 2 0>;
+	status = "okay";
+};
-- 
1.7.9.5

[PATCH 3/3] leds: Add ktd2692 flash LED driver

[Ingi Kim]

This patch adds a driver to support the ktd2692 flash LEDs.
ktd2692 can control flash current by ExpressWire interface.

Signed-off-by: Ingi Kim <ingi2.kim@samsung.com>
---
 drivers/leds/Kconfig        |    8 ++
 drivers/leds/Makefile       |    1 +
 drivers/leds/leds-ktd2692.c |  245 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 254 insertions(+)
 create mode 100644 drivers/leds/leds-ktd2692.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 25b320d..f8870db 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -526,6 +526,14 @@ config LEDS_VERSATILE
 	  This option enabled support for the LEDs on the ARM Versatile
 	  and RealView boards. Say Y to enabled these.
 
+config LEDS_KTD2692
+	tristate "LED support for the KTD2692 Driver"
+	depends on LEDS_CLASS
+	depends on GPIOLIB
+	help
+	  This option enables support for the KTD2692 connected through
+	  ExpressWire Interface. Say Y to enable these.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cbba921..289513b 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_LEDS_BLINKM)		+= leds-blinkm.o
 obj-$(CONFIG_LEDS_SYSCON)		+= leds-syscon.o
 obj-$(CONFIG_LEDS_VERSATILE)		+= leds-versatile.o
 obj-$(CONFIG_LEDS_MENF21BMC)		+= leds-menf21bmc.o
+obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_DAC124S085)		+= leds-dac124s085.o
diff --git a/drivers/leds/leds-ktd2692.c b/drivers/leds/leds-ktd2692.c
new file mode 100644
index 0000000..9c98689
--- /dev/null
+++ b/drivers/leds/leds-ktd2692.c
@@ -0,0 +1,245 @@
+/*
+ * Kinetic Technologies KTD2692 Flash LED Driver
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ * Ingi Kim <ingi2.kim@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+
+#define GET_BIT(bit, val)			(((val) >> (bit)) & 0x01)
+
+/* Adjust a multiple of brightness */
+#define KTD2692_BRIGHTNESS_RANGE_255_TO_16(x)	(((x) >> 4) & 0x0F)
+#define KTD2692_BRIGHTNESS_RANGE_255_TO_8(x)	(((x) >> 5) & 0x0F)
+#define KTD2692_BRIGHTNESS_RANGE_255_TO_4(x)	(((x) >> 6) & 0x0F)
+
+/* Base register address */
+#define KTD2692_REG_LVP_BASE			0x00
+#define KTD2692_REG_FLASH_TIMEOUT_BASE		0x20
+#define KTD2692_REG_MIN_CURRENT_SET_BASE	0x40
+#define KTD2692_REG_MOVIE_CURRENT_BASE		0x60
+#define KTD2692_REG_FLASH_CURRENT_BASE		0x80
+#define KTD2692_REG_MODE_BASE			0xA0
+
+/* Set bit coding time for expresswire interface */
+#define KTD2692_TIME_RESET_US			700
+#define KTD2692_TIME_DATA_START_TIME_US		10
+#define KTD2692_TIME_HIGH_END_OF_DATA_US	350
+#define KTD2692_TIME_LOW_END_OF_DATA_US		10
+#define KTD2692_TIME_SHORT_BITSET_US		4
+#define KTD2692_TIME_LONG_BITSET_US		12
+
+/* KTD2692 default length of name */
+#define KTD2692_NAME_LENGTH			20
+
+/* KTD2692 default name */
+#define KTD2692_DEFAULT_NAME			"ktd2692"
+
+enum ktd2692_mode {
+	KTD2692_MODE_DISABLE = 0,
+	KTD2692_MODE_MOVIE,
+	KTD2692_MODE_FLASH,
+};
+
+enum ktd2692_bitset {
+	KTD2692_LOW = 0,
+	KTD2692_HIGH,
+};
+
+struct ktd2692_context {
+	struct led_classdev cdev;
+	u8 mode;
+	int strobe_gpio;
+};
+
+static int ktd2692_brightness_set_gpio(struct ktd2692_context *led)
+{
+	int ret;
+
+	ret = devm_gpio_request_one(led->cdev.dev,
+			led->strobe_gpio, GPIOF_INIT_LOW, "strobe-gpio");
+
+	if (ret)
+		dev_err(led->cdev.dev,
+			"failed to request strobe-gpio %d error %d\n",
+			led->strobe_gpio, ret);
+
+	return ret;
+}
+
+static void ktd2692_expresswire_start(struct ktd2692_context *led)
+{
+	gpio_set_value(led->strobe_gpio, KTD2692_HIGH);
+	udelay(KTD2692_TIME_DATA_START_TIME_US);
+}
+
+static void ktd2692_expresswire_reset(struct ktd2692_context *led)
+{
+	gpio_set_value(led->strobe_gpio, KTD2692_LOW);
+	udelay(KTD2692_TIME_RESET_US);
+}
+
+static void ktd2692_expresswire_end(struct ktd2692_context *led)
+{
+	gpio_set_value(led->strobe_gpio, KTD2692_LOW);
+	udelay(KTD2692_TIME_LOW_END_OF_DATA_US);
+	gpio_set_value(led->strobe_gpio, KTD2692_HIGH);
+	udelay(KTD2692_TIME_HIGH_END_OF_DATA_US);
+}
+
+static void ktd2692_expresswire_set_bit(struct ktd2692_context *led, bool bit)
+{
+	if (bit) {
+		gpio_set_value(led->strobe_gpio, KTD2692_LOW);
+		udelay(KTD2692_TIME_SHORT_BITSET_US);
+		gpio_set_value(led->strobe_gpio, KTD2692_HIGH);
+		udelay(KTD2692_TIME_LONG_BITSET_US);
+	} else {
+		gpio_set_value(led->strobe_gpio, KTD2692_LOW);
+		udelay(KTD2692_TIME_LONG_BITSET_US);
+		gpio_set_value(led->strobe_gpio, KTD2692_HIGH);
+		udelay(KTD2692_TIME_SHORT_BITSET_US);
+	}
+}
+
+static void ktd2692_expresswire_write(struct ktd2692_context *led, u8 value)
+{
+	int i;
+
+	ktd2692_expresswire_start(led);
+	for (i = 7; i >= 0; i--)
+		ktd2692_expresswire_set_bit(led, GET_BIT(i, value));
+	ktd2692_expresswire_end(led);
+}
+
+static void ktd2692_brightness_set(struct led_classdev *led_cdev,
+		enum led_brightness value)
+{
+	u8 mask;
+	int brightness = (int)value;
+	struct ktd2692_context *led =
+		container_of(led_cdev, struct ktd2692_context, cdev);
+
+	if (brightness == LED_OFF) {
+		gpio_set_value(led->strobe_gpio, 0);
+		led->cdev.brightness = brightness;
+		return;
+	}
+
+	if (brightness < 0 || brightness > led->cdev.max_brightness) {
+		dev_err(led_cdev->dev, "Invalid brightness %d\n", brightness);
+	} else {
+		mask = KTD2692_BRIGHTNESS_RANGE_255_TO_8(value);
+		mask |= KTD2692_REG_MOVIE_CURRENT_BASE;
+		ktd2692_expresswire_write(led, mask);
+
+		led->mode = KTD2692_REG_MODE_BASE | KTD2692_MODE_MOVIE;
+		ktd2692_expresswire_write(led, led->mode);
+		led->cdev.brightness = brightness;
+	}
+}
+
+static enum led_brightness ktd2692_brightness_get(struct led_classdev *led_cdev)
+{
+	struct ktd2692_context *led =
+		container_of(led_cdev, struct ktd2692_context, cdev);
+
+	return led->cdev.brightness;
+}
+
+static struct ktd2692_context *ktd2692_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct ktd2692_context *led;
+
+	led = devm_kzalloc(dev, sizeof(struct ktd2692_context), GFP_KERNEL);
+	if (!led)
+		return ERR_PTR((long)led);
+
+	led->strobe_gpio = of_get_named_gpio(np, "strobe-gpio", 0);
+	if (!gpio_is_valid(led->strobe_gpio)) {
+		dev_err(dev, "no strobe_gpio property found\n");
+		return ERR_PTR(led->strobe_gpio);
+	}
+
+	return led;
+}
+
+static int ktd2692_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ktd2692_context *led;
+	int ret;
+
+	if (!dev->of_node)
+		return -ENODEV;
+
+	led = ktd2692_parse_dt(dev);
+	if (IS_ERR(led))
+		return PTR_ERR(led);
+
+	led->cdev.name = KTD2692_DEFAULT_NAME;
+	led->cdev.brightness = LED_OFF;
+	led->cdev.max_brightness = LED_FULL;
+	led->cdev.flags |= LED_CORE_SUSPENDRESUME;
+	led->cdev.brightness_set = ktd2692_brightness_set;
+	led->cdev.brightness_get = ktd2692_brightness_get;
+	led->mode = KTD2692_REG_MODE_BASE | KTD2692_MODE_DISABLE;
+
+	platform_set_drvdata(pdev, led);
+
+	ret = led_classdev_register(&pdev->dev, &led->cdev);
+	if (ret) {
+		dev_err(dev, "couldn't register LED %s\n", led->cdev.name);
+		return ret;
+	}
+
+	ret = ktd2692_brightness_set_gpio(led);
+	if (ret) {
+		led_classdev_unregister(&led->cdev);
+		return ret;
+	}
+
+	ktd2692_expresswire_reset(led);
+
+	return ret;
+}
+
+static int ktd2692_remove(struct platform_device *pdev)
+{
+	struct ktd2692_context *led = platform_get_drvdata(pdev);
+
+	led_classdev_unregister(&led->cdev);
+
+	return 0;
+}
+
+static const struct of_device_id ktd2692_match[] = {
+	{ .compatible = "kinetic,ktd2692", },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver ktd2692_driver = {
+	.driver = {
+		.name  = "leds-ktd2692",
+		.of_match_table = ktd2692_match,
+	},
+	.probe  = ktd2692_probe,
+	.remove = ktd2692_remove,
+};
+
+module_platform_driver(ktd2692_driver);
+
+MODULE_AUTHOR("Ingi Kim <ingi2.kim@samsung.com>");
+MODULE_DESCRIPTION("Kinetic KTD2692 LED driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Re: [PATCH 3/3] leds: Add ktd2692 flash LED driver

[Varka Bhadram]

On 02/27/2015 06:31 AM, Ingi Kim wrote:
> This patch adds a driver to support the ktd2692 flash LEDs.
> ktd2692 can control flash current by ExpressWire interface.
>
> Signed-off-by: Ingi Kim <ingi2.kim@samsung.com>
> ---
>   drivers/leds/Kconfig        |    8 ++
>   drivers/leds/Makefile       |    1 +
>   drivers/leds/leds-ktd2692.c |  245 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 254 insertions(+)
>   create mode 100644 drivers/leds/leds-ktd2692.c
>
(...)

> +static struct ktd2692_context *ktd2692_parse_dt(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct ktd2692_context *led;
> +
> + led = devm_kzalloc(dev, sizeof(struct ktd2692_context), GFP_KERNEL);
> + if (!led)
> + return ERR_PTR((long)led);

What about using sizeof(*led) in place of sizeof(struct ktd2692_context)..?

Also the error return for devm_kzalloc() should be -ENOMEM.

> +
> + led->strobe_gpio = of_get_named_gpio(np, "strobe-gpio", 0);
> + if (!gpio_is_valid(led->strobe_gpio)) {
> + dev_err(dev, "no strobe_gpio property found\n");
> + return ERR_PTR(led->strobe_gpio);
> + }
> +
> + return led;
> +}
> +
> +static int ktd2692_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ktd2692_context *led;
> + int ret;
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + led = ktd2692_parse_dt(dev);
> + if (IS_ERR(led))
> + return PTR_ERR(led);
> +
> + led->cdev.name = KTD2692_DEFAULT_NAME;
> + led->cdev.brightness = LED_OFF;
> + led->cdev.max_brightness = LED_FULL;
> + led->cdev.flags |= LED_CORE_SUSPENDRESUME;
> + led->cdev.brightness_set = ktd2692_brightness_set;
> + led->cdev.brightness_get = ktd2692_brightness_get;
> + led->mode = KTD2692_REG_MODE_BASE | KTD2692_MODE_DISABLE;
> +
> + platform_set_drvdata(pdev, led);
> +
> + ret = led_classdev_register(&pdev->dev, &led->cdev);
> + if (ret) {
> + dev_err(dev, "couldn't register LED %s\n", led->cdev.name);
> + return ret;
> + }
> +
> + ret = ktd2692_brightness_set_gpio(led);
> + if (ret) {
> + led_classdev_unregister(&led->cdev);
> + return ret;
> + }
> +
> + ktd2692_expresswire_reset(led);
> +
> + return ret;

return 0 instead of ret...?



-- 
Thanks,
Varka Bhadram.

Re: [PATCH 3/3] leds: Add ktd2692 flash LED driver

[Ingi Kim]

hi

On 2015년 02월 27일 12:36, Varka Bhadram wrote:
> On 02/27/2015 06:31 AM, Ingi Kim wrote:
>> This patch adds a driver to support the ktd2692 flash LEDs.
>> ktd2692 can control flash current by ExpressWire interface.
>>
>> Signed-off-by: Ingi Kim <ingi2.kim@samsung.com>
>> ---
>>   drivers/leds/Kconfig        |    8 ++
>>   drivers/leds/Makefile       |    1 +
>>   drivers/leds/leds-ktd2692.c |  245 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 254 insertions(+)
>>   create mode 100644 drivers/leds/leds-ktd2692.c
>>
> (...)
> 
>> +static struct ktd2692_context *ktd2692_parse_dt(struct device *dev)
>> +{
>> + struct device_node *np = dev->of_node;
>> + struct ktd2692_context *led;
>> +
>> + led = devm_kzalloc(dev, sizeof(struct ktd2692_context), GFP_KERNEL);
>> + if (!led)
>> + return ERR_PTR((long)led);
> 
> What about using sizeof(*led) in place of sizeof(struct ktd2692_context)..?
> 
> Also the error return for devm_kzalloc() should be -ENOMEM.
> 

Thanks, I'll check and change sizeof() and error return style.

>> +
>> + led->strobe_gpio = of_get_named_gpio(np, "strobe-gpio", 0);
>> + if (!gpio_is_valid(led->strobe_gpio)) {
>> + dev_err(dev, "no strobe_gpio property found\n");
>> + return ERR_PTR(led->strobe_gpio);
>> + }
>> +
>> + return led;
>> +}
>> +
>> +static int ktd2692_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct ktd2692_context *led;
>> + int ret;
>> +
>> + if (!dev->of_node)
>> + return -ENODEV;
>> +
>> + led = ktd2692_parse_dt(dev);
>> + if (IS_ERR(led))
>> + return PTR_ERR(led);
>> +
>> + led->cdev.name = KTD2692_DEFAULT_NAME;
>> + led->cdev.brightness = LED_OFF;
>> + led->cdev.max_brightness = LED_FULL;
>> + led->cdev.flags |= LED_CORE_SUSPENDRESUME;
>> + led->cdev.brightness_set = ktd2692_brightness_set;
>> + led->cdev.brightness_get = ktd2692_brightness_get;
>> + led->mode = KTD2692_REG_MODE_BASE | KTD2692_MODE_DISABLE;
>> +
>> + platform_set_drvdata(pdev, led);
>> +
>> + ret = led_classdev_register(&pdev->dev, &led->cdev);
>> + if (ret) {
>> + dev_err(dev, "couldn't register LED %s\n", led->cdev.name);
>> + return ret;
>> + }
>> +
>> + ret = ktd2692_brightness_set_gpio(led);
>> + if (ret) {
>> + led_classdev_unregister(&led->cdev);
>> + return ret;
>> + }
>> +
>> + ktd2692_expresswire_reset(led);
>> +
>> + return ret;
> 
> return 0 instead of ret...?
> 
> 
> 

I'll check and try
Thanks,

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Jacek Anaszewski]

Hi Ingi,

On 02/27/2015 02:01 AM, Ingi Kim wrote:
> This patch supports KTD2692 flash LED driver
>
> Ingi Kim (3):
>    of: Add vendor prefix for Kinetic technologies
>    leds: ktd2692: add device tree bindings for ktd2692
>    leds: Add ktd2692 flash LED driver
>
>   .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
>   .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>   drivers/leds/Kconfig                               |    8 +
>   drivers/leds/Makefile                              |    1 +
>   drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
>   5 files changed, 274 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
>   create mode 100644 drivers/leds/leds-ktd2692.c
>

In your device tree binding documentation there is torch-gpio mentioned,
but you seem not to use it in the driver.

We have already LED Flash class (/drivers/leds/led-class-flash.c) for
this type of devices, which handles both torch and flash modes
(flash_strobe sysfs attribute is provided for strobing the flash).

The reference drivers using LED Flash class are still pending [1], but I
think that at least leds-aat1290 driver is almost ready for merging.
It controls very similar device to yours.

Another advantage of using LED Flash class is that it has been designed
to be compatible with Video for Linux 2 subsystem, which will allow for 
registering LED Flash class devices as a V4L2 sub-devices.

Adding Sakari.

-- 
Best Regards,
Jacek Anaszewski

[1] http://www.spinics.net/lists/linux-media/msg86632.html

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Ingi Kim]

Hi Jacek

On 2015년 02월 27일 17:42, Jacek Anaszewski wrote:
> Hi Ingi,
> 
> On 02/27/2015 02:01 AM, Ingi Kim wrote:
>> This patch supports KTD2692 flash LED driver
>>
>> Ingi Kim (3):
>>    of: Add vendor prefix for Kinetic technologies
>>    leds: ktd2692: add device tree bindings for ktd2692
>>    leds: Add ktd2692 flash LED driver
>>
>>   .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
>>   .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>>   drivers/leds/Kconfig                               |    8 +
>>   drivers/leds/Makefile                              |    1 +
>>   drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
>>   5 files changed, 274 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
>>   create mode 100644 drivers/leds/leds-ktd2692.c
>>
> 
> In your device tree binding documentation there is torch-gpio mentioned,
> but you seem not to use it in the driver.
> 
> We have already LED Flash class (/drivers/leds/led-class-flash.c) for
> this type of devices, which handles both torch and flash modes
> (flash_strobe sysfs attribute is provided for strobing the flash).
> 
> The reference drivers using LED Flash class are still pending [1], but I
> think that at least leds-aat1290 driver is almost ready for merging.
> It controls very similar device to yours.
> 
> Another advantage of using LED Flash class is that it has been designed
> to be compatible with Video for Linux 2 subsystem, which will allow for registering LED Flash class devices as a V4L2 sub-devices.
> 
> Adding Sakari.
> 

Ok, I'll check LED Flash class, and add torch-gpio
Thanks,

Adding Seung-woo Kim and Inha Song

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Sakari Ailus]

H Ingi,

On Mon, Mar 02, 2015 at 04:14:39PM +0900, Ingi Kim wrote:
> Hi Jacek
> 
> On 2015년 02월 27일 17:42, Jacek Anaszewski wrote:
> > Hi Ingi,
> > 
> > On 02/27/2015 02:01 AM, Ingi Kim wrote:
> >> This patch supports KTD2692 flash LED driver
> >>
> >> Ingi Kim (3):
> >>    of: Add vendor prefix for Kinetic technologies
> >>    leds: ktd2692: add device tree bindings for ktd2692
> >>    leds: Add ktd2692 flash LED driver
> >>
> >>   .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
> >>   .../devicetree/bindings/vendor-prefixes.txt        |    1 +
> >>   drivers/leds/Kconfig                               |    8 +
> >>   drivers/leds/Makefile                              |    1 +
> >>   drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
> >>   5 files changed, 274 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
> >>   create mode 100644 drivers/leds/leds-ktd2692.c
> >>
> > 
> > In your device tree binding documentation there is torch-gpio mentioned,
> > but you seem not to use it in the driver.
> > 
> > We have already LED Flash class (/drivers/leds/led-class-flash.c) for
> > this type of devices, which handles both torch and flash modes
> > (flash_strobe sysfs attribute is provided for strobing the flash).
> > 
> > The reference drivers using LED Flash class are still pending [1], but I
> > think that at least leds-aat1290 driver is almost ready for merging.
> > It controls very similar device to yours.
> > 
> > Another advantage of using LED Flash class is that it has been designed
> > to be compatible with Video for Linux 2 subsystem, which will allow for registering LED Flash class devices as a V4L2 sub-devices.
> > 
> > Adding Sakari.
> > 
> 
> Ok, I'll check LED Flash class, and add torch-gpio

Many LED flash chips include a hardware pin for torch control but few really
need it. If you don't, i.e. you can implement the torch using the control bus
instead, I think I'd probably drop it from the chip's DT bindings.

I wonder what others think.

-- 
Kind regards,

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

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Bryan Wu]

On Mon, Mar 2, 2015 at 1:15 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> H Ingi,
>
> On Mon, Mar 02, 2015 at 04:14:39PM +0900, Ingi Kim wrote:
>> Hi Jacek
>>
>> On 2015년 02월 27일 17:42, Jacek Anaszewski wrote:
>> > Hi Ingi,
>> >
>> > On 02/27/2015 02:01 AM, Ingi Kim wrote:
>> >> This patch supports KTD2692 flash LED driver
>> >>
>> >> Ingi Kim (3):
>> >>    of: Add vendor prefix for Kinetic technologies
>> >>    leds: ktd2692: add device tree bindings for ktd2692
>> >>    leds: Add ktd2692 flash LED driver
>> >>
>> >>   .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
>> >>   .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>> >>   drivers/leds/Kconfig                               |    8 +
>> >>   drivers/leds/Makefile                              |    1 +
>> >>   drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
>> >>   5 files changed, 274 insertions(+)
>> >>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
>> >>   create mode 100644 drivers/leds/leds-ktd2692.c
>> >>
>> >
>> > In your device tree binding documentation there is torch-gpio mentioned,
>> > but you seem not to use it in the driver.
>> >
>> > We have already LED Flash class (/drivers/leds/led-class-flash.c) for
>> > this type of devices, which handles both torch and flash modes
>> > (flash_strobe sysfs attribute is provided for strobing the flash).
>> >
>> > The reference drivers using LED Flash class are still pending [1], but I
>> > think that at least leds-aat1290 driver is almost ready for merging.
>> > It controls very similar device to yours.
>> >
>> > Another advantage of using LED Flash class is that it has been designed
>> > to be compatible with Video for Linux 2 subsystem, which will allow for registering LED Flash class devices as a V4L2 sub-devices.
>> >
>> > Adding Sakari.
>> >
>>
>> Ok, I'll check LED Flash class, and add torch-gpio
>
> Many LED flash chips include a hardware pin for torch control but few really
> need it. If you don't, i.e. you can implement the torch using the control bus
> instead, I think I'd probably drop it from the chip's DT bindings.
>

Ingi, please follow Jacek's advice to use LED Flash class interface.
I'm reviewing those leds flash drivers and probably merge them soon.

Jacek and Sakari thanks for the review.

Sakari, so what's the control bus your mentioned here?

Thanks,
-Bryan

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Sakari Ailus]

Hi Bryan,

On Mon, Mar 02, 2015 at 12:07:42PM -0800, Bryan Wu wrote:
> On Mon, Mar 2, 2015 at 1:15 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > H Ingi,
> >
> > On Mon, Mar 02, 2015 at 04:14:39PM +0900, Ingi Kim wrote:
> >> Hi Jacek
> >>
> >> On 2015년 02월 27일 17:42, Jacek Anaszewski wrote:
> >> > Hi Ingi,
> >> >
> >> > On 02/27/2015 02:01 AM, Ingi Kim wrote:
> >> >> This patch supports KTD2692 flash LED driver
> >> >>
> >> >> Ingi Kim (3):
> >> >>    of: Add vendor prefix for Kinetic technologies
> >> >>    leds: ktd2692: add device tree bindings for ktd2692
> >> >>    leds: Add ktd2692 flash LED driver
> >> >>
> >> >>   .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
> >> >>   .../devicetree/bindings/vendor-prefixes.txt        |    1 +
> >> >>   drivers/leds/Kconfig                               |    8 +
> >> >>   drivers/leds/Makefile                              |    1 +
> >> >>   drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
> >> >>   5 files changed, 274 insertions(+)
> >> >>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
> >> >>   create mode 100644 drivers/leds/leds-ktd2692.c
> >> >>
> >> >
> >> > In your device tree binding documentation there is torch-gpio mentioned,
> >> > but you seem not to use it in the driver.
> >> >
> >> > We have already LED Flash class (/drivers/leds/led-class-flash.c) for
> >> > this type of devices, which handles both torch and flash modes
> >> > (flash_strobe sysfs attribute is provided for strobing the flash).
> >> >
> >> > The reference drivers using LED Flash class are still pending [1], but I
> >> > think that at least leds-aat1290 driver is almost ready for merging.
> >> > It controls very similar device to yours.
> >> >
> >> > Another advantage of using LED Flash class is that it has been designed
> >> > to be compatible with Video for Linux 2 subsystem, which will allow for registering LED Flash class devices as a V4L2 sub-devices.
> >> >
> >> > Adding Sakari.
> >> >
> >>
> >> Ok, I'll check LED Flash class, and add torch-gpio
> >
> > Many LED flash chips include a hardware pin for torch control but few really
> > need it. If you don't, i.e. you can implement the torch using the control bus
> > instead, I think I'd probably drop it from the chip's DT bindings.
> >
> 
> Ingi, please follow Jacek's advice to use LED Flash class interface.
> I'm reviewing those leds flash drivers and probably merge them soon.
> 
> Jacek and Sakari thanks for the review.
> 
> Sakari, so what's the control bus your mentioned here?

I wasn't cc'd in the original patch so I didn't know whether it was I2C or
not. Doesn't appear to be.

-- 
Regards,

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

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Ingi Kim]

Hi

On 2015년 03월 03일 07:03, Sakari Ailus wrote:
> Hi Bryan,
> 
> On Mon, Mar 02, 2015 at 12:07:42PM -0800, Bryan Wu wrote:
>> On Mon, Mar 2, 2015 at 1:15 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>>> H Ingi,
>>>
>>> On Mon, Mar 02, 2015 at 04:14:39PM +0900, Ingi Kim wrote:
>>>> Hi Jacek
>>>>
>>>> On 2015년 02월 27일 17:42, Jacek Anaszewski wrote:
>>>>> Hi Ingi,
>>>>>
>>>>> On 02/27/2015 02:01 AM, Ingi Kim wrote:
>>>>>> This patch supports KTD2692 flash LED driver
>>>>>>
>>>>>> Ingi Kim (3):
>>>>>>    of: Add vendor prefix for Kinetic technologies
>>>>>>    leds: ktd2692: add device tree bindings for ktd2692
>>>>>>    leds: Add ktd2692 flash LED driver
>>>>>>
>>>>>>   .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
>>>>>>   .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>>>>>>   drivers/leds/Kconfig                               |    8 +
>>>>>>   drivers/leds/Makefile                              |    1 +
>>>>>>   drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
>>>>>>   5 files changed, 274 insertions(+)
>>>>>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
>>>>>>   create mode 100644 drivers/leds/leds-ktd2692.c
>>>>>>
>>>>>
>>>>> In your device tree binding documentation there is torch-gpio mentioned,
>>>>> but you seem not to use it in the driver.
>>>>>
>>>>> We have already LED Flash class (/drivers/leds/led-class-flash.c) for
>>>>> this type of devices, which handles both torch and flash modes
>>>>> (flash_strobe sysfs attribute is provided for strobing the flash).
>>>>>
>>>>> The reference drivers using LED Flash class are still pending [1], but I
>>>>> think that at least leds-aat1290 driver is almost ready for merging.
>>>>> It controls very similar device to yours.
>>>>>
>>>>> Another advantage of using LED Flash class is that it has been designed
>>>>> to be compatible with Video for Linux 2 subsystem, which will allow for registering LED Flash class devices as a V4L2 sub-devices.
>>>>>
>>>>> Adding Sakari.
>>>>>
>>>>
>>>> Ok, I'll check LED Flash class, and add torch-gpio
>>>
>>> Many LED flash chips include a hardware pin for torch control but few really
>>> need it. If you don't, i.e. you can implement the torch using the control bus
>>> instead, I think I'd probably drop it from the chip's DT bindings.
>>>
>>
>> Ingi, please follow Jacek's advice to use LED Flash class interface.
>> I'm reviewing those leds flash drivers and probably merge them soon.
>>
>> Jacek and Sakari thanks for the review.
>>
>> Sakari, so what's the control bus your mentioned here?
> 
> I wasn't cc'd in the original patch so I didn't know whether it was I2C or
> not. Doesn't appear to be.
> 

Thanks for the review and I'll work to use LED Flash class interface
and try adding torch-gpio(flash mode enable) line (I'll rename those gpio lines)

Sakari,
ktd2692 flash led device is controlled through Expresswire single-wire interface
by using the strobe-gpio(control) pin. 


Adding pin information of the KTD2692.

CTRL pin can control LED on/off and Movie/Flash mode current,
Flash timeout, LVP(low voltage protection).

AUX pin(torch-gpio) allows highest priority ON/OFF flash mode control.

Re: [PATCH 0/3] Add ktd2692 Flash LED driver

[Jacek Anaszewski]

On 03/02/2015 09:07 PM, Bryan Wu wrote:
> On Mon, Mar 2, 2015 at 1:15 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>> H Ingi,
>>
>> On Mon, Mar 02, 2015 at 04:14:39PM +0900, Ingi Kim wrote:
>>> Hi Jacek
>>>
>>> On 2015년 02월 27일 17:42, Jacek Anaszewski wrote:
>>>> Hi Ingi,
>>>>
>>>> On 02/27/2015 02:01 AM, Ingi Kim wrote:
>>>>> This patch supports KTD2692 flash LED driver
>>>>>
>>>>> Ingi Kim (3):
>>>>>     of: Add vendor prefix for Kinetic technologies
>>>>>     leds: ktd2692: add device tree bindings for ktd2692
>>>>>     leds: Add ktd2692 flash LED driver
>>>>>
>>>>>    .../devicetree/bindings/leds/leds-ktd2692.txt      |   19 ++
>>>>>    .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>>>>>    drivers/leds/Kconfig                               |    8 +
>>>>>    drivers/leds/Makefile                              |    1 +
>>>>>    drivers/leds/leds-ktd2692.c                        |  245 ++++++++++++++++++++
>>>>>    5 files changed, 274 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
>>>>>    create mode 100644 drivers/leds/leds-ktd2692.c
>>>>>
>>>>
>>>> In your device tree binding documentation there is torch-gpio mentioned,
>>>> but you seem not to use it in the driver.
>>>>
>>>> We have already LED Flash class (/drivers/leds/led-class-flash.c) for
>>>> this type of devices, which handles both torch and flash modes
>>>> (flash_strobe sysfs attribute is provided for strobing the flash).
>>>>
>>>> The reference drivers using LED Flash class are still pending [1], but I
>>>> think that at least leds-aat1290 driver is almost ready for merging.
>>>> It controls very similar device to yours.
>>>>
>>>> Another advantage of using LED Flash class is that it has been designed
>>>> to be compatible with Video for Linux 2 subsystem, which will allow for registering LED Flash class devices as a V4L2 sub-devices.
>>>>
>>>> Adding Sakari.
>>>>
>>>
>>> Ok, I'll check LED Flash class, and add torch-gpio
>>
>> Many LED flash chips include a hardware pin for torch control but few really
>> need it. If you don't, i.e. you can implement the torch using the control bus
>> instead, I think I'd probably drop it from the chip's DT bindings.
>>
>
> Ingi, please follow Jacek's advice to use LED Flash class interface.
> I'm reviewing those leds flash drivers and probably merge them soon.

Bryan please hold on with merging them as I am about to send new patch
set, as we've agreed that synchronized strobe feature should be removed
from the LED Flash class. There will be also some tweaking around
aat1290 DT bindings.

-- 
Best Regards,
Jacek Anaszewski