Allwiner H3 and A83T SoCs have a thermal sensor, which is a large refactored
version of the old Allwinner "GPADC" (although it have already only
thermal part left in A33).
This patch tried to add support for the sensor in H3 and A83T based on
the A33 thermal sensor driver by Quentin Schulz, which is already merged.
This Patchseries is based on Icenowy Zhengs v4 patchseries [1].
The first 5 patches are reworked patches from the v4 patchseries.
The rest of the patches add step by step a feature to support multible
sensors, nvmem calibration and interupts. This patchseries should make it
easy also to add other sunxi SoCs, like the H5, A64 and A80.
Patches that adds support for H5, A64 and A80 SoCs are allready prepared,
and will be upstreamed if this patchseries is applied and the testing is done.
I tried to pick up all the feedback from [1]. I hope I didn't miss anything.
Regards,
Philipp
@Jonathan
Could you please check Patch 8 again. I moved some code from
Patch 8 to 9. Please chek it again, if your still ok with it.
But I think it sould be ok.
changes since v1:
* collecting all acks
* rewording commits/fix typos
* move code in place where it is used
* fix naming conventions of defines
* clarify commits
* update documentation to cover the new nvmem calibraion
* change nvmem calibration
Icenowy Zheng (1):
iio: adc: sun4i-gpadc-iio: rename A33-specified registers to contain
A33
Philipp Rossak (15):
dt-bindings: update the Allwinner GPADC device tree binding for H3 &
A83T
arm: config: sunxi_defconfig: enable SUN4I_GPADC
iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg
iio: adc: sun4i-gpadc-iio: rework: support clocks and reset
iio: adc: sun4i-gpadc-iio: rework: support multiple sensors
iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data
iio: adc: sun4i-gpadc-iio: rework: add interrupt support
iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor
arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5
arm: dts: sun8i: h3: add support for the thermal sensor in H3
arm: dts: sun8i: h3: add thermal zone to H3
arm: dts: sun8i: h3: enable H3 sid controller
arm: dts: sun8i: a83t: add support for the thermal sensor in A83T
arm: dts: sun8i: a83t: add thermal zone to A83T
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++-
arch/arm/boot/dts/sun8i-a83t.dtsi | 28 ++
arch/arm/boot/dts/sun8i-h3.dtsi | 21 ++
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +
arch/arm/configs/sunxi_defconfig | 1 +
drivers/iio/adc/sun4i-gpadc-iio.c | 367 ++++++++++++++++++++-
include/linux/mfd/sun4i-gpadc.h | 51 ++-
7 files changed, 503 insertions(+), 24 deletions(-)
--
2.11.0
For adding newer sensor some basic rework of the code is necessary.
The SoCs after H3 has newer thermal sensor ADCs, which have two clock
inputs (bus clock and sampling clock) and a reset. The registers are
also re-arranged.
This commit reworks the code, adds the process of the clocks and
resets.
Signed-off-by: Philipp Rossak <[email protected]>
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index db57d9fffe48..51ec0104d678 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -22,6 +22,7 @@
* shutdown for not being used.
*/
+#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -31,6 +32,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
+#include <linux/reset.h>
#include <linux/thermal.h>
#include <linux/delay.h>
@@ -68,6 +70,9 @@ struct gpadc_data {
unsigned int temp_data;
int (*sample_start)(struct sun4i_gpadc_iio *info);
int (*sample_end)(struct sun4i_gpadc_iio *info);
+ bool has_bus_clk;
+ bool has_bus_rst;
+ bool has_mod_clk;
};
static const struct gpadc_data sun4i_gpadc_data = {
@@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
atomic_t ignore_temp_data_irq;
const struct gpadc_data *data;
bool no_irq;
+ struct clk *bus_clk;
+ struct clk *mod_clk;
+ struct reset_control *reset;
/* prevents concurrent reads of temperature and ADC */
struct mutex mutex;
struct thermal_zone_device *tzd;
@@ -420,6 +428,10 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+ clk_disable(info->mod_clk);
+
+ clk_disable(info->bus_clk);
+
return info->data->sample_end(info);
}
@@ -446,6 +458,10 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+ clk_enable(info->mod_clk);
+
+ clk_enable(info->bus_clk);
+
return info->data->sample_start(info);
}
@@ -560,10 +576,59 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
return ret;
}
+ if (info->data->has_bus_rst) {
+ info->reset = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(info->reset)) {
+ ret = PTR_ERR(info->reset);
+ return ret;
+ }
+
+ ret = reset_control_deassert(info->reset);
+ if (ret)
+ return ret;
+ }
+
+ if (info->data->has_bus_clk) {
+ info->bus_clk = devm_clk_get(&pdev->dev, "bus");
+ if (IS_ERR(info->bus_clk)) {
+ ret = PTR_ERR(info->bus_clk);
+ goto assert_reset;
+ }
+
+ ret = clk_prepare_enable(info->bus_clk);
+ if (ret)
+ goto assert_reset;
+ }
+
+ if (info->data->has_mod_clk) {
+ info->mod_clk = devm_clk_get(&pdev->dev, "mod");
+ if (IS_ERR(info->mod_clk)) {
+ ret = PTR_ERR(info->mod_clk);
+ goto disable_bus_clk;
+ }
+
+ /* Running at 6MHz */
+ ret = clk_set_rate(info->mod_clk, 4000000);
+ if (ret)
+ goto disable_bus_clk;
+
+ ret = clk_prepare_enable(info->mod_clk);
+ if (ret)
+ goto disable_bus_clk;
+ }
+
if (IS_ENABLED(CONFIG_THERMAL_OF))
info->sensor_device = &pdev->dev;
return 0;
+
+disable_bus_clk:
+ clk_disable_unprepare(info->bus_clk);
+
+assert_reset:
+ reset_control_assert(info->reset);
+
+ return ret;
}
static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
@@ -729,6 +794,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
if (!info->no_irq)
iio_map_array_unregister(indio_dev);
+ clk_disable_unprepare(info->mod_clk);
+
+ clk_disable_unprepare(info->bus_clk);
+
+ reset_control_assert(info->reset);
+
return 0;
}
--
2.11.0
This commit enables the SUN4I_GPADC config option and
sets the value to yes. This is needed to enable the ths sensors.
Signed-off-by: Philipp Rossak <[email protected]>
---
arch/arm/configs/sunxi_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 5caaf971fb50..52c3990b9d91 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -131,6 +131,7 @@ CONFIG_DMA_SUN6I=y
CONFIG_EXTCON=y
CONFIG_IIO=y
CONFIG_AXP20X_ADC=y
+CONFIG_SUN4I_GPADC=y
CONFIG_PWM=y
CONFIG_PWM_SUN4I=y
CONFIG_PHY_SUN4I_USB=y
--
2.11.0
For adding newer sensor some basic rework of the code is necessary.
This patch reworks the driver to be able to handle more than one
thermal sensor. Newer SoC like the A80 have 4 thermal sensors.
Because of this the maximal sensor count value was set to 4.
The sensor_id value is set during sensor registration and is for each
registered sensor indiviual. This makes it able to differntiate the
sensors when the value is read from the register.
In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor
was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects
in the temp_read function automatically sensor 0. A check for the
sensor_id is here not required since the old sensors only have one
thermal sensor. In addition to that is the sun4i_gpadc_read_raw()
function only used by the "older" sensors (before A33) where the
thermal sensor was a cobination of an adc and a thermal sensor.
Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
include/linux/mfd/sun4i-gpadc.h | 3 +++
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 51ec0104d678..ac9ad2f8232f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -67,12 +67,13 @@ struct gpadc_data {
unsigned int tp_adc_select;
unsigned int (*adc_chan_select)(unsigned int chan);
unsigned int adc_chan_mask;
- unsigned int temp_data;
+ unsigned int temp_data[MAX_SENSOR_COUNT];
int (*sample_start)(struct sun4i_gpadc_iio *info);
int (*sample_end)(struct sun4i_gpadc_iio *info);
bool has_bus_clk;
bool has_bus_rst;
bool has_mod_clk;
+ int sensor_count;
};
static const struct gpadc_data sun4i_gpadc_data = {
@@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
.adc_chan_select = &sun4i_gpadc_chan_select,
.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
- .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
+ .sensor_count = 1,
};
static const struct gpadc_data sun5i_gpadc_data = {
@@ -94,9 +96,10 @@ static const struct gpadc_data sun5i_gpadc_data = {
.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
.adc_chan_select = &sun4i_gpadc_chan_select,
.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
- .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
+ .sensor_count = 1,
};
static const struct gpadc_data sun6i_gpadc_data = {
@@ -106,18 +109,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
.adc_chan_select = &sun6i_gpadc_chan_select,
.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
- .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
+ .sensor_count = 1,
};
static const struct gpadc_data sun8i_a33_gpadc_data = {
.temp_offset = -1662,
.temp_scale = 162,
.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
- .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
+ .sensor_count = 1,
};
struct sun4i_gpadc_iio {
@@ -135,6 +140,7 @@ struct sun4i_gpadc_iio {
struct clk *bus_clk;
struct clk *mod_clk;
struct reset_control *reset;
+ int sensor_id;
/* prevents concurrent reads of temperature and ADC */
struct mutex mutex;
struct thermal_zone_device *tzd;
@@ -302,14 +308,15 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
}
-static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
+static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
+ int sensor)
{
struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
if (info->no_irq) {
pm_runtime_get_sync(indio_dev->dev.parent);
- regmap_read(info->regmap, info->data->temp_data, val);
+ regmap_read(info->regmap, info->data->temp_data[sensor], val);
pm_runtime_mark_last_busy(indio_dev->dev.parent);
pm_runtime_put_autosuspend(indio_dev->dev.parent);
@@ -356,7 +363,7 @@ static int sun4i_gpadc_read_raw(struct iio_dev *indio_dev,
ret = sun4i_gpadc_adc_read(indio_dev, chan->channel,
val);
else
- ret = sun4i_gpadc_temp_read(indio_dev, val);
+ ret = sun4i_gpadc_temp_read(indio_dev, val, 0);
if (ret)
return ret;
@@ -470,7 +477,7 @@ static int sun4i_gpadc_get_temp(void *data, int *temp)
struct sun4i_gpadc_iio *info = data;
int val, scale, offset;
- if (sun4i_gpadc_temp_read(info->indio_dev, &val))
+ if (sun4i_gpadc_temp_read(info->indio_dev, &val, info->sensor_id))
return -ETIMEDOUT;
sun4i_gpadc_temp_scale(info->indio_dev, &scale);
@@ -712,7 +719,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
{
struct sun4i_gpadc_iio *info;
struct iio_dev *indio_dev;
- int ret;
+ int ret, i;
indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
if (!indio_dev)
@@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
pm_runtime_enable(&pdev->dev);
if (IS_ENABLED(CONFIG_THERMAL_OF)) {
- info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
- 0, info,
- &sun4i_ts_tz_ops);
+ for (i = 0; i < info->data->sensor_count; i++) {
+ info->sensor_id = i;
+ info->tzd = thermal_zone_of_sensor_register(
+ info->sensor_device,
+ i, info, &sun4i_ts_tz_ops);
+ }
/*
* Do not fail driver probing when failing to register in
* thermal because no thermal DT node is found.
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 78d31984a222..20fa02040007 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -90,6 +90,9 @@
/* 10s delay before suspending the IP */
#define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
+/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define MAX_SENSOR_COUNT 4
+
struct sun4i_gpadc_dev {
struct device *dev;
struct regmap *regmap;
--
2.11.0
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 8495deecedad..fbb007e5798e 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -128,3 +128,8 @@
&pio {
compatible = "allwinner,sun8i-h3-pinctrl";
};
+
+&ths {
+ compatible = "allwinner,sun8i-h3-ths";
+ #thermal-sensor-cells = <0>;
+};
--
2.11.0
This patch rewors the driver to support interrupts for the thermal part
of the sensor.
This is only available for the newer sensor (currently H3 and A83T).
The interrupt will be trigerd on data available and triggers the update
for the thermal sensors. All newer sensors have different amount of
sensors and different interrupts for each device the reset of the
interrupts need to be done different
For the newer sensors is the autosuspend disabled.
Signed-off-by: Philipp Rossak <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
include/linux/mfd/sun4i-gpadc.h | 2 ++
2 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 74eeb5cd5218..b7b5451226b0 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -71,11 +71,14 @@ struct gpadc_data {
unsigned int temp_data[MAX_SENSOR_COUNT];
int (*sample_start)(struct sun4i_gpadc_iio *info);
int (*sample_end)(struct sun4i_gpadc_iio *info);
+ u32 irq_clear_map;
+ u32 irq_control_map;
bool has_bus_clk;
bool has_bus_rst;
bool has_mod_clk;
int sensor_count;
bool supports_nvmem;
+ bool support_irq;
};
static const struct gpadc_data sun4i_gpadc_data = {
@@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};
static const struct gpadc_data sun5i_gpadc_data = {
@@ -104,6 +108,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};
static const struct gpadc_data sun6i_gpadc_data = {
@@ -118,6 +123,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};
static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -129,6 +135,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
.supports_nvmem = false,
+ .support_irq = false,
};
struct sun4i_gpadc_iio {
@@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
return 0;
}
+ if (info->data->support_irq) {
+ regmap_read(info->regmap, info->data->temp_data[sensor], val);
+ return 0;
+ }
+
return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
}
@@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static irqreturn_t sunxi_irq_thread(int irq, void *data)
+{
+ struct sun4i_gpadc_iio *info = data;
+
+ regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
+
+ thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE);
+
+ return IRQ_HANDLED;
+}
+
static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
{
/* Disable the ADC on IP */
@@ -572,12 +595,29 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
struct nvmem_cell *cell;
ssize_t cell_size;
u64 *cell_data;
+ int irq;
info->data = of_device_get_match_data(&pdev->dev);
if (!info->data)
return -ENODEV;
- info->no_irq = true;
+ if (info->data->support_irq) {
+ /* only the new versions of ths support right now irqs */
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+ return irq;
+ }
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ sunxi_irq_thread, IRQF_ONESHOT,
+ dev_name(&pdev->dev), info);
+ if (ret)
+ return ret;
+
+ } else
+ info->no_irq = true;
+
indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
indio_dev->channels = sun8i_a33_gpadc_channels;
@@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
if (ret)
return ret;
- pm_runtime_set_autosuspend_delay(&pdev->dev,
- SUN4I_GPADC_AUTOSUSPEND_DELAY);
- pm_runtime_use_autosuspend(&pdev->dev);
- pm_runtime_set_suspended(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
+ if (!info->data->support_irq) {
+ pm_runtime_set_autosuspend_delay(&pdev->dev,
+ SUN4I_GPADC_AUTOSUSPEND_DELAY);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ }
if (IS_ENABLED(CONFIG_THERMAL_OF)) {
for (i = 0; i < info->data->sensor_count; i++) {
@@ -814,6 +856,9 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
}
}
+ if (info->data->support_irq)
+ info->data->sample_start(info);
+
ret = devm_iio_device_register(&pdev->dev, indio_dev);
if (ret < 0) {
dev_err(&pdev->dev, "could not register the device\n");
@@ -843,6 +888,9 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
if (!IS_ENABLED(CONFIG_THERMAL_OF))
return 0;
+ if (info->data->support_irq)
+ info->data->sample_end(info);
+
thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);
if (!info->no_irq)
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 20fa02040007..458f2159a95a 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -91,6 +91,8 @@
#define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_INTC 0x44
+
#define MAX_SENSOR_COUNT 4
struct sun4i_gpadc_dev {
--
2.11.0
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 7f4955a5fab7..9e53ff5ac4ed 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -518,6 +518,14 @@
clocks = <&osc24M>;
};
+ ths: thermal-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
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 7a83b15225c7..413c789b588d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -426,6 +426,15 @@
};
};
+ ths: thermal-sensor@1c25000 {
+ reg = <0x01c25000 0x100>;
+ interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+ clock-names = "bus", "mod";
+ resets = <&ccu RST_BUS_THS>;
+ #io-channel-cells = <0>;
+ };
+
timer@1c20c00 {
compatible = "allwinner,sun4i-a10-timer";
reg = <0x01c20c00 0xa0>;
--
2.11.0
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 | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 9e53ff5ac4ed..4259a8726031 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -747,4 +747,24 @@
#size-cells = <0>;
};
};
+
+ thermal-zones {
+ cpu0_thermal: cpu0-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 0>;
+ };
+
+ cpu1_thermal: cpu1-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 1>;
+ };
+
+ gpu_thermal: gpu-thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 2>;
+ };
+ };
};
--
2.11.0
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 | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 8196203d65fe..9f7895ba1966 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
SUN8I_H3_THS_TEMP_PERIOD(0x7),
};
+static const struct gpadc_data sun8i_a83t_ths_data = {
+ .temp_offset = -2724,
+ .temp_scale = -70,
+ .temp_data = {SUN8I_H3_THS_TDATA0,
+ SUN8I_A83T_THS_TDATA1,
+ SUN8I_A83T_THS_TDATA2,
+ 0},
+ .sample_start = sunxi_ths_sample_start,
+ .sample_end = sunxi_ths_sample_end,
+ .sensor_count = 3,
+ .supports_nvmem = false,
+ .support_irq = true,
+ .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
+ .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
+ .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN1 |
+ SUN8I_A83T_THS_TEMP_SENSE_EN2,
+ .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+ .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_1 |
+ SUN8I_A83T_THS_INTS_ALARM_INT_2 |
+ SUN8I_H3_THS_INTS_SHUT_INT_0 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_1 |
+ SUN8I_A83T_THS_INTS_SHUT_INT_2 |
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+ SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
+ .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+ SUN8I_A83T_THS_INTC_TDATA_IRQ_EN1 |
+ SUN8I_A83T_THS_INTC_TDATA_IRQ_EN2 |
+ SUN8I_H3_THS_TEMP_PERIOD(0x257),
+};
+
struct sun4i_gpadc_iio {
struct iio_dev *indio_dev;
struct completion completion;
@@ -668,6 +702,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
.compatible = "allwinner,sun8i-h3-ths",
.data = &sun8i_h3_ths_data,
},
+ {
+ .compatible = "allwinner,sun8i-a83t-ths",
+ .data = &sun8i_a83t_ths_data,
+ },
{ /* sentinel */ }
};
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 80b79c31cea3..32f15cc03363 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -99,21 +99,39 @@
#define SUNXI_THS_CDATA_0_1 0x74
#define SUNXI_THS_CDATA_2_3 0x78
#define SUN8I_H3_THS_TDATA0 0x80
+#define SUN8I_A83T_THS_TDATA1 0x84
+#define SUN8I_A83T_THS_TDATA2 0x88
#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))
#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN1 BIT(1)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN2 BIT(2)
#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
#define SUN8I_H3_THS_INTS_ALARM_INT_0 BIT(0)
+#define SUN8I_A83T_THS_INTS_ALARM_INT_1 BIT(1)
+#define SUN8I_A83T_THS_INTS_ALARM_INT_2 BIT(2)
#define SUN8I_H3_THS_INTS_SHUT_INT_0 BIT(4)
+#define SUN8I_A83T_THS_INTS_SHUT_INT_1 BIT(5)
+#define SUN8I_A83T_THS_INTS_SHUT_INT_2 BIT(6)
#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
+#define SUN8I_A83T_THS_INTS_TDATA_IRQ_1 BIT(9)
+#define SUN8I_A83T_THS_INTS_TDATA_IRQ_2 BIT(10)
#define SUN8I_H3_THS_INTS_ALARM_OFF_0 BIT(12)
+#define SUN8I_A83T_THS_INTS_ALARM_OFF_1 BIT(13)
+#define SUN8I_A83T_THS_INTS_ALARM_OFF_2 BIT(14)
#define SUN8I_H3_THS_INTC_ALARM_INT_EN0 BIT(0)
+#define SUN8I_A83T_THS_INTC_ALARM_INT_EN1 BIT(1)
+#define SUN8I_A83T_THS_INTC_ALARM_INT_EN2 BIT(2)
#define SUN8I_H3_THS_INTC_SHUT_INT_EN0 BIT(4)
+#define SUN8I_A83T_THS_INTC_SHUT_INT_EN1 BIT(5)
+#define SUN8I_A83T_THS_INTC_SHUT_INT_EN2 BIT(6)
#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
+#define SUN8I_A83T_THS_INTC_TDATA_IRQ_EN1 BIT(9)
+#define SUN8I_A83T_THS_INTC_TDATA_IRQ_EN2 BIT(10)
#define MAX_SENSOR_COUNT 4
--
2.11.0
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 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
};
};
+ soc {
+ sid: eeprom@1c14000 {
+ compatible = "allwinner,sun8i-h3-sid";
+ reg = <0x01c14000 0x400>;
+ };
+ };
+
thermal-zones {
cpu-thermal {
/* milliseconds */
--
2.11.0
This patch adds support for the H3 ths sensor.
The H3 supports interrupts. The interrupt is configured to update the
the sensor values every second. The calibration data is writen at the
begin of the init process.
Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
2 files changed, 108 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index b7b5451226b0..8196203d65fe 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
+
struct gpadc_data {
int temp_offset;
int temp_scale;
@@ -71,6 +74,10 @@ struct gpadc_data {
unsigned int temp_data[MAX_SENSOR_COUNT];
int (*sample_start)(struct sun4i_gpadc_iio *info);
int (*sample_end)(struct sun4i_gpadc_iio *info);
+ u32 ctrl0_map;
+ u32 ctrl2_map;
+ u32 sensor_en_map;
+ u32 filter_map;
u32 irq_clear_map;
u32 irq_control_map;
bool has_bus_clk;
@@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.support_irq = false,
};
+static const struct gpadc_data sun8i_h3_ths_data = {
+ .temp_offset = -1791,
+ .temp_scale = -121,
+ .temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
+ .sample_start = sunxi_ths_sample_start,
+ .sample_end = sunxi_ths_sample_end,
+ .has_bus_clk = true,
+ .has_bus_rst = true,
+ .has_mod_clk = true,
+ .sensor_count = 1,
+ .supports_nvmem = true,
+ .support_irq = true,
+ .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
+ .ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
+ .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
+ .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+ SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+ .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+ SUN8I_H3_THS_INTS_SHUT_INT_0 |
+ SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+ SUN8I_H3_THS_INTS_ALARM_OFF_0,
+ .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+ SUN8I_H3_THS_TEMP_PERIOD(0x7),
+};
+
struct sun4i_gpadc_iio {
struct iio_dev *indio_dev;
struct completion completion;
@@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
return 0;
}
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
+{
+ /* Disable ths interrupt */
+ regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
+ /* Disable temperature sensor */
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
+
+ return 0;
+}
+
static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
@@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
return info->data->sample_end(info);
}
+static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
+{
+ if (info->has_calibration_data[0])
+ regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
+ info->calibration_data[0]);
+
+ if (info->has_calibration_data[1])
+ regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
+ info->calibration_data[1]);
+}
+
static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
{
/* clkin = 6MHz */
@@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
return 0;
}
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
+{
+ u32 value;
+ sunxi_calibrate(info);
+
+ if (info->data->ctrl0_map)
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
+ info->data->ctrl0_map);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+ info->data->ctrl2_map);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+ info->data->irq_clear_map);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
+ info->data->filter_map);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_INTC,
+ info->data->irq_control_map);
+
+ regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
+
+ regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+ info->data->sensor_en_map | value);
+
+ return 0;
+}
+
static int sun4i_gpadc_runtime_resume(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
@@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
.compatible = "allwinner,sun8i-a33-ths",
.data = &sun8i_a33_gpadc_data,
},
+ {
+ .compatible = "allwinner,sun8i-h3-ths",
+ .data = &sun8i_h3_ths_data,
+ },
{ /* sentinel */ }
};
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 458f2159a95a..80b79c31cea3 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -91,7 +91,29 @@
#define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_CTRL0 0x00
+#define SUN8I_H3_THS_CTRL2 0x40
#define SUN8I_H3_THS_INTC 0x44
+#define SUN8I_H3_THS_STAT 0x48
+#define SUN8I_H3_THS_FILTER 0x70
+#define SUNXI_THS_CDATA_0_1 0x74
+#define SUNXI_THS_CDATA_2_3 0x78
+#define SUN8I_H3_THS_TDATA0 0x80
+
+#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))
+
+#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
+
+#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
+
+#define SUN8I_H3_THS_INTS_ALARM_INT_0 BIT(0)
+#define SUN8I_H3_THS_INTS_SHUT_INT_0 BIT(4)
+#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
+#define SUN8I_H3_THS_INTS_ALARM_OFF_0 BIT(12)
+
+#define SUN8I_H3_THS_INTC_ALARM_INT_EN0 BIT(0)
+#define SUN8I_H3_THS_INTC_SHUT_INT_EN0 BIT(4)
+#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
#define MAX_SENSOR_COUNT 4
--
2.11.0
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 | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fbb007e5798e..3f83f6a27c74 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,15 @@
};
};
+ thermal-zones {
+ cpu-thermal {
+ /* milliseconds */
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths 0>;
+ };
+ };
+
timer {
compatible = "arm,armv7-timer";
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
--
2.11.0
For adding newer sensor some basic rework of the code is necessary.
This commit reworks the code and allows the sampling start/end code and
the position of value readout register to be altered. Later the start/end
functions will be used to configure the ths and start/stop the
sampling.
Signed-off-by: Icenowy Zheng <[email protected]>
Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 03804ff9c006..db57d9fffe48 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
}
+struct sun4i_gpadc_iio;
+
+/*
+ * Prototypes for these functions, which enable these functions to be
+ * referenced in gpadc_data structures.
+ */
+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
+
struct gpadc_data {
int temp_offset;
int temp_scale;
@@ -56,6 +65,9 @@ struct gpadc_data {
unsigned int tp_adc_select;
unsigned int (*adc_chan_select)(unsigned int chan);
unsigned int adc_chan_mask;
+ unsigned int temp_data;
+ int (*sample_start)(struct sun4i_gpadc_iio *info);
+ int (*sample_end)(struct sun4i_gpadc_iio *info);
};
static const struct gpadc_data sun4i_gpadc_data = {
@@ -65,6 +77,9 @@ static const struct gpadc_data sun4i_gpadc_data = {
.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
.adc_chan_select = &sun4i_gpadc_chan_select,
.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
+ .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};
static const struct gpadc_data sun5i_gpadc_data = {
@@ -74,6 +89,9 @@ static const struct gpadc_data sun5i_gpadc_data = {
.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
.adc_chan_select = &sun4i_gpadc_chan_select,
.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
+ .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};
static const struct gpadc_data sun6i_gpadc_data = {
@@ -83,12 +101,18 @@ static const struct gpadc_data sun6i_gpadc_data = {
.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
.adc_chan_select = &sun6i_gpadc_chan_select,
.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
+ .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};
static const struct gpadc_data sun8i_a33_gpadc_data = {
.temp_offset = -1662,
.temp_scale = 162,
.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
+ .temp_data = SUN4I_GPADC_TEMP_DATA,
+ .sample_start = sun4i_gpadc_sample_start,
+ .sample_end = sun4i_gpadc_sample_end,
};
struct sun4i_gpadc_iio {
@@ -277,7 +301,7 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
if (info->no_irq) {
pm_runtime_get_sync(indio_dev->dev.parent);
- regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
+ regmap_read(info->regmap, info->data->temp_data, val);
pm_runtime_mark_last_busy(indio_dev->dev.parent);
pm_runtime_put_autosuspend(indio_dev->dev.parent);
@@ -382,10 +406,8 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static int sun4i_gpadc_runtime_suspend(struct device *dev)
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
{
- struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
-
/* Disable the ADC on IP */
regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
/* Disable temperature sensor on IP */
@@ -394,10 +416,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
return 0;
}
-static int sun4i_gpadc_runtime_resume(struct device *dev)
+static int sun4i_gpadc_runtime_suspend(struct device *dev)
{
struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+ return info->data->sample_end(info);
+}
+
+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
+{
/* clkin = 6MHz */
regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
@@ -415,6 +442,13 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
return 0;
}
+static int sun4i_gpadc_runtime_resume(struct device *dev)
+{
+ struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+
+ return info->data->sample_start(info);
+}
+
static int sun4i_gpadc_get_temp(void *data, int *temp)
{
struct sun4i_gpadc_iio *info = data;
--
2.11.0
Allwinner H3 features a thermal sensor like the one in A33, but has its
register re-arranged, the clock divider moved to CCU (originally the
clock divider is in ADC) and added a pair of bus clock and reset.
Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
the bus clock and the reset was removed from the CCU. The THS in A83T
has a clock that is directly connected and runs with 24 MHz.
Update the binding document to cover H3 and A83T.
Signed-off-by: Philipp Rossak <[email protected]>
---
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index 86dd8191b04c..22df0c5c23d4 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
and sometimes as a touchscreen controller.
Required properties:
- - compatible: "allwinner,sun8i-a33-ths",
+ - compatible: must contain one of the following compatibles:
+ - "allwinner,sun8i-a33-ths"
+ - "allwinner,sun8i-h3-ths"
+ - "allwinner,sun8i-a83t-ths"
- reg: mmio address range of the chip,
- - #thermal-sensor-cells: shall be 0,
+ - #thermal-sensor-cells: shall be 0 or 1,
- #io-channel-cells: shall be 0,
-Example:
+Required properties for the following compatibles:
+ - "allwinner,sun8i-h3-ths"
+ - "allwinner,sun8i-a83t-ths"
+ - interrupts: the sampling interrupt of the ADC,
+
+Required properties for the following compatibles:
+ - "allwinner,sun8i-h3-ths"
+ - clocks: the bus clock and the input clock of the ADC,
+ - clock-names: should be "bus" and "mod",
+ - resets: the bus reset of the ADC,
+
+Optional properties for the following compatibles:
+ - "allwinner,sun8i-h3-ths"
+ - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
+ If unspecified default values shall be used. The size should
+ be 0x2 * sensorcount.
+ - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
+Example for A33:
ths: ths@1c25000 {
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
@@ -17,6 +40,27 @@ 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>;
+ };
+
+Example for A83T:
+ 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>;
+ };
+
sun4i, sun5i and sun6i SoCs are also supported via the older binding:
sun4i resistive touchscreen controller
--
2.11.0
This patch reworks the driver to support nvmem calibration cells.
The driver checks if the nvmem calibration is supported and reads out
the nvmem.
Signed-off-by: Philipp Rossak <[email protected]>
---
drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index ac9ad2f8232f..74eeb5cd5218 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
@@ -74,6 +75,7 @@ struct gpadc_data {
bool has_bus_rst;
bool has_mod_clk;
int sensor_count;
+ bool supports_nvmem;
};
static const struct gpadc_data sun4i_gpadc_data = {
@@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
+ .supports_nvmem = false,
};
static const struct gpadc_data sun5i_gpadc_data = {
@@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
+ .supports_nvmem = false,
};
static const struct gpadc_data sun6i_gpadc_data = {
@@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
+ .supports_nvmem = false,
};
static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
.sample_start = sun4i_gpadc_sample_start,
.sample_end = sun4i_gpadc_sample_end,
.sensor_count = 1,
+ .supports_nvmem = false,
};
struct sun4i_gpadc_iio {
@@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
struct clk *mod_clk;
struct reset_control *reset;
int sensor_id;
+ u32 calibration_data[2];
+ bool has_calibration_data[2];
/* prevents concurrent reads of temperature and ADC */
struct mutex mutex;
struct thermal_zone_device *tzd;
@@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
struct resource *mem;
void __iomem *base;
int ret;
+ struct nvmem_cell *cell;
+ ssize_t cell_size;
+ u64 *cell_data;
info->data = of_device_get_match_data(&pdev->dev);
if (!info->data)
@@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
if (IS_ERR(base))
return PTR_ERR(base);
+ info->has_calibration_data[0] = false;
+ info->has_calibration_data[1] = false;
+
+ if (!info->data->supports_nvmem)
+ goto no_nvmem;
+
+ cell = nvmem_cell_get(&pdev->dev, "calibration");
+ if (IS_ERR(cell)) {
+ if (PTR_ERR(cell) == -EPROBE_DEFER)
+ return PTR_ERR(cell);
+ goto no_nvmem;
+ }
+
+ cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
+ nvmem_cell_put(cell);
+ switch (cell_size) {
+ case 8:
+ case 6:
+ info->has_calibration_data[1] = true;
+ info->calibration_data[1] = be32_to_cpu(
+ upper_32_bits(cell_data[0]));
+ case 4:
+ case 2:
+ info->has_calibration_data[0] = true;
+ info->calibration_data[0] = be32_to_cpu(
+ lower_32_bits(cell_data[0]));
+ break;
+ default:
+ break;
+ }
+
+no_nvmem:
+
info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
&sun4i_gpadc_regmap_config);
if (IS_ERR(info->regmap)) {
--
2.11.0
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/mfd/sun4i-gpadc.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 04d7147e0110..03804ff9c006 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -88,7 +88,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
static const struct gpadc_data sun8i_a33_gpadc_data = {
.temp_offset = -1662,
.temp_scale = 162,
- .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
+ .tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
};
struct sun4i_gpadc_iio {
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 139872c2e0fe..78d31984a222 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -38,9 +38,9 @@
#define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x) (GENMASK(3, 0) & BIT(x))
#define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK GENMASK(3, 0)
-/* TP_CTRL1 bits for sun8i SoCs */
-#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
-#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
+/* TP_CTRL1 bits for A33 */
+#define SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
+#define SUN8I_A33_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
#define SUN4I_GPADC_CTRL2 0x08
--
2.11.0
Hi Philipp,
On Mon, Jan 29, 2018 at 12:29:04AM +0100, 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]>
Thanks a lot for tackling this.
> ---
> .../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> index 86dd8191b04c..22df0c5c23d4 100644
> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
> and sometimes as a touchscreen controller.
>
> Required properties:
> - - compatible: "allwinner,sun8i-a33-ths",
> + - compatible: must contain one of the following compatibles:
> + - "allwinner,sun8i-a33-ths"
> + - "allwinner,sun8i-h3-ths"
> + - "allwinner,sun8i-a83t-ths"
> - reg: mmio address range of the chip,
> - - #thermal-sensor-cells: shall be 0,
> + - #thermal-sensor-cells: shall be 0 or 1,
> - #io-channel-cells: shall be 0,
>
> -Example:
> +Required properties for the following compatibles:
> + - "allwinner,sun8i-h3-ths"
> + - "allwinner,sun8i-a83t-ths"
> + - interrupts: the sampling interrupt of the ADC,
> +
> +Required properties for the following compatibles:
> + - "allwinner,sun8i-h3-ths"
> + - clocks: the bus clock and the input clock of the ADC,
> + - clock-names: should be "bus" and "mod",
> + - resets: the bus reset of the ADC,
> +
> +Optional properties for the following compatibles:
> + - "allwinner,sun8i-h3-ths"
> + - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
> + If unspecified default values shall be used. The size should
> + be 0x2 * sensorcount.
> + - nvmem-cell-names: Should be "calibration".
> +
> +Details see: bindings/nvmem/nvmem.txt
> +
> +Example for A33:
> ths: ths@1c25000 {
> compatible = "allwinner,sun8i-a33-ths";
> reg = <0x01c25000 0x100>;
> @@ -17,6 +40,27 @@ 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>;
> + };
> +
> +Example for A83T:
> + 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>;
> + };
> +
I'm wondering if this is actually needed. We've used this convoluted
constructs to be compatible with the old driver, but I'm not sure this
is actually worth it now, and this is causing several issues, among
which:
- We need to have a bunch of quirks to handle all the DT cases.
- We need to have an MFD, which isn't really optimal
So I'd rather introduce a new compatible for the old SoCs, keep the
old driver around, and simplify a lot that driver code that will ease
further developments. And we can also get rid of the MFD in the
process. I discussed it with Quentin, and he was ok with it, what do
you think?
(and that would involve creating a new file for the bindings you
introduce here).
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi,
On Mon, Jan 29, 2018 at 12:29:05AM +0100, Philipp Rossak wrote:
> This commit enables the SUN4I_GPADC config option and
> sets the value to yes. This is needed to enable the ths sensors.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> arch/arm/configs/sunxi_defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
> index 5caaf971fb50..52c3990b9d91 100644
> --- a/arch/arm/configs/sunxi_defconfig
> +++ b/arch/arm/configs/sunxi_defconfig
> @@ -131,6 +131,7 @@ CONFIG_DMA_SUN6I=y
> CONFIG_EXTCON=y
> CONFIG_IIO=y
> CONFIG_AXP20X_ADC=y
> +CONFIG_SUN4I_GPADC=y
Unfortunately, we can't do that since TOUCHSCREEN_SUN4I is already
enabled and we have a depends on !TOUCHSCREEN_SUN4I.
One more reason to stop worrying about replacing that driver DT
binding.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi,
On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
>
> This commit reworks the code and allows the sampling start/end code and
> the position of value readout register to be altered. Later the start/end
> functions will be used to configure the ths and start/stop the
> sampling.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> Signed-off-by: Philipp Rossak <[email protected]>
That signed-off-by chain doesn't really make much sense. If Icenowy is
the author, she should be reported as such in the commit, and if
you're the author, you shouldn't have her Signed-off-by.
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 03804ff9c006..db57d9fffe48 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
> return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
> }
>
> +struct sun4i_gpadc_iio;
> +
> +/*
> + * Prototypes for these functions, which enable these functions to be
> + * referenced in gpadc_data structures.
> + */
> +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
> +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
> +
> struct gpadc_data {
> int temp_offset;
> int temp_scale;
> @@ -56,6 +65,9 @@ struct gpadc_data {
> unsigned int tp_adc_select;
> unsigned int (*adc_chan_select)(unsigned int chan);
> unsigned int adc_chan_mask;
> + unsigned int temp_data;
> + int (*sample_start)(struct sun4i_gpadc_iio *info);
> + int (*sample_end)(struct sun4i_gpadc_iio *info);
> };
>
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -65,6 +77,9 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
> .adc_chan_select = &sun4i_gpadc_chan_select,
> .adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
> + .temp_data = SUN4I_GPADC_TEMP_DATA,
You can use a regmap_field there.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 12:29:08AM +0100, 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]>
> Signed-off-by: Icenowy Zheng <[email protected]>
Same remark for the SoB
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index db57d9fffe48..51ec0104d678 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -22,6 +22,7 @@
> * shutdown for not being used.
> */
>
> +#include <linux/clk.h>
> #include <linux/completion.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> @@ -31,6 +32,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> +#include <linux/reset.h>
> #include <linux/thermal.h>
> #include <linux/delay.h>
>
> @@ -68,6 +70,9 @@ struct gpadc_data {
> unsigned int temp_data;
> int (*sample_start)(struct sun4i_gpadc_iio *info);
> int (*sample_end)(struct sun4i_gpadc_iio *info);
> + bool has_bus_clk;
> + bool has_bus_rst;
> + bool has_mod_clk;
Is there SoCs where this insn't all true, or all false?
> };
>
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
> atomic_t ignore_temp_data_irq;
> const struct gpadc_data *data;
> bool no_irq;
> + struct clk *bus_clk;
> + struct clk *mod_clk;
> + struct reset_control *reset;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> struct thermal_zone_device *tzd;
> @@ -420,6 +428,10 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>
> + clk_disable(info->mod_clk);
> +
Why clk_disable?
> + clk_disable(info->bus_clk);
> +
You can tie the bus_clk to the regmap directly, instead of having to
maintain it yourself here.
And you can probably put the device in reset and out of reset here as
well.
> return info->data->sample_end(info);
> }
>
> @@ -446,6 +458,10 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>
> + clk_enable(info->mod_clk);
> +
> + clk_enable(info->bus_clk);
> +
> return info->data->sample_start(info);
> }
>
> @@ -560,10 +576,59 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> return ret;
> }
>
> + if (info->data->has_bus_rst) {
> + info->reset = devm_reset_control_get(&pdev->dev, NULL);
> + if (IS_ERR(info->reset)) {
> + ret = PTR_ERR(info->reset);
> + return ret;
> + }
> +
> + ret = reset_control_deassert(info->reset);
> + if (ret)
> + return ret;
> + }
> +
> + if (info->data->has_bus_clk) {
> + info->bus_clk = devm_clk_get(&pdev->dev, "bus");
> + if (IS_ERR(info->bus_clk)) {
> + ret = PTR_ERR(info->bus_clk);
> + goto assert_reset;
> + }
> +
> + ret = clk_prepare_enable(info->bus_clk);
> + if (ret)
> + goto assert_reset;
> + }
> +
> + if (info->data->has_mod_clk) {
> + info->mod_clk = devm_clk_get(&pdev->dev, "mod");
> + if (IS_ERR(info->mod_clk)) {
> + ret = PTR_ERR(info->mod_clk);
> + goto disable_bus_clk;
> + }
> +
> + /* Running at 6MHz */
> + ret = clk_set_rate(info->mod_clk, 4000000);
Your comment and the line below doesn't really make much sense :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi,
On Mon, Jan 29, 2018 at 12:29:09AM +0100, 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 | 36 +++++++++++++++++++++++-------------
> include/linux/mfd/sun4i-gpadc.h | 3 +++
> 2 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 51ec0104d678..ac9ad2f8232f 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -67,12 +67,13 @@ struct gpadc_data {
> unsigned int tp_adc_select;
> unsigned int (*adc_chan_select)(unsigned int chan);
> unsigned int adc_chan_mask;
> - unsigned int temp_data;
> + unsigned int temp_data[MAX_SENSOR_COUNT];
> int (*sample_start)(struct sun4i_gpadc_iio *info);
> int (*sample_end)(struct sun4i_gpadc_iio *info);
> bool has_bus_clk;
> bool has_bus_rst;
> bool has_mod_clk;
> + int sensor_count;
> };
>
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
> .adc_chan_select = &sun4i_gpadc_chan_select,
> .adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
> - .temp_data = SUN4I_GPADC_TEMP_DATA,
> + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun5i_gpadc_data = {
> @@ -94,9 +96,10 @@ static const struct gpadc_data sun5i_gpadc_data = {
> .tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
> .adc_chan_select = &sun4i_gpadc_chan_select,
> .adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
> - .temp_data = SUN4I_GPADC_TEMP_DATA,
> + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun6i_gpadc_data = {
> @@ -106,18 +109,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
> .adc_chan_select = &sun6i_gpadc_chan_select,
> .adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
> - .temp_data = SUN4I_GPADC_TEMP_DATA,
> + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> + .sensor_count = 1,
> };
>
> static const struct gpadc_data sun8i_a33_gpadc_data = {
> .temp_offset = -1662,
> .temp_scale = 162,
> .tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
> - .temp_data = SUN4I_GPADC_TEMP_DATA,
> + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> + .sensor_count = 1,
> };
>
> struct sun4i_gpadc_iio {
> @@ -135,6 +140,7 @@ struct sun4i_gpadc_iio {
> struct clk *bus_clk;
> struct clk *mod_clk;
> struct reset_control *reset;
> + int sensor_id;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> struct thermal_zone_device *tzd;
> @@ -302,14 +308,15 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
> return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
> }
>
> -static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> +static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
> + int sensor)
> {
> struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> if (info->no_irq) {
> pm_runtime_get_sync(indio_dev->dev.parent);
>
> - regmap_read(info->regmap, info->data->temp_data, val);
> + regmap_read(info->regmap, info->data->temp_data[sensor], val);
>
> pm_runtime_mark_last_busy(indio_dev->dev.parent);
> pm_runtime_put_autosuspend(indio_dev->dev.parent);
> @@ -356,7 +363,7 @@ static int sun4i_gpadc_read_raw(struct iio_dev *indio_dev,
> ret = sun4i_gpadc_adc_read(indio_dev, chan->channel,
> val);
> else
> - ret = sun4i_gpadc_temp_read(indio_dev, val);
> + ret = sun4i_gpadc_temp_read(indio_dev, val, 0);
>
> if (ret)
> return ret;
> @@ -470,7 +477,7 @@ static int sun4i_gpadc_get_temp(void *data, int *temp)
> struct sun4i_gpadc_iio *info = data;
> int val, scale, offset;
>
> - if (sun4i_gpadc_temp_read(info->indio_dev, &val))
> + if (sun4i_gpadc_temp_read(info->indio_dev, &val, info->sensor_id))
> return -ETIMEDOUT;
>
> sun4i_gpadc_temp_scale(info->indio_dev, &scale);
> @@ -712,7 +719,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> {
> struct sun4i_gpadc_iio *info;
> struct iio_dev *indio_dev;
> - int ret;
> + int ret, i;
>
> indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> if (!indio_dev)
> @@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> pm_runtime_enable(&pdev->dev);
>
> if (IS_ENABLED(CONFIG_THERMAL_OF)) {
> - info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
> - 0, info,
> - &sun4i_ts_tz_ops);
> + for (i = 0; i < info->data->sensor_count; i++) {
> + info->sensor_id = i;
> + info->tzd = thermal_zone_of_sensor_register(
> + info->sensor_device,
> + i, info, &sun4i_ts_tz_ops);
> + }
I'm not sure how that works. Isn't the info structure shared between
all the sensors? The sensor_id value would be always set to the last
sensor then.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 12:29:10AM +0100, 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 | 44 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index ac9ad2f8232f..74eeb5cd5218 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -27,6 +27,7 @@
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> @@ -74,6 +75,7 @@ struct gpadc_data {
> bool has_bus_rst;
> bool has_mod_clk;
> int sensor_count;
> + bool supports_nvmem;
I think you should add some documentation along with all the fields
you're adding.
> };
>
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> .sensor_count = 1,
> + .supports_nvmem = false,
That's already its value if you leave it out.
> };
>
> static const struct gpadc_data sun5i_gpadc_data = {
> @@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> .sensor_count = 1,
> + .supports_nvmem = false,
> };
>
> static const struct gpadc_data sun6i_gpadc_data = {
> @@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> .sensor_count = 1,
> + .supports_nvmem = false,
> };
>
> static const struct gpadc_data sun8i_a33_gpadc_data = {
> @@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> .sensor_count = 1,
> + .supports_nvmem = false,
> };
>
> struct sun4i_gpadc_iio {
> @@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
> struct clk *mod_clk;
> struct reset_control *reset;
> int sensor_id;
> + u32 calibration_data[2];
> + bool has_calibration_data[2];
Why do you have two different values here?
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> struct thermal_zone_device *tzd;
> @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> struct resource *mem;
> void __iomem *base;
> int ret;
> + struct nvmem_cell *cell;
> + ssize_t cell_size;
> + u64 *cell_data;
>
> info->data = of_device_get_match_data(&pdev->dev);
> if (!info->data)
> @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + info->has_calibration_data[0] = false;
> + info->has_calibration_data[1] = false;
> +
> + if (!info->data->supports_nvmem)
> + goto no_nvmem;
> +
> + cell = nvmem_cell_get(&pdev->dev, "calibration");
> + if (IS_ERR(cell)) {
> + if (PTR_ERR(cell) == -EPROBE_DEFER)
> + return PTR_ERR(cell);
> + goto no_nvmem;
goto considered evil ? :)
> + }
> +
> + cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
> + nvmem_cell_put(cell);
> + switch (cell_size) {
> + case 8:
> + case 6:
> + info->has_calibration_data[1] = true;
> + info->calibration_data[1] = be32_to_cpu(
> + upper_32_bits(cell_data[0]));
> + case 4:
> + case 2:
> + info->has_calibration_data[0] = true;
> + info->calibration_data[0] = be32_to_cpu(
> + lower_32_bits(cell_data[0]));
Why do you need that switch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 12:29:12AM +0100, 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 | 86 +++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
> 2 files changed, 108 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index b7b5451226b0..8196203d65fe 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
> static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
> static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
> +
> struct gpadc_data {
> int temp_offset;
> int temp_scale;
> @@ -71,6 +74,10 @@ struct gpadc_data {
> unsigned int temp_data[MAX_SENSOR_COUNT];
> int (*sample_start)(struct sun4i_gpadc_iio *info);
> int (*sample_end)(struct sun4i_gpadc_iio *info);
> + u32 ctrl0_map;
> + u32 ctrl2_map;
> + u32 sensor_en_map;
> + u32 filter_map;
You can use a regmap_field for that.
> u32 irq_clear_map;
> u32 irq_control_map;
> bool has_bus_clk;
> @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .support_irq = false,
> };
>
> +static const struct gpadc_data sun8i_h3_ths_data = {
> + .temp_offset = -1791,
> + .temp_scale = -121,
> + .temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
> + .sample_start = sunxi_ths_sample_start,
> + .sample_end = sunxi_ths_sample_end,
> + .has_bus_clk = true,
> + .has_bus_rst = true,
> + .has_mod_clk = true,
> + .sensor_count = 1,
> + .supports_nvmem = true,
> + .support_irq = true,
> + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
> + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
> + .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
> + .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
> + .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
> + SUN8I_H3_THS_INTS_SHUT_INT_0 |
> + SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
> + SUN8I_H3_THS_INTS_ALARM_OFF_0,
> + .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> + SUN8I_H3_THS_TEMP_PERIOD(0x7),
> +};
> +
> struct sun4i_gpadc_iio {
> struct iio_dev *indio_dev;
> struct completion completion;
> @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
> +{
> + /* Disable ths interrupt */
> + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
> + /* Disable temperature sensor */
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
> +
> + return 0;
> +}
> +
> static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
> return info->data->sample_end(info);
> }
>
> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
> +{
> + if (info->has_calibration_data[0])
> + regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> + info->calibration_data[0]);
> +
> + if (info->has_calibration_data[1])
> + regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> + info->calibration_data[1]);
> +}
> +
> static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
> {
> /* clkin = 6MHz */
> @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
Please remain consistent with the prefixes used in the driver
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi,
On Mon, Jan 29, 2018 at 12:29:14AM +0100, Philipp Rossak wrote:
> 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 7a83b15225c7..413c789b588d 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -426,6 +426,15 @@
> };
> };
>
> + ths: thermal-sensor@1c25000 {
> + reg = <0x01c25000 0x100>;
The size is 0x400
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 12:29:13AM +0100, Philipp Rossak wrote:
> 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 | 38 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 8196203d65fe..9f7895ba1966 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
> SUN8I_H3_THS_TEMP_PERIOD(0x7),
> };
>
> +static const struct gpadc_data sun8i_a83t_ths_data = {
> + .temp_offset = -2724,
> + .temp_scale = -70,
> + .temp_data = {SUN8I_H3_THS_TDATA0,
> + SUN8I_A83T_THS_TDATA1,
> + SUN8I_A83T_THS_TDATA2,
> + 0},
> + .sample_start = sunxi_ths_sample_start,
> + .sample_end = sunxi_ths_sample_end,
> + .sensor_count = 3,
> + .supports_nvmem = false,
> + .support_irq = true,
> + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
> + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
Where are these values coming from?
> + .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
> + SUN8I_A83T_THS_TEMP_SENSE_EN1 |
> + SUN8I_A83T_THS_TEMP_SENSE_EN2,
> + .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
> + .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
> + SUN8I_A83T_THS_INTS_ALARM_INT_1 |
> + SUN8I_A83T_THS_INTS_ALARM_INT_2 |
> + SUN8I_H3_THS_INTS_SHUT_INT_0 |
> + SUN8I_A83T_THS_INTS_SHUT_INT_1 |
> + SUN8I_A83T_THS_INTS_SHUT_INT_2 |
> + SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
> + SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
> + SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
Do you reall need to clear all these interrupts if you're using only
one?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 12:29:16AM +0100, 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 | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index fbb007e5798e..3f83f6a27c74 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -72,6 +72,15 @@
> };
> };
>
> + thermal-zones {
> + cpu-thermal {
> + /* milliseconds */
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> + thermal-sensors = <&ths 0>;
if the thermal-sensor-cells value is indeed 0, the phandle parsing
will be broken here.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> 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 3f83f6a27c74..9bb5cc29fec5 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -72,6 +72,13 @@
> };
> };
>
> + soc {
> + sid: eeprom@1c14000 {
> + compatible = "allwinner,sun8i-h3-sid";
> + reg = <0x01c14000 0x400>;
> + };
> + };
> +
Shouldn't you also use a nvmem-cells property to the THS node?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On 29.01.2018 10:48, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:13AM +0100, Philipp Rossak wrote:
>> 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 | 38 ++++++++++++++++++++++++++++++++++++++
>> include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
>> 2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 8196203d65fe..9f7895ba1966 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
>> SUN8I_H3_THS_TEMP_PERIOD(0x7),
>> };
>>
>> +static const struct gpadc_data sun8i_a83t_ths_data = {
>> + .temp_offset = -2724,
>> + .temp_scale = -70,
>> + .temp_data = {SUN8I_H3_THS_TDATA0,
>> + SUN8I_A83T_THS_TDATA1,
>> + SUN8I_A83T_THS_TDATA2,
>> + 0},
>> + .sample_start = sunxi_ths_sample_start,
>> + .sample_end = sunxi_ths_sample_end,
>> + .sensor_count = 3,
>> + .supports_nvmem = false,
>> + .support_irq = true,
>> + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
>> + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
>
> Where are these values coming from?
>
These values are calculated with the formulas from the datasheet and
also tested on hardware. These settings seem ok.
>> + .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
>> + SUN8I_A83T_THS_TEMP_SENSE_EN1 |
>> + SUN8I_A83T_THS_TEMP_SENSE_EN2,
>> + .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
>> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
>> + .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
>> + SUN8I_A83T_THS_INTS_ALARM_INT_1 |
>> + SUN8I_A83T_THS_INTS_ALARM_INT_2 |
>> + SUN8I_H3_THS_INTS_SHUT_INT_0 |
>> + SUN8I_A83T_THS_INTS_SHUT_INT_1 |
>> + SUN8I_A83T_THS_INTS_SHUT_INT_2 |
>> + SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
>> + SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
>> + SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
>
> Do you reall need to clear all these interrupts if you're using only
> one?
>
No, I don't think so, I will remove them in the next version.
> Maxime
>
Philipp
On 29.01.2018 10:49, Maxime Ripard wrote:
> Hi,
>
> On Mon, Jan 29, 2018 at 12:29:14AM +0100, Philipp Rossak wrote:
>> 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 7a83b15225c7..413c789b588d 100644
>> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> @@ -426,6 +426,15 @@
>> };
>> };
>>
>> + ths: thermal-sensor@1c25000 {
>> + reg = <0x01c25000 0x100>;
>
> The size is 0x400
>
> Maxime
>
Ok, I will fix this.
Philipp
On 29.01.2018 10:50, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:16AM +0100, 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 | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index fbb007e5798e..3f83f6a27c74 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -72,6 +72,15 @@
>> };
>> };
>>
>> + thermal-zones {
>> + cpu-thermal {
>> + /* milliseconds */
>> + polling-delay-passive = <250>;
>> + polling-delay = <1000>;
>> + thermal-sensors = <&ths 0>;
>
> if the thermal-sensor-cells value is indeed 0, the phandle parsing
> will be broken here.
>
> Maxime
>
Ok, then I will remove the 0.
Thanks,
Philipp
On 29.01.2018 10:52, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>> 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 3f83f6a27c74..9bb5cc29fec5 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -72,6 +72,13 @@
>> };
>> };
>>
>> + soc {
>> + sid: eeprom@1c14000 {
>> + compatible = "allwinner,sun8i-h3-sid";
>> + reg = <0x01c14000 0x400>;
>> + };
>> + };
>> +
>
> Shouldn't you also use a nvmem-cells property to the THS node?
>
> Maxime
>
Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address
0x34. I will add that in the next version.
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp
>> +Example for A33:
>> ths: ths@1c25000 {
>> compatible = "allwinner,sun8i-a33-ths";
>> reg = <0x01c25000 0x100>;
>> @@ -17,6 +40,27 @@ 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>;
>> + };
>> +
>> +Example for A83T:
>> + 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>;
>> + };
>> +
>
> I'm wondering if this is actually needed. We've used this convoluted
> constructs to be compatible with the old driver, but I'm not sure this
> is actually worth it now, and this is causing several issues, among
> which:
> - We need to have a bunch of quirks to handle all the DT cases.
> - We need to have an MFD, which isn't really optimal
>
> So I'd rather introduce a new compatible for the old SoCs, keep the
> old driver around, and simplify a lot that driver code that will ease
> further developments. And we can also get rid of the MFD in the
> process. I discussed it with Quentin, and he was ok with it, what do
> you think?
>
> (and that would involve creating a new file for the bindings you
> introduce here).
>
> Maxime
>
I think this is a good idea, and the desired way to rework the driver.
To sum up what we talked on IRC:
This will end up in removing the MFD driver and moving the interrupt
handling into the iio driver. At the end this will also simplify the IRQ
part.
Philipp
On 29.01.2018 10:40, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:10AM +0100, 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 | 44 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index ac9ad2f8232f..74eeb5cd5218 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -27,6 +27,7 @@
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/module.h>
>> +#include <linux/nvmem-consumer.h>
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> @@ -74,6 +75,7 @@ struct gpadc_data {
>> bool has_bus_rst;
>> bool has_mod_clk;
>> int sensor_count;
>> + bool supports_nvmem;
>
> I think you should add some documentation along with all the fields
> you're adding.
ok I will add more informations in the next version into the commit message.
>
>> };
>>
>> static const struct gpadc_data sun4i_gpadc_data = {
>> @@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
>> .sample_start = sun4i_gpadc_sample_start,
>> .sample_end = sun4i_gpadc_sample_end,
>> .sensor_count = 1,
>> + .supports_nvmem = false,
>
> That's already its value if you leave it out.
>
>> };
>>
>> static const struct gpadc_data sun5i_gpadc_data = {
>> @@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
>> .sample_start = sun4i_gpadc_sample_start,
>> .sample_end = sun4i_gpadc_sample_end,
>> .sensor_count = 1,
>> + .supports_nvmem = false,
>> };
>>
>> static const struct gpadc_data sun6i_gpadc_data = {
>> @@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
>> .sample_start = sun4i_gpadc_sample_start,
>> .sample_end = sun4i_gpadc_sample_end,
>> .sensor_count = 1,
>> + .supports_nvmem = false,
>> };
>>
>> static const struct gpadc_data sun8i_a33_gpadc_data = {
>> @@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>> .sample_start = sun4i_gpadc_sample_start,
>> .sample_end = sun4i_gpadc_sample_end,
>> .sensor_count = 1,
>> + .supports_nvmem = false,
>> };
>>
>> struct sun4i_gpadc_iio {
>> @@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
>> struct clk *mod_clk;
>> struct reset_control *reset;
>> int sensor_id;
>> + u32 calibration_data[2];
>> + bool has_calibration_data[2];
>
> Why do you have two different values here?
>
I think my idea was too complex! I thought it would be better to check
if calibration data was read, and is able to be written to hardware.
those information were split per register.
I think a u64 should be fine for calibration_data. When I write the
calibration data I can check on the sensor count and write only the
lower 32 bits if there are less than 3 sensors.
Is this ok for you?
>> /* prevents concurrent reads of temperature and ADC */
>> struct mutex mutex;
>> struct thermal_zone_device *tzd;
>> @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>> struct resource *mem;
>> void __iomem *base;
>> int ret;
>> + struct nvmem_cell *cell;
>> + ssize_t cell_size;
>> + u64 *cell_data;
>>
>> info->data = of_device_get_match_data(&pdev->dev);
>> if (!info->data)
>> @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>> if (IS_ERR(base))
>> return PTR_ERR(base);
>>
>> + info->has_calibration_data[0] = false;
>> + info->has_calibration_data[1] = false;
>> +
>> + if (!info->data->supports_nvmem)
>> + goto no_nvmem;
>> +
>> + cell = nvmem_cell_get(&pdev->dev, "calibration");
>> + if (IS_ERR(cell)) {
>> + if (PTR_ERR(cell) == -EPROBE_DEFER)
>> + return PTR_ERR(cell);
>> + goto no_nvmem;
>
> goto considered evil ? :)
>
this was a suggestion from Jonatan in version one, to make the code
better readable.
.
>> + }
>> +
>> + cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
>> + nvmem_cell_put(cell);
>> + switch (cell_size) {
>> + case 8:
>> + case 6:
>> + info->has_calibration_data[1] = true;
>> + info->calibration_data[1] = be32_to_cpu(
>> + upper_32_bits(cell_data[0]));
>> + case 4:
>> + case 2:
>> + info->has_calibration_data[0] = true;
>> + info->calibration_data[0] = be32_to_cpu(
>> + lower_32_bits(cell_data[0]));
>
> Why do you need that switch?
You are right! The calibration reg seems to be always 64 bit wide. [1]
So I will just check for the length of 8.
>
> Thanks!
> Maxime
>
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
Thanks,
Philipp
1;5002;0c
On Mon, Jan 29, 2018 at 12:53:48PM +0100, Philipp Rossak wrote:
>
>
> On 29.01.2018 10:48, Maxime Ripard wrote:
> > On Mon, Jan 29, 2018 at 12:29:13AM +0100, Philipp Rossak wrote:
> > > 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 | 38 ++++++++++++++++++++++++++++++++++++++
> > > include/linux/mfd/sun4i-gpadc.h | 18 ++++++++++++++++++
> > > 2 files changed, 56 insertions(+)
> > >
> > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> > > index 8196203d65fe..9f7895ba1966 100644
> > > --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> > > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> > > @@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
> > > SUN8I_H3_THS_TEMP_PERIOD(0x7),
> > > };
> > > +static const struct gpadc_data sun8i_a83t_ths_data = {
> > > + .temp_offset = -2724,
> > > + .temp_scale = -70,
> > > + .temp_data = {SUN8I_H3_THS_TDATA0,
> > > + SUN8I_A83T_THS_TDATA1,
> > > + SUN8I_A83T_THS_TDATA2,
> > > + 0},
> > > + .sample_start = sunxi_ths_sample_start,
> > > + .sample_end = sunxi_ths_sample_end,
> > > + .sensor_count = 3,
> > > + .supports_nvmem = false,
> > > + .support_irq = true,
> > > + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
> > > + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
> >
> > Where are these values coming from?
> >
>
> These values are calculated with the formulas from the datasheet and also
> tested on hardware. These settings seem ok.
You should at least put a comment explaining how you got to these values.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Mon, Jan 29, 2018 at 01:33:12PM +0100, Philipp Rossak wrote:
> > > static const struct gpadc_data sun4i_gpadc_data = {
> > > @@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> > > .sample_start = sun4i_gpadc_sample_start,
> > > .sample_end = sun4i_gpadc_sample_end,
> > > .sensor_count = 1,
> > > + .supports_nvmem = false,
> >
> > That's already its value if you leave it out.
> >
> > > };
> > > static const struct gpadc_data sun5i_gpadc_data = {
> > > @@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
> > > .sample_start = sun4i_gpadc_sample_start,
> > > .sample_end = sun4i_gpadc_sample_end,
> > > .sensor_count = 1,
> > > + .supports_nvmem = false,
> > > };
> > > static const struct gpadc_data sun6i_gpadc_data = {
> > > @@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
> > > .sample_start = sun4i_gpadc_sample_start,
> > > .sample_end = sun4i_gpadc_sample_end,
> > > .sensor_count = 1,
> > > + .supports_nvmem = false,
> > > };
> > > static const struct gpadc_data sun8i_a33_gpadc_data = {
> > > @@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> > > .sample_start = sun4i_gpadc_sample_start,
> > > .sample_end = sun4i_gpadc_sample_end,
> > > .sensor_count = 1,
> > > + .supports_nvmem = false,
> > > };
> > > struct sun4i_gpadc_iio {
> > > @@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
> > > struct clk *mod_clk;
> > > struct reset_control *reset;
> > > int sensor_id;
> > > + u32 calibration_data[2];
> > > + bool has_calibration_data[2];
> >
> > Why do you have two different values here?
>
> I think my idea was too complex! I thought it would be better to check if
> calibration data was read, and is able to be written to hardware. those
> information were split per register.
>
> I think a u64 should be fine for calibration_data. When I write the
> calibration data I can check on the sensor count and write only the lower 32
> bits if there are less than 3 sensors.
>
> Is this ok for you?
I'd need to see the implementation, but make sure that this is
documented in your driver :)
>
> > > /* prevents concurrent reads of temperature and ADC */
> > > struct mutex mutex;
> > > struct thermal_zone_device *tzd;
> > > @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> > > struct resource *mem;
> > > void __iomem *base;
> > > int ret;
> > > + struct nvmem_cell *cell;
> > > + ssize_t cell_size;
> > > + u64 *cell_data;
> > > info->data = of_device_get_match_data(&pdev->dev);
> > > if (!info->data)
> > > @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> > > if (IS_ERR(base))
> > > return PTR_ERR(base);
> > > + info->has_calibration_data[0] = false;
> > > + info->has_calibration_data[1] = false;
> > > +
> > > + if (!info->data->supports_nvmem)
> > > + goto no_nvmem;
> > > +
> > > + cell = nvmem_cell_get(&pdev->dev, "calibration");
> > > + if (IS_ERR(cell)) {
> > > + if (PTR_ERR(cell) == -EPROBE_DEFER)
> > > + return PTR_ERR(cell);
> > > + goto no_nvmem;
> >
> > goto considered evil ? :)
>
> this was a suggestion from Jonatan in version one, to make the code better
> readable.
Isn't
if (info->data->supports_nvmem && IS_ERR(cell = nvmem_cell_get()))
pretty much the same thing?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi Philipp,
On Mon, Jan 29, 2018 at 12:29:04AM +0100, 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]>
> ---
> .../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> index 86dd8191b04c..22df0c5c23d4 100644
> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
> and sometimes as a touchscreen controller.
>
> Required properties:
> - - compatible: "allwinner,sun8i-a33-ths",
> + - compatible: must contain one of the following compatibles:
> + - "allwinner,sun8i-a33-ths"
> + - "allwinner,sun8i-h3-ths"
> + - "allwinner,sun8i-a83t-ths"
> - reg: mmio address range of the chip,
> - - #thermal-sensor-cells: shall be 0,
> + - #thermal-sensor-cells: shall be 0 or 1,
Well, thermal-sensor-cells is either 0 or 1 :)
Better to point to the documentation describing this
thermal-sensor-cells IMHO.
> - #io-channel-cells: shall be 0,
>
> -Example:
> +Required properties for the following compatibles:
> + - "allwinner,sun8i-h3-ths"
> + - "allwinner,sun8i-a83t-ths"
> + - interrupts: the sampling interrupt of the ADC,
> +
> +Required properties for the following compatibles:
> + - "allwinner,sun8i-h3-ths"
> + - clocks: the bus clock and the input clock of the ADC,
> + - clock-names: should be "bus" and "mod",
> + - resets: the bus reset of the ADC,
> +
> +Optional properties for the following compatibles:
> + - "allwinner,sun8i-h3-ths"
> + - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
> + If unspecified default values shall be used. The size should
> + be 0x2 * sensorcount.
"twice the number of sensors" ?
> + - 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,27 @@ 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>;
> + };
> +
> +Example for A83T:
> + 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>;
> + };
> +
Aside from Maxime's comment on how we would like to refactor GPADC/THS,
I'm not sure we really want an example for each an every thermal sensor
supported.
Quentin
Hi Philipp,
On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
>
> This commit reworks the code and allows the sampling start/end code and
> the position of value readout register to be altered. Later the start/end
> functions will be used to configure the ths and start/stop the
> sampling.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> Signed-off-by: Philipp Rossak <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 03804ff9c006..db57d9fffe48 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
> return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
> }
>
> +struct sun4i_gpadc_iio;
> +
> +/*
> + * Prototypes for these functions, which enable these functions to be
> + * referenced in gpadc_data structures.
> + */
Comment not needed.
> +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
> +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
> +
> struct gpadc_data {
> int temp_offset;
> int temp_scale;
> @@ -56,6 +65,9 @@ struct gpadc_data {
> unsigned int tp_adc_select;
> unsigned int (*adc_chan_select)(unsigned int chan);
> unsigned int adc_chan_mask;
> + unsigned int temp_data;
Does not really have anything to do with sample_start/end. I would have
made a different commit for it.
Otherwise,
Reviewed-by: Quentin Schulz <[email protected]>
Quentin
On 31.01.2018 18:40, Quentin Schulz wrote:
> Hi Philipp,
>
> On Mon, Jan 29, 2018 at 12:29:04AM +0100, 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]>
>> ---
>> .../devicetree/bindings/mfd/sun4i-gpadc.txt | 50 ++++++++++++++++++++--
>> 1 file changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> index 86dd8191b04c..22df0c5c23d4 100644
>> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
>> and sometimes as a touchscreen controller.
>>
>> Required properties:
>> - - compatible: "allwinner,sun8i-a33-ths",
>> + - compatible: must contain one of the following compatibles:
>> + - "allwinner,sun8i-a33-ths"
>> + - "allwinner,sun8i-h3-ths"
>> + - "allwinner,sun8i-a83t-ths"
>> - reg: mmio address range of the chip,
>> - - #thermal-sensor-cells: shall be 0,
>> + - #thermal-sensor-cells: shall be 0 or 1,
>
> Well, thermal-sensor-cells is either 0 or 1 :)
>
> Better to point to the documentation describing this
> thermal-sensor-cells IMHO.
>
I agree, I will change this in the next version.
>> - #io-channel-cells: shall be 0,
>>
>> -Example:
>> +Required properties for the following compatibles:
>> + - "allwinner,sun8i-h3-ths"
>> + - "allwinner,sun8i-a83t-ths"
>> + - interrupts: the sampling interrupt of the ADC,
>> +
>> +Required properties for the following compatibles:
>> + - "allwinner,sun8i-h3-ths"
>> + - clocks: the bus clock and the input clock of the ADC,
>> + - clock-names: should be "bus" and "mod",
>> + - resets: the bus reset of the ADC,
>> +
>> +Optional properties for the following compatibles:
>> + - "allwinner,sun8i-h3-ths"
>> + - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
>> + If unspecified default values shall be used. The size should
>> + be 0x2 * sensorcount.
>
> "twice the number of sensors" ?
>
As already mentioned in an other answers, this here is not correct.
I got somehow a wrong information or mixed something up. For H5, H3,
A83T and A64 the thermal sensor calibration data is always 64 bit wide
and placed on the eFuse address 0x34 [1].
>> + - 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,27 @@ 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>;
>> + };
>> +
>> +Example for A83T:
>> + 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>;
>> + };
>> +
>
> Aside from Maxime's comment on how we would like to refactor GPADC/THS,
> I'm not sure we really want an example for each an every thermal sensor
> supported.
>
> Quentin
>
I agree we don't want to have an example for every sensor, but I think
at least those two are interesting, since one has 3 sensors and no
clocks, and one has 1 sensor and clocks.
Thanks,
Philipp
[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
On 31.01.2018 18:51, Quentin Schulz wrote:
> Hi Philipp,
>
> On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote:
>> For adding newer sensor some basic rework of the code is necessary.
>>
>> This commit reworks the code and allows the sampling start/end code and
>> the position of value readout register to be altered. Later the start/end
>> functions will be used to configure the ths and start/stop the
>> sampling.
>>
>> Signed-off-by: Icenowy Zheng <[email protected]>
>> Signed-off-by: Philipp Rossak <[email protected]>
>> ---
>> drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 03804ff9c006..db57d9fffe48 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
>> return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
>> }
>>
>> +struct sun4i_gpadc_iio;
>> +
>> +/*
>> + * Prototypes for these functions, which enable these functions to be
>> + * referenced in gpadc_data structures.
>> + */
>
> Comment not needed.
>
>> +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
>> +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>> +
>> struct gpadc_data {
>> int temp_offset;
>> int temp_scale;
>> @@ -56,6 +65,9 @@ struct gpadc_data {
>> unsigned int tp_adc_select;
>> unsigned int (*adc_chan_select)(unsigned int chan);
>> unsigned int adc_chan_mask;
>> + unsigned int temp_data;
>
> Does not really have anything to do with sample_start/end. I would have
> made a different commit for it.
>
> Otherwise,
> Reviewed-by: Quentin Schulz <[email protected]>
>
> Quentin
>
Ok I will split this.
Thanks,
Philipp
Hi Philipp,
On Mon, Jan 29, 2018 at 12:29:09AM +0100, 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 | 36 +++++++++++++++++++++++-------------
> include/linux/mfd/sun4i-gpadc.h | 3 +++
> 2 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 51ec0104d678..ac9ad2f8232f 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -67,12 +67,13 @@ struct gpadc_data {
> unsigned int tp_adc_select;
> unsigned int (*adc_chan_select)(unsigned int chan);
> unsigned int adc_chan_mask;
> - unsigned int temp_data;
> + unsigned int temp_data[MAX_SENSOR_COUNT];
> int (*sample_start)(struct sun4i_gpadc_iio *info);
> int (*sample_end)(struct sun4i_gpadc_iio *info);
> bool has_bus_clk;
> bool has_bus_rst;
> bool has_mod_clk;
> + int sensor_count;
> };
>
I've noticed that for H3, A83T, A64 (at least), if DATA reg of sensor 0
is e.g. 0x80, DATA reg of sensor N is at 0x80 + 0x04 * N.
Is that verified for other SoCs? Does anyone have some input on this?
We could then just use temp_data as the DATA reg "base" and increment by
0x4 depending on the sensor id instead of using a fixed-size array.
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
> .adc_chan_select = &sun4i_gpadc_chan_select,
> .adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
> - .temp_data = SUN4I_GPADC_TEMP_DATA,
> + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
> .sample_start = sun4i_gpadc_sample_start,
> .sample_end = sun4i_gpadc_sample_end,
> + .sensor_count = 1,
If the solution above is not desirable/possible, could we use something
like:
unsigned int sun4i_temp_data[] = {SUN4I_GPADC_TEMP_DATA,};
static const struct gpadc_data sun4i_gpadc_data = {
.temp_data = &sun4i_temp_data,
.sensor_count = ARRAY_SIZE(sun4i_temp_data),
};
That avoids 1) inconsistencies between the array size and the array
itself, 2) does not require to pad the array with zeroes.
[...]
> @@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> pm_runtime_enable(&pdev->dev);
>
> if (IS_ENABLED(CONFIG_THERMAL_OF)) {
> - info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
> - 0, info,
> - &sun4i_ts_tz_ops);
> + for (i = 0; i < info->data->sensor_count; i++) {
> + info->sensor_id = i;
> + info->tzd = thermal_zone_of_sensor_register(
> + info->sensor_device,
> + i, info, &sun4i_ts_tz_ops);
> + }
As Maxime said, this does not work.
One way would be to have a new structure being:
struct sun4i_sensor_info {
struct sun4i_gpadc_iio *info;
unsigned int sensor_id;
};
Or since we only use the iio_dev within the sun4i_gpadc_iio in the
.get_temp function, we may replace info by struct iio_dev *indio_dev
above.
Quentin
Hi Philipp,
On Mon, Jan 29, 2018 at 12:29:11AM +0100, Philipp Rossak wrote:
> This patch rewors the driver to support interrupts for the thermal part
> of the sensor.
>
> This is only available for the newer sensor (currently H3 and A83T).
> The interrupt will be trigerd on data available and triggers the update
> for the thermal sensors. All newer sensors have different amount of
> sensors and different interrupts for each device the reset of the
> interrupts need to be done different
>
> For the newer sensors is the autosuspend disabled.
>
> Signed-off-by: Philipp Rossak <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
> include/linux/mfd/sun4i-gpadc.h | 2 ++
> 2 files changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 74eeb5cd5218..b7b5451226b0 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -71,11 +71,14 @@ struct gpadc_data {
> unsigned int temp_data[MAX_SENSOR_COUNT];
> int (*sample_start)(struct sun4i_gpadc_iio *info);
> int (*sample_end)(struct sun4i_gpadc_iio *info);
> + u32 irq_clear_map;
> + u32 irq_control_map;
I would say to use a regmap_irq_chip for controlling IRQs.
> bool has_bus_clk;
> bool has_bus_rst;
> bool has_mod_clk;
> int sensor_count;
> bool supports_nvmem;
> + bool support_irq;
> };
>
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> .sample_end = sun4i_gpadc_sample_end,
> .sensor_count = 1,
> .supports_nvmem = false,
> + .support_irq = false,
False is the default, no need to set support_irq.
[...]
> struct sun4i_gpadc_iio {
> @@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
> return 0;
> }
>
> + if (info->data->support_irq) {
> + regmap_read(info->regmap, info->data->temp_data[sensor], val);
> + return 0;
> + }
> +
Maybe you could define a new thermal_zone_of_device_ops for these new
thermal sensors? That way, you don't even need the boolean support_irq.
> return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
> }
>
> @@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t sunxi_irq_thread(int irq, void *data)
I think we're trying to avoid sunxi mentions but rather using the name
of the first IP (in term of product release, not support) using this
function.
> +{
> + struct sun4i_gpadc_iio *info = data;
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
> +
Will be handled by regmap_irq_chip.
[...]
> - info->no_irq = true;
> + if (info->data->support_irq) {
> + /* only the new versions of ths support right now irqs */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
> + return irq;
> + }
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + sunxi_irq_thread, IRQF_ONESHOT,
> + dev_name(&pdev->dev), info);
> + if (ret)
> + return ret;
> +
> + } else
> + info->no_irq = true;
> +
That's a bit funny to have two booleans named no_irq and support_irq :)
> indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
> indio_dev->channels = sun8i_a33_gpadc_channels;
>
> @@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - pm_runtime_set_autosuspend_delay(&pdev->dev,
> - SUN4I_GPADC_AUTOSUSPEND_DELAY);
> - pm_runtime_use_autosuspend(&pdev->dev);
> - pm_runtime_set_suspended(&pdev->dev);
> - pm_runtime_enable(&pdev->dev);
> + if (!info->data->support_irq) {
> + pm_runtime_set_autosuspend_delay(&pdev->dev,
> + SUN4I_GPADC_AUTOSUSPEND_DELAY);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> + }
Why would you not want your IP to do runtime PM?
Quentin
Hi Philipp,
On Mon, Jan 29, 2018 at 12:29:12AM +0100, 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 | 86 +++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
> 2 files changed, 108 insertions(+)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index b7b5451226b0..8196203d65fe 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
> static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
> static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
> +
We try to avoid using the generic sunxi prefix.
> struct gpadc_data {
> int temp_offset;
> int temp_scale;
> @@ -71,6 +74,10 @@ struct gpadc_data {
> unsigned int temp_data[MAX_SENSOR_COUNT];
> int (*sample_start)(struct sun4i_gpadc_iio *info);
> int (*sample_end)(struct sun4i_gpadc_iio *info);
> + u32 ctrl0_map;
> + u32 ctrl2_map;
> + u32 sensor_en_map;
> + u32 filter_map;
> u32 irq_clear_map;
> u32 irq_control_map;
> bool has_bus_clk;
> @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> .support_irq = false,
> };
>
> +static const struct gpadc_data sun8i_h3_ths_data = {
> + .temp_offset = -1791,
> + .temp_scale = -121,
> + .temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
> + .sample_start = sunxi_ths_sample_start,
> + .sample_end = sunxi_ths_sample_end,
> + .has_bus_clk = true,
> + .has_bus_rst = true,
> + .has_mod_clk = true,
> + .sensor_count = 1,
> + .supports_nvmem = true,
> + .support_irq = true,
> + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
> + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
> + .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
> + .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
> + .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
> + SUN8I_H3_THS_INTS_SHUT_INT_0 |
> + SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
> + SUN8I_H3_THS_INTS_ALARM_OFF_0,
> + .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> + SUN8I_H3_THS_TEMP_PERIOD(0x7),
From what I've understood, ACQ regs are basically clock dividers. We
should make a better job at explaining it :)
> +};
> +
> struct sun4i_gpadc_iio {
> struct iio_dev *indio_dev;
> struct completion completion;
> @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
> +{
> + /* Disable ths interrupt */
> + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
> + /* Disable temperature sensor */
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
> +
> + return 0;
> +}
> +
> static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
> return info->data->sample_end(info);
> }
>
> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
> +{
> + if (info->has_calibration_data[0])
> + regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> + info->calibration_data[0]);
> +
> + if (info->has_calibration_data[1])
> + regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> + info->calibration_data[1]);
> +}
> +
> static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
> {
> /* clkin = 6MHz */
> @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
> return 0;
> }
>
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
> +{
> + u32 value;
> + sunxi_calibrate(info);
> +
> + if (info->data->ctrl0_map)
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
> + info->data->ctrl0_map);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> + info->data->ctrl2_map);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> + info->data->irq_clear_map);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
> + info->data->filter_map);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_INTC,
> + info->data->irq_control_map);
> +
> + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
> +
> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> + info->data->sensor_en_map | value);
> +
> + return 0;
> +}
> +
I'm a bit worried. Will all the sensors have the same sample start? Or
will we need at some point also entries in info->data for register
addresses, if they have ctrl2 or filter, etc...
Maybe we could define a sample_start for the h3 only and reuse-it if
possible and if not declare another sample start? All depends if the
sample start is most likely to change in the near future or not. If it
is, then we should avoid adding a lot of variables in info->data and
just go for a single sample_start per SoC.
> static int sun4i_gpadc_runtime_resume(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
> .compatible = "allwinner,sun8i-a33-ths",
> .data = &sun8i_a33_gpadc_data,
> },
> + {
> + .compatible = "allwinner,sun8i-h3-ths",
> + .data = &sun8i_h3_ths_data,
> + },
> { /* sentinel */ }
> };
>
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 458f2159a95a..80b79c31cea3 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -91,7 +91,29 @@
> #define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
>
> /* SUNXI_THS COMMON REGISTERS + DEFINES */
> +#define SUN8I_H3_THS_CTRL0 0x00
> +#define SUN8I_H3_THS_CTRL2 0x40
> #define SUN8I_H3_THS_INTC 0x44
> +#define SUN8I_H3_THS_STAT 0x48
> +#define SUN8I_H3_THS_FILTER 0x70
> +#define SUNXI_THS_CDATA_0_1 0x74
> +#define SUNXI_THS_CDATA_2_3 0x78
> +#define SUN8I_H3_THS_TDATA0 0x80
> +
> +#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))
> +
> +#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
> +
> +#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
> +
> +#define SUN8I_H3_THS_INTS_ALARM_INT_0 BIT(0)
> +#define SUN8I_H3_THS_INTS_SHUT_INT_0 BIT(4)
> +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
> +#define SUN8I_H3_THS_INTS_ALARM_OFF_0 BIT(12)
> +
> +#define SUN8I_H3_THS_INTC_ALARM_INT_EN0 BIT(0)
> +#define SUN8I_H3_THS_INTC_SHUT_INT_EN0 BIT(4)
> +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
>
Personal taste here but I prefer the register bits to be defined just
below the register address define.
i.e.:
#define SUN8I_H3_THS_CTRL2 0x40
#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))
#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
that way we know for which register we should use which constants.
Quentin
Hi Philipp,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
Note: the linux-review/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415 HEAD e571c3ced84a9d7f150cb2d239919d31d25114e2 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/iio/adc/sun4i-gpadc-iio.c: In function 'sunxi_irq_thread':
>> drivers/iio/adc/sun4i-gpadc-iio.c:448:29: error: 'SUN8I_H3_THS_STAT' undeclared (first use in this function); did you mean 'SUN8I_H3_THS_INTC'?
regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
^~~~~~~~~~~~~~~~~
SUN8I_H3_THS_INTC
drivers/iio/adc/sun4i-gpadc-iio.c:448:29: note: each undeclared identifier is reported only once for each function it appears in
vim +448 drivers/iio/adc/sun4i-gpadc-iio.c
443
444 static irqreturn_t sunxi_irq_thread(int irq, void *data)
445 {
446 struct sun4i_gpadc_iio *info = data;
447
> 448 regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
449
450 thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE);
451
452 return IRQ_HANDLED;
453 }
454
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Philipp,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
vim +608 drivers/iio/adc/sun4i-gpadc-iio.c
564
565 static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
566 struct iio_dev *indio_dev)
567 {
568 struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
569 struct resource *mem;
570 void __iomem *base;
571 int ret;
572 struct nvmem_cell *cell;
573 ssize_t cell_size;
574 u64 *cell_data;
575
576 info->data = of_device_get_match_data(&pdev->dev);
577 if (!info->data)
578 return -ENODEV;
579
580 info->no_irq = true;
581 indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
582 indio_dev->channels = sun8i_a33_gpadc_channels;
583
584 mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
585 base = devm_ioremap_resource(&pdev->dev, mem);
586 if (IS_ERR(base))
587 return PTR_ERR(base);
588
589 info->has_calibration_data[0] = false;
590 info->has_calibration_data[1] = false;
591
592 if (!info->data->supports_nvmem)
593 goto no_nvmem;
594
595 cell = nvmem_cell_get(&pdev->dev, "calibration");
596 if (IS_ERR(cell)) {
597 if (PTR_ERR(cell) == -EPROBE_DEFER)
598 return PTR_ERR(cell);
599 goto no_nvmem;
600 }
601
602 cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
603 nvmem_cell_put(cell);
604 switch (cell_size) {
605 case 8:
606 case 6:
607 info->has_calibration_data[1] = true;
> 608 info->calibration_data[1] = be32_to_cpu(
609 upper_32_bits(cell_data[0]));
610 case 4:
611 case 2:
612 info->has_calibration_data[0] = true;
613 info->calibration_data[0] = be32_to_cpu(
614 lower_32_bits(cell_data[0]));
615 break;
616 default:
617 break;
618 }
619
620 no_nvmem:
621
622 info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
623 &sun4i_gpadc_regmap_config);
624 if (IS_ERR(info->regmap)) {
625 ret = PTR_ERR(info->regmap);
626 dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
627 return ret;
628 }
629
630 if (info->data->has_bus_rst) {
631 info->reset = devm_reset_control_get(&pdev->dev, NULL);
632 if (IS_ERR(info->reset)) {
633 ret = PTR_ERR(info->reset);
634 return ret;
635 }
636
637 ret = reset_control_deassert(info->reset);
638 if (ret)
639 return ret;
640 }
641
642 if (info->data->has_bus_clk) {
643 info->bus_clk = devm_clk_get(&pdev->dev, "bus");
644 if (IS_ERR(info->bus_clk)) {
645 ret = PTR_ERR(info->bus_clk);
646 goto assert_reset;
647 }
648
649 ret = clk_prepare_enable(info->bus_clk);
650 if (ret)
651 goto assert_reset;
652 }
653
654 if (info->data->has_mod_clk) {
655 info->mod_clk = devm_clk_get(&pdev->dev, "mod");
656 if (IS_ERR(info->mod_clk)) {
657 ret = PTR_ERR(info->mod_clk);
658 goto disable_bus_clk;
659 }
660
661 /* Running at 6MHz */
662 ret = clk_set_rate(info->mod_clk, 4000000);
663 if (ret)
664 goto disable_bus_clk;
665
666 ret = clk_prepare_enable(info->mod_clk);
667 if (ret)
668 goto disable_bus_clk;
669 }
670
671 if (IS_ENABLED(CONFIG_THERMAL_OF))
672 info->sensor_device = &pdev->dev;
673
674 return 0;
675
676 disable_bus_clk:
677 clk_disable_unprepare(info->bus_clk);
678
679 assert_reset:
680 reset_control_assert(info->reset);
681
682 return ret;
683 }
684
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 31.01.2018 19:42, Quentin Schulz wrote:
> Hi Philipp,
>
> On Mon, Jan 29, 2018 at 12:29:09AM +0100, 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 | 36 +++++++++++++++++++++++-------------
>> include/linux/mfd/sun4i-gpadc.h | 3 +++
>> 2 files changed, 26 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 51ec0104d678..ac9ad2f8232f 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -67,12 +67,13 @@ struct gpadc_data {
>> unsigned int tp_adc_select;
>> unsigned int (*adc_chan_select)(unsigned int chan);
>> unsigned int adc_chan_mask;
>> - unsigned int temp_data;
>> + unsigned int temp_data[MAX_SENSOR_COUNT];
>> int (*sample_start)(struct sun4i_gpadc_iio *info);
>> int (*sample_end)(struct sun4i_gpadc_iio *info);
>> bool has_bus_clk;
>> bool has_bus_rst;
>> bool has_mod_clk;
>> + int sensor_count;
>> };
>>
>
> I've noticed that for H3, A83T, A64 (at least), if DATA reg of sensor 0
> is e.g. 0x80, DATA reg of sensor N is at 0x80 + 0x04 * N.
>
> Is that verified for other SoCs? Does anyone have some input on this?
>
> We could then just use temp_data as the DATA reg "base" and increment by
> 0x4 depending on the sensor id instead of using a fixed-size array.
>
This sounds like a good idea! I will add this to the next version.
I can verify this with a table, I created during development. I will
upload it during the weekend here: [1]
>> static const struct gpadc_data sun4i_gpadc_data = {
>> @@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
>> .tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
>> .adc_chan_select = &sun4i_gpadc_chan_select,
>> .adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
>> - .temp_data = SUN4I_GPADC_TEMP_DATA,
>> + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
>> .sample_start = sun4i_gpadc_sample_start,
>> .sample_end = sun4i_gpadc_sample_end,
>> + .sensor_count = 1,
>
> If the solution above is not desirable/possible, could we use something
> like:
>
> unsigned int sun4i_temp_data[] = {SUN4I_GPADC_TEMP_DATA,};
>
> static const struct gpadc_data sun4i_gpadc_data = {
> .temp_data = &sun4i_temp_data,
> .sensor_count = ARRAY_SIZE(sun4i_temp_data),
> };
>
> That avoids 1) inconsistencies between the array size and the array
> itself, 2) does not require to pad the array with zeroes.
>
> [...]
>
>> @@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>> pm_runtime_enable(&pdev->dev);
>>
>> if (IS_ENABLED(CONFIG_THERMAL_OF)) {
>> - info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
>> - 0, info,
>> - &sun4i_ts_tz_ops);
>> + for (i = 0; i < info->data->sensor_count; i++) {
>> + info->sensor_id = i;
>> + info->tzd = thermal_zone_of_sensor_register(
>> + info->sensor_device,
>> + i, info, &sun4i_ts_tz_ops);
>> + }
>
> As Maxime said, this does not work.
>
> One way would be to have a new structure being:
> struct sun4i_sensor_info {
> struct sun4i_gpadc_iio *info;
> unsigned int sensor_id;
> };
>
> Or since we only use the iio_dev within the sun4i_gpadc_iio in the
> .get_temp function, we may replace info by struct iio_dev *indio_dev
> above.
>
> Quentin
>
I will have a closer look on this next week, when I start to work on the
next version..
Thanks,
Philipp
[1]: http://linux-sunxi.org/Thermal_Sensor
On 31.01.2018 20:07, Quentin Schulz wrote:
> Hi Philipp,
>
> On Mon, Jan 29, 2018 at 12:29:11AM +0100, Philipp Rossak wrote:
>> This patch rewors the driver to support interrupts for the thermal part
>> of the sensor.
>>
>> This is only available for the newer sensor (currently H3 and A83T).
>> The interrupt will be trigerd on data available and triggers the update
>> for the thermal sensors. All newer sensors have different amount of
>> sensors and different interrupts for each device the reset of the
>> interrupts need to be done different
>>
>> For the newer sensors is the autosuspend disabled.
>>
>> Signed-off-by: Philipp Rossak <[email protected]>
>> Acked-by: Jonathan Cameron <[email protected]>
>> ---
>> drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
>> include/linux/mfd/sun4i-gpadc.h | 2 ++
>> 2 files changed, 56 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 74eeb5cd5218..b7b5451226b0 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -71,11 +71,14 @@ struct gpadc_data {
>> unsigned int temp_data[MAX_SENSOR_COUNT];
>> int (*sample_start)(struct sun4i_gpadc_iio *info);
>> int (*sample_end)(struct sun4i_gpadc_iio *info);
>> + u32 irq_clear_map;
>> + u32 irq_control_map;
>
> I would say to use a regmap_irq_chip for controlling IRQs.
>
Sounds good for me! I will rework that in the next version.
>> bool has_bus_clk;
>> bool has_bus_rst;
>> bool has_mod_clk;
>> int sensor_count;
>> bool supports_nvmem;
>> + bool support_irq;
>> };
>>
>> static const struct gpadc_data sun4i_gpadc_data = {
>> @@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
>> .sample_end = sun4i_gpadc_sample_end,
>> .sensor_count = 1,
>> .supports_nvmem = false,
>> + .support_irq = false,
>
> False is the default, no need to set support_irq.
>
> [...]
>
>> struct sun4i_gpadc_iio {
>> @@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
>> return 0;
>> }
>>
>> + if (info->data->support_irq) {
>> + regmap_read(info->regmap, info->data->temp_data[sensor], val);
>> + return 0;
>> + }
>> +
>
> Maybe you could define a new thermal_zone_of_device_ops for these new
> thermal sensors? That way, you don't even need the boolean support_irq.
>
Sounds good for me! I will rework that in the next version.
>> return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
>> }
>>
>> @@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> +static irqreturn_t sunxi_irq_thread(int irq, void *data)
>
> I think we're trying to avoid sunxi mentions but rather using the name
> of the first IP (in term of product release, not support) using this
> function.
>
>> +{
>> + struct sun4i_gpadc_iio *info = data;
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
>> +
>
> Will be handled by regmap_irq_chip.
> [...]
>> - info->no_irq = true;
>> + if (info->data->support_irq) {
>> + /* only the new versions of ths support right now irqs */
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0) {
>> + dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
>> + return irq;
>> + }
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
>> + sunxi_irq_thread, IRQF_ONESHOT,
>> + dev_name(&pdev->dev), info);
>> + if (ret)
>> + return ret;
>> +
>> + } else
>> + info->no_irq = true;
>> +
>
> That's a bit funny to have two booleans named no_irq and support_irq :)
>
I know this looks very funny. I thought this would be better to keep, to
_not_ break anything. Since I will rework the whole driver and integrate
the mfd part I hope I can remove both.
>> indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
>> indio_dev->channels = sun8i_a33_gpadc_channels;
>>
>> @@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>> if (ret)
>> return ret;
>>
>> - pm_runtime_set_autosuspend_delay(&pdev->dev,
>> - SUN4I_GPADC_AUTOSUSPEND_DELAY);
>> - pm_runtime_use_autosuspend(&pdev->dev);
>> - pm_runtime_set_suspended(&pdev->dev);
>> - pm_runtime_enable(&pdev->dev);
>> + if (!info->data->support_irq) {
>> + pm_runtime_set_autosuspend_delay(&pdev->dev,
>> + SUN4I_GPADC_AUTOSUSPEND_DELAY);
>> + pm_runtime_use_autosuspend(&pdev->dev);
>> + pm_runtime_set_suspended(&pdev->dev);
>> + pm_runtime_enable(&pdev->dev);
>> + }
>
> Why would you not want your IP to do runtime PM?
I will enable this again, in the next version! I had some issues with
this, thus I disabled this, but I know now what I did wrong!
Thanks,
Philipp
On 31.01.2018 20:23, Quentin Schulz wrote:
> Hi Philipp,
>
> On Mon, Jan 29, 2018 at 12:29:12AM +0100, 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 | 86 +++++++++++++++++++++++++++++++++++++++
>> include/linux/mfd/sun4i-gpadc.h | 22 ++++++++++
>> 2 files changed, 108 insertions(+)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index b7b5451226b0..8196203d65fe 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
>> static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
>> static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>>
>> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
>> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
>> +
>
> We try to avoid using the generic sunxi prefix.
>
>> struct gpadc_data {
>> int temp_offset;
>> int temp_scale;
>> @@ -71,6 +74,10 @@ struct gpadc_data {
>> unsigned int temp_data[MAX_SENSOR_COUNT];
>> int (*sample_start)(struct sun4i_gpadc_iio *info);
>> int (*sample_end)(struct sun4i_gpadc_iio *info);
>> + u32 ctrl0_map;
>> + u32 ctrl2_map;
>> + u32 sensor_en_map;
>> + u32 filter_map;
>> u32 irq_clear_map;
>> u32 irq_control_map;
>> bool has_bus_clk;
>> @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>> .support_irq = false,
>> };
>>
>> +static const struct gpadc_data sun8i_h3_ths_data = {
>> + .temp_offset = -1791,
>> + .temp_scale = -121,
>> + .temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
>> + .sample_start = sunxi_ths_sample_start,
>> + .sample_end = sunxi_ths_sample_end,
>> + .has_bus_clk = true,
>> + .has_bus_rst = true,
>> + .has_mod_clk = true,
>> + .sensor_count = 1,
>> + .supports_nvmem = true,
>> + .support_irq = true,
>> + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
>> + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
>> + .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
>> + .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
>> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
>> + .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
>> + SUN8I_H3_THS_INTS_SHUT_INT_0 |
>> + SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
>> + SUN8I_H3_THS_INTS_ALARM_OFF_0,
>> + .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
>> + SUN8I_H3_THS_TEMP_PERIOD(0x7),
>
> From what I've understood, ACQ regs are basically clock dividers. We
> should make a better job at explaining it :)
>
I agree, I will add this in the next version in the commit message.
>> +};
>> +
>> struct sun4i_gpadc_iio {
>> struct iio_dev *indio_dev;
>> struct completion completion;
>> @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
>> return 0;
>> }
>>
>> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
>> +{
>> + /* Disable ths interrupt */
>> + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
>> + /* Disable temperature sensor */
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
>> +
>> + return 0;
>> +}
>> +
>> static int sun4i_gpadc_runtime_suspend(struct device *dev)
>> {
>> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>> @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>> return info->data->sample_end(info);
>> }
>>
>> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
>> +{
>> + if (info->has_calibration_data[0])
>> + regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
>> + info->calibration_data[0]);
>> +
>> + if (info->has_calibration_data[1])
>> + regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
>> + info->calibration_data[1]);
>> +}
>> +
>> static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>> {
>> /* clkin = 6MHz */
>> @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>> return 0;
>> }
>>
>> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
>> +{
>> + u32 value;
>> + sunxi_calibrate(info);
>> +
>> + if (info->data->ctrl0_map)
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
>> + info->data->ctrl0_map);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
>> + info->data->ctrl2_map);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_STAT,
>> + info->data->irq_clear_map);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
>> + info->data->filter_map);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_INTC,
>> + info->data->irq_control_map);
>> +
>> + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
>> +
>> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
>> + info->data->sensor_en_map | value);
>> +
>> + return 0;
>> +}
>> +
>
> I'm a bit worried. Will all the sensors have the same sample start? Or
> will we need at some point also entries in info->data for register
> addresses, if they have ctrl2 or filter, etc...
> > Maybe we could define a sample_start for the h3 only and reuse-it if
> possible and if not declare another sample start? All depends if the
> sample start is most likely to change in the near future or not. If it
> is, then we should avoid adding a lot of variables in info->data and
> just go for a single sample_start per SoC.
>
for H3, A83T, H5, A64 we can use the same sample start. So I would use
here a H3 sample start function and reuse it.
A80 is special since it has no crtl0 register, thus it would get also
its own function.
H6 will need also an own sample start function (looking in the near future).
>> static int sun4i_gpadc_runtime_resume(struct device *dev)
>> {
>> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>> @@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
>> .compatible = "allwinner,sun8i-a33-ths",
>> .data = &sun8i_a33_gpadc_data,
>> },
>> + {
>> + .compatible = "allwinner,sun8i-h3-ths",
>> + .data = &sun8i_h3_ths_data,
>> + },
>> { /* sentinel */ }
>> };
>>
>> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
>> index 458f2159a95a..80b79c31cea3 100644
>> --- a/include/linux/mfd/sun4i-gpadc.h
>> +++ b/include/linux/mfd/sun4i-gpadc.h
>> @@ -91,7 +91,29 @@
>> #define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
>>
>> /* SUNXI_THS COMMON REGISTERS + DEFINES */
>> +#define SUN8I_H3_THS_CTRL0 0x00
>> +#define SUN8I_H3_THS_CTRL2 0x40
>> #define SUN8I_H3_THS_INTC 0x44
>> +#define SUN8I_H3_THS_STAT 0x48
>> +#define SUN8I_H3_THS_FILTER 0x70
>> +#define SUNXI_THS_CDATA_0_1 0x74
>> +#define SUNXI_THS_CDATA_2_3 0x78
>> +#define SUN8I_H3_THS_TDATA0 0x80
>> +
>> +#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))
>> +
>> +#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
>> +
>> +#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12))
>> +
>> +#define SUN8I_H3_THS_INTS_ALARM_INT_0 BIT(0)
>> +#define SUN8I_H3_THS_INTS_SHUT_INT_0 BIT(4)
>> +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8)
>> +#define SUN8I_H3_THS_INTS_ALARM_OFF_0 BIT(12)
>> +
>> +#define SUN8I_H3_THS_INTC_ALARM_INT_EN0 BIT(0)
>> +#define SUN8I_H3_THS_INTC_SHUT_INT_EN0 BIT(4)
>> +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8)
>>
>
> Personal taste here but I prefer the register bits to be defined just
> below the register address define.
>
> i.e.:
>
> #define SUN8I_H3_THS_CTRL2 0x40
> #define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16))
> #define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0)
>
> that way we know for which register we should use which constants.
>
> Quentin
>
I agree, this the better way to do it.
Thanks,
Philipp
>>
>>>> /* prevents concurrent reads of temperature and ADC */
>>>> struct mutex mutex;
>>>> struct thermal_zone_device *tzd;
>>>> @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>>>> struct resource *mem;
>>>> void __iomem *base;
>>>> int ret;
>>>> + struct nvmem_cell *cell;
>>>> + ssize_t cell_size;
>>>> + u64 *cell_data;
>>>> info->data = of_device_get_match_data(&pdev->dev);
>>>> if (!info->data)
>>>> @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>>>> if (IS_ERR(base))
>>>> return PTR_ERR(base);
>>>> + info->has_calibration_data[0] = false;
>>>> + info->has_calibration_data[1] = false;
>>>> +
>>>> + if (!info->data->supports_nvmem)
>>>> + goto no_nvmem;
>>>> +
>>>> + cell = nvmem_cell_get(&pdev->dev, "calibration");
>>>> + if (IS_ERR(cell)) {
>>>> + if (PTR_ERR(cell) == -EPROBE_DEFER)
>>>> + return PTR_ERR(cell);
>>>> + goto no_nvmem;
>>>
>>> goto considered evil ? :)
>>
>> this was a suggestion from Jonatan in version one, to make the code better
>> readable.
>
> Isn't
>
> if (info->data->supports_nvmem && IS_ERR(cell = nvmem_cell_get()))
>
> pretty much the same thing?
>
> Maxime
>
I would say :
if (info->data->supports_nvmem && !IS_ERR(cell = nvmem_cell_get())) is
the same.
This would require an else if statement like this:
else if (info->data->supports_nvmem && PTR_ERR(cell) == -EPROBE_DEFER)
return PTR_ERR(cell);
to avoid errors if the thermal sensor is probed before the sid driver.
Philipp
On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]> wrote:
>
>
> On 29.01.2018 10:52, Maxime Ripard wrote:
>>
>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>
>>> 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 3f83f6a27c74..9bb5cc29fec5 100644
>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> @@ -72,6 +72,13 @@
>>> };
>>> };
>>> + soc {
>>> + sid: eeprom@1c14000 {
>>> + compatible = "allwinner,sun8i-h3-sid";
>>> + reg = <0x01c14000 0x400>;
>>> + };
>>> + };
>>> +
>>
>>
>> Shouldn't you also use a nvmem-cells property to the THS node?
>>
>> Maxime
>>
>
> Oh seems like I forgot that.
> As related to the wiki [1] this should be 64 bit wide at the address 0x34. I
> will add that in the next version.
>
>
> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>
> Thanks,
> Philipp
>
Hi,
Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)
Thanks,
Kyle Evans
于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <[email protected]> 写到:
>On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]>
>wrote:
>>
>>
>> On 29.01.2018 10:52, Maxime Ripard wrote:
>>>
>>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>>
>>>> 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 3f83f6a27c74..9bb5cc29fec5 100644
>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> @@ -72,6 +72,13 @@
>>>> };
>>>> };
>>>> + soc {
>>>> + sid: eeprom@1c14000 {
>>>> + compatible = "allwinner,sun8i-h3-sid";
>>>> + reg = <0x01c14000 0x400>;
>>>> + };
>>>> + };
>>>> +
>>>
>>>
>>> Shouldn't you also use a nvmem-cells property to the THS node?
>>>
>>> Maxime
>>>
>>
>> Oh seems like I forgot that.
>> As related to the wiki [1] this should be 64 bit wide at the address
>0x34. I
>> will add that in the next version.
>>
>>
>> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>>
>> Thanks,
>> Philipp
>>
>
>Hi,
>
>Any chance this will see a v3 soon? I'm kind of interested in sid node
>for h3. =)
This patch is independent and can be easily sent out
by its own.
>
>Thanks,
>
>Kyle Evans
On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <[email protected]> wrote:
>
>
> 于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <[email protected]> 写到:
>>On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]>
>>wrote:
>>>
>>>
>>> On 29.01.2018 10:52, Maxime Ripard wrote:
>>>>
>>>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>>>
>>>>> 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 3f83f6a27c74..9bb5cc29fec5 100644
>>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>> @@ -72,6 +72,13 @@
>>>>> };
>>>>> };
>>>>> + soc {
>>>>> + sid: eeprom@1c14000 {
>>>>> + compatible = "allwinner,sun8i-h3-sid";
>>>>> + reg = <0x01c14000 0x400>;
>>>>> + };
>>>>> + };
>>>>> +
>>>>
>>>>
>>>> Shouldn't you also use a nvmem-cells property to the THS node?
>>>>
>>>> Maxime
>>>>
>>>
>>> Oh seems like I forgot that.
>>> As related to the wiki [1] this should be 64 bit wide at the address
>>0x34. I
>>> will add that in the next version.
>>>
>>>
>>> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>>>
>>> Thanks,
>>> Philipp
>>>
>>
>>Hi,
>>
>>Any chance this will see a v3 soon? I'm kind of interested in sid node
>>for h3. =)
>
> This patch is independent and can be easily sent out
> by its own.
>
Right- I had considered doing so, but wanted to make sure I wasn't
going to collide with this series if a v3 is imminent.
Hi Kyle,
I'm already working on a Version 3 of this patch series. Right now this
slowed down since I'm very busy and the ToDo-List is still very long.
My plan is to send out a version during this release cycle.
If you need it right now feel free to submit patches!
Philipp
On 19.04.2018 17:19, Kyle Evans wrote:
> On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <[email protected]> wrote:
>>
>>
>> 于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <[email protected]> 写到:
>>> On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]>
>>> wrote:
>>>>
>>>>
>>>> On 29.01.2018 10:52, Maxime Ripard wrote:
>>>>>
>>>>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>>>>
>>>>>> 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 3f83f6a27c74..9bb5cc29fec5 100644
>>>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>>> @@ -72,6 +72,13 @@
>>>>>> };
>>>>>> };
>>>>>> + soc {
>>>>>> + sid: eeprom@1c14000 {
>>>>>> + compatible = "allwinner,sun8i-h3-sid";
>>>>>> + reg = <0x01c14000 0x400>;
>>>>>> + };
>>>>>> + };
>>>>>> +
>>>>>
>>>>>
>>>>> Shouldn't you also use a nvmem-cells property to the THS node?
>>>>>
>>>>> Maxime
>>>>>
>>>>
>>>> Oh seems like I forgot that.
>>>> As related to the wiki [1] this should be 64 bit wide at the address
>>> 0x34. I
>>>> will add that in the next version.
>>>>
>>>>
>>>> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>>>>
>>>> Thanks,
>>>> Philipp
>>>>
>>>
>>> Hi,
>>>
>>> Any chance this will see a v3 soon? I'm kind of interested in sid node
>>> for h3. =)
>>
>> This patch is independent and can be easily sent out
>> by its own.
>>
>
> Right- I had considered doing so, but wanted to make sure I wasn't
> going to collide with this series if a v3 is imminent.
>
Hi,
Le vendredi 20 avril 2018 à 11:35 +0200, Philipp Rossak a écrit :
> Hi Kyle,
>
> I'm already working on a Version 3 of this patch series. Right now this
> slowed down since I'm very busy and the ToDo-List is still very long.
>
> My plan is to send out a version during this release cycle.
>
> If you need it right now feel free to submit patches!
I just came around this patch while testing SID support for H3, that is
not enabled in the dts at this point despite driver support.
Maxime, would it make sense to merge this patch as-is, without support
for the thermal calibration section, since the two aspects seem rather
independent (and some other SoCs also have a SID node without linkage
for thermal support anyway)?
Cheers,
Paul
> Philipp
>
> On 19.04.2018 17:19, Kyle Evans wrote:
> > On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <[email protected]> wrote:
> > >
> > >
> > > 于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <[email protected]> 写到:
> > > > On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]>
> > > > wrote:
> > > > >
> > > > >
> > > > > On 29.01.2018 10:52, Maxime Ripard wrote:
> > > > > >
> > > > > > On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> > > > > > >
> > > > > > > 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 3f83f6a27c74..9bb5cc29fec5 100644
> > > > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > @@ -72,6 +72,13 @@
> > > > > > > };
> > > > > > > };
> > > > > > > + soc {
> > > > > > > + sid: eeprom@1c14000 {
> > > > > > > + compatible = "allwinner,sun8i-h3-sid";
> > > > > > > + reg = <0x01c14000 0x400>;
> > > > > > > + };
> > > > > > > + };
> > > > > > > +
> > > > > >
> > > > > >
> > > > > > Shouldn't you also use a nvmem-cells property to the THS node?
> > > > > >
> > > > > > Maxime
> > > > > >
> > > > >
> > > > > Oh seems like I forgot that.
> > > > > As related to the wiki [1] this should be 64 bit wide at the address
> > > >
> > > > 0x34. I
> > > > > will add that in the next version.
> > > > >
> > > > >
> > > > > [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
> > > > >
> > > > > Thanks,
> > > > > Philipp
> > > > >
> > > >
> > > > Hi,
> > > >
> > > > Any chance this will see a v3 soon? I'm kind of interested in sid node
> > > > for h3. =)
> > >
> > > This patch is independent and can be easily sent out
> > > by its own.
> > >
> >
> > Right- I had considered doing so, but wanted to make sure I wasn't
> > going to collide with this series if a v3 is imminent.
> >
>
>
--
Developer of free digital technology and hardware support.
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
On Tue, 24 Jul 2018 19:19:54 +0200
Paul Kocialkowski <[email protected]> wrote:
> Hi,
>
> Le vendredi 20 avril 2018 ? 11:35 +0200, Philipp Rossak a ?crit :
> > Hi Kyle,
> >
> > I'm already working on a Version 3 of this patch series. Right now this
> > slowed down since I'm very busy and the ToDo-List is still very long.
> >
> > My plan is to send out a version during this release cycle.
> >
> > If you need it right now feel free to submit patches!
>
> I just came around this patch while testing SID support for H3, that is
> not enabled in the dts at this point despite driver support.
>
> Maxime, would it make sense to merge this patch as-is, without support
> for the thermal calibration section, since the two aspects seem rather
> independent (and some other SoCs also have a SID node without linkage
> for thermal support anyway)?
>
> Cheers,
>
> Paul
Hello Paul,
I've sent a serie yesterday for SID on A64/H3/H5.
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=761
Cheers,
> > Philipp
> >
> > On 19.04.2018 17:19, Kyle Evans wrote:
> > > On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <[email protected]> wrote:
> > > >
> > > >
> > > > ? 2018?4?19? GMT+08:00 ??11:11:22, Kyle Evans <[email protected]> ??:
> > > > > On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]>
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > On 29.01.2018 10:52, Maxime Ripard wrote:
> > > > > > >
> > > > > > > On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> > > > > > > >
> > > > > > > > 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 3f83f6a27c74..9bb5cc29fec5 100644
> > > > > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > @@ -72,6 +72,13 @@
> > > > > > > > };
> > > > > > > > };
> > > > > > > > + soc {
> > > > > > > > + sid: eeprom@1c14000 {
> > > > > > > > + compatible = "allwinner,sun8i-h3-sid";
> > > > > > > > + reg = <0x01c14000 0x400>;
> > > > > > > > + };
> > > > > > > > + };
> > > > > > > > +
> > > > > > >
> > > > > > >
> > > > > > > Shouldn't you also use a nvmem-cells property to the THS node?
> > > > > > >
> > > > > > > Maxime
> > > > > > >
> > > > > >
> > > > > > Oh seems like I forgot that.
> > > > > > As related to the wiki [1] this should be 64 bit wide at the address
> > > > >
> > > > > 0x34. I
> > > > > > will add that in the next version.
> > > > > >
> > > > > >
> > > > > > [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
> > > > > >
> > > > > > Thanks,
> > > > > > Philipp
> > > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > Any chance this will see a v3 soon? I'm kind of interested in sid node
> > > > > for h3. =)
> > > >
> > > > This patch is independent and can be easily sent out
> > > > by its own.
> > > >
> > >
> > > Right- I had considered doing so, but wanted to make sure I wasn't
> > > going to collide with this series if a v3 is imminent.
> > >
> >
> >
> --
> Developer of free digital technology and hardware support.
>
> Website: https://www.paulk.fr/
> Coding blog: https://code.paulk.fr/
> Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
--
Emmanuel Vadot <[email protected]> <[email protected]>
Hi,
On Wed, 2018-07-25 at 11:05 +0200, Emmanuel Vadot wrote:
[...]
> Hello Paul,
>
> I've sent a serie yesterday for SID on A64/H3/H5.
> https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=761
Awesome, thanks for taking care of that :)
Cheers,
Paul
> > > On 19.04.2018 17:19, Kyle Evans wrote:
> > > > On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <[email protected]> wrote:
> > > > >
> > > > >
> > > > > ? 2018?4?19? GMT+08:00 ??11:11:22, Kyle Evans <[email protected]> ??:
> > > > > > On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <[email protected]>
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 29.01.2018 10:52, Maxime Ripard wrote:
> > > > > > > >
> > > > > > > > On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> > > > > > > > >
> > > > > > > > > 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 3f83f6a27c74..9bb5cc29fec5 100644
> > > > > > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > > @@ -72,6 +72,13 @@
> > > > > > > > > };
> > > > > > > > > };
> > > > > > > > > + soc {
> > > > > > > > > + sid: eeprom@1c14000 {
> > > > > > > > > + compatible = "allwinner,sun8i-h3-sid";
> > > > > > > > > + reg = <0x01c14000 0x400>;
> > > > > > > > > + };
> > > > > > > > > + };
> > > > > > > > > +
> > > > > > > >
> > > > > > > >
> > > > > > > > Shouldn't you also use a nvmem-cells property to the THS node?
> > > > > > > >
> > > > > > > > Maxime
> > > > > > > >
> > > > > > >
> > > > > > > Oh seems like I forgot that.
> > > > > > > As related to the wiki [1] this should be 64 bit wide at the address
> > > > > >
> > > > > > 0x34. I
> > > > > > > will add that in the next version.
> > > > > > >
> > > > > > >
> > > > > > > [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Philipp
> > > > > > >
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Any chance this will see a v3 soon? I'm kind of interested in sid node
> > > > > > for h3. =)
> > > > >
> > > > > This patch is independent and can be easily sent out
> > > > > by its own.
> > > > >
> > > >
> > > > Right- I had considered doing so, but wanted to make sure I wasn't
> > > > going to collide with this series if a v3 is imminent.
> > > >
> > >
> > >
> >
> > --
> > Developer of free digital technology and hardware support.
> >
> > Website: https://www.paulk.fr/
> > Coding blog: https://code.paulk.fr/
> > Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
>
>
--
Developer of free digital technology and hardware support.
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/