linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan
@ 2012-12-14  1:46 Marek Vasut
  2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Marek Vasut @ 2012-12-14  1:46 UTC (permalink / raw)
  To: linux-arm-kernel

This structure definition seems to be some kind of remnant from
previous development that accidentally made it mainline. Remove
it as it's unused.

Remove unused "enable" field from struct mxs_lradc as well. This
seems to be remnant of early development too.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/staging/iio/adc/mxs-lradc.c |    7 -------
 1 file changed, 7 deletions(-)

V2: The patch is now combination of previously two separate patches:
[PATCH 1/4] iio: mxs: Remove unused struct mxs_lradc_chan
[PATCH 2/4] iio: mxs: Remove unused fields from mxs_lradc

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index ca7c1fa..6249957 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -75,11 +75,6 @@ static const char * const mxs_lradc_irq_name[] = {
 	"mxs-lradc-button1",
 };
 
-struct mxs_lradc_chan {
-	uint8_t				slot;
-	uint8_t				flags;
-};
-
 struct mxs_lradc {
 	struct device		*dev;
 	void __iomem		*base;
@@ -90,8 +85,6 @@ struct mxs_lradc {
 
 	struct mutex		lock;
 
-	uint8_t			enable;
-
 	struct completion	completion;
 };
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-14  1:46 [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Marek Vasut
@ 2012-12-14  1:46 ` Marek Vasut
  2012-12-14  1:58   ` Fabio Estevam
                     ` (2 more replies)
  2012-12-14  1:46 ` [PATCH 3/3] iio: mxs: Enable touchscreen on m28evk Marek Vasut
  2012-12-27 11:25 ` [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Jonathan Cameron
  2 siblings, 3 replies; 20+ messages in thread
From: Marek Vasut @ 2012-12-14  1:46 UTC (permalink / raw)
  To: linux-arm-kernel

This patch implements support for sampling of a touchscreen into
the MXS LRADC driver. The LRADC block allows configuring some of
it's channels into special mode where they either output the drive
voltage or sample it, allowing it to operate a 4-wire or 5-wire
resistive touchscreen.

In case the touchscreen mode is enabled, the LRADC slot #7 is
reserved for touchscreen only, therefore it is not possible to
sample 8 LRADC channels at time, but only 7 channels.

The touchscreen controller is configured such that the PENDOWN event
disables touchscreen interrupts and triggers execution of worker
thread, which then polls the touchscreen controller for X, Y and
Pressure values. This reduces the overhead of interrupt-driven
operation. Upon the PENUP event, the worker thread re-enables the
PENDOWN detection interrupt and exits.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
---
 .../bindings/staging/iio/adc/mxs-lradc.txt         |    6 +
 drivers/staging/iio/adc/mxs-lradc.c                |  469 +++++++++++++++++++-
 2 files changed, 454 insertions(+), 21 deletions(-)

V2: - Replace the use_touchscreen* with enum mxs_lradc_ts, which now tracks
      touchscreen state (OFF, 4-wire, 5-wire)
    - Add "stop_touchscreen" bit, which indicates the touchscreen is going down
      and makes the driver not re-enable touchscreen interrupts and start any
      new touchscreen works.
    - Make all bitfields "unsigned int" instead of plain "int"
    - Use switch(plate) in mxs_lradc_ts_sample()
    - Cancel touchscreen thread in mxs_lradc_ts_close(), do this only after
      we are sure touchscreen interrupt is off and will never be re-enabled.
      This avoids serious race condition.
    - Call input_free_device() if input_register_device() fails to avoid memory
      leak.
    - Do not call input_free_device() after input_unregister_device() in
      mxs_lradc_ts_unregister() to avoid double-free

diff --git a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
index 801d58c..4688205 100644
--- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
+++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
@@ -5,6 +5,12 @@ Required properties:
 - reg: Address and length of the register set for the device
 - interrupts: Should contain the LRADC interrupts
 
+Optional properties:
+- fsl,lradc-touchscreen-wires: Number of wires used to connect the touchscreen
+                               to LRADC. Valid value is either 4 or 5. If this
+                               property is not present, then the touchscreen is
+                               disabled.
+
 Examples:
 
 	lradc at 80050000 {
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 6249957..52317ef 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -32,6 +32,8 @@
 #include <linux/stmp_device.h>
 #include <linux/bitops.h>
 #include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/input.h>
 
 #include <mach/mxs.h>
 #include <mach/common.h>
@@ -59,6 +61,21 @@
 #define LRADC_DELAY_TIMER_PER	200
 #define LRADC_DELAY_TIMER_LOOP	5
 
+/*
+ * Once the pen touches the touchscreen, the touchscreen switches from
+ * IRQ-driven mode to polling mode to prevent interrupt storm. The polling
+ * is realized by worker thread, which is called every 20 or so milliseconds.
+ * This gives the touchscreen enough fluence and does not strain the system
+ * too much.
+ */
+#define LRADC_TS_SAMPLE_DELAY_MS	5
+
+/*
+ * The LRADC reads the following amount of samples from each touchscreen
+ * channel and the driver then computes avarage of these.
+ */
+#define LRADC_TS_SAMPLE_AMOUNT		4
+
 static const char * const mxs_lradc_irq_name[] = {
 	"mxs-lradc-touchscreen",
 	"mxs-lradc-thresh0",
@@ -75,6 +92,12 @@ static const char * const mxs_lradc_irq_name[] = {
 	"mxs-lradc-button1",
 };
 
+enum mxs_lradc_ts {
+	MXS_LRADC_TOUCHSCREEN_NONE = 0,
+	MXS_LRADC_TOUCHSCREEN_4WIRE,
+	MXS_LRADC_TOUCHSCREEN_5WIRE,
+};
+
 struct mxs_lradc {
 	struct device		*dev;
 	void __iomem		*base;
@@ -86,21 +109,69 @@ struct mxs_lradc {
 	struct mutex		lock;
 
 	struct completion	completion;
+
+	/*
+	 * Touchscreen LRADC channels receives a private slot in the CTRL4
+	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
+	 * CTRL4 register can be mapped to LRADC channels when using the
+	 * touchscreen.
+	 *
+	 * Furthermore, certain LRADC channels are shared between touchscreen
+	 * and/or touch-buttons and generic LRADC block. Therefore when using
+	 * either of these, these channels are not available for the regular
+	 * sampling. The shared channels are as follows:
+	 *
+	 * CH0 -- Touch button #0
+	 * CH1 -- Touch button #1
+	 * CH2 -- Touch screen XPUL
+	 * CH3 -- Touch screen YPLL
+	 * CH4 -- Touch screen XNUL
+	 * CH5 -- Touch screen YNLR
+	 * CH6 -- Touch screen WIPER (5-wire only)
+	 *
+	 * The bitfields below represents which parts of the LRADC block are
+	 * switched into special mode of operation. These channels can not
+	 * be sampled as regular LRADC channels. The driver will refuse any
+	 * attempt to sample these channels.
+	 */
+#define CHAN_MASK_TOUCHBUTTON		(0x3 << 0)
+#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 2)
+#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 2)
+	enum mxs_lradc_ts	use_touchscreen;
+	unsigned int		stop_touchscreen:1;
+	unsigned int		use_touchbutton:1;
+
+	struct input_dev	*ts_input;
+	struct work_struct	ts_work;
 };
 
 #define	LRADC_CTRL0				0x00
-#define LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
-#define LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
+#define	LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
+#define	LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
+#define	LRADC_CTRL0_YNNSW	/* YM */	(1 << 21)
+#define	LRADC_CTRL0_YPNSW	/* YP */	(1 << 20)
+#define	LRADC_CTRL0_YPPSW	/* YP */	(1 << 19)
+#define	LRADC_CTRL0_XNNSW	/* XM */	(1 << 18)
+#define	LRADC_CTRL0_XNPSW	/* XM */	(1 << 17)
+#define	LRADC_CTRL0_XPPSW	/* XP */	(1 << 16)
+#define	LRADC_CTRL0_PLATE_MASK			(0x3f << 16)
 
 #define	LRADC_CTRL1				0x10
-#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
-#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
+#define	LRADC_CTRL1_TOUCH_DETECT_IRQ_EN		(1 << 24)
 #define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
 #define	LRADC_CTRL1_LRADC_IRQ_EN_MASK		(0x1fff << 16)
+#define	LRADC_CTRL1_LRADC_IRQ_EN_OFFSET		16
+#define	LRADC_CTRL1_TOUCH_DETECT_IRQ		(1 << 8)
+#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
+#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
+#define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
 
 #define	LRADC_CTRL2				0x20
 #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
 
+#define	LRADC_STATUS				0x40
+#define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
+
 #define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
 #define	LRADC_CH_ACCUMULATE			(1 << 29)
 #define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
@@ -132,6 +203,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 {
 	struct mxs_lradc *lradc = iio_priv(iio_dev);
 	int ret;
+	unsigned long mask;
 
 	if (m != IIO_CHAN_INFO_RAW)
 		return -EINVAL;
@@ -140,6 +212,12 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	if (chan->channel > LRADC_MAX_TOTAL_CHANS)
 		return -EINVAL;
 
+	/* Validate the channel if it doesn't intersect with reserved chans. */
+	bitmap_set(&mask, chan->channel, 1);
+	ret = iio_validate_scan_mask_onehot(iio_dev, &mask);
+	if (ret)
+		return -EINVAL;
+
 	/*
 	 * See if there is no buffered operation in progess. If there is, simply
 	 * bail out. This can be improved to support both buffered and raw IO at
@@ -161,7 +239,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
-	writel(chan->channel, lradc->base + LRADC_CTRL4);
+	/* Clean the slot's previous content, then set new one. */
+	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
+		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
+	writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
+
 	writel(0, lradc->base + LRADC_CH(0));
 
 	/* Enable the IRQ and start sampling the channel. */
@@ -195,6 +277,272 @@ static const struct iio_info mxs_lradc_iio_info = {
 };
 
 /*
+ * Touchscreen handling
+ */
+enum lradc_ts_plate {
+	LRADC_SAMPLE_X,
+	LRADC_SAMPLE_Y,
+	LRADC_SAMPLE_PRESSURE,
+};
+
+static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
+{
+	uint32_t reg;
+
+	/* Enable touch detection. */
+	writel(LRADC_CTRL0_PLATE_MASK,
+		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+
+	msleep(LRADC_TS_SAMPLE_DELAY_MS);
+
+	reg = readl(lradc->base + LRADC_STATUS);
+
+	return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
+}
+
+static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
+				enum lradc_ts_plate plate, int change)
+{
+	unsigned long delay, jiff;
+	uint32_t reg, ctrl0 = 0, chan = 0;
+	/* The touchscreen always uses CTRL4 slot #7. */
+	const uint8_t slot = 7;
+	uint32_t val;
+
+	/*
+	 * There are three correct configurations of the controller sampling
+	 * the touchscreen, each of these configuration provides different
+	 * information from the touchscreen.
+	 *
+	 * The following table describes the sampling configurations:
+	 * +-------------+-------+-------+-------+
+	 * | Wire \ Axis |   X   |   Y   |   Z   |
+	 * +---------------------+-------+-------+
+	 * |   X+ (CH2)  |   HI  |   TS  |   TS  |
+	 * +-------------+-------+-------+-------+
+	 * |   X- (CH4)  |   LO  |   SH  |   HI  |
+	 * +-------------+-------+-------+-------+
+	 * |   Y+ (CH3)  |   SH  |   HI  |   HI  |
+	 * +-------------+-------+-------+-------+
+	 * |   Y- (CH5)  |   TS  |   LO  |   SH  |
+	 * +-------------+-------+-------+-------+
+	 *
+	 * HI ... strong '1'  ; LO ... strong '0'
+	 * SH ... sample here ; TS ... tri-state
+	 *
+	 * There are a few other ways of obtaining the Z coordinate
+	 * (aka. pressure), but the one in the table seems to be the
+	 * most reliable one.
+	 */
+	switch (plate) {
+	case LRADC_SAMPLE_X:
+		ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
+		chan = 3;
+		break;
+	case LRADC_SAMPLE_Y:
+		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
+		chan = 4;
+		break;
+	case LRADC_SAMPLE_PRESSURE:
+		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
+		chan = 5;
+		break;
+	}
+
+	if (change) {
+		writel(LRADC_CTRL0_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+
+		writel(LRADC_CTRL4_LRADCSELECT_MASK(slot),
+			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
+		writel(chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
+			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
+	}
+
+	writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
+	writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+
+	delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
+	do {
+		jiff = jiffies;
+		reg = readl_relaxed(lradc->base + LRADC_CTRL1);
+		if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
+			break;
+	} while (time_before(jiff, delay));
+
+	writel(LRADC_CTRL1_LRADC_IRQ(slot),
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+
+	if (time_after_eq(jiff, delay))
+		return -ETIMEDOUT;
+
+	val = readl(lradc->base + LRADC_CH(slot));
+	val &= LRADC_CH_VALUE_MASK;
+
+	return val;
+}
+
+static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
+				enum lradc_ts_plate plate)
+{
+	int32_t val, tot = 0;
+	int i;
+
+	val = mxs_lradc_ts_sample(lradc, plate, 1);
+
+	/* Delay a bit so the touchscreen is stable. */
+	mdelay(2);
+
+	for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
+		val = mxs_lradc_ts_sample(lradc, plate, 0);
+		tot += val;
+	}
+
+	return tot / LRADC_TS_SAMPLE_AMOUNT;
+}
+
+static void mxs_lradc_ts_work(struct work_struct *ts_work)
+{
+	struct mxs_lradc *lradc = container_of(ts_work,
+				struct mxs_lradc, ts_work);
+	int val_x, val_y, val_p;
+	bool valid = false;
+
+	while (mxs_lradc_ts_touched(lradc)) {
+		/* Disable touch detector so we can sample the touchscreen. */
+		writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+
+		if (likely(valid)) {
+			input_report_abs(lradc->ts_input, ABS_X, val_x);
+			input_report_abs(lradc->ts_input, ABS_Y, val_y);
+			input_report_abs(lradc->ts_input, ABS_PRESSURE, val_p);
+			input_report_key(lradc->ts_input, BTN_TOUCH, 1);
+			input_sync(lradc->ts_input);
+		}
+
+		valid = false;
+
+		val_x = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_X);
+		if (val_x < 0)
+			continue;
+		val_y = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_Y);
+		if (val_y < 0)
+			continue;
+		val_p = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_PRESSURE);
+		if (val_p < 0)
+			continue;
+
+		valid = true;
+	}
+
+	input_report_abs(lradc->ts_input, ABS_PRESSURE, 0);
+	input_report_key(lradc->ts_input, BTN_TOUCH, 0);
+	input_sync(lradc->ts_input);
+
+	/* Do not restart the TS IRQ if the driver is shutting down. */
+	if (lradc->stop_touchscreen)
+		return;
+
+	/* Restart the touchscreen interrupts. */
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+}
+
+static int mxs_lradc_ts_open(struct input_dev *dev)
+{
+	struct mxs_lradc *lradc = input_get_drvdata(dev);
+
+	/* The touchscreen is starting. */
+	lradc->stop_touchscreen = 0;
+
+	/* Enable the touch-detect circuitry. */
+	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+
+	/* Enable the touch-detect IRQ. */
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+
+	return 0;
+}
+
+static void mxs_lradc_ts_close(struct input_dev *dev)
+{
+	struct mxs_lradc *lradc = input_get_drvdata(dev);
+
+	/* Indicate the touchscreen is stopping. */
+	lradc->stop_touchscreen = 1;
+
+	/* Disable touchscreen touch-detect IRQ. */
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+
+	/* Power-down touchscreen touch-detect circuitry. */
+	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+
+	/* Wait until touchscreen thread finishes any possible remnants. */
+	cancel_work_sync(&lradc->ts_work);
+}
+
+static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
+{
+	struct input_dev *input;
+	struct device *dev = lradc->dev;
+	int ret;
+
+	if (!lradc->use_touchscreen)
+		return 0;
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_warn(dev, "Failed to allocate TS device, disabling.\n");
+		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
+		return 0;
+	}
+
+	input->name = DRIVER_NAME;
+	input->id.bustype = BUS_HOST;
+	input->dev.parent = dev;
+	input->open = mxs_lradc_ts_open;
+	input->close = mxs_lradc_ts_close;
+
+	__set_bit(EV_ABS, input->evbit);
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(BTN_TOUCH, input->keybit);
+	input_set_abs_params(input, ABS_X, 0, LRADC_CH_VALUE_MASK, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, LRADC_CH_VALUE_MASK, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE, 0, LRADC_CH_VALUE_MASK, 0, 0);
+
+	lradc->ts_input = input;
+	input_set_drvdata(input, lradc);
+	ret = input_register_device(input);
+	if (ret)
+		input_free_device(lradc->ts_input);
+
+	return ret;
+}
+
+static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
+{
+	if (!lradc->use_touchscreen)
+		return;
+
+	if (!lradc->ts_input)
+		return;
+
+	cancel_work_sync(&lradc->ts_work);
+
+	input_unregister_device(lradc->ts_input);
+}
+
+/*
  * IRQ Handling
  */
 static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
@@ -202,14 +550,24 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 	struct iio_dev *iio = data;
 	struct mxs_lradc *lradc = iio_priv(iio);
 	unsigned long reg = readl(lradc->base + LRADC_CTRL1);
+	const uint32_t ts_irq_mask =
+		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
+		LRADC_CTRL1_TOUCH_DETECT_IRQ;
 
 	if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
 		return IRQ_NONE;
 
 	/*
-	 * Touchscreen IRQ handling code shall probably have priority
-	 * and therefore shall be placed here.
+	 * Touchscreen IRQ handling code has priority and therefore
+	 * is placed here. In case touchscreen IRQ arrives, disable
+	 * it ASAP
 	 */
+	if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
+		writel(ts_irq_mask,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+		if (!lradc->stop_touchscreen)
+			schedule_work(&lradc->ts_work);
+	}
 
 	if (iio_buffer_enabled(iio))
 		iio_trigger_poll(iio->trig, iio_get_time_ns());
@@ -306,8 +664,10 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 {
 	struct mxs_lradc *lradc = iio_priv(iio);
 	struct iio_buffer *buffer = iio->buffer;
-	int ret = 0, chan, ofs = 0, enable = 0;
-	uint32_t ctrl4 = 0;
+	int ret = 0, chan, ofs = 0;
+	unsigned long enable = 0;
+	uint32_t ctrl4_set = 0;
+	uint32_t ctrl4_clr = 0;
 	uint32_t ctrl1_irq = 0;
 	const uint32_t chan_value = LRADC_CH_ACCUMULATE |
 		((LRADC_DELAY_TIMER_LOOP - 1) << LRADC_CH_NUM_SAMPLES_OFFSET);
@@ -339,17 +699,20 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
 	for_each_set_bit(chan, buffer->scan_mask, LRADC_MAX_TOTAL_CHANS) {
-		ctrl4 |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
+		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
+		ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
 		ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
 		writel(chan_value, lradc->base + LRADC_CH(ofs));
-		enable |= 1 << ofs;
+		bitmap_set(&enable, ofs, 1);
 		ofs++;
 	};
 
 	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
 		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
 
-	writel(ctrl4, lradc->base + LRADC_CTRL4);
+	writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
+	writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
+
 	writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
 
 	writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
@@ -384,9 +747,33 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
 static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
 					const unsigned long *mask)
 {
-	const int mw = bitmap_weight(mask, iio->masklength);
-
-	return mw <= LRADC_MAX_MAPPED_CHANS;
+	struct mxs_lradc *lradc = iio_priv(iio);
+	const int len = iio->masklength;
+	const int map_chans = bitmap_weight(mask, len);
+	int rsvd_chans = 0;
+	unsigned long rsvd_mask = 0;
+
+	if (lradc->use_touchbutton)
+		rsvd_mask |= CHAN_MASK_TOUCHBUTTON;
+	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_4WIRE)
+		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_4WIRE;
+	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
+		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_5WIRE;
+
+	if (lradc->use_touchbutton)
+		rsvd_chans++;
+	if (lradc->use_touchscreen)
+		rsvd_chans++;
+
+	/* Test for attempts to map channels with special mode of operation. */
+	if (bitmap_intersects(mask, &rsvd_mask, len))
+		return 0;
+
+	/* Test for attempts to map more channels then available slots. */
+	if (map_chans + rsvd_chans > LRADC_MAX_MAPPED_CHANS)
+		return 0;
+
+	return 1;
 }
 
 static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
@@ -435,15 +822,29 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
 
 static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
 {
-	int i;
-	const uint32_t cfg =
+	/* The ADC always uses DELAY CHANNEL 0. */
+	const uint32_t adc_cfg =
+		(1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + 0)) |
 		(LRADC_DELAY_TIMER_PER << LRADC_DELAY_DELAY_OFFSET);
 
 	stmp_reset_block(lradc->base);
 
-	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
-		writel(cfg | (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)),
-			lradc->base + LRADC_DELAY(i));
+	/* Configure DELAY CHANNEL 0 for generic ADC sampling. */
+	writel(adc_cfg, lradc->base + LRADC_DELAY(0));
+
+	/* Disable remaining DELAY CHANNELs */
+	writel(0, lradc->base + LRADC_DELAY(1));
+	writel(0, lradc->base + LRADC_DELAY(2));
+	writel(0, lradc->base + LRADC_DELAY(3));
+
+	/* Configure the touchscreen type */
+	writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
+		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+
+	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
+		writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	}
 
 	/* Start internal temperature sensing. */
 	writel(0, lradc->base + LRADC_CTRL2);
@@ -463,9 +864,11 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
 static int __devinit mxs_lradc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
 	struct mxs_lradc *lradc;
 	struct iio_dev *iio;
 	struct resource *iores;
+	uint32_t ts_wires = 0;
 	int ret = 0;
 	int i;
 
@@ -487,6 +890,21 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
 		goto err_addr;
 	}
 
+	INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
+
+	/* Check if touchscreen is enabled in DT. */
+	ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
+				&ts_wires);
+	if (ret)
+		dev_info(dev, "Touchscreen not enabled.\n");
+	else if (ts_wires == 4)
+		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_4WIRE;
+	else if (ts_wires == 5)
+		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_5WIRE;
+	else
+		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
+				ts_wires);
+
 	/* Grab all IRQ sources */
 	for (i = 0; i < 13; i++) {
 		lradc->irq[i] = platform_get_irq(pdev, i);
@@ -524,11 +942,16 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_trig;
 
+	/* Register the touchscreen input device. */
+	ret = mxs_lradc_ts_register(lradc);
+	if (ret)
+		goto err_dev;
+
 	/* Register IIO device. */
 	ret = iio_device_register(iio);
 	if (ret) {
 		dev_err(dev, "Failed to register IIO device\n");
-		goto err_dev;
+		goto err_ts;
 	}
 
 	/* Configure the hardware. */
@@ -536,6 +959,8 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_ts:
+	mxs_lradc_ts_unregister(lradc);
 err_dev:
 	mxs_lradc_trigger_remove(iio);
 err_trig:
@@ -550,6 +975,8 @@ static int __devexit mxs_lradc_remove(struct platform_device *pdev)
 	struct iio_dev *iio = platform_get_drvdata(pdev);
 	struct mxs_lradc *lradc = iio_priv(iio);
 
+	mxs_lradc_ts_unregister(lradc);
+
 	mxs_lradc_hw_stop(lradc);
 
 	iio_device_unregister(iio);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] iio: mxs: Enable touchscreen on m28evk
  2012-12-14  1:46 [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Marek Vasut
  2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
@ 2012-12-14  1:46 ` Marek Vasut
  2012-12-27 11:25 ` [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Jonathan Cameron
  2 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2012-12-14  1:46 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds necessary DT properties to m28evk to enable touchscreen
in the LRADC block. The M28EVK is shipped with 4-wire resistive touchscreen.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/boot/dts/imx28-m28evk.dts |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/imx28-m28evk.dts b/arch/arm/boot/dts/imx28-m28evk.dts
index c718f91..712d99a 100644
--- a/arch/arm/boot/dts/imx28-m28evk.dts
+++ b/arch/arm/boot/dts/imx28-m28evk.dts
@@ -213,6 +213,7 @@
 
 			lradc at 80050000 {
 				status = "okay";
+				fsl,lradc-touchscreen-wires = <4>;
 			};
 
 			duart: serial at 80074000 {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
@ 2012-12-14  1:58   ` Fabio Estevam
  2012-12-14  2:04     ` Marek Vasut
  2012-12-19 17:01   ` Marek Vasut
  2012-12-27 11:31   ` Jonathan Cameron
  2 siblings, 1 reply; 20+ messages in thread
From: Fabio Estevam @ 2012-12-14  1:58 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marek,

On Thu, Dec 13, 2012 at 11:46 PM, Marek Vasut <marex@denx.de> wrote:

> +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> +{
> +       struct input_dev *input;
> +       struct device *dev = lradc->dev;
> +       int ret;
> +
> +       if (!lradc->use_touchscreen)
> +               return 0;

Shouldn't we return an error code here?

> +
> +       input = input_allocate_device();
> +       if (!input) {
> +               dev_warn(dev, "Failed to allocate TS device, disabling.\n");
> +               lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
> +               return 0;
> +       }

Shouldn't we return an error code here?

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-14  1:58   ` Fabio Estevam
@ 2012-12-14  2:04     ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2012-12-14  2:04 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Fabio Estevam,

> Hi Marek,
> 
> On Thu, Dec 13, 2012 at 11:46 PM, Marek Vasut <marex@denx.de> wrote:
> > +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> > +{
> > +       struct input_dev *input;
> > +       struct device *dev = lradc->dev;
> > +       int ret;
> > +
> > +       if (!lradc->use_touchscreen)
> > +               return 0;
> 
> Shouldn't we return an error code here?

No, why ? No touchscreen present, we register nothing.

> > +
> > +       input = input_allocate_device();
> > +       if (!input) {
> > +               dev_warn(dev, "Failed to allocate TS device,
> > disabling.\n"); +               lradc->use_touchscreen =
> > MXS_LRADC_TOUCHSCREEN_NONE; +               return 0;
> > +       }
> 
> Shouldn't we return an error code here?

Yes.

> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
  2012-12-14  1:58   ` Fabio Estevam
@ 2012-12-19 17:01   ` Marek Vasut
  2012-12-27 12:16     ` Jonathan Cameron
  2013-01-10  0:57     ` Dmitry Torokhov
  2012-12-27 11:31   ` Jonathan Cameron
  2 siblings, 2 replies; 20+ messages in thread
From: Marek Vasut @ 2012-12-19 17:01 UTC (permalink / raw)
  To: linux-arm-kernel

Ccing Dmitry.

Also, Jonathan, can I get your review please?

> This patch implements support for sampling of a touchscreen into
> the MXS LRADC driver. The LRADC block allows configuring some of
> it's channels into special mode where they either output the drive
> voltage or sample it, allowing it to operate a 4-wire or 5-wire
> resistive touchscreen.
> 
> In case the touchscreen mode is enabled, the LRADC slot #7 is
> reserved for touchscreen only, therefore it is not possible to
> sample 8 LRADC channels at time, but only 7 channels.
> 
> The touchscreen controller is configured such that the PENDOWN event
> disables touchscreen interrupts and triggers execution of worker
> thread, which then polls the touchscreen controller for X, Y and
> Pressure values. This reduces the overhead of interrupt-driven
> operation. Upon the PENUP event, the worker thread re-enables the
> PENDOWN detection interrupt and exits.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> ---
>  .../bindings/staging/iio/adc/mxs-lradc.txt         |    6 +
>  drivers/staging/iio/adc/mxs-lradc.c                |  469
> +++++++++++++++++++- 2 files changed, 454 insertions(+), 21 deletions(-)
> 
> V2: - Replace the use_touchscreen* with enum mxs_lradc_ts, which now tracks
>       touchscreen state (OFF, 4-wire, 5-wire)
>     - Add "stop_touchscreen" bit, which indicates the touchscreen is going
> down and makes the driver not re-enable touchscreen interrupts and start
> any new touchscreen works.
>     - Make all bitfields "unsigned int" instead of plain "int"
>     - Use switch(plate) in mxs_lradc_ts_sample()
>     - Cancel touchscreen thread in mxs_lradc_ts_close(), do this only after
>       we are sure touchscreen interrupt is off and will never be
> re-enabled. This avoids serious race condition.
>     - Call input_free_device() if input_register_device() fails to avoid
> memory leak.
>     - Do not call input_free_device() after input_unregister_device() in
>       mxs_lradc_ts_unregister() to avoid double-free
> 
> diff --git
> a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt index
> 801d58c..4688205 100644
> --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> @@ -5,6 +5,12 @@ Required properties:
>  - reg: Address and length of the register set for the device
>  - interrupts: Should contain the LRADC interrupts
> 
> +Optional properties:
> +- fsl,lradc-touchscreen-wires: Number of wires used to connect the
> touchscreen +                               to LRADC. Valid value is
> either 4 or 5. If this +                               property is not
> present, then the touchscreen is +                               disabled.
> +
>  Examples:
> 
>  	lradc at 80050000 {
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index 6249957..52317ef 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -32,6 +32,8 @@
>  #include <linux/stmp_device.h>
>  #include <linux/bitops.h>
>  #include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/input.h>
> 
>  #include <mach/mxs.h>
>  #include <mach/common.h>
> @@ -59,6 +61,21 @@
>  #define LRADC_DELAY_TIMER_PER	200
>  #define LRADC_DELAY_TIMER_LOOP	5
> 
> +/*
> + * Once the pen touches the touchscreen, the touchscreen switches from
> + * IRQ-driven mode to polling mode to prevent interrupt storm. The polling
> + * is realized by worker thread, which is called every 20 or so
> milliseconds. + * This gives the touchscreen enough fluence and does not
> strain the system + * too much.
> + */
> +#define LRADC_TS_SAMPLE_DELAY_MS	5
> +
> +/*
> + * The LRADC reads the following amount of samples from each touchscreen
> + * channel and the driver then computes avarage of these.
> + */
> +#define LRADC_TS_SAMPLE_AMOUNT		4
> +
>  static const char * const mxs_lradc_irq_name[] = {
>  	"mxs-lradc-touchscreen",
>  	"mxs-lradc-thresh0",
> @@ -75,6 +92,12 @@ static const char * const mxs_lradc_irq_name[] = {
>  	"mxs-lradc-button1",
>  };
> 
> +enum mxs_lradc_ts {
> +	MXS_LRADC_TOUCHSCREEN_NONE = 0,
> +	MXS_LRADC_TOUCHSCREEN_4WIRE,
> +	MXS_LRADC_TOUCHSCREEN_5WIRE,
> +};
> +
>  struct mxs_lradc {
>  	struct device		*dev;
>  	void __iomem		*base;
> @@ -86,21 +109,69 @@ struct mxs_lradc {
>  	struct mutex		lock;
> 
>  	struct completion	completion;
> +
> +	/*
> +	 * Touchscreen LRADC channels receives a private slot in the CTRL4
> +	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
> +	 * CTRL4 register can be mapped to LRADC channels when using the
> +	 * touchscreen.
> +	 *
> +	 * Furthermore, certain LRADC channels are shared between touchscreen
> +	 * and/or touch-buttons and generic LRADC block. Therefore when using
> +	 * either of these, these channels are not available for the regular
> +	 * sampling. The shared channels are as follows:
> +	 *
> +	 * CH0 -- Touch button #0
> +	 * CH1 -- Touch button #1
> +	 * CH2 -- Touch screen XPUL
> +	 * CH3 -- Touch screen YPLL
> +	 * CH4 -- Touch screen XNUL
> +	 * CH5 -- Touch screen YNLR
> +	 * CH6 -- Touch screen WIPER (5-wire only)
> +	 *
> +	 * The bitfields below represents which parts of the LRADC block are
> +	 * switched into special mode of operation. These channels can not
> +	 * be sampled as regular LRADC channels. The driver will refuse any
> +	 * attempt to sample these channels.
> +	 */
> +#define CHAN_MASK_TOUCHBUTTON		(0x3 << 0)
> +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 2)
> +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 2)
> +	enum mxs_lradc_ts	use_touchscreen;
> +	unsigned int		stop_touchscreen:1;
> +	unsigned int		use_touchbutton:1;
> +
> +	struct input_dev	*ts_input;
> +	struct work_struct	ts_work;
>  };
> 
>  #define	LRADC_CTRL0				0x00
> -#define LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
> -#define LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
> +#define	LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
> +#define	LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
> +#define	LRADC_CTRL0_YNNSW	/* YM */	(1 << 21)
> +#define	LRADC_CTRL0_YPNSW	/* YP */	(1 << 20)
> +#define	LRADC_CTRL0_YPPSW	/* YP */	(1 << 19)
> +#define	LRADC_CTRL0_XNNSW	/* XM */	(1 << 18)
> +#define	LRADC_CTRL0_XNPSW	/* XM */	(1 << 17)
> +#define	LRADC_CTRL0_XPPSW	/* XP */	(1 << 16)
> +#define	LRADC_CTRL0_PLATE_MASK			(0x3f << 16)
> 
>  #define	LRADC_CTRL1				0x10
> -#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
> -#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
> +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ_EN		(1 << 24)
>  #define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
>  #define	LRADC_CTRL1_LRADC_IRQ_EN_MASK		(0x1fff << 16)
> +#define	LRADC_CTRL1_LRADC_IRQ_EN_OFFSET		16
> +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ		(1 << 8)
> +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
> +#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
> +#define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
> 
>  #define	LRADC_CTRL2				0x20
>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
> 
> +#define	LRADC_STATUS				0x40
> +#define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
> +
>  #define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
>  #define	LRADC_CH_ACCUMULATE			(1 << 29)
>  #define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
> @@ -132,6 +203,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  {
>  	struct mxs_lradc *lradc = iio_priv(iio_dev);
>  	int ret;
> +	unsigned long mask;
> 
>  	if (m != IIO_CHAN_INFO_RAW)
>  		return -EINVAL;
> @@ -140,6 +212,12 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	if (chan->channel > LRADC_MAX_TOTAL_CHANS)
>  		return -EINVAL;
> 
> +	/* Validate the channel if it doesn't intersect with reserved chans. */
> +	bitmap_set(&mask, chan->channel, 1);
> +	ret = iio_validate_scan_mask_onehot(iio_dev, &mask);
> +	if (ret)
> +		return -EINVAL;
> +
>  	/*
>  	 * See if there is no buffered operation in progess. If there is, simply
>  	 * bail out. This can be improved to support both buffered and raw IO at
> @@ -161,7 +239,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> 
> -	writel(chan->channel, lradc->base + LRADC_CTRL4);
> +	/* Clean the slot's previous content, then set new one. */
> +	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
> +		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> +	writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +
>  	writel(0, lradc->base + LRADC_CH(0));
> 
>  	/* Enable the IRQ and start sampling the channel. */
> @@ -195,6 +277,272 @@ static const struct iio_info mxs_lradc_iio_info = {
>  };
> 
>  /*
> + * Touchscreen handling
> + */
> +enum lradc_ts_plate {
> +	LRADC_SAMPLE_X,
> +	LRADC_SAMPLE_Y,
> +	LRADC_SAMPLE_PRESSURE,
> +};
> +
> +static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
> +{
> +	uint32_t reg;
> +
> +	/* Enable touch detection. */
> +	writel(LRADC_CTRL0_PLATE_MASK,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +	msleep(LRADC_TS_SAMPLE_DELAY_MS);
> +
> +	reg = readl(lradc->base + LRADC_STATUS);
> +
> +	return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
> +}
> +
> +static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
> +				enum lradc_ts_plate plate, int change)
> +{
> +	unsigned long delay, jiff;
> +	uint32_t reg, ctrl0 = 0, chan = 0;
> +	/* The touchscreen always uses CTRL4 slot #7. */
> +	const uint8_t slot = 7;
> +	uint32_t val;
> +
> +	/*
> +	 * There are three correct configurations of the controller sampling
> +	 * the touchscreen, each of these configuration provides different
> +	 * information from the touchscreen.
> +	 *
> +	 * The following table describes the sampling configurations:
> +	 * +-------------+-------+-------+-------+
> +	 * | Wire \ Axis |   X   |   Y   |   Z   |
> +	 * +---------------------+-------+-------+
> +	 * |   X+ (CH2)  |   HI  |   TS  |   TS  |
> +	 * +-------------+-------+-------+-------+
> +	 * |   X- (CH4)  |   LO  |   SH  |   HI  |
> +	 * +-------------+-------+-------+-------+
> +	 * |   Y+ (CH3)  |   SH  |   HI  |   HI  |
> +	 * +-------------+-------+-------+-------+
> +	 * |   Y- (CH5)  |   TS  |   LO  |   SH  |
> +	 * +-------------+-------+-------+-------+
> +	 *
> +	 * HI ... strong '1'  ; LO ... strong '0'
> +	 * SH ... sample here ; TS ... tri-state
> +	 *
> +	 * There are a few other ways of obtaining the Z coordinate
> +	 * (aka. pressure), but the one in the table seems to be the
> +	 * most reliable one.
> +	 */
> +	switch (plate) {
> +	case LRADC_SAMPLE_X:
> +		ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
> +		chan = 3;
> +		break;
> +	case LRADC_SAMPLE_Y:
> +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
> +		chan = 4;
> +		break;
> +	case LRADC_SAMPLE_PRESSURE:
> +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
> +		chan = 5;
> +		break;
> +	}
> +
> +	if (change) {
> +		writel(LRADC_CTRL0_PLATE_MASK,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +		writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +		writel(LRADC_CTRL4_LRADCSELECT_MASK(slot),
> +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> +		writel(chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
> +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +	}
> +
> +	writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
> +	writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +	delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
> +	do {
> +		jiff = jiffies;
> +		reg = readl_relaxed(lradc->base + LRADC_CTRL1);
> +		if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
> +			break;
> +	} while (time_before(jiff, delay));
> +
> +	writel(LRADC_CTRL1_LRADC_IRQ(slot),
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +
> +	if (time_after_eq(jiff, delay))
> +		return -ETIMEDOUT;
> +
> +	val = readl(lradc->base + LRADC_CH(slot));
> +	val &= LRADC_CH_VALUE_MASK;
> +
> +	return val;
> +}
> +
> +static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
> +				enum lradc_ts_plate plate)
> +{
> +	int32_t val, tot = 0;
> +	int i;
> +
> +	val = mxs_lradc_ts_sample(lradc, plate, 1);
> +
> +	/* Delay a bit so the touchscreen is stable. */
> +	mdelay(2);
> +
> +	for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
> +		val = mxs_lradc_ts_sample(lradc, plate, 0);
> +		tot += val;
> +	}
> +
> +	return tot / LRADC_TS_SAMPLE_AMOUNT;
> +}
> +
> +static void mxs_lradc_ts_work(struct work_struct *ts_work)
> +{
> +	struct mxs_lradc *lradc = container_of(ts_work,
> +				struct mxs_lradc, ts_work);
> +	int val_x, val_y, val_p;
> +	bool valid = false;
> +
> +	while (mxs_lradc_ts_touched(lradc)) {
> +		/* Disable touch detector so we can sample the touchscreen. */
> +		writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> +		if (likely(valid)) {
> +			input_report_abs(lradc->ts_input, ABS_X, val_x);
> +			input_report_abs(lradc->ts_input, ABS_Y, val_y);
> +			input_report_abs(lradc->ts_input, ABS_PRESSURE, val_p);
> +			input_report_key(lradc->ts_input, BTN_TOUCH, 1);
> +			input_sync(lradc->ts_input);
> +		}
> +
> +		valid = false;
> +
> +		val_x = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_X);
> +		if (val_x < 0)
> +			continue;
> +		val_y = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_Y);
> +		if (val_y < 0)
> +			continue;
> +		val_p = mxs_lradc_ts_sample_filter(lradc, 
LRADC_SAMPLE_PRESSURE);
> +		if (val_p < 0)
> +			continue;
> +
> +		valid = true;
> +	}
> +
> +	input_report_abs(lradc->ts_input, ABS_PRESSURE, 0);
> +	input_report_key(lradc->ts_input, BTN_TOUCH, 0);
> +	input_sync(lradc->ts_input);
> +
> +	/* Do not restart the TS IRQ if the driver is shutting down. */
> +	if (lradc->stop_touchscreen)
> +		return;
> +
> +	/* Restart the touchscreen interrupts. */
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> +}
> +
> +static int mxs_lradc_ts_open(struct input_dev *dev)
> +{
> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> +
> +	/* The touchscreen is starting. */
> +	lradc->stop_touchscreen = 0;
> +
> +	/* Enable the touch-detect circuitry. */
> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +	/* Enable the touch-detect IRQ. */
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> +
> +	return 0;
> +}
> +
> +static void mxs_lradc_ts_close(struct input_dev *dev)
> +{
> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> +
> +	/* Indicate the touchscreen is stopping. */
> +	lradc->stop_touchscreen = 1;
> +
> +	/* Disable touchscreen touch-detect IRQ. */
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +
> +	/* Power-down touchscreen touch-detect circuitry. */
> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> +	/* Wait until touchscreen thread finishes any possible remnants. */
> +	cancel_work_sync(&lradc->ts_work);
> +}
> +
> +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> +{
> +	struct input_dev *input;
> +	struct device *dev = lradc->dev;
> +	int ret;
> +
> +	if (!lradc->use_touchscreen)
> +		return 0;
> +
> +	input = input_allocate_device();
> +	if (!input) {
> +		dev_warn(dev, "Failed to allocate TS device, disabling.\n");
> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
> +		return 0;
> +	}
> +
> +	input->name = DRIVER_NAME;
> +	input->id.bustype = BUS_HOST;
> +	input->dev.parent = dev;
> +	input->open = mxs_lradc_ts_open;
> +	input->close = mxs_lradc_ts_close;
> +
> +	__set_bit(EV_ABS, input->evbit);
> +	__set_bit(EV_KEY, input->evbit);
> +	__set_bit(BTN_TOUCH, input->keybit);
> +	input_set_abs_params(input, ABS_X, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +	input_set_abs_params(input, ABS_Y, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +	input_set_abs_params(input, ABS_PRESSURE, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +
> +	lradc->ts_input = input;
> +	input_set_drvdata(input, lradc);
> +	ret = input_register_device(input);
> +	if (ret)
> +		input_free_device(lradc->ts_input);
> +
> +	return ret;
> +}
> +
> +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> +{
> +	if (!lradc->use_touchscreen)
> +		return;
> +
> +	if (!lradc->ts_input)
> +		return;
> +
> +	cancel_work_sync(&lradc->ts_work);
> +
> +	input_unregister_device(lradc->ts_input);
> +}
> +
> +/*
>   * IRQ Handling
>   */
>  static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> @@ -202,14 +550,24 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void
> *data) struct iio_dev *iio = data;
>  	struct mxs_lradc *lradc = iio_priv(iio);
>  	unsigned long reg = readl(lradc->base + LRADC_CTRL1);
> +	const uint32_t ts_irq_mask =
> +		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
> +		LRADC_CTRL1_TOUCH_DETECT_IRQ;
> 
>  	if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
>  		return IRQ_NONE;
> 
>  	/*
> -	 * Touchscreen IRQ handling code shall probably have priority
> -	 * and therefore shall be placed here.
> +	 * Touchscreen IRQ handling code has priority and therefore
> +	 * is placed here. In case touchscreen IRQ arrives, disable
> +	 * it ASAP
>  	 */
> +	if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
> +		writel(ts_irq_mask,
> +			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +		if (!lradc->stop_touchscreen)
> +			schedule_work(&lradc->ts_work);
> +	}
> 
>  	if (iio_buffer_enabled(iio))
>  		iio_trigger_poll(iio->trig, iio_get_time_ns());
> @@ -306,8 +664,10 @@ static int mxs_lradc_buffer_preenable(struct iio_dev
> *iio) {
>  	struct mxs_lradc *lradc = iio_priv(iio);
>  	struct iio_buffer *buffer = iio->buffer;
> -	int ret = 0, chan, ofs = 0, enable = 0;
> -	uint32_t ctrl4 = 0;
> +	int ret = 0, chan, ofs = 0;
> +	unsigned long enable = 0;
> +	uint32_t ctrl4_set = 0;
> +	uint32_t ctrl4_clr = 0;
>  	uint32_t ctrl1_irq = 0;
>  	const uint32_t chan_value = LRADC_CH_ACCUMULATE |
>  		((LRADC_DELAY_TIMER_LOOP - 1) << LRADC_CH_NUM_SAMPLES_OFFSET);
> @@ -339,17 +699,20 @@ static int mxs_lradc_buffer_preenable(struct iio_dev
> *iio) writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> 
>  	for_each_set_bit(chan, buffer->scan_mask, LRADC_MAX_TOTAL_CHANS) {
> -		ctrl4 |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> +		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> +		ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
>  		ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
>  		writel(chan_value, lradc->base + LRADC_CH(ofs));
> -		enable |= 1 << ofs;
> +		bitmap_set(&enable, ofs, 1);
>  		ofs++;
>  	};
> 
>  	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
>  		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
> 
> -	writel(ctrl4, lradc->base + LRADC_CTRL4);
> +	writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> +	writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +
>  	writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> 
>  	writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
> @@ -384,9 +747,33 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev
> *iio) static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
>  					const unsigned long *mask)
>  {
> -	const int mw = bitmap_weight(mask, iio->masklength);
> -
> -	return mw <= LRADC_MAX_MAPPED_CHANS;
> +	struct mxs_lradc *lradc = iio_priv(iio);
> +	const int len = iio->masklength;
> +	const int map_chans = bitmap_weight(mask, len);
> +	int rsvd_chans = 0;
> +	unsigned long rsvd_mask = 0;
> +
> +	if (lradc->use_touchbutton)
> +		rsvd_mask |= CHAN_MASK_TOUCHBUTTON;
> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_4WIRE)
> +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_4WIRE;
> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
> +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_5WIRE;
> +
> +	if (lradc->use_touchbutton)
> +		rsvd_chans++;
> +	if (lradc->use_touchscreen)
> +		rsvd_chans++;
> +
> +	/* Test for attempts to map channels with special mode of operation. */
> +	if (bitmap_intersects(mask, &rsvd_mask, len))
> +		return 0;
> +
> +	/* Test for attempts to map more channels then available slots. */
> +	if (map_chans + rsvd_chans > LRADC_MAX_MAPPED_CHANS)
> +		return 0;
> +
> +	return 1;
>  }
> 
>  static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
> @@ -435,15 +822,29 @@ static const struct iio_chan_spec
> mxs_lradc_chan_spec[] = {
> 
>  static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
>  {
> -	int i;
> -	const uint32_t cfg =
> +	/* The ADC always uses DELAY CHANNEL 0. */
> +	const uint32_t adc_cfg =
> +		(1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + 0)) |
>  		(LRADC_DELAY_TIMER_PER << LRADC_DELAY_DELAY_OFFSET);
> 
>  	stmp_reset_block(lradc->base);
> 
> -	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> -		writel(cfg | (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)),
> -			lradc->base + LRADC_DELAY(i));
> +	/* Configure DELAY CHANNEL 0 for generic ADC sampling. */
> +	writel(adc_cfg, lradc->base + LRADC_DELAY(0));
> +
> +	/* Disable remaining DELAY CHANNELs */
> +	writel(0, lradc->base + LRADC_DELAY(1));
> +	writel(0, lradc->base + LRADC_DELAY(2));
> +	writel(0, lradc->base + LRADC_DELAY(3));
> +
> +	/* Configure the touchscreen type */
> +	writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
> +		writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +	}
> 
>  	/* Start internal temperature sensing. */
>  	writel(0, lradc->base + LRADC_CTRL2);
> @@ -463,9 +864,11 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
>  static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
>  	struct mxs_lradc *lradc;
>  	struct iio_dev *iio;
>  	struct resource *iores;
> +	uint32_t ts_wires = 0;
>  	int ret = 0;
>  	int i;
> 
> @@ -487,6 +890,21 @@ static int __devinit mxs_lradc_probe(struct
> platform_device *pdev) goto err_addr;
>  	}
> 
> +	INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
> +
> +	/* Check if touchscreen is enabled in DT. */
> +	ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
> +				&ts_wires);
> +	if (ret)
> +		dev_info(dev, "Touchscreen not enabled.\n");
> +	else if (ts_wires == 4)
> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_4WIRE;
> +	else if (ts_wires == 5)
> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_5WIRE;
> +	else
> +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
> +				ts_wires);
> +
>  	/* Grab all IRQ sources */
>  	for (i = 0; i < 13; i++) {
>  		lradc->irq[i] = platform_get_irq(pdev, i);
> @@ -524,11 +942,16 @@ static int __devinit mxs_lradc_probe(struct
> platform_device *pdev) if (ret)
>  		goto err_trig;
> 
> +	/* Register the touchscreen input device. */
> +	ret = mxs_lradc_ts_register(lradc);
> +	if (ret)
> +		goto err_dev;
> +
>  	/* Register IIO device. */
>  	ret = iio_device_register(iio);
>  	if (ret) {
>  		dev_err(dev, "Failed to register IIO device\n");
> -		goto err_dev;
> +		goto err_ts;
>  	}
> 
>  	/* Configure the hardware. */
> @@ -536,6 +959,8 @@ static int __devinit mxs_lradc_probe(struct
> platform_device *pdev)
> 
>  	return 0;
> 
> +err_ts:
> +	mxs_lradc_ts_unregister(lradc);
>  err_dev:
>  	mxs_lradc_trigger_remove(iio);
>  err_trig:
> @@ -550,6 +975,8 @@ static int __devexit mxs_lradc_remove(struct
> platform_device *pdev) struct iio_dev *iio = platform_get_drvdata(pdev);
>  	struct mxs_lradc *lradc = iio_priv(iio);
> 
> +	mxs_lradc_ts_unregister(lradc);
> +
>  	mxs_lradc_hw_stop(lradc);
> 
>  	iio_device_unregister(iio);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan
  2012-12-14  1:46 [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Marek Vasut
  2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
  2012-12-14  1:46 ` [PATCH 3/3] iio: mxs: Enable touchscreen on m28evk Marek Vasut
@ 2012-12-27 11:25 ` Jonathan Cameron
  2 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2012-12-27 11:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/14/2012 01:46 AM, Marek Vasut wrote:
> This structure definition seems to be some kind of remnant from
> previous development that accidentally made it mainline. Remove
> it as it's unused.
> 
> Remove unused "enable" field from struct mxs_lradc as well. This
> seems to be remnant of early development too.
> 
Added to togreg branch of

git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Jonathan
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c |    7 -------
>  1 file changed, 7 deletions(-)
> 
> V2: The patch is now combination of previously two separate patches:
> [PATCH 1/4] iio: mxs: Remove unused struct mxs_lradc_chan
> [PATCH 2/4] iio: mxs: Remove unused fields from mxs_lradc
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index ca7c1fa..6249957 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -75,11 +75,6 @@ static const char * const mxs_lradc_irq_name[] = {
>  	"mxs-lradc-button1",
>  };
>  
> -struct mxs_lradc_chan {
> -	uint8_t				slot;
> -	uint8_t				flags;
> -};
> -
>  struct mxs_lradc {
>  	struct device		*dev;
>  	void __iomem		*base;
> @@ -90,8 +85,6 @@ struct mxs_lradc {
>  
>  	struct mutex		lock;
>  
> -	uint8_t			enable;
> -
>  	struct completion	completion;
>  };
>  
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
  2012-12-14  1:58   ` Fabio Estevam
  2012-12-19 17:01   ` Marek Vasut
@ 2012-12-27 11:31   ` Jonathan Cameron
  2012-12-27 18:19     ` Marek Vasut
  2 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2012-12-27 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/14/2012 01:46 AM, Marek Vasut wrote:
> This patch implements support for sampling of a touchscreen into
> the MXS LRADC driver. The LRADC block allows configuring some of
> it's channels into special mode where they either output the drive
> voltage or sample it, allowing it to operate a 4-wire or 5-wire
> resistive touchscreen.
> 
> In case the touchscreen mode is enabled, the LRADC slot #7 is
> reserved for touchscreen only, therefore it is not possible to
> sample 8 LRADC channels at time, but only 7 channels.
> 
> The touchscreen controller is configured such that the PENDOWN event
> disables touchscreen interrupts and triggers execution of worker
> thread, which then polls the touchscreen controller for X, Y and
> Pressure values. This reduces the overhead of interrupt-driven
> operation. Upon the PENUP event, the worker thread re-enables the
> PENDOWN detection interrupt and exits.
> 
I still have a few reservations about this patch.  Firstly
I would love to see a more generic approach to this as I outlined
before.  Still we can't have everything we want sometimes :)
Also, this is a lot of input related code in a driver that isn't
in the input subsystem.  Really up to Dmitry on whether he is
happy with this, or whether he insists on an mfd.

So all in all, with reservations I'll add this to the IIO tree
IF Dmitry is happy with it and there are no other issues raised
in the meantime.

Jonathan

p.s. If anyone has time, I'd also like to get this out of staging
in the coming cycle.

> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> ---
>  .../bindings/staging/iio/adc/mxs-lradc.txt         |    6 +
>  drivers/staging/iio/adc/mxs-lradc.c                |  469 +++++++++++++++++++-
>  2 files changed, 454 insertions(+), 21 deletions(-)
> 
> V2: - Replace the use_touchscreen* with enum mxs_lradc_ts, which now tracks
>       touchscreen state (OFF, 4-wire, 5-wire)
>     - Add "stop_touchscreen" bit, which indicates the touchscreen is going down
>       and makes the driver not re-enable touchscreen interrupts and start any
>       new touchscreen works.
>     - Make all bitfields "unsigned int" instead of plain "int"
>     - Use switch(plate) in mxs_lradc_ts_sample()
>     - Cancel touchscreen thread in mxs_lradc_ts_close(), do this only after
>       we are sure touchscreen interrupt is off and will never be re-enabled.
>       This avoids serious race condition.
>     - Call input_free_device() if input_register_device() fails to avoid memory
>       leak.
>     - Do not call input_free_device() after input_unregister_device() in
>       mxs_lradc_ts_unregister() to avoid double-free
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> index 801d58c..4688205 100644
> --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> @@ -5,6 +5,12 @@ Required properties:
>  - reg: Address and length of the register set for the device
>  - interrupts: Should contain the LRADC interrupts
>  
> +Optional properties:
> +- fsl,lradc-touchscreen-wires: Number of wires used to connect the touchscreen
> +                               to LRADC. Valid value is either 4 or 5. If this
> +                               property is not present, then the touchscreen is
> +                               disabled.
> +
>  Examples:
>  
>  	lradc at 80050000 {
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 6249957..52317ef 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -32,6 +32,8 @@
>  #include <linux/stmp_device.h>
>  #include <linux/bitops.h>
>  #include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/input.h>
>  
>  #include <mach/mxs.h>
>  #include <mach/common.h>
> @@ -59,6 +61,21 @@
>  #define LRADC_DELAY_TIMER_PER	200
>  #define LRADC_DELAY_TIMER_LOOP	5
>  
> +/*
> + * Once the pen touches the touchscreen, the touchscreen switches from
> + * IRQ-driven mode to polling mode to prevent interrupt storm. The polling
> + * is realized by worker thread, which is called every 20 or so milliseconds.
> + * This gives the touchscreen enough fluence and does not strain the system
> + * too much.
> + */
> +#define LRADC_TS_SAMPLE_DELAY_MS	5
> +
> +/*
> + * The LRADC reads the following amount of samples from each touchscreen
> + * channel and the driver then computes avarage of these.
> + */
> +#define LRADC_TS_SAMPLE_AMOUNT		4
> +
>  static const char * const mxs_lradc_irq_name[] = {
>  	"mxs-lradc-touchscreen",
>  	"mxs-lradc-thresh0",
> @@ -75,6 +92,12 @@ static const char * const mxs_lradc_irq_name[] = {
>  	"mxs-lradc-button1",
>  };
>  
> +enum mxs_lradc_ts {
> +	MXS_LRADC_TOUCHSCREEN_NONE = 0,
> +	MXS_LRADC_TOUCHSCREEN_4WIRE,
> +	MXS_LRADC_TOUCHSCREEN_5WIRE,
> +};
> +
>  struct mxs_lradc {
>  	struct device		*dev;
>  	void __iomem		*base;
> @@ -86,21 +109,69 @@ struct mxs_lradc {
>  	struct mutex		lock;
>  
>  	struct completion	completion;
> +
> +	/*
> +	 * Touchscreen LRADC channels receives a private slot in the CTRL4
> +	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
> +	 * CTRL4 register can be mapped to LRADC channels when using the
> +	 * touchscreen.
> +	 *
> +	 * Furthermore, certain LRADC channels are shared between touchscreen
> +	 * and/or touch-buttons and generic LRADC block. Therefore when using
> +	 * either of these, these channels are not available for the regular
> +	 * sampling. The shared channels are as follows:
> +	 *
> +	 * CH0 -- Touch button #0
> +	 * CH1 -- Touch button #1
> +	 * CH2 -- Touch screen XPUL
> +	 * CH3 -- Touch screen YPLL
> +	 * CH4 -- Touch screen XNUL
> +	 * CH5 -- Touch screen YNLR
> +	 * CH6 -- Touch screen WIPER (5-wire only)
> +	 *
> +	 * The bitfields below represents which parts of the LRADC block are
> +	 * switched into special mode of operation. These channels can not
> +	 * be sampled as regular LRADC channels. The driver will refuse any
> +	 * attempt to sample these channels.
> +	 */
> +#define CHAN_MASK_TOUCHBUTTON		(0x3 << 0)
> +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 2)
> +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 2)
> +	enum mxs_lradc_ts	use_touchscreen;
> +	unsigned int		stop_touchscreen:1;
> +	unsigned int		use_touchbutton:1;
> +
> +	struct input_dev	*ts_input;
> +	struct work_struct	ts_work;
>  };
>  
>  #define	LRADC_CTRL0				0x00
> -#define LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
> -#define LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
> +#define	LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
> +#define	LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
> +#define	LRADC_CTRL0_YNNSW	/* YM */	(1 << 21)
> +#define	LRADC_CTRL0_YPNSW	/* YP */	(1 << 20)
> +#define	LRADC_CTRL0_YPPSW	/* YP */	(1 << 19)
> +#define	LRADC_CTRL0_XNNSW	/* XM */	(1 << 18)
> +#define	LRADC_CTRL0_XNPSW	/* XM */	(1 << 17)
> +#define	LRADC_CTRL0_XPPSW	/* XP */	(1 << 16)
> +#define	LRADC_CTRL0_PLATE_MASK			(0x3f << 16)
>  
>  #define	LRADC_CTRL1				0x10
> -#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
> -#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
> +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ_EN		(1 << 24)
>  #define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
>  #define	LRADC_CTRL1_LRADC_IRQ_EN_MASK		(0x1fff << 16)
> +#define	LRADC_CTRL1_LRADC_IRQ_EN_OFFSET		16
> +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ		(1 << 8)
> +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
> +#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
> +#define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
>  
>  #define	LRADC_CTRL2				0x20
>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
>  
> +#define	LRADC_STATUS				0x40
> +#define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
> +
>  #define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
>  #define	LRADC_CH_ACCUMULATE			(1 << 29)
>  #define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
> @@ -132,6 +203,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  {
>  	struct mxs_lradc *lradc = iio_priv(iio_dev);
>  	int ret;
> +	unsigned long mask;
>  
>  	if (m != IIO_CHAN_INFO_RAW)
>  		return -EINVAL;
> @@ -140,6 +212,12 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	if (chan->channel > LRADC_MAX_TOTAL_CHANS)
>  		return -EINVAL;
>  
> +	/* Validate the channel if it doesn't intersect with reserved chans. */
> +	bitmap_set(&mask, chan->channel, 1);
> +	ret = iio_validate_scan_mask_onehot(iio_dev, &mask);
> +	if (ret)
> +		return -EINVAL;
> +
>  	/*
>  	 * See if there is no buffered operation in progess. If there is, simply
>  	 * bail out. This can be improved to support both buffered and raw IO at
> @@ -161,7 +239,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>  
> -	writel(chan->channel, lradc->base + LRADC_CTRL4);
> +	/* Clean the slot's previous content, then set new one. */
> +	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
> +		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> +	writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +
>  	writel(0, lradc->base + LRADC_CH(0));
>  
>  	/* Enable the IRQ and start sampling the channel. */
> @@ -195,6 +277,272 @@ static const struct iio_info mxs_lradc_iio_info = {
>  };
>  
>  /*
> + * Touchscreen handling
> + */
> +enum lradc_ts_plate {
> +	LRADC_SAMPLE_X,
> +	LRADC_SAMPLE_Y,
> +	LRADC_SAMPLE_PRESSURE,
> +};
> +
> +static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
> +{
> +	uint32_t reg;
> +
> +	/* Enable touch detection. */
> +	writel(LRADC_CTRL0_PLATE_MASK,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +	msleep(LRADC_TS_SAMPLE_DELAY_MS);
> +
> +	reg = readl(lradc->base + LRADC_STATUS);
> +
> +	return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
> +}
> +
> +static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
> +				enum lradc_ts_plate plate, int change)
> +{
> +	unsigned long delay, jiff;
> +	uint32_t reg, ctrl0 = 0, chan = 0;
> +	/* The touchscreen always uses CTRL4 slot #7. */
> +	const uint8_t slot = 7;
> +	uint32_t val;
> +
> +	/*
> +	 * There are three correct configurations of the controller sampling
> +	 * the touchscreen, each of these configuration provides different
> +	 * information from the touchscreen.
> +	 *
> +	 * The following table describes the sampling configurations:
> +	 * +-------------+-------+-------+-------+
> +	 * | Wire \ Axis |   X   |   Y   |   Z   |
> +	 * +---------------------+-------+-------+
> +	 * |   X+ (CH2)  |   HI  |   TS  |   TS  |
> +	 * +-------------+-------+-------+-------+
> +	 * |   X- (CH4)  |   LO  |   SH  |   HI  |
> +	 * +-------------+-------+-------+-------+
> +	 * |   Y+ (CH3)  |   SH  |   HI  |   HI  |
> +	 * +-------------+-------+-------+-------+
> +	 * |   Y- (CH5)  |   TS  |   LO  |   SH  |
> +	 * +-------------+-------+-------+-------+
> +	 *
> +	 * HI ... strong '1'  ; LO ... strong '0'
> +	 * SH ... sample here ; TS ... tri-state
> +	 *
> +	 * There are a few other ways of obtaining the Z coordinate
> +	 * (aka. pressure), but the one in the table seems to be the
> +	 * most reliable one.
> +	 */
> +	switch (plate) {
> +	case LRADC_SAMPLE_X:
> +		ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
> +		chan = 3;
> +		break;
> +	case LRADC_SAMPLE_Y:
> +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
> +		chan = 4;
> +		break;
> +	case LRADC_SAMPLE_PRESSURE:
> +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
> +		chan = 5;
> +		break;
> +	}
> +
> +	if (change) {
> +		writel(LRADC_CTRL0_PLATE_MASK,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +		writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +		writel(LRADC_CTRL4_LRADCSELECT_MASK(slot),
> +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> +		writel(chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
> +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +	}
> +
> +	writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
> +	writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +	delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
> +	do {
> +		jiff = jiffies;
> +		reg = readl_relaxed(lradc->base + LRADC_CTRL1);
> +		if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
> +			break;
> +	} while (time_before(jiff, delay));
> +
> +	writel(LRADC_CTRL1_LRADC_IRQ(slot),
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +
> +	if (time_after_eq(jiff, delay))
> +		return -ETIMEDOUT;
> +
> +	val = readl(lradc->base + LRADC_CH(slot));
> +	val &= LRADC_CH_VALUE_MASK;
> +
> +	return val;
> +}
> +
> +static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
> +				enum lradc_ts_plate plate)
> +{
> +	int32_t val, tot = 0;
> +	int i;
> +
> +	val = mxs_lradc_ts_sample(lradc, plate, 1);
> +
> +	/* Delay a bit so the touchscreen is stable. */
> +	mdelay(2);
> +
> +	for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
> +		val = mxs_lradc_ts_sample(lradc, plate, 0);
> +		tot += val;
> +	}
> +
> +	return tot / LRADC_TS_SAMPLE_AMOUNT;
> +}
> +
> +static void mxs_lradc_ts_work(struct work_struct *ts_work)
> +{
> +	struct mxs_lradc *lradc = container_of(ts_work,
> +				struct mxs_lradc, ts_work);
> +	int val_x, val_y, val_p;
> +	bool valid = false;
> +
> +	while (mxs_lradc_ts_touched(lradc)) {
> +		/* Disable touch detector so we can sample the touchscreen. */
> +		writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> +		if (likely(valid)) {
> +			input_report_abs(lradc->ts_input, ABS_X, val_x);
> +			input_report_abs(lradc->ts_input, ABS_Y, val_y);
> +			input_report_abs(lradc->ts_input, ABS_PRESSURE, val_p);
> +			input_report_key(lradc->ts_input, BTN_TOUCH, 1);
> +			input_sync(lradc->ts_input);
> +		}
> +
> +		valid = false;
> +
> +		val_x = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_X);
> +		if (val_x < 0)
> +			continue;
> +		val_y = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_Y);
> +		if (val_y < 0)
> +			continue;
> +		val_p = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_PRESSURE);
> +		if (val_p < 0)
> +			continue;
> +
> +		valid = true;
> +	}
> +
> +	input_report_abs(lradc->ts_input, ABS_PRESSURE, 0);
> +	input_report_key(lradc->ts_input, BTN_TOUCH, 0);
> +	input_sync(lradc->ts_input);
> +
> +	/* Do not restart the TS IRQ if the driver is shutting down. */
> +	if (lradc->stop_touchscreen)
> +		return;
> +
> +	/* Restart the touchscreen interrupts. */
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> +}
> +
> +static int mxs_lradc_ts_open(struct input_dev *dev)
> +{
> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> +
> +	/* The touchscreen is starting. */
> +	lradc->stop_touchscreen = 0;
> +
> +	/* Enable the touch-detect circuitry. */
> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +
> +	/* Enable the touch-detect IRQ. */
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> +
> +	return 0;
> +}
> +
> +static void mxs_lradc_ts_close(struct input_dev *dev)
> +{
> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> +
> +	/* Indicate the touchscreen is stopping. */
> +	lradc->stop_touchscreen = 1;
> +
> +	/* Disable touchscreen touch-detect IRQ. */
> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +
> +	/* Power-down touchscreen touch-detect circuitry. */
> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> +	/* Wait until touchscreen thread finishes any possible remnants. */
> +	cancel_work_sync(&lradc->ts_work);
> +}
> +
> +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> +{
> +	struct input_dev *input;
> +	struct device *dev = lradc->dev;
> +	int ret;
> +
> +	if (!lradc->use_touchscreen)
> +		return 0;
> +
> +	input = input_allocate_device();
> +	if (!input) {
> +		dev_warn(dev, "Failed to allocate TS device, disabling.\n");
> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
> +		return 0;
> +	}
> +
> +	input->name = DRIVER_NAME;
> +	input->id.bustype = BUS_HOST;
> +	input->dev.parent = dev;
> +	input->open = mxs_lradc_ts_open;
> +	input->close = mxs_lradc_ts_close;
> +
> +	__set_bit(EV_ABS, input->evbit);
> +	__set_bit(EV_KEY, input->evbit);
> +	__set_bit(BTN_TOUCH, input->keybit);
> +	input_set_abs_params(input, ABS_X, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +	input_set_abs_params(input, ABS_Y, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +	input_set_abs_params(input, ABS_PRESSURE, 0, LRADC_CH_VALUE_MASK, 0, 0);
> +
> +	lradc->ts_input = input;
> +	input_set_drvdata(input, lradc);
> +	ret = input_register_device(input);
> +	if (ret)
> +		input_free_device(lradc->ts_input);
> +
> +	return ret;
> +}
> +
> +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> +{
> +	if (!lradc->use_touchscreen)
> +		return;
> +
> +	if (!lradc->ts_input)
> +		return;
> +
> +	cancel_work_sync(&lradc->ts_work);
> +
> +	input_unregister_device(lradc->ts_input);
> +}
> +
> +/*
>   * IRQ Handling
>   */
>  static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> @@ -202,14 +550,24 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
>  	struct iio_dev *iio = data;
>  	struct mxs_lradc *lradc = iio_priv(iio);
>  	unsigned long reg = readl(lradc->base + LRADC_CTRL1);
> +	const uint32_t ts_irq_mask =
> +		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
> +		LRADC_CTRL1_TOUCH_DETECT_IRQ;
>  
>  	if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
>  		return IRQ_NONE;
>  
>  	/*
> -	 * Touchscreen IRQ handling code shall probably have priority
> -	 * and therefore shall be placed here.
> +	 * Touchscreen IRQ handling code has priority and therefore
> +	 * is placed here. In case touchscreen IRQ arrives, disable
> +	 * it ASAP
>  	 */
> +	if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
> +		writel(ts_irq_mask,
> +			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> +		if (!lradc->stop_touchscreen)
> +			schedule_work(&lradc->ts_work);
> +	}
>  
>  	if (iio_buffer_enabled(iio))
>  		iio_trigger_poll(iio->trig, iio_get_time_ns());
> @@ -306,8 +664,10 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
>  {
>  	struct mxs_lradc *lradc = iio_priv(iio);
>  	struct iio_buffer *buffer = iio->buffer;
> -	int ret = 0, chan, ofs = 0, enable = 0;
> -	uint32_t ctrl4 = 0;
> +	int ret = 0, chan, ofs = 0;
> +	unsigned long enable = 0;
> +	uint32_t ctrl4_set = 0;
> +	uint32_t ctrl4_clr = 0;
>  	uint32_t ctrl1_irq = 0;
>  	const uint32_t chan_value = LRADC_CH_ACCUMULATE |
>  		((LRADC_DELAY_TIMER_LOOP - 1) << LRADC_CH_NUM_SAMPLES_OFFSET);
> @@ -339,17 +699,20 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>  
>  	for_each_set_bit(chan, buffer->scan_mask, LRADC_MAX_TOTAL_CHANS) {
> -		ctrl4 |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> +		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> +		ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
>  		ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
>  		writel(chan_value, lradc->base + LRADC_CH(ofs));
> -		enable |= 1 << ofs;
> +		bitmap_set(&enable, ofs, 1);
>  		ofs++;
>  	};
>  
>  	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
>  		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
>  
> -	writel(ctrl4, lradc->base + LRADC_CTRL4);
> +	writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> +	writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> +
>  	writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
>  
>  	writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
> @@ -384,9 +747,33 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
>  static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
>  					const unsigned long *mask)
>  {
> -	const int mw = bitmap_weight(mask, iio->masklength);
> -
> -	return mw <= LRADC_MAX_MAPPED_CHANS;
> +	struct mxs_lradc *lradc = iio_priv(iio);
> +	const int len = iio->masklength;
> +	const int map_chans = bitmap_weight(mask, len);
> +	int rsvd_chans = 0;
> +	unsigned long rsvd_mask = 0;
> +
> +	if (lradc->use_touchbutton)
> +		rsvd_mask |= CHAN_MASK_TOUCHBUTTON;
> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_4WIRE)
> +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_4WIRE;
> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
> +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_5WIRE;
> +
> +	if (lradc->use_touchbutton)
> +		rsvd_chans++;
> +	if (lradc->use_touchscreen)
> +		rsvd_chans++;
> +
> +	/* Test for attempts to map channels with special mode of operation. */
> +	if (bitmap_intersects(mask, &rsvd_mask, len))
> +		return 0;
> +
> +	/* Test for attempts to map more channels then available slots. */
> +	if (map_chans + rsvd_chans > LRADC_MAX_MAPPED_CHANS)
> +		return 0;
> +
> +	return 1;
>  }
>  
>  static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
> @@ -435,15 +822,29 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
>  
>  static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
>  {
> -	int i;
> -	const uint32_t cfg =
> +	/* The ADC always uses DELAY CHANNEL 0. */
> +	const uint32_t adc_cfg =
> +		(1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + 0)) |
>  		(LRADC_DELAY_TIMER_PER << LRADC_DELAY_DELAY_OFFSET);
>  
>  	stmp_reset_block(lradc->base);
>  
> -	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> -		writel(cfg | (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)),
> -			lradc->base + LRADC_DELAY(i));
> +	/* Configure DELAY CHANNEL 0 for generic ADC sampling. */
> +	writel(adc_cfg, lradc->base + LRADC_DELAY(0));
> +
> +	/* Disable remaining DELAY CHANNELs */
> +	writel(0, lradc->base + LRADC_DELAY(1));
> +	writel(0, lradc->base + LRADC_DELAY(2));
> +	writel(0, lradc->base + LRADC_DELAY(3));
> +
> +	/* Configure the touchscreen type */
> +	writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +
> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
> +		writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +	}
>  
>  	/* Start internal temperature sensing. */
>  	writel(0, lradc->base + LRADC_CTRL2);
> @@ -463,9 +864,11 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
>  static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
>  	struct mxs_lradc *lradc;
>  	struct iio_dev *iio;
>  	struct resource *iores;
> +	uint32_t ts_wires = 0;
>  	int ret = 0;
>  	int i;
>  
> @@ -487,6 +890,21 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>  		goto err_addr;
>  	}
>  
> +	INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
> +
> +	/* Check if touchscreen is enabled in DT. */
> +	ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
> +				&ts_wires);
> +	if (ret)
> +		dev_info(dev, "Touchscreen not enabled.\n");
> +	else if (ts_wires == 4)
> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_4WIRE;
> +	else if (ts_wires == 5)
> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_5WIRE;
> +	else
> +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
> +				ts_wires);
> +
>  	/* Grab all IRQ sources */
>  	for (i = 0; i < 13; i++) {
>  		lradc->irq[i] = platform_get_irq(pdev, i);
> @@ -524,11 +942,16 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_trig;
>  
> +	/* Register the touchscreen input device. */
> +	ret = mxs_lradc_ts_register(lradc);
> +	if (ret)
> +		goto err_dev;
> +
>  	/* Register IIO device. */
>  	ret = iio_device_register(iio);
>  	if (ret) {
>  		dev_err(dev, "Failed to register IIO device\n");
> -		goto err_dev;
> +		goto err_ts;
>  	}
>  
>  	/* Configure the hardware. */
> @@ -536,6 +959,8 @@ static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> +err_ts:
> +	mxs_lradc_ts_unregister(lradc);
>  err_dev:
>  	mxs_lradc_trigger_remove(iio);
>  err_trig:
> @@ -550,6 +975,8 @@ static int __devexit mxs_lradc_remove(struct platform_device *pdev)
>  	struct iio_dev *iio = platform_get_drvdata(pdev);
>  	struct mxs_lradc *lradc = iio_priv(iio);
>  
> +	mxs_lradc_ts_unregister(lradc);
> +
>  	mxs_lradc_hw_stop(lradc);
>  
>  	iio_device_unregister(iio);
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-19 17:01   ` Marek Vasut
@ 2012-12-27 12:16     ` Jonathan Cameron
  2013-01-08 19:28       ` Marek Vasut
  2013-01-10  0:57     ` Dmitry Torokhov
  1 sibling, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2012-12-27 12:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/19/2012 05:01 PM, Marek Vasut wrote:
> Ccing Dmitry.
> 
> Also, Jonathan, can I get your review please?
Dratt caught me hiding from this one and hoping it would
go away ;)
> 
>> This patch implements support for sampling of a touchscreen into
>> the MXS LRADC driver. The LRADC block allows configuring some of
>> it's channels into special mode where they either output the drive
>> voltage or sample it, allowing it to operate a 4-wire or 5-wire
>> resistive touchscreen.
>>
>> In case the touchscreen mode is enabled, the LRADC slot #7 is
>> reserved for touchscreen only, therefore it is not possible to
>> sample 8 LRADC channels at time, but only 7 channels.
>>
>> The touchscreen controller is configured such that the PENDOWN event
>> disables touchscreen interrupts and triggers execution of worker
>> thread, which then polls the touchscreen controller for X, Y and
>> Pressure values. This reduces the overhead of interrupt-driven
>> operation. Upon the PENUP event, the worker thread re-enables the
>> PENDOWN detection interrupt and exits.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> Cc: Jonathan Cameron <jic23@kernel.org>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> ---
>>  .../bindings/staging/iio/adc/mxs-lradc.txt         |    6 +
>>  drivers/staging/iio/adc/mxs-lradc.c                |  469
>> +++++++++++++++++++- 2 files changed, 454 insertions(+), 21 deletions(-)
>>
>> V2: - Replace the use_touchscreen* with enum mxs_lradc_ts, which now tracks
>>       touchscreen state (OFF, 4-wire, 5-wire)
>>     - Add "stop_touchscreen" bit, which indicates the touchscreen is going
>> down and makes the driver not re-enable touchscreen interrupts and start
>> any new touchscreen works.
>>     - Make all bitfields "unsigned int" instead of plain "int"
>>     - Use switch(plate) in mxs_lradc_ts_sample()
>>     - Cancel touchscreen thread in mxs_lradc_ts_close(), do this only after
>>       we are sure touchscreen interrupt is off and will never be
>> re-enabled. This avoids serious race condition.
>>     - Call input_free_device() if input_register_device() fails to avoid
>> memory leak.
>>     - Do not call input_free_device() after input_unregister_device() in
>>       mxs_lradc_ts_unregister() to avoid double-free
>>
>> diff --git
>> a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
>> b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt index
>> 801d58c..4688205 100644
>> --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
>> +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
>> @@ -5,6 +5,12 @@ Required properties:
>>  - reg: Address and length of the register set for the device
>>  - interrupts: Should contain the LRADC interrupts
>>
>> +Optional properties:
>> +- fsl,lradc-touchscreen-wires: Number of wires used to connect the
>> touchscreen +                               to LRADC. Valid value is
>> either 4 or 5. If this +                               property is not
>> present, then the touchscreen is +                               disabled.
>> +
>>  Examples:
>>
>>  	lradc at 80050000 {
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>> b/drivers/staging/iio/adc/mxs-lradc.c index 6249957..52317ef 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -32,6 +32,8 @@
>>  #include <linux/stmp_device.h>
>>  #include <linux/bitops.h>
>>  #include <linux/completion.h>
>> +#include <linux/delay.h>
>> +#include <linux/input.h>
>>
>>  #include <mach/mxs.h>
>>  #include <mach/common.h>
>> @@ -59,6 +61,21 @@
>>  #define LRADC_DELAY_TIMER_PER	200
>>  #define LRADC_DELAY_TIMER_LOOP	5
>>
>> +/*
>> + * Once the pen touches the touchscreen, the touchscreen switches from
>> + * IRQ-driven mode to polling mode to prevent interrupt storm. The polling
>> + * is realized by worker thread, which is called every 20 or so
>> milliseconds. + * This gives the touchscreen enough fluence and does not
>> strain the system + * too much.
>> + */
>> +#define LRADC_TS_SAMPLE_DELAY_MS	5
>> +
>> +/*
>> + * The LRADC reads the following amount of samples from each touchscreen
>> + * channel and the driver then computes avarage of these.
>> + */
>> +#define LRADC_TS_SAMPLE_AMOUNT		4
>> +
>>  static const char * const mxs_lradc_irq_name[] = {
>>  	"mxs-lradc-touchscreen",
>>  	"mxs-lradc-thresh0",
>> @@ -75,6 +92,12 @@ static const char * const mxs_lradc_irq_name[] = {
>>  	"mxs-lradc-button1",
>>  };
>>
>> +enum mxs_lradc_ts {
>> +	MXS_LRADC_TOUCHSCREEN_NONE = 0,
>> +	MXS_LRADC_TOUCHSCREEN_4WIRE,
>> +	MXS_LRADC_TOUCHSCREEN_5WIRE,
>> +};
>> +
>>  struct mxs_lradc {
>>  	struct device		*dev;
>>  	void __iomem		*base;
>> @@ -86,21 +109,69 @@ struct mxs_lradc {
>>  	struct mutex		lock;
>>
>>  	struct completion	completion;
>> +
>> +	/*
>> +	 * Touchscreen LRADC channels receives a private slot in the CTRL4
>> +	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
>> +	 * CTRL4 register can be mapped to LRADC channels when using the
>> +	 * touchscreen.
>> +	 *
>> +	 * Furthermore, certain LRADC channels are shared between touchscreen
>> +	 * and/or touch-buttons and generic LRADC block. Therefore when using
>> +	 * either of these, these channels are not available for the regular
>> +	 * sampling. The shared channels are as follows:
>> +	 *
>> +	 * CH0 -- Touch button #0
>> +	 * CH1 -- Touch button #1
>> +	 * CH2 -- Touch screen XPUL
>> +	 * CH3 -- Touch screen YPLL
>> +	 * CH4 -- Touch screen XNUL
>> +	 * CH5 -- Touch screen YNLR
>> +	 * CH6 -- Touch screen WIPER (5-wire only)
>> +	 *
>> +	 * The bitfields below represents which parts of the LRADC block are
>> +	 * switched into special mode of operation. These channels can not
>> +	 * be sampled as regular LRADC channels. The driver will refuse any
>> +	 * attempt to sample these channels.
>> +	 */
>> +#define CHAN_MASK_TOUCHBUTTON		(0x3 << 0)
>> +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 2)
>> +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 2)
>> +	enum mxs_lradc_ts	use_touchscreen;
>> +	unsigned int		stop_touchscreen:1;
>> +	unsigned int		use_touchbutton:1;
>> +
>> +	struct input_dev	*ts_input;
>> +	struct work_struct	ts_work;
>>  };
>>
>>  #define	LRADC_CTRL0				0x00
>> -#define LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
>> -#define LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
>> +#define	LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
>> +#define	LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
>> +#define	LRADC_CTRL0_YNNSW	/* YM */	(1 << 21)
>> +#define	LRADC_CTRL0_YPNSW	/* YP */	(1 << 20)
>> +#define	LRADC_CTRL0_YPPSW	/* YP */	(1 << 19)
>> +#define	LRADC_CTRL0_XNNSW	/* XM */	(1 << 18)
>> +#define	LRADC_CTRL0_XNPSW	/* XM */	(1 << 17)
>> +#define	LRADC_CTRL0_XPPSW	/* XP */	(1 << 16)
>> +#define	LRADC_CTRL0_PLATE_MASK			(0x3f << 16)
>>
>>  #define	LRADC_CTRL1				0x10
>> -#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
>> -#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
>> +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ_EN		(1 << 24)
>>  #define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
>>  #define	LRADC_CTRL1_LRADC_IRQ_EN_MASK		(0x1fff << 16)
>> +#define	LRADC_CTRL1_LRADC_IRQ_EN_OFFSET		16
>> +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ		(1 << 8)
>> +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
>> +#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
>> +#define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
>>
>>  #define	LRADC_CTRL2				0x20
>>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
>>
>> +#define	LRADC_STATUS				0x40
>> +#define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
>> +
>>  #define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
>>  #define	LRADC_CH_ACCUMULATE			(1 << 29)
>>  #define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
>> @@ -132,6 +203,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>  {
>>  	struct mxs_lradc *lradc = iio_priv(iio_dev);
>>  	int ret;
>> +	unsigned long mask;
>>
>>  	if (m != IIO_CHAN_INFO_RAW)
>>  		return -EINVAL;
>> @@ -140,6 +212,12 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>  	if (chan->channel > LRADC_MAX_TOTAL_CHANS)
>>  		return -EINVAL;
>>
>> +	/* Validate the channel if it doesn't intersect with reserved chans. */
>> +	bitmap_set(&mask, chan->channel, 1);
>> +	ret = iio_validate_scan_mask_onehot(iio_dev, &mask);
>> +	if (ret)
>> +		return -EINVAL;
>> +
>>  	/*
>>  	 * See if there is no buffered operation in progess. If there is, simply
>>  	 * bail out. This can be improved to support both buffered and raw IO at
>> @@ -161,7 +239,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>>
>> -	writel(chan->channel, lradc->base + LRADC_CTRL4);
>> +	/* Clean the slot's previous content, then set new one. */
>> +	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
>> +		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
>> +	writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
>> +
>>  	writel(0, lradc->base + LRADC_CH(0));
>>
>>  	/* Enable the IRQ and start sampling the channel. */
>> @@ -195,6 +277,272 @@ static const struct iio_info mxs_lradc_iio_info = {
>>  };
>>
>>  /*
>> + * Touchscreen handling
>> + */
>> +enum lradc_ts_plate {
>> +	LRADC_SAMPLE_X,
>> +	LRADC_SAMPLE_Y,
>> +	LRADC_SAMPLE_PRESSURE,
>> +};
>> +
>> +static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
>> +{
>> +	uint32_t reg;
>> +
>> +	/* Enable touch detection. */
>> +	writel(LRADC_CTRL0_PLATE_MASK,
>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
>> +
>> +	msleep(LRADC_TS_SAMPLE_DELAY_MS);
>> +
>> +	reg = readl(lradc->base + LRADC_STATUS);
>> +
>> +	return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
>> +}
>> +
>> +static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
>> +				enum lradc_ts_plate plate, int change)
>> +{
>> +	unsigned long delay, jiff;
>> +	uint32_t reg, ctrl0 = 0, chan = 0;
>> +	/* The touchscreen always uses CTRL4 slot #7. */
>> +	const uint8_t slot = 7;
>> +	uint32_t val;
>> +
>> +	/*
>> +	 * There are three correct configurations of the controller sampling
>> +	 * the touchscreen, each of these configuration provides different
>> +	 * information from the touchscreen.
>> +	 *
>> +	 * The following table describes the sampling configurations:
>> +	 * +-------------+-------+-------+-------+
>> +	 * | Wire \ Axis |   X   |   Y   |   Z   |
>> +	 * +---------------------+-------+-------+
>> +	 * |   X+ (CH2)  |   HI  |   TS  |   TS  |
>> +	 * +-------------+-------+-------+-------+
>> +	 * |   X- (CH4)  |   LO  |   SH  |   HI  |
>> +	 * +-------------+-------+-------+-------+
>> +	 * |   Y+ (CH3)  |   SH  |   HI  |   HI  |
>> +	 * +-------------+-------+-------+-------+
>> +	 * |   Y- (CH5)  |   TS  |   LO  |   SH  |
>> +	 * +-------------+-------+-------+-------+
>> +	 *
>> +	 * HI ... strong '1'  ; LO ... strong '0'
>> +	 * SH ... sample here ; TS ... tri-state
>> +	 *
>> +	 * There are a few other ways of obtaining the Z coordinate
>> +	 * (aka. pressure), but the one in the table seems to be the
>> +	 * most reliable one.
>> +	 */
>> +	switch (plate) {
>> +	case LRADC_SAMPLE_X:
>> +		ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
>> +		chan = 3;
>> +		break;
>> +	case LRADC_SAMPLE_Y:
>> +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
>> +		chan = 4;
>> +		break;
>> +	case LRADC_SAMPLE_PRESSURE:
>> +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
>> +		chan = 5;
>> +		break;
>> +	}
>> +
>> +	if (change) {
>> +		writel(LRADC_CTRL0_PLATE_MASK,
>> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>> +		writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
>> +
>> +		writel(LRADC_CTRL4_LRADCSELECT_MASK(slot),
>> +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
>> +		writel(chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
>> +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
>> +	}
>> +
>> +	writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
>> +	writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
>> +
>> +	delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
>> +	do {
>> +		jiff = jiffies;
>> +		reg = readl_relaxed(lradc->base + LRADC_CTRL1);
>> +		if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
>> +			break;
>> +	} while (time_before(jiff, delay));
>> +
>> +	writel(LRADC_CTRL1_LRADC_IRQ(slot),
>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>> +
>> +	if (time_after_eq(jiff, delay))
>> +		return -ETIMEDOUT;
>> +
>> +	val = readl(lradc->base + LRADC_CH(slot));
>> +	val &= LRADC_CH_VALUE_MASK;
>> +
>> +	return val;
>> +}
>> +
>> +static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
>> +				enum lradc_ts_plate plate)
>> +{
>> +	int32_t val, tot = 0;
>> +	int i;
>> +
>> +	val = mxs_lradc_ts_sample(lradc, plate, 1);
>> +
>> +	/* Delay a bit so the touchscreen is stable. */
>> +	mdelay(2);
>> +
>> +	for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
>> +		val = mxs_lradc_ts_sample(lradc, plate, 0);
>> +		tot += val;
>> +	}
>> +
>> +	return tot / LRADC_TS_SAMPLE_AMOUNT;
>> +}
>> +
>> +static void mxs_lradc_ts_work(struct work_struct *ts_work)
>> +{
>> +	struct mxs_lradc *lradc = container_of(ts_work,
>> +				struct mxs_lradc, ts_work);
>> +	int val_x, val_y, val_p;
>> +	bool valid = false;
>> +
>> +	while (mxs_lradc_ts_touched(lradc)) {
>> +		/* Disable touch detector so we can sample the touchscreen. */
>> +		writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
>> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>> +
>> +		if (likely(valid)) {
>> +			input_report_abs(lradc->ts_input, ABS_X, val_x);
>> +			input_report_abs(lradc->ts_input, ABS_Y, val_y);
>> +			input_report_abs(lradc->ts_input, ABS_PRESSURE, val_p);
>> +			input_report_key(lradc->ts_input, BTN_TOUCH, 1);
>> +			input_sync(lradc->ts_input);
>> +		}
>> +
>> +		valid = false;
>> +
>> +		val_x = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_X);
>> +		if (val_x < 0)
>> +			continue;
>> +		val_y = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_Y);
>> +		if (val_y < 0)
>> +			continue;
>> +		val_p = mxs_lradc_ts_sample_filter(lradc, 
> LRADC_SAMPLE_PRESSURE);
>> +		if (val_p < 0)
>> +			continue;
>> +
>> +		valid = true;
>> +	}
>> +
>> +	input_report_abs(lradc->ts_input, ABS_PRESSURE, 0);
>> +	input_report_key(lradc->ts_input, BTN_TOUCH, 0);
>> +	input_sync(lradc->ts_input);
>> +
>> +	/* Do not restart the TS IRQ if the driver is shutting down. */
>> +	if (lradc->stop_touchscreen)
>> +		return;
>> +
>> +	/* Restart the touchscreen interrupts. */
>> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
>> +}
>> +
>> +static int mxs_lradc_ts_open(struct input_dev *dev)
>> +{
>> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
>> +
>> +	/* The touchscreen is starting. */
>> +	lradc->stop_touchscreen = 0;
>> +
>> +	/* Enable the touch-detect circuitry. */
>> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
>> +
>> +	/* Enable the touch-detect IRQ. */
>> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
>> +
>> +	return 0;
>> +}
>> +
>> +static void mxs_lradc_ts_close(struct input_dev *dev)
>> +{
>> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
>> +
>> +	/* Indicate the touchscreen is stopping. */
>> +	lradc->stop_touchscreen = 1;
>> +
>> +	/* Disable touchscreen touch-detect IRQ. */
>> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>> +
>> +	/* Power-down touchscreen touch-detect circuitry. */
>> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>> +
>> +	/* Wait until touchscreen thread finishes any possible remnants. */
>> +	cancel_work_sync(&lradc->ts_work);
>> +}
>> +
>> +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
>> +{
>> +	struct input_dev *input;
>> +	struct device *dev = lradc->dev;
>> +	int ret;
>> +
>> +	if (!lradc->use_touchscreen)
>> +		return 0;
>> +
>> +	input = input_allocate_device();
>> +	if (!input) {
>> +		dev_warn(dev, "Failed to allocate TS device, disabling.\n");
>> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
>> +		return 0;
>> +	}
>> +
>> +	input->name = DRIVER_NAME;
>> +	input->id.bustype = BUS_HOST;
>> +	input->dev.parent = dev;
>> +	input->open = mxs_lradc_ts_open;
>> +	input->close = mxs_lradc_ts_close;
>> +
>> +	__set_bit(EV_ABS, input->evbit);
>> +	__set_bit(EV_KEY, input->evbit);
>> +	__set_bit(BTN_TOUCH, input->keybit);
>> +	input_set_abs_params(input, ABS_X, 0, LRADC_CH_VALUE_MASK, 0, 0);
>> +	input_set_abs_params(input, ABS_Y, 0, LRADC_CH_VALUE_MASK, 0, 0);
>> +	input_set_abs_params(input, ABS_PRESSURE, 0, LRADC_CH_VALUE_MASK, 0, 0);
>> +
>> +	lradc->ts_input = input;
>> +	input_set_drvdata(input, lradc);
>> +	ret = input_register_device(input);
>> +	if (ret)
>> +		input_free_device(lradc->ts_input);
>> +
>> +	return ret;
>> +}
>> +
>> +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
>> +{
>> +	if (!lradc->use_touchscreen)
>> +		return;
>> +
>> +	if (!lradc->ts_input)
>> +		return;
>> +
>> +	cancel_work_sync(&lradc->ts_work);
>> +
>> +	input_unregister_device(lradc->ts_input);
>> +}
>> +
>> +/*
>>   * IRQ Handling
>>   */
>>  static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
>> @@ -202,14 +550,24 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void
>> *data) struct iio_dev *iio = data;
>>  	struct mxs_lradc *lradc = iio_priv(iio);
>>  	unsigned long reg = readl(lradc->base + LRADC_CTRL1);
>> +	const uint32_t ts_irq_mask =
>> +		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
>> +		LRADC_CTRL1_TOUCH_DETECT_IRQ;
>>
>>  	if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
>>  		return IRQ_NONE;
>>
>>  	/*
>> -	 * Touchscreen IRQ handling code shall probably have priority
>> -	 * and therefore shall be placed here.
>> +	 * Touchscreen IRQ handling code has priority and therefore
>> +	 * is placed here. In case touchscreen IRQ arrives, disable
>> +	 * it ASAP
>>  	 */
>> +	if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
>> +		writel(ts_irq_mask,
>> +			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>> +		if (!lradc->stop_touchscreen)
>> +			schedule_work(&lradc->ts_work);
>> +	}
>>
>>  	if (iio_buffer_enabled(iio))
>>  		iio_trigger_poll(iio->trig, iio_get_time_ns());
>> @@ -306,8 +664,10 @@ static int mxs_lradc_buffer_preenable(struct iio_dev
>> *iio) {
>>  	struct mxs_lradc *lradc = iio_priv(iio);
>>  	struct iio_buffer *buffer = iio->buffer;
>> -	int ret = 0, chan, ofs = 0, enable = 0;
>> -	uint32_t ctrl4 = 0;
>> +	int ret = 0, chan, ofs = 0;
>> +	unsigned long enable = 0;
>> +	uint32_t ctrl4_set = 0;
>> +	uint32_t ctrl4_clr = 0;
>>  	uint32_t ctrl1_irq = 0;
>>  	const uint32_t chan_value = LRADC_CH_ACCUMULATE |
>>  		((LRADC_DELAY_TIMER_LOOP - 1) << LRADC_CH_NUM_SAMPLES_OFFSET);
>> @@ -339,17 +699,20 @@ static int mxs_lradc_buffer_preenable(struct iio_dev
>> *iio) writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>>
>>  	for_each_set_bit(chan, buffer->scan_mask, LRADC_MAX_TOTAL_CHANS) {
>> -		ctrl4 |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
>> +		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
>> +		ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
>>  		ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
>>  		writel(chan_value, lradc->base + LRADC_CH(ofs));
>> -		enable |= 1 << ofs;
>> +		bitmap_set(&enable, ofs, 1);
>>  		ofs++;
>>  	};
>>
>>  	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
>>  		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
>>
>> -	writel(ctrl4, lradc->base + LRADC_CTRL4);
>> +	writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
>> +	writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
>> +
>>  	writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
>>
>>  	writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
>> @@ -384,9 +747,33 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev
>> *iio) static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
>>  					const unsigned long *mask)
>>  {
>> -	const int mw = bitmap_weight(mask, iio->masklength);
>> -
>> -	return mw <= LRADC_MAX_MAPPED_CHANS;
>> +	struct mxs_lradc *lradc = iio_priv(iio);
>> +	const int len = iio->masklength;
>> +	const int map_chans = bitmap_weight(mask, len);
>> +	int rsvd_chans = 0;
>> +	unsigned long rsvd_mask = 0;
>> +
>> +	if (lradc->use_touchbutton)
>> +		rsvd_mask |= CHAN_MASK_TOUCHBUTTON;
>> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_4WIRE)
>> +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_4WIRE;
>> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
>> +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_5WIRE;
>> +
>> +	if (lradc->use_touchbutton)
>> +		rsvd_chans++;
>> +	if (lradc->use_touchscreen)
>> +		rsvd_chans++;
>> +
>> +	/* Test for attempts to map channels with special mode of operation. */
>> +	if (bitmap_intersects(mask, &rsvd_mask, len))
>> +		return 0;
>> +
>> +	/* Test for attempts to map more channels then available slots. */
>> +	if (map_chans + rsvd_chans > LRADC_MAX_MAPPED_CHANS)
>> +		return 0;
>> +
>> +	return 1;
>>  }
>>
>>  static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>> @@ -435,15 +822,29 @@ static const struct iio_chan_spec
>> mxs_lradc_chan_spec[] = {
>>
>>  static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
>>  {
>> -	int i;
>> -	const uint32_t cfg =
>> +	/* The ADC always uses DELAY CHANNEL 0. */
>> +	const uint32_t adc_cfg =
>> +		(1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + 0)) |
>>  		(LRADC_DELAY_TIMER_PER << LRADC_DELAY_DELAY_OFFSET);
>>
>>  	stmp_reset_block(lradc->base);
>>
>> -	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
>> -		writel(cfg | (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)),
>> -			lradc->base + LRADC_DELAY(i));
>> +	/* Configure DELAY CHANNEL 0 for generic ADC sampling. */
>> +	writel(adc_cfg, lradc->base + LRADC_DELAY(0));
>> +
>> +	/* Disable remaining DELAY CHANNELs */
>> +	writel(0, lradc->base + LRADC_DELAY(1));
>> +	writel(0, lradc->base + LRADC_DELAY(2));
>> +	writel(0, lradc->base + LRADC_DELAY(3));
>> +
>> +	/* Configure the touchscreen type */
>> +	writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>> +
>> +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
>> +		writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
>> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
>> +	}
>>
>>  	/* Start internal temperature sensing. */
>>  	writel(0, lradc->base + LRADC_CTRL2);
>> @@ -463,9 +864,11 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
>>  static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> +	struct device_node *node = dev->of_node;
>>  	struct mxs_lradc *lradc;
>>  	struct iio_dev *iio;
>>  	struct resource *iores;
>> +	uint32_t ts_wires = 0;
>>  	int ret = 0;
>>  	int i;
>>
>> @@ -487,6 +890,21 @@ static int __devinit mxs_lradc_probe(struct
>> platform_device *pdev) goto err_addr;
>>  	}
>>
>> +	INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
>> +
>> +	/* Check if touchscreen is enabled in DT. */
>> +	ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
>> +				&ts_wires);
>> +	if (ret)
>> +		dev_info(dev, "Touchscreen not enabled.\n");
>> +	else if (ts_wires == 4)
>> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_4WIRE;
>> +	else if (ts_wires == 5)
>> +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_5WIRE;
>> +	else
>> +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
>> +				ts_wires);
>> +
>>  	/* Grab all IRQ sources */
>>  	for (i = 0; i < 13; i++) {
>>  		lradc->irq[i] = platform_get_irq(pdev, i);
>> @@ -524,11 +942,16 @@ static int __devinit mxs_lradc_probe(struct
>> platform_device *pdev) if (ret)
>>  		goto err_trig;
>>
>> +	/* Register the touchscreen input device. */
>> +	ret = mxs_lradc_ts_register(lradc);
>> +	if (ret)
>> +		goto err_dev;
>> +
>>  	/* Register IIO device. */
>>  	ret = iio_device_register(iio);
>>  	if (ret) {
>>  		dev_err(dev, "Failed to register IIO device\n");
>> -		goto err_dev;
>> +		goto err_ts;
>>  	}
>>
>>  	/* Configure the hardware. */
>> @@ -536,6 +959,8 @@ static int __devinit mxs_lradc_probe(struct
>> platform_device *pdev)
>>
>>  	return 0;
>>
>> +err_ts:
>> +	mxs_lradc_ts_unregister(lradc);
>>  err_dev:
>>  	mxs_lradc_trigger_remove(iio);
>>  err_trig:
>> @@ -550,6 +975,8 @@ static int __devexit mxs_lradc_remove(struct
>> platform_device *pdev) struct iio_dev *iio = platform_get_drvdata(pdev);
>>  	struct mxs_lradc *lradc = iio_priv(iio);
>>
>> +	mxs_lradc_ts_unregister(lradc);
>> +
>>  	mxs_lradc_hw_stop(lradc);
>>
>>  	iio_device_unregister(iio);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-27 11:31   ` Jonathan Cameron
@ 2012-12-27 18:19     ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2012-12-27 18:19 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Jonathan Cameron,

> On 12/14/2012 01:46 AM, Marek Vasut wrote:
> > This patch implements support for sampling of a touchscreen into
> > the MXS LRADC driver. The LRADC block allows configuring some of
> > it's channels into special mode where they either output the drive
> > voltage or sample it, allowing it to operate a 4-wire or 5-wire
> > resistive touchscreen.
> > 
> > In case the touchscreen mode is enabled, the LRADC slot #7 is
> > reserved for touchscreen only, therefore it is not possible to
> > sample 8 LRADC channels at time, but only 7 channels.
> > 
> > The touchscreen controller is configured such that the PENDOWN event
> > disables touchscreen interrupts and triggers execution of worker
> > thread, which then polls the touchscreen controller for X, Y and
> > Pressure values. This reduces the overhead of interrupt-driven
> > operation. Upon the PENUP event, the worker thread re-enables the
> > PENDOWN detection interrupt and exits.
> 
> I still have a few reservations about this patch.  Firstly
> I would love to see a more generic approach to this as I outlined
> before.  Still we can't have everything we want sometimes :)
> Also, this is a lot of input related code in a driver that isn't
> in the input subsystem.  Really up to Dmitry on whether he is
> happy with this, or whether he insists on an mfd.
> 
> So all in all, with reservations I'll add this to the IIO tree
> IF Dmitry is happy with it and there are no other issues raised
> in the meantime.

Understood, I'll wait for Dmitry.

> Jonathan
> 
> p.s. If anyone has time, I'd also like to get this out of staging
> in the coming cycle.

[...]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-27 12:16     ` Jonathan Cameron
@ 2013-01-08 19:28       ` Marek Vasut
  2013-01-09 20:01         ` Jonathan Cameron
  0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2013-01-08 19:28 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Jonathan Cameron,

> On 12/19/2012 05:01 PM, Marek Vasut wrote:
> > Ccing Dmitry.
> > 
> > Also, Jonathan, can I get your review please?
> 
> Dratt caught me hiding from this one and hoping it would
> go away ;)

Still not going away :)

Did I miss your reply maybe?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-08 19:28       ` Marek Vasut
@ 2013-01-09 20:01         ` Jonathan Cameron
  2013-01-09 20:17           ` Marek Vasut
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2013-01-09 20:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/08/2013 07:28 PM, Marek Vasut wrote:
> Dear Jonathan Cameron,
>
>> On 12/19/2012 05:01 PM, Marek Vasut wrote:
>>> Ccing Dmitry.
>>>
>>> Also, Jonathan, can I get your review please?
>> Dratt caught me hiding from this one and hoping it would
>> go away ;)
> Still not going away :)
>
> Did I miss your reply maybe?
>
I certainly remember writing one!

http://marc.info/?l=linux-iio&m=135660790520224&w=2
looks about right...

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-09 20:01         ` Jonathan Cameron
@ 2013-01-09 20:17           ` Marek Vasut
  2013-01-09 21:38             ` Jonathan Cameron
  0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2013-01-09 20:17 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Jonathan Cameron,

> On 01/08/2013 07:28 PM, Marek Vasut wrote:
> > Dear Jonathan Cameron,
> > 
> >> On 12/19/2012 05:01 PM, Marek Vasut wrote:
> >>> Ccing Dmitry.
> >>> 
> >>> Also, Jonathan, can I get your review please?
> >> 
> >> Dratt caught me hiding from this one and hoping it would
> >> go away ;)
> > 
> > Still not going away :)
> > 
> > Did I miss your reply maybe?
> 
> I certainly remember writing one!
> 
> http://marc.info/?l=linux-iio&m=135660790520224&w=2
> looks about right...

Damn, sorry. I don't see it in my mailer so maybe it got lost. Anyway, did I 
miss the reply from Dmitry as well?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-09 20:17           ` Marek Vasut
@ 2013-01-09 21:38             ` Jonathan Cameron
  2013-01-09 21:54               ` Marek Vasut
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2013-01-09 21:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/09/2013 08:17 PM, Marek Vasut wrote:
> Dear Jonathan Cameron,
> 
>> On 01/08/2013 07:28 PM, Marek Vasut wrote:
>>> Dear Jonathan Cameron,
>>>
>>>> On 12/19/2012 05:01 PM, Marek Vasut wrote:
>>>>> Ccing Dmitry.
>>>>>
>>>>> Also, Jonathan, can I get your review please?
>>>>
>>>> Dratt caught me hiding from this one and hoping it would
>>>> go away ;)
>>>
>>> Still not going away :)
>>>
>>> Did I miss your reply maybe?
>>
>> I certainly remember writing one!
>>
>> http://marc.info/?l=linux-iio&m=135660790520224&w=2
>> looks about right...
> 
> Damn, sorry. I don't see it in my mailer so maybe it got lost. Anyway, did I 
> miss the reply from Dmitry as well?
> 
If you did, then so did I!  Don't think we've had any comment from Dmitry on
this yet.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-09 21:38             ` Jonathan Cameron
@ 2013-01-09 21:54               ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2013-01-09 21:54 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Jonathan Cameron,

> On 01/09/2013 08:17 PM, Marek Vasut wrote:
> > Dear Jonathan Cameron,
> > 
> >> On 01/08/2013 07:28 PM, Marek Vasut wrote:
> >>> Dear Jonathan Cameron,
> >>> 
> >>>> On 12/19/2012 05:01 PM, Marek Vasut wrote:
> >>>>> Ccing Dmitry.
> >>>>> 
> >>>>> Also, Jonathan, can I get your review please?
> >>>> 
> >>>> Dratt caught me hiding from this one and hoping it would
> >>>> go away ;)
> >>> 
> >>> Still not going away :)
> >>> 
> >>> Did I miss your reply maybe?
> >> 
> >> I certainly remember writing one!
> >> 
> >> http://marc.info/?l=linux-iio&m=135660790520224&w=2
> >> looks about right...
> > 
> > Damn, sorry. I don't see it in my mailer so maybe it got lost. Anyway,
> > did I miss the reply from Dmitry as well?
> 
> If you did, then so did I!  Don't think we've had any comment from Dmitry
> on this yet.

Whew, then that's good. Dmitry, shall I repost maybe? I know I forgot to Cc you 
(stupid me).

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2012-12-19 17:01   ` Marek Vasut
  2012-12-27 12:16     ` Jonathan Cameron
@ 2013-01-10  0:57     ` Dmitry Torokhov
  2013-01-10  9:48       ` Marek Vasut
  1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2013-01-10  0:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marek,

On Wed, Dec 19, 2012 at 06:01:31PM +0100, Marek Vasut wrote:
> Ccing Dmitry.
> 
> Also, Jonathan, can I get your review please?
> 
> > This patch implements support for sampling of a touchscreen into
> > the MXS LRADC driver. The LRADC block allows configuring some of
> > it's channels into special mode where they either output the drive
> > voltage or sample it, allowing it to operate a 4-wire or 5-wire
> > resistive touchscreen.
> > 
> > In case the touchscreen mode is enabled, the LRADC slot #7 is
> > reserved for touchscreen only, therefore it is not possible to
> > sample 8 LRADC channels at time, but only 7 channels.
> > 
> > The touchscreen controller is configured such that the PENDOWN event
> > disables touchscreen interrupts and triggers execution of worker
> > thread, which then polls the touchscreen controller for X, Y and
> > Pressure values. This reduces the overhead of interrupt-driven
> > operation. Upon the PENUP event, the worker thread re-enables the
> > PENDOWN detection interrupt and exits.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  .../bindings/staging/iio/adc/mxs-lradc.txt         |    6 +
> >  drivers/staging/iio/adc/mxs-lradc.c                |  469
> > +++++++++++++++++++- 2 files changed, 454 insertions(+), 21 deletions(-)
> > 
> > V2: - Replace the use_touchscreen* with enum mxs_lradc_ts, which now tracks
> >       touchscreen state (OFF, 4-wire, 5-wire)
> >     - Add "stop_touchscreen" bit, which indicates the touchscreen is going
> > down and makes the driver not re-enable touchscreen interrupts and start
> > any new touchscreen works.
> >     - Make all bitfields "unsigned int" instead of plain "int"
> >     - Use switch(plate) in mxs_lradc_ts_sample()
> >     - Cancel touchscreen thread in mxs_lradc_ts_close(), do this only after
> >       we are sure touchscreen interrupt is off and will never be
> > re-enabled. This avoids serious race condition.
> >     - Call input_free_device() if input_register_device() fails to avoid
> > memory leak.
> >     - Do not call input_free_device() after input_unregister_device() in
> >       mxs_lradc_ts_unregister() to avoid double-free
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> > b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt index
> > 801d58c..4688205 100644
> > --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> > +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> > @@ -5,6 +5,12 @@ Required properties:
> >  - reg: Address and length of the register set for the device
> >  - interrupts: Should contain the LRADC interrupts
> > 
> > +Optional properties:
> > +- fsl,lradc-touchscreen-wires: Number of wires used to connect the
> > touchscreen +                               to LRADC. Valid value is
> > either 4 or 5. If this +                               property is not
> > present, then the touchscreen is +                               disabled.
> > +
> >  Examples:
> > 
> >  	lradc at 80050000 {
> > diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> > b/drivers/staging/iio/adc/mxs-lradc.c index 6249957..52317ef 100644
> > --- a/drivers/staging/iio/adc/mxs-lradc.c
> > +++ b/drivers/staging/iio/adc/mxs-lradc.c
> > @@ -32,6 +32,8 @@
> >  #include <linux/stmp_device.h>
> >  #include <linux/bitops.h>
> >  #include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/input.h>
> > 
> >  #include <mach/mxs.h>
> >  #include <mach/common.h>
> > @@ -59,6 +61,21 @@
> >  #define LRADC_DELAY_TIMER_PER	200
> >  #define LRADC_DELAY_TIMER_LOOP	5
> > 
> > +/*
> > + * Once the pen touches the touchscreen, the touchscreen switches from
> > + * IRQ-driven mode to polling mode to prevent interrupt storm. The polling
> > + * is realized by worker thread, which is called every 20 or so
> > milliseconds. + * This gives the touchscreen enough fluence and does not
> > strain the system + * too much.
> > + */
> > +#define LRADC_TS_SAMPLE_DELAY_MS	5
> > +
> > +/*
> > + * The LRADC reads the following amount of samples from each touchscreen
> > + * channel and the driver then computes avarage of these.
> > + */
> > +#define LRADC_TS_SAMPLE_AMOUNT		4
> > +
> >  static const char * const mxs_lradc_irq_name[] = {
> >  	"mxs-lradc-touchscreen",
> >  	"mxs-lradc-thresh0",
> > @@ -75,6 +92,12 @@ static const char * const mxs_lradc_irq_name[] = {
> >  	"mxs-lradc-button1",
> >  };
> > 
> > +enum mxs_lradc_ts {
> > +	MXS_LRADC_TOUCHSCREEN_NONE = 0,
> > +	MXS_LRADC_TOUCHSCREEN_4WIRE,
> > +	MXS_LRADC_TOUCHSCREEN_5WIRE,
> > +};
> > +
> >  struct mxs_lradc {
> >  	struct device		*dev;
> >  	void __iomem		*base;
> > @@ -86,21 +109,69 @@ struct mxs_lradc {
> >  	struct mutex		lock;
> > 
> >  	struct completion	completion;
> > +
> > +	/*
> > +	 * Touchscreen LRADC channels receives a private slot in the CTRL4
> > +	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
> > +	 * CTRL4 register can be mapped to LRADC channels when using the
> > +	 * touchscreen.
> > +	 *
> > +	 * Furthermore, certain LRADC channels are shared between touchscreen
> > +	 * and/or touch-buttons and generic LRADC block. Therefore when using
> > +	 * either of these, these channels are not available for the regular
> > +	 * sampling. The shared channels are as follows:
> > +	 *
> > +	 * CH0 -- Touch button #0
> > +	 * CH1 -- Touch button #1
> > +	 * CH2 -- Touch screen XPUL
> > +	 * CH3 -- Touch screen YPLL
> > +	 * CH4 -- Touch screen XNUL
> > +	 * CH5 -- Touch screen YNLR
> > +	 * CH6 -- Touch screen WIPER (5-wire only)
> > +	 *
> > +	 * The bitfields below represents which parts of the LRADC block are
> > +	 * switched into special mode of operation. These channels can not
> > +	 * be sampled as regular LRADC channels. The driver will refuse any
> > +	 * attempt to sample these channels.
> > +	 */
> > +#define CHAN_MASK_TOUCHBUTTON		(0x3 << 0)
> > +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 2)
> > +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 2)
> > +	enum mxs_lradc_ts	use_touchscreen;
> > +	unsigned int		stop_touchscreen:1;
> > +	unsigned int		use_touchbutton:1;

Can we make them bools instead of bit fields?

> > +
> > +	struct input_dev	*ts_input;
> > +	struct work_struct	ts_work;
> >  };
> > 
> >  #define	LRADC_CTRL0				0x00
> > -#define LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
> > -#define LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
> > +#define	LRADC_CTRL0_TOUCH_DETECT_ENABLE		(1 << 23)
> > +#define	LRADC_CTRL0_TOUCH_SCREEN_TYPE		(1 << 22)
> > +#define	LRADC_CTRL0_YNNSW	/* YM */	(1 << 21)
> > +#define	LRADC_CTRL0_YPNSW	/* YP */	(1 << 20)
> > +#define	LRADC_CTRL0_YPPSW	/* YP */	(1 << 19)
> > +#define	LRADC_CTRL0_XNNSW	/* XM */	(1 << 18)
> > +#define	LRADC_CTRL0_XNPSW	/* XM */	(1 << 17)
> > +#define	LRADC_CTRL0_XPPSW	/* XP */	(1 << 16)
> > +#define	LRADC_CTRL0_PLATE_MASK			(0x3f << 16)
> > 
> >  #define	LRADC_CTRL1				0x10
> > -#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
> > -#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
> > +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ_EN		(1 << 24)
> >  #define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
> >  #define	LRADC_CTRL1_LRADC_IRQ_EN_MASK		(0x1fff << 16)
> > +#define	LRADC_CTRL1_LRADC_IRQ_EN_OFFSET		16
> > +#define	LRADC_CTRL1_TOUCH_DETECT_IRQ		(1 << 8)
> > +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
> > +#define	LRADC_CTRL1_LRADC_IRQ_MASK		0x1fff
> > +#define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
> > 
> >  #define	LRADC_CTRL2				0x20
> >  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
> > 
> > +#define	LRADC_STATUS				0x40
> > +#define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
> > +
> >  #define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
> >  #define	LRADC_CH_ACCUMULATE			(1 << 29)
> >  #define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
> > @@ -132,6 +203,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> >  {
> >  	struct mxs_lradc *lradc = iio_priv(iio_dev);
> >  	int ret;
> > +	unsigned long mask;
> > 
> >  	if (m != IIO_CHAN_INFO_RAW)
> >  		return -EINVAL;
> > @@ -140,6 +212,12 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> >  	if (chan->channel > LRADC_MAX_TOTAL_CHANS)
> >  		return -EINVAL;
> > 
> > +	/* Validate the channel if it doesn't intersect with reserved chans. */
> > +	bitmap_set(&mask, chan->channel, 1);
> > +	ret = iio_validate_scan_mask_onehot(iio_dev, &mask);
> > +	if (ret)
> > +		return -EINVAL;
> > +
> >  	/*
> >  	 * See if there is no buffered operation in progess. If there is, simply
> >  	 * bail out. This can be improved to support both buffered and raw IO at
> > @@ -161,7 +239,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> >  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> >  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > 
> > -	writel(chan->channel, lradc->base + LRADC_CTRL4);
> > +	/* Clean the slot's previous content, then set new one. */
> > +	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
> > +		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> > +	writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> > +
> >  	writel(0, lradc->base + LRADC_CH(0));
> > 
> >  	/* Enable the IRQ and start sampling the channel. */
> > @@ -195,6 +277,272 @@ static const struct iio_info mxs_lradc_iio_info = {
> >  };
> > 
> >  /*
> > + * Touchscreen handling
> > + */
> > +enum lradc_ts_plate {
> > +	LRADC_SAMPLE_X,
> > +	LRADC_SAMPLE_Y,
> > +	LRADC_SAMPLE_PRESSURE,
> > +};
> > +
> > +static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
> > +{
> > +	uint32_t reg;
> > +
> > +	/* Enable touch detection. */
> > +	writel(LRADC_CTRL0_PLATE_MASK,
> > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> > +
> > +	msleep(LRADC_TS_SAMPLE_DELAY_MS);
> > +
> > +	reg = readl(lradc->base + LRADC_STATUS);
> > +
> > +	return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
> > +}
> > +
> > +static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
> > +				enum lradc_ts_plate plate, int change)
> > +{
> > +	unsigned long delay, jiff;
> > +	uint32_t reg, ctrl0 = 0, chan = 0;
> > +	/* The touchscreen always uses CTRL4 slot #7. */
> > +	const uint8_t slot = 7;
> > +	uint32_t val;
> > +
> > +	/*
> > +	 * There are three correct configurations of the controller sampling
> > +	 * the touchscreen, each of these configuration provides different
> > +	 * information from the touchscreen.
> > +	 *
> > +	 * The following table describes the sampling configurations:
> > +	 * +-------------+-------+-------+-------+
> > +	 * | Wire \ Axis |   X   |   Y   |   Z   |
> > +	 * +---------------------+-------+-------+
> > +	 * |   X+ (CH2)  |   HI  |   TS  |   TS  |
> > +	 * +-------------+-------+-------+-------+
> > +	 * |   X- (CH4)  |   LO  |   SH  |   HI  |
> > +	 * +-------------+-------+-------+-------+
> > +	 * |   Y+ (CH3)  |   SH  |   HI  |   HI  |
> > +	 * +-------------+-------+-------+-------+
> > +	 * |   Y- (CH5)  |   TS  |   LO  |   SH  |
> > +	 * +-------------+-------+-------+-------+
> > +	 *
> > +	 * HI ... strong '1'  ; LO ... strong '0'
> > +	 * SH ... sample here ; TS ... tri-state
> > +	 *
> > +	 * There are a few other ways of obtaining the Z coordinate
> > +	 * (aka. pressure), but the one in the table seems to be the
> > +	 * most reliable one.
> > +	 */
> > +	switch (plate) {
> > +	case LRADC_SAMPLE_X:
> > +		ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
> > +		chan = 3;
> > +		break;
> > +	case LRADC_SAMPLE_Y:
> > +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
> > +		chan = 4;
> > +		break;
> > +	case LRADC_SAMPLE_PRESSURE:
> > +		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
> > +		chan = 5;
> > +		break;
> > +	}
> > +
> > +	if (change) {
> > +		writel(LRADC_CTRL0_PLATE_MASK,
> > +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > +		writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> > +
> > +		writel(LRADC_CTRL4_LRADCSELECT_MASK(slot),
> > +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> > +		writel(chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
> > +			lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> > +	}
> > +
> > +	writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
> > +	writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> > +
> > +	delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
> > +	do {
> > +		jiff = jiffies;
> > +		reg = readl_relaxed(lradc->base + LRADC_CTRL1);
> > +		if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
> > +			break;
> > +	} while (time_before(jiff, delay));
> > +
> > +	writel(LRADC_CTRL1_LRADC_IRQ(slot),
> > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > +
> > +	if (time_after_eq(jiff, delay))
> > +		return -ETIMEDOUT;
> > +
> > +	val = readl(lradc->base + LRADC_CH(slot));
> > +	val &= LRADC_CH_VALUE_MASK;
> > +
> > +	return val;
> > +}
> > +
> > +static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
> > +				enum lradc_ts_plate plate)
> > +{
> > +	int32_t val, tot = 0;
> > +	int i;
> > +
> > +	val = mxs_lradc_ts_sample(lradc, plate, 1);
> > +
> > +	/* Delay a bit so the touchscreen is stable. */
> > +	mdelay(2);
> > +
> > +	for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
> > +		val = mxs_lradc_ts_sample(lradc, plate, 0);
> > +		tot += val;
> > +	}
> > +
> > +	return tot / LRADC_TS_SAMPLE_AMOUNT;
> > +}
> > +
> > +static void mxs_lradc_ts_work(struct work_struct *ts_work)
> > +{
> > +	struct mxs_lradc *lradc = container_of(ts_work,
> > +				struct mxs_lradc, ts_work);
> > +	int val_x, val_y, val_p;
> > +	bool valid = false;
> > +
> > +	while (mxs_lradc_ts_touched(lradc)) {
> > +		/* Disable touch detector so we can sample the touchscreen. */
> > +		writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > +
> > +		if (likely(valid)) {
> > +			input_report_abs(lradc->ts_input, ABS_X, val_x);
> > +			input_report_abs(lradc->ts_input, ABS_Y, val_y);
> > +			input_report_abs(lradc->ts_input, ABS_PRESSURE, val_p);
> > +			input_report_key(lradc->ts_input, BTN_TOUCH, 1);
> > +			input_sync(lradc->ts_input);
> > +		}
> > +
> > +		valid = false;
> > +
> > +		val_x = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_X);
> > +		if (val_x < 0)
> > +			continue;
> > +		val_y = mxs_lradc_ts_sample_filter(lradc, LRADC_SAMPLE_Y);
> > +		if (val_y < 0)
> > +			continue;
> > +		val_p = mxs_lradc_ts_sample_filter(lradc, 
> LRADC_SAMPLE_PRESSURE);
> > +		if (val_p < 0)
> > +			continue;
> > +
> > +		valid = true;
> > +	}
> > +
> > +	input_report_abs(lradc->ts_input, ABS_PRESSURE, 0);
> > +	input_report_key(lradc->ts_input, BTN_TOUCH, 0);
> > +	input_sync(lradc->ts_input);
> > +
> > +	/* Do not restart the TS IRQ if the driver is shutting down. */
> > +	if (lradc->stop_touchscreen)
> > +		return;
> > +
> > +	/* Restart the touchscreen interrupts. */
> > +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
> > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> > +}
> > +
> > +static int mxs_lradc_ts_open(struct input_dev *dev)
> > +{
> > +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> > +
> > +	/* The touchscreen is starting. */
> > +	lradc->stop_touchscreen = 0;
> > +
> > +	/* Enable the touch-detect circuitry. */
> > +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> > +
> > +	/* Enable the touch-detect IRQ. */
> > +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> > +
> > +	return 0;
> > +}
> > +
> > +static void mxs_lradc_ts_close(struct input_dev *dev)
> > +{
> > +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> > +
> > +	/* Indicate the touchscreen is stopping. */
> > +	lradc->stop_touchscreen = 1;
> > +
> > +	/* Disable touchscreen touch-detect IRQ. */
> > +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > +
> > +	/* Power-down touchscreen touch-detect circuitry. */
> > +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);

These 2 writes are racing with writes in mxs_lradc_ts_work(). I think
you need to:

	lradc->stop_touchscreen = true;
	mb();
	cancel_work_sync(&lradc->ts_work);
	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
	...

> > +
> > +	/* Wait until touchscreen thread finishes any possible remnants. */
> > +	cancel_work_sync(&lradc->ts_work);
> > +}
> > +
> > +static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> > +{
> > +	struct input_dev *input;
> > +	struct device *dev = lradc->dev;
> > +	int ret;
> > +
> > +	if (!lradc->use_touchscreen)
> > +		return 0;
> > +
> > +	input = input_allocate_device();
> > +	if (!input) {
> > +		dev_warn(dev, "Failed to allocate TS device, disabling.\n");
> > +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_NONE;
> > +		return 0;
> > +	}
> > +
> > +	input->name = DRIVER_NAME;
> > +	input->id.bustype = BUS_HOST;
> > +	input->dev.parent = dev;
> > +	input->open = mxs_lradc_ts_open;
> > +	input->close = mxs_lradc_ts_close;
> > +
> > +	__set_bit(EV_ABS, input->evbit);
> > +	__set_bit(EV_KEY, input->evbit);
> > +	__set_bit(BTN_TOUCH, input->keybit);
> > +	input_set_abs_params(input, ABS_X, 0, LRADC_CH_VALUE_MASK, 0, 0);
> > +	input_set_abs_params(input, ABS_Y, 0, LRADC_CH_VALUE_MASK, 0, 0);
> > +	input_set_abs_params(input, ABS_PRESSURE, 0, LRADC_CH_VALUE_MASK, 0, 0);
> > +
> > +	lradc->ts_input = input;
> > +	input_set_drvdata(input, lradc);
> > +	ret = input_register_device(input);
> > +	if (ret)
> > +		input_free_device(lradc->ts_input);
> > +
> > +	return ret;

So why is allocation failure is not fatal bur registration is? I'd make
both fatal.

> > +}
> > +
> > +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> > +{
> > +	if (!lradc->use_touchscreen)
> > +		return;
> > +
> > +	if (!lradc->ts_input)
> > +		return;

Do we really need to check both conditions?

> > +
> > +	cancel_work_sync(&lradc->ts_work);
> > +
> > +	input_unregister_device(lradc->ts_input);
> > +}
> > +
> > +/*
> >   * IRQ Handling
> >   */
> >  static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> > @@ -202,14 +550,24 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void
> > *data) struct iio_dev *iio = data;
> >  	struct mxs_lradc *lradc = iio_priv(iio);
> >  	unsigned long reg = readl(lradc->base + LRADC_CTRL1);
> > +	const uint32_t ts_irq_mask =
> > +		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
> > +		LRADC_CTRL1_TOUCH_DETECT_IRQ;
> > 
> >  	if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
> >  		return IRQ_NONE;
> > 
> >  	/*
> > -	 * Touchscreen IRQ handling code shall probably have priority
> > -	 * and therefore shall be placed here.
> > +	 * Touchscreen IRQ handling code has priority and therefore
> > +	 * is placed here. In case touchscreen IRQ arrives, disable
> > +	 * it ASAP
> >  	 */
> > +	if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
> > +		writel(ts_irq_mask,
> > +			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > +		if (!lradc->stop_touchscreen)
> > +			schedule_work(&lradc->ts_work);
> > +	}
> > 
> >  	if (iio_buffer_enabled(iio))
> >  		iio_trigger_poll(iio->trig, iio_get_time_ns());
> > @@ -306,8 +664,10 @@ static int mxs_lradc_buffer_preenable(struct iio_dev
> > *iio) {
> >  	struct mxs_lradc *lradc = iio_priv(iio);
> >  	struct iio_buffer *buffer = iio->buffer;
> > -	int ret = 0, chan, ofs = 0, enable = 0;
> > -	uint32_t ctrl4 = 0;
> > +	int ret = 0, chan, ofs = 0;
> > +	unsigned long enable = 0;
> > +	uint32_t ctrl4_set = 0;
> > +	uint32_t ctrl4_clr = 0;
> >  	uint32_t ctrl1_irq = 0;
> >  	const uint32_t chan_value = LRADC_CH_ACCUMULATE |
> >  		((LRADC_DELAY_TIMER_LOOP - 1) << LRADC_CH_NUM_SAMPLES_OFFSET);
> > @@ -339,17 +699,20 @@ static int mxs_lradc_buffer_preenable(struct iio_dev
> > *iio) writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > 
> >  	for_each_set_bit(chan, buffer->scan_mask, LRADC_MAX_TOTAL_CHANS) {
> > -		ctrl4 |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> > +		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
> > +		ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
> >  		ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
> >  		writel(chan_value, lradc->base + LRADC_CH(ofs));
> > -		enable |= 1 << ofs;
> > +		bitmap_set(&enable, ofs, 1);
> >  		ofs++;
> >  	};
> > 
> >  	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
> >  		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
> > 
> > -	writel(ctrl4, lradc->base + LRADC_CTRL4);
> > +	writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> > +	writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
> > +
> >  	writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
> > 
> >  	writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
> > @@ -384,9 +747,33 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev
> > *iio) static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
> >  					const unsigned long *mask)
> >  {
> > -	const int mw = bitmap_weight(mask, iio->masklength);
> > -
> > -	return mw <= LRADC_MAX_MAPPED_CHANS;
> > +	struct mxs_lradc *lradc = iio_priv(iio);
> > +	const int len = iio->masklength;
> > +	const int map_chans = bitmap_weight(mask, len);
> > +	int rsvd_chans = 0;
> > +	unsigned long rsvd_mask = 0;
> > +
> > +	if (lradc->use_touchbutton)
> > +		rsvd_mask |= CHAN_MASK_TOUCHBUTTON;
> > +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_4WIRE)
> > +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_4WIRE;
> > +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
> > +		rsvd_mask |= CHAN_MASK_TOUCHSCREEN_5WIRE;
> > +
> > +	if (lradc->use_touchbutton)
> > +		rsvd_chans++;
> > +	if (lradc->use_touchscreen)
> > +		rsvd_chans++;
> > +
> > +	/* Test for attempts to map channels with special mode of operation. */
> > +	if (bitmap_intersects(mask, &rsvd_mask, len))
> > +		return 0;
> > +
> > +	/* Test for attempts to map more channels then available slots. */
> > +	if (map_chans + rsvd_chans > LRADC_MAX_MAPPED_CHANS)
> > +		return 0;
> > +
> > +	return 1;
> >  }
> > 
> >  static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
> > @@ -435,15 +822,29 @@ static const struct iio_chan_spec
> > mxs_lradc_chan_spec[] = {
> > 
> >  static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
> >  {
> > -	int i;
> > -	const uint32_t cfg =
> > +	/* The ADC always uses DELAY CHANNEL 0. */
> > +	const uint32_t adc_cfg =
> > +		(1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + 0)) |
> >  		(LRADC_DELAY_TIMER_PER << LRADC_DELAY_DELAY_OFFSET);
> > 
> >  	stmp_reset_block(lradc->base);
> > 
> > -	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> > -		writel(cfg | (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)),
> > -			lradc->base + LRADC_DELAY(i));
> > +	/* Configure DELAY CHANNEL 0 for generic ADC sampling. */
> > +	writel(adc_cfg, lradc->base + LRADC_DELAY(0));
> > +
> > +	/* Disable remaining DELAY CHANNELs */
> > +	writel(0, lradc->base + LRADC_DELAY(1));
> > +	writel(0, lradc->base + LRADC_DELAY(2));
> > +	writel(0, lradc->base + LRADC_DELAY(3));
> > +
> > +	/* Configure the touchscreen type */
> > +	writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > +
> > +	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
> > +		writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
> > +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> > +	}
> > 
> >  	/* Start internal temperature sensing. */
> >  	writel(0, lradc->base + LRADC_CTRL2);
> > @@ -463,9 +864,11 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
> >  static int __devinit mxs_lradc_probe(struct platform_device *pdev)


__devinits must go now.

> >  {
> >  	struct device *dev = &pdev->dev;
> > +	struct device_node *node = dev->of_node;
> >  	struct mxs_lradc *lradc;
> >  	struct iio_dev *iio;
> >  	struct resource *iores;
> > +	uint32_t ts_wires = 0;
> >  	int ret = 0;
> >  	int i;
> > 
> > @@ -487,6 +890,21 @@ static int __devinit mxs_lradc_probe(struct
> > platform_device *pdev) goto err_addr;
> >  	}
> > 
> > +	INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
> > +
> > +	/* Check if touchscreen is enabled in DT. */
> > +	ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
> > +				&ts_wires);
> > +	if (ret)
> > +		dev_info(dev, "Touchscreen not enabled.\n");
> > +	else if (ts_wires == 4)
> > +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_4WIRE;
> > +	else if (ts_wires == 5)
> > +		lradc->use_touchscreen = MXS_LRADC_TOUCHSCREEN_5WIRE;
> > +	else
> > +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
> > +				ts_wires);
> > +
> >  	/* Grab all IRQ sources */
> >  	for (i = 0; i < 13; i++) {
> >  		lradc->irq[i] = platform_get_irq(pdev, i);
> > @@ -524,11 +942,16 @@ static int __devinit mxs_lradc_probe(struct
> > platform_device *pdev) if (ret)
> >  		goto err_trig;
> > 
> > +	/* Register the touchscreen input device. */
> > +	ret = mxs_lradc_ts_register(lradc);
> > +	if (ret)
> > +		goto err_dev;
> > +
> >  	/* Register IIO device. */
> >  	ret = iio_device_register(iio);
> >  	if (ret) {
> >  		dev_err(dev, "Failed to register IIO device\n");
> > -		goto err_dev;
> > +		goto err_ts;
> >  	}
> > 
> >  	/* Configure the hardware. */
> > @@ -536,6 +959,8 @@ static int __devinit mxs_lradc_probe(struct
> > platform_device *pdev)
> > 
> >  	return 0;
> > 
> > +err_ts:
> > +	mxs_lradc_ts_unregister(lradc);
> >  err_dev:
> >  	mxs_lradc_trigger_remove(iio);
> >  err_trig:
> > @@ -550,6 +975,8 @@ static int __devexit mxs_lradc_remove(struct
> > platform_device *pdev) struct iio_dev *iio = platform_get_drvdata(pdev);
> >  	struct mxs_lradc *lradc = iio_priv(iio);
> > 
> > +	mxs_lradc_ts_unregister(lradc);
> > +
> >  	mxs_lradc_hw_stop(lradc);
> > 
> >  	iio_device_unregister(iio);

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-10  0:57     ` Dmitry Torokhov
@ 2013-01-10  9:48       ` Marek Vasut
  2013-01-10 17:46         ` Dmitry Torokhov
  0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2013-01-10  9:48 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Dmitry Torokhov,

[...]
> > > +	enum mxs_lradc_ts	use_touchscreen;
> > > +	unsigned int		stop_touchscreen:1;
> > > +	unsigned int		use_touchbutton:1;
> 
> Can we make them bools instead of bit fields?

Sure.
[...]

> > > +static void mxs_lradc_ts_close(struct input_dev *dev)
> > > +{
> > > +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> > > +
> > > +	/* Indicate the touchscreen is stopping. */
> > > +	lradc->stop_touchscreen = 1;
> > > +
> > > +	/* Disable touchscreen touch-detect IRQ. */
> > > +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> > > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > > +
> > > +	/* Power-down touchscreen touch-detect circuitry. */
> > > +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> 
> These 2 writes are racing with writes in mxs_lradc_ts_work(). I think
> you need to:
> 
> 	lradc->stop_touchscreen = true;
> 	mb();

Nice catch, do we need the memory barrier here though, is it not enough to 
reorder the cancel_work_sync() just before the register writes?

> 	cancel_work_sync(&lradc->ts_work);
> 	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> 	...

[...]

> > > +	lradc->ts_input = input;
> > > +	input_set_drvdata(input, lradc);
> > > +	ret = input_register_device(input);
> > > +	if (ret)
> > > +		input_free_device(lradc->ts_input);
> > > +
> > > +	return ret;
> 
> So why is allocation failure is not fatal bur registration is? I'd make
> both fatal.

Since it's still possible to operate the block even if touchscreen fails to 
probe. If that happens, certainly something is already weird. I'd say both 
should be fatal.

> > > +}
> > > +
> > > +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> > > +{
> > > +	if (!lradc->use_touchscreen)
> > > +		return;
> > > +
> > > +	if (!lradc->ts_input)
> > > +		return;
> 
> Do we really need to check both conditions?

Better safe than sorry, but lradc->ts_input should be non-NULL in case lradc-
>use_touchscreen is set, so I'll remove it.

[...]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-10  9:48       ` Marek Vasut
@ 2013-01-10 17:46         ` Dmitry Torokhov
  2013-01-10 18:19           ` Lars-Peter Clausen
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2013-01-10 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 10, 2013 at 10:48:37AM +0100, Marek Vasut wrote:
> Dear Dmitry Torokhov,
> 
> [...]
> > > > +	enum mxs_lradc_ts	use_touchscreen;
> > > > +	unsigned int		stop_touchscreen:1;
> > > > +	unsigned int		use_touchbutton:1;
> > 
> > Can we make them bools instead of bit fields?
> 
> Sure.
> [...]
> 
> > > > +static void mxs_lradc_ts_close(struct input_dev *dev)
> > > > +{
> > > > +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> > > > +
> > > > +	/* Indicate the touchscreen is stopping. */
> > > > +	lradc->stop_touchscreen = 1;
> > > > +
> > > > +	/* Disable touchscreen touch-detect IRQ. */
> > > > +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> > > > +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > > > +
> > > > +	/* Power-down touchscreen touch-detect circuitry. */
> > > > +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > > > +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> > 
> > These 2 writes are racing with writes in mxs_lradc_ts_work(). I think
> > you need to:
> > 
> > 	lradc->stop_touchscreen = true;
> > 	mb();
> 
> Nice catch, do we need the memory barrier here though, is it not enough to 
> reorder the cancel_work_sync() just before the register writes?

You really need to make sure that setting lradc->stop_touchscreen =
true; is not reordered, because otherwise you may cancel the work,
interrupt happens and reschedules it, and then you set up the flag.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-10 17:46         ` Dmitry Torokhov
@ 2013-01-10 18:19           ` Lars-Peter Clausen
  2013-01-10 18:44             ` Dmitry Torokhov
  0 siblings, 1 reply; 20+ messages in thread
From: Lars-Peter Clausen @ 2013-01-10 18:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/10/2013 06:46 PM, Dmitry Torokhov wrote:
> On Thu, Jan 10, 2013 at 10:48:37AM +0100, Marek Vasut wrote:
>> Dear Dmitry Torokhov,
>>
>> [...]
>>>>> +	enum mxs_lradc_ts	use_touchscreen;
>>>>> +	unsigned int		stop_touchscreen:1;
>>>>> +	unsigned int		use_touchbutton:1;
>>>
>>> Can we make them bools instead of bit fields?
>>
>> Sure.
>> [...]
>>
>>>>> +static void mxs_lradc_ts_close(struct input_dev *dev)
>>>>> +{
>>>>> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
>>>>> +
>>>>> +	/* Indicate the touchscreen is stopping. */
>>>>> +	lradc->stop_touchscreen = 1;
>>>>> +
>>>>> +	/* Disable touchscreen touch-detect IRQ. */
>>>>> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
>>>>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>>>>> +
>>>>> +	/* Power-down touchscreen touch-detect circuitry. */
>>>>> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
>>>>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>>>
>>> These 2 writes are racing with writes in mxs_lradc_ts_work(). I think
>>> you need to:
>>>
>>> 	lradc->stop_touchscreen = true;
>>> 	mb();
>>
>> Nice catch, do we need the memory barrier here though, is it not enough to 
>> reorder the cancel_work_sync() just before the register writes?
> 
> You really need to make sure that setting lradc->stop_touchscreen =
> true; is not reordered, because otherwise you may cancel the work,
> interrupt happens and reschedules it, and then you set up the flag.

Does it really matter? If it is rescheduled you get one event more reported
after the device has been closed, but input core should ignore it anyway, or
doesn't it?

Btw. why not use threaded interrupts instead of irq + workqueue combo?

- Lars

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen
  2013-01-10 18:19           ` Lars-Peter Clausen
@ 2013-01-10 18:44             ` Dmitry Torokhov
  0 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2013-01-10 18:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 10, 2013 at 07:19:34PM +0100, Lars-Peter Clausen wrote:
> On 01/10/2013 06:46 PM, Dmitry Torokhov wrote:
> > On Thu, Jan 10, 2013 at 10:48:37AM +0100, Marek Vasut wrote:
> >> Dear Dmitry Torokhov,
> >>
> >> [...]
> >>>>> +	enum mxs_lradc_ts	use_touchscreen;
> >>>>> +	unsigned int		stop_touchscreen:1;
> >>>>> +	unsigned int		use_touchbutton:1;
> >>>
> >>> Can we make them bools instead of bit fields?
> >>
> >> Sure.
> >> [...]
> >>
> >>>>> +static void mxs_lradc_ts_close(struct input_dev *dev)
> >>>>> +{
> >>>>> +	struct mxs_lradc *lradc = input_get_drvdata(dev);
> >>>>> +
> >>>>> +	/* Indicate the touchscreen is stopping. */
> >>>>> +	lradc->stop_touchscreen = 1;
> >>>>> +
> >>>>> +	/* Disable touchscreen touch-detect IRQ. */
> >>>>> +	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> >>>>> +		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> >>>>> +
> >>>>> +	/* Power-down touchscreen touch-detect circuitry. */
> >>>>> +	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> >>>>> +		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> >>>
> >>> These 2 writes are racing with writes in mxs_lradc_ts_work(). I think
> >>> you need to:
> >>>
> >>> 	lradc->stop_touchscreen = true;
> >>> 	mb();
> >>
> >> Nice catch, do we need the memory barrier here though, is it not enough to 
> >> reorder the cancel_work_sync() just before the register writes?
> > 
> > You really need to make sure that setting lradc->stop_touchscreen =
> > true; is not reordered, because otherwise you may cancel the work,
> > interrupt happens and reschedules it, and then you set up the flag.
> 
> Does it really matter? If it is rescheduled you get one event more reported
> after the device has been closed, but input core should ignore it anyway, or
> doesn't it?

But the problem is that if work runs again it will enable the interrupt.
It is better to have all ducks in a row.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-01-10 18:44 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-14  1:46 [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Marek Vasut
2012-12-14  1:46 ` [PATCH 2/3 V2] iio: mxs: Implement support for touchscreen Marek Vasut
2012-12-14  1:58   ` Fabio Estevam
2012-12-14  2:04     ` Marek Vasut
2012-12-19 17:01   ` Marek Vasut
2012-12-27 12:16     ` Jonathan Cameron
2013-01-08 19:28       ` Marek Vasut
2013-01-09 20:01         ` Jonathan Cameron
2013-01-09 20:17           ` Marek Vasut
2013-01-09 21:38             ` Jonathan Cameron
2013-01-09 21:54               ` Marek Vasut
2013-01-10  0:57     ` Dmitry Torokhov
2013-01-10  9:48       ` Marek Vasut
2013-01-10 17:46         ` Dmitry Torokhov
2013-01-10 18:19           ` Lars-Peter Clausen
2013-01-10 18:44             ` Dmitry Torokhov
2012-12-27 11:31   ` Jonathan Cameron
2012-12-27 18:19     ` Marek Vasut
2012-12-14  1:46 ` [PATCH 3/3] iio: mxs: Enable touchscreen on m28evk Marek Vasut
2012-12-27 11:25 ` [PATCH 1/3 V2] iio: mxs: Remove unused struct mxs_lradc_chan Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).