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

Joel Selvaraj
2023-05-12T00:43:38Z

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