lkml.org 
[lkml]   [2009]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] leds: Add LED class driver for regulator driven LEDs.
On Wed, 2 Dec 2009 20:41:22 +0000
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Dec 02, 2009 at 09:31:31PM +0100, Antonio Ospite wrote:
> > Liam Girdwood <lrg@slimlogic.co.uk> wrote:
>
> > > Some regulators may not support voltage change (via hw design or
> > > constraints) so set_voltage() will fail. We probably want to handle this
> > > regulator type so we don't call set_voltage().
>
> > Ok, I'll read more about the regulator framework to find out how to
> > check regulator capabilities.
>
> Checking for errors when counting and listing the voltages ought to
> cover this.

This is the incremental change I have in mind; if it's ok, then
a v2 is on its way.

diff --git a/drivers/leds/leds-regulator.c b/drivers/leds/leds-regulator.c
index f4c508b..8146d12 100644
--- a/drivers/leds/leds-regulator.c
+++ b/drivers/leds/leds-regulator.c
@@ -34,14 +34,23 @@ struct regulator_led {

static inline int led_regulator_get_max_brightness(struct regulator *supply)
{
- return regulator_count_voltages(supply);
+ int ret = regulator_count_voltages(supply);
+
+ /* even if regulator can't change voltages,
+ * we still assume it can change status
+ * and the LED can be turned on and off.
+ */
+ if (ret == -EINVAL)
+ return 1;
+
+ return ret;
}

static int led_regulator_get_vdd(struct regulator *supply,
enum led_brightness brightness)
{
if (brightness == 0)
- return 0;
+ return -EINVAL;

return regulator_list_voltage(supply, brightness - 1);
}
@@ -95,13 +104,15 @@ static void led_work(struct work_struct *work)
}

voltage = led_regulator_get_vdd(led->vcc, led->value);
- dev_dbg(led->cdev.dev, "brightness: %d voltage: %d",
- led->value, voltage);
-
- ret = regulator_set_voltage(led->vcc, voltage, voltage);
- if (ret != 0)
- dev_err(led->cdev.dev, "Failed to set voltage %d: %d\n",
- voltage, ret);
+ if (voltage) {
+ dev_dbg(led->cdev.dev, "brightness: %d voltage: %d",
+ led->value, voltage);
+
+ ret = regulator_set_voltage(led->vcc, voltage, voltage);
+ if (ret != 0)
+ dev_err(led->cdev.dev, "Failed to set voltage %d: %d\n",
+ voltage, ret);
+ }

regulator_led_enable(led);

In the last hunk I could have checked (max_brightness > 1) in place
of (voltage) given the first hunk, any opinion?
Thanks,
Antonio

--
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2009-12-04 14:01    [from the cache]
©2003-2011 Jasper Spaans