input/imx_keypad: add PM support

PATCH Superseded Eric Bénard 5 participants 19 messages Jun 17, 2010 – Jul 17, 2010

Subsystems (9)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Memory technology devices (MTD) +5 more subsystems

lore

Share & tools

lore

Eric Bénard

author

2010-06-17 18:59 UTC #0 lore raw

PATCH v2 1/1 input/imx_keypad: add PM support

We use keypad_data to enable wakeup from platform data

suspend/resume is tested on an i.MX27 when keypad is not in use and when it's is in use (in which case we immediatly get the key pressed at wakeup)

Signed-off-by Eric B?nard <[email protected]>

Cc maramaopercheseimorto at gmail.com

Cc linux-input at vger.kernel.org

Cc dmitry.torokhov at gmail.com

Cc linux-arm-kernel at lists.infradead.org

---
v2 :
- simplify suspend & resume functions
- convert to pm_ops

 drivers/input/keyboard/imx_keypad.c |   40 +++++++++++++++++++++++++++++++++-
 1 files changed, 38 insertions(+), 2 deletions(-)

drivers/input/keyboard/imx_keypad.c +38 -2

index d92c15c..9aa0005 100644
@@ -364,6 +364,38 @@ static void imx_keypad_inhibit(struct imx_keypad *keypad)
 	writew(0xff00, keypad->mmio_base + KPCR);
 }
 
+#ifdef CONFIG_PM
+static int mxc_kpp_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+
+	keypad->enabled = false;
+	synchronize_irq(keypad->irq);
+	disable_irq(keypad->irq);
+	clk_disable(keypad->clk);
+
+	return 0;
+}
+
+static int mxc_kpp_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+
+	keypad->enabled = true;
+	clk_enable(keypad->clk);
+	enable_irq(keypad->irq);
+
+	return 0;
+}
+
+static const struct dev_pm_ops mxc_kpp_pm_ops = {
+	.suspend	= mxc_kpp_suspend,
+	.resume		= mxc_kpp_resume,
+};
+#endif
+
 static void imx_keypad_close(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);

@@ -410,7 +442,8 @@ open_err:
 
 static int __devinit imx_keypad_probe(struct platform_device *pdev)
 {
-	const struct matrix_keymap_data *keymap_data = pdev->dev.platform_data;
+	const struct matrix_keypad_platform_data *keypad_data = pdev->dev.platform_data;
+	const struct matrix_keymap_data *keymap_data = keypad_data->keymap_data;
 	struct imx_keypad *keypad;
 	struct input_dev *input_dev;
 	struct resource *res;

@@ -525,7 +558,7 @@ static int __devinit imx_keypad_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, keypad);
-	device_init_wakeup(&pdev->dev, 1);
+	device_init_wakeup(&pdev->dev, keypad_data->wakeup);
 
 	return 0;

@@ -571,6 +604,9 @@ static struct platform_driver imx_keypad_driver = {
 	.driver		= {
 		.name	= "imx-keypad",
 		.owner	= THIS_MODULE,
+#ifdef CONFIG_PM
+		.pm	= &mxc_kpp_pm_ops,
+#endif
 	},
 	.probe		= imx_keypad_probe,
 	.remove		= __devexit_p(imx_keypad_remove),

Signature · 1 line

1.6.3.3

Eric Bénard

author

2010-06-17 18:59 UTC #1 lore raw

PATCH 1/1 eukrea_mbimx27: update keyboard platform data

use matrix_keypad_platform_data
enable wakeup

Signed-off-by Eric B?nard <[email protected]>

Cc s.hauer at pengutronix.de

Cc linux-arm-kernel at lists.infradead.org

---
 arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c +6 -1

index 80f6268..e7fd65a 100644
@@ -116,6 +116,11 @@ static struct matrix_keymap_data eukrea_mbimx27_keymap_data = {
 	.keymap_size    = ARRAY_SIZE(eukrea_mbimx27_keymap),
 };
 
+static struct matrix_keypad_platform_data eukrea_mbimx27_keypad_data = {
+	.keymap_data	= &eukrea_mbimx27_keymap_data,
+	.wakeup		= 1,
+};
+
 static struct gpio_led gpio_leds[] = {
 	{
 		.name			= "led1",

@@ -392,7 +397,7 @@ void __init eukrea_mbimx27_baseboard_init(void)
 	gpio_request(GPIO_PORTA | 25, "lcd_enable");
 	platform_device_register(&eukrea_mbimx27_lcd_powerdev);
 
-	mxc_register_device(&imx_kpp_device, &eukrea_mbimx27_keymap_data);
+	mxc_register_device(&imx_kpp_device, &eukrea_mbimx27_keypad_data);
 
 	platform_add_devices(platform_devices, ARRAY_SIZE(platform_devices));
 }

Signature · 1 line

1.6.3.3

Eric Bénard

author

2010-06-17 18:59 UTC #2 lore raw

PATCH 1/1 eukrea_mbimx27-baseboard: fix compile when CONFIG_SPI_IMX is disabled

the i2c_board_info structure landed into the SPI #if/#endif which is wrong.

Signed-off-by Eric B?nard <[email protected]>

Cc s.hauer at pengutronix.de

Cc linux-arm-kernel at lists.infradead.org

---
 arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c +5 -5

index e7fd65a..2e57912 100644
@@ -301,11 +301,6 @@ static struct spi_board_info eukrea_mbimx27_spi_board_info[] __initdata = {
 	},
 };
 
-static struct i2c_board_info eukrea_mbimx27_i2c_devices[] = {
-	{
-		I2C_BOARD_INFO("tlv320aic23", 0x1a),
-	},
-};
 
 static int eukrea_mbimx27_spi_cs[] = {GPIO_PORTD | 28};

@@ -315,6 +310,11 @@ static struct spi_imx_master eukrea_mbimx27_spi_0_data = {
 };
 #endif
 
+static struct i2c_board_info eukrea_mbimx27_i2c_devices[] = {
+	{
+		I2C_BOARD_INFO("tlv320aic23", 0x1a),
+	},
+};
 static struct platform_device *platform_devices[] __initdata = {
 	&leds_gpio,
 };

Signature · 1 line

1.6.3.3

Eric Bénard

author

2010-06-17 18:59 UTC #3 lore raw

PATCH v2 1/1 mtd/mxc_nand: don't call mtd->{suspend,resume} explicitly

The mtd layer is responsible to call mtd->{suspend,resume}. Doing it again in the driver is wrong and results in a warning:

nand_resume called for a chip which is not in suspended state

at resume time. Removing the calls from the resume and suspend functions makes them empty allowing them to be deleted completely.

Signed-off-by Eric B?nard <[email protected]>

Acked-by Uwe Kleine-K?nig <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc linux-arm-kernel at lists.infradead.org

Cc linux-mtd at lists.infradead.org

---
v2:
fix commit log (writen by Uwe Kleine-K?nig)

 drivers/mtd/nand/mxc_nand.c |   44 +------------------------------------------
 1 files changed, 1 insertions(+), 43 deletions(-)

drivers/mtd/nand/mxc_nand.c +1 ~~-43~~

index 0d76b16..ad19dc6 100644
@@ -873,53 +873,11 @@ static int __devexit mxcnd_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int mxcnd_suspend(struct platform_device *pdev, pm_message_t state)
-{
-	struct mtd_info *mtd = platform_get_drvdata(pdev);
-	struct nand_chip *nand_chip = mtd->priv;
-	struct mxc_nand_host *host = nand_chip->priv;
-	int ret = 0;
-
-	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND suspend\n");
-
-	ret = mtd->suspend(mtd);
-
-	/*
-	 * nand_suspend locks the device for exclusive access, so
-	 * the clock must already be off.
-	 */
-	BUG_ON(!ret && host->clk_act);
-
-	return ret;
-}
-
-static int mxcnd_resume(struct platform_device *pdev)
-{
-	struct mtd_info *mtd = platform_get_drvdata(pdev);
-	struct nand_chip *nand_chip = mtd->priv;
-	struct mxc_nand_host *host = nand_chip->priv;
-	int ret = 0;
-
-	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND resume\n");
-
-	mtd->resume(mtd);
-
-	return ret;
-}
-
-#else
-# define mxcnd_suspend   NULL
-# define mxcnd_resume    NULL
-#endif				/* CONFIG_PM */
-
 static struct platform_driver mxcnd_driver = {
 	.driver = {
 		   .name = DRIVER_NAME,
-		   },
+	},
 	.remove = __devexit_p(mxcnd_remove),
-	.suspend = mxcnd_suspend,
-	.resume = mxcnd_resume,
 };
 
 static int __init mxc_nd_init(void)

Signature · 1 line

1.6.3.3

Eric Bénard

author

2010-06-17 18:59 UTC #4 lore raw

PATCH 1/1 video/imxfb: fix suspend when screen is blanked

if the screen is blanked when suspending, the clock is already disabled, leading to the following log :

WARNING: at arch/arm/plat-mxc/clock.c:59 __clk_disable+0x68/0x74()
Modules linked in:
[<c002d200>] (unwind_backtrace+0x0/0xf8) from [<c0046008>] (warn_slowpath_common+0x48/0x78)
[<c0046008>] (warn_slowpath_common+0x48/0x78) from [<c0031e28>] (__clk_disable+0x68/0x74)
[<c0031e28>] (__clk_disable+0x68/0x74) from [<c0031ddc>] (__clk_disable+0x1c/0x74)
[<c0031ddc>] (__clk_disable+0x1c/0x74) from [<c0031e58>] (clk_disable+0x24/0x34)
[<c0031e58>] (clk_disable+0x24/0x34) from [<c019808c>] (imxfb_disable_controller+0x38/0x48)
[<c019808c>] (imxfb_disable_controller+0x38/0x48) from [<c01980b0>] (imxfb_suspend+0x14/0x1c)

Signed-off-by Eric B?nard <[email protected]>

Cc kernel at pengutronix.de

Cc u.kleine-koenig at pengutronix.de

Cc linux-fbdev at vger.kernel.org

Cc linux-arm-kernel at lists.infradead.org

---
 drivers/video/imxfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/video/imxfb.c +2 -2

index b4b6dec..6cbfbff 100644
@@ -597,8 +597,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
-
-	imxfb_disable_controller(fbi);
+	if (readl(fbi->regs + LCDC_RMCR))
+		imxfb_disable_controller(fbi);
 	return 0;
 }

Signature · 1 line

1.6.3.3

Eric Bénard

author

2010-06-17 18:59 UTC #5 lore raw

PATCH 1/1 mach-mx3/mach-mx31_3ds: update keypad registration

depends on "input/imx_keypad: add PM support" Note : this patch is not tested

Signed-off-by Eric B?nard <[email protected]>

Cc s.hauer at pengutronix.de

Cc linux-arm-kernel at lists.infradead.org

Cc maramaopercheseimorto at gmail.com

---
 arch/arm/mach-mx3/mach-mx31_3ds.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

arch/arm/mach-mx3/mach-mx31_3ds.c +6 -1

index 58e5729..04c8700 100644
@@ -114,6 +114,11 @@ static struct matrix_keymap_data mx31_3ds_keymap_data = {
 	.keymap_size	= ARRAY_SIZE(mx31_3ds_keymap),
 };
 
+static struct matrix_keypad_platform_data mx31_3ds_keypad_data = {
+	.keymap_data	= &mx31_3ds_keymap_data,
+	.wakeup		= 1,
+};
+
 /* Regulators */
 static struct regulator_init_data pwgtx_init = {
 	.constraints = {

@@ -397,7 +402,7 @@ static void __init mxc_board_init(void)
 	spi_register_board_info(mx31_3ds_spi_devs,
 						ARRAY_SIZE(mx31_3ds_spi_devs));
 
-	mxc_register_device(&imx_kpp_device, &mx31_3ds_keymap_data);
+	mxc_register_device(&imx_kpp_device, &mx31_3ds_keypad_data);
 
 	mx31_3ds_usbotg_init();
 	mxc_register_device(&mxc_otg_udc_device, &usbotg_pdata);

Signature · 1 line

1.6.3.3

Eric Bénard

author

2010-06-17 18:59 UTC #6 lore raw

PATCH 1/1 mxcmmc: convert to pm_ops and enable/disable clock

Signed-off-by Eric B?nard <[email protected]>

Cc s.hauer at pengutronix.de

Cc linux-mmc at vger.kernel.org

Cc linux-arm-kernel at lists.infradead.org

---
 drivers/mmc/host/mxcmmc.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

drivers/mmc/host/mxcmmc.c +18 ~~-14~~

index fdf33e8..350f78e 100644
@@ -936,43 +936,47 @@ static int mxcmci_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int mxcmci_suspend(struct platform_device *dev, pm_message_t state)
+static int mxcmci_suspend(struct device *dev)
 {
-	struct mmc_host *mmc = platform_get_drvdata(dev);
+	struct mmc_host *mmc = dev_get_drvdata(dev);
+	struct mxcmci_host *host = mmc_priv(mmc);
 	int ret = 0;
 
 	if (mmc)
 		ret = mmc_suspend_host(mmc);
+	clk_disable(host->clk);
 
 	return ret;
 }
 
-static int mxcmci_resume(struct platform_device *dev)
+static int mxcmci_resume(struct device *dev)
 {
-	struct mmc_host *mmc = platform_get_drvdata(dev);
-	struct mxcmci_host *host;
+	struct mmc_host *mmc = dev_get_drvdata(dev);
+	struct mxcmci_host *host = mmc_priv(mmc);
 	int ret = 0;
 
-	if (mmc) {
-		host = mmc_priv(mmc);
+	clk_enable(host->clk);
+	if (mmc)
 		ret = mmc_resume_host(mmc);
-	}
 
 	return ret;
 }
-#else
-#define mxcmci_suspend  NULL
-#define mxcmci_resume   NULL
-#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops mxcmci_pm_ops = {
+	.suspend	= mxcmci_suspend,
+	.resume		= mxcmci_resume,
+};
+#endif
 
 static struct platform_driver mxcmci_driver = {
 	.probe		= mxcmci_probe,
 	.remove		= mxcmci_remove,
-	.suspend	= mxcmci_suspend,
-	.resume		= mxcmci_resume,
 	.driver		= {
 		.name		= DRIVER_NAME,
 		.owner		= THIS_MODULE,
+#ifdef CONFIG_PM
+		.pm	= &mxcmci_pm_ops,
+#endif
 	}
 };

Signature · 1 line

1.6.3.3

Luotao Fu

2010-06-17 19:22 UTC #7 lore raw

PATCH 1/1 video/imxfb: fix suspend when screen is blanked

In reply to Eric Bénard

Hi,

Quoted 25 lines · Eric B?nard

if the screen is blanked when suspending, the clock is already disabled, leading to the following log :

WARNING: at arch/arm/plat-mxc/clock.c:59 __clk_disable+0x68/0x74()
Modules linked in:
[<c002d200>] (unwind_backtrace+0x0/0xf8) from [<c0046008>] (warn_slowpath_common+0x48/0x78)
[<c0046008>] (warn_slowpath_common+0x48/0x78) from [<c0031e28>] (__clk_disable+0x68/0x74)
[<c0031e28>] (__clk_disable+0x68/0x74) from [<c0031ddc>] (__clk_disable+0x1c/0x74)
[<c0031ddc>] (__clk_disable+0x1c/0x74) from [<c0031e58>] (clk_disable+0x24/0x34)
[<c0031e58>] (clk_disable+0x24/0x34) from [<c019808c>] (imxfb_disable_controller+0x38/0x48)
[<c019808c>] (imxfb_disable_controller+0x38/0x48) from [<c01980b0>] (imxfb_suspend+0x14/0x1c)

Signed-off-by Eric B?nard <[email protected]>

Cc kernel at pengutronix.de

Cc u.kleine-koenig at pengutronix.de

Cc linux-fbdev at vger.kernel.org

Cc linux-arm-kernel at lists.infradead.org

---
 drivers/video/imxfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/video/imxfb.c +0 -0

index b4b6dec..6cbfbff 100644

drivers/video/imxfb.c static int imxfb_suspend(struct platform_device *dev, pm_message_t state) · line 603

Quoted 9 lines

drivers/video/imxfb.c +2 -2

@@ -597,8 +597,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
-
-	imxfb_disable_controller(fbi);
+	if (readl(fbi->regs + LCDC_RMCR))
+		imxfb_disable_controller(fbi);
 	return 0;
 }

I've already submitted a patch to fix this issue:
http://lkml.org/lkml/2010/6/3/187

cheers
Luotao Fu

Signature · 11 lines

Pengutronix e.K.                           | Dipl.-Ing. Luotao Fu        |
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 |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100617/0d8a4f3e/attachment.sig>

Eric Bénard

author

2010-06-18 06:30 UTC #8 lore raw

PATCH 1/1 video/imxfb: fix suspend when screen is blanked

Le 17/06/2010 21:22, Luotao Fu a ?crit :

Quoted 3 lines

I've already submitted a patch to fix this issue:
http://lkml.org/lkml/2010/6/3/187

I also already submited this same patch before :
http://permalink.gmane.org/gmane.linux.ports.arm.kernel/80975

Eric

Artem Bityutskiy

Applied

2010-07-07 14:17 UTC #9 lore raw

PATCH v2 1/1 mtd/mxc_nand: don't call mtd->{suspend,resume} explicitly

In reply to Eric Bénard

Quoted 10 lines · Eric B?nard

The mtd layer is responsible to call mtd->{suspend,resume}. Doing it again in the driver is wrong and results in a warning:
nand_resume called for a chip which is not in suspended state
at resume time. Removing the calls from the resume and suspend functions makes them empty allowing them to be deleted completely.
Signed-off-by Eric B?nard <[email protected]>
Acked-by Uwe Kleine-K?nig <[email protected]>

Pushed to l2-mtd-2.6.git / dunno

Signature · 2 lines

Best Regards,
Artem Bityutskiy (????? ????????)

Eric Bénard

author

2010-07-17 07:24 UTC #10 lore raw

PATCH v2 1/1 mtd/mxc_nand: don't call mtd->{suspend, resume} explicitly

In reply to Eric Bénard

Hi Artem,

can you also pick this patch for mxc_nand ?

Thanks
Eric

Le 17/06/2010 20:59, Eric B?nard a ?crit :

Quoted 79 lines

The mtd layer is responsible to call mtd->{suspend,resume}. Doing it again in the driver is wrong and results in a warning:

nand_resume called for a chip which is not in suspended state

at resume time. Removing the calls from the resume and suspend functions makes them empty allowing them to be deleted completely.

Signed-off-by Eric B?nard<[email protected]>

Acked-by Uwe Kleine-K?nig<[email protected]>

Cc Sascha Hauer<[email protected]>

Cc linux-arm-kernel at lists.infradead.org

Cc linux-mtd at lists.infradead.org

---
v2:
fix commit log (writen by Uwe Kleine-K?nig)

  drivers/mtd/nand/mxc_nand.c |   44 +------------------------------------------
  1 files changed, 1 insertions(+), 43 deletions(-)

drivers/mtd/nand/mxc_nand.c +1 ~~-43~~

index 0d76b16..ad19dc6 100644
@@ -873,53 +873,11 @@ static int __devexit mxcnd_remove(struct platform_device *pdev)
  	return 0;
  }
 
-#ifdef CONFIG_PM
-static int mxcnd_suspend(struct platform_device *pdev, pm_message_t state)
-{
-	struct mtd_info *mtd = platform_get_drvdata(pdev);
-	struct nand_chip *nand_chip = mtd->priv;
-	struct mxc_nand_host *host = nand_chip->priv;
-	int ret = 0;
-
-	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND suspend\n");
-
-	ret = mtd->suspend(mtd);
-
-	/*
-	 * nand_suspend locks the device for exclusive access, so
-	 * the clock must already be off.
-	 */
-	BUG_ON(!ret&&  host->clk_act);
-
-	return ret;
-}
-
-static int mxcnd_resume(struct platform_device *pdev)
-{
-	struct mtd_info *mtd = platform_get_drvdata(pdev);
-	struct nand_chip *nand_chip = mtd->priv;
-	struct mxc_nand_host *host = nand_chip->priv;
-	int ret = 0;
-
-	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND resume\n");
-
-	mtd->resume(mtd);
-
-	return ret;
-}
-
-#else
-# define mxcnd_suspend   NULL
-# define mxcnd_resume    NULL
-#endif				/* CONFIG_PM */
-
  static struct platform_driver mxcnd_driver = {
  	.driver = {
  		   .name = DRIVER_NAME,
-		   },
+	},
  	.remove = __devexit_p(mxcnd_remove),
-	.suspend = mxcnd_suspend,
-	.resume = mxcnd_resume,
  };
 
  static int __init mxc_nd_init(void)

Artem Bityutskiy

2010-07-17 07:33 UTC #11 lore raw

PATCH v2 1/1 mtd/mxc_nand: don't call mtd->{suspend,resume} explicitly

Quoted 3 lines · Eric B?nard

Hi Artem,
can you also pick this patch for mxc_nand ?

It is in my tree already, and I sent a notification as well :-)

Signature · 2 lines

Best Regards,
Artem Bityutskiy (????? ????????)

Eric Bénard

author

2010-07-17 12:49 UTC #12 lore raw

PATCH v2 1/1 mtd/mxc_nand: don't call mtd->{suspend, resume} explicitly

Hi Artem,

Le 17/07/2010 09:33, Artem Bityutskiy a ?crit :

Quoted 5 lines · Eric B?nard

Quoted 1 line
can you also pick this patch for mxc_nand ?
It is in my tree already, and I sent a notification as well :-)

OK, sorry, I didn't see the notification mail and only checked your master branch, not the dunno branch where you applied this patch !

Thanks,

Eric

Artem Bityutskiy

2010-07-17 15:13 UTC #13 lore raw

PATCH v2 1/1 mtd/mxc_nand: don't call mtd->{suspend,resume} explicitly

Quoted 11 lines · Eric B?nard

Hi Artem,
Le 17/07/2010 09:33, Artem Bityutskiy a ?crit :
Quoted 5 lines · Eric B?nard
Quoted 1 line
can you also pick this patch for mxc_nand ?
It is in my tree already, and I sent a notification as well :-)
OK, sorry, I didn't see the notification mail and only checked your master branch, not the dunno branch where you applied this patch !

Yeah, I put to master only the stuff I understand and hence, can sight-off. The rest goes to dunno - David then processes it.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (????? ????????)

Alberto Panizzo

2010-06-18 10:16 UTC #14 lore raw

PATCH v2 1/1 input/imx_keypad: add PM support

In reply to Eric Bénard

Hi Eric,

Quoted 42 lines · Eric B?nard

We use keypad_data to enable wakeup from platform data
suspend/resume is tested on an i.MX27 when keypad is not in use and when it's is in use (in which case we immediatly get the key pressed at wakeup)
Signed-off-by Eric B?nard <[email protected]>
Cc maramaopercheseimorto at gmail.com
Cc linux-input at vger.kernel.org
Cc dmitry.torokhov at gmail.com
Cc linux-arm-kernel at lists.infradead.org
---
v2 :
- simplify suspend & resume functions
- convert to pm_ops
 drivers/input/keyboard/imx_keypad.c |   40 +++++++++++++++++++++++++++++++++-
 1 files changed, 38 insertions(+), 2 deletions(-)
drivers/input/keyboard/imx_keypad.c +15 -0
index d92c15c..9aa0005 100644
@@ -364,6 +364,38 @@ static void imx_keypad_inhibit(struct imx_keypad *keypad)
 	writew(0xff00, keypad->mmio_base + KPCR);
 }
 
+#ifdef CONFIG_PM
+static int mxc_kpp_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+
+	keypad->enabled = false;
+	synchronize_irq(keypad->irq);
+	disable_irq(keypad->irq);
+	clk_disable(keypad->clk);
+
+	return 0;
+}
+
+static int mxc_kpp_resume(struct device *dev)

drivers/input/keyboard/imx_keypad.c static void imx_keypad_inhibit(struct imx_keypad *keypad) · line 391

Quoted 10 lines

drivers/input/keyboard/imx_keypad.c +10 -0

+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+
+	keypad->enabled = true;
+	clk_enable(keypad->clk);
+	enable_irq(keypad->irq);
+
+	return 0;
+}

Ok, so with this patch the keypad cannot wake the system? It is scheduled another patch?

Signature · 4 lines

Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

Eric Bénard

author

2010-06-18 14:37 UTC #15 lore raw

PATCH v2 1/1 input/imx_keypad: add PM support

Hi Alberto,

Le 18/06/2010 12:16, Alberto Panizzo a ?crit :

Quoted 3 lines

Ok, so with this patch the keypad cannot wake the system? It is scheduled another patch?

well, actually, the keypad wakes the system, at least on my board here.

I was quite surprised to get this working this way as I would have expected disable_irq + clk_disable would prevent the keypad from being a wake source.
Can you try on your board as this cleanup may be wrong on i.MX3 ?

Thanks,
Eric

Alberto Panizzo

2010-06-18 17:40 UTC #16 lore raw

PATCH v2 1/1 input/imx_keypad: add PM support

Quoted 8 lines · Eric B?nard

Hi Alberto,
Le 18/06/2010 12:16, Alberto Panizzo a ?crit :
Quoted 3 lines
Ok, so with this patch the keypad cannot wake the system? It is scheduled another patch?
well, actually, the keypad wakes the system, at least on my board here.

Quoted 7 lines

I was quite surprised to get this working this way as I would have expected disable_irq + clk_disable would prevent the keypad from being a wake source.
Can you try on your board as this cleanup may be wrong on i.MX3 ?
Thanks,
Eric

Well.. it is a surprise to me too. I have to look down the documentation ad soon I will try on the mx31pdk.
I don't wont to tell you wrong things.

Signature · 4 lines

Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

Eric Bénard

author

2010-06-18 17:50 UTC #17 lore raw

PATCH v2 1/1 input/imx_keypad: add PM support

Le 18/06/2010 19:40, Alberto Panizzo a ?crit :

Quoted 2 lines

Well.. it is a surprise to me too. I have to look down the documentation ad soon I will try on the mx31pdk.

OK, I'm waiting for your feedback on the mx31pdk.

From my understanding of the documentation, in order to be able to
wakeup the CPU, the interrupt of a peripheral and its clock should stay enabled while going to suspend.

Eric

Dmitry Torokhov

maintainer

2010-06-24 16:39 UTC #18 lore raw

PATCH v2 1/1 input/imx_keypad: add PM support

In reply to Eric Bénard

Quoted 28 lines · Eric B?nard

We use keypad_data to enable wakeup from platform data
suspend/resume is tested on an i.MX27 when keypad is not in use and when it's is in use (in which case we immediatly get the key pressed at wakeup)
Signed-off-by Eric B?nard <[email protected]>
Cc maramaopercheseimorto at gmail.com
Cc linux-input at vger.kernel.org
Cc dmitry.torokhov at gmail.com
Cc linux-arm-kernel at lists.infradead.org
---
v2 :
- simplify suspend & resume functions
- convert to pm_ops
 drivers/input/keyboard/imx_keypad.c |   40 +++++++++++++++++++++++++++++++++-
 1 files changed, 38 insertions(+), 2 deletions(-)
drivers/input/keyboard/imx_keypad.c +1 -0
index d92c15c..9aa0005 100644
@@ -364,6 +364,38 @@ static void imx_keypad_inhibit(struct imx_keypad *keypad)
 	writew(0xff00, keypad->mmio_base + KPCR);
 }
 
+#ifdef CONFIG_PM

drivers/input/keyboard/imx_keypad.c static void imx_keypad_inhibit(struct imx_keypad *keypad) · line 377

Quoted 10 lines

drivers/input/keyboard/imx_keypad.c +10 -0

+static int mxc_kpp_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+
+	keypad->enabled = false;
+	synchronize_irq(keypad->irq);
+	disable_irq(keypad->irq);
+	clk_disable(keypad->clk);
+

This is not correct. imx_keypad_probe() keypad->enabled = false, waiting for imx_keypad_open() to be called. However if nobody opens the device and you go through suspend/resume cycle you'll leave keypad->enabled = true.

BTW, why do you call new functions mxc_kpp_XXX while the rest of the driver uses imx_keypad_XXX naming?

Thanks.

Signature · 1 line

Dmitry