[RFC PATCH v2 2/4] psci: add features/reset2 support
Igor Opaniuk
igor.opaniuk at foundries.io
Wed Mar 31 17:32:54 CEST 2021
Hi Patrick,
On Wed, Mar 31, 2021 at 11:32 AM Patrick DELAUNAY <
patrick.delaunay at foss.st.com> wrote:
> Hi Igor,
>
> On 3/30/21 11:16 PM, Igor Opaniuk wrote:
> > From: Igor Opaniuk <igor.opaniuk at foundries.io>
> >
> > Adds support for:
> > * PSCI_FEATURES, which was introduced in PSCI 1.0. This provides API
> > that allows discovering whether a specific PSCI function is implemented
> > and its features.
> > * SYSTEM_RESET2, which was introduced in PSCI 1.1, which extends existing
> > SYSTEM_RESET. It provides support for vendor-specific resets, providing
> > reset_type as an additional param.
> >
> > For additional details visit [1].
> >
> > Implementations of some functions were borrowed from Linux PSCI driver
> > code [2].
> >
> > [1] https://developer.arm.com/documentation/den0022/latest/
> > [2] drivers/firmware/psci/psci.c
> >
> > Signed-off-by: Igor Opaniuk <igor.opaniuk at foundries.io>
> > ---
> >
> > (no changes since v1)
> >
> > drivers/firmware/psci.c | 70 +++++++++++++++++++++++++++++++++++++++++
> > include/linux/psci.h | 3 ++
> > 2 files changed, 73 insertions(+)
> >
> > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> > index 68953cc4f4..8416cd445f 100644
> > --- a/drivers/firmware/psci.c
> > +++ b/drivers/firmware/psci.c
> > @@ -13,6 +13,7 @@
> > #include <log.h>
> > #include <dm/lists.h>
> > #include <efi_loader.h>
> > +#include <sysreset.h>
> > #include <linux/delay.h>
> > #include <linux/libfdt.h>
> > #include <linux/arm-smccc.h>
> > @@ -26,12 +27,26 @@
> > #define PSCI_METHOD_HVC 1
> > #define PSCI_METHOD_SMC 2
> >
> > +/*
> > + * While a 64-bit OS can make calls with SMC32 calling conventions, for
> some
> > + * calls it is necessary to use SMC64 to pass or return 64-bit values.
> > + * For such calls PSCI_FN_NATIVE(version, name) will choose the
> appropriate
> > + * (native-width) function ID.
> > + */
> > +#if defined(CONFIG_ARM64)
> > +#define PSCI_FN_NATIVE(version, name)
> PSCI_##version##_FN64_##name
> > +#else
> > +#define PSCI_FN_NATIVE(version, name) PSCI_##version##_FN_##name
> > +#endif
> > +
> > #if CONFIG_IS_ENABLED(EFI_LOADER)
> > int __efi_runtime_data psci_method;
> > #else
> > int psci_method __attribute__ ((section(".data")));
> > #endif
> >
> > +static bool reset2_supported;
> > +
>
> I think this global variable here can cause issue before relocation and
> for UEFI ?
>
right will move it to private data .
>
> => solved for psci_method...
>
>
> I think it should be moved in psci private data if it is needed (see after)
>
>
> > unsigned long __efi_runtime invoke_psci_fn
> > (unsigned long function_id, unsigned long arg0,
> > unsigned long arg1, unsigned long arg2)
> > @@ -53,6 +68,27 @@ unsigned long __efi_runtime invoke_psci_fn
> > return res.a0;
> > }
> >
> > +static int psci_features(u32 psci_func_id)
> > +{
> > + return invoke_psci_fn(PSCI_1_0_FN_PSCI_FEATURES,
> > + psci_func_id, 0, 0);
> > +}
> > +
> > +static u32 psci_0_2_get_version(void)
> > +{
> > + return invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> > +}
> > +
> > +static void psci_init_system_reset2(void)
> > +{
> > + int ret;
> > +
> > + ret = psci_features(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2));
> > +
> > + if (ret != PSCI_RET_NOT_SUPPORTED)
> > + reset2_supported = true;
> > +}
> > +
> > static int psci_bind(struct udevice *dev)
> > {
> > /* No SYSTEM_RESET support for PSCI 0.1 */
> > @@ -73,6 +109,7 @@ static int psci_bind(struct udevice *dev)
> > static int psci_probe(struct udevice *dev)
> > {
> > const char *method;
> > + u32 ver;
> >
> > #if defined(CONFIG_ARM64)
> > if (current_el() == 3)
> > @@ -94,6 +131,16 @@ static int psci_probe(struct udevice *dev)
> > return -EINVAL;
> > }
>
> > + ver = psci_0_2_get_version();
> > +
>
> priv->ver = psci_0_2_get_version();
>
>
> > + pr_debug("PSCIv%d.%d detected in firmware.\n",
> > + PSCI_VERSION_MAJOR(ver),
> > + PSCI_VERSION_MINOR(ver));
> > +
> > + if (PSCI_VERSION_MAJOR(ver) >= 1) {
> > + psci_init_system_reset2();
>
> in other functions, the version check is based on the compatible..
>
> static int psci_bind(struct udevice *dev)
> {
> /* No SYSTEM_RESET support for PSCI 0.1 */
> if (device_is_compatible(dev, "arm,psci-0.2") ||
> device_is_compatible(dev, "arm,psci-1.0")) {
>
> it should be updated also ?
>
Well, it's a chicken-egg problem. We can not obtain a real version of PSCI
FW in psci_bind,
as we haven't probed/initialized the device yet.
On the other hand it sysreset driver should not really care about what PSCI
sysreset method
we use for rebooting (this decision is made here in psci_sys_reset ()).
>
> .....
>
> priv->reset2 = psci_init_system_reset2();
>
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -141,6 +188,29 @@ void reset_misc(void)
> > }
> > #endif /* CONFIG_PSCI_RESET */
> >
> > +void psci_sys_reset(u32 type)
> > +{
> > + do_psci_probe();
> > +
>
> dev = do_psci_probe();
>
> priv = get_priv(dev);
>
>
> Then private data are available (it is safer than global)
>
>
> > + if (type == SYSRESET_WARM && reset2_supported) {
> > + /*
> > + * reset_type[31] = 0 (architectural)
> > + * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
> > + * cookie = 0 (ignored by the implementation)
> > + */
> > + invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0,
> 0);
> > + } else {
> > + invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> > + }
> > +}
>
> These almost same function already exist in
> ./drivers/sysreset/sysreset_psci.c
>
> => handle in do_reset() / do_poweroff() :
> drivers/sysreset/sysreset-uclass.c
>
> bind by:
>
> drivers/firmware/psci.c:64: ret = device_bind_driver(dev,
> "psci-sysreset", "psci-sysreset",
>
Please check the full series [1], as I'm also changing the sysreset driver
(for some
unknown reason patman didn't add your email to CC for other patches, will
do that
manually for the next version).
[1] http://patchwork.ozlabs.org/project/uboot/list/?series=236613
>
>
> I think your modication should be moved in this part.
>
>
> > +
> > +void psci_sys_poweroff(void)
> > +{
> > + do_psci_probe();
> > +
> > + invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> > +}
> > +
>
> > #ifdef CONFIG_CMD_POWEROFF
> > int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> argv[])
> > {
> > diff --git a/include/linux/psci.h b/include/linux/psci.h
> > index 38edde3137..c78c1079a8 100644
> > --- a/include/linux/psci.h
> > +++ b/include/linux/psci.h
> > @@ -118,6 +118,9 @@
> > #ifdef CONFIG_ARM_PSCI_FW
> > unsigned long invoke_psci_fn(unsigned long a0, unsigned long a1,
> > unsigned long a2, unsigned long a3);
> > +void psci_sys_reset(u32 type);
> > +void psci_sys_poweroff(void);
> > +
> > #else
> > static inline unsigned long invoke_psci_fn(unsigned long a0, unsigned
> long a1,
> > unsigned long a2, unsigned long
> a3)
>
>
> Regards
>
>
> Patrick
>
>
Thanks
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
Embedded Software Engineer
T: +380 938364067
E: igor.opaniuk at foundries.io
W: www.foundries.io
More information about the U-Boot
mailing list