RE: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Daniel, On Wed, Apr 03, 2013 at 17:22:41, Daniel Mack wrote: Hi Vaibhav, On Mon, Dec 31, 2012 at 2:07 PM, Vaibhav Bedia vaibhav.be...@ti.com wrote: AM335x supports various low power modes as documented in section 8.1.4.3 of the AM335x TRM which is available @ http://www.ti.com/litv/pdf/spruh73f May I ask about the plans for this series? Will you be re-spinning them for a current tree, and what's the planned merge window for it? I am tied up in some other stuff right now. I plan to address Kevin's comments and repost the patches soon. Since rc5 is already out i think v3.10 is not a realistic target any more but I do hope to have this accepted for v3.11. Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Vaibhav, On Mon, Dec 31, 2012 at 2:07 PM, Vaibhav Bedia vaibhav.be...@ti.com wrote: AM335x supports various low power modes as documented in section 8.1.4.3 of the AM335x TRM which is available @ http://www.ti.com/litv/pdf/spruh73f May I ask about the plans for this series? Will you be re-spinning them for a current tree, and what's the planned merge window for it? Many thanks, Daniel DeepSleep0 mode offers the lowest power mode with limited wakeup sources without a system reboot and is mapped as the suspend state in the kernel. In this state, MPU and PER domains are turned off with the internal RAM held in retention to facilitate resume process. As part of the boot process, the assembly code is copied over to OCMCRAM using the OMAP SRAM code. AM335x has a Cortex-M3 (WKUP_M3) which assists the MPU in DeepSleep0 entry and exit. WKUP_M3 takes care of the clockdomain and powerdomain transitions based on the intended low power state. MPU needs to load the appropriate WKUP_M3 binary onto the WKUP_M3 memory space before it can leverage any of the PM features like DeepSleep. The IPC mechanism between MPU and WKUP_M3 uses a mailbox sub-module and 8 IPC registers in the Control module. MPU uses the assigned Mailbox for issuing an interrupt to WKUP_M3 which then goes and checks the IPC registers for the payload. WKUP_M3 has the ability to trigger on interrupt to MPU by executing the sev instruction. In the current implementation when the suspend process is initiated MPU interrupts the WKUP_M3 to let it know about the intent of entering DeepSleep0 and waits for an ACK. When the ACK is received MPU continues with its suspend process to suspend all the drivers and then jumps to assembly in OCMC RAM. The assembly code puts the PLLs in bypass, puts the external RAM in self-refresh mode and then finally execute the WFI instruction. Execution of the WFI instruction triggers another interrupt to the WKUP_M3 which then continues wiht the power down sequence wherein the clockdomain and powerdomain transition takes place. As part of the sleep sequence, WKUP_M3 unmasks the interrupt lines for the wakeup sources. WFI execution on WKUP_M3 causes the hardware to disable the main oscillator of the SoC. When a wakeup event occurs, WKUP_M3 starts the power-up sequence by switching on the power domains and finally enabling the clock to MPU. Since the MPU gets powered down as part of the sleep sequence in the resume path ROM code starts executing. The ROM code detects a wakeup from sleep and then jumps to the resume location in OCMC which was populated in one of the IPC registers as part of the suspend sequence. The low level code in OCMC relocks the PLLs, enables access to external RAM and then jumps to the cpu_resume code of the kernel to finish the resume process. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Cc: Tony Lingren t...@atomide.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@deeprootsystems.com --- v1-v2: Move assembly code addition, control module access and hookup in OMAP PM framework in separate patches. Address other comments from Kevin Hilman and Santosh Shilimkar on v1. The discussion on v1 is present @ http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129979.html Note: The mailbox change will need slight rework once the driver is finalized. arch/arm/mach-omap2/pm33xx.c | 469 ++ arch/arm/mach-omap2/pm33xx.h | 56 + 2 files changed, 525 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-omap2/pm33xx.c create mode 100644 arch/arm/mach-omap2/pm33xx.h diff --git a/arch/arm/mach-omap2/pm33xx.c b/arch/arm/mach-omap2/pm33xx.c new file mode 100644 index 000..aaa4daa --- /dev/null +++ b/arch/arm/mach-omap2/pm33xx.c @@ -0,0 +1,469 @@ +/* + * AM33XX Power Management Routines + * + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ + * Vaibhav Bedia vaibhav.be...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed as is WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/kernel.h +#include linux/init.h +#include linux/err.h +#include linux/firmware.h +#include linux/io.h +#include linux/platform_device.h +#include linux/sched.h +#include linux/suspend.h +#include linux/completion.h +#include linux/module.h +#include linux/mailbox.h +#include linux/interrupt.h + +#include
RE: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
On Mon, Feb 18, 2013 at 21:41:49, Kevin Hilman wrote: [...] By default these IPs don't have MSTANDBY asserted. When you say by default, I guess you mean after reset (and/or context loss), right? Yes When a low power transition happens, the peripheral power domain loses context and hence the forced MSTANDBY configuration in the IP is lost. To work around this problem we need to assert MSTANDBY in every suspend-resume iteration. Yuck. More clever hardware. ;) We are getting this gradually :) I agree that this might hide PM bugs in the driver but to solve this problem we need some mechanism for the PM code to know whether or not a driver is bound to the corresponding platform devices. Any suggestions on this? Driver bound status can be tracked easily using bus notifiers. You can see an example in the omap_device core. Ok. I'll try to use the driver bound status in the next version. [...] +/* + * GFX_L4LS clock domain needs to be woken up to + * ensure thet L4LS clock domain does not get stuck in transition + * If that happens L3 module does not get disabled, thereby leading + * to PER power domain transition failing + * + * The clock framework should take care of ensuring + * that the clock domain is in the right state when + * GFX driver is active. Are you suggesting that the clock framework is not doing this already? No. This clkdm_*() calls are here to work-around an issue that I observed when implementing suspend-resume. The force wakeup and sleep of the gfx_l4ls clock domain across every suspend-resume is something I don't think can be pushed to the clock framework. I still don't follow what you're suggesting the clock framework should do. Are you describing the case when there is a GFX driver vs. when there isn't a driver? If so, it needs to be more clear. No. The issue with GFX_L4LS happens irrespective of the state of the GFX driver and needs to be handled in the suspend-resume sequence. I guess the second part of comment is what created the confusion. I'll get rid of that. Also, some more description about why the device gets 'stuck in transition' would be helpful. Is this an erratum workaround? I'll follow up with the design folks to find out more. From some past discussions this is not expected so looks like we need to an erratum published for this issue. I see, then probably a TODO here with more description would be more helpful. So, IIUC, without implemeting this, the drivers will never be able to detect loss of context, correct? Sounds like something that should be decribed in the changelog as that's a rather important limitation to this implementaion. Ok. I'll address this limitation in the next version and improve the changelog. + +/* Give some time to M3 to respond. 500msec is a random value here */ random? really? Sort of. I don't have any numbers from the h/w guys on the worst case delay in getting an interrupt from M3 to MPU. At the same time I want to handle the scenario where something goes wrong on the M3 side and it doesn't respond. OK, then it's not random. You have some reasoning behind the number that should be documented. Will do. That being said, in the absence of numbers from HW folks, can't you measure the typical times so you know roughly what's normal. I'll do some timer based profiling and get rough numbers for this. [...] Why not omap_device_enable(pdev) ? The objective is to leverage the hwmod code to get the WKUP-M3 functional and not have OMAP runtime PM code coming in the way. FWIW, it is not runtime PM getting in the way. Using omap_device_enable() triggers the following dev_warn() from omap_device_enable(). Looking closer at the trace, you'll see it's not omap_device_enable() that is triggering this warning. What is happening is that omap_device has a late_initcall which forcibly idles omap_devices that have been enabled, but that don't have a driver. You're hacking around that. Thanks for the explanation. I should have looked closer :( IMO, this would be a much cleaner implementation if you just created a small driver for the wkup_m3. You're already doing a bunch of driver-like stuff for it (requesting base/IRQs, mapping regions, firmware, etc.) I think this should separated out into a real driver. Then it will be bound to the right omap_device, and normal PM operations will work as expected. Also, doing it this way will be more flexible for those wanting to use their own firmware on the M3, or customize the current firmware. Hmm... that definitely sounds more flexible. It should also help in the next SoC AM437x which has a similar PM architecture. Where would you suggest placing this minimal driver? Regards, Vaibhav -- To unsubscribe from this list: send the line
Re: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Bedia, Vaibhav vaibhav.be...@ti.com writes: [...] IMO, this would be a much cleaner implementation if you just created a small driver for the wkup_m3. You're already doing a bunch of driver-like stuff for it (requesting base/IRQs, mapping regions, firmware, etc.) I think this should separated out into a real driver. Then it will be bound to the right omap_device, and normal PM operations will work as expected. Also, doing it this way will be more flexible for those wanting to use their own firmware on the M3, or customize the current firmware. Hmm... that definitely sounds more flexible. It should also help in the next SoC AM437x which has a similar PM architecture. Where would you suggest placing this minimal driver? For now, just leave it in mach-omap2 and we can figure out the right home for it eventually. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Bedia, Vaibhav vaibhav.be...@ti.com writes:
Hi Kevin,
On Tue, Feb 12, 2013 at 06:57:50, Kevin Hilman wrote:
[...]
+
+void (*am33xx_do_wfi_sram)(void);
static?
Will fix.
[...]
+
+ /*
+ * By default the following IPs do not have MSTANDBY asserted
+ * which is necessary for PER domain transition. If the drivers
+ * are not compiled into the kernel HWMOD code will not change the
+ * state of the IPs if the IP was not never enabled. To ensure
+ * that there no issues with or without the drivers being compiled
+ * in the kernel, we forcefully put these IPs to idle.
+ */
+ omap_hwmod_enable(usb_oh);
+ omap_hwmod_enable(tptc0_oh);
+ omap_hwmod_enable(tptc1_oh);
+ omap_hwmod_enable(tptc2_oh);
+ omap_hwmod_enable(cpsw_oh);
+
+ omap_hwmod_idle(usb_oh);
+ omap_hwmod_idle(tptc0_oh);
+ omap_hwmod_idle(tptc1_oh);
+ omap_hwmod_idle(tptc2_oh);
+ omap_hwmod_idle(cpsw_oh);
I think I asked this in my review of v1, but why does this need to
happen on every suspend attempt?
This should happen once on init, which will handle the case where there
are no drivers, and if there are drivers, then the drivers need to
handle this properly.
I don't like this happening here on every suspend attempt, because it
will surely hide bugs where drivers are not properly managing their own PM.
By default these IPs don't have MSTANDBY asserted.
When you say by default, I guess you mean after reset (and/or context
loss), right?
When a low power transition happens, the peripheral power domain loses
context and hence the forced MSTANDBY configuration in the IP is
lost. To work around this problem we need to assert MSTANDBY in every
suspend-resume iteration.
Yuck. More clever hardware. ;)
I agree that this might hide PM bugs in the driver but to solve this problem
we
need some mechanism for the PM code to know whether or not a driver is bound
to the corresponding platform devices. Any suggestions on this?
Driver bound status can be tracked easily using bus notifiers. You can
see an example in the omap_device core.
+ /* Try to put GFX to sleep */
+ pwrdm_set_next_fpwrst(gfx_pwrdm, PWRDM_FUNC_PWRST_OFF);
+
+ ret = cpu_suspend(0, am33xx_do_sram_idle);
+ status = pwrdm_read_fpwrst(gfx_pwrdm);
+ if (status != PWRDM_FUNC_PWRST_OFF)
+ pr_err(GFX domain did not transition\n);
+ else
+ pr_info(GFX domain entered low power state\n);
Do you really want this printed every time?
Hmm... it could perhaps be clubbed with the overall status that's
printed. I kept it here since the GFX power domain is completely
under MPU control and hence this information would be useful in
finding out if there's a problem with the GFX suspend-resume.
OK.
+ /*
+ * GFX_L4LS clock domain needs to be woken up to
+ * ensure thet L4LS clock domain does not get stuck in transition
+ * If that happens L3 module does not get disabled, thereby leading
+ * to PER power domain transition failing
+ *
+ * The clock framework should take care of ensuring
+ * that the clock domain is in the right state when
+ * GFX driver is active.
Are you suggesting that the clock framework is not doing this already?
No. This clkdm_*() calls are here to work-around an issue that I observed
when implementing suspend-resume. The force wakeup and sleep of the gfx_l4ls
clock domain across every suspend-resume is something I don't think can
be pushed to the clock framework.
I still don't follow what you're suggesting the clock framework should
do. Are you describing the case when there is a GFX driver vs. when
there isn't a driver? If so, it needs to be more clear.
Also, some more description about why the device gets 'stuck in
transition' would be helpful. Is this an erratum workaround?
+ */
+ clkdm_wakeup(gfx_l4ls_clkdm);
+ clkdm_sleep(gfx_l4ls_clkdm);
+
+ if (ret) {
+ pr_err(Kernel suspend failure\n);
+ } else {
+ status = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG1);
We're trying to git rid of direct control module register access, and
consolidate them into control.c (for an eventual move to a driver.) I
see you've mostly done that in other parts of the series, but here's one
that needs to move.
Yes, I somehow missed this one. Will take care of it in the next version.
+ status = IPC_RESP_MASK;
+ status = __ffs(IPC_RESP_MASK);
+
+ switch (status) {
+ case 0:
+ pr_info(Successfully put all powerdomains to target
state\n);
+ /*
+ * XXX: Leads to loss of logic state in PER power domain
+ * Use SOC specific ops for this?
+ */
huh?
Ah... this is more of a TODO. There's no previous state entered information
in the PRCM registers. So to ensure that the drivers get the right information
when they
RE: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Kevin,
On Tue, Feb 12, 2013 at 06:57:50, Kevin Hilman wrote:
[...]
+
+void (*am33xx_do_wfi_sram)(void);
static?
Will fix.
[...]
+
+ /*
+* By default the following IPs do not have MSTANDBY asserted
+* which is necessary for PER domain transition. If the drivers
+* are not compiled into the kernel HWMOD code will not change the
+* state of the IPs if the IP was not never enabled. To ensure
+* that there no issues with or without the drivers being compiled
+* in the kernel, we forcefully put these IPs to idle.
+*/
+ omap_hwmod_enable(usb_oh);
+ omap_hwmod_enable(tptc0_oh);
+ omap_hwmod_enable(tptc1_oh);
+ omap_hwmod_enable(tptc2_oh);
+ omap_hwmod_enable(cpsw_oh);
+
+ omap_hwmod_idle(usb_oh);
+ omap_hwmod_idle(tptc0_oh);
+ omap_hwmod_idle(tptc1_oh);
+ omap_hwmod_idle(tptc2_oh);
+ omap_hwmod_idle(cpsw_oh);
I think I asked this in my review of v1, but why does this need to
happen on every suspend attempt?
This should happen once on init, which will handle the case where there
are no drivers, and if there are drivers, then the drivers need to
handle this properly.
I don't like this happening here on every suspend attempt, because it
will surely hide bugs where drivers are not properly managing their own PM.
By default these IPs don't have MSTANDBY asserted. When a low power transition
happens, the peripheral power domain loses context and hence the forced
MSTANDBY configuration in the IP is lost. To work around this problem we need
to assert MSTANDBY in every suspend-resume iteration.
I agree that this might hide PM bugs in the driver but to solve this problem we
need some mechanism for the PM code to know whether or not a driver is bound
to the corresponding platform devices. Any suggestions on this?
+ /* Try to put GFX to sleep */
+ pwrdm_set_next_fpwrst(gfx_pwrdm, PWRDM_FUNC_PWRST_OFF);
+
+ ret = cpu_suspend(0, am33xx_do_sram_idle);
+ status = pwrdm_read_fpwrst(gfx_pwrdm);
+ if (status != PWRDM_FUNC_PWRST_OFF)
+ pr_err(GFX domain did not transition\n);
+ else
+ pr_info(GFX domain entered low power state\n);
Do you really want this printed every time?
Hmm... it could perhaps be clubbed with the overall status that's
printed. I kept it here since the GFX power domain is completely
under MPU control and hence this information would be useful in
finding out if there's a problem with the GFX suspend-resume.
+ /*
+* GFX_L4LS clock domain needs to be woken up to
+* ensure thet L4LS clock domain does not get stuck in transition
+* If that happens L3 module does not get disabled, thereby leading
+* to PER power domain transition failing
+*
+* The clock framework should take care of ensuring
+* that the clock domain is in the right state when
+* GFX driver is active.
Are you suggesting that the clock framework is not doing this already?
No. This clkdm_*() calls are here to work-around an issue that I observed
when implementing suspend-resume. The force wakeup and sleep of the gfx_l4ls
clock domain across every suspend-resume is something I don't think can
be pushed to the clock framework.
+*/
+ clkdm_wakeup(gfx_l4ls_clkdm);
+ clkdm_sleep(gfx_l4ls_clkdm);
+
+ if (ret) {
+ pr_err(Kernel suspend failure\n);
+ } else {
+ status = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG1);
We're trying to git rid of direct control module register access, and
consolidate them into control.c (for an eventual move to a driver.) I
see you've mostly done that in other parts of the series, but here's one
that needs to move.
Yes, I somehow missed this one. Will take care of it in the next version.
+ status = IPC_RESP_MASK;
+ status = __ffs(IPC_RESP_MASK);
+
+ switch (status) {
+ case 0:
+ pr_info(Successfully put all powerdomains to target
state\n);
+ /*
+* XXX: Leads to loss of logic state in PER power domain
+* Use SOC specific ops for this?
+*/
huh?
Ah... this is more of a TODO. There's no previous state entered information
in the PRCM registers. So to ensure that the drivers get the right information
when they check with the PM layer about the loss of context and hence the need
to restore the registers, I need to update the logic and membank state counters
for the PER power domain manually. I was thinking of leveraging the SoC specific
power domain ops for doing this.
[...]
+
+ /* Give some time to M3 to respond. 500msec is a random value here */
random? really?
Sort of. I don't have any numbers from the h/w guys on the worst
case delay in getting an interrupt from M3 to MPU. At the same time
I want to handle the scenario where something goes wrong on the M3
side and it
Re: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Viabhav, Vaibhav Bedia vaibhav.be...@ti.com writes: AM335x supports various low power modes as documented in section 8.1.4.3 of the AM335x TRM which is available @ http://www.ti.com/litv/pdf/spruh73f DeepSleep0 mode offers the lowest power mode with limited wakeup sources without a system reboot and is mapped as the suspend state in the kernel. In this state, MPU and PER domains are turned off with the internal RAM held in retention to facilitate resume process. As part of the boot process, the assembly code is copied over to OCMCRAM using the OMAP SRAM code. AM335x has a Cortex-M3 (WKUP_M3) which assists the MPU in DeepSleep0 entry and exit. WKUP_M3 takes care of the clockdomain and powerdomain transitions based on the intended low power state. MPU needs to load the appropriate WKUP_M3 binary onto the WKUP_M3 memory space before it can leverage any of the PM features like DeepSleep. The IPC mechanism between MPU and WKUP_M3 uses a mailbox sub-module and 8 IPC registers in the Control module. MPU uses the assigned Mailbox for issuing an interrupt to WKUP_M3 which then goes and checks the IPC registers for the payload. WKUP_M3 has the ability to trigger on interrupt to MPU by executing the sev instruction. In the current implementation when the suspend process is initiated MPU interrupts the WKUP_M3 to let it know about the intent of entering DeepSleep0 and waits for an ACK. When the ACK is received MPU continues with its suspend process to suspend all the drivers and then jumps to assembly in OCMC RAM. The assembly code puts the PLLs in bypass, puts the external RAM in self-refresh mode and then finally execute the WFI instruction. Execution of the WFI instruction triggers another interrupt to the WKUP_M3 which then continues wiht the power down sequence wherein the clockdomain and powerdomain transition takes place. As part of the sleep sequence, WKUP_M3 unmasks the interrupt lines for the wakeup sources. WFI execution on WKUP_M3 causes the hardware to disable the main oscillator of the SoC. When a wakeup event occurs, WKUP_M3 starts the power-up sequence by switching on the power domains and finally enabling the clock to MPU. Since the MPU gets powered down as part of the sleep sequence in the resume path ROM code starts executing. The ROM code detects a wakeup from sleep and then jumps to the resume location in OCMC which was populated in one of the IPC registers as part of the suspend sequence. The low level code in OCMC relocks the PLLs, enables access to external RAM and then jumps to the cpu_resume code of the kernel to finish the resume process. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Cc: Tony Lingren t...@atomide.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@deeprootsystems.com Thanks for the updated series here, and my apologies for the delayed review. I've just had a quick scan of this patch, and have a few general comments, I'll probably have a few more comments after a closer look. --- v1-v2: Move assembly code addition, control module access and hookup in OMAP PM framework in separate patches. Address other comments from Kevin Hilman and Santosh Shilimkar on v1. The discussion on v1 is present @ http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129979.html Note: The mailbox change will need slight rework once the driver is finalized. arch/arm/mach-omap2/pm33xx.c | 469 ++ arch/arm/mach-omap2/pm33xx.h | 56 + 2 files changed, 525 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-omap2/pm33xx.c create mode 100644 arch/arm/mach-omap2/pm33xx.h diff --git a/arch/arm/mach-omap2/pm33xx.c b/arch/arm/mach-omap2/pm33xx.c new file mode 100644 index 000..aaa4daa --- /dev/null +++ b/arch/arm/mach-omap2/pm33xx.c @@ -0,0 +1,469 @@ +/* + * AM33XX Power Management Routines + * + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ + * Vaibhav Bedia vaibhav.be...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed as is WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/kernel.h +#include linux/init.h +#include linux/err.h +#include linux/firmware.h +#include linux/io.h +#include linux/platform_device.h +#include linux/sched.h +#include linux/suspend.h +#include linux/completion.h +#include linux/module.h +#include linux/mailbox.h +#include linux/interrupt.h + +#include
Re: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
V == Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi, V + memcpy((void *)wkup_m3_context-code, fw-data, fw-size); A size check would be good. I don't know much about the finer details about the m3 and how it is connected, but don't you need to ensure data is flushed before resetting the m3? V Will add the reg property in the next version. The wkup-m3 memory is V coming via ioremap() so AFAIK it should be ok. I realized that I do V need to replace the memcpy() with memcpy_toio() here. Ok, thanks. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Peter,
On Thu, Jan 17, 2013 at 19:57:20, Peter Korsgaard wrote:
V == Vaibhav Bedia vaibhav.be...@ti.com writes:
Hi,
V +static void am33xx_pm_firmware_cb(const struct firmware *fw, void
*context)
V +{
V + struct wkup_m3_context *wkup_m3_context = context;
V + struct platform_device *pdev = to_platform_device(wkup_m3_context-dev);
V + int ret = 0;
V +
V + /* no firmware found */
V + if (!fw) {
V + dev_err(wkup_m3_context-dev, request_firmware failed\n);
V + goto err;
V + }
V +
V + memcpy((void *)wkup_m3_context-code, fw-data, fw-size);
A size check would be good. I don't know much about the finer details
about the m3 and how it is connected, but don't you need to ensure data
is flushed before resetting the m3?
Will add the reg property in the next version. The wkup-m3 memory is coming via
ioremap() so AFAIK it should be ok. I realized that I do need to replace the
memcpy()
with memcpy_toio() here.
Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
V == Vaibhav Bedia vaibhav.be...@ti.com writes:
Hi,
V +static void am33xx_pm_firmware_cb(const struct firmware *fw, void *context)
V +{
V + struct wkup_m3_context *wkup_m3_context = context;
V + struct platform_device *pdev = to_platform_device(wkup_m3_context-dev);
V + int ret = 0;
V +
V + /* no firmware found */
V + if (!fw) {
V + dev_err(wkup_m3_context-dev, request_firmware failed\n);
V + goto err;
V + }
V +
V + memcpy((void *)wkup_m3_context-code, fw-data, fw-size);
A size check would be good. I don't know much about the finer details
about the m3 and how it is connected, but don't you need to ensure data
is flushed before resetting the m3?
V + pr_info(Copied the M3 firmware to UMEM\n);
V +
V + wkup_m3-state = M3_STATE_RESET;
V +
V + ret = omap_device_deassert_hardreset(pdev, wkup_m3);
V + if (ret) {
V + pr_err(Could not deassert the reset for WKUP_M3\n);
V + goto err;
V + } else {
V +#ifdef CONFIG_SUSPEND
V + suspend_set_ops(am33xx_pm_ops);
V + /*
V +* Physical resume address to be used by ROM code
V +*/
V + wkup_m3-ipc_data.resume_addr = (AM33XX_OCMC_END -
V + am33xx_do_wfi_sz + am33xx_resume_offset + 0x4);
V +#endif
V + return;
V + }
V +
V +err:
V + mailbox_put(wkup_m3_context-mbox, wkup_mbox_notifier);
V +}
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
