* [PATCH 0/4] Input: goodix - pen support + misc patches
@ 2021-12-06 16:47 Hans de Goede
2021-12-06 16:47 ` [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model Hans de Goede
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Hans de Goede @ 2021-12-06 16:47 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input
Hi Dmitry,
Here is a patch-series for the Goodix touchscreen drivers, this is based
on top of the " Input: goodix - Try not to touch the reset-pin on
x86/ACPI devices" fix from earlier today (there are no conflicts
AFAIK, so the 2 can be merged independently).
This series consist of 3 small fixes/cleanups, followed by adding
pen (Goodix active stylus) support, which is supported on some
Goodix touchscreens.
Regards,
Hans
Hans de Goede (4):
Input: goodix - Add id->model mapping for the "9111" model
Input: goodix - Improve gpiod_get() error logging
Input: goodix - Use the new soc_intel_is_byt() helper
Input: goodix - Add pen support
drivers/input/touchscreen/goodix.c | 162 +++++++++++++++++++++++------
drivers/input/touchscreen/goodix.h | 1 +
2 files changed, 131 insertions(+), 32 deletions(-)
--
2.33.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model 2021-12-06 16:47 [PATCH 0/4] Input: goodix - pen support + misc patches Hans de Goede @ 2021-12-06 16:47 ` Hans de Goede 2021-12-07 7:29 ` Dmitry Torokhov 2021-12-06 16:47 ` [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging Hans de Goede ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-12-06 16:47 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input Add d->model mapping for the "9111" model, this fixes uses using a wrong config_len of 240 bytes while the "9111" model uses only 186 bytes of config. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/touchscreen/goodix.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index eaa659969097..aaa3c455e01e 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -102,6 +102,7 @@ static const struct goodix_chip_id goodix_chip_ids[] = { { .id = "911", .data = >911_chip_data }, { .id = "9271", .data = >911_chip_data }, { .id = "9110", .data = >911_chip_data }, + { .id = "9111", .data = >911_chip_data }, { .id = "927", .data = >911_chip_data }, { .id = "928", .data = >911_chip_data }, -- 2.33.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model 2021-12-06 16:47 ` [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model Hans de Goede @ 2021-12-07 7:29 ` Dmitry Torokhov 0 siblings, 0 replies; 12+ messages in thread From: Dmitry Torokhov @ 2021-12-07 7:29 UTC (permalink / raw) To: Hans de Goede; +Cc: Bastien Nocera, linux-input On Mon, Dec 06, 2021 at 05:47:44PM +0100, Hans de Goede wrote: > Add d->model mapping for the "9111" model, this fixes uses using > a wrong config_len of 240 bytes while the "9111" model uses > only 186 bytes of config. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Applied, thank you. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging 2021-12-06 16:47 [PATCH 0/4] Input: goodix - pen support + misc patches Hans de Goede 2021-12-06 16:47 ` [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model Hans de Goede @ 2021-12-06 16:47 ` Hans de Goede 2021-12-07 7:31 ` Dmitry Torokhov 2021-12-06 16:47 ` [PATCH 3/4] Input: goodix - Use the new soc_intel_is_byt() helper Hans de Goede 2021-12-06 16:47 ` [PATCH 4/4] Input: goodix - Add pen support Hans de Goede 3 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-12-06 16:47 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input goodix_get_gpio_config() errors are fatal (abort probe()) so log them at KERN_ERR level rather then as debug messages. This change uses dev_err_probe() to automatically suppress the errors in case of -EPROBE_DEFER. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/touchscreen/goodix.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index aaa3c455e01e..73f3b24f7f1e 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -854,13 +854,10 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts) retry_get_irq_gpio: /* Get the interrupt GPIO pin number */ gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN); - if (IS_ERR(gpiod)) { - error = PTR_ERR(gpiod); - if (error != -EPROBE_DEFER) - dev_dbg(dev, "Failed to get %s GPIO: %d\n", - GOODIX_GPIO_INT_NAME, error); - return error; - } + if (IS_ERR(gpiod)) + return dev_err_probe(dev, PTR_ERR(gpiod), "getting %s GPIO\n", + GOODIX_GPIO_INT_NAME); + if (!gpiod && has_acpi_companion(dev) && !added_acpi_mappings) { added_acpi_mappings = true; if (goodix_add_acpi_gpio_mappings(ts) == 0) @@ -871,13 +868,9 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts) /* Get the reset line GPIO pin number */ gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, ts->gpiod_rst_flags); - if (IS_ERR(gpiod)) { - error = PTR_ERR(gpiod); - if (error != -EPROBE_DEFER) - dev_dbg(dev, "Failed to get %s GPIO: %d\n", - GOODIX_GPIO_RST_NAME, error); - return error; - } + if (IS_ERR(gpiod)) + return dev_err_probe(dev, PTR_ERR(gpiod), "getting %s GPIO\n", + GOODIX_GPIO_RST_NAME); ts->gpiod_rst = gpiod; -- 2.33.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging 2021-12-06 16:47 ` [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging Hans de Goede @ 2021-12-07 7:31 ` Dmitry Torokhov 2021-12-07 10:07 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Dmitry Torokhov @ 2021-12-07 7:31 UTC (permalink / raw) To: Hans de Goede; +Cc: Bastien Nocera, linux-input Hi Hans, On Mon, Dec 06, 2021 at 05:47:45PM +0100, Hans de Goede wrote: > goodix_get_gpio_config() errors are fatal (abort probe()) so log them > at KERN_ERR level rather then as debug messages. > > This change uses dev_err_probe() to automatically suppress the errors > in case of -EPROBE_DEFER. I really believe that dev_err_probe() is wrong API as the providers should be setting the reason for deferred probe failures. Could you simply swap dev_dbg() for dev_err()? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging 2021-12-07 7:31 ` Dmitry Torokhov @ 2021-12-07 10:07 ` Hans de Goede 0 siblings, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-12-07 10:07 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Bastien Nocera, linux-input Hi Dmitry, On 12/7/21 08:31, Dmitry Torokhov wrote: > Hi Hans, > > On Mon, Dec 06, 2021 at 05:47:45PM +0100, Hans de Goede wrote: >> goodix_get_gpio_config() errors are fatal (abort probe()) so log them >> at KERN_ERR level rather then as debug messages. >> >> This change uses dev_err_probe() to automatically suppress the errors >> in case of -EPROBE_DEFER. > > I really believe that dev_err_probe() is wrong API as the providers > should be setting the reason for deferred probe failures. > > Could you simply swap dev_dbg() for dev_err()? Done for v2, which I'm sending out right away. Note I've dropped the first patch for v2 since you said you applied this. Regards, Hans p.s. Any chance you could also take a look at this 2 patch patch-series, this has been pending for a while now: https://lore.kernel.org/linux-input/20211122220637.11386-1-hdegoede@redhat.com/T/#t ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] Input: goodix - Use the new soc_intel_is_byt() helper 2021-12-06 16:47 [PATCH 0/4] Input: goodix - pen support + misc patches Hans de Goede 2021-12-06 16:47 ` [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model Hans de Goede 2021-12-06 16:47 ` [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging Hans de Goede @ 2021-12-06 16:47 ` Hans de Goede 2021-12-06 16:47 ` [PATCH 4/4] Input: goodix - Add pen support Hans de Goede 3 siblings, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-12-06 16:47 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input 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 73f3b24f7f1e..2d38a941e7e4 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] Input: goodix - Add pen support 2021-12-06 16:47 [PATCH 0/4] Input: goodix - pen support + misc patches Hans de Goede ` (2 preceding siblings ...) 2021-12-06 16:47 ` [PATCH 3/4] Input: goodix - Use the new soc_intel_is_byt() helper Hans de Goede @ 2021-12-06 16:47 ` Hans de Goede 2021-12-07 17:50 ` Dmitry Torokhov 3 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-12-06 16:47 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input 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 2d38a941e7e4..691e4505cf4a 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Input: goodix - Add pen support 2021-12-06 16:47 ` [PATCH 4/4] Input: goodix - Add pen support Hans de Goede @ 2021-12-07 17:50 ` Dmitry Torokhov 2021-12-08 8:37 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Dmitry Torokhov @ 2021-12-07 17:50 UTC (permalink / raw) To: Hans de Goede; +Cc: Bastien Nocera, linux-input On Mon, Dec 06, 2021 at 05:47:47PM +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> > --- > 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 2d38a941e7e4..691e4505cf4a 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; > + } Please drop this as input_abs_set_max() will do allocation and input_register_device() will reject device with ABS_* events without absinfo allocated. > + > + input->absinfo[ABS_X] = ts->input_dev->absinfo[ABS_MT_POSITION_X]; input_abs_set_max(input, ABS_X, input_abs_get_max(input, ABS_MT_POSITION_X); or even maybe input_set_abs_params(input, ABS_X, 0, input_abs_get_max(input, ABS_MT_POSITION_X), 0, 0); > + input->absinfo[ABS_Y] = ts->input_dev->absinfo[ABS_MT_POSITION_Y]; > + __set_bit(ABS_X, input->absbit); > + __set_bit(ABS_Y, input->absbit); This might not be needed, depending... > + 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); Could it be moved next to setting up axes? > + > + 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); Warrants a comment on why we need to free devm. Is it going to be safely destroyed on removal? It is likely to happen very first thing, before we deal with interrupts, etc. > + 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); Use BIT? > + } 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 { Not sure why we need else with goto... > + 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 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Input: goodix - Add pen support 2021-12-07 17:50 ` Dmitry Torokhov @ 2021-12-08 8:37 ` Hans de Goede 2021-12-08 8:51 ` Dmitry Torokhov 0 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2021-12-08 8:37 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Bastien Nocera, linux-input Hi Dmitry, Thank you for the review. On 12/7/21 18:50, Dmitry Torokhov wrote: > On Mon, Dec 06, 2021 at 05:47:47PM +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> >> --- >> 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 2d38a941e7e4..691e4505cf4a 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; >> + } > > Please drop this as input_abs_set_max() will do allocation and > input_register_device() will reject device with ABS_* events without > absinfo allocated. > >> + >> + input->absinfo[ABS_X] = ts->input_dev->absinfo[ABS_MT_POSITION_X]; > > input_abs_set_max(input, ABS_X, > input_abs_get_max(input, ABS_MT_POSITION_X); > > or even maybe > > input_set_abs_params(input, ABS_X, > 0, input_abs_get_max(input, ABS_MT_POSITION_X), 0, 0); The reason why I'm just copying the entire absinfo struct (and thus need the NULL check above) is because this driver uses touchscreen_parse_properties(), so the min and fuzz values might (theoretically) also be set through device-properties and I wanted to cover that. Since you don't like the above approach, I will go with the following for the next version: input_set_abs_params(input, ABS_X, input_abs_get_min(ts->input_dev, ABS_MT_POSITION_X), input_abs_get_max(ts->input_dev, ABS_MT_POSITION_X), input_abs_get_fuzz(ts->input_dev, ABS_MT_POSITION_X), input_abs_get_flat(ts->input_dev, ABS_MT_POSITION_X)); (and the same for the Y axis). > > >> + input->absinfo[ABS_Y] = ts->input_dev->absinfo[ABS_MT_POSITION_Y]; >> + __set_bit(ABS_X, input->absbit); >> + __set_bit(ABS_Y, input->absbit); > > This might not be needed, depending... Ack, will drop. > >> + 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); > > Could it be moved next to setting up axes? Ack, will do. > >> + >> + 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); > > Warrants a comment on why we need to free devm. > > Is it going to be safely destroyed on removal? It is likely to happen > very first thing, before we deal with interrupts, etc. That is a very good point, since I've chosen to create the pen input device on the first pen event, it happens after the interrupts have been registered. So that means the driver can no longer rely on the devm reverse-teardown order to get things right. For the next version I will always allocate the device and only register it on the fly. Since the devres is created when allocating this will fix this ordering problem. The device will then still unregister before the irq gets disabled, but it will not be free-ed, so the irq can still safely use it. >> + 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); > > Use BIT? Ack. > >> + } 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 { > > Not sure why we need else with goto... It is not needed, I wrote it this way for readability, one is the pen down path and the other the pen up path, so having that as an if ... else ... feels more natural to me. I plan to keep this for the next version, but if you strongly dislike this let me know and/or feel free to modify this when you merge the next version. Regards, Hans > >> + 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 >> > > Thanks. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Input: goodix - Add pen support 2021-12-08 8:37 ` Hans de Goede @ 2021-12-08 8:51 ` Dmitry Torokhov 2021-12-08 17:02 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Dmitry Torokhov @ 2021-12-08 8:51 UTC (permalink / raw) To: Hans de Goede; +Cc: Bastien Nocera, linux-input On Wed, Dec 8, 2021 at 12:37 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi Dmitry, > > Thank you for the review. > > On 12/7/21 18:50, Dmitry Torokhov wrote: > > On Mon, Dec 06, 2021 at 05:47:47PM +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> > >> --- > >> 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 2d38a941e7e4..691e4505cf4a 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; > >> + } > > > > Please drop this as input_abs_set_max() will do allocation and > > input_register_device() will reject device with ABS_* events without > > absinfo allocated. > > > >> + > >> + input->absinfo[ABS_X] = ts->input_dev->absinfo[ABS_MT_POSITION_X]; > > > > input_abs_set_max(input, ABS_X, > > input_abs_get_max(input, ABS_MT_POSITION_X); > > > > or even maybe > > > > input_set_abs_params(input, ABS_X, > > 0, input_abs_get_max(input, ABS_MT_POSITION_X), 0, 0); > > The reason why I'm just copying the entire absinfo struct > (and thus need the NULL check above) is because this driver uses > touchscreen_parse_properties(), so the min and fuzz values > might (theoretically) also be set through device-properties and > I wanted to cover that. > > Since you don't like the above approach, I will go with the following > for the next version: > > input_set_abs_params(input, ABS_X, > input_abs_get_min(ts->input_dev, ABS_MT_POSITION_X), > input_abs_get_max(ts->input_dev, ABS_MT_POSITION_X), > input_abs_get_fuzz(ts->input_dev, ABS_MT_POSITION_X), > input_abs_get_flat(ts->input_dev, ABS_MT_POSITION_X)); > > (and the same for the Y axis). Ah, sorry, I misread the code. It is fine as is then, or we could even consider adding input_copy_abs(input, axis, src) that would allocate absinfo if needed, set capability, and do the copy. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Input: goodix - Add pen support 2021-12-08 8:51 ` Dmitry Torokhov @ 2021-12-08 17:02 ` Hans de Goede 0 siblings, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-12-08 17:02 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Bastien Nocera, linux-input Hi Dmitry, On 12/8/21 09:51, Dmitry Torokhov wrote: > On Wed, Dec 8, 2021 at 12:37 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi Dmitry, >> >> Thank you for the review. >> >> On 12/7/21 18:50, Dmitry Torokhov wrote: >>> On Mon, Dec 06, 2021 at 05:47:47PM +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> >>>> --- >>>> 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 2d38a941e7e4..691e4505cf4a 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; >>>> + } >>> >>> Please drop this as input_abs_set_max() will do allocation and >>> input_register_device() will reject device with ABS_* events without >>> absinfo allocated. >>> >>>> + >>>> + input->absinfo[ABS_X] = ts->input_dev->absinfo[ABS_MT_POSITION_X]; >>> >>> input_abs_set_max(input, ABS_X, >>> input_abs_get_max(input, ABS_MT_POSITION_X); >>> >>> or even maybe >>> >>> input_set_abs_params(input, ABS_X, >>> 0, input_abs_get_max(input, ABS_MT_POSITION_X), 0, 0); >> >> The reason why I'm just copying the entire absinfo struct >> (and thus need the NULL check above) is because this driver uses >> touchscreen_parse_properties(), so the min and fuzz values >> might (theoretically) also be set through device-properties and >> I wanted to cover that. >> >> Since you don't like the above approach, I will go with the following >> for the next version: >> >> input_set_abs_params(input, ABS_X, >> input_abs_get_min(ts->input_dev, ABS_MT_POSITION_X), >> input_abs_get_max(ts->input_dev, ABS_MT_POSITION_X), >> input_abs_get_fuzz(ts->input_dev, ABS_MT_POSITION_X), >> input_abs_get_flat(ts->input_dev, ABS_MT_POSITION_X)); >> >> (and the same for the Y axis). > > Ah, sorry, I misread the code. It is fine as is then, or we could even > consider adding input_copy_abs(input, axis, src) that would allocate > absinfo if needed, set capability, and do the copy. Oh, I think that adding an input_copy_abs() helper for this is actually a good idea. I'll do that for the next version (in a separate patch of course). Regards, Hans ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-08 17:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-06 16:47 [PATCH 0/4] Input: goodix - pen support + misc patches Hans de Goede 2021-12-06 16:47 ` [PATCH 1/4] Input: goodix - Add id->model mapping for the "9111" model Hans de Goede 2021-12-07 7:29 ` Dmitry Torokhov 2021-12-06 16:47 ` [PATCH 2/4] Input: goodix - Improve gpiod_get() error logging Hans de Goede 2021-12-07 7:31 ` Dmitry Torokhov 2021-12-07 10:07 ` Hans de Goede 2021-12-06 16:47 ` [PATCH 3/4] Input: goodix - Use the new soc_intel_is_byt() helper Hans de Goede 2021-12-06 16:47 ` [PATCH 4/4] Input: goodix - Add pen support Hans de Goede 2021-12-07 17:50 ` Dmitry Torokhov 2021-12-08 8:37 ` Hans de Goede 2021-12-08 8:51 ` Dmitry Torokhov 2021-12-08 17:02 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.