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....@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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev