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 | 76 +++++++++++++++++++++++++++++++++++++--
include/linux/reboot-mode.h | 6 ++++
2 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index fba53f638da04655e756b5f8b7d2d666d1379535..ae03f2d96a84477f1e9f281bf3110911d7044a70 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -3,6 +3,8 @@
* 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>
@@ -19,6 +21,56 @@ struct mode_info {
struct list_head list;
};
+static ssize_t reboot_modes_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct reboot_mode_driver *reboot;
+ struct mode_info *info;
+ ssize_t size = 0;
+
+ reboot = container_of(dev, struct reboot_mode_driver, reboot_mode_device);
+ if (!reboot)
+ return -ENODATA;
+ scoped_guard(mutex, &reboot->reboot_mode_mutex) {
+ list_for_each_entry(info, &reboot->head, list)
+ size += sysfs_emit_at(buf, size, "%s ", 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 void reboot_mode_device_release(struct device *dev)
+{
+ /* place holder to avoid warning on device_unregister. nothing to free */
+}
+
+static void reboot_mode_register_device(struct reboot_mode_driver *reboot)
+{
+ int ret;
+
+ reboot->reboot_mode_device.class = &reboot_mode_class;
+ reboot->reboot_mode_device.release = reboot_mode_device_release;
+ dev_set_name(&reboot->reboot_mode_device, reboot->driver_name);
+ /* Check return value to avoid compiler warning */
+ ret = device_register(&reboot->reboot_mode_device);
+ if (ret)
+ pr_debug("device_register failed for %s : %d\n", reboot->driver_name, ret);
+}
+
static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
const char *cmd)
{
@@ -76,6 +128,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
size_t len = strlen(PREFIX);
int ret;
+ mutex_init(&reboot->reboot_mode_mutex);
INIT_LIST_HEAD(&reboot->head);
for_each_property_of_node(np, prop) {
@@ -112,6 +165,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
reboot->reboot_notifier.notifier_call = reboot_mode_notify;
register_reboot_notifier(&reboot->reboot_notifier);
+ reboot_mode_register_device(reboot);
return 0;
@@ -132,9 +186,13 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
struct mode_info *info;
unregister_reboot_notifier(&reboot->reboot_notifier);
+ if (device_is_registered(&reboot->reboot_mode_device))
+ device_unregister(&reboot->reboot_mode_device);
- list_for_each_entry(info, &reboot->head, list)
- kfree_const(info->mode);
+ scoped_guard(mutex, &reboot->reboot_mode_mutex) {
+ list_for_each_entry(info, &reboot->head, list)
+ kfree_const(info->mode);
+ }
return 0;
}
@@ -162,6 +220,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);
if (rc) {
devres_free(dr);
@@ -199,6 +258,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..76900c1b78003559a7b7812bad34206bb3ba5f75 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -2,9 +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;
+ const char *driver_name;
+ struct device reboot_mode_device;
+ /* protects reboot_mode list */
+ struct mutex reboot_mode_mutex;
int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
struct notifier_block reboot_notifier;
};
--
2.34.1
On Sun, Nov 30, 2025 at 7:21 PM 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 should also be documented under Documentation/ABI/.
> 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 | 76 +++++++++++++++++++++++++++++++++++++--
> include/linux/reboot-mode.h | 6 ++++
> 2 files changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index fba53f638da04655e756b5f8b7d2d666d1379535..ae03f2d96a84477f1e9f281bf3110911d7044a70 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -3,6 +3,8 @@
> * 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>
> @@ -19,6 +21,56 @@ struct mode_info {
> struct list_head list;
> };
>
> +static ssize_t reboot_modes_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct reboot_mode_driver *reboot;
This is not related to this patch but please consider proposing
renaming of this structure to struct reboot_mode_devicd because
calling it a driver is quite confusing where in reality it's a device.
> + struct mode_info *info;
> + ssize_t size = 0;
> +
> + reboot = container_of(dev, struct reboot_mode_driver, reboot_mode_device);
> + if (!reboot)
> + return -ENODATA;
container_of(p, t, m) returns the address of the structure of type t
containing the member pointed to by p known under the name m in the
type of t. It just calculates the offset between the address of m in t
and p. It's not possible for it to return NULL.
> + scoped_guard(mutex, &reboot->reboot_mode_mutex) {
> + list_for_each_entry(info, &reboot->head, list)
> + size += sysfs_emit_at(buf, size, "%s ", 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 void reboot_mode_device_release(struct device *dev)
> +{
> + /* place holder to avoid warning on device_unregister. nothing to free */
> +}
> +
> +static void reboot_mode_register_device(struct reboot_mode_driver *reboot)
> +{
> + int ret;
> +
> + reboot->reboot_mode_device.class = &reboot_mode_class;
> + reboot->reboot_mode_device.release = reboot_mode_device_release;
> + dev_set_name(&reboot->reboot_mode_device, reboot->driver_name);
> + /* Check return value to avoid compiler warning */
> + ret = device_register(&reboot->reboot_mode_device);
> + if (ret)
> + pr_debug("device_register failed for %s : %d\n", reboot->driver_name, ret);
> +}
I'm not sure if this has been addressed before but why is this even
optional? Why don't we just return -1 here if we fail to register the
device?
And just for the record: I'm still convinced that using
device_create() here will result in nicer code and allow us to contain
the associated reboot_mode_device inside this compilation unit.
If Bjorn really insists on keeping it this way though, then you need
at least a call to device_initialize() here.
> +
> static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> const char *cmd)
> {
> @@ -76,6 +128,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
> size_t len = strlen(PREFIX);
> int ret;
>
> + mutex_init(&reboot->reboot_mode_mutex);
> INIT_LIST_HEAD(&reboot->head);
>
> for_each_property_of_node(np, prop) {
> @@ -112,6 +165,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>
> reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> register_reboot_notifier(&reboot->reboot_notifier);
> + reboot_mode_register_device(reboot);
>
> return 0;
>
> @@ -132,9 +186,13 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
> struct mode_info *info;
>
> unregister_reboot_notifier(&reboot->reboot_notifier);
> + if (device_is_registered(&reboot->reboot_mode_device))
> + device_unregister(&reboot->reboot_mode_device);
If you bail out of reboot_mode_register_device(), you don't need the
above check anymore because the device could
Bart
On 12/1/2025 6:51 PM, Bartosz Golaszewski wrote:
> On Sun, Nov 30, 2025 at 7:21 PM Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>
[SNIP..]
>>
>> +static ssize_t reboot_modes_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> + struct reboot_mode_driver *reboot;
>
> This is not related to this patch but please consider proposing
> renaming of this structure to struct reboot_mode_devicd because
> calling it a driver is quite confusing where in reality it's a device.
>
>> + struct mode_info *info;
>> + ssize_t size = 0;
>> +
>> + reboot = container_of(dev, struct reboot_mode_driver, reboot_mode_device);
>> + if (!reboot)
>> + return -ENODATA;
>
> container_of(p, t, m) returns the address of the structure of type t
> containing the member pointed to by p known under the name m in the
> type of t. It just calculates the offset between the address of m in t
> and p. It's not possible for it to return NULL.
Ack. Will remove this check. thanks.
>
>> + scoped_guard(mutex, &reboot->reboot_mode_mutex) {
>> + list_for_each_entry(info, &reboot->head, list)
>> + size += sysfs_emit_at(buf, size, "%s ", 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 void reboot_mode_device_release(struct device *dev)
>> +{
>> + /* place holder to avoid warning on device_unregister. nothing to free */
>> +}
>> +
>> +static void reboot_mode_register_device(struct reboot_mode_driver *reboot)
>> +{
>> + int ret;
>> +
>> + reboot->reboot_mode_device.class = &reboot_mode_class;
>> + reboot->reboot_mode_device.release = reboot_mode_device_release;
>> + dev_set_name(&reboot->reboot_mode_device, reboot->driver_name);
>> + /* Check return value to avoid compiler warning */
>> + ret = device_register(&reboot->reboot_mode_device);
>> + if (ret)
>> + pr_debug("device_register failed for %s : %d\n", reboot->driver_name, ret);
>> +}
>
> I'm not sure if this has been addressed before but why is this even
> optional? Why don't we just return -1 here if we fail to register the
> device?
Sure. we can return -1 and then continue to register reboot-modes.
As per reviews on old thread, we wanted to continue reboot-mode
registration, even if creation of reboot_mode_device fails.
https://lore.kernel.org/all/qhlxxfsyc42xemerhi36myvil3bf45isgmpugkuqzsvgcc3ifn@njrtwuooij2q/
>
> And just for the record: I'm still convinced that using
> device_create() here will result in nicer code and allow us to contain
> the associated reboot_mode_device inside this compilation unit.
>
> If Bjorn really insists on keeping it this way though, then you need
> at least a call to device_initialize() here.
>
>> +
>> static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> const char *cmd)
>> {
>> @@ -76,6 +128,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>> size_t len = strlen(PREFIX);
>> int ret;
>>
>> + mutex_init(&reboot->reboot_mode_mutex);
>> INIT_LIST_HEAD(&reboot->head);
>>
>> for_each_property_of_node(np, prop) {
>> @@ -112,6 +165,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>
>> reboot->reboot_notifier.notifier_call = reboot_mode_notify;
>> register_reboot_notifier(&reboot->reboot_notifier);
>> + reboot_mode_register_device(reboot);
>>
>> return 0;
>>
>> @@ -132,9 +186,13 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>> struct mode_info *info;
>>
>> unregister_reboot_notifier(&reboot->reboot_notifier);
>> + if (device_is_registered(&reboot->reboot_mode_device))
>> + device_unregister(&reboot->reboot_mode_device);
>
> If you bail out of reboot_mode_register_device(), you don't need the
> above check anymore because the device could
We wanted to continue on failure, as per reviews.
thanks,
Shivendra
On Mon, Dec 1, 2025 at 6:43 PM Shivendra Pratap <shivendra.pratap@oss.qualcomm.com> wrote: > > >> @@ -132,9 +186,13 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot) > >> struct mode_info *info; > >> > >> unregister_reboot_notifier(&reboot->reboot_notifier); > >> + if (device_is_registered(&reboot->reboot_mode_device)) > >> + device_unregister(&reboot->reboot_mode_device); > > > > If you bail out of reboot_mode_register_device(), you don't need the > > above check anymore because the device could > > We wanted to continue on failure, as per reviews. > You're probably referring to this bit: -- On that note, I would argue that aborting the registration of reboot-modes, just because we failed to create the convenient "debug" interface, doesn't make sense. I think it would be better to just continue even when create_reboot_mode_device() returns an error. -- Anything in sysfs is not "debug". It if is, then it should go into debugfs instead. Trying to register a sysfs object with the same name will result in a WARN() splat from sysfs core. I'd say we should definitely return an error if sysfs registration fails and just make sure it can't by assigning a unique name. Bartosz
On 12/1/2025 11:23 PM, Bartosz Golaszewski wrote: > On Mon, Dec 1, 2025 at 6:43 PM Shivendra Pratap > <shivendra.pratap@oss.qualcomm.com> wrote: >> >>>> @@ -132,9 +186,13 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot) >>>> struct mode_info *info; >>>> >>>> unregister_reboot_notifier(&reboot->reboot_notifier); >>>> + if (device_is_registered(&reboot->reboot_mode_device)) >>>> + device_unregister(&reboot->reboot_mode_device); >>> >>> If you bail out of reboot_mode_register_device(), you don't need the >>> above check anymore because the device could >> >> We wanted to continue on failure, as per reviews. >> > > You're probably referring to this bit: > > -- > On that note, I would argue that aborting the registration of > reboot-modes, just because we failed to create the convenient "debug" > interface, doesn't make sense. I think it would be better to just > continue even when create_reboot_mode_device() returns an error. > -- yes. > > Anything in sysfs is not "debug". It if is, then it should go into > debugfs instead. > > Trying to register a sysfs object with the same name will result in a > WARN() splat from sysfs core. I'd say we should definitely return an > error if sysfs registration fails and just make sure it can't by > assigning a unique name. So, should we go ahead and make the change to fail reboot_mode_register, if device registration fails? thanks, Shivendra
© 2016 - 2026 Red Hat, Inc.