Re: [PATCH] rockchip: boot_mode: fix download key detection

[Tianling Shen <cnsztl@gmail.com>]

Hi Jonas,

On 2026/6/10 21:25, Jonas Karlman wrote:
> Hi Tianling,
> 
> On 6/10/2026 5:01 AM, Tianling Shen wrote:
>> rockchip_dnl_key_pressed() looks for the ADC device by checking
>> whether the device name starts with "saradc".
>>
>> On RK3328, RK3576, RK3588 etc., the SARADC node is named "adc@...",
>> so the device name no longer has the "saradc" prefix. As a result,
>> U-Boot fails to find the SARADC device and does not sample the
>> download key state.
>>
>> Do not rely on the DT node name. Match the bound Rockchip SARADC
>> driver instead, which works for both "saradc@..." and "adc@..."
>> node names.
>>
>> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
>> ---
>>   arch/arm/mach-rockchip/boot_mode.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
>> index 55e9456668ae..363fad523cbd 100644
>> --- a/arch/arm/mach-rockchip/boot_mode.c
>> +++ b/arch/arm/mach-rockchip/boot_mode.c
>> @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
>>   
>>   	ret = -ENODEV;
>>   	uclass_foreach_dev(dev, uc) {
>> -		if (!strncmp(dev->name, "saradc", 6)) {
>> +		if (!strcmp(dev->driver->name, "rockchip_saradc")) {
> 
> The adc channel used below may not fit all boards/SoCs, so this change
> may have unintended consequences.
> 
> I have previously avoided "fixing"/enable this code path for RK35xx
> because the download key selection should really be improved when this
> is fixed/expanded to more boards/SoCs.
> 
> E.g. maybe add support to declare a recovery button in u-boot,config
> node and use UCLASS_BUTTON to check if such button was pressed or
> something similar?

Yes this is much better than evaluating raw ADC values, but BUTTON is 
not available in SPL so we cannot have this functionality in SPL (though 
this idea was already rejected :P).

I have made an initial version that use UCLASS_BUTTON:

```
__weak int rockchip_dnl_key_pressed(void)
{
#if CONFIG_IS_ENABLED(BUTTON)
	const char *path;
	struct udevice *button;
	ofnode node;
	int ret;

	path = ofnode_options_read_str("recovery-key");
	if (!path)
		path = ofnode_conf_read_str("u-boot,recovery-key");
	if (!path)
		return false;

	if (path[0] == '/')
		node = ofnode_path(path);
	else
		node = ofnode_get_aliases_node(path);

	if (!ofnode_valid(node)) {
		pr_err("%s: invalid recovery key '%s'\n", __func__, path);
		return false;
	}

	ret = uclass_get_device_by_ofnode(UCLASS_BUTTON, node, &button);
	if (ret) {
		pr_err("%s: recovery key is not a button: %d\n", __func__, ret);
		return false;
	}

	ret = button_get_state(button);
	if (ret < 0) {
		pr_err("%s: failed to read recovery key: %d\n", __func__, ret);
		return false;
	}

	return ret == BUTTON_ON;
#else
	return false;
#endif
}
```

... so it can be configured in u-boot.dtsi like this:
```
/ {
	options {
		u-boot {
			recovery-key = "/keys-1/button-recovery";
		};
	};
};
```

I have tested this on my DshanPi A1 board and it seems to work okay.

Is this an acceptable way to configure recovery key? I'm also wondering 
if it's necessary to keep compatibility with old adc_channel_single_shot 
implementation.

Thanks,
Tianling.

> 
> Regards,
> Jonas
> 
>>   			ret = adc_channel_single_shot(dev->name, 1, &val);
>>   			break;
>>   		}
>