* [PATCH V3] thermal: qoriq: Add thermal management support
@ 2015-09-23 8:28 Jia Hongtao
2015-09-24 22:09 ` Scott Wood
0 siblings, 1 reply; 8+ messages in thread
From: Jia Hongtao @ 2015-09-23 8:28 UTC (permalink / raw)
To: edubezval; +Cc: linux-pm, linuxppc-dev, scottwood, hongtao.jia
This driver add thermal management support by enabling TMU (Thermal
Monitoring Unit) on QorIQ platform.
It's based on thermal of framework:
- Trip points defined in device tree.
- Cpufreq as cooling device registered in qoriq cpufreq driver.
Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
---
V3: Using thermal of framework.
drivers/thermal/Kconfig | 11 ++
drivers/thermal/Makefile | 1 +
drivers/thermal/qoriq_thermal.c | 267 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 279 insertions(+)
create mode 100644 drivers/thermal/qoriq_thermal.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 0390044..c91041b 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -180,6 +180,17 @@ config IMX_THERMAL
cpufreq is used as the cooling device to throttle CPUs when the
passive trip is crossed.
+config QORIQ_THERMAL
+ tristate "Freescale QorIQ Thermal Monitoring Unit"
+ depends on CPU_THERMAL
+ depends on THERMAL_OF
+ default n
+ help
+ Enable thermal management based on Freescale QorIQ Thermal Monitoring
+ Unit (TMU). It supports one critical trip point and one passive trip
+ point. The cpufreq is used as the cooling device to throttle CPUs when
+ the passive trip is crossed.
+
config SPEAR_THERMAL
bool "SPEAr thermal sensor driver"
depends on PLAT_SPEAR
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 26f1608..e55d703 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_DOVE_THERMAL) += dove_thermal.o
obj-$(CONFIG_DB8500_THERMAL) += db8500_thermal.o
obj-$(CONFIG_ARMADA_THERMAL) += armada_thermal.o
obj-$(CONFIG_IMX_THERMAL) += imx_thermal.o
+obj-$(CONFIG_QORIQ_THERMAL) += qoriq_thermal.o
obj-$(CONFIG_DB8500_CPUFREQ_COOLING) += db8500_cpufreq_cooling.o
obj-$(CONFIG_INTEL_POWERCLAMP) += intel_powerclamp.o
obj-$(CONFIG_X86_PKG_TEMP_THERMAL) += x86_pkg_temp_thermal.o
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
new file mode 100644
index 0000000..7c2a3261
--- /dev/null
+++ b/drivers/thermal/qoriq_thermal.c
@@ -0,0 +1,267 @@
+/*
+ * Copyright 2015 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+/*
+ * Based on Freescale QorIQ Thermal Monitoring Unit (TMU)
+ */
+#include <linux/cpufreq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+#define SITES_MAX 16
+
+/*
+ * QorIQ TMU Registers
+ */
+struct qoriq_tmu_site_regs {
+ __be32 tritsr; /* Immediate Temperature Site Register */
+ __be32 tratsr; /* Average Temperature Site Register */
+ u8 res0[0x8];
+} __packed;
+
+struct qoriq_tmu_regs {
+ __be32 tmr; /* Mode Register */
+#define TMR_DISABLE 0x0
+#define TMR_ME 0x80000000
+#define TMR_ALPF 0x0c000000
+#define TMR_MSITE 0x00008000 /* Core temperature site */
+#define TMR_ALL (TMR_ME | TMR_ALPF | TMR_MSITE)
+ __be32 tsr; /* Status Register */
+ __be32 tmtmir; /* Temperature measurement interval Register */
+#define TMTMIR_DEFAULT 0x00000007
+ u8 res0[0x14];
+ __be32 tier; /* Interrupt Enable Register */
+#define TIER_DISABLE 0x0
+ __be32 tidr; /* Interrupt Detect Register */
+ __be32 tiscr; /* Interrupt Site Capture Register */
+ __be32 ticscr; /* Interrupt Critical Site Capture Register */
+ u8 res1[0x10];
+ __be32 tmhtcrh; /* High Temperature Capture Register */
+ __be32 tmhtcrl; /* Low Temperature Capture Register */
+ u8 res2[0x8];
+ __be32 tmhtitr; /* High Temperature Immediate Threshold */
+ __be32 tmhtatr; /* High Temperature Average Threshold */
+ __be32 tmhtactr; /* High Temperature Average Crit Threshold */
+ u8 res3[0x24];
+ __be32 ttcfgr; /* Temperature Configuration Register */
+ __be32 tscfgr; /* Sensor Configuration Register */
+ u8 res4[0x78];
+ struct qoriq_tmu_site_regs site[SITES_MAX];
+ u8 res5[0x9f8];
+ __be32 ipbrr0; /* IP Block Revision Register 0 */
+ __be32 ipbrr1; /* IP Block Revision Register 1 */
+ u8 res6[0x310];
+ __be32 ttr0cr; /* Temperature Range 0 Control Register */
+ __be32 ttr1cr; /* Temperature Range 1 Control Register */
+ __be32 ttr2cr; /* Temperature Range 2 Control Register */
+ __be32 ttr3cr; /* Temperature Range 3 Control Register */
+};
+
+/*
+ * Thermal zone data
+ */
+struct qoriq_tmu_data {
+ struct thermal_zone_device *tz;
+ struct qoriq_tmu_regs __iomem *regs;
+};
+
+static int tmu_get_temp(void *p, int *temp)
+{
+ u8 val;
+ struct qoriq_tmu_data *data = p;
+
+ val = ioread32be(&data->regs->site[0].tritsr);
+ *temp = (int)val * 1000;
+
+ return 0;
+}
+
+static int qoriq_tmu_calibration(struct platform_device *pdev)
+{
+ int i, val, len;
+ u32 range[4];
+ const __be32 *calibration;
+ struct device_node *node = pdev->dev.of_node;
+ struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
+
+ /* Disable monitoring before calibration */
+ iowrite32be(TMR_DISABLE, &data->regs->tmr);
+
+ if (of_property_read_u32_array(node, "fsl,tmu-range", range, 4))
+ return -1;
+
+ /* Init temperature range registers */
+ iowrite32be(range[0], &data->regs->ttr0cr);
+ iowrite32be(range[1], &data->regs->ttr1cr);
+ iowrite32be(range[2], &data->regs->ttr2cr);
+ iowrite32be(range[3], &data->regs->ttr3cr);
+
+ calibration = of_get_property(node, "fsl,tmu-calibration", &len);
+ if (calibration == NULL)
+ return -1;
+
+ for (i = 0; i < len; i += 8, calibration += 2) {
+ val = (int)of_read_number(calibration, 1);
+ iowrite32be(val, &data->regs->ttcfgr);
+ val = (int)of_read_number(calibration + 1, 1);
+ iowrite32be(val, &data->regs->tscfgr);
+ }
+
+ return 0;
+}
+
+static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
+{
+ /* Disable interrupt, using polling instead */
+ iowrite32be(TIER_DISABLE, &data->regs->tier);
+
+ /* Set update_interval */
+ iowrite32be(TMTMIR_DEFAULT, &data->regs->tmtmir);
+
+ /* Enable monitoring */
+ iowrite32be(TMR_ALL, &data->regs->tmr);
+}
+
+static struct thermal_zone_of_device_ops tmu_tz_ops = {
+ .get_temp = tmu_get_temp,
+};
+
+static int qoriq_tmu_probe(struct platform_device *pdev)
+{
+ int ret;
+ const struct thermal_trip *trip;
+ struct qoriq_tmu_data *data;
+
+ if (!pdev->dev.of_node) {
+ dev_err(&pdev->dev, "Device OF-Node is NULL");
+ return -EFAULT;
+ }
+
+ data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, data);
+ data->regs = of_iomap(pdev->dev.of_node, 0);
+
+ if (!data->regs) {
+ dev_err(&pdev->dev, "Failed to get memory region\n");
+ ret = -ENODEV;
+ goto err_iomap;
+ }
+
+ ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
+ if (ret < 0) {
+ dev_err(&pdev->dev, "TMU calibration failed.\n");
+ ret = -ENODEV;
+ goto err_iomap;
+ }
+
+ qoriq_tmu_init_device(data); /* TMU initialization */
+
+ data->tz = thermal_zone_of_sensor_register(&pdev->dev, 0,
+ data, &tmu_tz_ops);
+ if (IS_ERR(data->tz)) {
+ ret = PTR_ERR(data->tz);
+ dev_err(&pdev->dev,
+ "Failed to register thermal zone device %d\n", ret);
+ goto err_thermal;
+ }
+
+ trip = of_thermal_get_trip_points(data->tz);
+
+ data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
+
+ return 0;
+
+err_thermal:
+ iounmap(data->regs);
+
+err_iomap:
+ platform_set_drvdata(pdev, NULL);
+ devm_kfree(&pdev->dev, data);
+
+ return ret;
+}
+
+static int qoriq_tmu_remove(struct platform_device *pdev)
+{
+ struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
+
+ /* Disable monitoring */
+ iowrite32be(TMR_DISABLE, &data->regs->tmr);
+
+ thermal_zone_of_sensor_unregister(&pdev->dev, data->tz);
+ iounmap(data->regs);
+
+ platform_set_drvdata(pdev, NULL);
+ devm_kfree(&pdev->dev, data);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int qoriq_tmu_suspend(struct device *dev)
+{
+ struct qoriq_tmu_data *data = dev_get_drvdata(dev);
+
+ /* Disable monitoring */
+ iowrite32be(TMR_DISABLE, &data->regs->tmr);
+ data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_DISABLED);
+
+ return 0;
+}
+
+static int qoriq_tmu_resume(struct device *dev)
+{
+ struct qoriq_tmu_data *data = dev_get_drvdata(dev);
+
+ /* Enable monitoring */
+ iowrite32be(TMR_ALL, &data->regs->tmr);
+ data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(qoriq_tmu_pm_ops,
+ qoriq_tmu_suspend, qoriq_tmu_resume);
+
+static const struct of_device_id qoriq_tmu_match[] = {
+ { .compatible = "fsl,qoriq-tmu", },
+ {},
+};
+
+static struct platform_driver qoriq_tmu = {
+ .driver = {
+ .name = "qoriq_thermal",
+ .pm = &qoriq_tmu_pm_ops,
+ .of_match_table = qoriq_tmu_match,
+ },
+ .probe = qoriq_tmu_probe,
+ .remove = qoriq_tmu_remove,
+};
+module_platform_driver(qoriq_tmu);
+
+MODULE_AUTHOR("Jia Hongtao <hongtao.jia@freescale.com>");
+MODULE_DESCRIPTION("Freescale QorIQ Thermal Monitoring Unit driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0.27.g96db324
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V3] thermal: qoriq: Add thermal management support
2015-09-23 8:28 [PATCH V3] thermal: qoriq: Add thermal management support Jia Hongtao
@ 2015-09-24 22:09 ` Scott Wood
2015-09-25 3:09 ` Hongtao Jia
0 siblings, 1 reply; 8+ messages in thread
From: Scott Wood @ 2015-09-24 22:09 UTC (permalink / raw)
To: Jia Hongtao; +Cc: edubezval, linux-pm, linuxppc-dev
On Wed, 2015-09-23 at 16:28 +0800, Jia Hongtao wrote:
> This driver add thermal management support by enabling TMU (Thermal
> Monitoring Unit) on QorIQ platform.
>
> It's based on thermal of framework:
> - Trip points defined in device tree.
> - Cpufreq as cooling device registered in qoriq cpufreq driver.
I don't see any cooling device registered in the qoriq cpufreq driver. Is
this dependent on some other patch?
>
> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> ---
> V3: Using thermal of framework.
>
> drivers/thermal/Kconfig | 11 ++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/qoriq_thermal.c | 267
> ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 279 insertions(+)
> create mode 100644 drivers/thermal/qoriq_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 0390044..c91041b 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -180,6 +180,17 @@ config IMX_THERMAL
> cpufreq is used as the cooling device to throttle CPUs when the
> passive trip is crossed.
>
> +config QORIQ_THERMAL
> + tristate "Freescale QorIQ Thermal Monitoring Unit"
> + depends on CPU_THERMAL
> + depends on THERMAL_OF
> + default n
> + help
> + Enable thermal management based on Freescale QorIQ Thermal Monitoring
> + Unit (TMU). It supports one critical trip point and one passive trip
> + point. The cpufreq is used as the cooling device to throttle CPUs when
> + the passive trip is crossed.
"default n" is unnecessary -- n is already the default.
Where is the interaction between this driver and cpufreq?
> config SPEAR_THERMAL
> bool "SPEAr thermal sensor driver"
> depends on PLAT_SPEAR
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 26f1608..e55d703 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_DOVE_THERMAL) += dove_thermal.o
> obj-$(CONFIG_DB8500_THERMAL) += db8500_thermal.o
> obj-$(CONFIG_ARMADA_THERMAL) += armada_thermal.o
> obj-$(CONFIG_IMX_THERMAL) += imx_thermal.o
> +obj-$(CONFIG_QORIQ_THERMAL) += qoriq_thermal.o
> obj-$(CONFIG_DB8500_CPUFREQ_COOLING) += db8500_cpufreq_cooling.o
> obj-$(CONFIG_INTEL_POWERCLAMP) += intel_powerclamp.o
> obj-$(CONFIG_X86_PKG_TEMP_THERMAL) += x86_pkg_temp_thermal.o
> diff --git a/drivers/thermal/qoriq_thermal.c
> b/drivers/thermal/qoriq_thermal.c
> new file mode 100644
> index 0000000..7c2a3261
> --- /dev/null
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -0,0 +1,267 @@
> +/*
> + * Copyright 2015 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + */
> +
> +/*
> + * Based on Freescale QorIQ Thermal Monitoring Unit (TMU)
> + */
What does this comment mean? This *is* the "Freescale QorIQ Thermal
Monitoring Unit" driver.
> +#include <linux/cpufreq.h>
What do you use from this header?
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/thermal.h>
> +
> +#include "thermal_core.h"
> +
> +#define SITES_MAX 16
> +
> +/*
> + * QorIQ TMU Registers
> + */
> +struct qoriq_tmu_site_regs {
> + __be32 tritsr; /* Immediate Temperature Site Register */
> + __be32 tratsr; /* Average Temperature Site Register */
> + u8 res0[0x8];
> +} __packed;
> +
> +struct qoriq_tmu_regs {
> + __be32 tmr; /* Mode Register */
> +#define TMR_DISABLE 0x0
> +#define TMR_ME 0x80000000
> +#define TMR_ALPF 0x0c000000
> +#define TMR_MSITE 0x00008000 /* Core temperature site */
> +#define TMR_ALL (TMR_ME | TMR_ALPF | TMR_MSITE)
> + __be32 tsr; /* Status Register */
> + __be32 tmtmir; /* Temperature measurement interval Register */
> +#define TMTMIR_DEFAULT 0x00000007
> + u8 res0[0x14];
> + __be32 tier; /* Interrupt Enable Register */
> +#define TIER_DISABLE 0x0
> + __be32 tidr; /* Interrupt Detect Register */
> + __be32 tiscr; /* Interrupt Site Capture Register */
> + __be32 ticscr; /* Interrupt Critical Site Capture Register */
> + u8 res1[0x10];
> + __be32 tmhtcrh; /* High Temperature Capture Register */
> + __be32 tmhtcrl; /* Low Temperature Capture Register */
> + u8 res2[0x8];
> + __be32 tmhtitr; /* High Temperature Immediate Threshold */
> + __be32 tmhtatr; /* High Temperature Average Threshold */
> + __be32 tmhtactr; /* High Temperature Average Crit Threshold */
> + u8 res3[0x24];
> + __be32 ttcfgr; /* Temperature Configuration Register */
> + __be32 tscfgr; /* Sensor Configuration Register */
> + u8 res4[0x78];
> + struct qoriq_tmu_site_regs site[SITES_MAX];
> + u8 res5[0x9f8];
> + __be32 ipbrr0; /* IP Block Revision Register 0 */
> + __be32 ipbrr1; /* IP Block Revision Register 1 */
> + u8 res6[0x310];
> + __be32 ttr0cr; /* Temperature Range 0 Control Register */
> + __be32 ttr1cr; /* Temperature Range 1 Control Register */
> + __be32 ttr2cr; /* Temperature Range 2 Control Register */
> + __be32 ttr3cr; /* Temperature Range 3 Control Register */
> +};
> +
> +/*
> + * Thermal zone data
> + */
> +struct qoriq_tmu_data {
> + struct thermal_zone_device *tz;
> + struct qoriq_tmu_regs __iomem *regs;
> +};
> +
> +static int tmu_get_temp(void *p, int *temp)
> +{
> + u8 val;
> + struct qoriq_tmu_data *data = p;
> +
> + val = ioread32be(&data->regs->site[0].tritsr);
> + *temp = (int)val * 1000;
Why don't you declare val as int in the first place?
> + for (i = 0; i < len; i += 8, calibration += 2) {
> + val = (int)of_read_number(calibration, 1);
> + iowrite32be(val, &data->regs->ttcfgr);
> + val = (int)of_read_number(calibration + 1, 1);
> + iowrite32be(val, &data->regs->tscfgr);
Unnecessary casts.
> + }
> +
> + return 0;
> +}
> +
> +static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> +{
> + /* Disable interrupt, using polling instead */
> + iowrite32be(TIER_DISABLE, &data->regs->tier);
> +
> + /* Set update_interval */
> + iowrite32be(TMTMIR_DEFAULT, &data->regs->tmtmir);
> +
> + /* Enable monitoring */
> + iowrite32be(TMR_ALL, &data->regs->tmr);
> +}
> +
> +static struct thermal_zone_of_device_ops tmu_tz_ops = {
> + .get_temp = tmu_get_temp,
> +};
> +
> +static int qoriq_tmu_probe(struct platform_device *pdev)
> +{
> + int ret;
> + const struct thermal_trip *trip;
> + struct qoriq_tmu_data *data;
> +
> + if (!pdev->dev.of_node) {
> + dev_err(&pdev->dev, "Device OF-Node is NULL");
> + return -EFAULT;
> + }
EFAULT is for bad user addresses and is not appropriate here.
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
> + GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, data);
> + data->regs = of_iomap(pdev->dev.of_node, 0);
> +
> + if (!data->regs) {
> + dev_err(&pdev->dev, "Failed to get memory region\n");
> + ret = -ENODEV;
> + goto err_iomap;
> + }
> +
> + ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> + if (ret < 0) {
> + dev_err(&pdev->dev, "TMU calibration failed.\n");
> + ret = -ENODEV;
> + goto err_iomap;
> + }
That function returns negative when device tree properties are missing, not
when a calibration procedure fails.
> +
> + qoriq_tmu_init_device(data); /* TMU initialization */
> +
> + data->tz = thermal_zone_of_sensor_register(&pdev->dev, 0,
> + data, &tmu_tz_ops);
> + if (IS_ERR(data->tz)) {
> + ret = PTR_ERR(data->tz);
> + dev_err(&pdev->dev,
> + "Failed to register thermal zone device %d\n", ret);
> + goto err_thermal;
> + }
> +
> + trip = of_thermal_get_trip_points(data->tz);
> +
> + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
Doesn't thermal_zone_of_sensor_register() already call ops->set_mode with
TERMAL_DEVICE_ENABLED?
> +
> + return 0;
> +
> +err_thermal:
> + iounmap(data->regs);
> +
> +err_iomap:
> + platform_set_drvdata(pdev, NULL);
> + devm_kfree(&pdev->dev, data);
> +
> + return ret;
> +}
> +
> +static int qoriq_tmu_remove(struct platform_device *pdev)
> +{
> + struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> +
> + /* Disable monitoring */
> + iowrite32be(TMR_DISABLE, &data->regs->tmr);
> +
> + thermal_zone_of_sensor_unregister(&pdev->dev, data->tz);
> + iounmap(data->regs);
> +
> + platform_set_drvdata(pdev, NULL);
> + devm_kfree(&pdev->dev, data);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int qoriq_tmu_suspend(struct device *dev)
> +{
> + struct qoriq_tmu_data *data = dev_get_drvdata(dev);
> +
> + /* Disable monitoring */
> + iowrite32be(TMR_DISABLE, &data->regs->tmr);
> + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_DISABLED);
> +
> + return 0;
> +}
> +
> +static int qoriq_tmu_resume(struct device *dev)
> +{
> + struct qoriq_tmu_data *data = dev_get_drvdata(dev);
> +
> + /* Enable monitoring */
> + iowrite32be(TMR_ALL, &data->regs->tmr);
> + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(qoriq_tmu_pm_ops,
> + qoriq_tmu_suspend, qoriq_tmu_resume);
> +
> +static const struct of_device_id qoriq_tmu_match[] = {
> + { .compatible = "fsl,qoriq-tmu", },
> + {},
> +};
Binding?
> +
> +static struct platform_driver qoriq_tmu = {
> + .driver = {
> + .name = "qoriq_thermal",
> + .pm = &qoriq_tmu_pm_ops,
> + .of_match_table = qoriq_tmu_match,
> + },
Why the tabs after ".name"?
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V3] thermal: qoriq: Add thermal management support
2015-09-24 22:09 ` Scott Wood
@ 2015-09-25 3:09 ` Hongtao Jia
2015-09-25 5:12 ` Scott Wood
2015-11-04 19:24 ` Eduardo Valentin
0 siblings, 2 replies; 8+ messages in thread
From: Hongtao Jia @ 2015-09-25 3:09 UTC (permalink / raw)
To: Scott Wood; +Cc: edubezval, linux-pm, linuxppc-dev
DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIx
DQo+IFNlbnQ6IEZyaWRheSwgU2VwdGVtYmVyIDI1LCAyMDE1IDY6MTAgQU0NCj4gVG86IEppYSBI
b25ndGFvLUIzODk1MQ0KPiBDYzogZWR1YmV6dmFsQGdtYWlsLmNvbTsgbGludXgtcG1Admdlci5r
ZXJuZWwub3JnOyBsaW51eHBwYy0NCj4gZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDog
UmU6IFtQQVRDSCBWM10gdGhlcm1hbDogcW9yaXE6IEFkZCB0aGVybWFsIG1hbmFnZW1lbnQgc3Vw
cG9ydA0KPiANCj4gT24gV2VkLCAyMDE1LTA5LTIzIGF0IDE2OjI4ICswODAwLCBKaWEgSG9uZ3Rh
byB3cm90ZToNCj4gPiBUaGlzIGRyaXZlciBhZGQgdGhlcm1hbCBtYW5hZ2VtZW50IHN1cHBvcnQg
YnkgZW5hYmxpbmcgVE1VIChUaGVybWFsDQo+ID4gTW9uaXRvcmluZyBVbml0KSBvbiBRb3JJUSBw
bGF0Zm9ybS4NCj4gPg0KPiA+IEl0J3MgYmFzZWQgb24gdGhlcm1hbCBvZiBmcmFtZXdvcms6DQo+
ID4gLSBUcmlwIHBvaW50cyBkZWZpbmVkIGluIGRldmljZSB0cmVlLg0KPiA+IC0gQ3B1ZnJlcSBh
cyBjb29saW5nIGRldmljZSByZWdpc3RlcmVkIGluIHFvcmlxIGNwdWZyZXEgZHJpdmVyLg0KPiAN
Cj4gSSBkb24ndCBzZWUgYW55IGNvb2xpbmcgZGV2aWNlIHJlZ2lzdGVyZWQgaW4gdGhlIHFvcmlx
IGNwdWZyZXEgZHJpdmVyLg0KPiBJcyB0aGlzIGRlcGVuZGVudCBvbiBzb21lIG90aGVyIHBhdGNo
Pw0KDQpJdCdzIG5vdCBkZXBlbmQgb24gYW55IHBhdGNoLiBCdXQgSSBzYXcgeW91ciBwYXRjaCBi
ZWxvdzoNCltQQVRDSCB2MyA1LzVdIGNwdWZyZXE6IHFvcmlxOiBEb24ndCBsb29rIGF0IGNsb2Nr
IGltcGxlbWVudGF0aW9uIGRldGFpbHMNClNvIEkgaG9sZCBteSBwYXRjaCB3YWl0aW5nIGZvciB5
b3VyIHBhdGNoIG1lcmdlZCBvciB0aGVyZSB3aWxsIGJlIGNvbmZsaWN0Lg0KDQpJIGNvdWxkIHNl
bmQgaXQgb3V0IHRvbyBpZiB5b3UgYXJlIGZpbmUgd2l0aCBpdC4NCg0KPiANCj4gPg0KPiA+IFNp
Z25lZC1vZmYtYnk6IEppYSBIb25ndGFvIDxob25ndGFvLmppYUBmcmVlc2NhbGUuY29tPg0KPiA+
IC0tLQ0KPiA+IFYzOiBVc2luZyB0aGVybWFsIG9mIGZyYW1ld29yay4NCj4gPg0KPiA+ICBkcml2
ZXJzL3RoZXJtYWwvS2NvbmZpZyAgICAgICAgIHwgIDExICsrDQo+ID4gIGRyaXZlcnMvdGhlcm1h
bC9NYWtlZmlsZSAgICAgICAgfCAgIDEgKw0KPiA+ICBkcml2ZXJzL3RoZXJtYWwvcW9yaXFfdGhl
cm1hbC5jIHwgMjY3DQo+ID4gKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
Kw0KPiA+ICAzIGZpbGVzIGNoYW5nZWQsIDI3OSBpbnNlcnRpb25zKCspDQo+ID4gIGNyZWF0ZSBt
b2RlIDEwMDY0NCBkcml2ZXJzL3RoZXJtYWwvcW9yaXFfdGhlcm1hbC5jDQo+ID4NCj4gPiBkaWZm
IC0tZ2l0IGEvZHJpdmVycy90aGVybWFsL0tjb25maWcgYi9kcml2ZXJzL3RoZXJtYWwvS2NvbmZp
ZyBpbmRleA0KPiA+IDAzOTAwNDQuLmM5MTA0MWIgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy90
aGVybWFsL0tjb25maWcNCj4gPiArKysgYi9kcml2ZXJzL3RoZXJtYWwvS2NvbmZpZw0KPiA+IEBA
IC0xODAsNiArMTgwLDE3IEBAIGNvbmZpZyBJTVhfVEhFUk1BTA0KPiA+ICAgICAgICAgY3B1ZnJl
cSBpcyB1c2VkIGFzIHRoZSBjb29saW5nIGRldmljZSB0byB0aHJvdHRsZSBDUFVzIHdoZW4gdGhl
DQo+ID4gICAgICAgICBwYXNzaXZlIHRyaXAgaXMgY3Jvc3NlZC4NCj4gPg0KPiA+ICtjb25maWcg
UU9SSVFfVEhFUk1BTA0KPiA+ICsgICAgIHRyaXN0YXRlICJGcmVlc2NhbGUgUW9ySVEgVGhlcm1h
bCBNb25pdG9yaW5nIFVuaXQiDQo+ID4gKyAgICAgZGVwZW5kcyBvbiBDUFVfVEhFUk1BTA0KPiA+
ICsgICAgIGRlcGVuZHMgb24gVEhFUk1BTF9PRg0KPiA+ICsgICAgIGRlZmF1bHQgbg0KPiA+ICsg
ICAgIGhlbHANCj4gPiArICAgICAgIEVuYWJsZSB0aGVybWFsIG1hbmFnZW1lbnQgYmFzZWQgb24g
RnJlZXNjYWxlIFFvcklRIFRoZXJtYWwNCj4gTW9uaXRvcmluZw0KPiA+ICsgICAgICAgVW5pdCAo
VE1VKS4gSXQgc3VwcG9ydHMgb25lIGNyaXRpY2FsIHRyaXAgcG9pbnQgYW5kIG9uZSBwYXNzaXZl
DQo+IHRyaXANCj4gPiArICAgICAgIHBvaW50LiBUaGUgY3B1ZnJlcSBpcyB1c2VkIGFzIHRoZSBj
b29saW5nIGRldmljZSB0byB0aHJvdHRsZQ0KPiBDUFVzIHdoZW4NCj4gPiArICAgICAgIHRoZSBw
YXNzaXZlIHRyaXAgaXMgY3Jvc3NlZC4NCj4gDQo+ICJkZWZhdWx0IG4iIGlzIHVubmVjZXNzYXJ5
IC0tIG4gaXMgYWxyZWFkeSB0aGUgZGVmYXVsdC4NCg0KUmlnaHQuDQoNCj4gDQo+IFdoZXJlIGlz
IHRoZSBpbnRlcmFjdGlvbiBiZXR3ZWVuIHRoaXMgZHJpdmVyIGFuZCBjcHVmcmVxPw0KDQpJdCdz
IGFsbCBpbiBjcHVmcmVxIHBhdGNoIEkgbWVudGlvbmVkIGFib3ZlLg0KDQo+IA0KPiA+IGNvbmZp
ZyBTUEVBUl9USEVSTUFMDQo+ID4gICAgICAgYm9vbCAiU1BFQXIgdGhlcm1hbCBzZW5zb3IgZHJp
dmVyIg0KPiA+ICAgICAgIGRlcGVuZHMgb24gUExBVF9TUEVBUg0KPiA+IGRpZmYgLS1naXQgYS9k
cml2ZXJzL3RoZXJtYWwvTWFrZWZpbGUgYi9kcml2ZXJzL3RoZXJtYWwvTWFrZWZpbGUgaW5kZXgN
Cj4gPiAyNmYxNjA4Li5lNTVkNzAzIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvdGhlcm1hbC9N
YWtlZmlsZQ0KPiA+ICsrKyBiL2RyaXZlcnMvdGhlcm1hbC9NYWtlZmlsZQ0KPiA+IEBAIC0zMyw2
ICszMyw3IEBAIG9iai0kKENPTkZJR19ET1ZFX1RIRVJNQUwpICAgICAgICAgICs9IGRvdmVfdGhl
cm1hbC5vDQo+ID4gIG9iai0kKENPTkZJR19EQjg1MDBfVEhFUk1BTCkgKz0gZGI4NTAwX3RoZXJt
YWwubw0KPiA+ICBvYmotJChDT05GSUdfQVJNQURBX1RIRVJNQUwpICs9IGFybWFkYV90aGVybWFs
Lm8NCj4gPiAgb2JqLSQoQ09ORklHX0lNWF9USEVSTUFMKSAgICArPSBpbXhfdGhlcm1hbC5vDQo+
ID4gK29iai0kKENPTkZJR19RT1JJUV9USEVSTUFMKSAgKz0gcW9yaXFfdGhlcm1hbC5vDQo+ID4g
IG9iai0kKENPTkZJR19EQjg1MDBfQ1BVRlJFUV9DT09MSU5HKSArPSBkYjg1MDBfY3B1ZnJlcV9j
b29saW5nLm8NCj4gPiAgb2JqLSQoQ09ORklHX0lOVEVMX1BPV0VSQ0xBTVApICAgICAgICs9IGlu
dGVsX3Bvd2VyY2xhbXAubw0KPiA+ICBvYmotJChDT05GSUdfWDg2X1BLR19URU1QX1RIRVJNQUwp
ICAgKz0geDg2X3BrZ190ZW1wX3RoZXJtYWwubw0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3Ro
ZXJtYWwvcW9yaXFfdGhlcm1hbC5jDQo+ID4gYi9kcml2ZXJzL3RoZXJtYWwvcW9yaXFfdGhlcm1h
bC5jIG5ldyBmaWxlIG1vZGUgMTAwNjQ0IGluZGV4DQo+ID4gMDAwMDAwMC4uN2MyYTMyNjENCj4g
PiAtLS0gL2Rldi9udWxsDQo+ID4gKysrIGIvZHJpdmVycy90aGVybWFsL3FvcmlxX3RoZXJtYWwu
Yw0KPiA+IEBAIC0wLDAgKzEsMjY3IEBADQo+ID4gKy8qDQo+ID4gKyAqIENvcHlyaWdodCAyMDE1
IEZyZWVzY2FsZSBTZW1pY29uZHVjdG9yLCBJbmMuDQo+ID4gKyAqDQo+ID4gKyAqIFRoaXMgcHJv
Z3JhbSBpcyBmcmVlIHNvZnR3YXJlOyB5b3UgY2FuIHJlZGlzdHJpYnV0ZSBpdCBhbmQvb3INCj4g
PiArbW9kaWZ5IGl0DQo+ID4gKyAqIHVuZGVyIHRoZSB0ZXJtcyBhbmQgY29uZGl0aW9ucyBvZiB0
aGUgR05VIEdlbmVyYWwgUHVibGljIExpY2Vuc2UsDQo+ID4gKyAqIHZlcnNpb24gMiwgYXMgcHVi
bGlzaGVkIGJ5IHRoZSBGcmVlIFNvZnR3YXJlIEZvdW5kYXRpb24uDQo+ID4gKyAqDQo+ID4gKyAq
IFRoaXMgcHJvZ3JhbSBpcyBkaXN0cmlidXRlZCBpbiB0aGUgaG9wZSBpdCB3aWxsIGJlIHVzZWZ1
bCwgYnV0DQo+ID4gK1dJVEhPVVQNCj4gPiArICogQU5ZIFdBUlJBTlRZOyB3aXRob3V0IGV2ZW4g
dGhlIGltcGxpZWQgd2FycmFudHkgb2YgTUVSQ0hBTlRBQklMSVRZDQo+ID4gK29yDQo+ID4gKyAq
IEZJVE5FU1MgRk9SIEEgUEFSVElDVUxBUiBQVVJQT1NFLiAgU2VlIHRoZSBHTlUgR2VuZXJhbCBQ
dWJsaWMNCj4gPiArTGljZW5zZQ0KPiA+IGZvcg0KPiA+ICsgKiBtb3JlIGRldGFpbHMuDQo+ID4g
KyAqDQo+ID4gKyAqLw0KPiA+ICsNCj4gPiArLyoNCj4gPiArICogQmFzZWQgb24gRnJlZXNjYWxl
IFFvcklRIFRoZXJtYWwgTW9uaXRvcmluZyBVbml0IChUTVUpICAqLw0KPiANCj4gV2hhdCBkb2Vz
IHRoaXMgY29tbWVudCBtZWFuPyAgVGhpcyAqaXMqIHRoZSAiRnJlZXNjYWxlIFFvcklRIFRoZXJt
YWwNCj4gTW9uaXRvcmluZyBVbml0IiBkcml2ZXIuDQoNCkkgbWVhbiB0aGVybWFsIG1hbmFnZW1l
bnQgYmFzZWQgb24gdGhlIG1vbml0b3IgVE1VLg0KSSBjb3VsZCByZW1vdmUgdGhpcyBjb21tZW50
IHRob3VnaC4NCg0KPiANCj4gPiArI2luY2x1ZGUgPGxpbnV4L2NwdWZyZXEuaD4NCj4gDQo+IFdo
YXQgZG8geW91IHVzZSBmcm9tIHRoaXMgaGVhZGVyPw0KDQpTb3JyeSwgZm9yZ2V0IHRvIHJlbW92
ZSBpdCBmcm9tIGxhc3QgdmVyc2lvbiBpbiB3aGljaCBjb29saW5nIGRldmljZXMgYXJlDQpyZWdp
c3RlcmVkIGluIHRoZXJtYWwgZHJpdmVyIGluc3RlYWQgb2YgY3B1ZnJlcSBkcml2ZXIuDQoNCj4g
DQo+ID4gKyNpbmNsdWRlIDxsaW51eC9tb2R1bGUuaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L3Bs
YXRmb3JtX2RldmljZS5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgvZXJyLmg+DQo+ID4gKyNpbmNs
dWRlIDxsaW51eC9pby5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgvb2YuaD4NCj4gPiArI2luY2x1
ZGUgPGxpbnV4L29mX2FkZHJlc3MuaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L3RoZXJtYWwuaD4N
Cj4gPiArDQo+ID4gKyNpbmNsdWRlICJ0aGVybWFsX2NvcmUuaCINCj4gPiArDQo+ID4gKyNkZWZp
bmUgU0lURVNfTUFYICAgIDE2DQo+ID4gKw0KPiA+ICsvKg0KPiA+ICsgKiBRb3JJUSBUTVUgUmVn
aXN0ZXJzDQo+ID4gKyAqLw0KPiA+ICtzdHJ1Y3QgcW9yaXFfdG11X3NpdGVfcmVncyB7DQo+ID4g
KyAgICAgX19iZTMyIHRyaXRzcjsgICAgICAgICAgLyogSW1tZWRpYXRlIFRlbXBlcmF0dXJlIFNp
dGUgUmVnaXN0ZXIgKi8NCj4gPiArICAgICBfX2JlMzIgdHJhdHNyOyAgICAgICAgICAvKiBBdmVy
YWdlIFRlbXBlcmF0dXJlIFNpdGUgUmVnaXN0ZXIgKi8NCj4gPiArICAgICB1OCByZXMwWzB4OF07
DQo+ID4gK30gX19wYWNrZWQ7DQo+ID4gKw0KPiA+ICtzdHJ1Y3QgcW9yaXFfdG11X3JlZ3Mgew0K
PiA+ICsgICAgIF9fYmUzMiB0bXI7ICAgICAgICAgICAgIC8qIE1vZGUgUmVnaXN0ZXIgKi8NCj4g
PiArI2RlZmluZSBUTVJfRElTQUJMRSAgMHgwDQo+ID4gKyNkZWZpbmUgVE1SX01FICAgICAgICAg
ICAgICAgMHg4MDAwMDAwMA0KPiA+ICsjZGVmaW5lIFRNUl9BTFBGICAgICAweDBjMDAwMDAwDQo+
ID4gKyNkZWZpbmUgVE1SX01TSVRFICAgIDB4MDAwMDgwMDAgICAgICAvKiBDb3JlIHRlbXBlcmF0
dXJlIHNpdGUgKi8NCj4gPiArI2RlZmluZSBUTVJfQUxMICAgICAgICAgICAgICAoVE1SX01FIHwg
VE1SX0FMUEYgfCBUTVJfTVNJVEUpDQo+ID4gKyAgICAgX19iZTMyIHRzcjsgICAgICAgICAgICAg
LyogU3RhdHVzIFJlZ2lzdGVyICovDQo+ID4gKyAgICAgX19iZTMyIHRtdG1pcjsgICAgICAgICAg
LyogVGVtcGVyYXR1cmUgbWVhc3VyZW1lbnQgaW50ZXJ2YWwNCj4gUmVnaXN0ZXIgKi8NCj4gPiAr
I2RlZmluZSBUTVRNSVJfREVGQVVMVCAgICAgICAweDAwMDAwMDA3DQo+ID4gKyAgICAgdTggcmVz
MFsweDE0XTsNCj4gPiArICAgICBfX2JlMzIgdGllcjsgICAgICAgICAgICAvKiBJbnRlcnJ1cHQg
RW5hYmxlIFJlZ2lzdGVyICovDQo+ID4gKyNkZWZpbmUgVElFUl9ESVNBQkxFIDB4MA0KPiA+ICsg
ICAgIF9fYmUzMiB0aWRyOyAgICAgICAgICAgIC8qIEludGVycnVwdCBEZXRlY3QgUmVnaXN0ZXIg
Ki8NCj4gPiArICAgICBfX2JlMzIgdGlzY3I7ICAgICAgICAgICAvKiBJbnRlcnJ1cHQgU2l0ZSBD
YXB0dXJlIFJlZ2lzdGVyICovDQo+ID4gKyAgICAgX19iZTMyIHRpY3NjcjsgICAgICAgICAgLyog
SW50ZXJydXB0IENyaXRpY2FsIFNpdGUgQ2FwdHVyZQ0KPiBSZWdpc3RlciAqLw0KPiA+ICsgICAg
IHU4IHJlczFbMHgxMF07DQo+ID4gKyAgICAgX19iZTMyIHRtaHRjcmg7ICAgICAgICAgLyogSGln
aCBUZW1wZXJhdHVyZSBDYXB0dXJlIFJlZ2lzdGVyICovDQo+ID4gKyAgICAgX19iZTMyIHRtaHRj
cmw7ICAgICAgICAgLyogTG93IFRlbXBlcmF0dXJlIENhcHR1cmUgUmVnaXN0ZXIgKi8NCj4gPiAr
ICAgICB1OCByZXMyWzB4OF07DQo+ID4gKyAgICAgX19iZTMyIHRtaHRpdHI7ICAgICAgICAgLyog
SGlnaCBUZW1wZXJhdHVyZSBJbW1lZGlhdGUgVGhyZXNob2xkDQo+ICovDQo+ID4gKyAgICAgX19i
ZTMyIHRtaHRhdHI7ICAgICAgICAgLyogSGlnaCBUZW1wZXJhdHVyZSBBdmVyYWdlIFRocmVzaG9s
ZCAqLw0KPiA+ICsgICAgIF9fYmUzMiB0bWh0YWN0cjsgICAgICAgIC8qIEhpZ2ggVGVtcGVyYXR1
cmUgQXZlcmFnZSBDcml0DQo+IFRocmVzaG9sZCAqLw0KPiA+ICsgICAgIHU4IHJlczNbMHgyNF07
DQo+ID4gKyAgICAgX19iZTMyIHR0Y2ZncjsgICAgICAgICAgLyogVGVtcGVyYXR1cmUgQ29uZmln
dXJhdGlvbiBSZWdpc3RlciAqLw0KPiA+ICsgICAgIF9fYmUzMiB0c2NmZ3I7ICAgICAgICAgIC8q
IFNlbnNvciBDb25maWd1cmF0aW9uIFJlZ2lzdGVyICovDQo+ID4gKyAgICAgdTggcmVzNFsweDc4
XTsNCj4gPiArICAgICBzdHJ1Y3QgcW9yaXFfdG11X3NpdGVfcmVncyBzaXRlW1NJVEVTX01BWF07
DQo+ID4gKyAgICAgdTggcmVzNVsweDlmOF07DQo+ID4gKyAgICAgX19iZTMyIGlwYnJyMDsgICAg
ICAgICAgLyogSVAgQmxvY2sgUmV2aXNpb24gUmVnaXN0ZXIgMCAqLw0KPiA+ICsgICAgIF9fYmUz
MiBpcGJycjE7ICAgICAgICAgIC8qIElQIEJsb2NrIFJldmlzaW9uIFJlZ2lzdGVyIDEgKi8NCj4g
PiArICAgICB1OCByZXM2WzB4MzEwXTsNCj4gPiArICAgICBfX2JlMzIgdHRyMGNyOyAgICAgICAg
ICAvKiBUZW1wZXJhdHVyZSBSYW5nZSAwIENvbnRyb2wgUmVnaXN0ZXINCj4gKi8NCj4gPiArICAg
ICBfX2JlMzIgdHRyMWNyOyAgICAgICAgICAvKiBUZW1wZXJhdHVyZSBSYW5nZSAxIENvbnRyb2wg
UmVnaXN0ZXINCj4gKi8NCj4gPiArICAgICBfX2JlMzIgdHRyMmNyOyAgICAgICAgICAvKiBUZW1w
ZXJhdHVyZSBSYW5nZSAyIENvbnRyb2wgUmVnaXN0ZXINCj4gKi8NCj4gPiArICAgICBfX2JlMzIg
dHRyM2NyOyAgICAgICAgICAvKiBUZW1wZXJhdHVyZSBSYW5nZSAzIENvbnRyb2wgUmVnaXN0ZXIN
Cj4gKi8NCj4gPiArfTsNCj4gPiArDQo+ID4gKy8qDQo+ID4gKyAqIFRoZXJtYWwgem9uZSBkYXRh
DQo+ID4gKyAqLw0KPiA+ICtzdHJ1Y3QgcW9yaXFfdG11X2RhdGEgew0KPiA+ICsgICAgIHN0cnVj
dCB0aGVybWFsX3pvbmVfZGV2aWNlICp0ejsNCj4gPiArICAgICBzdHJ1Y3QgcW9yaXFfdG11X3Jl
Z3MgX19pb21lbSAqcmVnczsgfTsNCj4gPiArDQo+IA0KPiA+ICtzdGF0aWMgaW50IHRtdV9nZXRf
dGVtcCh2b2lkICpwLCBpbnQgKnRlbXApIHsNCj4gPiArICAgICB1OCB2YWw7DQo+ID4gKyAgICAg
c3RydWN0IHFvcmlxX3RtdV9kYXRhICpkYXRhID0gcDsNCj4gPiArDQo+ID4gKyAgICAgdmFsID0g
aW9yZWFkMzJiZSgmZGF0YS0+cmVncy0+c2l0ZVswXS50cml0c3IpOw0KPiA+ICsgICAgICp0ZW1w
ID0gKGludCl2YWwgKiAxMDAwOw0KPiANCj4gV2h5IGRvbid0IHlvdSBkZWNsYXJlIHZhbCBhcyBp
bnQgaW4gdGhlIGZpcnN0IHBsYWNlPw0KDQpJdCdzIGEgMzJiaXQgcmVnaXN0ZXIuDQpPbmx5IHRo
ZSBsZWFzdCBzaWduaWZpY2FudCA4IGJpdHMgcmVwcmVzZW50IHRoZSB0ZW1wZXJhdHVyZS4NClRo
ZSBtb3N0IHNpZ25pZmljYW50IGJpdCBzaG93cyB0aGUgdmFsaWRuZXNzIG9mIHRoZSB2YWx1ZS4N
CkkgdXNlIHU4IHR5cGUgdG8gcmVtb3ZlIHRoZSByZXN0IDI0Yml0cyBpbmZsdWVuY2UuDQoNCj4g
DQo+ID4gKyAgICAgZm9yIChpID0gMDsgaSA8IGxlbjsgaSArPSA4LCBjYWxpYnJhdGlvbiArPSAy
KSB7DQo+ID4gKyAgICAgICAgICAgICB2YWwgPSAoaW50KW9mX3JlYWRfbnVtYmVyKGNhbGlicmF0
aW9uLCAxKTsNCj4gPiArICAgICAgICAgICAgIGlvd3JpdGUzMmJlKHZhbCwgJmRhdGEtPnJlZ3Mt
PnR0Y2Zncik7DQo+ID4gKyAgICAgICAgICAgICB2YWwgPSAoaW50KW9mX3JlYWRfbnVtYmVyKGNh
bGlicmF0aW9uICsgMSwgMSk7DQo+ID4gKyAgICAgICAgICAgICBpb3dyaXRlMzJiZSh2YWwsICZk
YXRhLT5yZWdzLT50c2NmZ3IpOw0KPiANCj4gVW5uZWNlc3NhcnkgY2FzdHMuDQoNCk9rLg0KDQo+
IA0KPiA+ICsgICAgIH0NCj4gPiArDQo+ID4gKyAgICAgcmV0dXJuIDA7DQo+ID4gK30NCj4gPiAr
DQo+ID4gK3N0YXRpYyB2b2lkIHFvcmlxX3RtdV9pbml0X2RldmljZShzdHJ1Y3QgcW9yaXFfdG11
X2RhdGEgKmRhdGEpIHsNCj4gPiArICAgICAvKiBEaXNhYmxlIGludGVycnVwdCwgdXNpbmcgcG9s
bGluZyBpbnN0ZWFkICovDQo+ID4gKyAgICAgaW93cml0ZTMyYmUoVElFUl9ESVNBQkxFLCAmZGF0
YS0+cmVncy0+dGllcik7DQo+ID4gKw0KPiA+ICsgICAgIC8qIFNldCB1cGRhdGVfaW50ZXJ2YWwg
Ki8NCj4gPiArICAgICBpb3dyaXRlMzJiZShUTVRNSVJfREVGQVVMVCwgJmRhdGEtPnJlZ3MtPnRt
dG1pcik7DQo+ID4gKw0KPiA+ICsgICAgIC8qIEVuYWJsZSBtb25pdG9yaW5nICovDQo+ID4gKyAg
ICAgaW93cml0ZTMyYmUoVE1SX0FMTCwgJmRhdGEtPnJlZ3MtPnRtcik7IH0NCj4gPiArDQo+ID4g
K3N0YXRpYyBzdHJ1Y3QgdGhlcm1hbF96b25lX29mX2RldmljZV9vcHMgdG11X3R6X29wcyA9IHsN
Cj4gPiArICAgICAuZ2V0X3RlbXAgPSB0bXVfZ2V0X3RlbXAsDQo+ID4gK307DQo+ID4gKw0KPiA+
ICtzdGF0aWMgaW50IHFvcmlxX3RtdV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2
KSB7DQo+ID4gKyAgICAgaW50IHJldDsNCj4gPiArICAgICBjb25zdCBzdHJ1Y3QgdGhlcm1hbF90
cmlwICp0cmlwOw0KPiA+ICsgICAgIHN0cnVjdCBxb3JpcV90bXVfZGF0YSAqZGF0YTsNCj4gPiAr
DQo+ID4gKyAgICAgaWYgKCFwZGV2LT5kZXYub2Zfbm9kZSkgew0KPiA+ICsgICAgICAgICAgICAg
ZGV2X2VycigmcGRldi0+ZGV2LCAiRGV2aWNlIE9GLU5vZGUgaXMgTlVMTCIpOw0KPiA+ICsgICAg
ICAgICAgICAgcmV0dXJuIC1FRkFVTFQ7DQo+ID4gKyAgICAgfQ0KPiANCj4gRUZBVUxUIGlzIGZv
ciBiYWQgdXNlciBhZGRyZXNzZXMgYW5kIGlzIG5vdCBhcHByb3ByaWF0ZSBoZXJlLg0KDQpJIHdp
bGwgY2hhbmdlIGl0IHRvIEVOT0RFVi4NCg0KPiANCj4gPiArDQo+ID4gKyAgICAgZGF0YSA9IGRl
dm1fa3phbGxvYygmcGRldi0+ZGV2LCBzaXplb2Yoc3RydWN0IHFvcmlxX3RtdV9kYXRhKSwNCj4g
PiArICAgICAgICAgICAgICAgICAgICAgICAgIEdGUF9LRVJORUwpOw0KPiA+ICsgICAgIGlmICgh
ZGF0YSkNCj4gPiArICAgICAgICAgICAgIHJldHVybiAtRU5PTUVNOw0KPiA+ICsNCj4gPiArICAg
ICBwbGF0Zm9ybV9zZXRfZHJ2ZGF0YShwZGV2LCBkYXRhKTsNCj4gPiArICAgICBkYXRhLT5yZWdz
ID0gb2ZfaW9tYXAocGRldi0+ZGV2Lm9mX25vZGUsIDApOw0KPiA+ICsNCj4gPiArICAgICBpZiAo
IWRhdGEtPnJlZ3MpIHsNCj4gPiArICAgICAgICAgICAgIGRldl9lcnIoJnBkZXYtPmRldiwgIkZh
aWxlZCB0byBnZXQgbWVtb3J5IHJlZ2lvblxuIik7DQo+ID4gKyAgICAgICAgICAgICByZXQgPSAt
RU5PREVWOw0KPiA+ICsgICAgICAgICAgICAgZ290byBlcnJfaW9tYXA7DQo+ID4gKyAgICAgfQ0K
PiA+ICsNCj4gPiArICAgICByZXQgPSBxb3JpcV90bXVfY2FsaWJyYXRpb24ocGRldik7ICAgICAg
LyogVE1VIGNhbGlicmF0aW9uICovDQo+ID4gKyAgICAgaWYgKHJldCA8IDApIHsNCj4gPiArICAg
ICAgICAgICAgIGRldl9lcnIoJnBkZXYtPmRldiwgIlRNVSBjYWxpYnJhdGlvbiBmYWlsZWQuXG4i
KTsNCj4gPiArICAgICAgICAgICAgIHJldCA9IC1FTk9ERVY7DQo+ID4gKyAgICAgICAgICAgICBn
b3RvIGVycl9pb21hcDsNCj4gPiArICAgICB9DQo+IA0KPiBUaGF0IGZ1bmN0aW9uIHJldHVybnMg
bmVnYXRpdmUgd2hlbiBkZXZpY2UgdHJlZSBwcm9wZXJ0aWVzIGFyZSBtaXNzaW5nLA0KPiBub3Qg
d2hlbiBhIGNhbGlicmF0aW9uIHByb2NlZHVyZSBmYWlscy4NCg0KV2hhdCdzIHlvdXIgc3VnZ2Vz
dGlvbiBoZXJlIHRvIHJldHVybiB0aGVuPw0KDQo+IA0KPiA+ICsNCj4gPiArICAgICBxb3JpcV90
bXVfaW5pdF9kZXZpY2UoZGF0YSk7ICAgIC8qIFRNVSBpbml0aWFsaXphdGlvbiAqLw0KPiA+ICsN
Cj4gPiArICAgICBkYXRhLT50eiA9IHRoZXJtYWxfem9uZV9vZl9zZW5zb3JfcmVnaXN0ZXIoJnBk
ZXYtPmRldiwgMCwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICBkYXRhLCAmdG11
X3R6X29wcyk7DQo+ID4gKyAgICAgaWYgKElTX0VSUihkYXRhLT50eikpIHsNCj4gPiArICAgICAg
ICAgICAgIHJldCA9IFBUUl9FUlIoZGF0YS0+dHopOw0KPiA+ICsgICAgICAgICAgICAgZGV2X2Vy
cigmcGRldi0+ZGV2LA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAiRmFpbGVkIHRvIHJlZ2lz
dGVyIHRoZXJtYWwgem9uZSBkZXZpY2UgJWRcbiIsDQo+IHJldCk7DQo+ID4gKyAgICAgICAgICAg
ICBnb3RvIGVycl90aGVybWFsOw0KPiA+ICsgICAgIH0NCj4gPiArDQo+ID4gKyAgICAgdHJpcCA9
IG9mX3RoZXJtYWxfZ2V0X3RyaXBfcG9pbnRzKGRhdGEtPnR6KTsNCj4gPiArDQo+ID4gKyAgICAg
ZGF0YS0+dHotPm9wcy0+c2V0X21vZGUoZGF0YS0+dHosIFRIRVJNQUxfREVWSUNFX0VOQUJMRUQp
Ow0KPiANCj4gRG9lc24ndCB0aGVybWFsX3pvbmVfb2Zfc2Vuc29yX3JlZ2lzdGVyKCkgYWxyZWFk
eSBjYWxsIG9wcy0+c2V0X21vZGUgd2l0aA0KPiBURVJNQUxfREVWSUNFX0VOQUJMRUQ/DQoNCllv
dSBhcmUgcmlnaHQuDQpJIHJlZmVyZW5jZWQgdGhlIGNvZGUgZnJvbSBvdGhlciBwbGF0Zm9ybSBh
bmQgZGlkIG5vdCBub3RpY2UgdGhpcy4NClRoYW5rcy4NCg0KPiANCj4gPiArDQo+ID4gKyAgICAg
cmV0dXJuIDA7DQo+ID4gKw0KPiA+ICtlcnJfdGhlcm1hbDoNCj4gPiArICAgICBpb3VubWFwKGRh
dGEtPnJlZ3MpOw0KPiA+ICsNCj4gPiArZXJyX2lvbWFwOg0KPiA+ICsgICAgIHBsYXRmb3JtX3Nl
dF9kcnZkYXRhKHBkZXYsIE5VTEwpOw0KPiA+ICsgICAgIGRldm1fa2ZyZWUoJnBkZXYtPmRldiwg
ZGF0YSk7DQo+ID4gKw0KPiA+ICsgICAgIHJldHVybiByZXQ7DQo+ID4gK30NCj4gPiArDQo+ID4g
K3N0YXRpYyBpbnQgcW9yaXFfdG11X3JlbW92ZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2
KSB7DQo+ID4gKyAgICAgc3RydWN0IHFvcmlxX3RtdV9kYXRhICpkYXRhID0gcGxhdGZvcm1fZ2V0
X2RydmRhdGEocGRldik7DQo+ID4gKw0KPiA+ICsgICAgIC8qIERpc2FibGUgbW9uaXRvcmluZyAq
Lw0KPiA+ICsgICAgIGlvd3JpdGUzMmJlKFRNUl9ESVNBQkxFLCAmZGF0YS0+cmVncy0+dG1yKTsN
Cj4gPiArDQo+ID4gKyAgICAgdGhlcm1hbF96b25lX29mX3NlbnNvcl91bnJlZ2lzdGVyKCZwZGV2
LT5kZXYsIGRhdGEtPnR6KTsNCj4gPiArICAgICBpb3VubWFwKGRhdGEtPnJlZ3MpOw0KPiA+ICsN
Cj4gPiArICAgICBwbGF0Zm9ybV9zZXRfZHJ2ZGF0YShwZGV2LCBOVUxMKTsNCj4gPiArICAgICBk
ZXZtX2tmcmVlKCZwZGV2LT5kZXYsIGRhdGEpOw0KPiA+ICsNCj4gPiArICAgICByZXR1cm4gMDsN
Cj4gPiArfQ0KPiA+ICsNCj4gPiArI2lmZGVmIENPTkZJR19QTV9TTEVFUA0KPiA+ICtzdGF0aWMg
aW50IHFvcmlxX3RtdV9zdXNwZW5kKHN0cnVjdCBkZXZpY2UgKmRldikgew0KPiA+ICsgICAgIHN0
cnVjdCBxb3JpcV90bXVfZGF0YSAqZGF0YSA9IGRldl9nZXRfZHJ2ZGF0YShkZXYpOw0KPiA+ICsN
Cj4gPiArICAgICAvKiBEaXNhYmxlIG1vbml0b3JpbmcgKi8NCj4gPiArICAgICBpb3dyaXRlMzJi
ZShUTVJfRElTQUJMRSwgJmRhdGEtPnJlZ3MtPnRtcik7DQo+ID4gKyAgICAgZGF0YS0+dHotPm9w
cy0+c2V0X21vZGUoZGF0YS0+dHosIFRIRVJNQUxfREVWSUNFX0RJU0FCTEVEKTsNCj4gPiArDQo+
ID4gKyAgICAgcmV0dXJuIDA7DQo+ID4gK30NCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgcW9yaXFf
dG11X3Jlc3VtZShzdHJ1Y3QgZGV2aWNlICpkZXYpIHsNCj4gPiArICAgICBzdHJ1Y3QgcW9yaXFf
dG11X2RhdGEgKmRhdGEgPSBkZXZfZ2V0X2RydmRhdGEoZGV2KTsNCj4gPiArDQo+ID4gKyAgICAg
LyogRW5hYmxlIG1vbml0b3JpbmcgKi8NCj4gPiArICAgICBpb3dyaXRlMzJiZShUTVJfQUxMLCAm
ZGF0YS0+cmVncy0+dG1yKTsNCj4gPiArICAgICBkYXRhLT50ei0+b3BzLT5zZXRfbW9kZShkYXRh
LT50eiwgVEhFUk1BTF9ERVZJQ0VfRU5BQkxFRCk7DQo+ID4gKw0KPiA+ICsgICAgIHJldHVybiAw
Ow0KPiA+ICt9DQo+ID4gKyNlbmRpZg0KPiA+ICsNCj4gPiArc3RhdGljIFNJTVBMRV9ERVZfUE1f
T1BTKHFvcmlxX3RtdV9wbV9vcHMsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICBxb3JpcV90
bXVfc3VzcGVuZCwgcW9yaXFfdG11X3Jlc3VtZSk7DQo+ID4gKw0KPiANCj4gDQo+ID4gK3N0YXRp
YyBjb25zdCBzdHJ1Y3Qgb2ZfZGV2aWNlX2lkIHFvcmlxX3RtdV9tYXRjaFtdID0gew0KPiA+ICsg
ICAgIHsgLmNvbXBhdGlibGUgPSAiZnNsLHFvcmlxLXRtdSIsIH0sDQo+ID4gKyAgICAge30sDQo+
ID4gK307DQo+IA0KPiBCaW5kaW5nPw0KDQpOb3Qgc2VuZCBvdXQgeWV0Lg0KSSBwbGFubmVkIHRv
IHNlbmQgdGhlIHBhdGNoIHdpdGggZGV2aWNlIHRyZWUgcGF0Y2hzZXQgaW5jbHVkaW5nDQphZGRp
bmcgVE1VIG5vZGUgdG8gVDEwNDAvVDEwMjQvTFMxMDIxQSBwbGF0Zm9ybS4NCg0KSXQncyBkZXBl
bmRpbmcgb24gdGhlIGRldmljZSB0cmVlIGZpbGVzIG1vdmluZyBwYXRjaCBJIHNlbnQgb3V0DQpu
b3QgbG9uZyBhZ28uDQoNCj4gDQo+ID4gKw0KPiA+ICtzdGF0aWMgc3RydWN0IHBsYXRmb3JtX2Ry
aXZlciBxb3JpcV90bXUgPSB7DQo+ID4gKyAgICAgLmRyaXZlciA9IHsNCj4gPiArICAgICAgICAg
ICAgIC5uYW1lICAgICAgICAgICA9ICJxb3JpcV90aGVybWFsIiwNCj4gPiArICAgICAgICAgICAg
IC5wbSA9ICZxb3JpcV90bXVfcG1fb3BzLA0KPiA+ICsgICAgICAgICAgICAgLm9mX21hdGNoX3Rh
YmxlID0gcW9yaXFfdG11X21hdGNoLA0KPiA+ICsgICAgIH0sDQo+IA0KPiBXaHkgdGhlIHRhYnMg
YWZ0ZXIgIi5uYW1lIj8NCg0KV2lsbCBpbmRlbnQgdGhlIG90aGVyIHR3by4NCg0KPiANCj4gLVNj
b3R0DQoNCg==
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V3] thermal: qoriq: Add thermal management support
2015-09-25 3:09 ` Hongtao Jia
@ 2015-09-25 5:12 ` Scott Wood
2015-09-25 7:36 ` Hongtao Jia
2015-11-04 19:24 ` Eduardo Valentin
1 sibling, 1 reply; 8+ messages in thread
From: Scott Wood @ 2015-09-25 5:12 UTC (permalink / raw)
To: Jia Hongtao-B38951; +Cc: edubezval, linux-pm, linuxppc-dev
On Thu, 2015-09-24 at 22:09 -0500, Jia Hongtao-B38951 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, September 25, 2015 6:10 AM
> > To: Jia Hongtao-B38951
> > Cc: edubezval@gmail.com; linux-pm@vger.kernel.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH V3] thermal: qoriq: Add thermal management support
> >
> > On Wed, 2015-09-23 at 16:28 +0800, Jia Hongtao wrote:
> > > This driver add thermal management support by enabling TMU (Thermal
> > > Monitoring Unit) on QorIQ platform.
> > >
> > > It's based on thermal of framework:
> > > - Trip points defined in device tree.
> > > - Cpufreq as cooling device registered in qoriq cpufreq driver.
> >
> > I don't see any cooling device registered in the qoriq cpufreq driver.
> > Is this dependent on some other patch?
>
> It's not depend on any patch. But I saw your patch below:
> [PATCH v3 5/5] cpufreq: qoriq: Don't look at clock implementation details
> So I hold my patch waiting for your patch merged or there will be conflict.
>
> I could send it out too if you are fine with it.
Yes, rebase it on that patch and send it. Mention below the --- that you're
depending on that patch, and provide a patchwork/archive link.
> > > +static int tmu_get_temp(void *p, int *temp) {
> > > + u8 val;
> > > + struct qoriq_tmu_data *data = p;
> > > +
> > > + val = ioread32be(&data->regs->site[0].tritsr);
> > > + *temp = (int)val * 1000;
> >
> > Why don't you declare val as int in the first place?
>
> It's a 32bit register.
> Only the least significant 8 bits represent the temperature.
> The most significant bit shows the validness of the value.
> I use u8 type to remove the rest 24bits influence.
That's even worse. Use an explicit & operation to extract the field you're
interested in.
> > > + ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> > > + if (ret < 0) {
> > > + dev_err(&pdev->dev, "TMU calibration failed.\n");
> > > + ret = -ENODEV;
> > > + goto err_iomap;
> > > + }
> >
> > That function returns negative when device tree properties are missing,
> > not when a calibration procedure fails.
>
> What's your suggestion here to return then?
Remove this message and add a message in qoriq_tmu_calibration for each error
condition. Also have qoriq_tmu_calibration pass back a proper error code,
not -1.
> > > +static const struct of_device_id qoriq_tmu_match[] = {
> > > + { .compatible = "fsl,qoriq-tmu", },
> > > + {},
> > > +};
> >
> > Binding?
>
> Not send out yet.
The binding needs to come before the driver that uses it.
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V3] thermal: qoriq: Add thermal management support
2015-09-25 5:12 ` Scott Wood
@ 2015-09-25 7:36 ` Hongtao Jia
0 siblings, 0 replies; 8+ messages in thread
From: Hongtao Jia @ 2015-09-25 7:36 UTC (permalink / raw)
To: Scott Wood; +Cc: edubezval, linux-pm, linuxppc-dev
DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIx
DQo+IFNlbnQ6IEZyaWRheSwgU2VwdGVtYmVyIDI1LCAyMDE1IDE6MTIgUE0NCj4gVG86IEppYSBI
b25ndGFvLUIzODk1MQ0KPiBDYzogZWR1YmV6dmFsQGdtYWlsLmNvbTsgbGludXgtcG1Admdlci5r
ZXJuZWwub3JnOyBsaW51eHBwYy0NCj4gZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDog
UmU6IFtQQVRDSCBWM10gdGhlcm1hbDogcW9yaXE6IEFkZCB0aGVybWFsIG1hbmFnZW1lbnQgc3Vw
cG9ydA0KPiANCj4gT24gVGh1LCAyMDE1LTA5LTI0IGF0IDIyOjA5IC0wNTAwLCBKaWEgSG9uZ3Rh
by1CMzg5NTEgd3JvdGU6DQo+ID4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4g
RnJvbTogV29vZCBTY290dC1CMDc0MjENCj4gPiA+IFNlbnQ6IEZyaWRheSwgU2VwdGVtYmVyIDI1
LCAyMDE1IDY6MTAgQU0NCj4gPiA+IFRvOiBKaWEgSG9uZ3Rhby1CMzg5NTENCj4gPiA+IENjOiBl
ZHViZXp2YWxAZ21haWwuY29tOyBsaW51eC1wbUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLQ0K
PiA+ID4gZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjNd
IHRoZXJtYWw6IHFvcmlxOiBBZGQgdGhlcm1hbCBtYW5hZ2VtZW50DQo+IHN1cHBvcnQNCj4gPiA+
DQo+ID4gPiBPbiBXZWQsIDIwMTUtMDktMjMgYXQgMTY6MjggKzA4MDAsIEppYSBIb25ndGFvIHdy
b3RlOg0KPiA+ID4gPiBUaGlzIGRyaXZlciBhZGQgdGhlcm1hbCBtYW5hZ2VtZW50IHN1cHBvcnQg
YnkgZW5hYmxpbmcgVE1VIChUaGVybWFsDQo+ID4gPiA+IE1vbml0b3JpbmcgVW5pdCkgb24gUW9y
SVEgcGxhdGZvcm0uDQo+ID4gPiA+DQo+ID4gPiA+IEl0J3MgYmFzZWQgb24gdGhlcm1hbCBvZiBm
cmFtZXdvcms6DQo+ID4gPiA+IC0gVHJpcCBwb2ludHMgZGVmaW5lZCBpbiBkZXZpY2UgdHJlZS4N
Cj4gPiA+ID4gLSBDcHVmcmVxIGFzIGNvb2xpbmcgZGV2aWNlIHJlZ2lzdGVyZWQgaW4gcW9yaXEg
Y3B1ZnJlcSBkcml2ZXIuDQo+ID4gPg0KPiA+ID4gSSBkb24ndCBzZWUgYW55IGNvb2xpbmcgZGV2
aWNlIHJlZ2lzdGVyZWQgaW4gdGhlIHFvcmlxIGNwdWZyZXEgZHJpdmVyLg0KPiA+ID4gSXMgdGhp
cyBkZXBlbmRlbnQgb24gc29tZSBvdGhlciBwYXRjaD8NCj4gPg0KPiA+IEl0J3Mgbm90IGRlcGVu
ZCBvbiBhbnkgcGF0Y2guIEJ1dCBJIHNhdyB5b3VyIHBhdGNoIGJlbG93Og0KPiA+IFtQQVRDSCB2
MyA1LzVdIGNwdWZyZXE6IHFvcmlxOiBEb24ndCBsb29rIGF0IGNsb2NrIGltcGxlbWVudGF0aW9u
DQo+IGRldGFpbHMNCj4gPiBTbyBJIGhvbGQgbXkgcGF0Y2ggd2FpdGluZyBmb3IgeW91ciBwYXRj
aCBtZXJnZWQgb3IgdGhlcmUgd2lsbCBiZQ0KPiBjb25mbGljdC4NCj4gPg0KPiA+IEkgY291bGQg
c2VuZCBpdCBvdXQgdG9vIGlmIHlvdSBhcmUgZmluZSB3aXRoIGl0Lg0KPiANCj4gWWVzLCByZWJh
c2UgaXQgb24gdGhhdCBwYXRjaCBhbmQgc2VuZCBpdC4gIE1lbnRpb24gYmVsb3cgdGhlIC0tLSB0
aGF0DQo+IHlvdSdyZQ0KPiBkZXBlbmRpbmcgb24gdGhhdCBwYXRjaCwgYW5kIHByb3ZpZGUgYSBw
YXRjaHdvcmsvYXJjaGl2ZSBsaW5rLg0KDQpPay4gQnV0IEkgbmVlZCByZWJhc2UgYWdhaW4gaWYg
eW91IGhhdmUgZnVydGhlciBjaGFuZ2VzIG9uIHRoYXQgcGF0Y2guDQpBbnl3YXksIHRoaXMgaXMg
bm90IGEgYmlnIHByb2JsZW0gZm9yIG1lLg0KDQo+IA0KPiA+ID4gPiArc3RhdGljIGludCB0bXVf
Z2V0X3RlbXAodm9pZCAqcCwgaW50ICp0ZW1wKSB7DQo+ID4gPiA+ICsgICAgIHU4IHZhbDsNCj4g
PiA+ID4gKyAgICAgc3RydWN0IHFvcmlxX3RtdV9kYXRhICpkYXRhID0gcDsNCj4gPiA+ID4gKw0K
PiA+ID4gPiArICAgICB2YWwgPSBpb3JlYWQzMmJlKCZkYXRhLT5yZWdzLT5zaXRlWzBdLnRyaXRz
cik7DQo+ID4gPiA+ICsgICAgICp0ZW1wID0gKGludCl2YWwgKiAxMDAwOw0KPiA+ID4NCj4gPiA+
IFdoeSBkb24ndCB5b3UgZGVjbGFyZSB2YWwgYXMgaW50IGluIHRoZSBmaXJzdCBwbGFjZT8NCj4g
Pg0KPiA+IEl0J3MgYSAzMmJpdCByZWdpc3Rlci4NCj4gPiBPbmx5IHRoZSBsZWFzdCBzaWduaWZp
Y2FudCA4IGJpdHMgcmVwcmVzZW50IHRoZSB0ZW1wZXJhdHVyZS4NCj4gPiBUaGUgbW9zdCBzaWdu
aWZpY2FudCBiaXQgc2hvd3MgdGhlIHZhbGlkbmVzcyBvZiB0aGUgdmFsdWUuDQo+ID4gSSB1c2Ug
dTggdHlwZSB0byByZW1vdmUgdGhlIHJlc3QgMjRiaXRzIGluZmx1ZW5jZS4NCj4gDQo+IFRoYXQn
cyBldmVuIHdvcnNlLiAgVXNlIGFuIGV4cGxpY2l0ICYgb3BlcmF0aW9uIHRvIGV4dHJhY3QgdGhl
IGZpZWxkDQo+IHlvdSdyZQ0KPiBpbnRlcmVzdGVkIGluLg0KDQpXaWxsIGNoYW5nZSBmb3IgbmV4
dCB2ZXJzaW9uLg0KDQo+IA0KPiA+ID4gPiArICAgICByZXQgPSBxb3JpcV90bXVfY2FsaWJyYXRp
b24ocGRldik7ICAgICAgLyogVE1VIGNhbGlicmF0aW9uICovDQo+ID4gPiA+ICsgICAgIGlmIChy
ZXQgPCAwKSB7DQo+ID4gPiA+ICsgICAgICAgICAgICAgZGV2X2VycigmcGRldi0+ZGV2LCAiVE1V
IGNhbGlicmF0aW9uIGZhaWxlZC5cbiIpOw0KPiA+ID4gPiArICAgICAgICAgICAgIHJldCA9IC1F
Tk9ERVY7DQo+ID4gPiA+ICsgICAgICAgICAgICAgZ290byBlcnJfaW9tYXA7DQo+ID4gPiA+ICsg
ICAgIH0NCj4gPiA+DQo+ID4gPiBUaGF0IGZ1bmN0aW9uIHJldHVybnMgbmVnYXRpdmUgd2hlbiBk
ZXZpY2UgdHJlZSBwcm9wZXJ0aWVzIGFyZQ0KPiBtaXNzaW5nLA0KPiA+ID4gbm90IHdoZW4gYSBj
YWxpYnJhdGlvbiBwcm9jZWR1cmUgZmFpbHMuDQo+ID4NCj4gPiBXaGF0J3MgeW91ciBzdWdnZXN0
aW9uIGhlcmUgdG8gcmV0dXJuIHRoZW4/DQo+IA0KPiBSZW1vdmUgdGhpcyBtZXNzYWdlIGFuZCBh
ZGQgYSBtZXNzYWdlIGluIHFvcmlxX3RtdV9jYWxpYnJhdGlvbiBmb3IgZWFjaA0KPiBlcnJvcg0K
PiBjb25kaXRpb24uICBBbHNvIGhhdmUgcW9yaXFfdG11X2NhbGlicmF0aW9uIHBhc3MgYmFjayBh
IHByb3BlciBlcnJvciBjb2RlLA0KPiBub3QgLTEuDQoNClllcywgdGhpcyBpcyBtb3JlIHJlYXNv
bmFibGUuDQoNCj4gDQo+ID4gPiA+ICtzdGF0aWMgY29uc3Qgc3RydWN0IG9mX2RldmljZV9pZCBx
b3JpcV90bXVfbWF0Y2hbXSA9IHsNCj4gPiA+ID4gKyAgICAgeyAuY29tcGF0aWJsZSA9ICJmc2ws
cW9yaXEtdG11IiwgfSwNCj4gPiA+ID4gKyAgICAge30sDQo+ID4gPiA+ICt9Ow0KPiA+ID4NCj4g
PiA+IEJpbmRpbmc/DQo+ID4NCj4gPiBOb3Qgc2VuZCBvdXQgeWV0Lg0KPiANCj4gVGhlIGJpbmRp
bmcgbmVlZHMgdG8gY29tZSBiZWZvcmUgdGhlIGRyaXZlciB0aGF0IHVzZXMgaXQuDQoNClJpZ2h0
LiBJIHdpbGwgc2VuZCBvdXQgdGhlIHdob2xlIHBhdGNoc2V0Lg0KDQo+IA0KPiAtU2NvdHQNCj4g
DQoNCg==
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V3] thermal: qoriq: Add thermal management support
2015-09-25 3:09 ` Hongtao Jia
2015-09-25 5:12 ` Scott Wood
@ 2015-11-04 19:24 ` Eduardo Valentin
2015-11-04 22:45 ` Scott Wood
2015-11-05 3:56 ` Hongtao Jia
1 sibling, 2 replies; 8+ messages in thread
From: Eduardo Valentin @ 2015-11-04 19:24 UTC (permalink / raw)
To: Hongtao Jia; +Cc: Scott Wood, linux-pm, linuxppc-dev
On Fri, Sep 25, 2015 at 03:09:42AM +0000, Hongtao Jia wrote:
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, September 25, 2015 6:10 AM
> > To: Jia Hongtao-B38951
> > Cc: edubezval@gmail.com; linux-pm@vger.kernel.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH V3] thermal: qoriq: Add thermal management support
> >
> > On Wed, 2015-09-23 at 16:28 +0800, Jia Hongtao wrote:
> > > This driver add thermal management support by enabling TMU (Thermal
> > > Monitoring Unit) on QorIQ platform.
> > >
> > > It's based on thermal of framework:
> > > - Trip points defined in device tree.
> > > - Cpufreq as cooling device registered in qoriq cpufreq driver.
> >
> > I don't see any cooling device registered in the qoriq cpufreq driver.
> > Is this dependent on some other patch?
>
> It's not depend on any patch. But I saw your patch below:
> [PATCH v3 5/5] cpufreq: qoriq: Don't look at clock implementation details
> So I hold my patch waiting for your patch merged or there will be conflict.
>
> I could send it out too if you are fine with it.
Would you guys benefit of cpufreq-cpu0?
>
> >
> > >
> > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> > > ---
> > > V3: Using thermal of framework.
> > >
> > > drivers/thermal/Kconfig | 11 ++
> > > drivers/thermal/Makefile | 1 +
> > > drivers/thermal/qoriq_thermal.c | 267
Please include a binding description in your next version. Also,
remember to include an example of your binding.
> > > ++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 279 insertions(+)
> > > create mode 100644 drivers/thermal/qoriq_thermal.c
> > >
> > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> > > 0390044..c91041b 100644
> > > --- a/drivers/thermal/Kconfig
> > > +++ b/drivers/thermal/Kconfig
> > > @@ -180,6 +180,17 @@ config IMX_THERMAL
> > > cpufreq is used as the cooling device to throttle CPUs when the
> > > passive trip is crossed.
> > >
> > > +config QORIQ_THERMAL
It would be really great if you could consolidate this driver with
IMX_THERMAL. Do you think it is doable?
> > > + tristate "Freescale QorIQ Thermal Monitoring Unit"
> > > + depends on CPU_THERMAL
> > > + depends on THERMAL_OF
> > > + default n
> > > + help
> > > + Enable thermal management based on Freescale QorIQ Thermal
> > Monitoring
> > > + Unit (TMU). It supports one critical trip point and one passive
> > trip
> > > + point. The cpufreq is used as the cooling device to throttle
> > CPUs when
> > > + the passive trip is crossed.
> >
> > "default n" is unnecessary -- n is already the default.
>
> Right.
>
> >
> > Where is the interaction between this driver and cpufreq?
>
> It's all in cpufreq patch I mentioned above.
>
> >
> > > config SPEAR_THERMAL
> > > bool "SPEAr thermal sensor driver"
> > > depends on PLAT_SPEAR
> > > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index
> > > 26f1608..e55d703 100644
> > > --- a/drivers/thermal/Makefile
> > > +++ b/drivers/thermal/Makefile
> > > @@ -33,6 +33,7 @@ obj-$(CONFIG_DOVE_THERMAL) += dove_thermal.o
> > > obj-$(CONFIG_DB8500_THERMAL) += db8500_thermal.o
> > > obj-$(CONFIG_ARMADA_THERMAL) += armada_thermal.o
> > > obj-$(CONFIG_IMX_THERMAL) += imx_thermal.o
> > > +obj-$(CONFIG_QORIQ_THERMAL) += qoriq_thermal.o
> > > obj-$(CONFIG_DB8500_CPUFREQ_COOLING) += db8500_cpufreq_cooling.o
> > > obj-$(CONFIG_INTEL_POWERCLAMP) += intel_powerclamp.o
> > > obj-$(CONFIG_X86_PKG_TEMP_THERMAL) += x86_pkg_temp_thermal.o
> > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > b/drivers/thermal/qoriq_thermal.c new file mode 100644 index
> > > 0000000..7c2a3261
> > > --- /dev/null
> > > +++ b/drivers/thermal/qoriq_thermal.c
> > > @@ -0,0 +1,267 @@
> > > +/*
> > > + * Copyright 2015 Freescale Semiconductor, Inc.
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > +modify it
> > > + * under the terms and conditions of the GNU General Public License,
> > > + * version 2, as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope 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.
> > > + *
> > > + */
> > > +
> > > +/*
> > > + * Based on Freescale QorIQ Thermal Monitoring Unit (TMU) */
> >
> > What does this comment mean? This *is* the "Freescale QorIQ Thermal
> > Monitoring Unit" driver.
>
> I mean thermal management based on the monitor TMU.
> I could remove this comment though.
>
> >
> > > +#include <linux/cpufreq.h>
> >
> > What do you use from this header?
>
> Sorry, forget to remove it from last version in which cooling devices are
> registered in thermal driver instead of cpufreq driver.
>
> >
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/io.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/thermal.h>
> > > +
> > > +#include "thermal_core.h"
> > > +
> > > +#define SITES_MAX 16
> > > +
> > > +/*
> > > + * QorIQ TMU Registers
> > > + */
> > > +struct qoriq_tmu_site_regs {
> > > + __be32 tritsr; /* Immediate Temperature Site Register */
> > > + __be32 tratsr; /* Average Temperature Site Register */
> > > + u8 res0[0x8];
> > > +} __packed;
> > > +
> > > +struct qoriq_tmu_regs {
> > > + __be32 tmr; /* Mode Register */
> > > +#define TMR_DISABLE 0x0
> > > +#define TMR_ME 0x80000000
> > > +#define TMR_ALPF 0x0c000000
> > > +#define TMR_MSITE 0x00008000 /* Core temperature site */
> > > +#define TMR_ALL (TMR_ME | TMR_ALPF | TMR_MSITE)
> > > + __be32 tsr; /* Status Register */
> > > + __be32 tmtmir; /* Temperature measurement interval
> > Register */
> > > +#define TMTMIR_DEFAULT 0x00000007
> > > + u8 res0[0x14];
> > > + __be32 tier; /* Interrupt Enable Register */
> > > +#define TIER_DISABLE 0x0
> > > + __be32 tidr; /* Interrupt Detect Register */
> > > + __be32 tiscr; /* Interrupt Site Capture Register */
> > > + __be32 ticscr; /* Interrupt Critical Site Capture
> > Register */
> > > + u8 res1[0x10];
> > > + __be32 tmhtcrh; /* High Temperature Capture Register */
> > > + __be32 tmhtcrl; /* Low Temperature Capture Register */
> > > + u8 res2[0x8];
> > > + __be32 tmhtitr; /* High Temperature Immediate Threshold
> > */
> > > + __be32 tmhtatr; /* High Temperature Average Threshold */
> > > + __be32 tmhtactr; /* High Temperature Average Crit
> > Threshold */
> > > + u8 res3[0x24];
> > > + __be32 ttcfgr; /* Temperature Configuration Register */
> > > + __be32 tscfgr; /* Sensor Configuration Register */
> > > + u8 res4[0x78];
> > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > + u8 res5[0x9f8];
> > > + __be32 ipbrr0; /* IP Block Revision Register 0 */
> > > + __be32 ipbrr1; /* IP Block Revision Register 1 */
> > > + u8 res6[0x310];
> > > + __be32 ttr0cr; /* Temperature Range 0 Control Register
> > */
> > > + __be32 ttr1cr; /* Temperature Range 1 Control Register
> > */
> > > + __be32 ttr2cr; /* Temperature Range 2 Control Register
> > */
> > > + __be32 ttr3cr; /* Temperature Range 3 Control Register
> > */
> > > +};
> > > +
> > > +/*
> > > + * Thermal zone data
> > > + */
> > > +struct qoriq_tmu_data {
> > > + struct thermal_zone_device *tz;
> > > + struct qoriq_tmu_regs __iomem *regs; };
> > > +
> >
> > > +static int tmu_get_temp(void *p, int *temp) {
> > > + u8 val;
> > > + struct qoriq_tmu_data *data = p;
> > > +
> > > + val = ioread32be(&data->regs->site[0].tritsr);
> > > + *temp = (int)val * 1000;
> >
> > Why don't you declare val as int in the first place?
>
> It's a 32bit register.
> Only the least significant 8 bits represent the temperature.
> The most significant bit shows the validness of the value.
> I use u8 type to remove the rest 24bits influence.
>
> >
> > > + for (i = 0; i < len; i += 8, calibration += 2) {
> > > + val = (int)of_read_number(calibration, 1);
> > > + iowrite32be(val, &data->regs->ttcfgr);
> > > + val = (int)of_read_number(calibration + 1, 1);
> > > + iowrite32be(val, &data->regs->tscfgr);
> >
> > Unnecessary casts.
>
> Ok.
>
> >
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void qoriq_tmu_init_device(struct qoriq_tmu_data *data) {
> > > + /* Disable interrupt, using polling instead */
> > > + iowrite32be(TIER_DISABLE, &data->regs->tier);
> > > +
> > > + /* Set update_interval */
> > > + iowrite32be(TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > +
> > > + /* Enable monitoring */
> > > + iowrite32be(TMR_ALL, &data->regs->tmr); }
> > > +
> > > +static struct thermal_zone_of_device_ops tmu_tz_ops = {
> > > + .get_temp = tmu_get_temp,
> > > +};
> > > +
> > > +static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > + int ret;
> > > + const struct thermal_trip *trip;
> > > + struct qoriq_tmu_data *data;
> > > +
> > > + if (!pdev->dev.of_node) {
> > > + dev_err(&pdev->dev, "Device OF-Node is NULL");
> > > + return -EFAULT;
> > > + }
> >
> > EFAULT is for bad user addresses and is not appropriate here.
>
> I will change it to ENODEV.
>
> >
> > > +
> > > + data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
> > > + GFP_KERNEL);
> > > + if (!data)
> > > + return -ENOMEM;
> > > +
> > > + platform_set_drvdata(pdev, data);
> > > + data->regs = of_iomap(pdev->dev.of_node, 0);
> > > +
> > > + if (!data->regs) {
> > > + dev_err(&pdev->dev, "Failed to get memory region\n");
> > > + ret = -ENODEV;
> > > + goto err_iomap;
> > > + }
> > > +
> > > + ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
> > > + if (ret < 0) {
> > > + dev_err(&pdev->dev, "TMU calibration failed.\n");
> > > + ret = -ENODEV;
> > > + goto err_iomap;
> > > + }
> >
> > That function returns negative when device tree properties are missing,
> > not when a calibration procedure fails.
>
> What's your suggestion here to return then?
>
> >
> > > +
> > > + qoriq_tmu_init_device(data); /* TMU initialization */
> > > +
> > > + data->tz = thermal_zone_of_sensor_register(&pdev->dev, 0,
> > > + data, &tmu_tz_ops);
> > > + if (IS_ERR(data->tz)) {
> > > + ret = PTR_ERR(data->tz);
> > > + dev_err(&pdev->dev,
> > > + "Failed to register thermal zone device %d\n",
> > ret);
> > > + goto err_thermal;
> > > + }
> > > +
> > > + trip = of_thermal_get_trip_points(data->tz);
> > > +
> > > + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
> >
> > Doesn't thermal_zone_of_sensor_register() already call ops->set_mode with
> > TERMAL_DEVICE_ENABLED?
>
> You are right.
> I referenced the code from other platform and did not notice this.
> Thanks.
>
> >
> > > +
> > > + return 0;
> > > +
> > > +err_thermal:
> > > + iounmap(data->regs);
> > > +
> > > +err_iomap:
> > > + platform_set_drvdata(pdev, NULL);
> > > + devm_kfree(&pdev->dev, data);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int qoriq_tmu_remove(struct platform_device *pdev) {
> > > + struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
> > > +
> > > + /* Disable monitoring */
> > > + iowrite32be(TMR_DISABLE, &data->regs->tmr);
> > > +
> > > + thermal_zone_of_sensor_unregister(&pdev->dev, data->tz);
> > > + iounmap(data->regs);
> > > +
> > > + platform_set_drvdata(pdev, NULL);
> > > + devm_kfree(&pdev->dev, data);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int qoriq_tmu_suspend(struct device *dev) {
> > > + struct qoriq_tmu_data *data = dev_get_drvdata(dev);
> > > +
> > > + /* Disable monitoring */
> > > + iowrite32be(TMR_DISABLE, &data->regs->tmr);
> > > + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_DISABLED);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int qoriq_tmu_resume(struct device *dev) {
> > > + struct qoriq_tmu_data *data = dev_get_drvdata(dev);
> > > +
> > > + /* Enable monitoring */
> > > + iowrite32be(TMR_ALL, &data->regs->tmr);
> > > + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
> > > +
> > > + return 0;
> > > +}
> > > +#endif
> > > +
> > > +static SIMPLE_DEV_PM_OPS(qoriq_tmu_pm_ops,
> > > + qoriq_tmu_suspend, qoriq_tmu_resume);
> > > +
> >
> >
> > > +static const struct of_device_id qoriq_tmu_match[] = {
> > > + { .compatible = "fsl,qoriq-tmu", },
> > > + {},
> > > +};
> >
> > Binding?
>
> Not send out yet.
> I planned to send the patch with device tree patchset including
> adding TMU node to T1040/T1024/LS1021A platform.
>
> It's depending on the device tree files moving patch I sent out
> not long ago.
>
> >
> > > +
> > > +static struct platform_driver qoriq_tmu = {
> > > + .driver = {
> > > + .name = "qoriq_thermal",
> > > + .pm = &qoriq_tmu_pm_ops,
> > > + .of_match_table = qoriq_tmu_match,
> > > + },
> >
> > Why the tabs after ".name"?
>
> Will indent the other two.
>
> >
> > -Scott
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V3] thermal: qoriq: Add thermal management support
2015-11-04 19:24 ` Eduardo Valentin
@ 2015-11-04 22:45 ` Scott Wood
2015-11-05 3:56 ` Hongtao Jia
1 sibling, 0 replies; 8+ messages in thread
From: Scott Wood @ 2015-11-04 22:45 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: Hongtao Jia, linux-pm, linuxppc-dev
On Wed, 2015-11-04 at 11:24 -0800, Eduardo Valentin wrote:
> On Fri, Sep 25, 2015 at 03:09:42AM +0000, Hongtao Jia wrote:
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Friday, September 25, 2015 6:10 AM
> > > To: Jia Hongtao-B38951
> > > Cc: edubezval@gmail.com; linux-pm@vger.kernel.org; linuxppc-
> > > dev@lists.ozlabs.org
> > > Subject: Re: [PATCH V3] thermal: qoriq: Add thermal management support
> > >
> > > On Wed, 2015-09-23 at 16:28 +0800, Jia Hongtao wrote:
> > > > This driver add thermal management support by enabling TMU (Thermal
> > > > Monitoring Unit) on QorIQ platform.
> > > >
> > > > It's based on thermal of framework:
> > > > - Trip points defined in device tree.
> > > > - Cpufreq as cooling device registered in qoriq cpufreq driver.
> > >
> > > I don't see any cooling device registered in the qoriq cpufreq driver.
> > > Is this dependent on some other patch?
> >
> > It's not depend on any patch. But I saw your patch below:
> > [PATCH v3 5/5] cpufreq: qoriq: Don't look at clock implementation details
> > So I hold my patch waiting for your patch merged or there will be
> > conflict.
> >
> > I could send it out too if you are fine with it.
>
> Would you guys benefit of cpufreq-cpu0?
If you mean cpufreq-dt, no, as I explained in
https://lkml.org/lkml/2015/9/18/699
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V3] thermal: qoriq: Add thermal management support
2015-11-04 19:24 ` Eduardo Valentin
2015-11-04 22:45 ` Scott Wood
@ 2015-11-05 3:56 ` Hongtao Jia
1 sibling, 0 replies; 8+ messages in thread
From: Hongtao Jia @ 2015-11-05 3:56 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: Scott Wood, linux-pm, linuxppc-dev
> -----Original Message-----
> From: Eduardo Valentin [mailto:edubezval@gmail.com]
> Sent: Thursday, November 05, 2015 3:25 AM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linux-pm@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org
> Subject: Re: [PATCH V3] thermal: qoriq: Add thermal management support
>=20
> On Fri, Sep 25, 2015 at 03:09:42AM +0000, Hongtao Jia wrote:
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Friday, September 25, 2015 6:10 AM
> > > To: Jia Hongtao-B38951
> > > Cc: edubezval@gmail.com; linux-pm@vger.kernel.org; linuxppc-
> > > dev@lists.ozlabs.org
> > > Subject: Re: [PATCH V3] thermal: qoriq: Add thermal management
> > > support
> > >
> > > On Wed, 2015-09-23 at 16:28 +0800, Jia Hongtao wrote:
> > > > This driver add thermal management support by enabling TMU
> > > > (Thermal Monitoring Unit) on QorIQ platform.
> > > >
> > > > It's based on thermal of framework:
> > > > - Trip points defined in device tree.
> > > > - Cpufreq as cooling device registered in qoriq cpufreq driver.
> > >
> > > I don't see any cooling device registered in the qoriq cpufreq driver=
.
> > > Is this dependent on some other patch?
> >
> > It's not depend on any patch. But I saw your patch below:
> > [PATCH v3 5/5] cpufreq: qoriq: Don't look at clock implementation
> > details So I hold my patch waiting for your patch merged or there will
> be conflict.
> >
> > I could send it out too if you are fine with it.
>=20
> Would you guys benefit of cpufreq-cpu0?
>=20
>=20
>=20
>=20
> >
> > >
> > > >
> > > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> > > > ---
> > > > V3: Using thermal of framework.
> > > >
> > > > drivers/thermal/Kconfig | 11 ++
> > > > drivers/thermal/Makefile | 1 +
> > > > drivers/thermal/qoriq_thermal.c | 267
>=20
> Please include a binding description in your next version. Also, remember
> to include an example of your binding.
>=20
>=20
>=20
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 279 insertions(+) create mode 100644
> > > > drivers/thermal/qoriq_thermal.c
> > > >
> > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > > index 0390044..c91041b 100644
> > > > --- a/drivers/thermal/Kconfig
> > > > +++ b/drivers/thermal/Kconfig
> > > > @@ -180,6 +180,17 @@ config IMX_THERMAL
> > > > cpufreq is used as the cooling device to throttle CPUs when
> the
> > > > passive trip is crossed.
> > > >
> > > > +config QORIQ_THERMAL
>=20
>=20
> It would be really great if you could consolidate this driver with
> IMX_THERMAL. Do you think it is doable?
The TMU IP on I.MX and QorIQ platform are completely different and they
use the different cpufreq driver.
I don't think they could share the same thermal driver elegantly.
-Hongtao.=20
>=20
> > > > + tristate "Freescale QorIQ Thermal Monitoring Unit"
> > > > + depends on CPU_THERMAL
> > > > + depends on THERMAL_OF
> > > > + default n
> > > > + help
> > > > + Enable thermal management based on Freescale QorIQ Thermal
> > > Monitoring
> > > > + Unit (TMU). It supports one critical trip point and one
> > > > + passive
> > > trip
> > > > + point. The cpufreq is used as the cooling device to
> > > > + throttle
> > > CPUs when
> > > > + the passive trip is crossed.
> > >
> > > "default n" is unnecessary -- n is already the default.
> >
> > Right.
> >
> > >
> > > Where is the interaction between this driver and cpufreq?
> >
> > It's all in cpufreq patch I mentioned above.
> >
> > >
> > > > config SPEAR_THERMAL
> > > > bool "SPEAr thermal sensor driver"
> > > > depends on PLAT_SPEAR
> > > > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > > > index
> > > > 26f1608..e55d703 100644
> > > > --- a/drivers/thermal/Makefile
> > > > +++ b/drivers/thermal/Makefile
> > > > @@ -33,6 +33,7 @@ obj-$(CONFIG_DOVE_THERMAL) +=3D
> dove_thermal.o
> > > > obj-$(CONFIG_DB8500_THERMAL) +=3D db8500_thermal.o
> > > > obj-$(CONFIG_ARMADA_THERMAL) +=3D armada_thermal.o
> > > > obj-$(CONFIG_IMX_THERMAL) +=3D imx_thermal.o
> > > > +obj-$(CONFIG_QORIQ_THERMAL) +=3D qoriq_thermal.o
> > > > obj-$(CONFIG_DB8500_CPUFREQ_COOLING) +=3D db8500_cpufreq_cooling.o
> > > > obj-$(CONFIG_INTEL_POWERCLAMP) +=3D intel_powerclamp.o
> > > > obj-$(CONFIG_X86_PKG_TEMP_THERMAL) +=3D x86_pkg_temp_thermal.o
> > > > diff --git a/drivers/thermal/qoriq_thermal.c
> > > > b/drivers/thermal/qoriq_thermal.c new file mode 100644 index
> > > > 0000000..7c2a3261
> > > > --- /dev/null
> > > > +++ b/drivers/thermal/qoriq_thermal.c
> > > > @@ -0,0 +1,267 @@
> > > > +/*
> > > > + * Copyright 2015 Freescale Semiconductor, Inc.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > +modify it
> > > > + * under the terms and conditions of the GNU General Public
> > > > +License,
> > > > + * version 2, as published by the Free Software Foundation.
> > > > + *
> > > > + * This program is distributed in the hope 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.
> > > > + *
> > > > + */
> > > > +
> > > > +/*
> > > > + * Based on Freescale QorIQ Thermal Monitoring Unit (TMU) */
> > >
> > > What does this comment mean? This *is* the "Freescale QorIQ Thermal
> > > Monitoring Unit" driver.
> >
> > I mean thermal management based on the monitor TMU.
> > I could remove this comment though.
> >
> > >
> > > > +#include <linux/cpufreq.h>
> > >
> > > What do you use from this header?
> >
> > Sorry, forget to remove it from last version in which cooling devices
> > are registered in thermal driver instead of cpufreq driver.
> >
> > >
> > > > +#include <linux/module.h>
> > > > +#include <linux/platform_device.h> #include <linux/err.h>
> > > > +#include <linux/io.h> #include <linux/of.h> #include
> > > > +<linux/of_address.h> #include <linux/thermal.h>
> > > > +
> > > > +#include "thermal_core.h"
> > > > +
> > > > +#define SITES_MAX 16
> > > > +
> > > > +/*
> > > > + * QorIQ TMU Registers
> > > > + */
> > > > +struct qoriq_tmu_site_regs {
> > > > + __be32 tritsr; /* Immediate Temperature Site
> Register */
> > > > + __be32 tratsr; /* Average Temperature Site Register
> */
> > > > + u8 res0[0x8];
> > > > +} __packed;
> > > > +
> > > > +struct qoriq_tmu_regs {
> > > > + __be32 tmr; /* Mode Register */
> > > > +#define TMR_DISABLE 0x0
> > > > +#define TMR_ME 0x80000000
> > > > +#define TMR_ALPF 0x0c000000
> > > > +#define TMR_MSITE 0x00008000 /* Core temperature site */
> > > > +#define TMR_ALL (TMR_ME | TMR_ALPF | TMR_MSITE)
> > > > + __be32 tsr; /* Status Register */
> > > > + __be32 tmtmir; /* Temperature measurement interval
> > > Register */
> > > > +#define TMTMIR_DEFAULT 0x00000007
> > > > + u8 res0[0x14];
> > > > + __be32 tier; /* Interrupt Enable Register */
> > > > +#define TIER_DISABLE 0x0
> > > > + __be32 tidr; /* Interrupt Detect Register */
> > > > + __be32 tiscr; /* Interrupt Site Capture Register */
> > > > + __be32 ticscr; /* Interrupt Critical Site Capture
> > > Register */
> > > > + u8 res1[0x10];
> > > > + __be32 tmhtcrh; /* High Temperature Capture Register
> */
> > > > + __be32 tmhtcrl; /* Low Temperature Capture Register
> */
> > > > + u8 res2[0x8];
> > > > + __be32 tmhtitr; /* High Temperature Immediate
> Threshold
> > > */
> > > > + __be32 tmhtatr; /* High Temperature Average Threshold
> */
> > > > + __be32 tmhtactr; /* High Temperature Average Crit
> > > Threshold */
> > > > + u8 res3[0x24];
> > > > + __be32 ttcfgr; /* Temperature Configuration Register
> */
> > > > + __be32 tscfgr; /* Sensor Configuration Register */
> > > > + u8 res4[0x78];
> > > > + struct qoriq_tmu_site_regs site[SITES_MAX];
> > > > + u8 res5[0x9f8];
> > > > + __be32 ipbrr0; /* IP Block Revision Register 0 */
> > > > + __be32 ipbrr1; /* IP Block Revision Register 1 */
> > > > + u8 res6[0x310];
> > > > + __be32 ttr0cr; /* Temperature Range 0 Control
> Register
> > > */
> > > > + __be32 ttr1cr; /* Temperature Range 1 Control
> Register
> > > */
> > > > + __be32 ttr2cr; /* Temperature Range 2 Control
> Register
> > > */
> > > > + __be32 ttr3cr; /* Temperature Range 3 Control
> Register
> > > */
> > > > +};
> > > > +
> > > > +/*
> > > > + * Thermal zone data
> > > > + */
> > > > +struct qoriq_tmu_data {
> > > > + struct thermal_zone_device *tz;
> > > > + struct qoriq_tmu_regs __iomem *regs; };
> > > > +
> > >
> > > > +static int tmu_get_temp(void *p, int *temp) {
> > > > + u8 val;
> > > > + struct qoriq_tmu_data *data =3D p;
> > > > +
> > > > + val =3D ioread32be(&data->regs->site[0].tritsr);
> > > > + *temp =3D (int)val * 1000;
> > >
> > > Why don't you declare val as int in the first place?
> >
> > It's a 32bit register.
> > Only the least significant 8 bits represent the temperature.
> > The most significant bit shows the validness of the value.
> > I use u8 type to remove the rest 24bits influence.
> >
> > >
> > > > + for (i =3D 0; i < len; i +=3D 8, calibration +=3D 2) {
> > > > + val =3D (int)of_read_number(calibration, 1);
> > > > + iowrite32be(val, &data->regs->ttcfgr);
> > > > + val =3D (int)of_read_number(calibration + 1, 1);
> > > > + iowrite32be(val, &data->regs->tscfgr);
> > >
> > > Unnecessary casts.
> >
> > Ok.
> >
> > >
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static void qoriq_tmu_init_device(struct qoriq_tmu_data *data) {
> > > > + /* Disable interrupt, using polling instead */
> > > > + iowrite32be(TIER_DISABLE, &data->regs->tier);
> > > > +
> > > > + /* Set update_interval */
> > > > + iowrite32be(TMTMIR_DEFAULT, &data->regs->tmtmir);
> > > > +
> > > > + /* Enable monitoring */
> > > > + iowrite32be(TMR_ALL, &data->regs->tmr); }
> > > > +
> > > > +static struct thermal_zone_of_device_ops tmu_tz_ops =3D {
> > > > + .get_temp =3D tmu_get_temp,
> > > > +};
> > > > +
> > > > +static int qoriq_tmu_probe(struct platform_device *pdev) {
> > > > + int ret;
> > > > + const struct thermal_trip *trip;
> > > > + struct qoriq_tmu_data *data;
> > > > +
> > > > + if (!pdev->dev.of_node) {
> > > > + dev_err(&pdev->dev, "Device OF-Node is NULL");
> > > > + return -EFAULT;
> > > > + }
> > >
> > > EFAULT is for bad user addresses and is not appropriate here.
> >
> > I will change it to ENODEV.
> >
> > >
> > > > +
> > > > + data =3D devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_dat=
a),
> > > > + GFP_KERNEL);
> > > > + if (!data)
> > > > + return -ENOMEM;
> > > > +
> > > > + platform_set_drvdata(pdev, data);
> > > > + data->regs =3D of_iomap(pdev->dev.of_node, 0);
> > > > +
> > > > + if (!data->regs) {
> > > > + dev_err(&pdev->dev, "Failed to get memory region\n");
> > > > + ret =3D -ENODEV;
> > > > + goto err_iomap;
> > > > + }
> > > > +
> > > > + ret =3D qoriq_tmu_calibration(pdev); /* TMU calibration =
*/
> > > > + if (ret < 0) {
> > > > + dev_err(&pdev->dev, "TMU calibration failed.\n");
> > > > + ret =3D -ENODEV;
> > > > + goto err_iomap;
> > > > + }
> > >
> > > That function returns negative when device tree properties are
> > > missing, not when a calibration procedure fails.
> >
> > What's your suggestion here to return then?
> >
> > >
> > > > +
> > > > + qoriq_tmu_init_device(data); /* TMU initialization */
> > > > +
> > > > + data->tz =3D thermal_zone_of_sensor_register(&pdev->dev, 0,
> > > > + data, &tmu_tz_ops);
> > > > + if (IS_ERR(data->tz)) {
> > > > + ret =3D PTR_ERR(data->tz);
> > > > + dev_err(&pdev->dev,
> > > > + "Failed to register thermal zone device
> > > > + %d\n",
> > > ret);
> > > > + goto err_thermal;
> > > > + }
> > > > +
> > > > + trip =3D of_thermal_get_trip_points(data->tz);
> > > > +
> > > > + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
> > >
> > > Doesn't thermal_zone_of_sensor_register() already call ops->set_mode
> > > with TERMAL_DEVICE_ENABLED?
> >
> > You are right.
> > I referenced the code from other platform and did not notice this.
> > Thanks.
> >
> > >
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_thermal:
> > > > + iounmap(data->regs);
> > > > +
> > > > +err_iomap:
> > > > + platform_set_drvdata(pdev, NULL);
> > > > + devm_kfree(&pdev->dev, data);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int qoriq_tmu_remove(struct platform_device *pdev) {
> > > > + struct qoriq_tmu_data *data =3D platform_get_drvdata(pdev);
> > > > +
> > > > + /* Disable monitoring */
> > > > + iowrite32be(TMR_DISABLE, &data->regs->tmr);
> > > > +
> > > > + thermal_zone_of_sensor_unregister(&pdev->dev, data->tz);
> > > > + iounmap(data->regs);
> > > > +
> > > > + platform_set_drvdata(pdev, NULL);
> > > > + devm_kfree(&pdev->dev, data);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +#ifdef CONFIG_PM_SLEEP
> > > > +static int qoriq_tmu_suspend(struct device *dev) {
> > > > + struct qoriq_tmu_data *data =3D dev_get_drvdata(dev);
> > > > +
> > > > + /* Disable monitoring */
> > > > + iowrite32be(TMR_DISABLE, &data->regs->tmr);
> > > > + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_DISABLED);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int qoriq_tmu_resume(struct device *dev) {
> > > > + struct qoriq_tmu_data *data =3D dev_get_drvdata(dev);
> > > > +
> > > > + /* Enable monitoring */
> > > > + iowrite32be(TMR_ALL, &data->regs->tmr);
> > > > + data->tz->ops->set_mode(data->tz, THERMAL_DEVICE_ENABLED);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +#endif
> > > > +
> > > > +static SIMPLE_DEV_PM_OPS(qoriq_tmu_pm_ops,
> > > > + qoriq_tmu_suspend, qoriq_tmu_resume);
> > > > +
> > >
> > >
> > > > +static const struct of_device_id qoriq_tmu_match[] =3D {
> > > > + { .compatible =3D "fsl,qoriq-tmu", },
> > > > + {},
> > > > +};
> > >
> > > Binding?
> >
> > Not send out yet.
> > I planned to send the patch with device tree patchset including adding
> > TMU node to T1040/T1024/LS1021A platform.
> >
> > It's depending on the device tree files moving patch I sent out not
> > long ago.
> >
> > >
> > > > +
> > > > +static struct platform_driver qoriq_tmu =3D {
> > > > + .driver =3D {
> > > > + .name =3D "qoriq_thermal",
> > > > + .pm =3D &qoriq_tmu_pm_ops,
> > > > + .of_match_table =3D qoriq_tmu_match,
> > > > + },
> > >
> > > Why the tabs after ".name"?
> >
> > Will indent the other two.
> >
> > >
> > > -Scott
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-05 4:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-23 8:28 [PATCH V3] thermal: qoriq: Add thermal management support Jia Hongtao
2015-09-24 22:09 ` Scott Wood
2015-09-25 3:09 ` Hongtao Jia
2015-09-25 5:12 ` Scott Wood
2015-09-25 7:36 ` Hongtao Jia
2015-11-04 19:24 ` Eduardo Valentin
2015-11-04 22:45 ` Scott Wood
2015-11-05 3:56 ` Hongtao Jia
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).