Hi all,
Here's v2 set of thermal fixes for single mode read. Turns out the
EOCZ and SOC bit handling is quite different between the various
SoCs.
With these changes we fix the following issues for reading a
single sample:
- Get rid of pointless register access and loops for dra7
- Fix omap3 to use proper timeouts, the current looping is
way too short and always times out probably leading to
bogus values as folks have reported
- Fix omap4430 where EOCZ only seems to work for continuous
mode
Regards,
Tony
Changes since v1:
- Use better MODE_CONFIG checks as suggested by Peter
- Fix issues for omap3 as noted by Adam
- Fix handling for dra7
Tony Lindgren (4):
thermal: ti-soc-thermal: Skip pointless register access for dra7
thermal: ti-soc-thermal: Fix stuck sensor with continuous mode for
4430
thermal: ti-soc-thermal: Simplify polling with iopoll
thermal: ti-soc-thermal: Use non-inverted define for omap4
.../ti-soc-thermal/omap4-thermal-data.c | 7 +--
.../thermal/ti-soc-thermal/omap4xxx-bandgap.h | 4 +-
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 54 ++++++++++---------
drivers/thermal/ti-soc-thermal/ti-bandgap.h | 2 +
4 files changed, 38 insertions(+), 29 deletions(-)
--
2.30.0
When we set bit 10 high we use continuous mode and not single
mode. Let's correct this to avoid confusion. No functional
changes here, the code does the right thing with bit 10.
Cc: Adam Ford <[email protected]>
Cc: Carl Philipp Klemm <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: H. Nikolaus Schaller <[email protected]>
Cc: Merlijn Wajer <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Peter Ujfalusi <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/thermal/ti-soc-thermal/omap4-thermal-data.c | 4 ++--
drivers/thermal/ti-soc-thermal/omap4xxx-bandgap.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c b/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
--- a/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
+++ b/drivers/thermal/ti-soc-thermal/omap4-thermal-data.c
@@ -24,7 +24,7 @@ omap4430_mpu_temp_sensor_registers = {
.bgap_dtemp_mask = OMAP4430_BGAP_TEMP_SENSOR_DTEMP_MASK,
.bgap_mode_ctrl = OMAP4430_TEMP_SENSOR_CTRL_OFFSET,
- .mode_ctrl_mask = OMAP4430_SINGLE_MODE_MASK,
+ .mode_ctrl_mask = OMAP4430_CONTINUOUS_MODE_MASK,
.bgap_efuse = OMAP4430_FUSE_OPP_BGAP,
};
@@ -97,7 +97,7 @@ omap4460_mpu_temp_sensor_registers = {
.mask_cold_mask = OMAP4460_MASK_COLD_MASK,
.bgap_mode_ctrl = OMAP4460_BGAP_CTRL_OFFSET,
- .mode_ctrl_mask = OMAP4460_SINGLE_MODE_MASK,
+ .mode_ctrl_mask = OMAP4460_CONTINUOUS_MODE_MASK,
.bgap_counter = OMAP4460_BGAP_COUNTER_OFFSET,
.counter_mask = OMAP4460_COUNTER_MASK,
diff --git a/drivers/thermal/ti-soc-thermal/omap4xxx-bandgap.h b/drivers/thermal/ti-soc-thermal/omap4xxx-bandgap.h
--- a/drivers/thermal/ti-soc-thermal/omap4xxx-bandgap.h
+++ b/drivers/thermal/ti-soc-thermal/omap4xxx-bandgap.h
@@ -40,7 +40,7 @@
/* OMAP4430.TEMP_SENSOR bits */
#define OMAP4430_BGAP_TEMPSOFF_MASK BIT(12)
#define OMAP4430_BGAP_TSHUT_MASK BIT(11)
-#define OMAP4430_SINGLE_MODE_MASK BIT(10)
+#define OMAP4430_CONTINUOUS_MODE_MASK BIT(10)
#define OMAP4430_BGAP_TEMP_SENSOR_SOC_MASK BIT(9)
#define OMAP4430_BGAP_TEMP_SENSOR_EOCZ_MASK BIT(8)
#define OMAP4430_BGAP_TEMP_SENSOR_DTEMP_MASK (0xff << 0)
@@ -113,7 +113,7 @@
#define OMAP4460_BGAP_TEMP_SENSOR_DTEMP_MASK (0x3ff << 0)
/* OMAP4460.BANDGAP_CTRL bits */
-#define OMAP4460_SINGLE_MODE_MASK BIT(31)
+#define OMAP4460_CONTINUOUS_MODE_MASK BIT(31)
#define OMAP4460_MASK_HOT_MASK BIT(1)
#define OMAP4460_MASK_COLD_MASK BIT(0)
--
2.30.0
On dra7, there is no Start of Conversion (SOC) register bit and we have an
empty bgap_soc_mask in the configuration for the thermal driver. Let's not
do pointless reads and writes with the empty mask.
There's also no point waiting for End of Conversion bit (EOCZ) to go high
on dra7. We only care about it going down, and are now mostly timing out
waiting for EOCZ high while it has already gone down.
When we add checking for the timeout errors in a later patch, waiting for
EOCZ high would cause bogus time out errors.
Cc: Adam Ford <[email protected]>
Cc: Carl Philipp Klemm <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: H. Nikolaus Schaller <[email protected]>
Cc: Merlijn Wajer <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Peter Ujfalusi <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 29 +++++++++++----------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -602,29 +602,30 @@ void *ti_bandgap_get_sensor_data(struct ti_bandgap *bgp, int id)
static int
ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
{
- u32 counter = 1000;
- struct temp_sensor_registers *tsr;
+ struct temp_sensor_registers *tsr = bgp->conf->sensors[id].registers;
+ u32 counter;
/* Select single conversion mode */
if (TI_BANDGAP_HAS(bgp, MODE_CONFIG))
RMW_BITS(bgp, id, bgap_mode_ctrl, mode_ctrl_mask, 0);
- /* Start of Conversion = 1 */
- RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 1);
+ /* Set Start of Conversion if available */
+ if (tsr->bgap_soc_mask) {
+ RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 1);
- /* Wait for EOCZ going up */
- tsr = bgp->conf->sensors[id].registers;
+ /* Wait for EOCZ going up */
+ counter = 1000;
+ while (--counter) {
+ if (ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
+ tsr->bgap_eocz_mask)
+ break;
+ }
- while (--counter) {
- if (ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
- tsr->bgap_eocz_mask)
- break;
+ /* Clear Start of Conversion if available */
+ RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 0);
}
- /* Start of Conversion = 0 */
- RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 0);
-
- /* Wait for EOCZ going down */
+ /* Wait for EOCZ going down, always needed even if no bgap_soc_mask */
counter = 1000;
while (--counter) {
if (!(ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
--
2.30.0
We can use iopoll for checking the EOCZ (end of conversion) bit. And with
this we now also want to handle the timeout errors properly.
For omap3, we need about 1.2ms for the single mode sampling to wait for
EOCZ down, so let's use 1.5ms timeout there. Waiting for sampling to start
is faster and we can use 1ms timeout.
Cc: Adam Ford <[email protected]>
Cc: Carl Philipp Klemm <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: H. Nikolaus Schaller <[email protected]>
Cc: Merlijn Wajer <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Peter Ujfalusi <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 30 ++++++++++-----------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -15,7 +15,6 @@
#include <linux/kernel.h>
#include <linux/interrupt.h>
#include <linux/clk.h>
-#include <linux/delay.h>
#include <linux/gpio/consumer.h>
#include <linux/platform_device.h>
#include <linux/err.h>
@@ -27,6 +26,7 @@
#include <linux/of_platform.h>
#include <linux/of_irq.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/cpu_pm.h>
#include <linux/device.h>
#include <linux/pm_runtime.h>
@@ -604,7 +604,9 @@ static int
ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
{
struct temp_sensor_registers *tsr = bgp->conf->sensors[id].registers;
- u32 counter;
+ void __iomem *temp_sensor_ctrl = bgp->base + tsr->temp_sensor_ctrl;
+ int error;
+ u32 val;
/* Select continuous or single conversion mode */
if (TI_BANDGAP_HAS(bgp, MODE_CONFIG)) {
@@ -619,26 +621,22 @@ ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 1);
/* Wait for EOCZ going up */
- counter = 1000;
- while (--counter) {
- if (ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
- tsr->bgap_eocz_mask)
- break;
- udelay(1);
- }
+ error = readl_poll_timeout_atomic(temp_sensor_ctrl, val,
+ val & tsr->bgap_eocz_mask,
+ 1, 1000);
+ if (error)
+ dev_warn(bgp->dev, "eocz timed out waiting high\n");
/* Clear Start of Conversion if available */
RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 0);
}
/* Wait for EOCZ going down, always needed even if no bgap_soc_mask */
- counter = 1000;
- while (--counter) {
- if (!(ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
- tsr->bgap_eocz_mask))
- break;
- udelay(1);
- }
+ error = readl_poll_timeout_atomic(temp_sensor_ctrl, val,
+ !(val & tsr->bgap_eocz_mask),
+ 1, 1500);
+ if (error)
+ dev_warn(bgp->dev, "eocz timed out waiting low\n");
return 0;
}
--
2.30.0
Hi!
> When we set bit 10 high we use continuous mode and not single
> mode. Let's correct this to avoid confusion. No functional
> changes here, the code does the right thing with bit 10.
Seems okay to me. 1/4:
Acked-by: Pavel Machek <[email protected]>
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
On Fri, Feb 5, 2021 at 7:45 AM Tony Lindgren <[email protected]> wrote:
>
> We can use iopoll for checking the EOCZ (end of conversion) bit. And with
> this we now also want to handle the timeout errors properly.
>
> For omap3, we need about 1.2ms for the single mode sampling to wait for
> EOCZ down, so let's use 1.5ms timeout there. Waiting for sampling to start
> is faster and we can use 1ms timeout.
>
> Cc: Adam Ford <[email protected]>
> Cc: Carl Philipp Klemm <[email protected]>
> Cc: Eduardo Valentin <[email protected]>
> Cc: H. Nikolaus Schaller <[email protected]>
> Cc: Merlijn Wajer <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Peter Ujfalusi <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
For the series,
Tested-by: Adam Ford <[email protected]> #logicpd-torpedo-37xx-devkit
> ---
> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 30 ++++++++++-----------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> @@ -15,7 +15,6 @@
> #include <linux/kernel.h>
> #include <linux/interrupt.h>
> #include <linux/clk.h>
> -#include <linux/delay.h>
> #include <linux/gpio/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/err.h>
> @@ -27,6 +26,7 @@
> #include <linux/of_platform.h>
> #include <linux/of_irq.h>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/cpu_pm.h>
> #include <linux/device.h>
> #include <linux/pm_runtime.h>
> @@ -604,7 +604,9 @@ static int
> ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
> {
> struct temp_sensor_registers *tsr = bgp->conf->sensors[id].registers;
> - u32 counter;
> + void __iomem *temp_sensor_ctrl = bgp->base + tsr->temp_sensor_ctrl;
> + int error;
> + u32 val;
>
> /* Select continuous or single conversion mode */
> if (TI_BANDGAP_HAS(bgp, MODE_CONFIG)) {
> @@ -619,26 +621,22 @@ ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
> RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 1);
>
> /* Wait for EOCZ going up */
> - counter = 1000;
> - while (--counter) {
> - if (ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
> - tsr->bgap_eocz_mask)
> - break;
> - udelay(1);
> - }
> + error = readl_poll_timeout_atomic(temp_sensor_ctrl, val,
> + val & tsr->bgap_eocz_mask,
> + 1, 1000);
> + if (error)
> + dev_warn(bgp->dev, "eocz timed out waiting high\n");
>
> /* Clear Start of Conversion if available */
> RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 0);
> }
>
> /* Wait for EOCZ going down, always needed even if no bgap_soc_mask */
> - counter = 1000;
> - while (--counter) {
> - if (!(ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) &
> - tsr->bgap_eocz_mask))
> - break;
> - udelay(1);
> - }
> + error = readl_poll_timeout_atomic(temp_sensor_ctrl, val,
> + !(val & tsr->bgap_eocz_mask),
> + 1, 1500);
> + if (error)
> + dev_warn(bgp->dev, "eocz timed out waiting low\n");
>
> return 0;
> }
> --
> 2.30.0