2023-06-06 14:38:38

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH RFC 3/4] input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC

Add initial support for the new Goodix "Berlin" touchscreen ICs
over the I2C interface.

This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.

The current implementation only supports BerlinD, aka GT9916.

[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers

Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/input/touchscreen/Kconfig | 14 +++++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_i2c.c | 76 +++++++++++++++++++++++++++
3 files changed, 91 insertions(+)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 1a6f6f6da991..da6d5d75c42d 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -421,6 +421,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_CORE
depends on REGMAP
tristate

+config TOUCHSCREEN_GOODIX_BERLIN_I2C
+ tristate "Goodix Berlin I2C touchscreen"
+ depends on I2C
+ depends on REGMAP_I2C
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have the a touchscreen connected to your
+ system using the Goodix Berlin IC connection via I2C.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_i2c.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 29cdb042e104..921a2da0c2be 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
new file mode 100644
index 000000000000..fc32b8077287
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <asm/unaligned.h>
+
+#include "goodix_berlin.h"
+
+#define I2C_MAX_TRANSFER_SIZE 256
+
+static const struct regmap_config goodix_berlin_i2c_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .max_raw_read = I2C_MAX_TRANSFER_SIZE,
+ .max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+static const struct input_id goodix_berlin_i2c_input_id = {
+ .bustype = BUS_I2C,
+ .vendor = 0x0416,
+ .product = 0x1001,
+};
+
+static int goodix_berlin_i2c_probe(struct i2c_client *client)
+{
+ struct regmap *map;
+
+ map = devm_regmap_init_i2c(client, &goodix_berlin_i2c_regmap_conf);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ return goodix_berlin_probe(&client->dev, client->irq,
+ &goodix_berlin_i2c_input_id, map);
+}
+
+static void goodix_berlin_i2c_remove(struct i2c_client *client)
+{
+ goodix_berlin_remove(&client->dev);
+}
+
+static const struct i2c_device_id goodix_berlin_i2c_id[] = {
+ { "gt9916", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
+
+static const struct of_device_id goodix_berlin_i2c_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { },
+};
+
+static struct i2c_driver goodix_berlin_i2c_driver = {
+ .driver = {
+ .name = "goodix-berlin-i2c",
+ .of_match_table = goodix_berlin_i2c_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_i2c_probe,
+ .remove = goodix_berlin_i2c_remove,
+ .id_table = goodix_berlin_i2c_id,
+};
+module_i2c_driver(goodix_berlin_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin I2C Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <[email protected]>");

--
2.34.1



2023-06-12 03:41:31

by Jeff LaBundy

[permalink] [raw]
Subject: Re: [PATCH RFC 3/4] input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC

Hi Neil,

Excellent work; just a few nits on this particular instance.

On Tue, Jun 06, 2023 at 04:31:58PM +0200, Neil Armstrong wrote:
> Add initial support for the new Goodix "Berlin" touchscreen ICs
> over the I2C interface.
>
> This initial driver is derived from the Goodix goodix_ts_berlin
> available at [1] and [2] and only supports the GT9916 IC
> present on the Qualcomm SM8550 MTP & QRD touch panel.
>
> The current implementation only supports BerlinD, aka GT9916.
>
> [1] https://github.com/goodix/goodix_ts_berlin
> [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/input/touchscreen/Kconfig | 14 +++++
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/goodix_berlin_i2c.c | 76 +++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 1a6f6f6da991..da6d5d75c42d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -421,6 +421,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_CORE
> depends on REGMAP
> tristate
>
> +config TOUCHSCREEN_GOODIX_BERLIN_I2C
> + tristate "Goodix Berlin I2C touchscreen"
> + depends on I2C
> + depends on REGMAP_I2C

As you already depend upon REGMAP, I think you can simply select REGMAP_I2C
here instead of depending upon it as well.

> + select TOUCHSCREEN_GOODIX_BERLIN_CORE
> + help
> + Say Y here if you have the a touchscreen connected to your
> + system using the Goodix Berlin IC connection via I2C.

This language was a bit of a tongue-twister; perhaps it is better to say
"...if you have a Goodix Berlin IC connected to your system via I2C."

> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called goodix_berlin_i2c.
> +
> config TOUCHSCREEN_HIDEEP
> tristate "HiDeep Touch IC"
> depends on I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 29cdb042e104..921a2da0c2be 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
> obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
> +obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
> obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
> obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
> new file mode 100644
> index 000000000000..fc32b8077287
> --- /dev/null
> +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Goodix Berlin Touchscreen Driver
> + *
> + * Copyright (C) 2020 - 2021 Goodix, Inc.
> + * Copyright (C) 2023 Linaro Ltd.
> + *
> + * Based on goodix_ts_berlin driver.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <asm/unaligned.h>

This last include does not seem to provide anything.

> +
> +#include "goodix_berlin.h"
> +
> +#define I2C_MAX_TRANSFER_SIZE 256
> +
> +static const struct regmap_config goodix_berlin_i2c_regmap_conf = {
> + .reg_bits = 32,
> + .val_bits = 8,
> + .max_raw_read = I2C_MAX_TRANSFER_SIZE,
> + .max_raw_write = I2C_MAX_TRANSFER_SIZE,
> +};
> +
> +static const struct input_id goodix_berlin_i2c_input_id = {
> + .bustype = BUS_I2C,
> + .vendor = 0x0416,
> + .product = 0x1001,
> +};
> +
> +static int goodix_berlin_i2c_probe(struct i2c_client *client)
> +{
> + struct regmap *map;

'regmap' tends to be more common than simply 'map'.

> +
> + map = devm_regmap_init_i2c(client, &goodix_berlin_i2c_regmap_conf);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + return goodix_berlin_probe(&client->dev, client->irq,
> + &goodix_berlin_i2c_input_id, map);
> +}
> +
> +static void goodix_berlin_i2c_remove(struct i2c_client *client)
> +{
> + goodix_berlin_remove(&client->dev);
> +}
> +
> +static const struct i2c_device_id goodix_berlin_i2c_id[] = {
> + { "gt9916", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
> +
> +static const struct of_device_id goodix_berlin_i2c_of_match[] = {
> + { .compatible = "goodix,gt9916", },
> + { },

No need for a trailing comma following the sentinel.

> +};
> +
> +static struct i2c_driver goodix_berlin_i2c_driver = {
> + .driver = {
> + .name = "goodix-berlin-i2c",
> + .of_match_table = goodix_berlin_i2c_of_match,
> + .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
> + },
> + .probe = goodix_berlin_i2c_probe,
> + .remove = goodix_berlin_i2c_remove,
> + .id_table = goodix_berlin_i2c_id,
> +};
> +module_i2c_driver(goodix_berlin_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Goodix Berlin I2C Touchscreen driver");
> +MODULE_AUTHOR("Neil Armstrong <[email protected]>");
>
> --
> 2.34.1
>

Kind regards,
Jeff LaBundy