hid: add suspend/resume hooks for hid drivers
in [Kernel]
|
From: Jiri Kosina on 31 Mar 2010 08:20 On Tue, 30 Mar 2010, Bruno Prémont wrote: > 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 <bonbons(a)linux-vserver.org> > --- > > Note: > this patch needs improvements as mentionned by Olivier Neukum: > - suspend hook for both system suspend and autosuspend > - no call of hook on USB-HID-resume failure I agree with Oliver's comments. We should also consider putting the hook calls into bluetooth implementation as well (though there is currently no driver using it, but I don't know, maybe Wacom could benefit from it as well). Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Bruno Prémont on 11 Apr 2010 07:10 On Wed, 31 March 2010 Jiri Kosina <jkosina(a)suse.cz> wrote: > On Tue, 30 Mar 2010, Bruno Prémont wrote: > > > 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 <bonbons(a)linux-vserver.org> > > --- > > > > Note: > > this patch needs improvements as mentionned by Olivier Neukum: > > - suspend hook for both system suspend and autosuspend > > - no call of hook on USB-HID-resume failure > > I agree with Oliver's comments. > > We should also consider putting the hook calls into bluetooth > implementation as well (though there is currently no driver using it, but > I don't know, maybe Wacom could benefit from it as well). Where do I find the bluetooth HID bits? I've been searching through kernel sources (under drivers/) but did not find bluetooth code trying to register a HID device... Below an updated patch which also calls the hook for auto-suspend case. The patch compiles but I've not runtime-tested it yet (especially for the auto-suspend part, as I'm not sure what should trigger it). As far as I understand the code, in auto-suspend case if the driver sends commands to the device it will have to call usbhid_wait_io(). Please correct me if I'm wrong! Thanks, Bruno --- Add suspend/resume hooks for HID drivers so these can do some additional state adjustment when device gets suspended/resumed. v2: - Adds auto_suspend parameter to suspend hook - Only calls HID driver's resume hooks when previous code did succeed Signed-off-by: Bruno Prémont <bonbons(a)linux-vserver.org> --- drivers/hid/usbhid/hid-core.c | 24 +++++++++++++++++++++++- include/linux/hid.h | 8 ++++++++ 2 files changed, 31 insertions(+), 1 deletions(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 3e7909b..1f40695 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1291,6 +1291,11 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message) { set_bit(HID_REPORTED_IDLE, &usbhid->iofl); spin_unlock_irq(&usbhid->lock); + if (hid->driver && hid->driver->suspend) { + status = hid->driver->suspend(hid, 1); + if (status < 0) + return status; + } } else { usbhid_mark_busy(usbhid); spin_unlock_irq(&usbhid->lock); @@ -1298,6 +1303,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, 0); + if (status < 0) + return status; + } spin_lock_irq(&usbhid->lock); set_bit(HID_REPORTED_IDLE, &usbhid->iofl); spin_unlock_irq(&usbhid->lock); @@ -1352,6 +1362,11 @@ static int hid_resume(struct usb_interface *intf) hid_io_error(hid); usbhid_restart_queues(usbhid); + if (status >= 0 && hid->driver && hid->driver->resume) { + int ret = hid->driver->resume(hid); + if (ret < 0) + status = ret; + } dev_dbg(&intf->dev, "resume status %d\n", status); return 0; } @@ -1360,9 +1375,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 (status >= 0 && hid->driver && hid->driver->reset_resume) { + int ret = hid->driver->reset_resume(hid); + if (ret < 0) + status = ret; + } + return status; } #endif /* CONFIG_PM */ diff --git a/include/linux/hid.h b/include/linux/hid.h index b1344ec..a2f3612 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 auto_suspend); + int (*resume)(struct hid_device *hdev); + int (*reset_resume)(struct hid_device *hdev); +#endif /* private: */ struct device_driver driver; }; -- 1.6.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Kosina on 11 Apr 2010 14:40 On Sun, 11 Apr 2010, Bruno Prémont wrote: > > We should also consider putting the hook calls into bluetooth > > implementation as well (though there is currently no driver using it, but > > I don't know, maybe Wacom could benefit from it as well). > > Where do I find the bluetooth HID bits? I've been searching through kernel > sources (under drivers/) but did not find bluetooth code trying to > register a HID device... They are in net/bluetooth/hidp. Not really obvious, I know, but Marcel wanted it this way. > As far as I understand the code, in auto-suspend case if the driver > sends commands to the device it will have to call usbhid_wait_io(). > Please correct me if I'm wrong! It's not needed. There is a queuing mechanism in place on 'resumption_waker' workqueue. -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Bruno Prémont on 11 Apr 2010 14:50 On Sun, 11 April 2010 Jiri Kosina <jkosina(a)suse.cz> wrote: > > As far as I understand the code, in auto-suspend case if the driver > > sends commands to the device it will have to call usbhid_wait_io(). > > Please correct me if I'm wrong! > > It's not needed. There is a queuing mechanism in place on > 'resumption_waker' workqueue. So the driver is not allowed to ask the device to do something at auto-suspend time (as that would abort the auto-suspension). Am I right? Thanks, Bruno -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Kosina on 12 Apr 2010 07:50
On Sun, 11 Apr 2010, Bruno Pr�mont wrote: > > > As far as I understand the code, in auto-suspend case if the driver > > > sends commands to the device it will have to call usbhid_wait_io(). > > > Please correct me if I'm wrong! > > > > It's not needed. There is a queuing mechanism in place on > > 'resumption_waker' workqueue. > > So the driver is not allowed to ask the device to do something > at auto-suspend time (as that would abort the auto-suspension). What do you mean by "is not allowed" exactly? Yes, it will wake the device up from auto-suspension. However, that's usually what you want to happen, is there is anything you need to dispatch to the device, isn't it? What exactly is the situation you are trying to handle? Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |