[PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
the gpiolib interface so that other drivers can use them.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/input/touchscreen/ad7879.c |  149 ++++++++++++++++++++++-------------
 include/linux/spi/ad7879.h         |   12 ++-
 2 files changed, 102 insertions(+), 59 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index c21e6d3..2f43550 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -47,6 +47,7 @@
 #include <linux/workqueue.h>
 #include <linux/spi/spi.h>
 #include <linux/i2c.h>
+#include <linux/gpio.h>
 
 #include <linux/spi/ad7879.h>
 
@@ -132,7 +133,9 @@ struct ad7879 {
 	struct input_dev	*input;
 	struct work_struct	work;
 	struct timer_list	timer;
-
+#ifdef CONFIG_GPIOLIB
+	struct gpio_chip 	gc;
+#endif
 	struct mutex		mutex;
 	unsigned		disabled:1;	/* P: mutex */
 
@@ -150,11 +153,9 @@ struct ad7879 {
 	u8			median;
 	u16			x_plate_ohms;
 	u16			pressure_max;
-	u16			gpio_init;
 	u16			cmd_crtl1;
 	u16			cmd_crtl2;
 	u16			cmd_crtl3;
-	unsigned		gpio:1;
 };
 
 static int ad7879_read(bus_device *, u8);
@@ -237,24 +238,6 @@ static irqreturn_t ad7879_irq(int irq, void *handle)
 
 static void ad7879_setup(struct ad7879 *ts)
 {
-	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
-			AD7879_XPLUS_BIT |
-			AD7879_Z2_BIT |
-			AD7879_Z1_BIT |
-			AD7879_TEMPMASK_BIT |
-			AD7879_AUXVBATMASK_BIT |
-			AD7879_GPIOALERTMASK_BIT;
-
-	ts->cmd_crtl2 = AD7879_PM(AD7879_PM_DYN) | AD7879_DFR |
-			AD7879_AVG(ts->averaging) |
-			AD7879_MFS(ts->median) |
-			AD7879_FCD(ts->first_conversion_delay) |
-			ts->gpio_init;
-
-	ts->cmd_crtl1 = AD7879_MODE_INT | AD7879_MODE_SEQ1 |
-			AD7879_ACQ(ts->acquisition_time) |
-			AD7879_TMR(ts->pen_down_acc_interval);
-
 	ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
 	ad7879_write(ts->bus, AD7879_REG_CTRL3, ts->cmd_crtl3);
 	ad7879_write(ts->bus, AD7879_REG_CTRL1, ts->cmd_crtl1);
@@ -324,48 +307,74 @@ static ssize_t ad7879_disable_store(struct device *dev,
 
 static DEVICE_ATTR(disable, 0664, ad7879_disable_show, ad7879_disable_store);
 
-static ssize_t ad7879_gpio_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
+static struct attribute *ad7879_attributes[] = {
+	&dev_attr_disable.attr,
+	NULL
+};
+
+static const struct attribute_group ad7879_attr_group = {
+	.attrs = ad7879_attributes,
+};
+
+#ifdef CONFIG_GPIOLIB
+int ad7879_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 {
-	struct ad7879 *ts = dev_get_drvdata(dev);
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	int err;
+
+	mutex_lock(&ts->mutex);
+	ts->cmd_crtl2 |= AD7879_GPIO_EN | AD7879_GPIODIR | AD7879_GPIOPOL;
+	err = ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
 
-	return sprintf(buf, "%u\n", ts->gpio);
+	return err;
 }
 
-static ssize_t ad7879_gpio_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
+int ad7879_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int level)
 {
-	struct ad7879 *ts = dev_get_drvdata(dev);
-	unsigned long val;
-	int error;
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	int err;
 
-	error = strict_strtoul(buf, 10, &val);
-	if (error)
-		return error;
+	mutex_lock(&ts->mutex);
+	ts->cmd_crtl2 &= ~AD7879_GPIODIR;
+	ts->cmd_crtl2 |= AD7879_GPIO_EN | AD7879_GPIOPOL;
+	if (level)
+		ts->cmd_crtl2 |= AD7879_GPIO_DATA;
+	else
+		ts->cmd_crtl2 &= ~AD7879_GPIO_DATA;
+
+	err = ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+
+	return err;
+}
+
+int ad7879_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
+{
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	u16 val;
 
 	mutex_lock(&ts->mutex);
-	ts->gpio = !!val;
-	error = ad7879_write(ts->bus, AD7879_REG_CTRL2,
-			   ts->gpio ?
-				ts->cmd_crtl2 & ~AD7879_GPIO_DATA :
-				ts->cmd_crtl2 | AD7879_GPIO_DATA);
+	val = ad7879_read(ts->bus, AD7879_REG_CTRL2);
 	mutex_unlock(&ts->mutex);
 
-	return error ? : count;
+	return !!(val & AD7879_GPIO_DATA);
 }
 
-static DEVICE_ATTR(gpio, 0664, ad7879_gpio_show, ad7879_gpio_store);
+void ad7879_gpio_set_value(struct gpio_chip *chip, unsigned gpio, int value)
+{
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
 
-static struct attribute *ad7879_attributes[] = {
-	&dev_attr_disable.attr,
-	&dev_attr_gpio.attr,
-	NULL
-};
+	mutex_lock(&ts->mutex);
+	if (value)
+		ts->cmd_crtl2 |= AD7879_GPIO_DATA;
+	else
+		ts->cmd_crtl2 &= ~AD7879_GPIO_DATA;
 
-static const struct attribute_group ad7879_attr_group = {
-	.attrs = ad7879_attributes,
-};
+	ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+}
+#endif
 
 static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 {
@@ -403,12 +412,6 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
 	ts->median = pdata->median;
 
-	if (pdata->gpio_output)
-		ts->gpio_init = AD7879_GPIO_EN |
-				(pdata->gpio_default ? 0 : AD7879_GPIO_DATA);
-	else
-		ts->gpio_init = AD7879_GPIO_EN | AD7879_GPIODIR;
-
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&bus->dev));
 
 	input_dev->name = "AD7879 Touchscreen";
@@ -446,6 +449,23 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 		goto err_free_mem;
 	}
 
+	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
+			AD7879_XPLUS_BIT |
+			AD7879_Z2_BIT |
+			AD7879_Z1_BIT |
+			AD7879_TEMPMASK_BIT |
+			AD7879_AUXVBATMASK_BIT |
+			AD7879_GPIOALERTMASK_BIT;
+
+	ts->cmd_crtl2 = AD7879_PM(AD7879_PM_DYN) | AD7879_DFR |
+			AD7879_AVG(ts->averaging) |
+			AD7879_MFS(ts->median) |
+			AD7879_FCD(ts->first_conversion_delay);
+
+	ts->cmd_crtl1 = AD7879_MODE_INT | AD7879_MODE_SEQ1 |
+			AD7879_ACQ(ts->acquisition_time) |
+			AD7879_TMR(ts->pen_down_acc_interval);
+
 	ad7879_setup(ts);
 
 	err = request_irq(bus->irq, ad7879_irq,
@@ -467,6 +487,25 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 	dev_info(&bus->dev, "Rev.%d touchscreen, irq %d\n",
 		 revid >> 8, bus->irq);
 
+#ifdef CONFIG_GPIOLIB
+	if (pdata->gpio_export) {
+		ts->gc.direction_input = ad7879_gpio_direction_input;
+		ts->gc.direction_output = ad7879_gpio_direction_output;
+		ts->gc.get = ad7879_gpio_get_value;
+		ts->gc.set = ad7879_gpio_set_value;
+		ts->gc.can_sleep = 1;
+		ts->gc.base = pdata->gpio_base;
+		ts->gc.ngpio = 1;
+		ts->gc.label = "AD7879-GPIO";
+		ts->gc.owner = THIS_MODULE;
+
+		err = gpiochip_add(&ts->gc);
+		if (err)
+			dev_err(&bus->dev, "failed to register gpio %d\n",
+				ts->gc.base);
+	}
+#endif
+
 	return 0;
 
 err_remove_attr:
diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
index 4231104..7231eab 100644
--- a/include/linux/spi/ad7879.h
+++ b/include/linux/spi/ad7879.h
@@ -28,8 +28,12 @@ struct ad7879_platform_data {
 	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
 	 */
 	u8	median;
-	/* 1 = AUX/VBAT/GPIO set to GPIO Output */
-	u8	gpio_output;
-	/* Initial GPIO pin state (valid if gpio_output = 1) */
-	u8	gpio_default;
+	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
+	 * requires CONFIG_GPIOLIB
+	 */
+	u8	gpio_export;
+	/* identifies the first GPIO number handled by this chip;
+	 * or, if negative, requests dynamic ID allocation.
+	 */
+	s32	gpio_base;
 };
-- 
1.6.6

Re: [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Dmitry Torokhov]

Hi Michael,

On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
> the gpiolib interface so that other drivers can use them.
> 

Would not that mess up current users (if any) that may rely on the
existing sysfs attributes?

Also I do no see you removing the chip when unbinding the driver.

Thanks.

-- 
Dmitry

Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Robin Getz]

On Thu 7 Jan 2010 02:31, Dmitry Torokhov pondered:
> Hi Michael,
> 
> On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
> > From: Michael Hennerich <michael.hennerich@analog.com>
> > 
> > Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
> > the gpiolib interface so that other drivers can use them.
>
> Would not that mess up current users (if any) that may rely on the
> existing sysfs attributes?

We have talked  to the existing users of this (that we know of), and they 
agreed that doing things the "standard" way with gpiolib is the better way to 
move forward on things.

-Robin

Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Mike Frysinger]

On Thu, Jan 7, 2010 at 11:16, Robin Getz wrote:
> On Thu 7 Jan 2010 02:31, Dmitry Torokhov pondered:
>> On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
>> > From: Michael Hennerich <michael.hennerich@analog.com>
>> >
>> > Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
>> > the gpiolib interface so that other drivers can use them.
>>
>> Would not that mess up current users (if any) that may rely on the
>> existing sysfs attributes?
>
> We have talked  to the existing users of this (that we know of), and they
> agreed that doing things the "standard" way with gpiolib is the better way to
> move forward on things.

there is a standard sysfs interface for accessing gpios already too
which this change allows people to utilize
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Dmitry Torokhov]

On Thu, Jan 07, 2010 at 11:46:15AM -0500, Mike Frysinger wrote:
> On Thu, Jan 7, 2010 at 11:16, Robin Getz wrote:
> > On Thu 7 Jan 2010 02:31, Dmitry Torokhov pondered:
> >> On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
> >> > From: Michael Hennerich <michael.hennerich@analog.com>
> >> >
> >> > Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
> >> > the gpiolib interface so that other drivers can use them.
> >>
> >> Would not that mess up current users (if any) that may rely on the
> >> existing sysfs attributes?
> >
> > We have talked  to the existing users of this (that we know of), and they
> > agreed that doing things the "standard" way with gpiolib is the better way to
> > move forward on things.
> 
> there is a standard sysfs interface for accessing gpios already too
> which this change allows people to utilize

Using (and providing) standard interfaces are laudible goal, however
that does not mean we can screw existing users over. Now Robin sais that
you talked to users (at least some) and this being an embedded platform
taht might be OK but generally sysfs is userspace interface and thus are
not to be changed lightly, only extended.

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

Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Robin Getz]

On Thu 7 Jan 2010 12:07, Dmitry Torokhov pondered:
> On Thu, Jan 07, 2010 at 11:46:15AM -0500, Mike Frysinger wrote:
> > On Thu, Jan 7, 2010 at 11:16, Robin Getz wrote:
> > > On Thu 7 Jan 2010 02:31, Dmitry Torokhov pondered:
> > >> On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
> > >> > From: Michael Hennerich <michael.hennerich@analog.com>
> > >> >
> > >> > Drop the simple fancy sysfs hooks for the aux GPIOs and expose these 
via
> > >> > the gpiolib interface so that other drivers can use them.
> > >>
> > >> Would not that mess up current users (if any) that may rely on the
> > >> existing sysfs attributes?
> > >
> > > We have talked  to the existing users of this (that we know of), and 
they
> > > agreed that doing things the "standard" way with gpiolib is the better 
way to
> > > move forward on things.
> > 
> > there is a standard sysfs interface for accessing gpios already too
> > which this change allows people to utilize
> 
> Using (and providing) standard interfaces are laudible goal, however
> that does not mean we can screw existing users over. Now Robin sais that
> you talked to users (at least some) and this being an embedded platform
> taht might be OK but generally sysfs is userspace interface and thus are
> not to be changed lightly, only extended.

I agree - but in this case - the driver in question has only been in mainline 
a few kernel versions (since March'09).

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=history;f=drivers/input/touchscreen/ad7879.c;hb=HEAD

And the only feedback we have gotten was to make this change - so people could 
use the gpio in other drivers more than requests about userspace...

-Robin

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

Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Dmitry Torokhov]

On Thu, Jan 07, 2010 at 12:43:41PM -0500, Robin Getz wrote:
> On Thu 7 Jan 2010 12:07, Dmitry Torokhov pondered:
> > On Thu, Jan 07, 2010 at 11:46:15AM -0500, Mike Frysinger wrote:
> > > On Thu, Jan 7, 2010 at 11:16, Robin Getz wrote:
> > > > On Thu 7 Jan 2010 02:31, Dmitry Torokhov pondered:
> > > >> On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
> > > >> > From: Michael Hennerich <michael.hennerich@analog.com>
> > > >> >
> > > >> > Drop the simple fancy sysfs hooks for the aux GPIOs and expose these 
> via
> > > >> > the gpiolib interface so that other drivers can use them.
> > > >>
> > > >> Would not that mess up current users (if any) that may rely on the
> > > >> existing sysfs attributes?
> > > >
> > > > We have talked  to the existing users of this (that we know of), and 
> they
> > > > agreed that doing things the "standard" way with gpiolib is the better 
> way to
> > > > move forward on things.
> > > 
> > > there is a standard sysfs interface for accessing gpios already too
> > > which this change allows people to utilize
> > 
> > Using (and providing) standard interfaces are laudible goal, however
> > that does not mean we can screw existing users over. Now Robin sais that
> > you talked to users (at least some) and this being an embedded platform
> > taht might be OK but generally sysfs is userspace interface and thus are
> > not to be changed lightly, only extended.
> 
> I agree - but in this case - the driver in question has only been in mainline 
> a few kernel versions (since March'09).
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=history;f=drivers/input/touchscreen/ad7879.c;hb=HEAD
> 
> And the only feedback we have gotten was to make this change - so people could 
> use the gpio in other drivers more than requests about userspace...
> 

OK, fair enough. ANd to limit exposure we probably want to get it into
.33... So is there an updated version of the patch coming (addressing
missing gpiochip_remove)?

Thanks.

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

RE: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib

[Hennerich, Michael]

>From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
>On Thu, Jan 07, 2010 at 12:43:41PM -0500, Robin Getz wrote:
>> On Thu 7 Jan 2010 12:07, Dmitry Torokhov pondered:
>> > On Thu, Jan 07, 2010 at 11:46:15AM -0500, Mike Frysinger wrote:
>> > > On Thu, Jan 7, 2010 at 11:16, Robin Getz wrote:
>> > > > On Thu 7 Jan 2010 02:31, Dmitry Torokhov pondered:
>> > > >> On Thu, Jan 07, 2010 at 02:18:22AM -0500, Mike Frysinger wrote:
>> > > >> > From: Michael Hennerich <michael.hennerich@analog.com>
>> > > >> >
>> > > >> > Drop the simple fancy sysfs hooks for the aux GPIOs and
>expose these
>> via
>> > > >> > the gpiolib interface so that other drivers can use them.
>> > > >>
>> > > >> Would not that mess up current users (if any) that may rely on
>the
>> > > >> existing sysfs attributes?
>> > > >
>> > > > We have talked  to the existing users of this (that we know of),
>and
>> they
>> > > > agreed that doing things the "standard" way with gpiolib is the
>better
>> way to
>> > > > move forward on things.
>> > >
>> > > there is a standard sysfs interface for accessing gpios already
>too
>> > > which this change allows people to utilize
>> >
>> > Using (and providing) standard interfaces are laudible goal, however
>> > that does not mean we can screw existing users over. Now Robin sais
>that
>> > you talked to users (at least some) and this being an embedded
>platform
>> > taht might be OK but generally sysfs is userspace interface and thus
>are
>> > not to be changed lightly, only extended.
>>
>> I agree - but in this case - the driver in question has only been in
>mainline
>> a few kernel versions (since March'09).
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
>2.6.git;a=history;f=drivers/input/touchscreen/ad7879.c;hb=HEAD
>>
>> And the only feedback we have gotten was to make this change - so
>people could
>> use the gpio in other drivers more than requests about userspace...
>>
>
>OK, fair enough. ANd to limit exposure we probably want to get it into
>.33... So is there an updated version of the patch coming (addressing
>missing gpiochip_remove)?

We're going to send an updated patch shortly.

Thanks and best regards,
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
the gpiolib interface so that other drivers can use them.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- call gpiochip_remove as pointed out by Dmitry

 drivers/input/touchscreen/ad7879.c |  158 +++++++++++++++++++++++-------------
 include/linux/spi/ad7879.h         |   12 ++-
 2 files changed, 111 insertions(+), 59 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index c21e6d3..d319d2c 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -47,6 +47,7 @@
 #include <linux/workqueue.h>
 #include <linux/spi/spi.h>
 #include <linux/i2c.h>
+#include <linux/gpio.h>
 
 #include <linux/spi/ad7879.h>
 
@@ -132,7 +133,9 @@ struct ad7879 {
 	struct input_dev	*input;
 	struct work_struct	work;
 	struct timer_list	timer;
-
+#ifdef CONFIG_GPIOLIB
+	struct gpio_chip 	gc;
+#endif
 	struct mutex		mutex;
 	unsigned		disabled:1;	/* P: mutex */
 
@@ -150,11 +153,9 @@ struct ad7879 {
 	u8			median;
 	u16			x_plate_ohms;
 	u16			pressure_max;
-	u16			gpio_init;
 	u16			cmd_crtl1;
 	u16			cmd_crtl2;
 	u16			cmd_crtl3;
-	unsigned		gpio:1;
 };
 
 static int ad7879_read(bus_device *, u8);
@@ -237,24 +238,6 @@ static irqreturn_t ad7879_irq(int irq, void *handle)
 
 static void ad7879_setup(struct ad7879 *ts)
 {
-	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
-			AD7879_XPLUS_BIT |
-			AD7879_Z2_BIT |
-			AD7879_Z1_BIT |
-			AD7879_TEMPMASK_BIT |
-			AD7879_AUXVBATMASK_BIT |
-			AD7879_GPIOALERTMASK_BIT;
-
-	ts->cmd_crtl2 = AD7879_PM(AD7879_PM_DYN) | AD7879_DFR |
-			AD7879_AVG(ts->averaging) |
-			AD7879_MFS(ts->median) |
-			AD7879_FCD(ts->first_conversion_delay) |
-			ts->gpio_init;
-
-	ts->cmd_crtl1 = AD7879_MODE_INT | AD7879_MODE_SEQ1 |
-			AD7879_ACQ(ts->acquisition_time) |
-			AD7879_TMR(ts->pen_down_acc_interval);
-
 	ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
 	ad7879_write(ts->bus, AD7879_REG_CTRL3, ts->cmd_crtl3);
 	ad7879_write(ts->bus, AD7879_REG_CTRL1, ts->cmd_crtl1);
@@ -324,48 +307,74 @@ static ssize_t ad7879_disable_store(struct device *dev,
 
 static DEVICE_ATTR(disable, 0664, ad7879_disable_show, ad7879_disable_store);
 
-static ssize_t ad7879_gpio_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
+static struct attribute *ad7879_attributes[] = {
+	&dev_attr_disable.attr,
+	NULL
+};
+
+static const struct attribute_group ad7879_attr_group = {
+	.attrs = ad7879_attributes,
+};
+
+#ifdef CONFIG_GPIOLIB
+int ad7879_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 {
-	struct ad7879 *ts = dev_get_drvdata(dev);
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	int err;
+
+	mutex_lock(&ts->mutex);
+	ts->cmd_crtl2 |= AD7879_GPIO_EN | AD7879_GPIODIR | AD7879_GPIOPOL;
+	err = ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
 
-	return sprintf(buf, "%u\n", ts->gpio);
+	return err;
 }
 
-static ssize_t ad7879_gpio_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
+int ad7879_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int level)
 {
-	struct ad7879 *ts = dev_get_drvdata(dev);
-	unsigned long val;
-	int error;
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	int err;
 
-	error = strict_strtoul(buf, 10, &val);
-	if (error)
-		return error;
+	mutex_lock(&ts->mutex);
+	ts->cmd_crtl2 &= ~AD7879_GPIODIR;
+	ts->cmd_crtl2 |= AD7879_GPIO_EN | AD7879_GPIOPOL;
+	if (level)
+		ts->cmd_crtl2 |= AD7879_GPIO_DATA;
+	else
+		ts->cmd_crtl2 &= ~AD7879_GPIO_DATA;
+
+	err = ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+
+	return err;
+}
+
+int ad7879_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
+{
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	u16 val;
 
 	mutex_lock(&ts->mutex);
-	ts->gpio = !!val;
-	error = ad7879_write(ts->bus, AD7879_REG_CTRL2,
-			   ts->gpio ?
-				ts->cmd_crtl2 & ~AD7879_GPIO_DATA :
-				ts->cmd_crtl2 | AD7879_GPIO_DATA);
+	val = ad7879_read(ts->bus, AD7879_REG_CTRL2);
 	mutex_unlock(&ts->mutex);
 
-	return error ? : count;
+	return !!(val & AD7879_GPIO_DATA);
 }
 
-static DEVICE_ATTR(gpio, 0664, ad7879_gpio_show, ad7879_gpio_store);
+void ad7879_gpio_set_value(struct gpio_chip *chip, unsigned gpio, int value)
+{
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
 
-static struct attribute *ad7879_attributes[] = {
-	&dev_attr_disable.attr,
-	&dev_attr_gpio.attr,
-	NULL
-};
+	mutex_lock(&ts->mutex);
+	if (value)
+		ts->cmd_crtl2 |= AD7879_GPIO_DATA;
+	else
+		ts->cmd_crtl2 &= ~AD7879_GPIO_DATA;
 
-static const struct attribute_group ad7879_attr_group = {
-	.attrs = ad7879_attributes,
-};
+	ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+}
+#endif
 
 static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 {
@@ -403,12 +412,6 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
 	ts->median = pdata->median;
 
-	if (pdata->gpio_output)
-		ts->gpio_init = AD7879_GPIO_EN |
-				(pdata->gpio_default ? 0 : AD7879_GPIO_DATA);
-	else
-		ts->gpio_init = AD7879_GPIO_EN | AD7879_GPIODIR;
-
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&bus->dev));
 
 	input_dev->name = "AD7879 Touchscreen";
@@ -446,6 +449,23 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 		goto err_free_mem;
 	}
 
+	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
+			AD7879_XPLUS_BIT |
+			AD7879_Z2_BIT |
+			AD7879_Z1_BIT |
+			AD7879_TEMPMASK_BIT |
+			AD7879_AUXVBATMASK_BIT |
+			AD7879_GPIOALERTMASK_BIT;
+
+	ts->cmd_crtl2 = AD7879_PM(AD7879_PM_DYN) | AD7879_DFR |
+			AD7879_AVG(ts->averaging) |
+			AD7879_MFS(ts->median) |
+			AD7879_FCD(ts->first_conversion_delay);
+
+	ts->cmd_crtl1 = AD7879_MODE_INT | AD7879_MODE_SEQ1 |
+			AD7879_ACQ(ts->acquisition_time) |
+			AD7879_TMR(ts->pen_down_acc_interval);
+
 	ad7879_setup(ts);
 
 	err = request_irq(bus->irq, ad7879_irq,
@@ -467,6 +487,25 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 	dev_info(&bus->dev, "Rev.%d touchscreen, irq %d\n",
 		 revid >> 8, bus->irq);
 
+#ifdef CONFIG_GPIOLIB
+	if (pdata->gpio_export) {
+		ts->gc.direction_input = ad7879_gpio_direction_input;
+		ts->gc.direction_output = ad7879_gpio_direction_output;
+		ts->gc.get = ad7879_gpio_get_value;
+		ts->gc.set = ad7879_gpio_set_value;
+		ts->gc.can_sleep = 1;
+		ts->gc.base = pdata->gpio_base;
+		ts->gc.ngpio = 1;
+		ts->gc.label = "AD7879-GPIO";
+		ts->gc.owner = THIS_MODULE;
+
+		err = gpiochip_add(&ts->gc);
+		if (err)
+			dev_err(&bus->dev, "failed to register gpio %d\n",
+				ts->gc.base);
+	}
+#endif
+
 	return 0;
 
 err_remove_attr:
@@ -481,6 +520,15 @@ err_free_mem:
 
 static int __devexit ad7879_destroy(bus_device *bus, struct ad7879 *ts)
 {
+#ifdef CONFIG_GPIOLIB
+	struct ad7879_platform_data *pdata = bus->dev.platform_data;
+	if (pdata->gpio_export) {
+		if (gpiochip_remove(&ts->gc))
+			dev_err(&bus->dev, "failed to remove gpio %d\n",
+				ts->gc.base);
+	}
+#endif
+
 	ad7879_disable(ts);
 	sysfs_remove_group(&ts->bus->dev.kobj, &ad7879_attr_group);
 	free_irq(ts->bus->irq, ts);
diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
index 4231104..7231eab 100644
--- a/include/linux/spi/ad7879.h
+++ b/include/linux/spi/ad7879.h
@@ -28,8 +28,12 @@ struct ad7879_platform_data {
 	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
 	 */
 	u8	median;
-	/* 1 = AUX/VBAT/GPIO set to GPIO Output */
-	u8	gpio_output;
-	/* Initial GPIO pin state (valid if gpio_output = 1) */
-	u8	gpio_default;
+	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
+	 * requires CONFIG_GPIOLIB
+	 */
+	u8	gpio_export;
+	/* identifies the first GPIO number handled by this chip;
+	 * or, if negative, requests dynamic ID allocation.
+	 */
+	s32	gpio_base;
 };
-- 
1.6.6

Re: [PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib

[Dmitry Torokhov]

Hi Mike,

On Tue, Jan 12, 2010 at 04:04:57PM -0500, Mike Frysinger wrote:
> +
> +		err = gpiochip_add(&ts->gc);
> +		if (err)
> +			dev_err(&bus->dev, "failed to register gpio %d\n",
> +				ts->gc.base);
> +	}

So if it fails here...

> +	struct ad7879_platform_data *pdata = bus->dev.platform_data;
> +	if (pdata->gpio_export) {
> +		if (gpiochip_remove(&ts->gc))
> +			dev_err(&bus->dev, "failed to remove gpio %d\n",
> +				ts->gc.base);

... how valid is it to remove it here?

Also, could you stub out ad7879_gptiochip_export/ad7879_gptiochip_unexport
in case of !CONFIG_GPIOLIB?

> +	}
> +#endif
> +
>  	ad7879_disable(ts);
>  	sysfs_remove_group(&ts->bus->dev.kobj, &ad7879_attr_group);
>  	free_irq(ts->bus->irq, ts);
> diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
> index 4231104..7231eab 100644
> --- a/include/linux/spi/ad7879.h
> +++ b/include/linux/spi/ad7879.h
> @@ -28,8 +28,12 @@ struct ad7879_platform_data {
>  	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
>  	 */
>  	u8	median;
> -	/* 1 = AUX/VBAT/GPIO set to GPIO Output */
> -	u8	gpio_output;
> -	/* Initial GPIO pin state (valid if gpio_output = 1) */
> -	u8	gpio_default;
> +	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
> +	 * requires CONFIG_GPIOLIB
> +	 */
> +	u8	gpio_export;

And make this a bool while you are at it.

Thanks!

-- 
Dmitry

RE: [PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib

[Hennerich, Michael]

>From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
>
>Hi Mike,
>
>On Tue, Jan 12, 2010 at 04:04:57PM -0500, Mike Frysinger wrote:
>> +
>> +		err = gpiochip_add(&ts->gc);
>> +		if (err)
>> +			dev_err(&bus->dev, "failed to register gpio
%d\n",
>> +				ts->gc.base);
>> +	}
>
>So if it fails here...
>
>> +	struct ad7879_platform_data *pdata = bus->dev.platform_data;
>> +	if (pdata->gpio_export) {
>> +		if (gpiochip_remove(&ts->gc))
>> +			dev_err(&bus->dev, "failed to remove gpio %d\n",
>> +				ts->gc.base);
>
>... how valid is it to remove it here?

Right - I should use a flag to sense whether the gpiochip was added or
not.

>
>Also, could you stub out
>ad7879_gptiochip_export/ad7879_gptiochip_unexport
>in case of !CONFIG_GPIOLIB?

Can you explain - where do you see those?


>
>> +	}
>> +#endif
>> +
>>  	ad7879_disable(ts);
>>  	sysfs_remove_group(&ts->bus->dev.kobj, &ad7879_attr_group);
>>  	free_irq(ts->bus->irq, ts);
>> diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
>> index 4231104..7231eab 100644
>> --- a/include/linux/spi/ad7879.h
>> +++ b/include/linux/spi/ad7879.h
>> @@ -28,8 +28,12 @@ struct ad7879_platform_data {
>>  	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
>>  	 */
>>  	u8	median;
>> -	/* 1 = AUX/VBAT/GPIO set to GPIO Output */
>> -	u8	gpio_output;
>> -	/* Initial GPIO pin state (valid if gpio_output = 1) */
>> -	u8	gpio_default;
>> +	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
>> +	 * requires CONFIG_GPIOLIB
>> +	 */
>> +	u8	gpio_export;
>
>And make this a bool while you are at it.
>
>Thanks!
>
>--
>Dmitry

-Michael

Re: [PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib

[Dmitry Torokhov]

On Wed, Jan 13, 2010 at 09:46:42AM +0000, Hennerich, Michael wrote:
> >From: Dmitry Torokhov [mailto:dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> >
> >Hi Mike,
> >
> >On Tue, Jan 12, 2010 at 04:04:57PM -0500, Mike Frysinger wrote:
> >> +
> >> +		err = gpiochip_add(&ts->gc);
> >> +		if (err)
> >> +			dev_err(&bus->dev, "failed to register gpio
> %d\n",
> >> +				ts->gc.base);
> >> +	}
> >
> >So if it fails here...
> >
> >> +	struct ad7879_platform_data *pdata = bus->dev.platform_data;
> >> +	if (pdata->gpio_export) {
> >> +		if (gpiochip_remove(&ts->gc))
> >> +			dev_err(&bus->dev, "failed to remove gpio %d\n",
> >> +				ts->gc.base);
> >
> >... how valid is it to remove it here?
> 
> Right - I should use a flag to sense whether the gpiochip was added or
> not.

Another option would be to simply abort loading the driver, I don't have
a strong preference.

> 
> >
> >Also, could you stub out
> >ad7879_gptiochip_export/ad7879_gptiochip_unexport
> >in case of !CONFIG_GPIOLIB?
> 
> Can you explain - where do you see those?
> 

Sorry for not being clear. I was proposing to move the gpio export code
into new functions (and those were suggested name) and stub them out in
case gpiolib is configured out.

-- 
Dmitry

[PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Drop the simple fancy sysfs hooks for the aux GPIOs and expose these via
the gpiolib interface so that other drivers can use them.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- add fixes from Dmitry
	 - abort on gpiochip_add failure
	 - split out the add/remove steps into dedicated funcs
	 - use a bool for gpiochip support

 drivers/input/touchscreen/ad7879.c |  192 +++++++++++++++++++++++++-----------
 include/linux/spi/ad7879.h         |   12 ++-
 2 files changed, 144 insertions(+), 60 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index c21e6d3..a0876a9 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -47,6 +47,7 @@
 #include <linux/workqueue.h>
 #include <linux/spi/spi.h>
 #include <linux/i2c.h>
+#include <linux/gpio.h>
 
 #include <linux/spi/ad7879.h>
 
@@ -132,7 +133,9 @@ struct ad7879 {
 	struct input_dev	*input;
 	struct work_struct	work;
 	struct timer_list	timer;
-
+#ifdef CONFIG_GPIOLIB
+	struct gpio_chip 	gc;
+#endif
 	struct mutex		mutex;
 	unsigned		disabled:1;	/* P: mutex */
 
@@ -150,11 +153,9 @@ struct ad7879 {
 	u8			median;
 	u16			x_plate_ohms;
 	u16			pressure_max;
-	u16			gpio_init;
 	u16			cmd_crtl1;
 	u16			cmd_crtl2;
 	u16			cmd_crtl3;
-	unsigned		gpio:1;
 };
 
 static int ad7879_read(bus_device *, u8);
@@ -237,24 +238,6 @@ static irqreturn_t ad7879_irq(int irq, void *handle)
 
 static void ad7879_setup(struct ad7879 *ts)
 {
-	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
-			AD7879_XPLUS_BIT |
-			AD7879_Z2_BIT |
-			AD7879_Z1_BIT |
-			AD7879_TEMPMASK_BIT |
-			AD7879_AUXVBATMASK_BIT |
-			AD7879_GPIOALERTMASK_BIT;
-
-	ts->cmd_crtl2 = AD7879_PM(AD7879_PM_DYN) | AD7879_DFR |
-			AD7879_AVG(ts->averaging) |
-			AD7879_MFS(ts->median) |
-			AD7879_FCD(ts->first_conversion_delay) |
-			ts->gpio_init;
-
-	ts->cmd_crtl1 = AD7879_MODE_INT | AD7879_MODE_SEQ1 |
-			AD7879_ACQ(ts->acquisition_time) |
-			AD7879_TMR(ts->pen_down_acc_interval);
-
 	ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
 	ad7879_write(ts->bus, AD7879_REG_CTRL3, ts->cmd_crtl3);
 	ad7879_write(ts->bus, AD7879_REG_CTRL1, ts->cmd_crtl1);
@@ -324,48 +307,127 @@ static ssize_t ad7879_disable_store(struct device *dev,
 
 static DEVICE_ATTR(disable, 0664, ad7879_disable_show, ad7879_disable_store);
 
-static ssize_t ad7879_gpio_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
+static struct attribute *ad7879_attributes[] = {
+	&dev_attr_disable.attr,
+	NULL
+};
+
+static const struct attribute_group ad7879_attr_group = {
+	.attrs = ad7879_attributes,
+};
+
+#ifdef CONFIG_GPIOLIB
+int ad7879_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 {
-	struct ad7879 *ts = dev_get_drvdata(dev);
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	int err;
 
-	return sprintf(buf, "%u\n", ts->gpio);
+	mutex_lock(&ts->mutex);
+	ts->cmd_crtl2 |= AD7879_GPIO_EN | AD7879_GPIODIR | AD7879_GPIOPOL;
+	err = ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+
+	return err;
 }
 
-static ssize_t ad7879_gpio_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
+int ad7879_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int level)
 {
-	struct ad7879 *ts = dev_get_drvdata(dev);
-	unsigned long val;
-	int error;
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	int err;
 
-	error = strict_strtoul(buf, 10, &val);
-	if (error)
-		return error;
+	mutex_lock(&ts->mutex);
+	ts->cmd_crtl2 &= ~AD7879_GPIODIR;
+	ts->cmd_crtl2 |= AD7879_GPIO_EN | AD7879_GPIOPOL;
+	if (level)
+		ts->cmd_crtl2 |= AD7879_GPIO_DATA;
+	else
+		ts->cmd_crtl2 &= ~AD7879_GPIO_DATA;
+
+	err = ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+
+	return err;
+}
+
+int ad7879_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
+{
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
+	u16 val;
 
 	mutex_lock(&ts->mutex);
-	ts->gpio = !!val;
-	error = ad7879_write(ts->bus, AD7879_REG_CTRL2,
-			   ts->gpio ?
-				ts->cmd_crtl2 & ~AD7879_GPIO_DATA :
-				ts->cmd_crtl2 | AD7879_GPIO_DATA);
+	val = ad7879_read(ts->bus, AD7879_REG_CTRL2);
 	mutex_unlock(&ts->mutex);
 
-	return error ? : count;
+	return !!(val & AD7879_GPIO_DATA);
 }
 
-static DEVICE_ATTR(gpio, 0664, ad7879_gpio_show, ad7879_gpio_store);
+void ad7879_gpio_set_value(struct gpio_chip *chip, unsigned gpio, int value)
+{
+	struct ad7879 *ts = container_of(chip, struct ad7879, gc);
 
-static struct attribute *ad7879_attributes[] = {
-	&dev_attr_disable.attr,
-	&dev_attr_gpio.attr,
-	NULL
-};
+	mutex_lock(&ts->mutex);
+	if (value)
+		ts->cmd_crtl2 |= AD7879_GPIO_DATA;
+	else
+		ts->cmd_crtl2 &= ~AD7879_GPIO_DATA;
 
-static const struct attribute_group ad7879_attr_group = {
-	.attrs = ad7879_attributes,
-};
+	ad7879_write(ts->bus, AD7879_REG_CTRL2, ts->cmd_crtl2);
+	mutex_unlock(&ts->mutex);
+}
+
+static inline int ad7879_gpio_add(struct device *dev)
+{
+	struct ad7879 *ts = dev_get_drvdata(dev);
+	struct ad7879_platform_data *pdata = dev->platform_data;
+	int ret = 0;
+
+	if (pdata->gpio_export) {
+		ts->gc.direction_input = ad7879_gpio_direction_input;
+		ts->gc.direction_output = ad7879_gpio_direction_output;
+		ts->gc.get = ad7879_gpio_get_value;
+		ts->gc.set = ad7879_gpio_set_value;
+		ts->gc.can_sleep = 1;
+		ts->gc.base = pdata->gpio_base;
+		ts->gc.ngpio = 1;
+		ts->gc.label = "AD7879-GPIO";
+		ts->gc.owner = THIS_MODULE;
+		ts->gc.dev = dev;
+
+		ret = gpiochip_add(&ts->gc);
+		if (ret)
+			dev_err(dev, "failed to register gpio %d\n",
+				ts->gc.base);
+	}
+
+	return ret;
+}
+
+static int ad7879_gpio_remove(struct device *dev)
+{
+	struct ad7879 *ts = dev_get_drvdata(dev);
+	struct ad7879_platform_data *pdata = dev->platform_data;
+	int ret = 0;
+
+	if (pdata->gpio_export) {
+		ret = gpiochip_remove(&ts->gc);
+		if (ret)
+			dev_err(dev, "failed to remove gpio %d\n",
+				ts->gc.base);
+	}
+
+	return ret;
+}
+#else
+static inline int ad7879_gpio_add(struct device *dev)
+{
+	return 0;
+}
+
+static inline int ad7879_gpio_remove(struct device *dev)
+{
+	return 0;
+}
+#endif
 
 static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 {
@@ -403,12 +465,6 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
 	ts->median = pdata->median;
 
-	if (pdata->gpio_output)
-		ts->gpio_init = AD7879_GPIO_EN |
-				(pdata->gpio_default ? 0 : AD7879_GPIO_DATA);
-	else
-		ts->gpio_init = AD7879_GPIO_EN | AD7879_GPIODIR;
-
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&bus->dev));
 
 	input_dev->name = "AD7879 Touchscreen";
@@ -446,6 +502,23 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 		goto err_free_mem;
 	}
 
+	ts->cmd_crtl3 = AD7879_YPLUS_BIT |
+			AD7879_XPLUS_BIT |
+			AD7879_Z2_BIT |
+			AD7879_Z1_BIT |
+			AD7879_TEMPMASK_BIT |
+			AD7879_AUXVBATMASK_BIT |
+			AD7879_GPIOALERTMASK_BIT;
+
+	ts->cmd_crtl2 = AD7879_PM(AD7879_PM_DYN) | AD7879_DFR |
+			AD7879_AVG(ts->averaging) |
+			AD7879_MFS(ts->median) |
+			AD7879_FCD(ts->first_conversion_delay);
+
+	ts->cmd_crtl1 = AD7879_MODE_INT | AD7879_MODE_SEQ1 |
+			AD7879_ACQ(ts->acquisition_time) |
+			AD7879_TMR(ts->pen_down_acc_interval);
+
 	ad7879_setup(ts);
 
 	err = request_irq(bus->irq, ad7879_irq,
@@ -460,15 +533,21 @@ static int __devinit ad7879_construct(bus_device *bus, struct ad7879 *ts)
 	if (err)
 		goto err_free_irq;
 
-	err = input_register_device(input_dev);
+	err = ad7879_gpio_add(&bus->dev);
 	if (err)
 		goto err_remove_attr;
 
+	err = input_register_device(input_dev);
+	if (err)
+		goto err_remove_gpio;
+
 	dev_info(&bus->dev, "Rev.%d touchscreen, irq %d\n",
 		 revid >> 8, bus->irq);
 
 	return 0;
 
+err_remove_gpio:
+	ad7879_gpio_remove(&bus->dev);
 err_remove_attr:
 	sysfs_remove_group(&bus->dev.kobj, &ad7879_attr_group);
 err_free_irq:
@@ -481,6 +560,7 @@ err_free_mem:
 
 static int __devexit ad7879_destroy(bus_device *bus, struct ad7879 *ts)
 {
+	ad7879_gpio_remove(&bus->dev);
 	ad7879_disable(ts);
 	sysfs_remove_group(&ts->bus->dev.kobj, &ad7879_attr_group);
 	free_irq(ts->bus->irq, ts);
diff --git a/include/linux/spi/ad7879.h b/include/linux/spi/ad7879.h
index 4231104..6334cee 100644
--- a/include/linux/spi/ad7879.h
+++ b/include/linux/spi/ad7879.h
@@ -28,8 +28,12 @@ struct ad7879_platform_data {
 	 * 1 = 4, 2 = 8, 3 = 16 (median > averaging)
 	 */
 	u8	median;
-	/* 1 = AUX/VBAT/GPIO set to GPIO Output */
-	u8	gpio_output;
-	/* Initial GPIO pin state (valid if gpio_output = 1) */
-	u8	gpio_default;
+	/* 1 = AUX/VBAT/GPIO export GPIO to gpiolib
+	 * requires CONFIG_GPIOLIB
+	 */
+	bool	gpio_export;
+	/* identifies the first GPIO number handled by this chip;
+	 * or, if negative, requests dynamic ID allocation.
+	 */
+	s32	gpio_base;
 };
-- 
1.6.6

Re: [PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib

[Dmitry Torokhov]

Hi Mike,

On Sun, Jan 17, 2010 at 10:36:42AM -0500, Mike Frysinger wrote:
> +
> +#ifdef CONFIG_GPIOLIB
> +int ad7879_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
>  {

Sparse recommends making this static and it seems like a good idea. The
same goes for the 3 new fucntions below.

> +
> +static inline int ad7879_gpio_add(struct device *dev)

Why inline? Let's compiler decide, it's certainly not a hot path.

> +{
> +	struct ad7879 *ts = dev_get_drvdata(dev);
> +	struct ad7879_platform_data *pdata = dev->platform_data;
> +	int ret = 0;
> +
> +	if (pdata->gpio_export) {
> +		ts->gc.direction_input = ad7879_gpio_direction_input;
> +		ts->gc.direction_output = ad7879_gpio_direction_output;
> +		ts->gc.get = ad7879_gpio_get_value;
> +		ts->gc.set = ad7879_gpio_set_value;
> +		ts->gc.can_sleep = 1;
> +		ts->gc.base = pdata->gpio_base;
> +		ts->gc.ngpio = 1;
> +		ts->gc.label = "AD7879-GPIO";
> +		ts->gc.owner = THIS_MODULE;
> +		ts->gc.dev = dev;
> +
> +		ret = gpiochip_add(&ts->gc);
> +		if (ret)
> +			dev_err(dev, "failed to register gpio %d\n",
> +				ts->gc.base);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ad7879_gpio_remove(struct device *dev)

There is no chance we can handle errors returned by this function so I'd
make it 'void'.

Let me know if you agree with the above, there is no need to resubmit.

Thanks.

-- 
Dmitry

Re: [Uclinux-dist-devel] [PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib

[Mike Frysinger]

On Mon, Jan 18, 2010 at 01:35, Dmitry Torokhov wrote:
> On Sun, Jan 17, 2010 at 10:36:42AM -0500, Mike Frysinger wrote:
>> +#ifdef CONFIG_GPIOLIB
>> +int ad7879_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
>>  {
>
> Sparse recommends making this static and it seems like a good idea. The
> same goes for the 3 new fucntions below.

yep

>> +static inline int ad7879_gpio_add(struct device *dev)
>
> Why inline? Let's compiler decide, it's certainly not a hot path.

ad7879_gpio_remove needed to be inlined because it's called from both
the probe (__devinit) and the remove (__devexit) functions.  might
able to write a smaller version though for the probe function to avoid
this.

ad7879_gpio_add is fine to remove the "inline" as long as "__devinit" is added.

>> +{
>> +     struct ad7879 *ts = dev_get_drvdata(dev);
>> +     struct ad7879_platform_data *pdata = dev->platform_data;
>> +     int ret = 0;
>> +
>> +     if (pdata->gpio_export) {
>> +             ts->gc.direction_input = ad7879_gpio_direction_input;
>> +             ts->gc.direction_output = ad7879_gpio_direction_output;
>> +             ts->gc.get = ad7879_gpio_get_value;
>> +             ts->gc.set = ad7879_gpio_set_value;
>> +             ts->gc.can_sleep = 1;
>> +             ts->gc.base = pdata->gpio_base;
>> +             ts->gc.ngpio = 1;
>> +             ts->gc.label = "AD7879-GPIO";
>> +             ts->gc.owner = THIS_MODULE;
>> +             ts->gc.dev = dev;
>> +
>> +             ret = gpiochip_add(&ts->gc);
>> +             if (ret)
>> +                     dev_err(dev, "failed to register gpio %d\n",
>> +                             ts->gc.base);
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +static int ad7879_gpio_remove(struct device *dev)
>
> There is no chance we can handle errors returned by this function so I'd
> make it 'void'.

right
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html