On Monday, July 29, 2013 09:04:41 PM Felipe Balbi wrote:

Hi,

On Mon, Jul 29, 2013 at 07:45:09PM +0300, Illia Smyrnov wrote:

quoted

Enable/disable IRQ wake in suspend/resume handlers
to make the keypad wakeup capable.

Signed-off-by: Illia Smyrnov <redacted>
---

 drivers/input/keyboard/omap4-keypad.c |   43
 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)

diff --git a/drivers/input/keyboard/omap4-keypad.c

b/drivers/input/keyboard/omap4-keypad.c index 0244262..feab00f 100644

--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c

@@ -74,6 +74,7 @@ struct omap4_keypad {

 	struct input_dev *input;
 	
 	void __iomem *base;

+	bool irq_wake_enabled;

this flag is a bit weird... but I can't find a better way to handle this
situation. In one way, you shouldn't prevent system suspend, so you can
error out in case enable_irq_wake() fails, otoh if enable_irq_wake()
fails and you return 0, on resume disable_irq_wake() will throw
unbalanced calls warning. Maybe someone else has a better idea.

quoted

@@ -439,12 +444,50 @@ static const struct of_device_id

omap_keypad_dt_match[] = {> 
 MODULE_DEVICE_TABLE(of, omap_keypad_dt_match);
 #endif

+#ifdef CONFIG_PM_SLEEP
+static int omap4_keypad_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);

you don't need to access the platform_device...

quoted

+	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);

... since this can become:

	struct omap4_keypad *keypad_data = dev_get_drvdata(dev);

No, please use correct accessors for the objects. Platform drivers deal
with platform devices and I prefer using platform_get_drvdata() on them.

Thanks.

-- 
Dmitry