On Tue, 12 Mar 2019 09:21:45 +0100
David Jander <david@xxxxxxxxxxx> wrote:
> The ads7846.c touchscreen driver has several issues that stem from bad
> overall design of the sampling- and filtering routines. This patch can
> be seen as a nearly complete re-write of the driver, trying to fix the
> various issues. Most importantly, a near 100 times reduction of CPU usage
> caused by excessive context switching due to sending many different small
> SPI transfers, instead of one single (bigger) transfer to acquire all
> measurement data. The new filtering code is a bit better, less buggy
> and hopefully a lot easier to read and maintain. A potential integer-
> overflow that could cause fake touches has been fixed as well as an issue
> that caused the driver to report a pen-release when HWMON measurements are
> taken while the screen is being touched.
> Also, the unnecessary code duplication for different chip types has been
> reduced.
> This version of the driver makes use of the 16-clock-per-conversion mode
> as described in the data-sheets. It was tested with a tsc2046 but should
> work also with all other variants.
>
> Signed-off-by: David Jander <david@xxxxxxxxxxx>
> ---
> .../bindings/input/touchscreen/ads7846.txt | 32 +-
> drivers/input/touchscreen/ads7846.c | 862 +++++++-----------
> include/linux/spi/ads7846.h | 27 +-
> 3 files changed, 370 insertions(+), 551 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt b/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
> index 946f4fd0133e..ebc3614609d9 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
> @@ -35,18 +35,6 @@ Optional properties:
> ti,swap-xy swap x and y axis
> ti,invert_x invert x axis
> ti,invert_y invert y axis
> - ti,settle-delay-usec Settling time of the analog signals;
> - a function of Vcc and the capacitance
> - on the X/Y drivers. If set to non-zero,
> - two samples are taken with settle_delay
> - us apart, and the second one is used.
> - ~150 uSec with 0.01uF caps (u16).
> - ti,penirq-recheck-delay-usecs If set to non-zero, after samples are
> - taken this delay is applied and penirq
> - is rechecked, to help avoid false
> - events. This value is affected by the
> - material used to build the touch layer
> - (u16).
> ti,x-plate-ohms Resistance of the X-plate,
> in Ohms (u16).
> ti,y-plate-ohms Resistance of the Y-plate,
> @@ -58,13 +46,23 @@ Optional properties:
> ti,pressure-min Minimum reported pressure value
> (threshold) - u16.
> ti,pressure-max Maximum reported pressure value (u16).
> - ti,debounce-max Max number of additional readings per
> - sample (u16).
> - ti,debounce-tol Tolerance used for filtering (u16).
> - ti,debounce-rep Additional consecutive good readings
> - required after the first two (u16).
> + ti,skip-samples Skip n samples after switching plates.
> + Maximum of 4. The delay this introduces
> + depends on the SPI clock frequency.
> + ti,touch-resistance-threshold Threshold of touchresistance (in Ohms)
> + for detecting a valid touch.
> + ti,filter-tolerance Maximum X/Y measurement deviation (raw)
> + of consecutive measurements.
> + ti,sample-period-msecs Touchscreen sample period in ms.
> + ti,report-period-msecs Touch report generation period in ms.
> ti,pendown-gpio-debounce Platform specific debounce time for the
> pendown-gpio (u32).
> + ti,pendown-glitch-max Maximum sample times of pendown signal
> + glitch. If sliding over a worn touch-
> + panel, touch-contact can be interrupted
> + briefly. Avoid detecting a pen release
> + signal too early by waiting this amount
> + of samples.
> pendown-gpio GPIO handle describing the pin the !PENIRQ
> line is connected to.
> wakeup-source use any event on touchscreen as wakeup event.
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index b853e889c6ec..27b09d5d7476 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -1,6 +1,9 @@
> /*
> * ADS7846 based touchscreen and sensor driver
> *
> + * Re-write sampling and filtering code:
> + * Copyright (c) 2019 David Jander <david@xxxxxxxxxxx>
> + *
> * Copyright (c) 2005 David Brownell
> * Copyright (c) 2006 Nokia Corporation
> * Various changes: Imre Deak <imre.deak@xxxxxxxxx>
> @@ -59,37 +62,21 @@
> */
>
> #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 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;
> - bool ignore;
> - u8 x_buf[3];
> - u8 y_buf[3];
> +struct ads7846_vfilter_buf {
> + bool susp;
> + int x;
> + int y;
> + int rt;
> };
>
> -/*
> - * We allocate this separately to avoid cache line sharing issues when
> - * driver is used with DMA-based SPI controllers (like atmel_spi) on
> - * systems where main memory is not DMA-coherent (most non-x86 boards).
> - */
> -struct ads7846_packet {
> - u8 read_x, read_y, read_z1, read_z2, pwrdown;
> - u16 dummy; /* for the pwrdown read */
> - struct ts_event tc;
> - /* for ads7845 with mpc5121 psc spi we use 3-byte buffers */
> - u8 read_x_cmd[3], read_y_cmd[3], pwrdown_cmd[3];
> +struct ads7846_vfilter {
> + int bidx;
> + int bcnt;
> + struct ads7846_vfilter_buf fbuf[4];
> };
>
> struct ads7846 {
> @@ -115,24 +102,34 @@ struct ads7846 {
> bool invert_y;
> bool use_internal;
>
> - struct ads7846_packet *packet;
> + struct ads7846_vfilter vfilter;
>
> - struct spi_transfer xfer[18];
> - struct spi_message msg[5];
> - int msg_count;
> - wait_queue_head_t wait;
> +#define MAX_SKIP_SAMPLES 4
> +/* 4 measurements, 16 bits each, skipped samples + 2, 3 byte powerdown command */
> +#define MAX_XFER_LEN ((4 * 2 * (MAX_SKIP_SAMPLES + 2)) + 3)
>
> - bool pendown;
> + unsigned int skip_samples;
> + unsigned int filter_tolerance;
> + unsigned int rt_threshold;
> +
> + unsigned int sample_period;
> + unsigned int report_period;
> +
> + struct ads7846_vfilter_buf touch;
> + bool touch_valid;
> +
> + /* DMA-safe buffers for SPI transfers */
> + u8 rx_buf[MAX_XFER_LEN] ____cacheline_aligned;
> + u8 tx_buf[MAX_XFER_LEN] ____cacheline_aligned;
>
> - int read_cnt;
> - int read_rep;
> - int last_read;
> + struct spi_transfer xfer;
> + struct spi_message msg;
>
> - u16 debounce_max;
> - u16 debounce_tol;
> - u16 debounce_rep;
> + wait_queue_head_t wait;
>
> - u16 penirq_recheck_delay_usecs;
> + bool pendown;
> + int penup_count;
> + unsigned int pendown_glitch_max;
>
> struct mutex lock;
> bool stopped; /* P: lock */
> @@ -148,12 +145,7 @@ struct ads7846 {
> void (*wait_for_sync)(void);
> };
>
> -/* leave chip selected when we're done, for quicker re-select? */
> -#if 0
> #define CS_CHANGE(xfer) ((xfer).cs_change = 1)
> -#else
> -#define CS_CHANGE(xfer) ((xfer).cs_change = 0)
> -#endif
>
> /*--------------------------------------------------------------------------*/
>
> @@ -287,28 +279,14 @@ static void ads7846_enable(struct ads7846 *ts)
> */
>
> struct ser_req {
> - u8 ref_on;
> - u8 command;
> - u8 ref_off;
> - u16 scratch;
> - struct spi_message msg;
> - struct spi_transfer xfer[6];
> - /*
> - * DMA (thus cache coherency maintenance) requires the
> - * transfer buffers to live in their own cache lines.
> - */
> - __be16 sample ____cacheline_aligned;
> -};
> -
> -struct ads7845_ser_req {
> - u8 command[3];
> struct spi_message msg;
> struct spi_transfer xfer[2];
> /*
> * DMA (thus cache coherency maintenance) requires the
> * transfer buffers to live in their own cache lines.
> */
> - u8 sample[3] ____cacheline_aligned;
> + u8 tx_buf[8] ____cacheline_aligned;
> + u8 rx_buf[5] ____cacheline_aligned;
> };
>
> static int ads7846_read12_ser(struct device *dev, unsigned command)
> @@ -317,26 +295,29 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
> struct ads7846 *ts = dev_get_drvdata(dev);
> struct ser_req *req;
> int status;
> + int i = 0;
> + u8 *txb;
>
> req = kzalloc(sizeof *req, GFP_KERNEL);
> if (!req)
> return -ENOMEM;
>
> spi_message_init(&req->msg);
> + txb = &req->tx_buf[0];
>
> - /* maybe turn on internal vREF, and let it settle */
> + /*
> + * maybe turn on internal vREF, and let it settle
> + * note: use_internal can only be true on ads7846
> + */
> if (ts->use_internal) {
> - req->ref_on = REF_ON;
> - req->xfer[0].tx_buf = &req->ref_on;
> - req->xfer[0].len = 1;
> - spi_message_add_tail(&req->xfer[0], &req->msg);
> -
> - req->xfer[1].rx_buf = &req->scratch;
> - req->xfer[1].len = 2;
> + txb[i] = REF_ON;
> + req->xfer[0].tx_buf = &txb[i];
> + req->xfer[0].len = 3;
> + i += 3;
>
> /* for 1uF, settle for 800 usec; no cap, 100 usec. */
> - req->xfer[1].delay_usecs = ts->vref_delay_usecs;
> - spi_message_add_tail(&req->xfer[1], &req->msg);
> + req->xfer[0].delay_usecs = ts->vref_delay_usecs;
> + spi_message_add_tail(&req->xfer[0], &req->msg);
>
> /* Enable reference voltage */
> command |= ADS_PD10_REF_ON;
> @@ -345,76 +326,23 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
> /* Enable ADC in every case */
> command |= ADS_PD10_ADC_ON;
>
> - /* take sample */
> - req->command = (u8) command;
> - req->xfer[2].tx_buf = &req->command;
> - req->xfer[2].len = 1;
> - spi_message_add_tail(&req->xfer[2], &req->msg);
> -
> - req->xfer[3].rx_buf = &req->sample;
> - req->xfer[3].len = 2;
> - spi_message_add_tail(&req->xfer[3], &req->msg);
> -
> - /* REVISIT: take a few more samples, and compare ... */
> -
> - /* converter in low power mode & enable PENIRQ */
> - req->ref_off = PWRDOWN;
> - req->xfer[4].tx_buf = &req->ref_off;
> - req->xfer[4].len = 1;
> - spi_message_add_tail(&req->xfer[4], &req->msg);
> -
> - req->xfer[5].rx_buf = &req->scratch;
> - req->xfer[5].len = 2;
> - CS_CHANGE(req->xfer[5]);
> - spi_message_add_tail(&req->xfer[5], &req->msg);
> + req->xfer[1].tx_buf = &txb[i];
> + txb[i] = command;
> + i += 2;
> + txb[i] = PWRDOWN;
> + i += 3;
> + req->xfer[1].rx_buf = &req->rx_buf[0];
> + req->xfer[1].len = 5;
> + CS_CHANGE(req->xfer[1]);
> + spi_message_add_tail(&req->xfer[1], &req->msg);
>
> mutex_lock(&ts->lock);
> - ads7846_stop(ts);
> status = spi_sync(spi, &req->msg);
> - ads7846_restart(ts);
> mutex_unlock(&ts->lock);
>
> if (status == 0) {
> /* on-wire is a must-ignore bit, a BE12 value, then padding */
> - status = be16_to_cpu(req->sample);
> - status = status >> 3;
> - status &= 0x0fff;
> - }
> -
> - kfree(req);
> - return status;
> -}
> -
> -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;
> - int status;
> -
> - req = kzalloc(sizeof *req, GFP_KERNEL);
> - if (!req)
> - return -ENOMEM;
> -
> - spi_message_init(&req->msg);
> -
> - req->command[0] = (u8) command;
> - req->xfer[0].tx_buf = req->command;
> - req->xfer[0].rx_buf = req->sample;
> - req->xfer[0].len = 3;
> - spi_message_add_tail(&req->xfer[0], &req->msg);
> -
> - mutex_lock(&ts->lock);
> - ads7846_stop(ts);
> - status = spi_sync(spi, &req->msg);
> - ads7846_restart(ts);
> - mutex_unlock(&ts->lock);
> -
> - if (status == 0) {
> - /* BE12 value, then padding */
> - status = be16_to_cpu(*((u16 *)&req->sample[1]));
> - status = status >> 3;
> - status &= 0x0fff;
> + status = (be16_to_cpup((__be16 *)&req->rx_buf[1]) >> 3) & 0x0fff;
> }
>
> kfree(req);
> @@ -619,201 +547,242 @@ static void null_wait_for_sync(void)
> {
> }
>
> -static int ads7846_debounce_filter(void *ads, int data_idx, int *val)
> +static void ads7846_vfilter_reset(struct ads7846 *ts)
> {
> - struct ads7846 *ts = ads;
> + struct ads7846_vfilter *vf = &ts->vfilter;
>
> - 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;
> - }
> - }
> + vf->bidx = 0;
> + vf->bcnt = 0;
> }
>
> -static int ads7846_no_filter(void *ads, int data_idx, int *val)
> +static inline int ads7846_calc_rt(struct ads7846 *ts, int x, int z1, int z2,
> + bool pd)
> {
> - return ADS7846_FILTER_OK;
> -}
> -
> -static int ads7846_get_value(struct ads7846 *ts, struct spi_message *m)
> -{
> - int value;
> - struct spi_transfer *t =
> - list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
> + int Rt;
> + bool validz = ((ts->model == 7846) || (ts->model == 7873));
>
> - if (ts->model == 7845) {
> - value = be16_to_cpup((__be16 *)&(((char *)t->rx_buf)[1]));
> - } else {
> + if (!validz) {
> + /* ads7843 and ads7845 cannot measure pressure, so we fake it */
> + if (pd)
> + Rt = ts->rt_threshold / 2;
> + else
> + Rt = ts->pressure_max;
> + } else if (likely(x && z1) && pd) {
> /*
> - * adjust: on-wire is a must-ignore bit, a BE12 value, then
> - * padding; built from two 8 bit values written msb-first.
> + * Compute touch pressure resistance using equation #2
> + * To avoid a potential integer overflow on spurious
> + * measurements causing a false high pressure touch, we do the
> + * final division by 4096 in two steps to scale-down
> + * intermediate results with minimal loss of precision.
> + * We multiply by "x_plate_resistance" first, which has a
> + * guaranteed lower bound higher than the first partial
> + * divisor (16), so no resolution of the (z2-z1) term is lost.
> */
> - value = be16_to_cpup((__be16 *)t->rx_buf);
> + Rt = z2;
> + Rt -= z1;
> + Rt *= ts->x_plate_ohms;
> + Rt = (Rt + 8) >> 4; /* Pre-shift to avoid integer overflow */
> + Rt *= x;
> + Rt /= z1;
> + Rt = (Rt + 128) >> 8; /* Rest of total 12 bit shift */
> + } else {
> + Rt = ts->pressure_max;
> }
> -
> - /* enforce ADC output is 12 bits width */
> - return (value >> 3) & 0xfff;
> -}
> -
> -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;
> + return Rt;
> }
>
> -static void ads7846_read_state(struct ads7846 *ts)
> +/*
> + * Touch sample filter
> + *
> + * This filter does several things:
> + * 1. Each sample is comprised of two measurements for X, Y and RT.
> + * Those pairs of samples are first analyzed to discriminate good from
> + * bad measurments. If they are both good, the average is taken for further
> + * processing. Bad samples are entirely discarded and TS goes back to
> + * detection in released state.
> + * 2. If the current sample is far away from the previous one, it is marked as
> + * suspicious and not taken into account in this report. The next sample is
> + * Then compared to both the suspicious sample and the one preceding it.
> + * If the new sample is closer to the older sample, the suspicious sample is
> + * discarded. Distance comparisions are done in manhattan distance for
> + * reasons of simplicity.
> + * 3. Up to 4 successive samples are held in a buffer.
> + * All samples in the buffer, except suspicious ones, are averaged and
> + * reported.
> + *
> + * Note that due to interaction between 2 and 3, even when a sample is marked
> + * as suspicious, a new report is generated, excluding this sample, but also
> + * excluding the oldest sample from the previous report, due to buffer size,
> + * so this report is not identical to the previous one! In case of a slide
> + * movement, it will be a little further into the sliding direction, which is
> + * a good thing.
> + */
> +static void ads7846_vfilter_process(struct ads7846 *ts, int x, int y, int rt)
> {
> - 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_sync --> %d\n", error);
> - packet->tc.ignore = true;
> - return;
> + struct ads7846_vfilter *vf = &ts->vfilter;
> + struct ads7846_vfilter_buf *this, *prev;
> + int dx, dy, dx2, dy2;
> + int t, i;
> + int ax, ay, ar;
> +
> + prev = &vf->fbuf[(vf->bidx - 1) & 3];
> + this = &vf->fbuf[vf->bidx];
> + dx = abs(prev->x - x);
> + dy = abs(prev->y - y);
> +
> + this->susp = false;
> + if (!prev->susp && vf->bcnt) {
> + if ((dx > ts->filter_tolerance) || (dy > ts->filter_tolerance)) {
> + this->susp = true;
> + dev_vdbg(&ts->spi->dev,
> + "marked suspect: dx: %d, dy: %d\n", dx, dy);
> }
> + } else if (prev->susp && (vf->bcnt > 1)) {
> + t = (vf->bidx - 2) & 3;
> + dx2 = abs(x - vf->fbuf[t].x);
> + dy2 = abs(y - vf->fbuf[t].y);
> + if ((dx2 + dy2) < (dx + dy)) {
> + /* Overwrite previous sample */
> + vf->bidx = (vf->bidx - 1) & 3;
> + this = &vf->fbuf[vf->bidx];
> + this->susp = false;
> + vf->bcnt--;
> + dev_vdbg(&ts->spi->dev,
> + "skipped over suspect: dx: %d, dy: %d, dx2: %d, dy2: %d\n",
> + dx, dy, dx2, dy2);
> + if ((dx2 > ts->filter_tolerance) ||
> + (dy2 > ts->filter_tolerance)) {
> + this->susp = true;
> + dev_vdbg(&ts->spi->dev,
> + "marked suspect again: dx: %d, dy: %d\n",
> + dx2, dy2);
> + }
> + }
> + }
> + this->x = x;
> + this->y = y;
> + this->rt = rt;
>
> - /*
> - * 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;
> - msg_idx = ts->msg_count - 1;
> - continue;
> + vf->bidx = (vf->bidx + 1) & 3;
> + if (vf->bcnt < 4)
> + vf->bcnt++;
>
> - case ADS7846_FILTER_OK:
> - ads7846_update_value(m, val);
> - packet->tc.ignore = false;
> - msg_idx++;
> - break;
> + if ((vf->bcnt - this->susp) < 2) {
> + dev_vdbg(&ts->spi->dev, "counting: %d\n", vf->bcnt);
> + return;
> + }
>
> - default:
> - BUG();
> - }
> - } else {
> - msg_idx++;
> - }
> + /* Average up to 4 most recent samples (3 if this is suspect) */
> + if (this->susp) {
> + ax = ay = ar = 0;
> + } else {
> + ax = x;
> + ay = y;
> + ar = rt;
> }
> + for (t = 0; t < (vf->bcnt - 1); t++) {
> + i = (vf->bidx - vf->bcnt + t) & 3;
> + ax += vf->fbuf[i].x;
> + ay += vf->fbuf[i].y;
> + ar += vf->fbuf[i].rt;
> + }
> + ax /= (vf->bcnt - this->susp);
> + ay /= (vf->bcnt - this->susp);
> + ar /= (vf->bcnt - this->susp);
> +
> + ts->touch.x = ax;
> + ts->touch.y = ay;
> + ts->touch.rt = ar;
> + ts->touch_valid = true;
> }
>
> -static void ads7846_report_state(struct ads7846 *ts)
> +static bool ads7846_read_state(struct ads7846 *ts)
> {
> - struct ads7846_packet *packet = ts->packet;
> - unsigned int Rt;
> - u16 x, y, z1, z2;
> + int error;
> + char *rxbuf = &ts->rx_buf[0];
> + int x0, x1, y0, y1, z10, z11, z20, z21;
> + int rt0, rt1;
> + int dx, dy;
> + int skp = ts->skip_samples;
> + int stride = (skp + 2) * 2; /* Total samples = skip_samples + 2 */
> + int offs;
> + bool pd;
> +
> + ts->wait_for_sync();
> +
> + mutex_lock(&ts->lock);
> + pd = get_pendown_state(ts);
> + error = spi_sync(ts->spi, &ts->msg);
> + mutex_unlock(&ts->lock);
> + if (error) {
> + dev_err(&ts->spi->dev, "spi_sync --> %d\n", error);
> + ts->touch_valid = false;
> + return false;
> + }
>
> /*
> - * ads7846_get_value() does in-place conversion (including byte swap)
> - * from on-the-wire format as part of debouncing to get stable
> - * readings.
> + * Extract samples from RX buffer.
> + * RX data starts with a dummy byte (0) and then contains the sample
> + * data in BE16 ordering (every 2 bytes, one sample).
> + * First all Y-samples (2 + skip_samples total), then the same amount
> + * of X-samples, then same amount of Z1 and finally Z2.
> */
> - if (ts->model == 7845) {
> - x = *(u16 *)packet->tc.x_buf;
> - y = *(u16 *)packet->tc.y_buf;
> - z1 = 0;
> - z2 = 0;
> + offs = skp * 2 + 1;
> + y0 = (be16_to_cpup((__be16 *)&rxbuf[offs]) >> 3) & 0x0fff;
> + x0 = (be16_to_cpup((__be16 *)&rxbuf[offs + stride]) >> 3) & 0x0fff;
> + z10 = (be16_to_cpup((__be16 *)&rxbuf[offs + stride * 2]) >> 3) & 0x0fff;
> + z20 = (be16_to_cpup((__be16 *)&rxbuf[offs + stride * 3]) >> 3) & 0x0fff;
> +
> + offs = skp * 2 + 3;
> + y1 = (be16_to_cpup((__be16 *)&rxbuf[offs]) >> 3) & 0x0fff;
> + x1 = (be16_to_cpup((__be16 *)&rxbuf[offs + stride]) >> 3) & 0x0fff;
> + z11 = (be16_to_cpup((__be16 *)&rxbuf[offs + stride * 2]) >> 3) & 0x0fff;
> + z21 = (be16_to_cpup((__be16 *)&rxbuf[offs + stride * 3]) >> 3) & 0x0fff;
> +
> + dx = abs(x0 - x1);
> + dy = abs(y0 - y1);
> + rt0 = ads7846_calc_rt(ts, x0, z10, z20, pd);
> + rt1 = ads7846_calc_rt(ts, x1, z11, z21, pd);
> +
> + dev_vdbg(&ts->spi->dev,
> + "x0: %d, x1: %d, y0: %d, y1: %d, z1: %d, z2: %d, rt0: %d, rt2: %d pd: %d\n",
> + x0, x1, y0, y1, z11, z21, rt0, rt1, pd);
> + if ((rt0 < ts->rt_threshold) && (rt1 < ts->rt_threshold) &&
> + (dx < ts->filter_tolerance) &&
> + (dy < ts->filter_tolerance)) {
> + ads7846_vfilter_process(ts, (x0 + x1) / 2, (y0 + y1) / 2,
> + (rt0 + rt1) / 2);
> } else {
> - x = packet->tc.x;
> - y = packet->tc.y;
> - z1 = packet->tc.z1;
> - z2 = packet->tc.z2;
> + dev_vdbg(&ts->spi->dev, "Sample not good: dx: %d, dy: %d\n",
> + dx, dy);
> }
>
> - /* range filtering */
> - if (x == MAX_12BIT)
> - x = 0;
> -
> - if (ts->model == 7843) {
> - Rt = ts->pressure_max / 2;
> - } else if (ts->model == 7845) {
> - if (get_pendown_state(ts))
> - Rt = ts->pressure_max / 2;
> - else
> - Rt = 0;
> - dev_vdbg(&ts->spi->dev, "x/y: %d/%d, PD %d\n", x, y, Rt);
> - } else if (likely(x && z1)) {
> - /* compute touch pressure resistance using equation #2 */
> - Rt = z2;
> - Rt -= z1;
> - Rt *= x;
> - Rt *= ts->x_plate_ohms;
> - Rt /= z1;
> - Rt = (Rt + 2047) >> 12;
> + /* Pendown detection glitch filter */
> + if (pd) {
> + ts->penup_count = 0;
> } else {
> - Rt = 0;
> + ads7846_vfilter_reset(ts);
> + if (ts->penup_count++ < ts->pendown_glitch_max)
> + pd = true;
> }
> + return pd;
> +}
> +
> +static int ads7846_report_state(struct ads7846 *ts)
> +{
> + unsigned int Rt = ts->touch.rt;
> + unsigned int x = ts->touch.x;
> + unsigned int y = ts->touch.y;
>
> /*
> * 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);
> - return;
> - }
> -
> - /*
> - * Maybe check the pendown state before reporting. This discards
> - * false readings when the pen is lifted.
> - */
> - if (ts->penirq_recheck_delay_usecs) {
> - udelay(ts->penirq_recheck_delay_usecs);
> - if (!get_pendown_state(ts))
> - Rt = 0;
> + if (!ts->touch_valid || Rt > ts->pressure_max) {
> + dev_vdbg(&ts->spi->dev, "valid: %d pressure: %d\n",
> + ts->touch_valid, Rt);
> + return 0;
> }
>
> /*
> @@ -849,7 +818,10 @@ static void ads7846_report_state(struct ads7846 *ts)
>
> input_sync(input);
> dev_vdbg(&ts->spi->dev, "%4d/%4d/%4d\n", x, y, Rt);
> + return 1;
> }
> + ts->touch_valid = false; /* Mark as processed */
> + return 0;
> }
>
> static irqreturn_t ads7846_hard_irq(int irq, void *handle)
> @@ -863,21 +835,31 @@ static irqreturn_t ads7846_hard_irq(int irq, void *handle)
> static irqreturn_t ads7846_irq(int irq, void *handle)
> {
> struct ads7846 *ts = handle;
> + unsigned int rj = 0;
> + bool pd = true;
> + unsigned int rjperiod = msecs_to_jiffies(ts->report_period);
> + unsigned int sjperiod = msecs_to_jiffies(ts->sample_period);
Unfortunately this is wrong. Due to a last-minute cleanup fix this slipped
through untested, these 4 lines should read:
+ unsigned long rj = jiffies;
+ bool pd = true;
+ unsigned long rjperiod = msecs_to_jiffies(ts->report_period);
+ unsigned long sjperiod = msecs_to_jiffies(ts->sample_period);
Sorry for the mistake. I will await review and comments for the rest of the
patch before re-submitting this.
> /* Start with a small delay before checking pendown state */
> msleep(TS_POLL_DELAY);
>
> - while (!ts->stopped && get_pendown_state(ts)) {
> + ts->touch_valid = false;
> + while (!ts->stopped && pd) {
>
> /* pen is down, continue with the measurement */
> - ads7846_read_state(ts);
> + pd = ads7846_read_state(ts);
>
> - if (!ts->stopped)
> - ads7846_report_state(ts);
> + if ((!ts->stopped) && time_after(jiffies, rj)) {
And this should strictly be time_after_eq... *sigh*
> + if (ads7846_report_state(ts))
> + rj = jiffies + rjperiod;
> + }
>
> - wait_event_timeout(ts->wait, ts->stopped,
> - msecs_to_jiffies(TS_POLL_PERIOD));
> + if (ts->pendown)
> + wait_event_timeout(ts->wait, ts->stopped, sjperiod);
> + else
> + wait_event_timeout(ts->wait, ts->stopped, 1);
> }
> + ads7846_vfilter_reset(ts);
>
> if (ts->pendown && !ts->stopped) {
> struct input_dev *input = ts->input;
> @@ -978,16 +960,20 @@ static int ads7846_setup_pendown(struct spi_device *spi,
> }
>
> /*
> - * Set up the transfers to read touchscreen state; this assumes we
> - * use formula #2 for pressure, not #3.
> + * Setup the command buffer to read out all touchscreen measurements.
> + * All commands will be transmitted in one single SPI transfer in order to
> + * reduce scheduling overhead, making use of the 16 clocks-per-conversion
> + * timing mode of the ADS784x.
> */
> static void ads7846_setup_spi_msg(struct ads7846 *ts,
> const struct ads7846_platform_data *pdata)
> {
> - 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;
> + char *txbuf = &ts->tx_buf[0];
> + unsigned int skp = ts->skip_samples;
> + unsigned int stride = (skp + 2) * 2; /* total samples = skip + 2 */
> + unsigned int i;
> + unsigned int offs;
>
> if (ts->model == 7873) {
> /*
> @@ -999,185 +985,48 @@ static void ads7846_setup_spi_msg(struct ads7846 *ts,
> vref = 0;
> }
>
> - ts->msg_count = 1;
> - spi_message_init(m);
> - m->context = ts;
> -
> - if (ts->model == 7845) {
> - packet->read_y_cmd[0] = READ_Y(vref);
> - packet->read_y_cmd[1] = 0;
> - packet->read_y_cmd[2] = 0;
> - x->tx_buf = &packet->read_y_cmd[0];
> - x->rx_buf = &packet->tc.y_buf[0];
> - x->len = 3;
> - spi_message_add_tail(x, m);
> - } else {
> - /* y- still on; turn on only y+ (and ADC) */
> - packet->read_y = READ_Y(vref);
> - x->tx_buf = &packet->read_y;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.y;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> - }
> + memset(txbuf, 0, sizeof(ts->tx_buf));
>
> /*
> - * 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.
> + * Fill the TX buffer with command data. The ADS874x can process 1
> + * command every 2 bytes. Command bytes are filled starting at
> + * index 0 of the TX buffer.
> + * Every measurement command will be repeated (2 + skip_samples) times.
> + * First Y measurements, then X, Z1 and finally Z2.
> + * The skip_samples parameter is used to insert up to 2 dummy
> + * measurements before the 2 real measurements. The results of these
> + * dummy measurements are skipped. The idea is to give the signals time
> + * to settle after switching the plates.
> + * The last 2 consecutive measurements are always used for validation
> + * and filtering.
> */
> - if (pdata->settle_delay_usecs) {
> - x->delay_usecs = pdata->settle_delay_usecs;
> -
> - x++;
> - x->tx_buf = &packet->read_y;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.y;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> - }
> -
> - ts->msg_count++;
> - m++;
> - spi_message_init(m);
> - m->context = ts;
> -
> - if (ts->model == 7845) {
> - x++;
> - packet->read_x_cmd[0] = READ_X(vref);
> - packet->read_x_cmd[1] = 0;
> - packet->read_x_cmd[2] = 0;
> - x->tx_buf = &packet->read_x_cmd[0];
> - x->rx_buf = &packet->tc.x_buf[0];
> - x->len = 3;
> - spi_message_add_tail(x, m);
> - } else {
> - /* turn y- off, x+ on, then leave in lowpower */
> - x++;
> - packet->read_x = READ_X(vref);
> - x->tx_buf = &packet->read_x;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.x;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> - }
> -
> - /* ... maybe discard first sample ... */
> - if (pdata->settle_delay_usecs) {
> - x->delay_usecs = pdata->settle_delay_usecs;
> -
> - x++;
> - x->tx_buf = &packet->read_x;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.x;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> - }
> -
> - /* 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);
> - x->tx_buf = &packet->read_z1;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.z1;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> -
> - /* ... maybe discard first sample ... */
> - if (pdata->settle_delay_usecs) {
> - x->delay_usecs = pdata->settle_delay_usecs;
> -
> - x++;
> - x->tx_buf = &packet->read_z1;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.z1;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> - }
> -
> - ts->msg_count++;
> - m++;
> - spi_message_init(m);
> - m->context = ts;
> -
> - x++;
> - packet->read_z2 = READ_Z2(vref);
> - x->tx_buf = &packet->read_z2;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.z2;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> -
> - /* ... maybe discard first sample ... */
> - if (pdata->settle_delay_usecs) {
> - x->delay_usecs = pdata->settle_delay_usecs;
> -
> - x++;
> - x->tx_buf = &packet->read_z2;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->tc.z2;
> - x->len = 2;
> - spi_message_add_tail(x, m);
> + for (i = 0; i < (skp + 2); i++) {
> + offs = i * 2;
> + txbuf[offs] = READ_Y(vref);
> + txbuf[offs + stride] = READ_X(vref);
> + if (ts->model == 7846) {
> + txbuf[offs + stride * 2] = READ_Z1(vref);
> + txbuf[offs + stride * 3] = READ_Z2(vref);
> }
> }
> + if (ts->model == 7846)
> + txbuf[stride * 4] = PWRDOWN;
> + else
> + txbuf[stride * 2] = PWRDOWN;
>
> - /* power down */
> - ts->msg_count++;
> - m++;
> - spi_message_init(m);
> - m->context = ts;
> -
> - if (ts->model == 7845) {
> - x++;
> - packet->pwrdown_cmd[0] = PWRDOWN;
> - packet->pwrdown_cmd[1] = 0;
> - packet->pwrdown_cmd[2] = 0;
> - x->tx_buf = &packet->pwrdown_cmd[0];
> - x->len = 3;
> - } else {
> - x++;
> - packet->pwrdown = PWRDOWN;
> - x->tx_buf = &packet->pwrdown;
> - x->len = 1;
> - spi_message_add_tail(x, m);
> -
> - x++;
> - x->rx_buf = &packet->dummy;
> - x->len = 2;
> - }
> + spi_message_init(&ts->msg);
> + ts->msg.context = ts;
>
> - CS_CHANGE(*x);
> - spi_message_add_tail(x, m);
> + ts->xfer.rx_buf = &ts->rx_buf[0];
> + ts->xfer.tx_buf = txbuf;
> + if (ts->model == 7846)
> + ts->xfer.len = stride * 4 + 3;
> + else
> + ts->xfer.len = stride * 2 + 3;
> + ts->xfer.bits_per_word = 8;
> +
> + CS_CHANGE(ts->xfer);
> + spi_message_add_tail(&ts->xfer, &ts->msg);
> }
>
> #ifdef CONFIG_OF
> @@ -1223,11 +1072,6 @@ static const struct ads7846_platform_data *ads7846_probe_dt(struct device *dev)
> pdata->invert_x = of_property_read_bool(node, "ti,invert_x");
> pdata->invert_y = of_property_read_bool(node, "ti,invert_y");
>
> - of_property_read_u16(node, "ti,settle-delay-usec",
> - &pdata->settle_delay_usecs);
> - of_property_read_u16(node, "ti,penirq-recheck-delay-usecs",
> - &pdata->penirq_recheck_delay_usecs);
> -
> of_property_read_u16(node, "ti,x-plate-ohms", &pdata->x_plate_ohms);
> of_property_read_u16(node, "ti,y-plate-ohms", &pdata->y_plate_ohms);
>
> @@ -1239,13 +1083,21 @@ static const struct ads7846_platform_data *ads7846_probe_dt(struct device *dev)
> of_property_read_u16(node, "ti,pressure-min", &pdata->pressure_min);
> of_property_read_u16(node, "ti,pressure-max", &pdata->pressure_max);
>
> - of_property_read_u16(node, "ti,debounce-max", &pdata->debounce_max);
> - of_property_read_u16(node, "ti,debounce-tol", &pdata->debounce_tol);
> - of_property_read_u16(node, "ti,debounce-rep", &pdata->debounce_rep);
> -
> of_property_read_u32(node, "ti,pendown-gpio-debounce",
> &pdata->gpio_pendown_debounce);
>
> + of_property_read_u32(node, "ti,skip-samples", &pdata->skip_samples);
> + of_property_read_u32(node, "ti,sample-period-msecs",
> + &pdata->sample_period);
> + of_property_read_u32(node, "ti,report-period-msecs",
> + &pdata->report_period);
> + of_property_read_u32(node, "ti,filter-tolerance",
> + &pdata->filter_tolerance);
> + of_property_read_u32(node, "ti,touch-resistance-threshold",
> + &pdata->rt_threshold);
> + of_property_read_u32(node, "ti,pendown-glitch-max",
> + &pdata->pendown_glitch_max);
> +
> pdata->wakeup = of_property_read_bool(node, "wakeup-source") ||
> of_property_read_bool(node, "linux,wakeup");
>
> @@ -1265,7 +1117,6 @@ static int ads7846_probe(struct spi_device *spi)
> {
> const struct ads7846_platform_data *pdata;
> struct ads7846 *ts;
> - struct ads7846_packet *packet;
> struct input_dev *input_dev;
> unsigned long irq_flags;
> int err;
> @@ -1294,16 +1145,14 @@ static int ads7846_probe(struct spi_device *spi)
> 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) {
> + if (!ts || !input_dev) {
> err = -ENOMEM;
> goto err_free_mem;
> }
>
> spi_set_drvdata(spi, ts);
>
> - ts->packet = packet;
> ts->spi = spi;
> ts->input = input_dev;
>
> @@ -1323,40 +1172,23 @@ static int ads7846_probe(struct spi_device *spi)
> 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;
> + ts->skip_samples = (pdata->skip_samples <= MAX_SKIP_SAMPLES) ?
> + pdata->skip_samples : 0;
> + ts->sample_period = pdata->sample_period ? : 10;
> + ts->report_period = pdata->report_period ? : 10;
> + ts->filter_tolerance = pdata->filter_tolerance ? : 80;
> + ts->rt_threshold = pdata->rt_threshold ? : 3500;
> + ts->pendown_glitch_max = pdata->pendown_glitch_max;
>
> ts->vref_mv = pdata->vref_mv;
> ts->swap_xy = pdata->swap_xy;
> ts->invert_x = pdata->invert_x;
> ts->invert_y = pdata->invert_y;
>
> - 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, pdata);
> 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));
> @@ -1424,7 +1256,7 @@ static int ads7846_probe(struct spi_device *spi)
> * the touchscreen, in case it's not connected.
> */
> if (ts->model == 7845)
> - ads7845_read12_ser(&spi->dev, PWRDOWN);
> + ads7846_read12_ser(&spi->dev, PWRDOWN);
> else
> (void) ads7846_read12_ser(&spi->dev, READ_12BIT_SER(vaux));
>
> @@ -1465,7 +1297,6 @@ static int ads7846_probe(struct spi_device *spi)
> ts->filter_cleanup(ts->filter_data);
> err_free_mem:
> input_free_device(input_dev);
> - kfree(packet);
> kfree(ts);
> return err;
> }
> @@ -1496,7 +1327,6 @@ static int ads7846_remove(struct spi_device *spi)
> if (ts->filter_cleanup)
> ts->filter_cleanup(ts->filter_data);
>
> - kfree(ts->packet);
> kfree(ts);
>
> dev_dbg(&spi->dev, "unregistered touchscreen\n");
> diff --git a/include/linux/spi/ads7846.h b/include/linux/spi/ads7846.h
> index 6b8022c625a3..95d616797f52 100644
> --- a/include/linux/spi/ads7846.h
> +++ b/include/linux/spi/ads7846.h
> @@ -23,19 +23,6 @@ struct ads7846_platform_data {
> bool invert_x; /* Invert x axis */
> bool invert_y; /* Invert y axis */
>
> - /* Settling time of the analog signals; a function of Vcc and the
> - * capacitance on the X/Y drivers. If set to non-zero, two samples
> - * are taken with settle_delay us apart, and the second one is used.
> - * ~150 uSec with 0.01uF caps.
> - */
> - u16 settle_delay_usecs;
> -
> - /* If set to non-zero, after samples are taken this delay is applied
> - * and penirq is rechecked, to help avoid false events. This value
> - * is affected by the material used to build the touch layer.
> - */
> - u16 penirq_recheck_delay_usecs;
> -
> u16 x_plate_ohms;
> u16 y_plate_ohms;
>
> @@ -43,15 +30,19 @@ struct ads7846_platform_data {
> u16 y_min, y_max;
> u16 pressure_min, pressure_max;
>
> - u16 debounce_max; /* max number of additional readings
> - * per sample */
> - u16 debounce_tol; /* tolerance used for filtering */
> - u16 debounce_rep; /* additional consecutive good readings
> - * required after the first two */
> int gpio_pendown; /* the GPIO used to decide the pendown
> * state if get_pendown_state == NULL */
> int gpio_pendown_debounce; /* platform specific debounce time for
> * the gpio_pendown */
> + unsigned int skip_samples; /* Skip n (<=2) samples after switching
> + * plates. */
> + unsigned int rt_threshold; /* Rt threshold for valid touch */
> + unsigned int filter_tolerance; /* Filter X/Y tolerance (raw) */
> + unsigned int sample_period; /* Touchscreen sample period in ms */
> + unsigned int report_period; /* Touchscreen reporting period in ms.
> + * Must be a multiple of sample_period */
> + unsigned int pendown_glitch_max; /* Maximum samples of pendown signal
> + * glitch */
> int (*get_pendown_state)(void);
> int (*filter_init) (const struct ads7846_platform_data *pdata,
> void **filter_data);
Best regards,
--
David Jander
Protonic Holland.
