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.

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.

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 as “reset-types” and are based on the
reboot-mode based commands.

Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/firmware/psci/Kconfig |  2 ++
 drivers/firmware/psci/psci.c  | 57 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 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..e14bcdbec1750db8aa9297c8bcdb242f58cc420e 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -17,6 +17,7 @@
 #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 +52,14 @@ 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;
+
 bool psci_tos_resident_on(int cpu)
 {
 	return cpu == resident_cpu;
@@ -309,7 +318,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 +559,49 @@ 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 & 0xFFFFFFFF;
+		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
+		vendor_reset.cookie = (magic >> 32) & 0xFFFFFFFF;
+		vendor_reset.valid = true;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static int __init psci_init_vendor_reset(void)
+{
+	struct reboot_mode_driver *reboot;
+	struct device_node *np;
+	int ret;
+
+	np = of_find_node_by_path("/psci/reboot-mode");
+	if (!np)
+		return -ENODEV;
+
+	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
+	if (!reboot) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	reboot->write = psci_set_vendor_sys_reset2;
+
+	ret = reboot_mode_register(reboot, np, "psci");
+	if (ret) {
+		of_node_put(np);
+		kfree(reboot);
+		return ret;
+	}
+
+	return 0;
+}
+late_initcall(psci_init_vendor_reset)
+
 static void __init psci_init_system_reset2(void)
 {
 	int ret;

-- 
2.34.1

On Sun, 2025-07-27 at 21:54 +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.
> 
> 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.
> 
> 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 as “reset-types” and are based on the
> reboot-mode based commands.
> 
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/firmware/psci/Kconfig |  2 ++
>  drivers/firmware/psci/psci.c  | 57 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 58 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..e14bcdbec1750db8aa9297c8bcdb242f58cc420e 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -17,6 +17,7 @@
>  #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 +52,14 @@ 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;
> +
>  bool psci_tos_resident_on(int cpu)
>  {
>  	return cpu == resident_cpu;
> @@ -309,7 +318,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)

I don't know the PSCI spec, but it looks like with this code it's not
possible to set a reboot mode (in DT) and at the same time instruct
the firmware whether a warm or a cold reboot was requested.

Doing warm reboot is useful if e.g. RAM contents needs to be retained
for crash recovery handling, or other reasons, while in normal cases
doing a more secure cold reboot.

On the other hand, of course it's useful to be able to specify the
reboot target for normal reboots.

Is this a problem with the PSCI spec or with this specific change
geared at the Qcom implementation?


> @@ -547,6 +559,49 @@ 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 & 0xFFFFFFFF;

I believe usual kernel style is to use lower case for
hex values.

> +		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
> +		vendor_reset.cookie = (magic >> 32) & 0xFFFFFFFF;

dito.

Cheers,
Andre'

> +		vendor_reset.valid = true;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int __init psci_init_vendor_reset(void)
> +{
> +	struct reboot_mode_driver *reboot;
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_node_by_path("/psci/reboot-mode");
> +	if (!np)
> +		return -ENODEV;
> +
> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
> +	if (!reboot) {
> +		of_node_put(np);
> +		return -ENOMEM;
> +	}
> +
> +	reboot->write = psci_set_vendor_sys_reset2;
> +
> +	ret = reboot_mode_register(reboot, np, "psci");
> +	if (ret) {
> +		of_node_put(np);
> +		kfree(reboot);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +late_initcall(psci_init_vendor_reset)
> +
>  static void __init psci_init_system_reset2(void)
>  {
>  	int ret;

On 7/30/2025 2:14 PM, André Draszik wrote:
> On Sun, 2025-07-27 at 21:54 +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.
>>
>> 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.
>>
>> 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 as “reset-types” and are based on the
>> reboot-mode based commands.
>>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>>  drivers/firmware/psci/Kconfig |  2 ++
>>  drivers/firmware/psci/psci.c  | 57 ++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 58 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..e14bcdbec1750db8aa9297c8bcdb242f58cc420e 100644
>> --- a/drivers/firmware/psci/psci.c
>> +++ b/drivers/firmware/psci/psci.c
>> @@ -17,6 +17,7 @@
>>  #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 +52,14 @@ 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;
>> +
>>  bool psci_tos_resident_on(int cpu)
>>  {
>>  	return cpu == resident_cpu;
>> @@ -309,7 +318,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)
> 
> I don't know the PSCI spec, but it looks like with this code it's not
> possible to set a reboot mode (in DT) and at the same time instruct
> the firmware whether a warm or a cold reboot was requested.

The code added in this patch is kind of dead, until vendor_reset.valid is set to true.
It can be true, only when both below conditions are true.
 1. A SoC DT defines a psci->reboot-mode command say - "bootloader".
 2. reboot sys call is issued using LINUX_REBOOT_CMD_RESTART2 and the arg* as "bootloader".
      reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2, LINUX_REBOOT_CMD_RESTART2, "bootloader");

With that in place, warm and cold reboot just work same as before until above conditions are true.
There is no effect on regular reboots or the reboots with a "command" which is not defined in
psci->reboot-mode DT.

Now lets take a case below, where a SoC vendor wants a combination of psci->reboo-mode command and
warm/cold to work in combination. For ex. a requirement below:
 - reboot command say - "bootlaoder" should do a cold reboot along with some extra HW reg writes.
 - reboot command say - "edl" should do a warm reboot along with some extra HW reg writes.

1. For this, both commands will be defined in the psci->reboot-mode DT Node with the arguments that
   are defined and supported by the firmware.
2. Further, such requirement will now be taken care by the underlying firmware that supports
   PSCI vendor-specific reset. When we call into firmware with vendor specific reset arguments,
   firmware will take care of the defined HW writes and warm/cold reset as per the mapping of the
   defined arguments. Firmware and the Linux kernel here are in agreement for executing the
   vendor-specific resets.

> 
> Doing warm reboot is useful if e.g. RAM contents needs to be retained
> for crash recovery handling, or other reasons, while in normal cases
> doing a more secure cold reboot.
> 
> On the other hand, of course it's useful to be able to specify the
> reboot target for normal reboots.
> 
> Is this a problem with the PSCI spec or with this specific change
> geared at the Qcom implementation?

SoC vendor should define a vendor-specific reset in psci DT only when they support them in their
firmware. 

Do we still think we are breaking anything in the spec or in the warm or the cold
reset path? If so can we discuss such use-cases?

> 
> 
>> @@ -547,6 +559,49 @@ 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 & 0xFFFFFFFF;
> 
> I believe usual kernel style is to use lower case for
> hex values.

Sure, will make it lower case.

> 
>> +		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
>> +		vendor_reset.cookie = (magic >> 32) & 0xFFFFFFFF;
> 
> dito.

Ack.

thanks.

> 
> Cheers,
> Andre'
> 
>> +		vendor_reset.valid = true;
>> +	}
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static int __init psci_init_vendor_reset(void)
>> +{
>> +	struct reboot_mode_driver *reboot;
>> +	struct device_node *np;
>> +	int ret;
>> +
>> +	np = of_find_node_by_path("/psci/reboot-mode");
>> +	if (!np)
>> +		return -ENODEV;
>> +
>> +	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
>> +	if (!reboot) {
>> +		of_node_put(np);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	reboot->write = psci_set_vendor_sys_reset2;
>> +
>> +	ret = reboot_mode_register(reboot, np, "psci");
>> +	if (ret) {
>> +		of_node_put(np);
>> +		kfree(reboot);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +late_initcall(psci_init_vendor_reset)
>> +
>>  static void __init psci_init_system_reset2(void)
>>  {
>>  	int ret;

On Wed, 2025-07-30 at 18:33 +0530, Shivendra Pratap wrote:
> 
> 
> On 7/30/2025 2:14 PM, André Draszik wrote:
> > On Sun, 2025-07-27 at 21:54 +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.
> > > 
> > > 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.
> > > 
> > > 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 as “reset-types” and are based on the
> > > reboot-mode based commands.
> > > 
> > > Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> > > ---
> > >  drivers/firmware/psci/Kconfig |  2 ++
> > >  drivers/firmware/psci/psci.c  | 57 ++++++++++++++++++++++++++++++++++++++++++-
> > >  2 files changed, 58 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..e14bcdbec1750db8aa9297c8bcdb242f58cc420e 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -17,6 +17,7 @@
> > >  #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 +52,14 @@ 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;
> > > +
> > >  bool psci_tos_resident_on(int cpu)
> > >  {
> > >  	return cpu == resident_cpu;
> > > @@ -309,7 +318,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)
> > 
> > I don't know the PSCI spec, but it looks like with this code it's not
> > possible to set a reboot mode (in DT) and at the same time instruct
> > the firmware whether a warm or a cold reboot was requested.
> 
> The code added in this patch is kind of dead, until vendor_reset.valid is set to true.
> It can be true, only when both below conditions are true.
>  1. A SoC DT defines a psci->reboot-mode command say - "bootloader".
>  2. reboot sys call is issued using LINUX_REBOOT_CMD_RESTART2 and the arg* as "bootloader".
>       reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2, LINUX_REBOOT_CMD_RESTART2, "bootloader");
> 
> With that in place, warm and cold reboot just work same as before until above conditions are true.
> There is no effect on regular reboots or the reboots with a "command" which is not defined in
> psci->reboot-mode DT.
> 
> Now lets take a case below, where a SoC vendor wants a combination of psci->reboo-mode command and
> warm/cold to work in combination. For ex. a requirement below:
>  - reboot command say - "bootlaoder" should do a cold reboot along with some extra HW reg writes.
>  - reboot command say - "edl" should do a warm reboot along with some extra HW reg writes.
> 
> 1. For this, both commands will be defined in the psci->reboot-mode DT Node with the arguments that
>    are defined and supported by the firmware.
> 2. Further, such requirement will now be taken care by the underlying firmware that supports
>    PSCI vendor-specific reset. When we call into firmware with vendor specific reset arguments,
>    firmware will take care of the defined HW writes and warm/cold reset as per the mapping of the
>    defined arguments. Firmware and the Linux kernel here are in agreement for executing the
>    vendor-specific resets.

So that means

    echo warm > /sys/kernel/reboot/mode
    reboot bootloader

and

    echo cold > /sys/kernel/reboot/mode
    reboot bootloader

can not be distinguished.

The firmware can not know whether or not cold or warm reboot was
requested in this case by the user.

More importantly, if e.g. an OOPS / panic happens after the reboot
notifier has run (and set vendor_reset.valid because a reboot mode
was requested), a panic handler changing reboot_mode to warm to
retain RAM contents will have no effect, because the the original
code above making those distinctions can not be reached anymore.

Above scenario with OOPS / panic after reboot notifier could e.g.
happen as part of device_shutdown() - see kernel_shutdown_prepare()


> > 
> > Doing warm reboot is useful if e.g. RAM contents needs to be retained
> > for crash recovery handling, or other reasons, while in normal cases
> > doing a more secure cold reboot.
> > 
> > On the other hand, of course it's useful to be able to specify the
> > reboot target for normal reboots.
> > 
> > Is this a problem with the PSCI spec or with this specific change
> > geared at the Qcom implementation?
> 
> SoC vendor should define a vendor-specific reset in psci DT only when they support them in their
> firmware. 
> 
> Do we still think we are breaking anything in the spec or in the warm or the cold
> reset path? If so can we discuss such use-cases?

I don't know the spec, but see examples above.

Cheers,
Andre'

On 7/30/2025 8:53 PM, André Draszik wrote:
> On Wed, 2025-07-30 at 18:33 +0530, Shivendra Pratap wrote:
>>
>>
>> On 7/30/2025 2:14 PM, André Draszik wrote:
>>> On Sun, 2025-07-27 at 21:54 +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.
>>>>
>>>> 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.
>>>>
>>>> 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 as “reset-types” and are based on the
>>>> reboot-mode based commands.
>>>>
>>>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>>>> ---
>>>>  drivers/firmware/psci/Kconfig |  2 ++
>>>>  drivers/firmware/psci/psci.c  | 57 ++++++++++++++++++++++++++++++++++++++++++-
>>>>  2 files changed, 58 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..e14bcdbec1750db8aa9297c8bcdb242f58cc420e 100644
>>>> --- a/drivers/firmware/psci/psci.c
>>>> +++ b/drivers/firmware/psci/psci.c
>>>> @@ -17,6 +17,7 @@
>>>>  #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 +52,14 @@ 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;
>>>> +
>>>>  bool psci_tos_resident_on(int cpu)
>>>>  {
>>>>  	return cpu == resident_cpu;
>>>> @@ -309,7 +318,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)
>>>
>>> I don't know the PSCI spec, but it looks like with this code it's not
>>> possible to set a reboot mode (in DT) and at the same time instruct
>>> the firmware whether a warm or a cold reboot was requested.
>>
>> The code added in this patch is kind of dead, until vendor_reset.valid is set to true.
>> It can be true, only when both below conditions are true.
>>  1. A SoC DT defines a psci->reboot-mode command say - "bootloader".
>>  2. reboot sys call is issued using LINUX_REBOOT_CMD_RESTART2 and the arg* as "bootloader".
>>       reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2, LINUX_REBOOT_CMD_RESTART2, "bootloader");
>>
>> With that in place, warm and cold reboot just work same as before until above conditions are true.
>> There is no effect on regular reboots or the reboots with a "command" which is not defined in
>> psci->reboot-mode DT.
>>
>> Now lets take a case below, where a SoC vendor wants a combination of psci->reboo-mode command and
>> warm/cold to work in combination. For ex. a requirement below:
>>  - reboot command say - "bootlaoder" should do a cold reboot along with some extra HW reg writes.
>>  - reboot command say - "edl" should do a warm reboot along with some extra HW reg writes.
>>
>> 1. For this, both commands will be defined in the psci->reboot-mode DT Node with the arguments that
>>    are defined and supported by the firmware.
>> 2. Further, such requirement will now be taken care by the underlying firmware that supports
>>    PSCI vendor-specific reset. When we call into firmware with vendor specific reset arguments,
>>    firmware will take care of the defined HW writes and warm/cold reset as per the mapping of the
>>    defined arguments. Firmware and the Linux kernel here are in agreement for executing the
>>    vendor-specific resets.
> 
> So that means
> 
>     echo warm > /sys/kernel/reboot/mode
>     reboot bootloader
> 
> and
> 
>     echo cold > /sys/kernel/reboot/mode
>     reboot bootloader
> 
> can not be distinguished.
> 
> The firmware can not know whether or not cold or warm reboot was
> requested in this case by the user.
> 
> More importantly, if e.g. an OOPS / panic happens after the reboot
> notifier has run (and set vendor_reset.valid because a reboot mode
> was requested), a panic handler changing reboot_mode to warm to
> retain RAM contents will have no effect, because the the original
> code above making those distinctions can not be reached anymore.
> 
> Above scenario with OOPS / panic after reboot notifier could e.g.
> happen as part of device_shutdown() - see kernel_shutdown_prepare()

We can handle the panic path by adding a panic_notifier in psci
and make vendor_reset.valid = false. Do you think adding this can clear
the panic scenario above or there can still be some leak?

thanks.

> 
> 
>>>
>>> Doing warm reboot is useful if e.g. RAM contents needs to be retained
>>> for crash recovery handling, or other reasons, while in normal cases
>>> doing a more secure cold reboot.
>>>
>>> On the other hand, of course it's useful to be able to specify the
>>> reboot target for normal reboots.
>>>
>>> Is this a problem with the PSCI spec or with this specific change
>>> geared at the Qcom implementation?
>>
>> SoC vendor should define a vendor-specific reset in psci DT only when they support them in their
>> firmware. 
>>
>> Do we still think we are breaking anything in the spec or in the warm or the cold
>> reset path? If so can we discuss such use-cases?
> 
> I don't know the spec, but see examples above.
> 
> Cheers,
> Andre'

On Thu, 2025-07-31 at 11:05 +0530, Shivendra Pratap wrote:
> 
> 
> On 7/30/2025 8:53 PM, André Draszik wrote:
> > More importantly, if e.g. an OOPS / panic happens after the reboot
> > notifier has run (and set vendor_reset.valid because a reboot mode
> > was requested), a panic handler changing reboot_mode to warm to
> > retain RAM contents will have no effect, because the the original
> > code above making those distinctions can not be reached anymore.
> > 
> > Above scenario with OOPS / panic after reboot notifier could e.g.
> > happen as part of device_shutdown() - see kernel_shutdown_prepare()
> 
> We can handle the panic path by adding a panic_notifier in psci
> and make vendor_reset.valid = false. Do you think adding this can clear
> the panic scenario above or there can still be some leak?

I think that would work. You then can't convey the reboot command, but
at that stage (panic/oops) it probably doesn't matter anymore, it only
cares about the crash handling which probably is enough.

Cheers,
Andre'

On Wed, Jul 30, 2025, at 17:23, André Draszik wrote:
> On Wed, 2025-07-30 at 18:33 +0530, Shivendra Pratap wrote:
>> On 7/30/2025 2:14 PM, André Draszik wrote:

>> 1. For this, both commands will be defined in the psci->reboot-mode DT Node with the arguments that
>>    are defined and supported by the firmware.
>> 2. Further, such requirement will now be taken care by the underlying firmware that supports
>>    PSCI vendor-specific reset. When we call into firmware with vendor specific reset arguments,
>>    firmware will take care of the defined HW writes and warm/cold reset as per the mapping of the
>>    defined arguments. Firmware and the Linux kernel here are in agreement for executing the
>>    vendor-specific resets.
>
> So that means
>
>     echo warm > /sys/kernel/reboot/mode
>     reboot bootloader
>
> and
>
>     echo cold > /sys/kernel/reboot/mode
>     reboot bootloader
>
> can not be distinguished.
>
> The firmware can not know whether or not cold or warm reboot was
> requested in this case by the user.

My feeling is that this is fine: the /sys/kernel/reboot/mode
interface is not really used on anything other than 32-bit
arm and x86 machines, and the way it is specific as
cold/warm/hard/soft/gpio is not that useful.

I think for the purpose of the new code here, we should talk
about reboot "commands" instead of "modes" to avoid confusing
between the global "enum reboot_mode" variable and the
firmware interface for reboot modes as listed in DT.

    Arnd

On Wed, 2025-07-30 at 19:31 +0200, Arnd Bergmann wrote:
> On Wed, Jul 30, 2025, at 17:23, André Draszik wrote:
> > On Wed, 2025-07-30 at 18:33 +0530, Shivendra Pratap wrote:
> > > On 7/30/2025 2:14 PM, André Draszik wrote:
> 
> > > 1. For this, both commands will be defined in the psci->reboot-mode DT Node with the arguments that
> > >    are defined and supported by the firmware.
> > > 2. Further, such requirement will now be taken care by the underlying firmware that supports
> > >    PSCI vendor-specific reset. When we call into firmware with vendor specific reset arguments,
> > >    firmware will take care of the defined HW writes and warm/cold reset as per the mapping of the
> > >    defined arguments. Firmware and the Linux kernel here are in agreement for executing the
> > >    vendor-specific resets.
> > 
> > So that means
> > 
> >     echo warm > /sys/kernel/reboot/mode
> >     reboot bootloader
> > 
> > and
> > 
> >     echo cold > /sys/kernel/reboot/mode
> >     reboot bootloader
> > 
> > can not be distinguished.
> > 
> > The firmware can not know whether or not cold or warm reboot was
> > requested in this case by the user.
> 
> My feeling is that this is fine: the /sys/kernel/reboot/mode
> interface is not really used on anything other than 32-bit
> arm and x86 machines, and the way it is specific as
> cold/warm/hard/soft/gpio is not that useful.

Currently, reboot_mode as such is also used by gs101 and later, we do
distinguish between the reboot_modes (cold/hard and warm/soft), although
generally not via sysfs indeed. And yes, gpio is a strangely specific
one.

I don't have insight into newer SoCs / board designs in that family,
but my understanding is that newer SoCs use PSCI for reboot as well
and I do believe being able to do cold reboot by default and warm
reboot in some cases (crash handling in particular) to still be a
valid use-case.

> I think for the purpose of the new code here, we should talk
> about reboot "commands" instead of "modes" to avoid confusing
> between the global "enum reboot_mode" variable and the
> firmware interface for reboot modes as listed in DT.

+1 It should really have been added as reboot command or reboot target
in the first place.

A.

On 27/07/2025 18:24, Shivendra Pratap wrote:
> +
> +static int __init psci_init_vendor_reset(void)
> +{
> +	struct reboot_mode_driver *reboot;
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_node_by_path("/psci/reboot-mode");


Why are you looking by full path, not by compatible? Is the ABI - above
path - expressed anywhere?


Best regards,
Krzysztof

On Mon, Jul 28, 2025 at 06:53:14AM +0200, Krzysztof Kozlowski wrote:
> On 27/07/2025 18:24, Shivendra Pratap wrote:
> > +
> > +static int __init psci_init_vendor_reset(void)
> > +{
> > +	struct reboot_mode_driver *reboot;
> > +	struct device_node *np;
> > +	int ret;
> > +
> > +	np = of_find_node_by_path("/psci/reboot-mode");
> 
> 
> Why are you looking by full path, not by compatible? Is the ABI - above
> path - expressed anywhere?

PSCI node is required to have a node name of psci, it doesn't have MMIO,
so it resides in the root node and the reboot-mode is defined in the
previous patch. So, I'd assume, the path is defined.

However the question might be slightly different: why do we need a
separate initcall for reboot handling? The reasons for that need to be
documented in the commit message.

-- 
With best wishes
Dmitry

On 28/07/2025 11:44, Dmitry Baryshkov wrote:
> On Mon, Jul 28, 2025 at 06:53:14AM +0200, Krzysztof Kozlowski wrote:
>> On 27/07/2025 18:24, Shivendra Pratap wrote:
>>> +
>>> +static int __init psci_init_vendor_reset(void)
>>> +{
>>> +	struct reboot_mode_driver *reboot;
>>> +	struct device_node *np;
>>> +	int ret;
>>> +
>>> +	np = of_find_node_by_path("/psci/reboot-mode");
>>
>>
>> Why are you looking by full path, not by compatible? Is the ABI - above
>> path - expressed anywhere?
> 
> PSCI node is required to have a node name of psci, it doesn't have MMIO,

This is true

> so it resides in the root node

This might be or not might be true. It is not defined by ABI. Anyway,
you answered where the ABI would be documented, even though as I said it
is not (/psci is not), but does not answer to first part: why you are
not using compatibles which is always the preferred method?


> and the reboot-mode is defined in the
> previous patch. So, I'd assume, the path is defined.

As I said, path is not. only psci/reboot-mode is.

> 
Best regards,
Krzysztof

On 28/07/2025 14:52, Krzysztof Kozlowski wrote:
> On 28/07/2025 11:44, Dmitry Baryshkov wrote:
>> On Mon, Jul 28, 2025 at 06:53:14AM +0200, Krzysztof Kozlowski wrote:
>>> On 27/07/2025 18:24, Shivendra Pratap wrote:
>>>> +
>>>> +static int __init psci_init_vendor_reset(void)
>>>> +{
>>>> +	struct reboot_mode_driver *reboot;
>>>> +	struct device_node *np;
>>>> +	int ret;
>>>> +
>>>> +	np = of_find_node_by_path("/psci/reboot-mode");
>>>
>>>
>>> Why are you looking by full path, not by compatible? Is the ABI - above
>>> path - expressed anywhere?
>>
>> PSCI node is required to have a node name of psci, it doesn't have MMIO,
> 
> This is true
> 
>> so it resides in the root node
> 
> This might be or not might be true. It is not defined by ABI. Anyway,
> you answered where the ABI would be documented, even though as I said it
> is not (/psci is not), but does not answer to first part: why you are
> not using compatibles which is always the preferred method?

That's a good question, I've added one from my side: why do we need an 
extra late_init call.

> 
> 
>> and the reboot-mode is defined in the
>> previous patch. So, I'd assume, the path is defined.
> 
> As I said, path is not. only psci/reboot-mode is.

Do we have an _actual_ use case where PSCI node is not at at root node? 
If not, it's obviously a deficiency of the schema. Could you please 
provide suggestions on how to describe that in DT schema?


-- 
With best wishes
Dmitry

On 7/28/2025 5:33 PM, Dmitry Baryshkov wrote:
> On 28/07/2025 14:52, Krzysztof Kozlowski wrote:
>> On 28/07/2025 11:44, Dmitry Baryshkov wrote:
>>> On Mon, Jul 28, 2025 at 06:53:14AM +0200, Krzysztof Kozlowski wrote:
>>>> On 27/07/2025 18:24, Shivendra Pratap wrote:
>>>>> +
>>>>> +static int __init psci_init_vendor_reset(void)
>>>>> +{
>>>>> +    struct reboot_mode_driver *reboot;
>>>>> +    struct device_node *np;
>>>>> +    int ret;
>>>>> +
>>>>> +    np = of_find_node_by_path("/psci/reboot-mode");
>>>>
>>>>
>>>> Why are you looking by full path, not by compatible? Is the ABI - above
>>>> path - expressed anywhere?
>>>
>>> PSCI node is required to have a node name of psci, it doesn't have MMIO,
>>
>> This is true
>>
>>> so it resides in the root node
>>
>> This might be or not might be true. It is not defined by ABI. Anyway,
>> you answered where the ABI would be documented, even though as I said it
>> is not (/psci is not), but does not answer to first part: why you are
>> not using compatibles which is always the preferred method?
> 
> That's a good question, I've added one from my side: why do we need an extra late_init call.
"psci" registers with reboot-mode which creates a class and a device under it for exposing
the sysfs.
psci_dt_init is called very early around setup_kernel. At that stage the class creating fails,
so psci cannot register with reboot-mode at this stage.
At early_init, the class creation is success, but the created class and the sysfs does not
enumerates under /sys/class/.
So i added explicit late_init call for this where the sysfs creation seems to work fine.
> 
>>
>>
>>> and the reboot-mode is defined in the
>>> previous patch. So, I'd assume, the path is defined.
>>
>> As I said, path is not. only psci/reboot-mode is.
> 
> Do we have an _actual_ use case where PSCI node is not at at root node? If not, it's obviously a deficiency of the schema. Could you please provide suggestions on how to describe that in DT schema?
> 
> 

On 28/07/2025 14:03, Dmitry Baryshkov wrote:
>>
>>> and the reboot-mode is defined in the
>>> previous patch. So, I'd assume, the path is defined.
>>
>> As I said, path is not. only psci/reboot-mode is.
> 
> Do we have an _actual_ use case where PSCI node is not at at root node? 

Yes, many cases, because it belongs as well to firmware node.

> If not, it's obviously a deficiency of the schema. Could you please 
> provide suggestions on how to describe that in DT schema?

I do not see deficiency. There is no ABI that psci must be root node, so
there is no issue to fix there.

If you want to add such ABI, I will answer: no, don't, because we do not
want paths or node names to be the ABI.

Compatible is the ABI.

Best regards,
Krzysztof

On 7/28/2025 5:53 PM, Krzysztof Kozlowski wrote:
> On 28/07/2025 14:03, Dmitry Baryshkov wrote:
>>>
>>>> and the reboot-mode is defined in the
>>>> previous patch. So, I'd assume, the path is defined.
>>>
>>> As I said, path is not. only psci/reboot-mode is.
>>
>> Do we have an _actual_ use case where PSCI node is not at at root node? 
> 
> Yes, many cases, because it belongs as well to firmware node.
> 
>> If not, it's obviously a deficiency of the schema. Could you please 
>> provide suggestions on how to describe that in DT schema?
> 
> I do not see deficiency. There is no ABI that psci must be root node, so
> there is no issue to fix there.
> 
> If you want to add such ABI, I will answer: no, don't, because we do not
> want paths or node names to be the ABI.
> 
> Compatible is the ABI.
Will define a compatible for psci->reboot-mode node and use it to find the
node. Hope its fine to define a compatible for reboot-mode which is defined
as a property inside psci?

thanks.
> 
> Best regards,
> Krzysztof

On 28/07/2025 18:54, Shivendra Pratap wrote:
> 
> 
> On 7/28/2025 5:53 PM, Krzysztof Kozlowski wrote:
>> On 28/07/2025 14:03, Dmitry Baryshkov wrote:
>>>>
>>>>> and the reboot-mode is defined in the
>>>>> previous patch. So, I'd assume, the path is defined.
>>>>
>>>> As I said, path is not. only psci/reboot-mode is.
>>>
>>> Do we have an _actual_ use case where PSCI node is not at at root node?
>>
>> Yes, many cases, because it belongs as well to firmware node.
>>
>>> If not, it's obviously a deficiency of the schema. Could you please
>>> provide suggestions on how to describe that in DT schema?
>>
>> I do not see deficiency. There is no ABI that psci must be root node, so
>> there is no issue to fix there.
>>
>> If you want to add such ABI, I will answer: no, don't, because we do not
>> want paths or node names to be the ABI.
>>
>> Compatible is the ABI.
> Will define a compatible for psci->reboot-mode node and use it to find the
> node. Hope its fine to define a compatible for reboot-mode which is defined
> as a property inside psci?

I think it was more about finding the PSCI node.

> 
> thanks.
>>
>> Best regards,
>> Krzysztof


-- 
With best wishes
Dmitry

On 7/28/2025 11:59 PM, Dmitry Baryshkov wrote:
> On 28/07/2025 18:54, Shivendra Pratap wrote:
>>
>>
>> On 7/28/2025 5:53 PM, Krzysztof Kozlowski wrote:
>>> On 28/07/2025 14:03, Dmitry Baryshkov wrote:
>>>>>
>>>>>> and the reboot-mode is defined in the
>>>>>> previous patch. So, I'd assume, the path is defined.
>>>>>
>>>>> As I said, path is not. only psci/reboot-mode is.
>>>>
>>>> Do we have an _actual_ use case where PSCI node is not at at root node?
>>>
>>> Yes, many cases, because it belongs as well to firmware node.
>>>
>>>> If not, it's obviously a deficiency of the schema. Could you please
>>>> provide suggestions on how to describe that in DT schema?
>>>
>>> I do not see deficiency. There is no ABI that psci must be root node, so
>>> there is no issue to fix there.
>>>
>>> If you want to add such ABI, I will answer: no, don't, because we do not
>>> want paths or node names to be the ABI.
>>>
>>> Compatible is the ABI.
>> Will define a compatible for psci->reboot-mode node and use it to find the
>> node. Hope its fine to define a compatible for reboot-mode which is defined
>> as a property inside psci?
> 
> I think it was more about finding the PSCI node.
can either find for psci node using psci compatible "arm,psci-1.0". And then
look for reboot-mode node inside psci.
or can directly define a compatible for reboot-mode and find it using compatible.
Is there any other suggestion to find this node? 
> 
>>
>> thanks.
>>>
>>> Best regards,
>>> Krzysztof
> 
>

On Mon, Jul 28, 2025 at 02:23:14PM +0200, Krzysztof Kozlowski wrote:
> On 28/07/2025 14:03, Dmitry Baryshkov wrote:
> >>
> >>> and the reboot-mode is defined in the
> >>> previous patch. So, I'd assume, the path is defined.
> >>
> >> As I said, path is not. only psci/reboot-mode is.
> > 
> > Do we have an _actual_ use case where PSCI node is not at at root node? 
> 
> Yes, many cases, because it belongs as well to firmware node.
> 

+1, I was about to make similar comment. /psci predates the formal push
to put all firmware related nodes under /firmware, so /firmware/psci is
equally possible path and should be recommended one so that all such
firmware related nodes are contained under /firmware.

-- 
Regards,
Sudeep

On Mon, Jul 28, 2025 at 02:23:17PM +0100, Sudeep Holla wrote:
> On Mon, Jul 28, 2025 at 02:23:14PM +0200, Krzysztof Kozlowski wrote:
> > On 28/07/2025 14:03, Dmitry Baryshkov wrote:
> > >>
> > >>> and the reboot-mode is defined in the
> > >>> previous patch. So, I'd assume, the path is defined.
> > >>
> > >> As I said, path is not. only psci/reboot-mode is.
> > > 
> > > Do we have an _actual_ use case where PSCI node is not at at root node? 
> > 
> > Yes, many cases, because it belongs as well to firmware node.
> > 
> 
> +1, I was about to make similar comment. /psci predates the formal push
> to put all firmware related nodes under /firmware, so /firmware/psci is
> equally possible path and should be recommended one so that all such
> firmware related nodes are contained under /firmware.

Ack, thanks for the explanation.

-- 
With best wishes
Dmitry