Re: [PATCH] i2c: cyttsp i2c touchscreen driver init submit

From: Trilok Soni
Date: Tue Jul 13 2010 - 04:43:02 EST


On 7/13/2010 1:25 PM, Dmitry Torokhov wrote:
> On Tue, Jul 13, 2010 at 01:01:32PM +0530, Trilok Soni wrote:
>> Hi Kevin,
>>
>> Thanks for posting this driver.
>>
>> Adding Jean Delvar for i2c bits.
>>
>> On 7/13/2010 2:26 AM, Kevin McNeely wrote:
>>> From: Fred <fwk@xxxxxxxxxxxxxxxxxxxxxxxxx>
>>
>> E-mail id looks wrong. Do you mean fwk@xxxxxxxxxxx?
>>
>>>
>>> This is a new touchscreen driver for the Cypress Semiconductor
>>> cyttsp family of devices. This driver is for the i2c version
>>> of cyttsp parts.
>>
>> Please explain in commit text which exact version of the chips this driver is supporting.
>> It is hard to make out that from this text.
>>>
>>> Signed-off-by: Kevin McNeely <kev@xxxxxxxxxxx>
>>> ---
>>> drivers/input/touchscreen/Kconfig | 13 +
>>> drivers/input/touchscreen/Makefile | 1 +
>>> drivers/input/touchscreen/cyttsp-i2c.c | 2016 ++++++++++++++++++++++++++++++++
>>> include/linux/cyttsp.h | 649 ++++++++++
>>
>> Please move this file to include/linux/input directory.
>>
>
> Or even keep it in drivers/input/touchscreen/

They are having platform data structure in this header file which might be accessed from the board-xxx.c
files under mach-xxx directories of ARM like architecture.

>
>>
>>>
>>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>>> index 3b9d5e2..a7a69a0 100644
>>> --- a/drivers/input/touchscreen/Kconfig
>>> +++ b/drivers/input/touchscreen/Kconfig
>>> @@ -603,4 +603,17 @@ config TOUCHSCREEN_TPS6507X
>>> To compile this driver as a module, choose M here: the
>>> module will be called tps6507x_ts.
>>>
>>> +config TOUCHSCREEN_CYTTSP_I2C
>>> + default n
>>
>> Do we need to provide this if it is no by default?
>>
>>> + tristate "Cypress TTSP i2c touchscreen"
>>> + depends on I2C
>>> + help
>>> + Say Y here if you have a Cypress TTSP touchscreen
>>> + connected to your system's i2c bus.
>>
>> What is TTSP?
>>
>>> +
>>> + If unsure, say N.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called cyttsp_i2c.
>>> +
>>> endif
>>
>
> Since there is SPI part should we prepare for the support and split
> bus-independent parts off? Are you working on SPI support?

I don't think the current patch is divided neatly to drive this chip over
multiple bus protocols.

>
>>
>>> diff --git a/drivers/input/touchscreen/cyttsp-i2c.c b/drivers/input/touchscreen/cyttsp-i2c.c
>>> new file mode 100644
>>> index 0000000..8397aa1
>>> --- /dev/null
>>> +++ b/drivers/input/touchscreen/cyttsp-i2c.c
>>> @@ -0,0 +1,2016 @@
>>> +/* Source for:
>>> + * Cypress TrueTouch(TM) Standard Product I2C touchscreen driver.
>>> + * drivers/input/touchscreen/cyttsp-i2c.c
>>
>> No file paths please. Already commented on it by Christoph.
>>
>>> + *
>>> + * Copyright (C) 2009, 2010 Cypress Semiconductor, Inc.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License
>>> + * version 2, and only version 2, as published by the
>>> + * Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, write to the Free Software Foundation, Inc.,
>>> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
>>> + *
>>> + * Cypress reserves the right to make changes without further notice
>>> + * to the materials described herein. Cypress does not assume any
>>> + * liability arising out of the application described herein.
>>> + *
>>> + * Contact Cypress Semiconductor at www.cypress.com
>>
>> I would like Dmitry to comment on it. Dmitry?
>>
>
> Not a lwayer but I do not really see an issue here. It is still GPL and
> they as copyright holders obviously can modify the code. What exactly
> troubles you here?

No issue from my side too.

---Trilok Soni

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/