Re: [PATCH v3 2/5] Input: add driver for Focaltech FTS touchscreen

[Joel Selvaraj <joelselvaraj.oss@gmail.com>]

Hi Jeff LaBundy,

On 4/23/23 21:39, Jeff LaBundy wrote:
> Hi Joel,
> 
> Great work so far! It's coming along nicely. Please find my latest
> feedback below.

Sorry for the late reply, university semester end got me hooked up.
I have a sad kind of good news... As pointed out by Hans de Goede, the 
edt-ft5x06 driver works out of the box for the fts8719 touchscreen in my 
device. So I think this patch series is no longer needed. I did have a 
look at the driver once when working on this patch series, but it looked 
different/not compatible at that time. After mentioned by Hans de Goede, 
I had a more closer look and the main touch buffer handling seems to be 
the same.

I am sorry as we put some considerable time in this patch series. I 
should have noted more carefully. Thank you though as I learnt things 
working on this patch series.

I guess I will send a different patch to add the compatible data to the 
existing edt-ft5x06 driver and dts changes to include that driver to my 
device.

> On Fri, Apr 14, 2023 at 09:02:19PM -0500, Joel Selvaraj wrote:
>> The Focaltech FTS driver supports several variants of focaltech
>> touchscreens found in ~2018 era smartphones including variants found on
>> the PocoPhone F1 and the SHIFT6mq which are already present in mainline.
>> This driver is loosely based on the original driver from Focaltech
>> but has been simplified and largely reworked.
>>
>> Co-developed-by: Caleb Connolly <caleb@connolly.tech>
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>>   MAINTAINERS                                   |   8 +
>>   drivers/input/touchscreen/Kconfig             |  12 +
>>   drivers/input/touchscreen/Makefile            |   1 +
>>   drivers/input/touchscreen/focaltech_fts5452.c | 432 ++++++++++++++++++
>>   4 files changed, 453 insertions(+)
>>   create mode 100644 drivers/input/touchscreen/focaltech_fts5452.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7ec4ce64f66d..1a3ea61e1f52 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8028,6 +8028,14 @@ L:	linux-input@vger.kernel.org
>>   S:	Maintained
>>   F:	drivers/input/joystick/fsia6b.c
>>   
>> +FOCALTECH FTS5452 TOUCHSCREEN DRIVER
>> +M:	Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> +M:	Caleb Connolly <caleb@connolly.tech>
>> +L:	linux-input@vger.kernel.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml
>> +F:	drivers/input/touchscreen/focaltech_fts5452.c
>> +
>>   FOCUSRITE SCARLETT GEN 2/3 MIXER DRIVER
>>   M:	Geoffrey D. Bennett <g@b4.vu>
>>   L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>> index 1feecd7ed3cb..11af91504969 100644
>> --- a/drivers/input/touchscreen/Kconfig
>> +++ b/drivers/input/touchscreen/Kconfig
>> @@ -388,6 +388,18 @@ config TOUCHSCREEN_EXC3000
>>   	  To compile this driver as a module, choose M here: the
>>   	  module will be called exc3000.
>>   
>> +config TOUCHSCREEN_FOCALTECH_FTS5452
>> +	tristate "Focaltech FTS Touchscreen"
>> +	depends on I2C
>> +	help
>> +	  Say Y here to enable support for I2C connected Focaltech FTS
>> +	  based touch panels, including the 5452 and 8917 panels.
> 
> This language is a bit misleading, as it seems to suggest three or more
> models are supported. It seems the title should simply be "FocalTech
> FTS5452 touchscreen controller" with the description as "...FocalTech
> FTS5452 and compatible touchscreen controllers."
> 
> As more are found to be compatible (e.g. FTS8917), the compatible strings
> can simply be appended.
> 
>> +
>> +	  If unsure, say N.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called focaltech_fts.
>> +
>>   config TOUCHSCREEN_FUJITSU
>>   	tristate "Fujitsu serial touchscreen"
>>   	select SERIO
>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>> index 159cd5136fdb..47d78c9cff21 100644
>> --- a/drivers/input/touchscreen/Makefile
>> +++ b/drivers/input/touchscreen/Makefile
>> @@ -45,6 +45,7 @@ obj-$(CONFIG_TOUCHSCREEN_ELO)		+= elo.o
>>   obj-$(CONFIG_TOUCHSCREEN_EGALAX)	+= egalax_ts.o
>>   obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL)	+= egalax_ts_serial.o
>>   obj-$(CONFIG_TOUCHSCREEN_EXC3000)	+= exc3000.o
>> +obj-$(CONFIG_TOUCHSCREEN_FOCALTECH_FTS5452)	+= focaltech_fts5452.o
>>   obj-$(CONFIG_TOUCHSCREEN_FUJITSU)	+= fujitsu_ts.o
>>   obj-$(CONFIG_TOUCHSCREEN_GOODIX)	+= goodix_ts.o
>>   obj-$(CONFIG_TOUCHSCREEN_HIDEEP)	+= hideep.o
>> diff --git a/drivers/input/touchscreen/focaltech_fts5452.c b/drivers/input/touchscreen/focaltech_fts5452.c
>> new file mode 100644
>> index 000000000000..abf8a2f271ca
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/focaltech_fts5452.c
>> @@ -0,0 +1,432 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * FocalTech touchscreen driver.
>> + *
>> + * Copyright (c) 2010-2017, FocalTech Systems, Ltd., all rights reserved.
>> + * Copyright (C) 2018 XiaoMi, Inc.
>> + * Copyright (c) 2021 Caleb Connolly <caleb@connolly.tech>
>> + * Copyright (c) 2023 Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> + */

[skip]

>> +static const struct of_device_id fts_of_match[] = {
>> +	{ .compatible = "focaltech,fts5452", .data = &fts5452_chip_data },
>> +	{ .compatible = "focaltech,fts8719", .data = &fts8719_chip_data },
>> +	{ /* sentinel */ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, fts_of_match);
>> +
>> +static struct i2c_driver fts_ts_driver = {
>> +	.probe_new = fts_ts_probe,
>> +	.id_table = fts_i2c_id,
>> +	.driver = {
>> +		.name = FTS_DRIVER_NAME,
>> +		.pm = pm_sleep_ptr(&fts_dev_pm_ops),
>> +		.of_match_table = fts_of_match,
>> +	},
>> +};
>> +module_i2c_driver(fts_ts_driver);
>> +
>> +MODULE_AUTHOR("Joel Selvaraj <joelselvaraj.oss@gmail.com>");
>> +MODULE_AUTHOR("Caleb Connolly <caleb@connolly.tech>");
>> +MODULE_DESCRIPTION("Focaltech Touchscreen Driver");
> 
> Nit: mixing 'FocalTech' and 'Focaltech' throughout.
> 
>> +MODULE_LICENSE("GPL");
>> -- 
>> 2.40.0
>>
> 
> Kind regards,
> Jeff LaBundy

Thank you,
Joel Selvaraj