[PATCH v3 0/5] Thermal: Support for hardware-tracked trip points

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 0/5] Thermal: Support for hardware-tracked trip points

Caesar Wang-2
The history patches come from Mikko and Sascha.
    http://thread.gmane.org/gmane.linux.power-management.general/59451

Now, I pick them up to continue upstream.
Nevermind!

This series history patches:
v2: https://lkml.org/lkml/2016/5/3/220
v1: https://lkml.org/lkml/2016/4/24/227

This series adds support for hardware trip points. It picks up earlier
work from Mikko Perttunen. Mikko implemented hardware trip points as part
of the device tree support. It was suggested back then to move the
functionality to the thermal core instead of putting more code into the
device tree support. This series does exactly that.

This series patches rebase the conflicts.
Note that the hardware-tracked trip points are very well tested currently.

Verified and tested on https://github.com/Caesar-github/rockchip/tree/wip/fixes-thermal-0525
That's based on linux-kernel 20160524.


Changes in v3:
- as Javi comments on https://patchwork.kernel.org/patch/9001281/.
- add the lock for preventing the called from multi placce
- add the note for pre_low/high_trip.
- as the Javi comments on https://patchwork.kernel.org/patch/9001311/.
- add the select if they set the option for devicetree.
- Add the peter's ACK.

Changes in v2:
- update the sysfs-api.txt for set_trips.
- add the commit in patch[v2 2/5].
- Update the commit for patch[v2 4/5].

Caesar Wang (1):
  thermal: rockchip: add the set_trips function

Sascha Hauer (4):
  thermal: Add support for hardware-tracked trip points
  thermal: of: implement .set_trips for device tree thermal zones
  thermal: streamline get_trend callbacks
  thermal: bang-bang governor: act on lower trip boundary

 Documentation/thermal/sysfs-api.txt                |  7 +++
 drivers/thermal/gov_bang_bang.c                    |  2 +-
 drivers/thermal/of-thermal.c                       | 30 ++++++++-----
 drivers/thermal/rockchip_thermal.c                 | 39 ++++++++++++++++
 drivers/thermal/thermal_core.c                     | 52 ++++++++++++++++++++++
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 ++++-------
 include/linux/thermal.h                            | 13 +++++-
 7 files changed, 139 insertions(+), 29 deletions(-)

--
1.9.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 3/5] thermal: streamline get_trend callbacks

Caesar Wang-2
From: Sascha Hauer <[hidden email]>

The .get_trend callback in struct thermal_zone_device_ops has
the prototype:
        int (*get_trend) (struct thermal_zone_device *, int,
                          enum thermal_trend *);
whereas the .get_trend callback in struct thermal_zone_of_device_ops
has:
        int (*get_trend)(void *, long *);

Streamline both prototypes and add the trip argument to the OF callback
aswell and use enum thermal_trend * instead of an integer pointer.

While the OF prototype may be the better one, this should be decided at
framework level and not on OF level.

Signed-off-by: Sascha Hauer <[hidden email]>
Signed-off-by: Caesar Wang <[hidden email]>
Cc: Zhang Rui <[hidden email]>
Cc: Eduardo Valentin <[hidden email]>
Cc: [hidden email]
---

Changes in v3: None
Changes in v2: None

 drivers/thermal/of-thermal.c                       | 11 +---------
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++---------------
 include/linux/thermal.h                            |  2 +-
 3 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 2d2a06f..ba72c35 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -202,24 +202,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
  enum thermal_trend *trend)
 {
  struct __thermal_zone *data = tz->devdata;
- long dev_trend;
  int r;
 
  if (!data->ops->get_trend)
  return -EINVAL;
 
- r = data->ops->get_trend(data->sensor_data, &dev_trend);
+ r = data->ops->get_trend(data->sensor_data, trip, trend);
  if (r)
  return r;
 
- /* TODO: These intervals might have some thresholds, but in core code */
- if (dev_trend > 0)
- *trend = THERMAL_TREND_RAISING;
- else if (dev_trend < 0)
- *trend = THERMAL_TREND_DROPPING;
- else
- *trend = THERMAL_TREND_STABLE;
-
  return 0;
 }
 
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 15c0a9a..4a6757c 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -239,7 +239,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal,
  return 0;
 }
 
-static int __ti_thermal_get_trend(void *p, long *trend)
+static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend)
 {
  struct ti_thermal_data *data = p;
  struct ti_bandgap *bgp;
@@ -252,22 +252,6 @@ static int __ti_thermal_get_trend(void *p, long *trend)
  if (ret)
  return ret;
 
- *trend = tr;
-
- return 0;
-}
-
-/* Get the temperature trend callback functions for thermal zone */
-static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
- int trip, enum thermal_trend *trend)
-{
- int ret;
- long tr;
-
- ret = __ti_thermal_get_trend(thermal->devdata, &tr);
- if (ret)
- return ret;
-
  if (tr > 0)
  *trend = THERMAL_TREND_RAISING;
  else if (tr < 0)
@@ -278,6 +262,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
  return 0;
 }
 
+/* Get the temperature trend callback functions for thermal zone */
+static int ti_thermal_get_trend(struct thermal_zone_device *thermal,
+ int trip, enum thermal_trend *trend)
+{
+ return __ti_thermal_get_trend(thermal->devdata, trip, trend);
+}
+
 /* Get critical temperature callback functions for thermal zone */
 static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal,
     int *temp)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index f68da05..669093c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -348,7 +348,7 @@ struct thermal_genl_event {
  */
 struct thermal_zone_of_device_ops {
  int (*get_temp)(void *, int *);
- int (*get_trend)(void *, long *);
+ int (*get_trend)(void *, int, enum thermal_trend *);
  int (*set_trips)(void *, int, int);
  int (*set_emul_temp)(void *, int);
  int (*set_trip_temp)(void *, int, int);
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 5/5] thermal: rockchip: add the set_trips function

Caesar Wang-2
In reply to this post by Caesar Wang-2
Whenever the current temperature is updated, the trip points immediately
below and above the current temperature are found. A sensor driver
callback `set_trips' is then called with the temperatures.
Lastly, The sensor will trigger the hardware high temperature interrupts
to increase the sampleing rate and throttle frequency to limit the
temperature rising When performing passive cooling.

Signed-off-by: Caesar Wang <[hidden email]>
Cc: Zhang Rui <[hidden email]>
Cc: Eduardo Valentin <[hidden email]>
Cc: [hidden email]

---

Changes in v3: None
Changes in v2: None

 drivers/thermal/rockchip_thermal.c | 39 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 86a1ab0..39e2eec 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -96,6 +96,7 @@ struct chip_tsadc_table {
  * @initialize: SoC special initialize tsadc controller method
  * @irq_ack: clear the interrupt
  * @get_temp: get the temperature
+ * @set_alarm_temp: set the high temperature interrupt
  * @set_tshut_temp: set the hardware-controlled shutdown temperature
  * @set_tshut_mode: set the hardware-controlled shutdown mode
  * @table: the chip-specific conversion table
@@ -119,6 +120,8 @@ struct rockchip_tsadc_chip {
  /* Per-sensor methods */
  int (*get_temp)(struct chip_tsadc_table table,
  int chn, void __iomem *reg, int *temp);
+ void (*set_alarm_temp)(struct chip_tsadc_table table,
+       int chn, void __iomem *reg, int temp);
  void (*set_tshut_temp)(struct chip_tsadc_table table,
        int chn, void __iomem *reg, int temp);
  void (*set_tshut_mode)(int chn, void __iomem *reg, enum tshut_mode m);
@@ -183,6 +186,7 @@ struct rockchip_thermal_data {
 #define TSADCV2_INT_EN 0x08
 #define TSADCV2_INT_PD 0x0c
 #define TSADCV2_DATA(chn) (0x20 + (chn) * 0x04)
+#define TSADCV2_COMP_INT(chn)        (0x30 + (chn) * 0x04)
 #define TSADCV2_COMP_SHUT(chn)        (0x40 + (chn) * 0x04)
 #define TSADCV2_HIGHT_INT_DEBOUNCE 0x60
 #define TSADCV2_HIGHT_TSHUT_DEBOUNCE 0x64
@@ -628,6 +632,20 @@ static int rk_tsadcv2_get_temp(struct chip_tsadc_table table,
  return rk_tsadcv2_code_to_temp(table, val, temp);
 }
 
+static void rk_tsadcv2_alarm_temp(struct chip_tsadc_table table,
+  int chn, void __iomem *regs, int temp)
+{
+ u32 alarm_value, int_en;
+
+ alarm_value = rk_tsadcv2_temp_to_code(table, temp);
+ writel_relaxed(alarm_value & table.data_mask,
+       regs + TSADCV2_COMP_INT(chn));
+
+ int_en = readl_relaxed(regs + TSADCV2_INT_EN);
+ int_en |= TSADCV2_INT_SRC_EN(chn);
+ writel_relaxed(int_en, regs + TSADCV2_INT_EN);
+}
+
 static void rk_tsadcv2_tshut_temp(struct chip_tsadc_table table,
   int chn, void __iomem *regs, int temp)
 {
@@ -670,6 +688,7 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
  .irq_ack = rk_tsadcv3_irq_ack,
  .control = rk_tsadcv3_control,
  .get_temp = rk_tsadcv2_get_temp,
+ .set_alarm_temp = rk_tsadcv2_alarm_temp,
  .set_tshut_temp = rk_tsadcv2_tshut_temp,
  .set_tshut_mode = rk_tsadcv2_tshut_mode,
 
@@ -694,6 +713,7 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
  .irq_ack = rk_tsadcv2_irq_ack,
  .control = rk_tsadcv2_control,
  .get_temp = rk_tsadcv2_get_temp,
+ .set_alarm_temp = rk_tsadcv2_alarm_temp,
  .set_tshut_temp = rk_tsadcv2_tshut_temp,
  .set_tshut_mode = rk_tsadcv2_tshut_mode,
 
@@ -718,6 +738,7 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
  .irq_ack = rk_tsadcv3_irq_ack,
  .control = rk_tsadcv3_control,
  .get_temp = rk_tsadcv2_get_temp,
+ .set_alarm_temp = rk_tsadcv2_alarm_temp,
  .set_tshut_temp = rk_tsadcv2_tshut_temp,
  .set_tshut_mode = rk_tsadcv2_tshut_mode,
 
@@ -742,6 +763,7 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
  .irq_ack = rk_tsadcv2_irq_ack,
  .control = rk_tsadcv2_control,
  .get_temp = rk_tsadcv2_get_temp,
+ .set_alarm_temp = rk_tsadcv2_alarm_temp,
  .set_tshut_temp = rk_tsadcv2_tshut_temp,
  .set_tshut_mode = rk_tsadcv2_tshut_mode,
 
@@ -766,6 +788,7 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
  .irq_ack = rk_tsadcv3_irq_ack,
  .control = rk_tsadcv3_control,
  .get_temp = rk_tsadcv2_get_temp,
+ .set_alarm_temp = rk_tsadcv2_alarm_temp,
  .set_tshut_temp = rk_tsadcv2_tshut_temp,
  .set_tshut_mode = rk_tsadcv2_tshut_mode,
 
@@ -826,6 +849,21 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
  return IRQ_HANDLED;
 }
 
+static int rockchip_thermal_set_trips(void *_sensor, int low, int high)
+{
+ struct rockchip_thermal_sensor *sensor = _sensor;
+ struct rockchip_thermal_data *thermal = sensor->thermal;
+ const struct rockchip_tsadc_chip *tsadc = thermal->chip;
+
+ dev_dbg(&thermal->pdev->dev, "%s: sensor %d: low: %d, high %d\n",
+ __func__, sensor->id, low, high);
+
+ tsadc->set_alarm_temp(tsadc->table,
+      sensor->id, thermal->regs, high);
+
+ return 0;
+}
+
 static int rockchip_thermal_get_temp(void *_sensor, int *out_temp)
 {
  struct rockchip_thermal_sensor *sensor = _sensor;
@@ -843,6 +881,7 @@ static int rockchip_thermal_get_temp(void *_sensor, int *out_temp)
 
 static const struct thermal_zone_of_device_ops rockchip_of_thermal_ops = {
  .get_temp = rockchip_thermal_get_temp,
+ .set_trips = rockchip_thermal_set_trips,
 };
 
 static int rockchip_configure_from_dt(struct device *dev,
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 2/5] thermal: of: implement .set_trips for device tree thermal zones

Caesar Wang-2
In reply to this post by Caesar Wang-2
From: Sascha Hauer <[hidden email]>

This patch implemnets .set_trips for device tree thermal zones.
As the hardware-tracked trip points is supported by thermal core patch[0].

patch[0]
"thermal: Add support for hardware-tracked trip points".

Signed-off-by: Sascha Hauer <[hidden email]>
Signed-off-by: Caesar Wang <[hidden email]>
Cc: Zhang Rui <[hidden email]>
Cc: Eduardo Valentin <[hidden email]>

---

Changes in v3:
- as the Javi comments on https://patchwork.kernel.org/patch/9001311/.
- add the select if they set the option for devicetree.

Changes in v2:
- add the commit in patch[v2 2/5].

 drivers/thermal/of-thermal.c | 19 +++++++++++++++++++
 include/linux/thermal.h      |  4 ++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index b8e509c..2d2a06f 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -101,6 +101,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz,
  return data->ops->get_temp(data->sensor_data, temp);
 }
 
+static int of_thermal_set_trips(struct thermal_zone_device *tz,
+ int low, int high)
+{
+ struct __thermal_zone *data = tz->devdata;
+
+ if (!data->ops || !data->ops->set_trips)
+ return -EINVAL;
+
+ return data->ops->set_trips(data->sensor_data, low, high);
+}
+
 /**
  * of_thermal_get_ntrips - function to export number of available trip
  *   points.
@@ -427,6 +438,14 @@ thermal_zone_of_add_sensor(struct device_node *zone,
 
  tzd->ops->get_temp = of_thermal_get_temp;
  tzd->ops->get_trend = of_thermal_get_trend;
+
+ /*
+ * The thermal zone core will calculate the window if they have set the
+ * optional set_trips pointer.
+ */
+ if (ops->set_trips)
+ tzd->ops->set_trips = of_thermal_set_trips;
+
  tzd->ops->set_emul_temp = of_thermal_set_emul_temp;
  mutex_unlock(&tzd->lock);
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 635c4003..f68da05 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -340,12 +340,16 @@ struct thermal_genl_event {
  *
  * Optional:
  * @get_trend: a pointer to a function that reads the sensor temperature trend.
+ * @set_trips: a pointer to a function that sets a temperature window. When
+ *       this window is left the driver must inform the thermal core via
+ *       thermal_zone_device_update.
  * @set_emul_temp: a pointer to a function that sets sensor emulated
  *   temperature.
  */
 struct thermal_zone_of_device_ops {
  int (*get_temp)(void *, int *);
  int (*get_trend)(void *, long *);
+ int (*set_trips)(void *, int, int);
  int (*set_emul_temp)(void *, int);
  int (*set_trip_temp)(void *, int, int);
 };
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 4/5] thermal: bang-bang governor: act on lower trip boundary

Caesar Wang-2
In reply to this post by Caesar Wang-2
From: Sascha Hauer <[hidden email]>

With interrupt driven thermal zones we pass the lower and upper
temperature on which shall be acted, so in the governor we have to act on
the exact lower temperature to be consistent. Otherwise an interrupt maybe
generated on the exact lower temperature, but the bang bang governor does
not react since The polling driven zones have to be one step cooler before
the governor reacts.

Signed-off-by: Sascha Hauer <[hidden email]>
Signed-off-by: Caesar Wang <[hidden email]>
Cc: Zhang Rui <[hidden email]>
Cc: Eduardo Valentin <[hidden email]>
Cc: [hidden email]
Acked-by: Peter Feuerer <[hidden email]>

---

Changes in v3:
- Add the peter's ACK.

Changes in v2:
- Update the commit for patch[v2 4/5].

 drivers/thermal/gov_bang_bang.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c
index 70836c5..9d1dfea 100644
--- a/drivers/thermal/gov_bang_bang.c
+++ b/drivers/thermal/gov_bang_bang.c
@@ -59,7 +59,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
  if (instance->target == 0 && tz->temperature >= trip_temp)
  instance->target = 1;
  else if (instance->target == 1 &&
- tz->temperature < trip_temp - trip_hyst)
+ tz->temperature <= trip_temp - trip_hyst)
  instance->target = 0;
 
  dev_dbg(&instance->cdev->device, "target=%d\n",
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 1/5] thermal: Add support for hardware-tracked trip points

Caesar Wang-2
In reply to this post by Caesar Wang-2
From: Sascha Hauer <[hidden email]>

This adds support for hardware-tracked trip points to the device tree
thermal sensor framework.

The framework supports an arbitrary number of trip points. Whenever
the current temperature is updated, the trip points immediately
below and above the current temperature are found. A .set_trips
callback is then called with the temperatures. If there is no trip
point above or below the current temperature, the passed trip
temperature will be -INT_MAX or INT_MAX respectively. In this callback,
the driver should program the hardware such that it is notified
when either of these trip points are triggered. When a trip point
is triggered, the driver should call `thermal_zone_device_update'
for the respective thermal zone. This will cause the trip points
to be updated again.

If .set_trips is not implemented, the framework behaves as before.

This patch is based on an earlier version from Mikko Perttunen
<[hidden email]>

Signed-off-by: Sascha Hauer <[hidden email]>
Signed-off-by: Caesar Wang <[hidden email]>
Cc: Zhang Rui <[hidden email]>
Cc: Eduardo Valentin <[hidden email]>
Cc: [hidden email]

---

Changes in v3:
- as Javi comments on https://patchwork.kernel.org/patch/9001281/.
- add the lock for preventing the called from multi placce
- add the note for pre_low/high_trip.

Changes in v2:
- update the sysfs-api.txt for set_trips.

 Documentation/thermal/sysfs-api.txt |  7 +++++
 drivers/thermal/thermal_core.c      | 52 +++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h             |  7 +++++
 3 files changed, 66 insertions(+)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index efc3f3d..75d8838 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -49,6 +49,9 @@ temperature) and throttle appropriate devices.
  .bind: bind the thermal zone device with a thermal cooling device.
  .unbind: unbind the thermal zone device with a thermal cooling device.
  .get_temp: get the current temperature of the thermal zone.
+ .set_trips: set the trip points window. Whenever the current temperature
+    is updated, the trip points immediately below and above the
+    current temperature are found.
  .get_mode: get the current mode (enabled/disabled) of the thermal zone.
     - "enabled" means the kernel thermal management is enabled.
     - "disabled" will prevent kernel thermal driver action upon trip points
@@ -95,6 +98,10 @@ temperature) and throttle appropriate devices.
  get_temp: a pointer to a function that reads the
  sensor temperature. This is mandatory
  callback provided by sensor driver.
+ set_trips:      a pointer to a function that sets a
+ temperature window. When this window is
+ left the driver must inform the thermal
+ core via thermal_zone_device_update.
  get_trend: a pointer to a function that reads the
  sensor temperature trend.
  set_emul_temp: a pointer to a function that sets
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 5133cd1..e5bfbd3 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -520,6 +520,51 @@ exit:
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 
+static void thermal_zone_set_trips(struct thermal_zone_device *tz)
+{
+ int low = -INT_MAX;
+ int high = INT_MAX;
+ int trip_temp, hysteresis;
+ int temp = tz->temperature;
+ int i, ret;
+
+ if (!tz->ops->set_trips)
+ return;
+
+ for (i = 0; i < tz->trips; i++) {
+ int trip_low;
+
+ tz->ops->get_trip_temp(tz, i, &trip_temp);
+ tz->ops->get_trip_hyst(tz, i, &hysteresis);
+
+ trip_low = trip_temp - hysteresis;
+
+ if (trip_low < temp && trip_low > low)
+ low = trip_low;
+
+ if (trip_temp > temp && trip_temp < high)
+ high = trip_temp;
+ }
+
+ /* No need to change trip points */
+ if (tz->prev_low_trip == low && tz->prev_high_trip == high)
+ return;
+
+ tz->prev_low_trip = low;
+ tz->prev_high_trip = high;
+
+ dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
+ low, high);
+
+ /*
+ * Set a temperature window. When this window is left the driver
+ * must inform the thermal core via thermal_zone_device_update.
+ */
+ ret = tz->ops->set_trips(tz, low, high);
+ if (ret)
+ dev_err(&tz->device, "Failed to set trips: %d\n", ret);
+}
+
 static void update_temperature(struct thermal_zone_device *tz)
 {
  int temp, ret;
@@ -569,6 +614,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 
  update_temperature(tz);
 
+ thermal_zone_set_trips(tz);
+
  for (count = 0; count < tz->trips; count++)
  handle_thermal_trip(tz, count);
 }
@@ -754,6 +801,9 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr,
  */
  ret = tz->ops->set_trip_hyst(tz, trip, temperature);
 
+ if (!ret)
+ thermal_zone_set_trips(tz);
+
  return ret ? ret : count;
 }
 
@@ -1843,6 +1893,8 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
  tz->trips = trips;
  tz->passive_delay = passive_delay;
  tz->polling_delay = polling_delay;
+ tz->prev_low_trip = INT_MAX;
+ tz->prev_high_trip = -INT_MAX;
  /* A new thermal zone needs to be updated anyway. */
  atomic_set(&tz->need_update, 1);
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index e45abe7..635c4003 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -98,6 +98,7 @@ struct thermal_zone_device_ops {
  int (*unbind) (struct thermal_zone_device *,
        struct thermal_cooling_device *);
  int (*get_temp) (struct thermal_zone_device *, int *);
+ int (*set_trips) (struct thermal_zone_device *, int, int);
  int (*get_mode) (struct thermal_zone_device *,
  enum thermal_device_mode *);
  int (*set_mode) (struct thermal_zone_device *,
@@ -168,6 +169,10 @@ struct thermal_attr {
  * @last_temperature: previous temperature read
  * @emul_temperature: emulated temperature when using CONFIG_THERMAL_EMULATION
  * @passive: 1 if you've crossed a passive trip point, 0 otherwise.
+ * @prev_low_trip: the low current temperature if you've crossed a passive
+ trip point.
+ * @prev_high_trip: the above current temperature if you've crossed a
+ passive trip point.
  * @forced_passive: If > 0, temperature at which to switch on all ACPI
  * processor cooling devices.  Currently only used by the
  * step-wise governor.
@@ -199,6 +204,8 @@ struct thermal_zone_device {
  int last_temperature;
  int emul_temperature;
  int passive;
+ int prev_low_trip;
+ int prev_high_trip;
  unsigned int forced_passive;
  atomic_t need_update;
  struct thermal_zone_device_ops *ops;
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 1/5] thermal: Add support for hardware-tracked trip points

Javi Merino
Hi Caesar,

On Wed, May 25, 2016 at 11:47:45AM +0800, Caesar Wang wrote:

> From: Sascha Hauer <[hidden email]>
>
> This adds support for hardware-tracked trip points to the device tree
> thermal sensor framework.
>
> The framework supports an arbitrary number of trip points. Whenever
> the current temperature is updated, the trip points immediately
> below and above the current temperature are found. A .set_trips
> callback is then called with the temperatures. If there is no trip
> point above or below the current temperature, the passed trip
> temperature will be -INT_MAX or INT_MAX respectively. In this callback,
> the driver should program the hardware such that it is notified
> when either of these trip points are triggered. When a trip point
> is triggered, the driver should call `thermal_zone_device_update'
> for the respective thermal zone. This will cause the trip points
> to be updated again.
>
> If .set_trips is not implemented, the framework behaves as before.
>
> This patch is based on an earlier version from Mikko Perttunen
> <[hidden email]>
>
> Signed-off-by: Sascha Hauer <[hidden email]>
> Signed-off-by: Caesar Wang <[hidden email]>
> Cc: Zhang Rui <[hidden email]>
> Cc: Eduardo Valentin <[hidden email]>
> Cc: [hidden email]
>
> ---
>
> Changes in v3:
> - as Javi comments on https://patchwork.kernel.org/patch/9001281/.
> - add the lock for preventing the called from multi placce
> - add the note for pre_low/high_trip.
>
> Changes in v2:
> - update the sysfs-api.txt for set_trips.
>
>  Documentation/thermal/sysfs-api.txt |  7 +++++
>  drivers/thermal/thermal_core.c      | 52 +++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h             |  7 +++++
>  3 files changed, 66 insertions(+)
>
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index efc3f3d..75d8838 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -49,6 +49,9 @@ temperature) and throttle appropriate devices.
>   .bind: bind the thermal zone device with a thermal cooling device.
>   .unbind: unbind the thermal zone device with a thermal cooling device.
>   .get_temp: get the current temperature of the thermal zone.
> + .set_trips: set the trip points window. Whenever the current temperature
> +    is updated, the trip points immediately below and above the
> +    current temperature are found.
>   .get_mode: get the current mode (enabled/disabled) of the thermal zone.
>      - "enabled" means the kernel thermal management is enabled.
>      - "disabled" will prevent kernel thermal driver action upon trip points
> @@ -95,6 +98,10 @@ temperature) and throttle appropriate devices.
>   get_temp: a pointer to a function that reads the
>   sensor temperature. This is mandatory
>   callback provided by sensor driver.
> + set_trips:      a pointer to a function that sets a
> + temperature window. When this window is
> + left the driver must inform the thermal
> + core via thermal_zone_device_update.
>   get_trend: a pointer to a function that reads the
>   sensor temperature trend.
>   set_emul_temp: a pointer to a function that sets
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 5133cd1..e5bfbd3 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -520,6 +520,51 @@ exit:
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
>  
> +static void thermal_zone_set_trips(struct thermal_zone_device *tz)
> +{
> + int low = -INT_MAX;
> + int high = INT_MAX;
> + int trip_temp, hysteresis;
> + int temp = tz->temperature;
> + int i, ret;
> +
> + if (!tz->ops->set_trips)
> + return;
> +
> + for (i = 0; i < tz->trips; i++) {
> + int trip_low;
> +
> + tz->ops->get_trip_temp(tz, i, &trip_temp);
> + tz->ops->get_trip_hyst(tz, i, &hysteresis);
> +
> + trip_low = trip_temp - hysteresis;
> +
> + if (trip_low < temp && trip_low > low)
> + low = trip_low;
> +
> + if (trip_temp > temp && trip_temp < high)
> + high = trip_temp;
> + }
> +
> + /* No need to change trip points */
> + if (tz->prev_low_trip == low && tz->prev_high_trip == high)
> + return;
> +
> + tz->prev_low_trip = low;
> + tz->prev_high_trip = high;
> +
> + dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
> + low, high);
> +
> + /*
> + * Set a temperature window. When this window is left the driver
> + * must inform the thermal core via thermal_zone_device_update.
> + */
> + ret = tz->ops->set_trips(tz, low, high);
> + if (ret)
> + dev_err(&tz->device, "Failed to set trips: %d\n", ret);

In the v2 review
(http://article.gmane.org/gmane.linux.documentation/38442) you agreed
to add locking but you haven't done it.  Maybe you sent the wrong
version?

Cheers,
Javi
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 2/5] thermal: of: implement .set_trips for device tree thermal zones

Javi Merino
In reply to this post by Caesar Wang-2
Hi Caesar,

On Wed, May 25, 2016 at 11:47:46AM +0800, Caesar Wang wrote:
> From: Sascha Hauer <[hidden email]>
>
> This patch implemnets .set_trips for device tree thermal zones.
             ^
             implements

> As the hardware-tracked trip points is supported by thermal core patch[0].
>
> patch[0]
> "thermal: Add support for hardware-tracked trip points".
>
> Signed-off-by: Sascha Hauer <[hidden email]>
> Signed-off-by: Caesar Wang <[hidden email]>
> Cc: Zhang Rui <[hidden email]>
> Cc: Eduardo Valentin <[hidden email]>

Other than that, it looks good to me.  You can add my

Reviewed-by: Javi Merino <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 1/5] thermal: Add support for hardware-tracked trip points

Caesar Wang-2
In reply to this post by Javi Merino

On 2016年05月26日 00:42, Javi Merino wrote:

> Hi Caesar,
>
> On Wed, May 25, 2016 at 11:47:45AM +0800, Caesar Wang wrote:
>> From: Sascha Hauer <[hidden email]>
>>
>> This adds support for hardware-tracked trip points to the device tree
>> thermal sensor framework.
>>
>> The framework supports an arbitrary number of trip points. Whenever
>> the current temperature is updated, the trip points immediately
>> below and above the current temperature are found. A .set_trips
>> callback is then called with the temperatures. If there is no trip
>> point above or below the current temperature, the passed trip
>> temperature will be -INT_MAX or INT_MAX respectively. In this callback,
>> the driver should program the hardware such that it is notified
>> when either of these trip points are triggered. When a trip point
>> is triggered, the driver should call `thermal_zone_device_update'
>> for the respective thermal zone. This will cause the trip points
>> to be updated again.
>>
>> If .set_trips is not implemented, the framework behaves as before.
>>
>> This patch is based on an earlier version from Mikko Perttunen
>> <[hidden email]>
>>
>> Signed-off-by: Sascha Hauer <[hidden email]>
>> Signed-off-by: Caesar Wang <[hidden email]>
>> Cc: Zhang Rui <[hidden email]>
>> Cc: Eduardo Valentin <[hidden email]>
>> Cc: [hidden email]
>>
>> ---
>>
>> Changes in v3:
>> - as Javi comments on https://patchwork.kernel.org/patch/9001281/.
>> - add the lock for preventing the called from multi placce
>> - add the note for pre_low/high_trip.
>>
>> Changes in v2:
>> - update the sysfs-api.txt for set_trips.
>>
>>   Documentation/thermal/sysfs-api.txt |  7 +++++
>>   drivers/thermal/thermal_core.c      | 52 +++++++++++++++++++++++++++++++++++++
>>   include/linux/thermal.h             |  7 +++++
>>   3 files changed, 66 insertions(+)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> index efc3f3d..75d8838 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -49,6 +49,9 @@ temperature) and throttle appropriate devices.
>>   .bind: bind the thermal zone device with a thermal cooling device.
>>   .unbind: unbind the thermal zone device with a thermal cooling device.
>>   .get_temp: get the current temperature of the thermal zone.
>> + .set_trips: set the trip points window. Whenever the current temperature
>> +    is updated, the trip points immediately below and above the
>> +    current temperature are found.
>>   .get_mode: get the current mode (enabled/disabled) of the thermal zone.
>>      - "enabled" means the kernel thermal management is enabled.
>>      - "disabled" will prevent kernel thermal driver action upon trip points
>> @@ -95,6 +98,10 @@ temperature) and throttle appropriate devices.
>>   get_temp: a pointer to a function that reads the
>>   sensor temperature. This is mandatory
>>   callback provided by sensor driver.
>> + set_trips:      a pointer to a function that sets a
>> + temperature window. When this window is
>> + left the driver must inform the thermal
>> + core via thermal_zone_device_update.
>>   get_trend: a pointer to a function that reads the
>>   sensor temperature trend.
>>   set_emul_temp: a pointer to a function that sets
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 5133cd1..e5bfbd3 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -520,6 +520,51 @@ exit:
>>   }
>>   EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
>>  
>> +static void thermal_zone_set_trips(struct thermal_zone_device *tz)
>> +{
>> + int low = -INT_MAX;
>> + int high = INT_MAX;
>> + int trip_temp, hysteresis;
>> + int temp = tz->temperature;
>> + int i, ret;
>> +
>> + if (!tz->ops->set_trips)
>> + return;
>> +
>> + for (i = 0; i < tz->trips; i++) {
>> + int trip_low;
>> +
>> + tz->ops->get_trip_temp(tz, i, &trip_temp);
>> + tz->ops->get_trip_hyst(tz, i, &hysteresis);
>> +
>> + trip_low = trip_temp - hysteresis;
>> +
>> + if (trip_low < temp && trip_low > low)
>> + low = trip_low;
>> +
>> + if (trip_temp > temp && trip_temp < high)
>> + high = trip_temp;
>> + }
>> +
>> + /* No need to change trip points */
>> + if (tz->prev_low_trip == low && tz->prev_high_trip == high)
>> + return;
>> +
>> + tz->prev_low_trip = low;
>> + tz->prev_high_trip = high;
>> +
>> + dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
>> + low, high);
>> +
>> + /*
>> + * Set a temperature window. When this window is left the driver
>> + * must inform the thermal core via thermal_zone_device_update.
>> + */
>> + ret = tz->ops->set_trips(tz, low, high);
>> + if (ret)
>> + dev_err(&tz->device, "Failed to set trips: %d\n", ret);
> In the v2 review
> (http://article.gmane.org/gmane.linux.documentation/38442) you agreed
> to add locking but you haven't done it.  Maybe you sent the wrong
> version?

My god!
Yep, I remember to change it. Somehow why ...

Thanks Javi for looking over it.

Hoping the next version is the final version.:-)

-Caesar

>
> Cheers,
> Javi
>
> _______________________________________________
> Linux-rockchip mailing list
> [hidden email]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


--
caesar wang | software engineer | [hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 1/5] thermal: Add support for hardware-tracked trip points

Caesar Wang-2

On 2016年05月26日 08:34, Caesar Wang wrote:

>
> On 2016年05月26日 00:42, Javi Merino wrote:
>> Hi Caesar,
>>
>> On Wed, May 25, 2016 at 11:47:45AM +0800, Caesar Wang wrote:
>>> From: Sascha Hauer <[hidden email]>
>>>
>>> This adds support for hardware-tracked trip points to the device tree
>>> thermal sensor framework.
>>>
>>> The framework supports an arbitrary number of trip points. Whenever
>>> the current temperature is updated, the trip points immediately
>>> below and above the current temperature are found. A .set_trips
>>> callback is then called with the temperatures. If there is no trip
>>> point above or below the current temperature, the passed trip
>>> temperature will be -INT_MAX or INT_MAX respectively. In this callback,
>>> the driver should program the hardware such that it is notified
>>> when either of these trip points are triggered. When a trip point
>>> is triggered, the driver should call `thermal_zone_device_update'
>>> for the respective thermal zone. This will cause the trip points
>>> to be updated again.
>>>
>>> If .set_trips is not implemented, the framework behaves as before.
>>>
>>> This patch is based on an earlier version from Mikko Perttunen
>>> <[hidden email]>
>>>
>>> Signed-off-by: Sascha Hauer <[hidden email]>
>>> Signed-off-by: Caesar Wang <[hidden email]>
>>> Cc: Zhang Rui <[hidden email]>
>>> Cc: Eduardo Valentin <[hidden email]>
>>> Cc: [hidden email]
>>>
>>> ---
>>>
>>> Changes in v3:
>>> - as Javi comments on https://patchwork.kernel.org/patch/9001281/.
>>> - add the lock for preventing the called from multi placce
>>> - add the note for pre_low/high_trip.
>>>
>>> Changes in v2:
>>> - update the sysfs-api.txt for set_trips.
>>>
>>>   Documentation/thermal/sysfs-api.txt |  7 +++++
>>>   drivers/thermal/thermal_core.c      | 52
>>> +++++++++++++++++++++++++++++++++++++
>>>   include/linux/thermal.h             |  7 +++++
>>>   3 files changed, 66 insertions(+)
>>>
>>> diff --git a/Documentation/thermal/sysfs-api.txt
>>> b/Documentation/thermal/sysfs-api.txt
>>> index efc3f3d..75d8838 100644
>>> --- a/Documentation/thermal/sysfs-api.txt
>>> +++ b/Documentation/thermal/sysfs-api.txt
>>> @@ -49,6 +49,9 @@ temperature) and throttle appropriate devices.
>>>       .bind: bind the thermal zone device with a thermal cooling
>>> device.
>>>       .unbind: unbind the thermal zone device with a thermal cooling
>>> device.
>>>       .get_temp: get the current temperature of the thermal zone.
>>> +    .set_trips: set the trip points window. Whenever the current
>>> temperature
>>> +            is updated, the trip points immediately below and above
>>> the
>>> +            current temperature are found.
>>>       .get_mode: get the current mode (enabled/disabled) of the
>>> thermal zone.
>>>           - "enabled" means the kernel thermal management is enabled.
>>>           - "disabled" will prevent kernel thermal driver action
>>> upon trip points
>>> @@ -95,6 +98,10 @@ temperature) and throttle appropriate devices.
>>>               get_temp:    a pointer to a function that reads the
>>>                       sensor temperature. This is mandatory
>>>                       callback provided by sensor driver.
>>> +            set_trips:      a pointer to a function that sets a
>>> +                    temperature window. When this window is
>>> +                    left the driver must inform the thermal
>>> +                    core via thermal_zone_device_update.
>>>               get_trend:     a pointer to a function that reads the
>>>                       sensor temperature trend.
>>>               set_emul_temp:    a pointer to a function that sets
>>> diff --git a/drivers/thermal/thermal_core.c
>>> b/drivers/thermal/thermal_core.c
>>> index 5133cd1..e5bfbd3 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -520,6 +520,51 @@ exit:
>>>   }
>>>   EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
>>>   +static void thermal_zone_set_trips(struct thermal_zone_device *tz)
>>> +{
>>> +    int low = -INT_MAX;
>>> +    int high = INT_MAX;
>>> +    int trip_temp, hysteresis;
>>> +    int temp = tz->temperature;
>>> +    int i, ret;
>>> +
>>> +    if (!tz->ops->set_trips)
>>> +        return;
>>> +
>>> +    for (i = 0; i < tz->trips; i++) {
>>> +        int trip_low;
>>> +
>>> +        tz->ops->get_trip_temp(tz, i, &trip_temp);
>>> +        tz->ops->get_trip_hyst(tz, i, &hysteresis);
>>> +
>>> +        trip_low = trip_temp - hysteresis;
>>> +
>>> +        if (trip_low < temp && trip_low > low)
>>> +            low = trip_low;
>>> +
>>> +        if (trip_temp > temp && trip_temp < high)
>>> +            high = trip_temp;
>>> +    }
>>> +
>>> +    /* No need to change trip points */
>>> +    if (tz->prev_low_trip == low && tz->prev_high_trip == high)
>>> +        return;
>>> +
>>> +    tz->prev_low_trip = low;
>>> +    tz->prev_high_trip = high;
>>> +
>>> +    dev_dbg(&tz->device, "new temperature boundaries: %d < x < %d\n",
>>> +            low, high);
>>> +
>>> +    /*
>>> +     * Set a temperature window. When this window is left the driver
>>> +     * must inform the thermal core via thermal_zone_device_update.
>>> +     */
>>> +    ret = tz->ops->set_trips(tz, low, high);
>>> +    if (ret)
>>> +        dev_err(&tz->device, "Failed to set trips: %d\n", ret);
>> In the v2 review
>> (http://article.gmane.org/gmane.linux.documentation/38442) you agreed
>> to add locking but you haven't done it.  Maybe you sent the wrong
>> version?
>
> My god!
> Yep, I remember to change it. Somehow why ...
>
> Thanks Javi for looking over it.
>
> Hoping the next version is the final version.:-)
>

If add the lock, there is a deadlock for board,  maybe I will keep to
stay with this version.

   2.895038] [ INFO: possible recursive locking detected ]
[    2.900459] 4.4.4 #56 Not tainted
[    2.903774] ---------------------------------------------
[    2.909171] swapper/0/1 is trying to acquire lock:
[    2.913974]  (&tz->lock){+.+.+.}, at: [<ffffffc00069b428>]
handle_thermal_trip+0x230/0x304
[    2.922336]
[    2.922336] but task is already holding lock:
[    2.928166]  (&tz->lock){+.+.+.}, at: [<ffffffc000699194>]
thermal_zone_set_trips+0xd8/0x180
[    2.936670]
[    2.936670] other info that might help us debug this:
[    2.943199]  Possible unsafe locking scenario:
[    2.943199]
[    2.949117]        CPU0
[    2.951565]        ----
[    2.954012]   lock(&tz->lock);
[    2.957098]   lock(&tz->lock);
[    2.960176]
[    2.960176]  *** DEADLOCK ***
[    2.960176]
[    2.966098]  May be due to missing lock nesting notation
[    2.966098]
[    2.972885] 3 locks held by swapper/0/1:
[    2.976809]  #0:  (&dev->mutex){......}, at: [<ffffffc0005920b0>]
__driver_attach+0x40/0xa8
[    2.985243]  #1:  (&dev->mutex){......}, at: [<ffffffc0005920d8>]
__driver_attach+0x68/0xa8
[    2.993671]  #2:  (&tz->lock){+.+.+.}, at: [<ffffffc000699194>]
thermal_zone_set_trips+0xd8/0x180
...







> -Caesar
>
>>
>> Cheers,
>> Javi
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> [hidden email]
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>

--
caesar wang | software engineer | [hidden email]