* [PATCH v2 1/3] thermal/drivers/bcm2835: Remove buggy call to thermal_of_zone_unregister
@ 2023-04-04 7:51 ` Daniel Lezcano
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2023-04-04 7:51 UTC (permalink / raw
To: daniel.lezcano, rafael
Cc: rui.zhang, linux-pm, linux-kernel, Florian Fainelli, Ray Jui,
Scott Branden, Amit Kucheria,
Broadcom internal kernel review list, Niklas Söderlund,
Kunihiko Hayashi, ye xingchen,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
The driver is using the devm_thermal_of_zone_device_register().
In the error path of the function calling
devm_thermal_of_zone_device_register(), the function
devm_thermal_of_zone_unregister() should be called instead of
thermal_of_zone_unregister(), otherwise this one will be called twice
when the device is freed.
The same happens for the remove function where the devm_ guarantee the
thermal_of_zone_unregister() will be called, so adding this call in
the remove function will lead to a double free also.
Use devm_ variant in the error path of the probe function.
Remove thermal_of_zone_unregister() in the remove function.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
V2:
- Fixed wrong label call on the error path
---
drivers/thermal/broadcom/bcm2835_thermal.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c
index a217d832f24e..3acc9288b310 100644
--- a/drivers/thermal/broadcom/bcm2835_thermal.c
+++ b/drivers/thermal/broadcom/bcm2835_thermal.c
@@ -275,7 +275,7 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
return 0;
err_tz:
- thermal_of_zone_unregister(tz);
+ devm_thermal_of_zone_unregister(&pdev->dev, tz);
err_clk:
clk_disable_unprepare(data->clk);
@@ -285,10 +285,8 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
static int bcm2835_thermal_remove(struct platform_device *pdev)
{
struct bcm2835_thermal_data *data = platform_get_drvdata(pdev);
- struct thermal_zone_device *tz = data->tz;
debugfs_remove_recursive(data->debugfsdir);
- thermal_of_zone_unregister(tz);
clk_disable_unprepare(data->clk);
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] thermal/drivers/bcm2835: Remove buggy call to thermal_of_zone_unregister
@ 2023-04-04 7:51 ` Daniel Lezcano
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2023-04-04 7:51 UTC (permalink / raw
To: daniel.lezcano, rafael
Cc: rui.zhang, linux-pm, linux-kernel, Florian Fainelli, Ray Jui,
Scott Branden, Amit Kucheria,
Broadcom internal kernel review list, Niklas Söderlund,
Kunihiko Hayashi, ye xingchen,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
The driver is using the devm_thermal_of_zone_device_register().
In the error path of the function calling
devm_thermal_of_zone_device_register(), the function
devm_thermal_of_zone_unregister() should be called instead of
thermal_of_zone_unregister(), otherwise this one will be called twice
when the device is freed.
The same happens for the remove function where the devm_ guarantee the
thermal_of_zone_unregister() will be called, so adding this call in
the remove function will lead to a double free also.
Use devm_ variant in the error path of the probe function.
Remove thermal_of_zone_unregister() in the remove function.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
V2:
- Fixed wrong label call on the error path
---
drivers/thermal/broadcom/bcm2835_thermal.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c
index a217d832f24e..3acc9288b310 100644
--- a/drivers/thermal/broadcom/bcm2835_thermal.c
+++ b/drivers/thermal/broadcom/bcm2835_thermal.c
@@ -275,7 +275,7 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
return 0;
err_tz:
- thermal_of_zone_unregister(tz);
+ devm_thermal_of_zone_unregister(&pdev->dev, tz);
err_clk:
clk_disable_unprepare(data->clk);
@@ -285,10 +285,8 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
static int bcm2835_thermal_remove(struct platform_device *pdev)
{
struct bcm2835_thermal_data *data = platform_get_drvdata(pdev);
- struct thermal_zone_device *tz = data->tz;
debugfs_remove_recursive(data->debugfsdir);
- thermal_of_zone_unregister(tz);
clk_disable_unprepare(data->clk);
return 0;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] thermal/of: Unexport unused OF functions
2023-04-04 7:51 ` Daniel Lezcano
(?)
@ 2023-04-04 7:51 ` Daniel Lezcano
-1 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2023-04-04 7:51 UTC (permalink / raw
To: daniel.lezcano, rafael; +Cc: rui.zhang, linux-pm, linux-kernel, Amit Kucheria
The functions thermal_of_zone_register() and
thermal_of_zone_unregister() are no longer needed from the drivers as
the devm_ variant is always used.
Make them static in the C file and remove their declaration from thermal.h
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
V2:
- No changes
---
drivers/thermal/thermal_of.c | 8 +++-----
include/linux/thermal.h | 17 -----------------
2 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index ff4d12ef51bc..6fb14e521197 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -439,7 +439,7 @@ static int thermal_of_unbind(struct thermal_zone_device *tz,
*
* @tz: a pointer to the thermal zone structure
*/
-void thermal_of_zone_unregister(struct thermal_zone_device *tz)
+static void thermal_of_zone_unregister(struct thermal_zone_device *tz)
{
struct thermal_trip *trips = tz->trips;
struct thermal_zone_params *tzp = tz->tzp;
@@ -451,7 +451,6 @@ void thermal_of_zone_unregister(struct thermal_zone_device *tz)
kfree(tzp);
kfree(ops);
}
-EXPORT_SYMBOL_GPL(thermal_of_zone_unregister);
/**
* thermal_of_zone_register - Register a thermal zone with device node
@@ -473,8 +472,8 @@ EXPORT_SYMBOL_GPL(thermal_of_zone_unregister);
* - ENOMEM: if one structure can not be allocated
* - Other negative errors are returned by the underlying called functions
*/
-struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor, int id, void *data,
- const struct thermal_zone_device_ops *ops)
+static struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor, int id, void *data,
+ const struct thermal_zone_device_ops *ops)
{
struct thermal_zone_device *tz;
struct thermal_trip *trips;
@@ -550,7 +549,6 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
return ERR_PTR(ret);
}
-EXPORT_SYMBOL_GPL(thermal_of_zone_register);
static void devm_thermal_of_zone_release(struct device *dev, void *res)
{
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 204116dd97bf..d26b8e2e404e 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -266,25 +266,12 @@ struct thermal_zone_params {
/* Function declarations */
#ifdef CONFIG_THERMAL_OF
-struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor, int id, void *data,
- const struct thermal_zone_device_ops *ops);
-
struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, int id, void *data,
const struct thermal_zone_device_ops *ops);
-void thermal_of_zone_unregister(struct thermal_zone_device *tz);
-
void devm_thermal_of_zone_unregister(struct device *dev, struct thermal_zone_device *tz);
-void thermal_of_zone_unregister(struct thermal_zone_device *tz);
-
#else
-static inline
-struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor, int id, void *data,
- const struct thermal_zone_device_ops *ops)
-{
- return ERR_PTR(-ENOTSUPP);
-}
static inline
struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, int id, void *data,
@@ -293,10 +280,6 @@ struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, in
return ERR_PTR(-ENOTSUPP);
}
-static inline void thermal_of_zone_unregister(struct thermal_zone_device *tz)
-{
-}
-
static inline void devm_thermal_of_zone_unregister(struct device *dev,
struct thermal_zone_device *tz)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] thermal/core: Alloc-copy-free the thermal zone parameters structure
2023-04-04 7:51 ` Daniel Lezcano
(?)
(?)
@ 2023-04-04 7:51 ` Daniel Lezcano
-1 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2023-04-04 7:51 UTC (permalink / raw
To: daniel.lezcano, rafael; +Cc: rui.zhang, linux-pm, linux-kernel, Amit Kucheria
The caller of the function thermal_zone_device_register_with_trips()
can pass a thermal_zone_params structure parameter.
This one is used by the thermal core code until the thermal zone is
destroyed. That forces the caller, so the driver, to keep the pointer
valid until it unregisters the thermal zone if we want to make the
thermal zone device structure private the core code.
As the thermal zone device structure would be private, the driver can
not access to thermal zone device structure to retrieve the tzp field
after it passed it to register the thermal zone.
So instead of forcing the users of the function to deal with the tzp
structure life cycle, make the usage easier by allocating our own
thermal zone params, copying the parameter content and by freeing at
unregister time. The user can then create the parameters on the stack,
pass it to the registering function and forget about it.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
V2:
- Fixed 'result' uninitialized on the error path
---
drivers/thermal/thermal_core.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 8196d35f4c08..518b87cddaeb 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1170,13 +1170,21 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
if (!tz)
return ERR_PTR(-ENOMEM);
+ if (tzp) {
+ tz->tzp = kmemdup(tzp, sizeof(*tzp), GFP_KERNEL);
+ if (!tz->tzp) {
+ result = -ENOMEM;
+ goto free_tz;
+ }
+ }
+
INIT_LIST_HEAD(&tz->thermal_instances);
ida_init(&tz->ida);
mutex_init(&tz->lock);
id = ida_alloc(&thermal_tz_ida, GFP_KERNEL);
if (id < 0) {
result = id;
- goto free_tz;
+ goto free_tzp;
}
tz->id = id;
@@ -1186,7 +1194,6 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
ops->critical = thermal_zone_device_critical;
tz->ops = ops;
- tz->tzp = tzp;
tz->device.class = thermal_class;
tz->devdata = devdata;
tz->trips = trips;
@@ -1284,6 +1291,8 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
tz = NULL;
remove_id:
ida_free(&thermal_tz_ida, id);
+free_tzp:
+ kfree(tz->tzp);
free_tz:
kfree(tz);
return ERR_PTR(result);
@@ -1364,6 +1373,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
device_del(&tz->device);
mutex_unlock(&tz->lock);
+ kfree(tz->tzp);
+
put_device(&tz->device);
thermal_notify_tz_delete(tz_id);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] thermal/drivers/bcm2835: Remove buggy call to thermal_of_zone_unregister
2023-04-04 7:51 ` Daniel Lezcano
@ 2023-04-07 16:35 ` Daniel Lezcano
-1 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2023-04-07 16:35 UTC (permalink / raw
To: rafael
Cc: rui.zhang, linux-pm, linux-kernel, Florian Fainelli, Ray Jui,
Scott Branden, Amit Kucheria,
Broadcom internal kernel review list, Niklas Söderlund,
Kunihiko Hayashi, ye xingchen,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
Hi,
it seems like the remaining patches of the previous series does not
raise feedbacks, I assume they are fine.
If nobody raises concern about it after this week-end, they will be
applied to candidate for linux-next
Thanks
-- Daniel
On 04/04/2023 09:51, Daniel Lezcano wrote:
> The driver is using the devm_thermal_of_zone_device_register().
>
> In the error path of the function calling
> devm_thermal_of_zone_device_register(), the function
> devm_thermal_of_zone_unregister() should be called instead of
> thermal_of_zone_unregister(), otherwise this one will be called twice
> when the device is freed.
>
> The same happens for the remove function where the devm_ guarantee the
> thermal_of_zone_unregister() will be called, so adding this call in
> the remove function will lead to a double free also.
>
> Use devm_ variant in the error path of the probe function.
>
> Remove thermal_of_zone_unregister() in the remove function.
>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> V2:
> - Fixed wrong label call on the error path
> ---
> drivers/thermal/broadcom/bcm2835_thermal.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c
> index a217d832f24e..3acc9288b310 100644
> --- a/drivers/thermal/broadcom/bcm2835_thermal.c
> +++ b/drivers/thermal/broadcom/bcm2835_thermal.c
> @@ -275,7 +275,7 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
>
> return 0;
> err_tz:
> - thermal_of_zone_unregister(tz);
> + devm_thermal_of_zone_unregister(&pdev->dev, tz);
> err_clk:
> clk_disable_unprepare(data->clk);
>
> @@ -285,10 +285,8 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
> static int bcm2835_thermal_remove(struct platform_device *pdev)
> {
> struct bcm2835_thermal_data *data = platform_get_drvdata(pdev);
> - struct thermal_zone_device *tz = data->tz;
>
> debugfs_remove_recursive(data->debugfsdir);
> - thermal_of_zone_unregister(tz);
> clk_disable_unprepare(data->clk);
>
> return 0;
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] thermal/drivers/bcm2835: Remove buggy call to thermal_of_zone_unregister
@ 2023-04-07 16:35 ` Daniel Lezcano
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2023-04-07 16:35 UTC (permalink / raw
To: rafael
Cc: rui.zhang, linux-pm, linux-kernel, Florian Fainelli, Ray Jui,
Scott Branden, Amit Kucheria,
Broadcom internal kernel review list, Niklas Söderlund,
Kunihiko Hayashi, ye xingchen,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
Hi,
it seems like the remaining patches of the previous series does not
raise feedbacks, I assume they are fine.
If nobody raises concern about it after this week-end, they will be
applied to candidate for linux-next
Thanks
-- Daniel
On 04/04/2023 09:51, Daniel Lezcano wrote:
> The driver is using the devm_thermal_of_zone_device_register().
>
> In the error path of the function calling
> devm_thermal_of_zone_device_register(), the function
> devm_thermal_of_zone_unregister() should be called instead of
> thermal_of_zone_unregister(), otherwise this one will be called twice
> when the device is freed.
>
> The same happens for the remove function where the devm_ guarantee the
> thermal_of_zone_unregister() will be called, so adding this call in
> the remove function will lead to a double free also.
>
> Use devm_ variant in the error path of the probe function.
>
> Remove thermal_of_zone_unregister() in the remove function.
>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> V2:
> - Fixed wrong label call on the error path
> ---
> drivers/thermal/broadcom/bcm2835_thermal.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c
> index a217d832f24e..3acc9288b310 100644
> --- a/drivers/thermal/broadcom/bcm2835_thermal.c
> +++ b/drivers/thermal/broadcom/bcm2835_thermal.c
> @@ -275,7 +275,7 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
>
> return 0;
> err_tz:
> - thermal_of_zone_unregister(tz);
> + devm_thermal_of_zone_unregister(&pdev->dev, tz);
> err_clk:
> clk_disable_unprepare(data->clk);
>
> @@ -285,10 +285,8 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
> static int bcm2835_thermal_remove(struct platform_device *pdev)
> {
> struct bcm2835_thermal_data *data = platform_get_drvdata(pdev);
> - struct thermal_zone_device *tz = data->tz;
>
> debugfs_remove_recursive(data->debugfsdir);
> - thermal_of_zone_unregister(tz);
> clk_disable_unprepare(data->clk);
>
> return 0;
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-07 16:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-04 7:51 [PATCH v2 1/3] thermal/drivers/bcm2835: Remove buggy call to thermal_of_zone_unregister Daniel Lezcano
2023-04-04 7:51 ` Daniel Lezcano
2023-04-04 7:51 ` [PATCH v2 2/3] thermal/of: Unexport unused OF functions Daniel Lezcano
2023-04-04 7:51 ` [PATCH v2 3/3] thermal/core: Alloc-copy-free the thermal zone parameters structure Daniel Lezcano
2023-04-07 16:35 ` [PATCH v2 1/3] thermal/drivers/bcm2835: Remove buggy call to thermal_of_zone_unregister Daniel Lezcano
2023-04-07 16:35 ` Daniel Lezcano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.