Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933801AbbKSDpL (ORCPT ); Wed, 18 Nov 2015 22:45:11 -0500 Received: from smtp.csie.ntu.edu.tw ([140.112.30.61]:57896 "EHLO smtp.csie.ntu.edu.tw" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933724AbbKSDpI (ORCPT ); Wed, 18 Nov 2015 22:45:08 -0500 MIME-Version: 1.0 In-Reply-To: <20151118205147.GA19779@maud.lan> References: <20151118205147.GA19779@maud.lan> From: Chen-Yu Tsai Date: Thu, 19 Nov 2015 11:44:44 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] thermal: Add support for Sunxi THS on the Allwinner H3 To: Josef Gajdusek Cc: linux-sunxi , linux-clk , linux-pm@vger.kernel.org, linux-kernel , linux-arm-kernel , devicetree , gpatchesrdh@mveas.com, Mike Turquette , Hans De Goede , Stephen Boyd , Michael Turquette , Emilio Lopez , Russell King - ARM Linux , Eduardo Valentin , Zhang Rui , Chen-Yu Tsai , Maxime Ripard , Kumar Gala , Ian Campbell , Mark Rutland , Pawel Moll , Rob Herring Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 19950 Lines: 542 On Thu, Nov 19, 2015 at 4:51 AM, Josef Gajdusek wrote: > This patch adds support for the Sunxi thermal sensor on the Allwinner H3. > Also adds declaration of the H3 THS clock to clk-sunxi.c ignoring the > dividers as they are not continuous (clk-divider.c cannot be used as it > does not support setting an enable bit). That is not right. See below. > Should be easily extendable for the A33/A83T/... as they have similar but > not completely identical sensors. > > Signed-off-by: Josef Gajdusek > --- > Documentation/devicetree/bindings/clock/sunxi.txt | 1 + > .../devicetree/bindings/thermal/sunxi-ths.txt | 24 ++ > arch/arm/boot/dts/sun8i-h3.dtsi | 27 +++ > drivers/clk/sunxi/clk-sunxi.c | 16 ++ > drivers/thermal/Kconfig | 7 + > drivers/thermal/Makefile | 1 + > drivers/thermal/sunxi_ths.c | 263 +++++++++++++++++++++ Thanks for working on this. This patch should be split into 5 patches: clock binding, clock driver, THS binding, THS driver, and DTS updates. > 7 files changed, 339 insertions(+) > create mode 100644 Documentation/devicetree/bindings/thermal/sunxi-ths.txt > create mode 100644 drivers/thermal/sunxi_ths.c > > diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt > index 23e7bce..6d63b35 100644 > --- a/Documentation/devicetree/bindings/clock/sunxi.txt > +++ b/Documentation/devicetree/bindings/clock/sunxi.txt > @@ -73,6 +73,7 @@ Required properties: > "allwinner,sun8i-h3-usb-clk" - for usb gates + resets on H3 > "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80 > "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80 > + "allwinner,sun8i-h3-ths-clk" - for THS on H3 > > Required properties for all clocks: > - reg : shall be the control register address for the clock. > diff --git a/Documentation/devicetree/bindings/thermal/sunxi-ths.txt b/Documentation/devicetree/bindings/thermal/sunxi-ths.txt > new file mode 100644 > index 0000000..75c9211 > --- /dev/null > +++ b/Documentation/devicetree/bindings/thermal/sunxi-ths.txt > @@ -0,0 +1,24 @@ > +* sunxi THS > + > +Required properties: > +- compatible : "allwinner,sun8i-h3-ths" > +- reg : Address range of the thermal registers and location of the calibration > + value The latter is part of the Security ID (efuses) block, for which we have Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt. Please use a phandle to the security id block instead. See Documentation/devicetree/bindings/nvmem/nvmem.txt for the generic nvmem bindings. > +- 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 > + > +Example: > +ths: ths@01c25000 { > + #thermal-sensor-cells = <0>; > + compatible = "allwinner,sun8i-h3-ths"; > + reg = <0x01c25000 0x88>, <0x01c14234 0x4>; > + interrupts = ; > + resets = <&bus_rst 136>; > + reset-names = "ahb"; > + clocks = <&bus_gates 72>, <&ths_clk>; > + clock-names = "ahb", "ths"; > +}; > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi > index 0faa38a..b82881d 100644 > --- a/arch/arm/boot/dts/sun8i-h3.dtsi > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi > @@ -77,6 +77,14 @@ > }; > }; > > + thermal-zones { > + cpu_thermal: cpu_thermal { > + polling-delay-passive = <1000>; > + polling-delay = <5000>; > + thermal-sensors = <&ths 0>; > + }; > + }; > + > timer { > compatible = "arm,armv7-timer"; > interrupts = , > @@ -236,6 +244,14 @@ > "ahb1_ephy", "ahb1_dbg"; > }; > > + ths_clk: clk@01c20074 { > + #clock-cells = <0>; > + compatible = "allwinner,sun8i-h3-ths-clk"; > + reg = <0x01c20074 0x4>; > + clocks = <&osc24M>; > + clock-output-names = "ths"; > + }; > + > mmc0_clk: clk@01c20088 { > #clock-cells = <1>; > compatible = "allwinner,sun4i-a10-mmc-clk"; > @@ -522,6 +538,17 @@ > interrupts = ; > }; > > + ths: ths@01c25000 { > + #thermal-sensor-cells = <0>; > + compatible = "allwinner,sun8i-h3-ths"; > + reg = <0x01c25000 0x88>, <0x01c14234 0x4>; > + interrupts = ; > + resets = <&bus_rst 104>; > + reset-names = "ahb"; > + clocks = <&bus_gates 72>, <&ths_clk>; > + clock-names = "ahb", "ths"; > + }; > + > uart0: serial@01c28000 { > compatible = "snps,dw-apb-uart"; > reg = <0x01c28000 0x400>; > diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c > index 6293c65..637401a 100644 > --- a/drivers/clk/sunxi/clk-sunxi.c > +++ b/drivers/clk/sunxi/clk-sunxi.c > @@ -604,6 +604,13 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate, > *p = calcp; > } > > +static void sun8i_h3_get_ths_factors(u32 *freq, u32 parent_rate, > + u8 *n, u8 *k, u8 *m, u8 *p) > +{ > + /* Ignore the dividers as they are not continuous */ > + *freq = parent_rate; > +} > + > /** > * sunxi_factors_clk_setup() - Setup function for factor clocks > */ > @@ -668,6 +675,8 @@ static struct clk_factors_config sun4i_apb1_config = { > .pwidth = 2, > }; > > +static struct clk_factors_config sun8i_h3_ths_config = {}; > + > /* user manual says "n" but it's really "p" */ > static struct clk_factors_config sun7i_a20_out_config = { > .mshift = 8, > @@ -734,6 +743,12 @@ static const struct factors_data sun7i_a20_out_data __initconst = { > .getter = sun7i_a20_get_out_factors, > }; > > +static const struct factors_data sun8i_h3_ths_data __initconst = { > + .enable = 31, > + .table = &sun8i_h3_ths_config, > + .getter = sun8i_h3_get_ths_factors, > +}; > + > static struct clk * __init sunxi_factors_clk_setup(struct device_node *node, > const struct factors_data *data) > { > @@ -1127,6 +1142,7 @@ static const struct of_device_id clk_factors_match[] __initconst = { > {.compatible = "allwinner,sun5i-a13-ahb-clk", .data = &sun5i_a13_ahb_data,}, > {.compatible = "allwinner,sun4i-a10-apb1-clk", .data = &sun4i_apb1_data,}, > {.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,}, > + {.compatible = "allwinner,sun8i-h3-ths-clk", .data = &sun8i_h3_ths_data,}, Please don't add clocks to this list. We are breaking them out into individual CLK_OF_DECLARE statements so they work better with the CCF's clock dependency probe ordering stuff. Instead, do a standalone driver under drivers/clk/sunxi, like drivers/clk/sunxi/clk-a23-ths.c Note we normally name a module/driver/compatible based on the first SoC it appeared in. Since the dividers aren't contiguous, you should use a divider table. And it also has a clock gate, so you should probably use composite clk to bring together the gate and divider into a single clk. See https://github.com/wens/linux/blob/sun4i-ve-clocks/drivers/clk/sunxi/clk-a10-ve.c for a similar driver. You can ignore the reset control bits. This driver hasn't been submitted. > {} > }; > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index c463c89..0111d4d 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -365,6 +365,13 @@ config INTEL_PCH_THERMAL > Thermal reporting device will provide temperature reading, > programmable trip points and other information. > > +config SUNXI_THS > + tristate "Sunxi THS driver" > + depends on ARCH_SUNXI > + depends on OF > + help > + Enable this to support thermal reporting on some newer Allwinner SoC. Nit: SoCs, plural. > + > menu "Texas Instruments thermal drivers" > depends on ARCH_HAS_BANDGAP || COMPILE_TEST > source "drivers/thermal/ti-soc-thermal/Kconfig" > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index cfae6a6..3a25e3c 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -48,3 +48,4 @@ obj-$(CONFIG_INTEL_PCH_THERMAL) += intel_pch_thermal.o > obj-$(CONFIG_ST_THERMAL) += st/ > obj-$(CONFIG_TEGRA_SOCTHERM) += tegra_soctherm.o > obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o > +obj-$(CONFIG_SUNXI_THS) += sunxi_ths.o Hmm, is there supposed to be some sort of ordering here? > diff --git a/drivers/thermal/sunxi_ths.c b/drivers/thermal/sunxi_ths.c > new file mode 100644 > index 0000000..650cd39 > --- /dev/null > +++ b/drivers/thermal/sunxi_ths.c > @@ -0,0 +1,263 @@ > +/* > + * Sunxi THS driver > + * > + * Copyright (C) 2015 Josef Gajdusek > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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 0 > + > +#define THS_H3_CTRL1_ADC_CALI_EN 17 > +#define THS_H3_CTRL1_OP_BIAS 20 > + > +#define THS_H3_CTRL2_SENSE_EN 0 > +#define THS_H3_CTRL2_SENSOR_ACQ1 16 > + > +#define THS_H3_INT_CTRL_ALARM_INT_EN 0 > +#define THS_H3_INT_CTRL_SHUT_INT_EN 4 > +#define THS_H3_INT_CTRL_DATA_IRQ_EN 8 > +#define THS_H3_INT_CTRL_THERMAL_PER 12 > + > +#define THS_H3_STAT_ALARM_INT_STS 0 > +#define THS_H3_STAT_SHUT_INT_STS 4 > +#define THS_H3_STAT_DATA_IRQ_STS 8 > +#define THS_H3_STAT_ALARM_OFF_STS 12 > + > +#define THS_H3_ALARM_CTRL_ALARM0_T_HYST 0 > +#define THS_H3_ALARM_CTRL_ALARM0_T_HOT 16 > + > +#define THS_H3_SHUTDOWN_CTRL_SHUT0_T_HOT 16 > + > +#define THS_H3_FILTER_TYPE 0 > +#define THS_H3_FILTER_EN 2 > + > +struct sunxi_ths_data { > + struct sunxi_ths_type *type; > + struct reset_control *reset; > + struct clk *clk; > + struct clk *busclk; > + void __iomem *regs; > + void __iomem *calreg; > + struct platform_device *pdev; > + struct thermal_zone_device *tzd; > + int irq; > +}; > + > +struct sunxi_ths_type { > + int (*init)(struct sunxi_ths_data *); > + int (*get_temp)(struct sunxi_ths_data *, int *out); > +}; Move this before struct sunxi_ths_data. > + > +static int sunxi_ths_reg_to_temperature(int32_t reg, int divisor, int minus) > +{ > + return minus - (reg * 1000000) / divisor; > +} > + > +static int sunxi_ths_get_temp(void *_data, int *out) > +{ > + struct sunxi_ths_data *data = _data; > + > + return data->type->get_temp(data, out); > +} > + > +static int sunxi_ths_h3_init(struct sunxi_ths_data *data) > +{ > + if (data->calreg) > + writel(readl(data->calreg) & 0xfff, data->regs + THS_H3_CDATA); Again, use the nvmem API to read out the data. > + /* Magical constants mostly taken from Allwinner 3.4 kernel. > + * Seem to work fine, though this could be configurable in DT/sysft sysfs Please check the user manual for the meaning of these constants. They are not magic. ACQ0, ACQ1, and THERMAL_PER control the time each step of acquiring the temperature takes. The programming guideline requires "THERMAL_PER > ACQ1 + ACQ0 + conversion time" The hardware also has an interrupt line, which can be used to signal there's data worth reading, or a warning threshold was exceeded. You can also look at drivers/input/touchscreen/sun4i-ts.c, which includes the older thermal sensor. > + */ > + writel(0xff << THS_H3_CTRL0_SENSOR_ACQ0, > + data->regs + THS_H3_CTRL0); > + writel((0x3f << THS_H3_CTRL2_SENSOR_ACQ1) | BIT(THS_H3_CTRL2_SENSE_EN), > + data->regs + THS_H3_CTRL2); > + writel((0x390 << THS_H3_INT_CTRL_THERMAL_PER) | BIT(THS_H3_INT_CTRL_DATA_IRQ_EN), > + data->regs + THS_H3_INT_CTRL); > + writel(BIT(THS_H3_FILTER_EN) | (0x2 << THS_H3_FILTER_TYPE), > + data->regs + THS_H3_FILTER); > + return 0; > +} > + > +static int sunxi_ths_h3_get_temp(struct sunxi_ths_data *data, int *out) > +{ > + *out = sunxi_ths_reg_to_temperature(readl(data->regs + THS_H3_DATA), > + 8253, 217000); > + return 0; > +} > + > +static const struct thermal_zone_of_device_ops sunxi_ths_thermal_ops = { > + .get_temp = sunxi_ths_get_temp, > +}; > + > +static const struct sunxi_ths_type sunxi_ths_device_h3 = { > + .init = sunxi_ths_h3_init, > + .get_temp = sunxi_ths_h3_get_temp, > +}; > + > +static const struct of_device_id sunxi_ths_id_table[] = { > + { > + .compatible = "allwinner,sun8i-h3-ths", > + .data = &sunxi_ths_device_h3, > + }, > + { > + /* sentinel */ > + }, > +}; > +MODULE_DEVICE_TABLE(of, sunxi_ths_id_table); > + > +static int sunxi_ths_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + const struct of_device_id *match; > + struct sunxi_ths_data *data; > + struct resource *res; > + int ret; > + > + match = of_match_node(sunxi_ths_id_table, np); > + if (!match) > + return -ENXIO; > + > + data = > + devm_kzalloc(&pdev->dev, sizeof(struct sunxi_ths_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->type = (struct sunxi_ths_type *) match->data; > + data->pdev = pdev; > + > + 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; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + data->calreg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(data->calreg)) > + data->calreg = NULL; /* No calibration register */ The calibration data is part of the Security ID efuse module, for which we have an nvmem driver. Please use the nvmem API for this. > + > + 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_optional(&pdev->dev, "ahb"); Why is this optional? Regards ChenYu > + 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 = reset_control_deassert(data->reset); > + if (ret) { > + dev_err(&pdev->dev, "reset deassert failed: %d\n", ret); > + goto err_disable_ths; > + } > + > + ret = data->type->init(data); > + if (ret) > + goto err_reset_assert; > + > + data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data, > + &sunxi_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_reset_assert; > + } > + > + platform_set_drvdata(pdev, data); > + return 0; > + > +err_reset_assert: > + 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 sunxi_ths_remove(struct platform_device *pdev) > +{ > + struct sunxi_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 struct platform_driver sunxi_ths_driver = { > + .probe = sunxi_ths_probe, > + .remove = sunxi_ths_remove, > + .driver = { > + .name = "sunxi_ths", > + .of_match_table = sunxi_ths_id_table, > + }, > +}; > + > +module_platform_driver(sunxi_ths_driver); > + > +MODULE_AUTHOR("Josef Gajdusek "); > +MODULE_DESCRIPTION("Sunxi THS driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.4.10 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/