* [PATCH RESEND 0/4] thermal sys: couple of fixes and cleanups
@ 2013-01-02 15:29 Eduardo Valentin
2013-01-02 15:29 ` [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages Eduardo Valentin
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-02 15:29 UTC (permalink / raw)
To: rui.zhang; +Cc: durgadoss.r, linux-pm, linux-pm, Eduardo Valentin
Hello Rui,
Here is a couple of fixes and cleanups on the thermal_sys.c.
I am resending this series because it has been idling in the mainling list
for some months already. So, before we forget them, I decided to resend.
Again, one important change is the netlink channel, which I am proposing
to have the zone id in each message.
So far, looks like we can deprecate this API, but in this series, I keep
it but now every message has the zone id.
All best,
Eduardo Valentin (4):
thermal: Use thermal zone device id in netlink messages
thermal: remove unnecessary include
thermal: cleanup: use dev_* helper functions
thermal: check for invalid trip setup when registering thermal device
Documentation/thermal/sysfs-api.txt | 5 +++--
drivers/thermal/thermal_sys.c | 22 +++++++++++++++-------
include/linux/thermal.h | 6 ++++--
3 files changed, 22 insertions(+), 11 deletions(-)
--
1.7.7.1.488.ge8e1c
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages
2013-01-02 15:29 [PATCH RESEND 0/4] thermal sys: couple of fixes and cleanups Eduardo Valentin
@ 2013-01-02 15:29 ` Eduardo Valentin
2013-01-02 15:48 ` R, Durgadoss
2013-01-16 2:43 ` Zhang Rui
2013-01-02 15:29 ` [PATCH RESEND 2/4] thermal: remove unnecessary include Eduardo Valentin
` (2 subsequent siblings)
3 siblings, 2 replies; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-02 15:29 UTC (permalink / raw)
To: rui.zhang; +Cc: durgadoss.r, linux-pm, linux-pm, Eduardo Valentin
This patch changes the function thermal_generate_netlink_event
to receive a thermal zone device instead of a originator id.
This way, the messages will always be bound to a thermal zone.
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
Documentation/thermal/sysfs-api.txt | 5 +++--
drivers/thermal/thermal_sys.c | 8 ++++++--
include/linux/thermal.h | 6 ++++--
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 88c0233..526d4b9 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -329,8 +329,9 @@ The framework includes a simple notification mechanism, in the form of a
netlink event. Netlink socket initialization is done during the _init_
of the framework. Drivers which intend to use the notification mechanism
just need to call thermal_generate_netlink_event() with two arguments viz
-(originator, event). Typically the originator will be an integer assigned
-to a thermal_zone_device when it registers itself with the framework. The
+(originator, event). The originator is a pointer to struct thermal_zone_device
+from where the event has been originated. An integer which represents the
+thermal zone device will be used in the message to identify the zone. The
event will be one of:{THERMAL_AUX0, THERMAL_AUX1, THERMAL_CRITICAL,
THERMAL_DEV_FAULT}. Notification can be sent when the current temperature
crosses any of the configured thresholds.
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 8c8ce80..d85f51f 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1711,7 +1711,8 @@ static struct genl_multicast_group thermal_event_mcgrp = {
.name = THERMAL_GENL_MCAST_GROUP_NAME,
};
-int thermal_generate_netlink_event(u32 orig, enum events event)
+int thermal_generate_netlink_event(struct thermal_zone_device *tz,
+ enum events event)
{
struct sk_buff *skb;
struct nlattr *attr;
@@ -1721,6 +1722,9 @@ int thermal_generate_netlink_event(u32 orig, enum events event)
int result;
static unsigned int thermal_event_seqnum;
+ if (!tz)
+ return -EINVAL;
+
/* allocate memory */
size = nla_total_size(sizeof(struct thermal_genl_event)) +
nla_total_size(0);
@@ -1755,7 +1759,7 @@ int thermal_generate_netlink_event(u32 orig, enum events event)
memset(thermal_event, 0, sizeof(struct thermal_genl_event));
- thermal_event->orig = orig;
+ thermal_event->orig = tz->id;
thermal_event->event = event;
/* send multicast genetlink message */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index fe82022..e85ac70 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -244,9 +244,11 @@ int thermal_register_governor(struct thermal_governor *);
void thermal_unregister_governor(struct thermal_governor *);
#ifdef CONFIG_NET
-extern int thermal_generate_netlink_event(u32 orig, enum events event);
+extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
+ enum events event);
#else
-static inline int thermal_generate_netlink_event(u32 orig, enum events event)
+static int thermal_generate_netlink_event(struct thermal_zone_device *tz,
+ enum events event)
{
return 0;
}
--
1.7.7.1.488.ge8e1c
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH RESEND 2/4] thermal: remove unnecessary include
2013-01-02 15:29 [PATCH RESEND 0/4] thermal sys: couple of fixes and cleanups Eduardo Valentin
2013-01-02 15:29 ` [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages Eduardo Valentin
@ 2013-01-02 15:29 ` Eduardo Valentin
2013-01-02 15:48 ` R, Durgadoss
2013-01-16 2:43 ` Zhang Rui
2013-01-02 15:29 ` [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions Eduardo Valentin
2013-01-02 15:29 ` [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device Eduardo Valentin
3 siblings, 2 replies; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-02 15:29 UTC (permalink / raw)
To: rui.zhang; +Cc: durgadoss.r, linux-pm, linux-pm, Eduardo Valentin
No need for spinlocks in this file, then removing its header.
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
drivers/thermal/thermal_sys.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index d85f51f..70ce100 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -32,7 +32,6 @@
#include <linux/kdev_t.h>
#include <linux/idr.h>
#include <linux/thermal.h>
-#include <linux/spinlock.h>
#include <linux/reboot.h>
#include <net/netlink.h>
#include <net/genetlink.h>
--
1.7.7.1.488.ge8e1c
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions
2013-01-02 15:29 [PATCH RESEND 0/4] thermal sys: couple of fixes and cleanups Eduardo Valentin
2013-01-02 15:29 ` [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages Eduardo Valentin
2013-01-02 15:29 ` [PATCH RESEND 2/4] thermal: remove unnecessary include Eduardo Valentin
@ 2013-01-02 15:29 ` Eduardo Valentin
2013-01-02 15:51 ` R, Durgadoss
2013-01-16 2:44 ` Zhang Rui
2013-01-02 15:29 ` [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device Eduardo Valentin
3 siblings, 2 replies; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-02 15:29 UTC (permalink / raw)
To: rui.zhang; +Cc: durgadoss.r, linux-pm, linux-pm, Eduardo Valentin
Change the logging messages to used dev_* helper functions.
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
drivers/thermal/thermal_sys.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 70ce100..fba27c3 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -354,8 +354,9 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
tz->ops->notify(tz, trip, trip_type);
if (trip_type == THERMAL_TRIP_CRITICAL) {
- pr_emerg("Critical temperature reached(%d C),shutting down\n",
- tz->temperature / 1000);
+ dev_emerg(&tz->device,
+ "critical temperature reached(%d C),shutting down\n",
+ tz->temperature / 1000);
orderly_poweroff(true);
}
}
@@ -386,7 +387,8 @@ static void update_temperature(struct thermal_zone_device *tz)
ret = tz->ops->get_temp(tz, &temp);
if (ret) {
- pr_warn("failed to read out thermal zone %d\n", tz->id);
+ dev_warn(&tz->device, "failed to read out thermal zone %d\n",
+ tz->id);
goto exit;
}
@@ -1770,7 +1772,7 @@ int thermal_generate_netlink_event(struct thermal_zone_device *tz,
result = genlmsg_multicast(skb, 0, thermal_event_mcgrp.id, GFP_ATOMIC);
if (result)
- pr_info("failed to send netlink event:%d\n", result);
+ dev_err(&tz->device, "Failed to send netlink event:%d", result);
return result;
}
--
1.7.7.1.488.ge8e1c
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device
2013-01-02 15:29 [PATCH RESEND 0/4] thermal sys: couple of fixes and cleanups Eduardo Valentin
` (2 preceding siblings ...)
2013-01-02 15:29 ` [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions Eduardo Valentin
@ 2013-01-02 15:29 ` Eduardo Valentin
2013-01-02 15:53 ` R, Durgadoss
2013-01-16 2:45 ` Zhang Rui
3 siblings, 2 replies; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-02 15:29 UTC (permalink / raw)
To: rui.zhang; +Cc: durgadoss.r, linux-pm, linux-pm, Eduardo Valentin
This patch adds an extra check in the data structure while registering
a thermal device. The check is to avoid registering zones with a number
of trips greater than zero, but with no .get_trip_temp nor .get_trip_type
callbacks. Receiving such data structure may end in wrong data access.
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
drivers/thermal/thermal_sys.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index fba27c3..0a1bf6b 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1530,6 +1530,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
if (!ops || !ops->get_temp)
return ERR_PTR(-EINVAL);
+ if (trips > 0 && !ops->get_trip_type)
+ return ERR_PTR(-EINVAL);
+
tz = kzalloc(sizeof(struct thermal_zone_device), GFP_KERNEL);
if (!tz)
return ERR_PTR(-ENOMEM);
--
1.7.7.1.488.ge8e1c
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages
2013-01-02 15:29 ` [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages Eduardo Valentin
@ 2013-01-02 15:48 ` R, Durgadoss
2013-01-02 15:55 ` Eduardo Valentin
2013-01-16 2:43 ` Zhang Rui
1 sibling, 1 reply; 16+ messages in thread
From: R, Durgadoss @ 2013-01-02 15:48 UTC (permalink / raw)
To: Eduardo Valentin, Zhang, Rui
Cc: linux-pm@lists.linux-foundation.org, linux-pm@vger.kernel.org
> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Wednesday, January 02, 2013 9:00 PM
> To: Zhang, Rui
> Cc: R, Durgadoss; linux-pm@lists.linux-foundation.org; linux-
> pm@vger.kernel.org; Eduardo Valentin
> Subject: [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink
> messages
>
> This patch changes the function thermal_generate_netlink_event
> to receive a thermal zone device instead of a originator id.
>
> This way, the messages will always be bound to a thermal zone.
As you have mentioned, we are planning to deprecate this API.
But, for now, this change is Ok :-)
And I believe all these four patches are on Rui's tree ?
>
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> Documentation/thermal/sysfs-api.txt | 5 +++--
> drivers/thermal/thermal_sys.c | 8 ++++++--
> include/linux/thermal.h | 6 ++++--
> 3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> index 88c0233..526d4b9 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -329,8 +329,9 @@ The framework includes a simple notification
> mechanism, in the form of a
> netlink event. Netlink socket initialization is done during the _init_
> of the framework. Drivers which intend to use the notification mechanism
> just need to call thermal_generate_netlink_event() with two arguments viz
> -(originator, event). Typically the originator will be an integer assigned
> -to a thermal_zone_device when it registers itself with the framework. The
> +(originator, event). The originator is a pointer to struct
> thermal_zone_device
> +from where the event has been originated. An integer which represents
> the
> +thermal zone device will be used in the message to identify the zone. The
> event will be one of:{THERMAL_AUX0, THERMAL_AUX1,
> THERMAL_CRITICAL,
> THERMAL_DEV_FAULT}. Notification can be sent when the current
> temperature
> crosses any of the configured thresholds.
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 8c8ce80..d85f51f 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1711,7 +1711,8 @@ static struct genl_multicast_group
> thermal_event_mcgrp = {
> .name = THERMAL_GENL_MCAST_GROUP_NAME,
> };
>
> -int thermal_generate_netlink_event(u32 orig, enum events event)
> +int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> + enum events event)
> {
> struct sk_buff *skb;
> struct nlattr *attr;
> @@ -1721,6 +1722,9 @@ int thermal_generate_netlink_event(u32 orig,
> enum events event)
> int result;
> static unsigned int thermal_event_seqnum;
>
> + if (!tz)
> + return -EINVAL;
> +
> /* allocate memory */
> size = nla_total_size(sizeof(struct thermal_genl_event)) +
> nla_total_size(0);
> @@ -1755,7 +1759,7 @@ int thermal_generate_netlink_event(u32 orig,
> enum events event)
>
> memset(thermal_event, 0, sizeof(struct thermal_genl_event));
>
> - thermal_event->orig = orig;
> + thermal_event->orig = tz->id;
> thermal_event->event = event;
>
> /* send multicast genetlink message */
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index fe82022..e85ac70 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -244,9 +244,11 @@ int thermal_register_governor(struct
> thermal_governor *);
> void thermal_unregister_governor(struct thermal_governor *);
>
> #ifdef CONFIG_NET
> -extern int thermal_generate_netlink_event(u32 orig, enum events event);
> +extern int thermal_generate_netlink_event(struct thermal_zone_device
> *tz,
> + enum events event);
> #else
> -static inline int thermal_generate_netlink_event(u32 orig, enum events
> event)
> +static int thermal_generate_netlink_event(struct thermal_zone_device
> *tz,
> + enum events event)
> {
> return 0;
> }
> --
> 1.7.7.1.488.ge8e1c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH RESEND 2/4] thermal: remove unnecessary include
2013-01-02 15:29 ` [PATCH RESEND 2/4] thermal: remove unnecessary include Eduardo Valentin
@ 2013-01-02 15:48 ` R, Durgadoss
2013-01-16 2:43 ` Zhang Rui
1 sibling, 0 replies; 16+ messages in thread
From: R, Durgadoss @ 2013-01-02 15:48 UTC (permalink / raw)
To: Eduardo Valentin, Zhang, Rui
Cc: linux-pm@lists.linux-foundation.org, linux-pm@vger.kernel.org
> -----Original Message-----
> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
> Sent: Wednesday, January 02, 2013 9:00 PM
> To: Zhang, Rui
> Cc: R, Durgadoss; linux-pm@lists.linux-foundation.org; linux-
> pm@vger.kernel.org; Eduardo Valentin
> Subject: [PATCH RESEND 2/4] thermal: remove unnecessary include
>
> No need for spinlocks in this file, then removing its header.
Good Catch !!
>
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> drivers/thermal/thermal_sys.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index d85f51f..70ce100 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -32,7 +32,6 @@
> #include <linux/kdev_t.h>
> #include <linux/idr.h>
> #include <linux/thermal.h>
> -#include <linux/spinlock.h>
> #include <linux/reboot.h>
> #include <net/netlink.h>
> #include <net/genetlink.h>
> --
> 1.7.7.1.488.ge8e1c
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions
2013-01-02 15:29 ` [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions Eduardo Valentin
@ 2013-01-02 15:51 ` R, Durgadoss
2013-01-16 2:44 ` Zhang Rui
1 sibling, 0 replies; 16+ messages in thread
From: R, Durgadoss @ 2013-01-02 15:51 UTC (permalink / raw)
To: Eduardo Valentin, Zhang, Rui
Cc: linux-pm@lists.linux-foundation.org, linux-pm@vger.kernel.org
> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Wednesday, January 02, 2013 9:00 PM
> To: Zhang, Rui
> Cc: R, Durgadoss; linux-pm@lists.linux-foundation.org; linux-
> pm@vger.kernel.org; Eduardo Valentin
> Subject: [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions
>
> Change the logging messages to used dev_* helper functions.
A good to have Change :-)
>
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> drivers/thermal/thermal_sys.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 70ce100..fba27c3 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -354,8 +354,9 @@ static void handle_critical_trips(struct
> thermal_zone_device *tz,
> tz->ops->notify(tz, trip, trip_type);
>
> if (trip_type == THERMAL_TRIP_CRITICAL) {
> - pr_emerg("Critical temperature reached(%d C),shutting
> down\n",
> - tz->temperature / 1000);
> + dev_emerg(&tz->device,
> + "critical temperature reached(%d C),shutting
> down\n",
> + tz->temperature / 1000);
> orderly_poweroff(true);
> }
> }
> @@ -386,7 +387,8 @@ static void update_temperature(struct
> thermal_zone_device *tz)
>
> ret = tz->ops->get_temp(tz, &temp);
> if (ret) {
> - pr_warn("failed to read out thermal zone %d\n", tz->id);
> + dev_warn(&tz->device, "failed to read out thermal zone
> %d\n",
> + tz->id);
> goto exit;
> }
>
> @@ -1770,7 +1772,7 @@ int thermal_generate_netlink_event(struct
> thermal_zone_device *tz,
>
> result = genlmsg_multicast(skb, 0, thermal_event_mcgrp.id,
> GFP_ATOMIC);
> if (result)
> - pr_info("failed to send netlink event:%d\n", result);
> + dev_err(&tz->device, "Failed to send netlink event:%d",
> result);
>
> return result;
> }
> --
> 1.7.7.1.488.ge8e1c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device
2013-01-02 15:29 ` [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device Eduardo Valentin
@ 2013-01-02 15:53 ` R, Durgadoss
2013-01-16 2:45 ` Zhang Rui
1 sibling, 0 replies; 16+ messages in thread
From: R, Durgadoss @ 2013-01-02 15:53 UTC (permalink / raw)
To: Eduardo Valentin, Zhang, Rui
Cc: linux-pm@lists.linux-foundation.org, linux-pm@vger.kernel.org
> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Wednesday, January 02, 2013 9:00 PM
> To: Zhang, Rui
> Cc: R, Durgadoss; linux-pm@lists.linux-foundation.org; linux-
> pm@vger.kernel.org; Eduardo Valentin
> Subject: [PATCH RESEND 4/4] thermal: check for invalid trip setup when
> registering thermal device
>
> This patch adds an extra check in the data structure while registering
> a thermal device. The check is to avoid registering zones with a number
> of trips greater than zero, but with no .get_trip_temp nor .get_trip_type
> callbacks. Receiving such data structure may end in wrong data access.
Yes, agreed. we are moving away from this API too :-(
But Nothing hurts to have this fix. So,
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> drivers/thermal/thermal_sys.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index fba27c3..0a1bf6b 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1530,6 +1530,9 @@ struct thermal_zone_device
> *thermal_zone_device_register(const char *type,
> if (!ops || !ops->get_temp)
> return ERR_PTR(-EINVAL);
>
> + if (trips > 0 && !ops->get_trip_type)
> + return ERR_PTR(-EINVAL);
> +
> tz = kzalloc(sizeof(struct thermal_zone_device), GFP_KERNEL);
> if (!tz)
> return ERR_PTR(-ENOMEM);
> --
> 1.7.7.1.488.ge8e1c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages
2013-01-02 15:48 ` R, Durgadoss
@ 2013-01-02 15:55 ` Eduardo Valentin
0 siblings, 0 replies; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-02 15:55 UTC (permalink / raw)
To: R, Durgadoss
Cc: Zhang, Rui, linux-pm@lists.linux-foundation.org,
linux-pm@vger.kernel.org
Hello,
On 02-01-2013 17:48, R, Durgadoss wrote:
>> -----Original Message-----
>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
>> Sent: Wednesday, January 02, 2013 9:00 PM
>> To: Zhang, Rui
>> Cc: R, Durgadoss; linux-pm@lists.linux-foundation.org; linux-
>> pm@vger.kernel.org; Eduardo Valentin
>> Subject: [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink
>> messages
>>
>> This patch changes the function thermal_generate_netlink_event
>> to receive a thermal zone device instead of a originator id.
>>
>> This way, the messages will always be bound to a thermal zone.
>
> As you have mentioned, we are planning to deprecate this API.
> But, for now, this change is Ok :-)
Good.
>
> And I believe all these four patches are on Rui's tree ?
They are not :-(
>
>>
>
> Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
>> Documentation/thermal/sysfs-api.txt | 5 +++--
>> drivers/thermal/thermal_sys.c | 8 ++++++--
>> include/linux/thermal.h | 6 ++++--
>> 3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt
>> b/Documentation/thermal/sysfs-api.txt
>> index 88c0233..526d4b9 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -329,8 +329,9 @@ The framework includes a simple notification
>> mechanism, in the form of a
>> netlink event. Netlink socket initialization is done during the _init_
>> of the framework. Drivers which intend to use the notification mechanism
>> just need to call thermal_generate_netlink_event() with two arguments viz
>> -(originator, event). Typically the originator will be an integer assigned
>> -to a thermal_zone_device when it registers itself with the framework. The
>> +(originator, event). The originator is a pointer to struct
>> thermal_zone_device
>> +from where the event has been originated. An integer which represents
>> the
>> +thermal zone device will be used in the message to identify the zone. The
>> event will be one of:{THERMAL_AUX0, THERMAL_AUX1,
>> THERMAL_CRITICAL,
>> THERMAL_DEV_FAULT}. Notification can be sent when the current
>> temperature
>> crosses any of the configured thresholds.
>> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> index 8c8ce80..d85f51f 100644
>> --- a/drivers/thermal/thermal_sys.c
>> +++ b/drivers/thermal/thermal_sys.c
>> @@ -1711,7 +1711,8 @@ static struct genl_multicast_group
>> thermal_event_mcgrp = {
>> .name = THERMAL_GENL_MCAST_GROUP_NAME,
>> };
>>
>> -int thermal_generate_netlink_event(u32 orig, enum events event)
>> +int thermal_generate_netlink_event(struct thermal_zone_device *tz,
>> + enum events event)
>> {
>> struct sk_buff *skb;
>> struct nlattr *attr;
>> @@ -1721,6 +1722,9 @@ int thermal_generate_netlink_event(u32 orig,
>> enum events event)
>> int result;
>> static unsigned int thermal_event_seqnum;
>>
>> + if (!tz)
>> + return -EINVAL;
>> +
>> /* allocate memory */
>> size = nla_total_size(sizeof(struct thermal_genl_event)) +
>> nla_total_size(0);
>> @@ -1755,7 +1759,7 @@ int thermal_generate_netlink_event(u32 orig,
>> enum events event)
>>
>> memset(thermal_event, 0, sizeof(struct thermal_genl_event));
>>
>> - thermal_event->orig = orig;
>> + thermal_event->orig = tz->id;
>> thermal_event->event = event;
>>
>> /* send multicast genetlink message */
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index fe82022..e85ac70 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -244,9 +244,11 @@ int thermal_register_governor(struct
>> thermal_governor *);
>> void thermal_unregister_governor(struct thermal_governor *);
>>
>> #ifdef CONFIG_NET
>> -extern int thermal_generate_netlink_event(u32 orig, enum events event);
>> +extern int thermal_generate_netlink_event(struct thermal_zone_device
>> *tz,
>> + enum events event);
>> #else
>> -static inline int thermal_generate_netlink_event(u32 orig, enum events
>> event)
>> +static int thermal_generate_netlink_event(struct thermal_zone_device
>> *tz,
>> + enum events event)
>> {
>> return 0;
>> }
>> --
>> 1.7.7.1.488.ge8e1c
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages
2013-01-02 15:29 ` [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages Eduardo Valentin
2013-01-02 15:48 ` R, Durgadoss
@ 2013-01-16 2:43 ` Zhang Rui
1 sibling, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2013-01-16 2:43 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: durgadoss.r, linux-pm, linux-pm
On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote:
> This patch changes the function thermal_generate_netlink_event
> to receive a thermal zone device instead of a originator id.
>
> This way, the messages will always be bound to a thermal zone.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
applied to thermal -next.
thanks,
rui
> ---
> Documentation/thermal/sysfs-api.txt | 5 +++--
> drivers/thermal/thermal_sys.c | 8 ++++++--
> include/linux/thermal.h | 6 ++++--
> 3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 88c0233..526d4b9 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -329,8 +329,9 @@ The framework includes a simple notification mechanism, in the form of a
> netlink event. Netlink socket initialization is done during the _init_
> of the framework. Drivers which intend to use the notification mechanism
> just need to call thermal_generate_netlink_event() with two arguments viz
> -(originator, event). Typically the originator will be an integer assigned
> -to a thermal_zone_device when it registers itself with the framework. The
> +(originator, event). The originator is a pointer to struct thermal_zone_device
> +from where the event has been originated. An integer which represents the
> +thermal zone device will be used in the message to identify the zone. The
> event will be one of:{THERMAL_AUX0, THERMAL_AUX1, THERMAL_CRITICAL,
> THERMAL_DEV_FAULT}. Notification can be sent when the current temperature
> crosses any of the configured thresholds.
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 8c8ce80..d85f51f 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1711,7 +1711,8 @@ static struct genl_multicast_group thermal_event_mcgrp = {
> .name = THERMAL_GENL_MCAST_GROUP_NAME,
> };
>
> -int thermal_generate_netlink_event(u32 orig, enum events event)
> +int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> + enum events event)
> {
> struct sk_buff *skb;
> struct nlattr *attr;
> @@ -1721,6 +1722,9 @@ int thermal_generate_netlink_event(u32 orig, enum events event)
> int result;
> static unsigned int thermal_event_seqnum;
>
> + if (!tz)
> + return -EINVAL;
> +
> /* allocate memory */
> size = nla_total_size(sizeof(struct thermal_genl_event)) +
> nla_total_size(0);
> @@ -1755,7 +1759,7 @@ int thermal_generate_netlink_event(u32 orig, enum events event)
>
> memset(thermal_event, 0, sizeof(struct thermal_genl_event));
>
> - thermal_event->orig = orig;
> + thermal_event->orig = tz->id;
> thermal_event->event = event;
>
> /* send multicast genetlink message */
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index fe82022..e85ac70 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -244,9 +244,11 @@ int thermal_register_governor(struct thermal_governor *);
> void thermal_unregister_governor(struct thermal_governor *);
>
> #ifdef CONFIG_NET
> -extern int thermal_generate_netlink_event(u32 orig, enum events event);
> +extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> + enum events event);
> #else
> -static inline int thermal_generate_netlink_event(u32 orig, enum events event)
> +static int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> + enum events event)
> {
> return 0;
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 2/4] thermal: remove unnecessary include
2013-01-02 15:29 ` [PATCH RESEND 2/4] thermal: remove unnecessary include Eduardo Valentin
2013-01-02 15:48 ` R, Durgadoss
@ 2013-01-16 2:43 ` Zhang Rui
1 sibling, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2013-01-16 2:43 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: durgadoss.r, linux-pm, linux-pm
On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote:
> No need for spinlocks in this file, then removing its header.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
applied to thermal -next.
thanks,
rui
> ---
> drivers/thermal/thermal_sys.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index d85f51f..70ce100 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -32,7 +32,6 @@
> #include <linux/kdev_t.h>
> #include <linux/idr.h>
> #include <linux/thermal.h>
> -#include <linux/spinlock.h>
> #include <linux/reboot.h>
> #include <net/netlink.h>
> #include <net/genetlink.h>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions
2013-01-02 15:29 ` [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions Eduardo Valentin
2013-01-02 15:51 ` R, Durgadoss
@ 2013-01-16 2:44 ` Zhang Rui
1 sibling, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2013-01-16 2:44 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: durgadoss.r, linux-pm, linux-pm
On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote:
> Change the logging messages to used dev_* helper functions.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
applied to thermal -next.
thanks,
rui
> ---
> drivers/thermal/thermal_sys.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 70ce100..fba27c3 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -354,8 +354,9 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> tz->ops->notify(tz, trip, trip_type);
>
> if (trip_type == THERMAL_TRIP_CRITICAL) {
> - pr_emerg("Critical temperature reached(%d C),shutting down\n",
> - tz->temperature / 1000);
> + dev_emerg(&tz->device,
> + "critical temperature reached(%d C),shutting down\n",
> + tz->temperature / 1000);
> orderly_poweroff(true);
> }
> }
> @@ -386,7 +387,8 @@ static void update_temperature(struct thermal_zone_device *tz)
>
> ret = tz->ops->get_temp(tz, &temp);
> if (ret) {
> - pr_warn("failed to read out thermal zone %d\n", tz->id);
> + dev_warn(&tz->device, "failed to read out thermal zone %d\n",
> + tz->id);
> goto exit;
> }
>
> @@ -1770,7 +1772,7 @@ int thermal_generate_netlink_event(struct thermal_zone_device *tz,
>
> result = genlmsg_multicast(skb, 0, thermal_event_mcgrp.id, GFP_ATOMIC);
> if (result)
> - pr_info("failed to send netlink event:%d\n", result);
> + dev_err(&tz->device, "Failed to send netlink event:%d", result);
>
> return result;
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device
2013-01-02 15:29 ` [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device Eduardo Valentin
2013-01-02 15:53 ` R, Durgadoss
@ 2013-01-16 2:45 ` Zhang Rui
2013-01-16 14:22 ` Eduardo Valentin
1 sibling, 1 reply; 16+ messages in thread
From: Zhang Rui @ 2013-01-16 2:45 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: durgadoss.r, linux-pm, linux-pm
On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote:
> This patch adds an extra check in the data structure while registering
> a thermal device. The check is to avoid registering zones with a number
> of trips greater than zero, but with no .get_trip_temp nor .get_trip_type
> callbacks. Receiving such data structure may end in wrong data access.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> drivers/thermal/thermal_sys.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index fba27c3..0a1bf6b 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1530,6 +1530,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
> if (!ops || !ops->get_temp)
> return ERR_PTR(-EINVAL);
>
> + if (trips > 0 && !ops->get_trip_type)
> + return ERR_PTR(-EINVAL);
> +
Hmmm, I do not think we need this.
the sysfs I/F trip_point_X_type already does this check.
thanks,
rui
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device
2013-01-16 2:45 ` Zhang Rui
@ 2013-01-16 14:22 ` Eduardo Valentin
2013-01-17 7:10 ` Zhang Rui
0 siblings, 1 reply; 16+ messages in thread
From: Eduardo Valentin @ 2013-01-16 14:22 UTC (permalink / raw)
To: Zhang Rui; +Cc: durgadoss.r, linux-pm, linux-pm
On 16-01-2013 04:45, Zhang Rui wrote:
> On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote:
>> This patch adds an extra check in the data structure while registering
>> a thermal device. The check is to avoid registering zones with a number
>> of trips greater than zero, but with no .get_trip_temp nor .get_trip_type
>> callbacks. Receiving such data structure may end in wrong data access.
>>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
>> drivers/thermal/thermal_sys.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> index fba27c3..0a1bf6b 100644
>> --- a/drivers/thermal/thermal_sys.c
>> +++ b/drivers/thermal/thermal_sys.c
>> @@ -1530,6 +1530,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>> if (!ops || !ops->get_temp)
>> return ERR_PTR(-EINVAL);
>>
>> + if (trips > 0 && !ops->get_trip_type)
>> + return ERR_PTR(-EINVAL);
>> +
> Hmmm, I do not think we need this.
>
> the sysfs I/F trip_point_X_type already does this check.
Well that's not the point. If you trust only the sysfs handling, fine.
The fix is very punctual. If you pass wrong parameters to
thermal_zone_device_register, it will crash.
Of course, in either case, with or without his patch, it won't register
the device, so you won't get to the sysfs handling part. At least with
this fix, the crash won't happen and the driver writer will receive an
error code to treat.
Ok. being specific, what happens is that there are accesses to
ops->get_trip_type, even if trips > 0 and if .get_trip_type is NULL,
right below inside the register function. Instead of checking for
.get_trip_type every time before using it, this patch adds a single
constraint, so that, if you have trips, you must specify their types..
To me sounds reasonable enough.
>
> thanks,
> rui
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device
2013-01-16 14:22 ` Eduardo Valentin
@ 2013-01-17 7:10 ` Zhang Rui
0 siblings, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2013-01-17 7:10 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: durgadoss.r, linux-pm, linux-pm
On Wed, 2013-01-16 at 16:22 +0200, Eduardo Valentin wrote:
> On 16-01-2013 04:45, Zhang Rui wrote:
> > On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote:
> >> This patch adds an extra check in the data structure while registering
> >> a thermal device. The check is to avoid registering zones with a number
> >> of trips greater than zero, but with no .get_trip_temp nor .get_trip_type
> >> callbacks. Receiving such data structure may end in wrong data access.
> >>
> >> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> >> ---
> >> drivers/thermal/thermal_sys.c | 3 +++
> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> >> index fba27c3..0a1bf6b 100644
> >> --- a/drivers/thermal/thermal_sys.c
> >> +++ b/drivers/thermal/thermal_sys.c
> >> @@ -1530,6 +1530,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
> >> if (!ops || !ops->get_temp)
> >> return ERR_PTR(-EINVAL);
> >>
> >> + if (trips > 0 && !ops->get_trip_type)
> >> + return ERR_PTR(-EINVAL);
> >> +
> > Hmmm, I do not think we need this.
> >
> > the sysfs I/F trip_point_X_type already does this check.
>
> Well that's not the point. If you trust only the sysfs handling, fine.
> The fix is very punctual. If you pass wrong parameters to
> thermal_zone_device_register, it will crash.
>
> Of course, in either case, with or without his patch, it won't register
> the device, so you won't get to the sysfs handling part. At least with
> this fix, the crash won't happen and the driver writer will receive an
> error code to treat.
>
> Ok. being specific, what happens is that there are accesses to
> ops->get_trip_type, even if trips > 0 and if .get_trip_type is NULL,
> right below inside the register function. Instead of checking for
> .get_trip_type every time before using it, this patch adds a single
> constraint, so that, if you have trips, you must specify their types..
> To me sounds reasonable enough.
>
agreed.
applied to thermal -next.
thanks,
rui
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-01-17 7:10 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-02 15:29 [PATCH RESEND 0/4] thermal sys: couple of fixes and cleanups Eduardo Valentin
2013-01-02 15:29 ` [PATCH RESEND 1/4] thermal: Use thermal zone device id in netlink messages Eduardo Valentin
2013-01-02 15:48 ` R, Durgadoss
2013-01-02 15:55 ` Eduardo Valentin
2013-01-16 2:43 ` Zhang Rui
2013-01-02 15:29 ` [PATCH RESEND 2/4] thermal: remove unnecessary include Eduardo Valentin
2013-01-02 15:48 ` R, Durgadoss
2013-01-16 2:43 ` Zhang Rui
2013-01-02 15:29 ` [PATCH RESEND 3/4] thermal: cleanup: use dev_* helper functions Eduardo Valentin
2013-01-02 15:51 ` R, Durgadoss
2013-01-16 2:44 ` Zhang Rui
2013-01-02 15:29 ` [PATCH RESEND 4/4] thermal: check for invalid trip setup when registering thermal device Eduardo Valentin
2013-01-02 15:53 ` R, Durgadoss
2013-01-16 2:45 ` Zhang Rui
2013-01-16 14:22 ` Eduardo Valentin
2013-01-17 7:10 ` Zhang Rui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).