On Wed, Oct 26, 2011 at 04:41:41PM +0800, Hui Wang wrote: > Sascha Hauer wrote: > >On Thu, Oct 13, 2011 at 10:13:33AM +0800, Hui Wang wrote: > >>The imx_keypad driver is set wake capable in the imx_keypad_probe(), > >>but it doesn't implement suspend and reusme callback interface. > >> > >>From the i.MX series MCU Reference Manual, the kpp (keypad port) is > >>a major wake up source which can detect any key press even in low > >>power modes and even when there is no clock. > >> > >>Now add suspend and resume callback functions for this driver. > >> > >>Signed-off-by: Hui Wang <jason77.wang@xxxxxxxxx> > >>--- > >> > >>Sorry for resending this patch, the previous sent patch with [v2] > >>as a prefix, i am afraid that patch was filtered out from mailing list. > >>Now resending this patch with [PATCH v2] as a prefix. > >> > >>In the V2: > >>1) add a check for input->users before disable clock. > >>2) add mutex_lock/unlock to protect input->users checking. > >>3) use SIMPLE_DEV_PM_OPS. > >> > >> drivers/input/keyboard/imx_keypad.c | 46 +++++++++++++++++++++++++++++++++++ > >> 1 files changed, 46 insertions(+), 0 deletions(-) > >> > >>diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c > >>index d92c15c..c43fa97 100644 > >>--- a/drivers/input/keyboard/imx_keypad.c > >>+++ b/drivers/input/keyboard/imx_keypad.c > >>@@ -567,10 +567,56 @@ static int __devexit imx_keypad_remove(struct platform_device *pdev) > >> return 0; > >> } > >>+#ifdef CONFIG_PM > >>+static int imx_kbd_suspend(struct device *dev) > >>+{ > >>+ struct platform_device *pdev = to_platform_device(dev); > >>+ struct imx_keypad *kbd = platform_get_drvdata(pdev); > >>+ struct input_dev *input_dev = kbd->input_dev; > >>+ > >>+ /* imx kbd can wake up system even clock is disabled */ > >>+ mutex_lock(&input_dev->mutex); > >>+ > >>+ if (input_dev->users) > >>+ clk_disable(kbd->clk); > >>+ > >>+ mutex_unlock(&input_dev->mutex); > >>+ > >>+ if (device_may_wakeup(&pdev->dev)) > >>+ enable_irq_wake(kbd->irq); > > > >enable_irq_wake can fail. More specifically it *will* fail on all non > >tzic i.MX boards as this function is not yet implemented for the avic > >irq controller. > >You should check the return value. > > > Hi Sascha, > > I have sent a patch to change plat-mxc/avic.c to use generic irq > chip, in this patch, two lines to handle wakeup_set issues: > > gc->wake_enabled = IRQ_MSK(32); > ct->chip.irq_set_wake = irq_gc_set_wake; > > These two lines will make enable_irq_wake successful unconditionally. > > And i recall you have sent this patch to Arnd through a pull request. Ok, so currently this function can't fail, but this a very bad excuse for not checking error values. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html