Add suspend/resume hooks for HID drivers so these can do some
additional state adjustment when device gets suspended/resumed.
This patch calls these hooks from usbhid suspend/resume functions,
only calling suspend on plain suspend, not autosuspend.
(it might be worth adding an autosuspend parameter to suspend
hook and calling suspend in both cases)
Signed-off-by: Bruno Prémont <[email protected]>
---
drivers/hid/hid-core.c | 19 ++-
include/linux/hid.h | 8 +
2 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index e2997a8..8ad1a98 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1272,6 +1272,11 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
}
} else {
+ if (hid->driver && hid->driver->suspend) {
+ status = hid->driver->suspend(hid);
+ if (status < 0)
+ return status;
+ }
spin_lock_irq(&usbhid->lock);
set_bit(HID_REPORTED_IDLE, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
@@ -1326,6 +1331,11 @@ static int hid_resume(struct usb_interface *intf)
hid_io_error(hid);
usbhid_restart_queues(usbhid);
+ if (hid->driver && hid->driver->resume) {
+ int ret = hid->driver->resume(hid);
+ if (ret < 0 && status == 0)
+ status = ret;
+ }
dev_dbg(&intf->dev, "resume status %d\n", status);
return 0;
}
@@ -1334,9 +1344,16 @@ static int hid_reset_resume(struct usb_interface *intf)
{
struct hid_device *hid = usb_get_intfdata(intf);
struct usbhid_device *usbhid = hid->driver_data;
+ int status;
clear_bit(HID_REPORTED_IDLE, &usbhid->iofl);
- return hid_post_reset(intf);
+ status = hid_post_reset(intf);
+ if (hid->driver && hid->driver->reset_resume) {
+ int ret = hid->driver->reset_resume(hid);
+ if (ret < 0 && status == 0)
+ status = ret;
+ }
+ return status;
}
#endif /* CONFIG_PM */
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8709365..b4409f1 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -589,6 +589,9 @@ struct hid_usage_id {
* @report_fixup: called before report descriptor parsing (NULL means nop)
* @input_mapping: invoked on input registering before mapping an usage
* @input_mapped: invoked on input registering after mapping an usage
+ * @suspend: invoked on suspend (NULL means nop)
+ * @resume: invoked on resume if device was not reset (NULL means nop)
+ * @reset_resume: invoked on resume if device was reset (NULL means nop)
*
* raw_event and event should return 0 on no action performed, 1 when no
* further processing should be done and negative on error
@@ -629,6 +632,11 @@ struct hid_driver {
int (*input_mapped)(struct hid_device *hdev,
struct hid_input *hidinput, struct hid_field *field,
struct hid_usage *usage, unsigned long **bit, int *max);
+#ifdef CONFIG_PM
+ int (*suspend)(struct hid_device *hdev);
+ int (*resume)(struct hid_device *hdev);
+ int (*reset_resume)(struct hid_device *hdev);
+#endif
/* private: */
struct device_driver driver;
};
Am Mittwoch, 24. Februar 2010 17:01:12 schrieb Bruno Prémont:
> Add suspend/resume hooks for HID drivers so these can do some
> additional state adjustment when device gets suspended/resumed.
>
> This patch calls these hooks from usbhid suspend/resume functions,
> only calling suspend on plain suspend, not autosuspend.
> (it might be worth adding an autosuspend parameter to suspend
> hook and calling suspend in both cases)
This is quite dirty. A driver that was autosuspended may be non-auto resumed.
Secondly, do you really want to call the hook for reset_resume() if
hid_post_reset() has failed?
Regards
Oliver
On Thu, 25 February 2010 Oliver Neukum <[email protected]> wrote:
> Am Mittwoch, 24. Februar 2010 17:01:12 schrieb Bruno Prémont:
> > Add suspend/resume hooks for HID drivers so these can do some
> > additional state adjustment when device gets suspended/resumed.
> >
> > This patch calls these hooks from usbhid suspend/resume functions,
> > only calling suspend on plain suspend, not autosuspend.
> > (it might be worth adding an autosuspend parameter to suspend
> > hook and calling suspend in both cases)
>
> This is quite dirty.
Yeah, it covers what I did need (at least for success path). For the
rest I was expecting feedback (and probably should have labeled the
patch RFC)
> A driver that was autosuspended may be non-auto resumed. Secondly, do
> you really want to call the hook for reset_resume() if
> hid_post_reset() has failed?
Possibly not though depending on why hid_post_reset failed the first
USB operation would fail as well, thus it would fall under "normal"
error conditions for the driver...
Opinion of USB/HID experts is welcome!
> Regards
> Oliver
Thanks for the review!
Regards,
Bruno