2009-09-05 13:09:35

by Mark Brown

[permalink] [raw]
Subject: [PATCH 1/2] backlight: Add WM831x backlight driver

The WM831x series of PMICs provide DC-DC boost convertors and current
sinks which can be used together to drive LEDs for use as backlights.
Expose this functionality via the backlight API.

Since when used in this configuration the current sink and boost
convertor are very tightly coupled with a multi-stage startup for
the current sink which overlaps with the boost convertor startup
this driver bypasses the regulator API. Machine inititialisation
is responsible for ensuring that the regulators are not accessed
via both APIs.

Signed-off-by: Mark Brown <[email protected]>
Cc: Richard Purdie <[email protected]>
---
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/wm831x_bl.c | 250 +++++++++++++++++++++++++++++++++++
3 files changed, 258 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/backlight/wm831x_bl.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 90861cd..205de1a 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -229,3 +229,10 @@ config BACKLIGHT_SAHARA
help
If you have a Tabletkiosk Sahara Touch-iT, say y to enable the
backlight driver.
+
+config BACKLIGHT_WM831X
+ tristate "WM831x PMIC Backlight Driver"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_WM831X
+ help
+ If you have a backlight driven by the ISINK and DCDC of a
+ WM831x PMIC say y to enable the backlight driver for it.
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 4eb178c..df0b67c 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -24,4 +24,5 @@ obj-$(CONFIG_BACKLIGHT_DA903X) += da903x_bl.o
obj-$(CONFIG_BACKLIGHT_MBP_NVIDIA) += mbp_nvidia_bl.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o

diff --git a/drivers/video/backlight/wm831x_bl.c b/drivers/video/backlight/wm831x_bl.c
new file mode 100644
index 0000000..467bdb7
--- /dev/null
+++ b/drivers/video/backlight/wm831x_bl.c
@@ -0,0 +1,250 @@
+/*
+ * Backlight driver for Wolfson Microelectronics WM831x PMICs
+ *
+ * Copyright 2009 Wolfson Microelectonics plc
+ *
+ * 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/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/fb.h>
+#include <linux/backlight.h>
+
+#include <linux/mfd/wm831x/core.h>
+#include <linux/mfd/wm831x/pdata.h>
+#include <linux/mfd/wm831x/regulator.h>
+
+struct wm831x_backlight_data {
+ struct wm831x *wm831x;
+ int isink_reg;
+ int current_brightness;
+};
+
+static int wm831x_backlight_set(struct backlight_device *bl, int brightness)
+{
+ struct wm831x_backlight_data *data = bl_get_data(bl);
+ struct wm831x *wm831x = data->wm831x;
+ int power_up = !data->current_brightness && brightness;
+ int power_down = data->current_brightness && !brightness;
+ int ret;
+
+ if (power_up) {
+ /* Enable the ISINK */
+ ret = wm831x_set_bits(wm831x, data->isink_reg,
+ WM831X_CS1_ENA, WM831X_CS1_ENA);
+ if (ret < 0)
+ goto err;
+
+ /* Enable the DC-DC */
+ ret = wm831x_set_bits(wm831x, WM831X_DCDC_ENABLE,
+ WM831X_DC4_ENA, WM831X_DC4_ENA);
+ if (ret < 0)
+ goto err;
+ }
+
+ if (power_down) {
+ /* DCDC first */
+ ret = wm831x_set_bits(wm831x, WM831X_DCDC_ENABLE,
+ WM831X_DC4_ENA, 0);
+ if (ret < 0)
+ goto err;
+
+ /* ISINK */
+ ret = wm831x_set_bits(wm831x, data->isink_reg,
+ WM831X_CS1_DRIVE | WM831X_CS1_ENA, 0);
+ if (ret < 0)
+ goto err;
+ }
+
+ /* Set the new brightness */
+ ret = wm831x_set_bits(wm831x, data->isink_reg,
+ WM831X_CS1_ISEL_MASK, brightness);
+ if (ret < 0)
+ goto err;
+
+ if (power_up) {
+ /* Drive current through the ISINK */
+ ret = wm831x_set_bits(wm831x, data->isink_reg,
+ WM831X_CS1_DRIVE, WM831X_CS1_DRIVE);
+ if (ret < 0)
+ return ret;
+ }
+
+ data->current_brightness = brightness;
+
+ return 0;
+
+err:
+ /* If we were in the middle of a power transition always shut down
+ * for safety.
+ */
+ if (power_up || power_down) {
+ wm831x_set_bits(wm831x, WM831X_DCDC_ENABLE, WM831X_DC4_ENA, 0);
+ wm831x_set_bits(wm831x, data->isink_reg, WM831X_CS1_ENA, 0);
+ }
+
+ return ret;
+}
+
+static int wm831x_backlight_update_status(struct backlight_device *bl)
+{
+ int brightness = bl->props.brightness;
+
+ if (bl->props.power != FB_BLANK_UNBLANK)
+ brightness = 0;
+
+ if (bl->props.fb_blank != FB_BLANK_UNBLANK)
+ brightness = 0;
+
+ if (bl->props.state & BL_CORE_SUSPENDED)
+ brightness = 0;
+
+ return wm831x_backlight_set(bl, brightness);
+}
+
+static int wm831x_backlight_get_brightness(struct backlight_device *bl)
+{
+ struct wm831x_backlight_data *data = bl_get_data(bl);
+ return data->current_brightness;
+}
+
+static struct backlight_ops wm831x_backlight_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = wm831x_backlight_update_status,
+ .get_brightness = wm831x_backlight_get_brightness,
+};
+
+static int wm831x_backlight_probe(struct platform_device *pdev)
+{
+ struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
+ struct wm831x_pdata *wm831x_pdata;
+ struct wm831x_backlight_pdata *pdata;
+ struct wm831x_backlight_data *data;
+ struct backlight_device *bl;
+ int ret, i, max_isel, isink_reg, dcdc_cfg;
+
+ /* We need platform data */
+ if (pdev->dev.parent->platform_data) {
+ wm831x_pdata = pdev->dev.parent->platform_data;
+ pdata = wm831x_pdata->backlight;
+ } else {
+ pdata = NULL;
+ }
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "No platform data supplied\n");
+ return -EINVAL;
+ }
+
+ /* Figure out the maximum current we can use */
+ for (i = 0; i < WM831X_ISINK_MAX_ISEL; i++) {
+ if (wm831x_isinkv_values[i] > pdata->max_uA)
+ break;
+ }
+
+ if (i == 0) {
+ dev_err(&pdev->dev, "Invalid max_uA: %duA\n", pdata->max_uA);
+ return -EINVAL;
+ }
+ max_isel = i - 1;
+
+ if (pdata->max_uA != wm831x_isinkv_values[max_isel])
+ dev_warn(&pdev->dev,
+ "Maximum current is %duA not %duA as requested\n",
+ wm831x_isinkv_values[max_isel], pdata->max_uA);
+
+ switch (pdata->isink) {
+ case 1:
+ isink_reg = WM831X_CURRENT_SINK_1;
+ dcdc_cfg = 0;
+ break;
+ case 2:
+ isink_reg = WM831X_CURRENT_SINK_2;
+ dcdc_cfg = WM831X_DC4_FBSRC;
+ break;
+ default:
+ dev_err(&pdev->dev, "Invalid ISINK %d\n", pdata->isink);
+ return -EINVAL;
+ }
+
+ /* Configure the ISINK to use for feedback */
+ ret = wm831x_reg_unlock(wm831x);
+ if (ret < 0)
+ return ret;
+
+ ret = wm831x_set_bits(wm831x, WM831X_DC4_CONTROL, WM831X_DC4_FBSRC,
+ dcdc_cfg);
+
+ wm831x_reg_lock(wm831x);
+ if (ret < 0)
+ return ret;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (data == NULL)
+ return -ENOMEM;
+
+ data->wm831x = wm831x;
+ data->current_brightness = 0;
+ data->isink_reg = isink_reg;
+
+ bl = backlight_device_register("wm831x", &pdev->dev,
+ data, &wm831x_backlight_ops);
+ if (IS_ERR(bl)) {
+ dev_err(&pdev->dev, "failed to register backlight\n");
+ kfree(data);
+ return PTR_ERR(bl);
+ }
+
+ bl->props.max_brightness = max_isel;
+ bl->props.brightness = max_isel;
+
+ platform_set_drvdata(pdev, bl);
+
+ /* Disable the DCDC if it was started so we can bootstrap */
+ wm831x_set_bits(wm831x, WM831X_DCDC_ENABLE, WM831X_DC4_ENA, 0);
+
+
+ backlight_update_status(bl);
+
+ return 0;
+}
+
+static int wm831x_backlight_remove(struct platform_device *pdev)
+{
+ struct backlight_device *bl = platform_get_drvdata(pdev);
+ struct wm831x_backlight_data *data = bl_get_data(bl);
+
+ backlight_device_unregister(bl);
+ kfree(data);
+ return 0;
+}
+
+static struct platform_driver wm831x_backlight_driver = {
+ .driver = {
+ .name = "wm831x-backlight",
+ .owner = THIS_MODULE,
+ },
+ .probe = wm831x_backlight_probe,
+ .remove = wm831x_backlight_remove,
+};
+
+static int __init wm831x_backlight_init(void)
+{
+ return platform_driver_register(&wm831x_backlight_driver);
+}
+module_init(wm831x_backlight_init);
+
+static void __exit wm831x_backlight_exit(void)
+{
+ platform_driver_unregister(&wm831x_backlight_driver);
+}
+module_exit(wm831x_backlight_exit);
+
+MODULE_DESCRIPTION("Backlight Driver for WM831x PMICs");
+MODULE_AUTHOR("Mark Brown <[email protected]");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:wm831x-backlight");
--
1.6.3.3


2009-09-05 13:09:41

by Mark Brown

[permalink] [raw]
Subject: [PATCH 2/2] leds: Add WM831x status LED driver

The WM831x devices feature two software controlled status LEDs with
hardware assisted blinking.

The device can also autonomously control the LEDs based on a selection
of sources. This can be configured at boot time using either platform
data or the chip OTP. A sysfs file in the style of that for triggers
allowing the control source to be configured at run time. Triggers
can't be used here since they can't depend on the implementation details
of a specific LED type.

Signed-off-by: Mark Brown <[email protected]>
Cc: Richard Purdie <[email protected]>
---
drivers/leds/Kconfig | 7 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-wm831x-status.c | 341 +++++++++++++++++++++++++++++++++++++
include/linux/mfd/wm831x/status.h | 34 ++++
4 files changed, 383 insertions(+), 0 deletions(-)
create mode 100644 drivers/leds/leds-wm831x-status.c
create mode 100644 include/linux/mfd/wm831x/status.h

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7c8e712..edfd4e3 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -195,6 +195,13 @@ config LEDS_PCA955X
LED driver chips accessed via the I2C bus. Supported
devices include PCA9550, PCA9551, PCA9552, and PCA9553.

+config LEDS_WM831X_STATUS
+ tristate "LED support for status LEDs on WM831x PMICs"
+ depends on LEDS_CLASS && MFD_WM831X
+ help
+ This option enables support for the status LEDs of the WM831x
+ series of PMICs.
+
config LEDS_WM8350
tristate "LED Support for WM8350 AudioPlus PMIC"
depends on LEDS_CLASS && MFD_WM8350
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e8cdcf7..46d7270 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
obj-$(CONFIG_LEDS_FSG) += leds-fsg.o
obj-$(CONFIG_LEDS_PCA955X) += leds-pca955x.o
obj-$(CONFIG_LEDS_DA903X) += leds-da903x.o
+obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o
obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
obj-$(CONFIG_LEDS_PWM) += leds-pwm.o

diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
new file mode 100644
index 0000000..c586d05
--- /dev/null
+++ b/drivers/leds/leds-wm831x-status.c
@@ -0,0 +1,341 @@
+/*
+ * LED driver for WM831x status LEDs
+ *
+ * Copyright(C) 2009 Wolfson Microelectronics PLC.
+ *
+ * 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/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/leds.h>
+#include <linux/err.h>
+#include <linux/mfd/wm831x/core.h>
+#include <linux/mfd/wm831x/pdata.h>
+#include <linux/mfd/wm831x/status.h>
+
+
+struct wm831x_status {
+ struct led_classdev cdev;
+ struct wm831x *wm831x;
+ struct work_struct work;
+ struct mutex mutex;
+
+ spinlock_t value_lock;
+ int reg; /* Control register */
+ int reg_val; /* Control register value */
+
+ int blink;
+ int blink_time;
+ int blink_cyc;
+ int src;
+ enum led_brightness brightness;
+};
+
+#define to_wm831x_status(led_cdev) \
+ container_of(led_cdev, struct wm831x_status, cdev)
+
+static void wm831x_status_work(struct work_struct *work)
+{
+ struct wm831x_status *led = container_of(work, struct wm831x_status,
+ work);
+ unsigned long flags;
+
+ mutex_lock(&led->mutex);
+
+ led->reg_val &= ~(WM831X_LED_SRC_MASK | WM831X_LED_MODE_MASK |
+ WM831X_LED_DUTY_CYC_MASK | WM831X_LED_DUR_MASK);
+
+ spin_lock_irqsave(&led->value_lock, flags);
+
+ led->reg_val |= led->src << WM831X_LED_SRC_SHIFT;
+ if (led->blink) {
+ led->reg_val |= 2 << WM831X_LED_MODE_SHIFT;
+ led->reg_val |= led->blink_time << WM831X_LED_DUR_SHIFT;
+ led->reg_val |= led->blink_cyc;
+ } else {
+ if (led->brightness != LED_OFF)
+ led->reg_val |= 1 << WM831X_LED_MODE_SHIFT;
+ }
+
+ spin_unlock_irqrestore(&led->value_lock, flags);
+
+ wm831x_reg_write(led->wm831x, led->reg, led->reg_val);
+
+ mutex_unlock(&led->mutex);
+}
+
+static void wm831x_status_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct wm831x_status *led = to_wm831x_status(led_cdev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->value_lock, flags);
+ led->brightness = value;
+ if (value == LED_OFF)
+ led->blink = 0;
+ schedule_work(&led->work);
+ spin_unlock_irqrestore(&led->value_lock, flags);
+}
+
+static int wm831x_status_blink_set(struct led_classdev *led_cdev,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ struct wm831x_status *led = to_wm831x_status(led_cdev);
+ unsigned long flags;
+ int ret = 0;
+
+ /* Pick some defaults if we've not been given times */
+ if (*delay_on == 0 && *delay_off == 0) {
+ *delay_on = 250;
+ *delay_off = 250;
+ }
+
+ spin_lock_irqsave(&led->value_lock, flags);
+
+ /* We only have a limited selection of settings, see if we can
+ * support the configuration we're being given */
+ switch (*delay_on) {
+ case 1000:
+ led->blink_time = 0;
+ break;
+ case 250:
+ led->blink_time = 1;
+ break;
+ case 125:
+ led->blink_time = 2;
+ break;
+ case 62:
+ case 63:
+ /* Actually 62.5ms */
+ led->blink_time = 3;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret == 0) {
+ switch (*delay_off / *delay_on) {
+ case 1:
+ led->blink_cyc = 0;
+ break;
+ case 3:
+ led->blink_cyc = 1;
+ break;
+ case 4:
+ led->blink_cyc = 2;
+ break;
+ case 8:
+ led->blink_cyc = 3;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+ if (ret == 0)
+ led->blink = 1;
+ else
+ led->blink = 0;
+
+ /* Always update; if we fail turn off blinking since we expect
+ * a software fallback. */
+ schedule_work(&led->work);
+
+ spin_unlock_irqrestore(&led->value_lock, flags);
+
+ return ret;
+}
+
+static const char *led_src_texts[] = {
+ "otp",
+ "power",
+ "charger",
+ "soft",
+};
+
+static ssize_t wm831x_status_src_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct wm831x_status *led = to_wm831x_status(led_cdev);
+ int i;
+ ssize_t ret = 0;
+
+ mutex_lock(&led->mutex);
+
+ for (i = 0; i < ARRAY_SIZE(led_src_texts); i++)
+ if (i == led->src)
+ ret += sprintf(&buf[ret], "[%s] ", led_src_texts[i]);
+ else
+ ret += sprintf(&buf[ret], "%s ", led_src_texts[i]);
+
+ mutex_unlock(&led->mutex);
+
+ ret += sprintf(&buf[ret], "\n");
+
+ return ret;
+}
+
+static ssize_t wm831x_status_src_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct wm831x_status *led = to_wm831x_status(led_cdev);
+ char name[20];
+ int i;
+ size_t len;
+
+ name[sizeof(name) - 1] = '\0';
+ strncpy(name, buf, sizeof(name) - 1);
+ len = strlen(name);
+
+ if (len && name[len - 1] == '\n')
+ name[len - 1] = '\0';
+
+ for (i = 0; i < ARRAY_SIZE(led_src_texts); i++) {
+ if (!strcmp(name, led_src_texts[i])) {
+ mutex_lock(&led->mutex);
+
+ led->src = i;
+ schedule_work(&led->work);
+
+ mutex_unlock(&led->mutex);
+ }
+ }
+
+ return size;
+}
+
+static DEVICE_ATTR(src, 0644, wm831x_status_src_show, wm831x_status_src_store);
+
+static int wm831x_status_probe(struct platform_device *pdev)
+{
+ struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
+ struct wm831x_pdata *chip_pdata;
+ struct wm831x_status_pdata pdata;
+ struct wm831x_status *drvdata;
+ struct resource *res;
+ int id = pdev->id % ARRAY_SIZE(chip_pdata->status);
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (res == NULL) {
+ dev_err(&pdev->dev, "No I/O resource\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ drvdata = kzalloc(sizeof(struct wm831x_status), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+ dev_set_drvdata(&pdev->dev, drvdata);
+
+ drvdata->wm831x = wm831x;
+ drvdata->reg = res->start;
+
+ if (wm831x->dev->platform_data)
+ chip_pdata = wm831x->dev->platform_data;
+ else
+ chip_pdata = NULL;
+
+ memset(&pdata, 0, sizeof(pdata));
+ if (chip_pdata && chip_pdata->status[id])
+ memcpy(&pdata, chip_pdata->status[id], sizeof(pdata));
+ else
+ pdata.name = dev_name(&pdev->dev);
+
+ mutex_init(&drvdata->mutex);
+ INIT_WORK(&drvdata->work, wm831x_status_work);
+ spin_lock_init(&drvdata->value_lock);
+
+ /* We cache the configuration register and read startup values
+ * from it. */
+ drvdata->reg_val = wm831x_reg_read(wm831x, drvdata->reg);
+
+ if (drvdata->reg_val & WM831X_LED_MODE_MASK)
+ drvdata->brightness = LED_FULL;
+ else
+ drvdata->brightness = LED_OFF;
+
+ /* Set a default source if configured, otherwise leave the
+ * current hardware setting.
+ */
+ if (pdata.default_src == WM831X_STATUS_PRESERVE) {
+ drvdata->src = drvdata->reg_val;
+ drvdata->src &= WM831X_LED_SRC_MASK;
+ drvdata->src >>= WM831X_LED_SRC_SHIFT;
+ } else {
+ drvdata->src = pdata.default_src - 1;
+ }
+
+ drvdata->cdev.name = pdata.name;
+ drvdata->cdev.default_trigger = pdata.default_trigger;
+ drvdata->cdev.brightness_set = wm831x_status_set;
+ drvdata->cdev.blink_set = wm831x_status_blink_set;
+
+ ret = led_classdev_register(wm831x->dev, &drvdata->cdev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register LED: %d\n", ret);
+ goto err_led;
+ }
+
+ ret = device_create_file(drvdata->cdev.dev, &dev_attr_src);
+ if (ret != 0)
+ dev_err(&pdev->dev,
+ "No source control for LED: %d\n", ret);
+
+ return 0;
+
+err_led:
+ led_classdev_unregister(&drvdata->cdev);
+ kfree(drvdata);
+err:
+ return ret;
+}
+
+static int wm831x_status_remove(struct platform_device *pdev)
+{
+ struct wm831x_status *drvdata = platform_get_drvdata(pdev);
+
+ device_remove_file(drvdata->cdev.dev, &dev_attr_src);
+ led_classdev_unregister(&drvdata->cdev);
+ kfree(drvdata);
+
+ return 0;
+}
+
+static struct platform_driver wm831x_status_driver = {
+ .driver = {
+ .name = "wm831x-status",
+ .owner = THIS_MODULE,
+ },
+ .probe = wm831x_status_probe,
+ .remove = wm831x_status_remove,
+};
+
+static int __devinit wm831x_status_init(void)
+{
+ return platform_driver_register(&wm831x_status_driver);
+}
+module_init(wm831x_status_init);
+
+static void wm831x_status_exit(void)
+{
+ platform_driver_unregister(&wm831x_status_driver);
+}
+module_exit(wm831x_status_exit);
+
+MODULE_AUTHOR("Mark Brown <[email protected]>");
+MODULE_DESCRIPTION("WM831x status LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:wm831x-status");
diff --git a/include/linux/mfd/wm831x/status.h b/include/linux/mfd/wm831x/status.h
new file mode 100644
index 0000000..6bc090d
--- /dev/null
+++ b/include/linux/mfd/wm831x/status.h
@@ -0,0 +1,34 @@
+/*
+ * include/linux/mfd/wm831x/status.h -- Status LEDs for WM831x
+ *
+ * Copyright 2009 Wolfson Microelectronics PLC.
+ *
+ * Author: Mark Brown <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef __MFD_WM831X_STATUS_H__
+#define __MFD_WM831X_STATUS_H__
+
+#define WM831X_LED_SRC_MASK 0xC000 /* LED_SRC - [15:14] */
+#define WM831X_LED_SRC_SHIFT 14 /* LED_SRC - [15:14] */
+#define WM831X_LED_SRC_WIDTH 2 /* LED_SRC - [15:14] */
+#define WM831X_LED_MODE_MASK 0x0300 /* LED_MODE - [9:8] */
+#define WM831X_LED_MODE_SHIFT 8 /* LED_MODE - [9:8] */
+#define WM831X_LED_MODE_WIDTH 2 /* LED_MODE - [9:8] */
+#define WM831X_LED_SEQ_LEN_MASK 0x0030 /* LED_SEQ_LEN - [5:4] */
+#define WM831X_LED_SEQ_LEN_SHIFT 4 /* LED_SEQ_LEN - [5:4] */
+#define WM831X_LED_SEQ_LEN_WIDTH 2 /* LED_SEQ_LEN - [5:4] */
+#define WM831X_LED_DUR_MASK 0x000C /* LED_DUR - [3:2] */
+#define WM831X_LED_DUR_SHIFT 2 /* LED_DUR - [3:2] */
+#define WM831X_LED_DUR_WIDTH 2 /* LED_DUR - [3:2] */
+#define WM831X_LED_DUTY_CYC_MASK 0x0003 /* LED_DUTY_CYC - [1:0] */
+#define WM831X_LED_DUTY_CYC_SHIFT 0 /* LED_DUTY_CYC - [1:0] */
+#define WM831X_LED_DUTY_CYC_WIDTH 2 /* LED_DUTY_CYC - [1:0] */
+
+#endif
--
1.6.3.3

2009-09-07 07:25:11

by Andrey Panin

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

On 248, 09 05, 2009 at 02:09:21PM +0100, Mark Brown wrote:
> The WM831x devices feature two software controlled status LEDs with
> hardware assisted blinking.
>
> The device can also autonomously control the LEDs based on a selection
> of sources. This can be configured at boot time using either platform
> data or the chip OTP. A sysfs file in the style of that for triggers
> allowing the control source to be configured at run time. Triggers
> can't be used here since they can't depend on the implementation details
> of a specific LED type.
>
> Signed-off-by: Mark Brown <[email protected]>

> +static int wm831x_status_probe(struct platform_device *pdev)
> +{
> + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
> + struct wm831x_pdata *chip_pdata;
> + struct wm831x_status_pdata pdata;
> + struct wm831x_status *drvdata;
> + struct resource *res;
> + int id = pdev->id % ARRAY_SIZE(chip_pdata->status);
> + int ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + if (res == NULL) {
> + dev_err(&pdev->dev, "No I/O resource\n");
> + ret = -EINVAL;
> + goto err;

Why not return -EINVAL right here ?

> + }
> +
> + drvdata = kzalloc(sizeof(struct wm831x_status), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> + dev_set_drvdata(&pdev->dev, drvdata);
> +
> + drvdata->wm831x = wm831x;
> + drvdata->reg = res->start;
> +
> + if (wm831x->dev->platform_data)
> + chip_pdata = wm831x->dev->platform_data;
> + else
> + chip_pdata = NULL;
> +
> + memset(&pdata, 0, sizeof(pdata));
> + if (chip_pdata && chip_pdata->status[id])
> + memcpy(&pdata, chip_pdata->status[id], sizeof(pdata));
> + else
> + pdata.name = dev_name(&pdev->dev);
> +
> + mutex_init(&drvdata->mutex);
> + INIT_WORK(&drvdata->work, wm831x_status_work);
> + spin_lock_init(&drvdata->value_lock);
> +
> + /* We cache the configuration register and read startup values
> + * from it. */
> + drvdata->reg_val = wm831x_reg_read(wm831x, drvdata->reg);
> +
> + if (drvdata->reg_val & WM831X_LED_MODE_MASK)
> + drvdata->brightness = LED_FULL;
> + else
> + drvdata->brightness = LED_OFF;
> +
> + /* Set a default source if configured, otherwise leave the
> + * current hardware setting.
> + */
> + if (pdata.default_src == WM831X_STATUS_PRESERVE) {
> + drvdata->src = drvdata->reg_val;
> + drvdata->src &= WM831X_LED_SRC_MASK;
> + drvdata->src >>= WM831X_LED_SRC_SHIFT;
> + } else {
> + drvdata->src = pdata.default_src - 1;
> + }
> +
> + drvdata->cdev.name = pdata.name;
> + drvdata->cdev.default_trigger = pdata.default_trigger;
> + drvdata->cdev.brightness_set = wm831x_status_set;
> + drvdata->cdev.blink_set = wm831x_status_blink_set;
> +
> + ret = led_classdev_register(wm831x->dev, &drvdata->cdev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register LED: %d\n", ret);
> + goto err_led;
> + }
> +
> + ret = device_create_file(drvdata->cdev.dev, &dev_attr_src);
> + if (ret != 0)
> + dev_err(&pdev->dev,
> + "No source control for LED: %d\n", ret);
> +
> + return 0;
> +
> +err_led:
> + led_classdev_unregister(&drvdata->cdev);

This line looks useless and possibly unsafe, led device was not registered.
Also this part of code can be removed if you move next line up.

> + kfree(drvdata);
> +err:
> + return ret;
> +}

2009-09-07 09:07:57

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

On Mon, Sep 07, 2009 at 11:25:07AM +0400, Andrey Panin wrote:
> On 248, 09 05, 2009 at 02:09:21PM +0100, Mark Brown wrote:

> > + if (res == NULL) {
> > + dev_err(&pdev->dev, "No I/O resource\n");
> > + ret = -EINVAL;
> > + goto err;

> Why not return -EINVAL right here ?

For consistency with the rest of the function which uses this style for
unwinding (though I see there's a cut'n'pasted return early on which
it'd be nice to clean up).

> > +err_led:
> > + led_classdev_unregister(&drvdata->cdev);

> This line looks useless and possibly unsafe, led device was not registered.

True.

> Also this part of code can be removed if you move next line up.

I'd prefer to keep the unwinding at the end, it makes the initialisation
much easier to modify.

2009-09-07 13:22:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

On Sat 2009-09-05 14:09:21, Mark Brown wrote:
> The WM831x devices feature two software controlled status LEDs with
> hardware assisted blinking.
>
> The device can also autonomously control the LEDs based on a selection
> of sources. This can be configured at boot time using either platform
> data or the chip OTP. A sysfs file in the style of that for triggers
> allowing the control source to be configured at run time. Triggers
> can't be used here since they can't depend on the implementation details
> of a specific LED type.

I believe people *were* doing that with triggers in other drivers...?

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

2009-09-07 13:51:07

by Richard Purdie

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

On Mon, 2009-09-07 at 15:22 +0200, Pavel Machek wrote:
> On Sat 2009-09-05 14:09:21, Mark Brown wrote:
> > The WM831x devices feature two software controlled status LEDs with
> > hardware assisted blinking.
> >
> > The device can also autonomously control the LEDs based on a selection
> > of sources. This can be configured at boot time using either platform
> > data or the chip OTP. A sysfs file in the style of that for triggers
> > allowing the control source to be configured at run time. Triggers
> > can't be used here since they can't depend on the implementation details
> > of a specific LED type.
>
> I believe people *were* doing that with triggers in other drivers...?

We did talk about allowing the LED triggers to choose whether they were
visible to a given LED. At present the mechanism doesn't exist because
we've never had a user.

It would just be a case of adding a new optional function to the trigger
to say whether it supported a given LED or not and calling that function
from led_trigger_show().

I've queued this driver but that would be a nice enhancement.

Cheers,

Richard

--
Richard Purdie
Intel Open Source Technology Centre

2009-09-09 09:10:43

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

Hi Richard,

On Mon, Sep 07, 2009 at 02:51:24PM +0100, Richard Purdie wrote:
> On Mon, 2009-09-07 at 15:22 +0200, Pavel Machek wrote:
> > On Sat 2009-09-05 14:09:21, Mark Brown wrote:
> > > The WM831x devices feature two software controlled status LEDs with
> > > hardware assisted blinking.
> > >
> > > The device can also autonomously control the LEDs based on a selection
> > > of sources. This can be configured at boot time using either platform
> > > data or the chip OTP. A sysfs file in the style of that for triggers
> > > allowing the control source to be configured at run time. Triggers
> > > can't be used here since they can't depend on the implementation details
> > > of a specific LED type.
> >
> > I believe people *were* doing that with triggers in other drivers...?
>
> We did talk about allowing the LED triggers to choose whether they were
> visible to a given LED. At present the mechanism doesn't exist because
> we've never had a user.
>
> It would just be a case of adding a new optional function to the trigger
> to say whether it supported a given LED or not and calling that function
> from led_trigger_show().
>
> I've queued this driver but that would be a nice enhancement.
Unless you're basing your tree on linux-next, you may have issues applying
this patch since the wm831x header is not in Linus tree yet.
If you have troubles with this patch, let me know and I'll queue it.

Cheers,
Samuel.


> Cheers,
>
> Richard
>
> --
> Richard Purdie
> Intel Open Source Technology Centre
>

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

2009-09-09 09:38:30

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

On Wed, Sep 09, 2009 at 11:12:42AM +0200, Samuel Ortiz wrote:

> Unless you're basing your tree on linux-next, you may have issues applying
> this patch since the wm831x header is not in Linus tree yet.
> If you have troubles with this patch, let me know and I'll queue it.

It's fine, all the leaf drivers depend on the core driver (they need to
anyway) so Kconfig won't try to build them until the core driver has
been merged.

2009-09-09 09:50:35

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add WM831x status LED driver

On Wed, Sep 09, 2009 at 10:38:30AM +0100, Mark Brown wrote:
> On Wed, Sep 09, 2009 at 11:12:42AM +0200, Samuel Ortiz wrote:
>
> > Unless you're basing your tree on linux-next, you may have issues applying
> > this patch since the wm831x header is not in Linus tree yet.
> > If you have troubles with this patch, let me know and I'll queue it.
>
> It's fine, all the leaf drivers depend on the core driver (they need to
> anyway) so Kconfig won't try to build them until the core driver has
> been merged.
I was thinking about actually being able to apply the patch, but this patch is
creating status.h so it should be all fine, yes.

Cheers,
Samuel.


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