Re: [PATCH] HID: rmi: Make sure the HID device is opened on resume

From: Benjamin Tissoires
Date: Mon Jul 24 2017 - 04:15:54 EST


On Jul 22 2017 or thereabouts, Lyude wrote:
> So it looks like that suspend/resume has actually always been broken on
> hid-rmi. The fact it worked was a rather silly coincidence that was
> relying on the HID device to already be opened upon resume. This means
> that so long as anything was reading the /dev/input/eventX node for for
> an RMI device, it would suspend and resume correctly. As well, if
> nothing happened to be keeping the HID device away it would shut off,
> then the RMI driver would get confused on resume when it stopped
> responding and explode.

Oh, good finding. However, given that there are few other drivers not
calling hid_hw_open during their .reset_resume() callback and those
drivers also are communicating with the device, I wonder if we should
not have something more generic, that will call hid_hw_open/close in the
transport layer directly.

I do not recall having seen bugs for Wacom devices, so maybe this is
something i2c-hid related, but it wouldn't hurt I guess to open/close
the device before calling reset_resume.

Cheers,
Benjamin

>
> So, call hid_hw_open() in rmi_post_resume() so we make sure that the
> device is alive before we try talking to it.
>
> This fixes RMI device suspend/resume over HID.
>
> Signed-off-by: Lyude <lyude@xxxxxxxxxx>
> Cc: Andrew Duggan <aduggan@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/hid/hid-rmi.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index 5b40c2614599..e7d124f9a27f 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -431,22 +431,29 @@ static int rmi_post_resume(struct hid_device *hdev)
> {
> struct rmi_data *data = hid_get_drvdata(hdev);
> struct rmi_device *rmi_dev = data->xport.rmi_dev;
> - int ret;
> + int ret = 0;
>
> if (!(data->device_flags & RMI_DEVICE))
> return 0;
>
> - ret = rmi_reset_attn_mode(hdev);
> + /* Make sure the HID device is ready to receive events */
> + ret = hid_hw_open(hdev);
> if (ret)
> return ret;
>
> + ret = rmi_reset_attn_mode(hdev);
> + if (ret)
> + goto out;
> +
> ret = rmi_driver_resume(rmi_dev, false);
> if (ret) {
> hid_warn(hdev, "Failed to resume device: %d\n", ret);
> - return ret;
> + goto out;
> }
>
> - return 0;
> +out:
> + hid_hw_close(hdev);
> + return ret;
> }
> #endif /* CONFIG_PM */
>
> --
> 2.13.3
>