[PATCH v6 3/5] sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs
Tom Rini
trini at konsulko.com
Tue Jun 23 18:50:21 CEST 2026
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20260623/0e09ea83/attachment.sig>
More information about the U-Boot
mailing list