Re: [PATCH v4 1/6] hid: new driver for PicoLCD device

["Bruno Prémont" <bonbons@linux-vserver.org>]

On Mon, 29 Mar 2010 11:47:27 +0200 Jiri Kosina <jkosina@suse.cz> wrote:
> On Sat, 27 Mar 2010, Bruno Prémont wrote:
> 
> > +#ifdef CONFIG_PM
> > +static int picolcd_suspend(struct hid_device *hdev)
> > +{
> > +	dbg_hid(PICOLCD_NAME " device ready for suspend\n");
> > +	return 0;
> > +}
> > +
> > +static int picolcd_resume(struct hid_device *hdev)
> > +{
> > +	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);
> > +	return 0;
> > +}
> > +#endif
> [ ... ]
> > +static struct hid_driver picolcd_driver = {
> > +	.name =          "hid-picolcd",
> > +	.id_table =      picolcd_devices,
> > +	.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
> > +};
> 
> struct hid_driver doesn't provide power-management related callbacks.
> So I guess that you have either not tested this feature at all, or
> you have some extra patch somewhere which adds such callbacks to HID
> core, but you haven't sent it out for review?
> 
> In any cases, this will very likely cause compilation failure with 
> CONFIG_PM turned on.

That's the patch I referred to (under series introduction mail) when
saying:
> The series depends on my previous patch adding HID suspend support
> (I've not yet looked at improving it).

Link to the patch:
  http://lkml.org/lkml/2010/2/24/233

Especially the reset-resume part is important as the device has to be
reprogrammed with framebuffer content, brightness/contrast in that case.


As stated, Oliver didn't like the implementation of the addition of
those hooks too much and I have yet to look at improving it.

Thanks,
Bruno