[PATCH 0/4] thermal sys: couple of fixes and cleanups

[PATCH 0/4] thermal sys: couple of fixes and cleanups

[Eduardo Valentin]

Hello Rui and thermal folks,

Here is a couple of fixes and cleanups on the thermal_sys.c.

One important change is the netlink channel, which I am proposing
to have the zone id in each message. Rui, do you know if this could
cause a breakage on userspace applications?

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 sys: check for invalid trip setup when registering thermal
    device

 Documentation/thermal/sysfs-api.txt |    5 +++--
 drivers/thermal/thermal_sys.c       |   23 ++++++++++++++++-------
 include/linux/thermal.h             |    6 ++++--
 3 files changed, 23 insertions(+), 11 deletions(-)

-- 
1.7.7.1.488.ge8e1c

[RFC] [PATCH 1/4] thermal: Use thermal zone device id in netlink messages

[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 1733ab9..670644a 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -285,8 +285,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 022bacb..650cefb 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1287,7 +1287,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;
@@ -1297,6 +1298,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);
@@ -1331,7 +1335,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 796f1ff..b754203 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -152,9 +152,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
 
 #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

Re: [RFC] [PATCH 1/4] thermal: Use thermal zone device id in netlink messages

[R, Durgadoss]

Hi Eduardo,

> 
> 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.

I agree with you on this patch implementation.

But I am thinking most user space Apps today are using UEvent based
mechanism, may be it is time for us to re-visit this implemention.
There are standard UEvent parsers available which can be used.
(instead of writing custom netlink parsing implementations)

I submitted this patch an year ago, (inspired by some acpi driver,
which uses this same netlink mechanism) and have a couple of
platforms using this. I can change them for my side.

I don't know what other platforms are using this. Need Rui's
help/suggestion here.

Thanks,
Durga

Re: [RFC] [PATCH 1/4] thermal: Use thermal zone device id in netlink messages

[Valentin, Eduardo]

Hello Durga,

On Tue, Jun 12, 2012 at 8:02 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
> Hi Eduardo,
>
>>
>> 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.
>
> I agree with you on this patch implementation.
>

Ok..

> But I am thinking most user space Apps today are using UEvent based
> mechanism, may be it is time for us to re-visit this implemention.
> There are standard UEvent parsers available which can be used.
> (instead of writing custom netlink parsing implementations)

Right... But do we have any legacy application relying on this netlink channel?

>
> I submitted this patch an year ago, (inspired by some acpi driver,
> which uses this same netlink mechanism) and have a couple of
> platforms using this. I can change them for my side.

Oh.. Ok.  Well, on my side, I don't really have any application
relying on it. I just want to make sure we don't break things while
changing the current implementation. If you are OK to change your
applications then fine. We can also, introduce the sysfs notify and
mark the netlink as deprecated, while the applications are being
modified. Then finally remove it.

>
> I don't know what other platforms are using this. Need Rui's
> help/suggestion here.

Indeed. Rui?

>
> Thanks,
> Durga

All best,

-- 

Eduardo Valentin

[PATCH 2/4] thermal: remove unnecessary include

[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 650cefb..1ef9fc1 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

[PATCH 3/4] thermal: cleanup: use dev_* helper functions

[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 |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 1ef9fc1..61d0003 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1022,7 +1022,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 
 	if (tz->ops->get_temp(tz, &temp)) {
 		/* get_temp failed - retry it later */
-		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 leave;
 	}
 
@@ -1037,8 +1038,10 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);
 				if (!ret) {
-					pr_emerg("Critical temperature reached (%ld C), shutting down\n",
-						 temp/1000);
+					dev_emerg(&tz->device,
+						  "Critical temperature reached"
+						  " (%ld C), shutting down.\n",
+						  temp/1000);
 					orderly_poweroff(true);
 				}
 			}
@@ -1346,7 +1349,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

[PATCH 4/4] thermal sys: check for invalid trip setup when registering thermal device

[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 61d0003..50830f2 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1129,6 +1129,9 @@ struct thermal_zone_device *thermal_zone_device_register(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