Hello everyone,
this is v2 of my THS patchset
Changelog:
* Some stylistic changes
* devm_reset_control_get_optional -> devm_reset_control_get
* Added the clk-h3-ths clock driver
- Note: A23/A33/A83T do not have a separate clock, H3 seems to be the first
(and only?) SoC with it
- Because of this, I moved the clock init code to the H3-specific init
function.
* Use the nvmem cell abstraction instead of accessing the configuration memory directly
* Use the IRQ line (and fixed incorrect interrupt number in the DTS)
* Renamed to sun8i_ths
Ad the "magical constants": what I meant is that altough the datasheet explains
what they are, it does not explain how to pick their values. "ADC" and "Sensor"
"acquire time" are also not exactly the most helpful descriptions.
Anyway, I changed the values such as the final sampling rate is about 1Hz.
Josef Gajdusek (5):
ARM: dts: sun8i: Add SID node
clk: sunxi: Add driver for the H3 THS clock
thermal: Add a driver for the Allwinner THS sensor
dt-bindings: document sun8i_ths
ARM: dts: sun8i: Add THS node to the H3 .dtsi
Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
.../devicetree/bindings/thermal/sun8i-ths.txt | 31 ++
arch/arm/boot/dts/sun8i-h3.dtsi | 40 +++
drivers/clk/sunxi/Makefile | 1 +
drivers/clk/sunxi/clk-h3-ths.c | 98 ++++++
drivers/thermal/Kconfig | 7 +
drivers/thermal/Makefile | 1 +
drivers/thermal/sun8i_ths.c | 365 +++++++++++++++++++++
8 files changed, 544 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-ths.txt
create mode 100644 drivers/clk/sunxi/clk-h3-ths.c
create mode 100644 drivers/thermal/sun8i_ths.c
--
2.4.10
Add a node describing the Security ID memory to the
Allwinner H3 .dtsi file.
Signed-off-by: Josef Gajdusek <[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 0faa38a..58de718 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -359,6 +359,13 @@
#size-cells = <0>;
};
+ sid: eeprom@01c14000 {
+ compatible = "allwinner,sun4i-a10-sid";
+ reg = <0x01c14000 0x400>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ };
+
usbphy: phy@01c19400 {
compatible = "allwinner,sun8i-h3-usb-phy";
reg = <0x01c19400 0x2c>,
--
2.4.10
This patch adds a driver for the THS clock which is present on the
Allwinner H3.
Signed-off-by: Josef Gajdusek <[email protected]>
---
Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
drivers/clk/sunxi/Makefile | 1 +
drivers/clk/sunxi/clk-h3-ths.c | 98 +++++++++++++++++++++++
3 files changed, 100 insertions(+)
create mode 100644 drivers/clk/sunxi/clk-h3-ths.c
diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 23e7bce..6d63b35 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -73,6 +73,7 @@ Required properties:
"allwinner,sun8i-h3-usb-clk" - for usb gates + resets on H3
"allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80
"allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80
+ "allwinner,sun8i-h3-ths-clk" - for THS on H3
Required properties for all clocks:
- reg : shall be the control register address for the clock.
diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index f520af6..1bf8e1c 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -8,6 +8,7 @@ obj-y += clk-a10-hosc.o
obj-y += clk-a10-mod1.o
obj-y += clk-a10-pll2.o
obj-y += clk-a20-gmac.o
+obj-y += clk-h3-ths.o
obj-y += clk-mod0.o
obj-y += clk-simple-gates.o
obj-y += clk-sun8i-bus-gates.o
diff --git a/drivers/clk/sunxi/clk-h3-ths.c b/drivers/clk/sunxi/clk-h3-ths.c
new file mode 100644
index 0000000..663afc0
--- /dev/null
+++ b/drivers/clk/sunxi/clk-h3-ths.c
@@ -0,0 +1,98 @@
+/*
+ * Sunxi THS clock driver
+ *
+ * Copyright (C) 2015 Josef Gajdusek
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define SUN8I_H3_THS_CLK_ENABLE 31
+#define SUN8I_H3_THS_CLK_DIVIDER_SHIFT 0
+#define SUN8I_H3_THS_CLK_DIVIDER_WIDTH 2
+
+static DEFINE_SPINLOCK(sun8i_h3_ths_clk_lock);
+
+static const struct clk_div_table sun8i_h3_ths_clk_table[] __initconst = {
+ { .val = 0, .div = 1 },
+ { .val = 1, .div = 2 },
+ { .val = 2, .div = 4 },
+ { .val = 3, .div = 6 },
+ { } /* sentinel */
+};
+
+static void __init sun8i_h3_ths_clk_setup(struct device_node *node)
+{
+ struct clk *clk;
+ struct clk_gate *gate;
+ struct clk_divider *div;
+ const char *parent;
+ const char *clk_name = node->name;
+ void __iomem *reg;
+ int err;
+
+ reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+
+ if (IS_ERR(reg))
+ return;
+
+ gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+ if (!gate)
+ goto err_unmap;
+
+ div = kzalloc(sizeof(*gate), GFP_KERNEL);
+ if (!div)
+ goto err_gate_free;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+ parent = of_clk_get_parent_name(node, 0);
+
+ gate->reg = reg;
+ gate->bit_idx = SUN8I_H3_THS_CLK_ENABLE;
+ gate->lock = &sun8i_h3_ths_clk_lock;
+
+ div->reg = reg;
+ div->shift = SUN8I_H3_THS_CLK_DIVIDER_SHIFT;
+ div->width = SUN8I_H3_THS_CLK_DIVIDER_WIDTH;
+ div->table = sun8i_h3_ths_clk_table;
+ div->lock = &sun8i_h3_ths_clk_lock;
+
+ clk = clk_register_composite(NULL, clk_name, &parent, 1,
+ NULL, NULL,
+ &div->hw, &clk_divider_ops,
+ &gate->hw, &clk_gate_ops,
+ CLK_SET_RATE_PARENT);
+
+ if (IS_ERR(clk))
+ goto err_div_free;
+
+ err = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+ if (err)
+ goto err_unregister_clk;
+
+ return;
+
+err_unregister_clk:
+ clk_unregister(clk);
+err_gate_free:
+ kfree(gate);
+err_div_free:
+ kfree(div);
+err_unmap:
+ iounmap(reg);
+}
+
+CLK_OF_DECLARE(sun8i_h3_ths_clk, "allwinner,sun8i-h3-ths-clk",
+ sun8i_h3_ths_clk_setup);
--
2.4.10
This patch adds support for the Sunxi thermal sensor on the Allwinner H3.
Should be easily extendable for the A33/A83T/... as they have similar but
not completely identical sensors.
Signed-off-by: Josef Gajdusek <[email protected]>
---
drivers/thermal/Kconfig | 7 +
drivers/thermal/Makefile | 1 +
drivers/thermal/sun8i_ths.c | 365 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 373 insertions(+)
create mode 100644 drivers/thermal/sun8i_ths.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index c463c89..2b41147 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -365,6 +365,13 @@ config INTEL_PCH_THERMAL
Thermal reporting device will provide temperature reading,
programmable trip points and other information.
+config SUN8I_THS
+ tristate "sun8i THS driver"
+ depends on MACH_SUN8I
+ depends on OF
+ help
+ Enable this to support thermal reporting on some newer Allwinner SoCs.
+
menu "Texas Instruments thermal drivers"
depends on ARCH_HAS_BANDGAP || COMPILE_TEST
source "drivers/thermal/ti-soc-thermal/Kconfig"
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index cfae6a6..227e1a1 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -48,3 +48,4 @@ obj-$(CONFIG_INTEL_PCH_THERMAL) += intel_pch_thermal.o
obj-$(CONFIG_ST_THERMAL) += st/
obj-$(CONFIG_TEGRA_SOCTHERM) += tegra_soctherm.o
obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
+obj-$(CONFIG_SUN8I_THS) += sun8i_ths.o
diff --git a/drivers/thermal/sun8i_ths.c b/drivers/thermal/sun8i_ths.c
new file mode 100644
index 0000000..2c976ac
--- /dev/null
+++ b/drivers/thermal/sun8i_ths.c
@@ -0,0 +1,365 @@
+/*
+ * Sunxi THS driver
+ *
+ * Copyright (C) 2015 Josef Gajdusek
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/thermal.h>
+
+#define THS_H3_CTRL0 0x00
+#define THS_H3_CTRL1 0x04
+#define THS_H3_CDAT 0x14
+#define THS_H3_CTRL2 0x40
+#define THS_H3_INT_CTRL 0x44
+#define THS_H3_STAT 0x48
+#define THS_H3_ALARM_CTRL 0x50
+#define THS_H3_SHUTDOWN_CTRL 0x60
+#define THS_H3_FILTER 0x70
+#define THS_H3_CDATA 0x74
+#define THS_H3_DATA 0x80
+
+#define THS_H3_CTRL0_SENSOR_ACQ0_OFFS 0
+#define THS_H3_CTRL0_SENSOR_ACQ0(x) \
+ ((x) << THS_H3_CTRL0_SENSOR_ACQ0_OFFS)
+#define THS_H3_CTRL1_ADC_CALI_EN_OFFS 17
+#define THS_H3_CTRL1_ADC_CALI_EN \
+ BIT(THS_H3_CTRL1_ADC_CALI_EN_OFFS)
+#define THS_H3_CTRL1_OP_BIAS_OFFS 20
+#define THS_H3_CTRL1_OP_BIAS(x) \
+ ((x) << THS_H3_CTRL1_OP_BIAS_OFFS)
+#define THS_H3_CTRL2_SENSE_EN_OFFS 0
+#define THS_H3_CTRL2_SENSE_EN \
+ BIT(THS_H3_CTRL2_SENSE_EN_OFFS)
+#define THS_H3_CTRL2_SENSOR_ACQ1_OFFS 16
+#define THS_H3_CTRL2_SENSOR_ACQ1(x) \
+ ((x) << THS_H3_CTRL2_SENSOR_ACQ1_OFFS)
+
+#define THS_H3_INT_CTRL_ALARM_INT_EN_OFFS 0
+#define THS_H3_INT_CTRL_ALARM_INT_EN \
+ BIT(THS_H3_INT_CTRL_ALARM_INT_EN_OFFS)
+#define THS_H3_INT_CTRL_SHUT_INT_EN_OFFS 4
+#define THS_H3_INT_CTRL_SHUT_INT_EN \
+ BIT(THS_H3_INT_CTRL_SHUT_INT_EN_OFFS)
+#define THS_H3_INT_CTRL_DATA_IRQ_EN_OFFS 8
+#define THS_H3_INT_CTRL_DATA_IRQ_EN \
+ BIT(THS_H3_INT_CTRL_DATA_IRQ_EN_OFFS)
+#define THS_H3_INT_CTRL_THERMAL_PER_OFFS 12
+#define THS_H3_INT_CTRL_THERMAL_PER(x) \
+ ((x) << THS_H3_INT_CTRL_THERMAL_PER_OFFS)
+
+#define THS_H3_STAT_ALARM_INT_STS_OFFS 0
+#define THS_H3_STAT_ALARM_INT_STS \
+ BIT(THS_H3_STAT_ALARM_INT_STS_OFFS)
+#define THS_H3_STAT_SHUT_INT_STS_OFFS 4
+#define THS_H3_STAT_SHUT_INT_STS \
+ BIT(THS_H3_STAT_SHUT_INT_STS_OFFS)
+#define THS_H3_STAT_DATA_IRQ_STS_OFFS 8
+#define THS_H3_STAT_DATA_IRQ_STS \
+ BIT(THS_H3_STAT_DATA_IRQ_STS_OFFS)
+#define THS_H3_STAT_ALARM_OFF_STS_OFFS 12
+#define THS_H3_STAT_ALARM_OFF_STS \
+ BIT(THS_H3_STAT_ALARM_OFF_STS_OFFS)
+
+#define THS_H3_ALARM_CTRL_ALARM0_T_HYST_OFFS 0
+#define THS_H3_ALARM_CTRL_ALARM0_T_HYST(x) \
+ ((x) << THS_H3_ALARM_CTRL_ALARM0_T_HYST_OFFS)
+#define THS_H3_ALARM_CTRL_ALARM0_T_HOT_OFFS 16
+#define THS_H3_ALARM_CTRL_ALARM0_T_HOT(x) \
+ ((x) << THS_H3_ALARM_CTRL_ALARM0_T_HOT_OFFS)
+
+#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT_OFFS 16
+#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT(x) \
+ ((x) << THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT_OFFS)
+
+#define THS_H3_FILTER_TYPE_OFFS 0
+#define THS_H3_FILTER_TYPE(x) \
+ ((x) << THS_H3_FILTER_TYPE_OFFS)
+#define THS_H3_FILTER_EN_OFFS 2
+#define THS_H3_FILTER_EN \
+ BIT(THS_H3_FILTER_EN_OFFS)
+
+#define THS_H3_CTRL0_SENSOR_ACQ0_VALUE 0xff
+#define THS_H3_INT_CTRL_THERMAL_PER_VALUE 0x79
+#define THS_H3_FILTER_TYPE_VALUE 0x2
+#define THS_H3_CTRL2_SENSOR_ACQ1_VALUE 0x3f
+
+struct sun8i_ths_data {
+ struct sun8i_ths_type *type;
+ struct reset_control *reset;
+ struct clk *clk;
+ struct clk *busclk;
+ void __iomem *regs;
+ struct nvmem_cell *calcell;
+ struct platform_device *pdev;
+ struct thermal_zone_device *tzd;
+};
+
+struct sun8i_ths_type {
+ int (*init)(struct platform_device *, struct sun8i_ths_data *);
+ int (*get_temp)(struct sun8i_ths_data *, int *out);
+ void (*irq)(struct sun8i_ths_data *);
+ void (*deinit)(struct sun8i_ths_data *);
+};
+
+/* Formula and parameters from the Allwinner 3.4 kernel */
+static int sun8i_ths_reg_to_temperature(s32 reg, int divisor, int constant)
+{
+ return constant - (reg * 1000000) / divisor;
+}
+
+static int sun8i_ths_get_temp(void *_data, int *out)
+{
+ struct sun8i_ths_data *data = _data;
+
+ return data->type->get_temp(data, out);
+}
+
+static irqreturn_t sun8i_ths_irq_thread(int irq, void *_data)
+{
+ struct sun8i_ths_data *data = _data;
+
+ data->type->irq(data);
+ thermal_zone_device_update(data->tzd);
+
+ return IRQ_HANDLED;
+}
+
+static int sun8i_ths_h3_init(struct platform_device *pdev,
+ struct sun8i_ths_data *data)
+{
+ int ret;
+ size_t callen;
+ s32 *caldata;
+
+ data->busclk = devm_clk_get(&pdev->dev, "ahb");
+ if (IS_ERR(data->busclk)) {
+ ret = PTR_ERR(data->busclk);
+ dev_err(&pdev->dev, "failed to get ahb clk: %d\n", ret);
+ return ret;
+ }
+
+ data->clk = devm_clk_get(&pdev->dev, "ths");
+ if (IS_ERR(data->clk)) {
+ ret = PTR_ERR(data->clk);
+ dev_err(&pdev->dev, "failed to get ths clk: %d\n", ret);
+ return ret;
+ }
+
+ data->reset = devm_reset_control_get(&pdev->dev, "ahb");
+ if (IS_ERR(data->reset)) {
+ ret = PTR_ERR(data->reset);
+ dev_err(&pdev->dev, "failed to get reset: %d\n", ret);
+ return ret;
+ }
+
+ if (data->calcell) {
+ caldata = nvmem_cell_read(data->calcell, &callen);
+ if (IS_ERR(caldata))
+ return PTR_ERR(caldata);
+ writel(be32_to_cpu(*caldata), data->regs + THS_H3_CDATA);
+ kfree(caldata);
+ }
+
+ ret = clk_prepare_enable(data->busclk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable bus clk: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(data->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable ths clk: %d\n", ret);
+ goto err_disable_bus;
+ }
+
+ ret = reset_control_deassert(data->reset);
+ if (ret) {
+ dev_err(&pdev->dev, "reset deassert failed: %d\n", ret);
+ goto err_disable_ths;
+ }
+
+ /* The final sample period is calculated as follows:
+ * (THERMAL_PER + 1) * 4096 / f_clk * 2^(FILTER_TYPE + 1)
+ *
+ * This results to about 1Hz with these settings.
+ */
+ ret = clk_set_rate(data->clk, 4000000);
+ if (ret)
+ goto err_disable_ths;
+ writel(THS_H3_CTRL0_SENSOR_ACQ0(THS_H3_CTRL0_SENSOR_ACQ0_VALUE),
+ data->regs + THS_H3_CTRL0);
+ writel(THS_H3_INT_CTRL_THERMAL_PER(THS_H3_INT_CTRL_THERMAL_PER_VALUE) |
+ THS_H3_INT_CTRL_DATA_IRQ_EN,
+ data->regs + THS_H3_INT_CTRL);
+ writel(THS_H3_FILTER_EN | THS_H3_FILTER_TYPE(THS_H3_FILTER_TYPE_VALUE),
+ data->regs + THS_H3_FILTER);
+ writel(THS_H3_CTRL2_SENSOR_ACQ1(THS_H3_CTRL2_SENSOR_ACQ1_VALUE) |
+ THS_H3_CTRL2_SENSE_EN,
+ data->regs + THS_H3_CTRL2);
+ return 0;
+
+err_disable_ths:
+ clk_disable_unprepare(data->clk);
+err_disable_bus:
+ clk_disable_unprepare(data->busclk);
+
+ return ret;
+}
+
+static int sun8i_ths_h3_get_temp(struct sun8i_ths_data *data, int *out)
+{
+ int val = readl(data->regs + THS_H3_DATA);
+ *out = sun8i_ths_reg_to_temperature(val, 8253, 217000);
+ return 0;
+}
+
+static void sun8i_ths_h3_irq(struct sun8i_ths_data *data)
+{
+ writel(THS_H3_STAT_DATA_IRQ_STS |
+ THS_H3_STAT_ALARM_INT_STS |
+ THS_H3_STAT_ALARM_OFF_STS |
+ THS_H3_STAT_SHUT_INT_STS,
+ data->regs + THS_H3_STAT);
+}
+
+static void sun8i_ths_h3_deinit(struct sun8i_ths_data *data)
+{
+ reset_control_assert(data->reset);
+ clk_disable_unprepare(data->clk);
+ clk_disable_unprepare(data->busclk);
+}
+
+static const struct thermal_zone_of_device_ops sun8i_ths_thermal_ops = {
+ .get_temp = sun8i_ths_get_temp,
+};
+
+static const struct sun8i_ths_type sun8i_ths_device_h3 = {
+ .init = sun8i_ths_h3_init,
+ .get_temp = sun8i_ths_h3_get_temp,
+ .irq = sun8i_ths_h3_irq,
+ .deinit = sun8i_ths_h3_deinit,
+};
+
+static const struct of_device_id sun8i_ths_id_table[] = {
+ {
+ .compatible = "allwinner,sun8i-h3-ths",
+ .data = &sun8i_ths_device_h3,
+ },
+ {
+ /* sentinel */
+ },
+};
+MODULE_DEVICE_TABLE(of, sun8i_ths_id_table);
+
+static int sun8i_ths_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *match;
+ struct sun8i_ths_data *data;
+ struct resource *res;
+ int ret;
+ int irq;
+
+ match = of_match_node(sun8i_ths_id_table, np);
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->type = (struct sun8i_ths_type *)match->data;
+ data->pdev = pdev;
+
+ data->calcell = devm_nvmem_cell_get(&pdev->dev, "calibration");
+ if (IS_ERR(data->calcell)) {
+ if (PTR_ERR(data->calcell) == -EPROBE_DEFER)
+ return PTR_ERR(data->calcell);
+ data->calcell = NULL; /* No calibration register */
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ data->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(data->regs)) {
+ ret = PTR_ERR(data->regs);
+ dev_err(&pdev->dev,
+ "failed to ioremap THS registers: %d\n", ret);
+ return ret;
+ }
+
+ 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,
+ sun8i_ths_irq_thread, IRQF_ONESHOT,
+ dev_name(&pdev->dev), data);
+ if (ret)
+ return ret;
+
+ ret = data->type->init(pdev, data);
+ if (ret)
+ return ret;
+
+ data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
+ &sun8i_ths_thermal_ops);
+ if (IS_ERR(data->tzd)) {
+ ret = PTR_ERR(data->tzd);
+ dev_err(&pdev->dev, "failed to register thermal zone: %d\n",
+ ret);
+ goto err_deinit;
+ }
+
+ platform_set_drvdata(pdev, data);
+ return 0;
+
+err_deinit:
+ data->type->deinit(data);
+ return ret;
+}
+
+static int sun8i_ths_remove(struct platform_device *pdev)
+{
+ struct sun8i_ths_data *data = platform_get_drvdata(pdev);
+
+ thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd);
+ data->type->deinit(data);
+ return 0;
+}
+
+static struct platform_driver sun8i_ths_driver = {
+ .probe = sun8i_ths_probe,
+ .remove = sun8i_ths_remove,
+ .driver = {
+ .name = "sun8i_ths",
+ .of_match_table = sun8i_ths_id_table,
+ },
+};
+
+module_platform_driver(sun8i_ths_driver);
+
+MODULE_AUTHOR("Josef Gajdusek <[email protected]>");
+MODULE_DESCRIPTION("Sunxi THS driver");
+MODULE_LICENSE("GPL v2");
--
2.4.10
This patch adds the binding documentation for the sun8i_ths driver
Signed-off-by: Josef Gajdusek <[email protected]>
---
.../devicetree/bindings/thermal/sun8i-ths.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-ths.txt
diff --git a/Documentation/devicetree/bindings/thermal/sun8i-ths.txt b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
new file mode 100644
index 0000000..67056bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
@@ -0,0 +1,31 @@
+* sun8i THS
+
+Required properties:
+- compatible : "allwinner,sun8i-h3-ths"
+- reg : Address range of the thermal registers and location of the calibration
+ value
+- resets : Must contain an entry for each entry in reset-names.
+ see ../reset/reset.txt for details
+- reset-names : Must include the name "ahb"
+- clocks : Must contain an entry for each entry in clock-names.
+- clock-names : Must contain "ahb" for the bus gate and "ths" for the THS
+ clock
+
+Optional properties:
+- nvmem-cells : Must contain an entry for each entry in nvmem-cell-names
+- nvmem-cell-names : Must contain "calibration" for the cell containing the
+ temperature calibration cell, if available
+
+Example:
+ths: ths@01c25000 {
+ #thermal-sensor-cells = <0>;
+ compatible = "allwinner,sun8i-h3-ths";
+ reg = <0x01c25000 0x88>, <0x01c14234 0x4>;
+ interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
+ resets = <&bus_rst 136>;
+ reset-names = "ahb";
+ clocks = <&bus_gates 72>, <&ths_clk>;
+ clock-names = "ahb", "ths";
+ nvmem-cells = <&ths_calibration>;
+ nvmem-cell-names = "calibration";
+};
--
2.4.10
This patch adds nodes for the THS driver and the THS clock to the Allwinner
H3 .dtsi file.
Signed-off-by: Josef Gajdusek <[email protected]>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 58de718..48500d4 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -77,6 +77,14 @@
};
};
+ thermal-zones {
+ cpu_thermal: cpu_thermal {
+ polling-delay-passive = <1000>;
+ polling-delay = <5000>;
+ thermal-sensors = <&ths 0>;
+ };
+ };
+
timer {
compatible = "arm,armv7-timer";
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
@@ -236,6 +244,14 @@
"ahb1_ephy", "ahb1_dbg";
};
+ ths_clk: clk@01c20074 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun8i-h3-ths-clk";
+ reg = <0x01c20074 0x4>;
+ clocks = <&osc24M>;
+ clock-output-names = "ths";
+ };
+
mmc0_clk: clk@01c20088 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-a10-mmc-clk";
@@ -364,6 +380,10 @@
reg = <0x01c14000 0x400>;
#address-cells = <1>;
#size-cells = <1>;
+
+ ths_calibration: calib@234 {
+ reg = <0x234 0x4>;
+ };
};
usbphy: phy@01c19400 {
@@ -529,6 +549,19 @@
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
};
+ ths: ths@01c25000 {
+ #thermal-sensor-cells = <0>;
+ compatible = "allwinner,sun8i-h3-ths";
+ reg = <0x01c25000 0x88>;
+ interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+ resets = <&bus_rst 104>;
+ reset-names = "ahb";
+ clocks = <&bus_gates 72>, <&ths_clk>;
+ clock-names = "ahb", "ths";
+ nvmem-cells = <&ths_calibration>;
+ nvmem-cell-names = "calibration";
+ };
+
uart0: serial@01c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
--
2.4.10
On Mon, Nov 23, 2015 at 09:02:49AM +0100, Josef Gajdusek wrote:
> This patch adds a driver for the THS clock which is present on the
> Allwinner H3.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
Hello
Just a minor comment below.
> +static void __init sun8i_h3_ths_clk_setup(struct device_node *node)
> +{
> + struct clk *clk;
> + struct clk_gate *gate;
> + struct clk_divider *div;
> + const char *parent;
> + const char *clk_name = node->name;
> + void __iomem *reg;
> + int err;
> +
> + reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +
> + if (IS_ERR(reg))
> + return;
> +
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + goto err_unmap;
> +
> + div = kzalloc(sizeof(*gate), GFP_KERNEL);
copy/paste error, you mean sizeof(*div) ?
> + if (!div)
> + goto err_gate_free;
> +
> + of_property_read_string(node, "clock-output-names", &clk_name);
Regards
On Mon, Nov 23, 2015 at 4:02 PM, Josef Gajdusek <[email protected]> wrote:
> This patch adds the binding documentation for the sun8i_ths driver
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> .../devicetree/bindings/thermal/sun8i-ths.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-ths.txt
>
> diff --git a/Documentation/devicetree/bindings/thermal/sun8i-ths.txt b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
> new file mode 100644
> index 0000000..67056bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
> @@ -0,0 +1,31 @@
> +* sun8i THS
> +
> +Required properties:
> +- compatible : "allwinner,sun8i-h3-ths"
> +- reg : Address range of the thermal registers and location of the calibration
> + value
You are now using nvmem for the calibration data. You don't need the second
entry.
> +- resets : Must contain an entry for each entry in reset-names.
> + see ../reset/reset.txt for details
> +- reset-names : Must include the name "ahb"
> +- clocks : Must contain an entry for each entry in clock-names.
> +- clock-names : Must contain "ahb" for the bus gate and "ths" for the THS
> + clock
> +
> +Optional properties:
> +- nvmem-cells : Must contain an entry for each entry in nvmem-cell-names
> +- nvmem-cell-names : Must contain "calibration" for the cell containing the
> + temperature calibration cell, if available
> +
> +Example:
> +ths: ths@01c25000 {
> + #thermal-sensor-cells = <0>;
> + compatible = "allwinner,sun8i-h3-ths";
> + reg = <0x01c25000 0x88>, <0x01c14234 0x4>;
Same here.
ChenYu
> + interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&bus_rst 136>;
> + reset-names = "ahb";
> + clocks = <&bus_gates 72>, <&ths_clk>;
> + clock-names = "ahb", "ths";
> + nvmem-cells = <&ths_calibration>;
> + nvmem-cell-names = "calibration";
> +};
> --
> 2.4.10
>
On Mon, 2015-11-23 at 09:02 +0100, Josef Gajdusek wrote:
> This patch adds a driver for the THS clock which is present on the
> Allwinner H3.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
> drivers/clk/sunxi/Makefile | 1 +
> drivers/clk/sunxi/clk-h3-ths.c | 98
> +++++++++++++++++++++++
> 3 files changed, 100 insertions(+)
> create mode 100644 drivers/clk/sunxi/clk-h3-ths.c
>
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt
> b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 23e7bce..6d63b35 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -73,6 +73,7 @@ Required properties:
> "allwinner,sun8i-h3-usb-clk" - for usb gates + resets on H3
> "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets
> on A80
> "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates +
> resets on A80
> + "allwinner,sun8i-h3-ths-clk" - for THS on H3
>
> Required properties for all clocks:
> - reg : shall be the control register address for the clock.
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index f520af6..1bf8e1c 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -8,6 +8,7 @@ obj-y += clk-a10-hosc.o
> obj-y += clk-a10-mod1.o
> obj-y += clk-a10-pll2.o
> obj-y += clk-a20-gmac.o
> +obj-y += clk-h3-ths.o
> obj-y += clk-mod0.o
> obj-y += clk-simple-gates.o
> obj-y += clk-sun8i-bus-gates.o
> diff --git a/drivers/clk/sunxi/clk-h3-ths.c b/drivers/clk/sunxi/clk-
> h3-ths.c
> new file mode 100644
> index 0000000..663afc0
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-h3-ths.c
> @@ -0,0 +1,98 @@
> +/*
> + * Sunxi THS clock driver
This should be "Allwinner H3 THS clock driver"
> + *
> + * Copyright (C) 2015 Josef Gajdusek
> + *
> + * This software is licensed under the terms of the GNU General
> Public
> + * License version 2, as published by the Free Software Foundation,
> and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define SUN8I_H3_THS_CLK_ENABLE 31
> +#define SUN8I_H3_THS_CLK_DIVIDER_SHIFT 0
> +#define SUN8I_H3_THS_CLK_DIVIDER_WIDTH 2
> +
> +static DEFINE_SPINLOCK(sun8i_h3_ths_clk_lock);
> +
> +static const struct clk_div_table sun8i_h3_ths_clk_table[]
> __initconst = {
> + { .val = 0, .div = 1 },
> + { .val = 1, .div = 2 },
> + { .val = 2, .div = 4 },
> + { .val = 3, .div = 6 },
> + { } /* sentinel */
> +};
> +
> +static void __init sun8i_h3_ths_clk_setup(struct device_node *node)
> +{
> + struct clk *clk;
> + struct clk_gate *gate;
> + struct clk_divider *div;
> + const char *parent;
> + const char *clk_name = node->name;
> + void __iomem *reg;
> + int err;
> +
> + reg = of_io_request_and_map(node, 0,
> of_node_full_name(node));
> +
> + if (IS_ERR(reg))
> + return;
> +
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + goto err_unmap;
> +
> + div = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!div)
> + goto err_gate_free;
> +
> + of_property_read_string(node, "clock-output-names",
> &clk_name);
> + parent = of_clk_get_parent_name(node, 0);
> +
> + gate->reg = reg;
> + gate->bit_idx = SUN8I_H3_THS_CLK_ENABLE;
> + gate->lock = &sun8i_h3_ths_clk_lock;
> +
> + div->reg = reg;
> + div->shift = SUN8I_H3_THS_CLK_DIVIDER_SHIFT;
> + div->width = SUN8I_H3_THS_CLK_DIVIDER_WIDTH;
> + div->table = sun8i_h3_ths_clk_table;
> + div->lock = &sun8i_h3_ths_clk_lock;
> +
> + clk = clk_register_composite(NULL, clk_name, &parent, 1,
> + NULL, NULL,
> + &div->hw, &clk_divider_ops,
> + &gate->hw, &clk_gate_ops,
> + CLK_SET_RATE_PARENT);
> +
> + if (IS_ERR(clk))
> + goto err_div_free;
> +
> + err = of_clk_add_provider(node, of_clk_src_simple_get, clk);
> + if (err)
> + goto err_unregister_clk;
> +
> + return;
> +
> +err_unregister_clk:
> + clk_unregister(clk);
> +err_gate_free:
> + kfree(gate);
> +err_div_free:
> + kfree(div);
> +err_unmap:
> + iounmap(reg);
> +}
> +
> +CLK_OF_DECLARE(sun8i_h3_ths_clk, "allwinner,sun8i-h3-ths-clk",
> + sun8i_h3_ths_clk_setup);
> --
> 2.4.10
>
Hi,
On Mon, Nov 23, 2015 at 09:02:51AM +0100, Josef Gajdusek wrote:
> This patch adds the binding documentation for the sun8i_ths driver
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> .../devicetree/bindings/thermal/sun8i-ths.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-ths.txt
>
> diff --git a/Documentation/devicetree/bindings/thermal/sun8i-ths.txt b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
> new file mode 100644
> index 0000000..67056bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
> @@ -0,0 +1,31 @@
> +* sun8i THS
> +
> +Required properties:
> +- compatible : "allwinner,sun8i-h3-ths"
> +- reg : Address range of the thermal registers and location of the calibration
> + value
> +- resets : Must contain an entry for each entry in reset-names.
> + see ../reset/reset.txt for details
> +- reset-names : Must include the name "ahb"
If you have a single reset line, you don't need reset-names.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi,
On Mon, Nov 23, 2015 at 09:02:48AM +0100, Josef Gajdusek wrote:
> Add a node describing the Security ID memory to the
> Allwinner H3 .dtsi file.
>
> Signed-off-by: Josef Gajdusek <[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 0faa38a..58de718 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -359,6 +359,13 @@
> #size-cells = <0>;
> };
>
> + sid: eeprom@01c14000 {
> + compatible = "allwinner,sun4i-a10-sid";
> + reg = <0x01c14000 0x400>;
The datasheet says it's 256 bytes wide, while the size here is of 1kB,
is it intentional?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Mon, Nov 23, 2015 at 09:02:49AM +0100, Josef Gajdusek wrote:
> This patch adds a driver for the THS clock which is present on the
> Allwinner H3.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
Acked-by: Rob Herring <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
> drivers/clk/sunxi/Makefile | 1 +
> drivers/clk/sunxi/clk-h3-ths.c | 98 +++++++++++++++++++++++
> 3 files changed, 100 insertions(+)
> create mode 100644 drivers/clk/sunxi/clk-h3-ths.c
>
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 23e7bce..6d63b35 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -73,6 +73,7 @@ Required properties:
> "allwinner,sun8i-h3-usb-clk" - for usb gates + resets on H3
> "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80
> "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80
> + "allwinner,sun8i-h3-ths-clk" - for THS on H3
>
> Required properties for all clocks:
> - reg : shall be the control register address for the clock.
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index f520af6..1bf8e1c 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -8,6 +8,7 @@ obj-y += clk-a10-hosc.o
> obj-y += clk-a10-mod1.o
> obj-y += clk-a10-pll2.o
> obj-y += clk-a20-gmac.o
> +obj-y += clk-h3-ths.o
> obj-y += clk-mod0.o
> obj-y += clk-simple-gates.o
> obj-y += clk-sun8i-bus-gates.o
> diff --git a/drivers/clk/sunxi/clk-h3-ths.c b/drivers/clk/sunxi/clk-h3-ths.c
> new file mode 100644
> index 0000000..663afc0
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-h3-ths.c
> @@ -0,0 +1,98 @@
> +/*
> + * Sunxi THS clock driver
> + *
> + * Copyright (C) 2015 Josef Gajdusek
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define SUN8I_H3_THS_CLK_ENABLE 31
> +#define SUN8I_H3_THS_CLK_DIVIDER_SHIFT 0
> +#define SUN8I_H3_THS_CLK_DIVIDER_WIDTH 2
> +
> +static DEFINE_SPINLOCK(sun8i_h3_ths_clk_lock);
> +
> +static const struct clk_div_table sun8i_h3_ths_clk_table[] __initconst = {
> + { .val = 0, .div = 1 },
> + { .val = 1, .div = 2 },
> + { .val = 2, .div = 4 },
> + { .val = 3, .div = 6 },
> + { } /* sentinel */
> +};
> +
> +static void __init sun8i_h3_ths_clk_setup(struct device_node *node)
> +{
> + struct clk *clk;
> + struct clk_gate *gate;
> + struct clk_divider *div;
> + const char *parent;
> + const char *clk_name = node->name;
> + void __iomem *reg;
> + int err;
> +
> + reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +
> + if (IS_ERR(reg))
> + return;
> +
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + goto err_unmap;
> +
> + div = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!div)
> + goto err_gate_free;
> +
> + of_property_read_string(node, "clock-output-names", &clk_name);
> + parent = of_clk_get_parent_name(node, 0);
> +
> + gate->reg = reg;
> + gate->bit_idx = SUN8I_H3_THS_CLK_ENABLE;
> + gate->lock = &sun8i_h3_ths_clk_lock;
> +
> + div->reg = reg;
> + div->shift = SUN8I_H3_THS_CLK_DIVIDER_SHIFT;
> + div->width = SUN8I_H3_THS_CLK_DIVIDER_WIDTH;
> + div->table = sun8i_h3_ths_clk_table;
> + div->lock = &sun8i_h3_ths_clk_lock;
> +
> + clk = clk_register_composite(NULL, clk_name, &parent, 1,
> + NULL, NULL,
> + &div->hw, &clk_divider_ops,
> + &gate->hw, &clk_gate_ops,
> + CLK_SET_RATE_PARENT);
> +
> + if (IS_ERR(clk))
> + goto err_div_free;
> +
> + err = of_clk_add_provider(node, of_clk_src_simple_get, clk);
> + if (err)
> + goto err_unregister_clk;
> +
> + return;
> +
> +err_unregister_clk:
> + clk_unregister(clk);
> +err_gate_free:
> + kfree(gate);
> +err_div_free:
> + kfree(div);
> +err_unmap:
> + iounmap(reg);
> +}
> +
> +CLK_OF_DECLARE(sun8i_h3_ths_clk, "allwinner,sun8i-h3-ths-clk",
> + sun8i_h3_ths_clk_setup);
> --
> 2.4.10
>
Hi,
On Mon, Nov 23, 2015 at 8:43 PM, Maxime Ripard
<[email protected]> wrote:
> Hi,
>
> On Mon, Nov 23, 2015 at 09:02:48AM +0100, Josef Gajdusek wrote:
>> Add a node describing the Security ID memory to the
>> Allwinner H3 .dtsi file.
>>
>> Signed-off-by: Josef Gajdusek <[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 0faa38a..58de718 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -359,6 +359,13 @@
>> #size-cells = <0>;
>> };
>>
>> + sid: eeprom@01c14000 {
>> + compatible = "allwinner,sun4i-a10-sid";
>> + reg = <0x01c14000 0x400>;
>
> The datasheet says it's 256 bytes wide, while the size here is of 1kB,
> is it intentional?
My H3 datasheet (v1.1) says its 1 kB wide.
It'd be nice if Allwinner actually listed the "usable" E-fuse offsets
and widths, instead of having us dig through the SDK code.
Regards
ChenYu
On Tue, Nov 24, 2015 at 11:13:13AM +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Mon, Nov 23, 2015 at 8:43 PM, Maxime Ripard
> <[email protected]> wrote:
> > Hi,
> >
> > On Mon, Nov 23, 2015 at 09:02:48AM +0100, Josef Gajdusek wrote:
> >> Add a node describing the Security ID memory to the
> >> Allwinner H3 .dtsi file.
> >>
> >> Signed-off-by: Josef Gajdusek <[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 0faa38a..58de718 100644
> >> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> >> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> >> @@ -359,6 +359,13 @@
> >> #size-cells = <0>;
> >> };
> >>
> >> + sid: eeprom@01c14000 {
> >> + compatible = "allwinner,sun4i-a10-sid";
> >> + reg = <0x01c14000 0x400>;
> >
> > The datasheet says it's 256 bytes wide, while the size here is of 1kB,
> > is it intentional?
>
> My H3 datasheet (v1.1) says its 1 kB wide.
Is it? in the Security ID section, it is said to be 2kb == 256B wide.
> It'd be nice if Allwinner actually listed the "usable" E-fuse
> offsets and widths, instead of having us dig through the SDK code.
Yep.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Mon, Nov 23, 2015 at 07:24:40PM -0800, Sugar Wu wrote:
> I will give you the right widths as soon.
Great, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Tue, Nov 24, 2015 at 2:38 PM, Maxime Ripard
<[email protected]> wrote:
> On Tue, Nov 24, 2015 at 11:13:13AM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Mon, Nov 23, 2015 at 8:43 PM, Maxime Ripard
>> <[email protected]> wrote:
>> > Hi,
>> >
>> > On Mon, Nov 23, 2015 at 09:02:48AM +0100, Josef Gajdusek wrote:
>> >> Add a node describing the Security ID memory to the
>> >> Allwinner H3 .dtsi file.
>> >>
>> >> Signed-off-by: Josef Gajdusek <[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 0faa38a..58de718 100644
>> >> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> >> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> >> @@ -359,6 +359,13 @@
>> >> #size-cells = <0>;
>> >> };
>> >>
>> >> + sid: eeprom@01c14000 {
>> >> + compatible = "allwinner,sun4i-a10-sid";
>> >> + reg = <0x01c14000 0x400>;
>> >
>> > The datasheet says it's 256 bytes wide, while the size here is of 1kB,
>> > is it intentional?
>>
>> My H3 datasheet (v1.1) says its 1 kB wide.
>
> Is it? in the Security ID section, it is said to be 2kb == 256B wide.
Right. I was looking at the memory map. Maybe it's sparsely mapped?
I guess we'll know soon.
ChenYu
>> It'd be nice if Allwinner actually listed the "usable" E-fuse
>> offsets and widths, instead of having us dig through the SDK code.
>
> Yep.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
On Tue, Nov 24, 2015 at 02:42:26PM +0800, Chen-Yu Tsai wrote:
> On Tue, Nov 24, 2015 at 2:38 PM, Maxime Ripard
> <[email protected]> wrote:
> > On Tue, Nov 24, 2015 at 11:13:13AM +0800, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Mon, Nov 23, 2015 at 8:43 PM, Maxime Ripard
> >> <[email protected]> wrote:
> >> > Hi,
> >> >
> >> > On Mon, Nov 23, 2015 at 09:02:48AM +0100, Josef Gajdusek wrote:
> >> >> Add a node describing the Security ID memory to the
> >> >> Allwinner H3 .dtsi file.
> >> >>
> >> >> Signed-off-by: Josef Gajdusek <[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 0faa38a..58de718 100644
> >> >> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> >> >> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> >> >> @@ -359,6 +359,13 @@
> >> >> #size-cells = <0>;
> >> >> };
> >> >>
> >> >> + sid: eeprom@01c14000 {
> >> >> + compatible = "allwinner,sun4i-a10-sid";
> >> >> + reg = <0x01c14000 0x400>;
> >> >
> >> > The datasheet says it's 256 bytes wide, while the size here is of 1kB,
> >> > is it intentional?
> >>
> >> My H3 datasheet (v1.1) says its 1 kB wide.
> >
> > Is it? in the Security ID section, it is said to be 2kb == 256B wide.
>
> Right. I was looking at the memory map. Maybe it's sparsely mapped?
> I guess we'll know soon.
If it is just like the A20, I think there's a few registers at the end
to control the writes (that we don't use).
Which means that the size of the fuses is smaller than the size of the
mapped area (which also measn that our driver is broken making that
assumption).
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Mon, Nov 23, 2015 at 09:02:50AM +0100, Josef Gajdusek wrote:
> This patch adds support for the Sunxi thermal sensor on the Allwinner H3.
You can drop the sunxi here.
> Should be easily extendable for the A33/A83T/... as they have similar but
> not completely identical sensors.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> drivers/thermal/Kconfig | 7 +
> drivers/thermal/Makefile | 1 +
> drivers/thermal/sun8i_ths.c | 365 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 373 insertions(+)
> create mode 100644 drivers/thermal/sun8i_ths.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index c463c89..2b41147 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -365,6 +365,13 @@ config INTEL_PCH_THERMAL
> Thermal reporting device will provide temperature reading,
> programmable trip points and other information.
>
> +config SUN8I_THS
> + tristate "sun8i THS driver"
> + depends on MACH_SUN8I
> + depends on OF
> + help
> + Enable this to support thermal reporting on some newer Allwinner SoCs.
> +
> menu "Texas Instruments thermal drivers"
> depends on ARCH_HAS_BANDGAP || COMPILE_TEST
> source "drivers/thermal/ti-soc-thermal/Kconfig"
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index cfae6a6..227e1a1 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_INTEL_PCH_THERMAL) += intel_pch_thermal.o
> obj-$(CONFIG_ST_THERMAL) += st/
> obj-$(CONFIG_TEGRA_SOCTHERM) += tegra_soctherm.o
> obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
> +obj-$(CONFIG_SUN8I_THS) += sun8i_ths.o
> diff --git a/drivers/thermal/sun8i_ths.c b/drivers/thermal/sun8i_ths.c
> new file mode 100644
> index 0000000..2c976ac
> --- /dev/null
> +++ b/drivers/thermal/sun8i_ths.c
> @@ -0,0 +1,365 @@
> +/*
> + * Sunxi THS driver
sun8i Thermal Sensor Driver
> + * Copyright (C) 2015 Josef Gajdusek
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
Are you using this header?
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
You probably don't need this one too.
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#define THS_H3_CTRL0 0x00
> +#define THS_H3_CTRL1 0x04
> +#define THS_H3_CDAT 0x14
> +#define THS_H3_CTRL2 0x40
> +#define THS_H3_INT_CTRL 0x44
> +#define THS_H3_STAT 0x48
> +#define THS_H3_ALARM_CTRL 0x50
> +#define THS_H3_SHUTDOWN_CTRL 0x60
> +#define THS_H3_FILTER 0x70
> +#define THS_H3_CDATA 0x74
> +#define THS_H3_DATA 0x80
> +
> +#define THS_H3_CTRL0_SENSOR_ACQ0_OFFS 0
> +#define THS_H3_CTRL0_SENSOR_ACQ0(x) \
> + ((x) << THS_H3_CTRL0_SENSOR_ACQ0_OFFS)
> +#define THS_H3_CTRL1_ADC_CALI_EN_OFFS 17
> +#define THS_H3_CTRL1_ADC_CALI_EN \
> + BIT(THS_H3_CTRL1_ADC_CALI_EN_OFFS)
> +#define THS_H3_CTRL1_OP_BIAS_OFFS 20
> +#define THS_H3_CTRL1_OP_BIAS(x) \
> + ((x) << THS_H3_CTRL1_OP_BIAS_OFFS)
> +#define THS_H3_CTRL2_SENSE_EN_OFFS 0
> +#define THS_H3_CTRL2_SENSE_EN \
> + BIT(THS_H3_CTRL2_SENSE_EN_OFFS)
> +#define THS_H3_CTRL2_SENSOR_ACQ1_OFFS 16
> +#define THS_H3_CTRL2_SENSOR_ACQ1(x) \
> + ((x) << THS_H3_CTRL2_SENSOR_ACQ1_OFFS)
> +
> +#define THS_H3_INT_CTRL_ALARM_INT_EN_OFFS 0
> +#define THS_H3_INT_CTRL_ALARM_INT_EN \
> + BIT(THS_H3_INT_CTRL_ALARM_INT_EN_OFFS)
> +#define THS_H3_INT_CTRL_SHUT_INT_EN_OFFS 4
> +#define THS_H3_INT_CTRL_SHUT_INT_EN \
> + BIT(THS_H3_INT_CTRL_SHUT_INT_EN_OFFS)
> +#define THS_H3_INT_CTRL_DATA_IRQ_EN_OFFS 8
> +#define THS_H3_INT_CTRL_DATA_IRQ_EN \
> + BIT(THS_H3_INT_CTRL_DATA_IRQ_EN_OFFS)
> +#define THS_H3_INT_CTRL_THERMAL_PER_OFFS 12
> +#define THS_H3_INT_CTRL_THERMAL_PER(x) \
> + ((x) << THS_H3_INT_CTRL_THERMAL_PER_OFFS)
> +
> +#define THS_H3_STAT_ALARM_INT_STS_OFFS 0
> +#define THS_H3_STAT_ALARM_INT_STS \
> + BIT(THS_H3_STAT_ALARM_INT_STS_OFFS)
> +#define THS_H3_STAT_SHUT_INT_STS_OFFS 4
> +#define THS_H3_STAT_SHUT_INT_STS \
> + BIT(THS_H3_STAT_SHUT_INT_STS_OFFS)
> +#define THS_H3_STAT_DATA_IRQ_STS_OFFS 8
> +#define THS_H3_STAT_DATA_IRQ_STS \
> + BIT(THS_H3_STAT_DATA_IRQ_STS_OFFS)
> +#define THS_H3_STAT_ALARM_OFF_STS_OFFS 12
> +#define THS_H3_STAT_ALARM_OFF_STS \
> + BIT(THS_H3_STAT_ALARM_OFF_STS_OFFS)
> +
> +#define THS_H3_ALARM_CTRL_ALARM0_T_HYST_OFFS 0
> +#define THS_H3_ALARM_CTRL_ALARM0_T_HYST(x) \
> + ((x) << THS_H3_ALARM_CTRL_ALARM0_T_HYST_OFFS)
> +#define THS_H3_ALARM_CTRL_ALARM0_T_HOT_OFFS 16
> +#define THS_H3_ALARM_CTRL_ALARM0_T_HOT(x) \
> + ((x) << THS_H3_ALARM_CTRL_ALARM0_T_HOT_OFFS)
> +
> +#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT_OFFS 16
> +#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT(x) \
> + ((x) << THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT_OFFS)
> +
> +#define THS_H3_FILTER_TYPE_OFFS 0
> +#define THS_H3_FILTER_TYPE(x) \
> + ((x) << THS_H3_FILTER_TYPE_OFFS)
> +#define THS_H3_FILTER_EN_OFFS 2
> +#define THS_H3_FILTER_EN \
> + BIT(THS_H3_FILTER_EN_OFFS)
Are you using these offsets anywhere?
> +
> +#define THS_H3_CTRL0_SENSOR_ACQ0_VALUE 0xff
> +#define THS_H3_INT_CTRL_THERMAL_PER_VALUE 0x79
> +#define THS_H3_FILTER_TYPE_VALUE 0x2
> +#define THS_H3_CTRL2_SENSOR_ACQ1_VALUE 0x3f
> +
> +struct sun8i_ths_data {
> + struct sun8i_ths_type *type;
> + struct reset_control *reset;
> + struct clk *clk;
> + struct clk *busclk;
> + void __iomem *regs;
> + struct nvmem_cell *calcell;
> + struct platform_device *pdev;
> + struct thermal_zone_device *tzd;
> +};
> +
> +struct sun8i_ths_type {
> + int (*init)(struct platform_device *, struct sun8i_ths_data *);
> + int (*get_temp)(struct sun8i_ths_data *, int *out);
> + void (*irq)(struct sun8i_ths_data *);
> + void (*deinit)(struct sun8i_ths_data *);
> +};
AFAIK, you never got back on why it was actually needed, instead of
directly calling these functions.
> +/* Formula and parameters from the Allwinner 3.4 kernel */
> +static int sun8i_ths_reg_to_temperature(s32 reg, int divisor, int constant)
> +{
> + return constant - (reg * 1000000) / divisor;
> +}
> +
> +static int sun8i_ths_get_temp(void *_data, int *out)
> +{
> + struct sun8i_ths_data *data = _data;
> +
> + return data->type->get_temp(data, out);
> +}
> +
> +static irqreturn_t sun8i_ths_irq_thread(int irq, void *_data)
> +{
> + struct sun8i_ths_data *data = _data;
> +
> + data->type->irq(data);
> + thermal_zone_device_update(data->tzd);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sun8i_ths_h3_init(struct platform_device *pdev,
> + struct sun8i_ths_data *data)
> +{
> + int ret;
> + size_t callen;
> + s32 *caldata;
> +
> + data->busclk = devm_clk_get(&pdev->dev, "ahb");
> + if (IS_ERR(data->busclk)) {
> + ret = PTR_ERR(data->busclk);
> + dev_err(&pdev->dev, "failed to get ahb clk: %d\n", ret);
> + return ret;
> + }
> +
> + data->clk = devm_clk_get(&pdev->dev, "ths");
> + if (IS_ERR(data->clk)) {
> + ret = PTR_ERR(data->clk);
> + dev_err(&pdev->dev, "failed to get ths clk: %d\n", ret);
> + return ret;
> + }
> +
> + data->reset = devm_reset_control_get(&pdev->dev, "ahb");
> + if (IS_ERR(data->reset)) {
> + ret = PTR_ERR(data->reset);
> + dev_err(&pdev->dev, "failed to get reset: %d\n", ret);
> + return ret;
> + }
> +
> + if (data->calcell) {
> + caldata = nvmem_cell_read(data->calcell, &callen);
> + if (IS_ERR(caldata))
> + return PTR_ERR(caldata);
> + writel(be32_to_cpu(*caldata), data->regs + THS_H3_CDATA);
> + kfree(caldata);
> + }
> +
> + ret = clk_prepare_enable(data->busclk);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable bus clk: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(data->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable ths clk: %d\n", ret);
> + goto err_disable_bus;
> + }
> +
> + ret = reset_control_deassert(data->reset);
> + if (ret) {
> + dev_err(&pdev->dev, "reset deassert failed: %d\n", ret);
> + goto err_disable_ths;
> + }
> +
> + /* The final sample period is calculated as follows:
> + * (THERMAL_PER + 1) * 4096 / f_clk * 2^(FILTER_TYPE + 1)
> + *
> + * This results to about 1Hz with these settings.
> + */
> + ret = clk_set_rate(data->clk, 4000000);
I don't follow you here. You have a complicated math function, that
has many input variables, and then, you just set the clock rate to a
constant?
> + if (ret)
> + goto err_disable_ths;
A new line here please
> + writel(THS_H3_CTRL0_SENSOR_ACQ0(THS_H3_CTRL0_SENSOR_ACQ0_VALUE),
> + data->regs + THS_H3_CTRL0);
> + writel(THS_H3_INT_CTRL_THERMAL_PER(THS_H3_INT_CTRL_THERMAL_PER_VALUE) |
> + THS_H3_INT_CTRL_DATA_IRQ_EN,
> + data->regs + THS_H3_INT_CTRL);
> + writel(THS_H3_FILTER_EN | THS_H3_FILTER_TYPE(THS_H3_FILTER_TYPE_VALUE),
> + data->regs + THS_H3_FILTER);
> + writel(THS_H3_CTRL2_SENSOR_ACQ1(THS_H3_CTRL2_SENSOR_ACQ1_VALUE) |
> + THS_H3_CTRL2_SENSE_EN,
> + data->regs + THS_H3_CTRL2);
And here too.
> + return 0;
> +
> +err_disable_ths:
> + clk_disable_unprepare(data->clk);
> +err_disable_bus:
> + clk_disable_unprepare(data->busclk);
> +
> + return ret;
> +}
> +
> +static int sun8i_ths_h3_get_temp(struct sun8i_ths_data *data, int *out)
> +{
> + int val = readl(data->regs + THS_H3_DATA);
> + *out = sun8i_ths_reg_to_temperature(val, 8253, 217000);
> + return 0;
Can't you just return the value directly?
> +}
> +
> +static void sun8i_ths_h3_irq(struct sun8i_ths_data *data)
> +{
> + writel(THS_H3_STAT_DATA_IRQ_STS |
> + THS_H3_STAT_ALARM_INT_STS |
> + THS_H3_STAT_ALARM_OFF_STS |
> + THS_H3_STAT_SHUT_INT_STS,
> + data->regs + THS_H3_STAT);
So you're always clearing all the interrupts? Shouldn't you just clear
only the interrupt you received?
> +}
> +
> +static void sun8i_ths_h3_deinit(struct sun8i_ths_data *data)
> +{
> + reset_control_assert(data->reset);
> + clk_disable_unprepare(data->clk);
> + clk_disable_unprepare(data->busclk);
> +}
> +
> +static const struct thermal_zone_of_device_ops sun8i_ths_thermal_ops = {
> + .get_temp = sun8i_ths_get_temp,
> +};
> +
> +static const struct sun8i_ths_type sun8i_ths_device_h3 = {
> + .init = sun8i_ths_h3_init,
> + .get_temp = sun8i_ths_h3_get_temp,
> + .irq = sun8i_ths_h3_irq,
> + .deinit = sun8i_ths_h3_deinit,
> +};
> +
> +static const struct of_device_id sun8i_ths_id_table[] = {
> + {
> + .compatible = "allwinner,sun8i-h3-ths",
> + .data = &sun8i_ths_device_h3,
> + },
> + {
> + /* sentinel */
> + },
> +};
> +MODULE_DEVICE_TABLE(of, sun8i_ths_id_table);
> +
> +static int sun8i_ths_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match;
> + struct sun8i_ths_data *data;
> + struct resource *res;
> + int ret;
> + int irq;
> +
> + match = of_match_node(sun8i_ths_id_table, np);
If you *really* need to (but I still don't really see why), you can
use of_device_get_match_data here.
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->type = (struct sun8i_ths_type *)match->data;
> + data->pdev = pdev;
> +
> + data->calcell = devm_nvmem_cell_get(&pdev->dev, "calibration");
> + if (IS_ERR(data->calcell)) {
> + if (PTR_ERR(data->calcell) == -EPROBE_DEFER)
> + return PTR_ERR(data->calcell);
New line
> + data->calcell = NULL; /* No calibration register */
s/register/data/ ?
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + data->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(data->regs)) {
> + ret = PTR_ERR(data->regs);
> + dev_err(&pdev->dev,
> + "failed to ioremap THS registers: %d\n", ret);
> + return ret;
> + }
> +
> + 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,
> + sun8i_ths_irq_thread, IRQF_ONESHOT,
> + dev_name(&pdev->dev), data);
Why a threaded irq?
> + if (ret)
> + return ret;
> +
> + ret = data->type->init(pdev, data);
> + if (ret)
> + return ret;
> +
> + data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
> + &sun8i_ths_thermal_ops);
> + if (IS_ERR(data->tzd)) {
> + ret = PTR_ERR(data->tzd);
> + dev_err(&pdev->dev, "failed to register thermal zone: %d\n",
> + ret);
> + goto err_deinit;
> + }
> +
> + platform_set_drvdata(pdev, data);
> + return 0;
> +
> +err_deinit:
> + data->type->deinit(data);
> + return ret;
> +}
> +
> +static int sun8i_ths_remove(struct platform_device *pdev)
> +{
> + struct sun8i_ths_data *data = platform_get_drvdata(pdev);
> +
> + thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd);
> + data->type->deinit(data);
> + return 0;
> +}
> +
> +static struct platform_driver sun8i_ths_driver = {
> + .probe = sun8i_ths_probe,
> + .remove = sun8i_ths_remove,
> + .driver = {
> + .name = "sun8i_ths",
> + .of_match_table = sun8i_ths_id_table,
> + },
> +};
> +
> +module_platform_driver(sun8i_ths_driver);
> +
> +MODULE_AUTHOR("Josef Gajdusek <[email protected]>");
> +MODULE_DESCRIPTION("Sunxi THS driver");
Please change the description here too to match the header.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Mon, Nov 23, 2015 at 09:02:52AM +0100, Josef Gajdusek wrote:
> + ths: ths@01c25000 {
> + #thermal-sensor-cells = <0>;
> + compatible = "allwinner,sun8i-h3-ths";
> + reg = <0x01c25000 0x88>;
The datasheet says the size is 0x400, any particular reason to have a
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
November 24 2015 9:43 AM, "Maxime Ripard" <[email protected]> wrote:
> On Mon, Nov 23, 2015 at 09:02:50AM +0100, Josef Gajdusek wrote:
>
>> This patch adds support for the Sunxi thermal sensor on the Allwinner H3.
>
> You can drop the sunxi here.
>
>> Should be easily extendable for the A33/A83T/... as they have similar but
>> not completely identical sensors.
>>
>> Signed-off-by: Josef Gajdusek <[email protected]>
>> ---
>> drivers/thermal/Kconfig | 7 +
>> drivers/thermal/Makefile | 1 +
>> drivers/thermal/sun8i_ths.c | 365 ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 373 insertions(+)
>> create mode 100644 drivers/thermal/sun8i_ths.c
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index c463c89..2b41147 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -365,6 +365,13 @@ config INTEL_PCH_THERMAL
>> Thermal reporting device will provide temperature reading,
>> programmable trip points and other information.
>>
>> +config SUN8I_THS
>> + tristate "sun8i THS driver"
>> + depends on MACH_SUN8I
>> + depends on OF
>> + help
>> + Enable this to support thermal reporting on some newer Allwinner SoCs.
>> +
>> menu "Texas Instruments thermal drivers"
>> depends on ARCH_HAS_BANDGAP || COMPILE_TEST
>> source "drivers/thermal/ti-soc-thermal/Kconfig"
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index cfae6a6..227e1a1 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -48,3 +48,4 @@ obj-$(CONFIG_INTEL_PCH_THERMAL) += intel_pch_thermal.o
>> obj-$(CONFIG_ST_THERMAL) += st/
>> obj-$(CONFIG_TEGRA_SOCTHERM) += tegra_soctherm.o
>> obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
>> +obj-$(CONFIG_SUN8I_THS) += sun8i_ths.o
>> diff --git a/drivers/thermal/sun8i_ths.c b/drivers/thermal/sun8i_ths.c
>> new file mode 100644
>> index 0000000..2c976ac
>> --- /dev/null
>> +++ b/drivers/thermal/sun8i_ths.c
>> @@ -0,0 +1,365 @@
>> +/*
>> + * Sunxi THS driver
>
> sun8i Thermal Sensor Driver
>
>> + * Copyright (C) 2015 Josef Gajdusek
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>
> Are you using this header?
>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/irq.h>
>
> You probably don't need this one too.
>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-consumer.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/printk.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +#include <linux/thermal.h>
>> +
>> +#define THS_H3_CTRL0 0x00
>> +#define THS_H3_CTRL1 0x04
>> +#define THS_H3_CDAT 0x14
>> +#define THS_H3_CTRL2 0x40
>> +#define THS_H3_INT_CTRL 0x44
>> +#define THS_H3_STAT 0x48
>> +#define THS_H3_ALARM_CTRL 0x50
>> +#define THS_H3_SHUTDOWN_CTRL 0x60
>> +#define THS_H3_FILTER 0x70
>> +#define THS_H3_CDATA 0x74
>> +#define THS_H3_DATA 0x80
>> +
>> +#define THS_H3_CTRL0_SENSOR_ACQ0_OFFS 0
>> +#define THS_H3_CTRL0_SENSOR_ACQ0(x) \
>> + ((x) << THS_H3_CTRL0_SENSOR_ACQ0_OFFS)
>> +#define THS_H3_CTRL1_ADC_CALI_EN_OFFS 17
>> +#define THS_H3_CTRL1_ADC_CALI_EN \
>> + BIT(THS_H3_CTRL1_ADC_CALI_EN_OFFS)
>> +#define THS_H3_CTRL1_OP_BIAS_OFFS 20
>> +#define THS_H3_CTRL1_OP_BIAS(x) \
>> + ((x) << THS_H3_CTRL1_OP_BIAS_OFFS)
>> +#define THS_H3_CTRL2_SENSE_EN_OFFS 0
>> +#define THS_H3_CTRL2_SENSE_EN \
>> + BIT(THS_H3_CTRL2_SENSE_EN_OFFS)
>> +#define THS_H3_CTRL2_SENSOR_ACQ1_OFFS 16
>> +#define THS_H3_CTRL2_SENSOR_ACQ1(x) \
>> + ((x) << THS_H3_CTRL2_SENSOR_ACQ1_OFFS)
>> +
>> +#define THS_H3_INT_CTRL_ALARM_INT_EN_OFFS 0
>> +#define THS_H3_INT_CTRL_ALARM_INT_EN \
>> + BIT(THS_H3_INT_CTRL_ALARM_INT_EN_OFFS)
>> +#define THS_H3_INT_CTRL_SHUT_INT_EN_OFFS 4
>> +#define THS_H3_INT_CTRL_SHUT_INT_EN \
>> + BIT(THS_H3_INT_CTRL_SHUT_INT_EN_OFFS)
>> +#define THS_H3_INT_CTRL_DATA_IRQ_EN_OFFS 8
>> +#define THS_H3_INT_CTRL_DATA_IRQ_EN \
>> + BIT(THS_H3_INT_CTRL_DATA_IRQ_EN_OFFS)
>> +#define THS_H3_INT_CTRL_THERMAL_PER_OFFS 12
>> +#define THS_H3_INT_CTRL_THERMAL_PER(x) \
>> + ((x) << THS_H3_INT_CTRL_THERMAL_PER_OFFS)
>> +
>> +#define THS_H3_STAT_ALARM_INT_STS_OFFS 0
>> +#define THS_H3_STAT_ALARM_INT_STS \
>> + BIT(THS_H3_STAT_ALARM_INT_STS_OFFS)
>> +#define THS_H3_STAT_SHUT_INT_STS_OFFS 4
>> +#define THS_H3_STAT_SHUT_INT_STS \
>> + BIT(THS_H3_STAT_SHUT_INT_STS_OFFS)
>> +#define THS_H3_STAT_DATA_IRQ_STS_OFFS 8
>> +#define THS_H3_STAT_DATA_IRQ_STS \
>> + BIT(THS_H3_STAT_DATA_IRQ_STS_OFFS)
>> +#define THS_H3_STAT_ALARM_OFF_STS_OFFS 12
>> +#define THS_H3_STAT_ALARM_OFF_STS \
>> + BIT(THS_H3_STAT_ALARM_OFF_STS_OFFS)
>> +
>> +#define THS_H3_ALARM_CTRL_ALARM0_T_HYST_OFFS 0
>> +#define THS_H3_ALARM_CTRL_ALARM0_T_HYST(x) \
>> + ((x) << THS_H3_ALARM_CTRL_ALARM0_T_HYST_OFFS)
>> +#define THS_H3_ALARM_CTRL_ALARM0_T_HOT_OFFS 16
>> +#define THS_H3_ALARM_CTRL_ALARM0_T_HOT(x) \
>> + ((x) << THS_H3_ALARM_CTRL_ALARM0_T_HOT_OFFS)
>> +
>> +#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT_OFFS 16
>> +#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT(x) \
>> + ((x) << THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT_OFFS)
>> +
>> +#define THS_H3_FILTER_TYPE_OFFS 0
>> +#define THS_H3_FILTER_TYPE(x) \
>> + ((x) << THS_H3_FILTER_TYPE_OFFS)
>> +#define THS_H3_FILTER_EN_OFFS 2
>> +#define THS_H3_FILTER_EN \
>> + BIT(THS_H3_FILTER_EN_OFFS)
>
> Are you using these offsets anywhere?
>> +
>> +#define THS_H3_CTRL0_SENSOR_ACQ0_VALUE 0xff
>> +#define THS_H3_INT_CTRL_THERMAL_PER_VALUE 0x79
>> +#define THS_H3_FILTER_TYPE_VALUE 0x2
>> +#define THS_H3_CTRL2_SENSOR_ACQ1_VALUE 0x3f
>> +
>> +struct sun8i_ths_data {
>> + struct sun8i_ths_type *type;
>> + struct reset_control *reset;
>> + struct clk *clk;
>> + struct clk *busclk;
>> + void __iomem *regs;
>> + struct nvmem_cell *calcell;
>> + struct platform_device *pdev;
>> + struct thermal_zone_device *tzd;
>> +};
>> +
>> +struct sun8i_ths_type {
>> + int (*init)(struct platform_device *, struct sun8i_ths_data *);
>> + int (*get_temp)(struct sun8i_ths_data *, int *out);
>> + void (*irq)(struct sun8i_ths_data *);
>> + void (*deinit)(struct sun8i_ths_data *);
>> +};
>
> AFAIK, you never got back on why it was actually needed, instead of
> directly calling these functions.
It is preparation for supporting the other SoCs with THS as they have
slightly different register layouts and thus cannot be controlled by the
same code.
>> +/* Formula and parameters from the Allwinner 3.4 kernel */
>> +static int sun8i_ths_reg_to_temperature(s32 reg, int divisor, int constant)
>> +{
>> + return constant - (reg * 1000000) / divisor;
>> +}
>> +
>> +static int sun8i_ths_get_temp(void *_data, int *out)
>> +{
>> + struct sun8i_ths_data *data = _data;
>> +
>> + return data->type->get_temp(data, out);
>> +}
>> +
>> +static irqreturn_t sun8i_ths_irq_thread(int irq, void *_data)
>> +{
>> + struct sun8i_ths_data *data = _data;
>> +
>> + data->type->irq(data);
>> + thermal_zone_device_update(data->tzd);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int sun8i_ths_h3_init(struct platform_device *pdev,
>> + struct sun8i_ths_data *data)
>> +{
>> + int ret;
>> + size_t callen;
>> + s32 *caldata;
>> +
>> + data->busclk = devm_clk_get(&pdev->dev, "ahb");
>> + if (IS_ERR(data->busclk)) {
>> + ret = PTR_ERR(data->busclk);
>> + dev_err(&pdev->dev, "failed to get ahb clk: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + data->clk = devm_clk_get(&pdev->dev, "ths");
>> + if (IS_ERR(data->clk)) {
>> + ret = PTR_ERR(data->clk);
>> + dev_err(&pdev->dev, "failed to get ths clk: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + data->reset = devm_reset_control_get(&pdev->dev, "ahb");
>> + if (IS_ERR(data->reset)) {
>> + ret = PTR_ERR(data->reset);
>> + dev_err(&pdev->dev, "failed to get reset: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + if (data->calcell) {
>> + caldata = nvmem_cell_read(data->calcell, &callen);
>> + if (IS_ERR(caldata))
>> + return PTR_ERR(caldata);
>> + writel(be32_to_cpu(*caldata), data->regs + THS_H3_CDATA);
>> + kfree(caldata);
>> + }
>> +
>> + ret = clk_prepare_enable(data->busclk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to enable bus clk: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(data->clk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to enable ths clk: %d\n", ret);
>> + goto err_disable_bus;
>> + }
>> +
>> + ret = reset_control_deassert(data->reset);
>> + if (ret) {
>> + dev_err(&pdev->dev, "reset deassert failed: %d\n", ret);
>> + goto err_disable_ths;
>> + }
>> +
>> + /* The final sample period is calculated as follows:
>> + * (THERMAL_PER + 1) * 4096 / f_clk * 2^(FILTER_TYPE + 1)
>> + *
>> + * This results to about 1Hz with these settings.
>> + */
>> + ret = clk_set_rate(data->clk, 4000000);
>
> I don't follow you here. You have a complicated math function, that
> has many input variables, and then, you just set the clock rate to a
> constant?
How should this be handled then? I guess the sampling rate could
be set in the device tree and then the values calculated, but none
of the other thermal drivers seem to have configurable sample rate.
>> + if (ret)
>> + goto err_disable_ths;
>
> A new line here please
>
>> + writel(THS_H3_CTRL0_SENSOR_ACQ0(THS_H3_CTRL0_SENSOR_ACQ0_VALUE),
>> + data->regs + THS_H3_CTRL0);
>> + writel(THS_H3_INT_CTRL_THERMAL_PER(THS_H3_INT_CTRL_THERMAL_PER_VALUE) |
>> + THS_H3_INT_CTRL_DATA_IRQ_EN,
>> + data->regs + THS_H3_INT_CTRL);
>> + writel(THS_H3_FILTER_EN | THS_H3_FILTER_TYPE(THS_H3_FILTER_TYPE_VALUE),
>> + data->regs + THS_H3_FILTER);
>> + writel(THS_H3_CTRL2_SENSOR_ACQ1(THS_H3_CTRL2_SENSOR_ACQ1_VALUE) |
>> + THS_H3_CTRL2_SENSE_EN,
>> + data->regs + THS_H3_CTRL2);
>
> And here too.
>
>> + return 0;
>> +
>> +err_disable_ths:
>> + clk_disable_unprepare(data->clk);
>> +err_disable_bus:
>> + clk_disable_unprepare(data->busclk);
>> +
>> + return ret;
>> +}
>> +
>> +static int sun8i_ths_h3_get_temp(struct sun8i_ths_data *data, int *out)
>> +{
>> + int val = readl(data->regs + THS_H3_DATA);
>> + *out = sun8i_ths_reg_to_temperature(val, 8253, 217000);
>> + return 0;
>
> Can't you just return the value directly?
I did that in the v1, clabbe.montjoie suggested to use temp variable to
avoid column wrap.
>> +}
>> +
>> +static void sun8i_ths_h3_irq(struct sun8i_ths_data *data)
>> +{
>> + writel(THS_H3_STAT_DATA_IRQ_STS |
>> + THS_H3_STAT_ALARM_INT_STS |
>> + THS_H3_STAT_ALARM_OFF_STS |
>> + THS_H3_STAT_SHUT_INT_STS,
>> + data->regs + THS_H3_STAT);
>
> So you're always clearing all the interrupts? Shouldn't you just clear
> only the interrupt you received?
>
>> +}
>> +
>> +static void sun8i_ths_h3_deinit(struct sun8i_ths_data *data)
>> +{
>> + reset_control_assert(data->reset);
>> + clk_disable_unprepare(data->clk);
>> + clk_disable_unprepare(data->busclk);
>> +}
>> +
>> +static const struct thermal_zone_of_device_ops sun8i_ths_thermal_ops = {
>> + .get_temp = sun8i_ths_get_temp,
>> +};
>> +
>> +static const struct sun8i_ths_type sun8i_ths_device_h3 = {
>> + .init = sun8i_ths_h3_init,
>> + .get_temp = sun8i_ths_h3_get_temp,
>> + .irq = sun8i_ths_h3_irq,
>> + .deinit = sun8i_ths_h3_deinit,
>> +};
>> +
>> +static const struct of_device_id sun8i_ths_id_table[] = {
>> + {
>> + .compatible = "allwinner,sun8i-h3-ths",
>> + .data = &sun8i_ths_device_h3,
>> + },
>> + {
>> + /* sentinel */
>> + },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun8i_ths_id_table);
>> +
>> +static int sun8i_ths_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + const struct of_device_id *match;
>> + struct sun8i_ths_data *data;
>> + struct resource *res;
>> + int ret;
>> + int irq;
>> +
>> + match = of_match_node(sun8i_ths_id_table, np);
>
> If you *really* need to (but I still don't really see why), you can
> use of_device_get_match_data here.
>
>> +
>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->type = (struct sun8i_ths_type *)match->data;
>> + data->pdev = pdev;
>> +
>> + data->calcell = devm_nvmem_cell_get(&pdev->dev, "calibration");
>> + if (IS_ERR(data->calcell)) {
>> + if (PTR_ERR(data->calcell) == -EPROBE_DEFER)
>> + return PTR_ERR(data->calcell);
>
> New line
>
>> + data->calcell = NULL; /* No calibration register */
>
> s/register/data/ ?
>
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + data->regs = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(data->regs)) {
>> + ret = PTR_ERR(data->regs);
>> + dev_err(&pdev->dev,
>> + "failed to ioremap THS registers: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + 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,
>> + sun8i_ths_irq_thread, IRQF_ONESHOT,
>> + dev_name(&pdev->dev), data);
>
> Why a threaded irq?
I thought threaded IRQs are preferred? Other thermal drivers
use them too. I am also not really sure thermal_zone_device_update()
can even be called in interrupt context.
>> + if (ret)
>> + return ret;
>> +
>> + ret = data->type->init(pdev, data);
>> + if (ret)
>> + return ret;
>> +
>> + data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
>> + &sun8i_ths_thermal_ops);
>> + if (IS_ERR(data->tzd)) {
>> + ret = PTR_ERR(data->tzd);
>> + dev_err(&pdev->dev, "failed to register thermal zone: %d\n",
>> + ret);
>> + goto err_deinit;
>> + }
>> +
>> + platform_set_drvdata(pdev, data);
>> + return 0;
>> +
>> +err_deinit:
>> + data->type->deinit(data);
>> + return ret;
>> +}
>> +
>> +static int sun8i_ths_remove(struct platform_device *pdev)
>> +{
>> + struct sun8i_ths_data *data = platform_get_drvdata(pdev);
>> +
>> + thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd);
>> + data->type->deinit(data);
>> + return 0;
>> +}
>> +
>> +static struct platform_driver sun8i_ths_driver = {
>> + .probe = sun8i_ths_probe,
>> + .remove = sun8i_ths_remove,
>> + .driver = {
>> + .name = "sun8i_ths",
>> + .of_match_table = sun8i_ths_id_table,
>> + },
>> +};
>> +
>> +module_platform_driver(sun8i_ths_driver);
>> +
>> +MODULE_AUTHOR("Josef Gajdusek <[email protected]>");
>> +MODULE_DESCRIPTION("Sunxi THS driver");
>
> Please change the description here too to match the header.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to
> [email protected].
> For more options, visit https://groups.google.com/d/optout.
Josef Gajdusek <[email protected]>
On Wed, Nov 25, 2015 at 11:02:34AM +0000, Josef Gajdusek wrote:
> >> +struct sun8i_ths_type {
> >> + int (*init)(struct platform_device *, struct sun8i_ths_data *);
> >> + int (*get_temp)(struct sun8i_ths_data *, int *out);
> >> + void (*irq)(struct sun8i_ths_data *);
> >> + void (*deinit)(struct sun8i_ths_data *);
> >> +};
> >
> > AFAIK, you never got back on why it was actually needed, instead of
> > directly calling these functions.
>
> It is preparation for supporting the other SoCs with THS as they have
> slightly different register layouts and thus cannot be controlled by the
> same code.
Do you have support and / or informations on what's going to be needed
for these other SoCs yet?
Which SoCs are we talking about?
> >> + /* The final sample period is calculated as follows:
> >> + * (THERMAL_PER + 1) * 4096 / f_clk * 2^(FILTER_TYPE + 1)
> >> + *
> >> + * This results to about 1Hz with these settings.
> >> + */
> >> + ret = clk_set_rate(data->clk, 4000000);
> >
> > I don't follow you here. You have a complicated math function, that
> > has many input variables, and then, you just set the clock rate to a
> > constant?
>
> How should this be handled then? I guess the sampling rate could
> be set in the device tree and then the values calculated, but none
> of the other thermal drivers seem to have configurable sample rate.
I don't know, I would have expected some actual computation, like a
function taking the frequency as a parameter and returning the clock
rate. At least that way we now what we're doing and which part might
change and which will not.
But you do not need to change the sample rate itself.
> >> +static int sun8i_ths_h3_get_temp(struct sun8i_ths_data *data, int *out)
> >> +{
> >> + int val = readl(data->regs + THS_H3_DATA);
> >> + *out = sun8i_ths_reg_to_temperature(val, 8253, 217000);
> >> + return 0;
> >
> > Can't you just return the value directly?
>
> I did that in the v1, clabbe.montjoie suggested to use temp variable to
> avoid column wrap.
I was talking about the out pointer. Can the value not be returned?
> >> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> >> + sun8i_ths_irq_thread, IRQF_ONESHOT,
> >> + dev_name(&pdev->dev), data);
> >
> > Why a threaded irq?
>
> I thought threaded IRQs are preferred? Other thermal drivers
> use them too.
It's close to pointless in this case. You're not doing much more than
what the default handler will do anyway, and you avoid scheduling a
thread doing so.
And other thermal drivers use a regular interrupt handler too :)
> I am also not really sure thermal_zone_device_update() can even be
> called in interrupt context.
I can't really tell on this one. Judging from a quick look, I can't
see anything that could prevent it, and since others are using it, it
seems doable.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com