* [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
@ 2010-01-07 7:18 Mike Frysinger
2010-01-07 7:31 ` Dmitry Torokhov
0 siblings, 1 reply; 15+ messages in thread
From: Mike Frysinger @ 2010-01-07 7:18 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: uclinux-dist-devel, Michael Hennerich
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 7:18 [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib Mike Frysinger
@ 2010-01-07 7:31 ` Dmitry Torokhov
2010-01-07 16:16 ` [Uclinux-dist-devel] " Robin Getz
2010-01-12 21:04 ` [PATCH v2] " Mike Frysinger
0 siblings, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2010-01-07 7:31 UTC (permalink / raw)
To: Mike Frysinger; +Cc: linux-input, uclinux-dist-devel, Michael Hennerich
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 7:31 ` Dmitry Torokhov
@ 2010-01-07 16:16 ` Robin Getz
2010-01-07 16:46 ` Mike Frysinger
2010-01-12 21:04 ` [PATCH v2] " Mike Frysinger
1 sibling, 1 reply; 15+ messages in thread
From: Robin Getz @ 2010-01-07 16:16 UTC (permalink / raw)
To: uclinux-dist-devel
Cc: Dmitry Torokhov, Mike Frysinger, Hennerich, Michael, linux-input
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 16:16 ` [Uclinux-dist-devel] " Robin Getz
@ 2010-01-07 16:46 ` Mike Frysinger
2010-01-07 17:07 ` Dmitry Torokhov
0 siblings, 1 reply; 15+ messages in thread
From: Mike Frysinger @ 2010-01-07 16:46 UTC (permalink / raw)
To: Robin Getz
Cc: uclinux-dist-devel, Dmitry Torokhov, Hennerich, Michael, linux-input
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 16:46 ` Mike Frysinger
@ 2010-01-07 17:07 ` Dmitry Torokhov
2010-01-07 17:43 ` Robin Getz
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2010-01-07 17:07 UTC (permalink / raw)
To: Mike Frysinger
Cc: Robin Getz, uclinux-dist-devel, Hennerich, Michael, linux-input
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 17:07 ` Dmitry Torokhov
@ 2010-01-07 17:43 ` Robin Getz
2010-01-10 7:48 ` Dmitry Torokhov
0 siblings, 1 reply; 15+ messages in thread
From: Robin Getz @ 2010-01-07 17:43 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mike Frysinger, uclinux-dist-devel, Hennerich, Michael, linux-input
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 17:43 ` Robin Getz
@ 2010-01-10 7:48 ` Dmitry Torokhov
2010-01-12 13:57 ` Hennerich, Michael
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2010-01-10 7:48 UTC (permalink / raw)
To: Robin Getz
Cc: Mike Frysinger, uclinux-dist-devel, Hennerich, Michael, linux-input
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [Uclinux-dist-devel] [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-10 7:48 ` Dmitry Torokhov
@ 2010-01-12 13:57 ` Hennerich, Michael
0 siblings, 0 replies; 15+ messages in thread
From: Hennerich, Michael @ 2010-01-12 13:57 UTC (permalink / raw)
To: Dmitry Torokhov, Robin Getz
Cc: Mike Frysinger, uclinux-dist-devel, linux-input
>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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-07 7:31 ` Dmitry Torokhov
2010-01-07 16:16 ` [Uclinux-dist-devel] " Robin Getz
@ 2010-01-12 21:04 ` Mike Frysinger
2010-01-13 2:54 ` Dmitry Torokhov
1 sibling, 1 reply; 15+ messages in thread
From: Mike Frysinger @ 2010-01-12 21:04 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: uclinux-dist-devel, Michael Hennerich
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-12 21:04 ` [PATCH v2] " Mike Frysinger
@ 2010-01-13 2:54 ` Dmitry Torokhov
2010-01-13 9:46 ` Hennerich, Michael
2010-01-17 15:36 ` [PATCH v3] " Mike Frysinger
0 siblings, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2010-01-13 2:54 UTC (permalink / raw)
To: Mike Frysinger; +Cc: linux-input, uclinux-dist-devel, Michael Hennerich
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH v2] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-13 2:54 ` Dmitry Torokhov
@ 2010-01-13 9:46 ` Hennerich, Michael
[not found] ` <8A42379416420646B9BFAC9682273B6D0F0DF094-pcKY8lWzTjquVPpjEGsWsTcYPEmu4y7e@public.gmane.org>
2010-01-17 15:36 ` [PATCH v3] " Mike Frysinger
1 sibling, 1 reply; 15+ messages in thread
From: Hennerich, Michael @ 2010-01-13 9:46 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger; +Cc: linux-input, uclinux-dist-devel
>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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-13 2:54 ` Dmitry Torokhov
2010-01-13 9:46 ` Hennerich, Michael
@ 2010-01-17 15:36 ` Mike Frysinger
2010-01-18 6:35 ` Dmitry Torokhov
1 sibling, 1 reply; 15+ messages in thread
From: Mike Frysinger @ 2010-01-17 15:36 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: uclinux-dist-devel, Michael Hennerich
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-17 15:36 ` [PATCH v3] " Mike Frysinger
@ 2010-01-18 6:35 ` Dmitry Torokhov
2010-01-18 6:48 ` [Uclinux-dist-devel] " Mike Frysinger
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2010-01-18 6:35 UTC (permalink / raw)
To: Mike Frysinger; +Cc: linux-input, uclinux-dist-devel, Michael Hennerich
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Uclinux-dist-devel] [PATCH v3] Input: ad7879: support auxiliary GPIOs via gpiolib
2010-01-18 6:35 ` Dmitry Torokhov
@ 2010-01-18 6:48 ` Mike Frysinger
0 siblings, 0 replies; 15+ messages in thread
From: Mike Frysinger @ 2010-01-18 6:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: uclinux-dist-devel, Michael Hennerich, linux-input
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2010-01-18 6:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-07 7:18 [PATCH] Input: ad7879: support auxiliary GPIOs via gpiolib Mike Frysinger
2010-01-07 7:31 ` Dmitry Torokhov
2010-01-07 16:16 ` [Uclinux-dist-devel] " Robin Getz
2010-01-07 16:46 ` Mike Frysinger
2010-01-07 17:07 ` Dmitry Torokhov
2010-01-07 17:43 ` Robin Getz
2010-01-10 7:48 ` Dmitry Torokhov
2010-01-12 13:57 ` Hennerich, Michael
2010-01-12 21:04 ` [PATCH v2] " Mike Frysinger
2010-01-13 2:54 ` Dmitry Torokhov
2010-01-13 9:46 ` Hennerich, Michael
[not found] ` <8A42379416420646B9BFAC9682273B6D0F0DF094-pcKY8lWzTjquVPpjEGsWsTcYPEmu4y7e@public.gmane.org>
2010-01-14 6:24 ` Dmitry Torokhov
2010-01-17 15:36 ` [PATCH v3] " Mike Frysinger
2010-01-18 6:35 ` Dmitry Torokhov
2010-01-18 6:48 ` [Uclinux-dist-devel] " Mike Frysinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).