input/touch: LPC32xx: Introduce touch driver for the LPC32xx (v2)
Subsystems (3)
This patch set introduces support for the LPC32xx touchscreen controller driver. The LPC32xx touchscreen controller supports automated event detection and conversion for resistive touchscreens.
The patch can be pulled from:
git://git.lpclinux.com/linux-2.6-lpc ts-lpc32xx_v2
v2 includes updates from the initial review of the TSC driver..
All local TSC_* macros have been prefixed with LCP32XX_ Added lpc32xx_ prefix to stop_tsc() and start_tsc() Moved clk_enable and clk_disable into lpc32xx_start_tsc()
and lpc32xx_stop_tsc() functions
Added ->open and ->close methods using the start and
stop tsc functions
Removed use of devm_* functions. Modified error unwinding
and _remove() based on this change.
Fixed an error return condition from input_register_device() Fixed use of input_free_device
Added a use count check for suspend and resume so device
state isn't altered if it isn't open
PATCH 1/1 input/touch: Introduce the LPC32xx touchscreen controller driver (v2)
From: Kevin Wells <[email protected]>
This patch set introduces support for the LPC32xx touchscreen controller driver. The LPC32xx touchscreen controller supports automated event detection and X/Y data conversion for resistive touchscreens.
--- drivers/input/touchscreen/Kconfig | 10 + drivers/input/touchscreen/Makefile | 1 + drivers/input/touchscreen/lpc32xx_ts.c | 404 ++++++++++++++++++++++++++++++++ 3 files changed, 415 insertions(+), 0 deletions(-) create mode 100644 drivers/input/touchscreen/lpc32xx_ts.c
drivers/input/touchscreen/Kconfig +10 -0
@@ -214,6 +214,16 @@ config TOUCHSCREEN_WACOM_W8001 To compile this driver as a module, choose M here: the module will be called wacom_w8001. +config TOUCHSCREEN_LPC32XX + tristate "LPC32XX touchscreen controller" + depends on ARCH_LPC32XX + help + Say Y here if you have a LPC32XX device and want + to support the built-in touchscreen. + + To compile this driver as a module, choose M here: the + module will be called lpc32xx_ts. + config TOUCHSCREEN_MCS5000 tristate "MELFAS MCS-5000 touchscreen" depends on I2C
drivers/input/touchscreen/Makefile +1 -0
@@ -23,6 +23,7 @@ obj-$(CONFIG_TOUCHSCREEN_EETI) += eeti_ts.o obj-$(CONFIG_TOUCHSCREEN_ELO) += elo.o obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o +obj-$(CONFIG_TOUCHSCREEN_LPC32XX) += lpc32xx_ts.o obj-$(CONFIG_TOUCHSCREEN_MC13783) += mc13783_ts.o obj-$(CONFIG_TOUCHSCREEN_MCS5000) += mcs5000_ts.o obj-$(CONFIG_TOUCHSCREEN_MIGOR) += migor_ts.o
drivers/input/touchscreen/lpc32xx_ts.c new file +404 -0
404 lines not on this page · raw message
Signature · 1 line
1.7.1.1
PATCH 1/1 input/touch: Introduce the LPC32xx touchscreen controller driver (v2)
Quoted 6 lines · On 08/17/2010 09:21 AM, wellsk40 at gmail.com wrote:
From: Kevin Wells <[email protected]>
This patch set introduces support for the LPC32xx touchscreen controller driver. The LPC32xx touchscreen controller supports automated event detection and X/Y data conversion for resistive touchscreens.
Hi Kevin,
A few comments below.
drivers/input/touchscreen/lpc32xx_ts.c line 59
Quoted 10 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +10
-0+ +#define LPC32XX_TSC_ADCCON_IRQ_TO_FIFO_4 (0x1 << 11) +#define LPC32XX_TSC_ADCCON_X_SAMPLE_SIZE (s) ((10 - (s)) << 7) +#define LPC32XX_TSC_ADCCON_Y_SAMPLE_SIZE (s) ((10 - (s)) << 4) +#define LPC32XX_TSC_ADCCON_POWER_UP (1 << 2) +#define LPC32XX_TSC_ADCCON_AUTO_EN (1 << 0) + +#define LPC32XX_TSC_FIFO_TS_P_LEVEL (1 << 31) +#define LPC32XX_TSC_FIFO_NORMALIZE_X_VAL (x) (((x) & 0x03FF0000) >> 16) +#define LPC32XX_TSC_FIFO_NORMALIZE_Y_VAL (y) ((y) & 0x000003FF)
Some of these names are really long which causes lots of line breaks in the code. Can you shorten some of the names to make it more readable.
drivers/input/touchscreen/lpc32xx_ts.c line 71
Quoted 11 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +11
-0+#define LPC32XX_TSC_ADCDAT_VALUE_MASK 0x000003FF + +#define LPC32XX_TSC_MIN_XY_VAL 0x0 +#define LPC32XX_TSC_MAX_XY_VAL 0x3FF + +#define MOD_NAME "ts-lpc32xx" + +#define tsc_readl (dev, reg) \ + __raw_readl((dev)->tsc_base + (reg)) +#define tsc_writel (dev, reg, val) \ + __raw_writel((val), (dev)->tsc_base + (reg))
inline functions are better for this sort of thing.
drivers/input/touchscreen/lpc32xx_ts.c line 78
Quoted 7 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +7
-0+ +struct lpc32xx_tsc { +struct input_dev *dev; +void __iomem *tsc_base; +int irq; +struct clk *clk; +};
(Nitpicky) Tab delimit this to make it a bit more readable.
drivers/input/touchscreen/lpc32xx_ts.c line 85
Quoted 6 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +6
-0+static void lpc32xx_fifo_clear (struct lpc32xx_tsc *tsc) +{ +while (!(tsc_readl (tsc,LPC32XX_TSC_STAT ) & +LPC32XX_TSC_STAT_FIFO_EMPTY )) +tsc_readl (tsc,LPC32XX_TSC_FIFO ); +}
The FIFO_EMTPY bit gets used a couple of times, so maybe:
static inline tsc_fifo_empty(struct lpc32xx_tsc *tsc)
{
return tsc_readl(tsc, LPC32XX_TSC_STAT) &
LPC32XX_TSC_STAT_FIFO_EMPTY;
}Also, is it worth having a timeout on that while loop so that if the i2c bus dies or something that you don't get stuck there forever?
drivers/input/touchscreen/lpc32xx_ts.c line 97
Quoted 11 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +11
-0+static irqreturn_t lpc32xx_ts_interrupt (int irq,void *dev_id) +{ +u32 tmp, rv[4 ], xs[4 ], ys[4 ]; +int idx; +struct lpc32xx_tsc *tsc = dev_id; +struct input_dev *input = tsc->dev; + + tmp =tsc_readl (tsc,LPC32XX_TSC_STAT ); + +if (tmp &LPC32XX_TSC_STAT_FIFO_OVRRN ) { +/* FIFO overflow - throw away samples */
Should there be a dev_err/warn here to let the user know?
drivers/input/touchscreen/lpc32xx_ts.c line 110
Quoted 13 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +13
-0+lpc32xx_fifo_clear (tsc); +return IRQ_HANDLED ; + } + +/* +* Gather and normalize 4 samples. Pen-up events may have less +* than 4 samples, but its ok to pop 4 and let the last sample +* pen status check drop the samples. +*/ + idx =0 ; +while ((idx <4 ) && + (!(tsc_readl (tsc,LPC32XX_TSC_STAT ) & +LPC32XX_TSC_STAT_FIFO_EMPTY ))) {
for (idx = 0; idx < 4 && !tsc_fifo_empty(); idx++) {
...Is a bit more readable.
Quoted 56 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +56
-0+ tmp =tsc_readl (tsc,LPC32XX_TSC_FIFO ); + xs[idx] =LPC32XX_TSC_ADCDAT_VALUE_MASK - +LPC32XX_TSC_FIFO_NORMALIZE_X_VAL (tmp); + ys[idx] =LPC32XX_TSC_ADCDAT_VALUE_MASK - +LPC32XX_TSC_FIFO_NORMALIZE_Y_VAL (tmp); + rv[idx] = tmp; + idx++; + } + +/* Data is only valid if pen is still down in last sample */ +if ((!(rv[3 ] &LPC32XX_TSC_FIFO_TS_P_LEVEL )) && (idx ==4 )) { +/* Use average of 2nd and 3rd sample for position */ +input_report_abs (input,ABS_X , ((xs[1 ] + xs[2 ]) /2 )); +input_report_abs (input,ABS_Y , ((ys[1 ] + ys[2 ]) /2 )); +input_report_key (input,BTN_TOUCH ,1 ); + }else { +input_report_key (input,BTN_TOUCH ,0 ); + } + +input_sync (input); + +return IRQ_HANDLED ; +} + +static void lpc32xx_stop_tsc (struct lpc32xx_tsc *tsc) +{ +/* Disable auto mode */ +tsc_writel (tsc,LPC32XX_TSC_CON , +tsc_readl (tsc,LPC32XX_TSC_CON ) & + ~LPC32XX_TSC_ADCCON_AUTO_EN ); + +clk_disable (tsc->clk); +} + +static void lpc32xx_setup_tsc (struct lpc32xx_tsc *tsc) +{ +u32 tmp; + +clk_enable (tsc->clk); + + tmp =tsc_readl (tsc,LPC32XX_TSC_CON ) & ~LPC32XX_TSC_ADCCON_POWER_UP ; + +/* Set the TSC FIFO depth to 4 samples @ 10-bits per sample (max) */ + tmp = (LPC32XX_TSC_ADCCON_IRQ_TO_FIFO_4 | +LPC32XX_TSC_ADCCON_X_SAMPLE_SIZE (10 ) | +LPC32XX_TSC_ADCCON_Y_SAMPLE_SIZE (10 )); +tsc_writel (tsc,LPC32XX_TSC_CON , tmp); + +/* These values are all preset */ +tsc_writel (tsc,LPC32XX_TSC_SEL ,LPC32XX_TSC_SEL_DEFVAL ); +tsc_writel (tsc,LPC32XX_TSC_MIN_X ,LPC32XX_TSC_MIN_XY_VAL ); +tsc_writel (tsc,LPC32XX_TSC_MAX_X ,LPC32XX_TSC_MAX_XY_VAL ); +tsc_writel (tsc,LPC32XX_TSC_MIN_Y ,LPC32XX_TSC_MIN_XY_VAL ); +tsc_writel (tsc,LPC32XX_TSC_MAX_Y ,LPC32XX_TSC_MAX_XY_VAL ); + +/* Aux support is not used */
drivers/input/touchscreen/lpc32xx_ts.c line 176
Quoted 10 lines
drivers/input/touchscreen/lpc32xx_ts.c new file +10
-0+tsc_writel (tsc,LPC32XX_TSC_AUX_UTR ,0 ); +tsc_writel (tsc,LPC32XX_TSC_AUX_MIN ,0 ); +tsc_writel (tsc,LPC32XX_TSC_AUX_MAX ,0 ); + +/* +* Set sample rate to about 240Hz per X/Y pair. A single measurement +* consists of 4 pairs which gives about a 60Hz sample rate based on +* a stable 32768Hz clock source. Values are in clocks. +* Rate is (32768 / (RTR + XCONV + RTR + YCONV + DXP + TTR + UTR) / 4 +*/
Is the clock source internal, or at least always 32.768kHz? If not, should the timing be controlled via some platform data so that this driver is compatible with other board setups?
~Ryan
Signature · 7 lines
Bluewater Systems Ltd - ARM Technology Solution Centre Ryan Mallon 5 Amuri Park, 404 Barbadoes St ryan at bluewatersys.com PO Box 13 889, Christchurch 8013 http://www.bluewatersys.com New Zealand Phone: +64 3 3779127 Freecall: Australia 1800 148 751 Fax: +64 3 3779135 USA 1800 261 2934
PATCH 1/1 input/touch: Introduce the LPC32xx touchscreen controller driver (v2)
Hi Ryan,
Thanks for taking the time to review this.
Quoted 8 lines
Quoted 7 lines
+
+ ? ? /*
+ ? ? ?* Set sample rate to about 240Hz per X/Y pair. A single measurement
+ ? ? ?* consists of 4 pairs which gives about a 60Hz sample rate based on
+ ? ? ?* a stable 32768Hz clock source. Values are in clocks.
+ ? ? ?* Rate is (32768 / (RTR + XCONV + RTR + YCONV + DXP + TTR + UTR) / 4
+ ? ? ?*/
Quoted 4 lines
Is the clock source internal, or at least always 32.768kHz? If not, should the timing be controlled via some platform data so that this driver is compatible with other board setups?
The clock source for this is internal and locked in the driver and clock control handler at 32768Hz.
As much as I want to argue not using platform data :), I can't think of one good reason not to use it for this (the keyscanner driver has it). I probably should of
used it from the start.
PATCH 1/1 input/touch: Introduce the LPC32xx touchscreen controller driver (v2)
In reply to wellsk40 at gmail.comQuoted 9 lines · On Mon, Aug 16, 2010 at 02:21:01PM -0700, wellsk40 at gmail.com wrote:
From: Kevin Wells <[email protected]>
This patch set introduces support for the LPC32xx touchscreen controller driver. The LPC32xx touchscreen controller supports automated event detection and X/Y data conversion for resistive touchscreens.
Signed-off-by Kevin Wells <[email protected]>Signed-off-by Durgesh Pattamatta <[email protected]>
Applied to my .37 queue, thanks Kevin.
Signature · 1 line
Dmitry
PATCH 1/1 input/touch: Introduce the LPC32xx touchscreen controller driver (v2)
Quoted 13 lines
Subject: Re: [PATCH] input/touch: Introduce the LPC32xx touchscreen controller driver (v2)
Quoted 7 lines · On Mon, Aug 16, 2010 at 02:21:01PM -0700, wellsk40 at gmail.com wrote:
From: Kevin Wells <[email protected]>
This patch set introduces support for the LPC32xx touchscreen controller driver. The LPC32xx touchscreen controller supports automated event detection and X/Y data conversion for resistive touchscreens.
Quoted 5 lines
Quoted 2 lines
Signed-off-by Kevin Wells <[email protected]>Signed-off-by Durgesh Pattamatta <[email protected]>Applied to my .37 queue, thanks Kevin.
Thanks for applying this Dmitry.
Ryan M also had some good comments about the v2 driver that I'll resolve once some other associated fixes get pulled in.
Quoted 7 lines
--
Dmitry_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel