Date: Fri, 4 Dec 2009 13:57:57 +0100
From: Antonio Ospite <ospite@studenti.unina.it>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Liam Girdwood <lrg@slimlogic.co.uk>, Richard Purdie <rpurdie@rpsys.net>,
       Daniel Ribeiro <drwyrm@gmail.com>, linux-kernel@vger.kernel.org,
       linux-arm-kernel@lists.infradead.org, openezx-devel@lists.openezx.org
Subject: Re: [PATCH] leds: Add LED class driver for regulator driven LEDs.
Message-Id: <20091204135757.d7533e20.ospite@studenti.unina.it>
In-Reply-To: <20091202204121.GB18311@rakim.wolfsonmicro.main>
References: <1259775625-25973-1-git-send-email-ospite@studenti.unina.it>
	<1259778235.3588.141.camel@odin>
	<20091202213131.1393a956.ospite@studenti.unina.it>
	<20091202204121.GB18311@rakim.wolfsonmicro.main>
Mime-Version: 1.0
Content-Type: multipart/signed; protocol="application/pgp-signature";
 micalg="PGP-SHA1";
 boundary="Signature=_Fri__4_Dec_2009_13_57_57_+0100_OlMHRmQ_/qdaLBG2"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3725
Lines: 116

--Signature=_Fri__4_Dec_2009_13_57_57_+0100_OlMHRmQ_/qdaLBG2
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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:
>=20
> > > Some regulators may not support voltage change (via hw design or
> > > constraints) so set_voltage() will fail. We probably want to handle t=
his
> > > regulator type so we don't call set_voltage().
>=20
> > Ok, I'll read more about the regulator framework to find out how to
> > check regulator capabilities.
>=20
> 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 *suppl=
y)
 {
-       return regulator_count_voltages(supply);
+       int ret =3D 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 =3D=3D -EINVAL)
+               return 1;
+
+       return ret;
 }

 static int led_regulator_get_vdd(struct regulator *supply,
                enum led_brightness brightness)
 {
        if (brightness =3D=3D 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 =3D led_regulator_get_vdd(led->vcc, led->value);
-       dev_dbg(led->cdev.dev, "brightness: %d voltage: %d",
-                       led->value, voltage);
-
-       ret =3D regulator_set_voltage(led->vcc, voltage, voltage);
-       if (ret !=3D 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 =3D regulator_set_voltage(led->vcc, voltage, voltage);
+               if (ret !=3D 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

--=20
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?

--Signature=_Fri__4_Dec_2009_13_57_57_+0100_OlMHRmQ_/qdaLBG2
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAksZB1UACgkQ5xr2akVTsAGC4QCgpUmeTBpelYRR8kGJoLtvBiml
23AAoJn6qqnqgnSXPzIGa2DaBa0VuN5F
=lmPP
-----END PGP SIGNATURE-----

--Signature=_Fri__4_Dec_2009_13_57_57_+0100_OlMHRmQ_/qdaLBG2--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/