Skip to contentSkip to conversation

Input/Touchscreen Driver: add support AD7877 touchscreen driver (try #5)

PATCH In review Bryan Wu 3 participants 4 messages Oct 16, 2008 – Mar 9, 2009

Subsystems (3)
HID Input drivers AD7877 touchscreen driver
lore
Share & tools
lore

Bryan Wu

author
#0 lore raw

PATCH 1/1 Input/Touchscreen Driver: add support AD7877 touchscreen driver (try #5)

From: Michael Hennerich <[email protected]>

[try #5]
- Fix indention
- Lock spi_async()
- Remove useless header comments
- Use strict_strtoul
- Use EDGE triggered interrupts ?\226?\128?\147 this simplifies some of the IRQ handling.
- Fix error cleanup path
- Remove duplicated code
- SPI access functions parameter use struct spi_device

[try #4] Changelog (based on LKML feedback by Dmitry Torokhov)
- Fix req use after free.
- Fix sysfs interaction (we migh re-eanble irgs upon a sysfs read).
- Fix DAC bogus data read case

[try #3] Changlog (Add feedback from Dmitry Torokhov):
- Change handling of spi_sync / spi_async return value handling
- Remove depreciated dev->power.power_state
- Fix error return path in ad7877_probe
- delete pending kernel timer
- Some minor cleanup (indention, use dev_err etc.)

[try #2] Changelog:
- move locking inside ad7877_enable and ad7877_disable
- use setup_timer
- use input_dev->dev.parent
- fix unregister device
- kill EV_KEY since it's not used
- fix indention style

Apply patch from Philip Douglass:
Make ad7877 ts driver not dependent on Blackfin specific cs_change_per_word_option

Cc Dmitry Torokhov <[email protected]>
Signed-off-by Michael Hennerich <[email protected]>
Signed-off-by Bryan Wu <[email protected]>
---
 drivers/input/touchscreen/Kconfig  |   13 +
 drivers/input/touchscreen/Makefile |    1 +
 drivers/input/touchscreen/ad7877.c |  871 ++++++++++++++++++++++++++++++++++++
 3 files changed, 885 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/ad7877.c
drivers/input/touchscreen/Kconfig +13 -0
index 6e1e8c6..2fc51d0 100644
@@ -29,6 +29,19 @@ config TOUCHSCREEN_ADS7846
 	  To compile this driver as a module, choose M here: the
 	  module will be called ads7846.
 
+config TOUCHSCREEN_AD7877
+	tristate "AD7877 based touchscreens"
+	depends on SPI_MASTER
+	help
+	  Say Y here if you have a touchscreen interface using the
+	  AD7877 controller, and your board-specific initialization
+	  code includes that in its table of SPI devices.
+
+	  If unsure, say N (but it's safe to say "Y").
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7877.
+
 config TOUCHSCREEN_BITSY
 	tristate "Compaq iPAQ H3600 (Bitsy) touchscreen"
 	depends on SA1100_BITSY
drivers/input/touchscreen/Makefile +1 -0
index 15cf290..89a3660 100644
@@ -6,6 +6,7 @@
 
 wm97xx-ts-y := wm97xx-core.o
 
+obj-$(CONFIG_TOUCHSCREEN_AD7877)	+= ad7877.o
 obj-$(CONFIG_TOUCHSCREEN_ADS7846)	+= ads7846.o
 obj-$(CONFIG_TOUCHSCREEN_ATMEL_TSADCC)	+= atmel_tsadcc.o
 obj-$(CONFIG_TOUCHSCREEN_BITSY)		+= h3600_ts_input.o
drivers/input/touchscreen/ad7877.c new file +871 -0

871 lines not on this page · raw message

Signature · 1 line
1.5.6
maintainer
#1 lore raw

Hi Bryan, Michael,

Quoted 12 lines · Bryan Wu

From: Michael Hennerich <[email protected]>

[try #5]
- Fix indention
- Lock spi_async()
- Remove useless header comments
- Use strict_strtoul
- Use EDGE triggered interrupts ?\226?\128?\147 this simplifies some of the IRQ handling.
- Fix error cleanup path
- Remove duplicated code
- SPI access functions parameter use struct spi_device

I looked at the latest version of the driver in Andrew's queue and I think it needs the following changes:

- you can't have parts of bitfield updated from IRQ context and part from process context unless every field is protected by the same spinlock.

- You need more full mutual exclusion between enable and disable so you don't inadvertingly kill the timer if you disable and enable from different processes.

- I think you need serialization in various store() methods so that consistent values are written into chip's registers.

- There were coule of inverted logic errors in disabling chip code.

- Kay is trying to get rid of bus_id in devices, you need to use dev_name() instead.

Could you please try the patch below and let me know if the driver still works so I can commit it. I also did some formatting changes, I hope you don't mind. Thanks!

Signature · 9 lines
Dmitry

Input: ad7877 fixups

Signed-off-by: Dmitry Torokhov <[email protected]>
---

 drivers/input/touchscreen/ad7877.c |  226 ++++++++++++++++--------------------
 1 files changed, 99 insertions(+), 127 deletions(-)
drivers/input/touchscreen/ad7877.c +99 -127
index 6615bcd..6702364 100644
@@ -1,7 +1,7 @@
 /*
  * File:        drivers/input/touchscreen/ad7877.c
  *
- * Based on: 	ads7846.c
+ * Based on:	ads7846.c
  *
  *		Copyright (C) 2006-2008 Michael Hennerich, Analog Devices Inc.
  *

@@ -185,21 +185,24 @@ struct ad7877 {
 	u8			averaging;
 	u8			pen_down_acc_interval;
 
-	u16 			conversion_data[AD7877_NR_SENSE];
+	u16			conversion_data[AD7877_NR_SENSE];
 
 	struct spi_transfer	xfer[AD7877_NR_SENSE + 2];
 	struct spi_message	msg;
 
+	struct mutex		mutex;
+	unsigned		disabled:1;	/* P: mutex */
+	unsigned		gpio3:1;	/* P: mutex */
+	unsigned		gpio4:1;	/* P: mutex */
+
 	spinlock_t		lock;
 	struct timer_list	timer;		/* P: lock */
-	unsigned		pendown:1;	/* P: lock */
 	unsigned		pending:1;	/* P: lock */
-	unsigned		disabled:1;
-	unsigned		gpio3:1;
-	unsigned		gpio4:1;
 };
 
 static int gpio3;
+module_param(gpio3, int, 0);
+MODULE_PARM_DESC(gpio3, "If gpio3 is set to 1 AUX3 acts as GPIO3");
 
 /*
  * ad7877_read/write are only used for initial setup and for sysfs controls.

@@ -208,9 +211,10 @@ static int gpio3;
 
 static int ad7877_read(struct spi_device *spi, u16 reg)
 {
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status, ret;
+	struct ser_req *req;
+	int status, ret;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 

@@ -228,11 +232,8 @@ static int ad7877_read(struct spi_device *spi, u16 reg)
 	spi_message_add_tail(&req->xfer[1], &req->msg);
 
 	status = spi_sync(spi, &req->msg);
+	ret = status ? : req->sample;
 
-	if (status == 0)
-		status = req->msg.status;
-
-	ret = status ? status : req->sample;
 	kfree(req);
 
 	return ret;

@@ -240,9 +241,10 @@ static int ad7877_read(struct spi_device *spi, u16 reg)
 
 static int ad7877_write(struct spi_device *spi, u16 reg, u16 val)
 {
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status;
+	struct ser_req *req;
+	int status;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 

@@ -256,9 +258,6 @@ static int ad7877_write(struct spi_device *spi, u16 reg, u16 val)
 
 	status = spi_sync(spi, &req->msg);
 
-	if (status == 0)
-		status = req->msg.status;
-
 	kfree(req);
 
 	return status;

@@ -266,12 +265,13 @@ static int ad7877_write(struct spi_device *spi, u16 reg, u16 val)
 
 static int ad7877_read_adc(struct spi_device *spi, unsigned command)
 {
-	struct ad7877 		*ts = dev_get_drvdata(&spi->dev);
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status;
-	int			sample;
-	int			i;
+	struct ad7877 *ts = dev_get_drvdata(&spi->dev);
+	struct ser_req *req;
+	int status;
+	int sample;
+	int i;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 

@@ -314,21 +314,18 @@ static int ad7877_read_adc(struct spi_device *spi, unsigned command)
 		spi_message_add_tail(&req->xfer[i], &req->msg);
 
 	status = spi_sync(spi, &req->msg);
-
-	if (status == 0)
-		status = req->msg.status;
-
 	sample = req->sample;
 
 	kfree(req);
-	return status ? status : sample;
+
+	return status ? : sample;
 }
 
 static void ad7877_rx(struct ad7877 *ts)
 {
-	struct input_dev	*input_dev = ts->input;
-	unsigned		Rt;
-	u16			x, y, z1, z2;
+	struct input_dev *input_dev = ts->input;
+	unsigned Rt;
+	u16 x, y, z1, z2;
 
 	x = ts->conversion_data[AD7877_SEQ_XPOS] & MAX_12BIT;
 	y = ts->conversion_data[AD7877_SEQ_YPOS] & MAX_12BIT;

@@ -350,10 +347,7 @@ static void ad7877_rx(struct ad7877 *ts)
 		Rt = (z2 - z1) * x * ts->x_plate_ohms;
 		Rt /= z1;
 		Rt = (Rt + 2047) >> 12;
-	} else
-		Rt = 0;
 
-	if (Rt) {
 		input_report_abs(input_dev, ABS_X, x);
 		input_report_abs(input_dev, ABS_Y, y);
 		input_report_abs(input_dev, ABS_PRESSURE, Rt);

@@ -371,7 +365,7 @@ static inline void ad7877_ts_event_release(struct ad7877 *ts)
 
 static void ad7877_timer(unsigned long handle)
 {
-	struct ad7877	*ts = (void *)handle;
+	struct ad7877 *ts = (void *)handle;
 
 	ad7877_ts_event_release(ts);
 }

@@ -382,78 +376,72 @@ static irqreturn_t ad7877_irq(int irq, void *handle)
 	unsigned long flags;
 	int status;
 
-	/* The repeated conversion sequencer controlled by TMR kicked off too fast.
-	 * We ignore the last and process the sample sequence currently in the queue
-	 * It can't be older than 9.4ms, and we need to avoid that ts->msg
-	 * doesn't get issued twice while in work.
+	/*
+	 * The repeated conversion sequencer controlled by TMR kicked off
+	 * too fast. We ignore the last and process the sample sequence
+	 * currently in the queue. It can't be older than 9.4ms, and we
+	 * need to avoid that ts->msg doesn't get issued twice while in work.
 	 */
 
 	spin_lock_irqsave(&ts->lock, flags);
-	if (ts->pending) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return IRQ_HANDLED;
+	if (!ts->pending) {
+		ts->pending = 1;
+
+		status = spi_async(ts->spi, &ts->msg);
+		if (status)
+			dev_err(&ts->spi->dev, "spi_sync --> %d\n", status);
 	}
-	ts->pending = 1;
 	spin_unlock_irqrestore(&ts->lock, flags);
 
-	status = spi_async(ts->spi, &ts->msg);
-
-	if (status)
-		dev_err(&ts->spi->dev, "spi_sync --> %d\n", status);
-
 	return IRQ_HANDLED;
 }
 
 static void ad7877_callback(void *_ts)
 {
 	struct ad7877 *ts = _ts;
-	unsigned long flags;
 
-	ad7877_rx(ts);
+	spin_lock_irq(&ts->lock);
 
-	spin_lock_irqsave(&ts->lock, flags);
+	ad7877_rx(ts);
 	ts->pending = 0;
 	mod_timer(&ts->timer, jiffies + TS_PEN_UP_TIMEOUT);
-	spin_unlock_irqrestore(&ts->lock, flags);
+
+	spin_unlock_irq(&ts->lock);
 }
 
 static void ad7877_disable(struct ad7877 *ts)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ts->lock, flags);
-	if (ts->disabled) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return;
-	}
+	mutex_lock(&ts->mutex);
 
-	ts->disabled = 1;
-	disable_irq(ts->spi->irq);
-	spin_unlock_irqrestore(&ts->lock, flags);
+	if (!ts->disabled) {
+		ts->disabled = 1;
+		disable_irq(ts->spi->irq);
 
-	while (ts->pending) /* Wait for spi_async callback */
-		msleep(1);
+		/* Wait for spi_async callback */
+		while (ts->pending)
+			msleep(1);
 
-	if (del_timer_sync(&ts->timer))
-		ad7877_ts_event_release(ts);
+		if (del_timer_sync(&ts->timer))
+			ad7877_ts_event_release(ts);
+	}
 
 	/* we know the chip's in lowpower mode since we always
 	 * leave it that way after every request
 	 */
+
+	mutex_unlock(&ts->mutex);
 }
 
 static void ad7877_enable(struct ad7877 *ts)
 {
-	unsigned long flags;
+	mutex_lock(&ts->mutex);
 
-	spin_lock_irqsave(&ts->lock, flags);
 	if (ts->disabled) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return;
+		ts->disabled = 0;
+		enable_irq(ts->spi->irq);
 	}
-	ts->disabled = 0;
-	enable_irq(ts->spi->irq);
-	spin_unlock_irqrestore(&ts->lock, flags);
+
+	mutex_unlock(&ts->mutex);
 }
 
 #define SHOW(name) static ssize_t \

@@ -490,12 +478,11 @@ static ssize_t ad7877_disable_store(struct device *dev,
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);
-
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
 	if (val)
 		ad7877_disable(ts);

@@ -521,16 +508,16 @@ static ssize_t ad7877_dac_store(struct device *dev,
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
-
-	ret = strict_strtoul(buf, 10, &val);
+	int error;
 
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->dac = val & 0xFF;
-
 	ad7877_write(ts->spi, AD7877_REG_DAC, (ts->dac << 4) | AD7877_DAC_CONF);
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -551,16 +538,17 @@ static ssize_t ad7877_gpio3_store(struct device *dev,
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->gpio3 = !!val;
-
 	ad7877_write(ts->spi, AD7877_REG_EXTWRITE, AD7877_EXTW_GPIO_DATA |
 		 (ts->gpio4 << 4) | (ts->gpio3 << 5));
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -581,16 +569,17 @@ static ssize_t ad7877_gpio4_store(struct device *dev,
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->gpio4 = !!val;
-
 	ad7877_write(ts->spi, AD7877_REG_EXTWRITE, AD7877_EXTW_GPIO_DATA |
-		 (ts->gpio4 << 4) | (ts->gpio3 << 5));
+		     (ts->gpio4 << 4) | (ts->gpio3 << 5));
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -685,14 +674,10 @@ static int __devinit ad7877_probe(struct spi_device *spi)
 	}
 
 	ts = kzalloc(sizeof(struct ad7877), GFP_KERNEL);
-	if (!ts)
-		return -ENOMEM;
-
-
 	input_dev = input_allocate_device();
-	if (!input_dev) {
-		kfree(ts);
-		return -ENOMEM;
+	if (!ts || !input_dev) {
+		err = -ENOMEM;
+		goto err_free_mem;
 	}
 
 	dev_set_drvdata(&spi->dev, ts);

@@ -700,6 +685,7 @@ static int __devinit ad7877_probe(struct spi_device *spi)
 	ts->input = input_dev;
 
 	setup_timer(&ts->timer, ad7877_timer, (unsigned long) ts);
+	mutex_init(&ts->mutex);
 	spin_lock_init(&ts->lock);
 
 	ts->model = pdata->model ? : 7877;

@@ -713,7 +699,7 @@ static int __devinit ad7877_probe(struct spi_device *spi)
 	ts->averaging = pdata->averaging;
 	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
 
-	snprintf(ts->phys, sizeof(ts->phys), "%s/inputX", spi->dev.bus_id);
+	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&spi->dev));
 
 	input_dev->name = "AD7877 Touchscreen";
 	input_dev->phys = ts->phys;

@@ -761,30 +747,25 @@ static int __devinit ad7877_probe(struct spi_device *spi)
 	}
 
 	err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
-
-	if (gpio3)
-		err |= device_create_file(&spi->dev, &dev_attr_gpio3);
-	else
-		err |= device_create_file(&spi->dev, &dev_attr_aux3);
-
 	if (err)
 		goto err_free_irq;
 
+	err = device_create_file(&spi->dev,
+				 gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
+	if (err)
+		goto err_remove_attr_group;
+
 	err = input_register_device(input_dev);
 	if (err)
 		goto err_remove_attr;
 
-	dev_info(&spi->dev, "touchscreen, irq %d\n", spi->irq);
-
 	return 0;
 
 err_remove_attr:
+	device_remove_file(&spi->dev,
+			   gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
+err_remove_attr_group:
 	sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
-
-	if (gpio3)
-		device_remove_file(&spi->dev, &dev_attr_gpio3);
-	else
-		device_remove_file(&spi->dev, &dev_attr_aux3);
 err_free_irq:
 	free_irq(spi->irq, ts);
 err_free_mem:

@@ -798,19 +779,14 @@ static int __devexit ad7877_remove(struct spi_device *spi)
 {
 	struct ad7877		*ts = dev_get_drvdata(&spi->dev);
 
-	ad7877_disable(ts);
-
 	sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
+	device_remove_file(&spi->dev,
+			   gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
 
-	if (gpio3)
-		device_remove_file(&spi->dev, &dev_attr_gpio3);
-	else
-		device_remove_file(&spi->dev, &dev_attr_aux3);
-
+	ad7877_disable(ts);
 	free_irq(ts->spi->irq, ts);
 
 	input_unregister_device(ts->input);
-
 	kfree(ts);
 
 	dev_dbg(&spi->dev, "unregistered touchscreen\n");

@@ -866,10 +842,6 @@ static void __exit ad7877_exit(void)
 }
 module_exit(ad7877_exit);
 
-module_param(gpio3, int, 0);
-MODULE_PARM_DESC(gpio3,
-	"If gpio3 is set to 1 AUX3 acts as GPIO3");
-
 MODULE_AUTHOR("Michael Hennerich <[email protected]>");
 MODULE_DESCRIPTION("AD7877 touchscreen Driver");
 MODULE_LICENSE("GPL");
maintainer
#2 lore raw

RE: [PATCH 1/1] Input/Touchscreen Driver: add support AD7877touchscreen driver (try #5)

Quoted 4 lines

From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH 1/1] Input/Touchscreen Driver: add support AD7877touchscreen driver (try #5)

Quoted 11 lines

Hi Bryan, Michael,

Quoted 8 lines · Bryan Wu

From: Michael Hennerich <[email protected]>

[try #5]
- Fix indention
- Lock spi_async()
- Remove useless header comments
- Use strict_strtoul
- Use EDGE triggered interrupts ?\226?\128?\147 this simplifies some

of

Quoted 10 lines

the IRQ handling.

Quoted 4 lines

- Fix error cleanup path
- Remove duplicated code
- SPI access functions parameter use struct spi_device

I looked at the latest version of the driver in Andrew's queue and I think it needs the following changes:

- you can't have parts of bitfield updated from IRQ context and part

from

Quoted 6 lines

process context unless every field is protected by the same spinlock.

- You need more full mutual exclusion between enable and disable so you don't inadvertingly kill the timer if you disable and enable from different processes.

Quoted 7 lines

- I think you need serialization in various store() methods so that consistent values are written into chip's registers.

- There were coule of inverted logic errors in disabling chip code.

- Kay is trying to get rid of bus_id in devices, you need to use dev_name() instead.

Andrew folded a patch addressing this issue, submitted by [[email protected]: touchscreen/ad787x: don't use bus_id] into his

[patch 03/22] input: AD7879 Touchscreen driver patch.

You must have missed this one.

Quoted 2 lines

Could you please try the patch below and let me know if the driver

still

Quoted 1 line

works so I can commit it. I also did some formatting changes, I hope

you

Quoted 1 line

don't mind. Thanks!

Thanks a lot for your great efforts!

But your patch doesn't apply on Andrews folded patch send to you last Wednesday. I also tried it without the "don't use bus_id" patch.

Can you please send me your modified files?

Best regards,
Michael

Quoted 10 lines

--
Dmitry

Input: ad7877 fixups

Signed-off-by Dmitry Torokhov <[email protected]>

---

drivers/input/touchscreen/ad7877.c | 226

++++++++++++++++----------------

Quoted 10 lines

----

1 files changed, 99 insertions(+), 127 deletions(-)
drivers/input/touchscreen/ad7877.c +0 -0
index 6615bcd..6702364 100644

drivers/input/touchscreen/ad7877.c line 6

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +1 -1
@@ -1,7 +1,7 @@
 /*
 * File:        drivers/input/touchscreen/ad7877.c
 *
- * Based on: 	ads7846.c
+ * Based on:	ads7846.c
 *
 *		Copyright (C) 2006-2008 Michael Hennerich, Analog

Devices Inc.

drivers/input/touchscreen/ad7877.c static int ad7877_write(struct spi_device *spi,

Quoted 85 lines
drivers/input/touchscreen/ad7877.c +8 -5
 *

@@ -185,21 +185,24 @@ struct ad7877 {
 	u8			averaging;
 	u8			pen_down_acc_interval;
 
-	u16 			conversion_data[AD7877_NR_SENSE];
+	u16			conversion_data[AD7877_NR_SENSE];
 
 	struct spi_transfer	xfer[AD7877_NR_SENSE + 2];
 	struct spi_message	msg;
 
+	struct mutex		mutex;
+	unsigned		disabled:1;	/* P: mutex */
+	unsigned		gpio3:1;	/* P: mutex */
+	unsigned		gpio4:1;	/* P: mutex */
+
 	spinlock_t		lock;
 	struct timer_list	timer;		/* P: lock */
-	unsigned		pendown:1;	/* P: lock */
 	unsigned		pending:1;	/* P: lock */
-	unsigned		disabled:1;
-	unsigned		gpio3:1;
-	unsigned		gpio4:1;
 };
 
 static int gpio3;
+module_param(gpio3, int, 0);
+MODULE_PARM_DESC(gpio3, "If gpio3 is set to 1 AUX3 acts as GPIO3");
 
 /*
 * ad7877_read/write are only used for initial setup and for sysfs

controls.

drivers/input/touchscreen/ad7877.c +4 -6
@@ -208,9 +211,10 @@ static int gpio3;
 
 static int ad7877_read(struct spi_device *spi, u16 reg)
 {
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status, ret;
+	struct ser_req *req;
+	int status, ret;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 

@@ -228,11 +232,8 @@ static int ad7877_read(struct spi_device *spi, u16
 reg)
 	spi_message_add_tail(&req->xfer[1], &req->msg);
 
 	status = spi_sync(spi, &req->msg);
+	ret = status ? : req->sample;
 
-	if (status == 0)
-		status = req->msg.status;
-
-	ret = status ? status : req->sample;
 	kfree(req);
 

	return ret;
drivers/input/touchscreen/ad7877.c +3 -5
@@ -240,9 +241,10 @@ static int ad7877_read(struct spi_device *spi, u16
 reg)
 
 static int ad7877_write(struct spi_device *spi, u16 reg, u16 val)
 {
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status;
+	struct ser_req *req;
+	int status;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;

@@ -256,9 +258,6 @@ static int ad7877_write(struct spi_device *spi, u16
 reg, u16 val)
 
 	status = spi_sync(spi, &req->msg);
 
-	if (status == 0)
-		status = req->msg.status;
-
 	kfree(req);
 

	return status;
drivers/input/touchscreen/ad7877.c +0 -0
@@ -266,12 +265,13 @@ static int ad7877_write(struct spi_device *spi,

u16

drivers/input/touchscreen/ad7877.c static int ad7877_read_adc(struct spi_device

Quoted 20 lines
drivers/input/touchscreen/ad7877.c +6 -5
 reg, u16 val)
 
 static int ad7877_read_adc(struct spi_device *spi, unsigned command)
 {
-	struct ad7877 		*ts = dev_get_drvdata(&spi->dev);
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status;
-	int			sample;
-	int			i;
+	struct ad7877 *ts = dev_get_drvdata(&spi->dev);
+	struct ser_req *req;
+	int status;
+	int sample;
+	int i;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;

@@ -314,21 +314,18 @@ static int ad7877_read_adc(struct spi_device

*spi,

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void

Quoted 49 lines
drivers/input/touchscreen/ad7877.c +5 -8
 unsigned command)
 		spi_message_add_tail(&req->xfer[i], &req->msg);
 
 	status = spi_sync(spi, &req->msg);
-
-	if (status == 0)
-		status = req->msg.status;
-
 	sample = req->sample;
 
 	kfree(req);
-	return status ? status : sample;
+
+	return status ? : sample;
 }
 
 static void ad7877_rx(struct ad7877 *ts)
 {
-	struct input_dev	*input_dev = ts->input;
-	unsigned		Rt;
-	u16			x, y, z1, z2;
+	struct input_dev *input_dev = ts->input;
+	unsigned Rt;
+	u16 x, y, z1, z2;
 
 	x = ts->conversion_data[AD7877_SEQ_XPOS] & MAX_12BIT;

	y = ts->conversion_data[AD7877_SEQ_YPOS] & MAX_12BIT;
drivers/input/touchscreen/ad7877.c +1 -4
@@ -350,10 +347,7 @@ static void ad7877_rx(struct ad7877 *ts)
 		Rt = (z2 - z1) * x * ts->x_plate_ohms;
 		Rt /= z1;
 		Rt = (Rt + 2047) >> 12;
-	} else
-		Rt = 0;
 
-	if (Rt) {
 		input_report_abs(input_dev, ABS_X, x);
 		input_report_abs(input_dev, ABS_Y, y);
 		input_report_abs(input_dev, ABS_PRESSURE, Rt);

@@ -371,7 +365,7 @@ static inline void ad7877_ts_event_release(struct
 ad7877 *ts)
 
 static void ad7877_timer(unsigned long handle)
 {
-	struct ad7877	*ts = (void *)handle;
+	struct ad7877 *ts = (void *)handle;
 
 	ad7877_ts_event_release(ts);

}

drivers/input/touchscreen/ad7877.c +0 -0
@@ -382,78 +376,72 @@ static irqreturn_t ad7877_irq(int irq, void

*handle)

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 4 lines
drivers/input/touchscreen/ad7877.c +0 -1
 	unsigned long flags;
 	int status;
 
-	/* The repeated conversion sequencer controlled by TMR kicked

off too

drivers/input/touchscreen/ad7877.c

Quoted 2 lines

fast.

drivers/input/touchscreen/ad7877.c +0 -1
-	 * We ignore the last and process the sample sequence currently

in

drivers/input/touchscreen/ad7877.c

Quoted 2 lines

the queue

drivers/input/touchscreen/ad7877.c +0 -1
-	 * It can't be older than 9.4ms, and we need to avoid that

ts->msg

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 3 lines
drivers/input/touchscreen/ad7877.c +2 -1
-	 * doesn't get issued twice while in work.
+	/*
+	 * The repeated conversion sequencer controlled by TMR kicked

off

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 3 lines
drivers/input/touchscreen/ad7877.c +3 -0
+	 * too fast. We ignore the last and process the sample sequence
+	 * currently in the queue. It can't be older than 9.4ms, and we
+	 * need to avoid that ts->msg doesn't get issued twice while in

work.

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 12 lines
	 */

	spin_lock_irqsave(&ts->lock, flags);
drivers/input/touchscreen/ad7877.c +6 -3
-	if (ts->pending) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return IRQ_HANDLED;
+	if (!ts->pending) {
+		ts->pending = 1;
+
+		status = spi_async(ts->spi, &ts->msg);
+		if (status)
+			dev_err(&ts->spi->dev, "spi_sync --> %d\n",

status);

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_dac_store(struct device

Quoted 105 lines
	}
drivers/input/touchscreen/ad7877.c +0 -1
-	ts->pending = 1;

	spin_unlock_irqrestore(&ts->lock, flags);
drivers/input/touchscreen/ad7877.c +0 -5
-	status = spi_async(ts->spi, &ts->msg);
-
-	if (status)
-		dev_err(&ts->spi->dev, "spi_sync --> %d\n", status);
-

	return IRQ_HANDLED;
}

static void ad7877_callback(void *_ts)

{
	struct ad7877 *ts = _ts;
drivers/input/touchscreen/ad7877.c +2 -3
-	unsigned long flags;
 
-	ad7877_rx(ts);
+	spin_lock_irq(&ts->lock);
 
-	spin_lock_irqsave(&ts->lock, flags);
+	ad7877_rx(ts);

	ts->pending = 0;
	mod_timer(&ts->timer, jiffies + TS_PEN_UP_TIMEOUT);
drivers/input/touchscreen/ad7877.c +2 -1
-	spin_unlock_irqrestore(&ts->lock, flags);
+
+	spin_unlock_irq(&ts->lock);

}

static void ad7877_disable(struct ad7877 *ts)
{

drivers/input/touchscreen/ad7877.c +10 -14
-	unsigned long flags;
-
-	spin_lock_irqsave(&ts->lock, flags);
-	if (ts->disabled) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return;
-	}
+	mutex_lock(&ts->mutex);
 
-	ts->disabled = 1;
-	disable_irq(ts->spi->irq);
-	spin_unlock_irqrestore(&ts->lock, flags);
+	if (!ts->disabled) {
+		ts->disabled = 1;
+		disable_irq(ts->spi->irq);
 
-	while (ts->pending) /* Wait for spi_async callback */
-		msleep(1);
+		/* Wait for spi_async callback */
+		while (ts->pending)
+			msleep(1);
 
-	if (del_timer_sync(&ts->timer))
-		ad7877_ts_event_release(ts);
+		if (del_timer_sync(&ts->timer))
+			ad7877_ts_event_release(ts);
+	}

	/* we know the chip's in lowpower mode since we always
	 * leave it that way after every request
	 */
drivers/input/touchscreen/ad7877.c +2 -0
+
+	mutex_unlock(&ts->mutex);

}

static void ad7877_enable(struct ad7877 *ts)
{

drivers/input/touchscreen/ad7877.c +1 -2
-	unsigned long flags;
+	mutex_lock(&ts->mutex);
 
-	spin_lock_irqsave(&ts->lock, flags);

	if (ts->disabled) {
drivers/input/touchscreen/ad7877.c +2 -2
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return;
+		ts->disabled = 0;
+		enable_irq(ts->spi->irq);

	}
drivers/input/touchscreen/ad7877.c +2 -3
-	ts->disabled = 0;
-	enable_irq(ts->spi->irq);
-	spin_unlock_irqrestore(&ts->lock, flags);
+
+	mutex_unlock(&ts->mutex);

}

#define SHOW(name) static ssize_t \

drivers/input/touchscreen/ad7877.c +4 -5
@@ -490,12 +478,11 @@ static ssize_t ad7877_disable_store(struct device
 *dev,
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);
-
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
 	if (val)

		ad7877_disable(ts);
drivers/input/touchscreen/ad7877.c +0 -0
@@ -521,16 +508,16 @@ static ssize_t ad7877_dac_store(struct device

*dev,

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio3_store(struct device

Quoted 24 lines
drivers/input/touchscreen/ad7877.c +6 -6
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
-
-	ret = strict_strtoul(buf, 10, &val);
+	int error;
 
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->dac = val & 0xFF;
-
 	ad7877_write(ts->spi, AD7877_REG_DAC, (ts->dac << 4) |
 AD7877_DAC_CONF);
+	mutex_unlock(&ts->mutex);
 
 	return count;

}

drivers/input/touchscreen/ad7877.c +0 -0
@@ -551,16 +538,17 @@ static ssize_t ad7877_gpio3_store(struct device

*dev,

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +1 -2
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio3_store(struct device *dev,

Quoted 10 lines
drivers/input/touchscreen/ad7877.c +4 -3
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->gpio3 = !!val;
-
 	ad7877_write(ts->spi, AD7877_REG_EXTWRITE, AD7877_EXTW_GPIO_DATA

|

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio4_store(struct device

Quoted 6 lines
drivers/input/touchscreen/ad7877.c +1 -0
 		 (ts->gpio4 << 4) | (ts->gpio3 << 5));
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -581,16 +569,17 @@ static ssize_t ad7877_gpio4_store(struct device

*dev,

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +1 -2
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio4_store(struct device *dev,

Quoted 10 lines
drivers/input/touchscreen/ad7877.c +4 -3
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->gpio4 = !!val;
-
 	ad7877_write(ts->spi, AD7877_REG_EXTWRITE, AD7877_EXTW_GPIO_DATA

|

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +2 -1
-		 (ts->gpio4 << 4) | (ts->gpio3 << 5));
+		     (ts->gpio4 << 4) | (ts->gpio3 << 5));
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -685,14 +674,10 @@ static int __devinit ad7877_probe(struct

spi_device

Quoted 28 lines
drivers/input/touchscreen/ad7877.c +3 -7
 *spi)
 	}
 
 	ts = kzalloc(sizeof(struct ad7877), GFP_KERNEL);
-	if (!ts)
-		return -ENOMEM;
-
-
 	input_dev = input_allocate_device();
-	if (!input_dev) {
-		kfree(ts);
-		return -ENOMEM;
+	if (!ts || !input_dev) {
+		err = -ENOMEM;
+		goto err_free_mem;
 	}
 

	dev_set_drvdata(&spi->dev, ts);
drivers/input/touchscreen/ad7877.c +1 -0
@@ -700,6 +685,7 @@ static int __devinit ad7877_probe(struct spi_device
 *spi)
 	ts->input = input_dev;
 
 	setup_timer(&ts->timer, ad7877_timer, (unsigned long) ts);
+	mutex_init(&ts->mutex);
 	spin_lock_init(&ts->lock);
 

	ts->model = pdata->model ? : 7877;

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct spi_device · line 717

Quoted 5 lines
drivers/input/touchscreen/ad7877.c +0 -1
@@ -713,7 +699,7 @@ static int __devinit ad7877_probe(struct spi_device
 *spi)
 	ts->averaging = pdata->averaging;
 	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
 
-	snprintf(ts->phys, sizeof(ts->phys), "%s/inputX",

spi->dev.bus_id);

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct

Quoted 6 lines
drivers/input/touchscreen/ad7877.c +1 -0
+	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&spi-
 >dev));
 

	input_dev->name = "AD7877 Touchscreen";
	input_dev->phys = ts->phys;
drivers/input/touchscreen/ad7877.c +0 -0
@@ -761,30 +747,25 @@ static int __devinit ad7877_probe(struct

spi_device

Quoted 5 lines
drivers/input/touchscreen/ad7877.c +0 -1
 *spi)
 	}
 
 	err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
-

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct spi_device *spi)

Quoted 10 lines
drivers/input/touchscreen/ad7877.c +2 -5
-	if (gpio3)
-		err |= device_create_file(&spi->dev, &dev_attr_gpio3);
-	else
-		err |= device_create_file(&spi->dev, &dev_attr_aux3);
-
 	if (err)
 		goto err_free_irq;
 
+	err = device_create_file(&spi->dev,
+				 gpio3 ? &dev_attr_gpio3 :

&dev_attr_aux3);

drivers/input/touchscreen/ad7877.c static int __devexit ad7877_remove(struct

Quoted 25 lines
drivers/input/touchscreen/ad7877.c +3 -2
+	if (err)
+		goto err_remove_attr_group;
+
 	err = input_register_device(input_dev);
 	if (err)
 		goto err_remove_attr;
 
-	dev_info(&spi->dev, "touchscreen, irq %d\n", spi->irq);
-
 	return 0;

err_remove_attr:

drivers/input/touchscreen/ad7877.c +3 -0
+	device_remove_file(&spi->dev,
+			   gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
+err_remove_attr_group:

	sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
drivers/input/touchscreen/ad7877.c +0 -5
-
-	if (gpio3)
-		device_remove_file(&spi->dev, &dev_attr_gpio3);
-	else
-		device_remove_file(&spi->dev, &dev_attr_aux3);

err_free_irq:

	free_irq(spi->irq, ts);

err_free_mem:

drivers/input/touchscreen/ad7877.c +0 -0
@@ -798,19 +779,14 @@ static int __devexit ad7877_remove(struct

spi_device

Quoted 34 lines
drivers/input/touchscreen/ad7877.c +3 -8
 *spi)
 {
 	struct ad7877		*ts = dev_get_drvdata(&spi->dev);
 
-	ad7877_disable(ts);
-
 	sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
+	device_remove_file(&spi->dev,
+			   gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
 
-	if (gpio3)
-		device_remove_file(&spi->dev, &dev_attr_gpio3);
-	else
-		device_remove_file(&spi->dev, &dev_attr_aux3);
-
+	ad7877_disable(ts);
 	free_irq(ts->spi->irq, ts);
 
 	input_unregister_device(ts->input);
-
 	kfree(ts);
 

	dev_dbg(&spi->dev, "unregistered touchscreen\n");
drivers/input/touchscreen/ad7877.c +0 -4
@@ -866,10 +842,6 @@ static void __exit ad7877_exit(void)
 }
 module_exit(ad7877_exit);
 
-module_param(gpio3, int, 0);
-MODULE_PARM_DESC(gpio3,
-	"If gpio3 is set to 1 AUX3 acts as GPIO3");
-
 MODULE_AUTHOR("Michael Hennerich <[email protected]>");
 MODULE_DESCRIPTION("AD7877 touchscreen Driver");
 MODULE_LICENSE("GPL");
maintainer
#3 lore raw

RE: [PATCH 1/1] Input/Touchscreen Driver: add support AD7877touchscreen driver (try #5)

In reply to Dmitry Torokhov

Hi Dmitry,

Please ignore my last email.
Your patch applied cleanly without the [[email protected]:
touchscreen/ad787x: don't use bus_id] patch.

The other troubles I had were caused by my stupid mail program.

Quoted 1 line

Could you please try the patch below and let me know if the driver

still

Quoted 1 line

works so I can commit it. I also did some formatting changes, I hope

you

Quoted 1 line

don't mind. Thanks!

Thanks a lot. And yes it works perfectly.

Signed-off-by Michael Hennerich <[email protected]>

Best regards,
Michael

Quoted 9 lines

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Sunday, March 08, 2009 7:39 AM
To: Bryan Wu; Hennerich, Michael
Cc: [email protected]; [email protected]; linux- [email protected]
Subject: Re: [PATCH 1/1] Input/Touchscreen Driver: add support AD7877touchscreen driver (try #5)

Quoted 11 lines

Hi Bryan, Michael,

Quoted 8 lines · Bryan Wu

From: Michael Hennerich <[email protected]>

[try #5]
- Fix indention
- Lock spi_async()
- Remove useless header comments
- Use strict_strtoul
- Use EDGE triggered interrupts ?\226?\128?\147 this simplifies some

of

Quoted 10 lines

the IRQ handling.

Quoted 4 lines

- Fix error cleanup path
- Remove duplicated code
- SPI access functions parameter use struct spi_device

I looked at the latest version of the driver in Andrew's queue and I think it needs the following changes:

- you can't have parts of bitfield updated from IRQ context and part

from

Quoted 9 lines

process context unless every field is protected by the same spinlock.

- You need more full mutual exclusion between enable and disable so you don't inadvertingly kill the timer if you disable and enable from different processes.

- I think you need serialization in various store() methods so that consistent values are written into chip's registers.

Quoted 6 lines

- There were coule of inverted logic errors in disabling chip code.

- Kay is trying to get rid of bus_id in devices, you need to use dev_name() instead.

Could you please try the patch below and let me know if the driver

still

Quoted 1 line

works so I can commit it. I also did some formatting changes, I hope

you

Quoted 11 lines

don't mind. Thanks!

--
Dmitry

Input: ad7877 fixups

Signed-off-by Dmitry Torokhov <[email protected]>

---

drivers/input/touchscreen/ad7877.c | 226

++++++++++++++++----------------

Quoted 10 lines

----

1 files changed, 99 insertions(+), 127 deletions(-)
drivers/input/touchscreen/ad7877.c +0 -0
index 6615bcd..6702364 100644

drivers/input/touchscreen/ad7877.c line 6

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +1 -1
@@ -1,7 +1,7 @@
 /*
 * File:        drivers/input/touchscreen/ad7877.c
 *
- * Based on: 	ads7846.c
+ * Based on:	ads7846.c
 *
 *		Copyright (C) 2006-2008 Michael Hennerich, Analog

Devices Inc.

drivers/input/touchscreen/ad7877.c static int ad7877_write(struct spi_device *spi,

Quoted 85 lines
drivers/input/touchscreen/ad7877.c +8 -5
 *

@@ -185,21 +185,24 @@ struct ad7877 {
 	u8			averaging;
 	u8			pen_down_acc_interval;
 
-	u16 			conversion_data[AD7877_NR_SENSE];
+	u16			conversion_data[AD7877_NR_SENSE];
 
 	struct spi_transfer	xfer[AD7877_NR_SENSE + 2];
 	struct spi_message	msg;
 
+	struct mutex		mutex;
+	unsigned		disabled:1;	/* P: mutex */
+	unsigned		gpio3:1;	/* P: mutex */
+	unsigned		gpio4:1;	/* P: mutex */
+
 	spinlock_t		lock;
 	struct timer_list	timer;		/* P: lock */
-	unsigned		pendown:1;	/* P: lock */
 	unsigned		pending:1;	/* P: lock */
-	unsigned		disabled:1;
-	unsigned		gpio3:1;
-	unsigned		gpio4:1;
 };
 
 static int gpio3;
+module_param(gpio3, int, 0);
+MODULE_PARM_DESC(gpio3, "If gpio3 is set to 1 AUX3 acts as GPIO3");
 
 /*
 * ad7877_read/write are only used for initial setup and for sysfs

controls.

drivers/input/touchscreen/ad7877.c +4 -6
@@ -208,9 +211,10 @@ static int gpio3;
 
 static int ad7877_read(struct spi_device *spi, u16 reg)
 {
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status, ret;
+	struct ser_req *req;
+	int status, ret;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 

@@ -228,11 +232,8 @@ static int ad7877_read(struct spi_device *spi, u16
 reg)
 	spi_message_add_tail(&req->xfer[1], &req->msg);
 
 	status = spi_sync(spi, &req->msg);
+	ret = status ? : req->sample;
 
-	if (status == 0)
-		status = req->msg.status;
-
-	ret = status ? status : req->sample;
 	kfree(req);
 

	return ret;
drivers/input/touchscreen/ad7877.c +3 -5
@@ -240,9 +241,10 @@ static int ad7877_read(struct spi_device *spi, u16
 reg)
 
 static int ad7877_write(struct spi_device *spi, u16 reg, u16 val)
 {
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status;
+	struct ser_req *req;
+	int status;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;

@@ -256,9 +258,6 @@ static int ad7877_write(struct spi_device *spi, u16
 reg, u16 val)
 
 	status = spi_sync(spi, &req->msg);
 
-	if (status == 0)
-		status = req->msg.status;
-
 	kfree(req);
 

	return status;
drivers/input/touchscreen/ad7877.c +0 -0
@@ -266,12 +265,13 @@ static int ad7877_write(struct spi_device *spi,

u16

drivers/input/touchscreen/ad7877.c static int ad7877_read_adc(struct spi_device

Quoted 20 lines
drivers/input/touchscreen/ad7877.c +6 -5
 reg, u16 val)
 
 static int ad7877_read_adc(struct spi_device *spi, unsigned command)
 {
-	struct ad7877 		*ts = dev_get_drvdata(&spi->dev);
-	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
-	int			status;
-	int			sample;
-	int			i;
+	struct ad7877 *ts = dev_get_drvdata(&spi->dev);
+	struct ser_req *req;
+	int status;
+	int sample;
+	int i;
 
+	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;

@@ -314,21 +314,18 @@ static int ad7877_read_adc(struct spi_device

*spi,

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void

Quoted 49 lines
drivers/input/touchscreen/ad7877.c +5 -8
 unsigned command)
 		spi_message_add_tail(&req->xfer[i], &req->msg);
 
 	status = spi_sync(spi, &req->msg);
-
-	if (status == 0)
-		status = req->msg.status;
-
 	sample = req->sample;
 
 	kfree(req);
-	return status ? status : sample;
+
+	return status ? : sample;
 }
 
 static void ad7877_rx(struct ad7877 *ts)
 {
-	struct input_dev	*input_dev = ts->input;
-	unsigned		Rt;
-	u16			x, y, z1, z2;
+	struct input_dev *input_dev = ts->input;
+	unsigned Rt;
+	u16 x, y, z1, z2;
 
 	x = ts->conversion_data[AD7877_SEQ_XPOS] & MAX_12BIT;

	y = ts->conversion_data[AD7877_SEQ_YPOS] & MAX_12BIT;
drivers/input/touchscreen/ad7877.c +1 -4
@@ -350,10 +347,7 @@ static void ad7877_rx(struct ad7877 *ts)
 		Rt = (z2 - z1) * x * ts->x_plate_ohms;
 		Rt /= z1;
 		Rt = (Rt + 2047) >> 12;
-	} else
-		Rt = 0;
 
-	if (Rt) {
 		input_report_abs(input_dev, ABS_X, x);
 		input_report_abs(input_dev, ABS_Y, y);
 		input_report_abs(input_dev, ABS_PRESSURE, Rt);

@@ -371,7 +365,7 @@ static inline void ad7877_ts_event_release(struct
 ad7877 *ts)
 
 static void ad7877_timer(unsigned long handle)
 {
-	struct ad7877	*ts = (void *)handle;
+	struct ad7877 *ts = (void *)handle;
 
 	ad7877_ts_event_release(ts);

}

drivers/input/touchscreen/ad7877.c +0 -0
@@ -382,78 +376,72 @@ static irqreturn_t ad7877_irq(int irq, void

*handle)

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 4 lines
drivers/input/touchscreen/ad7877.c +0 -1
 	unsigned long flags;
 	int status;
 
-	/* The repeated conversion sequencer controlled by TMR kicked

off too

drivers/input/touchscreen/ad7877.c

Quoted 2 lines

fast.

drivers/input/touchscreen/ad7877.c +0 -1
-	 * We ignore the last and process the sample sequence currently

in

drivers/input/touchscreen/ad7877.c

Quoted 2 lines

the queue

drivers/input/touchscreen/ad7877.c +0 -1
-	 * It can't be older than 9.4ms, and we need to avoid that

ts->msg

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 3 lines
drivers/input/touchscreen/ad7877.c +2 -1
-	 * doesn't get issued twice while in work.
+	/*
+	 * The repeated conversion sequencer controlled by TMR kicked

off

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 3 lines
drivers/input/touchscreen/ad7877.c +3 -0
+	 * too fast. We ignore the last and process the sample sequence
+	 * currently in the queue. It can't be older than 9.4ms, and we
+	 * need to avoid that ts->msg doesn't get issued twice while in

work.

drivers/input/touchscreen/ad7877.c static irqreturn_t ad7877_irq(int irq, void *handle)

Quoted 12 lines
	 */

	spin_lock_irqsave(&ts->lock, flags);
drivers/input/touchscreen/ad7877.c +6 -3
-	if (ts->pending) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return IRQ_HANDLED;
+	if (!ts->pending) {
+		ts->pending = 1;
+
+		status = spi_async(ts->spi, &ts->msg);
+		if (status)
+			dev_err(&ts->spi->dev, "spi_sync --> %d\n",

status);

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_dac_store(struct device

Quoted 105 lines
	}
drivers/input/touchscreen/ad7877.c +0 -1
-	ts->pending = 1;

	spin_unlock_irqrestore(&ts->lock, flags);
drivers/input/touchscreen/ad7877.c +0 -5
-	status = spi_async(ts->spi, &ts->msg);
-
-	if (status)
-		dev_err(&ts->spi->dev, "spi_sync --> %d\n", status);
-

	return IRQ_HANDLED;
}

static void ad7877_callback(void *_ts)

{
	struct ad7877 *ts = _ts;
drivers/input/touchscreen/ad7877.c +2 -3
-	unsigned long flags;
 
-	ad7877_rx(ts);
+	spin_lock_irq(&ts->lock);
 
-	spin_lock_irqsave(&ts->lock, flags);
+	ad7877_rx(ts);

	ts->pending = 0;
	mod_timer(&ts->timer, jiffies + TS_PEN_UP_TIMEOUT);
drivers/input/touchscreen/ad7877.c +2 -1
-	spin_unlock_irqrestore(&ts->lock, flags);
+
+	spin_unlock_irq(&ts->lock);

}

static void ad7877_disable(struct ad7877 *ts)
{

drivers/input/touchscreen/ad7877.c +10 -14
-	unsigned long flags;
-
-	spin_lock_irqsave(&ts->lock, flags);
-	if (ts->disabled) {
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return;
-	}
+	mutex_lock(&ts->mutex);
 
-	ts->disabled = 1;
-	disable_irq(ts->spi->irq);
-	spin_unlock_irqrestore(&ts->lock, flags);
+	if (!ts->disabled) {
+		ts->disabled = 1;
+		disable_irq(ts->spi->irq);
 
-	while (ts->pending) /* Wait for spi_async callback */
-		msleep(1);
+		/* Wait for spi_async callback */
+		while (ts->pending)
+			msleep(1);
 
-	if (del_timer_sync(&ts->timer))
-		ad7877_ts_event_release(ts);
+		if (del_timer_sync(&ts->timer))
+			ad7877_ts_event_release(ts);
+	}

	/* we know the chip's in lowpower mode since we always
	 * leave it that way after every request
	 */
drivers/input/touchscreen/ad7877.c +2 -0
+
+	mutex_unlock(&ts->mutex);

}

static void ad7877_enable(struct ad7877 *ts)
{

drivers/input/touchscreen/ad7877.c +1 -2
-	unsigned long flags;
+	mutex_lock(&ts->mutex);
 
-	spin_lock_irqsave(&ts->lock, flags);

	if (ts->disabled) {
drivers/input/touchscreen/ad7877.c +2 -2
-		spin_unlock_irqrestore(&ts->lock, flags);
-		return;
+		ts->disabled = 0;
+		enable_irq(ts->spi->irq);

	}
drivers/input/touchscreen/ad7877.c +2 -3
-	ts->disabled = 0;
-	enable_irq(ts->spi->irq);
-	spin_unlock_irqrestore(&ts->lock, flags);
+
+	mutex_unlock(&ts->mutex);

}

#define SHOW(name) static ssize_t \

drivers/input/touchscreen/ad7877.c +4 -5
@@ -490,12 +478,11 @@ static ssize_t ad7877_disable_store(struct device
 *dev,
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);
-
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
 	if (val)

		ad7877_disable(ts);
drivers/input/touchscreen/ad7877.c +0 -0
@@ -521,16 +508,16 @@ static ssize_t ad7877_dac_store(struct device

*dev,

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio3_store(struct device

Quoted 24 lines
drivers/input/touchscreen/ad7877.c +6 -6
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
-
-	ret = strict_strtoul(buf, 10, &val);
+	int error;
 
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->dac = val & 0xFF;
-
 	ad7877_write(ts->spi, AD7877_REG_DAC, (ts->dac << 4) |
 AD7877_DAC_CONF);
+	mutex_unlock(&ts->mutex);
 
 	return count;

}

drivers/input/touchscreen/ad7877.c +0 -0
@@ -551,16 +538,17 @@ static ssize_t ad7877_gpio3_store(struct device

*dev,

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +1 -2
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio3_store(struct device *dev,

Quoted 10 lines
drivers/input/touchscreen/ad7877.c +4 -3
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->gpio3 = !!val;
-
 	ad7877_write(ts->spi, AD7877_REG_EXTWRITE, AD7877_EXTW_GPIO_DATA

|

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio4_store(struct device

Quoted 6 lines
drivers/input/touchscreen/ad7877.c +1 -0
 		 (ts->gpio4 << 4) | (ts->gpio3 << 5));
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -581,16 +569,17 @@ static ssize_t ad7877_gpio4_store(struct device

*dev,

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +1 -2
 {
 	struct ad7877 *ts = dev_get_drvdata(dev);
 	unsigned long val;
-	int ret;
+	int error;
 
-	ret = strict_strtoul(buf, 10, &val);

drivers/input/touchscreen/ad7877.c static ssize_t ad7877_gpio4_store(struct device *dev,

Quoted 10 lines
drivers/input/touchscreen/ad7877.c +4 -3
-	if (ret)
-		return ret;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
+	mutex_lock(&ts->mutex);
 	ts->gpio4 = !!val;
-
 	ad7877_write(ts->spi, AD7877_REG_EXTWRITE, AD7877_EXTW_GPIO_DATA

|

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct

Quoted 7 lines
drivers/input/touchscreen/ad7877.c +2 -1
-		 (ts->gpio4 << 4) | (ts->gpio3 << 5));
+		     (ts->gpio4 << 4) | (ts->gpio3 << 5));
+	mutex_unlock(&ts->mutex);
 
 	return count;
 }

@@ -685,14 +674,10 @@ static int __devinit ad7877_probe(struct

spi_device

Quoted 28 lines
drivers/input/touchscreen/ad7877.c +3 -7
 *spi)
 	}
 
 	ts = kzalloc(sizeof(struct ad7877), GFP_KERNEL);
-	if (!ts)
-		return -ENOMEM;
-
-
 	input_dev = input_allocate_device();
-	if (!input_dev) {
-		kfree(ts);
-		return -ENOMEM;
+	if (!ts || !input_dev) {
+		err = -ENOMEM;
+		goto err_free_mem;
 	}
 

	dev_set_drvdata(&spi->dev, ts);
drivers/input/touchscreen/ad7877.c +1 -0
@@ -700,6 +685,7 @@ static int __devinit ad7877_probe(struct spi_device
 *spi)
 	ts->input = input_dev;
 
 	setup_timer(&ts->timer, ad7877_timer, (unsigned long) ts);
+	mutex_init(&ts->mutex);
 	spin_lock_init(&ts->lock);
 

	ts->model = pdata->model ? : 7877;

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct spi_device · line 717

Quoted 5 lines
drivers/input/touchscreen/ad7877.c +0 -1
@@ -713,7 +699,7 @@ static int __devinit ad7877_probe(struct spi_device
 *spi)
 	ts->averaging = pdata->averaging;
 	ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
 
-	snprintf(ts->phys, sizeof(ts->phys), "%s/inputX",

spi->dev.bus_id);

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct

Quoted 6 lines
drivers/input/touchscreen/ad7877.c +1 -0
+	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&spi-
 >dev));
 

	input_dev->name = "AD7877 Touchscreen";
	input_dev->phys = ts->phys;
drivers/input/touchscreen/ad7877.c +0 -0
@@ -761,30 +747,25 @@ static int __devinit ad7877_probe(struct

spi_device

Quoted 5 lines
drivers/input/touchscreen/ad7877.c +0 -1
 *spi)
 	}
 
 	err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
-

drivers/input/touchscreen/ad7877.c static int __devinit ad7877_probe(struct spi_device *spi)

Quoted 10 lines
drivers/input/touchscreen/ad7877.c +2 -5
-	if (gpio3)
-		err |= device_create_file(&spi->dev, &dev_attr_gpio3);
-	else
-		err |= device_create_file(&spi->dev, &dev_attr_aux3);
-
 	if (err)
 		goto err_free_irq;
 
+	err = device_create_file(&spi->dev,
+				 gpio3 ? &dev_attr_gpio3 :

&dev_attr_aux3);

drivers/input/touchscreen/ad7877.c static int __devexit ad7877_remove(struct

Quoted 25 lines
drivers/input/touchscreen/ad7877.c +3 -2
+	if (err)
+		goto err_remove_attr_group;
+
 	err = input_register_device(input_dev);
 	if (err)
 		goto err_remove_attr;
 
-	dev_info(&spi->dev, "touchscreen, irq %d\n", spi->irq);
-
 	return 0;

err_remove_attr:

drivers/input/touchscreen/ad7877.c +3 -0
+	device_remove_file(&spi->dev,
+			   gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
+err_remove_attr_group:

	sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
drivers/input/touchscreen/ad7877.c +0 -5
-
-	if (gpio3)
-		device_remove_file(&spi->dev, &dev_attr_gpio3);
-	else
-		device_remove_file(&spi->dev, &dev_attr_aux3);

err_free_irq:

	free_irq(spi->irq, ts);

err_free_mem:

drivers/input/touchscreen/ad7877.c +0 -0
@@ -798,19 +779,14 @@ static int __devexit ad7877_remove(struct

spi_device

Quoted 34 lines
drivers/input/touchscreen/ad7877.c +3 -8
 *spi)
 {
 	struct ad7877		*ts = dev_get_drvdata(&spi->dev);
 
-	ad7877_disable(ts);
-
 	sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
+	device_remove_file(&spi->dev,
+			   gpio3 ? &dev_attr_gpio3 : &dev_attr_aux3);
 
-	if (gpio3)
-		device_remove_file(&spi->dev, &dev_attr_gpio3);
-	else
-		device_remove_file(&spi->dev, &dev_attr_aux3);
-
+	ad7877_disable(ts);
 	free_irq(ts->spi->irq, ts);
 
 	input_unregister_device(ts->input);
-
 	kfree(ts);
 

	dev_dbg(&spi->dev, "unregistered touchscreen\n");
drivers/input/touchscreen/ad7877.c +0 -4
@@ -866,10 +842,6 @@ static void __exit ad7877_exit(void)
 }
 module_exit(ad7877_exit);
 
-module_param(gpio3, int, 0);
-MODULE_PARM_DESC(gpio3,
-	"If gpio3 is set to 1 AUX3 acts as GPIO3");
-
 MODULE_AUTHOR("Michael Hennerich <[email protected]>");
 MODULE_DESCRIPTION("AD7877 touchscreen Driver");
 MODULE_LICENSE("GPL");