* [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation
@ 2013-09-11 8:18 Juergen Beisert
2013-09-11 8:18 ` [PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC Juergen Beisert
` (5 more replies)
0 siblings, 6 replies; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
The following series replaces the current busy loop touchscreen implementation
for i.MX28/i.MX23 SoCs by a fully interrupt driven implementation.
Since i.MX23 and i.Mx28 silicon differs, the existing implementation can
be used for the i.MX28 SoC only.
So, the first two patches of this series move the i.MX28 specific definitions
out of the way. The third patch simplifies the register access to make it easier
to add the i.MX23 support. Then the i.MX23 specific definitions are added, also
the code to distinguish both SoCs at run-time.
Up to here the existing touchscreen driver will now run on an i.MX23 Soc as well.
When the i.MX23 SoC is running from battery it seems not to be a good idea to
run a busy loop to detect touches and their location. The fourth patch adds a
fully interrupt driven implementation which makes use of the built-in delay
and multiple sample features of the touchscreen controller. This will reduce
the interrupt load to a minimum.
The last patch in this series just removes the existing busy loop
implementation.
Still some restrictions/questions:
- the touchscreen part is yet tested on i.MX23 SoC only
- touchscreen parametrisation ability is provided (with fixed values for now)
but should be done via device tree. Some recommendations how to define the
bindings would be helpful
- has someone a good idea how to implement a reliable pressure measurement if
the resistances of the touchscree's plates are unknown?
Changes since v3:
- split adding register access functions and i.MX23 support into two patches
Changes since v2:
- useless debug output removed
Changes since v1:
- adding register access functions to make the existing code more readable
- adding some functions to distinguish the SoCs at run-time to avoid if-else
contructs whenever differences in the register layout between i.MX23 and
i.MX28 must be handled
Comments are welcome.
Juergen
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
@ 2013-09-11 8:18 ` Juergen Beisert
2013-09-11 8:18 ` [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits Juergen Beisert
` (4 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
2013-09-11 8:18 ` [PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC Juergen Beisert
@ 2013-09-11 8:18 ` Juergen Beisert
2013-09-15 10:27 ` Jonathan Cameron
2013-09-11 8:18 ` [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
` (3 subsequent siblings)
5 siblings, 1 reply; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
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 | 61 ++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index dffca90..5535fed 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -179,24 +179,29 @@ 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_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_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 +269,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 +324,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 +372,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 +447,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 +496,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 +522,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 +586,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 +606,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 +727,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 +768,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 +872,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 +890,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
2013-09-11 8:18 ` [PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC Juergen Beisert
2013-09-11 8:18 ` [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits Juergen Beisert
@ 2013-09-11 8:18 ` Juergen Beisert
2013-09-15 10:35 ` Jonathan Cameron
2013-09-16 14:15 ` Marek Vasut
2013-09-11 8:18 ` [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver Juergen Beisert
` (2 subsequent siblings)
5 siblings, 2 replies; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
Replace the individual register access by a few shared access function to make the
code easier to read and in order to add the i.MX23 SoC in the next step.
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 | 204 +++++++++++++++++++++---------------
1 file changed, 120 insertions(+), 84 deletions(-)
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 5535fed..4317fee 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -235,6 +235,56 @@ struct mxs_lradc {
#define LRADC_RESOLUTION 12
#define LRADC_SINGLE_SAMPLE_MASK ((1 << LRADC_RESOLUTION) - 1)
+static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
+{
+ writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
+}
+
+static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
+{
+ writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
+}
+
+static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 val, u32 reg)
+{
+ writel(val, lradc->base + reg);
+}
+
+static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL0_MX28_PLATE_MASK;
+}
+
+static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
+}
+
+static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL1_MX28_LRADC_IRQ_MASK;
+}
+
+static u32 mxs_lradc_touch_detect_bit(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE;
+}
+
+static u32 mxs_lradc_drive_x_plate(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
+}
+
+static u32 mxs_lradc_drive_y_plate(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
+}
+
+static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
+{
+ return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
+}
+
/*
* Raw I/O operations
*/
@@ -269,21 +319,19 @@ 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);
- writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+ LRADC_CTRL1);
+ mxs_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);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(0), LRADC_CTRL4);
+ mxs_lradc_reg_set(lradc, chan->channel, LRADC_CTRL4);
- writel(0, lradc->base + LRADC_CH(0));
+ mxs_lradc_reg(lradc, 0, 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);
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0), LRADC_CTRL1);
+ mxs_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);
@@ -297,8 +345,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);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0), LRADC_CTRL1);
mutex_unlock(&lradc->lock);
@@ -324,10 +371,9 @@ 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);
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+ mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
+ LRADC_CTRL0);
msleep(LRADC_TS_SAMPLE_DELAY_MS);
@@ -372,32 +418,33 @@ 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;
+ ctrl0 = mxs_lradc_drive_x_plate(lradc);
chan = 3;
break;
case LRADC_SAMPLE_Y:
- ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
+ ctrl0 = mxs_lradc_drive_y_plate(lradc);
chan = 4;
break;
case LRADC_SAMPLE_PRESSURE:
- ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
+ ctrl0 = mxs_lradc_drive_pressure(lradc);
chan = 5;
break;
}
if (change) {
- writel(LRADC_CTRL0_MX28_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);
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc),
+ LRADC_CTRL0);
+ mxs_lradc_reg_set(lradc, ctrl0, LRADC_CTRL0);
+
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(slot),
+ LRADC_CTRL4);
+ mxs_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);
+ mxs_lradc_reg_clear(lradc, 0xffffffff, LRADC_CH(slot));
+ mxs_lradc_reg_set(lradc, 1 << slot, LRADC_CTRL0);
delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
do {
@@ -407,8 +454,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);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(slot), LRADC_CTRL1);
if (time_after_eq(jiff, delay))
return -ETIMEDOUT;
@@ -447,8 +493,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_MX28_TOUCH_DETECT_ENABLE,
- lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, mxs_lradc_touch_detect_bit(lradc),
+ LRADC_CTRL0);
if (likely(valid)) {
input_report_abs(lradc->ts_input, ABS_X, val_x);
@@ -482,10 +528,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);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
}
static int mxs_lradc_ts_open(struct input_dev *dev)
@@ -496,12 +540,11 @@ 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);
+ mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
+ LRADC_CTRL0);
/* Enable the touch-detect IRQ. */
- writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
- lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
return 0;
}
@@ -518,12 +561,11 @@ 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);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+ LRADC_CTRL1);
/* Power-down touchscreen touch-detect circuitry. */
- writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
- lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, mxs_lradc_touch_detect_bit(lradc), LRADC_CTRL0);
}
static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
@@ -586,7 +628,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_MX28_LRADC_IRQ_MASK))
+ if (!(reg & mxs_lradc_irq_mask(lradc)))
return IRQ_NONE;
/*
@@ -595,8 +637,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);
+ mxs_lradc_reg_clear(lradc, ts_irq_mask, LRADC_CTRL1);
if (!lradc->stop_touchscreen)
schedule_work(&lradc->ts_work);
}
@@ -606,8 +647,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_MX28_LRADC_IRQ_MASK,
- lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, reg & mxs_lradc_irq_mask(lradc), LRADC_CTRL1);
return IRQ_HANDLED;
}
@@ -626,7 +666,7 @@ static irqreturn_t mxs_lradc_trigger_handler(int irq, void *p)
for_each_set_bit(i, iio->active_scan_mask, LRADC_MAX_TOTAL_CHANS) {
lradc->buffer[j] = readl(lradc->base + LRADC_CH(j));
- writel(chan_value, lradc->base + LRADC_CH(j));
+ mxs_lradc_reg(lradc, chan_value, LRADC_CH(j));
lradc->buffer[j] &= LRADC_CH_VALUE_MASK;
lradc->buffer[j] /= LRADC_DELAY_TIMER_LOOP;
j++;
@@ -651,7 +691,7 @@ static int mxs_lradc_configure_trigger(struct iio_trigger *trig, bool state)
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);
+ mxs_lradc_reg(lradc, LRADC_DELAY_KICK, LRADC_DELAY(0) + st);
return 0;
}
@@ -727,29 +767,26 @@ 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);
- writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+ LRADC_CTRL1);
+ mxs_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);
ctrl4_clr |= LRADC_CTRL4_LRADCSELECT_MASK(ofs);
ctrl1_irq |= LRADC_CTRL1_LRADC_IRQ_EN(ofs);
- writel(chan_value, lradc->base + LRADC_CH(ofs));
+ mxs_lradc_reg(lradc, chan_value, LRADC_CH(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_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,
- lradc->base + LRADC_DELAY(0) + STMP_OFFSET_REG_SET);
+ mxs_lradc_reg_clear(lradc, LRADC_DELAY_TRIGGER_LRADCS_MASK |
+ LRADC_DELAY_KICK, LRADC_DELAY(0));
+ mxs_lradc_reg_clear(lradc, ctrl4_clr, LRADC_CTRL4);
+ mxs_lradc_reg_set(lradc, ctrl4_set, LRADC_CTRL4);
+ mxs_lradc_reg_set(lradc, ctrl1_irq, LRADC_CTRL1);
+ mxs_lradc_reg_set(lradc, enable << LRADC_DELAY_TRIGGER_LRADCS_OFFSET,
+ LRADC_DELAY(0));
return 0;
@@ -764,12 +801,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);
+ mxs_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_MX28_LRADC_IRQ_EN_MASK,
- lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+ LRADC_CTRL1);
kfree(lradc->buffer);
mutex_unlock(&lradc->lock);
@@ -864,24 +901,24 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
return ret;
/* Configure DELAY CHANNEL 0 for generic ADC sampling. */
- writel(adc_cfg, lradc->base + LRADC_DELAY(0));
+ mxs_lradc_reg(lradc, adc_cfg, 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));
+ mxs_lradc_reg(lradc, 0, LRADC_DELAY(1));
+ mxs_lradc_reg(lradc, 0, LRADC_DELAY(2));
+ mxs_lradc_reg(lradc, 0, LRADC_DELAY(3));
/* Configure the touchscreen type */
- writel(LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
- lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
+ LRADC_CTRL0);
- if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE) {
- writel(LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
- lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+ if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
+ mxs_lradc_reg_set(lradc, LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
+ LRADC_CTRL0);
}
/* Start internal temperature sensing. */
- writel(0, lradc->base + LRADC_CTRL2);
+ mxs_lradc_reg(lradc, 0, LRADC_CTRL2);
return 0;
}
@@ -890,11 +927,10 @@ 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);
+ mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
- writel(0, lradc->base + LRADC_DELAY(i));
+ mxs_lradc_reg(lradc, 0, LRADC_DELAY(i));
}
static const struct of_device_id mxs_lradc_dt_ids[] = {
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
` (2 preceding siblings ...)
2013-09-11 8:18 ` [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
@ 2013-09-11 8:18 ` Juergen Beisert
2013-09-15 10:50 ` Jonathan Cameron
2013-09-11 8:18 ` [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection Juergen Beisert
2013-09-11 8:18 ` [PATCH 6/6] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation Juergen Beisert
5 siblings, 1 reply; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC at least
for the 4 wire touchscreen.
Note: support for the remaining LRADC channels is not tested on an
i.MX23 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 | 45 +++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 4317fee..c81b186 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -188,20 +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
@@ -252,36 +265,50 @@ static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 val, u32 reg)
static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL0_MX23_PLATE_MASK;
return LRADC_CTRL0_MX28_PLATE_MASK;
}
static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
}
static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL1_MX23_LRADC_IRQ_MASK;
return LRADC_CTRL1_MX28_LRADC_IRQ_MASK;
}
static u32 mxs_lradc_touch_detect_bit(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE;
return LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE;
}
static u32 mxs_lradc_drive_x_plate(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL0_MX23_XP | LRADC_CTRL0_MX23_XM;
return LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
}
static u32 mxs_lradc_drive_y_plate(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_YM;
return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
}
static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
{
+ if (lradc->soc == IMX23_LRADC)
+ return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
}
@@ -319,7 +346,8 @@ 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.
*/
- mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+ if (lradc->soc == IMX28_LRADC)
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
LRADC_CTRL1);
mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
@@ -767,7 +795,8 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
if (ret < 0)
goto err_buf;
- mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+ if (lradc->soc == IMX28_LRADC)
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
LRADC_CTRL1);
mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
@@ -805,7 +834,8 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
LRADC_DELAY_KICK, LRADC_DELAY(0));
mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
- mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+ if (lradc->soc == IMX28_LRADC)
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
LRADC_CTRL1);
kfree(lradc->buffer);
@@ -909,7 +939,8 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
mxs_lradc_reg(lradc, 0, LRADC_DELAY(3));
/* Configure the touchscreen type */
- mxs_lradc_reg_clear(lradc, LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
+ if (lradc->soc == IMX28_LRADC) {
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
LRADC_CTRL0);
if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
@@ -987,6 +1018,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
` (3 preceding siblings ...)
2013-09-11 8:18 ` [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver Juergen Beisert
@ 2013-09-11 8:18 ` Juergen Beisert
2013-09-15 10:56 ` Jonathan Cameron
2013-09-11 8:18 ` [PATCH 6/6] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation Juergen Beisert
5 siblings, 1 reply; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
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 an 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 | 530 ++++++++++++++++++++++++++++++++----
1 file changed, 476 insertions(+), 54 deletions(-)
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index c81b186..185f068 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))
@@ -312,6 +349,404 @@ static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
}
+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."
+ */
+ mxs_lradc_reg(lradc, LRADC_CH_ACCUMULATE |
+ LRADC_CH_NUM_SAMPLES(lradc->over_sample_cnt - 1),
+ LRADC_CH(ch));
+
+ /* from the datasheet:
+ * "Software must clear this register in preparation for a
+ * multi-cycle accumulation.
+ */
+ mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch));
+
+ /* prepare the delay/loop unit according to the oversampling count */
+ mxs_lradc_reg(lradc, 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_DELAY(3));
+
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(2) |
+ LRADC_CTRL1_LRADC_IRQ(3) | LRADC_CTRL1_LRADC_IRQ(4) |
+ LRADC_CTRL1_LRADC_IRQ(5), LRADC_CTRL1);
+
+ /* wake us again, when the complete conversion is done */
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(ch), LRADC_CTRL1);
+ /*
+ * 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.
+ */
+ mxs_lradc_reg(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+ LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
+ LRADC_DELAY_KICK |
+ LRADC_DELAY_DELAY(lradc->settling_delay),
+ 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);
+ mxs_lradc_reg(lradc, reg, LRADC_CH(ch1));
+ mxs_lradc_reg(lradc, reg, LRADC_CH(ch2));
+
+ /* from the datasheet:
+ * "Software must clear this register in preparation for a
+ * multi-cycle accumulation.
+ */
+ mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch1));
+ mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch2));
+
+ /* prepare the delay/loop unit according to the oversampling count */
+ mxs_lradc_reg(lradc, 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_DELAY(3));
+
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(2) |
+ LRADC_CTRL1_LRADC_IRQ(3) | LRADC_CTRL1_LRADC_IRQ(4) |
+ LRADC_CTRL1_LRADC_IRQ(5), LRADC_CTRL1);
+
+ /* wake us again, when the conversions are done */
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(ch2), LRADC_CTRL1);
+ /*
+ * 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.
+ */
+ mxs_lradc_reg(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+ LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
+ LRADC_DELAY_KICK |
+ LRADC_DELAY_DELAY(lradc->settling_delay), 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 (m2 == 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)) {
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_YP) |
+ LRADC_CTRL1_LRADC_IRQ(TS_CH_YP), LRADC_CTRL1);
+ val = mxs_lradc_read_raw_channel(lradc, TS_CH_YP);
+ } else if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_XM)) {
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_XM) |
+ LRADC_CTRL1_LRADC_IRQ(TS_CH_XM), LRADC_CTRL1);
+ val = mxs_lradc_read_raw_channel(lradc, TS_CH_XM);
+ } else if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_YM)) {
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_YM) |
+ LRADC_CTRL1_LRADC_IRQ(TS_CH_YM), LRADC_CTRL1);
+ val = mxs_lradc_read_raw_channel(lradc, TS_CH_YM);
+ } else {
+ return -EIO;
+ }
+
+ mxs_lradc_reg(lradc, 0, LRADC_DELAY(2));
+ mxs_lradc_reg(lradc, 0, 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
+ */
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+ mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
+ LRADC_CTRL0);
+}
+
+/*
+ * 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)
+{
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+ mxs_lradc_reg_set(lradc, mxs_lradc_drive_x_plate(lradc), LRADC_CTRL0);
+
+ lradc->cur_plate = LRADC_SAMPLE_X;
+ mxs_lradc_setup_ts_channel(lradc, TS_CH_YP);
+}
+
+/*
+ * 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)
+{
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+ mxs_lradc_reg_set(lradc, mxs_lradc_drive_y_plate(lradc), LRADC_CTRL0);
+
+ lradc->cur_plate = LRADC_SAMPLE_Y;
+ mxs_lradc_setup_ts_channel(lradc, TS_CH_XM);
+}
+
+/*
+ * YP(+)--+-------------+
+ * | |--+
+ * | | |
+ * YM(meas)--+-------------+ |
+ * +--------------+
+ * | |
+ * XP(meas) XM(-)
+ *
+ * (+) means here 1.85 V
+ * (-) means here GND
+ */
+static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)
+{
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+ mxs_lradc_reg_set(lradc, mxs_lradc_drive_pressure(lradc), LRADC_CTRL0);
+
+ lradc->cur_plate = LRADC_SAMPLE_PRESSURE;
+ mxs_lradc_setup_ts_pressure(lradc, TS_CH_XP, TS_CH_YM);
+}
+
+static void mxs_lradc_enable_touch_detection(struct mxs_lradc *lradc)
+{
+ mxs_lradc_setup_touch_detection(lradc);
+
+ lradc->cur_plate = LRADC_TOUCH;
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ |
+ LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
+}
+
+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
+ */
+ mxs_lradc_reg(lradc, 0, LRADC_CH(5));
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(5), LRADC_CTRL1);
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(5), LRADC_CTRL1);
+ mxs_lradc_reg(lradc, LRADC_DELAY_TRIGGER(1 << 5) |
+ LRADC_DELAY_KICK | LRADC_DELAY_DELAY(10), /* waste 5 ms */
+ 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);
+ }
+
+ /* 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);
+ }
+
+ /* if it is released, wait for the next touch via IRQ */
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
+ mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
+}
+
+/* touchscreen's state machine */
+static void mxs_lradc_handle_touch(struct mxs_lradc *lradc)
+{
+ int val;
+
+ switch (lradc->cur_plate) {
+ case LRADC_TOUCH:
+ /*
+ * start with the Y-pos, because it uses nearly the same plate
+ * settings like the touch detection
+ */
+ if (mxs_lradc_check_touch_event(lradc)) {
+ mxs_lradc_reg_clear(lradc,
+ LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+ LRADC_CTRL1);
+ mxs_lradc_prepare_y_pos(lradc);
+ }
+ mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ,
+ LRADC_CTRL1);
+ return;
+
+ case LRADC_SAMPLE_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:
+ 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:
+ 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:
+ val = mxs_lradc_read_ts_channel(lradc); /* ignore the value */
+ mxs_lradc_finish_touch_event(lradc, 1);
+ break;
+ }
+}
+
/*
* Raw I/O operations
*/
@@ -385,15 +820,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;
@@ -551,10 +977,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. */
mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
@@ -564,36 +986,29 @@ 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. */
- mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
- LRADC_CTRL0);
-
- /* Enable the touch-detect IRQ. */
- mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
+ 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();
+ /* stop all interrupts from firing */
+ mxs_lradc_reg_clear(lradc, 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_CTRL1);
- /* Wait until touchscreen thread finishes any possible remnants. */
- cancel_work_sync(&lradc->ts_work);
+ /* Power-down touchscreen touch-detect circuitry. */
+ mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+}
- /* Disable touchscreen touch-detect IRQ. */
- mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
- LRADC_CTRL1);
+static void mxs_lradc_ts_close(struct input_dev *dev)
+{
+ struct mxs_lradc *lradc = input_get_drvdata(dev);
- /* Power-down touchscreen touch-detect circuitry. */
- mxs_lradc_reg_clear(lradc, mxs_lradc_touch_detect_bit(lradc), LRADC_CTRL0);
+ mxs_lradc_disable_ts(lradc);
}
static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
@@ -641,6 +1056,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);
}
@@ -653,30 +1069,23 @@ 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 (!(reg & mxs_lradc_irq_mask(lradc)))
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) {
- mxs_lradc_reg_clear(lradc, ts_irq_mask, LRADC_CTRL1);
- 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);
- mxs_lradc_reg_clear(lradc, reg & mxs_lradc_irq_mask(lradc), LRADC_CTRL1);
-
return IRQ_HANDLED;
}
@@ -971,6 +1380,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 =
@@ -983,7 +1403,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. */
@@ -1003,7 +1423,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",
@@ -1066,9 +1486,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 6/6] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
` (4 preceding siblings ...)
2013-09-11 8:18 ` [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection Juergen Beisert
@ 2013-09-11 8:18 ` Juergen Beisert
5 siblings, 0 replies; 28+ messages in thread
From: Juergen Beisert @ 2013-09-11 8:18 UTC (permalink / raw)
To: linux-arm-kernel
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 | 165 ------------------------------------
1 file changed, 165 deletions(-)
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 185f068..cb791c8 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 */
@@ -820,168 +819,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. */
- mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
- mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
- LRADC_CTRL0);
-
- 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 = mxs_lradc_drive_x_plate(lradc);
- chan = 3;
- break;
- case LRADC_SAMPLE_Y:
- ctrl0 = mxs_lradc_drive_y_plate(lradc);
- chan = 4;
- break;
- case LRADC_SAMPLE_PRESSURE:
- ctrl0 = mxs_lradc_drive_pressure(lradc);
- chan = 5;
- break;
- }
-
- if (change) {
- mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc),
- LRADC_CTRL0);
- mxs_lradc_reg_set(lradc, ctrl0, LRADC_CTRL0);
-
- mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(slot),
- LRADC_CTRL4);
- mxs_lradc_reg_set(lradc,
- chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
- LRADC_CTRL4);
- }
-
- mxs_lradc_reg_clear(lradc, 0xffffffff, LRADC_CH(slot));
- mxs_lradc_reg_set(lradc, 1 << slot, LRADC_CTRL0);
-
- 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));
-
- mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(slot), LRADC_CTRL1);
-
- 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. */
- mxs_lradc_reg_clear(lradc, mxs_lradc_touch_detect_bit(lradc),
- LRADC_CTRL0);
-
- 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. */
- mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
- mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
-}
-
static int mxs_lradc_ts_open(struct input_dev *dev)
{
struct mxs_lradc *lradc = input_get_drvdata(dev);
@@ -1054,8 +891,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits
2013-09-11 8:18 ` [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits Juergen Beisert
@ 2013-09-15 10:27 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
On 09/11/13 09:18, Juergen Beisert wrote:
> 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>
I'd personally have gone for LRADC_MX28_CTRL0_* but I'm not that fussed.
I am looking for an Acked-by from Marek for this series though.
> ---
> drivers/staging/iio/adc/mxs-lradc.c | 61 ++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index dffca90..5535fed 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -179,24 +179,29 @@ 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_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_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 +269,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 +324,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 +372,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 +447,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 +496,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 +522,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 +586,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 +606,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 +727,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 +768,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 +872,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 +890,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++)
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
2013-09-11 8:18 ` [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
@ 2013-09-15 10:35 ` Jonathan Cameron
2013-09-16 8:17 ` Jürgen Beisert
2013-09-16 14:15 ` Marek Vasut
1 sibling, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-15 10:35 UTC (permalink / raw)
To: linux-arm-kernel
On 09/11/13 09:18, Juergen Beisert wrote:
> Replace the individual register access by a few shared access function to make the
> code easier to read and in order to add the i.MX23 SoC in the next step.
>
> 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>
One little suggestion inline but I'm basically happy with this.
> ---
> drivers/staging/iio/adc/mxs-lradc.c | 204 +++++++++++++++++++++---------------
> 1 file changed, 120 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 5535fed..4317fee 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -235,6 +235,56 @@ struct mxs_lradc {
> #define LRADC_RESOLUTION 12
> #define LRADC_SINGLE_SAMPLE_MASK ((1 << LRADC_RESOLUTION) - 1)
>
> +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
> +{
> + writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
> +}
> +
> +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
> +{
> + writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
> +}
> +
mxs_lradc_reg_write might be clearer?
> +static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 val, u32 reg)
> +{
> + writel(val, lradc->base + reg);
> +}
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver
2013-09-11 8:18 ` [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver Juergen Beisert
@ 2013-09-15 10:50 ` Jonathan Cameron
2013-09-16 8:14 ` Jürgen Beisert
0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-15 10:50 UTC (permalink / raw)
To: linux-arm-kernel
On 09/11/13 09:18, Juergen Beisert wrote:
> Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC at least
> for the 4 wire touchscreen.
>
> Note: support for the remaining LRADC channels is not tested on an
> i.MX23 yet.
An ominous comment. Are you likely to test them soon?
Minor code layout comment inline but otherwise I'm just looking for an
ack from Marek.
>
> 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 | 45 +++++++++++++++++++++++++++++++++----
> 1 file changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 4317fee..c81b186 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -188,20 +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
> @@ -252,36 +265,50 @@ static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 val, u32 reg)
>
> static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL0_MX23_PLATE_MASK;
> return LRADC_CTRL0_MX28_PLATE_MASK;
> }
>
> static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
> return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
> }
>
> static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL1_MX23_LRADC_IRQ_MASK;
> return LRADC_CTRL1_MX28_LRADC_IRQ_MASK;
> }
>
> static u32 mxs_lradc_touch_detect_bit(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE;
> return LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE;
> }
>
> static u32 mxs_lradc_drive_x_plate(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL0_MX23_XP | LRADC_CTRL0_MX23_XM;
> return LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
> }
>
> static u32 mxs_lradc_drive_y_plate(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_YM;
> return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
> }
>
> static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
> {
> + if (lradc->soc == IMX23_LRADC)
> + return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
> return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
Whilst it obviously doesn't actually matter, having an else
in there would make the code more consistent so personally
I would prefer it to be there.
> }
>
> @@ -319,7 +346,8 @@ 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.
> */
> - mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
> + if (lradc->soc == IMX28_LRADC)
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
> LRADC_CTRL1);
> mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
>
> @@ -767,7 +795,8 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
> if (ret < 0)
> goto err_buf;
>
> - mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
> + if (lradc->soc == IMX28_LRADC)
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
> LRADC_CTRL1);
> mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
>
> @@ -805,7 +834,8 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
> LRADC_DELAY_KICK, LRADC_DELAY(0));
>
> mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
> - mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
> + if (lradc->soc == IMX28_LRADC)
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
> LRADC_CTRL1);
>
> kfree(lradc->buffer);
> @@ -909,7 +939,8 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
> mxs_lradc_reg(lradc, 0, LRADC_DELAY(3));
>
> /* Configure the touchscreen type */
> - mxs_lradc_reg_clear(lradc, LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
> + if (lradc->soc == IMX28_LRADC) {
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE,
> LRADC_CTRL0);
>
> if (lradc->use_touchscreen == MXS_LRADC_TOUCHSCREEN_5WIRE)
> @@ -987,6 +1018,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);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-11 8:18 ` [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection Juergen Beisert
@ 2013-09-15 10:56 ` Jonathan Cameron
2013-09-15 16:10 ` Jonathan Cameron
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-15 10:56 UTC (permalink / raw)
To: linux-arm-kernel
On 09/11/13 09:18, Juergen Beisert wrote:
> 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 an 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>
While this driver is placed in IIO within staging at the moment, these changes are definitely
input related. Hence I have cc'd Dmitry and the input list.
I am personaly a little uncomfortable that we have such a complex bit of input code sat
within an IIO driver but such is life.
> ---
> drivers/staging/iio/adc/mxs-lradc.c | 530 ++++++++++++++++++++++++++++++++----
> 1 file changed, 476 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index c81b186..185f068 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))
> @@ -312,6 +349,404 @@ static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
> return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
> }
>
> +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."
> + */
> + mxs_lradc_reg(lradc, LRADC_CH_ACCUMULATE |
> + LRADC_CH_NUM_SAMPLES(lradc->over_sample_cnt - 1),
> + LRADC_CH(ch));
> +
> + /* from the datasheet:
> + * "Software must clear this register in preparation for a
> + * multi-cycle accumulation.
> + */
> + mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch));
> +
> + /* prepare the delay/loop unit according to the oversampling count */
> + mxs_lradc_reg(lradc, 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_DELAY(3));
> +
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(2) |
> + LRADC_CTRL1_LRADC_IRQ(3) | LRADC_CTRL1_LRADC_IRQ(4) |
> + LRADC_CTRL1_LRADC_IRQ(5), LRADC_CTRL1);
> +
> + /* wake us again, when the complete conversion is done */
> + mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(ch), LRADC_CTRL1);
> + /*
> + * 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.
> + */
> + mxs_lradc_reg(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
> + LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
> + LRADC_DELAY_KICK |
> + LRADC_DELAY_DELAY(lradc->settling_delay),
> + 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);
> + mxs_lradc_reg(lradc, reg, LRADC_CH(ch1));
> + mxs_lradc_reg(lradc, reg, LRADC_CH(ch2));
> +
> + /* from the datasheet:
> + * "Software must clear this register in preparation for a
> + * multi-cycle accumulation.
> + */
> + mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch1));
> + mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch2));
> +
> + /* prepare the delay/loop unit according to the oversampling count */
> + mxs_lradc_reg(lradc, 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_DELAY(3));
> +
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(2) |
> + LRADC_CTRL1_LRADC_IRQ(3) | LRADC_CTRL1_LRADC_IRQ(4) |
> + LRADC_CTRL1_LRADC_IRQ(5), LRADC_CTRL1);
> +
> + /* wake us again, when the conversions are done */
> + mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(ch2), LRADC_CTRL1);
> + /*
> + * 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.
> + */
> + mxs_lradc_reg(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
> + LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
> + LRADC_DELAY_KICK |
> + LRADC_DELAY_DELAY(lradc->settling_delay), 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 (m2 == 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)) {
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_YP) |
> + LRADC_CTRL1_LRADC_IRQ(TS_CH_YP), LRADC_CTRL1);
> + val = mxs_lradc_read_raw_channel(lradc, TS_CH_YP);
> + } else if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_XM)) {
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_XM) |
> + LRADC_CTRL1_LRADC_IRQ(TS_CH_XM), LRADC_CTRL1);
> + val = mxs_lradc_read_raw_channel(lradc, TS_CH_XM);
> + } else if (reg & LRADC_CTRL1_LRADC_IRQ(TS_CH_YM)) {
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_CH_YM) |
> + LRADC_CTRL1_LRADC_IRQ(TS_CH_YM), LRADC_CTRL1);
> + val = mxs_lradc_read_raw_channel(lradc, TS_CH_YM);
> + } else {
> + return -EIO;
> + }
> +
> + mxs_lradc_reg(lradc, 0, LRADC_DELAY(2));
> + mxs_lradc_reg(lradc, 0, 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
> + */
> + mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
> + mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
> + LRADC_CTRL0);
> +}
> +
> +/*
> + * 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)
> +{
> + mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
> + mxs_lradc_reg_set(lradc, mxs_lradc_drive_x_plate(lradc), LRADC_CTRL0);
> +
> + lradc->cur_plate = LRADC_SAMPLE_X;
> + mxs_lradc_setup_ts_channel(lradc, TS_CH_YP);
> +}
> +
> +/*
> + * 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)
> +{
> + mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
> + mxs_lradc_reg_set(lradc, mxs_lradc_drive_y_plate(lradc), LRADC_CTRL0);
> +
> + lradc->cur_plate = LRADC_SAMPLE_Y;
> + mxs_lradc_setup_ts_channel(lradc, TS_CH_XM);
> +}
> +
> +/*
> + * YP(+)--+-------------+
> + * | |--+
> + * | | |
> + * YM(meas)--+-------------+ |
> + * +--------------+
> + * | |
> + * XP(meas) XM(-)
> + *
> + * (+) means here 1.85 V
> + * (-) means here GND
> + */
> +static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)
> +{
> + mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
> + mxs_lradc_reg_set(lradc, mxs_lradc_drive_pressure(lradc), LRADC_CTRL0);
> +
> + lradc->cur_plate = LRADC_SAMPLE_PRESSURE;
> + mxs_lradc_setup_ts_pressure(lradc, TS_CH_XP, TS_CH_YM);
> +}
> +
> +static void mxs_lradc_enable_touch_detection(struct mxs_lradc *lradc)
> +{
> + mxs_lradc_setup_touch_detection(lradc);
> +
> + lradc->cur_plate = LRADC_TOUCH;
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ |
> + LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> + mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> +}
> +
> +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
> + */
> + mxs_lradc_reg(lradc, 0, LRADC_CH(5));
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(5), LRADC_CTRL1);
> + mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(5), LRADC_CTRL1);
> + mxs_lradc_reg(lradc, LRADC_DELAY_TRIGGER(1 << 5) |
> + LRADC_DELAY_KICK | LRADC_DELAY_DELAY(10), /* waste 5 ms */
> + 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);
> + }
> +
> + /* 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);
> + }
> +
> + /* if it is released, wait for the next touch via IRQ */
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
> + mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> +}
> +
> +/* touchscreen's state machine */
> +static void mxs_lradc_handle_touch(struct mxs_lradc *lradc)
> +{
> + int val;
> +
> + switch (lradc->cur_plate) {
> + case LRADC_TOUCH:
> + /*
> + * start with the Y-pos, because it uses nearly the same plate
> + * settings like the touch detection
> + */
> + if (mxs_lradc_check_touch_event(lradc)) {
> + mxs_lradc_reg_clear(lradc,
> + LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> + LRADC_CTRL1);
> + mxs_lradc_prepare_y_pos(lradc);
> + }
> + mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ,
> + LRADC_CTRL1);
> + return;
> +
> + case LRADC_SAMPLE_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:
> + 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:
> + 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:
> + val = mxs_lradc_read_ts_channel(lradc); /* ignore the value */
> + mxs_lradc_finish_touch_event(lradc, 1);
> + break;
> + }
> +}
> +
> /*
> * Raw I/O operations
> */
> @@ -385,15 +820,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;
> @@ -551,10 +977,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. */
> mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
> mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> @@ -564,36 +986,29 @@ 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. */
> - mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
> - LRADC_CTRL0);
> -
> - /* Enable the touch-detect IRQ. */
> - mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> + 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();
> + /* stop all interrupts from firing */
> + mxs_lradc_reg_clear(lradc, 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_CTRL1);
>
> - /* Wait until touchscreen thread finishes any possible remnants. */
> - cancel_work_sync(&lradc->ts_work);
> + /* Power-down touchscreen touch-detect circuitry. */
> + mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
> +}
>
> - /* Disable touchscreen touch-detect IRQ. */
> - mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> - LRADC_CTRL1);
> +static void mxs_lradc_ts_close(struct input_dev *dev)
> +{
> + struct mxs_lradc *lradc = input_get_drvdata(dev);
>
> - /* Power-down touchscreen touch-detect circuitry. */
> - mxs_lradc_reg_clear(lradc, mxs_lradc_touch_detect_bit(lradc), LRADC_CTRL0);
> + mxs_lradc_disable_ts(lradc);
> }
>
> static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> @@ -641,6 +1056,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);
> }
>
> @@ -653,30 +1069,23 @@ 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 (!(reg & mxs_lradc_irq_mask(lradc)))
> 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) {
> - mxs_lradc_reg_clear(lradc, ts_irq_mask, LRADC_CTRL1);
> - 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);
>
> - mxs_lradc_reg_clear(lradc, reg & mxs_lradc_irq_mask(lradc), LRADC_CTRL1);
> -
> return IRQ_HANDLED;
> }
>
> @@ -971,6 +1380,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 =
> @@ -983,7 +1403,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. */
> @@ -1003,7 +1423,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",
> @@ -1066,9 +1486,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);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-15 10:56 ` Jonathan Cameron
@ 2013-09-15 16:10 ` Jonathan Cameron
2013-09-16 8:38 ` Jürgen Beisert
2013-09-16 8:10 ` Jürgen Beisert
2013-09-16 15:28 ` Dmitry Torokhov
2 siblings, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-15 16:10 UTC (permalink / raw)
To: linux-arm-kernel
On 09/15/13 11:56, Jonathan Cameron wrote:
> On 09/11/13 09:18, Juergen Beisert wrote:
>> 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 an 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>
> While this driver is placed in IIO within staging at the moment, these changes are definitely
> input related. Hence I have cc'd Dmitry and the input list.
>
> I am personaly a little uncomfortable that we have such a complex bit of input code sat
> within an IIO driver but such is life.
The logic in here looks reasonable to me. I am far from a specialist in how these touch
screens are normally handled though.
One thing to note is that you really want to get a proposed device tree spec out asap
as that can take longer to review than the driver. If you are proposing to do that as a future
patch, then take into account that you'll need to ensure these are the defaults if
it is not specified in the device tree for ever more (which is more painful than
hammering out he device tree stuff now!)
...
>> +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;
>> +}
>> +
...
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-15 10:56 ` Jonathan Cameron
2013-09-15 16:10 ` Jonathan Cameron
@ 2013-09-16 8:10 ` Jürgen Beisert
2013-09-16 9:37 ` Jürgen Beisert
` (2 more replies)
2013-09-16 15:28 ` Dmitry Torokhov
2 siblings, 3 replies; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-16 8:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jonathan,
On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
> On 09/11/13 09:18, Juergen Beisert wrote:
> > 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 an 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>
>
> While this driver is placed in IIO within staging at the moment, these
> changes are definitely input related. Hence I have cc'd Dmitry and the
> input list.
>
> I am personaly a little uncomfortable that we have such a complex bit of
> input code sat within an IIO driver but such is life.
Maybe an MFD for this ADC unit would be a better way to go? Currently I have a
different problem with this driver, because the ADC unit monitors the battery
as well. And the charging driver from the power subsystem needs these values
to charge the battery in a correct manner.
> [...]
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver
2013-09-15 10:50 ` Jonathan Cameron
@ 2013-09-16 8:14 ` Jürgen Beisert
2013-09-16 14:19 ` Marek Vasut
0 siblings, 1 reply; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-16 8:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jonathan,
On Sunday 15 September 2013 12:50:09 Jonathan Cameron wrote:
> On 09/11/13 09:18, Juergen Beisert wrote:
> > Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC at
> > least for the 4 wire touchscreen.
> >
> > Note: support for the remaining LRADC channels is not tested on an
> > i.MX23 yet.
>
> An ominous comment. Are you likely to test them soon?
Sorry, currently no i.MX28 hardware available. Someone out here with an i.MX28
based system with a touchscreen to test?
> Minor code layout comment inline but otherwise I'm just looking for an
> ack from Marek.
>
> > [...]
> > static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
> > {
> > + if (lradc->soc == IMX23_LRADC)
> > + return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
> > return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
>
> Whilst it obviously doesn't actually matter, having an else
> in there would make the code more consistent so personally
> I would prefer it to be there.
I can change it. Anyone here with objections against such a change?
Regards
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
2013-09-15 10:35 ` Jonathan Cameron
@ 2013-09-16 8:17 ` Jürgen Beisert
2013-09-16 18:39 ` Jonathan Cameron
0 siblings, 1 reply; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-16 8:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jonathan,
On Sunday 15 September 2013 12:35:29 Jonathan Cameron wrote:
> [...]
> > +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
> > +{
> > + writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
> > +}
> > +
> > +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
> > +{
> > + writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
> > +}
> > +
>
> mxs_lradc_reg_write might be clearer?
I would prefer the shorter name. Due to long macro names and long function
names the lines in the code below needs more and more line breaks which is
IMHO less helpfull to read the code.
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-15 16:10 ` Jonathan Cameron
@ 2013-09-16 8:38 ` Jürgen Beisert
0 siblings, 0 replies; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-16 8:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jonathan,
On Sunday 15 September 2013 18:10:18 Jonathan Cameron wrote:
> On 09/15/13 11:56, Jonathan Cameron wrote:
> > On 09/11/13 09:18, Juergen Beisert wrote:
> >> 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 an 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>
> >
> > While this driver is placed in IIO within staging at the moment, these
> > changes are definitely input related. Hence I have cc'd Dmitry and the
> > input list.
> >
> > I am personaly a little uncomfortable that we have such a complex bit of
> > input code sat within an IIO driver but such is life.
>
> The logic in here looks reasonable to me. I am far from a specialist in how
> these touch screens are normally handled though.
>
> One thing to note is that you really want to get a proposed device tree
> spec out asap as that can take longer to review than the driver. If you
> are proposing to do that as a future patch, then take into account that
> you'll need to ensure these are the defaults if it is not specified in the
> device tree for ever more (which is more painful than hammering out he
> device tree stuff now!)
> ...
Will do.
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 8:10 ` Jürgen Beisert
@ 2013-09-16 9:37 ` Jürgen Beisert
2013-09-16 14:23 ` Marek Vasut
2013-09-16 15:30 ` Jonathan Cameron
2 siblings, 0 replies; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-16 9:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jonathan,
On Monday 16 September 2013 10:10:22 J?rgen Beisert wrote:
> [...]
> > While this driver is placed in IIO within staging at the moment, these
> > changes are definitely input related. Hence I have cc'd Dmitry and the
> > input list.
> >
> > I am personaly a little uncomfortable that we have such a complex bit of
> > input code sat within an IIO driver but such is life.
>
> Maybe an MFD for this ADC unit would be a better way to go? Currently I
> have a different problem with this driver, because the ADC unit monitors
> the battery as well. And the charging driver from the power subsystem needs
> these values to charge the battery in a correct manner.
I would suggest:
diff --git a/drivers/staging/iio/TODO b/drivers/staging/iio/TODO
index 04c2326..c22a0ed 100644
--- a/drivers/staging/iio/TODO
+++ b/drivers/staging/iio/TODO
@@ -13,6 +13,17 @@ Would be nice
3) Expand device set. Lots of other maxim adc's have very
similar interfaces.
+MXS LRADC driver:
+This is a classic MFD device as it combines the following subdevices
+ - touchscreen controller (input subsystem related device)
+ - general purpose ADC channels
+ - battery voltage monitor (power subsystem related device)
+ - die temperature monitor (thermal management)
+
+At least the battery voltage and die temperature feature is required in-kernel
+by a driver of the SoC's battery charging unit to avoid any damage to the
+silicon and the battery.
+
TSL2561
Would be nice
1) Open question of userspace vs kernel space balance when
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
2013-09-11 8:18 ` [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
2013-09-15 10:35 ` Jonathan Cameron
@ 2013-09-16 14:15 ` Marek Vasut
1 sibling, 0 replies; 28+ messages in thread
From: Marek Vasut @ 2013-09-16 14:15 UTC (permalink / raw)
To: linux-arm-kernel
Dear Juergen Beisert,
> Replace the individual register access by a few shared access function to
> make the code easier to read and in order to add the i.MX23 SoC in the
> next step.
>
> 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 | 204
> +++++++++++++++++++++--------------- 1 file changed, 120 insertions(+), 84
> deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index 5535fed..4317fee 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -235,6 +235,56 @@ struct mxs_lradc {
> #define LRADC_RESOLUTION 12
> #define LRADC_SINGLE_SAMPLE_MASK ((1 << LRADC_RESOLUTION) - 1)
>
> +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
> +{
> + writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
> +}
> +
> +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
> +{
> + writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
> +}
> +
> +static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 val, u32 reg)
> +{
> + writel(val, lradc->base + reg);
> +}
Idea for later patch: Should we not move these functions to a MXS-specific
header _later_ on so other drivers can leverage them too ?
[...]
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver
2013-09-16 8:14 ` Jürgen Beisert
@ 2013-09-16 14:19 ` Marek Vasut
0 siblings, 0 replies; 28+ messages in thread
From: Marek Vasut @ 2013-09-16 14:19 UTC (permalink / raw)
To: linux-arm-kernel
Dear J?rgen Beisert,
> Hi Jonathan,
>
> On Sunday 15 September 2013 12:50:09 Jonathan Cameron wrote:
> > On 09/11/13 09:18, Juergen Beisert wrote:
> > > Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC
> > > at least for the 4 wire touchscreen.
> > >
> > > Note: support for the remaining LRADC channels is not tested on an
> > > i.MX23 yet.
> >
> > An ominous comment. Are you likely to test them soon?
>
> Sorry, currently no i.MX28 hardware available. Someone out here with an
> i.MX28 based system with a touchscreen to test?
You mean 23, no? I can test both, I have the DENX EVK as well as both MX23EVK
and MX28EVK. Just need to find some more time to do my maintainance rounds, but
that should happen this week nonetheless. If I do forget, please ping me again.
> > Minor code layout comment inline but otherwise I'm just looking for an
> > ack from Marek.
> >
> > > [...]
> > >
> > > static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
> > > {
> > >
> > > + if (lradc->soc == IMX23_LRADC)
> > > + return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
> > >
> > > return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
> >
> > Whilst it obviously doesn't actually matter, having an else
> > in there would make the code more consistent so personally
> > I would prefer it to be there.
>
> I can change it. Anyone here with objections against such a change?
>
> Regards
> Juergen
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 8:10 ` Jürgen Beisert
2013-09-16 9:37 ` Jürgen Beisert
@ 2013-09-16 14:23 ` Marek Vasut
2013-09-16 14:34 ` Jürgen Beisert
2013-09-16 15:30 ` Jonathan Cameron
2 siblings, 1 reply; 28+ messages in thread
From: Marek Vasut @ 2013-09-16 14:23 UTC (permalink / raw)
To: linux-arm-kernel
Dear J?rgen Beisert,
> Hi Jonathan,
>
> On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
> > On 09/11/13 09:18, Juergen Beisert wrote:
> > > 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 an 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>
> >
> > While this driver is placed in IIO within staging at the moment, these
> > changes are definitely input related. Hence I have cc'd Dmitry and the
> > input list.
> >
> > I am personaly a little uncomfortable that we have such a complex bit of
> > input code sat within an IIO driver but such is life.
>
> Maybe an MFD for this ADC unit would be a better way to go? Currently I
> have a different problem with this driver, because the ADC unit monitors
> the battery as well. And the charging driver from the power subsystem
> needs these values to charge the battery in a correct manner.
Are you planning to post the power block patches too ?
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 14:23 ` Marek Vasut
@ 2013-09-16 14:34 ` Jürgen Beisert
2013-09-16 15:10 ` Marek Vasut
0 siblings, 1 reply; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-16 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marek,
On Monday 16 September 2013 16:23:48 Marek Vasut wrote:
> > On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
> > > On 09/11/13 09:18, Juergen Beisert wrote:
> > > > 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 an 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>
> > >
> > > While this driver is placed in IIO within staging at the moment, these
> > > changes are definitely input related. Hence I have cc'd Dmitry and the
> > > input list.
> > >
> > > I am personaly a little uncomfortable that we have such a complex bit
> > > of input code sat within an IIO driver but such is life.
> >
> > Maybe an MFD for this ADC unit would be a better way to go? Currently I
> > have a different problem with this driver, because the ADC unit monitors
> > the battery as well. And the charging driver from the power subsystem
> > needs these values to charge the battery in a correct manner.
>
> Are you planning to post the power block patches too ?
When they will work: yes. Currently it crashes all the time. The PMIC is a
beast...
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 14:34 ` Jürgen Beisert
@ 2013-09-16 15:10 ` Marek Vasut
0 siblings, 0 replies; 28+ messages in thread
From: Marek Vasut @ 2013-09-16 15:10 UTC (permalink / raw)
To: linux-arm-kernel
Dear J?rgen Beisert,
> Hi Marek,
>
> On Monday 16 September 2013 16:23:48 Marek Vasut wrote:
> > > On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
> > > > On 09/11/13 09:18, Juergen Beisert wrote:
> > > > > 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 an 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>
> > > >
> > > > While this driver is placed in IIO within staging at the moment,
> > > > these changes are definitely input related. Hence I have cc'd
> > > > Dmitry and the input list.
> > > >
> > > > I am personaly a little uncomfortable that we have such a complex bit
> > > > of input code sat within an IIO driver but such is life.
> > >
> > > Maybe an MFD for this ADC unit would be a better way to go? Currently I
> > > have a different problem with this driver, because the ADC unit
> > > monitors the battery as well. And the charging driver from the power
> > > subsystem needs these values to charge the battery in a correct
> > > manner.
> >
> > Are you planning to post the power block patches too ?
>
> When they will work: yes. Currently it crashes all the time. The PMIC is a
> beast...
Full ACK
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-15 10:56 ` Jonathan Cameron
2013-09-15 16:10 ` Jonathan Cameron
2013-09-16 8:10 ` Jürgen Beisert
@ 2013-09-16 15:28 ` Dmitry Torokhov
2013-09-17 7:33 ` Jürgen Beisert
2 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2013-09-16 15:28 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Sep 15, 2013 at 11:56:25AM +0100, Jonathan Cameron wrote:
> On 09/11/13 09:18, Juergen Beisert wrote:
> > 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 an 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>
> While this driver is placed in IIO within staging at the moment, these changes are definitely
> input related. Hence I have cc'd Dmitry and the input list.
>
> I am personaly a little uncomfortable that we have such a complex bit of input code sat
> within an IIO driver but such is life.
>
...
> >
> > static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> > @@ -641,6 +1056,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);
> > }
This looks iffy... Normally you disable the device so that it does not
generate more interrupts, and then cancel outstanding work(s), otherwise
newly generated interrupts may cause more work to be scheduled. Or I
missed some of the context and this is not a concern here?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 8:10 ` Jürgen Beisert
2013-09-16 9:37 ` Jürgen Beisert
2013-09-16 14:23 ` Marek Vasut
@ 2013-09-16 15:30 ` Jonathan Cameron
2013-09-19 12:49 ` Jürgen Beisert
2 siblings, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-16 15:30 UTC (permalink / raw)
To: linux-arm-kernel
"J?rgen Beisert" <jbe@pengutronix.de> wrote:
>Hi Jonathan,
>
>On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
>> On 09/11/13 09:18, Juergen Beisert wrote:
>> > 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 an 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>
>>
>> While this driver is placed in IIO within staging at the moment,
>these
>> changes are definitely input related. Hence I have cc'd Dmitry and
>the
>> input list.
>>
>> I am personaly a little uncomfortable that we have such a complex bit
>of
>> input code sat within an IIO driver but such is life.
>
>Maybe an MFD for this ADC unit would be a better way to go?
That would be great and is definitely the preferred method.
Currently I
>have a
>different problem with this driver, because the ADC unit monitors the
>battery
>as well. And the charging driver from the power subsystem needs these
>values
>to charge the battery in a correct manner.
There is an iio client battery driver as generic_ADC_battery.c but it is currently only doing polled access. Not too hard to add buffered access though ideally we would want a generic way of combining consumers doing polled and interrupt driven accesses.
>
>> [...]
>
>Regards,
>Juergen
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
2013-09-16 8:17 ` Jürgen Beisert
@ 2013-09-16 18:39 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-16 18:39 UTC (permalink / raw)
To: linux-arm-kernel
On 09/16/13 09:17, J?rgen Beisert wrote:
> Hi Jonathan,
>
> On Sunday 15 September 2013 12:35:29 Jonathan Cameron wrote:
>> [...]
>>> +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
>>> +{
>>> + writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
>>> +}
>>> +
>>> +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
>>> +{
>>> + writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
>>> +}
>>> +
>>
>> mxs_lradc_reg_write might be clearer?
>
> I would prefer the shorter name. Due to long macro names and long function
> names the lines in the code below needs more and more line breaks which is
> IMHO less helpfull to read the code.
Hmm. It's not entirely obvious what else this might be doing, but it is
also not obvious from the name that it is writing to the register...
I remain unconvinced by the short name!
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 15:28 ` Dmitry Torokhov
@ 2013-09-17 7:33 ` Jürgen Beisert
0 siblings, 0 replies; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-17 7:33 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dmitry,
On Monday 16 September 2013 17:28:46 Dmitry Torokhov wrote:
> [...]
> > > static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> > > @@ -641,6 +1056,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);
> > > }
>
> This looks iffy... Normally you disable the device so that it does not
> generate more interrupts, and then cancel outstanding work(s), otherwise
> newly generated interrupts may cause more work to be scheduled. Or I
> missed some of the context and this is not a concern here?
This part gets removed in patch 6/6:
@@ -1054,8 +891,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);
?}
But you are right, I should move this into patch 5/6.
Thanks
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-16 15:30 ` Jonathan Cameron
@ 2013-09-19 12:49 ` Jürgen Beisert
2013-09-19 16:12 ` Jonathan Cameron
0 siblings, 1 reply; 28+ messages in thread
From: Jürgen Beisert @ 2013-09-19 12:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jonathan,
On Monday 16 September 2013 17:30:32 Jonathan Cameron wrote:
> >On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
> >> On 09/11/13 09:18, Juergen Beisert wrote:
> >> > 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 an 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>
> >>
> >> While this driver is placed in IIO within staging at the moment,
> >> these changes are definitely input related. Hence I have cc'd Dmitry and
> >> the input list.
> >>
> >> I am personaly a little uncomfortable that we have such a complex bit
> >> of input code sat within an IIO driver but such is life.
> >
> > Maybe an MFD for this ADC unit would be a better way to go?
>
> That would be great and is definitely the preferred method.
Cannot continue to convert the driver into an MFD device. The project does not
give me the time to do so.
Regards,
Juergen
--
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Beisert ? ? ? ? ? ? |
Linux Solutions for Science and Industry ? ? ?| http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
2013-09-19 12:49 ` Jürgen Beisert
@ 2013-09-19 16:12 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-09-19 16:12 UTC (permalink / raw)
To: linux-arm-kernel
.
"J?rgen Beisert" <jbe@pengutronix.de> wrote:
>Hi Jonathan,
>
>On Monday 16 September 2013 17:30:32 Jonathan Cameron wrote:
>> >On Sunday 15 September 2013 12:56:25 Jonathan ,,, wrote:
>> >> On 09/11/13 09:18, Juergen Beisert wrote:
>> >> > 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 an 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>
>> >>
>> >> While this driver is placed in IIO within staging at the moment,
>> >> these changes are definitely input related. Hence I have cc'd
>Dmitry and
>> >> the input list.
>> >>
>> >> I am personaly a little uncomfortable that we have such a complex
>bit
>> >> of input code sat within an IIO driver but such is life.
>> >
>> > Maybe an MFD for this ADC unit would be a better way to go?
>>
>> That would be great and is definitely the preferred method.
>
>Cannot continue to convert the driver into an MFD device. The project
>does not
>give me the time to do so.
>
>Regards,
>Juergen
Fair enough. Perhaps someone else will pick up the gauntlet :)
The old call of why there isn't enough time in the day!
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2013-09-19 16:12 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-11 8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
2013-09-11 8:18 ` [PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC Juergen Beisert
2013-09-11 8:18 ` [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits Juergen Beisert
2013-09-15 10:27 ` Jonathan Cameron
2013-09-11 8:18 ` [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
2013-09-15 10:35 ` Jonathan Cameron
2013-09-16 8:17 ` Jürgen Beisert
2013-09-16 18:39 ` Jonathan Cameron
2013-09-16 14:15 ` Marek Vasut
2013-09-11 8:18 ` [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver Juergen Beisert
2013-09-15 10:50 ` Jonathan Cameron
2013-09-16 8:14 ` Jürgen Beisert
2013-09-16 14:19 ` Marek Vasut
2013-09-11 8:18 ` [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection Juergen Beisert
2013-09-15 10:56 ` Jonathan Cameron
2013-09-15 16:10 ` Jonathan Cameron
2013-09-16 8:38 ` Jürgen Beisert
2013-09-16 8:10 ` Jürgen Beisert
2013-09-16 9:37 ` Jürgen Beisert
2013-09-16 14:23 ` Marek Vasut
2013-09-16 14:34 ` Jürgen Beisert
2013-09-16 15:10 ` Marek Vasut
2013-09-16 15:30 ` Jonathan Cameron
2013-09-19 12:49 ` Jürgen Beisert
2013-09-19 16:12 ` Jonathan Cameron
2013-09-16 15:28 ` Dmitry Torokhov
2013-09-17 7:33 ` Jürgen Beisert
2013-09-11 8:18 ` [PATCH 6/6] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation Juergen Beisert
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).