SoC vendors have different types of resets which are controlled
through various hardware registers. For instance, Qualcomm SoC
may have a requirement that reboot with “bootloader” command
should reboot the device to bootloader flashing mode and reboot
with “edl” should reboot the device into Emergency flashing mode.
Setting up such reboots on Qualcomm devices can be inconsistent
across SoC platforms and may require setting different HW
registers, where some of these registers may not be accessible to
HLOS. These knobs evolve over product generations and require
more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
reset which can help align this requirement. Add support for PSCI
SYSTEM_RESET2, vendor-specific resets and align the implementation
to allow user-space initiated reboots to trigger these resets.

Implement the PSCI vendor-specific resets by registering to the
reboot-mode framework. As psci init is done at early kernel init,
reboot-mode registration cannot be done at the time of psci init.
This is because reboot-mode creates a “reboot-mode” class for
exposing sysfs, which can fail at early kernel init. To overcome
this, introduce a late_initcall to register PSCI vendor-specific
resets as reboot modes. Implement a reboot-mode write function
that sets reset_type and cookie values during the reboot notifier
callback.  Introduce a firmware-based call for SYSTEM_RESET2
vendor-specific reset in the psci_sys_reset path, using
reset_type and cookie if supported by secure firmware. Register a
panic notifier and clear vendor_reset valid status during panic.
This is needed for any kernel panic that occurs post
reboot_notifiers.

By using the above implementation, userspace will be able to issue
such resets using the reboot() system call with the "*arg"
parameter as a string based command. The commands can be defined
in PSCI device tree node under “reboot-mode” and are based on the
reboot-mode based commands.

Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
Reviewed-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/firmware/psci/Kconfig |  2 +
 drivers/firmware/psci/psci.c  | 92 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/psci/Kconfig b/drivers/firmware/psci/Kconfig
index 97944168b5e66aea1e38a7eb2d4ced8348fce64b..93ff7b071a0c364a376699733e6bc5654d56a17f 100644
--- a/drivers/firmware/psci/Kconfig
+++ b/drivers/firmware/psci/Kconfig
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config ARM_PSCI_FW
 	bool
+	select POWER_RESET
+	select REBOOT_MODE
 
 config ARM_PSCI_CHECKER
 	bool "ARM PSCI checker"
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..ff82e7f4c27d1609a75cedc3a9790affaf839801 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -8,15 +8,18 @@
 
 #include <linux/acpi.h>
 #include <linux/arm-smccc.h>
+#include <linux/bitops.h>
 #include <linux/cpuidle.h>
 #include <linux/debugfs.h>
 #include <linux/errno.h>
 #include <linux/linkage.h>
 #include <linux/of.h>
+#include <linux/panic_notifier.h>
 #include <linux/pm.h>
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/reboot-mode.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
 
@@ -51,6 +54,24 @@ static int resident_cpu = -1;
 struct psci_operations psci_ops;
 static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
 
+struct psci_vendor_sysreset2 {
+	u32 reset_type;
+	u32 cookie;
+	bool valid;
+};
+
+static struct psci_vendor_sysreset2 vendor_reset;
+
+static int psci_panic_event(struct notifier_block *nb, unsigned long v, void *p)
+{
+	vendor_reset.valid = false;
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block psci_panic_block = {
+	.notifier_call = psci_panic_event
+};
+
 bool psci_tos_resident_on(int cpu)
 {
 	return cpu == resident_cpu;
@@ -309,7 +330,10 @@ static int get_set_conduit_method(const struct device_node *np)
 static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
 			  void *data)
 {
-	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
+	if (vendor_reset.valid && psci_system_reset2_supported) {
+		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
+			       vendor_reset.cookie, 0);
+	} else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
 	    psci_system_reset2_supported) {
 		/*
 		 * reset_type[31] = 0 (architectural)
@@ -547,6 +571,72 @@ static const struct platform_suspend_ops psci_suspend_ops = {
 	.enter          = psci_system_suspend_enter,
 };
 
+static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
+{
+	u32 magic_32;
+
+	if (psci_system_reset2_supported) {
+		magic_32 = magic & GENMASK(31, 0);
+		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
+		vendor_reset.cookie = (magic >> 32) & GENMASK(31, 0);
+		vendor_reset.valid = true;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static int __init psci_init_vendor_reset(void)
+{
+	struct reboot_mode_driver *reboot;
+	struct device_node *psci_np;
+	struct device_node *np;
+	int ret;
+
+	if (!psci_system_reset2_supported)
+		return -EINVAL;
+
+	psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
+	if (!psci_np)
+		return -ENODEV;
+
+	np = of_find_node_by_name(psci_np, "reboot-mode");
+	if (!np) {
+		of_node_put(psci_np);
+		return -ENODEV;
+	}
+
+	ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
+	if (ret)
+		goto err_notifier;
+
+	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
+	if (!reboot) {
+		ret = -ENOMEM;
+		goto err_kzalloc;
+	}
+
+	reboot->write = psci_set_vendor_sys_reset2;
+	reboot->driver_name = "psci";
+
+	ret = reboot_mode_register(reboot, of_fwnode_handle(np));
+	if (ret)
+		goto err_register;
+
+	of_node_put(psci_np);
+	of_node_put(np);
+	return 0;
+
+err_register:
+	kfree(reboot);
+err_kzalloc:
+	atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
+err_notifier:
+	of_node_put(psci_np);
+	of_node_put(np);
+	return ret;
+}
+late_initcall(psci_init_vendor_reset)
+
 static void __init psci_init_system_reset2(void)
 {
 	int ret;

-- 
2.34.1

On Sun, Nov 09, 2025 at 08:07:20PM +0530, Shivendra Pratap wrote:
> SoC vendors have different types of resets which are controlled
> through various hardware registers. For instance, Qualcomm SoC
> may have a requirement that reboot with “bootloader” command
> should reboot the device to bootloader flashing mode and reboot
> with “edl” should reboot the device into Emergency flashing mode.
> Setting up such reboots on Qualcomm devices can be inconsistent
> across SoC platforms and may require setting different HW
> registers, where some of these registers may not be accessible to
> HLOS. These knobs evolve over product generations and require
> more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
> reset which can help align this requirement. Add support for PSCI
> SYSTEM_RESET2, vendor-specific resets and align the implementation
> to allow user-space initiated reboots to trigger these resets.
> 
> Implement the PSCI vendor-specific resets by registering to the
> reboot-mode framework.

I think that we should expose to user space _all_ PSCI reset types,
cold, warm + vendor specific - as a departure from using the reboot_mode
variable (and possibly deprecate it - or at least stop using it).

> As psci init is done at early kernel init, reboot-mode registration cannot
> be done at the time of psci init.  This is because reboot-mode creates a
> “reboot-mode” class for exposing sysfs, which can fail at early kernel init.
> To overcome this, introduce a late_initcall to register PSCI vendor-specific
> resets as reboot modes. Implement a reboot-mode write function that sets
> reset_type and cookie values during the reboot notifier callback.  Introduce
> a firmware-based call for SYSTEM_RESET2 vendor-specific reset in the
> psci_sys_reset path, using reset_type and cookie if supported by secure
> firmware. Register a panic notifier and clear vendor_reset valid status
> during panic.  This is needed for any kernel panic that occurs post
> reboot_notifiers.

Is it because panic uses reboot_mode to determine the reset to issue ?

> By using the above implementation, userspace will be able to issue
> such resets using the reboot() system call with the "*arg"
> parameter as a string based command. The commands can be defined
> in PSCI device tree node under “reboot-mode” and are based on the
> reboot-mode based commands.

IMHO - it would be nice if could add mode-cold (or mode-normal in reboot mode
speak) and mode-warm by default (if PSCI supports them) so that userspace
could issue those resets too without having to set the reboot_mode variable.

Reason is, since we are doing this it is worth going the whole nine
yards and try to decouple the reboot_mode variable from the RESTART2
syscall argument.

Reworded: just use the new userspace interface you are adding for
all PSCI reset types.

Thoughts very much welcome - I understand this is controversial.

> Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
> Reviewed-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/firmware/psci/Kconfig |  2 +
>  drivers/firmware/psci/psci.c  | 92 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/psci/Kconfig b/drivers/firmware/psci/Kconfig
> index 97944168b5e66aea1e38a7eb2d4ced8348fce64b..93ff7b071a0c364a376699733e6bc5654d56a17f 100644
> --- a/drivers/firmware/psci/Kconfig
> +++ b/drivers/firmware/psci/Kconfig
> @@ -1,6 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config ARM_PSCI_FW
>  	bool
> +	select POWER_RESET
> +	select REBOOT_MODE
>  
>  config ARM_PSCI_CHECKER
>  	bool "ARM PSCI checker"
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..ff82e7f4c27d1609a75cedc3a9790affaf839801 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -8,15 +8,18 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/arm-smccc.h>
> +#include <linux/bitops.h>
>  #include <linux/cpuidle.h>
>  #include <linux/debugfs.h>
>  #include <linux/errno.h>
>  #include <linux/linkage.h>
>  #include <linux/of.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/pm.h>
>  #include <linux/printk.h>
>  #include <linux/psci.h>
>  #include <linux/reboot.h>
> +#include <linux/reboot-mode.h>
>  #include <linux/slab.h>
>  #include <linux/suspend.h>
>  
> @@ -51,6 +54,24 @@ static int resident_cpu = -1;
>  struct psci_operations psci_ops;
>  static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
>  
> +struct psci_vendor_sysreset2 {
> +	u32 reset_type;
> +	u32 cookie;
> +	bool valid;
> +};
> +
> +static struct psci_vendor_sysreset2 vendor_reset;

I think this should represent all possible PSCI reset types, not vendor only
and its value is set by the reboot mode framework.

> +
> +static int psci_panic_event(struct notifier_block *nb, unsigned long v, void *p)
> +{
> +	vendor_reset.valid = false;

I don't like this. Basically all you want this for is to make sure that
we don't override the reboot_mode variable.

One (hack) would consist in checking the reboot_mode variable here and
set the struct I mentioned above to the value represented in reboot_mode.

Good luck if reboot_mode == REBOOT_GPIO :-)

> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block psci_panic_block = {
> +	.notifier_call = psci_panic_event
> +};
> +
>  bool psci_tos_resident_on(int cpu)
>  {
>  	return cpu == resident_cpu;
> @@ -309,7 +330,10 @@ static int get_set_conduit_method(const struct device_node *np)
>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>  			  void *data)
>  {
> -	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> +	if (vendor_reset.valid && psci_system_reset2_supported) {
> +		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
> +			       vendor_reset.cookie, 0);

See above. Two calls here: one for resets issued using the new userspace
interface you are adding and legacy below - no vendor vs reboot_mode, this
is a mess.

> +	} else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>  	    psci_system_reset2_supported) {
>  		/*
>  		 * reset_type[31] = 0 (architectural)
> @@ -547,6 +571,72 @@ static const struct platform_suspend_ops psci_suspend_ops = {
>  	.enter          = psci_system_suspend_enter,
>  };
>  
> +static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
> +{
> +	u32 magic_32;
> +
> +	if (psci_system_reset2_supported) {
> +		magic_32 = magic & GENMASK(31, 0);
> +		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
> +		vendor_reset.cookie = (magic >> 32) & GENMASK(31, 0);

Use FIELD_PREP/GET() please (but as mentioned above the vendor reset type
bit[31] should be part of the reboot mode magic value, see above).

> +		vendor_reset.valid = true;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int __init psci_init_vendor_reset(void)
> +{
> +	struct reboot_mode_driver *reboot;
> +	struct device_node *psci_np;
> +	struct device_node *np;
> +	int ret;
> +
> +	if (!psci_system_reset2_supported)
> +		return -EINVAL;
> +
> +	psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
> +	if (!psci_np)
> +		return -ENODEV;
> +
> +	np = of_find_node_by_name(psci_np, "reboot-mode");
> +	if (!np) {
> +		of_node_put(psci_np);
> +		return -ENODEV;
> +	}
> +
> +	ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
> +	if (ret)
> +		goto err_notifier;
> +
> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
> +	if (!reboot) {
> +		ret = -ENOMEM;
> +		goto err_kzalloc;
> +	}
> +
> +	reboot->write = psci_set_vendor_sys_reset2;
> +	reboot->driver_name = "psci";
> +
> +	ret = reboot_mode_register(reboot, of_fwnode_handle(np));
> +	if (ret)
> +		goto err_register;
> +
> +	of_node_put(psci_np);
> +	of_node_put(np);
> +	return 0;
> +
> +err_register:
> +	kfree(reboot);
> +err_kzalloc:
> +	atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
> +err_notifier:
> +	of_node_put(psci_np);
> +	of_node_put(np);
> +	return ret;
> +}
> +late_initcall(psci_init_vendor_reset)

I don't like adding another initcall here.

I wonder whether this code belongs in a PSCI reboot mode driver, possibly a
faux device in a way similar to what we did for cpuidle-psci (that after all
is a consumer of PSCI_CPU_SUSPEND in a similar way as this code is a
PSCI_SYSTEM_RESET{2} consumer), that communicates with
drivers/firmware/psci/psci.c with the struct mentioned above.

Thanks,
Lorenzo

> +
>  static void __init psci_init_system_reset2(void)
>  {
>  	int ret;
> 
> -- 
> 2.34.1
> 

On 11/10/2025 10:52 PM, Lorenzo Pieralisi wrote:
> On Sun, Nov 09, 2025 at 08:07:20PM +0530, Shivendra Pratap wrote:
>> SoC vendors have different types of resets which are controlled
>> through various hardware registers. For instance, Qualcomm SoC
>> may have a requirement that reboot with “bootloader” command
>> should reboot the device to bootloader flashing mode and reboot
>> with “edl” should reboot the device into Emergency flashing mode.
>> Setting up such reboots on Qualcomm devices can be inconsistent
>> across SoC platforms and may require setting different HW
>> registers, where some of these registers may not be accessible to
>> HLOS. These knobs evolve over product generations and require
>> more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
>> reset which can help align this requirement. Add support for PSCI
>> SYSTEM_RESET2, vendor-specific resets and align the implementation
>> to allow user-space initiated reboots to trigger these resets.
>>
>> Implement the PSCI vendor-specific resets by registering to the
>> reboot-mode framework.
> 
> I think that we should expose to user space _all_ PSCI reset types,
> cold, warm + vendor specific - as a departure from using the reboot_mode
> variable (and possibly deprecate it - or at least stop using it).

sure. We can try that. Have tried to compile it all at the end of this thread.

> 
>> As psci init is done at early kernel init, reboot-mode registration cannot
>> be done at the time of psci init.  This is because reboot-mode creates a
>> “reboot-mode” class for exposing sysfs, which can fail at early kernel init.
>> To overcome this, introduce a late_initcall to register PSCI vendor-specific
>> resets as reboot modes. Implement a reboot-mode write function that sets
>> reset_type and cookie values during the reboot notifier callback.  Introduce
>> a firmware-based call for SYSTEM_RESET2 vendor-specific reset in the
>> psci_sys_reset path, using reset_type and cookie if supported by secure
>> firmware. Register a panic notifier and clear vendor_reset valid status
>> during panic.  This is needed for any kernel panic that occurs post
>> reboot_notifiers.
> 
> Is it because panic uses reboot_mode to determine the reset to issue ?

Yes. As we know, currently psci supports only two resets,
psci_sys_reset2 (ARCH warm reset) and psci_sys_reset(COLD RESET). And kernel
panic path should take the path set by reboot_mode to maintain backward
compatibility. 

> 
>> By using the above implementation, userspace will be able to issue
>> such resets using the reboot() system call with the "*arg"
>> parameter as a string based command. The commands can be defined
>> in PSCI device tree node under “reboot-mode” and are based on the
>> reboot-mode based commands.
> 
> IMHO - it would be nice if could add mode-cold (or mode-normal in reboot mode
> speak) and mode-warm by default (if PSCI supports them) so that userspace

Default mode in current kernel is cold, until explicitly set to warm.
So should it be defaulted to cold?

> could issue those resets too without having to set the reboot_mode variable.
> 
> Reason is, since we are doing this it is worth going the whole nine
> yards and try to decouple the reboot_mode variable from the RESTART2
> syscall argument.
> 
> Reworded: just use the new userspace interface you are adding for
> all PSCI reset types.
> 
> Thoughts very much welcome - I understand this is controversial.

We can remove the dependency on reboot_mode and include all supported
modes in a new psci_reset driver with some default modes (warm, cold)
and other vendor-specific resets which may be picked from device tree.
And then, reset based on the command being passed from userspace.

But yes some platforms that already reply on reboot_mode may break here
and will need to adjust to the new design being proposed.

Have summarized at the end of the thread.

> 
>> Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
>> Reviewed-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>> Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>>  drivers/firmware/psci/Kconfig |  2 +
>>  drivers/firmware/psci/psci.c  | 92 ++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 93 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/psci/Kconfig b/drivers/firmware/psci/Kconfig
>> index 97944168b5e66aea1e38a7eb2d4ced8348fce64b..93ff7b071a0c364a376699733e6bc5654d56a17f 100644
>> --- a/drivers/firmware/psci/Kconfig
>> +++ b/drivers/firmware/psci/Kconfig
>> @@ -1,6 +1,8 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  config ARM_PSCI_FW
>>  	bool
>> +	select POWER_RESET
>> +	select REBOOT_MODE
>>  
>>  config ARM_PSCI_CHECKER
>>  	bool "ARM PSCI checker"
>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>> index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..ff82e7f4c27d1609a75cedc3a9790affaf839801 100644
>> --- a/drivers/firmware/psci/psci.c
>> +++ b/drivers/firmware/psci/psci.c
>> @@ -8,15 +8,18 @@
>>  
>>  #include <linux/acpi.h>
>>  #include <linux/arm-smccc.h>
>> +#include <linux/bitops.h>
>>  #include <linux/cpuidle.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/errno.h>
>>  #include <linux/linkage.h>
>>  #include <linux/of.h>
>> +#include <linux/panic_notifier.h>
>>  #include <linux/pm.h>
>>  #include <linux/printk.h>
>>  #include <linux/psci.h>
>>  #include <linux/reboot.h>
>> +#include <linux/reboot-mode.h>
>>  #include <linux/slab.h>
>>  #include <linux/suspend.h>
>>  
>> @@ -51,6 +54,24 @@ static int resident_cpu = -1;
>>  struct psci_operations psci_ops;
>>  static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
>>  
>> +struct psci_vendor_sysreset2 {
>> +	u32 reset_type;
>> +	u32 cookie;
>> +	bool valid;
>> +};
>> +
>> +static struct psci_vendor_sysreset2 vendor_reset;
> 
> I think this should represent all possible PSCI reset types, not vendor only
> and its value is set by the reboot mode framework.
> 
>> +
>> +static int psci_panic_event(struct notifier_block *nb, unsigned long v, void *p)
>> +{
>> +	vendor_reset.valid = false;
> 
> I don't like this. Basically all you want this for is to make sure that
> we don't override the reboot_mode variable.

Yes, it does not look good but as we planned to use reboot-mode framework earlier, which
sets the modes at the at reboot_notifiers. This needs to be taken care for any panic
that occurs between reboot_notifier and restart_notifier.

> 
> One (hack) would consist in checking the reboot_mode variable here and
> set the struct I mentioned above to the value represented in reboot_mode.
> 
> Good luck if reboot_mode == REBOOT_GPIO :-)

psci supports only two modes, ARCH_WARM and cold, so anything else except WARM/SOFT
should default to cold? So even if REBOOT_GPIO is set in reboot_mode, we should default
it to cold reset.

> 
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block psci_panic_block = {
>> +	.notifier_call = psci_panic_event
>> +};
>> +
>>  bool psci_tos_resident_on(int cpu)
>>  {
>>  	return cpu == resident_cpu;
>> @@ -309,7 +330,10 @@ static int get_set_conduit_method(const struct device_node *np)
>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>  			  void *data)
>>  {
>> -	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>> +	if (vendor_reset.valid && psci_system_reset2_supported) {
>> +		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
>> +			       vendor_reset.cookie, 0);
> 
> See above. Two calls here: one for resets issued using the new userspace
> interface you are adding and legacy below - no vendor vs reboot_mode, this
> is a mess.

Are we suggesting to completely remove the reboot_mode check from here in the new
design and base it on reboot <CMD> param?

> 
>> +	} else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>  	    psci_system_reset2_supported) {
>>  		/*
>>  		 * reset_type[31] = 0 (architectural)
>> @@ -547,6 +571,72 @@ static const struct platform_suspend_ops psci_suspend_ops = {
>>  	.enter          = psci_system_suspend_enter,
>>  };
>>  
>> +static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
>> +{
>> +	u32 magic_32;
>> +
>> +	if (psci_system_reset2_supported) {
>> +		magic_32 = magic & GENMASK(31, 0);
>> +		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
>> +		vendor_reset.cookie = (magic >> 32) & GENMASK(31, 0);
> 
> Use FIELD_PREP/GET() please (but as mentioned above the vendor reset type
> bit[31] should be part of the reboot mode magic value, see above).

sure. Will align this. thanks.

> 
>> +		vendor_reset.valid = true;
>> +	}
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static int __init psci_init_vendor_reset(void)
>> +{
>> +	struct reboot_mode_driver *reboot;
>> +	struct device_node *psci_np;
>> +	struct device_node *np;
>> +	int ret;
>> +
>> +	if (!psci_system_reset2_supported)
>> +		return -EINVAL;
>> +
>> +	psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
>> +	if (!psci_np)
>> +		return -ENODEV;
>> +
>> +	np = of_find_node_by_name(psci_np, "reboot-mode");
>> +	if (!np) {
>> +		of_node_put(psci_np);
>> +		return -ENODEV;
>> +	}
>> +
>> +	ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
>> +	if (ret)
>> +		goto err_notifier;
>> +
>> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
>> +	if (!reboot) {
>> +		ret = -ENOMEM;
>> +		goto err_kzalloc;
>> +	}
>> +
>> +	reboot->write = psci_set_vendor_sys_reset2;
>> +	reboot->driver_name = "psci";
>> +
>> +	ret = reboot_mode_register(reboot, of_fwnode_handle(np));
>> +	if (ret)
>> +		goto err_register;
>> +
>> +	of_node_put(psci_np);
>> +	of_node_put(np);
>> +	return 0;
>> +
>> +err_register:
>> +	kfree(reboot);
>> +err_kzalloc:
>> +	atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
>> +err_notifier:
>> +	of_node_put(psci_np);
>> +	of_node_put(np);
>> +	return ret;
>> +}
>> +late_initcall(psci_init_vendor_reset)
> 
> I don't like adding another initcall here.
> 
> I wonder whether this code belongs in a PSCI reboot mode driver, possibly a
> faux device in a way similar to what we did for cpuidle-psci (that after all
> is a consumer of PSCI_CPU_SUSPEND in a similar way as this code is a
> PSCI_SYSTEM_RESET{2} consumer), that communicates with
> drivers/firmware/psci/psci.c with the struct mentioned above.

sure. we can create a new driver and try it as in cpuidle: cpuidle-psci.
Can you suggest a bit more on the overall approach we want to take here?
Have tried to summarize the potential changes and few questions below.

- new driver registers a faux device - say - power: reset: psci_reset.
- struct with pre-built psci reset_types - (warm, soft, cold). Currently
  only two modes supported, anything other than warm/soft defaults to cold.
- vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
- psci_reset registers with reboot-mode for registering  vendor resets. Here, we
  have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
  reboot-mode framework. Should the new psci_reset driver, move away from reboot-mode
  framework as-well? And define its own parsing logic for psci_reset_types, and have
  its own restart_notifier instead of reboot_notifier?
- If new psci_reset driver move away from reboot-mode, we can get rid of the panic_notifier
  added in the psci code. Else, we may still need the panic_notifier for any kernel panic
  that occurs between reboot_notifier and restart_notifier?
- psci driver will export a function which will be called externally to set the current
  psci reset_type.
- psci_sys_reset in psci driver should remove the check on reboot_mode. It will default to
  cold reset (for the reason the current kernel defaults to cold reset in psci.)
  example change in psci_sys_reset:
    if(psci_system_reset2_supported && <psci_reset_new_struct_var> != cold)
       psci_sys_reset2(AS PER PARAMS FROM new psci_reset driver)
    else
       psci_sys_reset(COLD RESET)

thanks,
Shivendra

On Mon, Nov 17, 2025 at 11:14:48PM +0530, Shivendra Pratap wrote:
> 
> 
> On 11/10/2025 10:52 PM, Lorenzo Pieralisi wrote:
> > On Sun, Nov 09, 2025 at 08:07:20PM +0530, Shivendra Pratap wrote:
> >> SoC vendors have different types of resets which are controlled
> >> through various hardware registers. For instance, Qualcomm SoC
> >> may have a requirement that reboot with “bootloader” command
> >> should reboot the device to bootloader flashing mode and reboot
> >> with “edl” should reboot the device into Emergency flashing mode.
> >> Setting up such reboots on Qualcomm devices can be inconsistent
> >> across SoC platforms and may require setting different HW
> >> registers, where some of these registers may not be accessible to
> >> HLOS. These knobs evolve over product generations and require
> >> more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
> >> reset which can help align this requirement. Add support for PSCI
> >> SYSTEM_RESET2, vendor-specific resets and align the implementation
> >> to allow user-space initiated reboots to trigger these resets.
> >>
> >> Implement the PSCI vendor-specific resets by registering to the
> >> reboot-mode framework.
> > 
> > I think that we should expose to user space _all_ PSCI reset types,
> > cold, warm + vendor specific - as a departure from using the reboot_mode
> > variable (and possibly deprecate it - or at least stop using it).
> 
> sure. We can try that. Have tried to compile it all at the end of this thread.
> 
> > 
> >> As psci init is done at early kernel init, reboot-mode registration cannot
> >> be done at the time of psci init.  This is because reboot-mode creates a
> >> “reboot-mode” class for exposing sysfs, which can fail at early kernel init.
> >> To overcome this, introduce a late_initcall to register PSCI vendor-specific
> >> resets as reboot modes. Implement a reboot-mode write function that sets
> >> reset_type and cookie values during the reboot notifier callback.  Introduce
> >> a firmware-based call for SYSTEM_RESET2 vendor-specific reset in the
> >> psci_sys_reset path, using reset_type and cookie if supported by secure
> >> firmware. Register a panic notifier and clear vendor_reset valid status
> >> during panic.  This is needed for any kernel panic that occurs post
> >> reboot_notifiers.
> > 
> > Is it because panic uses reboot_mode to determine the reset to issue ?
> 
> Yes. As we know, currently psci supports only two resets,
> psci_sys_reset2 (ARCH warm reset) and psci_sys_reset(COLD RESET). And kernel
> panic path should take the path set by reboot_mode to maintain backward
> compatibility. 
> 
> > 
> >> By using the above implementation, userspace will be able to issue
> >> such resets using the reboot() system call with the "*arg"
> >> parameter as a string based command. The commands can be defined
> >> in PSCI device tree node under “reboot-mode” and are based on the
> >> reboot-mode based commands.
> > 
> > IMHO - it would be nice if could add mode-cold (or mode-normal in reboot mode
> > speak) and mode-warm by default (if PSCI supports them) so that userspace
> 
> Default mode in current kernel is cold, until explicitly set to warm.
> So should it be defaulted to cold?

I managed to confuse you sorry. What I wanted to say is that user space
should be able to issue _all_ PSCI resets (inclusive of cold and warm if
supported - ie if SYSTEM_RESET2 is supported) not just vendor resets.

I misused "by default" - I meant cold and warm PSCI resets should be part
of the reboot-mode list.

[...]

> >>  
> >> +struct psci_vendor_sysreset2 {
> >> +	u32 reset_type;
> >> +	u32 cookie;
> >> +	bool valid;
> >> +};
> >> +
> >> +static struct psci_vendor_sysreset2 vendor_reset;
> > 
> > I think this should represent all possible PSCI reset types, not vendor only
> > and its value is set by the reboot mode framework.
> > 
> >> +
> >> +static int psci_panic_event(struct notifier_block *nb, unsigned long v, void *p)
> >> +{
> >> +	vendor_reset.valid = false;
> > 
> > I don't like this. Basically all you want this for is to make sure that
> > we don't override the reboot_mode variable.
> 
> Yes, it does not look good but as we planned to use reboot-mode framework earlier, which
> sets the modes at the at reboot_notifiers. This needs to be taken care for any panic
> that occurs between reboot_notifier and restart_notifier.

Isn't there a simpler way to detect whether we are in panic mode and
consequently we just issue a reset based on reboot_mode ?

panic_in_progress() ?

> > One (hack) would consist in checking the reboot_mode variable here and
> > set the struct I mentioned above to the value represented in reboot_mode.
> > 
> > Good luck if reboot_mode == REBOOT_GPIO :-)
> 
> psci supports only two modes, ARCH_WARM and cold, so anything else except WARM/SOFT
> should default to cold? So even if REBOOT_GPIO is set in reboot_mode, we should default
> it to cold reset.
> 
> > 
> >> +	return NOTIFY_DONE;
> >> +}
> >> +
> >> +static struct notifier_block psci_panic_block = {
> >> +	.notifier_call = psci_panic_event
> >> +};
> >> +
> >>  bool psci_tos_resident_on(int cpu)
> >>  {
> >>  	return cpu == resident_cpu;
> >> @@ -309,7 +330,10 @@ static int get_set_conduit_method(const struct device_node *np)
> >>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> >>  			  void *data)
> >>  {
> >> -	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >> +	if (vendor_reset.valid && psci_system_reset2_supported) {
> >> +		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
> >> +			       vendor_reset.cookie, 0);
> > 
> > See above. Two calls here: one for resets issued using the new userspace
> > interface you are adding and legacy below - no vendor vs reboot_mode, this
> > is a mess.
> 
> Are we suggesting to completely remove the reboot_mode check from here in the new
> design and base it on reboot <CMD> param?

I am suggesting that there must be two reset options:

- based on reboot mode set by user space
- based on reboot_mode variable (as a fallback and while panic is in progress)

> > 
> >> +	} else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >>  	    psci_system_reset2_supported) {
> >>  		/*
> >>  		 * reset_type[31] = 0 (architectural)
> >> @@ -547,6 +571,72 @@ static const struct platform_suspend_ops psci_suspend_ops = {
> >>  	.enter          = psci_system_suspend_enter,
> >>  };
> >>  
> >> +static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
> >> +{
> >> +	u32 magic_32;
> >> +
> >> +	if (psci_system_reset2_supported) {
> >> +		magic_32 = magic & GENMASK(31, 0);
> >> +		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
> >> +		vendor_reset.cookie = (magic >> 32) & GENMASK(31, 0);
> > 
> > Use FIELD_PREP/GET() please (but as mentioned above the vendor reset type
> > bit[31] should be part of the reboot mode magic value, see above).
> 
> sure. Will align this. thanks.
> 
> > 
> >> +		vendor_reset.valid = true;
> >> +	}
> >> +
> >> +	return NOTIFY_DONE;
> >> +}
> >> +
> >> +static int __init psci_init_vendor_reset(void)
> >> +{
> >> +	struct reboot_mode_driver *reboot;
> >> +	struct device_node *psci_np;
> >> +	struct device_node *np;
> >> +	int ret;
> >> +
> >> +	if (!psci_system_reset2_supported)
> >> +		return -EINVAL;
> >> +
> >> +	psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
> >> +	if (!psci_np)
> >> +		return -ENODEV;
> >> +
> >> +	np = of_find_node_by_name(psci_np, "reboot-mode");
> >> +	if (!np) {
> >> +		of_node_put(psci_np);
> >> +		return -ENODEV;
> >> +	}
> >> +
> >> +	ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
> >> +	if (ret)
> >> +		goto err_notifier;
> >> +
> >> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
> >> +	if (!reboot) {
> >> +		ret = -ENOMEM;
> >> +		goto err_kzalloc;
> >> +	}
> >> +
> >> +	reboot->write = psci_set_vendor_sys_reset2;
> >> +	reboot->driver_name = "psci";
> >> +
> >> +	ret = reboot_mode_register(reboot, of_fwnode_handle(np));
> >> +	if (ret)
> >> +		goto err_register;
> >> +
> >> +	of_node_put(psci_np);
> >> +	of_node_put(np);
> >> +	return 0;
> >> +
> >> +err_register:
> >> +	kfree(reboot);
> >> +err_kzalloc:
> >> +	atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
> >> +err_notifier:
> >> +	of_node_put(psci_np);
> >> +	of_node_put(np);
> >> +	return ret;
> >> +}
> >> +late_initcall(psci_init_vendor_reset)
> > 
> > I don't like adding another initcall here.
> > 
> > I wonder whether this code belongs in a PSCI reboot mode driver, possibly a
> > faux device in a way similar to what we did for cpuidle-psci (that after all
> > is a consumer of PSCI_CPU_SUSPEND in a similar way as this code is a
> > PSCI_SYSTEM_RESET{2} consumer), that communicates with
> > drivers/firmware/psci/psci.c with the struct mentioned above.
> 
> sure. we can create a new driver and try it as in cpuidle: cpuidle-psci.
> Can you suggest a bit more on the overall approach we want to take here?
> Have tried to summarize the potential changes and few questions below.
> 
> - new driver registers a faux device - say - power: reset: psci_reset.

Yes this could be a potential way forward but that's decoupled from the
options below. If we take this route PSCI maintainers should be added
as maintainers for this reboot mode driver.

> - struct with pre-built psci reset_types - (warm, soft, cold). Currently
>   only two modes supported, anything other than warm/soft defaults to cold.
> - vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
> - psci_reset registers with reboot-mode for registering  vendor resets. Here, we
>   have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
>   reboot-mode framework.

Why ?

>   Should the new psci_reset driver, move away from reboot-mode
>   framework as-well? And define its own parsing logic for psci_reset_types,
>   and have its own restart_notifier instead of reboot_notifier?

No. As I said earlier, I think it makes sense to allow user space to
select _all_ PSCI reset types - architected and vendor specific in
a single reboot mode driver.

I believe that we must be able to have two well defined ways for
issuing resets:

- one based on reboot mode driver
- one based on reboot_mode variable interface

Does this make sense everyone ? I don't know the history behind
reboot_mode and the reboot mode driver framework I am just stating
what I think makes sense to do for PSCI.

Thanks,
Lorenzo

> - If new psci_reset driver move away from reboot-mode, we can get rid of the panic_notifier
>   added in the psci code. Else, we may still need the panic_notifier for any kernel panic
>   that occurs between reboot_notifier and restart_notifier?
> - psci driver will export a function which will be called externally to set the current
>   psci reset_type.
> - psci_sys_reset in psci driver should remove the check on reboot_mode. It will default to
>   cold reset (for the reason the current kernel defaults to cold reset in psci.)
>   example change in psci_sys_reset:
>     if(psci_system_reset2_supported && <psci_reset_new_struct_var> != cold)
>        psci_sys_reset2(AS PER PARAMS FROM new psci_reset driver)
>     else
>        psci_sys_reset(COLD RESET)
> 
> thanks,
> Shivendra

On 11/18/2025 5:58 PM, Lorenzo Pieralisi wrote:
> On Mon, Nov 17, 2025 at 11:14:48PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 11/10/2025 10:52 PM, Lorenzo Pieralisi wrote:
>>> On Sun, Nov 09, 2025 at 08:07:20PM +0530, Shivendra Pratap wrote:
>>>> SoC vendors have different types of resets which are controlled
>>>> through various hardware registers. For instance, Qualcomm SoC
>>>> may have a requirement that reboot with “bootloader” command
>>>> should reboot the device to bootloader flashing mode and reboot
>>>> with “edl” should reboot the device into Emergency flashing mode.
>>>> Setting up such reboots on Qualcomm devices can be inconsistent
>>>> across SoC platforms and may require setting different HW
>>>> registers, where some of these registers may not be accessible to
>>>> HLOS. These knobs evolve over product generations and require
>>>> more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
>>>> reset which can help align this requirement. Add support for PSCI
>>>> SYSTEM_RESET2, vendor-specific resets and align the implementation
>>>> to allow user-space initiated reboots to trigger these resets.
>>>>
>>>> Implement the PSCI vendor-specific resets by registering to the
>>>> reboot-mode framework.
>>>
>>> I think that we should expose to user space _all_ PSCI reset types,
>>> cold, warm + vendor specific - as a departure from using the reboot_mode
>>> variable (and possibly deprecate it - or at least stop using it).
>>
>> sure. We can try that. Have tried to compile it all at the end of this thread.
>>
>>>
>>>> As psci init is done at early kernel init, reboot-mode registration cannot
>>>> be done at the time of psci init.  This is because reboot-mode creates a
>>>> “reboot-mode” class for exposing sysfs, which can fail at early kernel init.
>>>> To overcome this, introduce a late_initcall to register PSCI vendor-specific
>>>> resets as reboot modes. Implement a reboot-mode write function that sets
>>>> reset_type and cookie values during the reboot notifier callback.  Introduce
>>>> a firmware-based call for SYSTEM_RESET2 vendor-specific reset in the
>>>> psci_sys_reset path, using reset_type and cookie if supported by secure
>>>> firmware. Register a panic notifier and clear vendor_reset valid status
>>>> during panic.  This is needed for any kernel panic that occurs post
>>>> reboot_notifiers.
>>>
>>> Is it because panic uses reboot_mode to determine the reset to issue ?
>>
>> Yes. As we know, currently psci supports only two resets,
>> psci_sys_reset2 (ARCH warm reset) and psci_sys_reset(COLD RESET). And kernel
>> panic path should take the path set by reboot_mode to maintain backward
>> compatibility. 
>>
>>>
>>>> By using the above implementation, userspace will be able to issue
>>>> such resets using the reboot() system call with the "*arg"
>>>> parameter as a string based command. The commands can be defined
>>>> in PSCI device tree node under “reboot-mode” and are based on the
>>>> reboot-mode based commands.
>>>
>>> IMHO - it would be nice if could add mode-cold (or mode-normal in reboot mode
>>> speak) and mode-warm by default (if PSCI supports them) so that userspace
>>
>> Default mode in current kernel is cold, until explicitly set to warm.
>> So should it be defaulted to cold?
> 
> I managed to confuse you sorry. What I wanted to say is that user space
> should be able to issue _all_ PSCI resets (inclusive of cold and warm if
> supported - ie if SYSTEM_RESET2 is supported) not just vendor resets.
> 
> I misused "by default" - I meant cold and warm PSCI resets should be part
> of the reboot-mode list.
> 
> [...]
> 
>>>>  
>>>> +struct psci_vendor_sysreset2 {
>>>> +	u32 reset_type;
>>>> +	u32 cookie;
>>>> +	bool valid;
>>>> +};
>>>> +
>>>> +static struct psci_vendor_sysreset2 vendor_reset;
>>>
>>> I think this should represent all possible PSCI reset types, not vendor only
>>> and its value is set by the reboot mode framework.
>>>
>>>> +
>>>> +static int psci_panic_event(struct notifier_block *nb, unsigned long v, void *p)
>>>> +{
>>>> +	vendor_reset.valid = false;
>>>
>>> I don't like this. Basically all you want this for is to make sure that
>>> we don't override the reboot_mode variable.
>>
>> Yes, it does not look good but as we planned to use reboot-mode framework earlier, which
>> sets the modes at the at reboot_notifiers. This needs to be taken care for any panic
>> that occurs between reboot_notifier and restart_notifier.
> 
> Isn't there a simpler way to detect whether we are in panic mode and
> consequently we just issue a reset based on reboot_mode ?
> 
> panic_in_progress() ?

Yes. panic_in_progress is added in 6.18.rc1. We can use that. Thanks. Seems i was
outdated.

> 
>>> One (hack) would consist in checking the reboot_mode variable here and
>>> set the struct I mentioned above to the value represented in reboot_mode.
>>>
>>> Good luck if reboot_mode == REBOOT_GPIO :-)
>>
>> psci supports only two modes, ARCH_WARM and cold, so anything else except WARM/SOFT
>> should default to cold? So even if REBOOT_GPIO is set in reboot_mode, we should default
>> it to cold reset.
>>
>>>
>>>> +	return NOTIFY_DONE;
>>>> +}
>>>> +
>>>> +static struct notifier_block psci_panic_block = {
>>>> +	.notifier_call = psci_panic_event
>>>> +};
>>>> +
>>>>  bool psci_tos_resident_on(int cpu)
>>>>  {
>>>>  	return cpu == resident_cpu;
>>>> @@ -309,7 +330,10 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>  			  void *data)
>>>>  {
>>>> -	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>> +	if (vendor_reset.valid && psci_system_reset2_supported) {
>>>> +		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
>>>> +			       vendor_reset.cookie, 0);
>>>
>>> See above. Two calls here: one for resets issued using the new userspace
>>> interface you are adding and legacy below - no vendor vs reboot_mode, this
>>> is a mess.
>>
>> Are we suggesting to completely remove the reboot_mode check from here in the new
>> design and base it on reboot <CMD> param?
> 
> I am suggesting that there must be two reset options:
> 
> - based on reboot mode set by user space
> - based on reboot_mode variable (as a fallback and while panic is in progress)
> 
>>>
>>>> +	} else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>  	    psci_system_reset2_supported) {
>>>>  		/*
>>>>  		 * reset_type[31] = 0 (architectural)
>>>> @@ -547,6 +571,72 @@ static const struct platform_suspend_ops psci_suspend_ops = {
>>>>  	.enter          = psci_system_suspend_enter,
>>>>  };
>>>>  
>>>> +static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
>>>> +{
>>>> +	u32 magic_32;
>>>> +
>>>> +	if (psci_system_reset2_supported) {
>>>> +		magic_32 = magic & GENMASK(31, 0);
>>>> +		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
>>>> +		vendor_reset.cookie = (magic >> 32) & GENMASK(31, 0);
>>>
>>> Use FIELD_PREP/GET() please (but as mentioned above the vendor reset type
>>> bit[31] should be part of the reboot mode magic value, see above).
>>
>> sure. Will align this. thanks.
>>
>>>
>>>> +		vendor_reset.valid = true;
>>>> +	}
>>>> +
>>>> +	return NOTIFY_DONE;
>>>> +}
>>>> +
>>>> +static int __init psci_init_vendor_reset(void)
>>>> +{
>>>> +	struct reboot_mode_driver *reboot;
>>>> +	struct device_node *psci_np;
>>>> +	struct device_node *np;
>>>> +	int ret;
>>>> +
>>>> +	if (!psci_system_reset2_supported)
>>>> +		return -EINVAL;
>>>> +
>>>> +	psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
>>>> +	if (!psci_np)
>>>> +		return -ENODEV;
>>>> +
>>>> +	np = of_find_node_by_name(psci_np, "reboot-mode");
>>>> +	if (!np) {
>>>> +		of_node_put(psci_np);
>>>> +		return -ENODEV;
>>>> +	}
>>>> +
>>>> +	ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
>>>> +	if (ret)
>>>> +		goto err_notifier;
>>>> +
>>>> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
>>>> +	if (!reboot) {
>>>> +		ret = -ENOMEM;
>>>> +		goto err_kzalloc;
>>>> +	}
>>>> +
>>>> +	reboot->write = psci_set_vendor_sys_reset2;
>>>> +	reboot->driver_name = "psci";
>>>> +
>>>> +	ret = reboot_mode_register(reboot, of_fwnode_handle(np));
>>>> +	if (ret)
>>>> +		goto err_register;
>>>> +
>>>> +	of_node_put(psci_np);
>>>> +	of_node_put(np);
>>>> +	return 0;
>>>> +
>>>> +err_register:
>>>> +	kfree(reboot);
>>>> +err_kzalloc:
>>>> +	atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
>>>> +err_notifier:
>>>> +	of_node_put(psci_np);
>>>> +	of_node_put(np);
>>>> +	return ret;
>>>> +}
>>>> +late_initcall(psci_init_vendor_reset)
>>>
>>> I don't like adding another initcall here.
>>>
>>> I wonder whether this code belongs in a PSCI reboot mode driver, possibly a
>>> faux device in a way similar to what we did for cpuidle-psci (that after all
>>> is a consumer of PSCI_CPU_SUSPEND in a similar way as this code is a
>>> PSCI_SYSTEM_RESET{2} consumer), that communicates with
>>> drivers/firmware/psci/psci.c with the struct mentioned above.
>>
>> sure. we can create a new driver and try it as in cpuidle: cpuidle-psci.
>> Can you suggest a bit more on the overall approach we want to take here?
>> Have tried to summarize the potential changes and few questions below.
>>
>> - new driver registers a faux device - say - power: reset: psci_reset.
> 
> Yes this could be a potential way forward but that's decoupled from the
> options below. If we take this route PSCI maintainers should be added
> as maintainers for this reboot mode driver.

you mean the new psci_reset driver? yes. Maintainer would be PSCI maintainer,
if we create a new  psci_reset reboot mode driver.

> 
>> - struct with pre-built psci reset_types - (warm, soft, cold). Currently
>>   only two modes supported, anything other than warm/soft defaults to cold.
>> - vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
>> - psci_reset registers with reboot-mode for registering  vendor resets. Here, we
>>   have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
>>   reboot-mode framework.
> 
> Why ?

If we want the new psci_reset to take the reboot-mode framework route, is it ok to
add default modes (warm, cold) in the device tree?
If not, then the design of reboot-mode framework(power:reset:reboot-mode.c) needs to be
further changed to equip this new feature. 

If new psci_reset driver move away from reboot-mode framework(power:reset:reboot-mode.c), the driver
can have its own design, its own sysfs interface and maintained under psci Maintainer.

> 
>>   Should the new psci_reset driver, move away from reboot-mode
>>   framework as-well? And define its own parsing logic for psci_reset_types,
>>   and have its own restart_notifier instead of reboot_notifier?
> 
> No. As I said earlier, I think it makes sense to allow user space to
> select _all_ PSCI reset types - architected and vendor specific in
> a single reboot mode driver.
> 
> I believe that we must be able to have two well defined ways for
> issuing resets:
> 
> - one based on reboot mode driver
> - one based on reboot_mode variable interface

So may be in more details-
user space issues - reboot cold
   -> go for psci_reset (as psci_sysrest2 does not has cold reset?)
user space issues - reboot warm or a vendor_reset
   -> if psci_sysreset2 is supported - call psci_sysreset2 with required params.
   ->   else
   ->  go for psci_reset COLD

user space issues - reboot (no commands) or a panic_in_progress
   -> fallback to reboot_mode 
   ->  if (reboot_mode == WARM and psci_sysreset2 is supported )
   ->     call psci_sysreset2 (ARCH WARM RESET)
   ->  else
   ->     go for psci_reset COLD


And we want to do this in two conditional statements in firmware:psci: psci_sys_reset
function?
Or am i not getting the point here?

thanks,
Shivendra

> 
> Does this make sense everyone ? I don't know the history behind
> reboot_mode and the reboot mode driver framework I am just stating
> what I think makes sense to do for PSCI.
> 
> Thanks,
> Lorenzo
> 
>> - If new psci_reset driver move away from reboot-mode, we can get rid of the panic_notifier
>>   added in the psci code. Else, we may still need the panic_notifier for any kernel panic
>>   that occurs between reboot_notifier and restart_notifier?
>> - psci driver will export a function which will be called externally to set the current
>>   psci reset_type.
>> - psci_sys_reset in psci driver should remove the check on reboot_mode. It will default to
>>   cold reset (for the reason the current kernel defaults to cold reset in psci.)
>>   example change in psci_sys_reset:
>>     if(psci_system_reset2_supported && <psci_reset_new_struct_var> != cold)
>>        psci_sys_reset2(AS PER PARAMS FROM new psci_reset driver)
>>     else
>>        psci_sys_reset(COLD RESET)
>>
>> thanks,
>> Shivendra

On Tue, Nov 18, 2025 at 11:11:33PM +0530, Shivendra Pratap wrote:

[...]

> > Yes this could be a potential way forward but that's decoupled from the
> > options below. If we take this route PSCI maintainers should be added
> > as maintainers for this reboot mode driver.
> 
> you mean the new psci_reset driver? yes. Maintainer would be PSCI maintainer,
> if we create a new  psci_reset reboot mode driver.

Yes.

> >> - struct with pre-built psci reset_types - (warm, soft, cold). Currently
> >>   only two modes supported, anything other than warm/soft defaults to cold.
> >> - vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
> >> - psci_reset registers with reboot-mode for registering  vendor resets. Here, we
> >>   have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
> >>   reboot-mode framework.
> > 
> > Why ?
> 
> If we want the new psci_reset to take the reboot-mode framework route, is it ok to
> add default modes (warm, cold) in the device tree?
> If not, then the design of reboot-mode framework(power:reset:reboot-mode.c) needs to be
> further changed to equip this new feature. 

Well, yes, all it needs to do is allowing prepopulated reboot modes on top
of which DT based ones are added.

I don't see any point in adding properties to the DT node to provide
information we can already probe.

> If new psci_reset driver move away from reboot-mode framework(power:reset:reboot-mode.c), the driver
> can have its own design, its own sysfs interface and maintained under psci Maintainer.
> 
> > 
> >>   Should the new psci_reset driver, move away from reboot-mode
> >>   framework as-well? And define its own parsing logic for psci_reset_types,
> >>   and have its own restart_notifier instead of reboot_notifier?
> > 
> > No. As I said earlier, I think it makes sense to allow user space to
> > select _all_ PSCI reset types - architected and vendor specific in
> > a single reboot mode driver.
> > 
> > I believe that we must be able to have two well defined ways for
> > issuing resets:
> > 
> > - one based on reboot mode driver
> > - one based on reboot_mode variable interface
> 
> So may be in more details-
> user space issues - reboot cold
>    -> go for psci_reset (as psci_sysrest2 does not has cold reset?)
> user space issues - reboot warm or a vendor_reset
>    -> if psci_sysreset2 is supported - call psci_sysreset2 with required params.
>    ->   else
>    ->  go for psci_reset COLD
> 
> user space issues - reboot (no commands) or a panic_in_progress
>    -> fallback to reboot_mode 
>    ->  if (reboot_mode == WARM and psci_sysreset2 is supported )
>    ->     call psci_sysreset2 (ARCH WARM RESET)
>    ->  else
>    ->     go for psci_reset COLD
> 
> 
> And we want to do this in two conditional statements in firmware:psci: psci_sys_reset
> function?
> Or am i not getting the point here?

You are getting the point.

Thanks,
Lorenzo

> thanks,
> Shivendra
> 
> > 
> > Does this make sense everyone ? I don't know the history behind
> > reboot_mode and the reboot mode driver framework I am just stating
> > what I think makes sense to do for PSCI.
> > 
> > Thanks,
> > Lorenzo
> > 
> >> - If new psci_reset driver move away from reboot-mode, we can get rid of the panic_notifier
> >>   added in the psci code. Else, we may still need the panic_notifier for any kernel panic
> >>   that occurs between reboot_notifier and restart_notifier?
> >> - psci driver will export a function which will be called externally to set the current
> >>   psci reset_type.
> >> - psci_sys_reset in psci driver should remove the check on reboot_mode. It will default to
> >>   cold reset (for the reason the current kernel defaults to cold reset in psci.)
> >>   example change in psci_sys_reset:
> >>     if(psci_system_reset2_supported && <psci_reset_new_struct_var> != cold)
> >>        psci_sys_reset2(AS PER PARAMS FROM new psci_reset driver)
> >>     else
> >>        psci_sys_reset(COLD RESET)
> >>
> >> thanks,
> >> Shivendra

On 11/19/2025 3:07 PM, Lorenzo Pieralisi wrote:
> On Tue, Nov 18, 2025 at 11:11:33PM +0530, Shivendra Pratap wrote:
> 
> [...]
> 
>>> Yes this could be a potential way forward but that's decoupled from the
>>> options below. If we take this route PSCI maintainers should be added
>>> as maintainers for this reboot mode driver.
>>
>> you mean the new psci_reset driver? yes. Maintainer would be PSCI maintainer,
>> if we create a new  psci_reset reboot mode driver.
> 
> Yes.
> 
>>>> - struct with pre-built psci reset_types - (warm, soft, cold). Currently
>>>>   only two modes supported, anything other than warm/soft defaults to cold.
>>>> - vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
>>>> - psci_reset registers with reboot-mode for registering  vendor resets. Here, we
>>>>   have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
>>>>   reboot-mode framework.
>>>
>>> Why ?
>>
>> If we want the new psci_reset to take the reboot-mode framework route, is it ok to
>> add default modes (warm, cold) in the device tree?
>> If not, then the design of reboot-mode framework(power:reset:reboot-mode.c) needs to be
>> further changed to equip this new feature. 
> 
> Well, yes, all it needs to do is allowing prepopulated reboot modes on top
> of which DT based ones are added.

The mode-cold , adds a third variable to reboot-modes as the first parameter for 
invoke_psci_fn is different for cold vs warm/vendor.

cold reset call       : invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
vendor/warm reset call: invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor, cookiee, 0);

Each mode will have 3 argument - like:
_ _ _ _ _ _ _ _ _ _ _ _ _  _ _ _ _ _ _ 
MODE   , cold reset, reset_type, cookie
_ _ _ _ _ _ _ _ _ _ _ _ _  _ _ _ _ _ _ - 
COLD   ,   1       ,    0      ,     0
WARM   ,   0       ,    0      ,     0
vendor1,   0       ,0x80000000 ,     1
vendor2,   0       ,0x80000010 ,     0

So reboot-mode framework will now define and support upto three 32 bit arguments for each mode?

thanks,
Shivendra

On Wed, Nov 19, 2025 at 05:32:42PM +0530, Shivendra Pratap wrote:
> 
> 
> On 11/19/2025 3:07 PM, Lorenzo Pieralisi wrote:
> > On Tue, Nov 18, 2025 at 11:11:33PM +0530, Shivendra Pratap wrote:
> > 
> > [...]
> > 
> >>> Yes this could be a potential way forward but that's decoupled from the
> >>> options below. If we take this route PSCI maintainers should be added
> >>> as maintainers for this reboot mode driver.
> >>
> >> you mean the new psci_reset driver? yes. Maintainer would be PSCI maintainer,
> >> if we create a new  psci_reset reboot mode driver.
> > 
> > Yes.
> > 
> >>>> - struct with pre-built psci reset_types - (warm, soft, cold). Currently
> >>>>   only two modes supported, anything other than warm/soft defaults to cold.
> >>>> - vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
> >>>> - psci_reset registers with reboot-mode for registering  vendor resets. Here, we
> >>>>   have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
> >>>>   reboot-mode framework.
> >>>
> >>> Why ?
> >>
> >> If we want the new psci_reset to take the reboot-mode framework route, is it ok to
> >> add default modes (warm, cold) in the device tree?
> >> If not, then the design of reboot-mode framework(power:reset:reboot-mode.c) needs to be
> >> further changed to equip this new feature. 
> > 
> > Well, yes, all it needs to do is allowing prepopulated reboot modes on top
> > of which DT based ones are added.
> 
> The mode-cold , adds a third variable to reboot-modes as the first parameter for 
> invoke_psci_fn is different for cold vs warm/vendor.
> 
> cold reset call       : invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> vendor/warm reset call: invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor, cookiee, 0);
> 
> Each mode will have 3 argument - like:
> _ _ _ _ _ _ _ _ _ _ _ _ _  _ _ _ _ _ _ 
> MODE   , cold reset, reset_type, cookie
> _ _ _ _ _ _ _ _ _ _ _ _ _  _ _ _ _ _ _ - 
> COLD   ,   1       ,    0      ,     0
> WARM   ,   0       ,    0      ,     0
> vendor1,   0       ,0x80000000 ,     1
> vendor2,   0       ,0x80000010 ,     0
> 
> So reboot-mode framework will now define and support upto three 32 bit arguments for each mode?

The cookie value is unused for SYSTEM_WARM_RESET, you can encode there whether
it is a cold (SYSTEM_RESET) or warm (SYSTEM_RESET2 - SYSTEM_WARM_RESET) architectural
reset when the magic value(aka reset_type) == 0x0 ?

The reboot mode parameters do not necessarily need to map to PSCI function
calls parameters - provided we define that explicitly.

Lorenzo

On 11/26/2025 10:48 PM, Lorenzo Pieralisi wrote:
> On Wed, Nov 19, 2025 at 05:32:42PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 11/19/2025 3:07 PM, Lorenzo Pieralisi wrote:
>>> On Tue, Nov 18, 2025 at 11:11:33PM +0530, Shivendra Pratap wrote:
>>>
>>> [...]
>>>
>>>>> Yes this could be a potential way forward but that's decoupled from the
>>>>> options below. If we take this route PSCI maintainers should be added
>>>>> as maintainers for this reboot mode driver.
>>>>
>>>> you mean the new psci_reset driver? yes. Maintainer would be PSCI maintainer,
>>>> if we create a new  psci_reset reboot mode driver.
>>>
>>> Yes.
>>>
>>>>>> - struct with pre-built psci reset_types - (warm, soft, cold). Currently
>>>>>>   only two modes supported, anything other than warm/soft defaults to cold.
>>>>>> - vendor resets to be added as per vendor choice, inside psci device tree(SOC specific).
>>>>>> - psci_reset registers with reboot-mode for registering  vendor resets. Here, we
>>>>>>   have a problem, the pre-built psci reset_types - (warm, soft, cold) cannot be added via
>>>>>>   reboot-mode framework.
>>>>>
>>>>> Why ?
>>>>
>>>> If we want the new psci_reset to take the reboot-mode framework route, is it ok to
>>>> add default modes (warm, cold) in the device tree?
>>>> If not, then the design of reboot-mode framework(power:reset:reboot-mode.c) needs to be
>>>> further changed to equip this new feature. 
>>>
>>> Well, yes, all it needs to do is allowing prepopulated reboot modes on top
>>> of which DT based ones are added.
>>
>> The mode-cold , adds a third variable to reboot-modes as the first parameter for 
>> invoke_psci_fn is different for cold vs warm/vendor.
>>
>> cold reset call       : invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
>> vendor/warm reset call: invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor, cookiee, 0);
>>
>> Each mode will have 3 argument - like:
>> _ _ _ _ _ _ _ _ _ _ _ _ _  _ _ _ _ _ _ 
>> MODE   , cold reset, reset_type, cookie
>> _ _ _ _ _ _ _ _ _ _ _ _ _  _ _ _ _ _ _ - 
>> COLD   ,   1       ,    0      ,     0
>> WARM   ,   0       ,    0      ,     0
>> vendor1,   0       ,0x80000000 ,     1
>> vendor2,   0       ,0x80000010 ,     0
>>
>> So reboot-mode framework will now define and support upto three 32 bit arguments for each mode?
> 
> The cookie value is unused for SYSTEM_WARM_RESET, you can encode there whether
> it is a cold (SYSTEM_RESET) or warm (SYSTEM_RESET2 - SYSTEM_WARM_RESET) architectural
> reset when the magic value(aka reset_type) == 0x0 ?

sure that should work. if reset_type is 0, cookie to decide warm vs cold.

> 
> The reboot mode parameters do not necessarily need to map to PSCI function
> calls parameters - provided we define that explicitly.

got it.

Sorry for out of inline question - 
So the psci_sys_reset() may be looking like below after the changes suggested?
Is this on track?

if( panic_in_progress() || !psci_reset_cmd.valid) {
        if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
            psci_system_reset2_supported) {
                /*
                 * reset_type[31] = 0 (architectural)
                 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
                 * cookie = 0 (ignored by the implementation)
                 */
                invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0, 0);
        } else {
                invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
        }
} else {
        invoke_psci_fn(<psci_reset_cmd.system_reset>, <psci_reset_cmd.reset_type>, <psci_reset_cmd.cookie>, 0);
}

------
where psci_reset_cmd is defined like below?

struct psci_sysreset {
        u32 system_reset; // this will be set as PSCI_FN_NATIVE(1_1, SYSTEM_RESET2) or PSCI_0_2_FN_SYSTEM_RESET.
        u32 reset_type;
        u32 cookie;
        bool valid;
};

static struct psci_sysreset psci_reset_cmd;
--

thanks,
Shivendra

On 11/9/2025 8:07 PM, Shivendra Pratap wrote:
> +static int __init psci_init_vendor_reset(void)
> +{
> +	struct reboot_mode_driver *reboot;
> +	struct device_node *psci_np;
> +	struct device_node *np;


We can take advantage of __cleanup() attribute to simply the code paths.

Declare the variables like below

     struct device_node *psci_np __free(device_node) = NULL;
     struct device_node *np __free(device_node) = NULL;

and get rid of the explicit of_node_put().

I think, we can take up this an improvement once this series landed. But 
if you happen to respin to address other issues, please take care of 
this as well.


> +	int ret;
> +
> +	if (!psci_system_reset2_supported)
> +		return -EINVAL;
> +
> +	psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
> +	if (!psci_np)
> +		return -ENODEV;
> +
> +	np = of_find_node_by_name(psci_np, "reboot-mode");
> +	if (!np) {
> +		of_node_put(psci_np);
> +		return -ENODEV;
> +	}
> +
> +	ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
> +	if (ret)
> +		goto err_notifier;
> +
> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
> +	if (!reboot) {
> +		ret = -ENOMEM;
> +		goto err_kzalloc;
> +	}
> +
> +	reboot->write = psci_set_vendor_sys_reset2;
> +	reboot->driver_name = "psci";
> +
> +	ret = reboot_mode_register(reboot, of_fwnode_handle(np));
> +	if (ret)
> +		goto err_register;
> +
> +	of_node_put(psci_np);
> +	of_node_put(np);
> +	return 0;
> +
> +err_register:
> +	kfree(reboot);
> +err_kzalloc:
> +	atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
> +err_notifier:
> +	of_node_put(psci_np);
> +	of_node_put(np);
> +	return ret;
> +}
> +late_initcall(psci_init_vendor_reset)

On 11/10/2025 10:10 AM, Kathiravan Thirumoorthy wrote:
> 
> On 11/9/2025 8:07 PM, Shivendra Pratap wrote:
>> +static int __init psci_init_vendor_reset(void)
>> +{
>> +    struct reboot_mode_driver *reboot;
>> +    struct device_node *psci_np;
>> +    struct device_node *np;
> 
> 
> We can take advantage of __cleanup() attribute to simply the code paths.
> 
> Declare the variables like below
> 
>     struct device_node *psci_np __free(device_node) = NULL;
>     struct device_node *np __free(device_node) = NULL;
> 
> and get rid of the explicit of_node_put().
> 
> I think, we can take up this an improvement once this series landed. But if you happen to respin to address other issues, please take care of this as well.

Let me evaluate this for next respin.

thanks,
Shivendra