* [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
@ 2026-05-05 14:44 Daniel Lezcano
2026-05-07 8:29 ` Daniel Lezcano
2026-05-07 10:02 ` Lukasz Luba
0 siblings, 2 replies; 8+ messages in thread
From: Daniel Lezcano @ 2026-05-05 14:44 UTC (permalink / raw)
To: rafael; +Cc: rui.zhang, lukasz.luba, daniel.lezcano, linux-pm, linux-kernel
In preparation for the upcoming changes separating OF and non-OF code,
split __thermal_cooling_device_register() into allocation and addition
phases.
This allows moving the device node assignment out of the core
initialization path.
This change is not a trivial split. The lifetime of the cooling device
is managed by the device core through put_device(), which triggers
thermal_release() to free all associated resources.
With the introduction of thermal_cooling_device_alloc(), the allocation
path must mirror what thermal_release() undoes. In contrast,
thermal_cooling_device_add() must not perform any rollback and relies
on put_device() for cleanup on error paths. This avoids both double
free and resource leaks.
As part of this rework, add the missing device_initialize() call when
allocating the cooling device.
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
drivers/thermal/thermal_core.c | 117 ++++++++++++++++++++-------------
1 file changed, 73 insertions(+), 44 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 664a4cc95199..fd4c61197d1e 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1040,29 +1040,10 @@ static void thermal_cooling_device_init_complete(struct thermal_cooling_device *
thermal_zone_cdev_bind(tz, cdev);
}
-/**
- * __thermal_cooling_device_register() - register a new thermal cooling device
- * @np: a pointer to a device tree node.
- * @type: the thermal cooling device type.
- * @devdata: device private data.
- * @ops: standard thermal cooling devices callbacks.
- *
- * This interface function adds a new thermal cooling device (fan/processor/...)
- * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
- * to all the thermal zone devices registered at the same time.
- * It also gives the opportunity to link the cooling device to a device tree
- * node, so that it can be bound to a thermal zone created out of device tree.
- *
- * Return: a pointer to the created struct thermal_cooling_device or an
- * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
- */
static struct thermal_cooling_device *
-__thermal_cooling_device_register(struct device_node *np,
- const char *type, void *devdata,
- const struct thermal_cooling_device_ops *ops)
+thermal_cooling_device_alloc(const char *type, const struct thermal_cooling_device_ops *ops)
{
struct thermal_cooling_device *cdev;
- unsigned long current_state;
int ret;
if (!ops || !ops->get_max_state || !ops->get_cur_state ||
@@ -1076,6 +1057,8 @@ __thermal_cooling_device_register(struct device_node *np,
if (!cdev)
return ERR_PTR(-ENOMEM);
+ cdev->ops = ops;
+
ret = ida_alloc(&thermal_cdev_ida, GFP_KERNEL);
if (ret < 0)
goto out_kfree_cdev;
@@ -1087,17 +1070,36 @@ __thermal_cooling_device_register(struct device_node *np,
goto out_ida_remove;
}
+ return cdev;
+
+out_ida_remove:
+ ida_free(&thermal_cdev_ida, cdev->id);
+out_kfree_cdev:
+ kfree(cdev);
+ return ERR_PTR(ret);
+}
+
+static int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdata)
+{
+ unsigned long current_state;
+ int ret;
+
mutex_init(&cdev->lock);
INIT_LIST_HEAD(&cdev->thermal_instances);
- cdev->np = np;
- cdev->ops = ops;
cdev->updated = false;
cdev->device.class = &thermal_class;
+ device_initialize(&cdev->device);
cdev->devdata = devdata;
+ thermal_cooling_device_setup_sysfs(cdev);
+
+ ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
+ if (ret)
+ goto out_put_device;
+
ret = cdev->ops->get_max_state(cdev, &cdev->max_state);
if (ret)
- goto out_cdev_type;
+ goto out_put_device;
/*
* The cooling device's current state is only needed for debug
@@ -1111,35 +1113,62 @@ __thermal_cooling_device_register(struct device_node *np,
if (ret)
current_state = ULONG_MAX;
- thermal_cooling_device_setup_sysfs(cdev);
-
- ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
- if (ret)
- goto out_cooling_dev;
-
ret = device_register(&cdev->device);
- if (ret) {
- /* thermal_release() handles rest of the cleanup */
- put_device(&cdev->device);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto out_put_device;
if (current_state <= cdev->max_state)
thermal_debug_cdev_add(cdev, current_state);
thermal_cooling_device_init_complete(cdev);
- return cdev;
+ return 0;
-out_cooling_dev:
- thermal_cooling_device_destroy_sysfs(cdev);
-out_cdev_type:
- kfree_const(cdev->type);
-out_ida_remove:
- ida_free(&thermal_cdev_ida, cdev->id);
-out_kfree_cdev:
- kfree(cdev);
- return ERR_PTR(ret);
+out_put_device:
+ /*
+ * The device core will release the memory via
+ * thermal_release() after put_device() is called in the error
+ * path
+ */
+ put_device(&cdev->device);
+ return ret;
+}
+
+/**
+ * __thermal_cooling_device_register() - register a new thermal cooling device
+ * @np: a pointer to a device tree node.
+ * @type: the thermal cooling device type.
+ * @devdata: device private data.
+ * @ops: standard thermal cooling devices callbacks.
+ *
+ * This interface function adds a new thermal cooling device (fan/processor/...)
+ * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
+ * to all the thermal zone devices registered at the same time.
+ * It also gives the opportunity to link the cooling device to a device tree
+ * node, so that it can be bound to a thermal zone created out of device tree.
+ *
+ * Return: a pointer to the created struct thermal_cooling_device or an
+ * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
+ */
+static struct thermal_cooling_device *
+__thermal_cooling_device_register(struct device_node *np,
+ const char *type, void *devdata,
+ const struct thermal_cooling_device_ops *ops)
+{
+ struct thermal_cooling_device *cdev;
+ int ret;
+
+ cdev = thermal_cooling_device_alloc(type, ops);
+ if (IS_ERR(cdev))
+ return cdev;
+
+ cdev->np = np;
+
+ ret = thermal_cooling_device_add(cdev, devdata);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return cdev;
}
/**
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-05 14:44 [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions Daniel Lezcano
@ 2026-05-07 8:29 ` Daniel Lezcano
2026-05-07 10:02 ` Lukasz Luba
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Lezcano @ 2026-05-07 8:29 UTC (permalink / raw)
To: rafael; +Cc: rui.zhang, lukasz.luba, linux-pm, linux-kernel
Hi Rafael,
On 5/5/26 16:44, Daniel Lezcano wrote:
> In preparation for the upcoming changes separating OF and non-OF code,
> split __thermal_cooling_device_register() into allocation and addition
> phases.
>
> This allows moving the device node assignment out of the core
> initialization path.
>
> This change is not a trivial split. The lifetime of the cooling device
> is managed by the device core through put_device(), which triggers
> thermal_release() to free all associated resources.
>
> With the introduction of thermal_cooling_device_alloc(), the allocation
> path must mirror what thermal_release() undoes. In contrast,
> thermal_cooling_device_add() must not perform any rollback and relies
> on put_device() for cleanup on error paths. This avoids both double
> free and resource leaks.
>
> As part of this rework, add the missing device_initialize() call when
> allocating the cooling device.
>
> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> ---
Are you fine with this change ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-05 14:44 [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions Daniel Lezcano
2026-05-07 8:29 ` Daniel Lezcano
@ 2026-05-07 10:02 ` Lukasz Luba
2026-05-07 18:26 ` Daniel Lezcano
1 sibling, 1 reply; 8+ messages in thread
From: Lukasz Luba @ 2026-05-07 10:02 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: rui.zhang, linux-pm, linux-kernel, rafael
Hi Daniel,
On 5/5/26 15:44, Daniel Lezcano wrote:
> In preparation for the upcoming changes separating OF and non-OF code,
> split __thermal_cooling_device_register() into allocation and addition
> phases.
>
> This allows moving the device node assignment out of the core
> initialization path.
>
> This change is not a trivial split. The lifetime of the cooling device
> is managed by the device core through put_device(), which triggers
> thermal_release() to free all associated resources.
>
> With the introduction of thermal_cooling_device_alloc(), the allocation
> path must mirror what thermal_release() undoes. In contrast,
> thermal_cooling_device_add() must not perform any rollback and relies
> on put_device() for cleanup on error paths. This avoids both double
> free and resource leaks.
>
> As part of this rework, add the missing device_initialize() call when
> allocating the cooling device.
>
> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> ---
> drivers/thermal/thermal_core.c | 117 ++++++++++++++++++++-------------
> 1 file changed, 73 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 664a4cc95199..fd4c61197d1e 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1040,29 +1040,10 @@ static void thermal_cooling_device_init_complete(struct thermal_cooling_device *
> thermal_zone_cdev_bind(tz, cdev);
> }
>
> -/**
> - * __thermal_cooling_device_register() - register a new thermal cooling device
> - * @np: a pointer to a device tree node.
> - * @type: the thermal cooling device type.
> - * @devdata: device private data.
> - * @ops: standard thermal cooling devices callbacks.
> - *
> - * This interface function adds a new thermal cooling device (fan/processor/...)
> - * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
> - * to all the thermal zone devices registered at the same time.
> - * It also gives the opportunity to link the cooling device to a device tree
> - * node, so that it can be bound to a thermal zone created out of device tree.
> - *
> - * Return: a pointer to the created struct thermal_cooling_device or an
> - * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
> - */
> static struct thermal_cooling_device *
> -__thermal_cooling_device_register(struct device_node *np,
> - const char *type, void *devdata,
> - const struct thermal_cooling_device_ops *ops)
> +thermal_cooling_device_alloc(const char *type, const struct thermal_cooling_device_ops *ops)
> {
> struct thermal_cooling_device *cdev;
> - unsigned long current_state;
> int ret;
>
> if (!ops || !ops->get_max_state || !ops->get_cur_state ||
> @@ -1076,6 +1057,8 @@ __thermal_cooling_device_register(struct device_node *np,
> if (!cdev)
> return ERR_PTR(-ENOMEM);
>
> + cdev->ops = ops;
> +
> ret = ida_alloc(&thermal_cdev_ida, GFP_KERNEL);
> if (ret < 0)
> goto out_kfree_cdev;
> @@ -1087,17 +1070,36 @@ __thermal_cooling_device_register(struct device_node *np,
> goto out_ida_remove;
> }
>
> + return cdev;
> +
> +out_ida_remove:
> + ida_free(&thermal_cdev_ida, cdev->id);
> +out_kfree_cdev:
> + kfree(cdev);
> + return ERR_PTR(ret);
> +}
> +
> +static int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdata)
> +{
> + unsigned long current_state;
> + int ret;
> +
> mutex_init(&cdev->lock);
> INIT_LIST_HEAD(&cdev->thermal_instances);
> - cdev->np = np;
> - cdev->ops = ops;
> cdev->updated = false;
> cdev->device.class = &thermal_class;
> + device_initialize(&cdev->device);
> cdev->devdata = devdata;
>
> + thermal_cooling_device_setup_sysfs(cdev);
> +
> + ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
> + if (ret)
In case of error here, when the cdev->device won't have this name,
would thermal_release() still be able to call the cleanup
for the sysfs bits?
IMHO the check 'if()' there might bite us and we might not free
the sysfs allocated memory.
> + goto out_put_device;
> +
> ret = cdev->ops->get_max_state(cdev, &cdev->max_state);
> if (ret)
> - goto out_cdev_type;
> + goto out_put_device;
>
> /*
> * The cooling device's current state is only needed for debug
> @@ -1111,35 +1113,62 @@ __thermal_cooling_device_register(struct device_node *np,
> if (ret)
> current_state = ULONG_MAX;
>
> - thermal_cooling_device_setup_sysfs(cdev);
> -
> - ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
> - if (ret)
In this current code case, we explicitly call the
thermal_cooling_device_destroy_sysfs() which will definitely clean-up
all stuff.
> - goto out_cooling_dev;
> -
> ret = device_register(&cdev->device);
> - if (ret) {
> - /* thermal_release() handles rest of the cleanup */
> - put_device(&cdev->device);
> - return ERR_PTR(ret);
> - }
> + if (ret)
> + goto out_put_device;
>
> if (current_state <= cdev->max_state)
> thermal_debug_cdev_add(cdev, current_state);
>
> thermal_cooling_device_init_complete(cdev);
>
> - return cdev;
> + return 0;
>
> -out_cooling_dev:
> - thermal_cooling_device_destroy_sysfs(cdev);
> -out_cdev_type:
> - kfree_const(cdev->type);
> -out_ida_remove:
> - ida_free(&thermal_cdev_ida, cdev->id);
> -out_kfree_cdev:
> - kfree(cdev);
> - return ERR_PTR(ret);
> +out_put_device:
> + /*
> + * The device core will release the memory via
> + * thermal_release() after put_device() is called in the error
> + * path
> + */
> + put_device(&cdev->device);
> + return ret;
> +}
> +
> +/**
> + * __thermal_cooling_device_register() - register a new thermal cooling device
> + * @np: a pointer to a device tree node.
> + * @type: the thermal cooling device type.
> + * @devdata: device private data.
> + * @ops: standard thermal cooling devices callbacks.
> + *
> + * This interface function adds a new thermal cooling device (fan/processor/...)
> + * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
> + * to all the thermal zone devices registered at the same time.
> + * It also gives the opportunity to link the cooling device to a device tree
> + * node, so that it can be bound to a thermal zone created out of device tree.
> + *
> + * Return: a pointer to the created struct thermal_cooling_device or an
> + * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
> + */
> +static struct thermal_cooling_device *
> +__thermal_cooling_device_register(struct device_node *np,
> + const char *type, void *devdata,
> + const struct thermal_cooling_device_ops *ops)
> +{
> + struct thermal_cooling_device *cdev;
> + int ret;
> +
> + cdev = thermal_cooling_device_alloc(type, ops);
> + if (IS_ERR(cdev))
> + return cdev;
> +
> + cdev->np = np;
> +
> + ret = thermal_cooling_device_add(cdev, devdata);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return cdev;
> }
>
> /**
The rest looks OK
Regards,
Lukasz
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-07 10:02 ` Lukasz Luba
@ 2026-05-07 18:26 ` Daniel Lezcano
2026-05-08 10:35 ` Lukasz Luba
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Lezcano @ 2026-05-07 18:26 UTC (permalink / raw)
To: Lukasz Luba; +Cc: rui.zhang, linux-pm, linux-kernel, rafael
On 5/7/26 12:02, Lukasz Luba wrote:
> Hi Daniel,
>
> On 5/5/26 15:44, Daniel Lezcano wrote:
>> In preparation for the upcoming changes separating OF and non-OF code,
>> split __thermal_cooling_device_register() into allocation and addition
>> phases.
>>
>> This allows moving the device node assignment out of the core
>> initialization path.
>>
>> This change is not a trivial split. The lifetime of the cooling device
>> is managed by the device core through put_device(), which triggers
>> thermal_release() to free all associated resources.
>>
>> With the introduction of thermal_cooling_device_alloc(), the allocation
>> path must mirror what thermal_release() undoes. In contrast,
>> thermal_cooling_device_add() must not perform any rollback and relies
>> on put_device() for cleanup on error paths. This avoids both double
>> free and resource leaks.
>>
>> As part of this rework, add the missing device_initialize() call when
>> allocating the cooling device.
>>
>> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
>> ---
[ ... ]
>> +static int thermal_cooling_device_add(struct thermal_cooling_device
>> *cdev, void *devdata)
>> +{
>> + unsigned long current_state;
>> + int ret;
>> +
>> mutex_init(&cdev->lock);
>> INIT_LIST_HEAD(&cdev->thermal_instances);
>> - cdev->np = np;
>> - cdev->ops = ops;
>> cdev->updated = false;
>> cdev->device.class = &thermal_class;
>> + device_initialize(&cdev->device);
>> cdev->devdata = devdata;
>> + thermal_cooling_device_setup_sysfs(cdev);
>> +
>> + ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
>> + if (ret)
>
> In case of error here, when the cdev->device won't have this name,
> would thermal_release() still be able to call the cleanup
> for the sysfs bits?
> IMHO the check 'if()' there might bite us and we might not free
> the sysfs allocated memory.
Hmm, I wondering if we can invert dev_set_name() and
thermal_cooling_device_setup_sysfs() ?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-07 18:26 ` Daniel Lezcano
@ 2026-05-08 10:35 ` Lukasz Luba
2026-05-08 11:25 ` Daniel Lezcano
0 siblings, 1 reply; 8+ messages in thread
From: Lukasz Luba @ 2026-05-08 10:35 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: rui.zhang, linux-pm, linux-kernel, rafael
On 5/7/26 19:26, Daniel Lezcano wrote:
> On 5/7/26 12:02, Lukasz Luba wrote:
>> Hi Daniel,
>>
>> On 5/5/26 15:44, Daniel Lezcano wrote:
>>> In preparation for the upcoming changes separating OF and non-OF code,
>>> split __thermal_cooling_device_register() into allocation and addition
>>> phases.
>>>
>>> This allows moving the device node assignment out of the core
>>> initialization path.
>>>
>>> This change is not a trivial split. The lifetime of the cooling device
>>> is managed by the device core through put_device(), which triggers
>>> thermal_release() to free all associated resources.
>>>
>>> With the introduction of thermal_cooling_device_alloc(), the allocation
>>> path must mirror what thermal_release() undoes. In contrast,
>>> thermal_cooling_device_add() must not perform any rollback and relies
>>> on put_device() for cleanup on error paths. This avoids both double
>>> free and resource leaks.
>>>
>>> As part of this rework, add the missing device_initialize() call when
>>> allocating the cooling device.
>>>
>>> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
>>> ---
>
> [ ... ]
>
>>> +static int thermal_cooling_device_add(struct thermal_cooling_device
>>> *cdev, void *devdata)
>>> +{
>>> + unsigned long current_state;
>>> + int ret;
>>> +
>>> mutex_init(&cdev->lock);
>>> INIT_LIST_HEAD(&cdev->thermal_instances);
>>> - cdev->np = np;
>>> - cdev->ops = ops;
>>> cdev->updated = false;
>>> cdev->device.class = &thermal_class;
>>> + device_initialize(&cdev->device);
>>> cdev->devdata = devdata;
>>> + thermal_cooling_device_setup_sysfs(cdev);
>>> +
>>> + ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
>>> + if (ret)
>>
>> In case of error here, when the cdev->device won't have this name,
>> would thermal_release() still be able to call the cleanup
>> for the sysfs bits?
>> IMHO the check 'if()' there might bite us and we might not free
>> the sysfs allocated memory.
>
> Hmm, I wondering if we can invert dev_set_name() and
> thermal_cooling_device_setup_sysfs() ?
>
I've checked that. It looks like the guarded 'if()'
in thermal_release which needs a dev_name() to clean-up the memory i
an issue in this case.
When this dev_set_name() fails, we won't go into the
clean up code to free cdev->type, ida, cdev...
I would call them explicitly in case of 'if(ret)'
error from dev_set_name()...
Then the rest could end in case of error with that new
'put_device()' logic at the bottom.
Would you agree?
A different option would be to refactor thermal_release()
and somehow recognize the cooling device not based on name.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-08 10:35 ` Lukasz Luba
@ 2026-05-08 11:25 ` Daniel Lezcano
2026-05-08 12:08 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Lezcano @ 2026-05-08 11:25 UTC (permalink / raw)
To: Lukasz Luba; +Cc: rui.zhang, linux-pm, linux-kernel, rafael
On 5/8/26 12:35, Lukasz Luba wrote:
>
>
> On 5/7/26 19:26, Daniel Lezcano wrote:
>> On 5/7/26 12:02, Lukasz Luba wrote:
[ ... ]
> I would call them explicitly in case of 'if(ret)'
> error from dev_set_name()...
> Then the rest could end in case of error with that new
> 'put_device()' logic at the bottom.
> Would you agree?
>
> A different option would be to refactor thermal_release()
> and somehow recognize the cooling device not based on name.
This mechanism is not adequate.
thermal_release should not be used this way, neither it should be needed.
We should have:
thermal_class = class_create("thermal");
if (IS_ERR(thermal_class))
...
No thermal_release needed.
But,
static void cooling_dev_release(struct device *dev)
{
thermal_cooling_device *cdev;
cdev = to_cooling_device(dev);
thermal_cooling_device_destroy_sysfs(cdev);
kfree_const(cdev->type);
ida_free(&thermal_cdev_ida, cdev->id);
kfree(cdev);
}
static void tz_dev_release(struct device *dev)
{
thermal_zone_device *tz;
tz = to_thermal_zone(dev);
thermal_zone_destroy_device_groups(tz);
thermal_set_governor(tz, NULL);
ida_destroy(&tz->ida);
mutex_destroy(&tz->lock);
complete(&tz->removal);
}
In thermal_cooling_device_add()
{
...
cdev->device.release = cooling_dev_release
...
}
In thermal_zone_device_register_with_trips()
{
...
tz->device.release = tz_dev_release
...
}
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-08 11:25 ` Daniel Lezcano
@ 2026-05-08 12:08 ` Rafael J. Wysocki
2026-05-08 12:40 ` Daniel Lezcano
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2026-05-08 12:08 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: Lukasz Luba, rui.zhang, linux-pm, linux-kernel, rafael
On Fri, May 8, 2026 at 1:25 PM Daniel Lezcano
<daniel.lezcano@oss.qualcomm.com> wrote:
>
> On 5/8/26 12:35, Lukasz Luba wrote:
> >
> >
> > On 5/7/26 19:26, Daniel Lezcano wrote:
> >> On 5/7/26 12:02, Lukasz Luba wrote:
>
> [ ... ]
>
> > I would call them explicitly in case of 'if(ret)'
> > error from dev_set_name()...
> > Then the rest could end in case of error with that new
> > 'put_device()' logic at the bottom.
> > Would you agree?
> >
> > A different option would be to refactor thermal_release()
> > and somehow recognize the cooling device not based on name.
>
> This mechanism is not adequate.
>
> thermal_release should not be used this way, neither it should be needed.
>
> We should have:
>
> thermal_class = class_create("thermal");
> if (IS_ERR(thermal_class))
> ...
>
> No thermal_release needed.
>
>
> But,
>
> static void cooling_dev_release(struct device *dev)
> {
> thermal_cooling_device *cdev;
>
> cdev = to_cooling_device(dev);
> thermal_cooling_device_destroy_sysfs(cdev);
> kfree_const(cdev->type);
> ida_free(&thermal_cdev_ida, cdev->id);
> kfree(cdev);
> }
>
> static void tz_dev_release(struct device *dev)
> {
> thermal_zone_device *tz;
>
> tz = to_thermal_zone(dev);
> thermal_zone_destroy_device_groups(tz);
> thermal_set_governor(tz, NULL);
> ida_destroy(&tz->ida);
> mutex_destroy(&tz->lock);
> complete(&tz->removal);
> }
>
> In thermal_cooling_device_add()
> {
> ...
> cdev->device.release = cooling_dev_release
> ...
> }
>
>
> In thermal_zone_device_register_with_trips()
> {
> ...
> tz->device.release = tz_dev_release
> ...
> }
Right, so why don't we clean that up to start with?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions
2026-05-08 12:08 ` Rafael J. Wysocki
@ 2026-05-08 12:40 ` Daniel Lezcano
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Lezcano @ 2026-05-08 12:40 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Lukasz Luba, rui.zhang, linux-pm, linux-kernel
On 5/8/26 14:08, Rafael J. Wysocki wrote:
> On Fri, May 8, 2026 at 1:25 PM Daniel Lezcano
> <daniel.lezcano@oss.qualcomm.com> wrote:
[ ... ]
>> In thermal_zone_device_register_with_trips()
>> {
>> ...
>> tz->device.release = tz_dev_release
>> ...
>> }
>
> Right, so why don't we clean that up to start with?
On the way ... :)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-08 12:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 14:44 [PATCH] thermal/core: Split __thermal_cooling_device_register() into two functions Daniel Lezcano
2026-05-07 8:29 ` Daniel Lezcano
2026-05-07 10:02 ` Lukasz Luba
2026-05-07 18:26 ` Daniel Lezcano
2026-05-08 10:35 ` Lukasz Luba
2026-05-08 11:25 ` Daniel Lezcano
2026-05-08 12:08 ` Rafael J. Wysocki
2026-05-08 12:40 ` Daniel Lezcano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox