Messages in this thread | | | From | <> | Subject | Re: [PATCH v2 2/2] ARM: at91: pm: add support for sama5d2 secure suspend | Date | Tue, 8 Mar 2022 08:21:01 +0000 |
| |
Hi Clément,
On 07.03.2022 12:15, Clément Léger wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > When running with OP-TEE, the suspend control is handled securely. > Suspend can be entered using PSCI support. Since the sama5d2 supports > multiple suspend modes, add a new CONFIG_ATMEL_SECURE_PM which will > send a SMC call to select the suspend mode at init time. > > "atmel.pm_modes" boot argument is still supported for compatibility > purposes but the standby value is actually ignored since PSCI suspend > is used and it only support one mode (suspend). > > Signed-off-by: Clément Léger <clement.leger@bootlin.com> > --- > arch/arm/mach-at91/Kconfig | 12 ++++++++++- > arch/arm/mach-at91/pm.c | 38 +++++++++++++++++++++++++++++++++ > arch/arm/mach-at91/sam_secure.h | 4 ++++ > 3 files changed, 53 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig > index 02f6b108fd5d..5a6ca38d6303 100644 > --- a/arch/arm/mach-at91/Kconfig > +++ b/arch/arm/mach-at91/Kconfig > @@ -208,7 +208,17 @@ config SOC_SAMA5 > select SRAM if PM > > config ATMEL_PM > - bool > + bool "Atmel PM support" > + > +config ATMEL_SECURE_PM > + bool "Atmel Secure PM support" > + depends on SOC_SAMA5D2 && ATMEL_PM > + select ARM_PSCI > + help > + When running under a TEE, the suspend mode must be requested to be set > + at TEE level. When enable, this option will use secure monitor calls > + to set the suspend level. PSCI is then used to enter suspend. > + NOTE: This support is mutually exclusive with CONFIG_ATMEL_PM
This note may be a bit confusing as ATMEL_SECURE_PM depends on CONFIG_ATMEL_PM.
> > config SOC_SAMA7 > bool > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c > index dd6f4ce3f766..e40515691540 100644 > --- a/arch/arm/mach-at91/pm.c > +++ b/arch/arm/mach-at91/pm.c > @@ -27,6 +27,7 @@ > > #include "generic.h" > #include "pm.h" > +#include "sam_secure.h" > > #define BACKUP_DDR_PHY_CALIBRATION (9) > > @@ -856,6 +857,35 @@ static int __init at91_pm_backup_init(void) > return ret; > } > > +static void at91_pm_secure_init(void) > +{ > + int suspend_mode; > + struct arm_smccc_res res; > + > + suspend_mode = soc_pm.data.suspend_mode; > + > + res = sam_smccc_call(SAMA5_SMC_SIP_SET_SUSPEND_MODE, > + suspend_mode, 0); > + if (res.a0 == 0) { > + pr_info("AT91: Secure PM: suspend mode set to %s\n", > + pm_modes[suspend_mode].pattern); > + return; > + } > + > + pr_warn("AT91: Secure PM: %s mode not supported !\n", > + pm_modes[suspend_mode].pattern); > + > + res = sam_smccc_call(SAMA5_SMC_SIP_GET_SUSPEND_MODE, 0, 0); > + if (res.a0 == 0) { > + pr_warn("AT91: Secure PM: failed to get default mode\n"); > + return; > + } > + > + pr_info("AT91: Secure PM: using default suspend mode %s\n", > + pm_modes[suspend_mode].pattern); > + > + soc_pm.data.suspend_mode = res.a1; > +} > static const struct of_device_id atmel_shdwc_ids[] = { > { .compatible = "atmel,sama5d2-shdwc" }, > { .compatible = "microchip,sam9x60-shdwc" }, > @@ -1188,6 +1218,11 @@ void __init sama5d2_pm_init(void) > if (!IS_ENABLED(CONFIG_SOC_SAMA5D2)) > return; > > + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) { > + at91_pm_secure_init(); > + return; > + } > + > at91_pm_modes_validate(modes, ARRAY_SIZE(modes)); > at91_pm_modes_init(iomaps, ARRAY_SIZE(iomaps)); > ret = at91_dt_ramc(false); > @@ -1262,6 +1297,9 @@ static int __init at91_pm_modes_select(char *str) > soc_pm.data.standby_mode = standby; > soc_pm.data.suspend_mode = suspend; > > + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) > + pr_warn("AT91: Secure PM: ignoring standby mode\n");
What happens with soc_pm.data.standby_mode? Can Linux still suspend to it? If that's the case then the proper data for standby mode is not initialized as sama5d2_pm_init() returns if CONFIG_ATMEL_SECURE_PM is enabled, thus system may hang in such a case. Is this intended?
Otherwise, maybe this pr_warn() could be moved in sama5d2_pm_init() as sama5d2 is the only user at the moment.
Thank you, Claudiu Beznea
> + > return 0; > } > early_param("atmel.pm_modes", at91_pm_modes_select); > diff --git a/arch/arm/mach-at91/sam_secure.h b/arch/arm/mach-at91/sam_secure.h > index 360036672f52..1e7d8b20ba1e 100644 > --- a/arch/arm/mach-at91/sam_secure.h > +++ b/arch/arm/mach-at91/sam_secure.h > @@ -8,6 +8,10 @@ > > #include <linux/arm-smccc.h> > > +/* Secure Monitor mode APIs */ > +#define SAMA5_SMC_SIP_SET_SUSPEND_MODE 0x400 > +#define SAMA5_SMC_SIP_GET_SUSPEND_MODE 0x401 > + > void __init sam_secure_init(void); > struct arm_smccc_res sam_smccc_call(u32 fn, u32 arg0, u32 arg1); > > -- > 2.34.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
| |