[PATCH 1/5] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC

[PATCH 1/5] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC

[Juergen Beisert]

The LRADC units in i.MX23 and i.MX28 differ and we need to distinguish both
SoC variants in order to make the touchscreen work on i.MX23

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
CC: linux-arm-kernel at lists.infradead.org
CC: devel at driverdev.osuosl.org
CC: Marek Vasut <marex@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
CC: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/mxs-lradc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index a08c173..dffca90 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -174,6 +174,8 @@ struct mxs_lradc {
 
 	struct input_dev	*ts_input;
 	struct work_struct	ts_work;
+
+	enum mxs_lradc_id	soc;
 };
 
 #define	LRADC_CTRL0				0x00
@@ -920,6 +922,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 	}
 
 	lradc = iio_priv(iio);
+	lradc->soc = (enum mxs_lradc_id)of_id->data;
 
 	/* Grab the memory area */
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.8.4.rc3

[PATCH 2/5] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits

[Juergen Beisert]

In order to support i.MX23 and i.MX28 within one driver we need to separate the
register definitions which differ in both SoC variants.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
CC: linux-arm-kernel at lists.infradead.org
CC: devel at driverdev.osuosl.org
CC: Marek Vasut <marex@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
CC: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/mxs-lradc.c | 60 ++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index dffca90..00e0c29 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -179,24 +179,28 @@ struct mxs_lradc {
 };
 
 #define	LRADC_CTRL0				0x00
-#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_CTRL0_MX28_TOUCH_DETECT_ENABLE	(1 << 23)
+# define LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE	(1 << 22)
+# define LRADC_CTRL0_MX28_YNNSW	/* YM */	(1 << 21)
+# define LRADC_CTRL0_MX28_YPNSW	/* YP */	(1 << 20)
+# define LRADC_CTRL0_MX28_YPPSW	/* YP */	(1 << 19)
+# define LRADC_CTRL0_MX28_XNNSW	/* XM */	(1 << 18)
+# define LRADC_CTRL0_MX28_XNPSW	/* XM */	(1 << 17)
+# define LRADC_CTRL0_MX28_XPPSW	/* XP */	(1 << 16)
+
+#define	LRADC_CTRL0_MX28_PLATE_MASK \
+		LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
+		LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
+		LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW
 
 #define	LRADC_CTRL1				0x10
 #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_MX28_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_MX28_LRADC_IRQ_MASK		0x1fff
 #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
 
 #define	LRADC_CTRL2				0x20
@@ -264,7 +268,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	 * Virtual channel 0 is always used here as the others are always not
 	 * used if doing raw sampling.
 	 */
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
+	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
@@ -319,9 +323,9 @@ static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
 	uint32_t reg;
 
 	/* Enable touch detection. */
-	writel(LRADC_CTRL0_PLATE_MASK,
+	writel(LRADC_CTRL0_MX28_PLATE_MASK,
 		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
 		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 
 	msleep(LRADC_TS_SAMPLE_DELAY_MS);
@@ -367,21 +371,21 @@ static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
 	 */
 	switch (plate) {
 	case LRADC_SAMPLE_X:
-		ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
+		ctrl0 = LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
 		chan = 3;
 		break;
 	case LRADC_SAMPLE_Y:
-		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
+		ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
 		chan = 4;
 		break;
 	case LRADC_SAMPLE_PRESSURE:
-		ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
+		ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
 		chan = 5;
 		break;
 	}
 
 	if (change) {
-		writel(LRADC_CTRL0_PLATE_MASK,
+		writel(LRADC_CTRL0_MX28_PLATE_MASK,
 			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 		writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 
@@ -442,7 +446,7 @@ static void mxs_lradc_ts_work(struct work_struct *ts_work)
 
 	while (mxs_lradc_ts_touched(lradc)) {
 		/* Disable touch detector so we can sample the touchscreen. */
-		writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
 			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
 		if (likely(valid)) {
@@ -491,7 +495,7 @@ static int mxs_lradc_ts_open(struct input_dev *dev)
 	lradc->stop_touchscreen = false;
 
 	/* Enable the touch-detect circuitry. */
-	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
 		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 
 	/* Enable the touch-detect IRQ. */
@@ -517,7 +521,7 @@ static void mxs_lradc_ts_close(struct input_dev *dev)
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	/* Power-down touchscreen touch-detect circuitry. */
-	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
 		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 }
 
@@ -581,7 +585,7 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
 		LRADC_CTRL1_TOUCH_DETECT_IRQ;
 
-	if (!(reg & LRADC_CTRL1_LRADC_IRQ_MASK))
+	if (!(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK))
 		return IRQ_NONE;
 
 	/*
@@ -601,7 +605,7 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
 		complete(&lradc->completion);
 
-	writel(reg & LRADC_CTRL1_LRADC_IRQ_MASK,
+	writel(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	return IRQ_HANDLED;
@@ -722,7 +726,7 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 	if (ret < 0)
 		goto err_buf;
 
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
+	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
@@ -763,7 +767,7 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
 		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
 
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
+	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	kfree(lradc->buffer);
@@ -867,11 +871,11 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
 	writel(0, lradc->base + LRADC_DELAY(3));
 
 	/* Configure the touchscreen type */
-	writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
+	writel(LRADC_CTRL0_MX28_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,
+		writel(LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
 			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 	}
 
@@ -885,7 +889,7 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
 {
 	int i;
 
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
+	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
-- 
1.8.4.rc3

[PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

[Juergen Beisert]

Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC at least
for the 4 wire touchscreen.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
CC: linux-arm-kernel at lists.infradead.org
CC: devel at driverdev.osuosl.org
CC: Marek Vasut <marex@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
CC: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/mxs-lradc.c | 134 +++++++++++++++++++++++++++---------
 1 file changed, 102 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 00e0c29..681ffd4 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -188,19 +188,33 @@ struct mxs_lradc {
 # define LRADC_CTRL0_MX28_XNPSW	/* XM */	(1 << 17)
 # define LRADC_CTRL0_MX28_XPPSW	/* XP */	(1 << 16)
 
+# define LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE	(1 << 20)
+# define LRADC_CTRL0_MX23_YM			(1 << 19)
+# define LRADC_CTRL0_MX23_XM			(1 << 18)
+# define LRADC_CTRL0_MX23_YP			(1 << 17)
+# define LRADC_CTRL0_MX23_XP			(1 << 16)
+
 #define	LRADC_CTRL0_MX28_PLATE_MASK \
+		LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE | \
 		LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
 		LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
 		LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW
 
+#define	LRADC_CTRL0_MX23_PLATE_MASK \
+		LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE | \
+		LRADC_CTRL0_MX23_YM | LRADC_CTRL0_MX23_XM | \
+		LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XP
+
 #define	LRADC_CTRL1				0x10
 #define	LRADC_CTRL1_TOUCH_DETECT_IRQ_EN		(1 << 24)
 #define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
 #define	LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK	(0x1fff << 16)
+#define	LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK	(0x01ff << 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_MX28_LRADC_IRQ_MASK		0x1fff
+#define	LRADC_CTRL1_MX23_LRADC_IRQ_MASK		0x01ff
 #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
 
 #define	LRADC_CTRL2				0x20
@@ -268,8 +282,9 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	 * Virtual channel 0 is always used here as the others are always not
 	 * used if doing raw sampling.
 	 */
-	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	if (lradc->soc == IMX28_LRADC)
+		writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
 	/* Clean the slot's previous content, then set new one. */
@@ -323,10 +338,17 @@ static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
 	uint32_t reg;
 
 	/* Enable touch detection. */
-	writel(LRADC_CTRL0_MX28_PLATE_MASK,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	if (lradc->soc == IMX28_LRADC) {
+		writel(LRADC_CTRL0_MX28_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	} else {
+		writel(LRADC_CTRL0_MX23_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	}
 
 	msleep(LRADC_TS_SAMPLE_DELAY_MS);
 
@@ -371,22 +393,36 @@ static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
 	 */
 	switch (plate) {
 	case LRADC_SAMPLE_X:
-		ctrl0 = LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
+		if (lradc->soc == IMX28_LRADC)
+			ctrl0 = LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
+		else
+			ctrl0 = LRADC_CTRL0_MX23_XP | LRADC_CTRL0_MX23_XM;
 		chan = 3;
 		break;
 	case LRADC_SAMPLE_Y:
-		ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
+		if (lradc->soc == IMX28_LRADC)
+			ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
+		else
+			ctrl0 = LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_YM;
 		chan = 4;
 		break;
 	case LRADC_SAMPLE_PRESSURE:
-		ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
+		if (lradc->soc == IMX28_LRADC)
+			ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
+		else
+			ctrl0 = LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
 		chan = 5;
 		break;
 	}
 
 	if (change) {
-		writel(LRADC_CTRL0_MX28_PLATE_MASK,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		if (lradc->soc == IMX28_LRADC)
+			writel(LRADC_CTRL0_MX28_PLATE_MASK,
+				lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		else
+			writel(LRADC_CTRL0_MX23_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),
@@ -446,8 +482,12 @@ static void mxs_lradc_ts_work(struct work_struct *ts_work)
 
 	while (mxs_lradc_ts_touched(lradc)) {
 		/* Disable touch detector so we can sample the touchscreen. */
-		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		if (lradc->soc == IMX28_LRADC)
+			writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
+				lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		else
+			writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
+				lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
 		if (likely(valid)) {
 			input_report_abs(lradc->ts_input, ABS_X, val_x);
@@ -495,8 +535,12 @@ static int mxs_lradc_ts_open(struct input_dev *dev)
 	lradc->stop_touchscreen = false;
 
 	/* Enable the touch-detect circuitry. */
-	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	if (lradc->soc == IMX28_LRADC)
+		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	else
+		writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 
 	/* Enable the touch-detect IRQ. */
 	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
@@ -521,8 +565,12 @@ static void mxs_lradc_ts_close(struct input_dev *dev)
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	/* Power-down touchscreen touch-detect circuitry. */
-	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+	if (lradc->soc == IMX28_LRADC)
+		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+	else
+		writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 }
 
 static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
@@ -585,8 +633,13 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 		LRADC_CTRL1_TOUCH_DETECT_IRQ_EN |
 		LRADC_CTRL1_TOUCH_DETECT_IRQ;
 
-	if (!(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK))
-		return IRQ_NONE;
+	if (lradc->soc == IMX28_LRADC) {
+		if (!(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK))
+			return IRQ_NONE;
+	} else {
+		if (!(reg & LRADC_CTRL1_MX23_LRADC_IRQ_MASK))
+			return IRQ_NONE;
+	}
 
 	/*
 	 * Touchscreen IRQ handling code has priority and therefore
@@ -605,8 +658,12 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
 		complete(&lradc->completion);
 
-	writel(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	if (lradc->soc == IMX28_LRADC)
+		writel(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	else
+		writel(reg & LRADC_CTRL1_MX23_LRADC_IRQ_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	return IRQ_HANDLED;
 }
@@ -726,8 +783,9 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 	if (ret < 0)
 		goto err_buf;
 
-	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	if (lradc->soc == IMX28_LRADC)
+		writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
 	for_each_set_bit(chan, iio->active_scan_mask, LRADC_MAX_TOTAL_CHANS) {
@@ -767,8 +825,9 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
 		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
 
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	if (lradc->soc == IMX28_LRADC)
+		writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	kfree(lradc->buffer);
 	mutex_unlock(&lradc->lock);
@@ -871,12 +930,13 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
 	writel(0, lradc->base + LRADC_DELAY(3));
 
 	/* Configure the touchscreen type */
-	writel(LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-
-	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
+	if (lradc->soc == IMX28_LRADC) {
 		writel(LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+
+		if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
+			writel(LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
+				lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 	}
 
 	/* Start internal temperature sensing. */
@@ -889,8 +949,12 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
 {
 	int i;
 
-	writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	if (lradc->soc == IMX28_LRADC)
+		writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	else
+		writel(LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
 		writel(0, lradc->base + LRADC_DELAY(i));
@@ -950,6 +1014,12 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
 				ts_wires);
 
+	if ((lradc->soc == IMX23_LRADC) && (ts_wires == 5)) {
+		dev_warn(dev, "No support for 5 wire touches on i.MX23\n");
+		dev_warn(dev, "Falling back to 4 wire\n");
+		ts_wires = 4;
+	}
+
 	/* Grab all IRQ sources */
 	for (i = 0; i < of_cfg->irq_count; i++) {
 		lradc->irq[i] = platform_get_irq(pdev, i);
-- 
1.8.4.rc3

[PATCH 4/5] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection

[Juergen Beisert]

For battery driven systems it is a very bad idea to collect the touchscreen
data within a kernel busy loop.

This change uses the features of the hardware to delay and accumulate samples in
hardware to avoid a high interrupt and CPU load.

Note: this is only tested on a i.MX23 SoC yet.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
CC: linux-arm-kernel at lists.infradead.org
CC: devel at driverdev.osuosl.org
CC: Marek Vasut <marex@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
CC: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/mxs-lradc.c | 609 ++++++++++++++++++++++++++++++++----
 1 file changed, 542 insertions(+), 67 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 681ffd4..9462af0 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -129,6 +129,17 @@ enum mxs_lradc_ts {
 	MXS_LRADC_TOUCHSCREEN_5WIRE,
 };
 
+/*
+ * Touchscreen handling
+ */
+enum lradc_ts_plate {
+	LRADC_TOUCH = 0,
+	LRADC_SAMPLE_X,
+	LRADC_SAMPLE_Y,
+	LRADC_SAMPLE_PRESSURE,
+	LRADC_SAMPLE_VALID,
+};
+
 struct mxs_lradc {
 	struct device		*dev;
 	void __iomem		*base;
@@ -169,13 +180,25 @@ struct mxs_lradc {
 #define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 2)
 #define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 2)
 	enum mxs_lradc_ts	use_touchscreen;
-	bool			stop_touchscreen;
 	bool			use_touchbutton;
 
 	struct input_dev	*ts_input;
 	struct work_struct	ts_work;
 
 	enum mxs_lradc_id	soc;
+	enum lradc_ts_plate	cur_plate; /* statemachine */
+	bool			ts_valid;
+	unsigned		ts_x_pos;
+	unsigned		ts_y_pos;
+	unsigned		ts_pressure;
+
+	/* handle touchscreen's physical behaviour */
+	/* samples per coordinate */
+	unsigned		over_sample_cnt;
+	/* time clocks between samples */
+	unsigned		over_sample_delay;
+	/* time in clocks to wait after the plates where switched */
+	unsigned		settling_delay;
 };
 
 #define	LRADC_CTRL0				0x00
@@ -227,19 +250,33 @@ struct mxs_lradc {
 #define	LRADC_CH_ACCUMULATE			(1 << 29)
 #define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
 #define	LRADC_CH_NUM_SAMPLES_OFFSET		24
+#define	LRADC_CH_NUM_SAMPLES(x) \
+				((x) << LRADC_CH_NUM_SAMPLES_OFFSET)
 #define	LRADC_CH_VALUE_MASK			0x3ffff
 #define	LRADC_CH_VALUE_OFFSET			0
 
 #define	LRADC_DELAY(n)				(0xd0 + (0x10 * (n)))
 #define	LRADC_DELAY_TRIGGER_LRADCS_MASK		(0xff << 24)
 #define	LRADC_DELAY_TRIGGER_LRADCS_OFFSET	24
+#define	LRADC_DELAY_TRIGGER(x) \
+				(((x) << LRADC_DELAY_TRIGGER_LRADCS_OFFSET) & \
+				LRADC_DELAY_TRIGGER_LRADCS_MASK)
 #define	LRADC_DELAY_KICK			(1 << 20)
 #define	LRADC_DELAY_TRIGGER_DELAYS_MASK		(0xf << 16)
 #define	LRADC_DELAY_TRIGGER_DELAYS_OFFSET	16
+#define	LRADC_DELAY_TRIGGER_DELAYS(x) \
+				(((x) << LRADC_DELAY_TRIGGER_DELAYS_OFFSET) & \
+				LRADC_DELAY_TRIGGER_DELAYS_MASK)
 #define	LRADC_DELAY_LOOP_COUNT_MASK		(0x1f << 11)
 #define	LRADC_DELAY_LOOP_COUNT_OFFSET		11
+#define	LRADC_DELAY_LOOP(x) \
+				(((x) << LRADC_DELAY_LOOP_COUNT_OFFSET) & \
+				LRADC_DELAY_LOOP_COUNT_MASK)
 #define	LRADC_DELAY_DELAY_MASK			0x7ff
 #define	LRADC_DELAY_DELAY_OFFSET		0
+#define	LRADC_DELAY_DELAY(x) \
+				(((x) << LRADC_DELAY_DELAY_OFFSET) & \
+				LRADC_DELAY_DELAY_MASK)
 
 #define	LRADC_CTRL4				0x140
 #define	LRADC_CTRL4_LRADCSELECT_MASK(n)		(0xf << ((n) * 4))
@@ -248,6 +285,466 @@ struct mxs_lradc {
 #define LRADC_RESOLUTION			12
 #define LRADC_SINGLE_SAMPLE_MASK		((1 << LRADC_RESOLUTION) - 1)
 
+static bool mxs_lradc_check_touch_event(struct mxs_lradc *lradc)
+{
+	return !!(readl(lradc->base + LRADC_STATUS) &
+					LRADC_STATUS_TOUCH_DETECT_RAW);
+}
+
+static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
+{
+	/*
+	 * prepare for oversampling conversion
+	 *
+	 * from the datasheet:
+	 * "The ACCUMULATE bit in the appropriate channel register
+	 * HW_LRADC_CHn must be set to 1 if NUM_SAMPLES is greater then 0;
+	 * otherwise, the IRQs will not fire."
+	 */
+	writel(LRADC_CH_ACCUMULATE |
+			LRADC_CH_NUM_SAMPLES(lradc->over_sample_cnt - 1),
+			lradc->base + LRADC_CH(ch));
+
+	/* from the datasheet:
+	 * "Software must clear this register in preparation for a
+	 * multi-cycle accumulation.
+	 */
+	writel(LRADC_CH_VALUE_MASK, lradc->base + LRADC_CH(ch) +
+							STMP_OFFSET_REG_CLR);
+
+	/* prepare the delay/loop unit according to the oversampling count */
+	writel(LRADC_DELAY_TRIGGER(1 << ch) |
+		LRADC_DELAY_TRIGGER_DELAYS(0) |
+		LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
+		LRADC_DELAY_DELAY(lradc->over_sample_delay - 1),
+			lradc->base + LRADC_DELAY(3));
+
+	writel(LRADC_CTRL1_LRADC_IRQ(2) | LRADC_CTRL1_LRADC_IRQ(3) |
+		LRADC_CTRL1_LRADC_IRQ(4) | LRADC_CTRL1_LRADC_IRQ(5),
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+
+	/* wake us again, when the complete conversion is done */
+	writel(LRADC_CTRL1_LRADC_IRQ_EN(ch),
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+	/*
+	 * after changing the touchscreen plates setting
+	 * the signals need some initial time to settle. Start the
+	 * SoC's delay unit and start the conversion later
+	 * and automatically.
+	 */
+	writel(LRADC_DELAY_TRIGGER(0) | /* do not trigger an ADC channel */
+		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit #3 */
+		LRADC_DELAY_KICK |
+		LRADC_DELAY_DELAY(lradc->settling_delay),
+			lradc->base + LRADC_DELAY(2));
+}
+
+/*
+ * Pressure detection is special:
+ * We want to do both required measurements for the pressure detection in
+ * one turn. Use the hardware features to chain both conversions and let the
+ * hardware report one interrupt if both conversions are done
+ */
+static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
+							unsigned ch2)
+{
+	u32 reg;
+
+	/*
+	 * prepare for oversampling conversion
+	 *
+	 * from the datasheet:
+	 * "The ACCUMULATE bit in the appropriate channel register
+	 * HW_LRADC_CHn must be set to 1 if NUM_SAMPLES is greater then 0;
+	 * otherwise, the IRQs will not fire."
+	 */
+	reg = LRADC_CH_ACCUMULATE |
+		LRADC_CH_NUM_SAMPLES(lradc->over_sample_cnt - 1);
+	writel(reg, lradc->base + LRADC_CH(ch1));
+	writel(reg, lradc->base + LRADC_CH(ch2));
+
+	/* from the datasheet:
+	 * "Software must clear this register in preparation for a
+	 * multi-cycle accumulation.
+	 */
+	writel(LRADC_CH_VALUE_MASK, lradc->base + LRADC_CH(ch1) +
+							STMP_OFFSET_REG_CLR);
+	writel(LRADC_CH_VALUE_MASK, lradc->base + LRADC_CH(ch2) +
+							STMP_OFFSET_REG_CLR);
+
+	/* prepare the delay/loop unit according to the oversampling count */
+	writel(LRADC_DELAY_TRIGGER(1 << ch1) |
+		LRADC_DELAY_TRIGGER(1 << ch2) | /* start both channels */
+		LRADC_DELAY_TRIGGER_DELAYS(0) |
+		LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
+		LRADC_DELAY_DELAY(lradc->over_sample_delay - 1),
+			lradc->base + LRADC_DELAY(3));
+
+	writel(LRADC_CTRL1_LRADC_IRQ(2) | LRADC_CTRL1_LRADC_IRQ(3) |
+		LRADC_CTRL1_LRADC_IRQ(4) | LRADC_CTRL1_LRADC_IRQ(5),
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+
+	/* wake us again, when the conversions are done */
+	writel(LRADC_CTRL1_LRADC_IRQ_EN(ch2),
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+	/*
+	 * after changing the touchscreen plates setting
+	 * the signals need some initial time to settle. Start the
+	 * SoC's delay unit and start the conversion later
+	 * and automatically.
+	 */
+	writel(LRADC_DELAY_TRIGGER(0) | /* do not trigger an ADC channel */
+		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
+		LRADC_DELAY_KICK |
+		LRADC_DELAY_DELAY(lradc->settling_delay),
+			lradc->base + LRADC_DELAY(2));
+}
+
+static unsigned mxs_lradc_read_raw_channel(struct mxs_lradc *lradc,
+							unsigned channel)
+{
+	u32 reg;
+	unsigned num_samples, val;
+
+	reg = readl(lradc->base + LRADC_CH(channel));
+	if (reg & LRADC_CH_ACCUMULATE)
+		num_samples = lradc->over_sample_cnt;
+	else
+		num_samples = 1;
+
+	val = (reg & LRADC_CH_VALUE_MASK) >> LRADC_CH_VALUE_OFFSET;
+	return val / num_samples;
+}
+
+static unsigned mxs_lradc_read_ts_pressure(struct mxs_lradc *lradc,
+						unsigned ch1, unsigned ch2)
+{
+	u32 reg, mask;
+	unsigned pressure, m1, m2;
+
+	mask = LRADC_CTRL1_LRADC_IRQ(ch1) | LRADC_CTRL1_LRADC_IRQ(ch2);
+	reg = readl(lradc->base + LRADC_CTRL1) & mask;
+
+	while (reg != mask) {
+		reg = readl(lradc->base + LRADC_CTRL1) & mask;
+		dev_dbg(lradc->dev, "One channel is still busy: %X\n", reg);
+	}
+
+	m1 = mxs_lradc_read_raw_channel(lradc, ch1);
+	m2 = mxs_lradc_read_raw_channel(lradc, ch2);
+
+	if (m1 == 0) {
+		dev_warn(lradc->dev, "Cannot calculate pressure\n");
+		return 1 << (LRADC_RESOLUTION - 1);
+	}
+
+	/* simply scale the value from 0 ... max ADC resolution */
+	pressure = m1;
+	pressure *= (1 << LRADC_RESOLUTION);
+	pressure /= m2;
+
+	dev_dbg(lradc->dev, "Pressure = %u\n", pressure);
+	return pressure;
+}
+
+#define TS_CH_XP 2
+#define TS_CH_YP 3
+#define TS_CH_XM 4
+#define TS_CH_YM 5
+
+static int mxs_lradc_read_ts_channel(struct mxs_lradc *lradc)
+{
+	u32 reg;
+	int val;
+
+	reg = readl(lradc->base + LRADC_CTRL1);
+
+	/* only channels 3 to 5 are of interest here */
+	if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_YP)) {
+		pr_devel("!3");
+		writel(LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_YP) |
+			LRADC_CTRL1_LRADC_IRQ(TS_CH_YP),
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+		val = mxs_lradc_read_raw_channel(lradc, TS_CH_YP);
+	} else if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_XM)) {
+		pr_devel("!4");
+		writel(LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_XM) |
+			LRADC_CTRL1_LRADC_IRQ(TS_CH_XM),
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+		val = mxs_lradc_read_raw_channel(lradc, TS_CH_XM);
+	} else if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_YM)) {
+		pr_devel("!5");
+		writel(LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_YM) |
+			LRADC_CTRL1_LRADC_IRQ(TS_CH_YM),
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+		val = mxs_lradc_read_raw_channel(lradc, TS_CH_YM);
+	} else {
+		pr_devel("!?");
+		return -EIO;
+	}
+
+	writel(0, lradc->base + LRADC_DELAY(2));
+	writel(0, lradc->base + LRADC_DELAY(3));
+
+	return val;
+}
+
+/*
+ * YP(open)--+-------------+
+ *           |             |--+
+ *           |             |  |
+ *    YM(-)--+-------------+  |
+ *             +--------------+
+ *             |              |
+ *         XP(weak+)        XM(open)
+ *
+ * "weak+" means 200k Ohm VDDIO
+ * (-) means GND
+ */
+static void mxs_lradc_setup_touch_detection(struct mxs_lradc *lradc)
+{
+	/*
+	 * In order to detect a touch event the 'touch detect enable' bit
+	 * enables:
+	 *  - a weak pullup to the X+ connector
+	 *  - a strong ground at the Y- connector
+	 */
+	if (lradc->soc == IMX28_LRADC) {
+		writel(LRADC_CTRL0_MX28_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	} else {
+		writel(LRADC_CTRL0_MX23_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	}
+	pr_devel(">T");
+}
+
+/*
+ * YP(meas)--+-------------+
+ *           |             |--+
+ *           |             |  |
+ * YM(open)--+-------------+  |
+ *             +--------------+
+ *             |              |
+ *           XP(+)          XM(-)
+ *
+ * (+) means here 1.85 V
+ * (-) means here GND
+ */
+static void mxs_lradc_prepare_x_pos(struct mxs_lradc *lradc)
+{
+	if (lradc->soc == IMX28_LRADC) {
+		writel(LRADC_CTRL0_MX28_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	} else {
+		writel(LRADC_CTRL0_MX23_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX23_XP | LRADC_CTRL0_MX23_XM,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	}
+	lradc->cur_plate = LRADC_SAMPLE_X;
+	mxs_lradc_setup_ts_channel(lradc, TS_CH_YP);
+	pr_devel(">X");
+}
+
+/*
+ *   YP(+)--+-------------+
+ *          |             |--+
+ *          |             |  |
+ *   YM(-)--+-------------+  |
+ *            +--------------+
+ *            |              |
+ *         XP(open)        XM(meas)
+ *
+ * (+) means here 1.85 V
+ * (-) means here GND
+ */
+static void mxs_lradc_prepare_y_pos(struct mxs_lradc *lradc)
+{
+	if (lradc->soc == IMX28_LRADC) {
+		writel(LRADC_CTRL0_MX28_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	} else {
+		writel(LRADC_CTRL0_MX23_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_YM,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	}
+	lradc->cur_plate = LRADC_SAMPLE_Y;
+	mxs_lradc_setup_ts_channel(lradc, TS_CH_XM);
+	pr_devel(">Y");
+}
+
+/*
+ *    YP(+)--+-------------+
+ *           |             |--+
+ *           |             |  |
+ * YM(meas)--+-------------+  |
+ *             +--------------+
+ *             |              |
+ *          XP(meas)        XM(-)
+ *
+ * (+) means here 1.85 V
+ * (-) means here GND
+ */
+static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)
+{
+	if (lradc->soc == IMX28_LRADC) {
+		writel(LRADC_CTRL0_MX28_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW, /* TODO */
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	} else {
+		writel(LRADC_CTRL0_MX23_PLATE_MASK,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+		writel(LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM,
+			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	}
+	lradc->cur_plate = LRADC_SAMPLE_PRESSURE;
+	mxs_lradc_setup_ts_pressure(lradc, TS_CH_XP, TS_CH_YM);
+	pr_devel(">P");
+}
+
+static void mxs_lradc_enable_touch_detection(struct mxs_lradc *lradc)
+{
+	mxs_lradc_setup_touch_detection(lradc);
+	lradc->cur_plate = LRADC_TOUCH;
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ | LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+}
+
+static void mxs_lradc_report_ts_event(struct mxs_lradc *lradc)
+{
+	input_report_abs(lradc->ts_input, ABS_X, lradc->ts_x_pos);
+	input_report_abs(lradc->ts_input, ABS_Y, lradc->ts_y_pos);
+	input_report_abs(lradc->ts_input, ABS_PRESSURE, lradc->ts_pressure);
+	input_report_key(lradc->ts_input, BTN_TOUCH, 1);
+	input_sync(lradc->ts_input);
+}
+
+static void mxs_lradc_complete_touch_event(struct mxs_lradc *lradc)
+{
+	mxs_lradc_setup_touch_detection(lradc);
+	lradc->cur_plate = LRADC_SAMPLE_VALID;
+	/*
+	 * start a dummy conversion to burn time to settle the signals
+	 * note: we are not interested in the conversion's value
+	 */
+	writel(0, lradc->base + LRADC_CH(5));
+	writel(LRADC_CTRL1_LRADC_IRQ(5),
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	writel(LRADC_CTRL1_LRADC_IRQ_EN(5),
+		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+	writel(LRADC_DELAY_TRIGGER(1 << 5) |
+		LRADC_DELAY_KICK | LRADC_DELAY_DELAY(10), /* waste 5 ms */
+			lradc->base + LRADC_DELAY(2));
+}
+
+/*
+ * in order to avoid false measurements, report only samples where
+ * the surface is still touched after the position measurement
+ */
+static void mxs_lradc_finish_touch_event(struct mxs_lradc *lradc, bool valid)
+{
+	/* if it is still touched, report the sample */
+	if (valid && mxs_lradc_check_touch_event(lradc)) {
+		lradc->ts_valid = true;
+		mxs_lradc_report_ts_event(lradc);
+		pr_devel("**");
+	}
+
+	/* if it is even still touched, continue with the next measurement */
+	if (mxs_lradc_check_touch_event(lradc)) {
+		mxs_lradc_prepare_y_pos(lradc);
+		return;
+	}
+
+	if (lradc->ts_valid) {
+		/* signal the release */
+		lradc->ts_valid = false;
+		input_report_key(lradc->ts_input, BTN_TOUCH, 0);
+		input_sync(lradc->ts_input);
+	}
+
+	pr_devel("+P");
+	/* if it is released, wait for the next touch via IRQ */
+	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);
+}
+
+/* touchscreen's state machine */
+static void mxs_lradc_handle_touch(struct mxs_lradc *lradc)
+{
+	int val;
+
+	pr_devel("#I");
+	switch (lradc->cur_plate) {
+	case LRADC_TOUCH:
+		pr_devel("#T");
+		/*
+		 * start with the Y-pos, because it uses nearly the same plate
+		 * settings like the touch detection
+		 */
+		if (mxs_lradc_check_touch_event(lradc)) {
+			pr_devel("-P");
+			writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+					lradc->base + LRADC_CTRL1 +
+							STMP_OFFSET_REG_CLR);
+			mxs_lradc_prepare_y_pos(lradc);
+		}
+		writel(LRADC_CTRL1_TOUCH_DETECT_IRQ,
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+		return;
+
+	case LRADC_SAMPLE_Y:
+		pr_devel("#Y");
+		val = mxs_lradc_read_ts_channel(lradc);
+		if (val < 0) {
+			mxs_lradc_enable_touch_detection(lradc); /* re-start */
+			return;
+		}
+		lradc->ts_y_pos = val;
+		mxs_lradc_prepare_x_pos(lradc);
+		return;
+
+	case LRADC_SAMPLE_X:
+		pr_devel("#X");
+		val = mxs_lradc_read_ts_channel(lradc);
+		if (val < 0) {
+			mxs_lradc_enable_touch_detection(lradc); /* re-start */
+			return;
+		}
+		lradc->ts_x_pos = val;
+		mxs_lradc_prepare_pressure(lradc);
+		return;
+
+	case LRADC_SAMPLE_PRESSURE:
+		pr_devel("#P");
+		lradc->ts_pressure =
+			mxs_lradc_read_ts_pressure(lradc, TS_CH_XP, TS_CH_YM);
+		mxs_lradc_complete_touch_event(lradc);
+		return;
+
+	case LRADC_SAMPLE_VALID:
+		pr_devel("!C\n");
+		val = mxs_lradc_read_ts_channel(lradc); /* ignore the value */
+		mxs_lradc_finish_touch_event(lradc, 1);
+		break;
+	}
+}
+
 /*
  * Raw I/O operations
  */
@@ -324,15 +821,6 @@ static const struct iio_info mxs_lradc_iio_info = {
 	.read_raw		= mxs_lradc_read_raw,
 };
 
-/*
- * 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;
@@ -516,10 +1004,6 @@ static void mxs_lradc_ts_work(struct work_struct *ts_work)
 	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);
@@ -531,48 +1015,38 @@ 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 = false;
-
 	/* Enable the touch-detect circuitry. */
-	if (lradc->soc == IMX28_LRADC)
-		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
-	else
-		writel(LRADC_CTRL0_MX23_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);
+	mxs_lradc_enable_touch_detection(lradc);
 
 	return 0;
 }
 
-static void mxs_lradc_ts_close(struct input_dev *dev)
+static void mxs_lradc_disable_ts(struct mxs_lradc *lradc)
 {
-	struct mxs_lradc *lradc = input_get_drvdata(dev);
-
-	/* Indicate the touchscreen is stopping. */
-	lradc->stop_touchscreen = true;
-	mb();
-
-	/* Wait until touchscreen thread finishes any possible remnants. */
-	cancel_work_sync(&lradc->ts_work);
-
-	/* Disable touchscreen touch-detect IRQ. */
-	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	/* stop all interrupts from firing */
+	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN | LRADC_CTRL1_LRADC_IRQ_EN(2) |
+		LRADC_CTRL1_LRADC_IRQ_EN(3) | LRADC_CTRL1_LRADC_IRQ_EN(4) |
+		LRADC_CTRL1_LRADC_IRQ_EN(5),
+			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	/* Power-down touchscreen touch-detect circuitry. */
 	if (lradc->soc == IMX28_LRADC)
-		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
+		writel(LRADC_CTRL0_MX28_PLATE_MASK |
+					LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
 			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 	else
-		writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
+		writel(LRADC_CTRL0_MX23_PLATE_MASK |
+					LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
 			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 }
 
+static void mxs_lradc_ts_close(struct input_dev *dev)
+{
+	struct mxs_lradc *lradc = input_get_drvdata(dev);
+
+	mxs_lradc_disable_ts(lradc);
+}
+
 static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
 {
 	struct input_dev *input;
@@ -618,6 +1092,7 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
 
 	cancel_work_sync(&lradc->ts_work);
 
+	mxs_lradc_disable_ts(lradc);
 	input_unregister_device(lradc->ts_input);
 }
 
@@ -630,8 +1105,11 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *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;
+		LRADC_CTRL1_TOUCH_DETECT_IRQ |
+		LRADC_CTRL1_LRADC_IRQ(2) |
+		LRADC_CTRL1_LRADC_IRQ(3) |
+		LRADC_CTRL1_LRADC_IRQ(4) |
+		LRADC_CTRL1_LRADC_IRQ(5);
 
 	if (lradc->soc == IMX28_LRADC) {
 		if (!(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK))
@@ -641,30 +1119,14 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 			return IRQ_NONE;
 	}
 
-	/*
-	 * 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 (lradc->use_touchscreen && (reg & ts_irq_mask))
+		mxs_lradc_handle_touch(lradc);
 
 	if (iio_buffer_enabled(iio))
 		iio_trigger_poll(iio->trig, iio_get_time_ns());
 	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
 		complete(&lradc->completion);
 
-	if (lradc->soc == IMX28_LRADC)
-		writel(reg & LRADC_CTRL1_MX28_LRADC_IRQ_MASK,
-			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
-	else
-		writel(reg & LRADC_CTRL1_MX23_LRADC_IRQ_MASK,
-			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
-
 	return IRQ_HANDLED;
 }
 
@@ -967,6 +1429,17 @@ static const struct of_device_id mxs_lradc_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mxs_lradc_dt_ids);
 
+static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc,
+						struct device_node *lradc_node)
+{
+	/* TODO retrieve from device tree */
+	lradc->over_sample_cnt = 4;
+	lradc->over_sample_delay = 2;
+	lradc->settling_delay = 10;
+
+	return 0;
+}
+
 static int mxs_lradc_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -979,7 +1452,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 	struct iio_dev *iio;
 	struct resource *iores;
 	uint32_t ts_wires = 0;
-	int ret = 0;
+	int ret = 0, touch_ret;
 	int i;
 
 	/* Allocate the IIO device. */
@@ -999,7 +1472,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 	if (IS_ERR(lradc->base))
 		return PTR_ERR(lradc->base);
 
-	INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work);
+	touch_ret = mxs_lradc_probe_touchscreen(lradc, node);
 
 	/* Check if touchscreen is enabled in DT. */
 	ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires",
@@ -1062,9 +1535,11 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 		goto err_dev;
 
 	/* Register the touchscreen input device. */
-	ret = mxs_lradc_ts_register(lradc);
-	if (ret)
-		goto err_dev;
+	if (touch_ret == 0) {
+		ret = mxs_lradc_ts_register(lradc);
+		if (ret)
+			goto err_dev;
+	}
 
 	/* Register IIO device. */
 	ret = iio_device_register(iio);
-- 
1.8.4.rc3

[PATCH 5/5] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation

[Juergen Beisert]

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
CC: linux-arm-kernel at lists.infradead.org
CC: devel at driverdev.osuosl.org
CC: Marek Vasut <marex@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
CC: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/mxs-lradc.c | 193 ------------------------------------
 1 file changed, 193 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 9462af0..f81d827 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -183,7 +183,6 @@ struct mxs_lradc {
 	bool			use_touchbutton;
 
 	struct input_dev	*ts_input;
-	struct work_struct	ts_work;
 
 	enum mxs_lradc_id	soc;
 	enum lradc_ts_plate	cur_plate; /* statemachine */
@@ -821,196 +820,6 @@ static const struct iio_info mxs_lradc_iio_info = {
 	.read_raw		= mxs_lradc_read_raw,
 };
 
-static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
-{
-	uint32_t reg;
-
-	/* Enable touch detection. */
-	if (lradc->soc == IMX28_LRADC) {
-		writel(LRADC_CTRL0_MX28_PLATE_MASK,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
-	} else {
-		writel(LRADC_CTRL0_MX23_PLATE_MASK,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-		writel(LRADC_CTRL0_MX23_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:
-		if (lradc->soc == IMX28_LRADC)
-			ctrl0 = LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
-		else
-			ctrl0 = LRADC_CTRL0_MX23_XP | LRADC_CTRL0_MX23_XM;
-		chan = 3;
-		break;
-	case LRADC_SAMPLE_Y:
-		if (lradc->soc == IMX28_LRADC)
-			ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
-		else
-			ctrl0 = LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_YM;
-		chan = 4;
-		break;
-	case LRADC_SAMPLE_PRESSURE:
-		if (lradc->soc == IMX28_LRADC)
-			ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
-		else
-			ctrl0 = LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
-		chan = 5;
-		break;
-	}
-
-	if (change) {
-		if (lradc->soc == IMX28_LRADC)
-			writel(LRADC_CTRL0_MX28_PLATE_MASK,
-				lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-		else
-			writel(LRADC_CTRL0_MX23_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. */
-		if (lradc->soc == IMX28_LRADC)
-			writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-				lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-		else
-			writel(LRADC_CTRL0_MX23_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);
-
-	/* 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);
@@ -1090,8 +899,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
 	if (!lradc->use_touchscreen)
 		return;
 
-	cancel_work_sync(&lradc->ts_work);
-
 	mxs_lradc_disable_ts(lradc);
 	input_unregister_device(lradc->ts_input);
 }
-- 
1.8.4.rc3

[PATCH 2/5] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits

[Marek Vasut]

Dear Juergen Beisert,

> In order to support i.MX23 and i.MX28 within one driver we need to separate
> the register definitions which differ in both SoC variants.
> 
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> CC: linux-arm-kernel at lists.infradead.org
> CC: devel at driverdev.osuosl.org
> CC: Marek Vasut <marex@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> CC: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 60
> ++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 28
> deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index dffca90..00e0c29 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -179,24 +179,28 @@ struct mxs_lradc {
>  };
> 
>  #define	LRADC_CTRL0				0x00
> -#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_CTRL0_MX28_TOUCH_DETECT_ENABLE	(1 << 23)
> +# define LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE	(1 << 22)
> +# define LRADC_CTRL0_MX28_YNNSW	/* YM */	(1 << 21)
> +# define LRADC_CTRL0_MX28_YPNSW	/* YP */	(1 << 20)
> +# define LRADC_CTRL0_MX28_YPPSW	/* YP */	(1 << 19)
> +# define LRADC_CTRL0_MX28_XNNSW	/* XM */	(1 << 18)
> +# define LRADC_CTRL0_MX28_XNPSW	/* XM */	(1 << 17)
> +# define LRADC_CTRL0_MX28_XPPSW	/* XP */	(1 << 16)

Why do you put this space between # and define?

> +#define	LRADC_CTRL0_MX28_PLATE_MASK \
> +		LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
> +		LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
> +		LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW
> 
>  #define	LRADC_CTRL1				0x10
>  #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_MX28_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_MX28_LRADC_IRQ_MASK		0x1fff

MIght just be easier to define this as

LRADC....IRQ_MASK(id) (((id) == MX23) ? 0x1ff : 0x1fff)

just like the MXS SSP driver does it. Then there won't be so much churn.

[...]

Best regards,
Marek Vasut

[PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

[Dan Carpenter]

On Wed, Sep 04, 2013 at 03:01:44PM +0200, Juergen Beisert wrote:
> @@ -323,10 +338,17 @@ static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
>  	uint32_t reg;
>  
>  	/* Enable touch detection. */
> -	writel(LRADC_CTRL0_MX28_PLATE_MASK,
> -		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> -	writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
> -		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +	if (lradc->soc == IMX28_LRADC) {
> +		writel(LRADC_CTRL0_MX28_PLATE_MASK,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +		writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +	} else {
> +		writel(LRADC_CTRL0_MX23_PLATE_MASK,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +		writel(LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE,
> +			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
> +	}

I don't like the way this patch takes the driver and makes every line
an if else statement.  It would be cleaner to do this:

	writel(lradc_plate_mask(lradc),
		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
	writel(lradc_touch_detect_enable(lradc),
		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);

Btw, LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE just enables the touch screen
to detect touches?  It seems like we could leave the "DETECT_" out of
the name.

Actually it would better yet if there were a function:

static inline void lradc_writel(struct mxs_lradc *lradc, u32 val,
				size_t chan, size_t offset)
{
	writel(val, lradc->base + chan + offset);
}

That way we could do:

	lradc_writel(lradc_enable_touch(lradc), LRADC_CTRL0,
		     STMP_OFFSET_REG_SET);

ACTUALLY!  When I look at it now the third argument is almost always
"set", "clear" or "toggle".

So we could do:

static inline void lradc_reg_set(struct mxs_lradc *lradc, u32 val,
				size_t chan)
{
	writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
}

So then it would be:

	lradc_reg_clear(lradc_plate_mask(lradc), LRADC_CTRL0);
	lradc_reg_set(lradc_enable_touch(lradc), LRADC_CTRL0);

I've just changed 11 lines of code down to 2 lines of code by hiding the
if statement in the header file.

Please redo this patch along those lines.

regards,
dan carpenter

[PATCH 2/5] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits

[Dan Carpenter]

On Wed, Sep 04, 2013 at 03:01:43PM +0200, Juergen Beisert wrote:
> +#define	LRADC_CTRL0_MX28_PLATE_MASK \
> +		LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
> +		LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
> +		LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW

Put parenthesis are this macro.

regards,
dan carpenter

[PATCH 5/5] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation

[Dan Carpenter]

On Wed, Sep 04, 2013 at 03:01:46PM +0200, Juergen Beisert wrote:
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> CC: linux-arm-kernel at lists.infradead.org
> CC: devel at driverdev.osuosl.org
> CC: Marek Vasut <marex@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> CC: Jonathan Cameron <jic23@cam.ac.uk>
> ---

The change log is missing.

regards,
dan carpenter

[PATCH 5/5] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation

[Jürgen Beisert]

Hi Dan,

On Wednesday 04 September 2013 16:37:14 Dan Carpenter wrote:
> On Wed, Sep 04, 2013 at 03:01:46PM +0200, Juergen Beisert wrote:
> > Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> > CC: linux-arm-kernel at lists.infradead.org
> > CC: devel at driverdev.osuosl.org
> > CC: Marek Vasut <marex@denx.de>
> > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > CC: Jonathan Cameron <jic23@cam.ac.uk>
> > ---
>
> The change log is missing.

Ups, sorry I missed to add all the CCs in the envelope too. Will do in the next 
turn of this patch set.

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/  |

[PATCH 2/5] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits

[Jürgen Beisert]

Hi Marek,

On Wednesday 04 September 2013 16:06:34 Marek Vasut wrote:
> [...]
> > +# define LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE	(1 << 23)
> > +# define LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE	(1 << 22)
> > +# define LRADC_CTRL0_MX28_YNNSW	/* YM */	(1 << 21)
> > +# define LRADC_CTRL0_MX28_YPNSW	/* YP */	(1 << 20)
> > +# define LRADC_CTRL0_MX28_YPPSW	/* YP */	(1 << 19)
> > +# define LRADC_CTRL0_MX28_XNNSW	/* XM */	(1 << 18)
> > +# define LRADC_CTRL0_MX28_XNPSW	/* XM */	(1 << 17)
> > +# define LRADC_CTRL0_MX28_XPPSW	/* XP */	(1 << 16)
>
> Why do you put this space between # and define?

Matter of taste: to visible show a difference between registers itself and
their bits inside. Its my default state to help readers to read my code.

> > [...]
> >  #define	LRADC_CTRL1				0x10
> >  #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_MX28_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_MX28_LRADC_IRQ_MASK		0x1fff
>
> MIght just be easier to define this as
>
> LRADC....IRQ_MASK(id) (((id) == MX23) ? 0x1ff : 0x1fff)
>
> just like the MXS SSP driver does it. Then there won't be so much churn.

Nice. Will change it.

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/  |

[PATCH 2/5] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits

[Jürgen Beisert]

Hi Dan,

On Wednesday 04 September 2013 16:36:24 Dan Carpenter wrote:
> On Wed, Sep 04, 2013 at 03:01:43PM +0200, Juergen Beisert wrote:
> > +#define	LRADC_CTRL0_MX28_PLATE_MASK \
> > +		LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
> > +		LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
> > +		LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW
>
> Put parenthesis are this macro.

Good catch. Thanks.

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/  |

[PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

[Jürgen Beisert]

Hi Dan,

On Wednesday 04 September 2013 16:27:39 Dan Carpenter wrote:
> [...]
> ACTUALLY!  When I look at it now the third argument is almost always
> "set", "clear" or "toggle".
>
> So we could do:
>
> static inline void lradc_reg_set(struct mxs_lradc *lradc, u32 val,
> 				size_t chan)
> {
> 	writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
> }
>
> So then it would be:
>
> 	lradc_reg_clear(lradc_plate_mask(lradc), LRADC_CTRL0);
> 	lradc_reg_set(lradc_enable_touch(lradc), LRADC_CTRL0);
>
> I've just changed 11 lines of code down to 2 lines of code by hiding the
> if statement in the header file.
>
> Please redo this patch along those lines.

I like this simplification, but all the other drivers for the MXS series of 
SoCs don't use such a method. Do you think this "new style" will be accepted?

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/  |

[PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

[Dan Carpenter]

On Thu, Sep 05, 2013 at 12:16:18PM +0200, J?rgen Beisert wrote:
> I like this simplification, but all the other drivers for the MXS series of 
> SoCs don't use such a method. Do you think this "new style" will be accepted?
> 

My lradc_plate_mask() is basically the same idea as what Marek said
about putting the logic into the define.

LRADC....IRQ_MASK(id) (((id) == MX23) ? 0x1ff : 0x1fff)

I don't know why Marek is complaining about churn though.  This is
linux!  We love churn!  And especially in staging, it's our favorite
thing.  :P

regards,
dan carpenter

[PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

[Marek Vasut]

Dear Dan Carpenter,

> On Thu, Sep 05, 2013 at 12:16:18PM +0200, J?rgen Beisert wrote:
> > I like this simplification, but all the other drivers for the MXS series
> > of SoCs don't use such a method. Do you think this "new style" will be
> > accepted?
> 
> My lradc_plate_mask() is basically the same idea as what Marek said
> about putting the logic into the define.
> 
> LRADC....IRQ_MASK(id) (((id) == MX23) ? 0x1ff : 0x1fff)
> 
> I don't know why Marek is complaining about churn though.  This is
> linux!  We love churn!  And especially in staging, it's our favorite
> thing.  :P

About time to move this driver out of staging then ;-)

Best regards,
Marek Vasut

[PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

[Dan Carpenter]

Since I work in staging I review hundreds of churn patches per cycle.
One thing which helps me is my rename_rev.pl script (attached).

So if I get a patch like the one I'm about to send which introduces
lradc_reg_set/clear() functions, then I can do this:

cat diff | rename_rev.pl -ea 's/,\n/,/' | \
	rename_rev.pl -e 's/writel\((.*?),\s+lradc->base \+ (.*?) .*SET\)/lradc_reg_set(lradc, $1, $2)/' \
		      -e 's/writel\((.*?),\s+lradc->base \+ (.*?) .*CLR\)/lradc_reg_clear(lradc, $1, $2)/'

It simplifies the patch down to its most minimal form.

I should probably automate the first part which strips out some
line breaks...  I'll post a new version of rename_rev.pl to LKML soon.

regards,
dan carpenter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: rename_rev.pl
Type: text/x-perl
Size: 4221 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130905/edde38f4/attachment.bin>

[patch] iio: mxs-lradc: use helper functions to simplify the code

[Dan Carpenter]

I have introduced lradc_reg_set() and lradc_reg_clear().  It simplifies
the callers and makes the lines shorter.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index a08c173..da5d04b 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -228,6 +228,16 @@ struct mxs_lradc {
 #define LRADC_RESOLUTION			12
 #define LRADC_SINGLE_SAMPLE_MASK		((1 << LRADC_RESOLUTION) - 1)
 
+static void lradc_reg_set(struct mxs_lradc *lradc, u32 val, size_t chan)
+{
+	writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
+}
+
+static void lradc_reg_clear(struct mxs_lradc *lradc, u32 val, size_t chan)
+{
+	writel(val, lradc->base + chan + STMP_OFFSET_REG_CLR);
+}
+
 /*
  * Raw I/O operations
  */
@@ -262,21 +272,18 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	 * Virtual channel 0 is always used here as the others are always not
 	 * used if doing raw sampling.
 	 */
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
-	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN_MASK, LRADC_CTRL1);
+	lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 
 	/* 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);
+	lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(0), LRADC_CTRL4);
+	lradc_reg_set(lradc, chan->channel, LRADC_CTRL4);
 
 	writel(0, lradc->base + LRADC_CH(0));
 
 	/* Enable the IRQ and start sampling the channel. */
-	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
-	writel(1 << 0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0), LRADC_CTRL1);
+	lradc_reg_set(lradc, 1 << 0, LRADC_CTRL0);
 
 	/* Wait for completion on the channel, 1 second max. */
 	ret = wait_for_completion_killable_timeout(&lradc->completion, HZ);
@@ -290,8 +297,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	ret = IIO_VAL_INT;
 
 err:
-	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0), LRADC_CTRL1);
 
 	mutex_unlock(&lradc->lock);
 
@@ -317,10 +323,8 @@ 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);
+	lradc_reg_clear(lradc, LRADC_CTRL0_PLATE_MASK, LRADC_CTRL0);
+	lradc_reg_set(lradc, LRADC_CTRL0_TOUCH_DETECT_ENABLE, LRADC_CTRL0);
 
 	msleep(LRADC_TS_SAMPLE_DELAY_MS);
 
@@ -379,18 +383,18 @@ static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
 	}
 
 	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);
+		lradc_reg_clear(lradc, LRADC_CTRL0_PLATE_MASK, LRADC_CTRL0);
+		lradc_reg_set(lradc, ctrl0, LRADC_CTRL0);
+
+		lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(slot),
+				LRADC_CTRL4);
+		lradc_reg_set(lradc,
+			      chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
+			      LRADC_CTRL4);
 	}
 
-	writel(0xffffffff, lradc->base + LRADC_CH(slot) + STMP_OFFSET_REG_CLR);
-	writel(1 << slot, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	lradc_reg_clear(lradc, 0xffffffff, LRADC_CH(slot));
+	lradc_reg_set(lradc, 1 << slot, LRADC_CTRL0);
 
 	delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
 	do {
@@ -400,8 +404,7 @@ static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
 			break;
 	} while (time_before(jiff, delay));
 
-	writel(LRADC_CTRL1_LRADC_IRQ(slot),
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(slot), LRADC_CTRL1);
 
 	if (time_after_eq(jiff, delay))
 		return -ETIMEDOUT;
@@ -440,8 +443,8 @@ static void mxs_lradc_ts_work(struct work_struct *ts_work)
 
 	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);
+		lradc_reg_clear(lradc, LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+				LRADC_CTRL0);
 
 		if (likely(valid)) {
 			input_report_abs(lradc->ts_input, ABS_X, val_x);
@@ -475,10 +478,8 @@ static void mxs_lradc_ts_work(struct work_struct *ts_work)
 		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);
+	lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
+	lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
 }
 
 static int mxs_lradc_ts_open(struct input_dev *dev)
@@ -489,12 +490,10 @@ static int mxs_lradc_ts_open(struct input_dev *dev)
 	lradc->stop_touchscreen = false;
 
 	/* Enable the touch-detect circuitry. */
-	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+	lradc_reg_set(lradc, LRADC_CTRL0_TOUCH_DETECT_ENABLE, LRADC_CTRL0);
 
 	/* Enable the touch-detect IRQ. */
-	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+	lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
 
 	return 0;
 }
@@ -511,12 +510,10 @@ static void mxs_lradc_ts_close(struct input_dev *dev)
 	cancel_work_sync(&lradc->ts_work);
 
 	/* Disable touchscreen touch-detect IRQ. */
-	writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
 
 	/* Power-down touchscreen touch-detect circuitry. */
-	writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
-		lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL0_TOUCH_DETECT_ENABLE, LRADC_CTRL0);
 }
 
 static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
@@ -588,8 +585,7 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 	 * it ASAP
 	 */
 	if (reg & LRADC_CTRL1_TOUCH_DETECT_IRQ) {
-		writel(ts_irq_mask,
-			lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+		lradc_reg_clear(lradc, ts_irq_mask, LRADC_CTRL1);
 		if (!lradc->stop_touchscreen)
 			schedule_work(&lradc->ts_work);
 	}
@@ -599,8 +595,7 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
 		complete(&lradc->completion);
 
-	writel(reg & LRADC_CTRL1_LRADC_IRQ_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, reg & LRADC_CTRL1_LRADC_IRQ_MASK, LRADC_CTRL1);
 
 	return IRQ_HANDLED;
 }
@@ -642,9 +637,11 @@ static int mxs_lradc_configure_trigger(struct iio_trigger *trig, bool state)
 {
 	struct iio_dev *iio = iio_trigger_get_drvdata(trig);
 	struct mxs_lradc *lradc = iio_priv(iio);
-	const uint32_t st = state ? STMP_OFFSET_REG_SET : STMP_OFFSET_REG_CLR;
 
-	writel(LRADC_DELAY_KICK, lradc->base + LRADC_DELAY(0) + st);
+	if (state)
+		lradc_reg_set(lradc, LRADC_DELAY_KICK, LRADC_DELAY(0));
+	else
+		lradc_reg_clear(lradc, LRADC_DELAY_KICK,  LRADC_DELAY(0));
 
 	return 0;
 }
@@ -720,9 +717,8 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 	if (ret < 0)
 		goto err_buf;
 
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
-	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN_MASK, LRADC_CTRL1);
+	lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 
 	for_each_set_bit(chan, iio->active_scan_mask, LRADC_MAX_TOTAL_CHANS) {
 		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
@@ -733,16 +729,17 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 		ofs++;
 	}
 
-	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
-		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc,
+			LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
+			LRADC_DELAY(0));
 
-	writel(ctrl4_clr, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
-	writel(ctrl4_set, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
+	lradc_reg_clear(lradc, ctrl4_clr, LRADC_CTRL4);
+	lradc_reg_set(lradc, ctrl4_set, LRADC_CTRL4);
 
-	writel(ctrl1_irq, lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+	lradc_reg_set(lradc, ctrl1_irq, LRADC_CTRL1);
 
-	writel(enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
-		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_SET);
+	lradc_reg_set(lradc, enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
+		      LRADC_DELAY(0));
 
 	return 0;
 
@@ -757,12 +754,12 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
 {
 	struct mxs_lradc *lradc = iio_priv(iio);
 
-	writel(LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
-		lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc,
+			LRADC_DELAY_TRIGGER_LRADCS_MASK | LRADC_DELAY_KICK,
+			LRADC_DELAY(0));
 
-	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
+	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN_MASK, LRADC_CTRL1);
 
 	kfree(lradc->buffer);
 	mutex_unlock(&lradc->lock);
@@ -865,12 +862,11 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
 	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);
+	lradc_reg_clear(lradc, LRADC_CTRL0_TOUCH_SCREEN_TYPE, LRADC_CTRL0);
 
 	if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
-		writel(LRADC_CTRL0_TOUCH_SCREEN_TYPE,
-			lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+		lradc_reg_set(lradc, LRADC_CTRL0_TOUCH_SCREEN_TYPE,
+			      LRADC_CTRL0);
 	}
 
 	/* Start internal temperature sensing. */
@@ -883,8 +879,7 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
 {
 	int i;
 
-	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
-		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN_MASK, LRADC_CTRL1);
 
 	for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
 		writel(0, lradc->base + LRADC_DELAY(i));

[patch] iio: mxs-lradc: use helper functions to simplify the code

[Fabio Estevam]

On Thu, Sep 5, 2013 at 3:16 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> I have introduced lradc_reg_set() and lradc_reg_clear().  It simplifies
> the callers and makes the lines shorter.

Looks good, just one minor suggestion:

> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index a08c173..da5d04b 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -228,6 +228,16 @@ struct mxs_lradc {
>  #define LRADC_RESOLUTION                       12
>  #define LRADC_SINGLE_SAMPLE_MASK               ((1 << LRADC_RESOLUTION) - 1)
>
> +static void lradc_reg_set(struct mxs_lradc *lradc, u32 val, size_t chan)
> +{
> +       writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
> +}
> +
> +static void lradc_reg_clear(struct mxs_lradc *lradc, u32 val, size_t chan)
> +{
> +       writel(val, lradc->base + chan + STMP_OFFSET_REG_CLR);
> +}

Maybe 'static inline' ?

[patch] iio: mxs-lradc: use helper functions to simplify the code

[Marek Vasut]

Dear Fabio Estevam,

> On Thu, Sep 5, 2013 at 3:16 PM, Dan Carpenter <dan.carpenter@oracle.com> 
wrote:
> > I have introduced lradc_reg_set() and lradc_reg_clear().  It simplifies
> > the callers and makes the lines shorter.
> 
> Looks good, just one minor suggestion:
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> > b/drivers/staging/iio/adc/mxs-lradc.c index a08c173..da5d04b 100644
> > --- a/drivers/staging/iio/adc/mxs-lradc.c
> > +++ b/drivers/staging/iio/adc/mxs-lradc.c
> > @@ -228,6 +228,16 @@ struct mxs_lradc {
> > 
> >  #define LRADC_RESOLUTION                       12
> >  #define LRADC_SINGLE_SAMPLE_MASK               ((1 << LRADC_RESOLUTION)
> >  - 1)
> > 
> > +static void lradc_reg_set(struct mxs_lradc *lradc, u32 val, size_t chan)
> > +{
> > +       writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
> > +}
> > +
> > +static void lradc_reg_clear(struct mxs_lradc *lradc, u32 val, size_t
> > chan) +{
> > +       writel(val, lradc->base + chan + STMP_OFFSET_REG_CLR);
> > +}
> 
> Maybe 'static inline' ?

static only, the inline is meaningless, GCC will handle that.

Best regards,
Marek Vasut

[patch] iio: mxs-lradc: use helper functions to simplify the code

[Jürgen Beisert]

Hi Dan,

On Thursday 05 September 2013 20:16:39 Dan Carpenter wrote:
> I have introduced lradc_reg_set() and lradc_reg_clear().  It simplifies
> the callers and makes the lines shorter.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index a08c173..da5d04b 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -228,6 +228,16 @@ struct mxs_lradc {
>  #define LRADC_RESOLUTION			12
>  #define LRADC_SINGLE_SAMPLE_MASK		((1 << LRADC_RESOLUTION) - 1)
>
> +static void lradc_reg_set(struct mxs_lradc *lradc, u32 val, size_t chan)
> +{
> +	writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
> +}
> +
> +static void lradc_reg_clear(struct mxs_lradc *lradc, u32 val, size_t chan)
> +{
> +	writel(val, lradc->base + chan + STMP_OFFSET_REG_CLR);
> +}
> +
>  /*
>   * Raw I/O operations
>   */
> @@ -262,21 +272,18 @@ static int mxs_lradc_read_raw(struct iio_dev
> *iio_dev, * Virtual channel 0 is always used here as the others are always
> not * used if doing raw sampling.
>  	 */
> -	writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
> -		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> -	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> +	lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN_MASK, LRADC_CTRL1);
> +	lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);

[...]

Oh, I did the same yesterday in preparation of the v2 of my patch series.
I will send this new version out today.

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/  |

[patch] iio: mxs-lradc: use helper functions to simplify the code

[Dan Carpenter]

On Fri, Sep 06, 2013 at 09:01:01AM +0200, J?rgen Beisert wrote:
> 
> Oh, I did the same yesterday in preparation of the v2 of my patch series.
> I will send this new version out today.
> 

Ah fine.  Then let's drop my version.

regards,
dan carpenter

[patch] iio: mxs-lradc: use helper functions to simplify the code

[Russell King - ARM Linux]

On Thu, Sep 05, 2013 at 05:15:02PM -0300, Fabio Estevam wrote:
> Looks good, just one minor suggestion:
> 
> On Thu, Sep 5, 2013 at 3:16 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > +static void lradc_reg_set(struct mxs_lradc *lradc, u32 val, size_t chan)
> > +{
> > +       writel(val, lradc->base + chan + STMP_OFFSET_REG_SET);
> > +}
> > +
> > +static void lradc_reg_clear(struct mxs_lradc *lradc, u32 val, size_t chan)
> > +{
> > +       writel(val, lradc->base + chan + STMP_OFFSET_REG_CLR);
> > +}
> 
> Maybe 'static inline' ?

GCC will already inline these if it thinks they're appropriate for that
treatment.