From: Kevin Hilman <khilman@ti.com>
To: Abhilash K V <abhilash.kv@ti.com>
Cc: linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, tony@atomide.com,
linux@arm.linux.org.uk, Ranjith Lohithakshan <ranjithl@ti.com>
Subject: Re: [PATCH 1/4] AM3517 : support for suspend/resume
Date: Tue, 30 Aug 2011 15:58:47 -0700 [thread overview]
Message-ID: <87fwkild5k.fsf@ti.com> (raw)
In-Reply-To: <1313754927-11992-2-git-send-email-abhilash.kv@ti.com> (Abhilash K. V.'s message of "Fri, 19 Aug 2011 17:25:24 +0530")
Abhilash K V <abhilash.kv@ti.com> writes:
> 1. Patch to disable dynamic sleep (as it is not supported
> on AM35xx).
> 2. Imported the unique suspend/resume sequence for AM3517,
> contained in the new file arch/arm/mach-omap2/sleep3517.S.
> 3. Added omap3517_ to symbol-names in sleep3517.S which are common
> with sleep34xx.S, and added appropriate checks.
>
> There are still 3 caveats:
>
> 1. If "no_console_suspend" is enabled (via boot-args), the device
> doesnot resume but simply hangs.
> 2. Every second and subsequent attempt to suspend/resume prints this slow-path
> WARNING (for both uart1 and uart2), while resuming :
> [ 70.943939] omap_hwmod: uart1: idle state can only be entered from
> enabled state
> 3. Wakeup using the TSC2004 touch-screen controller is not supported.
>
> Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
> Reviewed-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Signed-off-by: Abhilash K V <abhilash.kv@ti.com>
In addition to Russell's comments about using the latest code from
mainline, I have some comments below.
> ---
> arch/arm/mach-omap2/Makefile | 2 +-
> arch/arm/mach-omap2/control.c | 7 ++-
> arch/arm/mach-omap2/control.h | 1 +
> arch/arm/mach-omap2/pm.h | 4 +
> arch/arm/mach-omap2/pm34xx.c | 18 ++++-
> arch/arm/mach-omap2/sleep3517.S | 144 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 170 insertions(+), 6 deletions(-)
> create mode 100644 arch/arm/mach-omap2/sleep3517.S
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index 46f5fbc..3fdf086 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -61,7 +61,7 @@ endif
> ifeq ($(CONFIG_PM),y)
> obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o
> obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o
> -obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o \
> +obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o sleep3517.o \
> cpuidle34xx.o
> obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o
> obj-$(CONFIG_PM_DEBUG) += pm-debug.o
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index da53ba3..7d2d8a8 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -284,10 +284,13 @@ void omap3_save_scratchpad_contents(void)
> * The restore pointer is stored into the scratchpad.
> */
> scratchpad_contents.boot_config_ptr = 0x0;
> - if (cpu_is_omap3630())
> + if (cpu_is_omap3505() || cpu_is_omap3517()) {
> + scratchpad_contents.public_restore_ptr =
> + virt_to_phys(omap3517_get_restore_pointer());
Based on the contents of the get_restore_pointer() added below, this
value is obviously not being used. Either off-mode was not tested (or not
supported.) Either way, unused code like this should not be
added if it is not tested or supported.
> + } else if (cpu_is_omap3630()) {
> scratchpad_contents.public_restore_ptr =
> virt_to_phys(get_omap3630_restore_pointer());
> - else if (omap_rev() != OMAP3430_REV_ES3_0 &&
> + } else if (omap_rev() != OMAP3430_REV_ES3_0 &&
> omap_rev() != OMAP3430_REV_ES3_1)
> scratchpad_contents.public_restore_ptr =
> virt_to_phys(get_restore_pointer());
> diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
> index ad024df..3003940 100644
> --- a/arch/arm/mach-omap2/control.h
> +++ b/arch/arm/mach-omap2/control.h
> @@ -389,6 +389,7 @@ extern void omap4_ctrl_pad_writel(u32 val, u16 offset);
> extern void omap3_save_scratchpad_contents(void);
> extern void omap3_clear_scratchpad_contents(void);
> extern u32 *get_restore_pointer(void);
> +extern u32 *omap3517_get_restore_pointer(void);
> extern u32 *get_es3_restore_pointer(void);
> extern u32 *get_omap3630_restore_pointer(void);
> extern u32 omap3_arm_context[128];
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 5c2bd2f..d773e07 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -77,13 +77,17 @@ extern void omap24xx_idle_loop_suspend(void);
> extern void omap24xx_cpu_suspend(u32 dll_ctrl, void __iomem *sdrc_dlla_ctrl,
> void __iomem *sdrc_power);
> extern void omap34xx_cpu_suspend(u32 *addr, int save_state);
> +extern void omap3517_cpu_suspend(u32 *addr, int save_state);
> extern int save_secure_ram_context(u32 *addr);
> +extern void omap3517_save_secure_ram_context(u32 *addr);
> extern void omap3_save_scratchpad_contents(void);
>
> extern unsigned int omap24xx_idle_loop_suspend_sz;
> extern unsigned int save_secure_ram_context_sz;
> +extern unsigned int omap3517_save_secure_ram_context_sz;
> extern unsigned int omap24xx_cpu_suspend_sz;
> extern unsigned int omap34xx_cpu_suspend_sz;
> +extern unsigned int omap3517_cpu_suspend_sz;
>
> #define PM_RTA_ERRATUM_i608 (1 << 0)
> #define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1)
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 96a7624..12af5b9 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -497,6 +497,8 @@ console_still_active:
>
> int omap3_can_sleep(void)
> {
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + return 0;
> if (!omap_uart_can_sleep())
> return 0;
> return 1;
> @@ -848,11 +850,21 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
>
> void omap_push_sram_idle(void)
> {
> - _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + _omap_sram_idle = omap_sram_push(omap3517_cpu_suspend,
> + omap3517_cpu_suspend_sz);
> + else
> + _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> omap34xx_cpu_suspend_sz);
> if (omap_type() != OMAP2_DEVICE_TYPE_GP)
> - _omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
> - save_secure_ram_context_sz);
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + _omap_save_secure_sram = omap_sram_push(
> + omap3517_save_secure_ram_context,
> + omap3517_save_secure_ram_context_sz);
> + else
> + _omap_save_secure_sram = omap_sram_push(
> + save_secure_ram_context,
> + save_secure_ram_context_sz);
> }
You add a new assembly function for save secure context, but that
function is essentially a nop.
It would be better to just not have a function for these devices.
To that end, it would help to add an OMAP "feature" stating whether or
not the device has secure RAM. Then, instead of the cpu_is_* checks
here, that feature flag can be checked.
>
> static void __init pm_errata_configure(void)
> diff --git a/arch/arm/mach-omap2/sleep3517.S b/arch/arm/mach-omap2/sleep3517.S
> new file mode 100644
> index 0000000..3fceefc
> --- /dev/null
> +++ b/arch/arm/mach-omap2/sleep3517.S
> @@ -0,0 +1,144 @@
> +/*
> +/* linux/arch/arm/mach-omap2/sleep3517.S
You can leave out filenames in comments. Files tend to move, and the
comments don't get changed and become stale.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@ti.com>
To: Abhilash K V <abhilash.kv@ti.com>
Cc: linux@arm.linux.org.uk, Ranjith Lohithakshan <ranjithl@ti.com>,
tony@atomide.com, linux-kernel@vger.kernel.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/4] AM3517 : support for suspend/resume
Date: Tue, 30 Aug 2011 15:58:47 -0700 [thread overview]
Message-ID: <87fwkild5k.fsf@ti.com> (raw)
In-Reply-To: <1313754927-11992-2-git-send-email-abhilash.kv@ti.com> (Abhilash K. V.'s message of "Fri, 19 Aug 2011 17:25:24 +0530")
Abhilash K V <abhilash.kv@ti.com> writes:
> 1. Patch to disable dynamic sleep (as it is not supported
> on AM35xx).
> 2. Imported the unique suspend/resume sequence for AM3517,
> contained in the new file arch/arm/mach-omap2/sleep3517.S.
> 3. Added omap3517_ to symbol-names in sleep3517.S which are common
> with sleep34xx.S, and added appropriate checks.
>
> There are still 3 caveats:
>
> 1. If "no_console_suspend" is enabled (via boot-args), the device
> doesnot resume but simply hangs.
> 2. Every second and subsequent attempt to suspend/resume prints this slow-path
> WARNING (for both uart1 and uart2), while resuming :
> [ 70.943939] omap_hwmod: uart1: idle state can only be entered from
> enabled state
> 3. Wakeup using the TSC2004 touch-screen controller is not supported.
>
> Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
> Reviewed-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Signed-off-by: Abhilash K V <abhilash.kv@ti.com>
In addition to Russell's comments about using the latest code from
mainline, I have some comments below.
> ---
> arch/arm/mach-omap2/Makefile | 2 +-
> arch/arm/mach-omap2/control.c | 7 ++-
> arch/arm/mach-omap2/control.h | 1 +
> arch/arm/mach-omap2/pm.h | 4 +
> arch/arm/mach-omap2/pm34xx.c | 18 ++++-
> arch/arm/mach-omap2/sleep3517.S | 144 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 170 insertions(+), 6 deletions(-)
> create mode 100644 arch/arm/mach-omap2/sleep3517.S
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index 46f5fbc..3fdf086 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -61,7 +61,7 @@ endif
> ifeq ($(CONFIG_PM),y)
> obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o
> obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o
> -obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o \
> +obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o sleep3517.o \
> cpuidle34xx.o
> obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o
> obj-$(CONFIG_PM_DEBUG) += pm-debug.o
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index da53ba3..7d2d8a8 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -284,10 +284,13 @@ void omap3_save_scratchpad_contents(void)
> * The restore pointer is stored into the scratchpad.
> */
> scratchpad_contents.boot_config_ptr = 0x0;
> - if (cpu_is_omap3630())
> + if (cpu_is_omap3505() || cpu_is_omap3517()) {
> + scratchpad_contents.public_restore_ptr =
> + virt_to_phys(omap3517_get_restore_pointer());
Based on the contents of the get_restore_pointer() added below, this
value is obviously not being used. Either off-mode was not tested (or not
supported.) Either way, unused code like this should not be
added if it is not tested or supported.
> + } else if (cpu_is_omap3630()) {
> scratchpad_contents.public_restore_ptr =
> virt_to_phys(get_omap3630_restore_pointer());
> - else if (omap_rev() != OMAP3430_REV_ES3_0 &&
> + } else if (omap_rev() != OMAP3430_REV_ES3_0 &&
> omap_rev() != OMAP3430_REV_ES3_1)
> scratchpad_contents.public_restore_ptr =
> virt_to_phys(get_restore_pointer());
> diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
> index ad024df..3003940 100644
> --- a/arch/arm/mach-omap2/control.h
> +++ b/arch/arm/mach-omap2/control.h
> @@ -389,6 +389,7 @@ extern void omap4_ctrl_pad_writel(u32 val, u16 offset);
> extern void omap3_save_scratchpad_contents(void);
> extern void omap3_clear_scratchpad_contents(void);
> extern u32 *get_restore_pointer(void);
> +extern u32 *omap3517_get_restore_pointer(void);
> extern u32 *get_es3_restore_pointer(void);
> extern u32 *get_omap3630_restore_pointer(void);
> extern u32 omap3_arm_context[128];
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 5c2bd2f..d773e07 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -77,13 +77,17 @@ extern void omap24xx_idle_loop_suspend(void);
> extern void omap24xx_cpu_suspend(u32 dll_ctrl, void __iomem *sdrc_dlla_ctrl,
> void __iomem *sdrc_power);
> extern void omap34xx_cpu_suspend(u32 *addr, int save_state);
> +extern void omap3517_cpu_suspend(u32 *addr, int save_state);
> extern int save_secure_ram_context(u32 *addr);
> +extern void omap3517_save_secure_ram_context(u32 *addr);
> extern void omap3_save_scratchpad_contents(void);
>
> extern unsigned int omap24xx_idle_loop_suspend_sz;
> extern unsigned int save_secure_ram_context_sz;
> +extern unsigned int omap3517_save_secure_ram_context_sz;
> extern unsigned int omap24xx_cpu_suspend_sz;
> extern unsigned int omap34xx_cpu_suspend_sz;
> +extern unsigned int omap3517_cpu_suspend_sz;
>
> #define PM_RTA_ERRATUM_i608 (1 << 0)
> #define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1)
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 96a7624..12af5b9 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -497,6 +497,8 @@ console_still_active:
>
> int omap3_can_sleep(void)
> {
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + return 0;
> if (!omap_uart_can_sleep())
> return 0;
> return 1;
> @@ -848,11 +850,21 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
>
> void omap_push_sram_idle(void)
> {
> - _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + _omap_sram_idle = omap_sram_push(omap3517_cpu_suspend,
> + omap3517_cpu_suspend_sz);
> + else
> + _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> omap34xx_cpu_suspend_sz);
> if (omap_type() != OMAP2_DEVICE_TYPE_GP)
> - _omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
> - save_secure_ram_context_sz);
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + _omap_save_secure_sram = omap_sram_push(
> + omap3517_save_secure_ram_context,
> + omap3517_save_secure_ram_context_sz);
> + else
> + _omap_save_secure_sram = omap_sram_push(
> + save_secure_ram_context,
> + save_secure_ram_context_sz);
> }
You add a new assembly function for save secure context, but that
function is essentially a nop.
It would be better to just not have a function for these devices.
To that end, it would help to add an OMAP "feature" stating whether or
not the device has secure RAM. Then, instead of the cpu_is_* checks
here, that feature flag can be checked.
>
> static void __init pm_errata_configure(void)
> diff --git a/arch/arm/mach-omap2/sleep3517.S b/arch/arm/mach-omap2/sleep3517.S
> new file mode 100644
> index 0000000..3fceefc
> --- /dev/null
> +++ b/arch/arm/mach-omap2/sleep3517.S
> @@ -0,0 +1,144 @@
> +/*
> +/* linux/arch/arm/mach-omap2/sleep3517.S
You can leave out filenames in comments. Files tend to move, and the
comments don't get changed and become stale.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: khilman@ti.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] AM3517 : support for suspend/resume
Date: Tue, 30 Aug 2011 15:58:47 -0700 [thread overview]
Message-ID: <87fwkild5k.fsf@ti.com> (raw)
In-Reply-To: <1313754927-11992-2-git-send-email-abhilash.kv@ti.com> (Abhilash K. V.'s message of "Fri, 19 Aug 2011 17:25:24 +0530")
Abhilash K V <abhilash.kv@ti.com> writes:
> 1. Patch to disable dynamic sleep (as it is not supported
> on AM35xx).
> 2. Imported the unique suspend/resume sequence for AM3517,
> contained in the new file arch/arm/mach-omap2/sleep3517.S.
> 3. Added omap3517_ to symbol-names in sleep3517.S which are common
> with sleep34xx.S, and added appropriate checks.
>
> There are still 3 caveats:
>
> 1. If "no_console_suspend" is enabled (via boot-args), the device
> doesnot resume but simply hangs.
> 2. Every second and subsequent attempt to suspend/resume prints this slow-path
> WARNING (for both uart1 and uart2), while resuming :
> [ 70.943939] omap_hwmod: uart1: idle state can only be entered from
> enabled state
> 3. Wakeup using the TSC2004 touch-screen controller is not supported.
>
> Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
> Reviewed-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Signed-off-by: Abhilash K V <abhilash.kv@ti.com>
In addition to Russell's comments about using the latest code from
mainline, I have some comments below.
> ---
> arch/arm/mach-omap2/Makefile | 2 +-
> arch/arm/mach-omap2/control.c | 7 ++-
> arch/arm/mach-omap2/control.h | 1 +
> arch/arm/mach-omap2/pm.h | 4 +
> arch/arm/mach-omap2/pm34xx.c | 18 ++++-
> arch/arm/mach-omap2/sleep3517.S | 144 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 170 insertions(+), 6 deletions(-)
> create mode 100644 arch/arm/mach-omap2/sleep3517.S
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index 46f5fbc..3fdf086 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -61,7 +61,7 @@ endif
> ifeq ($(CONFIG_PM),y)
> obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o
> obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o
> -obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o \
> +obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o sleep3517.o \
> cpuidle34xx.o
> obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o
> obj-$(CONFIG_PM_DEBUG) += pm-debug.o
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index da53ba3..7d2d8a8 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -284,10 +284,13 @@ void omap3_save_scratchpad_contents(void)
> * The restore pointer is stored into the scratchpad.
> */
> scratchpad_contents.boot_config_ptr = 0x0;
> - if (cpu_is_omap3630())
> + if (cpu_is_omap3505() || cpu_is_omap3517()) {
> + scratchpad_contents.public_restore_ptr =
> + virt_to_phys(omap3517_get_restore_pointer());
Based on the contents of the get_restore_pointer() added below, this
value is obviously not being used. Either off-mode was not tested (or not
supported.) Either way, unused code like this should not be
added if it is not tested or supported.
> + } else if (cpu_is_omap3630()) {
> scratchpad_contents.public_restore_ptr =
> virt_to_phys(get_omap3630_restore_pointer());
> - else if (omap_rev() != OMAP3430_REV_ES3_0 &&
> + } else if (omap_rev() != OMAP3430_REV_ES3_0 &&
> omap_rev() != OMAP3430_REV_ES3_1)
> scratchpad_contents.public_restore_ptr =
> virt_to_phys(get_restore_pointer());
> diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
> index ad024df..3003940 100644
> --- a/arch/arm/mach-omap2/control.h
> +++ b/arch/arm/mach-omap2/control.h
> @@ -389,6 +389,7 @@ extern void omap4_ctrl_pad_writel(u32 val, u16 offset);
> extern void omap3_save_scratchpad_contents(void);
> extern void omap3_clear_scratchpad_contents(void);
> extern u32 *get_restore_pointer(void);
> +extern u32 *omap3517_get_restore_pointer(void);
> extern u32 *get_es3_restore_pointer(void);
> extern u32 *get_omap3630_restore_pointer(void);
> extern u32 omap3_arm_context[128];
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 5c2bd2f..d773e07 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -77,13 +77,17 @@ extern void omap24xx_idle_loop_suspend(void);
> extern void omap24xx_cpu_suspend(u32 dll_ctrl, void __iomem *sdrc_dlla_ctrl,
> void __iomem *sdrc_power);
> extern void omap34xx_cpu_suspend(u32 *addr, int save_state);
> +extern void omap3517_cpu_suspend(u32 *addr, int save_state);
> extern int save_secure_ram_context(u32 *addr);
> +extern void omap3517_save_secure_ram_context(u32 *addr);
> extern void omap3_save_scratchpad_contents(void);
>
> extern unsigned int omap24xx_idle_loop_suspend_sz;
> extern unsigned int save_secure_ram_context_sz;
> +extern unsigned int omap3517_save_secure_ram_context_sz;
> extern unsigned int omap24xx_cpu_suspend_sz;
> extern unsigned int omap34xx_cpu_suspend_sz;
> +extern unsigned int omap3517_cpu_suspend_sz;
>
> #define PM_RTA_ERRATUM_i608 (1 << 0)
> #define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1)
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 96a7624..12af5b9 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -497,6 +497,8 @@ console_still_active:
>
> int omap3_can_sleep(void)
> {
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + return 0;
> if (!omap_uart_can_sleep())
> return 0;
> return 1;
> @@ -848,11 +850,21 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
>
> void omap_push_sram_idle(void)
> {
> - _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + _omap_sram_idle = omap_sram_push(omap3517_cpu_suspend,
> + omap3517_cpu_suspend_sz);
> + else
> + _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> omap34xx_cpu_suspend_sz);
> if (omap_type() != OMAP2_DEVICE_TYPE_GP)
> - _omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
> - save_secure_ram_context_sz);
> + if (cpu_is_omap3505() || cpu_is_omap3517())
> + _omap_save_secure_sram = omap_sram_push(
> + omap3517_save_secure_ram_context,
> + omap3517_save_secure_ram_context_sz);
> + else
> + _omap_save_secure_sram = omap_sram_push(
> + save_secure_ram_context,
> + save_secure_ram_context_sz);
> }
You add a new assembly function for save secure context, but that
function is essentially a nop.
It would be better to just not have a function for these devices.
To that end, it would help to add an OMAP "feature" stating whether or
not the device has secure RAM. Then, instead of the cpu_is_* checks
here, that feature flag can be checked.
>
> static void __init pm_errata_configure(void)
> diff --git a/arch/arm/mach-omap2/sleep3517.S b/arch/arm/mach-omap2/sleep3517.S
> new file mode 100644
> index 0000000..3fceefc
> --- /dev/null
> +++ b/arch/arm/mach-omap2/sleep3517.S
> @@ -0,0 +1,144 @@
> +/*
> +/* linux/arch/arm/mach-omap2/sleep3517.S
You can leave out filenames in comments. Files tend to move, and the
comments don't get changed and become stale.
Kevin
next prev parent reply other threads:[~2011-08-30 22:58 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-19 11:55 [PATCH 0/4] AM3517: Adding support for suspend/resume Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` [PATCH 1/4] AM3517 : " Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` [PATCH 2/4] AM3517: Add armv7-a flag for sleepam3517.S Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` [PATCH 3/4] pm34xx: Warning FIX related to ambiguous else loop Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` [PATCH 4/4] AM3517: Fix suspend-resume sequence Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 11:55 ` Abhilash K V
2011-08-19 19:53 ` Russell King - ARM Linux
2011-08-19 19:53 ` Russell King - ARM Linux
2011-08-30 23:00 ` [PATCH 3/4] pm34xx: Warning FIX related to ambiguous else loop Kevin Hilman
2011-08-30 23:00 ` Kevin Hilman
2011-08-19 19:56 ` [PATCH 1/4] AM3517 : support for suspend/resume Russell King - ARM Linux
2011-08-19 19:56 ` Russell King - ARM Linux
2011-08-30 22:58 ` Kevin Hilman [this message]
2011-08-30 22:58 ` Kevin Hilman
2011-08-30 22:58 ` Kevin Hilman
2011-09-13 11:31 ` Koyamangalath, Abhilash
2011-09-13 11:31 ` Koyamangalath, Abhilash
2011-09-13 11:31 ` Koyamangalath, Abhilash
2011-09-13 18:24 ` Kevin Hilman
2011-09-13 18:24 ` Kevin Hilman
2011-09-13 18:24 ` Kevin Hilman
2011-09-14 13:00 ` Koyamangalath, Abhilash
2011-09-14 13:00 ` Koyamangalath, Abhilash
2011-09-14 13:00 ` Koyamangalath, Abhilash
2011-09-15 17:09 ` Kevin Hilman
2011-09-15 17:09 ` Kevin Hilman
2011-09-15 17:09 ` Kevin Hilman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fwkild5k.fsf@ti.com \
--to=khilman@ti.com \
--cc=abhilash.kv@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=ranjithl@ti.com \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.