All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC
@ 2015-01-17  0:22 Kristina Martšenko
  2015-01-17  0:22 ` [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels Kristina Martšenko
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-17  0:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Marek Vasut,
	Juergen Beisert, Alexandre Belloni, Fabio Estevam, Stefan Wahren,
	Greg Kroah-Hartman, linux-iio, devel, Kristina Martšenko

Hi,

These patches fix some issues with using the touchscreen and reading
other ADC channels at the same time.

The main issue is that the "virtual" channels the touchscreen uses can
overlap with the channels that the iio buffer is configured to read,
causing the touchscreen to read those channels instead, and report wrong
values. Patch #1 fixes that by separating the channels they use.

Patches #2 and #3 stop buffered mode and sysfs reads from disabling the
touchscreen. Patch #4 keeps the buffer from reporting wrong values when
the touchscreen is in use.

Note that I've only tested these patches on i.MX28. I checked the i.MX23
reference manual [1], and there don't seem to be any differences in the
parts I changed, but it would still be nice if someone could test on
i.MX23 as well.

The patches were tested on next-20150113 and rebased onto iio/togreg.

Thanks,
Kristina

[1]
i.MX23: http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf
i.MX28: http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf


Kristina Martšenko (4):
  iio: mxs-lradc: separate touchscreen and buffer virtual channels
  iio: mxs-lradc: make ADC reads not disable touchscreen interrupts
  iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions
  iio: mxs-lradc: only update the buffer when its conversions have
    finished

 drivers/staging/iio/adc/mxs-lradc.c | 192 +++++++++++++++++-------------------
 1 file changed, 91 insertions(+), 101 deletions(-)

-- 
2.2.0

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

* [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels
  2015-01-17  0:22 [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Kristina Martšenko
@ 2015-01-17  0:22 ` Kristina Martšenko
  2015-01-18  0:19   ` Marek Vasut
  2015-01-17  0:22 ` [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts Kristina Martšenko
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-17  0:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Marek Vasut,
	Juergen Beisert, Alexandre Belloni, Fabio Estevam, Stefan Wahren,
	Greg Kroah-Hartman, linux-iio, devel, Kristina Martšenko

The touchscreen was initially designed [1] to map all of its physical
channels to one virtual channel, leaving buffered capture to use the
remaining 7 virtual channels. When the touchscreen was reimplemented
[2], it was made to use four virtual channels, which overlap and
conflict with the channels the buffer uses.

As a result, when the buffer is enabled, the touchscreen's virtual
channels are remapped to whichever physical channels the buffer was
configured with, causing the touchscreen to read those instead of the
touch measurement channels. Effectively the touchscreen stops working.

So here we separate the channels again, giving the touchscreen 2 virtual
channels and the buffer 6. We can't give the touchscreen just 1 channel
as before, as the current pressure calculation requires 2 channels to be
read at the same time.

This makes the touchscreen continue to work during buffered capture. It
has been tested on i.MX28, but not on i.MX23.

[1] 06ddd353f5c8 ("iio: mxs: Implement support for touchscreen")
[2] dee05308f602 ("Staging/iio/adc/touchscreen/MXS: add interrupt driven
touch detection")

Signed-off-by: Kristina Martšenko <kristina.martsenko@gmail.com>
---
Note that this patch alone isn't very useful, as another bug causes buffered
capture to disable the touchscreen entirely (on i.MX28). Patch #2 in this
series fixes the other issue.

 drivers/staging/iio/adc/mxs-lradc.c | 158 ++++++++++++++++--------------------
 1 file changed, 68 insertions(+), 90 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index e0e91836eec1..fc65cd311be9 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -214,11 +214,14 @@ struct mxs_lradc {
 	unsigned long		is_divided;
 
 	/*
-	 * Touchscreen LRADC channels receives a private slot in the CTRL4
-	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
-	 * CTRL4 register can be mapped to LRADC channels when using the
-	 * touchscreen.
-	 *
+	 * When the touchscreen is enabled, we give it two private virtual
+	 * channels: #6 and #7. This means that only 6 virtual channels (instead
+	 * of 8) will be available for buffered capture.
+	 */
+#define TS_VCH1		7
+#define TS_VCH2		6
+
+	/*
 	 * Furthermore, certain LRADC channels are shared between touchscreen
 	 * and/or touch-buttons and generic LRADC block. Therefore when using
 	 * either of these, these channels are not available for the regular
@@ -342,6 +345,9 @@ struct mxs_lradc {
 #define	LRADC_CTRL4				0x140
 #define	LRADC_CTRL4_LRADCSELECT_MASK(n)		(0xf << ((n) * 4))
 #define	LRADC_CTRL4_LRADCSELECT_OFFSET(n)	((n) * 4)
+#define	LRADC_CTRL4_LRADCSELECT(n, x) \
+				(((x) << LRADC_CTRL4_LRADCSELECT_OFFSET(n)) & \
+				LRADC_CTRL4_LRADCSELECT_MASK(n))
 
 #define LRADC_RESOLUTION			12
 #define LRADC_SINGLE_SAMPLE_MASK		((1 << LRADC_RESOLUTION) - 1)
@@ -416,6 +422,14 @@ static bool mxs_lradc_check_touch_event(struct mxs_lradc *lradc)
 					LRADC_STATUS_TOUCH_DETECT_RAW);
 }
 
+static void mxs_lradc_map_channel(struct mxs_lradc *lradc, unsigned vch,
+				  unsigned ch)
+{
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(vch),
+				LRADC_CTRL4);
+	mxs_lradc_reg_set(lradc, LRADC_CTRL4_LRADCSELECT(vch, ch), LRADC_CTRL4);
+}
+
 static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 {
 	/*
@@ -450,12 +464,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 		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);
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(ch), 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
@@ -509,12 +519,8 @@ static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
 		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);
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(ch2), 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
@@ -580,36 +586,6 @@ static unsigned mxs_lradc_read_ts_pressure(struct mxs_lradc *lradc,
 #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_wrt(lradc, 0, LRADC_DELAY(2));
-	mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(3));
-
-	return val;
-}
-
 /*
  * YP(open)--+-------------+
  *           |             |--+
@@ -653,7 +629,8 @@ static void mxs_lradc_prepare_x_pos(struct mxs_lradc *lradc)
 	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);
+	mxs_lradc_map_channel(lradc, TS_VCH1, TS_CH_YP);
+	mxs_lradc_setup_ts_channel(lradc, TS_VCH1);
 }
 
 /*
@@ -674,7 +651,8 @@ static void mxs_lradc_prepare_y_pos(struct mxs_lradc *lradc)
 	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);
+	mxs_lradc_map_channel(lradc, TS_VCH1, TS_CH_XM);
+	mxs_lradc_setup_ts_channel(lradc, TS_VCH1);
 }
 
 /*
@@ -695,7 +673,9 @@ static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)
 	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);
+	mxs_lradc_map_channel(lradc, TS_VCH1, TS_CH_YM);
+	mxs_lradc_map_channel(lradc, TS_VCH2, TS_CH_XP);
+	mxs_lradc_setup_ts_pressure(lradc, TS_VCH2, TS_VCH1);
 }
 
 static void mxs_lradc_enable_touch_detection(struct mxs_lradc *lradc)
@@ -708,6 +688,19 @@ static void mxs_lradc_enable_touch_detection(struct mxs_lradc *lradc)
 	mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
 }
 
+static void mxs_lradc_start_touch_event(struct mxs_lradc *lradc)
+{
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
+				LRADC_CTRL1);
+	mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(TS_VCH1),
+				LRADC_CTRL1);
+	/*
+	 * start with the Y-pos, because it uses nearly the same plate
+	 * settings like the touch detection
+	 */
+	mxs_lradc_prepare_y_pos(lradc);
+}
+
 static void mxs_lradc_report_ts_event(struct mxs_lradc *lradc)
 {
 	input_report_abs(lradc->ts_input, ABS_X, lradc->ts_x_pos);
@@ -725,10 +718,10 @@ static void mxs_lradc_complete_touch_event(struct mxs_lradc *lradc)
 	 * start a dummy conversion to burn time to settle the signals
 	 * note: we are not interested in the conversion's value
 	 */
-	mxs_lradc_reg_wrt(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_wrt(lradc, LRADC_DELAY_TRIGGER(1 << 5) |
+	mxs_lradc_reg_wrt(lradc, 0, LRADC_CH(TS_VCH1));
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(TS_VCH1) |
+		LRADC_CTRL1_LRADC_IRQ(TS_VCH2), LRADC_CTRL1);
+	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << TS_VCH1) |
 		LRADC_DELAY_KICK | LRADC_DELAY_DELAY(10), /* waste 5 ms */
 			LRADC_DELAY(2));
 }
@@ -760,59 +753,42 @@ static void mxs_lradc_finish_touch_event(struct mxs_lradc *lradc, bool valid)
 
 	/* if it is released, wait for the next touch via IRQ */
 	lradc->cur_plate = LRADC_TOUCH;
-	mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
+	mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(2));
+	mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(3));
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ |
+			LRADC_CTRL1_LRADC_IRQ(TS_VCH1) |
+			LRADC_CTRL1_LRADC_IRQ_EN(TS_VCH1), 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);
-		}
+		if (mxs_lradc_check_touch_event(lradc))
+			mxs_lradc_start_touch_event(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;
+		lradc->ts_y_pos = mxs_lradc_read_raw_channel(lradc, TS_VCH1);
 		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;
+		lradc->ts_x_pos = mxs_lradc_read_raw_channel(lradc, TS_VCH1);
 		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_read_ts_pressure(lradc, TS_VCH2, TS_VCH1);
 		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;
 	}
@@ -1090,9 +1066,8 @@ static void mxs_lradc_disable_ts(struct mxs_lradc *lradc)
 {
 	/* 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);
+		LRADC_CTRL1_LRADC_IRQ_EN(TS_VCH1) |
+		LRADC_CTRL1_LRADC_IRQ_EN(TS_VCH2), LRADC_CTRL1);
 
 	/* Power-down touchscreen touch-detect circuitry. */
 	mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
@@ -1158,26 +1133,29 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 	struct iio_dev *iio = data;
 	struct mxs_lradc *lradc = iio_priv(iio);
 	unsigned long reg = readl(lradc->base + LRADC_CTRL1);
+	uint32_t clr_irq = mxs_lradc_irq_mask(lradc);
 	const uint32_t ts_irq_mask =
 		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);
+		LRADC_CTRL1_LRADC_IRQ(TS_VCH1) |
+		LRADC_CTRL1_LRADC_IRQ(TS_VCH2);
 
 	if (!(reg & mxs_lradc_irq_mask(lradc)))
 		return IRQ_NONE;
 
-	if (lradc->use_touchscreen && (reg & ts_irq_mask))
+	if (lradc->use_touchscreen && (reg & ts_irq_mask)) {
 		mxs_lradc_handle_touch(lradc);
 
+		/* Make sure we don't clear the next conversion's interrupt. */
+		clr_irq &= ~(LRADC_CTRL1_LRADC_IRQ(TS_VCH1) |
+				LRADC_CTRL1_LRADC_IRQ(TS_VCH2));
+	}
+
 	if (iio_buffer_enabled(iio))
 		iio_trigger_poll(iio->trig);
 	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
 		complete(&lradc->completion);
 
-	mxs_lradc_reg_clear(lradc, reg & mxs_lradc_irq_mask(lradc),
-			LRADC_CTRL1);
+	mxs_lradc_reg_clear(lradc, reg & clr_irq, LRADC_CTRL1);
 
 	return IRQ_HANDLED;
 }
@@ -1353,7 +1331,7 @@ static bool mxs_lradc_validate_scan_mask(struct iio_dev *iio,
 	if (lradc->use_touchbutton)
 		rsvd_chans++;
 	if (lradc->use_touchscreen)
-		rsvd_chans++;
+		rsvd_chans += 2;
 
 	/* Test for attempts to map channels with special mode of operation. */
 	if (bitmap_intersects(mask, &rsvd_mask, LRADC_MAX_TOTAL_CHANS))
-- 
2.2.0

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

* [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts
  2015-01-17  0:22 [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Kristina Martšenko
  2015-01-17  0:22 ` [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels Kristina Martšenko
@ 2015-01-17  0:22 ` Kristina Martšenko
  2015-01-18  0:21   ` Marek Vasut
  2015-01-17  0:22 ` [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions Kristina Martšenko
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-17  0:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Marek Vasut,
	Juergen Beisert, Alexandre Belloni, Fabio Estevam, Stefan Wahren,
	Greg Kroah-Hartman, linux-iio, devel, Kristina Martšenko

Reading a channel through sysfs, or starting a buffered capture, will
currently turn off the touchscreen. This is because the read_raw() and
buffer preenable()/postdisable() callbacks disable interrupts for all
LRADC channels, including those the touchscreen uses.

So make the callbacks only disable interrupts for the channels they use.
This means channel 0 for read_raw() and channels 0-5 for the buffer (if
the touchscreen is enabled). Since the touchscreen uses different
channels (6 and 7), it no longer gets turned off.

Note that only i.MX28 is affected by this issue, i.MX23 should be fine.

Signed-off-by: Kristina Martšenko <kristina.martsenko@gmail.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index fc65cd311be9..0cf276ff0dc5 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -218,8 +218,11 @@ struct mxs_lradc {
 	 * channels: #6 and #7. This means that only 6 virtual channels (instead
 	 * of 8) will be available for buffered capture.
 	 */
-#define TS_VCH1		7
-#define TS_VCH2		6
+#define TS_VCH1				7
+#define TS_VCH2				6
+#define BUFFER_VCHANS_LIMITED		0x3f
+#define BUFFER_VCHANS_ALL		0xff
+	u8			buffer_vchans;
 
 	/*
 	 * Furthermore, certain LRADC channels are shared between touchscreen
@@ -820,7 +823,7 @@ static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val)
 	 * used if doing raw sampling.
 	 */
 	if (lradc->soc == IMX28_LRADC)
-		mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+		mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0),
 			LRADC_CTRL1);
 	mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 
@@ -1267,8 +1270,9 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 	}
 
 	if (lradc->soc == IMX28_LRADC)
-		mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-							LRADC_CTRL1);
+		mxs_lradc_reg_clear(lradc,
+			lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
+			LRADC_CTRL1);
 	mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 
 	for_each_set_bit(chan, iio->active_scan_mask, LRADC_MAX_TOTAL_CHANS) {
@@ -1304,8 +1308,9 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
 
 	mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 	if (lradc->soc == IMX28_LRADC)
-		mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-					LRADC_CTRL1);
+		mxs_lradc_reg_clear(lradc,
+			lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
+			LRADC_CTRL1);
 
 	kfree(lradc->buffer);
 	mutex_unlock(&lradc->lock);
@@ -1561,6 +1566,11 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 
 	touch_ret = mxs_lradc_probe_touchscreen(lradc, node);
 
+	if (touch_ret == 0)
+		lradc->buffer_vchans = BUFFER_VCHANS_LIMITED;
+	else
+		lradc->buffer_vchans = BUFFER_VCHANS_ALL;
+
 	/* Grab all IRQ sources */
 	for (i = 0; i < of_cfg->irq_count; i++) {
 		lradc->irq[i] = platform_get_irq(pdev, i);
-- 
2.2.0

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

* [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions
  2015-01-17  0:22 [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Kristina Martšenko
  2015-01-17  0:22 ` [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels Kristina Martšenko
  2015-01-17  0:22 ` [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts Kristina Martšenko
@ 2015-01-17  0:22 ` Kristina Martšenko
  2015-01-18  0:22   ` Marek Vasut
  2015-01-19  7:20   ` Juergen Borleis
  2015-01-17  0:22 ` [PATCH 4/4] iio: mxs-lradc: only update the buffer when its conversions have finished Kristina Martšenko
  2015-01-18 11:21 ` [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Stefan Wahren
  4 siblings, 2 replies; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-17  0:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Marek Vasut,
	Juergen Beisert, Alexandre Belloni, Fabio Estevam, Stefan Wahren,
	Greg Kroah-Hartman, linux-iio, devel, Kristina Martšenko

Reading a channel through sysfs, or starting a buffered capture, can
occasionally turn off the touchscreen.

This is because the read_raw() and buffer preenable()/postdisable()
callbacks unschedule current conversions on all channels. If a delay
channel happens to schedule a touchscreen conversion at the same time,
the conversion gets cancelled and the touchscreen sequence stops.

This is probably related to this note from the reference manual:

	"If a delay group schedules channels to be sampled and a manual
	write to the schedule field in CTRL0 occurs while the block is
	discarding samples, the LRADC will switch to the new schedule
	and will not sample the channels that were previously scheduled.
	The time window for this to happen is very small and lasts only
	while the LRADC is discarding samples."

So make the callbacks only unschedule conversions for the channels they
use. This means channel 0 for read_raw() and channels 0-5 for the buffer
(if the touchscreen is enabled). Since the touchscreen uses different
channels (6 and 7), it no longer gets turned off.

This is tested and fixes the issue on i.MX28, but hasn't been tested on
i.MX23.

Signed-off-by: Kristina Martšenko <kristina.martsenko@gmail.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 0cf276ff0dc5..5b5cb205b9ed 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -825,7 +825,7 @@ static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val)
 	if (lradc->soc == IMX28_LRADC)
 		mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0),
 			LRADC_CTRL1);
-	mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
+	mxs_lradc_reg_clear(lradc, 0x1, LRADC_CTRL0);
 
 	/* Enable / disable the divider per requirement */
 	if (test_bit(chan, &lradc->is_divided))
@@ -1273,7 +1273,7 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
 		mxs_lradc_reg_clear(lradc,
 			lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
 			LRADC_CTRL1);
-	mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
+	mxs_lradc_reg_clear(lradc, lradc->buffer_vchans, LRADC_CTRL0);
 
 	for_each_set_bit(chan, iio->active_scan_mask, LRADC_MAX_TOTAL_CHANS) {
 		ctrl4_set |= chan << LRADC_CTRL4_LRADCSELECT_OFFSET(ofs);
@@ -1306,7 +1306,7 @@ static int mxs_lradc_buffer_postdisable(struct iio_dev *iio)
 	mxs_lradc_reg_clear(lradc, LRADC_DELAY_TRIGGER_LRADCS_MASK |
 					LRADC_DELAY_KICK, LRADC_DELAY(0));
 
-	mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
+	mxs_lradc_reg_clear(lradc, lradc->buffer_vchans, LRADC_CTRL0);
 	if (lradc->soc == IMX28_LRADC)
 		mxs_lradc_reg_clear(lradc,
 			lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
-- 
2.2.0

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

* [PATCH 4/4] iio: mxs-lradc: only update the buffer when its conversions have finished
  2015-01-17  0:22 [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Kristina Martšenko
                   ` (2 preceding siblings ...)
  2015-01-17  0:22 ` [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions Kristina Martšenko
@ 2015-01-17  0:22 ` Kristina Martšenko
  2015-01-18  0:23   ` Marek Vasut
  2015-01-18 11:21 ` [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Stefan Wahren
  4 siblings, 1 reply; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-17  0:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Marek Vasut,
	Juergen Beisert, Alexandre Belloni, Fabio Estevam, Stefan Wahren,
	Greg Kroah-Hartman, linux-iio, devel, Kristina Martšenko

Using the touchscreen while running buffered capture results in the
buffer reporting lots of wrong values, often just zeros. This is because
we push readings to the buffer every time a touchscreen interrupt
arrives, including when the buffer's own conversions have not yet
finished. So let's only push to the buffer when its conversions are
ready.

Signed-off-by: Kristina Martšenko <kristina.martsenko@gmail.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 5b5cb205b9ed..d60ca849175e 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -1153,10 +1153,12 @@ static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
 				LRADC_CTRL1_LRADC_IRQ(TS_VCH2));
 	}
 
-	if (iio_buffer_enabled(iio))
-		iio_trigger_poll(iio->trig);
-	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
+	if (iio_buffer_enabled(iio)) {
+		if (reg & lradc->buffer_vchans)
+			iio_trigger_poll(iio->trig);
+	} else if (reg & LRADC_CTRL1_LRADC_IRQ(0)) {
 		complete(&lradc->completion);
+	}
 
 	mxs_lradc_reg_clear(lradc, reg & clr_irq, LRADC_CTRL1);
 
-- 
2.2.0

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

* Re: [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels
  2015-01-17  0:22 ` [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels Kristina Martšenko
@ 2015-01-18  0:19   ` Marek Vasut
  2015-01-19 19:02     ` Kristina Martšenko
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Vasut @ 2015-01-18  0:19 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On Saturday, January 17, 2015 at 01:22:08 AM, Kristina Mart=C5=A1enko wrote:

Hi!

Good stuff, thank you! Just minor nitpicks below.

[...]

> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index e0e91836eec1..fc65cd311be9
> 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -214,11 +214,14 @@ struct mxs_lradc {
>  	unsigned long		is_divided;
>=20
>  	/*
> -	 * Touchscreen LRADC channels receives a private slot in the CTRL4
> -	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
> -	 * CTRL4 register can be mapped to LRADC channels when using the
> -	 * touchscreen.
> -	 *
> +	 * When the touchscreen is enabled, we give it two private virtual
> +	 * channels: #6 and #7. This means that only 6 virtual channels (instead
> +	 * of 8) will be available for buffered capture.
> +	 */
> +#define TS_VCH1		7
> +#define TS_VCH2		6

Please use a bit more explicit name for the macro, it's really not clear wh=
at
the macro represents from it's name. Something like TOUCHSCREEN_VCHANNEL1 m=
ight
work better for example. What do you think ?

> +
> +	/*
>  	 * Furthermore, certain LRADC channels are shared between touchscreen
>  	 * and/or touch-buttons and generic LRADC block. Therefore when using
>  	 * either of these, these channels are not available for the regular
[...]

I also have a general question/idea here, it's explicitly not something I'd=
 like
to force upon you to implement. I see we have some kind of a hardware, whic=
h can
sample up-to N inputs in parallel . Each input is muxed between M possible=
=20
sources . Is such a thing common in the ADC/DAC world ? Would it be worth to
implement generic helper to handle this kind of a N:M mapping ? What do you=
 all
think please ?

Best regards,
Marek Vasut

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

* Re: [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts
  2015-01-17  0:22 ` [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts Kristina Martšenko
@ 2015-01-18  0:21   ` Marek Vasut
  2015-01-19 19:03     ` Kristina Martšenko
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Vasut @ 2015-01-18  0:21 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On Saturday, January 17, 2015 at 01:22:09 AM, Kristina Mart=C5=A1enko wrote:
> Reading a channel through sysfs, or starting a buffered capture, will
> currently turn off the touchscreen. This is because the read_raw() and
> buffer preenable()/postdisable() callbacks disable interrupts for all
> LRADC channels, including those the touchscreen uses.
>=20
> So make the callbacks only disable interrupts for the channels they use.
> This means channel 0 for read_raw() and channels 0-5 for the buffer (if
> the touchscreen is enabled). Since the touchscreen uses different
> channels (6 and 7), it no longer gets turned off.
>=20
> Note that only i.MX28 is affected by this issue, i.MX23 should be fine.
>=20
> Signed-off-by: Kristina Mart=C5=A1enko <kristina.martsenko@gmail.com>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>=20
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index fc65cd311be9..0cf276ff0dc5
> 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -218,8 +218,11 @@ struct mxs_lradc {
>  	 * channels: #6 and #7. This means that only 6 virtual channels (instead
>  	 * of 8) will be available for buffered capture.
>  	 */
> -#define TS_VCH1		7
> -#define TS_VCH2		6
> +#define TS_VCH1				7
> +#define TS_VCH2				6

Please fix the indent in 1/4 , so you don't have to change it again this pa=
tch.

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks!

Best regards,
Marek Vasut

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

* Re: [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions
  2015-01-17  0:22 ` [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions Kristina Martšenko
@ 2015-01-18  0:22   ` Marek Vasut
  2015-01-19  7:20   ` Juergen Borleis
  1 sibling, 0 replies; 18+ messages in thread
From: Marek Vasut @ 2015-01-18  0:22 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On Saturday, January 17, 2015 at 01:22:10 AM, Kristina Mart=C5=A1enko wrote:
> Reading a channel through sysfs, or starting a buffered capture, can
> occasionally turn off the touchscreen.
>=20
> This is because the read_raw() and buffer preenable()/postdisable()
> callbacks unschedule current conversions on all channels. If a delay
> channel happens to schedule a touchscreen conversion at the same time,
> the conversion gets cancelled and the touchscreen sequence stops.
>=20
> This is probably related to this note from the reference manual:
>=20
> 	"If a delay group schedules channels to be sampled and a manual
> 	write to the schedule field in CTRL0 occurs while the block is
> 	discarding samples, the LRADC will switch to the new schedule
> 	and will not sample the channels that were previously scheduled.
> 	The time window for this to happen is very small and lasts only
> 	while the LRADC is discarding samples."
>=20
> So make the callbacks only unschedule conversions for the channels they
> use. This means channel 0 for read_raw() and channels 0-5 for the buffer
> (if the touchscreen is enabled). Since the touchscreen uses different
> channels (6 and 7), it no longer gets turned off.
>=20
> This is tested and fixes the issue on i.MX28, but hasn't been tested on
> i.MX23.
>=20
> Signed-off-by: Kristina Mart=C5=A1enko <kristina.martsenko@gmail.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

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

* Re: [PATCH 4/4] iio: mxs-lradc: only update the buffer when its conversions have finished
  2015-01-17  0:22 ` [PATCH 4/4] iio: mxs-lradc: only update the buffer when its conversions have finished Kristina Martšenko
@ 2015-01-18  0:23   ` Marek Vasut
  0 siblings, 0 replies; 18+ messages in thread
From: Marek Vasut @ 2015-01-18  0:23 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On Saturday, January 17, 2015 at 01:22:11 AM, Kristina Mart=C5=A1enko wrote:
> Using the touchscreen while running buffered capture results in the
> buffer reporting lots of wrong values, often just zeros. This is because
> we push readings to the buffer every time a touchscreen interrupt
> arrives, including when the buffer's own conversions have not yet
> finished. So let's only push to the buffer when its conversions are
> ready.
>=20
> Signed-off-by: Kristina Mart=C5=A1enko <kristina.martsenko@gmail.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

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

* Re: [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC
  2015-01-17  0:22 [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Kristina Martšenko
                   ` (3 preceding siblings ...)
  2015-01-17  0:22 ` [PATCH 4/4] iio: mxs-lradc: only update the buffer when its conversions have finished Kristina Martšenko
@ 2015-01-18 11:21 ` Stefan Wahren
  2015-01-19 20:30   ` Kristina Martšenko
  4 siblings, 1 reply; 18+ messages in thread
From: Stefan Wahren @ 2015-01-18 11:21 UTC (permalink / raw)
  To: "Kristina Martšenko"
  Cc: Peter Meerwald, Greg Kroah-Hartman, Hartmut Knaack,
	Lars-Peter Clausen, devel, Alexandre Belloni, Juergen Beisert,
	Fabio Estevam, linux-iio, Marek Vasut, Jonathan Cameron

Hi Kristina,

> Kristina Mart=C5=A1enko <kristina.martsenko@gmail.com> hat am 17. Januar =
2015 um
> 01:22 geschrieben:
>
>
> Hi,
>
> These patches fix some issues with using the touchscreen and reading
> other ADC channels at the same time.
>
> The main issue is that the "virtual" channels the touchscreen uses can
> overlap with the channels that the iio buffer is configured to read,
> causing the touchscreen to read those channels instead, and report wrong
> values. Patch #1 fixes that by separating the channels they use.
>
> Patches #2 and #3 stop buffered mode and sysfs reads from disabling the
> touchscreen. Patch #4 keeps the buffer from reporting wrong values when
> the touchscreen is in use.
>
> Note that I've only tested these patches on i.MX28. I checked the i.MX23
> reference manual [1], and there don't seem to be any differences in the
> parts I changed, but it would still be nice if someone could test on
> i.MX23 as well.

i have a iMX233-Olinuxino-MAXI, but no touchscreen. If it does make sense
to you, could you please send me testing instructions and i'll give it a tr=
y?

Stefan

>
> The patches were tested on next-20150113 and rebased onto iio/togreg.
>
> Thanks,
> Kristina

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

* Re: [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions
  2015-01-17  0:22 ` [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions Kristina Martšenko
  2015-01-18  0:22   ` Marek Vasut
@ 2015-01-19  7:20   ` Juergen Borleis
  2015-01-19  7:57     ` Marek Vasut
  1 sibling, 1 reply; 18+ messages in thread
From: Juergen Borleis @ 2015-01-19  7:20 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Marek Vasut, Alexandre Belloni, Fabio Estevam,
	Stefan Wahren, Greg Kroah-Hartman, linux-iio, devel

Hi Kristina,

On Saturday 17 January 2015 01:22:10 Kristina Martšenko wrote:
> Reading a channel through sysfs, or starting a buffered capture, can
> occasionally turn off the touchscreen.
> [...]

I have my old ChumbyOne around (i.MX23). How to test this buffered capture 
while using the touchscreen part of the ADC (I have never used the IIO 
before).

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Borleis             |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

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

* Re: [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions
  2015-01-19  7:20   ` Juergen Borleis
@ 2015-01-19  7:57     ` Marek Vasut
  2015-01-19 20:03       ` Kristina Martšenko
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Vasut @ 2015-01-19  7:57 UTC (permalink / raw)
  To: Juergen Borleis
  Cc: Kristina Martšenko, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On Monday, January 19, 2015 at 08:20:31 AM, Juergen Borleis wrote:
> Hi Kristina,

Hi all,

> On Saturday 17 January 2015 01:22:10 Kristina Martšenko wrote:
> > Reading a channel through sysfs, or starting a buffered capture, can
> > occasionally turn off the touchscreen.
> > [...]
> 
> I have my old ChumbyOne around (i.MX23). How to test this buffered capture
> while using the touchscreen part of the ADC (I have never used the IIO
> before).

Last time I checked there was drivers/staging/iio/Documentation/generic_buffer.c 
, but you can also wait for a more precise advice from Kristina . Hope it helps
at least a bit ;-)

Best regards,
Marek Vasut

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

* Re: [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels
  2015-01-18  0:19   ` Marek Vasut
@ 2015-01-19 19:02     ` Kristina Martšenko
  2015-01-20  1:29       ` Marek Vasut
  0 siblings, 1 reply; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-19 19:02 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On 18/01/15 02:19, Marek Vasut wrote:
> On Saturday, January 17, 2015 at 01:22:08 AM, Kristina Martšenko wrote:
> 
> Hi!

Hi!

> Good stuff, thank you! Just minor nitpicks below.
>
> [...]
> 
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>> b/drivers/staging/iio/adc/mxs-lradc.c index e0e91836eec1..fc65cd311be9
>> 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -214,11 +214,14 @@ struct mxs_lradc {
>>  	unsigned long		is_divided;
>>
>>  	/*
>> -	 * Touchscreen LRADC channels receives a private slot in the CTRL4
>> -	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
>> -	 * CTRL4 register can be mapped to LRADC channels when using the
>> -	 * touchscreen.
>> -	 *
>> +	 * When the touchscreen is enabled, we give it two private virtual
>> +	 * channels: #6 and #7. This means that only 6 virtual channels (instead
>> +	 * of 8) will be available for buffered capture.
>> +	 */
>> +#define TS_VCH1		7
>> +#define TS_VCH2		6
> 
> Please use a bit more explicit name for the macro, it's really not clear what
> the macro represents from it's name. Something like TOUCHSCREEN_VCHANNEL1 might
> work better for example. What do you think ?

Sure, I tried to keep it short and similar to the TS_CH_YP (etc) macros,
but clearer is better, so I'll change it to TOUCHSCREEN_VCHANNEL1 in v2.

>> +
>> +	/*
>>  	 * Furthermore, certain LRADC channels are shared between touchscreen
>>  	 * and/or touch-buttons and generic LRADC block. Therefore when using
>>  	 * either of these, these channels are not available for the regular
> [...]
> 
> I also have a general question/idea here, it's explicitly not something I'd like
> to force upon you to implement. I see we have some kind of a hardware, which can
> sample up-to N inputs in parallel . Each input is muxed between M possible 
> sources . Is such a thing common in the ADC/DAC world ? Would it be worth to
> implement generic helper to handle this kind of a N:M mapping ? What do you all
> think please ?

Hmm, I don't know how other drivers do this, or if there's anything in
common. The IIO people will probably have a better idea, so I'll wait
for them to respond.

Kristina

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

* Re: [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts
  2015-01-18  0:21   ` Marek Vasut
@ 2015-01-19 19:03     ` Kristina Martšenko
  0 siblings, 0 replies; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-19 19:03 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On 18/01/15 02:21, Marek Vasut wrote:
> On Saturday, January 17, 2015 at 01:22:09 AM, Kristina Martšenko wrote:
>> Reading a channel through sysfs, or starting a buffered capture, will
>> currently turn off the touchscreen. This is because the read_raw() and
>> buffer preenable()/postdisable() callbacks disable interrupts for all
>> LRADC channels, including those the touchscreen uses.
>>
>> So make the callbacks only disable interrupts for the channels they use.
>> This means channel 0 for read_raw() and channels 0-5 for the buffer (if
>> the touchscreen is enabled). Since the touchscreen uses different
>> channels (6 and 7), it no longer gets turned off.
>>
>> Note that only i.MX28 is affected by this issue, i.MX23 should be fine.
>>
>> Signed-off-by: Kristina Martšenko <kristina.martsenko@gmail.com>
>> ---
>>  drivers/staging/iio/adc/mxs-lradc.c | 24 +++++++++++++++++-------
>>  1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>> b/drivers/staging/iio/adc/mxs-lradc.c index fc65cd311be9..0cf276ff0dc5
>> 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -218,8 +218,11 @@ struct mxs_lradc {
>>  	 * channels: #6 and #7. This means that only 6 virtual channels (instead
>>  	 * of 8) will be available for buffered capture.
>>  	 */
>> -#define TS_VCH1		7
>> -#define TS_VCH2		6
>> +#define TS_VCH1				7
>> +#define TS_VCH2				6
> 
> Please fix the indent in 1/4 , so you don't have to change it again this patch.

Right, of course, I'll do that in v2.

> Reviewed-by: Marek Vasut <marex@denx.de>

Thanks for taking time to review all the patches!

Kristina

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

* Re: [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions
  2015-01-19  7:57     ` Marek Vasut
@ 2015-01-19 20:03       ` Kristina Martšenko
  0 siblings, 0 replies; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-19 20:03 UTC (permalink / raw)
  To: Marek Vasut, Juergen Borleis
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Alexandre Belloni, Fabio Estevam, Stefan Wahren,
	Greg Kroah-Hartman, linux-iio, devel

On 19/01/15 09:57, Marek Vasut wrote:
> On Monday, January 19, 2015 at 08:20:31 AM, Juergen Borleis wrote:
>> On Saturday 17 January 2015 01:22:10 Kristina Martšenko wrote:
>>> Reading a channel through sysfs, or starting a buffered capture, can
>>> occasionally turn off the touchscreen.
>>> [...]
>>
>> I have my old ChumbyOne around (i.MX23). How to test this buffered capture
>> while using the touchscreen part of the ADC (I have never used the IIO
>> before).

That's great, thanks for offering to test this.

> Last time I checked there was drivers/staging/iio/Documentation/generic_buffer.c 
> , but you can also wait for a more precise advice from Kristina . Hope it helps
> at least a bit ;-)

Yeah, I used generic_buffer.c for testing.

You can enable some channels like this:

echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltage7_en
echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltage10_en
echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltage12_en
echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltage13_en
echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltage14_en
echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltage15_en

And then start the capture and read 10 samples with generic_buffer:

./generic_buffer -n 80050000.lradc -c 10

And using the touchscreen at the same time should work now. Let me know
if it doesn't :)

Kristina

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

* Re: [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC
  2015-01-18 11:21 ` [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Stefan Wahren
@ 2015-01-19 20:30   ` Kristina Martšenko
  2015-01-20  1:32     ` Marek Vasut
  0 siblings, 1 reply; 18+ messages in thread
From: Kristina Martšenko @ 2015-01-19 20:30 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Peter Meerwald, Greg Kroah-Hartman, Hartmut Knaack,
	Lars-Peter Clausen, devel, Alexandre Belloni, Juergen Beisert,
	Fabio Estevam, linux-iio, Marek Vasut, Jonathan Cameron

On 18/01/15 13:21, Stefan Wahren wrote:
> Hi Kristina,

Hi Stefan!

>> Kristina Martšenko <kristina.martsenko@gmail.com> hat am 17. Januar 2015 um
>> 01:22 geschrieben:
>> These patches fix some issues with using the touchscreen and reading
>> other ADC channels at the same time.
>>
>> The main issue is that the "virtual" channels the touchscreen uses can
>> overlap with the channels that the iio buffer is configured to read,
>> causing the touchscreen to read those channels instead, and report wrong
>> values. Patch #1 fixes that by separating the channels they use.
>>
>> Patches #2 and #3 stop buffered mode and sysfs reads from disabling the
>> touchscreen. Patch #4 keeps the buffer from reporting wrong values when
>> the touchscreen is in use.
>>
>> Note that I've only tested these patches on i.MX28. I checked the i.MX23
>> reference manual [1], and there don't seem to be any differences in the
>> parts I changed, but it would still be nice if someone could test on
>> i.MX23 as well.
> 
> i have a iMX233-Olinuxino-MAXI, but no touchscreen. If it does make sense
> to you, could you please send me testing instructions and i'll give it a try?

Thanks a lot for offering! Unfortunately in this case all testing
involves using the touchscreen while reading other ADC channels, so
without a touchscreen there isn't much to test, sorry :(

Kristina

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

* Re: [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels
  2015-01-19 19:02     ` Kristina Martšenko
@ 2015-01-20  1:29       ` Marek Vasut
  0 siblings, 0 replies; 18+ messages in thread
From: Marek Vasut @ 2015-01-20  1:29 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Juergen Beisert, Alexandre Belloni,
	Fabio Estevam, Stefan Wahren, Greg Kroah-Hartman, linux-iio,
	devel

On Monday, January 19, 2015 at 08:02:03 PM, Kristina Martšenko wrote:
> On 18/01/15 02:19, Marek Vasut wrote:
> > On Saturday, January 17, 2015 at 01:22:08 AM, Kristina Martšenko wrote:
> > 
> > Hi!
> 
> Hi!

Hi!

> > Good stuff, thank you! Just minor nitpicks below.
> > 
> > [...]
> > 
> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> >> b/drivers/staging/iio/adc/mxs-lradc.c index e0e91836eec1..fc65cd311be9
> >> 100644
> >> --- a/drivers/staging/iio/adc/mxs-lradc.c
> >> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> >> @@ -214,11 +214,14 @@ struct mxs_lradc {
> >> 
> >>  	unsigned long		is_divided;
> >>  	
> >>  	/*
> >> 
> >> -	 * Touchscreen LRADC channels receives a private slot in the CTRL4
> >> -	 * register, the slot #7. Therefore only 7 slots instead of 8 in the
> >> -	 * CTRL4 register can be mapped to LRADC channels when using the
> >> -	 * touchscreen.
> >> -	 *
> >> +	 * When the touchscreen is enabled, we give it two private virtual
> >> +	 * channels: #6 and #7. This means that only 6 virtual channels
> >> (instead +	 * of 8) will be available for buffered capture.
> >> +	 */
> >> +#define TS_VCH1		7
> >> +#define TS_VCH2		6
> > 
> > Please use a bit more explicit name for the macro, it's really not clear
> > what the macro represents from it's name. Something like
> > TOUCHSCREEN_VCHANNEL1 might work better for example. What do you think ?
> 
> Sure, I tried to keep it short and similar to the TS_CH_YP (etc) macros,
> but clearer is better, so I'll change it to TOUCHSCREEN_VCHANNEL1 in v2.

Thank you!

> >> +
> >> +	/*
> >> 
> >>  	 * Furthermore, certain LRADC channels are shared between touchscreen
> >>  	 * and/or touch-buttons and generic LRADC block. Therefore when using
> >>  	 * either of these, these channels are not available for the regular
> > 
> > [...]
> > 
> > I also have a general question/idea here, it's explicitly not something
> > I'd like to force upon you to implement. I see we have some kind of a
> > hardware, which can sample up-to N inputs in parallel . Each input is
> > muxed between M possible sources . Is such a thing common in the ADC/DAC
> > world ? Would it be worth to implement generic helper to handle this
> > kind of a N:M mapping ? What do you all think please ?
> 
> Hmm, I don't know how other drivers do this, or if there's anything in
> common. The IIO people will probably have a better idea, so I'll wait
> for them to respond.

Yep.

Best regards,
Marek Vasut

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

* Re: [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC
  2015-01-19 20:30   ` Kristina Martšenko
@ 2015-01-20  1:32     ` Marek Vasut
  0 siblings, 0 replies; 18+ messages in thread
From: Marek Vasut @ 2015-01-20  1:32 UTC (permalink / raw)
  To: Kristina Martšenko
  Cc: Stefan Wahren, Peter Meerwald, Greg Kroah-Hartman,
	Hartmut Knaack, Lars-Peter Clausen, devel, Alexandre Belloni,
	Juergen Beisert, Fabio Estevam, linux-iio, Jonathan Cameron

On Monday, January 19, 2015 at 09:30:47 PM, Kristina Martšenko wrote:
> On 18/01/15 13:21, Stefan Wahren wrote:
> > Hi Kristina,
> 
> Hi Stefan!

Hi all,

[...]

> > i have a iMX233-Olinuxino-MAXI, but no touchscreen. If it does make sense
> > to you, could you please send me testing instructions and i'll give it a
> > try?
> 
> Thanks a lot for offering! Unfortunately in this case all testing
> involves using the touchscreen while reading other ADC channels, so
> without a touchscreen there isn't much to test, sorry :(

Well it's a resistive touchscreen, when push comes to shove, Stefan can always 
use a couple of variable resistors to simulate it (haha) :)

Best regards,
Marek Vasut

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

end of thread, other threads:[~2015-01-20  1:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-17  0:22 [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Kristina Martšenko
2015-01-17  0:22 ` [PATCH 1/4] iio: mxs-lradc: separate touchscreen and buffer virtual channels Kristina Martšenko
2015-01-18  0:19   ` Marek Vasut
2015-01-19 19:02     ` Kristina Martšenko
2015-01-20  1:29       ` Marek Vasut
2015-01-17  0:22 ` [PATCH 2/4] iio: mxs-lradc: make ADC reads not disable touchscreen interrupts Kristina Martšenko
2015-01-18  0:21   ` Marek Vasut
2015-01-19 19:03     ` Kristina Martšenko
2015-01-17  0:22 ` [PATCH 3/4] iio: mxs-lradc: make ADC reads not unschedule touchscreen conversions Kristina Martšenko
2015-01-18  0:22   ` Marek Vasut
2015-01-19  7:20   ` Juergen Borleis
2015-01-19  7:57     ` Marek Vasut
2015-01-19 20:03       ` Kristina Martšenko
2015-01-17  0:22 ` [PATCH 4/4] iio: mxs-lradc: only update the buffer when its conversions have finished Kristina Martšenko
2015-01-18  0:23   ` Marek Vasut
2015-01-18 11:21 ` [PATCH 0/4] iio: mxs-lradc: fix interactions between the touchscreen and the ADC Stefan Wahren
2015-01-19 20:30   ` Kristina Martšenko
2015-01-20  1:32     ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.