* Re: [PATCH v2] Input: ads7846 - Replace spinlock by mutex to wrap disable()/enable()
2010-09-01 16:48 ` Dmitry Torokhov
@ 2010-09-02 16:01 ` Dmitry Torokhov
2010-09-03 2:47 ` Jason Wang
2010-09-08 3:07 ` Jason Wang
0 siblings, 2 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2010-09-02 16:01 UTC (permalink / raw)
To: Jason Wang; +Cc: notasas, vapier, linux-input
On Wed, Sep 01, 2010 at 09:48:18AM -0700, Dmitry Torokhov wrote:
> On Wed, Sep 01, 2010 at 06:07:30PM +0800, Jason Wang wrote:
> > Dmitry Torokhov wrote:
> > >Hi Jason,
> > >
> > >On Wed, Aug 25, 2010 at 11:45:27AM +0800, Jason Wang wrote:
> > >>Hi dmitry and others,
> > >>
> > >>could you please to help me review this patch?
> > >>
> > >>Thanks,
> > >>Jason.
> > >>
> > >>Jason Wang wrote:
> > >>>The commit 9114337 introduces regulator operations in the ads7846
> > >>>touchscreen driver. Among these operations, some are called in the
> > >>>spinlock protected context.
> > >>>On most platforms, the regulator operation is achieved through
> > >>>i2c/spi bus transfer operations, some bus transfer operations will
> > >>>call wait_for_completion function. It isn't allowable to call
> > >>>sleepable function in the atomic context. So replace the spinlock with
> > >>>mutex to protect ads7846_disable()/ads7846_enable().
> > >>>
> > >
> > >I am afraid the patch is not correct. ads7846_disable() and
> > >ads7846_enable() check and modify flags (such as irq_disabled and
> > >pending) that are also accessed form timer/interrupt context. Moving to
> > >mutex removes the serialization that used to be there.
> > Thanks for your comments. you are right it is dangerous and
> > unreasonable to use different locks to protect a critical resource.
> > >I wonder if we should start by converting the driver to used threaded
> > >IRQ model with "long playing" interrupt handler so all access happens in
> > >process context and shutdown sequence is simplified.
> > It can solve most issues, but it can't solve this situation. Because
> > here the atomic region in which conflicts happened is from spin_lock
> > instead of irq handler.
> >
>
> Right, but switching to threaded IRQ will allow to use mutex in place
> of the spinlock everywhere.
>
I wonder if the following patch works (or can be made to work)...
--
Dmitry
Input: ads7846 - switch to using threaded IRQ
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/touchscreen/ads7846.c | 876 ++++++++++++++++++-----------------
1 files changed, 446 insertions(+), 430 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 1603193..d1fc8ef 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -17,6 +17,7 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+#include <linux/types.h>
#include <linux/hwmon.h>
#include <linux/init.h>
#include <linux/err.h>
@@ -52,22 +53,23 @@
* files.
*/
-#define TS_POLL_DELAY (1 * 1000000) /* ns delay before the first sample */
-#define TS_POLL_PERIOD (5 * 1000000) /* ns delay between samples */
+#define TS_POLL_DELAY 1 /* ms delay before the first sample */
+#define TS_POLL_PERIOD 5 /* ms delay between samples */
/* this driver doesn't aim at the peak continuous sample rate */
#define SAMPLE_BITS (8 /*cmd*/ + 16 /*sample*/ + 2 /* before, after */)
struct ts_event {
- /* For portability, we can't read 12 bit values using SPI (which
- * would make the controller deliver them as native byteorder u16
+ /*
+ * For portability, we can't read 12 bit values using SPI (which
+ * would make the controller deliver them as native byte order u16
* with msbs zeroed). Instead, we read them as two 8-bit values,
* *** WHICH NEED BYTESWAPPING *** and range adjustment.
*/
u16 x;
u16 y;
u16 z1, z2;
- int ignore;
+ bool ignore;
u8 x_buf[3];
u8 y_buf[3];
};
@@ -110,8 +112,11 @@ struct ads7846 {
struct spi_transfer xfer[18];
struct spi_message msg[5];
- struct spi_message *last_msg;
- int msg_idx;
+ int msg_count;
+ wait_queue_head_t wait;
+
+ bool pendown;
+
int read_cnt;
int read_rep;
int last_read;
@@ -122,14 +127,10 @@ struct ads7846 {
u16 penirq_recheck_delay_usecs;
- spinlock_t lock;
- struct hrtimer timer;
- unsigned pendown:1; /* P: lock */
- unsigned pending:1; /* P: lock */
-// FIXME remove "irq_disabled"
- unsigned irq_disabled:1; /* P: lock */
- unsigned disabled:1;
- unsigned is_suspended:1;
+ struct mutex lock;
+ bool stopped; /* P: lock */
+ bool disabled; /* P: lock */
+ bool suspended; /* P: lock */
int (*filter)(void *data, int data_idx, int *val);
void *filter_data;
@@ -165,7 +166,7 @@ struct ads7846 {
#define ADS_12_BIT (0 << 3)
#define ADS_SER (1 << 2) /* non-differential */
#define ADS_DFR (0 << 2) /* differential */
-#define ADS_PD10_PDOWN (0 << 0) /* lowpower mode + penirq */
+#define ADS_PD10_PDOWN (0 << 0) /* low power mode + penirq */
#define ADS_PD10_ADC_ON (1 << 0) /* ADC on */
#define ADS_PD10_REF_ON (2 << 0) /* vREF on + penirq */
#define ADS_PD10_ALL_ON (3 << 0) /* ADC + vREF on */
@@ -193,6 +194,72 @@ struct ads7846 {
#define REF_ON (READ_12BIT_DFR(x, 1, 1))
#define REF_OFF (READ_12BIT_DFR(y, 0, 0))
+/* Must be called with ts->lock held */
+static void ads7846_stop(struct ads7846 *ts)
+{
+ if (!ts->disabled && !ts->suspended) {
+ /* Signal IRQ thread to stop polling and disable the handler. */
+ ts->stopped = true;
+ mb();
+ wake_up(&ts->wait);
+ disable_irq(ts->spi->irq);
+ }
+}
+
+/* Must be called with ts->lock held */
+static void ads7846_restart(struct ads7846 *ts)
+{
+ if (!ts->disabled && !ts->suspended) {
+ /* Tell IRQ thread that it may poll the device. */
+ ts->stopped = false;
+ mb();
+ enable_irq(ts->spi->irq);
+ }
+}
+
+/* Must be called with ts->lock held */
+static void __ads7846_disable(struct ads7846 *ts)
+{
+ ads7846_stop(ts);
+ regulator_disable(ts->reg);
+
+ /*
+ * We know the chip's in low power mode since we always
+ * leave it that way after every request
+ */
+}
+
+/* Must be called with ts->lock held */
+static void __ads7846_enable(struct ads7846 *ts)
+{
+ regulator_enable(ts->reg);
+ ads7846_restart(ts);
+}
+
+static void ads7846_disable(struct ads7846 *ts)
+{
+ mutex_lock(&ts->lock);
+
+ if (!ts->disabled && !ts->suspended)
+ __ads7846_disable(ts);
+
+ ts->disabled = true;
+
+ mutex_unlock(&ts->lock);
+}
+
+static void ads7846_enable(struct ads7846 *ts)
+{
+ mutex_lock(&ts->lock);
+
+ if (ts->disabled && !ts->suspended)
+ __ads7846_enable(ts);
+
+ ts->disabled = false;
+
+ mutex_unlock(&ts->lock);
+}
+
/*--------------------------------------------------------------------------*/
/*
@@ -219,23 +286,15 @@ struct ads7845_ser_req {
struct spi_transfer xfer[2];
};
-static void ads7846_enable(struct ads7846 *ts);
-static void ads7846_disable(struct ads7846 *ts);
-
-static int device_suspended(struct device *dev)
-{
- struct ads7846 *ts = dev_get_drvdata(dev);
- return ts->is_suspended || ts->disabled;
-}
-
static int ads7846_read12_ser(struct device *dev, unsigned command)
{
- struct spi_device *spi = to_spi_device(dev);
- struct ads7846 *ts = dev_get_drvdata(dev);
- struct ser_req *req = kzalloc(sizeof *req, GFP_KERNEL);
- int status;
- int use_internal;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ads7846 *ts = dev_get_drvdata(dev);
+ struct ser_req *req;
+ int status;
+ int use_internal;
+ req = kzalloc(sizeof *req, GFP_KERNEL);
if (!req)
return -ENOMEM;
@@ -282,11 +341,11 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
CS_CHANGE(req->xfer[5]);
spi_message_add_tail(&req->xfer[5], &req->msg);
- ts->irq_disabled = 1;
- disable_irq(spi->irq);
+ mutex_lock(&ts->lock);
+ ads7846_stop(ts);
status = spi_sync(spi, &req->msg);
- ts->irq_disabled = 0;
- enable_irq(spi->irq);
+ ads7846_restart(ts);
+ mutex_unlock(&ts->lock);
if (status == 0) {
/* on-wire is a must-ignore bit, a BE12 value, then padding */
@@ -301,11 +360,12 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
static int ads7845_read12_ser(struct device *dev, unsigned command)
{
- struct spi_device *spi = to_spi_device(dev);
- struct ads7846 *ts = dev_get_drvdata(dev);
- struct ads7845_ser_req *req = kzalloc(sizeof *req, GFP_KERNEL);
- int status;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ads7846 *ts = dev_get_drvdata(dev);
+ struct ads7845_ser_req *req;
+ int status;
+ req = kzalloc(sizeof *req, GFP_KERNEL);
if (!req)
return -ENOMEM;
@@ -317,11 +377,11 @@ static int ads7845_read12_ser(struct device *dev, unsigned command)
req->xfer[0].len = 3;
spi_message_add_tail(&req->xfer[0], &req->msg);
- ts->irq_disabled = 1;
- disable_irq(spi->irq);
+ mutex_lock(&ts->lock);
+ ads7846_stop(ts);
status = spi_sync(spi, &req->msg);
- ts->irq_disabled = 0;
- enable_irq(spi->irq);
+ ads7846_restart(ts);
+ mutex_unlock(&ts->lock);
if (status == 0) {
/* BE12 value, then padding */
@@ -374,6 +434,7 @@ static inline unsigned vaux_adjust(struct ads7846 *ts, ssize_t v)
/* external resistors may scale vAUX into 0..vREF */
retval *= ts->vref_mv;
retval = retval >> 12;
+
return retval;
}
@@ -384,13 +445,13 @@ static inline unsigned vbatt_adjust(struct ads7846 *ts, ssize_t v)
/* ads7846 has a resistor ladder to scale this signal down */
if (ts->model == 7846)
retval *= 4;
+
return retval;
}
SHOW(in0_input, vaux, vaux_adjust)
SHOW(in1_input, vbatt, vbatt_adjust)
-
static struct attribute *ads7846_attributes[] = {
&dev_attr_temp0.attr,
&dev_attr_temp1.attr,
@@ -498,17 +559,12 @@ static inline void ads784x_hwmon_unregister(struct spi_device *spi,
}
#endif
-static int is_pen_down(struct device *dev)
-{
- struct ads7846 *ts = dev_get_drvdata(dev);
-
- return ts->pendown;
-}
-
static ssize_t ads7846_pen_down_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%u\n", is_pen_down(dev));
+ struct ads7846 *ts = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", ts->pendown);
}
static DEVICE_ATTR(pen_down, S_IRUGO, ads7846_pen_down_show, NULL);
@@ -516,7 +572,7 @@ static DEVICE_ATTR(pen_down, S_IRUGO, ads7846_pen_down_show, NULL);
static ssize_t ads7846_disable_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- struct ads7846 *ts = dev_get_drvdata(dev);
+ struct ads7846 *ts = dev_get_drvdata(dev);
return sprintf(buf, "%u\n", ts->disabled);
}
@@ -531,15 +587,11 @@ static ssize_t ads7846_disable_store(struct device *dev,
if (strict_strtoul(buf, 10, &i))
return -EINVAL;
- spin_lock_irq(&ts->lock);
-
if (i)
ads7846_disable(ts);
else
ads7846_enable(ts);
- spin_unlock_irq(&ts->lock);
-
return count;
}
@@ -569,23 +621,138 @@ static void null_wait_for_sync(void)
{
}
-/*
- * PENIRQ only kicks the timer. The timer only reissues the SPI transfer,
- * to retrieve touchscreen status.
- *
- * The SPI transfer completion callback does the real work. It reports
- * touchscreen events and reactivates the timer (or IRQ) as appropriate.
- */
+static int ads7846_debounce_filter(void *ads, int data_idx, int *val)
+{
+ struct ads7846 *ts = ads;
+
+ if (!ts->read_cnt || (abs(ts->last_read - *val) > ts->debounce_tol)) {
+ /* Start over collecting consistent readings. */
+ ts->read_rep = 0;
+ /*
+ * Repeat it, if this was the first read or the read
+ * wasn't consistent enough.
+ */
+ if (ts->read_cnt < ts->debounce_max) {
+ ts->last_read = *val;
+ ts->read_cnt++;
+ return ADS7846_FILTER_REPEAT;
+ } else {
+ /*
+ * Maximum number of debouncing reached and still
+ * not enough number of consistent readings. Abort
+ * the whole sample, repeat it in the next sampling
+ * period.
+ */
+ ts->read_cnt = 0;
+ return ADS7846_FILTER_IGNORE;
+ }
+ } else {
+ if (++ts->read_rep > ts->debounce_rep) {
+ /*
+ * Got a good reading for this coordinate,
+ * go for the next one.
+ */
+ ts->read_cnt = 0;
+ ts->read_rep = 0;
+ return ADS7846_FILTER_OK;
+ } else {
+ /* Read more values that are consistent. */
+ ts->read_cnt++;
+ return ADS7846_FILTER_REPEAT;
+ }
+ }
+}
+
+static int ads7846_no_filter(void *ads, int data_idx, int *val)
+{
+ return ADS7846_FILTER_OK;
+}
+
+static int ads7846_get_value(struct ads7846 *ts, struct spi_message *m)
+{
+ struct spi_transfer *t =
+ list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
+
+ if (ts->model == 7845) {
+ return be16_to_cpup((__be16 *)&(((char*)t->rx_buf)[1])) >> 3;
+ } else {
+ /*
+ * adjust: on-wire is a must-ignore bit, a BE12 value, then
+ * padding; built from two 8 bit values written msb-first.
+ */
+ return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
+ }
+}
+
+static void ads7846_update_value(struct spi_message *m, int val)
+{
+ struct spi_transfer *t =
+ list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
+
+ *(u16 *)t->rx_buf = val;
+}
+
+static void ads7846_read_state(struct ads7846 *ts)
+{
+ struct ads7846_packet *packet = ts->packet;
+ struct spi_message *m;
+ int msg_idx = 0;
+ int val;
+ int action;
+ int error;
+
+ while (msg_idx < ts->msg_count) {
+
+ ts->wait_for_sync();
+
+ m = &ts->msg[msg_idx];
+ error = spi_sync(ts->spi, m);
+ if (error) {
+ dev_err(&ts->spi->dev, "spi_async --> %d\n", error);
+ packet->tc.ignore = true;
+ return;
+ }
+
+ /*
+ * Last message is power down request, no need to convert
+ * or filter the value.
+ */
+ if (msg_idx < ts->msg_count - 1) {
+
+ val = ads7846_get_value(ts, m);
+
+ action = ts->filter(ts->filter_data, msg_idx, &val);
+ switch (action) {
+ case ADS7846_FILTER_REPEAT:
+ continue;
+
+ case ADS7846_FILTER_IGNORE:
+ packet->tc.ignore = true;
+ return;
-static void ads7846_rx(void *ads)
+ case ADS7846_FILTER_OK:
+ ads7846_update_value(m, val);
+ packet->tc.ignore = false;
+ msg_idx++;
+ break;
+
+ default:
+ BUG();
+ }
+ }
+ }
+}
+
+static void ads7846_report_state(struct ads7846 *ts)
{
- struct ads7846 *ts = ads;
- struct ads7846_packet *packet = ts->packet;
- unsigned Rt;
- u16 x, y, z1, z2;
+ struct ads7846_packet *packet = ts->packet;
+ unsigned int Rt;
+ u16 x, y, z1, z2;
- /* ads7846_rx_val() did in-place conversion (including byteswap) from
- * on-the-wire format as part of debouncing to get stable readings.
+ /*
+ * ads7846_get_value() does in-place conversion (including byte swap)
+ * from on-the-wire format as part of debouncing to get stable
+ * readings.
*/
if (ts->model == 7845) {
x = *(u16 *)packet->tc.x_buf;
@@ -623,19 +790,19 @@ static void ads7846_rx(void *ads)
Rt = 0;
}
- /* Sample found inconsistent by debouncing or pressure is beyond
+ /*
+ * Sample found inconsistent by debouncing or pressure is beyond
* the maximum. Don't report it to user space, repeat at least
* once more the measurement
*/
if (packet->tc.ignore || Rt > ts->pressure_max) {
dev_vdbg(&ts->spi->dev, "ignored %d pressure %d\n",
packet->tc.ignore, Rt);
- hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD),
- HRTIMER_MODE_REL);
return;
}
- /* Maybe check the pendown state before reporting. This discards
+ /*
+ * Maybe check the pendown state before reporting. This discards
* false readings when the pen is lifted.
*/
if (ts->penirq_recheck_delay_usecs) {
@@ -644,8 +811,9 @@ static void ads7846_rx(void *ads)
Rt = 0;
}
- /* NOTE: We can't rely on the pressure to determine the pen down
- * state, even this controller has a pressure sensor. The pressure
+ /*
+ * NOTE: We can't rely on the pressure to determine the pen down
+ * state, even this controller has a pressure sensor. The pressure
* value can fluctuate for quite a while after lifting the pen and
* in some cases may not even settle at the expected value.
*
@@ -655,15 +823,15 @@ static void ads7846_rx(void *ads)
if (Rt) {
struct input_dev *input = ts->input;
+ if (ts->swap_xy)
+ swap(x, y);
+
if (!ts->pendown) {
input_report_key(input, BTN_TOUCH, 1);
- ts->pendown = 1;
+ ts->pendown = true;
dev_vdbg(&ts->spi->dev, "DOWN\n");
}
- if (ts->swap_xy)
- swap(x, y);
-
input_report_abs(input, ABS_X, x);
input_report_abs(input, ABS_Y, y);
input_report_abs(input, ABS_PRESSURE, ts->pressure_max - Rt);
@@ -671,246 +839,94 @@ static void ads7846_rx(void *ads)
input_sync(input);
dev_vdbg(&ts->spi->dev, "%4d/%4d/%4d\n", x, y, Rt);
}
-
- hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD),
- HRTIMER_MODE_REL);
}
-static int ads7846_debounce(void *ads, int data_idx, int *val)
+static irqreturn_t ads7846_hard_irq(int irq, void *handle)
{
- struct ads7846 *ts = ads;
-
- if (!ts->read_cnt || (abs(ts->last_read - *val) > ts->debounce_tol)) {
- /* Start over collecting consistent readings. */
- ts->read_rep = 0;
- /* Repeat it, if this was the first read or the read
- * wasn't consistent enough. */
- if (ts->read_cnt < ts->debounce_max) {
- ts->last_read = *val;
- ts->read_cnt++;
- return ADS7846_FILTER_REPEAT;
- } else {
- /* Maximum number of debouncing reached and still
- * not enough number of consistent readings. Abort
- * the whole sample, repeat it in the next sampling
- * period.
- */
- ts->read_cnt = 0;
- return ADS7846_FILTER_IGNORE;
- }
- } else {
- if (++ts->read_rep > ts->debounce_rep) {
- /* Got a good reading for this coordinate,
- * go for the next one. */
- ts->read_cnt = 0;
- ts->read_rep = 0;
- return ADS7846_FILTER_OK;
- } else {
- /* Read more values that are consistent. */
- ts->read_cnt++;
- return ADS7846_FILTER_REPEAT;
- }
- }
-}
-
-static int ads7846_no_filter(void *ads, int data_idx, int *val)
-{
- return ADS7846_FILTER_OK;
-}
-
-static void ads7846_rx_val(void *ads)
-{
- struct ads7846 *ts = ads;
- struct ads7846_packet *packet = ts->packet;
- struct spi_message *m;
- struct spi_transfer *t;
- int val;
- int action;
- int status;
-
- m = &ts->msg[ts->msg_idx];
- t = list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
-
- if (ts->model == 7845) {
- val = be16_to_cpup((__be16 *)&(((char*)t->rx_buf)[1])) >> 3;
- } else {
- /* adjust: on-wire is a must-ignore bit, a BE12 value, then
- * padding; built from two 8 bit values written msb-first.
- */
- val = be16_to_cpup((__be16 *)t->rx_buf) >> 3;
- }
+ struct ads7846 *ts = handle;
- action = ts->filter(ts->filter_data, ts->msg_idx, &val);
- switch (action) {
- case ADS7846_FILTER_REPEAT:
- break;
- case ADS7846_FILTER_IGNORE:
- packet->tc.ignore = 1;
- /* Last message will contain ads7846_rx() as the
- * completion function.
- */
- m = ts->last_msg;
- break;
- case ADS7846_FILTER_OK:
- *(u16 *)t->rx_buf = val;
- packet->tc.ignore = 0;
- m = &ts->msg[++ts->msg_idx];
- break;
- default:
- BUG();
- }
- ts->wait_for_sync();
- status = spi_async(ts->spi, m);
- if (status)
- dev_err(&ts->spi->dev, "spi_async --> %d\n",
- status);
+ return get_pendown_state(ts) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
}
-static enum hrtimer_restart ads7846_timer(struct hrtimer *handle)
-{
- struct ads7846 *ts = container_of(handle, struct ads7846, timer);
- int status = 0;
-
- spin_lock(&ts->lock);
-
- if (unlikely(!get_pendown_state(ts) ||
- device_suspended(&ts->spi->dev))) {
- if (ts->pendown) {
- struct input_dev *input = ts->input;
-
- input_report_key(input, BTN_TOUCH, 0);
- input_report_abs(input, ABS_PRESSURE, 0);
- input_sync(input);
-
- ts->pendown = 0;
- dev_vdbg(&ts->spi->dev, "UP\n");
- }
-
- /* measurement cycle ended */
- if (!device_suspended(&ts->spi->dev)) {
- ts->irq_disabled = 0;
- enable_irq(ts->spi->irq);
- }
- ts->pending = 0;
- } else {
- /* pen is still down, continue with the measurement */
- ts->msg_idx = 0;
- ts->wait_for_sync();
- status = spi_async(ts->spi, &ts->msg[0]);
- if (status)
- dev_err(&ts->spi->dev, "spi_async --> %d\n", status);
- }
-
- spin_unlock(&ts->lock);
- return HRTIMER_NORESTART;
-}
static irqreturn_t ads7846_irq(int irq, void *handle)
{
struct ads7846 *ts = handle;
- unsigned long flags;
-
- spin_lock_irqsave(&ts->lock, flags);
- if (likely(get_pendown_state(ts))) {
- if (!ts->irq_disabled) {
- /* The ARM do_simple_IRQ() dispatcher doesn't act
- * like the other dispatchers: it will report IRQs
- * even after they've been disabled. We work around
- * that here. (The "generic irq" framework may help...)
- */
- ts->irq_disabled = 1;
- disable_irq_nosync(ts->spi->irq);
- ts->pending = 1;
- hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_DELAY),
- HRTIMER_MODE_REL);
- }
- }
- spin_unlock_irqrestore(&ts->lock, flags);
- return IRQ_HANDLED;
-}
+ /* Start with a small delay before checking pendown state */
+ msleep(TS_POLL_DELAY);
-/*--------------------------------------------------------------------------*/
+ while (!ts->stopped && get_pendown_state(ts)) {
-/* Must be called with ts->lock held */
-static void ads7846_disable(struct ads7846 *ts)
-{
- if (ts->disabled)
- return;
+ /* pen is down, continue with the measurement */
+ ads7846_read_state(ts);
- ts->disabled = 1;
+ if (!ts->stopped)
+ ads7846_report_state(ts);
- /* are we waiting for IRQ, or polling? */
- if (!ts->pending) {
- ts->irq_disabled = 1;
- disable_irq(ts->spi->irq);
- } else {
- /* the timer will run at least once more, and
- * leave everything in a clean state, IRQ disabled
- */
- while (ts->pending) {
- spin_unlock_irq(&ts->lock);
- msleep(1);
- spin_lock_irq(&ts->lock);
- }
+ wait_event_timeout(ts->wait, ts->stopped,
+ msecs_to_jiffies(TS_POLL_PERIOD));
}
- regulator_disable(ts->reg);
-
- /* we know the chip's in lowpower mode since we always
- * leave it that way after every request
- */
-}
+ if (ts->pendown) {
+ struct input_dev *input = ts->input;
-/* Must be called with ts->lock held */
-static void ads7846_enable(struct ads7846 *ts)
-{
- if (!ts->disabled)
- return;
+ input_report_key(input, BTN_TOUCH, 0);
+ input_report_abs(input, ABS_PRESSURE, 0);
+ input_sync(input);
- regulator_enable(ts->reg);
+ ts->pendown = false;
+ dev_vdbg(&ts->spi->dev, "UP\n");
+ }
- ts->disabled = 0;
- ts->irq_disabled = 0;
- enable_irq(ts->spi->irq);
+ return IRQ_HANDLED;
}
static int ads7846_suspend(struct spi_device *spi, pm_message_t message)
{
struct ads7846 *ts = dev_get_drvdata(&spi->dev);
- spin_lock_irq(&ts->lock);
+ mutex_lock(&ts->lock);
- ts->is_suspended = 1;
- ads7846_disable(ts);
+ if (!ts->suspended) {
- spin_unlock_irq(&ts->lock);
+ if (!ts->disabled)
+ __ads7846_disable(ts);
- if (device_may_wakeup(&ts->spi->dev))
- enable_irq_wake(ts->spi->irq);
+ if (device_may_wakeup(&ts->spi->dev))
+ enable_irq_wake(ts->spi->irq);
- return 0;
+ ts->suspended = true;
+ }
+
+ mutex_unlock(&ts->lock);
+ return 0;
}
static int ads7846_resume(struct spi_device *spi)
{
struct ads7846 *ts = dev_get_drvdata(&spi->dev);
- if (device_may_wakeup(&ts->spi->dev))
- disable_irq_wake(ts->spi->irq);
+ mutex_lock(&ts->lock);
- spin_lock_irq(&ts->lock);
+ if (ts->suspended) {
- ts->is_suspended = 0;
- ads7846_enable(ts);
+ if (device_may_wakeup(&ts->spi->dev))
+ disable_irq_wake(ts->spi->irq);
- spin_unlock_irq(&ts->lock);
+ if (!ts->disabled)
+ __ads7846_enable(ts);
+
+ ts->suspended = false;
+ }
+
+ mutex_unlock(&ts->lock);
return 0;
}
-static int __devinit setup_pendown(struct spi_device *spi, struct ads7846 *ts)
+static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads7846 *ts)
{
struct ads7846_platform_data *pdata = spi->dev.platform_data;
int err;
@@ -932,146 +948,40 @@ static int __devinit setup_pendown(struct spi_device *spi, struct ads7846 *ts)
err = gpio_request(pdata->gpio_pendown, "ads7846_pendown");
if (err) {
dev_err(&spi->dev, "failed to request pendown GPIO%d\n",
- pdata->gpio_pendown);
+ pdata->gpio_pendown);
return err;
}
ts->gpio_pendown = pdata->gpio_pendown;
+
return 0;
}
-static int __devinit ads7846_probe(struct spi_device *spi)
+/*
+ * Set up the transfers to read touchscreen state; this assumes we
+ * use formula #2 for pressure, not #3.
+ */
+static void __devinit ads7846_setup_spi_msg(struct ads7846 *ts,
+ const struct ads7846_platform_data *pdata)
{
- struct ads7846 *ts;
- struct ads7846_packet *packet;
- struct input_dev *input_dev;
- const struct ads7846_platform_data *pdata = spi->dev.platform_data;
- struct spi_message *m;
- struct spi_transfer *x;
- unsigned long irq_flags;
- int vref;
- int err;
-
- if (!spi->irq) {
- dev_dbg(&spi->dev, "no IRQ?\n");
- return -ENODEV;
- }
-
- if (!pdata) {
- dev_dbg(&spi->dev, "no platform data?\n");
- return -ENODEV;
- }
-
- /* don't exceed max specified sample rate */
- if (spi->max_speed_hz > (125000 * SAMPLE_BITS)) {
- dev_dbg(&spi->dev, "f(sample) %d KHz?\n",
- (spi->max_speed_hz/SAMPLE_BITS)/1000);
- return -EINVAL;
- }
-
- /* We'd set TX wordsize 8 bits and RX wordsize to 13 bits ... except
- * that even if the hardware can do that, the SPI controller driver
- * may not. So we stick to very-portable 8 bit words, both RX and TX.
- */
- spi->bits_per_word = 8;
- spi->mode = SPI_MODE_0;
- err = spi_setup(spi);
- if (err < 0)
- return err;
-
- ts = kzalloc(sizeof(struct ads7846), GFP_KERNEL);
- packet = kzalloc(sizeof(struct ads7846_packet), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!ts || !packet || !input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
-
- dev_set_drvdata(&spi->dev, ts);
-
- ts->packet = packet;
- ts->spi = spi;
- ts->input = input_dev;
- ts->vref_mv = pdata->vref_mv;
- ts->swap_xy = pdata->swap_xy;
-
- hrtimer_init(&ts->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- ts->timer.function = ads7846_timer;
-
- spin_lock_init(&ts->lock);
-
- ts->model = pdata->model ? : 7846;
- ts->vref_delay_usecs = pdata->vref_delay_usecs ? : 100;
- ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
- ts->pressure_max = pdata->pressure_max ? : ~0;
-
- if (pdata->filter != NULL) {
- if (pdata->filter_init != NULL) {
- err = pdata->filter_init(pdata, &ts->filter_data);
- if (err < 0)
- goto err_free_mem;
- }
- ts->filter = pdata->filter;
- ts->filter_cleanup = pdata->filter_cleanup;
- } else if (pdata->debounce_max) {
- ts->debounce_max = pdata->debounce_max;
- if (ts->debounce_max < 2)
- ts->debounce_max = 2;
- ts->debounce_tol = pdata->debounce_tol;
- ts->debounce_rep = pdata->debounce_rep;
- ts->filter = ads7846_debounce;
- ts->filter_data = ts;
- } else
- ts->filter = ads7846_no_filter;
-
- err = setup_pendown(spi, ts);
- if (err)
- goto err_cleanup_filter;
-
- if (pdata->penirq_recheck_delay_usecs)
- ts->penirq_recheck_delay_usecs =
- pdata->penirq_recheck_delay_usecs;
-
- ts->wait_for_sync = pdata->wait_for_sync ? : null_wait_for_sync;
-
- snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&spi->dev));
- snprintf(ts->name, sizeof(ts->name), "ADS%d Touchscreen", ts->model);
-
- input_dev->name = ts->name;
- input_dev->phys = ts->phys;
- input_dev->dev.parent = &spi->dev;
-
- input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
- input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
- input_set_abs_params(input_dev, ABS_X,
- pdata->x_min ? : 0,
- pdata->x_max ? : MAX_12BIT,
- 0, 0);
- input_set_abs_params(input_dev, ABS_Y,
- pdata->y_min ? : 0,
- pdata->y_max ? : MAX_12BIT,
- 0, 0);
- input_set_abs_params(input_dev, ABS_PRESSURE,
- pdata->pressure_min, pdata->pressure_max, 0, 0);
-
- vref = pdata->keep_vref_on;
+ struct spi_message *m = &ts->msg[0];
+ struct spi_transfer *x = ts->xfer;
+ struct ads7846_packet *packet = ts->packet;
+ int vref = pdata->keep_vref_on;
if (ts->model == 7873) {
- /* The AD7873 is almost identical to the ADS7846
+ /*
+ * The AD7873 is almost identical to the ADS7846
* keep VREF off during differential/ratiometric
- * conversion modes
+ * conversion modes.
*/
ts->model = 7846;
vref = 0;
}
- /* set up the transfers to read touchscreen state; this assumes we
- * use formula #2 for pressure, not #3.
- */
- m = &ts->msg[0];
- x = ts->xfer;
-
+ ts->msg_count = 1;
spi_message_init(m);
+ m->context = ts;
if (ts->model == 7845) {
packet->read_y_cmd[0] = READ_Y(vref);
@@ -1094,7 +1004,8 @@ static int __devinit ads7846_probe(struct spi_device *spi)
spi_message_add_tail(x, m);
}
- /* the first sample after switching drivers can be low quality;
+ /*
+ * The first sample after switching drivers can be low quality;
* optionally discard it, using a second one after the signals
* have had enough time to stabilize.
*/
@@ -1112,11 +1023,10 @@ static int __devinit ads7846_probe(struct spi_device *spi)
spi_message_add_tail(x, m);
}
- m->complete = ads7846_rx_val;
- m->context = ts;
-
+ ts->msg_count++;
m++;
spi_message_init(m);
+ m->context = ts;
if (ts->model == 7845) {
x++;
@@ -1156,13 +1066,12 @@ static int __devinit ads7846_probe(struct spi_device *spi)
spi_message_add_tail(x, m);
}
- m->complete = ads7846_rx_val;
- m->context = ts;
-
/* turn y+ off, x- on; we'll use formula #2 */
if (ts->model == 7846) {
+ ts->msg_count++;
m++;
spi_message_init(m);
+ m->context = ts;
x++;
packet->read_z1 = READ_Z1(vref);
@@ -1190,11 +1099,10 @@ static int __devinit ads7846_probe(struct spi_device *spi)
spi_message_add_tail(x, m);
}
- m->complete = ads7846_rx_val;
- m->context = ts;
-
+ ts->msg_count++;
m++;
spi_message_init(m);
+ m->context = ts;
x++;
packet->read_z2 = READ_Z2(vref);
@@ -1221,14 +1129,13 @@ static int __devinit ads7846_probe(struct spi_device *spi)
x->len = 2;
spi_message_add_tail(x, m);
}
-
- m->complete = ads7846_rx_val;
- m->context = ts;
}
/* power down */
+ ts->msg_count++;
m++;
spi_message_init(m);
+ m->context = ts;
if (ts->model == 7845) {
x++;
@@ -1251,11 +1158,119 @@ static int __devinit ads7846_probe(struct spi_device *spi)
CS_CHANGE(*x);
spi_message_add_tail(x, m);
+}
- m->complete = ads7846_rx;
- m->context = ts;
+static int __devinit ads7846_probe(struct spi_device *spi)
+{
+ struct ads7846 *ts;
+ struct ads7846_packet *packet;
+ struct input_dev *input_dev;
+ struct ads7846_platform_data *pdata = spi->dev.platform_data;
+ unsigned long irq_flags;
+ int err;
- ts->last_msg = m;
+ if (!spi->irq) {
+ dev_dbg(&spi->dev, "no IRQ?\n");
+ return -ENODEV;
+ }
+
+ if (!pdata) {
+ dev_dbg(&spi->dev, "no platform data?\n");
+ return -ENODEV;
+ }
+
+ /* don't exceed max specified sample rate */
+ if (spi->max_speed_hz > (125000 * SAMPLE_BITS)) {
+ dev_dbg(&spi->dev, "f(sample) %d KHz?\n",
+ (spi->max_speed_hz/SAMPLE_BITS)/1000);
+ return -EINVAL;
+ }
+
+ /* We'd set TX word size 8 bits and RX word size to 13 bits ... except
+ * that even if the hardware can do that, the SPI controller driver
+ * may not. So we stick to very-portable 8 bit words, both RX and TX.
+ */
+ spi->bits_per_word = 8;
+ spi->mode = SPI_MODE_0;
+ err = spi_setup(spi);
+ if (err < 0)
+ return err;
+
+ ts = kzalloc(sizeof(struct ads7846), GFP_KERNEL);
+ packet = kzalloc(sizeof(struct ads7846_packet), GFP_KERNEL);
+ input_dev = input_allocate_device();
+ if (!ts || !packet || !input_dev) {
+ err = -ENOMEM;
+ goto err_free_mem;
+ }
+
+ dev_set_drvdata(&spi->dev, ts);
+
+ ts->packet = packet;
+ ts->spi = spi;
+ ts->input = input_dev;
+ ts->vref_mv = pdata->vref_mv;
+ ts->swap_xy = pdata->swap_xy;
+
+ mutex_init(&ts->lock);
+ init_waitqueue_head(&ts->wait);
+
+ ts->model = pdata->model ? : 7846;
+ ts->vref_delay_usecs = pdata->vref_delay_usecs ? : 100;
+ ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
+ ts->pressure_max = pdata->pressure_max ? : ~0;
+
+ if (pdata->filter != NULL) {
+ if (pdata->filter_init != NULL) {
+ err = pdata->filter_init(pdata, &ts->filter_data);
+ if (err < 0)
+ goto err_free_mem;
+ }
+ ts->filter = pdata->filter;
+ ts->filter_cleanup = pdata->filter_cleanup;
+ } else if (pdata->debounce_max) {
+ ts->debounce_max = pdata->debounce_max;
+ if (ts->debounce_max < 2)
+ ts->debounce_max = 2;
+ ts->debounce_tol = pdata->debounce_tol;
+ ts->debounce_rep = pdata->debounce_rep;
+ ts->filter = ads7846_debounce_filter;
+ ts->filter_data = ts;
+ } else {
+ ts->filter = ads7846_no_filter;
+ }
+
+ err = ads7846_setup_pendown(spi, ts);
+ if (err)
+ goto err_cleanup_filter;
+
+ if (pdata->penirq_recheck_delay_usecs)
+ ts->penirq_recheck_delay_usecs =
+ pdata->penirq_recheck_delay_usecs;
+
+ ts->wait_for_sync = pdata->wait_for_sync ? : null_wait_for_sync;
+
+ snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&spi->dev));
+ snprintf(ts->name, sizeof(ts->name), "ADS%d Touchscreen", ts->model);
+
+ input_dev->name = ts->name;
+ input_dev->phys = ts->phys;
+ input_dev->dev.parent = &spi->dev;
+
+ input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+ input_set_abs_params(input_dev, ABS_X,
+ pdata->x_min ? : 0,
+ pdata->x_max ? : MAX_12BIT,
+ 0, 0);
+ input_set_abs_params(input_dev, ABS_Y,
+ pdata->y_min ? : 0,
+ pdata->y_max ? : MAX_12BIT,
+ 0, 0);
+ input_set_abs_params(input_dev, ABS_PRESSURE,
+ pdata->pressure_min, pdata->pressure_max, 0, 0);
+
+ ads7846_setup_spi_msg(ts, pdata);
ts->reg = regulator_get(&spi->dev, "vcc");
if (IS_ERR(ts->reg)) {
@@ -1271,16 +1286,17 @@ static int __devinit ads7846_probe(struct spi_device *spi)
}
irq_flags = pdata->irq_flags ? : IRQF_TRIGGER_FALLING;
+ irq_flags |= IRQF_ONESHOT;
- err = request_irq(spi->irq, ads7846_irq, irq_flags,
- spi->dev.driver->name, ts);
-
+ err = request_threaded_irq(spi->irq, ads7846_hard_irq, ads7846_irq,
+ irq_flags, spi->dev.driver->name, ts);
if (err && !pdata->irq_flags) {
dev_info(&spi->dev,
"trying pin change workaround on irq %d\n", spi->irq);
- err = request_irq(spi->irq, ads7846_irq,
- IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
- spi->dev.driver->name, ts);
+ irq_flags |= IRQF_TRIGGER_RISING;
+ err = request_threaded_irq(spi->irq,
+ ads7846_hard_irq, ads7846_irq,
+ irq_flags, spi->dev.driver->name, ts);
}
if (err) {
@@ -1294,7 +1310,8 @@ static int __devinit ads7846_probe(struct spi_device *spi)
dev_info(&spi->dev, "touchscreen, irq %d\n", spi->irq);
- /* take a first sample, leaving nPENIRQ active and vREF off; avoid
+ /*
+ * Take a first sample, leaving nPENIRQ active and vREF off; avoid
* the touchscreen, in case it's not connected.
*/
if (ts->model == 7845)
@@ -1340,20 +1357,18 @@ static int __devinit ads7846_probe(struct spi_device *spi)
static int __devexit ads7846_remove(struct spi_device *spi)
{
- struct ads7846 *ts = dev_get_drvdata(&spi->dev);
+ struct ads7846 *ts = dev_get_drvdata(&spi->dev);
device_init_wakeup(&spi->dev, false);
- ads784x_hwmon_unregister(spi, ts);
- input_unregister_device(ts->input);
-
- ads7846_suspend(spi, PMSG_SUSPEND);
-
sysfs_remove_group(&spi->dev.kobj, &ads784x_attr_group);
+ ads7846_disable(ts);
free_irq(ts->spi->irq, ts);
- /* suspend left the IRQ disabled */
- enable_irq(ts->spi->irq);
+
+ input_unregister_device(ts->input);
+
+ ads784x_hwmon_unregister(spi, ts);
regulator_disable(ts->reg);
regulator_put(ts->reg);
@@ -1368,6 +1383,7 @@ static int __devexit ads7846_remove(struct spi_device *spi)
kfree(ts);
dev_dbg(&spi->dev, "unregistered touchscreen\n");
+
return 0;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread