diff mbox series

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

Message ID 20180731185917.176074-1-mka@chromium.org
State Not Applicable, archived
Headers show
Series [v6,1/5] dt-bindings: thermal: qcom-spmi-temp-alarm: Fix documentation of 'reg' | expand

Commit Message

Matthias Kaehlcke July 31, 2018, 6:59 p.m. UTC
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 <mka@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v6:
- patch added to the series
---
 .../devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt     | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Rob Herring July 31, 2018, 7:18 p.m. UTC | #1
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 <mka@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> 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 <robh@kernel.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Anderson Aug. 9, 2018, 9:28 p.m. UTC | #2
Hi,

On Tue, Jul 31, 2018 at 11:59 AM, Matthias Kaehlcke <mka@chromium.org> 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 <dianders@chromium.org>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> 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 <dianders@chromium.org>

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
Eduardo Valentin Aug. 24, 2018, 11:11 p.m. UTC | #3
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 <edubezval@gmail.com>

> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> 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
>
diff mbox series

Patch

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>;