All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/4] thermal: core: Add helpers to browse the cdev, tz and governor list
@ 2020-07-06 10:55 Daniel Lezcano
  2020-07-06 10:55 ` [PATCH v4 2/4] thermal: core: Get thermal zone by id Daniel Lezcano
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-06 10:55 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

The cdev, tz and governor list, as well as their respective locks are
statically defined in the thermal_core.c file.

In order to give a sane access to these list, like browsing all the
thermal zones or all the cooling devices, let's define a set of
helpers where we pass a callback as a parameter to be called for each
thermal entity.

We keep the self-encapsulation and ensure the locks are correctly
taken when looking at the list.

Acked-by: Zhang Rui <rui.zhang@intel.com>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/thermal_core.c | 51 ++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_core.h |  9 ++++++
 2 files changed, 60 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index b71196eaf90e..9caaa0b6d662 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -617,6 +617,57 @@ void thermal_zone_device_rebind_exception(struct thermal_zone_device *tz,
 	mutex_unlock(&thermal_list_lock);
 }
 
+int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *),
+			      void *data)
+{
+	struct thermal_governor *gov;
+	int ret = 0;
+
+	mutex_lock(&thermal_governor_lock);
+	list_for_each_entry(gov, &thermal_governor_list, governor_list) {
+		ret = cb(gov, data);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&thermal_governor_lock);
+
+	return ret;
+}
+
+int for_each_thermal_cooling_device(int (*cb)(struct thermal_cooling_device *,
+					      void *), void *data)
+{
+	struct thermal_cooling_device *cdev;
+	int ret = 0;
+
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(cdev, &thermal_cdev_list, node) {
+		ret = cb(cdev, data);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&thermal_list_lock);
+
+	return ret;
+}
+
+int for_each_thermal_zone(int (*cb)(struct thermal_zone_device *, void *),
+			  void *data)
+{
+	struct thermal_zone_device *tz;
+	int ret = 0;
+
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(tz, &thermal_tz_list, node) {
+		ret = cb(tz, data);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&thermal_list_lock);
+
+	return ret;
+}
+
 void thermal_zone_device_unbind_exception(struct thermal_zone_device *tz,
 					  const char *cdev_type, size_t size)
 {
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index c95689586e19..71d88dac0791 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -41,6 +41,15 @@ extern struct thermal_governor *__governor_thermal_table_end[];
 	     __governor < __governor_thermal_table_end;	\
 	     __governor++)
 
+int for_each_thermal_zone(int (*cb)(struct thermal_zone_device *, void *),
+			  void *);
+
+int for_each_thermal_cooling_device(int (*cb)(struct thermal_cooling_device *,
+					      void *), void *);
+
+int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *),
+			      void *thermal_governor);
+
 struct thermal_attr {
 	struct device_attribute attr;
 	char name[THERMAL_NAME_LENGTH];
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 2/4] thermal: core: Get thermal zone by id
  2020-07-06 10:55 [PATCH v4 1/4] thermal: core: Add helpers to browse the cdev, tz and governor list Daniel Lezcano
@ 2020-07-06 10:55 ` Daniel Lezcano
  2020-07-07  1:53   ` Zhang Rui
  2020-07-06 10:55 ` [PATCH v4 3/4] thermal: core: genetlink support for events/cmd/sampling Daniel Lezcano
  2020-07-06 10:55 ` [PATCH v4 4/4] thermal: core: Add notifications call in the framework Daniel Lezcano
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-06 10:55 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

The next patch will introduce the generic netlink protocol to handle
events, sampling and command from the thermal framework. In order to
deal with the thermal zone, it uses its unique identifier to
characterize it in the message. Passing an integer is more efficient
than passing an entire string.

This change provides a function returning back a thermal zone pointer
corresponding to the identifier passed as parameter.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 drivers/thermal/thermal_core.c | 14 ++++++++++++++
 drivers/thermal/thermal_core.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 9caaa0b6d662..5fae1621fb01 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -668,6 +668,20 @@ int for_each_thermal_zone(int (*cb)(struct thermal_zone_device *, void *),
 	return ret;
 }
 
+struct thermal_zone_device *thermal_zone_get_by_id(int id)
+{
+	struct thermal_zone_device *tz = NULL;
+
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(tz, &thermal_tz_list, node) {
+		if (tz->id == id)
+			break;
+	}
+	mutex_unlock(&thermal_list_lock);
+
+	return tz;
+}
+
 void thermal_zone_device_unbind_exception(struct thermal_zone_device *tz,
 					  const char *cdev_type, size_t size)
 {
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 71d88dac0791..4f8389efaa62 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -50,6 +50,8 @@ int for_each_thermal_cooling_device(int (*cb)(struct thermal_cooling_device *,
 int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *),
 			      void *thermal_governor);
 
+struct thermal_zone_device *thermal_zone_get_by_id(int id);
+
 struct thermal_attr {
 	struct device_attribute attr;
 	char name[THERMAL_NAME_LENGTH];
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 3/4] thermal: core: genetlink support for events/cmd/sampling
  2020-07-06 10:55 [PATCH v4 1/4] thermal: core: Add helpers to browse the cdev, tz and governor list Daniel Lezcano
  2020-07-06 10:55 ` [PATCH v4 2/4] thermal: core: Get thermal zone by id Daniel Lezcano
@ 2020-07-06 10:55 ` Daniel Lezcano
  2020-07-07  1:54   ` Zhang Rui
  2020-07-06 10:55 ` [PATCH v4 4/4] thermal: core: Add notifications call in the framework Daniel Lezcano
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-06 10:55 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

Initially the thermal framework had a very simple notification
mechanism to send generic netlink messages to the userspace.

The notification function was never called from anywhere and the
corresponding dead code was removed. It was probably a first attempt
to introduce the netlink notification.

At LPC2018, the presentation "Linux thermal: User kernel interface",
proposed to create the notifications to the userspace via a kfifo.

The advantage of the kfifo is the performance. It is usually used from
a 1:1 communication channel where a driver captures data and sends it
as fast as possible to a userspace process.

The drawback is that only one process uses the notification channel
exclusively, thus no other process is allowed to use the channel to
get temperature or notifications.

This patch defines a generic netlink API to discover the current
thermal setup and adds event notifications as well as temperature
sampling. As any genetlink protocol, it can evolve and the versioning
allows to keep the backward compatibility.

In order to prevent the user from getting flooded with data on a
single channel, there are two multicast channels, one for the
temperature sampling when the thermal zone is updated and another one
for the events, so the user can get the events only without the
thermal zone temperature sampling.

Also, a list of commands to discover the thermal setup is added and
can be extended when needed.

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
  v4:
      - Removed max cdev state and renamed function and attributes
      	with CDEV_STATE_UPDATE
  v3:
      - Fixed changelog from Amit Kucheria suggestions
      - Prefixed fields in the parameter structure (trip_*, cdev_*)
      - Fixed leading whitespaces errors
      - Replaced id by trip_id
      - s/THERMAL_GENL_CMD_TZ_GET/THERMAL_GENL_CMD_TZ_GET_ID/
      - Added the cdev max state in the cdev change event
      - Removed min state
      - Fixed checkpatch warnings
---
---
 drivers/thermal/Makefile          |   2 +-
 drivers/thermal/thermal_core.h    |  18 +
 drivers/thermal/thermal_netlink.c | 648 ++++++++++++++++++++++++++++++
 include/linux/thermal.h           |  17 -
 include/uapi/linux/thermal.h      |  89 +++-
 5 files changed, 739 insertions(+), 35 deletions(-)
 create mode 100644 drivers/thermal/thermal_netlink.c

diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 0c8b84a09b9a..1bbf0805fb04 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 thermal_sys-y			+= thermal_core.o thermal_sysfs.o \
-					thermal_helpers.o
+					thermal_helpers.o thermal_netlink.o
 
 # interface to/from other layers providing sensors
 thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 4f8389efaa62..b44969d50ec0 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -52,6 +52,24 @@ int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *),
 
 struct thermal_zone_device *thermal_zone_get_by_id(int id);
 
+/* Netlink notification function */
+int thermal_notify_tz_create(int tz_id, const char *name);
+int thermal_notify_tz_delete(int tz_id);
+int thermal_notify_tz_enable(int tz_id);
+int thermal_notify_tz_disable(int tz_id);
+int thermal_notify_tz_trip_down(int tz_id, int id);
+int thermal_notify_tz_trip_up(int tz_id, int id);
+int thermal_notify_tz_trip_delete(int tz_id, int id);
+int thermal_notify_tz_trip_add(int tz_id, int id, int type,
+			       int temp, int hyst);
+int thermal_notify_tz_trip_change(int tz_id, int id, int type,
+				  int temp, int hyst);
+int thermal_notify_cdev_state_update(int cdev_id, int state);
+int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state);
+int thermal_notify_cdev_delete(int cdev_id);
+int thermal_notify_tz_gov_change(int tz_id, const char *name);
+int thermal_genl_sampling_temp(int id, int temp);
+
 struct thermal_attr {
 	struct device_attribute attr;
 	char name[THERMAL_NAME_LENGTH];
diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
new file mode 100644
index 000000000000..dd0a3b889674
--- /dev/null
+++ b/drivers/thermal/thermal_netlink.c
@@ -0,0 +1,648 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020 Linaro Limited
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ * Generic netlink for thermal management framework
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <net/genetlink.h>
+#include <uapi/linux/thermal.h>
+
+#include "thermal_core.h"
+
+static const struct genl_multicast_group thermal_genl_mcgrps[] = {
+	{ .name = THERMAL_GENL_SAMPLING_GROUP_NAME, },
+	{ .name = THERMAL_GENL_EVENT_GROUP_NAME,  },
+};
+
+static const struct nla_policy thermal_genl_policy[THERMAL_GENL_ATTR_MAX + 1] = {
+	/* Thermal zone */
+	[THERMAL_GENL_ATTR_TZ]			= { .type = NLA_NESTED },
+	[THERMAL_GENL_ATTR_TZ_ID]		= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_TEMP]		= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_TRIP]		= { .type = NLA_NESTED },
+	[THERMAL_GENL_ATTR_TZ_TRIP_ID]		= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_TRIP_TEMP]	= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_TRIP_TYPE]	= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_TRIP_HYST]	= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_MODE]		= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_CDEV_WEIGHT]	= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_TZ_NAME]		= { .type = NLA_STRING,
+						    .len = THERMAL_NAME_LENGTH },
+	/* Governor(s) */
+	[THERMAL_GENL_ATTR_TZ_GOV]		= { .type = NLA_NESTED },
+	[THERMAL_GENL_ATTR_TZ_GOV_NAME]		= { .type = NLA_STRING,
+						    .len = THERMAL_NAME_LENGTH },
+	/* Cooling devices */
+	[THERMAL_GENL_ATTR_CDEV]		= { .type = NLA_NESTED },
+	[THERMAL_GENL_ATTR_CDEV_ID]		= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_CDEV_CUR_STATE]	= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_CDEV_MAX_STATE]	= { .type = NLA_U32 },
+	[THERMAL_GENL_ATTR_CDEV_NAME]		= { .type = NLA_STRING,
+						    .len = THERMAL_NAME_LENGTH },
+};
+
+struct param {
+	struct nlattr **attrs;
+	struct sk_buff *msg;
+	const char *name;
+	int tz_id;
+	int cdev_id;
+	int trip_id;
+	int trip_temp;
+	int trip_type;
+	int trip_hyst;
+	int temp;
+	int cdev_state;
+	int cdev_max_state;
+};
+
+typedef int (*cb_t)(struct param *);
+
+static struct genl_family thermal_gnl_family;
+
+/************************** Sampling encoding *******************************/
+
+int thermal_genl_sampling_temp(int id, int temp)
+{
+	struct sk_buff *skb;
+	void *hdr;
+
+	skb = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, 0, 0, &thermal_gnl_family, 0,
+			  THERMAL_GENL_SAMPLING_TEMP);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(skb, THERMAL_GENL_ATTR_TZ_ID, id))
+		goto out_cancel;
+
+	if (nla_put_u32(skb, THERMAL_GENL_ATTR_TZ_TEMP, temp))
+		goto out_cancel;
+
+	genlmsg_end(skb, hdr);
+
+	genlmsg_multicast(&thermal_gnl_family, skb, 0, 0, GFP_KERNEL);
+
+	return 0;
+out_cancel:
+	genlmsg_cancel(skb, hdr);
+	nlmsg_free(skb);
+
+	return -EMSGSIZE;
+}
+
+/**************************** Event encoding *********************************/
+
+static int thermal_genl_event_tz_create(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
+	    nla_put_string(p->msg, THERMAL_GENL_ATTR_TZ_NAME, p->name))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_tz(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_tz_trip_up(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, p->trip_id))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_tz_trip_add(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, p->trip_id) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, p->trip_type) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, p->trip_temp) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, p->trip_hyst))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_tz_trip_delete(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, p->trip_id))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_cdev_add(struct param *p)
+{
+	if (nla_put_string(p->msg, THERMAL_GENL_ATTR_CDEV_NAME,
+			   p->name) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID,
+			p->cdev_id) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_MAX_STATE,
+			p->cdev_max_state))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_cdev_delete(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, p->cdev_id))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_cdev_state_update(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID,
+			p->cdev_id) ||
+	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_CUR_STATE,
+			p->cdev_state))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_event_gov_change(struct param *p)
+{
+	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
+	    nla_put_string(p->msg, THERMAL_GENL_ATTR_GOV_NAME, p->name))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+int thermal_genl_event_tz_delete(struct param *p)
+	__attribute__((alias("thermal_genl_event_tz")));
+
+int thermal_genl_event_tz_enable(struct param *p)
+	__attribute__((alias("thermal_genl_event_tz")));
+
+int thermal_genl_event_tz_disable(struct param *p)
+	__attribute__((alias("thermal_genl_event_tz")));
+
+int thermal_genl_event_tz_trip_down(struct param *p)
+	__attribute__((alias("thermal_genl_event_tz_trip_up")));
+
+int thermal_genl_event_tz_trip_change(struct param *p)
+	__attribute__((alias("thermal_genl_event_tz_trip_add")));
+
+static cb_t event_cb[] = {
+	[THERMAL_GENL_EVENT_TZ_CREATE]		= thermal_genl_event_tz_create,
+	[THERMAL_GENL_EVENT_TZ_DELETE]		= thermal_genl_event_tz_delete,
+	[THERMAL_GENL_EVENT_TZ_ENABLE]		= thermal_genl_event_tz_enable,
+	[THERMAL_GENL_EVENT_TZ_DISABLE]		= thermal_genl_event_tz_disable,
+	[THERMAL_GENL_EVENT_TZ_TRIP_UP]		= thermal_genl_event_tz_trip_up,
+	[THERMAL_GENL_EVENT_TZ_TRIP_DOWN]	= thermal_genl_event_tz_trip_down,
+	[THERMAL_GENL_EVENT_TZ_TRIP_CHANGE]	= thermal_genl_event_tz_trip_change,
+	[THERMAL_GENL_EVENT_TZ_TRIP_ADD]	= thermal_genl_event_tz_trip_add,
+	[THERMAL_GENL_EVENT_TZ_TRIP_DELETE]	= thermal_genl_event_tz_trip_delete,
+	[THERMAL_GENL_EVENT_CDEV_ADD]		= thermal_genl_event_cdev_add,
+	[THERMAL_GENL_EVENT_CDEV_DELETE]	= thermal_genl_event_cdev_delete,
+	[THERMAL_GENL_EVENT_CDEV_STATE_UPDATE]	= thermal_genl_event_cdev_state_update,
+	[THERMAL_GENL_EVENT_TZ_GOV_CHANGE]	= thermal_genl_event_gov_change,
+};
+
+/*
+ * Generic netlink event encoding
+ */
+static int thermal_genl_send_event(enum thermal_genl_event event,
+				   struct param *p)
+{
+	struct sk_buff *msg;
+	int ret = -EMSGSIZE;
+	void *hdr;
+
+	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+	p->msg = msg;
+
+	hdr = genlmsg_put(msg, 0, 0, &thermal_gnl_family, 0, event);
+	if (!hdr)
+		goto out_free_msg;
+
+	ret = event_cb[event](p);
+	if (ret)
+		goto out_cancel_msg;
+
+	genlmsg_end(msg, hdr);
+
+	genlmsg_multicast(&thermal_gnl_family, msg, 0, 1, GFP_KERNEL);
+
+	return 0;
+
+out_cancel_msg:
+	genlmsg_cancel(msg, hdr);
+out_free_msg:
+	nlmsg_free(msg);
+
+	return ret;
+}
+
+int thermal_notify_tz_create(int tz_id, const char *name)
+{
+	struct param p = { .tz_id = tz_id, .name = name };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_CREATE, &p);
+}
+
+int thermal_notify_tz_delete(int tz_id)
+{
+	struct param p = { .tz_id = tz_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_DELETE, &p);
+}
+
+int thermal_notify_tz_enable(int tz_id)
+{
+	struct param p = { .tz_id = tz_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_ENABLE, &p);
+}
+
+int thermal_notify_tz_disable(int tz_id)
+{
+	struct param p = { .tz_id = tz_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_DISABLE, &p);
+}
+
+int thermal_notify_tz_trip_down(int tz_id, int trip_id)
+{
+	struct param p = { .tz_id = tz_id, .trip_id = trip_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DOWN, &p);
+}
+
+int thermal_notify_tz_trip_up(int tz_id, int trip_id)
+{
+	struct param p = { .tz_id = tz_id, .trip_id = trip_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_UP, &p);
+}
+
+int thermal_notify_tz_trip_add(int tz_id, int trip_id, int trip_type,
+			       int trip_temp, int trip_hyst)
+{
+	struct param p = { .tz_id = tz_id, .trip_id = trip_id,
+			   .trip_type = trip_type, .trip_temp = trip_temp,
+			   .trip_hyst = trip_hyst };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_ADD, &p);
+}
+
+int thermal_notify_tz_trip_delete(int tz_id, int trip_id)
+{
+	struct param p = { .tz_id = tz_id, .trip_id = trip_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DELETE, &p);
+}
+
+int thermal_notify_tz_trip_change(int tz_id, int trip_id, int trip_type,
+				  int trip_temp, int trip_hyst)
+{
+	struct param p = { .tz_id = tz_id, .trip_id = trip_id,
+			   .trip_type = trip_type, .trip_temp = trip_temp,
+			   .trip_hyst = trip_hyst };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p);
+}
+
+int thermal_notify_cdev_state_update(int cdev_id, int cdev_state)
+{
+	struct param p = { .cdev_id = cdev_id, .cdev_state = cdev_state };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, &p);
+}
+
+int thermal_notify_cdev_add(int cdev_id, const char *name, int cdev_max_state)
+{
+	struct param p = { .cdev_id = cdev_id, .name = name,
+			   .cdev_max_state = cdev_max_state };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_ADD, &p);
+}
+
+int thermal_notify_cdev_delete(int cdev_id)
+{
+	struct param p = { .cdev_id = cdev_id };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_DELETE, &p);
+}
+
+int thermal_notify_tz_gov_change(int tz_id, const char *name)
+{
+	struct param p = { .tz_id = tz_id, .name = name };
+
+	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_GOV_CHANGE, &p);
+}
+
+/*************************** Command encoding ********************************/
+
+static int __thermal_genl_cmd_tz_get_id(struct thermal_zone_device *tz,
+					void *data)
+{
+	struct sk_buff *msg = data;
+
+	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, tz->id) ||
+	    nla_put_string(msg, THERMAL_GENL_ATTR_TZ_NAME, tz->type))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_cmd_tz_get_id(struct param *p)
+{
+	struct sk_buff *msg = p->msg;
+	struct nlattr *start_tz;
+	int ret;
+
+	start_tz = nla_nest_start(msg, THERMAL_GENL_ATTR_TZ);
+	if (!start_tz)
+		return -EMSGSIZE;
+
+	ret = for_each_thermal_zone(__thermal_genl_cmd_tz_get_id, msg);
+	if (ret)
+		goto out_cancel_nest;
+
+	nla_nest_end(msg, start_tz);
+
+	return 0;
+
+out_cancel_nest:
+	nla_nest_cancel(msg, start_tz);
+
+	return ret;
+}
+
+static int thermal_genl_cmd_tz_get_trip(struct param *p)
+{
+	struct sk_buff *msg = p->msg;
+	struct thermal_zone_device *tz;
+	struct nlattr *start_trip;
+	int i, id;
+
+	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
+		return -EINVAL;
+
+	id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]);
+
+	tz = thermal_zone_get_by_id(id);
+	if (!tz)
+		return -EINVAL;
+
+	start_trip = nla_nest_start(msg, THERMAL_GENL_ATTR_TZ_TRIP);
+	if (!start_trip)
+		return -EMSGSIZE;
+
+	mutex_lock(&tz->lock);
+
+	for (i = 0; i < tz->trips; i++) {
+
+		enum thermal_trip_type type;
+		int temp, hyst;
+
+		tz->ops->get_trip_type(tz, i, &type);
+		tz->ops->get_trip_temp(tz, i, &temp);
+		tz->ops->get_trip_hyst(tz, i, &hyst);
+
+		if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, i) ||
+		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, type) ||
+		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, temp) ||
+		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, hyst))
+			goto out_cancel_nest;
+	}
+
+	mutex_unlock(&tz->lock);
+
+	nla_nest_end(msg, start_trip);
+
+	return 0;
+
+out_cancel_nest:
+	mutex_unlock(&tz->lock);
+
+	return -EMSGSIZE;
+}
+
+static int thermal_genl_cmd_tz_get_temp(struct param *p)
+{
+	struct sk_buff *msg = p->msg;
+	struct thermal_zone_device *tz;
+	int temp, ret, id;
+
+	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
+		return -EINVAL;
+
+	id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]);
+
+	tz = thermal_zone_get_by_id(id);
+	if (!tz)
+		return -EINVAL;
+
+	ret = thermal_zone_get_temp(tz, &temp);
+	if (ret)
+		return ret;
+
+	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, id) ||
+	    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TEMP, temp))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_cmd_tz_get_gov(struct param *p)
+{
+	struct sk_buff *msg = p->msg;
+	struct thermal_zone_device *tz;
+	int id, ret = 0;
+
+	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
+		return -EINVAL;
+
+	id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]);
+
+	tz = thermal_zone_get_by_id(id);
+	if (!tz)
+		return -EINVAL;
+
+	mutex_lock(&tz->lock);
+
+	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, id) ||
+	    nla_put_string(msg, THERMAL_GENL_ATTR_TZ_GOV_NAME,
+			   tz->governor->name))
+		ret = -EMSGSIZE;
+
+	mutex_unlock(&tz->lock);
+
+	return ret;
+}
+
+static int __thermal_genl_cmd_cdev_get(struct thermal_cooling_device *cdev,
+				       void *data)
+{
+	struct sk_buff *msg = data;
+
+	if (nla_put_u32(msg, THERMAL_GENL_ATTR_CDEV_ID, cdev->id))
+		return -EMSGSIZE;
+
+	if (nla_put_string(msg, THERMAL_GENL_ATTR_CDEV_NAME, cdev->type))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int thermal_genl_cmd_cdev_get(struct param *p)
+{
+	struct sk_buff *msg = p->msg;
+	struct nlattr *start_cdev;
+	int ret;
+
+	start_cdev = nla_nest_start(msg, THERMAL_GENL_ATTR_CDEV);
+	if (!start_cdev)
+		return -EMSGSIZE;
+
+	ret = for_each_thermal_cooling_device(__thermal_genl_cmd_cdev_get, msg);
+	if (ret)
+		goto out_cancel_nest;
+
+	nla_nest_end(msg, start_cdev);
+
+	return 0;
+out_cancel_nest:
+	nla_nest_cancel(msg, start_cdev);
+
+	return ret;
+}
+
+static cb_t cmd_cb[] = {
+	[THERMAL_GENL_CMD_TZ_GET_ID]	= thermal_genl_cmd_tz_get_id,
+	[THERMAL_GENL_CMD_TZ_GET_TRIP]	= thermal_genl_cmd_tz_get_trip,
+	[THERMAL_GENL_CMD_TZ_GET_TEMP]	= thermal_genl_cmd_tz_get_temp,
+	[THERMAL_GENL_CMD_TZ_GET_GOV]	= thermal_genl_cmd_tz_get_gov,
+	[THERMAL_GENL_CMD_CDEV_GET]	= thermal_genl_cmd_cdev_get,
+};
+
+static int thermal_genl_cmd_dumpit(struct sk_buff *skb,
+				   struct netlink_callback *cb)
+{
+	struct param p = { .msg = skb };
+	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+	int cmd = info->ops->cmd;
+	int ret = -EMSGSIZE;
+	void *hdr;
+
+	hdr = genlmsg_put(skb, 0, 0, &thermal_gnl_family, 0, cmd);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	ret = cmd_cb[cmd](&p);
+	if (ret)
+		goto out_cancel_msg;
+
+	genlmsg_end(skb, hdr);
+
+	return 0;
+
+out_cancel_msg:
+	genlmsg_cancel(skb, hdr);
+
+	return ret;
+}
+
+static int thermal_genl_cmd_doit(struct sk_buff *skb,
+				 struct genl_info *info)
+{
+	struct param p = { .attrs = info->attrs };
+	struct sk_buff *msg;
+	void *hdr;
+	int cmd = info->genlhdr->cmd;
+	int ret = -EMSGSIZE;
+
+	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+	p.msg = msg;
+
+	hdr = genlmsg_put_reply(msg, info, &thermal_gnl_family, 0, cmd);
+	if (!hdr)
+		goto out_free_msg;
+
+	ret = cmd_cb[cmd](&p);
+	if (ret)
+		goto out_cancel_msg;
+
+	genlmsg_end(msg, hdr);
+
+	return genlmsg_reply(msg, info);
+
+out_cancel_msg:
+	genlmsg_cancel(msg, hdr);
+out_free_msg:
+	nlmsg_free(msg);
+
+	return ret;
+}
+
+static const struct genl_ops thermal_genl_ops[] = {
+	{
+		.cmd = THERMAL_GENL_CMD_TZ_GET_ID,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.dumpit = thermal_genl_cmd_dumpit,
+	},
+	{
+		.cmd = THERMAL_GENL_CMD_TZ_GET_TRIP,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = thermal_genl_cmd_doit,
+	},
+	{
+		.cmd = THERMAL_GENL_CMD_TZ_GET_TEMP,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = thermal_genl_cmd_doit,
+	},
+	{
+		.cmd = THERMAL_GENL_CMD_TZ_GET_GOV,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = thermal_genl_cmd_doit,
+	},
+	{
+		.cmd = THERMAL_GENL_CMD_CDEV_GET,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.dumpit = thermal_genl_cmd_dumpit,
+	},
+};
+
+static struct genl_family thermal_gnl_family __ro_after_init = {
+	.hdrsize	= 0,
+	.name		= THERMAL_GENL_FAMILY_NAME,
+	.version	= THERMAL_GENL_VERSION,
+	.maxattr	= THERMAL_GENL_ATTR_MAX,
+	.policy		= thermal_genl_policy,
+	.ops		= thermal_genl_ops,
+	.n_ops		= ARRAY_SIZE(thermal_genl_ops),
+	.mcgrps		= thermal_genl_mcgrps,
+	.n_mcgrps	= ARRAY_SIZE(thermal_genl_mcgrps),
+};
+
+static int __init thermal_netlink_init(void)
+{
+	return genl_register_family(&thermal_gnl_family);
+}
+core_initcall(thermal_netlink_init);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 216185bb3014..3d4d8ae8c16a 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -37,18 +37,6 @@ struct thermal_cooling_device;
 struct thermal_instance;
 struct thermal_attr;
 
-enum thermal_device_mode {
-	THERMAL_DEVICE_DISABLED = 0,
-	THERMAL_DEVICE_ENABLED,
-};
-
-enum thermal_trip_type {
-	THERMAL_TRIP_ACTIVE = 0,
-	THERMAL_TRIP_PASSIVE,
-	THERMAL_TRIP_HOT,
-	THERMAL_TRIP_CRITICAL,
-};
-
 enum thermal_trend {
 	THERMAL_TREND_STABLE, /* temperature is stable */
 	THERMAL_TREND_RAISING, /* temperature is raising */
@@ -303,11 +291,6 @@ struct thermal_zone_params {
 	int offset;
 };
 
-struct thermal_genl_event {
-	u32 orig;
-	enum events event;
-};
-
 /**
  * struct thermal_zone_of_device_ops - scallbacks for handling DT based zones
  *
diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
index 96218378dda8..c105054cbb57 100644
--- a/include/uapi/linux/thermal.h
+++ b/include/uapi/linux/thermal.h
@@ -4,31 +4,86 @@
 
 #define THERMAL_NAME_LENGTH	20
 
-/* Adding event notification support elements */
-#define THERMAL_GENL_FAMILY_NAME                "thermal_event"
-#define THERMAL_GENL_VERSION                    0x01
-#define THERMAL_GENL_MCAST_GROUP_NAME           "thermal_mc_grp"
-
-/* Events supported by Thermal Netlink */
-enum events {
-	THERMAL_AUX0,
-	THERMAL_AUX1,
-	THERMAL_CRITICAL,
-	THERMAL_DEV_FAULT,
+enum thermal_device_mode {
+	THERMAL_DEVICE_DISABLED = 0,
+	THERMAL_DEVICE_ENABLED,
+};
+
+enum thermal_trip_type {
+	THERMAL_TRIP_ACTIVE = 0,
+	THERMAL_TRIP_PASSIVE,
+	THERMAL_TRIP_HOT,
+	THERMAL_TRIP_CRITICAL,
 };
 
-/* attributes of thermal_genl_family */
-enum {
+/* Adding event notification support elements */
+#define THERMAL_GENL_FAMILY_NAME		"thermal"
+#define THERMAL_GENL_VERSION			0x01
+#define THERMAL_GENL_SAMPLING_GROUP_NAME	"sampling"
+#define THERMAL_GENL_EVENT_GROUP_NAME		"event"
+
+/* Attributes of thermal_genl_family */
+enum thermal_genl_attr {
 	THERMAL_GENL_ATTR_UNSPEC,
-	THERMAL_GENL_ATTR_EVENT,
+	THERMAL_GENL_ATTR_TZ,
+	THERMAL_GENL_ATTR_TZ_ID,
+	THERMAL_GENL_ATTR_TZ_TEMP,
+	THERMAL_GENL_ATTR_TZ_TRIP,
+	THERMAL_GENL_ATTR_TZ_TRIP_ID,
+	THERMAL_GENL_ATTR_TZ_TRIP_TYPE,
+	THERMAL_GENL_ATTR_TZ_TRIP_TEMP,
+	THERMAL_GENL_ATTR_TZ_TRIP_HYST,
+	THERMAL_GENL_ATTR_TZ_MODE,
+	THERMAL_GENL_ATTR_TZ_NAME,
+	THERMAL_GENL_ATTR_TZ_CDEV_WEIGHT,
+	THERMAL_GENL_ATTR_TZ_GOV,
+	THERMAL_GENL_ATTR_TZ_GOV_NAME,
+	THERMAL_GENL_ATTR_CDEV,
+	THERMAL_GENL_ATTR_CDEV_ID,
+	THERMAL_GENL_ATTR_CDEV_CUR_STATE,
+	THERMAL_GENL_ATTR_CDEV_MAX_STATE,
+	THERMAL_GENL_ATTR_CDEV_NAME,
+	THERMAL_GENL_ATTR_GOV_NAME,
+
 	__THERMAL_GENL_ATTR_MAX,
 };
 #define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
 
-/* commands supported by the thermal_genl_family */
-enum {
+enum thermal_genl_sampling {
+	THERMAL_GENL_SAMPLING_TEMP,
+	__THERMAL_GENL_SAMPLING_MAX,
+};
+#define THERMAL_GENL_SAMPLING_MAX (__THERMAL_GENL_SAMPLING_MAX - 1)
+
+/* Events of thermal_genl_family */
+enum thermal_genl_event {
+	THERMAL_GENL_EVENT_UNSPEC,
+	THERMAL_GENL_EVENT_TZ_CREATE,		/* Thermal zone creation */
+	THERMAL_GENL_EVENT_TZ_DELETE,		/* Thermal zone deletion */
+	THERMAL_GENL_EVENT_TZ_DISABLE,		/* Thermal zone disabed */
+	THERMAL_GENL_EVENT_TZ_ENABLE,		/* Thermal zone enabled */
+	THERMAL_GENL_EVENT_TZ_TRIP_UP,		/* Trip point crossed the way up */
+	THERMAL_GENL_EVENT_TZ_TRIP_DOWN,	/* Trip point crossed the way down */
+	THERMAL_GENL_EVENT_TZ_TRIP_CHANGE,	/* Trip point changed */
+	THERMAL_GENL_EVENT_TZ_TRIP_ADD,		/* Trip point added */
+	THERMAL_GENL_EVENT_TZ_TRIP_DELETE,	/* Trip point deleted */
+	THERMAL_GENL_EVENT_CDEV_ADD,		/* Cdev bound to the thermal zone */
+	THERMAL_GENL_EVENT_CDEV_DELETE,		/* Cdev unbound */
+	THERMAL_GENL_EVENT_CDEV_STATE_UPDATE,	/* Cdev state updated */
+	THERMAL_GENL_EVENT_TZ_GOV_CHANGE,	/* Governor policy changed  */
+	__THERMAL_GENL_EVENT_MAX,
+};
+#define THERMAL_GENL_EVENT_MAX (__THERMAL_GENL_EVENT_MAX - 1)
+
+/* Commands supported by the thermal_genl_family */
+enum thermal_genl_cmd {
 	THERMAL_GENL_CMD_UNSPEC,
-	THERMAL_GENL_CMD_EVENT,
+	THERMAL_GENL_CMD_TZ_GET_ID,	/* List of thermal zones id */
+	THERMAL_GENL_CMD_TZ_GET_TRIP,	/* List of thermal trips */
+	THERMAL_GENL_CMD_TZ_GET_TEMP,	/* Get the thermal zone temperature */
+	THERMAL_GENL_CMD_TZ_GET_GOV,	/* Get the thermal zone governor */
+	THERMAL_GENL_CMD_TZ_GET_MODE,	/* Get the thermal zone mode */
+	THERMAL_GENL_CMD_CDEV_GET,	/* List of cdev id */
 	__THERMAL_GENL_CMD_MAX,
 };
 #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-06 10:55 [PATCH v4 1/4] thermal: core: Add helpers to browse the cdev, tz and governor list Daniel Lezcano
  2020-07-06 10:55 ` [PATCH v4 2/4] thermal: core: Get thermal zone by id Daniel Lezcano
  2020-07-06 10:55 ` [PATCH v4 3/4] thermal: core: genetlink support for events/cmd/sampling Daniel Lezcano
@ 2020-07-06 10:55 ` Daniel Lezcano
       [not found]   ` <CGME20200706131708eucas1p1487955a7632584c17df724399f48825a@eucas1p1.samsung.com>
  2020-07-07  1:55   ` Zhang Rui
  2 siblings, 2 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-06 10:55 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

The generic netlink protocol is implemented but the different
notification functions are not yet connected to the core code.

These changes add the notification calls in the different
corresponding places.

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
  v4:
     - Fixed missing static declaration, reported by kbuild-bot
     - Removed max state notification
---
 drivers/thermal/thermal_core.c    | 21 +++++++++++++++++++++
 drivers/thermal/thermal_helpers.c | 13 +++++++++++--
 drivers/thermal/thermal_sysfs.c   | 15 ++++++++++++++-
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 5fae1621fb01..25ef29123f72 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -215,6 +215,8 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *tz,
 	mutex_unlock(&tz->lock);
 	mutex_unlock(&thermal_governor_lock);
 
+	thermal_notify_tz_gov_change(tz->id, policy);
+
 	return ret;
 }
 
@@ -406,12 +408,25 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
 static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
 {
 	enum thermal_trip_type type;
+	int trip_temp, hyst = 0;
 
 	/* Ignore disabled trip points */
 	if (test_bit(trip, &tz->trips_disabled))
 		return;
 
+	tz->ops->get_trip_temp(tz, trip, &trip_temp);
 	tz->ops->get_trip_type(tz, trip, &type);
+	if (tz->ops->get_trip_hyst)
+		tz->ops->get_trip_hyst(tz, trip, &hyst);
+
+	if (tz->last_temperature != THERMAL_TEMP_INVALID) {
+		if (tz->last_temperature < trip_temp &&
+		    tz->temperature >= trip_temp)
+			thermal_notify_tz_trip_up(tz->id, trip);
+		if (tz->last_temperature >= trip_temp &&
+		    tz->temperature < (trip_temp - hyst))
+			thermal_notify_tz_trip_down(tz->id, trip);
+	}
 
 	if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
 		handle_critical_trips(tz, trip, type);
@@ -443,6 +458,8 @@ static void update_temperature(struct thermal_zone_device *tz)
 	mutex_unlock(&tz->lock);
 
 	trace_thermal_temperature(tz);
+
+	thermal_genl_sampling_temp(tz->id, temp);
 }
 
 static void thermal_zone_device_init(struct thermal_zone_device *tz)
@@ -1405,6 +1422,8 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 	if (atomic_cmpxchg(&tz->need_update, 1, 0))
 		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
+	thermal_notify_tz_create(tz->id, tz->type);
+
 	return tz;
 
 unregister:
@@ -1476,6 +1495,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 	ida_destroy(&tz->ida);
 	mutex_destroy(&tz->lock);
 	device_unregister(&tz->device);
+
+	thermal_notify_tz_delete(tz->id);
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
 
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index 87b1256fa2f2..c94bc824e5d3 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -175,6 +175,16 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
 	mutex_unlock(&tz->lock);
 }
 
+static void thermal_cdev_set_cur_state(struct thermal_cooling_device *cdev,
+				       int target)
+{
+	if (cdev->ops->set_cur_state(cdev, target))
+		return;
+
+	thermal_notify_cdev_state_update(cdev->id, target);
+	thermal_cooling_device_stats_update(cdev, target);
+}
+
 void thermal_cdev_update(struct thermal_cooling_device *cdev)
 {
 	struct thermal_instance *instance;
@@ -197,8 +207,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
 			target = instance->target;
 	}
 
-	if (!cdev->ops->set_cur_state(cdev, target))
-		thermal_cooling_device_stats_update(cdev, target);
+	thermal_cdev_set_cur_state(cdev, target);
 
 	cdev->updated = true;
 	mutex_unlock(&cdev->lock);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index aa99edb4dff7..ff449943f757 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -124,7 +124,8 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	int trip, ret;
-	int temperature;
+	int temperature, hyst = 0;
+	enum thermal_trip_type type;
 
 	if (!tz->ops->set_trip_temp)
 		return -EPERM;
@@ -139,6 +140,18 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
+	if (tz->ops->get_trip_hyst) {
+		ret = tz->ops->get_trip_hyst(tz, trip, &hyst);
+		if (ret)
+			return ret;
+	}
+
+	ret = tz->ops->get_trip_type(tz, trip, &type);
+	if (ret)
+		return ret;
+
+	thermal_notify_tz_trip_change(tz->id, trip, type, temperature, hyst);
+
 	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
 	return count;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
       [not found]   ` <CGME20200706131708eucas1p1487955a7632584c17df724399f48825a@eucas1p1.samsung.com>
@ 2020-07-06 13:17     ` Marek Szyprowski
  2020-07-06 13:46       ` Daniel Lezcano
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Szyprowski @ 2020-07-06 13:17 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz

Hi Daniel,

On 06.07.2020 12:55, Daniel Lezcano wrote:
> The generic netlink protocol is implemented but the different
> notification functions are not yet connected to the core code.
>
> These changes add the notification calls in the different
> corresponding places.
>
> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

This patch landed in today's linux-next 20200706 as commit 5df786e46560 
("thermal: core: Add notifications call in the framework"). Sadly it 
breaks booting various Samsung Exynos based boards. Here is an example 
log from Odroid U3 board:

Unable to handle kernel NULL pointer dereference at virtual address 00000010
pgd = (ptrval)
[00000010] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560 
#1146
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at kmem_cache_alloc+0x13c/0x418
LR is at kmem_cache_alloc+0x48/0x418
pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
...
Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 4000404a  DAC: 00000051
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xee8f1cf8 to 0xee8f2000)
...
[<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] (__alloc_skb+0x5c/0x170)
[<c08cd170>] (__alloc_skb) from [<c07ec19c>] 
(thermal_genl_send_event+0x24/0x174)
[<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>] 
(thermal_notify_tz_create+0x58/0x74)
[<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>] 
(thermal_zone_device_register+0x358/0x650)
[<c07e9058>] (thermal_zone_device_register) from [<c1028d34>] 
(of_parse_thermal_zones+0x304/0x7a4)
[<c1028d34>] (of_parse_thermal_zones) from [<c1028964>] 
(thermal_init+0xdc/0x154)
[<c1028964>] (thermal_init) from [<c0102378>] (do_one_initcall+0x8c/0x424)
[<c0102378>] (do_one_initcall) from [<c1001158>] 
(kernel_init_freeable+0x190/0x204)
[<c1001158>] (kernel_init_freeable) from [<c0ab85f4>] 
(kernel_init+0x8/0x118)
[<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)

Reverting it on top of linux-next fixes the boot issue. I will 
investigate it further soon.



> ---
>    v4:
>       - Fixed missing static declaration, reported by kbuild-bot
>       - Removed max state notification
> ---
>   drivers/thermal/thermal_core.c    | 21 +++++++++++++++++++++
>   drivers/thermal/thermal_helpers.c | 13 +++++++++++--
>   drivers/thermal/thermal_sysfs.c   | 15 ++++++++++++++-
>   3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 5fae1621fb01..25ef29123f72 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -215,6 +215,8 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *tz,
>   	mutex_unlock(&tz->lock);
>   	mutex_unlock(&thermal_governor_lock);
>   
> +	thermal_notify_tz_gov_change(tz->id, policy);
> +
>   	return ret;
>   }
>   
> @@ -406,12 +408,25 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>   static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
>   {
>   	enum thermal_trip_type type;
> +	int trip_temp, hyst = 0;
>   
>   	/* Ignore disabled trip points */
>   	if (test_bit(trip, &tz->trips_disabled))
>   		return;
>   
> +	tz->ops->get_trip_temp(tz, trip, &trip_temp);
>   	tz->ops->get_trip_type(tz, trip, &type);
> +	if (tz->ops->get_trip_hyst)
> +		tz->ops->get_trip_hyst(tz, trip, &hyst);
> +
> +	if (tz->last_temperature != THERMAL_TEMP_INVALID) {
> +		if (tz->last_temperature < trip_temp &&
> +		    tz->temperature >= trip_temp)
> +			thermal_notify_tz_trip_up(tz->id, trip);
> +		if (tz->last_temperature >= trip_temp &&
> +		    tz->temperature < (trip_temp - hyst))
> +			thermal_notify_tz_trip_down(tz->id, trip);
> +	}
>   
>   	if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
>   		handle_critical_trips(tz, trip, type);
> @@ -443,6 +458,8 @@ static void update_temperature(struct thermal_zone_device *tz)
>   	mutex_unlock(&tz->lock);
>   
>   	trace_thermal_temperature(tz);
> +
> +	thermal_genl_sampling_temp(tz->id, temp);
>   }
>   
>   static void thermal_zone_device_init(struct thermal_zone_device *tz)
> @@ -1405,6 +1422,8 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>   	if (atomic_cmpxchg(&tz->need_update, 1, 0))
>   		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   
> +	thermal_notify_tz_create(tz->id, tz->type);
> +
>   	return tz;
>   
>   unregister:
> @@ -1476,6 +1495,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>   	ida_destroy(&tz->ida);
>   	mutex_destroy(&tz->lock);
>   	device_unregister(&tz->device);
> +
> +	thermal_notify_tz_delete(tz->id);
>   }
>   EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>   
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index 87b1256fa2f2..c94bc824e5d3 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -175,6 +175,16 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
>   	mutex_unlock(&tz->lock);
>   }
>   
> +static void thermal_cdev_set_cur_state(struct thermal_cooling_device *cdev,
> +				       int target)
> +{
> +	if (cdev->ops->set_cur_state(cdev, target))
> +		return;
> +
> +	thermal_notify_cdev_state_update(cdev->id, target);
> +	thermal_cooling_device_stats_update(cdev, target);
> +}
> +
>   void thermal_cdev_update(struct thermal_cooling_device *cdev)
>   {
>   	struct thermal_instance *instance;
> @@ -197,8 +207,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
>   			target = instance->target;
>   	}
>   
> -	if (!cdev->ops->set_cur_state(cdev, target))
> -		thermal_cooling_device_stats_update(cdev, target);
> +	thermal_cdev_set_cur_state(cdev, target);
>   
>   	cdev->updated = true;
>   	mutex_unlock(&cdev->lock);
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa99edb4dff7..ff449943f757 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -124,7 +124,8 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>   {
>   	struct thermal_zone_device *tz = to_thermal_zone(dev);
>   	int trip, ret;
> -	int temperature;
> +	int temperature, hyst = 0;
> +	enum thermal_trip_type type;
>   
>   	if (!tz->ops->set_trip_temp)
>   		return -EPERM;
> @@ -139,6 +140,18 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>   	if (ret)
>   		return ret;
>   
> +	if (tz->ops->get_trip_hyst) {
> +		ret = tz->ops->get_trip_hyst(tz, trip, &hyst);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = tz->ops->get_trip_type(tz, trip, &type);
> +	if (ret)
> +		return ret;
> +
> +	thermal_notify_tz_trip_change(tz->id, trip, type, temperature, hyst);
> +
>   	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   
>   	return count;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-06 13:17     ` Marek Szyprowski
@ 2020-07-06 13:46       ` Daniel Lezcano
  2020-07-07  9:15         ` Marek Szyprowski
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-06 13:46 UTC (permalink / raw)
  To: Marek Szyprowski, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz

On 06/07/2020 15:17, Marek Szyprowski wrote:
> Hi Daniel,
> 
> On 06.07.2020 12:55, Daniel Lezcano wrote:
>> The generic netlink protocol is implemented but the different
>> notification functions are not yet connected to the core code.
>>
>> These changes add the notification calls in the different
>> corresponding places.
>>
>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> This patch landed in today's linux-next 20200706 as commit 5df786e46560 
> ("thermal: core: Add notifications call in the framework"). Sadly it 
> breaks booting various Samsung Exynos based boards. Here is an example 
> log from Odroid U3 board:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 00000010
> pgd = (ptrval)
> [00000010] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560 
> #1146
> Hardware name: Samsung Exynos (Flattened Device Tree)
> PC is at kmem_cache_alloc+0x13c/0x418
> LR is at kmem_cache_alloc+0x48/0x418
> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
> ...
> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 4000404a  DAC: 00000051
> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> Stack: (0xee8f1cf8 to 0xee8f2000)
> ...
> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] (__alloc_skb+0x5c/0x170)
> [<c08cd170>] (__alloc_skb) from [<c07ec19c>] 
> (thermal_genl_send_event+0x24/0x174)
> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>] 
> (thermal_notify_tz_create+0x58/0x74)
> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>] 
> (thermal_zone_device_register+0x358/0x650)
> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>] 
> (of_parse_thermal_zones+0x304/0x7a4)
> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>] 
> (thermal_init+0xdc/0x154)
> [<c1028964>] (thermal_init) from [<c0102378>] (do_one_initcall+0x8c/0x424)
> [<c0102378>] (do_one_initcall) from [<c1001158>] 
> (kernel_init_freeable+0x190/0x204)
> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>] 
> (kernel_init+0x8/0x118)
> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
> 
> Reverting it on top of linux-next fixes the boot issue. I will 
> investigate it further soon.

Thanks for reporting this.

Can you send the addr2line result and code it points to ?


>> ---
>>    v4:
>>       - Fixed missing static declaration, reported by kbuild-bot
>>       - Removed max state notification
>> ---
>>   drivers/thermal/thermal_core.c    | 21 +++++++++++++++++++++
>>   drivers/thermal/thermal_helpers.c | 13 +++++++++++--
>>   drivers/thermal/thermal_sysfs.c   | 15 ++++++++++++++-
>>   3 files changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 5fae1621fb01..25ef29123f72 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -215,6 +215,8 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *tz,
>>   	mutex_unlock(&tz->lock);
>>   	mutex_unlock(&thermal_governor_lock);
>>   
>> +	thermal_notify_tz_gov_change(tz->id, policy);
>> +
>>   	return ret;
>>   }
>>   
>> @@ -406,12 +408,25 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>>   static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
>>   {
>>   	enum thermal_trip_type type;
>> +	int trip_temp, hyst = 0;
>>   
>>   	/* Ignore disabled trip points */
>>   	if (test_bit(trip, &tz->trips_disabled))
>>   		return;
>>   
>> +	tz->ops->get_trip_temp(tz, trip, &trip_temp);
>>   	tz->ops->get_trip_type(tz, trip, &type);
>> +	if (tz->ops->get_trip_hyst)
>> +		tz->ops->get_trip_hyst(tz, trip, &hyst);
>> +
>> +	if (tz->last_temperature != THERMAL_TEMP_INVALID) {
>> +		if (tz->last_temperature < trip_temp &&
>> +		    tz->temperature >= trip_temp)
>> +			thermal_notify_tz_trip_up(tz->id, trip);
>> +		if (tz->last_temperature >= trip_temp &&
>> +		    tz->temperature < (trip_temp - hyst))
>> +			thermal_notify_tz_trip_down(tz->id, trip);
>> +	}
>>   
>>   	if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
>>   		handle_critical_trips(tz, trip, type);
>> @@ -443,6 +458,8 @@ static void update_temperature(struct thermal_zone_device *tz)
>>   	mutex_unlock(&tz->lock);
>>   
>>   	trace_thermal_temperature(tz);
>> +
>> +	thermal_genl_sampling_temp(tz->id, temp);
>>   }
>>   
>>   static void thermal_zone_device_init(struct thermal_zone_device *tz)
>> @@ -1405,6 +1422,8 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>>   	if (atomic_cmpxchg(&tz->need_update, 1, 0))
>>   		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>>   
>> +	thermal_notify_tz_create(tz->id, tz->type);
>> +
>>   	return tz;
>>   
>>   unregister:
>> @@ -1476,6 +1495,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>>   	ida_destroy(&tz->ida);
>>   	mutex_destroy(&tz->lock);
>>   	device_unregister(&tz->device);
>> +
>> +	thermal_notify_tz_delete(tz->id);
>>   }
>>   EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>>   
>> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
>> index 87b1256fa2f2..c94bc824e5d3 100644
>> --- a/drivers/thermal/thermal_helpers.c
>> +++ b/drivers/thermal/thermal_helpers.c
>> @@ -175,6 +175,16 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
>>   	mutex_unlock(&tz->lock);
>>   }
>>   
>> +static void thermal_cdev_set_cur_state(struct thermal_cooling_device *cdev,
>> +				       int target)
>> +{
>> +	if (cdev->ops->set_cur_state(cdev, target))
>> +		return;
>> +
>> +	thermal_notify_cdev_state_update(cdev->id, target);
>> +	thermal_cooling_device_stats_update(cdev, target);
>> +}
>> +
>>   void thermal_cdev_update(struct thermal_cooling_device *cdev)
>>   {
>>   	struct thermal_instance *instance;
>> @@ -197,8 +207,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
>>   			target = instance->target;
>>   	}
>>   
>> -	if (!cdev->ops->set_cur_state(cdev, target))
>> -		thermal_cooling_device_stats_update(cdev, target);
>> +	thermal_cdev_set_cur_state(cdev, target);
>>   
>>   	cdev->updated = true;
>>   	mutex_unlock(&cdev->lock);
>> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
>> index aa99edb4dff7..ff449943f757 100644
>> --- a/drivers/thermal/thermal_sysfs.c
>> +++ b/drivers/thermal/thermal_sysfs.c
>> @@ -124,7 +124,8 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>>   {
>>   	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>   	int trip, ret;
>> -	int temperature;
>> +	int temperature, hyst = 0;
>> +	enum thermal_trip_type type;
>>   
>>   	if (!tz->ops->set_trip_temp)
>>   		return -EPERM;
>> @@ -139,6 +140,18 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>>   	if (ret)
>>   		return ret;
>>   
>> +	if (tz->ops->get_trip_hyst) {
>> +		ret = tz->ops->get_trip_hyst(tz, trip, &hyst);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = tz->ops->get_trip_type(tz, trip, &type);
>> +	if (ret)
>> +		return ret;
>> +
>> +	thermal_notify_tz_trip_change(tz->id, trip, type, temperature, hyst);
>> +
>>   	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>>   
>>   	return count;
> 
> Best regards
> 


-- 
<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] 16+ messages in thread

* Re: [PATCH v4 2/4] thermal: core: Get thermal zone by id
  2020-07-06 10:55 ` [PATCH v4 2/4] thermal: core: Get thermal zone by id Daniel Lezcano
@ 2020-07-07  1:53   ` Zhang Rui
  0 siblings, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2020-07-07  1:53 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

On Mon, 2020-07-06 at 12:55 +0200, Daniel Lezcano wrote:
> The next patch will introduce the generic netlink protocol to handle
> events, sampling and command from the thermal framework. In order to
> deal with the thermal zone, it uses its unique identifier to
> characterize it in the message. Passing an integer is more efficient
> than passing an entire string.
> 
> This change provides a function returning back a thermal zone pointer
> corresponding to the identifier passed as parameter.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

Acked-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/thermal_core.c | 14 ++++++++++++++
>  drivers/thermal/thermal_core.h |  2 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 9caaa0b6d662..5fae1621fb01 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -668,6 +668,20 @@ int for_each_thermal_zone(int (*cb)(struct
> thermal_zone_device *, void *),
>  	return ret;
>  }
>  
> +struct thermal_zone_device *thermal_zone_get_by_id(int id)
> +{
> +	struct thermal_zone_device *tz = NULL;
> +
> +	mutex_lock(&thermal_list_lock);
> +	list_for_each_entry(tz, &thermal_tz_list, node) {
> +		if (tz->id == id)
> +			break;
> +	}
> +	mutex_unlock(&thermal_list_lock);
> +
> +	return tz;
> +}
> +
>  void thermal_zone_device_unbind_exception(struct thermal_zone_device
> *tz,
>  					  const char *cdev_type, size_t
> size)
>  {
> diff --git a/drivers/thermal/thermal_core.h
> b/drivers/thermal/thermal_core.h
> index 71d88dac0791..4f8389efaa62 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -50,6 +50,8 @@ int for_each_thermal_cooling_device(int
> (*cb)(struct thermal_cooling_device *,
>  int for_each_thermal_governor(int (*cb)(struct thermal_governor *,
> void *),
>  			      void *thermal_governor);
>  
> +struct thermal_zone_device *thermal_zone_get_by_id(int id);
> +
>  struct thermal_attr {
>  	struct device_attribute attr;
>  	char name[THERMAL_NAME_LENGTH];


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/4] thermal: core: genetlink support for events/cmd/sampling
  2020-07-06 10:55 ` [PATCH v4 3/4] thermal: core: genetlink support for events/cmd/sampling Daniel Lezcano
@ 2020-07-07  1:54   ` Zhang Rui
  0 siblings, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2020-07-07  1:54 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

On Mon, 2020-07-06 at 12:55 +0200, Daniel Lezcano wrote:
> Initially the thermal framework had a very simple notification
> mechanism to send generic netlink messages to the userspace.
> 
> The notification function was never called from anywhere and the
> corresponding dead code was removed. It was probably a first attempt
> to introduce the netlink notification.
> 
> At LPC2018, the presentation "Linux thermal: User kernel interface",
> proposed to create the notifications to the userspace via a kfifo.
> 
> The advantage of the kfifo is the performance. It is usually used
> from
> a 1:1 communication channel where a driver captures data and sends it
> as fast as possible to a userspace process.
> 
> The drawback is that only one process uses the notification channel
> exclusively, thus no other process is allowed to use the channel to
> get temperature or notifications.
> 
> This patch defines a generic netlink API to discover the current
> thermal setup and adds event notifications as well as temperature
> sampling. As any genetlink protocol, it can evolve and the versioning
> allows to keep the backward compatibility.
> 
> In order to prevent the user from getting flooded with data on a
> single channel, there are two multicast channels, one for the
> temperature sampling when the thermal zone is updated and another one
> for the events, so the user can get the events only without the
> thermal zone temperature sampling.
> 
> Also, a list of commands to discover the thermal setup is added and
> can be extended when needed.
> 
> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Acked-by: Zhang Rui <rui.zhang@intel.com>
> ---
>   v4:
>       - Removed max cdev state and renamed function and attributes
>       	with CDEV_STATE_UPDATE
>   v3:
>       - Fixed changelog from Amit Kucheria suggestions
>       - Prefixed fields in the parameter structure (trip_*, cdev_*)
>       - Fixed leading whitespaces errors
>       - Replaced id by trip_id
>       - s/THERMAL_GENL_CMD_TZ_GET/THERMAL_GENL_CMD_TZ_GET_ID/
>       - Added the cdev max state in the cdev change event
>       - Removed min state
>       - Fixed checkpatch warnings
> ---
> ---
>  drivers/thermal/Makefile          |   2 +-
>  drivers/thermal/thermal_core.h    |  18 +
>  drivers/thermal/thermal_netlink.c | 648
> ++++++++++++++++++++++++++++++
>  include/linux/thermal.h           |  17 -
>  include/uapi/linux/thermal.h      |  89 +++-
>  5 files changed, 739 insertions(+), 35 deletions(-)
>  create mode 100644 drivers/thermal/thermal_netlink.c
> 
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 0c8b84a09b9a..1bbf0805fb04 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -5,7 +5,7 @@
>  
>  obj-$(CONFIG_THERMAL)		+= thermal_sys.o
>  thermal_sys-y			+= thermal_core.o
> thermal_sysfs.o \
> -					thermal_helpers.o
> +					thermal_helpers.o
> thermal_netlink.o
>  
>  # interface to/from other layers providing sensors
>  thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
> diff --git a/drivers/thermal/thermal_core.h
> b/drivers/thermal/thermal_core.h
> index 4f8389efaa62..b44969d50ec0 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -52,6 +52,24 @@ int for_each_thermal_governor(int (*cb)(struct
> thermal_governor *, void *),
>  
>  struct thermal_zone_device *thermal_zone_get_by_id(int id);
>  
> +/* Netlink notification function */
> +int thermal_notify_tz_create(int tz_id, const char *name);
> +int thermal_notify_tz_delete(int tz_id);
> +int thermal_notify_tz_enable(int tz_id);
> +int thermal_notify_tz_disable(int tz_id);
> +int thermal_notify_tz_trip_down(int tz_id, int id);
> +int thermal_notify_tz_trip_up(int tz_id, int id);
> +int thermal_notify_tz_trip_delete(int tz_id, int id);
> +int thermal_notify_tz_trip_add(int tz_id, int id, int type,
> +			       int temp, int hyst);
> +int thermal_notify_tz_trip_change(int tz_id, int id, int type,
> +				  int temp, int hyst);
> +int thermal_notify_cdev_state_update(int cdev_id, int state);
> +int thermal_notify_cdev_add(int cdev_id, const char *name, int
> max_state);
> +int thermal_notify_cdev_delete(int cdev_id);
> +int thermal_notify_tz_gov_change(int tz_id, const char *name);
> +int thermal_genl_sampling_temp(int id, int temp);
> +
>  struct thermal_attr {
>  	struct device_attribute attr;
>  	char name[THERMAL_NAME_LENGTH];
> diff --git a/drivers/thermal/thermal_netlink.c
> b/drivers/thermal/thermal_netlink.c
> new file mode 100644
> index 000000000000..dd0a3b889674
> --- /dev/null
> +++ b/drivers/thermal/thermal_netlink.c
> @@ -0,0 +1,648 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 Linaro Limited
> + *
> + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> + *
> + * Generic netlink for thermal management framework
> + */
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <net/genetlink.h>
> +#include <uapi/linux/thermal.h>
> +
> +#include "thermal_core.h"
> +
> +static const struct genl_multicast_group thermal_genl_mcgrps[] = {
> +	{ .name = THERMAL_GENL_SAMPLING_GROUP_NAME, },
> +	{ .name = THERMAL_GENL_EVENT_GROUP_NAME,  },
> +};
> +
> +static const struct nla_policy
> thermal_genl_policy[THERMAL_GENL_ATTR_MAX + 1] = {
> +	/* Thermal zone */
> +	[THERMAL_GENL_ATTR_TZ]			= { .type =
> NLA_NESTED },
> +	[THERMAL_GENL_ATTR_TZ_ID]		= { .type = NLA_U32 },
> +	[THERMAL_GENL_ATTR_TZ_TEMP]		= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_TZ_TRIP]		= { .type =
> NLA_NESTED },
> +	[THERMAL_GENL_ATTR_TZ_TRIP_ID]		= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_TZ_TRIP_TEMP]	= { .type = NLA_U32 },
> +	[THERMAL_GENL_ATTR_TZ_TRIP_TYPE]	= { .type = NLA_U32 },
> +	[THERMAL_GENL_ATTR_TZ_TRIP_HYST]	= { .type = NLA_U32 },
> +	[THERMAL_GENL_ATTR_TZ_MODE]		= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_TZ_CDEV_WEIGHT]	= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_TZ_NAME]		= { .type =
> NLA_STRING,
> +						    .len =
> THERMAL_NAME_LENGTH },
> +	/* Governor(s) */
> +	[THERMAL_GENL_ATTR_TZ_GOV]		= { .type =
> NLA_NESTED },
> +	[THERMAL_GENL_ATTR_TZ_GOV_NAME]		= { .type =
> NLA_STRING,
> +						    .len =
> THERMAL_NAME_LENGTH },
> +	/* Cooling devices */
> +	[THERMAL_GENL_ATTR_CDEV]		= { .type = NLA_NESTED },
> +	[THERMAL_GENL_ATTR_CDEV_ID]		= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_CDEV_CUR_STATE]	= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_CDEV_MAX_STATE]	= { .type = NLA_U32
> },
> +	[THERMAL_GENL_ATTR_CDEV_NAME]		= { .type =
> NLA_STRING,
> +						    .len =
> THERMAL_NAME_LENGTH },
> +};
> +
> +struct param {
> +	struct nlattr **attrs;
> +	struct sk_buff *msg;
> +	const char *name;
> +	int tz_id;
> +	int cdev_id;
> +	int trip_id;
> +	int trip_temp;
> +	int trip_type;
> +	int trip_hyst;
> +	int temp;
> +	int cdev_state;
> +	int cdev_max_state;
> +};
> +
> +typedef int (*cb_t)(struct param *);
> +
> +static struct genl_family thermal_gnl_family;
> +
> +/************************** Sampling encoding
> *******************************/
> +
> +int thermal_genl_sampling_temp(int id, int temp)
> +{
> +	struct sk_buff *skb;
> +	void *hdr;
> +
> +	skb = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_put(skb, 0, 0, &thermal_gnl_family, 0,
> +			  THERMAL_GENL_SAMPLING_TEMP);
> +	if (!hdr)
> +		return -EMSGSIZE;
> +
> +	if (nla_put_u32(skb, THERMAL_GENL_ATTR_TZ_ID, id))
> +		goto out_cancel;
> +
> +	if (nla_put_u32(skb, THERMAL_GENL_ATTR_TZ_TEMP, temp))
> +		goto out_cancel;
> +
> +	genlmsg_end(skb, hdr);
> +
> +	genlmsg_multicast(&thermal_gnl_family, skb, 0, 0, GFP_KERNEL);
> +
> +	return 0;
> +out_cancel:
> +	genlmsg_cancel(skb, hdr);
> +	nlmsg_free(skb);
> +
> +	return -EMSGSIZE;
> +}
> +
> +/**************************** Event encoding
> *********************************/
> +
> +static int thermal_genl_event_tz_create(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
> +	    nla_put_string(p->msg, THERMAL_GENL_ATTR_TZ_NAME, p->name))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_tz(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_tz_trip_up(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, p-
> >trip_id))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_tz_trip_add(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, p-
> >trip_id) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, p-
> >trip_type) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, p-
> >trip_temp) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, p-
> >trip_hyst))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_tz_trip_delete(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, p-
> >trip_id))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_cdev_add(struct param *p)
> +{
> +	if (nla_put_string(p->msg, THERMAL_GENL_ATTR_CDEV_NAME,
> +			   p->name) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID,
> +			p->cdev_id) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_MAX_STATE,
> +			p->cdev_max_state))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_cdev_delete(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, p->cdev_id))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_cdev_state_update(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID,
> +			p->cdev_id) ||
> +	    nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_CUR_STATE,
> +			p->cdev_state))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_event_gov_change(struct param *p)
> +{
> +	if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id) ||
> +	    nla_put_string(p->msg, THERMAL_GENL_ATTR_GOV_NAME, p-
> >name))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +int thermal_genl_event_tz_delete(struct param *p)
> +	__attribute__((alias("thermal_genl_event_tz")));
> +
> +int thermal_genl_event_tz_enable(struct param *p)
> +	__attribute__((alias("thermal_genl_event_tz")));
> +
> +int thermal_genl_event_tz_disable(struct param *p)
> +	__attribute__((alias("thermal_genl_event_tz")));
> +
> +int thermal_genl_event_tz_trip_down(struct param *p)
> +	__attribute__((alias("thermal_genl_event_tz_trip_up")));
> +
> +int thermal_genl_event_tz_trip_change(struct param *p)
> +	__attribute__((alias("thermal_genl_event_tz_trip_add")));
> +
> +static cb_t event_cb[] = {
> +	[THERMAL_GENL_EVENT_TZ_CREATE]		=
> thermal_genl_event_tz_create,
> +	[THERMAL_GENL_EVENT_TZ_DELETE]		=
> thermal_genl_event_tz_delete,
> +	[THERMAL_GENL_EVENT_TZ_ENABLE]		=
> thermal_genl_event_tz_enable,
> +	[THERMAL_GENL_EVENT_TZ_DISABLE]		=
> thermal_genl_event_tz_disable,
> +	[THERMAL_GENL_EVENT_TZ_TRIP_UP]		=
> thermal_genl_event_tz_trip_up,
> +	[THERMAL_GENL_EVENT_TZ_TRIP_DOWN]	=
> thermal_genl_event_tz_trip_down,
> +	[THERMAL_GENL_EVENT_TZ_TRIP_CHANGE]	=
> thermal_genl_event_tz_trip_change,
> +	[THERMAL_GENL_EVENT_TZ_TRIP_ADD]	=
> thermal_genl_event_tz_trip_add,
> +	[THERMAL_GENL_EVENT_TZ_TRIP_DELETE]	=
> thermal_genl_event_tz_trip_delete,
> +	[THERMAL_GENL_EVENT_CDEV_ADD]		=
> thermal_genl_event_cdev_add,
> +	[THERMAL_GENL_EVENT_CDEV_DELETE]	=
> thermal_genl_event_cdev_delete,
> +	[THERMAL_GENL_EVENT_CDEV_STATE_UPDATE]	=
> thermal_genl_event_cdev_state_update,
> +	[THERMAL_GENL_EVENT_TZ_GOV_CHANGE]	=
> thermal_genl_event_gov_change,
> +};
> +
> +/*
> + * Generic netlink event encoding
> + */
> +static int thermal_genl_send_event(enum thermal_genl_event event,
> +				   struct param *p)
> +{
> +	struct sk_buff *msg;
> +	int ret = -EMSGSIZE;
> +	void *hdr;
> +
> +	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +	p->msg = msg;
> +
> +	hdr = genlmsg_put(msg, 0, 0, &thermal_gnl_family, 0, event);
> +	if (!hdr)
> +		goto out_free_msg;
> +
> +	ret = event_cb[event](p);
> +	if (ret)
> +		goto out_cancel_msg;
> +
> +	genlmsg_end(msg, hdr);
> +
> +	genlmsg_multicast(&thermal_gnl_family, msg, 0, 1, GFP_KERNEL);
> +
> +	return 0;
> +
> +out_cancel_msg:
> +	genlmsg_cancel(msg, hdr);
> +out_free_msg:
> +	nlmsg_free(msg);
> +
> +	return ret;
> +}
> +
> +int thermal_notify_tz_create(int tz_id, const char *name)
> +{
> +	struct param p = { .tz_id = tz_id, .name = name };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_CREATE,
> &p);
> +}
> +
> +int thermal_notify_tz_delete(int tz_id)
> +{
> +	struct param p = { .tz_id = tz_id };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_DELETE,
> &p);
> +}
> +
> +int thermal_notify_tz_enable(int tz_id)
> +{
> +	struct param p = { .tz_id = tz_id };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_ENABLE,
> &p);
> +}
> +
> +int thermal_notify_tz_disable(int tz_id)
> +{
> +	struct param p = { .tz_id = tz_id };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_DISABLE,
> &p);
> +}
> +
> +int thermal_notify_tz_trip_down(int tz_id, int trip_id)
> +{
> +	struct param p = { .tz_id = tz_id, .trip_id = trip_id };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DOWN,
> &p);
> +}
> +
> +int thermal_notify_tz_trip_up(int tz_id, int trip_id)
> +{
> +	struct param p = { .tz_id = tz_id, .trip_id = trip_id };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_UP,
> &p);
> +}
> +
> +int thermal_notify_tz_trip_add(int tz_id, int trip_id, int
> trip_type,
> +			       int trip_temp, int trip_hyst)
> +{
> +	struct param p = { .tz_id = tz_id, .trip_id = trip_id,
> +			   .trip_type = trip_type, .trip_temp =
> trip_temp,
> +			   .trip_hyst = trip_hyst };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_ADD,
> &p);
> +}
> +
> +int thermal_notify_tz_trip_delete(int tz_id, int trip_id)
> +{
> +	struct param p = { .tz_id = tz_id, .trip_id = trip_id };
> +
> +	return
> thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DELETE, &p);
> +}
> +
> +int thermal_notify_tz_trip_change(int tz_id, int trip_id, int
> trip_type,
> +				  int trip_temp, int trip_hyst)
> +{
> +	struct param p = { .tz_id = tz_id, .trip_id = trip_id,
> +			   .trip_type = trip_type, .trip_temp =
> trip_temp,
> +			   .trip_hyst = trip_hyst };
> +
> +	return
> thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p);
> +}
> +
> +int thermal_notify_cdev_state_update(int cdev_id, int cdev_state)
> +{
> +	struct param p = { .cdev_id = cdev_id, .cdev_state = cdev_state
> };
> +
> +	return
> thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, &p);
> +}
> +
> +int thermal_notify_cdev_add(int cdev_id, const char *name, int
> cdev_max_state)
> +{
> +	struct param p = { .cdev_id = cdev_id, .name = name,
> +			   .cdev_max_state = cdev_max_state };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_ADD,
> &p);
> +}
> +
> +int thermal_notify_cdev_delete(int cdev_id)
> +{
> +	struct param p = { .cdev_id = cdev_id };
> +
> +	return thermal_genl_send_event(THERMAL_GENL_EVENT_CDEV_DELETE,
> &p);
> +}
> +
> +int thermal_notify_tz_gov_change(int tz_id, const char *name)
> +{
> +	struct param p = { .tz_id = tz_id, .name = name };
> +
> +	return
> thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_GOV_CHANGE, &p);
> +}
> +
> +/*************************** Command encoding
> ********************************/
> +
> +static int __thermal_genl_cmd_tz_get_id(struct thermal_zone_device
> *tz,
> +					void *data)
> +{
> +	struct sk_buff *msg = data;
> +
> +	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, tz->id) ||
> +	    nla_put_string(msg, THERMAL_GENL_ATTR_TZ_NAME, tz->type))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_cmd_tz_get_id(struct param *p)
> +{
> +	struct sk_buff *msg = p->msg;
> +	struct nlattr *start_tz;
> +	int ret;
> +
> +	start_tz = nla_nest_start(msg, THERMAL_GENL_ATTR_TZ);
> +	if (!start_tz)
> +		return -EMSGSIZE;
> +
> +	ret = for_each_thermal_zone(__thermal_genl_cmd_tz_get_id, msg);
> +	if (ret)
> +		goto out_cancel_nest;
> +
> +	nla_nest_end(msg, start_tz);
> +
> +	return 0;
> +
> +out_cancel_nest:
> +	nla_nest_cancel(msg, start_tz);
> +
> +	return ret;
> +}
> +
> +static int thermal_genl_cmd_tz_get_trip(struct param *p)
> +{
> +	struct sk_buff *msg = p->msg;
> +	struct thermal_zone_device *tz;
> +	struct nlattr *start_trip;
> +	int i, id;
> +
> +	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
> +		return -EINVAL;
> +
> +	id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]);
> +
> +	tz = thermal_zone_get_by_id(id);
> +	if (!tz)
> +		return -EINVAL;
> +
> +	start_trip = nla_nest_start(msg, THERMAL_GENL_ATTR_TZ_TRIP);
> +	if (!start_trip)
> +		return -EMSGSIZE;
> +
> +	mutex_lock(&tz->lock);
> +
> +	for (i = 0; i < tz->trips; i++) {
> +
> +		enum thermal_trip_type type;
> +		int temp, hyst;
> +
> +		tz->ops->get_trip_type(tz, i, &type);
> +		tz->ops->get_trip_temp(tz, i, &temp);
> +		tz->ops->get_trip_hyst(tz, i, &hyst);
> +
> +		if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, i)
> ||
> +		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE,
> type) ||
> +		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP,
> temp) ||
> +		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST,
> hyst))
> +			goto out_cancel_nest;
> +	}
> +
> +	mutex_unlock(&tz->lock);
> +
> +	nla_nest_end(msg, start_trip);
> +
> +	return 0;
> +
> +out_cancel_nest:
> +	mutex_unlock(&tz->lock);
> +
> +	return -EMSGSIZE;
> +}
> +
> +static int thermal_genl_cmd_tz_get_temp(struct param *p)
> +{
> +	struct sk_buff *msg = p->msg;
> +	struct thermal_zone_device *tz;
> +	int temp, ret, id;
> +
> +	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
> +		return -EINVAL;
> +
> +	id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]);
> +
> +	tz = thermal_zone_get_by_id(id);
> +	if (!tz)
> +		return -EINVAL;
> +
> +	ret = thermal_zone_get_temp(tz, &temp);
> +	if (ret)
> +		return ret;
> +
> +	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, id) ||
> +	    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TEMP, temp))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_cmd_tz_get_gov(struct param *p)
> +{
> +	struct sk_buff *msg = p->msg;
> +	struct thermal_zone_device *tz;
> +	int id, ret = 0;
> +
> +	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
> +		return -EINVAL;
> +
> +	id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]);
> +
> +	tz = thermal_zone_get_by_id(id);
> +	if (!tz)
> +		return -EINVAL;
> +
> +	mutex_lock(&tz->lock);
> +
> +	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, id) ||
> +	    nla_put_string(msg, THERMAL_GENL_ATTR_TZ_GOV_NAME,
> +			   tz->governor->name))
> +		ret = -EMSGSIZE;
> +
> +	mutex_unlock(&tz->lock);
> +
> +	return ret;
> +}
> +
> +static int __thermal_genl_cmd_cdev_get(struct thermal_cooling_device
> *cdev,
> +				       void *data)
> +{
> +	struct sk_buff *msg = data;
> +
> +	if (nla_put_u32(msg, THERMAL_GENL_ATTR_CDEV_ID, cdev->id))
> +		return -EMSGSIZE;
> +
> +	if (nla_put_string(msg, THERMAL_GENL_ATTR_CDEV_NAME, cdev-
> >type))
> +		return -EMSGSIZE;
> +
> +	return 0;
> +}
> +
> +static int thermal_genl_cmd_cdev_get(struct param *p)
> +{
> +	struct sk_buff *msg = p->msg;
> +	struct nlattr *start_cdev;
> +	int ret;
> +
> +	start_cdev = nla_nest_start(msg, THERMAL_GENL_ATTR_CDEV);
> +	if (!start_cdev)
> +		return -EMSGSIZE;
> +
> +	ret =
> for_each_thermal_cooling_device(__thermal_genl_cmd_cdev_get, msg);
> +	if (ret)
> +		goto out_cancel_nest;
> +
> +	nla_nest_end(msg, start_cdev);
> +
> +	return 0;
> +out_cancel_nest:
> +	nla_nest_cancel(msg, start_cdev);
> +
> +	return ret;
> +}
> +
> +static cb_t cmd_cb[] = {
> +	[THERMAL_GENL_CMD_TZ_GET_ID]	= thermal_genl_cmd_tz_get_id,
> +	[THERMAL_GENL_CMD_TZ_GET_TRIP]	=
> thermal_genl_cmd_tz_get_trip,
> +	[THERMAL_GENL_CMD_TZ_GET_TEMP]	=
> thermal_genl_cmd_tz_get_temp,
> +	[THERMAL_GENL_CMD_TZ_GET_GOV]	=
> thermal_genl_cmd_tz_get_gov,
> +	[THERMAL_GENL_CMD_CDEV_GET]	= thermal_genl_cmd_cdev_get,
> +};
> +
> +static int thermal_genl_cmd_dumpit(struct sk_buff *skb,
> +				   struct netlink_callback *cb)
> +{
> +	struct param p = { .msg = skb };
> +	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
> +	int cmd = info->ops->cmd;
> +	int ret = -EMSGSIZE;
> +	void *hdr;
> +
> +	hdr = genlmsg_put(skb, 0, 0, &thermal_gnl_family, 0, cmd);
> +	if (!hdr)
> +		return -EMSGSIZE;
> +
> +	ret = cmd_cb[cmd](&p);
> +	if (ret)
> +		goto out_cancel_msg;
> +
> +	genlmsg_end(skb, hdr);
> +
> +	return 0;
> +
> +out_cancel_msg:
> +	genlmsg_cancel(skb, hdr);
> +
> +	return ret;
> +}
> +
> +static int thermal_genl_cmd_doit(struct sk_buff *skb,
> +				 struct genl_info *info)
> +{
> +	struct param p = { .attrs = info->attrs };
> +	struct sk_buff *msg;
> +	void *hdr;
> +	int cmd = info->genlhdr->cmd;
> +	int ret = -EMSGSIZE;
> +
> +	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +	p.msg = msg;
> +
> +	hdr = genlmsg_put_reply(msg, info, &thermal_gnl_family, 0,
> cmd);
> +	if (!hdr)
> +		goto out_free_msg;
> +
> +	ret = cmd_cb[cmd](&p);
> +	if (ret)
> +		goto out_cancel_msg;
> +
> +	genlmsg_end(msg, hdr);
> +
> +	return genlmsg_reply(msg, info);
> +
> +out_cancel_msg:
> +	genlmsg_cancel(msg, hdr);
> +out_free_msg:
> +	nlmsg_free(msg);
> +
> +	return ret;
> +}
> +
> +static const struct genl_ops thermal_genl_ops[] = {
> +	{
> +		.cmd = THERMAL_GENL_CMD_TZ_GET_ID,
> +		.validate = GENL_DONT_VALIDATE_STRICT |
> GENL_DONT_VALIDATE_DUMP,
> +		.dumpit = thermal_genl_cmd_dumpit,
> +	},
> +	{
> +		.cmd = THERMAL_GENL_CMD_TZ_GET_TRIP,
> +		.validate = GENL_DONT_VALIDATE_STRICT |
> GENL_DONT_VALIDATE_DUMP,
> +		.doit = thermal_genl_cmd_doit,
> +	},
> +	{
> +		.cmd = THERMAL_GENL_CMD_TZ_GET_TEMP,
> +		.validate = GENL_DONT_VALIDATE_STRICT |
> GENL_DONT_VALIDATE_DUMP,
> +		.doit = thermal_genl_cmd_doit,
> +	},
> +	{
> +		.cmd = THERMAL_GENL_CMD_TZ_GET_GOV,
> +		.validate = GENL_DONT_VALIDATE_STRICT |
> GENL_DONT_VALIDATE_DUMP,
> +		.doit = thermal_genl_cmd_doit,
> +	},
> +	{
> +		.cmd = THERMAL_GENL_CMD_CDEV_GET,
> +		.validate = GENL_DONT_VALIDATE_STRICT |
> GENL_DONT_VALIDATE_DUMP,
> +		.dumpit = thermal_genl_cmd_dumpit,
> +	},
> +};
> +
> +static struct genl_family thermal_gnl_family __ro_after_init = {
> +	.hdrsize	= 0,
> +	.name		= THERMAL_GENL_FAMILY_NAME,
> +	.version	= THERMAL_GENL_VERSION,
> +	.maxattr	= THERMAL_GENL_ATTR_MAX,
> +	.policy		= thermal_genl_policy,
> +	.ops		= thermal_genl_ops,
> +	.n_ops		= ARRAY_SIZE(thermal_genl_ops),
> +	.mcgrps		= thermal_genl_mcgrps,
> +	.n_mcgrps	= ARRAY_SIZE(thermal_genl_mcgrps),
> +};
> +
> +static int __init thermal_netlink_init(void)
> +{
> +	return genl_register_family(&thermal_gnl_family);
> +}
> +core_initcall(thermal_netlink_init);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 216185bb3014..3d4d8ae8c16a 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -37,18 +37,6 @@ struct thermal_cooling_device;
>  struct thermal_instance;
>  struct thermal_attr;
>  
> -enum thermal_device_mode {
> -	THERMAL_DEVICE_DISABLED = 0,
> -	THERMAL_DEVICE_ENABLED,
> -};
> -
> -enum thermal_trip_type {
> -	THERMAL_TRIP_ACTIVE = 0,
> -	THERMAL_TRIP_PASSIVE,
> -	THERMAL_TRIP_HOT,
> -	THERMAL_TRIP_CRITICAL,
> -};
> -
>  enum thermal_trend {
>  	THERMAL_TREND_STABLE, /* temperature is stable */
>  	THERMAL_TREND_RAISING, /* temperature is raising */
> @@ -303,11 +291,6 @@ struct thermal_zone_params {
>  	int offset;
>  };
>  
> -struct thermal_genl_event {
> -	u32 orig;
> -	enum events event;
> -};
> -
>  /**
>   * struct thermal_zone_of_device_ops - scallbacks for handling DT
> based zones
>   *
> diff --git a/include/uapi/linux/thermal.h
> b/include/uapi/linux/thermal.h
> index 96218378dda8..c105054cbb57 100644
> --- a/include/uapi/linux/thermal.h
> +++ b/include/uapi/linux/thermal.h
> @@ -4,31 +4,86 @@
>  
>  #define THERMAL_NAME_LENGTH	20
>  
> -/* Adding event notification support elements */
> -#define THERMAL_GENL_FAMILY_NAME                "thermal_event"
> -#define THERMAL_GENL_VERSION                    0x01
> -#define THERMAL_GENL_MCAST_GROUP_NAME           "thermal_mc_grp"
> -
> -/* Events supported by Thermal Netlink */
> -enum events {
> -	THERMAL_AUX0,
> -	THERMAL_AUX1,
> -	THERMAL_CRITICAL,
> -	THERMAL_DEV_FAULT,
> +enum thermal_device_mode {
> +	THERMAL_DEVICE_DISABLED = 0,
> +	THERMAL_DEVICE_ENABLED,
> +};
> +
> +enum thermal_trip_type {
> +	THERMAL_TRIP_ACTIVE = 0,
> +	THERMAL_TRIP_PASSIVE,
> +	THERMAL_TRIP_HOT,
> +	THERMAL_TRIP_CRITICAL,
>  };
>  
> -/* attributes of thermal_genl_family */
> -enum {
> +/* Adding event notification support elements */
> +#define THERMAL_GENL_FAMILY_NAME		"thermal"
> +#define THERMAL_GENL_VERSION			0x01
> +#define THERMAL_GENL_SAMPLING_GROUP_NAME	"sampling"
> +#define THERMAL_GENL_EVENT_GROUP_NAME		"event"
> +
> +/* Attributes of thermal_genl_family */
> +enum thermal_genl_attr {
>  	THERMAL_GENL_ATTR_UNSPEC,
> -	THERMAL_GENL_ATTR_EVENT,
> +	THERMAL_GENL_ATTR_TZ,
> +	THERMAL_GENL_ATTR_TZ_ID,
> +	THERMAL_GENL_ATTR_TZ_TEMP,
> +	THERMAL_GENL_ATTR_TZ_TRIP,
> +	THERMAL_GENL_ATTR_TZ_TRIP_ID,
> +	THERMAL_GENL_ATTR_TZ_TRIP_TYPE,
> +	THERMAL_GENL_ATTR_TZ_TRIP_TEMP,
> +	THERMAL_GENL_ATTR_TZ_TRIP_HYST,
> +	THERMAL_GENL_ATTR_TZ_MODE,
> +	THERMAL_GENL_ATTR_TZ_NAME,
> +	THERMAL_GENL_ATTR_TZ_CDEV_WEIGHT,
> +	THERMAL_GENL_ATTR_TZ_GOV,
> +	THERMAL_GENL_ATTR_TZ_GOV_NAME,
> +	THERMAL_GENL_ATTR_CDEV,
> +	THERMAL_GENL_ATTR_CDEV_ID,
> +	THERMAL_GENL_ATTR_CDEV_CUR_STATE,
> +	THERMAL_GENL_ATTR_CDEV_MAX_STATE,
> +	THERMAL_GENL_ATTR_CDEV_NAME,
> +	THERMAL_GENL_ATTR_GOV_NAME,
> +
>  	__THERMAL_GENL_ATTR_MAX,
>  };
>  #define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
>  
> -/* commands supported by the thermal_genl_family */
> -enum {
> +enum thermal_genl_sampling {
> +	THERMAL_GENL_SAMPLING_TEMP,
> +	__THERMAL_GENL_SAMPLING_MAX,
> +};
> +#define THERMAL_GENL_SAMPLING_MAX (__THERMAL_GENL_SAMPLING_MAX - 1)
> +
> +/* Events of thermal_genl_family */
> +enum thermal_genl_event {
> +	THERMAL_GENL_EVENT_UNSPEC,
> +	THERMAL_GENL_EVENT_TZ_CREATE,		/* Thermal zone
> creation */
> +	THERMAL_GENL_EVENT_TZ_DELETE,		/* Thermal zone
> deletion */
> +	THERMAL_GENL_EVENT_TZ_DISABLE,		/* Thermal zone
> disabed */
> +	THERMAL_GENL_EVENT_TZ_ENABLE,		/* Thermal zone
> enabled */
> +	THERMAL_GENL_EVENT_TZ_TRIP_UP,		/* Trip point crossed
> the way up */
> +	THERMAL_GENL_EVENT_TZ_TRIP_DOWN,	/* Trip point crossed the way
> down */
> +	THERMAL_GENL_EVENT_TZ_TRIP_CHANGE,	/* Trip point changed
> */
> +	THERMAL_GENL_EVENT_TZ_TRIP_ADD,		/* Trip point added
> */
> +	THERMAL_GENL_EVENT_TZ_TRIP_DELETE,	/* Trip point deleted
> */
> +	THERMAL_GENL_EVENT_CDEV_ADD,		/* Cdev bound to the
> thermal zone */
> +	THERMAL_GENL_EVENT_CDEV_DELETE,		/* Cdev unbound */
> +	THERMAL_GENL_EVENT_CDEV_STATE_UPDATE,	/* Cdev state updated
> */
> +	THERMAL_GENL_EVENT_TZ_GOV_CHANGE,	/* Governor policy
> changed  */
> +	__THERMAL_GENL_EVENT_MAX,
> +};
> +#define THERMAL_GENL_EVENT_MAX (__THERMAL_GENL_EVENT_MAX - 1)
> +
> +/* Commands supported by the thermal_genl_family */
> +enum thermal_genl_cmd {
>  	THERMAL_GENL_CMD_UNSPEC,
> -	THERMAL_GENL_CMD_EVENT,
> +	THERMAL_GENL_CMD_TZ_GET_ID,	/* List of thermal zones id
> */
> +	THERMAL_GENL_CMD_TZ_GET_TRIP,	/* List of thermal trips */
> +	THERMAL_GENL_CMD_TZ_GET_TEMP,	/* Get the thermal zone
> temperature */
> +	THERMAL_GENL_CMD_TZ_GET_GOV,	/* Get the thermal zone
> governor */
> +	THERMAL_GENL_CMD_TZ_GET_MODE,	/* Get the thermal zone mode
> */
> +	THERMAL_GENL_CMD_CDEV_GET,	/* List of cdev id */
>  	__THERMAL_GENL_CMD_MAX,
>  };
>  #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-06 10:55 ` [PATCH v4 4/4] thermal: core: Add notifications call in the framework Daniel Lezcano
       [not found]   ` <CGME20200706131708eucas1p1487955a7632584c17df724399f48825a@eucas1p1.samsung.com>
@ 2020-07-07  1:55   ` Zhang Rui
  1 sibling, 0 replies; 16+ messages in thread
From: Zhang Rui @ 2020-07-07  1:55 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel, linux-pm

On Mon, 2020-07-06 at 12:55 +0200, Daniel Lezcano wrote:
> The generic netlink protocol is implemented but the different
> notification functions are not yet connected to the core code.
> 
> These changes add the notification calls in the different
> corresponding places.
> 
> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Acked-by: Zhang Rui <rui.zhang@intel.com>
> ---
>   v4:
>      - Fixed missing static declaration, reported by kbuild-bot
>      - Removed max state notification
> ---
>  drivers/thermal/thermal_core.c    | 21 +++++++++++++++++++++
>  drivers/thermal/thermal_helpers.c | 13 +++++++++++--
>  drivers/thermal/thermal_sysfs.c   | 15 ++++++++++++++-
>  3 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 5fae1621fb01..25ef29123f72 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -215,6 +215,8 @@ int thermal_zone_device_set_policy(struct
> thermal_zone_device *tz,
>  	mutex_unlock(&tz->lock);
>  	mutex_unlock(&thermal_governor_lock);
>  
> +	thermal_notify_tz_gov_change(tz->id, policy);
> +
>  	return ret;
>  }
>  
> @@ -406,12 +408,25 @@ static void handle_critical_trips(struct
> thermal_zone_device *tz,
>  static void handle_thermal_trip(struct thermal_zone_device *tz, int
> trip)
>  {
>  	enum thermal_trip_type type;
> +	int trip_temp, hyst = 0;
>  
>  	/* Ignore disabled trip points */
>  	if (test_bit(trip, &tz->trips_disabled))
>  		return;
>  
> +	tz->ops->get_trip_temp(tz, trip, &trip_temp);
>  	tz->ops->get_trip_type(tz, trip, &type);
> +	if (tz->ops->get_trip_hyst)
> +		tz->ops->get_trip_hyst(tz, trip, &hyst);
> +
> +	if (tz->last_temperature != THERMAL_TEMP_INVALID) {
> +		if (tz->last_temperature < trip_temp &&
> +		    tz->temperature >= trip_temp)
> +			thermal_notify_tz_trip_up(tz->id, trip);
> +		if (tz->last_temperature >= trip_temp &&
> +		    tz->temperature < (trip_temp - hyst))
> +			thermal_notify_tz_trip_down(tz->id, trip);
> +	}
>  
>  	if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
>  		handle_critical_trips(tz, trip, type);
> @@ -443,6 +458,8 @@ static void update_temperature(struct
> thermal_zone_device *tz)
>  	mutex_unlock(&tz->lock);
>  
>  	trace_thermal_temperature(tz);
> +
> +	thermal_genl_sampling_temp(tz->id, temp);
>  }
>  
>  static void thermal_zone_device_init(struct thermal_zone_device *tz)
> @@ -1405,6 +1422,8 @@ thermal_zone_device_register(const char *type,
> int trips, int mask,
>  	if (atomic_cmpxchg(&tz->need_update, 1, 0))
>  		thermal_zone_device_update(tz,
> THERMAL_EVENT_UNSPECIFIED);
>  
> +	thermal_notify_tz_create(tz->id, tz->type);
> +
>  	return tz;
>  
>  unregister:
> @@ -1476,6 +1495,8 @@ void thermal_zone_device_unregister(struct
> thermal_zone_device *tz)
>  	ida_destroy(&tz->ida);
>  	mutex_destroy(&tz->lock);
>  	device_unregister(&tz->device);
> +
> +	thermal_notify_tz_delete(tz->id);
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>  
> diff --git a/drivers/thermal/thermal_helpers.c
> b/drivers/thermal/thermal_helpers.c
> index 87b1256fa2f2..c94bc824e5d3 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -175,6 +175,16 @@ void thermal_zone_set_trips(struct
> thermal_zone_device *tz)
>  	mutex_unlock(&tz->lock);
>  }
>  
> +static void thermal_cdev_set_cur_state(struct thermal_cooling_device
> *cdev,
> +				       int target)
> +{
> +	if (cdev->ops->set_cur_state(cdev, target))
> +		return;
> +
> +	thermal_notify_cdev_state_update(cdev->id, target);
> +	thermal_cooling_device_stats_update(cdev, target);
> +}
> +
>  void thermal_cdev_update(struct thermal_cooling_device *cdev)
>  {
>  	struct thermal_instance *instance;
> @@ -197,8 +207,7 @@ void thermal_cdev_update(struct
> thermal_cooling_device *cdev)
>  			target = instance->target;
>  	}
>  
> -	if (!cdev->ops->set_cur_state(cdev, target))
> -		thermal_cooling_device_stats_update(cdev, target);
> +	thermal_cdev_set_cur_state(cdev, target);
>  
>  	cdev->updated = true;
>  	mutex_unlock(&cdev->lock);
> diff --git a/drivers/thermal/thermal_sysfs.c
> b/drivers/thermal/thermal_sysfs.c
> index aa99edb4dff7..ff449943f757 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -124,7 +124,8 @@ trip_point_temp_store(struct device *dev, struct
> device_attribute *attr,
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>  	int trip, ret;
> -	int temperature;
> +	int temperature, hyst = 0;
> +	enum thermal_trip_type type;
>  
>  	if (!tz->ops->set_trip_temp)
>  		return -EPERM;
> @@ -139,6 +140,18 @@ trip_point_temp_store(struct device *dev, struct
> device_attribute *attr,
>  	if (ret)
>  		return ret;
>  
> +	if (tz->ops->get_trip_hyst) {
> +		ret = tz->ops->get_trip_hyst(tz, trip, &hyst);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = tz->ops->get_trip_type(tz, trip, &type);
> +	if (ret)
> +		return ret;
> +
> +	thermal_notify_tz_trip_change(tz->id, trip, type, temperature,
> hyst);
> +
>  	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>  
>  	return count;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-06 13:46       ` Daniel Lezcano
@ 2020-07-07  9:15         ` Marek Szyprowski
  2020-07-13  9:31           ` Marek Szyprowski
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Szyprowski @ 2020-07-07  9:15 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz

Hi Daniel,

On 06.07.2020 15:46, Daniel Lezcano wrote:
> On 06/07/2020 15:17, Marek Szyprowski wrote:
>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>> The generic netlink protocol is implemented but the different
>>> notification functions are not yet connected to the core code.
>>>
>>> These changes add the notification calls in the different
>>> corresponding places.
>>>
>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>> ("thermal: core: Add notifications call in the framework"). Sadly it
>> breaks booting various Samsung Exynos based boards. Here is an example
>> log from Odroid U3 board:
>>
>> Unable to handle kernel NULL pointer dereference at virtual address 00000010
>> pgd = (ptrval)
>> [00000010] *pgd=00000000
>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
>> #1146
>> Hardware name: Samsung Exynos (Flattened Device Tree)
>> PC is at kmem_cache_alloc+0x13c/0x418
>> LR is at kmem_cache_alloc+0x48/0x418
>> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
>> ...
>> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
>> Control: 10c5387d  Table: 4000404a  DAC: 00000051
>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>> Stack: (0xee8f1cf8 to 0xee8f2000)
>> ...
>> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] (__alloc_skb+0x5c/0x170)
>> [<c08cd170>] (__alloc_skb) from [<c07ec19c>]
>> (thermal_genl_send_event+0x24/0x174)
>> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
>> (thermal_notify_tz_create+0x58/0x74)
>> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
>> (thermal_zone_device_register+0x358/0x650)
>> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
>> (of_parse_thermal_zones+0x304/0x7a4)
>> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
>> (thermal_init+0xdc/0x154)
>> [<c1028964>] (thermal_init) from [<c0102378>] (do_one_initcall+0x8c/0x424)
>> [<c0102378>] (do_one_initcall) from [<c1001158>]
>> (kernel_init_freeable+0x190/0x204)
>> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
>> (kernel_init+0x8/0x118)
>> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
>>
>> Reverting it on top of linux-next fixes the boot issue. I will
>> investigate it further soon.
> Thanks for reporting this.
>
> Can you send the addr2line result and code it points to ?

addr2line of c02b5cac (kmem_cache_alloc+0x13c/0x418) points to mm/slub.c 
+2839, but I'm not sure if we can trust it. imho it looks like some 
trashed memory somewhere, but I don't have time right now to analyze it 
further now...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-07  9:15         ` Marek Szyprowski
@ 2020-07-13  9:31           ` Marek Szyprowski
  2020-07-13  9:45             ` Daniel Lezcano
                               ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Marek Szyprowski @ 2020-07-13  9:31 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz

Hi

On 07.07.2020 11:15, Marek Szyprowski wrote:
> On 06.07.2020 15:46, Daniel Lezcano wrote:
>> On 06/07/2020 15:17, Marek Szyprowski wrote:
>>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>>> The generic netlink protocol is implemented but the different
>>>> notification functions are not yet connected to the core code.
>>>>
>>>> These changes add the notification calls in the different
>>>> corresponding places.
>>>>
>>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>>> ("thermal: core: Add notifications call in the framework"). Sadly it
>>> breaks booting various Samsung Exynos based boards. Here is an example
>>> log from Odroid U3 board:
>>>
>>> Unable to handle kernel NULL pointer dereference at virtual address 
>>> 00000010
>>> pgd = (ptrval)
>>> [00000010] *pgd=00000000
>>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
>>> Modules linked in:
>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
>>> #1146
>>> Hardware name: Samsung Exynos (Flattened Device Tree)
>>> PC is at kmem_cache_alloc+0x13c/0x418
>>> LR is at kmem_cache_alloc+0x48/0x418
>>> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
>>> ...
>>> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
>>> Control: 10c5387d  Table: 4000404a  DAC: 00000051
>>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>>> Stack: (0xee8f1cf8 to 0xee8f2000)
>>> ...
>>> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] 
>>> (__alloc_skb+0x5c/0x170)
>>> [<c08cd170>] (__alloc_skb) from [<c07ec19c>]
>>> (thermal_genl_send_event+0x24/0x174)
>>> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
>>> (thermal_notify_tz_create+0x58/0x74)
>>> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
>>> (thermal_zone_device_register+0x358/0x650)
>>> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
>>> (of_parse_thermal_zones+0x304/0x7a4)
>>> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
>>> (thermal_init+0xdc/0x154)
>>> [<c1028964>] (thermal_init) from [<c0102378>] 
>>> (do_one_initcall+0x8c/0x424)
>>> [<c0102378>] (do_one_initcall) from [<c1001158>]
>>> (kernel_init_freeable+0x190/0x204)
>>> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
>>> (kernel_init+0x8/0x118)
>>> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
>>>
>>> Reverting it on top of linux-next fixes the boot issue. I will
>>> investigate it further soon.
>> Thanks for reporting this.
>>
>> Can you send the addr2line result and code it points to ?
>
> addr2line of c02b5cac (kmem_cache_alloc+0x13c/0x418) points to 
> mm/slub.c +2839, but I'm not sure if we can trust it. imho it looks 
> like some trashed memory somewhere, but I don't have time right now to 
> analyze it further now...

Just one more thing I've noticed. The crash happens only if the kernel 
is compiled with old GCC (tested with arm-linux-gnueabi-gcc (Linaro GCC 
4.9-2017.01) 4.9.4). If I compile kernel with newed GCC (like 
arm-linux-gnueabi-gcc (Linaro GCC 6.4-2017.11) 6.4.1 20171012), it works 
fine...

This happens also with Linux next-20200710, which again got this commit.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-13  9:31           ` Marek Szyprowski
@ 2020-07-13  9:45             ` Daniel Lezcano
  2020-07-13 10:28             ` Daniel Lezcano
  2020-07-13 20:32             ` Daniel Lezcano
  2 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-13  9:45 UTC (permalink / raw)
  To: Marek Szyprowski, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz, Arnd Bergmann


Added Arnd in Cc.

On 13/07/2020 11:31, Marek Szyprowski wrote:
> Hi
> 
> On 07.07.2020 11:15, Marek Szyprowski wrote:
>> On 06.07.2020 15:46, Daniel Lezcano wrote:
>>> On 06/07/2020 15:17, Marek Szyprowski wrote:
>>>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>>>> The generic netlink protocol is implemented but the different
>>>>> notification functions are not yet connected to the core code.
>>>>>
>>>>> These changes add the notification calls in the different
>>>>> corresponding places.
>>>>>
>>>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>>>> ("thermal: core: Add notifications call in the framework"). Sadly it
>>>> breaks booting various Samsung Exynos based boards. Here is an example
>>>> log from Odroid U3 board:
>>>>
>>>> Unable to handle kernel NULL pointer dereference at virtual address 
>>>> 00000010
>>>> pgd = (ptrval)
>>>> [00000010] *pgd=00000000
>>>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
>>>> Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
>>>> #1146
>>>> Hardware name: Samsung Exynos (Flattened Device Tree)
>>>> PC is at kmem_cache_alloc+0x13c/0x418
>>>> LR is at kmem_cache_alloc+0x48/0x418
>>>> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
>>>> ...
>>>> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
>>>> Control: 10c5387d  Table: 4000404a  DAC: 00000051
>>>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>>>> Stack: (0xee8f1cf8 to 0xee8f2000)
>>>> ...
>>>> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] 
>>>> (__alloc_skb+0x5c/0x170)
>>>> [<c08cd170>] (__alloc_skb) from [<c07ec19c>]
>>>> (thermal_genl_send_event+0x24/0x174)
>>>> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
>>>> (thermal_notify_tz_create+0x58/0x74)
>>>> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
>>>> (thermal_zone_device_register+0x358/0x650)
>>>> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
>>>> (of_parse_thermal_zones+0x304/0x7a4)
>>>> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
>>>> (thermal_init+0xdc/0x154)
>>>> [<c1028964>] (thermal_init) from [<c0102378>] 
>>>> (do_one_initcall+0x8c/0x424)
>>>> [<c0102378>] (do_one_initcall) from [<c1001158>]
>>>> (kernel_init_freeable+0x190/0x204)
>>>> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
>>>> (kernel_init+0x8/0x118)
>>>> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
>>>>
>>>> Reverting it on top of linux-next fixes the boot issue. I will
>>>> investigate it further soon.
>>> Thanks for reporting this.
>>>
>>> Can you send the addr2line result and code it points to ?
>>
>> addr2line of c02b5cac (kmem_cache_alloc+0x13c/0x418) points to 
>> mm/slub.c +2839, but I'm not sure if we can trust it. imho it looks 
>> like some trashed memory somewhere, but I don't have time right now to 
>> analyze it further now...
> 
> Just one more thing I've noticed. The crash happens only if the kernel 
> is compiled with old GCC (tested with arm-linux-gnueabi-gcc (Linaro GCC 
> 4.9-2017.01) 4.9.4). If I compile kernel with newed GCC (like 
> arm-linux-gnueabi-gcc (Linaro GCC 6.4-2017.11) 6.4.1 20171012), it works 
> fine...
> 
> This happens also with Linux next-20200710, which again got this commit.
Arnd,

are you aware of any issue with this gcc version which can explain this
kernel panic ? Sounds like the problem does not appear with more recent
version.

-- 
<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] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-13  9:31           ` Marek Szyprowski
  2020-07-13  9:45             ` Daniel Lezcano
@ 2020-07-13 10:28             ` Daniel Lezcano
  2020-07-13 20:32             ` Daniel Lezcano
  2 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-13 10:28 UTC (permalink / raw)
  To: Marek Szyprowski, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz

On 13/07/2020 11:31, Marek Szyprowski wrote:
> Hi
> 
> On 07.07.2020 11:15, Marek Szyprowski wrote:
>> On 06.07.2020 15:46, Daniel Lezcano wrote:
>>> On 06/07/2020 15:17, Marek Szyprowski wrote:
>>>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>>>> The generic netlink protocol is implemented but the different
>>>>> notification functions are not yet connected to the core code.
>>>>>
>>>>> These changes add the notification calls in the different
>>>>> corresponding places.
>>>>>
>>>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>>>> ("thermal: core: Add notifications call in the framework"). Sadly it
>>>> breaks booting various Samsung Exynos based boards. Here is an example
>>>> log from Odroid U3 board:

Does it break also arm64 platforms?


-- 
<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] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-13  9:31           ` Marek Szyprowski
  2020-07-13  9:45             ` Daniel Lezcano
  2020-07-13 10:28             ` Daniel Lezcano
@ 2020-07-13 20:32             ` Daniel Lezcano
  2020-07-14 23:20               ` Daniel Lezcano
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-13 20:32 UTC (permalink / raw)
  To: Marek Szyprowski, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz

On 13/07/2020 11:31, Marek Szyprowski wrote:
> Hi
> 
> On 07.07.2020 11:15, Marek Szyprowski wrote:
>> On 06.07.2020 15:46, Daniel Lezcano wrote:
>>> On 06/07/2020 15:17, Marek Szyprowski wrote:
>>>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>>>> The generic netlink protocol is implemented but the different
>>>>> notification functions are not yet connected to the core code.
>>>>>
>>>>> These changes add the notification calls in the different
>>>>> corresponding places.
>>>>>
>>>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>>>> ("thermal: core: Add notifications call in the framework"). Sadly it
>>>> breaks booting various Samsung Exynos based boards. Here is an example
>>>> log from Odroid U3 board:
>>>>
>>>> Unable to handle kernel NULL pointer dereference at virtual address 
>>>> 00000010
>>>> pgd = (ptrval)
>>>> [00000010] *pgd=00000000
>>>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
>>>> Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
>>>> #1146
>>>> Hardware name: Samsung Exynos (Flattened Device Tree)
>>>> PC is at kmem_cache_alloc+0x13c/0x418
>>>> LR is at kmem_cache_alloc+0x48/0x418
>>>> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
>>>> ...
>>>> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
>>>> Control: 10c5387d  Table: 4000404a  DAC: 00000051
>>>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>>>> Stack: (0xee8f1cf8 to 0xee8f2000)
>>>> ...
>>>> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] 
>>>> (__alloc_skb+0x5c/0x170)
>>>> [<c08cd170>] (__alloc_skb) from [<c07ec19c>]
>>>> (thermal_genl_send_event+0x24/0x174)
>>>> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
>>>> (thermal_notify_tz_create+0x58/0x74)
>>>> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
>>>> (thermal_zone_device_register+0x358/0x650)
>>>> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
>>>> (of_parse_thermal_zones+0x304/0x7a4)
>>>> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
>>>> (thermal_init+0xdc/0x154)
>>>> [<c1028964>] (thermal_init) from [<c0102378>] 
>>>> (do_one_initcall+0x8c/0x424)
>>>> [<c0102378>] (do_one_initcall) from [<c1001158>]
>>>> (kernel_init_freeable+0x190/0x204)
>>>> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
>>>> (kernel_init+0x8/0x118)
>>>> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
>>>>
>>>> Reverting it on top of linux-next fixes the boot issue. I will
>>>> investigate it further soon.
>>> Thanks for reporting this.
>>>
>>> Can you send the addr2line result and code it points to ?
>>
>> addr2line of c02b5cac (kmem_cache_alloc+0x13c/0x418) points to 
>> mm/slub.c +2839, but I'm not sure if we can trust it. imho it looks 
>> like some trashed memory somewhere, but I don't have time right now to 
>> analyze it further now...
> 
> Just one more thing I've noticed. The crash happens only if the kernel 
> is compiled with old GCC (tested with arm-linux-gnueabi-gcc (Linaro GCC 
> 4.9-2017.01) 4.9.4). If I compile kernel with newed GCC (like 
> arm-linux-gnueabi-gcc (Linaro GCC 6.4-2017.11) 6.4.1 20171012), it works 
> fine...
> 
> This happens also with Linux next-20200710, which again got this commit.

So I finally succeed to reproduce on an ARM64 with a recent compiler,
earlycon, and the option CONFIG_INIT_ON_ALLOC_DEFAULT_ON.



-- 
<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] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-13 20:32             ` Daniel Lezcano
@ 2020-07-14 23:20               ` Daniel Lezcano
  2020-07-15  6:09                 ` Marek Szyprowski
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-14 23:20 UTC (permalink / raw)
  To: Marek Szyprowski, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz, Arnd Bergmann

On 13/07/2020 22:32, Daniel Lezcano wrote:
> On 13/07/2020 11:31, Marek Szyprowski wrote:
>> Hi
>>
>> On 07.07.2020 11:15, Marek Szyprowski wrote:
>>> On 06.07.2020 15:46, Daniel Lezcano wrote:
>>>> On 06/07/2020 15:17, Marek Szyprowski wrote:
>>>>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>>>>> The generic netlink protocol is implemented but the different
>>>>>> notification functions are not yet connected to the core code.
>>>>>>
>>>>>> These changes add the notification calls in the different
>>>>>> corresponding places.
>>>>>>
>>>>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>>>>> ("thermal: core: Add notifications call in the framework"). Sadly it
>>>>> breaks booting various Samsung Exynos based boards. Here is an example
>>>>> log from Odroid U3 board:
>>>>>
>>>>> Unable to handle kernel NULL pointer dereference at virtual address 
>>>>> 00000010
>>>>> pgd = (ptrval)
>>>>> [00000010] *pgd=00000000
>>>>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
>>>>> Modules linked in:
>>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
>>>>> #1146
>>>>> Hardware name: Samsung Exynos (Flattened Device Tree)
>>>>> PC is at kmem_cache_alloc+0x13c/0x418
>>>>> LR is at kmem_cache_alloc+0x48/0x418
>>>>> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
>>>>> ...
>>>>> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
>>>>> Control: 10c5387d  Table: 4000404a  DAC: 00000051
>>>>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>>>>> Stack: (0xee8f1cf8 to 0xee8f2000)
>>>>> ...
>>>>> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] 
>>>>> (__alloc_skb+0x5c/0x170)
>>>>> [<c08cd170>] (__alloc_skb) from [<c07ec19c>]
>>>>> (thermal_genl_send_event+0x24/0x174)
>>>>> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
>>>>> (thermal_notify_tz_create+0x58/0x74)
>>>>> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
>>>>> (thermal_zone_device_register+0x358/0x650)
>>>>> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
>>>>> (of_parse_thermal_zones+0x304/0x7a4)
>>>>> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
>>>>> (thermal_init+0xdc/0x154)
>>>>> [<c1028964>] (thermal_init) from [<c0102378>] 
>>>>> (do_one_initcall+0x8c/0x424)
>>>>> [<c0102378>] (do_one_initcall) from [<c1001158>]
>>>>> (kernel_init_freeable+0x190/0x204)
>>>>> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
>>>>> (kernel_init+0x8/0x118)
>>>>> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
>>>>>
>>>>> Reverting it on top of linux-next fixes the boot issue. I will
>>>>> investigate it further soon.
>>>> Thanks for reporting this.
>>>>
>>>> Can you send the addr2line result and code it points to ?
>>>
>>> addr2line of c02b5cac (kmem_cache_alloc+0x13c/0x418) points to 
>>> mm/slub.c +2839, but I'm not sure if we can trust it. imho it looks 
>>> like some trashed memory somewhere, but I don't have time right now to 
>>> analyze it further now...
>>
>> Just one more thing I've noticed. The crash happens only if the kernel 
>> is compiled with old GCC (tested with arm-linux-gnueabi-gcc (Linaro GCC 
>> 4.9-2017.01) 4.9.4). If I compile kernel with newed GCC (like 
>> arm-linux-gnueabi-gcc (Linaro GCC 6.4-2017.11) 6.4.1 20171012), it works 
>> fine...
>>
>> This happens also with Linux next-20200710, which again got this commit.
> 
> So I finally succeed to reproduce on an ARM64 with a recent compiler,
> earlycon, and the option CONFIG_INIT_ON_ALLOC_DEFAULT_ON.


Finally, narrowed down the issue.

 - genetlink initialization is done at subsys initcall.
 - thermal netlink init is done at core initcall
 - netlink is done at core initcall

By changing the order:

 - netlink and genetlink at core initcall
 - thermal init at postcore initcall

That fixes the problem. The genetlink initcall order is from 2005 and
IMO it makes sense to come right after the netlink initialization.

It is acceptable to have the thermal init at the postcore initcall. It
is very recently we moved from fs_initcall to core_initcall.

Thanks to Arnd who give me a direction to look at.

-- 
<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] 16+ messages in thread

* Re: [PATCH v4 4/4] thermal: core: Add notifications call in the framework
  2020-07-14 23:20               ` Daniel Lezcano
@ 2020-07-15  6:09                 ` Marek Szyprowski
  0 siblings, 0 replies; 16+ messages in thread
From: Marek Szyprowski @ 2020-07-15  6:09 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang
  Cc: srinivas.pandruvada, rkumbako, amit.kucheria, linux-kernel,
	linux-pm, Bartlomiej Zolnierkiewicz, Arnd Bergmann

Hi Daniel,

On 15.07.2020 01:20, Daniel Lezcano wrote:
> On 13/07/2020 22:32, Daniel Lezcano wrote:
>> On 13/07/2020 11:31, Marek Szyprowski wrote:
>>> On 07.07.2020 11:15, Marek Szyprowski wrote:
>>>> On 06.07.2020 15:46, Daniel Lezcano wrote:
>>>>> On 06/07/2020 15:17, Marek Szyprowski wrote:
>>>>>> On 06.07.2020 12:55, Daniel Lezcano wrote:
>>>>>>> The generic netlink protocol is implemented but the different
>>>>>>> notification functions are not yet connected to the core code.
>>>>>>>
>>>>>>> These changes add the notification calls in the different
>>>>>>> corresponding places.
>>>>>>>
>>>>>>> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
>>>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>>>> This patch landed in today's linux-next 20200706 as commit 5df786e46560
>>>>>> ("thermal: core: Add notifications call in the framework"). Sadly it
>>>>>> breaks booting various Samsung Exynos based boards. Here is an example
>>>>>> log from Odroid U3 board:
>>>>>>
>>>>>> Unable to handle kernel NULL pointer dereference at virtual address
>>>>>> 00000010
>>>>>> pgd = (ptrval)
>>>>>> [00000010] *pgd=00000000
>>>>>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
>>>>>> Modules linked in:
>>>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
>>>>>> #1146
>>>>>> Hardware name: Samsung Exynos (Flattened Device Tree)
>>>>>> PC is at kmem_cache_alloc+0x13c/0x418
>>>>>> LR is at kmem_cache_alloc+0x48/0x418
>>>>>> pc : [<c02b5cac>]    lr : [<c02b5bb8>]    psr: 20000053
>>>>>> ...
>>>>>> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
>>>>>> Control: 10c5387d  Table: 4000404a  DAC: 00000051
>>>>>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>>>>>> Stack: (0xee8f1cf8 to 0xee8f2000)
>>>>>> ...
>>>>>> [<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>]
>>>>>> (__alloc_skb+0x5c/0x170)
>>>>>> [<c08cd170>] (__alloc_skb) from [<c07ec19c>]
>>>>>> (thermal_genl_send_event+0x24/0x174)
>>>>>> [<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
>>>>>> (thermal_notify_tz_create+0x58/0x74)
>>>>>> [<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
>>>>>> (thermal_zone_device_register+0x358/0x650)
>>>>>> [<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
>>>>>> (of_parse_thermal_zones+0x304/0x7a4)
>>>>>> [<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
>>>>>> (thermal_init+0xdc/0x154)
>>>>>> [<c1028964>] (thermal_init) from [<c0102378>]
>>>>>> (do_one_initcall+0x8c/0x424)
>>>>>> [<c0102378>] (do_one_initcall) from [<c1001158>]
>>>>>> (kernel_init_freeable+0x190/0x204)
>>>>>> [<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
>>>>>> (kernel_init+0x8/0x118)
>>>>>> [<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
>>>>>>
>>>>>> Reverting it on top of linux-next fixes the boot issue. I will
>>>>>> investigate it further soon.
>>>>> Thanks for reporting this.
>>>>>
>>>>> Can you send the addr2line result and code it points to ?
>>>> addr2line of c02b5cac (kmem_cache_alloc+0x13c/0x418) points to
>>>> mm/slub.c +2839, but I'm not sure if we can trust it. imho it looks
>>>> like some trashed memory somewhere, but I don't have time right now to
>>>> analyze it further now...
>>> Just one more thing I've noticed. The crash happens only if the kernel
>>> is compiled with old GCC (tested with arm-linux-gnueabi-gcc (Linaro GCC
>>> 4.9-2017.01) 4.9.4). If I compile kernel with newed GCC (like
>>> arm-linux-gnueabi-gcc (Linaro GCC 6.4-2017.11) 6.4.1 20171012), it works
>>> fine...
>>>
>>> This happens also with Linux next-20200710, which again got this commit.
>> So I finally succeed to reproduce on an ARM64 with a recent compiler,
>> earlycon, and the option CONFIG_INIT_ON_ALLOC_DEFAULT_ON.
>
> Finally, narrowed down the issue.
>
>   - genetlink initialization is done at subsys initcall.
>   - thermal netlink init is done at core initcall
>   - netlink is done at core initcall
>
> By changing the order:
>
>   - netlink and genetlink at core initcall
>   - thermal init at postcore initcall
>
> That fixes the problem.
I confirm that such change fixes the issue! Feel free to add:

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

to the final patch.

> The genetlink initcall order is from 2005 and
> IMO it makes sense to come right after the netlink initialization.
>
> It is acceptable to have the thermal init at the postcore initcall. It
> is very recently we moved from fs_initcall to core_initcall.
>
> Thanks to Arnd who give me a direction to look at.
Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-07-15  6:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 10:55 [PATCH v4 1/4] thermal: core: Add helpers to browse the cdev, tz and governor list Daniel Lezcano
2020-07-06 10:55 ` [PATCH v4 2/4] thermal: core: Get thermal zone by id Daniel Lezcano
2020-07-07  1:53   ` Zhang Rui
2020-07-06 10:55 ` [PATCH v4 3/4] thermal: core: genetlink support for events/cmd/sampling Daniel Lezcano
2020-07-07  1:54   ` Zhang Rui
2020-07-06 10:55 ` [PATCH v4 4/4] thermal: core: Add notifications call in the framework Daniel Lezcano
     [not found]   ` <CGME20200706131708eucas1p1487955a7632584c17df724399f48825a@eucas1p1.samsung.com>
2020-07-06 13:17     ` Marek Szyprowski
2020-07-06 13:46       ` Daniel Lezcano
2020-07-07  9:15         ` Marek Szyprowski
2020-07-13  9:31           ` Marek Szyprowski
2020-07-13  9:45             ` Daniel Lezcano
2020-07-13 10:28             ` Daniel Lezcano
2020-07-13 20:32             ` Daniel Lezcano
2020-07-14 23:20               ` Daniel Lezcano
2020-07-15  6:09                 ` Marek Szyprowski
2020-07-07  1:55   ` Zhang Rui

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.