2018-08-30 15:50:51

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 00/30] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

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

This Patchtseries was in the beginning based on Icenowy Zengs v4 patchseries [1]. Since we decided to merge the mfd driver into the GPADC this changed. So only one patch could be reused.

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

Sorry for delaying this.

Regards,
Philipp

changes since v2:
* mfd driver is now merged into the gpadc driver
* complete rework

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 (29):
mfd: Makefile: Remove build option for MFD:sun4i-gpadc
mfd: Kconfig: Remove MFD_SUN4I_GPADC config option
iio: adc: Remove ID table
iio: adc: Kconfig: Update Kconfig to new build options
iio: adc: move SUN4I_GPADC_CHANNEL define to header file
iio: adc: remove ofnode options
iio: adc: remove mfd_probe & sunwi_irq_init function
iio: adc: remove hwmon structure
iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i
channel
iio: adc: rework irq and adc_channel handling
iio: adc: add new compatibles
mfd: Remove old mfd driver & Move sun4i-gpadc.h to iio/adc/
arm: config: Enable SUN4I_GPADC in defconfig
dt-bindings: update the Allwinner GPADC device tree binding for H3 &
A83T
iio: adc: sun4i-gpadc-iio: rework: readout temp_data
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: device specific suspend & resume
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: h3: use calibration for ths
ARM: dts: sun8i: a83t: add support for the thermal sensor in A83T
ARM: dts: sun8i: a83t: add thermal zone to A83T
ARM: sun8i: a83t: full range OPP tables and CPUfreq

.../devicetree/bindings/iio/adc/sun4i-gpadc.txt | 41 +-
arch/arm/boot/dts/sun8i-a83t.dtsi | 143 +++++
arch/arm/boot/dts/sun8i-h3.dtsi | 52 ++
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
arch/arm/configs/sunxi_defconfig | 1 +
drivers/iio/adc/Kconfig | 11 +-
drivers/iio/adc/sun4i-gpadc-iio.c | 617 +++++++++++++--------
drivers/mfd/Kconfig | 17 -
drivers/mfd/Makefile | 1 -
drivers/mfd/sun4i-gpadc.c | 181 ------
include/linux/{mfd => iio/adc}/sun4i-gpadc.h | 47 +-
11 files changed, 681 insertions(+), 440 deletions(-)
delete mode 100644 drivers/mfd/sun4i-gpadc.c
rename include/linux/{mfd => iio/adc}/sun4i-gpadc.h (72%)

--
2.11.0



2018-08-30 15:47:15

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 14/30] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

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

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

Update the binding document to cover H3 and A83T.

Signed-off-by: Philipp Rossak <[email protected]>
---
.../devicetree/bindings/iio/adc/sun4i-gpadc.txt | 41 ++++++++++++++++++++--
1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/sun4i-gpadc.txt b/Documentation/devicetree/bindings/iio/adc/sun4i-gpadc.txt
index a7ef9dd21f04..9116ad308cf1 100644
--- a/Documentation/devicetree/bindings/iio/adc/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/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:
+ Please refer <devicetree/bindings/thermal/thermal.txt>,
- #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"
+ - "allwinner,sun8i-a83t-ths"
+ - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
+ - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
+Example for A33:
ths: ths@1c25000 {
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
@@ -17,6 +40,18 @@ Example:
#io-channel-cells = <0>;
};

+Example for H3:
+ ths: thermal-sensor@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>;
+ };
+
sun4i, sun5i and sun6i SoCs are also supported via these bindings:

Required properties:
--
2.11.0


2018-08-30 15:47:18

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 01/30] mfd: Makefile: Remove build option for MFD:sun4i-gpadc

Since we are merging the mfd driver into the sun4i-gpadc driver we need
to remove the build options for the sun4i-gpadc driver.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/mfd/Makefile | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e9fd20dba18d..c680994db988 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -220,7 +220,6 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
obj-$(CONFIG_MFD_MT6397) += mt6397-core.o

obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
-obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o

obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
--
2.11.0


2018-08-30 15:47:18

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 19/30] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

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

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 18ab72e52d78..2fd73d143815 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>
@@ -70,6 +71,7 @@ struct gpadc_data {
bool has_mod_clk;
u32 temp_data_base;
int sensor_count;
+ bool supports_nvmem;
};

static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
@@ -146,6 +148,7 @@ struct sun4i_gpadc_iio {
struct clk *bus_clk;
struct clk *mod_clk;
struct reset_control *reset;
+ u32 calibration_data[2];
};

static const struct iio_chan_spec sun4i_gpadc_channels[] = {
@@ -484,6 +487,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;
+ u32 *cell_data;

info->data = of_device_get_match_data(&pdev->dev);
if (!info->data)
@@ -494,6 +500,24 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
if (IS_ERR(base))
return PTR_ERR(base);

+ if (info->data->supports_nvmem) {
+
+ cell = nvmem_cell_get(&pdev->dev, "calibration");
+ if (IS_ERR(cell)) {
+ if (PTR_ERR(cell) == -EPROBE_DEFER)
+ return PTR_ERR(cell);
+ } else {
+ cell_data = (u32 *)nvmem_cell_read(cell, &cell_size);
+ if (cell_size != 8)
+ dev_err(&pdev->dev,
+ "Calibration data has wrong size\n");
+ else {
+ info->calibration_data[0] = cell_data[0];
+ info->calibration_data[1] = cell_data[1];
+ }
+ }
+ }
+
if (info->data->has_bus_clk)
info->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "bus",
base, &sun4i_gpadc_regmap_config);
--
2.11.0


2018-08-30 15:47:21

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 23/30] ARM: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5

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 <[email protected]>
---
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 c3bff1105e5d..3520e4ad6042 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -488,6 +488,15 @@
};
};

+ ths: thermal-sensor@1c25000 {
+ reg = <0x01c25000 0x400>;
+ 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>;
+ };
+
timer@1c20c00 {
compatible = "allwinner,sun4i-a10-timer";
reg = <0x01c20c00 0xa0>;
--
2.11.0


2018-08-30 15:47:27

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

Since we have now thermal trotteling enabeled we can now add the full
range of the OPP table.

The operating points were found in Allwinner BSP and fex files.

Signed-off-by: Philipp Rossak <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 78aa448e869f..ddcf404f9c80 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -250,6 +250,22 @@
opp-microvolt = <840000>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
+
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <920000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp-1800000000 { /* BOOT FREQ */
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <1000000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <1080000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
};

cpu1_opp_table: opp_table1 {
@@ -303,6 +319,22 @@
opp-microvolt = <840000>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
+
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <920000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp-1800000000 { /* BOOT FREQ */
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <1000000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <1080000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
};

soc {
--
2.11.0


2018-08-30 15:47:33

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 29/30] ARM: dts: sun8i: a83t: add thermal zone to A83T

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 <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 103 ++++++++++++++++++++++++++++++++++++++
1 file changed, 103 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index f2f745930b08..78aa448e869f 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -50,6 +50,7 @@
#include <dt-bindings/reset/sun8i-a83t-ccu.h>
#include <dt-bindings/reset/sun8i-de2.h>
#include <dt-bindings/reset/sun8i-r-ccu.h>
+#include <dt-bindings/thermal/thermal.h>

/ {
interrupt-parent = <&gic>;
@@ -69,6 +70,9 @@
cci-control-port = <&cci_control0>;
enable-method = "allwinner,sun8i-a83t-smp";
reg = <0>;
+ #cooling-cells = <2>;
+ cooling-min-level = <0>;
+ cooling-max-level = <7>;
};

cpu@1 {
@@ -107,6 +111,9 @@
cci-control-port = <&cci_control1>;
enable-method = "allwinner,sun8i-a83t-smp";
reg = <0x100>;
+ #cooling-cells = <2>;
+ cooling-min-level = <0>;
+ cooling-max-level = <7>;
};

cpu@101 {
@@ -1035,4 +1042,100 @@
#size-cells = <0>;
};
};
+
+ thermal-zones {
+ cpu0_thermal: cpu0-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 0>;
+
+ trips {
+ cpu0_warm: cpu_warm {
+ temperature = <70000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu0_hot: cpu_hot {
+ temperature = <80000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu0_very_hot: cpu_very_hot {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu0_crit: cpu_crit {
+ temperature = <105000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ cpu_warm_limit_cpu {
+ trip = <&cpu0_warm>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT 4>;
+ };
+ cpu_hot_limit_cpu {
+ trip = <&cpu0_hot>;
+ cooling-device = <&cpu0 5 5>;
+ };
+ cpu_very_hot_limit_cpu {
+ trip = <&cpu0_very_hot>;
+ cooling-device = <&cpu0 7 THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu1_thermal: cpu1-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 1>;
+
+ trips {
+ cpu1_warm: cpu_warm {
+ temperature = <70000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu1_hot: cpu_hot {
+ temperature = <80000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu1_very_hot: cpu_very_hot {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu1_crit: cpu_crit {
+ temperature = <105000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ cpu_warm_limit_cpu {
+ trip = <&cpu1_warm>;
+ cooling-device = <&cpu100 THERMAL_NO_LIMIT 4>;
+ };
+ cpu_hot_limit_cpu {
+ trip = <&cpu1_hot>;
+ cooling-device = <&cpu100 5 5>;
+ };
+ cpu_very_hot_limit_cpu {
+ trip = <&cpu1_very_hot>;
+ cooling-device = <&cpu100 7 THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ gpu_thermal: gpu-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 2>;
+ };
+ };
};
--
2.11.0


2018-08-30 15:47:42

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths

The H3 SID is supported by the kernel so we can add a NVMEM Data cell,
that contains the calibration data.

On the H3 the eFuses are located at the offset 0x200. The thermal data
itself has an offset of 0x34 from the eFuse base. So we end on an offset
of 0x234.

Signed-off-by: Philipp Rossak <[email protected]>
---
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 1866aec69ec1..0fc447f0c02a 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -106,8 +106,15 @@

soc {
sid: eeprom@1c14000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
compatible = "allwinner,sun8i-h3-sid";
reg = <0x01c14000 0x400>;
+
+ /* Data cells */
+ thermal_calibration: calib@234 {
+ reg = <0x234 0x8>;
+ };
};
};

@@ -227,4 +234,6 @@
&ths {
compatible = "allwinner,sun8i-h3-ths";
#thermal-sensor-cells = <0>;
+ nvmem-cells = <&thermal_calibration>;
+ nvmem-cell-names = "calibration";
};
--
2.11.0


2018-08-30 15:47:50

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 26/30] ARM: dts: sun8i: h3: enable H3 sid controller

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

Signed-off-by: Philipp Rossak <[email protected]>
---
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 954848d5df50..1866aec69ec1 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -104,6 +104,13 @@
};
};

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


2018-08-30 15:47:58

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 25/30] ARM: dts: sun8i: h3: add thermal zone to H3

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 <[email protected]>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 31 +++++++++++++++++++++++++++++++
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
2 files changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 5b7994cb1471..954848d5df50 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -78,6 +78,8 @@
clock-names = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
#cooling-cells = <2>;
+ cooling-min-level = <0>;
+ cooling-max-level = <15>;
};

cpu@1 {
@@ -102,6 +104,35 @@
};
};

+ thermal-zones {
+ cpu-thermal {
+ /* milliseconds */
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths>;
+
+ trips {
+ cpu_hot_trip: cpu-warm {
+ temperature = <65000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu_very_hot_trip: cpu-very-hot {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ cpu-warm-limit {
+ trip = <&cpu_hot_trip>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+ };
+
timer {
compatible = "arm,armv7-timer";
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 3520e4ad6042..2c83f4893757 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -47,6 +47,7 @@
#include <dt-bindings/reset/sun8i-de2.h>
#include <dt-bindings/reset/sun8i-h3-ccu.h>
#include <dt-bindings/reset/sun8i-r-ccu.h>
+#include <dt-bindings/thermal/thermal.h>

/ {
interrupt-parent = <&gic>;
--
2.11.0


2018-08-30 15:47:59

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 28/30] ARM: dts: sun8i: a83t: add support for the thermal sensor in A83T

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 <[email protected]>
---
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 2be23d600957..f2f745930b08 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -757,6 +757,14 @@
clocks = <&osc24M>;
};

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


2018-08-30 15:48:26

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

This patch adds support for the H3 ths sensor.

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

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 91 +++++++++++++++++++++++++++++++++++++
include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++
2 files changed, 109 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index c7b46c82e3e5..d5c7971b2558 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -72,6 +72,7 @@ struct gpadc_data {
u32 temp_data_base;
int sensor_count;
bool supports_nvmem;
+ u32 ths_irq_clear;
};

static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
@@ -79,6 +80,10 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
static int sun4i_ths_resume(struct sun4i_gpadc_iio *info);
static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info);

+static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info);
+static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info);
+static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data);
+
static const struct gpadc_data sun4i_gpadc_data = {
.temp_offset = -1932,
.temp_scale = 133,
@@ -137,6 +142,22 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.sensor_count = 1,
};

+static const struct gpadc_data sun8i_h3_ths_data = {
+ .temp_offset = -1791,
+ .temp_scale = -121,
+ .temp_data_base = SUN8I_H3_THS_TDATA0,
+ .ths_irq_thread = sunx8i_h3_irq_thread,
+ .support_irq = true,
+ .has_bus_clk = true,
+ .has_bus_rst = true,
+ .has_mod_clk = true,
+ .sensor_count = 1,
+ .supports_nvmem = true,
+ .ths_resume = sun8i_h3_ths_resume,
+ .ths_suspend = sun8i_h3_ths_suspend,
+ .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0,
+};
+
struct sun4i_sensor_tzd {
struct sun4i_gpadc_iio *info;
struct thermal_zone_device *tzd;
@@ -409,6 +430,31 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}

+static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data)
+{
+ struct sun4i_gpadc_iio *info = data;
+ int i;
+
+ regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+ info->data->ths_irq_clear);
+
+ for (i = 0; i < info->data->sensor_count; i++)
+ thermal_zone_device_update(info->tzds[i].tzd,
+ THERMAL_EVENT_TEMP_SAMPLE);
+
+ return IRQ_HANDLED;
+}
+
+static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info)
+{
+// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
+// info->calibration_data[0]);
+// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
+// info->calibration_data[1]);
+
+ return 0;
+}
+
static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
@@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)
return 0;
}

+static int sun8i_h3_ths_suspend(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_resume(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
@@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct sun4i_gpadc_iio *info)
return 0;
}

+static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info)
+{
+ u32 value;
+
+ sun8i_h3_calibrate(info);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
+ SUN4I_GPADC_CTRL0_T_ACQ(0xff));
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+ SUN8I_H3_THS_ACQ1(0x3f));
+
+ regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
+ SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2));
+
+ regmap_write(info->regmap, SUN8I_H3_THS_INTC,
+ SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+ SUN8I_H3_THS_TEMP_PERIOD(0x55));
+
+ regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+ SUN8I_H3_THS_TEMP_SENSE_EN0 | value);
+
+ return 0;
+}
+
static int sun4i_gpadc_get_temp(void *data, int *temp)
{
struct sun4i_sensor_tzd *tzd = data;
@@ -497,6 +584,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
.compatible = "allwinner,sun6i-a31-gpadc",
.data = &sun6i_gpadc_data
},
+ {
+ .compatible = "allwinner,sun8i-h3-ths",
+ .data = &sun8i_h3_ths_data,
+ },
{ /* sentinel */ }
};

diff --git a/include/linux/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
index 99feeba28105..169b4de9a34d 100644
--- a/include/linux/iio/adc/sun4i-gpadc.h
+++ b/include/linux/iio/adc/sun4i-gpadc.h
@@ -100,6 +100,24 @@
}

/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_CTRL0 0x00
+
+#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)
+
+#define SUN8I_H3_THS_INTC 0x44
+#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
+#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
+
+#define SUN8I_H3_THS_STAT 0x48
+#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
+
+#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 MAX_SENSOR_COUNT 4

#endif
--
2.11.0


2018-08-30 15:48:27

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 24/30] ARM: dts: sun8i: h3: add support for the thermal sensor in H3

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

Signed-off-by: Philipp Rossak <[email protected]>
---
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 41d57c76f290..5b7994cb1471 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -185,3 +185,8 @@
&pio {
compatible = "allwinner,sun8i-h3-pinctrl";
};
+
+&ths {
+ compatible = "allwinner,sun8i-h3-ths";
+ #thermal-sensor-cells = <0>;
+};
--
2.11.0


2018-08-30 15:48:34

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 20/30] iio: adc: sun4i-gpadc-iio: rework: device specific suspend & resume

Different sensors will have different suspend and resume functions. So
we are modularize the suspend and resume functions.

The resume function configures and initializes the thermal sensor and
the suspend function disables the sensors.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 2fd73d143815..c7b46c82e3e5 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -76,6 +76,9 @@ struct gpadc_data {

static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);

+static int sun4i_ths_resume(struct sun4i_gpadc_iio *info);
+static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info);
+
static const struct gpadc_data sun4i_gpadc_data = {
.temp_offset = -1932,
.temp_scale = 133,
@@ -87,6 +90,8 @@ static const struct gpadc_data sun4i_gpadc_data = {
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .ths_resume = sun4i_ths_resume,
+ .ths_suspend = sun4i_ths_suspend,
.sensor_count = 1,
};

@@ -101,6 +106,8 @@ static const struct gpadc_data sun5i_gpadc_data = {
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .ths_resume = sun4i_ths_resume,
+ .ths_suspend = sun4i_ths_suspend,
.sensor_count = 1,
};

@@ -115,6 +122,8 @@ static const struct gpadc_data sun6i_gpadc_data = {
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .ths_resume = sun4i_ths_resume,
+ .ths_suspend = sun4i_ths_suspend,
.sensor_count = 1,
};

@@ -123,6 +132,8 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.temp_scale = 162,
.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .ths_resume = sun4i_ths_resume,
+ .ths_suspend = sun4i_ths_suspend,
.sensor_count = 1,
};

@@ -401,6 +412,11 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+ return info->data->ths_suspend(info);
+}
+
+static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)
+{

/* Disable the ADC on IP */
regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
@@ -415,7 +431,11 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
static int sun4i_gpadc_runtime_resume(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+ return info->data->ths_resume(info);
+}

+static int sun4i_ths_resume(struct sun4i_gpadc_iio *info)
+{
/* clkin = 6MHz */
regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
--
2.11.0


2018-08-30 15:48:39

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 18/30] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

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

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

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

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

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 63 +++++++++++++++++++++++++------------
include/linux/iio/adc/sun4i-gpadc.h | 3 ++
2 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index c12de48c4e86..18ab72e52d78 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -69,6 +69,7 @@ struct gpadc_data {
bool has_bus_rst;
bool has_mod_clk;
u32 temp_data_base;
+ int sensor_count;
};

static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
@@ -84,6 +85,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .sensor_count = 1,
};

static const struct gpadc_data sun5i_gpadc_data = {
@@ -97,6 +99,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .sensor_count = 1,
};

static const struct gpadc_data sun6i_gpadc_data = {
@@ -110,6 +113,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .sensor_count = 1,
};

static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -117,6 +121,13 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.temp_scale = 162,
.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
.temp_data_base = SUN4I_GPADC_TEMP_DATA,
+ .sensor_count = 1,
+};
+
+struct sun4i_sensor_tzd {
+ struct sun4i_gpadc_iio *info;
+ struct thermal_zone_device *tzd;
+ unsigned int sensor_id;
};

struct sun4i_gpadc_iio {
@@ -130,7 +141,7 @@ struct sun4i_gpadc_iio {
const struct gpadc_data *data;
/* prevents concurrent reads of temperature and ADC */
struct mutex mutex;
- struct thermal_zone_device *tzd;
+ struct sun4i_sensor_tzd tzds[MAX_SENSOR_COUNT];
struct device *sensor_device;
struct clk *bus_clk;
struct clk *mod_clk;
@@ -280,7 +291,8 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
SUN4I_GPADC_IRQ_FIFO_DATA);
}

-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);

@@ -290,7 +302,8 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)

pm_runtime_get_sync(indio_dev->dev.parent);

- regmap_read(info->regmap, info->data->temp_data_base, val);
+ regmap_read(info->regmap, info->data->temp_data_base + 0x4 * sensor,
+ val);

pm_runtime_mark_last_busy(indio_dev->dev.parent);
pm_runtime_put_autosuspend(indio_dev->dev.parent);
@@ -334,7 +347,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;
@@ -420,10 +433,11 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)

static int sun4i_gpadc_get_temp(void *data, int *temp)
{
- struct sun4i_gpadc_iio *info = data;
+ struct sun4i_sensor_tzd *tzd = data;
+ struct sun4i_gpadc_iio *info = tzd->info;
int val, scale, offset;

- if (sun4i_gpadc_temp_read(info->indio_dev, &val))
+ if (sun4i_gpadc_temp_read(info->indio_dev, &val, tzd->sensor_id))
return -ETIMEDOUT;

sun4i_gpadc_temp_scale(info->indio_dev, &scale);
@@ -569,7 +583,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)
@@ -603,18 +617,24 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_enable(&pdev->dev);

- info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
- 0, info,
- &sun4i_ts_tz_ops);
- /*
- * Do not fail driver probing when failing to register in
- * thermal because no thermal DT node is found.
- */
- if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
- dev_err(&pdev->dev,
- "could not register thermal sensor: %ld\n",
- PTR_ERR(info->tzd));
- return PTR_ERR(info->tzd);
+ for (i = 0; i < info->data->sensor_count; i++) {
+ info->tzds[i].info = info;
+ info->tzds[i].sensor_id = i;
+
+ info->tzds[i].tzd = thermal_zone_of_sensor_register(
+ info->sensor_device,
+ i, &info->tzds[i], &sun4i_ts_tz_ops);
+ /*
+ * Do not fail driver probing when failing to register in
+ * thermal because no thermal DT node is found.
+ */
+ if (IS_ERR(info->tzds[i].tzd) && \
+ PTR_ERR(info->tzds[i].tzd) != -ENODEV) {
+ dev_err(&pdev->dev,
+ "could not register thermal sensor: %ld\n",
+ PTR_ERR(info->tzds[i].tzd));
+ return PTR_ERR(info->tzds[i].tzd);
+ }
}

ret = devm_iio_device_register(&pdev->dev, indio_dev);
@@ -639,11 +659,14 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
+ int i;

pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);

- thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);
+ for (i = 0; i < info->data->sensor_count; i++)
+ thermal_zone_of_sensor_unregister(info->sensor_device,
+ info->tzds[i].tzd);

if (!info->data->support_irq)
iio_map_array_unregister(indio_dev);
diff --git a/include/linux/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
index d6850f39dcfb..99feeba28105 100644
--- a/include/linux/iio/adc/sun4i-gpadc.h
+++ b/include/linux/iio/adc/sun4i-gpadc.h
@@ -99,4 +99,7 @@
.datasheet_name = _name, \
}

+/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define MAX_SENSOR_COUNT 4
+
#endif
--
2.11.0


2018-08-30 15:48:42

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 22/30] iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor

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 <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 59 +++++++++++++++++++++++++++++++++++++
include/linux/iio/adc/sun4i-gpadc.h | 6 ++++
2 files changed, 65 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index d5c7971b2558..a184a87c56d4 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -84,6 +84,8 @@ static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info);
static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info);
static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data);

+static int sun8i_a83t_ths_resume(struct sun4i_gpadc_iio *info);
+
static const struct gpadc_data sun4i_gpadc_data = {
.temp_offset = -1932,
.temp_scale = 133,
@@ -158,6 +160,21 @@ static const struct gpadc_data sun8i_h3_ths_data = {
.ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0,
};

+static const struct gpadc_data sun8i_a83t_ths_data = {
+ .temp_offset = -2724,
+ .temp_scale = -70,
+ .temp_data_base = SUN8I_H3_THS_TDATA0,
+ .ths_irq_thread = sunx8i_h3_irq_thread,
+ .support_irq = true,
+ .sensor_count = 3,
+ .supports_nvmem = true,
+ .ths_resume = sun8i_a83t_ths_resume,
+ .ths_suspend = sun8i_h3_ths_suspend,
+ .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
+};
+
struct sun4i_sensor_tzd {
struct sun4i_gpadc_iio *info;
struct thermal_zone_device *tzd;
@@ -541,6 +558,44 @@ static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info)
return 0;
}

+static int sun8i_a83t_ths_resume(struct sun4i_gpadc_iio *info)
+{
+ u32 value;
+
+ sun8i_h3_calibrate(info);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
+ SUN4I_GPADC_CTRL0_T_ACQ(0x13f));
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+ SUN8I_H3_THS_ACQ1(0x13f));
+
+ regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_2);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
+ SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2));
+
+ regmap_write(info->regmap, SUN8I_H3_THS_INTC,
+ 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));
+
+ regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+ SUN8I_H3_THS_TEMP_SENSE_EN0 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN1 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN2 |
+ value);
+
+ return 0;
+}
+
static int sun4i_gpadc_get_temp(void *data, int *temp)
{
struct sun4i_sensor_tzd *tzd = data;
@@ -588,6 +643,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/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
index 169b4de9a34d..673459bb3ec3 100644
--- a/include/linux/iio/adc/sun4i-gpadc.h
+++ b/include/linux/iio/adc/sun4i-gpadc.h
@@ -105,13 +105,19 @@
#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)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN1 BIT(1)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN2 BIT(2)

#define SUN8I_H3_THS_INTC 0x44
#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
#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 SUN8I_H3_THS_STAT 0x48
#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_FILTER 0x70
#define SUNXI_THS_CDATA_0_1 0x74
--
2.11.0


2018-08-30 15:48:47

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 17/30] iio: adc: sun4i-gpadc-iio: rework: support clocks and reset

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

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

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

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 72 +++++++++++++++++++++++++++++++++++++--
1 file changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index c278e165e161..c12de48c4e86 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>

@@ -63,6 +65,9 @@ struct gpadc_data {
int (*ths_suspend)(struct sun4i_gpadc_iio *info);
int (*ths_resume)(struct sun4i_gpadc_iio *info);
bool support_irq;
+ bool has_bus_clk;
+ bool has_bus_rst;
+ bool has_mod_clk;
u32 temp_data_base;
};

@@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
struct mutex mutex;
struct thermal_zone_device *tzd;
struct device *sensor_device;
+ struct clk *bus_clk;
+ struct clk *mod_clk;
+ struct reset_control *reset;
};

static const struct iio_chan_spec sun4i_gpadc_channels[] = {
@@ -472,8 +480,13 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
if (IS_ERR(base))
return PTR_ERR(base);

- info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
- &sun4i_gpadc_regmap_config);
+ if (info->data->has_bus_clk)
+ info->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "bus",
+ base, &sun4i_gpadc_regmap_config);
+ else
+ info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
+ &sun4i_gpadc_regmap_config);
+
if (IS_ERR(info->regmap)) {
ret = PTR_ERR(info->regmap);
dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
@@ -498,9 +511,58 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
}
}

+ 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 4MHz */
+ 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;
+ }
+
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(struct platform_device *pdev)
@@ -586,6 +648,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
if (!info->data->support_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;
}

--
2.11.0


2018-08-30 15:48:53

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 16/30] iio: adc: sun4i-gpadc-iio: rework: readout temp_data

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

This commit reworks the code and uses regmap field to read out
temp_data.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index d48f338af563..c278e165e161 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -63,6 +63,7 @@ struct gpadc_data {
int (*ths_suspend)(struct sun4i_gpadc_iio *info);
int (*ths_resume)(struct sun4i_gpadc_iio *info);
bool support_irq;
+ u32 temp_data_base;
};

static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
@@ -77,6 +78,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
.adc_channel = true,
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
+ .temp_data_base = SUN4I_GPADC_TEMP_DATA,
};

static const struct gpadc_data sun5i_gpadc_data = {
@@ -89,6 +91,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
.adc_channel = true,
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
+ .temp_data_base = SUN4I_GPADC_TEMP_DATA,
};

static const struct gpadc_data sun6i_gpadc_data = {
@@ -101,12 +104,14 @@ static const struct gpadc_data sun6i_gpadc_data = {
.adc_channel = true,
.ths_irq_thread = sun4i_gpadc_data_irq_handler,
.support_irq = true,
+ .temp_data_base = SUN4I_GPADC_TEMP_DATA,
};

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_base = SUN4I_GPADC_TEMP_DATA,
};

struct sun4i_gpadc_iio {
@@ -271,18 +276,18 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
{
struct sun4i_gpadc_iio *info = iio_priv(indio_dev);

- if (!info->data->support_irq) {
- pm_runtime_get_sync(indio_dev->dev.parent);
+ if (info->data->adc_channel)
+ return sun4i_gpadc_read(indio_dev, 0, val,
+ SUN4I_GPADC_IRQ_TEMP_DATA);

- regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
+ pm_runtime_get_sync(indio_dev->dev.parent);

- pm_runtime_mark_last_busy(indio_dev->dev.parent);
- pm_runtime_put_autosuspend(indio_dev->dev.parent);
+ regmap_read(info->regmap, info->data->temp_data_base, val);

- return 0;
- }
+ pm_runtime_mark_last_busy(indio_dev->dev.parent);
+ pm_runtime_put_autosuspend(indio_dev->dev.parent);

- return sun4i_gpadc_read(indio_dev, 0, val, SUN4I_GPADC_IRQ_TEMP_DATA);
+ return 0;
}

static int sun4i_gpadc_temp_offset(struct iio_dev *indio_dev, int *val)
--
2.11.0


2018-08-30 15:49:18

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 10/30] iio: adc: rework irq and adc_channel handling

We rework the irq handling and the adc_channel handling.
This is requiered since we merge the mfd driver into the adc driver.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 157 ++++++++++++++++++++++++--------------
include/linux/mfd/sun4i-gpadc.h | 7 --
2 files changed, 98 insertions(+), 66 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 658a7e3e3370..a2027614ee0c 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -49,6 +49,8 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
}

+struct sun4i_gpadc_iio;
+
struct gpadc_data {
int temp_offset;
int temp_scale;
@@ -56,8 +58,15 @@ struct gpadc_data {
unsigned int tp_adc_select;
unsigned int (*adc_chan_select)(unsigned int chan);
unsigned int adc_chan_mask;
+ bool adc_channel;
+ irqreturn_t (*ths_irq_thread)(int irq, void *dev_id);
+ int (*ths_suspend)(struct sun4i_gpadc_iio *info);
+ int (*ths_resume)(struct sun4i_gpadc_iio *info);
+ bool support_irq;
};

+static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
+
static const struct gpadc_data sun4i_gpadc_data = {
.temp_offset = -1932,
.temp_scale = 133,
@@ -65,6 +74,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,
+ .adc_channel = true,
+ .ths_irq_thread = sun4i_gpadc_data_irq_handler,
+ .support_irq = true,
};

static const struct gpadc_data sun5i_gpadc_data = {
@@ -74,6 +86,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,
+ .adc_channel = true,
+ .ths_irq_thread = sun4i_gpadc_data_irq_handler,
+ .support_irq = true,
};

static const struct gpadc_data sun6i_gpadc_data = {
@@ -83,6 +98,9 @@ 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,
+ .adc_channel = true,
+ .ths_irq_thread = sun4i_gpadc_data_irq_handler,
+ .support_irq = true,
};

static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -96,13 +114,10 @@ struct sun4i_gpadc_iio {
struct completion completion;
int temp_data;
u32 adc_data;
+ unsigned int irq_data_type;
struct regmap *regmap;
- unsigned int fifo_data_irq;
- atomic_t ignore_fifo_data_irq;
- unsigned int temp_data_irq;
- atomic_t ignore_temp_data_irq;
+ unsigned int irq;
const struct gpadc_data *data;
- bool no_irq;
/* prevents concurrent reads of temperature and ADC */
struct mutex mutex;
struct thermal_zone_device *tzd;
@@ -130,6 +145,20 @@ static const struct regmap_config sun4i_gpadc_regmap_config = {
.fast_io = true,
};

+static int sun4i_gpadc_irq_init(struct sun4i_gpadc_iio *info)
+{
+ u32 reg;
+
+ if (info->irq_data_type == SUN4I_GPADC_IRQ_FIFO_DATA)
+ reg = SUN4I_GPADC_INT_FIFOC_TEMP_IRQ_EN;
+ else
+ reg = SUN4I_GPADC_INT_FIFOC_TEMP_IRQ_EN;
+
+ regmap_write(info->regmap, SUN4I_GPADC_INT_FIFOC, reg);
+
+ return 0;
+}
+
static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
unsigned int irq)
{
@@ -151,7 +180,7 @@ static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
if (ret)
return ret;

- if (irq == info->fifo_data_irq) {
+ if (irq == SUN4I_GPADC_IRQ_FIFO_DATA) {
ret = regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
info->data->tp_mode_en |
info->data->tp_adc_select |
@@ -172,6 +201,8 @@ static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
ret = regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
info->data->tp_mode_en);
}
+ if (info->data->support_irq)
+ sun4i_gpadc_irq_init(info);

if (ret)
return ret;
@@ -194,11 +225,12 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,

mutex_lock(&info->mutex);

+ info->irq_data_type = irq;
ret = sun4i_prepare_for_irq(indio_dev, channel, irq);
if (ret)
goto err;

- enable_irq(irq);
+ enable_irq(info->irq);

/*
* The temperature sensor throws an interruption periodically (currently
@@ -212,7 +244,7 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,
goto err;
}

- if (irq == info->fifo_data_irq)
+ if (irq == SUN4I_GPADC_IRQ_FIFO_DATA)
*val = info->adc_data;
else
*val = info->temp_data;
@@ -222,7 +254,7 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,

err:
pm_runtime_put_autosuspend(indio_dev->dev.parent);
- disable_irq(irq);
+ disable_irq(info->irq);
mutex_unlock(&info->mutex);

return ret;
@@ -231,16 +263,15 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,
static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
int *val)
{
- struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
-
- return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
+ return sun4i_gpadc_read(indio_dev, channel, val,
+ SUN4I_GPADC_IRQ_FIFO_DATA);
}

static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
{
struct sun4i_gpadc_iio *info = iio_priv(indio_dev);

- if (info->no_irq) {
+ if (!info->data->support_irq) {
pm_runtime_get_sync(indio_dev->dev.parent);

regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
@@ -251,7 +282,7 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
return 0;
}

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

static int sun4i_gpadc_temp_offset(struct iio_dev *indio_dev, int *val)
@@ -320,31 +351,21 @@ static const struct iio_info sun4i_gpadc_iio_info = {
.read_raw = sun4i_gpadc_read_raw,
};

-static irqreturn_t sun4i_gpadc_temp_data_irq_handler(int irq, void *dev_id)
+static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
{
struct sun4i_gpadc_iio *info = dev_id;

- if (atomic_read(&info->ignore_temp_data_irq))
- goto out;
-
- if (!regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, &info->temp_data))
- complete(&info->completion);
-
-out:
- return IRQ_HANDLED;
-}
-
-static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
-{
- struct sun4i_gpadc_iio *info = dev_id;
-
- if (atomic_read(&info->ignore_fifo_data_irq))
- goto out;
-
- if (!regmap_read(info->regmap, SUN4I_GPADC_DATA, &info->adc_data))
- complete(&info->completion);
-
-out:
+ if (info->irq_data_type == SUN4I_GPADC_IRQ_FIFO_DATA) {
+ /* read fifo data */
+ if (!regmap_read(info->regmap, SUN4I_GPADC_DATA,
+ &info->adc_data))
+ complete(&info->completion);
+ } else {
+ /* read temp data */
+ if (!regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA,
+ &info->temp_data))
+ complete(&info->completion);
+ }
return IRQ_HANDLED;
}

@@ -356,6 +377,8 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
/* Disable temperature sensor on IP */
regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
+ /* Disable irq*/
+ regmap_write(info->regmap, SUN4I_GPADC_INT_FIFOC, 0);

return 0;
}
@@ -378,6 +401,7 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
SUN4I_GPADC_TPR_TEMP_ENABLE |
SUN4I_GPADC_TPR_TEMP_PERIOD(800));

+
return 0;
}

@@ -426,8 +450,6 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
if (!info->data)
return -ENODEV;

- info->no_irq = true;
-
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(base))
@@ -441,8 +463,25 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
return ret;
}

- if (IS_ENABLED(CONFIG_THERMAL_OF))
- info->sensor_device = &pdev->dev;
+ if (info->data->support_irq) {
+
+ /* ths interrupt */
+ info->irq = platform_get_irq(pdev, 0);
+
+ ret = devm_request_threaded_irq(&pdev->dev, info->irq,
+ NULL, info->data->ths_irq_thread,
+ IRQF_ONESHOT, dev_name(&pdev->dev), info);
+
+ if (info->data->adc_channel)
+ disable_irq(info->irq);
+
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add ths irq: %d\n", ret);
+ return ret;
+ }
+ }
+
+ info->sensor_device = &pdev->dev;

return 0;
}
@@ -469,6 +508,11 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
indio_dev->info = &sun4i_gpadc_iio_info;
indio_dev->modes = INDIO_DIRECT_MODE;

+ if (&info->data->adc_channel) {
+ indio_dev->num_channels = ARRAY_SIZE(sun4i_gpadc_channels);
+ indio_dev->channels = sun4i_gpadc_channels;
+ }
+
ret = sun4i_gpadc_probe_dt(pdev, indio_dev);

if (ret)
@@ -480,20 +524,18 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
pm_runtime_set_suspended(&pdev->dev);
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);
- /*
- * Do not fail driver probing when failing to register in
- * thermal because no thermal DT node is found.
- */
- if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
- dev_err(&pdev->dev,
- "could not register thermal sensor: %ld\n",
- PTR_ERR(info->tzd));
- return PTR_ERR(info->tzd);
- }
+ info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
+ 0, info,
+ &sun4i_ts_tz_ops);
+ /*
+ * Do not fail driver probing when failing to register in
+ * thermal because no thermal DT node is found.
+ */
+ if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
+ dev_err(&pdev->dev,
+ "could not register thermal sensor: %ld\n",
+ PTR_ERR(info->tzd));
+ return PTR_ERR(info->tzd);
}

ret = devm_iio_device_register(&pdev->dev, indio_dev);
@@ -505,7 +547,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
return 0;

err_map:
- if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
+ if (!info->data->support_irq)
iio_map_array_unregister(indio_dev);

pm_runtime_put(&pdev->dev);
@@ -522,12 +564,9 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);

- if (!IS_ENABLED(CONFIG_THERMAL_OF))
- return 0;
-
thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);

- if (!info->no_irq)
+ if (!info->data->support_irq)
iio_map_array_unregister(indio_dev);

return 0;
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 54c7c9375c1b..ca59336f246b 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -99,11 +99,4 @@
.datasheet_name = _name, \
}

-struct sun4i_gpadc_dev {
- struct device *dev;
- struct regmap *regmap;
- struct regmap_irq_chip_data *regmap_irqc;
- void __iomem *base;
-};
-
#endif
--
2.11.0


2018-08-30 15:49:19

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 13/30] arm: config: Enable SUN4I_GPADC in defconfig

Since we have now new compatibles we can enable the SUN4I_GPADC driver
next to the sun4i-ts driver.

Signed-off-by: Philipp Rossak <[email protected]>
---
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 df433abfcb02..2189349820ac 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -132,6 +132,7 @@ CONFIG_DMA_SUN6I=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_EXTCON=y
CONFIG_IIO=y
+CONFIG_SUN4I_GPADC=y
CONFIG_AXP20X_ADC=y
CONFIG_PWM=y
CONFIG_PWM_SUN4I=y
--
2.11.0


2018-08-30 15:49:21

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 11/30] iio: adc: add new compatibles

We are now adding the new compatibles.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index a2027614ee0c..79b8efdab803 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -435,6 +435,18 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
.compatible = "allwinner,sun8i-a33-ths",
.data = &sun8i_a33_gpadc_data,
},
+ {
+ .compatible = "allwinner,sun4i-a10-gpadc",
+ .data = &sun4i_gpadc_data
+ },
+ {
+ .compatible = "allwinner,sun5i-a13-gpadc",
+ .data = &sun5i_gpadc_data
+ },
+ {
+ .compatible = "allwinner,sun6i-a31-gpadc",
+ .data = &sun6i_gpadc_data
+ },
{ /* sentinel */ }
};

--
2.11.0


2018-08-30 15:49:29

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 15/30] iio: adc: sun4i-gpadc-iio: rename A33-specified registers to contain A33

From: Icenowy Zheng <[email protected]>

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 <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
Acked-by: Lee Jones <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
include/linux/iio/adc/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 e1fe5e8e9dc0..d48f338af563 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -106,7 +106,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/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
index ca59336f246b..d6850f39dcfb 100644
--- a/include/linux/iio/adc/sun4i-gpadc.h
+++ b/include/linux/iio/adc/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

--
2.11.0


2018-08-30 15:49:37

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 09/30] iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i channel

We want to use this driver mostly as thermal sensor, that still supports
the adc for the older chips, thus we threat the A33 as thermal sensor.
We also remove the adc channel without thermal support.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 19 -------------------
1 file changed, 19 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index ab474ce86fb6..658a7e3e3370 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -123,23 +123,6 @@ static const struct iio_chan_spec sun4i_gpadc_channels[] = {
},
};

-static const struct iio_chan_spec sun4i_gpadc_channels_no_temp[] = {
- SUN4I_GPADC_ADC_CHANNEL(0, "adc_chan0"),
- SUN4I_GPADC_ADC_CHANNEL(1, "adc_chan1"),
- SUN4I_GPADC_ADC_CHANNEL(2, "adc_chan2"),
- SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
-};
-
-static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
- {
- .type = IIO_TEMP,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_OFFSET),
- .datasheet_name = "temp_adc",
- },
-};
-
static const struct regmap_config sun4i_gpadc_regmap_config = {
.reg_bits = 32,
.val_bits = 32,
@@ -444,8 +427,6 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
return -ENODEV;

info->no_irq = true;
- indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
- indio_dev->channels = sun8i_a33_gpadc_channels;

mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(&pdev->dev, mem);
--
2.11.0


2018-08-30 15:49:39

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 07/30] iio: adc: remove mfd_probe & sunwi_irq_init function

In the previous commit we removed the function call, now we remove the
unused functions.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 126 --------------------------------------
1 file changed, 126 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index d6f00d3b802d..f787442a9e5f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -431,55 +431,6 @@ static const struct dev_pm_ops sun4i_gpadc_pm_ops = {
.runtime_resume = &sun4i_gpadc_runtime_resume,
};

-static int sun4i_irq_init(struct platform_device *pdev, const char *name,
- irq_handler_t handler, const char *devname,
- unsigned int *irq, atomic_t *atomic)
-{
- int ret;
- struct sun4i_gpadc_dev *mfd_dev = dev_get_drvdata(pdev->dev.parent);
- struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(&pdev->dev));
-
- /*
- * Once the interrupt is activated, the IP continuously performs
- * conversions thus throws interrupts. The interrupt is activated right
- * after being requested but we want to control when these interrupts
- * occur thus we disable it right after being requested. However, an
- * interrupt might occur between these two instructions and we have to
- * make sure that does not happen, by using atomic flags. We set the
- * flag before requesting the interrupt and unset it right after
- * disabling the interrupt. When an interrupt occurs between these two
- * instructions, reading the atomic flag will tell us to ignore the
- * interrupt.
- */
- atomic_set(atomic, 1);
-
- ret = platform_get_irq_byname(pdev, name);
- if (ret < 0) {
- dev_err(&pdev->dev, "no %s interrupt registered\n", name);
- return ret;
- }
-
- ret = regmap_irq_get_virq(mfd_dev->regmap_irqc, ret);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to get virq for irq %s\n", name);
- return ret;
- }
-
- *irq = ret;
- ret = devm_request_any_context_irq(&pdev->dev, *irq, handler, 0,
- devname, info);
- if (ret < 0) {
- dev_err(&pdev->dev, "could not request %s interrupt: %d\n",
- name, ret);
- return ret;
- }
-
- disable_irq(*irq);
- atomic_set(atomic, 0);
-
- return 0;
-}
-
static const struct of_device_id sun4i_gpadc_of_id[] = {
{
.compatible = "allwinner,sun8i-a33-ths",
@@ -523,83 +474,6 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
return 0;
}

-static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
- struct iio_dev *indio_dev)
-{
- struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
- struct sun4i_gpadc_dev *sun4i_gpadc_dev =
- dev_get_drvdata(pdev->dev.parent);
- int ret;
-
- info->no_irq = false;
- info->regmap = sun4i_gpadc_dev->regmap;
-
- indio_dev->num_channels = ARRAY_SIZE(sun4i_gpadc_channels);
- indio_dev->channels = sun4i_gpadc_channels;
-
- info->data = (struct gpadc_data *)platform_get_device_id(pdev)->driver_data;
-
- /*
- * Since the controller needs to be in touchscreen mode for its thermal
- * sensor to operate properly, and that switching between the two modes
- * needs a delay, always registering in the thermal framework will
- * significantly slow down the conversion rate of the ADCs.
- *
- * Therefore, instead of depending on THERMAL_OF in Kconfig, we only
- * register the sensor if that option is enabled, eventually leaving
- * that choice to the user.
- */
-
- if (IS_ENABLED(CONFIG_THERMAL_OF)) {
- /*
- * This driver is a child of an MFD which has a node in the DT
- * but not its children, because of DT backward compatibility
- * for A10, A13 and A31 SoCs. Therefore, the resulting devices
- * of this driver do not have an of_node variable.
- * However, its parent (the MFD driver) has an of_node variable
- * and since devm_thermal_zone_of_sensor_register uses its first
- * argument to match the phandle defined in the node of the
- * thermal driver with the of_node of the device passed as first
- * argument and the third argument to call ops from
- * thermal_zone_of_device_ops, the solution is to use the parent
- * device as first argument to match the phandle with its
- * of_node, and the device from this driver as third argument to
- * return the temperature.
- */
- info->sensor_device = pdev->dev.parent;
- } else {
- indio_dev->num_channels =
- ARRAY_SIZE(sun4i_gpadc_channels_no_temp);
- indio_dev->channels = sun4i_gpadc_channels_no_temp;
- }
-
- if (IS_ENABLED(CONFIG_THERMAL_OF)) {
- ret = sun4i_irq_init(pdev, "TEMP_DATA_PENDING",
- sun4i_gpadc_temp_data_irq_handler,
- "temp_data", &info->temp_data_irq,
- &info->ignore_temp_data_irq);
- if (ret < 0)
- return ret;
- }
-
- ret = sun4i_irq_init(pdev, "FIFO_DATA_PENDING",
- sun4i_gpadc_fifo_data_irq_handler, "fifo_data",
- &info->fifo_data_irq, &info->ignore_fifo_data_irq);
- if (ret < 0)
- return ret;
-
- if (IS_ENABLED(CONFIG_THERMAL_OF)) {
- ret = iio_map_array_register(indio_dev, sun4i_gpadc_hwmon_maps);
- if (ret < 0) {
- dev_err(&pdev->dev,
- "failed to register iio map array\n");
- return ret;
- }
- }
-
- return 0;
-}
-
static int sun4i_gpadc_probe(struct platform_device *pdev)
{
struct sun4i_gpadc_iio *info;
--
2.11.0


2018-08-30 15:49:41

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 12/30] mfd: Remove old mfd driver & Move sun4i-gpadc.h to iio/adc/

Since we reworked the sun4i-gpadc iio driver we can now remove the mfd
driver and move it's header to include/linux/iio/adc.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
drivers/mfd/sun4i-gpadc.c | 181 ---------------------------
include/linux/{mfd => iio/adc}/sun4i-gpadc.h | 0
3 files changed, 1 insertion(+), 182 deletions(-)
delete mode 100644 drivers/mfd/sun4i-gpadc.c
rename include/linux/{mfd => iio/adc}/sun4i-gpadc.h (100%)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 79b8efdab803..e1fe5e8e9dc0 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -37,7 +37,7 @@
#include <linux/iio/iio.h>
#include <linux/iio/driver.h>
#include <linux/iio/machine.h>
-#include <linux/mfd/sun4i-gpadc.h>
+#include <linux/iio/adc/sun4i-gpadc.h>

static unsigned int sun4i_gpadc_chan_select(unsigned int chan)
{
diff --git a/drivers/mfd/sun4i-gpadc.c b/drivers/mfd/sun4i-gpadc.c
deleted file mode 100644
index 9cfc88134d03..000000000000
--- a/drivers/mfd/sun4i-gpadc.c
+++ /dev/null
@@ -1,181 +0,0 @@
-/* ADC MFD core driver for sunxi platforms
- *
- * Copyright (c) 2016 Quentin Schulz <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- */
-
-#include <linux/interrupt.h>
-#include <linux/kernel.h>
-#include <linux/mfd/core.h>
-#include <linux/module.h>
-#include <linux/of_device.h>
-#include <linux/of_irq.h>
-#include <linux/regmap.h>
-
-#include <linux/mfd/sun4i-gpadc.h>
-
-#define ARCH_SUN4I_A10 0
-#define ARCH_SUN5I_A13 1
-#define ARCH_SUN6I_A31 2
-
-static struct resource adc_resources[] = {
- DEFINE_RES_IRQ_NAMED(SUN4I_GPADC_IRQ_FIFO_DATA, "FIFO_DATA_PENDING"),
- DEFINE_RES_IRQ_NAMED(SUN4I_GPADC_IRQ_TEMP_DATA, "TEMP_DATA_PENDING"),
-};
-
-static const struct regmap_irq sun4i_gpadc_regmap_irq[] = {
- REGMAP_IRQ_REG(SUN4I_GPADC_IRQ_FIFO_DATA, 0,
- SUN4I_GPADC_INT_FIFOC_TP_DATA_IRQ_EN),
- REGMAP_IRQ_REG(SUN4I_GPADC_IRQ_TEMP_DATA, 0,
- SUN4I_GPADC_INT_FIFOC_TEMP_IRQ_EN),
-};
-
-static const struct regmap_irq_chip sun4i_gpadc_regmap_irq_chip = {
- .name = "sun4i_gpadc_irq_chip",
- .status_base = SUN4I_GPADC_INT_FIFOS,
- .ack_base = SUN4I_GPADC_INT_FIFOS,
- .mask_base = SUN4I_GPADC_INT_FIFOC,
- .init_ack_masked = true,
- .mask_invert = true,
- .irqs = sun4i_gpadc_regmap_irq,
- .num_irqs = ARRAY_SIZE(sun4i_gpadc_regmap_irq),
- .num_regs = 1,
-};
-
-static struct mfd_cell sun4i_gpadc_cells[] = {
- {
- .name = "sun4i-a10-gpadc-iio",
- .resources = adc_resources,
- .num_resources = ARRAY_SIZE(adc_resources),
- },
- { .name = "iio_hwmon" }
-};
-
-static struct mfd_cell sun5i_gpadc_cells[] = {
- {
- .name = "sun5i-a13-gpadc-iio",
- .resources = adc_resources,
- .num_resources = ARRAY_SIZE(adc_resources),
- },
- { .name = "iio_hwmon" },
-};
-
-static struct mfd_cell sun6i_gpadc_cells[] = {
- {
- .name = "sun6i-a31-gpadc-iio",
- .resources = adc_resources,
- .num_resources = ARRAY_SIZE(adc_resources),
- },
- { .name = "iio_hwmon" },
-};
-
-static const struct regmap_config sun4i_gpadc_regmap_config = {
- .reg_bits = 32,
- .val_bits = 32,
- .reg_stride = 4,
- .fast_io = true,
-};
-
-static const struct of_device_id sun4i_gpadc_of_match[] = {
- {
- .compatible = "allwinner,sun4i-a10-ts",
- .data = (void *)ARCH_SUN4I_A10,
- }, {
- .compatible = "allwinner,sun5i-a13-ts",
- .data = (void *)ARCH_SUN5I_A13,
- }, {
- .compatible = "allwinner,sun6i-a31-ts",
- .data = (void *)ARCH_SUN6I_A31,
- }, { /* sentinel */ }
-};
-
-MODULE_DEVICE_TABLE(of, sun4i_gpadc_of_match);
-
-static int sun4i_gpadc_probe(struct platform_device *pdev)
-{
- struct sun4i_gpadc_dev *dev;
- struct resource *mem;
- const struct of_device_id *of_id;
- const struct mfd_cell *cells;
- unsigned int irq, size;
- int ret;
-
- of_id = of_match_node(sun4i_gpadc_of_match, pdev->dev.of_node);
- if (!of_id)
- return -EINVAL;
-
- switch ((long)of_id->data) {
- case ARCH_SUN4I_A10:
- cells = sun4i_gpadc_cells;
- size = ARRAY_SIZE(sun4i_gpadc_cells);
- break;
- case ARCH_SUN5I_A13:
- cells = sun5i_gpadc_cells;
- size = ARRAY_SIZE(sun5i_gpadc_cells);
- break;
- case ARCH_SUN6I_A31:
- cells = sun6i_gpadc_cells;
- size = ARRAY_SIZE(sun6i_gpadc_cells);
- break;
- default:
- return -EINVAL;
- }
-
- dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
- if (!dev)
- return -ENOMEM;
-
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- dev->base = devm_ioremap_resource(&pdev->dev, mem);
- if (IS_ERR(dev->base))
- return PTR_ERR(dev->base);
-
- dev->dev = &pdev->dev;
- dev_set_drvdata(dev->dev, dev);
-
- dev->regmap = devm_regmap_init_mmio(dev->dev, dev->base,
- &sun4i_gpadc_regmap_config);
- if (IS_ERR(dev->regmap)) {
- ret = PTR_ERR(dev->regmap);
- dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
- return ret;
- }
-
- /* Disable all interrupts */
- regmap_write(dev->regmap, SUN4I_GPADC_INT_FIFOC, 0);
-
- irq = platform_get_irq(pdev, 0);
- ret = devm_regmap_add_irq_chip(&pdev->dev, dev->regmap, irq,
- IRQF_ONESHOT, 0,
- &sun4i_gpadc_regmap_irq_chip,
- &dev->regmap_irqc);
- if (ret) {
- dev_err(&pdev->dev, "failed to add irq chip: %d\n", ret);
- return ret;
- }
-
- ret = devm_mfd_add_devices(dev->dev, 0, cells, size, NULL, 0, NULL);
- if (ret) {
- dev_err(&pdev->dev, "failed to add MFD devices: %d\n", ret);
- return ret;
- }
-
- return 0;
-}
-
-static struct platform_driver sun4i_gpadc_driver = {
- .driver = {
- .name = "sun4i-gpadc",
- .of_match_table = of_match_ptr(sun4i_gpadc_of_match),
- },
- .probe = sun4i_gpadc_probe,
-};
-
-module_platform_driver(sun4i_gpadc_driver);
-
-MODULE_DESCRIPTION("Allwinner sunxi platforms' GPADC MFD core driver");
-MODULE_AUTHOR("Quentin Schulz <[email protected]>");
-MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
similarity index 100%
rename from include/linux/mfd/sun4i-gpadc.h
rename to include/linux/iio/adc/sun4i-gpadc.h
--
2.11.0


2018-08-30 15:50:02

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 06/30] iio: adc: remove ofnode options

Since we are merging the mfd dirver into the adc driver we don't need
two different probing functions. Thus we remove the ofnode options

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 666329940e1e..d6f00d3b802d 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -622,10 +622,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
indio_dev->info = &sun4i_gpadc_iio_info;
indio_dev->modes = INDIO_DIRECT_MODE;

- if (pdev->dev.of_node)
- ret = sun4i_gpadc_probe_dt(pdev, indio_dev);
- else
- ret = sun4i_gpadc_probe_mfd(pdev, indio_dev);
+ ret = sun4i_gpadc_probe_dt(pdev, indio_dev);

if (ret)
return ret;
--
2.11.0


2018-08-30 15:50:08

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 08/30] iio: adc: remove hwmon structure

We remove the hwmon structure that was requiered for the mfd driver.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index f787442a9e5f..ab474ce86fb6 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -109,14 +109,6 @@ struct sun4i_gpadc_iio {
struct device *sensor_device;
};

-static struct iio_map sun4i_gpadc_hwmon_maps[] = {
- {
- .adc_channel_label = "temp_adc",
- .consumer_dev_name = "iio_hwmon.0",
- },
- { /* sentinel */ },
-};
-
static const struct iio_chan_spec sun4i_gpadc_channels[] = {
SUN4I_GPADC_ADC_CHANNEL(0, "adc_chan0"),
SUN4I_GPADC_ADC_CHANNEL(1, "adc_chan1"),
--
2.11.0


2018-08-30 15:50:14

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 03/30] iio: adc: Remove ID table

To disable the driver we are removing the compatibles.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 9 ---------
1 file changed, 9 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 04d7147e0110..d95dd0fde2a6 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -698,21 +698,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
return 0;
}

-static const struct platform_device_id sun4i_gpadc_id[] = {
- { "sun4i-a10-gpadc-iio", (kernel_ulong_t)&sun4i_gpadc_data },
- { "sun5i-a13-gpadc-iio", (kernel_ulong_t)&sun5i_gpadc_data },
- { "sun6i-a31-gpadc-iio", (kernel_ulong_t)&sun6i_gpadc_data },
- { /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(platform, sun4i_gpadc_id);
-
static struct platform_driver sun4i_gpadc_driver = {
.driver = {
.name = "sun4i-gpadc-iio",
.of_match_table = sun4i_gpadc_of_id,
.pm = &sun4i_gpadc_pm_ops,
},
- .id_table = sun4i_gpadc_id,
.probe = sun4i_gpadc_probe,
.remove = sun4i_gpadc_remove,
};
--
2.11.0


2018-08-30 15:50:22

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 05/30] iio: adc: move SUN4I_GPADC_CHANNEL define to header file

We are moving the SUN4I_GPADC_CHANNEL define to the header file.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 9 ---------
include/linux/mfd/sun4i-gpadc.h | 9 +++++++++
2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index d95dd0fde2a6..666329940e1e 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -109,15 +109,6 @@ struct sun4i_gpadc_iio {
struct device *sensor_device;
};

-#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
- .type = IIO_VOLTAGE, \
- .indexed = 1, \
- .channel = _channel, \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
- .datasheet_name = _name, \
-}
-
static struct iio_map sun4i_gpadc_hwmon_maps[] = {
{
.adc_channel_label = "temp_adc",
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 139872c2e0fe..54c7c9375c1b 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -90,6 +90,15 @@
/* 10s delay before suspending the IP */
#define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000

+#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .channel = _channel, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .datasheet_name = _name, \
+}
+
struct sun4i_gpadc_dev {
struct device *dev;
struct regmap *regmap;
--
2.11.0


2018-08-30 15:50:29

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 04/30] iio: adc: Kconfig: Update Kconfig to new build options

Since we are merging the mfd driver into the iio adc driver we need to
update the Kconfig build options.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/Kconfig | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9da79070357c..5d0cffd6d2e4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -713,13 +713,16 @@ config STX104
array module parameter.

config SUN4I_GPADC
- tristate "Support for the Allwinner SoCs GPADC"
+ tristate "Allwinner sunxi platforms' GPADC/Thermal driver"
+ select REGMAP_MMIO
+ select REGMAP_IRQ
depends on IIO
- depends on MFD_SUN4I_GPADC || MACH_SUN8I
- depends on THERMAL || !THERMAL_OF
+ depends on ARCH_SUNXI || MACH_SUN8I
+ depends on THERMAL && THERMAL_OF
help
Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
- GPADC. This ADC provides 4 channels which can be used as an ADC or as
+ GPADC or newer SOCs (A33, H3, A83T, ...) Thermal sensor driver.
+ This ADC provides 4 channels which can be used as an ADC or as
a touchscreen input and one channel for thermal sensor.

The thermal sensor slows down ADC readings and can be disabled by
--
2.11.0


2018-08-30 15:50:33

by Philipp Rossak

[permalink] [raw]
Subject: [PATCH v3 02/30] mfd: Kconfig: Remove MFD_SUN4I_GPADC config option

We are merging the mfd:sun4i-gpadc driver into the
iio/adc/sun4i-gpadc driver. So we need to remove the MFD_SUN4I_GPADC
config option.

Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/mfd/Kconfig | 17 -----------------
1 file changed, 17 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..c7ab57d65610 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -40,23 +40,6 @@ config MFD_ACT8945A
linear regulators, along with a complete ActivePath battery
charger.

-config MFD_SUN4I_GPADC
- tristate "Allwinner sunxi platforms' GPADC MFD driver"
- select MFD_CORE
- select REGMAP_MMIO
- select REGMAP_IRQ
- depends on ARCH_SUNXI || COMPILE_TEST
- depends on !TOUCHSCREEN_SUN4I
- help
- Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
- This driver will only map the hardware interrupt and registers, you
- have to select individual drivers based on this MFD to be able to use
- the ADC or the thermal sensor. This will try to probe the ADC driver
- sun4i-gpadc-iio and the hwmon driver iio_hwmon.
-
- To compile this driver as a module, choose M here: the module will be
- called sun4i-gpadc.
-
config MFD_AS3711
bool "AMS AS3711"
select MFD_CORE
--
2.11.0


2018-08-30 16:38:16

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

Hello,

On Thu, Aug 30, 2018 at 05:45:09PM +0200, Philipp Rossak wrote:
> This patch adds support for the H3 ths sensor.
>
> The H3 supports interrupts. The interrupt is configured to update the
> the sensor values every second. The calibration data is writen at the
> begin of the init process.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 91 +++++++++++++++++++++++++++++++++++++
> include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++
> 2 files changed, 109 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index c7b46c82e3e5..d5c7971b2558 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -72,6 +72,7 @@ struct gpadc_data {
> u32 temp_data_base;
> int sensor_count;
> bool supports_nvmem;
> + u32 ths_irq_clear;
> };
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> @@ -79,6 +80,10 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> static int sun4i_ths_resume(struct sun4i_gpadc_iio *info);
> static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info);
>
> +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info);
> +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info);
> +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data);
> +
> static const struct gpadc_data sun4i_gpadc_data = {
> .temp_offset = -1932,
> .temp_scale = 133,
> @@ -137,6 +142,22 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .sensor_count = 1,
> };
>
> +static const struct gpadc_data sun8i_h3_ths_data = {
> + .temp_offset = -1791,
> + .temp_scale = -121,
> + .temp_data_base = SUN8I_H3_THS_TDATA0,
> + .ths_irq_thread = sunx8i_h3_irq_thread,
> + .support_irq = true,
> + .has_bus_clk = true,
> + .has_bus_rst = true,
> + .has_mod_clk = true,
> + .sensor_count = 1,
> + .supports_nvmem = true,
> + .ths_resume = sun8i_h3_ths_resume,
> + .ths_suspend = sun8i_h3_ths_suspend,
> + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0,
> +};
> +
> struct sun4i_sensor_tzd {
> struct sun4i_gpadc_iio *info;
> struct thermal_zone_device *tzd;
> @@ -409,6 +430,31 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data)
> +{
> + struct sun4i_gpadc_iio *info = data;
> + int i;
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> + info->data->ths_irq_clear);
> +
> + for (i = 0; i < info->data->sensor_count; i++)
> + thermal_zone_device_update(info->tzds[i].tzd,
> + THERMAL_EVENT_TEMP_SAMPLE);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info)
> +{
> +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> +// info->calibration_data[0]);
> +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> +// info->calibration_data[1]);

This should probably be implemented, or left out completely.

regards,
o.

> + return 0;
> +}
> +
> static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sun8i_h3_ths_suspend(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_resume(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info)
> +{
> + u32 value;
> +
> + sun8i_h3_calibrate(info);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
> + SUN4I_GPADC_CTRL0_T_ACQ(0xff));
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> + SUN8I_H3_THS_ACQ1(0x3f));
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> + SUN8I_H3_THS_INTS_TDATA_IRQ_0);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
> + SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2));
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_INTC,
> + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> + SUN8I_H3_THS_TEMP_PERIOD(0x55));
> +
> + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> + SUN8I_H3_THS_TEMP_SENSE_EN0 | value);
> +
> + return 0;
> +}
> +
> static int sun4i_gpadc_get_temp(void *data, int *temp)
> {
> struct sun4i_sensor_tzd *tzd = data;
> @@ -497,6 +584,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
> .compatible = "allwinner,sun6i-a31-gpadc",
> .data = &sun6i_gpadc_data
> },
> + {
> + .compatible = "allwinner,sun8i-h3-ths",
> + .data = &sun8i_h3_ths_data,
> + },
> { /* sentinel */ }
> };
>
> diff --git a/include/linux/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
> index 99feeba28105..169b4de9a34d 100644
> --- a/include/linux/iio/adc/sun4i-gpadc.h
> +++ b/include/linux/iio/adc/sun4i-gpadc.h
> @@ -100,6 +100,24 @@
> }
>
> /* SUNXI_THS COMMON REGISTERS + DEFINES */
> +#define SUN8I_H3_THS_CTRL0 0x00
> +
> +#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)
> +
> +#define SUN8I_H3_THS_INTC 0x44
> +#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
> +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
> +
> +#define SUN8I_H3_THS_STAT 0x48
> +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
> +
> +#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 MAX_SENSOR_COUNT 4
>
> #endif
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.

2018-08-30 16:40:03

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

Hello,

On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:
> Since we have now thermal trotteling enabeled we can now add the full
> range of the OPP table.

I'm not sure we can. I have a tablet with A83T SoC and it gets unstable
at these frequencies even with thermal throttling on mainline kernel. (Though
I have my own THS driver, but I doubt a different driver will change much.)

There might be some other issue left in the cpufreq code. I'll let others
test this on a better cooled boards though.

Did you/someone test this?

regards,
o.

> The operating points were found in Allwinner BSP and fex files.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t.dtsi | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index 78aa448e869f..ddcf404f9c80 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -250,6 +250,22 @@
> opp-microvolt = <840000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
> +
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <920000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-1800000000 { /* BOOT FREQ */
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <1000000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + opp-microvolt = <1080000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> };
>
> cpu1_opp_table: opp_table1 {
> @@ -303,6 +319,22 @@
> opp-microvolt = <840000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
> +
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <920000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-1800000000 { /* BOOT FREQ */
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <1000000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + opp-microvolt = <1080000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> };
>
> soc {
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.

2018-08-30 16:42:18

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH v3 09/30] iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i channel

On Thu, Aug 30, 2018 at 05:44:57PM +0200, Philipp Rossak wrote:
> We want to use this driver mostly as thermal sensor, that still supports
> the adc for the older chips, thus we threat the A33 as thermal sensor.
> We also remove the adc channel without thermal support.

Threat -> treat (in the title and in the message body too)

> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 19 -------------------
> 1 file changed, 19 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index ab474ce86fb6..658a7e3e3370 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -123,23 +123,6 @@ static const struct iio_chan_spec sun4i_gpadc_channels[] = {
> },
> };
>
> -static const struct iio_chan_spec sun4i_gpadc_channels_no_temp[] = {
> - SUN4I_GPADC_ADC_CHANNEL(0, "adc_chan0"),
> - SUN4I_GPADC_ADC_CHANNEL(1, "adc_chan1"),
> - SUN4I_GPADC_ADC_CHANNEL(2, "adc_chan2"),
> - SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
> -};
> -
> -static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
> - {
> - .type = IIO_TEMP,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_SCALE) |
> - BIT(IIO_CHAN_INFO_OFFSET),
> - .datasheet_name = "temp_adc",
> - },
> -};
> -
> static const struct regmap_config sun4i_gpadc_regmap_config = {
> .reg_bits = 32,
> .val_bits = 32,
> @@ -444,8 +427,6 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> return -ENODEV;
>
> info->no_irq = true;
> - indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
> - indio_dev->channels = sun8i_a33_gpadc_channels;
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> base = devm_ioremap_resource(&pdev->dev, mem);
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.

2018-08-30 20:03:57

by Philipp Rossak

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

On 30.08.2018 18:27, Ondřej Jirman wrote:
>> +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info)
>> +{
>> +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
>> +// info->calibration_data[0]);
>> +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
>> +// info->calibration_data[1]);
> This should probably be implemented, or left out completely.
>
> regards,
> o.
>
Thanks you are right!
This should be implemented! I will fix this in the next version!

Thanks,
Philipp

2018-08-30 20:32:31

by Philipp Rossak

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

On 30.08.2018 18:38, Ondřej Jirman wrote:
> Hello,
>
> On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:
>> Since we have now thermal trotteling enabeled we can now add the full
>> range of the OPP table.
>
> I'm not sure we can. I have a tablet with A83T SoC and it gets unstable
> at these frequencies even with thermal throttling on mainline kernel. (Though
> I have my own THS driver, but I doubt a different driver will change much.)
>
> There might be some other issue left in the cpufreq code. I'll let others
> test this on a better cooled boards though.
>
> Did you/someone test this?
>
> regards,
> o.
I have a good cooled device, with big heatsinks and a fan blowing
directly on it. But there is some big issue left!

cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
[ 85.076270] Unable to handle kernel paging request at virtual address
2e83c684
[ 85.083519] pgd = (ptrval)
[ 85.086220] [2e83c684] *pgd=00000000
[ 85.089813] Internal error: Oops: 5 [#3] SMP ARM
[ 85.094429] Modules linked in:
[ 85.097483] CPU: 4 PID: 127 Comm: sh Tainted: G D W
4.18.0-00031-g8f59917020b9-dirty #2
[ 85.106597] Hardware name: Allwinner A83t board
[ 85.111130] PC is at down_write+0x14/0x54
[ 85.115135] LR is at anon_vma_clone+0x9c/0x1e4
[ 85.119571] pc : [<c066a3d4>] lr : [<c01e71b8>] psr: 60000013
[ 85.125826] sp : ede45e70 ip : 000001a0 fp : eea3c690
[ 85.131041] r10: eea4193c r9 : 00400200 r8 : c0a942a4
[ 85.136255] r7 : 2e83c680 r6 : eea3aea0 r5 : eea3b220 r4 : 2e83c684
[ 85.142771] r3 : ffff0001 r2 : 2e83c680 r1 : ede45e58 r0 : 2e83c684
[ 85.149287] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment none
[ 85.156410] Control: 10c5387d Table: 6df1806a DAC: 00000051
[ 85.162145] Process sh (pid: 127, stack limit = 0x(ptrval))
[ 85.167706] Stack: (0xede45e70 to 0xede46000)
[ 85.172057] 5e60: eea3b900
c01e71b8 eea41900 ffebd684
[ 85.180221] 5e80: c0a637b0 c07ea07c c0a10754 eea3aea0 eea41900
c0a04c48 edd71880 00000000
[ 85.188385] 5ea0: eea3aea0 eea41900 c0a69030 c01e7324 00000002
edd701c0 c0a04c48 edd71880
[ 85.196548] 5ec0: 00000000 c011c2f4 00000069 00000000 00000002
00000000 00000000 00000000
[ 85.204711] 5ee0: 00000000 edd701c0 edd82280 eea3af00 edd701f8
edd718b8 eea3af08 eea3af14
[ 85.212875] 5f00: eea3af10 ede44000 edd8255c 01200011 00000000
ede45f14 ede45f14 b61ea11a
[ 85.221039] 5f20: edd805c0 01200011 c0a04c48 beef38b0 00000000
00000000 00000000 00000078
[ 85.229202] 5f40: beef38dc c011ce0c 00000000 00000000 00000000
ede44000 000000ae c012d9e0
[ 85.237365] 5f60: eea43480 00000000 04000000 b61ea11a 00000000
b6fb5068 b6f8b670 beef38b0
[ 85.245529] 5f80: 00000078 c0101204 ede44000 00000078 beef38dc
c011d1bc b6fb5068 00000000
[ 85.253692] 5fa0: 000000ae c0101000 b6fb5068 b6f8b670 01200011
00000000 00000000 00000000
[ 85.261856] 5fc0: b6fb5068 b6f8b670 beef38b0 00000078 b6f8ac4c
b6fb5490 ffffffff beef38dc
[ 85.270020] 5fe0: 00000002 beef38b0 00000000 b6f5bfd0 60000010
01200011 00000000 00000000
[ 85.278191] [<c066a3d4>] (down_write) from [<c01e71b8>]
(anon_vma_clone+0x9c/0x1e4)
[ 85.285836] [<c01e71b8>] (anon_vma_clone) from [<c01e7324>]
(anon_vma_fork+0x24/0x160)
[ 85.293741] [<c01e7324>] (anon_vma_fork) from [<c011c2f4>]
(copy_process.part.3+0xbc4/0x158c)
[ 85.302253] [<c011c2f4>] (copy_process.part.3) from [<c011ce0c>]
(_do_fork+0xb0/0x394)
[ 85.310157] [<c011ce0c>] (_do_fork) from [<c011d1bc>]
(sys_clone+0x20/0x28)
[ 85.317107] [<c011d1bc>] (sys_clone) from [<c0101000>]
(ret_fast_syscall+0x0/0x54)
[ 85.324661] Exception stack(0xede45fa8 to 0xede45ff0)
[ 85.329704] 5fa0: b6fb5068 b6f8b670 01200011
00000000 00000000 00000000
[ 85.337868] 5fc0: b6fb5068 b6f8b670 beef38b0 00000078 b6f8ac4c
b6fb5490 ffffffff beef38dc
[ 85.346021] 5fe0: 00000002 beef38b0 00000000 b6f5bfd0
[ 85.351068] Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f)
[ 85.357200] ---[ end trace aad10e0b4fcbf194 ]---


OR

cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq
[ 73.873939] ------------[ cut here ]------------
[ 73.878584] WARNING: CPU: 5 PID: 132 at mm/rmap.c:235
unlink_anon_vmas+0x1f4/0x1fc
[ 73.886210] Modules linked in:
[ 73.889276] CPU: 5 PID: 132 Comm: sh Tainted: G D
4.18.0-00031-g8f59917020b9-dirty #2
[ 73.898391] Hardware name: Allwinner A83t board
[ 73.902934] [<c010f310>] (unwind_backtrace) from [<c010bd54>]
(show_stack+0x10/0x14)
[ 73.910671] [<c010bd54>] (show_stack) from [<c0652f98>]
(dump_stack+0x84/0x98)
[ 73.917887] [<c0652f98>] (dump_stack) from [<c011dd14>]
(__warn+0xfc/0x114)
[ 73.924840] [<c011dd14>] (__warn) from [<c011de44>]
(warn_slowpath_null+0x40/0x48)
[ 73.932398] [<c011de44>] (warn_slowpath_null) from [<c01e7114>]
(unlink_anon_vmas+0x1f4/0x1fc)
[ 73.941006] [<c01e7114>] (unlink_anon_vmas) from [<c01da874>]
(free_pgtables+0x78/0xcc)
[ 73.948999] [<c01da874>] (free_pgtables) from [<c01e1e88>]
(exit_mmap+0xe4/0x188)
[ 73.956471] [<c01e1e88>] (exit_mmap) from [<c011b3a8>] (mmput+0x40/0xf0)
[ 73.963169] [<c011b3a8>] (mmput) from [<c0208f1c>]
(flush_old_exec+0x550/0x6e0)
[ 73.970473] [<c0208f1c>] (flush_old_exec) from [<c0250e74>]
(load_elf_binary+0x2f0/0x1324)
[ 73.978726] [<c0250e74>] (load_elf_binary) from [<c02086c4>]
(search_binary_handler+0xa0/0x234)
[ 73.987412] [<c02086c4>] (search_binary_handler) from [<c02098b0>]
(__do_execve_file+0x57c/0x6bc)
[ 73.996271] [<c02098b0>] (__do_execve_file) from [<c0209c54>]
(sys_execve+0x34/0x3c)
[ 74.004003] [<c0209c54>] (sys_execve) from [<c0101000>]
(ret_fast_syscall+0x0/0x54)
[ 74.011645] Exception stack(0xede71fa8 to 0xede71ff0)
[ 74.016690] 1fa0: 000c530c 000c52d0 000c530c
000c52d0 000c52dc 00000000
[ 74.024853] 1fc0: 000c530c 000c52d0 000a2a0c 0000000b 000c52dc
000c4b5c 000c4b5c 000c530c
[ 74.033016] 1fe0: b6f25ed4 beef38b8 00042038 b6f25ee0
[ 74.038086] ---[ end trace aad10e0b4fcbf192 ]---
[ 74.042726] Unable to handle kernel paging request at virtual address
2e83c684
[ 74.049958] pgd = (ptrval)
[ 74.052665] [2e83c684] *pgd=00000000
[ 74.056243] Internal error: Oops: 5 [#2] SMP ARM
[ 74.060851] Modules linked in:
[ 74.063904] CPU: 5 PID: 132 Comm: sh Tainted: G D W
4.18.0-00031-g8f59917020b9-dirty #2
[ 74.073019] Hardware name: Allwinner A83t board
[ 74.077548] PC is at down_write+0x14/0x54
[ 74.081553] LR is at unlink_anon_vmas+0xa8/0x1fc
[ 74.086160] pc : [<c066a3d4>] lr : [<c01e6fc8>] psr: 60000013
[ 74.092416] sp : ede71d88 ip : 00000000 fp : eea3b680
[ 74.097630] r10: eea3c690 r9 : c0a637b0 r8 : eea41e40
[ 74.102845] r7 : c0a942a4 r6 : 2e83c680 r5 : eea41e7c r4 : 2e83c684
[ 74.109360] r3 : ffff0001 r2 : ffff0001 r1 : 00000000 r0 : 2e83c684
[ 74.115878] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment none
[ 74.123000] Control: 10c5387d Table: 6de6406a DAC: 00000051
[ 74.128735] Process sh (pid: 132, stack limit = 0x(ptrval))
[ 74.134296] Stack: (0xede71d88 to 0xede72000)
[ 74.138645] 1d80: eea41e74 c01e6fc8 c07ea07c
eea3c690 eea41f00 eea41e40
[ 74.146809] 1da0: eea41d20 b6f17000 ede71df4 00002000 00000000
edd9c300 ede71e90 c01da874
[ 74.154973] 1dc0: b6f17000 c01dbf94 00000000 eea41360 00000000
c0a04c48 edd716c0 edd826b0
[ 74.163136] 1de0: edd90540 c01e1e88 eeabc6c8 00000001 00000000
edd716c0 00000001 00000000
[ 74.171299] 1e00: 00000000 ffffffff c0a68f40 00000000 000000ac
00000400 eeacf000 eeb9f038
[ 74.179463] 1e20: 00000100 b61ea11a 00000000 edd716c0 00000000
edda8540 edd716c0 b61ea11a
[ 74.187627] 1e40: edd716c0 00000000 edda8540 c011b3a8 edd716c0
edd82280 edda8540 c0208f1c
[ 74.195790] 1e60: 000000a0 c024f610 000000d4 c0a04c48 eeab4100
00000000 edd9c300 eeab4134
[ 74.203954] 1e80: eeabc6c0 edd9c400 ede71e90 c0250e74 effcec80
effcec80 00000001 eeabc6c0
[ 74.212118] 1ea0: eeaab180 eeabb000 00000000 c01d9704 effcec80
80000013 beffff22 00000000
[ 74.220281] 1ec0: ede70000 effcec80 edd9c300 c01d9344 00000000
00000004 beffff22 00000000
[ 74.228445] 1ee0: 00000034 00000000 00000011 ede71f20 00000000
00000000 00000051 b61ea11a
[ 74.236609] 1f00: befff000 c0a1130c edd9c300 c0a9558c fffffff8
c0a10e3c c0a9558c c07ebbb4
[ 74.244772] 1f20: 00000001 c02086c4 c0a0bbd4 c0a04c48 edda2000
ffffe000 00000084 edd9c300
[ 74.252936] 1f40: 00000001 00000000 00000000 c02098b0 c0a04f34
edda8540 eeab8de0 edda8578
[ 74.261100] 1f60: 00000000 b61ea11a 000c530c 000c52d0 000c52dc
000a2a0c 0000000b c0101204
[ 74.269263] 1f80: ede70000 0000000b 000c530c c0209c54 00000000
00000000 20000010 000c530c
[ 74.277428] 1fa0: 000c52d0 c0101000 000c530c 000c52d0 000c530c
000c52d0 000c52dc 00000000
[ 74.285592] 1fc0: 000c530c 000c52d0 000a2a0c 0000000b 000c52dc
000c4b5c 000c4b5c 000c530c
[ 74.293755] 1fe0: b6f25ed4 beef38b8 00042038 b6f25ee0 a0000010
000c530c 00000000 00000000
[ 74.301924] [<c066a3d4>] (down_write) from [<c01e6fc8>]
(unlink_anon_vmas+0xa8/0x1fc)
[ 74.309743] [<c01e6fc8>] (unlink_anon_vmas) from [<c01da874>]
(free_pgtables+0x78/0xcc)
[ 74.317733] [<c01da874>] (free_pgtables) from [<c01e1e88>]
(exit_mmap+0xe4/0x188)
[ 74.325204] [<c01e1e88>] (exit_mmap) from [<c011b3a8>] (mmput+0x40/0xf0)
[ 74.331898] [<c011b3a8>] (mmput) from [<c0208f1c>]
(flush_old_exec+0x550/0x6e0)
[ 74.339195] [<c0208f1c>] (flush_old_exec) from [<c0250e74>]
(load_elf_binary+0x2f0/0x1324)
[ 74.347447] [<c0250e74>] (load_elf_binary) from [<c02086c4>]
(search_binary_handler+0xa0/0x234)
[ 74.356134] [<c02086c4>] (search_binary_handler) from [<c02098b0>]
(__do_execve_file+0x57c/0x6bc)
[ 74.364993] [<c02098b0>] (__do_execve_file) from [<c0209c54>]
(sys_execve+0x34/0x3c)
[ 74.372724] [<c0209c54>] (sys_execve) from [<c0101000>]
(ret_fast_syscall+0x0/0x54)
[ 74.380365] Exception stack(0xede71fa8 to 0xede71ff0)
[ 74.385407] 1fa0: 000c530c 000c52d0 000c530c
000c52d0 000c52dc 00000000
[ 74.393571] 1fc0: 000c530c 000c52d0 000a2a0c 0000000b 000c52dc
000c4b5c 000c4b5c 000c530c
[ 74.401733] 1fe0: b6f25ed4 beef38b8 00042038 b6f25ee0
[ 74.406778] Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f)
[ 74.412895] ---[ end trace aad10e0b4fcbf193 ]---

I think I will drop this patch until this issue is fixed.

Philipp

2018-08-30 20:48:50

by Philipp Rossak

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor



On 30.08.2018 22:00, Philipp Rossak wrote:
> On 30.08.2018 18:27, Ondřej Jirman wrote:
>>> +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info)
>>> +{
>>> +//    regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
>>> +//            info->calibration_data[0]);
>>> +//    regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
>>> +//            info->calibration_data[1]);
>> This should probably be implemented, or left out completely.
>>
>> regards,
>>    o.
>>
> Thanks you are right!
> This should be implemented! I will fix this in the next version!
>
> Thanks,
> Philipp

I just realized this function need to check if calibration datas are
available. Writing zeros to the calibration data regs "breaks" the
thermal sensor.

Philipp

2018-08-31 08:48:54

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 07/30] iio: adc: remove mfd_probe & sunwi_irq_init function

On Thu, Aug 30, 2018 at 05:44:55PM +0200, Philipp Rossak wrote:
> In the previous commit we removed the function call, now we remove the
> unused functions.
>
> Signed-off-by: Philipp Rossak <[email protected]>

This will create a warning between the two commits, it should be
merged in the previous patch.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (417.00 B)
signature.asc (849.00 B)
Download all attachments

2018-08-31 08:53:52

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 10/30] iio: adc: rework irq and adc_channel handling

Hi,

On Thu, Aug 30, 2018 at 05:44:58PM +0200, Philipp Rossak wrote:
> We rework the irq handling and the adc_channel handling.
> This is requiered since we merge the mfd driver into the adc driver.
>
> Signed-off-by: Philipp Rossak <[email protected]>

This patch is actually the opposite of the previous ones, it has too
many things in it, and sohuld be split :)

> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 157 ++++++++++++++++++++++++--------------
> include/linux/mfd/sun4i-gpadc.h | 7 --
> 2 files changed, 98 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 658a7e3e3370..a2027614ee0c 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -49,6 +49,8 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
> return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
> }
>
> +struct sun4i_gpadc_iio;
> +
> struct gpadc_data {
> int temp_offset;
> int temp_scale;
> @@ -56,8 +58,15 @@ struct gpadc_data {
> unsigned int tp_adc_select;
> unsigned int (*adc_chan_select)(unsigned int chan);
> unsigned int adc_chan_mask;
> + bool adc_channel;
> + irqreturn_t (*ths_irq_thread)(int irq, void *dev_id);
> + int (*ths_suspend)(struct sun4i_gpadc_iio *info);
> + int (*ths_resume)(struct sun4i_gpadc_iio *info);
> + bool support_irq;
> };
>
> +static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> +
> static const struct gpadc_data sun4i_gpadc_data = {
> .temp_offset = -1932,
> .temp_scale = 133,
> @@ -65,6 +74,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,
> + .adc_channel = true,
> + .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> + .support_irq = true,
> };
>
> static const struct gpadc_data sun5i_gpadc_data = {
> @@ -74,6 +86,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,
> + .adc_channel = true,
> + .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> + .support_irq = true,
> };
>
> static const struct gpadc_data sun6i_gpadc_data = {
> @@ -83,6 +98,9 @@ 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,
> + .adc_channel = true,
> + .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> + .support_irq = true,

So this seems to enable the support for the interrupts without going
through the MFD regmap that was removed in the previous patches.

> };
>
> static const struct gpadc_data sun8i_a33_gpadc_data = {
> @@ -96,13 +114,10 @@ struct sun4i_gpadc_iio {
> struct completion completion;
> int temp_data;
> u32 adc_data;
> + unsigned int irq_data_type;
> struct regmap *regmap;
> - unsigned int fifo_data_irq;
> - atomic_t ignore_fifo_data_irq;
> - unsigned int temp_data_irq;
> - atomic_t ignore_temp_data_irq;
> + unsigned int irq;
> const struct gpadc_data *data;
> - bool no_irq;

But at the same time, you remove some fields of that structure, and
rename some other without any explanation.

> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> struct thermal_zone_device *tzd;
> @@ -130,6 +145,20 @@ static const struct regmap_config sun4i_gpadc_regmap_config = {
> .fast_io = true,
> };
>
> +static int sun4i_gpadc_irq_init(struct sun4i_gpadc_iio *info)
> +{
> + u32 reg;
> +
> + if (info->irq_data_type == SUN4I_GPADC_IRQ_FIFO_DATA)
> + reg = SUN4I_GPADC_INT_FIFOC_TEMP_IRQ_EN;
> + else
> + reg = SUN4I_GPADC_INT_FIFOC_TEMP_IRQ_EN;
> +
> + regmap_write(info->regmap, SUN4I_GPADC_INT_FIFOC, reg);
> +
> + return 0;
> +}
> +
> static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
> unsigned int irq)
> {
> @@ -151,7 +180,7 @@ static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
> if (ret)
> return ret;
>
> - if (irq == info->fifo_data_irq) {
> + if (irq == SUN4I_GPADC_IRQ_FIFO_DATA) {
> ret = regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
> info->data->tp_mode_en |
> info->data->tp_adc_select |
> @@ -172,6 +201,8 @@ static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
> ret = regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
> info->data->tp_mode_en);
> }
> + if (info->data->support_irq)
> + sun4i_gpadc_irq_init(info);
>
> if (ret)
> return ret;
> @@ -194,11 +225,12 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,
>
> mutex_lock(&info->mutex);
>
> + info->irq_data_type = irq;

This would need to be explained too

> ret = sun4i_prepare_for_irq(indio_dev, channel, irq);
> if (ret)
> goto err;
>
> - enable_irq(irq);
> + enable_irq(info->irq);
>
> /*
> * The temperature sensor throws an interruption periodically (currently
> @@ -212,7 +244,7 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,
> goto err;
> }
>
> - if (irq == info->fifo_data_irq)
> + if (irq == SUN4I_GPADC_IRQ_FIFO_DATA)
> *val = info->adc_data;
> else
> *val = info->temp_data;
> @@ -222,7 +254,7 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,
>
> err:
> pm_runtime_put_autosuspend(indio_dev->dev.parent);
> - disable_irq(irq);
> + disable_irq(info->irq);
> mutex_unlock(&info->mutex);
>
> return ret;
> @@ -231,16 +263,15 @@ static int sun4i_gpadc_read(struct iio_dev *indio_dev, int channel, int *val,
> static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
> int *val)
> {
> - struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
> -
> - return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
> + return sun4i_gpadc_read(indio_dev, channel, val,
> + SUN4I_GPADC_IRQ_FIFO_DATA);
> }
>
> static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> {
> struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> - if (info->no_irq) {
> + if (!info->data->support_irq) {
> pm_runtime_get_sync(indio_dev->dev.parent);
>
> regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> @@ -251,7 +282,7 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> return 0;
> }
>
> - return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
> + return sun4i_gpadc_read(indio_dev, 0, val, SUN4I_GPADC_IRQ_TEMP_DATA);
> }
>
> static int sun4i_gpadc_temp_offset(struct iio_dev *indio_dev, int *val)
> @@ -320,31 +351,21 @@ static const struct iio_info sun4i_gpadc_iio_info = {
> .read_raw = sun4i_gpadc_read_raw,
> };
>
> -static irqreturn_t sun4i_gpadc_temp_data_irq_handler(int irq, void *dev_id)
> +static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
> {
> struct sun4i_gpadc_iio *info = dev_id;
>
> - if (atomic_read(&info->ignore_temp_data_irq))
> - goto out;
> -
> - if (!regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, &info->temp_data))
> - complete(&info->completion);
> -
> -out:
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
> -{
> - struct sun4i_gpadc_iio *info = dev_id;
> -
> - if (atomic_read(&info->ignore_fifo_data_irq))
> - goto out;
> -
> - if (!regmap_read(info->regmap, SUN4I_GPADC_DATA, &info->adc_data))
> - complete(&info->completion);
> -
> -out:
> + if (info->irq_data_type == SUN4I_GPADC_IRQ_FIFO_DATA) {
> + /* read fifo data */
> + if (!regmap_read(info->regmap, SUN4I_GPADC_DATA,
> + &info->adc_data))
> + complete(&info->completion);
> + } else {
> + /* read temp data */
> + if (!regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA,
> + &info->temp_data))
> + complete(&info->completion);
> + }
> return IRQ_HANDLED;
> }
>
> @@ -356,6 +377,8 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
> regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
> /* Disable temperature sensor on IP */
> regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
> + /* Disable irq*/
> + regmap_write(info->regmap, SUN4I_GPADC_INT_FIFOC, 0);
>
> return 0;
> }
> @@ -378,6 +401,7 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
> SUN4I_GPADC_TPR_TEMP_ENABLE |
> SUN4I_GPADC_TPR_TEMP_PERIOD(800));
>
> +
> return 0;
> }
>
> @@ -426,8 +450,6 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> if (!info->data)
> return -ENODEV;
>
> - info->no_irq = true;
> -
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> base = devm_ioremap_resource(&pdev->dev, mem);
> if (IS_ERR(base))
> @@ -441,8 +463,25 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> return ret;
> }
>
> - if (IS_ENABLED(CONFIG_THERMAL_OF))
> - info->sensor_device = &pdev->dev;

You're also dropping the THERMAL_OF conditions

> + if (info->data->support_irq) {
> +
> + /* ths interrupt */
> + info->irq = platform_get_irq(pdev, 0);
> +
> + ret = devm_request_threaded_irq(&pdev->dev, info->irq,
> + NULL, info->data->ths_irq_thread,
> + IRQF_ONESHOT, dev_name(&pdev->dev), info);

Why do you need a threaded interrupt?

> + if (info->data->adc_channel)
> + disable_irq(info->irq);

Does it still needs to be disabled? Why?

> + if (ret) {
> + dev_err(&pdev->dev, "failed to add ths irq: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + info->sensor_device = &pdev->dev;
>
> return 0;
> }
> @@ -469,6 +508,11 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> indio_dev->info = &sun4i_gpadc_iio_info;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> + if (&info->data->adc_channel) {
> + indio_dev->num_channels = ARRAY_SIZE(sun4i_gpadc_channels);
> + indio_dev->channels = sun4i_gpadc_channels;
> + }
> +
> ret = sun4i_gpadc_probe_dt(pdev, indio_dev);
>
> if (ret)
> @@ -480,20 +524,18 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> pm_runtime_set_suspended(&pdev->dev);
> 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);
> - /*
> - * Do not fail driver probing when failing to register in
> - * thermal because no thermal DT node is found.
> - */
> - if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
> - dev_err(&pdev->dev,
> - "could not register thermal sensor: %ld\n",
> - PTR_ERR(info->tzd));
> - return PTR_ERR(info->tzd);
> - }
> + info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
> + 0, info,
> + &sun4i_ts_tz_ops);
> + /*
> + * Do not fail driver probing when failing to register in
> + * thermal because no thermal DT node is found.
> + */
> + if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
> + dev_err(&pdev->dev,
> + "could not register thermal sensor: %ld\n",
> + PTR_ERR(info->tzd));
> + return PTR_ERR(info->tzd);
> }
>
> ret = devm_iio_device_register(&pdev->dev, indio_dev);
> @@ -505,7 +547,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> return 0;
>
> err_map:
> - if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> + if (!info->data->support_irq)
> iio_map_array_unregister(indio_dev);
>
> pm_runtime_put(&pdev->dev);
> @@ -522,12 +564,9 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> - if (!IS_ENABLED(CONFIG_THERMAL_OF))
> - return 0;
> -
> thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);
>
> - if (!info->no_irq)
> + if (!info->data->support_irq)
> iio_map_array_unregister(indio_dev);
>
> return 0;
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 54c7c9375c1b..ca59336f246b 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -99,11 +99,4 @@
> .datasheet_name = _name, \
> }
>
> -struct sun4i_gpadc_dev {
> - struct device *dev;
> - struct regmap *regmap;
> - struct regmap_irq_chip_data *regmap_irqc;
> - void __iomem *base;
> -};

And you're dropping that structure.

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (12.64 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 08:53:56

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 11/30] iio: adc: add new compatibles

On Thu, Aug 30, 2018 at 05:44:59PM +0200, Philipp Rossak wrote:
> We are now adding the new compatibles.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index a2027614ee0c..79b8efdab803 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -435,6 +435,18 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
> .compatible = "allwinner,sun8i-a33-ths",
> .data = &sun8i_a33_gpadc_data,
> },
> + {
> + .compatible = "allwinner,sun4i-a10-gpadc",
> + .data = &sun4i_gpadc_data
> + },
> + {
> + .compatible = "allwinner,sun5i-a13-gpadc",
> + .data = &sun5i_gpadc_data
> + },
> + {
> + .compatible = "allwinner,sun6i-a31-gpadc",
> + .data = &sun6i_gpadc_data
> + },

Usually the bindings come before the code that use them, and the
commit log is also lacking to explain how we should use them, that
they won't be used at the moment, why, etc.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.20 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 08:54:06

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 14/30] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

On Thu, Aug 30, 2018 at 05:45:02PM +0200, Philipp Rossak wrote:
> Allwinner H3 features a thermal sensor like the one in A33, but has its
> register re-arranged, the clock divider moved to CCU (originally the
> clock divider is in ADC) and added a pair of bus clock and reset.
>
> Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
> the bus clock and the reset was removed from the CCU. The THS in A83T
> has a clock that is directly connected and runs with 24 MHz.
>
> Update the binding document to cover H3 and A83T.
>
> Signed-off-by: Philipp Rossak <[email protected]>

You probably want to have a look at:
https://www.spinics.net/lists/arm-kernel/msg670167.html

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (812.00 B)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:04:35

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 17/30] iio: adc: sun4i-gpadc-iio: rework: support clocks and reset

On Thu, Aug 30, 2018 at 05:45:05PM +0200, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
>
> The SoCs after H3 has newer thermal sensor ADCs, which have two clock
> inputs (bus clock and sampling clock) and a reset. The registers are
> also re-arranged.
>
> This commit reworks the code, adds the process of the clocks and resets.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 72 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index c278e165e161..c12de48c4e86 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>
>
> @@ -63,6 +65,9 @@ struct gpadc_data {
> int (*ths_suspend)(struct sun4i_gpadc_iio *info);
> int (*ths_resume)(struct sun4i_gpadc_iio *info);
> bool support_irq;
> + bool has_bus_clk;
> + bool has_bus_rst;
> + bool has_mod_clk;
> u32 temp_data_base;
> };
>
> @@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
> struct mutex mutex;
> struct thermal_zone_device *tzd;
> struct device *sensor_device;
> + struct clk *bus_clk;
> + struct clk *mod_clk;
> + struct reset_control *reset;
> };
>
> static const struct iio_chan_spec sun4i_gpadc_channels[] = {
> @@ -472,8 +480,13 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> - info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> - &sun4i_gpadc_regmap_config);
> + if (info->data->has_bus_clk)
> + info->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "bus",
> + base, &sun4i_gpadc_regmap_config);
> + else
> + info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun4i_gpadc_regmap_config);
> +
> if (IS_ERR(info->regmap)) {
> ret = PTR_ERR(info->regmap);
> dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
> @@ -498,9 +511,58 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> }
> }
>
> + 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;

That should be done in the runtime_resume hook

> + }
> +
> + 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 4MHz */
> + ret = clk_set_rate(info->mod_clk, 4000000);
> + if (ret)
> + goto disable_bus_clk;

Why?

> + ret = clk_prepare_enable(info->mod_clk);
> + if (ret)
> + goto disable_bus_clk;
> + }
> +
> info->sensor_device = &pdev->dev;
>
> return 0;
> +
> +disable_bus_clk:
> + clk_disable_unprepare(info->bus_clk);
> +
> +assert_reset:
> + reset_control_assert(info->reset);

You should check for the variables here before calling those
functions, if you end up here with your variable not set, you'll have
improper refcounting.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (4.00 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:07:45

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 18/30] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

On Thu, Aug 30, 2018 at 05:45:06PM +0200, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
>
> This patch reworks the driver to be able to handle more than one
> thermal sensor. Newer SoC like the A80 have 4 thermal sensors.
> Because of this the maximal sensor count value was set to 4.
>
> The sensor_id value is set during sensor registration and is for each
> registered sensor indiviual. This makes it able to differntiate the
> sensors when the value is read from the register.
>
> In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor
> was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects
> in the temp_read function automatically sensor 0. A check for the
> sensor_id is here not required since the old sensors only have one
> thermal sensor. In addition to that is the sun4i_gpadc_read_raw()
> function only used by the "older" sensors (before A33) where the
> thermal sensor was a cobination of an adc and a thermal sensor.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 63 +++++++++++++++++++++++++------------
> include/linux/iio/adc/sun4i-gpadc.h | 3 ++
> 2 files changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index c12de48c4e86..18ab72e52d78 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -69,6 +69,7 @@ struct gpadc_data {
> bool has_bus_rst;
> bool has_mod_clk;
> u32 temp_data_base;
> + int sensor_count;
> };
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> @@ -84,6 +85,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun5i_gpadc_data = {
> @@ -97,6 +99,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun6i_gpadc_data = {
> @@ -110,6 +113,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun8i_a33_gpadc_data = {
> @@ -117,6 +121,13 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .temp_scale = 162,
> .tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> +};
> +
> +struct sun4i_sensor_tzd {
> + struct sun4i_gpadc_iio *info;
> + struct thermal_zone_device *tzd;
> + unsigned int sensor_id;
> };
>
> struct sun4i_gpadc_iio {
> @@ -130,7 +141,7 @@ struct sun4i_gpadc_iio {
> const struct gpadc_data *data;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> - struct thermal_zone_device *tzd;
> + struct sun4i_sensor_tzd tzds[MAX_SENSOR_COUNT];
> struct device *sensor_device;
> struct clk *bus_clk;
> struct clk *mod_clk;
> @@ -280,7 +291,8 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
> SUN4I_GPADC_IRQ_FIFO_DATA);
> }
>
> -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);
>
> @@ -290,7 +302,8 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
>
> pm_runtime_get_sync(indio_dev->dev.parent);
>
> - regmap_read(info->regmap, info->data->temp_data_base, val);
> + regmap_read(info->regmap, info->data->temp_data_base + 0x4 * sensor,
> + val);
>
> pm_runtime_mark_last_busy(indio_dev->dev.parent);
> pm_runtime_put_autosuspend(indio_dev->dev.parent);
> @@ -334,7 +347,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;
> @@ -420,10 +433,11 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
>
> static int sun4i_gpadc_get_temp(void *data, int *temp)
> {
> - struct sun4i_gpadc_iio *info = data;
> + struct sun4i_sensor_tzd *tzd = data;
> + struct sun4i_gpadc_iio *info = tzd->info;
> int val, scale, offset;
>
> - if (sun4i_gpadc_temp_read(info->indio_dev, &val))
> + if (sun4i_gpadc_temp_read(info->indio_dev, &val, tzd->sensor_id))
> return -ETIMEDOUT;
>
> sun4i_gpadc_temp_scale(info->indio_dev, &scale);
> @@ -569,7 +583,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)
> @@ -603,18 +617,24 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> pm_runtime_set_suspended(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> - info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
> - 0, info,
> - &sun4i_ts_tz_ops);
> - /*
> - * Do not fail driver probing when failing to register in
> - * thermal because no thermal DT node is found.
> - */
> - if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
> - dev_err(&pdev->dev,
> - "could not register thermal sensor: %ld\n",
> - PTR_ERR(info->tzd));
> - return PTR_ERR(info->tzd);
> + for (i = 0; i < info->data->sensor_count; i++) {
> + info->tzds[i].info = info;
> + info->tzds[i].sensor_id = i;
> +
> + info->tzds[i].tzd = thermal_zone_of_sensor_register(

This isn't where should wrap your line.

> + info->sensor_device,
> + i, &info->tzds[i], &sun4i_ts_tz_ops);
> + /*
> + * Do not fail driver probing when failing to register in
> + * thermal because no thermal DT node is found.
> + */
> + if (IS_ERR(info->tzds[i].tzd) && \

And you don't need the \ here.

> + PTR_ERR(info->tzds[i].tzd) != -ENODEV) {
> + dev_err(&pdev->dev,
> + "could not register thermal sensor: %ld\n",
> + PTR_ERR(info->tzds[i].tzd));
> + return PTR_ERR(info->tzds[i].tzd);
> + }

If you're failing half way through the registration, you won't free
the first sensors registered.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (6.70 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:09:27

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 19/30] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

On Thu, Aug 30, 2018 at 05:45:07PM +0200, Philipp Rossak wrote:
> This patch reworks the driver to support nvmem calibration cells.
> The driver checks if the nvmem calibration is supported and reads out
> the nvmem.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 18ab72e52d78..2fd73d143815 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>
> @@ -70,6 +71,7 @@ struct gpadc_data {
> bool has_mod_clk;
> u32 temp_data_base;
> int sensor_count;
> + bool supports_nvmem;
> };
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> @@ -146,6 +148,7 @@ struct sun4i_gpadc_iio {
> struct clk *bus_clk;
> struct clk *mod_clk;
> struct reset_control *reset;
> + u32 calibration_data[2];
> };
>
> static const struct iio_chan_spec sun4i_gpadc_channels[] = {
> @@ -484,6 +487,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;
> + u32 *cell_data;
>
> info->data = of_device_get_match_data(&pdev->dev);
> if (!info->data)
> @@ -494,6 +500,24 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + if (info->data->supports_nvmem) {
> +
> + cell = nvmem_cell_get(&pdev->dev, "calibration");
> + if (IS_ERR(cell)) {
> + if (PTR_ERR(cell) == -EPROBE_DEFER)
> + return PTR_ERR(cell);

You're masking real errors here, if the issue isn't that the property
isn't found.

> + } else {
> + cell_data = (u32 *)nvmem_cell_read(cell, &cell_size);
> + if (cell_size != 8)
> + dev_err(&pdev->dev,
> + "Calibration data has wrong size\n");
> + else {
> + info->calibration_data[0] = cell_data[0];
> + info->calibration_data[1] = cell_data[1];

How are those calibration data organized when there is multiple channels?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (2.48 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:10:39

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 20/30] iio: adc: sun4i-gpadc-iio: rework: device specific suspend & resume

On Thu, Aug 30, 2018 at 05:45:08PM +0200, Philipp Rossak wrote:
> Different sensors will have different suspend and resume functions. So
> we are modularize the suspend and resume functions.
>
> The resume function configures and initializes the thermal sensor and
> the suspend function disables the sensors.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 2fd73d143815..c7b46c82e3e5 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -76,6 +76,9 @@ struct gpadc_data {
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
>
> +static int sun4i_ths_resume(struct sun4i_gpadc_iio *info);
> +static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info);
> +
> static const struct gpadc_data sun4i_gpadc_data = {
> .temp_offset = -1932,
> .temp_scale = 133,
> @@ -87,6 +90,8 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .ths_resume = sun4i_ths_resume,
> + .ths_suspend = sun4i_ths_suspend,
> .sensor_count = 1,
> };
>
> @@ -101,6 +106,8 @@ static const struct gpadc_data sun5i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .ths_resume = sun4i_ths_resume,
> + .ths_suspend = sun4i_ths_suspend,
> .sensor_count = 1,
> };
>
> @@ -115,6 +122,8 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .ths_resume = sun4i_ths_resume,
> + .ths_suspend = sun4i_ths_suspend,
> .sensor_count = 1,
> };
>
> @@ -123,6 +132,8 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .temp_scale = 162,
> .tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .ths_resume = sun4i_ths_resume,
> + .ths_suspend = sun4i_ths_suspend,
> .sensor_count = 1,
> };
>
> @@ -401,6 +412,11 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
> static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> + return info->data->ths_suspend(info);
> +}
> +
> +static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)

suspend is already a hook in the kernel, which hasn't the same meaning
than runtime_suspend (and the same applies to resume), so we'd rather
pick a better name. And all the functions (and the driver) use gpadc,
please continue to use that prefix.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (2.95 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:13:55

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

On Thu, Aug 30, 2018 at 05:45:09PM +0200, Philipp Rossak wrote:
> This patch adds support for the H3 ths sensor.
>
> The H3 supports interrupts. The interrupt is configured to update the
> the sensor values every second. The calibration data is writen at the
> begin of the init process.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 91 +++++++++++++++++++++++++++++++++++++
> include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++
> 2 files changed, 109 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index c7b46c82e3e5..d5c7971b2558 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -72,6 +72,7 @@ struct gpadc_data {
> u32 temp_data_base;
> int sensor_count;
> bool supports_nvmem;
> + u32 ths_irq_clear;
> };
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> @@ -79,6 +80,10 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> static int sun4i_ths_resume(struct sun4i_gpadc_iio *info);
> static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info);
>
> +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info);
> +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info);
> +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data);
> +
> static const struct gpadc_data sun4i_gpadc_data = {
> .temp_offset = -1932,
> .temp_scale = 133,
> @@ -137,6 +142,22 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .sensor_count = 1,
> };
>
> +static const struct gpadc_data sun8i_h3_ths_data = {
> + .temp_offset = -1791,
> + .temp_scale = -121,
> + .temp_data_base = SUN8I_H3_THS_TDATA0,
> + .ths_irq_thread = sunx8i_h3_irq_thread,
> + .support_irq = true,
> + .has_bus_clk = true,
> + .has_bus_rst = true,
> + .has_mod_clk = true,
> + .sensor_count = 1,
> + .supports_nvmem = true,
> + .ths_resume = sun8i_h3_ths_resume,
> + .ths_suspend = sun8i_h3_ths_suspend,
> + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0,
> +};
> +
> struct sun4i_sensor_tzd {
> struct sun4i_gpadc_iio *info;
> struct thermal_zone_device *tzd;
> @@ -409,6 +430,31 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data)
> +{
> + struct sun4i_gpadc_iio *info = data;
> + int i;
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> + info->data->ths_irq_clear);
> +
> + for (i = 0; i < info->data->sensor_count; i++)
> + thermal_zone_device_update(info->tzds[i].tzd,
> + THERMAL_EVENT_TEMP_SAMPLE);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info)
> +{
> +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> +// info->calibration_data[0]);
> +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> +// info->calibration_data[1]);

Don't put commented code.

> +
> + return 0;
> +}
> +
> static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sun8i_h3_ths_suspend(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_resume(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info)
> +{
> + u32 value;
> +
> + sun8i_h3_calibrate(info);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
> + SUN4I_GPADC_CTRL0_T_ACQ(0xff));
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> + SUN8I_H3_THS_ACQ1(0x3f));
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> + SUN8I_H3_THS_INTS_TDATA_IRQ_0);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
> + SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2));
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_INTC,
> + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> + SUN8I_H3_THS_TEMP_PERIOD(0x55));
> +
> + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> + SUN8I_H3_THS_TEMP_SENSE_EN0 | value);

Ideally, all these values should have a comment explaining what they
are.

And we really start to have a lot of registers defines. We'd be better
off using regmap_fields.

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (4.87 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:15:35

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 25/30] ARM: dts: sun8i: h3: add thermal zone to H3

On Thu, Aug 30, 2018 at 05:45:13PM +0200, Philipp Rossak wrote:
> 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 <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-h3.dtsi | 31 +++++++++++++++++++++++++++++++
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> 2 files changed, 32 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 5b7994cb1471..954848d5df50 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -78,6 +78,8 @@
> clock-names = "cpu";
> operating-points-v2 = <&cpu0_opp_table>;
> #cooling-cells = <2>;
> + cooling-min-level = <0>;
> + cooling-max-level = <15>;
> };
>
> cpu@1 {
> @@ -102,6 +104,35 @@
> };
> };
>
> + thermal-zones {
> + cpu-thermal {
> + /* milliseconds */
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> + thermal-sensors = <&ths>;
> +
> + trips {
> + cpu_hot_trip: cpu-warm {
> + temperature = <65000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cpu_very_hot_trip: cpu-very-hot {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };

Where are those trip points coming from?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.46 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:26:09

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 01/30] mfd: Makefile: Remove build option for MFD:sun4i-gpadc

Hi Philipp,

First, thanks for doing that rework. It was needed, and it's very much
appreciated :)

As you can imagine though, I have a bunch of comments.

On Thu, Aug 30, 2018 at 05:44:49PM +0200, Philipp Rossak wrote:
> Since we are merging the mfd driver into the sun4i-gpadc driver we need
> to remove the build options for the sun4i-gpadc driver.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/mfd/Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e9fd20dba18d..c680994db988 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -220,7 +220,6 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
> obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
>
> obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
> -obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o

One of the things we should strive for is bisectability, which means
being able to have a working driver at every point in time while
introducing the features.

In this particular case, this isn't really a problem since you're
removing part of code that were never really enabled, but you should
at least document why in your commit log.

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.33 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:28:35

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 04/30] iio: adc: Kconfig: Update Kconfig to new build options

On Thu, Aug 30, 2018 at 05:44:52PM +0200, Philipp Rossak wrote:
> Since we are merging the mfd driver into the iio adc driver we need to
> update the Kconfig build options.

Most of your commit logs a pretty short, and this is an issue for
people lacking context. This would be the reviewers, but also users
bisecting to see where there's a bug, or you in a couple of years from
now :)

Every commit log should be as complete as possible. In this case, you
are saying what you are doing, but this is actually redundant with the
patch, I can already tell what you are doing by reading the rest of
the mail. However, what is really important is *why* you are doing it.

> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/Kconfig | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9da79070357c..5d0cffd6d2e4 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -713,13 +713,16 @@ config STX104
> array module parameter.
>
> config SUN4I_GPADC
> - tristate "Support for the Allwinner SoCs GPADC"
> + tristate "Allwinner sunxi platforms' GPADC/Thermal driver"

I'm really not sure this is worth updating

> + select REGMAP_MMIO
> + select REGMAP_IRQ
> depends on IIO
> - depends on MFD_SUN4I_GPADC || MACH_SUN8I
> - depends on THERMAL || !THERMAL_OF
> + depends on ARCH_SUNXI || MACH_SUN8I
> + depends on THERMAL && THERMAL_OF

For example, why you are changing the thermal dependency is not really
obvious to anyone.

> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> - GPADC. This ADC provides 4 channels which can be used as an ADC or as
> + GPADC or newer SOCs (A33, H3, A83T, ...) Thermal sensor driver.
> + This ADC provides 4 channels which can be used as an ADC or as

I'm not sure this is worth updating either, especially when there's
information that has been dropped.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (2.07 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:29:16

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 08/30] iio: adc: remove hwmon structure

On Thu, Aug 30, 2018 at 05:44:56PM +0200, Philipp Rossak wrote:
> We remove the hwmon structure that was requiered for the mfd driver.
>
> Signed-off-by: Philipp Rossak <[email protected]>

Same thing here, you should merge that in the commit that removes the
probe_mfd call.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (385.00 B)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:29:37

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 09/30] iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i channel

On Thu, Aug 30, 2018 at 05:44:57PM +0200, Philipp Rossak wrote:
> We want to use this driver mostly as thermal sensor, that still supports
> the adc for the older chips, thus we threat the A33 as thermal sensor.
> We also remove the adc channel without thermal support.

The A33 can definitely be used as an ADC, so we need to keep that support.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (453.00 B)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:52:58

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 16/30] iio: adc: sun4i-gpadc-iio: rework: readout temp_data

On Thu, Aug 30, 2018 at 05:45:04PM +0200, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
>
> This commit reworks the code and uses regmap field to read out
> temp_data.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index d48f338af563..c278e165e161 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -63,6 +63,7 @@ struct gpadc_data {
> int (*ths_suspend)(struct sun4i_gpadc_iio *info);
> int (*ths_resume)(struct sun4i_gpadc_iio *info);
> bool support_irq;
> + u32 temp_data_base;
> };
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> @@ -77,6 +78,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .adc_channel = true,
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> + .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> };
>
> static const struct gpadc_data sun5i_gpadc_data = {
> @@ -89,6 +91,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
> .adc_channel = true,
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> + .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> };
>
> static const struct gpadc_data sun6i_gpadc_data = {
> @@ -101,12 +104,14 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .adc_channel = true,
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> + .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> };
>
> 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_base = SUN4I_GPADC_TEMP_DATA,
> };
>
> struct sun4i_gpadc_iio {
> @@ -271,18 +276,18 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> {
> struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> - if (!info->data->support_irq) {
> - pm_runtime_get_sync(indio_dev->dev.parent);
> + if (info->data->adc_channel)
> + return sun4i_gpadc_read(indio_dev, 0, val,
> + SUN4I_GPADC_IRQ_TEMP_DATA);
>
> - regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> + pm_runtime_get_sync(indio_dev->dev.parent);
>
> - pm_runtime_mark_last_busy(indio_dev->dev.parent);
> - pm_runtime_put_autosuspend(indio_dev->dev.parent);
> + regmap_read(info->regmap, info->data->temp_data_base, val);
>
> - return 0;
> - }
> + pm_runtime_mark_last_busy(indio_dev->dev.parent);
> + pm_runtime_put_autosuspend(indio_dev->dev.parent);
>
> - return sun4i_gpadc_read(indio_dev, 0, val, SUN4I_GPADC_IRQ_TEMP_DATA);
> + return 0;

You should make this patch as small as possible. If you want to add a
variable and to invert a condition, please make two patches.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (3.05 kB)
signature.asc (849.00 B)
Download all attachments

2018-08-31 09:56:19

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

在 2018-08-31五的 11:11 +0200,Maxime Ripard写道:
> On Thu, Aug 30, 2018 at 05:45:09PM +0200, Philipp Rossak wrote:
> > This patch adds support for the H3 ths sensor.
> >
> > The H3 supports interrupts. The interrupt is configured to update
> > the
> > the sensor values every second. The calibration data is writen at
> > the
> > begin of the init process.
> >
> > Signed-off-by: Philipp Rossak <[email protected]>
> > ---
> > drivers/iio/adc/sun4i-gpadc-iio.c | 91
> > +++++++++++++++++++++++++++++++++++++
> > include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++
> > 2 files changed, 109 insertions(+)
> >
> > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c
> > b/drivers/iio/adc/sun4i-gpadc-iio.c
> > index c7b46c82e3e5..d5c7971b2558 100644
> > --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> > @@ -72,6 +72,7 @@ struct gpadc_data {
> > u32 temp_data_base;
> > int sensor_count;
> > bool supports_nvmem;
> > + u32 ths_irq_clear;
> > };
> >
> > static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void
> > *dev_id);
> > @@ -79,6 +80,10 @@ static irqreturn_t
> > sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> > static int sun4i_ths_resume(struct sun4i_gpadc_iio *info);
> > static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info);
> >
> > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info);
> > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info);
> > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data);
> > +
> > static const struct gpadc_data sun4i_gpadc_data = {
> > .temp_offset = -1932,
> > .temp_scale = 133,
> > @@ -137,6 +142,22 @@ static const struct gpadc_data
> > sun8i_a33_gpadc_data = {
> > .sensor_count = 1,
> > };
> >
> > +static const struct gpadc_data sun8i_h3_ths_data = {
> > + .temp_offset = -1791,
> > + .temp_scale = -121,
> > + .temp_data_base = SUN8I_H3_THS_TDATA0,
> > + .ths_irq_thread = sunx8i_h3_irq_thread,
> > + .support_irq = true,
> > + .has_bus_clk = true,
> > + .has_bus_rst = true,
> > + .has_mod_clk = true,
> > + .sensor_count = 1,
> > + .supports_nvmem = true,
> > + .ths_resume = sun8i_h3_ths_resume,
> > + .ths_suspend = sun8i_h3_ths_suspend,
> > + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0,
> > +};
> > +
> > struct sun4i_sensor_tzd {
> > struct sun4i_gpadc_iio *info;
> > struct thermal_zone_device *tzd;
> > @@ -409,6 +430,31 @@ static irqreturn_t
> > sun4i_gpadc_data_irq_handler(int irq, void *dev_id)
> > return IRQ_HANDLED;
> > }
> >
> > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data)
> > +{
> > + struct sun4i_gpadc_iio *info = data;
> > + int i;
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> > + info->data->ths_irq_clear);
> > +
> > + for (i = 0; i < info->data->sensor_count; i++)
> > + thermal_zone_device_update(info->tzds[i].tzd,
> > + THERMAL_EVENT_TEMP_SAMP
> > LE);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info)
> > +{
> > +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> > +// info->calibration_data[0]);
> > +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> > +// info->calibration_data[1]);
>
> Don't put commented code.

Personally I suggest to leave out all SID or calibration related
patches here.

Currently we seems to be wrongly converting SID to big endian, however,
the orgnization of the THS calibration data on H6 shows that it's
surely little endian:

It consists a temperature value in 1/10 celsuis as unit, and some
thermal register readout values, which are the values read out at the
given temperature, and every value here (the temperature and the
readout) are all half word length.

Let the temperature value be AABB, the two readout values be XXYY and
ZZWW, the oragnization is:
BB AA YY XX WW ZZ ** ** .

When converting the SID to big endian, it becomes:
XX YY AA BB ** ** ZZ WW ,
which is non-sense, and not able to do sub-word cell addressing.

Maxime, should I drop the LE2BE conversion in SID driver? (I doubt
whether it will break compatibility.)

Philipp, could you delay to send any code that uses SID?

>
> > +
> > + return 0;
> > +}
> > +
> > static int sun4i_gpadc_runtime_suspend(struct device *dev)
> > {
> > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> > @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct
> > sun4i_gpadc_iio *info)
> > return 0;
> > }
> >
> > +static int sun8i_h3_ths_suspend(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_resume(struct device *dev)
> > {
> > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> > @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct
> > sun4i_gpadc_iio *info)
> > return 0;
> > }
> >
> > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info)
> > +{
> > + u32 value;
> > +
> > + sun8i_h3_calibrate(info);
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
> > + SUN4I_GPADC_CTRL0_T_ACQ(0xff));
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> > + SUN8I_H3_THS_ACQ1(0x3f));
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> > + SUN8I_H3_THS_INTS_TDATA_IRQ_0);
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
> > + SUN4I_GPADC_CTRL3_FILTER_EN |
> > + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2));
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_INTC,
> > + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> > + SUN8I_H3_THS_TEMP_PERIOD(0x55));
> > +
> > + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
> > +
> > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> > + SUN8I_H3_THS_TEMP_SENSE_EN0 | value);
>
> Ideally, all these values should have a comment explaining what they
> are.
>
> And we really start to have a lot of registers defines. We'd be
> better
> off using regmap_fields.
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


2018-08-31 12:08:09

by Philipp Rossak

[permalink] [raw]
Subject: Re: [PATCH v3 20/30] iio: adc: sun4i-gpadc-iio: rework: device specific suspend & resume



On 31.08.2018 11:09, Maxime Ripard wrote:
>> +static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)
> suspend is already a hook in the kernel, which hasn't the same meaning
> than runtime_suspend (and the same applies to resume), so we'd rather
> pick a better name. And all the functions (and the driver) use gpadc,
> please continue to use that prefix.

I agree.
For the newer sensors (from H3) the Sensor is referenced in the
datasheets as Thermal Sensor short THS. So I would like to use for the
newer sensors that prefix. Is that ok?

Philipp

2018-08-31 12:55:51

by Philipp Rossak

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor



On 31.08.2018 11:51, Icenowy Zheng wrote:
> Personally I suggest to leave out all SID or calibration related
> patches here.
>
> Currently we seems to be wrongly converting SID to big endian, however,
> the orgnization of the THS calibration data on H6 shows that it's
> surely little endian:
>
> It consists a temperature value in 1/10 celsuis as unit, and some
> thermal register readout values, which are the values read out at the
> given temperature, and every value here (the temperature and the
> readout) are all half word length.
>
> Let the temperature value be AABB, the two readout values be XXYY and
> ZZWW, the oragnization is:
> BB AA YY XX WW ZZ ** ** .
>
> When converting the SID to big endian, it becomes:
> XX YY AA BB ** ** ZZ WW ,
> which is non-sense, and not able to do sub-word cell addressing.
>
> Maxime, should I drop the LE2BE conversion in SID driver? (I doubt
> whether it will break compatibility.)
>
> Philipp, could you delay to send any code that uses SID?

Yes for sure! I will move the related patches to the end of the series
and add a DO-NOT-MERGE flag in the title. So I can get those also
ready/reviewed but not merged.

Icenowy, do you know more about the A83T SID? From the general specs it
could be the same like on the A64 or the H3.

Thanks,
Philipp

2018-08-31 12:55:57

by Philipp Rossak

[permalink] [raw]
Subject: Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

On 31.08.2018 11:11, Maxime Ripard wrote:
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
>> + SUN4I_GPADC_CTRL0_T_ACQ(0xff));
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
>> + SUN8I_H3_THS_ACQ1(0x3f));
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
>> + SUN8I_H3_THS_INTS_TDATA_IRQ_0);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
>> + SUN4I_GPADC_CTRL3_FILTER_EN |
>> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2));
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_INTC,
>> + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
>> + SUN8I_H3_THS_TEMP_PERIOD(0x55));
>> +
>> + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
>> + SUN8I_H3_THS_TEMP_SENSE_EN0 | value);
> Ideally, all these values should have a comment explaining what they
> are.
>
> And we really start to have a lot of registers defines. We'd be better
> off using regmap_fields.

I will rework this in the next version.

Thanks,
Philipp

2018-09-02 19:59:50

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 02/30] mfd: Kconfig: Remove MFD_SUN4I_GPADC config option

On Thu, 30 Aug 2018 17:44:50 +0200
Philipp Rossak <[email protected]> wrote:

> We are merging the mfd:sun4i-gpadc driver into the
> iio/adc/sun4i-gpadc driver. So we need to remove the MFD_SUN4I_GPADC
> config option.
>
> Signed-off-by: Philipp Rossak <[email protected]>
Do need to separate this and patch 1 as they are really two
facets of the same thing.

Jonathan

> ---
> drivers/mfd/Kconfig | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b860eb5aa194..c7ab57d65610 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -40,23 +40,6 @@ config MFD_ACT8945A
> linear regulators, along with a complete ActivePath battery
> charger.
>
> -config MFD_SUN4I_GPADC
> - tristate "Allwinner sunxi platforms' GPADC MFD driver"
> - select MFD_CORE
> - select REGMAP_MMIO
> - select REGMAP_IRQ
> - depends on ARCH_SUNXI || COMPILE_TEST
> - depends on !TOUCHSCREEN_SUN4I
> - help
> - Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
> - This driver will only map the hardware interrupt and registers, you
> - have to select individual drivers based on this MFD to be able to use
> - the ADC or the thermal sensor. This will try to probe the ADC driver
> - sun4i-gpadc-iio and the hwmon driver iio_hwmon.
> -
> - To compile this driver as a module, choose M here: the module will be
> - called sun4i-gpadc.
> -
> config MFD_AS3711
> bool "AMS AS3711"
> select MFD_CORE


2018-09-02 20:00:25

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 01/30] mfd: Makefile: Remove build option for MFD:sun4i-gpadc

On Fri, 31 Aug 2018 10:25:45 +0200
Maxime Ripard <[email protected]> wrote:

> Hi Philipp,
>
> First, thanks for doing that rework. It was needed, and it's very much
> appreciated :)
>
> As you can imagine though, I have a bunch of comments.
>
> On Thu, Aug 30, 2018 at 05:44:49PM +0200, Philipp Rossak wrote:
> > Since we are merging the mfd driver into the sun4i-gpadc driver we need
> > to remove the build options for the sun4i-gpadc driver.
> >
> > Signed-off-by: Philipp Rossak <[email protected]>
> > ---
> > drivers/mfd/Makefile | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index e9fd20dba18d..c680994db988 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -220,7 +220,6 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
> > obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
> >
> > obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
> > -obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o
>
> One of the things we should strive for is bisectability, which means
> being able to have a working driver at every point in time while
> introducing the features.
>
> In this particular case, this isn't really a problem since you're
> removing part of code that were never really enabled, but you should
> at least document why in your commit log.
>
Agreed. I for one don't know / can't remember why this would make sense!

Jonathan
> Thanks!
> Maxime
>


2018-09-02 20:02:37

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 05/30] iio: adc: move SUN4I_GPADC_CHANNEL define to header file

On Thu, 30 Aug 2018 17:44:53 +0200
Philipp Rossak <[email protected]> wrote:

> We are moving the SUN4I_GPADC_CHANNEL define to the header file.
Maxime has raised this point in other patches...

Why? Obvious what but I have no idea why you are doing this.

Thanks,

Jonathan
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 9 ---------
> include/linux/mfd/sun4i-gpadc.h | 9 +++++++++
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index d95dd0fde2a6..666329940e1e 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -109,15 +109,6 @@ struct sun4i_gpadc_iio {
> struct device *sensor_device;
> };
>
> -#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
> - .type = IIO_VOLTAGE, \
> - .indexed = 1, \
> - .channel = _channel, \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> - .datasheet_name = _name, \
> -}
> -
> static struct iio_map sun4i_gpadc_hwmon_maps[] = {
> {
> .adc_channel_label = "temp_adc",
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 139872c2e0fe..54c7c9375c1b 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -90,6 +90,15 @@
> /* 10s delay before suspending the IP */
> #define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
>
> +#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .channel = _channel, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .datasheet_name = _name, \
> +}
> +
> struct sun4i_gpadc_dev {
> struct device *dev;
> struct regmap *regmap;


2018-09-02 20:15:08

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 18/30] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

On Thu, 30 Aug 2018 17:45:06 +0200
Philipp Rossak <[email protected]> wrote:

> For adding newer sensor some basic rework of the code is necessary.
>
> This patch reworks the driver to be able to handle more than one
> thermal sensor. Newer SoC like the A80 have 4 thermal sensors.
> Because of this the maximal sensor count value was set to 4.
>
> The sensor_id value is set during sensor registration and is for each
> registered sensor indiviual. This makes it able to differntiate the
> sensors when the value is read from the register.
>
> In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor
> was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects
> in the temp_read function automatically sensor 0. A check for the
> sensor_id is here not required since the old sensors only have one
> thermal sensor. In addition to that is the sun4i_gpadc_read_raw()
> function only used by the "older" sensors (before A33) where the
> thermal sensor was a cobination of an adc and a thermal sensor.
>
> Signed-off-by: Philipp Rossak <[email protected]>
One additional comment inline..

Just a suggestion to make things slightly easier to read / review.

Thanks,

Jonathan

> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 63 +++++++++++++++++++++++++------------
> include/linux/iio/adc/sun4i-gpadc.h | 3 ++
> 2 files changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index c12de48c4e86..18ab72e52d78 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -69,6 +69,7 @@ struct gpadc_data {
> bool has_bus_rst;
> bool has_mod_clk;
> u32 temp_data_base;
> + int sensor_count;
> };
>
> static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id);
> @@ -84,6 +85,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun5i_gpadc_data = {
> @@ -97,6 +99,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun6i_gpadc_data = {
> @@ -110,6 +113,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .ths_irq_thread = sun4i_gpadc_data_irq_handler,
> .support_irq = true,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun8i_a33_gpadc_data = {
> @@ -117,6 +121,13 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .temp_scale = 162,
> .tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
> .temp_data_base = SUN4I_GPADC_TEMP_DATA,
> + .sensor_count = 1,
> +};
> +
> +struct sun4i_sensor_tzd {
> + struct sun4i_gpadc_iio *info;
> + struct thermal_zone_device *tzd;
> + unsigned int sensor_id;
> };
>
> struct sun4i_gpadc_iio {
> @@ -130,7 +141,7 @@ struct sun4i_gpadc_iio {
> const struct gpadc_data *data;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> - struct thermal_zone_device *tzd;
> + struct sun4i_sensor_tzd tzds[MAX_SENSOR_COUNT];
> struct device *sensor_device;
> struct clk *bus_clk;
> struct clk *mod_clk;
> @@ -280,7 +291,8 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
> SUN4I_GPADC_IRQ_FIFO_DATA);
> }
>
> -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);
>
> @@ -290,7 +302,8 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
>
> pm_runtime_get_sync(indio_dev->dev.parent);
>
> - regmap_read(info->regmap, info->data->temp_data_base, val);
> + regmap_read(info->regmap, info->data->temp_data_base + 0x4 * sensor,
> + val);
>
> pm_runtime_mark_last_busy(indio_dev->dev.parent);
> pm_runtime_put_autosuspend(indio_dev->dev.parent);
> @@ -334,7 +347,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;
> @@ -420,10 +433,11 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
>
> static int sun4i_gpadc_get_temp(void *data, int *temp)
> {
> - struct sun4i_gpadc_iio *info = data;
> + struct sun4i_sensor_tzd *tzd = data;
> + struct sun4i_gpadc_iio *info = tzd->info;
> int val, scale, offset;
>
> - if (sun4i_gpadc_temp_read(info->indio_dev, &val))
> + if (sun4i_gpadc_temp_read(info->indio_dev, &val, tzd->sensor_id))
> return -ETIMEDOUT;
>
> sun4i_gpadc_temp_scale(info->indio_dev, &scale);
> @@ -569,7 +583,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)
> @@ -603,18 +617,24 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> pm_runtime_set_suspended(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> - info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
> - 0, info,
> - &sun4i_ts_tz_ops);
> - /*
> - * Do not fail driver probing when failing to register in
> - * thermal because no thermal DT node is found.
> - */
> - if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
> - dev_err(&pdev->dev,
> - "could not register thermal sensor: %ld\n",
> - PTR_ERR(info->tzd));
> - return PTR_ERR(info->tzd);
> + for (i = 0; i < info->data->sensor_count; i++) {

This feels like a good place to factor out the code into a utility
function that just does one of them. That should hopefully
reduce the indenting etc enough to make the code easier to read.

> + info->tzds[i].info = info;
> + info->tzds[i].sensor_id = i;
> +
> + info->tzds[i].tzd = thermal_zone_of_sensor_register(
> + info->sensor_device,
> + i, &info->tzds[i], &sun4i_ts_tz_ops);
> + /*
> + * Do not fail driver probing when failing to register in
> + * thermal because no thermal DT node is found.
> + */
> + if (IS_ERR(info->tzds[i].tzd) && \
> + PTR_ERR(info->tzds[i].tzd) != -ENODEV) {
> + dev_err(&pdev->dev,
> + "could not register thermal sensor: %ld\n",
> + PTR_ERR(info->tzds[i].tzd));
> + return PTR_ERR(info->tzds[i].tzd);
> + }
> }
>
> ret = devm_iio_device_register(&pdev->dev, indio_dev);
> @@ -639,11 +659,14 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
> + int i;
>
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> - thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);
> + for (i = 0; i < info->data->sensor_count; i++)
> + thermal_zone_of_sensor_unregister(info->sensor_device,
> + info->tzds[i].tzd);
>
> if (!info->data->support_irq)
> iio_map_array_unregister(indio_dev);
> diff --git a/include/linux/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h
> index d6850f39dcfb..99feeba28105 100644
> --- a/include/linux/iio/adc/sun4i-gpadc.h
> +++ b/include/linux/iio/adc/sun4i-gpadc.h
> @@ -99,4 +99,7 @@
> .datasheet_name = _name, \
> }
>
> +/* SUNXI_THS COMMON REGISTERS + DEFINES */
> +#define MAX_SENSOR_COUNT 4
> +
> #endif


2018-09-03 09:45:37

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 20/30] iio: adc: sun4i-gpadc-iio: rework: device specific suspend & resume

On Fri, Aug 31, 2018 at 02:05:59PM +0200, Philipp Rossak wrote:
>
>
> On 31.08.2018 11:09, Maxime Ripard wrote:
> > > +static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)
> > suspend is already a hook in the kernel, which hasn't the same meaning
> > than runtime_suspend (and the same applies to resume), so we'd rather
> > pick a better name. And all the functions (and the driver) use gpadc,
> > please continue to use that prefix.
>
> I agree.
> For the newer sensors (from H3) the Sensor is referenced in the datasheets
> as Thermal Sensor short THS. So I would like to use for the newer sensors
> that prefix. Is that ok?

Not really. The consistency within the driver is more important.

Maxiem

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (820.00 B)
signature.asc (849.00 B)
Download all attachments

2018-09-03 10:21:55

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

On Fri, Aug 31, 2018 at 05:51:41PM +0800, Icenowy Zheng wrote:
> Personally I suggest to leave out all SID or calibration related
> patches here.
>
> Currently we seems to be wrongly converting SID to big endian, however,
> the orgnization of the THS calibration data on H6 shows that it's
> surely little endian:
>
> It consists a temperature value in 1/10 celsuis as unit, and some
> thermal register readout values, which are the values read out at the
> given temperature, and every value here (the temperature and the
> readout) are all half word length.
>
> Let the temperature value be AABB, the two readout values be XXYY and
> ZZWW, the oragnization is:
> BB AA YY XX WW ZZ ** ** .
>
> When converting the SID to big endian, it becomes:
> XX YY AA BB ** ** ZZ WW ,
> which is non-sense, and not able to do sub-word cell addressing.
>
> Maxime, should I drop the LE2BE conversion in SID driver? (I doubt
> whether it will break compatibility.)

This is exposed to the userspace, so no.

Maxime
>

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.10 kB)
signature.asc (849.00 B)
Download all attachments

2018-09-03 11:03:30

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor



于 2018年9月3日 GMT+08:00 下午6:20:22, Maxime Ripard <[email protected]> 写到:
>On Fri, Aug 31, 2018 at 05:51:41PM +0800, Icenowy Zheng wrote:
>> Personally I suggest to leave out all SID or calibration related
>> patches here.
>>
>> Currently we seems to be wrongly converting SID to big endian,
>however,
>> the orgnization of the THS calibration data on H6 shows that it's
>> surely little endian:
>>
>> It consists a temperature value in 1/10 celsuis as unit, and some
>> thermal register readout values, which are the values read out at the
>> given temperature, and every value here (the temperature and the
>> readout) are all half word length.
>>
>> Let the temperature value be AABB, the two readout values be XXYY and
>> ZZWW, the oragnization is:
>> BB AA YY XX WW ZZ ** ** .
>>
>> When converting the SID to big endian, it becomes:
>> XX YY AA BB ** ** ZZ WW ,
>> which is non-sense, and not able to do sub-word cell addressing.
>>
>> Maxime, should I drop the LE2BE conversion in SID driver? (I doubt
>> whether it will break compatibility.)
>
>This is exposed to the userspace, so no.

Please note the LE2BE totally breaks the SID addressing.

Without it dropped all cells must be referenced with
4 byte word as unit, and half word addressing of
SID is thus not possible. The driver will also need then to split
the half words if needed.

I think this is kind of hardware misbehavior, and not a simple
UAPI change, so the UAPI stability shouldn't affect this change.

>
>Maxime
>>

2018-09-03 14:00:25

by Philipp Rossak

[permalink] [raw]
Subject: Re: [PATCH v3 18/30] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

On 02.09.2018 22:11, Jonathan Cameron wrote:
> This feels like a good place to factor out the code into a utility
> function that just does one of them. That should hopefully
> reduce the indenting etc enough to make the code easier to read.
>
>> + info->tzds[i].info = info;
>> + info->tzds[i].sensor_id = i;
>> +
>> + info->tzds[i].tzd = thermal_zone_of_sensor_register(
>> + info->sensor_device,
>> + i, &info->tzds[i], &sun4i_ts_tz_ops);
>> + /*
>> + * Do not fail driver probing when failing to register in
>> + * thermal because no thermal DT node is found.
>> + */
>> + if (IS_ERR(info->tzds[i].tzd) && \
>> + PTR_ERR(info->tzds[i].tzd) != -ENODEV) {
>> + dev_err(&pdev->dev,
>> + "could not register thermal sensor: %ld\n",
>> + PTR_ERR(info->tzds[i].tzd));
>> + return PTR_ERR(info->tzds[i].tzd);
>> + }

So this code above should be placed in a separate function and called by
the for loop?
Did I understand that right?

Philipp

2018-09-03 14:26:01

by Philipp Rossak

[permalink] [raw]
Subject: Re: [PATCH v3 05/30] iio: adc: move SUN4I_GPADC_CHANNEL define to header file

On 02.09.2018 22:01, Jonathan Cameron wrote:
> On Thu, 30 Aug 2018 17:44:53 +0200
> Philipp Rossak <[email protected]> wrote:
>
>> We are moving the SUN4I_GPADC_CHANNEL define to the header file.
> Maxime has raised this point in other patches...
>
> Why? Obvious what but I have no idea why you are doing this.
>
> Thanks,
>
> Jonathan
There are two reasons:
1. Personal taste: I like to have the #define stuff in the header file.
2. When I started the rework I had to get some better overview, so I
moved it...

Since those two reasons are no good reasons to submit a patch I will
drop it and keep it in the *.c file.

Philipp

>>
>> Signed-off-by: Philipp Rossak <[email protected]>
>> ---
>> drivers/iio/adc/sun4i-gpadc-iio.c | 9 ---------
>> include/linux/mfd/sun4i-gpadc.h | 9 +++++++++
>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index d95dd0fde2a6..666329940e1e 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -109,15 +109,6 @@ struct sun4i_gpadc_iio {
>> struct device *sensor_device;
>> };
>>
>> -#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
>> - .type = IIO_VOLTAGE, \
>> - .indexed = 1, \
>> - .channel = _channel, \
>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> - .datasheet_name = _name, \
>> -}
>> -
>> static struct iio_map sun4i_gpadc_hwmon_maps[] = {
>> {
>> .adc_channel_label = "temp_adc",
>> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
>> index 139872c2e0fe..54c7c9375c1b 100644
>> --- a/include/linux/mfd/sun4i-gpadc.h
>> +++ b/include/linux/mfd/sun4i-gpadc.h
>> @@ -90,6 +90,15 @@
>> /* 10s delay before suspending the IP */
>> #define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
>>
>> +#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
>> + .type = IIO_VOLTAGE, \
>> + .indexed = 1, \
>> + .channel = _channel, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> + .datasheet_name = _name, \
>> +}
>> +
>> struct sun4i_gpadc_dev {
>> struct device *dev;
>> struct regmap *regmap;
>

2018-09-03 17:30:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 05/30] iio: adc: move SUN4I_GPADC_CHANNEL define to header file

On Mon, 3 Sep 2018 16:24:32 +0200
Philipp Rossak <[email protected]> wrote:

> On 02.09.2018 22:01, Jonathan Cameron wrote:
> > On Thu, 30 Aug 2018 17:44:53 +0200
> > Philipp Rossak <[email protected]> wrote:
> >
> >> We are moving the SUN4I_GPADC_CHANNEL define to the header file.
> > Maxime has raised this point in other patches...
> >
> > Why? Obvious what but I have no idea why you are doing this.
> >
> > Thanks,
> >
> > Jonathan
> There are two reasons:
> 1. Personal taste: I like to have the #define stuff in the header file.
> 2. When I started the rework I had to get some better overview, so I
> moved it...
>
> Since those two reasons are no good reasons to submit a patch I will
> drop it and keep it in the *.c file.
Don't move it.

For a 'utility' type define like this that is just about cutting
down on code repetition it is nice to be able to see what it does near
to where it is used.

Also, as a general rule kernel style is to not put things in a header
unless they are needed in multiple files. There are exceptions, but
it is generally felt keeping things local to where they are used
leads to easier to review code.

Jonathan
>
> Philipp
>
> >>
> >> Signed-off-by: Philipp Rossak <[email protected]>
> >> ---
> >> drivers/iio/adc/sun4i-gpadc-iio.c | 9 ---------
> >> include/linux/mfd/sun4i-gpadc.h | 9 +++++++++
> >> 2 files changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> >> index d95dd0fde2a6..666329940e1e 100644
> >> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> >> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> >> @@ -109,15 +109,6 @@ struct sun4i_gpadc_iio {
> >> struct device *sensor_device;
> >> };
> >>
> >> -#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
> >> - .type = IIO_VOLTAGE, \
> >> - .indexed = 1, \
> >> - .channel = _channel, \
> >> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> >> - .datasheet_name = _name, \
> >> -}
> >> -
> >> static struct iio_map sun4i_gpadc_hwmon_maps[] = {
> >> {
> >> .adc_channel_label = "temp_adc",
> >> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> >> index 139872c2e0fe..54c7c9375c1b 100644
> >> --- a/include/linux/mfd/sun4i-gpadc.h
> >> +++ b/include/linux/mfd/sun4i-gpadc.h
> >> @@ -90,6 +90,15 @@
> >> /* 10s delay before suspending the IP */
> >> #define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
> >>
> >> +#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) { \
> >> + .type = IIO_VOLTAGE, \
> >> + .indexed = 1, \
> >> + .channel = _channel, \
> >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> >> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> >> + .datasheet_name = _name, \
> >> +}
> >> +
> >> struct sun4i_gpadc_dev {
> >> struct device *dev;
> >> struct regmap *regmap;
> >


2018-09-03 17:31:07

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 18/30] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

On Mon, 3 Sep 2018 15:58:30 +0200
Philipp Rossak <[email protected]> wrote:

> On 02.09.2018 22:11, Jonathan Cameron wrote:
> > This feels like a good place to factor out the code into a utility
> > function that just does one of them. That should hopefully
> > reduce the indenting etc enough to make the code easier to read.
> >
> >> + info->tzds[i].info = info;
> >> + info->tzds[i].sensor_id = i;
> >> +
> >> + info->tzds[i].tzd = thermal_zone_of_sensor_register(
> >> + info->sensor_device,
> >> + i, &info->tzds[i], &sun4i_ts_tz_ops);
> >> + /*
> >> + * Do not fail driver probing when failing to register in
> >> + * thermal because no thermal DT node is found.
> >> + */
> >> + if (IS_ERR(info->tzds[i].tzd) && \
> >> + PTR_ERR(info->tzds[i].tzd) != -ENODEV) {
> >> + dev_err(&pdev->dev,
> >> + "could not register thermal sensor: %ld\n",
> >> + PTR_ERR(info->tzds[i].tzd));
> >> + return PTR_ERR(info->tzds[i].tzd);
> >> + }
>
> So this code above should be placed in a separate function and called by
> the for loop?
> Did I understand that right?
Yes - exactly.

>
> Philipp


2018-09-04 16:47:49

by Emmanuel Vadot

[permalink] [raw]
Subject: Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths


Hi Philipp,

On Thu, 30 Aug 2018 17:45:15 +0200
Philipp Rossak <[email protected]> wrote:

> The H3 SID is supported by the kernel so we can add a NVMEM Data cell,
> that contains the calibration data.
>
> On the H3 the eFuses are located at the offset 0x200. The thermal data
> itself has an offset of 0x34 from the eFuse base. So we end on an offset
> of 0x234.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> 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 1866aec69ec1..0fc447f0c02a 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -106,8 +106,15 @@
>
> soc {
> sid: eeprom@1c14000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> compatible = "allwinner,sun8i-h3-sid";
> reg = <0x01c14000 0x400>;
> +
> + /* Data cells */
> + thermal_calibration: calib@234 {
> + reg = <0x234 0x8>;
> + };

You are declaring 8 bytes of calibration data but to my knowledge it's
only 2 bytes per sensor, so 2 bytes for H3.
Am I missing something ?

Thanks,
> };
> };
>
> @@ -227,4 +234,6 @@
> &ths {
> compatible = "allwinner,sun8i-h3-ths";
> #thermal-sensor-cells = <0>;
> + nvmem-cells = <&thermal_calibration>;
> + nvmem-cell-names = "calibration";
> };
> --
> 2.11.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
Emmanuel Vadot <[email protected]> <[email protected]>

2018-09-05 15:00:33

by Maxime Ripard

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

On Mon, Sep 03, 2018 at 07:01:47PM +0800, Icenowy Zheng wrote:
> 于 2018年9月3日 GMT+08:00 下午6:20:22, Maxime Ripard <[email protected]> 写到:
> >On Fri, Aug 31, 2018 at 05:51:41PM +0800, Icenowy Zheng wrote:
> >> Personally I suggest to leave out all SID or calibration related
> >> patches here.
> >>
> >> Currently we seems to be wrongly converting SID to big endian,
> >however,
> >> the orgnization of the THS calibration data on H6 shows that it's
> >> surely little endian:
> >>
> >> It consists a temperature value in 1/10 celsuis as unit, and some
> >> thermal register readout values, which are the values read out at the
> >> given temperature, and every value here (the temperature and the
> >> readout) are all half word length.
> >>
> >> Let the temperature value be AABB, the two readout values be XXYY and
> >> ZZWW, the oragnization is:
> >> BB AA YY XX WW ZZ ** ** .
> >>
> >> When converting the SID to big endian, it becomes:
> >> XX YY AA BB ** ** ZZ WW ,
> >> which is non-sense, and not able to do sub-word cell addressing.
> >>
> >> Maxime, should I drop the LE2BE conversion in SID driver? (I doubt
> >> whether it will break compatibility.)
> >
> >This is exposed to the userspace, so no.
>
> Please note the LE2BE totally breaks the SID addressing.

On which SoCs?

> Without it dropped all cells must be referenced with 4 byte word as
> unit, and half word addressing of SID is thus not possible. The
> driver will also need then to split the half words if needed.
>
> I think this is kind of hardware misbehavior, and not a simple
> UAPI change, so the UAPI stability shouldn't affect this change.

How would you feel having your files on your filesystem suddenly not
in the same endianness and having all the bytes reordered? This is
definitely about the userspace ABI.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.92 kB)
signature.asc (849.00 B)
Download all attachments

2018-09-06 07:26:48

by Quentin Schulz

[permalink] [raw]
Subject: Re: [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

Hi Philipp,

On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:
> Since we have now thermal trotteling enabeled we can now add the full
> range of the OPP table.
>

That's not the reason why they were not added.

Please see commit 2db639d8c1663d7543c9ab5323383d94c8a76c63[1].

Basically, you only want the OPPs which can work below or at the default
voltage of the CPU supply, because the CPU supply is specific to each
board.

If you set your CPU to work at a given frequency and the voltage isn't
updated (saying opp-microvolt = <x>; in DT isn't enough, you need
cpu-supply to be provided and functional), the CPU might just crash.

Without cpu-supply property, underclocking isn't effective in term of
thermal cooling or power saving. Overclocking is very, very, very likely
to make the CPU crash.

It's not a very difficult thing to do to test if a given frequency work
well but it needs a specific test environment and it's a lengthy test,
you can have a look at those tools here[3] if you like. It's not because
it works in a given test case that'll work on the long term under heavy
load and constant frequency changes.

For A83T, I already did it and the outcome is the patch in [1]. Same for
A33.

So, if you want to use these three higher OPPs, you need to define them
in your board DTS and add the cpu-supply property. See what's done for
the A33 and more specifically the Sinlinx SinA33[2] as an example.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2db639d8c1663d7543c9ab5323383d94c8a76c63
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
[3] http://linux-sunxi.org/Hardware_Reliability_Tests#CPU

Quentin

> The operating points were found in Allwinner BSP and fex files.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t.dtsi | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index 78aa448e869f..ddcf404f9c80 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -250,6 +250,22 @@
> opp-microvolt = <840000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
> +
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <920000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-1800000000 { /* BOOT FREQ */
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <1000000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + opp-microvolt = <1080000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> };
>
> cpu1_opp_table: opp_table1 {
> @@ -303,6 +319,22 @@
> opp-microvolt = <840000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
> +
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <920000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-1800000000 { /* BOOT FREQ */
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <1000000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + opp-microvolt = <1080000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> };
>
> soc {
> --
> 2.11.0
>


Attachments:
(No filename) (3.51 kB)
signature.asc (849.00 B)
Download all attachments

2018-09-06 11:47:05

by Philipp Rossak

[permalink] [raw]
Subject: Re: [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

On 06.09.2018 09:24, Quentin Schulz wrote:
> Hi Philipp,
>
> On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:
>> Since we have now thermal trotteling enabeled we can now add the full
>> range of the OPP table.
>>
> That's not the reason why they were not added.
>
> Please see commit 2db639d8c1663d7543c9ab5323383d94c8a76c63[1].
>
> Basically, you only want the OPPs which can work below or at the default
> voltage of the CPU supply, because the CPU supply is specific to each
> board.
>
> If you set your CPU to work at a given frequency and the voltage isn't
> updated (saying opp-microvolt = <x>; in DT isn't enough, you need
> cpu-supply to be provided and functional), the CPU might just crash.
>
> Without cpu-supply property, underclocking isn't effective in term of
> thermal cooling or power saving. Overclocking is very, very, very likely
> to make the CPU crash.
>
> It's not a very difficult thing to do to test if a given frequency work
> well but it needs a specific test environment and it's a lengthy test,
> you can have a look at those tools here[3] if you like. It's not because
> it works in a given test case that'll work on the long term under heavy
> load and constant frequency changes.
>
> For A83T, I already did it and the outcome is the patch in [1]. Same for
> A33.
>
> So, if you want to use these three higher OPPs, you need to define them
> in your board DTS and add the cpu-supply property. See what's done for
> the A33 and more specifically the Sinlinx SinA33[2] as an example.
>
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2db639d8c1663d7543c9ab5323383d94c8a76c63
> [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
> [3]http://linux-sunxi.org/Hardware_Reliability_Tests#CPU
>
> Quentin
>

Hey Quentin,

thanks for your feedback!

Sounds like we will never be able to run the A83T on its maximum
frequency in mainline.

I will do some testing, during the next weeks/months when I have time.
With the old Allwinner kernel I was able to run the A83T with its
maximum frequency without any problems since my board is very good cooled.

For now I will drop this patch.

Philipp

2018-09-06 11:47:54

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

On Thu, Sep 06, 2018 at 01:39:43PM +0200, Philipp Rossak wrote:
> On 06.09.2018 09:24, Quentin Schulz wrote:
> > Hi Philipp,
> >
> > On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:
> > > Since we have now thermal trotteling enabeled we can now add the full
> > > range of the OPP table.
> > >
> > That's not the reason why they were not added.
> >
> > Please see commit 2db639d8c1663d7543c9ab5323383d94c8a76c63[1].
> >
> > Basically, you only want the OPPs which can work below or at the default
> > voltage of the CPU supply, because the CPU supply is specific to each
> > board.
> >
> > If you set your CPU to work at a given frequency and the voltage isn't
> > updated (saying opp-microvolt = <x>; in DT isn't enough, you need
> > cpu-supply to be provided and functional), the CPU might just crash.
> >
> > Without cpu-supply property, underclocking isn't effective in term of
> > thermal cooling or power saving. Overclocking is very, very, very likely
> > to make the CPU crash.
> >
> > It's not a very difficult thing to do to test if a given frequency work
> > well but it needs a specific test environment and it's a lengthy test,
> > you can have a look at those tools here[3] if you like. It's not because
> > it works in a given test case that'll work on the long term under heavy
> > load and constant frequency changes.
> >
> > For A83T, I already did it and the outcome is the patch in [1]. Same for
> > A33.
> >
> > So, if you want to use these three higher OPPs, you need to define them
> > in your board DTS and add the cpu-supply property. See what's done for
> > the A33 and more specifically the Sinlinx SinA33[2] as an example.
> >
> > [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2db639d8c1663d7543c9ab5323383d94c8a76c63
> > [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
> > [3]http://linux-sunxi.org/Hardware_Reliability_Tests#CPU
> >
> > Quentin
> >
>
> Hey Quentin,
>
> thanks for your feedback!
>
> Sounds like we will never be able to run the A83T on its maximum frequency
> in mainline.
>
> I will do some testing, during the next weeks/months when I have time.
> With the old Allwinner kernel I was able to run the A83T with its maximum
> frequency without any problems since my board is very good cooled.

You definitely can, but I think Quentin's point was to do it on a
per-board basis, not for all of the A83t boards at once.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (2.60 kB)
signature.asc (849.00 B)
Download all attachments

2018-09-06 12:06:16

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths



于 2018年9月6日 GMT+08:00 下午7:51:15, Maxime Ripard <[email protected]> 写到:
>On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
>> On 04.09.2018 18:46, Emmanuel Vadot wrote:
>> > > + /* Data cells */
>> > > + thermal_calibration: calib@234 {
>> > > + reg = <0x234 0x8>;
>> > > + };
>> > You are declaring 8 bytes of calibration data but to my knowledge
>it's
>> > only 2 bytes per sensor, so 2 bytes for H3.
>> > Am I missing something ?
>> >
>> > Thanks,
>>
>> Emmanuel you are right, it is 2 bytes per Sensor and should be 2
>bytes for
>> H3, but the thermal calibration data field is on all chips 64 bit
>wide - so
>> 8 bytes. So I'm reading here the complete calibration data field.
>
>Having one cell per channel would make more sense I guess.

I have mentioned that this is impossible because of wrong
addressing caused by LE2BE in SID driver.

>
>Maxime

2018-09-06 12:07:55

by Quentin Schulz

[permalink] [raw]
Subject: Re: [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

Hi Maxime, Philipp,

On Thu, Sep 06, 2018 at 01:42:41PM +0200, Maxime Ripard wrote:
> On Thu, Sep 06, 2018 at 01:39:43PM +0200, Philipp Rossak wrote:
> > On 06.09.2018 09:24, Quentin Schulz wrote:
> > > Hi Philipp,
> > >
> > > On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:
> > > > Since we have now thermal trotteling enabeled we can now add the full
> > > > range of the OPP table.
> > > >
> > > That's not the reason why they were not added.
> > >
> > > Please see commit 2db639d8c1663d7543c9ab5323383d94c8a76c63[1].
> > >
> > > Basically, you only want the OPPs which can work below or at the default
> > > voltage of the CPU supply, because the CPU supply is specific to each
> > > board.
> > >
> > > If you set your CPU to work at a given frequency and the voltage isn't
> > > updated (saying opp-microvolt = <x>; in DT isn't enough, you need
> > > cpu-supply to be provided and functional), the CPU might just crash.
> > >
> > > Without cpu-supply property, underclocking isn't effective in term of
> > > thermal cooling or power saving. Overclocking is very, very, very likely
> > > to make the CPU crash.
> > >
> > > It's not a very difficult thing to do to test if a given frequency work
> > > well but it needs a specific test environment and it's a lengthy test,
> > > you can have a look at those tools here[3] if you like. It's not because
> > > it works in a given test case that'll work on the long term under heavy
> > > load and constant frequency changes.
> > >
> > > For A83T, I already did it and the outcome is the patch in [1]. Same for
> > > A33.
> > >
> > > So, if you want to use these three higher OPPs, you need to define them
> > > in your board DTS and add the cpu-supply property. See what's done for
> > > the A33 and more specifically the Sinlinx SinA33[2] as an example.
> > >
> > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2db639d8c1663d7543c9ab5323383d94c8a76c63
> > > [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
> > > [3]http://linux-sunxi.org/Hardware_Reliability_Tests#CPU
> > >
> > > Quentin
> > >
> >
> > Hey Quentin,
> >
> > thanks for your feedback!
> >
> > Sounds like we will never be able to run the A83T on its maximum frequency
> > in mainline.
> >
> > I will do some testing, during the next weeks/months when I have time.
> > With the old Allwinner kernel I was able to run the A83T with its maximum
> > frequency without any problems since my board is very good cooled.
>
> You definitely can, but I think Quentin's point was to do it on a
> per-board basis, not for all of the A83t boards at once.
>

Yes, exactly. That's what we did for the Sinlinx SinA33. Just take
inspiration from it.

You can run the CPU at max frequency with the old Allwinner kernel
because it most likely updates the CPU voltage. Let's say overheating is
not an issue, you still need to have a way to overclock AND overvolt to
the values in the OPP you're targetting. Your CPU won't be stable with
just overclocking without overvolting it, trust me, I've been there.

It can even be possible that the Allwinner BSP is only stable in your
use case.

The CPU voltage is specific to every board (cpu-supply property). So we
do not declare the CPU frequencies that require overvolting since we
cannot be sure a board will define the cpu-supply property and thus, be
able to overvolt the CPU.

So, though thermal throttling is an important feature because
overheating the CPU too much will shut it down on the hardware level
(among other consequences), it does not make the higher OPPs magically
work without a valid and working cpu-supply.

The link I gave you is an important piece of software to test the
stability of a system under heavy load and a lot of frequency changes.
It's important to know that because it works for you in your use case
doesn't make it work in any use case. Testing with these pieces of
software helps to cover more hardcore use cases but isn't perfect of
course. That's a start though.

Quentin


Attachments:
(No filename) (4.09 kB)
signature.asc (849.00 B)
Download all attachments

2018-09-06 17:53:51

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths

On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
> On 04.09.2018 18:46, Emmanuel Vadot wrote:
> > > + /* Data cells */
> > > + thermal_calibration: calib@234 {
> > > + reg = <0x234 0x8>;
> > > + };
> > You are declaring 8 bytes of calibration data but to my knowledge it's
> > only 2 bytes per sensor, so 2 bytes for H3.
> > Am I missing something ?
> >
> > Thanks,
>
> Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes for
> H3, but the thermal calibration data field is on all chips 64 bit wide - so
> 8 bytes. So I'm reading here the complete calibration data field.

Having one cell per channel would make more sense I guess.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (801.00 B)
signature.asc (849.00 B)
Download all attachments

2018-09-06 17:54:50

by Philipp Rossak

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths



On 06.09.2018 14:04, Icenowy Zheng wrote:
>
>
> 于 2018年9月6日 GMT+08:00 下午7:51:15, Maxime Ripard <[email protected]> 写到:
>> On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
>>> On 04.09.2018 18:46, Emmanuel Vadot wrote:
>>>>> + /* Data cells */
>>>>> + thermal_calibration: calib@234 {
>>>>> + reg = <0x234 0x8>;
>>>>> + };
>>>> You are declaring 8 bytes of calibration data but to my knowledge
>> it's
>>>> only 2 bytes per sensor, so 2 bytes for H3.
>>>> Am I missing something ?
>>>>
>>>> Thanks,
>>>
>>> Emmanuel you are right, it is 2 bytes per Sensor and should be 2
>> bytes for
>>> H3, but the thermal calibration data field is on all chips 64 bit
>> wide - so
>>> 8 bytes. So I'm reading here the complete calibration data field.
>>
>> Having one cell per channel would make more sense I guess.
Ok I will change this.
>
> I have mentioned that this is impossible because of wrong
> addressing caused by LE2BE in SID driver.
>
I know! But I would like to prepare patches for it, that they can be
merged when this is fixed.

Philipp


2018-09-06 18:23:58

by Philipp Rossak

[permalink] [raw]
Subject: Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths

On 04.09.2018 18:46, Emmanuel Vadot wrote:
>> + /* Data cells */
>> + thermal_calibration: calib@234 {
>> + reg = <0x234 0x8>;
>> + };
> You are declaring 8 bytes of calibration data but to my knowledge it's
> only 2 bytes per sensor, so 2 bytes for H3.
> Am I missing something ?
>
> Thanks,

Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes
for H3, but the thermal calibration data field is on all chips 64 bit
wide - so 8 bytes. So I'm reading here the complete calibration data field.

Philipp

2018-09-10 19:44:54

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 14/30] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

On Fri, Aug 31, 2018 at 10:48:54AM +0200, Maxime Ripard wrote:
> On Thu, Aug 30, 2018 at 05:45:02PM +0200, Philipp Rossak wrote:
> > Allwinner H3 features a thermal sensor like the one in A33, but has its
> > register re-arranged, the clock divider moved to CCU (originally the
> > clock divider is in ADC) and added a pair of bus clock and reset.
> >
> > Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
> > the bus clock and the reset was removed from the CCU. The THS in A83T
> > has a clock that is directly connected and runs with 24 MHz.
> >
> > Update the binding document to cover H3 and A83T.
> >
> > Signed-off-by: Philipp Rossak <[email protected]>
>
> You probably want to have a look at:
> https://www.spinics.net/lists/arm-kernel/msg670167.html

Well, which is it? An ADC or thermal sensor?

Rob

2018-09-11 09:14:24

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 14/30] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

On Mon, Sep 10, 2018 at 02:44:24PM -0500, Rob Herring wrote:
> On Fri, Aug 31, 2018 at 10:48:54AM +0200, Maxime Ripard wrote:
> > On Thu, Aug 30, 2018 at 05:45:02PM +0200, Philipp Rossak wrote:
> > > Allwinner H3 features a thermal sensor like the one in A33, but has its
> > > register re-arranged, the clock divider moved to CCU (originally the
> > > clock divider is in ADC) and added a pair of bus clock and reset.
> > >
> > > Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
> > > the bus clock and the reset was removed from the CCU. The THS in A83T
> > > has a clock that is directly connected and runs with 24 MHz.
> > >
> > > Update the binding document to cover H3 and A83T.
> > >
> > > Signed-off-by: Philipp Rossak <[email protected]>
> >
> > You probably want to have a look at:
> > https://www.spinics.net/lists/arm-kernel/msg670167.html
>
> Well, which is it? An ADC or thermal sensor?

It's both actually. This IP used to be called GPADC, and had a thermal
sensor + some ADC channels. The design evolved across several
generations of SoCs to drop the ADC channels and be used only to have
thermal sensors.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.25 kB)
signature.asc (849.00 B)
Download all attachments

2019-02-19 07:54:49

by Chen-Yu Tsai

[permalink] [raw]
Subject: Allwinner SID THS calibration data cell representation?

Sorry for resurrecting an old discussion, but since someone posted patches
for H5 and H6, I thought we should resolve this. I'm working on patches to
fix / replace the big-endian issue.

On Thu, Sep 6, 2018 at 7:51 PM Maxime Ripard <[email protected]> wrote:
>
> On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
> > On 04.09.2018 18:46, Emmanuel Vadot wrote:
> > > > + /* Data cells */
> > > > + thermal_calibration: calib@234 {
> > > > + reg = <0x234 0x8>;
> > > > + };
> > > You are declaring 8 bytes of calibration data but to my knowledge it's
> > > only 2 bytes per sensor, so 2 bytes for H3.
> > > Am I missing something ?
> > >
> > > Thanks,
> >
> > Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes for
> > H3, but the thermal calibration data field is on all chips 64 bit wide - so
> > 8 bytes. So I'm reading here the complete calibration data field.
>
> Having one cell per channel would make more sense I guess.

Would it? The 2 32-bit words directly map onto the registers 0x74 / 0x78 in
the THS. As far as the SID is concerned, their is just one consumer for this
data, the thermal sensor. How the thermal sensor uses that data is really not
its concern. And the thermal sensor is really just copying the data from the
e-fuses into its registers. Nothing more.

Furthermore, with the register access interface, the e-fuses are read/write
32 bits at a time. Seems to me it would make more sense to enforce a 32-bit
word size, so cells should be multiples of 32 bits.

Regards
ChenYu

2019-02-20 14:56:51

by Maxime Ripard

[permalink] [raw]
Subject: Re: Allwinner SID THS calibration data cell representation?

On Tue, Feb 19, 2019 at 03:54:00PM +0800, Chen-Yu Tsai wrote:
> Sorry for resurrecting an old discussion, but since someone posted patches
> for H5 and H6, I thought we should resolve this. I'm working on patches to
> fix / replace the big-endian issue.
>
> On Thu, Sep 6, 2018 at 7:51 PM Maxime Ripard <[email protected]> wrote:
> >
> > On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
> > > On 04.09.2018 18:46, Emmanuel Vadot wrote:
> > > > > + /* Data cells */
> > > > > + thermal_calibration: calib@234 {
> > > > > + reg = <0x234 0x8>;
> > > > > + };
> > > > You are declaring 8 bytes of calibration data but to my knowledge it's
> > > > only 2 bytes per sensor, so 2 bytes for H3.
> > > > Am I missing something ?
> > > >
> > > > Thanks,
> > >
> > > Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes for
> > > H3, but the thermal calibration data field is on all chips 64 bit wide - so
> > > 8 bytes. So I'm reading here the complete calibration data field.
> >
> > Having one cell per channel would make more sense I guess.
>
> Would it? The 2 32-bit words directly map onto the registers 0x74 / 0x78 in
> the THS. As far as the SID is concerned, their is just one consumer for this
> data, the thermal sensor. How the thermal sensor uses that data is really not
> its concern. And the thermal sensor is really just copying the data from the
> e-fuses into its registers. Nothing more.
>
> Furthermore, with the register access interface, the e-fuses are read/write
> 32 bits at a time. Seems to me it would make more sense to enforce a 32-bit
> word size, so cells should be multiples of 32 bits.

I guess you convinced me :)

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.85 kB)
signature.asc (235.00 B)
Download all attachments

2019-02-21 10:18:49

by Emmanuel Vadot

[permalink] [raw]
Subject: Re: Allwinner SID THS calibration data cell representation?

On Tue, 19 Feb 2019 15:54:00 +0800
Chen-Yu Tsai <[email protected]> wrote:

> Sorry for resurrecting an old discussion, but since someone posted patches
> for H5 and H6, I thought we should resolve this. I'm working on patches to
> fix / replace the big-endian issue.
>
> On Thu, Sep 6, 2018 at 7:51 PM Maxime Ripard <[email protected]> wrote:
> >
> > On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
> > > On 04.09.2018 18:46, Emmanuel Vadot wrote:
> > > > > + /* Data cells */
> > > > > + thermal_calibration: calib@234 {
> > > > > + reg = <0x234 0x8>;
> > > > > + };
> > > > You are declaring 8 bytes of calibration data but to my knowledge it's
> > > > only 2 bytes per sensor, so 2 bytes for H3.
> > > > Am I missing something ?
> > > >
> > > > Thanks,
> > >
> > > Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes for
> > > H3, but the thermal calibration data field is on all chips 64 bit wide - so
> > > 8 bytes. So I'm reading here the complete calibration data field.
> >
> > Having one cell per channel would make more sense I guess.
>
> Would it? The 2 32-bit words directly map onto the registers 0x74 / 0x78 in
> the THS. As far as the SID is concerned, their is just one consumer for this
> data, the thermal sensor. How the thermal sensor uses that data is really not
> its concern. And the thermal sensor is really just copying the data from the
> e-fuses into its registers. Nothing more.

If you see this at the controller lever and not at the sensor level it
might make sense to use the full 32bits data even some bits are ignored
I guess.

> Furthermore, with the register access interface, the e-fuses are read/write
> 32 bits at a time. Seems to me it would make more sense to enforce a 32-bit
> word size, so cells should be multiples of 32 bits.

Ok, honestly at this point I don't really care what choice is done as
long as the dts is updated, I'll deal with the diff for FreeBSD and
ping the others BSD (Net and Open who also have a driver) for this
change.

> Regards
> ChenYu

Cheers,

--
Emmanuel Vadot <[email protected]> <[email protected]>

2019-02-25 20:38:35

by Philipp Rossak

[permalink] [raw]
Subject: Re: Allwinner SID THS calibration data cell representation?



On 19.02.19 08:54, Chen-Yu Tsai wrote:
> Sorry for resurrecting an old discussion, but since someone posted patches
> for H5 and H6, I thought we should resolve this. I'm working on patches to
> fix / replace the big-endian issue.
>
> On Thu, Sep 6, 2018 at 7:51 PM Maxime Ripard <[email protected]> wrote:
>>
>> On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:
>>> On 04.09.2018 18:46, Emmanuel Vadot wrote:
>>>>> + /* Data cells */
>>>>> + thermal_calibration: calib@234 {
>>>>> + reg = <0x234 0x8>;
>>>>> + };
>>>> You are declaring 8 bytes of calibration data but to my knowledge it's
>>>> only 2 bytes per sensor, so 2 bytes for H3.
>>>> Am I missing something ?
>>>>
>>>> Thanks,
>>>
>>> Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes for
>>> H3, but the thermal calibration data field is on all chips 64 bit wide - so
>>> 8 bytes. So I'm reading here the complete calibration data field.
>>
>> Having one cell per channel would make more sense I guess.
>
> Would it? The 2 32-bit words directly map onto the registers 0x74 / 0x78 in
> the THS. As far as the SID is concerned, their is just one consumer for this
> data, the thermal sensor. How the thermal sensor uses that data is really not
> its concern. And the thermal sensor is really just copying the data from the
> e-fuses into its registers. Nothing more.

Using 2 32-bit words for the THS would be also ok (from my perspective).
>
> Furthermore, with the register access interface, the e-fuses are read/write
> 32 bits at a time. Seems to me it would make more sense to enforce a 32-bit
> word size, so cells should be multiples of 32 bits.
>
For THS I'm ok with that.

> Regards
> ChenYu
>
Regards,
Philipp

2019-03-19 12:32:45

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v3 00/30] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

Philipp Rossak <[email protected]> writes:

> 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
>
> This Patchtseries was in the beginning based on Icenowy Zengs v4 patchseries [1]. Since we decided to merge the mfd driver into the GPADC this changed. So only one patch could be reused.
>
> Patches that adds support for H5, A64, A80 and H6 SoCs are allready prepared,
> and will be upstreamed if this patchseries is applied and the testing is done.
>
> Sorry for delaying this.
>
> Regards,
> Philipp
>
> changes since v2:
> * mfd driver is now merged into the gpadc driver
> * complete rework
>
> 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 (29):
> mfd: Makefile: Remove build option for MFD:sun4i-gpadc
> mfd: Kconfig: Remove MFD_SUN4I_GPADC config option
> iio: adc: Remove ID table
> iio: adc: Kconfig: Update Kconfig to new build options
> iio: adc: move SUN4I_GPADC_CHANNEL define to header file
> iio: adc: remove ofnode options
> iio: adc: remove mfd_probe & sunwi_irq_init function
> iio: adc: remove hwmon structure
> iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i
> channel
> iio: adc: rework irq and adc_channel handling
> iio: adc: add new compatibles
> mfd: Remove old mfd driver & Move sun4i-gpadc.h to iio/adc/
> arm: config: Enable SUN4I_GPADC in defconfig
> dt-bindings: update the Allwinner GPADC device tree binding for H3 &
> A83T
> iio: adc: sun4i-gpadc-iio: rework: readout temp_data
> 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: device specific suspend & resume
> 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: h3: use calibration for ths
> ARM: dts: sun8i: a83t: add support for the thermal sensor in A83T
> ARM: dts: sun8i: a83t: add thermal zone to A83T
> ARM: sun8i: a83t: full range OPP tables and CPUfreq
>
> .../devicetree/bindings/iio/adc/sun4i-gpadc.txt | 41 +-
> arch/arm/boot/dts/sun8i-a83t.dtsi | 143 +++++
> arch/arm/boot/dts/sun8i-h3.dtsi | 52 ++
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
> arch/arm/configs/sunxi_defconfig | 1 +
> drivers/iio/adc/Kconfig | 11 +-
> drivers/iio/adc/sun4i-gpadc-iio.c | 617 +++++++++++++--------
> drivers/mfd/Kconfig | 17 -
> drivers/mfd/Makefile | 1 -
> drivers/mfd/sun4i-gpadc.c | 181 ------
> include/linux/{mfd => iio/adc}/sun4i-gpadc.h | 47 +-
> 11 files changed, 681 insertions(+), 440 deletions(-)
> delete mode 100644 drivers/mfd/sun4i-gpadc.c
> rename include/linux/{mfd => iio/adc}/sun4i-gpadc.h (72%)

What became of these patches? I can't seem to find any follow-ups. Was
the approach abandoned in favour of something else?

--
M?ns Rullg?rd

2019-03-19 12:38:23

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 00/30] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

On Tue, Mar 19, 2019 at 12:30:25PM +0000, M?ns Rullg?rd wrote:
> Philipp Rossak <[email protected]> writes:
>
> > 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
> >
> > This Patchtseries was in the beginning based on Icenowy Zengs v4 patchseries [1]. Since we decided to merge the mfd driver into the GPADC this changed. So only one patch could be reused.
> >
> > Patches that adds support for H5, A64, A80 and H6 SoCs are allready prepared,
> > and will be upstreamed if this patchseries is applied and the testing is done.
> >
> > Sorry for delaying this.
> >
> > Regards,
> > Philipp
> >
> > changes since v2:
> > * mfd driver is now merged into the gpadc driver
> > * complete rework
> >
> > 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 (29):
> > mfd: Makefile: Remove build option for MFD:sun4i-gpadc
> > mfd: Kconfig: Remove MFD_SUN4I_GPADC config option
> > iio: adc: Remove ID table
> > iio: adc: Kconfig: Update Kconfig to new build options
> > iio: adc: move SUN4I_GPADC_CHANNEL define to header file
> > iio: adc: remove ofnode options
> > iio: adc: remove mfd_probe & sunwi_irq_init function
> > iio: adc: remove hwmon structure
> > iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i
> > channel
> > iio: adc: rework irq and adc_channel handling
> > iio: adc: add new compatibles
> > mfd: Remove old mfd driver & Move sun4i-gpadc.h to iio/adc/
> > arm: config: Enable SUN4I_GPADC in defconfig
> > dt-bindings: update the Allwinner GPADC device tree binding for H3 &
> > A83T
> > iio: adc: sun4i-gpadc-iio: rework: readout temp_data
> > 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: device specific suspend & resume
> > 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: h3: use calibration for ths
> > ARM: dts: sun8i: a83t: add support for the thermal sensor in A83T
> > ARM: dts: sun8i: a83t: add thermal zone to A83T
> > ARM: sun8i: a83t: full range OPP tables and CPUfreq
> >
> > .../devicetree/bindings/iio/adc/sun4i-gpadc.txt | 41 +-
> > arch/arm/boot/dts/sun8i-a83t.dtsi | 143 +++++
> > arch/arm/boot/dts/sun8i-h3.dtsi | 52 ++
> > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
> > arch/arm/configs/sunxi_defconfig | 1 +
> > drivers/iio/adc/Kconfig | 11 +-
> > drivers/iio/adc/sun4i-gpadc-iio.c | 617 +++++++++++++--------
> > drivers/mfd/Kconfig | 17 -
> > drivers/mfd/Makefile | 1 -
> > drivers/mfd/sun4i-gpadc.c | 181 ------
> > include/linux/{mfd => iio/adc}/sun4i-gpadc.h | 47 +-
> > 11 files changed, 681 insertions(+), 440 deletions(-)
> > delete mode 100644 drivers/mfd/sun4i-gpadc.c
> > rename include/linux/{mfd => iio/adc}/sun4i-gpadc.h (72%)
>
> What became of these patches? I can't seem to find any follow-ups. Was
> the approach abandoned in favour of something else?

It didn't, I don't think we ever got any follow-up to that series so
that's the main reason it's been held back.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (4.33 kB)
signature.asc (235.00 B)
Download all attachments

2019-03-19 13:06:19

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH v3 00/30] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

On Tue, Mar 19, 2019 at 8:37 PM Maxime Ripard <[email protected]> wrote:
>
> On Tue, Mar 19, 2019 at 12:30:25PM +0000, Måns Rullgård wrote:
> > Philipp Rossak <[email protected]> writes:
> >
> > > 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
> > >
> > > This Patchtseries was in the beginning based on Icenowy Zengs v4 patchseries [1]. Since we decided to merge the mfd driver into the GPADC this changed. So only one patch could be reused.
> > >
> > > Patches that adds support for H5, A64, A80 and H6 SoCs are allready prepared,
> > > and will be upstreamed if this patchseries is applied and the testing is done.
> > >
> > > Sorry for delaying this.
> > >
> > > Regards,
> > > Philipp
> > >
> > > changes since v2:
> > > * mfd driver is now merged into the gpadc driver
> > > * complete rework
> > >
> > > 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 (29):
> > > mfd: Makefile: Remove build option for MFD:sun4i-gpadc
> > > mfd: Kconfig: Remove MFD_SUN4I_GPADC config option
> > > iio: adc: Remove ID table
> > > iio: adc: Kconfig: Update Kconfig to new build options
> > > iio: adc: move SUN4I_GPADC_CHANNEL define to header file
> > > iio: adc: remove ofnode options
> > > iio: adc: remove mfd_probe & sunwi_irq_init function
> > > iio: adc: remove hwmon structure
> > > iio: adc: Threat A33 as thermal sensor and remove non thermal sun4i
> > > channel
> > > iio: adc: rework irq and adc_channel handling
> > > iio: adc: add new compatibles
> > > mfd: Remove old mfd driver & Move sun4i-gpadc.h to iio/adc/
> > > arm: config: Enable SUN4I_GPADC in defconfig
> > > dt-bindings: update the Allwinner GPADC device tree binding for H3 &
> > > A83T
> > > iio: adc: sun4i-gpadc-iio: rework: readout temp_data
> > > 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: device specific suspend & resume
> > > 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: h3: use calibration for ths
> > > ARM: dts: sun8i: a83t: add support for the thermal sensor in A83T
> > > ARM: dts: sun8i: a83t: add thermal zone to A83T
> > > ARM: sun8i: a83t: full range OPP tables and CPUfreq
> > >
> > > .../devicetree/bindings/iio/adc/sun4i-gpadc.txt | 41 +-
> > > arch/arm/boot/dts/sun8i-a83t.dtsi | 143 +++++
> > > arch/arm/boot/dts/sun8i-h3.dtsi | 52 ++
> > > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
> > > arch/arm/configs/sunxi_defconfig | 1 +
> > > drivers/iio/adc/Kconfig | 11 +-
> > > drivers/iio/adc/sun4i-gpadc-iio.c | 617 +++++++++++++--------
> > > drivers/mfd/Kconfig | 17 -
> > > drivers/mfd/Makefile | 1 -
> > > drivers/mfd/sun4i-gpadc.c | 181 ------
> > > include/linux/{mfd => iio/adc}/sun4i-gpadc.h | 47 +-
> > > 11 files changed, 681 insertions(+), 440 deletions(-)
> > > delete mode 100644 drivers/mfd/sun4i-gpadc.c
> > > rename include/linux/{mfd => iio/adc}/sun4i-gpadc.h (72%)
> >
> > What became of these patches? I can't seem to find any follow-ups. Was
> > the approach abandoned in favour of something else?
>
> It didn't, I don't think we ever got any follow-up to that series so
> that's the main reason it's been held back.

IIRC it was blocked by a disagreement on how to handle the calibration data
in the SID. Hopefully this is now resolved.

ChenYu