[PATCH v19 0/2] reboot-mode: Expose sysfs for registered reboot modes

[PATCH v19 0/2] reboot-mode: Expose sysfs for registered reboot modes

[Shivendra Pratap]

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 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

---
---
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                  | 86 ++++++++++++++++++++++
 include/linux/reboot-mode.h                        |  3 +
 3 files changed, 125 insertions(+)
---
base-commit: 0f2995693867bfb26197b117cd55624ddc57582f
change-id: 20251116-next-15nov_expose_sysfs-c0dbcf0d59da

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

[PATCH v19 1/2] Documentation: ABI: Add sysfs-class-reboot-mode-reboot_modes

[Shivendra Pratap]

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   | 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..a50f9433942b8cab4cb3d699e9c0955299ec48f0
--- /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:		November 2025
+KernelVersion:	6.18.0-rc5
+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

[PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes

[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 | 86 +++++++++++++++++++++++++++++++++++++++
 include/linux/reboot-mode.h       |  3 ++
 2 files changed, 89 insertions(+)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index fba53f638da04655e756b5f8b7d2d666d1379535..71cd97ffb3ee8da530514ab918bc6d58a122b4ce 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -13,12 +13,61 @@
 
 #define PREFIX "mode-"
 
+static bool reboot_mode_class_registered;
+
 struct mode_info {
 	const char *mode;
 	u32 magic;
 	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;
+
+	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)
+{
+	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);
+	if (!device_register(&reboot->reboot_mode_device))
+		reboot->reboot_mode_device_registered = true;
+	else
+		reboot->reboot_mode_device_registered = false;
+}
+
 static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
 					  const char *cmd)
 {
@@ -78,6 +127,9 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
 
 	INIT_LIST_HEAD(&reboot->head);
 
+	if (reboot_mode_class_registered)
+		reboot_mode_register_device(reboot);
+
 	for_each_property_of_node(np, prop) {
 		if (strncmp(prop->name, PREFIX, len))
 			continue;
@@ -119,6 +171,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
 	list_for_each_entry(info, &reboot->head, list)
 		kfree_const(info->mode);
 
+	if (reboot->reboot_mode_device_registered) {
+		device_unregister(&reboot->reboot_mode_device);
+		reboot->reboot_mode_device_registered = false;
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(reboot_mode_register);
@@ -136,6 +193,11 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
 	list_for_each_entry(info, &reboot->head, list)
 		kfree_const(info->mode);
 
+	if (reboot->reboot_mode_device_registered) {
+		device_unregister(&reboot->reboot_mode_device);
+		reboot->reboot_mode_device_registered = false;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(reboot_mode_unregister);
@@ -162,6 +224,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 +262,29 @@ void devm_reboot_mode_unregister(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_reboot_mode_unregister);
 
+static int __init reboot_mode_init(void)
+{
+	if (class_register(&reboot_mode_class))
+		reboot_mode_class_registered = false;
+	else
+		reboot_mode_class_registered = true;
+
+	return 0;
+}
+
+static void __exit reboot_mode_exit(void)
+{
+	if (reboot_mode_class_registered)
+		class_unregister(&reboot_mode_class);
+}
+
+#ifdef MODULE
+module_init(reboot_mode_init);
+module_exit(reboot_mode_exit);
+#else
+subsys_initcall(reboot_mode_init);
+#endif
+
 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..400cfde0e029aef14ff90a11b9d12d0c3ce8dee6 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -5,6 +5,9 @@
 struct reboot_mode_driver {
 	struct device *dev;
 	struct list_head head;
+	const char *driver_name;
+	struct device reboot_mode_device;
+	bool reboot_mode_device_registered;
 	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
 	struct notifier_block reboot_notifier;
 };

-- 
2.34.1

Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes

[Bartosz Golaszewski]

On Sat, 22 Nov 2025 19:05:45 +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 | 86 +++++++++++++++++++++++++++++++++++++++
>  include/linux/reboot-mode.h       |  3 ++
>  2 files changed, 89 insertions(+)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index fba53f638da04655e756b5f8b7d2d666d1379535..71cd97ffb3ee8da530514ab918bc6d58a122b4ce 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -13,12 +13,61 @@
>
>  #define PREFIX "mode-"
>
> +static bool reboot_mode_class_registered;

You don't need this, please see below.

> +
>  struct mode_info {
>  	const char *mode;
>  	u32 magic;
>  	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;
> +
> +	list_for_each_entry(info, &reboot->head, list)
> +		size += sysfs_emit_at(buf, size, "%s ", info->mode);
> +

Don't you need locking here? What if we'll end up adding a new reboot mode at
the same time?

> +	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)
> +{
> +	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);
> +	if (!device_register(&reboot->reboot_mode_device))
> +		reboot->reboot_mode_device_registered = true;
> +	else
> +		reboot->reboot_mode_device_registered = false;

Just use device_create(). I would also suggest creating a private structure
that embeds the pointer to the struct device created by device_create() and
the pointer to the reboot_mode_driver. If you pass it as driver data to
device_create(), you'll be able to retrieve it with dev_get_drvdata() in
sysfs callbacks.

> +}
> +
>  static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>  					  const char *cmd)
>  {
> @@ -78,6 +127,9 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>
>  	INIT_LIST_HEAD(&reboot->head);
>
> +	if (reboot_mode_class_registered)
> +		reboot_mode_register_device(reboot);
> +
>  	for_each_property_of_node(np, prop) {
>  		if (strncmp(prop->name, PREFIX, len))
>  			continue;
> @@ -119,6 +171,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>  	list_for_each_entry(info, &reboot->head, list)
>  		kfree_const(info->mode);
>
> +	if (reboot->reboot_mode_device_registered) {
> +		device_unregister(&reboot->reboot_mode_device);
> +		reboot->reboot_mode_device_registered = false;
> +	}
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(reboot_mode_register);
> @@ -136,6 +193,11 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>  	list_for_each_entry(info, &reboot->head, list)
>  		kfree_const(info->mode);
>
> +	if (reboot->reboot_mode_device_registered) {
> +		device_unregister(&reboot->reboot_mode_device);
> +		reboot->reboot_mode_device_registered = false;
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(reboot_mode_unregister);
> @@ -162,6 +224,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 +262,29 @@ void devm_reboot_mode_unregister(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(devm_reboot_mode_unregister);
>
> +static int __init reboot_mode_init(void)
> +{
> +	if (class_register(&reboot_mode_class))
> +		reboot_mode_class_registered = false;
> +	else
> +		reboot_mode_class_registered = true;
> +
> +	return 0;
> +}
> +
> +static void __exit reboot_mode_exit(void)
> +{
> +	if (reboot_mode_class_registered)
> +		class_unregister(&reboot_mode_class);

Please take a moment to read the code of the functions you're calling. The
class_unregister() function will check if the class you're passing as argument
was previously registered. If not, it will do nothing. You don't need all this
checking.

> +}
> +
> +#ifdef MODULE
> +module_init(reboot_mode_init);
> +module_exit(reboot_mode_exit);
> +#else
> +subsys_initcall(reboot_mode_init);
> +#endif

You don't need to do this. It's perfectly fine to just do:

subsys_initcall(reboot_mode_init);
module_exit(reboot_mode_exit);

This is handled in the module.h header[1].

> +
>  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..400cfde0e029aef14ff90a11b9d12d0c3ce8dee6 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -5,6 +5,9 @@
>  struct reboot_mode_driver {
>  	struct device *dev;
>  	struct list_head head;
> +	const char *driver_name;
> +	struct device reboot_mode_device;
> +	bool reboot_mode_device_registered;

No need for this, just use device_is_registered() wherever you need to check
it.

>  	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
>  	struct notifier_block reboot_notifier;
>  };
>
> --
> 2.34.1
>
>
>

Thanks,
Bart

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/module.h#n103

Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes

[Shivendra Pratap]

On 11/25/2025 8:14 PM, Bartosz Golaszewski wrote:
> On Sat, 22 Nov 2025 19:05:45 +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 | 86 +++++++++++++++++++++++++++++++++++++++
>>  include/linux/reboot-mode.h       |  3 ++
>>  2 files changed, 89 insertions(+)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index fba53f638da04655e756b5f8b7d2d666d1379535..71cd97ffb3ee8da530514ab918bc6d58a122b4ce 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -13,12 +13,61 @@
>>
>>  #define PREFIX "mode-"
>>
>> +static bool reboot_mode_class_registered;
> 
> You don't need this, please see below.

reboot_mode_class_registered was used for two reason.
one is resolved: will directly call class_unregister.

for second : If class_register fails, we want don't call register device
in reboot_mode_register.

at -
	if (reboot_mode_class_registered)
		reboot_mode_register_device(reboot);
 

> 
>> +
>>  struct mode_info {
>>  	const char *mode;
>>  	u32 magic;
>>  	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;
>> +
>> +	list_for_each_entry(info, &reboot->head, list)
>> +		size += sysfs_emit_at(buf, size, "%s ", info->mode);
>> +
> 
> Don't you need locking here? What if we'll end up adding a new reboot mode at
> the same time?

reboot-modes are added only once the time of reboot_mode_register_device. Yes,
there can be a race here if someone read while module load or probe was in progress.
Sure we can add a lock here to protect against the addition in reboot_mode_register_device.

> 
>> +	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)
>> +{
>> +	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);
>> +	if (!device_register(&reboot->reboot_mode_device))
>> +		reboot->reboot_mode_device_registered = true;
>> +	else
>> +		reboot->reboot_mode_device_registered = false;
> 
> Just use device_create(). I would also suggest creating a private structure
> that embeds the pointer to the struct device created by device_create() and
> the pointer to the reboot_mode_driver. If you pass it as driver data to
> device_create(), you'll be able to retrieve it with dev_get_drvdata() in
> sysfs callbacks.

Had made change to use device_create and dev_get_drvdata in below change, and have then
changed it to above as per the reviews on the same.
https://lore.kernel.org/all/qhlxxfsyc42xemerhi36myvil3bf45isgmpugkuqzsvgcc3ifn@njrtwuooij2q/

Should we change to device_create?

> 
>> +}
>> +
>>  static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>>  					  const char *cmd)
>>  {
>> @@ -78,6 +127,9 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>
>>  	INIT_LIST_HEAD(&reboot->head);
>>
>> +	if (reboot_mode_class_registered)
>> +		reboot_mode_register_device(reboot);
>> +
>>  	for_each_property_of_node(np, prop) {
>>  		if (strncmp(prop->name, PREFIX, len))
>>  			continue;
>> @@ -119,6 +171,11 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>>  	list_for_each_entry(info, &reboot->head, list)
>>  		kfree_const(info->mode);
>>
>> +	if (reboot->reboot_mode_device_registered) {
>> +		device_unregister(&reboot->reboot_mode_device);
>> +		reboot->reboot_mode_device_registered = false;
>> +	}
>> +
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(reboot_mode_register);
>> @@ -136,6 +193,11 @@ int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>>  	list_for_each_entry(info, &reboot->head, list)
>>  		kfree_const(info->mode);
>>
>> +	if (reboot->reboot_mode_device_registered) {
>> +		device_unregister(&reboot->reboot_mode_device);
>> +		reboot->reboot_mode_device_registered = false;
>> +	}
>> +
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(reboot_mode_unregister);
>> @@ -162,6 +224,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 +262,29 @@ void devm_reboot_mode_unregister(struct device *dev,
>>  }
>>  EXPORT_SYMBOL_GPL(devm_reboot_mode_unregister);
>>
>> +static int __init reboot_mode_init(void)
>> +{
>> +	if (class_register(&reboot_mode_class))
>> +		reboot_mode_class_registered = false;
>> +	else
>> +		reboot_mode_class_registered = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static void __exit reboot_mode_exit(void)
>> +{
>> +	if (reboot_mode_class_registered)
>> +		class_unregister(&reboot_mode_class);
> 
> Please take a moment to read the code of the functions you're calling. The
> class_unregister() function will check if the class you're passing as argument
> was previously registered. If not, it will do nothing. You don't need all this
> checking.

Ack. thanks.

> 
>> +}
>> +
>> +#ifdef MODULE
>> +module_init(reboot_mode_init);
>> +module_exit(reboot_mode_exit);
>> +#else
>> +subsys_initcall(reboot_mode_init);
>> +#endif
> 
> You don't need to do this. It's perfectly fine to just do:
> 
> subsys_initcall(reboot_mode_init);
> module_exit(reboot_mode_exit);
> 
> This is handled in the module.h header[1].

Ack. sure. Will remove it.

> 
>> +
>>  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..400cfde0e029aef14ff90a11b9d12d0c3ce8dee6 100644
>> --- a/include/linux/reboot-mode.h
>> +++ b/include/linux/reboot-mode.h
>> @@ -5,6 +5,9 @@
>>  struct reboot_mode_driver {
>>  	struct device *dev;
>>  	struct list_head head;
>> +	const char *driver_name;
>> +	struct device reboot_mode_device;
>> +	bool reboot_mode_device_registered;
> 
> No need for this, just use device_is_registered() wherever you need to check
> it.

Ack.

thanks,
Shivendra

Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes

[Bartosz Golaszewski]

On Wed, Nov 26, 2025 at 5:48 PM Shivendra Pratap
<shivendra.pratap@oss.qualcomm.com> wrote:
>
> >>
> >> +static bool reboot_mode_class_registered;
> >
> > You don't need this, please see below.
>
> reboot_mode_class_registered was used for two reason.
> one is resolved: will directly call class_unregister.
>
> for second : If class_register fails, we want don't call register device
> in reboot_mode_register.
>
> at -
>         if (reboot_mode_class_registered)
>                 reboot_mode_register_device(reboot);
>

I'd just error out of the initcall if registering the class fails.
It's very unlikely anyway and points to a bigger problem.

> >> +
> >> +static void reboot_mode_register_device(struct reboot_mode_driver *reboot)
> >> +{
> >> +    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);
> >> +    if (!device_register(&reboot->reboot_mode_device))
> >> +            reboot->reboot_mode_device_registered = true;
> >> +    else
> >> +            reboot->reboot_mode_device_registered = false;
> >
> > Just use device_create(). I would also suggest creating a private structure
> > that embeds the pointer to the struct device created by device_create() and
> > the pointer to the reboot_mode_driver. If you pass it as driver data to
> > device_create(), you'll be able to retrieve it with dev_get_drvdata() in
> > sysfs callbacks.
>
> Had made change to use device_create and dev_get_drvdata in below change, and have then
> changed it to above as per the reviews on the same.
> https://lore.kernel.org/all/qhlxxfsyc42xemerhi36myvil3bf45isgmpugkuqzsvgcc3ifn@njrtwuooij2q/
>
> Should we change to device_create?
>

Ah, I missed that part. My preference is for device_create() as IMO it
results in much more elegant code (especially if we don't end up
extending the public struct) and memory is cheap but I'll let Bjorn
decide.

Bart

Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes

[Shivendra Pratap]

On 11/26/2025 10:26 PM, Bartosz Golaszewski wrote:
> On Wed, Nov 26, 2025 at 5:48 PM Shivendra Pratap
> <shivendra.pratap@oss.qualcomm.com> wrote:
>>
>>>>
>>>> +static bool reboot_mode_class_registered;
>>>
>>> You don't need this, please see below.
>>
>> reboot_mode_class_registered was used for two reason.
>> one is resolved: will directly call class_unregister.
>>
>> for second : If class_register fails, we want don't call register device
>> in reboot_mode_register.
>>
>> at -
>>         if (reboot_mode_class_registered)
>>                 reboot_mode_register_device(reboot);
>>
> 
> I'd just error out of the initcall if registering the class fails.
> It's very unlikely anyway and points to a bigger problem.

sure. thanks. will update this.

> 
>>>> +
>>>> +static void reboot_mode_register_device(struct reboot_mode_driver *reboot)
>>>> +{
>>>> +    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);
>>>> +    if (!device_register(&reboot->reboot_mode_device))
>>>> +            reboot->reboot_mode_device_registered = true;
>>>> +    else
>>>> +            reboot->reboot_mode_device_registered = false;
>>>
>>> Just use device_create(). I would also suggest creating a private structure
>>> that embeds the pointer to the struct device created by device_create() and
>>> the pointer to the reboot_mode_driver. If you pass it as driver data to
>>> device_create(), you'll be able to retrieve it with dev_get_drvdata() in
>>> sysfs callbacks.
>>
>> Had made change to use device_create and dev_get_drvdata in below change, and have then
>> changed it to above as per the reviews on the same.
>> https://lore.kernel.org/all/qhlxxfsyc42xemerhi36myvil3bf45isgmpugkuqzsvgcc3ifn@njrtwuooij2q/
>>
>> Should we change to device_create?
>>
> 
> Ah, I missed that part. My preference is for device_create() as IMO it
> results in much more elegant code (especially if we don't end up
> extending the public struct) and memory is cheap but I'll let Bjorn
> decide.
> 
> Bart