From: Maxime Ripard <maxime.ripard@free-electrons.com> To: megous@megous.com Cc: dev@linux-sunxi.org, linux-arm-kernel@lists.infradead.org, Zhang Rui <rui.zhang@intel.com>, Eduardo Valentin <edubezval@gmail.com>, Chen-Yu Tsai <wens@csie.org>, open list <linux-kernel@vger.kernel.org>, "open list:THERMAL" <linux-pm@vger.kernel.org> Subject: Re: [PATCH v2 02/14] thermal: sun8i_ths: Add support for the thermal sensor on Allwinner H3 Date: Sat, 25 Jun 2016 09:10:44 +0200 [thread overview] Message-ID: <20160625071044.GB4000@lukather> (raw) In-Reply-To: <20160625034511.7966-3-megous@megous.com> [-- Attachment #1: Type: text/plain, Size: 11038 bytes --] On Sat, Jun 25, 2016 at 05:44:59AM +0200, megous@megous.com wrote: > From: Ondrej Jirman <megous@megous.com> > > This patch adds support for the sun8i thermal sensor on > Allwinner H3 SoC. > > Signed-off-by: Ondřej Jirman <megous@megous.com> > --- > v2: > - removed incorrect use of SID driver in sun8i_ths > - read calibration data directly from iomem > - better explanation for the thermal sensor driver > - dt documentation fixes > - dropped unncecessary macros and init code reorganization > - moved resource aquisition from init to probe function > - deassert reset after clock rate is set, not before > - enable irq after all other registers are configured > --- > drivers/thermal/Kconfig | 7 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/sun8i_ths.c | 260 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 268 insertions(+) > create mode 100644 drivers/thermal/sun8i_ths.c > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 2d702ca..d3209d9 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -351,6 +351,13 @@ config MTK_THERMAL > Enable this option if you want to have support for thermal management > controller present in Mediatek SoCs > > +config SUN8I_THS > + tristate "Thermal sensor driver for Allwinner H3" > + 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 > depends on HAS_IOMEM > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 10b07c1..7261ee8 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -51,3 +51,4 @@ obj-$(CONFIG_TEGRA_SOCTHERM) += tegra/ > obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o > obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o > obj-$(CONFIG_GENERIC_ADC_THERMAL) += thermal-generic-adc.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..9ba0f96 > --- /dev/null > +++ b/drivers/thermal/sun8i_ths.c > @@ -0,0 +1,260 @@ > +/* > + * Thermal sensor driver for Allwinner H3 SoC > + * > + * Copyright (C) 2016 Ondřej Jirman > + * Based on the work of Josef Gajdusek <atx@atx.name> > + * > + * 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/interrupt.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/reset.h> > +#include <linux/slab.h> > +#include <linux/thermal.h> > +#include <linux/printk.h> > + > +#define THS_H3_CTRL0 0x00 > +#define THS_H3_CTRL2 0x40 > +#define THS_H3_INT_CTRL 0x44 > +#define THS_H3_STAT 0x48 > +#define THS_H3_FILTER 0x70 > +#define THS_H3_CDATA 0x74 > +#define THS_H3_DATA 0x80 > + > +#define THS_H3_CTRL0_SENSOR_ACQ0(x) (x) > +#define THS_H3_CTRL2_SENSE_EN BIT(0) > +#define THS_H3_CTRL2_SENSOR_ACQ1(x) ((x) << 16) > +#define THS_H3_INT_CTRL_DATA_IRQ_EN BIT(8) > +#define THS_H3_INT_CTRL_THERMAL_PER(x) ((x) << 12) > +#define THS_H3_STAT_DATA_IRQ_STS BIT(8) > +#define THS_H3_FILTER_TYPE(x) ((x) << 0) > +#define THS_H3_FILTER_EN BIT(2) > + > +#define THS_H3_CLK_IN 40000000 /* Hz */ > +#define THS_H3_DATA_PERIOD 330 /* ms */ > + > +#define THS_H3_FILTER_TYPE_VALUE 2 /* average over 2^(n+1) samples */ > +#define THS_H3_FILTER_DIV (1 << (THS_H3_FILTER_TYPE_VALUE + 1)) > +#define THS_H3_INT_CTRL_THERMAL_PER_VALUE \ > + (THS_H3_DATA_PERIOD * (THS_H3_CLK_IN / 1000) / THS_H3_FILTER_DIV / 4096 - 1) > +#define THS_H3_CTRL0_SENSOR_ACQ0_VALUE 0x3f /* 16us */ > +#define THS_H3_CTRL2_SENSOR_ACQ1_VALUE 0x3f > + > +struct sun8i_ths_data { > + struct reset_control *reset; > + struct clk *clk; > + struct clk *busclk; > + void __iomem *regs; > + void __iomem *calreg; > + struct thermal_zone_device *tzd; > + u32 temp; > +}; > + > +static int sun8i_ths_get_temp(void *_data, int *out) > +{ > + struct sun8i_ths_data *data = _data; > + > + if (data->temp == 0) > + return -EINVAL; > + > + /* Formula and parameters from the Allwinner 3.4 kernel */ > + *out = 217000 - (int)((data->temp * 1000000) / 8253); > + return 0; > +} > + > +static irqreturn_t sun8i_ths_irq_thread(int irq, void *_data) > +{ > + struct sun8i_ths_data *data = _data; > + > + writel(THS_H3_STAT_DATA_IRQ_STS, data->regs + THS_H3_STAT); > + > + data->temp = readl(data->regs + THS_H3_DATA); > + if (data->temp) > + thermal_zone_device_update(data->tzd); > + > + return IRQ_HANDLED; > +} > + > +static void sun8i_ths_h3_init(struct sun8i_ths_data *data) > +{ > + u32 caldata; > + > + caldata = readl(data->calreg) & 0xfff; > + if (caldata != 0) > + writel(caldata, data->regs + THS_H3_CDATA); > + > + writel(THS_H3_CTRL0_SENSOR_ACQ0(THS_H3_CTRL0_SENSOR_ACQ0_VALUE), > + data->regs + THS_H3_CTRL0); > + 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); > + 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); > +} > + > +static const struct thermal_zone_of_device_ops sun8i_ths_thermal_ops = { > + .get_temp = sun8i_ths_get_temp, > +}; > + > +static int sun8i_ths_probe(struct platform_device *pdev) > +{ > + struct sun8i_ths_data *data; > + struct resource *res; > + int ret; > + int irq; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "no memory resources defined\n"); > + return -EINVAL; > + } > + > + 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; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res) { > + dev_err(&pdev->dev, "no calibration data memory resource defined\n"); > + return -EINVAL; > + } > + > + data->calreg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(data->calreg)) { > + ret = PTR_ERR(data->calreg); > + dev_err(&pdev->dev, "failed to ioremap THS registers: %d\n", ret); > + return ret; > + } Why did you remove the SID use through the nvmem framework ?! > + > + 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; > + > + 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; > + } > + > + 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 = clk_set_rate(data->clk, THS_H3_CLK_IN); > + if (ret) > + goto err_disable_ths; > + > + ret = reset_control_deassert(data->reset); > + if (ret) { > + dev_err(&pdev->dev, "reset deassert failed: %d\n", ret); > + goto err_disable_ths; > + } Having runtime_pm support would be great. > + sun8i_ths_h3_init(data); > + > + 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_assert_reset; > + } You reference data->tzd in your interrupt handler, and the interrupts have been activated before initializing that field. That is likely to cause a kernel crash when you receive an interrupt between your request_irq and that call. > + > + platform_set_drvdata(pdev, data); > + return 0; > + > +err_assert_reset: > + reset_control_assert(data->reset); > +err_disable_ths: > + clk_disable_unprepare(data->clk); > +err_disable_bus: > + clk_disable_unprepare(data->busclk); > + 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); > + reset_control_assert(data->reset); > + clk_disable_unprepare(data->clk); > + clk_disable_unprepare(data->busclk); > + return 0; > +} > + > +static const struct of_device_id sun8i_ths_id_table[] = { > + { .compatible = "allwinner,sun8i-h3-ths", }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, sun8i_ths_id_table); > + > +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("Ondřej Jirman <megous@megous.com>"); > +MODULE_DESCRIPTION("Thermal sensor driver for Allwinner H3 SoC"); > +MODULE_LICENSE("GPL v2"); Looks quite good otherwise. It looks very similar to the older touchscreen driver (without the touchscreen part). Have you tried to merge the two? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: maxime.ripard@free-electrons.com (Maxime Ripard) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 02/14] thermal: sun8i_ths: Add support for the thermal sensor on Allwinner H3 Date: Sat, 25 Jun 2016 09:10:44 +0200 [thread overview] Message-ID: <20160625071044.GB4000@lukather> (raw) In-Reply-To: <20160625034511.7966-3-megous@megous.com> On Sat, Jun 25, 2016 at 05:44:59AM +0200, megous at megous.com wrote: > From: Ondrej Jirman <megous@megous.com> > > This patch adds support for the sun8i thermal sensor on > Allwinner H3 SoC. > > Signed-off-by: Ond?ej Jirman <megous@megous.com> > --- > v2: > - removed incorrect use of SID driver in sun8i_ths > - read calibration data directly from iomem > - better explanation for the thermal sensor driver > - dt documentation fixes > - dropped unncecessary macros and init code reorganization > - moved resource aquisition from init to probe function > - deassert reset after clock rate is set, not before > - enable irq after all other registers are configured > --- > drivers/thermal/Kconfig | 7 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/sun8i_ths.c | 260 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 268 insertions(+) > create mode 100644 drivers/thermal/sun8i_ths.c > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 2d702ca..d3209d9 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -351,6 +351,13 @@ config MTK_THERMAL > Enable this option if you want to have support for thermal management > controller present in Mediatek SoCs > > +config SUN8I_THS > + tristate "Thermal sensor driver for Allwinner H3" > + 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 > depends on HAS_IOMEM > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 10b07c1..7261ee8 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -51,3 +51,4 @@ obj-$(CONFIG_TEGRA_SOCTHERM) += tegra/ > obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o > obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o > obj-$(CONFIG_GENERIC_ADC_THERMAL) += thermal-generic-adc.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..9ba0f96 > --- /dev/null > +++ b/drivers/thermal/sun8i_ths.c > @@ -0,0 +1,260 @@ > +/* > + * Thermal sensor driver for Allwinner H3 SoC > + * > + * Copyright (C) 2016 Ond?ej Jirman > + * Based on the work of Josef Gajdusek <atx@atx.name> > + * > + * 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/interrupt.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/reset.h> > +#include <linux/slab.h> > +#include <linux/thermal.h> > +#include <linux/printk.h> > + > +#define THS_H3_CTRL0 0x00 > +#define THS_H3_CTRL2 0x40 > +#define THS_H3_INT_CTRL 0x44 > +#define THS_H3_STAT 0x48 > +#define THS_H3_FILTER 0x70 > +#define THS_H3_CDATA 0x74 > +#define THS_H3_DATA 0x80 > + > +#define THS_H3_CTRL0_SENSOR_ACQ0(x) (x) > +#define THS_H3_CTRL2_SENSE_EN BIT(0) > +#define THS_H3_CTRL2_SENSOR_ACQ1(x) ((x) << 16) > +#define THS_H3_INT_CTRL_DATA_IRQ_EN BIT(8) > +#define THS_H3_INT_CTRL_THERMAL_PER(x) ((x) << 12) > +#define THS_H3_STAT_DATA_IRQ_STS BIT(8) > +#define THS_H3_FILTER_TYPE(x) ((x) << 0) > +#define THS_H3_FILTER_EN BIT(2) > + > +#define THS_H3_CLK_IN 40000000 /* Hz */ > +#define THS_H3_DATA_PERIOD 330 /* ms */ > + > +#define THS_H3_FILTER_TYPE_VALUE 2 /* average over 2^(n+1) samples */ > +#define THS_H3_FILTER_DIV (1 << (THS_H3_FILTER_TYPE_VALUE + 1)) > +#define THS_H3_INT_CTRL_THERMAL_PER_VALUE \ > + (THS_H3_DATA_PERIOD * (THS_H3_CLK_IN / 1000) / THS_H3_FILTER_DIV / 4096 - 1) > +#define THS_H3_CTRL0_SENSOR_ACQ0_VALUE 0x3f /* 16us */ > +#define THS_H3_CTRL2_SENSOR_ACQ1_VALUE 0x3f > + > +struct sun8i_ths_data { > + struct reset_control *reset; > + struct clk *clk; > + struct clk *busclk; > + void __iomem *regs; > + void __iomem *calreg; > + struct thermal_zone_device *tzd; > + u32 temp; > +}; > + > +static int sun8i_ths_get_temp(void *_data, int *out) > +{ > + struct sun8i_ths_data *data = _data; > + > + if (data->temp == 0) > + return -EINVAL; > + > + /* Formula and parameters from the Allwinner 3.4 kernel */ > + *out = 217000 - (int)((data->temp * 1000000) / 8253); > + return 0; > +} > + > +static irqreturn_t sun8i_ths_irq_thread(int irq, void *_data) > +{ > + struct sun8i_ths_data *data = _data; > + > + writel(THS_H3_STAT_DATA_IRQ_STS, data->regs + THS_H3_STAT); > + > + data->temp = readl(data->regs + THS_H3_DATA); > + if (data->temp) > + thermal_zone_device_update(data->tzd); > + > + return IRQ_HANDLED; > +} > + > +static void sun8i_ths_h3_init(struct sun8i_ths_data *data) > +{ > + u32 caldata; > + > + caldata = readl(data->calreg) & 0xfff; > + if (caldata != 0) > + writel(caldata, data->regs + THS_H3_CDATA); > + > + writel(THS_H3_CTRL0_SENSOR_ACQ0(THS_H3_CTRL0_SENSOR_ACQ0_VALUE), > + data->regs + THS_H3_CTRL0); > + 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); > + 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); > +} > + > +static const struct thermal_zone_of_device_ops sun8i_ths_thermal_ops = { > + .get_temp = sun8i_ths_get_temp, > +}; > + > +static int sun8i_ths_probe(struct platform_device *pdev) > +{ > + struct sun8i_ths_data *data; > + struct resource *res; > + int ret; > + int irq; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "no memory resources defined\n"); > + return -EINVAL; > + } > + > + 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; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res) { > + dev_err(&pdev->dev, "no calibration data memory resource defined\n"); > + return -EINVAL; > + } > + > + data->calreg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(data->calreg)) { > + ret = PTR_ERR(data->calreg); > + dev_err(&pdev->dev, "failed to ioremap THS registers: %d\n", ret); > + return ret; > + } Why did you remove the SID use through the nvmem framework ?! > + > + 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; > + > + 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; > + } > + > + 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 = clk_set_rate(data->clk, THS_H3_CLK_IN); > + if (ret) > + goto err_disable_ths; > + > + ret = reset_control_deassert(data->reset); > + if (ret) { > + dev_err(&pdev->dev, "reset deassert failed: %d\n", ret); > + goto err_disable_ths; > + } Having runtime_pm support would be great. > + sun8i_ths_h3_init(data); > + > + 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_assert_reset; > + } You reference data->tzd in your interrupt handler, and the interrupts have been activated before initializing that field. That is likely to cause a kernel crash when you receive an interrupt between your request_irq and that call. > + > + platform_set_drvdata(pdev, data); > + return 0; > + > +err_assert_reset: > + reset_control_assert(data->reset); > +err_disable_ths: > + clk_disable_unprepare(data->clk); > +err_disable_bus: > + clk_disable_unprepare(data->busclk); > + 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); > + reset_control_assert(data->reset); > + clk_disable_unprepare(data->clk); > + clk_disable_unprepare(data->busclk); > + return 0; > +} > + > +static const struct of_device_id sun8i_ths_id_table[] = { > + { .compatible = "allwinner,sun8i-h3-ths", }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, sun8i_ths_id_table); > + > +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("Ond?ej Jirman <megous@megous.com>"); > +MODULE_DESCRIPTION("Thermal sensor driver for Allwinner H3 SoC"); > +MODULE_LICENSE("GPL v2"); Looks quite good otherwise. It looks very similar to the older touchscreen driver (without the touchscreen part). Have you tried to merge the two? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160625/eae212e8/attachment-0001.sig>
next prev parent reply other threads:[~2016-06-25 7:10 UTC|newest] Thread overview: 183+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-06-25 3:44 [PATCH v2] Thermal regulation for Orange Pi PC and Orange Pi One megous at megous.com 2016-06-25 3:44 ` [PATCH v2 01/14] ARM: clk: sunxi: Add driver for the H3 THS clock megous 2016-06-25 3:44 ` megous at megous.com 2016-06-25 3:44 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 7:13 ` Maxime Ripard 2016-06-25 7:13 ` Maxime Ripard 2016-06-25 7:13 ` Maxime Ripard 2016-06-25 15:23 ` Ondřej Jirman 2016-06-25 15:23 ` Ondřej Jirman 2016-06-25 15:23 ` Ondřej Jirman 2016-06-28 11:52 ` Maxime Ripard 2016-06-28 11:52 ` Maxime Ripard 2016-06-28 11:52 ` Maxime Ripard 2016-06-25 3:44 ` [PATCH v2 02/14] thermal: sun8i_ths: Add support for the thermal sensor on Allwinner H3 megous 2016-06-25 3:44 ` megous at megous.com 2016-06-25 3:44 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 7:10 ` Maxime Ripard [this message] 2016-06-25 7:10 ` Maxime Ripard 2016-06-25 15:12 ` Ondřej Jirman 2016-06-25 15:12 ` Ondřej Jirman 2016-06-28 11:39 ` Maxime Ripard 2016-06-28 11:39 ` Maxime Ripard 2016-06-28 11:39 ` Maxime Ripard 2016-06-25 3:45 ` [PATCH v2 03/14] dt-bindings: document sun8i_ths - H3 thermal sensor driver megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-28 20:56 ` Rob Herring 2016-06-28 20:56 ` Rob Herring 2016-06-28 20:56 ` Rob Herring 2016-06-25 3:45 ` [PATCH v2 04/14] regulator: SY8106A regulator driver megous 2016-06-25 3:45 ` megous at megous.com 2016-06-26 11:26 ` Mark Brown 2016-06-26 11:26 ` Mark Brown 2016-06-26 15:07 ` Ondřej Jirman 2016-06-26 15:07 ` Ondřej Jirman 2016-06-27 14:54 ` Mark Brown 2016-06-27 14:54 ` Mark Brown 2016-06-28 16:27 ` Ondřej Jirman 2016-06-28 16:27 ` Ondřej Jirman 2016-06-27 15:10 ` Mark Brown 2016-06-27 15:10 ` Mark Brown 2016-06-25 3:45 ` [PATCH v2 05/14] dt-bindings: document " megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-26 11:27 ` Mark Brown 2016-06-26 11:27 ` Mark Brown 2016-06-26 11:27 ` Mark Brown 2016-06-26 15:10 ` Ondřej Jirman 2016-06-26 15:10 ` Ondřej Jirman 2016-06-26 15:10 ` Ondřej Jirman 2016-06-26 18:52 ` Mark Brown 2016-06-26 18:52 ` Mark Brown 2016-06-26 18:52 ` Mark Brown 2016-06-25 3:45 ` [PATCH v2 06/14] ARM: sun8i: clk: Add clk-factor rate application method megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-30 20:40 ` Maxime Ripard 2016-06-30 20:40 ` Maxime Ripard 2016-06-30 20:40 ` Maxime Ripard 2016-07-01 0:50 ` Ondřej Jirman 2016-07-01 0:50 ` Ondřej Jirman 2016-07-01 0:50 ` Ondřej Jirman 2016-07-01 5:37 ` Jean-Francois Moine 2016-07-01 5:37 ` Jean-Francois Moine 2016-07-01 6:34 ` Ondřej Jirman 2016-07-01 6:34 ` Ondřej Jirman 2016-07-01 6:34 ` Ondřej Jirman 2016-07-01 7:47 ` Jean-Francois Moine 2016-07-01 7:47 ` Jean-Francois Moine 2016-07-01 7:47 ` Jean-Francois Moine 2016-07-15 8:53 ` Maxime Ripard 2016-07-15 8:53 ` Maxime Ripard 2016-07-15 8:53 ` Maxime Ripard 2016-07-15 10:38 ` Ondřej Jirman 2016-07-15 10:38 ` Ondřej Jirman 2016-07-15 10:38 ` Ondřej Jirman 2016-07-15 13:27 ` Jean-Francois Moine 2016-07-15 13:27 ` Jean-Francois Moine 2016-07-15 13:48 ` Ondřej Jirman 2016-07-15 13:48 ` Ondřej Jirman 2016-07-15 13:48 ` Ondřej Jirman 2016-07-15 14:22 ` [linux-sunxi] " Michal Suchanek 2016-07-15 14:22 ` Michal Suchanek 2016-07-15 14:22 ` Michal Suchanek 2016-07-15 16:33 ` Ondřej Jirman 2016-07-15 16:33 ` Ondřej Jirman 2016-07-21 9:51 ` Maxime Ripard 2016-07-21 9:51 ` Maxime Ripard 2016-07-21 9:51 ` Maxime Ripard 2016-07-21 9:48 ` Maxime Ripard 2016-07-21 9:48 ` Maxime Ripard 2016-07-21 9:48 ` Maxime Ripard 2016-07-21 9:52 ` Ondřej Jirman 2016-07-21 9:52 ` Ondřej Jirman 2016-07-26 6:32 ` Maxime Ripard 2016-07-26 6:32 ` Maxime Ripard 2016-07-28 11:27 ` Changed: sunxi-ng clock code - NKMP clock implementation is wrong Ondřej Jirman 2016-07-28 11:27 ` Ondřej Jirman 2016-07-28 11:27 ` Ondřej Jirman 2016-07-28 11:38 ` [linux-sunxi] " Chen-Yu Tsai 2016-07-28 11:38 ` Chen-Yu Tsai 2016-07-28 11:38 ` Chen-Yu Tsai 2016-07-28 21:00 ` Maxime Ripard 2016-07-28 21:00 ` Maxime Ripard 2016-07-28 21:00 ` Maxime Ripard 2016-07-28 22:01 ` Ondřej Jirman 2016-07-28 22:01 ` Ondřej Jirman 2016-07-28 22:01 ` Ondřej Jirman 2016-07-31 10:31 ` Maxime Ripard 2016-07-31 10:31 ` Maxime Ripard 2016-07-31 10:31 ` Maxime Ripard 2016-07-31 22:01 ` Ondřej Jirman 2016-07-31 22:01 ` Ondřej Jirman 2016-07-31 22:01 ` Ondřej Jirman 2016-08-31 20:25 ` Maxime Ripard 2016-08-31 20:25 ` Maxime Ripard 2016-08-31 20:25 ` Maxime Ripard 2016-07-01 0:53 ` [PATCH v2 06/14] ARM: sun8i: clk: Add clk-factor rate application method Ondřej Jirman 2016-07-01 0:53 ` Ondřej Jirman 2016-07-01 0:53 ` Ondřej Jirman 2016-07-15 8:19 ` Maxime Ripard 2016-07-15 8:19 ` Maxime Ripard 2016-07-15 8:19 ` Maxime Ripard 2016-06-25 3:45 ` [PATCH v2 07/14] ARM: dts: sun8i: Use sun8i-h3-pll1-clk for pll1 in H3 megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 3:45 ` [PATCH v2 08/14] ARM: dts: sun8i: Add thermal sensor node to the sun8i-h3.dtsi megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 3:45 ` [PATCH v2 09/14] ARM: dts: sun8i: Add cpu0 label to sun8i-h3.dtsi megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 3:45 ` [PATCH v2 10/14] ARM: dts: sun8i: Add r_twi I2C controller megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 7:16 ` Maxime Ripard 2016-06-25 7:16 ` Maxime Ripard 2016-06-25 7:16 ` Maxime Ripard 2016-06-25 3:45 ` [PATCH v2 11/14] ARM: dts: sun8i: Add sy8106a regulator to Orange Pi PC megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 3:45 ` [PATCH v2 12/14] ARM: dts: sun8i: Setup CPU operating points for Onrage PI PC megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 3:45 ` [PATCH v2 13/14] ARM: dts: sun8i: Add gpio-regulator used on Orange Pi One megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-25 7:18 ` Maxime Ripard 2016-06-25 7:18 ` Maxime Ripard 2016-06-25 7:18 ` Maxime Ripard 2016-06-25 3:45 ` [PATCH v2 14/14] ARM: dts: sun8i: Enable DVFS " megous 2016-06-25 3:45 ` megous at megous.com 2016-06-25 3:45 ` megous-5qf/QAjKc83QT0dZR+AlfA 2016-06-30 11:13 ` [linux-sunxi] " Michal Suchanek 2016-06-30 11:13 ` Michal Suchanek 2016-06-30 11:13 ` Michal Suchanek 2016-06-30 14:19 ` Ondřej Jirman 2016-06-30 14:19 ` Ondřej Jirman 2016-06-30 14:19 ` Ondřej Jirman 2016-06-30 15:16 ` [linux-sunxi] " Michal Suchanek 2016-06-30 15:16 ` Michal Suchanek 2016-06-30 15:16 ` Michal Suchanek 2016-06-30 15:32 ` [linux-sunxi] " Ondřej Jirman 2016-06-30 15:32 ` Ondřej Jirman 2016-06-30 15:32 ` Ondřej Jirman 2016-06-30 15:50 ` [linux-sunxi] " Michal Suchanek 2016-06-30 15:50 ` Michal Suchanek 2016-06-30 15:50 ` Michal Suchanek 2016-06-30 15:53 ` [linux-sunxi] " Ondřej Jirman 2016-06-30 15:53 ` Ondřej Jirman 2016-06-30 15:53 ` Ondřej Jirman 2016-07-01 10:54 ` [linux-sunxi] " Michal Suchanek 2016-07-01 10:54 ` Michal Suchanek 2016-07-01 10:54 ` Michal Suchanek 2016-06-30 14:23 ` [linux-sunxi] " Siarhei Siamashka 2016-06-30 14:23 ` Siarhei Siamashka 2016-06-30 14:23 ` Siarhei Siamashka 2016-07-01 1:17 ` [linux-sunxi] " Ondřej Jirman 2016-07-01 1:17 ` Ondřej Jirman 2016-07-01 1:17 ` Ondřej Jirman 2016-07-22 0:41 ` [linux-sunxi] " Ondřej Jirman 2016-07-22 0:41 ` Ondřej Jirman 2016-07-22 0:41 ` Ondřej Jirman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160625071044.GB4000@lukather \ --to=maxime.ripard@free-electrons.com \ --cc=dev@linux-sunxi.org \ --cc=edubezval@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=megous@megous.com \ --cc=rui.zhang@intel.com \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.