[PATCH v1 0/3] Implement reset to EDL for qcs9100

[PATCH v1 0/3] Implement reset to EDL for qcs9100

[Varadarajan Narayanan]

Using the 'reboot edl' command in Linux, the platform can reboot to the
Emergency Download mode. Implement the same for U-Boot.

Varadarajan Narayanan (3):
  mach-snapdragon: Do not define reset_cpu() if SYSRESET is enabled
  sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs
  configs: qcs9100_defconfig: Enable SYSRESET

 arch/arm/mach-snapdragon/board.c      |  2 ++
 configs/qcs9100_defconfig             |  2 ++
 drivers/sysreset/Kconfig              |  5 +++
 drivers/sysreset/Makefile             |  1 +
 drivers/sysreset/sysreset-uclass.c    |  7 ++--
 drivers/sysreset/sysreset_qcom-psci.c | 47 +++++++++++++++++++++++++++
 include/sysreset.h                    |  2 ++
 7 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 drivers/sysreset/sysreset_qcom-psci.c

-- 
2.34.1

[PATCH v1 1/3] mach-snapdragon: Do not define reset_cpu() if SYSRESET is enabled

[Varadarajan Narayanan]

If CONFIG_SYSRESET is enabled, the reset_cpu() function defined in
arch/arm/mach-snapdragon/board.c conflicts with the definition of
reset_cpu() in drivers/sysreset/sysreset-uclass.c resulting in duplicate
symbol error while compiling. So, do not include it if CONFIG_SYSRESET
is enabled.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 arch/arm/mach-snapdragon/board.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
index 2ef936aab75..2254e0b55a1 100644
--- a/arch/arm/mach-snapdragon/board.c
+++ b/arch/arm/mach-snapdragon/board.c
@@ -187,10 +187,12 @@ int board_fdt_blob_setup(void **fdtp)
 	return ret;
 }
 
+#if !IS_ENABLED(CONFIG_SYSRESET)
 void reset_cpu(void)
 {
 	psci_system_reset();
 }
+#endif
 
 /*
  * Some Qualcomm boards require GPIO configuration when switching USB modes.
-- 
2.34.1

Re: [PATCH v1 1/3] mach-snapdragon: Do not define reset_cpu() if SYSRESET is enabled

[Sumit Garg]

On Thu, Apr 10, 2025 at 05:32:06PM +0530, Varadarajan Narayanan wrote:
> If CONFIG_SYSRESET is enabled, the reset_cpu() function defined in
> arch/arm/mach-snapdragon/board.c conflicts with the definition of
> reset_cpu() in drivers/sysreset/sysreset-uclass.c resulting in duplicate
> symbol error while compiling. So, do not include it if CONFIG_SYSRESET
> is enabled.

I would rather like to see all Qcom platforms migrating to SYSRESET but
this looks reasonable to me as of now, FWIW:

Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>

-Sumit

> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  arch/arm/mach-snapdragon/board.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> index 2ef936aab75..2254e0b55a1 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -187,10 +187,12 @@ int board_fdt_blob_setup(void **fdtp)
>  	return ret;
>  }
>  
> +#if !IS_ENABLED(CONFIG_SYSRESET)
>  void reset_cpu(void)
>  {
>  	psci_system_reset();
>  }
> +#endif
>  
>  /*
>   * Some Qualcomm boards require GPIO configuration when switching USB modes.
> -- 
> 2.34.1
> 

Re: [PATCH v1 1/3] mach-snapdragon: Do not define reset_cpu() if SYSRESET is enabled

[Sam Day]

Hi Sumit, Varadarajan,

On Monday, April 28th, 2025 at 6:25 PM, Sumit Garg <sumit.garg@kernel.org> wrote:

> 
> 
> On Thu, Apr 10, 2025 at 05:32:06PM +0530, Varadarajan Narayanan wrote:
> 
> > If CONFIG_SYSRESET is enabled, the reset_cpu() function defined in
> > arch/arm/mach-snapdragon/board.c conflicts with the definition of
> > reset_cpu() in drivers/sysreset/sysreset-uclass.c resulting in duplicate
> > symbol error while compiling. So, do not include it if CONFIG_SYSRESET
> > is enabled.
> 
> 
> I would rather like to see all Qcom platforms migrating to SYSRESET but
> this looks reasonable to me as of now, FWIW:
> 
> Reviewed-by: Sumit Garg sumit.garg@oss.qualcomm.com

reset_cpu was already dropped in commit 61a1a1b

So I think this patch can be dropped from the series?

Cheers,
-Sam

> 
> 
> -Sumit
> 
> > Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com
> > ---
> > arch/arm/mach-snapdragon/board.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> > index 2ef936aab75..2254e0b55a1 100644
> > --- a/arch/arm/mach-snapdragon/board.c
> > +++ b/arch/arm/mach-snapdragon/board.c
> > @@ -187,10 +187,12 @@ int board_fdt_blob_setup(void **fdtp)
> > return ret;
> > }
> > 
> > +#if !IS_ENABLED(CONFIG_SYSRESET)
> > void reset_cpu(void)
> > {
> > psci_system_reset();
> > }
> > +#endif
> > 
> > /*
> > * Some Qualcomm boards require GPIO configuration when switching USB modes.
> > --
> > 2.34.1

Re: [PATCH v1 1/3] mach-snapdragon: Do not define reset_cpu() if SYSRESET is enabled

[Casey Connolly]

Hi Varadarajan,

Please rebase on U-Boot master.

On 4/10/25 14:02, Varadarajan Narayanan wrote:
> If CONFIG_SYSRESET is enabled, the reset_cpu() function defined in
> arch/arm/mach-snapdragon/board.c conflicts with the definition of
> reset_cpu() in drivers/sysreset/sysreset-uclass.c resulting in duplicate
> symbol error while compiling. So, do not include it if CONFIG_SYSRESET
> is enabled.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>   arch/arm/mach-snapdragon/board.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> index 2ef936aab75..2254e0b55a1 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -187,10 +187,12 @@ int board_fdt_blob_setup(void **fdtp)
>   	return ret;
>   }
>   
> +#if !IS_ENABLED(CONFIG_SYSRESET)
>   void reset_cpu(void)
>   {
>   	psci_system_reset();
>   }
> +#endif
>   
>   /*
>    * Some Qualcomm boards require GPIO configuration when switching USB modes.

-- 
Casey (she/they)

[PATCH v1 2/3] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs

[Varadarajan Narayanan]

Add SYSRESET_EDL to sysreset_t and handle the different SYSRESET_xxx
requests in sysreset_qcom-psci.c.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/sysreset/Kconfig              |  5 +++
 drivers/sysreset/Makefile             |  1 +
 drivers/sysreset/sysreset-uclass.c    |  7 ++--
 drivers/sysreset/sysreset_qcom-psci.c | 47 +++++++++++++++++++++++++++
 include/sysreset.h                    |  2 ++
 5 files changed, 60 insertions(+), 2 deletions(-)
 create mode 100644 drivers/sysreset/sysreset_qcom-psci.c

diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index 121194e4418..a6a1e57b603 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -240,6 +240,11 @@ config SYSRESET_RAA215300
 	help
 	  Add support for the system reboot via the Renesas RAA215300 PMIC.
 
+config SYSRESET_QCOM_PSCI
+	bool "Support sysreset for Qualcomm SoCs via PSCI"
+	help
+	  Add support for the system reboot on Qualcomm SoCs via PSCI.
+
 config SYSRESET_QCOM_PSHOLD
 	bool "Support sysreset for Qualcomm SoCs via PSHOLD"
 	depends on ARCH_IPQ40XX
diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
index 796fc9effa5..12dbad5254a 100644
--- a/drivers/sysreset/Makefile
+++ b/drivers/sysreset/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
 obj-$(CONFIG_$(PHASE_)SYSRESET_AT91) += sysreset_at91.o
 obj-$(CONFIG_$(PHASE_)SYSRESET_X86) += sysreset_x86.o
 obj-$(CONFIG_SYSRESET_RAA215300) += sysreset_raa215300.o
+obj-$(CONFIG_SYSRESET_QCOM_PSCI) += sysreset_qcom-psci.o
 obj-$(CONFIG_SYSRESET_QCOM_PSHOLD) += sysreset_qcom-pshold.o
 obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o
diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
index 536ac727142..d06c0a6908a 100644
--- a/drivers/sysreset/sysreset-uclass.c
+++ b/drivers/sysreset/sysreset-uclass.c
@@ -125,8 +125,11 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	if (argc > 2)
 		return CMD_RET_USAGE;
 
-	if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
-		reset_type = SYSRESET_WARM;
+	if (argc == 2) {
+		if (argv[1][0] == '-' && argv[1][1] == 'w')
+			reset_type = SYSRESET_WARM;
+		else if (!strncmp("edl", argv[1], 3))
+			reset_type = SYSRESET_EDL;
 	}
 
 	printf("resetting ...\n");
diff --git a/drivers/sysreset/sysreset_qcom-psci.c b/drivers/sysreset/sysreset_qcom-psci.c
new file mode 100644
index 00000000000..3afb79e2d2e
--- /dev/null
+++ b/drivers/sysreset/sysreset_qcom-psci.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2017 Masahiro Yamada <yamada.masahiro@socionext.com>
+ */
+
+#include <dm.h>
+#include <sysreset.h>
+#include <asm/system.h>
+#include <linux/errno.h>
+#include <linux/psci.h>
+
+__weak int qcom_psci_sysreset_get_status(struct udevice *dev, char *buf, int size)
+{
+	return -EOPNOTSUPP;
+}
+
+static int qcom_psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+	switch (type) {
+	case SYSRESET_WARM:
+	case SYSRESET_COLD:
+		psci_sys_reset(type);
+		break;
+	case SYSRESET_POWER_OFF:
+		psci_sys_poweroff();
+		break;
+	case SYSRESET_EDL:
+		psci_system_reset2(0, 1);
+		break;
+	default:
+		return -EPROTONOSUPPORT;
+	}
+
+	return -EINPROGRESS;
+}
+
+static struct sysreset_ops qcom_psci_sysreset_ops = {
+	.request = qcom_psci_sysreset_request,
+	.get_status = qcom_psci_sysreset_get_status,
+};
+
+U_BOOT_DRIVER(qcom_psci_sysreset) = {
+	.name = "qcom_psci-sysreset",
+	.id = UCLASS_SYSRESET,
+	.ops = &qcom_psci_sysreset_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
diff --git a/include/sysreset.h b/include/sysreset.h
index ff20abdeed3..8bda9703cd9 100644
--- a/include/sysreset.h
+++ b/include/sysreset.h
@@ -21,6 +21,8 @@ enum sysreset_t {
 	SYSRESET_POWER,
 	/** @SYSRESET_POWER_OFF: turn off power */
 	SYSRESET_POWER_OFF,
+	/** @SYSRESET_EDL: reset and boot into Emergency DownLoader */
+	SYSRESET_EDL,
 	/** @SYSRESET_COUNT: number of available reset types */
 	SYSRESET_COUNT,
 };
-- 
2.34.1

Re: [PATCH v1 2/3] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs

[Sumit Garg]

On Thu, Apr 10, 2025 at 05:32:07PM +0530, Varadarajan Narayanan wrote:
> Add SYSRESET_EDL to sysreset_t and handle the different SYSRESET_xxx
> requests in sysreset_qcom-psci.c.

Kindly add ellborative commit message as to what problem you are trying
to address here and what is actually EDL mode? Is it specific to Qcom
platforms or does any other silicon vendor supports similar method?

> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/sysreset/Kconfig              |  5 +++
>  drivers/sysreset/Makefile             |  1 +
>  drivers/sysreset/sysreset-uclass.c    |  7 ++--
>  drivers/sysreset/sysreset_qcom-psci.c | 47 +++++++++++++++++++++++++++
>  include/sysreset.h                    |  2 ++
>  5 files changed, 60 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/sysreset/sysreset_qcom-psci.c
> 
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 121194e4418..a6a1e57b603 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -240,6 +240,11 @@ config SYSRESET_RAA215300
>  	help
>  	  Add support for the system reboot via the Renesas RAA215300 PMIC.
>  
> +config SYSRESET_QCOM_PSCI
> +	bool "Support sysreset for Qualcomm SoCs via PSCI"
> +	help
> +	  Add support for the system reboot on Qualcomm SoCs via PSCI.

Ellaborate here to mention support for poweroff and EDL mode.

> +
>  config SYSRESET_QCOM_PSHOLD
>  	bool "Support sysreset for Qualcomm SoCs via PSHOLD"
>  	depends on ARCH_IPQ40XX
> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> index 796fc9effa5..12dbad5254a 100644
> --- a/drivers/sysreset/Makefile
> +++ b/drivers/sysreset/Makefile
> @@ -29,5 +29,6 @@ obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
>  obj-$(CONFIG_$(PHASE_)SYSRESET_AT91) += sysreset_at91.o
>  obj-$(CONFIG_$(PHASE_)SYSRESET_X86) += sysreset_x86.o
>  obj-$(CONFIG_SYSRESET_RAA215300) += sysreset_raa215300.o
> +obj-$(CONFIG_SYSRESET_QCOM_PSCI) += sysreset_qcom-psci.o
>  obj-$(CONFIG_SYSRESET_QCOM_PSHOLD) += sysreset_qcom-pshold.o
>  obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o
> diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> index 536ac727142..d06c0a6908a 100644
> --- a/drivers/sysreset/sysreset-uclass.c
> +++ b/drivers/sysreset/sysreset-uclass.c
> @@ -125,8 +125,11 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  	if (argc > 2)
>  		return CMD_RET_USAGE;
>  
> -	if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> -		reset_type = SYSRESET_WARM;
> +	if (argc == 2) {
> +		if (argv[1][0] == '-' && argv[1][1] == 'w')
> +			reset_type = SYSRESET_WARM;
> +		else if (!strncmp("edl", argv[1], 3))

Try to follow existing pattern "reset -w" like "reset -edl".

> +			reset_type = SYSRESET_EDL;
>  	}
>  
>  	printf("resetting ...\n");
> diff --git a/drivers/sysreset/sysreset_qcom-psci.c b/drivers/sysreset/sysreset_qcom-psci.c
> new file mode 100644
> index 00000000000..3afb79e2d2e
> --- /dev/null
> +++ b/drivers/sysreset/sysreset_qcom-psci.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2017 Masahiro Yamada <yamada.masahiro@socionext.com>

Forgot to append copyright here?

> + */
> +
> +#include <dm.h>
> +#include <sysreset.h>
> +#include <asm/system.h>
> +#include <linux/errno.h>
> +#include <linux/psci.h>
> +
> +__weak int qcom_psci_sysreset_get_status(struct udevice *dev, char *buf, int size)

Why is it declared weak?

> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int qcom_psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
> +{
> +	switch (type) {
> +	case SYSRESET_WARM:
> +	case SYSRESET_COLD:
> +		psci_sys_reset(type);
> +		break;
> +	case SYSRESET_POWER_OFF:
> +		psci_sys_poweroff();
> +		break;
> +	case SYSRESET_EDL:
> +		psci_system_reset2(0, 1);
> +		break;
> +	default:
> +		return -EPROTONOSUPPORT;
> +	}
> +
> +	return -EINPROGRESS;
> +}
> +
> +static struct sysreset_ops qcom_psci_sysreset_ops = {
> +	.request = qcom_psci_sysreset_request,
> +	.get_status = qcom_psci_sysreset_get_status,
> +};
> +
> +U_BOOT_DRIVER(qcom_psci_sysreset) = {
> +	.name = "qcom_psci-sysreset",
> +	.id = UCLASS_SYSRESET,
> +	.ops = &qcom_psci_sysreset_ops,
> +	.flags = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/include/sysreset.h b/include/sysreset.h
> index ff20abdeed3..8bda9703cd9 100644
> --- a/include/sysreset.h
> +++ b/include/sysreset.h
> @@ -21,6 +21,8 @@ enum sysreset_t {
>  	SYSRESET_POWER,
>  	/** @SYSRESET_POWER_OFF: turn off power */
>  	SYSRESET_POWER_OFF,
> +	/** @SYSRESET_EDL: reset and boot into Emergency DownLoader */

Is it Qcom specific then add proper comments here?

-Sumit

> +	SYSRESET_EDL,
>  	/** @SYSRESET_COUNT: number of available reset types */
>  	SYSRESET_COUNT,
>  };
> -- 
> 2.34.1
> 

Re: [PATCH v1 2/3] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs

[Casey Connolly]

On 4/10/25 14:02, Varadarajan Narayanan wrote:
> Add SYSRESET_EDL to sysreset_t and handle the different SYSRESET_xxx
> requests in sysreset_qcom-psci.c.

To be honest, I'm not really excited about this. Copying the entire PSCI 
reset code and polluting the global sysreset reason enum with entirely 
platform specific symbols is not good practise.

Additionally, while this specific EDL implementation works on your 
platform, it does not work on (for example) sdm845 and sm8250 which are 
platforms we still very much care about. I'm also not sure if it works 
on SC7280.

I think this should be handled in an/the SCM driver like it is in Linux, 
not sure if Sam is working on that or if I can send a barebones version 
of the one I have in my tree. It will need to be based on the compatible 
string for the scm node in DT (and checking for the qcom,dload-mode 
property too I think).

As I proposed before on IRC, I think it makes sense to pass the "reset" 
arguments into sysreset and allow the sysreset driver to implement a 
callback/handler for them, this way the "edl" argument makes it all the 
way to our platform-specific driver which can then call 
"psci_system_reset2()"

I would implement this by adding a new op to sysreset_ops, something 
like .request_args, then have a second for loop which calls this, since 
we want any reset handler which /can/ parse the args to do so before we 
call the normal .request callback (otherwise some other handler might do 
a normal reset before we have parsed the arguments).

For now I'm fine with this new sysreset-qcom.c driver being specific to 
qcs9100, but please include a comment that the psci reset2 call is only 
supported on some newer qualcomm platforms and that others can be 
supported via SCM.

On that note, how does this new driver actually get bound? It doesn't 
have any compatible strings associated? Or am I missing something 
here... Please explain this.

Otherwise, I'm looking forward to v2.

Kind regards,

> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>   drivers/sysreset/Kconfig              |  5 +++
>   drivers/sysreset/Makefile             |  1 +
>   drivers/sysreset/sysreset-uclass.c    |  7 ++--
>   drivers/sysreset/sysreset_qcom-psci.c | 47 +++++++++++++++++++++++++++
>   include/sysreset.h                    |  2 ++
>   5 files changed, 60 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/sysreset/sysreset_qcom-psci.c
> 
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 121194e4418..a6a1e57b603 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -240,6 +240,11 @@ config SYSRESET_RAA215300
>   	help
>   	  Add support for the system reboot via the Renesas RAA215300 PMIC.
>   
> +config SYSRESET_QCOM_PSCI
> +	bool "Support sysreset for Qualcomm SoCs via PSCI"
> +	help
> +	  Add support for the system reboot on Qualcomm SoCs via PSCI.
> +
>   config SYSRESET_QCOM_PSHOLD
>   	bool "Support sysreset for Qualcomm SoCs via PSHOLD"
>   	depends on ARCH_IPQ40XX
> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> index 796fc9effa5..12dbad5254a 100644
> --- a/drivers/sysreset/Makefile
> +++ b/drivers/sysreset/Makefile
> @@ -29,5 +29,6 @@ obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
>   obj-$(CONFIG_$(PHASE_)SYSRESET_AT91) += sysreset_at91.o
>   obj-$(CONFIG_$(PHASE_)SYSRESET_X86) += sysreset_x86.o
>   obj-$(CONFIG_SYSRESET_RAA215300) += sysreset_raa215300.o
> +obj-$(CONFIG_SYSRESET_QCOM_PSCI) += sysreset_qcom-psci.o
>   obj-$(CONFIG_SYSRESET_QCOM_PSHOLD) += sysreset_qcom-pshold.o
>   obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o
> diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> index 536ac727142..d06c0a6908a 100644
> --- a/drivers/sysreset/sysreset-uclass.c
> +++ b/drivers/sysreset/sysreset-uclass.c
> @@ -125,8 +125,11 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   	if (argc > 2)
>   		return CMD_RET_USAGE;
>   
> -	if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> -		reset_type = SYSRESET_WARM;
> +	if (argc == 2) {
> +		if (argv[1][0] == '-' && argv[1][1] == 'w')
> +			reset_type = SYSRESET_WARM;
> +		else if (!strncmp("edl", argv[1], 3))
> +			reset_type = SYSRESET_EDL;
>   	}
>   
>   	printf("resetting ...\n");
> diff --git a/drivers/sysreset/sysreset_qcom-psci.c b/drivers/sysreset/sysreset_qcom-psci.c
> new file mode 100644
> index 00000000000..3afb79e2d2e
> --- /dev/null
> +++ b/drivers/sysreset/sysreset_qcom-psci.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2017 Masahiro Yamada <yamada.masahiro@socionext.com>
> + */
> +
> +#include <dm.h>
> +#include <sysreset.h>
> +#include <asm/system.h>
> +#include <linux/errno.h>
> +#include <linux/psci.h>
> +
> +__weak int qcom_psci_sysreset_get_status(struct udevice *dev, char *buf, int size)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int qcom_psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
> +{
> +	switch (type) {
> +	case SYSRESET_WARM:
> +	case SYSRESET_COLD:
> +		psci_sys_reset(type);
> +		break;
> +	case SYSRESET_POWER_OFF:
> +		psci_sys_poweroff();
> +		break;
> +	case SYSRESET_EDL:
> +		psci_system_reset2(0, 1);
> +		break;
> +	default:
> +		return -EPROTONOSUPPORT;
> +	}
> +
> +	return -EINPROGRESS;
> +}
> +
> +static struct sysreset_ops qcom_psci_sysreset_ops = {
> +	.request = qcom_psci_sysreset_request,
> +	.get_status = qcom_psci_sysreset_get_status,
> +};
> +
> +U_BOOT_DRIVER(qcom_psci_sysreset) = {
> +	.name = "qcom_psci-sysreset",
> +	.id = UCLASS_SYSRESET,
> +	.ops = &qcom_psci_sysreset_ops,
> +	.flags = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/include/sysreset.h b/include/sysreset.h
> index ff20abdeed3..8bda9703cd9 100644
> --- a/include/sysreset.h
> +++ b/include/sysreset.h
> @@ -21,6 +21,8 @@ enum sysreset_t {
>   	SYSRESET_POWER,
>   	/** @SYSRESET_POWER_OFF: turn off power */
>   	SYSRESET_POWER_OFF,
> +	/** @SYSRESET_EDL: reset and boot into Emergency DownLoader */
> +	SYSRESET_EDL,
>   	/** @SYSRESET_COUNT: number of available reset types */
>   	SYSRESET_COUNT,
>   };
-- 
Casey (she/they)

Re: [PATCH v1 2/3] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs

[Varadarajan Narayanan]

On Mon, Apr 28, 2025 at 04:22:47PM +0200, Casey Connolly wrote:
>
>
> On 4/10/25 14:02, Varadarajan Narayanan wrote:
> > Add SYSRESET_EDL to sysreset_t and handle the different SYSRESET_xxx
> > requests in sysreset_qcom-psci.c.
>
> To be honest, I'm not really excited about this. Copying the entire PSCI
> reset code and polluting the global sysreset reason enum with entirely
> platform specific symbols is not good practise.
>
> Additionally, while this specific EDL implementation works on your platform,
> it does not work on (for example) sdm845 and sm8250 which are platforms we
> still very much care about. I'm also not sure if it works on SC7280.
>
> I think this should be handled in an/the SCM driver like it is in Linux, not
> sure if Sam is working on that or if I can send a barebones version of the
> one I have in my tree. It will need to be based on the compatible string for
> the scm node in DT (and checking for the qcom,dload-mode property too I
> think).
>
> As I proposed before on IRC, I think it makes sense to pass the "reset"
> arguments into sysreset and allow the sysreset driver to implement a
> callback/handler for them, this way the "edl" argument makes it all the way
> to our platform-specific driver which can then call "psci_system_reset2()"
>
> I would implement this by adding a new op to sysreset_ops, something like
> .request_args, then have a second for loop which calls this, since we want
> any reset handler which /can/ parse the args to do so before we call the
> normal .request callback (otherwise some other handler might do a normal
> reset before we have parsed the arguments).
>
> For now I'm fine with this new sysreset-qcom.c driver being specific to
> qcs9100, but please include a comment that the psci reset2 call is only
> supported on some newer qualcomm platforms and that others can be supported
> via SCM.
>
> On that note, how does this new driver actually get bound? It doesn't have
> any compatible strings associated? Or am I missing something here... Please
> explain this.

Apologies, my bad for not mentioning it. Presently it is not getting bound.
Wanted to get feedback about the approach.

> Otherwise, I'm looking forward to v2.

Will post v2, with device_bind_driver("qcom_psci-sysreset") in psci_bind().

Thanks
Varada

> Kind regards,
>
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >   drivers/sysreset/Kconfig              |  5 +++
> >   drivers/sysreset/Makefile             |  1 +
> >   drivers/sysreset/sysreset-uclass.c    |  7 ++--
> >   drivers/sysreset/sysreset_qcom-psci.c | 47 +++++++++++++++++++++++++++
> >   include/sysreset.h                    |  2 ++
> >   5 files changed, 60 insertions(+), 2 deletions(-)
> >   create mode 100644 drivers/sysreset/sysreset_qcom-psci.c
> >
> > diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> > index 121194e4418..a6a1e57b603 100644
> > --- a/drivers/sysreset/Kconfig
> > +++ b/drivers/sysreset/Kconfig
> > @@ -240,6 +240,11 @@ config SYSRESET_RAA215300
> >   	help
> >   	  Add support for the system reboot via the Renesas RAA215300 PMIC.
> > +config SYSRESET_QCOM_PSCI
> > +	bool "Support sysreset for Qualcomm SoCs via PSCI"
> > +	help
> > +	  Add support for the system reboot on Qualcomm SoCs via PSCI.
> > +
> >   config SYSRESET_QCOM_PSHOLD
> >   	bool "Support sysreset for Qualcomm SoCs via PSHOLD"
> >   	depends on ARCH_IPQ40XX
> > diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> > index 796fc9effa5..12dbad5254a 100644
> > --- a/drivers/sysreset/Makefile
> > +++ b/drivers/sysreset/Makefile
> > @@ -29,5 +29,6 @@ obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
> >   obj-$(CONFIG_$(PHASE_)SYSRESET_AT91) += sysreset_at91.o
> >   obj-$(CONFIG_$(PHASE_)SYSRESET_X86) += sysreset_x86.o
> >   obj-$(CONFIG_SYSRESET_RAA215300) += sysreset_raa215300.o
> > +obj-$(CONFIG_SYSRESET_QCOM_PSCI) += sysreset_qcom-psci.o
> >   obj-$(CONFIG_SYSRESET_QCOM_PSHOLD) += sysreset_qcom-pshold.o
> >   obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o
> > diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> > index 536ac727142..d06c0a6908a 100644
> > --- a/drivers/sysreset/sysreset-uclass.c
> > +++ b/drivers/sysreset/sysreset-uclass.c
> > @@ -125,8 +125,11 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> >   	if (argc > 2)
> >   		return CMD_RET_USAGE;
> > -	if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> > -		reset_type = SYSRESET_WARM;
> > +	if (argc == 2) {
> > +		if (argv[1][0] == '-' && argv[1][1] == 'w')
> > +			reset_type = SYSRESET_WARM;
> > +		else if (!strncmp("edl", argv[1], 3))
> > +			reset_type = SYSRESET_EDL;
> >   	}
> >   	printf("resetting ...\n");
> > diff --git a/drivers/sysreset/sysreset_qcom-psci.c b/drivers/sysreset/sysreset_qcom-psci.c
> > new file mode 100644
> > index 00000000000..3afb79e2d2e
> > --- /dev/null
> > +++ b/drivers/sysreset/sysreset_qcom-psci.c
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2017 Masahiro Yamada <yamada.masahiro@socionext.com>
> > + */
> > +
> > +#include <dm.h>
> > +#include <sysreset.h>
> > +#include <asm/system.h>
> > +#include <linux/errno.h>
> > +#include <linux/psci.h>
> > +
> > +__weak int qcom_psci_sysreset_get_status(struct udevice *dev, char *buf, int size)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +
> > +static int qcom_psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
> > +{
> > +	switch (type) {
> > +	case SYSRESET_WARM:
> > +	case SYSRESET_COLD:
> > +		psci_sys_reset(type);
> > +		break;
> > +	case SYSRESET_POWER_OFF:
> > +		psci_sys_poweroff();
> > +		break;
> > +	case SYSRESET_EDL:
> > +		psci_system_reset2(0, 1);
> > +		break;
> > +	default:
> > +		return -EPROTONOSUPPORT;
> > +	}
> > +
> > +	return -EINPROGRESS;
> > +}
> > +
> > +static struct sysreset_ops qcom_psci_sysreset_ops = {
> > +	.request = qcom_psci_sysreset_request,
> > +	.get_status = qcom_psci_sysreset_get_status,
> > +};
> > +
> > +U_BOOT_DRIVER(qcom_psci_sysreset) = {
> > +	.name = "qcom_psci-sysreset",
> > +	.id = UCLASS_SYSRESET,
> > +	.ops = &qcom_psci_sysreset_ops,
> > +	.flags = DM_FLAG_PRE_RELOC,
> > +};
> > diff --git a/include/sysreset.h b/include/sysreset.h
> > index ff20abdeed3..8bda9703cd9 100644
> > --- a/include/sysreset.h
> > +++ b/include/sysreset.h
> > @@ -21,6 +21,8 @@ enum sysreset_t {
> >   	SYSRESET_POWER,
> >   	/** @SYSRESET_POWER_OFF: turn off power */
> >   	SYSRESET_POWER_OFF,
> > +	/** @SYSRESET_EDL: reset and boot into Emergency DownLoader */
> > +	SYSRESET_EDL,
> >   	/** @SYSRESET_COUNT: number of available reset types */
> >   	SYSRESET_COUNT,
> >   };
> --
> Casey (she/they)
>

Re: [PATCH v1 2/3] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs

[Sam Day]

Hey Casey, Varadarajan,

On Tuesday, April 29th, 2025 at 12:22 AM, Casey Connolly <casey.connolly@linaro.org> wrote:

> 
> 
> 
> 
> On 4/10/25 14:02, Varadarajan Narayanan wrote:
> 
> > Add SYSRESET_EDL to sysreset_t and handle the different SYSRESET_xxx
> > requests in sysreset_qcom-psci.c.
> 
> 
> To be honest, I'm not really excited about this. Copying the entire PSCI
> reset code and polluting the global sysreset reason enum with entirely
> platform specific symbols is not good practise.
> 
> Additionally, while this specific EDL implementation works on your
> platform, it does not work on (for example) sdm845 and sm8250 which are
> platforms we still very much care about. I'm also not sure if it works
> on SC7280.
> 
> I think this should be handled in an/the SCM driver like it is in Linux,
> not sure if Sam is working on that or if I can send a barebones version
> of the one I have in my tree. It will need to be based on the compatible
> string for the scm node in DT (and checking for the qcom,dload-mode
> property too I think).

I've started on the v2 of the SCM driver patch, but I'm behind on
everything atm, and not expecting to resend that series for another
couple of weeks.

If someone wants to pick up the SCM part please be my guest :D

Thanks,
-Sam

> 
> As I proposed before on IRC, I think it makes sense to pass the "reset"
> arguments into sysreset and allow the sysreset driver to implement a
> callback/handler for them, this way the "edl" argument makes it all the
> way to our platform-specific driver which can then call
> "psci_system_reset2()"
> 
> I would implement this by adding a new op to sysreset_ops, something
> like .request_args, then have a second for loop which calls this, since
> we want any reset handler which /can/ parse the args to do so before we
> call the normal .request callback (otherwise some other handler might do
> a normal reset before we have parsed the arguments).
> 
> For now I'm fine with this new sysreset-qcom.c driver being specific to
> qcs9100, but please include a comment that the psci reset2 call is only
> supported on some newer qualcomm platforms and that others can be
> supported via SCM.
> 
> On that note, how does this new driver actually get bound? It doesn't
> have any compatible strings associated? Or am I missing something
> here... Please explain this.
> 
> Otherwise, I'm looking forward to v2.
> 
> Kind regards,
> 
> > Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com
> > ---
> > drivers/sysreset/Kconfig | 5 +++
> > drivers/sysreset/Makefile | 1 +
> > drivers/sysreset/sysreset-uclass.c | 7 ++--
> > drivers/sysreset/sysreset_qcom-psci.c | 47 +++++++++++++++++++++++++++
> > include/sysreset.h | 2 ++
> > 5 files changed, 60 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/sysreset/sysreset_qcom-psci.c
> > 
> > diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> > index 121194e4418..a6a1e57b603 100644
> > --- a/drivers/sysreset/Kconfig
> > +++ b/drivers/sysreset/Kconfig
> > @@ -240,6 +240,11 @@ config SYSRESET_RAA215300
> > help
> > Add support for the system reboot via the Renesas RAA215300 PMIC.
> > 
> > +config SYSRESET_QCOM_PSCI
> > + bool "Support sysreset for Qualcomm SoCs via PSCI"
> > + help
> > + Add support for the system reboot on Qualcomm SoCs via PSCI.
> > +
> > config SYSRESET_QCOM_PSHOLD
> > bool "Support sysreset for Qualcomm SoCs via PSHOLD"
> > depends on ARCH_IPQ40XX
> > diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> > index 796fc9effa5..12dbad5254a 100644
> > --- a/drivers/sysreset/Makefile
> > +++ b/drivers/sysreset/Makefile
> > @@ -29,5 +29,6 @@ obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
> > obj-$(CONFIG_$(PHASE_)SYSRESET_AT91) += sysreset_at91.o
> > obj-$(CONFIG_$(PHASE_)SYSRESET_X86) += sysreset_x86.o
> > obj-$(CONFIG_SYSRESET_RAA215300) += sysreset_raa215300.o
> > +obj-$(CONFIG_SYSRESET_QCOM_PSCI) += sysreset_qcom-psci.o
> > obj-$(CONFIG_SYSRESET_QCOM_PSHOLD) += sysreset_qcom-pshold.o
> > obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o
> > diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> > index 536ac727142..d06c0a6908a 100644
> > --- a/drivers/sysreset/sysreset-uclass.c
> > +++ b/drivers/sysreset/sysreset-uclass.c
> > @@ -125,8 +125,11 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > if (argc > 2)
> > return CMD_RET_USAGE;
> > 
> > - if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> > - reset_type = SYSRESET_WARM;
> > + if (argc == 2) {
> > + if (argv[1][0] == '-' && argv[1][1] == 'w')
> > + reset_type = SYSRESET_WARM;
> > + else if (!strncmp("edl", argv[1], 3))
> > + reset_type = SYSRESET_EDL;
> > }
> > 
> > printf("resetting ...\n");
> > diff --git a/drivers/sysreset/sysreset_qcom-psci.c b/drivers/sysreset/sysreset_qcom-psci.c
> > new file mode 100644
> > index 00000000000..3afb79e2d2e
> > --- /dev/null
> > +++ b/drivers/sysreset/sysreset_qcom-psci.c
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2017 Masahiro Yamada yamada.masahiro@socionext.com
> > + /
> > +
> > +#include <dm.h>
> > +#include <sysreset.h>
> > +#include <asm/system.h>
> > +#include <linux/errno.h>
> > +#include <linux/psci.h>
> > +
> > +__weak int qcom_psci_sysreset_get_status(struct udevice dev, char buf, int size)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static int qcom_psci_sysreset_request(struct udevice dev, enum sysreset_t type)
> > +{
> > + switch (type) {
> > + case SYSRESET_WARM:
> > + case SYSRESET_COLD:
> > + psci_sys_reset(type);
> > + break;
> > + case SYSRESET_POWER_OFF:
> > + psci_sys_poweroff();
> > + break;
> > + case SYSRESET_EDL:
> > + psci_system_reset2(0, 1);
> > + break;
> > + default:
> > + return -EPROTONOSUPPORT;
> > + }
> > +
> > + return -EINPROGRESS;
> > +}
> > +
> > +static struct sysreset_ops qcom_psci_sysreset_ops = {
> > + .request = qcom_psci_sysreset_request,
> > + .get_status = qcom_psci_sysreset_get_status,
> > +};
> > +
> > +U_BOOT_DRIVER(qcom_psci_sysreset) = {
> > + .name = "qcom_psci-sysreset",
> > + .id = UCLASS_SYSRESET,
> > + .ops = &qcom_psci_sysreset_ops,
> > + .flags = DM_FLAG_PRE_RELOC,
> > +};
> > diff --git a/include/sysreset.h b/include/sysreset.h
> > index ff20abdeed3..8bda9703cd9 100644
> > --- a/include/sysreset.h
> > +++ b/include/sysreset.h
> > @@ -21,6 +21,8 @@ enum sysreset_t {
> > SYSRESET_POWER,
> > / @SYSRESET_POWER_OFF: turn off power /
> > SYSRESET_POWER_OFF,
> > + / @SYSRESET_EDL: reset and boot into Emergency DownLoader /
> > + SYSRESET_EDL,
> > / @SYSRESET_COUNT: number of available reset types */
> > SYSRESET_COUNT,
> > };
> 
> --
> Casey (she/they)

Re: [PATCH v1 2/3] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs

[Varadarajan Narayanan]

On Mon, Apr 28, 2025 at 04:22:47PM +0200, Casey Connolly wrote:
>
>
> On 4/10/25 14:02, Varadarajan Narayanan wrote:
> > Add SYSRESET_EDL to sysreset_t and handle the different SYSRESET_xxx
> > requests in sysreset_qcom-psci.c.
>
> To be honest, I'm not really excited about this. Copying the entire PSCI
> reset code and polluting the global sysreset reason enum with entirely
> platform specific symbols is not good practise.
>
> Additionally, while this specific EDL implementation works on your platform,
> it does not work on (for example) sdm845 and sm8250 which are platforms we
> still very much care about. I'm also not sure if it works on SC7280.
>
> I think this should be handled in an/the SCM driver like it is in Linux, not
> sure if Sam is working on that or if I can send a barebones version of the
> one I have in my tree. It will need to be based on the compatible string for
> the scm node in DT (and checking for the qcom,dload-mode property too I
> think).
>
> As I proposed before on IRC, I think it makes sense to pass the "reset"
> arguments into sysreset and allow the sysreset driver to implement a
> callback/handler for them, this way the "edl" argument makes it all the way
> to our platform-specific driver which can then call "psci_system_reset2()"
>
> I would implement this by adding a new op to sysreset_ops, something like
> .request_args, then have a second for loop which calls this, since we want
> any reset handler which /can/ parse the args to do so before we call the
> normal .request callback (otherwise some other handler might do a normal
> reset before we have parsed the arguments).
>
> For now I'm fine with this new sysreset-qcom.c driver being specific to
> qcs9100, but please include a comment that the psci reset2 call is only
> supported on some newer qualcomm platforms and that others can be supported
> via SCM.
>
> On that note, how does this new driver actually get bound? It doesn't have
> any compatible strings associated? Or am I missing something here... Please
> explain this.
>
> Otherwise, I'm looking forward to v2.

Sumit & Casey,

Have posted V2 addressing the comments. Please take a look.

Thanks
Varada

[ . . . ]

[PATCH v1 3/3] configs: qcs9100_defconfig: Enable SYSRESET

[Varadarajan Narayanan]

Move to SYSRESET for implementing the reset command.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 configs/qcs9100_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/qcs9100_defconfig b/configs/qcs9100_defconfig
index 10ff4d25398..1fb621b27ce 100644
--- a/configs/qcs9100_defconfig
+++ b/configs/qcs9100_defconfig
@@ -16,3 +16,5 @@ CONFIG_TEXT_BASE=0xaf000000
 CONFIG_REMAKE_ELF=y
 
 CONFIG_DEFAULT_DEVICE_TREE="qcom/qcs9100-ride-r3"
+CONFIG_SYSRESET=y
+CONFIG_SYSRESET_QCOM_PSCI=y
-- 
2.34.1

Re: [PATCH v1 3/3] configs: qcs9100_defconfig: Enable SYSRESET

[Sumit Garg]

On Thu, Apr 10, 2025 at 05:32:08PM +0530, Varadarajan Narayanan wrote:
> Move to SYSRESET for implementing the reset command.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  configs/qcs9100_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>

-Sumit

> 
> diff --git a/configs/qcs9100_defconfig b/configs/qcs9100_defconfig
> index 10ff4d25398..1fb621b27ce 100644
> --- a/configs/qcs9100_defconfig
> +++ b/configs/qcs9100_defconfig
> @@ -16,3 +16,5 @@ CONFIG_TEXT_BASE=0xaf000000
>  CONFIG_REMAKE_ELF=y
>  
>  CONFIG_DEFAULT_DEVICE_TREE="qcom/qcs9100-ride-r3"
> +CONFIG_SYSRESET=y
> +CONFIG_SYSRESET_QCOM_PSCI=y
> -- 
> 2.34.1
>