From e941dc13fd3717122207d74539ab95da07ef797f Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Fri, 4 Mar 2022 13:17:33 -0800 Subject: Input: zinitix - do not report shadow fingers I observed the following problem with the BT404 touch pad running the Phosh UI: When e.g. typing on the virtual keyboard pressing "g" would produce "ggg". After some analysis it turns out the firmware reports that three fingers hit that coordinate at the same time, finger 0, 2 and 4 (of the five available 0,1,2,3,4). DOWN Zinitix-TS 3-0020: finger 0 down (246, 395) Zinitix-TS 3-0020: finger 1 up (0, 0) Zinitix-TS 3-0020: finger 2 down (246, 395) Zinitix-TS 3-0020: finger 3 up (0, 0) Zinitix-TS 3-0020: finger 4 down (246, 395) UP Zinitix-TS 3-0020: finger 0 up (246, 395) Zinitix-TS 3-0020: finger 2 up (246, 395) Zinitix-TS 3-0020: finger 4 up (246, 395) This is one touch and release: i.e. this is all reported on touch (down) and release. There is a field in the struct touch_event called finger_cnt which is actually a bitmask of the fingers active in the event. Rename this field finger_mask as this matches the use contents better, then use for_each_set_bit() to iterate over just the fingers that are actally active. Factor out a finger reporting function zinitix_report_fingers() to handle all fingers. Also be more careful in reporting finger down/up: we were reporting every event with input_mt_report_slot_state(..., true); but this should only be reported on finger down or move, not on finger up, so also add code to check p->sub_status to see what is happening and report correctly. After this my Zinitix BT404 touchscreen report fingers flawlessly. The vendor drive I have notably does not use the "finger_cnt" and contains obviously incorrect code like this: if (touch_dev->touch_info.finger_cnt > MAX_SUPPORTED_FINGER_NUM) touch_dev->touch_info.finger_cnt = MAX_SUPPORTED_FINGER_NUM; As MAX_SUPPORTED_FINGER_NUM is an ordinal and the field is a bitmask this seems quite confused. Signed-off-by: Linus Walleij Link: https://lore.kernel.org/r/20220228233017.2270599-1-linus.walleij@linaro.org Signed-off-by: Dmitry Torokhov --- drivers/input/touchscreen/zinitix.c | 44 +++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) (limited to 'drivers/input') diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c index 129ebc810de8..8bd03278ad9a 100644 --- a/drivers/input/touchscreen/zinitix.c +++ b/drivers/input/touchscreen/zinitix.c @@ -135,7 +135,7 @@ struct point_coord { struct touch_event { __le16 status; - u8 finger_cnt; + u8 finger_mask; u8 time_stamp; struct point_coord point_coord[MAX_SUPPORTED_FINGER_NUM]; }; @@ -322,11 +322,32 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541) static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot, const struct point_coord *p) { + u16 x, y; + + if (unlikely(!(p->sub_status & + (SUB_BIT_UP | SUB_BIT_DOWN | SUB_BIT_MOVE)))) { + dev_dbg(&bt541->client->dev, "unknown finger event %#02x\n", + p->sub_status); + return; + } + + x = le16_to_cpu(p->x); + y = le16_to_cpu(p->y); + input_mt_slot(bt541->input_dev, slot); - input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); - touchscreen_report_pos(bt541->input_dev, &bt541->prop, - le16_to_cpu(p->x), le16_to_cpu(p->y), true); - input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); + if (input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, + !(p->sub_status & SUB_BIT_UP))) { + touchscreen_report_pos(bt541->input_dev, + &bt541->prop, x, y, true); + input_report_abs(bt541->input_dev, + ABS_MT_TOUCH_MAJOR, p->width); + dev_dbg(&bt541->client->dev, "finger %d %s (%u, %u)\n", + slot, p->sub_status & SUB_BIT_DOWN ? "down" : "move", + x, y); + } else { + dev_dbg(&bt541->client->dev, "finger %d up (%u, %u)\n", + slot, x, y); + } } static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) @@ -334,6 +355,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) struct bt541_ts_data *bt541 = bt541_handler; struct i2c_client *client = bt541->client; struct touch_event touch_event; + unsigned long finger_mask; int error; int i; @@ -346,10 +368,14 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) goto out; } - for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) - if (touch_event.point_coord[i].sub_status & SUB_BIT_EXIST) - zinitix_report_finger(bt541, i, - &touch_event.point_coord[i]); + finger_mask = touch_event.finger_mask; + for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) { + const struct point_coord *p = &touch_event.point_coord[i]; + + /* Only process contacts that are actually reported */ + if (p->sub_status & SUB_BIT_EXIST) + zinitix_report_finger(bt541, i, p); + } input_mt_sync_frame(bt541->input_dev); input_sync(bt541->input_dev); -- cgit v1.2.3-70-g09d2 From 5600f6986628dde8881734090588474f54a540a8 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Sun, 13 Mar 2022 22:56:32 -0700 Subject: Input: aiptek - properly check endpoint type Syzbot reported warning in usb_submit_urb() which is caused by wrong endpoint type. There was a check for the number of endpoints, but not for the type of endpoint. Fix it by replacing old desc.bNumEndpoints check with usb_find_common_endpoints() helper for finding endpoints Fail log: usb 5-1: BOGUS urb xfer, pipe 1 != type 3 WARNING: CPU: 2 PID: 48 at drivers/usb/core/urb.c:502 usb_submit_urb+0xed2/0x18a0 drivers/usb/core/urb.c:502 Modules linked in: CPU: 2 PID: 48 Comm: kworker/2:2 Not tainted 5.17.0-rc6-syzkaller-00226-g07ebd38a0da2 #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Workqueue: usb_hub_wq hub_event ... Call Trace: aiptek_open+0xd5/0x130 drivers/input/tablet/aiptek.c:830 input_open_device+0x1bb/0x320 drivers/input/input.c:629 kbd_connect+0xfe/0x160 drivers/tty/vt/keyboard.c:1593 Fixes: 8e20cf2bce12 ("Input: aiptek - fix crash on detecting device without endpoints") Reported-and-tested-by: syzbot+75cccf2b7da87fb6f84b@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin Link: https://lore.kernel.org/r/20220308194328.26220-1-paskripkin@gmail.com Signed-off-by: Dmitry Torokhov --- drivers/input/tablet/aiptek.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/input') diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c index fcb1b646436a..1581f6ef0927 100644 --- a/drivers/input/tablet/aiptek.c +++ b/drivers/input/tablet/aiptek.c @@ -1787,15 +1787,13 @@ aiptek_probe(struct usb_interface *intf, const struct usb_device_id *id) input_set_abs_params(inputdev, ABS_TILT_Y, AIPTEK_TILT_MIN, AIPTEK_TILT_MAX, 0, 0); input_set_abs_params(inputdev, ABS_WHEEL, AIPTEK_WHEEL_MIN, AIPTEK_WHEEL_MAX - 1, 0, 0); - /* Verify that a device really has an endpoint */ - if (intf->cur_altsetting->desc.bNumEndpoints < 1) { + err = usb_find_common_endpoints(intf->cur_altsetting, + NULL, NULL, &endpoint, NULL); + if (err) { dev_err(&intf->dev, - "interface has %d endpoints, but must have minimum 1\n", - intf->cur_altsetting->desc.bNumEndpoints); - err = -EINVAL; + "interface has no int in endpoints, but must have minimum 1\n"); goto fail3; } - endpoint = &intf->cur_altsetting->endpoint[0].desc; /* Go set up our URB, which is called when the tablet receives * input. -- cgit v1.2.3-70-g09d2 From ca1eadbfcd36bec73f2a2111c28e8c7e9e8ae6c0 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 16 May 2022 13:29:58 -0700 Subject: Input: cros-ec-keyb - allow skipping keyboard registration If the device is a detachable (and therefore lacks full keyboard), we may still want to load this driver because the device might have some other buttons or switches (e.g. volume and power buttons or a tablet mode switch). In such case we do not want to register the "main" keyboard device to allow userspace detect when the detachable keyboard is disconnected and adjust the system behavior for the tablet mode. Originally it was suggested to simply skip keyboard registration if row and columns properties didn't exist, but that approach did not convey the intent strongly enough and also had a slight problem for migrating existing DTBs without updating the kernel first, so it was decided to introduce new google,cros-ec-keyb-switches to explicitly mark devices that only have axillary buttons and switches. Reviewed-by: Douglas Anderson Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20220516183452.942008-3-swboyd@chromium.org Signed-off-by: Dmitry Torokhov --- drivers/input/keyboard/cros_ec_keyb.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) (limited to 'drivers/input') diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c index fc02c540636e..06a90fe82a83 100644 --- a/drivers/input/keyboard/cros_ec_keyb.c +++ b/drivers/input/keyboard/cros_ec_keyb.c @@ -439,10 +439,13 @@ static __maybe_unused int cros_ec_keyb_resume(struct device *dev) * but the ckdev->bs_idev will remain NULL when this function exits. * * @ckdev: The keyboard device + * @expect_buttons_switches: Indicates that EC must report button and/or + * switch events * * Returns 0 if no error or -error upon error. */ -static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev) +static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev, + bool expect_buttons_switches) { struct cros_ec_device *ec_dev = ckdev->ec; struct device *dev = ckdev->dev; @@ -469,7 +472,7 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev) switches = get_unaligned_le32(&event_data.switches); if (!buttons && !switches) - return 0; + return expect_buttons_switches ? -EINVAL : 0; /* * We call the non-matrix buttons/switches 'input1', if present. @@ -520,7 +523,7 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev) } /** - * cros_ec_keyb_register_bs - Register matrix keys + * cros_ec_keyb_register_matrix - Register matrix keys * * Handles all the bits of the keyboard driver related to matrix keys. * @@ -659,12 +662,12 @@ static const struct attribute_group cros_ec_keyb_attr_group = { .attrs = cros_ec_keyb_attrs, }; - static int cros_ec_keyb_probe(struct platform_device *pdev) { struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent); struct device *dev = &pdev->dev; struct cros_ec_keyb *ckdev; + bool buttons_switches_only = device_get_match_data(dev); int err; if (!dev->of_node) @@ -678,13 +681,16 @@ static int cros_ec_keyb_probe(struct platform_device *pdev) ckdev->dev = dev; dev_set_drvdata(dev, ckdev); - err = cros_ec_keyb_register_matrix(ckdev); - if (err) { - dev_err(dev, "cannot register matrix inputs: %d\n", err); - return err; + if (!buttons_switches_only) { + err = cros_ec_keyb_register_matrix(ckdev); + if (err) { + dev_err(dev, "cannot register matrix inputs: %d\n", + err); + return err; + } } - err = cros_ec_keyb_register_bs(ckdev); + err = cros_ec_keyb_register_bs(ckdev, buttons_switches_only); if (err) { dev_err(dev, "cannot register non-matrix inputs: %d\n", err); return err; @@ -692,7 +698,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev) err = devm_device_add_group(dev, &cros_ec_keyb_attr_group); if (err) { - dev_err(dev, "failed to create attributes. err=%d\n", err); + dev_err(dev, "failed to create attributes: %d\n", err); return err; } @@ -721,7 +727,8 @@ static int cros_ec_keyb_remove(struct platform_device *pdev) #ifdef CONFIG_OF static const struct of_device_id cros_ec_keyb_of_match[] = { { .compatible = "google,cros-ec-keyb" }, - {}, + { .compatible = "google,cros-ec-keyb-switches", .data = (void *)true }, + {} }; MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match); #endif -- cgit v1.2.3-70-g09d2