Message ID | 1354922151-3250-1-git-send-email-iwamatsu@nigauri.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sat, Dec 08, 2012 at 08:15:50AM +0900, Nobuhiro Iwamatsu wrote: > 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. Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Jason
On Fri, Dec 07, 2012 at 04:24:59PM -0700, Jason Gunthorpe wrote: > On Sat, Dec 08, 2012 at 08:15:50AM +0900, Nobuhiro Iwamatsu wrote: > > 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
Hello, On Sat, 8 Dec 2012 08:15:50 +0900, Nobuhiro Iwamatsu wrote: > +config KIRKWOOD_THERMAL > + tristate "Temperature sensor on Marvel Kirkwood" Marvell > + /* 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. > + 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. > +static int kirkwood_thermal_exit(struct platform_device *pdev) > +{ > + struct thermal_zone_device *kirkwood_thermal > + = platform_get_drvdata(pdev); Ditto. > +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
On Fri, Dec 07, 2012 at 04:59:04PM -0700, Jason Gunthorpe wrote: > On Fri, Dec 07, 2012 at 04:24:59PM -0700, Jason Gunthorpe wrote: > > On Sat, Dec 08, 2012 at 08:15:50AM +0900, Nobuhiro Iwamatsu wrote: > > > 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; 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.
On Sat, Dec 08, 2012 at 01:07:08AM +0100, Thomas Petazzoni wrote: > Hello, > > On Sat, 8 Dec 2012 08:15:50 +0900, Nobuhiro Iwamatsu wrote: > > +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). 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?
On 12/08/2012 01:11 AM, Russell King - ARM Linux wrote: > On Sat, Dec 08, 2012 at 01:07:08AM +0100, Thomas Petazzoni wrote: >> Hello, >> >> On Sat, 8 Dec 2012 08:15:50 +0900, Nobuhiro Iwamatsu wrote: >>> +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). > > 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
On Sat, Dec 8, 2012 at 8:59 AM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Fri, Dec 07, 2012 at 04:24:59PM -0700, Jason Gunthorpe wrote: >> On Sat, Dec 08, 2012 at 08:15:50AM +0900, Nobuhiro Iwamatsu wrote: >> > 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; > OK, I will fix this. Thanks for your review. Best regards, Nobuhiro
Hi, On Sat, Dec 8, 2012 at 9:07 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Sat, 8 Dec 2012 08:15:50 +0900, Nobuhiro Iwamatsu wrote: > >> +config KIRKWOOD_THERMAL >> + tristate "Temperature sensor on Marvel Kirkwood" > > Marvell Thanks, I will fix. > >> + /* 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. > >> + 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. > >> +static int kirkwood_thermal_exit(struct platform_device *pdev) >> +{ >> + struct thermal_zone_device *kirkwood_thermal >> + = platform_get_drvdata(pdev); > > Ditto. > I will fix. >> +static const struct of_device_id kirkwood_thermal_id_table[] = { >> + { .compatible = "marvel,thermal-kirkwood" }, > > marvel -> marvell Thanks, I will fix. > > 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. > 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
Hi, On Sat, Dec 8, 2012 at 9:08 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Dec 07, 2012 at 04:59:04PM -0700, Jason Gunthorpe wrote: >> On Fri, Dec 07, 2012 at 04:24:59PM -0700, Jason Gunthorpe wrote: >> > On Sat, Dec 08, 2012 at 08:15:50AM +0900, Nobuhiro Iwamatsu wrote: >> > > 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; > > 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
Hi, On Sat, Dec 8, 2012 at 9:11 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Sat, Dec 08, 2012 at 01:07:08AM +0100, Thomas Petazzoni wrote: >> Hello, >> >> On Sat, 8 Dec 2012 08:15:50 +0900, Nobuhiro Iwamatsu wrote: >> > +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). > > 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
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index e1cb6bd..8710ac2 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -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. diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index 885550d..4dbe5e1 100644 --- a/drivers/thermal/Makefile +++ b/drivers/thermal/Makefile @@ -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 diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c new file mode 100644 index 0000000..bddcf49 --- /dev/null +++ b/drivers/thermal/kirkwood_thermal.c @@ -0,0 +1,131 @@ +/* + * kirkwood thermal sensor driver + * + * Copyright (C) 2012 Nobuhiro Iwamatsu <iwamatsu@nigauri.org> + * + * 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/device.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/of.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/thermal.h> + +#define THERMAL_VALID_OFFSET 9 +#define THERMAL_VALID_MASK 0x1 +#define THERMAL_TEMP_OFFSET 10 +#define THERMAL_TEMP_MASK 0x1FF + +/* Kirkwood Thermal Sensor Dev Structure */ +struct kirkwood_thermal_dev { + void __iomem *base_addr; +}; + +static inline int kirkwood_get_temp(struct thermal_zone_device *thermal, + unsigned long *temp) +{ + unsigned long reg; + struct kirkwood_thermal_dev *thermal_dev = thermal->devdata; + + reg = readl_relaxed(thermal_dev->base_addr); + + /* Valid check */ + if (!(reg >> THERMAL_VALID_OFFSET) & THERMAL_VALID_MASK) { + dev_info(&thermal->device, "Reading temperature is not valid\n"); + return -EIO; + } + + reg = (reg >> THERMAL_TEMP_OFFSET) & THERMAL_TEMP_MASK; + /* Calculate temperature. See Table 814 in hardware manual. */ + *temp = ((322 - reg) * 10000) / 13625; + + return 0; +} + +static struct thermal_zone_device_ops ops = { + .get_temp = kirkwood_get_temp, +}; + +static int kirkwood_thermal_probe(struct platform_device *pdev) +{ + struct thermal_zone_device *thermal = NULL; + struct kirkwood_thermal_dev *thermal_dev; + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&pdev->dev, "Failed to get platform resource\n"); + return -ENODEV; + } + + thermal_dev + = devm_kzalloc(&pdev->dev, sizeof(*thermal_dev), GFP_KERNEL); + if (!thermal_dev) { + dev_err(&pdev->dev, "kzalloc fail\n"); + return -ENOMEM; + } + + thermal_dev->base_addr = devm_ioremap(&pdev->dev, res->start, + resource_size(res)); + if (!thermal_dev->base_addr) { + dev_err(&pdev->dev, "Failed to ioremap memory\n"); + return -ENOMEM; + } + + thermal = thermal_zone_device_register("kirkwood_thermal", 0, 0, + thermal_dev, &ops, 0, 0); + if (IS_ERR(thermal)) { + dev_err(&pdev->dev, "Failed to register thermal zone device\n"); + return PTR_ERR(thermal); + } + + platform_set_drvdata(pdev, thermal); + + dev_info(&thermal->device, KBUILD_MODNAME ": Thermal sensor registered\n"); + + return 0; +} + +static int kirkwood_thermal_exit(struct platform_device *pdev) +{ + struct thermal_zone_device *kirkwood_thermal + = platform_get_drvdata(pdev); + + thermal_zone_device_unregister(kirkwood_thermal); + platform_set_drvdata(pdev, NULL); + + return 0; +} + +static const struct of_device_id kirkwood_thermal_id_table[] = { + { .compatible = "marvel,thermal-kirkwood" }, + {} +}; +MODULE_DEVICE_TABLE(of, kirkwood_thermal_id_table); + +static struct platform_driver kirkwood_thermal_driver = { + .probe = kirkwood_thermal_probe, + .remove = kirkwood_thermal_exit, + .driver = { + .name = "kirkwood_thermal", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(kirkwood_thermal_id_table), + }, +}; + +module_platform_driver(kirkwood_thermal_driver); + +MODULE_AUTHOR("Nobuhiro Iwamatsu <iwamatsu@nigauri.org>"); +MODULE_DESCRIPTION("kirkwood thermal driver"); +MODULE_LICENSE("GPL");
Some kirkwood SoC has thermal sensor. This patch adds support for 88F6282 and 88F6283. Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org> --- 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