From: Hans de Goede <hidden> Date: 2021-05-05 21:40:07
Hi Jiri, Benjamin,
Here is v2 of what started out as a small series to fix spurious wakeups
on T101HA 2-in-1s.
This adds the discussed hid_is_usb_device() helper and uses that in:
"HID: multitouch: Disable event reporting on suspend when our parent is
not a wakeup-source"
To avoid needing to add a "depends on USB_HID" to hid-multitouch Kconfig
settings.
I've checked all other hid_is_using_ll_driver(hdev, &usb_hid_driver) callers
and the only one which can truely benefit from the new helper is the
hid-asus driver, which also deals with some I2C devices on some Asus hw.
All other drivers using hid_is_using_ll_driver(hdev, &usb_hid_driver)
are only for USB devices, so dropping the "depends on USB_HID" does not
make sense for them.
The one other driver which may benefit from the new hid_is_usb_device()
helper would be the Wacom driver which seems to also support I2C devices,
but that contains a lot of USB specific code, so I don't think we can
easily drop the "depends on USB_HID" there.
Even though this is a bit if a mixed-bag of patches, their are several
dependencies between them, so these should probably all go on a single
topic branch.
Regards,
Hans
Hans de Goede (6):
HID: core: Remove extraneous empty line before
EXPORT_SYMBOL_GPL(hid_check_keys_pressed)
HID: core: Add a hid_is_usb_device() helper function
HID: multitouch: Disable event reporting on suspend on the Asus T101HA
touchpad
HID: multitouch: Disable event reporting on suspend when our parent is
not a wakeup-source
HID: asus: Cleanup Asus T101HA keyboard-dock handling
HID: asus: Switch to the new hid_is_usb_device() helper
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-asus.c | 26 ++++++++-----------
drivers/hid/hid-core.c | 11 +++++++-
drivers/hid/hid-multitouch.c | 49 ++++++++++++++++++++++++++++++++++--
include/linux/hid.h | 1 +
5 files changed, 70 insertions(+), 19 deletions(-)
--
2.31.1
From: Hans de Goede <hidden> Date: 2021-05-05 21:40:07
Normally the EXPORT_SYMBOL of a function immediately follows the
declaration of the function and all the other functions in hid-core.c
follow this pattern, drop the extraneous empty line before the
EXPORT_SYMBOL_GPL(hid_check_keys_pressed); line.
Signed-off-by: Hans de Goede <redacted>
---
drivers/hid/hid-core.c | 1 -
1 file changed, 1 deletion(-)
From: Hans de Goede <hidden> Date: 2021-05-05 21:40:08
Sometimes HID drivers want to know if the hid_device with which they
are dealing is using the usb_hid_driver. For example this is often
done to check if it is safe to cast hid_device->dev.parent to an
usb_interface like this:
struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
If drivers directly call hid_is_using_ll_driver(hdev, &usb_hid_driver))
for this, then this leads to a "missing symbol usb_hid_driver"
compilation error when CONFIG_USB_HID is not enabled. Requiring
the driver to have a depends on USB_HID in their Kconfig entry
to work around this.
Add a hid_is_usb_device() helper function which drivers can use
to safely check if they are dealing with a usb_hid device without
needing to worry about the CONFIG_USB_HID setting.
Signed-off-by: Hans de Goede <redacted>
---
drivers/hid/hid-core.c | 10 ++++++++++
include/linux/hid.h | 1 +
2 files changed, 11 insertions(+)
From: Hans de Goede <hidden> Date: 2021-05-05 21:40:09
The Asus T101HA has a problem with spurious wakeups when the lid is
closed, this is caused by the screen sitting so close to the touchpad
that the touchpad ends up reporting touch events, causing these wakeups.
Add a quirk which disables event reporting on suspend when set, and
enable this quirk for the Asus T101HA touchpad fixing the spurious
wakeups, while still allowing the device to be woken by pressing a
key on the keyboard (which is part of the same USB device).
Signed-off-by: Hans de Goede <redacted>
---
drivers/hid/hid-multitouch.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
@@ -1749,8 +1761,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)#ifdef CONFIG_PMstaticintmt_suspend(structhid_device*hdev,pm_message_tstate){+structmt_device*td=hid_get_drvdata(hdev);+/* High latency is desirable for power savings during S3/S0ix */-mt_set_modes(hdev,HID_LATENCY_HIGH,true,true);+if(td->mtclass.quirks&MT_QUIRK_DISABLE_WAKEUP)+mt_set_modes(hdev,HID_LATENCY_HIGH,false,false);+else+mt_set_modes(hdev,HID_LATENCY_HIGH,true,true);+return0;}
@@ -1809,6 +1827,12 @@ static const struct hid_device_id mt_devices[] = {MT_USB_DEVICE(USB_VENDOR_ID_ANTON,USB_DEVICE_ID_ANTON_TOUCH_PAD)},+/* Asus T101HA */+{.driver_data=MT_CLS_WIN_8_DISABLE_WAKEUP,+HID_DEVICE(BUS_USB,HID_GROUP_MULTITOUCH_WIN_8,+USB_VENDOR_ID_ASUSTEK,+USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD)},+/* Asus T304UA */{.driver_data=MT_CLS_ASUS,HID_DEVICE(BUS_USB,HID_GROUP_MULTITOUCH_WIN_8,
From: Hans de Goede <hidden> Date: 2021-05-05 21:40:12
Disable event reporting on suspend when our parent is not
a wakeup-source. This should help save some extra power in
this case.
Signed-off-by: Hans de Goede <redacted>
---
Changes in v2:
- Use new hid_is_usb_device() helper
---
drivers/hid/hid-multitouch.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
@@ -1759,12 +1759,33 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)}#ifdef CONFIG_PM++/* Check if the parent which has the power/wakeup* sysfs attributes may wake the hdev */+staticboolmt_parent_may_wake(structhid_device*hdev)+{+structdevice*parent=hdev->dev.parent;++/*+*USB-HIDisattachedtotheusb_interface(ourparent),the+*power/wakeup*attrarepartoftheusb-devicewhichisitsparent.+*/+if(hid_is_usb_device(hdev)&&parent)+parent=parent->parent;++if(parent)+returndevice_may_wakeup(parent);++/* Huh? Play it safe and keep reporting events. */+returntrue;+}+staticintmt_suspend(structhid_device*hdev,pm_message_tstate){structmt_device*td=hid_get_drvdata(hdev);/* High latency is desirable for power savings during S3/S0ix */-if(td->mtclass.quirks&MT_QUIRK_DISABLE_WAKEUP)+if((td->mtclass.quirks&MT_QUIRK_DISABLE_WAKEUP)||+!mt_parent_may_wake(hdev))mt_set_modes(hdev,HID_LATENCY_HIGH,false,false);elsemt_set_modes(hdev,HID_LATENCY_HIGH,true,true);
From: Hans de Goede <hidden> Date: 2021-05-05 21:40:13
There is no need to use a quirk and then return -ENODEV from the
asus_probe() function to avoid that hid-asus binds to the hiddev
for the USB-interface for the hid-multitouch touchpad.
The hid-multitouch hiddev has a group of HID_GROUP_MULTITOUCH_WIN_8,
so the same result can be achieved by making the hid_device_id entry
for the dock in the asus_devices[] table only match on HID_GROUP_GENERIC
instead of having it match HID_GROUP_ANY.
Signed-off-by: Hans de Goede <redacted>
---
drivers/hid/hid-asus.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
From: Hans de Goede <hidden> Date: 2021-05-05 21:40:13
Switch to the new hid_is_usb_device() helper.
With this new helper building without USB_HID being enabled should work,
so also change the Kconfig depends on from USB_HID to plain HID.
Signed-off-by: Hans de Goede <redacted>
---
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-asus.c | 6 ++----
2 files changed, 3 insertions(+), 5 deletions(-)
Hi Jiri, Benjamin,
Here is v2 of what started out as a small series to fix spurious wakeups
on T101HA 2-in-1s.
This adds the discussed hid_is_usb_device() helper and uses that in:
"HID: multitouch: Disable event reporting on suspend when our parent is
not a wakeup-source"
To avoid needing to add a "depends on USB_HID" to hid-multitouch Kconfig
settings.
I've checked all other hid_is_using_ll_driver(hdev, &usb_hid_driver) callers
and the only one which can truely benefit from the new helper is the
hid-asus driver, which also deals with some I2C devices on some Asus hw.
All other drivers using hid_is_using_ll_driver(hdev, &usb_hid_driver)
are only for USB devices, so dropping the "depends on USB_HID" does not
make sense for them.
The one other driver which may benefit from the new hid_is_usb_device()
helper would be the Wacom driver which seems to also support I2C devices,
but that contains a lot of USB specific code, so I don't think we can
easily drop the "depends on USB_HID" there.
Even though this is a bit if a mixed-bag of patches, their are several
dependencies between them, so these should probably all go on a single
topic branch.
Now in for-5.13/upstream-fixes. Thanks,
--
Jiri Kosina
SUSE Labs
From: Hans de Goede <hidden> Date: 2021-05-26 10:46:48
Hi,
On 5/26/21 12:38 PM, Jiri Kosina wrote:
On Wed, 5 May 2021, Hans de Goede wrote:
quoted
Hi Jiri, Benjamin,
Here is v2 of what started out as a small series to fix spurious wakeups
on T101HA 2-in-1s.
This adds the discussed hid_is_usb_device() helper and uses that in:
"HID: multitouch: Disable event reporting on suspend when our parent is
not a wakeup-source"
To avoid needing to add a "depends on USB_HID" to hid-multitouch Kconfig
settings.
I've checked all other hid_is_using_ll_driver(hdev, &usb_hid_driver) callers
and the only one which can truely benefit from the new helper is the
hid-asus driver, which also deals with some I2C devices on some Asus hw.
All other drivers using hid_is_using_ll_driver(hdev, &usb_hid_driver)
are only for USB devices, so dropping the "depends on USB_HID" does not
make sense for them.
The one other driver which may benefit from the new hid_is_usb_device()
helper would be the Wacom driver which seems to also support I2C devices,
but that contains a lot of USB specific code, so I don't think we can
easily drop the "depends on USB_HID" there.
Even though this is a bit if a mixed-bag of patches, their are several
dependencies between them, so these should probably all go on a single
topic branch.
Hi Jiri, Benjamin,
Here is v2 of what started out as a small series to fix spurious wakeups
on T101HA 2-in-1s.
This adds the discussed hid_is_usb_device() helper and uses that in:
"HID: multitouch: Disable event reporting on suspend when our parent is
not a wakeup-source"
To avoid needing to add a "depends on USB_HID" to hid-multitouch Kconfig
settings.
I've checked all other hid_is_using_ll_driver(hdev, &usb_hid_driver) callers
and the only one which can truely benefit from the new helper is the
hid-asus driver, which also deals with some I2C devices on some Asus hw.
All other drivers using hid_is_using_ll_driver(hdev, &usb_hid_driver)
are only for USB devices, so dropping the "depends on USB_HID" does not
make sense for them.
The one other driver which may benefit from the new hid_is_usb_device()
helper would be the Wacom driver which seems to also support I2C devices,
but that contains a lot of USB specific code, so I don't think we can
easily drop the "depends on USB_HID" there.
Even though this is a bit if a mixed-bag of patches, their are several
dependencies between them, so these should probably all go on a single
topic branch.