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

[Quentin Schulz]

Hi Varadarajan, Casey,

On 1/21/26 7:39 AM, Varadarajan Narayanan wrote:
> Implement request_arg() sysreset_op for QCOM SoCs that use
> PSCI to reset to EDL (Emergency Download) mode.
> 
> Reviewed-by: Casey Connolly <casey.connolly at linaro.org>
> Reviewed-by: Sumit Garg <sumit.garg at oss.qualcomm.com>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan at oss.qualcomm.com>
> ---
> v4: * Check if ARM_PSCI_1_1_FN64_SYSTEM_RESET2 is supported before
>        issuing it
> 
> v3: * Move argument handling to a separate function and pass the
>        arguments to the actual handler to process
>      * Drop Qcom specific SYSRESET_EDL from the common enum
> 
> v2: * Update commit message
>      * Elaborate Kconfig help text
>      * Use '-edl' instead of 'edl' for consistency with other arguments of reset
>        command
>      * Remove 'weak' for qcom_psci_sysreset_get_status() and make it static
>      * Mention 'SYSRESET_EDL' is Qcom specific in the enum's comments
> ---
>   drivers/firmware/psci.c               |  4 +++
>   drivers/sysreset/Kconfig              |  6 ++++
>   drivers/sysreset/Makefile             |  1 +
>   drivers/sysreset/sysreset_qcom-psci.c | 45 +++++++++++++++++++++++++++
>   4 files changed, 56 insertions(+)
>   create mode 100644 drivers/sysreset/sysreset_qcom-psci.c
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index 2e3223e1c32..b6838a244d2 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -186,6 +186,10 @@ static int psci_bind(struct udevice *dev)
>   					 NULL);
>   		if (ret)
>   			pr_debug("PSCI System Reset was not bound.\n");
> +		if (IS_ENABLED(CONFIG_SYSRESET_QCOM_PSCI) &&
> +		    device_bind_driver(dev, "qcom_psci-sysreset",
> +				       "qcom_psci-sysreset", NULL))
> +			pr_debug("QCOM PSCI System Reset was not bound.\n");
>   	}
>   
>   	/* From PSCI v1.0 onward we can discover services through ARM_SMCCC_FEATURE */
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 6fb0ca81dc6..905ad2a7769 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -301,6 +301,12 @@ config SYSRESET_RAA215300
>   	help
>   	  Add support for the system reboot via the Renesas RAA215300 PMIC.
>   
> +config SYSRESET_QCOM_PSCI
> +	bool "Support reset to EDL for Qualcomm SoCs via PSCI"
> +	help
> +	  Add support for the reset to EDL (Emergency Download) on Qualcomm
> +	  SoCs via PSCI.
> +
>   config SYSRESET_QCOM_PSHOLD
>   	bool "Support sysreset for Qualcomm SoCs via PSHOLD"
>   	help
> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> index f5c78b25896..8bb1eabd48e 100644
> --- a/drivers/sysreset/Makefile
> +++ b/drivers/sysreset/Makefile
> @@ -30,5 +30,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_qcom-psci.c b/drivers/sysreset/sysreset_qcom-psci.c
> new file mode 100644
> index 00000000000..3627bbf5c82
> --- /dev/null
> +++ b/drivers/sysreset/sysreset_qcom-psci.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2017 Masahiro Yamada <yamada.masahiro at socionext.com>
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <dm.h>
> +#include <sysreset.h>
> +#include <asm/system.h>
> +#include <linux/errno.h>
> +#include <linux/psci.h>
> +#include <asm/psci.h>
> +
> +static int qcom_psci_sysreset_get_status(struct udevice *dev, char *buf, int size)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int qcom_psci_sysreset_request_arg(struct udevice *dev, int argc,
> +					  char * const argv[])
> +{
> +	if (!strncasecmp(argv[1], "-edl", 4)) {
> +		/* Supported in qcs9100, qcs8300, sc7280, qcs615 */
> +		if (psci_features(ARM_PSCI_1_1_FN64_SYSTEM_RESET2) ==
> +							ARM_PSCI_RET_SUCCESS) {
> +			psci_system_reset2(0, 1);
> +			return -EINPROGRESS;
> +		}
> +		printf("PSCI SYSTEM_RESET2 not supported\n");
> +	}
> +
> +	return -EPROTONOSUPPORT;

I think this is a logic bug.

For reference, what calls this function:

   int sysreset_walk_arg(int argc, char * const argv[])
   {
           struct udevice *dev;
           int ret = -ENOSYS;

           while (ret != -EINPROGRESS && ret != -EPROTONOSUPPORT) {
                   for (uclass_first_device(UCLASS_SYSRESET, &dev);
                        dev;
                        uclass_next_device(&dev)) {
                           ret = sysreset_request_arg(dev, argc, argv);
                           if (ret == -EINPROGRESS || ret == 
-EPROTONOSUPPORT)
                                   break;
                   }
           }

           return ret;
   }

The issue is that the first sysreset device implementing the request_arg 
callback will consume the args and return EPROTONOSUPPORT which will 
stop the loop.

Think about multiple sysreset devices, and let's say we have a new 
'-dummy' argument to the reset CLI command, then it'll depend on the 
registration order of the sysreset driver whether the dummy argument 
will be handled by the qcom driver which will return -
EPROTONOSUPPORT since it isn't '-edl', or by the dummy sysreset driver.

I think we should have a fourth possible return code which is "I don't 
know how to handle this argument", e.g. à-la IRQ_NONE in Linux kernel 
for interrupts that aren't for the device this IRQ handler was triggered 
for. Or maybe we should really be using -EPROTONOSUPPORT for that 
meaning and not exit the loop if that's the return code?

Or maybe I misread or misunderstood the code?

Cheers,
Quentin