vhaudiquet/u-boot-krane
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

drivers: sysreset: revert support for args in request

Browse Source 
This reverts:
- commit e49c84f7bb ("doc: usage: cmd: reset: specify when the -edl
  option is available")
- commit 1076feb8a3 ("cmd: boot: fix edl being shown when not
  supported")
- commit 63c806ba0e ("qcom_defconfig: enable psci based sysreset")
- commit ef06c5d76f ("cmd: boot: Add '-edl' option to reset command
  documentation")
- commit 32825eaddc ("sysreset: Implement PSCI based reset to EDL mode
  for QCOM SoCs")
- commit fcb48b8981 ("drivers: sysreset: Add sysreset op that can take
  arguments")

There was a conflict reverting commit 63c806ba0e ("qcom_defconfig:
enable psci based sysreset") due to commit 02ef1859b4 ("configs:
Resync with savedefconfig"), but the conflict resolution was trivial.

The args support for the sysreset uclass contains a logic bug. The first
sysreset device implementing the request_arg callback will consume the
args, not support the specified arg and thus return -EPROTONOSUPPORT
which will stop the iteration over all sysreset devices.

This is an issue if one has multiple sysreset devices and each with
support for different (valid) args. If a sysreset device implements a
-dummy argument and another -foo and a user calls reset -dummy from the
U-Boot CLI, it'll depend on which sysreset device will be attempted
first. If it is the one implementing -foo, it'll return it doesn't
support the argument with -EPROTONOSUPPORT in which case the device
implementing -dummy will never be attempted and instead we'll do a cold
reset which is very likely not what's expected from the user.

Casey suggested[1] we revert this and start from scratch again with a
different implementation instead.

[1] https://lore.kernel.org/u-boot/77ff0f56-5c3b-42e7-bdd1-bf90296da900@linaro.org/

Acked-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
This commit is contained in:
Quentin Schulz
2026-07-03 13:45:58 -06:00
committed by Tom Rini
parent f605dcee10
commit c480bbd92b
9 changed files with 1 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/boot.c
-6
View File
@@ -60,12 +60,6 @@ U_BOOT_CMD(
reset, 2, 0, do_reset,
"Perform RESET of the CPU",
"- cold boot without level specifier\n"
#if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_ARGS)
// All options handled by sysreset drivers via their sysreset_ops.request_arg callback
#ifdef CONFIG_SYSRESET_QCOM_PSCI
"reset -edl - Boot to Emergency DownLoad mode\n"
#endif
#endif
"reset -w - warm reset if implemented"
);
configs/qcom_defconfig
-2
View File
@@ -133,8 +133,6 @@ CONFIG_QCOM_RPMH=y
CONFIG_SPMI_MSM=y
CONFIG_SYSINFO=y
CONFIG_SYSINFO_SMBIOS=y
CONFIG_SYSRESET_CMD_RESET_ARGS=y
CONFIG_SYSRESET_QCOM_PSCI=y
CONFIG_SYSRESET_QCOM_PSHOLD=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
doc/usage/cmd/reset.rst
+1 -9
View File
@@ -11,9 +11,7 @@ Synopsis
::
reset
reset -w
reset -edl
reset [-w]
Description
-----------
@@ -24,12 +22,6 @@ DDR and peripherals, on some boards also resets external PMIC.
-w
Do WARM reset: reset CPU but keep peripheral/DDR/PMIC active.
All other options require CONFIG_SYSRESET_CMD_RESET_ARGS=y.
-edl
Boot to Emergency DownLoad mode on supported Qualcomm platforms. Unsupported
platforms will print an error message but the command will successfully
return (having done nothing). Requires CONFIG_SYSRESET_QCOM_PSCI=y.
Return value
------------
drivers/firmware/psci.c
-4
View File
@@ -186,10 +186,6 @@ 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 */
drivers/sysreset/Kconfig
-15
View File
@@ -49,14 +49,6 @@ config SYSRESET_CMD_RESET
help
Enable sysreset implementation of the reset command.
config SYSRESET_CMD_RESET_ARGS
bool "Enable reset command to take arguments"
help
Pass on the arguments received by the 'reset' command to the
sysreset driver(s). The sysreset driver(s) may make use of the
additional arguments for implementing arch/board specific
functionality.
if CMD_POWEROFF
config SYSRESET_CMD_POWEROFF
@@ -301,13 +293,6 @@ 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"
depends on ARM_SMCCC
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
drivers/sysreset/Makefile
-1
View File
@@ -30,7 +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
obj-$(CONFIG_SYSRESET_QEMU_VIRT_CTRL) += sysreset_qemu_virt_ctrl.o
drivers/sysreset/sysreset-uclass.c
-37
View File
@@ -32,18 +32,6 @@ int sysreset_request(struct udevice *dev, enum sysreset_t type)
return ops->request(dev, type);
}
#if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_ARGS)
int sysreset_request_arg(struct udevice *dev, int argc, char * const argv[])
{
struct sysreset_ops *ops = sysreset_get_ops(dev);
if (!ops->request_arg)
return -ENOSYS;
return ops->request_arg(dev, argc, argv);
}
#endif /* CONFIG_SYSRESET_CMD_RESET_ARGS */
int sysreset_get_status(struct udevice *dev, char *buf, int size)
{
struct sysreset_ops *ops = sysreset_get_ops(dev);
@@ -83,26 +71,6 @@ int sysreset_walk(enum sysreset_t type)
return ret;
}
#if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_ARGS)
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;
}
#endif /* CONFIG_SYSRESET_CMD_RESET_ARGS */
int sysreset_get_last_walk(void)
{
struct udevice *dev;
@@ -164,11 +132,6 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
printf("resetting ...\n");
mdelay(100);
#if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_ARGS)
if (argc > 1 && sysreset_walk_arg(argc, argv) == -EINPROGRESS)
return 0;
#endif
sysreset_walk_halt(reset_type);
return 0;
drivers/sysreset/sysreset_qcom-psci.c
-45
View File
@@ -1,45 +0,0 @@
// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright (C) 2017 Masahiro Yamada <yamada.masahiro@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;
}
static struct sysreset_ops qcom_psci_sysreset_ops = {
.request_arg = qcom_psci_sysreset_request_arg,
.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,
};
include/sysreset.h
-18
View File
@@ -43,24 +43,6 @@ struct sysreset_ops {
* (in which case this method will not actually return)
*/
int (*request)(struct udevice *dev, enum sysreset_t type);
/**
* @request_arg: Reset handler implementations that might need to process
* arguments given to the 'reset' command.
*
* Note that this function may return before the reset takes effect.
*
* @dev: Device to be used for system reset
* @argc: No. of items in @argv
* @argv: Arguments given to 'reset' command
* Return:
* -EINPROGRESS if the reset has started and will complete soon
* -EPROTONOSUPPORT if not supported by this device
* 0 if the reset has already happened
* (in which case this method will not actually return)
*/
int (*request_arg)(struct udevice *dev, int argc, char * const argv[]);
/**
* @get_status: get printable reset status information
*