diff mbox

[RFC,v2,16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support

Message ID 1356959231-17335-17-git-send-email-vaibhav.bedia@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vaibhav Bedia Dec. 31, 2012, 1:07 p.m. UTC
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.bedia@ti.com>
Cc: Tony Lingren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@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

Comments

Peter Korsgaard Jan. 17, 2013, 2:27 p.m. UTC | #1
>>>>> "V" == Vaibhav Bedia <vaibhav.bedia@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> +}
Vaibhav Bedia Jan. 21, 2013, 10:37 a.m. UTC | #2
Hi Peter,

On Thu, Jan 17, 2013 at 19:57:20, Peter Korsgaard wrote:
> >>>>> "V" == Vaibhav Bedia <vaibhav.bedia@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
Peter Korsgaard Jan. 22, 2013, 12:50 p.m. UTC | #3
>>>>> "V" == Bedia, Vaibhav <vaibhav.bedia@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.
Kevin Hilman Feb. 12, 2013, 1:27 a.m. UTC | #4
Hi Viabhav,

Vaibhav Bedia <vaibhav.bedia@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.bedia@ti.com>
> Cc: Tony Lingren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@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 0000000..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.bedia@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 <asm/suspend.h>
> +#include <asm/proc-fns.h>
> +#include <asm/sizes.h>
> +#include <asm/fncpy.h>
> +#include <asm/system_misc.h>
> +
> +#include "pm.h"
> +#include "cm33xx.h"
> +#include "pm33xx.h"
> +#include "control.h"
> +#include "clockdomain.h"
> +#include "powerdomain.h"
> +#include "omap_hwmod.h"
> +#include "omap_device.h"
> +#include "soc.h"
> +#include "sram.h"
> +
> +void (*am33xx_do_wfi_sram)(void);

static?

> +static void __iomem *am33xx_emif_base;
> +static struct powerdomain *cefuse_pwrdm, *gfx_pwrdm, *per_pwrdm;
> +static struct clockdomain *gfx_l4ls_clkdm;
> +static struct omap_hwmod *usb_oh, *cpsw_oh, *tptc0_oh, *tptc1_oh, *tptc2_oh;
> +static struct wkup_m3_context *wkup_m3;
> +
> +static DECLARE_COMPLETION(wkup_m3_sync);
> +
> +#ifdef CONFIG_SUSPEND
> +static int am33xx_do_sram_idle(long unsigned int unused)
> +{
> +	am33xx_do_wfi_sram();
> +	return 0;
> +}
> +
> +static int am33xx_pm_suspend(void)
> +{
> +	int status, ret = 0;
> +
> +	/*
> +	 * 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.

> +	/* 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?

> +	/*
> +	 * 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?

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

> +		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?

> +			break;
> +		case 1:
> +			pr_err("Could not transition all powerdomains to target state\n");
> +			ret = -1;
> +			break;
> +		default:
> +			pr_err("Something went wrong :(\nStatus = %d\n",
> +				status);
> +			ret = -1;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int am33xx_pm_enter(suspend_state_t suspend_state)
> +{
> +	int ret = 0;
> +
> +	switch (suspend_state) {
> +	case PM_SUSPEND_STANDBY:
> +	case PM_SUSPEND_MEM:
> +		ret = am33xx_pm_suspend();
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int am33xx_pm_begin(suspend_state_t state)
> +{
> +	int ret = 0;
> +	struct mailbox_msg dummy_msg;
> +
> +	disable_hlt();
> +
> +	MAILBOX_FILL_HEADER_MSG(dummy_msg, 0xABCDABCD);
> +
> +	wkup_m3->ipc_data.sleep_mode = IPC_CMD_DS0;
> +	wkup_m3->ipc_data.param1  = DS_IPC_DEFAULT;
> +	wkup_m3->ipc_data.param2  = DS_IPC_DEFAULT;
> +
> +	am33xx_wkup_m3_ipc_cmd(&wkup_m3->ipc_data);
> +
> +	wkup_m3->state = M3_STATE_MSG_FOR_LP;
> +
> +	mailbox_enable_irq(wkup_m3->mbox, IRQ_RX);
> +
> +	ret = mailbox_msg_send(wkup_m3->mbox, &dummy_msg);
> +	if (ret) {
> +		pr_err("A8<->CM3 MSG for LP failed\n");
> +		am33xx_m3_state_machine_reset();
> +		ret = -1;
> +	}
> +
> +	/* Give some time to M3 to respond. 500msec is a random value here */

random?  really?

> +	if (!wait_for_completion_timeout(&wkup_m3_sync,
> +					msecs_to_jiffies(500))) {
> +		pr_err("A8<->CM3 sync failure\n");
> +		am33xx_m3_state_machine_reset();
> +		ret = -1;
> +	} else {
> +		pr_debug("Message sent for entering DeepSleep mode\n");
> +		mailbox_disable_irq(wkup_m3->mbox, IRQ_RX);
> +	}
> +
> +	return ret;
> +}
> +
> +static void am33xx_pm_end(void)
> +{
> +	mailbox_enable_irq(wkup_m3->mbox, IRQ_RX);
> +
> +	am33xx_m3_state_machine_reset();
> +
> +	enable_hlt();
> +
> +	return;
> +}
> +
> +static const struct platform_suspend_ops am33xx_pm_ops = {
> +	.begin		= am33xx_pm_begin,
> +	.end		= am33xx_pm_end,
> +	.enter		= am33xx_pm_enter,
> +	.valid		= suspend_valid_only_mem,
> +};
> +
> +static void am33xx_m3_state_machine_reset(void)
> +{
> +	int ret = 0;
> +	struct mailbox_msg dummy_msg;
> +
> +	MAILBOX_FILL_HEADER_MSG(dummy_msg, 0xABCDABCD);
> +
> +	wkup_m3->ipc_data.sleep_mode	= IPC_CMD_RESET;
> +	wkup_m3->ipc_data.param1	= DS_IPC_DEFAULT;
> +	wkup_m3->ipc_data.param2	= DS_IPC_DEFAULT;
> +
> +	am33xx_wkup_m3_ipc_cmd(&wkup_m3->ipc_data);
> +
> +	wkup_m3->state = M3_STATE_MSG_FOR_RESET;
> +
> +	ret = mailbox_msg_send(wkup_m3->mbox, &dummy_msg);
> +	if (!ret) {
> +		pr_debug("Message sent for resetting M3 state machine\n");
> +		/* Give some to M3 to respond. 500msec is a random value here */
> +		if (!wait_for_completion_timeout(&wkup_m3_sync,
> +						msecs_to_jiffies(500)))
> +			pr_err("A8<->CM3 sync failure\n");
> +	} else {
> +		pr_err("Could not reset M3 state machine!!!\n");
> +		wkup_m3->state = M3_STATE_UNKNOWN;
> +	}
> +}
> +#endif /* CONFIG_SUSPEND */
> +
> +/*
> + * Dummy notifier for the mailbox
> + * XXX: Get rid of this requirement once the MBX driver has been finalized

IIRC, I suggested a trivial fix to the mailbox driver that would remove
the need for this, which could be done today.

> + */
> +static int wkup_mbox_msg(struct notifier_block *self, unsigned long len,
> +		void *msg)
> +{
> +	return 0;
> +}
> +
> +static struct notifier_block wkup_mbox_notifier = {
> +	.notifier_call = wkup_mbox_msg,
> +};
> +
> +static irqreturn_t wkup_m3_txev_handler(int irq, void *unused)
> +{
> +	am33xx_txev_eoi();
> +
> +	switch (wkup_m3->state) {
> +	case M3_STATE_RESET:
> +		wkup_m3->state = M3_STATE_INITED;
> +		break;
> +	case M3_STATE_MSG_FOR_RESET:
> +		wkup_m3->state = M3_STATE_INITED;
> +		mailbox_rx_flush(wkup_m3->mbox);
> +		complete(&wkup_m3_sync);
> +		break;
> +	case M3_STATE_MSG_FOR_LP:
> +		mailbox_rx_flush(wkup_m3->mbox);
> +		complete(&wkup_m3_sync);
> +		break;
> +	case M3_STATE_UNKNOWN:
> +		pr_err("IRQ %d with WKUP_M3 in unknown state\n", irq);
> +		mailbox_rx_flush(wkup_m3->mbox);
> +		return IRQ_NONE;
> +	}
> +
> +	am33xx_txev_enable();
> +	return IRQ_HANDLED;
> +}
> +
> +static void am33xx_pm_firmware_cb(const struct firmware *fw, void *context)
> +{
> +	struct wkup_m3_context *wkup_m3_context = context;
> +	struct platform_device *pdev = to_platform_device(wkup_m3_context->dev);
> +	int ret = 0;
> +
> +	/* no firmware found */
> +	if (!fw) {
> +		dev_err(wkup_m3_context->dev, "request_firmware failed\n");
> +		goto err;
> +	}
> +
> +	memcpy((void *)wkup_m3_context->code, fw->data, fw->size);
> +	pr_info("Copied the M3 firmware to UMEM\n");
> +
> +	wkup_m3->state = M3_STATE_RESET;
> +
> +	ret = omap_device_deassert_hardreset(pdev, "wkup_m3");
> +	if (ret) {
> +		pr_err("Could not deassert the reset for WKUP_M3\n");
> +		goto err;
> +	} else {
> +#ifdef CONFIG_SUSPEND
> +		suspend_set_ops(&am33xx_pm_ops);
> +		/*
> +		 * Physical resume address to be used by ROM code
> +		 */
> +		wkup_m3->ipc_data.resume_addr = (AM33XX_OCMC_END -
> +				am33xx_do_wfi_sz + am33xx_resume_offset + 0x4);
> +#endif
> +		return;
> +	}
> +
> +err:
> +	mailbox_put(wkup_m3_context->mbox, &wkup_mbox_notifier);
> +}
> +
> +static int wkup_m3_init(void)
> +{
> +	int irq, ret = 0;
> +	struct resource *mem;
> +	struct platform_device *pdev = to_platform_device(wkup_m3->dev);
> +
> +	omap_device_enable_hwmods(to_omap_device(pdev));

Why not omap_device_enable(pdev) ?

> +
> +	/* Reserve the MBOX for sending messages to M3 */
> +	wkup_m3->mbox = mailbox_get("wkup_m3", &wkup_mbox_notifier);
> +	if (IS_ERR(wkup_m3->mbox)) {
> +		pr_err("Could not reserve mailbox for A8->M3 IPC\n");
> +		ret = -ENODEV;
> +		goto exit;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
> +		dev_err(wkup_m3->dev, "no irq resource\n");
> +		ret = -ENXIO;
> +		goto err;
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem) {
> +		dev_err(wkup_m3->dev, "no memory resource\n");
> +		ret = -ENXIO;
> +		goto err;
> +	}
> +
> +	wkup_m3->code = devm_request_and_ioremap(wkup_m3->dev, mem);
> +	if (!wkup_m3->code) {
> +		dev_err(wkup_m3->dev, "could not ioremap\n");
> +		ret = -EADDRNOTAVAIL;
> +		goto err;
> +	}
> +
> +	ret = devm_request_irq(wkup_m3->dev, irq, wkup_m3_txev_handler,
> +		  IRQF_DISABLED, "wkup_m3_txev", NULL);
> +	if (ret) {
> +		dev_err(wkup_m3->dev, "request_irq failed\n");
> +		goto err;
> +	} else {
> +		am33xx_txev_enable();
> +	}
> +
> +	pr_info("Trying to load am335x-pm-firmware.bin");
> +
> +	/* We don't want to delay boot */
> +	request_firmware_nowait(THIS_MODULE, 0, "am335x-pm-firmware.bin",
> +				wkup_m3->dev, GFP_KERNEL, wkup_m3,
> +				am33xx_pm_firmware_cb);
> +	return 0;
> +
> +err:
> +	mailbox_put(wkup_m3->mbox, &wkup_mbox_notifier);
> +exit:
> +	return ret;
> +}

Kevin
Vaibhav Bedia Feb. 13, 2013, 1:43 p.m. UTC | #5
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 doesn't respond.

[...]

> > +
> > +/*
> > + * Dummy notifier for the mailbox
> > + * XXX: Get rid of this requirement once the MBX driver has been finalized
> 
> IIRC, I suggested a trivial fix to the mailbox driver that would remove
> the need for this, which could be done today.
> 

Yes. I plan to do that once the mailbox code movement to drivers/ along with
other changes to make it generic enough are finalized. It was adding in one
more dependency and hence I kept this as a TODO in this version.

[...]

> > +static int wkup_m3_init(void)
> > +{
> > +	int irq, ret = 0;
> > +	struct resource *mem;
> > +	struct platform_device *pdev = to_platform_device(wkup_m3->dev);
> > +
> > +	omap_device_enable_hwmods(to_omap_device(pdev));
> 
> 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. 
Using omap_device_enable() triggers the following dev_warn()
from omap_device_enable().

[    2.033718] platform 44d00000.wkup_m3: omap_device_late_idle: enabled but no driver.  Idling
[    2.042676] ------------[ cut here ]------------
[    2.047572] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:2187 _idle+0x164/0x1c0()
[    2.055459] omap_hwmod: wkup_m3: idle state can only be entered from enabled state
[    2.063435] Modules linked in:
[    2.066712] [<c001afe4>] (unwind_backtrace+0x0/0xf0) from [<c004380c>] (warn_slowpath_common+0x4c/0x64)
[    2.076626] [<c004380c>] (warn_slowpath_common+0x4c/0x64) from [<c00438b8>] (warn_slowpath_fmt+0x30/0x40)
[    2.086720] [<c00438b8>] (warn_slowpath_fmt+0x30/0x40) from [<c002aad0>] (_idle+0x164/0x1c0)
[    2.095624] [<c002aad0>] (_idle+0x164/0x1c0) from [<c002af98>] (omap_hwmod_idle+0x24/0x40)
[    2.104348] [<c002af98>] (omap_hwmod_idle+0x24/0x40) from [<c002bd60>] (omap_device_idle_hwmods+0x24/0x3c)
[    2.114536] [<c002bd60>] (omap_device_idle_hwmods+0x24/0x3c) from [<c002bf4c>] (_omap_device_deactivate+0x98/0x134)
[    2.125546] [<c002bf4c>] (_omap_device_deactivate+0x98/0x134) from [<c002c87c>] (omap_device_idle+0x28/0x54)
[    2.135921] [<c002c87c>] (omap_device_idle+0x28/0x54) from [<c06ec00c>] (omap_device_late_idle+0x44/0x54)
[    2.146025] [<c06ec00c>] (omap_device_late_idle+0x44/0x54) from [<c031f850>] (bus_for_each_dev+0x50/0x7c)
[    2.156120] [<c031f850>] (bus_for_each_dev+0x50/0x7c) from [<c06ebe10>] (omap_device_late_init+0x18/0x28)
[    2.166216] [<c06ebe10>] (omap_device_late_init+0x18/0x28) from [<c00086e4>] (do_one_initcall+0x34/0x180)
[    2.176314] [<c00086e4>] (do_one_initcall+0x34/0x180) from [<c06df8f8>] (kernel_init_freeable+0xfc/0x1cc)
[    2.186408] [<c06df8f8>] (kernel_init_freeable+0xfc/0x1cc) from [<c04d4b20>] (kernel_init+0x8/0xe4)
[    2.195962] [<c04d4b20>] (kernel_init+0x8/0xe4) from [<c0013410>] (ret_from_fork+0x14/0x24)
[    2.204905] ---[ end trace 8f61b319779f6e57 ]---

Regards,
Vaibhav
Kevin Hilman Feb. 18, 2013, 4:11 p.m. UTC | #6
"Bedia, Vaibhav" <vaibhav.bedia@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 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.
>
> [...]

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.

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

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".  

>
>> > +
>> > +/*
>> > + * Dummy notifier for the mailbox
>> > + * XXX: Get rid of this requirement once the MBX driver has been finalized
>> 
>> IIRC, I suggested a trivial fix to the mailbox driver that would remove
>> the need for this, which could be done today.
>> 
>
> Yes. I plan to do that once the mailbox code movement to drivers/ along with
> other changes to make it generic enough are finalized. It was adding in one
> more dependency and hence I kept this as a TODO in this version.

OK, fair enough.

>> > +static int wkup_m3_init(void)
>> > +{
>> > +	int irq, ret = 0;
>> > +	struct resource *mem;
>> > +	struct platform_device *pdev = to_platform_device(wkup_m3->dev);
>> > +
>> > +	omap_device_enable_hwmods(to_omap_device(pdev));
>> 
>> 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.

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.

> [    2.033718] platform 44d00000.wkup_m3: omap_device_late_idle: enabled but no driver.  Idling
> [    2.042676] ------------[ cut here ]------------
> [    2.047572] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:2187 _idle+0x164/0x1c0()
> [    2.055459] omap_hwmod: wkup_m3: idle state can only be entered from enabled state
> [    2.063435] Modules linked in:
> [    2.066712] [<c001afe4>] (unwind_backtrace+0x0/0xf0) from [<c004380c>] (warn_slowpath_common+0x4c/0x64)
> [ 2.076626] [<c004380c>] (warn_slowpath_common+0x4c/0x64) from
> [<c00438b8>] (warn_slowpath_fmt+0x30/0x40)
> [    2.086720] [<c00438b8>] (warn_slowpath_fmt+0x30/0x40) from [<c002aad0>] (_idle+0x164/0x1c0)
> [    2.095624] [<c002aad0>] (_idle+0x164/0x1c0) from [<c002af98>] (omap_hwmod_idle+0x24/0x40)
> [ 2.104348] [<c002af98>] (omap_hwmod_idle+0x24/0x40) from [<c002bd60>]
> (omap_device_idle_hwmods+0x24/0x3c)
> [ 2.114536] [<c002bd60>] (omap_device_idle_hwmods+0x24/0x3c) from
> [<c002bf4c>] (_omap_device_deactivate+0x98/0x134)
> [ 2.125546] [<c002bf4c>] (_omap_device_deactivate+0x98/0x134) from
> [<c002c87c>] (omap_device_idle+0x28/0x54)
> [ 2.135921] [<c002c87c>] (omap_device_idle+0x28/0x54) from
> [<c06ec00c>] (omap_device_late_idle+0x44/0x54)
> [ 2.146025] [<c06ec00c>] (omap_device_late_idle+0x44/0x54) from
> [<c031f850>] (bus_for_each_dev+0x50/0x7c)
> [ 2.156120] [<c031f850>] (bus_for_each_dev+0x50/0x7c) from
> [<c06ebe10>] (omap_device_late_init+0x18/0x28)
> [ 2.166216] [<c06ebe10>] (omap_device_late_init+0x18/0x28) from
> [<c00086e4>] (do_one_initcall+0x34/0x180)
> [ 2.176314] [<c00086e4>] (do_one_initcall+0x34/0x180) from
> [<c06df8f8>] (kernel_init_freeable+0xfc/0x1cc)
> [    2.186408] [<c06df8f8>] (kernel_init_freeable+0xfc/0x1cc) from [<c04d4b20>] (kernel_init+0x8/0xe4)
> [    2.195962] [<c04d4b20>] (kernel_init+0x8/0xe4) from [<c0013410>] (ret_from_fork+0x14/0x24)
> [    2.204905] ---[ end trace 8f61b319779f6e57 ]---

Kevin
Vaibhav Bedia Feb. 20, 2013, 9:21 a.m. UTC | #7
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
Kevin Hilman Feb. 20, 2013, 2:30 p.m. UTC | #8
"Bedia, Vaibhav" <vaibhav.bedia@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
Daniel Mack April 3, 2013, 11:52 a.m. UTC | #9
Hi Vaibhav,

On Mon, Dec 31, 2012 at 2:07 PM, Vaibhav Bedia <vaibhav.bedia@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.bedia@ti.com>
> Cc: Tony Lingren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@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 0000000..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.bedia@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 <asm/suspend.h>
> +#include <asm/proc-fns.h>
> +#include <asm/sizes.h>
> +#include <asm/fncpy.h>
> +#include <asm/system_misc.h>
> +
> +#include "pm.h"
> +#include "cm33xx.h"
> +#include "pm33xx.h"
> +#include "control.h"
> +#include "clockdomain.h"
> +#include "powerdomain.h"
> +#include "omap_hwmod.h"
> +#include "omap_device.h"
> +#include "soc.h"
> +#include "sram.h"
> +
> +void (*am33xx_do_wfi_sram)(void);
> +
> +static void __iomem *am33xx_emif_base;
> +static struct powerdomain *cefuse_pwrdm, *gfx_pwrdm, *per_pwrdm;
> +static struct clockdomain *gfx_l4ls_clkdm;
> +static struct omap_hwmod *usb_oh, *cpsw_oh, *tptc0_oh, *tptc1_oh, *tptc2_oh;
> +static struct wkup_m3_context *wkup_m3;
> +
> +static DECLARE_COMPLETION(wkup_m3_sync);
> +
> +#ifdef CONFIG_SUSPEND
> +static int am33xx_do_sram_idle(long unsigned int unused)
> +{
> +       am33xx_do_wfi_sram();
> +       return 0;
> +}
> +
> +static int am33xx_pm_suspend(void)
> +{
> +       int status, ret = 0;
> +
> +       /*
> +        * 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);
> +
> +       /* 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");
> +
> +       /*
> +        * 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.
> +        */
> +       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);
> +               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?
> +                        */
> +                       break;
> +               case 1:
> +                       pr_err("Could not transition all powerdomains to target state\n");
> +                       ret = -1;
> +                       break;
> +               default:
> +                       pr_err("Something went wrong :(\nStatus = %d\n",
> +                               status);
> +                       ret = -1;
> +               }
> +       }
> +
> +       return ret;
> +}
> +
> +static int am33xx_pm_enter(suspend_state_t suspend_state)
> +{
> +       int ret = 0;
> +
> +       switch (suspend_state) {
> +       case PM_SUSPEND_STANDBY:
> +       case PM_SUSPEND_MEM:
> +               ret = am33xx_pm_suspend();
> +               break;
> +       default:
> +               ret = -EINVAL;
> +       }
> +
> +       return ret;
> +}
> +
> +static int am33xx_pm_begin(suspend_state_t state)
> +{
> +       int ret = 0;
> +       struct mailbox_msg dummy_msg;
> +
> +       disable_hlt();
> +
> +       MAILBOX_FILL_HEADER_MSG(dummy_msg, 0xABCDABCD);
> +
> +       wkup_m3->ipc_data.sleep_mode = IPC_CMD_DS0;
> +       wkup_m3->ipc_data.param1  = DS_IPC_DEFAULT;
> +       wkup_m3->ipc_data.param2  = DS_IPC_DEFAULT;
> +
> +       am33xx_wkup_m3_ipc_cmd(&wkup_m3->ipc_data);
> +
> +       wkup_m3->state = M3_STATE_MSG_FOR_LP;
> +
> +       mailbox_enable_irq(wkup_m3->mbox, IRQ_RX);
> +
> +       ret = mailbox_msg_send(wkup_m3->mbox, &dummy_msg);
> +       if (ret) {
> +               pr_err("A8<->CM3 MSG for LP failed\n");
> +               am33xx_m3_state_machine_reset();
> +               ret = -1;
> +       }
> +
> +       /* Give some time to M3 to respond. 500msec is a random value here */
> +       if (!wait_for_completion_timeout(&wkup_m3_sync,
> +                                       msecs_to_jiffies(500))) {
> +               pr_err("A8<->CM3 sync failure\n");
> +               am33xx_m3_state_machine_reset();
> +               ret = -1;
> +       } else {
> +               pr_debug("Message sent for entering DeepSleep mode\n");
> +               mailbox_disable_irq(wkup_m3->mbox, IRQ_RX);
> +       }
> +
> +       return ret;
> +}
> +
> +static void am33xx_pm_end(void)
> +{
> +       mailbox_enable_irq(wkup_m3->mbox, IRQ_RX);
> +
> +       am33xx_m3_state_machine_reset();
> +
> +       enable_hlt();
> +
> +       return;
> +}
> +
> +static const struct platform_suspend_ops am33xx_pm_ops = {
> +       .begin          = am33xx_pm_begin,
> +       .end            = am33xx_pm_end,
> +       .enter          = am33xx_pm_enter,
> +       .valid          = suspend_valid_only_mem,
> +};
> +
> +static void am33xx_m3_state_machine_reset(void)
> +{
> +       int ret = 0;
> +       struct mailbox_msg dummy_msg;
> +
> +       MAILBOX_FILL_HEADER_MSG(dummy_msg, 0xABCDABCD);
> +
> +       wkup_m3->ipc_data.sleep_mode    = IPC_CMD_RESET;
> +       wkup_m3->ipc_data.param1        = DS_IPC_DEFAULT;
> +       wkup_m3->ipc_data.param2        = DS_IPC_DEFAULT;
> +
> +       am33xx_wkup_m3_ipc_cmd(&wkup_m3->ipc_data);
> +
> +       wkup_m3->state = M3_STATE_MSG_FOR_RESET;
> +
> +       ret = mailbox_msg_send(wkup_m3->mbox, &dummy_msg);
> +       if (!ret) {
> +               pr_debug("Message sent for resetting M3 state machine\n");
> +               /* Give some to M3 to respond. 500msec is a random value here */
> +               if (!wait_for_completion_timeout(&wkup_m3_sync,
> +                                               msecs_to_jiffies(500)))
> +                       pr_err("A8<->CM3 sync failure\n");
> +       } else {
> +               pr_err("Could not reset M3 state machine!!!\n");
> +               wkup_m3->state = M3_STATE_UNKNOWN;
> +       }
> +}
> +#endif /* CONFIG_SUSPEND */
> +
> +/*
> + * Dummy notifier for the mailbox
> + * XXX: Get rid of this requirement once the MBX driver has been finalized
> + */
> +static int wkup_mbox_msg(struct notifier_block *self, unsigned long len,
> +               void *msg)
> +{
> +       return 0;
> +}
> +
> +static struct notifier_block wkup_mbox_notifier = {
> +       .notifier_call = wkup_mbox_msg,
> +};
> +
> +static irqreturn_t wkup_m3_txev_handler(int irq, void *unused)
> +{
> +       am33xx_txev_eoi();
> +
> +       switch (wkup_m3->state) {
> +       case M3_STATE_RESET:
> +               wkup_m3->state = M3_STATE_INITED;
> +               break;
> +       case M3_STATE_MSG_FOR_RESET:
> +               wkup_m3->state = M3_STATE_INITED;
> +               mailbox_rx_flush(wkup_m3->mbox);
> +               complete(&wkup_m3_sync);
> +               break;
> +       case M3_STATE_MSG_FOR_LP:
> +               mailbox_rx_flush(wkup_m3->mbox);
> +               complete(&wkup_m3_sync);
> +               break;
> +       case M3_STATE_UNKNOWN:
> +               pr_err("IRQ %d with WKUP_M3 in unknown state\n", irq);
> +               mailbox_rx_flush(wkup_m3->mbox);
> +               return IRQ_NONE;
> +       }
> +
> +       am33xx_txev_enable();
> +       return IRQ_HANDLED;
> +}
> +
> +static void am33xx_pm_firmware_cb(const struct firmware *fw, void *context)
> +{
> +       struct wkup_m3_context *wkup_m3_context = context;
> +       struct platform_device *pdev = to_platform_device(wkup_m3_context->dev);
> +       int ret = 0;
> +
> +       /* no firmware found */
> +       if (!fw) {
> +               dev_err(wkup_m3_context->dev, "request_firmware failed\n");
> +               goto err;
> +       }
> +
> +       memcpy((void *)wkup_m3_context->code, fw->data, fw->size);
> +       pr_info("Copied the M3 firmware to UMEM\n");
> +
> +       wkup_m3->state = M3_STATE_RESET;
> +
> +       ret = omap_device_deassert_hardreset(pdev, "wkup_m3");
> +       if (ret) {
> +               pr_err("Could not deassert the reset for WKUP_M3\n");
> +               goto err;
> +       } else {
> +#ifdef CONFIG_SUSPEND
> +               suspend_set_ops(&am33xx_pm_ops);
> +               /*
> +                * Physical resume address to be used by ROM code
> +                */
> +               wkup_m3->ipc_data.resume_addr = (AM33XX_OCMC_END -
> +                               am33xx_do_wfi_sz + am33xx_resume_offset + 0x4);
> +#endif
> +               return;
> +       }
> +
> +err:
> +       mailbox_put(wkup_m3_context->mbox, &wkup_mbox_notifier);
> +}
> +
> +static int wkup_m3_init(void)
> +{
> +       int irq, ret = 0;
> +       struct resource *mem;
> +       struct platform_device *pdev = to_platform_device(wkup_m3->dev);
> +
> +       omap_device_enable_hwmods(to_omap_device(pdev));
> +
> +       /* Reserve the MBOX for sending messages to M3 */
> +       wkup_m3->mbox = mailbox_get("wkup_m3", &wkup_mbox_notifier);
> +       if (IS_ERR(wkup_m3->mbox)) {
> +               pr_err("Could not reserve mailbox for A8->M3 IPC\n");
> +               ret = -ENODEV;
> +               goto exit;
> +       }
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (!irq) {
> +               dev_err(wkup_m3->dev, "no irq resource\n");
> +               ret = -ENXIO;
> +               goto err;
> +       }
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!mem) {
> +               dev_err(wkup_m3->dev, "no memory resource\n");
> +               ret = -ENXIO;
> +               goto err;
> +       }
> +
> +       wkup_m3->code = devm_request_and_ioremap(wkup_m3->dev, mem);
> +       if (!wkup_m3->code) {
> +               dev_err(wkup_m3->dev, "could not ioremap\n");
> +               ret = -EADDRNOTAVAIL;
> +               goto err;
> +       }
> +
> +       ret = devm_request_irq(wkup_m3->dev, irq, wkup_m3_txev_handler,
> +                 IRQF_DISABLED, "wkup_m3_txev", NULL);
> +       if (ret) {
> +               dev_err(wkup_m3->dev, "request_irq failed\n");
> +               goto err;
> +       } else {
> +               am33xx_txev_enable();
> +       }
> +
> +       pr_info("Trying to load am335x-pm-firmware.bin");
> +
> +       /* We don't want to delay boot */
> +       request_firmware_nowait(THIS_MODULE, 0, "am335x-pm-firmware.bin",
> +                               wkup_m3->dev, GFP_KERNEL, wkup_m3,
> +                               am33xx_pm_firmware_cb);
> +       return 0;
> +
> +err:
> +       mailbox_put(wkup_m3->mbox, &wkup_mbox_notifier);
> +exit:
> +       return ret;
> +}
> +
> +/*
> + * Push the minimal suspend-resume code to SRAM
> + */
> +void am33xx_push_sram_idle(void)
> +{
> +       am33xx_do_wfi_sram = (void *)omap_sram_push
> +                                       (am33xx_do_wfi, am33xx_do_wfi_sz);
> +}
> +
> +static int __init am33xx_map_emif(void)
> +{
> +       am33xx_emif_base = ioremap(AM33XX_EMIF_BASE, SZ_32K);
> +
> +       if (!am33xx_emif_base)
> +               return -ENOMEM;
> +
> +       return 0;
> +}
> +
> +void __iomem *am33xx_get_emif_base(void)
> +{
> +       return am33xx_emif_base;
> +}
> +
> +int __init am33xx_pm_init(void)
> +{
> +       int ret;
> +
> +       if (!soc_is_am33xx())
> +               return -ENODEV;
> +
> +       pr_info("Power Management for AM33XX family\n");
> +
> +       /*
> +        * 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
> +        */
> +       usb_oh          = omap_hwmod_lookup("usb_otg_hs");
> +       tptc0_oh        = omap_hwmod_lookup("tptc0");
> +       tptc1_oh        = omap_hwmod_lookup("tptc1");
> +       tptc2_oh        = omap_hwmod_lookup("tptc2");
> +       cpsw_oh         = omap_hwmod_lookup("cpgmac0");
> +
> +       gfx_pwrdm = pwrdm_lookup("gfx_pwrdm");
> +       per_pwrdm = pwrdm_lookup("per_pwrdm");
> +
> +       gfx_l4ls_clkdm = clkdm_lookup("gfx_l4ls_gfx_clkdm");
> +
> +       if ((!usb_oh) || (!tptc0_oh) || (!tptc1_oh) || (!tptc2_oh) ||
> +               (!cpsw_oh) || (!gfx_pwrdm) || (!per_pwrdm) ||
> +               (!gfx_l4ls_clkdm)) {
> +               ret = -ENODEV;
> +               goto err;
> +       }
> +
> +       wkup_m3 = kzalloc(sizeof(struct wkup_m3_context), GFP_KERNEL);
> +       if (!wkup_m3) {
> +               pr_err("Memory allocation failed\n");
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +
> +       ret = am33xx_map_emif();
> +       if (ret) {
> +               pr_err("Could not ioremap EMIF\n");
> +               goto err;
> +       }
> +
> +       (void) clkdm_for_each(omap_pm_clkdms_setup, NULL);
> +
> +       /* CEFUSE domain can be turned off post bootup */
> +       cefuse_pwrdm = pwrdm_lookup("cefuse_pwrdm");
> +       if (cefuse_pwrdm)
> +               pwrdm_set_next_fpwrst(cefuse_pwrdm, PWRDM_FUNC_PWRST_OFF);
> +       else
> +               pr_err("Failed to get cefuse_pwrdm\n");
> +
> +       wkup_m3->dev = omap_device_get_by_hwmod_name("wkup_m3");
> +
> +       ret = wkup_m3_init();
> +       if (ret)
> +               pr_err("Could not initialise firmware loading\n");
> +
> +err:
> +       return ret;
> +}
> diff --git a/arch/arm/mach-omap2/pm33xx.h b/arch/arm/mach-omap2/pm33xx.h
> new file mode 100644
> index 0000000..13a2c85
> --- /dev/null
> +++ b/arch/arm/mach-omap2/pm33xx.h
> @@ -0,0 +1,56 @@
> +/*
> + * AM33XX Power Management Routines
> + *
> + * Copyright (C) 2012 Texas Instruments Inc.
> + * Vaibhav Bedia <vaibhav.bedia@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.
> + */
> +#ifndef __ARCH_ARM_MACH_OMAP2_PM33XX_H
> +#define __ARCH_ARM_MACH_OMAP2_PM33XX_H
> +
> +#include "control.h"
> +
> +#ifndef __ASSEMBLER__
> +struct wkup_m3_context {
> +       struct am33xx_ipc_data  ipc_data;
> +       struct device           *dev;
> +       struct firmware         *firmware;
> +       struct mailbox          *mbox;
> +       void __iomem            *code;
> +       u8                      state;
> +};
> +
> +#ifdef CONFIG_SUSPEND
> +static void am33xx_m3_state_machine_reset(void);
> +#else
> +static inline void am33xx_m3_state_machine_reset(void) {}
> +#endif /* CONFIG_SUSPEND */
> +
> +extern void __iomem *am33xx_get_emif_base(void);
> +#endif
> +
> +#define        IPC_CMD_DS0                     0x3
> +#define IPC_CMD_RESET                   0xe
> +#define DS_IPC_DEFAULT                 0xffffffff
> +
> +#define IPC_RESP_SHIFT                 16
> +#define IPC_RESP_MASK                  (0xffff << 16)
> +
> +#define M3_STATE_UNKNOWN               0
> +#define M3_STATE_RESET                 1
> +#define M3_STATE_INITED                        2
> +#define M3_STATE_MSG_FOR_LP            3
> +#define M3_STATE_MSG_FOR_RESET         4
> +
> +#define AM33XX_OCMC_END                        0x40310000
> +#define AM33XX_EMIF_BASE               0x4C000000
> +
> +#endif
> --
> 1.7.0.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Vaibhav Bedia April 4, 2013, 8:47 a.m. UTC | #10
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.bedia@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
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/pm33xx.c b/arch/arm/mach-omap2/pm33xx.c
new file mode 100644
index 0000000..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.bedia@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 <asm/suspend.h>
+#include <asm/proc-fns.h>
+#include <asm/sizes.h>
+#include <asm/fncpy.h>
+#include <asm/system_misc.h>
+
+#include "pm.h"
+#include "cm33xx.h"
+#include "pm33xx.h"
+#include "control.h"
+#include "clockdomain.h"
+#include "powerdomain.h"
+#include "omap_hwmod.h"
+#include "omap_device.h"
+#include "soc.h"
+#include "sram.h"
+
+void (*am33xx_do_wfi_sram)(void);
+
+static void __iomem *am33xx_emif_base;
+static struct powerdomain *cefuse_pwrdm, *gfx_pwrdm, *per_pwrdm;
+static struct clockdomain *gfx_l4ls_clkdm;
+static struct omap_hwmod *usb_oh, *cpsw_oh, *tptc0_oh, *tptc1_oh, *tptc2_oh;
+static struct wkup_m3_context *wkup_m3;
+
+static DECLARE_COMPLETION(wkup_m3_sync);
+
+#ifdef CONFIG_SUSPEND
+static int am33xx_do_sram_idle(long unsigned int unused)
+{
+	am33xx_do_wfi_sram();
+	return 0;
+}
+
+static int am33xx_pm_suspend(void)
+{
+	int status, ret = 0;
+
+	/*
+	 * 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);
+
+	/* 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");
+
+	/*
+	 * 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.
+	 */
+	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);
+		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?
+			 */
+			break;
+		case 1:
+			pr_err("Could not transition all powerdomains to target state\n");
+			ret = -1;
+			break;
+		default:
+			pr_err("Something went wrong :(\nStatus = %d\n",
+				status);
+			ret = -1;
+		}
+	}
+
+	return ret;
+}
+
+static int am33xx_pm_enter(suspend_state_t suspend_state)
+{
+	int ret = 0;
+
+	switch (suspend_state) {
+	case PM_SUSPEND_STANDBY:
+	case PM_SUSPEND_MEM:
+		ret = am33xx_pm_suspend();
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int am33xx_pm_begin(suspend_state_t state)
+{
+	int ret = 0;
+	struct mailbox_msg dummy_msg;
+
+	disable_hlt();
+
+	MAILBOX_FILL_HEADER_MSG(dummy_msg, 0xABCDABCD);
+
+	wkup_m3->ipc_data.sleep_mode = IPC_CMD_DS0;
+	wkup_m3->ipc_data.param1  = DS_IPC_DEFAULT;
+	wkup_m3->ipc_data.param2  = DS_IPC_DEFAULT;
+
+	am33xx_wkup_m3_ipc_cmd(&wkup_m3->ipc_data);
+
+	wkup_m3->state = M3_STATE_MSG_FOR_LP;
+
+	mailbox_enable_irq(wkup_m3->mbox, IRQ_RX);
+
+	ret = mailbox_msg_send(wkup_m3->mbox, &dummy_msg);
+	if (ret) {
+		pr_err("A8<->CM3 MSG for LP failed\n");
+		am33xx_m3_state_machine_reset();
+		ret = -1;
+	}
+
+	/* Give some time to M3 to respond. 500msec is a random value here */
+	if (!wait_for_completion_timeout(&wkup_m3_sync,
+					msecs_to_jiffies(500))) {
+		pr_err("A8<->CM3 sync failure\n");
+		am33xx_m3_state_machine_reset();
+		ret = -1;
+	} else {
+		pr_debug("Message sent for entering DeepSleep mode\n");
+		mailbox_disable_irq(wkup_m3->mbox, IRQ_RX);
+	}
+
+	return ret;
+}
+
+static void am33xx_pm_end(void)
+{
+	mailbox_enable_irq(wkup_m3->mbox, IRQ_RX);
+
+	am33xx_m3_state_machine_reset();
+
+	enable_hlt();
+
+	return;
+}
+
+static const struct platform_suspend_ops am33xx_pm_ops = {
+	.begin		= am33xx_pm_begin,
+	.end		= am33xx_pm_end,
+	.enter		= am33xx_pm_enter,
+	.valid		= suspend_valid_only_mem,
+};
+
+static void am33xx_m3_state_machine_reset(void)
+{
+	int ret = 0;
+	struct mailbox_msg dummy_msg;
+
+	MAILBOX_FILL_HEADER_MSG(dummy_msg, 0xABCDABCD);
+
+	wkup_m3->ipc_data.sleep_mode	= IPC_CMD_RESET;
+	wkup_m3->ipc_data.param1	= DS_IPC_DEFAULT;
+	wkup_m3->ipc_data.param2	= DS_IPC_DEFAULT;
+
+	am33xx_wkup_m3_ipc_cmd(&wkup_m3->ipc_data);
+
+	wkup_m3->state = M3_STATE_MSG_FOR_RESET;
+
+	ret = mailbox_msg_send(wkup_m3->mbox, &dummy_msg);
+	if (!ret) {
+		pr_debug("Message sent for resetting M3 state machine\n");
+		/* Give some to M3 to respond. 500msec is a random value here */
+		if (!wait_for_completion_timeout(&wkup_m3_sync,
+						msecs_to_jiffies(500)))
+			pr_err("A8<->CM3 sync failure\n");
+	} else {
+		pr_err("Could not reset M3 state machine!!!\n");
+		wkup_m3->state = M3_STATE_UNKNOWN;
+	}
+}
+#endif /* CONFIG_SUSPEND */
+
+/*
+ * Dummy notifier for the mailbox
+ * XXX: Get rid of this requirement once the MBX driver has been finalized
+ */
+static int wkup_mbox_msg(struct notifier_block *self, unsigned long len,
+		void *msg)
+{
+	return 0;
+}
+
+static struct notifier_block wkup_mbox_notifier = {
+	.notifier_call = wkup_mbox_msg,
+};
+
+static irqreturn_t wkup_m3_txev_handler(int irq, void *unused)
+{
+	am33xx_txev_eoi();
+
+	switch (wkup_m3->state) {
+	case M3_STATE_RESET:
+		wkup_m3->state = M3_STATE_INITED;
+		break;
+	case M3_STATE_MSG_FOR_RESET:
+		wkup_m3->state = M3_STATE_INITED;
+		mailbox_rx_flush(wkup_m3->mbox);
+		complete(&wkup_m3_sync);
+		break;
+	case M3_STATE_MSG_FOR_LP:
+		mailbox_rx_flush(wkup_m3->mbox);
+		complete(&wkup_m3_sync);
+		break;
+	case M3_STATE_UNKNOWN:
+		pr_err("IRQ %d with WKUP_M3 in unknown state\n", irq);
+		mailbox_rx_flush(wkup_m3->mbox);
+		return IRQ_NONE;
+	}
+
+	am33xx_txev_enable();
+	return IRQ_HANDLED;
+}
+
+static void am33xx_pm_firmware_cb(const struct firmware *fw, void *context)
+{
+	struct wkup_m3_context *wkup_m3_context = context;
+	struct platform_device *pdev = to_platform_device(wkup_m3_context->dev);
+	int ret = 0;
+
+	/* no firmware found */
+	if (!fw) {
+		dev_err(wkup_m3_context->dev, "request_firmware failed\n");
+		goto err;
+	}
+
+	memcpy((void *)wkup_m3_context->code, fw->data, fw->size);
+	pr_info("Copied the M3 firmware to UMEM\n");
+
+	wkup_m3->state = M3_STATE_RESET;
+
+	ret = omap_device_deassert_hardreset(pdev, "wkup_m3");
+	if (ret) {
+		pr_err("Could not deassert the reset for WKUP_M3\n");
+		goto err;
+	} else {
+#ifdef CONFIG_SUSPEND
+		suspend_set_ops(&am33xx_pm_ops);
+		/*
+		 * Physical resume address to be used by ROM code
+		 */
+		wkup_m3->ipc_data.resume_addr = (AM33XX_OCMC_END -
+				am33xx_do_wfi_sz + am33xx_resume_offset + 0x4);
+#endif
+		return;
+	}
+
+err:
+	mailbox_put(wkup_m3_context->mbox, &wkup_mbox_notifier);
+}
+
+static int wkup_m3_init(void)
+{
+	int irq, ret = 0;
+	struct resource *mem;
+	struct platform_device *pdev = to_platform_device(wkup_m3->dev);
+
+	omap_device_enable_hwmods(to_omap_device(pdev));
+
+	/* Reserve the MBOX for sending messages to M3 */
+	wkup_m3->mbox = mailbox_get("wkup_m3", &wkup_mbox_notifier);
+	if (IS_ERR(wkup_m3->mbox)) {
+		pr_err("Could not reserve mailbox for A8->M3 IPC\n");
+		ret = -ENODEV;
+		goto exit;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (!irq) {
+		dev_err(wkup_m3->dev, "no irq resource\n");
+		ret = -ENXIO;
+		goto err;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		dev_err(wkup_m3->dev, "no memory resource\n");
+		ret = -ENXIO;
+		goto err;
+	}
+
+	wkup_m3->code = devm_request_and_ioremap(wkup_m3->dev, mem);
+	if (!wkup_m3->code) {
+		dev_err(wkup_m3->dev, "could not ioremap\n");
+		ret = -EADDRNOTAVAIL;
+		goto err;
+	}
+
+	ret = devm_request_irq(wkup_m3->dev, irq, wkup_m3_txev_handler,
+		  IRQF_DISABLED, "wkup_m3_txev", NULL);
+	if (ret) {
+		dev_err(wkup_m3->dev, "request_irq failed\n");
+		goto err;
+	} else {
+		am33xx_txev_enable();
+	}
+
+	pr_info("Trying to load am335x-pm-firmware.bin");
+
+	/* We don't want to delay boot */
+	request_firmware_nowait(THIS_MODULE, 0, "am335x-pm-firmware.bin",
+				wkup_m3->dev, GFP_KERNEL, wkup_m3,
+				am33xx_pm_firmware_cb);
+	return 0;
+
+err:
+	mailbox_put(wkup_m3->mbox, &wkup_mbox_notifier);
+exit:
+	return ret;
+}
+
+/*
+ * Push the minimal suspend-resume code to SRAM
+ */
+void am33xx_push_sram_idle(void)
+{
+	am33xx_do_wfi_sram = (void *)omap_sram_push
+					(am33xx_do_wfi, am33xx_do_wfi_sz);
+}
+
+static int __init am33xx_map_emif(void)
+{
+	am33xx_emif_base = ioremap(AM33XX_EMIF_BASE, SZ_32K);
+
+	if (!am33xx_emif_base)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void __iomem *am33xx_get_emif_base(void)
+{
+	return am33xx_emif_base;
+}
+
+int __init am33xx_pm_init(void)
+{
+	int ret;
+
+	if (!soc_is_am33xx())
+		return -ENODEV;
+
+	pr_info("Power Management for AM33XX family\n");
+
+	/*
+	 * 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
+	 */
+	usb_oh		= omap_hwmod_lookup("usb_otg_hs");
+	tptc0_oh	= omap_hwmod_lookup("tptc0");
+	tptc1_oh	= omap_hwmod_lookup("tptc1");
+	tptc2_oh	= omap_hwmod_lookup("tptc2");
+	cpsw_oh		= omap_hwmod_lookup("cpgmac0");
+
+	gfx_pwrdm = pwrdm_lookup("gfx_pwrdm");
+	per_pwrdm = pwrdm_lookup("per_pwrdm");
+
+	gfx_l4ls_clkdm = clkdm_lookup("gfx_l4ls_gfx_clkdm");
+
+	if ((!usb_oh) || (!tptc0_oh) || (!tptc1_oh) || (!tptc2_oh) ||
+		(!cpsw_oh) || (!gfx_pwrdm) || (!per_pwrdm) ||
+		(!gfx_l4ls_clkdm)) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	wkup_m3 = kzalloc(sizeof(struct wkup_m3_context), GFP_KERNEL);
+	if (!wkup_m3) {
+		pr_err("Memory allocation failed\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ret = am33xx_map_emif();
+	if (ret) {
+		pr_err("Could not ioremap EMIF\n");
+		goto err;
+	}
+
+	(void) clkdm_for_each(omap_pm_clkdms_setup, NULL);
+
+	/* CEFUSE domain can be turned off post bootup */
+	cefuse_pwrdm = pwrdm_lookup("cefuse_pwrdm");
+	if (cefuse_pwrdm)
+		pwrdm_set_next_fpwrst(cefuse_pwrdm, PWRDM_FUNC_PWRST_OFF);
+	else
+		pr_err("Failed to get cefuse_pwrdm\n");
+
+	wkup_m3->dev = omap_device_get_by_hwmod_name("wkup_m3");
+
+	ret = wkup_m3_init();
+	if (ret)
+		pr_err("Could not initialise firmware loading\n");
+
+err:
+	return ret;
+}
diff --git a/arch/arm/mach-omap2/pm33xx.h b/arch/arm/mach-omap2/pm33xx.h
new file mode 100644
index 0000000..13a2c85
--- /dev/null
+++ b/arch/arm/mach-omap2/pm33xx.h
@@ -0,0 +1,56 @@ 
+/*
+ * AM33XX Power Management Routines
+ *
+ * Copyright (C) 2012 Texas Instruments Inc.
+ * Vaibhav Bedia <vaibhav.bedia@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.
+ */
+#ifndef __ARCH_ARM_MACH_OMAP2_PM33XX_H
+#define __ARCH_ARM_MACH_OMAP2_PM33XX_H
+
+#include "control.h"
+
+#ifndef __ASSEMBLER__
+struct wkup_m3_context {
+	struct am33xx_ipc_data	ipc_data;
+	struct device		*dev;
+	struct firmware		*firmware;
+	struct mailbox		*mbox;
+	void __iomem		*code;
+	u8			state;
+};
+
+#ifdef CONFIG_SUSPEND
+static void am33xx_m3_state_machine_reset(void);
+#else
+static inline void am33xx_m3_state_machine_reset(void) {}
+#endif /* CONFIG_SUSPEND */
+
+extern void __iomem *am33xx_get_emif_base(void);
+#endif
+
+#define	IPC_CMD_DS0			0x3
+#define IPC_CMD_RESET                   0xe
+#define DS_IPC_DEFAULT			0xffffffff
+
+#define IPC_RESP_SHIFT			16
+#define IPC_RESP_MASK			(0xffff << 16)
+
+#define M3_STATE_UNKNOWN		0
+#define M3_STATE_RESET			1
+#define M3_STATE_INITED			2
+#define M3_STATE_MSG_FOR_LP		3
+#define M3_STATE_MSG_FOR_RESET		4
+
+#define AM33XX_OCMC_END			0x40310000
+#define AM33XX_EMIF_BASE		0x4C000000
+
+#endif