The goal of this patchset is to improve stm32 thermal driver:
* add support for multiple trip points. Currently the driver supports only
2 trip points.
* rework interrupt management to avoid receiving hundreds of
interrupts when the temperature is close to a low threshold.
* fix a mistake regarding the role of an engineering value.
* suppress passive trip point on stm32mp157c because it is useless.
Pascal Paillet (4):
thermal: stm32: implement set_trips callback
thermal: stm32: fix IRQ flood on low threshold
thermal: stm32: fix engineering calibration value
ARM: dts: stm32: remove thermal passive trip point on stm32mp157c
arch/arm/boot/dts/stm32mp157c.dtsi | 6 -
drivers/thermal/st/stm_thermal.c | 441 +++++++++--------------------
2 files changed, 138 insertions(+), 309 deletions(-)
--
2.17.1
Implement set_trips() so that the trip points are managed by the thermal
framework. The user is free to define any trip points he needs.
Simplify interrupt handling.
Signed-off-by: Pascal Paillet <[email protected]>
Change-Id: I2929d4fa5b4c5dca45cec7cb3b93fffc277394d2
---
drivers/thermal/st/stm_thermal.c | 370 +++++++++++--------------------
1 file changed, 125 insertions(+), 245 deletions(-)
diff --git a/drivers/thermal/st/stm_thermal.c b/drivers/thermal/st/stm_thermal.c
index cf9ddc52f30e..0aae1cc14235 100644
--- a/drivers/thermal/st/stm_thermal.c
+++ b/drivers/thermal/st/stm_thermal.c
@@ -30,7 +30,7 @@
#define DTS_DR_OFFSET 0x1C
#define DTS_SR_OFFSET 0x20
#define DTS_ITENR_OFFSET 0x24
-#define DTS_CIFR_OFFSET 0x28
+#define DTS_ICIFR_OFFSET 0x28
/* DTS_CFGR1 register mask definitions */
#define HSREF_CLK_DIV_MASK GENMASK(30, 24)
@@ -51,10 +51,17 @@
/* DTS_DR register mask definitions */
#define TS1_MFREQ_MASK GENMASK(15, 0)
+/* DTS_ITENR register mask definitions */
+#define ITENR_MASK (GENMASK(2, 0) | GENMASK(6, 4))
+
+/* DTS_ICIFR register mask definitions */
+#define ICIFR_MASK (GENMASK(2, 0) | GENMASK(6, 4))
+
/* Less significant bit position definitions */
#define TS1_T0_POS 16
#define TS1_SMP_TIME_POS 16
#define TS1_HITTHD_POS 16
+#define TS1_LITTHD_POS 0
#define HSREF_CLK_DIV_POS 24
/* DTS_CFGR1 bit definitions */
@@ -91,44 +98,59 @@ struct stm_thermal_sensor {
struct thermal_zone_device *th_dev;
enum thermal_device_mode mode;
struct clk *clk;
- int high_temp;
- int low_temp;
- int temp_critical;
- int temp_passive;
- unsigned int low_temp_enabled;
- int num_trips;
int irq;
- unsigned int irq_enabled;
void __iomem *base;
int t0, fmt0, ramp_coeff;
+ int low_en, high_en;
};
-static irqreturn_t stm_thermal_alarm_irq(int irq, void *sdata)
+static void stm_thermal_disable_irq(struct stm_thermal_sensor *sensor)
{
- struct stm_thermal_sensor *sensor = sdata;
+ u32 itenr;
+
+ /* Disable IT generation */
+ itenr = readl_relaxed(sensor->base + DTS_ITENR_OFFSET);
+ itenr &= ~ITENR_MASK;
+ writel_relaxed(itenr, sensor->base + DTS_ITENR_OFFSET);
+}
+
+static void stm_thermal_set_irq_state(struct stm_thermal_sensor *sensor)
+{
+ u32 itenr;
+
+ dev_dbg(sensor->dev, "low:%d high:%d\n", sensor->low_en,
+ sensor->high_en);
+
+ /* Disable IT generation for low and high thresholds */
+ itenr = readl_relaxed(sensor->base + DTS_ITENR_OFFSET);
+ itenr &= ~(LOW_THRESHOLD | HIGH_THRESHOLD);
+
+ if (sensor->low_en)
+ itenr |= HIGH_THRESHOLD;
- disable_irq_nosync(irq);
- sensor->irq_enabled = false;
+ if (sensor->high_en)
+ itenr |= LOW_THRESHOLD;
- return IRQ_WAKE_THREAD;
+ /* Enable interrupts */
+ writel_relaxed(itenr, sensor->base + DTS_ITENR_OFFSET);
}
static irqreturn_t stm_thermal_alarm_irq_thread(int irq, void *sdata)
{
- u32 value;
struct stm_thermal_sensor *sensor = sdata;
- /* read IT reason in SR and clear flags */
- value = readl_relaxed(sensor->base + DTS_SR_OFFSET);
-
- if ((value & LOW_THRESHOLD) == LOW_THRESHOLD)
- writel_relaxed(LOW_THRESHOLD, sensor->base + DTS_CIFR_OFFSET);
+ dev_dbg(sensor->dev, "sr:%d\n",
+ readl_relaxed(sensor->base + DTS_SR_OFFSET));
- if ((value & HIGH_THRESHOLD) == HIGH_THRESHOLD)
- writel_relaxed(HIGH_THRESHOLD, sensor->base + DTS_CIFR_OFFSET);
+ stm_thermal_disable_irq(sensor);
thermal_zone_device_update(sensor->th_dev, THERMAL_EVENT_UNSPECIFIED);
+ stm_thermal_set_irq_state(sensor);
+
+ /* Acknoledge all DTS irqs */
+ writel_relaxed(ICIFR_MASK, sensor->base + DTS_ICIFR_OFFSET);
+
return IRQ_HANDLED;
}
@@ -160,6 +182,8 @@ static int stm_sensor_power_on(struct stm_thermal_sensor *sensor)
writel_relaxed(value, sensor->base +
DTS_CFGR1_OFFSET);
+ sensor->mode = THERMAL_DEVICE_ENABLED;
+
return 0;
}
@@ -167,6 +191,8 @@ static int stm_sensor_power_off(struct stm_thermal_sensor *sensor)
{
u32 value;
+ sensor->mode = THERMAL_DEVICE_DISABLED;
+
/* Stop measuring */
value = readl_relaxed(sensor->base + DTS_CFGR1_OFFSET);
value &= ~TS1_START;
@@ -285,118 +311,7 @@ static int stm_thermal_calculate_threshold(struct stm_thermal_sensor *sensor,
*th *= sampling_time;
- return 0;
-}
-
-static int stm_thermal_set_threshold(struct stm_thermal_sensor *sensor)
-{
- u32 value, th;
- int ret;
-
- value = readl_relaxed(sensor->base + DTS_ITR1_OFFSET);
-
- /* Erase threshold content */
- value &= ~(TS1_LITTHD_MASK | TS1_HITTHD_MASK);
-
- /* Retrieve the sample threshold number th for a given temperature */
- ret = stm_thermal_calculate_threshold(sensor, sensor->high_temp, &th);
- if (ret)
- return ret;
-
- value |= th & TS1_LITTHD_MASK;
-
- if (sensor->low_temp_enabled) {
- /* Retrieve the sample threshold */
- ret = stm_thermal_calculate_threshold(sensor, sensor->low_temp,
- &th);
- if (ret)
- return ret;
-
- value |= (TS1_HITTHD_MASK & (th << TS1_HITTHD_POS));
- }
-
- /* Write value on the Low interrupt threshold */
- writel_relaxed(value, sensor->base + DTS_ITR1_OFFSET);
-
- return 0;
-}
-
-/* Disable temperature interrupt */
-static int stm_disable_irq(struct stm_thermal_sensor *sensor)
-{
- u32 value;
-
- /* Disable IT generation for low and high thresholds */
- value = readl_relaxed(sensor->base + DTS_ITENR_OFFSET);
- writel_relaxed(value & ~(LOW_THRESHOLD | HIGH_THRESHOLD),
- sensor->base + DTS_ITENR_OFFSET);
-
- dev_dbg(sensor->dev, "%s: IT disabled on sensor side", __func__);
-
- return 0;
-}
-
-/* Enable temperature interrupt */
-static int stm_enable_irq(struct stm_thermal_sensor *sensor)
-{
- u32 value;
-
- /*
- * Code below enables High temperature threshold using a low threshold
- * sampling value
- */
-
- /* Make sure LOW_THRESHOLD IT is clear before enabling */
- writel_relaxed(LOW_THRESHOLD, sensor->base + DTS_CIFR_OFFSET);
-
- /* Enable IT generation for low threshold */
- value = readl_relaxed(sensor->base + DTS_ITENR_OFFSET);
- value |= LOW_THRESHOLD;
-
- /* Enable the low temperature threshold if needed */
- if (sensor->low_temp_enabled) {
- /* Make sure HIGH_THRESHOLD IT is clear before enabling */
- writel_relaxed(HIGH_THRESHOLD, sensor->base + DTS_CIFR_OFFSET);
-
- /* Enable IT generation for high threshold */
- value |= HIGH_THRESHOLD;
- }
-
- /* Enable thresholds */
- writel_relaxed(value, sensor->base + DTS_ITENR_OFFSET);
-
- dev_dbg(sensor->dev, "%s: IT enabled on sensor side", __func__);
-
- return 0;
-}
-
-static int stm_thermal_update_threshold(struct stm_thermal_sensor *sensor)
-{
- int ret;
-
- sensor->mode = THERMAL_DEVICE_DISABLED;
-
- ret = stm_sensor_power_off(sensor);
- if (ret)
- return ret;
-
- ret = stm_disable_irq(sensor);
- if (ret)
- return ret;
-
- ret = stm_thermal_set_threshold(sensor);
- if (ret)
- return ret;
-
- ret = stm_enable_irq(sensor);
- if (ret)
- return ret;
-
- ret = stm_sensor_power_on(sensor);
- if (ret)
- return ret;
-
- sensor->mode = THERMAL_DEVICE_ENABLED;
+ dev_dbg(sensor->dev, "freqM=%d Hz, threshold=0x%x", freqM, *th);
return 0;
}
@@ -440,42 +355,54 @@ static int stm_thermal_get_temp(void *data, int *temp)
*temp = mcelsius(sensor->t0 + ((freqM - sensor->fmt0) /
sensor->ramp_coeff));
- dev_dbg(sensor->dev, "%s: temperature = %d millicelsius",
- __func__, *temp);
-
- /* Update thresholds */
- if (sensor->num_trips > 1) {
- /* Update alarm threshold value to next higher trip point */
- if (sensor->high_temp == sensor->temp_passive &&
- celsius(*temp) >= sensor->temp_passive) {
- sensor->high_temp = sensor->temp_critical;
- sensor->low_temp = sensor->temp_passive;
- sensor->low_temp_enabled = true;
- ret = stm_thermal_update_threshold(sensor);
- if (ret)
- return ret;
- }
-
- if (sensor->high_temp == sensor->temp_critical &&
- celsius(*temp) < sensor->temp_passive) {
- sensor->high_temp = sensor->temp_passive;
- sensor->low_temp_enabled = false;
- ret = stm_thermal_update_threshold(sensor);
- if (ret)
- return ret;
- }
-
- /*
- * Re-enable alarm IRQ if temperature below critical
- * temperature
- */
- if (!sensor->irq_enabled &&
- (celsius(*temp) < sensor->temp_critical)) {
- sensor->irq_enabled = true;
- enable_irq(sensor->irq);
- }
+ dev_dbg(sensor->dev, "temperature = %d millicelsius", *temp);
+
+ return 0;
+}
+
+static int stm_thermal_set_trips(void *data, int low, int high)
+{
+ struct stm_thermal_sensor *sensor = data;
+ u32 itr1, th;
+ int ret;
+
+ dev_dbg(sensor->dev, "set trips %d <--> %d\n", low, high);
+
+ /* Erase threshold content */
+ itr1 = readl_relaxed(sensor->base + DTS_ITR1_OFFSET);
+ itr1 &= ~(TS1_LITTHD_MASK | TS1_HITTHD_MASK);
+
+ /*
+ * Disable low-temp if "low" is too small. As per thermal framework
+ * API, we use -INT_MAX rather than INT_MIN.
+ */
+
+ if (low > -INT_MAX) {
+ sensor->low_en = 1;
+ ret = stm_thermal_calculate_threshold(sensor, low, &th);
+ if (ret)
+ return ret;
+
+ itr1 |= (TS1_HITTHD_MASK & (th << TS1_HITTHD_POS));
+ } else {
+ sensor->low_en = 0;
}
+ /* Disable high-temp if "high" is too big. */
+ if (high < INT_MAX) {
+ sensor->high_en = 1;
+ ret = stm_thermal_calculate_threshold(sensor, high, &th);
+ if (ret)
+ return ret;
+
+ itr1 |= (TS1_LITTHD_MASK & (th << TS1_LITTHD_POS));
+ } else {
+ sensor->high_en = 0;
+ }
+
+ /* Write new threshod values*/
+ writel_relaxed(itr1, sensor->base + DTS_ITR1_OFFSET);
+
return 0;
}
@@ -493,8 +420,7 @@ static int stm_register_irq(struct stm_thermal_sensor *sensor)
}
ret = devm_request_threaded_irq(dev, sensor->irq,
- stm_thermal_alarm_irq,
- stm_thermal_alarm_irq_thread,
+ NULL, stm_thermal_alarm_irq_thread,
IRQF_ONESHOT,
dev->driver->name, sensor);
if (ret) {
@@ -503,8 +429,6 @@ static int stm_register_irq(struct stm_thermal_sensor *sensor)
return ret;
}
- sensor->irq_enabled = true;
-
dev_dbg(dev, "%s: thermal IRQ registered", __func__);
return 0;
@@ -514,6 +438,8 @@ static int stm_thermal_sensor_off(struct stm_thermal_sensor *sensor)
{
int ret;
+ stm_thermal_disable_irq(sensor);
+
ret = stm_sensor_power_off(sensor);
if (ret)
return ret;
@@ -526,7 +452,6 @@ static int stm_thermal_sensor_off(struct stm_thermal_sensor *sensor)
static int stm_thermal_prepare(struct stm_thermal_sensor *sensor)
{
int ret;
- struct device *dev = sensor->dev;
ret = clk_prepare_enable(sensor->clk);
if (ret)
@@ -540,26 +465,8 @@ static int stm_thermal_prepare(struct stm_thermal_sensor *sensor)
if (ret)
goto thermal_unprepare;
- /* Set threshold(s) for IRQ */
- ret = stm_thermal_set_threshold(sensor);
- if (ret)
- goto thermal_unprepare;
-
- ret = stm_enable_irq(sensor);
- if (ret)
- goto thermal_unprepare;
-
- ret = stm_sensor_power_on(sensor);
- if (ret) {
- dev_err(dev, "%s: failed to power on sensor\n", __func__);
- goto irq_disable;
- }
-
return 0;
-irq_disable:
- stm_disable_irq(sensor);
-
thermal_unprepare:
clk_disable_unprepare(sensor->clk);
@@ -576,8 +483,6 @@ static int stm_thermal_suspend(struct device *dev)
if (ret)
return ret;
- sensor->mode = THERMAL_DEVICE_DISABLED;
-
return 0;
}
@@ -590,7 +495,12 @@ static int stm_thermal_resume(struct device *dev)
if (ret)
return ret;
- sensor->mode = THERMAL_DEVICE_ENABLED;
+ ret = stm_sensor_power_on(sensor);
+ if (ret)
+ return ret;
+
+ thermal_zone_device_update(sensor->th_dev, THERMAL_EVENT_UNSPECIFIED);
+ stm_thermal_set_irq_state(sensor);
return 0;
}
@@ -600,6 +510,7 @@ SIMPLE_DEV_PM_OPS(stm_thermal_pm_ops, stm_thermal_suspend, stm_thermal_resume);
static const struct thermal_zone_of_device_ops stm_tz_ops = {
.get_temp = stm_thermal_get_temp,
+ .set_trips = stm_thermal_set_trips,
};
static const struct of_device_id stm_thermal_of_match[] = {
@@ -612,9 +523,8 @@ static int stm_thermal_probe(struct platform_device *pdev)
{
struct stm_thermal_sensor *sensor;
struct resource *res;
- const struct thermal_trip *trip;
void __iomem *base;
- int ret, i;
+ int ret;
if (!pdev->dev.of_node) {
dev_err(&pdev->dev, "%s: device tree node not found\n",
@@ -645,10 +555,23 @@ static int stm_thermal_probe(struct platform_device *pdev)
return PTR_ERR(sensor->clk);
}
- /* Register IRQ into GIC */
- ret = stm_register_irq(sensor);
- if (ret)
+ stm_thermal_disable_irq(sensor);
+
+ /* Clear irq flags */
+ writel_relaxed(ICIFR_MASK, sensor->base + DTS_ICIFR_OFFSET);
+
+ /* Configure and enable HW sensor */
+ ret = stm_thermal_prepare(sensor);
+ if (ret) {
+ dev_err(&pdev->dev, "Error preprare sensor: %d\n", ret);
return ret;
+ }
+
+ ret = stm_sensor_power_on(sensor);
+ if (ret) {
+ dev_err(&pdev->dev, "Error power on sensor: %d\n", ret);
+ return ret;
+ }
sensor->th_dev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
sensor,
@@ -661,53 +584,12 @@ static int stm_thermal_probe(struct platform_device *pdev)
return ret;
}
- if (!sensor->th_dev->ops->get_crit_temp) {
- /* Critical point must be provided */
- ret = -EINVAL;
- goto err_tz;
- }
-
- ret = sensor->th_dev->ops->get_crit_temp(sensor->th_dev,
- &sensor->temp_critical);
- if (ret) {
- dev_err(&pdev->dev,
- "Not able to read critical_temp: %d\n", ret);
+ /* Register IRQ into GIC */
+ ret = stm_register_irq(sensor);
+ if (ret)
goto err_tz;
- }
- sensor->temp_critical = celsius(sensor->temp_critical);
-
- /* Set thresholds for IRQ */
- sensor->high_temp = sensor->temp_critical;
-
- trip = of_thermal_get_trip_points(sensor->th_dev);
- sensor->num_trips = of_thermal_get_ntrips(sensor->th_dev);
-
- /* Find out passive temperature if it exists */
- for (i = (sensor->num_trips - 1); i >= 0; i--) {
- if (trip[i].type == THERMAL_TRIP_PASSIVE) {
- sensor->temp_passive = celsius(trip[i].temperature);
- /* Update high temperature threshold */
- sensor->high_temp = sensor->temp_passive;
- }
- }
-
- /*
- * Ensure low_temp_enabled flag is disabled.
- * By disabling low_temp_enabled, low threshold IT will not be
- * configured neither enabled because it is not needed as high
- * threshold is set on the lowest temperature trip point after
- * probe.
- */
- sensor->low_temp_enabled = false;
-
- /* Configure and enable HW sensor */
- ret = stm_thermal_prepare(sensor);
- if (ret) {
- dev_err(&pdev->dev,
- "Not able to enable sensor: %d\n", ret);
- goto err_tz;
- }
+ stm_thermal_set_irq_state(sensor);
/*
* Thermal_zone doesn't enable hwmon as default,
@@ -718,8 +600,6 @@ static int stm_thermal_probe(struct platform_device *pdev)
if (ret)
goto err_tz;
- sensor->mode = THERMAL_DEVICE_ENABLED;
-
dev_info(&pdev->dev, "%s: Driver initialized successfully\n",
__func__);
--
2.17.1
Remove thermal passive trip point.
Signed-off-by: Pascal Paillet <[email protected]>
Change-Id: I494313cf467eea491236e73bd2fbe1803345586f
---
arch/arm/boot/dts/stm32mp157c.dtsi | 6 ------
1 file changed, 6 deletions(-)
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 9b11654a0a39..799b2aedd2c9 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -91,12 +91,6 @@
thermal-sensors = <&dts>;
trips {
- cpu_alert1: cpu-alert1 {
- temperature = <85000>;
- hysteresis = <0>;
- type = "passive";
- };
-
cpu-crit {
temperature = <120000>;
hysteresis = <0>;
--
2.17.1
TS1_T0 is only used by engineering during calibration.
The temperature offset is actually fixed to 30 Celsius.
Signed-off-by: Pascal Paillet <[email protected]>
Change-Id: I83abea65ff3f58f8954256495f2d5d38f0d868d9
---
drivers/thermal/st/stm_thermal.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/thermal/st/stm_thermal.c b/drivers/thermal/st/stm_thermal.c
index 4bc1bbece0de..37abbdffe573 100644
--- a/drivers/thermal/st/stm_thermal.c
+++ b/drivers/thermal/st/stm_thermal.c
@@ -82,8 +82,7 @@
#define ONE_MHZ 1000000
#define POLL_TIMEOUT 5000
#define STARTUP_TIME 40
-#define TS1_T0_VAL0 30000 /* 30 celsius */
-#define TS1_T0_VAL1 130000 /* 130 celsius */
+#define T0 30000 /* 30 celsius */
#define NO_HW_TRIG 0
#define SAMPLING_TIME 15
@@ -94,7 +93,7 @@ struct stm_thermal_sensor {
struct clk *clk;
int irq;
void __iomem *base;
- int t0, fmt0, ramp_coeff;
+ int fmt0, ramp_coeff;
int low_en, high_en;
};
@@ -248,14 +247,6 @@ static int stm_thermal_calibration(struct stm_thermal_sensor *sensor)
/* Fill in DTS structure with factory sensor values */
static int stm_thermal_read_factory_settings(struct stm_thermal_sensor *sensor)
{
- /* Retrieve engineering calibration temperature */
- sensor->t0 = readl_relaxed(sensor->base + DTS_T0VALR1_OFFSET) &
- TS1_T0_MASK;
- if (!sensor->t0)
- sensor->t0 = TS1_T0_VAL0;
- else
- sensor->t0 = TS1_T0_VAL1;
-
/* Retrieve fmt0 and put it on Hz */
sensor->fmt0 = ADJUST * (readl_relaxed(sensor->base +
DTS_T0VALR1_OFFSET) & TS1_FMT0_MASK);
@@ -269,8 +260,8 @@ static int stm_thermal_read_factory_settings(struct stm_thermal_sensor *sensor)
return -EINVAL;
}
- dev_dbg(sensor->dev, "%s: T0 = %doC, FMT0 = %dHz, RAMP_COEFF = %dHz/oC",
- __func__, sensor->t0, sensor->fmt0, sensor->ramp_coeff);
+ dev_dbg(sensor->dev, "%s: FMT0 = %dHz, RAMP_COEFF = %dHz/oC",
+ __func__, sensor->fmt0, sensor->ramp_coeff);
return 0;
}
@@ -281,8 +272,7 @@ static int stm_thermal_calculate_threshold(struct stm_thermal_sensor *sensor,
int freqM;
/* Figure out the CLK_PTAT frequency for a given temperature */
- freqM = ((temp - sensor->t0) * sensor->ramp_coeff) / 1000 +
- sensor->fmt0;
+ freqM = ((temp - T0) * sensor->ramp_coeff) / 1000 + sensor->fmt0;
/* Figure out the threshold sample number */
*th = clk_get_rate(sensor->clk) * SAMPLING_TIME / freqM;
@@ -317,7 +307,7 @@ static int stm_thermal_get_temp(void *data, int *temp)
return -EINVAL;
/* Figure out the temperature in mili celsius */
- *temp = (freqM - sensor->fmt0) * 1000 / sensor->ramp_coeff + sensor->t0;
+ *temp = (freqM - sensor->fmt0) * 1000 / sensor->ramp_coeff + T0;
dev_dbg(sensor->dev, "periods=0x%x t=%d mC", periods, *temp);
--
2.17.1
Fix IRQ flood on low threshold by too ways:
- improve temperature reading resolution,
- add an hysteresis to the low threshold: on low threshold interrupt,
it is not possible to get the temperature value that has fired the
interrupt. The time to acquire a new value is enough for the CPU to
become hotter than the current low threshold.
Signed-off-by: Pascal Paillet <[email protected]>
Change-Id: I3b63b8aab38fd651a165c4e69a2d090b3c6f5db3
---
drivers/thermal/st/stm_thermal.c | 73 +++++++++-----------------------
1 file changed, 19 insertions(+), 54 deletions(-)
diff --git a/drivers/thermal/st/stm_thermal.c b/drivers/thermal/st/stm_thermal.c
index 0aae1cc14235..4bc1bbece0de 100644
--- a/drivers/thermal/st/stm_thermal.c
+++ b/drivers/thermal/st/stm_thermal.c
@@ -59,7 +59,6 @@
/* Less significant bit position definitions */
#define TS1_T0_POS 16
-#define TS1_SMP_TIME_POS 16
#define TS1_HITTHD_POS 16
#define TS1_LITTHD_POS 0
#define HSREF_CLK_DIV_POS 24
@@ -83,15 +82,10 @@
#define ONE_MHZ 1000000
#define POLL_TIMEOUT 5000
#define STARTUP_TIME 40
-#define TS1_T0_VAL0 30
-#define TS1_T0_VAL1 130
+#define TS1_T0_VAL0 30000 /* 30 celsius */
+#define TS1_T0_VAL1 130000 /* 130 celsius */
#define NO_HW_TRIG 0
-
-/* The Thermal Framework expects millidegrees */
-#define mcelsius(temp) ((temp) * 1000)
-
-/* The Sensor expects oC degrees */
-#define celsius(temp) ((temp) / 1000)
+#define SAMPLING_TIME 15
struct stm_thermal_sensor {
struct device *dev;
@@ -222,12 +216,8 @@ static int stm_thermal_calibration(struct stm_thermal_sensor *sensor)
if (!clk_freq)
return -EINVAL;
- prescaler = 0;
- clk_freq /= ONE_MHZ;
- if (clk_freq) {
- while (prescaler <= clk_freq)
- prescaler++;
- }
+ /* calculate divider for maximum 1MHz PCLK */
+ prescaler = clk_freq / ONE_MHZ + 1;
value = readl_relaxed(sensor->base + DTS_CFGR1_OFFSET);
@@ -235,7 +225,7 @@ static int stm_thermal_calibration(struct stm_thermal_sensor *sensor)
value &= ~HSREF_CLK_DIV_MASK;
/* Set prescaler. pclk_freq/prescaler < 1MHz */
- value |= (prescaler << HSREF_CLK_DIV_POS);
+ value |= (prescaler << HSREF_CLK_DIV_POS) & HSREF_CLK_DIV_MASK;
/* Select PCLK as reference clock */
value &= ~REFCLK_SEL;
@@ -289,28 +279,16 @@ static int stm_thermal_calculate_threshold(struct stm_thermal_sensor *sensor,
int temp, u32 *th)
{
int freqM;
- u32 sampling_time;
-
- /* Retrieve the number of periods to sample */
- sampling_time = (readl_relaxed(sensor->base + DTS_CFGR1_OFFSET) &
- TS1_SMP_TIME_MASK) >> TS1_SMP_TIME_POS;
/* Figure out the CLK_PTAT frequency for a given temperature */
- freqM = ((temp - sensor->t0) * sensor->ramp_coeff)
- + sensor->fmt0;
-
- dev_dbg(sensor->dev, "%s: freqM for threshold = %d Hz",
- __func__, freqM);
+ freqM = ((temp - sensor->t0) * sensor->ramp_coeff) / 1000 +
+ sensor->fmt0;
/* Figure out the threshold sample number */
- *th = clk_get_rate(sensor->clk);
+ *th = clk_get_rate(sensor->clk) * SAMPLING_TIME / freqM;
if (!*th)
return -EINVAL;
- *th = *th / freqM;
-
- *th *= sampling_time;
-
dev_dbg(sensor->dev, "freqM=%d Hz, threshold=0x%x", freqM, *th);
return 0;
@@ -320,42 +298,28 @@ static int stm_thermal_calculate_threshold(struct stm_thermal_sensor *sensor,
static int stm_thermal_get_temp(void *data, int *temp)
{
struct stm_thermal_sensor *sensor = data;
- u32 sampling_time;
+ u32 periods;
int freqM, ret;
if (sensor->mode != THERMAL_DEVICE_ENABLED)
return -EAGAIN;
- /* Retrieve the number of samples */
- ret = readl_poll_timeout(sensor->base + DTS_DR_OFFSET, freqM,
- (freqM & TS1_MFREQ_MASK), STARTUP_TIME,
- POLL_TIMEOUT);
-
+ /* Retrieve the number of periods sampled */
+ ret = readl_relaxed_poll_timeout(sensor->base + DTS_DR_OFFSET, periods,
+ (periods & TS1_MFREQ_MASK),
+ STARTUP_TIME, POLL_TIMEOUT);
if (ret)
return ret;
- if (!freqM)
- return -ENODATA;
-
- /* Retrieve the number of periods sampled */
- sampling_time = (readl_relaxed(sensor->base + DTS_CFGR1_OFFSET) &
- TS1_SMP_TIME_MASK) >> TS1_SMP_TIME_POS;
-
- /* Figure out the number of samples per period */
- freqM /= sampling_time;
-
/* Figure out the CLK_PTAT frequency */
- freqM = clk_get_rate(sensor->clk) / freqM;
+ freqM = (clk_get_rate(sensor->clk) * SAMPLING_TIME) / periods;
if (!freqM)
return -EINVAL;
- dev_dbg(sensor->dev, "%s: freqM=%d\n", __func__, freqM);
-
/* Figure out the temperature in mili celsius */
- *temp = mcelsius(sensor->t0 + ((freqM - sensor->fmt0) /
- sensor->ramp_coeff));
+ *temp = (freqM - sensor->fmt0) * 1000 / sensor->ramp_coeff + sensor->t0;
- dev_dbg(sensor->dev, "temperature = %d millicelsius", *temp);
+ dev_dbg(sensor->dev, "periods=0x%x t=%d mC", periods, *temp);
return 0;
}
@@ -379,7 +343,8 @@ static int stm_thermal_set_trips(void *data, int low, int high)
if (low > -INT_MAX) {
sensor->low_en = 1;
- ret = stm_thermal_calculate_threshold(sensor, low, &th);
+ /* add 0.5 of hysteresis due to measurement error */
+ ret = stm_thermal_calculate_threshold(sensor, low - 500, &th);
if (ret)
return ret;
--
2.17.1
Hi Pascal,
On 29/10/2019 17:45, Pascal Paillet wrote:
> Fix IRQ flood on low threshold by too ways:
Can you state the issue first ?
> - improve temperature reading resolution,
> - add an hysteresis to the low threshold: on low threshold interrupt,
> it is not possible to get the temperature value that has fired the
> interrupt. The time to acquire a new value is enough for the CPU to
> become hotter than the current low threshold.> Signed-off-by: Pascal Paillet <[email protected]>
> Change-Id: I3b63b8aab38fd651a165c4e69a2d090b3c6f5db3
Please remove the Change-Id tag.
Joe, Andy? checkpatch does not see the Change-Id, is it the expected
behavior?
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Tue, 2019-10-29 at 18:11 +0100, Daniel Lezcano wrote:
> On 29/10/2019 17:45, Pascal Paillet wrote:
> > Fix IRQ flood on low threshold by too ways:
>
> Can you state the issue first ?
>
> > - improve temperature reading resolution,
> > - add an hysteresis to the low threshold: on low threshold interrupt,
> > it is not possible to get the temperature value that has fired the
> > interrupt. The time to acquire a new value is enough for the CPU to
> > become hotter than the current low threshold.
[]
> > Signed-off-by: Pascal Paillet <[email protected]>
> > Change-Id: I3b63b8aab38fd651a165c4e69a2d090b3c6f5db3
>
> Please remove the Change-Id tag.
>
> Joe, Andy? checkpatch does not see the Change-Id, is it the expected
> behavior?
Yes. It's after a sign-off so checkpatch doesn't care.
On 29/10/2019 18:15, Joe Perches wrote:
> On Tue, 2019-10-29 at 18:11 +0100, Daniel Lezcano wrote:
>> On 29/10/2019 17:45, Pascal Paillet wrote:
>>> Fix IRQ flood on low threshold by too ways:
>>
>> Can you state the issue first ?
>>
>>> - improve temperature reading resolution,
>>> - add an hysteresis to the low threshold: on low threshold interrupt,
>>> it is not possible to get the temperature value that has fired the
>>> interrupt. The time to acquire a new value is enough for the CPU to
>>> become hotter than the current low threshold.
> []
>>> Signed-off-by: Pascal Paillet <[email protected]>
>>> Change-Id: I3b63b8aab38fd651a165c4e69a2d090b3c6f5db3
>>
>> Please remove the Change-Id tag.
>>
>> Joe, Andy? checkpatch does not see the Change-Id, is it the expected
>> behavior?
>
> Yes. It's after a sign-off so checkpatch doesn't care.
Ah, I guess it is for Gerrit but we don't want those Change-Id in the
kernel history, right?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 29/10/2019 18:24, Joe Perches wrote:
> On Tue, 2019-10-29 at 18:21 +0100, Daniel Lezcano wrote:
>> On 29/10/2019 18:15, Joe Perches wrote:
>>> On Tue, 2019-10-29 at 18:11 +0100, Daniel Lezcano wrote:
>>>> On 29/10/2019 17:45, Pascal Paillet wrote:
>>>>> Fix IRQ flood on low threshold by too ways:
>>>>
>>>> Can you state the issue first ?
>>>>
>>>>> - improve temperature reading resolution,
>>>>> - add an hysteresis to the low threshold: on low threshold interrupt,
>>>>> it is not possible to get the temperature value that has fired the
>>>>> interrupt. The time to acquire a new value is enough for the CPU to
>>>>> become hotter than the current low threshold.
>>> []
>>>>> Signed-off-by: Pascal Paillet <[email protected]>
>>>>> Change-Id: I3b63b8aab38fd651a165c4e69a2d090b3c6f5db3
>>>>
>>>> Please remove the Change-Id tag.
>>>>
>>>> Joe, Andy? checkpatch does not see the Change-Id, is it the expected
>>>> behavior?
>>>
>>> Yes. It's after a sign-off so checkpatch doesn't care.
>>
>> Ah, I guess it is for Gerrit but we don't want those Change-Id in the
>> kernel history, right?
>
> So remove it from the patch.
It was not a sarcastic question. I just wanted to be sure the Change-Id
is something we always want to remove. There are some of them in the
kernel log and I got a doubt.
checkpatch is perfectly fine for me.
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 29/10/2019 17:45, Pascal Paillet wrote:
> Remove thermal passive trip point.
Why?
> Signed-off-by: Pascal Paillet <[email protected]>
> Change-Id: I494313cf467eea491236e73bd2fbe1803345586f
Remove Change-Id
> ---
> arch/arm/boot/dts/stm32mp157c.dtsi | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 9b11654a0a39..799b2aedd2c9 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -91,12 +91,6 @@
> thermal-sensors = <&dts>;
>
> trips {
> - cpu_alert1: cpu-alert1 {
> - temperature = <85000>;
> - hysteresis = <0>;
> - type = "passive";
> - };
> -
> cpu-crit {
> temperature = <120000>;
> hysteresis = <0>;
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Tue, 2019-10-29 at 18:21 +0100, Daniel Lezcano wrote:
> On 29/10/2019 18:15, Joe Perches wrote:
> > On Tue, 2019-10-29 at 18:11 +0100, Daniel Lezcano wrote:
> > > On 29/10/2019 17:45, Pascal Paillet wrote:
> > > > Fix IRQ flood on low threshold by too ways:
> > >
> > > Can you state the issue first ?
> > >
> > > > - improve temperature reading resolution,
> > > > - add an hysteresis to the low threshold: on low threshold interrupt,
> > > > it is not possible to get the temperature value that has fired the
> > > > interrupt. The time to acquire a new value is enough for the CPU to
> > > > become hotter than the current low threshold.
> > []
> > > > Signed-off-by: Pascal Paillet <[email protected]>
> > > > Change-Id: I3b63b8aab38fd651a165c4e69a2d090b3c6f5db3
> > >
> > > Please remove the Change-Id tag.
> > >
> > > Joe, Andy? checkpatch does not see the Change-Id, is it the expected
> > > behavior?
> >
> > Yes. It's after a sign-off so checkpatch doesn't care.
>
> Ah, I guess it is for Gerrit but we don't want those Change-Id in the
> kernel history, right?
So remove it from the patch.
checkpatch is not a perfect tool.
checkpatch will never be a perfect tool.
It's not possible for checkpatch to be a perfect tool.
On 29/10/2019 17:45, Pascal Paillet wrote:
> Implement set_trips() so that the trip points are managed by the thermal
> framework. The user is free to define any trip points he needs.
> Simplify interrupt handling.
The patch is touching too many things at the same time and the change
description does not help to understand the changes.
> Signed-off-by: Pascal Paillet <[email protected]>
> Change-Id: I2929d4fa5b4c5dca45cec7cb3b93fffc277394d2
Remove Change-Id
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog