[linux-sunxi] [PATCH v2 00/16] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

[Philipp Rossak]

Allwiner H3 and A83T SoCs have a thermal sensor, which is a large refactored
version of the old Allwinner "GPADC" (although it have already only
thermal part left in A33).

This patch tried to add support for the sensor in H3 and A83T based on
the A33 thermal sensor driver by Quentin Schulz, which is already merged.

This Patchseries is based on Icenowy Zhengs v4 patchseries [1].
The first 5 patches are reworked patches from the v4 patchseries.
The rest of the patches add step by step a feature to support multible
sensors, nvmem calibration and interupts. This patchseries should make it
easy also to add other sunxi SoCs, like the H5, A64 and A80.

Patches that adds support for H5, A64 and A80 SoCs are allready prepared,
and will be upstreamed if this patchseries is applied and the testing is done.

I tried to pick up all the feedback from [1]. I hope I didn't miss anything.

Regards,
Philipp

@Jonathan
Could you please check Patch 8 again. I moved some code from
Patch 8 to 9. Please chek it again, if your still ok with it.
But I think it sould be ok.

changes since v1:
* collecting all acks
* rewording commits/fix typos
* move code in place where it is used
* fix naming conventions of defines
* clarify commits
* update documentation to cover the new nvmem calibraion
* change nvmem calibration

Icenowy Zheng (1):
iio: adc: sun4i-gpadc-iio: rename A33-specified registers to contain
A33

Philipp Rossak (15):
dt-bindings: update the Allwinner GPADC device tree binding for H3 &
A83T
arm: config: sunxi_defconfig: enable SUN4I_GPADC
iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg
iio: adc: sun4i-gpadc-iio: rework: support clocks and reset
iio: adc: sun4i-gpadc-iio: rework: support multiple sensors
iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data
iio: adc: sun4i-gpadc-iio: rework: add interrupt support
iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor
arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5
arm: dts: sun8i: h3: add support for the thermal sensor in H3
arm: dts: sun8i: h3: add thermal zone to H3
arm: dts: sun8i: h3: enable H3 sid controller
arm: dts: sun8i: a83t: add support for the thermal sensor in A83T
arm: dts: sun8i: a83t: add thermal zone to A83T

.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++-
arch/arm/boot/dts/sun8i-a83t.dtsi | 28 ++
arch/arm/boot/dts/sun8i-h3.dtsi | 21 ++
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +
arch/arm/configs/sunxi_defconfig | 1 +
drivers/iio/adc/sun4i-gpadc-iio.c | 367 ++++++++++++++++++++-
include/linux/mfd/sun4i-gpadc.h | 51 ++-
7 files changed, 503 insertions(+), 24 deletions(-)

[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 <***@gmail.com>
---
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index 86dd8191b04c..22df0c5c23d4 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -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: ***@1c25000 {
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
@@ -17,6 +40,27 @@ Example:
#io-channel-cells = <0>;
};

+Example for H3:
+ ths: thermal-***@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>;
+ };
+
+Example for A83T:
+ ths: thermal-***@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>;
+ };
+
sun4i, sun5i and sun6i SoCs are also supported via the older binding:

sun4i resistive touchscreen controller

[Maxime Ripard]

Hi Philipp,

Post by 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.

Thanks a lot for tackling this.

Post by Philipp Rossak
---
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index 86dd8191b04c..22df0c5c23d4 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -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.
- - compatible: "allwinner,sun8i-a33-ths",
+ - "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,
+ - "allwinner,sun8i-h3-ths"
+ - "allwinner,sun8i-a83t-ths"
+ - interrupts: the sampling interrupt of the ADC,
+
+ - "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,
+
+ - "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
+
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
#io-channel-cells = <0>;
};
+ 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>;
+ };
+
+ 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

[Philipp Rossak]

Post by Maxime Ripard

Post by Philipp Rossak
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
#io-channel-cells = <0>;
};
+ 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>;
+ };
+
+ 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
- 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

I think this is a good idea, and the desired way to rework the driver.

To sum up what we talked on IRC:

This will end up in removing the MFD driver and moving the interrupt
handling into the iio driver. At the end this will also simplify the IRQ
part.

Philipp

[Quentin Schulz]

Hi Philipp,

Post by 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.
---
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index 86dd8191b04c..22df0c5c23d4 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -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.
- - compatible: "allwinner,sun8i-a33-ths",
+ - "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.

Post by Philipp Rossak
- #io-channel-cells: shall be 0,
+ - "allwinner,sun8i-h3-ths"
+ - "allwinner,sun8i-a83t-ths"
+ - interrupts: the sampling interrupt of the ADC,
+
+ - "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,
+
+ - "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" ?

Post by Philipp Rossak
+ - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
#io-channel-cells = <0>;
};
+ 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>;
+ };
+
+ 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

[Philipp Rossak]

Post by Maxime Ripard
Hi Philipp,

Post by 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.
---
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index 86dd8191b04c..22df0c5c23d4 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -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.
- - compatible: "allwinner,sun8i-a33-ths",
+ - "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.

I agree, I will change this in the next version.

Post by Maxime Ripard

Post by Philipp Rossak
- #io-channel-cells: shall be 0,
+ - "allwinner,sun8i-h3-ths"
+ - "allwinner,sun8i-a83t-ths"
+ - interrupts: the sampling interrupt of the ADC,
+
+ - "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,
+
+ - "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" ?

As already mentioned in an other answers, this here is not correct.
I got somehow a wrong information or mixed something up. For H5, H3,
A83T and A64 the thermal sensor calibration data is always 64 bit wide
and placed on the eFuse address 0x34 [1].

Post by Maxime Ripard

Post by Philipp Rossak
+ - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
#io-channel-cells = <0>;
};
+ 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>;
+ };
+
+ 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

I agree we don't want to have an example for every sensor, but I think
at least those two are interesting, since one has 3 sensors and no
clocks, and one has 1 sensor and clocks.

Thanks,
Philipp

[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE

[Philipp Rossak]

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 <***@gmail.com>
---
arch/arm/configs/sunxi_defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 5caaf971fb50..52c3990b9d91 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -131,6 +131,7 @@ CONFIG_DMA_SUN6I=y
CONFIG_EXTCON=y
CONFIG_IIO=y
CONFIG_AXP20X_ADC=y
+CONFIG_SUN4I_GPADC=y
CONFIG_PWM=y
CONFIG_PWM_SUN4I=y
CONFIG_PHY_SUN4I_USB=y

[Maxime Ripard]

Hi,

Post by Philipp Rossak
This commit enables the SUN4I_GPADC config option and
sets the value to yes. This is needed to enable the ths sensors.
---
arch/arm/configs/sunxi_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 5caaf971fb50..52c3990b9d91 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -131,6 +131,7 @@ CONFIG_DMA_SUN6I=y
CONFIG_EXTCON=y
CONFIG_IIO=y
CONFIG_AXP20X_ADC=y
+CONFIG_SUN4I_GPADC=y

Unfortunately, we can't do that since TOUCHSCREEN_SUN4I is already
enabled and we have a depends on !TOUCHSCREEN_SUN4I.

One more reason to stop worrying about replacing that driver DT
binding.

Maxime

[Philipp Rossak]

From: Icenowy Zheng <***@aosc.io>

As the H3 SoC, which is also in sun8i line, has totally different
register map for the thermal sensor (a cut down version of GPADC), we
should rename A23/A33-specified registers to contain A33, in order to
prevent obfuscation with H3 registers. Currently these registers are
only prefixed "SUN8I", not "SUN8I_A33".

Add "_A33" after "SUN8I" on the register names.

Signed-off-by: Icenowy Zheng <***@aosc.io>
Reviewed-by: Chen-Yu Tsai <***@csie.org>
Acked-by: Maxime Ripard <***@free-electrons.com>
Acked-by: Lee Jones <***@linaro.org>
Acked-by: Jonathan Cameron <***@huawei.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
include/linux/mfd/sun4i-gpadc.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 04d7147e0110..03804ff9c006 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -88,7 +88,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
static const struct gpadc_data sun8i_a33_gpadc_data = {
.temp_offset = -1662,
.temp_scale = 162,
- .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
+ .tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
};

struct sun4i_gpadc_iio {
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 139872c2e0fe..78d31984a222 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -38,9 +38,9 @@
#define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x) (GENMASK(3, 0) & BIT(x))
#define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK GENMASK(3, 0)

-/* TP_CTRL1 bits for sun8i SoCs */
-#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
-#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
+/* TP_CTRL1 bits for A33 */
+#define SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
+#define SUN8I_A33_GPADC_CTRL1_GPADC_CALI_EN BIT(7)

#define SUN4I_GPADC_CTRL2 0x08

[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 <***@aosc.io>
Signed-off-by: Philipp Rossak <***@gmail.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 03804ff9c006..db57d9fffe48 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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 = {
@@ -65,6 +77,9 @@ 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,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};

static const struct gpadc_data sun5i_gpadc_data = {
@@ -74,6 +89,9 @@ 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,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};

static const struct gpadc_data sun6i_gpadc_data = {
@@ -83,12 +101,18 @@ 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,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};

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,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};

struct sun4i_gpadc_iio {
@@ -277,7 +301,7 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
if (info->no_irq) {
pm_runtime_get_sync(indio_dev->dev.parent);

- regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
+ regmap_read(info->regmap, info->data->temp_data, val);

pm_runtime_mark_last_busy(indio_dev->dev.parent);
pm_runtime_put_autosuspend(indio_dev->dev.parent);
@@ -382,10 +406,8 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static int sun4i_gpadc_runtime_suspend(struct device *dev)
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
{
- struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
-
/* Disable the ADC on IP */
regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
/* Disable temperature sensor on IP */
@@ -394,10 +416,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
return 0;
}

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

+ return info->data->sample_end(info);
+}
+
+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
+{
/* clkin = 6MHz */
regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
@@ -415,6 +442,13 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
return 0;
}

+static int sun4i_gpadc_runtime_resume(struct device *dev)
+{
+ struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+
+ return info->data->sample_start(info);
+}
+
static int sun4i_gpadc_get_temp(void *data, int *temp)
{
struct sun4i_gpadc_iio *info = data;

[Maxime Ripard]

Hi,

Post by 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.

That signed-off-by chain doesn't really make much sense. If Icenowy is
the author, she should be reported as such in the commit, and if
you're the author, you shouldn't have her Signed-off-by.

Post by Philipp Rossak
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 03804ff9c006..db57d9fffe48 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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 = {
@@ -65,6 +77,9 @@ 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,

You can use a regmap_field there.

Thanks!
Maxime

[Quentin Schulz]

Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 03804ff9c006..db57d9fffe48 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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.

Post by Philipp Rossak
+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 <***@free-electrons.com>

Quentin

[Philipp Rossak]

Post by Maxime Ripard
Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 03804ff9c006..db57d9fffe48 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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.

Post by Philipp Rossak
+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,
Quentin

Ok I will split this.

Thanks,
Philipp

[Philipp Rossak]

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 <***@gmail.com>
Signed-off-by: Icenowy Zheng <***@aosc.io>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index db57d9fffe48..51ec0104d678 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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>

@@ -68,6 +70,9 @@ struct gpadc_data {
unsigned int temp_data;
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;
};

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;
@@ -420,6 +428,10 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));

+ clk_disable(info->mod_clk);
+
+ clk_disable(info->bus_clk);
+
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;
+ }
+
+ if (info->data->has_mod_clk) {
+ info->mod_clk = devm_clk_get(&pdev->dev, "mod");
+ if (IS_ERR(info->mod_clk)) {
+ ret = PTR_ERR(info->mod_clk);
+ goto disable_bus_clk;
+ }
+
+ /* Running at 6MHz */
+ ret = clk_set_rate(info->mod_clk, 4000000);
+ if (ret)
+ goto disable_bus_clk;
+
+ ret = clk_prepare_enable(info->mod_clk);
+ if (ret)
+ goto disable_bus_clk;
+ }
+
if (IS_ENABLED(CONFIG_THERMAL_OF))
info->sensor_device = &pdev->dev;

return 0;
+
+disable_bus_clk:
+ clk_disable_unprepare(info->bus_clk);
+
+assert_reset:
+ reset_control_assert(info->reset);
+
+ return ret;
}

static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
@@ -729,6 +794,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
if (!info->no_irq)
iio_map_array_unregister(indio_dev);

+ clk_disable_unprepare(info->mod_clk);
+
+ clk_disable_unprepare(info->bus_clk);
+
+ reset_control_assert(info->reset);
+
return 0;
}

[Maxime Ripard]

Post by Philipp Rossak
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.

Same remark for the SoB

Post by Philipp Rossak
---
drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index db57d9fffe48..51ec0104d678 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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>
@@ -68,6 +70,9 @@ struct gpadc_data {
unsigned int temp_data;
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;

Is there SoCs where this insn't all true, or all false?

Post by Philipp Rossak
};
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;
@@ -420,6 +428,10 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+ clk_disable(info->mod_clk);
+

Why clk_disable?

Post by Philipp Rossak
+ clk_disable(info->bus_clk);
+

You can tie the bus_clk to the regmap directly, instead of having to
maintain it yourself here.

And you can probably put the device in reset and out of reset here as
well.

Post by Philipp Rossak
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;
+ }
+
+ if (info->data->has_mod_clk) {
+ info->mod_clk = devm_clk_get(&pdev->dev, "mod");
+ if (IS_ERR(info->mod_clk)) {
+ ret = PTR_ERR(info->mod_clk);
+ goto disable_bus_clk;
+ }
+
+ /* Running at 6MHz */
+ ret = clk_set_rate(info->mod_clk, 4000000);

Your comment and the line below doesn't really make much sense :)

Maxime

[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 <***@gmail.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
include/linux/mfd/sun4i-gpadc.h | 3 +++
2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 51ec0104d678..ac9ad2f8232f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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);

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);
+ }
/*
* Do not fail driver probing when failing to register in
* thermal because no thermal DT node is found.
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 78d31984a222..20fa02040007 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -90,6 +90,9 @@
/* 10s delay before suspending the IP */
#define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000

+/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define MAX_SENSOR_COUNT 4
+
struct sun4i_gpadc_dev {
struct device *dev;
struct regmap *regmap;

[Maxime Ripard]

Hi,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
include/linux/mfd/sun4i-gpadc.h | 3 +++
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 51ec0104d678..ac9ad2f8232f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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);
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);
+ }

I'm not sure how that works. Isn't the info structure shared between
all the sensors? The sensor_id value would be always set to the last
sensor then.

Thanks!
Maxime

[Quentin Schulz]

Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
include/linux/mfd/sun4i-gpadc.h | 3 +++
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 51ec0104d678..ac9ad2f8232f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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;
};

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.

Post by Philipp Rossak
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.

[...]

Post by Philipp Rossak
@@ -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

[Philipp Rossak]

Post by Maxime Ripard
Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
include/linux/mfd/sun4i-gpadc.h | 3 +++
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 51ec0104d678..ac9ad2f8232f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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;
};

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.

This sounds like a good idea! I will add this to the next version.

I can verify this with a table, I created during development. I will
upload it during the weekend here: [1]

Post by Maxime Ripard

Post by Philipp Rossak
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
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.
[...]

Post by Philipp Rossak
@@ -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.
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

I will have a closer look on this next week, when I start to work on the
next version..

Thanks,
Philipp

[1]: http://linux-sunxi.org/Thermal_Sensor

[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 <***@gmail.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index ac9ad2f8232f..74eeb5cd5218 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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>
@@ -74,6 +75,7 @@ struct gpadc_data {
bool has_bus_rst;
bool has_mod_clk;
int sensor_count;
+ bool supports_nvmem;
};

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,
};

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 {
@@ -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];
/* 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;
+
+ cell = nvmem_cell_get(&pdev->dev, "calibration");
+ if (IS_ERR(cell)) {
+ if (PTR_ERR(cell) == -EPROBE_DEFER)
+ return PTR_ERR(cell);
+ goto no_nvmem;
+ }
+
+ 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(
+ 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]));
+ break;
+ default:
+ break;
+ }
+
+no_nvmem:
+
info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
&sun4i_gpadc_regmap_config);
if (IS_ERR(info->regmap)) {

[Maxime Ripard]

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index ac9ad2f8232f..74eeb5cd5218 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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>
@@ -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.

Post by Philipp Rossak
};
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.

Post by Philipp Rossak
};
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 {
@@ -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?

Post by Philipp Rossak
/* 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;
+
+ 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 ? :)

Post by Philipp Rossak
+ }
+
+ cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
+ nvmem_cell_put(cell);
+ switch (cell_size) {
+ info->has_calibration_data[1] = true;
+ info->calibration_data[1] = be32_to_cpu(
+ upper_32_bits(cell_data[0]));
+ 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

[Philipp Rossak]

Post by Maxime Ripard

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index ac9ad2f8232f..74eeb5cd5218 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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>
@@ -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.

ok I will add more informations in the next version into the commit message.

Post by Maxime Ripard

Post by Philipp Rossak
};
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.

Post by Philipp Rossak
};
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 {
@@ -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?

Post by Maxime Ripard

Post by Philipp Rossak
/* 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;
+
+ 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.
.

Post by Maxime Ripard

Post by Philipp Rossak
+ }
+
+ cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
+ nvmem_cell_put(cell);
+ switch (cell_size) {
+ info->has_calibration_data[1] = true;
+ info->calibration_data[1] = be32_to_cpu(
+ upper_32_bits(cell_data[0]));
+ 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?

You are right! The calibration reg seems to be always 64 bit wide. [1]
So I will just check for the length of 8.

Post by Maxime Ripard
Thanks!
Maxime

[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE

Thanks,
Philipp

[Maxime Ripard]

Post by Maxime Ripard

Post by Philipp Rossak
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.

Post by Philipp Rossak
};
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 {
@@ -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?

I'd need to see the implementation, but make sure that this is
documented in your driver :)

Post by Maxime Ripard

Post by Philipp Rossak
/* 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;
+
+ 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.

Isn't

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

pretty much the same thing?

Maxime

[Philipp Rossak]

Post by Maxime Ripard

Post by Maxime Ripard

Post by Philipp Rossak
/* 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;
+
+ 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.

Isn't
if (info->data->supports_nvmem && IS_ERR(cell = nvmem_cell_get()))
pretty much the same thing?
Maxime

I would say :

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

the same.
This would require an else if statement like this:

else if (info->data->supports_nvmem && PTR_ERR(cell) == -EPROBE_DEFER)
return PTR_ERR(cell);

to avoid errors if the thermal sensor is probed before the sid driver.

Philipp

[kbuild test robot]

Hi Philipp,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32

drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32

vim +608 drivers/iio/adc/sun4i-gpadc-iio.c

564
565 static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
566 struct iio_dev *indio_dev)
567 {
568 struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
569 struct resource *mem;
570 void __iomem *base;
571 int ret;
572 struct nvmem_cell *cell;
573 ssize_t cell_size;
574 u64 *cell_data;
575
576 info->data = of_device_get_match_data(&pdev->dev);
577 if (!info->data)
578 return -ENODEV;
579
580 info->no_irq = true;
581 indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
582 indio_dev->channels = sun8i_a33_gpadc_channels;
583
584 mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
585 base = devm_ioremap_resource(&pdev->dev, mem);
586 if (IS_ERR(base))
587 return PTR_ERR(base);
588
589 info->has_calibration_data[0] = false;
590 info->has_calibration_data[1] = false;
591
592 if (!info->data->supports_nvmem)
593 goto no_nvmem;
594
595 cell = nvmem_cell_get(&pdev->dev, "calibration");
596 if (IS_ERR(cell)) {
597 if (PTR_ERR(cell) == -EPROBE_DEFER)
598 return PTR_ERR(cell);
599 goto no_nvmem;
600 }
601
602 cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
603 nvmem_cell_put(cell);
604 switch (cell_size) {
605 case 8:
606 case 6:
607 info->has_calibration_data[1] = true;

608 info->calibration_data[1] = be32_to_cpu(

609 upper_32_bits(cell_data[0]));
610 case 4:
611 case 2:
612 info->has_calibration_data[0] = true;
613 info->calibration_data[0] = be32_to_cpu(
614 lower_32_bits(cell_data[0]));
615 break;
616 default:
617 break;
618 }
619
620 no_nvmem:
621
622 info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
623 &sun4i_gpadc_regmap_config);
624 if (IS_ERR(info->regmap)) {
625 ret = PTR_ERR(info->regmap);
626 dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
627 return ret;
628 }
629
630 if (info->data->has_bus_rst) {
631 info->reset = devm_reset_control_get(&pdev->dev, NULL);
632 if (IS_ERR(info->reset)) {
633 ret = PTR_ERR(info->reset);
634 return ret;
635 }
636
637 ret = reset_control_deassert(info->reset);
638 if (ret)
639 return ret;
640 }
641
642 if (info->data->has_bus_clk) {
643 info->bus_clk = devm_clk_get(&pdev->dev, "bus");
644 if (IS_ERR(info->bus_clk)) {
645 ret = PTR_ERR(info->bus_clk);
646 goto assert_reset;
647 }
648
649 ret = clk_prepare_enable(info->bus_clk);
650 if (ret)
651 goto assert_reset;
652 }
653
654 if (info->data->has_mod_clk) {
655 info->mod_clk = devm_clk_get(&pdev->dev, "mod");
656 if (IS_ERR(info->mod_clk)) {
657 ret = PTR_ERR(info->mod_clk);
658 goto disable_bus_clk;
659 }
660
661 /* Running at 6MHz */
662 ret = clk_set_rate(info->mod_clk, 4000000);
663 if (ret)
664 goto disable_bus_clk;
665
666 ret = clk_prepare_enable(info->mod_clk);
667 if (ret)
668 goto disable_bus_clk;
669 }
670
671 if (IS_ENABLED(CONFIG_THERMAL_OF))
672 info->sensor_device = &pdev->dev;
673
674 return 0;
675
676 disable_bus_clk:
677 clk_disable_unprepare(info->bus_clk);
678
679 assert_reset:
680 reset_control_assert(info->reset);
681
682 return ret;
683 }
684

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

[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 <***@gmail.com>
Acked-by: Jonathan Cameron <***@huawei.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
include/linux/mfd/sun4i-gpadc.h | 2 ++
2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 74eeb5cd5218..b7b5451226b0 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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;
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 = {
@@ -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,
};

static const struct gpadc_data sun5i_gpadc_data = {
@@ -104,6 +108,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};

static const struct gpadc_data sun6i_gpadc_data = {
@@ -118,6 +123,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};

static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -129,6 +135,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};

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

+ if (info->data->support_irq) {
+ regmap_read(info->regmap, info->data->temp_data[sensor], val);
+ return 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)
+{
+ struct sun4i_gpadc_iio *info = data;
+
+ regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
+
+ thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE);
+
+ return IRQ_HANDLED;
+}
+
static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
{
/* Disable the ADC on IP */
@@ -572,12 +595,29 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
struct nvmem_cell *cell;
ssize_t cell_size;
u64 *cell_data;
+ int irq;

info->data = of_device_get_match_data(&pdev->dev);
if (!info->data)
return -ENODEV;

- 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);
+ if (ret)
+ return ret;
+
+ } else
+ info->no_irq = true;
+
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);
- 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);
+ }

if (IS_ENABLED(CONFIG_THERMAL_OF)) {
for (i = 0; i < info->data->sensor_count; i++) {
@@ -814,6 +856,9 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
}
}

+ if (info->data->support_irq)
+ info->data->sample_start(info);
+
ret = devm_iio_device_register(&pdev->dev, indio_dev);
if (ret < 0) {
dev_err(&pdev->dev, "could not register the device\n");
@@ -843,6 +888,9 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
if (!IS_ENABLED(CONFIG_THERMAL_OF))
return 0;

+ if (info->data->support_irq)
+ info->data->sample_end(info);
+
thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);

if (!info->no_irq)
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 20fa02040007..458f2159a95a 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -91,6 +91,8 @@
#define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000

/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_INTC 0x44
+
#define MAX_SENSOR_COUNT 4

struct sun4i_gpadc_dev {

[Quentin Schulz]

Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
include/linux/mfd/sun4i-gpadc.h | 2 ++
2 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 74eeb5cd5218..b7b5451226b0 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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.

Post by Philipp Rossak
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 = {
@@ -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.

[...]

Post by Philipp Rossak
struct sun4i_gpadc_iio {
@@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
return 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.

Post by Philipp Rossak
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.

Post by Philipp Rossak
+{
+ 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.
[...]

Post by Philipp Rossak
- 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);
+ if (ret)
+ return ret;
+
+ } else
+ info->no_irq = true;
+

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

Post by Philipp Rossak
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);
- 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?

Quentin

[Philipp Rossak]

Post by Maxime Ripard
Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
include/linux/mfd/sun4i-gpadc.h | 2 ++
2 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 74eeb5cd5218..b7b5451226b0 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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.

Sounds good for me! I will rework that in the next version.

Post by Maxime Ripard

Post by Philipp Rossak
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 = {
@@ -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.
[...]

Post by Philipp Rossak
struct sun4i_gpadc_iio {
@@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
return 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.

Sounds good for me! I will rework that in the next version.

Post by Maxime Ripard

Post by Philipp Rossak
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.

Post by Philipp Rossak
+{
+ 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.
[...]

Post by Philipp Rossak
- 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);
+ if (ret)
+ return ret;
+
+ } else
+ info->no_irq = true;
+

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

I know this looks very funny. I thought this would be better to keep, to
_not_ break anything. Since I will rework the whole driver and integrate
the mfd part I hope I can remove both.

Post by Maxime Ripard

Post by Philipp Rossak
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);
- 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?

I will enable this again, in the next version! I had some issues with
this, thus I disabled this, but I know now what I did wrong!

Thanks,
Philipp

[kbuild test robot]

Hi Philipp,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa

Note: the linux-review/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415 HEAD e571c3ced84a9d7f150cb2d239919d31d25114e2 builds fine.
It only hurts bisectibility.

drivers/iio/adc/sun4i-gpadc-iio.c:448:29: error: 'SUN8I_H3_THS_STAT' undeclared (first use in this function); did you mean 'SUN8I_H3_THS_INTC'?

regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
^~~~~~~~~~~~~~~~~
SUN8I_H3_THS_INTC
drivers/iio/adc/sun4i-gpadc-iio.c:448:29: note: each undeclared identifier is reported only once for each function it appears in

vim +448 drivers/iio/adc/sun4i-gpadc-iio.c

443
444 static irqreturn_t sunxi_irq_thread(int irq, void *data)
445 {
446 struct sun4i_gpadc_iio *info = data;
447

448 regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);

449
450 thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE);
451
452 return IRQ_HANDLED;
453 }
454

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

[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 <***@gmail.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
2 files changed, 108 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index b7b5451226b0..8196203d65fe 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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;
@@ -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),
+};
+
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,
+ 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;
+}
+
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 */ }
};

diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 458f2159a95a..80b79c31cea3 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -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))
+
+#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)

#define MAX_SENSOR_COUNT 4

[Maxime Ripard]

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
2 files changed, 108 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index b7b5451226b0..8196203d65fe 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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;
@@ -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;

You can use a regmap_field for that.

Post by Philipp Rossak
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 */
@@ -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)

Please remain consistent with the prefixes used in the driver

Maxime

[Quentin Schulz]

Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
2 files changed, 108 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index b7b5451226b0..8196203d65fe 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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.

Post by Philipp Rossak
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
should make a better job at explaining it :)

Post by Philipp Rossak
+};
+
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,
+ 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...

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.

Post by Philipp Rossak
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 */ }
};
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 458f2159a95a..80b79c31cea3 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -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))
+
+#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

[Philipp Rossak]

Post by Maxime Ripard
Hi Philipp,

Post by 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.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
2 files changed, 108 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index b7b5451226b0..8196203d65fe 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -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.

Post by Philipp Rossak
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
should make a better job at explaining it :)

I agree, I will add this in the next version in the commit message.

Post by Maxime Ripard

Post by Philipp Rossak
+};
+
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,
+ 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...

Post by Philipp Rossak
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.

for H3, A83T, H5, A64 we can use the same sample start. So I would use
here a H3 sample start function and reuse it.

A80 is special since it has no crtl0 register, thus it would get also
its own function.

H6 will need also an own sample start function (looking in the near future).

Post by Maxime Ripard

Post by Philipp Rossak
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 */ }
};
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 458f2159a95a..80b79c31cea3 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -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))
+
+#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.
#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

I agree, this the better way to do it.


Thanks,
Philipp

[Philipp Rossak]

This patch adds support for the A83T ths sensor.

The A83T supports interrupts. The interrupt is configured to update the
the sensor values every second.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
2 files changed, 56 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 8196203d65fe..9f7895ba1966 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
SUN8I_H3_THS_TEMP_PERIOD(0x7),
};

+static const struct gpadc_data sun8i_a83t_ths_data = {
+ .temp_offset = -2724,
+ .temp_scale = -70,
+ .temp_data = {SUN8I_H3_THS_TDATA0,
+ SUN8I_A83T_THS_TDATA1,
+ SUN8I_A83T_THS_TDATA2,
+ 0},
+ .sample_start = sunxi_ths_sample_start,
+ .sample_end = sunxi_ths_sample_end,
+ .sensor_count = 3,
+ .supports_nvmem = false,
+ .support_irq = true,
+ .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
+ .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
+ .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN1 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN2,
+ .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+ .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_1 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_2 |
+ SUN8I_H3_THS_INTS_SHUT_INT_0 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_1 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_2 |
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
+ .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+ SUN8I_A83T_THS_INTC_TDATA_IRQ_EN1 |
+ SUN8I_A83T_THS_INTC_TDATA_IRQ_EN2 |
+ SUN8I_H3_THS_TEMP_PERIOD(0x257),
+};
+
struct sun4i_gpadc_iio {
struct iio_dev *indio_dev;
struct completion completion;
@@ -668,6 +702,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
.compatible = "allwinner,sun8i-h3-ths",
.data = &sun8i_h3_ths_data,
},
+ {
+ .compatible = "allwinner,sun8i-a83t-ths",
+ .data = &sun8i_a83t_ths_data,
+ },
{ /* sentinel */ }
};

diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 80b79c31cea3..32f15cc03363 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -99,21 +99,39 @@
#define SUNXI_THS_CDATA_0_1 0x74
#define SUNXI_THS_CDATA_2_3 0x78
#define SUN8I_H3_THS_TDATA0 0x80
+#define SUN8I_A83T_THS_TDATA1 0x84
+#define SUN8I_A83T_THS_TDATA2 0x88

#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))

#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN1 BIT(1)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN2 BIT(2)

#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))

#define SUN8I_H3_THS_INTS_ALARM_INT_0 BIT(0)
+#define SUN8I_A83T_THS_INTS_ALARM_INT_1 BIT(1)
+#define SUN8I_A83T_THS_INTS_ALARM_INT_2 BIT(2)
#define SUN8I_H3_THS_INTS_SHUT_INT_0 BIT(4)
+#define SUN8I_A83T_THS_INTS_SHUT_INT_1 BIT(5)
+#define SUN8I_A83T_THS_INTS_SHUT_INT_2 BIT(6)
#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
+#define SUN8I_A83T_THS_INTS_TDATA_IRQ_1 BIT(9)
+#define SUN8I_A83T_THS_INTS_TDATA_IRQ_2 BIT(10)
#define SUN8I_H3_THS_INTS_ALARM_OFF_0 BIT(12)
+#define SUN8I_A83T_THS_INTS_ALARM_OFF_1 BIT(13)
+#define SUN8I_A83T_THS_INTS_ALARM_OFF_2 BIT(14)

#define SUN8I_H3_THS_INTC_ALARM_INT_EN0 BIT(0)
+#define SUN8I_A83T_THS_INTC_ALARM_INT_EN1 BIT(1)
+#define SUN8I_A83T_THS_INTC_ALARM_INT_EN2 BIT(2)
#define SUN8I_H3_THS_INTC_SHUT_INT_EN0 BIT(4)
+#define SUN8I_A83T_THS_INTC_SHUT_INT_EN1 BIT(5)
+#define SUN8I_A83T_THS_INTC_SHUT_INT_EN2 BIT(6)
#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
+#define SUN8I_A83T_THS_INTC_TDATA_IRQ_EN1 BIT(9)
+#define SUN8I_A83T_THS_INTC_TDATA_IRQ_EN2 BIT(10)

#define MAX_SENSOR_COUNT 4

[Maxime Ripard]

Post by Philipp Rossak
This patch adds support for the A83T ths sensor.
The A83T supports interrupts. The interrupt is configured to update the
the sensor values every second.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 8196203d65fe..9f7895ba1966 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
SUN8I_H3_THS_TEMP_PERIOD(0x7),
};
+static const struct gpadc_data sun8i_a83t_ths_data = {
+ .temp_offset = -2724,
+ .temp_scale = -70,
+ .temp_data = {SUN8I_H3_THS_TDATA0,
+ SUN8I_A83T_THS_TDATA1,
+ SUN8I_A83T_THS_TDATA2,
+ 0},
+ .sample_start = sunxi_ths_sample_start,
+ .sample_end = sunxi_ths_sample_end,
+ .sensor_count = 3,
+ .supports_nvmem = false,
+ .support_irq = true,
+ .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
+ .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),

Where are these values coming from?

Post by Philipp Rossak
+ .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN1 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN2,
+ .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+ .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_1 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_2 |
+ SUN8I_H3_THS_INTS_SHUT_INT_0 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_1 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_2 |
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_2,

Do you reall need to clear all these interrupts if you're using only
one?

Maxime

[Philipp Rossak]

Post by Maxime Ripard

Post by Philipp Rossak
This patch adds support for the A83T ths sensor.
The A83T supports interrupts. The interrupt is configured to update the
the sensor values every second.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 8196203d65fe..9f7895ba1966 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
SUN8I_H3_THS_TEMP_PERIOD(0x7),
};
+static const struct gpadc_data sun8i_a83t_ths_data = {
+ .temp_offset = -2724,
+ .temp_scale = -70,
+ .temp_data = {SUN8I_H3_THS_TDATA0,
+ SUN8I_A83T_THS_TDATA1,
+ SUN8I_A83T_THS_TDATA2,
+ 0},
+ .sample_start = sunxi_ths_sample_start,
+ .sample_end = sunxi_ths_sample_end,
+ .sensor_count = 3,
+ .supports_nvmem = false,
+ .support_irq = true,
+ .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
+ .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),

Where are these values coming from?

These values are calculated with the formulas from the datasheet and
also tested on hardware. These settings seem ok.

Post by Maxime Ripard

Post by Philipp Rossak
+ .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN1 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN2,
+ .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+ .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_1 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_2 |
+ SUN8I_H3_THS_INTS_SHUT_INT_0 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_1 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_2 |
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_2,

Do you reall need to clear all these interrupts if you're using only
one?

No, I don't think so, I will remove them in the next version.

Post by Maxime Ripard
Maxime

Philipp

[Maxime Ripard]

1;5002;0c

Post by Maxime Ripard

Post by Philipp Rossak
This patch adds support for the A83T ths sensor.
The A83T supports interrupts. The interrupt is configured to update the
the sensor values every second.
---
drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 8196203d65fe..9f7895ba1966 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
SUN8I_H3_THS_TEMP_PERIOD(0x7),
};
+static const struct gpadc_data sun8i_a83t_ths_data = {
+ .temp_offset = -2724,
+ .temp_scale = -70,
+ .temp_data = {SUN8I_H3_THS_TDATA0,
+ SUN8I_A83T_THS_TDATA1,
+ SUN8I_A83T_THS_TDATA2,
+ 0},
+ .sample_start = sunxi_ths_sample_start,
+ .sample_end = sunxi_ths_sample_end,
+ .sensor_count = 3,
+ .supports_nvmem = false,
+ .support_irq = true,
+ .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
+ .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),

Where are these values coming from?

These values are calculated with the formulas from the datasheet and also
tested on hardware. These settings seem ok.

You should at least put a comment explaining how you got to these values.

Maxime

[Philipp Rossak]

As we have gained the support for the thermal sensor in H3 and H5,
we can now add its device nodes to the device tree. The H3 and H5 share
most of its compatible. The compatible and the thermal sensor cells
will be added in an additional patch per device.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 7a83b15225c7..413c789b588d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -426,6 +426,15 @@
};
};

+ ths: thermal-***@1c25000 {
+ reg = <0x01c25000 0x100>;
+ interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+ clock-names = "bus", "mod";
+ resets = <&ccu RST_BUS_THS>;
+ #io-channel-cells = <0>;
+ };
+
***@1c20c00 {
compatible = "allwinner,sun4i-a10-timer";
reg = <0x01c20c00 0xa0>;

[Maxime Ripard]

Hi,

Post by Philipp Rossak
As we have gained the support for the thermal sensor in H3 and H5,
we can now add its device nodes to the device tree. The H3 and H5 share
most of its compatible. The compatible and the thermal sensor cells
will be added in an additional patch per device.
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 7a83b15225c7..413c789b588d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -426,6 +426,15 @@
};
};
+ reg = <0x01c25000 0x100>;

The size is 0x400

Maxime

[Philipp Rossak]

Post by Maxime Ripard
Hi,

Post by Philipp Rossak
As we have gained the support for the thermal sensor in H3 and H5,
we can now add its device nodes to the device tree. The H3 and H5 share
most of its compatible. The compatible and the thermal sensor cells
will be added in an additional patch per device.
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 7a83b15225c7..413c789b588d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -426,6 +426,15 @@
};
};
+ reg = <0x01c25000 0x100>;

The size is 0x400
Maxime

Ok, I will fix this.

Philipp

[Philipp Rossak]

This patch adds the missing compatible and the thermal sensor cells.
The H3 has one sensor.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 8495deecedad..fbb007e5798e 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -128,3 +128,8 @@
&pio {
compatible = "allwinner,sun8i-h3-pinctrl";
};
+
+&ths {
+ compatible = "allwinner,sun8i-h3-ths";
+ #thermal-sensor-cells = <0>;
+};

[Philipp Rossak]

This patch adds the thermal zones to the H3. We have only one sensor and
that is placed in the cpu.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fbb007e5798e..3f83f6a27c74 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,15 @@
};
};

+ thermal-zones {
+ cpu-thermal {
+ /* milliseconds */
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths 0>;
+ };
+ };
+
timer {
compatible = "arm,armv7-timer";
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,

[Maxime Ripard]

Post by Philipp Rossak
This patch adds the thermal zones to the H3. We have only one sensor and
that is placed in the cpu.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fbb007e5798e..3f83f6a27c74 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,15 @@
};
};
+ thermal-zones {
+ cpu-thermal {
+ /* milliseconds */
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths 0>;

if the thermal-sensor-cells value is indeed 0, the phandle parsing
will be broken here.

Maxime

[Philipp Rossak]

Post by Maxime Ripard

Post by Philipp Rossak
This patch adds the thermal zones to the H3. We have only one sensor and
that is placed in the cpu.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fbb007e5798e..3f83f6a27c74 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,15 @@
};
};
+ thermal-zones {
+ cpu-thermal {
+ /* milliseconds */
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths 0>;

if the thermal-sensor-cells value is indeed 0, the phandle parsing
will be broken here.
Maxime

Ok, then I will remove the 0.

Thanks,

Philipp

[Philipp Rossak]

This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};

+ soc {
+ sid: ***@1c14000 {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+
thermal-zones {
cpu-thermal {
/* milliseconds */

[Maxime Ripard]

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?

Maxime

[Philipp Rossak]

Post by Maxime Ripard

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?
Maxime

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address
0x34. I will add that in the next version.


[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE

Thanks,
Philipp

[Icenowy Zheng]

Post by Philipp Rossak

Post by Philipp Rossak

Post by Maxime Ripard

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?
Maxime

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address

0x34. I

Post by Philipp Rossak
will add that in the next version.
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp

Hi,
Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)

This patch is independent and can be easily sent out
by its own.

Post by Philipp Rossak
Thanks,
Kyle Evans

[Kyle Evans]

Post by Icenowy Zheng

Post by Philipp Rossak

Post by Philipp Rossak

Post by Maxime Ripard

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?
Maxime

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address

0x34. I

Post by Philipp Rossak
will add that in the next version.
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp

Hi,
Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)

This patch is independent and can be easily sent out
by its own.

Right- I had considered doing so, but wanted to make sure I wasn't
going to collide with this series if a v3 is imminent.

[Philipp Rossak]

Hi Kyle,

I'm already working on a Version 3 of this patch series. Right now this
slowed down since I'm very busy and the ToDo-List is still very long.

My plan is to send out a version during this release cycle.

If you need it right now feel free to submit patches!

Philipp

Post by Kyle Evans

Post by Icenowy Zheng

Post by Philipp Rossak

Post by Philipp Rossak

Post by Maxime Ripard

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?
Maxime

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address

0x34. I

Post by Philipp Rossak
will add that in the next version.
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp

Hi,
Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)

This patch is independent and can be easily sent out
by its own.

Right- I had considered doing so, but wanted to make sure I wasn't
going to collide with this series if a v3 is imminent.

[Paul Kocialkowski]

Hi,

Post by Philipp Rossak
Hi Kyle,
I'm already working on a Version 3 of this patch series. Right now this
slowed down since I'm very busy and the ToDo-List is still very long.
My plan is to send out a version during this release cycle.
If you need it right now feel free to submit patches!

I just came around this patch while testing SID support for H3, that is
not enabled in the dts at this point despite driver support.

Maxime, would it make sense to merge this patch as-is, without support
for the thermal calibration section, since the two aspects seem rather
independent (and some other SoCs also have a SID node without linkage
for thermal support anyway)?

Cheers,

Paul

Post by Philipp Rossak
Philipp

Post by Kyle Evans

Post by Icenowy Zheng

Post by Philipp Rossak

Post by Philipp Rossak

Post by Maxime Ripard

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?
Maxime

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address

0x34. I

Post by Philipp Rossak
will add that in the next version.
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp

Hi,
Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)

This patch is independent and can be easily sent out
by its own.

Right- I had considered doing so, but wanted to make sure I wasn't
going to collide with this series if a v3 is imminent.

--
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[Paul Kocialkowski]

Hi,

On Wed, 2018-07-25 at 11:05 +0200, Emmanuel Vadot wrote:

[...]

Hello Paul,
I've sent a serie yesterday for SID on A64/H3/H5.
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=761

Awesome, thanks for taking care of that :)

Cheers,

Paul

Post by Paul Kocialkowski

Post by Kyle Evans

Post by Icenowy Zheng

Post by Philipp Rossak

Post by Philipp Rossak

Post by Maxime Ripard

Post by Philipp Rossak
This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.
---
arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+

Shouldn't you also use a nvmem-cells property to the THS node?
Maxime

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address

0x34. I

Post by Philipp Rossak
will add that in the next version.
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp

Hi,
Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)

This patch is independent and can be easily sent out
by its own.

Right- I had considered doing so, but wanted to make sure I wasn't
going to collide with this series if a v3 is imminent.

--
Developer of free digital technology and hardware support.
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

--
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[Philipp Rossak]

As we have gained the support for the thermal sensor in A83T,
we can now add its device nodes to the device tree.

The A83T seems to have a broken IRQ 31, thus we use here IRQ 41.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 7f4955a5fab7..9e53ff5ac4ed 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -518,6 +518,14 @@
clocks = <&osc24M>;
};

+ ths: thermal-***@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>;
+ };
+
***@1c20ca0 {
compatible = "allwinner,sun6i-a31-wdt";
reg = <0x01c20ca0 0x20>;

[Philipp Rossak]

This patch adds the thermal zones to the A83T. Sensor 0 is located besides the
cpu cluster 0. Sensor 1 is located besides cluster 1 and sensor 2 is located
besides in the gpu.

Signed-off-by: Philipp Rossak <***@gmail.com>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 9e53ff5ac4ed..4259a8726031 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -747,4 +747,24 @@
#size-cells = <0>;
};
};
+
+ thermal-zones {
+ cpu0_thermal: cpu0-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 0>;
+ };
+
+ cpu1_thermal: cpu1-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 1>;
+ };
+
+ gpu_thermal: gpu-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 2>;
+ };
+ };
};