On Mon, 29 Mar 2010, Bruno Prémont wrote:

quoted

quoted

+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:

quoted

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.

Ah, OK, I have missed that one, thanks for pointing it out.

The driver itself now looks fine to me and I am willing to merge it, 
modulo the power-management code -- so please either remove the parts that 
depend on power management for now, or I'll wait on your respin of the 
generic HID-PM patch.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html