* [PATCH v22 0/2] reboot-mode: Expose sysfs for registered reboot modes
@ 2025-12-26 18:56 Shivendra Pratap
2025-12-26 18:56 ` [PATCH v22 1/2] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes Shivendra Pratap
2025-12-26 18:56 ` [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap
0 siblings, 2 replies; 7+ messages in thread
From: Shivendra Pratap @ 2025-12-26 18:56 UTC (permalink / raw)
To: Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, linux-pm, Bartosz Golaszewski,
Shivendra Pratap, Bartosz Golaszewski, Sebastian Reichel
The reboot-mode framework provides infrastructure for drivers that want
to implement a userspace reboot command interface. However, there is
currently no standardized way for userspace to discover the list of
supported commands at runtime. This series introduces a sysfs interface
in the reboot-mode framework to expose the list of supported reboot-mode
commands to userspace. This will enable userspace tools to query
available reboot modes using the sysfs interface.
Example:
cat /sys/class/reboot-mode/<driver-name>/reboot_modes
The series consists of two patches:
1. power: reset: reboot-mode: Expose sysfs for registered reboot_modes
2. Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes
These patches were previously being reviewed as part of “vendor resets
for PSCI SYSTEM_RESET2”, until v17. Following the suggestions from
Bjorn, the reboot-mode sysfs patches have been split into a separate
series here, for focused discussions and better alignment.
Previous discussion on these patches:
https://lore.kernel.org/all/20251109-arm-psci-system_reset2-vendor-reboots-v17-5-46e085bca4cc@oss.qualcomm.com/
https://lore.kernel.org/all/20251109-arm-psci-system_reset2-vendor-reboots-v17-4-46e085bca4cc@oss.qualcomm.com/
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
Changes in v22:
By Bart:
- Maintain separate struct for sysfs data which should be private to
reboot-mode.
- No need to have driver_name in reboot struct.
For Alignment:
- Add a new function to store sysfs data and create device.
- Move reboot-mode device creation call after parsing of modes in
complete.
- Free stored sysfs driver_data and device in unregister call.
- Remove mutex lock as sysfs is created after modes are stored and
device is unregistered before freeing the sysfs data.
- Link to v21: https://lore.kernel.org/r/20251222-next-15nov_expose_sysfs-v21-0-244614135fd8@oss.qualcomm.com
Changes in v21:
By Bart/ Bjorn:
- Convert “reboot_mode_device” to a pointer and use device_create
instead of register_device.
- Return an error from reboot_mode_register, if device creation fails.
By Bart:
- Use dev_get_drvdata to retrieve mode strings in reboot_modes_show.
- Add mutex lock on list_add_tail in reboot_mode_register. This will
synchronize addition of modes with sysfs access. This was omitted in
previous patch as list additions was completed before sysfs creation.
For Alignment:
- Remove: "#define pr_fmt(fmt)" – not needed.
- Remove: Functions reboot_mode_register_device and reboot_mode_device_release.
- device_create being a single call now, moved inline.
- device_create handles device_release internally.
- Directly call reboot_mode_unregister in “error” path of reboot_mode_register.
- In reboot_mode_unregister:
- Add a NULL check for "reboot_mode_device" pointer.
- Move device_unregister towards end of function call and explicitly set
reboot_mode_device as NULL.
- Link to v20: https://lore.kernel.org/r/20251130-next-15nov_expose_sysfs-v20-0-18c80f8248dd@oss.qualcomm.com
Changes in v20:
By Bart
- Use device_is_registered() instead of maintaining an explicit variable
for status of device_register.
- class_unregister already maintains an internal check; remove explicit
check on status of class_register.
- Protect the reboot_mode_list with mutex lock in reboot_modes_show.
- Remove explicit "#ifdef MODULE"; subsys_initcall is just fine.
For Alignment
- Move reboot_mode_register_device after the reboot_mode list additions
are completed in reboot_mode_register. This also avoids explicit
device_unregister in error paths and need for a lock in addition.
- reboot-mode list protected in reboot_mode_unregister against a lock
in reboot_modes_show.
- Unregister reboot_mode_device before deleting the list in
reboot_mode_unregister.
- Although, we want to continue irrespective of the status of reboot_mode
device_register, add a check and pr_debug to avoid compiler warning.
- Link to v19: https://lore.kernel.org/r/20251122-next-15nov_expose_sysfs-v19-0-4d3d578ad9ee@oss.qualcomm.com
Changes in v19:
By Bart
- Added subsys_initcall and moved class_register to initcall.
By Bjorn
- Remove example of reboot SYSCALL from ABI documentation and mention
about this being standard interface.
For Alignment
- Added module_init/module_exit, in case reboot-mode is compiled as module.
- Call class_unregister on module_exit.
- Remove mutex lock on class_register as its not needed now.
- Added a static bool reboot_mode_class_registered, to save the status of
class registration.
- Rename reboot_mode_create_device to reboot_mode_register_device.
- Removed class_register as its moved to initcall.
- Version correction for split series: Previous changed to v18.
- Corrected Typo in Bjorn's Name in last change history.
- Link to v18: https://lore.kernel.org/r/20251116-next-15nov_expose_sysfs-v1-0-3b7880e5b40e@oss.qualcomm.com
- *v18 was sent as v1 in last post.
Changes in v18:
By Bjorn
- class is made static const and moved on the stack and registered
using class_register.
- Renamed name of class variable from rb_class to reboot_mode_class –
Bart/ Bjorn
- Renamed function name to prefix reboot_mode* to better align naming
convention in reboot-mode.
- Changed reboot_mode_device as static in reboot struct and registered
using device_register.
- Used dev_groups, instead of creating the sysfs attr file manually.
- Continued the reboot-mode registration even if the sysfs creation
fails at reboot_mode_create_device.
- Used container of dev in show_reboot_modes to get the structure
pointer of reboot.
By Bart
-Synchronize class registration, as there may be race in this lazy
class_register.
-Remove inversion kind of logic and align the return path of
show_reboot_modes
Other changes
- reboot_dev is renamed to reboot_mode_device to align the naming
conventions.
- Keep a check on status of device_register with bool flag as
device_unregister should be called only if the registration was
successful.
- Add a dummy function reboot_mode_device_release to avoid warn in
driver unload path.
- Date and version change in ABI documentation.
Link to v17:
https://lore.kernel.org/all/20251109-arm-psci-system_reset2-vendor-reboots-v17-0-46e085bca4cc@oss.qualcomm.com
---
To: Sebastian Reichel <sre@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
Shivendra Pratap (2):
Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes
power: reset: reboot-mode: Expose sysfs for registered reboot_modes
.../testing/sysfs-class-reboot-mode-reboot_modes | 36 ++++++
drivers/power/reset/reboot-mode.c | 139 ++++++++++++++++++++-
include/linux/reboot-mode.h | 1 +
3 files changed, 173 insertions(+), 3 deletions(-)
---
base-commit: cc3aa43b44bdb43dfbac0fcb51c56594a11338a8
change-id: 20251116-next-15nov_expose_sysfs-c0dbcf0d59da
Best regards,
--
Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v22 1/2] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes 2025-12-26 18:56 [PATCH v22 0/2] reboot-mode: Expose sysfs for registered reboot modes Shivendra Pratap @ 2025-12-26 18:56 ` Shivendra Pratap 2025-12-26 18:56 ` [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap 1 sibling, 0 replies; 7+ messages in thread From: Shivendra Pratap @ 2025-12-26 18:56 UTC (permalink / raw) To: Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, linux-pm, Bartosz Golaszewski, Shivendra Pratap, Bartosz Golaszewski, 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: Bartosz Golaszewski <brgl@kernel.org> 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 | 36 ++++++++++++++++++++++ 1 file changed, 36 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..dc27cb4e698eebc99e72821225e8bf3bbe7cc7e6 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-reboot-mode-reboot_modes @@ -0,0 +1,36 @@ +What: /sys/class/reboot-mode/<driver>/reboot_modes +Date: December 2025 +KernelVersion: 6.19-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 standard reboot() system + call interface, with the "argument" as string to "*arg" + parameter along with LINUX_REBOOT_CMD_RESTART2. + + 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] 7+ messages in thread
* [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes 2025-12-26 18:56 [PATCH v22 0/2] reboot-mode: Expose sysfs for registered reboot modes Shivendra Pratap 2025-12-26 18:56 ` [PATCH v22 1/2] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes Shivendra Pratap @ 2025-12-26 18:56 ` Shivendra Pratap 2025-12-30 16:47 ` Dan Carpenter 2026-01-02 13:25 ` Bartosz Golaszewski 1 sibling, 2 replies; 7+ messages in thread From: Shivendra Pratap @ 2025-12-26 18:56 UTC (permalink / raw) To: Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, linux-pm, Bartosz Golaszewski, Shivendra Pratap 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. Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com> --- drivers/power/reset/reboot-mode.c | 139 +++++++++++++++++++++++++++++++++++++- include/linux/reboot-mode.h | 1 + 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c index fba53f638da04655e756b5f8b7d2d666d1379535..96d0201697a539c6d048dac021db97e4e3063366 100644 --- a/drivers/power/reset/reboot-mode.c +++ b/drivers/power/reset/reboot-mode.c @@ -6,10 +6,12 @@ #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-" @@ -19,6 +21,42 @@ struct mode_info { struct list_head list; }; +struct sysfs_data { + const char *mode; + struct list_head list; +}; + +static ssize_t reboot_modes_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct sysfs_data *sysfs_info; + struct list_head *head; + ssize_t size = 0; + + head = dev_get_drvdata(dev); + if (!head) + return -ENODATA; + + list_for_each_entry(sysfs_info, head, list) + size += sysfs_emit_at(buf, size, "%s ", sysfs_info->mode); + + if (!size) + return -ENODATA; + + return size + sysfs_emit_at(buf, size - 1, "\n"); +} +static DEVICE_ATTR_RO(reboot_modes); + +static struct attribute *reboot_mode_attrs[] = { + &dev_attr_reboot_modes.attr, + NULL, +}; +ATTRIBUTE_GROUPS(reboot_mode); + +static const struct class reboot_mode_class = { + .name = "reboot-mode", + .dev_groups = reboot_mode_groups, +}; + static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot, const char *cmd) { @@ -62,6 +100,61 @@ static int reboot_mode_notify(struct notifier_block *this, return NOTIFY_DONE; } +static int reboot_mode_create_device(struct reboot_mode_driver *reboot) +{ + struct sysfs_data *sysfs_info; + struct sysfs_data *next; + struct list_head *head; + struct mode_info *info; + int ret; + + head = kzalloc(sizeof(*head), GFP_KERNEL); + if (!head) { + ret = -ENOMEM; + goto error; + } + + INIT_LIST_HEAD(head); + + list_for_each_entry(info, &reboot->head, list) { + sysfs_info = kzalloc(sizeof(*sysfs_info), GFP_KERNEL); + if (!sysfs_info) { + ret = -ENOMEM; + goto error; + } + + sysfs_info->mode = kstrdup_const(info->mode, GFP_KERNEL); + if (!sysfs_info->mode) { + kfree(sysfs_info); + ret = -ENOMEM; + goto error; + } + + list_add_tail(&sysfs_info->list, head); + } + + reboot->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0, + (void *)head, reboot->dev->driver->name); + + if (IS_ERR(reboot->reboot_mode_device)) { + ret = PTR_ERR(reboot->reboot_mode_device); + goto error; + } + + return 0; + +error: + list_for_each_entry_safe(sysfs_info, next, head, list) { + list_del(&sysfs_info->list); + kfree_const(sysfs_info->mode); + kfree(sysfs_info); + } + + kfree(head); + reboot->reboot_mode_device = NULL; + return ret; +} + /** * reboot_mode_register - register a reboot mode driver * @reboot: reboot mode driver @@ -113,16 +206,39 @@ int reboot_mode_register(struct reboot_mode_driver *reboot) reboot->reboot_notifier.notifier_call = reboot_mode_notify; register_reboot_notifier(&reboot->reboot_notifier); + ret = reboot_mode_create_device(reboot); + if (ret) + goto error; + return 0; error: - list_for_each_entry(info, &reboot->head, list) - kfree_const(info->mode); - + reboot_mode_unregister(reboot); return ret; } EXPORT_SYMBOL_GPL(reboot_mode_register); +static inline void reboot_mode_unregister_device(struct reboot_mode_driver *reboot) +{ + struct sysfs_data *sysfs_info; + struct sysfs_data *next; + struct list_head *head; + + head = dev_get_drvdata(reboot->reboot_mode_device); + device_unregister(reboot->reboot_mode_device); + reboot->reboot_mode_device = NULL; + + if (head) { + list_for_each_entry_safe(sysfs_info, next, head, list) { + list_del(&sysfs_info->list); + kfree_const(sysfs_info->mode); + kfree(sysfs_info); + } + } + + kfree(head); +} + /** * reboot_mode_unregister - unregister a reboot mode driver * @reboot: reboot mode driver @@ -131,7 +247,11 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot) { struct mode_info *info; + if (!reboot->reboot_mode_device) + return -ENODEV; + unregister_reboot_notifier(&reboot->reboot_notifier); + reboot_mode_unregister_device(reboot); list_for_each_entry(info, &reboot->head, list) kfree_const(info->mode); @@ -199,6 +319,19 @@ void devm_reboot_mode_unregister(struct device *dev, } EXPORT_SYMBOL_GPL(devm_reboot_mode_unregister); +static int __init reboot_mode_init(void) +{ + return class_register(&reboot_mode_class); +} + +static void __exit reboot_mode_exit(void) +{ + class_unregister(&reboot_mode_class); +} + +subsys_initcall(reboot_mode_init); +module_exit(reboot_mode_exit); + MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>"); MODULE_DESCRIPTION("System reboot mode core library"); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b56783c32068096325f92445b9530d1856c4826c 100644 --- a/include/linux/reboot-mode.h +++ b/include/linux/reboot-mode.h @@ -5,6 +5,7 @@ struct reboot_mode_driver { struct device *dev; struct list_head head; + struct device *reboot_mode_device; int (*write)(struct reboot_mode_driver *reboot, unsigned int magic); struct notifier_block reboot_notifier; }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes 2025-12-26 18:56 ` [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap @ 2025-12-30 16:47 ` Dan Carpenter 2026-01-02 13:25 ` Bartosz Golaszewski 1 sibling, 0 replies; 7+ messages in thread From: Dan Carpenter @ 2025-12-30 16:47 UTC (permalink / raw) To: oe-kbuild, Shivendra Pratap, Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson Cc: lkp, oe-kbuild-all, linux-kernel, linux-arm-msm, linux-pm, Shivendra Pratap Hi Shivendra, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Shivendra-Pratap/Documentation-ABI-Add-sysfs-class-reboot-mode-reboot_modes/20251227-025914 base: cc3aa43b44bdb43dfbac0fcb51c56594a11338a8 patch link: https://lore.kernel.org/r/20251227-next-15nov_expose_sysfs-v22-2-2d153438ba19%40oss.qualcomm.com patch subject: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes config: x86_64-randconfig-161-20251227 (https://download.01.org/0day-ci/archive/20251227/202512271806.n2lycyZw-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) rustc: rustc 1.88.0 (6b00bc388 2025-06-23) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202512271806.n2lycyZw-lkp@intel.com/ smatch warnings: drivers/power/reset/reboot-mode.c:147 reboot_mode_create_device() error: we previously assumed 'head' could be null (see line 112) vim +/head +147 drivers/power/reset/reboot-mode.c e5f49083a20ae0 Shivendra Pratap 2025-12-27 103 static int reboot_mode_create_device(struct reboot_mode_driver *reboot) e5f49083a20ae0 Shivendra Pratap 2025-12-27 104 { e5f49083a20ae0 Shivendra Pratap 2025-12-27 105 struct sysfs_data *sysfs_info; e5f49083a20ae0 Shivendra Pratap 2025-12-27 106 struct sysfs_data *next; e5f49083a20ae0 Shivendra Pratap 2025-12-27 107 struct list_head *head; e5f49083a20ae0 Shivendra Pratap 2025-12-27 108 struct mode_info *info; e5f49083a20ae0 Shivendra Pratap 2025-12-27 109 int ret; e5f49083a20ae0 Shivendra Pratap 2025-12-27 110 e5f49083a20ae0 Shivendra Pratap 2025-12-27 111 head = kzalloc(sizeof(*head), GFP_KERNEL); e5f49083a20ae0 Shivendra Pratap 2025-12-27 @112 if (!head) { e5f49083a20ae0 Shivendra Pratap 2025-12-27 113 ret = -ENOMEM; e5f49083a20ae0 Shivendra Pratap 2025-12-27 114 goto error; This should just be return -ENOMEM; e5f49083a20ae0 Shivendra Pratap 2025-12-27 115 } e5f49083a20ae0 Shivendra Pratap 2025-12-27 116 e5f49083a20ae0 Shivendra Pratap 2025-12-27 117 INIT_LIST_HEAD(head); e5f49083a20ae0 Shivendra Pratap 2025-12-27 118 e5f49083a20ae0 Shivendra Pratap 2025-12-27 119 list_for_each_entry(info, &reboot->head, list) { e5f49083a20ae0 Shivendra Pratap 2025-12-27 120 sysfs_info = kzalloc(sizeof(*sysfs_info), GFP_KERNEL); e5f49083a20ae0 Shivendra Pratap 2025-12-27 121 if (!sysfs_info) { e5f49083a20ae0 Shivendra Pratap 2025-12-27 122 ret = -ENOMEM; e5f49083a20ae0 Shivendra Pratap 2025-12-27 123 goto error; e5f49083a20ae0 Shivendra Pratap 2025-12-27 124 } e5f49083a20ae0 Shivendra Pratap 2025-12-27 125 e5f49083a20ae0 Shivendra Pratap 2025-12-27 126 sysfs_info->mode = kstrdup_const(info->mode, GFP_KERNEL); e5f49083a20ae0 Shivendra Pratap 2025-12-27 127 if (!sysfs_info->mode) { e5f49083a20ae0 Shivendra Pratap 2025-12-27 128 kfree(sysfs_info); e5f49083a20ae0 Shivendra Pratap 2025-12-27 129 ret = -ENOMEM; e5f49083a20ae0 Shivendra Pratap 2025-12-27 130 goto error; e5f49083a20ae0 Shivendra Pratap 2025-12-27 131 } e5f49083a20ae0 Shivendra Pratap 2025-12-27 132 e5f49083a20ae0 Shivendra Pratap 2025-12-27 133 list_add_tail(&sysfs_info->list, head); e5f49083a20ae0 Shivendra Pratap 2025-12-27 134 } e5f49083a20ae0 Shivendra Pratap 2025-12-27 135 e5f49083a20ae0 Shivendra Pratap 2025-12-27 136 reboot->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0, e5f49083a20ae0 Shivendra Pratap 2025-12-27 137 (void *)head, reboot->dev->driver->name); e5f49083a20ae0 Shivendra Pratap 2025-12-27 138 e5f49083a20ae0 Shivendra Pratap 2025-12-27 139 if (IS_ERR(reboot->reboot_mode_device)) { e5f49083a20ae0 Shivendra Pratap 2025-12-27 140 ret = PTR_ERR(reboot->reboot_mode_device); e5f49083a20ae0 Shivendra Pratap 2025-12-27 141 goto error; e5f49083a20ae0 Shivendra Pratap 2025-12-27 142 } e5f49083a20ae0 Shivendra Pratap 2025-12-27 143 e5f49083a20ae0 Shivendra Pratap 2025-12-27 144 return 0; e5f49083a20ae0 Shivendra Pratap 2025-12-27 145 e5f49083a20ae0 Shivendra Pratap 2025-12-27 146 error: e5f49083a20ae0 Shivendra Pratap 2025-12-27 @147 list_for_each_entry_safe(sysfs_info, next, head, list) { ^^^^ But it is a crash instead. e5f49083a20ae0 Shivendra Pratap 2025-12-27 148 list_del(&sysfs_info->list); e5f49083a20ae0 Shivendra Pratap 2025-12-27 149 kfree_const(sysfs_info->mode); e5f49083a20ae0 Shivendra Pratap 2025-12-27 150 kfree(sysfs_info); e5f49083a20ae0 Shivendra Pratap 2025-12-27 151 } e5f49083a20ae0 Shivendra Pratap 2025-12-27 152 e5f49083a20ae0 Shivendra Pratap 2025-12-27 153 kfree(head); e5f49083a20ae0 Shivendra Pratap 2025-12-27 154 reboot->reboot_mode_device = NULL; e5f49083a20ae0 Shivendra Pratap 2025-12-27 155 return ret; e5f49083a20ae0 Shivendra Pratap 2025-12-27 156 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes 2025-12-26 18:56 ` [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap 2025-12-30 16:47 ` Dan Carpenter @ 2026-01-02 13:25 ` Bartosz Golaszewski 2026-01-05 17:45 ` Shivendra Pratap 1 sibling, 1 reply; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-02 13:25 UTC (permalink / raw) To: Shivendra Pratap Cc: linux-kernel, linux-arm-msm, linux-pm, Bartosz Golaszewski, Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson On Fri, 26 Dec 2025 19:56:34 +0100, Shivendra Pratap <shivendra.pratap@oss.qualcomm.com> said: > 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. > > Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com> > --- > drivers/power/reset/reboot-mode.c | 139 +++++++++++++++++++++++++++++++++++++- > include/linux/reboot-mode.h | 1 + > 2 files changed, 137 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c > index fba53f638da04655e756b5f8b7d2d666d1379535..96d0201697a539c6d048dac021db97e4e3063366 100644 > --- a/drivers/power/reset/reboot-mode.c > +++ b/drivers/power/reset/reboot-mode.c > @@ -6,10 +6,12 @@ > #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-" > > @@ -19,6 +21,42 @@ struct mode_info { > struct list_head list; > }; > > +struct sysfs_data { Let's make this more descriptive? struct reboot_mode_sysfs_data? > + const char *mode; > + struct list_head list; > +}; > + > +static ssize_t reboot_modes_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct sysfs_data *sysfs_info; > + struct list_head *head; > + ssize_t size = 0; > + > + head = dev_get_drvdata(dev); > + if (!head) > + return -ENODATA; > + > + list_for_each_entry(sysfs_info, head, list) > + size += sysfs_emit_at(buf, size, "%s ", sysfs_info->mode); > + > + if (!size) > + return -ENODATA; > + > + return size + sysfs_emit_at(buf, size - 1, "\n"); > +} > +static DEVICE_ATTR_RO(reboot_modes); > + > +static struct attribute *reboot_mode_attrs[] = { > + &dev_attr_reboot_modes.attr, > + NULL, > +}; > +ATTRIBUTE_GROUPS(reboot_mode); > + > +static const struct class reboot_mode_class = { > + .name = "reboot-mode", > + .dev_groups = reboot_mode_groups, > +}; > + > static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot, > const char *cmd) > { > @@ -62,6 +100,61 @@ static int reboot_mode_notify(struct notifier_block *this, > return NOTIFY_DONE; > } > > +static int reboot_mode_create_device(struct reboot_mode_driver *reboot) > +{ > + struct sysfs_data *sysfs_info; > + struct sysfs_data *next; > + struct list_head *head; > + struct mode_info *info; > + int ret; > + > + head = kzalloc(sizeof(*head), GFP_KERNEL); > + if (!head) { > + ret = -ENOMEM; > + goto error; > + } > + > + INIT_LIST_HEAD(head); > + > + list_for_each_entry(info, &reboot->head, list) { > + sysfs_info = kzalloc(sizeof(*sysfs_info), GFP_KERNEL); > + if (!sysfs_info) { > + ret = -ENOMEM; > + goto error; > + } > + > + sysfs_info->mode = kstrdup_const(info->mode, GFP_KERNEL); > + if (!sysfs_info->mode) { > + kfree(sysfs_info); > + ret = -ENOMEM; > + goto error; > + } > + > + list_add_tail(&sysfs_info->list, head); > + } > + > + reboot->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0, > + (void *)head, reboot->dev->driver->name); No, why pass the list? You should create an instance of struct sysfs_data per device_create(). If it needs to contain a list, then let it contain a list but don't allocate the list_head, that's really unusual. > + > + if (IS_ERR(reboot->reboot_mode_device)) { > + ret = PTR_ERR(reboot->reboot_mode_device); > + goto error; > + } > + > + return 0; > + > +error: > + list_for_each_entry_safe(sysfs_info, next, head, list) { > + list_del(&sysfs_info->list); > + kfree_const(sysfs_info->mode); > + kfree(sysfs_info); > + } > + > + kfree(head); > + reboot->reboot_mode_device = NULL; > + return ret; > +} > + > /** > * reboot_mode_register - register a reboot mode driver > * @reboot: reboot mode driver > @@ -113,16 +206,39 @@ int reboot_mode_register(struct reboot_mode_driver *reboot) > reboot->reboot_notifier.notifier_call = reboot_mode_notify; > register_reboot_notifier(&reboot->reboot_notifier); > > + ret = reboot_mode_create_device(reboot); > + if (ret) > + goto error; > + > return 0; > > error: > - list_for_each_entry(info, &reboot->head, list) > - kfree_const(info->mode); > - > + reboot_mode_unregister(reboot); > return ret; > } > EXPORT_SYMBOL_GPL(reboot_mode_register); > > +static inline void reboot_mode_unregister_device(struct reboot_mode_driver *reboot) > +{ > + struct sysfs_data *sysfs_info; > + struct sysfs_data *next; > + struct list_head *head; > + > + head = dev_get_drvdata(reboot->reboot_mode_device); > + device_unregister(reboot->reboot_mode_device); > + reboot->reboot_mode_device = NULL; > + > + if (head) { > + list_for_each_entry_safe(sysfs_info, next, head, list) { > + list_del(&sysfs_info->list); > + kfree_const(sysfs_info->mode); > + kfree(sysfs_info); > + } This loop is duplicated, can you please factor it out into a dedicated function? > + } > + > + kfree(head); > +} > + > /** > * reboot_mode_unregister - unregister a reboot mode driver > * @reboot: reboot mode driver > @@ -131,7 +247,11 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot) > { > struct mode_info *info; > > + if (!reboot->reboot_mode_device) > + return -ENODEV; > + > unregister_reboot_notifier(&reboot->reboot_notifier); > + reboot_mode_unregister_device(reboot); > > list_for_each_entry(info, &reboot->head, list) > kfree_const(info->mode); > @@ -199,6 +319,19 @@ void devm_reboot_mode_unregister(struct device *dev, > } > EXPORT_SYMBOL_GPL(devm_reboot_mode_unregister); > > +static int __init reboot_mode_init(void) > +{ > + return class_register(&reboot_mode_class); > +} > + > +static void __exit reboot_mode_exit(void) > +{ > + class_unregister(&reboot_mode_class); > +} > + > +subsys_initcall(reboot_mode_init); > +module_exit(reboot_mode_exit); > + > MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>"); > MODULE_DESCRIPTION("System reboot mode core library"); > MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h > index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b56783c32068096325f92445b9530d1856c4826c 100644 > --- a/include/linux/reboot-mode.h > +++ b/include/linux/reboot-mode.h > @@ -5,6 +5,7 @@ > struct reboot_mode_driver { > struct device *dev; > struct list_head head; > + struct device *reboot_mode_device; Why can't this be part of struct (reboot_mode_)sysfs_data? Bart > int (*write)(struct reboot_mode_driver *reboot, unsigned int magic); > struct notifier_block reboot_notifier; > }; > > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes 2026-01-02 13:25 ` Bartosz Golaszewski @ 2026-01-05 17:45 ` Shivendra Pratap 2026-01-13 14:59 ` Bartosz Golaszewski 0 siblings, 1 reply; 7+ messages in thread From: Shivendra Pratap @ 2026-01-05 17:45 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-kernel, linux-arm-msm, linux-pm, Bartosz Golaszewski, Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson On 1/2/2026 6:55 PM, Bartosz Golaszewski wrote: > On Fri, 26 Dec 2025 19:56:34 +0100, Shivendra Pratap > <shivendra.pratap@oss.qualcomm.com> said: >> 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. [SNIP..] >> >> +struct sysfs_data { > > Let's make this more descriptive? struct reboot_mode_sysfs_data? Ack. thanks. > >> + const char *mode; >> + struct list_head list; >> +}; >> + [SNIP..] >> + >> + reboot->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0, >> + (void *)head, reboot->dev->driver->name); > > No, why pass the list? You should create an instance of struct sysfs_data per > device_create(). If it needs to contain a list, then let it contain a list but > don't allocate the list_head, that's really unusual. > ok. Will create struct reboot_mode_sysfs_data with a list head and allocate it as data. >> + [SNIP..] >> >> +static inline void reboot_mode_unregister_device(struct reboot_mode_driver *reboot) >> +{ >> + struct sysfs_data *sysfs_info; >> + struct sysfs_data *next; >> + struct list_head *head; >> + >> + head = dev_get_drvdata(reboot->reboot_mode_device); >> + device_unregister(reboot->reboot_mode_device); >> + reboot->reboot_mode_device = NULL; >> + >> + if (head) { >> + list_for_each_entry_safe(sysfs_info, next, head, list) { >> + list_del(&sysfs_info->list); >> + kfree_const(sysfs_info->mode); >> + kfree(sysfs_info); >> + } > > This loop is duplicated, can you please factor it out into a dedicated > function? The loop frees the sysfs data. You mean i should directly call reboot_mode_unregister_device in error path of reboot_mode_create_device as not to duplicate the loop? > [SNIP..] >> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h >> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b56783c32068096325f92445b9530d1856c4826c 100644 >> --- a/include/linux/reboot-mode.h >> +++ b/include/linux/reboot-mode.h >> @@ -5,6 +5,7 @@ >> struct reboot_mode_driver { >> struct device *dev; >> struct list_head head; >> + struct device *reboot_mode_device; > > Why can't this be part of struct (reboot_mode_)sysfs_data? > If reboot_mode_device is kept in sysfs_data, we need a reference to free it. Should I maintain reference for it in "reboot struct" and store sysfs_data pointer, so that it can be used to call device_unregister()? Eg: struct reboot { .. .. void *priv; }; struct reboot_mode_sysfs_data { struct device *reboot_mode_device; struct list_head head; }; --- data = kzalloc(reboot_mode_sysfs_data); reboot->priv = data; -- thanks, Shivendra ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes 2026-01-05 17:45 ` Shivendra Pratap @ 2026-01-13 14:59 ` Bartosz Golaszewski 0 siblings, 0 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-13 14:59 UTC (permalink / raw) To: Shivendra Pratap Cc: linux-kernel, linux-arm-msm, linux-pm, Sebastian Reichel, Bartosz Golaszewski, Bjorn Andersson On Mon, Jan 5, 2026 at 6:45 PM Shivendra Pratap <shivendra.pratap@oss.qualcomm.com> wrote: > > > > On 1/2/2026 6:55 PM, Bartosz Golaszewski wrote: > > On Fri, 26 Dec 2025 19:56:34 +0100, Shivendra Pratap > > <shivendra.pratap@oss.qualcomm.com> said: > >> 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. > Sorry for the delayed response. > [SNIP..] > > >> > >> +struct sysfs_data { > > > > Let's make this more descriptive? struct reboot_mode_sysfs_data? > > Ack. thanks. > > > > >> + const char *mode; > >> + struct list_head list; > >> +}; > >> + > > [SNIP..] > > >> + > >> + reboot->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0, > >> + (void *)head, reboot->dev->driver->name); > > > > No, why pass the list? You should create an instance of struct sysfs_data per > > device_create(). If it needs to contain a list, then let it contain a list but > > don't allocate the list_head, that's really unusual. > > > > ok. Will create struct reboot_mode_sysfs_data with a list head and > allocate it as data. > > >> + > > [SNIP..] > > >> > >> +static inline void reboot_mode_unregister_device(struct reboot_mode_driver *reboot) > >> +{ > >> + struct sysfs_data *sysfs_info; > >> + struct sysfs_data *next; > >> + struct list_head *head; > >> + > >> + head = dev_get_drvdata(reboot->reboot_mode_device); > >> + device_unregister(reboot->reboot_mode_device); > >> + reboot->reboot_mode_device = NULL; > >> + > >> + if (head) { > >> + list_for_each_entry_safe(sysfs_info, next, head, list) { > >> + list_del(&sysfs_info->list); > >> + kfree_const(sysfs_info->mode); > >> + kfree(sysfs_info); > >> + } > > > > This loop is duplicated, can you please factor it out into a dedicated > > function? > > The loop frees the sysfs data. You mean i should directly call > reboot_mode_unregister_device in error path of reboot_mode_create_device > as not to duplicate the loop? > I was thinking about wrapping it in a dedicated function and calling it here and in the error path in reboot_mode_create_device(). > > > > [SNIP..] > > >> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h > >> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b56783c32068096325f92445b9530d1856c4826c 100644 > >> --- a/include/linux/reboot-mode.h > >> +++ b/include/linux/reboot-mode.h > >> @@ -5,6 +5,7 @@ > >> struct reboot_mode_driver { > >> struct device *dev; > >> struct list_head head; > >> + struct device *reboot_mode_device; > > > > Why can't this be part of struct (reboot_mode_)sysfs_data? > > > > If reboot_mode_device is kept in sysfs_data, we need a reference to free > it. Should I maintain reference for it in "reboot struct" and store > sysfs_data pointer, so that it can be used to call device_unregister()? > > Eg: > struct reboot > { > .. > .. > void *priv; > }; > > struct reboot_mode_sysfs_data { > struct device *reboot_mode_device; > struct list_head head; > }; > You can use class_find_device(). Store the address of the associated reboot_mode_driver in the private structure and compare by it in the match callback. Bart ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-01-13 14:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-26 18:56 [PATCH v22 0/2] reboot-mode: Expose sysfs for registered reboot modes Shivendra Pratap 2025-12-26 18:56 ` [PATCH v22 1/2] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes Shivendra Pratap 2025-12-26 18:56 ` [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes Shivendra Pratap 2025-12-30 16:47 ` Dan Carpenter 2026-01-02 13:25 ` Bartosz Golaszewski 2026-01-05 17:45 ` Shivendra Pratap 2026-01-13 14:59 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox