[PATCH v2 1/8] i.MX27: add suspend to RAM support

[PATCH v2 1/8] i.MX27: add suspend to RAM support

[Eric Bénard]

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

[PATCH v2 2/8] mxc_nand: fix PM

[Eric Bénard]

* when resuming, we get the following log :
nand_resume called for a chip which is not in suspended state
* this happens 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
* the fix is to remove these call and thus remove the supend and
resume functions of mxc_nand

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

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 314e2bf..217b247 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -892,53 +892,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)
-- 
1.6.3.3

[PATCH v2 3/8] imx_keypad: add PM support

[Eric Bénard]

* add suspend/resume support
* use keypad_data to enable wakeup from platform data
* suspend/resume tested when keypad is not in use and when it's
is use (in which case we immediatly get the key pressed for
wakeup)

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

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index d92c15c..b303cc9 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -364,6 +364,55 @@ 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 {
+		keypad->enabled = false;
+		synchronize_irq(keypad->irq);
+		disable_irq(keypad->irq);
+	}
+
+	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 {
+		keypad->enabled = true;
+		enable_irq(keypad->irq);
+	}
+
+	return 0;
+}
+
 static void imx_keypad_close(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);
@@ -410,7 +459,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 +575,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 +624,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

[PATCH v2 4/8] eukrea_mbimx27: update keyboard platform data

[Eric Bénard]

* 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

[PATCH v2 5/8] serial/imx.c: fix suspend/resume

[Eric Bénard]

* when uart is used as a kernel's console, its baudrate is wrong
after resume :
when resuming, serial_core call set_termios which fails to
get the right baudrate.

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

diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
index eacb588..936f810 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,6 +1219,7 @@ static struct uart_driver imx_reg = {
 	.cons           = IMX_CONSOLE,
 };
 
+#ifdef CONFIG_PM
 static int serial_imx_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct imx_port *sport = platform_get_drvdata(dev);
@@ -1226,6 +1239,7 @@ static int serial_imx_resume(struct platform_device *dev)
 
 	return 0;
 }
+#endif
 
 static int serial_imx_probe(struct platform_device *pdev)
 {
@@ -1343,8 +1357,10 @@ static struct platform_driver serial_imx_driver = {
 	.probe		= serial_imx_probe,
 	.remove		= serial_imx_remove,
 
+#ifdef CONFIG_PM
 	.suspend	= serial_imx_suspend,
 	.resume		= serial_imx_resume,
+#endif
 	.driver		= {
 		.name	= "imx-uart",
 		.owner	= THIS_MODULE,
-- 
1.6.3.3

[PATCH v2 6/8] serial/imx: drvdata is sport and not &sport->port

[Eric Bénard]

even if that's the same pointer this is more readable like this

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

diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
index 936f810..a0ebe91 100644
--- a/drivers/serial/imx.c
+++ b/drivers/serial/imx.c
@@ -1311,7 +1311,7 @@ static int serial_imx_probe(struct platform_device *pdev)
 	ret = uart_add_one_port(&imx_reg, &sport->port);
 	if (ret)
 		goto deinit;
-	platform_set_drvdata(pdev, &sport->port);
+	platform_set_drvdata(pdev, sport);
 
 	return 0;
 deinit:
-- 
1.6.3.3

[PATCH v2 7/8] net/fec: fix pm to survive to suspend/resume

[Eric Bénard]

* with this patch, if the connection if active before suspend, it
will be active after resume.
* before this patch, it was necessary to close the interface and
reopen it to recover the connection.

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 drivers/net/fec.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 9b4e8f7..654abaf 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1971,13 +1971,14 @@ static int
 fec_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct net_device *ndev = platform_get_drvdata(dev);
-	struct fec_enet_private *fep;
+	struct fec_enet_private *fep = netdev_priv(ndev);
 
 	if (ndev) {
 		fep = netdev_priv(ndev);
 		if (netif_running(ndev)) {
+			netif_stop_queue(ndev);
 			netif_device_detach(ndev);
-			fec_stop(ndev);
+			clk_disable(fep->clk);
 		}
 	}
 	return 0;
@@ -1987,11 +1988,13 @@ static int
 fec_resume(struct platform_device *dev)
 {
 	struct net_device *ndev = platform_get_drvdata(dev);
+	struct fec_enet_private *fep = netdev_priv(ndev);
 
 	if (ndev) {
 		if (netif_running(ndev)) {
-			fec_enet_init(ndev, 0);
+			clk_enable(fep->clk);
 			netif_device_attach(ndev);
+			netif_start_queue(ndev);
 		}
 	}
 	return 0;
-- 
1.6.3.3

[PATCH v2 8/8] imxfb: fix PM

[Eric Bénard]

* if the screen is blanked, the clock is already disabled, so handle
this case
* before this patch, the following log was triggered :
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 <eric@eukrea.com>
---
 drivers/video/imxfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index b4b6dec..6cbfbff 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -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;
 }
 
-- 
1.6.3.3

[PATCH v2 7/8] net/fec: fix pm to survive to suspend/resume

[Sergei Shtylyov]

Hello.

Eric B?nard wrote:

> * with this patch, if the connection if active before suspend, it
> will be active after resume.
> * before this patch, it was necessary to close the interface and
> reopen it to recover the connection.

> Signed-off-by: Eric B?nard <eric@eukrea.com>

> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 9b4e8f7..654abaf 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -1971,13 +1971,14 @@ static int
>  fec_suspend(struct platform_device *dev, pm_message_t state)
>  {
>  	struct net_device *ndev = platform_get_drvdata(dev);
> -	struct fec_enet_private *fep;
> +	struct fec_enet_private *fep = netdev_priv(ndev);

    Can netdev_priv() be called if 'ndev' is NULL (and can it be really 
NULL?)?

>  
>  	if (ndev) {
>  		fep = netdev_priv(ndev);

    Duplicated now.

WBR, Sergei

[PATCH v3] net/fec: fix pm to survive to suspend/resume

[Eric Bénard]

* with this patch, if the connection if active before suspend, it
will be active after resume.
* before this patch, it was necessary to close the interface and
reopen it to recover the connection.

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
v3 : fix following Sergei Shtylyov's comments

 drivers/net/fec.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 9b4e8f7..1521972 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1974,10 +1974,11 @@ fec_suspend(struct platform_device *dev, pm_message_t state)
 	struct fec_enet_private *fep;
 
 	if (ndev) {
-		fep = netdev_priv(ndev);
 		if (netif_running(ndev)) {
+			fep = netdev_priv(ndev);
+			netif_stop_queue(ndev);
 			netif_device_detach(ndev);
-			fec_stop(ndev);
+			clk_disable(fep->clk);
 		}
 	}
 	return 0;
@@ -1987,11 +1988,14 @@ static int
 fec_resume(struct platform_device *dev)
 {
 	struct net_device *ndev = platform_get_drvdata(dev);
+	struct fec_enet_private *fep;
 
 	if (ndev) {
 		if (netif_running(ndev)) {
-			fec_enet_init(ndev, 0);
+			fep = netdev_priv(ndev);
+			clk_enable(fep->clk);
 			netif_device_attach(ndev);
+			netif_start_queue(ndev);
 		}
 	}
 	return 0;
-- 
1.6.3.3

[PATCH v3] net/fec: fix pm to survive to suspend/resume

[Fabio Estevam]

Eric,

--- On Thu, 5/27/10, Eric B?nard <eric@eukrea.com> wrote:

> From: Eric B?nard <eric@eukrea.com>
> Subject: [PATCH v3] net/fec: fix pm to survive to suspend/resume
> To: linux-arm-kernel at lists.infradead.org
> Cc: s.hauer at pengutronix.de, sshtylyov at mvista.com
> Date: Thursday, May 27, 2010, 11:19 AM
> * with this patch, if the connection
> if active before suspend, it
> will be active after resume.
> * before this patch, it was necessary to close the
> interface and
> reopen it to recover the connection.
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
> v3 : fix following Sergei Shtylyov's comments
> 
>  drivers/net/fec.c |???10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 9b4e8f7..1521972 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c

I think you should have CC'ed the linux-netdev list also.

Regards,

Fabio Estevam


      

[PATCH] net/fec: fix pm to survive to suspend/resume

[Eric Bénard]

* with this patch, if the connection if active before suspend, it
will be active after resume.
* before this patch, it was necessary to close the interface and
reopen it to recover the connection.

Signed-off-by: Eric B?nard <eric@eukrea.com>
Cc: s.hauer at pengutronix.de
Cc: sshtylyov at mvista.com
Cc: linux-arm-kernel at lists.infradead.org
Cc: fabioestevam at yahoo.com
Cc: netdev at vger.kernel.org
---
 drivers/net/fec.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 9b4e8f7..1521972 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1974,10 +1974,11 @@ fec_suspend(struct platform_device *dev, pm_message_t state)
 	struct fec_enet_private *fep;
 
 	if (ndev) {
-		fep = netdev_priv(ndev);
 		if (netif_running(ndev)) {
+			fep = netdev_priv(ndev);
+			netif_stop_queue(ndev);
 			netif_device_detach(ndev);
-			fec_stop(ndev);
+			clk_disable(fep->clk);
 		}
 	}
 	return 0;
@@ -1987,11 +1988,14 @@ static int
 fec_resume(struct platform_device *dev)
 {
 	struct net_device *ndev = platform_get_drvdata(dev);
+	struct fec_enet_private *fep;
 
 	if (ndev) {
 		if (netif_running(ndev)) {
-			fec_enet_init(ndev, 0);
+			fep = netdev_priv(ndev);
+			clk_enable(fep->clk);
 			netif_device_attach(ndev);
+			netif_start_queue(ndev);
 		}
 	}
 	return 0;
-- 
1.6.3.3

[PATCH] net/fec: fix pm to survive to suspend/resume

[David Miller]

From: Eric B?nard <eric@eukrea.com>
Date: Thu, 27 May 2010 19:53:48 +0200

> * with this patch, if the connection if active before suspend, it
> will be active after resume.
> * before this patch, it was necessary to close the interface and
> reopen it to recover the connection.
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>

On resume the PHY should be reset and the link renegotiated (or set to
a fixed speed/duplex if set by the user via ethtool) just like as
happens on ->open().

You seem to be avoiding that here.

If the PHY is not functioning properly after the ->resume() triggered
reset, fix that instead.

I'm not applying this patch, it doesn't look the right thing to do at
all.

[PATCH v2] net/fec: fix pm to survive to suspend/resume

[Eric Bénard]

* in the actual driver, calling fec_stop and fec_enet_init doesn't
allow to have a working network interface at resume (where a
ifconfig down and up is required to recover the interface)
* by using fec_enet_close and fec_enet_open, this patch solves this
problem and handle the case where the link changed between suspend
and resume
* this patch also disable clock at suspend and reenable it at resume

Signed-off-by: Eric B?nard <eric@eukrea.com>
Cc: s.hauer at pengutronix.de
Cc: sshtylyov at mvista.com
Cc: linux-arm-kernel at lists.infradead.org
Cc: fabioestevam at yahoo.com
Cc: davem at davemloft.net
---
 drivers/net/fec.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 9b4e8f7..40ffdb8 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1975,10 +1975,9 @@ fec_suspend(struct platform_device *dev, pm_message_t state)
 
 	if (ndev) {
 		fep = netdev_priv(ndev);
-		if (netif_running(ndev)) {
-			netif_device_detach(ndev);
-			fec_stop(ndev);
-		}
+		if (netif_running(ndev))
+			fec_enet_close(ndev);
+		clk_disable(fep->clk);
 	}
 	return 0;
 }
@@ -1987,12 +1986,13 @@ static int
 fec_resume(struct platform_device *dev)
 {
 	struct net_device *ndev = platform_get_drvdata(dev);
+	struct fec_enet_private *fep;
 
 	if (ndev) {
-		if (netif_running(ndev)) {
-			fec_enet_init(ndev, 0);
-			netif_device_attach(ndev);
-		}
+		fep = netdev_priv(ndev);
+		clk_enable(fep->clk);
+		if (netif_running(ndev))
+			fec_enet_open(ndev);
 	}
 	return 0;
 }
-- 
1.6.3.3

[PATCH v2] net/fec: fix pm to survive to suspend/resume

[David Miller]

From: Eric B?nard <eric@eukrea.com>
Date: Sat, 29 May 2010 22:04:50 +0200

> * in the actual driver, calling fec_stop and fec_enet_init doesn't
> allow to have a working network interface at resume (where a
> ifconfig down and up is required to recover the interface)
> * by using fec_enet_close and fec_enet_open, this patch solves this
> problem and handle the case where the link changed between suspend
> and resume
> * this patch also disable clock at suspend and reenable it at resume
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>

Applied, thanks.

[PATCH v2 3/8] imx_keypad: add PM support

[Alberto Panizzo]

Eric,
On gio, 2010-05-27 at 08:33 +0200, Eric B?nard wrote:
> * add suspend/resume support
> * use keypad_data to enable wakeup from platform data
> * suspend/resume tested when keypad is not in use and when it's
> is use (in which case we immediatly get the key pressed for
> wakeup)
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  drivers/input/keyboard/imx_keypad.c |   56 +++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index d92c15c..b303cc9 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -364,6 +364,55 @@ 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 {
> +		keypad->enabled = false;
> +		synchronize_irq(keypad->irq);
> +		disable_irq(keypad->irq);
> +	}
> +
> +	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 {
> +		keypad->enabled = true;
> +		enable_irq(keypad->irq);
> +	}
> +
> +	return 0;
> +}
> +
>  static void imx_keypad_close(struct input_dev *dev)
>  {
>  	struct imx_keypad *keypad = input_get_drvdata(dev);
> @@ -410,7 +459,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 +575,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 +624,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)

This patch need to follow the path through linux-input.

Generally, to know what are the addresses to add in CC
when you send a patch you can do as follow:

>From a patch:
linux_root$ ./scripts/get_maintainer.pl patch_name.patch

>From a file:
linux_root$ ./scripts/get_maintainer.pl -f drivers/input/keyboard/imx_keypad.c

The result contains: right mailing lists, the maintainers of
the touched subsystems and all the people that had contribute on
the files touched by the patch.

Best regards

-- 
Alberto!

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

[PATCH] imx_keypad: add PM support

[Eric Bénard]

* add suspend/resume support
* use keypad_data to enable wakeup from platform data
* suspend/resume tested when keypad is not in use and when it's
is use (in which case we immediatly get the key pressed for
wakeup)

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

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index d92c15c..b303cc9 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -364,6 +364,55 @@ 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 {
+		keypad->enabled = false;
+		synchronize_irq(keypad->irq);
+		disable_irq(keypad->irq);
+	}
+
+	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 {
+		keypad->enabled = true;
+		enable_irq(keypad->irq);
+	}
+
+	return 0;
+}
+
 static void imx_keypad_close(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);
@@ -410,7 +459,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 +575,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 +624,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

[PATCH] imx_keypad: add PM support

[Dmitry Torokhov]

Hi Eric,

On Sun, May 30, 2010 at 10:51:06AM +0200, Eric B?nard wrote:
> * add suspend/resume support
> * use keypad_data to enable wakeup from platform data
> * suspend/resume tested when keypad is not in use and when it's
> is use (in which case we immediatly get the key pressed for
> wakeup)
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  drivers/input/keyboard/imx_keypad.c |   56 +++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index d92c15c..b303cc9 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -364,6 +364,55 @@ 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 {
> +		keypad->enabled = false;
> +		synchronize_irq(keypad->irq);
> +		disable_irq(keypad->irq);

I'd factor it out as it is used in both suspend and close.

Does everythig (clk_disable, eit) work well in case when device is still
closed when it get ssuspended?

Also, do you really need such an elaborate setup when device is/is not
wakeup source? Do you really loose keypress if you always dosable IRQ in
suspend and enable it in resume?

> +	}
> +
> +	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.
> + */

This is not the standard kerneldoc markup.

> +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 {
> +		keypad->enabled = true;
> +		enable_irq(keypad->irq);
> +	}
> +
> +	return 0;
> +}
> +
>  static void imx_keypad_close(struct input_dev *dev)
>  {
>  	struct imx_keypad *keypad = input_get_drvdata(dev);
> @@ -410,7 +459,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 +575,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 +624,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,

Please use pm_ops.

Thanks.

-- 
Dmitry

[PATCH] mxc_nand: fix PM

[Eric Bénard]

* when resuming, we get the following log :
nand_resume called for a chip which is not in suspended state
* this happens 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
* the fix is to remove these call and thus remove the supend and
resume functions of mxc_nand

Signed-off-by: Eric B?nard <eric@eukrea.com>
Cc: s.hauer at pengutronix.de
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-mtd at lists.infradead.org
---
 drivers/mtd/nand/mxc_nand.c |   44 +------------------------------------------
 1 files changed, 1 insertions(+), 43 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 314e2bf..217b247 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -892,53 +892,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)
-- 
1.6.3.3

[PATCH] mxc_nand: fix PM

[Eric Bénard]

Hi Sascha,

Le 27/05/2010 19:55, Eric B?nard a ?crit :
> * when resuming, we get the following log :
> nand_resume called for a chip which is not in suspended state
> * this happens 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
> * the fix is to remove these call and thus remove the supend and
> resume functions of mxc_nand
>
any news concerning this patch ?

Eric

> Signed-off-by: Eric B?nard<eric@eukrea.com>
> Cc: s.hauer at pengutronix.de
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-mtd at lists.infradead.org
> ---
>   drivers/mtd/nand/mxc_nand.c |   44 +------------------------------------------
>   1 files changed, 1 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 314e2bf..217b247 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -892,53 +892,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)

[PATCH] mxc_nand: fix PM

[Artem Bityutskiy]

On Mon, 2010-06-14 at 09:07 +0200, Eric B?nard wrote:
> Hi Sascha,
> 
> Le 27/05/2010 19:55, Eric B?nard a ?crit :
> > * when resuming, we get the following log :
> > nand_resume called for a chip which is not in suspended state
> > * this happens 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
> > * the fix is to remove these call and thus remove the supend and
> > resume functions of mxc_nand
> >
> any news concerning this patch ?
> 

Pushed to l2-mtd-2.6.git / dunno

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

[PATCH] mxc_nand: fix PM

[Artem Bityutskiy]

On Mon, 2010-06-14 at 09:07 +0200, Eric B?nard wrote:
> Hi Sascha,
> 
> Le 27/05/2010 19:55, Eric B?nard a ?crit :
> > * when resuming, we get the following log :
> > nand_resume called for a chip which is not in suspended state
> > * this happens 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
> > * the fix is to remove these call and thus remove the supend and
> > resume functions of mxc_nand
> >
> any news concerning this patch ?

Pushed to l2-mtd-2.6.git / dunno

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

[PATCH] mxc_nand: fix PM

[Uwe Kleine-König]

Hello Eric,

On Thu, May 27, 2010 at 07:55:19PM +0200, Eric B?nard wrote:
> * when resuming, we get the following log :
> nand_resume called for a chip which is not in suspended state
> * this happens 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
> * the fix is to remove these call and thus remove the supend and
> resume functions of mxc_nand
the commit log looks IMHO ugly.  Better make it (including Subject):

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 <eric@eukrea.com>
Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-mtd at lists.infradead.org

Best regards
Uwe

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

[PATCH v3] i.MX27: add suspend to RAM support

[Eric Bénard]

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
v3 :
	return -EINVAL instead of -1

 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..f9a7760
--- /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 -EINVAL;
+	}
+	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

[PATCH v3] i.MX27: add suspend to RAM support

[Russell King - ARM Linux]

On Thu, May 27, 2010 at 04:13:55PM +0200, Eric B?nard wrote:
> +#include <linux/kernel.h>
> +#include <linux/suspend.h>
> +#include <asm/io.h>

linux/io.h

(checkpatch.pl will tell you this, although the general rule that "if
there's a linux/foo.h and an asm/foo.h, linux/foo.h should be used"
doesn't always apply - there are some exceptions.)

[PATCH v3] i.MX27: add suspend to RAM support

[Eric Bénard]

Hi Russell,

Le 27/05/2010 21:43, Russell King - ARM Linux a ?crit :
> On Thu, May 27, 2010 at 04:13:55PM +0200, Eric B?nard wrote:
>> +#include<linux/kernel.h>
>> +#include<linux/suspend.h>
>> +#include<asm/io.h>
>
> linux/io.h
>
OK sorry for this one, you already told me this at the first review and
I forget to fix it.

> (checkpatch.pl will tell you this, although the general rule that "if
> there's a linux/foo.h and an asm/foo.h, linux/foo.h should be used"
> doesn't always apply - there are some exceptions.)
>
In fact checkpatch only returned the following warning (which I didn't
fixed as I didn't found any platform_suspend_ops which is const).
Do you think I should also fix this warning ?

./linux-2.6/scripts/checkpatch.pl 
0001-i.MX27-add-suspend-to-RAM-support.patch
WARNING: struct platform_suspend_ops should normally be const
#72: FILE: arch/arm/mach-mx2/pm-imx27.c:35:
+static struct platform_suspend_ops mx27_suspend_ops = {

total: 0 errors, 1 warnings, 54 lines checked

0001-i.MX27-add-suspend-to-RAM-support.patch has style problems, please 
review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Eric

[PATCH v3] i.MX27: add suspend to RAM support

[Russell King - ARM Linux]

On Thu, May 27, 2010 at 10:06:04PM +0200, Eric B?nard wrote:
> Le 27/05/2010 21:43, Russell King - ARM Linux a ?crit :
>> On Thu, May 27, 2010 at 04:13:55PM +0200, Eric B?nard wrote:
> OK sorry for this one, you already told me this at the first review and
> I forget to fix it.

np.

>> (checkpatch.pl will tell you this, although the general rule that "if
>> there's a linux/foo.h and an asm/foo.h, linux/foo.h should be used"
>> doesn't always apply - there are some exceptions.)
>>
> In fact checkpatch only returned the following warning (which I didn't
> fixed as I didn't found any platform_suspend_ops which is const).

It seems to be classified as a "--subjective" check - I suspect because
there's some variance between arch maintainers on their policy on this
issue, and also because there are some exceptions as I've said above.

Eg, we have asm/memory.h, and there is a linux/memory.h, but the two
are completely unrelated - so the checkpatch warnings about "use
linux/memory.h rather than asm/memory.h" are rather misleading.

> Do you think I should also fix this warning ?
>
> ./linux-2.6/scripts/checkpatch.pl  
> 0001-i.MX27-add-suspend-to-RAM-support.patch
> WARNING: struct platform_suspend_ops should normally be const
> #72: FILE: arch/arm/mach-mx2/pm-imx27.c:35:
> +static struct platform_suspend_ops mx27_suspend_ops = {

It's trivial enough to fix, and doesn't cause any issues by doing so,
so I'd suggest adding the const.

[PATCH v4] i.MX27: add suspend to RAM support

[Eric Bénard]

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
v4:
	fixed include + const struct
v3:
	return -EINVAL instead of -1

 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..2153ca7
--- /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 <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 -EINVAL;
+	}
+	return 0;
+}
+
+static const 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

[PATCH v3] i.MX27: add suspend to RAM support

[Eric Bénard]

Le 27/05/2010 22:16, Russell King - ARM Linux a ?crit :
> On Thu, May 27, 2010 at 10:06:04PM +0200, Eric B?nard wrote:
>> Do you think I should also fix this warning ?
>>
>> ./linux-2.6/scripts/checkpatch.pl
>> 0001-i.MX27-add-suspend-to-RAM-support.patch
>> WARNING: struct platform_suspend_ops should normally be const
>> #72: FILE: arch/arm/mach-mx2/pm-imx27.c:35:
>> +static struct platform_suspend_ops mx27_suspend_ops = {
>
> It's trivial enough to fix, and doesn't cause any issues by doing so,
> so I'd suggest adding the const.
>
well now this triggers the following warning so forget the patch v4, 
checkpatch seems wrong on this case :
   CC      arch/arm/mach-mx2/pm-imx27.o
arch/arm/mach-mx2/pm-imx27.c: In function 'mx27_pm_init':
arch/arm/mach-mx2/pm-imx27.c:42: warning: passing argument 1 of 
'suspend_set_ops' discards qualifiers from pointer target type

Eric

[PATCH v3] i.MX27: add suspend to RAM support

[Russell King - ARM Linux]

On Thu, May 27, 2010 at 10:54:40PM +0200, Eric B?nard wrote:
> Le 27/05/2010 22:16, Russell King - ARM Linux a ?crit :
>> On Thu, May 27, 2010 at 10:06:04PM +0200, Eric B?nard wrote:
>>> Do you think I should also fix this warning ?
>>>
>>> ./linux-2.6/scripts/checkpatch.pl
>>> 0001-i.MX27-add-suspend-to-RAM-support.patch
>>> WARNING: struct platform_suspend_ops should normally be const
>>> #72: FILE: arch/arm/mach-mx2/pm-imx27.c:35:
>>> +static struct platform_suspend_ops mx27_suspend_ops = {
>>
>> It's trivial enough to fix, and doesn't cause any issues by doing so,
>> so I'd suggest adding the const.
>>
> well now this triggers the following warning so forget the patch v4,  
> checkpatch seems wrong on this case :
>   CC      arch/arm/mach-mx2/pm-imx27.o
> arch/arm/mach-mx2/pm-imx27.c: In function 'mx27_pm_init':
> arch/arm/mach-mx2/pm-imx27.c:42: warning: passing argument 1 of  
> 'suspend_set_ops' discards qualifiers from pointer target type

Hmm, seems that checkpatch is incorrect on this point then.
include/linux/suspend.h has:

extern void suspend_set_ops(struct platform_suspend_ops *ops);
static inline void suspend_set_ops(struct platform_suspend_ops *ops) {}

and I can't find any suggestion that these ever took a const pointer.

[PATCH v5] i.MX27: add suspend to RAM support

[Eric Bénard]

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
v5:
	remove const struct
v4:
	fixed include + const struct
v3:
	return -EINVAL instead of -1

 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..afc17ce
--- /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 <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 -EINVAL;
+	}
+	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

[PATCH v5] i.MX27: add suspend to RAM support

[Sascha Hauer]

On Tue, Jun 01, 2010 at 09:57:44PM +0200, Eric B?nard wrote:
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
> v5:
> 	remove const struct
> v4:
> 	fixed include + const struct
> v3:
> 	return -EINVAL instead of -1
> 
>  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

Ok, applied

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 |

[PATCH v3] i.MX27: add suspend to RAM support

[Wolfram Sang]

> Eg, we have asm/memory.h, and there is a linux/memory.h, but the two
> are completely unrelated - so the checkpatch warnings about "use
> linux/memory.h rather than asm/memory.h" are rather misleading.

checkpatch can blacklist .h-files for this warning (as it does for irq.h at the
moment). Do you think adding memory.h there is worthwhile? I'd prepare a patch,
then.

-- 
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/20100527/86c6e21d/attachment.sig>