Skip to contentSkip to conversation

thermal: Add support for thermal sensor for Kirkwood SoC

PATCH New Nobuhiro Iwamatsu 5 participants 12 messages Dec 7, 2012 – Dec 14, 2012

Subsystems (3)
Device tree bindings Arm (32-bit) Thermal
lore
Share & tools
lore

Nobuhiro Iwamatsu

author
#0 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

Some kirkwood SoC has thermal sensor.
This patch adds support for 88F6282 and 88F6283.

Signed-off-by Nobuhiro Iwamatsu <[email protected]>
---
 drivers/thermal/Kconfig            |    7 ++
 drivers/thermal/Makefile           |    1 +
 drivers/thermal/kirkwood_thermal.c |  131 ++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 drivers/thermal/kirkwood_thermal.c
drivers/thermal/Kconfig +7 -0
index e1cb6bd..8710ac2 100644
@@ -55,3 +55,10 @@ config EXYNOS_THERMAL
 	help
 	  If you say yes here you get support for TMU (Thermal Managment
 	  Unit) on SAMSUNG EXYNOS series of SoC.
+
+config KIRKWOOD_THERMAL
+	tristate "Temperature sensor on Marvel Kirkwood"
+	depends on ARCH_KIRKWOOD && THERMAL
+	help
+	  Support for the Kirkwood thermal sensor driver into the Linux thermal
+	  framework. This supports only 88F6282 and 88F6283.
drivers/thermal/Makefile +1 -0
index 885550d..4dbe5e1 100644
@@ -6,4 +6,5 @@ obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 obj-$(CONFIG_CPU_THERMAL)		+= cpu_cooling.o
 obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
+obj-$(CONFIG_KIRKWOOD_THERMAL)         += kirkwood_thermal.o
 obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
drivers/thermal/kirkwood_thermal.c new file +131 -0

131 lines not on this page · raw message

Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 2/2 ARM: Kirkwood: Add support thermal sensor for 88F6282 and 88F6283

88F6282 and 88F6283 has thermal sensor.
This patch enables DT and kernel config.

Signed-off-by Nobuhiro Iwamatsu <[email protected]>
---
 arch/arm/boot/dts/kirkwood-6282.dtsi |    6 ++++++
 arch/arm/configs/kirkwood_defconfig  |    2 ++
 2 files changed, 8 insertions(+)
arch/arm/boot/dts/kirkwood-6282.dtsi +6 -0
index 9ae2004..4d9cd74 100644
@@ -41,5 +41,11 @@
 			clock-frequency = <100000>;
 			status = "disabled";
 		};
+
+		thermal@10078 {
+			compatible = "marvel,thermal-kirkwood";
+			reg = <0x10078 0x4>;
+			status = "okay";
+		};
 	};
 };
arch/arm/configs/kirkwood_defconfig +2 -0
index 93f3794..3d8667f 100644
@@ -118,6 +118,8 @@ CONFIG_SPI=y
 CONFIG_SPI_ORION=y
 CONFIG_GPIO_SYSFS=y
 # CONFIG_HWMON is not set
+CONFIG_THERMAL=y
+CONFIG_KIRKWOOD_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_ORION_WATCHDOG=y
 CONFIG_HID_DRAGONRISE=y
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

Quoted 5 lines · Jason Gunthorpe
Quoted 2 lines · Nobuhiro Iwamatsu

Some kirkwood SoC has thermal sensor.
This patch adds support for 88F6282 and 88F6283.

Thanks! I was just about to write this.. Looks good here.

Ah, looking closer:

$ cat /sys/class/thermal/thermal_zone0/temp 
37

That should be 37000, the value out of the driver should be in milli-Celsius.

I'd use this equation instead:

	*temp = ((322 - reg) * 10000 * 1000) / 13625;

Regards,
Jason

#4 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

Quoted 12 lines · Jason Gunthorpe
Quoted 5 lines · Jason Gunthorpe
Quoted 2 lines · Nobuhiro Iwamatsu

Some kirkwood SoC has thermal sensor.
This patch adds support for 88F6282 and 88F6283.

Thanks! I was just about to write this.. Looks good here.

Ah, looking closer:

$ cat /sys/class/thermal/thermal_zone0/temp 
37
Quoted 6 lines

That should be 37000, the value out of the driver should be in milli-Celsius.

I'd use this equation instead:

	*temp = ((322 - reg) * 10000 * 1000) / 13625;

Be careful of math overflows... make sure you do this calculation using unsigned arithmetic as temperatures above 157 degress will cause this to look like a negative number using signed math... However, that probably won't ever be noticed because at 157 degrees, you'll definitely be outside the operating limits of the device.

author
#5 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

Hi,

Quoted 18 lines · Russell King - ARM Linux
Quoted 17 lines · Jason Gunthorpe
Quoted 5 lines · Jason Gunthorpe
Quoted 2 lines · Nobuhiro Iwamatsu

Some kirkwood SoC has thermal sensor.
This patch adds support for 88F6282 and 88F6283.

Thanks! I was just about to write this.. Looks good here.

Ah, looking closer:

$ cat /sys/class/thermal/thermal_zone0/temp
37

That should be 37000, the value out of the driver should be in milli-Celsius.

I'd use this equation instead:

Quoted 7 lines
Quoted 1 line
      *temp = ((322 - reg) * 10000 * 1000) / 13625;

Be careful of math overflows... make sure you do this calculation using unsigned arithmetic as temperatures above 157 degress will cause this to look like a negative number using signed math... However, that probably won't ever be noticed because at 157 degrees, you'll definitely be outside the operating limits of the device.

Oh, OK. I would like to thank you pointed out.
I will fix.

Best regards,
Nobuhiro

Signature · 3 lines
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6
author
#6 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

In reply to Jason Gunthorpe
Quoted 12 lines · Jason Gunthorpe
Quoted 5 lines · Jason Gunthorpe
Quoted 2 lines · Nobuhiro Iwamatsu

Some kirkwood SoC has thermal sensor.
This patch adds support for 88F6282 and 88F6283.

Thanks! I was just about to write this.. Looks good here.

Ah, looking closer:

$ cat /sys/class/thermal/thermal_zone0/temp
37
Quoted 7 lines

That should be 37000, the value out of the driver should be in milli-Celsius.

I'd use this equation instead:

        *temp = ((322 - reg) * 10000 * 1000) / 13625;

OK, I will fix this.
Thanks for your review.

Best regards,
Nobuhiro

Signature · 3 lines
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6
#7 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

In reply to Nobuhiro Iwamatsu

Hello,

drivers/thermal/Kconfig config EXYNOS_THERMAL · line 60

Quoted 2 lines · Nobuhiro Iwamatsu
drivers/thermal/Kconfig +2 -0
+config KIRKWOOD_THERMAL
+	tristate "Temperature sensor on Marvel Kirkwood"

Marvell

drivers/thermal/kirkwood_thermal.c line 45

Quoted 3 lines
drivers/thermal/kirkwood_thermal.c new file +3 -0
+	/* Valid check */
+	if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) {
+		dev_info(&thermal->device, "Reading temperature is not valid\n");

"Temperature read is not valid" maybe? An native english speaker could say.

drivers/thermal/kirkwood_thermal.c line 73

Quoted 2 lines
drivers/thermal/kirkwood_thermal.c new file +2 -0
+	thermal_dev
+		= devm_kzalloc(&pdev->dev, sizeof(*thermal_dev), GFP_KERNEL);

I think the usual coding style is to have the = on the first line.

drivers/thermal/kirkwood_thermal.c line 103

Quoted 4 lines
drivers/thermal/kirkwood_thermal.c new file +4 -0
+static int kirkwood_thermal_exit(struct platform_device *pdev)
+{
+	struct thermal_zone_device *kirkwood_thermal
+			= platform_get_drvdata(pdev);

Ditto.

drivers/thermal/kirkwood_thermal.c line 112

Quoted 2 lines
drivers/thermal/kirkwood_thermal.c new file +2 -0
+static const struct of_device_id kirkwood_thermal_id_table[] = {
+	{ .compatible = "marvel,thermal-kirkwood" },

marvel -> marvell

Also, I think it should be marvell,kirkwood-thermal, since most other DT compatible strings that we have for Marvell SoCs are marvell,<soc>-<function>.

Also, the Device Tree binding documentation is missing (even though it is admittedly going to be a very short documentation).

Thanks!

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#8 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

Quoted 8 lines · Thomas Petazzoni

Hello,

drivers/thermal/kirkwood_thermal.c line 112

Quoted 2 lines · Nobuhiro Iwamatsu
drivers/thermal/kirkwood_thermal.c new file +2 -0
+static const struct of_device_id kirkwood_thermal_id_table[] = {
+	{ .compatible = "marvel,thermal-kirkwood" },

marvel -> marvell

Quoted 6 lines

Also, I think it should be marvell,kirkwood-thermal, since most other DT compatible strings that we have for Marvell SoCs are marvell,<soc>-<function>.

Also, the Device Tree binding documentation is missing (even though it is admittedly going to be a very short documentation).

Is this in any way compatible with the thermal monitoring found on Dove (510) stuff? If so, should it have the SoC prefix in there, or should it be "armada-thermal" for the SoC family?

#9 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

Quoted 13 lines · Russell King - ARM Linux
Quoted 12 lines · Thomas Petazzoni

Hello,

drivers/thermal/kirkwood_thermal.c line 112

Quoted 2 lines · Nobuhiro Iwamatsu
drivers/thermal/kirkwood_thermal.c new file +2 -0
+static const struct of_device_id kirkwood_thermal_id_table[] = {
+	{ .compatible = "marvel,thermal-kirkwood" },

marvel -> marvell

Also, I think it should be marvell,kirkwood-thermal, since most other DT compatible strings that we have for Marvell SoCs are marvell,<soc>-<function>.

Quoted 6 lines
Quoted 2 lines

Also, the Device Tree binding documentation is missing (even though it is admittedly going to be a very short documentation).

Is this in any way compatible with the thermal monitoring found on Dove (510) stuff? If so, should it have the SoC prefix in there, or should it be "armada-thermal" for the SoC family?

I haven't checked the driver in detail but at least register offsets and the register-to-temperature function are different for Dove. This is no big deal and can be handled with compatible strings.

But more important, "kirkwood" includes 88f618x, 88f619x, and 88f6281 that have no thermal diode - at least it is not mentioned in the public datasheet. So, finally for Nobuhiro's patch I suggest to have two compatible strings, one for marvell,88f6282-thermal and one for marvell,88f6282-thermal. Numbering scheme of Marvell SoCs is a mess..

For the driver, the name should be either orion_thermal.c (as we will reuse it for Dove), or mvebu_thermal.c if there is also a thermal diode on Armada 370/XP. Using "armada" alone is not a good idea, as it also includes some pxa-based SoCs - naming scheme of Marvell SoCs is even more broken than numbering scheme ;)

Sebastian

author
#10 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

In reply to Russell King - ARM Linux

Hi,

Quoted 13 lines · Russell King - ARM Linux
Quoted 12 lines · Thomas Petazzoni

Hello,

drivers/thermal/kirkwood_thermal.c line 112

Quoted 2 lines · Nobuhiro Iwamatsu
drivers/thermal/kirkwood_thermal.c new file +2 -0
+static const struct of_device_id kirkwood_thermal_id_table[] = {
+   { .compatible = "marvel,thermal-kirkwood" },

marvel -> marvell

Also, I think it should be marvell,kirkwood-thermal, since most other DT compatible strings that we have for Marvell SoCs are marvell,<soc>-<function>.

Quoted 6 lines
Quoted 2 lines

Also, the Device Tree binding documentation is missing (even though it is admittedly going to be a very short documentation).

Is this in any way compatible with the thermal monitoring found on Dove (510) stuff? If so, should it have the SoC prefix in there, or should it be "armada-thermal" for the SoC family?

I have document of armada and kirkwood only.
I dont know about Dove. if Dove has same device, we had better change it is a good idea.

Best regards,
Nobuhiro

Signature · 3 lines
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6
author
#11 lore raw

PATCH 1/2 thermal: Add support for thermal sensor for Kirkwood SoC

In reply to Thomas Petazzoni

Hi,

Quoted 8 lines · Thomas Petazzoni

Hello,

drivers/thermal/Kconfig config EXYNOS_THERMAL · line 60

Quoted 2 lines · Nobuhiro Iwamatsu
drivers/thermal/Kconfig +2 -0
+config KIRKWOOD_THERMAL
+     tristate "Temperature sensor on Marvel Kirkwood"

Marvell

Thanks, I will fix.

Quoted 7 lines

drivers/thermal/kirkwood_thermal.c line 45

Quoted 4 lines
drivers/thermal/kirkwood_thermal.c new file +3 -0
+     /* Valid check */
+     if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) {
+             dev_info(&thermal->device, "Reading temperature is not valid\n");

"Temperature read is not valid" maybe? An native english speaker could say.

I will fix.

Quoted 4 lines

drivers/thermal/kirkwood_thermal.c line 73

Quoted 3 lines
drivers/thermal/kirkwood_thermal.c new file +2 -0
+     thermal_dev
+             = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev), GFP_KERNEL);
Quoted 9 lines

I think the usual coding style is to have the = on the first line.

drivers/thermal/kirkwood_thermal.c line 103

Quoted 4 lines
drivers/thermal/kirkwood_thermal.c new file +4 -0
+static int kirkwood_thermal_exit(struct platform_device *pdev)
+{
+     struct thermal_zone_device *kirkwood_thermal
+                     = platform_get_drvdata(pdev);

Ditto.

I will fix.

Quoted 4 lines

drivers/thermal/kirkwood_thermal.c line 112

Quoted 2 lines
drivers/thermal/kirkwood_thermal.c new file +2 -0
+static const struct of_device_id kirkwood_thermal_id_table[] = {
+     { .compatible = "marvel,thermal-kirkwood" },

marvel -> marvell

Thanks, I will fix.

Quoted 8 lines

Also, I think it should be marvell,kirkwood-thermal, since most other DT compatible strings that we have for Marvell SoCs are marvell,<soc>-<function>.

Also, the Device Tree binding documentation is missing (even though it is admittedly going to be a very short documentation).

OK, I forgot this.
I will write documentation.

Quoted 8 lines

Thanks!

Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Best regards,
Nobuhiro

Signature · 3 lines
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6