[PATCH v2] Input: ads7846: add regulator support

[PATCH v2] Input: ads7846: add regulator support

[Grazvydas Ignotas]

The ADS7846/TSC2046 touchscreen controllers can (and usually are)
connected to various regulators for power, so add regulator support.

Valid regulator will now be required, so boards without complete
regulator setup should either disable regulator framework or enable
CONFIG_REGULATOR_DUMMY.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/input/touchscreen/ads7846.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 52d2ca1..8b05d8e 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -27,6 +27,7 @@
 #include <linux/gpio.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/ads7846.h>
+#include <linux/regulator/consumer.h>
 #include <asm/irq.h>
 
 /*
@@ -85,6 +86,7 @@ struct ads7846 {
 	char			name[32];
 
 	struct spi_device	*spi;
+	struct regulator	*reg;
 
 #if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
 	struct attribute_group	*attr_group;
@@ -788,6 +790,8 @@ static void ads7846_disable(struct ads7846 *ts)
 		}
 	}
 
+	regulator_disable(ts->reg);
+
 	/* we know the chip's in lowpower mode since we always
 	 * leave it that way after every request
 	 */
@@ -799,6 +803,8 @@ static void ads7846_enable(struct ads7846 *ts)
 	if (!ts->disabled)
 		return;
 
+	regulator_enable(ts->reg);
+
 	ts->disabled = 0;
 	ts->irq_disabled = 0;
 	enable_irq(ts->spi->irq);
@@ -1139,6 +1145,19 @@ static int __devinit ads7846_probe(struct spi_device *spi)
 
 	ts->last_msg = m;
 
+	ts->reg = regulator_get(&spi->dev, "vcc");
+	if (IS_ERR(ts->reg)) {
+		dev_err(&spi->dev, "unable to get regulator: %ld\n",
+			PTR_ERR(ts->reg));
+		goto err_free_gpio;
+	}
+
+	err = regulator_enable(ts->reg);
+	if (err) {
+		dev_err(&spi->dev, "unable to enable regulator: %d\n", err);
+		goto err_put_regulator;
+	}
+
 	if (request_irq(spi->irq, ads7846_irq, IRQF_TRIGGER_FALLING,
 			spi->dev.driver->name, ts)) {
 		dev_info(&spi->dev,
@@ -1148,7 +1167,7 @@ static int __devinit ads7846_probe(struct spi_device *spi)
 				  spi->dev.driver->name, ts);
 		if (err) {
 			dev_dbg(&spi->dev, "irq %d busy?\n", spi->irq);
-			goto err_free_gpio;
+			goto err_disable_regulator;
 		}
 	}
 
@@ -1180,6 +1199,10 @@ static int __devinit ads7846_probe(struct spi_device *spi)
 	ads784x_hwmon_unregister(spi, ts);
  err_free_irq:
 	free_irq(spi->irq, ts);
+ err_disable_regulator:
+	regulator_disable(ts->reg);
+ err_put_regulator:
+	regulator_put(ts->reg);
  err_free_gpio:
 	if (ts->gpio_pendown != -1)
 		gpio_free(ts->gpio_pendown);
@@ -1208,6 +1231,9 @@ static int __devexit ads7846_remove(struct spi_device *spi)
 	/* suspend left the IRQ disabled */
 	enable_irq(ts->spi->irq);
 
+	regulator_disable(ts->reg);
+	regulator_put(ts->reg);
+
 	if (ts->gpio_pendown != -1)
 		gpio_free(ts->gpio_pendown);
 
-- 
1.6.3.3

Re: [PATCH v2] Input: ads7846: add regulator support

[Mark Brown]

On Sat, Feb 13, 2010 at 06:31:54PM +0200, Grazvydas Ignotas wrote:
> The ADS7846/TSC2046 touchscreen controllers can (and usually are)
> connected to various regulators for power, so add regulator support.
> 
> Valid regulator will now be required, so boards without complete
> regulator setup should either disable regulator framework or enable
> CONFIG_REGULATOR_DUMMY.
> 
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>

Looks good from a regulator API point of view:

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH v2] Input: ads7846: add regulator support

[Mike Rapoport]

Just one nitpicking comment below:

On Sat, Feb 13, 2010 at 6:31 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> The ADS7846/TSC2046 touchscreen controllers can (and usually are)
> connected to various regulators for power, so add regulator support.
>
> Valid regulator will now be required, so boards without complete
> regulator setup should either disable regulator framework or enable
> CONFIG_REGULATOR_DUMMY.
>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> ---
>  drivers/input/touchscreen/ads7846.c |   28 +++++++++++++++++++++++++++-
>  1 files changed, 27 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index 52d2ca1..8b05d8e 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -27,6 +27,7 @@
>  #include <linux/gpio.h>
>  #include <linux/spi/spi.h>
>  #include <linux/spi/ads7846.h>
> +#include <linux/regulator/consumer.h>
>  #include <asm/irq.h>
>
>  /*
> @@ -85,6 +86,7 @@ struct ads7846 {
>        char                    name[32];
>
>        struct spi_device       *spi;
> +       struct regulator        *reg;
>
>  #if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
>        struct attribute_group  *attr_group;
> @@ -788,6 +790,8 @@ static void ads7846_disable(struct ads7846 *ts)
>                }
>        }
>
> +       regulator_disable(ts->reg);
> +
>        /* we know the chip's in lowpower mode since we always
>         * leave it that way after every request
>         */
> @@ -799,6 +803,8 @@ static void ads7846_enable(struct ads7846 *ts)
>        if (!ts->disabled)
>                return;
>
> +       regulator_enable(ts->reg);
> +
>        ts->disabled = 0;
>        ts->irq_disabled = 0;
>        enable_irq(ts->spi->irq);
> @@ -1139,6 +1145,19 @@ static int __devinit ads7846_probe(struct spi_device *spi)
>
>        ts->last_msg = m;
>
> +       ts->reg = regulator_get(&spi->dev, "vcc");

"vcc" is way too generic name. What about "vcc-ts" or "vcc-touch"?

> +       if (IS_ERR(ts->reg)) {
> +               dev_err(&spi->dev, "unable to get regulator: %ld\n",
> +                       PTR_ERR(ts->reg));
> +               goto err_free_gpio;
> +       }
> +
> +       err = regulator_enable(ts->reg);
> +       if (err) {
> +               dev_err(&spi->dev, "unable to enable regulator: %d\n", err);
> +               goto err_put_regulator;
> +       }
> +
>        if (request_irq(spi->irq, ads7846_irq, IRQF_TRIGGER_FALLING,
>                        spi->dev.driver->name, ts)) {
>                dev_info(&spi->dev,
> @@ -1148,7 +1167,7 @@ static int __devinit ads7846_probe(struct spi_device *spi)
>                                  spi->dev.driver->name, ts);
>                if (err) {
>                        dev_dbg(&spi->dev, "irq %d busy?\n", spi->irq);
> -                       goto err_free_gpio;
> +                       goto err_disable_regulator;
>                }
>        }
>
> @@ -1180,6 +1199,10 @@ static int __devinit ads7846_probe(struct spi_device *spi)
>        ads784x_hwmon_unregister(spi, ts);
>  err_free_irq:
>        free_irq(spi->irq, ts);
> + err_disable_regulator:
> +       regulator_disable(ts->reg);
> + err_put_regulator:
> +       regulator_put(ts->reg);
>  err_free_gpio:
>        if (ts->gpio_pendown != -1)
>                gpio_free(ts->gpio_pendown);
> @@ -1208,6 +1231,9 @@ static int __devexit ads7846_remove(struct spi_device *spi)
>        /* suspend left the IRQ disabled */
>        enable_irq(ts->spi->irq);
>
> +       regulator_disable(ts->reg);
> +       regulator_put(ts->reg);
> +
>        if (ts->gpio_pendown != -1)
>                gpio_free(ts->gpio_pendown);
>
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
	Sincerely Yours,
		Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Input: ads7846: add regulator support

[Mark Brown]

On Sat, Feb 13, 2010 at 09:40:26PM +0200, Mike Rapoport wrote:
> On Sat, Feb 13, 2010 at 6:31 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:

> > +       ts->reg = regulator_get(&spi->dev, "vcc");

> "vcc" is way too generic name. What about "vcc-ts" or "vcc-touch"?

This should be OK as-is - the general style for regulators is to use
whatever the name the datasheet gives to the supply.  The string used
is namespaced using the supplied struct device and isn't connected to
the strings used to name the regulator providing the supply so there
should be no cause for name collisions.

Only supplies for things like CPUs which don't currently have struct
devices associated with them have issues with name collisions.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Input: ads7846: add regulator support

[Dmitry Torokhov]

On Sat, Feb 13, 2010 at 06:49:11PM +0000, Mark Brown wrote:
> On Sat, Feb 13, 2010 at 06:31:54PM +0200, Grazvydas Ignotas wrote:
> > The ADS7846/TSC2046 touchscreen controllers can (and usually are)
> > connected to various regulators for power, so add regulator support.
> > 
> > Valid regulator will now be required, so boards without complete
> > regulator setup should either disable regulator framework or enable
> > CONFIG_REGULATOR_DUMMY.
> > 
> > Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> 
> Looks good from a regulator API point of view:
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Will apply to next, thank you Grazvydas.

-- 
Dmitry

Re: [PATCH v2] Input: ads7846: add regulator support

[Steve Sakoman]

On Sat, Feb 13, 2010 at 9:31 AM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> The ADS7846/TSC2046 touchscreen controllers can (and usually are)
> connected to various regulators for power, so add regulator support.
>
> Valid regulator will now be required, so boards without complete
> regulator setup should either disable regulator framework or enable
> CONFIG_REGULATOR_DUMMY.

I'd prefer not to completely disable the regulator framework and my
2.6.33 kernel tree doesn't seem to have the CONFIG_REGULATOR_DUMMY
option.

Is there a simple way to handle the case (as exists with the Gumstix
Overo) where the ADS7846 is supplied by a fixed regulator with no
software controls?

Steve

Re: [PATCH v2] Input: ads7846: add regulator support

[Grazvydas Ignotas]

On Tue, Apr 20, 2010 at 7:31 PM, Steve Sakoman <sakoman@gmail.com> wrote:
> On Sat, Feb 13, 2010 at 9:31 AM, Grazvydas Ignotas <notasas@gmail.com> wrote:
>> The ADS7846/TSC2046 touchscreen controllers can (and usually are)
>> connected to various regulators for power, so add regulator support.
>>
>> Valid regulator will now be required, so boards without complete
>> regulator setup should either disable regulator framework or enable
>> CONFIG_REGULATOR_DUMMY.
>
> I'd prefer not to completely disable the regulator framework and my
> 2.6.33 kernel tree doesn't seem to have the CONFIG_REGULATOR_DUMMY
> option.

Then how did this patch reach your tree? I don't see this in mainline 2.6.33.

> Is there a simple way to handle the case (as exists with the Gumstix
> Overo) where the ADS7846 is supplied by a fixed regulator with no
> software controls?

Not without patching I guess. Otherwise you can set up
reg-fixed-voltage platform device in board file (see
arch/arm/mach-pxa/raumfeld.c for an example), add dummy regulator
patches from mainline of just revert this patch.

>
> Steve
>