linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2
@ 2025-07-17 12:46 Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla, Elliot Berman, Konrad Dybcio, Konrad Dybcio

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

User-space should be able to reboot a device into different
operational boot-states supported by underlying bootloader and
firmware. Generally, some HW registers need to be written, based
on which the bootloader and firmware decide the next boot state
of device, after the reset. For example, a requirement on
Qualcomm platforms may state that reboot with "bootloader"
command, should reboot the device into bootloader flashing mode
and reboot with “edl” command, should reboot the device into an
Emergency flashing mode.  Setting up such reboots on Qualcomm
devices can be inconsistent across SoC platforms and may require
setting different HW registers, where some of these registers may
not be accessible to HLOS. These knobs evolve over product
generations and require more drivers.  PSCI defines a
vendor-specific reset in SYSTEM_RESET2 spec, which enables the
firmware to take care of underlying setting for any such
supported vendor-specific reboot. Qualcomm firmwares are
beginning to support and expose PSCI SYSTEM_RESET2
vendor-specific reset types to simplify driver requirements from
Linux. With such support added in the firmware, we now need a
Linux interface which can make use of the firmware calls for PSCI
vendor-specific resets. This will align such reboot requirement
across platforms and vendors.

The current psci driver supports two types of resets –
SYSTEM_RESET2 Arch warm-reset and SYSTEM_RESET cold-reset. The
patchset introduces the PSCI SYSTEM_RESET2 vendor-specific reset
into the reset path of the psci driver and aligns it to work with
reboot system call - LINUX_REBOOT_CMD_RESTART2, when used along
with a supported string-based command in “*arg”.

The patchset uses reboot-mode based commands, to define the
supported vendor reset-types commands in psci device tree node
and registers these commands with the reboot-mode framework.

The PSCI vendor-specific reset takes two arguments, being,
reset_type and cookie as defined by the spec. To accommodate this
requirement, enhance the reboot-mode framework to support two
32-bit arguments by switching to 64-bit magic values.

Along this line, the patchset also extends the reboot-mode
framework to add a non-device-based registration function, which
will allow drivers to register using device tree node, while
keeping backward compatibility for existing users of reboot-mode.
This will enable psci driver to register for reboot-mode and
implement a write function, which will save the magic and then
use it in psci reset path to make a vendor-specific reset call
into the firmware. In addition, the patchset will expose a sysfs
entry interface within reboot-mode which can be used by userspace
to view the supported reboot-mode commands.

The list of vendor-specific reset commands remains open due to
divergent requirements across vendors, but this can be
streamlined and standardized through dedicated device tree
bindings.

Currently three drivers register with reboot-mode framework -
syscon-reboot-mode, nvmem-reboot-mode and qcom-pon. Consolidated
list of commands currently added across various vendor DTs:
 mode-loader
 mode-normal
 mode-bootloader
 mode-charge
 mode-fastboot
 mode-reboot-ab-update
 mode-recovery
 mode-rescue
 mode-shutdown-thermal
 mode-shutdown-thermal-battery

Detailed list of commands being used by syscon-reboot-mode:
    arm64/boot/dts/exynos/exynosautov9.dtsi:
	mode-bootloader = <EXYNOSAUTOV9_BOOT_BOOTLOADER>;
	mode-fastboot = <EXYNOSAUTOV9_BOOT_FASTBOOT>;
	mode-recovery = <EXYNOSAUTOV9_BOOT_RECOVERY>;

    arm64/boot/dts/exynos/google/gs101.dtsi:
    	mode-bootloader = <0xfc>;
    	mode-charge = <0x0a>;
    	mode-fastboot = <0xfa>;
    	mode-reboot-ab-update = <0x52>;
    	mode-recovery = <0xff>;
    	mode-rescue = <0xf9>;
    	mode-shutdown-thermal = <0x51>;
    	mode-shutdown-thermal-battery = <0x51>;

    arm64/boot/dts/hisilicon/hi3660-hikey960.dts:
    	mode-normal = <0x77665501>;
    	mode-bootloader = <0x77665500>;
    	mode-recovery = <0x77665502>;

    arm64/boot/dts/hisilicon/hi6220-hikey.dts:
    	mode-normal = <0x77665501>;
    	mode-bootloader = <0x77665500>;
    	mode-recovery = <0x77665502>;

    arm64/boot/dts/rockchip/px30.dtsi:
    	mode-bootloader = <BOOT_BL_DOWNLOAD>;
    	mode-fastboot = <BOOT_FASTBOOT>;
    	mode-loader = <BOOT_BL_DOWNLOAD>;
    	mode-normal = <BOOT_NORMAL>;
    	mode-recovery = <BOOT_RECOVERY>;

    arm64/boot/dts/rockchip/rk3308.dtsi:
    	mode-bootloader = <BOOT_BL_DOWNLOAD>;
    	mode-loader = <BOOT_BL_DOWNLOAD>;
    	mode-normal = <BOOT_NORMAL>;
    	mode-recovery = <BOOT_RECOVERY>;
    	mode-fastboot = <BOOT_FASTBOOT>;

    arm64/boot/dts/rockchip/rk3566-lckfb-tspi.dts:
    	mode-normal = <BOOT_NORMAL>;
    	mode-loader = <BOOT_BL_DOWNLOAD>;
			mode-recovery = <BOOT_RECOVERY>;
			mode-bootloader = <BOOT_FASTBOOT>;

Detailed list of commands being used by nvmem-reboot-mode:
    arm64/boot/dts/qcom/pmXXXX.dtsi:(multiple qcom DTs)
			mode-recovery = <0x01>;
			mode-bootloader = <0x02>;

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>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>

Changes in v11:
- Remove reference of cookie in reboot-mode – Arnd/Rob
- Introduce 64-bit magic in reboot-mode to accommodate two 32-bit
  arguments – Arnd
- Change reset-type to reboot-mode in psci device tree binding – Arnd
	- binding no more mandates two arguments as in v10.
	- dt changes done to support this binding.
- Remove obvious comments in psci reset path – Konrad
- Merge sysfs and ABI doc into single patch.
- Fix compilation issue on X86 configs.
- Fix warnings for pr_fmt.
- Link to v10: https://lore.kernel.org/all/569f154d-c714-1714-b898-83a42a38771c@oss.qualcomm.com/

Changes in V10:
- Change in reset-type binding to make cookie as a mandatory
  argument.
- Change reboot-mode binding to support additional argument
  "cookie".
 From Lorenzo:
- Use reboot-mode framework for implementing vendor-resets.
- Modify reboot-mode framework to support two arguments
  (magic and cookie).
- Expose sysfs for supported reboot-modes commands.
- List out all existing reboot-mode commands and their users.
   - Added this to cover letter.
 From Dmitry:
- Modify reboot-mode to support non-device based registration.
- Modify reboot-mode to create a class and device to expose
  sysfs interface.
- Link to v9: https://lore.kernel.org/all/20250303-arm-psci-system_reset2-vendor-reboots-v9-0-b2cf4a20feda@oss.qualcomm.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 (4):
      dt-bindings: arm: Document reboot mode magic
      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

Shivendra Pratap (4):
      power: reset: reboot-mode: Add device tree node-based registration
      power: reset: reboot-mode: Add support for 64 bit magic
      firmware: psci: Implement vendor-specific resets as reboot-mode
      power: reset: reboot-mode: Expose sysfs for registered reboot_modes

 .../testing/sysfs-class-reboot-mode-reboot_modes   |  38 ++++++
 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/Kconfig                      |   1 +
 drivers/firmware/psci/psci.c                       |  53 +++++++-
 drivers/power/reset/nvmem-reboot-mode.c            |   5 +-
 drivers/power/reset/qcom-pon.c                     |   5 +-
 drivers/power/reset/reboot-mode.c                  | 142 ++++++++++++++++-----
 drivers/power/reset/syscon-reboot-mode.c           |   5 +-
 include/linux/reboot-mode.h                        |   7 +-
 14 files changed, 281 insertions(+), 43 deletions(-)
---
base-commit: 024e09e444bd2b06aee9d1f3fe7b313c7a2df1bb
change-id: 20250709-arm-psci-system_reset2-vendor-reboots-46c80044afcf

Best regards,
-- 
Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-18 18:37   ` Dmitry Baryshkov
  2025-07-17 12:46 ` [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla

The reboot-mode driver does not have a strict requirement for
device-based registration. It primarily uses the device's of_node
to read mode-<cmd> properties and the device pointer for logging.

Remove the dependency on struct device and introduce support for
Device Tree (DT) node-based registration. This enables drivers
that are not associated with a struct device to leverage the
reboot-mode framework.

Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/power/reset/reboot-mode.c | 29 +++++++++++++++++++----------
 include/linux/reboot-mode.h       |  2 +-
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index fba53f638da04655e756b5f8b7d2d666d1379535..0269ec55106472cf2f2b12bd65704dd0114bf4a3 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -3,13 +3,17 @@
  * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
  */
 
+#define pr_fmt(fmt)	"reboot-mode: " fmt
+
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/reboot.h>
 #include <linux/reboot-mode.h>
+#include <linux/slab.h>
 
 #define PREFIX "mode-"
 
@@ -65,33 +69,35 @@ static int reboot_mode_notify(struct notifier_block *this,
 /**
  * reboot_mode_register - register a reboot mode driver
  * @reboot: reboot mode driver
+ * @np: Pointer to device tree node
  *
  * Returns: 0 on success or a negative error code on failure.
  */
-int reboot_mode_register(struct reboot_mode_driver *reboot)
+int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np)
 {
 	struct mode_info *info;
 	struct property *prop;
-	struct device_node *np = reboot->dev->of_node;
 	size_t len = strlen(PREFIX);
 	int ret;
 
+	if (!np)
+		return -EINVAL;
+
 	INIT_LIST_HEAD(&reboot->head);
 
 	for_each_property_of_node(np, prop) {
 		if (strncmp(prop->name, PREFIX, len))
 			continue;
 
-		info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
+		info = kzalloc(sizeof(*info), GFP_KERNEL);
 		if (!info) {
 			ret = -ENOMEM;
 			goto error;
 		}
 
 		if (of_property_read_u32(np, prop->name, &info->magic)) {
-			dev_err(reboot->dev, "reboot mode %s without magic number\n",
-				info->mode);
-			devm_kfree(reboot->dev, info);
+			pr_err("reboot mode %s without magic number\n", info->mode);
+			kfree(info);
 			continue;
 		}
 
@@ -102,8 +108,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
 		} else if (info->mode[0] == '\0') {
 			kfree_const(info->mode);
 			ret = -EINVAL;
-			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
-				prop->name);
+			pr_err("invalid mode name(%s): too short!\n", prop->name);
 			goto error;
 		}
 
@@ -130,11 +135,15 @@ EXPORT_SYMBOL_GPL(reboot_mode_register);
 int reboot_mode_unregister(struct reboot_mode_driver *reboot)
 {
 	struct mode_info *info;
+	struct mode_info *next;
 
 	unregister_reboot_notifier(&reboot->reboot_notifier);
 
-	list_for_each_entry(info, &reboot->head, list)
+	list_for_each_entry_safe(info, next, &reboot->head, list) {
 		kfree_const(info->mode);
+		list_del(&info->list);
+		kfree(info);
+	}
 
 	return 0;
 }
@@ -162,7 +171,7 @@ int devm_reboot_mode_register(struct device *dev,
 	if (!dr)
 		return -ENOMEM;
 
-	rc = reboot_mode_register(reboot);
+	rc = reboot_mode_register(reboot, reboot->dev->of_node);
 	if (rc) {
 		devres_free(dr);
 		return rc;
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..36f071f4b82e1fc255d8dd679a18e537655c3179 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -9,7 +9,7 @@ struct reboot_mode_driver {
 	struct notifier_block reboot_notifier;
 };
 
-int reboot_mode_register(struct reboot_mode_driver *reboot);
+int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np);
 int reboot_mode_unregister(struct reboot_mode_driver *reboot);
 int devm_reboot_mode_register(struct device *dev,
 			      struct reboot_mode_driver *reboot);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-18 18:40   ` Dmitry Baryshkov
  2025-07-17 12:46 ` [PATCH v11 3/8] dt-bindings: arm: Document reboot mode magic Shivendra Pratap
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla

Current reboot-mode supports a single 32-bit argument for any
supported mode. Some reboot-mode based drivers may require
passing two independent 32-bit arguments during a reboot
sequence, for uses-cases, where a mode requires an additional
argument. Such drivers may not be able to use the reboot-mode
driver. For example, ARM PSCI vendor-specific resets, need two
arguments for its operation – reset_type and cookie, to complete
the reset operation. If a driver wants to implement this
firmware-based reset, it cannot use reboot-mode framework.

Introduce 64-bit magic values in reboot-mode driver to
accommodate dual 32-bit arguments when specified via device tree.
In cases, where no second argument is passed from device tree,
keep the upper 32-bit of magic un-changed(0) to maintain backward
compatibility.

Update the current drivers using reboot-mode for a 64-bit magic
value.

Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/power/reset/nvmem-reboot-mode.c  |  5 ++---
 drivers/power/reset/qcom-pon.c           |  5 ++---
 drivers/power/reset/reboot-mode.c        | 27 +++++++++++++++------------
 drivers/power/reset/syscon-reboot-mode.c |  5 ++---
 include/linux/reboot-mode.h              |  4 +++-
 5 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..e3eed943fdefa271b22e1f1891abef5c9095b9a5 100644
--- a/drivers/power/reset/nvmem-reboot-mode.c
+++ b/drivers/power/reset/nvmem-reboot-mode.c
@@ -16,15 +16,14 @@ struct nvmem_reboot_mode {
 	struct nvmem_cell *cell;
 };
 
-static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot,
-				    unsigned int magic)
+static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
 {
 	int ret;
 	struct nvmem_reboot_mode *nvmem_rbm;
 
 	nvmem_rbm = container_of(reboot, struct nvmem_reboot_mode, reboot);
 
-	ret = nvmem_cell_write(nvmem_rbm->cell, &magic, sizeof(magic));
+	ret = nvmem_cell_write(nvmem_rbm->cell, (u32 *)&magic, sizeof(u32));
 	if (ret < 0)
 		dev_err(reboot->dev, "update reboot mode bits failed\n");
 
diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
index 7e108982a582e8243c5c806bd4a793646b87189f..93daf93c097f970afbaf43d36b1e4f725ca7a81f 100644
--- a/drivers/power/reset/qcom-pon.c
+++ b/drivers/power/reset/qcom-pon.c
@@ -27,8 +27,7 @@ struct qcom_pon {
 	long reason_shift;
 };
 
-static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
-				    unsigned int magic)
+static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
 {
 	struct qcom_pon *pon = container_of
 			(reboot, struct qcom_pon, reboot_mode);
@@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
 	ret = regmap_update_bits(pon->regmap,
 				 pon->baseaddr + PON_SOFT_RB_SPARE,
 				 GENMASK(7, pon->reason_shift),
-				 magic << pon->reason_shift);
+				 ((u32)magic) << pon->reason_shift);
 	if (ret < 0)
 		dev_err(pon->dev, "update reboot mode bits failed\n");
 
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index 0269ec55106472cf2f2b12bd65704dd0114bf4a3..1196627fbf98d87eec57a3d4ee544e403e6eb946 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -19,12 +19,11 @@
 
 struct mode_info {
 	const char *mode;
-	u32 magic;
+	u64 magic;
 	struct list_head list;
 };
 
-static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
-					  const char *cmd)
+static struct mode_info *get_reboot_mode_info(struct reboot_mode_driver *reboot, const char *cmd)
 {
 	const char *normal = "normal";
 	struct mode_info *info;
@@ -35,11 +34,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
 
 	list_for_each_entry(info, &reboot->head, list)
 		if (!strcmp(info->mode, cmd))
-			return info->magic;
+			return info;
 
 	/* try to match again, replacing characters impossible in DT */
 	if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
-		return 0;
+		return NULL;
 
 	strreplace(cmd_, ' ', '-');
 	strreplace(cmd_, ',', '-');
@@ -47,21 +46,21 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
 
 	list_for_each_entry(info, &reboot->head, list)
 		if (!strcmp(info->mode, cmd_))
-			return info->magic;
+			return info;
 
-	return 0;
+	return NULL;
 }
 
 static int reboot_mode_notify(struct notifier_block *this,
 			      unsigned long mode, void *cmd)
 {
 	struct reboot_mode_driver *reboot;
-	unsigned int magic;
+	struct mode_info *info;
 
 	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
-	magic = get_reboot_mode_magic(reboot, cmd);
-	if (magic)
-		reboot->write(reboot, magic);
+	info = get_reboot_mode_info(reboot, cmd);
+	if (info)
+		reboot->write(reboot, info->magic);
 
 	return NOTIFY_DONE;
 }
@@ -78,6 +77,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
 	struct mode_info *info;
 	struct property *prop;
 	size_t len = strlen(PREFIX);
+	u32 magic_64;
 	int ret;
 
 	if (!np)
@@ -95,12 +95,15 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
 			goto error;
 		}
 
-		if (of_property_read_u32(np, prop->name, &info->magic)) {
+		if (of_property_read_u32(np, prop->name, (u32 *)&info->magic)) {
 			pr_err("reboot mode %s without magic number\n", info->mode);
 			kfree(info);
 			continue;
 		}
 
+		if (!of_property_read_u32_index(np, prop->name, 1, &magic_64))
+			info->magic |= (u64)magic_64 << 32;
+
 		info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
 		if (!info->mode) {
 			ret =  -ENOMEM;
diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
index e0772c9f70f7a19cd8ec8a0b7fdbbaa7ba44afd0..6783d05e80fbc2c812b45ffe69755478af90d30a 100644
--- a/drivers/power/reset/syscon-reboot-mode.c
+++ b/drivers/power/reset/syscon-reboot-mode.c
@@ -20,8 +20,7 @@ struct syscon_reboot_mode {
 	u32 mask;
 };
 
-static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
-				    unsigned int magic)
+static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
 {
 	struct syscon_reboot_mode *syscon_rbm;
 	int ret;
@@ -29,7 +28,7 @@ static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
 	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
 
 	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
-				 syscon_rbm->mask, magic);
+				 syscon_rbm->mask, (u32)magic);
 	if (ret < 0)
 		dev_err(reboot->dev, "update reboot mode bits failed\n");
 
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index 36f071f4b82e1fc255d8dd679a18e537655c3179..d9d9165a8635e5d55d92197a69c7fae179ac2045 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -2,10 +2,12 @@
 #ifndef __REBOOT_MODE_H__
 #define __REBOOT_MODE_H__
 
+#include <linux/types.h>
+
 struct reboot_mode_driver {
 	struct device *dev;
 	struct list_head head;
-	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
+	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
 	struct notifier_block reboot_notifier;
 };
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 3/8] dt-bindings: arm: Document reboot mode magic
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 4/8] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla, 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>
Signed-off-by: Shivendra Pratap <shivendra.pratap@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 7360a2849b5bd1e4cbadac533c1a7228573288d4..da3a584c76b48e313a5d4cb7c8e0c3a3628fd1bb 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
 
+  reboot-mode:
+    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";
+
+      reboot-mode {
+        mode-edl = <0>;
+        mode-bootloader = <1 2>;
+      };
+    };
 ...

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 4/8] firmware: psci: Implement vendor-specific resets as reboot-mode
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
                   ` (2 preceding siblings ...)
  2025-07-17 12:46 ` [PATCH v11 3/8] dt-bindings: arm: Document reboot mode magic Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 5/8] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types Shivendra Pratap
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla

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

Introduce a late_initcall to register PSCI vendor-specific resets
as reboot modes. Implement a reboot-mode write function that sets
reset_type and cookie values during the reboot notifier callback.
Introduce a firmware-based call for SYSTEM_RESET2 vendor-specific
reset in the psci_sys_reset path, using reset_type and cookie if
supported by secure firmware.

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

Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/firmware/psci/Kconfig |  1 +
 drivers/firmware/psci/psci.c  | 53 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/psci/Kconfig b/drivers/firmware/psci/Kconfig
index 97944168b5e66aea1e38a7eb2d4ced8348fce64b..9d65fe7b06a6429de8a26d06f9384e5c93f36e5f 100644
--- a/drivers/firmware/psci/Kconfig
+++ b/drivers/firmware/psci/Kconfig
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config ARM_PSCI_FW
 	bool
+	select REBOOT_MODE
 
 config ARM_PSCI_CHECKER
 	bool "ARM PSCI checker"
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..28018c283eb627bcb4ce6223c56899a43ed56399 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -17,6 +17,7 @@
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/reboot-mode.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
 
@@ -51,6 +52,14 @@ static int resident_cpu = -1;
 struct psci_operations psci_ops;
 static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
 
+struct psci_vendor_sysreset2 {
+	u32 reset_type;
+	u32 cookie;
+	bool valid;
+};
+
+static struct psci_vendor_sysreset2 vendor_reset;
+
 bool psci_tos_resident_on(int cpu)
 {
 	return cpu == resident_cpu;
@@ -309,7 +318,10 @@ static int get_set_conduit_method(const struct device_node *np)
 static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
 			  void *data)
 {
-	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
+	if (vendor_reset.valid && psci_system_reset2_supported) {
+		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
+			       vendor_reset.cookie, 0);
+	} else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
 	    psci_system_reset2_supported) {
 		/*
 		 * reset_type[31] = 0 (architectural)
@@ -547,6 +559,45 @@ static const struct platform_suspend_ops psci_suspend_ops = {
 	.enter          = psci_system_suspend_enter,
 };
 
+static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
+{
+	if (psci_system_reset2_supported) {
+		vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | (u32)magic;
+		vendor_reset.cookie = (u32)(magic >> 32);
+		vendor_reset.valid = true;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static int __init psci_init_vendor_reset(void)
+{
+	struct reboot_mode_driver *reboot;
+	struct device_node *np;
+	int ret;
+
+	np = of_find_node_by_path("/psci/reboot-mode");
+	if (!np)
+		return -ENODEV;
+
+	reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
+	if (!reboot) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	reboot->write = psci_set_vendor_sys_reset2;
+
+	ret = reboot_mode_register(reboot, np);
+	if (ret) {
+		of_node_put(np);
+		return ret;
+	}
+
+	return 0;
+}
+late_initcall(psci_init_vendor_reset)
+
 static void __init psci_init_system_reset2(void)
 {
 	int ret;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 5/8] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
                   ` (3 preceding siblings ...)
  2025-07-17 12:46 ` [PATCH v11 4/8] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 6/8] arm64: dts: qcom: qcs6490-rb3gen2: " Shivendra Pratap
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla, Elliot Berman, Konrad Dybcio

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

Add support for SYSTEM_RESET2 vendor-specific resets in
qcm6490-idp as reboot-modes.  Describe the resets: "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@linaro.org>
Signed-off-by: Elliot Berman <elliot.berman@oss.qualcomm.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@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 7a155ef6492e176390faa3f2dbe419f5cfa62f0d..d25dbdde7dcee9fa3f1bd48a38baeae0f063023c 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -654,6 +654,13 @@ &pon_resin {
 	status = "okay";
 };
 
+&psci {
+	reboot-mode {
+		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 64a2abd3010018e94eb50c534a509d6b4cf2473b..e6a99ef50b9b9ade9d2a71a14fcde429f8076dd5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -857,7 +857,7 @@ pmu-a78 {
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
 	};
 
-	psci {
+	psci: psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 6/8] arm64: dts: qcom: qcs6490-rb3gen2: Add PSCI SYSTEM_RESET2 types
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
                   ` (4 preceding siblings ...)
  2025-07-17 12:46 ` [PATCH v11 5/8] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 7/8] arm64: dts: qcom: sa8775p-ride: " Shivendra Pratap
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla, Elliot Berman, Konrad Dybcio

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

Add support for SYSTEM_RESET2 vendor-specific resets in
qcs6490-rb3gen2 as reboot-modes.  Describe the resets:
"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>
Signed-off-by: Shivendra Pratap <shivendra.pratap@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 5fbcd48f2e2d839835fa464a8d5682f00557f82e..8cf289a3cfc4ac7014885494c27fb003a7258519 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -966,6 +966,13 @@ &pon_resin {
 	status = "okay";
 };
 
+&psci {
+	reboot-mode {
+		mode-bootloader = <0x10001 0x2>;
+		mode-edl = <0 0x1>;
+	};
+};
+
 &qup_uart7_cts {
 	/*
 	 * Configure a bias-bus-hold on CTS to lower power

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 7/8] arm64: dts: qcom: sa8775p-ride: Add PSCI SYSTEM_RESET2 types
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
                   ` (5 preceding siblings ...)
  2025-07-17 12:46 ` [PATCH v11 6/8] arm64: dts: qcom: qcs6490-rb3gen2: " Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-17 12:46 ` [PATCH v11 8/8] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
  2025-07-18 21:29 ` [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Florian Fainelli
  8 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla, Elliot Berman, Konrad Dybcio

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

Add support for SYSTEM_RESET2 vendor-specific resets in
sa8775p-ride as reboot-modes.  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>
Signed-off-by: Shivendra Pratap <shivendra.pratap@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 63b3031cfcc15fe2cce58c0fb7502b19f058b80c..b1bbd3270afc65e9e036e8098ae7cec3d3363a32 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -874,6 +874,13 @@ &pmm8654au_3_gpios {
 			  "GNSS_BOOT_MODE";
 };
 
+&psci {
+	reboot-mode {
+		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 9997a29901f57d7894dc1eacb6a809caa427c6c4..52466607035e663f4dc6e5c97223d5c0404650e2 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -622,7 +622,7 @@ pmu {
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
-	psci {
+	psci: psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v11 8/8] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
                   ` (6 preceding siblings ...)
  2025-07-17 12:46 ` [PATCH v11 7/8] arm64: dts: qcom: sa8775p-ride: " Shivendra Pratap
@ 2025-07-17 12:46 ` Shivendra Pratap
  2025-07-18 21:29 ` [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Florian Fainelli
  8 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-17 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Shivendra Pratap,
	Srinivas Kandagatla

Currently, there is no standardized mechanism for userspace to
discover which reboot-modes are supported on a given platform.
This limitation forces tools and scripts to rely on hardcoded
assumptions about the supported reboot-modes.

Create a class 'reboot-mode' and a device under it to expose a
sysfs interface to show the available reboot mode arguments to
userspace.

Create the device using the node name of the driver that
registers with reboot mode driver.

This results in the creation of:
  /sys/class/reboot-mode/<driver>/reboot_modes

This read-only sysfs file will exposes the list of supported
reboot modes arguments provided by the driver, enabling userspace
to query the list of arguments.

Align the clean up path to maintain backward compatibility for
existing reboot-mode based drivers.

Add ABI documentation for /sys/class/reboot-mode/*/reboot_modes.

Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 .../testing/sysfs-class-reboot-mode-reboot_modes   | 38 +++++++++
 drivers/power/reset/reboot-mode.c                  | 96 +++++++++++++++++++---
 include/linux/reboot-mode.h                        |  1 +
 3 files changed, 123 insertions(+), 12 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-reboot-mode-reboot_modes b/Documentation/ABI/testing/sysfs-class-reboot-mode-reboot_modes
new file mode 100644
index 0000000000000000000000000000000000000000..63b3ef1ecc66c73744fc831fa9864592c514bd56
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-reboot-mode-reboot_modes
@@ -0,0 +1,38 @@
+What:		/sys/class/reboot-mode/<driver>/reboot_modes
+Date:		July 2025
+KernelVersion:	6.16
+Contact:	linux-pm@vger.kernel.org
+		Description:
+		This interface exposes the reboot-mode arguments
+		registered with the reboot-mode framework. It is
+		a read-only interface and provides a comma
+		separated list of reboot-mode arguments supported
+		on the current platform.
+		Example:
+		 normal,recovery,fastboot,bootloader
+
+		The exact sysfs path may vary depending on the
+		driver that registers the arguments.
+		Example:
+		 /sys/class/reboot-mode/reboot-mode/reboot_modes
+		 /sys/class/reboot-mode/pon/reboot_modes
+
+		The supported arguments can be used by userspace
+		to invoke device reset using the reboot() system
+		call, with the "argument" as string to "*arg"
+		parameter along with LINUX_REBOOT_CMD_RESTART2.
+		Example:
+		 reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2,
+		        LINUX_REBOOT_CMD_RESTART2, "bootloader");
+
+		A driver can expose the supported arguments by
+		registering them with the reboot-mode framework
+		using the property names that follow the
+		mode-<argument> format.
+		Example:
+		 mode-bootloader, mode-recovery.
+
+		This attribute is useful for scripts or initramfs
+		logic that need to programmatically determine
+		which reboot-mode arguments are valid before
+		triggering a reboot.
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index 1196627fbf98d87eec57a3d4ee544e403e6eb946..af3551fe2f625aacd31699604b7cbdd66c318c00 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -6,6 +6,7 @@
 #define pr_fmt(fmt)	"reboot-mode: " fmt
 
 #include <linux/device.h>
+#include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -23,6 +24,8 @@ struct mode_info {
 	struct list_head list;
 };
 
+static struct class *rb_class;
+
 static struct mode_info *get_reboot_mode_info(struct reboot_mode_driver *reboot, const char *cmd)
 {
 	const char *normal = "normal";
@@ -65,6 +68,79 @@ static int reboot_mode_notify(struct notifier_block *this,
 	return NOTIFY_DONE;
 }
 
+static void release_reboot_mode_device(struct device *dev, void *res);
+
+static ssize_t reboot_modes_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct reboot_mode_driver **devres_reboot;
+	struct reboot_mode_driver *reboot;
+	struct mode_info *info;
+	ssize_t size = 0;
+
+	devres_reboot = devres_find(dev, release_reboot_mode_device, NULL, 0);
+	reboot = *devres_reboot;
+	list_for_each_entry(info, &reboot->head, list) {
+		size += sprintf(buf + size, "%s,", info->mode);
+	}
+
+	if (size) {
+		size += sprintf(buf + size - 1, "\n");
+		return size;
+	}
+
+	return -ENODATA;
+}
+static DEVICE_ATTR_RO(reboot_modes);
+
+static void release_reboot_mode_device(struct device *dev, void *res)
+{
+	struct reboot_mode_driver *reboot = *(struct reboot_mode_driver **)res;
+	struct mode_info *info;
+	struct mode_info *next;
+
+	unregister_reboot_notifier(&reboot->reboot_notifier);
+
+	list_for_each_entry_safe(info, next, &reboot->head, list) {
+		kfree_const(info->mode);
+		list_del(&info->list);
+		kfree(info);
+	}
+
+	device_remove_file(reboot->dev, &dev_attr_reboot_modes);
+}
+
+static int create_reboot_mode_device(struct reboot_mode_driver *reboot,
+				     const char *dev_name)
+{
+	struct reboot_mode_driver **dr;
+	int ret = 0;
+
+	if (!rb_class) {
+		rb_class = class_create("reboot-mode");
+		if (IS_ERR(rb_class))
+			return PTR_ERR(rb_class);
+	}
+
+	reboot->reboot_dev = device_create(rb_class, NULL, 0, NULL, dev_name);
+	if (IS_ERR(reboot->reboot_dev))
+		return PTR_ERR(reboot->reboot_dev);
+
+	ret = device_create_file(reboot->reboot_dev, &dev_attr_reboot_modes);
+	if (ret)
+		return ret;
+
+	dr = devres_alloc(release_reboot_mode_device, sizeof(*dr), GFP_KERNEL);
+	if (!dr) {
+		device_remove_file(reboot->reboot_dev, &dev_attr_reboot_modes);
+		return -ENOMEM;
+	}
+
+	*dr = reboot;
+	devres_add(reboot->reboot_dev, dr);
+
+	return ret;
+}
+
 /**
  * reboot_mode_register - register a reboot mode driver
  * @reboot: reboot mode driver
@@ -83,6 +159,10 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
 	if (!np)
 		return -EINVAL;
 
+	ret = create_reboot_mode_device(reboot, np->name ? np->name : "reboot-mode-dev");
+	if (ret)
+		return ret;
+
 	INIT_LIST_HEAD(&reboot->head);
 
 	for_each_property_of_node(np, prop) {
@@ -137,24 +217,16 @@ EXPORT_SYMBOL_GPL(reboot_mode_register);
  */
 int reboot_mode_unregister(struct reboot_mode_driver *reboot)
 {
-	struct mode_info *info;
-	struct mode_info *next;
-
-	unregister_reboot_notifier(&reboot->reboot_notifier);
-
-	list_for_each_entry_safe(info, next, &reboot->head, list) {
-		kfree_const(info->mode);
-		list_del(&info->list);
-		kfree(info);
-	}
-
+	device_unregister(reboot->reboot_dev);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(reboot_mode_unregister);
 
 static void devm_reboot_mode_release(struct device *dev, void *res)
 {
-	reboot_mode_unregister(*(struct reboot_mode_driver **)res);
+	struct reboot_mode_driver *reboot = *(struct reboot_mode_driver **)res;
+
+	device_unregister(reboot->reboot_dev);
 }
 
 /**
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index d9d9165a8635e5d55d92197a69c7fae179ac2045..8fb35ef1c00749500a99d088b5f7010d57547844 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -6,6 +6,7 @@
 
 struct reboot_mode_driver {
 	struct device *dev;
+	struct device *reboot_dev;
 	struct list_head head;
 	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
 	struct notifier_block reboot_notifier;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration
  2025-07-17 12:46 ` [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
@ 2025-07-18 18:37   ` Dmitry Baryshkov
  2025-07-19 15:35     ` Shivendra Pratap
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2025-07-18 18:37 UTC (permalink / raw)
  To: Shivendra Pratap
  Cc: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla

On Thu, Jul 17, 2025 at 06:16:47PM +0530, Shivendra Pratap wrote:
> The reboot-mode driver does not have a strict requirement for
> device-based registration. It primarily uses the device's of_node
> to read mode-<cmd> properties and the device pointer for logging.
> 
> Remove the dependency on struct device and introduce support for
> Device Tree (DT) node-based registration. This enables drivers
> that are not associated with a struct device to leverage the
> reboot-mode framework.
> 
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/power/reset/reboot-mode.c | 29 +++++++++++++++++++----------
>  include/linux/reboot-mode.h       |  2 +-
>  2 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index fba53f638da04655e756b5f8b7d2d666d1379535..0269ec55106472cf2f2b12bd65704dd0114bf4a3 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -3,13 +3,17 @@
>   * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
>   */
>  
> +#define pr_fmt(fmt)	"reboot-mode: " fmt
> +
>  #include <linux/device.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
> +#include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/reboot.h>
>  #include <linux/reboot-mode.h>
> +#include <linux/slab.h>
>  
>  #define PREFIX "mode-"
>  
> @@ -65,33 +69,35 @@ static int reboot_mode_notify(struct notifier_block *this,
>  /**
>   * reboot_mode_register - register a reboot mode driver
>   * @reboot: reboot mode driver
> + * @np: Pointer to device tree node
>   *
>   * Returns: 0 on success or a negative error code on failure.
>   */
> -int reboot_mode_register(struct reboot_mode_driver *reboot)
> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np)
>  {
>  	struct mode_info *info;
>  	struct property *prop;
> -	struct device_node *np = reboot->dev->of_node;
>  	size_t len = strlen(PREFIX);
>  	int ret;
>  
> +	if (!np)
> +		return -EINVAL;
> +
>  	INIT_LIST_HEAD(&reboot->head);
>  
>  	for_each_property_of_node(np, prop) {
>  		if (strncmp(prop->name, PREFIX, len))
>  			continue;
>  
> -		info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
> +		info = kzalloc(sizeof(*info), GFP_KERNEL);
>  		if (!info) {
>  			ret = -ENOMEM;
>  			goto error;
>  		}
>  
>  		if (of_property_read_u32(np, prop->name, &info->magic)) {
> -			dev_err(reboot->dev, "reboot mode %s without magic number\n",
> -				info->mode);
> -			devm_kfree(reboot->dev, info);
> +			pr_err("reboot mode %s without magic number\n", info->mode);
> +			kfree(info);
>  			continue;
>  		}
>  
> @@ -102,8 +108,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>  		} else if (info->mode[0] == '\0') {
>  			kfree_const(info->mode);
>  			ret = -EINVAL;
> -			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
> -				prop->name);
> +			pr_err("invalid mode name(%s): too short!\n", prop->name);
>  			goto error;
>  		}
>  
> @@ -130,11 +135,15 @@ EXPORT_SYMBOL_GPL(reboot_mode_register);
>  int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>  {
>  	struct mode_info *info;
> +	struct mode_info *next;
>  
>  	unregister_reboot_notifier(&reboot->reboot_notifier);
>  
> -	list_for_each_entry(info, &reboot->head, list)
> +	list_for_each_entry_safe(info, next, &reboot->head, list) {

This feels liks a missing lock.

>  		kfree_const(info->mode);
> +		list_del(&info->list);

list_del should come before kfree, otherwise it's possible to access
freed memory while traversing the list.

> +		kfree(info);
> +	}
>  
>  	return 0;
>  }
> @@ -162,7 +171,7 @@ int devm_reboot_mode_register(struct device *dev,
>  	if (!dr)
>  		return -ENOMEM;
>  
> -	rc = reboot_mode_register(reboot);
> +	rc = reboot_mode_register(reboot, reboot->dev->of_node);
>  	if (rc) {
>  		devres_free(dr);
>  		return rc;
> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..36f071f4b82e1fc255d8dd679a18e537655c3179 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -9,7 +9,7 @@ struct reboot_mode_driver {
>  	struct notifier_block reboot_notifier;
>  };
>  
> -int reboot_mode_register(struct reboot_mode_driver *reboot);
> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np);
>  int reboot_mode_unregister(struct reboot_mode_driver *reboot);
>  int devm_reboot_mode_register(struct device *dev,
>  			      struct reboot_mode_driver *reboot);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-17 12:46 ` [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
@ 2025-07-18 18:40   ` Dmitry Baryshkov
  2025-07-19 15:13     ` Shivendra Pratap
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2025-07-18 18:40 UTC (permalink / raw)
  To: Shivendra Pratap
  Cc: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla

On Thu, Jul 17, 2025 at 06:16:48PM +0530, Shivendra Pratap wrote:
> Current reboot-mode supports a single 32-bit argument for any
> supported mode. Some reboot-mode based drivers may require
> passing two independent 32-bit arguments during a reboot
> sequence, for uses-cases, where a mode requires an additional
> argument. Such drivers may not be able to use the reboot-mode
> driver. For example, ARM PSCI vendor-specific resets, need two
> arguments for its operation – reset_type and cookie, to complete
> the reset operation. If a driver wants to implement this
> firmware-based reset, it cannot use reboot-mode framework.
> 
> Introduce 64-bit magic values in reboot-mode driver to
> accommodate dual 32-bit arguments when specified via device tree.
> In cases, where no second argument is passed from device tree,
> keep the upper 32-bit of magic un-changed(0) to maintain backward
> compatibility.
> 
> Update the current drivers using reboot-mode for a 64-bit magic
> value.
> 
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/power/reset/nvmem-reboot-mode.c  |  5 ++---
>  drivers/power/reset/qcom-pon.c           |  5 ++---
>  drivers/power/reset/reboot-mode.c        | 27 +++++++++++++++------------
>  drivers/power/reset/syscon-reboot-mode.c |  5 ++---
>  include/linux/reboot-mode.h              |  4 +++-
>  5 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
> index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..e3eed943fdefa271b22e1f1891abef5c9095b9a5 100644
> --- a/drivers/power/reset/nvmem-reboot-mode.c
> +++ b/drivers/power/reset/nvmem-reboot-mode.c
> @@ -16,15 +16,14 @@ struct nvmem_reboot_mode {
>  	struct nvmem_cell *cell;
>  };
>  
> -static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot,
> -				    unsigned int magic)
> +static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>  {
>  	int ret;
>  	struct nvmem_reboot_mode *nvmem_rbm;
>  
>  	nvmem_rbm = container_of(reboot, struct nvmem_reboot_mode, reboot);
>  
> -	ret = nvmem_cell_write(nvmem_rbm->cell, &magic, sizeof(magic));
> +	ret = nvmem_cell_write(nvmem_rbm->cell, (u32 *)&magic, sizeof(u32));

This will work differently on BE and LE systems. Assign a temp var and
use it.

>  	if (ret < 0)
>  		dev_err(reboot->dev, "update reboot mode bits failed\n");
>  
> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582e8243c5c806bd4a793646b87189f..93daf93c097f970afbaf43d36b1e4f725ca7a81f 100644
> --- a/drivers/power/reset/qcom-pon.c
> +++ b/drivers/power/reset/qcom-pon.c
> @@ -27,8 +27,7 @@ struct qcom_pon {
>  	long reason_shift;
>  };
>  
> -static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
> -				    unsigned int magic)
> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>  {
>  	struct qcom_pon *pon = container_of
>  			(reboot, struct qcom_pon, reboot_mode);
> @@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  	ret = regmap_update_bits(pon->regmap,
>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
>  				 GENMASK(7, pon->reason_shift),
> -				 magic << pon->reason_shift);
> +				 ((u32)magic) << pon->reason_shift);
>  	if (ret < 0)
>  		dev_err(pon->dev, "update reboot mode bits failed\n");
>  
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index 0269ec55106472cf2f2b12bd65704dd0114bf4a3..1196627fbf98d87eec57a3d4ee544e403e6eb946 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -19,12 +19,11 @@
>  
>  struct mode_info {
>  	const char *mode;
> -	u32 magic;
> +	u64 magic;
>  	struct list_head list;
>  };
>  
> -static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> -					  const char *cmd)
> +static struct mode_info *get_reboot_mode_info(struct reboot_mode_driver *reboot, const char *cmd)

Why? This doesn't seem relevant to u32 -> u64 conversion.

>  {
>  	const char *normal = "normal";
>  	struct mode_info *info;
> @@ -35,11 +34,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>  
>  	list_for_each_entry(info, &reboot->head, list)
>  		if (!strcmp(info->mode, cmd))
> -			return info->magic;
> +			return info;
>  
>  	/* try to match again, replacing characters impossible in DT */
>  	if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
> -		return 0;
> +		return NULL;
>  
>  	strreplace(cmd_, ' ', '-');
>  	strreplace(cmd_, ',', '-');
> @@ -47,21 +46,21 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>  
>  	list_for_each_entry(info, &reboot->head, list)
>  		if (!strcmp(info->mode, cmd_))
> -			return info->magic;
> +			return info;
>  
> -	return 0;
> +	return NULL;
>  }
>  
>  static int reboot_mode_notify(struct notifier_block *this,
>  			      unsigned long mode, void *cmd)
>  {
>  	struct reboot_mode_driver *reboot;
> -	unsigned int magic;
> +	struct mode_info *info;
>  
>  	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
> -	magic = get_reboot_mode_magic(reboot, cmd);
> -	if (magic)
> -		reboot->write(reboot, magic);
> +	info = get_reboot_mode_info(reboot, cmd);
> +	if (info)
> +		reboot->write(reboot, info->magic);
>  
>  	return NOTIFY_DONE;
>  }
> @@ -78,6 +77,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
>  	struct mode_info *info;
>  	struct property *prop;
>  	size_t len = strlen(PREFIX);
> +	u32 magic_64;
>  	int ret;
>  
>  	if (!np)
> @@ -95,12 +95,15 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
>  			goto error;
>  		}
>  
> -		if (of_property_read_u32(np, prop->name, &info->magic)) {
> +		if (of_property_read_u32(np, prop->name, (u32 *)&info->magic)) {

Again, somebody didn't think about endianness.

>  			pr_err("reboot mode %s without magic number\n", info->mode);
>  			kfree(info);
>  			continue;
>  		}
>  
> +		if (!of_property_read_u32_index(np, prop->name, 1, &magic_64))
> +			info->magic |= (u64)magic_64 << 32;
> +
>  		info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>  		if (!info->mode) {
>  			ret =  -ENOMEM;
> diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
> index e0772c9f70f7a19cd8ec8a0b7fdbbaa7ba44afd0..6783d05e80fbc2c812b45ffe69755478af90d30a 100644
> --- a/drivers/power/reset/syscon-reboot-mode.c
> +++ b/drivers/power/reset/syscon-reboot-mode.c
> @@ -20,8 +20,7 @@ struct syscon_reboot_mode {
>  	u32 mask;
>  };
>  
> -static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
> -				    unsigned int magic)
> +static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>  {
>  	struct syscon_reboot_mode *syscon_rbm;
>  	int ret;
> @@ -29,7 +28,7 @@ static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>  
>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
> -				 syscon_rbm->mask, magic);
> +				 syscon_rbm->mask, (u32)magic);
>  	if (ret < 0)
>  		dev_err(reboot->dev, "update reboot mode bits failed\n");
>  
> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> index 36f071f4b82e1fc255d8dd679a18e537655c3179..d9d9165a8635e5d55d92197a69c7fae179ac2045 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -2,10 +2,12 @@
>  #ifndef __REBOOT_MODE_H__
>  #define __REBOOT_MODE_H__
>  
> +#include <linux/types.h>
> +
>  struct reboot_mode_driver {
>  	struct device *dev;
>  	struct list_head head;
> -	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
> +	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
>  	struct notifier_block reboot_notifier;
>  };
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2
  2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
                   ` (7 preceding siblings ...)
  2025-07-17 12:46 ` [PATCH v11 8/8] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
@ 2025-07-18 21:29 ` Florian Fainelli
  8 siblings, 0 replies; 19+ messages in thread
From: Florian Fainelli @ 2025-07-18 21:29 UTC (permalink / raw)
  To: Shivendra Pratap, Bartosz Golaszewski, Bjorn Andersson,
	Sebastian Reichel, Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon
  Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla, Elliot Berman,
	Konrad Dybcio

On 7/17/25 05:46, Shivendra Pratap wrote:
> The PSCI SYSTEM_RESET2 call allows vendor firmware to define
> additional reset types which could be mapped to the reboot
> argument.
> 
> User-space should be able to reboot a device into different
> operational boot-states supported by underlying bootloader and
> firmware. Generally, some HW registers need to be written, based
> on which the bootloader and firmware decide the next boot state
> of device, after the reset. For example, a requirement on
> Qualcomm platforms may state that reboot with "bootloader"
> command, should reboot the device into bootloader flashing mode
> and reboot with “edl” command, should reboot the device into an
> Emergency flashing mode.  Setting up such reboots on Qualcomm
> devices can be inconsistent across SoC platforms and may require
> setting different HW registers, where some of these registers may
> not be accessible to HLOS. These knobs evolve over product
> generations and require more drivers.  PSCI defines a
> vendor-specific reset in SYSTEM_RESET2 spec, which enables the
> firmware to take care of underlying setting for any such
> supported vendor-specific reboot. Qualcomm firmwares are
> beginning to support and expose PSCI SYSTEM_RESET2
> vendor-specific reset types to simplify driver requirements from
> Linux. With such support added in the firmware, we now need a
> Linux interface which can make use of the firmware calls for PSCI
> vendor-specific resets. This will align such reboot requirement
> across platforms and vendors.
> 
> The current psci driver supports two types of resets –
> SYSTEM_RESET2 Arch warm-reset and SYSTEM_RESET cold-reset. The
> patchset introduces the PSCI SYSTEM_RESET2 vendor-specific reset
> into the reset path of the psci driver and aligns it to work with
> reboot system call - LINUX_REBOOT_CMD_RESTART2, when used along
> with a supported string-based command in “*arg”.
> 
> The patchset uses reboot-mode based commands, to define the
> supported vendor reset-types commands in psci device tree node
> and registers these commands with the reboot-mode framework.
> 
> The PSCI vendor-specific reset takes two arguments, being,
> reset_type and cookie as defined by the spec. To accommodate this
> requirement, enhance the reboot-mode framework to support two
> 32-bit arguments by switching to 64-bit magic values.
> 
> Along this line, the patchset also extends the reboot-mode
> framework to add a non-device-based registration function, which
> will allow drivers to register using device tree node, while
> keeping backward compatibility for existing users of reboot-mode.
> This will enable psci driver to register for reboot-mode and
> implement a write function, which will save the magic and then
> use it in psci reset path to make a vendor-specific reset call
> into the firmware. In addition, the patchset will expose a sysfs
> entry interface within reboot-mode which can be used by userspace
> to view the supported reboot-mode commands.
> 
> The list of vendor-specific reset commands remains open due to
> divergent requirements across vendors, but this can be
> streamlined and standardized through dedicated device tree
> bindings.
> 
> Currently three drivers register with reboot-mode framework -
> syscon-reboot-mode, nvmem-reboot-mode and qcom-pon. Consolidated
> list of commands currently added across various vendor DTs:
>   mode-loader
>   mode-normal
>   mode-bootloader
>   mode-charge
>   mode-fastboot
>   mode-reboot-ab-update
>   mode-recovery
>   mode-rescue
>   mode-shutdown-thermal
>   mode-shutdown-thermal-battery
> 
> Detailed list of commands being used by syscon-reboot-mode:
>      arm64/boot/dts/exynos/exynosautov9.dtsi:
> 	mode-bootloader = <EXYNOSAUTOV9_BOOT_BOOTLOADER>;
> 	mode-fastboot = <EXYNOSAUTOV9_BOOT_FASTBOOT>;
> 	mode-recovery = <EXYNOSAUTOV9_BOOT_RECOVERY>;
> 
>      arm64/boot/dts/exynos/google/gs101.dtsi:
>      	mode-bootloader = <0xfc>;
>      	mode-charge = <0x0a>;
>      	mode-fastboot = <0xfa>;
>      	mode-reboot-ab-update = <0x52>;
>      	mode-recovery = <0xff>;
>      	mode-rescue = <0xf9>;
>      	mode-shutdown-thermal = <0x51>;
>      	mode-shutdown-thermal-battery = <0x51>;
> 
>      arm64/boot/dts/hisilicon/hi3660-hikey960.dts:
>      	mode-normal = <0x77665501>;
>      	mode-bootloader = <0x77665500>;
>      	mode-recovery = <0x77665502>;
> 
>      arm64/boot/dts/hisilicon/hi6220-hikey.dts:
>      	mode-normal = <0x77665501>;
>      	mode-bootloader = <0x77665500>;
>      	mode-recovery = <0x77665502>;
> 
>      arm64/boot/dts/rockchip/px30.dtsi:
>      	mode-bootloader = <BOOT_BL_DOWNLOAD>;
>      	mode-fastboot = <BOOT_FASTBOOT>;
>      	mode-loader = <BOOT_BL_DOWNLOAD>;
>      	mode-normal = <BOOT_NORMAL>;
>      	mode-recovery = <BOOT_RECOVERY>;
> 
>      arm64/boot/dts/rockchip/rk3308.dtsi:
>      	mode-bootloader = <BOOT_BL_DOWNLOAD>;
>      	mode-loader = <BOOT_BL_DOWNLOAD>;
>      	mode-normal = <BOOT_NORMAL>;
>      	mode-recovery = <BOOT_RECOVERY>;
>      	mode-fastboot = <BOOT_FASTBOOT>;
> 
>      arm64/boot/dts/rockchip/rk3566-lckfb-tspi.dts:
>      	mode-normal = <BOOT_NORMAL>;
>      	mode-loader = <BOOT_BL_DOWNLOAD>;
> 			mode-recovery = <BOOT_RECOVERY>;
> 			mode-bootloader = <BOOT_FASTBOOT>;
> 
> Detailed list of commands being used by nvmem-reboot-mode:
>      arm64/boot/dts/qcom/pmXXXX.dtsi:(multiple qcom DTs)
> 			mode-recovery = <0x01>;
> 			mode-bootloader = <0x02>;
> 
> 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>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>

I really wished we had stuck with using "/psci/reset-types" because I 
thought that node name was stable, but regardless, on ARCH_BRCMSTB:

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-18 18:40   ` Dmitry Baryshkov
@ 2025-07-19 15:13     ` Shivendra Pratap
  2025-07-19 16:57       ` Andrew Lunn
  0 siblings, 1 reply; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-19 15:13 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla



On 7/19/2025 12:10 AM, Dmitry Baryshkov wrote:
> On Thu, Jul 17, 2025 at 06:16:48PM +0530, Shivendra Pratap wrote:
>> Current reboot-mode supports a single 32-bit argument for any
>> supported mode. Some reboot-mode based drivers may require
>> passing two independent 32-bit arguments during a reboot
>> sequence, for uses-cases, where a mode requires an additional
>> argument. Such drivers may not be able to use the reboot-mode
>> driver. For example, ARM PSCI vendor-specific resets, need two
>> arguments for its operation – reset_type and cookie, to complete
>> the reset operation. If a driver wants to implement this
>> firmware-based reset, it cannot use reboot-mode framework.
>>
>> Introduce 64-bit magic values in reboot-mode driver to
>> accommodate dual 32-bit arguments when specified via device tree.
>> In cases, where no second argument is passed from device tree,
>> keep the upper 32-bit of magic un-changed(0) to maintain backward
>> compatibility.
>>
>> Update the current drivers using reboot-mode for a 64-bit magic
>> value.
>>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>>  drivers/power/reset/nvmem-reboot-mode.c  |  5 ++---
>>  drivers/power/reset/qcom-pon.c           |  5 ++---
>>  drivers/power/reset/reboot-mode.c        | 27 +++++++++++++++------------
>>  drivers/power/reset/syscon-reboot-mode.c |  5 ++---
>>  include/linux/reboot-mode.h              |  4 +++-
>>  5 files changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
>> index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..e3eed943fdefa271b22e1f1891abef5c9095b9a5 100644
>> --- a/drivers/power/reset/nvmem-reboot-mode.c
>> +++ b/drivers/power/reset/nvmem-reboot-mode.c
>> @@ -16,15 +16,14 @@ struct nvmem_reboot_mode {
>>  	struct nvmem_cell *cell;
>>  };
>>  
>> -static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot,
>> -				    unsigned int magic)
>> +static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>>  {
>>  	int ret;
>>  	struct nvmem_reboot_mode *nvmem_rbm;
>>  
>>  	nvmem_rbm = container_of(reboot, struct nvmem_reboot_mode, reboot);
>>  
>> -	ret = nvmem_cell_write(nvmem_rbm->cell, &magic, sizeof(magic));
>> +	ret = nvmem_cell_write(nvmem_rbm->cell, (u32 *)&magic, sizeof(u32));
> 
> This will work differently on BE and LE systems. Assign a temp var and
> use it.
Ack.
> 
>>  	if (ret < 0)
>>  		dev_err(reboot->dev, "update reboot mode bits failed\n");
>>  
>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
>> index 7e108982a582e8243c5c806bd4a793646b87189f..93daf93c097f970afbaf43d36b1e4f725ca7a81f 100644
>> --- a/drivers/power/reset/qcom-pon.c
>> +++ b/drivers/power/reset/qcom-pon.c
>> @@ -27,8 +27,7 @@ struct qcom_pon {
>>  	long reason_shift;
>>  };
>>  
>> -static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>> -				    unsigned int magic)
>> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>>  {
>>  	struct qcom_pon *pon = container_of
>>  			(reboot, struct qcom_pon, reboot_mode);
>> @@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>>  	ret = regmap_update_bits(pon->regmap,
>>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
>>  				 GENMASK(7, pon->reason_shift),
>> -				 magic << pon->reason_shift);
>> +				 ((u32)magic) << pon->reason_shift);
>>  	if (ret < 0)
>>  		dev_err(pon->dev, "update reboot mode bits failed\n");
>>  
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index 0269ec55106472cf2f2b12bd65704dd0114bf4a3..1196627fbf98d87eec57a3d4ee544e403e6eb946 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -19,12 +19,11 @@
>>  
>>  struct mode_info {
>>  	const char *mode;
>> -	u32 magic;
>> +	u64 magic;
>>  	struct list_head list;
>>  };
>>  
>> -static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> -					  const char *cmd)
>> +static struct mode_info *get_reboot_mode_info(struct reboot_mode_driver *reboot, const char *cmd)
> 
> Why? This doesn't seem relevant to u32 -> u64 conversion.
yes. This function is no more needed. Will update it for u64.
> 
>>  {
>>  	const char *normal = "normal";
>>  	struct mode_info *info;
>> @@ -35,11 +34,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>>  
>>  	list_for_each_entry(info, &reboot->head, list)
>>  		if (!strcmp(info->mode, cmd))
>> -			return info->magic;
>> +			return info;
>>  
>>  	/* try to match again, replacing characters impossible in DT */
>>  	if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
>> -		return 0;
>> +		return NULL;
>>  
>>  	strreplace(cmd_, ' ', '-');
>>  	strreplace(cmd_, ',', '-');
>> @@ -47,21 +46,21 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>>  
>>  	list_for_each_entry(info, &reboot->head, list)
>>  		if (!strcmp(info->mode, cmd_))
>> -			return info->magic;
>> +			return info;
>>  
>> -	return 0;
>> +	return NULL;
>>  }
>>  
>>  static int reboot_mode_notify(struct notifier_block *this,
>>  			      unsigned long mode, void *cmd)
>>  {
>>  	struct reboot_mode_driver *reboot;
>> -	unsigned int magic;
>> +	struct mode_info *info;
>>  
>>  	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
>> -	magic = get_reboot_mode_magic(reboot, cmd);
>> -	if (magic)
>> -		reboot->write(reboot, magic);
>> +	info = get_reboot_mode_info(reboot, cmd);
>> +	if (info)
>> +		reboot->write(reboot, info->magic);
>>  
>>  	return NOTIFY_DONE;
>>  }
>> @@ -78,6 +77,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
>>  	struct mode_info *info;
>>  	struct property *prop;
>>  	size_t len = strlen(PREFIX);
>> +	u32 magic_64;
>>  	int ret;
>>  
>>  	if (!np)
>> @@ -95,12 +95,15 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *
>>  			goto error;
>>  		}
>>  
>> -		if (of_property_read_u32(np, prop->name, &info->magic)) {
>> +		if (of_property_read_u32(np, prop->name, (u32 *)&info->magic)) {
> 
> Again, somebody didn't think about endianness.
Ack. will update this wherever applicable.
> 
>>  			pr_err("reboot mode %s without magic number\n", info->mode);
>>  			kfree(info);
>>  			continue;
>>  		}
>>  
>> +		if (!of_property_read_u32_index(np, prop->name, 1, &magic_64))
>> +			info->magic |= (u64)magic_64 << 32;
>> +
>>  		info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>  		if (!info->mode) {
>>  			ret =  -ENOMEM;
>> diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
>> index e0772c9f70f7a19cd8ec8a0b7fdbbaa7ba44afd0..6783d05e80fbc2c812b45ffe69755478af90d30a 100644
>> --- a/drivers/power/reset/syscon-reboot-mode.c
>> +++ b/drivers/power/reset/syscon-reboot-mode.c
>> @@ -20,8 +20,7 @@ struct syscon_reboot_mode {
>>  	u32 mask;
>>  };
>>  
>> -static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
>> -				    unsigned int magic)
>> +static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>>  {
>>  	struct syscon_reboot_mode *syscon_rbm;
>>  	int ret;
>> @@ -29,7 +28,7 @@ static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
>>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>>  
>>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
>> -				 syscon_rbm->mask, magic);
>> +				 syscon_rbm->mask, (u32)magic);
>>  	if (ret < 0)
>>  		dev_err(reboot->dev, "update reboot mode bits failed\n");
>>  
>> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
>> index 36f071f4b82e1fc255d8dd679a18e537655c3179..d9d9165a8635e5d55d92197a69c7fae179ac2045 100644
>> --- a/include/linux/reboot-mode.h
>> +++ b/include/linux/reboot-mode.h
>> @@ -2,10 +2,12 @@
>>  #ifndef __REBOOT_MODE_H__
>>  #define __REBOOT_MODE_H__
>>  
>> +#include <linux/types.h>
>> +
>>  struct reboot_mode_driver {
>>  	struct device *dev;
>>  	struct list_head head;
>> -	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
>> +	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
>>  	struct notifier_block reboot_notifier;
>>  };
>>  
>>
>> -- 
>> 2.34.1
>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration
  2025-07-18 18:37   ` Dmitry Baryshkov
@ 2025-07-19 15:35     ` Shivendra Pratap
  2025-07-20 18:43       ` Dmitry Baryshkov
  0 siblings, 1 reply; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-19 15:35 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla



On 7/19/2025 12:07 AM, Dmitry Baryshkov wrote:
> On Thu, Jul 17, 2025 at 06:16:47PM +0530, Shivendra Pratap wrote:
>> The reboot-mode driver does not have a strict requirement for
>> device-based registration. It primarily uses the device's of_node
>> to read mode-<cmd> properties and the device pointer for logging.
>>
>> Remove the dependency on struct device and introduce support for
>> Device Tree (DT) node-based registration. This enables drivers
>> that are not associated with a struct device to leverage the
>> reboot-mode framework.
>>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>>  drivers/power/reset/reboot-mode.c | 29 +++++++++++++++++++----------
>>  include/linux/reboot-mode.h       |  2 +-
>>  2 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index fba53f638da04655e756b5f8b7d2d666d1379535..0269ec55106472cf2f2b12bd65704dd0114bf4a3 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -3,13 +3,17 @@
>>   * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
>>   */
>>  
>> +#define pr_fmt(fmt)	"reboot-mode: " fmt
>> +
>>  #include <linux/device.h>
>>  #include <linux/init.h>
>>  #include <linux/kernel.h>
>> +#include <linux/list.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/reboot.h>
>>  #include <linux/reboot-mode.h>
>> +#include <linux/slab.h>
>>  
>>  #define PREFIX "mode-"
>>  
>> @@ -65,33 +69,35 @@ static int reboot_mode_notify(struct notifier_block *this,
>>  /**
>>   * reboot_mode_register - register a reboot mode driver
>>   * @reboot: reboot mode driver
>> + * @np: Pointer to device tree node
>>   *
>>   * Returns: 0 on success or a negative error code on failure.
>>   */
>> -int reboot_mode_register(struct reboot_mode_driver *reboot)
>> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np)
>>  {
>>  	struct mode_info *info;
>>  	struct property *prop;
>> -	struct device_node *np = reboot->dev->of_node;
>>  	size_t len = strlen(PREFIX);
>>  	int ret;
>>  
>> +	if (!np)
>> +		return -EINVAL;
>> +
>>  	INIT_LIST_HEAD(&reboot->head);
>>  
>>  	for_each_property_of_node(np, prop) {
>>  		if (strncmp(prop->name, PREFIX, len))
>>  			continue;
>>  
>> -		info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
>> +		info = kzalloc(sizeof(*info), GFP_KERNEL);
>>  		if (!info) {
>>  			ret = -ENOMEM;
>>  			goto error;
>>  		}
>>  
>>  		if (of_property_read_u32(np, prop->name, &info->magic)) {
>> -			dev_err(reboot->dev, "reboot mode %s without magic number\n",
>> -				info->mode);
>> -			devm_kfree(reboot->dev, info);
>> +			pr_err("reboot mode %s without magic number\n", info->mode);
>> +			kfree(info);
>>  			continue;
>>  		}
>>  
>> @@ -102,8 +108,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>  		} else if (info->mode[0] == '\0') {
>>  			kfree_const(info->mode);
>>  			ret = -EINVAL;
>> -			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
>> -				prop->name);
>> +			pr_err("invalid mode name(%s): too short!\n", prop->name);
>>  			goto error;
>>  		}
>>  
>> @@ -130,11 +135,15 @@ EXPORT_SYMBOL_GPL(reboot_mode_register);
>>  int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>>  {
>>  	struct mode_info *info;
>> +	struct mode_info *next;
>>  
>>  	unregister_reboot_notifier(&reboot->reboot_notifier);
>>  
>> -	list_for_each_entry(info, &reboot->head, list)
>> +	list_for_each_entry_safe(info, next, &reboot->head, list) {
> 
> This feels liks a missing lock.
Should we add a lock here? The list will mostly be fully created only
once at the time of registration.
- thanks.
> 
>>  		kfree_const(info->mode);
>> +		list_del(&info->list);
> 
> list_del should come before kfree, otherwise it's possible to access
> freed memory while traversing the list.
sure. will make it list_del(&info->list) and then kfree_const(info->mode).
- thanks.
> 
>> +		kfree(info);
>> +	}
>>  
>>  	return 0;
>>  }
>> @@ -162,7 +171,7 @@ int devm_reboot_mode_register(struct device *dev,
>>  	if (!dr)
>>  		return -ENOMEM;
>>  
>> -	rc = reboot_mode_register(reboot);
>> +	rc = reboot_mode_register(reboot, reboot->dev->of_node);
>>  	if (rc) {
>>  		devres_free(dr);
>>  		return rc;
>> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
>> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..36f071f4b82e1fc255d8dd679a18e537655c3179 100644
>> --- a/include/linux/reboot-mode.h
>> +++ b/include/linux/reboot-mode.h
>> @@ -9,7 +9,7 @@ struct reboot_mode_driver {
>>  	struct notifier_block reboot_notifier;
>>  };
>>  
>> -int reboot_mode_register(struct reboot_mode_driver *reboot);
>> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np);
>>  int reboot_mode_unregister(struct reboot_mode_driver *reboot);
>>  int devm_reboot_mode_register(struct device *dev,
>>  			      struct reboot_mode_driver *reboot);
>>
>> -- 
>> 2.34.1
>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-19 15:13     ` Shivendra Pratap
@ 2025-07-19 16:57       ` Andrew Lunn
  2025-07-19 17:37         ` Shivendra Pratap
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Lunn @ 2025-07-19 16:57 UTC (permalink / raw)
  To: Shivendra Pratap
  Cc: Dmitry Baryshkov, Bartosz Golaszewski, Bjorn Andersson,
	Sebastian Reichel, Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla

> >> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
> >>  {
> >>  	struct qcom_pon *pon = container_of
> >>  			(reboot, struct qcom_pon, reboot_mode);
> >> @@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
> >>  	ret = regmap_update_bits(pon->regmap,
> >>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
> >>  				 GENMASK(7, pon->reason_shift),
> >> -				 magic << pon->reason_shift);
> >> +				 ((u32)magic) << pon->reason_shift);

As a general rule of thumb, code with casts is poor quality code. Try
to write the code without casts.

Maybe something like

      If (magic > MAX_U32)
      	   return -EINVAL;

      magic_32 = magic;

You might be able to go further, and validate that magic actually fits
into the field when you consider the << pon->reason_shift.

	Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-19 16:57       ` Andrew Lunn
@ 2025-07-19 17:37         ` Shivendra Pratap
  2025-07-20 15:16           ` Andrew Lunn
  0 siblings, 1 reply; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-19 17:37 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Dmitry Baryshkov, Bartosz Golaszewski, Bjorn Andersson,
	Sebastian Reichel, Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla



On 7/19/2025 10:27 PM, Andrew Lunn wrote:
>>>> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>>>>  {
>>>>  	struct qcom_pon *pon = container_of
>>>>  			(reboot, struct qcom_pon, reboot_mode);
>>>> @@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>>>>  	ret = regmap_update_bits(pon->regmap,
>>>>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
>>>>  				 GENMASK(7, pon->reason_shift),
>>>> -				 magic << pon->reason_shift);
>>>> +				 ((u32)magic) << pon->reason_shift);
> 
> As a general rule of thumb, code with casts is poor quality code. Try
> to write the code without casts.
> 
> Maybe something like
> 
>       If (magic > MAX_U32)
>       	   return -EINVAL;
> 
>       magic_32 = magic;
sure will update it. And in above, should it be recommended to add a explicit
cast(for any avoiding any compiler complains)?
 like: magic_32 = (u32)magic;
> 
> You might be able to go further, and validate that magic actually fits
> into the field when you consider the << pon->reason_shift.
will add a check to see make sure the value is in range after "<< pon->reason_shift".

- thanks.
> 
> 	Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-19 17:37         ` Shivendra Pratap
@ 2025-07-20 15:16           ` Andrew Lunn
  2025-07-20 15:24             ` Shivendra Pratap
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Lunn @ 2025-07-20 15:16 UTC (permalink / raw)
  To: Shivendra Pratap
  Cc: Dmitry Baryshkov, Bartosz Golaszewski, Bjorn Andersson,
	Sebastian Reichel, Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla

On Sat, Jul 19, 2025 at 11:07:47PM +0530, Shivendra Pratap wrote:
> 
> 
> On 7/19/2025 10:27 PM, Andrew Lunn wrote:
> >>>> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
> >>>>  {
> >>>>  	struct qcom_pon *pon = container_of
> >>>>  			(reboot, struct qcom_pon, reboot_mode);
> >>>> @@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
> >>>>  	ret = regmap_update_bits(pon->regmap,
> >>>>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
> >>>>  				 GENMASK(7, pon->reason_shift),
> >>>> -				 magic << pon->reason_shift);
> >>>> +				 ((u32)magic) << pon->reason_shift);
> > 
> > As a general rule of thumb, code with casts is poor quality code. Try
> > to write the code without casts.
> > 
> > Maybe something like
> > 
> >       If (magic > MAX_U32)
> >       	   return -EINVAL;
> > 
> >       magic_32 = magic;
> sure will update it. And in above, should it be recommended to add a explicit
> cast(for any avoiding any compiler complains)?
>  like: magic_32 = (u32)magic;

I would hope the compiler/optimiser can figure out from the test the
line before that this is a safe conversion. So i don't think you need
a cast.

	Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic
  2025-07-20 15:16           ` Andrew Lunn
@ 2025-07-20 15:24             ` Shivendra Pratap
  0 siblings, 0 replies; 19+ messages in thread
From: Shivendra Pratap @ 2025-07-20 15:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Dmitry Baryshkov, Bartosz Golaszewski, Bjorn Andersson,
	Sebastian Reichel, Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla



On 7/20/2025 8:46 PM, Andrew Lunn wrote:
> On Sat, Jul 19, 2025 at 11:07:47PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 7/19/2025 10:27 PM, Andrew Lunn wrote:
>>>>>> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>>>>>>  {
>>>>>>  	struct qcom_pon *pon = container_of
>>>>>>  			(reboot, struct qcom_pon, reboot_mode);
>>>>>> @@ -37,7 +36,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>>>>>>  	ret = regmap_update_bits(pon->regmap,
>>>>>>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
>>>>>>  				 GENMASK(7, pon->reason_shift),
>>>>>> -				 magic << pon->reason_shift);
>>>>>> +				 ((u32)magic) << pon->reason_shift);
>>>
>>> As a general rule of thumb, code with casts is poor quality code. Try
>>> to write the code without casts.
>>>
>>> Maybe something like
>>>
>>>       If (magic > MAX_U32)
>>>       	   return -EINVAL;
>>>
>>>       magic_32 = magic;
>> sure will update it. And in above, should it be recommended to add a explicit
>> cast(for any avoiding any compiler complains)?
>>  like: magic_32 = (u32)magic;
> 
> I would hope the compiler/optimiser can figure out from the test the
> line before that this is a safe conversion. So i don't think you need
> a cast.
sure, thanks.
> 
> 	Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration
  2025-07-19 15:35     ` Shivendra Pratap
@ 2025-07-20 18:43       ` Dmitry Baryshkov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2025-07-20 18:43 UTC (permalink / raw)
  To: Shivendra Pratap
  Cc: Bartosz Golaszewski, Bjorn Andersson, Sebastian Reichel,
	Rob Herring, Sudeep Holla, Souvik Chakravarty,
	Krzysztof Kozlowski, Conor Dooley, Andy Yan, Mark Rutland,
	Lorenzo Pieralisi, Arnd Bergmann, Konrad Dybcio,
	cros-qcom-dts-watchers, Vinod Koul, Catalin Marinas, Will Deacon,
	Florian Fainelli, Mukesh Ojha, Stephen Boyd, Andre Draszik,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, Elliot Berman, Srinivas Kandagatla

On Sat, Jul 19, 2025 at 09:05:16PM +0530, Shivendra Pratap wrote:
> 
> 
> On 7/19/2025 12:07 AM, Dmitry Baryshkov wrote:
> > On Thu, Jul 17, 2025 at 06:16:47PM +0530, Shivendra Pratap wrote:
> >> The reboot-mode driver does not have a strict requirement for
> >> device-based registration. It primarily uses the device's of_node
> >> to read mode-<cmd> properties and the device pointer for logging.
> >>
> >> Remove the dependency on struct device and introduce support for
> >> Device Tree (DT) node-based registration. This enables drivers
> >> that are not associated with a struct device to leverage the
> >> reboot-mode framework.
> >>
> >> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> >> ---
> >>  drivers/power/reset/reboot-mode.c | 29 +++++++++++++++++++----------
> >>  include/linux/reboot-mode.h       |  2 +-
> >>  2 files changed, 20 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> >> index fba53f638da04655e756b5f8b7d2d666d1379535..0269ec55106472cf2f2b12bd65704dd0114bf4a3 100644
> >> --- a/drivers/power/reset/reboot-mode.c
> >> +++ b/drivers/power/reset/reboot-mode.c
> >> @@ -3,13 +3,17 @@
> >>   * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
> >>   */
> >>  
> >> +#define pr_fmt(fmt)	"reboot-mode: " fmt
> >> +
> >>  #include <linux/device.h>
> >>  #include <linux/init.h>
> >>  #include <linux/kernel.h>
> >> +#include <linux/list.h>
> >>  #include <linux/module.h>
> >>  #include <linux/of.h>
> >>  #include <linux/reboot.h>
> >>  #include <linux/reboot-mode.h>
> >> +#include <linux/slab.h>
> >>  
> >>  #define PREFIX "mode-"
> >>  
> >> @@ -65,33 +69,35 @@ static int reboot_mode_notify(struct notifier_block *this,
> >>  /**
> >>   * reboot_mode_register - register a reboot mode driver
> >>   * @reboot: reboot mode driver
> >> + * @np: Pointer to device tree node
> >>   *
> >>   * Returns: 0 on success or a negative error code on failure.
> >>   */
> >> -int reboot_mode_register(struct reboot_mode_driver *reboot)
> >> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np)
> >>  {
> >>  	struct mode_info *info;
> >>  	struct property *prop;
> >> -	struct device_node *np = reboot->dev->of_node;
> >>  	size_t len = strlen(PREFIX);
> >>  	int ret;
> >>  
> >> +	if (!np)
> >> +		return -EINVAL;
> >> +
> >>  	INIT_LIST_HEAD(&reboot->head);
> >>  
> >>  	for_each_property_of_node(np, prop) {
> >>  		if (strncmp(prop->name, PREFIX, len))
> >>  			continue;
> >>  
> >> -		info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
> >> +		info = kzalloc(sizeof(*info), GFP_KERNEL);
> >>  		if (!info) {
> >>  			ret = -ENOMEM;
> >>  			goto error;
> >>  		}
> >>  
> >>  		if (of_property_read_u32(np, prop->name, &info->magic)) {
> >> -			dev_err(reboot->dev, "reboot mode %s without magic number\n",
> >> -				info->mode);
> >> -			devm_kfree(reboot->dev, info);
> >> +			pr_err("reboot mode %s without magic number\n", info->mode);
> >> +			kfree(info);
> >>  			continue;
> >>  		}
> >>  
> >> @@ -102,8 +108,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
> >>  		} else if (info->mode[0] == '\0') {
> >>  			kfree_const(info->mode);
> >>  			ret = -EINVAL;
> >> -			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
> >> -				prop->name);
> >> +			pr_err("invalid mode name(%s): too short!\n", prop->name);
> >>  			goto error;
> >>  		}
> >>  
> >> @@ -130,11 +135,15 @@ EXPORT_SYMBOL_GPL(reboot_mode_register);
> >>  int reboot_mode_unregister(struct reboot_mode_driver *reboot)
> >>  {
> >>  	struct mode_info *info;
> >> +	struct mode_info *next;
> >>  
> >>  	unregister_reboot_notifier(&reboot->reboot_notifier);
> >>  
> >> -	list_for_each_entry(info, &reboot->head, list)
> >> +	list_for_each_entry_safe(info, next, &reboot->head, list) {
> > 
> > This feels liks a missing lock.
> Should we add a lock here? The list will mostly be fully created only
> once at the time of registration.

Does it matter? The code should be correct. List traversal should be
protected by the lock in all the cases.

> - thanks.
> > 
> >>  		kfree_const(info->mode);
> >> +		list_del(&info->list);
> > 
> > list_del should come before kfree, otherwise it's possible to access
> > freed memory while traversing the list.
> sure. will make it list_del(&info->list) and then kfree_const(info->mode).
> - thanks.
> > 
> >> +		kfree(info);
> >> +	}
> >>  
> >>  	return 0;
> >>  }
> >> @@ -162,7 +171,7 @@ int devm_reboot_mode_register(struct device *dev,
> >>  	if (!dr)
> >>  		return -ENOMEM;
> >>  
> >> -	rc = reboot_mode_register(reboot);
> >> +	rc = reboot_mode_register(reboot, reboot->dev->of_node);
> >>  	if (rc) {
> >>  		devres_free(dr);
> >>  		return rc;
> >> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> >> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..36f071f4b82e1fc255d8dd679a18e537655c3179 100644
> >> --- a/include/linux/reboot-mode.h
> >> +++ b/include/linux/reboot-mode.h
> >> @@ -9,7 +9,7 @@ struct reboot_mode_driver {
> >>  	struct notifier_block reboot_notifier;
> >>  };
> >>  
> >> -int reboot_mode_register(struct reboot_mode_driver *reboot);
> >> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np);
> >>  int reboot_mode_unregister(struct reboot_mode_driver *reboot);
> >>  int devm_reboot_mode_register(struct device *dev,
> >>  			      struct reboot_mode_driver *reboot);
> >>
> >> -- 
> >> 2.34.1
> >>
> > 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-07-20 18:43 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 12:46 [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 1/8] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
2025-07-18 18:37   ` Dmitry Baryshkov
2025-07-19 15:35     ` Shivendra Pratap
2025-07-20 18:43       ` Dmitry Baryshkov
2025-07-17 12:46 ` [PATCH v11 2/8] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
2025-07-18 18:40   ` Dmitry Baryshkov
2025-07-19 15:13     ` Shivendra Pratap
2025-07-19 16:57       ` Andrew Lunn
2025-07-19 17:37         ` Shivendra Pratap
2025-07-20 15:16           ` Andrew Lunn
2025-07-20 15:24             ` Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 3/8] dt-bindings: arm: Document reboot mode magic Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 4/8] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 5/8] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 6/8] arm64: dts: qcom: qcs6490-rb3gen2: " Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 7/8] arm64: dts: qcom: sa8775p-ride: " Shivendra Pratap
2025-07-17 12:46 ` [PATCH v11 8/8] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
2025-07-18 21:29 ` [PATCH v11 0/8] Implement vendor resets for PSCI SYSTEM_RESET2 Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).