IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

Allwinner H3 features a thermal sensor like the one in A33, but has its register re-arranged, the clock divider moved to CCU (originally the clock divider is in ADC) and added a pair of bus clock and reset.

Allwinner A83T features a thermal sensor similar to the H3, the ths clock, the bus clock and the reset was removed from the CCU. The THS in A83T has a clock that is directly connected and runs with 24 MHz.

Update the binding document to cover H3 and A83T.

Signed-off-by Philipp Rossak <[email protected]>

---
 .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +37 -3

index 86dd8191b04c..22df0c5c23d4 100644
@@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
 and sometimes as a touchscreen controller.
 
 Required properties:
-  - compatible: "allwinner,sun8i-a33-ths",
+  - compatible: must contain one of the following compatibles:
+		- "allwinner,sun8i-a33-ths"
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
   - reg: mmio address range of the chip,
-  - #thermal-sensor-cells: shall be 0,
+  - #thermal-sensor-cells: shall be 0 or 1,
   - #io-channel-cells: shall be 0,
 
-Example:
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
+  - interrupts: the sampling interrupt of the ADC,
+
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - clocks: the bus clock and the input clock of the ADC,
+  - clock-names: should be "bus" and "mod",
+  - resets: the bus reset of the ADC,
+
+Optional properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
+		If unspecified default values shall be used. The size should
+		be 0x2 * sensorcount.
+  - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
+Example for A33:
 	ths: ths at 1c25000 {
 		compatible = "allwinner,sun8i-a33-ths";
 		reg = <0x01c25000 0x100>;

@@ -17,6 +40,27 @@ Example:
 		#io-channel-cells = <0>;
 	};
 
+Example for H3:
+	ths: thermal-sensor at 1c25000 {
+		compatible = "allwinner,sun8i-h3-ths";
+		reg = <0x01c25000 0x400>;
+		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+		clock-names = "bus", "mod";
+		resets = <&ccu RST_BUS_THS>;
+		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <0>;
+		#io-channel-cells = <0>;
+	};

Quoted 19 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +12 -0

+Example for A33:
  	ths: ths at 1c25000 {
  		compatible = "allwinner,sun8i-a33-ths";
  		reg = <0x01c25000 0x100>;

@@ -17,6 +40,27 @@ Example:
  		#io-channel-cells = <0>;
  	};
 
+Example for H3:
+	ths: thermal-sensor at 1c25000 {
+		compatible = "allwinner,sun8i-h3-ths";
+		reg = <0x01c25000 0x400>;
+		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+		clock-names = "bus", "mod";
+		resets = <&ccu RST_BUS_THS>;
+		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <0>;
+		#io-channel-cells = <0>;
+	};

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt Example: · line 63

Quoted 10 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +10 -0

+
+Example for A83T:
+	ths: thermal-sensor at 1f04000 {
+		compatible = "allwinner,sun8i-a83t-ths";
+		reg = <0x01f04000 0x100>;
+		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <1>;
+		#io-channel-cells = <0>;
+	};
+

I'm wondering if this is actually needed. We've used this convoluted constructs to be compatible with the old driver, but I'm not sure this is actually worth it now, and this is causing several issues, among which:
- We need to have a bunch of quirks to handle all the DT cases.
- We need to have an MFD, which isn't really optimal

So I'd rather introduce a new compatible for the old SoCs, keep the old driver around, and simplify a lot that driver code that will ease further developments. And we can also get rid of the MFD in the process. I discussed it with Quentin, and he was ok with it, what do you think?

(and that would involve creating a new file for the bindings you introduce here).

Maxime

Allwinner H3 features a thermal sensor like the one in A33, but has its register re-arranged, the clock divider moved to CCU (originally the clock divider is in ADC) and added a pair of bus clock and reset.

Allwinner A83T features a thermal sensor similar to the H3, the ths clock, the bus clock and the reset was removed from the CCU. The THS in A83T has a clock that is directly connected and runs with 24 MHz.

Update the binding document to cover H3 and A83T.

Signed-off-by Philipp Rossak <[email protected]>

---
 .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +0 -0

index 86dd8191b04c..22df0c5c23d4 100644
@@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
 and sometimes as a touchscreen controller.

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +6 -1

   - #io-channel-cells: shall be 0,
 
-Example:
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
+  - interrupts: the sampling interrupt of the ADC,
+
+Required properties for the following compatibles:

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +16 -0

+  - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
+Example for A33:
 	ths: ths at 1c25000 {
 		compatible = "allwinner,sun8i-a33-ths";
 		reg = <0x01c25000 0x100>;

@@ -17,6 +40,27 @@ Example:
 		#io-channel-cells = <0>;
 	};
 
+Example for H3:
+	ths: thermal-sensor at 1c25000 {
+		compatible = "allwinner,sun8i-h3-ths";
+		reg = <0x01c25000 0x400>;
+		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+		clock-names = "bus", "mod";
+		resets = <&ccu RST_BUS_THS>;
+		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <0>;
+		#io-channel-cells = <0>;
+	};

Hi Philipp,

Quoted 21 lines · Philipp Rossak

Allwinner H3 features a thermal sensor like the one in A33, but has its register re-arranged, the clock divider moved to CCU (originally the clock divider is in ADC) and added a pair of bus clock and reset.

Allwinner A83T features a thermal sensor similar to the H3, the ths clock, the bus clock and the reset was removed from the CCU. The THS in A83T has a clock that is directly connected and runs with 24 MHz.

Update the binding document to cover H3 and A83T.

Signed-off-by Philipp Rossak <[email protected]>

---
  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
  1 file changed, 47 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +0 -0

index 86dd8191b04c..22df0c5c23d4 100644
@@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
  and sometimes as a touchscreen controller.

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt The Allwinner SoCs all have an ADC that can also act as a thermal sensor · line 12

Quoted 10 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +5 -2

 
  Required properties:
-  - compatible: "allwinner,sun8i-a33-ths",
+  - compatible: must contain one of the following compatibles:
+		- "allwinner,sun8i-a33-ths"
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
    - reg: mmio address range of the chip,
-  - #thermal-sensor-cells: shall be 0,
+  - #thermal-sensor-cells: shall be 0 or 1,

Well, thermal-sensor-cells is either 0 or 1 :)

Better to point to the documentation describing this thermal-sensor-cells IMHO.

Quoted 13 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +10 -1

    - #io-channel-cells: shall be 0,
 
-Example:
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
+  - interrupts: the sampling interrupt of the ADC,
+
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - clocks: the bus clock and the input clock of the ADC,
+  - clock-names: should be "bus" and "mod",
+  - resets: the bus reset of the ADC,

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt The Allwinner SoCs all have an ADC that can also act as a thermal sensor · line 30

Quoted 6 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +6 -0

+
+Optional properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
+		If unspecified default values shall be used. The size should
+		be 0x2 * sensorcount.

"twice the number of sensors" ?

Quoted 23 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +16 -0

+  - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
+Example for A33:
  	ths: ths at 1c25000 {
  		compatible = "allwinner,sun8i-a33-ths";
  		reg = <0x01c25000 0x100>;

@@ -17,6 +40,27 @@ Example:
  		#io-channel-cells = <0>;
  	};
 
+Example for H3:
+	ths: thermal-sensor at 1c25000 {
+		compatible = "allwinner,sun8i-h3-ths";
+		reg = <0x01c25000 0x400>;
+		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+		clock-names = "bus", "mod";
+		resets = <&ccu RST_BUS_THS>;
+		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <0>;
+		#io-channel-cells = <0>;
+	};

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt Example: · line 63

Quoted 10 lines

Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +10 -0

+
+Example for A83T:
+	ths: thermal-sensor at 1f04000 {
+		compatible = "allwinner,sun8i-a83t-ths";
+		reg = <0x01f04000 0x100>;
+		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <1>;
+		#io-channel-cells = <0>;
+	};
+

Aside from Maxime's comment on how we would like to refactor GPADC/THS, I'm not sure we really want an example for each an every thermal sensor supported.

Quentin

This commit enables the SUN4I_GPADC config option and sets the value to yes. This is needed to enable the ths sensors.

Signed-off-by Philipp Rossak <[email protected]>

---
 arch/arm/configs/sunxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

arch/arm/configs/sunxi_defconfig +0 -0

index 5caaf971fb50..52c3990b9d91 100644

For adding newer sensor some basic rework of the code is necessary.

This commit reworks the code and allows the sampling start/end code and the position of value readout register to be altered. Later the start/end functions will be used to configure the ths and start/stop the sampling.

Signed-off-by Icenowy Zheng <[email protected]>

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +12 -0

index 03804ff9c006..db57d9fffe48 100644
@@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
 	return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
 }
 
+struct sun4i_gpadc_iio;
+
+/*
+ * Prototypes for these functions, which enable these functions to be
+ * referenced in gpadc_data structures.
+ */
+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
+
 struct gpadc_data {
 	int		temp_offset;
 	int		temp_scale;

@@ -56,6 +65,9 @@ struct gpadc_data {
 	unsigned int	tp_adc_select;
 	unsigned int	(*adc_chan_select)(unsigned int chan);
 	unsigned int	adc_chan_mask;
+	unsigned int	temp_data;
+	int		(*sample_start)(struct sun4i_gpadc_iio *info);
+	int		(*sample_end)(struct sun4i_gpadc_iio *info);
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {

For adding newer sensor some basic rework of the code is necessary.

This commit reworks the code and allows the sampling start/end code and the position of value readout register to be altered. Later the start/end functions will be used to configure the ths and start/stop the sampling.

Signed-off-by Icenowy Zheng <[email protected]>

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index 03804ff9c006..db57d9fffe48 100644

Hi Philipp,

Quoted 18 lines · Philipp Rossak

For adding newer sensor some basic rework of the code is necessary.

This commit reworks the code and allows the sampling start/end code and the position of value readout register to be altered. Later the start/end functions will be used to configure the ths and start/stop the sampling.

Signed-off-by Icenowy Zheng <[email protected]>

Signed-off-by Philipp Rossak <[email protected]>

---
  drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
  1 file changed, 39 insertions(+), 5 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index 03804ff9c006..db57d9fffe48 100644

drivers/iio/adc/sun4i-gpadc-iio.c static unsigned int sun6i_gpadc_chan_select(unsigned int chan) · line 57

Quoted 9 lines

drivers/iio/adc/sun4i-gpadc-iio.c +6 -0

@@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
  	return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
  }
 
+struct sun4i_gpadc_iio;
+
+/*
+ * Prototypes for these functions, which enable these functions to be
+ * referenced in gpadc_data structures.
+ */

Comment not needed.

drivers/iio/adc/sun4i-gpadc-iio.c struct gpadc_data { · line 68

Quoted 11 lines

drivers/iio/adc/sun4i-gpadc-iio.c +4 -0

+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
+
  struct gpadc_data {
  	int		temp_offset;
  	int		temp_scale;

@@ -56,6 +65,9 @@ struct gpadc_data {
  	unsigned int	tp_adc_select;
  	unsigned int	(*adc_chan_select)(unsigned int chan);
  	unsigned int	adc_chan_mask;
+	unsigned int	temp_data;

Does not really have anything to do with sample_start/end. I would have made a different commit for it.

Otherwise,

Reviewed-by Quentin Schulz <[email protected]>

Quentin

For adding newer sensor some basic rework of the code is necessary.

The SoCs after H3 has newer thermal sensor ADCs, which have two clock inputs (bus clock and sampling clock) and a reset. The registers are also re-arranged.

This commit reworks the code, adds the process of the clocks and resets.

Signed-off-by Philipp Rossak <[email protected]>

Signed-off-by Icenowy Zheng <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

drivers/iio/adc/sun4i-gpadc-iio.c +2 -0

index db57d9fffe48..51ec0104d678 100644
@@ -22,6 +22,7 @@
  * shutdown for not being used.
  */
 
+#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>

@@ -31,6 +32,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/thermal.h>
 #include <linux/delay.h>
 

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

 };
 
 static const struct gpadc_data sun4i_gpadc_data = {

@@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
 	atomic_t			ignore_temp_data_irq;
 	const struct gpadc_data		*data;
 	bool				no_irq;
+	struct clk			*bus_clk;
+	struct clk			*mod_clk;
+	struct reset_control		*reset;
 	/* prevents concurrent reads of temperature and ADC */
 	struct mutex			mutex;
 	struct thermal_zone_device	*tzd;

drivers/iio/adc/sun4i-gpadc-iio.c +27 -0

 	return info->data->sample_end(info);
 }
 

@@ -446,6 +458,10 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
 
+	clk_enable(info->mod_clk);
+
+	clk_enable(info->bus_clk);
+
 	return info->data->sample_start(info);
 }
 

@@ -560,10 +576,59 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 		return ret;
 	}
 
+	if (info->data->has_bus_rst) {
+		info->reset = devm_reset_control_get(&pdev->dev, NULL);
+		if (IS_ERR(info->reset)) {
+			ret = PTR_ERR(info->reset);
+			return ret;
+		}
+
+		ret = reset_control_deassert(info->reset);
+		if (ret)
+			return ret;
+	}
+
+	if (info->data->has_bus_clk) {
+		info->bus_clk = devm_clk_get(&pdev->dev, "bus");
+		if (IS_ERR(info->bus_clk)) {
+			ret = PTR_ERR(info->bus_clk);
+			goto assert_reset;
+		}
+
+		ret = clk_prepare_enable(info->bus_clk);
+		if (ret)
+			goto assert_reset;
+	}

For adding newer sensor some basic rework of the code is necessary.

This patch reworks the driver to be able to handle more than one thermal sensor. Newer SoC like the A80 have 4 thermal sensors. Because of this the maximal sensor count value was set to 4.

The sensor_id value is set during sensor registration and is for each registered sensor indiviual. This makes it able to differntiate the sensors when the value is read from the register.

In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects in the temp_read function automatically sensor 0. A check for the sensor_id is here not required since the old sensors only have one thermal sensor. In addition to that is the sun4i_gpadc_read_raw() function only used by the "older" sensors (before A33) where the thermal sensor was a cobination of an adc and a thermal sensor.

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
 include/linux/mfd/sun4i-gpadc.h   |  3 +++
 2 files changed, 26 insertions(+), 13 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +17 -10

index 51ec0104d678..ac9ad2f8232f 100644
@@ -67,12 +67,13 @@ struct gpadc_data {
 	unsigned int	tp_adc_select;
 	unsigned int	(*adc_chan_select)(unsigned int chan);
 	unsigned int	adc_chan_mask;
-	unsigned int	temp_data;
+	unsigned int	temp_data[MAX_SENSOR_COUNT];
 	int		(*sample_start)(struct sun4i_gpadc_iio *info);
 	int		(*sample_end)(struct sun4i_gpadc_iio *info);
 	bool		has_bus_clk;
 	bool		has_bus_rst;
 	bool		has_mod_clk;
+	int		sensor_count;
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {

@@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
 	.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun4i_gpadc_chan_select,
 	.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
-	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
+	.sensor_count = 1,
 };
 
 static const struct gpadc_data sun5i_gpadc_data = {

@@ -94,9 +96,10 @@ static const struct gpadc_data sun5i_gpadc_data = {
 	.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun4i_gpadc_chan_select,
 	.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
-	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
+	.sensor_count = 1,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {

@@ -106,18 +109,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
 	.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun6i_gpadc_chan_select,
 	.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
-	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
+	.sensor_count = 1,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.temp_offset = -1662,
 	.temp_scale = 162,
 	.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
-	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
+	.sensor_count = 1,
 };
 
 struct sun4i_gpadc_iio {

@@ -135,6 +140,7 @@ struct sun4i_gpadc_iio {
 	struct clk			*bus_clk;
 	struct clk			*mod_clk;
 	struct reset_control		*reset;
+	int				sensor_id;
 	/* prevents concurrent reads of temperature and ADC */
 	struct mutex			mutex;
 	struct thermal_zone_device	*tzd;

@@ -302,14 +308,15 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
 	return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
 }
 
-static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
+static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
+				int sensor)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
 
 	if (info->no_irq) {
 		pm_runtime_get_sync(indio_dev->dev.parent);
 
-		regmap_read(info->regmap, info->data->temp_data, val);
+		regmap_read(info->regmap, info->data->temp_data[sensor], val);
 
 		pm_runtime_mark_last_busy(indio_dev->dev.parent);
 		pm_runtime_put_autosuspend(indio_dev->dev.parent);

@@ -356,7 +363,7 @@ static int sun4i_gpadc_read_raw(struct iio_dev *indio_dev,
 			ret = sun4i_gpadc_adc_read(indio_dev, chan->channel,
 						   val);
 		else
-			ret = sun4i_gpadc_temp_read(indio_dev, val);
+			ret = sun4i_gpadc_temp_read(indio_dev, val, 0);
 
 		if (ret)
 			return ret;

@@ -470,7 +477,7 @@ static int sun4i_gpadc_get_temp(void *data, int *temp)
 	struct sun4i_gpadc_iio *info = data;
 	int val, scale, offset;
 
-	if (sun4i_gpadc_temp_read(info->indio_dev, &val))
+	if (sun4i_gpadc_temp_read(info->indio_dev, &val, info->sensor_id))
 		return -ETIMEDOUT;
 
 	sun4i_gpadc_temp_scale(info->indio_dev, &scale);

@@ -712,7 +719,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 {
 	struct sun4i_gpadc_iio *info;
 	struct iio_dev *indio_dev;
-	int ret;
+	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
 	if (!indio_dev)

@@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 

For adding newer sensor some basic rework of the code is necessary.

This patch reworks the driver to be able to handle more than one thermal sensor. Newer SoC like the A80 have 4 thermal sensors. Because of this the maximal sensor count value was set to 4.

The sensor_id value is set during sensor registration and is for each registered sensor indiviual. This makes it able to differntiate the sensors when the value is read from the register.

In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects in the temp_read function automatically sensor 0. A check for the sensor_id is here not required since the old sensors only have one thermal sensor. In addition to that is the sun4i_gpadc_read_raw() function only used by the "older" sensors (before A33) where the thermal sensor was a cobination of an adc and a thermal sensor.

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
 include/linux/mfd/sun4i-gpadc.h   |  3 +++
 2 files changed, 26 insertions(+), 13 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index 51ec0104d678..ac9ad2f8232f 100644
@@ -67,12 +67,13 @@ struct gpadc_data {
 	unsigned int	tp_adc_select;
 	unsigned int	(*adc_chan_select)(unsigned int chan);
 	unsigned int	adc_chan_mask;

Hi Philipp,

Quoted 32 lines · Philipp Rossak

For adding newer sensor some basic rework of the code is necessary.

This patch reworks the driver to be able to handle more than one thermal sensor. Newer SoC like the A80 have 4 thermal sensors. Because of this the maximal sensor count value was set to 4.

The sensor_id value is set during sensor registration and is for each registered sensor indiviual. This makes it able to differntiate the sensors when the value is read from the register.

In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects in the temp_read function automatically sensor 0. A check for the sensor_id is here not required since the old sensors only have one thermal sensor. In addition to that is the sun4i_gpadc_read_raw() function only used by the "older" sensors (before A33) where the thermal sensor was a cobination of an adc and a thermal sensor.

Signed-off-by Philipp Rossak <[email protected]>

---
  drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
  include/linux/mfd/sun4i-gpadc.h   |  3 +++
  2 files changed, 26 insertions(+), 13 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index 51ec0104d678..ac9ad2f8232f 100644
@@ -67,12 +67,13 @@ struct gpadc_data {
  	unsigned int	tp_adc_select;
  	unsigned int	(*adc_chan_select)(unsigned int chan);
  	unsigned int	adc_chan_mask;

drivers/iio/adc/sun4i-gpadc-iio.c struct gpadc_data { · line 78

Quoted 10 lines

drivers/iio/adc/sun4i-gpadc-iio.c +2 -1

-	unsigned int	temp_data;
+	unsigned int	temp_data[MAX_SENSOR_COUNT];
  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
  	bool		has_bus_clk;
  	bool		has_bus_rst;
  	bool		has_mod_clk;
+	int		sensor_count;
  };
 

I've noticed that for H3, A83T, A64 (at least), if DATA reg of sensor 0 is e.g. 0x80, DATA reg of sensor N is at 0x80 + 0x04 * N.

Is that verified for other SoCs? Does anyone have some input on this?

We could then just use temp_data as the DATA reg "base" and increment by 0x4 depending on the sensor id instead of using a fixed-size array.

drivers/iio/adc/sun4i-gpadc-iio.c static const struct gpadc_data sun4i_gpadc_data = { · line 89

Quoted 10 lines

drivers/iio/adc/sun4i-gpadc-iio.c +2 -1

  static const struct gpadc_data sun4i_gpadc_data = {

@@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
  	.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
  	.adc_chan_select = &sun4i_gpadc_chan_select,
  	.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
-	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
+	.sensor_count = 1,

If the solution above is not desirable/possible, could we use something like:

unsigned int sun4i_temp_data[] = {SUN4I_GPADC_TEMP_DATA,};

static const struct gpadc_data sun4i_gpadc_data = {
	.temp_data = &sun4i_temp_data,
	.sensor_count = ARRAY_SIZE(sun4i_temp_data),
};

That avoids 1) inconsistencies between the array size and the array itself, 2) does not require to pad the array with zeroes.

[...]

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe(struct platform_device *pdev) · line 760

Quoted 13 lines

drivers/iio/adc/sun4i-gpadc-iio.c +6 -3

@@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
  	pm_runtime_enable(&pdev->dev);
 
  	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
-		info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
-							    0, info,
-							    &sun4i_ts_tz_ops);
+		for (i = 0; i < info->data->sensor_count; i++) {
+			info->sensor_id = i;
+			info->tzd = thermal_zone_of_sensor_register(
+					info->sensor_device,
+					i, info, &sun4i_ts_tz_ops);
+		}

As Maxime said, this does not work.

One way would be to have a new structure being:

struct sun4i_sensor_info {
	struct sun4i_gpadc_iio	*info;
	unsigned int		sensor_id;
};

Or since we only use the iio_dev within the sun4i_gpadc_iio in the .get_temp function, we may replace info by struct iio_dev *indio_dev above.

Quentin

This patch reworks the driver to support nvmem calibration cells. The driver checks if the nvmem calibration is supported and reads out the nvmem.

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

index ac9ad2f8232f..74eeb5cd5218 100644
@@ -27,6 +27,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

 };
 
 static const struct gpadc_data sun5i_gpadc_data = {

@@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {

@@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {

@@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 struct sun4i_gpadc_iio {

drivers/iio/adc/sun4i-gpadc-iio.c +4 -0

 	/* prevents concurrent reads of temperature and ADC */
 	struct mutex			mutex;
 	struct thermal_zone_device	*tzd;

@@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 	struct resource *mem;
 	void __iomem *base;
 	int ret;
+	struct nvmem_cell *cell;
+	ssize_t cell_size;
+	u64 *cell_data;
 
 	info->data = of_device_get_match_data(&pdev->dev);
 	if (!info->data)

@@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	info->has_calibration_data[0] = false;

drivers/iio/adc/sun4i-gpadc-iio.c +5 -0

+	}
+
+	cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
+	nvmem_cell_put(cell);
+	switch (cell_size) {

Quoted 21 lines · Philipp Rossak

This patch reworks the driver to support nvmem calibration cells. The driver checks if the nvmem calibration is supported and reads out the nvmem.

Signed-off-by Philipp Rossak <[email protected]>

---
  drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
  1 file changed, 44 insertions(+)

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

index ac9ad2f8232f..74eeb5cd5218 100644
@@ -27,6 +27,7 @@
  #include <linux/interrupt.h>
  #include <linux/io.h>
  #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
  #include <linux/of.h>
  #include <linux/of_device.h>
  #include <linux/platform_device.h>

drivers/iio/adc/sun4i-gpadc-iio.c struct gpadc_data { · line 78

Quoted 5 lines

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

@@ -74,6 +75,7 @@ struct gpadc_data {
  	bool		has_bus_rst;
  	bool		has_mod_clk;
  	int		sensor_count;
+	bool		supports_nvmem;

I think you should add some documentation along with all the fields you're adding.

drivers/iio/adc/sun4i-gpadc-iio.c static const struct gpadc_data sun4i_gpadc_data = { · line 92

Quoted 9 lines

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

  };
 
  static const struct gpadc_data sun4i_gpadc_data = {

@@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,

That's already its value if you leave it out.

Quoted 26 lines

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

  };
 
  static const struct gpadc_data sun5i_gpadc_data = {

@@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,
  };
 
  static const struct gpadc_data sun6i_gpadc_data = {

@@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,
  };
 
  static const struct gpadc_data sun8i_a33_gpadc_data = {

@@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,
  };
 

drivers/iio/adc/sun4i-gpadc-iio.c struct sun4i_gpadc_iio { · line 151

Quoted 7 lines

drivers/iio/adc/sun4i-gpadc-iio.c +2 -0

  struct sun4i_gpadc_iio {

@@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
  	struct clk			*mod_clk;
  	struct reset_control		*reset;
  	int				sensor_id;
+	u32				calibration_data[2];
+	bool				has_calibration_data[2];

Why do you have two different values here?

Quoted 17 lines

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

  	/* prevents concurrent reads of temperature and ADC */
  	struct mutex			mutex;
  	struct thermal_zone_device	*tzd;

@@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
  	struct resource *mem;
  	void __iomem *base;
  	int ret;
+	struct nvmem_cell *cell;
+	ssize_t cell_size;
+	u64 *cell_data;
 
  	info->data = of_device_get_match_data(&pdev->dev);
  	if (!info->data)

@@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
  	if (IS_ERR(base))
  		return PTR_ERR(base);
 

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe_dt(struct platform_device *pdev, · line 599

Quoted 11 lines

drivers/iio/adc/sun4i-gpadc-iio.c +11 -0

+	info->has_calibration_data[0] = false;
+	info->has_calibration_data[1] = false;
+
+	if (!info->data->supports_nvmem)
+		goto no_nvmem;
+
+	cell = nvmem_cell_get(&pdev->dev, "calibration");
+	if (IS_ERR(cell)) {
+		if (PTR_ERR(cell) == -EPROBE_DEFER)
+			return PTR_ERR(cell);
+		goto no_nvmem;

goto considered evil ? :)

Quoted 9 lines

drivers/iio/adc/sun4i-gpadc-iio.c +9 -0

+	}
+
+	cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
+	nvmem_cell_put(cell);
+	switch (cell_size) {
+		case 8:
+		case 6:
+			info->has_calibration_data[1] = true;
+			info->calibration_data[1] = be32_to_cpu(

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe_dt(struct platform_device *pdev, · line 614

Quoted 6 lines

drivers/iio/adc/sun4i-gpadc-iio.c +6 -0

+					upper_32_bits(cell_data[0]));
+		case 4:
+		case 2:
+			info->has_calibration_data[0] = true;
+			info->calibration_data[0] = be32_to_cpu(
+					lower_32_bits(cell_data[0]));

Why do you need that switch?

Thanks!
Maxime

Quoted 32 lines

drivers/iio/adc/sun4i-gpadc-iio.c static const struct gpadc_data sun4i_gpadc_data = { · line 92

Quoted 6 lines

  static const struct gpadc_data sun4i_gpadc_data = {

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

@@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,

That's already its value if you leave it out.

Quoted 23 lines

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

  };
  static const struct gpadc_data sun5i_gpadc_data = {

@@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,
  };
  static const struct gpadc_data sun6i_gpadc_data = {

@@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,
  };
  static const struct gpadc_data sun8i_a33_gpadc_data = {

@@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
+	.supports_nvmem = false,
  };
  struct sun4i_gpadc_iio {

Quoted 8 lines

drivers/iio/adc/sun4i-gpadc-iio.c struct sun4i_gpadc_iio { · line 151

Quoted 6 lines

drivers/iio/adc/sun4i-gpadc-iio.c +2 -0

@@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
  	struct clk			*mod_clk;
  	struct reset_control		*reset;
  	int				sensor_id;
+	u32				calibration_data[2];
+	bool				has_calibration_data[2];

Why do you have two different values here?

I think my idea was too complex! I thought it would be better to check if calibration data was read, and is able to be written to hardware. those information were split per register.

I think a u64 should be fine for calibration_data. When I write the calibration data I can check on the sensor count and write only the lower 32 bits if there are less than 3 sensors.

Is this ok for you?

Quoted 21 lines

Quoted 21 lines

drivers/iio/adc/sun4i-gpadc-iio.c +8 -0

  	/* prevents concurrent reads of temperature and ADC */
  	struct mutex			mutex;
  	struct thermal_zone_device	*tzd;

@@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
  	struct resource *mem;
  	void __iomem *base;
  	int ret;
+	struct nvmem_cell *cell;
+	ssize_t cell_size;
+	u64 *cell_data;
  	info->data = of_device_get_match_data(&pdev->dev);
  	if (!info->data)

@@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
  	if (IS_ERR(base))
  		return PTR_ERR(base);
+	info->has_calibration_data[0] = false;
+	info->has_calibration_data[1] = false;
+
+	if (!info->data->supports_nvmem)
+		goto no_nvmem;

Quoted 8 lines

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe_dt(struct platform_device *pdev, · line 599

Quoted 6 lines

drivers/iio/adc/sun4i-gpadc-iio.c +6 -0

+
+	cell = nvmem_cell_get(&pdev->dev, "calibration");
+	if (IS_ERR(cell)) {
+		if (PTR_ERR(cell) == -EPROBE_DEFER)
+			return PTR_ERR(cell);
+		goto no_nvmem;

goto considered evil ? :)

this was a suggestion from Jonatan in version one, to make the code better readable.

Quoted 31 lines

Quoted 22 lines

Quoted 22 lines

   	/* prevents concurrent reads of temperature and ADC */
   	struct mutex			mutex;
   	struct thermal_zone_device	*tzd;

drivers/iio/adc/sun4i-gpadc-iio.c +8 -0

@@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
   	struct resource *mem;
   	void __iomem *base;
   	int ret;
+	struct nvmem_cell *cell;
+	ssize_t cell_size;
+	u64 *cell_data;
   	info->data = of_device_get_match_data(&pdev->dev);
   	if (!info->data)

@@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
   	if (IS_ERR(base))
   		return PTR_ERR(base);
+	info->has_calibration_data[0] = false;
+	info->has_calibration_data[1] = false;
+
+	if (!info->data->supports_nvmem)
+		goto no_nvmem;

Quoted 8 lines

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe_dt(struct platform_device *pdev, · line 599

Quoted 6 lines

drivers/iio/adc/sun4i-gpadc-iio.c +6 -0

+
+	cell = nvmem_cell_get(&pdev->dev, "calibration");
+	if (IS_ERR(cell)) {
+		if (PTR_ERR(cell) == -EPROBE_DEFER)
+			return PTR_ERR(cell);
+		goto no_nvmem;

goto considered evil ? :)

Quoted 2 lines

this was a suggestion from Jonatan in version one, to make the code better readable.

Isn't

if (info->data->supports_nvmem && IS_ERR(cell = nvmem_cell_get()))

pretty much the same thing?

Maxime

This patch rewors the driver to support interrupts for the thermal part of the sensor.

This is only available for the newer sensor (currently H3 and A83T). The interrupt will be trigerd on data available and triggers the update for the thermal sensors. All newer sensors have different amount of sensors and different interrupts for each device the reset of the interrupts need to be done different

For the newer sensors is the autosuspend disabled.

Signed-off-by Philipp Rossak <[email protected]>

Acked-by Jonathan Cameron <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
 include/linux/mfd/sun4i-gpadc.h   |  2 ++
 2 files changed, 56 insertions(+), 6 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index 74eeb5cd5218..b7b5451226b0 100644

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

 	bool		has_bus_clk;
 	bool		has_bus_rst;
 	bool		has_mod_clk;
 	int		sensor_count;
 	bool		supports_nvmem;
+	bool		support_irq;
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {

drivers/iio/adc/sun4i-gpadc-iio.c +7 -1

-	info->no_irq = true;
+	if (info->data->support_irq) {
+		/* only the new versions of ths support right now irqs */
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+			return irq;
+		}

drivers/iio/adc/sun4i-gpadc-iio.c +0 -2

 	indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
 	indio_dev->channels = sun8i_a33_gpadc_channels;

@@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pm_runtime_set_autosuspend_delay(&pdev->dev,
-					 SUN4I_GPADC_AUTOSUSPEND_DELAY);

Hi Philipp,

Quoted 18 lines · Philipp Rossak

This patch rewors the driver to support interrupts for the thermal part of the sensor.

This is only available for the newer sensor (currently H3 and A83T). The interrupt will be trigerd on data available and triggers the update for the thermal sensors. All newer sensors have different amount of sensors and different interrupts for each device the reset of the interrupts need to be done different

For the newer sensors is the autosuspend disabled.

Signed-off-by Philipp Rossak <[email protected]>

Acked-by Jonathan Cameron <[email protected]>

---
  drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
  include/linux/mfd/sun4i-gpadc.h   |  2 ++
  2 files changed, 56 insertions(+), 6 deletions(-)

drivers/iio/adc/sun4i-gpadc-iio.c struct gpadc_data { · line 75

Quoted 10 lines

drivers/iio/adc/sun4i-gpadc-iio.c +2 -0

index 74eeb5cd5218..b7b5451226b0 100644
@@ -71,11 +71,14 @@ struct gpadc_data {
  	unsigned int	temp_data[MAX_SENSOR_COUNT];
  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
+	u32		irq_clear_map;
+	u32		irq_control_map;

I would say to use a regmap_irq_chip for controlling IRQs.

Quoted 10 lines

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

  	bool		has_bus_clk;
  	bool		has_bus_rst;
  	bool		has_mod_clk;
  	int		sensor_count;
  	bool		supports_nvmem;
+	bool		support_irq;
  };
 
  static const struct gpadc_data sun4i_gpadc_data = {

drivers/iio/adc/sun4i-gpadc-iio.c static const struct gpadc_data sun4i_gpadc_data = { · line 96

Quoted 4 lines

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

@@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
  	.sample_end = sun4i_gpadc_sample_end,
  	.sensor_count = 1,
  	.supports_nvmem = false,
+	.support_irq = false,

False is the default, no need to set support_irq.

[...]

Quoted 5 lines

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

  struct sun4i_gpadc_iio {

@@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
  		return 0;
  	}
 

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val, · line 346

Quoted 5 lines

drivers/iio/adc/sun4i-gpadc-iio.c +5 -0

+	if (info->data->support_irq) {
+		regmap_read(info->regmap, info->data->temp_data[sensor], val);
+		return 0;
+	}
+

Maybe you could define a new thermal_zone_of_device_ops for these new thermal sensors? That way, you don't even need the boolean support_irq.

drivers/iio/adc/sun4i-gpadc-iio.c static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id) · line 444

Quoted 8 lines

drivers/iio/adc/sun4i-gpadc-iio.c +1 -0

  	return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
  }
 

@@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
  	return IRQ_HANDLED;
  }
 
+static irqreturn_t sunxi_irq_thread(int irq, void *data)

I think we're trying to avoid sunxi mentions but rather using the name of the first IP (in term of product release, not support) using this function.

drivers/iio/adc/sun4i-gpadc-iio.c static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id) · line 449

Quoted 5 lines

drivers/iio/adc/sun4i-gpadc-iio.c +5 -0

+{
+	struct sun4i_gpadc_iio *info = data;
+
+	regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
+

Will be handled by regmap_irq_chip.
[...]

Quoted 12 lines

drivers/iio/adc/sun4i-gpadc-iio.c +11 -1

-	info->no_irq = true;
+	if (info->data->support_irq) {
+		/* only the new versions of ths support right now irqs */
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+			return irq;
+		}
+
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+				sunxi_irq_thread, IRQF_ONESHOT,
+				dev_name(&pdev->dev), info);

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe_dt(struct platform_device *pdev, · line 620

Quoted 6 lines

drivers/iio/adc/sun4i-gpadc-iio.c +6 -0

+		if (ret)
+			return ret;
+
+	} else
+		info->no_irq = true;
+

That's a bit funny to have two booleans named no_irq and support_irq :)

Quoted 7 lines

  	indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
  	indio_dev->channels = sun8i_a33_gpadc_channels;

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

@@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
  	if (ret)
  		return ret;
 

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_probe(struct platform_device *pdev) · line 838

Quoted 12 lines

drivers/iio/adc/sun4i-gpadc-iio.c +7 -5

-	pm_runtime_set_autosuspend_delay(&pdev->dev,
-					 SUN4I_GPADC_AUTOSUSPEND_DELAY);
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_suspended(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
+	if (!info->data->support_irq) {
+		pm_runtime_set_autosuspend_delay(&pdev->dev,
+						 SUN4I_GPADC_AUTOSUSPEND_DELAY);
+		pm_runtime_use_autosuspend(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}

Why would you not want your IP to do runtime PM?

This patch adds support for the H3 ths sensor.

The H3 supports interrupts. The interrupt is configured to update the the sensor values every second. The calibration data is writen at the begin of the init process.

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
 2 files changed, 108 insertions(+)

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

index b7b5451226b0..8196203d65fe 100644
@@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
 static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
 static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
 
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
+
 struct gpadc_data {
 	int		temp_offset;
 	int		temp_scale;

drivers/iio/adc/sun4i-gpadc-iio.c +46 -0

 	u32		irq_clear_map;
 	u32		irq_control_map;
 	bool		has_bus_clk;

@@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.support_irq = false,
 };
 
+static const struct gpadc_data sun8i_h3_ths_data = {
+	.temp_offset = -1791,
+	.temp_scale = -121,
+	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
+	.sample_start = sunxi_ths_sample_start,
+	.sample_end = sunxi_ths_sample_end,
+	.has_bus_clk = true,
+	.has_bus_rst = true,
+	.has_mod_clk = true,
+	.sensor_count = 1,
+	.supports_nvmem = true,
+	.support_irq = true,
+	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
+	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
+	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
+	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+			SUN8I_H3_THS_INTS_SHUT_INT_0   |
+			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
+			SUN8I_H3_THS_INTS_ALARM_OFF_0,
+	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+		SUN8I_H3_THS_TEMP_PERIOD(0x7),
+};
+
 struct sun4i_gpadc_iio {
 	struct iio_dev			*indio_dev;
 	struct completion		completion;

@@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
 	return 0;
 }
 
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
+{
+	/* Disable ths interrupt */
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
+	/* Disable temperature sensor */
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
+
+	return 0;
+}
+
 static int sun4i_gpadc_runtime_suspend(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));

@@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
 	return info->data->sample_end(info);
 }
 
+static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
+{
+	if (info->has_calibration_data[0])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
+			info->calibration_data[0]);
+
+	if (info->has_calibration_data[1])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
+			info->calibration_data[1]);
+}
+
 static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
 {
 	/* clkin = 6MHz */

This patch adds support for the H3 ths sensor.

The H3 supports interrupts. The interrupt is configured to update the the sensor values every second. The calibration data is writen at the begin of the init process.

Signed-off-by Philipp Rossak <[email protected]>

---
 drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
 2 files changed, 108 insertions(+)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index b7b5451226b0..8196203d65fe 100644

drivers/iio/adc/sun4i-gpadc-iio.c +27 -0

 struct gpadc_data {
 	int		temp_offset;
 	int		temp_scale;

@@ -71,6 +74,10 @@ struct gpadc_data {
 	unsigned int	temp_data[MAX_SENSOR_COUNT];
 	int		(*sample_start)(struct sun4i_gpadc_iio *info);
 	int		(*sample_end)(struct sun4i_gpadc_iio *info);
+	u32		ctrl0_map;
+	u32		ctrl2_map;
+	u32		sensor_en_map;
+	u32		filter_map;
 	u32		irq_clear_map;
 	u32		irq_control_map;
 	bool		has_bus_clk;

@@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.support_irq = false,
 };
 
+static const struct gpadc_data sun8i_h3_ths_data = {
+	.temp_offset = -1791,
+	.temp_scale = -121,
+	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
+	.sample_start = sunxi_ths_sample_start,
+	.sample_end = sunxi_ths_sample_end,
+	.has_bus_clk = true,
+	.has_bus_rst = true,
+	.has_mod_clk = true,
+	.sensor_count = 1,
+	.supports_nvmem = true,
+	.support_irq = true,
+	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
+	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
+	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
+	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+			SUN8I_H3_THS_INTS_SHUT_INT_0   |
+			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
+			SUN8I_H3_THS_INTS_ALARM_OFF_0,
+	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+		SUN8I_H3_THS_TEMP_PERIOD(0x7),
 

From what I've understood, ACQ regs are basically clock dividers. We

drivers/iio/adc/sun4i-gpadc-iio.c +42 -0

+};
+
 struct sun4i_gpadc_iio {
 	struct iio_dev			*indio_dev;
 	struct completion		completion;

@@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
 	return 0;
 }
 
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
+{
+	/* Disable ths interrupt */
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
+	/* Disable temperature sensor */
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
+
+	return 0;
+}
+
 static int sun4i_gpadc_runtime_suspend(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));

@@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
 	return info->data->sample_end(info);
 }
 
+static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
+{
+	if (info->has_calibration_data[0])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
+			info->calibration_data[0]);
+
+	if (info->has_calibration_data[1])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
+			info->calibration_data[1]);
+}
+
 static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
 {
 	/* clkin = 6MHz */

@@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
 	return 0;
 }
 
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
+{
+	u32 value;
+	sunxi_calibrate(info);
+
+	if (info->data->ctrl0_map)
+		regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
+			info->data->ctrl0_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+		info->data->ctrl2_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+			info->data->irq_clear_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
+		info->data->filter_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC,

drivers/iio/adc/sun4i-gpadc-iio.c +4 -0

 static int sun4i_gpadc_runtime_resume(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));

@@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
 		.compatible = "allwinner,sun8i-a33-ths",
 		.data = &sun8i_a33_gpadc_data,
 	},
+	{
+		.compatible = "allwinner,sun8i-h3-ths",
+		.data = &sun8i_h3_ths_data,
+	},
 	{ /* sentinel */ }
 };
 

include/linux/mfd/sun4i-gpadc.h +13 -0

index 458f2159a95a..80b79c31cea3 100644
@@ -91,7 +91,29 @@
 #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
 
 /* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_CTRL0				0x00
+#define SUN8I_H3_THS_CTRL2				0x40
 #define SUN8I_H3_THS_INTC				0x44
+#define SUN8I_H3_THS_STAT				0x48
+#define SUN8I_H3_THS_FILTER				0x70
+#define SUNXI_THS_CDATA_0_1				0x74
+#define SUNXI_THS_CDATA_2_3				0x78
+#define SUN8I_H3_THS_TDATA0				0x80
+
+#define SUN8I_H3_THS_ACQ1(x)				(GENMASK(31, 16) & ((x) << 16))
+
+#define SUN8I_H3_THS_TEMP_SENSE_EN0			BIT(0)
+
+#define SUN8I_H3_THS_TEMP_PERIOD(x)			(GENMASK(31, 12) & ((x) << 12))

Hi Philipp,

Quoted 17 lines · Philipp Rossak

This patch adds support for the H3 ths sensor.

The H3 supports interrupts. The interrupt is configured to update the the sensor values every second. The calibration data is writen at the begin of the init process.

Signed-off-by Philipp Rossak <[email protected]>

---
  drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
  include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
  2 files changed, 108 insertions(+)

drivers/iio/adc/sun4i-gpadc-iio.c +0 -0

index b7b5451226b0..8196203d65fe 100644

drivers/iio/adc/sun4i-gpadc-iio.c struct sun4i_gpadc_iio; · line 66

Quoted 6 lines

drivers/iio/adc/sun4i-gpadc-iio.c +3 -0

@@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
  static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
  static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
 
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
+

We try to avoid using the generic sunxi prefix.

Quoted 36 lines

drivers/iio/adc/sun4i-gpadc-iio.c +22 -0

  struct gpadc_data {
  	int		temp_offset;
  	int		temp_scale;

@@ -71,6 +74,10 @@ struct gpadc_data {
  	unsigned int	temp_data[MAX_SENSOR_COUNT];
  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
+	u32		ctrl0_map;
+	u32		ctrl2_map;
+	u32		sensor_en_map;
+	u32		filter_map;
  	u32		irq_clear_map;
  	u32		irq_control_map;
  	bool		has_bus_clk;

@@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
  	.support_irq = false,
  };
 
+static const struct gpadc_data sun8i_h3_ths_data = {
+	.temp_offset = -1791,
+	.temp_scale = -121,
+	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
+	.sample_start = sunxi_ths_sample_start,
+	.sample_end = sunxi_ths_sample_end,
+	.has_bus_clk = true,
+	.has_bus_rst = true,
+	.has_mod_clk = true,
+	.sensor_count = 1,
+	.supports_nvmem = true,
+	.support_irq = true,
+	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
+	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
+	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
+	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |

drivers/iio/adc/sun4i-gpadc-iio.c static const struct gpadc_data sun8i_a33_gpadc_data = { · line 170

Quoted 5 lines

drivers/iio/adc/sun4i-gpadc-iio.c +5 -0

+			SUN8I_H3_THS_INTS_SHUT_INT_0   |
+			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
+			SUN8I_H3_THS_INTS_ALARM_OFF_0,
+	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+		SUN8I_H3_THS_TEMP_PERIOD(0x7),

From what I've understood, ACQ regs are basically clock dividers. We
should make a better job at explaining it :)

Quoted 63 lines

drivers/iio/adc/sun4i-gpadc-iio.c +42 -0

+};
+
  struct sun4i_gpadc_iio {
  	struct iio_dev			*indio_dev;
  	struct completion		completion;

@@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
  	return 0;
  }
 
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
+{
+	/* Disable ths interrupt */
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
+	/* Disable temperature sensor */
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
+
+	return 0;
+}
+
  static int sun4i_gpadc_runtime_suspend(struct device *dev)
  {
  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));

@@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
  	return info->data->sample_end(info);
  }
 
+static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
+{
+	if (info->has_calibration_data[0])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
+			info->calibration_data[0]);
+
+	if (info->has_calibration_data[1])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
+			info->calibration_data[1]);
+}
+
  static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
  {
  	/* clkin = 6MHz */

@@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
  	return 0;
  }
 
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
+{
+	u32 value;
+	sunxi_calibrate(info);
+
+	if (info->data->ctrl0_map)
+		regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
+			info->data->ctrl0_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+		info->data->ctrl2_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+			info->data->irq_clear_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
+		info->data->filter_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC,

drivers/iio/adc/sun4i-gpadc-iio.c static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info) · line 576

Quoted 10 lines

drivers/iio/adc/sun4i-gpadc-iio.c +10 -0

+		info->data->irq_control_map);
+
+	regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+		info->data->sensor_en_map | value);
+
+	return 0;
+}
+

I'm a bit worried. Will all the sensors have the same sample start? Or will we need at some point also entries in info->data for register addresses, if they have ctrl2 or filter, etc...

Quoted 1 line

Maybe we could define a sample_start for the h3 only and reuse-it if

possible and if not declare another sample start? All depends if the sample start is most likely to change in the near future or not. If it is, then we should avoid adding a lot of variables in info->data and just go for a single sample_start per SoC.

Quoted 36 lines

  static int sun4i_gpadc_runtime_resume(struct device *dev)
  {
  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));

drivers/iio/adc/sun4i-gpadc-iio.c +4 -0

@@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
  		.compatible = "allwinner,sun8i-a33-ths",
  		.data = &sun8i_a33_gpadc_data,
  	},
+	{
+		.compatible = "allwinner,sun8i-h3-ths",
+		.data = &sun8i_h3_ths_data,
+	},
  	{ /* sentinel */ }
  };
 

include/linux/mfd/sun4i-gpadc.h +13 -0

index 458f2159a95a..80b79c31cea3 100644
@@ -91,7 +91,29 @@
  #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
 
  /* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_CTRL0				0x00
+#define SUN8I_H3_THS_CTRL2				0x40
  #define SUN8I_H3_THS_INTC				0x44
+#define SUN8I_H3_THS_STAT				0x48
+#define SUN8I_H3_THS_FILTER				0x70
+#define SUNXI_THS_CDATA_0_1				0x74
+#define SUNXI_THS_CDATA_2_3				0x78
+#define SUN8I_H3_THS_TDATA0				0x80
+
+#define SUN8I_H3_THS_ACQ1(x)				(GENMASK(31, 16) & ((x) << 16))
+
+#define SUN8I_H3_THS_TEMP_SENSE_EN0			BIT(0)
+
+#define SUN8I_H3_THS_TEMP_PERIOD(x)			(GENMASK(31, 12) & ((x) << 12))

include/linux/mfd/sun4i-gpadc.h line 117

Quoted 10 lines

include/linux/mfd/sun4i-gpadc.h +9 -0

+
+#define SUN8I_H3_THS_INTS_ALARM_INT_0			BIT(0)
+#define SUN8I_H3_THS_INTS_SHUT_INT_0			BIT(4)
+#define SUN8I_H3_THS_INTS_TDATA_IRQ_0			BIT(8)
+#define SUN8I_H3_THS_INTS_ALARM_OFF_0			BIT(12)
+
+#define SUN8I_H3_THS_INTC_ALARM_INT_EN0			BIT(0)
+#define SUN8I_H3_THS_INTC_SHUT_INT_EN0			BIT(4)
+#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0			BIT(8)
 

Personal taste here but I prefer the register bits to be defined just below the register address define.

i.e.:

#define SUN8I_H3_THS_CTRL2                           0x40
#define SUN8I_H3_THS_ACQ1(x)                         (GENMASK(31, 16) & ((x) << 16))
#define SUN8I_H3_THS_TEMP_SENSE_EN0                  BIT(0)

that way we know for which register we should use which constants.

Quentin