Add suspend/resume hooks for HID drivers so these can do some
additional state adjustment when device gets suspended/resumed.
v3:
- Pass full message to hid driver's suspend hook
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 <[email protected]>
---
Note, I've not added PM parts to bluetooth as I'm not sure how/where
to catch the PM events generated by bluetooth.
It looks like PM on bluetooth side does just some bluetooth housekeeping
and sends an event.
Marcel, do you have some pointers for wiring up these HID suspend hooks
in the bluetooth/hidp code?
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..17eb3ec 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, message);
+ 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, message);
+ 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..069e587 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, pm_message_t message);
+ int (*resume)(struct hid_device *hdev);
+ int (*reset_resume)(struct hid_device *hdev);
+#endif
/* private: */
struct device_driver driver;
};
--
1.6.4.4
Add PM support in order to turn off backlight on suspend, restore
it on resume and especially restore complete state on reset-resume.
Signed-off-by: Bruno Prémont <[email protected]>
---
drivers/hid/hid-picolcd.c | 61 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index aa6f2e1..c652390 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -852,6 +852,20 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data)
return picolcd_set_brightness(data->backlight);
}
+#ifdef CONFIG_PM
+static void picolcd_suspend_backlight(struct picolcd_data *data)
+{
+ int bl_power = data->lcd_power;
+ if (!data->backlight)
+ return;
+
+ data->backlight->props.power = FB_BLANK_POWERDOWN;
+ picolcd_set_brightness(data->backlight);
+ data->lcd_power = data->backlight->props.power = bl_power;
+}
+#else
+#define picolcd_suspend_backlight(a)
+#endif /* CONFIG_PM */
#else
static inline int picolcd_init_backlight(struct picolcd_data *data,
struct hid_report *report)
@@ -865,6 +879,7 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data)
{
return 0;
}
+#define picolcd_suspend_backlight(a)
#endif /* CONFIG_HID_PICOLCD_BACKLIGHT */
#ifdef CONFIG_HID_PICOLCD_LCD
@@ -2259,6 +2274,46 @@ static int picolcd_raw_event(struct hid_device *hdev,
return 1;
}
+#ifdef CONFIG_PM
+static int picolcd_suspend(struct hid_device *hdev, pm_message_t message)
+{
+ if (message.event & PM_EVENT_AUTO)
+ return 0;
+
+ picolcd_suspend_backlight(hid_get_drvdata(hdev));
+ dbg_hid(PICOLCD_NAME " device ready for suspend\n");
+ return 0;
+}
+
+static int picolcd_resume(struct hid_device *hdev)
+{
+ int ret;
+ ret = picolcd_resume_backlight(hid_get_drvdata(hdev));
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring backlight failed: %d\n", ret);
+ return 0;
+}
+
+static int picolcd_reset_resume(struct hid_device *hdev)
+{
+ int ret;
+ ret = picolcd_reset(hdev);
+ if (ret)
+ dbg_hid(PICOLCD_NAME " resetting our device failed: %d\n", ret);
+ ret = picolcd_fb_reset(hid_get_drvdata(hdev), 0);
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring framebuffer content failed: %d\n", ret);
+ ret = picolcd_resume_lcd(hid_get_drvdata(hdev));
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring lcd failed: %d\n", ret);
+ ret = picolcd_resume_backlight(hid_get_drvdata(hdev));
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring backlight failed: %d\n", ret);
+ picolcd_leds_set(hid_get_drvdata(hdev));
+ return 0;
+}
+#endif
+
/* initialize keypad input device */
static int picolcd_init_keys(struct picolcd_data *data,
struct hid_report *report)
@@ -2544,6 +2599,11 @@ static struct hid_driver picolcd_driver = {
.probe = picolcd_probe,
.remove = picolcd_remove,
.raw_event = picolcd_raw_event,
+#ifdef CONFIG_PM
+ .suspend = picolcd_suspend,
+ .resume = picolcd_resume,
+ .reset_resume = picolcd_reset_resume,
+#endif
};
static int __init picolcd_init(void)
--
1.6.4.4
On Sun, 25 Apr 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.
>
> v3:
> - Pass full message to hid driver's suspend hook
> v2:
> - Adds auto_suspend parameter to suspend hook
> - Only calls HID driver's resume hooks when previous code
> did succeed
>
I have applied this to 'hid-suspend' branch of my tree for now. Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Sun, 25 Apr 2010, Bruno Prémont wrote:
> Add PM support in order to turn off backlight on suspend, restore
> it on resume and especially restore complete state on reset-resume.
>
> Signed-off-by: Bruno Prémont <[email protected]>
> ---
> drivers/hid/hid-picolcd.c | 61 +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
> index aa6f2e1..c652390 100644
> --- a/drivers/hid/hid-picolcd.c
> +++ b/drivers/hid/hid-picolcd.c
> @@ -852,6 +852,20 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data)
> return picolcd_set_brightness(data->backlight);
> }
>
> +#ifdef CONFIG_PM
> +static void picolcd_suspend_backlight(struct picolcd_data *data)
> +{
> + int bl_power = data->lcd_power;
> + if (!data->backlight)
> + return;
> +
> + data->backlight->props.power = FB_BLANK_POWERDOWN;
> + picolcd_set_brightness(data->backlight);
> + data->lcd_power = data->backlight->props.power = bl_power;
> +}
> +#else
> +#define picolcd_suspend_backlight(a)
> +#endif /* CONFIG_PM */
Stylistic thing -- it would be nice if this was actually
static inline void picolcd_suspend_backlight(struct picolcd_data *data)
{
return 0;
}
But why do you need to have it defined in !CONFIG_PM situation anyway?
It's not used otherwise at all.
> #else
> static inline int picolcd_init_backlight(struct picolcd_data *data,
> struct hid_report *report)
> @@ -865,6 +879,7 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data)
> {
> return 0;
> }
> +#define picolcd_suspend_backlight(a)
Any why is it defined once more here?
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, 27 April 2010 Jiri Kosina <[email protected]> wrote:
> On Sun, 25 Apr 2010, Bruno Prémont wrote:
>
> > Add PM support in order to turn off backlight on suspend, restore
> > it on resume and especially restore complete state on reset-resume.
> >
> > Signed-off-by: Bruno Prémont <[email protected]>
> > ---
> > drivers/hid/hid-picolcd.c | 61 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 61 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
> > index aa6f2e1..c652390 100644
> > --- a/drivers/hid/hid-picolcd.c
> > +++ b/drivers/hid/hid-picolcd.c
> > @@ -852,6 +852,20 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data)
> > return picolcd_set_brightness(data->backlight);
> > }
> >
> > +#ifdef CONFIG_PM
> > +static void picolcd_suspend_backlight(struct picolcd_data *data)
> > +{
> > + int bl_power = data->lcd_power;
> > + if (!data->backlight)
> > + return;
> > +
> > + data->backlight->props.power = FB_BLANK_POWERDOWN;
> > + picolcd_set_brightness(data->backlight);
> > + data->lcd_power = data->backlight->props.power = bl_power;
> > +}
> > +#else
> > +#define picolcd_suspend_backlight(a)
> > +#endif /* CONFIG_PM */
>
> Stylistic thing -- it would be nice if this was actually
>
> static inline void picolcd_suspend_backlight(struct picolcd_data *data)
> {
> return 0;
> }
>
> But why do you need to have it defined in !CONFIG_PM situation anyway?
> It's not used otherwise at all.
Yeah, I could drop the #else branch (of #ifconfig CONFIG_PM).
Is gcc quiet about defined but unused static inline functions?
It's in order to avoid those warnings that I used #define bla().
(same in other occasions of stubs returning void.
Might be I forgot the inline keyword for the case I got the warning
about defined but unused function. Will check tomorrow.
> > #else
> > static inline int picolcd_init_backlight(struct picolcd_data *data,
> > struct hid_report *report)
> > @@ -865,6 +879,7 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data)
> > {
> > return 0;
> > }
> > +#define picolcd_suspend_backlight(a)
>
> Any why is it defined once more here?
Thats in the case where CONFIG_HID_PICOLCD_BACKLIGHT is not defined.
c.f. the #else at the beginning of this quote.
Thanks,
Bruno
On Tue, 27 Apr 2010, Bruno Prémont wrote:
> > > +#ifdef CONFIG_PM
> > > +static void picolcd_suspend_backlight(struct picolcd_data *data)
> > > +{
> > > + int bl_power = data->lcd_power;
> > > + if (!data->backlight)
> > > + return;
> > > +
> > > + data->backlight->props.power = FB_BLANK_POWERDOWN;
> > > + picolcd_set_brightness(data->backlight);
> > > + data->lcd_power = data->backlight->props.power = bl_power;
> > > +}
> > > +#else
> > > +#define picolcd_suspend_backlight(a)
> > > +#endif /* CONFIG_PM */
> >
> > Stylistic thing -- it would be nice if this was actually
> >
> > static inline void picolcd_suspend_backlight(struct picolcd_data *data)
> > {
> > return 0;
> > }
> >
> > But why do you need to have it defined in !CONFIG_PM situation anyway?
> > It's not used otherwise at all.
>
> Yeah, I could drop the #else branch (of #ifconfig CONFIG_PM).
Yup, please do so.
> Is gcc quiet about defined but unused static inline functions?
Yup, -Wunused-function doesn't warn about unused static inline function.
--
Jiri Kosina
SUSE Labs, Novell Inc.