Hi Hans,
On Sat, Mar 6, 2021 at 2:37 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> 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 <hdegoede@xxxxxxxxxx>
> ---
> 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
> 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
>
