[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54eaf8e8-2b4d-b0bd-b65f-1262e72a54dd@oss.qualcomm.com>
Date: Sat, 19 Jul 2025 21:05:16 +0530
From: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>,
Sudeep Holla <sudeep.holla@....com>,
Souvik Chakravarty <Souvik.Chakravarty@....com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Andy Yan <andy.yan@...k-chips.com>,
Mark Rutland <mark.rutland@....com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Arnd Bergmann <arnd@...db.de>, Konrad Dybcio <konradybcio@...nel.org>,
cros-qcom-dts-watchers@...omium.org, Vinod Koul <vkoul@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Mukesh Ojha <mukesh.ojha@....qualcomm.com>,
Stephen Boyd <swboyd@...omium.org>,
Andre Draszik
<andre.draszik@...aro.org>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
Elliot Berman <quic_eberman@...cinc.com>,
Srinivas Kandagatla <srini@...nel.org>
Subject: Re: [PATCH v11 1/8] power: reset: reboot-mode: Add device tree
node-based registration
On 7/19/2025 12:07 AM, Dmitry Baryshkov wrote:
> On Thu, Jul 17, 2025 at 06:16:47PM +0530, Shivendra Pratap 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
>> Device Tree (DT) node-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@....qualcomm.com>
>> ---
>> drivers/power/reset/reboot-mode.c | 29 +++++++++++++++++++----------
>> include/linux/reboot-mode.h | 2 +-
>> 2 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index fba53f638da04655e756b5f8b7d2d666d1379535..0269ec55106472cf2f2b12bd65704dd0114bf4a3 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-"
>>
>> @@ -65,33 +69,35 @@ static int reboot_mode_notify(struct notifier_block *this,
>> /**
>> * reboot_mode_register - register a reboot mode driver
>> * @reboot: reboot mode driver
>> + * @np: Pointer to device tree node
>> *
>> * 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 device_node *np)
>> {
>> struct mode_info *info;
>> struct property *prop;
>> - struct device_node *np = reboot->dev->of_node;
>> size_t len = strlen(PREFIX);
>> int ret;
>>
>> + if (!np)
>> + return -EINVAL;
>> +
>> INIT_LIST_HEAD(&reboot->head);
>>
>> for_each_property_of_node(np, prop) {
>> if (strncmp(prop->name, PREFIX, len))
>> continue;
>>
>> - info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
>> + 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;
>> }
>>
>> @@ -102,8 +108,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot)
>> } else if (info->mode[0] == '\0') {
>> kfree_const(info->mode);
>> ret = -EINVAL;
>> - dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
>> - prop->name);
>> + pr_err("invalid mode name(%s): too short!\n", prop->name);
>> goto error;
>> }
>>
>> @@ -130,11 +135,15 @@ EXPORT_SYMBOL_GPL(reboot_mode_register);
>> int reboot_mode_unregister(struct reboot_mode_driver *reboot)
>> {
>> struct mode_info *info;
>> + struct mode_info *next;
>>
>> unregister_reboot_notifier(&reboot->reboot_notifier);
>>
>> - list_for_each_entry(info, &reboot->head, list)
>> + list_for_each_entry_safe(info, next, &reboot->head, list) {
>
> This feels liks a missing lock.
Should we add a lock here? The list will mostly be fully created only
once at the time of registration.
- thanks.
>
>> kfree_const(info->mode);
>> + list_del(&info->list);
>
> list_del should come before kfree, otherwise it's possible to access
> freed memory while traversing the list.
sure. will make it list_del(&info->list) and then kfree_const(info->mode).
- thanks.
>
>> + kfree(info);
>> + }
>>
>> return 0;
>> }
>> @@ -162,7 +171,7 @@ int devm_reboot_mode_register(struct device *dev,
>> if (!dr)
>> return -ENOMEM;
>>
>> - rc = reboot_mode_register(reboot);
>> + rc = reboot_mode_register(reboot, reboot->dev->of_node);
>> if (rc) {
>> devres_free(dr);
>> return rc;
>> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
>> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..36f071f4b82e1fc255d8dd679a18e537655c3179 100644
>> --- a/include/linux/reboot-mode.h
>> +++ b/include/linux/reboot-mode.h
>> @@ -9,7 +9,7 @@ struct reboot_mode_driver {
>> struct notifier_block reboot_notifier;
>> };
>>
>> -int reboot_mode_register(struct reboot_mode_driver *reboot);
>> +int reboot_mode_register(struct reboot_mode_driver *reboot, struct device_node *np);
>> int reboot_mode_unregister(struct reboot_mode_driver *reboot);
>> int devm_reboot_mode_register(struct device *dev,
>> struct reboot_mode_driver *reboot);
>>
>> --
>> 2.34.1
>>
>
Powered by blists - more mailing lists