| Message ID | 20190322171619.4180-4-j-keerthy@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AM437x: Add rtc-only + DDR mode support | expand |
Hi, * Keerthy <j-keerthy@ti.com> [190322 17:16]: > +static int am43xx_check_off_mode_enable(void) > +{ > + /* > + * Check for am437x-sk-evm which due to HW design cannot support > + * this mode reliably. > + */ > + if (of_machine_is_compatible("ti,am437x-sk-evm") && enable_off_mode) { > + pr_warn("WARNING: This platform does not support off-mode, entering DeepSleep suspend.\n"); > + return 0; > + } > + > + return enable_off_mode; > +} Considering off-mode suspend depends on how the board is wired for various things such as memory, PMIC and the related signal lines, I agree using the machine compatible is the best check we can do here. But since the device can hang during suspend unless things are configured right for the board, I suggest you rather list allowed boards here that are known to work with off-mode. Otherwise many out-of-tree boards might hang during suspend mysteriously. Regards, Tony
On Mon, 1 Apr 2019 10:52:45 -0700 Tony Lindgren <tony@atomide.com> wrote: > Hi, > > * Keerthy <j-keerthy@ti.com> [190322 17:16]: > > +static int am43xx_check_off_mode_enable(void) > > +{ > > + /* > > + * Check for am437x-sk-evm which due to HW design cannot support > > + * this mode reliably. > > + */ > > + if (of_machine_is_compatible("ti,am437x-sk-evm") && enable_off_mode) { > > + pr_warn("WARNING: This platform does not support off-mode, entering DeepSleep suspend.\n"); > > + return 0; > > + } > > + > > + return enable_off_mode; > > +} > > Considering off-mode suspend depends on how the board is > wired for various things such as memory, PMIC and the related > signal lines, I agree using the machine compatible is the best > check we can do here. > > But since the device can hang during suspend unless things are > configured right for the board, I suggest you rather list allowed > boards here that are known to work with off-mode. > Could we somehow describe this property of the hardware (is-offmode-capable or is-wired-for-offmode) as a separate devicetree property of the soc? In mmc we have for example "cap-power-off-card" for indicating some is-wired-suitable-for thing. That looks to be clearer as looking for some strange conditions derived from something. I have still my offmode patch os the boilerplate... Regards, Andreas
* Andreas Kemnade <andreas@kemnade.info> [190401 18:39]: > On Mon, 1 Apr 2019 10:52:45 -0700 > Tony Lindgren <tony@atomide.com> wrote: > > > Hi, > > > > * Keerthy <j-keerthy@ti.com> [190322 17:16]: > > > +static int am43xx_check_off_mode_enable(void) > > > +{ > > > + /* > > > + * Check for am437x-sk-evm which due to HW design cannot support > > > + * this mode reliably. > > > + */ > > > + if (of_machine_is_compatible("ti,am437x-sk-evm") && enable_off_mode) { > > > + pr_warn("WARNING: This platform does not support off-mode, entering DeepSleep suspend.\n"); > > > + return 0; > > > + } > > > + > > > + return enable_off_mode; > > > +} > > > > Considering off-mode suspend depends on how the board is > > wired for various things such as memory, PMIC and the related > > signal lines, I agree using the machine compatible is the best > > check we can do here. > > > > But since the device can hang during suspend unless things are > > configured right for the board, I suggest you rather list allowed > > boards here that are known to work with off-mode. > > > Could we somehow describe this property of the hardware > (is-offmode-capable or is-wired-for-offmode) as a separate devicetree > property of the soc? > > In mmc we have for example "cap-power-off-card" for > indicating some is-wired-suitable-for thing. And we also have "regulator-off-in-suspend". How about "soc-off-in-suspend" for the generic name? > That looks to be clearer as looking for some strange conditions > derived from something. > > I have still my offmode patch os the boilerplate... Care to send a binding patch to devicetree@vger.kernel.org for that? And then when it gets acked, we can just start using it :) Regards, Tony
On Mon, 1 Apr 2019 13:40:33 -0700 Tony Lindgren <tony@atomide.com> wrote: > * Andreas Kemnade <andreas@kemnade.info> [190401 18:39]: > > On Mon, 1 Apr 2019 10:52:45 -0700 > > Tony Lindgren <tony@atomide.com> wrote: > > > > > Hi, > > > > > > * Keerthy <j-keerthy@ti.com> [190322 17:16]: > > > > +static int am43xx_check_off_mode_enable(void) > > > > +{ > > > > + /* > > > > + * Check for am437x-sk-evm which due to HW design cannot support > > > > + * this mode reliably. > > > > + */ > > > > + if (of_machine_is_compatible("ti,am437x-sk-evm") && enable_off_mode) { > > > > + pr_warn("WARNING: This platform does not support off-mode, entering DeepSleep suspend.\n"); > > > > + return 0; > > > > + } > > > > + > > > > + return enable_off_mode; > > > > +} > > > > > > Considering off-mode suspend depends on how the board is > > > wired for various things such as memory, PMIC and the related > > > signal lines, I agree using the machine compatible is the best > > > check we can do here. > > > > > > But since the device can hang during suspend unless things are > > > configured right for the board, I suggest you rather list allowed > > > boards here that are known to work with off-mode. > > > > > Could we somehow describe this property of the hardware > > (is-offmode-capable or is-wired-for-offmode) as a separate devicetree > > property of the soc? > > > > In mmc we have for example "cap-power-off-card" for > > indicating some is-wired-suitable-for thing. > > And we also have "regulator-off-in-suspend". > > How about "soc-off-in-suspend" for the generic name? > Well, remember my "omap3: give off mode enable a more prominent place" maybe we can use the same capability property for both proposes. I was a bit unsure about it, so I did not continue with it yet Regards, Andreas
* Andreas Kemnade <andreas@kemnade.info> [190404 10:53]: > On Mon, 1 Apr 2019 13:40:33 -0700 > Tony Lindgren <tony@atomide.com> wrote: > > * Andreas Kemnade <andreas@kemnade.info> [190401 18:39]: > > > Could we somehow describe this property of the hardware > > > (is-offmode-capable or is-wired-for-offmode) as a separate devicetree > > > property of the soc? > > > > > > In mmc we have for example "cap-power-off-card" for > > > indicating some is-wired-suitable-for thing. > > > > And we also have "regulator-off-in-suspend". > > > > How about "soc-off-in-suspend" for the generic name? > > > Well, remember my "omap3: give off mode enable a more prominent place" > maybe we can use the same capability property for both proposes. Yes. I don't think we need a Kconfig option for it though. Seems just a device specific property should be enough for now. Regards, Tony
diff --git a/arch/arm/mach-omap2/pm33xx-core.c b/arch/arm/mach-omap2/pm33xx-core.c index 724cf5774a6c..8c02fc73a7a5 100644 --- a/arch/arm/mach-omap2/pm33xx-core.c +++ b/arch/arm/mach-omap2/pm33xx-core.c @@ -10,6 +10,12 @@ #include <asm/suspend.h> #include <linux/errno.h> #include <linux/platform_data/pm33xx.h> +#include <linux/clk.h> +#include <linux/platform_data/gpio-omap.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/wkup_m3_ipc.h> +#include <linux/of.h> +#include <linux/rtc.h> #include "cm33xx.h" #include "common.h" @@ -38,6 +44,29 @@ static int am43xx_map_scu(void) return 0; } +static int am33xx_check_off_mode_enable(void) +{ + if (enable_off_mode) + pr_warn("WARNING: This platform does not support off-mode, entering DeepSleep suspend.\n"); + + /* off mode not supported on am335x so return 0 always */ + return 0; +} + +static int am43xx_check_off_mode_enable(void) +{ + /* + * Check for am437x-sk-evm which due to HW design cannot support + * this mode reliably. + */ + if (of_machine_is_compatible("ti,am437x-sk-evm") && enable_off_mode) { + pr_warn("WARNING: This platform does not support off-mode, entering DeepSleep suspend.\n"); + return 0; + } + + return enable_off_mode; +} + static int amx3_common_init(void) { gfx_pwrdm = pwrdm_lookup("gfx_pwrdm"); @@ -139,7 +168,9 @@ static int am43xx_suspend(unsigned int state, int (*fn)(unsigned long), scu_power_mode(scu_base, SCU_PM_POWEROFF); ret = cpu_suspend(args, fn); scu_power_mode(scu_base, SCU_PM_NORMAL); - amx3_post_suspend_common(); + + if (!am43xx_check_off_mode_enable()) + amx3_post_suspend_common(); return ret; } @@ -161,10 +192,48 @@ void __iomem *am43xx_get_rtc_base_addr(void) return omap_hwmod_get_mpu_rt_va(rtc_oh); } +static void am43xx_save_context(void) +{ +} + +static void am33xx_save_context(void) +{ + omap_intc_save_context(); +} + +static void am33xx_restore_context(void) +{ + omap_intc_restore_context(); +} + +static void am43xx_restore_context(void) +{ + /* + * HACK: restore dpll_per_clkdcoldo register contents, to avoid + * breaking suspend-resume + */ + writel_relaxed(0x0, AM33XX_L4_WK_IO_ADDRESS(0x44df2e14)); +} + +static void am43xx_prepare_rtc_suspend(void) +{ + omap_hwmod_enable(rtc_oh); +} + +static void am43xx_prepare_rtc_resume(void) +{ + omap_hwmod_idle(rtc_oh); +} + static struct am33xx_pm_platform_data am33xx_ops = { .init = am33xx_suspend_init, .soc_suspend = am33xx_suspend, .get_sram_addrs = amx3_get_sram_addrs, + .save_context = am33xx_save_context, + .restore_context = am33xx_restore_context, + .prepare_rtc_suspend = am43xx_prepare_rtc_suspend, + .prepare_rtc_resume = am43xx_prepare_rtc_resume, + .check_off_mode_enable = am33xx_check_off_mode_enable, .get_rtc_base_addr = am43xx_get_rtc_base_addr, }; @@ -172,6 +241,11 @@ static struct am33xx_pm_platform_data am43xx_ops = { .init = am43xx_suspend_init, .soc_suspend = am43xx_suspend, .get_sram_addrs = amx3_get_sram_addrs, + .save_context = am43xx_save_context, + .restore_context = am43xx_restore_context, + .prepare_rtc_suspend = am43xx_prepare_rtc_suspend, + .prepare_rtc_resume = am43xx_prepare_rtc_resume, + .check_off_mode_enable = am43xx_check_off_mode_enable, .get_rtc_base_addr = am43xx_get_rtc_base_addr, }; diff --git a/include/linux/platform_data/pm33xx.h b/include/linux/platform_data/pm33xx.h index fbf5ed73c7cc..dd5971937a64 100644 --- a/include/linux/platform_data/pm33xx.h +++ b/include/linux/platform_data/pm33xx.h @@ -51,6 +51,11 @@ struct am33xx_pm_platform_data { unsigned long args); struct am33xx_pm_sram_addr *(*get_sram_addrs)(void); void __iomem *(*get_rtc_base_addr)(void); + void (*save_context)(void); + void (*restore_context)(void); + void (*prepare_rtc_suspend)(void); + void (*prepare_rtc_resume)(void); + int (*check_off_mode_enable)(void); }; struct am33xx_pm_sram_data {
Add support for rtc+ddr in self refresh mode. Add addtional pm hooks for save/restore and rtc suspend/resume. Signed-off-by: Keerthy <j-keerthy@ti.com> --- arch/arm/mach-omap2/pm33xx-core.c | 76 +++++++++++++++++++++++++++- include/linux/platform_data/pm33xx.h | 5 ++ 2 files changed, 80 insertions(+), 1 deletion(-)