[PATCH 0/7] Input: goodix - Add support for controllers without flash

[PATCH 0/7] Input: goodix - Add support for controllers without flash

[Hans de Goede]

Hi All,

A while ago I bought a Glavey TM800A550L tablet. This is a Bay Trail (x86)
tablet which comes with Android from the factory.

It is mostly just another Bay Trail tablet, but the way the Goodix
touchscreen on it works is interesting. The controller needs to have
firmware uploaded to it before it will work. After that it pretty much
works 100% the same as any other Goodix touchscreen controller.

I've described this patch-set in the Subject as "Add support for
controllers without flash", but I suspect that the Goodix controllers
used on Bay Trail devices which ship with Windows pre-installed may also
lack flash; and that the Windows version of the BIOS-es on these devices
does the upload for us. Anyways just something which I'm wondering about,
these patches are necessary to make things work regardless.

These patches should also be useful for supporting the Goodix controllers
on some ARM based devices, as some of those need the OS to upload the
firmware too AFAIK.

Note patch 7/7 is included to get an overview of the entire set.
I'll merge that one myself through the pdx86 tree. Patches 1-6 are
intended for merging through the input tree.

Regards,

Hans


Hans de Goede (7):
  Input: goodix - Change goodix_i2c_write() len parameter type to int
  Input: goodix - Add a goodix.h header file
  Input: goodix - Refactor reset handling
  Input: goodix - Push error logging up into i2c_read and i2c_write
    helpers
  Input: goodix - Allow specifying the config filename through a
    "goodix,config-name" device-property
  Input: goodix - Add support for controllers without flash
  platform/x86: touchscreen_dmi: Add info for the Goodix GT912 panel of
    TM800A550L tablets

 MAINTAINERS                                 |   3 +-
 drivers/input/touchscreen/Makefile          |   3 +-
 drivers/input/touchscreen/goodix.c          | 239 +++++------
 drivers/input/touchscreen/goodix.h          | 118 ++++++
 drivers/input/touchscreen/goodix_fwupload.c | 437 ++++++++++++++++++++
 drivers/platform/x86/touchscreen_dmi.c      |  21 +
 6 files changed, 684 insertions(+), 137 deletions(-)
 create mode 100644 drivers/input/touchscreen/goodix.h
 create mode 100644 drivers/input/touchscreen/goodix_fwupload.c

-- 
2.31.1

[PATCH 1/7] Input: goodix - Change goodix_i2c_write() len parameter type to int

[Hans de Goede]

Change the type of the goodix_i2c_write() len parameter to from 'unsigned'
to 'int' to avoid bare use of 'unsigned', changing it to 'int' makes
goodix_i2c_write()' prototype consistent with goodix_i2c_read().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index c682b028f0a2..fddbc6c5127d 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -289,8 +289,7 @@ static int goodix_i2c_read(struct i2c_client *client,
  * @buf: raw data buffer to write.
  * @len: length of the buffer to write
  */
-static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf,
-			    unsigned len)
+static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len)
 {
 	u8 *addr_buf;
 	struct i2c_msg msg;
-- 
2.31.1

[PATCH 2/7] Input: goodix - Add a goodix.h header file

[Hans de Goede]

Add a goodix.h header file, and move the register definitions,
and struct declarations there and add prototypes for various
helper functions.

This is a preparation patch for adding support for controllers
without flash, which need to have their firmware uploaded and
need some other special handling too.

Since MAINTAINERS needs updating because of this change anyways,
also add myself as co-maintainer.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 MAINTAINERS                        |  3 +-
 drivers/input/touchscreen/goodix.c | 73 +++---------------------------
 drivers/input/touchscreen/goodix.h | 73 ++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 68 deletions(-)
 create mode 100644 drivers/input/touchscreen/goodix.h

diff --git a/MAINTAINERS b/MAINTAINERS
index cec5f5ae51e9..446aeb4e4d3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7593,9 +7593,10 @@ F:	drivers/media/usb/go7007/
 
 GOODIX TOUCHSCREEN
 M:	Bastien Nocera <hadess@hadess.net>
+M:	Hans de Goede <hdegoede@redhat.com>
 L:	linux-input@vger.kernel.org
 S:	Maintained
-F:	drivers/input/touchscreen/goodix.c
+F:	drivers/input/touchscreen/goodix*
 
 GOOGLE ETHERNET DRIVERS
 M:	Catherine Sullivan <csully@google.com>
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index fddbc6c5127d..76e93bc8389a 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -14,20 +14,15 @@
 #include <linux/kernel.h>
 #include <linux/dmi.h>
 #include <linux/firmware.h>
-#include <linux/gpio/consumer.h>
-#include <linux/i2c.h>
-#include <linux/input.h>
-#include <linux/input/mt.h>
-#include <linux/input/touchscreen.h>
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
-#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/of.h>
 #include <asm/unaligned.h>
+#include "goodix.h"
 
 #define GOODIX_GPIO_INT_NAME		"irq"
 #define GOODIX_GPIO_RST_NAME		"reset"
@@ -38,22 +33,11 @@
 #define GOODIX_CONTACT_SIZE		8
 #define GOODIX_MAX_CONTACT_SIZE		9
 #define GOODIX_MAX_CONTACTS		10
-#define GOODIX_MAX_KEYS			7
 
 #define GOODIX_CONFIG_MIN_LENGTH	186
 #define GOODIX_CONFIG_911_LENGTH	186
 #define GOODIX_CONFIG_967_LENGTH	228
 #define GOODIX_CONFIG_GT9X_LENGTH	240
-#define GOODIX_CONFIG_MAX_LENGTH	240
-
-/* Register defines */
-#define GOODIX_REG_COMMAND		0x8040
-#define GOODIX_CMD_SCREEN_OFF		0x05
-
-#define GOODIX_READ_COOR_ADDR		0x814E
-#define GOODIX_GT1X_REG_CONFIG_DATA	0x8050
-#define GOODIX_GT9X_REG_CONFIG_DATA	0x8047
-#define GOODIX_REG_ID			0x8140
 
 #define GOODIX_BUFFER_STATUS_READY	BIT(7)
 #define GOODIX_HAVE_KEY			BIT(4)
@@ -68,55 +52,11 @@
 #define ACPI_GPIO_SUPPORT
 #endif
 
-struct goodix_ts_data;
-
-enum goodix_irq_pin_access_method {
-	IRQ_PIN_ACCESS_NONE,
-	IRQ_PIN_ACCESS_GPIO,
-	IRQ_PIN_ACCESS_ACPI_GPIO,
-	IRQ_PIN_ACCESS_ACPI_METHOD,
-};
-
-struct goodix_chip_data {
-	u16 config_addr;
-	int config_len;
-	int (*check_config)(struct goodix_ts_data *ts, const u8 *cfg, int len);
-	void (*calc_config_checksum)(struct goodix_ts_data *ts);
-};
-
 struct goodix_chip_id {
 	const char *id;
 	const struct goodix_chip_data *data;
 };
 
-#define GOODIX_ID_MAX_LEN	4
-
-struct goodix_ts_data {
-	struct i2c_client *client;
-	struct input_dev *input_dev;
-	const struct goodix_chip_data *chip;
-	struct touchscreen_properties prop;
-	unsigned int max_touch_num;
-	unsigned int int_trigger_type;
-	struct regulator *avdd28;
-	struct regulator *vddio;
-	struct gpio_desc *gpiod_int;
-	struct gpio_desc *gpiod_rst;
-	int gpio_count;
-	int gpio_int_idx;
-	char id[GOODIX_ID_MAX_LEN + 1];
-	u16 version;
-	const char *cfg_name;
-	bool reset_controller_at_probe;
-	bool load_cfg_from_disk;
-	struct completion firmware_loading_complete;
-	unsigned long irq_flags;
-	enum goodix_irq_pin_access_method irq_pin_access_method;
-	unsigned int contact_size;
-	u8 config[GOODIX_CONFIG_MAX_LENGTH];
-	unsigned short keymap[GOODIX_MAX_KEYS];
-};
-
 static int goodix_check_cfg_8(struct goodix_ts_data *ts,
 			      const u8 *cfg, int len);
 static int goodix_check_cfg_16(struct goodix_ts_data *ts,
@@ -260,8 +200,7 @@ static const struct dmi_system_id inverted_x_screen[] = {
  * @buf: raw write data buffer.
  * @len: length of the buffer to write
  */
-static int goodix_i2c_read(struct i2c_client *client,
-			   u16 reg, u8 *buf, int len)
+int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len)
 {
 	struct i2c_msg msgs[2];
 	__be16 wbuf = cpu_to_be16(reg);
@@ -289,7 +228,7 @@ static int goodix_i2c_read(struct i2c_client *client,
  * @buf: raw data buffer to write.
  * @len: length of the buffer to write
  */
-static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len)
+int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len)
 {
 	u8 *addr_buf;
 	struct i2c_msg msg;
@@ -313,7 +252,7 @@ static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, i
 	return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
 }
 
-static int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
+int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
 {
 	return goodix_i2c_write(client, reg, &value, sizeof(value));
 }
@@ -597,7 +536,7 @@ static int goodix_check_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len)
  * @cfg: config firmware to write to device
  * @len: config data length
  */
-static int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len)
+int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len)
 {
 	int error;
 
@@ -695,7 +634,7 @@ static int goodix_irq_direction_input(struct goodix_ts_data *ts)
 	return -EINVAL; /* Never reached */
 }
 
-static int goodix_int_sync(struct goodix_ts_data *ts)
+int goodix_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
 
diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h
new file mode 100644
index 000000000000..cdaced4f2980
--- /dev/null
+++ b/drivers/input/touchscreen/goodix.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __GOODIX_H__
+#define __GOODIX_H__
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regulator/consumer.h>
+
+/* Register defines */
+#define GOODIX_REG_COMMAND			0x8040
+#define GOODIX_CMD_SCREEN_OFF			0x05
+
+#define GOODIX_GT1X_REG_CONFIG_DATA		0x8050
+#define GOODIX_GT9X_REG_CONFIG_DATA		0x8047
+#define GOODIX_REG_ID				0x8140
+#define GOODIX_READ_COOR_ADDR			0x814E
+
+#define GOODIX_ID_MAX_LEN			4
+#define GOODIX_CONFIG_MAX_LENGTH		240
+#define GOODIX_MAX_KEYS				7
+
+enum goodix_irq_pin_access_method {
+	IRQ_PIN_ACCESS_NONE,
+	IRQ_PIN_ACCESS_GPIO,
+	IRQ_PIN_ACCESS_ACPI_GPIO,
+	IRQ_PIN_ACCESS_ACPI_METHOD,
+};
+
+struct goodix_ts_data;
+
+struct goodix_chip_data {
+	u16 config_addr;
+	int config_len;
+	int (*check_config)(struct goodix_ts_data *ts, const u8 *cfg, int len);
+	void (*calc_config_checksum)(struct goodix_ts_data *ts);
+};
+
+struct goodix_ts_data {
+	struct i2c_client *client;
+	struct input_dev *input_dev;
+	const struct goodix_chip_data *chip;
+	struct touchscreen_properties prop;
+	unsigned int max_touch_num;
+	unsigned int int_trigger_type;
+	struct regulator *avdd28;
+	struct regulator *vddio;
+	struct gpio_desc *gpiod_int;
+	struct gpio_desc *gpiod_rst;
+	int gpio_count;
+	int gpio_int_idx;
+	char id[GOODIX_ID_MAX_LEN + 1];
+	u16 version;
+	const char *cfg_name;
+	bool reset_controller_at_probe;
+	bool load_cfg_from_disk;
+	struct completion firmware_loading_complete;
+	unsigned long irq_flags;
+	enum goodix_irq_pin_access_method irq_pin_access_method;
+	unsigned int contact_size;
+	u8 config[GOODIX_CONFIG_MAX_LENGTH];
+	unsigned short keymap[GOODIX_MAX_KEYS];
+};
+
+int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len);
+int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len);
+int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value);
+int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len);
+int goodix_int_sync(struct goodix_ts_data *ts);
+
+#endif
-- 
2.31.1

[PATCH 3/7] Input: goodix - Refactor reset handling

[Hans de Goede]

Refactor reset handling a bit, change the main reset handler
into a new goodix_reset_no_int_sync() helper and add a
goodix_reset() wrapper which calls goodix_int_sync()
separately.

Also push the dev_err() call on reset failure into the
goodix_reset_no_int_sync() and goodix_int_sync() functions,
so that we don't need to have separate dev_err() calls in
all their callers.

This is a preparation patch for adding support for controllers
without flash, which need to have their firmware uploaded and
need some other special handling too.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 46 +++++++++++++++++++-----------
 drivers/input/touchscreen/goodix.h |  1 +
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 76e93bc8389a..f0fe8960e3df 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -640,56 +640,74 @@ int goodix_int_sync(struct goodix_ts_data *ts)
 
 	error = goodix_irq_direction_output(ts, 0);
 	if (error)
-		return error;
+		goto error;
 
 	msleep(50);				/* T5: 50ms */
 
 	error = goodix_irq_direction_input(ts);
 	if (error)
-		return error;
+		goto error;
 
 	return 0;
+error:
+	dev_err(&ts->client->dev, "Controller irq sync failed.\n");
+	return error;
 }
 
 /**
- * goodix_reset - Reset device during power on
+ * goodix_reset_no_int_sync - Reset device, leaving interrupt line in output mode
  *
  * @ts: goodix_ts_data pointer
  */
-static int goodix_reset(struct goodix_ts_data *ts)
+int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
 
 	/* begin select I2C slave addr */
 	error = gpiod_direction_output(ts->gpiod_rst, 0);
 	if (error)
-		return error;
+		goto error;
 
 	msleep(20);				/* T2: > 10ms */
 
 	/* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */
 	error = goodix_irq_direction_output(ts, ts->client->addr == 0x14);
 	if (error)
-		return error;
+		goto error;
 
 	usleep_range(100, 2000);		/* T3: > 100us */
 
 	error = gpiod_direction_output(ts->gpiod_rst, 1);
 	if (error)
-		return error;
+		goto error;
 
 	usleep_range(6000, 10000);		/* T4: > 5ms */
 
 	/* end select I2C slave addr */
 	error = gpiod_direction_input(ts->gpiod_rst);
 	if (error)
-		return error;
+		goto error;
 
-	error = goodix_int_sync(ts);
+	return 0;
+error:
+	dev_err(&ts->client->dev, "Controller reset failed.\n");
+	return error;
+}
+
+/**
+ * goodix_reset - Reset device during power on
+ *
+ * @ts: goodix_ts_data pointer
+ */
+static int goodix_reset(struct goodix_ts_data *ts)
+{
+	int error;
+
+	error = goodix_reset_no_int_sync(ts);
 	if (error)
 		return error;
 
-	return 0;
+	return  goodix_int_sync(ts);
 }
 
 #ifdef ACPI_GPIO_SUPPORT
@@ -1195,10 +1213,8 @@ static int goodix_ts_probe(struct i2c_client *client,
 	if (ts->reset_controller_at_probe) {
 		/* reset the controller */
 		error = goodix_reset(ts);
-		if (error) {
-			dev_err(&client->dev, "Controller reset failed.\n");
+		if (error)
 			return error;
-		}
 	}
 
 	error = goodix_i2c_test(client);
@@ -1340,10 +1356,8 @@ static int __maybe_unused goodix_resume(struct device *dev)
 
 	if (error != 0 || config_ver != ts->config[0]) {
 		error = goodix_reset(ts);
-		if (error) {
-			dev_err(dev, "Controller reset failed.\n");
+		if (error)
 			return error;
-		}
 
 		error = goodix_send_cfg(ts, ts->config, ts->chip->config_len);
 		if (error)
diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h
index cdaced4f2980..0b88554ba2ae 100644
--- a/drivers/input/touchscreen/goodix.h
+++ b/drivers/input/touchscreen/goodix.h
@@ -69,5 +69,6 @@ int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len)
 int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value);
 int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len);
 int goodix_int_sync(struct goodix_ts_data *ts);
+int goodix_reset_no_int_sync(struct goodix_ts_data *ts);
 
 #endif
-- 
2.31.1

[PATCH 4/7] Input: goodix - Push error logging up into i2c_read and i2c_write helpers

[Hans de Goede]

Make the goodix_i2c_read() and goodix_i2c_write*() helpers log errors
themselves. This allows removing all the error logging from their callers.

This already results in a nice cleanup with the current code and it also
helps to make the upcoming support for controllers without flash cleaner.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 68 ++++++++++++------------------
 1 file changed, 28 insertions(+), 40 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f0fe8960e3df..ce3050e45a48 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -217,7 +217,13 @@ int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len)
 	msgs[1].buf   = buf;
 
 	ret = i2c_transfer(client->adapter, msgs, 2);
-	return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
+	if (ret >= 0)
+		ret = (ret == ARRAY_SIZE(msgs) ? 0 : -EIO);
+
+	if (ret)
+		dev_err(&client->dev, "Error reading %d bytes from 0x%04x: %d\n",
+			len, reg, ret);
+	return ret;
 }
 
 /**
@@ -248,8 +254,15 @@ int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len)
 	msg.len = len + 2;
 
 	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret >= 0)
+		ret = (ret == 1 ? 0 : -EIO);
+
 	kfree(addr_buf);
-	return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
+
+	if (ret)
+		dev_err(&client->dev, "Error writing %d bytes to 0x%04x: %d\n",
+			len, reg, ret);
+	return ret;
 }
 
 int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
@@ -289,13 +302,9 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 	 */
 	max_timeout = jiffies + msecs_to_jiffies(GOODIX_BUFFER_STATUS_TIMEOUT);
 	do {
-		error = goodix_i2c_read(ts->client, addr, data,
-					header_contact_keycode_size);
-		if (error) {
-			dev_err(&ts->client->dev, "I2C transfer error: %d\n",
-					error);
+		error = goodix_i2c_read(ts->client, addr, data, header_contact_keycode_size);
+		if (error)
 			return error;
-		}
 
 		if (data[0] & GOODIX_BUFFER_STATUS_READY) {
 			touch_num = data[0] & 0x0f;
@@ -305,10 +314,8 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 			if (touch_num > 1) {
 				addr += header_contact_keycode_size;
 				data += header_contact_keycode_size;
-				error = goodix_i2c_read(ts->client,
-						addr, data,
-						ts->contact_size *
-							(touch_num - 1));
+				error = goodix_i2c_read(ts->client, addr, data,
+							ts->contact_size * (touch_num - 1));
 				if (error)
 					return error;
 			}
@@ -418,9 +425,7 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
 	struct goodix_ts_data *ts = dev_id;
 
 	goodix_process_events(ts);
-
-	if (goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0) < 0)
-		dev_err(&ts->client->dev, "I2C write end_cmd error\n");
+	goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0);
 
 	return IRQ_HANDLED;
 }
@@ -545,11 +550,9 @@ int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len)
 		return error;
 
 	error = goodix_i2c_write(ts->client, ts->chip->config_addr, cfg, len);
-	if (error) {
-		dev_err(&ts->client->dev, "Failed to write config data: %d",
-			error);
+	if (error)
 		return error;
-	}
+
 	dev_dbg(&ts->client->dev, "Config sent successfully.");
 
 	/* Let the firmware reconfigure itself, so sleep for 10ms */
@@ -935,8 +938,6 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 	error = goodix_i2c_read(ts->client, ts->chip->config_addr,
 				ts->config, ts->chip->config_len);
 	if (error) {
-		dev_warn(&ts->client->dev, "Error reading config: %d\n",
-			 error);
 		ts->int_trigger_type = GOODIX_INT_TRIGGER;
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 		return;
@@ -967,10 +968,8 @@ static int goodix_read_version(struct goodix_ts_data *ts)
 	char id_str[GOODIX_ID_MAX_LEN + 1];
 
 	error = goodix_i2c_read(ts->client, GOODIX_REG_ID, buf, sizeof(buf));
-	if (error) {
-		dev_err(&ts->client->dev, "read version failed: %d\n", error);
+	if (error)
 		return error;
-	}
 
 	memcpy(id_str, buf, GOODIX_ID_MAX_LEN);
 	id_str[GOODIX_ID_MAX_LEN] = 0;
@@ -996,13 +995,10 @@ static int goodix_i2c_test(struct i2c_client *client)
 	u8 test;
 
 	while (retry++ < 2) {
-		error = goodix_i2c_read(client, GOODIX_REG_ID,
-					&test, 1);
+		error = goodix_i2c_read(client, GOODIX_REG_ID, &test, 1);
 		if (!error)
 			return 0;
 
-		dev_err(&client->dev, "i2c test failed attempt %d: %d\n",
-			retry, error);
 		msleep(20);
 	}
 
@@ -1230,10 +1226,8 @@ static int goodix_ts_probe(struct i2c_client *client,
 	}
 
 	error = goodix_read_version(ts);
-	if (error) {
-		dev_err(&client->dev, "Read version failed.\n");
+	if (error)
 		return error;
-	}
 
 	ts->chip = goodix_get_chip_data(ts->id);
 
@@ -1301,10 +1295,8 @@ static int __maybe_unused goodix_suspend(struct device *dev)
 
 	usleep_range(5000, 6000);
 
-	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_COMMAND,
-				    GOODIX_CMD_SCREEN_OFF);
+	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_COMMAND, GOODIX_CMD_SCREEN_OFF);
 	if (error) {
-		dev_err(&ts->client->dev, "Screen off command failed\n");
 		goodix_irq_direction_input(ts);
 		goodix_request_irq(ts);
 		return -EAGAIN;
@@ -1345,12 +1337,8 @@ static int __maybe_unused goodix_resume(struct device *dev)
 	if (error)
 		return error;
 
-	error = goodix_i2c_read(ts->client, ts->chip->config_addr,
-				&config_ver, 1);
-	if (error)
-		dev_warn(dev, "Error reading config version: %d, resetting controller\n",
-			 error);
-	else if (config_ver != ts->config[0])
+	error = goodix_i2c_read(ts->client, ts->chip->config_addr, &config_ver, 1);
+	if (!error && config_ver != ts->config[0])
 		dev_info(dev, "Config version mismatch %d != %d, resetting controller\n",
 			 config_ver, ts->config[0]);
 
-- 
2.31.1

[PATCH 5/7] Input: goodix - Allow specifying the config filename through a "goodix,config-name" device-property

[Hans de Goede]

The config which needs to be send to the controller on some device-models
is model-specific. Allow specifying a model-specific filename through
a device-property, rather then always using a fixed filename.

Note the "goodix,config-name" device-property used by this is
*not* documented in the
Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
device-tree bindings for now.

Not documenting these is intentional. This is done because this code has
only been tested on x86/ACPI so far, where devicetree is not used.
Instead these properties are set through a software-fwnode attached to the
device by the drivers/platform/x86/touchscreen_dmi.c code. This means that
the use of this property for now is purely a kernel-internal thing and
the name/working of the property may still be changed for now.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 10 ++++++----
 drivers/input/touchscreen/goodix.h |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index ce3050e45a48..14d16a2a111d 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -1160,6 +1160,7 @@ static int goodix_ts_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)
 {
 	struct goodix_ts_data *ts;
+	const char *cfg_name;
 	int error;
 
 	dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
@@ -1233,10 +1234,11 @@ static int goodix_ts_probe(struct i2c_client *client,
 
 	if (ts->load_cfg_from_disk) {
 		/* update device config */
-		ts->cfg_name = devm_kasprintf(&client->dev, GFP_KERNEL,
-					      "goodix_%s_cfg.bin", ts->id);
-		if (!ts->cfg_name)
-			return -ENOMEM;
+		error = device_property_read_string(&client->dev, "goodix,config-name", &cfg_name);
+		if (!error)
+			snprintf(ts->cfg_name, sizeof(ts->cfg_name), "goodix/%s", cfg_name);
+		else
+			snprintf(ts->cfg_name, sizeof(ts->cfg_name), "goodix_%s_cfg.bin", ts->id);
 
 		error = request_firmware_nowait(THIS_MODULE, true, ts->cfg_name,
 						&client->dev, GFP_KERNEL, ts,
diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h
index 0b88554ba2ae..b1d1bc004091 100644
--- a/drivers/input/touchscreen/goodix.h
+++ b/drivers/input/touchscreen/goodix.h
@@ -52,8 +52,8 @@ struct goodix_ts_data {
 	int gpio_count;
 	int gpio_int_idx;
 	char id[GOODIX_ID_MAX_LEN + 1];
+	char cfg_name[64];
 	u16 version;
-	const char *cfg_name;
 	bool reset_controller_at_probe;
 	bool load_cfg_from_disk;
 	struct completion firmware_loading_complete;
-- 
2.31.1

[PATCH 6/7] Input: goodix - Add support for controllers without flash

[Hans de Goede]

Some Goodix touchscreen controllers, such as for example the GT912,
don't have flash-storage for their firmware.

These models require the OS to load the firmware at runtime, as well as
some other special handling. Add support for this to the goodix driver.

This patch was developed and tested on a Glavey TM800A550L tablet.

Note the "goodix,main-clk" and "firmware-name" device-properties used
by the new code are *not* documented in the
Documentation/devicetree/bindings/input/touchscreen/goodix.yaml
device-tree bindings for now.

Not documenting these is intentional. This is done because this code has
only been tested on x86/ACPI so far, where devicetree is not used.
Instead these properties are set through a software-fwnode attached to the
device by the drivers/platform/x86/touchscreen_dmi.c code. This means that
the use of this properties for now is purely a kernel-internal thing and
the name/working of the properties may still be changed for now.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/Makefile          |   3 +-
 drivers/input/touchscreen/goodix.c          |  41 +-
 drivers/input/touchscreen/goodix.h          |  44 ++
 drivers/input/touchscreen/goodix_fwupload.c | 437 ++++++++++++++++++++
 4 files changed, 517 insertions(+), 8 deletions(-)
 create mode 100644 drivers/input/touchscreen/goodix_fwupload.c

diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 6233541e9173..64511207b49d 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -6,6 +6,7 @@
 # Each configuration option enables a list of files.
 
 wm97xx-ts-y := wm97xx-core.o
+goodix_ts-y := goodix.o goodix_fwupload.o
 
 obj-$(CONFIG_TOUCHSCREEN_PROPERTIES)	+= of_touchscreen.o
 obj-$(CONFIG_TOUCHSCREEN_88PM860X)	+= 88pm860x-ts.o
@@ -44,7 +45,7 @@ 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_FUJITSU)	+= fujitsu_ts.o
-obj-$(CONFIG_TOUCHSCREEN_GOODIX)	+= goodix.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX)	+= goodix_ts.o
 obj-$(CONFIG_TOUCHSCREEN_HIDEEP)	+= hideep.o
 obj-$(CONFIG_TOUCHSCREEN_ILI210X)	+= ili210x.o
 obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC)	+= imx6ul_tsc.o
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 14d16a2a111d..e743709b55f0 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -323,6 +323,11 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 			return touch_num;
 		}
 
+		if (data[0] == 0 && ts->controller_has_no_flash) {
+			if (goodix_handle_fw_request(ts))
+				return 0;
+		}
+
 		usleep_range(1000, 2000); /* Poll every 1 - 2 ms */
 	} while (time_before(jiffies, max_timeout));
 
@@ -935,12 +940,18 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 	int x_max, y_max;
 	int error;
 
-	error = goodix_i2c_read(ts->client, ts->chip->config_addr,
-				ts->config, ts->chip->config_len);
-	if (error) {
-		ts->int_trigger_type = GOODIX_INT_TRIGGER;
-		ts->max_touch_num = GOODIX_MAX_CONTACTS;
-		return;
+	/*
+	 * When the controller has no flash ts->config already has the config at
+	 * this point and the controller itself does not have it yet!
+	 */
+	if (!ts->controller_has_no_flash) {
+		error = goodix_i2c_read(ts->client, ts->chip->config_addr,
+					ts->config, ts->chip->config_len);
+		if (error) {
+			ts->int_trigger_type = GOODIX_INT_TRIGGER;
+			ts->max_touch_num = GOODIX_MAX_CONTACTS;
+			return;
+		}
 	}
 
 	ts->int_trigger_type = ts->config[TRIGGER_LOC] & 0x03;
@@ -1134,7 +1145,16 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx)
 	struct goodix_ts_data *ts = ctx;
 	int error;
 
-	if (cfg) {
+	if (ts->controller_has_no_flash) {
+		if (!cfg)
+			goto err_release_cfg;
+
+		error = goodix_check_cfg(ts, cfg->data, cfg->size);
+		if (error)
+			goto err_release_cfg;
+
+		memcpy(ts->config, cfg->data, cfg->size);
+	} else if (cfg) {
 		/* send device configuration to the firmware */
 		error = goodix_send_cfg(ts, cfg->data, cfg->size);
 		if (error)
@@ -1226,6 +1246,10 @@ static int goodix_ts_probe(struct i2c_client *client,
 		return error;
 	}
 
+	error = goodix_firmware_check(ts);
+	if (error)
+		return error;
+
 	error = goodix_read_version(ts);
 	if (error)
 		return error;
@@ -1288,6 +1312,9 @@ static int __maybe_unused goodix_suspend(struct device *dev)
 	/* Free IRQ as IRQ pin is used as output in the suspend sequence */
 	goodix_free_irq(ts);
 
+	/* Save reference (calibration) info if necessary */
+	goodix_save_bak_ref(ts);
+
 	/* Output LOW on the INT pin for 5 ms */
 	error = goodix_irq_direction_output(ts, 0);
 	if (error) {
diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h
index b1d1bc004091..5d233f698df0 100644
--- a/drivers/input/touchscreen/goodix.h
+++ b/drivers/input/touchscreen/goodix.h
@@ -10,13 +10,48 @@
 #include <linux/regulator/consumer.h>
 
 /* Register defines */
+#define GOODIX_REG_MISCTL_DSP_CTL		0x4010
+#define GOODIX_REG_MISCTL_SRAM_BANK		0x4048
+#define GOODIX_REG_MISCTL_MEM_CD_EN		0x4049
+#define GOODIX_REG_MISCTL_CACHE_EN		0x404B
+#define GOODIX_REG_MISCTL_TMR0_EN		0x40B0
+#define GOODIX_REG_MISCTL_SWRST			0x4180
+#define GOODIX_REG_MISCTL_CPU_SWRST_PULSE	0x4184
+#define GOODIX_REG_MISCTL_BOOTCTL		0x4190
+#define GOODIX_REG_MISCTL_BOOT_OPT		0x4218
+#define GOODIX_REG_MISCTL_BOOT_CTL		0x5094
+
+#define GOODIX_REG_FW_SIG			0x8000
+#define GOODIX_FW_SIG_LEN			10
+
+#define GOODIX_REG_MAIN_CLK			0x8020
+#define GOODIX_MAIN_CLK_LEN			6
+
 #define GOODIX_REG_COMMAND			0x8040
 #define GOODIX_CMD_SCREEN_OFF			0x05
 
+#define GOODIX_REG_SW_WDT			0x8041
+
+#define GOODIX_REG_REQUEST			0x8043
+#define GOODIX_RQST_RESPONDED			0x00
+#define GOODIX_RQST_CONFIG			0x01
+#define GOODIX_RQST_BAK_REF			0x02
+#define GOODIX_RQST_RESET			0x03
+#define GOODIX_RQST_MAIN_CLOCK			0x04
+/*
+ * Unknown request which gets send by the controller aprox.
+ * every 34 seconds once it is up and running.
+ */
+#define GOODIX_RQST_UNKNOWN			0x06
+#define GOODIX_RQST_IDLE			0xFF
+
+#define GOODIX_REG_STATUS			0x8044
+
 #define GOODIX_GT1X_REG_CONFIG_DATA		0x8050
 #define GOODIX_GT9X_REG_CONFIG_DATA		0x8047
 #define GOODIX_REG_ID				0x8140
 #define GOODIX_READ_COOR_ADDR			0x814E
+#define GOODIX_REG_BAK_REF			0x99D0
 
 #define GOODIX_ID_MAX_LEN			4
 #define GOODIX_CONFIG_MAX_LENGTH		240
@@ -53,15 +88,20 @@ struct goodix_ts_data {
 	int gpio_int_idx;
 	char id[GOODIX_ID_MAX_LEN + 1];
 	char cfg_name[64];
+	char fw_name[64];
 	u16 version;
 	bool reset_controller_at_probe;
 	bool load_cfg_from_disk;
+	bool controller_has_no_flash;
 	struct completion firmware_loading_complete;
 	unsigned long irq_flags;
 	enum goodix_irq_pin_access_method irq_pin_access_method;
 	unsigned int contact_size;
 	u8 config[GOODIX_CONFIG_MAX_LENGTH];
 	unsigned short keymap[GOODIX_MAX_KEYS];
+	u8 main_clk[GOODIX_MAIN_CLK_LEN];
+	int bak_ref_len;
+	u8 *bak_ref;
 };
 
 int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len);
@@ -71,4 +111,8 @@ int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len);
 int goodix_int_sync(struct goodix_ts_data *ts);
 int goodix_reset_no_int_sync(struct goodix_ts_data *ts);
 
+int goodix_firmware_check(struct goodix_ts_data *ts);
+bool goodix_handle_fw_request(struct goodix_ts_data *ts);
+void goodix_save_bak_ref(struct goodix_ts_data *ts);
+
 #endif
diff --git a/drivers/input/touchscreen/goodix_fwupload.c b/drivers/input/touchscreen/goodix_fwupload.c
new file mode 100644
index 000000000000..d44946eefa65
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_fwupload.c
@@ -0,0 +1,437 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Goodix Touchscreen firmware upload support
+ *
+ * Copyright (c) 2021 Hans de Goede <hdegoede@redhat.com>
+ *
+ * This is a rewrite of gt9xx_update.c from the Allwinner H3 BSP which is:
+ * Copyright (c) 2010 - 2012 Goodix Technology.
+ * Author: andrew@goodix.com
+ */
+
+#include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include "goodix.h"
+
+#define GOODIX_FW_HEADER_LENGTH		sizeof(struct goodix_fw_header)
+#define GOODIX_FW_SECTION_LENGTH	0x2000
+#define GOODIX_FW_DSP_LENGTH		0x1000
+#define GOODIX_FW_UPLOAD_ADDRESS	0xc000
+
+#define GOODIX_CFG_LOC_HAVE_KEY		 7
+#define GOODIX_CFG_LOC_DRVA_NUM		27
+#define GOODIX_CFG_LOC_DRVB_NUM		28
+#define GOODIX_CFG_LOC_SENS_NUM		29
+
+struct goodix_fw_header {
+	u8 hw_info[4];
+	u8 pid[8];
+	u8 vid[2];
+} __packed;
+
+static int goodix_check_controller_type(struct goodix_ts_data *ts)
+{
+	u8 buf[GOODIX_FW_SIG_LEN + 1] = { 0, };
+	int error;
+
+	error = goodix_i2c_read(ts->client, GOODIX_REG_FW_SIG, buf, GOODIX_FW_SIG_LEN);
+	if (error)
+		return error;
+
+	/*
+	 * Devices with ROM have a fw-signature of GOODIX_GT9, devices without
+	 * ROM have a fw-signature of GT900_FLAS after upload to SRAM and
+	 * contain random data before the first upload.
+	 */
+	ts->controller_has_no_flash = memcmp(buf, "GOODIX", 6) != 0;
+	return 0;
+}
+
+static u16 goodix_firmware_checksum(const u8 *data, int size)
+{
+	u16 checksum = 0;
+	int i;
+
+	for (i = 0; i < size; i += 2)
+		checksum += (data[i] << 8) + data[i + 1];
+
+	return checksum;
+}
+
+static int goodix_firmware_verify(struct device *dev, const struct firmware *fw)
+{
+	const struct goodix_fw_header *fw_header;
+	size_t expected_size;
+	const u8 *data;
+	u16 checksum;
+	char buf[9];
+
+	expected_size = GOODIX_FW_HEADER_LENGTH + 4 * GOODIX_FW_SECTION_LENGTH +
+			GOODIX_FW_DSP_LENGTH;
+	if (fw->size != expected_size) {
+		dev_err(dev, "Firmware has wrong size, expected %zu got %zu\n",
+			expected_size, fw->size);
+		return -EINVAL;
+	}
+
+	data = fw->data + GOODIX_FW_HEADER_LENGTH;
+	checksum = goodix_firmware_checksum(data, 4 * GOODIX_FW_SECTION_LENGTH);
+	if (checksum) {
+		dev_err(dev, "Main firmware checksum error\n");
+		return -EINVAL;
+	}
+
+	data += 4 * GOODIX_FW_SECTION_LENGTH;
+	checksum = goodix_firmware_checksum(data, GOODIX_FW_DSP_LENGTH);
+	if (checksum) {
+		dev_err(dev, "DSP firmware checksum error\n");
+		return -EINVAL;
+	}
+
+	fw_header = (const struct goodix_fw_header *)fw->data;
+	dev_info(dev, "Firmware hardware info %02x%02x%02x%02x\n",
+		 fw_header->hw_info[0], fw_header->hw_info[1],
+		 fw_header->hw_info[2], fw_header->hw_info[3]);
+	/* pid is a 8 byte buffer containing a string, weird I know */
+	memcpy(buf, fw_header->pid, 8);
+	buf[8] = 0;
+	dev_info(dev, "Firmware PID: %s VID: %02x%02x\n", buf,
+		 fw_header->vid[0], fw_header->vid[1]);
+	return 0;
+}
+
+static int goodix_enter_upload_mode(struct i2c_client *client)
+{
+	int tries, error;
+	u8 val;
+
+	tries = 200;
+	do {
+		error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_SWRST, 0x0c);
+		if (error)
+			return error;
+
+		error = goodix_i2c_read(client, GOODIX_REG_MISCTL_SWRST, &val, 1);
+		if (error)
+			return error;
+
+		if (val == 0x0c)
+			break;
+	} while (--tries);
+
+	if (!tries) {
+		dev_err(&client->dev, "Error could not hold ss51 & dsp\n");
+		return -EIO;
+	}
+
+	/* DSP_CK and DSP_ALU_CK PowerOn */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_DSP_CTL, 0x00);
+	if (error)
+		return error;
+
+	/* Disable watchdog */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_TMR0_EN, 0x00);
+	if (error)
+		return error;
+
+	/* Clear cache enable */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_CACHE_EN, 0x00);
+	if (error)
+		return error;
+
+	/* Set boot from SRAM */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_BOOTCTL, 0x02);
+	if (error)
+		return error;
+
+	/* Software reboot */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_CPU_SWRST_PULSE, 0x01);
+	if (error)
+		return error;
+
+	/* Clear control flag */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_BOOTCTL, 0x00);
+	if (error)
+		return error;
+
+	/* Set scramble */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_BOOT_OPT, 0x00);
+	if (error)
+		return error;
+
+	/* Enable accessing code */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_MEM_CD_EN, 0x01);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static int goodix_start_firmware(struct i2c_client *client)
+{
+	int error;
+	u8 val;
+
+	/* Init software watchdog */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_SW_WDT, 0xaa);
+	if (error)
+		return error;
+
+	/* Release SS51 & DSP */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_MISCTL_SWRST, 0x00);
+	if (error)
+		return error;
+
+	error = goodix_i2c_read(client, GOODIX_REG_SW_WDT, &val, 1);
+	if (error)
+		return error;
+
+	/* The value we've written to SW_WDT should have been cleared now */
+	if (val == 0xaa) {
+		dev_err(&client->dev, "Error SW_WDT reg not cleared on fw startup\n");
+		return -EIO;
+	}
+
+	/* Re-init software watchdog */
+	error = goodix_i2c_write_u8(client, GOODIX_REG_SW_WDT, 0xaa);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static int goodix_firmware_upload(struct goodix_ts_data *ts)
+{
+	const char *firmware_name = "gt9xx.fw";
+	const struct firmware *fw;
+	const u8 *data;
+	int error;
+
+	device_property_read_string(&ts->client->dev, "firmware-name", &firmware_name);
+	snprintf(ts->fw_name, sizeof(ts->fw_name), "goodix/%s", firmware_name);
+
+	error = request_firmware(&fw, ts->fw_name, &ts->client->dev);
+	if (error) {
+		dev_err(&ts->client->dev, "Firmware request error %d\n", error);
+		return error;
+	}
+
+	error = goodix_firmware_verify(&ts->client->dev, fw);
+	if (error)
+		goto release;
+
+	error = goodix_reset_no_int_sync(ts);
+	if (error)
+		return error;
+
+	error = goodix_enter_upload_mode(ts->client);
+	if (error)
+		goto release;
+
+	/* Select SRAM bank 0 and upload section 1 & 2 */
+	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_MISCTL_SRAM_BANK, 0x00);
+	if (error)
+		goto release;
+
+	data = fw->data + GOODIX_FW_HEADER_LENGTH;
+	error = goodix_i2c_write(ts->client, GOODIX_FW_UPLOAD_ADDRESS,
+				 data, 2 * GOODIX_FW_SECTION_LENGTH);
+	if (error)
+		goto release;
+
+	/* Select SRAM bank 1 and upload section 3 & 4 */
+	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_MISCTL_SRAM_BANK, 0x01);
+	if (error)
+		goto release;
+
+	data += 2 * GOODIX_FW_SECTION_LENGTH;
+	error = goodix_i2c_write(ts->client, GOODIX_FW_UPLOAD_ADDRESS,
+				 data, 2 * GOODIX_FW_SECTION_LENGTH);
+	if (error)
+		goto release;
+
+	/* Select SRAM bank 2 and upload the DSP firmware */
+	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_MISCTL_SRAM_BANK, 0x02);
+	if (error)
+		goto release;
+
+	data += 2 * GOODIX_FW_SECTION_LENGTH;
+	error = goodix_i2c_write(ts->client, GOODIX_FW_UPLOAD_ADDRESS,
+				 data, GOODIX_FW_DSP_LENGTH);
+	if (error)
+		goto release;
+
+	error = goodix_start_firmware(ts->client);
+	if (error)
+		goto release;
+
+	error = goodix_int_sync(ts);
+release:
+	release_firmware(fw);
+	return error;
+}
+
+static int goodix_prepare_bak_ref(struct goodix_ts_data *ts)
+{
+	u8 have_key, driver_num, sensor_num;
+
+	if (ts->bak_ref)
+		return 0; /* Already done */
+
+	have_key = (ts->config[GOODIX_CFG_LOC_HAVE_KEY] & 0x01);
+
+	driver_num = (ts->config[GOODIX_CFG_LOC_DRVA_NUM] & 0x1f) +
+		     (ts->config[GOODIX_CFG_LOC_DRVB_NUM] & 0x1f);
+	if (have_key)
+		driver_num--;
+
+	sensor_num = (ts->config[GOODIX_CFG_LOC_SENS_NUM] & 0x0f) +
+		     ((ts->config[GOODIX_CFG_LOC_SENS_NUM] >> 4) & 0x0f);
+
+	dev_dbg(&ts->client->dev, "Drv %d Sen %d Key %d\n", driver_num, sensor_num, have_key);
+
+	ts->bak_ref_len = (driver_num * (sensor_num - 2) + 2) * 2;
+
+	ts->bak_ref = devm_kzalloc(&ts->client->dev, ts->bak_ref_len, GFP_KERNEL);
+	if (!ts->bak_ref)
+		return -ENOMEM;
+
+	/*
+	 * The bak_ref array contains the backup of an array of (self/auto)
+	 * calibration related values which the Android version of the driver
+	 * stores on the filesystem so that it can be restored after reboot.
+	 * The mainline kernel never writes directly to the filesystem like
+	 * this, we always start will all the values which give a correction
+	 * factor in approx. the -20 - +20 range (in 2s complement) set to 0.
+	 *
+	 * Note the touchscreen works fine without restoring the reference
+	 * values after a reboot / power-cycle.
+	 *
+	 * The last 2 bytes are a 16 bits unsigned checksum which is expected
+	 * to make the addition al all 16 bit unsigned values in the array add
+	 * up to 1 (rather then the usual 0), so we must set the last byte to 1.
+	 */
+	ts->bak_ref[ts->bak_ref_len - 1] = 1;
+
+	return 0;
+}
+
+static int goodix_send_main_clock(struct goodix_ts_data *ts)
+{
+	u32 main_clk = 54; /* Default main clock */
+	u8 checksum = 0;
+	int i;
+
+	device_property_read_u32(&ts->client->dev, "goodix,main-clk", &main_clk);
+
+	for (i = 0; i < (GOODIX_MAIN_CLK_LEN - 1); i++) {
+		ts->main_clk[i] = main_clk;
+		checksum += main_clk;
+	}
+
+	/* The value of all bytes combines must be 0 */
+	ts->main_clk[GOODIX_MAIN_CLK_LEN - 1] = 256 - checksum;
+
+	return goodix_i2c_write(ts->client, GOODIX_REG_MAIN_CLK,
+				ts->main_clk, GOODIX_MAIN_CLK_LEN);
+}
+
+int goodix_firmware_check(struct goodix_ts_data *ts)
+{
+	int error;
+
+	error = goodix_check_controller_type(ts);
+	if (error)
+		return error;
+
+	if (!ts->controller_has_no_flash)
+		return 0;
+
+	if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) {
+		dev_err(&ts->client->dev, "Error no IRQ-pin access method, cannot upload fw.\n");
+		return -EINVAL;
+	}
+
+	dev_info(&ts->client->dev, "Touchscreen controller needs fw-upload\n");
+	ts->load_cfg_from_disk = true;
+
+	return goodix_firmware_upload(ts);
+}
+
+bool goodix_handle_fw_request(struct goodix_ts_data *ts)
+{
+	int error;
+	u8 val;
+
+	error = goodix_i2c_read(ts->client, GOODIX_REG_REQUEST, &val, 1);
+	if (error)
+		return false;
+
+	switch (val) {
+	case GOODIX_RQST_RESPONDED:
+		/* If we read back our own last ack the IRQ was not for a request */
+		return false;
+	case GOODIX_RQST_CONFIG:
+		error = goodix_send_cfg(ts, ts->config, ts->chip->config_len);
+		if (error)
+			return false;
+
+		break;
+	case GOODIX_RQST_BAK_REF:
+		error = goodix_prepare_bak_ref(ts);
+		if (error)
+			return false;
+
+		error = goodix_i2c_write(ts->client, GOODIX_REG_BAK_REF,
+					 ts->bak_ref, ts->bak_ref_len);
+		if (error)
+			return false;
+
+		break;
+	case GOODIX_RQST_RESET:
+		error = goodix_firmware_upload(ts);
+		if (error)
+			return false;
+
+		break;
+	case GOODIX_RQST_MAIN_CLOCK:
+		error = goodix_send_main_clock(ts);
+		if (error)
+			return false;
+
+		break;
+	case GOODIX_RQST_UNKNOWN:
+	case GOODIX_RQST_IDLE:
+		break;
+	default:
+		dev_err_ratelimited(&ts->client->dev, "Unknown Request: 0x%02x\n", val);
+	}
+
+	/* Ack the request */
+	goodix_i2c_write_u8(ts->client, GOODIX_REG_REQUEST, GOODIX_RQST_RESPONDED);
+	return true;
+}
+
+void goodix_save_bak_ref(struct goodix_ts_data *ts)
+{
+	int error;
+	u8 val;
+
+	if (!ts->controller_has_no_flash)
+		return;
+
+	error = goodix_i2c_read(ts->client, GOODIX_REG_STATUS, &val, 1);
+	if (error)
+		return;
+
+	if (!(val & 0x80))
+		return;
+
+	error = goodix_i2c_read(ts->client, GOODIX_REG_BAK_REF,
+				ts->bak_ref, ts->bak_ref_len);
+	if (error) {
+		memset(ts->bak_ref, 0, ts->bak_ref_len);
+		ts->bak_ref[ts->bak_ref_len - 1] = 1;
+	}
+}
-- 
2.31.1

[PATCH 7/7] platform/x86: touchscreen_dmi: Add info for the Goodix GT912 panel of TM800A550L tablets

[Hans de Goede]

The Bay Trail Glavey TM800A550L tablet, which ships with Android installed
from the factory, uses a GT912 touchscreen controller which needs to have
its firmware uploaded by the OS to work (this is a first for a x86 based
device with a Goodix touchscreen controller).

Add a touchscreen_dmi entry for this which specifies the filenames
to use for the firmware and config files needed for this.

Note this matches on a GDIX1001 ACPI HID, while the original DSDT uses
a HID of GODX0911. For the touchscreen to work on these devices a DSDT
override is necessary to fix a missing IRQ and broken GPIO settings in
the ACPI-resources for the touchscreen. This override also changes the
HID to the standard GDIX1001 id typically used for Goodix touchscreens.
The DSDT override is available here:
https://fedorapeople.org/~jwrdegoede/glavey-tm800a550l-dsdt-override/

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/touchscreen_dmi.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index 90fe4f8f3c2c..d95f04b61111 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -273,6 +273,18 @@ static const struct ts_dmi_data estar_beauty_hd_data = {
 	.properties	= estar_beauty_hd_props,
 };
 
+static const struct property_entry glavey_tm800a550l_props[] = {
+	PROPERTY_ENTRY_STRING("firmware-name", "gt912-glavey-tm800a550l.fw"),
+	PROPERTY_ENTRY_STRING("goodix,config-name", "gt912-glavey-tm800a550l.cfg"),
+	PROPERTY_ENTRY_U32("goodix,main-clk", 54),
+	{ }
+};
+
+static const struct ts_dmi_data glavey_tm800a550l_data = {
+	.acpi_name	= "GDIX1001:00",
+	.properties	= glavey_tm800a550l_props,
+};
+
 static const struct property_entry gp_electronic_t701_props[] = {
 	PROPERTY_ENTRY_U32("touchscreen-size-x", 960),
 	PROPERTY_ENTRY_U32("touchscreen-size-y", 640),
@@ -1003,6 +1015,15 @@ const struct dmi_system_id touchscreen_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "eSTAR BEAUTY HD Intel Quad core"),
 		},
 	},
+	{	/* Glavey TM800A550L */
+		.driver_data = (void *)&glavey_tm800a550l_data,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
+			DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"),
+			/* Above strings are too generic, also match on BIOS version */
+			DMI_MATCH(DMI_BIOS_VERSION, "ZY-8-BI-PX4S70VTR400-X423B-005-D"),
+		},
+	},
 	{
 		/* GP-electronic T701 */
 		.driver_data = (void *)&gp_electronic_t701_data,
-- 
2.31.1

Re: [PATCH 0/7] Input: goodix - Add support for controllers without flash

[Bastien Nocera]

On Wed, 2021-04-28 at 13:46 +0200, Hans de Goede wrote:
> Hi All,
> 
> A while ago I bought a Glavey TM800A550L tablet. This is a Bay Trail
> (x86)
> tablet which comes with Android from the factory.
> 
> It is mostly just another Bay Trail tablet, but the way the Goodix
> touchscreen on it works is interesting. The controller needs to have
> firmware uploaded to it before it will work. After that it pretty
> much
> works 100% the same as any other Goodix touchscreen controller.
> 
> I've described this patch-set in the Subject as "Add support for
> controllers without flash", but I suspect that the Goodix controllers
> used on Bay Trail devices which ship with Windows pre-installed may
> also
> lack flash; and that the Windows version of the BIOS-es on these
> devices
> does the upload for us. Anyways just something which I'm wondering
> about,
> these patches are necessary to make things work regardless.
> 
> These patches should also be useful for supporting the Goodix
> controllers
> on some ARM based devices, as some of those need the OS to upload the
> firmware too AFAIK.
> 
> Note patch 7/7 is included to get an overview of the entire set.
> I'll merge that one myself through the pdx86 tree. Patches 1-6 are
> intended for merging through the input tree.


I've done a cursory review, and didn't find anything particularly
egregious. Did you check that the intermediate patches all compiled?
Feel free to add my Reviewed-by:

My only question would be where the firmware are supposed to come from
after this gets merged.

Cheers

Re: [PATCH 0/7] Input: goodix - Add support for controllers without flash

[Hans de Goede]

Hi,

On 4/28/21 2:03 PM, Bastien Nocera wrote:
> On Wed, 2021-04-28 at 13:46 +0200, Hans de Goede wrote:
>> Hi All,
>>
>> A while ago I bought a Glavey TM800A550L tablet. This is a Bay Trail
>> (x86)
>> tablet which comes with Android from the factory.
>>
>> It is mostly just another Bay Trail tablet, but the way the Goodix
>> touchscreen on it works is interesting. The controller needs to have
>> firmware uploaded to it before it will work. After that it pretty
>> much
>> works 100% the same as any other Goodix touchscreen controller.
>>
>> I've described this patch-set in the Subject as "Add support for
>> controllers without flash", but I suspect that the Goodix controllers
>> used on Bay Trail devices which ship with Windows pre-installed may
>> also
>> lack flash; and that the Windows version of the BIOS-es on these
>> devices
>> does the upload for us. Anyways just something which I'm wondering
>> about,
>> these patches are necessary to make things work regardless.
>>
>> These patches should also be useful for supporting the Goodix
>> controllers
>> on some ARM based devices, as some of those need the OS to upload the
>> firmware too AFAIK.
>>
>> Note patch 7/7 is included to get an overview of the entire set.
>> I'll merge that one myself through the pdx86 tree. Patches 1-6 are
>> intended for merging through the input tree.
> 
> 
> I've done a cursory review, and didn't find anything particularly
> egregious. Did you check that the intermediate patches all compiled?

Yes I did.

> Feel free to add my Reviewed-by:

Thanks, I've added this locally so it will be part of a v2 if
a v2 is necessary.

> My only question would be where the firmware are supposed to come from
> after this gets merged.

That is a good question. I guess step 1 would be to see if we can 
get permission to distribute these from Goodix themselves, so
that these can be added to linux-firmware.

Alternatively I can ask the gsl-firmware repo maintainer if it
is ok to also add Goodix firmware there:

https://github.com/onitake/agsl-firmware/tree/master/firmware/linux

ATM that contains a whole bunch of firmwares for devices with
Silead GSLx6xx controllers. I've contacted Silead about this in
the past but I've failed to get permission to add these to
linux-firmware.

Regards,

Hans

Re: [PATCH 7/7] platform/x86: touchscreen_dmi: Add info for the Goodix GT912 panel of TM800A550L tablets

[Hans de Goede]

Hi,

On 4/28/21 1:46 PM, Hans de Goede wrote:
> The Bay Trail Glavey TM800A550L tablet, which ships with Android installed
> from the factory, uses a GT912 touchscreen controller which needs to have
> its firmware uploaded by the OS to work (this is a first for a x86 based
> device with a Goodix touchscreen controller).
> 
> Add a touchscreen_dmi entry for this which specifies the filenames
> to use for the firmware and config files needed for this.
> 
> Note this matches on a GDIX1001 ACPI HID, while the original DSDT uses
> a HID of GODX0911. For the touchscreen to work on these devices a DSDT
> override is necessary to fix a missing IRQ and broken GPIO settings in
> the ACPI-resources for the touchscreen. This override also changes the
> HID to the standard GDIX1001 id typically used for Goodix touchscreens.
> The DSDT override is available here:
> https://fedorapeople.org/~jwrdegoede/glavey-tm800a550l-dsdt-override/
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Note this patch is part of the
"[GIT PULL] Immutable branch between drivers/platform/x86 and drivers/input due for the v5.14 merge window"
pull-req which I just send out, leaving patches 1-6 to be merged through
the input tree (after merging the pull-req).

Regards,

Hans


> ---
>  drivers/platform/x86/touchscreen_dmi.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index 90fe4f8f3c2c..d95f04b61111 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -273,6 +273,18 @@ static const struct ts_dmi_data estar_beauty_hd_data = {
>  	.properties	= estar_beauty_hd_props,
>  };
>  
> +static const struct property_entry glavey_tm800a550l_props[] = {
> +	PROPERTY_ENTRY_STRING("firmware-name", "gt912-glavey-tm800a550l.fw"),
> +	PROPERTY_ENTRY_STRING("goodix,config-name", "gt912-glavey-tm800a550l.cfg"),
> +	PROPERTY_ENTRY_U32("goodix,main-clk", 54),
> +	{ }
> +};
> +
> +static const struct ts_dmi_data glavey_tm800a550l_data = {
> +	.acpi_name	= "GDIX1001:00",
> +	.properties	= glavey_tm800a550l_props,
> +};
> +
>  static const struct property_entry gp_electronic_t701_props[] = {
>  	PROPERTY_ENTRY_U32("touchscreen-size-x", 960),
>  	PROPERTY_ENTRY_U32("touchscreen-size-y", 640),
> @@ -1003,6 +1015,15 @@ const struct dmi_system_id touchscreen_dmi_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "eSTAR BEAUTY HD Intel Quad core"),
>  		},
>  	},
> +	{	/* Glavey TM800A550L */
> +		.driver_data = (void *)&glavey_tm800a550l_data,
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
> +			DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"),
> +			/* Above strings are too generic, also match on BIOS version */
> +			DMI_MATCH(DMI_BIOS_VERSION, "ZY-8-BI-PX4S70VTR400-X423B-005-D"),
> +		},
> +	},
>  	{
>  		/* GP-electronic T701 */
>  		.driver_data = (void *)&gp_electronic_t701_data,
> 

Re: [PATCH 0/7] Input: goodix - Add support for controllers without flash

[Hans de Goede]

Hi,

On 4/28/21 1:46 PM, Hans de Goede wrote:
> Hi All,
> 
> A while ago I bought a Glavey TM800A550L tablet. This is a Bay Trail (x86)
> tablet which comes with Android from the factory.
> 
> It is mostly just another Bay Trail tablet, but the way the Goodix
> touchscreen on it works is interesting. The controller needs to have
> firmware uploaded to it before it will work. After that it pretty much
> works 100% the same as any other Goodix touchscreen controller.
> 
> I've described this patch-set in the Subject as "Add support for
> controllers without flash", but I suspect that the Goodix controllers
> used on Bay Trail devices which ship with Windows pre-installed may also
> lack flash; and that the Windows version of the BIOS-es on these devices
> does the upload for us. Anyways just something which I'm wondering about,
> these patches are necessary to make things work regardless.
> 
> These patches should also be useful for supporting the Goodix controllers
> on some ARM based devices, as some of those need the OS to upload the
> firmware too AFAIK.
> 
> Note patch 7/7 is included to get an overview of the entire set.
> I'll merge that one myself through the pdx86 tree. Patches 1-6 are
> intended for merging through the input tree.

Self NACK for this series I just found out that the controller on
a Chuwi Hi13 gets mis-indentified as needing fw upload even though
that is not the case.  I'll send out a fixed v2 rebased on top of
5.14-rc1 once 5.14-rc1 is out.

Regards,

Hans



> Hans de Goede (7):
>   Input: goodix - Change goodix_i2c_write() len parameter type to int
>   Input: goodix - Add a goodix.h header file
>   Input: goodix - Refactor reset handling
>   Input: goodix - Push error logging up into i2c_read and i2c_write
>     helpers
>   Input: goodix - Allow specifying the config filename through a
>     "goodix,config-name" device-property
>   Input: goodix - Add support for controllers without flash
>   platform/x86: touchscreen_dmi: Add info for the Goodix GT912 panel of
>     TM800A550L tablets
> 
>  MAINTAINERS                                 |   3 +-
>  drivers/input/touchscreen/Makefile          |   3 +-
>  drivers/input/touchscreen/goodix.c          | 239 +++++------
>  drivers/input/touchscreen/goodix.h          | 118 ++++++
>  drivers/input/touchscreen/goodix_fwupload.c | 437 ++++++++++++++++++++
>  drivers/platform/x86/touchscreen_dmi.c      |  21 +
>  6 files changed, 684 insertions(+), 137 deletions(-)
>  create mode 100644 drivers/input/touchscreen/goodix.h
>  create mode 100644 drivers/input/touchscreen/goodix_fwupload.c
>