On 12/6/18 8:18 PM, Lukasz Luba wrote:

Hi Rui,

On 12/5/18 4:09 PM, Zhang Rui wrote:

quoted

On 三, 2018-11-07 at 18:09 +0100, Lukasz Luba wrote:

quoted

This patch adds support irq mode in trip point.
When that flag is set in DT, there is no need for polling
in thermal framework. Crossing the trip point will rise an IRQ.
The naming convention for tip point 'type' can be confussing
and 'passive' (whic is passive cooling) might be interpretted
wrongly.

This mechanism prevents from missue and adds explicit setting
for hardware which support interrupts for pre-configured temperature
threshold.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <redacted>
Signed-off-by: Lukasz Luba <redacted>
---
  drivers/thermal/of-thermal.c   | 17 +++++++++++++++++
  drivers/thermal/thermal_core.c | 10 ++++++++--
  include/linux/thermal.h        |  5 +++++
  3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-

thermal.c
index 4bfdb4a..1a75946a 100644

--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c

@@ -312,6 +312,20 @@ static int of_thermal_get_trip_type(struct

thermal_zone_device *tz, int trip,
      return 0;
  }
+static int
+of_thermal_get_trip_irq_mode(struct thermal_zone_device *tz, int
trip,
+                 bool *mode)
+{
+    struct __thermal_zone *data = tz->devdata;
+
+    if (trip >= data->ntrips || trip < 0)
+        return -EDOM;
+
+    *mode = data->trips[trip].irq_mode;
+
+    return 0;
+}
+
  static int of_thermal_get_trip_temp(struct thermal_zone_device *tz,
int trip,
                      int *temp)
  {

@@ -394,6 +408,7 @@ static struct thermal_zone_device_ops

of_thermal_ops = {
      .set_mode = of_thermal_set_mode,
      .get_trip_type = of_thermal_get_trip_type,
+    .get_trip_irq_mode = of_thermal_get_trip_irq_mode,
      .get_trip_temp = of_thermal_get_trip_temp,
      .set_trip_temp = of_thermal_set_trip_temp,
      .get_trip_hyst = of_thermal_get_trip_hyst,

@@ -827,6 +842,8 @@ static int thermal_of_populate_trip(struct

device_node *np,
          return ret;
      }
+    trip->irq_mode = of_property_read_bool(np, "irq-mode");
+
      /* Required for cooling map matching */
      trip->np = np;
      of_node_get(np);

diff --git a/drivers/thermal/thermal_core.c

b/drivers/thermal/thermal_core.c
index 39fc812..6d41e08 100644

--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c

@@ -406,6 +406,7 @@ 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;
+    bool irq_mode = false;
      /* Ignore disabled trip points */
      if (test_bit(trip, &tz->trips_disabled))

@@ -419,9 +420,14 @@ static void handle_thermal_trip(struct

thermal_zone_device *tz, int trip)
          handle_non_critical_trips(tz, trip);
      /*
       * Alright, we handled this trip successfully.
-     * So, start monitoring again.
+     * So, start monitoring in polling mode if
+     * trip is not using irq HW support.
       */
-    monitor_thermal_zone(tz);
+    if (tz->ops->get_trip_irq_mode)
+        tz->ops->get_trip_irq_mode(tz, trip, &irq_mode);
+
+    if (!irq_mode)
+        monitor_thermal_zone(tz);
  }

handle_thermal_trip() is called from thermal_zone_device_update(), and
it is invoked for every trip points.
say, you have a passive trip point 1 that supports irq_mode, and
another passive trip point 2 that does not support irq_mode,
monitor_thermal_zone() is still called in handle_thermal_trip(tz, 2),
and the passive timer will be activated anyway, do I miss something?

Yes, the passive timer will be activated in your example. It is correct
behavior and does not break anything.
case 1: there is 'k' passive trip points which have irq_mode and 1
additional which does not have 'irq_mode', the framework will start
polling but skip check for those 'k' trip points.
case 2: all of the passive trip points have irq_mode set, the framework
will not register 'scheduled_work' because it will not call 
'monitor_thermal_zone()'.
This is the case of most Exynos platforms, but there is one exception 
which has 'case 1' with 2 trip points not supporting irq_mode.

Do you suggest to cover the 'case 1'?
It would be possible after adding a new enum THERMAL_FRAMEWORK_POLL, 
then function:
thermal_zone_device_check() will call
thermal_zone_device_update(tz, THERMAL_FRAMEWORK_POLL)
and in handle_thermal_trip() implement something like:
--------------8<----------------
static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
{
	enum thermal_trip_type type;
	bool irq_mode = false;

	/* Ignore disabled trip points */
	if (test_bit(trip, &tz->trips_disabled))
		return;

	if (tz->ops->get_trip_irq_mode)
		tz->ops->get_trip_irq_mode(tz, trip, &irq_mode)

	if (tz->notify_event == THERMAL_FRAMEWORK_POLL && irq_mode)
		return;
...

	if (!irq_mode)
		monitor_thermal_zone(tz);
}

---------->8-----------------------

I could implement it in v3 if you don't see that it add too much of mess 
and agree for this approach.

Regards,
Lukasz

Regards,
Lukasz

quoted

thanks,
rui

quoted

  static void update_temperature(struct thermal_zone_device *tz)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5f4705f..b064565 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h

@@ -103,6 +103,7 @@ struct thermal_zone_device_ops {

          enum thermal_device_mode);
      int (*get_trip_type) (struct thermal_zone_device *, int,
          enum thermal_trip_type *);
+    int (*get_trip_irq_mode) (struct thermal_zone_device *, int,
bool *);
      int (*get_trip_temp) (struct thermal_zone_device *, int, int
*);
      int (*set_trip_temp) (struct thermal_zone_device *, int,
int);
      int (*get_trip_hyst) (struct thermal_zone_device *, int, int
*);

@@ -196,6 +197,7 @@ struct thermal_zone_device {

      struct thermal_attr *trip_temp_attrs;
      struct thermal_attr *trip_type_attrs;
      struct thermal_attr *trip_hyst_attrs;
+    struct thermal_attr *trip_irq_mode_attrs;
      void *devdata;
      int trips;
      unsigned long trips_disabled;    /* bitmap for disabled
trips */

@@ -364,6 +366,8 @@ struct thermal_zone_of_device_ops {

   * @temperature: temperature value in miliCelsius
   * @hysteresis: relative hysteresis in miliCelsius
   * @type: trip point type
+ * @irq_mode: to not use polling in framework set support of HW irq
(which will
+ *          be triggered when temperature reaches this level).
   */
  struct thermal_trip {

@@ -371,6 +375,7 @@ struct thermal_trip {

      int temperature;
      int hysteresis;
      enum thermal_trip_type type;
+    bool irq_mode;
  };
  /* Function declarations */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel