[PATCH 0/6] drivers: sysreset: revert support for args

[Quentin Schulz]

Hi Casey,

Thanks for the prompt answer!

On 7/3/26 2:27 PM, Casey Connolly wrote:
> Hi Quentin,
> 
> On 7/2/26 20:27, Quentin Schulz 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'm so sorry I missed this feedback. That really isn't great.
> 

It happens, I'm glad you see it now :)

> I'm with you that this is a pretty fundamental issue with the 
> implementation here and I think the best course of action is to revert 
> it and come up with a more suitable approach that isn't so open to 
> trivial logic bugs like this, likely with some deeper refactoring of 
> sysreset/reboot-mode code in U-Boot.
> 
> In hindsight I think I should have pushed back harder on this 
> implementation, this is something I'll be more cautious of in the future.
> 

Just to be clear I wasn't trying to shame anyone here, contributor, 
reviewer or maintainer. Mistakes happen.

> Acked-by: Casey Connolly <casey.connolly at linaro.org>
> 
> I don't have a chance today to test this today as I'm OOO but if it 
> doesn't break other platforms then it ought to be fine on Qualcomm too, 
> maybe Neil can give it a spin on 8650.
> 
> Tom: feel free to pick this up if you're happy with it.
> 

Varada sent a series trying to fix the logic, c.f. 
https://lore.kernel.org/u-boot/d7e003b3-e693-4579-8e3a-b09ce9f78d10@cherry.de/T/#mf098752c30cd6a6c8e7700f61cd54cbc8f307d6d. 
But if you feel like we should start from a clean slate again instead 
then we can revert this now and take the time to implement this 
differently for next release. I think your feedback on what to do next 
is important here. But I appreciate you're OOO so we'll do without if 
you don't have time :)

Thanks,
Quentin