> Add Texas Instruments TSC2005 chip touchscreen driver.
>
> Signed-off-by: Trilok Soni <soni.trilok@xxxxxxxxx>
Hi Trilok.
A few nitpicks below.
In general a very clean written driver with adequate comments - nice work!
Sam
> ---
> drivers/input/touchscreen/Kconfig | 6 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/tsc2005.c | 728 +++++++++++++++++++++++++++++++++++
> 3 files changed, 735 insertions(+), 0 deletions(-)
> create mode 100644 drivers/input/touchscreen/tsc2005.c
>
> diff --git a/drivers/input/touchscreen/Kconfig
> b/drivers/input/touchscreen/Kconfig
> index 3d1ab8f..77e6729 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -221,6 +221,12 @@ config TOUCHSCREEN_ATMEL_TSADCC
> To compile this driver as a module, choose M here: the
> module will be called atmel_tsadcc.
>
> +config TOUCHSCREEN_TSC2005
> + tristate "TSC2005 touchscreen support"
I would be good to see Texas Instruments spelled out here.
> + depends on SPI_MASTER
> + help
> + Say Y here for if you are using the touchscreen features of TSC2005.
And maybe with a bit text explaining where it is used or maybe where to find info.
> +#define TSC2005_VDD_LOWER_27
> +
> +#ifdef TSC2005_VDD_LOWER_27
> +#define TSC2005_HZ (10000000)
> +#else
> +#define TSC2005_HZ (25000000)
> +#endif
You define TSC2005_VDD_LOWER_27 and test for it two
lines later - looks strange.
> +
> +static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
> +{
> + u16 data = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
> + struct spi_message msg;
> + struct spi_transfer xfer = { 0 };
> +
> + xfer.tx_buf = &data;
> + xfer.rx_buf = NULL;
> + xfer.len = 1;
> + xfer.bits_per_word = 8;
data is a 16 bit quantity yet you specify a len of 1.
Maybe len is counted from 0?
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfer, &msg);
> + spi_sync(ts->spi, &msg);
> +}
> +
> +static void tsc2005_write(struct tsc2005 *ts, u8 reg, u16 value)
> +{
> + u32 tx;
> + struct spi_message msg;
> + struct spi_transfer xfer = { 0 };
> +
> + tx = (TSC2005_REG | reg | TSC2005_REG_PND0 |
> + TSC2005_REG_WRITE) << 16;
> + tx |= value;
Is this endian safe? Does spi know about LSB/MSB in the value?
> +
> + xfer.tx_buf = &tx;
> + xfer.rx_buf = NULL;
> + xfer.len = 4;
> + xfer.bits_per_word = 24;
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfer, &msg);
> + spi_sync(ts->spi, &msg);
> +}
Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
