All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tyler Hall <tylerwhall@gmail.com>
To: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org,
	Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
	Zhang Rui <rui.zhang@intel.com>,
	Eduardo Valentin <edubezval@gmail.com>,
	Tyler Hall <tylerwhall@gmail.com>
Subject: [PATCH] thermal: armada: read stable temp on Armada XP
Date: Tue, 10 Feb 2015 17:50:15 -0500	[thread overview]
Message-ID: <1423608615-6575-1-git-send-email-tylerwhall@gmail.com> (raw)

The current register being used to read the temperature returns a noisy value
that is prone to variance and occasional outliers. The value in the thermal
manager control and status register appears to have the same scale but much
less variability.

Add a "marvell,armadaxp-filtered-thermal" config which is set up to read from
the Thermal Manager Control and Status Register at 0x184c4 rather than the
Thermal Sensor Status Register at 0x182b0. The only difference is the
temperature value shift. The original "marvell,armadaxp-thermal" is retained
for device tree compatibility.

This also fixes Armada XP clearing the disable bit in the wrong register. Bit 0
of the sensor register was being cleared but that bit is read-only. The disable
bit doesn't seem to have an effect on the temperature sensor value, however, so
this doesn't make a material difference.

This problem was detected when running with the watchdog(8) daemon polling the
thermal value. In one instance I observed a single read of over 200 degrees C
which caused a spurious watchdog-initiated reboot. I have since observed
individual outliers of ~20 degrees C. With this change and the corresponding
device tree update, the temperature is much more stable.

Signed-off-by: Tyler Hall <tylerwhall@gmail.com>
---

If there's a better way to handle this than a separate binding, I'm open to
suggestions.

My conclusions about these registers are based on experimental data. The
documentation is very sparse, but the Thermal Manager Control and Status
Register looks like the preferred register given the way it is laid out in the
public spec.

I have the small corresponding patch to the dts which I can submit separately.

Thanks
Tyler

 .../devicetree/bindings/thermal/armada-thermal.txt          |  8 ++++++++
 drivers/thermal/armada_thermal.c                            | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 4698e0e..0d6a3f1 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -7,6 +7,14 @@ Required properties:
 		marvell,armada375-thermal
 		marvell,armada380-thermal
 		marvell,armadaxp-thermal
+		marvell,armadaxp-filtered-thermal
+
+		Note: "marvell,armadaxp-filtered-thermal" is adjusted to read
+		the hardware-filtered value in the thermal manager
+		control/status register rather than the raw sensor value in the
+		thermal sensor status register. "marvell,armadaxp-thermal"
+		remains for backward compatibility. The sensor reg address must
+		also point to the appropriate register.
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index c2556cf..d3c2ad3 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -196,6 +196,15 @@ static const struct armada_thermal_data armadaxp_data = {
 	.coef_div = 13825,
 };
 
+static const struct armada_thermal_data armadaxp_filt_data = {
+	.init_sensor = armadaxp_init_sensor,
+	.temp_shift = 1,
+	.temp_mask = 0x1ff,
+	.coef_b = 3153000000UL,
+	.coef_m = 10000000UL,
+	.coef_div = 13825,
+};
+
 static const struct armada_thermal_data armada370_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada370_init_sensor,
@@ -236,6 +245,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armadaxp_data,
 	},
 	{
+		.compatible = "marvell,armadaxp-filtered-thermal",
+		.data       = &armadaxp_filt_data,
+	},
+	{
 		.compatible = "marvell,armada370-thermal",
 		.data       = &armada370_data,
 	},
-- 
2.3.0


WARNING: multiple messages have this Message-ID (diff)
From: Tyler Hall <tylerwhall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Ezequiel Garcia
	<ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Eduardo Valentin
	<edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Tyler Hall <tylerwhall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: [PATCH] thermal: armada: read stable temp on Armada XP
Date: Tue, 10 Feb 2015 17:50:15 -0500	[thread overview]
Message-ID: <1423608615-6575-1-git-send-email-tylerwhall@gmail.com> (raw)

The current register being used to read the temperature returns a noisy value
that is prone to variance and occasional outliers. The value in the thermal
manager control and status register appears to have the same scale but much
less variability.

Add a "marvell,armadaxp-filtered-thermal" config which is set up to read from
the Thermal Manager Control and Status Register at 0x184c4 rather than the
Thermal Sensor Status Register at 0x182b0. The only difference is the
temperature value shift. The original "marvell,armadaxp-thermal" is retained
for device tree compatibility.

This also fixes Armada XP clearing the disable bit in the wrong register. Bit 0
of the sensor register was being cleared but that bit is read-only. The disable
bit doesn't seem to have an effect on the temperature sensor value, however, so
this doesn't make a material difference.

This problem was detected when running with the watchdog(8) daemon polling the
thermal value. In one instance I observed a single read of over 200 degrees C
which caused a spurious watchdog-initiated reboot. I have since observed
individual outliers of ~20 degrees C. With this change and the corresponding
device tree update, the temperature is much more stable.

Signed-off-by: Tyler Hall <tylerwhall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

If there's a better way to handle this than a separate binding, I'm open to
suggestions.

My conclusions about these registers are based on experimental data. The
documentation is very sparse, but the Thermal Manager Control and Status
Register looks like the preferred register given the way it is laid out in the
public spec.

I have the small corresponding patch to the dts which I can submit separately.

Thanks
Tyler

 .../devicetree/bindings/thermal/armada-thermal.txt          |  8 ++++++++
 drivers/thermal/armada_thermal.c                            | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 4698e0e..0d6a3f1 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -7,6 +7,14 @@ Required properties:
 		marvell,armada375-thermal
 		marvell,armada380-thermal
 		marvell,armadaxp-thermal
+		marvell,armadaxp-filtered-thermal
+
+		Note: "marvell,armadaxp-filtered-thermal" is adjusted to read
+		the hardware-filtered value in the thermal manager
+		control/status register rather than the raw sensor value in the
+		thermal sensor status register. "marvell,armadaxp-thermal"
+		remains for backward compatibility. The sensor reg address must
+		also point to the appropriate register.
 
 - reg:		Device's register space.
 		Two entries are expected, see the examples below.
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index c2556cf..d3c2ad3 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -196,6 +196,15 @@ static const struct armada_thermal_data armadaxp_data = {
 	.coef_div = 13825,
 };
 
+static const struct armada_thermal_data armadaxp_filt_data = {
+	.init_sensor = armadaxp_init_sensor,
+	.temp_shift = 1,
+	.temp_mask = 0x1ff,
+	.coef_b = 3153000000UL,
+	.coef_m = 10000000UL,
+	.coef_div = 13825,
+};
+
 static const struct armada_thermal_data armada370_data = {
 	.is_valid = armada_is_valid,
 	.init_sensor = armada370_init_sensor,
@@ -236,6 +245,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
 		.data       = &armadaxp_data,
 	},
 	{
+		.compatible = "marvell,armadaxp-filtered-thermal",
+		.data       = &armadaxp_filt_data,
+	},
+	{
 		.compatible = "marvell,armada370-thermal",
 		.data       = &armada370_data,
 	},
-- 
2.3.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

             reply	other threads:[~2015-02-10 22:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-10 22:50 Tyler Hall [this message]
2015-02-10 22:50 ` [PATCH] thermal: armada: read stable temp on Armada XP Tyler Hall
2015-02-24 18:36 ` Eduardo Valentin
2015-02-24 18:36   ` Eduardo Valentin
2015-02-24 19:56   ` Tyler Hall
2015-02-24 19:56     ` Tyler Hall
2015-02-25 16:10     ` Gregory CLEMENT
2015-02-25 18:39       ` Eduardo Valentin
2015-02-25 19:47         ` Tyler Hall
2015-02-25 19:47           ` Tyler Hall
2015-02-25 22:39           ` Tyler Hall
2015-02-25 17:04 ` Gregory CLEMENT
2015-02-25 18:17   ` Ezequiel Garcia
2015-02-25 18:38     ` Gregory CLEMENT

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1423608615-6575-1-git-send-email-tylerwhall@gmail.com \
    --to=tylerwhall@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=edubezval@gmail.com \
    --cc=ezequiel.garcia@free-electrons.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rui.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.