[RFC PATCH v2 0/4] psci: add support for SYSTEM_RESET2 and PSCI_FEATURES

[RFC PATCH v2 0/4] psci: add support for SYSTEM_RESET2 and PSCI_FEATURES

[Igor Opaniuk]

From: Igor Opaniuk <igor.opaniuk@foundries.io>

1. Adds support for:
* PSCI_FEATURES, introduced in PSCI 1.0. This provides API
that allows discovering whether a specific PSCI function is implemented
and its features.
* SYSTEM_RESET2, introduced in PSCI 1.1, which extends existing
SYSTEM_RESET. It provides support for vendor-specific resets, providing
reset_type as an additional param.

2. PSCI sysreset driver is refactored to use new API.
3. do_reset cmd is extended, optional param added for providing type of
reset

CI: https://dev.azure.com/igoropaniuk/u-boot/_build/results?buildId=19&view=results

v2:
- do_reset cmd updates

Igor Opaniuk (4):
  psci: add v1.0/v1.1 definitions from Linux
  psci: add features/reset2 support
  sysreset: psci: use psci driver exported functions
  sysreset: provide type of reset in do_reset cmd

 cmd/boot.c                         |  6 ++-
 drivers/firmware/psci.c            | 70 ++++++++++++++++++++++++++++++
 drivers/sysreset/sysreset-uclass.c | 23 +++++++++-
 drivers/sysreset/sysreset_psci.c   |  8 +---
 include/linux/psci.h               | 31 +++++++++++++
 5 files changed, 130 insertions(+), 8 deletions(-)

-- 
2.25.1

[RFC PATCH v2 1/4] psci: add v1.0/v1.1 definitions from Linux

[Igor Opaniuk]

From: Igor Opaniuk <igor.opaniuk@foundries.io>

Sync and add PSCI API versions 1.0/1.1 definitions from Linux.

Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
---

(no changes since v1)

 include/linux/psci.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/psci.h b/include/linux/psci.h
index 841dbc8da7..38edde3137 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -46,6 +46,14 @@
 #define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
 #define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
 
+#define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
+#define PSCI_1_0_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)
+#define PSCI_1_0_FN_SET_SUSPEND_MODE		PSCI_0_2_FN(15)
+#define PSCI_1_1_FN_SYSTEM_RESET2		PSCI_0_2_FN(18)
+
+#define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
+#define PSCI_1_1_FN64_SYSTEM_RESET2		PSCI_0_2_FN64(18)
+
 /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
 #define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
 #define PSCI_0_2_POWER_STATE_ID_SHIFT		0
@@ -56,6 +64,13 @@
 #define PSCI_0_2_POWER_STATE_AFFL_MASK		\
 				(0x3 << PSCI_0_2_POWER_STATE_AFFL_SHIFT)
 
+/* PSCI extended power state encoding for CPU_SUSPEND function */
+#define PSCI_1_0_EXT_POWER_STATE_ID_MASK	0xfffffff
+#define PSCI_1_0_EXT_POWER_STATE_ID_SHIFT	0
+#define PSCI_1_0_EXT_POWER_STATE_TYPE_SHIFT	30
+#define PSCI_1_0_EXT_POWER_STATE_TYPE_MASK	\
+				(0x1 << PSCI_1_0_EXT_POWER_STATE_TYPE_SHIFT)
+
 /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
 #define PSCI_0_2_AFFINITY_LEVEL_ON		0
 #define PSCI_0_2_AFFINITY_LEVEL_OFF		1
@@ -75,6 +90,18 @@
 		(((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
 #define PSCI_VERSION_MINOR(ver)			\
 		((ver) & PSCI_VERSION_MINOR_MASK)
+#define PSCI_VERSION(maj, min)						\
+	((((maj) << PSCI_VERSION_MAJOR_SHIFT) & PSCI_VERSION_MAJOR_MASK) | \
+	 ((min) & PSCI_VERSION_MINOR_MASK))
+
+/* PSCI features decoding (>=1.0) */
+#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT	1
+#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK	\
+			(0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
+
+#define PSCI_1_0_OS_INITIATED			BIT(0)
+#define PSCI_1_0_SUSPEND_MODE_PC		0
+#define PSCI_1_0_SUSPEND_MODE_OSI		1
 
 /* PSCI return values (inclusive of all PSCI versions) */
 #define PSCI_RET_SUCCESS			0
@@ -86,6 +113,7 @@
 #define PSCI_RET_INTERNAL_FAILURE		-6
 #define PSCI_RET_NOT_PRESENT			-7
 #define PSCI_RET_DISABLED			-8
+#define PSCI_RET_INVALID_ADDRESS		-9
 
 #ifdef CONFIG_ARM_PSCI_FW
 unsigned long invoke_psci_fn(unsigned long a0, unsigned long a1,
-- 
2.25.1

[RFC PATCH v2 2/4] psci: add features/reset2 support

[Igor Opaniuk]

From: Igor Opaniuk <igor.opaniuk@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@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;
+
 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();
+
+	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();
+	}
+
 	return 0;
 }
 
@@ -141,6 +188,29 @@ void reset_misc(void)
 }
 #endif /* CONFIG_PSCI_RESET */
 
+void psci_sys_reset(u32 type)
+{
+	do_psci_probe();
+
+	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);
+	}
+}
+
+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)
-- 
2.25.1

[RFC PATCH v2 3/4] sysreset: psci: use psci driver exported functions

[Igor Opaniuk]

From: Igor Opaniuk <igor.opaniuk@foundries.io>

Use psci driver exported functions for reset/poweroff, instead of
invoking directly invoke_psci_fn.

Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
---

(no changes since v1)

 drivers/sysreset/sysreset_psci.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/sysreset/sysreset_psci.c b/drivers/sysreset/sysreset_psci.c
index c7907b3226..83ecbcb9d2 100644
--- a/drivers/sysreset/sysreset_psci.c
+++ b/drivers/sysreset/sysreset_psci.c
@@ -11,22 +11,18 @@
 
 static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
 {
-	unsigned long function_id;
-
 	switch (type) {
 	case SYSRESET_WARM:
 	case SYSRESET_COLD:
-		function_id = PSCI_0_2_FN_SYSTEM_RESET;
+		psci_sys_reset(type);
 		break;
 	case SYSRESET_POWER_OFF:
-		function_id = PSCI_0_2_FN_SYSTEM_OFF;
+		psci_sys_poweroff();
 		break;
 	default:
 		return -ENOSYS;
 	}
 
-	invoke_psci_fn(function_id, 0, 0, 0);
-
 	return -EINPROGRESS;
 }
 
-- 
2.25.1

[RFC PATCH v2 4/4] sysreset: provide type of reset in do_reset cmd

[Igor Opaniuk]

From: Igor Opaniuk <igor.opaniuk@foundries.io>

Add additional param for reset cmd, which provides type of reset.

Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>

---

 cmd/boot.c                         |  6 +++++-
 drivers/sysreset/sysreset-uclass.c | 23 ++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/cmd/boot.c b/cmd/boot.c
index 36aba22b30..f27277dacf 100644
--- a/cmd/boot.c
+++ b/cmd/boot.c
@@ -56,8 +56,12 @@ U_BOOT_CMD(
 #endif
 
 U_BOOT_CMD(
-	reset, 1, 0,	do_reset,
+	reset, 2, 0,	do_reset,
 	"Perform RESET of the CPU",
+	"[0|1]\n"
+	"   no param  - cold reset [default]\n"
+	"   0 - cold reset\n"
+	"   1 - warm reset\n"
 	""
 );
 
diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
index 6c9dc7a384..65342d8832 100644
--- a/drivers/sysreset/sysreset-uclass.c
+++ b/drivers/sysreset/sysreset-uclass.c
@@ -122,10 +122,31 @@ void reset_cpu(ulong addr)
 #if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET)
 int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
+	u32 param;
+	enum sysreset_t reset_type = SYSRESET_COLD;
+
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (argc == 2) {
+		param = simple_strtoul(argv[1], NULL, 16);
+
+		switch (param) {
+		case 0:
+			reset_type = SYSRESET_COLD;
+			break;
+		case 1:
+			reset_type = SYSRESET_WARM;
+			break;
+		default:
+			return CMD_RET_USAGE;
+		}
+	}
+
 	printf("resetting ...\n");
 	mdelay(100);
 
-	sysreset_walk_halt(SYSRESET_COLD);
+	sysreset_walk_halt(reset_type);
 
 	return 0;
 }
-- 
2.25.1

[RFC PATCH v2 4/4] sysreset: provide type of reset in do_reset cmd

[Heinrich Schuchardt]

On 3/30/21 11:16 PM, Igor Opaniuk wrote:
> From: Igor Opaniuk <igor.opaniuk@foundries.io>
>
> Add additional param for reset cmd, which provides type of reset.
>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
>
> ---
>
>   cmd/boot.c                         |  6 +++++-
>   drivers/sysreset/sysreset-uclass.c | 23 ++++++++++++++++++++++-
>   2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/boot.c b/cmd/boot.c
> index 36aba22b30..f27277dacf 100644
> --- a/cmd/boot.c
> +++ b/cmd/boot.c
> @@ -56,8 +56,12 @@ U_BOOT_CMD(
>   #endif
>
>   U_BOOT_CMD(
> -	reset, 1, 0,	do_reset,
> +	reset, 2, 0,	do_reset,
>   	"Perform RESET of the CPU",

The reset command is included on most boards, even those where we are
very tight on memory.

"Reset the CPU" is enough.

> +	"[0|1]\n"
> +	"   no param  - cold reset [default]\n"
> +	"   0 - cold reset\n"

We can save another few bytes by joining the two lines:

+	"   0 - cold reset (default)\n"

> +	"   1 - warm reset\n"

Could you, please, contribute a man-page in doc/usage/.

Best regards

Heinrich

>   	""
>   );
>
> diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> index 6c9dc7a384..65342d8832 100644
> --- a/drivers/sysreset/sysreset-uclass.c
> +++ b/drivers/sysreset/sysreset-uclass.c
> @@ -122,10 +122,31 @@ void reset_cpu(ulong addr)
>   #if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET)
>   int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   {
> +	u32 param;
> +	enum sysreset_t reset_type = SYSRESET_COLD;
> +
> +	if (argc > 2)
> +		return CMD_RET_USAGE;
> +
> +	if (argc == 2) {
> +		param = simple_strtoul(argv[1], NULL, 16);
> +
> +		switch (param) {
> +		case 0:
> +			reset_type = SYSRESET_COLD;
> +			break;
> +		case 1:
> +			reset_type = SYSRESET_WARM;
> +			break;
> +		default:
> +			return CMD_RET_USAGE;
> +		}
> +	}
> +
>   	printf("resetting ...\n");
>   	mdelay(100);
>
> -	sysreset_walk_halt(SYSRESET_COLD);
> +	sysreset_walk_halt(reset_type);
>
>   	return 0;
>   }
>

[RFC PATCH v2 2/4] psci: add features/reset2 support

[Patrick DELAUNAY]

Hi Igor,

On 3/30/21 11:16 PM, Igor Opaniuk wrote:
> From: Igor Opaniuk <igor.opaniuk@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@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 ?

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

.....

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


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

[RFC PATCH v2 2/4] psci: add features/reset2 support

[Igor Opaniuk]

Hi Patrick,

On Wed, Mar 31, 2021 at 11:32 AM Patrick DELAUNAY <
patrick.delaunay@foss.st.com> wrote:

> Hi Igor,
>
> On 3/30/21 11:16 PM, Igor Opaniuk wrote:
> > From: Igor Opaniuk <igor.opaniuk@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@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

[RFC PATCH v2 4/4] sysreset: provide type of reset in do_reset cmd

[Igor Opaniuk]

Hi Heinrich,

On Wed, Mar 31, 2021 at 9:30 AM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> On 3/30/21 11:16 PM, Igor Opaniuk wrote:
> > From: Igor Opaniuk <igor.opaniuk@foundries.io>
> >
> > Add additional param for reset cmd, which provides type of reset.
> >
> > Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
> >
> > ---
> >
> >   cmd/boot.c                         |  6 +++++-
> >   drivers/sysreset/sysreset-uclass.c | 23 ++++++++++++++++++++++-
> >   2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/boot.c b/cmd/boot.c
> > index 36aba22b30..f27277dacf 100644
> > --- a/cmd/boot.c
> > +++ b/cmd/boot.c
> > @@ -56,8 +56,12 @@ U_BOOT_CMD(
> >   #endif
> >
> >   U_BOOT_CMD(
> > -     reset, 1, 0,    do_reset,
> > +     reset, 2, 0,    do_reset,
> >       "Perform RESET of the CPU",
>
> The reset command is included on most boards, even those where we are
> very tight on memory.
>
> "Reset the CPU" is enough.
>
Ok, will fix.
Also I plan to adjust a bit the param, to make it more self-explanatory.
Something like that to do warm reset:
-> reset -w

>
> > +     "[0|1]\n"
> > +     "   no param  - cold reset [default]\n"
> > +     "   0 - cold reset\n"
>
> We can save another few bytes by joining the two lines:
>
> +       "   0 - cold reset (default)\n"
>
> > +     "   1 - warm reset\n"
>
> Could you, please, contribute a man-page in doc/usage/.
>
Sure, will do.

>
> Best regards
>
> Heinrich
>
> >       ""
> >   );
> >
> > diff --git a/drivers/sysreset/sysreset-uclass.c
> b/drivers/sysreset/sysreset-uclass.c
> > index 6c9dc7a384..65342d8832 100644
> > --- a/drivers/sysreset/sysreset-uclass.c
> > +++ b/drivers/sysreset/sysreset-uclass.c
> > @@ -122,10 +122,31 @@ void reset_cpu(ulong addr)
> >   #if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET)
> >   int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> argv[])
> >   {
> > +     u32 param;
> > +     enum sysreset_t reset_type = SYSRESET_COLD;
> > +
> > +     if (argc > 2)
> > +             return CMD_RET_USAGE;
> > +
> > +     if (argc == 2) {
> > +             param = simple_strtoul(argv[1], NULL, 16);
> > +
> > +             switch (param) {
> > +             case 0:
> > +                     reset_type = SYSRESET_COLD;
> > +                     break;
> > +             case 1:
> > +                     reset_type = SYSRESET_WARM;
> > +                     break;
> > +             default:
> > +                     return CMD_RET_USAGE;
> > +             }
> > +     }
> > +
> >       printf("resetting ...\n");
> >       mdelay(100);
> >
> > -     sysreset_walk_halt(SYSRESET_COLD);
> > +     sysreset_walk_halt(reset_type);
> >
> >       return 0;
> >   }
> >
>
> 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