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 <ly...@redhat.com>
> Cc: Andrew Duggan <adug...@synaptics.com>
> Cc: sta...@vger.kernel.org
> ---
>  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
> 

Reply via email to