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

[Sumit Garg <sumit.garg@kernel.org>]

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
>