All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Complete the mrst touchscreen tidy
@ 2010-07-23 13:51 Alan Cox
  2010-07-23 13:51 ` [PATCH 1/6] mrst_touchscreen: clean up input side Alan Cox
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:51 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

If these are all ok it can then move from staging
---

Alan Cox (1):
      mrst_touchscreen: clean up input side

Alek Du (1):
      *	Register platform interface

Andy Ross (1):
      Simplify en/disable of interrupts for NEC.

Arjan van de Ven (2):
      fix channel allocation in the touch screen driver
      mrst-touchscreen: Fix use before initialize in mrst_touch   [Fix bug 2561]

Dmitry Torokhov (1):
      Input: mrst - more fixes


 drivers/staging/Makefile                           |    2 
 drivers/staging/mrst-touchscreen/Kconfig           |    2 
 drivers/staging/mrst-touchscreen/Makefile          |    2 
 drivers/staging/mrst-touchscreen/TODO              |    2 
 drivers/staging/mrst-touchscreen/intel-mid-touch.c |  941 ++++++++------------
 5 files changed, 384 insertions(+), 565 deletions(-)

--
"What they forget is their girlfriends don't care if they've got a brand new
 iPhone4, they care if they remember to phone..." 


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

* [PATCH 1/6] mrst_touchscreen: clean up input side
  2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
@ 2010-07-23 13:51 ` Alan Cox
  2010-07-27 18:18   ` [PATCH 1/6] Staging: " Greg KH
  2010-07-23 13:52 ` [PATCH 2/6] Input: mrst - more fixes Alan Cox
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:51 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

Fix most of the stuff that Dmitry pointed out. This leaves the mutex in IRQ
and misuse of SPI to sort out.

Also fix the build bits so it actually builds in staging - whoops.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/staging/Makefile                           |    2 
 drivers/staging/mrst-touchscreen/Kconfig           |    2 
 drivers/staging/mrst-touchscreen/Makefile          |    2 
 drivers/staging/mrst-touchscreen/TODO              |    2 
 drivers/staging/mrst-touchscreen/intel-mid-touch.c |   98 ++++----------------
 5 files changed, 25 insertions(+), 81 deletions(-)


diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 26942aa..4df0df9 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -52,7 +52,7 @@ obj-$(CONFIG_CXT1E1)		+= cxt1e1/
 obj-$(CONFIG_TI_ST)		+= ti-st/
 obj-$(CONFIG_ADIS16255)		+= adis16255/
 obj-$(CONFIG_FB_XGI)		+= xgifb/
-obj-$(CONFIG_TOUCHSCREEN_MRSTOUCH)	+= mrst-touchscreen/
+obj-$(CONFIG_TOUCHSCREEN_INTEL_MID)	+= mrst-touchscreen/
 obj-$(CONFIG_MSM_STAGING)	+= msm/
 obj-$(CONFIG_EASYCAP)		+= easycap/
 obj-$(CONFIG_SOLO6X10)		+= solo6x10/
diff --git a/drivers/staging/mrst-touchscreen/Kconfig b/drivers/staging/mrst-touchscreen/Kconfig
index c2af492..775d740 100644
--- a/drivers/staging/mrst-touchscreen/Kconfig
+++ b/drivers/staging/mrst-touchscreen/Kconfig
@@ -1,6 +1,6 @@
 config TOUCHSCREEN_INTEL_MID
 	tristate "Intel MID platform resistive touchscreen"
-	depends on INTEL_SCU_IPC
+	depends on INTEL_SCU_IPC && INPUT_TOUCHSCREEN
 	default y
 	help
 	  Say Y here if you have a Intel MID based touchscreen
diff --git a/drivers/staging/mrst-touchscreen/Makefile b/drivers/staging/mrst-touchscreen/Makefile
index 2d638b0..155f58b 100644
--- a/drivers/staging/mrst-touchscreen/Makefile
+++ b/drivers/staging/mrst-touchscreen/Makefile
@@ -1,3 +1,3 @@
-obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) := intel_mid_touch.o
+obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) := intel-mid-touch.o
 
 
diff --git a/drivers/staging/mrst-touchscreen/TODO b/drivers/staging/mrst-touchscreen/TODO
index 7157028..84aa695 100644
--- a/drivers/staging/mrst-touchscreen/TODO
+++ b/drivers/staging/mrst-touchscreen/TODO
@@ -1,2 +1,4 @@
 - Move the driver to not think it is SPI (requires fixing some of the SFI
   and firmware side)
+- Fix mutex in IRQ stuff - threaded IRQ ?
+
diff --git a/drivers/staging/mrst-touchscreen/intel-mid-touch.c b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
index abba22f..6fed01e 100644
--- a/drivers/staging/mrst-touchscreen/intel-mid-touch.c
+++ b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
@@ -23,7 +23,6 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
  * TODO:
- *	kill off mrstouch_debug eventually
  *	review conversion of r/m/w sequences
  *	Replace interrupt mutex abuse
  *	Kill of mrstouchdevp pointer
@@ -43,16 +42,6 @@
 #include <asm/intel_scu_ipc.h>
 
 
-#if defined(MRSTOUCH_DEBUG)
-#define mrstouch_debug(fmt, args...)\
-	do { \
-		printk(KERN_DEBUG "\n[MRSTOUCH(%d)] - ", __LINE__); \
-		printk(KERN_DEBUG fmt, ##args); \
-	} while (0);
-#else
-#define mrstouch_debug(fmt, args...)
-#endif
-
 /* PMIC Interrupt registers */
 #define PMIC_REG_ID1   0x00 /*PMIC ID1 register */
 
@@ -100,6 +89,9 @@
 #define MAX_X		1024
 #define MIN_Y		10
 #define MAX_Y		1024
+#define MIN_P		0
+#define MAX_P		TOUCH_PRESSURE_FS
+
 #define WAIT_ADC_COMPLETION 10
 
 /* PMIC ADC round robin delays */
@@ -124,8 +116,6 @@ struct mrstouch_dev {
 	int             irq;    /* Touch screen IRQ # */
 	uint		vendor;  /* PMIC vendor */
 	uint		rev;  /* PMIC revision */
-	bool		suspended; /* Device suspended status */
-	bool		disabled;  /* Device disabled status */
 	u16		x;  /* X coordinate */
 	u16		y;  /* Y coordinate */
 	bool		pendown;  /* PEN position */
@@ -320,8 +310,6 @@ static int mrstouch_adc_init(struct mrstouch_dev *tsdev)
 
 	tsdev->asr = start;
 
-	mrstouch_debug("Channel offset(%d): 0x%X\n", tsdev->asr, tsdev->vendor);
-
 	/* ADC power on, start, enable PENDET and set loop delay
 	 * ADC loop delay is set to 4.5 ms approximately
 	 * Loop delay more than this results in jitter in adc readings
@@ -355,7 +343,6 @@ static void mrstouch_report_xy(struct mrstouch_dev *tsdev, u16 x, u16 y, u16 z)
 
 	if (tsdev->pendown && z <= TOUCH_PRESSURE) {
 		/* Pen removed, report button release */
-		mrstouch_debug("BTN REL(%d)", z);
 		input_report_key(tsdev->input, BTN_TOUCH, 0);
 		tsdev->pendown = false;
 	}
@@ -371,7 +358,6 @@ static void mrstouch_report_xy(struct mrstouch_dev *tsdev, u16 x, u16 y, u16 z)
 	if (x < MIN_X || x > MAX_X || y < MIN_Y || y > MAX_Y) {
 		/* Spurious values, release button if touched and return */
 		if (tsdev->pendown) {
-			mrstouch_debug("BTN REL(%d)", z);
 			input_report_key(tsdev->input, BTN_TOUCH, 0);
 			tsdev->pendown = false;
 		}
@@ -382,13 +368,13 @@ static void mrstouch_report_xy(struct mrstouch_dev *tsdev, u16 x, u16 y, u16 z)
 
 		input_report_abs(tsdev->input, ABS_X, x);
 		input_report_abs(tsdev->input, ABS_Y, y);
+		input_report_abs(tsdev->input, ABS_PRESSURE, z);
 		input_sync(tsdev->input);
 	}
 
 
 	if (!tsdev->pendown && z > TOUCH_PRESSURE) {
 		/* Pen touched, report button touch */
-		mrstouch_debug("BTN TCH(%d, %d, %d)", x, y, z);
 		input_report_key(tsdev->input, BTN_TOUCH, 1);
 		tsdev->pendown = true;
 	}
@@ -446,18 +432,7 @@ static int mrstouch_pmic_fs_adc_read(struct mrstouch_dev *tsdev)
 	z |= data[1] & 0x7; /* Lower 3 bits */
 	z &= 0x3FF;
 
-#if defined(MRSTOUCH_PRINT_XYZP)
-	mrstouch_debug("X: %d, Y: %d, Z: %d", x, y, z);
-#endif
-
-	if (z >= TOUCH_PRESSURE_FS) {
-		mrstouch_report_xy(tsdev, x, y, TOUCH_PRESSURE - 1); /* Pen Removed */
-		return TOUCH_PRESSURE - 1;
-	} else {
-		mrstouch_report_xy(tsdev, x, y, TOUCH_PRESSURE + 1); /* Pen Touched */
-		return TOUCH_PRESSURE + 1;
-	}
-
+	mrstouch_report_xy(tsdev, x, y, z);
 	return 0;
 
 ipc_error:
@@ -615,14 +590,6 @@ static int mrstouch_adc_read(struct mrstouch_dev *tsdev)
 	if (err)
 		goto ipc_error;
 
-#if defined(MRSTOUCH_PRINT_XYZP)
-	printk(KERN_INFO "X+: %d, Y+: %d, Z+: %d\n", xp, yp, zp);
-#endif
-
-#if defined(MRSTOUCH_PRINT_XYZM)
-	printk(KERN_INFO "X-: %d, Y-: %d, Z-: %d\n", xm, ym, zm);
-#endif
-
 	mrstouch_report_xy(tsdev, xp, yp, zp); /* report x and y to eventX */
 
 	return zp;
@@ -694,12 +661,10 @@ static int ts_input_dev_init(struct mrstouch_dev *tsdev, struct spi_device *spi)
 {
 	int err = 0;
 
-	mrstouch_debug("%s", __func__);
-
 	tsdev->input = input_allocate_device();
 	if (!tsdev->input) {
 		dev_err(&tsdev->spi->dev, "Unable to allocate input device.\n");
-		return -EINVAL;
+		return -ENOMEM;
 	}
 
 	tsdev->input->name = "mrst_touchscreen";
@@ -714,8 +679,9 @@ static int ts_input_dev_init(struct mrstouch_dev *tsdev, struct spi_device *spi)
 	tsdev->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	tsdev->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	input_set_abs_params(tsdev->input, ABS_X, MIN_X, MIN_Y, 0, 0);
-	input_set_abs_params(tsdev->input, ABS_Y, MIN_X, MIN_Y, 0, 0);
+	input_set_abs_params(tsdev->input, ABS_X, MIN_X, MAX_X, 0, 0);
+	input_set_abs_params(tsdev->input, ABS_Y, MIN_Y, MAX_Y, 0, 0);
+	input_set_abs_params(tsdev->input, ABS_PRESSURE, MIN_P, MAX_P, 0, 0);
 
 	err = input_register_device(tsdev->input);
 	if (err) {
@@ -723,9 +689,6 @@ static int ts_input_dev_init(struct mrstouch_dev *tsdev, struct spi_device *spi)
 		input_free_device(tsdev->input);
 		return err;
 	}
-
-	mrstouch_debug("%s", "mrstouch initialized");
-
 	return 0;
 
 }
@@ -737,8 +700,6 @@ static int __devinit mrstouch_probe(struct spi_device *mrstouch_spi)
 	unsigned int myirq;
 	struct mrstouch_dev *tsdev;
 
-	mrstouch_debug("%s(%p)", __func__, mrstouch_spi);
-
 	mrstouchdevp = NULL;
 	myirq = mrstouch_spi->irq;
 
@@ -768,18 +729,17 @@ static int __devinit mrstouch_probe(struct spi_device *mrstouch_spi)
 	err = ts_input_dev_init(tsdev, mrstouch_spi);
 	if (err) {
 		dev_err(&tsdev->spi->dev, "ts_input_dev_init failed");
-		goto mrstouch_err_free_mem;
+		goto mrstouch_err_free_dev;
 	}
 
 	mutex_init(&tsdev->lock);
-	mutex_lock(&tsdev->lock)
+	mutex_lock(&tsdev->lock);
 
-	mrstouch_debug("Requesting IRQ-%d", myirq);
 	err = request_irq(myirq, pendet_intr_handler,
 				0, "mrstouch", tsdev);
 	if (err) {
 		dev_err(&tsdev->spi->dev, "unable to allocate irq\n");
-		goto mrstouch_err_free_mem;
+		goto mrstouch_err_free_dev;
 	}
 
 	tsdev->pendet_thrd = kthread_run(mrstouch_pendet,
@@ -787,38 +747,24 @@ static int __devinit mrstouch_probe(struct spi_device *mrstouch_spi)
 	if (IS_ERR(tsdev->pendet_thrd)) {
 		dev_err(&tsdev->spi->dev, "kthread_run failed\n");
 		err = PTR_ERR(tsdev->pendet_thrd);
-		goto mrstouch_err_free_mem;
+		goto mrstouch_err_free_irq;
 	}
-	mrstouch_debug("%s", "Driver initialized");
 	return 0;
-
+mrstouch_err_free_irq:
+	free_irq(myirq, tsdev);
+mrstouch_err_free_dev:
+	input_unregister_device(tsdev->input);
 mrstouch_err_free_mem:
 	kfree(tsdev);
 	return err;
 }
 
-static int mrstouch_suspend(struct spi_device *spi, pm_message_t msg)
-{
-	mrstouch_debug("%s", __func__);
-	mrstouchdevp->suspended = 1;
-	return 0;
-}
-
-static int mrstouch_resume(struct spi_device *spi)
-{
-	mrstouch_debug("%s", __func__);
-	mrstouchdevp->suspended = 0;
-	return 0;
-}
-
 static int mrstouch_remove(struct spi_device *spi)
 {
-	mrstouch_debug("%s", __func__);
 	free_irq(mrstouchdevp->irq, mrstouchdevp);
-	input_unregister_device(mrstouchdevp->input);
-	input_free_device(mrstouchdevp->input);
 	if (mrstouchdevp->pendet_thrd)
 		kthread_stop(mrstouchdevp->pendet_thrd);
+	input_unregister_device(mrstouchdevp->input);
 	kfree(mrstouchdevp);
 	return 0;
 }
@@ -830,8 +776,6 @@ static struct spi_driver mrstouch_driver = {
 		.owner  = THIS_MODULE,
 	},
 	.probe          = mrstouch_probe,
-	.suspend        = mrstouch_suspend,
-	.resume         = mrstouch_resume,
 	.remove         = mrstouch_remove,
 };
 
@@ -839,11 +783,10 @@ static int __init mrstouch_module_init(void)
 {
 	int err;
 
-	mrstouch_debug("%s", __func__);
 	err = spi_register_driver(&mrstouch_driver);
 	if (err) {
-		mrstouch_debug("%s(%d)", "SPI PENDET failed", err);
-		return -1;
+		printk(KERN_ERR "%s(%d)", "SPI PENDET failed", err);
+		return err;;
 	}
 
 	return 0;
@@ -851,7 +794,6 @@ static int __init mrstouch_module_init(void)
 
 static void __exit mrstouch_module_exit(void)
 {
-	mrstouch_debug("%s", __func__);
 	spi_unregister_driver(&mrstouch_driver);
 	return;
 }


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

* [PATCH 2/6] Input: mrst - more fixes
  2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
  2010-07-23 13:51 ` [PATCH 1/6] mrst_touchscreen: clean up input side Alan Cox
@ 2010-07-23 13:52 ` Alan Cox
  2010-07-23 13:52 ` [PATCH 3/6] mrst-touchscreen: Fix use before initialize in mrst_touch [Fix bug 2561] Alan Cox
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:52 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

From: Dmitry Torokhov <dtor@mail.ru>

Changes:

 - switch to use theaded IRQ
 - more __devinit/__devexit annotations
 - rely on input core to remove jitter from events
 - global pointer removed
 - NEC/MAXIM/Freescale handling factored out

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/staging/mrst-touchscreen/intel-mid-touch.c |  886 +++++++++-----------
 1 files changed, 409 insertions(+), 477 deletions(-)


diff --git a/drivers/staging/mrst-touchscreen/intel-mid-touch.c b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
index 6fed01e..7e77737 100644
--- a/drivers/staging/mrst-touchscreen/intel-mid-touch.c
+++ b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
@@ -19,14 +19,11 @@
  * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
  *
  * Questions/Comments/Bug fixes to Sreedhara (sreedhara.ds@intel.com)
- * 			    Ramesh Agarwal (ramesh.agarwal@intel.com)
+ *			    Ramesh Agarwal (ramesh.agarwal@intel.com)
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
  * TODO:
  *	review conversion of r/m/w sequences
- *	Replace interrupt mutex abuse
- *	Kill of mrstouchdevp pointer
- *
  */
 
 #include <linux/module.h>
@@ -38,158 +35,104 @@
 #include <linux/spi/spi.h>
 #include <linux/irq.h>
 #include <linux/delay.h>
-#include <linux/kthread.h>
 #include <asm/intel_scu_ipc.h>
 
-
 /* PMIC Interrupt registers */
-#define PMIC_REG_ID1   0x00 /*PMIC ID1 register */
+#define PMIC_REG_ID1		0x00 /* PMIC ID1 register */
 
 /* PMIC Interrupt registers */
-#define PMIC_REG_INT   0x04 /*PMIC interrupt register */
-#define PMIC_REG_MINT  0x05 /*PMIC interrupt mask register */
+#define PMIC_REG_INT		0x04 /* PMIC interrupt register */
+#define PMIC_REG_MINT		0x05 /* PMIC interrupt mask register */
 
 /* ADC Interrupt registers */
-#define PMIC_REG_ADCINT   0x5F /*ADC interrupt register */
-#define PMIC_REG_MADCINT  0x60 /*ADC interrupt mask register */
+#define PMIC_REG_ADCINT		0x5F /* ADC interrupt register */
+#define PMIC_REG_MADCINT	0x60 /* ADC interrupt mask register */
 
 /* ADC Control registers */
-#define PMIC_REG_ADCCNTL1    0x61 /*ADC control register */
+#define PMIC_REG_ADCCNTL1	0x61 /* ADC control register */
 
 /* ADC Channel Selection registers */
-#define PMICADDR0     0xA4
-#define END_OF_CHANNEL 0x1F
+#define PMICADDR0		0xA4
+#define END_OF_CHANNEL		0x1F
 
 /* ADC Result register */
-#define PMIC_REG_ADCSNS0H   0x64
+#define PMIC_REG_ADCSNS0H	0x64
 
 /* ADC channels for touch screen */
-#define MRST_TS_CHAN10   0xA /* Touch screen X+ connection */
-#define MRST_TS_CHAN11   0xB /* Touch screen X- connection */
-#define MRST_TS_CHAN12   0xC /* Touch screen Y+ connection */
-#define MRST_TS_CHAN13   0xD /* Touch screen Y- connection */
-
-/* Touch screen coordinate constants */
-#define TOUCH_PRESSURE   	50
-#define TOUCH_PRESSURE_FS	100
-
-#define XMOVE_LIMIT	5
-#define YMOVE_LIMIT	5
-#define XYMOVE_CNT	3
-
-#define MAX_10BIT	((1<<10)-1)
+#define MRST_TS_CHAN10		0xA /* Touch screen X+ connection */
+#define MRST_TS_CHAN11		0xB /* Touch screen X- connection */
+#define MRST_TS_CHAN12		0xC /* Touch screen Y+ connection */
+#define MRST_TS_CHAN13		0xD /* Touch screen Y- connection */
 
 /* Touch screen channel BIAS constants */
-#define XBIAS		0x20
-#define YBIAS		0x40
-#define ZBIAS		0x80
+#define MRST_XBIAS		0x20
+#define MRST_YBIAS		0x40
+#define MRST_ZBIAS		0x80
 
 /* Touch screen coordinates */
-#define MIN_X		10
-#define MAX_X		1024
-#define MIN_Y		10
-#define MAX_Y		1024
-#define MIN_P		0
-#define MAX_P		TOUCH_PRESSURE_FS
-
-#define WAIT_ADC_COMPLETION 10
+#define MRST_X_MIN		10
+#define MRST_X_MAX		1024
+#define MRST_X_FUZZ		5
+#define MRST_Y_MIN		10
+#define MRST_Y_MAX		1024
+#define MRST_Y_FUZZ		5
+#define MRST_PRESSURE_MIN	0
+#define MRST_PRESSURE_NOMINAL	50
+#define MRST_PRESSURE_MAX	100
+
+#define WAIT_ADC_COMPLETION	10 /* msec */
 
 /* PMIC ADC round robin delays */
-#define ADC_LOOP_DELAY0 0x0 /* Continuous loop */
-#define ADC_LOOP_DELAY1 0x1 /* 4.5  ms approximate */
+#define ADC_LOOP_DELAY0		0x0 /* Continuous loop */
+#define ADC_LOOP_DELAY1		0x1 /* 4.5  ms approximate */
 
 /* PMIC Vendor Identifiers */
-#define PMIC_VENDOR_FS  0 /* PMIC vendor FreeScale */
-#define PMIC_VENDOR_MAXIM 1 /* PMIC vendor MAXIM */
-#define PMIC_VENDOR_NEC 2 /* PMIC vendor NEC */
-#define MRSTOUCH_MAX_CHANNELS 32 /* Maximum ADC channels */
+#define PMIC_VENDOR_FS		0 /* PMIC vendor FreeScale */
+#define PMIC_VENDOR_MAXIM	1 /* PMIC vendor MAXIM */
+#define PMIC_VENDOR_NEC		2 /* PMIC vendor NEC */
+#define MRSTOUCH_MAX_CHANNELS	32 /* Maximum ADC channels */
 
 /* Touch screen device structure */
 struct mrstouch_dev {
-	struct spi_device *spi; /* SPI device associated with touch screen */
-	struct input_dev *input; /* input device for touchscreen*/
-	char 		phys[32]; /* Device name */
-	struct task_struct *pendet_thrd; /* PENDET interrupt handler */
-	struct mutex lock; /* Sync between interrupt and PENDET handler */
-	bool            busy; /* Busy flag */
-	u16             asr; /* Address selection register */
-	int             irq;    /* Touch screen IRQ # */
-	uint		vendor;  /* PMIC vendor */
-	uint		rev;  /* PMIC revision */
-	u16		x;  /* X coordinate */
-	u16		y;  /* Y coordinate */
-	bool		pendown;  /* PEN position */
-} ;
-
-
-/* Global Pointer to Touch screen device */
-static struct mrstouch_dev *mrstouchdevp;
+	struct spi_device *spi;
+	struct input_dev *input;
+	char phys[32];
+	u16 asr;		/* Address selection register */
+	int irq;
+	unsigned int vendor;	/* PMIC vendor */
+	unsigned int rev;	/* PMIC revision */
+
+	int (*read_prepare)(struct mrstouch_dev *tsdev);
+	int (*read)(struct mrstouch_dev *tsdev, u16 *x, u16 *y, u16 *z);
+	int (*read_finish)(struct mrstouch_dev *tsdev);
+};
 
-/* Utility to read PMIC ID */
-static int mrstouch_pmic_id(uint *vendor, uint *rev)
+
+/*************************** NEC and Maxim Interface ************************/
+
+static int mrstouch_nec_adc_read_prepare(struct mrstouch_dev *tsdev)
 {
+	u16 reg;
 	int err;
-	u8 r;
 
-	err = intel_scu_ipc_ioread8(PMIC_REG_ID1, &r);
+	err = intel_scu_ipc_ioread16(PMIC_REG_MADCINT, &reg);
 	if (err)
 		return err;
 
-	*vendor = r & 0x7;
-	*rev = (r >> 3) & 0x7;
+	reg &= 0xDFFF; /* Disable pendet */
 
-	return 0;
+	/* Set MADCINT and update ADCCNTL1 (next reg byte) */
+	return intel_scu_ipc_iowrite16(PMIC_REG_MADCINT, reg);
 }
 
 /*
- * Parse ADC channels to find end of the channel configured by other ADC user
- * NEC and MAXIM requires 4 channels and FreeScale needs 18 channels
+ * Enables PENDET interrupt.
  */
-static int mrstouch_chan_parse(struct mrstouch_dev *tsdev)
-{
-	int err, i, j, found;
-	u32 r32;
-
-	found = -1;
-
-	for (i = 0; i < MRSTOUCH_MAX_CHANNELS; i++) {
-		if (found >= 0)
-			break;
-
-		err = intel_scu_ipc_ioread32(PMICADDR0, &r32);
-		if (err)
-			return err;
-
-		for (j = 0; j < 32; j+= 8) {
-			if (((r32 >> j) & 0xFF) == END_OF_CHANNEL) {
-				found = i;
-				break;
-			}
-		}
-	}
-	if (found < 0)
-		return 0;
-
-	if (tsdev->vendor == PMIC_VENDOR_FS) {
-		if (found && found > (MRSTOUCH_MAX_CHANNELS - 18))
-			return -ENOSPC;
-	} else {
-		if (found && found > (MRSTOUCH_MAX_CHANNELS - 4))
-			return -ENOSPC;
-	}
-	return found;
-}
-
-/* Utility to enable/disable pendet.
- * pendet set to true enables PENDET interrupt
- * pendet set to false disables PENDET interrupt
- * Also clears RND mask bit
-*/
-static int pendet_enable(struct mrstouch_dev *tsdev, bool pendet)
+static int mrstouch_nec_adc_read_finish(struct mrstouch_dev *tsdev)
 {
 	u16 reg;
 	u8 r;
-	u8 pendet_enabled = 0;
+	u8 pendet_enabled;
 	int retry = 0;
 	int err;
 
@@ -197,15 +140,12 @@ static int pendet_enable(struct mrstouch_dev *tsdev, bool pendet)
 	if (err)
 		return err;
 
-	if (pendet) {
-		reg &= ~0x0005;
-		reg |= 0x2000; /* Enable pendet */
-	} else
-		reg &= 0xDFFF; /* Disable pendet */
+	reg &= ~0x0005;
+	reg |= 0x2000; /* Enable pendet */
 
 	/* Set MADCINT and update ADCCNTL1 (next reg byte) */
 	err = intel_scu_ipc_iowrite16(PMIC_REG_MADCINT, reg);
-	if (!pendet || err)
+	if (err)
 		return err;
 
 	/*
@@ -213,31 +153,36 @@ static int pendet_enable(struct mrstouch_dev *tsdev, bool pendet)
 	 * the PMIC register value is not updated. Retry few iterations
 	 * to enable pendet.
 	 */
+	do {
+		err = intel_scu_ipc_ioread8(PMIC_REG_ADCCNTL1, &r);
+		if (err)
+			return err;
 
-	err = intel_scu_ipc_ioread8(PMIC_REG_ADCCNTL1, &r);
-	pendet_enabled = (r >> 5) & 0x01;
+		pendet_enabled = (r >> 5) & 0x01;
 
-	retry = 0;
-	while (!err && !pendet_enabled) {
-		retry++;
-		msleep(10);
-		err = intel_scu_ipc_iowrite8(PMIC_REG_ADCCNTL1, reg >> 8);
-		if (err)
-			break;
-		err = intel_scu_ipc_ioread8(PMIC_REG_ADCCNTL1, &r);
-		if (err == 0)
-			pendet_enabled = (r >> 5) & 0x01;
-		if (retry >= 10) {
-			dev_err(&tsdev->spi->dev, "Touch screen disabled.\n");
-			return -EIO;
+		if (!pendet_enabled) {
+			if (++retry >= 10) {
+				dev_err(&tsdev->spi->dev,
+					"Touch screen disabled.\n");
+				return -EIO;
+			}
+
+			msleep(10);
+
+			err = intel_scu_ipc_iowrite8(PMIC_REG_ADCCNTL1,
+						     reg >> 8);
+			if (err)
+				return err;
 		}
-	}
+	} while (!pendet_enabled);
+
 	return 0;
 }
 
-/* To read PMIC ADC touch screen result
- * Reads ADC storage registers for higher 7 and lower 3 bits
- * converts the two readings to single value and turns off gain bit
+/*
+ * Reads PMIC ADC touch screen result
+ * Reads ADC storage registers for higher 7 and lower 3 bits and
+ * converts the two readings into a single value and turns off gain bit
  */
 static int mrstouch_ts_chan_read(u16 offset, u16 chan, u16 *vp, u16 *vm)
 {
@@ -269,187 +214,93 @@ static int mrstouch_ts_chan_read(u16 offset, u16 chan, u16 *vp, u16 *vm)
 	return 0;
 }
 
-/* To configure touch screen channels
- * Writes touch screen channels to ADC address selection registers
+/*
+ * Enables X, Y and Z bias values
+ * Enables YPYM for X channels and XPXM for Y channels
  */
-static int mrstouch_ts_chan_set(uint offset)
+static int mrstouch_ts_bias_set(uint offset, uint bias)
 {
 	int count;
-	u16 chan;
-	u16 reg[5];
-	u8 data[5];
+	u16 chan, start;
+	u16 reg[4];
+	u8 data[4];
 
 	chan = PMICADDR0 + offset;
+	start = MRST_TS_CHAN10;
+
 	for (count = 0; count <= 3; count++) {
 		reg[count] = chan++;
-		data[count] = MRST_TS_CHAN10 + count;
-	}
-	reg[count] = chan;
-	data[count] = END_OF_CHANNEL;
-
-	return intel_scu_ipc_writev(reg, data, 5);
-}
-
-/* Initialize ADC */
-static int mrstouch_adc_init(struct mrstouch_dev *tsdev)
-{
-	int err, start;
-	u8 ra, rm;
-
-	err = mrstouch_pmic_id(&tsdev->vendor, &tsdev->rev);
-	if (err) {
-		dev_err(&tsdev->spi->dev, "Unable to read PMIC id\n");
-		return err;
-	}
-
-	start = mrstouch_chan_parse(tsdev);
-	if (start < 0) {
-		dev_err(&tsdev->spi->dev, "Unable to parse channels\n");
-		return start;
-	}
-
-	tsdev->asr = start;
-
-	/* ADC power on, start, enable PENDET and set loop delay
-	 * ADC loop delay is set to 4.5 ms approximately
-	 * Loop delay more than this results in jitter in adc readings
-	 * Setting loop delay to 0 (continous loop) in MAXIM stops PENDET
-	 * interrupt generation sometimes.
-	 */
-
-	if (tsdev->vendor == PMIC_VENDOR_FS) {
-		ra = 0xE0 | ADC_LOOP_DELAY0;
-		rm = 0x5;
-	} else {
-		/* NEC and MAXIm not consistent with loop delay 0 */
-		ra = 0xE0 | ADC_LOOP_DELAY1;
-		rm = 0x0;
-
-		/* configure touch screen channels */
-		err = mrstouch_ts_chan_set(tsdev->asr);
-		if (err)
-			return err;
-	}
-	err = intel_scu_ipc_update_register(PMIC_REG_ADCCNTL1, ra, 0xE7);
-	if (err == 0)
-		err = intel_scu_ipc_update_register(PMIC_REG_MADCINT, rm, 0x03);
-	return err;
-}
-
-/* Reports x,y coordinates to event subsystem */
-static void mrstouch_report_xy(struct mrstouch_dev *tsdev, u16 x, u16 y, u16 z)
-{
-	int xdiff, ydiff;
-
-	if (tsdev->pendown && z <= TOUCH_PRESSURE) {
-		/* Pen removed, report button release */
-		input_report_key(tsdev->input, BTN_TOUCH, 0);
-		tsdev->pendown = false;
-	}
-
-	xdiff = abs(x - tsdev->x);
-	ydiff = abs(y - tsdev->y);
-
-	/*
-	if x and y values changes for XYMOVE_CNT readings it is considered
-	as stylus is moving. This is required to differentiate between stylus
-	movement and jitter
-	*/
-	if (x < MIN_X || x > MAX_X || y < MIN_Y || y > MAX_Y) {
-		/* Spurious values, release button if touched and return */
-		if (tsdev->pendown) {
-			input_report_key(tsdev->input, BTN_TOUCH, 0);
-			tsdev->pendown = false;
-		}
-		return;
-	} else if (xdiff >= XMOVE_LIMIT || ydiff >= YMOVE_LIMIT) {
-		tsdev->x = x;
-		tsdev->y = y;
-
-		input_report_abs(tsdev->input, ABS_X, x);
-		input_report_abs(tsdev->input, ABS_Y, y);
-		input_report_abs(tsdev->input, ABS_PRESSURE, z);
-		input_sync(tsdev->input);
-	}
-
-
-	if (!tsdev->pendown && z > TOUCH_PRESSURE) {
-		/* Pen touched, report button touch */
-		input_report_key(tsdev->input, BTN_TOUCH, 1);
-		tsdev->pendown = true;
+		data[count] = bias | (start + count);
 	}
-}
-
-
-/* Utility to start ADC, used by freescale handler */
-static int pendet_mask(void)
-{
-	return 	intel_scu_ipc_update_register(PMIC_REG_MADCINT, 0x02, 0x02);
-}
 
-/* Utility to stop ADC, used by freescale handler */
-static int pendet_umask(void)
-{
-	return 	intel_scu_ipc_update_register(PMIC_REG_MADCINT, 0x00, 0x02);
+	return intel_scu_ipc_writev(reg, data, 4);
 }
 
-/* Utility to read ADC, used by freescale handler */
-static int mrstouch_pmic_fs_adc_read(struct mrstouch_dev *tsdev)
+/* To read touch screen channel values */
+static int mrstouch_nec_adc_read(struct mrstouch_dev *tsdev,
+				 u16 *x, u16 *y, u16 *z)
 {
 	int err;
-	u16 x, y, z, result;
-	u16 reg[4];
-	u8 data[4];
+	u16 xm, ym, zm;
 
-	result = PMIC_REG_ADCSNS0H + tsdev->asr;
+	/* configure Y bias for X channels */
+	err = mrstouch_ts_bias_set(tsdev->asr, MRST_YBIAS); // XXX XBIAS?
+	if (err)
+		goto ipc_error;
 
-	reg[0] = result + 4;
-	reg[1] = result + 5;
-	reg[2] = result + 16;
-	reg[3] = result + 17;
+	msleep(WAIT_ADC_COMPLETION);
 
-	err = intel_scu_ipc_readv(reg, data, 4);
+	/* read x+ and x- channels */
+	err = mrstouch_ts_chan_read(tsdev->asr, MRST_TS_CHAN10, x, &xm);
 	if (err)
 		goto ipc_error;
 
-	x = data[0] << 3; /* Higher 7 bits */
-	x |= data[1] & 0x7; /* Lower 3 bits */
-	x &= 0x3FF;
+	/* configure x bias for y channels */
+	err = mrstouch_ts_bias_set(tsdev->asr, MRST_XBIAS); // XXX YBIAS?
+	if (err)
+		goto ipc_error;
 
-	y = data[2] << 3; /* Higher 7 bits */
-	y |= data[3] & 0x7; /* Lower 3 bits */
-	y &= 0x3FF;
+	msleep(WAIT_ADC_COMPLETION);
 
-	/* Read Z value */
-	reg[0] = result + 28;
-	reg[1] = result + 29;
+	/* read y+ and y- channels */
+	err = mrstouch_ts_chan_read(tsdev->asr, MRST_TS_CHAN12, y, &ym);
+	if (err)
+		goto ipc_error;
 
-	err = intel_scu_ipc_readv(reg, data, 4);
+	/* configure z bias for x and y channels */
+	err = mrstouch_ts_bias_set(tsdev->asr, MRST_ZBIAS);
 	if (err)
 		goto ipc_error;
 
-	z = data[0] << 3; /* Higher 7 bits */
-	z |= data[1] & 0x7; /* Lower 3 bits */
-	z &= 0x3FF;
+	msleep(WAIT_ADC_COMPLETION);
+
+	/* read z+ and z- channels */
+	err = mrstouch_ts_chan_read(tsdev->asr, MRST_TS_CHAN10, z, &zm);
+	if (err)
+		goto ipc_error;
 
-	mrstouch_report_xy(tsdev, x, y, z);
 	return 0;
 
 ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during fs_adc read\n");
+	dev_err(&tsdev->spi->dev, "ipc error during adc read\n");
 	return err;
 }
 
-/* To handle free scale pmic pendet interrupt */
-static int pmic0_pendet(void *dev_id)
+
+/*************************** Freescale Interface ************************/
+
+static int mrstouch_fs_adc_read_prepare(struct mrstouch_dev *tsdev)
 {
 	int err, count;
 	u16 chan;
-	unsigned int touched;
-	struct mrstouch_dev *tsdev = (struct mrstouch_dev *)dev_id;
 	u16 reg[5];
 	u8 data[5];
 
+	/* Stop the ADC */
+	err = intel_scu_ipc_update_register(PMIC_REG_MADCINT, 0x00, 0x02);
+	if (err)
+		goto ipc_error;
+
 	chan = PMICADDR0 + tsdev->asr;
 
 	/* Set X BIAS */
@@ -487,16 +338,65 @@ static int pmic0_pendet(void *dev_id)
 
 	msleep(WAIT_ADC_COMPLETION);
 
-	/*Read touch screen channels till pen removed
-	 * Freescale reports constant value of z for all points
-	 * z is high when screen is not touched and low when touched
-	 * Map high z value to not touched and low z value to pen touched
-	 */
-	touched = mrstouch_pmic_fs_adc_read(tsdev);
-	while (touched > TOUCH_PRESSURE) {
-		touched = mrstouch_pmic_fs_adc_read(tsdev);
-		msleep(WAIT_ADC_COMPLETION);
-	}
+	return 0;
+
+ipc_error:
+	dev_err(&tsdev->spi->dev, "ipc error during %s\n", __func__);
+	return err;
+}
+
+static int mrstouch_fs_adc_read(struct mrstouch_dev *tsdev,
+				u16 *x, u16 *y, u16 *z)
+{
+	int err;
+	u16 result;
+	u16 reg[4];
+	u8 data[4];
+
+	result = PMIC_REG_ADCSNS0H + tsdev->asr;
+
+	reg[0] = result + 4;
+	reg[1] = result + 5;
+	reg[2] = result + 16;
+	reg[3] = result + 17;
+
+	err = intel_scu_ipc_readv(reg, data, 4);
+	if (err)
+		goto ipc_error;
+
+	*x = data[0] << 3; /* Higher 7 bits */
+	*x |= data[1] & 0x7; /* Lower 3 bits */
+	*x &= 0x3FF;
+
+	*y = data[2] << 3; /* Higher 7 bits */
+	*y |= data[3] & 0x7; /* Lower 3 bits */
+	*y &= 0x3FF;
+
+	/* Read Z value */
+	reg[0] = result + 28;
+	reg[1] = result + 29;
+
+	err = intel_scu_ipc_readv(reg, data, 4);
+	if (err)
+		goto ipc_error;
+
+	*z = data[0] << 3; /* Higher 7 bits */
+	*z |= data[1] & 0x7; /* Lower 3 bits */
+	*z &= 0x3FF;
+
+	return 0;
+
+ipc_error:
+	dev_err(&tsdev->spi->dev, "ipc error during %s\n", __func__);
+	return err;
+}
+
+static int mrstouch_fs_adc_read_finish(struct mrstouch_dev *tsdev)
+{
+	int err, count;
+	u16 chan;
+	u16 reg[5];
+	u8 data[5];
 
 	/* Clear all TS channels */
 	chan = PMICADDR0 + tsdev->asr;
@@ -520,252 +420,294 @@ static int pmic0_pendet(void *dev_id)
 	if (err)
 		goto ipc_error;
 
+	/* Start ADC */
+	err = intel_scu_ipc_update_register(PMIC_REG_MADCINT, 0x02, 0x02);
+	if (err)
+		goto ipc_error;
+
 	return 0;
 
 ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during pendet\n");
+	dev_err(&tsdev->spi->dev, "ipc error during %s\n", __func__);
 	return err;
 }
 
-
-/* To enable X, Y and Z bias values
- * Enables YPYM for X channels and XPXM for Y channels
- */
-static int mrstouch_ts_bias_set(uint offset, uint bias)
+static void mrstouch_report_event(struct input_dev *input,
+			unsigned int x, unsigned int y, unsigned int z)
 {
-	int count;
-	u16 chan, start;
-	u16 reg[4];
-	u8 data[4];
-
-	chan = PMICADDR0 + offset;
-	start = MRST_TS_CHAN10;
-
-	for (count = 0; count <= 3; count++) {
-		reg[count] = chan++;
-		data[count] = bias | (start + count);
+	if (z > MRST_PRESSURE_NOMINAL) {
+		/* Pen touched, report button touch and coordinates */
+		input_report_key(input, BTN_TOUCH, 1);
+		input_report_abs(input, ABS_X, x);
+		input_report_abs(input, ABS_Y, y);
+	} else {
+		input_report_key(input, BTN_TOUCH, 0);
 	}
-	return intel_scu_ipc_writev(reg, data, 4);
+
+	input_report_abs(input, ABS_PRESSURE, z);
+	input_sync(input);
 }
 
-/* To read touch screen channel values */
-static int mrstouch_adc_read(struct mrstouch_dev *tsdev)
+/* PENDET interrupt handler */
+static irqreturn_t mrstouch_pendet_irq(int irq, void *dev_id)
 {
-	int err;
-	u16 xp, xm, yp, ym, zp, zm;
+	struct mrstouch_dev *tsdev = dev_id;
+	u16 x, y, z;
 
-	/* configure Y bias for X channels */
-	err = mrstouch_ts_bias_set(tsdev->asr, YBIAS);
-	if (err)
-		goto ipc_error;
-
-	msleep(WAIT_ADC_COMPLETION);
+	// FIXME: should we lower thread priority? Maybe not, we
+	// not spinnig but sleeping...
 
-	/* read x+ and x- channels */
-	err = mrstouch_ts_chan_read(tsdev->asr, MRST_TS_CHAN10, &xp, &xm);
-	if (err)
-		goto ipc_error;
+	if (tsdev->read_prepare(tsdev))
+		goto out;
 
-	/* configure x bias for y channels */
-	err = mrstouch_ts_bias_set(tsdev->asr, XBIAS);
-	if (err)
-		goto ipc_error;
+	do {
+		if (tsdev->read(tsdev, &x, &y, &z))
+			break;
 
-	msleep(WAIT_ADC_COMPLETION);
+		mrstouch_report_event(tsdev->input, x, y, z);
+	} while (z > MRST_PRESSURE_NOMINAL);
 
-	/* read y+ and y- channels */
-	err = mrstouch_ts_chan_read(tsdev->asr, MRST_TS_CHAN12, &yp, &ym);
-	if (err)
-		goto ipc_error;
+	tsdev->read_finish(tsdev);
 
-	/* configure z bias for x and y channels */
-	err = mrstouch_ts_bias_set(tsdev->asr, ZBIAS);
-	if (err)
-		goto ipc_error;
+out:
+	return IRQ_HANDLED;
+}
 
-	msleep(WAIT_ADC_COMPLETION);
+/* Utility to read PMIC ID */
+static int __devinit mrstouch_read_pmic_id(uint *vendor, uint *rev)
+{
+	int err;
+	u8 r;
 
-	/* read z+ and z- channels */
-	err = mrstouch_ts_chan_read(tsdev->asr, MRST_TS_CHAN10, &zp, &zm);
+	err = intel_scu_ipc_ioread8(PMIC_REG_ID1, &r);
 	if (err)
-		goto ipc_error;
-
-	mrstouch_report_xy(tsdev, xp, yp, zp); /* report x and y to eventX */
-
-	return zp;
-
-ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during adc read\n");
-	return err;
-}
+		return err;
 
-/* PENDET interrupt handler function for NEC and MAXIM */
-static void pmic12_pendet(void *data)
-{
-	unsigned int touched;
-	struct mrstouch_dev *tsdev = (struct mrstouch_dev *)data;
+	*vendor = r & 0x7;
+	*rev = (r >> 3) & 0x7;
 
-	/* read touch screen channels till pen removed */
-	do {
-		touched = mrstouch_adc_read(tsdev);
-	} while (touched > TOUCH_PRESSURE);
+	return 0;
 }
 
-/* Handler to process PENDET interrupt */
-int mrstouch_pendet(void *data)
+/*
+ * Parse ADC channels to find end of the channel configured by other ADC user
+ * NEC and MAXIM requires 4 channels and FreeScale needs 18 channels
+ */
+static int __devinit mrstouch_chan_parse(struct mrstouch_dev *tsdev)
 {
-	struct mrstouch_dev *tsdev = (struct mrstouch_dev *)data;
-	while (1) {
-		/* Wait for PENDET interrupt */
-		if (mutex_lock_interruptible(&tsdev->lock)) {
-			msleep(WAIT_ADC_COMPLETION);
-			continue;
-		}
+	int err, i, j, found;
+	u32 r32;
 
-		if (tsdev->busy)
-			return 0;
+	found = -1;
 
-		tsdev->busy = true;
+	for (i = 0; i < MRSTOUCH_MAX_CHANNELS; i++) {
+		if (found >= 0)
+			break;
 
-		if (tsdev->vendor == PMIC_VENDOR_NEC ||
-			tsdev->vendor == PMIC_VENDOR_MAXIM) {
-			/* PENDET must be disabled in NEC before reading ADC */
-			pendet_enable(tsdev,false); /* Disbale PENDET */
-			pmic12_pendet(tsdev);
-			pendet_enable(tsdev, true); /*Enable PENDET */
-		} else if (tsdev->vendor == PMIC_VENDOR_FS) {
-			pendet_umask(); /* Stop ADC */
-			pmic0_pendet(tsdev);
-			pendet_mask(); /* Stop ADC */
-		} else
-		dev_err(&tsdev->spi->dev, "Unsupported touchscreen: %d\n",
-				tsdev->vendor);
+		err = intel_scu_ipc_ioread32(PMICADDR0, &r32);
+		if (err)
+			return err;
 
-		tsdev->busy = false;
+		for (j = 0; j < 32; j+= 8) {
+			if (((r32 >> j) & 0xFF) == END_OF_CHANNEL) {
+				found = i;
+				break;
+			}
+		}
+	}
+	if (found < 0)
+		return 0;
 
+	if (tsdev->vendor == PMIC_VENDOR_FS) {
+		if (found && found > (MRSTOUCH_MAX_CHANNELS - 18))
+			return -ENOSPC;
+	} else {
+		if (found && found > (MRSTOUCH_MAX_CHANNELS - 4))
+			return -ENOSPC;
 	}
-	return 0;
+	return found;
 }
 
-/* PENDET interrupt handler */
-static irqreturn_t pendet_intr_handler(int irq, void *handle)
+
+/*
+ * Writes touch screen channels to ADC address selection registers
+ */
+static int __devinit mrstouch_ts_chan_set(uint offset)
 {
-	struct mrstouch_dev *tsdev = (struct mrstouch_dev *)handle;
+	int count;
+	u16 chan;
+	u16 reg[5];
+	u8 data[5];
 
-	mutex_unlock(&tsdev->lock);
-	return IRQ_HANDLED;
+	chan = PMICADDR0 + offset;
+	for (count = 0; count <= 3; count++) {
+		reg[count] = chan++;
+		data[count] = MRST_TS_CHAN10 + count;
+	}
+	reg[count] = chan;
+	data[count] = END_OF_CHANNEL;
+
+	return intel_scu_ipc_writev(reg, data, 5);
 }
 
-/* Intializes input device and registers with input subsystem */
-static int ts_input_dev_init(struct mrstouch_dev *tsdev, struct spi_device *spi)
+/* Initialize ADC */
+static int __devinit mrstouch_adc_init(struct mrstouch_dev *tsdev)
 {
-	int err = 0;
+	int err, start;
+	u8 ra, rm;
 
-	tsdev->input = input_allocate_device();
-	if (!tsdev->input) {
-		dev_err(&tsdev->spi->dev, "Unable to allocate input device.\n");
-		return -ENOMEM;
+	err = mrstouch_read_pmic_id(&tsdev->vendor, &tsdev->rev);
+	if (err) {
+		dev_err(&tsdev->spi->dev, "Unable to read PMIC id\n");
+		return err;
 	}
 
-	tsdev->input->name = "mrst_touchscreen";
-	snprintf(tsdev->phys, sizeof(tsdev->phys),
-			"%s/input0", dev_name(&spi->dev));
-	tsdev->input->phys = tsdev->phys;
-	tsdev->input->dev.parent = &spi->dev;
+	switch (tsdev->vendor) {
+	case PMIC_VENDOR_NEC:
+	case PMIC_VENDOR_MAXIM:
+		tsdev->read_prepare = mrstouch_nec_adc_read_prepare;
+		tsdev->read = mrstouch_nec_adc_read;
+		tsdev->read_finish = mrstouch_nec_adc_read_finish;
+		break;
+
+	case PMIC_VENDOR_FS:
+		tsdev->read_prepare = mrstouch_fs_adc_read_prepare;
+		tsdev->read = mrstouch_fs_adc_read;
+		tsdev->read_finish = mrstouch_fs_adc_read_finish;
+		break;
+
+	default:
+		dev_err(&tsdev->spi->dev,
+			"Unsupported touchscreen: %d\n", tsdev->vendor);
+		return -ENXIO;
+	}
 
-	tsdev->input->id.vendor = tsdev->vendor;
-	tsdev->input->id.version = tsdev->rev;
+	start = mrstouch_chan_parse(tsdev);
+	if (start < 0) {
+		dev_err(&tsdev->spi->dev, "Unable to parse channels\n");
+		return start;
+	}
 
-	tsdev->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
-	tsdev->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+	tsdev->asr = start;
 
-	input_set_abs_params(tsdev->input, ABS_X, MIN_X, MAX_X, 0, 0);
-	input_set_abs_params(tsdev->input, ABS_Y, MIN_Y, MAX_Y, 0, 0);
-	input_set_abs_params(tsdev->input, ABS_PRESSURE, MIN_P, MAX_P, 0, 0);
+	/*
+	 * ADC power on, start, enable PENDET and set loop delay
+	 * ADC loop delay is set to 4.5 ms approximately
+	 * Loop delay more than this results in jitter in adc readings
+	 * Setting loop delay to 0 (continous loop) in MAXIM stops PENDET
+	 * interrupt generation sometimes.
+	 */
 
-	err = input_register_device(tsdev->input);
-	if (err) {
-		dev_err(&tsdev->spi->dev, "unable to register input device\n");
-		input_free_device(tsdev->input);
-		return err;
+	if (tsdev->vendor == PMIC_VENDOR_FS) {
+		ra = 0xE0 | ADC_LOOP_DELAY0;
+		rm = 0x5;
+	} else {
+		/* NEC and MAXIm not consistent with loop delay 0 */
+		ra = 0xE0 | ADC_LOOP_DELAY1;
+		rm = 0x0;
+
+		/* configure touch screen channels */
+		err = mrstouch_ts_chan_set(tsdev->asr);
+		if (err)
+			return err;
 	}
-	return 0;
 
+	err = intel_scu_ipc_update_register(PMIC_REG_ADCCNTL1, ra, 0xE7);
+	if (err)
+		return err;
+
+	err = intel_scu_ipc_update_register(PMIC_REG_MADCINT, rm, 0x03);
+	if (err)
+		return err;
+
+	return 0;
 }
 
+
 /* Probe function for touch screen driver */
-static int __devinit mrstouch_probe(struct spi_device *mrstouch_spi)
+static int __devinit mrstouch_probe(struct spi_device *spi)
 {
-	int err;
-	unsigned int myirq;
 	struct mrstouch_dev *tsdev;
+	struct input_dev *input;
+	int err;
 
-	mrstouchdevp = NULL;
-	myirq = mrstouch_spi->irq;
-
-	if (!mrstouch_spi->irq) {
-		dev_err(&mrstouch_spi->dev, "no interrupt assigned\n");
+	if (!spi->irq) {
+		dev_err(&spi->dev, "no interrupt assigned\n");
 		return -EINVAL;
 	}
 
 	tsdev = kzalloc(sizeof(struct mrstouch_dev), GFP_KERNEL);
-	if (!tsdev) {
-		dev_err(&mrstouch_spi->dev, "unable to allocate memory\n");
-		return -ENOMEM;
+	input = input_allocate_device();
+	if (!tsdev || !input) {
+		dev_err(&spi->dev, "unable to allocate memory\n");
+		err = -ENOMEM;
+		goto err_free_mem;
 	}
 
-	tsdev->irq = myirq;
-	mrstouchdevp = tsdev;
+	tsdev->spi = spi;
+	tsdev->input = input;
+	tsdev->irq = spi->irq;
+
+	snprintf(tsdev->phys, sizeof(tsdev->phys),
+		 "%s/input0", dev_name(&spi->dev));
 
 	err = mrstouch_adc_init(tsdev);
 	if (err) {
-		dev_err(&mrstouch_spi->dev, "ADC init failed\n");
-		goto mrstouch_err_free_mem;
+		dev_err(&spi->dev, "ADC initialization failed\n");
+		goto err_free_mem;
 	}
 
-	dev_set_drvdata(&mrstouch_spi->dev, tsdev);
-	tsdev->spi = mrstouch_spi;
+	input->name = "mrst_touchscreen";
+	input->phys = tsdev->phys;
+	input->dev.parent = &spi->dev;
 
-	err = ts_input_dev_init(tsdev, mrstouch_spi);
-	if (err) {
-		dev_err(&tsdev->spi->dev, "ts_input_dev_init failed");
-		goto mrstouch_err_free_dev;
-	}
+	input->id.vendor = tsdev->vendor;
+	input->id.version = tsdev->rev;
 
-	mutex_init(&tsdev->lock);
-	mutex_lock(&tsdev->lock);
+	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	err = request_irq(myirq, pendet_intr_handler,
-				0, "mrstouch", tsdev);
+	input_set_abs_params(tsdev->input, ABS_X,
+			     MRST_X_MIN, MRST_X_MAX, MRST_X_FUZZ, 0);
+	input_set_abs_params(tsdev->input, ABS_Y,
+			     MRST_Y_MIN, MRST_Y_MAX, MRST_Y_FUZZ, 0);
+	input_set_abs_params(tsdev->input, ABS_PRESSURE,
+			     MRST_PRESSURE_MIN, MRST_PRESSURE_MAX, 0, 0);
+
+	err = request_threaded_irq(tsdev->irq, NULL, mrstouch_pendet_irq,
+				   0, "mrstouch", tsdev);
 	if (err) {
 		dev_err(&tsdev->spi->dev, "unable to allocate irq\n");
-		goto mrstouch_err_free_dev;
+		goto err_free_mem;
 	}
 
-	tsdev->pendet_thrd = kthread_run(mrstouch_pendet,
-				(void *)tsdev, "pendet handler");
-	if (IS_ERR(tsdev->pendet_thrd)) {
-		dev_err(&tsdev->spi->dev, "kthread_run failed\n");
-		err = PTR_ERR(tsdev->pendet_thrd);
-		goto mrstouch_err_free_irq;
+	err = input_register_device(tsdev->input);
+	if (err) {
+		dev_err(&tsdev->spi->dev, "unable to register input device\n");
+		goto err_free_irq;
 	}
+
+	spi_set_drvdata(spi, tsdev);
 	return 0;
-mrstouch_err_free_irq:
-	free_irq(myirq, tsdev);
-mrstouch_err_free_dev:
-	input_unregister_device(tsdev->input);
-mrstouch_err_free_mem:
+
+err_free_irq:
+	free_irq(tsdev->irq, tsdev);
+err_free_mem:
+	input_free_device(input);
 	kfree(tsdev);
 	return err;
 }
 
-static int mrstouch_remove(struct spi_device *spi)
+static int __devexit mrstouch_remove(struct spi_device *spi)
 {
-	free_irq(mrstouchdevp->irq, mrstouchdevp);
-	if (mrstouchdevp->pendet_thrd)
-		kthread_stop(mrstouchdevp->pendet_thrd);
-	input_unregister_device(mrstouchdevp->input);
-	kfree(mrstouchdevp);
+	struct mrstouch_dev *tsdev = spi_get_drvdata(spi);
+
+	free_irq(tsdev->irq, tsdev);
+	input_unregister_device(tsdev->input);
+	kfree(tsdev);
+
+	spi_set_drvdata(spi, NULL);
+
 	return 0;
 }
 
@@ -776,30 +718,20 @@ static struct spi_driver mrstouch_driver = {
 		.owner  = THIS_MODULE,
 	},
 	.probe          = mrstouch_probe,
-	.remove         = mrstouch_remove,
+	.remove         = __devexit_p(mrstouch_remove),
 };
 
-static int __init mrstouch_module_init(void)
+static int __init mrstouch_init(void)
 {
-	int err;
-
-	err = spi_register_driver(&mrstouch_driver);
-	if (err) {
-		printk(KERN_ERR "%s(%d)", "SPI PENDET failed", err);
-		return err;;
-	}
-
-	return 0;
+	return spi_register_driver(&mrstouch_driver);
 }
+module_init(mrstouch_init);
 
-static void __exit mrstouch_module_exit(void)
+static void __exit mrstouch_exit(void)
 {
 	spi_unregister_driver(&mrstouch_driver);
-	return;
 }
-
-module_init(mrstouch_module_init);
-module_exit(mrstouch_module_exit);
+module_exit(mrstouch_exit);
 
 MODULE_AUTHOR("Sreedhara Murthy. D.S, sreedhara.ds@intel.com");
 MODULE_DESCRIPTION("Intel Moorestown Resistive Touch Screen Driver");


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

* [PATCH 3/6] mrst-touchscreen: Fix use before initialize in mrst_touch [Fix bug 2561]
  2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
  2010-07-23 13:51 ` [PATCH 1/6] mrst_touchscreen: clean up input side Alan Cox
  2010-07-23 13:52 ` [PATCH 2/6] Input: mrst - more fixes Alan Cox
@ 2010-07-23 13:52 ` Alan Cox
  2010-07-27 18:17   ` Greg KH
  2010-07-23 13:52 ` [PATCH 4/6] * Register platform interface Alan Cox
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:52 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

From: Arjan van de Ven <arjan@linux.intel.com>

The mrst touch screen driver, in mrstouch_adc_init does

                 dev_err(&tsdev->spi->dev, "Unable to read PMIC id\n");

which does not work very well since the tsdev->spi member does not get
initialized until after the call to mrstouch_adc_init

this patch makes sure the ->spi member is initialized prior to the call
to mrstouch_adc_init

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>


--- linux-2.6.34/drivers/staging/mrst-touchscreen/intel-mid-touch.c~
  2010-06-23 03:13:39.000000000 +0000
+++ linux-2.6.34/drivers/staging/mrst-touchscreen/intel-mid-touch.c
  2010-06-23 03:14:47.736742734 +0000
@@ -759,6 +759,8 @@
      tsdev->irq = myirq;
      mrstouchdevp = tsdev;

+    tsdev->spi = mrstouch_spi;
+
      err = mrstouch_adc_init(tsdev);
      if (err) {
          dev_err(&mrstouch_spi->dev, "ADC init failed\n");
@@ -766,7 +768,6 @@
      }

      dev_set_drvdata(&mrstouch_spi->dev, tsdev);
-    tsdev->spi = mrstouch_spi;

      err = ts_input_dev_init(tsdev, mrstouch_spi);
      if (err) {


--------------030606040305060606070104
Content-Type: text/plain;
 name="Q"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="Q"

--- linux-2.6.34/drivers/staging/mrst-touchscreen/intel-mid-touch.c~	2010-06-23 03:13:39.000000000 +0000
+++ linux-2.6.34/drivers/staging/mrst-touchscreen/intel-mid-touch.c	2010-06-23 03:14:47.736742734 +0000
@@ -759,6 +759,8 @@
 	tsdev->irq = myirq;
 	mrstouchdevp = tsdev;

+	tsdev->spi = mrstouch_spi;
+
 	err = mrstouch_adc_init(tsdev);
 	if (err) {
 		dev_err(&mrstouch_spi->dev, "ADC init failed\n");
@@ -766,7 +768,6 @@
 	}

 	dev_set_drvdata(&mrstouch_spi->dev, tsdev);
-	tsdev->spi = mrstouch_spi;

 	err = ts_input_dev_init(tsdev, mrstouch_spi);
 	if (err) {

--------------030606040305060606070104--
---

 drivers/staging/mrst-touchscreen/intel-mid-touch.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)


diff --git a/drivers/staging/mrst-touchscreen/intel-mid-touch.c b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
index 7e77737..c41989b 100644
--- a/drivers/staging/mrst-touchscreen/intel-mid-touch.c
+++ b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
@@ -651,6 +651,8 @@ static int __devinit mrstouch_probe(struct spi_device *spi)
 	snprintf(tsdev->phys, sizeof(tsdev->phys),
 		 "%s/input0", dev_name(&spi->dev));
 
+	tsdev->spi = mrstouch_spi;
+
 	err = mrstouch_adc_init(tsdev);
 	if (err) {
 		dev_err(&spi->dev, "ADC initialization failed\n");


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

* [PATCH 4/6] * Register platform interface
  2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
                   ` (2 preceding siblings ...)
  2010-07-23 13:52 ` [PATCH 3/6] mrst-touchscreen: Fix use before initialize in mrst_touch [Fix bug 2561] Alan Cox
@ 2010-07-23 13:52 ` Alan Cox
  2010-07-23 16:07   ` Dmitry Torokhov
  2010-07-23 13:52 ` [PATCH 5/6] fix channel allocation in the touch screen driver Alan Cox
  2010-07-23 13:52 ` [PATCH 6/6] Simplify en/disable of interrupts for NEC Alan Cox
  5 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:52 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

From: Alek Du <alek.du@intel.com>

*	Fix some coding style

AC: Reworked to merge with upstream input device work from Dmitry et al.

Signed-off-by: Alek Du <alek.du@intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/staging/mrst-touchscreen/intel-mid-touch.c |   63 ++++++++++----------
 1 files changed, 30 insertions(+), 33 deletions(-)


diff --git a/drivers/staging/mrst-touchscreen/intel-mid-touch.c b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
index c41989b..db286eb 100644
--- a/drivers/staging/mrst-touchscreen/intel-mid-touch.c
+++ b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
@@ -32,7 +32,8 @@
 #include <linux/interrupt.h>
 #include <linux/err.h>
 #include <linux/param.h>
-#include <linux/spi/spi.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
 #include <linux/irq.h>
 #include <linux/delay.h>
 #include <asm/intel_scu_ipc.h>
@@ -94,7 +95,7 @@
 
 /* Touch screen device structure */
 struct mrstouch_dev {
-	struct spi_device *spi;
+	struct device *dev; /* device associated with touch screen */
 	struct input_dev *input;
 	char phys[32];
 	u16 asr;		/* Address selection register */
@@ -162,7 +163,7 @@ static int mrstouch_nec_adc_read_finish(struct mrstouch_dev *tsdev)
 
 		if (!pendet_enabled) {
 			if (++retry >= 10) {
-				dev_err(&tsdev->spi->dev,
+				dev_err(tsdev->dev,
 					"Touch screen disabled.\n");
 				return -EIO;
 			}
@@ -282,7 +283,7 @@ static int mrstouch_nec_adc_read(struct mrstouch_dev *tsdev,
 	return 0;
 
 ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during adc read\n");
+	dev_err(tsdev->dev, "ipc error during adc read\n");
 	return err;
 }
 
@@ -341,7 +342,7 @@ static int mrstouch_fs_adc_read_prepare(struct mrstouch_dev *tsdev)
 	return 0;
 
 ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during %s\n", __func__);
+	dev_err(tsdev->dev, "ipc error during %s\n", __func__);
 	return err;
 }
 
@@ -387,7 +388,7 @@ static int mrstouch_fs_adc_read(struct mrstouch_dev *tsdev,
 	return 0;
 
 ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during %s\n", __func__);
+	dev_err(tsdev->dev, "ipc error during %s\n", __func__);
 	return err;
 }
 
@@ -428,7 +429,7 @@ static int mrstouch_fs_adc_read_finish(struct mrstouch_dev *tsdev)
 	return 0;
 
 ipc_error:
-	dev_err(&tsdev->spi->dev, "ipc error during %s\n", __func__);
+	dev_err(tsdev->dev, "ipc error during %s\n", __func__);
 	return err;
 }
 
@@ -558,7 +559,7 @@ static int __devinit mrstouch_adc_init(struct mrstouch_dev *tsdev)
 
 	err = mrstouch_read_pmic_id(&tsdev->vendor, &tsdev->rev);
 	if (err) {
-		dev_err(&tsdev->spi->dev, "Unable to read PMIC id\n");
+		dev_err(tsdev->dev, "Unable to read PMIC id\n");
 		return err;
 	}
 
@@ -577,14 +578,14 @@ static int __devinit mrstouch_adc_init(struct mrstouch_dev *tsdev)
 		break;
 
 	default:
-		dev_err(&tsdev->spi->dev,
+		dev_err(tsdev->dev,
 			"Unsupported touchscreen: %d\n", tsdev->vendor);
 		return -ENXIO;
 	}
 
 	start = mrstouch_chan_parse(tsdev);
 	if (start < 0) {
-		dev_err(&tsdev->spi->dev, "Unable to parse channels\n");
+		dev_err(tsdev->dev, "Unable to parse channels\n");
 		return start;
 	}
 
@@ -625,43 +626,43 @@ static int __devinit mrstouch_adc_init(struct mrstouch_dev *tsdev)
 
 
 /* Probe function for touch screen driver */
-static int __devinit mrstouch_probe(struct spi_device *spi)
+static int __devinit mrstouch_probe(struct platform_device *pdev)
 {
 	struct mrstouch_dev *tsdev;
 	struct input_dev *input;
 	int err;
+	int irq;
 
-	if (!spi->irq) {
-		dev_err(&spi->dev, "no interrupt assigned\n");
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no interrupt assigned\n");
 		return -EINVAL;
 	}
 
 	tsdev = kzalloc(sizeof(struct mrstouch_dev), GFP_KERNEL);
 	input = input_allocate_device();
 	if (!tsdev || !input) {
-		dev_err(&spi->dev, "unable to allocate memory\n");
+		dev_err(&pdev->dev, "unable to allocate memory\n");
 		err = -ENOMEM;
 		goto err_free_mem;
 	}
 
-	tsdev->spi = spi;
+	tsdev->dev = &pdev->dev;
 	tsdev->input = input;
-	tsdev->irq = spi->irq;
+	tsdev->irq = irq;
 
 	snprintf(tsdev->phys, sizeof(tsdev->phys),
-		 "%s/input0", dev_name(&spi->dev));
-
-	tsdev->spi = mrstouch_spi;
+		 "%s/input0", dev_name(tsdev->dev));
 
 	err = mrstouch_adc_init(tsdev);
 	if (err) {
-		dev_err(&spi->dev, "ADC initialization failed\n");
+		dev_err(&pdev->dev, "ADC initialization failed\n");
 		goto err_free_mem;
 	}
 
 	input->name = "mrst_touchscreen";
 	input->phys = tsdev->phys;
-	input->dev.parent = &spi->dev;
+	input->dev.parent = tsdev->dev;
 
 	input->id.vendor = tsdev->vendor;
 	input->id.version = tsdev->rev;
@@ -679,17 +680,17 @@ static int __devinit mrstouch_probe(struct spi_device *spi)
 	err = request_threaded_irq(tsdev->irq, NULL, mrstouch_pendet_irq,
 				   0, "mrstouch", tsdev);
 	if (err) {
-		dev_err(&tsdev->spi->dev, "unable to allocate irq\n");
+		dev_err(tsdev->dev, "unable to allocate irq\n");
 		goto err_free_mem;
 	}
 
 	err = input_register_device(tsdev->input);
 	if (err) {
-		dev_err(&tsdev->spi->dev, "unable to register input device\n");
+		dev_err(tsdev->dev, "unable to register input device\n");
 		goto err_free_irq;
 	}
 
-	spi_set_drvdata(spi, tsdev);
+	dev_set_drvdata(tsdev->dev, tsdev);
 	return 0;
 
 err_free_irq:
@@ -700,23 +701,19 @@ err_free_mem:
 	return err;
 }
 
-static int __devexit mrstouch_remove(struct spi_device *spi)
+static int __devexit mrstouch_remove(struct platform_device *pdev)
 {
-	struct mrstouch_dev *tsdev = spi_get_drvdata(spi);
+	struct mrstouch_dev *tsdev = dev_get_drvdata(&pdev->dev);
 
 	free_irq(tsdev->irq, tsdev);
 	input_unregister_device(tsdev->input);
 	kfree(tsdev);
-
-	spi_set_drvdata(spi, NULL);
-
 	return 0;
 }
 
-static struct spi_driver mrstouch_driver = {
+static struct platform_driver mrstouch_driver = {
 	.driver = {
 		.name   = "pmic_touch",
-		.bus    = &spi_bus_type,
 		.owner  = THIS_MODULE,
 	},
 	.probe          = mrstouch_probe,
@@ -725,13 +722,13 @@ static struct spi_driver mrstouch_driver = {
 
 static int __init mrstouch_init(void)
 {
-	return spi_register_driver(&mrstouch_driver);
+	return platform_driver_register(&mrstouch_driver);
 }
 module_init(mrstouch_init);
 
 static void __exit mrstouch_exit(void)
 {
-	spi_unregister_driver(&mrstouch_driver);
+	platform_driver_unregister(&mrstouch_driver);
 }
 module_exit(mrstouch_exit);
 


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

* [PATCH 5/6] fix channel allocation in the touch screen driver
  2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
                   ` (3 preceding siblings ...)
  2010-07-23 13:52 ` [PATCH 4/6] * Register platform interface Alan Cox
@ 2010-07-23 13:52 ` Alan Cox
  2010-07-23 13:52 ` [PATCH 6/6] Simplify en/disable of interrupts for NEC Alan Cox
  5 siblings, 0 replies; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:52 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

From: Arjan van de Ven <arjan@linux.intel.com>

the touch screen driver tries to find a range of free channels (which
are an array of bytes), by scanning for the "end of used channel" marker.
however it tries to be WAAAAY too smart and does 32 bit logic on 8 bit
quantities, and in the process completely gets it wrong
(repeatedly read the same register instead of incrementing in the loop,
assuming that if any of the 4 bytes in the 32 byte quantity is free,
all four are free, returning the channel number divided by 4 rather than
the actual first free channel number)

On the setting side, the same mistakes are made by and large; changed
this to just use the byte SCU write functions....

with these fixes we go from a completely non detected touchscreen to
something that appears to completely get detected.
(after also fixing the ordering issue that Jacobs patch should solve)

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/staging/mrst-touchscreen/intel-mid-touch.c |   29 ++++++++------------
 1 files changed, 12 insertions(+), 17 deletions(-)


diff --git a/drivers/staging/mrst-touchscreen/intel-mid-touch.c b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
index db286eb..760bc7b 100644
--- a/drivers/staging/mrst-touchscreen/intel-mid-touch.c
+++ b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
@@ -496,8 +496,8 @@ static int __devinit mrstouch_read_pmic_id(uint *vendor, uint *rev)
  */
 static int __devinit mrstouch_chan_parse(struct mrstouch_dev *tsdev)
 {
-	int err, i, j, found;
-	u32 r32;
+	int err, i, found;
+	u8 r8;
 
 	found = -1;
 
@@ -505,15 +505,13 @@ static int __devinit mrstouch_chan_parse(struct mrstouch_dev *tsdev)
 		if (found >= 0)
 			break;
 
-		err = intel_scu_ipc_ioread32(PMICADDR0, &r32);
+		err = intel_scu_ipc_ioread8(PMICADDR0 + i, &r8);
 		if (err)
 			return err;
 
-		for (j = 0; j < 32; j+= 8) {
-			if (((r32 >> j) & 0xFF) == END_OF_CHANNEL) {
-				found = i;
-				break;
-			}
+		if (r8 == END_OF_CHANNEL) {
+			found = i;
+			break;
 		}
 	}
 	if (found < 0)
@@ -535,20 +533,17 @@ static int __devinit mrstouch_chan_parse(struct mrstouch_dev *tsdev)
  */
 static int __devinit mrstouch_ts_chan_set(uint offset)
 {
-	int count;
 	u16 chan;
-	u16 reg[5];
-	u8 data[5];
+
+	int ret, count;
 
 	chan = PMICADDR0 + offset;
 	for (count = 0; count <= 3; count++) {
-		reg[count] = chan++;
-		data[count] = MRST_TS_CHAN10 + count;
+		ret = intel_scu_ipc_iowrite8(chan++, MRST_TS_CHAN10 + count);
+		if (ret)
+			return ret;
 	}
-	reg[count] = chan;
-	data[count] = END_OF_CHANNEL;
-
-	return intel_scu_ipc_writev(reg, data, 5);
+	return intel_scu_ipc_iowrite8(chan++, END_OF_CHANNEL);
 }
 
 /* Initialize ADC */


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

* [PATCH 6/6] Simplify en/disable of interrupts for NEC.
  2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
                   ` (4 preceding siblings ...)
  2010-07-23 13:52 ` [PATCH 5/6] fix channel allocation in the touch screen driver Alan Cox
@ 2010-07-23 13:52 ` Alan Cox
  5 siblings, 0 replies; 16+ messages in thread
From: Alan Cox @ 2010-07-23 13:52 UTC (permalink / raw)
  To: greg, linux-input, dmitry.torokhov

From: Andy Ross <andy.ross@windriver.com>

Use 8 bit update commands instead of a 16 bit unaligned read/write
pair which fails after the first few calls; the voodoo in the original
doesn't seem to be required with this mechanism.

Tested-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Andy Ross <andy.ross@windriver.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/staging/mrst-touchscreen/intel-mid-touch.c |   65 ++------------------
 1 files changed, 7 insertions(+), 58 deletions(-)


diff --git a/drivers/staging/mrst-touchscreen/intel-mid-touch.c b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
index 760bc7b..4d27d6c 100644
--- a/drivers/staging/mrst-touchscreen/intel-mid-touch.c
+++ b/drivers/staging/mrst-touchscreen/intel-mid-touch.c
@@ -113,17 +113,7 @@ struct mrstouch_dev {
 
 static int mrstouch_nec_adc_read_prepare(struct mrstouch_dev *tsdev)
 {
-	u16 reg;
-	int err;
-
-	err = intel_scu_ipc_ioread16(PMIC_REG_MADCINT, &reg);
-	if (err)
-		return err;
-
-	reg &= 0xDFFF; /* Disable pendet */
-
-	/* Set MADCINT and update ADCCNTL1 (next reg byte) */
-	return intel_scu_ipc_iowrite16(PMIC_REG_MADCINT, reg);
+	return intel_scu_ipc_update_register(PMIC_REG_MADCINT, 0, 0x20);
 }
 
 /*
@@ -131,53 +121,12 @@ static int mrstouch_nec_adc_read_prepare(struct mrstouch_dev *tsdev)
  */
 static int mrstouch_nec_adc_read_finish(struct mrstouch_dev *tsdev)
 {
-	u16 reg;
-	u8 r;
-	u8 pendet_enabled;
-	int retry = 0;
-	int err;
-
-	err = intel_scu_ipc_ioread16(PMIC_REG_MADCINT, &reg);
-	if (err)
-		return err;
-
-	reg &= ~0x0005;
-	reg |= 0x2000; /* Enable pendet */
-
-	/* Set MADCINT and update ADCCNTL1 (next reg byte) */
-	err = intel_scu_ipc_iowrite16(PMIC_REG_MADCINT, reg);
-	if (err)
-		return err;
-
-	/*
-	 * Sometimes even after the register write succeeds
-	 * the PMIC register value is not updated. Retry few iterations
-	 * to enable pendet.
-	 */
-	do {
-		err = intel_scu_ipc_ioread8(PMIC_REG_ADCCNTL1, &r);
-		if (err)
-			return err;
-
-		pendet_enabled = (r >> 5) & 0x01;
-
-		if (!pendet_enabled) {
-			if (++retry >= 10) {
-				dev_err(tsdev->dev,
-					"Touch screen disabled.\n");
-				return -EIO;
-			}
-
-			msleep(10);
-
-			err = intel_scu_ipc_iowrite8(PMIC_REG_ADCCNTL1,
-						     reg >> 8);
-			if (err)
-				return err;
-		}
-	} while (!pendet_enabled);
-
-	return 0;
+	int err = 0;
+	u8 madc = 0x20;
+	err = intel_scu_ipc_update_register(PMIC_REG_MADCINT, 0x20, 0x20);
+	if (!err)
+		err = intel_scu_ipc_update_register(PMIC_REG_ADCCNTL1, 0, 0x05);
+	return err;
 }
 
 /*


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

* Re: [PATCH 4/6] * Register platform interface
  2010-07-23 16:07   ` Dmitry Torokhov
@ 2010-07-23 15:38     ` Alan Cox
  2010-07-23 16:43       ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2010-07-23 15:38 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: greg, linux-input

> I am confused here... Are you having a separate SPI driver create a
> platform device and then you have mrstouch to bind to this
> intermediate platform device? Are you doing that so you can introduce
> I2C interface later? If so I think I prefer how adxl34x and ad7879
> drivers are structured - they are split into core and interface parts
> but do not require extra devices/drivers (see in my 'next' brnach).

There is no SPI interface to the device. It ended up in the kernel SPI
because old versions of the device firmware listed it in the firmware
tables as SPI and rather than doing the right thing (correcting the
type) the x86 code created an SPI device for it.

At a certain level it may be SPI, but it's all hidden behind the
firmware on the SCU and nothing to do with Linux SPI at all.


Alan

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

* Re: [PATCH 4/6] * Register platform interface
  2010-07-23 13:52 ` [PATCH 4/6] * Register platform interface Alan Cox
@ 2010-07-23 16:07   ` Dmitry Torokhov
  2010-07-23 15:38     ` Alan Cox
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2010-07-23 16:07 UTC (permalink / raw)
  To: Alan Cox; +Cc: greg, linux-input

Hi Alan,

On Fri, Jul 23, 2010 at 02:52:21PM +0100, Alan Cox wrote:
> -static struct spi_driver mrstouch_driver = {
> +static struct platform_driver mrstouch_driver = {
>  	.driver = {
>  		.name   = "pmic_touch",
> -		.bus    = &spi_bus_type,
>  		.owner  = THIS_MODULE,
>  	},
>  	.probe          = mrstouch_probe,
> @@ -725,13 +722,13 @@ static struct spi_driver mrstouch_driver = {
>  
>  static int __init mrstouch_init(void)
>  {
> -	return spi_register_driver(&mrstouch_driver);
> +	return platform_driver_register(&mrstouch_driver);
>  }
>  module_init(mrstouch_init);
>  
>  static void __exit mrstouch_exit(void)
>  {
> -	spi_unregister_driver(&mrstouch_driver);
> +	platform_driver_unregister(&mrstouch_driver);
>  }
>  module_exit(mrstouch_exit);
>  

I am confused here... Are you having a separate SPI driver create a
platform device and then you have mrstouch to bind to this intermediate
platform device? Are you doing that so you can introduce I2C interface
later? If so I think I prefer how adxl34x and ad7879 drivers are
structured - they are split into core and interface parts but do not
require extra devices/drivers (see in my 'next' brnach).

Thanks.

-- 
Dmitry

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

* Re: [PATCH 4/6] * Register platform interface
  2010-07-23 15:38     ` Alan Cox
@ 2010-07-23 16:43       ` Dmitry Torokhov
  2010-07-26 22:49         ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2010-07-23 16:43 UTC (permalink / raw)
  To: Alan Cox; +Cc: greg, linux-input

On Fri, Jul 23, 2010 at 04:38:59PM +0100, Alan Cox wrote:
> > I am confused here... Are you having a separate SPI driver create a
> > platform device and then you have mrstouch to bind to this
> > intermediate platform device? Are you doing that so you can introduce
> > I2C interface later? If so I think I prefer how adxl34x and ad7879
> > drivers are structured - they are split into core and interface parts
> > but do not require extra devices/drivers (see in my 'next' brnach).
> 
> There is no SPI interface to the device. It ended up in the kernel SPI
> because old versions of the device firmware listed it in the firmware
> tables as SPI and rather than doing the right thing (correcting the
> type) the x86 code created an SPI device for it.
> 
> At a certain level it may be SPI, but it's all hidden behind the
> firmware on the SCU and nothing to do with Linux SPI at all.
> 

Ah, OK then.

So, what is the plan of action? Greg, are you going to apply the patches
to staging and move over to input or shoudl I do that (apply Alan's
patches to staging _in my tree_ and move over to drivers.input)?

Thanks.

-- 
Dmitry

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

* Re: [PATCH 4/6] * Register platform interface
  2010-07-23 16:43       ` Dmitry Torokhov
@ 2010-07-26 22:49         ` Greg KH
  2010-07-27  8:16           ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2010-07-26 22:49 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Alan Cox, linux-input

On Fri, Jul 23, 2010 at 09:43:17AM -0700, Dmitry Torokhov wrote:
> On Fri, Jul 23, 2010 at 04:38:59PM +0100, Alan Cox wrote:
> > > I am confused here... Are you having a separate SPI driver create a
> > > platform device and then you have mrstouch to bind to this
> > > intermediate platform device? Are you doing that so you can introduce
> > > I2C interface later? If so I think I prefer how adxl34x and ad7879
> > > drivers are structured - they are split into core and interface parts
> > > but do not require extra devices/drivers (see in my 'next' brnach).
> > 
> > There is no SPI interface to the device. It ended up in the kernel SPI
> > because old versions of the device firmware listed it in the firmware
> > tables as SPI and rather than doing the right thing (correcting the
> > type) the x86 code created an SPI device for it.
> > 
> > At a certain level it may be SPI, but it's all hidden behind the
> > firmware on the SCU and nothing to do with Linux SPI at all.
> > 
> 
> Ah, OK then.
> 
> So, what is the plan of action? Greg, are you going to apply the patches
> to staging and move over to input or shoudl I do that (apply Alan's
> patches to staging _in my tree_ and move over to drivers.input)?

I already have the driver in my staging-next tree, so I can apply these
6 patches as well, and then move it to the drivers/input subdirectory if
you want.

Or I can leave it all in drivers/staging/ and then after 2.6.36-rc1
comes, you can move the driver into drivers/input/ subdir.

Which do you prefer?

thanks,

greg k-h

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

* Re: [PATCH 4/6] * Register platform interface
  2010-07-26 22:49         ` Greg KH
@ 2010-07-27  8:16           ` Dmitry Torokhov
  2010-07-27 15:22             ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2010-07-27  8:16 UTC (permalink / raw)
  To: Greg KH; +Cc: Alan Cox, linux-input

On Mon, Jul 26, 2010 at 03:49:42PM -0700, Greg KH wrote:
> On Fri, Jul 23, 2010 at 09:43:17AM -0700, Dmitry Torokhov wrote:
> > On Fri, Jul 23, 2010 at 04:38:59PM +0100, Alan Cox wrote:
> > > > I am confused here... Are you having a separate SPI driver create a
> > > > platform device and then you have mrstouch to bind to this
> > > > intermediate platform device? Are you doing that so you can introduce
> > > > I2C interface later? If so I think I prefer how adxl34x and ad7879
> > > > drivers are structured - they are split into core and interface parts
> > > > but do not require extra devices/drivers (see in my 'next' brnach).
> > > 
> > > There is no SPI interface to the device. It ended up in the kernel SPI
> > > because old versions of the device firmware listed it in the firmware
> > > tables as SPI and rather than doing the right thing (correcting the
> > > type) the x86 code created an SPI device for it.
> > > 
> > > At a certain level it may be SPI, but it's all hidden behind the
> > > firmware on the SCU and nothing to do with Linux SPI at all.
> > > 
> > 
> > Ah, OK then.
> > 
> > So, what is the plan of action? Greg, are you going to apply the patches
> > to staging and move over to input or shoudl I do that (apply Alan's
> > patches to staging _in my tree_ and move over to drivers.input)?
> 
> I already have the driver in my staging-next tree, so I can apply these
> 6 patches as well, and then move it to the drivers/input subdirectory if
> you want.
> 
> Or I can leave it all in drivers/staging/ and then after 2.6.36-rc1
> comes, you can move the driver into drivers/input/ subdir.
> 

The 2nd one I think is the best.

Thanks.

-- 
Dmitry

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

* Re: [PATCH 4/6] * Register platform interface
  2010-07-27  8:16           ` Dmitry Torokhov
@ 2010-07-27 15:22             ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2010-07-27 15:22 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Alan Cox, linux-input

On Tue, Jul 27, 2010 at 01:16:33AM -0700, Dmitry Torokhov wrote:
> On Mon, Jul 26, 2010 at 03:49:42PM -0700, Greg KH wrote:
> > On Fri, Jul 23, 2010 at 09:43:17AM -0700, Dmitry Torokhov wrote:
> > > On Fri, Jul 23, 2010 at 04:38:59PM +0100, Alan Cox wrote:
> > > > > I am confused here... Are you having a separate SPI driver create a
> > > > > platform device and then you have mrstouch to bind to this
> > > > > intermediate platform device? Are you doing that so you can introduce
> > > > > I2C interface later? If so I think I prefer how adxl34x and ad7879
> > > > > drivers are structured - they are split into core and interface parts
> > > > > but do not require extra devices/drivers (see in my 'next' brnach).
> > > > 
> > > > There is no SPI interface to the device. It ended up in the kernel SPI
> > > > because old versions of the device firmware listed it in the firmware
> > > > tables as SPI and rather than doing the right thing (correcting the
> > > > type) the x86 code created an SPI device for it.
> > > > 
> > > > At a certain level it may be SPI, but it's all hidden behind the
> > > > firmware on the SCU and nothing to do with Linux SPI at all.
> > > > 
> > > 
> > > Ah, OK then.
> > > 
> > > So, what is the plan of action? Greg, are you going to apply the patches
> > > to staging and move over to input or shoudl I do that (apply Alan's
> > > patches to staging _in my tree_ and move over to drivers.input)?
> > 
> > I already have the driver in my staging-next tree, so I can apply these
> > 6 patches as well, and then move it to the drivers/input subdirectory if
> > you want.
> > 
> > Or I can leave it all in drivers/staging/ and then after 2.6.36-rc1
> > comes, you can move the driver into drivers/input/ subdir.
> > 
> 
> The 2nd one I think is the best.

Great.  I'll queue up these patches then.  Alan, when 2.6.36-rc1 is out,
ask Dmitry to move the driver into the input subdir and all should be
good.

thanks,

greg k-h

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

* Re: [PATCH 3/6] mrst-touchscreen: Fix use before initialize in mrst_touch [Fix bug 2561]
  2010-07-23 13:52 ` [PATCH 3/6] mrst-touchscreen: Fix use before initialize in mrst_touch [Fix bug 2561] Alan Cox
@ 2010-07-27 18:17   ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2010-07-27 18:17 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-input, dmitry.torokhov

On Fri, Jul 23, 2010 at 02:52:12PM +0100, Alan Cox wrote:
> From: Arjan van de Ven <arjan@linux.intel.com>
> 
> The mrst touch screen driver, in mrstouch_adc_init does
> 
>                  dev_err(&tsdev->spi->dev, "Unable to read PMIC id\n");
> 
> which does not work very well since the tsdev->spi member does not get
> initialized until after the call to mrstouch_adc_init
> 
> this patch makes sure the ->spi member is initialized prior to the call
> to mrstouch_adc_init
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> Signed-off-by: Alan Cox <alan@linux.intel.com>

Why are there 3 patches attached to this email, all a bit different?

Care to resend this one so I know which to apply?

thanks,

greg k-h

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

* Re: [PATCH 1/6] Staging: mrst_touchscreen: clean up input side
  2010-07-23 13:51 ` [PATCH 1/6] mrst_touchscreen: clean up input side Alan Cox
@ 2010-07-27 18:18   ` Greg KH
  2010-08-25 14:46     ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2010-07-27 18:18 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-input, dmitry.torokhov

On Fri, Jul 23, 2010 at 02:51:52PM +0100, Alan Cox wrote:
> Fix most of the stuff that Dmitry pointed out. This leaves the mutex in IRQ
> and misuse of SPI to sort out.
> 
> Also fix the build bits so it actually builds in staging - whoops.
> 
> Signed-off-by: Alan Cox <alan@linux.intel.com>

This doesn't apply for some reason.  Care to resend all 6 of these so I
can apply them?

thanks,

greg k-h

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

* Re: [PATCH 1/6] Staging: mrst_touchscreen: clean up input side
  2010-07-27 18:18   ` [PATCH 1/6] Staging: " Greg KH
@ 2010-08-25 14:46     ` Dmitry Torokhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2010-08-25 14:46 UTC (permalink / raw)
  To: Greg KH; +Cc: Alan Cox, linux-input

On Tue, Jul 27, 2010 at 11:18:31AM -0700, Greg KH wrote:
> On Fri, Jul 23, 2010 at 02:51:52PM +0100, Alan Cox wrote:
> > Fix most of the stuff that Dmitry pointed out. This leaves the mutex in IRQ
> > and misuse of SPI to sort out.
> > 
> > Also fix the build bits so it actually builds in staging - whoops.
> > 
> > Signed-off-by: Alan Cox <alan@linux.intel.com>
> 
> This doesn't apply for some reason.  Care to resend all 6 of these so I
> can apply them?
> 

*ping* Do not see anything in linux-next/mainline... And the
driver/patches looked good to me.

-- 
Dmitry

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

end of thread, other threads:[~2010-08-25 14:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-23 13:51 [PATCH 0/6] Complete the mrst touchscreen tidy Alan Cox
2010-07-23 13:51 ` [PATCH 1/6] mrst_touchscreen: clean up input side Alan Cox
2010-07-27 18:18   ` [PATCH 1/6] Staging: " Greg KH
2010-08-25 14:46     ` Dmitry Torokhov
2010-07-23 13:52 ` [PATCH 2/6] Input: mrst - more fixes Alan Cox
2010-07-23 13:52 ` [PATCH 3/6] mrst-touchscreen: Fix use before initialize in mrst_touch [Fix bug 2561] Alan Cox
2010-07-27 18:17   ` Greg KH
2010-07-23 13:52 ` [PATCH 4/6] * Register platform interface Alan Cox
2010-07-23 16:07   ` Dmitry Torokhov
2010-07-23 15:38     ` Alan Cox
2010-07-23 16:43       ` Dmitry Torokhov
2010-07-26 22:49         ` Greg KH
2010-07-27  8:16           ` Dmitry Torokhov
2010-07-27 15:22             ` Greg KH
2010-07-23 13:52 ` [PATCH 5/6] fix channel allocation in the touch screen driver Alan Cox
2010-07-23 13:52 ` [PATCH 6/6] Simplify en/disable of interrupts for NEC Alan Cox

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.