* [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2
@ 2025-10-15 4:38 Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal Shivendra Pratap
` (13 more replies)
0 siblings, 14 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla, Umang Chheda, Sebastian Reichel,
Elliot Berman, Konrad Dybcio, Song Xue, 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
On gs101 we also pass kernel-generated modes from kernel_restart()
or panic(), specifically DM verity's 'dm-verity device corrupted':
mode-dm-verity-device-corrupted = <0x50>;
- thanks Andre' for providing this.
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>;
The patch is tested on rb3Gen2, lemans-ride, lemans-evk, monaco-ride.
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>
Tested-by: Florian Fainelli <florian.fainelli@broadcom.com> # On ARCH_BRCMSTB
Tested-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com> # IPQ5424-RDP466
Changes in v16:
firmware: psci: Implement vendor-specific resets as reboot-mode
- Use GENMASK(31, 0) instead of 0xffffffff - by Kathiravan
- Link to v15: https://lore.kernel.org/r/20250922-arm-psci-system_reset2-vendor-reboots-v15-0-7ce3a08878f1@oss.qualcomm.com
Changes in v15:
By Sebastian:
power: reset: reboot-mode: Synchronize list traversal
- Change mutex locking to scoped_guard() and a Fixes: tag
power: reset: reboot-mode: Add device tree node-based registration
- Change reboot_mode_register external call to use fwnode
power: reset: reboot-mode: Expose sysfs for registered reboot_modes
- Use sysfs_emit_at for printing sysfs entries
- Add driver_name to struct reboot_mode_driver instead of passing
as argument
- Update reboot_mode_register, devm_reboot_mode_register and
create_reboot_mode_device for same.
firmware: psci: Implement vendor-specific resets as reboot-mode
- Update psci to use updated reboot_mode_register and store driver_name
to struct reboot_mode_driver
- Add DT nodes for PSCI SYSTEM_RESET2 types for lemans-evk, qcs8300-ride,
monaco-evk and qcs615-ride boards.
- Link to v14: https://lore.kernel.org/r/20250815-arm-psci-system_reset2-vendor-reboots-v14-0-37d29f59ac9a@oss.qualcomm.com
Changes in v14:
- mode-dm-verity-device-corrupted documented in cover letter -by André
ABI Documentation:
- Updated KernelVersion in ABI documentation to reflect base commit
version. – by André
- Revised ABI documentation to clarify space-separated format for
supported reboot-mode commands. – by André
power: reset: reboot-mode: Expose sysfs patch
- Modified `show_modes` to output a space-separated list of supported
reboot modes – by André
- Added error handling in `create_reboot_mode_device()` to ensure
proper cleanup on failure.
firmware: psci:
- Locate psci/reboot-mode node using psci compatible. - by Krzysztof,
Dmitry, Sudeep.
- Added error handling for additional code for compatible.
- Converted hex values to lowercase for consistency. – by André
- Introduced panic notifier to disable valid vendor-reset flag in
panic path. – by André
- Added check for `psci_system_reset2` before registering vendor reset
commands.
- Updated Commit text.
dts: sa8775p:
- DT file name changed from sa8775p to lemans and commit text updated
accordingly. – for dt renaming in base commit (sa8775p to lemans).
- Link to v13: https://lore.kernel.org/r/20250727-arm-psci-system_reset2-vendor-reboots-v13-0-6b8d23315898@oss.qualcomm.com
Changes in v13:
- Split patch1 into two (Synchronize list traversal and DT node-based
registration) - by Dmitry.
- Move mutex lock inside get_reboot_mode_magic - by Dmitry.
- Reorder the patches – pull patch8 for exposing reboot-mode sysfs before
psci patch - to align the change in reboot-mode sysfs patch.
- Update patch- reboot-mode: Expose sysfs for registered reboot_modes
- Introduce a driver_name in reboot_mode_register. This will be used
in sysfs creation - by Arnd.
- Update documentation and commit text for above.
- Fix release function to properly call delete attr file.
- Fix sparse warning for devres_find.
- Add error handling for devres_find.
- Split ABI documentation as a separate patch and update ABI documentation
for usage of driver-name in sysfs - by Arnd
- Update patch - psci: Implement vendor-specific resets as reboot-mode
- Fix Kconfig for CONFIG related warning.
- Add driver_name as "psci" in register call to reboot-mode - by Arnd
- Link to v12: https://lore.kernel.org/r/20250721-arm-psci-system_reset2-vendor-reboots-v12-0-87bac3ec422e@oss.qualcomm.com
Changes in v12:
- Added lock for list traversals in reboot-mode - by Dmitry.
- Added proper handling for BE and LE cases in reboot-mode - by Dmitry.
- Removed type casting for u64 to u32 conversions. Added limit checks
and used bitwise operations for same - by Andrew.
- Link to v11: https://lore.kernel.org/r/20250717-arm-psci-system_reset2-vendor-reboots-v11-0-df3e2b2183c3@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: lemans-ride: Add PSCI SYSTEM_RESET2 types
Shivendra Pratap (9):
power: reset: reboot-mode: Synchronize list traversal
power: reset: reboot-mode: Add device tree node-based registration
power: reset: reboot-mode: Add support for 64 bit magic
Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes
power: reset: reboot-mode: Expose sysfs for registered reboot_modes
firmware: psci: Implement vendor-specific resets as reboot-mode
arm64: dts: qcom: lemans-evk: Add PSCI SYSTEM_RESET2 types
arm64: dts: qcom: qcs8300-ride: Add PSCI SYSTEM_RESET2 types
arm64: dts: qcom: monaco-evk: Add PSCI SYSTEM_RESET2 types
Song Xue (1):
arm64: dts: qcom: qcs615-ride: Add PSCI SYSTEM_RESET2 types
.../testing/sysfs-class-reboot-mode-reboot_modes | 39 ++++
Documentation/devicetree/bindings/arm/psci.yaml | 43 ++++
arch/arm64/boot/dts/qcom/lemans-evk.dts | 7 +
arch/arm64/boot/dts/qcom/lemans-ride-common.dtsi | 7 +
arch/arm64/boot/dts/qcom/lemans.dtsi | 2 +-
arch/arm64/boot/dts/qcom/monaco-evk.dts | 7 +
arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 7 +
arch/arm64/boot/dts/qcom/qcs615-ride.dts | 7 +
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 7 +
arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 7 +
arch/arm64/boot/dts/qcom/qcs8300.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sm6150.dtsi | 2 +-
drivers/firmware/psci/Kconfig | 2 +
drivers/firmware/psci/psci.c | 90 ++++++++-
drivers/power/reset/nvmem-reboot-mode.c | 13 +-
drivers/power/reset/qcom-pon.c | 11 +-
drivers/power/reset/reboot-mode.c | 217 ++++++++++++++++-----
drivers/power/reset/syscon-reboot-mode.c | 11 +-
include/linux/reboot-mode.h | 12 +-
20 files changed, 427 insertions(+), 68 deletions(-)
---
base-commit: 13863a59e410cab46d26751941980dc8f088b9b3
change-id: 20250709-arm-psci-system_reset2-vendor-reboots-46c80044afcf
Best regards,
--
Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 14:32 ` Bartosz Golaszewski
2025-10-28 3:34 ` Bjorn Andersson
2025-10-15 4:38 ` [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
` (12 subsequent siblings)
13 siblings, 2 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla
List traversals must be synchronized to prevent race conditions
and data corruption. The reboot-mode list is not protected by a
lock currently, which can lead to concurrent access and race.
Introduce a mutex lock to guard all operations on the reboot-mode
list and ensure thread-safe access. The change prevents unsafe
concurrent access on reboot-mode list.
Fixes: 4fcd504edbf7 ("power: reset: add reboot mode driver")
Fixes: ca3d2ea52314 ("power: reset: reboot-mode: better compatibility with DT (replace ' ,/')")
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
drivers/power/reset/reboot-mode.c | 96 +++++++++++++++++++++------------------
include/linux/reboot-mode.h | 4 ++
2 files changed, 57 insertions(+), 43 deletions(-)
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index fba53f638da04655e756b5f8b7d2d666d1379535..8fc3e14638ea757c8dc3808c240ff569cbd74786 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -29,9 +29,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
if (!cmd)
cmd = normal;
- list_for_each_entry(info, &reboot->head, list)
- if (!strcmp(info->mode, cmd))
- return info->magic;
+ scoped_guard(mutex, &reboot->rb_lock) {
+ list_for_each_entry(info, &reboot->head, list)
+ if (!strcmp(info->mode, cmd))
+ return info->magic;
+ }
/* try to match again, replacing characters impossible in DT */
if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
@@ -41,9 +43,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
strreplace(cmd_, ',', '-');
strreplace(cmd_, '/', '-');
- list_for_each_entry(info, &reboot->head, list)
- if (!strcmp(info->mode, cmd_))
- return info->magic;
+ scoped_guard(mutex, &reboot->rb_lock) {
+ list_for_each_entry(info, &reboot->head, list)
+ if (!strcmp(info->mode, cmd_))
+ return info->magic;
+ }
return 0;
}
@@ -78,46 +82,50 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
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);
- 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);
- continue;
- }
-
- info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
- if (!info->mode) {
- ret = -ENOMEM;
- goto error;
- } 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);
- goto error;
+ mutex_init(&reboot->rb_lock);
+
+ scoped_guard(mutex, &reboot->rb_lock) {
+ for_each_property_of_node(np, prop) {
+ if (strncmp(prop->name, PREFIX, len))
+ continue;
+
+ info = devm_kzalloc(reboot->dev, 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);
+ continue;
+ }
+
+ info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
+ if (!info->mode) {
+ ret = -ENOMEM;
+ goto error;
+ } 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);
+ goto error;
+ }
+
+ list_add_tail(&info->list, &reboot->head);
}
- list_add_tail(&info->list, &reboot->head);
- }
-
- reboot->reboot_notifier.notifier_call = reboot_mode_notify;
- register_reboot_notifier(&reboot->reboot_notifier);
+ reboot->reboot_notifier.notifier_call = reboot_mode_notify;
+ register_reboot_notifier(&reboot->reboot_notifier);
- return 0;
+ return 0;
error:
- list_for_each_entry(info, &reboot->head, list)
- kfree_const(info->mode);
+ list_for_each_entry(info, &reboot->head, list)
+ kfree_const(info->mode);
+ }
return ret;
}
@@ -133,8 +141,10 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
unregister_reboot_notifier(&reboot->reboot_notifier);
- list_for_each_entry(info, &reboot->head, list)
- kfree_const(info->mode);
+ scoped_guard(mutex, &reboot->rb_lock) {
+ list_for_each_entry(info, &reboot->head, list)
+ kfree_const(info->mode);
+ }
return 0;
}
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b73f80708197677db8dc2e43affc519782b7146e 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -2,11 +2,15 @@
#ifndef __REBOOT_MODE_H__
#define __REBOOT_MODE_H__
+#include <linux/mutex.h>
+
struct reboot_mode_driver {
struct device *dev;
struct list_head head;
int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
struct notifier_block reboot_notifier;
+ /*Protects access to reboot mode list*/
+ struct mutex rb_lock;
};
int reboot_mode_register(struct reboot_mode_driver *reboot);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 14:40 ` Bartosz Golaszewski
2025-10-15 4:38 ` [PATCH v16 03/14] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
` (11 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, 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
firmware node (fwnode) 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 | 45 +++++++++++++++++++++++++++++----------
include/linux/reboot-mode.h | 3 ++-
2 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index 8fc3e14638ea757c8dc3808c240ff569cbd74786..c8f71e6f661ae14eb72bdcb1f412cd05faee3dd9 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-"
@@ -69,17 +73,26 @@ static int reboot_mode_notify(struct notifier_block *this,
/**
* reboot_mode_register - register a reboot mode driver
* @reboot: reboot mode driver
+ * @fwnode: Firmware node with reboot-mode configuration
*
* 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 fwnode_handle *fwnode)
{
struct mode_info *info;
+ struct mode_info *next;
+ struct device_node *np;
struct property *prop;
- struct device_node *np = reboot->dev->of_node;
size_t len = strlen(PREFIX);
int ret;
+ if (!fwnode)
+ return -EINVAL;
+
+ np = to_of_node(fwnode);
+ if (!np)
+ return -EINVAL;
+
INIT_LIST_HEAD(&reboot->head);
mutex_init(&reboot->rb_lock);
@@ -89,28 +102,28 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
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;
}
info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
if (!info->mode) {
ret = -ENOMEM;
+ kfree(info);
goto error;
} else if (info->mode[0] == '\0') {
kfree_const(info->mode);
+ kfree(info);
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;
}
@@ -123,8 +136,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
return 0;
error:
- list_for_each_entry(info, &reboot->head, list)
+ list_for_each_entry_safe(info, next, &reboot->head, list) {
+ list_del(&info->list);
kfree_const(info->mode);
+ kfree(info);
+ }
}
return ret;
@@ -138,12 +154,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);
scoped_guard(mutex, &reboot->rb_lock) {
- list_for_each_entry(info, &reboot->head, list)
+ list_for_each_entry_safe(info, next, &reboot->head, list) {
+ list_del(&info->list);
kfree_const(info->mode);
+ kfree(info);
+ }
}
return 0;
@@ -168,11 +188,14 @@ int devm_reboot_mode_register(struct device *dev,
struct reboot_mode_driver **dr;
int rc;
+ if (!reboot->dev || !reboot->dev->of_node)
+ return -EINVAL;
+
dr = devres_alloc(devm_reboot_mode_release, sizeof(*dr), GFP_KERNEL);
if (!dr)
return -ENOMEM;
- rc = reboot_mode_register(reboot);
+ rc = reboot_mode_register(reboot, of_fwnode_handle(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 b73f80708197677db8dc2e43affc519782b7146e..7f05fd873e95ca8249bc167c21aa6b76faba7849 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -2,6 +2,7 @@
#ifndef __REBOOT_MODE_H__
#define __REBOOT_MODE_H__
+#include <linux/fwnode.h>
#include <linux/mutex.h>
struct reboot_mode_driver {
@@ -13,7 +14,7 @@ struct reboot_mode_driver {
struct mutex rb_lock;
};
-int reboot_mode_register(struct reboot_mode_driver *reboot);
+int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle *fwnode);
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] 33+ messages in thread
* [PATCH v16 03/14] power: reset: reboot-mode: Add support for 64 bit magic
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 8:40 ` Nirmesh Kumar Singh
2025-10-15 4:38 ` [PATCH v16 04/14] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes Shivendra Pratap
` (10 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla, Umang Chheda
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.
Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
drivers/power/reset/nvmem-reboot-mode.c | 13 +++++++++----
drivers/power/reset/qcom-pon.c | 11 ++++++++---
drivers/power/reset/reboot-mode.c | 19 +++++++++++++------
drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++---
include/linux/reboot-mode.h | 3 ++-
5 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..5d73dde585b1fd438b1847f884feb37cd9e4dd5c 100644
--- a/drivers/power/reset/nvmem-reboot-mode.c
+++ b/drivers/power/reset/nvmem-reboot-mode.c
@@ -16,15 +16,20 @@ 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;
+ u32 magic_32;
+ int ret;
+
+ if (magic > U32_MAX)
+ return -EINVAL;
+
+ magic_32 = magic;
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, &magic_32, sizeof(magic_32));
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..d0ed9431a02313a7bbaa93743c16fa1ae713ddfe 100644
--- a/drivers/power/reset/qcom-pon.c
+++ b/drivers/power/reset/qcom-pon.c
@@ -27,17 +27,22 @@ 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);
+ u32 magic_32;
int ret;
+ if (magic > U32_MAX || (magic << pon->reason_shift) > U32_MAX)
+ return -EINVAL;
+
+ magic_32 = magic << pon->reason_shift;
+
ret = regmap_update_bits(pon->regmap,
pon->baseaddr + PON_SOFT_RB_SPARE,
GENMASK(7, pon->reason_shift),
- magic << pon->reason_shift);
+ magic_32);
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 c8f71e6f661ae14eb72bdcb1f412cd05faee3dd9..79763a839c9b0161b4acb6afb625f50a880971cc 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 u64 get_reboot_mode_magic(struct reboot_mode_driver *reboot, const char *cmd)
{
const char *normal = "normal";
struct mode_info *info;
@@ -60,7 +59,7 @@ static int reboot_mode_notify(struct notifier_block *this,
unsigned long mode, void *cmd)
{
struct reboot_mode_driver *reboot;
- unsigned int magic;
+ u64 magic;
reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
magic = get_reboot_mode_magic(reboot, cmd);
@@ -84,6 +83,8 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
struct device_node *np;
struct property *prop;
size_t len = strlen(PREFIX);
+ u32 magic_arg1;
+ u32 magic_arg2;
int ret;
if (!fwnode)
@@ -108,12 +109,18 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
goto error;
}
- if (of_property_read_u32(np, prop->name, &info->magic)) {
- pr_err("reboot mode %s without magic number\n", info->mode);
+ if (of_property_read_u32(np, prop->name, &magic_arg1)) {
+ pr_err("reboot mode without magic number\n");
kfree(info);
continue;
}
+ if (of_property_read_u32_index(np, prop->name, 1, &magic_arg2))
+ magic_arg2 = 0;
+
+ info->magic = magic_arg2;
+ info->magic = (info->magic << 32) | magic_arg1;
+
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..3cbd000c512239b12ec51987e900d260540a9dea 100644
--- a/drivers/power/reset/syscon-reboot-mode.c
+++ b/drivers/power/reset/syscon-reboot-mode.c
@@ -20,16 +20,21 @@ 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;
+ u32 magic_32;
int ret;
+ if (magic > U32_MAX)
+ return -EINVAL;
+
+ magic_32 = magic;
+
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, magic_32);
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 7f05fd873e95ca8249bc167c21aa6b76faba7849..3a14df2ddd1db4181ea76f99ef447ed8368a3594 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -4,11 +4,12 @@
#include <linux/fwnode.h>
#include <linux/mutex.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;
/*Protects access to reboot mode list*/
struct mutex rb_lock;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v16 04/14] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (2 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 03/14] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
` (9 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla, Sebastian Reichel
Add ABI documentation for /sys/class/reboot-mode/*/reboot_modes,
a read-only sysfs attribute exposing the list of supported
reboot-mode arguments. This file is created by reboot-mode
framework and provides a user-readable interface to query
available reboot-mode arguments.
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
.../testing/sysfs-class-reboot-mode-reboot_modes | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
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..6a3fc379afae3a6caf56ad0b73b1c06c43a9fee7
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-reboot-mode-reboot_modes
@@ -0,0 +1,39 @@
+What: /sys/class/reboot-mode/<driver>/reboot_modes
+Date: August 2025
+KernelVersion: 6.17.0-rc1
+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 space
+ separated list of reboot-mode arguments supported
+ on the current platform.
+ Example:
+ recovery fastboot bootloader
+
+ The exact sysfs path may vary depending on the
+ name of the driver that registers the arguments.
+ Example:
+ /sys/class/reboot-mode/nvmem-reboot-mode/reboot_modes
+ /sys/class/reboot-mode/syscon-reboot-mode/reboot_modes
+ /sys/class/reboot-mode/qcom-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.
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (3 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 04/14] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 8:45 ` Nirmesh Kumar Singh
2025-10-15 14:47 ` Bartosz Golaszewski
2025-10-15 4:38 ` [PATCH v16 06/14] dt-bindings: arm: Document reboot mode magic Shivendra Pratap
` (8 subsequent siblings)
13 siblings, 2 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, 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. Use the driver_name field of the struct
reboot_mode_driver to create the device. For device-based
drivers, configure the device driver name as driver_name.
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.
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
drivers/power/reset/reboot-mode.c | 127 ++++++++++++++++++++++++++++++--------
include/linux/reboot-mode.h | 2 +
2 files changed, 103 insertions(+), 26 deletions(-)
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index 79763a839c9b0161b4acb6afb625f50a880971cc..1e78eb3d0fe513c934b37bf7f0829e1f9f4634f0 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 u64 get_reboot_mode_magic(struct reboot_mode_driver *reboot, const char *cmd)
{
const char *normal = "normal";
@@ -69,6 +72,89 @@ 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, NULL);
+ if (!devres_reboot || !(*devres_reboot))
+ return -ENODATA;
+
+ reboot = *devres_reboot;
+ scoped_guard(mutex, &reboot->rb_lock) {
+ list_for_each_entry(info, &reboot->head, list)
+ size += sysfs_emit_at(buf, size, "%s ", info->mode);
+ }
+
+ if (size) {
+ size += sysfs_emit_at(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);
+
+ scoped_guard(mutex, &reboot->rb_lock) {
+ list_for_each_entry_safe(info, next, &reboot->head, list) {
+ list_del(&info->list);
+ kfree_const(info->mode);
+ kfree(info);
+ }
+ }
+
+ device_remove_file(reboot->reboot_dev, &dev_attr_reboot_modes);
+}
+
+static int create_reboot_mode_device(struct reboot_mode_driver *reboot)
+{
+ 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, reboot->driver_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)
+ goto create_file_err;
+
+ dr = devres_alloc(release_reboot_mode_device, sizeof(*dr), GFP_KERNEL);
+ if (!dr) {
+ ret = -ENOMEM;
+ goto devres_alloc_error;
+ }
+
+ *dr = reboot;
+ devres_add(reboot->reboot_dev, dr);
+ return ret;
+
+devres_alloc_error:
+ device_remove_file(reboot->reboot_dev, &dev_attr_reboot_modes);
+create_file_err:
+ device_unregister(reboot->reboot_dev);
+ return ret;
+}
+
/**
* reboot_mode_register - register a reboot mode driver
* @reboot: reboot mode driver
@@ -79,7 +165,6 @@ static int reboot_mode_notify(struct notifier_block *this,
int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle *fwnode)
{
struct mode_info *info;
- struct mode_info *next;
struct device_node *np;
struct property *prop;
size_t len = strlen(PREFIX);
@@ -87,13 +172,17 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
u32 magic_arg2;
int ret;
- if (!fwnode)
+ if (!fwnode || !reboot->driver_name)
return -EINVAL;
np = to_of_node(fwnode);
if (!np)
return -EINVAL;
+ ret = create_reboot_mode_device(reboot);
+ if (ret)
+ return ret;
+
INIT_LIST_HEAD(&reboot->head);
mutex_init(&reboot->rb_lock);
@@ -136,20 +225,15 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
list_add_tail(&info->list, &reboot->head);
}
+ }
- reboot->reboot_notifier.notifier_call = reboot_mode_notify;
- register_reboot_notifier(&reboot->reboot_notifier);
+ reboot->reboot_notifier.notifier_call = reboot_mode_notify;
+ register_reboot_notifier(&reboot->reboot_notifier);
- return 0;
+ return 0;
error:
- list_for_each_entry_safe(info, next, &reboot->head, list) {
- list_del(&info->list);
- kfree_const(info->mode);
- kfree(info);
- }
- }
-
+ device_unregister(reboot->reboot_dev);
return ret;
}
EXPORT_SYMBOL_GPL(reboot_mode_register);
@@ -160,26 +244,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);
-
- scoped_guard(mutex, &reboot->rb_lock) {
- list_for_each_entry_safe(info, next, &reboot->head, list) {
- list_del(&info->list);
- kfree_const(info->mode);
- 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);
}
/**
@@ -202,6 +276,7 @@ int devm_reboot_mode_register(struct device *dev,
if (!dr)
return -ENOMEM;
+ reboot->driver_name = reboot->dev->driver->name;
rc = reboot_mode_register(reboot, of_fwnode_handle(reboot->dev->of_node));
if (rc) {
devres_free(dr);
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index 3a14df2ddd1db4181ea76f99ef447ed8368a3594..c68a671f6947f2346e1e6a0ce3c6ebc18722b98e 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -8,6 +8,8 @@
struct reboot_mode_driver {
struct device *dev;
+ struct device *reboot_dev;
+ const char *driver_name;
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] 33+ messages in thread
* [PATCH v16 06/14] dt-bindings: arm: Document reboot mode magic
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (4 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
` (7 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, 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..eca38f8747d320e8371c1dc37cee2287d71821c4 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:
+ reboot-mode: 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] 33+ messages in thread
* [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (5 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 06/14] dt-bindings: arm: Document reboot mode magic Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 6:55 ` Pavan Kondeti
2025-10-15 4:38 ` [PATCH v16 08/14] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types Shivendra Pratap
` (6 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla, Umang Chheda
SoC vendors have different types of resets which are controlled
through various hardware registers. For instance, Qualcomm SoC
may have a requirement that reboot with “bootloader” command
should reboot the device to bootloader flashing mode and reboot
with “edl” should reboot the device into Emergency flashing mode.
Setting up such reboots on Qualcomm devices can be inconsistent
across SoC platforms and may require setting different HW
registers, where some of these registers may not be accessible to
HLOS. These knobs evolve over product generations and require
more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
reset which can help align this requirement. Add support for PSCI
SYSTEM_RESET2, vendor-specific resets and align the implementation
to allow user-space initiated reboots to trigger these resets.
Implement the PSCI vendor-specific resets by registering to the
reboot-mode framework. As psci init is done at early kernel init,
reboot-mode registration cannot be done at the time of psci init.
This is because reboot-mode creates a “reboot-mode” class for
exposing sysfs, which can fail at early kernel init. To overcome
this, introduce a late_initcall to register PSCI vendor-specific
resets as reboot modes. Implement a reboot-mode write function
that sets reset_type and cookie values during the reboot notifier
callback. Introduce a firmware-based call for SYSTEM_RESET2
vendor-specific reset in the psci_sys_reset path, using
reset_type and cookie if supported by secure firmware. Register a
panic notifier and clear vendor_reset valid status during panic.
This is needed for any kernel panic that occurs post
reboot_notifiers.
By using the above implementation, userspace will be able to issue
such resets using the reboot() system call with the "*arg"
parameter as a string based command. The commands can be defined
in PSCI device tree node under “reboot-mode” and are based on the
reboot-mode based commands.
Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
Reviewed-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
drivers/firmware/psci/Kconfig | 2 +
drivers/firmware/psci/psci.c | 90 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci/Kconfig b/drivers/firmware/psci/Kconfig
index 97944168b5e66aea1e38a7eb2d4ced8348fce64b..93ff7b071a0c364a376699733e6bc5654d56a17f 100644
--- a/drivers/firmware/psci/Kconfig
+++ b/drivers/firmware/psci/Kconfig
@@ -1,6 +1,8 @@
# SPDX-License-Identifier: GPL-2.0-only
config ARM_PSCI_FW
bool
+ select POWER_RESET
+ select REBOOT_MODE
config ARM_PSCI_CHECKER
bool "ARM PSCI checker"
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..771e164d6649f15a1f8f5ba999a24f5c04110932 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -8,15 +8,18 @@
#include <linux/acpi.h>
#include <linux/arm-smccc.h>
+#include <linux/bitops.h>
#include <linux/cpuidle.h>
#include <linux/debugfs.h>
#include <linux/errno.h>
#include <linux/linkage.h>
#include <linux/of.h>
+#include <linux/panic_notifier.h>
#include <linux/pm.h>
#include <linux/printk.h>
#include <linux/psci.h>
#include <linux/reboot.h>
+#include <linux/reboot-mode.h>
#include <linux/slab.h>
#include <linux/suspend.h>
@@ -51,6 +54,24 @@ static int resident_cpu = -1;
struct psci_operations psci_ops;
static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
+struct psci_vendor_sysreset2 {
+ u32 reset_type;
+ u32 cookie;
+ bool valid;
+};
+
+static struct psci_vendor_sysreset2 vendor_reset;
+
+static int psci_panic_event(struct notifier_block *nb, unsigned long v, void *p)
+{
+ vendor_reset.valid = false;
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block psci_panic_block = {
+ .notifier_call = psci_panic_event
+};
+
bool psci_tos_resident_on(int cpu)
{
return cpu == resident_cpu;
@@ -309,7 +330,10 @@ static int get_set_conduit_method(const struct device_node *np)
static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
void *data)
{
- if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
+ if (vendor_reset.valid && psci_system_reset2_supported) {
+ invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
+ vendor_reset.cookie, 0);
+ } else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
psci_system_reset2_supported) {
/*
* reset_type[31] = 0 (architectural)
@@ -547,6 +571,70 @@ static const struct platform_suspend_ops psci_suspend_ops = {
.enter = psci_system_suspend_enter,
};
+static int psci_set_vendor_sys_reset2(struct reboot_mode_driver *reboot, u64 magic)
+{
+ u32 magic_32;
+
+ if (psci_system_reset2_supported) {
+ magic_32 = magic & GENMASK(31, 0);
+ vendor_reset.reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic_32;
+ vendor_reset.cookie = (magic >> 32) & GENMASK(31, 0);
+ vendor_reset.valid = true;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static int __init psci_init_vendor_reset(void)
+{
+ struct reboot_mode_driver *reboot;
+ struct device_node *psci_np;
+ struct device_node *np;
+ int ret;
+
+ if (!psci_system_reset2_supported)
+ return -EINVAL;
+
+ psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
+ if (!psci_np)
+ return -ENODEV;
+
+ np = of_find_node_by_name(psci_np, "reboot-mode");
+ if (!np) {
+ of_node_put(psci_np);
+ return -ENODEV;
+ }
+
+ ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
+ if (ret)
+ goto err_notifier;
+
+ reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
+ if (!reboot) {
+ ret = -ENOMEM;
+ goto err_kzalloc;
+ }
+
+ reboot->write = psci_set_vendor_sys_reset2;
+ reboot->driver_name = "psci";
+
+ ret = reboot_mode_register(reboot, of_fwnode_handle(np));
+ if (ret)
+ goto err_register;
+
+ return 0;
+
+err_register:
+ kfree(reboot);
+err_kzalloc:
+ atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
+err_notifier:
+ of_node_put(psci_np);
+ of_node_put(np);
+ return ret;
+}
+late_initcall(psci_init_vendor_reset)
+
static void __init psci_init_system_reset2(void)
{
int ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v16 08/14] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (6 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 09/14] arm64: dts: qcom: qcs6490-rb3gen2: " Shivendra Pratap
` (5 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, 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 73fce639370cd356687f14a3091848b8f422e36c..84322b74917f3a70adce5a4182adfa5d787ab11c 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -695,6 +695,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 4b04dea57ec8cc652e37f1d93c410404adaadd5d..211c2f223a03c63a79ddc736c18a5b79faec56a4 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -858,7 +858,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] 33+ messages in thread
* [PATCH v16 09/14] arm64: dts: qcom: qcs6490-rb3gen2: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (7 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 08/14] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 10/14] arm64: dts: qcom: lemans-ride: " Shivendra Pratap
` (4 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, 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 18cea8812001421456dc85547c3c711e2c42182a..f38fa2e52a635100a7505c615c0e96f1111d0b5a 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -975,6 +975,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] 33+ messages in thread
* [PATCH v16 10/14] arm64: dts: qcom: lemans-ride: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (8 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 09/14] arm64: dts: qcom: qcs6490-rb3gen2: " Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 11/14] arm64: dts: qcom: lemans-evk: " Shivendra Pratap
` (3 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, 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
lemans-ride 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/lemans-ride-common.dtsi | 7 +++++++
arch/arm64/boot/dts/qcom/lemans.dtsi | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/lemans-ride-common.dtsi b/arch/arm64/boot/dts/qcom/lemans-ride-common.dtsi
index c69aa2f41ce29f9f841cc6e6651a8efc38690e19..41ba0f4d437727cfe0c51e3d355427f37dce7f46 100644
--- a/arch/arm64/boot/dts/qcom/lemans-ride-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/lemans-ride-common.dtsi
@@ -722,6 +722,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/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
index cf685cb186edcade643790ba22f6a900beb85679..5bb27665cfa95954543f7a66ec424452ddeb24c5 100644
--- a/arch/arm64/boot/dts/qcom/lemans.dtsi
+++ b/arch/arm64/boot/dts/qcom/lemans.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] 33+ messages in thread
* [PATCH v16 11/14] arm64: dts: qcom: lemans-evk: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (9 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 10/14] arm64: dts: qcom: lemans-ride: " Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 12/14] arm64: dts: qcom: qcs8300-ride: " Shivendra Pratap
` (2 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla
Add support for SYSTEM_RESET2 vendor-specific resets in
lemans-evk 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.
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/lemans-evk.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/lemans-evk.dts b/arch/arm64/boot/dts/qcom/lemans-evk.dts
index c7dc9b8f445787a87ba4869bb426f70f14c1dc9f..09460441888a7011ff613c4fe9fa4b6872e12172 100644
--- a/arch/arm64/boot/dts/qcom/lemans-evk.dts
+++ b/arch/arm64/boot/dts/qcom/lemans-evk.dts
@@ -587,6 +587,13 @@ &pcie1_phy {
status = "okay";
};
+&psci {
+ reboot-mode {
+ mode-bootloader = <0x10001 0x2>;
+ mode-edl = <0 0x1>;
+ };
+};
+
&qupv3_id_0 {
status = "okay";
};
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v16 12/14] arm64: dts: qcom: qcs8300-ride: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (10 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 11/14] arm64: dts: qcom: lemans-evk: " Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 13/14] arm64: dts: qcom: monaco-evk: " Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 14/14] arm64: dts: qcom: qcs615-ride: " Shivendra Pratap
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla
Add support for SYSTEM_RESET2 vendor-specific resets in
qcs8300-ride 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.
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 7 +++++++
arch/arm64/boot/dts/qcom/qcs8300.dtsi | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
index cabb3f508704bc9eb0038bd797cc547d0c8cb3ed..7ebe292f1029393b43f34e66cb15c2cf97ebf30d 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
@@ -317,6 +317,13 @@ &iris {
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/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
index 8d78ccac411e495592a6ff532c99e7aba087d18c..89db75aa0777b9ce90732956024b6ccbec93428d 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
@@ -663,7 +663,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] 33+ messages in thread
* [PATCH v16 13/14] arm64: dts: qcom: monaco-evk: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (11 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 12/14] arm64: dts: qcom: qcs8300-ride: " Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 14/14] arm64: dts: qcom: qcs615-ride: " Shivendra Pratap
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla
Add support for SYSTEM_RESET2 vendor-specific resets in
monaco-evk 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.
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/monaco-evk.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/monaco-evk.dts b/arch/arm64/boot/dts/qcom/monaco-evk.dts
index e72cf6725a52c0c0e017be800bfac1773fad1059..87d613094e551e1c3b56dc45362a588eeae07444 100644
--- a/arch/arm64/boot/dts/qcom/monaco-evk.dts
+++ b/arch/arm64/boot/dts/qcom/monaco-evk.dts
@@ -400,6 +400,13 @@ &iris {
status = "okay";
};
+&psci {
+ reboot-mode {
+ mode-bootloader = <0x10001 0x2>;
+ mode-edl = <0 0x1>;
+ };
+};
+
&qupv3_id_0 {
status = "okay";
};
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v16 14/14] arm64: dts: qcom: qcs615-ride: Add PSCI SYSTEM_RESET2 types
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
` (12 preceding siblings ...)
2025-10-15 4:38 ` [PATCH v16 13/14] arm64: dts: qcom: monaco-evk: " Shivendra Pratap
@ 2025-10-15 4:38 ` Shivendra Pratap
13 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 4:38 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman, Shivendra Pratap,
Srinivas Kandagatla, Song Xue
From: Song Xue <quic_songxue@quicinc.com>
Add support for SYSTEM_RESET2 vendor-specific resets in
qcs615-ride 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.
Signed-off-by: Song Xue <quic_songxue@quicinc.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/qcs615-ride.dts | 7 +++++++
arch/arm64/boot/dts/qcom/sm6150.dtsi | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
index 705ea71b07a10aea82b5789e8ab9f757683f678a..bfb504db43368fe73ff200476ff5220334872c8a 100644
--- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
@@ -343,6 +343,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/sm6150.dtsi b/arch/arm64/boot/dts/qcom/sm6150.dtsi
index 3d2a1cb02b628a5db7ca14bea784429be5a020f9..9df5e94069f604e5393350f5d8906097d6d01209 100644
--- a/arch/arm64/boot/dts/qcom/sm6150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6150.dtsi
@@ -416,7 +416,7 @@ opp-128000000 {
};
};
- psci {
+ psci: psci {
compatible = "arm,psci-1.0";
method = "smc";
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode
2025-10-15 4:38 ` [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
@ 2025-10-15 6:55 ` Pavan Kondeti
2025-10-15 7:47 ` Shivendra Pratap
0 siblings, 1 reply; 33+ messages in thread
From: Pavan Kondeti @ 2025-10-15 6:55 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla, Umang Chheda
On Wed, Oct 15, 2025 at 10:08:22AM +0530, Shivendra Pratap wrote:
> +static int __init psci_init_vendor_reset(void)
> +{
> + struct reboot_mode_driver *reboot;
> + struct device_node *psci_np;
> + struct device_node *np;
> + int ret;
> +
> + if (!psci_system_reset2_supported)
> + return -EINVAL;
> +
> + psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
> + if (!psci_np)
> + return -ENODEV;
> +
> + np = of_find_node_by_name(psci_np, "reboot-mode");
> + if (!np) {
> + of_node_put(psci_np);
> + return -ENODEV;
> + }
> +
> + ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
> + if (ret)
> + goto err_notifier;
> +
> + reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
> + if (!reboot) {
> + ret = -ENOMEM;
> + goto err_kzalloc;
> + }
> +
> + reboot->write = psci_set_vendor_sys_reset2;
> + reboot->driver_name = "psci";
> +
> + ret = reboot_mode_register(reboot, of_fwnode_handle(np));
> + if (ret)
> + goto err_register;
> +
minor nit: np and psci_np reference must be dropped since we are done
using it.
> + return 0;
> +
> +err_register:
> + kfree(reboot);
> +err_kzalloc:
> + atomic_notifier_chain_unregister(&panic_notifier_list, &psci_panic_block);
> +err_notifier:
> + of_node_put(psci_np);
> + of_node_put(np);
> + return ret;
> +}
> +late_initcall(psci_init_vendor_reset)
> +
> static void __init psci_init_system_reset2(void)
> {
> int ret;
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode
2025-10-15 6:55 ` Pavan Kondeti
@ 2025-10-15 7:47 ` Shivendra Pratap
0 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-15 7:47 UTC (permalink / raw)
To: Pavan Kondeti
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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla, Umang Chheda
On 10/15/2025 12:25 PM, Pavan Kondeti wrote:
> On Wed, Oct 15, 2025 at 10:08:22AM +0530, Shivendra Pratap wrote:
>> +static int __init psci_init_vendor_reset(void)
>> +{
>> + struct reboot_mode_driver *reboot;
>> + struct device_node *psci_np;
>> + struct device_node *np;
>> + int ret;
>> +
>> + if (!psci_system_reset2_supported)
>> + return -EINVAL;
>> +
>> + psci_np = of_find_compatible_node(NULL, NULL, "arm,psci-1.0");
>> + if (!psci_np)
>> + return -ENODEV;
>> +
>> + np = of_find_node_by_name(psci_np, "reboot-mode");
>> + if (!np) {
>> + of_node_put(psci_np);
>> + return -ENODEV;
>> + }
>> +
>> + ret = atomic_notifier_chain_register(&panic_notifier_list, &psci_panic_block);
>> + if (ret)
>> + goto err_notifier;
>> +
>> + reboot = kzalloc(sizeof(*reboot), GFP_KERNEL);
>> + if (!reboot) {
>> + ret = -ENOMEM;
>> + goto err_kzalloc;
>> + }
>> +
>> + reboot->write = psci_set_vendor_sys_reset2;
>> + reboot->driver_name = "psci";
>> +
>> + ret = reboot_mode_register(reboot, of_fwnode_handle(np));
>> + if (ret)
>> + goto err_register;
>> +
>
> minor nit: np and psci_np reference must be dropped since we are done
> using it.
Ack. Sure. thanks. Will update this.
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 03/14] power: reset: reboot-mode: Add support for 64 bit magic
2025-10-15 4:38 ` [PATCH v16 03/14] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
@ 2025-10-15 8:40 ` Nirmesh Kumar Singh
0 siblings, 0 replies; 33+ messages in thread
From: Nirmesh Kumar Singh @ 2025-10-15 8:40 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,
Florian Fainelli, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla, Umang Chheda
On 10/15/2025 10:08 AM, 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.
>
> Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
> drivers/power/reset/nvmem-reboot-mode.c | 13 +++++++++----
> drivers/power/reset/qcom-pon.c | 11 ++++++++---
> drivers/power/reset/reboot-mode.c | 19 +++++++++++++------
> drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++---
> include/linux/reboot-mode.h | 3 ++-
> 5 files changed, 40 insertions(+), 17 deletions(-)
Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
Thanks,
Nirmesh
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-15 4:38 ` [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
@ 2025-10-15 8:45 ` Nirmesh Kumar Singh
2025-10-15 14:47 ` Bartosz Golaszewski
1 sibling, 0 replies; 33+ messages in thread
From: Nirmesh Kumar Singh @ 2025-10-15 8:45 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,
Florian Fainelli, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd, Andre Draszik,
Kathiravan Thirumoorthy, linux-pm, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/15/2025 10:08 AM, Shivendra Pratap wrote:
> 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. Use the driver_name field of the struct
> reboot_mode_driver to create the device. For device-based
> drivers, configure the device driver name as driver_name.
>
> 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.
>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
> drivers/power/reset/reboot-mode.c | 127 ++++++++++++++++++++++++++++++--------
> include/linux/reboot-mode.h | 2 +
> 2 files changed, 103 insertions(+), 26 deletions(-)
Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
Thanks,
Nirmesh
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal
2025-10-15 4:38 ` [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal Shivendra Pratap
@ 2025-10-15 14:32 ` Bartosz Golaszewski
2025-10-17 14:47 ` Shivendra Pratap
2025-10-28 3:34 ` Bjorn Andersson
1 sibling, 1 reply; 33+ messages in thread
From: Bartosz Golaszewski @ 2025-10-15 14:32 UTC (permalink / raw)
To: Shivendra Pratap
Cc: 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Wed, 15 Oct 2025 at 06:38, Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> wrote:
>
> List traversals must be synchronized to prevent race conditions
> and data corruption. The reboot-mode list is not protected by a
> lock currently, which can lead to concurrent access and race.
>
> Introduce a mutex lock to guard all operations on the reboot-mode
> list and ensure thread-safe access. The change prevents unsafe
> concurrent access on reboot-mode list.
>
> Fixes: 4fcd504edbf7 ("power: reset: add reboot mode driver")
> Fixes: ca3d2ea52314 ("power: reset: reboot-mode: better compatibility with DT (replace ' ,/')")
>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
> drivers/power/reset/reboot-mode.c | 96 +++++++++++++++++++++------------------
> include/linux/reboot-mode.h | 4 ++
> 2 files changed, 57 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index fba53f638da04655e756b5f8b7d2d666d1379535..8fc3e14638ea757c8dc3808c240ff569cbd74786 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -29,9 +29,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> if (!cmd)
> cmd = normal;
>
> - list_for_each_entry(info, &reboot->head, list)
> - if (!strcmp(info->mode, cmd))
> - return info->magic;
> + scoped_guard(mutex, &reboot->rb_lock) {
> + list_for_each_entry(info, &reboot->head, list)
> + if (!strcmp(info->mode, cmd))
> + return info->magic;
> + }
>
> /* try to match again, replacing characters impossible in DT */
> if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
> @@ -41,9 +43,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> strreplace(cmd_, ',', '-');
> strreplace(cmd_, '/', '-');
>
> - list_for_each_entry(info, &reboot->head, list)
> - if (!strcmp(info->mode, cmd_))
> - return info->magic;
> + scoped_guard(mutex, &reboot->rb_lock) {
> + list_for_each_entry(info, &reboot->head, list)
> + if (!strcmp(info->mode, cmd_))
> + return info->magic;
> + }
>
> return 0;
> }
> @@ -78,46 +82,50 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>
> 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);
> - 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);
> - continue;
> - }
> -
> - info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> - if (!info->mode) {
> - ret = -ENOMEM;
> - goto error;
> - } 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);
> - goto error;
> + mutex_init(&reboot->rb_lock);
> +
> + scoped_guard(mutex, &reboot->rb_lock) {
> + for_each_property_of_node(np, prop) {
> + if (strncmp(prop->name, PREFIX, len))
> + continue;
> +
> + info = devm_kzalloc(reboot->dev, 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);
> + continue;
> + }
> +
> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> + if (!info->mode) {
> + ret = -ENOMEM;
> + goto error;
> + } 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);
> + goto error;
> + }
> +
> + list_add_tail(&info->list, &reboot->head);
This seems to be the only call that actually needs synchronization.
All of the above can be run outside the critical section.
> }
>
> - list_add_tail(&info->list, &reboot->head);
> - }
> -
> - reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> - register_reboot_notifier(&reboot->reboot_notifier);
> + reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> + register_reboot_notifier(&reboot->reboot_notifier);
>
> - return 0;
> + return 0;
>
> error:
> - list_for_each_entry(info, &reboot->head, list)
> - kfree_const(info->mode);
> + list_for_each_entry(info, &reboot->head, list)
> + kfree_const(info->mode);
> + }
>
> return ret;
> }
> @@ -133,8 +141,10 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>
> unregister_reboot_notifier(&reboot->reboot_notifier);
>
> - list_for_each_entry(info, &reboot->head, list)
> - kfree_const(info->mode);
> + scoped_guard(mutex, &reboot->rb_lock) {
> + list_for_each_entry(info, &reboot->head, list)
> + kfree_const(info->mode);
> + }
Please destroy the mutex here.
Bart
>
> return 0;
> }
> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b73f80708197677db8dc2e43affc519782b7146e 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -2,11 +2,15 @@
> #ifndef __REBOOT_MODE_H__
> #define __REBOOT_MODE_H__
>
> +#include <linux/mutex.h>
> +
> struct reboot_mode_driver {
> struct device *dev;
> struct list_head head;
> int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
> struct notifier_block reboot_notifier;
> + /*Protects access to reboot mode list*/
> + struct mutex rb_lock;
> };
>
> int reboot_mode_register(struct reboot_mode_driver *reboot);
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration
2025-10-15 4:38 ` [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
@ 2025-10-15 14:40 ` Bartosz Golaszewski
2025-10-16 17:19 ` Shivendra Pratap
0 siblings, 1 reply; 33+ messages in thread
From: Bartosz Golaszewski @ 2025-10-15 14:40 UTC (permalink / raw)
To: Shivendra Pratap
Cc: 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Wed, 15 Oct 2025 at 06:38, Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> 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
> firmware node (fwnode) 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 | 45 +++++++++++++++++++++++++++++----------
> include/linux/reboot-mode.h | 3 ++-
> 2 files changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index 8fc3e14638ea757c8dc3808c240ff569cbd74786..c8f71e6f661ae14eb72bdcb1f412cd05faee3dd9 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-"
>
> @@ -69,17 +73,26 @@ static int reboot_mode_notify(struct notifier_block *this,
> /**
> * reboot_mode_register - register a reboot mode driver
> * @reboot: reboot mode driver
> + * @fwnode: Firmware node with reboot-mode configuration
> *
> * 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 fwnode_handle *fwnode)
> {
> struct mode_info *info;
> + struct mode_info *next;
> + struct device_node *np;
> struct property *prop;
> - struct device_node *np = reboot->dev->of_node;
> size_t len = strlen(PREFIX);
> int ret;
>
> + if (!fwnode)
> + return -EINVAL;
> +
> + np = to_of_node(fwnode);
> + if (!np)
> + return -EINVAL;
> +
> INIT_LIST_HEAD(&reboot->head);
>
> mutex_init(&reboot->rb_lock);
> @@ -89,28 +102,28 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
> if (strncmp(prop->name, PREFIX, len))
> continue;
>
> - info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
This change is good - devres should not be used in subsystem library
code, only in drivers - but it doesn't seem to belong here, can you
please separate it out and make it backportable?
> + 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;
> }
>
> info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> if (!info->mode) {
> ret = -ENOMEM;
> + kfree(info);
> goto error;
> } else if (info->mode[0] == '\0') {
> kfree_const(info->mode);
> + kfree(info);
> 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;
> }
>
> @@ -123,8 +136,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
> return 0;
>
> error:
> - list_for_each_entry(info, &reboot->head, list)
> + list_for_each_entry_safe(info, next, &reboot->head, list) {
> + list_del(&info->list);
Same here, not deleting the entries currently seems like a bug? Do we
depend on the driver detach to clean up the resources on failure?
> kfree_const(info->mode);
> + kfree(info);
> + }
> }
>
> return ret;
> @@ -138,12 +154,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);
>
> scoped_guard(mutex, &reboot->rb_lock) {
> - list_for_each_entry(info, &reboot->head, list)
> + list_for_each_entry_safe(info, next, &reboot->head, list) {
> + list_del(&info->list);
> kfree_const(info->mode);
> + kfree(info);
> + }
> }
>
> return 0;
> @@ -168,11 +188,14 @@ int devm_reboot_mode_register(struct device *dev,
> struct reboot_mode_driver **dr;
> int rc;
>
> + if (!reboot->dev || !reboot->dev->of_node)
> + return -EINVAL;
> +
> dr = devres_alloc(devm_reboot_mode_release, sizeof(*dr), GFP_KERNEL);
> if (!dr)
> return -ENOMEM;
>
> - rc = reboot_mode_register(reboot);
> + rc = reboot_mode_register(reboot, of_fwnode_handle(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 b73f80708197677db8dc2e43affc519782b7146e..7f05fd873e95ca8249bc167c21aa6b76faba7849 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -2,6 +2,7 @@
> #ifndef __REBOOT_MODE_H__
> #define __REBOOT_MODE_H__
>
> +#include <linux/fwnode.h>
> #include <linux/mutex.h>
>
> struct reboot_mode_driver {
> @@ -13,7 +14,7 @@ struct reboot_mode_driver {
> struct mutex rb_lock;
> };
>
> -int reboot_mode_register(struct reboot_mode_driver *reboot);
> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle *fwnode);
> 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
>
Bartosz
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-15 4:38 ` [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
2025-10-15 8:45 ` Nirmesh Kumar Singh
@ 2025-10-15 14:47 ` Bartosz Golaszewski
2025-10-17 19:40 ` Shivendra Pratap
1 sibling, 1 reply; 33+ messages in thread
From: Bartosz Golaszewski @ 2025-10-15 14:47 UTC (permalink / raw)
To: Shivendra Pratap
Cc: 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, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Wed, 15 Oct 2025 at 06:39, Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> wrote:
>
> 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. Use the driver_name field of the struct
> reboot_mode_driver to create the device. For device-based
> drivers, configure the device driver name as driver_name.
>
> 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.
>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
[snip]
> +
> +static int create_reboot_mode_device(struct reboot_mode_driver *reboot)
> +{
> + 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, reboot->driver_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)
> + goto create_file_err;
> +
> + dr = devres_alloc(release_reboot_mode_device, sizeof(*dr), GFP_KERNEL);
> + if (!dr) {
> + ret = -ENOMEM;
> + goto devres_alloc_error;
> + }
> +
> + *dr = reboot;
> + devres_add(reboot->reboot_dev, dr);
If you're using devres here - at least make it obvious by adding the
devm_ prefix to the function name and make it take an explicit struct
device * parameter so that it's clear who owns the managed resource.
Bart
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration
2025-10-15 14:40 ` Bartosz Golaszewski
@ 2025-10-16 17:19 ` Shivendra Pratap
2025-10-17 9:06 ` Bartosz Golaszewski
0 siblings, 1 reply; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-16 17:19 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/15/2025 8:10 PM, Bartosz Golaszewski wrote:
> On Wed, 15 Oct 2025 at 06:38, Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> 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
>> firmware node (fwnode) 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 | 45 +++++++++++++++++++++++++++++----------
>> include/linux/reboot-mode.h | 3 ++-
>> 2 files changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index 8fc3e14638ea757c8dc3808c240ff569cbd74786..c8f71e6f661ae14eb72bdcb1f412cd05faee3dd9 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-"
>>
>> @@ -69,17 +73,26 @@ static int reboot_mode_notify(struct notifier_block *this,
>> /**
>> * reboot_mode_register - register a reboot mode driver
>> * @reboot: reboot mode driver
>> + * @fwnode: Firmware node with reboot-mode configuration
>> *
>> * 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 fwnode_handle *fwnode)
>> {
>> struct mode_info *info;
>> + struct mode_info *next;
>> + struct device_node *np;
>> struct property *prop;
>> - struct device_node *np = reboot->dev->of_node;
>> size_t len = strlen(PREFIX);
>> int ret;
>>
>> + if (!fwnode)
>> + return -EINVAL;
>> +
>> + np = to_of_node(fwnode);
>> + if (!np)
>> + return -EINVAL;
>> +
>> INIT_LIST_HEAD(&reboot->head);
>>
>> mutex_init(&reboot->rb_lock);
>> @@ -89,28 +102,28 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>> if (strncmp(prop->name, PREFIX, len))
>> continue;
>>
>> - info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
>
> This change is good - devres should not be used in subsystem library
> code, only in drivers - but it doesn't seem to belong here, can you
> please separate it out and make it backportable?
sure. Just to confirm we should separate out the devm_kzalloc part of the
change and add a fixes tag.
>
>> + 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;
>> }
>>
>> info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>> if (!info->mode) {
>> ret = -ENOMEM;
>> + kfree(info);
>> goto error;
>> } else if (info->mode[0] == '\0') {
>> kfree_const(info->mode);
>> + kfree(info);
>> 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;
>> }
>>
>> @@ -123,8 +136,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>> return 0;
>>
>> error:
>> - list_for_each_entry(info, &reboot->head, list)
>> + list_for_each_entry_safe(info, next, &reboot->head, list) {
>> + list_del(&info->list);
>
> Same here, not deleting the entries currently seems like a bug? Do we
> depend on the driver detach to clean up the resources on failure?
sure, so this should also go as fixes? and should we remove the other
dev_err(printk) also as fixes? or that can still got with the change
where we add fwnode based registration?
thanks for review!
-
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration
2025-10-16 17:19 ` Shivendra Pratap
@ 2025-10-17 9:06 ` Bartosz Golaszewski
2025-10-17 14:24 ` Shivendra Pratap
0 siblings, 1 reply; 33+ messages in thread
From: Bartosz Golaszewski @ 2025-10-17 9:06 UTC (permalink / raw)
To: Shivendra Pratap
Cc: 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Thu, 16 Oct 2025 at 19:19, Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> wrote:
> >>
> >> - info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
> >
> > This change is good - devres should not be used in subsystem library
> > code, only in drivers - but it doesn't seem to belong here, can you
> > please separate it out and make it backportable?
>
> sure. Just to confirm we should separate out the devm_kzalloc part of the
> change and add a fixes tag.
>
And preferably put it first in the series to avoid conflicts.
> >> @@ -123,8 +136,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
> >> return 0;
> >>
> >> error:
> >> - list_for_each_entry(info, &reboot->head, list)
> >> + list_for_each_entry_safe(info, next, &reboot->head, list) {
> >> + list_del(&info->list);
> >
> > Same here, not deleting the entries currently seems like a bug? Do we
> > depend on the driver detach to clean up the resources on failure?
>
> sure, so this should also go as fixes? and should we remove the other
> dev_err(printk) also as fixes? or that can still got with the change
> where we add fwnode based registration?
>
It doesn't seem to be strictly required by current code as the users
use it "correctly" but if the API becomes used in different ways - for
instance the structure may be reused after failure - it's a good idea
to backport it. In general we should undo everything we did in the
same function if we fail at some point.
Bart
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration
2025-10-17 9:06 ` Bartosz Golaszewski
@ 2025-10-17 14:24 ` Shivendra Pratap
0 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-17 14:24 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/17/2025 2:36 PM, Bartosz Golaszewski wrote:
> On Thu, 16 Oct 2025 at 19:19, Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>>>
>>>> - info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
>>>
>>> This change is good - devres should not be used in subsystem library
>>> code, only in drivers - but it doesn't seem to belong here, can you
>>> please separate it out and make it backportable?
>>
>> sure. Just to confirm we should separate out the devm_kzalloc part of the
>> change and add a fixes tag.
>>
>
> And preferably put it first in the series to avoid conflicts.
Ack.
>
>>>> @@ -123,8 +136,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>>> return 0;
>>>>
>>>> error:
>>>> - list_for_each_entry(info, &reboot->head, list)
>>>> + list_for_each_entry_safe(info, next, &reboot->head, list) {
>>>> + list_del(&info->list);
>>>
>>> Same here, not deleting the entries currently seems like a bug? Do we
>>> depend on the driver detach to clean up the resources on failure?
>>
>> sure, so this should also go as fixes? and should we remove the other
>> dev_err(printk) also as fixes? or that can still got with the change
>> where we add fwnode based registration?
>>
>
> It doesn't seem to be strictly required by current code as the users
> use it "correctly" but if the API becomes used in different ways - for
> instance the structure may be reused after failure - it's a good idea
> to backport it. In general we should undo everything we did in the
> same function if we fail at some point.
sure. will update it.
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal
2025-10-15 14:32 ` Bartosz Golaszewski
@ 2025-10-17 14:47 ` Shivendra Pratap
0 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-17 14:47 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/15/2025 8:02 PM, Bartosz Golaszewski wrote:
> On Wed, 15 Oct 2025 at 06:38, Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>
>> List traversals must be synchronized to prevent race conditions
>> and data corruption. The reboot-mode list is not protected by a
>> lock currently, which can lead to concurrent access and race.
>>
>> Introduce a mutex lock to guard all operations on the reboot-mode
>> list and ensure thread-safe access. The change prevents unsafe
>> concurrent access on reboot-mode list.
>>
>> Fixes: 4fcd504edbf7 ("power: reset: add reboot mode driver")
>> Fixes: ca3d2ea52314 ("power: reset: reboot-mode: better compatibility with DT (replace ' ,/')")
>>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>> drivers/power/reset/reboot-mode.c | 96 +++++++++++++++++++++------------------
>> include/linux/reboot-mode.h | 4 ++
>> 2 files changed, 57 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index fba53f638da04655e756b5f8b7d2d666d1379535..8fc3e14638ea757c8dc3808c240ff569cbd74786 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -29,9 +29,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> if (!cmd)
>> cmd = normal;
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> - if (!strcmp(info->mode, cmd))
>> - return info->magic;
>> + scoped_guard(mutex, &reboot->rb_lock) {
>> + list_for_each_entry(info, &reboot->head, list)
>> + if (!strcmp(info->mode, cmd))
>> + return info->magic;
>> + }
>>
>> /* try to match again, replacing characters impossible in DT */
>> if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
>> @@ -41,9 +43,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> strreplace(cmd_, ',', '-');
>> strreplace(cmd_, '/', '-');
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> - if (!strcmp(info->mode, cmd_))
>> - return info->magic;
>> + scoped_guard(mutex, &reboot->rb_lock) {
>> + list_for_each_entry(info, &reboot->head, list)
>> + if (!strcmp(info->mode, cmd_))
>> + return info->magic;
>> + }
>>
>> return 0;
>> }
>> @@ -78,46 +82,50 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>
>> 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);
>> - 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);
>> - continue;
>> - }
>> -
>> - info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>> - if (!info->mode) {
>> - ret = -ENOMEM;
>> - goto error;
>> - } 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);
>> - goto error;
>> + mutex_init(&reboot->rb_lock);
>> +
>> + scoped_guard(mutex, &reboot->rb_lock) {
>> + for_each_property_of_node(np, prop) {
>> + if (strncmp(prop->name, PREFIX, len))
>> + continue;
>> +
>> + info = devm_kzalloc(reboot->dev, 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);
>> + continue;
>> + }
>> +
>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>> + if (!info->mode) {
>> + ret = -ENOMEM;
>> + goto error;
>> + } 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);
>> + goto error;
>> + }
>> +
>> + list_add_tail(&info->list, &reboot->head);
>
> This seems to be the only call that actually needs synchronization.
> All of the above can be run outside the critical section.
sure. will add it only around the required lines.
>
>> }
>>
>> - list_add_tail(&info->list, &reboot->head);
>> - }
>> -
>> - reboot->reboot_notifier.notifier_call = reboot_mode_notify;
>> - register_reboot_notifier(&reboot->reboot_notifier);
>> + reboot->reboot_notifier.notifier_call = reboot_mode_notify;
>> + register_reboot_notifier(&reboot->reboot_notifier);
>>
>> - return 0;
>> + return 0;
>>
>> error:
>> - list_for_each_entry(info, &reboot->head, list)
>> - kfree_const(info->mode);
>> + list_for_each_entry(info, &reboot->head, list)
>> + kfree_const(info->mode);
>> + }
>>
>> return ret;
>> }
>> @@ -133,8 +141,10 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>>
>> unregister_reboot_notifier(&reboot->reboot_notifier);
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> - kfree_const(info->mode);
>> + scoped_guard(mutex, &reboot->rb_lock) {
>> + list_for_each_entry(info, &reboot->head, list)
>> + kfree_const(info->mode);
>> + }
>
> Please destroy the mutex here.
sure thanks. will add destroy here.
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-15 14:47 ` Bartosz Golaszewski
@ 2025-10-17 19:40 ` Shivendra Pratap
2025-10-20 7:40 ` Bartosz Golaszewski
0 siblings, 1 reply; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-17 19:40 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: 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, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/15/2025 8:17 PM, Bartosz Golaszewski wrote:
> On Wed, 15 Oct 2025 at 06:39, Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>
>> 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. Use the driver_name field of the struct
>> reboot_mode_driver to create the device. For device-based
>> drivers, configure the device driver name as driver_name.
>>
>> 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.
>>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>
> [snip]
>
>> +
>> +static int create_reboot_mode_device(struct reboot_mode_driver *reboot)
>> +{
>> + 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, reboot->driver_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)
>> + goto create_file_err;
>> +
>> + dr = devres_alloc(release_reboot_mode_device, sizeof(*dr), GFP_KERNEL);
>> + if (!dr) {
>> + ret = -ENOMEM;
>> + goto devres_alloc_error;
>> + }
>> +
>> + *dr = reboot;
>> + devres_add(reboot->reboot_dev, dr);
>
> If you're using devres here - at least make it obvious by adding the
> devm_ prefix to the function name and make it take an explicit struct
> device * parameter so that it's clear who owns the managed resource.
>
sure. we can add devm_ prefix to the function name.
reboot->reboot_dev is an internal member of struct reboot_mode_driver *reboot.
The struct reboot_mode_driver *reboot is owned by the calling driver.
If we want to PASS reboot->reboot_dev to the devm_ prefixed function call, we
will need to kind of split create_reboot_mode_device into two calls - device_create
in a separate function and then call the devm_ prefix function where we add the devres_alloc.
Can you suggest a bit more on this?
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-17 19:40 ` Shivendra Pratap
@ 2025-10-20 7:40 ` Bartosz Golaszewski
2025-10-22 14:21 ` Shivendra Pratap
0 siblings, 1 reply; 33+ messages in thread
From: Bartosz Golaszewski @ 2025-10-20 7:40 UTC (permalink / raw)
To: Shivendra Pratap
Cc: 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, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Fri, 17 Oct 2025 at 21:40, Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> wrote:
>
> >
> > If you're using devres here - at least make it obvious by adding the
> > devm_ prefix to the function name and make it take an explicit struct
> > device * parameter so that it's clear who owns the managed resource.
> >
>
> sure. we can add devm_ prefix to the function name.
> reboot->reboot_dev is an internal member of struct reboot_mode_driver *reboot.
> The struct reboot_mode_driver *reboot is owned by the calling driver.
> If we want to PASS reboot->reboot_dev to the devm_ prefixed function call, we
> will need to kind of split create_reboot_mode_device into two calls - device_create
> in a separate function and then call the devm_ prefix function where we add the devres_alloc.
> Can you suggest a bit more on this?
>
Ah, ok I missed the broken logic here. Devres should only be used in
devices already *attached* to a driver as all managed resources will
get released on driver *detach*. What you have here may "work" by
accident but that's not correct and is very fragile as soon as you
have some non-standard behavior or error paths. Devres won't fly here,
please just use regular allocation and free whatever you need in the
corresponding release/free/whatever routine.
Bart
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-20 7:40 ` Bartosz Golaszewski
@ 2025-10-22 14:21 ` Shivendra Pratap
2025-10-23 15:02 ` Bartosz Golaszewski
0 siblings, 1 reply; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-22 14:21 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: 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, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/20/2025 1:10 PM, Bartosz Golaszewski wrote:
> On Fri, 17 Oct 2025 at 21:40, Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>
>>>
>>> If you're using devres here - at least make it obvious by adding the
>>> devm_ prefix to the function name and make it take an explicit struct
>>> device * parameter so that it's clear who owns the managed resource.
>>>
>>
>> sure. we can add devm_ prefix to the function name.
>> reboot->reboot_dev is an internal member of struct reboot_mode_driver *reboot.
>> The struct reboot_mode_driver *reboot is owned by the calling driver.
>> If we want to PASS reboot->reboot_dev to the devm_ prefixed function call, we
>> will need to kind of split create_reboot_mode_device into two calls - device_create
>> in a separate function and then call the devm_ prefix function where we add the devres_alloc.
>> Can you suggest a bit more on this?
>>
>
> Ah, ok I missed the broken logic here. Devres should only be used in
> devices already *attached* to a driver as all managed resources will
> get released on driver *detach*. What you have here may "work" by
> accident but that's not correct and is very fragile as soon as you
> have some non-standard behavior or error paths. Devres won't fly here,
> please just use regular allocation and free whatever you need in the
> corresponding release/free/whatever routine.
Thanks, got the problem here. Was using devres to associate the reboot_mode struct
with the driver, so that it could be retrieved later when reboot_modes_show is called.
When reboot_modes_show is invoked, there's no direct way to identify which reboot_mode
instance is tied to the current driver, as multiple drivers can register with the reboot-mode
framework at the same time. Without devres, will need to maintain a global list of mapping for
all device driver structs and their corresponding reboot_mode struct. Then reboot_modes_show
would have to look up the correct reboot_mode struct using the device driver's pointer.
Hope its ok to maintain that separate logic here?
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-22 14:21 ` Shivendra Pratap
@ 2025-10-23 15:02 ` Bartosz Golaszewski
2025-10-23 15:54 ` Shivendra Pratap
0 siblings, 1 reply; 33+ messages in thread
From: Bartosz Golaszewski @ 2025-10-23 15:02 UTC (permalink / raw)
To: Shivendra Pratap
Cc: 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, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Wed, 22 Oct 2025 at 16:21, Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> wrote:
>
>
>
> On 10/20/2025 1:10 PM, Bartosz Golaszewski wrote:
> > On Fri, 17 Oct 2025 at 21:40, Shivendra Pratap
> > <shivendra.pratap@oss.qualcomm.com> wrote:
> >>
> >>>
> >>> If you're using devres here - at least make it obvious by adding the
> >>> devm_ prefix to the function name and make it take an explicit struct
> >>> device * parameter so that it's clear who owns the managed resource.
> >>>
> >>
> >> sure. we can add devm_ prefix to the function name.
> >> reboot->reboot_dev is an internal member of struct reboot_mode_driver *reboot.
> >> The struct reboot_mode_driver *reboot is owned by the calling driver.
> >> If we want to PASS reboot->reboot_dev to the devm_ prefixed function call, we
> >> will need to kind of split create_reboot_mode_device into two calls - device_create
> >> in a separate function and then call the devm_ prefix function where we add the devres_alloc.
> >> Can you suggest a bit more on this?
> >>
> >
> > Ah, ok I missed the broken logic here. Devres should only be used in
> > devices already *attached* to a driver as all managed resources will
> > get released on driver *detach*. What you have here may "work" by
> > accident but that's not correct and is very fragile as soon as you
> > have some non-standard behavior or error paths. Devres won't fly here,
> > please just use regular allocation and free whatever you need in the
> > corresponding release/free/whatever routine.
>
> Thanks, got the problem here. Was using devres to associate the reboot_mode struct
> with the driver, so that it could be retrieved later when reboot_modes_show is called.
>
> When reboot_modes_show is invoked, there's no direct way to identify which reboot_mode
> instance is tied to the current driver, as multiple drivers can register with the reboot-mode
> framework at the same time. Without devres, will need to maintain a global list of mapping for
> all device driver structs and their corresponding reboot_mode struct. Then reboot_modes_show
> would have to look up the correct reboot_mode struct using the device driver's pointer.
>
> Hope its ok to maintain that separate logic here?
>
Why can't you just do:
device_create(rb_class, NULL, 0, data reboot->driver_name);
Where data is whatever driver data you want to associate with the new
class device? You can then retrieve it with dev_get_drvdata() in
callbacks.
Bartosz
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2025-10-23 15:02 ` Bartosz Golaszewski
@ 2025-10-23 15:54 ` Shivendra Pratap
0 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-23 15:54 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: 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, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/23/2025 8:32 PM, Bartosz Golaszewski wrote:
> On Wed, 22 Oct 2025 at 16:21, Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>
>>
>>
>> On 10/20/2025 1:10 PM, Bartosz Golaszewski wrote:
>>> On Fri, 17 Oct 2025 at 21:40, Shivendra Pratap
>>> <shivendra.pratap@oss.qualcomm.com> wrote:
>>>>
>>>>>
>>>>> If you're using devres here - at least make it obvious by adding the
>>>>> devm_ prefix to the function name and make it take an explicit struct
>>>>> device * parameter so that it's clear who owns the managed resource.
>>>>>
>>>>
>>>> sure. we can add devm_ prefix to the function name.
>>>> reboot->reboot_dev is an internal member of struct reboot_mode_driver *reboot.
>>>> The struct reboot_mode_driver *reboot is owned by the calling driver.
>>>> If we want to PASS reboot->reboot_dev to the devm_ prefixed function call, we
>>>> will need to kind of split create_reboot_mode_device into two calls - device_create
>>>> in a separate function and then call the devm_ prefix function where we add the devres_alloc.
>>>> Can you suggest a bit more on this?
>>>>
>>>
>>> Ah, ok I missed the broken logic here. Devres should only be used in
>>> devices already *attached* to a driver as all managed resources will
>>> get released on driver *detach*. What you have here may "work" by
>>> accident but that's not correct and is very fragile as soon as you
>>> have some non-standard behavior or error paths. Devres won't fly here,
>>> please just use regular allocation and free whatever you need in the
>>> corresponding release/free/whatever routine.
>>
>> Thanks, got the problem here. Was using devres to associate the reboot_mode struct
>> with the driver, so that it could be retrieved later when reboot_modes_show is called.
>>
>> When reboot_modes_show is invoked, there's no direct way to identify which reboot_mode
>> instance is tied to the current driver, as multiple drivers can register with the reboot-mode
>> framework at the same time. Without devres, will need to maintain a global list of mapping for
>> all device driver structs and their corresponding reboot_mode struct. Then reboot_modes_show
>> would have to look up the correct reboot_mode struct using the device driver's pointer.
>>
>> Hope its ok to maintain that separate logic here?
>>
>
> Why can't you just do:
>
> device_create(rb_class, NULL, 0, data reboot->driver_name);
>
> Where data is whatever driver data you want to associate with the new
> class device? You can then retrieve it with dev_get_drvdata() in
> callbacks.
sure. thanks for the suggestion. That will make it much simpler.
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal
2025-10-15 4:38 ` [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal Shivendra Pratap
2025-10-15 14:32 ` Bartosz Golaszewski
@ 2025-10-28 3:34 ` Bjorn Andersson
2025-10-29 17:48 ` Shivendra Pratap
1 sibling, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2025-10-28 3:34 UTC (permalink / raw)
To: Shivendra Pratap
Cc: Bartosz Golaszewski, 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On Wed, Oct 15, 2025 at 10:08:16AM +0530, Shivendra Pratap wrote:
> List traversals must be synchronized to prevent race conditions
> and data corruption. The reboot-mode list is not protected by a
> lock currently, which can lead to concurrent access and race.
Is it a theoretical future race or something that we can hit in the
current implementation?
>
> Introduce a mutex lock to guard all operations on the reboot-mode
> list and ensure thread-safe access. The change prevents unsafe
> concurrent access on reboot-mode list.
I was under the impression that these lists where created during boot
and then used at some later point, which at best would bring a
theoretical window for a race... Reviewing the code supports my
understanding, but perhaps I'm missing something?
>
> Fixes: 4fcd504edbf7 ("power: reset: add reboot mode driver")
> Fixes: ca3d2ea52314 ("power: reset: reboot-mode: better compatibility with DT (replace ' ,/')")
>
Skip this empty line, please.
And given that you have fixes here, I guess this is a problem today. In
which case, this shouldn't have been carried for 16 versions - but have
sent and been merged on its own already.
So please, if this is a real issue, start your commit message with a
descriptive problem description, to make it clear that this needs to be
merged yesterday - or drop the fixes.
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
> drivers/power/reset/reboot-mode.c | 96 +++++++++++++++++++++------------------
> include/linux/reboot-mode.h | 4 ++
> 2 files changed, 57 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index fba53f638da04655e756b5f8b7d2d666d1379535..8fc3e14638ea757c8dc3808c240ff569cbd74786 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -29,9 +29,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> if (!cmd)
> cmd = normal;
>
> - list_for_each_entry(info, &reboot->head, list)
> - if (!strcmp(info->mode, cmd))
> - return info->magic;
> + scoped_guard(mutex, &reboot->rb_lock) {
> + list_for_each_entry(info, &reboot->head, list)
> + if (!strcmp(info->mode, cmd))
> + return info->magic;
> + }
>
> /* try to match again, replacing characters impossible in DT */
> if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
> @@ -41,9 +43,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> strreplace(cmd_, ',', '-');
> strreplace(cmd_, '/', '-');
>
> - list_for_each_entry(info, &reboot->head, list)
> - if (!strcmp(info->mode, cmd_))
> - return info->magic;
> + scoped_guard(mutex, &reboot->rb_lock) {
> + list_for_each_entry(info, &reboot->head, list)
> + if (!strcmp(info->mode, cmd_))
> + return info->magic;
> + }
>
> return 0;
> }
> @@ -78,46 +82,50 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>
> 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);
> - 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);
> - continue;
> - }
> -
> - info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> - if (!info->mode) {
> - ret = -ENOMEM;
> - goto error;
> - } 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);
> - goto error;
> + mutex_init(&reboot->rb_lock);
> +
> + scoped_guard(mutex, &reboot->rb_lock) {
I don't see how this can race with anything, reboot_mode_register() is
supposed to be called from some probe function, with reboot_mode_driver
being a "local" object.
The guard here "protects" &reboot->head, but that is not a shared
resources at this point.
> + for_each_property_of_node(np, prop) {
> + if (strncmp(prop->name, PREFIX, len))
> + continue;
> +
> + info = devm_kzalloc(reboot->dev, 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);
> + continue;
> + }
> +
> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> + if (!info->mode) {
> + ret = -ENOMEM;
> + goto error;
> + } 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);
> + goto error;
> + }
> +
> + list_add_tail(&info->list, &reboot->head);
> }
>
> - list_add_tail(&info->list, &reboot->head);
> - }
> -
> - reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> - register_reboot_notifier(&reboot->reboot_notifier);
> + reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> + register_reboot_notifier(&reboot->reboot_notifier);
Once register_reboot_notifier() has been called, &reboot->head is
visible outside the specific driver instance.
So, there's no reason to lock in reboot_mode_register().
>
> - return 0;
> + return 0;
>
> error:
> - list_for_each_entry(info, &reboot->head, list)
> - kfree_const(info->mode);
> + list_for_each_entry(info, &reboot->head, list)
> + kfree_const(info->mode);
> + }
>
> return ret;
> }
> @@ -133,8 +141,10 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>
> unregister_reboot_notifier(&reboot->reboot_notifier);
>
> - list_for_each_entry(info, &reboot->head, list)
> - kfree_const(info->mode);
> + scoped_guard(mutex, &reboot->rb_lock) {
get_reboot_mode_magic() is only called from reboot_mode_notify(), which
is only invoked by blocking_notifier_call_chain().
blocking_notifier_call_chain() takes a read semaphore.
unregister_reboot_notifier() take a write semaphore.
So, if we're racing with a shutdown or reboot, I see two possible
things:
1) blocking_notifier_call_chain() happens first and calls
reboot_mode_notify(), blocking unregister_reboot_notifier(). Once it
returns, the unregister proceeds and we enter case #2
2) unregister_reboot_notifier() happens first (or after the
blocking_notifier_call_chain() returns). Our reboot object is removed
from the list and blocking_notifier_call_chain() will not invoke
reboot_mode_notify().
In either case, the list has a single owner here.
As far as I can see, the only race left is if multiple concurrent calls
happens to blocking_notifier_call_chain(), the behavior of
reboot->write() might be undefined. But I think that is reasonable.
Please let me know if I'm missing something.
Thanks,
Bjorn
> + list_for_each_entry(info, &reboot->head, list)
> + kfree_const(info->mode);
> + }
>
> return 0;
> }
> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b73f80708197677db8dc2e43affc519782b7146e 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -2,11 +2,15 @@
> #ifndef __REBOOT_MODE_H__
> #define __REBOOT_MODE_H__
>
> +#include <linux/mutex.h>
> +
> struct reboot_mode_driver {
> struct device *dev;
> struct list_head head;
> int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
> struct notifier_block reboot_notifier;
> + /*Protects access to reboot mode list*/
> + struct mutex rb_lock;
> };
>
> int reboot_mode_register(struct reboot_mode_driver *reboot);
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal
2025-10-28 3:34 ` Bjorn Andersson
@ 2025-10-29 17:48 ` Shivendra Pratap
0 siblings, 0 replies; 33+ messages in thread
From: Shivendra Pratap @ 2025-10-29 17:48 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Bartosz Golaszewski, 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, Moritz Fischer, John Stultz, Matthias Brugger,
Krzysztof Kozlowski, Dmitry Baryshkov, Mukesh Ojha, Stephen Boyd,
Andre Draszik, Kathiravan Thirumoorthy, linux-pm, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, Elliot Berman,
Srinivas Kandagatla
On 10/28/2025 9:04 AM, Bjorn Andersson wrote:
> On Wed, Oct 15, 2025 at 10:08:16AM +0530, Shivendra Pratap wrote:
>> List traversals must be synchronized to prevent race conditions
>> and data corruption. The reboot-mode list is not protected by a
>> lock currently, which can lead to concurrent access and race.
>
> Is it a theoretical future race or something that we can hit in the
> current implementation?
>
>>
>> Introduce a mutex lock to guard all operations on the reboot-mode
>> list and ensure thread-safe access. The change prevents unsafe
>> concurrent access on reboot-mode list.
>
> I was under the impression that these lists where created during boot
> and then used at some later point, which at best would bring a
> theoretical window for a race... Reviewing the code supports my
> understanding, but perhaps I'm missing something?
>
>>
>> Fixes: 4fcd504edbf7 ("power: reset: add reboot mode driver")
>> Fixes: ca3d2ea52314 ("power: reset: reboot-mode: better compatibility with DT (replace ' ,/')")
>>
>
> Skip this empty line, please.
>
>
> And given that you have fixes here, I guess this is a problem today. In
> which case, this shouldn't have been carried for 16 versions - but have
> sent and been merged on its own already.
>
> So please, if this is a real issue, start your commit message with a
> descriptive problem description, to make it clear that this needs to be
> merged yesterday - or drop the fixes.
>
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>> drivers/power/reset/reboot-mode.c | 96 +++++++++++++++++++++------------------
>> include/linux/reboot-mode.h | 4 ++
>> 2 files changed, 57 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index fba53f638da04655e756b5f8b7d2d666d1379535..8fc3e14638ea757c8dc3808c240ff569cbd74786 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -29,9 +29,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> if (!cmd)
>> cmd = normal;
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> - if (!strcmp(info->mode, cmd))
>> - return info->magic;
>> + scoped_guard(mutex, &reboot->rb_lock) {
>> + list_for_each_entry(info, &reboot->head, list)
>> + if (!strcmp(info->mode, cmd))
>> + return info->magic;
>> + }
>>
>> /* try to match again, replacing characters impossible in DT */
>> if (strscpy(cmd_, cmd, sizeof(cmd_)) == -E2BIG)
>> @@ -41,9 +43,11 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> strreplace(cmd_, ',', '-');
>> strreplace(cmd_, '/', '-');
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> - if (!strcmp(info->mode, cmd_))
>> - return info->magic;
>> + scoped_guard(mutex, &reboot->rb_lock) {
>> + list_for_each_entry(info, &reboot->head, list)
>> + if (!strcmp(info->mode, cmd_))
>> + return info->magic;
>> + }
>>
>> return 0;
>> }
>> @@ -78,46 +82,50 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>
>> 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);
>> - 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);
>> - continue;
>> - }
>> -
>> - info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>> - if (!info->mode) {
>> - ret = -ENOMEM;
>> - goto error;
>> - } 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);
>> - goto error;
>> + mutex_init(&reboot->rb_lock);
>> +
>> + scoped_guard(mutex, &reboot->rb_lock) {
>
> I don't see how this can race with anything, reboot_mode_register() is
> supposed to be called from some probe function, with reboot_mode_driver
> being a "local" object.
>
> The guard here "protects" &reboot->head, but that is not a shared
> resources at this point.
>
>> + for_each_property_of_node(np, prop) {
>> + if (strncmp(prop->name, PREFIX, len))
>> + continue;
>> +
>> + info = devm_kzalloc(reboot->dev, 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);
>> + continue;
>> + }
>> +
>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>> + if (!info->mode) {
>> + ret = -ENOMEM;
>> + goto error;
>> + } 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);
>> + goto error;
>> + }
>> +
>> + list_add_tail(&info->list, &reboot->head);
>> }
>>
>> - list_add_tail(&info->list, &reboot->head);
>> - }
>> -
>> - reboot->reboot_notifier.notifier_call = reboot_mode_notify;
>> - register_reboot_notifier(&reboot->reboot_notifier);
>> + reboot->reboot_notifier.notifier_call = reboot_mode_notify;
>> + register_reboot_notifier(&reboot->reboot_notifier);
>
> Once register_reboot_notifier() has been called, &reboot->head is
> visible outside the specific driver instance.
>
> So, there's no reason to lock in reboot_mode_register().
>
>>
>> - return 0;
>> + return 0;
>>
>> error:
>> - list_for_each_entry(info, &reboot->head, list)
>> - kfree_const(info->mode);
>> + list_for_each_entry(info, &reboot->head, list)
>> + kfree_const(info->mode);
>> + }
>>
>> return ret;
>> }
>> @@ -133,8 +141,10 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>>
>> unregister_reboot_notifier(&reboot->reboot_notifier);
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> - kfree_const(info->mode);
>> + scoped_guard(mutex, &reboot->rb_lock) {
>
> get_reboot_mode_magic() is only called from reboot_mode_notify(), which
> is only invoked by blocking_notifier_call_chain().
>
> blocking_notifier_call_chain() takes a read semaphore.
> unregister_reboot_notifier() take a write semaphore.
>
> So, if we're racing with a shutdown or reboot, I see two possible
> things:
>
> 1) blocking_notifier_call_chain() happens first and calls
> reboot_mode_notify(), blocking unregister_reboot_notifier(). Once it
> returns, the unregister proceeds and we enter case #2
>
> 2) unregister_reboot_notifier() happens first (or after the
> blocking_notifier_call_chain() returns). Our reboot object is removed
> from the list and blocking_notifier_call_chain() will not invoke
> reboot_mode_notify().
>
> In either case, the list has a single owner here.
>
>
> As far as I can see, the only race left is if multiple concurrent calls
> happens to blocking_notifier_call_chain(), the behavior of
> reboot->write() might be undefined. But I think that is reasonable.
>
>
> Please let me know if I'm missing something.
Thank you for the detailed review. Tried to summarize below:
The mutex lock was introduced in v13 following earlier discussions about
whether the issue was a theoretical future race or something that could
occur in the current implementation.
At the time (prior to v13), we concluded that while no race condition was
observable in the current code, there could be a potential in future
changes or usage patterns — making it a theoretical concern.
During review, there was further discussion around whether it's acceptable
to leave the list unprotected simply because no race is currently suspected.
The general consensus was that it's better practice to protect shared data
structures like lists with a mutex to ensure correctness and future-proofing.
Based on that feedback, the mutex lock introduced in v13.
Later in v15, the reboot-mode maintainer suggested that the patch should
include a Fixes tag, which was incorporated accordingly.
So both the mutex addition in v13 and the Fixes tag in v15 were made in
response to upstream review comments.
Need some guidance on how to take this forward or is it ok to protect all
list operation, as done in this patch and keep the fixes tag as suggested
in earlier reviews?
thanks,
Shivendra
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2025-10-29 17:49 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-15 4:38 [PATCH v16 00/14] Implement vendor resets for PSCI SYSTEM_RESET2 Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 01/14] power: reset: reboot-mode: Synchronize list traversal Shivendra Pratap
2025-10-15 14:32 ` Bartosz Golaszewski
2025-10-17 14:47 ` Shivendra Pratap
2025-10-28 3:34 ` Bjorn Andersson
2025-10-29 17:48 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration Shivendra Pratap
2025-10-15 14:40 ` Bartosz Golaszewski
2025-10-16 17:19 ` Shivendra Pratap
2025-10-17 9:06 ` Bartosz Golaszewski
2025-10-17 14:24 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 03/14] power: reset: reboot-mode: Add support for 64 bit magic Shivendra Pratap
2025-10-15 8:40 ` Nirmesh Kumar Singh
2025-10-15 4:38 ` [PATCH v16 04/14] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 05/14] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
2025-10-15 8:45 ` Nirmesh Kumar Singh
2025-10-15 14:47 ` Bartosz Golaszewski
2025-10-17 19:40 ` Shivendra Pratap
2025-10-20 7:40 ` Bartosz Golaszewski
2025-10-22 14:21 ` Shivendra Pratap
2025-10-23 15:02 ` Bartosz Golaszewski
2025-10-23 15:54 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 06/14] dt-bindings: arm: Document reboot mode magic Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 07/14] firmware: psci: Implement vendor-specific resets as reboot-mode Shivendra Pratap
2025-10-15 6:55 ` Pavan Kondeti
2025-10-15 7:47 ` Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 08/14] arm64: dts: qcom: qcm6490-idp: Add PSCI SYSTEM_RESET2 types Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 09/14] arm64: dts: qcom: qcs6490-rb3gen2: " Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 10/14] arm64: dts: qcom: lemans-ride: " Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 11/14] arm64: dts: qcom: lemans-evk: " Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 12/14] arm64: dts: qcom: qcs8300-ride: " Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 13/14] arm64: dts: qcom: monaco-evk: " Shivendra Pratap
2025-10-15 4:38 ` [PATCH v16 14/14] arm64: dts: qcom: qcs615-ride: " Shivendra Pratap
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).