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

[Quentin Schulz]

On 6/23/26 6:50 PM, Tom Rini wrote:
> On Mon, Jun 08, 2026 at 04:53:23PM +0200, Quentin Schulz wrote:
>> Hi all,
>>
>> Gentle ping. It'd be nice if we can fix this new API before we have a
>> release made with it?
>>
>> On 5/26/26 3:32 PM, Quentin Schulz wrote:
>>> 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?
> 
> Following up here.
> 

Do we end up reverting just before the release then?