linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] i.MX27: add suspend to RAM support
@ 2010-05-21 11:12 Eric Bénard
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 11:12 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 arch/arm/mach-mx2/Makefile   |    2 +-
 arch/arm/mach-mx2/pm-imx27.c |   46 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mx2/pm-imx27.c

diff --git a/arch/arm/mach-mx2/Makefile b/arch/arm/mach-mx2/Makefile
index e3254fa..27d496c 100644
--- a/arch/arm/mach-mx2/Makefile
+++ b/arch/arm/mach-mx2/Makefile
@@ -8,7 +8,7 @@ obj-y	:=  devices.o serial.o
 
 obj-$(CONFIG_MACH_MX21) += clock_imx21.o mm-imx21.o
 
-obj-$(CONFIG_MACH_MX27) += cpu_imx27.o
+obj-$(CONFIG_MACH_MX27) += cpu_imx27.o pm-imx27.o
 obj-$(CONFIG_MACH_MX27) += clock_imx27.o mm-imx27.o
 
 obj-$(CONFIG_MACH_MX21ADS) += mach-mx21ads.o
diff --git a/arch/arm/mach-mx2/pm-imx27.c b/arch/arm/mach-mx2/pm-imx27.c
new file mode 100644
index 0000000..a82fb6d
--- /dev/null
+++ b/arch/arm/mach-mx2/pm-imx27.c
@@ -0,0 +1,46 @@
+/*
+ * i.MX27 Power Management Routines
+ *
+ * Based on Freescale's BSP
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+#include <asm/io.h>
+#include <mach/system.h>
+#include <mach/mx27.h>
+
+static int mx27_suspend_enter(suspend_state_t state)
+{
+	u32 cscr;
+	switch (state) {
+	case PM_SUSPEND_MEM:
+		/* Clear MPEN and SPEN to disable MPLL/SPLL */
+		cscr = __raw_readl(MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
+		cscr &= 0xFFFFFFFC;
+		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
+		/* Executes WFI */
+		arch_idle();
+		break;
+
+	default:
+		return -1;
+	}
+	return 0;
+}
+
+static struct platform_suspend_ops mx27_suspend_ops = {
+	.enter = mx27_suspend_enter,
+	.valid = suspend_valid_only_mem,
+};
+
+static int __init mx27_pm_init(void)
+{
+	suspend_set_ops(&mx27_suspend_ops);
+	return 0;
+}
+
+device_initcall(mx27_pm_init);
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-21 11:12 [PATCH 1/5] i.MX27: add suspend to RAM support Eric Bénard
@ 2010-05-21 11:12 ` Eric Bénard
  2010-05-21 11:12   ` [PATCH 3/5] imx_keypad: add PM support Eric Bénard
                     ` (3 more replies)
  2010-05-21 12:02 ` [PATCH 1/5] i.MX27: add suspend to RAM support Lothar Waßmann
  2010-05-21 19:19 ` Russell King - ARM Linux
  2 siblings, 4 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 11:12 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 drivers/mtd/nand/mxc_nand.c |   27 ++++++++++-----------------
 1 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index fb03aff..5e51d4d 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -895,16 +895,13 @@ static int __devexit mxcnd_remove(struct platform_device *pdev)
 #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;
+	struct mxc_nand_host *host = platform_get_drvdata(pdev);
 	int ret = 0;
 
 	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND suspend\n");
-	if (mtd) {
-		ret = mtd->suspend(mtd);
-		/* Disable the NFC clock */
+	if (host->clk_act) {
 		clk_disable(host->clk);
+		host->clk_act = 0;
 	}
 
 	return ret;
@@ -912,31 +909,27 @@ static int mxcnd_suspend(struct platform_device *pdev, pm_message_t state)
 
 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;
+	struct mxc_nand_host *host = platform_get_drvdata(pdev);
 	int ret = 0;
 
 	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND resume\n");
-
-	if (mtd) {
-		/* Enable the NFC clock */
+	if (!host->clk_act) {
 		clk_enable(host->clk);
-		mtd->resume(mtd);
+		host->clk_act = 1;
 	}
 
 	return ret;
 }
 
 #else
-# define mxcnd_suspend   NULL
-# define mxcnd_resume    NULL
-#endif				/* CONFIG_PM */
+#define mxcnd_suspend	NULL
+#define mxcnd_resume	NULL
+#endif
 
 static struct platform_driver mxcnd_driver = {
 	.driver = {
 		   .name = DRIVER_NAME,
-		   },
+	},
 	.remove = __devexit_p(mxcnd_remove),
 	.suspend = mxcnd_suspend,
 	.resume = mxcnd_resume,
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] imx_keypad: add PM support
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
@ 2010-05-21 11:12   ` Eric Bénard
  2010-05-21 11:12     ` [PATCH 4/5] eukrea_mbimx27: update keyboard platform data Eric Bénard
  2010-05-21 19:22     ` [PATCH 3/5] imx_keypad: add PM support Russell King - ARM Linux
  2010-05-21 11:41   ` [PATCH 2/5] mxc_nand: fix PM Lothar Waßmann
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 11:12 UTC (permalink / raw)
  To: linux-arm-kernel

* add suspend/resume support
* use keypad_data to enable wakeup from platform data

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 drivers/input/keyboard/imx_keypad.c |   57 +++++++++++++++++++++++++++++++++-
 1 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index d92c15c..0d44bce 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -364,6 +364,56 @@ static void imx_keypad_inhibit(struct imx_keypad *keypad)
 	writew(0xff00, keypad->mmio_base + KPCR);
 }
 
+static int mxc_kpp_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+	unsigned short reg_val;
+
+	clk_disable(keypad->clk);
+
+	if (device_may_wakeup(&pdev->dev)) {
+		reg_val = readw(keypad->mmio_base + KPSR);
+		if ((reg_val & KBD_STAT_KDIE) == 0) {
+			/* If no depress interrupt enable the release interrupt */
+			reg_val |= KBD_STAT_KRIE;
+			writew(reg_val, keypad->mmio_base + KPSR);
+		}
+		enable_irq_wake(keypad->irq);
+	} else {
+		disable_irq(keypad->irq);
+		keypad->enabled = false;
+		imx_keypad_inhibit(keypad);
+	}
+
+	return 0;
+}
+
+/*!
+ * This function brings the Keypad controller back from low-power state.
+ * If Keypad is enabled as a wake source(i.e. it can resume the system
+ * from suspend mode), the Keypad controller doesn't enter low-power state.
+ *
+ * @param   pdev  the device structure used to give information on Keypad
+ *                to resume
+ *
+ * @return  The function always returns 0.
+ */
+static int mxc_kpp_resume(struct platform_device *pdev)
+{
+	struct imx_keypad *keypad = platform_get_drvdata(pdev);
+
+	clk_enable(keypad->clk);
+	if (device_may_wakeup(&pdev->dev)) {
+		/* The irq routine already cleared KRIE if it was set */
+	} else {
+		enable_irq(keypad->irq);
+		keypad->enabled = true;
+		imx_keypad_config(keypad);
+	}
+
+	return 0;
+}
+
 static void imx_keypad_close(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);
@@ -410,7 +460,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 +576,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;
 
@@ -574,6 +625,8 @@ static struct platform_driver imx_keypad_driver = {
 	},
 	.probe		= imx_keypad_probe,
 	.remove		= __devexit_p(imx_keypad_remove),
+	.suspend = mxc_kpp_suspend,
+	.resume = mxc_kpp_resume,
 };
 
 static int __init imx_keypad_init(void)
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] eukrea_mbimx27: update keyboard platform data
  2010-05-21 11:12   ` [PATCH 3/5] imx_keypad: add PM support Eric Bénard
@ 2010-05-21 11:12     ` Eric Bénard
  2010-05-21 11:12       ` [PATCH 5/5] serial/imx.c: fix suspend/resume Eric Bénard
  2010-05-21 19:22     ` [PATCH 3/5] imx_keypad: add PM support Russell King - ARM Linux
  1 sibling, 1 reply; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 11:12 UTC (permalink / raw)
  To: linux-arm-kernel

* use matrix_keypad_platform_data
* enable wakeup

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c b/arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c
index 34bdc26..9038e1f 100644
--- a/arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c
+++ b/arch/arm/mach-mx2/eukrea_mbimx27-baseboard.c
@@ -107,6 +107,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",
@@ -350,7 +355,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));
 }
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] serial/imx.c: fix suspend/resume
  2010-05-21 11:12     ` [PATCH 4/5] eukrea_mbimx27: update keyboard platform data Eric Bénard
@ 2010-05-21 11:12       ` Eric Bénard
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 11:12 UTC (permalink / raw)
  To: linux-arm-kernel

* there is a stange behaviour in suspend & resume functions :
if console is enabled on serial the baudrate is not properly restored
at resume. The "fix" is tho use platform_get_drvdata directly on
dev in suspend/resume instead of using to_platform_device.
Someone more used to this driver than me may find a better fix !

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 drivers/serial/imx.c |   33 ++++++++++++++++++++++++++-------
 1 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
index eacb588..606566e 100644
--- a/drivers/serial/imx.c
+++ b/drivers/serial/imx.c
@@ -1020,6 +1020,17 @@ imx_verify_port(struct uart_port *port, struct serial_struct *ser)
 	return ret;
 }
 
+static void
+imx_suspend(struct uart_port *port, unsigned int state, unsigned int oldstate)
+{
+	struct imx_port *sport = (struct imx_port *)port;
+
+	if (!state)
+		clk_enable(sport->clk);
+	else
+		clk_disable(sport->clk);
+}
+
 static struct uart_ops imx_pops = {
 	.tx_empty	= imx_tx_empty,
 	.set_mctrl	= imx_set_mctrl,
@@ -1037,6 +1048,7 @@ static struct uart_ops imx_pops = {
 	.request_port	= imx_request_port,
 	.config_port	= imx_config_port,
 	.verify_port	= imx_verify_port,
+	.pm		= imx_suspend,
 };
 
 static struct imx_port *imx_ports[UART_NR];
@@ -1207,9 +1219,11 @@ static struct uart_driver imx_reg = {
 	.cons           = IMX_CONSOLE,
 };
 
-static int serial_imx_suspend(struct platform_device *dev, pm_message_t state)
+#ifdef CONFIG_PM
+static int serial_imx_suspend(struct device *dev)
 {
-	struct imx_port *sport = platform_get_drvdata(dev);
+/* FIXME	struct platform_device *pdev = to_platform_device(dev); */
+	struct imx_port *sport = platform_get_drvdata((struct platform_device *)dev);
 
 	if (sport)
 		uart_suspend_port(&imx_reg, &sport->port);
@@ -1217,9 +1231,10 @@ static int serial_imx_suspend(struct platform_device *dev, pm_message_t state)
 	return 0;
 }
 
-static int serial_imx_resume(struct platform_device *dev)
+static int serial_imx_resume(struct device *dev)
 {
-	struct imx_port *sport = platform_get_drvdata(dev);
+/* FIXME	struct platform_device *pdev = to_platform_device(dev); */
+	struct imx_port *sport = platform_get_drvdata((struct platform_device *)dev);
 
 	if (sport)
 		uart_resume_port(&imx_reg, &sport->port);
@@ -1227,6 +1242,10 @@ static int serial_imx_resume(struct platform_device *dev)
 	return 0;
 }
 
+static const SIMPLE_DEV_PM_OPS(serial_imx_pm_ops,
+				serial_imx_suspend, serial_imx_resume);
+#endif
+
 static int serial_imx_probe(struct platform_device *pdev)
 {
 	struct imx_port *sport;
@@ -1342,12 +1361,12 @@ static int serial_imx_remove(struct platform_device *pdev)
 static struct platform_driver serial_imx_driver = {
 	.probe		= serial_imx_probe,
 	.remove		= serial_imx_remove,
-
-	.suspend	= serial_imx_suspend,
-	.resume		= serial_imx_resume,
 	.driver		= {
 		.name	= "imx-uart",
 		.owner	= THIS_MODULE,
+#ifdef CONFIG_PM
+		.pm	= &serial_imx_pm_ops,
+#endif
 	},
 };
 
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
  2010-05-21 11:12   ` [PATCH 3/5] imx_keypad: add PM support Eric Bénard
@ 2010-05-21 11:41   ` Lothar Waßmann
  2010-05-21 12:32     ` Eric Bénard
  2010-05-21 13:10   ` Wolfram Sang
  2010-05-24 21:10   ` Uwe Kleine-König
  3 siblings, 1 reply; 14+ messages in thread
From: Lothar Waßmann @ 2010-05-21 11:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

>  drivers/mtd/nand/mxc_nand.c |   27 ++++++++++-----------------
>  1 files changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index fb03aff..5e51d4d 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -895,16 +895,13 @@ static int __devexit mxcnd_remove(struct platform_device *pdev)
>  #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;
> +	struct mxc_nand_host *host = platform_get_drvdata(pdev);
>  	int ret = 0;
>  
>  	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND suspend\n");
> -	if (mtd) {
> -		ret = mtd->suspend(mtd);
> -		/* Disable the NFC clock */
> +	if (host->clk_act) {
>  		clk_disable(host->clk);
> +		host->clk_act = 0;
[...]
> @@ -912,31 +909,27 @@ static int mxcnd_suspend(struct platform_device *pdev, pm_message_t state)
>  
>  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;
> +	struct mxc_nand_host *host = platform_get_drvdata(pdev);
>  	int ret = 0;
>  
>  	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND resume\n");
> -
> -	if (mtd) {
> -		/* Enable the NFC clock */
> +	if (!host->clk_act) {
>  		clk_enable(host->clk);
> -		mtd->resume(mtd);
> +		host->clk_act = 1;
>  	}
>
Since you are clearing host->clk_act during suspend, this will
unconditionally enable the clock no matter what state it had before
suspend (in case that's intended, the check for !host->clk_act is
useless, since it will always yield true).
I think it's more sensible to leave host->clk_act alone, and reenable
the clock only if host->clk_act is set, so that the clock is left in
the same state after resume in which it was before suspend.


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] i.MX27: add suspend to RAM support
  2010-05-21 11:12 [PATCH 1/5] i.MX27: add suspend to RAM support Eric Bénard
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
@ 2010-05-21 12:02 ` Lothar Waßmann
  2010-05-21 19:19 ` Russell King - ARM Linux
  2 siblings, 0 replies; 14+ messages in thread
From: Lothar Waßmann @ 2010-05-21 12:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Eric B?nard writes:
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  arch/arm/mach-mx2/Makefile   |    2 +-
>  arch/arm/mach-mx2/pm-imx27.c |   46 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/mach-mx2/pm-imx27.c
> 
> diff --git a/arch/arm/mach-mx2/Makefile b/arch/arm/mach-mx2/Makefile
> index e3254fa..27d496c 100644
> --- a/arch/arm/mach-mx2/Makefile
> +++ b/arch/arm/mach-mx2/Makefile
> @@ -8,7 +8,7 @@ obj-y	:=  devices.o serial.o
>  
>  obj-$(CONFIG_MACH_MX21) += clock_imx21.o mm-imx21.o
>  
> -obj-$(CONFIG_MACH_MX27) += cpu_imx27.o
> +obj-$(CONFIG_MACH_MX27) += cpu_imx27.o pm-imx27.o
>  obj-$(CONFIG_MACH_MX27) += clock_imx27.o mm-imx27.o
>  
>  obj-$(CONFIG_MACH_MX21ADS) += mach-mx21ads.o
> diff --git a/arch/arm/mach-mx2/pm-imx27.c b/arch/arm/mach-mx2/pm-imx27.c
> new file mode 100644
> index 0000000..a82fb6d
> --- /dev/null
> +++ b/arch/arm/mach-mx2/pm-imx27.c
> @@ -0,0 +1,46 @@
> +/*
> + * i.MX27 Power Management Routines
> + *
> + * Based on Freescale's BSP
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/suspend.h>
> +#include <asm/io.h>
> +#include <mach/system.h>
> +#include <mach/mx27.h>
> +
> +static int mx27_suspend_enter(suspend_state_t state)
> +{
> +	u32 cscr;
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +		/* Clear MPEN and SPEN to disable MPLL/SPLL */
> +		cscr = __raw_readl(MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
> +		cscr &= 0xFFFFFFFC;
> +		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
> +		/* Executes WFI */
> +		arch_idle();
> +		break;
> +
> +	default:
> +		return -1;
>
an errno value would be more appropriate here.


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-21 11:41   ` [PATCH 2/5] mxc_nand: fix PM Lothar Waßmann
@ 2010-05-21 12:32     ` Eric Bénard
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 12:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lothar,

Le 21/05/2010 13:41, Lothar Wa?mann a ?crit :
>> -	if (mtd) {
>> -		/* Enable the NFC clock */
>> +	if (!host->clk_act) {
>>   		clk_enable(host->clk);
>> -		mtd->resume(mtd);
>> +		host->clk_act = 1;
>>   	}
>>
> Since you are clearing host->clk_act during suspend, this will
> unconditionally enable the clock no matter what state it had before
> suspend (in case that's intended, the check for !host->clk_act is
> useless, since it will always yield true).

Yes, you're right.
In fact, the clock is turned on & off by mxc_nand_select_chip so when 
clk_enable & clk_disable are called in suspend & resume without checking 
it the clock is already active, I had log complaining for the clock 
already being off as mxc_nand_select_chip hard already turned it off, so 
I put the checks in both functions.

 > I think it's more sensible to leave host->clk_act alone, and reenable
 > the clock only if host->clk_act is set, so that the clock is left in
 > the same state after resume in which it was before suspend.
 >
OK I'll try this. In fact, I think the clock is always off when we reach 
suspend & resume as nand_base has already deselected the chip and thus 
the NFC's clock is turned off.

While we're at this : is there a good reason to select/deselect the chip 
at every access and turn on/off the clock at the same time ?

Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
  2010-05-21 11:12   ` [PATCH 3/5] imx_keypad: add PM support Eric Bénard
  2010-05-21 11:41   ` [PATCH 2/5] mxc_nand: fix PM Lothar Waßmann
@ 2010-05-21 13:10   ` Wolfram Sang
  2010-05-21 13:22     ` Eric Bénard
  2010-05-24 21:10   ` Uwe Kleine-König
  3 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2010-05-21 13:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 21, 2010 at 01:12:40PM +0200, Eric B?nard wrote:

A short description what was fixed and how it was fixed would be great.

> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  drivers/mtd/nand/mxc_nand.c |   27 ++++++++++-----------------
>  1 files changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index fb03aff..5e51d4d 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -895,16 +895,13 @@ static int __devexit mxcnd_remove(struct platform_device *pdev)
>  #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;
> +	struct mxc_nand_host *host = platform_get_drvdata(pdev);
>  	int ret = 0;
>  
>  	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND suspend\n");
> -	if (mtd) {
> -		ret = mtd->suspend(mtd);
> -		/* Disable the NFC clock */
> +	if (host->clk_act) {
>  		clk_disable(host->clk);
> +		host->clk_act = 0;
>  	}
>  
>  	return ret;
> @@ -912,31 +909,27 @@ static int mxcnd_suspend(struct platform_device *pdev, pm_message_t state)
>  
>  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;
> +	struct mxc_nand_host *host = platform_get_drvdata(pdev);
>  	int ret = 0;
>  
>  	DEBUG(MTD_DEBUG_LEVEL0, "MXC_ND : NAND resume\n");
> -
> -	if (mtd) {
> -		/* Enable the NFC clock */
> +	if (!host->clk_act) {
>  		clk_enable(host->clk);
> -		mtd->resume(mtd);
> +		host->clk_act = 1;
>  	}
>  
>  	return ret;
>  }
>  
>  #else
> -# define mxcnd_suspend   NULL
> -# define mxcnd_resume    NULL
> -#endif				/* CONFIG_PM */
> +#define mxcnd_suspend	NULL
> +#define mxcnd_resume	NULL
> +#endif
>  
>  static struct platform_driver mxcnd_driver = {
>  	.driver = {
>  		   .name = DRIVER_NAME,
> -		   },
> +	},
>  	.remove = __devexit_p(mxcnd_remove),
>  	.suspend = mxcnd_suspend,
>  	.resume = mxcnd_resume,
> -- 
> 1.6.3.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- 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/20100521/49433094/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-21 13:10   ` Wolfram Sang
@ 2010-05-21 13:22     ` Eric Bénard
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-21 13:22 UTC (permalink / raw)
  To: linux-arm-kernel

Le 21/05/2010 15:10, Wolfram Sang a ?crit :
> On Fri, May 21, 2010 at 01:12:40PM +0200, Eric B?nard wrote:
>
> A short description what was fixed and how it was fixed would be great.
>
sure :
*** 1st part of the log before the fix :
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 [<c01e57b4>] 
(mxcnd_suspend+0x34/0x3c)
[<c01e57b4>] (mxcnd_suspend+0x34/0x3c) from [<c01bee10>] 
(platform_pm_suspend+0x50/0x54)

* why :
because the clock is already turned off by mxc_nand_select_chip

* fix :
check if the clock is actually really on before turning it off

*** 2nd part of the log before the fix :
nand_resume called for a chip which is not in suspended state

* why :
because mtd->suspend and mtd->resume are already called from the mtd 
layer, thus they were called a second time in mxc_nand so mtd complains 
when trying to resume for the second time

* fix :
don't call mtd->suspend / mtd->resume in mxc_nand.

Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] i.MX27: add suspend to RAM support
  2010-05-21 11:12 [PATCH 1/5] i.MX27: add suspend to RAM support Eric Bénard
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
  2010-05-21 12:02 ` [PATCH 1/5] i.MX27: add suspend to RAM support Lothar Waßmann
@ 2010-05-21 19:19 ` Russell King - ARM Linux
  2 siblings, 0 replies; 14+ messages in thread
From: Russell King - ARM Linux @ 2010-05-21 19:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 21, 2010 at 01:12:39PM +0200, Eric B?nard wrote:
> +#include <linux/kernel.h>
> +#include <linux/suspend.h>
> +#include <asm/io.h>

linux/io.h

> +#include <mach/system.h>
> +#include <mach/mx27.h>
> +
> +static int mx27_suspend_enter(suspend_state_t state)
> +{
> +	u32 cscr;
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +		/* Clear MPEN and SPEN to disable MPLL/SPLL */
> +		cscr = __raw_readl(MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
> +		cscr &= 0xFFFFFFFC;
> +		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
> +		/* Executes WFI */
> +		arch_idle();
> +		break;
> +
> +	default:
> +		return -1;

-Exxx ?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/5] imx_keypad: add PM support
  2010-05-21 11:12   ` [PATCH 3/5] imx_keypad: add PM support Eric Bénard
  2010-05-21 11:12     ` [PATCH 4/5] eukrea_mbimx27: update keyboard platform data Eric Bénard
@ 2010-05-21 19:22     ` Russell King - ARM Linux
  1 sibling, 0 replies; 14+ messages in thread
From: Russell King - ARM Linux @ 2010-05-21 19:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 21, 2010 at 01:12:41PM +0200, Eric B?nard wrote:
> +static int mxc_kpp_resume(struct platform_device *pdev)
> +{
> +	struct imx_keypad *keypad = platform_get_drvdata(pdev);
> +
> +	clk_enable(keypad->clk);
> +	if (device_may_wakeup(&pdev->dev)) {
> +		/* The irq routine already cleared KRIE if it was set */
> +	} else {
> +		enable_irq(keypad->irq);

What happens if an IRQ happens here?

> +		keypad->enabled = true;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
                     ` (2 preceding siblings ...)
  2010-05-21 13:10   ` Wolfram Sang
@ 2010-05-24 21:10   ` Uwe Kleine-König
  2010-05-24 21:16     ` Eric Bénard
  3 siblings, 1 reply; 14+ messages in thread
From: Uwe Kleine-König @ 2010-05-24 21:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 21, 2010 at 01:12:40PM +0200, Eric B?nard wrote:
> Signed-off-by: Eric B?nard <eric@eukrea.com>
See http://patchwork.ozlabs.org/patch/42633/ for an alternative fix.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/5] mxc_nand: fix PM
  2010-05-24 21:10   ` Uwe Kleine-König
@ 2010-05-24 21:16     ` Eric Bénard
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Bénard @ 2010-05-24 21:16 UTC (permalink / raw)
  To: linux-arm-kernel

Le 24/05/2010 23:10, Uwe Kleine-K?nig a ?crit :
> On Fri, May 21, 2010 at 01:12:40PM +0200, Eric B?nard wrote:
>> Signed-off-by: Eric B?nard<eric@eukrea.com>
> See http://patchwork.ozlabs.org/patch/42633/ for an alternative fix.
>
alternative and better ;-) so I'll remove this patch when I'll send an 
updated patchset for PM

Thanks,
Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-05-24 21:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-21 11:12 [PATCH 1/5] i.MX27: add suspend to RAM support Eric Bénard
2010-05-21 11:12 ` [PATCH 2/5] mxc_nand: fix PM Eric Bénard
2010-05-21 11:12   ` [PATCH 3/5] imx_keypad: add PM support Eric Bénard
2010-05-21 11:12     ` [PATCH 4/5] eukrea_mbimx27: update keyboard platform data Eric Bénard
2010-05-21 11:12       ` [PATCH 5/5] serial/imx.c: fix suspend/resume Eric Bénard
2010-05-21 19:22     ` [PATCH 3/5] imx_keypad: add PM support Russell King - ARM Linux
2010-05-21 11:41   ` [PATCH 2/5] mxc_nand: fix PM Lothar Waßmann
2010-05-21 12:32     ` Eric Bénard
2010-05-21 13:10   ` Wolfram Sang
2010-05-21 13:22     ` Eric Bénard
2010-05-24 21:10   ` Uwe Kleine-König
2010-05-24 21:16     ` Eric Bénard
2010-05-21 12:02 ` [PATCH 1/5] i.MX27: add suspend to RAM support Lothar Waßmann
2010-05-21 19:19 ` Russell King - ARM Linux

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).