Re: [PATCH 3/3] HID: multitouch: Disable event reporting on suspend when our parent is not a wakeup-source
flat view
From: Benjamin Tissoires <hidden>
Date: 2021-05-05 13:40:46
Hi Hans, On Sat, Mar 6, 2021 at 2:37 PM Hans de Goede [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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> --- drivers/hid/Kconfig | 2 +- drivers/hid/hid-multitouch.c | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 786b71ef7738..5cbe4adfd816 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig@@ -675,7 +675,7 @@ config HID_MONTEREY config HID_MULTITOUCH tristate "HID Multitouch panels" - depends on HID + depends on USB_HID
I tried really hard during the past 8 years to not have a usbhid dependency on hid-multitouch. The code below should not break the test suite, but still I am not that happy about the Kconfig change. I don't see an immediate and better way of doing what you are achieving here, but maybe you have some magic I did not think about that would help to no pull USB_HID with HID_MULTITOUCH. FTR, I think the use case of hid-multitouch *without* USB is rather non-existent, but there might be some weird systems with I2C only (edge computing?). Cheers, Benjamin
quoted hunk ↗ jump to hunk
help Generic support for HID multitouch panels.diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index cfb68e443ddd..7926295bab81 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c@@ -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 */ +static bool mt_parent_may_wake(struct hid_device *hdev) +{ + struct device *parent = hdev->dev.parent; + + /* + * USB-HID is attached to the usb_interface (our parent), the + * power/wakeup* attr are part of the usb-device which is its parent. + */ + if (hid_is_using_ll_driver(hdev, &usb_hid_driver) && parent) + parent = parent->parent; + + if (parent) + return device_may_wakeup(parent); + + /* Huh? Play it safe and keep reporting events. */ + return true; +} + static int mt_suspend(struct hid_device *hdev, pm_message_t state) { struct mt_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); else mt_set_modes(hdev, HID_LATENCY_HIGH, true, true); --2.30.1