Hi Quentin, On Thu, 2 Jul 2026 at 19:27, Quentin Schulz <[email protected]> wrote: > > 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
Given the timing, I suppose a revert. Apart from what is mentioned here, passing argc/argv directly from a command to a driver seems wrong to me, so I think the new series should use a new reset type instead. Regards, SImon > > Best regards, > -- > Quentin Schulz <[email protected]> >
