* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-07 16:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel @ 2017-01-07 16:55 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Lee Jones, Eric Anholt, linux-pm,
linux-rpi-kernel, linux-arm-kernel
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
Add basic thermal driver for bcm2835 SOC.
This driver currently relies on the firmware setting up the
tsense HW block and does not set it up itself.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Acked-by: Eric Anholt <eric@anholt.net>
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/thermal/Kconfig | 8 +
drivers/thermal/Makefile | 1 +
drivers/thermal/bcm2835_thermal.c | 354 ++++++++++++++++++++++++++++++++++++++
3 files changed, 363 insertions(+)
create mode 100644 drivers/thermal/bcm2835_thermal.c
ChangeLog:
V1 -> V2: added specific settings depending on compatiblity
added trip point based on register
setting up ctrl-register if HW is not enabled by firmware
as per recommendation of Eric (untested)
check that clock frequency is in range
(1.9 - 5MHz - as per comment in clk-bcm2835.c)
V2 -> V4: moved back to thermal (not using bcm sub-directory)
set polling interval to 1second (was 0ms, so interrupt driven)
V5 -> V6: added correct depends in KConfig
removed defined default for RESET_DELAY
removed obvious comments
clarify HW setup comments if not set up by FW already
move clk_prepare_enable to an earlier stage and add error handling
clarify warning when TS-clock runs out of recommended range
clk_disable_unprepare added in bcm2835_thermal_remove
added comment on recommended temperature ranges for SOC
V6 -> V7: removed depends on ARCH_BCM2836 || ARCH_BCM2837 in Kconfig
V7 -> V8: rebased
V8 -> V9: moved to use the thermal framework offset and slope in
thermal_zone_parameters as per request
A few additional notes on this latest patch:
* all the other portions of the patch (dt, bindings, defconf) have already
been merged into 4.10.0-rc2 - this only leaves the driver itself which
is now just a single patch to get applied.
* the driver has been moved to use thermal_zone_parameters->offset and slope
making use of the thermal_zone_get_offset and thermal_zone_get_slope
methods where relevant
Note that we cannot use those during initial setup of the HW - this code
section is typically not used anyway, as the FW sets up the device already.
This so modified patch actually increases code size by 14 lines:
drivers/thermal/bcm2835_thermal.c | 42 ++++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 14 deletions(-)
* /sys/class/thermal/*/ contains now slope and offset files
and those expose the correct values
* for some reason on module load the kernel emits now the message:
(NULL device *): hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
But this seems framework specific and needs to get addressed there.
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index c2c056c..18f2de6 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -436,4 +436,12 @@ depends on (ARCH_QCOM && OF) || COMPILE_TEST
source "drivers/thermal/qcom/Kconfig"
endmenu
+config BCM2835_THERMAL
+ tristate "Thermal sensors on bcm2835 SoC"
+ depends on ARCH_BCM2835 || COMPILE_TEST
+ depends on HAS_IOMEM
+ depends on OF
+ help
+ Support for thermal sensors on Broadcom bcm2835 SoCs.
+
endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 6a3d7b5..677c6d9 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -56,3 +56,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_BCM2835_THERMAL) += bcm2835_thermal.o
diff --git a/drivers/thermal/bcm2835_thermal.c b/drivers/thermal/bcm2835_thermal.c
new file mode 100644
index 0000000..5e2fea9
--- /dev/null
+++ b/drivers/thermal/bcm2835_thermal.c
@@ -0,0 +1,354 @@
+/*
+ * Driver for Broadcom BCM2835 soc temperature sensor
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define BCM2835_TS_TSENSCTL 0x00
+#define BCM2835_TS_TSENSSTAT 0x04
+
+#define BCM2835_TS_TSENSCTL_PRWDW BIT(0)
+#define BCM2835_TS_TSENSCTL_RSTB BIT(1)
+#define BCM2835_TS_TSENSCTL_CTRL_BITS 3
+#define BCM2835_TS_TSENSCTL_CTRL_SHIFT 2
+#define BCM2835_TS_TSENSCTL_CTRL_MASK \
+ GENMASK(BCM2835_TS_TSENSCTL_CTRL_BITS + \
+ BCM2835_TS_TSENSCTL_CTRL_SHIFT - 1, \
+ BCM2835_TS_TSENSCTL_CTRL_SHIFT)
+#define BCM2835_TS_TSENSCTL_CTRL_DEFAULT 1
+#define BCM2835_TS_TSENSCTL_EN_INT BIT(5)
+#define BCM2835_TS_TSENSCTL_DIRECT BIT(6)
+#define BCM2835_TS_TSENSCTL_CLR_INT BIT(7)
+#define BCM2835_TS_TSENSCTL_THOLD_SHIFT 8
+#define BCM2835_TS_TSENSCTL_THOLD_BITS 10
+#define BCM2835_TS_TSENSCTL_THOLD_MASK \
+ GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
+ BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
+ BCM2835_TS_TSENSCTL_THOLD_SHIFT)
+#define BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT 18
+#define BCM2835_TS_TSENSCTL_RSTDELAY_BITS 8
+#define BCM2835_TS_TSENSCTL_REGULEN BIT(26)
+
+#define BCM2835_TS_TSENSSTAT_DATA_BITS 10
+#define BCM2835_TS_TSENSSTAT_DATA_SHIFT 0
+#define BCM2835_TS_TSENSSTAT_DATA_MASK \
+ GENMASK(BCM2835_TS_TSENSSTAT_DATA_BITS + \
+ BCM2835_TS_TSENSSTAT_DATA_SHIFT - 1, \
+ BCM2835_TS_TSENSSTAT_DATA_SHIFT)
+#define BCM2835_TS_TSENSSTAT_VALID BIT(10)
+#define BCM2835_TS_TSENSSTAT_INTERRUPT BIT(11)
+
+struct bcm2835_thermal_info {
+ int offset;
+ int slope;
+ int trip_temp;
+};
+
+struct bcm2835_thermal_data {
+ const struct bcm2835_thermal_info *info;
+ void __iomem *regs;
+ struct clk *clk;
+ struct dentry *debugfsdir;
+};
+
+static int bcm2835_thermal_adc2temp(u32 adc, int offset, int slope)
+{
+ return offset + slope * adc;
+}
+
+static int bcm2835_thermal_temp2adc(int temp, int offset, int slope)
+{
+ temp -= offset;
+ temp /= slope;
+
+ if (temp < 0)
+ temp = 0;
+ if (temp >= BIT(BCM2835_TS_TSENSSTAT_DATA_BITS))
+ temp = BIT(BCM2835_TS_TSENSSTAT_DATA_BITS) - 1;
+
+ return temp;
+}
+
+static int bcm2835_thermal_get_trip_type(
+ struct thermal_zone_device *tz, int trip,
+ enum thermal_trip_type *type)
+{
+ *type = THERMAL_TRIP_CRITICAL;
+ return 0;
+}
+
+static int bcm2835_thermal_get_trip_temp(
+ struct thermal_zone_device *tz, int trip, int *temp)
+{
+ struct bcm2835_thermal_data *data = tz->devdata;
+ u32 val = readl(data->regs + BCM2835_TS_TSENSCTL);
+
+ /* get the THOLD bits */
+ val &= BCM2835_TS_TSENSCTL_THOLD_MASK;
+ val >>= BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+ /* if it is zero then use the info value */
+ if (val)
+ *temp = bcm2835_thermal_adc2temp(
+ val,
+ thermal_zone_get_offset(tz),
+ thermal_zone_get_slope(tz));
+ else
+ *temp = data->info->trip_temp;
+
+ return 0;
+}
+
+static int bcm2835_thermal_get_temp(struct thermal_zone_device *tz,
+ int *temp)
+{
+ struct bcm2835_thermal_data *data = tz->devdata;
+ u32 val = readl(data->regs + BCM2835_TS_TSENSSTAT);
+
+ if (!(val & BCM2835_TS_TSENSSTAT_VALID))
+ return -EIO;
+
+ val &= BCM2835_TS_TSENSSTAT_DATA_MASK;
+
+ *temp = bcm2835_thermal_adc2temp(
+ val,
+ thermal_zone_get_offset(tz),
+ thermal_zone_get_slope(tz));
+
+ return 0;
+}
+
+static const struct debugfs_reg32 bcm2835_thermal_regs[] = {
+ {
+ .name = "ctl",
+ .offset = 0
+ },
+ {
+ .name = "stat",
+ .offset = 4
+ }
+};
+
+static void bcm2835_thermal_debugfs(struct platform_device *pdev)
+{
+ struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+ struct bcm2835_thermal_data *data = tz->devdata;
+ struct debugfs_regset32 *regset;
+
+ data->debugfsdir = debugfs_create_dir("bcm2835_thermal", NULL);
+ if (!data->debugfsdir)
+ return;
+
+ regset = devm_kzalloc(&pdev->dev, sizeof(*regset), GFP_KERNEL);
+ if (!regset)
+ return;
+
+ regset->regs = bcm2835_thermal_regs;
+ regset->nregs = ARRAY_SIZE(bcm2835_thermal_regs);
+ regset->base = data->regs;
+
+ debugfs_create_regset32("regset", S_IRUGO,
+ data->debugfsdir, regset);
+}
+
+static struct thermal_zone_device_ops bcm2835_thermal_ops = {
+ .get_temp = bcm2835_thermal_get_temp,
+ .get_trip_temp = bcm2835_thermal_get_trip_temp,
+ .get_trip_type = bcm2835_thermal_get_trip_type,
+};
+
+static const struct of_device_id bcm2835_thermal_of_match_table[];
+static int bcm2835_thermal_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+ struct thermal_zone_device *tz;
+ struct thermal_zone_params *tzp;
+ const struct bcm2835_thermal_info *ti;
+ struct bcm2835_thermal_data *data;
+ struct resource *res;
+ int err;
+ u32 val;
+ unsigned long rate;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ tzp = devm_kzalloc(&pdev->dev, sizeof(*tzp), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ match = of_match_device(bcm2835_thermal_of_match_table,
+ &pdev->dev);
+ if (!match)
+ return -EINVAL;
+ ti = match->data;
+ data->info = ti;
+ tzp->slope = ti->slope;
+ tzp->offset = ti->offset;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ data->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(data->regs)) {
+ err = PTR_ERR(data->regs);
+ dev_err(&pdev->dev, "Could not get registers: %d\n", err);
+ return err;
+ }
+
+ data->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(data->clk)) {
+ err = PTR_ERR(data->clk);
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Could not get clk: %d\n", err);
+ return err;
+ }
+
+ err = clk_prepare_enable(data->clk);
+ if (err)
+ return err;
+
+ rate = clk_get_rate(data->clk);
+ if ((rate < 1920000) || (rate > 5000000))
+ dev_warn(&pdev->dev,
+ "Clock %pCn running at %pCr Hz is outside of the recommended range: 1.92 to 5MHz\n",
+ data->clk, data->clk);
+
+ /*
+ * right now the FW does set up the HW-block, so we are not
+ * touching the configuration registers.
+ * But if the HW is not enabled, then set it up
+ * using "sane" values used by the firmware right now.
+ */
+ val = readl(data->regs + BCM2835_TS_TSENSCTL);
+ if (!(val & BCM2835_TS_TSENSCTL_RSTB)) {
+ /* the basic required flags */
+ val = (BCM2835_TS_TSENSCTL_CTRL_DEFAULT <<
+ BCM2835_TS_TSENSCTL_CTRL_SHIFT) |
+ BCM2835_TS_TSENSCTL_REGULEN;
+
+ /*
+ * reset delay using the current firmware value of 14
+ * - units of time are unknown.
+ */
+ val |= (14 << BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT);
+
+ /* trip_adc value from info */
+ val |= bcm2835_thermal_temp2adc(data->info->trip_temp,
+ data->info->offset,
+ data->info->slope)
+ << BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+ /* write the value back to the register as 2 steps */
+ writel(val, data->regs + BCM2835_TS_TSENSCTL);
+ val |= BCM2835_TS_TSENSCTL_RSTB;
+ writel(val, data->regs + BCM2835_TS_TSENSCTL);
+ }
+
+ /* register thermal zone with 1 trip point an 1s polling */
+ tz = thermal_zone_device_register("bcm2835_thermal",
+ 1, 0, data,
+ &bcm2835_thermal_ops,
+ tzp,
+ 0, 1000);
+ if (IS_ERR(tz)) {
+ clk_disable_unprepare(data->clk);
+ err = PTR_ERR(tz);
+ dev_err(&pdev->dev,
+ "Failed to register the thermal device: %d\n",
+ err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, tz);
+
+ bcm2835_thermal_debugfs(pdev);
+
+ return 0;
+}
+
+static int bcm2835_thermal_remove(struct platform_device *pdev)
+{
+ struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+ struct bcm2835_thermal_data *data = tz->devdata;
+
+ debugfs_remove_recursive(data->debugfsdir);
+ thermal_zone_device_unregister(tz);
+ clk_disable_unprepare(data->clk);
+
+ return 0;
+}
+
+/*
+ * Note: as per Raspberry Foundation FAQ
+ * (https://www.raspberrypi.org/help/faqs/#performanceOperatingTemperature)
+ * the recommended temperature range for the SOC -40C to +85C
+ * so the trip limit is set to 80C.
+ * this applies to all the BCM283X SOC
+ */
+
+static const struct of_device_id bcm2835_thermal_of_match_table[] = {
+ {
+ .compatible = "brcm,bcm2835-thermal",
+ .data = &(struct bcm2835_thermal_info) {
+ .offset = 407000,
+ .slope = -538,
+ .trip_temp = 80000
+ }
+ },
+ {
+ .compatible = "brcm,bcm2836-thermal",
+ .data = &(struct bcm2835_thermal_info) {
+ .offset = 407000,
+ .slope = -538,
+ .trip_temp = 80000
+ }
+ },
+ {
+ .compatible = "brcm,bcm2837-thermal",
+ .data = &(struct bcm2835_thermal_info) {
+ /* the bcm2837 needs adjustment of +5C */
+ .offset = 407000 + 5000,
+ .slope = -538,
+ .trip_temp = 80000
+ }
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
+
+static struct platform_driver bcm2835_thermal_driver = {
+ .probe = bcm2835_thermal_probe,
+ .remove = bcm2835_thermal_remove,
+ .driver = {
+ .name = "bcm2835_thermal",
+ .of_match_table = bcm2835_thermal_of_match_table,
+ },
+};
+module_platform_driver(bcm2835_thermal_driver);
+
+MODULE_AUTHOR("Martin Sperl");
+MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
+MODULE_LICENSE("GPL");
--
2.1.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-07 16:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-01-07 16:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Martin Sperl <kernel@martin.sperl.org>
Add basic thermal driver for bcm2835 SOC.
This driver currently relies on the firmware setting up the
tsense HW block and does not set it up itself.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Acked-by: Eric Anholt <eric@anholt.net>
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/thermal/Kconfig | 8 +
drivers/thermal/Makefile | 1 +
drivers/thermal/bcm2835_thermal.c | 354 ++++++++++++++++++++++++++++++++++++++
3 files changed, 363 insertions(+)
create mode 100644 drivers/thermal/bcm2835_thermal.c
ChangeLog:
V1 -> V2: added specific settings depending on compatiblity
added trip point based on register
setting up ctrl-register if HW is not enabled by firmware
as per recommendation of Eric (untested)
check that clock frequency is in range
(1.9 - 5MHz - as per comment in clk-bcm2835.c)
V2 -> V4: moved back to thermal (not using bcm sub-directory)
set polling interval to 1second (was 0ms, so interrupt driven)
V5 -> V6: added correct depends in KConfig
removed defined default for RESET_DELAY
removed obvious comments
clarify HW setup comments if not set up by FW already
move clk_prepare_enable to an earlier stage and add error handling
clarify warning when TS-clock runs out of recommended range
clk_disable_unprepare added in bcm2835_thermal_remove
added comment on recommended temperature ranges for SOC
V6 -> V7: removed depends on ARCH_BCM2836 || ARCH_BCM2837 in Kconfig
V7 -> V8: rebased
V8 -> V9: moved to use the thermal framework offset and slope in
thermal_zone_parameters as per request
A few additional notes on this latest patch:
* all the other portions of the patch (dt, bindings, defconf) have already
been merged into 4.10.0-rc2 - this only leaves the driver itself which
is now just a single patch to get applied.
* the driver has been moved to use thermal_zone_parameters->offset and slope
making use of the thermal_zone_get_offset and thermal_zone_get_slope
methods where relevant
Note that we cannot use those during initial setup of the HW - this code
section is typically not used anyway, as the FW sets up the device already.
This so modified patch actually increases code size by 14 lines:
drivers/thermal/bcm2835_thermal.c | 42 ++++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 14 deletions(-)
* /sys/class/thermal/*/ contains now slope and offset files
and those expose the correct values
* for some reason on module load the kernel emits now the message:
(NULL device *): hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
But this seems framework specific and needs to get addressed there.
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index c2c056c..18f2de6 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -436,4 +436,12 @@ depends on (ARCH_QCOM && OF) || COMPILE_TEST
source "drivers/thermal/qcom/Kconfig"
endmenu
+config BCM2835_THERMAL
+ tristate "Thermal sensors on bcm2835 SoC"
+ depends on ARCH_BCM2835 || COMPILE_TEST
+ depends on HAS_IOMEM
+ depends on OF
+ help
+ Support for thermal sensors on Broadcom bcm2835 SoCs.
+
endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 6a3d7b5..677c6d9 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -56,3 +56,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_BCM2835_THERMAL) += bcm2835_thermal.o
diff --git a/drivers/thermal/bcm2835_thermal.c b/drivers/thermal/bcm2835_thermal.c
new file mode 100644
index 0000000..5e2fea9
--- /dev/null
+++ b/drivers/thermal/bcm2835_thermal.c
@@ -0,0 +1,354 @@
+/*
+ * Driver for Broadcom BCM2835 soc temperature sensor
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define BCM2835_TS_TSENSCTL 0x00
+#define BCM2835_TS_TSENSSTAT 0x04
+
+#define BCM2835_TS_TSENSCTL_PRWDW BIT(0)
+#define BCM2835_TS_TSENSCTL_RSTB BIT(1)
+#define BCM2835_TS_TSENSCTL_CTRL_BITS 3
+#define BCM2835_TS_TSENSCTL_CTRL_SHIFT 2
+#define BCM2835_TS_TSENSCTL_CTRL_MASK \
+ GENMASK(BCM2835_TS_TSENSCTL_CTRL_BITS + \
+ BCM2835_TS_TSENSCTL_CTRL_SHIFT - 1, \
+ BCM2835_TS_TSENSCTL_CTRL_SHIFT)
+#define BCM2835_TS_TSENSCTL_CTRL_DEFAULT 1
+#define BCM2835_TS_TSENSCTL_EN_INT BIT(5)
+#define BCM2835_TS_TSENSCTL_DIRECT BIT(6)
+#define BCM2835_TS_TSENSCTL_CLR_INT BIT(7)
+#define BCM2835_TS_TSENSCTL_THOLD_SHIFT 8
+#define BCM2835_TS_TSENSCTL_THOLD_BITS 10
+#define BCM2835_TS_TSENSCTL_THOLD_MASK \
+ GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
+ BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
+ BCM2835_TS_TSENSCTL_THOLD_SHIFT)
+#define BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT 18
+#define BCM2835_TS_TSENSCTL_RSTDELAY_BITS 8
+#define BCM2835_TS_TSENSCTL_REGULEN BIT(26)
+
+#define BCM2835_TS_TSENSSTAT_DATA_BITS 10
+#define BCM2835_TS_TSENSSTAT_DATA_SHIFT 0
+#define BCM2835_TS_TSENSSTAT_DATA_MASK \
+ GENMASK(BCM2835_TS_TSENSSTAT_DATA_BITS + \
+ BCM2835_TS_TSENSSTAT_DATA_SHIFT - 1, \
+ BCM2835_TS_TSENSSTAT_DATA_SHIFT)
+#define BCM2835_TS_TSENSSTAT_VALID BIT(10)
+#define BCM2835_TS_TSENSSTAT_INTERRUPT BIT(11)
+
+struct bcm2835_thermal_info {
+ int offset;
+ int slope;
+ int trip_temp;
+};
+
+struct bcm2835_thermal_data {
+ const struct bcm2835_thermal_info *info;
+ void __iomem *regs;
+ struct clk *clk;
+ struct dentry *debugfsdir;
+};
+
+static int bcm2835_thermal_adc2temp(u32 adc, int offset, int slope)
+{
+ return offset + slope * adc;
+}
+
+static int bcm2835_thermal_temp2adc(int temp, int offset, int slope)
+{
+ temp -= offset;
+ temp /= slope;
+
+ if (temp < 0)
+ temp = 0;
+ if (temp >= BIT(BCM2835_TS_TSENSSTAT_DATA_BITS))
+ temp = BIT(BCM2835_TS_TSENSSTAT_DATA_BITS) - 1;
+
+ return temp;
+}
+
+static int bcm2835_thermal_get_trip_type(
+ struct thermal_zone_device *tz, int trip,
+ enum thermal_trip_type *type)
+{
+ *type = THERMAL_TRIP_CRITICAL;
+ return 0;
+}
+
+static int bcm2835_thermal_get_trip_temp(
+ struct thermal_zone_device *tz, int trip, int *temp)
+{
+ struct bcm2835_thermal_data *data = tz->devdata;
+ u32 val = readl(data->regs + BCM2835_TS_TSENSCTL);
+
+ /* get the THOLD bits */
+ val &= BCM2835_TS_TSENSCTL_THOLD_MASK;
+ val >>= BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+ /* if it is zero then use the info value */
+ if (val)
+ *temp = bcm2835_thermal_adc2temp(
+ val,
+ thermal_zone_get_offset(tz),
+ thermal_zone_get_slope(tz));
+ else
+ *temp = data->info->trip_temp;
+
+ return 0;
+}
+
+static int bcm2835_thermal_get_temp(struct thermal_zone_device *tz,
+ int *temp)
+{
+ struct bcm2835_thermal_data *data = tz->devdata;
+ u32 val = readl(data->regs + BCM2835_TS_TSENSSTAT);
+
+ if (!(val & BCM2835_TS_TSENSSTAT_VALID))
+ return -EIO;
+
+ val &= BCM2835_TS_TSENSSTAT_DATA_MASK;
+
+ *temp = bcm2835_thermal_adc2temp(
+ val,
+ thermal_zone_get_offset(tz),
+ thermal_zone_get_slope(tz));
+
+ return 0;
+}
+
+static const struct debugfs_reg32 bcm2835_thermal_regs[] = {
+ {
+ .name = "ctl",
+ .offset = 0
+ },
+ {
+ .name = "stat",
+ .offset = 4
+ }
+};
+
+static void bcm2835_thermal_debugfs(struct platform_device *pdev)
+{
+ struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+ struct bcm2835_thermal_data *data = tz->devdata;
+ struct debugfs_regset32 *regset;
+
+ data->debugfsdir = debugfs_create_dir("bcm2835_thermal", NULL);
+ if (!data->debugfsdir)
+ return;
+
+ regset = devm_kzalloc(&pdev->dev, sizeof(*regset), GFP_KERNEL);
+ if (!regset)
+ return;
+
+ regset->regs = bcm2835_thermal_regs;
+ regset->nregs = ARRAY_SIZE(bcm2835_thermal_regs);
+ regset->base = data->regs;
+
+ debugfs_create_regset32("regset", S_IRUGO,
+ data->debugfsdir, regset);
+}
+
+static struct thermal_zone_device_ops bcm2835_thermal_ops = {
+ .get_temp = bcm2835_thermal_get_temp,
+ .get_trip_temp = bcm2835_thermal_get_trip_temp,
+ .get_trip_type = bcm2835_thermal_get_trip_type,
+};
+
+static const struct of_device_id bcm2835_thermal_of_match_table[];
+static int bcm2835_thermal_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+ struct thermal_zone_device *tz;
+ struct thermal_zone_params *tzp;
+ const struct bcm2835_thermal_info *ti;
+ struct bcm2835_thermal_data *data;
+ struct resource *res;
+ int err;
+ u32 val;
+ unsigned long rate;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ tzp = devm_kzalloc(&pdev->dev, sizeof(*tzp), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ match = of_match_device(bcm2835_thermal_of_match_table,
+ &pdev->dev);
+ if (!match)
+ return -EINVAL;
+ ti = match->data;
+ data->info = ti;
+ tzp->slope = ti->slope;
+ tzp->offset = ti->offset;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ data->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(data->regs)) {
+ err = PTR_ERR(data->regs);
+ dev_err(&pdev->dev, "Could not get registers: %d\n", err);
+ return err;
+ }
+
+ data->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(data->clk)) {
+ err = PTR_ERR(data->clk);
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Could not get clk: %d\n", err);
+ return err;
+ }
+
+ err = clk_prepare_enable(data->clk);
+ if (err)
+ return err;
+
+ rate = clk_get_rate(data->clk);
+ if ((rate < 1920000) || (rate > 5000000))
+ dev_warn(&pdev->dev,
+ "Clock %pCn running at %pCr Hz is outside of the recommended range: 1.92 to 5MHz\n",
+ data->clk, data->clk);
+
+ /*
+ * right now the FW does set up the HW-block, so we are not
+ * touching the configuration registers.
+ * But if the HW is not enabled, then set it up
+ * using "sane" values used by the firmware right now.
+ */
+ val = readl(data->regs + BCM2835_TS_TSENSCTL);
+ if (!(val & BCM2835_TS_TSENSCTL_RSTB)) {
+ /* the basic required flags */
+ val = (BCM2835_TS_TSENSCTL_CTRL_DEFAULT <<
+ BCM2835_TS_TSENSCTL_CTRL_SHIFT) |
+ BCM2835_TS_TSENSCTL_REGULEN;
+
+ /*
+ * reset delay using the current firmware value of 14
+ * - units of time are unknown.
+ */
+ val |= (14 << BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT);
+
+ /* trip_adc value from info */
+ val |= bcm2835_thermal_temp2adc(data->info->trip_temp,
+ data->info->offset,
+ data->info->slope)
+ << BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+ /* write the value back to the register as 2 steps */
+ writel(val, data->regs + BCM2835_TS_TSENSCTL);
+ val |= BCM2835_TS_TSENSCTL_RSTB;
+ writel(val, data->regs + BCM2835_TS_TSENSCTL);
+ }
+
+ /* register thermal zone with 1 trip point an 1s polling */
+ tz = thermal_zone_device_register("bcm2835_thermal",
+ 1, 0, data,
+ &bcm2835_thermal_ops,
+ tzp,
+ 0, 1000);
+ if (IS_ERR(tz)) {
+ clk_disable_unprepare(data->clk);
+ err = PTR_ERR(tz);
+ dev_err(&pdev->dev,
+ "Failed to register the thermal device: %d\n",
+ err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, tz);
+
+ bcm2835_thermal_debugfs(pdev);
+
+ return 0;
+}
+
+static int bcm2835_thermal_remove(struct platform_device *pdev)
+{
+ struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+ struct bcm2835_thermal_data *data = tz->devdata;
+
+ debugfs_remove_recursive(data->debugfsdir);
+ thermal_zone_device_unregister(tz);
+ clk_disable_unprepare(data->clk);
+
+ return 0;
+}
+
+/*
+ * Note: as per Raspberry Foundation FAQ
+ * (https://www.raspberrypi.org/help/faqs/#performanceOperatingTemperature)
+ * the recommended temperature range for the SOC -40C to +85C
+ * so the trip limit is set to 80C.
+ * this applies to all the BCM283X SOC
+ */
+
+static const struct of_device_id bcm2835_thermal_of_match_table[] = {
+ {
+ .compatible = "brcm,bcm2835-thermal",
+ .data = &(struct bcm2835_thermal_info) {
+ .offset = 407000,
+ .slope = -538,
+ .trip_temp = 80000
+ }
+ },
+ {
+ .compatible = "brcm,bcm2836-thermal",
+ .data = &(struct bcm2835_thermal_info) {
+ .offset = 407000,
+ .slope = -538,
+ .trip_temp = 80000
+ }
+ },
+ {
+ .compatible = "brcm,bcm2837-thermal",
+ .data = &(struct bcm2835_thermal_info) {
+ /* the bcm2837 needs adjustment of +5C */
+ .offset = 407000 + 5000,
+ .slope = -538,
+ .trip_temp = 80000
+ }
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
+
+static struct platform_driver bcm2835_thermal_driver = {
+ .probe = bcm2835_thermal_probe,
+ .remove = bcm2835_thermal_remove,
+ .driver = {
+ .name = "bcm2835_thermal",
+ .of_match_table = bcm2835_thermal_of_match_table,
+ },
+};
+module_platform_driver(bcm2835_thermal_driver);
+
+MODULE_AUTHOR("Martin Sperl");
+MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
+MODULE_LICENSE("GPL");
--
2.1.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-07 16:55 ` kernel at martin.sperl.org
@ 2017-01-20 4:14 ` Eduardo Valentin
-1 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-20 4:14 UTC (permalink / raw)
To: kernel
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
Hello Martin,
On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
>
> Add basic thermal driver for bcm2835 SOC.
>
> This driver currently relies on the firmware setting up the
> tsense HW block and does not set it up itself.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> Acked-by: Eric Anholt <eric@anholt.net>
> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>
<cut>
> +
> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> + {
> + .compatible = "brcm,bcm2835-thermal",
> + .data = &(struct bcm2835_thermal_info) {
> + .offset = 407000,
> + .slope = -538,
> + .trip_temp = 80000
> + }
> + },
> + {
> + .compatible = "brcm,bcm2836-thermal",
> + .data = &(struct bcm2835_thermal_info) {
> + .offset = 407000,
> + .slope = -538,
> + .trip_temp = 80000
> + }
> + },
> + {
> + .compatible = "brcm,bcm2837-thermal",
> + .data = &(struct bcm2835_thermal_info) {
> + /* the bcm2837 needs adjustment of +5C */
> + .offset = 407000 + 5000,
> + .slope = -538,
> + .trip_temp = 80000
> + }
> + },
> + {},
Just for the same of clarification, is there anything preventing this
driver of using of-thermal API? the above data (slope, offset, and
trip_temps) would be in DT the place where they are supposed to be,
instead of code.
> +};
> +MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
> +
> +static struct platform_driver bcm2835_thermal_driver = {
> + .probe = bcm2835_thermal_probe,
> + .remove = bcm2835_thermal_remove,
> + .driver = {
> + .name = "bcm2835_thermal",
> + .of_match_table = bcm2835_thermal_of_match_table,
> + },
> +};
> +module_platform_driver(bcm2835_thermal_driver);
> +
> +MODULE_AUTHOR("Martin Sperl");
> +MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
> +MODULE_LICENSE("GPL");
> --
> 2.1.4
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-20 4:14 ` Eduardo Valentin
0 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-20 4:14 UTC (permalink / raw)
To: linux-arm-kernel
Hello Martin,
On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
>
> Add basic thermal driver for bcm2835 SOC.
>
> This driver currently relies on the firmware setting up the
> tsense HW block and does not set it up itself.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> Acked-by: Eric Anholt <eric@anholt.net>
> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>
<cut>
> +
> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> + {
> + .compatible = "brcm,bcm2835-thermal",
> + .data = &(struct bcm2835_thermal_info) {
> + .offset = 407000,
> + .slope = -538,
> + .trip_temp = 80000
> + }
> + },
> + {
> + .compatible = "brcm,bcm2836-thermal",
> + .data = &(struct bcm2835_thermal_info) {
> + .offset = 407000,
> + .slope = -538,
> + .trip_temp = 80000
> + }
> + },
> + {
> + .compatible = "brcm,bcm2837-thermal",
> + .data = &(struct bcm2835_thermal_info) {
> + /* the bcm2837 needs adjustment of +5C */
> + .offset = 407000 + 5000,
> + .slope = -538,
> + .trip_temp = 80000
> + }
> + },
> + {},
Just for the same of clarification, is there anything preventing this
driver of using of-thermal API? the above data (slope, offset, and
trip_temps) would be in DT the place where they are supposed to be,
instead of code.
> +};
> +MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
> +
> +static struct platform_driver bcm2835_thermal_driver = {
> + .probe = bcm2835_thermal_probe,
> + .remove = bcm2835_thermal_remove,
> + .driver = {
> + .name = "bcm2835_thermal",
> + .of_match_table = bcm2835_thermal_of_match_table,
> + },
> +};
> +module_platform_driver(bcm2835_thermal_driver);
> +
> +MODULE_AUTHOR("Martin Sperl");
> +MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
> +MODULE_LICENSE("GPL");
> --
> 2.1.4
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-20 4:14 ` Eduardo Valentin
@ 2017-01-20 4:23 ` Eduardo Valentin
-1 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-20 4:23 UTC (permalink / raw)
To: kernel
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
> Hello Martin,
>
> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
> > From: Martin Sperl <kernel@martin.sperl.org>
> >
> > Add basic thermal driver for bcm2835 SOC.
> >
> > This driver currently relies on the firmware setting up the
> > tsense HW block and does not set it up itself.
> >
> > Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> > Acked-by: Eric Anholt <eric@anholt.net>
> > Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >
>
> <cut>
Also, I am getting this warn from sparse:
drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
from constant value (3ffffffffff00 becomes ffffff00)
drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
from constant value (3ffffffffff becomes ffffffff)
Have you seen this?
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-20 4:23 ` Eduardo Valentin
0 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-20 4:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
> Hello Martin,
>
> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
> > From: Martin Sperl <kernel@martin.sperl.org>
> >
> > Add basic thermal driver for bcm2835 SOC.
> >
> > This driver currently relies on the firmware setting up the
> > tsense HW block and does not set it up itself.
> >
> > Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> > Acked-by: Eric Anholt <eric@anholt.net>
> > Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >
>
> <cut>
Also, I am getting this warn from sparse:
drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
from constant value (3ffffffffff00 becomes ffffff00)
drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
from constant value (3ffffffffff becomes ffffffff)
Have you seen this?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-20 4:14 ` Eduardo Valentin
@ 2017-01-20 7:54 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 37+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2017-01-20 7:54 UTC (permalink / raw)
To: Eduardo Valentin
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, Lee Jones, linux-rpi-kernel,
Zhang Rui, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Hello Martin,
>
> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>
>> Add basic thermal driver for bcm2835 SOC.
>>
>> This driver currently relies on the firmware setting up the
>> tsense HW block and does not set it up itself.
>>
>> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>> Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>
>
> <cut>
>
>> +
>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>> + {
>> + .compatible = "brcm,bcm2835-thermal",
>> + .data = &(struct bcm2835_thermal_info) {
>> + .offset = 407000,
>> + .slope = -538,
>> + .trip_temp = 80000
>> + }
>> + },
>> + {
>> + .compatible = "brcm,bcm2836-thermal",
>> + .data = &(struct bcm2835_thermal_info) {
>> + .offset = 407000,
>> + .slope = -538,
>> + .trip_temp = 80000
>> + }
>> + },
>> + {
>> + .compatible = "brcm,bcm2837-thermal",
>> + .data = &(struct bcm2835_thermal_info) {
>> + /* the bcm2837 needs adjustment of +5C */
>> + .offset = 407000 + 5000,
>> + .slope = -538,
>> + .trip_temp = 80000
>> + }
>> + },
>> + {},
>
> Just for the same of clarification, is there anything preventing this
> driver of using of-thermal API? the above data (slope, offset, and
> trip_temps) would be in DT the place where they are supposed to be,
> instead of code.
>
As the DT changes, that only define compatible strings, have already gone
in without any such properties set, we need to define defaults for the
slope/offset and trip_temp values.
I guess (for newer SOC) you still can use the values in the DT,
as (I guess) these are parsed and set in thermal_zone_device_register
after the defaults are set in thermal_zone_params.
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-20 7:54 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-01-20 7:54 UTC (permalink / raw)
To: linux-arm-kernel
> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
>> From: Martin Sperl <kernel@martin.sperl.org>
>>
>> Add basic thermal driver for bcm2835 SOC.
>>
>> This driver currently relies on the firmware setting up the
>> tsense HW block and does not set it up itself.
>>
>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>> Acked-by: Eric Anholt <eric@anholt.net>
>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>
>
> <cut>
>
>> +
>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>> + {
>> + .compatible = "brcm,bcm2835-thermal",
>> + .data = &(struct bcm2835_thermal_info) {
>> + .offset = 407000,
>> + .slope = -538,
>> + .trip_temp = 80000
>> + }
>> + },
>> + {
>> + .compatible = "brcm,bcm2836-thermal",
>> + .data = &(struct bcm2835_thermal_info) {
>> + .offset = 407000,
>> + .slope = -538,
>> + .trip_temp = 80000
>> + }
>> + },
>> + {
>> + .compatible = "brcm,bcm2837-thermal",
>> + .data = &(struct bcm2835_thermal_info) {
>> + /* the bcm2837 needs adjustment of +5C */
>> + .offset = 407000 + 5000,
>> + .slope = -538,
>> + .trip_temp = 80000
>> + }
>> + },
>> + {},
>
> Just for the same of clarification, is there anything preventing this
> driver of using of-thermal API? the above data (slope, offset, and
> trip_temps) would be in DT the place where they are supposed to be,
> instead of code.
>
As the DT changes, that only define compatible strings, have already gone
in without any such properties set, we need to define defaults for the
slope/offset and trip_temp values.
I guess (for newer SOC) you still can use the values in the DT,
as (I guess) these are parsed and set in thermal_zone_device_register
after the defaults are set in thermal_zone_params.
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-20 4:23 ` Eduardo Valentin
@ 2017-01-20 8:43 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 37+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2017-01-20 8:43 UTC (permalink / raw)
To: Eduardo Valentin
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Zhang Rui,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
> On 20.01.2017, at 05:23, Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
>> Hello Martin,
>>
>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>
>>> Add basic thermal driver for bcm2835 SOC.
>>>
>>> This driver currently relies on the firmware setting up the
>>> tsense HW block and does not set it up itself.
>>>
>>> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>>> Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>>
>>
>> <cut>
>
>
> Also, I am getting this warn from sparse:
> drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
> from constant value (3ffffffffff00 becomes ffffff00)
> drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
> from constant value (3ffffffffff becomes ffffffff)
>
> Have you seen this?
No, I have not checked sparse.
These values are defined via GENMASK on line 47 and 57 respectively
and should actually compute to the following values:
for line 110 (line 47 has the define):
GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
BCM2835_TS_TSENSCTL_THOLD_SHIFT)
= GENMASK(10 + 8 - 1, 8)
= GENMASK(17, 8)
= (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
= 0x3ff00
for line 134 (line 57 has the define):
GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
BCM2835_TS_TSENSCTL_THOLD_SHIFT)
= GENMASK(10 + 0 - 1, 0)
= GENMASK(9, 0)
= (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
= 0x003ff
Note that the preprocessor expansions have been verified by
looking at the preprocessed driver source
(drivers/thermal/bcm2835_thermal.i)
I wonder why sparse is computing these GENMASK values as:
0x3ffffffffff00 and 0x3ffffffffff
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-20 8:43 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-01-20 8:43 UTC (permalink / raw)
To: linux-arm-kernel
> On 20.01.2017, at 05:23, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
>> Hello Martin,
>>
>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>
>>> Add basic thermal driver for bcm2835 SOC.
>>>
>>> This driver currently relies on the firmware setting up the
>>> tsense HW block and does not set it up itself.
>>>
>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>> Acked-by: Eric Anholt <eric@anholt.net>
>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>
>>
>> <cut>
>
>
> Also, I am getting this warn from sparse:
> drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
> from constant value (3ffffffffff00 becomes ffffff00)
> drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
> from constant value (3ffffffffff becomes ffffffff)
>
> Have you seen this?
No, I have not checked sparse.
These values are defined via GENMASK on line 47 and 57 respectively
and should actually compute to the following values:
for line 110 (line 47 has the define):
GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
BCM2835_TS_TSENSCTL_THOLD_SHIFT)
= GENMASK(10 + 8 - 1, 8)
= GENMASK(17, 8)
= (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
= 0x3ff00
for line 134 (line 57 has the define):
GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
BCM2835_TS_TSENSCTL_THOLD_SHIFT)
= GENMASK(10 + 0 - 1, 0)
= GENMASK(9, 0)
= (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
= 0x003ff
Note that the preprocessor expansions have been verified by
looking at the preprocessed driver source
(drivers/thermal/bcm2835_thermal.i)
I wonder why sparse is computing these GENMASK values as:
0x3ffffffffff00 and 0x3ffffffffff
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-20 8:43 ` kernel at martin.sperl.org
@ 2017-01-24 9:26 ` Eduardo Valentin
-1 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-24 9:26 UTC (permalink / raw)
To: kernel
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
Hello Martin,
On Fri, Jan 20, 2017 at 09:43:02AM +0100, kernel@martin.sperl.org wrote:
>
> > On 20.01.2017, at 05:23, Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
> >> Hello Martin,
> >>
> >> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
> >>> From: Martin Sperl <kernel@martin.sperl.org>
> >>>
> >>> Add basic thermal driver for bcm2835 SOC.
> >>>
> >>> This driver currently relies on the firmware setting up the
> >>> tsense HW block and does not set it up itself.
> >>>
> >>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> >>> Acked-by: Eric Anholt <eric@anholt.net>
> >>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >>>
> >>
> >> <cut>
> >
> >
> > Also, I am getting this warn from sparse:
> > drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
> > from constant value (3ffffffffff00 becomes ffffff00)
> > drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
> > from constant value (3ffffffffff becomes ffffffff)
> >
> > Have you seen this?
>
> No, I have not checked sparse.
>
> These values are defined via GENMASK on line 47 and 57 respectively
> and should actually compute to the following values:
> for line 110 (line 47 has the define):
> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
> = GENMASK(10 + 8 - 1, 8)
> = GENMASK(17, 8)
> = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
> = 0x3ff00
> for line 134 (line 57 has the define):
> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
> = GENMASK(10 + 0 - 1, 0)
> = GENMASK(9, 0)
> = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
> = 0x003ff
>
> Note that the preprocessor expansions have been verified by
> looking at the preprocessed driver source
> (drivers/thermal/bcm2835_thermal.i)
OK then.
>
> I wonder why sparse is computing these GENMASK values as:
> 0x3ffffffffff00 and 0x3ffffffffff
In the case you can confirm that the values are correct, I believe this
could be a false positive report on sparse, in this case.
>
> Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-24 9:26 ` Eduardo Valentin
0 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-24 9:26 UTC (permalink / raw)
To: linux-arm-kernel
Hello Martin,
On Fri, Jan 20, 2017 at 09:43:02AM +0100, kernel at martin.sperl.org wrote:
>
> > On 20.01.2017, at 05:23, Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
> >> Hello Martin,
> >>
> >> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
> >>> From: Martin Sperl <kernel@martin.sperl.org>
> >>>
> >>> Add basic thermal driver for bcm2835 SOC.
> >>>
> >>> This driver currently relies on the firmware setting up the
> >>> tsense HW block and does not set it up itself.
> >>>
> >>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> >>> Acked-by: Eric Anholt <eric@anholt.net>
> >>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >>>
> >>
> >> <cut>
> >
> >
> > Also, I am getting this warn from sparse:
> > drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
> > from constant value (3ffffffffff00 becomes ffffff00)
> > drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
> > from constant value (3ffffffffff becomes ffffffff)
> >
> > Have you seen this?
>
> No, I have not checked sparse.
>
> These values are defined via GENMASK on line 47 and 57 respectively
> and should actually compute to the following values:
> for line 110 (line 47 has the define):
> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
> = GENMASK(10 + 8 - 1, 8)
> = GENMASK(17, 8)
> = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
> = 0x3ff00
> for line 134 (line 57 has the define):
> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
> = GENMASK(10 + 0 - 1, 0)
> = GENMASK(9, 0)
> = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
> = 0x003ff
>
> Note that the preprocessor expansions have been verified by
> looking at the preprocessed driver source
> (drivers/thermal/bcm2835_thermal.i)
OK then.
>
> I wonder why sparse is computing these GENMASK values as:
> 0x3ffffffffff00 and 0x3ffffffffff
In the case you can confirm that the values are correct, I believe this
could be a false positive report on sparse, in this case.
>
> Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-20 7:54 ` kernel at martin.sperl.org
@ 2017-01-24 9:31 ` Eduardo Valentin
-1 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-24 9:31 UTC (permalink / raw)
To: kernel-TqfNSX0MhmxHKSADF0wUEw
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, Lee Jones, linux-rpi-kernel,
Zhang Rui, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hello Martin,
On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>
> > On 20.01.2017, at 05:14, Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > Hello Martin,
> >
> > On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> >> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> >>
> >> Add basic thermal driver for bcm2835 SOC.
> >>
> >> This driver currently relies on the firmware setting up the
> >> tsense HW block and does not set it up itself.
> >>
> >> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> >> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> >> Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >>
> >
> > <cut>
> >
> >> +
> >> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> >> + {
> >> + .compatible = "brcm,bcm2835-thermal",
> >> + .data = &(struct bcm2835_thermal_info) {
> >> + .offset = 407000,
> >> + .slope = -538,
> >> + .trip_temp = 80000
> >> + }
> >> + },
> >> + {
> >> + .compatible = "brcm,bcm2836-thermal",
> >> + .data = &(struct bcm2835_thermal_info) {
> >> + .offset = 407000,
> >> + .slope = -538,
> >> + .trip_temp = 80000
> >> + }
> >> + },
> >> + {
> >> + .compatible = "brcm,bcm2837-thermal",
> >> + .data = &(struct bcm2835_thermal_info) {
> >> + /* the bcm2837 needs adjustment of +5C */
> >> + .offset = 407000 + 5000,
> >> + .slope = -538,
> >> + .trip_temp = 80000
> >> + }
> >> + },
> >> + {},
> >
> > Just for the same of clarification, is there anything preventing this
> > driver of using of-thermal API? the above data (slope, offset, and
> > trip_temps) would be in DT the place where they are supposed to be,
> > instead of code.
> >
>
> As the DT changes, that only define compatible strings, have already gone
> in without any such properties set, we need to define defaults for the
> slope/offset and trip_temp values.
>
These properties won't go into the same node you are referring to. They
go into the thermal-zone node you would create, which would then refer
to the node you referred (already merged). Therefore, I do not see
anything blocking a proper of-thermal usage to cover for the above data.
> I guess (for newer SOC) you still can use the values in the DT,
> as (I guess) these are parsed and set in thermal_zone_device_register
> after the defaults are set in thermal_zone_params.
Not sure what you meant here, but these values, when correctly used in
DT, they would come as part of the thermal_zone_params and in the
thermal trips of the thermal zones, as the of-thermal code with already
deal with those for you.
Please have a look at:
a. Documentation/devicetree/bindings/thermal/thermal.txt
b. drivers/thermal/of-thermal.c
And let me know if you see anything that would prevent this driver of
using the correct API to describe hardware data with DT.
BR,
>
> Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-24 9:31 ` Eduardo Valentin
0 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-01-24 9:31 UTC (permalink / raw)
To: linux-arm-kernel
Hello Martin,
On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel at martin.sperl.org wrote:
>
> > On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > Hello Martin,
> >
> > On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
> >> From: Martin Sperl <kernel@martin.sperl.org>
> >>
> >> Add basic thermal driver for bcm2835 SOC.
> >>
> >> This driver currently relies on the firmware setting up the
> >> tsense HW block and does not set it up itself.
> >>
> >> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> >> Acked-by: Eric Anholt <eric@anholt.net>
> >> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >>
> >
> > <cut>
> >
> >> +
> >> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> >> + {
> >> + .compatible = "brcm,bcm2835-thermal",
> >> + .data = &(struct bcm2835_thermal_info) {
> >> + .offset = 407000,
> >> + .slope = -538,
> >> + .trip_temp = 80000
> >> + }
> >> + },
> >> + {
> >> + .compatible = "brcm,bcm2836-thermal",
> >> + .data = &(struct bcm2835_thermal_info) {
> >> + .offset = 407000,
> >> + .slope = -538,
> >> + .trip_temp = 80000
> >> + }
> >> + },
> >> + {
> >> + .compatible = "brcm,bcm2837-thermal",
> >> + .data = &(struct bcm2835_thermal_info) {
> >> + /* the bcm2837 needs adjustment of +5C */
> >> + .offset = 407000 + 5000,
> >> + .slope = -538,
> >> + .trip_temp = 80000
> >> + }
> >> + },
> >> + {},
> >
> > Just for the same of clarification, is there anything preventing this
> > driver of using of-thermal API? the above data (slope, offset, and
> > trip_temps) would be in DT the place where they are supposed to be,
> > instead of code.
> >
>
> As the DT changes, that only define compatible strings, have already gone
> in without any such properties set, we need to define defaults for the
> slope/offset and trip_temp values.
>
These properties won't go into the same node you are referring to. They
go into the thermal-zone node you would create, which would then refer
to the node you referred (already merged). Therefore, I do not see
anything blocking a proper of-thermal usage to cover for the above data.
> I guess (for newer SOC) you still can use the values in the DT,
> as (I guess) these are parsed and set in thermal_zone_device_register
> after the defaults are set in thermal_zone_params.
Not sure what you meant here, but these values, when correctly used in
DT, they would come as part of the thermal_zone_params and in the
thermal trips of the thermal zones, as the of-thermal code with already
deal with those for you.
Please have a look at:
a. Documentation/devicetree/bindings/thermal/thermal.txt
b. drivers/thermal/of-thermal.c
And let me know if you see anything that would prevent this driver of
using the correct API to describe hardware data with DT.
BR,
>
> Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-24 9:26 ` Eduardo Valentin
@ 2017-01-24 9:37 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 37+ messages in thread
From: kernel @ 2017-01-24 9:37 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
> On 24.01.2017, at 10:26, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Fri, Jan 20, 2017 at 09:43:02AM +0100, kernel@martin.sperl.org wrote:
>>
>>> On 20.01.2017, at 05:23, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>
>>> On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
>>>> Hello Martin,
>>>>
>>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
>>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>>
>>>>> Add basic thermal driver for bcm2835 SOC.
>>>>>
>>>>> This driver currently relies on the firmware setting up the
>>>>> tsense HW block and does not set it up itself.
>>>>>
>>>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>>
>>>>
>>>> <cut>
>>>
>>>
>>> Also, I am getting this warn from sparse:
>>> drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
>>> from constant value (3ffffffffff00 becomes ffffff00)
>>> drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
>>> from constant value (3ffffffffff becomes ffffffff)
>>>
>>> Have you seen this?
>>
>> No, I have not checked sparse.
>>
>> These values are defined via GENMASK on line 47 and 57 respectively
>> and should actually compute to the following values:
>> for line 110 (line 47 has the define):
>> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>> = GENMASK(10 + 8 - 1, 8)
>> = GENMASK(17, 8)
>> = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
>> = 0x3ff00
>> for line 134 (line 57 has the define):
>> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>> = GENMASK(10 + 0 - 1, 0)
>> = GENMASK(9, 0)
>> = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
>> = 0x003ff
>>
>> Note that the preprocessor expansions have been verified by
>> looking at the preprocessed driver source
>> (drivers/thermal/bcm2835_thermal.i)
>
> OK then.
>
>>
>> I wonder why sparse is computing these GENMASK values as:
>> 0x3ffffffffff00 and 0x3ffffffffff
>
> In the case you can confirm that the values are correct, I believe this
> could be a false positive report on sparse, in this case.
The correct values are the ones in my email: 0x3ff00 and 0x003ff
The ones reported by sparse (0x3ffffffffff00 and 0x3ffffffffff) are
not calculated correctly - to me it looks as if it is possibly some
sort of 64 bit issue of sparse in conjunction with the generic macro
GENMASK.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-24 9:37 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-01-24 9:37 UTC (permalink / raw)
To: linux-arm-kernel
> On 24.01.2017, at 10:26, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Fri, Jan 20, 2017 at 09:43:02AM +0100, kernel at martin.sperl.org wrote:
>>
>>> On 20.01.2017, at 05:23, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>
>>> On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
>>>> Hello Martin,
>>>>
>>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
>>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>>
>>>>> Add basic thermal driver for bcm2835 SOC.
>>>>>
>>>>> This driver currently relies on the firmware setting up the
>>>>> tsense HW block and does not set it up itself.
>>>>>
>>>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>>
>>>>
>>>> <cut>
>>>
>>>
>>> Also, I am getting this warn from sparse:
>>> drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
>>> from constant value (3ffffffffff00 becomes ffffff00)
>>> drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
>>> from constant value (3ffffffffff becomes ffffffff)
>>>
>>> Have you seen this?
>>
>> No, I have not checked sparse.
>>
>> These values are defined via GENMASK on line 47 and 57 respectively
>> and should actually compute to the following values:
>> for line 110 (line 47 has the define):
>> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>> = GENMASK(10 + 8 - 1, 8)
>> = GENMASK(17, 8)
>> = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
>> = 0x3ff00
>> for line 134 (line 57 has the define):
>> GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS + \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>> BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>> = GENMASK(10 + 0 - 1, 0)
>> = GENMASK(9, 0)
>> = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
>> = 0x003ff
>>
>> Note that the preprocessor expansions have been verified by
>> looking at the preprocessed driver source
>> (drivers/thermal/bcm2835_thermal.i)
>
> OK then.
>
>>
>> I wonder why sparse is computing these GENMASK values as:
>> 0x3ffffffffff00 and 0x3ffffffffff
>
> In the case you can confirm that the values are correct, I believe this
> could be a false positive report on sparse, in this case.
The correct values are the ones in my email: 0x3ff00 and 0x003ff
The ones reported by sparse (0x3ffffffffff00 and 0x3ffffffffff) are
not calculated correctly - to me it looks as if it is possibly some
sort of 64 bit issue of sparse in conjunction with the generic macro
GENMASK.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-24 9:31 ` Eduardo Valentin
@ 2017-01-24 9:52 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 37+ messages in thread
From: kernel @ 2017-01-24 9:52 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
> On 24.01.2017, at 10:31, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel@martin.sperl.org wrote:
>>
>>> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>
>>> Hello Martin,
>>>
>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>
>>>> Add basic thermal driver for bcm2835 SOC.
>>>>
>>>> This driver currently relies on the firmware setting up the
>>>> tsense HW block and does not set it up itself.
>>>>
>>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>
>>>
>>> <cut>
>>>
>>>> +
>>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>>>> + {
>>>> + .compatible = "brcm,bcm2835-thermal",
>>>> + .data = &(struct bcm2835_thermal_info) {
>>>> + .offset = 407000,
>>>> + .slope = -538,
>>>> + .trip_temp = 80000
>>>> + }
>>>> + },
>>>> + {
>>>> + .compatible = "brcm,bcm2836-thermal",
>>>> + .data = &(struct bcm2835_thermal_info) {
>>>> + .offset = 407000,
>>>> + .slope = -538,
>>>> + .trip_temp = 80000
>>>> + }
>>>> + },
>>>> + {
>>>> + .compatible = "brcm,bcm2837-thermal",
>>>> + .data = &(struct bcm2835_thermal_info) {
>>>> + /* the bcm2837 needs adjustment of +5C */
>>>> + .offset = 407000 + 5000,
>>>> + .slope = -538,
>>>> + .trip_temp = 80000
>>>> + }
>>>> + },
>>>> + {},
>>>
>>> Just for the same of clarification, is there anything preventing this
>>> driver of using of-thermal API? the above data (slope, offset, and
>>> trip_temps) would be in DT the place where they are supposed to be,
>>> instead of code.
>>>
>>
>> As the DT changes, that only define compatible strings, have already gone
>> in without any such properties set, we need to define defaults for the
>> slope/offset and trip_temp values.
>>
>
> These properties won't go into the same node you are referring to. They
> go into the thermal-zone node you would create, which would then refer
> to the node you referred (already merged). Therefore, I do not see
> anything blocking a proper of-thermal usage to cover for the above data.
>
>> I guess (for newer SOC) you still can use the values in the DT,
>> as (I guess) these are parsed and set in thermal_zone_device_register
>> after the defaults are set in thermal_zone_params.
>
> Not sure what you meant here, but these values, when correctly used in
> DT, they would come as part of the thermal_zone_params and in the
> thermal trips of the thermal zones, as the of-thermal code with already
> deal with those for you.
>
> Please have a look at:
> a. Documentation/devicetree/bindings/thermal/thermal.txt
> b. drivers/thermal/of-thermal.c
>
> And let me know if you see anything that would prevent this driver of
> using the correct API to describe hardware data with DT.
I guess you miss my point:
The argument is that we have DT in the 4.10.0-rc2 kernel right now that
look like this:
thermal@7e212000 {
compatible = "brcm,bcm2835-thermal";
clocks = <0x6 0x1b>;
status = "okay";
reg = <0x7e212000 0x8>;
}
so we still need to be compatible with those without any extra defines.
Hence we need to define those slopes and offsets in the driver itself
to stay compatible with those older device-trees.
As for if it works with the framework - I have to admit I do not
have the slightest clue - it looks way to complicated for the soc right
now.
As a note: afaiu the trip_temp register is the temperature at which the
soc will reboot on its own - similar to a watchdog, but for temperatures.
(main reason for the assumption is because there is no interrupt that
can get assigned a handler to catch this situation).
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-01-24 9:52 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-01-24 9:52 UTC (permalink / raw)
To: linux-arm-kernel
> On 24.01.2017, at 10:31, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel at martin.sperl.org wrote:
>>
>>> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>
>>> Hello Martin,
>>>
>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>
>>>> Add basic thermal driver for bcm2835 SOC.
>>>>
>>>> This driver currently relies on the firmware setting up the
>>>> tsense HW block and does not set it up itself.
>>>>
>>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>
>>>
>>> <cut>
>>>
>>>> +
>>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>>>> + {
>>>> + .compatible = "brcm,bcm2835-thermal",
>>>> + .data = &(struct bcm2835_thermal_info) {
>>>> + .offset = 407000,
>>>> + .slope = -538,
>>>> + .trip_temp = 80000
>>>> + }
>>>> + },
>>>> + {
>>>> + .compatible = "brcm,bcm2836-thermal",
>>>> + .data = &(struct bcm2835_thermal_info) {
>>>> + .offset = 407000,
>>>> + .slope = -538,
>>>> + .trip_temp = 80000
>>>> + }
>>>> + },
>>>> + {
>>>> + .compatible = "brcm,bcm2837-thermal",
>>>> + .data = &(struct bcm2835_thermal_info) {
>>>> + /* the bcm2837 needs adjustment of +5C */
>>>> + .offset = 407000 + 5000,
>>>> + .slope = -538,
>>>> + .trip_temp = 80000
>>>> + }
>>>> + },
>>>> + {},
>>>
>>> Just for the same of clarification, is there anything preventing this
>>> driver of using of-thermal API? the above data (slope, offset, and
>>> trip_temps) would be in DT the place where they are supposed to be,
>>> instead of code.
>>>
>>
>> As the DT changes, that only define compatible strings, have already gone
>> in without any such properties set, we need to define defaults for the
>> slope/offset and trip_temp values.
>>
>
> These properties won't go into the same node you are referring to. They
> go into the thermal-zone node you would create, which would then refer
> to the node you referred (already merged). Therefore, I do not see
> anything blocking a proper of-thermal usage to cover for the above data.
>
>> I guess (for newer SOC) you still can use the values in the DT,
>> as (I guess) these are parsed and set in thermal_zone_device_register
>> after the defaults are set in thermal_zone_params.
>
> Not sure what you meant here, but these values, when correctly used in
> DT, they would come as part of the thermal_zone_params and in the
> thermal trips of the thermal zones, as the of-thermal code with already
> deal with those for you.
>
> Please have a look at:
> a. Documentation/devicetree/bindings/thermal/thermal.txt
> b. drivers/thermal/of-thermal.c
>
> And let me know if you see anything that would prevent this driver of
> using the correct API to describe hardware data with DT.
I guess you miss my point:
The argument is that we have DT in the 4.10.0-rc2 kernel right now that
look like this:
thermal at 7e212000 {
compatible = "brcm,bcm2835-thermal";
clocks = <0x6 0x1b>;
status = "okay";
reg = <0x7e212000 0x8>;
}
so we still need to be compatible with those without any extra defines.
Hence we need to define those slopes and offsets in the driver itself
to stay compatible with those older device-trees.
As for if it works with the framework - I have to admit I do not
have the slightest clue - it looks way to complicated for the soc right
now.
As a note: afaiu the trip_temp register is the temperature at which the
soc will reboot on its own - similar to a watchdog, but for temperatures.
(main reason for the assumption is because there is no interrupt that
can get assigned a handler to catch this situation).
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-01-24 9:52 ` kernel at martin.sperl.org
@ 2017-02-02 4:29 ` Eduardo Valentin
-1 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-02-02 4:29 UTC (permalink / raw)
To: kernel
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
Hello Martin,
On Tue, Jan 24, 2017 at 10:52:43AM +0100, kernel@martin.sperl.org wrote:
>
> > On 24.01.2017, at 10:31, Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > Hello Martin,
> >
> > On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel@martin.sperl.org wrote:
> >>
> >>> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
> >>>
> >>> Hello Martin,
> >>>
> >>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
> >>>> From: Martin Sperl <kernel@martin.sperl.org>
> >>>>
> >>>> Add basic thermal driver for bcm2835 SOC.
> >>>>
> >>>> This driver currently relies on the firmware setting up the
> >>>> tsense HW block and does not set it up itself.
> >>>>
> >>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> >>>> Acked-by: Eric Anholt <eric@anholt.net>
> >>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >>>>
> >>>
> >>> <cut>
> >>>
> >>>> +
> >>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> >>>> + {
> >>>> + .compatible = "brcm,bcm2835-thermal",
> >>>> + .data = &(struct bcm2835_thermal_info) {
> >>>> + .offset = 407000,
> >>>> + .slope = -538,
> >>>> + .trip_temp = 80000
> >>>> + }
> >>>> + },
> >>>> + {
> >>>> + .compatible = "brcm,bcm2836-thermal",
> >>>> + .data = &(struct bcm2835_thermal_info) {
> >>>> + .offset = 407000,
> >>>> + .slope = -538,
> >>>> + .trip_temp = 80000
> >>>> + }
> >>>> + },
> >>>> + {
> >>>> + .compatible = "brcm,bcm2837-thermal",
> >>>> + .data = &(struct bcm2835_thermal_info) {
> >>>> + /* the bcm2837 needs adjustment of +5C */
> >>>> + .offset = 407000 + 5000,
> >>>> + .slope = -538,
> >>>> + .trip_temp = 80000
> >>>> + }
> >>>> + },
> >>>> + {},
> >>>
> >>> Just for the same of clarification, is there anything preventing this
> >>> driver of using of-thermal API? the above data (slope, offset, and
> >>> trip_temps) would be in DT the place where they are supposed to be,
> >>> instead of code.
> >>>
> >>
> >> As the DT changes, that only define compatible strings, have already gone
> >> in without any such properties set, we need to define defaults for the
> >> slope/offset and trip_temp values.
> >>
> >
> > These properties won't go into the same node you are referring to. They
> > go into the thermal-zone node you would create, which would then refer
> > to the node you referred (already merged). Therefore, I do not see
> > anything blocking a proper of-thermal usage to cover for the above data.
> >
> >> I guess (for newer SOC) you still can use the values in the DT,
> >> as (I guess) these are parsed and set in thermal_zone_device_register
> >> after the defaults are set in thermal_zone_params.
> >
> > Not sure what you meant here, but these values, when correctly used in
> > DT, they would come as part of the thermal_zone_params and in the
> > thermal trips of the thermal zones, as the of-thermal code with already
> > deal with those for you.
> >
> > Please have a look at:
> > a. Documentation/devicetree/bindings/thermal/thermal.txt
> > b. drivers/thermal/of-thermal.c
> >
> > And let me know if you see anything that would prevent this driver of
> > using the correct API to describe hardware data with DT.
>
>
> I guess you miss my point:
Maybe you missed, or did not read the doc I pointed you...
> The argument is that we have DT in the 4.10.0-rc2 kernel right now that
> look like this:
> thermal@7e212000 {
> compatible = "brcm,bcm2835-thermal";
> clocks = <0x6 0x1b>;
> status = "okay";
> reg = <0x7e212000 0x8>;
> }
> so we still need to be compatible with those without any extra defines.
Yes, but the above DT entry will still be valid if you add the correct
of-thermal support. In fact, you would add in your DTS a thermal-zones
node, and in one of the defined zone, you would then reference the node
you already got into mainline. Below is a copy of the doc I mentioned
before:
#include <dt-bindings/thermal/thermal.h>
ocp {
...
/*
* A simple IC with several bandgap temperature sensors.
*/
bandgap0: bandgap@0x0000ED00 {
...
#thermal-sensor-cells = <1>;
};
};
thermal-zones {
cpu_thermal: cpu-thermal {
polling-delay-passive = <250>; /* milliseconds */
polling-delay = <1000>; /* milliseconds */
/* sensor ID */
thermal-sensors = <&bandgap0 0>;
trips {
/* each zone within the SoC may have its own trips */
cpu_alert: cpu-alert {
temperature = <100000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "passive";
};
cpu_crit: cpu-crit {
temperature = <125000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "critical";
};
};
cooling-maps {
/* each zone within the SoC may have its own cooling */
...
};
};
>
> Hence we need to define those slopes and offsets in the driver itself
> to stay compatible with those older device-trees.
not really, as long as we do not merge the driver with the missing
of-thermal support, I see no need to have both supports in your driver,
i.e., if we start correct for the beggining there is no need to have
offsets and slopes data in your driver code.
>
> As for if it works with the framework - I have to admit I do not
> have the slightest clue - it looks way to complicated for the soc right
> now.
Well, there is the documentation I mentioned, which several other
drivers used as base for their support. You can also look at other
thermal zones already defined in the existing DTS(I)'s.
>
> As a note: afaiu the trip_temp register is the temperature at which the
> soc will reboot on its own - similar to a watchdog, but for temperatures.
> (main reason for the assumption is because there is no interrupt that
> can get assigned a handler to catch this situation).
>
OK. But that does not prevent you to have other trips so your running
system can act before the shutdown trip is crossed.
> Martin
>
>
BR,
Eduardo Valentin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-02-02 4:29 ` Eduardo Valentin
0 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-02-02 4:29 UTC (permalink / raw)
To: linux-arm-kernel
Hello Martin,
On Tue, Jan 24, 2017 at 10:52:43AM +0100, kernel at martin.sperl.org wrote:
>
> > On 24.01.2017, at 10:31, Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > Hello Martin,
> >
> > On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel at martin.sperl.org wrote:
> >>
> >>> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
> >>>
> >>> Hello Martin,
> >>>
> >>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
> >>>> From: Martin Sperl <kernel@martin.sperl.org>
> >>>>
> >>>> Add basic thermal driver for bcm2835 SOC.
> >>>>
> >>>> This driver currently relies on the firmware setting up the
> >>>> tsense HW block and does not set it up itself.
> >>>>
> >>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> >>>> Acked-by: Eric Anholt <eric@anholt.net>
> >>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> >>>>
> >>>
> >>> <cut>
> >>>
> >>>> +
> >>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> >>>> + {
> >>>> + .compatible = "brcm,bcm2835-thermal",
> >>>> + .data = &(struct bcm2835_thermal_info) {
> >>>> + .offset = 407000,
> >>>> + .slope = -538,
> >>>> + .trip_temp = 80000
> >>>> + }
> >>>> + },
> >>>> + {
> >>>> + .compatible = "brcm,bcm2836-thermal",
> >>>> + .data = &(struct bcm2835_thermal_info) {
> >>>> + .offset = 407000,
> >>>> + .slope = -538,
> >>>> + .trip_temp = 80000
> >>>> + }
> >>>> + },
> >>>> + {
> >>>> + .compatible = "brcm,bcm2837-thermal",
> >>>> + .data = &(struct bcm2835_thermal_info) {
> >>>> + /* the bcm2837 needs adjustment of +5C */
> >>>> + .offset = 407000 + 5000,
> >>>> + .slope = -538,
> >>>> + .trip_temp = 80000
> >>>> + }
> >>>> + },
> >>>> + {},
> >>>
> >>> Just for the same of clarification, is there anything preventing this
> >>> driver of using of-thermal API? the above data (slope, offset, and
> >>> trip_temps) would be in DT the place where they are supposed to be,
> >>> instead of code.
> >>>
> >>
> >> As the DT changes, that only define compatible strings, have already gone
> >> in without any such properties set, we need to define defaults for the
> >> slope/offset and trip_temp values.
> >>
> >
> > These properties won't go into the same node you are referring to. They
> > go into the thermal-zone node you would create, which would then refer
> > to the node you referred (already merged). Therefore, I do not see
> > anything blocking a proper of-thermal usage to cover for the above data.
> >
> >> I guess (for newer SOC) you still can use the values in the DT,
> >> as (I guess) these are parsed and set in thermal_zone_device_register
> >> after the defaults are set in thermal_zone_params.
> >
> > Not sure what you meant here, but these values, when correctly used in
> > DT, they would come as part of the thermal_zone_params and in the
> > thermal trips of the thermal zones, as the of-thermal code with already
> > deal with those for you.
> >
> > Please have a look at:
> > a. Documentation/devicetree/bindings/thermal/thermal.txt
> > b. drivers/thermal/of-thermal.c
> >
> > And let me know if you see anything that would prevent this driver of
> > using the correct API to describe hardware data with DT.
>
>
> I guess you miss my point:
Maybe you missed, or did not read the doc I pointed you...
> The argument is that we have DT in the 4.10.0-rc2 kernel right now that
> look like this:
> thermal at 7e212000 {
> compatible = "brcm,bcm2835-thermal";
> clocks = <0x6 0x1b>;
> status = "okay";
> reg = <0x7e212000 0x8>;
> }
> so we still need to be compatible with those without any extra defines.
Yes, but the above DT entry will still be valid if you add the correct
of-thermal support. In fact, you would add in your DTS a thermal-zones
node, and in one of the defined zone, you would then reference the node
you already got into mainline. Below is a copy of the doc I mentioned
before:
#include <dt-bindings/thermal/thermal.h>
ocp {
...
/*
* A simple IC with several bandgap temperature sensors.
*/
bandgap0: bandgap at 0x0000ED00 {
...
#thermal-sensor-cells = <1>;
};
};
thermal-zones {
cpu_thermal: cpu-thermal {
polling-delay-passive = <250>; /* milliseconds */
polling-delay = <1000>; /* milliseconds */
/* sensor ID */
thermal-sensors = <&bandgap0 0>;
trips {
/* each zone within the SoC may have its own trips */
cpu_alert: cpu-alert {
temperature = <100000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "passive";
};
cpu_crit: cpu-crit {
temperature = <125000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "critical";
};
};
cooling-maps {
/* each zone within the SoC may have its own cooling */
...
};
};
>
> Hence we need to define those slopes and offsets in the driver itself
> to stay compatible with those older device-trees.
not really, as long as we do not merge the driver with the missing
of-thermal support, I see no need to have both supports in your driver,
i.e., if we start correct for the beggining there is no need to have
offsets and slopes data in your driver code.
>
> As for if it works with the framework - I have to admit I do not
> have the slightest clue - it looks way to complicated for the soc right
> now.
Well, there is the documentation I mentioned, which several other
drivers used as base for their support. You can also look at other
thermal zones already defined in the existing DTS(I)'s.
>
> As a note: afaiu the trip_temp register is the temperature at which the
> soc will reboot on its own - similar to a watchdog, but for temperatures.
> (main reason for the assumption is because there is no interrupt that
> can get assigned a handler to catch this situation).
>
OK. But that does not prevent you to have other trips so your running
system can act before the shutdown trip is crossed.
> Martin
>
>
BR,
Eduardo Valentin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-02-02 4:29 ` Eduardo Valentin
@ 2017-02-04 8:35 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 37+ messages in thread
From: kernel @ 2017-02-04 8:35 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Zhang Rui, Lee Jones, Eric Anholt, linux-pm, linux-rpi-kernel,
linux-arm-kernel
> On 02.02.2017, at 05:29, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Tue, Jan 24, 2017 at 10:52:43AM +0100, kernel@martin.sperl.org wrote:
>>
>>> On 24.01.2017, at 10:31, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>
>>> Hello Martin,
>>>
>>> On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel@martin.sperl.org wrote:
>>>>
>>>>> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>>>
>>>>> Hello Martin,
>>>>>
>>>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel@martin.sperl.org wrote:
>>>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>>>
>>>>>> Add basic thermal driver for bcm2835 SOC.
>>>>>>
>>>>>> This driver currently relies on the firmware setting up the
>>>>>> tsense HW block and does not set it up itself.
>>>>>>
>>>>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>>>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>>>
>>>>>
>>>>> <cut>
>>>>>
>>>>>> +
>>>>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>>>>>> + {
>>>>>> + .compatible = "brcm,bcm2835-thermal",
>>>>>> + .data = &(struct bcm2835_thermal_info) {
>>>>>> + .offset = 407000,
>>>>>> + .slope = -538,
>>>>>> + .trip_temp = 80000
>>>>>> + }
>>>>>> + },
>>>>>> + {
>>>>>> + .compatible = "brcm,bcm2836-thermal",
>>>>>> + .data = &(struct bcm2835_thermal_info) {
>>>>>> + .offset = 407000,
>>>>>> + .slope = -538,
>>>>>> + .trip_temp = 80000
>>>>>> + }
>>>>>> + },
>>>>>> + {
>>>>>> + .compatible = "brcm,bcm2837-thermal",
>>>>>> + .data = &(struct bcm2835_thermal_info) {
>>>>>> + /* the bcm2837 needs adjustment of +5C */
>>>>>> + .offset = 407000 + 5000,
>>>>>> + .slope = -538,
>>>>>> + .trip_temp = 80000
>>>>>> + }
>>>>>> + },
>>>>>> + {},
>>>>>
>>>>> Just for the same of clarification, is there anything preventing this
>>>>> driver of using of-thermal API? the above data (slope, offset, and
>>>>> trip_temps) would be in DT the place where they are supposed to be,
>>>>> instead of code.
>>>>>
>>>>
>>>> As the DT changes, that only define compatible strings, have already gone
>>>> in without any such properties set, we need to define defaults for the
>>>> slope/offset and trip_temp values.
>>>>
>>>
>>> These properties won't go into the same node you are referring to. They
>>> go into the thermal-zone node you would create, which would then refer
>>> to the node you referred (already merged). Therefore, I do not see
>>> anything blocking a proper of-thermal usage to cover for the above data.
>>>
>>>> I guess (for newer SOC) you still can use the values in the DT,
>>>> as (I guess) these are parsed and set in thermal_zone_device_register
>>>> after the defaults are set in thermal_zone_params.
>>>
>>> Not sure what you meant here, but these values, when correctly used in
>>> DT, they would come as part of the thermal_zone_params and in the
>>> thermal trips of the thermal zones, as the of-thermal code with already
>>> deal with those for you.
>>>
>>> Please have a look at:
>>> a. Documentation/devicetree/bindings/thermal/thermal.txt
>>> b. drivers/thermal/of-thermal.c
>>>
>>> And let me know if you see anything that would prevent this driver of
>>> using the correct API to describe hardware data with DT.
>>
>>
>> I guess you miss my point:
>
> Maybe you missed, or did not read the doc I pointed you...
>
>> The argument is that we have DT in the 4.10.0-rc2 kernel right now that
>> look like this:
>> thermal@7e212000 {
>> compatible = "brcm,bcm2835-thermal";
>> clocks = <0x6 0x1b>;
>> status = "okay";
>> reg = <0x7e212000 0x8>;
>> }
>> so we still need to be compatible with those without any extra defines.
>
> Yes, but the above DT entry will still be valid if you add the correct
> of-thermal support. In fact, you would add in your DTS a thermal-zones
> node, and in one of the defined zone, you would then reference the node
> you already got into mainline. Below is a copy of the doc I mentioned
> before:
>
>
> #include <dt-bindings/thermal/thermal.h>
>
> ocp {
> ...
> /*
> * A simple IC with several bandgap temperature sensors.
> */
> bandgap0: bandgap@0x0000ED00 {
> ...
> #thermal-sensor-cells = <1>;
> };
> };
>
> thermal-zones {
> cpu_thermal: cpu-thermal {
> polling-delay-passive = <250>; /* milliseconds */
> polling-delay = <1000>; /* milliseconds */
>
> /* sensor ID */
> thermal-sensors = <&bandgap0 0>;
>
> trips {
> /* each zone within the SoC may have its own trips */
> cpu_alert: cpu-alert {
> temperature = <100000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "passive";
> };
> cpu_crit: cpu-crit {
> temperature = <125000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "critical";
> };
> };
>
> cooling-maps {
> /* each zone within the SoC may have its own cooling */
> ...
> };
> };
>
>>
>> Hence we need to define those slopes and offsets in the driver itself
>> to stay compatible with those older device-trees.
>
> not really, as long as we do not merge the driver with the missing
> of-thermal support, I see no need to have both supports in your driver,
> i.e., if we start correct for the beggining there is no need to have
> offsets and slopes data in your driver code.
>
>>
>> As for if it works with the framework - I have to admit I do not
>> have the slightest clue - it looks way to complicated for the soc right
>> now.
>
> Well, there is the documentation I mentioned, which several other
> drivers used as base for their support. You can also look at other
> thermal zones already defined in the existing DTS(I)'s.
>
>>
>> As a note: afaiu the trip_temp register is the temperature at which the
>> soc will reboot on its own - similar to a watchdog, but for temperatures.
>> (main reason for the assumption is because there is no interrupt that
>> can get assigned a handler to catch this situation).
>>
>
> OK. But that does not prevent you to have other trips so your running
> system can act before the shutdown trip is crossed.
>
>> Martin
>>
>>
So how does this change/impact the driver code itself?
Defining a thermal zone in the dts is just an additional feature
that only impacts the device tree.
The DT as is works fine and at least allows to read the current temperature.
So can we just merge the driver now and look into the DT separately?
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-02-04 8:35 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-02-04 8:35 UTC (permalink / raw)
To: linux-arm-kernel
> On 02.02.2017, at 05:29, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> Hello Martin,
>
> On Tue, Jan 24, 2017 at 10:52:43AM +0100, kernel at martin.sperl.org wrote:
>>
>>> On 24.01.2017, at 10:31, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>
>>> Hello Martin,
>>>
>>> On Fri, Jan 20, 2017 at 08:54:07AM +0100, kernel at martin.sperl.org wrote:
>>>>
>>>>> On 20.01.2017, at 05:14, Eduardo Valentin <edubezval@gmail.com> wrote:
>>>>>
>>>>> Hello Martin,
>>>>>
>>>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, kernel at martin.sperl.org wrote:
>>>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>>>
>>>>>> Add basic thermal driver for bcm2835 SOC.
>>>>>>
>>>>>> This driver currently relies on the firmware setting up the
>>>>>> tsense HW block and does not set it up itself.
>>>>>>
>>>>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>>>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>>>
>>>>>
>>>>> <cut>
>>>>>
>>>>>> +
>>>>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>>>>>> + {
>>>>>> + .compatible = "brcm,bcm2835-thermal",
>>>>>> + .data = &(struct bcm2835_thermal_info) {
>>>>>> + .offset = 407000,
>>>>>> + .slope = -538,
>>>>>> + .trip_temp = 80000
>>>>>> + }
>>>>>> + },
>>>>>> + {
>>>>>> + .compatible = "brcm,bcm2836-thermal",
>>>>>> + .data = &(struct bcm2835_thermal_info) {
>>>>>> + .offset = 407000,
>>>>>> + .slope = -538,
>>>>>> + .trip_temp = 80000
>>>>>> + }
>>>>>> + },
>>>>>> + {
>>>>>> + .compatible = "brcm,bcm2837-thermal",
>>>>>> + .data = &(struct bcm2835_thermal_info) {
>>>>>> + /* the bcm2837 needs adjustment of +5C */
>>>>>> + .offset = 407000 + 5000,
>>>>>> + .slope = -538,
>>>>>> + .trip_temp = 80000
>>>>>> + }
>>>>>> + },
>>>>>> + {},
>>>>>
>>>>> Just for the same of clarification, is there anything preventing this
>>>>> driver of using of-thermal API? the above data (slope, offset, and
>>>>> trip_temps) would be in DT the place where they are supposed to be,
>>>>> instead of code.
>>>>>
>>>>
>>>> As the DT changes, that only define compatible strings, have already gone
>>>> in without any such properties set, we need to define defaults for the
>>>> slope/offset and trip_temp values.
>>>>
>>>
>>> These properties won't go into the same node you are referring to. They
>>> go into the thermal-zone node you would create, which would then refer
>>> to the node you referred (already merged). Therefore, I do not see
>>> anything blocking a proper of-thermal usage to cover for the above data.
>>>
>>>> I guess (for newer SOC) you still can use the values in the DT,
>>>> as (I guess) these are parsed and set in thermal_zone_device_register
>>>> after the defaults are set in thermal_zone_params.
>>>
>>> Not sure what you meant here, but these values, when correctly used in
>>> DT, they would come as part of the thermal_zone_params and in the
>>> thermal trips of the thermal zones, as the of-thermal code with already
>>> deal with those for you.
>>>
>>> Please have a look at:
>>> a. Documentation/devicetree/bindings/thermal/thermal.txt
>>> b. drivers/thermal/of-thermal.c
>>>
>>> And let me know if you see anything that would prevent this driver of
>>> using the correct API to describe hardware data with DT.
>>
>>
>> I guess you miss my point:
>
> Maybe you missed, or did not read the doc I pointed you...
>
>> The argument is that we have DT in the 4.10.0-rc2 kernel right now that
>> look like this:
>> thermal at 7e212000 {
>> compatible = "brcm,bcm2835-thermal";
>> clocks = <0x6 0x1b>;
>> status = "okay";
>> reg = <0x7e212000 0x8>;
>> }
>> so we still need to be compatible with those without any extra defines.
>
> Yes, but the above DT entry will still be valid if you add the correct
> of-thermal support. In fact, you would add in your DTS a thermal-zones
> node, and in one of the defined zone, you would then reference the node
> you already got into mainline. Below is a copy of the doc I mentioned
> before:
>
>
> #include <dt-bindings/thermal/thermal.h>
>
> ocp {
> ...
> /*
> * A simple IC with several bandgap temperature sensors.
> */
> bandgap0: bandgap at 0x0000ED00 {
> ...
> #thermal-sensor-cells = <1>;
> };
> };
>
> thermal-zones {
> cpu_thermal: cpu-thermal {
> polling-delay-passive = <250>; /* milliseconds */
> polling-delay = <1000>; /* milliseconds */
>
> /* sensor ID */
> thermal-sensors = <&bandgap0 0>;
>
> trips {
> /* each zone within the SoC may have its own trips */
> cpu_alert: cpu-alert {
> temperature = <100000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "passive";
> };
> cpu_crit: cpu-crit {
> temperature = <125000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "critical";
> };
> };
>
> cooling-maps {
> /* each zone within the SoC may have its own cooling */
> ...
> };
> };
>
>>
>> Hence we need to define those slopes and offsets in the driver itself
>> to stay compatible with those older device-trees.
>
> not really, as long as we do not merge the driver with the missing
> of-thermal support, I see no need to have both supports in your driver,
> i.e., if we start correct for the beggining there is no need to have
> offsets and slopes data in your driver code.
>
>>
>> As for if it works with the framework - I have to admit I do not
>> have the slightest clue - it looks way to complicated for the soc right
>> now.
>
> Well, there is the documentation I mentioned, which several other
> drivers used as base for their support. You can also look at other
> thermal zones already defined in the existing DTS(I)'s.
>
>>
>> As a note: afaiu the trip_temp register is the temperature at which the
>> soc will reboot on its own - similar to a watchdog, but for temperatures.
>> (main reason for the assumption is because there is no interrupt that
>> can get assigned a handler to catch this situation).
>>
>
> OK. But that does not prevent you to have other trips so your running
> system can act before the shutdown trip is crossed.
>
>> Martin
>>
>>
So how does this change/impact the driver code itself?
Defining a thermal zone in the dts is just an additional feature
that only impacts the device tree.
The DT as is works fine and at least allows to read the current temperature.
So can we just merge the driver now and look into the DT separately?
Martin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-02-02 4:29 ` Eduardo Valentin
@ 2017-02-04 9:36 ` Stefan Wahren
-1 siblings, 0 replies; 37+ messages in thread
From: Stefan Wahren @ 2017-02-04 9:36 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Lee Jones, Zhang Rui, linux-pm, linux-rpi-kernel,
linux-arm-kernel, kernel
Hi Eduardo,
> Eduardo Valentin <edubezval@gmail.com> hat am 2. Februar 2017 um 05:29 geschrieben:
>
>
> Hello Martin,
>
> ...
>
>
> #include <dt-bindings/thermal/thermal.h>
>
> ocp {
> ...
> /*
> * A simple IC with several bandgap temperature sensors.
> */
> bandgap0: bandgap@0x0000ED00 {
> ...
> #thermal-sensor-cells = <1>;
> };
> };
>
> thermal-zones {
> cpu_thermal: cpu-thermal {
> polling-delay-passive = <250>; /* milliseconds */
> polling-delay = <1000>; /* milliseconds */
>
> /* sensor ID */
> thermal-sensors = <&bandgap0 0>;
>
> trips {
> /* each zone within the SoC may have its own trips */
> cpu_alert: cpu-alert {
> temperature = <100000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "passive";
> };
> cpu_crit: cpu-crit {
> temperature = <125000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "critical";
> };
> };
>
> cooling-maps {
> /* each zone within the SoC may have its own cooling */
> ...
> };
> };
>
if i get it right the device tree binding requires also a cooling map. But how should we model this without a fan or a DVFS driver? Is there something like a placeholder? Do you have an example?
Regards
Stefan
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-02-04 9:36 ` Stefan Wahren
0 siblings, 0 replies; 37+ messages in thread
From: Stefan Wahren @ 2017-02-04 9:36 UTC (permalink / raw)
To: linux-arm-kernel
Hi Eduardo,
> Eduardo Valentin <edubezval@gmail.com> hat am 2. Februar 2017 um 05:29 geschrieben:
>
>
> Hello Martin,
>
> ...
>
>
> #include <dt-bindings/thermal/thermal.h>
>
> ocp {
> ...
> /*
> * A simple IC with several bandgap temperature sensors.
> */
> bandgap0: bandgap at 0x0000ED00 {
> ...
> #thermal-sensor-cells = <1>;
> };
> };
>
> thermal-zones {
> cpu_thermal: cpu-thermal {
> polling-delay-passive = <250>; /* milliseconds */
> polling-delay = <1000>; /* milliseconds */
>
> /* sensor ID */
> thermal-sensors = <&bandgap0 0>;
>
> trips {
> /* each zone within the SoC may have its own trips */
> cpu_alert: cpu-alert {
> temperature = <100000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "passive";
> };
> cpu_crit: cpu-crit {
> temperature = <125000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "critical";
> };
> };
>
> cooling-maps {
> /* each zone within the SoC may have its own cooling */
> ...
> };
> };
>
if i get it right the device tree binding requires also a cooling map. But how should we model this without a fan or a DVFS driver? Is there something like a placeholder? Do you have an example?
Regards
Stefan
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example
2017-02-04 8:35 ` kernel at martin.sperl.org
(?)
@ 2017-02-04 14:16 ` Stefan Wahren
2017-02-04 14:16 ` [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU Stefan Wahren
2017-02-08 22:02 ` [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example Rob Herring
-1 siblings, 2 replies; 37+ messages in thread
From: Stefan Wahren @ 2017-02-04 14:16 UTC (permalink / raw)
To: Eduardo Valentin, Zhang Rui, kernel
Cc: Eric Anholt, Rob Herring, Frank Rowand, Florian Fainelli,
linux-rpi-kernel, devicetree, linux-pm, Stefan Wahren
Add a thermal zone in order to make the example complete.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
.../bindings/thermal/brcm,bcm2835-thermal.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
index 474531d..680aca9 100644
--- a/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
@@ -10,6 +10,25 @@ clocks: Phandle of the clock used by the thermal sensor.
Example:
+thermal-zones {
+ gpu_thermal: gpu-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&thermal>;
+
+ trips {
+ cpu-crit {
+ temperature = <80000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ cooling-maps {
+ };
+ };
+};
+
thermal: thermal@7e212000 {
compatible = "brcm,bcm2835-thermal";
reg = <0x7e212000 0x8>;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
2017-02-04 14:16 ` [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example Stefan Wahren
@ 2017-02-04 14:16 ` Stefan Wahren
[not found] ` <1486217787-15703-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2017-02-08 22:02 ` [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example Rob Herring
1 sibling, 1 reply; 37+ messages in thread
From: Stefan Wahren @ 2017-02-04 14:16 UTC (permalink / raw)
To: Eduardo Valentin, Zhang Rui, kernel
Cc: Eric Anholt, Rob Herring, Frank Rowand, Florian Fainelli,
linux-rpi-kernel, devicetree, linux-pm, Stefan Wahren
As suggested by Eduardo Valentin this adds the thermal zone for
the bcm2835 SoC. Since we currently don't have any cooling devices
leave this section empty.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index 9c7ec7e..359da9d 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -19,6 +19,25 @@
bootargs = "earlyprintk console=ttyAMA0";
};
+ thermal-zones {
+ gpu_thermal: gpu-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&thermal>;
+
+ trips {
+ cpu-crit {
+ temperature = <80000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ cooling-maps {
+ };
+ };
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <1>;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
[not found] ` <1486217787-15703-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
@ 2017-02-08 4:19 ` Eduardo Valentin
[not found] ` <20170208041929.GA6809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
0 siblings, 1 reply; 37+ messages in thread
From: Eduardo Valentin @ 2017-02-08 4:19 UTC (permalink / raw)
To: Stefan Wahren
Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Eric Anholt,
Rob Herring, Frank Rowand, Florian Fainelli,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA
On Sat, Feb 04, 2017 at 02:16:27PM +0000, Stefan Wahren wrote:
> As suggested by Eduardo Valentin this adds the thermal zone for
> the bcm2835 SoC. Since we currently don't have any cooling devices
> leave this section empty.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> index 9c7ec7e..359da9d 100644
> --- a/arch/arm/boot/dts/bcm283x.dtsi
> +++ b/arch/arm/boot/dts/bcm283x.dtsi
> @@ -19,6 +19,25 @@
> bootargs = "earlyprintk console=ttyAMA0";
> };
>
> + thermal-zones {
> + gpu_thermal: gpu-thermal {
> + polling-delay-passive = <0>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&thermal>;
> +
> + trips {
> + cpu-crit {
> + temperature = <80000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> + };
> + cooling-maps {
> + };
> + };
> + };
> +
This is fine with me. Checking the other patches in the series.
> soc {
> compatible = "simple-bus";
> #address-cells = <1>;
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
[not found] ` <20170208041929.GA6809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2017-02-08 4:23 ` Eduardo Valentin
[not found] ` <20170208042351.GB6809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
0 siblings, 1 reply; 37+ messages in thread
From: Eduardo Valentin @ 2017-02-08 4:23 UTC (permalink / raw)
To: Stefan Wahren
Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Eric Anholt,
Rob Herring, Frank Rowand, Florian Fainelli,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA
On Tue, Feb 07, 2017 at 08:19:31PM -0800, Eduardo Valentin wrote:
> On Sat, Feb 04, 2017 at 02:16:27PM +0000, Stefan Wahren wrote:
> > As suggested by Eduardo Valentin this adds the thermal zone for
> > the bcm2835 SoC. Since we currently don't have any cooling devices
> > leave this section empty.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > ---
> > arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> > index 9c7ec7e..359da9d 100644
> > --- a/arch/arm/boot/dts/bcm283x.dtsi
> > +++ b/arch/arm/boot/dts/bcm283x.dtsi
> > @@ -19,6 +19,25 @@
> > bootargs = "earlyprintk console=ttyAMA0";
> > };
> >
> > + thermal-zones {
> > + gpu_thermal: gpu-thermal {
> > + polling-delay-passive = <0>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&thermal>;
Just came to my mind, dont you need to have an id to specify with sensor
points to gpu?
> > +
> > + trips {
> > + cpu-crit {
> > + temperature = <80000>;
> > + hysteresis = <0>;
> > + type = "critical";
> > + };
> > + };
> > + cooling-maps {
> > + };
> > + };
> > + };
> > +
>
> This is fine with me. Checking the other patches in the series.
>
> > soc {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > --
> > 1.7.9.5
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-02-04 8:35 ` kernel at martin.sperl.org
@ 2017-02-08 4:31 ` Eduardo Valentin
-1 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-02-08 4:31 UTC (permalink / raw)
To: kernel-TqfNSX0MhmxHKSADF0wUEw
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, Lee Jones, linux-rpi-kernel,
Zhang Rui, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Sat, Feb 04, 2017 at 09:35:47AM +0100, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>
> So how does this change/impact the driver code itself?
>
> Defining a thermal zone in the dts is just an additional feature
> that only impacts the device tree.
> The DT as is works fine and at least allows to read the current temperature.
Well, your driver is still half finished. It is a DT only driver, which
does not follow the DT spec on thermal. The impact on your code is that
it wont need to carry your hardware data in the source code.
Also, having the data described in DT allows you porting your driver
without patching it, in case you need, or any other system engineer,
to have different thermal data, trips values, number or trips, offset
and slope per sensor, on different boards, or even on different chip
version.
Not to say that having the data described in DT also allows system
engineers to deploy different thermal zones, based on your
driver/sensor, to extrapolate different hotspots.
> Martin
>
BR,
Eduardo Valentin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-02-08 4:31 ` Eduardo Valentin
0 siblings, 0 replies; 37+ messages in thread
From: Eduardo Valentin @ 2017-02-08 4:31 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Feb 04, 2017 at 09:35:47AM +0100, kernel at martin.sperl.org wrote:
>
> So how does this change/impact the driver code itself?
>
> Defining a thermal zone in the dts is just an additional feature
> that only impacts the device tree.
> The DT as is works fine and at least allows to read the current temperature.
Well, your driver is still half finished. It is a DT only driver, which
does not follow the DT spec on thermal. The impact on your code is that
it wont need to carry your hardware data in the source code.
Also, having the data described in DT allows you porting your driver
without patching it, in case you need, or any other system engineer,
to have different thermal data, trips values, number or trips, offset
and slope per sensor, on different boards, or even on different chip
version.
Not to say that having the data described in DT also allows system
engineers to deploy different thermal zones, based on your
driver/sensor, to extrapolate different hotspots.
> Martin
>
BR,
Eduardo Valentin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
2017-02-08 4:31 ` Eduardo Valentin
@ 2017-02-08 8:19 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 37+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2017-02-08 8:19 UTC (permalink / raw)
To: Eduardo Valentin
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, Lee Jones, linux-rpi-kernel,
Zhang Rui, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
> On 08.02.2017, at 05:31, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> On Sat, Feb 04, 2017 at 09:35:47AM +0100, kernel@martin.sperl.org wrote:
>>
>
>> So how does this change/impact the driver code itself?
>>
>> Defining a thermal zone in the dts is just an additional feature
>> that only impacts the device tree.
>> The DT as is works fine and at least allows to read the current temperature.
>
> Well, your driver is still half finished. It is a DT only driver, which
> does not follow the DT spec on thermal. The impact on your code is that
> it wont need to carry your hardware data in the source code.
>
> Also, having the data described in DT allows you porting your driver
> without patching it, in case you need, or any other system engineer,
> to have different thermal data, trips values, number or trips, offset
> and slope per sensor, on different boards, or even on different chip
> version.
>
> Not to say that having the data described in DT also allows system
> engineers to deploy different thermal zones, based on your
> driver/sensor, to extrapolate different hotspots.
>
That is all true and as far as I understand you can do all of that
using the current driver - see the patch-sets by Stefan Wahren that
incorporates this into the dts.
My point is still: there are dtb’s that come with 4.10-rcX
that do not have all of those defined
But as the mantra for DT is: "it is an API”, we have to be backwards
compatible from the driver perspective.
So we need those values in the driver to ensure the older version
of dtb’s are working correctly. That is why it is still included.
Also the settings of the trip are for the HW trip point for the case
that a future version of the firmware does not set up the thermal
HW block.
Do you really want to break this Mantra of backwards compatibility
by having those compatiblity “defines” stripped out with all the
possible negative side-effects if things are not set up correctly?
Thanks,
Martin
P.s: I find it strange that V3 of the driver was sent out in May 2016
(way before the DTS changes got merged) and you only started to
give feedback in November only in V8 (that primarily incorporated
minor changes and a rebase to 4.9) by which time the changes to the
dts have already been merged.
_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc
@ 2017-02-08 8:19 ` kernel at martin.sperl.org
0 siblings, 0 replies; 37+ messages in thread
From: kernel at martin.sperl.org @ 2017-02-08 8:19 UTC (permalink / raw)
To: linux-arm-kernel
> On 08.02.2017, at 05:31, Eduardo Valentin <edubezval@gmail.com> wrote:
>
> On Sat, Feb 04, 2017 at 09:35:47AM +0100, kernel at martin.sperl.org wrote:
>>
>
>> So how does this change/impact the driver code itself?
>>
>> Defining a thermal zone in the dts is just an additional feature
>> that only impacts the device tree.
>> The DT as is works fine and at least allows to read the current temperature.
>
> Well, your driver is still half finished. It is a DT only driver, which
> does not follow the DT spec on thermal. The impact on your code is that
> it wont need to carry your hardware data in the source code.
>
> Also, having the data described in DT allows you porting your driver
> without patching it, in case you need, or any other system engineer,
> to have different thermal data, trips values, number or trips, offset
> and slope per sensor, on different boards, or even on different chip
> version.
>
> Not to say that having the data described in DT also allows system
> engineers to deploy different thermal zones, based on your
> driver/sensor, to extrapolate different hotspots.
>
That is all true and as far as I understand you can do all of that
using the current driver - see the patch-sets by Stefan Wahren that
incorporates this into the dts.
My point is still: there are dtb?s that come with 4.10-rcX
that do not have all of those defined
But as the mantra for DT is: "it is an API?, we have to be backwards
compatible from the driver perspective.
So we need those values in the driver to ensure the older version
of dtb?s are working correctly. That is why it is still included.
Also the settings of the trip are for the HW trip point for the case
that a future version of the firmware does not set up the thermal
HW block.
Do you really want to break this Mantra of backwards compatibility
by having those compatiblity ?defines? stripped out with all the
possible negative side-effects if things are not set up correctly?
Thanks,
Martin
P.s: I find it strange that V3 of the driver was sent out in May 2016
(way before the DTS changes got merged) and you only started to
give feedback in November only in V8 (that primarily incorporated
minor changes and a rebase to 4.9) by which time the changes to the
dts have already been merged.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
[not found] ` <20170208042351.GB6809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2017-02-08 9:56 ` Stefan Wahren
2017-02-08 19:50 ` Eric Anholt
0 siblings, 1 reply; 37+ messages in thread
From: Stefan Wahren @ 2017-02-08 9:56 UTC (permalink / raw)
To: Eduardo Valentin, Eric Anholt
Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Rob Herring,
Frank Rowand, Florian Fainelli,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA
Am 08.02.2017 um 05:23 schrieb Eduardo Valentin:
> On Tue, Feb 07, 2017 at 08:19:31PM -0800, Eduardo Valentin wrote:
>> On Sat, Feb 04, 2017 at 02:16:27PM +0000, Stefan Wahren wrote:
>>> As suggested by Eduardo Valentin this adds the thermal zone for
>>> the bcm2835 SoC. Since we currently don't have any cooling devices
>>> leave this section empty.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>> ---
>>> arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
>>> index 9c7ec7e..359da9d 100644
>>> --- a/arch/arm/boot/dts/bcm283x.dtsi
>>> +++ b/arch/arm/boot/dts/bcm283x.dtsi
>>> @@ -19,6 +19,25 @@
>>> bootargs = "earlyprintk console=ttyAMA0";
>>> };
>>>
>>> + thermal-zones {
>>> + gpu_thermal: gpu-thermal {
>>> + polling-delay-passive = <0>;
>>> + polling-delay = <1000>;
>>> +
>>> + thermal-sensors = <&thermal>;
> Just came to my mind, dont you need to have an id to specify with sensor
> points to gpu?
Sorry, i don't know the exact setup of the single thermal sensor on the
SoC (datasheet doesn't provide any helpful information). I adapted the
Renesas R-Car thermal binding because i think it would be the best match.
@Eric: What's your opinion?
>
>>> +
>>> + trips {
>>> + cpu-crit {
>>> + temperature = <80000>;
>>> + hysteresis = <0>;
>>> + type = "critical";
>>> + };
>>> + };
>>> + cooling-maps {
>>> + };
>>> + };
>>> + };
>>> +
>> This is fine with me. Checking the other patches in the series.
>>
>>> soc {
>>> compatible = "simple-bus";
>>> #address-cells = <1>;
>>> --
>>> 1.7.9.5
>>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
2017-02-08 9:56 ` Stefan Wahren
@ 2017-02-08 19:50 ` Eric Anholt
2017-02-09 17:48 ` Stefan Wahren
0 siblings, 1 reply; 37+ messages in thread
From: Eric Anholt @ 2017-02-08 19:50 UTC (permalink / raw)
To: Stefan Wahren, Eduardo Valentin
Cc: Zhang Rui, kernel, Rob Herring, Frank Rowand, Florian Fainelli,
linux-rpi-kernel, devicetree, linux-pm
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
Stefan Wahren <stefan.wahren@i2se.com> writes:
> Am 08.02.2017 um 05:23 schrieb Eduardo Valentin:
>> On Tue, Feb 07, 2017 at 08:19:31PM -0800, Eduardo Valentin wrote:
>>> On Sat, Feb 04, 2017 at 02:16:27PM +0000, Stefan Wahren wrote:
>>>> As suggested by Eduardo Valentin this adds the thermal zone for
>>>> the bcm2835 SoC. Since we currently don't have any cooling devices
>>>> leave this section empty.
>>>>
>>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>> ---
>>>> arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
>>>> 1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
>>>> index 9c7ec7e..359da9d 100644
>>>> --- a/arch/arm/boot/dts/bcm283x.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm283x.dtsi
>>>> @@ -19,6 +19,25 @@
>>>> bootargs = "earlyprintk console=ttyAMA0";
>>>> };
>>>>
>>>> + thermal-zones {
>>>> + gpu_thermal: gpu-thermal {
>>>> + polling-delay-passive = <0>;
>>>> + polling-delay = <1000>;
>>>> +
>>>> + thermal-sensors = <&thermal>;
>> Just came to my mind, dont you need to have an id to specify with sensor
>> points to gpu?
>
> Sorry, i don't know the exact setup of the single thermal sensor on the
> SoC (datasheet doesn't provide any helpful information). I adapted the
> Renesas R-Car thermal binding because i think it would be the best match.
>
> @Eric: What's your opinion?
I don't understand the question.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example
2017-02-04 14:16 ` [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example Stefan Wahren
2017-02-04 14:16 ` [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU Stefan Wahren
@ 2017-02-08 22:02 ` Rob Herring
1 sibling, 0 replies; 37+ messages in thread
From: Rob Herring @ 2017-02-08 22:02 UTC (permalink / raw)
To: Stefan Wahren
Cc: Eduardo Valentin, Zhang Rui, kernel, Eric Anholt, Frank Rowand,
Florian Fainelli, linux-rpi-kernel, devicetree, linux-pm
On Sat, Feb 04, 2017 at 02:16:26PM +0000, Stefan Wahren wrote:
> Add a thermal zone in order to make the example complete.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> .../bindings/thermal/brcm,bcm2835-thermal.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
2017-02-08 19:50 ` Eric Anholt
@ 2017-02-09 17:48 ` Stefan Wahren
[not found] ` <124007607.358509.1486662532562-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
0 siblings, 1 reply; 37+ messages in thread
From: Stefan Wahren @ 2017-02-09 17:48 UTC (permalink / raw)
To: Eric Anholt, Eduardo Valentin
Cc: Frank Rowand, kernel, Zhang Rui, Rob Herring, Florian Fainelli,
linux-rpi-kernel, linux-pm, devicetree
Hi Eric,
> Eric Anholt <eric@anholt.net> hat am 8. Februar 2017 um 20:50 geschrieben:
>
>
> Stefan Wahren <stefan.wahren@i2se.com> writes:
>
> > Am 08.02.2017 um 05:23 schrieb Eduardo Valentin:
> >> On Tue, Feb 07, 2017 at 08:19:31PM -0800, Eduardo Valentin wrote:
> >>> On Sat, Feb 04, 2017 at 02:16:27PM +0000, Stefan Wahren wrote:
> >>>> As suggested by Eduardo Valentin this adds the thermal zone for
> >>>> the bcm2835 SoC. Since we currently don't have any cooling devices
> >>>> leave this section empty.
> >>>>
> >>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> >>>> ---
> >>>> arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
> >>>> 1 file changed, 19 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> >>>> index 9c7ec7e..359da9d 100644
> >>>> --- a/arch/arm/boot/dts/bcm283x.dtsi
> >>>> +++ b/arch/arm/boot/dts/bcm283x.dtsi
> >>>> @@ -19,6 +19,25 @@
> >>>> bootargs = "earlyprintk console=ttyAMA0";
> >>>> };
> >>>>
> >>>> + thermal-zones {
> >>>> + gpu_thermal: gpu-thermal {
> >>>> + polling-delay-passive = <0>;
> >>>> + polling-delay = <1000>;
> >>>> +
> >>>> + thermal-sensors = <&thermal>;
> >> Just came to my mind, dont you need to have an id to specify with sensor
> >> points to gpu?
> >
> > Sorry, i don't know the exact setup of the single thermal sensor on the
> > SoC (datasheet doesn't provide any helpful information). I adapted the
> > Renesas R-Car thermal binding because i think it would be the best match.
> >
> > @Eric: What's your opinion?
>
> I don't understand the question.
i hope to get it right. We are talking about defining the thermal zone [1].
There are 4 examples:
(a) - CPU thermal zone with one internal sensor
(b) - IC with several internal sensors
(c) - Several sensors within one single thermal zone
(d) - Board thermal
I decided to choose (a) for the patch and Eduardo tends to (b).
Here are the questions:
Where is the thermal sensor TSENS located (ARM core or VideoCore 4)?
Do we expect several internal sensors?
[1] - http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/thermal/thermal.txt
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU
[not found] ` <124007607.358509.1486662532562-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
@ 2017-02-09 23:34 ` Eric Anholt
0 siblings, 0 replies; 37+ messages in thread
From: Eric Anholt @ 2017-02-09 23:34 UTC (permalink / raw)
To: Stefan Wahren, Eduardo Valentin
Cc: Frank Rowand, kernel-TqfNSX0MhmxHKSADF0wUEw, Zhang Rui,
Rob Herring, Florian Fainelli,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]
Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
> Hi Eric,
>
>> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 8. Februar 2017 um 20:50 geschrieben:
>>
>>
>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
>>
>> > Am 08.02.2017 um 05:23 schrieb Eduardo Valentin:
>> >> On Tue, Feb 07, 2017 at 08:19:31PM -0800, Eduardo Valentin wrote:
>> >>> On Sat, Feb 04, 2017 at 02:16:27PM +0000, Stefan Wahren wrote:
>> >>>> As suggested by Eduardo Valentin this adds the thermal zone for
>> >>>> the bcm2835 SoC. Since we currently don't have any cooling devices
>> >>>> leave this section empty.
>> >>>>
>> >>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> >>>> ---
>> >>>> arch/arm/boot/dts/bcm283x.dtsi | 19 +++++++++++++++++++
>> >>>> 1 file changed, 19 insertions(+)
>> >>>>
>> >>>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
>> >>>> index 9c7ec7e..359da9d 100644
>> >>>> --- a/arch/arm/boot/dts/bcm283x.dtsi
>> >>>> +++ b/arch/arm/boot/dts/bcm283x.dtsi
>> >>>> @@ -19,6 +19,25 @@
>> >>>> bootargs = "earlyprintk console=ttyAMA0";
>> >>>> };
>> >>>>
>> >>>> + thermal-zones {
>> >>>> + gpu_thermal: gpu-thermal {
>> >>>> + polling-delay-passive = <0>;
>> >>>> + polling-delay = <1000>;
>> >>>> +
>> >>>> + thermal-sensors = <&thermal>;
>> >> Just came to my mind, dont you need to have an id to specify with sensor
>> >> points to gpu?
>> >
>> > Sorry, i don't know the exact setup of the single thermal sensor on the
>> > SoC (datasheet doesn't provide any helpful information). I adapted the
>> > Renesas R-Car thermal binding because i think it would be the best match.
>> >
>> > @Eric: What's your opinion?
>>
>> I don't understand the question.
>
> i hope to get it right. We are talking about defining the thermal zone [1].
>
> There are 4 examples:
>
> (a) - CPU thermal zone with one internal sensor
> (b) - IC with several internal sensors
> (c) - Several sensors within one single thermal zone
> (d) - Board thermal
>
> I decided to choose (a) for the patch and Eduardo tends to (b).
>
> Here are the questions:
>
> Where is the thermal sensor TSENS located (ARM core or VideoCore 4)?
>
> Do we expect several internal sensors?
There is only one temperature sensor on the chip, and no others on the
board that I know of. The docs for tsens don't say if the actual sensor
is closer to the center of the ARM or some VC4 component within the
chip.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2017-02-09 23:34 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-07 16:55 [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc kernel
2017-01-07 16:55 ` kernel at martin.sperl.org
2017-01-20 4:14 ` Eduardo Valentin
2017-01-20 4:14 ` Eduardo Valentin
2017-01-20 4:23 ` Eduardo Valentin
2017-01-20 4:23 ` Eduardo Valentin
[not found] ` <20170120042323.GA6651-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-01-20 8:43 ` kernel-TqfNSX0MhmxHKSADF0wUEw
2017-01-20 8:43 ` kernel at martin.sperl.org
2017-01-24 9:26 ` Eduardo Valentin
2017-01-24 9:26 ` Eduardo Valentin
2017-01-24 9:37 ` kernel
2017-01-24 9:37 ` kernel at martin.sperl.org
[not found] ` <20170120041400.GA24617-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-01-20 7:54 ` kernel-TqfNSX0MhmxHKSADF0wUEw
2017-01-20 7:54 ` kernel at martin.sperl.org
[not found] ` <060918B6-A773-46A5-8D10-C9F6BBA6D3F1-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2017-01-24 9:31 ` Eduardo Valentin
2017-01-24 9:31 ` Eduardo Valentin
2017-01-24 9:52 ` kernel
2017-01-24 9:52 ` kernel at martin.sperl.org
2017-02-02 4:29 ` Eduardo Valentin
2017-02-02 4:29 ` Eduardo Valentin
2017-02-04 8:35 ` kernel
2017-02-04 8:35 ` kernel at martin.sperl.org
2017-02-04 14:16 ` [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example Stefan Wahren
2017-02-04 14:16 ` [PATCH 2/2] ARM: dts: bcm283x: Add critical thermal zone for GPU Stefan Wahren
[not found] ` <1486217787-15703-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2017-02-08 4:19 ` Eduardo Valentin
[not found] ` <20170208041929.GA6809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-02-08 4:23 ` Eduardo Valentin
[not found] ` <20170208042351.GB6809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-02-08 9:56 ` Stefan Wahren
2017-02-08 19:50 ` Eric Anholt
2017-02-09 17:48 ` Stefan Wahren
[not found] ` <124007607.358509.1486662532562-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
2017-02-09 23:34 ` Eric Anholt
2017-02-08 22:02 ` [PATCH 1/2] dt-bindings: Add thermal zone to bcm2835-thermal example Rob Herring
[not found] ` <E0A4388D-788A-40B4-9193-36FD75284654-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2017-02-08 4:31 ` [PATCH V9] thermal: bcm2835: add thermal driver for bcm2835 soc Eduardo Valentin
2017-02-08 4:31 ` Eduardo Valentin
[not found] ` <20170208043107.GA7097-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-02-08 8:19 ` kernel-TqfNSX0MhmxHKSADF0wUEw
2017-02-08 8:19 ` kernel at martin.sperl.org
2017-02-04 9:36 ` Stefan Wahren
2017-02-04 9:36 ` Stefan Wahren
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.