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 one calls reset -dummy on 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. I suggest to either add an additional return code for the case where the sysreset device knows it isn't supposed to handle this argument because it isn't among the ones it supports, and thus we try other sysreset devices until we find one that says it tried and either is in progress or couldn't realize the request, or no sysreset device supports this argument and we end up doing a cold reset (but should we even try to do a cold reset if the type of reset requested by the user isn't supported?). Or just don't try piggy-backing on the reset command and have a new command instead? Maybe I misunderstood something or misread the code, so please correct me if I'm wrong. We've tried bringing up this issue multiple times over the last month[1][2][3] but it seems like we're left on read. This is the last resort to get attention on this patches, hopefully we don't have to get through a revert a few days before the release though there is only two working days left before the release. Sending a revert as requested by Tom[4]. I've not even build tested this, only had a cursory look at what to revert so maybe I'm missing some patches or breaking stuff up. It's late here and I wanted to give a chance to people in Americas to have a look at this today, or people in Asia tomorrow morning before I come to the office. I don't have time to wait for the CI to complete, but it's running here: https://source.denx.de/u-boot/contributors/qschulz/u-boot/-/pipelines/30604 Yes the commit hash is different, I rewrote this cover letter to include the link to the CI pipeline. [1] https://lore.kernel.org/all/[email protected]/ [2] https://lore.kernel.org/all/[email protected]/ [3] https://lore.kernel.org/all/20260623165021.GJ382693@bill-the-cat/ [4] https://lore.kernel.org/all/20260702174613.GC749385@bill-the-cat/ Signed-off-by: Quentin Schulz <[email protected]> --- Quentin Schulz (6): Revert "qcom_defconfig: enable psci based sysreset" Revert "cmd: boot: fix edl being shown when not supported" Revert "doc: usage: cmd: reset: specify when the -edl option is available" Revert "cmd: boot: Add '-edl' option to reset command documentation" Revert "sysreset: Implement PSCI based reset to EDL mode for QCOM SoCs" Revert "drivers: sysreset: Add sysreset op that can take arguments" cmd/boot.c | 6 ----- configs/qcom_defconfig | 2 -- doc/usage/cmd/reset.rst | 10 +------- drivers/firmware/psci.c | 4 ---- drivers/sysreset/Kconfig | 15 ------------ drivers/sysreset/Makefile | 1 - drivers/sysreset/sysreset-uclass.c | 37 ---------------------------- drivers/sysreset/sysreset_qcom-psci.c | 45 ----------------------------------- include/sysreset.h | 18 -------------- 9 files changed, 1 insertion(+), 137 deletions(-) --- base-commit: f605dcee103c897b6f1a8873549a36949bd4e2a1 change-id: 20260702-revert-sysreset-4579ae458f2e Best regards, -- Quentin Schulz <[email protected]>
