All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] leds: driver for ab5500 high voltage leds
@ 2011-12-02 13:56 Linus Walleij
  2011-12-03 21:49 ` Mark Brown
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Linus Walleij @ 2011-12-02 13:56 UTC (permalink / raw)
  To: Richard Purdie, linux-kernel
  Cc: Andrew Morton, Samuel Ortiz, Shreshtha Kumar Sahu, Linus Walleij

From: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>

Simple HV LED controller driver for AB5500v1.0 MFD chips

Signed-off-by: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
Reviewed-by: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>
Tested-by: Naga Radesh Y <naga.radheshy@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This depends on the other movement patch since it handily puts the
platform data header in <linux/mfd/abx500/ab5500-leds.h>.
---
 Documentation/ABI/testing/sysfs-class-led-ab5500 |   27 +
 drivers/leds/Kconfig                             |    9 +
 drivers/leds/Makefile                            |    1 +
 drivers/leds/leds-ab5500.c                       |  830 ++++++++++++++++++++++
 include/linux/mfd/abx500/ab5500-leds.h           |   54 ++
 5 files changed, 921 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-ab5500
 create mode 100644 drivers/leds/leds-ab5500.c
 create mode 100644 include/linux/mfd/abx500/ab5500-leds.h

diff --git a/Documentation/ABI/testing/sysfs-class-led-ab5500 b/Documentation/ABI/testing/sysfs-class-led-ab5500
new file mode 100644
index 0000000..556b9c3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-led-ab5500
@@ -0,0 +1,27 @@
+What:		/sys/class/leds/<led>/fade_auto
+Date:		December 2011
+KernelVersion:	3.3
+Contact:	Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
+Description:
+		Set the auto fadeout mode for the LED.
+		0 = no auto fadeout
+		1 = auto fadeout
+
+What:		/sys/class/leds/<led>/fade_delay
+Date:		December 2011
+KernelVersion:	3.3
+Contact:	Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
+Description:
+		Set the auto fadeout delay for the LED.
+		0 = no delay
+		1 = 1/2 second
+		2 = 1 second
+		3 = 2 seconds
+
+What:		/sys/class/leds/<led>/led_current
+Date:		December 2011
+KernelVersion:	3.3
+Contact:	Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
+Description:
+		Read and set the current limit for the LED. Value in
+		milliampere.
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index ff203a4..0114c68 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -50,6 +50,15 @@ config LEDS_LM3530
 	  controlled manually or using PWM input or using ambient
 	  light automatically.
 
+config LEDS_AB5500
+	tristate "HVLED driver for AB5500"
+	depends on AB5500_CORE
+	default n
+	help
+	  This option enables support for the HVLED in AB5500
+	  multi function device. Currently Ab5500 v1.0 chip leds
+	  are supported.
+
 config LEDS_LOCOMO
 	tristate "LED Support for Locomo device"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e4f6bf5..614bad4 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_LEDS_ATMEL_PWM)		+= leds-atmel-pwm.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
+obj-$(CONFIG_LEDS_AB5500)              += leds-ab5500.o
 obj-$(CONFIG_LEDS_MIKROTIK_RB532)	+= leds-rb532.o
 obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
 obj-$(CONFIG_LEDS_AMS_DELTA)		+= leds-ams-delta.o
diff --git a/drivers/leds/leds-ab5500.c b/drivers/leds/leds-ab5500.c
new file mode 100644
index 0000000..31c1e8c
--- /dev/null
+++ b/drivers/leds/leds-ab5500.c
@@ -0,0 +1,830 @@
+/*
+ * leds-ab5500.c - driver for High Voltage (HV) LED in ST-Ericsson AB5500 chip
+ *
+ * Copyright (C) 2011 ST-Ericsson SA.
+ *
+ * License Terms: GNU General Public License v2
+ *
+ * Author: Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
+ */
+
+/*
+ * Driver for HVLED in ST-Ericsson AB5500 analog baseband controller
+ *
+ * This chip can drive upto 3 leds, of upto 40mA of led sink current.
+ * These leds can be programmed to blink between two intensities with
+ * fading delay of half, one or two seconds.
+ *
+ * Leds can be controlled via sysfs entries in
+ *	"/sys/class/leds/< red | green | blue >"
+ *
+ * For each led,
+ *
+ * Modes of operation:
+ *  - manual:	echo 0 > fade_auto (default, no auto blinking)
+ *  - auto:	echo 1 > fade_auto
+ *
+ * Soft scaling delay between two intensities:
+ *  - 1/2 sec:	echo 1 > fade_delay
+ *  - 1 sec:	echo 2 > fade_delay
+ *  - 2 sec:	echo 3 > fade_delay
+ *
+ * Possible sequence of operation:
+ *  - continuous glow: set brightness (brt)
+ *  - blink between LED_OFF and LED_FULL:
+ *	set fade delay -> set fade auto
+ *  - blink between previous two brightness (only for LED-1):
+ *	set brt1 -> set brt2 -> set fade auto
+ *
+ * Delay can be set in any step, its affect will be seen on switching mode.
+ *
+ * Note: Blink/Fade feature is supported in AB5500 v2 onwards
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/leds.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/mfd/abx500.h>
+#include <linux/mfd/abx500/ab5500.h>
+#include <linux/mfd/abx500/ab5500-leds.h>
+#include <linux/types.h>
+
+#define AB5500LED_NAME		"ab5500-leds"
+#define AB5500_LED_MAX		0x03
+
+/* Register offsets */
+#define AB5500_LED_REG_ENABLE	0x03
+#define AB5500_LED_FADE_CTRL	0x0D
+
+/* LED-0 Register Addr. Offsets */
+#define AB5500_LED0_PWM_DUTY	0x01
+#define AB5500_LED0_PWMFREQ	0x02
+#define AB5500_LED0_SINKCTL	0x0A
+#define AB5500_LED0_FADE_HI	0x11
+#define AB5500_LED0_FADE_LO	0x17
+
+/* LED-1 Register Addr. Offsets */
+#define AB5500_LED1_PWM_DUTY	0x05
+#define AB5500_LED1_PWMFREQ	0x06
+#define AB5500_LED1_SINKCTL	0x0B
+#define AB5500_LED1_FADE_HI	0x13
+#define AB5500_LED1_FADE_LO	0x19
+
+/* LED-2 Register Addr. Offsets */
+#define AB5500_LED2_PWM_DUTY	0x08
+#define AB5500_LED2_PWMFREQ	0x09
+#define AB5500_LED2_SINKCTL	0x0C
+#define AB5500_LED2_FADE_HI	0x15
+#define AB5500_LED2_FADE_LO	0x1B
+
+/* led-0/1/2 enable bit */
+#define AB5500_LED_ENABLE_MASK	0x04
+
+/* led intensity */
+#define AB5500_LED_INTENSITY_OFF	0x0
+#define AB5500_LED_INTENSITY_MAX	0x3FF
+#define AB5500_LED_INTENSITY_STEP	(AB5500_LED_INTENSITY_MAX/LED_FULL)
+
+/* pwm frequency */
+#define AB5500_LED_PWMFREQ_MAX		0x0F	/* 373.39 @sysclk=26MHz */
+#define AB5500_LED_PWMFREQ_SHIFT	4
+
+/* LED sink current control */
+#define AB5500_LED_SINKCURR_MAX		0x0F	/* 40mA MAX */
+#define AB5500_LED_SINKCURR_SHIFT	4
+
+/* fade Control shift and masks */
+#define AB5500_FADE_DELAY_SHIFT		0x00
+#define AB5500_FADE_MODE_MASK		0x80
+#define AB5500_FADE_DELAY_MASK		0x03
+#define AB5500_FADE_START_MASK		0x04
+#define AB5500_FADE_ON_MASK		0x70
+#define AB5500_LED_FADE_ENABLE(ledid)	(0x40 >> (ledid))
+
+/**
+ * struct ab5500_led
+ * @id: hvled index
+ * @max_current: max current permitted for hvled instance
+ * @brt_val: brightness value set for hvled
+ * @fade_hi: fadeout time high bits
+ * @fade_lo: fadeout time low bits
+ * @led_on: led is on
+ * @led_cdev: hvled class device
+ * @led_work: work to set the brightness of hvled
+ */
+struct ab5500_led {
+	u8 id;
+	u8 max_current;
+	u16 brt_val;
+	u16 fade_hi;
+	u16 fade_lo;
+	bool led_on;
+	struct led_classdev led_cdev;
+	struct work_struct led_work;
+};
+
+/**
+ * struct ab5500_hvleds
+ * @lock: lock to serialize access to hvleds
+ * @dev: device
+ * @pdata: platform data from board
+ * @leds: hvleds instances
+ * @hw_fade: this chip supports hardware fade
+ * @fade_auto: fade out leds automatically on this chip
+ * @fade_delay: fade out automatically after this delay time
+ */
+struct ab5500_hvleds {
+	struct mutex lock;
+	struct device *dev;
+	struct ab5500_hvleds_platform_data *pdata;
+	struct ab5500_led leds[AB5500_HVLEDS_MAX];
+	bool hw_fade;
+	bool fade_auto;
+	enum ab5500_fade_delay fade_delay;
+};
+
+static u8 ab5500_led_pwmduty_reg[AB5500_LED_MAX] = {
+			AB5500_LED0_PWM_DUTY,
+			AB5500_LED1_PWM_DUTY,
+			AB5500_LED2_PWM_DUTY,
+};
+
+static u8 ab5500_led_pwmfreq_reg[AB5500_LED_MAX] = {
+			AB5500_LED0_PWMFREQ,
+			AB5500_LED1_PWMFREQ,
+			AB5500_LED2_PWMFREQ,
+};
+
+static u8 ab5500_led_sinkctl_reg[AB5500_LED_MAX] = {
+			AB5500_LED0_SINKCTL,
+			AB5500_LED1_SINKCTL,
+			AB5500_LED2_SINKCTL
+};
+
+static u8 ab5500_led_fade_hi_reg[AB5500_LED_MAX] = {
+			AB5500_LED0_FADE_HI,
+			AB5500_LED1_FADE_HI,
+			AB5500_LED2_FADE_HI,
+};
+
+static u8 ab5500_led_fade_lo_reg[AB5500_LED_MAX] = {
+			AB5500_LED0_FADE_LO,
+			AB5500_LED1_FADE_LO,
+			AB5500_LED2_FADE_LO,
+};
+
+#define to_led(_x)	container_of(_x, struct ab5500_led, _x)
+
+static inline struct ab5500_hvleds *led_to_hvleds(struct ab5500_led *led)
+{
+	return container_of(led, struct ab5500_hvleds, leds[led->id]);
+}
+
+static int ab5500_led_enable(struct ab5500_hvleds *hvleds,
+		unsigned int led_id)
+{
+	int ret;
+
+	ret = abx500_mask_and_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_pwmduty_reg[led_id],
+			AB5500_LED_ENABLE_MASK,
+			AB5500_LED_ENABLE_MASK);
+	if (ret < 0)
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+				ab5500_led_pwmduty_reg[led_id], ret);
+
+	return ret;
+
+}
+
+static int ab5500_led_start_manual(struct ab5500_hvleds *hvleds)
+{
+	int ret;
+
+	mutex_lock(&hvleds->lock);
+
+	ret = abx500_mask_and_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			AB5500_LED_FADE_CTRL, AB5500_FADE_START_MASK,
+			AB5500_FADE_START_MASK);
+	if (ret < 0)
+		dev_err(hvleds->dev, "update reg 0x%x failed - %d\n",
+				AB5500_LED_FADE_CTRL, ret);
+
+	mutex_unlock(&hvleds->lock);
+
+	return ret;
+}
+
+static int ab5500_led_disable(struct ab5500_hvleds *hvleds,
+		unsigned int led_id)
+{
+	int ret;
+
+	ret = abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_pwmduty_reg[led_id] - 1, 0);
+	ret |= abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_pwmduty_reg[led_id], 0);
+	if (ret < 0)
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+			ab5500_led_pwmduty_reg[led_id], ret);
+
+	return ret;
+}
+
+static int ab5500_led_pwmduty_write(struct ab5500_hvleds *hvleds,
+			unsigned int led_id, u16 val)
+{
+	int ret;
+	u8 val_lsb = val & 0xFF;
+	u8 val_msb = (val & 0x300) >> 8;
+
+	mutex_lock(&hvleds->lock);
+
+	dev_dbg(hvleds->dev, "ab5500-leds: reg[%d] w val = %d\n"
+		"reg[%d] w val = %d\n",
+		ab5500_led_pwmduty_reg[led_id] - 1, val_lsb,
+		ab5500_led_pwmduty_reg[led_id], val_msb);
+
+	ret = abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_pwmduty_reg[led_id] - 1, val_lsb);
+	ret |= abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_pwmduty_reg[led_id], val_msb);
+	if (ret < 0)
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+			ab5500_led_pwmduty_reg[led_id], ret);
+
+	mutex_unlock(&hvleds->lock);
+
+	return ret;
+}
+
+static int ab5500_led_pwmfreq_write(struct ab5500_hvleds *hvleds,
+			unsigned int led_id, u8 val)
+{
+	int ret;
+
+	val = (val & 0x0F) << AB5500_LED_PWMFREQ_SHIFT;
+
+	mutex_lock(&hvleds->lock);
+
+	dev_dbg(hvleds->dev, "ab5500-leds: reg[%d] w val=%d\n",
+			ab5500_led_pwmfreq_reg[led_id], val);
+
+	ret = abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_pwmfreq_reg[led_id], val);
+	if (ret < 0)
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+			ab5500_led_pwmfreq_reg[led_id], ret);
+
+	mutex_unlock(&hvleds->lock);
+
+	return ret;
+}
+
+static int ab5500_led_sinkctl_write(struct ab5500_hvleds *hvleds,
+			unsigned int led_id, u8 val)
+{
+	int ret;
+
+	if (val > AB5500_LED_SINKCURR_MAX)
+		val = AB5500_LED_SINKCURR_MAX;
+
+	val = (val << AB5500_LED_SINKCURR_SHIFT);
+
+	dev_dbg(hvleds->dev, "ab5500-leds: reg[%d] w val=%d\n",
+			ab5500_led_sinkctl_reg[led_id], val);
+
+	mutex_lock(&hvleds->lock);
+
+	ret = abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_sinkctl_reg[led_id], val);
+	if (ret < 0)
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+			ab5500_led_sinkctl_reg[led_id], ret);
+
+	mutex_unlock(&hvleds->lock);
+
+	return ret;
+}
+
+static int ab5500_led_fade_write(struct ab5500_hvleds *hvleds,
+			unsigned int led_id, bool on, u16 val)
+{
+	int ret;
+	int val_lsb = val & 0xFF;
+	int val_msb = (val & 0x300) >> 8;
+	u8 *fade_reg;
+
+	if (on)
+		fade_reg = ab5500_led_fade_hi_reg;
+	else
+		fade_reg = ab5500_led_fade_lo_reg;
+
+	dev_dbg(hvleds->dev, "ab5500-leds: reg[%d] w val = %d\n"
+		"reg[%d] w val = %d\n",
+		fade_reg[led_id] - 1, val_lsb,
+		fade_reg[led_id], val_msb);
+
+	mutex_lock(&hvleds->lock);
+
+	ret = abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			fade_reg[led_id] - 1, val_lsb);
+	ret |= abx500_set_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			fade_reg[led_id], val_msb);
+	if (ret < 0)
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+			fade_reg[led_id], ret);
+
+	mutex_unlock(&hvleds->lock);
+
+	return ret;
+}
+
+static int ab5500_led_sinkctl_read(struct ab5500_hvleds *hvleds,
+			unsigned int led_id)
+{
+	int ret;
+	u8 val;
+
+	mutex_lock(&hvleds->lock);
+
+	ret = abx500_get_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			ab5500_led_sinkctl_reg[led_id], &val);
+	if (ret < 0) {
+		dev_err(hvleds->dev, "reg[%d] r failed: %d\n",
+			ab5500_led_sinkctl_reg[led_id], ret);
+		mutex_unlock(&hvleds->lock);
+		return ret;
+	}
+
+	val = (val & 0xF0) >> AB5500_LED_SINKCURR_SHIFT;
+
+	mutex_unlock(&hvleds->lock);
+
+	return val;
+}
+
+static void ab5500_led_brightness_set(struct led_classdev *led_cdev,
+			enum led_brightness brt_val)
+{
+	struct ab5500_led *led = to_led(led_cdev);
+
+	/* adjust LED_FULL to 10bit range */
+	brt_val &= LED_FULL;
+	led->brt_val = brt_val * AB5500_LED_INTENSITY_STEP;
+
+	schedule_work(&led->led_work);
+}
+
+static void ab5500_led_work(struct work_struct *led_work)
+{
+	struct ab5500_led *led = to_led(led_work);
+	struct ab5500_hvleds *hvleds = led_to_hvleds(led);
+
+	if (led->led_on == true) {
+		ab5500_led_pwmduty_write(hvleds, led->id, led->brt_val);
+		if (hvleds->hw_fade && led->brt_val) {
+			ab5500_led_enable(hvleds, led->id);
+			ab5500_led_start_manual(hvleds);
+		}
+	}
+}
+
+static ssize_t ab5500_led_show_current(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	int led_curr = 0;
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct ab5500_led *led = to_led(led_cdev);
+	struct ab5500_hvleds *hvleds = led_to_hvleds(led);
+
+	led_curr = ab5500_led_sinkctl_read(hvleds, led->id);
+
+	if (led_curr < 0)
+		return led_curr;
+
+	return sprintf(buf, "%d\n", led_curr);
+}
+
+static ssize_t ab5500_led_store_current(struct device *dev,
+			struct device_attribute *attr,
+			const char *buf, size_t len)
+{
+	int ret;
+	unsigned long led_curr;
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct ab5500_led *led = to_led(led_cdev);
+	struct ab5500_hvleds *hvleds = led_to_hvleds(led);
+
+	if (strict_strtoul(buf, 0, &led_curr))
+		return -EINVAL;
+
+	if (led_curr > led->max_current)
+		led_curr = led->max_current;
+
+	ret = ab5500_led_sinkctl_write(hvleds, led->id, led_curr);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static ssize_t ab5500_led_store_fade_auto(struct device *dev,
+			struct device_attribute *attr,
+			const char *buf, size_t len)
+{
+	int ret;
+	u8 fade_ctrl = 0;
+	unsigned long fade_auto;
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct ab5500_led *led = to_led(led_cdev);
+	struct ab5500_hvleds *hvleds = led_to_hvleds(led);
+
+	if (strict_strtoul(buf, 0, &fade_auto))
+		return -EINVAL;
+
+	if (fade_auto > 1) {
+		dev_err(hvleds->dev, "invalid mode\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&hvleds->lock);
+
+	ret = abx500_get_register_interruptible(
+			hvleds->dev, AB5500_BANK_LED,
+			AB5500_LED_FADE_CTRL, &fade_ctrl);
+	if (ret < 0) {
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+				AB5500_LED_FADE_CTRL, ret);
+		goto unlock_and_return;
+	}
+
+	/* manual mode */
+	if (fade_auto == false) {
+		fade_ctrl &= ~(AB5500_LED_FADE_ENABLE(led->id));
+		if (!(fade_ctrl & AB5500_FADE_ON_MASK))
+			fade_ctrl = 0;
+
+		ret = ab5500_led_disable(hvleds, led->id);
+		if (ret < 0)
+			goto unlock_and_return;
+	} else {
+		/* set led auto enable bit */
+		fade_ctrl |= AB5500_FADE_MODE_MASK;
+		fade_ctrl |= AB5500_LED_FADE_ENABLE(led->id);
+
+		/* set fade delay */
+		fade_ctrl &= ~AB5500_FADE_DELAY_MASK;
+		fade_ctrl |= hvleds->fade_delay << AB5500_FADE_DELAY_SHIFT;
+
+		/* set fade start manual */
+		fade_ctrl |= AB5500_FADE_START_MASK;
+
+		/* enble corresponding led */
+		ret = ab5500_led_enable(hvleds, led->id);
+		if (ret < 0)
+			goto unlock_and_return;
+
+	}
+
+	ret = abx500_set_register_interruptible(
+				hvleds->dev, AB5500_BANK_LED,
+				AB5500_LED_FADE_CTRL, fade_ctrl);
+	if (ret < 0) {
+		dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+			       AB5500_LED_FADE_CTRL, ret);
+		goto unlock_and_return;
+	}
+
+	hvleds->fade_auto = fade_auto;
+
+	ret = len;
+
+unlock_and_return:
+	mutex_unlock(&hvleds->lock);
+
+	return ret;
+}
+
+static ssize_t ab5500_led_show_fade_auto(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct ab5500_led *led = to_led(led_cdev);
+	struct ab5500_hvleds *hvleds = led_to_hvleds(led);
+
+	return sprintf(buf, "%d\n", hvleds->fade_auto);
+}
+
+static ssize_t ab5500_led_store_fade_delay(struct device *dev,
+			struct device_attribute *attr,
+			const char *buf, size_t len)
+{
+	unsigned long fade_delay;
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct ab5500_led *led = to_led(led_cdev);
+	struct ab5500_hvleds *hvleds = led_to_hvleds(led);
+
+	if (strict_strtoul(buf, 0, &fade_delay))
+		return -EINVAL;
+
+	if (fade_delay > AB5500_FADE_DELAY_TWOSEC) {
+		dev_err(hvleds->dev, "invalid mode\n");
+		return -EINVAL;
+	}
+
+	hvleds->fade_delay = fade_delay;
+
+	return len;
+}
+
+/* led class device attributes */
+static DEVICE_ATTR(led_current, S_IRUGO | S_IWUGO,
+	       ab5500_led_show_current, ab5500_led_store_current);
+static DEVICE_ATTR(fade_auto, S_IRUGO | S_IWUGO,
+	       ab5500_led_show_fade_auto, ab5500_led_store_fade_auto);
+static DEVICE_ATTR(fade_delay, S_IRUGO | S_IWUGO,
+	       NULL, ab5500_led_store_fade_delay);
+
+static int ab5500_led_init_registers(struct ab5500_hvleds *hvleds)
+{
+	int ret = 0;
+	unsigned int led_id;
+
+	/*  fade - manual : dur mid : pwm duty mid */
+	if (!hvleds->hw_fade) {
+		ret = abx500_set_register_interruptible(
+				hvleds->dev, AB5500_BANK_LED,
+				AB5500_LED_REG_ENABLE, true);
+		if (ret < 0) {
+			dev_err(hvleds->dev, "reg[%d] w failed: %d\n",
+					AB5500_LED_REG_ENABLE, ret);
+			return ret;
+		}
+	}
+
+	for (led_id = 0; led_id < AB5500_HVLEDS_MAX; led_id++) {
+		if (hvleds->leds[led_id].led_on == false)
+			continue;
+
+		ret = ab5500_led_sinkctl_write(
+				hvleds, led_id,
+				hvleds->leds[led_id].max_current);
+		if (ret < 0)
+			return ret;
+
+		if (hvleds->hw_fade) {
+			ret = ab5500_led_pwmfreq_write(
+					hvleds, led_id,
+					AB5500_LED_PWMFREQ_MAX / 2);
+			if (ret < 0)
+				return ret;
+
+			/* fade high intensity */
+			ret = ab5500_led_fade_write(
+					hvleds, led_id, true,
+					hvleds->leds[led_id].fade_hi);
+			if (ret < 0)
+				return ret;
+
+			/* fade low intensity */
+			ret = ab5500_led_fade_write(
+					hvleds, led_id, false,
+					hvleds->leds[led_id].fade_lo);
+			if (ret < 0)
+				return ret;
+		}
+
+		/* init led off */
+		ret |= ab5500_led_pwmduty_write(
+				hvleds, led_id, AB5500_LED_INTENSITY_OFF);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int ab5500_led_register_leds(struct device *dev,
+			struct ab5500_hvleds_platform_data *pdata,
+			struct ab5500_hvleds *hvleds)
+{
+	int i_led;
+	int ret = 0;
+	struct ab5500_led_conf *pled;
+	struct ab5500_led *led;
+
+	hvleds->dev = dev;
+	hvleds->pdata = pdata;
+
+	if (abx500_get_chip_id(dev) == AB5500_2_0)
+		hvleds->hw_fade = true;
+	else
+		hvleds->hw_fade = false;
+
+	for (i_led = 0; i_led < AB5500_HVLEDS_MAX; i_led++) {
+		pled = &pdata->leds[i_led];
+		led = &hvleds->leds[i_led];
+
+		INIT_WORK(&led->led_work, ab5500_led_work);
+
+		led->id = pled->led_id;
+		led->max_current = pled->max_current;
+		led->led_on = pled->led_on;
+		led->led_cdev.name = pled->name;
+		led->led_cdev.brightness_set = ab5500_led_brightness_set;
+
+		/* Provide interface only for enabled LEDs */
+		if (led->led_on == false)
+			continue;
+
+		if (hvleds->hw_fade) {
+			led->fade_hi = (pled->fade_hi & LED_FULL);
+			led->fade_hi *= AB5500_LED_INTENSITY_STEP;
+			led->fade_lo = (pled->fade_lo & LED_FULL);
+			led->fade_lo *= AB5500_LED_INTENSITY_STEP;
+		}
+
+		ret = led_classdev_register(dev, &led->led_cdev);
+		if (ret < 0) {
+			dev_err(dev, "Register led class failed: %d\n", ret);
+			goto bailout1;
+		}
+
+		ret = device_create_file(led->led_cdev.dev,
+						&dev_attr_led_current);
+		if (ret < 0) {
+			dev_err(dev, "sysfs device creation failed: %d\n", ret);
+			goto bailout2;
+		}
+
+		if (hvleds->hw_fade) {
+			ret = device_create_file(led->led_cdev.dev,
+					&dev_attr_fade_auto);
+			if (ret < 0) {
+				dev_err(dev, "sysfs device "
+					"creation failed: %d\n", ret);
+				goto bailout3;
+			}
+
+			ret = device_create_file(led->led_cdev.dev,
+					&dev_attr_fade_delay);
+			if (ret < 0) {
+				dev_err(dev, "sysfs device "
+					"creation failed: %d\n", ret);
+				goto bailout4;
+			}
+		}
+	}
+
+	return ret;
+	for (; i_led >= 0; i_led--) {
+		if (hvleds->leds[i_led].led_on == false)
+			continue;
+
+		if (hvleds->hw_fade) {
+			device_remove_file(hvleds->leds[i_led].led_cdev.dev,
+					&dev_attr_fade_delay);
+bailout4:
+			device_remove_file(hvleds->leds[i_led].led_cdev.dev,
+					&dev_attr_fade_auto);
+		}
+bailout3:
+		device_remove_file(hvleds->leds[i_led].led_cdev.dev,
+				&dev_attr_led_current);
+bailout2:
+		led_classdev_unregister(&hvleds->leds[i_led].led_cdev);
+bailout1:
+		cancel_work_sync(&hvleds->leds[i_led].led_work);
+	}
+	return ret;
+}
+
+static int __devinit ab5500_hvleds_probe(struct platform_device *pdev)
+{
+	struct ab5500_hvleds_platform_data *pdata = pdev->dev.platform_data;
+	struct ab5500_hvleds *hvleds = NULL;
+	int ret = 0, i;
+
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "platform data required\n");
+		ret = -ENODEV;
+		goto err_out;
+	}
+
+	hvleds = kzalloc(sizeof(struct ab5500_hvleds), GFP_KERNEL);
+	if (hvleds == NULL) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+
+	mutex_init(&hvleds->lock);
+
+	/* init leds data and register led_classdev */
+	ret = ab5500_led_register_leds(&pdev->dev, pdata, hvleds);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "leds registration failed\n");
+		goto err_out;
+	}
+
+	/* init device registers and set initial led current */
+	ret = ab5500_led_init_registers(hvleds);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "reg init failed: %d\n", ret);
+		goto err_reg_init;
+	}
+
+	if (hvleds->hw_fade)
+		dev_info(&pdev->dev, "v2 enabled\n");
+	else
+		dev_info(&pdev->dev, "v1 enabled\n");
+
+	return ret;
+
+err_reg_init:
+	for (i = 0; i < AB5500_HVLEDS_MAX; i++) {
+		struct ab5500_led *led = &hvleds->leds[i];
+
+		if (led->led_on == false)
+			continue;
+
+		device_remove_file(led->led_cdev.dev, &dev_attr_led_current);
+		if (hvleds->hw_fade) {
+			device_remove_file(led->led_cdev.dev,
+					&dev_attr_fade_auto);
+			device_remove_file(led->led_cdev.dev,
+					&dev_attr_fade_delay);
+		}
+		led_classdev_unregister(&led->led_cdev);
+		cancel_work_sync(&led->led_work);
+	}
+err_out:
+	kfree(hvleds);
+	return ret;
+}
+
+static int __devexit ab5500_hvleds_remove(struct platform_device *pdev)
+{
+	struct ab5500_hvleds *hvleds = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < AB5500_HVLEDS_MAX; i++) {
+		struct ab5500_led *led = &hvleds->leds[i];
+
+		if (led->led_on == false)
+			continue;
+
+		device_remove_file(led->led_cdev.dev, &dev_attr_led_current);
+		if (hvleds->hw_fade) {
+			device_remove_file(led->led_cdev.dev,
+					&dev_attr_fade_auto);
+			device_remove_file(led->led_cdev.dev,
+					&dev_attr_fade_delay);
+		}
+		led_classdev_unregister(&led->led_cdev);
+		cancel_work_sync(&led->led_work);
+	}
+	kfree(hvleds);
+	return 0;
+}
+
+static struct platform_driver ab5500_hvleds_driver = {
+	.driver   = {
+		   .name = AB5500LED_NAME,
+		   .owner = THIS_MODULE,
+	},
+	.probe    = ab5500_hvleds_probe,
+	.remove   = __devexit_p(ab5500_hvleds_remove),
+};
+
+static int __init ab5500_hvleds_module_init(void)
+{
+	return platform_driver_register(&ab5500_hvleds_driver);
+}
+
+static void __exit ab5500_hvleds_module_exit(void)
+{
+	platform_driver_unregister(&ab5500_hvleds_driver);
+}
+
+module_init(ab5500_hvleds_module_init);
+module_exit(ab5500_hvleds_module_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>");
+MODULE_DESCRIPTION("Driver for AB5500 HVLED");
diff --git a/include/linux/mfd/abx500/ab5500-leds.h b/include/linux/mfd/abx500/ab5500-leds.h
new file mode 100644
index 0000000..54ba53a
--- /dev/null
+++ b/include/linux/mfd/abx500/ab5500-leds.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2011 ST-Ericsson SA.
+ *
+ * License Terms: GNU General Public License v2
+ *
+ * Simple driver for HVLED in ST-Ericsson AB5500 Analog baseband Controller
+ *
+ * Author: Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
+ */
+
+#ifndef _LINUX_LEDS_AB5500_H__
+#define _LINUX_LEDS_AB5500_H__
+
+#define AB5500_HVLED0		0
+#define AB5500_HVLED1		1
+#define AB5500_HVLED2		2
+#define AB5500_HVLEDS_MAX	3
+
+enum ab5500_fade_delay {
+	AB5500_FADE_DELAY_BYPASS = 0,
+	AB5500_FADE_DELAY_HALFSEC,
+	AB5500_FADE_DELAY_ONESEC,
+	AB5500_FADE_DELAY_TWOSEC
+};
+
+/**
+ * struct ab5500_led_conf
+ * @name: hvled instance name
+ * @led_id: hvled index
+ * @max_current: max current permitted for hvled instance
+ * @fade_hi: fadeout time high bits
+ * @fade_lo: fadeout time low bits
+ * @led_on: led is on by default
+ */
+struct ab5500_led_conf {
+	char *name;
+	u8 led_id;
+	u8 max_current;
+	u8 fade_hi;
+	u8 fade_lo;
+	bool led_on;
+};
+
+/**
+ * struct ab5500_hvleds_platform_data
+ * @hw_fade: fadeout feature present in hardware
+ * @leds: hvleds instances
+ */
+struct ab5500_hvleds_platform_data {
+	bool hw_fade;
+	struct ab5500_led_conf leds[AB5500_HVLEDS_MAX];
+};
+
+#endif /* _LINUX_LEDS_AB5500_H__ */
-- 
1.7.3.2


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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-02 13:56 [PATCH] leds: driver for ab5500 high voltage leds Linus Walleij
@ 2011-12-03 21:49 ` Mark Brown
  2011-12-07 14:16   ` Linus Walleij
  2011-12-03 22:49 ` Denis Kuzmenko
  2011-12-19 11:26 ` Samuel Ortiz
  2 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2011-12-03 21:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Richard Purdie, linux-kernel, Andrew Morton, Samuel Ortiz,
	Shreshtha Kumar Sahu, Linus Walleij

On Fri, Dec 02, 2011 at 02:56:22PM +0100, Linus Walleij wrote:

> +What:		/sys/class/leds/<led>/led_current
> +Date:		December 2011
> +KernelVersion:	3.3
> +Contact:	Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
> +Description:
> +		Read and set the current limit for the LED. Value in
> +		milliampere.

This sounds like a platform data thing - normally if you get a current
limit wrong you're heading for magic smoke so it's not something you
want to be playing with at runtime from userspace.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-02 13:56 [PATCH] leds: driver for ab5500 high voltage leds Linus Walleij
  2011-12-03 21:49 ` Mark Brown
@ 2011-12-03 22:49 ` Denis Kuzmenko
  2011-12-07 14:07   ` Linus Walleij
  2011-12-19 11:26 ` Samuel Ortiz
  2 siblings, 1 reply; 18+ messages in thread
From: Denis Kuzmenko @ 2011-12-03 22:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Richard Purdie, linux-kernel, Andrew Morton, Samuel Ortiz,
	Shreshtha Kumar Sahu, Linus Walleij

Hi,
On 12/02/2011 03:56 PM, Linus Walleij wrote:
> From: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
> 
> Simple HV LED controller driver for AB5500v1.0 MFD chips
> 
> Signed-off-by: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
> Reviewed-by: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>
> Tested-by: Naga Radesh Y <naga.radheshy@stericsson.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
(snip)
> diff --git a/drivers/leds/leds-ab5500.c b/drivers/leds/leds-ab5500.c
(snip)
> +/*
> + * Driver for HVLED in ST-Ericsson AB5500 analog baseband controller
> + *
> + * This chip can drive upto 3 leds, of upto 40mA of led sink current.

Can't get these are High Voltage or High Current LEDs?
If Voltage why haven't you wrote about supplied voltage but wrote about
max. current?

-- 
Best regards, Denis Kuzmenko.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-03 22:49 ` Denis Kuzmenko
@ 2011-12-07 14:07   ` Linus Walleij
  2011-12-07 18:01     ` Denis Kuzmenko
  0 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2011-12-07 14:07 UTC (permalink / raw)
  To: Denis Kuzmenko
  Cc: Linus Walleij, Richard Purdie, linux-kernel, Andrew Morton,
	Samuel Ortiz, Shreshtha Kumar Sahu

On Sat, Dec 3, 2011 at 11:49 PM, Denis Kuzmenko <linux@solonet.org.ua> wrote:

> (snip)
>> diff --git a/drivers/leds/leds-ab5500.c b/drivers/leds/leds-ab5500.c
> (snip)
>> +/*
>> + * Driver for HVLED in ST-Ericsson AB5500 analog baseband controller
>> + *
>> + * This chip can drive upto 3 leds, of upto 40mA of led sink current.
>
> Can't get these are High Voltage or High Current LEDs?
> If Voltage why haven't you wrote about supplied voltage but wrote about
> max. current?

The three channels are indeed high-voltage LEDs, they supply
up to 20V from a supply voltage of some standard mobile handset
battery at say 3.8 V or so. But there is no register to control the
voltage or anything like that.

My naive understanding is that you set the current limit and then
the HV transformer (I guess this is a buck converter of some kind)
will raise the voltage level until it either (A) cannot raise it any more
at c:a 20V or (b) the current limit is reached.

I suspect this is because for LEDs of this type you get a
specified current but the voltage just has to be "high enough"
to break through some diode barrier threshold or so. After
that intensity is controlled by limiting the current.

Does this suffice as explanation...?

Yours,
Linus Walleij

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-03 21:49 ` Mark Brown
@ 2011-12-07 14:16   ` Linus Walleij
  2011-12-07 15:47     ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2011-12-07 14:16 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Richard Purdie, linux-kernel, Andrew Morton,
	Samuel Ortiz, Shreshtha Kumar Sahu

On Sat, Dec 3, 2011 at 10:49 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

> On Fri, Dec 02, 2011 at 02:56:22PM +0100, Linus Walleij wrote:
>
>> +What:                /sys/class/leds/<led>/led_current
>> +Date:                December 2011
>> +KernelVersion:       3.3
>> +Contact:     Shreshtha Kumar SAHU <shreshthakumar.sahu@stericsson.com>
>> +Description:
>> +             Read and set the current limit for the LED. Value in
>> +             milliampere.
>
> This sounds like a platform data thing - normally if you get a current
> limit wrong you're heading for magic smoke so it's not something you
> want to be playing with at runtime from userspace.

There is a platform thing, it sets the upper limit for the
current:

+/**
+ * struct ab5500_led
+ * @id: hvled index
+ * @max_current: max current permitted for hvled instance
+ * @brt_val: brightness value set for hvled
+ * @fade_hi: fadeout time high bits
+ * @fade_lo: fadeout time low bits
+ * @led_on: led is on
+ * @led_cdev: hvled class device
+ * @led_work: work to set the brightness of hvled
+ */
+struct ab5500_led {
+       u8 id;
+       u8 max_current;
+       u16 brt_val;
+       u16 fade_hi;
+       u16 fade_lo;
+       bool led_on;
+       struct led_classdev led_cdev;
+       struct work_struct led_work;
+};

Here max_current is the max current allowed by the platform.

Then in the sysfs interface:

+       if (led_curr > led->max_current)
+               led_curr = led->max_current;

You are only allowed to set a lower value than the max value.

Yours,
Linus Walleij

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-07 14:16   ` Linus Walleij
@ 2011-12-07 15:47     ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2011-12-07 15:47 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, Richard Purdie, linux-kernel, Andrew Morton,
	Samuel Ortiz, Shreshtha Kumar Sahu

On Wed, Dec 07, 2011 at 03:16:06PM +0100, Linus Walleij wrote:

> Then in the sysfs interface:

> +       if (led_curr > led->max_current)
> +               led_curr = led->max_current;

> You are only allowed to set a lower value than the max value.

Oh, that's surprising - my expectation had been that the standard
brightness setting would be managing the current limit and the user
wouldn't need to control it separately.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-07 14:07   ` Linus Walleij
@ 2011-12-07 18:01     ` Denis Kuzmenko
  2011-12-08  0:00       ` Andrew Morton
  0 siblings, 1 reply; 18+ messages in thread
From: Denis Kuzmenko @ 2011-12-07 18:01 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linus Walleij, Richard Purdie, linux-kernel, Andrew Morton,
	Samuel Ortiz, Shreshtha Kumar Sahu

On 12/07/2011 04:07 PM, Linus Walleij wrote:
> On Sat, Dec 3, 2011 at 11:49 PM, Denis Kuzmenko <linux@solonet.org.ua> wrote:
> 
>> (snip)
>>> diff --git a/drivers/leds/leds-ab5500.c b/drivers/leds/leds-ab5500.c
>> (snip)
>>> +/*
>>> + * Driver for HVLED in ST-Ericsson AB5500 analog baseband controller
>>> + *
>>> + * This chip can drive upto 3 leds, of upto 40mA of led sink current.
>>
>> Can't get these are High Voltage or High Current LEDs?
>> If Voltage why haven't you wrote about supplied voltage but wrote about
>> max. current?
> 
> The three channels are indeed high-voltage LEDs, they supply
> up to 20V from a supply voltage of some standard mobile handset
> battery at say 3.8 V or so. But there is no register to control the
> voltage or anything like that.
> 
> My naive understanding is that you set the current limit and then
> the HV transformer (I guess this is a buck converter of some kind)
> will raise the voltage level until it either (A) cannot raise it any more
> at c:a 20V or (b) the current limit is reached.
> 
> I suspect this is because for LEDs of this type you get a
> specified current but the voltage just has to be "high enough"
> to break through some diode barrier threshold or so. After
> that intensity is controlled by limiting the current.
> 
> Does this suffice as explanation...?
> 
> Yours,
> Linus Walleij

Yeah, It does, thank you.
Does it worth to make additional comment in code to avoid further questions?

-- 
Best regards, Denis Kuzmenko.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-07 18:01     ` Denis Kuzmenko
@ 2011-12-08  0:00       ` Andrew Morton
  2011-12-08  1:19         ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Morton @ 2011-12-08  0:00 UTC (permalink / raw)
  To: Denis Kuzmenko
  Cc: Linus Walleij, Linus Walleij, Richard Purdie, linux-kernel,
	Samuel Ortiz, Shreshtha Kumar Sahu, Mark Brown

On Wed, 07 Dec 2011 20:01:46 +0200
Denis Kuzmenko <linux@solonet.org.ua> wrote:

> > The three channels are indeed high-voltage LEDs, they supply
> > up to 20V from a supply voltage of some standard mobile handset
> > battery at say 3.8 V or so. But there is no register to control the
> > voltage or anything like that.
> > 
> > My naive understanding is that you set the current limit and then
> > the HV transformer (I guess this is a buck converter of some kind)
> > will raise the voltage level until it either (A) cannot raise it any more
> > at c:a 20V or (b) the current limit is reached.
> > 
> > I suspect this is because for LEDs of this type you get a
> > specified current but the voltage just has to be "high enough"
> > to break through some diode barrier threshold or so. After
> > that intensity is controlled by limiting the current.
> > 
> > Does this suffice as explanation...?
> > 
> > Yours,
> > Linus Walleij
> 
> Yeah, It does, thank you.
> Does it worth to make additional comment in code to avoid further questions?

Yes, I think that the fact that a reviewer asked a question such as
this is a reliable sign that subsequent readers will be wondering the
same thing, so it is an "oops it needs a comment" signal.

I won't do anything with this patch yet - I'm waiting to see if Mark
stops being surprised ;)


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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-08  0:00       ` Andrew Morton
@ 2011-12-08  1:19         ` Mark Brown
  2011-12-14 10:23           ` Shreshtha Kumar SAHU
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2011-12-08  1:19 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Denis Kuzmenko, Linus Walleij, Linus Walleij, Richard Purdie,
	linux-kernel, Samuel Ortiz, Shreshtha Kumar Sahu

On Wed, Dec 07, 2011 at 04:00:02PM -0800, Andrew Morton wrote:
> Denis Kuzmenko <linux@solonet.org.ua> wrote:

Hrm, for some reason lots of the mails here didn't reach my inbox...
anyway.

> > > My naive understanding is that you set the current limit and then
> > > the HV transformer (I guess this is a buck converter of some kind)

Boost convertor.  A buck is always step down I beleive.

> > > will raise the voltage level until it either (A) cannot raise it any more
> > > at c:a 20V or (b) the current limit is reached.

> > > I suspect this is because for LEDs of this type you get a
> > > specified current but the voltage just has to be "high enough"
> > > to break through some diode barrier threshold or so. After
> > > that intensity is controlled by limiting the current.

> > > Does this suffice as explanation...?

OK, that's exactly the sort of hardware I thought you were driving.
There's several drivers for this type of LED in the kernel already.

> I won't do anything with this patch yet - I'm waiting to see if Mark
> stops being surprised ;)

I'm still surprised I'm afraid.  The voltage is generally fixed by the
platform (approximately anyway) to be whatever is needed to get the LEDs
to start doing something and the current is varied to control the
brightness with an upper limit configured based on what the LEDs can
carry and what is sane for the physical design (blinding users tends not
to be popular).  The funciton of the current regulator in the system is
to constrain the brightness of the LED so that it doesn't just go to
maximum brightness without burning power on a resistive load.

This means that usually the current control does map fairly directly
onto the LED API brightness, normally with a scaling as the eye
percieves things on a log scale but we expect brightness to be linear.
This scaling may be pre-done by the choice of current limits in the
current regulator.

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

* RE: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-08  1:19         ` Mark Brown
@ 2011-12-14 10:23           ` Shreshtha Kumar SAHU
  2011-12-14 11:37             ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Shreshtha Kumar SAHU @ 2011-12-14 10:23 UTC (permalink / raw)
  To: Mark Brown, Andrew Morton
  Cc: Denis Kuzmenko, Linus Walleij, Linus WALLEIJ, Richard Purdie,
	linux-kernel, Samuel Ortiz, Shreshtha Kumar SAHU

Sorry for replying late, I was on vacation.
To minimize fragmentaion in different mails, I am replying all in this mail.

HVLED controller supports up to 20V on each input. In terms of VBAT = max 4.5V,
20V is considered as high voltage. The voltage must be provided by external
source, e.g. a boost converter. These converters can be dynamically controlled
but generally these are configured to provide fixed output voltage and done
during system design.

Given a constant external voltage LED(s) in series, max current is fixed based
on platform. In U5500 platform it is done as -
        ...
        max_current = 10, /* wrong value may damage h/w */
        ...

The max current flowing through pin will be limited in HVLED controller by:
        MIN(voltage source current drive capability; Set max current)

The actual amount of current flowing hence brightness variation can then be
altered using PWM, which is also the case in present driver.

Hence the idea of current control sysfs interface is provided to further
decrease the max current through the LED(s) if required for saving power
(but can never exceed the max current set by the platform).
After fixing the current, "brightness" interface can be used to control
brightness which internally cofigures the PWM register.

Hope adding this information in documentation will help avoiding confusion
for the users.

Regards,
Shreshtha

-----Original Message-----
From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] 
Sent: Thursday, December 08, 2011 6:49 AM
To: Andrew Morton
Cc: Denis Kuzmenko; Linus Walleij; Linus WALLEIJ; Richard Purdie; linux-kernel@vger.kernel.org; Samuel Ortiz; Shreshtha Kumar SAHU
Subject: Re: [PATCH] leds: driver for ab5500 high voltage leds

On Wed, Dec 07, 2011 at 04:00:02PM -0800, Andrew Morton wrote:
> Denis Kuzmenko <linux@solonet.org.ua> wrote:

Hrm, for some reason lots of the mails here didn't reach my inbox...
anyway.

> > > My naive understanding is that you set the current limit and then
> > > the HV transformer (I guess this is a buck converter of some kind)

Boost convertor.  A buck is always step down I beleive.

> > > will raise the voltage level until it either (A) cannot raise it any more
> > > at c:a 20V or (b) the current limit is reached.

> > > I suspect this is because for LEDs of this type you get a
> > > specified current but the voltage just has to be "high enough"
> > > to break through some diode barrier threshold or so. After
> > > that intensity is controlled by limiting the current.

> > > Does this suffice as explanation...?

OK, that's exactly the sort of hardware I thought you were driving.
There's several drivers for this type of LED in the kernel already.

> I won't do anything with this patch yet - I'm waiting to see if Mark
> stops being surprised ;)

I'm still surprised I'm afraid.  The voltage is generally fixed by the
platform (approximately anyway) to be whatever is needed to get the LEDs
to start doing something and the current is varied to control the
brightness with an upper limit configured based on what the LEDs can
carry and what is sane for the physical design (blinding users tends not
to be popular).  The funciton of the current regulator in the system is
to constrain the brightness of the LED so that it doesn't just go to
maximum brightness without burning power on a resistive load.

This means that usually the current control does map fairly directly
onto the LED API brightness, normally with a scaling as the eye
percieves things on a log scale but we expect brightness to be linear.
This scaling may be pre-done by the choice of current limits in the
current regulator.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-14 10:23           ` Shreshtha Kumar SAHU
@ 2011-12-14 11:37             ` Mark Brown
  2011-12-14 14:01               ` Shreshtha Kumar SAHU
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2011-12-14 11:37 UTC (permalink / raw)
  To: Shreshtha Kumar SAHU
  Cc: Andrew Morton, Denis Kuzmenko, Linus Walleij, Linus WALLEIJ,
	Richard Purdie, linux-kernel, Samuel Ortiz

On Wed, Dec 14, 2011 at 11:23:58AM +0100, Shreshtha Kumar SAHU wrote:

> The actual amount of current flowing hence brightness variation can then be
> altered using PWM, which is also the case in present driver.

> Hence the idea of current control sysfs interface is provided to further
> decrease the max current through the LED(s) if required for saving power
> (but can never exceed the max current set by the platform).
> After fixing the current, "brightness" interface can be used to control
> brightness which internally cofigures the PWM register.

The above basically sounds like you're using the current limit as a way
of adjusting the maximum brightness dynamically for power saving
reasons.  This doesn't sound like something that individual drivers
should be doing, it sounds like something that either the subsystem or
userspace code ought to be implementing.  It'd seem more natural for the
driver to scale both the PWM and current settings with brightness.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-14 11:37             ` Mark Brown
@ 2011-12-14 14:01               ` Shreshtha Kumar SAHU
  2011-12-14 14:47                 ` Mark Brown
  2011-12-14 18:34                 ` Linus Walleij
  0 siblings, 2 replies; 18+ messages in thread
From: Shreshtha Kumar SAHU @ 2011-12-14 14:01 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andrew Morton, Denis Kuzmenko, Linus Walleij, Linus WALLEIJ,
	Richard Purdie, linux-kernel, Samuel Ortiz

On Wed, Dec 14, 2011 at 12:37:15 +0100, Mark Brown wrote:
> On Wed, Dec 14, 2011 at 11:23:58AM +0100, Shreshtha Kumar SAHU wrote:
> 
> > The actual amount of current flowing hence brightness variation can then be
> > altered using PWM, which is also the case in present driver.
> 
> > Hence the idea of current control sysfs interface is provided to further
> > decrease the max current through the LED(s) if required for saving power
> > (but can never exceed the max current set by the platform).
> > After fixing the current, "brightness" interface can be used to control
> > brightness which internally cofigures the PWM register.
> 
> The above basically sounds like you're using the current limit as a way
> of adjusting the maximum brightness dynamically for power saving
> reasons.  This doesn't sound like something that individual drivers
> should be doing, it sounds like something that either the subsystem or
> userspace code ought to be implementing. 

As far as I know some boost converters can be controlled dynamically for
power optimized operation by PMIC. Yes exactly as you stated, I wanted to
provide something similar as done to boost converters by exposing current
control via sysfs. Otherwise this current control from sysfs can be
completely ignored by the user and simply use brightness for LED control.

> It'd seem more natural for the driver to scale both the PWM and current
> settings with brightness.
I will try to implement this. In AB5500, PWM provides 1024 steps from 0 to
full current, and current 16 steps from 2.5mA to 40mA. I need to derive a
way to optmially change both PWM and current to get requested brigtness in
range of 0-255 (i.e. LED_OFF to LED_FULL).
Bur I fear that changing both PWM and current may lead to non linear
variation and equation needs to be derived to get it correct which will
add complexity to ab5500_led_brightness_set and hence API brightness.
In addition if platform sets max current to 10mA then brightness_set()
will have only 4 steps for current variation and in thus an overhead if
we combined current and PWM control calculation.
Please suggest.

Regards,
Shreshtha




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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-14 14:01               ` Shreshtha Kumar SAHU
@ 2011-12-14 14:47                 ` Mark Brown
  2011-12-14 18:34                 ` Linus Walleij
  1 sibling, 0 replies; 18+ messages in thread
From: Mark Brown @ 2011-12-14 14:47 UTC (permalink / raw)
  To: Shreshtha Kumar SAHU
  Cc: Andrew Morton, Denis Kuzmenko, Linus Walleij, Linus WALLEIJ,
	Richard Purdie, linux-kernel, Samuel Ortiz

On Wed, Dec 14, 2011 at 07:31:55PM +0530, Shreshtha Kumar SAHU wrote:

> As far as I know some boost converters can be controlled dynamically for
> power optimized operation by PMIC. Yes exactly as you stated, I wanted to
> provide something similar as done to boost converters by exposing current
> control via sysfs. Otherwise this current control from sysfs can be
> completely ignored by the user and simply use brightness for LED control.

Well, there's also devices that don't bother with the PWM and only
implement brightness control via the current limit.

> > It'd seem more natural for the driver to scale both the PWM and current
> > settings with brightness.

> I will try to implement this. In AB5500, PWM provides 1024 steps from 0 to
> full current, and current 16 steps from 2.5mA to 40mA. I need to derive a
> way to optmially change both PWM and current to get requested brigtness in
> range of 0-255 (i.e. LED_OFF to LED_FULL).

> Bur I fear that changing both PWM and current may lead to non linear
> variation and equation needs to be derived to get it correct which will
> add complexity to ab5500_led_brightness_set and hence API brightness.
> In addition if platform sets max current to 10mA then brightness_set()
> will have only 4 steps for current variation and in thus an overhead if
> we combined current and PWM control calculation.

> Please suggest.

It doesn't seem like it should be *too* complex a problem to solve,
it'll depend on how the PWM and the current scale.  If they're both nice
smooth log scales it should be easy enough.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-14 14:01               ` Shreshtha Kumar SAHU
  2011-12-14 14:47                 ` Mark Brown
@ 2011-12-14 18:34                 ` Linus Walleij
  2011-12-15  7:11                   ` Mark Brown
  1 sibling, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2011-12-14 18:34 UTC (permalink / raw)
  To: Shreshtha Kumar SAHU
  Cc: Mark Brown, Andrew Morton, Denis Kuzmenko, Linus WALLEIJ,
	Richard Purdie, linux-kernel, Samuel Ortiz

On Wed, Dec 14, 2011 at 3:01 PM, Shreshtha Kumar SAHU
<shreshthakumar.sahu@stericsson.com> wrote:

> I will try to implement this. In AB5500, PWM provides 1024 steps from 0 to
> full current, and current 16 steps from 2.5mA to 40mA. I need to derive a
> way to optmially change both PWM and current to get requested brigtness in
> range of 0-255 (i.e. LED_OFF to LED_FULL).
> Bur I fear that changing both PWM and current may lead to non linear
> variation and equation needs to be derived to get it correct which will
> add complexity to ab5500_led_brightness_set and hence API brightness.
> In addition if platform sets max current to 10mA then brightness_set()
> will have only 4 steps for current variation and in thus an overhead if
> we combined current and PWM control calculation.

Don't fear this, it sounds like fun to me :-D

A simple linearization table with 256 steps 0.255 vs corresponding
16-bit max current and PWM settings could hide all the complexity
we have I guess?

Like

struct hvled_linearization {
    u16 i_max;
    u16 fade_hi;
    u16 fade_lo;
};

#define LIN_TAB_ENTRY(a, b, c) { .i_max = a, .fade_hi = b, fade_lo = c }

struct hvled_linearization lin_table[] = {
    LIN_TAB_ENTRY(0, 0, 0),
    ... insert the hard part here ;-) ...
    LIN_TAB_ENTRY(65535, 65535, 65535),
};

The problem is I think this table is dependent on the LED fitted
and needs to be supplied as platform data from the board,
which knows which LED is on that board.

Methods to generate table is reading the LED data sheet
(if you have it) or using a photometric instrument while locking
yourself in a totally dark room. (This is the fun part...)
http://en.wikipedia.org/wiki/Photometer

Yours,
Linus Walleij

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-14 18:34                 ` Linus Walleij
@ 2011-12-15  7:11                   ` Mark Brown
  2011-12-16  5:10                     ` Shreshtha Kumar SAHU
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2011-12-15  7:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Shreshtha Kumar SAHU, Andrew Morton, Denis Kuzmenko,
	Linus WALLEIJ, Richard Purdie, linux-kernel, Samuel Ortiz

On Wed, Dec 14, 2011 at 07:34:50PM +0100, Linus Walleij wrote:

> The problem is I think this table is dependent on the LED fitted
> and needs to be supplied as platform data from the board,
> which knows which LED is on that board.

To a good approximation the restriction from the LED comes from the
maximum current it can sustain so it's usually a fairly simple input.

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-15  7:11                   ` Mark Brown
@ 2011-12-16  5:10                     ` Shreshtha Kumar SAHU
  2011-12-22 17:43                       ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Shreshtha Kumar SAHU @ 2011-12-16  5:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Andrew Morton, Denis Kuzmenko, Linus WALLEIJ,
	Richard Purdie, linux-kernel, Samuel Ortiz

On Thu, Dec 15, 2011 at 08:11:14 +0100, Mark Brown wrote:
> On Wed, Dec 14, 2011 at 07:34:50PM +0100, Linus Walleij wrote:
> 
> > The problem is I think this table is dependent on the LED fitted
> > and needs to be supplied as platform data from the board,
> > which knows which LED is on that board.
> 
> To a good approximation the restriction from the LED comes from the
> maximum current it can sustain so it's usually a fairly simple input.

As I understnad, please corrent me if I am worng, it is decided during
platform design and setting the max current for each LED in a platform
fulfills the above requirement which is already taken care in driver.

Based on suggestion by Mark, to control both PWM and Sink current to
vary brightness, here is the outcome:

How it was:
Max sink current was set for each LED (range 2.5mA to 40mA), and PWM
was varied to get the desired brightness (PWM in range 0 to 1023).
Current increase on each PWM increase was constant i.e. max_curr/1023.

How it is now:
Max sink current is known, but sink current value is set along with
PWM to control the brightness. So now with each request to increase
brightness, sink current is determined based on:
	requested_brt / (LED_FULL/SINKCURR_MAX)
In this case requested_brt vary from 0 to 255 and SINKCURR_MAX is 15.

Now sink_current is changing, hence current increase on each PWM increase
will vary based on:
As current increase on each PWM increase is based on (sink_current / PWM_MAX)
where PWM_MAX is 1023. But now sink_currnet is changing so current increase
on each PWM increase will also change.

So the idea in implementation is to change the sink_current and calculate
the PWM value based on currently set sink current, so that the current
change on each PWM rise remains same.

Implementation is greatly simplified by following lookup table:

#define AB5500_LED_SINKCURR_MAX                0x0F
#define AB5500_LED_SINKCURR_STEPS(_max_curr)   (LED_FULL/(_max_curr))

/*
 * This table provides the multiplier to get linear  variation of current
 * and hence led brightness. In actual these are PWM steps multiplier to
 * get PWM value such that the current vary linearly (as sink current
 * along with PWM is configured) and hence the led brightness.
 *
 * rows are max_current set for the led instance
 * cols are current step of 16 step current stair
 *
 */
static u8 led_lin_table[AB5500_LED_SINKCURR_MAX+2]
                       [AB5500_LED_SINKCURR_MAX+2] = {
       {4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {8, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {12, 6, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {16, 8, 5, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {20, 10, 7, 5, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {24, 12, 8, 6, 5, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {28, 14, 9, 7, 6, 5, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
       {32, 16, 11, 8, 6, 5, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0,},
       {36, 18, 12, 9, 7, 6, 5, 4, 4, 0, 0, 0, 0, 0, 0, 0,},
       {40, 20, 13, 10, 8, 7, 6, 5, 4, 4, 0, 0, 0, 0, 0, 0,},
       {44, 22, 15, 11, 9, 7, 6, 5, 5, 4, 4, 0, 0, 0, 0, 0,},
       {48, 24, 16, 12, 10, 8, 7, 6, 5, 5, 4, 4, 0, 0, 0, 0,},
       {52, 26, 17, 13, 10, 9, 7, 6, 6, 5, 5, 4, 4, 0, 0, 0,},
       {56, 28, 19, 14, 11, 9, 8, 7, 6, 5, 5, 5, 4, 4, 0, 0,},
       {60, 30, 20, 15, 12, 10, 8, 7, 7, 6, 5, 5, 5, 4, 4, 0,},
       {64, 32, 21, 16, 13, 11, 9, 8, 7, 6, 6, 5, 5, 4, 4, 4,},
};

And the formula used when brightness change is requested from user:

u8 curr_max = led->max_current, curr_val;
u16 pwm_val;

curr_val = (led->brt_val / (AB5500_LED_SINKCURR_STEPS(curr_max)));
pwm_val = led->brt_val * led_lin_table[curr_max][curr_val];

Please provide feedback on this, if it fulfills the requirement.

Regards,
Shreshtha

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-02 13:56 [PATCH] leds: driver for ab5500 high voltage leds Linus Walleij
  2011-12-03 21:49 ` Mark Brown
  2011-12-03 22:49 ` Denis Kuzmenko
@ 2011-12-19 11:26 ` Samuel Ortiz
  2 siblings, 0 replies; 18+ messages in thread
From: Samuel Ortiz @ 2011-12-19 11:26 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Richard Purdie, linux-kernel, Andrew Morton,
	Shreshtha Kumar Sahu, Linus Walleij

Hi Linus,

On Fri, Dec 02, 2011 at 02:56:22PM +0100, Linus Walleij wrote:
> From: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
> 
> Simple HV LED controller driver for AB5500v1.0 MFD chips
> 
> Signed-off-by: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
> Reviewed-by: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>
> Tested-by: Naga Radesh Y <naga.radheshy@stericsson.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This depends on the other movement patch since it handily puts the
> platform data header in <linux/mfd/abx500/ab5500-leds.h>.
So I'm pushing this one through. Applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [PATCH] leds: driver for ab5500 high voltage leds
  2011-12-16  5:10                     ` Shreshtha Kumar SAHU
@ 2011-12-22 17:43                       ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2011-12-22 17:43 UTC (permalink / raw)
  To: Shreshtha Kumar SAHU
  Cc: Linus Walleij, Andrew Morton, Denis Kuzmenko, Linus WALLEIJ,
	Richard Purdie, linux-kernel, Samuel Ortiz

On Fri, Dec 16, 2011 at 10:40:31AM +0530, Shreshtha Kumar SAHU wrote:

> Please provide feedback on this, if it fulfills the requirement.

That seems reasonable to me, my only concern was that we had this extra
knob for essentially the same thing so so long as it all comes down to
one brightness control that's addressed.

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

end of thread, other threads:[~2011-12-22 17:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-02 13:56 [PATCH] leds: driver for ab5500 high voltage leds Linus Walleij
2011-12-03 21:49 ` Mark Brown
2011-12-07 14:16   ` Linus Walleij
2011-12-07 15:47     ` Mark Brown
2011-12-03 22:49 ` Denis Kuzmenko
2011-12-07 14:07   ` Linus Walleij
2011-12-07 18:01     ` Denis Kuzmenko
2011-12-08  0:00       ` Andrew Morton
2011-12-08  1:19         ` Mark Brown
2011-12-14 10:23           ` Shreshtha Kumar SAHU
2011-12-14 11:37             ` Mark Brown
2011-12-14 14:01               ` Shreshtha Kumar SAHU
2011-12-14 14:47                 ` Mark Brown
2011-12-14 18:34                 ` Linus Walleij
2011-12-15  7:11                   ` Mark Brown
2011-12-16  5:10                     ` Shreshtha Kumar SAHU
2011-12-22 17:43                       ` Mark Brown
2011-12-19 11:26 ` Samuel Ortiz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.