Skip to contentSkip to conversation

touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

PATCH New Hans de Goede 3 participants 9 messages Mar 8, 2015 – Mar 9, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

Hi Dmitry, Maxime, et al.

Here is a patch-set for the sun4i-ts driver temperature sensor support, it consists of 2 fixes:

1) Use a better temperature curve for A10 SoCs to avoid reporting a way to high temperature.

2) Start with reporting an "estimated" temperature of 40 degrees rather then returning -EAGAIN until we get the first temperature ready interrupt to avoid the kernel tempzone core logging errors because of our -EAGAIN return.

Dmitry can you please queue up the 2 sun4i-ts patches for 4.1, and Maxime, can you please queue up the matching dts change ?

Thanks & Regards,

Hans

#1 lore raw

PATCH 1/3 touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

Testing has revealed that the temperature in the rtp controller of the A10 (sun4i) SoC has a different curve then on the A13 (sun5i) and later models.

Add a new sun5i-a13-ts compatible to differentiate the newer models and set the curve based on the compatible string.

The new curve is still not ideal on all A10-s, that seems to have to do with there being a large spread between different A10-s out there, the new curve us based on callibration results on 4 completely different models:
raw min raw max temp min temp max stepsize offset

Tong Zhang's hackberry    2402    2680    45.0     80.0    0.125   -255.3
Hansg's Cubieboard        2207    2300    36.0     45.0    0.096   -175.8
Olliver's lime 1 (*):     2258    2537    48.3     87.1    0.139   -265.7
Olliver's lime 2 (*):     2222    2486    46.7     91.7    0.170   -331.0

*) from: http://linux-sunxi.org/Temperature_Calibration

Average all 4:                                             0.133   -257.0
Average without outliers (middle 2):                       0.132   -261.0

Since it is better to slightly overreport the temperature this patch uses the average of all 4 as curve.

This fixes the temperature reported on the A10 being much higher then expected.

---
 .../devicetree/bindings/input/touchscreen/sun4i.txt      |  3 ++-
 drivers/input/touchscreen/sun4i-ts.c                     | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/input/touchscreen/sun4i.txt +2 -1
index 433332d..d59d252 100644
@@ -2,7 +2,8 @@ sun4i resistive touchscreen controller
 --------------------------------------
 
 Required properties:
- - compatible: "allwinner,sun4i-a10-ts" or "allwinner,sun6i-a31-ts"
+ - compatible: "allwinner,sun4i-a10-ts", "allwinner,sun5i-a13-ts" or
+   "allwinner,sun6i-a31-ts"
  - reg: mmio address range of the chip
  - interrupts: interrupt to which the chip is connected
  - #thermal-sensor-cells: shall be 0
drivers/input/touchscreen/sun4i-ts.c +13 -3
index b93a28b..66ccd5a 100644
@@ -258,6 +258,15 @@ static int sun4i_ts_probe(struct platform_device *pdev)
 		/* Allwinner SDK has temperature = -271 + (value / 6) (C) */
 		ts->temp_offset = 1626;
 		ts->temp_step = 167;
+	} else if (of_device_is_compatible(np, "allwinner,sun4i-a10-ts")) {
+		/*
+		 * The A10 temperature sensor has quite a wide spread, these
+		 * parameters are based on the averaging of the calibration
+		 * results of 4 completely different boards, with a spread of
+		 * temp_step from 96 - 170 and temp_offset from 1758 - 3310.
+		 */
+		ts->temp_offset = 2570;
+		ts->temp_step = 133;
 	} else {
 		/*
 		 * The user manuals do not contain the formula for calculating
@@ -330,10 +339,10 @@ static int sun4i_ts_probe(struct platform_device *pdev)
 	 * finally enable tp mode.
 	 */
 	reg = STYLUS_UP_DEBOUN(5) | STYLUS_UP_DEBOUN_EN(1);
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-ts"))
-		reg |= TP_MODE_EN(1);
-	else
+	if (of_device_is_compatible(np, "allwinner,sun6i-a31-ts"))
 		reg |= SUN6I_TP_MODE_EN(1);
+	else
+		reg |= TP_MODE_EN(1);
 	writel(reg, ts->base + TP_CTRL1);
 
 	/*
@@ -383,6 +392,7 @@ static int sun4i_ts_remove(struct platform_device *pdev)
 
 static const struct of_device_id sun4i_ts_of_match[] = {
 	{ .compatible = "allwinner,sun4i-a10-ts", },
+	{ .compatible = "allwinner,sun5i-a13-ts", },
 	{ .compatible = "allwinner,sun6i-a31-ts", },
 	{ /* sentinel */ }
 };
Signature · 1 line
2.3.1
maintainer Applied
#2 lore raw

PATCH 1/3 touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

Quoted 23 lines · Hans de Goede

Testing has revealed that the temperature in the rtp controller of the A10 (sun4i) SoC has a different curve then on the A13 (sun5i) and later models.

Add a new sun5i-a13-ts compatible to differentiate the newer models and set the curve based on the compatible string.

The new curve is still not ideal on all A10-s, that seems to have to do with there being a large spread between different A10-s out there, the new curve us based on callibration results on 4 completely different models:
raw min raw max temp min temp max stepsize offset

Tong Zhang's hackberry    2402    2680    45.0     80.0    0.125   -255.3
Hansg's Cubieboard        2207    2300    36.0     45.0    0.096   -175.8
Olliver's lime 1 (*):     2258    2537    48.3     87.1    0.139   -265.7
Olliver's lime 2 (*):     2222    2486    46.7     91.7    0.170   -331.0

*) from: http://linux-sunxi.org/Temperature_Calibration

Average all 4:                                             0.133   -257.0
Average without outliers (middle 2):                       0.132   -261.0

Since it is better to slightly overreport the temperature this patch uses the average of all 4 as curve.

Quoted 7 lines

This fixes the temperature reported on the A10 being much higher then expected.

Applied, thank you.

Quoted 64 lines
---
 .../devicetree/bindings/input/touchscreen/sun4i.txt      |  3 ++-
 drivers/input/touchscreen/sun4i-ts.c                     | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/input/touchscreen/sun4i.txt +2 -1
index 433332d..d59d252 100644
@@ -2,7 +2,8 @@ sun4i resistive touchscreen controller
 --------------------------------------
 
 Required properties:
- - compatible: "allwinner,sun4i-a10-ts" or "allwinner,sun6i-a31-ts"
+ - compatible: "allwinner,sun4i-a10-ts", "allwinner,sun5i-a13-ts" or
+   "allwinner,sun6i-a31-ts"
  - reg: mmio address range of the chip
  - interrupts: interrupt to which the chip is connected
  - #thermal-sensor-cells: shall be 0
drivers/input/touchscreen/sun4i-ts.c +13 -3
index b93a28b..66ccd5a 100644
@@ -258,6 +258,15 @@ static int sun4i_ts_probe(struct platform_device *pdev)
 		/* Allwinner SDK has temperature = -271 + (value / 6) (C) */
 		ts->temp_offset = 1626;
 		ts->temp_step = 167;
+	} else if (of_device_is_compatible(np, "allwinner,sun4i-a10-ts")) {
+		/*
+		 * The A10 temperature sensor has quite a wide spread, these
+		 * parameters are based on the averaging of the calibration
+		 * results of 4 completely different boards, with a spread of
+		 * temp_step from 96 - 170 and temp_offset from 1758 - 3310.
+		 */
+		ts->temp_offset = 2570;
+		ts->temp_step = 133;
 	} else {
 		/*
 		 * The user manuals do not contain the formula for calculating
@@ -330,10 +339,10 @@ static int sun4i_ts_probe(struct platform_device *pdev)
 	 * finally enable tp mode.
 	 */
 	reg = STYLUS_UP_DEBOUN(5) | STYLUS_UP_DEBOUN_EN(1);
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-ts"))
-		reg |= TP_MODE_EN(1);
-	else
+	if (of_device_is_compatible(np, "allwinner,sun6i-a31-ts"))
 		reg |= SUN6I_TP_MODE_EN(1);
+	else
+		reg |= TP_MODE_EN(1);
 	writel(reg, ts->base + TP_CTRL1);
 
 	/*
@@ -383,6 +392,7 @@ static int sun4i_ts_remove(struct platform_device *pdev)
 
 static const struct of_device_id sun4i_ts_of_match[] = {
 	{ .compatible = "allwinner,sun4i-a10-ts", },
+	{ .compatible = "allwinner,sun5i-a13-ts", },
 	{ .compatible = "allwinner,sun6i-a31-ts", },
 	{ /* sentinel */ }
 };
Signature · 1 line
2.3.1
Signature · 1 line
Dmitry
#3 lore raw

PATCH 1/3 touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

Hi,

Quoted 27 lines · Dmitry Torokhov
Quoted 26 lines · Hans de Goede

Testing has revealed that the temperature in the rtp controller of the A10 (sun4i) SoC has a different curve then on the A13 (sun5i) and later models.

Add a new sun5i-a13-ts compatible to differentiate the newer models and set the curve based on the compatible string.

The new curve is still not ideal on all A10-s, that seems to have to do with there being a large spread between different A10-s out there, the new curve us based on callibration results on 4 completely different models:
raw min raw max temp min temp max stepsize offset

Tong Zhang's hackberry    2402    2680    45.0     80.0    0.125   -255.3
Hansg's Cubieboard        2207    2300    36.0     45.0    0.096   -175.8
Olliver's lime 1 (*):     2258    2537    48.3     87.1    0.139   -265.7
Olliver's lime 2 (*):     2222    2486    46.7     91.7    0.170   -331.0

*) from: http://linux-sunxi.org/Temperature_Calibration

Average all 4:                                             0.133   -257.0
Average without outliers (middle 2):                       0.132   -261.0

Since it is better to slightly overreport the temperature this patch uses the average of all 4 as curve.

This fixes the temperature reported on the A10 being much higher then expected.

Quoted 7 lines
Quoted 4 lines

Applied, thank you.

Thanks.

Note I've just found out that I've made a math error, the offset as I've used it is in degrees, but the actual code applies the offset before multiplying by stepsize :|

Given that this is rather backwards (every math course ever thought applies the multiplication before the offset for linear functions. I'll do a follow up patch fixing the code to do the logical thing, and adjusting the offset for the other models accordingly.

Regards,

Hans

Quoted 66 lines
Quoted 65 lines
---
  .../devicetree/bindings/input/touchscreen/sun4i.txt      |  3 ++-
  drivers/input/touchscreen/sun4i-ts.c                     | 16 +++++++++++++---
  2 files changed, 15 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/input/touchscreen/sun4i.txt +2 -1
index 433332d..d59d252 100644
@@ -2,7 +2,8 @@ sun4i resistive touchscreen controller
  --------------------------------------
 
  Required properties:
- - compatible: "allwinner,sun4i-a10-ts" or "allwinner,sun6i-a31-ts"
+ - compatible: "allwinner,sun4i-a10-ts", "allwinner,sun5i-a13-ts" or
+   "allwinner,sun6i-a31-ts"
   - reg: mmio address range of the chip
   - interrupts: interrupt to which the chip is connected
   - #thermal-sensor-cells: shall be 0
drivers/input/touchscreen/sun4i-ts.c +13 -4
index b93a28b..66ccd5a 100644
@@ -258,6 +258,15 @@ static int sun4i_ts_probe(struct platform_device *pdev)
  		/* Allwinner SDK has temperature = -271 + (value / 6) (C) */
  		ts->temp_offset = 1626;
  		ts->temp_step = 167;
+	} else if (of_device_is_compatible(np, "allwinner,sun4i-a10-ts")) {
+		/*
+		 * The A10 temperature sensor has quite a wide spread, these
+		 * parameters are based on the averaging of the calibration
+		 * results of 4 completely different boards, with a spread of
+		 * temp_step from 96 - 170 and temp_offset from 1758 - 3310.
+		 */
+		ts->temp_offset = 2570;
+		ts->temp_step = 133;
  	} else {
  		/*
  		 * The user manuals do not contain the formula for calculating
@@ -330,10 +339,10 @@ static int sun4i_ts_probe(struct platform_device *pdev)
  	 * finally enable tp mode.
  	 */
  	reg = STYLUS_UP_DEBOUN(5) | STYLUS_UP_DEBOUN_EN(1);
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-ts"))
-		reg |= TP_MODE_EN(1);
-	else
+	if (of_device_is_compatible(np, "allwinner,sun6i-a31-ts"))
  		reg |= SUN6I_TP_MODE_EN(1);
+	else
+		reg |= TP_MODE_EN(1);
  	writel(reg, ts->base + TP_CTRL1);
 
  	/*
@@ -383,6 +392,7 @@ static int sun4i_ts_remove(struct platform_device *pdev)
 
  static const struct of_device_id sun4i_ts_of_match[] = {
  	{ .compatible = "allwinner,sun4i-a10-ts", },
+	{ .compatible = "allwinner,sun5i-a13-ts", },
  	{ .compatible = "allwinner,sun6i-a31-ts", },
  	{ /* sentinel */ }
  };
--

2.3.1

#4 lore raw

PATCH 2/3 touchscreen: sun4i-ts: Do not report -EAGAIN from tempzone callback

In reply to Hans de Goede

Returning -EAGAIN while waiting for the first temperature ready interrupt, causes the tempzone core to log the following errors:

[    1.219353] thermal thermal_zone0: failed to read out thermal zone 0
[    2.015433] thermal thermal_zone0: failed to read out thermal zone 0
[    2.416737] thermal thermal_zone0: failed to read out thermal zone 0

So instead of returning -EAGAIN, return an intial temperature estimate of 40 degrees, and a success status.

---
 drivers/input/touchscreen/sun4i-ts.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
drivers/input/touchscreen/sun4i-ts.c +9 -10
index 66ccd5a..d641b12 100644
@@ -189,12 +189,7 @@ static void sun4i_ts_close(struct input_dev *dev)
 
 static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp)
 {
-	/* No temp_data until the first irq */
-	if (ts->temp_data == -1)
-		return -EAGAIN;
-
 	*temp = (ts->temp_data - ts->temp_offset) * ts->temp_step;
-
 	return 0;
 }
 
@@ -212,11 +207,8 @@ static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
 {
 	struct sun4i_ts_data *ts = dev_get_drvdata(dev);
 	long temp;
-	int error;
 
-	error = sun4i_get_temp(ts, &temp);
-	if (error)
-		return error;
+	sun4i_get_temp(ts, &temp);
 
 	return sprintf(buf, "%ld\n", temp);
 }
@@ -253,7 +245,7 @@ static int sun4i_ts_probe(struct platform_device *pdev)
 
 	ts->dev = dev;
 	ts->ignore_fifo_data = true;
-	ts->temp_data = -1;
+
 	if (of_device_is_compatible(np, "allwinner,sun6i-a31-ts")) {
 		/* Allwinner SDK has temperature = -271 + (value / 6) (C) */
 		ts->temp_offset = 1626;
@@ -283,6 +275,13 @@ static int sun4i_ts_probe(struct platform_device *pdev)
 		ts->temp_step = 100;
 	}
 
+	/*
+	 * Start by reporting 40 degrees as temperature, this avoids
+	 * "thermal_zone: failed to read out thermal zone" errors while
+	 * waiting for the first temp ready irq.
+	 */
+	ts->temp_data = ts->temp_offset + 40000 / ts->temp_step;
+
 	ts_attached = of_property_read_bool(np, "allwinner,ts-attached");
 	if (ts_attached) {
 		ts->input = devm_input_allocate_device(dev);
Signature · 1 line
2.3.1
#5 lore raw

PATCH 3/3 ARM: dts: sunxi: Adjust touchscreen compatible for sun5i and later

In reply to Hans de Goede

The touchscreen controller in the A13 and later has a different temperature curve than the one in the original A10, change the compatible for the A13 and later so that the kernel will use the correct curve.

---
 arch/arm/boot/dts/sun5i.dtsi     | 2 +-
 arch/arm/boot/dts/sun7i-a20.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/sun5i.dtsi +1 -1
index e42cbb0..610ea18 100644
@@ -494,7 +494,7 @@
 		};
 
 		rtp: rtp at 01c25000 {
-			compatible = "allwinner,sun4i-a10-ts";
+			compatible = "allwinner,sun5i-a13-ts";
 			reg = <0x01c25000 0x100>;
 			interrupts = <29>;
 			#thermal-sensor-cells = <0>;
arch/arm/boot/dts/sun7i-a20.dtsi +1 -1
index 3a8530b..8df77f5 100644
@@ -1043,7 +1043,7 @@
 		};
 
 		rtp: rtp at 01c25000 {
-			compatible = "allwinner,sun4i-a10-ts";
+			compatible = "allwinner,sun5i-a13-ts";
 			reg = <0x01c25000 0x100>;
 			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
 			#thermal-sensor-cells = <0>;
Signature · 1 line
2.3.1
#6 lore raw

PATCH 3/3 ARM: dts: sunxi: Adjust touchscreen compatible for sun5i and later

Quoted 6 lines · Hans de Goede

The touchscreen controller in the A13 and later has a different temperature curve than the one in the original A10, change the compatible for the A13 and later so that the kernel will use the correct curve.

Applied, thanks!

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150308/dd932ba1/attachment.sig>
maintainer
#7 lore raw

cover touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

In reply to Hans de Goede

Hi Hans,

Quoted 11 lines · Hans de Goede

Hi Dmitry, Maxime, et al.

Here is a patch-set for the sun4i-ts driver temperature sensor support, it consists of 2 fixes:

1) Use a better temperature curve for A10 SoCs to avoid reporting a way to high temperature.

2) Start with reporting an "estimated" temperature of 40 degrees rather then returning -EAGAIN until we get the first temperature ready interrupt to avoid the kernel tempzone core logging errors because of our -EAGAIN return.

No, I think we better teach thermal core not to complain about -EAGAINs.

Quoted 7 lines

Dmitry can you please queue up the 2 sun4i-ts patches for 4.1, and Maxime, can you please queue up the matching dts change ?

Thanks & Regards,

Hans

Signature · 1 line
Dmitry
#8 lore raw

cover touchscreen: sun4i-ts: A10 (sun4i) has a different temperature curve

Hi,

Quoted 11 lines · Dmitry Torokhov

Hi Hans,

Quoted 8 lines · Hans de Goede

Hi Dmitry, Maxime, et al.

Here is a patch-set for the sun4i-ts driver temperature sensor support, it consists of 2 fixes:

1) Use a better temperature curve for A10 SoCs to avoid reporting a way to high temperature.

Quoted 6 lines
Quoted 3 lines

2) Start with reporting an "estimated" temperature of 40 degrees rather then returning -EAGAIN until we get the first temperature ready interrupt to avoid the kernel tempzone core logging errors because of our -EAGAIN return.

No, I think we better teach thermal core not to complain about -EAGAINs.

I was wondering if that would not be a better fix myself, so ack. I'll go do a patch for that and submit it to the thermalzone maintainer.

Regards,

Hans