Patch[1/1]:Adding_Netlink_Support_to_ThermalFramework

Patch[1/1]:Adding_Netlink_Support_to_ThermalFramework

[R, Durgadoss]

Hi All,

Here is a patch to add netlink event support to the thermal framework.

As we discussed earlier, the following are the major changes:
1) Created a separate file thermal_netlink.c under /thermal/.
2) Added documentation on the same under Doc../thermal/thermal_netlink.txt
	a)I have left the THERMAL_WARN/EMERG/CRIT cases for Rui to add details
	  on them.
3) As per suggestion from Henry and Thomas, Included a separate list for
   Devices (that can generate thermal events) and event types.
	a) I have added the devices/event types that I could think off.
	   Suggestions on this are most welcome.

Kindly review and let me know the comments.
---------------------------------------------------------------------
From: Durgadoss R <durgadoss.r@intel.com>

Date: Tue, 1 Feb 2011 00:06:02 +0530
Subject: [PATCH] Adding_Netlink_Support_to_ThermalFramework

This patch adds Netlink Event Notification support to the thermal
framework. Events are sent in the form of (originator, event). The list
of possible originators and events are added in thermal.h. A detailed
documentation for the same is added in Doc../thermal/thermal_netlink.txt

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>

---
 Documentation/thermal/thermal_netlink.txt |   45 ++++++++++++++
 drivers/thermal/Kconfig                   |    7 ++
 drivers/thermal/Makefile                  |    1 +
 drivers/thermal/thermal_netlink.c         |   95 +++++++++++++++++++++++++++++
 include/linux/thermal.h                   |   54 ++++++++++++++++
 5 files changed, 202 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/thermal/thermal_netlink.txt
 create mode 100644 drivers/thermal/thermal_netlink.c

diff --git a/Documentation/thermal/thermal_netlink.txt b/Documentation/thermal/thermal_netlink.txt
new file mode 100644
index 0000000..92e0900
--- /dev/null
+++ b/Documentation/thermal/thermal_netlink.txt
@@ -0,0 +1,45 @@
+Event Notification Support
+==========================
+
+0. Introduction
+---------------
+The generic thermal sysfs provides a set of interfaces for thermal zone
+devices (sensors) and thermal cooling devices (fan, processor...) to register
+with the thermal management solution and to be a part of it.
+
+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
+must register with the thermal framework. Once registered, they will be
+assigned an unique id. They can call thermal_netlink_event() to send
+netlink events.
+
+This thermal_netlink_event function takes two arguments:
+	1.Originator id
+	2.Event Type
+
+The originator should be one of the devices added in the thermal_devices
+enum in the thermal.h. Likewise, the list of events have also been added
+as an enum in thermal.h.
+
+1. Events:
+----------
+The following are the list of events that the notification mechanism supports:
+1.THERMAL_WARN
+2.THERMAL_EMERG
+3.THERMAL_CRIT
+4.THERMAL_CT_AUX0
+    The coretemp module supports two thresholds for thermal management,
+    through IA32_THERM_INTERRUPT register. One of these thresholds is
+    lower than the current temperature and the other higher. When the CPU
+    temperature sensor crosses the programmed lower threshold, THERMAL_CT_AUX0
+    event is generated.
+5.THERMAL_CT_AUX1
+    This event is generated when the CPU temperature Sensor crosses the
+    programmed higher threshold limit.
+6.THERMAL_PT_AUX0
+    This is similar to the THERMAL_CT_AUX0, except that it is generated by
+    the package temperature sensor.
+7.THERMAL_PT_AUX1
+    This is similar to the THERMAL_CT_AUX1, except that it is generated by
+    the package temperature sensor.
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index bf7c687..7085794 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -22,3 +22,10 @@ config THERMAL_HWMON
 	  requires a 2.10.7/3.0.2 or later lm-sensors userspace.
 
 	  Say Y if your user-space is new enough.
+
+config THERMAL_NETLINK
+	tristate "Netlink Interface to Notify Thermal Events"
+	depends on THERMAL && NET
+	help
+	  Say Y here, if you want the thermal events to be notified via
+	  netlink events.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 31108a0..9ee8380 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_THERMAL)		+= thermal_sys.o
+obj-$(CONFIG_THERMAL_NETLINK)	+= thermal_netlink.o
diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
new file mode 100644
index 0000000..1678a13
--- /dev/null
+++ b/drivers/thermal/thermal_netlink.c
@@ -0,0 +1,95 @@
+#include <net/genetlink.h>
+#include <linux/thermal.h>
+
+static unsigned int thermal_event_seqnum;
+
+static struct genl_family thermal_event_genl_family = {
+	.id = GENL_ID_GENERATE,
+	.name = THERMAL_GENL_FAMILY_NAME,
+	.version = THERMAL_GENL_VERSION,
+	.maxattr = THERMAL_GENL_ATTR_MAX,
+};
+
+static struct genl_multicast_group thermal_event_mcgrp = {
+	.name = THERMAL_GENL_MCAST_GROUP_NAME,
+};
+
+int thermal_netlink_event(enum thermal_devices orig, enum thermal_events event)
+{
+	struct sk_buff *skb;
+	struct nlattr *attr;
+	struct thermal_genl_event *thermal_event;
+	void *msg_header;
+	int size;
+	int result;
+
+	/* allocate memory */
+	size = nla_total_size(sizeof(struct thermal_genl_event)) + \
+				nla_total_size(0);
+
+	skb = genlmsg_new(size, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	/* add the genetlink message header */
+	msg_header = genlmsg_put(skb, 0, thermal_event_seqnum++,
+				 &thermal_event_genl_family, 0,
+				 THERMAL_GENL_CMD_EVENT);
+	if (!msg_header) {
+		nlmsg_free(skb);
+		return -ENOMEM;
+	}
+
+	/* fill the data */
+	attr = nla_reserve(skb, THERMAL_GENL_ATTR_EVENT, \
+			sizeof(struct thermal_genl_event));
+
+	if (!attr) {
+		nlmsg_free(skb);
+		return -EINVAL;
+	}
+
+	thermal_event = nla_data(attr);
+	if (!thermal_event) {
+		nlmsg_free(skb);
+		return -EINVAL;
+	}
+
+	memset(thermal_event, 0, sizeof(struct thermal_genl_event));
+
+	thermal_event->device = orig;
+	thermal_event->event = event;
+
+	/* send multicast genetlink message */
+	result = genlmsg_end(skb, msg_header);
+	if (result < 0) {
+		nlmsg_free(skb);
+		return result;
+	}
+
+	return genlmsg_multicast(skb, 0, thermal_event_mcgrp.id, GFP_ATOMIC);
+}
+
+static int __init thermal_netlink_init(void)
+{
+	int result;
+
+	result = genl_register_family(&thermal_event_genl_family);
+	if (result)
+		return result;
+
+	result = genl_register_mc_group(&thermal_event_genl_family,
+					&thermal_event_mcgrp);
+	if (result)
+		genl_unregister_family(&thermal_event_genl_family);
+	return result;
+}
+
+static void __exit thermal_netlink_exit(void)
+{
+	genl_unregister_family(&thermal_event_genl_family);
+}
+
+subsys_initcall(thermal_netlink_init);
+module_exit(thermal_netlink_exit);
+
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 1de8b9e..bd79d18 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -147,4 +147,58 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
 							       *);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
 
+#ifdef CONFIG_NET
+
+/* Add Netlink Notification Support Elements */
+#define THERMAL_GENL_FAMILY_NAME                "thermal_event"
+#define THERMAL_GENL_VERSION                    0x01
+#define THERMAL_GENL_MCAST_GROUP_NAME           "thermal_mc_group"
+
+enum thermal_events {
+	/* generic thermal events */
+	THERMAL_WARN,
+	THERMAL_EMERG,
+	THERMAL_CRIT,
+	/* coretemp thermal events */
+	THERMAL_CT_AUX0,
+	THERMAL_CT_AUX1,
+	/* pkgtemp thermal events */
+	THERMAL_PT_AUX0,
+	THERMAL_PT_AUX1,
+	/* Add other events, if any */
+};
+
+enum thermal_devices {
+	/* For multi-core CPUs */
+	THERMAL_CPU_CORE0 = 0,
+	THERMAL_GPU = 32,
+	THERMAL_BATTERY,
+	/* Add other devices, if any */
+};
+
+struct thermal_genl_event {
+	enum thermal_devices device;
+	enum thermal_events event;
+};
+
+/* attributes of thermal_genl_family */
+enum {
+	THERMAL_GENL_ATTR_UNSPEC,
+	THERMAL_GENL_ATTR_EVENT,
+	__THERMAL_GENL_ATTR_MAX,
+};
+#define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
+
+/* commands supported by the thermal_genl_family */
+enum {
+	THERMAL_GENL_CMD_UNSPEC,
+	THERMAL_GENL_CMD_EVENT,
+	__THERMAL_GENL_CMD_MAX,
+};
+#define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
+
+int thermal_netlink_event(enum thermal_devices, enum thermal_events);
+
+#endif /* CONFIG_NET */
+
 #endif /* __THERMAL_H__ */
-- 
1.6.5.2

Re: Patch[1/1]:Adding_Netlink_Support_to_ThermalFramework

[Thomas Renninger]

Hi,

some quick comments:

> +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
> +must register with the thermal framework. Once registered, they will be
> +assigned an unique id. They can call thermal_netlink_event() to send
> +netlink events.
> +
> +This thermal_netlink_event function takes two arguments:
> +	1.Originator id
My idea was: pass the thermal_zone_device id, compare with:
include/linux/thermal.h:
struct thermal_zone_device {
        int id;
I expect it's used to create the sysfs directory:
/sys/devices/virtual/thermal/thermal_zone0
-> zero in the end should be the id (I did not verify this).

> +
> +enum thermal_devices {
> +	/* For multi-core CPUs */
> +	THERMAL_CPU_CORE0 = 0,
is this a placeholder for 32 cores? That's not much and not
generic enough. CPU should be enough. There also should
be THERMAL_DEVICE_UNKNOWN.
> +	THERMAL_GPU = 32,
> +	THERMAL_BATTERY,
> +	/* Add other devices, if any */
> +};
> +
> +struct thermal_genl_event {
> +	enum thermal_devices device;
> +	enum thermal_events event;
> +};
> +
> +/* attributes of thermal_genl_family */
> +enum {
> +	THERMAL_GENL_ATTR_UNSPEC,
> +	THERMAL_GENL_ATTR_EVENT,
> +	__THERMAL_GENL_ATTR_MAX,
> +};
> +#define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
> +
> +/* commands supported by the thermal_genl_family */
> +enum {
> +	THERMAL_GENL_CMD_UNSPEC,
> +	THERMAL_GENL_CMD_EVENT,
> +	__THERMAL_GENL_CMD_MAX,
> +};
> +#define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
> +
> +int thermal_netlink_event(enum thermal_devices, enum thermal_events);
not sure what is best, an int or even better request:
struct thermal_zone_device* and then pass the ->id (additionally?)
to userspace. I see that there might be drivers which cannot register as a
thermal driver (e.g. getting an emergency notification from BIOS and no temperature
or thermal trip points at all) and that there might be exceptions needed for the
future, they could pass NULL or a specific value so that userspace knows it
cannot read up further info from /sys/devices/virtual/thermal/thermal_zone-1.

Just some thoughts, I somehow like the idea of userspace being
able to look up the thermal device in sysfs which has thrown the event.

I try to find some time to read into the user definable trip points
(THERMAL_CT_AUX1,..) and might be able to give more feedback then.

About the userspace tool that should pick up these events,
is this integrated into an existing project, is there
code already you could point to?

Thanks,

          Thomas

RE: Patch[1/1]:Adding_Netlink_Support_to_ThermalFramework

[R, Durgadoss]

Hi Thomas,

> some quick comments:
> 
> > +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
> > +must register with the thermal framework. Once registered, they will be
> > +assigned an unique id. They can call thermal_netlink_event() to send
> > +netlink events.
> > +
> > +This thermal_netlink_event function takes two arguments:
> > +	1.Originator id
> My idea was: pass the thermal_zone_device id, compare with:
> include/linux/thermal.h:
> struct thermal_zone_device {
>         int id;
> I expect it's used to create the sysfs directory:
> /sys/devices/virtual/thermal/thermal_zone0
> -> zero in the end should be the id (I did not verify this).

I completely agree with this idea. I was waiting to see Jean's reply
On the other list, about making it register as a thermal driver.

> > +
> > +enum thermal_devices {
> > +	/* For multi-core CPUs */
> > +	THERMAL_CPU_CORE0 = 0,
> is this a placeholder for 32 cores? That's not much and not
> generic enough. CPU should be enough. There also should
> be THERMAL_DEVICE_UNKNOWN.

Ok. Shall change.

> > +	THERMAL_GPU = 32,
> > +	THERMAL_BATTERY,
> > +	/* Add other devices, if any */
> > +};
> > +
> > +struct thermal_genl_event {
> > +	enum thermal_devices device;
> > +	enum thermal_events event;
> > +};
> > +
> > +/* attributes of thermal_genl_family */
> > +enum {
> > +	THERMAL_GENL_ATTR_UNSPEC,
> > +	THERMAL_GENL_ATTR_EVENT,
> > +	__THERMAL_GENL_ATTR_MAX,
> > +};
> > +#define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
> > +
> > +/* commands supported by the thermal_genl_family */
> > +enum {
> > +	THERMAL_GENL_CMD_UNSPEC,
> > +	THERMAL_GENL_CMD_EVENT,
> > +	__THERMAL_GENL_CMD_MAX,
> > +};
> > +#define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
> > +
> > +int thermal_netlink_event(enum thermal_devices, enum thermal_events);
> not sure what is best, an int or even better request:
> struct thermal_zone_device* and then pass the ->id (additionally?)
> to userspace. I see that there might be drivers which cannot register as a
> thermal driver (e.g. getting an emergency notification from BIOS and no
> temperature
> or thermal trip points at all) and that there might be exceptions needed for
> the
> future, they could pass NULL or a specific value so that userspace knows it
> cannot read up further info from /sys/devices/virtual/thermal/thermal_zone-1.
> 
> Just some thoughts, I somehow like the idea of userspace being
> able to look up the thermal device in sysfs which has thrown the event.

Ok then I will make the first argument as the thermal_zone->id.

> 
> I try to find some time to read into the user definable trip points
> (THERMAL_CT_AUX1,..) and might be able to give more feedback then.
>

IMHO, you can refer to the IA32 Software developer's manual,(a pdf..
freely downloadable) to get a complete picture.

> About the userspace tool that should pick up these events,
> is this integrated into an existing project, is there
> code already you could point to?
> 

We are working on a Thermal Monitoring Agent for the MeeGo Distribution.
But there is no reason why it would not work on other distros.

Thanks,
Durga