2018-07-31 19:03:07

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v6 1/5] dt-bindings: thermal: qcom-spmi-temp-alarm: Fix documentation of 'reg'

The documentation claims that the 'reg' property consists of two values,
the SPMI address and the length of the controller's registers. However
the SPMI bus to which it is added specifies "#size-cells = <0>;". Remove
the controller register length from the documentation of the field and the
example.

Signed-off-by: Matthias Kaehlcke <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---
Changes in v6:
- patch added to the series
---
.../devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt b/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
index 290ec06fa33a..86fb41fe772f 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
@@ -6,8 +6,7 @@ interrupt signal and status register to identify high PMIC die temperature.

Required properties:
- compatible: Should contain "qcom,spmi-temp-alarm".
-- reg: Specifies the SPMI address and length of the controller's
- registers.
+- reg: Specifies the SPMI address.
- interrupts: PMIC temperature alarm interrupt.
- #thermal-sensor-cells: Should be 0. See thermal.txt for a description.

@@ -20,7 +19,7 @@ Example:

pm8941_temp: thermal-alarm@2400 {
compatible = "qcom,spmi-temp-alarm";
- reg = <0x2400 0x100>;
+ reg = <0x2400>;
interrupts = <0 0x24 0 IRQ_TYPE_EDGE_RISING>;
#thermal-sensor-cells = <0>;

--
2.18.0.345.g5c9ce644c3-goog



2018-07-31 19:01:15

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v6 5/5] arm64: dts: qcom: pm8998: Add pm8998 thermal zone

The thermal zone uses spmi-temp-alarm as sensor, the trip points
correspond to the PMIC thermal stages 1 and 2. The critical trip
point at 125°C disables the partial PMIC shutdown at stage 2.

Without an IIO input the sensor only reports a limited number of
temperatures:

- 37°C for temperatures below 105°C
- 107°C for temperatures >= 105°C and < 125°C
- 127°C for temperatures >= 125°C

(the numbers correspond to a stage 1 threshold of 105°C)

Signed-off-by: Matthias Kaehlcke <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---
Changes in v6:
- added 'Reviewed-by: Douglas Anderson <[email protected]>' tag

Changes in v5:
- removed 'stage2-shutdown-disabled' property from spmi-temp-alarm
- updated commit message

Changes in v4:
- updated trip point temperatures to match stage 1 and 2 ones
- disabled stage 2 shutdown
- updated commit message

Changes in v3:
- moved 'thermal-zones' node to the beginning of the .dtsi

Changes in v2:
- defined 'thermal-zones' node in pm8998.dtsi instead of using a label
to refer to it
- use 105°C hardware trip point as critical trip point
- reduced number of trip points to 2
- lowered temperature of passive trip point
- updated trip point names and added labels
- updated commit message
---
arch/arm64/boot/dts/qcom/pm8998.dtsi | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi
index 7eea94701b23..34c259ba3919 100644
--- a/arch/arm64/boot/dts/qcom/pm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi
@@ -3,6 +3,31 @@

#include <dt-bindings/spmi/spmi.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/thermal/thermal.h>
+
+/ {
+ thermal-zones {
+ pm8998 {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&pm8998_temp>;
+
+ trips {
+ pm8998_alert0: pm8998-alert0 {
+ temperature = <105000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ pm8998_crit: pm8998-crit {
+ temperature = <125000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+ };
+};

&spmi_bus {
pm8998_lsid0: pmic@0 {
--
2.18.0.345.g5c9ce644c3-goog


2018-07-31 19:01:26

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v6 3/5] thermal: qcom-spmi: Use PMIC thermal stage 2 for critical trip points

There are three thermal stages defined in the PMIC:

stage 1: warning
stage 2: system should shut down
stage 3: emergency shut down

By default the PMIC assumes that the OS isn't doing anything and thus
at stage 2 it does a partial PMIC shutdown and at stage 3 it kills
all power. When switching between thermal stages the PMIC generates an
interrupt which is handled by the driver. The partial PMIC shutdown at
stage 2 can be disabled by software, which allows the OS to initiate a
shutdown at stage 2 with a thermal zone configured accordingly.

If a critical trip point is configured in the thermal zone the driver
adjusts the stage 1-3 temperature thresholds to (closely) match the
critical temperature with a stage 2 threshold (125/130/135/140 °C).
If a suitable match is found the partial shutdown at stage 2 is
disabled. If for some reason the system doesn't shutdown at stage 2
the emergency shutdown at stage 3 kicks in.

The partial shutdown at stage 2 remains enabled in these cases:
- no critical trip point defined
- the temperature of the critical trip point is < 125°C
- the temperature of the critical trip point is > 140°C and no
ADC channel is configured (thus the OS is not notified when the critical
temperature is reached)

Suggested-by: Douglas Anderson <[email protected]>
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
Changes in v6:
- fixed condition to check if ADC is configured in
qpnp_tm_update_critical_trip_temp()
- changed °C in logs to C
- removed needless evaluation of qpnp_tm_write() return value in
qpnp_tm_update_critical_trip_temp()
- move assignment of chip->initialized to true to qpnp_tm_init(),
where the lock is held
- call thermal_zone_device_update() after initialization is
completed
- split some #define + comment in two lines to avoid exceeding
chars per line limit
- removed extra closing parenthesis in qpnp_tm_get_temp()
- remove unnecessary parentheses around conditions in
qpnp_tm_update_critical_trip_temp() and qpnp_tm_get_critical_trip_temp()
- fixed indentation of call devm_thermal_zone_of_sensor_register() call
in qpnp_tm_probe()

Changes in v5:
- patch added to the series
---
drivers/thermal/qcom-spmi-temp-alarm.c | 158 ++++++++++++++++++++++---
1 file changed, 140 insertions(+), 18 deletions(-)

diff --git a/drivers/thermal/qcom-spmi-temp-alarm.c b/drivers/thermal/qcom-spmi-temp-alarm.c
index ad4f3a8d6560..b2d5d5bf4a9b 100644
--- a/drivers/thermal/qcom-spmi-temp-alarm.c
+++ b/drivers/thermal/qcom-spmi-temp-alarm.c
@@ -23,6 +23,8 @@
#include <linux/regmap.h>
#include <linux/thermal.h>

+#include "thermal_core.h"
+
#define QPNP_TM_REG_TYPE 0x04
#define QPNP_TM_REG_SUBTYPE 0x05
#define QPNP_TM_REG_STATUS 0x08
@@ -37,9 +39,11 @@
#define STATUS_GEN2_STATE_MASK GENMASK(6, 4)
#define STATUS_GEN2_STATE_SHIFT 4

-#define SHUTDOWN_CTRL1_OVERRIDE_MASK GENMASK(7, 6)
+#define SHUTDOWN_CTRL1_OVERRIDE_S2 BIT(6)
#define SHUTDOWN_CTRL1_THRESHOLD_MASK GENMASK(1, 0)

+#define SHUTDOWN_CTRL1_RATE_25HZ BIT(3)
+
#define ALARM_CTRL_FORCE_ENABLE BIT(7)

/*
@@ -56,12 +60,19 @@
#define TEMP_THRESH_STEP 5000 /* Threshold step: 5 C */

#define THRESH_MIN 0
+#define THRESH_MAX 3
+
+/* Stage 2 Threshold Min: 125 C */
+#define STAGE2_THRESHOLD_MIN 125000
+/* Stage 2 Threshold Max: 140 C */
+#define STAGE2_THRESHOLD_MAX 140000

/* Temperature in Milli Celsius reported during stage 0 if no ADC is present */
#define DEFAULT_TEMP 37000

struct qpnp_tm_chip {
struct regmap *map;
+ struct device *dev;
struct thermal_zone_device *tz_dev;
unsigned int subtype;
long temp;
@@ -69,6 +80,10 @@ struct qpnp_tm_chip {
unsigned int stage;
unsigned int prev_stage;
unsigned int base;
+ /* protects .thresh, .stage and chip registers */
+ struct mutex lock;
+ bool initialized;
+
struct iio_channel *adc;
};

@@ -125,6 +140,8 @@ static int qpnp_tm_update_temp_no_adc(struct qpnp_tm_chip *chip)
unsigned int stage, stage_new, stage_old;
int ret;

+ WARN_ON(!mutex_is_locked(&chip->lock));
+
ret = qpnp_tm_get_temp_stage(chip);
if (ret < 0)
return ret;
@@ -163,8 +180,15 @@ static int qpnp_tm_get_temp(void *data, int *temp)
if (!temp)
return -EINVAL;

+ if (!chip->initialized) {
+ *temp = DEFAULT_TEMP;
+ return 0;
+ }
+
if (!chip->adc) {
+ mutex_lock(&chip->lock);
ret = qpnp_tm_update_temp_no_adc(chip);
+ mutex_unlock(&chip->lock);
if (ret < 0)
return ret;
} else {
@@ -180,8 +204,72 @@ static int qpnp_tm_get_temp(void *data, int *temp)
return 0;
}

+static int qpnp_tm_update_critical_trip_temp(struct qpnp_tm_chip *chip,
+ int temp)
+{
+ u8 reg;
+ bool disable_s2_shutdown = false;
+
+ WARN_ON(!mutex_is_locked(&chip->lock));
+
+ /*
+ * Default: S2 and S3 shutdown enabled, thresholds at
+ * 105C/125C/145C, monitoring at 25Hz
+ */
+ reg = SHUTDOWN_CTRL1_RATE_25HZ;
+
+ if (temp == THERMAL_TEMP_INVALID ||
+ temp < STAGE2_THRESHOLD_MIN) {
+ chip->thresh = THRESH_MIN;
+ goto skip;
+ }
+
+ if (temp <= STAGE2_THRESHOLD_MAX) {
+ chip->thresh = THRESH_MAX -
+ ((STAGE2_THRESHOLD_MAX - temp) /
+ TEMP_THRESH_STEP);
+ disable_s2_shutdown = true;
+ } else {
+ chip->thresh = THRESH_MAX;
+
+ if (chip->adc)
+ disable_s2_shutdown = true;
+ else
+ dev_warn(chip->dev,
+ "No ADC is configured and critical temperature is above the maximum stage 2 threshold of 140 C! Configuring stage 2 shutdown at 140 C.\n");
+ }
+
+skip:
+ reg |= chip->thresh;
+ if (disable_s2_shutdown)
+ reg |= SHUTDOWN_CTRL1_OVERRIDE_S2;
+
+ return qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
+}
+
+static int qpnp_tm_set_trip_temp(void *data, int trip, int temp)
+{
+ struct qpnp_tm_chip *chip = data;
+ const struct thermal_trip *trip_points;
+ int ret;
+
+ trip_points = of_thermal_get_trip_points(chip->tz_dev);
+ if (!trip_points)
+ return -EINVAL;
+
+ if (trip_points[trip].type != THERMAL_TRIP_CRITICAL)
+ return 0;
+
+ mutex_lock(&chip->lock);
+ ret = qpnp_tm_update_critical_trip_temp(chip, temp);
+ mutex_unlock(&chip->lock);
+
+ return ret;
+}
+
static const struct thermal_zone_of_device_ops qpnp_tm_sensor_ops = {
.get_temp = qpnp_tm_get_temp,
+ .set_trip_temp = qpnp_tm_set_trip_temp,
};

static irqreturn_t qpnp_tm_isr(int irq, void *data)
@@ -193,6 +281,29 @@ static irqreturn_t qpnp_tm_isr(int irq, void *data)
return IRQ_HANDLED;
}

+static int qpnp_tm_get_critical_trip_temp(struct qpnp_tm_chip *chip)
+{
+ int ntrips;
+ const struct thermal_trip *trips;
+ int i;
+
+ ntrips = of_thermal_get_ntrips(chip->tz_dev);
+ if (ntrips <= 0)
+ return THERMAL_TEMP_INVALID;
+
+ trips = of_thermal_get_trip_points(chip->tz_dev);
+ if (!trips)
+ return THERMAL_TEMP_INVALID;
+
+ for (i = 0; i < ntrips; i++) {
+ if (of_thermal_is_trip_valid(chip->tz_dev, i) &&
+ trips[i].type == THERMAL_TRIP_CRITICAL)
+ return trips[i].temperature;
+ }
+
+ return THERMAL_TEMP_INVALID;
+}
+
/*
* This function initializes the internal temp value based on only the
* current thermal stage and threshold. Setup threshold control and
@@ -203,17 +314,20 @@ static int qpnp_tm_init(struct qpnp_tm_chip *chip)
unsigned int stage;
int ret;
u8 reg = 0;
+ int crit_temp;
+
+ mutex_lock(&chip->lock);

ret = qpnp_tm_read(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, &reg);
if (ret < 0)
- return ret;
+ goto out;

chip->thresh = reg & SHUTDOWN_CTRL1_THRESHOLD_MASK;
chip->temp = DEFAULT_TEMP;

ret = qpnp_tm_get_temp_stage(chip);
if (ret < 0)
- return ret;
+ goto out;
chip->stage = ret;

stage = chip->subtype == QPNP_TM_SUBTYPE_GEN1
@@ -224,21 +338,19 @@ static int qpnp_tm_init(struct qpnp_tm_chip *chip)
(stage - 1) * TEMP_STAGE_STEP +
TEMP_THRESH_MIN;

- /*
- * Set threshold and disable software override of stage 2 and 3
- * shutdowns.
- */
- chip->thresh = THRESH_MIN;
- reg &= ~(SHUTDOWN_CTRL1_OVERRIDE_MASK | SHUTDOWN_CTRL1_THRESHOLD_MASK);
- reg |= chip->thresh & SHUTDOWN_CTRL1_THRESHOLD_MASK;
- ret = qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
+ crit_temp = qpnp_tm_get_critical_trip_temp(chip);
+ ret = qpnp_tm_update_critical_trip_temp(chip, crit_temp);
if (ret < 0)
- return ret;
+ goto out;

/* Enable the thermal alarm PMIC module in always-on mode. */
reg = ALARM_CTRL_FORCE_ENABLE;
ret = qpnp_tm_write(chip, QPNP_TM_REG_ALARM_CTRL, reg);

+ chip->initialized = true;
+
+out:
+ mutex_unlock(&chip->lock);
return ret;
}

@@ -257,6 +369,9 @@ static int qpnp_tm_probe(struct platform_device *pdev)
return -ENOMEM;

dev_set_drvdata(&pdev->dev, chip);
+ chip->dev = &pdev->dev;
+
+ mutex_init(&chip->lock);

chip->map = dev_get_regmap(pdev->dev.parent, NULL);
if (!chip->map)
@@ -302,6 +417,18 @@ static int qpnp_tm_probe(struct platform_device *pdev)

chip->subtype = subtype;

+ /*
+ * Register the sensor before initializing the hardware to be able to
+ * read the trip points. get_temp() returns the default temperature
+ * before the hardware initialization is completed.
+ */
+ chip->tz_dev = devm_thermal_zone_of_sensor_register(
+ &pdev->dev, 0, chip, &qpnp_tm_sensor_ops);
+ if (IS_ERR(chip->tz_dev)) {
+ dev_err(&pdev->dev, "failed to register sensor\n");
+ return PTR_ERR(chip->tz_dev);
+ }
+
ret = qpnp_tm_init(chip);
if (ret < 0) {
dev_err(&pdev->dev, "init failed\n");
@@ -313,12 +440,7 @@ static int qpnp_tm_probe(struct platform_device *pdev)
if (ret < 0)
return ret;

- chip->tz_dev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, chip,
- &qpnp_tm_sensor_ops);
- if (IS_ERR(chip->tz_dev)) {
- dev_err(&pdev->dev, "failed to register sensor\n");
- return PTR_ERR(chip->tz_dev);
- }
+ thermal_zone_device_update(chip->tz_dev, THERMAL_EVENT_UNSPECIFIED);

return 0;
}
--
2.18.0.345.g5c9ce644c3-goog


2018-07-31 19:01:31

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v6 4/5] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node

This adds the spmi-temp-alarm node to pm8998 based on the examples in the
bindings.

Signed-off-by: Matthias Kaehlcke <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---
Changes in v6:
- none

Changes in v5:
- added tag 'Reviewed-by: Douglas Anderson <[email protected]>'

Changes in v4:
- none

Changes in v3:
- changed node name from 'qcom,temp-alarm@2400' to 'temp-alarm@2400'
- removed controller register length value from 'reg'

Changes in v2:
- none
---
arch/arm64/boot/dts/qcom/pm8998.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi
index 92bed1e7d4bb..7eea94701b23 100644
--- a/arch/arm64/boot/dts/qcom/pm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi
@@ -11,6 +11,13 @@
#address-cells = <1>;
#size-cells = <0>;

+ pm8998_temp: temp-alarm@2400 {
+ compatible = "qcom,spmi-temp-alarm";
+ reg = <0x2400>;
+ interrupts = <0x0 0x24 0x0 IRQ_TYPE_EDGE_RISING>;
+ #thermal-sensor-cells = <0>;
+ };
+
pm8998_gpio: gpios@c000 {
compatible = "qcom,pm8998-gpio", "qcom,spmi-gpio";
reg = <0xc000>;
--
2.18.0.345.g5c9ce644c3-goog


2018-07-31 19:02:47

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v6 2/5] dt-bindings: thermal: qcom-spmi-temp-alarm: Improve thermal zone in example

The current example for a thermal zone isn't very useful as reference
since it would result in a hardware shutdown at 145°C, instead of
allowing the system to try to shutdown gracefully. Without an ADC
channel a maximum of two trip points is useful in practice for this
sensor, with temperatures corresponding to the stage 1 and stage 2
'hardware trip points'. A critical trip point at stage 2 may allow the
system to shutdown before a hardware shutdown at stage 3 kicks in. It
should be noted though that by default the chip performs a 'partial
shutdown' when the temperature reaches stage 2, which may prevent an
orderly shutdown. The 'partial shutdown' can be disabled by software.

Signed-off-by: Matthias Kaehlcke <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---
Changes in v6:
- patch added to the series
---
.../bindings/thermal/qcom-spmi-temp-alarm.txt | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt b/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
index 86fb41fe772f..0273a92a2a84 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
@@ -35,19 +35,14 @@ Example:
thermal-sensors = <&pm8941_temp>;

trips {
- passive {
- temperature = <1050000>;
+ stage1 {
+ temperature = <105000>;
hysteresis = <2000>;
type = "passive";
};
- alert {
+ stage2 {
temperature = <125000>;
hysteresis = <2000>;
- type = "hot";
- };
- crit {
- temperature = <145000>;
- hysteresis = <2000>;
type = "critical";
};
};
--
2.18.0.345.g5c9ce644c3-goog


2018-07-31 19:21:11

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v6 1/5] dt-bindings: thermal: qcom-spmi-temp-alarm: Fix documentation of 'reg'

On Tue, Jul 31, 2018 at 11:59:13AM -0700, Matthias Kaehlcke wrote:
> The documentation claims that the 'reg' property consists of two values,
> the SPMI address and the length of the controller's registers. However
> the SPMI bus to which it is added specifies "#size-cells = <0>;". Remove
> the controller register length from the documentation of the field and the
> example.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> Reviewed-by: Douglas Anderson <[email protected]>
> ---
> Changes in v6:
> - patch added to the series
> ---
> .../devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Rob Herring <[email protected]>


2018-07-31 19:24:27

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v6 2/5] dt-bindings: thermal: qcom-spmi-temp-alarm: Improve thermal zone in example

On Tue, Jul 31, 2018 at 11:59:14AM -0700, Matthias Kaehlcke wrote:
> The current example for a thermal zone isn't very useful as reference
> since it would result in a hardware shutdown at 145?C, instead of
> allowing the system to try to shutdown gracefully. Without an ADC
> channel a maximum of two trip points is useful in practice for this
> sensor, with temperatures corresponding to the stage 1 and stage 2
> 'hardware trip points'. A critical trip point at stage 2 may allow the
> system to shutdown before a hardware shutdown at stage 3 kicks in. It
> should be noted though that by default the chip performs a 'partial
> shutdown' when the temperature reaches stage 2, which may prevent an
> orderly shutdown. The 'partial shutdown' can be disabled by software.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> Reviewed-by: Douglas Anderson <[email protected]>
> ---
> Changes in v6:
> - patch added to the series
> ---
> .../bindings/thermal/qcom-spmi-temp-alarm.txt | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)

Reviewed-by: Rob Herring <[email protected]>

2018-08-09 21:30:06

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v6 3/5] thermal: qcom-spmi: Use PMIC thermal stage 2 for critical trip points

Hi,

On Tue, Jul 31, 2018 at 11:59 AM, Matthias Kaehlcke <[email protected]> wrote:
> There are three thermal stages defined in the PMIC:
>
> stage 1: warning
> stage 2: system should shut down
> stage 3: emergency shut down
>
> By default the PMIC assumes that the OS isn't doing anything and thus
> at stage 2 it does a partial PMIC shutdown and at stage 3 it kills
> all power. When switching between thermal stages the PMIC generates an
> interrupt which is handled by the driver. The partial PMIC shutdown at
> stage 2 can be disabled by software, which allows the OS to initiate a
> shutdown at stage 2 with a thermal zone configured accordingly.
>
> If a critical trip point is configured in the thermal zone the driver
> adjusts the stage 1-3 temperature thresholds to (closely) match the
> critical temperature with a stage 2 threshold (125/130/135/140 °C).
> If a suitable match is found the partial shutdown at stage 2 is
> disabled. If for some reason the system doesn't shutdown at stage 2
> the emergency shutdown at stage 3 kicks in.
>
> The partial shutdown at stage 2 remains enabled in these cases:
> - no critical trip point defined
> - the temperature of the critical trip point is < 125°C
> - the temperature of the critical trip point is > 140°C and no
> ADC channel is configured (thus the OS is not notified when the critical
> temperature is reached)
>
> Suggested-by: Douglas Anderson <[email protected]>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> Changes in v6:
> - fixed condition to check if ADC is configured in
> qpnp_tm_update_critical_trip_temp()
> - changed °C in logs to C
> - removed needless evaluation of qpnp_tm_write() return value in
> qpnp_tm_update_critical_trip_temp()
> - move assignment of chip->initialized to true to qpnp_tm_init(),
> where the lock is held
> - call thermal_zone_device_update() after initialization is
> completed
> - split some #define + comment in two lines to avoid exceeding
> chars per line limit
> - removed extra closing parenthesis in qpnp_tm_get_temp()
> - remove unnecessary parentheses around conditions in
> qpnp_tm_update_critical_trip_temp() and qpnp_tm_get_critical_trip_temp()
> - fixed indentation of call devm_thermal_zone_of_sensor_register() call
> in qpnp_tm_probe()
>
> Changes in v5:
> - patch added to the series
> ---
> drivers/thermal/qcom-spmi-temp-alarm.c | 158 ++++++++++++++++++++++---
> 1 file changed, 140 insertions(+), 18 deletions(-)

I won't claim to have spent too much time in the thermal framework,
but from what I can tell everything looks supergreat now. FWIW:

Reviewed-by: Douglas Anderson <[email protected]>

One minor comment I'd have is that I personally would have ordered
this in the series before the patch ("dt-bindings: thermal:
qcom-spmi-temp-alarm: Improve thermal zone in example") just because
that patch only really makes sense after this one lands. ...but I
don't think it's terribly important.


-Doug

2018-08-24 23:14:04

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCH v6 1/5] dt-bindings: thermal: qcom-spmi-temp-alarm: Fix documentation of 'reg'

Hey

On Tue, Jul 31, 2018 at 11:59:13AM -0700, Matthias Kaehlcke wrote:
> The documentation claims that the 'reg' property consists of two values,
> the SPMI address and the length of the controller's registers. However
> the SPMI bus to which it is added specifies "#size-cells = <0>;". Remove
> the controller register length from the documentation of the field and the
> example.


queuing this for next merge window. Applied up to patch 3. both
dt changes go via your arch tree and you can add my ack on them.
Acked-by: Eduardo Valentin <[email protected]>

>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> Reviewed-by: Douglas Anderson <[email protected]>
> ---
> Changes in v6:
> - patch added to the series
> ---
> .../devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt b/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
> index 290ec06fa33a..86fb41fe772f 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt
> @@ -6,8 +6,7 @@ interrupt signal and status register to identify high PMIC die temperature.
>
> Required properties:
> - compatible: Should contain "qcom,spmi-temp-alarm".
> -- reg: Specifies the SPMI address and length of the controller's
> - registers.
> +- reg: Specifies the SPMI address.
> - interrupts: PMIC temperature alarm interrupt.
> - #thermal-sensor-cells: Should be 0. See thermal.txt for a description.
>
> @@ -20,7 +19,7 @@ Example:
>
> pm8941_temp: thermal-alarm@2400 {
> compatible = "qcom,spmi-temp-alarm";
> - reg = <0x2400 0x100>;
> + reg = <0x2400>;
> interrupts = <0 0x24 0 IRQ_TYPE_EDGE_RISING>;
> #thermal-sensor-cells = <0>;
>
> --
> 2.18.0.345.g5c9ce644c3-goog
>