* [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
@ 2015-04-10 6:36 Keerthy
2015-04-16 7:49 ` Keerthy
0 siblings, 1 reply; 7+ messages in thread
From: Keerthy @ 2015-04-10 6:36 UTC (permalink / raw)
To: edubezval, rui.zhang; +Cc: linux-pm, linux-omap, j-keerthy
Bandgap Temperature read Dtemp can be corrupted
DESCRIPTION
Read accesses to registers listed below can be corrupted due to incorrect resynchronization between
clock domains.
Read access to registers below can be corrupted :
• CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
• CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
WORKAROUND
Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value:
1. Perform two successive reads to BGAP_DTEMP bit field.
(a) If read1 returns Val1 and read2 returns Val1, then right value is Val1.
(b) If read1 returns Val1, read 2 returns Val2, a third read is needed.
2. Perform third read
(a) If read3 returns Val2 then right value is Val2.
(b) If read3 returns Val3, then right value is Val3.
The above in gist means if val1 and val2 are the same then we can go ahead with that value
else we need a third read which will be right since synchronization will be complete by then.
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
Changes in V2:
* Added more details in the description part.
* Indented the comments.
* Renamed ti_dra7_bandgap_read_temp to ti_errata814_bandgap_read_temp.
.../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37 +++++++++++++++++++++-
drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
index a492927..58b5c66 100644
--- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
+++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
@@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
TI_BANDGAP_FEATURE_FREEZE_BIT |
TI_BANDGAP_FEATURE_TALERT |
TI_BANDGAP_FEATURE_COUNTER_DELAY |
- TI_BANDGAP_FEATURE_HISTORY_BUFFER,
+ TI_BANDGAP_FEATURE_HISTORY_BUFFER |
+ TI_BANDGAP_FEATURE_ERRATA_814,
.fclock_name = "l3instr_ts_gclk_div",
.div_ck_name = "l3instr_ts_gclk_div",
.conv_table = dra752_adc_to_temp,
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index 74c0e34..94acd80 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -119,6 +119,37 @@ exit:
}
/**
+ * ti_errata814_bandgap_read_temp() - helper function to read dra7 sensor temperature
+ * @bgp: pointer to ti_bandgap structure
+ * @reg: desired register (offset) to be read
+ *
+ * Function to read dra7 bandgap sensor temperature. This is done separately
+ * so as to workaround the errata "Bandgap Temperature read Dtemp can be
+ * corrupted" - Errata ID: i814".
+ * Read accesses to registers listed below can be corrupted due to incorrect
+ * resynchronization between clock domains.
+ * Read access to registers below can be corrupted :
+ * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
+ * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
+ *
+ * Return: the register value.
+ */
+static u32 ti_errata814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
+{
+ u32 val1, val2;
+
+ val1 = ti_bandgap_readl(bgp, reg);
+ val2 = ti_bandgap_readl(bgp, reg);
+
+ /* If both times we read the same value then that is right */
+ if (val1 == val2)
+ return val1;
+
+ /* if val1 and val2 are different read it third time */
+ return ti_bandgap_readl(bgp, reg);
+}
+
+/**
* ti_bandgap_read_temp() - helper function to read sensor temperature
* @bgp: pointer to ti_bandgap structure
* @id: bandgap sensor id
@@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
}
/* read temperature */
- temp = ti_bandgap_readl(bgp, reg);
+ if (TI_BANDGAP_HAS(bgp, ERRATA_814))
+ temp = ti_errata814_bandgap_read_temp(bgp, reg);
+ else
+ temp = ti_bandgap_readl(bgp, reg);
+
temp &= tsr->bgap_dtemp_mask;
if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
index b3adf72..b2da3fc 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
@@ -318,6 +318,9 @@ struct ti_temp_sensor {
* TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap device features
* a history buffer of temperatures.
*
+ * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the bandgap device
+ * has Errata 814
+ *
* TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is capable of a
* specific feature (above) or not. Return non-zero, if yes.
*/
@@ -331,6 +334,7 @@ struct ti_temp_sensor {
#define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
#define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
#define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
+#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
#define TI_BANDGAP_HAS(b, f) \
((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
2015-04-10 6:36 [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted Keerthy
@ 2015-04-16 7:49 ` Keerthy
0 siblings, 0 replies; 7+ messages in thread
From: Keerthy @ 2015-04-16 7:49 UTC (permalink / raw)
To: Keerthy, edubezval, rui.zhang; +Cc: linux-pm, linux-omap, Kristo, Tero
On Friday 10 April 2015 12:06 PM, Keerthy wrote:
> Bandgap Temperature read Dtemp can be corrupted
>
> DESCRIPTION
> Read accesses to registers listed below can be corrupted due to incorrect resynchronization between
> clock domains.
>
> Read access to registers below can be corrupted :
> • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
> • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
>
> WORKAROUND
> Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
> BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value:
> 1. Perform two successive reads to BGAP_DTEMP bit field.
> (a) If read1 returns Val1 and read2 returns Val1, then right value is Val1.
> (b) If read1 returns Val1, read 2 returns Val2, a third read is needed.
> 2. Perform third read
> (a) If read3 returns Val2 then right value is Val2.
> (b) If read3 returns Val3, then right value is Val3.
>
> The above in gist means if val1 and val2 are the same then we can go ahead with that value
> else we need a third read which will be right since synchronization will be complete by then.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
A Gentle ping on this. This has the comments fixed.
>
> Changes in V2:
>
> * Added more details in the description part.
> * Indented the comments.
> * Renamed ti_dra7_bandgap_read_temp to ti_errata814_bandgap_read_temp.
>
> .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37 +++++++++++++++++++++-
> drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> index a492927..58b5c66 100644
> --- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> +++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> @@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
> TI_BANDGAP_FEATURE_FREEZE_BIT |
> TI_BANDGAP_FEATURE_TALERT |
> TI_BANDGAP_FEATURE_COUNTER_DELAY |
> - TI_BANDGAP_FEATURE_HISTORY_BUFFER,
> + TI_BANDGAP_FEATURE_HISTORY_BUFFER |
> + TI_BANDGAP_FEATURE_ERRATA_814,
> .fclock_name = "l3instr_ts_gclk_div",
> .div_ck_name = "l3instr_ts_gclk_div",
> .conv_table = dra752_adc_to_temp,
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> index 74c0e34..94acd80 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> @@ -119,6 +119,37 @@ exit:
> }
>
> /**
> + * ti_errata814_bandgap_read_temp() - helper function to read dra7 sensor temperature
> + * @bgp: pointer to ti_bandgap structure
> + * @reg: desired register (offset) to be read
> + *
> + * Function to read dra7 bandgap sensor temperature. This is done separately
> + * so as to workaround the errata "Bandgap Temperature read Dtemp can be
> + * corrupted" - Errata ID: i814".
> + * Read accesses to registers listed below can be corrupted due to incorrect
> + * resynchronization between clock domains.
> + * Read access to registers below can be corrupted :
> + * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
> + * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
> + *
> + * Return: the register value.
> + */
> +static u32 ti_errata814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
> +{
> + u32 val1, val2;
> +
> + val1 = ti_bandgap_readl(bgp, reg);
> + val2 = ti_bandgap_readl(bgp, reg);
> +
> + /* If both times we read the same value then that is right */
> + if (val1 == val2)
> + return val1;
> +
> + /* if val1 and val2 are different read it third time */
> + return ti_bandgap_readl(bgp, reg);
> +}
> +
> +/**
> * ti_bandgap_read_temp() - helper function to read sensor temperature
> * @bgp: pointer to ti_bandgap structure
> * @id: bandgap sensor id
> @@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
> }
>
> /* read temperature */
> - temp = ti_bandgap_readl(bgp, reg);
> + if (TI_BANDGAP_HAS(bgp, ERRATA_814))
> + temp = ti_errata814_bandgap_read_temp(bgp, reg);
> + else
> + temp = ti_bandgap_readl(bgp, reg);
> +
> temp &= tsr->bgap_dtemp_mask;
>
> if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> index b3adf72..b2da3fc 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> @@ -318,6 +318,9 @@ struct ti_temp_sensor {
> * TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap device features
> * a history buffer of temperatures.
> *
> + * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the bandgap device
> + * has Errata 814
> + *
> * TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is capable of a
> * specific feature (above) or not. Return non-zero, if yes.
> */
> @@ -331,6 +334,7 @@ struct ti_temp_sensor {
> #define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
> #define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
> #define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
> +#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
> #define TI_BANDGAP_HAS(b, f) \
> ((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
@ 2015-03-27 8:45 Keerthy
2015-04-07 3:20 ` Keerthy
0 siblings, 1 reply; 7+ messages in thread
From: Keerthy @ 2015-03-27 8:45 UTC (permalink / raw)
To: edubezval, rui.zhang; +Cc: linux-pm, linux-omap, t-kristo, j-keerthy
Bandgap Temperature read Dtemp can be corrupted
DESCRIPTION
Read accesses to registers listed below can be corrupted due to incorrect resynchronization between
clock domains.
Read access to registers below can be corrupted :
• CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
• CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
WORKAROUND
Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value:
1. Perform two successive reads to BGAP_DTEMP bit field.
(a) If read1 returns Val1 and read2 returns Val1, then right value is Val1.
(b) If read1 returns Val1, read 2 returns Val2, a third read is needed.
2. Perform third read
(a) If read3 returns Val2 then right value is Val2.
(b) If read3 returns Val3, then right value is Val3.
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
Read all the temperatures from the 5 sensors and saw that
they were sane with time.
Ran cpuloadgen and saw that temperatures rising on all the sensors
and cooled down as soon as the load was reduced.
.../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37 +++++++++++++++++++++-
drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
index a492927..58b5c66 100644
--- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
+++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
@@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
TI_BANDGAP_FEATURE_FREEZE_BIT |
TI_BANDGAP_FEATURE_TALERT |
TI_BANDGAP_FEATURE_COUNTER_DELAY |
- TI_BANDGAP_FEATURE_HISTORY_BUFFER,
+ TI_BANDGAP_FEATURE_HISTORY_BUFFER |
+ TI_BANDGAP_FEATURE_ERRATA_814,
.fclock_name = "l3instr_ts_gclk_div",
.div_ck_name = "l3instr_ts_gclk_div",
.conv_table = dra752_adc_to_temp,
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index 74c0e34..baf822e 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -119,6 +119,37 @@ exit:
}
/**
+ * ti_dra7_bandgap_read_temp() - helper function to read dra7 sensor temperature
+ * @bgp: pointer to ti_bandgap structure
+ * @reg: desired register (offset) to be read
+ *
+ * Function to read dra7 bandgap sensor temperature. This is done separately
+ * so as to workaround the errata "Bandgap Temperature read Dtemp can be
+ * corrupted" - Errata ID: i814".
+ * Read accesses to registers listed below can be corrupted due to incorrect
+ * resynchronization between clock domains.
+ * Read access to registers below can be corrupted :
+ * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
+ * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
+ *
+ * Return: the register value.
+ */
+static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
+{
+ u32 val1, val2;
+
+ val1 = ti_bandgap_readl(bgp, reg);
+ val2 = ti_bandgap_readl(bgp, reg);
+
+/* If both times we read the same value then that is right */
+ if (val1 == val2)
+ return val1;
+
+/* if val1 and val2 are different read it third time */
+ return ti_bandgap_readl(bgp, reg);
+}
+
+/**
* ti_bandgap_read_temp() - helper function to read sensor temperature
* @bgp: pointer to ti_bandgap structure
* @id: bandgap sensor id
@@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
}
/* read temperature */
- temp = ti_bandgap_readl(bgp, reg);
+ if (TI_BANDGAP_HAS(bgp, ERRATA_814))
+ temp = ti_dra7_bandgap_read_temp(bgp, reg);
+ else
+ temp = ti_bandgap_readl(bgp, reg);
+
temp &= tsr->bgap_dtemp_mask;
if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
index b3adf72..b2da3fc 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
@@ -318,6 +318,9 @@ struct ti_temp_sensor {
* TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap device features
* a history buffer of temperatures.
*
+ * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the bandgap device
+ * has Errata 814
+ *
* TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is capable of a
* specific feature (above) or not. Return non-zero, if yes.
*/
@@ -331,6 +334,7 @@ struct ti_temp_sensor {
#define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
#define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
#define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
+#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
#define TI_BANDGAP_HAS(b, f) \
((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
2015-03-27 8:45 Keerthy
@ 2015-04-07 3:20 ` Keerthy
2015-04-07 20:05 ` Eduardo Valentin
0 siblings, 1 reply; 7+ messages in thread
From: Keerthy @ 2015-04-07 3:20 UTC (permalink / raw)
To: Keerthy, edubezval, rui.zhang; +Cc: linux-pm, linux-omap, t-kristo
Hi Eduardo,
On Friday 27 March 2015 02:15 PM, Keerthy wrote:
> Bandgap Temperature read Dtemp can be corrupted
>
> DESCRIPTION
> Read accesses to registers listed below can be corrupted due to incorrect resynchronization between
> clock domains.
>
> Read access to registers below can be corrupted :
> • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
> • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
>
> WORKAROUND
> Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
> BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value:
> 1. Perform two successive reads to BGAP_DTEMP bit field.
> (a) If read1 returns Val1 and read2 returns Val1, then right value is Val1.
> (b) If read1 returns Val1, read 2 returns Val2, a third read is needed.
> 2. Perform third read
> (a) If read3 returns Val2 then right value is Val2.
> (b) If read3 returns Val3, then right value is Val3.
>
A gentle ping on this.
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>
> Read all the temperatures from the 5 sensors and saw that
> they were sane with time.
>
> Ran cpuloadgen and saw that temperatures rising on all the sensors
> and cooled down as soon as the load was reduced.
>
> .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37 +++++++++++++++++++++-
> drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> index a492927..58b5c66 100644
> --- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> +++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> @@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
> TI_BANDGAP_FEATURE_FREEZE_BIT |
> TI_BANDGAP_FEATURE_TALERT |
> TI_BANDGAP_FEATURE_COUNTER_DELAY |
> - TI_BANDGAP_FEATURE_HISTORY_BUFFER,
> + TI_BANDGAP_FEATURE_HISTORY_BUFFER |
> + TI_BANDGAP_FEATURE_ERRATA_814,
> .fclock_name = "l3instr_ts_gclk_div",
> .div_ck_name = "l3instr_ts_gclk_div",
> .conv_table = dra752_adc_to_temp,
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> index 74c0e34..baf822e 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> @@ -119,6 +119,37 @@ exit:
> }
>
> /**
> + * ti_dra7_bandgap_read_temp() - helper function to read dra7 sensor temperature
> + * @bgp: pointer to ti_bandgap structure
> + * @reg: desired register (offset) to be read
> + *
> + * Function to read dra7 bandgap sensor temperature. This is done separately
> + * so as to workaround the errata "Bandgap Temperature read Dtemp can be
> + * corrupted" - Errata ID: i814".
> + * Read accesses to registers listed below can be corrupted due to incorrect
> + * resynchronization between clock domains.
> + * Read access to registers below can be corrupted :
> + * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
> + * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
> + *
> + * Return: the register value.
> + */
> +static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
> +{
> + u32 val1, val2;
> +
> + val1 = ti_bandgap_readl(bgp, reg);
> + val2 = ti_bandgap_readl(bgp, reg);
> +
> +/* If both times we read the same value then that is right */
> + if (val1 == val2)
> + return val1;
> +
> +/* if val1 and val2 are different read it third time */
> + return ti_bandgap_readl(bgp, reg);
> +}
> +
> +/**
> * ti_bandgap_read_temp() - helper function to read sensor temperature
> * @bgp: pointer to ti_bandgap structure
> * @id: bandgap sensor id
> @@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
> }
>
> /* read temperature */
> - temp = ti_bandgap_readl(bgp, reg);
> + if (TI_BANDGAP_HAS(bgp, ERRATA_814))
> + temp = ti_dra7_bandgap_read_temp(bgp, reg);
> + else
> + temp = ti_bandgap_readl(bgp, reg);
> +
> temp &= tsr->bgap_dtemp_mask;
>
> if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> index b3adf72..b2da3fc 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> @@ -318,6 +318,9 @@ struct ti_temp_sensor {
> * TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap device features
> * a history buffer of temperatures.
> *
> + * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the bandgap device
> + * has Errata 814
> + *
> * TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is capable of a
> * specific feature (above) or not. Return non-zero, if yes.
> */
> @@ -331,6 +334,7 @@ struct ti_temp_sensor {
> #define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
> #define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
> #define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
> +#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
> #define TI_BANDGAP_HAS(b, f) \
> ((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
2015-04-07 3:20 ` Keerthy
@ 2015-04-07 20:05 ` Eduardo Valentin
2015-04-08 4:24 ` Keerthy
0 siblings, 1 reply; 7+ messages in thread
From: Eduardo Valentin @ 2015-04-07 20:05 UTC (permalink / raw)
To: Keerthy; +Cc: Keerthy, rui.zhang, linux-pm, linux-omap, t-kristo
[-- Attachment #1: Type: text/plain, Size: 6568 bytes --]
Hello K,
On Tue, Apr 07, 2015 at 08:50:04AM +0530, Keerthy wrote:
> Hi Eduardo,
>
> On Friday 27 March 2015 02:15 PM, Keerthy wrote:
> >Bandgap Temperature read Dtemp can be corrupted
> >
> >DESCRIPTION
> > Read accesses to registers listed below can be corrupted due to incorrect resynchronization between
> > clock domains.
> >
> > Read access to registers below can be corrupted :
> > • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
> > • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
> >
> >WORKAROUND
> > Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
> > BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value:
> > 1. Perform two successive reads to BGAP_DTEMP bit field.
> > (a) If read1 returns Val1 and read2 returns Val1, then right value is Val1.
> > (b) If read1 returns Val1, read 2 returns Val2, a third read is needed.
> > 2. Perform third read
> > (a) If read3 returns Val2 then right value is Val2.
> > (b) If read3 returns Val3, then right value is Val3.
The third read description (b) is misleading/confusing. Does it mean
third read value is always correct or do we need to compare against val1
and val2? if val3 != val2 && val3 != val1, which one is correct? none?
> >
>
Thanks for the detailed errata description.
> A gentle ping on this.
>
Sorry for the late answer. (minor) Comment below:
> >Signed-off-by: Keerthy <j-keerthy@ti.com>
> >---
> >
> >Read all the temperatures from the 5 sensors and saw that
> >they were sane with time.
> >
> >Ran cpuloadgen and saw that temperatures rising on all the sensors
> >and cooled down as soon as the load was reduced.
> >
> > .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
> > drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37 +++++++++++++++++++++-
> > drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
> > 3 files changed, 42 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> >index a492927..58b5c66 100644
> >--- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> >+++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> >@@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
> > TI_BANDGAP_FEATURE_FREEZE_BIT |
> > TI_BANDGAP_FEATURE_TALERT |
> > TI_BANDGAP_FEATURE_COUNTER_DELAY |
> >- TI_BANDGAP_FEATURE_HISTORY_BUFFER,
> >+ TI_BANDGAP_FEATURE_HISTORY_BUFFER |
> >+ TI_BANDGAP_FEATURE_ERRATA_814,
> > .fclock_name = "l3instr_ts_gclk_div",
> > .div_ck_name = "l3instr_ts_gclk_div",
> > .conv_table = dra752_adc_to_temp,
> >diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> >index 74c0e34..baf822e 100644
> >--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> >+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> >@@ -119,6 +119,37 @@ exit:
> > }
> >
> > /**
> >+ * ti_dra7_bandgap_read_temp() - helper function to read dra7 sensor temperature
> >+ * @bgp: pointer to ti_bandgap structure
> >+ * @reg: desired register (offset) to be read
> >+ *
> >+ * Function to read dra7 bandgap sensor temperature. This is done separately
> >+ * so as to workaround the errata "Bandgap Temperature read Dtemp can be
> >+ * corrupted" - Errata ID: i814".
> >+ * Read accesses to registers listed below can be corrupted due to incorrect
> >+ * resynchronization between clock domains.
> >+ * Read access to registers below can be corrupted :
> >+ * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
> >+ * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
> >+ *
> >+ * Return: the register value.
> >+ */
> >+static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
Are you sure this bug affects only DRA7? I would prefer you name this
not specific to soc version. Maybe bind it to the errata:
+static u32 ti_i814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
> >+{
> >+ u32 val1, val2;
> >+
> >+ val1 = ti_bandgap_readl(bgp, reg);
> >+ val2 = ti_bandgap_readl(bgp, reg);
> >+
> >+/* If both times we read the same value then that is right */
Please, indent the comments too.
> >+ if (val1 == val2)
> >+ return val1;
> >+
> >+/* if val1 and val2 are different read it third time */
ditto.
> >+ return ti_bandgap_readl(bgp, reg);
Actually, if I understood the errata description correctly, you need to:
1. read a third time (val3)
2. compare val3 against val2, and return val2 if they are same
If they are different, the errata is not clear. Can you please clarify?
> >+}
> >+
> >+/**
> > * ti_bandgap_read_temp() - helper function to read sensor temperature
> > * @bgp: pointer to ti_bandgap structure
> > * @id: bandgap sensor id
> >@@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
> > }
> >
> > /* read temperature */
> >- temp = ti_bandgap_readl(bgp, reg);
> >+ if (TI_BANDGAP_HAS(bgp, ERRATA_814))
> >+ temp = ti_dra7_bandgap_read_temp(bgp, reg);
> >+ else
> >+ temp = ti_bandgap_readl(bgp, reg);
> >+
> > temp &= tsr->bgap_dtemp_mask;
> >
> > if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
> >diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> >index b3adf72..b2da3fc 100644
> >--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> >+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> >@@ -318,6 +318,9 @@ struct ti_temp_sensor {
> > * TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap device features
> > * a history buffer of temperatures.
> > *
> >+ * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the bandgap device
> >+ * has Errata 814
> >+ *
> > * TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is capable of a
> > * specific feature (above) or not. Return non-zero, if yes.
> > */
> >@@ -331,6 +334,7 @@ struct ti_temp_sensor {
> > #define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
> > #define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
> > #define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
> >+#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
> > #define TI_BANDGAP_HAS(b, f) \
> > ((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
> >
> >
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
2015-04-07 20:05 ` Eduardo Valentin
@ 2015-04-08 4:24 ` Keerthy
2015-04-10 5:32 ` Keerthy
0 siblings, 1 reply; 7+ messages in thread
From: Keerthy @ 2015-04-08 4:24 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: Keerthy, rui.zhang, linux-pm, linux-omap, t-kristo
Hi Eduardo,
Thanks for the review comments.
On Wednesday 08 April 2015 01:35 AM, Eduardo Valentin wrote:
> Hello K,
>
> On Tue, Apr 07, 2015 at 08:50:04AM +0530, Keerthy wrote:
>> Hi Eduardo,
>>
>> On Friday 27 March 2015 02:15 PM, Keerthy wrote:
>>> Bandgap Temperature read Dtemp can be corrupted
>>>
>>> DESCRIPTION
>>> Read accesses to registers listed below can be corrupted due to incorrect resynchronization between
>>> clock domains.
>>>
>>> Read access to registers below can be corrupted :
>>> • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
>>> • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
>>>
>>> WORKAROUND
>>> Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
>>> BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value:
>>> 1. Perform two successive reads to BGAP_DTEMP bit field.
>>> (a) If read1 returns Val1 and read2 returns Val1, then right value is Val1.
>>> (b) If read1 returns Val1, read 2 returns Val2, a third read is needed.
>>> 2. Perform third read
>>> (a) If read3 returns Val2 then right value is Val2.
>>> (b) If read3 returns Val3, then right value is Val3.
>
> The third read description (b) is misleading/confusing. Does it mean
> third read value is always correct or do we need to compare against val1
> and val2? if val3 != val2 && val3 != val1, which one is correct? none?
That is all is given in the errata description. Seems to assume third
one is right value always.
>
>>>
>>
>
> Thanks for the detailed errata description.
>
>> A gentle ping on this.
>>
>
> Sorry for the late answer. (minor) Comment below:
>
>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>> ---
>>>
>>> Read all the temperatures from the 5 sensors and saw that
>>> they were sane with time.
>>>
>>> Ran cpuloadgen and saw that temperatures rising on all the sensors
>>> and cooled down as soon as the load was reduced.
>>>
>>> .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
>>> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37 +++++++++++++++++++++-
>>> drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
>>> 3 files changed, 42 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>> index a492927..58b5c66 100644
>>> --- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>> +++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>> @@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
>>> TI_BANDGAP_FEATURE_FREEZE_BIT |
>>> TI_BANDGAP_FEATURE_TALERT |
>>> TI_BANDGAP_FEATURE_COUNTER_DELAY |
>>> - TI_BANDGAP_FEATURE_HISTORY_BUFFER,
>>> + TI_BANDGAP_FEATURE_HISTORY_BUFFER |
>>> + TI_BANDGAP_FEATURE_ERRATA_814,
>>> .fclock_name = "l3instr_ts_gclk_div",
>>> .div_ck_name = "l3instr_ts_gclk_div",
>>> .conv_table = dra752_adc_to_temp,
>>> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>> index 74c0e34..baf822e 100644
>>> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>> @@ -119,6 +119,37 @@ exit:
>>> }
>>>
>>> /**
>>> + * ti_dra7_bandgap_read_temp() - helper function to read dra7 sensor temperature
>>> + * @bgp: pointer to ti_bandgap structure
>>> + * @reg: desired register (offset) to be read
>>> + *
>>> + * Function to read dra7 bandgap sensor temperature. This is done separately
>>> + * so as to workaround the errata "Bandgap Temperature read Dtemp can be
>>> + * corrupted" - Errata ID: i814".
>>> + * Read accesses to registers listed below can be corrupted due to incorrect
>>> + * resynchronization between clock domains.
>>> + * Read access to registers below can be corrupted :
>>> + * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
>>> + * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
>>> + *
>>> + * Return: the register value.
>>> + */
>>> +static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
>
> Are you sure this bug affects only DRA7? I would prefer you name this
> not specific to soc version. Maybe bind it to the errata:
AFAIK it is impacting DRA7 not sure of OMAP5 and OMAP4. Okay I can name
it on the errata number. Yeah makes sense too. If other versions tend to
have this then we can just add this errata feature to them also.
>
> +static u32 ti_i814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
>
>>> +{
>>> + u32 val1, val2;
>>> +
>>> + val1 = ti_bandgap_readl(bgp, reg);
>>> + val2 = ti_bandgap_readl(bgp, reg);
>>> +
>>> +/* If both times we read the same value then that is right */
>
> Please, indent the comments too.
Okay.
>
>>> + if (val1 == val2)
>>> + return val1;
>>> +
>>> +/* if val1 and val2 are different read it third time */
>
> ditto.
Okay
>
>>> + return ti_bandgap_readl(bgp, reg);
>
> Actually, if I understood the errata description correctly, you need to:
> 1. read a third time (val3)
> 2. compare val3 against val2, and return val2 if they are same
>
> If they are different, the errata is not clear. Can you please clarify?
2. Perform third read
(a) If read3 returns Val2 then right value is Val2.
(b) If read3 returns Val3, then right value is Val3.
Which means errata assumes third value is the good value no matter what.
>
>>> +}
>>> +
>>> +/**
>>> * ti_bandgap_read_temp() - helper function to read sensor temperature
>>> * @bgp: pointer to ti_bandgap structure
>>> * @id: bandgap sensor id
>>> @@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
>>> }
>>>
>>> /* read temperature */
>>> - temp = ti_bandgap_readl(bgp, reg);
>>> + if (TI_BANDGAP_HAS(bgp, ERRATA_814))
>>> + temp = ti_dra7_bandgap_read_temp(bgp, reg);
>>> + else
>>> + temp = ti_bandgap_readl(bgp, reg);
>>> +
>>> temp &= tsr->bgap_dtemp_mask;
>>>
>>> if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
>>> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>> index b3adf72..b2da3fc 100644
>>> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>> @@ -318,6 +318,9 @@ struct ti_temp_sensor {
>>> * TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap device features
>>> * a history buffer of temperatures.
>>> *
>>> + * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the bandgap device
>>> + * has Errata 814
>>> + *
>>> * TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is capable of a
>>> * specific feature (above) or not. Return non-zero, if yes.
>>> */
>>> @@ -331,6 +334,7 @@ struct ti_temp_sensor {
>>> #define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
>>> #define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
>>> #define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
>>> +#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
>>> #define TI_BANDGAP_HAS(b, f) \
>>> ((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
>>>
>>>
-Keerthy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
2015-04-08 4:24 ` Keerthy
@ 2015-04-10 5:32 ` Keerthy
0 siblings, 0 replies; 7+ messages in thread
From: Keerthy @ 2015-04-10 5:32 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: Keerthy, rui.zhang, linux-pm, linux-omap, t-kristo
On Wednesday 08 April 2015 09:54 AM, Keerthy wrote:
> Hi Eduardo,
>
> Thanks for the review comments.
>
> On Wednesday 08 April 2015 01:35 AM, Eduardo Valentin wrote:
>> Hello K,
>>
>> On Tue, Apr 07, 2015 at 08:50:04AM +0530, Keerthy wrote:
>>> Hi Eduardo,
>>>
>>> On Friday 27 March 2015 02:15 PM, Keerthy wrote:
>>>> Bandgap Temperature read Dtemp can be corrupted
>>>>
>>>> DESCRIPTION
>>>> Read accesses to registers listed below can be corrupted
>>>> due to incorrect resynchronization between
>>>> clock domains.
>>>>
>>>> Read access to registers below can be corrupted :
>>>> • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0
>>>> to 4)
>>>> • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
>>>>
>>>> WORKAROUND
>>>> Multiple reads to
>>>> CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]:
>>>> BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard
>>>> false value and read right value:
>>>> 1. Perform two successive reads to BGAP_DTEMP bit
>>>> field.
>>>> (a) If read1 returns Val1 and read2 returns
>>>> Val1, then right value is Val1.
>>>> (b) If read1 returns Val1, read 2 returns
>>>> Val2, a third read is needed.
>>>> 2. Perform third read
>>>> (a) If read3 returns Val2 then right value
>>>> is Val2.
>>>> (b) If read3 returns Val3, then right value
>>>> is Val3.
>>
>> The third read description (b) is misleading/confusing. Does it mean
>> third read value is always correct or do we need to compare against val1
>> and val2? if val3 != val2 && val3 != val1, which one is correct? none?
>
> That is all is given in the errata description. Seems to assume third
> one is right value always.
Got a confirmation from the designer that synchronization shall be
complete by third read hence it is the right value irrespective of the
first and second..
>
>>
>>>>
>>>
>>
>> Thanks for the detailed errata description.
>>
>>> A gentle ping on this.
>>>
>>
>> Sorry for the late answer. (minor) Comment below:
>>
>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>> ---
>>>>
>>>> Read all the temperatures from the 5 sensors and saw that
>>>> they were sane with time.
>>>>
>>>> Ran cpuloadgen and saw that temperatures rising on all the sensors
>>>> and cooled down as soon as the load was reduced.
>>>>
>>>> .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +-
>>>> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 37
>>>> +++++++++++++++++++++-
>>>> drivers/thermal/ti-soc-thermal/ti-bandgap.h | 4 +++
>>>> 3 files changed, 42 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>>> b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>>> index a492927..58b5c66 100644
>>>> --- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>>> +++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
>>>> @@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = {
>>>> TI_BANDGAP_FEATURE_FREEZE_BIT |
>>>> TI_BANDGAP_FEATURE_TALERT |
>>>> TI_BANDGAP_FEATURE_COUNTER_DELAY |
>>>> - TI_BANDGAP_FEATURE_HISTORY_BUFFER,
>>>> + TI_BANDGAP_FEATURE_HISTORY_BUFFER |
>>>> + TI_BANDGAP_FEATURE_ERRATA_814,
>>>> .fclock_name = "l3instr_ts_gclk_div",
>>>> .div_ck_name = "l3instr_ts_gclk_div",
>>>> .conv_table = dra752_adc_to_temp,
>>>> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>>> b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>>> index 74c0e34..baf822e 100644
>>>> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>>> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
>>>> @@ -119,6 +119,37 @@ exit:
>>>> }
>>>>
>>>> /**
>>>> + * ti_dra7_bandgap_read_temp() - helper function to read dra7
>>>> sensor temperature
>>>> + * @bgp: pointer to ti_bandgap structure
>>>> + * @reg: desired register (offset) to be read
>>>> + *
>>>> + * Function to read dra7 bandgap sensor temperature. This is done
>>>> separately
>>>> + * so as to workaround the errata "Bandgap Temperature read Dtemp
>>>> can be
>>>> + * corrupted" - Errata ID: i814".
>>>> + * Read accesses to registers listed below can be corrupted due to
>>>> incorrect
>>>> + * resynchronization between clock domains.
>>>> + * Read access to registers below can be corrupted :
>>>> + * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4)
>>>> + * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n
>>>> + *
>>>> + * Return: the register value.
>>>> + */
>>>> +static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
>>
>> Are you sure this bug affects only DRA7? I would prefer you name this
>> not specific to soc version. Maybe bind it to the errata:
>
> AFAIK it is impacting DRA7 not sure of OMAP5 and OMAP4. Okay I can name
> it on the errata number. Yeah makes sense too. If other versions tend to
> have this then we can just add this errata feature to them also.
>
>>
>> +static u32 ti_i814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg)
>>
>>>> +{
>>>> + u32 val1, val2;
>>>> +
>>>> + val1 = ti_bandgap_readl(bgp, reg);
>>>> + val2 = ti_bandgap_readl(bgp, reg);
>>>> +
>>>> +/* If both times we read the same value then that is right */
>>
>> Please, indent the comments too.
>
> Okay.
>
>>
>>>> + if (val1 == val2)
>>>> + return val1;
>>>> +
>>>> +/* if val1 and val2 are different read it third time */
>>
>> ditto.
>
> Okay
>
>>
>>>> + return ti_bandgap_readl(bgp, reg);
>>
>> Actually, if I understood the errata description correctly, you need to:
>> 1. read a third time (val3)
>> 2. compare val3 against val2, and return val2 if they are same
>>
>> If they are different, the errata is not clear. Can you please clarify?
>
> 2. Perform third read
> (a) If read3 returns Val2 then right value is Val2.
> (b) If read3 returns Val3, then right value is Val3.
>
> Which means errata assumes third value is the good value no matter what.
>
>>
>>>> +}
>>>> +
>>>> +/**
>>>> * ti_bandgap_read_temp() - helper function to read sensor
>>>> temperature
>>>> * @bgp: pointer to ti_bandgap structure
>>>> * @id: bandgap sensor id
>>>> @@ -148,7 +179,11 @@ static u32 ti_bandgap_read_temp(struct
>>>> ti_bandgap *bgp, int id)
>>>> }
>>>>
>>>> /* read temperature */
>>>> - temp = ti_bandgap_readl(bgp, reg);
>>>> + if (TI_BANDGAP_HAS(bgp, ERRATA_814))
>>>> + temp = ti_dra7_bandgap_read_temp(bgp, reg);
>>>> + else
>>>> + temp = ti_bandgap_readl(bgp, reg);
>>>> +
>>>> temp &= tsr->bgap_dtemp_mask;
>>>>
>>>> if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
>>>> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>>> b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>>> index b3adf72..b2da3fc 100644
>>>> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>>> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
>>>> @@ -318,6 +318,9 @@ struct ti_temp_sensor {
>>>> * TI_BANDGAP_FEATURE_HISTORY_BUFFER - used when the bandgap
>>>> device features
>>>> * a history buffer of temperatures.
>>>> *
>>>> + * TI_BANDGAP_FEATURE_ERRATA_814 - used to workaorund when the
>>>> bandgap device
>>>> + * has Errata 814
>>>> + *
>>>> * TI_BANDGAP_HAS(b, f) - macro to check if a bandgap device is
>>>> capable of a
>>>> * specific feature (above) or not. Return non-zero, if yes.
>>>> */
>>>> @@ -331,6 +334,7 @@ struct ti_temp_sensor {
>>>> #define TI_BANDGAP_FEATURE_FREEZE_BIT BIT(7)
>>>> #define TI_BANDGAP_FEATURE_COUNTER_DELAY BIT(8)
>>>> #define TI_BANDGAP_FEATURE_HISTORY_BUFFER BIT(9)
>>>> +#define TI_BANDGAP_FEATURE_ERRATA_814 BIT(10)
>>>> #define TI_BANDGAP_HAS(b, f) \
>>>> ((b)->conf->features & TI_BANDGAP_FEATURE_ ## f)
>>>>
>>>>
>
> -Keerthy
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-16 7:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-10 6:36 [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted Keerthy
2015-04-16 7:49 ` Keerthy
-- strict thread matches above, loose matches on Subject: below --
2015-03-27 8:45 Keerthy
2015-04-07 3:20 ` Keerthy
2015-04-07 20:05 ` Eduardo Valentin
2015-04-08 4:24 ` Keerthy
2015-04-10 5:32 ` Keerthy
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.