sunxi: THS support

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(+)

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

index 0faa38a..58de718 100644

Hi,

Quoted 16 lines · Josef Gajdusek

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(+)

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

index 0faa38a..58de718 100644
@@ -359,6 +359,13 @@
                      #size-cells = <0>;
              };
 

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 3 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

+             sid: eeprom at 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?

Hi,

Quoted 6 lines · On Mon, Nov 23, 2015 at 8:43 PM, Maxime Ripard <[email protected]…

Hi,

Quoted 3 lines · Josef Gajdusek

Add a node describing the Security ID memory to the Allwinner H3 .dtsi file.

Quoted 6 lines

Review

Quoted 6 lines

Signed-off-by Josef Gajdusek <[email protected] <javascript:>>

---

 arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++ 
 1 file changed, 7 insertions(+) 

(quoted diff) +0 -0

b/arch/arm/boot/dts/sun8i-h3.dtsi

Quoted 7 lines

Quoted 7 lines

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

@@ -359,6 +359,13 @@ 
                      #size-cells = <0>; 
              }; 
 

Quoted 6 lines

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 3 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -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

I will give you the right widths as soon.

Hi,

Quoted 19 lines · Maxime Ripard

Hi,

Quoted 16 lines · Josef Gajdusek

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(+)

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

index 0faa38a..58de718 100644
@@ -359,6 +359,13 @@
                      #size-cells = <0>;
              };
 

Quoted 6 lines

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 3 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

+             sid: eeprom at 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.

Quoted 30 lines · Chen-Yu Tsai

Hi,

Quoted 25 lines · Maxime Ripard

Hi,

Quoted 13 lines · Josef Gajdusek

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(+)

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 6 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

@@ -359,6 +359,13 @@
                      #size-cells = <0>;
              };
 
+             sid: eeprom at 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?

Quoted 1 line

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.

Quoted 2 lines

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

Quoted 31 lines · Maxime Ripard

Quoted 30 lines · Chen-Yu Tsai

Hi,

Quoted 25 lines · Maxime Ripard

Hi,

Quoted 13 lines · Josef Gajdusek

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(+)

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 6 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

@@ -359,6 +359,13 @@
                      #size-cells = <0>;
              };
 
+             sid: eeprom at 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?

Quoted 3 lines

Quoted 1 line

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.

Hi,

Review

Quoted 9 lines · Josef Gajdusek

Add a node describing the Security ID memory to the Allwinner H3 .dtsi file.

Signed-off-by Josef Gajdusek <[email protected] <javascript:>>

---

 arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++ 
 1 file changed, 7 insertions(+) 

(quoted diff) +0 -0

b/arch/arm/boot/dts/sun8i-h3.dtsi

Quoted 7 lines

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

@@ -359,6 +359,13 @@ 
                         #size-cells = <0>; 
                 }; 
 

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 3 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -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?

Maxime

Signature · 3 lines

Maxime Ripard, Free Electrons 
Embedded Linux, Kernel and Android engineering 
http://free-electrons.com 

Quoted 21 lines · Maxime Ripard

Hi,

Review

Quoted 9 lines · Josef Gajdusek

Add a node describing the Security ID memory to the Allwinner H3 .dtsi file.

Signed-off-by Josef Gajdusek <[email protected] <javascript:>>

---

 arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++ 
 1 file changed, 7 insertions(+) 

(quoted diff) +0 -0

b/arch/arm/boot/dts/sun8i-h3.dtsi

Quoted 7 lines

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi +0 -0

@@ -359,6 +359,13 @@ 
                         #size-cells = <0>; 
                 }; 
 

Quoted 6 lines

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 3 lines

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

+                sid: eeprom at 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?

SID memory map is 0x01c14000 ~ 0x01c143FF, include 2048bits efuse space. H3 efuse space is SID_SRAM, its range is 0x01c14200 ~ +0x100.

Quoted 29 lines · Sugar Wu

Quoted 27 lines · Maxime Ripard

Hi,

Review

Quoted 9 lines · Josef Gajdusek

Add a node describing the Security ID memory to the Allwinner H3 .dtsi file.

Signed-off-by Josef Gajdusek <[email protected] <javascript:>>

---

 arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++ 
 1 file changed, 7 insertions(+) 

(quoted diff) +0 -0

b/arch/arm/boot/dts/sun8i-h3.dtsi

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 10 lines

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

@@ -359,6 +359,13 @@ 
                         #size-cells = <0>; 
                 }; 
 
+                sid: eeprom at 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?

Quoted 2 lines

SID memory map is 0x01c14000 ~ 0x01c143FF, include 2048bits efuse space. H3 efuse space is SID_SRAM, its range is 0x01c14200 ~ +0x100.

Interesting, what is below the 0x200 registers?

Thanks!
Maxime

Quoted 30 lines · Maxime Ripard

Quoted 29 lines · Sugar Wu

Quoted 27 lines · Maxime Ripard

Hi,

Review

Quoted 9 lines · Josef Gajdusek

Add a node describing the Security ID memory to the Allwinner H3 .dtsi file.

Signed-off-by Josef Gajdusek <[email protected] <javascript:>>

---

 arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++ 
 1 file changed, 7 insertions(+) 

(quoted diff) +0 -0

b/arch/arm/boot/dts/sun8i-h3.dtsi

arch/arm/boot/dts/sun8i-h3.dtsi line 364

Quoted 10 lines

index 0faa38a..58de718 100644

arch/arm/boot/dts/sun8i-h3.dtsi +3 -0

@@ -359,6 +359,13 @@ 
                         #size-cells = <0>; 
                 }; 
 
+                sid: eeprom at 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?

Quoted 5 lines

Quoted 2 lines

SID memory map is 0x01c14000 ~ 0x01c143FF, include 2048bits efuse space. H3 efuse space is SID_SRAM, its range is 0x01c14200 ~ +0x100.

Interesting, what is below the 0x200 registers?

Some control register about SID.
offset: 0x40 SID Program/Read Control Register
offset: 0x50 SID Program Key Value Register
offset: 0x60 SID Read Key Value Register
offset: 0x70 \
offset: 0x80 SJTAG Attribute 0 Register
offset: 0x84 SJTAG Attribute 1 Register
offset: 0x88 SJTAG Select Register
offset: 0x90 SID Program Ctrol register for burned timing

This patch adds a driver for the THS clock which is present on the Allwinner H3.

Signed-off-by Josef Gajdusek <[email protected]>

---

drivers/clk/sunxi/clk-h3-ths.c new file +10 -0

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

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

Documentation/devicetree/bindings/clock/sunxi.txt +0 -0

index 23e7bce..6d63b35 100644
@@ -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

Documentation/devicetree/bindings/clock/sunxi.txt +1 -0

+	"allwinner,sun8i-h3-ths-clk" - for THS on H3

?
?Required properties for all clocks:
?- reg : shall be the control register address for the clock.

drivers/clk/sunxi/Makefile +0 -0

index f520af6..1bf8e1c 100644
@@ -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

drivers/clk/sunxi/clk- renamed from drivers/clk/sunxi/clk-h3-ths.c +0 -0

h3-ths.c
new file mode 100644
index 0000000..663afc0

drivers/clk/sunxi/clk-h3-ths.c new file +4 -0

+ *
+ * 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.
+ *
+ */

drivers/clk/sunxi/clk-h3-ths.c new file +13 -0

+
+#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 = {

drivers/clk/sunxi/clk-h3-ths.c new file +18 -0

+	{ .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));

drivers/clk/sunxi/clk-h3-ths.c new file +13 -0

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

drivers/clk/sunxi/clk-h3-ths.c new file +39 -0

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

Signature · 1 line

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

Documentation/devicetree/bindings/clock/sunxi.txt +1 -0

index 23e7bce..6d63b35 100644
@@ -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.

drivers/clk/sunxi/Makefile +1 -0

index f520af6..1bf8e1c 100644
@@ -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

drivers/clk/sunxi/clk-h3-ths.c new file +98 -0

new file mode 100644
index 0000000..663afc0
@@ -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);

Signature · 1 line

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

drivers/thermal/Kconfig +7 -0

index c463c89..2b41147 100644
@@ -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"

drivers/thermal/Makefile +1 -0

index cfae6a6..227e1a1 100644
@@ -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

drivers/thermal/sun8i_ths.c new file +0 -0

new file mode 100644
index 0000000..2c976ac

drivers/thermal/sun8i_ths.c new file +5 -0

+ * 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.

drivers/thermal/sun8i_ths.c new file +70 -0

+#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)
+

drivers/thermal/sun8i_ths.c new file +13 -0

+
+#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;

drivers/thermal/sun8i_ths.c new file +73 -0

+/* 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) {

drivers/thermal/sun8i_ths.c new file +5 -0

+	return 0;
+
+err_disable_ths:
+	clk_disable_unprepare(data->clk);
+err_disable_bus:

drivers/thermal/sun8i_ths.c new file +31 -0

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

drivers/thermal/sun8i_ths.c new file +10 -0

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

drivers/thermal/sun8i_ths.c new file +36 -0

+	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,

Quoted 1 line · Josef Gajdusek

This patch adds support for the Sunxi thermal sensor on the Allwinner H3.

You can drop the sunxi here.

Quoted 44 lines

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

drivers/thermal/Kconfig +0 -0

index c463c89..2b41147 100644
@@ -365,6 +365,13 @@ config INTEL_PCH_THERMAL
 Thermal reporting device will provide temperature reading,

programmable trip points and other information.

drivers/thermal/Kconfig +7 -0

+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"

drivers/thermal/Makefile +0 -0

index cfae6a6..227e1a1 100644
@@ -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

drivers/thermal/Makefile +1 -0

+obj-$(CONFIG_SUN8I_THS) += sun8i_ths.o

drivers/thermal/sun8i_ths.c new file +0 -0

new file mode 100644
index 0000000..2c976ac

drivers/thermal/sun8i_ths.c line 2

Quoted 2 lines

drivers/thermal/sun8i_ths.c new file +2 -0

@@ -0,0 +1,365 @@
+/*
+ * Sunxi THS driver

sun8i Thermal Sensor Driver

Quoted 5 lines

drivers/thermal/sun8i_ths.c new file +5 -0

+ * 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.

drivers/thermal/sun8i_ths.c line 18

Quoted 10 lines

drivers/thermal/sun8i_ths.c new file +10 -0

+ *
+ * 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?

drivers/thermal/sun8i_ths.c line 21

Quoted 3 lines

drivers/thermal/sun8i_ths.c new file +3 -0

+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>

You probably don't need this one too.

Quoted 70 lines

drivers/thermal/sun8i_ths.c new file +70 -0

+#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)
+

drivers/thermal/sun8i_ths.c line 101

Quoted 10 lines

drivers/thermal/sun8i_ths.c new file +10 -0

+#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?

Quoted 18 lines

drivers/thermal/sun8i_ths.c new file +18 -0

+
+#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 {

drivers/thermal/sun8i_ths.c line 124

Quoted 5 lines

drivers/thermal/sun8i_ths.c new file +5 -0

+ 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.

Quoted 79 lines

drivers/thermal/sun8i_ths.c new file +79 -0

+/* 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)

drivers/thermal/sun8i_ths.c line 208

Quoted 4 lines

drivers/thermal/sun8i_ths.c new file +4 -0

+ *
+ * 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?

drivers/thermal/sun8i_ths.c line 210

Quoted 2 lines

drivers/thermal/sun8i_ths.c new file +2 -0

+ if (ret)
+ goto err_disable_ths;

A new line here please

drivers/thermal/sun8i_ths.c line 220

Quoted 10 lines

drivers/thermal/sun8i_ths.c new file +10 -0

+ 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.

Quoted 9 lines

drivers/thermal/sun8i_ths.c new file +9 -0

+ return 0;
+
+err_disable_ths:
+ clk_disable_unprepare(data->clk);
+err_disable_bus:
+ clk_disable_unprepare(data->busclk);
+
+ return ret;
+}

drivers/thermal/sun8i_ths.c line 235

Quoted 6 lines

drivers/thermal/sun8i_ths.c new file +6 -0

+
+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?

drivers/thermal/sun8i_ths.c line 244

Quoted 9 lines

drivers/thermal/sun8i_ths.c new file +9 -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);

So you're always clearing all the interrupts? Shouldn't you just clear only the interrupt you received?

Quoted 31 lines

drivers/thermal/sun8i_ths.c new file +31 -0

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

drivers/thermal/sun8i_ths.c line 285

Quoted 10 lines

drivers/thermal/sun8i_ths.c new file +10 -0

+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.

drivers/thermal/sun8i_ths.c line 297

Quoted 12 lines

drivers/thermal/sun8i_ths.c new file +12 -0

+
+ 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

drivers/thermal/sun8i_ths.c line 298

Quoted 1 line

drivers/thermal/sun8i_ths.c new file +1 -0

+ data->calcell = NULL; /* No calibration register */

s/register/data/ ?

Quoted 14 lines

drivers/thermal/sun8i_ths.c new file +14 -0

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

drivers/thermal/sun8i_ths.c line 318

Quoted 6 lines

drivers/thermal/sun8i_ths.c new file +6 -0

+ 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?

Quoted 36 lines

drivers/thermal/sun8i_ths.c new file +36 -0

+ 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,

drivers/thermal/sun8i_ths.c line 364

Quoted 10 lines

drivers/thermal/sun8i_ths.c new file +10 -0

+ .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 linux-sunxi+unsubscribe at googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Quoted 9 lines

drivers/thermal/sun8i_ths.c line 124

Quoted 6 lines

drivers/thermal/sun8i_ths.c new file +6 -0

+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.

Quoted 7 lines

Quoted 6 lines

+ /* 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);

Quoted 3 lines

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.

Quoted 7 lines

drivers/thermal/sun8i_ths.c line 235

Quoted 5 lines

drivers/thermal/sun8i_ths.c new file +5 -0

+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.

Quoted 5 lines

Quoted 3 lines

+ 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.

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

Documentation/devicetree/bindings/thermal/sun8i-ths.txt new file +0 -0

new file mode 100644
index 0000000..67056bf

Documentation/devicetree/bindings/thermal/sun8i-ths.txt new file +7 -0

+- 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
+

Documentation/devicetree/bindings/thermal/sun8i-ths.txt new file +8 -1

+       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 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

Documentation/devicetree/bindings/thermal/sun8i-ths.txt new file +0 -0

new file mode 100644
index 0000000..67056bf