[PATCH v16 02/14] power: reset: reboot-mode: Add device tree node-based registration

[Shivendra Pratap]

On 10/15/2025 8:10 PM, Bartosz Golaszewski wrote:
> On Wed, 15 Oct 2025 at 06:38, Shivendra Pratap
> <shivendra.pratap at 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 at 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