[PATCH v2 1/3] Input: goodix - Improve gpiod_get() error logging

[PATCH v2 1/3] Input: goodix - Improve gpiod_get() error logging

[Hans de Goede]

goodix_get_gpio_config() errors are fatal (abort probe()) so log them
at KERN_ERR level rather then as debug messages.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Just do s/dev_dbg/dev_err/, rather then switching to dev_err_probe()
---
 drivers/input/touchscreen/goodix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index aaa3c455e01e..828487d9ded5 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -857,7 +857,7 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
 	if (IS_ERR(gpiod)) {
 		error = PTR_ERR(gpiod);
 		if (error != -EPROBE_DEFER)
-			dev_dbg(dev, "Failed to get %s GPIO: %d\n",
+			dev_err(dev, "Failed to get %s GPIO: %d\n",
 				GOODIX_GPIO_INT_NAME, error);
 		return error;
 	}
@@ -874,7 +874,7 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
 	if (IS_ERR(gpiod)) {
 		error = PTR_ERR(gpiod);
 		if (error != -EPROBE_DEFER)
-			dev_dbg(dev, "Failed to get %s GPIO: %d\n",
+			dev_err(dev, "Failed to get %s GPIO: %d\n",
 				GOODIX_GPIO_RST_NAME, error);
 		return error;
 	}
-- 
2.33.1

[PATCH v2 2/3] Input: goodix - Use the new soc_intel_is_byt() helper

[Hans de Goede]

Use the new soc_intel_is_byt() helper from
linux/platform_data/x86/soc.h.

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

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 828487d9ded5..6e71d5c732af 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
+#include <linux/platform_data/x86/soc.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/of.h>
@@ -686,21 +687,6 @@ static int goodix_reset(struct goodix_ts_data *ts)
 }
 
 #ifdef ACPI_GPIO_SUPPORT
-#include <asm/cpu_device_id.h>
-#include <asm/intel-family.h>
-
-static const struct x86_cpu_id baytrail_cpu_ids[] = {
-	{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT, X86_FEATURE_ANY, },
-	{}
-};
-
-static inline bool is_byt(void)
-{
-	const struct x86_cpu_id *id = x86_match_cpu(baytrail_cpu_ids);
-
-	return !!id;
-}
-
 static const struct acpi_gpio_params first_gpio = { 0, 0, false };
 static const struct acpi_gpio_params second_gpio = { 1, 0, false };
 
@@ -784,7 +770,7 @@ static int goodix_add_acpi_gpio_mappings(struct goodix_ts_data *ts)
 		dev_info(dev, "Using ACPI INTI and INTO methods for IRQ pin access\n");
 		ts->irq_pin_access_method = IRQ_PIN_ACCESS_ACPI_METHOD;
 		gpio_mapping = acpi_goodix_reset_only_gpios;
-	} else if (is_byt() && ts->gpio_count == 2 && ts->gpio_int_idx == -1) {
+	} else if (soc_intel_is_byt() && ts->gpio_count == 2 && ts->gpio_int_idx == -1) {
 		dev_info(dev, "No ACPI GpioInt resource, assuming that the GPIO order is reset, int\n");
 		ts->irq_pin_access_method = IRQ_PIN_ACCESS_ACPI_GPIO;
 		gpio_mapping = acpi_goodix_int_last_gpios;
-- 
2.33.1

[PATCH v2 3/3] Input: goodix - Add pen support

[Hans de Goede]

Some Goodix touchscreens have support for a (Goodix) active pen, add
support for this. The info on how to detect when a pen is down and to
detect when the stylus buttons are pressed was lifted from the out
of tree Goodix driver with pen support written by Adya:
https://gitlab.com/AdyaAdya/goodix-touchscreen-linux-driver/

Since there is no way to tell if pen support is present, the registering
of the pen input_dev is delayed till the first pen event is detected.

This has been tested on a Trekstor Surftab duo W1, a Chuwi Hi13 and
a Cyberbook T116 tablet.

Link: https://gitlab.com/AdyaAdya/goodix-touchscreen-linux-driver/
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=202161
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204513
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 122 ++++++++++++++++++++++++++++-
 drivers/input/touchscreen/goodix.h |   1 +
 2 files changed, 121 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 6e71d5c732af..60874ff5a7f1 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -298,6 +298,107 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 	return -ENOMSG;
 }
 
+static struct input_dev *goodix_create_pen_input(struct goodix_ts_data *ts)
+{
+	struct device *dev = &ts->client->dev;
+	struct input_dev *input;
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return NULL;
+
+	input_alloc_absinfo(input);
+	if (!input->absinfo) {
+		input_free_device(input);
+		return NULL;
+	}
+
+	input->absinfo[ABS_X] = ts->input_dev->absinfo[ABS_MT_POSITION_X];
+	input->absinfo[ABS_Y] = ts->input_dev->absinfo[ABS_MT_POSITION_Y];
+	__set_bit(ABS_X, input->absbit);
+	__set_bit(ABS_Y, input->absbit);
+	input_set_abs_params(input, ABS_PRESSURE, 0, 255, 0, 0);
+
+	input_set_capability(input, EV_KEY, BTN_TOUCH);
+	input_set_capability(input, EV_KEY, BTN_TOOL_PEN);
+	input_set_capability(input, EV_KEY, BTN_STYLUS);
+	input_set_capability(input, EV_KEY, BTN_STYLUS2);
+	__set_bit(INPUT_PROP_DIRECT, input->propbit);
+	/*
+	 * The resolution of these touchscreens is about 10 units/mm, the actual
+	 * resolution does not matter much since we set INPUT_PROP_DIRECT.
+	 * Userspace wants something here though, so just set it to 10 units/mm.
+	 */
+	input_abs_set_res(input, ABS_X, 10);
+	input_abs_set_res(input, ABS_Y, 10);
+
+	input->name = "Goodix Active Pen";
+	input->phys = "input/pen";
+	input->id.bustype = BUS_I2C;
+	if (kstrtou16(ts->id, 10, &input->id.product))
+		input->id.product = 0x1001;
+	input->id.version = ts->version;
+
+	if (input_register_device(input) != 0) {
+		input_free_device(input);
+		return NULL;
+	}
+
+	return input;
+}
+
+static void goodix_ts_report_pen_down(struct goodix_ts_data *ts, u8 *data)
+{
+	int input_x, input_y, input_w;
+	u8 key_value;
+
+	if (!ts->input_pen) {
+		ts->input_pen = goodix_create_pen_input(ts);
+		if (!ts->input_pen)
+			return;
+	}
+
+	if (ts->contact_size == 9) {
+		input_x = get_unaligned_le16(&data[4]);
+		input_y = get_unaligned_le16(&data[6]);
+		input_w = get_unaligned_le16(&data[8]);
+	} else {
+		input_x = get_unaligned_le16(&data[2]);
+		input_y = get_unaligned_le16(&data[4]);
+		input_w = get_unaligned_le16(&data[6]);
+	}
+
+	touchscreen_report_pos(ts->input_pen, &ts->prop, input_x, input_y, false);
+	input_report_abs(ts->input_pen, ABS_PRESSURE, input_w);
+
+	input_report_key(ts->input_pen, BTN_TOUCH, 1);
+	input_report_key(ts->input_pen, BTN_TOOL_PEN, 1);
+
+	if (data[0] & GOODIX_HAVE_KEY) {
+		key_value = data[1 + ts->contact_size];
+		input_report_key(ts->input_pen, BTN_STYLUS, key_value & 0x10);
+		input_report_key(ts->input_pen, BTN_STYLUS2, key_value & 0x20);
+	} else {
+		input_report_key(ts->input_pen, BTN_STYLUS, 0);
+		input_report_key(ts->input_pen, BTN_STYLUS2, 0);
+	}
+
+	input_sync(ts->input_pen);
+}
+
+static void goodix_ts_report_pen_up(struct goodix_ts_data *ts)
+{
+	if (!ts->input_pen)
+		return;
+
+	input_report_key(ts->input_pen, BTN_TOUCH, 0);
+	input_report_key(ts->input_pen, BTN_TOOL_PEN, 0);
+	input_report_key(ts->input_pen, BTN_STYLUS, 0);
+	input_report_key(ts->input_pen, BTN_STYLUS2, 0);
+
+	input_sync(ts->input_pen);
+}
+
 static void goodix_ts_report_touch_8b(struct goodix_ts_data *ts, u8 *coor_data)
 {
 	int id = coor_data[0] & 0x0F;
@@ -328,6 +429,14 @@ static void goodix_ts_report_touch_9b(struct goodix_ts_data *ts, u8 *coor_data)
 	input_report_abs(ts->input_dev, ABS_MT_WIDTH_MAJOR, input_w);
 }
 
+static void goodix_ts_release_keys(struct goodix_ts_data *ts)
+{
+	int i;
+
+	for (i = 0; i < GOODIX_MAX_KEYS; i++)
+		input_report_key(ts->input_dev, ts->keymap[i], 0);
+}
+
 static void goodix_ts_report_key(struct goodix_ts_data *ts, u8 *data)
 {
 	int touch_num;
@@ -342,8 +451,7 @@ static void goodix_ts_report_key(struct goodix_ts_data *ts, u8 *data)
 				input_report_key(ts->input_dev,
 						 ts->keymap[i], 1);
 	} else {
-		for (i = 0; i < GOODIX_MAX_KEYS; i++)
-			input_report_key(ts->input_dev, ts->keymap[i], 0);
+		goodix_ts_release_keys(ts);
 	}
 }
 
@@ -365,6 +473,15 @@ static void goodix_process_events(struct goodix_ts_data *ts)
 	if (touch_num < 0)
 		return;
 
+	/* The pen being down is always reported as a single touch */
+	if (touch_num == 1 && (point_data[1] & 0x80)) {
+		goodix_ts_report_pen_down(ts, point_data);
+		goodix_ts_release_keys(ts);
+		goto sync; /* Release any previousle registered touches */
+	} else {
+		goodix_ts_report_pen_up(ts);
+	}
+
 	goodix_ts_report_key(ts, point_data);
 
 	for (i = 0; i < touch_num; i++)
@@ -375,6 +492,7 @@ static void goodix_process_events(struct goodix_ts_data *ts)
 			goodix_ts_report_touch_8b(ts,
 				&point_data[1 + ts->contact_size * i]);
 
+sync:
 	input_mt_sync_frame(ts->input_dev);
 	input_sync(ts->input_dev);
 }
diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h
index 02065d1c3263..fa8602e78a64 100644
--- a/drivers/input/touchscreen/goodix.h
+++ b/drivers/input/touchscreen/goodix.h
@@ -76,6 +76,7 @@ struct goodix_chip_data {
 struct goodix_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input_dev;
+	struct input_dev *input_pen;
 	const struct goodix_chip_data *chip;
 	const char *firmware_name;
 	struct touchscreen_properties prop;
-- 
2.33.1

Re: [PATCH v2 3/3] Input: goodix - Add pen support

[Dmitry Torokhov]

On Tue, Dec 07, 2021 at 11:07:54AM +0100, Hans de Goede wrote:
> Some Goodix touchscreens have support for a (Goodix) active pen, add
> support for this. The info on how to detect when a pen is down and to
> detect when the stylus buttons are pressed was lifted from the out
> of tree Goodix driver with pen support written by Adya:
> https://gitlab.com/AdyaAdya/goodix-touchscreen-linux-driver/
> 
> Since there is no way to tell if pen support is present, the registering
> of the pen input_dev is delayed till the first pen event is detected.
> 
> This has been tested on a Trekstor Surftab duo W1, a Chuwi Hi13 and
> a Cyberbook T116 tablet.
> 
> Link: https://gitlab.com/AdyaAdya/goodix-touchscreen-linux-driver/
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=202161
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204513
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

-- 
Dmitry

Re: [PATCH v2 2/3] Input: goodix - Use the new soc_intel_is_byt() helper

[Dmitry Torokhov]

On Tue, Dec 07, 2021 at 11:07:53AM +0100, Hans de Goede wrote:
> Use the new soc_intel_is_byt() helper from
> linux/platform_data/x86/soc.h.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/input/touchscreen/goodix.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index 828487d9ded5..6e71d5c732af 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -18,6 +18,7 @@
>  #include <linux/delay.h>
>  #include <linux/irq.h>
>  #include <linux/interrupt.h>
> +#include <linux/platform_data/x86/soc.h>

This header is not in 5.15 so we need to either wait or you need to land
thought your tree, hopefully there are no conflicts.

Thanks.

-- 
Dmitry

Re: [PATCH v2 2/3] Input: goodix - Use the new soc_intel_is_byt() helper

[Hans de Goede]

Hi Dmitry,

On 12/9/21 09:16, Dmitry Torokhov wrote:
> On Tue, Dec 07, 2021 at 11:07:53AM +0100, Hans de Goede wrote:
>> Use the new soc_intel_is_byt() helper from
>> linux/platform_data/x86/soc.h.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/input/touchscreen/goodix.c | 18 ++----------------
>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
>> index 828487d9ded5..6e71d5c732af 100644
>> --- a/drivers/input/touchscreen/goodix.c
>> +++ b/drivers/input/touchscreen/goodix.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/irq.h>
>>  #include <linux/interrupt.h>
>> +#include <linux/platform_data/x86/soc.h>
> 
> This header is not in 5.15 so we need to either wait or you need to land
> thought your tree, hopefully there are no conflicts.

Right, this landed in 5.16-rc1. Since this is just a cleanup waiting
is fine.

I'll re-submit this once 5.16 is out.

Regards,

Hans