[PATCH v9 0/5] Implement vendor resets for PSCI SYSTEM_RESET2

[PATCH v9 0/5] Implement vendor resets for PSCI SYSTEM_RESET2

[Elliot Berman]

The PSCI SYSTEM_RESET2 call allows vendor firmware to define additional
reset types which could be mapped to the reboot argument.

Setting up reboot on Qualcomm devices can be inconsistent from chipset
to chipset. Generally, there is a PMIC register that gets written to
decide the reboot type. There is also sometimes a cookie that can be
written to indicate that the bootloader should behave differently than a
regular boot. These knobs evolve over product generations and require
more drivers. Qualcomm firmwares are beginning to expose vendor
SYSTEM_RESET2 types to simplify driver requirements from Linux.

Add support in PSCI to statically wire reboot mode commands from
userspace to a vendor reset and cookie value using the device tree. The
DT bindings are similar to reboot mode framework except that 2
integers are accepted (the type and cookie). Also, reboot mode framework
is intended to program the cookies, but not actually reboot the host.
PSCI SYSTEM_RESET2 does both. I've not added support for reading ACPI
tables since I don't have any device which provides them + firmware that
supports vendor SYSTEM_RESET2 types.

Previous discussions around SYSTEM_RESET2:
- https://lore.kernel.org/lkml/20230724223057.1208122-2-quic_eberman@quicinc.com/T/
- https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>

Changes in v9:
- Don't fallback to architecturally defined resets from Lorenzo.
- Link to v8: https://lore.kernel.org/r/20241107-arm-psci-system_reset2-vendor-reboots-v8-0-e8715fa65cb5@quicinc.com

Changes in v8:
- Code style nits from Stephen
- Add rb3gen2
- Link to v7: https://lore.kernel.org/r/20241028-arm-psci-system_reset2-vendor-reboots-v7-0-a4c40b0ebc54@quicinc.com

Changes in v7:
- Code style nits from Stephen
- Dropped unnecessary hunk from the sa8775p-ride patch
- Link to v6: https://lore.kernel.org/r/20241018-arm-psci-system_reset2-vendor-reboots-v6-0-50cbe88b0a24@quicinc.com

Changes in v6:
- Rebase to v6.11 and fix trivial conflicts in qcm6490-idp
- Add sa8775p-ride support (same as qcm6490-idp)
- Link to v5: https://lore.kernel.org/r/20240617-arm-psci-system_reset2-vendor-reboots-v5-0-086950f650c8@quicinc.com

Changes in v5:
- Drop the nested "items" in prep for future dtschema tools
- Link to v4: https://lore.kernel.org/r/20240611-arm-psci-system_reset2-vendor-reboots-v4-0-98f55aa74ae8@quicinc.com

Changes in v4:
- Change mode- properties from uint32-matrix to uint32-array
- Restructure the reset-types node so only the restriction is in the
  if/then schemas and not the entire definition
- Link to v3: https://lore.kernel.org/r/20240515-arm-psci-system_reset2-vendor-reboots-v3-0-16dd4f9c0ab4@quicinc.com

Changes in v3:
- Limit outer number of items to 1 for mode-* properties
- Move the reboot-mode for psci under a subnode "reset-types"
- Fix the DT node in qcm6490-idp so it doesn't overwrite the one from
  sc7820.dtsi
- Link to v2: https://lore.kernel.org/r/20240414-arm-psci-system_reset2-vendor-reboots-v2-0-da9a055a648f@quicinc.com

Changes in v2:
- Fixes to schema as suggested by Rob and Krzysztof
- Add qcm6490 idp as first Qualcomm device to support
- Link to v1: https://lore.kernel.org/r/20231117-arm-psci-system_reset2-vendor-reboots-v1-0-03c4612153e2@quicinc.com

Changes in v1:
- Reference reboot-mode bindings as suggeted by Rob.
- Link to RFC: https://lore.kernel.org/r/20231030-arm-psci-system_reset2-vendor-reboots-v1-0-dcdd63352ad1@quicinc.com

---
Elliot Berman (5):
      dt-bindings: arm: Document reboot mode magic
      firmware: psci: Read and use vendor reset types
      arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types
      arm64: dts: qcom: qcs6490-rb3gen2: Add PSCI SYSTEM_RESET2 types
      arm64: dts: qcom: sa8775p-ride: Add PSCI SYSTEM_RESET2 types

 Documentation/devicetree/bindings/arm/psci.yaml |  43 ++++++++++
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts        |   7 ++
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts    |   7 ++
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi      |   7 ++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi           |   2 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi            |   2 +-
 drivers/firmware/psci/psci.c                    | 105 ++++++++++++++++++++++++
 7 files changed, 171 insertions(+), 2 deletions(-)
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20231016-arm-psci-system_reset2-vendor-reboots-cc3ad456c070

Best regards,
-- 
Elliot Berman <elliot.berman@oss.qualcomm.com>

[PATCH v9 1/5] dt-bindings: arm: Document reboot mode magic

[Elliot Berman]

From: Elliot Berman <elliot.berman@oss.qualcomm.com>

Add bindings to describe vendor-specific reboot modes. Values here
correspond to valid parameters to vendor-specific reset types in PSCI
SYSTEM_RESET2 call.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/arm/psci.yaml | 43 +++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
index cbb012e217ab80c1ca88e611e7acc06c6d56fad0..5e07c62fe5d74a8d001f6b8a8c9e777f43f3113f 100644
--- a/Documentation/devicetree/bindings/arm/psci.yaml
+++ b/Documentation/devicetree/bindings/arm/psci.yaml
@@ -98,6 +98,27 @@ properties:
       [1] Kernel documentation - ARM idle states bindings
         Documentation/devicetree/bindings/cpu/idle-states.yaml
 
+  reset-types:
+    type: object
+    $ref: /schemas/power/reset/reboot-mode.yaml#
+    unevaluatedProperties: false
+    properties:
+      # "mode-normal" is just SYSTEM_RESET
+      mode-normal: false
+    patternProperties:
+      "^mode-.*$":
+        minItems: 1
+        maxItems: 2
+        description: |
+          Describes a vendor-specific reset type. The string after "mode-"
+          maps a reboot mode to the parameters in the PSCI SYSTEM_RESET2 call.
+
+          Parameters are named mode-xxx = <type[, cookie]>, where xxx
+          is the name of the magic reboot mode, type is the lower 31 bits
+          of the reset_type, and, optionally, the cookie value. If the cookie
+          is not provided, it is defaulted to zero.
+          The 31st bit (vendor-resets) will be implicitly set by the driver.
+
 patternProperties:
   "^power-domain-":
     $ref: /schemas/power/power-domain.yaml#
@@ -137,6 +158,15 @@ allOf:
       required:
         - cpu_off
         - cpu_on
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: arm,psci-1.0
+    then:
+      properties:
+        reset-types: false
 
 additionalProperties: false
 
@@ -261,4 +291,17 @@ examples:
         domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWRDN>;
       };
     };
+
+  - |+
+
+    // Case 5: SYSTEM_RESET2 vendor resets
+    psci {
+      compatible = "arm,psci-1.0";
+      method = "smc";
+
+      reset-types {
+        mode-edl = <0>;
+        mode-bootloader = <1 2>;
+      };
+    };
 ...

-- 
2.34.1

[PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Elliot Berman]

From: Elliot Berman <elliot.berman@oss.qualcomm.com>

SoC vendors have different types of resets and are controlled through
various registers. For instance, Qualcomm chipsets can reboot to a
"download mode" that allows a RAM dump to be collected. Another example
is they also support writing a cookie that can be read by bootloader
during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
vendor reset types to be implemented without requiring drivers for every
register/cookie.

Add support in PSCI to statically map reboot mode commands from
userspace to a vendor reset and cookie value using the device tree.

A separate initcall is needed to parse the devicetree, instead of using
psci_dt_init because mm isn't sufficiently set up to allocate memory.

Reboot mode framework is close but doesn't quite fit with the
design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
be solved but doesn't seem reasonable in sum:
 1. reboot mode registers against the reboot_notifier_list, which is too
    early to call SYSTEM_RESET2. PSCI would need to remember the reset
    type from the reboot-mode framework callback and use it
    psci_sys_reset.
 2. reboot mode assumes only one cookie/parameter is described in the
    device tree. SYSTEM_RESET2 uses 2: one for the type and one for
    cookie.
 3. psci cpuidle driver already registers a driver against the
    arm,psci-1.0 compatible. Refactoring would be needed to have both a
    cpuidle and reboot-mode driver.

Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
---
 drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
 static bool psci_system_reset2_supported;
 static bool psci_system_off2_hibernate_supported;
 
+struct psci_reset_param {
+	const char *mode;
+	u32 reset_type;
+	u32 cookie;
+};
+static struct psci_reset_param *psci_reset_params __ro_after_init;
+static size_t num_psci_reset_params __ro_after_init;
+
 static inline bool psci_has_ext_power_state(void)
 {
 	return psci_cpu_suspend_feature &
@@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
 	return 0;
 }
 
+static int psci_vendor_system_reset2(const char *cmd)
+{
+	unsigned long ret;
+	size_t i;
+
+	for (i = 0; i < num_psci_reset_params; i++) {
+		if (!strcmp(psci_reset_params[i].mode, cmd)) {
+			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
+					     psci_reset_params[i].reset_type,
+					     psci_reset_params[i].cookie, 0);
+			/*
+			 * if vendor reset fails, log it and fall back to
+			 * architecture reset types
+			 */
+			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
+			       (long)ret);
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
 static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
 			  void *data)
 {
+	/*
+	 * try to do the vendor system_reset2
+	 * If there wasn't a matching command, fall back to architectural resets
+	 */
+	if (data && !psci_vendor_system_reset2(data))
+		return NOTIFY_DONE;
+
 	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
 	    psci_system_reset2_supported) {
 		/*
@@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
 	{},
 };
 
+#define REBOOT_PREFIX "mode-"
+
+static int __init psci_init_system_reset2_modes(void)
+{
+	const size_t len = strlen(REBOOT_PREFIX);
+	struct psci_reset_param *param;
+	struct device_node *psci_np __free(device_node) = NULL;
+	struct device_node *np __free(device_node) = NULL;
+	struct property *prop;
+	size_t count = 0;
+	u32 magic[2];
+	int num;
+
+	if (!psci_system_reset2_supported)
+		return 0;
+
+	psci_np = of_find_matching_node(NULL, psci_of_match);
+	if (!psci_np)
+		return 0;
+
+	np = of_find_node_by_name(psci_np, "reset-types");
+	if (!np)
+		return 0;
+
+	for_each_property_of_node(np, prop) {
+		if (strncmp(prop->name, REBOOT_PREFIX, len))
+			continue;
+		num = of_property_count_u32_elems(np, prop->name);
+		if (num != 1 && num != 2)
+			continue;
+
+		count++;
+	}
+
+	param = psci_reset_params =
+		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
+	if (!psci_reset_params)
+		return -ENOMEM;
+
+	for_each_property_of_node(np, prop) {
+		if (strncmp(prop->name, REBOOT_PREFIX, len))
+			continue;
+
+		num = of_property_read_variable_u32_array(np, prop->name, magic,
+							  1, ARRAY_SIZE(magic));
+		if (num < 0) {
+			pr_warn("Failed to parse vendor reboot mode %s\n",
+				param->mode);
+			kfree_const(param->mode);
+			continue;
+		}
+
+		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
+		if (!param->mode)
+			continue;
+
+		/* Force reset type to be in vendor space */
+		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
+		param->cookie = num > 1 ? magic[1] : 0;
+		param++;
+		num_psci_reset_params++;
+	}
+
+	return 0;
+}
+arch_initcall(psci_init_system_reset2_modes);
+
 int __init psci_dt_init(void)
 {
 	struct device_node *np;

-- 
2.34.1

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Florian Fainelli]

On 3/3/25 13:08, Elliot Berman wrote:
> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> 
> SoC vendors have different types of resets and are controlled through
> various registers. For instance, Qualcomm chipsets can reboot to a
> "download mode" that allows a RAM dump to be collected. Another example
> is they also support writing a cookie that can be read by bootloader
> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> vendor reset types to be implemented without requiring drivers for every
> register/cookie.
> 
> Add support in PSCI to statically map reboot mode commands from
> userspace to a vendor reset and cookie value using the device tree.
> 
> A separate initcall is needed to parse the devicetree, instead of using
> psci_dt_init because mm isn't sufficiently set up to allocate memory.
> 
> Reboot mode framework is close but doesn't quite fit with the
> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> be solved but doesn't seem reasonable in sum:
>   1. reboot mode registers against the reboot_notifier_list, which is too
>      early to call SYSTEM_RESET2. PSCI would need to remember the reset
>      type from the reboot-mode framework callback and use it
>      psci_sys_reset.
>   2. reboot mode assumes only one cookie/parameter is described in the
>      device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>      cookie.
>   3. psci cpuidle driver already registers a driver against the
>      arm,psci-1.0 compatible. Refactoring would be needed to have both a
>      cpuidle and reboot-mode driver.
> 
> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Lorenzo Pieralisi]

On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> 
> SoC vendors have different types of resets and are controlled through
> various registers. For instance, Qualcomm chipsets can reboot to a
> "download mode" that allows a RAM dump to be collected. Another example
> is they also support writing a cookie that can be read by bootloader
> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> vendor reset types to be implemented without requiring drivers for every
> register/cookie.
> 
> Add support in PSCI to statically map reboot mode commands from
> userspace to a vendor reset and cookie value using the device tree.

I have managed to discuss a little bit this patchset over the last
few days and I think we have defined a plan going forward.

A point that was raised is:

https://man7.org/linux/man-pages/man2/reboot.2.html

LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
represent ?

Is it the mode the system should reboot into OR it is the
actual command to be issued (which is what this patchset
implements) ?

LINUX_REBOOT_CMD_RESTART "..a default restart..."

It is unclear what "default" means. We wonder whether the
reboot_mode variable was introduced to _define_ that "default".

So, in short, my aim is trying to decouple reboot_mode from the
LINUX_REBOOT_CMD_RESTART2 *arg command.

I believe that adding a sysfs interface to reboot-mode driver
infrastructure would be useful, so that the commands would
be exposed to userspace and userspace can set the *arg command
specifically to issue a given reset/mode.

I wonder why this is not already in place for eg syscon-reboot-mode
resets, how does user space issue a command in those systems if the
available commands aren't exposed to userspace ?

Is there a kernel entity exposing those "modes" to userspace, somehow ?

> A separate initcall is needed to parse the devicetree, instead of using
> psci_dt_init because mm isn't sufficiently set up to allocate memory.
> 
> Reboot mode framework is close but doesn't quite fit with the
> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> be solved but doesn't seem reasonable in sum:
>  1. reboot mode registers against the reboot_notifier_list, which is too
>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
>     type from the reboot-mode framework callback and use it
>     psci_sys_reset.
>  2. reboot mode assumes only one cookie/parameter is described in the
>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>     cookie.

This can be changed and I think it should, so that the reboot modes
are exposed to user space and PSCI can use that.

>  3. psci cpuidle driver already registers a driver against the
>     arm,psci-1.0 compatible. Refactoring would be needed to have both a
>     cpuidle and reboot-mode driver.
> 
> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
> ---
>  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>  static bool psci_system_reset2_supported;
>  static bool psci_system_off2_hibernate_supported;
>  
> +struct psci_reset_param {
> +	const char *mode;
> +	u32 reset_type;
> +	u32 cookie;
> +};
> +static struct psci_reset_param *psci_reset_params __ro_after_init;
> +static size_t num_psci_reset_params __ro_after_init;
> +
>  static inline bool psci_has_ext_power_state(void)
>  {
>  	return psci_cpu_suspend_feature &
> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>  	return 0;
>  }
>  
> +static int psci_vendor_system_reset2(const char *cmd)
> +{
> +	unsigned long ret;
> +	size_t i;
> +
> +	for (i = 0; i < num_psci_reset_params; i++) {
> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> +					     psci_reset_params[i].reset_type,
> +					     psci_reset_params[i].cookie, 0);
> +			/*
> +			 * if vendor reset fails, log it and fall back to
> +			 * architecture reset types

That's not what the code does.

> +			 */
> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
> +			       (long)ret);
> +			return 0;
> +		}
> +	}
> +
> +	return -ENOENT;
> +}
> +
>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>  			  void *data)
>  {
> +	/*
> +	 * try to do the vendor system_reset2
> +	 * If there wasn't a matching command, fall back to architectural resets
> +	 */
> +	if (data && !psci_vendor_system_reset2(data))
> +		return NOTIFY_DONE;
> +
>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>  	    psci_system_reset2_supported) {
>  		/*
> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>  	{},
>  };
>  
> +#define REBOOT_PREFIX "mode-"
> +
> +static int __init psci_init_system_reset2_modes(void)
> +{
> +	const size_t len = strlen(REBOOT_PREFIX);
> +	struct psci_reset_param *param;
> +	struct device_node *psci_np __free(device_node) = NULL;
> +	struct device_node *np __free(device_node) = NULL;
> +	struct property *prop;
> +	size_t count = 0;
> +	u32 magic[2];
> +	int num;
> +
> +	if (!psci_system_reset2_supported)
> +		return 0;
> +
> +	psci_np = of_find_matching_node(NULL, psci_of_match);
> +	if (!psci_np)
> +		return 0;
> +
> +	np = of_find_node_by_name(psci_np, "reset-types");
> +	if (!np)
> +		return 0;

Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
is the actual reset to be issued, should we add a default mode "cold"
and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?

It all boils down to what *arg represents - adding "cold" and "warm"
modes would remove the dependency on reboot_mode for resets issued
through LINUX_REBOOT_CMD_RESTART2, the question is whether this
is the correct thing to do.

Comments very welcome.

Thanks,
Lorenzo

> +
> +	for_each_property_of_node(np, prop) {
> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> +			continue;
> +		num = of_property_count_u32_elems(np, prop->name);
> +		if (num != 1 && num != 2)
> +			continue;
> +
> +		count++;
> +	}
> +
> +	param = psci_reset_params =
> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
> +	if (!psci_reset_params)
> +		return -ENOMEM;
> +
> +	for_each_property_of_node(np, prop) {
> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> +			continue;
> +
> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
> +							  1, ARRAY_SIZE(magic));
> +		if (num < 0) {
> +			pr_warn("Failed to parse vendor reboot mode %s\n",
> +				param->mode);
> +			kfree_const(param->mode);
> +			continue;
> +		}
> +
> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> +		if (!param->mode)
> +			continue;
> +
> +		/* Force reset type to be in vendor space */
> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
> +		param->cookie = num > 1 ? magic[1] : 0;
> +		param++;
> +		num_psci_reset_params++;
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(psci_init_system_reset2_modes);
> +
>  int __init psci_dt_init(void)
>  {
>  	struct device_node *np;
> 
> -- 
> 2.34.1
> 

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Arnd Bergmann]

On Fri, Mar 14, 2025, at 12:19, Lorenzo Pieralisi wrote:
> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>> 
>> SoC vendors have different types of resets and are controlled through
>> various registers. For instance, Qualcomm chipsets can reboot to a
>> "download mode" that allows a RAM dump to be collected. Another example
>> is they also support writing a cookie that can be read by bootloader
>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>> vendor reset types to be implemented without requiring drivers for every
>> register/cookie.
>> 
>> Add support in PSCI to statically map reboot mode commands from
>> userspace to a vendor reset and cookie value using the device tree.
>
> I have managed to discuss a little bit this patchset over the last
> few days and I think we have defined a plan going forward.
>
> A point that was raised is:
>
> https://man7.org/linux/man-pages/man2/reboot.2.html
>
> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> represent ?
>
> Is it the mode the system should reboot into OR it is the
> actual command to be issued (which is what this patchset
> implements) ?
>
> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>
> It is unclear what "default" means. We wonder whether the
> reboot_mode variable was introduced to _define_ that "default".

I think the reboot_mode predates the 'cmd' argument: linux-2.1.30
introduced LINUX_REBOOT_CMD_RESTART2 and it already had
the warm/cold/bios/hard options for i386 reboot. I think the
argument went unused for a while after it got introduced though.

> So, in short, my aim is trying to decouple reboot_mode from the
> LINUX_REBOOT_CMD_RESTART2 *arg command.
>
> I believe that adding a sysfs interface to reboot-mode driver
> infrastructure would be useful, so that the commands would
> be exposed to userspace and userspace can set the *arg command
> specifically to issue a given reset/mode.
>
> I wonder why this is not already in place for eg syscon-reboot-mode
> resets, how does user space issue a command in those systems if the
> available commands aren't exposed to userspace ?
>
> Is there a kernel entity exposing those "modes" to userspace, somehow ?

Don't know one either.

>> A separate initcall is needed to parse the devicetree, instead of using
>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>> 
>> Reboot mode framework is close but doesn't quite fit with the
>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>> be solved but doesn't seem reasonable in sum:
>>  1. reboot mode registers against the reboot_notifier_list, which is too
>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>     type from the reboot-mode framework callback and use it
>>     psci_sys_reset.
>>  2. reboot mode assumes only one cookie/parameter is described in the
>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>     cookie.
>
> This can be changed and I think it should, so that the reboot modes
> are exposed to user space and PSCI can use that.

Can we try to call them 'arguments' rather than 'modes' while discussing?
I think it's way too easy to confuse them otherwise.

>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
>> +	if (!psci_np)
>> +		return 0;
>> +
>> +	np = of_find_node_by_name(psci_np, "reset-types");
>> +	if (!np)
>> +		return 0;
>
> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> is the actual reset to be issued, should we add a default mode "cold"
> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>
> It all boils down to what *arg represents - adding "cold" and "warm"
> modes would remove the dependency on reboot_mode for resets issued
> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> is the correct thing to do.
>
> Comments very welcome.

It would make some sense to me to treat all psci reboot as "warm" and
not do anything here if reboot="cold" is set: those would have to
be backed by a hardware specific driver.

A related problem is how to determine when to use UEFI reboot: at the
moment arm64 tries the UEFI runtime interface before it even attempts
any of the notifiers, so PSCI reboot won't ever be used if UEFI is
present.

     Arnd

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Mukesh Ojha]

On Fri, Mar 14, 2025 at 05:31:44PM +0100, Arnd Bergmann wrote:
> On Fri, Mar 14, 2025, at 12:19, Lorenzo Pieralisi wrote:
> > On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> >> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >> 
> >> SoC vendors have different types of resets and are controlled through
> >> various registers. For instance, Qualcomm chipsets can reboot to a
> >> "download mode" that allows a RAM dump to be collected. Another example
> >> is they also support writing a cookie that can be read by bootloader
> >> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> >> vendor reset types to be implemented without requiring drivers for every
> >> register/cookie.
> >> 
> >> Add support in PSCI to statically map reboot mode commands from
> >> userspace to a vendor reset and cookie value using the device tree.
> >
> > I have managed to discuss a little bit this patchset over the last
> > few days and I think we have defined a plan going forward.
> >
> > A point that was raised is:
> >
> > https://man7.org/linux/man-pages/man2/reboot.2.html
> >
> > LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> > represent ?
> >
> > Is it the mode the system should reboot into OR it is the
> > actual command to be issued (which is what this patchset
> > implements) ?
> >
> > LINUX_REBOOT_CMD_RESTART "..a default restart..."
> >
> > It is unclear what "default" means. We wonder whether the
> > reboot_mode variable was introduced to _define_ that "default".
> 
> I think the reboot_mode predates the 'cmd' argument: linux-2.1.30
> introduced LINUX_REBOOT_CMD_RESTART2 and it already had
> the warm/cold/bios/hard options for i386 reboot. I think the
> argument went unused for a while after it got introduced though.
> 
> > So, in short, my aim is trying to decouple reboot_mode from the
> > LINUX_REBOOT_CMD_RESTART2 *arg command.
> >
> > I believe that adding a sysfs interface to reboot-mode driver
> > infrastructure would be useful, so that the commands would
> > be exposed to userspace and userspace can set the *arg command
> > specifically to issue a given reset/mode.
> >
> > I wonder why this is not already in place for eg syscon-reboot-mode
> > resets, how does user space issue a command in those systems if the
> > available commands aren't exposed to userspace ?
> >
> > Is there a kernel entity exposing those "modes" to userspace, somehow ?
> 
> Don't know one either.
> 
> >> A separate initcall is needed to parse the devicetree, instead of using
> >> psci_dt_init because mm isn't sufficiently set up to allocate memory.
> >> 
> >> Reboot mode framework is close but doesn't quite fit with the
> >> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> >> be solved but doesn't seem reasonable in sum:
> >>  1. reboot mode registers against the reboot_notifier_list, which is too
> >>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> >>     type from the reboot-mode framework callback and use it
> >>     psci_sys_reset.
> >>  2. reboot mode assumes only one cookie/parameter is described in the
> >>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> >>     cookie.
> >
> > This can be changed and I think it should, so that the reboot modes
> > are exposed to user space and PSCI can use that.
> 
> Can we try to call them 'arguments' rather than 'modes' while discussing?
> I think it's way too easy to confuse them otherwise.
> 
> >> +	psci_np = of_find_matching_node(NULL, psci_of_match);
> >> +	if (!psci_np)
> >> +		return 0;
> >> +
> >> +	np = of_find_node_by_name(psci_np, "reset-types");
> >> +	if (!np)
> >> +		return 0;
> >
> > Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> > is the actual reset to be issued, should we add a default mode "cold"
> > and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
> >
> > It all boils down to what *arg represents - adding "cold" and "warm"
> > modes would remove the dependency on reboot_mode for resets issued
> > through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> > is the correct thing to do.
> >
> > Comments very welcome.
> 
> It would make some sense to me to treat all psci reboot as "warm" and
> not do anything here if reboot="cold" is set: those would have to
> be backed by a hardware specific driver.

The patch implements PSCI SYSTEM_RESET2 vendor reset types which seems
independent of reboot_mode (warm/cold/etc) as per specs.  As per ARM
PSCI SPEC:
https://documentation-service.arm.com/static/60feb9bf3d73a34b640e1b67%3Ftoken%3D&ved=2ahUKEwiX7ramjZCGAxUXrlYBHaPaBX4QFnoECBUQAQ&usg=AOvVaw1L35eQniULNRstfGKX5KXp

quoted from spec: -- 

""
5.1.11 

SYSTEM_RESET2 This function extends SYSTEM_RESET.

It provides:
	• Architectural reset definitions.
	• Support for vendor-specific resets.

Bit[31] Reserved must be zero. 
	o Set to 1 for vendor-specific resets.
	o Set to 0 for architectural resets.

Bits[30:0]
	o For vendor-specific resets, the format of these bits is IMPLEMENTATION DEFINED. 
	o For architectural resets, the following values are defined:
		- 0x0 SYSTEM_WARM_RESET.
		- All other values are reserved.

""

So, for vendor specific reset types it does not specify warm or cold.

-Mukesh

> 
> A related problem is how to determine when to use UEFI reboot: at the
> moment arm64 tries the UEFI runtime interface before it even attempts
> any of the notifiers, so PSCI reboot won't ever be used if UEFI is
> present.
> 
>      Arnd

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Mukesh Ojha]

On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> > From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> > 
> > SoC vendors have different types of resets and are controlled through
> > various registers. For instance, Qualcomm chipsets can reboot to a
> > "download mode" that allows a RAM dump to be collected. Another example
> > is they also support writing a cookie that can be read by bootloader
> > during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> > vendor reset types to be implemented without requiring drivers for every
> > register/cookie.
> > 
> > Add support in PSCI to statically map reboot mode commands from
> > userspace to a vendor reset and cookie value using the device tree.
> 
> I have managed to discuss a little bit this patchset over the last
> few days and I think we have defined a plan going forward.
> 
> A point that was raised is:
> 
> https://man7.org/linux/man-pages/man2/reboot.2.html
> 
> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> represent ?
> 
> Is it the mode the system should reboot into OR it is the
> actual command to be issued (which is what this patchset
> implements) ?
> 
> LINUX_REBOOT_CMD_RESTART "..a default restart..."
> 
> It is unclear what "default" means. We wonder whether the
> reboot_mode variable was introduced to _define_ that "default".
> 
> So, in short, my aim is trying to decouple reboot_mode from the
> LINUX_REBOOT_CMD_RESTART2 *arg command.
> 
> I believe that adding a sysfs interface to reboot-mode driver
> infrastructure would be useful, so that the commands would
> be exposed to userspace and userspace can set the *arg command
> specifically to issue a given reset/mode.
> 
> I wonder why this is not already in place for eg syscon-reboot-mode
> resets, how does user space issue a command in those systems if the
> available commands aren't exposed to userspace ?
> 
> Is there a kernel entity exposing those "modes" to userspace, somehow ?
> 
> > A separate initcall is needed to parse the devicetree, instead of using
> > psci_dt_init because mm isn't sufficiently set up to allocate memory.
> > 
> > Reboot mode framework is close but doesn't quite fit with the
> > design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> > be solved but doesn't seem reasonable in sum:
> >  1. reboot mode registers against the reboot_notifier_list, which is too
> >     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> >     type from the reboot-mode framework callback and use it
> >     psci_sys_reset.
> >  2. reboot mode assumes only one cookie/parameter is described in the
> >     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> >     cookie.
> 
> This can be changed and I think it should, so that the reboot modes
> are exposed to user space and PSCI can use that.
> 
In the case of a regular reboot or panic, the reboot/panic notifiers run
first, followed by the restart notifiers. The PSCI reset/reset2 should
be the last call from Linux, and ideally, this call should not fail.

Reboot mode notifiers => restart notifiers or Panic notifiers => restart
notifiers

So, if I understand correctly, you mean that we can change the reboot
mode framework to expose the arguments available to user space. We can
extend it to accept magic and cookies, save them in the reboot
framework, and retrieve them via a call from PSCI during a regular
reboot or panic based on the current arguments. Is this leading towards
writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
notifier callback saves the magic and cookies, and these magic and
cookies will be used during psci_sys_reset2()? Or is there something
wrong with my understanding?

P.S. We appreciate Elliot for his work and follow-up on this while being
employed at Qualcomm.

> >  3. psci cpuidle driver already registers a driver against the
> >     arm,psci-1.0 compatible. Refactoring would be needed to have both a
> >     cpuidle and reboot-mode driver.
> > 
> > Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
> > ---
> >  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 105 insertions(+)
> > 
> > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
> > --- a/drivers/firmware/psci/psci.c
> > +++ b/drivers/firmware/psci/psci.c
> > @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
> >  static bool psci_system_reset2_supported;
> >  static bool psci_system_off2_hibernate_supported;
> >  
> > +struct psci_reset_param {
> > +	const char *mode;
> > +	u32 reset_type;
> > +	u32 cookie;
> > +};
> > +static struct psci_reset_param *psci_reset_params __ro_after_init;
> > +static size_t num_psci_reset_params __ro_after_init;
> > +
> >  static inline bool psci_has_ext_power_state(void)
> >  {
> >  	return psci_cpu_suspend_feature &
> > @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
> >  	return 0;
> >  }
> >  
> > +static int psci_vendor_system_reset2(const char *cmd)
> > +{
> > +	unsigned long ret;
> > +	size_t i;
> > +
> > +	for (i = 0; i < num_psci_reset_params; i++) {
> > +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > +					     psci_reset_params[i].reset_type,
> > +					     psci_reset_params[i].cookie, 0);
> > +			/*
> > +			 * if vendor reset fails, log it and fall back to
> > +			 * architecture reset types
> 
> That's not what the code does.
> 
Ack.

-Mukesh

> > +			 */
> > +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
> > +			       (long)ret);
> > +			return 0;
> > +		}
> > +	}
> > +
> > +	return -ENOENT;
> > +}
> > +
> >  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> >  			  void *data)
> >  {
> > +	/*
> > +	 * try to do the vendor system_reset2
> > +	 * If there wasn't a matching command, fall back to architectural resets
> > +	 */
> > +	if (data && !psci_vendor_system_reset2(data))
> > +		return NOTIFY_DONE;
> > +
> >  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >  	    psci_system_reset2_supported) {
> >  		/*
> > @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
> >  	{},
> >  };
> >  
> > +#define REBOOT_PREFIX "mode-"
> > +
> > +static int __init psci_init_system_reset2_modes(void)
> > +{
> > +	const size_t len = strlen(REBOOT_PREFIX);
> > +	struct psci_reset_param *param;
> > +	struct device_node *psci_np __free(device_node) = NULL;
> > +	struct device_node *np __free(device_node) = NULL;
> > +	struct property *prop;
> > +	size_t count = 0;
> > +	u32 magic[2];
> > +	int num;
> > +
> > +	if (!psci_system_reset2_supported)
> > +		return 0;
> > +
> > +	psci_np = of_find_matching_node(NULL, psci_of_match);
> > +	if (!psci_np)
> > +		return 0;
> > +
> > +	np = of_find_node_by_name(psci_np, "reset-types");
> > +	if (!np)
> > +		return 0;
> 
> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> is the actual reset to be issued, should we add a default mode "cold"
> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
> 
> It all boils down to what *arg represents - adding "cold" and "warm"
> modes would remove the dependency on reboot_mode for resets issued
> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> is the correct thing to do.
> 
> Comments very welcome.
> 
> Thanks,
> Lorenzo
> 
> > +
> > +	for_each_property_of_node(np, prop) {
> > +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> > +			continue;
> > +		num = of_property_count_u32_elems(np, prop->name);
> > +		if (num != 1 && num != 2)
> > +			continue;
> > +
> > +		count++;
> > +	}
> > +
> > +	param = psci_reset_params =
> > +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
> > +	if (!psci_reset_params)
> > +		return -ENOMEM;
> > +
> > +	for_each_property_of_node(np, prop) {
> > +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> > +			continue;
> > +
> > +		num = of_property_read_variable_u32_array(np, prop->name, magic,
> > +							  1, ARRAY_SIZE(magic));
> > +		if (num < 0) {
> > +			pr_warn("Failed to parse vendor reboot mode %s\n",
> > +				param->mode);
> > +			kfree_const(param->mode);
> > +			continue;
> > +		}
> > +
> > +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> > +		if (!param->mode)
> > +			continue;
> > +
> > +		/* Force reset type to be in vendor space */
> > +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
> > +		param->cookie = num > 1 ? magic[1] : 0;
> > +		param++;
> > +		num_psci_reset_params++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +arch_initcall(psci_init_system_reset2_modes);
> > +
> >  int __init psci_dt_init(void)
> >  {
> >  	struct device_node *np;
> > 
> > -- 
> > 2.34.1
> > 

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Mukesh Ojha]

On Tue, Mar 25, 2025 at 7:33 PM Mukesh Ojha
<mukesh.ojha@oss.qualcomm.com> wrote:
>
> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> > > From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> > >
> > > SoC vendors have different types of resets and are controlled through
> > > various registers. For instance, Qualcomm chipsets can reboot to a
> > > "download mode" that allows a RAM dump to be collected. Another example
> > > is they also support writing a cookie that can be read by bootloader
> > > during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> > > vendor reset types to be implemented without requiring drivers for every
> > > register/cookie.
> > >
> > > Add support in PSCI to statically map reboot mode commands from
> > > userspace to a vendor reset and cookie value using the device tree.
> >
> > I have managed to discuss a little bit this patchset over the last
> > few days and I think we have defined a plan going forward.
> >
> > A point that was raised is:
> >
> > https://man7.org/linux/man-pages/man2/reboot.2.html
> >
> > LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> > represent ?
> >
> > Is it the mode the system should reboot into OR it is the
> > actual command to be issued (which is what this patchset
> > implements) ?
> >
> > LINUX_REBOOT_CMD_RESTART "..a default restart..."
> >
> > It is unclear what "default" means. We wonder whether the
> > reboot_mode variable was introduced to _define_ that "default".
> >
> > So, in short, my aim is trying to decouple reboot_mode from the
> > LINUX_REBOOT_CMD_RESTART2 *arg command.
> >
> > I believe that adding a sysfs interface to reboot-mode driver
> > infrastructure would be useful, so that the commands would
> > be exposed to userspace and userspace can set the *arg command
> > specifically to issue a given reset/mode.
> >
> > I wonder why this is not already in place for eg syscon-reboot-mode
> > resets, how does user space issue a command in those systems if the
> > available commands aren't exposed to userspace ?
> >
> > Is there a kernel entity exposing those "modes" to userspace, somehow ?
> >
> > > A separate initcall is needed to parse the devicetree, instead of using
> > > psci_dt_init because mm isn't sufficiently set up to allocate memory.
> > >
> > > Reboot mode framework is close but doesn't quite fit with the
> > > design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> > > be solved but doesn't seem reasonable in sum:
> > >  1. reboot mode registers against the reboot_notifier_list, which is too
> > >     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> > >     type from the reboot-mode framework callback and use it
> > >     psci_sys_reset.
> > >  2. reboot mode assumes only one cookie/parameter is described in the
> > >     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> > >     cookie.
> >
> > This can be changed and I think it should, so that the reboot modes
> > are exposed to user space and PSCI can use that.
> >
> In the case of a regular reboot or panic, the reboot/panic notifiers run
> first, followed by the restart notifiers. The PSCI reset/reset2 should
> be the last call from Linux, and ideally, this call should not fail.
>
> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
> notifiers
>
> So, if I understand correctly, you mean that we can change the reboot
> mode framework to expose the arguments available to user space. We can
> extend it to accept magic and cookies, save them in the reboot
> framework, and retrieve them via a call from PSCI during a regular
> reboot or panic based on the current arguments. Is this leading towards
> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
> notifier callback saves the magic and cookies, and these magic and
> cookies will be used during psci_sys_reset2()? Or is there something
> wrong with my understanding?

In case this mail got lost in your inbox, I wanted to float this up
again as this is a very important ARM feature and
its future course of action depends on the common understanding.

-Mukesh

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Lorenzo Pieralisi]

On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> > > From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> > > 
> > > SoC vendors have different types of resets and are controlled through
> > > various registers. For instance, Qualcomm chipsets can reboot to a
> > > "download mode" that allows a RAM dump to be collected. Another example
> > > is they also support writing a cookie that can be read by bootloader
> > > during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> > > vendor reset types to be implemented without requiring drivers for every
> > > register/cookie.
> > > 
> > > Add support in PSCI to statically map reboot mode commands from
> > > userspace to a vendor reset and cookie value using the device tree.
> > 
> > I have managed to discuss a little bit this patchset over the last
> > few days and I think we have defined a plan going forward.
> > 
> > A point that was raised is:
> > 
> > https://man7.org/linux/man-pages/man2/reboot.2.html
> > 
> > LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> > represent ?
> > 
> > Is it the mode the system should reboot into OR it is the
> > actual command to be issued (which is what this patchset
> > implements) ?
> > 
> > LINUX_REBOOT_CMD_RESTART "..a default restart..."
> > 
> > It is unclear what "default" means. We wonder whether the
> > reboot_mode variable was introduced to _define_ that "default".
> > 
> > So, in short, my aim is trying to decouple reboot_mode from the
> > LINUX_REBOOT_CMD_RESTART2 *arg command.
> > 
> > I believe that adding a sysfs interface to reboot-mode driver
> > infrastructure would be useful, so that the commands would
> > be exposed to userspace and userspace can set the *arg command
> > specifically to issue a given reset/mode.
> > 
> > I wonder why this is not already in place for eg syscon-reboot-mode
> > resets, how does user space issue a command in those systems if the
> > available commands aren't exposed to userspace ?
> > 
> > Is there a kernel entity exposing those "modes" to userspace, somehow ?
> > 
> > > A separate initcall is needed to parse the devicetree, instead of using
> > > psci_dt_init because mm isn't sufficiently set up to allocate memory.
> > > 
> > > Reboot mode framework is close but doesn't quite fit with the
> > > design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> > > be solved but doesn't seem reasonable in sum:
> > >  1. reboot mode registers against the reboot_notifier_list, which is too
> > >     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> > >     type from the reboot-mode framework callback and use it
> > >     psci_sys_reset.
> > >  2. reboot mode assumes only one cookie/parameter is described in the
> > >     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> > >     cookie.
> > 
> > This can be changed and I think it should, so that the reboot modes
> > are exposed to user space and PSCI can use that.
> > 
> In the case of a regular reboot or panic, the reboot/panic notifiers run
> first, followed by the restart notifiers. The PSCI reset/reset2 should
> be the last call from Linux, and ideally, this call should not fail.
> 
> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
> notifiers
> 
> So, if I understand correctly, you mean that we can change the reboot
> mode framework to expose the arguments available to user space. We can
> extend it to accept magic and cookies, save them in the reboot
> framework, and retrieve them via a call from PSCI during a regular
> reboot or panic based on the current arguments. Is this leading towards
> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
> notifier callback saves the magic and cookies, and these magic and
> cookies will be used during psci_sys_reset2()? Or is there something
> wrong with my understanding?

No, you got it right (apologies for the delay in replying) - if the
case for making reboot mode available to user space is accepted.

> P.S. We appreciate Elliot for his work and follow-up on this while being
> employed at Qualcomm.

Yes I sincerely do for his patience, thank you.

Lorenzo

> > >  3. psci cpuidle driver already registers a driver against the
> > >     arm,psci-1.0 compatible. Refactoring would be needed to have both a
> > >     cpuidle and reboot-mode driver.
> > > 
> > > Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
> > > ---
> > >  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 105 insertions(+)
> > > 
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
> > >  static bool psci_system_reset2_supported;
> > >  static bool psci_system_off2_hibernate_supported;
> > >  
> > > +struct psci_reset_param {
> > > +	const char *mode;
> > > +	u32 reset_type;
> > > +	u32 cookie;
> > > +};
> > > +static struct psci_reset_param *psci_reset_params __ro_after_init;
> > > +static size_t num_psci_reset_params __ro_after_init;
> > > +
> > >  static inline bool psci_has_ext_power_state(void)
> > >  {
> > >  	return psci_cpu_suspend_feature &
> > > @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
> > >  	return 0;
> > >  }
> > >  
> > > +static int psci_vendor_system_reset2(const char *cmd)
> > > +{
> > > +	unsigned long ret;
> > > +	size_t i;
> > > +
> > > +	for (i = 0; i < num_psci_reset_params; i++) {
> > > +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > +					     psci_reset_params[i].reset_type,
> > > +					     psci_reset_params[i].cookie, 0);
> > > +			/*
> > > +			 * if vendor reset fails, log it and fall back to
> > > +			 * architecture reset types
> > 
> > That's not what the code does.
> > 
> Ack.
> 
> -Mukesh
> 
> > > +			 */
> > > +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
> > > +			       (long)ret);
> > > +			return 0;
> > > +		}
> > > +	}
> > > +
> > > +	return -ENOENT;
> > > +}
> > > +
> > >  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > >  			  void *data)
> > >  {
> > > +	/*
> > > +	 * try to do the vendor system_reset2
> > > +	 * If there wasn't a matching command, fall back to architectural resets
> > > +	 */
> > > +	if (data && !psci_vendor_system_reset2(data))
> > > +		return NOTIFY_DONE;
> > > +
> > >  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> > >  	    psci_system_reset2_supported) {
> > >  		/*
> > > @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
> > >  	{},
> > >  };
> > >  
> > > +#define REBOOT_PREFIX "mode-"
> > > +
> > > +static int __init psci_init_system_reset2_modes(void)
> > > +{
> > > +	const size_t len = strlen(REBOOT_PREFIX);
> > > +	struct psci_reset_param *param;
> > > +	struct device_node *psci_np __free(device_node) = NULL;
> > > +	struct device_node *np __free(device_node) = NULL;
> > > +	struct property *prop;
> > > +	size_t count = 0;
> > > +	u32 magic[2];
> > > +	int num;
> > > +
> > > +	if (!psci_system_reset2_supported)
> > > +		return 0;
> > > +
> > > +	psci_np = of_find_matching_node(NULL, psci_of_match);
> > > +	if (!psci_np)
> > > +		return 0;
> > > +
> > > +	np = of_find_node_by_name(psci_np, "reset-types");
> > > +	if (!np)
> > > +		return 0;
> > 
> > Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> > is the actual reset to be issued, should we add a default mode "cold"
> > and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
> > 
> > It all boils down to what *arg represents - adding "cold" and "warm"
> > modes would remove the dependency on reboot_mode for resets issued
> > through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> > is the correct thing to do.
> > 
> > Comments very welcome.
> > 
> > Thanks,
> > Lorenzo
> > 
> > > +
> > > +	for_each_property_of_node(np, prop) {
> > > +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> > > +			continue;
> > > +		num = of_property_count_u32_elems(np, prop->name);
> > > +		if (num != 1 && num != 2)
> > > +			continue;
> > > +
> > > +		count++;
> > > +	}
> > > +
> > > +	param = psci_reset_params =
> > > +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
> > > +	if (!psci_reset_params)
> > > +		return -ENOMEM;
> > > +
> > > +	for_each_property_of_node(np, prop) {
> > > +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> > > +			continue;
> > > +
> > > +		num = of_property_read_variable_u32_array(np, prop->name, magic,
> > > +							  1, ARRAY_SIZE(magic));
> > > +		if (num < 0) {
> > > +			pr_warn("Failed to parse vendor reboot mode %s\n",
> > > +				param->mode);
> > > +			kfree_const(param->mode);
> > > +			continue;
> > > +		}
> > > +
> > > +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> > > +		if (!param->mode)
> > > +			continue;
> > > +
> > > +		/* Force reset type to be in vendor space */
> > > +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
> > > +		param->cookie = num > 1 ? magic[1] : 0;
> > > +		param++;
> > > +		num_psci_reset_params++;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +arch_initcall(psci_init_system_reset2_modes);
> > > +
> > >  int __init psci_dt_init(void)
> > >  {
> > >  	struct device_node *np;
> > > 
> > > -- 
> > > 2.34.1
> > > 

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Shivendra Pratap]

On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>
>>>> SoC vendors have different types of resets and are controlled through
>>>> various registers. For instance, Qualcomm chipsets can reboot to a
>>>> "download mode" that allows a RAM dump to be collected. Another example
>>>> is they also support writing a cookie that can be read by bootloader
>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>>>> vendor reset types to be implemented without requiring drivers for every
>>>> register/cookie.
>>>>
>>>> Add support in PSCI to statically map reboot mode commands from
>>>> userspace to a vendor reset and cookie value using the device tree.
>>>
>>> I have managed to discuss a little bit this patchset over the last
>>> few days and I think we have defined a plan going forward.
>>>
>>> A point that was raised is:
>>>
>>> https://man7.org/linux/man-pages/man2/reboot.2.html
>>>
>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
>>> represent ?
>>>
>>> Is it the mode the system should reboot into OR it is the
>>> actual command to be issued (which is what this patchset
>>> implements) ?
>>>
>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>>>
>>> It is unclear what "default" means. We wonder whether the
>>> reboot_mode variable was introduced to _define_ that "default".
>>>
>>> So, in short, my aim is trying to decouple reboot_mode from the
>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
>>>
>>> I believe that adding a sysfs interface to reboot-mode driver
>>> infrastructure would be useful, so that the commands would
>>> be exposed to userspace and userspace can set the *arg command
>>> specifically to issue a given reset/mode.
>>>
>>> I wonder why this is not already in place for eg syscon-reboot-mode
>>> resets, how does user space issue a command in those systems if the
>>> available commands aren't exposed to userspace ?
>>>
>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
>>>
>>>> A separate initcall is needed to parse the devicetree, instead of using
>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>>>>
>>>> Reboot mode framework is close but doesn't quite fit with the
>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>>>> be solved but doesn't seem reasonable in sum:
>>>>  1. reboot mode registers against the reboot_notifier_list, which is too
>>>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>>>     type from the reboot-mode framework callback and use it
>>>>     psci_sys_reset.
>>>>  2. reboot mode assumes only one cookie/parameter is described in the
>>>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>>>     cookie.
>>>
>>> This can be changed and I think it should, so that the reboot modes
>>> are exposed to user space and PSCI can use that.
>>>
>> In the case of a regular reboot or panic, the reboot/panic notifiers run
>> first, followed by the restart notifiers. The PSCI reset/reset2 should
>> be the last call from Linux, and ideally, this call should not fail.
>>
>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
>> notifiers
>>
>> So, if I understand correctly, you mean that we can change the reboot
>> mode framework to expose the arguments available to user space. We can
>> extend it to accept magic and cookies, save them in the reboot
>> framework, and retrieve them via a call from PSCI during a regular
>> reboot or panic based on the current arguments. Is this leading towards
>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
>> notifier callback saves the magic and cookies, and these magic and
>> cookies will be used during psci_sys_reset2()? Or is there something
>> wrong with my understanding?
> 
> No, you got it right (apologies for the delay in replying) - if the
> case for making reboot mode available to user space is accepted.
> 

Agree that the available modes should be exposed to usespace via sysfs interface
and we should implement it. Also #1 and #2 can be handled via some
changes in the design as mentioned in above discussion.

I have one doubt though when we implement this via reboot-mode framework.
The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
psci driver is initialized very early at boot but potential ARM psci reboot-mode
driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
types functionality will not be available in psci reset path until the reboot-mode
driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
types for early device resets?

One use-case may be an early device crash or a early reset where a vendor 
wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a 
specific state but may not be able to use this driver.
(eg: a kernel panic at early boot where a vendor wants to reset device 
to a specific state using vendor reset. Currently panic passes a NULL
(*arg command) while device reset but it may be explored for vendor specific
reset).

- Shivendra

>> P.S. We appreciate Elliot for his work and follow-up on this while being
>> employed at Qualcomm.
> 
> Yes I sincerely do for his patience, thank you.
> 
> Lorenzo
> 
>>>>  3. psci cpuidle driver already registers a driver against the
>>>>     arm,psci-1.0 compatible. Refactoring would be needed to have both a
>>>>     cpuidle and reboot-mode driver.
>>>>
>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>> ---
>>>>  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 105 insertions(+)
>>>>
>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
>>>> --- a/drivers/firmware/psci/psci.c
>>>> +++ b/drivers/firmware/psci/psci.c
>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>>>>  static bool psci_system_reset2_supported;
>>>>  static bool psci_system_off2_hibernate_supported;
>>>>  
>>>> +struct psci_reset_param {
>>>> +	const char *mode;
>>>> +	u32 reset_type;
>>>> +	u32 cookie;
>>>> +};
>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
>>>> +static size_t num_psci_reset_params __ro_after_init;
>>>> +
>>>>  static inline bool psci_has_ext_power_state(void)
>>>>  {
>>>>  	return psci_cpu_suspend_feature &
>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static int psci_vendor_system_reset2(const char *cmd)
>>>> +{
>>>> +	unsigned long ret;
>>>> +	size_t i;
>>>> +
>>>> +	for (i = 0; i < num_psci_reset_params; i++) {
>>>> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>> +					     psci_reset_params[i].reset_type,
>>>> +					     psci_reset_params[i].cookie, 0);
>>>> +			/*
>>>> +			 * if vendor reset fails, log it and fall back to
>>>> +			 * architecture reset types
>>>
>>> That's not what the code does.
>>>
>> Ack.
>>
>> -Mukesh
>>
>>>> +			 */
>>>> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
>>>> +			       (long)ret);
>>>> +			return 0;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return -ENOENT;
>>>> +}
>>>> +
>>>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>  			  void *data)
>>>>  {
>>>> +	/*
>>>> +	 * try to do the vendor system_reset2
>>>> +	 * If there wasn't a matching command, fall back to architectural resets
>>>> +	 */
>>>> +	if (data && !psci_vendor_system_reset2(data))
>>>> +		return NOTIFY_DONE;
>>>> +
>>>>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>  	    psci_system_reset2_supported) {
>>>>  		/*
>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>>>>  	{},
>>>>  };
>>>>  
>>>> +#define REBOOT_PREFIX "mode-"
>>>> +
>>>> +static int __init psci_init_system_reset2_modes(void)
>>>> +{
>>>> +	const size_t len = strlen(REBOOT_PREFIX);
>>>> +	struct psci_reset_param *param;
>>>> +	struct device_node *psci_np __free(device_node) = NULL;
>>>> +	struct device_node *np __free(device_node) = NULL;
>>>> +	struct property *prop;
>>>> +	size_t count = 0;
>>>> +	u32 magic[2];
>>>> +	int num;
>>>> +
>>>> +	if (!psci_system_reset2_supported)
>>>> +		return 0;
>>>> +
>>>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
>>>> +	if (!psci_np)
>>>> +		return 0;
>>>> +
>>>> +	np = of_find_node_by_name(psci_np, "reset-types");
>>>> +	if (!np)
>>>> +		return 0;
>>>
>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
>>> is the actual reset to be issued, should we add a default mode "cold"
>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>>>
>>> It all boils down to what *arg represents - adding "cold" and "warm"
>>> modes would remove the dependency on reboot_mode for resets issued
>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
>>> is the correct thing to do.
>>>
>>> Comments very welcome.
>>>
>>> Thanks,
>>> Lorenzo
>>>
>>>> +
>>>> +	for_each_property_of_node(np, prop) {
>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>> +			continue;
>>>> +		num = of_property_count_u32_elems(np, prop->name);
>>>> +		if (num != 1 && num != 2)
>>>> +			continue;
>>>> +
>>>> +		count++;
>>>> +	}
>>>> +
>>>> +	param = psci_reset_params =
>>>> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
>>>> +	if (!psci_reset_params)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	for_each_property_of_node(np, prop) {
>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>> +			continue;
>>>> +
>>>> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
>>>> +							  1, ARRAY_SIZE(magic));
>>>> +		if (num < 0) {
>>>> +			pr_warn("Failed to parse vendor reboot mode %s\n",
>>>> +				param->mode);
>>>> +			kfree_const(param->mode);
>>>> +			continue;
>>>> +		}
>>>> +
>>>> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>> +		if (!param->mode)
>>>> +			continue;
>>>> +
>>>> +		/* Force reset type to be in vendor space */
>>>> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
>>>> +		param->cookie = num > 1 ? magic[1] : 0;
>>>> +		param++;
>>>> +		num_psci_reset_params++;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +arch_initcall(psci_init_system_reset2_modes);
>>>> +
>>>>  int __init psci_dt_init(void)
>>>>  {
>>>>  	struct device_node *np;
>>>>
>>>> -- 
>>>> 2.34.1
>>>>

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Lorenzo Pieralisi]

On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
> 
> 
> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
> > On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
> >> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
> >>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> >>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >>>>
> >>>> SoC vendors have different types of resets and are controlled through
> >>>> various registers. For instance, Qualcomm chipsets can reboot to a
> >>>> "download mode" that allows a RAM dump to be collected. Another example
> >>>> is they also support writing a cookie that can be read by bootloader
> >>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> >>>> vendor reset types to be implemented without requiring drivers for every
> >>>> register/cookie.
> >>>>
> >>>> Add support in PSCI to statically map reboot mode commands from
> >>>> userspace to a vendor reset and cookie value using the device tree.
> >>>
> >>> I have managed to discuss a little bit this patchset over the last
> >>> few days and I think we have defined a plan going forward.
> >>>
> >>> A point that was raised is:
> >>>
> >>> https://man7.org/linux/man-pages/man2/reboot.2.html
> >>>
> >>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> >>> represent ?
> >>>
> >>> Is it the mode the system should reboot into OR it is the
> >>> actual command to be issued (which is what this patchset
> >>> implements) ?
> >>>
> >>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
> >>>
> >>> It is unclear what "default" means. We wonder whether the
> >>> reboot_mode variable was introduced to _define_ that "default".
> >>>
> >>> So, in short, my aim is trying to decouple reboot_mode from the
> >>> LINUX_REBOOT_CMD_RESTART2 *arg command.
> >>>
> >>> I believe that adding a sysfs interface to reboot-mode driver
> >>> infrastructure would be useful, so that the commands would
> >>> be exposed to userspace and userspace can set the *arg command
> >>> specifically to issue a given reset/mode.
> >>>
> >>> I wonder why this is not already in place for eg syscon-reboot-mode
> >>> resets, how does user space issue a command in those systems if the
> >>> available commands aren't exposed to userspace ?
> >>>
> >>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
> >>>
> >>>> A separate initcall is needed to parse the devicetree, instead of using
> >>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
> >>>>
> >>>> Reboot mode framework is close but doesn't quite fit with the
> >>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> >>>> be solved but doesn't seem reasonable in sum:
> >>>>  1. reboot mode registers against the reboot_notifier_list, which is too
> >>>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> >>>>     type from the reboot-mode framework callback and use it
> >>>>     psci_sys_reset.
> >>>>  2. reboot mode assumes only one cookie/parameter is described in the
> >>>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> >>>>     cookie.
> >>>
> >>> This can be changed and I think it should, so that the reboot modes
> >>> are exposed to user space and PSCI can use that.
> >>>
> >> In the case of a regular reboot or panic, the reboot/panic notifiers run
> >> first, followed by the restart notifiers. The PSCI reset/reset2 should
> >> be the last call from Linux, and ideally, this call should not fail.
> >>
> >> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
> >> notifiers
> >>
> >> So, if I understand correctly, you mean that we can change the reboot
> >> mode framework to expose the arguments available to user space. We can
> >> extend it to accept magic and cookies, save them in the reboot
> >> framework, and retrieve them via a call from PSCI during a regular
> >> reboot or panic based on the current arguments. Is this leading towards
> >> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
> >> notifier callback saves the magic and cookies, and these magic and
> >> cookies will be used during psci_sys_reset2()? Or is there something
> >> wrong with my understanding?
> > 
> > No, you got it right (apologies for the delay in replying) - if the
> > case for making reboot mode available to user space is accepted.
> > 
> 
> Agree that the available modes should be exposed to usespace via sysfs interface
> and we should implement it. Also #1 and #2 can be handled via some
> changes in the design as mentioned in above discussion.
> 
> I have one doubt though when we implement this via reboot-mode framework.
> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
> psci driver is initialized very early at boot but potential ARM psci reboot-mode
> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
> types functionality will not be available in psci reset path until the reboot-mode
> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
> types for early device resets?
> 
> One use-case may be an early device crash or a early reset where a vendor 
> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a 
> specific state but may not be able to use this driver.
> (eg: a kernel panic at early boot where a vendor wants to reset device 
> to a specific state using vendor reset. Currently panic passes a NULL
> (*arg command) while device reset but it may be explored for vendor specific
> reset).

As you said, that would not be a PSCI only issue - *if* we wanted to
plug in this use case we should find a way to do it at reboot mode
driver level.

As a matter of fact, this is not a mainline issue AFAICS.

Even if we did not design this as a reboot mode driver there would be a
time window where you would not be able to use vendor resets on panic.

I don't see it as a major roadblock at the moment.

Thanks,
Lorenzo

> 
> - Shivendra
> 
> >> P.S. We appreciate Elliot for his work and follow-up on this while being
> >> employed at Qualcomm.
> > 
> > Yes I sincerely do for his patience, thank you.
> > 
> > Lorenzo
> > 
> >>>>  3. psci cpuidle driver already registers a driver against the
> >>>>     arm,psci-1.0 compatible. Refactoring would be needed to have both a
> >>>>     cpuidle and reboot-mode driver.
> >>>>
> >>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >>>> ---
> >>>>  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 105 insertions(+)
> >>>>
> >>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> >>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
> >>>> --- a/drivers/firmware/psci/psci.c
> >>>> +++ b/drivers/firmware/psci/psci.c
> >>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
> >>>>  static bool psci_system_reset2_supported;
> >>>>  static bool psci_system_off2_hibernate_supported;
> >>>>  
> >>>> +struct psci_reset_param {
> >>>> +	const char *mode;
> >>>> +	u32 reset_type;
> >>>> +	u32 cookie;
> >>>> +};
> >>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
> >>>> +static size_t num_psci_reset_params __ro_after_init;
> >>>> +
> >>>>  static inline bool psci_has_ext_power_state(void)
> >>>>  {
> >>>>  	return psci_cpu_suspend_feature &
> >>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> +static int psci_vendor_system_reset2(const char *cmd)
> >>>> +{
> >>>> +	unsigned long ret;
> >>>> +	size_t i;
> >>>> +
> >>>> +	for (i = 0; i < num_psci_reset_params; i++) {
> >>>> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
> >>>> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> >>>> +					     psci_reset_params[i].reset_type,
> >>>> +					     psci_reset_params[i].cookie, 0);
> >>>> +			/*
> >>>> +			 * if vendor reset fails, log it and fall back to
> >>>> +			 * architecture reset types
> >>>
> >>> That's not what the code does.
> >>>
> >> Ack.
> >>
> >> -Mukesh
> >>
> >>>> +			 */
> >>>> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
> >>>> +			       (long)ret);
> >>>> +			return 0;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	return -ENOENT;
> >>>> +}
> >>>> +
> >>>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> >>>>  			  void *data)
> >>>>  {
> >>>> +	/*
> >>>> +	 * try to do the vendor system_reset2
> >>>> +	 * If there wasn't a matching command, fall back to architectural resets
> >>>> +	 */
> >>>> +	if (data && !psci_vendor_system_reset2(data))
> >>>> +		return NOTIFY_DONE;
> >>>> +
> >>>>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >>>>  	    psci_system_reset2_supported) {
> >>>>  		/*
> >>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
> >>>>  	{},
> >>>>  };
> >>>>  
> >>>> +#define REBOOT_PREFIX "mode-"
> >>>> +
> >>>> +static int __init psci_init_system_reset2_modes(void)
> >>>> +{
> >>>> +	const size_t len = strlen(REBOOT_PREFIX);
> >>>> +	struct psci_reset_param *param;
> >>>> +	struct device_node *psci_np __free(device_node) = NULL;
> >>>> +	struct device_node *np __free(device_node) = NULL;
> >>>> +	struct property *prop;
> >>>> +	size_t count = 0;
> >>>> +	u32 magic[2];
> >>>> +	int num;
> >>>> +
> >>>> +	if (!psci_system_reset2_supported)
> >>>> +		return 0;
> >>>> +
> >>>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
> >>>> +	if (!psci_np)
> >>>> +		return 0;
> >>>> +
> >>>> +	np = of_find_node_by_name(psci_np, "reset-types");
> >>>> +	if (!np)
> >>>> +		return 0;
> >>>
> >>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> >>> is the actual reset to be issued, should we add a default mode "cold"
> >>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
> >>>
> >>> It all boils down to what *arg represents - adding "cold" and "warm"
> >>> modes would remove the dependency on reboot_mode for resets issued
> >>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> >>> is the correct thing to do.
> >>>
> >>> Comments very welcome.
> >>>
> >>> Thanks,
> >>> Lorenzo
> >>>
> >>>> +
> >>>> +	for_each_property_of_node(np, prop) {
> >>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> >>>> +			continue;
> >>>> +		num = of_property_count_u32_elems(np, prop->name);
> >>>> +		if (num != 1 && num != 2)
> >>>> +			continue;
> >>>> +
> >>>> +		count++;
> >>>> +	}
> >>>> +
> >>>> +	param = psci_reset_params =
> >>>> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
> >>>> +	if (!psci_reset_params)
> >>>> +		return -ENOMEM;
> >>>> +
> >>>> +	for_each_property_of_node(np, prop) {
> >>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> >>>> +			continue;
> >>>> +
> >>>> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
> >>>> +							  1, ARRAY_SIZE(magic));
> >>>> +		if (num < 0) {
> >>>> +			pr_warn("Failed to parse vendor reboot mode %s\n",
> >>>> +				param->mode);
> >>>> +			kfree_const(param->mode);
> >>>> +			continue;
> >>>> +		}
> >>>> +
> >>>> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> >>>> +		if (!param->mode)
> >>>> +			continue;
> >>>> +
> >>>> +		/* Force reset type to be in vendor space */
> >>>> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
> >>>> +		param->cookie = num > 1 ? magic[1] : 0;
> >>>> +		param++;
> >>>> +		num_psci_reset_params++;
> >>>> +	}
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>>> +arch_initcall(psci_init_system_reset2_modes);
> >>>> +
> >>>>  int __init psci_dt_init(void)
> >>>>  {
> >>>>  	struct device_node *np;
> >>>>
> >>>> -- 
> >>>> 2.34.1
> >>>>

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Shivendra Pratap]

On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
> On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
>>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
>>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
>>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>
>>>>>> SoC vendors have different types of resets and are controlled through
>>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
>>>>>> "download mode" that allows a RAM dump to be collected. Another example
>>>>>> is they also support writing a cookie that can be read by bootloader
>>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>>>>>> vendor reset types to be implemented without requiring drivers for every
>>>>>> register/cookie.
>>>>>>
>>>>>> Add support in PSCI to statically map reboot mode commands from
>>>>>> userspace to a vendor reset and cookie value using the device tree.
>>>>>
>>>>> I have managed to discuss a little bit this patchset over the last
>>>>> few days and I think we have defined a plan going forward.
>>>>>
>>>>> A point that was raised is:
>>>>>
>>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
>>>>>
>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
>>>>> represent ?
>>>>>
>>>>> Is it the mode the system should reboot into OR it is the
>>>>> actual command to be issued (which is what this patchset
>>>>> implements) ?
>>>>>
>>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>>>>>
>>>>> It is unclear what "default" means. We wonder whether the
>>>>> reboot_mode variable was introduced to _define_ that "default".
>>>>>
>>>>> So, in short, my aim is trying to decouple reboot_mode from the
>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
>>>>>
>>>>> I believe that adding a sysfs interface to reboot-mode driver
>>>>> infrastructure would be useful, so that the commands would
>>>>> be exposed to userspace and userspace can set the *arg command
>>>>> specifically to issue a given reset/mode.
>>>>>
>>>>> I wonder why this is not already in place for eg syscon-reboot-mode
>>>>> resets, how does user space issue a command in those systems if the
>>>>> available commands aren't exposed to userspace ?
>>>>>
>>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
>>>>>
>>>>>> A separate initcall is needed to parse the devicetree, instead of using
>>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>>>>>>
>>>>>> Reboot mode framework is close but doesn't quite fit with the
>>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>>>>>> be solved but doesn't seem reasonable in sum:
>>>>>>  1. reboot mode registers against the reboot_notifier_list, which is too
>>>>>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>>>>>     type from the reboot-mode framework callback and use it
>>>>>>     psci_sys_reset.
>>>>>>  2. reboot mode assumes only one cookie/parameter is described in the
>>>>>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>>>>>     cookie.
>>>>>
>>>>> This can be changed and I think it should, so that the reboot modes
>>>>> are exposed to user space and PSCI can use that.
>>>>>
>>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
>>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
>>>> be the last call from Linux, and ideally, this call should not fail.
>>>>
>>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
>>>> notifiers
>>>>
>>>> So, if I understand correctly, you mean that we can change the reboot
>>>> mode framework to expose the arguments available to user space. We can
>>>> extend it to accept magic and cookies, save them in the reboot
>>>> framework, and retrieve them via a call from PSCI during a regular
>>>> reboot or panic based on the current arguments. Is this leading towards
>>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
>>>> notifier callback saves the magic and cookies, and these magic and
>>>> cookies will be used during psci_sys_reset2()? Or is there something
>>>> wrong with my understanding?
>>>
>>> No, you got it right (apologies for the delay in replying) - if the
>>> case for making reboot mode available to user space is accepted.
>>>
While moving this into reboot-mode framework, one more query came up.
The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
to be a Platform device driver for using reboot-mode framework.
As psci is not a platform device driver, a subdevice under it may not probe as a
platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
early_initcall("psci_xyz") and then create a platform device something as
below or any other suggestions for this?

power:reset:<psci-vendor-reset-driver>:
-----
static int __init psci_vendor_reset_init(void) {
..
..
	np = of_find_node_by_name(NULL, "psci-vendor-reset");
	if(!np)
		return -ENODEV;
	pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
..
..
}
-------

the sysfs we will expose from reboot-mode may show like below in above 
implementation:

###### 
/ # cat ./sys/devices/platform/psci-vendor-reset/available_modes
bootloader edl 
###### 

thanks,
Shivendra

>>
>> Agree that the available modes should be exposed to usespace via sysfs interface
>> and we should implement it. Also #1 and #2 can be handled via some
>> changes in the design as mentioned in above discussion.
>>
>> I have one doubt though when we implement this via reboot-mode framework.
>> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
>> psci driver is initialized very early at boot but potential ARM psci reboot-mode
>> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
>> types functionality will not be available in psci reset path until the reboot-mode
>> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
>> types for early device resets?
>>
>> One use-case may be an early device crash or a early reset where a vendor 
>> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a 
>> specific state but may not be able to use this driver.
>> (eg: a kernel panic at early boot where a vendor wants to reset device 
>> to a specific state using vendor reset. Currently panic passes a NULL
>> (*arg command) while device reset but it may be explored for vendor specific
>> reset).
> 
> As you said, that would not be a PSCI only issue - *if* we wanted to
> plug in this use case we should find a way to do it at reboot mode
> driver level.
> 
> As a matter of fact, this is not a mainline issue AFAICS.
> 
> Even if we did not design this as a reboot mode driver there would be a
> time window where you would not be able to use vendor resets on panic.
> 
> I don't see it as a major roadblock at the moment.
Got it.
> 
> Thanks,
> Lorenzo
> 
>>
>> - Shivendra
>>
>>>> P.S. We appreciate Elliot for his work and follow-up on this while being
>>>> employed at Qualcomm.
>>>
>>> Yes I sincerely do for his patience, thank you.
>>>
>>> Lorenzo
>>>
>>>>>>  3. psci cpuidle driver already registers a driver against the
>>>>>>     arm,psci-1.0 compatible. Refactoring would be needed to have both a
>>>>>>     cpuidle and reboot-mode driver.
>>>>>>
>>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>> ---
>>>>>>  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 105 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
>>>>>> --- a/drivers/firmware/psci/psci.c
>>>>>> +++ b/drivers/firmware/psci/psci.c
>>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>>>>>>  static bool psci_system_reset2_supported;
>>>>>>  static bool psci_system_off2_hibernate_supported;
>>>>>>  
>>>>>> +struct psci_reset_param {
>>>>>> +	const char *mode;
>>>>>> +	u32 reset_type;
>>>>>> +	u32 cookie;
>>>>>> +};
>>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
>>>>>> +static size_t num_psci_reset_params __ro_after_init;
>>>>>> +
>>>>>>  static inline bool psci_has_ext_power_state(void)
>>>>>>  {
>>>>>>  	return psci_cpu_suspend_feature &
>>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> +static int psci_vendor_system_reset2(const char *cmd)
>>>>>> +{
>>>>>> +	unsigned long ret;
>>>>>> +	size_t i;
>>>>>> +
>>>>>> +	for (i = 0; i < num_psci_reset_params; i++) {
>>>>>> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>>>> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>>>> +					     psci_reset_params[i].reset_type,
>>>>>> +					     psci_reset_params[i].cookie, 0);
>>>>>> +			/*
>>>>>> +			 * if vendor reset fails, log it and fall back to
>>>>>> +			 * architecture reset types
>>>>>
>>>>> That's not what the code does.
>>>>>
>>>> Ack.
>>>>
>>>> -Mukesh
>>>>
>>>>>> +			 */
>>>>>> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
>>>>>> +			       (long)ret);
>>>>>> +			return 0;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>> +	return -ENOENT;
>>>>>> +}
>>>>>> +
>>>>>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>>>  			  void *data)
>>>>>>  {
>>>>>> +	/*
>>>>>> +	 * try to do the vendor system_reset2
>>>>>> +	 * If there wasn't a matching command, fall back to architectural resets
>>>>>> +	 */
>>>>>> +	if (data && !psci_vendor_system_reset2(data))
>>>>>> +		return NOTIFY_DONE;
>>>>>> +
>>>>>>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>>>  	    psci_system_reset2_supported) {
>>>>>>  		/*
>>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>>>>>>  	{},
>>>>>>  };
>>>>>>  
>>>>>> +#define REBOOT_PREFIX "mode-"
>>>>>> +
>>>>>> +static int __init psci_init_system_reset2_modes(void)
>>>>>> +{
>>>>>> +	const size_t len = strlen(REBOOT_PREFIX);
>>>>>> +	struct psci_reset_param *param;
>>>>>> +	struct device_node *psci_np __free(device_node) = NULL;
>>>>>> +	struct device_node *np __free(device_node) = NULL;
>>>>>> +	struct property *prop;
>>>>>> +	size_t count = 0;
>>>>>> +	u32 magic[2];
>>>>>> +	int num;
>>>>>> +
>>>>>> +	if (!psci_system_reset2_supported)
>>>>>> +		return 0;
>>>>>> +
>>>>>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
>>>>>> +	if (!psci_np)
>>>>>> +		return 0;
>>>>>> +
>>>>>> +	np = of_find_node_by_name(psci_np, "reset-types");
>>>>>> +	if (!np)
>>>>>> +		return 0;
>>>>>
>>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
>>>>> is the actual reset to be issued, should we add a default mode "cold"
>>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>>>>>
>>>>> It all boils down to what *arg represents - adding "cold" and "warm"
>>>>> modes would remove the dependency on reboot_mode for resets issued
>>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
>>>>> is the correct thing to do.
>>>>>
>>>>> Comments very welcome.
>>>>>
>>>>> Thanks,
>>>>> Lorenzo
>>>>>
>>>>>> +
>>>>>> +	for_each_property_of_node(np, prop) {
>>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>> +			continue;
>>>>>> +		num = of_property_count_u32_elems(np, prop->name);
>>>>>> +		if (num != 1 && num != 2)
>>>>>> +			continue;
>>>>>> +
>>>>>> +		count++;
>>>>>> +	}
>>>>>> +
>>>>>> +	param = psci_reset_params =
>>>>>> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
>>>>>> +	if (!psci_reset_params)
>>>>>> +		return -ENOMEM;
>>>>>> +
>>>>>> +	for_each_property_of_node(np, prop) {
>>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>> +			continue;
>>>>>> +
>>>>>> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
>>>>>> +							  1, ARRAY_SIZE(magic));
>>>>>> +		if (num < 0) {
>>>>>> +			pr_warn("Failed to parse vendor reboot mode %s\n",
>>>>>> +				param->mode);
>>>>>> +			kfree_const(param->mode);
>>>>>> +			continue;
>>>>>> +		}
>>>>>> +
>>>>>> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>>>> +		if (!param->mode)
>>>>>> +			continue;
>>>>>> +
>>>>>> +		/* Force reset type to be in vendor space */
>>>>>> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
>>>>>> +		param->cookie = num > 1 ? magic[1] : 0;
>>>>>> +		param++;
>>>>>> +		num_psci_reset_params++;
>>>>>> +	}
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +arch_initcall(psci_init_system_reset2_modes);
>>>>>> +
>>>>>>  int __init psci_dt_init(void)
>>>>>>  {
>>>>>>  	struct device_node *np;
>>>>>>
>>>>>> -- 
>>>>>> 2.34.1
>>>>>>

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Dmitry Baryshkov]

On Tue, May 06, 2025 at 11:03:55PM +0530, Shivendra Pratap wrote:
> 
> 
> On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
> > On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
> >>
> >>
> >> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
> >>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
> >>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
> >>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> >>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >>>>>>
> >>>>>> SoC vendors have different types of resets and are controlled through
> >>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
> >>>>>> "download mode" that allows a RAM dump to be collected. Another example
> >>>>>> is they also support writing a cookie that can be read by bootloader
> >>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> >>>>>> vendor reset types to be implemented without requiring drivers for every
> >>>>>> register/cookie.
> >>>>>>
> >>>>>> Add support in PSCI to statically map reboot mode commands from
> >>>>>> userspace to a vendor reset and cookie value using the device tree.
> >>>>>
> >>>>> I have managed to discuss a little bit this patchset over the last
> >>>>> few days and I think we have defined a plan going forward.
> >>>>>
> >>>>> A point that was raised is:
> >>>>>
> >>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
> >>>>>
> >>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> >>>>> represent ?
> >>>>>
> >>>>> Is it the mode the system should reboot into OR it is the
> >>>>> actual command to be issued (which is what this patchset
> >>>>> implements) ?
> >>>>>
> >>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
> >>>>>
> >>>>> It is unclear what "default" means. We wonder whether the
> >>>>> reboot_mode variable was introduced to _define_ that "default".
> >>>>>
> >>>>> So, in short, my aim is trying to decouple reboot_mode from the
> >>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
> >>>>>
> >>>>> I believe that adding a sysfs interface to reboot-mode driver
> >>>>> infrastructure would be useful, so that the commands would
> >>>>> be exposed to userspace and userspace can set the *arg command
> >>>>> specifically to issue a given reset/mode.
> >>>>>
> >>>>> I wonder why this is not already in place for eg syscon-reboot-mode
> >>>>> resets, how does user space issue a command in those systems if the
> >>>>> available commands aren't exposed to userspace ?
> >>>>>
> >>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
> >>>>>
> >>>>>> A separate initcall is needed to parse the devicetree, instead of using
> >>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
> >>>>>>
> >>>>>> Reboot mode framework is close but doesn't quite fit with the
> >>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> >>>>>> be solved but doesn't seem reasonable in sum:
> >>>>>>  1. reboot mode registers against the reboot_notifier_list, which is too
> >>>>>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> >>>>>>     type from the reboot-mode framework callback and use it
> >>>>>>     psci_sys_reset.
> >>>>>>  2. reboot mode assumes only one cookie/parameter is described in the
> >>>>>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> >>>>>>     cookie.
> >>>>>
> >>>>> This can be changed and I think it should, so that the reboot modes
> >>>>> are exposed to user space and PSCI can use that.
> >>>>>
> >>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
> >>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
> >>>> be the last call from Linux, and ideally, this call should not fail.
> >>>>
> >>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
> >>>> notifiers
> >>>>
> >>>> So, if I understand correctly, you mean that we can change the reboot
> >>>> mode framework to expose the arguments available to user space. We can
> >>>> extend it to accept magic and cookies, save them in the reboot
> >>>> framework, and retrieve them via a call from PSCI during a regular
> >>>> reboot or panic based on the current arguments. Is this leading towards
> >>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
> >>>> notifier callback saves the magic and cookies, and these magic and
> >>>> cookies will be used during psci_sys_reset2()? Or is there something
> >>>> wrong with my understanding?
> >>>
> >>> No, you got it right (apologies for the delay in replying) - if the
> >>> case for making reboot mode available to user space is accepted.
> >>>
> While moving this into reboot-mode framework, one more query came up.
> The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
> to be a Platform device driver for using reboot-mode framework.

No, it doesn't. It rqeuires struct device, but there is no requirement
for struct platform_device at any place.

> As psci is not a platform device driver, a subdevice under it may not probe as a
> platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
> early_initcall("psci_xyz") and then create a platform device something as
> below or any other suggestions for this?

Change struct reboot_mode_driver to pass corresponding of_node (or
better fwnode) directly.  Corresponding device is used only in the
reboot_mode_register() and only to access of-node or to print error
messages.

> 
> power:reset:<psci-vendor-reset-driver>:
> -----
> static int __init psci_vendor_reset_init(void) {
> ..
> ..
> 	np = of_find_node_by_name(NULL, "psci-vendor-reset");
> 	if(!np)
> 		return -ENODEV;
> 	pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
> ..
> ..
> }
> -------
> 
> the sysfs we will expose from reboot-mode may show like below in above 
> implementation:
> 
> ###### 
> / # cat ./sys/devices/platform/psci-vendor-reset/available_modes
> bootloader edl 
> ###### 
> 
> thanks,
> Shivendra
> 
> >>
> >> Agree that the available modes should be exposed to usespace via sysfs interface
> >> and we should implement it. Also #1 and #2 can be handled via some
> >> changes in the design as mentioned in above discussion.
> >>
> >> I have one doubt though when we implement this via reboot-mode framework.
> >> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
> >> psci driver is initialized very early at boot but potential ARM psci reboot-mode
> >> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
> >> types functionality will not be available in psci reset path until the reboot-mode
> >> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
> >> types for early device resets?
> >>
> >> One use-case may be an early device crash or a early reset where a vendor 
> >> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a 
> >> specific state but may not be able to use this driver.
> >> (eg: a kernel panic at early boot where a vendor wants to reset device 
> >> to a specific state using vendor reset. Currently panic passes a NULL
> >> (*arg command) while device reset but it may be explored for vendor specific
> >> reset).
> > 
> > As you said, that would not be a PSCI only issue - *if* we wanted to
> > plug in this use case we should find a way to do it at reboot mode
> > driver level.
> > 
> > As a matter of fact, this is not a mainline issue AFAICS.
> > 
> > Even if we did not design this as a reboot mode driver there would be a
> > time window where you would not be able to use vendor resets on panic.
> > 
> > I don't see it as a major roadblock at the moment.
> Got it.
> > 
> > Thanks,
> > Lorenzo
> > 
> >>
> >> - Shivendra
> >>
> >>>> P.S. We appreciate Elliot for his work and follow-up on this while being
> >>>> employed at Qualcomm.
> >>>
> >>> Yes I sincerely do for his patience, thank you.
> >>>
> >>> Lorenzo
> >>>
> >>>>>>  3. psci cpuidle driver already registers a driver against the
> >>>>>>     arm,psci-1.0 compatible. Refactoring would be needed to have both a
> >>>>>>     cpuidle and reboot-mode driver.
> >>>>>>
> >>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >>>>>> ---
> >>>>>>  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> >>>>>>  1 file changed, 105 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> >>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
> >>>>>> --- a/drivers/firmware/psci/psci.c
> >>>>>> +++ b/drivers/firmware/psci/psci.c
> >>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
> >>>>>>  static bool psci_system_reset2_supported;
> >>>>>>  static bool psci_system_off2_hibernate_supported;
> >>>>>>  
> >>>>>> +struct psci_reset_param {
> >>>>>> +	const char *mode;
> >>>>>> +	u32 reset_type;
> >>>>>> +	u32 cookie;
> >>>>>> +};
> >>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
> >>>>>> +static size_t num_psci_reset_params __ro_after_init;
> >>>>>> +
> >>>>>>  static inline bool psci_has_ext_power_state(void)
> >>>>>>  {
> >>>>>>  	return psci_cpu_suspend_feature &
> >>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
> >>>>>>  	return 0;
> >>>>>>  }
> >>>>>>  
> >>>>>> +static int psci_vendor_system_reset2(const char *cmd)
> >>>>>> +{
> >>>>>> +	unsigned long ret;
> >>>>>> +	size_t i;
> >>>>>> +
> >>>>>> +	for (i = 0; i < num_psci_reset_params; i++) {
> >>>>>> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
> >>>>>> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> >>>>>> +					     psci_reset_params[i].reset_type,
> >>>>>> +					     psci_reset_params[i].cookie, 0);
> >>>>>> +			/*
> >>>>>> +			 * if vendor reset fails, log it and fall back to
> >>>>>> +			 * architecture reset types
> >>>>>
> >>>>> That's not what the code does.
> >>>>>
> >>>> Ack.
> >>>>
> >>>> -Mukesh
> >>>>
> >>>>>> +			 */
> >>>>>> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
> >>>>>> +			       (long)ret);
> >>>>>> +			return 0;
> >>>>>> +		}
> >>>>>> +	}
> >>>>>> +
> >>>>>> +	return -ENOENT;
> >>>>>> +}
> >>>>>> +
> >>>>>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> >>>>>>  			  void *data)
> >>>>>>  {
> >>>>>> +	/*
> >>>>>> +	 * try to do the vendor system_reset2
> >>>>>> +	 * If there wasn't a matching command, fall back to architectural resets
> >>>>>> +	 */
> >>>>>> +	if (data && !psci_vendor_system_reset2(data))
> >>>>>> +		return NOTIFY_DONE;
> >>>>>> +
> >>>>>>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >>>>>>  	    psci_system_reset2_supported) {
> >>>>>>  		/*
> >>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
> >>>>>>  	{},
> >>>>>>  };
> >>>>>>  
> >>>>>> +#define REBOOT_PREFIX "mode-"
> >>>>>> +
> >>>>>> +static int __init psci_init_system_reset2_modes(void)
> >>>>>> +{
> >>>>>> +	const size_t len = strlen(REBOOT_PREFIX);
> >>>>>> +	struct psci_reset_param *param;
> >>>>>> +	struct device_node *psci_np __free(device_node) = NULL;
> >>>>>> +	struct device_node *np __free(device_node) = NULL;
> >>>>>> +	struct property *prop;
> >>>>>> +	size_t count = 0;
> >>>>>> +	u32 magic[2];
> >>>>>> +	int num;
> >>>>>> +
> >>>>>> +	if (!psci_system_reset2_supported)
> >>>>>> +		return 0;
> >>>>>> +
> >>>>>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
> >>>>>> +	if (!psci_np)
> >>>>>> +		return 0;
> >>>>>> +
> >>>>>> +	np = of_find_node_by_name(psci_np, "reset-types");
> >>>>>> +	if (!np)
> >>>>>> +		return 0;
> >>>>>
> >>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> >>>>> is the actual reset to be issued, should we add a default mode "cold"
> >>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
> >>>>>
> >>>>> It all boils down to what *arg represents - adding "cold" and "warm"
> >>>>> modes would remove the dependency on reboot_mode for resets issued
> >>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> >>>>> is the correct thing to do.
> >>>>>
> >>>>> Comments very welcome.
> >>>>>
> >>>>> Thanks,
> >>>>> Lorenzo
> >>>>>
> >>>>>> +
> >>>>>> +	for_each_property_of_node(np, prop) {
> >>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> >>>>>> +			continue;
> >>>>>> +		num = of_property_count_u32_elems(np, prop->name);
> >>>>>> +		if (num != 1 && num != 2)
> >>>>>> +			continue;
> >>>>>> +
> >>>>>> +		count++;
> >>>>>> +	}
> >>>>>> +
> >>>>>> +	param = psci_reset_params =
> >>>>>> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
> >>>>>> +	if (!psci_reset_params)
> >>>>>> +		return -ENOMEM;
> >>>>>> +
> >>>>>> +	for_each_property_of_node(np, prop) {
> >>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
> >>>>>> +			continue;
> >>>>>> +
> >>>>>> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
> >>>>>> +							  1, ARRAY_SIZE(magic));
> >>>>>> +		if (num < 0) {
> >>>>>> +			pr_warn("Failed to parse vendor reboot mode %s\n",
> >>>>>> +				param->mode);
> >>>>>> +			kfree_const(param->mode);
> >>>>>> +			continue;
> >>>>>> +		}
> >>>>>> +
> >>>>>> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> >>>>>> +		if (!param->mode)
> >>>>>> +			continue;
> >>>>>> +
> >>>>>> +		/* Force reset type to be in vendor space */
> >>>>>> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
> >>>>>> +		param->cookie = num > 1 ? magic[1] : 0;
> >>>>>> +		param++;
> >>>>>> +		num_psci_reset_params++;
> >>>>>> +	}
> >>>>>> +
> >>>>>> +	return 0;
> >>>>>> +}
> >>>>>> +arch_initcall(psci_init_system_reset2_modes);
> >>>>>> +
> >>>>>>  int __init psci_dt_init(void)
> >>>>>>  {
> >>>>>>  	struct device_node *np;
> >>>>>>
> >>>>>> -- 
> >>>>>> 2.34.1
> >>>>>>

-- 
With best wishes
Dmitry

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Shivendra Pratap]

On 6/18/2025 6:44 PM, Dmitry Baryshkov wrote:
> On Tue, May 06, 2025 at 11:03:55PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
>>> On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
>>>>
>>>>
>>>> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
>>>>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
>>>>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
>>>>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>>>>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> SoC vendors have different types of resets and are controlled through
>>>>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
>>>>>>>> "download mode" that allows a RAM dump to be collected. Another example
>>>>>>>> is they also support writing a cookie that can be read by bootloader
>>>>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>>>>>>>> vendor reset types to be implemented without requiring drivers for every
>>>>>>>> register/cookie.
>>>>>>>>
>>>>>>>> Add support in PSCI to statically map reboot mode commands from
>>>>>>>> userspace to a vendor reset and cookie value using the device tree.
>>>>>>>
>>>>>>> I have managed to discuss a little bit this patchset over the last
>>>>>>> few days and I think we have defined a plan going forward.
>>>>>>>
>>>>>>> A point that was raised is:
>>>>>>>
>>>>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
>>>>>>>
>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
>>>>>>> represent ?
>>>>>>>
>>>>>>> Is it the mode the system should reboot into OR it is the
>>>>>>> actual command to be issued (which is what this patchset
>>>>>>> implements) ?
>>>>>>>
>>>>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>>>>>>>
>>>>>>> It is unclear what "default" means. We wonder whether the
>>>>>>> reboot_mode variable was introduced to _define_ that "default".
>>>>>>>
>>>>>>> So, in short, my aim is trying to decouple reboot_mode from the
>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
>>>>>>>
>>>>>>> I believe that adding a sysfs interface to reboot-mode driver
>>>>>>> infrastructure would be useful, so that the commands would
>>>>>>> be exposed to userspace and userspace can set the *arg command
>>>>>>> specifically to issue a given reset/mode.
>>>>>>>
>>>>>>> I wonder why this is not already in place for eg syscon-reboot-mode
>>>>>>> resets, how does user space issue a command in those systems if the
>>>>>>> available commands aren't exposed to userspace ?
>>>>>>>
>>>>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
>>>>>>>
>>>>>>>> A separate initcall is needed to parse the devicetree, instead of using
>>>>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>>>>>>>>
>>>>>>>> Reboot mode framework is close but doesn't quite fit with the
>>>>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>>>>>>>> be solved but doesn't seem reasonable in sum:
>>>>>>>>  1. reboot mode registers against the reboot_notifier_list, which is too
>>>>>>>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>>>>>>>     type from the reboot-mode framework callback and use it
>>>>>>>>     psci_sys_reset.
>>>>>>>>  2. reboot mode assumes only one cookie/parameter is described in the
>>>>>>>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>>>>>>>     cookie.
>>>>>>>
>>>>>>> This can be changed and I think it should, so that the reboot modes
>>>>>>> are exposed to user space and PSCI can use that.
>>>>>>>
>>>>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
>>>>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
>>>>>> be the last call from Linux, and ideally, this call should not fail.
>>>>>>
>>>>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
>>>>>> notifiers
>>>>>>
>>>>>> So, if I understand correctly, you mean that we can change the reboot
>>>>>> mode framework to expose the arguments available to user space. We can
>>>>>> extend it to accept magic and cookies, save them in the reboot
>>>>>> framework, and retrieve them via a call from PSCI during a regular
>>>>>> reboot or panic based on the current arguments. Is this leading towards
>>>>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
>>>>>> notifier callback saves the magic and cookies, and these magic and
>>>>>> cookies will be used during psci_sys_reset2()? Or is there something
>>>>>> wrong with my understanding?
>>>>>
>>>>> No, you got it right (apologies for the delay in replying) - if the
>>>>> case for making reboot mode available to user space is accepted.
>>>>>
>> While moving this into reboot-mode framework, one more query came up.
>> The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
>> to be a Platform device driver for using reboot-mode framework.
> 
> No, it doesn't. It rqeuires struct device, but there is no requirement
> for struct platform_device at any place.
yes, it can be struct device so may be create a virtual device
using reset-type node?
> 
>> As psci is not a platform device driver, a subdevice under it may not probe as a
>> platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
>> early_initcall("psci_xyz") and then create a platform device something as
>> below or any other suggestions for this?
> 
> Change struct reboot_mode_driver to pass corresponding of_node (or
> better fwnode) directly.  Corresponding device is used only in the
> reboot_mode_register() and only to access of-node or to print error
> messages.
struct reboot_mode_driver can be changed just to pass of_node. But then the other
suggestion was to expose sysfs from reboot-mode to show available commands.
For that we need a device. Any suggestion? A virtual device with reset-types node
passed to reboot-mode framework looks fine?
> 
>>
>> power:reset:<psci-vendor-reset-driver>:
>> -----
>> static int __init psci_vendor_reset_init(void) {
>> ..
>> ..
>> 	np = of_find_node_by_name(NULL, "psci-vendor-reset");
>> 	if(!np)
>> 		return -ENODEV;
>> 	pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
>> ..
>> ..
>> }
>> -------
>>
>> the sysfs we will expose from reboot-mode may show like below in above 
>> implementation:
>>
>> ###### 
>> / # cat ./sys/devices/platform/psci-vendor-reset/available_modes
>> bootloader edl 
>> ###### 
>>
>> thanks,
>> Shivendra
>>
>>>>
>>>> Agree that the available modes should be exposed to usespace via sysfs interface
>>>> and we should implement it. Also #1 and #2 can be handled via some
>>>> changes in the design as mentioned in above discussion.
>>>>
>>>> I have one doubt though when we implement this via reboot-mode framework.
>>>> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
>>>> psci driver is initialized very early at boot but potential ARM psci reboot-mode
>>>> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
>>>> types functionality will not be available in psci reset path until the reboot-mode
>>>> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
>>>> types for early device resets?
>>>>
>>>> One use-case may be an early device crash or a early reset where a vendor 
>>>> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a 
>>>> specific state but may not be able to use this driver.
>>>> (eg: a kernel panic at early boot where a vendor wants to reset device 
>>>> to a specific state using vendor reset. Currently panic passes a NULL
>>>> (*arg command) while device reset but it may be explored for vendor specific
>>>> reset).
>>>
>>> As you said, that would not be a PSCI only issue - *if* we wanted to
>>> plug in this use case we should find a way to do it at reboot mode
>>> driver level.
>>>
>>> As a matter of fact, this is not a mainline issue AFAICS.
>>>
>>> Even if we did not design this as a reboot mode driver there would be a
>>> time window where you would not be able to use vendor resets on panic.
>>>
>>> I don't see it as a major roadblock at the moment.
>> Got it.
>>>
>>> Thanks,
>>> Lorenzo
>>>
>>>>
>>>> - Shivendra
>>>>
>>>>>> P.S. We appreciate Elliot for his work and follow-up on this while being
>>>>>> employed at Qualcomm.
>>>>>
>>>>> Yes I sincerely do for his patience, thank you.
>>>>>
>>>>> Lorenzo
>>>>>
>>>>>>>>  3. psci cpuidle driver already registers a driver against the
>>>>>>>>     arm,psci-1.0 compatible. Refactoring would be needed to have both a
>>>>>>>>     cpuidle and reboot-mode driver.
>>>>>>>>
>>>>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>> ---
>>>>>>>>  drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>  1 file changed, 105 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
>>>>>>>> --- a/drivers/firmware/psci/psci.c
>>>>>>>> +++ b/drivers/firmware/psci/psci.c
>>>>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>>>>>>>>  static bool psci_system_reset2_supported;
>>>>>>>>  static bool psci_system_off2_hibernate_supported;
>>>>>>>>  
>>>>>>>> +struct psci_reset_param {
>>>>>>>> +	const char *mode;
>>>>>>>> +	u32 reset_type;
>>>>>>>> +	u32 cookie;
>>>>>>>> +};
>>>>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
>>>>>>>> +static size_t num_psci_reset_params __ro_after_init;
>>>>>>>> +
>>>>>>>>  static inline bool psci_has_ext_power_state(void)
>>>>>>>>  {
>>>>>>>>  	return psci_cpu_suspend_feature &
>>>>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>>>>>  	return 0;
>>>>>>>>  }
>>>>>>>>  
>>>>>>>> +static int psci_vendor_system_reset2(const char *cmd)
>>>>>>>> +{
>>>>>>>> +	unsigned long ret;
>>>>>>>> +	size_t i;
>>>>>>>> +
>>>>>>>> +	for (i = 0; i < num_psci_reset_params; i++) {
>>>>>>>> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>>>>>> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>>>>>> +					     psci_reset_params[i].reset_type,
>>>>>>>> +					     psci_reset_params[i].cookie, 0);
>>>>>>>> +			/*
>>>>>>>> +			 * if vendor reset fails, log it and fall back to
>>>>>>>> +			 * architecture reset types
>>>>>>>
>>>>>>> That's not what the code does.
>>>>>>>
>>>>>> Ack.
>>>>>>
>>>>>> -Mukesh
>>>>>>
>>>>>>>> +			 */
>>>>>>>> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
>>>>>>>> +			       (long)ret);
>>>>>>>> +			return 0;
>>>>>>>> +		}
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>> +	return -ENOENT;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>>>>>  			  void *data)
>>>>>>>>  {
>>>>>>>> +	/*
>>>>>>>> +	 * try to do the vendor system_reset2
>>>>>>>> +	 * If there wasn't a matching command, fall back to architectural resets
>>>>>>>> +	 */
>>>>>>>> +	if (data && !psci_vendor_system_reset2(data))
>>>>>>>> +		return NOTIFY_DONE;
>>>>>>>> +
>>>>>>>>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>>>>>  	    psci_system_reset2_supported) {
>>>>>>>>  		/*
>>>>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>>>>>>>>  	{},
>>>>>>>>  };
>>>>>>>>  
>>>>>>>> +#define REBOOT_PREFIX "mode-"
>>>>>>>> +
>>>>>>>> +static int __init psci_init_system_reset2_modes(void)
>>>>>>>> +{
>>>>>>>> +	const size_t len = strlen(REBOOT_PREFIX);
>>>>>>>> +	struct psci_reset_param *param;
>>>>>>>> +	struct device_node *psci_np __free(device_node) = NULL;
>>>>>>>> +	struct device_node *np __free(device_node) = NULL;
>>>>>>>> +	struct property *prop;
>>>>>>>> +	size_t count = 0;
>>>>>>>> +	u32 magic[2];
>>>>>>>> +	int num;
>>>>>>>> +
>>>>>>>> +	if (!psci_system_reset2_supported)
>>>>>>>> +		return 0;
>>>>>>>> +
>>>>>>>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
>>>>>>>> +	if (!psci_np)
>>>>>>>> +		return 0;
>>>>>>>> +
>>>>>>>> +	np = of_find_node_by_name(psci_np, "reset-types");
>>>>>>>> +	if (!np)
>>>>>>>> +		return 0;
>>>>>>>
>>>>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
>>>>>>> is the actual reset to be issued, should we add a default mode "cold"
>>>>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>>>>>>>
>>>>>>> It all boils down to what *arg represents - adding "cold" and "warm"
>>>>>>> modes would remove the dependency on reboot_mode for resets issued
>>>>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
>>>>>>> is the correct thing to do.
>>>>>>>
>>>>>>> Comments very welcome.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Lorenzo
>>>>>>>
>>>>>>>> +
>>>>>>>> +	for_each_property_of_node(np, prop) {
>>>>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>> +			continue;
>>>>>>>> +		num = of_property_count_u32_elems(np, prop->name);
>>>>>>>> +		if (num != 1 && num != 2)
>>>>>>>> +			continue;
>>>>>>>> +
>>>>>>>> +		count++;
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>> +	param = psci_reset_params =
>>>>>>>> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
>>>>>>>> +	if (!psci_reset_params)
>>>>>>>> +		return -ENOMEM;
>>>>>>>> +
>>>>>>>> +	for_each_property_of_node(np, prop) {
>>>>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>> +			continue;
>>>>>>>> +
>>>>>>>> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
>>>>>>>> +							  1, ARRAY_SIZE(magic));
>>>>>>>> +		if (num < 0) {
>>>>>>>> +			pr_warn("Failed to parse vendor reboot mode %s\n",
>>>>>>>> +				param->mode);
>>>>>>>> +			kfree_const(param->mode);
>>>>>>>> +			continue;
>>>>>>>> +		}
>>>>>>>> +
>>>>>>>> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>>>>>> +		if (!param->mode)
>>>>>>>> +			continue;
>>>>>>>> +
>>>>>>>> +		/* Force reset type to be in vendor space */
>>>>>>>> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
>>>>>>>> +		param->cookie = num > 1 ? magic[1] : 0;
>>>>>>>> +		param++;
>>>>>>>> +		num_psci_reset_params++;
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>> +	return 0;
>>>>>>>> +}
>>>>>>>> +arch_initcall(psci_init_system_reset2_modes);
>>>>>>>> +
>>>>>>>>  int __init psci_dt_init(void)
>>>>>>>>  {
>>>>>>>>  	struct device_node *np;
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> 2.34.1
>>>>>>>>
> 

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Dmitry Baryshkov]

On 19/06/2025 12:00, Shivendra Pratap wrote:
> 
> 
> On 6/18/2025 6:44 PM, Dmitry Baryshkov wrote:
>> On Tue, May 06, 2025 at 11:03:55PM +0530, Shivendra Pratap wrote:
>>>
>>>
>>> On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
>>>> On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
>>>>>
>>>>>
>>>>> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
>>>>>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
>>>>>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
>>>>>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>>>>>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>>
>>>>>>>>> SoC vendors have different types of resets and are controlled through
>>>>>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
>>>>>>>>> "download mode" that allows a RAM dump to be collected. Another example
>>>>>>>>> is they also support writing a cookie that can be read by bootloader
>>>>>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>>>>>>>>> vendor reset types to be implemented without requiring drivers for every
>>>>>>>>> register/cookie.
>>>>>>>>>
>>>>>>>>> Add support in PSCI to statically map reboot mode commands from
>>>>>>>>> userspace to a vendor reset and cookie value using the device tree.
>>>>>>>>
>>>>>>>> I have managed to discuss a little bit this patchset over the last
>>>>>>>> few days and I think we have defined a plan going forward.
>>>>>>>>
>>>>>>>> A point that was raised is:
>>>>>>>>
>>>>>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
>>>>>>>>
>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
>>>>>>>> represent ?
>>>>>>>>
>>>>>>>> Is it the mode the system should reboot into OR it is the
>>>>>>>> actual command to be issued (which is what this patchset
>>>>>>>> implements) ?
>>>>>>>>
>>>>>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>>>>>>>>
>>>>>>>> It is unclear what "default" means. We wonder whether the
>>>>>>>> reboot_mode variable was introduced to _define_ that "default".
>>>>>>>>
>>>>>>>> So, in short, my aim is trying to decouple reboot_mode from the
>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
>>>>>>>>
>>>>>>>> I believe that adding a sysfs interface to reboot-mode driver
>>>>>>>> infrastructure would be useful, so that the commands would
>>>>>>>> be exposed to userspace and userspace can set the *arg command
>>>>>>>> specifically to issue a given reset/mode.
>>>>>>>>
>>>>>>>> I wonder why this is not already in place for eg syscon-reboot-mode
>>>>>>>> resets, how does user space issue a command in those systems if the
>>>>>>>> available commands aren't exposed to userspace ?
>>>>>>>>
>>>>>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
>>>>>>>>
>>>>>>>>> A separate initcall is needed to parse the devicetree, instead of using
>>>>>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>>>>>>>>>
>>>>>>>>> Reboot mode framework is close but doesn't quite fit with the
>>>>>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>>>>>>>>> be solved but doesn't seem reasonable in sum:
>>>>>>>>>   1. reboot mode registers against the reboot_notifier_list, which is too
>>>>>>>>>      early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>>>>>>>>      type from the reboot-mode framework callback and use it
>>>>>>>>>      psci_sys_reset.
>>>>>>>>>   2. reboot mode assumes only one cookie/parameter is described in the
>>>>>>>>>      device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>>>>>>>>      cookie.
>>>>>>>>
>>>>>>>> This can be changed and I think it should, so that the reboot modes
>>>>>>>> are exposed to user space and PSCI can use that.
>>>>>>>>
>>>>>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
>>>>>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
>>>>>>> be the last call from Linux, and ideally, this call should not fail.
>>>>>>>
>>>>>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
>>>>>>> notifiers
>>>>>>>
>>>>>>> So, if I understand correctly, you mean that we can change the reboot
>>>>>>> mode framework to expose the arguments available to user space. We can
>>>>>>> extend it to accept magic and cookies, save them in the reboot
>>>>>>> framework, and retrieve them via a call from PSCI during a regular
>>>>>>> reboot or panic based on the current arguments. Is this leading towards
>>>>>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
>>>>>>> notifier callback saves the magic and cookies, and these magic and
>>>>>>> cookies will be used during psci_sys_reset2()? Or is there something
>>>>>>> wrong with my understanding?
>>>>>>
>>>>>> No, you got it right (apologies for the delay in replying) - if the
>>>>>> case for making reboot mode available to user space is accepted.
>>>>>>
>>> While moving this into reboot-mode framework, one more query came up.
>>> The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
>>> to be a Platform device driver for using reboot-mode framework.
>>
>> No, it doesn't. It rqeuires struct device, but there is no requirement
>> for struct platform_device at any place.
> yes, it can be struct device so may be create a virtual device
> using reset-type node?

It can be created, but I don't see a strong need for it.

>>
>>> As psci is not a platform device driver, a subdevice under it may not probe as a
>>> platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
>>> early_initcall("psci_xyz") and then create a platform device something as
>>> below or any other suggestions for this?
>>
>> Change struct reboot_mode_driver to pass corresponding of_node (or
>> better fwnode) directly.  Corresponding device is used only in the
>> reboot_mode_register() and only to access of-node or to print error
>> messages.
> struct reboot_mode_driver can be changed just to pass of_node. But then the other
> suggestion was to expose sysfs from reboot-mode to show available commands.
> For that we need a device. Any suggestion? A virtual device with reset-types node
> passed to reboot-mode framework looks fine?

You still don't need it. You'll create a new device, belonging to the 
new 'reboot' or 'reset' class to hold corresponding attributes.

>>
>>>
>>> power:reset:<psci-vendor-reset-driver>:
>>> -----
>>> static int __init psci_vendor_reset_init(void) {
>>> ..
>>> ..
>>> 	np = of_find_node_by_name(NULL, "psci-vendor-reset");
>>> 	if(!np)
>>> 		return -ENODEV;
>>> 	pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
>>> ..
>>> ..
>>> }
>>> -------
>>>
>>> the sysfs we will expose from reboot-mode may show like below in above
>>> implementation:
>>>
>>> ######
>>> / # cat ./sys/devices/platform/psci-vendor-reset/available_modes
>>> bootloader edl
>>> ######
>>>
>>> thanks,
>>> Shivendra
>>>
>>>>>
>>>>> Agree that the available modes should be exposed to usespace via sysfs interface
>>>>> and we should implement it. Also #1 and #2 can be handled via some
>>>>> changes in the design as mentioned in above discussion.
>>>>>
>>>>> I have one doubt though when we implement this via reboot-mode framework.
>>>>> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
>>>>> psci driver is initialized very early at boot but potential ARM psci reboot-mode
>>>>> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
>>>>> types functionality will not be available in psci reset path until the reboot-mode
>>>>> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
>>>>> types for early device resets?
>>>>>
>>>>> One use-case may be an early device crash or a early reset where a vendor
>>>>> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a
>>>>> specific state but may not be able to use this driver.
>>>>> (eg: a kernel panic at early boot where a vendor wants to reset device
>>>>> to a specific state using vendor reset. Currently panic passes a NULL
>>>>> (*arg command) while device reset but it may be explored for vendor specific
>>>>> reset).
>>>>
>>>> As you said, that would not be a PSCI only issue - *if* we wanted to
>>>> plug in this use case we should find a way to do it at reboot mode
>>>> driver level.
>>>>
>>>> As a matter of fact, this is not a mainline issue AFAICS.
>>>>
>>>> Even if we did not design this as a reboot mode driver there would be a
>>>> time window where you would not be able to use vendor resets on panic.
>>>>
>>>> I don't see it as a major roadblock at the moment.
>>> Got it.
>>>>
>>>> Thanks,
>>>> Lorenzo
>>>>
>>>>>
>>>>> - Shivendra
>>>>>
>>>>>>> P.S. We appreciate Elliot for his work and follow-up on this while being
>>>>>>> employed at Qualcomm.
>>>>>>
>>>>>> Yes I sincerely do for his patience, thank you.
>>>>>>
>>>>>> Lorenzo
>>>>>>
>>>>>>>>>   3. psci cpuidle driver already registers a driver against the
>>>>>>>>>      arm,psci-1.0 compatible. Refactoring would be needed to have both a
>>>>>>>>>      cpuidle and reboot-mode driver.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>> ---
>>>>>>>>>   drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>   1 file changed, 105 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>>>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
>>>>>>>>> --- a/drivers/firmware/psci/psci.c
>>>>>>>>> +++ b/drivers/firmware/psci/psci.c
>>>>>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>>>>>>>>>   static bool psci_system_reset2_supported;
>>>>>>>>>   static bool psci_system_off2_hibernate_supported;
>>>>>>>>>   
>>>>>>>>> +struct psci_reset_param {
>>>>>>>>> +	const char *mode;
>>>>>>>>> +	u32 reset_type;
>>>>>>>>> +	u32 cookie;
>>>>>>>>> +};
>>>>>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
>>>>>>>>> +static size_t num_psci_reset_params __ro_after_init;
>>>>>>>>> +
>>>>>>>>>   static inline bool psci_has_ext_power_state(void)
>>>>>>>>>   {
>>>>>>>>>   	return psci_cpu_suspend_feature &
>>>>>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>>>>>>   	return 0;
>>>>>>>>>   }
>>>>>>>>>   
>>>>>>>>> +static int psci_vendor_system_reset2(const char *cmd)
>>>>>>>>> +{
>>>>>>>>> +	unsigned long ret;
>>>>>>>>> +	size_t i;
>>>>>>>>> +
>>>>>>>>> +	for (i = 0; i < num_psci_reset_params; i++) {
>>>>>>>>> +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>>>>>>> +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>>>>>>> +					     psci_reset_params[i].reset_type,
>>>>>>>>> +					     psci_reset_params[i].cookie, 0);
>>>>>>>>> +			/*
>>>>>>>>> +			 * if vendor reset fails, log it and fall back to
>>>>>>>>> +			 * architecture reset types
>>>>>>>>
>>>>>>>> That's not what the code does.
>>>>>>>>
>>>>>>> Ack.
>>>>>>>
>>>>>>> -Mukesh
>>>>>>>
>>>>>>>>> +			 */
>>>>>>>>> +			pr_err("failed to perform reset \"%s\": %ld\n", cmd,
>>>>>>>>> +			       (long)ret);
>>>>>>>>> +			return 0;
>>>>>>>>> +		}
>>>>>>>>> +	}
>>>>>>>>> +
>>>>>>>>> +	return -ENOENT;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>>   static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>>>>>>   			  void *data)
>>>>>>>>>   {
>>>>>>>>> +	/*
>>>>>>>>> +	 * try to do the vendor system_reset2
>>>>>>>>> +	 * If there wasn't a matching command, fall back to architectural resets
>>>>>>>>> +	 */
>>>>>>>>> +	if (data && !psci_vendor_system_reset2(data))
>>>>>>>>> +		return NOTIFY_DONE;
>>>>>>>>> +
>>>>>>>>>   	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>>>>>>   	    psci_system_reset2_supported) {
>>>>>>>>>   		/*
>>>>>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>>>>>>>>>   	{},
>>>>>>>>>   };
>>>>>>>>>   
>>>>>>>>> +#define REBOOT_PREFIX "mode-"
>>>>>>>>> +
>>>>>>>>> +static int __init psci_init_system_reset2_modes(void)
>>>>>>>>> +{
>>>>>>>>> +	const size_t len = strlen(REBOOT_PREFIX);
>>>>>>>>> +	struct psci_reset_param *param;
>>>>>>>>> +	struct device_node *psci_np __free(device_node) = NULL;
>>>>>>>>> +	struct device_node *np __free(device_node) = NULL;
>>>>>>>>> +	struct property *prop;
>>>>>>>>> +	size_t count = 0;
>>>>>>>>> +	u32 magic[2];
>>>>>>>>> +	int num;
>>>>>>>>> +
>>>>>>>>> +	if (!psci_system_reset2_supported)
>>>>>>>>> +		return 0;
>>>>>>>>> +
>>>>>>>>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
>>>>>>>>> +	if (!psci_np)
>>>>>>>>> +		return 0;
>>>>>>>>> +
>>>>>>>>> +	np = of_find_node_by_name(psci_np, "reset-types");
>>>>>>>>> +	if (!np)
>>>>>>>>> +		return 0;
>>>>>>>>
>>>>>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
>>>>>>>> is the actual reset to be issued, should we add a default mode "cold"
>>>>>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>>>>>>>>
>>>>>>>> It all boils down to what *arg represents - adding "cold" and "warm"
>>>>>>>> modes would remove the dependency on reboot_mode for resets issued
>>>>>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
>>>>>>>> is the correct thing to do.
>>>>>>>>
>>>>>>>> Comments very welcome.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Lorenzo
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +	for_each_property_of_node(np, prop) {
>>>>>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>>> +			continue;
>>>>>>>>> +		num = of_property_count_u32_elems(np, prop->name);
>>>>>>>>> +		if (num != 1 && num != 2)
>>>>>>>>> +			continue;
>>>>>>>>> +
>>>>>>>>> +		count++;
>>>>>>>>> +	}
>>>>>>>>> +
>>>>>>>>> +	param = psci_reset_params =
>>>>>>>>> +		kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
>>>>>>>>> +	if (!psci_reset_params)
>>>>>>>>> +		return -ENOMEM;
>>>>>>>>> +
>>>>>>>>> +	for_each_property_of_node(np, prop) {
>>>>>>>>> +		if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>>> +			continue;
>>>>>>>>> +
>>>>>>>>> +		num = of_property_read_variable_u32_array(np, prop->name, magic,
>>>>>>>>> +							  1, ARRAY_SIZE(magic));
>>>>>>>>> +		if (num < 0) {
>>>>>>>>> +			pr_warn("Failed to parse vendor reboot mode %s\n",
>>>>>>>>> +				param->mode);
>>>>>>>>> +			kfree_const(param->mode);
>>>>>>>>> +			continue;
>>>>>>>>> +		}
>>>>>>>>> +
>>>>>>>>> +		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>>>>>>> +		if (!param->mode)
>>>>>>>>> +			continue;
>>>>>>>>> +
>>>>>>>>> +		/* Force reset type to be in vendor space */
>>>>>>>>> +		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
>>>>>>>>> +		param->cookie = num > 1 ? magic[1] : 0;
>>>>>>>>> +		param++;
>>>>>>>>> +		num_psci_reset_params++;
>>>>>>>>> +	}
>>>>>>>>> +
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>>> +arch_initcall(psci_init_system_reset2_modes);
>>>>>>>>> +
>>>>>>>>>   int __init psci_dt_init(void)
>>>>>>>>>   {
>>>>>>>>>   	struct device_node *np;
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> 2.34.1
>>>>>>>>>
>>


-- 
With best wishes
Dmitry

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Shivendra Pratap]

On 6/19/2025 4:34 PM, Dmitry Baryshkov wrote:
> On 19/06/2025 12:00, Shivendra Pratap wrote:
>>
>>
>> On 6/18/2025 6:44 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 06, 2025 at 11:03:55PM +0530, Shivendra Pratap wrote:
>>>>
>>>>
>>>> On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
>>>>> On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
>>>>>>
>>>>>>
>>>>>> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
>>>>>>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
>>>>>>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
>>>>>>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>>>>>>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>>>
>>>>>>>>>> SoC vendors have different types of resets and are controlled through
>>>>>>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
>>>>>>>>>> "download mode" that allows a RAM dump to be collected. Another example
>>>>>>>>>> is they also support writing a cookie that can be read by bootloader
>>>>>>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>>>>>>>>>> vendor reset types to be implemented without requiring drivers for every
>>>>>>>>>> register/cookie.
>>>>>>>>>>
>>>>>>>>>> Add support in PSCI to statically map reboot mode commands from
>>>>>>>>>> userspace to a vendor reset and cookie value using the device tree.
>>>>>>>>>
>>>>>>>>> I have managed to discuss a little bit this patchset over the last
>>>>>>>>> few days and I think we have defined a plan going forward.
>>>>>>>>>
>>>>>>>>> A point that was raised is:
>>>>>>>>>
>>>>>>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
>>>>>>>>>
>>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
>>>>>>>>> represent ?
>>>>>>>>>
>>>>>>>>> Is it the mode the system should reboot into OR it is the
>>>>>>>>> actual command to be issued (which is what this patchset
>>>>>>>>> implements) ?
>>>>>>>>>
>>>>>>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>>>>>>>>>
>>>>>>>>> It is unclear what "default" means. We wonder whether the
>>>>>>>>> reboot_mode variable was introduced to _define_ that "default".
>>>>>>>>>
>>>>>>>>> So, in short, my aim is trying to decouple reboot_mode from the
>>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
>>>>>>>>>
>>>>>>>>> I believe that adding a sysfs interface to reboot-mode driver
>>>>>>>>> infrastructure would be useful, so that the commands would
>>>>>>>>> be exposed to userspace and userspace can set the *arg command
>>>>>>>>> specifically to issue a given reset/mode.
>>>>>>>>>
>>>>>>>>> I wonder why this is not already in place for eg syscon-reboot-mode
>>>>>>>>> resets, how does user space issue a command in those systems if the
>>>>>>>>> available commands aren't exposed to userspace ?
>>>>>>>>>
>>>>>>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
>>>>>>>>>
>>>>>>>>>> A separate initcall is needed to parse the devicetree, instead of using
>>>>>>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>>>>>>>>>>
>>>>>>>>>> Reboot mode framework is close but doesn't quite fit with the
>>>>>>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>>>>>>>>>> be solved but doesn't seem reasonable in sum:
>>>>>>>>>>   1. reboot mode registers against the reboot_notifier_list, which is too
>>>>>>>>>>      early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>>>>>>>>>      type from the reboot-mode framework callback and use it
>>>>>>>>>>      psci_sys_reset.
>>>>>>>>>>   2. reboot mode assumes only one cookie/parameter is described in the
>>>>>>>>>>      device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>>>>>>>>>      cookie.
>>>>>>>>>
>>>>>>>>> This can be changed and I think it should, so that the reboot modes
>>>>>>>>> are exposed to user space and PSCI can use that.
>>>>>>>>>
>>>>>>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
>>>>>>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
>>>>>>>> be the last call from Linux, and ideally, this call should not fail.
>>>>>>>>
>>>>>>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
>>>>>>>> notifiers
>>>>>>>>
>>>>>>>> So, if I understand correctly, you mean that we can change the reboot
>>>>>>>> mode framework to expose the arguments available to user space. We can
>>>>>>>> extend it to accept magic and cookies, save them in the reboot
>>>>>>>> framework, and retrieve them via a call from PSCI during a regular
>>>>>>>> reboot or panic based on the current arguments. Is this leading towards
>>>>>>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
>>>>>>>> notifier callback saves the magic and cookies, and these magic and
>>>>>>>> cookies will be used during psci_sys_reset2()? Or is there something
>>>>>>>> wrong with my understanding?
>>>>>>>
>>>>>>> No, you got it right (apologies for the delay in replying) - if the
>>>>>>> case for making reboot mode available to user space is accepted.
>>>>>>>
>>>> While moving this into reboot-mode framework, one more query came up.
>>>> The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
>>>> to be a Platform device driver for using reboot-mode framework.
>>>
>>> No, it doesn't. It rqeuires struct device, but there is no requirement
>>> for struct platform_device at any place.
>> yes, it can be struct device so may be create a virtual device
>> using reset-type node?
> 
> It can be created, but I don't see a strong need for it.
> 
>>>
>>>> As psci is not a platform device driver, a subdevice under it may not probe as a
>>>> platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
>>>> early_initcall("psci_xyz") and then create a platform device something as
>>>> below or any other suggestions for this?
>>>
>>> Change struct reboot_mode_driver to pass corresponding of_node (or
>>> better fwnode) directly.  Corresponding device is used only in the
>>> reboot_mode_register() and only to access of-node or to print error
>>> messages.
>> struct reboot_mode_driver can be changed just to pass of_node. But then the other
>> suggestion was to expose sysfs from reboot-mode to show available commands.
>> For that we need a device. Any suggestion? A virtual device with reset-types node
>> passed to reboot-mode framework looks fine?
> 
> You still don't need it. You'll create a new device, belonging to the new 'reboot' or 'reset' class to hold corresponding attributes.
just understand this - So the reboot-mode framework will create a new class
and a device and expose the supported commands?
> 
>>>
>>>>
>>>> power:reset:<psci-vendor-reset-driver>:
>>>> -----
>>>> static int __init psci_vendor_reset_init(void) {
>>>> ..
>>>> ..
>>>>     np = of_find_node_by_name(NULL, "psci-vendor-reset");
>>>>     if(!np)
>>>>         return -ENODEV;
>>>>     pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
>>>> ..
>>>> ..
>>>> }
>>>> -------
>>>>
>>>> the sysfs we will expose from reboot-mode may show like below in above
>>>> implementation:
>>>>
>>>> ######
>>>> / # cat ./sys/devices/platform/psci-vendor-reset/available_modes
>>>> bootloader edl
>>>> ######
>>>>
>>>> thanks,
>>>> Shivendra
>>>>
>>>>>>
>>>>>> Agree that the available modes should be exposed to usespace via sysfs interface
>>>>>> and we should implement it. Also #1 and #2 can be handled via some
>>>>>> changes in the design as mentioned in above discussion.
>>>>>>
>>>>>> I have one doubt though when we implement this via reboot-mode framework.
>>>>>> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
>>>>>> psci driver is initialized very early at boot but potential ARM psci reboot-mode
>>>>>> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
>>>>>> types functionality will not be available in psci reset path until the reboot-mode
>>>>>> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
>>>>>> types for early device resets?
>>>>>>
>>>>>> One use-case may be an early device crash or a early reset where a vendor
>>>>>> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a
>>>>>> specific state but may not be able to use this driver.
>>>>>> (eg: a kernel panic at early boot where a vendor wants to reset device
>>>>>> to a specific state using vendor reset. Currently panic passes a NULL
>>>>>> (*arg command) while device reset but it may be explored for vendor specific
>>>>>> reset).
>>>>>
>>>>> As you said, that would not be a PSCI only issue - *if* we wanted to
>>>>> plug in this use case we should find a way to do it at reboot mode
>>>>> driver level.
>>>>>
>>>>> As a matter of fact, this is not a mainline issue AFAICS.
>>>>>
>>>>> Even if we did not design this as a reboot mode driver there would be a
>>>>> time window where you would not be able to use vendor resets on panic.
>>>>>
>>>>> I don't see it as a major roadblock at the moment.
>>>> Got it.
>>>>>
>>>>> Thanks,
>>>>> Lorenzo
>>>>>
>>>>>>
>>>>>> - Shivendra
>>>>>>
>>>>>>>> P.S. We appreciate Elliot for his work and follow-up on this while being
>>>>>>>> employed at Qualcomm.
>>>>>>>
>>>>>>> Yes I sincerely do for his patience, thank you.
>>>>>>>
>>>>>>> Lorenzo
>>>>>>>
>>>>>>>>>>   3. psci cpuidle driver already registers a driver against the
>>>>>>>>>>      arm,psci-1.0 compatible. Refactoring would be needed to have both a
>>>>>>>>>>      cpuidle and reboot-mode driver.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>>> ---
>>>>>>>>>>   drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>   1 file changed, 105 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>>>>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
>>>>>>>>>> --- a/drivers/firmware/psci/psci.c
>>>>>>>>>> +++ b/drivers/firmware/psci/psci.c
>>>>>>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>>>>>>>>>>   static bool psci_system_reset2_supported;
>>>>>>>>>>   static bool psci_system_off2_hibernate_supported;
>>>>>>>>>>   +struct psci_reset_param {
>>>>>>>>>> +    const char *mode;
>>>>>>>>>> +    u32 reset_type;
>>>>>>>>>> +    u32 cookie;
>>>>>>>>>> +};
>>>>>>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
>>>>>>>>>> +static size_t num_psci_reset_params __ro_after_init;
>>>>>>>>>> +
>>>>>>>>>>   static inline bool psci_has_ext_power_state(void)
>>>>>>>>>>   {
>>>>>>>>>>       return psci_cpu_suspend_feature &
>>>>>>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>>>>>>>       return 0;
>>>>>>>>>>   }
>>>>>>>>>>   +static int psci_vendor_system_reset2(const char *cmd)
>>>>>>>>>> +{
>>>>>>>>>> +    unsigned long ret;
>>>>>>>>>> +    size_t i;
>>>>>>>>>> +
>>>>>>>>>> +    for (i = 0; i < num_psci_reset_params; i++) {
>>>>>>>>>> +        if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>>>>>>>> +            ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>>>>>>>> +                         psci_reset_params[i].reset_type,
>>>>>>>>>> +                         psci_reset_params[i].cookie, 0);
>>>>>>>>>> +            /*
>>>>>>>>>> +             * if vendor reset fails, log it and fall back to
>>>>>>>>>> +             * architecture reset types
>>>>>>>>>
>>>>>>>>> That's not what the code does.
>>>>>>>>>
>>>>>>>> Ack.
>>>>>>>>
>>>>>>>> -Mukesh
>>>>>>>>
>>>>>>>>>> +             */
>>>>>>>>>> +            pr_err("failed to perform reset \"%s\": %ld\n", cmd,
>>>>>>>>>> +                   (long)ret);
>>>>>>>>>> +            return 0;
>>>>>>>>>> +        }
>>>>>>>>>> +    }
>>>>>>>>>> +
>>>>>>>>>> +    return -ENOENT;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>>   static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>>>>>>>                 void *data)
>>>>>>>>>>   {
>>>>>>>>>> +    /*
>>>>>>>>>> +     * try to do the vendor system_reset2
>>>>>>>>>> +     * If there wasn't a matching command, fall back to architectural resets
>>>>>>>>>> +     */
>>>>>>>>>> +    if (data && !psci_vendor_system_reset2(data))
>>>>>>>>>> +        return NOTIFY_DONE;
>>>>>>>>>> +
>>>>>>>>>>       if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>>>>>>>           psci_system_reset2_supported) {
>>>>>>>>>>           /*
>>>>>>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>>>>>>>>>>       {},
>>>>>>>>>>   };
>>>>>>>>>>   +#define REBOOT_PREFIX "mode-"
>>>>>>>>>> +
>>>>>>>>>> +static int __init psci_init_system_reset2_modes(void)
>>>>>>>>>> +{
>>>>>>>>>> +    const size_t len = strlen(REBOOT_PREFIX);
>>>>>>>>>> +    struct psci_reset_param *param;
>>>>>>>>>> +    struct device_node *psci_np __free(device_node) = NULL;
>>>>>>>>>> +    struct device_node *np __free(device_node) = NULL;
>>>>>>>>>> +    struct property *prop;
>>>>>>>>>> +    size_t count = 0;
>>>>>>>>>> +    u32 magic[2];
>>>>>>>>>> +    int num;
>>>>>>>>>> +
>>>>>>>>>> +    if (!psci_system_reset2_supported)
>>>>>>>>>> +        return 0;
>>>>>>>>>> +
>>>>>>>>>> +    psci_np = of_find_matching_node(NULL, psci_of_match);
>>>>>>>>>> +    if (!psci_np)
>>>>>>>>>> +        return 0;
>>>>>>>>>> +
>>>>>>>>>> +    np = of_find_node_by_name(psci_np, "reset-types");
>>>>>>>>>> +    if (!np)
>>>>>>>>>> +        return 0;
>>>>>>>>>
>>>>>>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
>>>>>>>>> is the actual reset to be issued, should we add a default mode "cold"
>>>>>>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>>>>>>>>>
>>>>>>>>> It all boils down to what *arg represents - adding "cold" and "warm"
>>>>>>>>> modes would remove the dependency on reboot_mode for resets issued
>>>>>>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
>>>>>>>>> is the correct thing to do.
>>>>>>>>>
>>>>>>>>> Comments very welcome.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Lorenzo
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +    for_each_property_of_node(np, prop) {
>>>>>>>>>> +        if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>>>> +            continue;
>>>>>>>>>> +        num = of_property_count_u32_elems(np, prop->name);
>>>>>>>>>> +        if (num != 1 && num != 2)
>>>>>>>>>> +            continue;
>>>>>>>>>> +
>>>>>>>>>> +        count++;
>>>>>>>>>> +    }
>>>>>>>>>> +
>>>>>>>>>> +    param = psci_reset_params =
>>>>>>>>>> +        kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
>>>>>>>>>> +    if (!psci_reset_params)
>>>>>>>>>> +        return -ENOMEM;
>>>>>>>>>> +
>>>>>>>>>> +    for_each_property_of_node(np, prop) {
>>>>>>>>>> +        if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>>>> +            continue;
>>>>>>>>>> +
>>>>>>>>>> +        num = of_property_read_variable_u32_array(np, prop->name, magic,
>>>>>>>>>> +                              1, ARRAY_SIZE(magic));
>>>>>>>>>> +        if (num < 0) {
>>>>>>>>>> +            pr_warn("Failed to parse vendor reboot mode %s\n",
>>>>>>>>>> +                param->mode);
>>>>>>>>>> +            kfree_const(param->mode);
>>>>>>>>>> +            continue;
>>>>>>>>>> +        }
>>>>>>>>>> +
>>>>>>>>>> +        param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>>>>>>>> +        if (!param->mode)
>>>>>>>>>> +            continue;
>>>>>>>>>> +
>>>>>>>>>> +        /* Force reset type to be in vendor space */
>>>>>>>>>> +        param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
>>>>>>>>>> +        param->cookie = num > 1 ? magic[1] : 0;
>>>>>>>>>> +        param++;
>>>>>>>>>> +        num_psci_reset_params++;
>>>>>>>>>> +    }
>>>>>>>>>> +
>>>>>>>>>> +    return 0;
>>>>>>>>>> +}
>>>>>>>>>> +arch_initcall(psci_init_system_reset2_modes);
>>>>>>>>>> +
>>>>>>>>>>   int __init psci_dt_init(void)
>>>>>>>>>>   {
>>>>>>>>>>       struct device_node *np;
>>>>>>>>>>
>>>>>>>>>> -- 
>>>>>>>>>> 2.34.1
>>>>>>>>>>
>>>
> 
> 

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Dmitry Baryshkov]

On Thu, 19 Jun 2025 at 15:32, Shivendra Pratap <quic_spratap@quicinc.com> wrote:
>
>
>
> On 6/19/2025 4:34 PM, Dmitry Baryshkov wrote:
> > On 19/06/2025 12:00, Shivendra Pratap wrote:
> >>
> >>
> >> On 6/18/2025 6:44 PM, Dmitry Baryshkov wrote:
> >>> On Tue, May 06, 2025 at 11:03:55PM +0530, Shivendra Pratap wrote:
> >>>>
> >>>>
> >>>> On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
> >>>>> On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
> >>>>>>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
> >>>>>>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
> >>>>>>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
> >>>>>>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >>>>>>>>>>
> >>>>>>>>>> SoC vendors have different types of resets and are controlled through
> >>>>>>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
> >>>>>>>>>> "download mode" that allows a RAM dump to be collected. Another example
> >>>>>>>>>> is they also support writing a cookie that can be read by bootloader
> >>>>>>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> >>>>>>>>>> vendor reset types to be implemented without requiring drivers for every
> >>>>>>>>>> register/cookie.
> >>>>>>>>>>
> >>>>>>>>>> Add support in PSCI to statically map reboot mode commands from
> >>>>>>>>>> userspace to a vendor reset and cookie value using the device tree.
> >>>>>>>>>
> >>>>>>>>> I have managed to discuss a little bit this patchset over the last
> >>>>>>>>> few days and I think we have defined a plan going forward.
> >>>>>>>>>
> >>>>>>>>> A point that was raised is:
> >>>>>>>>>
> >>>>>>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
> >>>>>>>>>
> >>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> >>>>>>>>> represent ?
> >>>>>>>>>
> >>>>>>>>> Is it the mode the system should reboot into OR it is the
> >>>>>>>>> actual command to be issued (which is what this patchset
> >>>>>>>>> implements) ?
> >>>>>>>>>
> >>>>>>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
> >>>>>>>>>
> >>>>>>>>> It is unclear what "default" means. We wonder whether the
> >>>>>>>>> reboot_mode variable was introduced to _define_ that "default".
> >>>>>>>>>
> >>>>>>>>> So, in short, my aim is trying to decouple reboot_mode from the
> >>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
> >>>>>>>>>
> >>>>>>>>> I believe that adding a sysfs interface to reboot-mode driver
> >>>>>>>>> infrastructure would be useful, so that the commands would
> >>>>>>>>> be exposed to userspace and userspace can set the *arg command
> >>>>>>>>> specifically to issue a given reset/mode.
> >>>>>>>>>
> >>>>>>>>> I wonder why this is not already in place for eg syscon-reboot-mode
> >>>>>>>>> resets, how does user space issue a command in those systems if the
> >>>>>>>>> available commands aren't exposed to userspace ?
> >>>>>>>>>
> >>>>>>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
> >>>>>>>>>
> >>>>>>>>>> A separate initcall is needed to parse the devicetree, instead of using
> >>>>>>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
> >>>>>>>>>>
> >>>>>>>>>> Reboot mode framework is close but doesn't quite fit with the
> >>>>>>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> >>>>>>>>>> be solved but doesn't seem reasonable in sum:
> >>>>>>>>>>   1. reboot mode registers against the reboot_notifier_list, which is too
> >>>>>>>>>>      early to call SYSTEM_RESET2. PSCI would need to remember the reset
> >>>>>>>>>>      type from the reboot-mode framework callback and use it
> >>>>>>>>>>      psci_sys_reset.
> >>>>>>>>>>   2. reboot mode assumes only one cookie/parameter is described in the
> >>>>>>>>>>      device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> >>>>>>>>>>      cookie.
> >>>>>>>>>
> >>>>>>>>> This can be changed and I think it should, so that the reboot modes
> >>>>>>>>> are exposed to user space and PSCI can use that.
> >>>>>>>>>
> >>>>>>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
> >>>>>>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
> >>>>>>>> be the last call from Linux, and ideally, this call should not fail.
> >>>>>>>>
> >>>>>>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
> >>>>>>>> notifiers
> >>>>>>>>
> >>>>>>>> So, if I understand correctly, you mean that we can change the reboot
> >>>>>>>> mode framework to expose the arguments available to user space. We can
> >>>>>>>> extend it to accept magic and cookies, save them in the reboot
> >>>>>>>> framework, and retrieve them via a call from PSCI during a regular
> >>>>>>>> reboot or panic based on the current arguments. Is this leading towards
> >>>>>>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
> >>>>>>>> notifier callback saves the magic and cookies, and these magic and
> >>>>>>>> cookies will be used during psci_sys_reset2()? Or is there something
> >>>>>>>> wrong with my understanding?
> >>>>>>>
> >>>>>>> No, you got it right (apologies for the delay in replying) - if the
> >>>>>>> case for making reboot mode available to user space is accepted.
> >>>>>>>
> >>>> While moving this into reboot-mode framework, one more query came up.
> >>>> The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
> >>>> to be a Platform device driver for using reboot-mode framework.
> >>>
> >>> No, it doesn't. It rqeuires struct device, but there is no requirement
> >>> for struct platform_device at any place.
> >> yes, it can be struct device so may be create a virtual device
> >> using reset-type node?
> >
> > It can be created, but I don't see a strong need for it.
> >
> >>>
> >>>> As psci is not a platform device driver, a subdevice under it may not probe as a
> >>>> platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
> >>>> early_initcall("psci_xyz") and then create a platform device something as
> >>>> below or any other suggestions for this?
> >>>
> >>> Change struct reboot_mode_driver to pass corresponding of_node (or
> >>> better fwnode) directly.  Corresponding device is used only in the
> >>> reboot_mode_register() and only to access of-node or to print error
> >>> messages.
> >> struct reboot_mode_driver can be changed just to pass of_node. But then the other
> >> suggestion was to expose sysfs from reboot-mode to show available commands.
> >> For that we need a device. Any suggestion? A virtual device with reset-types node
> >> passed to reboot-mode framework looks fine?
> >
> > You still don't need it. You'll create a new device, belonging to the new 'reboot' or 'reset' class to hold corresponding attributes.
> just understand this - So the reboot-mode framework will create a new class
> and a device and expose the supported commands?

Yes. Otherwise how would you create a vendor-independent userspace API?

> >
> >>>
> >>>>
> >>>> power:reset:<psci-vendor-reset-driver>:
> >>>> -----
> >>>> static int __init psci_vendor_reset_init(void) {
> >>>> ..
> >>>> ..
> >>>>     np = of_find_node_by_name(NULL, "psci-vendor-reset");
> >>>>     if(!np)
> >>>>         return -ENODEV;
> >>>>     pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
> >>>> ..
> >>>> ..
> >>>> }
> >>>> -------
> >>>>
> >>>> the sysfs we will expose from reboot-mode may show like below in above
> >>>> implementation:
> >>>>
> >>>> ######
> >>>> / # cat ./sys/devices/platform/psci-vendor-reset/available_modes
> >>>> bootloader edl
> >>>> ######
> >>>>
> >>>> thanks,
> >>>> Shivendra
> >>>>
> >>>>>>
> >>>>>> Agree that the available modes should be exposed to usespace via sysfs interface
> >>>>>> and we should implement it. Also #1 and #2 can be handled via some
> >>>>>> changes in the design as mentioned in above discussion.
> >>>>>>
> >>>>>> I have one doubt though when we implement this via reboot-mode framework.
> >>>>>> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
> >>>>>> psci driver is initialized very early at boot but potential ARM psci reboot-mode
> >>>>>> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
> >>>>>> types functionality will not be available in psci reset path until the reboot-mode
> >>>>>> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
> >>>>>> types for early device resets?
> >>>>>>
> >>>>>> One use-case may be an early device crash or a early reset where a vendor
> >>>>>> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a
> >>>>>> specific state but may not be able to use this driver.
> >>>>>> (eg: a kernel panic at early boot where a vendor wants to reset device
> >>>>>> to a specific state using vendor reset. Currently panic passes a NULL
> >>>>>> (*arg command) while device reset but it may be explored for vendor specific
> >>>>>> reset).
> >>>>>
> >>>>> As you said, that would not be a PSCI only issue - *if* we wanted to
> >>>>> plug in this use case we should find a way to do it at reboot mode
> >>>>> driver level.
> >>>>>
> >>>>> As a matter of fact, this is not a mainline issue AFAICS.
> >>>>>
> >>>>> Even if we did not design this as a reboot mode driver there would be a
> >>>>> time window where you would not be able to use vendor resets on panic.
> >>>>>
> >>>>> I don't see it as a major roadblock at the moment.
> >>>> Got it.
> >>>>>
> >>>>> Thanks,
> >>>>> Lorenzo
> >>>>>
> >>>>>>
> >>>>>> - Shivendra
> >>>>>>
> >>>>>>>> P.S. We appreciate Elliot for his work and follow-up on this while being
> >>>>>>>> employed at Qualcomm.
> >>>>>>>
> >>>>>>> Yes I sincerely do for his patience, thank you.
> >>>>>>>
> >>>>>>> Lorenzo
> >>>>>>>
> >>>>>>>>>>   3. psci cpuidle driver already registers a driver against the
> >>>>>>>>>>      arm,psci-1.0 compatible. Refactoring would be needed to have both a
> >>>>>>>>>>      cpuidle and reboot-mode driver.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
> >>>>>>>>>> ---
> >>>>>>>>>>   drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> >>>>>>>>>>   1 file changed, 105 insertions(+)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> >>>>>>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
> >>>>>>>>>> --- a/drivers/firmware/psci/psci.c
> >>>>>>>>>> +++ b/drivers/firmware/psci/psci.c
> >>>>>>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
> >>>>>>>>>>   static bool psci_system_reset2_supported;
> >>>>>>>>>>   static bool psci_system_off2_hibernate_supported;
> >>>>>>>>>>   +struct psci_reset_param {
> >>>>>>>>>> +    const char *mode;
> >>>>>>>>>> +    u32 reset_type;
> >>>>>>>>>> +    u32 cookie;
> >>>>>>>>>> +};
> >>>>>>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
> >>>>>>>>>> +static size_t num_psci_reset_params __ro_after_init;
> >>>>>>>>>> +
> >>>>>>>>>>   static inline bool psci_has_ext_power_state(void)
> >>>>>>>>>>   {
> >>>>>>>>>>       return psci_cpu_suspend_feature &
> >>>>>>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
> >>>>>>>>>>       return 0;
> >>>>>>>>>>   }
> >>>>>>>>>>   +static int psci_vendor_system_reset2(const char *cmd)
> >>>>>>>>>> +{
> >>>>>>>>>> +    unsigned long ret;
> >>>>>>>>>> +    size_t i;
> >>>>>>>>>> +
> >>>>>>>>>> +    for (i = 0; i < num_psci_reset_params; i++) {
> >>>>>>>>>> +        if (!strcmp(psci_reset_params[i].mode, cmd)) {
> >>>>>>>>>> +            ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> >>>>>>>>>> +                         psci_reset_params[i].reset_type,
> >>>>>>>>>> +                         psci_reset_params[i].cookie, 0);
> >>>>>>>>>> +            /*
> >>>>>>>>>> +             * if vendor reset fails, log it and fall back to
> >>>>>>>>>> +             * architecture reset types
> >>>>>>>>>
> >>>>>>>>> That's not what the code does.
> >>>>>>>>>
> >>>>>>>> Ack.
> >>>>>>>>
> >>>>>>>> -Mukesh
> >>>>>>>>
> >>>>>>>>>> +             */
> >>>>>>>>>> +            pr_err("failed to perform reset \"%s\": %ld\n", cmd,
> >>>>>>>>>> +                   (long)ret);
> >>>>>>>>>> +            return 0;
> >>>>>>>>>> +        }
> >>>>>>>>>> +    }
> >>>>>>>>>> +
> >>>>>>>>>> +    return -ENOENT;
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>>   static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> >>>>>>>>>>                 void *data)
> >>>>>>>>>>   {
> >>>>>>>>>> +    /*
> >>>>>>>>>> +     * try to do the vendor system_reset2
> >>>>>>>>>> +     * If there wasn't a matching command, fall back to architectural resets
> >>>>>>>>>> +     */
> >>>>>>>>>> +    if (data && !psci_vendor_system_reset2(data))
> >>>>>>>>>> +        return NOTIFY_DONE;
> >>>>>>>>>> +
> >>>>>>>>>>       if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >>>>>>>>>>           psci_system_reset2_supported) {
> >>>>>>>>>>           /*
> >>>>>>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
> >>>>>>>>>>       {},
> >>>>>>>>>>   };
> >>>>>>>>>>   +#define REBOOT_PREFIX "mode-"
> >>>>>>>>>> +
> >>>>>>>>>> +static int __init psci_init_system_reset2_modes(void)
> >>>>>>>>>> +{
> >>>>>>>>>> +    const size_t len = strlen(REBOOT_PREFIX);
> >>>>>>>>>> +    struct psci_reset_param *param;
> >>>>>>>>>> +    struct device_node *psci_np __free(device_node) = NULL;
> >>>>>>>>>> +    struct device_node *np __free(device_node) = NULL;
> >>>>>>>>>> +    struct property *prop;
> >>>>>>>>>> +    size_t count = 0;
> >>>>>>>>>> +    u32 magic[2];
> >>>>>>>>>> +    int num;
> >>>>>>>>>> +
> >>>>>>>>>> +    if (!psci_system_reset2_supported)
> >>>>>>>>>> +        return 0;
> >>>>>>>>>> +
> >>>>>>>>>> +    psci_np = of_find_matching_node(NULL, psci_of_match);
> >>>>>>>>>> +    if (!psci_np)
> >>>>>>>>>> +        return 0;
> >>>>>>>>>> +
> >>>>>>>>>> +    np = of_find_node_by_name(psci_np, "reset-types");
> >>>>>>>>>> +    if (!np)
> >>>>>>>>>> +        return 0;
> >>>>>>>>>
> >>>>>>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> >>>>>>>>> is the actual reset to be issued, should we add a default mode "cold"
> >>>>>>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
> >>>>>>>>>
> >>>>>>>>> It all boils down to what *arg represents - adding "cold" and "warm"
> >>>>>>>>> modes would remove the dependency on reboot_mode for resets issued
> >>>>>>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> >>>>>>>>> is the correct thing to do.
> >>>>>>>>>
> >>>>>>>>> Comments very welcome.
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Lorenzo
> >>>>>>>>>
> >>>>>>>>>> +
> >>>>>>>>>> +    for_each_property_of_node(np, prop) {
> >>>>>>>>>> +        if (strncmp(prop->name, REBOOT_PREFIX, len))
> >>>>>>>>>> +            continue;
> >>>>>>>>>> +        num = of_property_count_u32_elems(np, prop->name);
> >>>>>>>>>> +        if (num != 1 && num != 2)
> >>>>>>>>>> +            continue;
> >>>>>>>>>> +
> >>>>>>>>>> +        count++;
> >>>>>>>>>> +    }
> >>>>>>>>>> +
> >>>>>>>>>> +    param = psci_reset_params =
> >>>>>>>>>> +        kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
> >>>>>>>>>> +    if (!psci_reset_params)
> >>>>>>>>>> +        return -ENOMEM;
> >>>>>>>>>> +
> >>>>>>>>>> +    for_each_property_of_node(np, prop) {
> >>>>>>>>>> +        if (strncmp(prop->name, REBOOT_PREFIX, len))
> >>>>>>>>>> +            continue;
> >>>>>>>>>> +
> >>>>>>>>>> +        num = of_property_read_variable_u32_array(np, prop->name, magic,
> >>>>>>>>>> +                              1, ARRAY_SIZE(magic));
> >>>>>>>>>> +        if (num < 0) {
> >>>>>>>>>> +            pr_warn("Failed to parse vendor reboot mode %s\n",
> >>>>>>>>>> +                param->mode);
> >>>>>>>>>> +            kfree_const(param->mode);
> >>>>>>>>>> +            continue;
> >>>>>>>>>> +        }
> >>>>>>>>>> +
> >>>>>>>>>> +        param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> >>>>>>>>>> +        if (!param->mode)
> >>>>>>>>>> +            continue;
> >>>>>>>>>> +
> >>>>>>>>>> +        /* Force reset type to be in vendor space */
> >>>>>>>>>> +        param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
> >>>>>>>>>> +        param->cookie = num > 1 ? magic[1] : 0;
> >>>>>>>>>> +        param++;
> >>>>>>>>>> +        num_psci_reset_params++;
> >>>>>>>>>> +    }
> >>>>>>>>>> +
> >>>>>>>>>> +    return 0;
> >>>>>>>>>> +}
> >>>>>>>>>> +arch_initcall(psci_init_system_reset2_modes);
> >>>>>>>>>> +
> >>>>>>>>>>   int __init psci_dt_init(void)
> >>>>>>>>>>   {
> >>>>>>>>>>       struct device_node *np;
> >>>>>>>>>>
> >>>>>>>>>> --
> >>>>>>>>>> 2.34.1
> >>>>>>>>>>
> >>>
> >
> >



-- 
With best wishes
Dmitry

Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

[Shivendra Pratap]

On 6/19/2025 7:46 PM, Dmitry Baryshkov wrote:
> On Thu, 19 Jun 2025 at 15:32, Shivendra Pratap <quic_spratap@quicinc.com> wrote:
>>
>>
>>
>> On 6/19/2025 4:34 PM, Dmitry Baryshkov wrote:
>>> On 19/06/2025 12:00, Shivendra Pratap wrote:
>>>>
>>>>
>>>> On 6/18/2025 6:44 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, May 06, 2025 at 11:03:55PM +0530, Shivendra Pratap wrote:
>>>>>>
>>>>>>
>>>>>> On 4/16/2025 5:35 PM, Lorenzo Pieralisi wrote:
>>>>>>> On Wed, Apr 09, 2025 at 11:48:24PM +0530, Shivendra Pratap wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/8/2025 8:46 PM, Lorenzo Pieralisi wrote:
>>>>>>>>> On Tue, Mar 25, 2025 at 07:33:36PM +0530, Mukesh Ojha wrote:
>>>>>>>>>> On Fri, Mar 14, 2025 at 12:19:31PM +0100, Lorenzo Pieralisi wrote:
>>>>>>>>>>> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>>>>>>>>>>>> From: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>>>>>
>>>>>>>>>>>> SoC vendors have different types of resets and are controlled through
>>>>>>>>>>>> various registers. For instance, Qualcomm chipsets can reboot to a
>>>>>>>>>>>> "download mode" that allows a RAM dump to be collected. Another example
>>>>>>>>>>>> is they also support writing a cookie that can be read by bootloader
>>>>>>>>>>>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>>>>>>>>>>>> vendor reset types to be implemented without requiring drivers for every
>>>>>>>>>>>> register/cookie.
>>>>>>>>>>>>
>>>>>>>>>>>> Add support in PSCI to statically map reboot mode commands from
>>>>>>>>>>>> userspace to a vendor reset and cookie value using the device tree.
>>>>>>>>>>>
>>>>>>>>>>> I have managed to discuss a little bit this patchset over the last
>>>>>>>>>>> few days and I think we have defined a plan going forward.
>>>>>>>>>>>
>>>>>>>>>>> A point that was raised is:
>>>>>>>>>>>
>>>>>>>>>>> https://man7.org/linux/man-pages/man2/reboot.2.html
>>>>>>>>>>>
>>>>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
>>>>>>>>>>> represent ?
>>>>>>>>>>>
>>>>>>>>>>> Is it the mode the system should reboot into OR it is the
>>>>>>>>>>> actual command to be issued (which is what this patchset
>>>>>>>>>>> implements) ?
>>>>>>>>>>>
>>>>>>>>>>> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>>>>>>>>>>>
>>>>>>>>>>> It is unclear what "default" means. We wonder whether the
>>>>>>>>>>> reboot_mode variable was introduced to _define_ that "default".
>>>>>>>>>>>
>>>>>>>>>>> So, in short, my aim is trying to decouple reboot_mode from the
>>>>>>>>>>> LINUX_REBOOT_CMD_RESTART2 *arg command.
>>>>>>>>>>>
>>>>>>>>>>> I believe that adding a sysfs interface to reboot-mode driver
>>>>>>>>>>> infrastructure would be useful, so that the commands would
>>>>>>>>>>> be exposed to userspace and userspace can set the *arg command
>>>>>>>>>>> specifically to issue a given reset/mode.
>>>>>>>>>>>
>>>>>>>>>>> I wonder why this is not already in place for eg syscon-reboot-mode
>>>>>>>>>>> resets, how does user space issue a command in those systems if the
>>>>>>>>>>> available commands aren't exposed to userspace ?
>>>>>>>>>>>
>>>>>>>>>>> Is there a kernel entity exposing those "modes" to userspace, somehow ?
>>>>>>>>>>>
>>>>>>>>>>>> A separate initcall is needed to parse the devicetree, instead of using
>>>>>>>>>>>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>>>>>>>>>>>>
>>>>>>>>>>>> Reboot mode framework is close but doesn't quite fit with the
>>>>>>>>>>>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>>>>>>>>>>>> be solved but doesn't seem reasonable in sum:
>>>>>>>>>>>>   1. reboot mode registers against the reboot_notifier_list, which is too
>>>>>>>>>>>>      early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>>>>>>>>>>>      type from the reboot-mode framework callback and use it
>>>>>>>>>>>>      psci_sys_reset.
>>>>>>>>>>>>   2. reboot mode assumes only one cookie/parameter is described in the
>>>>>>>>>>>>      device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>>>>>>>>>>>      cookie.
>>>>>>>>>>>
>>>>>>>>>>> This can be changed and I think it should, so that the reboot modes
>>>>>>>>>>> are exposed to user space and PSCI can use that.
>>>>>>>>>>>
>>>>>>>>>> In the case of a regular reboot or panic, the reboot/panic notifiers run
>>>>>>>>>> first, followed by the restart notifiers. The PSCI reset/reset2 should
>>>>>>>>>> be the last call from Linux, and ideally, this call should not fail.
>>>>>>>>>>
>>>>>>>>>> Reboot mode notifiers => restart notifiers or Panic notifiers => restart
>>>>>>>>>> notifiers
>>>>>>>>>>
>>>>>>>>>> So, if I understand correctly, you mean that we can change the reboot
>>>>>>>>>> mode framework to expose the arguments available to user space. We can
>>>>>>>>>> extend it to accept magic and cookies, save them in the reboot
>>>>>>>>>> framework, and retrieve them via a call from PSCI during a regular
>>>>>>>>>> reboot or panic based on the current arguments. Is this leading towards
>>>>>>>>>> writing an ARM-specific PSCI-reboot-mode driver, which in its reboot
>>>>>>>>>> notifier callback saves the magic and cookies, and these magic and
>>>>>>>>>> cookies will be used during psci_sys_reset2()? Or is there something
>>>>>>>>>> wrong with my understanding?
>>>>>>>>>
>>>>>>>>> No, you got it right (apologies for the delay in replying) - if the
>>>>>>>>> case for making reboot mode available to user space is accepted.
>>>>>>>>>
>>>>>> While moving this into reboot-mode framework, one more query came up.
>>>>>> The "ARM-specific PSCI-reboot-mode driver" that we are going to write needs
>>>>>> to be a Platform device driver for using reboot-mode framework.
>>>>>
>>>>> No, it doesn't. It rqeuires struct device, but there is no requirement
>>>>> for struct platform_device at any place.
>>>> yes, it can be struct device so may be create a virtual device
>>>> using reset-type node?
>>>
>>> It can be created, but I don't see a strong need for it.
>>>
>>>>>
>>>>>> As psci is not a platform device driver, a subdevice under it may not probe as a
>>>>>> platform driver. Is it ok to implement the "PSCI-reboot-mode driver" as a
>>>>>> early_initcall("psci_xyz") and then create a platform device something as
>>>>>> below or any other suggestions for this?
>>>>>
>>>>> Change struct reboot_mode_driver to pass corresponding of_node (or
>>>>> better fwnode) directly.  Corresponding device is used only in the
>>>>> reboot_mode_register() and only to access of-node or to print error
>>>>> messages.
>>>> struct reboot_mode_driver can be changed just to pass of_node. But then the other
>>>> suggestion was to expose sysfs from reboot-mode to show available commands.
>>>> For that we need a device. Any suggestion? A virtual device with reset-types node
>>>> passed to reboot-mode framework looks fine?
>>>
>>> You still don't need it. You'll create a new device, belonging to the new 'reboot' or 'reset' class to hold corresponding attributes.
>> just understand this - So the reboot-mode framework will create a new class
>> and a device and expose the supported commands?
> 
> Yes. Otherwise how would you create a vendor-independent userspace API?
Ack. thanks.
> 
>>>
>>>>>
>>>>>>
>>>>>> power:reset:<psci-vendor-reset-driver>:
>>>>>> -----
>>>>>> static int __init psci_vendor_reset_init(void) {
>>>>>> ..
>>>>>> ..
>>>>>>     np = of_find_node_by_name(NULL, "psci-vendor-reset");
>>>>>>     if(!np)
>>>>>>         return -ENODEV;
>>>>>>     pdev = of_platform_device_create(np, "psci-vendor-reset", NULL);
>>>>>> ..
>>>>>> ..
>>>>>> }
>>>>>> -------
>>>>>>
>>>>>> the sysfs we will expose from reboot-mode may show like below in above
>>>>>> implementation:
>>>>>>
>>>>>> ######
>>>>>> / # cat ./sys/devices/platform/psci-vendor-reset/available_modes
>>>>>> bootloader edl
>>>>>> ######
>>>>>>
>>>>>> thanks,
>>>>>> Shivendra
>>>>>>
>>>>>>>>
>>>>>>>> Agree that the available modes should be exposed to usespace via sysfs interface
>>>>>>>> and we should implement it. Also #1 and #2 can be handled via some
>>>>>>>> changes in the design as mentioned in above discussion.
>>>>>>>>
>>>>>>>> I have one doubt though when we implement this via reboot-mode framework.
>>>>>>>> The current patch implements PSCI ARM PSCI SYSTEM RESET2 vendor reset types.
>>>>>>>> psci driver is initialized very early at boot but potential ARM psci reboot-mode
>>>>>>>> driver will not probe at that stage and the ARM PSCI SYSTEM RESET2 vendor reset
>>>>>>>> types functionality will not be available in psci reset path until the reboot-mode
>>>>>>>> driver probes. Will this cause any limitation on usage of ARM's PSCI vendor-reset
>>>>>>>> types for early device resets?
>>>>>>>>
>>>>>>>> One use-case may be an early device crash or a early reset where a vendor
>>>>>>>> wants to use PSCI SYSTEM RESET2 vendor reset type to a reset the device to a
>>>>>>>> specific state but may not be able to use this driver.
>>>>>>>> (eg: a kernel panic at early boot where a vendor wants to reset device
>>>>>>>> to a specific state using vendor reset. Currently panic passes a NULL
>>>>>>>> (*arg command) while device reset but it may be explored for vendor specific
>>>>>>>> reset).
>>>>>>>
>>>>>>> As you said, that would not be a PSCI only issue - *if* we wanted to
>>>>>>> plug in this use case we should find a way to do it at reboot mode
>>>>>>> driver level.
>>>>>>>
>>>>>>> As a matter of fact, this is not a mainline issue AFAICS.
>>>>>>>
>>>>>>> Even if we did not design this as a reboot mode driver there would be a
>>>>>>> time window where you would not be able to use vendor resets on panic.
>>>>>>>
>>>>>>> I don't see it as a major roadblock at the moment.
>>>>>> Got it.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Lorenzo
>>>>>>>
>>>>>>>>
>>>>>>>> - Shivendra
>>>>>>>>
>>>>>>>>>> P.S. We appreciate Elliot for his work and follow-up on this while being
>>>>>>>>>> employed at Qualcomm.
>>>>>>>>>
>>>>>>>>> Yes I sincerely do for his patience, thank you.
>>>>>>>>>
>>>>>>>>> Lorenzo
>>>>>>>>>
>>>>>>>>>>>>   3. psci cpuidle driver already registers a driver against the
>>>>>>>>>>>>      arm,psci-1.0 compatible. Refactoring would be needed to have both a
>>>>>>>>>>>>      cpuidle and reboot-mode driver.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>   drivers/firmware/psci/psci.c | 105 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>   1 file changed, 105 insertions(+)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>>>>>>>>>> index a1ebbe9b73b136218e9d9f9b8daa7756b3ab2fbe..6f8c47deaec0225f26704e1f3bcad52603127a85 100644
>>>>>>>>>>>> --- a/drivers/firmware/psci/psci.c
>>>>>>>>>>>> +++ b/drivers/firmware/psci/psci.c
>>>>>>>>>>>> @@ -80,6 +80,14 @@ static u32 psci_cpu_suspend_feature;
>>>>>>>>>>>>   static bool psci_system_reset2_supported;
>>>>>>>>>>>>   static bool psci_system_off2_hibernate_supported;
>>>>>>>>>>>>   +struct psci_reset_param {
>>>>>>>>>>>> +    const char *mode;
>>>>>>>>>>>> +    u32 reset_type;
>>>>>>>>>>>> +    u32 cookie;
>>>>>>>>>>>> +};
>>>>>>>>>>>> +static struct psci_reset_param *psci_reset_params __ro_after_init;
>>>>>>>>>>>> +static size_t num_psci_reset_params __ro_after_init;
>>>>>>>>>>>> +
>>>>>>>>>>>>   static inline bool psci_has_ext_power_state(void)
>>>>>>>>>>>>   {
>>>>>>>>>>>>       return psci_cpu_suspend_feature &
>>>>>>>>>>>> @@ -306,9 +314,39 @@ static int get_set_conduit_method(const struct device_node *np)
>>>>>>>>>>>>       return 0;
>>>>>>>>>>>>   }
>>>>>>>>>>>>   +static int psci_vendor_system_reset2(const char *cmd)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +    unsigned long ret;
>>>>>>>>>>>> +    size_t i;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    for (i = 0; i < num_psci_reset_params; i++) {
>>>>>>>>>>>> +        if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>>>>>>>>>> +            ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>>>>>>>>>> +                         psci_reset_params[i].reset_type,
>>>>>>>>>>>> +                         psci_reset_params[i].cookie, 0);
>>>>>>>>>>>> +            /*
>>>>>>>>>>>> +             * if vendor reset fails, log it and fall back to
>>>>>>>>>>>> +             * architecture reset types
>>>>>>>>>>>
>>>>>>>>>>> That's not what the code does.
>>>>>>>>>>>
>>>>>>>>>> Ack.
>>>>>>>>>>
>>>>>>>>>> -Mukesh
>>>>>>>>>>
>>>>>>>>>>>> +             */
>>>>>>>>>>>> +            pr_err("failed to perform reset \"%s\": %ld\n", cmd,
>>>>>>>>>>>> +                   (long)ret);
>>>>>>>>>>>> +            return 0;
>>>>>>>>>>>> +        }
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> +    return -ENOENT;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>>   static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>>>>>>>>>>>>                 void *data)
>>>>>>>>>>>>   {
>>>>>>>>>>>> +    /*
>>>>>>>>>>>> +     * try to do the vendor system_reset2
>>>>>>>>>>>> +     * If there wasn't a matching command, fall back to architectural resets
>>>>>>>>>>>> +     */
>>>>>>>>>>>> +    if (data && !psci_vendor_system_reset2(data))
>>>>>>>>>>>> +        return NOTIFY_DONE;
>>>>>>>>>>>> +
>>>>>>>>>>>>       if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>>>>>>>>>>>>           psci_system_reset2_supported) {
>>>>>>>>>>>>           /*
>>>>>>>>>>>> @@ -795,6 +833,73 @@ static const struct of_device_id psci_of_match[] __initconst = {
>>>>>>>>>>>>       {},
>>>>>>>>>>>>   };
>>>>>>>>>>>>   +#define REBOOT_PREFIX "mode-"
>>>>>>>>>>>> +
>>>>>>>>>>>> +static int __init psci_init_system_reset2_modes(void)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +    const size_t len = strlen(REBOOT_PREFIX);
>>>>>>>>>>>> +    struct psci_reset_param *param;
>>>>>>>>>>>> +    struct device_node *psci_np __free(device_node) = NULL;
>>>>>>>>>>>> +    struct device_node *np __free(device_node) = NULL;
>>>>>>>>>>>> +    struct property *prop;
>>>>>>>>>>>> +    size_t count = 0;
>>>>>>>>>>>> +    u32 magic[2];
>>>>>>>>>>>> +    int num;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    if (!psci_system_reset2_supported)
>>>>>>>>>>>> +        return 0;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    psci_np = of_find_matching_node(NULL, psci_of_match);
>>>>>>>>>>>> +    if (!psci_np)
>>>>>>>>>>>> +        return 0;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    np = of_find_node_by_name(psci_np, "reset-types");
>>>>>>>>>>>> +    if (!np)
>>>>>>>>>>>> +        return 0;
>>>>>>>>>>>
>>>>>>>>>>> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
>>>>>>>>>>> is the actual reset to be issued, should we add a default mode "cold"
>>>>>>>>>>> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>>>>>>>>>>>
>>>>>>>>>>> It all boils down to what *arg represents - adding "cold" and "warm"
>>>>>>>>>>> modes would remove the dependency on reboot_mode for resets issued
>>>>>>>>>>> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
>>>>>>>>>>> is the correct thing to do.
>>>>>>>>>>>
>>>>>>>>>>> Comments very welcome.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Lorenzo
>>>>>>>>>>>
>>>>>>>>>>>> +
>>>>>>>>>>>> +    for_each_property_of_node(np, prop) {
>>>>>>>>>>>> +        if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>>>>>> +            continue;
>>>>>>>>>>>> +        num = of_property_count_u32_elems(np, prop->name);
>>>>>>>>>>>> +        if (num != 1 && num != 2)
>>>>>>>>>>>> +            continue;
>>>>>>>>>>>> +
>>>>>>>>>>>> +        count++;
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> +    param = psci_reset_params =
>>>>>>>>>>>> +        kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
>>>>>>>>>>>> +    if (!psci_reset_params)
>>>>>>>>>>>> +        return -ENOMEM;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    for_each_property_of_node(np, prop) {
>>>>>>>>>>>> +        if (strncmp(prop->name, REBOOT_PREFIX, len))
>>>>>>>>>>>> +            continue;
>>>>>>>>>>>> +
>>>>>>>>>>>> +        num = of_property_read_variable_u32_array(np, prop->name, magic,
>>>>>>>>>>>> +                              1, ARRAY_SIZE(magic));
>>>>>>>>>>>> +        if (num < 0) {
>>>>>>>>>>>> +            pr_warn("Failed to parse vendor reboot mode %s\n",
>>>>>>>>>>>> +                param->mode);
>>>>>>>>>>>> +            kfree_const(param->mode);
>>>>>>>>>>>> +            continue;
>>>>>>>>>>>> +        }
>>>>>>>>>>>> +
>>>>>>>>>>>> +        param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>>>>>>>>>> +        if (!param->mode)
>>>>>>>>>>>> +            continue;
>>>>>>>>>>>> +
>>>>>>>>>>>> +        /* Force reset type to be in vendor space */
>>>>>>>>>>>> +        param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
>>>>>>>>>>>> +        param->cookie = num > 1 ? magic[1] : 0;
>>>>>>>>>>>> +        param++;
>>>>>>>>>>>> +        num_psci_reset_params++;
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +arch_initcall(psci_init_system_reset2_modes);
>>>>>>>>>>>> +
>>>>>>>>>>>>   int __init psci_dt_init(void)
>>>>>>>>>>>>   {
>>>>>>>>>>>>       struct device_node *np;
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> 2.34.1
>>>>>>>>>>>>
>>>>>
>>>
>>>
> 
> 
> 

[PATCH v9 3/5] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types

[Elliot Berman]

From: Elliot Berman <elliot.berman@oss.qualcomm.com>

qcm6490-idp firmware supports vendor-defined SYSTEM_RESET2 types.
Describe the reset types: "bootloader" will cause device to reboot and
stop in the bootloader's fastboot mode. "edl" will cause device to
reboot into "emergency download mode", which permits loading images via
the Firehose protocol.

Co-developed-by: Shivendra Pratap <quic_spratap@quicinc.com>
Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 7 +++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi     | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index c5fb153614e1f1d32a8d074d96829beb643c6919..471e71ab0db4db68d776e6b840d93e4e541ed5d7 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -625,6 +625,13 @@ &pon_resin {
 	status = "okay";
 };
 
+&psci {
+	reset-types {
+		mode-bootloader = <0x10001 0x2>;
+		mode-edl = <0 0x1>;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 55db1c83ef5517f82504816baeee583695065225..07474bd2301edea0037da1194a941b8a3d2fa452 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -855,7 +855,7 @@ pmu-a78 {
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
 	};
 
-	psci {
+	psci: psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
 

-- 
2.34.1

[PATCH v9 4/5] arm64: dts: qcom: qcs6490-rb3gen2: Add PSCI SYSTEM_RESET2 types

[Elliot Berman]

From: Elliot Berman <elliot.berman@oss.qualcomm.com>

qcs6490-rb3gen2 firmware supports vendor-defined SYSTEM_RESET2 types.
Describe the reset types: "bootloader" will cause device to reboot and
stop in the bootloader's fastboot mode. "edl" will cause device to
reboot into "emergency download mode", which permits loading images via
the Firehose protocol.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 27695bd5422055123e43b4081964b91bf252f274..c14a36a79db93766d8719e05434bf4aaac4deec2 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -758,6 +758,13 @@ &pon_resin {
 	status = "okay";
 };
 
+&psci {
+	reset-types {
+		mode-bootloader = <0x10001 0x2>;
+		mode-edl = <0 0x1>;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };

-- 
2.34.1

[PATCH v9 5/5] arm64: dts: qcom: sa8775p-ride: Add PSCI SYSTEM_RESET2 types

[Elliot Berman]

From: Elliot Berman <elliot.berman@oss.qualcomm.com>

sa8775p-ride firmware supports vendor-defined SYSTEM_RESET2 types.
Describe the reset types: "bootloader" will cause device to reboot and
stop in the bootloader's fastboot mode. "edl" will cause device to
reboot into "emergency download mode", which permits loading images via
the Firehose protocol.

Co-developed-by: Shivendra Pratap <quic_spratap@quicinc.com>
Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 7 +++++++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 3fc62e12368969e7cca246ddb55920db88eeb1e6..27cf3d35c48d2d79fdf054cadb00d6f4ca3b3c4b 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -589,6 +589,13 @@ &pmm8654au_3_gpios {
 			  "GNSS_BOOT_MODE";
 };
 
+&psci {
+	reset-types {
+		mode-bootloader = <0x10001 0x2>;
+		mode-edl = <0 0x1>;
+	};
+};
+
 &qupv3_id_1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 9da62d7c4d27f4cae341242c799b61dd1b06f483..73c04e4f858a39b78f7f130f781a9a9ce1b0a0e7 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -392,7 +392,7 @@ pmu {
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
-	psci {
+	psci: psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
 

-- 
2.34.1