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

[PATCH v3 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=20&view=results

Changes in v3:
- Drop RFC tag
- Add usage doc for reset cmd
- Reimplement param handling for reset cmd
- Droped updates in reset usage string

Changes in v2:
- do_reset cmd updates

Igor Opaniuk (4):
  psci: add features/reset2 support
  sysreset: psci: use psci driver exported functions
  sysreset: provide type of reset in do_reset cmd
  doc: usage: add usage details for reset cmd

 cmd/boot.c                         |  2 +-
 doc/usage/index.rst                |  1 +
 doc/usage/reset.rst                | 26 ++++++++++++
 drivers/firmware/psci.c            | 68 ++++++++++++++++++++++++++++++
 drivers/sysreset/sysreset-uclass.c | 11 ++++-
 drivers/sysreset/sysreset_psci.c   |  8 +---
 include/linux/psci.h               |  3 ++
 7 files changed, 111 insertions(+), 8 deletions(-)
 create mode 100644 doc/usage/reset.rst

-- 
2.25.1

[PATCH v3 1/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>
---

 drivers/firmware/psci.c | 68 +++++++++++++++++++++++++++++++++++++++++
 include/linux/psci.h    |  3 ++
 2 files changed, 71 insertions(+)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 68953cc4f4..be57552aba 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,6 +27,18 @@
 #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
@@ -53,6 +66,34 @@ 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 bool psci_is_system_reset2_supported(void)
+{
+	int ret;
+	u32 ver;
+
+	ver = psci_0_2_get_version();
+
+	if (PSCI_VERSION_MAJOR(ver) >= 1) {
+		ret = psci_features(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2));
+
+		if (ret != PSCI_RET_NOT_SUPPORTED)
+			return true;
+	}
+
+	return false;
+}
+
 static int psci_bind(struct udevice *dev)
 {
 	/* No SYSTEM_RESET support for PSCI 0.1 */
@@ -141,6 +182,33 @@ void reset_misc(void)
 }
 #endif /* CONFIG_PSCI_RESET */
 
+void psci_sys_reset(u32 type)
+{
+	bool reset2_supported;
+
+	do_psci_probe();
+
+	reset2_supported = psci_is_system_reset2_supported();
+
+	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

[PATCH v3 2/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>
---

 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

[PATCH v3 3/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                         |  2 +-
 drivers/sysreset/sysreset-uclass.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/cmd/boot.c b/cmd/boot.c
index 36aba22b30..b84c0ed89e 100644
--- a/cmd/boot.c
+++ b/cmd/boot.c
@@ -56,7 +56,7 @@ U_BOOT_CMD(
 #endif
 
 U_BOOT_CMD(
-	reset, 1, 0,	do_reset,
+	reset, 2, 0,	do_reset,
 	"Perform RESET of the CPU",
 	""
 );
diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
index 6c9dc7a384..0412c4a29b 100644
--- a/drivers/sysreset/sysreset-uclass.c
+++ b/drivers/sysreset/sysreset-uclass.c
@@ -122,10 +122,19 @@ 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[])
 {
+	enum sysreset_t reset_type = SYSRESET_COLD;
+
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
+		reset_type = SYSRESET_WARM;
+	}
+
 	printf("resetting ...\n");
 	mdelay(100);
 
-	sysreset_walk_halt(SYSRESET_COLD);
+	sysreset_walk_halt(reset_type);
 
 	return 0;
 }
-- 
2.25.1

[PATCH v3 4/4] doc: usage: add usage details for reset cmd

[Igor Opaniuk]

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

Add usage details for reset command.

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

---

 doc/usage/index.rst |  1 +
 doc/usage/reset.rst | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 doc/usage/reset.rst

diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 6c59bbadab..b1181011ae 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -34,3 +34,4 @@ Shell commands
    qfw
    sbi
    true
+   reset
diff --git a/doc/usage/reset.rst b/doc/usage/reset.rst
new file mode 100644
index 0000000000..f8cf3579c8
--- /dev/null
+++ b/doc/usage/reset.rst
@@ -0,0 +1,26 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+reset command
+============
+
+Synopsis
+--------
+
+::
+
+    reset [-w]
+
+Description
+-----------
+
+Perform reset of the CPU. By default does COLD reset, which resets CPU,
+DDR and peripherals, on some boards also resets external PMIC.
+
+-w
+    Do warm WARM, reset CPU but keep peripheral/DDR/PMIC active.
+
+
+Return value
+------------
+
+The return value $? is always set to 0 (true).
-- 
2.25.1

[PATCH v3 4/4] doc: usage: add usage details for reset cmd

[Heinrich Schuchardt]

On 3/31/21 10:16 PM, Igor Opaniuk wrote:
> From: Igor Opaniuk <igor.opaniuk@foundries.io>
>
> Add usage details for reset command.
>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
>
> ---
>
>   doc/usage/index.rst |  1 +
>   doc/usage/reset.rst | 26 ++++++++++++++++++++++++++
>   2 files changed, 27 insertions(+)
>   create mode 100644 doc/usage/reset.rst
>
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index 6c59bbadab..b1181011ae 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -34,3 +34,4 @@ Shell commands
>      qfw
>      sbi
>      true
> +   reset

Dear Igor,

please, keep the alphabetic order.

> diff --git a/doc/usage/reset.rst b/doc/usage/reset.rst
> new file mode 100644
> index 0000000000..f8cf3579c8
> --- /dev/null
> +++ b/doc/usage/reset.rst
> @@ -0,0 +1,26 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +reset command
> +============

reset.rst:4:Title underline too short.

This leads to a build error for 'make htmldocs'.

> +
> +Synopsis
> +--------
> +
> +::
> +
> +    reset [-w]
> +
> +Description
> +-----------
> +
> +Perform reset of the CPU. By default does COLD reset, which resets CPU,
> +DDR and peripherals, on some boards also resets external PMIC.
> +
> +-w
> +    Do warm WARM, reset CPU but keep peripheral/DDR/PMIC active.
> +
> +
> +Return value
> +------------
> +
> +The return value $? is always set to 0 (true).

Please, mention that reset only returns in case of a failure.

Best regards

Heinrich

>

[PATCH v3 3/4] sysreset: provide type of reset in do_reset cmd

[Heinrich Schuchardt]

On 3/31/21 10: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>

It seems you are missing a lot of do_reset() implementations in your
series which may also have to be adjusted to conform to the changed
reset command.

arch/arc/lib/reset.c:16:int do_reset(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])
arch/arm/lib/reset.c:33:int do_reset(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])
arch/m68k/cpu/mcf5227x/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf523x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:145:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:180:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:269:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:359:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:378:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf52x2/cpu.c:410:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf530x/cpu.c:15:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf532x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf5445x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/m68k/cpu/mcf547x_8x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/microblaze/cpu/spl.c:53:int do_reset(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
arch/mips/cpu/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int flag, int
argc, char *const argv[])
arch/nds32/cpu/n1213/ae3xx/cpu.c:42:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/nds32/cpu/n1213/ag101/cpu.c:42:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/nios2/cpu/cpu.c:37:int do_reset(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])
arch/powerpc/cpu/mpc83xx/cpu.c:128:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/powerpc/cpu/mpc85xx/cpu.c:301:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/powerpc/cpu/mpc86xx/cpu.c:112:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/powerpc/cpu/mpc8xx/cpu.c:199:int do_reset(struct cmd_tbl *cmdtp,
int flag, int argc, char *const argv[])
arch/riscv/lib/reset.c:10:int do_reset(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])
arch/sh/cpu/sh4/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])

Best regards

Heinrich

> ---
>
>   cmd/boot.c                         |  2 +-
>   drivers/sysreset/sysreset-uclass.c | 11 ++++++++++-
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/boot.c b/cmd/boot.c
> index 36aba22b30..b84c0ed89e 100644
> --- a/cmd/boot.c
> +++ b/cmd/boot.c
> @@ -56,7 +56,7 @@ U_BOOT_CMD(
>   #endif
>
>   U_BOOT_CMD(
> -	reset, 1, 0,	do_reset,
> +	reset, 2, 0,	do_reset,
>   	"Perform RESET of the CPU",
>   	""
>   );
> diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> index 6c9dc7a384..0412c4a29b 100644
> --- a/drivers/sysreset/sysreset-uclass.c
> +++ b/drivers/sysreset/sysreset-uclass.c
> @@ -122,10 +122,19 @@ 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[])
>   {
> +	enum sysreset_t reset_type = SYSRESET_COLD;
> +
> +	if (argc > 2)
> +		return CMD_RET_USAGE;
> +
> +	if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> +		reset_type = SYSRESET_WARM;
> +	}
> +
>   	printf("resetting ...\n");
>   	mdelay(100);
>
> -	sysreset_walk_halt(SYSRESET_COLD);
> +	sysreset_walk_halt(reset_type);
>
>   	return 0;
>   }
>

[PATCH v3 3/4] sysreset: provide type of reset in do_reset cmd

[Igor Opaniuk]

Hi Heinrich

On Thu, Apr 1, 2021 at 12:36 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 3/31/21 10: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>
>
> It seems you are missing a lot of do_reset() implementations in your
> series which may also have to be adjusted to conform to the changed
> reset command.
>
> arch/arc/lib/reset.c:16:int do_reset(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> arch/arm/lib/reset.c:33:int do_reset(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> arch/m68k/cpu/mcf5227x/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf523x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:145:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:180:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:269:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:359:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:378:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf52x2/cpu.c:410:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf530x/cpu.c:15:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf532x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf5445x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/m68k/cpu/mcf547x_8x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/microblaze/cpu/spl.c:53:int do_reset(struct cmd_tbl *cmdtp, int
> flag, int argc, char *const argv[])
> arch/mips/cpu/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int flag, int
> argc, char *const argv[])
> arch/nds32/cpu/n1213/ae3xx/cpu.c:42:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/nds32/cpu/n1213/ag101/cpu.c:42:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/nios2/cpu/cpu.c:37:int do_reset(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> arch/powerpc/cpu/mpc83xx/cpu.c:128:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/powerpc/cpu/mpc85xx/cpu.c:301:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/powerpc/cpu/mpc86xx/cpu.c:112:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/powerpc/cpu/mpc8xx/cpu.c:199:int do_reset(struct cmd_tbl *cmdtp,
> int flag, int argc, char *const argv[])
> arch/riscv/lib/reset.c:10:int do_reset(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> arch/sh/cpu/sh4/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])

This change don't really impact the behaviour of do_reset implementations
you mentioned.:
-       reset, 1, 0,    do_reset,
+       reset, 2, 0,    do_reset,

I have checked maybe half of that do_reset() implementations, and none of them
has any logic that relies somehow on argc/argv[]

>
> Best regards
>
> Heinrich
>
> > ---
> >
> >   cmd/boot.c                         |  2 +-
> >   drivers/sysreset/sysreset-uclass.c | 11 ++++++++++-
> >   2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/boot.c b/cmd/boot.c
> > index 36aba22b30..b84c0ed89e 100644
> > --- a/cmd/boot.c
> > +++ b/cmd/boot.c
> > @@ -56,7 +56,7 @@ U_BOOT_CMD(
> >   #endif
> >
> >   U_BOOT_CMD(
> > -     reset, 1, 0,    do_reset,
> > +     reset, 2, 0,    do_reset,
> >       "Perform RESET of the CPU",
> >       ""
> >   );
> > diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> > index 6c9dc7a384..0412c4a29b 100644
> > --- a/drivers/sysreset/sysreset-uclass.c
> > +++ b/drivers/sysreset/sysreset-uclass.c
> > @@ -122,10 +122,19 @@ 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[])
> >   {
> > +     enum sysreset_t reset_type = SYSRESET_COLD;
> > +
> > +     if (argc > 2)
> > +             return CMD_RET_USAGE;
> > +
> > +     if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> > +             reset_type = SYSRESET_WARM;
> > +     }
> > +
> >       printf("resetting ...\n");
> >       mdelay(100);
> >
> > -     sysreset_walk_halt(SYSRESET_COLD);
> > +     sysreset_walk_halt(reset_type);
> >
> >       return 0;
> >   }
> >
>


-- 
Best regards - Freundliche Gr?sse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[PATCH v3 4/4] doc: usage: add usage details for reset cmd

[Igor Opaniuk]

Hi Heinrich,

On Thu, Apr 1, 2021 at 12:30 AM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> On 3/31/21 10:16 PM, Igor Opaniuk wrote:
> > From: Igor Opaniuk <igor.opaniuk@foundries.io>
> >
> > Add usage details for reset command.
> >
> > Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
> >
> > ---
> >
> >   doc/usage/index.rst |  1 +
> >   doc/usage/reset.rst | 26 ++++++++++++++++++++++++++
> >   2 files changed, 27 insertions(+)
> >   create mode 100644 doc/usage/reset.rst
> >
> > diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> > index 6c59bbadab..b1181011ae 100644
> > --- a/doc/usage/index.rst
> > +++ b/doc/usage/index.rst
> > @@ -34,3 +34,4 @@ Shell commands
> >      qfw
> >      sbi
> >      true
> > +   reset
>
> Dear Igor,
>
> please, keep the alphabetic order.
>
Just saw you message, will send an update in v5.

>
> > diff --git a/doc/usage/reset.rst b/doc/usage/reset.rst
> > new file mode 100644
> > index 0000000000..f8cf3579c8
> > --- /dev/null
> > +++ b/doc/usage/reset.rst
> > @@ -0,0 +1,26 @@
> > +.. SPDX-License-Identifier: GPL-2.0+
> > +
> > +reset command
> > +============
>
> reset.rst:4:Title underline too short.
>
> This leads to a build error for 'make htmldocs'.
>
Yeah, fixed that already in v4

>
> > +
> > +Synopsis
> > +--------
> > +
> > +::
> > +
> > +    reset [-w]
> > +
> > +Description
> > +-----------
> > +
> > +Perform reset of the CPU. By default does COLD reset, which resets CPU,
> > +DDR and peripherals, on some boards also resets external PMIC.
> > +
> > +-w
> > +    Do warm WARM, reset CPU but keep peripheral/DDR/PMIC active.
> > +
> > +
> > +Return value
> > +------------
> > +
> > +The return value $? is always set to 0 (true).
>
> Please, mention that reset only returns in case of a failure.
>
Will fix in v5.

>
> Best regards
>
> Heinrich
>
> >
>
>
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

[PATCH v3 3/4] sysreset: provide type of reset in do_reset cmd

[Heinrich Schuchardt]

Am 1. April 2021 01:40:06 MESZ schrieb Igor Opaniuk <igor.opaniuk@gmail.com>:
>Hi Heinrich
>
>On Thu, Apr 1, 2021 at 12:36 AM Heinrich Schuchardt
><xypron.glpk@gmx.de> wrote:
>>
>> On 3/31/21 10: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>
>>
>> It seems you are missing a lot of do_reset() implementations in your
>> series which may also have to be adjusted to conform to the changed
>> reset command.
>>
>> arch/arc/lib/reset.c:16:int do_reset(struct cmd_tbl *cmdtp, int flag,
>> int argc, char *const argv[])
>> arch/arm/lib/reset.c:33:int do_reset(struct cmd_tbl *cmdtp, int flag,
>> int argc, char *const argv[])
>> arch/m68k/cpu/mcf5227x/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf523x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:145:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:180:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:269:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:359:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:378:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf52x2/cpu.c:410:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf530x/cpu.c:15:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf532x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf5445x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp,
>int
>> flag, int argc, char *const argv[])
>> arch/m68k/cpu/mcf547x_8x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/microblaze/cpu/spl.c:53:int do_reset(struct cmd_tbl *cmdtp, int
>> flag, int argc, char *const argv[])
>> arch/mips/cpu/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int flag,
>int
>> argc, char *const argv[])
>> arch/nds32/cpu/n1213/ae3xx/cpu.c:42:int do_reset(struct cmd_tbl
>*cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/nds32/cpu/n1213/ag101/cpu.c:42:int do_reset(struct cmd_tbl
>*cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/nios2/cpu/cpu.c:37:int do_reset(struct cmd_tbl *cmdtp, int flag,
>> int argc, char *const argv[])
>> arch/powerpc/cpu/mpc83xx/cpu.c:128:int do_reset(struct cmd_tbl
>*cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/powerpc/cpu/mpc85xx/cpu.c:301:int do_reset(struct cmd_tbl
>*cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/powerpc/cpu/mpc86xx/cpu.c:112:int do_reset(struct cmd_tbl
>*cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/powerpc/cpu/mpc8xx/cpu.c:199:int do_reset(struct cmd_tbl *cmdtp,
>> int flag, int argc, char *const argv[])
>> arch/riscv/lib/reset.c:10:int do_reset(struct cmd_tbl *cmdtp, int
>flag,
>> int argc, char *const argv[])
>> arch/sh/cpu/sh4/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int
>flag,
>> int argc, char *const argv[])
>
>This change don't really impact the behaviour of do_reset
>implementations
>you mentioned.:
>-       reset, 1, 0,    do_reset,
>+       reset, 2, 0,    do_reset,
>
>I have checked maybe half of that do_reset() implementations, and none
>of them
>has any logic that relies somehow on argc/argv[]


The changed reyet command is meant to allow choosing between cold and warm reset.

I would have expexted that this holds true for all architectures, e.g. RISC-V.

Best regards

Heinrich


>
>>
>> Best regards
>>
>> Heinrich
>>
>> > ---
>> >
>> >   cmd/boot.c                         |  2 +-
>> >   drivers/sysreset/sysreset-uclass.c | 11 ++++++++++-
>> >   2 files changed, 11 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/cmd/boot.c b/cmd/boot.c
>> > index 36aba22b30..b84c0ed89e 100644
>> > --- a/cmd/boot.c
>> > +++ b/cmd/boot.c
>> > @@ -56,7 +56,7 @@ U_BOOT_CMD(
>> >   #endif
>> >
>> >   U_BOOT_CMD(
>> > -     reset, 1, 0,    do_reset,
>> > +     reset, 2, 0,    do_reset,
>> >       "Perform RESET of the CPU",
>> >       ""
>> >   );
>> > diff --git a/drivers/sysreset/sysreset-uclass.c
>b/drivers/sysreset/sysreset-uclass.c
>> > index 6c9dc7a384..0412c4a29b 100644
>> > --- a/drivers/sysreset/sysreset-uclass.c
>> > +++ b/drivers/sysreset/sysreset-uclass.c
>> > @@ -122,10 +122,19 @@ 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[])
>> >   {
>> > +     enum sysreset_t reset_type = SYSRESET_COLD;
>> > +
>> > +     if (argc > 2)
>> > +             return CMD_RET_USAGE;
>> > +
>> > +     if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
>> > +             reset_type = SYSRESET_WARM;
>> > +     }
>> > +
>> >       printf("resetting ...\n");
>> >       mdelay(100);
>> >
>> > -     sysreset_walk_halt(SYSRESET_COLD);
>> > +     sysreset_walk_halt(reset_type);
>> >
>> >       return 0;
>> >   }
>> >
>>

[PATCH v3 3/4] sysreset: provide type of reset in do_reset cmd

[Igor Opaniuk]

On Thu, Apr 1, 2021 at 2:44 AM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> Am 1. April 2021 01:40:06 MESZ schrieb Igor Opaniuk <
> igor.opaniuk at gmail.com>:
> >Hi Heinrich
> >
> >On Thu, Apr 1, 2021 at 12:36 AM Heinrich Schuchardt
> ><xypron.glpk@gmx.de> wrote:
> >>
> >> On 3/31/21 10: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>
> >>
> >> It seems you are missing a lot of do_reset() implementations in your
> >> series which may also have to be adjusted to conform to the changed
> >> reset command.
> >>
> >> arch/arc/lib/reset.c:16:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >> int argc, char *const argv[])
> >> arch/arm/lib/reset.c:33:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >> int argc, char *const argv[])
> >> arch/m68k/cpu/mcf5227x/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf523x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:145:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:180:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:269:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:359:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:378:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:410:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf530x/cpu.c:15:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf532x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf5445x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf547x_8x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/microblaze/cpu/spl.c:53:int do_reset(struct cmd_tbl *cmdtp, int
> >> flag, int argc, char *const argv[])
> >> arch/mips/cpu/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >int
> >> argc, char *const argv[])
> >> arch/nds32/cpu/n1213/ae3xx/cpu.c:42:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/nds32/cpu/n1213/ag101/cpu.c:42:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/nios2/cpu/cpu.c:37:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >> int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc83xx/cpu.c:128:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc85xx/cpu.c:301:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc86xx/cpu.c:112:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc8xx/cpu.c:199:int do_reset(struct cmd_tbl *cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/riscv/lib/reset.c:10:int do_reset(struct cmd_tbl *cmdtp, int
> >flag,
> >> int argc, char *const argv[])
> >> arch/sh/cpu/sh4/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int
> >flag,
> >> int argc, char *const argv[])
> >
> >This change don't really impact the behaviour of do_reset
> >implementations
> >you mentioned.:
> >-       reset, 1, 0,    do_reset,
> >+       reset, 2, 0,    do_reset,
> >
> >I have checked maybe half of that do_reset() implementations, and none
> >of them
> >has any logic that relies somehow on argc/argv[]
>
>
> The changed reyet command is meant to allow choosing between cold and warm
> reset.
>
> I would have expexted that this holds true for all architectures, e.g.
> RISC-V.
>

Well, I think this goes beyond this patch series, as :
1. The main scope of this patch is adjustments in PSCI firmware/sysreset
drivers, adding support
for the new version of PSCI API so it's possible to use alternative PSCI
RESET2 API call.
Extending do_reset() was just a consequence, as basically I wanted to avoid
introducing a new
arch-specific command which could be used for that.
If that doesn't work for you, I can create something like "psci_reset"
instead.

2. All do_reset() implementations you listed are all arch/SoC specific,
that's not even
theoretically feasible to adjust every function as I don't have neither
hardware nor time
for doing that. I strongly believe that it should be done by arch/soc/board
maintainers in
case they want to have support of alternative resets.

I can add an explicit notice to doc/usage/reset.rst that warm reset might
not be implemented for your
arch/soc/board, and in that case regular reset will be triggered.
Let me know what option works best for you.

Thanks


> Best regards
>
> Heinrich
>
>
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >> > ---
> >> >
> >> >   cmd/boot.c                         |  2 +-
> >> >   drivers/sysreset/sysreset-uclass.c | 11 ++++++++++-
> >> >   2 files changed, 11 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/cmd/boot.c b/cmd/boot.c
> >> > index 36aba22b30..b84c0ed89e 100644
> >> > --- a/cmd/boot.c
> >> > +++ b/cmd/boot.c
> >> > @@ -56,7 +56,7 @@ U_BOOT_CMD(
> >> >   #endif
> >> >
> >> >   U_BOOT_CMD(
> >> > -     reset, 1, 0,    do_reset,
> >> > +     reset, 2, 0,    do_reset,
> >> >       "Perform RESET of the CPU",
> >> >       ""
> >> >   );
> >> > diff --git a/drivers/sysreset/sysreset-uclass.c
> >b/drivers/sysreset/sysreset-uclass.c
> >> > index 6c9dc7a384..0412c4a29b 100644
> >> > --- a/drivers/sysreset/sysreset-uclass.c
> >> > +++ b/drivers/sysreset/sysreset-uclass.c
> >> > @@ -122,10 +122,19 @@ 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[])
> >> >   {
> >> > +     enum sysreset_t reset_type = SYSRESET_COLD;
> >> > +
> >> > +     if (argc > 2)
> >> > +             return CMD_RET_USAGE;
> >> > +
> >> > +     if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> >> > +             reset_type = SYSRESET_WARM;
> >> > +     }
> >> > +
> >> >       printf("resetting ...\n");
> >> >       mdelay(100);
> >> >
> >> > -     sysreset_walk_halt(SYSRESET_COLD);
> >> > +     sysreset_walk_halt(reset_type);
> >> >
> >> >       return 0;
> >> >   }
> >> >
> >>
>
>

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

[PATCH v3 1/4] psci: add features/reset2 support

[Simon Glass]

Hi Igor,

On Thu, 1 Apr 2021 at 09:16, Igor Opaniuk <igor.opaniuk@foundries.io> 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>
> ---
>
>  drivers/firmware/psci.c | 68 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/psci.h    |  3 ++
>  2 files changed, 71 insertions(+)

Do we have a test for this?

Regards,
Simon