Hi Balaji,
On 2026-08-11T05:18:35, Balaji Selvanathan
<[email protected]> wrote:
> reboot-mode: add PSCI reboot-mode driver
>
> Add a PSCI backend for the reboot-mode framework that triggers a
> SYSTEM_RESET2 vendor-specific reset described by a "mode-*" property under
> a "reboot-mode" subnode of the PSCI device tree node (for example
> "mode-edl = <0x80000000 0x00000001>" to enter Qualcomm EDL/download mode).
>
> All device tree parsing and name matching already live in the reboot-mode
> uclass; this driver only turns a decoded set of magic cells into a
> firmware call.
>
> U-Boot's psci_system_reset2() client takes only a 32-bit cookie and ORs in
> PSCI_RESET2_TYPE_VENDOR itself, so the trigger folds the low cells into a
> cookie and rejects a 3-cell mode whose cookie_hi is non-zero rather than
> silently truncating it.
>
> psci_bind() binds the driver to the "reboot-mode" subnode when
> CONFIG_REBOOT_MODE_PSCI is enabled, using device_bind_driver_to_node().
>
> Signed-off-by: Balaji Selvanathan <[email protected]>
>
> drivers/firmware/psci.c | 16 ++++++++++
> drivers/reboot-mode/Kconfig | 11 +++++++
> drivers/reboot-mode/Makefile | 1 +
> drivers/reboot-mode/reboot-mode-psci.c | 58
> ++++++++++++++++++++++++++++++++++
> 4 files changed, 86 insertions(+)
> diff --git a/drivers/reboot-mode/Kconfig b/drivers/reboot-mode/Kconfig
> @@ -39,4 +39,15 @@ config REBOOT_MODE_NVMEM
> +config REBOOT_MODE_PSCI
> + bool "Use PSCI SYSTEM_RESET2 vendor resets as reboot modes"
> + depends on DM_REBOOT_MODE && ARM_SMCCC
psci_features() and psci_system_reset2() only exist for ARM64 -
fwcall.c lives under arch/arm/cpu/armv8, and the prototypes in
asm/system.h are inside the CONFIG_ARM64 block. ARM_SMCCC is available
on CPU_V7A too, so as written this can be enabled on a 32-bit ARM
build and then fail to link. Please add 'depends on ARM64' (or gate on
the appropriate ARMv8 PSCI symbol).
> diff --git a/drivers/reboot-mode/reboot-mode-psci.c
> b/drivers/reboot-mode/reboot-mode-psci.c
> @@ -0,0 +1,58 @@
> + u32 reset_type = magic[0];
> + u64 cookie = 0;
> + int i;
> +
> + if (count < 1 || count > REBOOT_MODE_MAX_MAGIC)
> + return -EINVAL;
> +
> + /*
> + * U-Boot's psci_system_reset2() takes only a 32-bit cookie, whereas
> + * the binding allows a 64-bit cookie in the 3-cell form (magic[1] is
> + * cookie_hi, magic[2] is cookie_lo). Reject a non-zero high half
> + * rather than silently truncating it.
> + */
> + if (count == REBOOT_MODE_MAX_MAGIC && magic[1]) {
> + dev_err(dev, "64-bit reset cookie is not supported\n");
> + return -EINVAL;
> + }
> +
> + for (i = 1; i < count; i++)
> + cookie = (cookie << 32) | magic[i];
Given that magic[1] is guaranteed to be zero in the 3-cell case and
the value is immediately truncated to u32 at the call site, the u64
shift/OR loop is misleading - it looks like it is assembling a real
64-bit cookie when it never can. Please make cookie a u32 and pick it
directly, e.g.
u32 cookie = 0;
if (count >= 2)
cookie = magic[count - 1];
That also removes the need for the loop variable and the cast on the call.
> diff --git a/drivers/reboot-mode/reboot-mode-psci.c
> b/drivers/reboot-mode/reboot-mode-psci.c
> @@ -0,0 +1,58 @@
> + /* Does not return on success. */
> + psci_system_reset2(reset_type, (u32)cookie);
> +
> + return -EINPROGRESS;
> +}
psci_system_reset2() is declared __noreturn in asm/system.h and ends
in a while(1), so the return statement is unreachable. I doubt you can
remove this 'return' though?
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> @@ -188,6 +188,22 @@ static int psci_bind(struct udevice *dev)
> + if (CONFIG_IS_ENABLED(REBOOT_MODE_PSCI)) {
> + ofnode rm_node = ofnode_find_subnode(dev_ofnode(dev),
> + "reboot-mode");
> +
> + if (ofnode_valid(rm_node) &&
> + device_bind_driver_to_node(dev, "reboot-mode-psci",
> + "reboot-mode-psci", rm_node,
> + NULL))
> + pr_warn("PSCI reboot-mode was not bound.\n");
> + }
Shouldn't we be checking errors here?
SYSTEM_RESET2 arrived in PSCI 1.1, but this bind runs for any PSCI
node (including the 0.2/1.0 block just above, which only binds the
sysreset driver conditionally). The runtime psci_features() check will
catch missing support, but consider gating the bind on 'arm,psci-1.1'
compat to avoid a warning on older PSCI DTs when the config happens to
be enabled. Worth a comment either way.
Regards,
Simon
