2019-02-18 19:36:39

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 00/12] QorIQ TMU multi-sensor and HWMON support

Everyone:

This series contains patches adding support for HWMON integration,
multi-sensor support as well as a small fix and general improvements
(hopefully) for TMU driver I made while working on it on i.MX8MQ.

Feedback is welcome!

Thanks,
Andrey Smirnov

Andrey Smirnov (12):
thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()
thermal: qoriq: Remove unnecessary DT node is NULL check
thermal: qoriq: Add local struct device pointer in qoriq_tmu_probe()
thermal: qoriq: Don't pass platform_device to qoriq_tmu_calibration()
thermal: qoriq: Convert driver to use devm_ioremap()
thermal: qoriq: Add hwmon support
thermal: qoriq: Convert driver to use regmap API
thermal: qoriq: Enable monitoring before querying temperature
thermal: qoriq: Do not report invalid temperature reading
thermal: qoriq: Simplify error handling in qoriq_tmu_get_sensor_id()
thermal: qoriq: Be more strict when parsing "thermal-sensors"
thermal: qoriq: Add support for multiple thremal sites

drivers/thermal/qoriq_thermal.c | 326 ++++++++++++++++++--------------
drivers/thermal/thermal_hwmon.c | 28 +++
drivers/thermal/thermal_hwmon.h | 7 +
3 files changed, 214 insertions(+), 147 deletions(-)

--
2.20.1



2019-02-18 19:33:04

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 02/12] thermal: qoriq: Remove unnecessary DT node is NULL check

This driver is meant to be used with Device Tree and there's no
use-case where device's DT node is going to be NULL. Remove code
protecting against that.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 18c711b19514..3e147f856bf5 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -189,11 +189,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
u32 site;

- if (!np) {
- dev_err(&pdev->dev, "Device OF-Node is NULL");
- return -ENODEV;
- }
-
data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
--
2.20.1


2019-02-18 19:33:14

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 09/12] thermal: qoriq: Do not report invalid temperature reading

Before returning measured temperature data to upper layer we need to
make sure that the reading was marked as "valid" to avoid reporting
bogus data.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 6478d7a8168f..3d520d3b2da4 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -37,6 +37,7 @@
#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
* Site Register
*/
+#define TRITSR_V BIT(31)
#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
* Control Register
*/
@@ -55,8 +56,10 @@ static int tmu_get_temp(void *p, int *temp)
struct qoriq_tmu_data *data = p;

regmap_read(data->regmap, REGS_TRITSR(data->sensor_id), &val);
- *temp = (val & 0xff) * 1000;
+ if (!(val & TRITSR_V))
+ return -ENODATA;

+ *temp = (val & 0xff) * 1000;
return 0;
}

--
2.20.1


2019-02-18 19:33:16

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 11/12] thermal: qoriq: Be more strict when parsing "thermal-sensors"

As per Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
thermal-sensor-cells _must_ be 1, so there is no reason to be more
lenient in the code and treat the absence of argument in
thermal-sensors specifier as a valid DT code. Drop that special case
to simplify sensor ID retreival code.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index d4f5e180e1ee..f746c62789b0 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -82,14 +82,14 @@ static int qoriq_tmu_get_sensor_id(void)
goto out;
}

- if (sensor_specs.args_count >= 1) {
- id = sensor_specs.args[0];
- WARN(sensor_specs.args_count > 1,
- "%pOFn: too many cells in sensor specifier %d\n",
- sensor_specs.np, sensor_specs.args_count);
- } else {
- id = 0;
+ if (sensor_specs.args_count != 1) {
+ pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
+ sensor_specs.np, sensor_specs.args_count);
+ id = -EINVAL;
+ goto out;
}
+
+ id = sensor_specs.args[0];
out:
of_node_put(np);
of_node_put(sensor_np);
--
2.20.1


2019-02-18 19:33:25

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 05/12] thermal: qoriq: Convert driver to use devm_ioremap()

Convert driver to use devm_ioremap() to simplify memory deallocation
and error handling code. No functional change intended.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 9bae001e8264..472594fca03b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -188,6 +188,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
+ struct resource *io;
u32 site;

data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
@@ -203,7 +204,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
return -ENODEV;
}

- data->regs = of_iomap(np, 0);
+ io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!io) {
+ dev_err(dev, "Failed to get memory region\n");
+ return -ENODEV;
+ }
+
+ data->regs = devm_ioremap(dev, io->start, resource_size(io));
if (!data->regs) {
dev_err(dev, "Failed to get memory region\n");
return -ENODEV;
@@ -213,7 +220,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)

ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
- goto err_tmu;
+ return ret;

data->tz = devm_thermal_zone_of_sensor_register(dev,
data->sensor_id,
@@ -222,7 +229,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
ret = PTR_ERR(data->tz);
dev_err(dev, "Failed to register thermal zone device %d\n",
ret);
- goto err_tmu;
+ return ret;
}

platform_set_drvdata(pdev, data);
@@ -232,11 +239,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
tmu_write(data, site | TMR_ME | TMR_ALPF, &data->regs->tmr);

return 0;
-
-err_tmu:
- iounmap(data->regs);
-
- return ret;
}

static int qoriq_tmu_remove(struct platform_device *pdev)
@@ -246,7 +248,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
/* Disable monitoring */
tmu_write(data, TMR_DISABLE, &data->regs->tmr);

- iounmap(data->regs);
platform_set_drvdata(pdev, NULL);

return 0;
--
2.20.1


2019-02-18 19:33:25

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 12/12] thermal: qoriq: Add support for multiple thremal sites

TMU IP block provides temerature measurement for up to 16 sites,
current implementation of the driver however hardcodes a single site
ID. Change the code so it would be possible to reference multiple
sites as indivudial sensors and get their temperature readings.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 132 ++++++++++++++++++++------------
1 file changed, 84 insertions(+), 48 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index f746c62789b0..6cc6e6b36fb0 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -24,6 +24,8 @@
#define TMR_DISABLE 0x0
#define TMR_ME 0x80000000
#define TMR_ALPF 0x0c000000
+#define TMR_ALPF_MASK GENMASK(27, 26)
+

#define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
#define TMTMIR_DEFAULT 0x0000000f
@@ -41,21 +43,32 @@
#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
* Control Register
*/
+struct qoriq_tmu_sensor {
+ struct thermal_zone_device *tz;
+ int id;
+};
+
/*
* Thermal zone data
*/
struct qoriq_tmu_data {
struct thermal_zone_device *tz;
struct regmap *regmap;
- int sensor_id;
+ struct qoriq_tmu_sensor sensors[SITES_MAX];
};

+static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_tmu_sensor *s)
+{
+ return container_of(s, struct qoriq_tmu_data, sensors[s->id]);
+}
+
static int tmu_get_temp(void *p, int *temp)
{
u32 val;
- struct qoriq_tmu_data *data = p;
+ struct qoriq_tmu_sensor *s = p;
+ struct qoriq_tmu_data *data = qoriq_sensor_to_data(s);

- regmap_read(data->regmap, REGS_TRITSR(data->sensor_id), &val);
+ regmap_read(data->regmap, REGS_TRITSR(s->id), &val);
if (!(val & TRITSR_V))
return -ENODATA;

@@ -63,38 +76,86 @@ static int tmu_get_temp(void *p, int *temp)
return 0;
}

-static int qoriq_tmu_get_sensor_id(void)
+static const struct thermal_zone_of_device_ops tmu_tz_ops = {
+ .get_temp = tmu_get_temp,
+};
+
+static int qoriq_tmu_populate_sensors(struct device *dev,
+ struct qoriq_tmu_data *data)
{
- int ret, id;
+ int ret, id, i, count = 0;
struct of_phandle_args sensor_specs;
struct device_node *np, *sensor_np;
+ struct qoriq_tmu_sensor *s;

np = of_find_node_by_name(NULL, "thermal-zones");
if (!np)
return -ENODEV;

- sensor_np = of_get_next_child(np, NULL);
- ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
- "#thermal-sensor-cells",
- 0, &sensor_specs);
- if (ret) {
- id = ret;
- goto out;
+ for_each_child_of_node(np, sensor_np) {
+ u16 msite;
+
+ ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
+ "#thermal-sensor-cells",
+ 0, &sensor_specs);
+ if (ret)
+ break;
+
+ if (sensor_specs.args_count != 1) {
+ pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
+ sensor_specs.np, sensor_specs.args_count);
+ ret = -EINVAL;
+ break;
+ }
+
+ id = sensor_specs.args[0];
+ if (id >= SITES_MAX) {
+ ret = -EINVAL;
+ dev_err(dev, "Sensor id %d is out of range\n", id);
+ break;
+ }
+
+ msite = BIT(15 - id);
+ /* Enable monitoring of that particular sensor*/
+ regmap_update_bits(data->regmap, REGS_TMR, msite, msite);
+
+ s = &data->sensors[id];
+ s->id = id;
+ count++;
}

- if (sensor_specs.args_count != 1) {
- pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
- sensor_specs.np, sensor_specs.args_count);
- id = -EINVAL;
- goto out;
+ of_node_put(np);
+ if (ret) {
+ /* We only need to "put" sensor_np if we exited the
+ * loop early with "break"
+ */
+ of_node_put(sensor_np);
+ return ret;
}

- id = sensor_specs.args[0];
-out:
- of_node_put(np);
- of_node_put(sensor_np);
+ /* Enable monitoring */
+ regmap_update_bits(data->regmap, REGS_TMR, TMR_ME | TMR_ALPF_MASK,
+ TMR_ME | TMR_ALPF);
+
+ for (i = 0; i < count; i++) {
+ s = &data->sensors[i];
+ s->tz = devm_thermal_zone_of_sensor_register(dev, s->id, s,
+ &tmu_tz_ops);
+ if (IS_ERR(s->tz)) {
+ ret = PTR_ERR(s->tz);
+ dev_err(dev,
+ "Failed to register thermal zone device %d\n",
+ ret);
+ break;
+ }
+
+ ret = devm_thermal_add_hwmon_sysfs(s->tz);
+ if (ret)
+ break;
+
+ }

- return id;
+ return ret;
}

static int qoriq_tmu_calibration(struct device *dev,
@@ -142,10 +203,6 @@ static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
}

-static const struct thermal_zone_of_device_ops tmu_tz_ops = {
- .get_temp = tmu_get_temp,
-};
-
static const struct regmap_range qiriq_wr_yes_ranges[] = {
regmap_reg_range(REGS_TMR, REGS_TSCFGR),
regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
@@ -187,19 +244,12 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
.max_register = SZ_4K,
};
void __iomem *base;
- u32 site;

data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
return -ENOMEM;

- data->sensor_id = qoriq_tmu_get_sensor_id();
- if (data->sensor_id < 0) {
- dev_err(dev, "Failed to get sensor id\n");
- return -ENODEV;
- }
-
io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!io) {
dev_err(dev, "Failed to get memory region\n");
@@ -225,21 +275,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (ret < 0)
return ret;

- /* Enable monitoring */
- site = 0x1 << (15 - data->sensor_id);
- regmap_write(data->regmap, REGS_TMR, site | TMR_ME | TMR_ALPF);
-
- data->tz = devm_thermal_zone_of_sensor_register(dev,
- data->sensor_id,
- data, &tmu_tz_ops);
- if (IS_ERR(data->tz)) {
- ret = PTR_ERR(data->tz);
- dev_err(dev, "Failed to register thermal zone device %d\n",
- ret);
- goto disable_monitoring;
- }
-
- ret = devm_thermal_add_hwmon_sysfs(data->tz);
+ ret = qoriq_tmu_populate_sensors(dev, data);
if (ret)
goto disable_monitoring;

--
2.20.1


2019-02-18 19:33:51

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 10/12] thermal: qoriq: Simplify error handling in qoriq_tmu_get_sensor_id()

Use goto to avoid repeating resource deallocation code.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 3d520d3b2da4..d4f5e180e1ee 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -78,9 +78,8 @@ static int qoriq_tmu_get_sensor_id(void)
"#thermal-sensor-cells",
0, &sensor_specs);
if (ret) {
- of_node_put(np);
- of_node_put(sensor_np);
- return ret;
+ id = ret;
+ goto out;
}

if (sensor_specs.args_count >= 1) {
@@ -91,7 +90,7 @@ static int qoriq_tmu_get_sensor_id(void)
} else {
id = 0;
}
-
+out:
of_node_put(np);
of_node_put(sensor_np);

--
2.20.1


2019-02-18 19:33:56

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 07/12] thermal: qoriq: Convert driver to use regmap API

Convert driver to use regmap API, drop custom LE/BE IO helpers and
simplify bit manipulation using regmap_update_bits(). This also allows
us to convert some register initialization to use loops and adds
convenient debug access to TMU registers via debugfs.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 162 +++++++++++++++-----------------
1 file changed, 77 insertions(+), 85 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 90af4c4caa52..97419ce70d83 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -8,6 +8,7 @@
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/regmap.h>
#include <linux/thermal.h>

#include "thermal_core.h"
@@ -18,80 +19,42 @@
/*
* QorIQ TMU Registers
*/
-struct qoriq_tmu_site_regs {
- u32 tritsr; /* Immediate Temperature Site Register */
- u32 tratsr; /* Average Temperature Site Register */
- u8 res0[0x8];
-};

-struct qoriq_tmu_regs {
- u32 tmr; /* Mode Register */
+#define REGS_TMR 0x000 /* Mode Register */
#define TMR_DISABLE 0x0
#define TMR_ME 0x80000000
#define TMR_ALPF 0x0c000000
- u32 tsr; /* Status Register */
- u32 tmtmir; /* Temperature measurement interval Register */
+
+#define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
#define TMTMIR_DEFAULT 0x0000000f
- u8 res0[0x14];
- u32 tier; /* Interrupt Enable Register */
+
+#define REGS_TIER 0x020 /* Interrupt Enable Register */
#define TIER_DISABLE 0x0
- u32 tidr; /* Interrupt Detect Register */
- u32 tiscr; /* Interrupt Site Capture Register */
- u32 ticscr; /* Interrupt Critical Site Capture Register */
- u8 res1[0x10];
- u32 tmhtcrh; /* High Temperature Capture Register */
- u32 tmhtcrl; /* Low Temperature Capture Register */
- u8 res2[0x8];
- u32 tmhtitr; /* High Temperature Immediate Threshold */
- u32 tmhtatr; /* High Temperature Average Threshold */
- u32 tmhtactr; /* High Temperature Average Crit Threshold */
- u8 res3[0x24];
- u32 ttcfgr; /* Temperature Configuration Register */
- u32 tscfgr; /* Sensor Configuration Register */
- u8 res4[0x78];
- struct qoriq_tmu_site_regs site[SITES_MAX];
- u8 res5[0x9f8];
- u32 ipbrr0; /* IP Block Revision Register 0 */
- u32 ipbrr1; /* IP Block Revision Register 1 */
- u8 res6[0x310];
- u32 ttr0cr; /* Temperature Range 0 Control Register */
- u32 ttr1cr; /* Temperature Range 1 Control Register */
- u32 ttr2cr; /* Temperature Range 2 Control Register */
- u32 ttr3cr; /* Temperature Range 3 Control Register */
-};

+#define REGS_TTCFGR 0x080 /* Temperature Configuration Register */
+#define REGS_TSCFGR 0x084 /* Sensor Configuration Register */
+
+#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
+ * Site Register
+ */
+#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
+ * Control Register
+ */
/*
* Thermal zone data
*/
struct qoriq_tmu_data {
struct thermal_zone_device *tz;
- struct qoriq_tmu_regs __iomem *regs;
+ struct regmap *regmap;
int sensor_id;
- bool little_endian;
};

-static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
-{
- if (p->little_endian)
- iowrite32(val, addr);
- else
- iowrite32be(val, addr);
-}
-
-static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
-{
- if (p->little_endian)
- return ioread32(addr);
- else
- return ioread32be(addr);
-}
-
static int tmu_get_temp(void *p, int *temp)
{
u32 val;
struct qoriq_tmu_data *data = p;

- val = tmu_read(data, &data->regs->site[data->sensor_id].tritsr);
+ regmap_read(data->regmap, REGS_TRITSR(data->sensor_id), &val);
*temp = (val & 0xff) * 1000;

return 0;
@@ -146,10 +109,8 @@ static int qoriq_tmu_calibration(struct device *dev,
}

/* Init temperature range registers */
- tmu_write(data, range[0], &data->regs->ttr0cr);
- tmu_write(data, range[1], &data->regs->ttr1cr);
- tmu_write(data, range[2], &data->regs->ttr2cr);
- tmu_write(data, range[3], &data->regs->ttr3cr);
+ for (i = 0; i < ARRAY_SIZE(range); i++)
+ regmap_write(data->regmap, REGS_TTRnCR(i), range[i]);

calibration = of_get_property(np, "fsl,tmu-calibration", &len);
if (calibration == NULL || len % 8) {
@@ -159,9 +120,9 @@ static int qoriq_tmu_calibration(struct device *dev,

for (i = 0; i < len; i += 8, calibration += 2) {
val = of_read_number(calibration, 1);
- tmu_write(data, val, &data->regs->ttcfgr);
+ regmap_write(data->regmap, REGS_TTCFGR, val);
val = of_read_number(calibration + 1, 1);
- tmu_write(data, val, &data->regs->tscfgr);
+ regmap_write(data->regmap, REGS_TSCFGR, val);
}

return 0;
@@ -170,19 +131,40 @@ static int qoriq_tmu_calibration(struct device *dev,
static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
{
/* Disable interrupt, using polling instead */
- tmu_write(data, TIER_DISABLE, &data->regs->tier);
+ regmap_write(data->regmap, REGS_TIER, TIER_DISABLE);

/* Set update_interval */
- tmu_write(data, TMTMIR_DEFAULT, &data->regs->tmtmir);
+ regmap_write(data->regmap, REGS_TMTMIR, TMTMIR_DEFAULT);

/* Disable monitoring */
- tmu_write(data, TMR_DISABLE, &data->regs->tmr);
+ regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
}

static const struct thermal_zone_of_device_ops tmu_tz_ops = {
.get_temp = tmu_get_temp,
};

+static const struct regmap_range qiriq_wr_yes_ranges[] = {
+ regmap_reg_range(REGS_TMR, REGS_TSCFGR),
+ regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
+};
+
+static const struct regmap_access_table qiriq_wr_table = {
+ .yes_ranges = qiriq_wr_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(qiriq_wr_yes_ranges),
+};
+
+static const struct regmap_range qiriq_rd_yes_ranges[] = {
+ regmap_reg_range(REGS_TMR, REGS_TSCFGR),
+ regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
+ regmap_reg_range(REGS_TRITSR(0), REGS_TRITSR(15)),
+};
+
+static const struct regmap_access_table qiriq_rd_table = {
+ .yes_ranges = qiriq_rd_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(qiriq_rd_yes_ranges),
+};
+
static int qoriq_tmu_probe(struct platform_device *pdev)
{
int ret;
@@ -190,6 +172,19 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
struct resource *io;
+ const bool little_endian = of_property_read_bool(np, "little-endian");
+ const enum regmap_endian format_endian =
+ little_endian ? REGMAP_ENDIAN_LITTLE : REGMAP_ENDIAN_BIG;
+ const struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .rd_table = &qiriq_rd_table,
+ .wr_table = &qiriq_wr_table,
+ .val_format_endian = format_endian,
+ .max_register = SZ_4K,
+ };
+ void __iomem *base;
u32 site;

data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
@@ -197,8 +192,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (!data)
return -ENOMEM;

- data->little_endian = of_property_read_bool(np, "little-endian");
-
data->sensor_id = qoriq_tmu_get_sensor_id();
if (data->sensor_id < 0) {
dev_err(dev, "Failed to get sensor id\n");
@@ -211,12 +204,19 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
return -ENODEV;
}

- data->regs = devm_ioremap(dev, io->start, resource_size(io));
- if (!data->regs) {
+ base = devm_ioremap(dev, io->start, resource_size(io));
+ if (!base) {
dev_err(dev, "Failed to get memory region\n");
return -ENODEV;
}

+ data->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
+ if (IS_ERR(data->regmap)) {
+ ret = PTR_ERR(data->regmap);
+ dev_err(dev, "Failed to init regmap (%d)\n", ret);
+ return ret;
+ }
+
qoriq_tmu_init_device(data); /* TMU initialization */

ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
@@ -241,7 +241,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)

/* Enable monitoring */
site = 0x1 << (15 - data->sensor_id);
- tmu_write(data, site | TMR_ME | TMR_ALPF, &data->regs->tmr);
+ regmap_write(data->regmap, REGS_TMR, site | TMR_ME | TMR_ALPF);

return 0;
}
@@ -251,7 +251,7 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
struct qoriq_tmu_data *data = platform_get_drvdata(pdev);

/* Disable monitoring */
- tmu_write(data, TMR_DISABLE, &data->regs->tmr);
+ regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);

platform_set_drvdata(pdev, NULL);

@@ -259,30 +259,22 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
}

#ifdef CONFIG_PM_SLEEP
-static int qoriq_tmu_suspend(struct device *dev)
+
+static int qoriq_tmu_suspend_resume(struct device *dev, unsigned int val)
{
- u32 tmr;
struct qoriq_tmu_data *data = dev_get_drvdata(dev);

- /* Disable monitoring */
- tmr = tmu_read(data, &data->regs->tmr);
- tmr &= ~TMR_ME;
- tmu_write(data, tmr, &data->regs->tmr);
+ return regmap_update_bits(data->regmap, REGS_TMR, TMR_ME, val);
+}

- return 0;
+static int qoriq_tmu_suspend(struct device *dev)
+{
+ return qoriq_tmu_suspend_resume(dev, 0);
}

static int qoriq_tmu_resume(struct device *dev)
{
- u32 tmr;
- struct qoriq_tmu_data *data = dev_get_drvdata(dev);
-
- /* Enable monitoring */
- tmr = tmu_read(data, &data->regs->tmr);
- tmr |= TMR_ME;
- tmu_write(data, tmr, &data->regs->tmr);
-
- return 0;
+ return qoriq_tmu_suspend_resume(dev, TMR_ME);
}
#endif

--
2.20.1


2019-02-18 19:34:02

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 01/12] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()

Add devres wrapper for thermal_add_hwmon_sysfs() to simplify driver
code.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/thermal_hwmon.c | 28 ++++++++++++++++++++++++++++
drivers/thermal/thermal_hwmon.h | 7 +++++++
2 files changed, 35 insertions(+)

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 40c69a533b24..4e79524182e1 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -244,3 +244,31 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
kfree(hwmon);
}
EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
+
+static void devm_thermal_hwmon_release(struct device *dev, void *res)
+{
+ thermal_remove_hwmon_sysfs(*(struct thermal_zone_device **)res);
+}
+
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+ struct thermal_zone_device **ptr;
+ int ret;
+
+ ptr = devres_alloc(devm_thermal_hwmon_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ ret = thermal_add_hwmon_sysfs(tz);
+ if (ret) {
+ devres_free(ptr);
+ return ret;
+ }
+
+ *ptr = tz;
+ devres_add(&tz->device, ptr);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_add_hwmon_sysfs);
diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
index a160b9d62dd0..1a9d65f6a6a8 100644
--- a/drivers/thermal/thermal_hwmon.h
+++ b/drivers/thermal/thermal_hwmon.h
@@ -17,6 +17,7 @@

#ifdef CONFIG_THERMAL_HWMON
int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
#else
static inline int
@@ -25,6 +26,12 @@ thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
return 0;
}

+static inline int
+devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+ return 0;
+}
+
static inline void
thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
{
--
2.20.1


2019-02-18 19:34:24

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 06/12] thermal: qoriq: Add hwmon support

Expose thermal readings as a HWMON device, so that it could be
accessed using lm-sensors.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 472594fca03b..90af4c4caa52 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -11,6 +11,7 @@
#include <linux/thermal.h>

#include "thermal_core.h"
+#include "thermal_hwmon.h"

#define SITES_MAX 16

@@ -232,6 +233,10 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
return ret;
}

+ ret = devm_thermal_add_hwmon_sysfs(data->tz);
+ if (ret)
+ return ret;
+
platform_set_drvdata(pdev, data);

/* Enable monitoring */
--
2.20.1


2019-02-18 19:34:36

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 04/12] thermal: qoriq: Don't pass platform_device to qoriq_tmu_calibration()

We can simplify error cleanup code if instead of passing a "struct
platform_device *" to qoriq_tmu_calibration() and deriving a bunch of
pointers from it, we pass those pointers directly. This way we won't
be force to call platform_set_drvdata() as early in qoriq_tmu_probe()
and consequently would be able to drop the "err_iomap" error path.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index c24629b1b8c2..9bae001e8264 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -131,16 +131,16 @@ static int qoriq_tmu_get_sensor_id(void)
return id;
}

-static int qoriq_tmu_calibration(struct platform_device *pdev)
+static int qoriq_tmu_calibration(struct device *dev,
+ struct qoriq_tmu_data *data)
{
int i, val, len;
u32 range[4];
const u32 *calibration;
- struct device_node *np = pdev->dev.of_node;
- struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
+ struct device_node *np = dev->of_node;

if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
- dev_err(&pdev->dev, "missing calibration range.\n");
+ dev_err(dev, "missing calibration range.\n");
return -ENODEV;
}

@@ -152,7 +152,7 @@ static int qoriq_tmu_calibration(struct platform_device *pdev)

calibration = of_get_property(np, "fsl,tmu-calibration", &len);
if (calibration == NULL || len % 8) {
- dev_err(&pdev->dev, "invalid calibration data.\n");
+ dev_err(dev, "invalid calibration data.\n");
return -ENODEV;
}

@@ -195,27 +195,23 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (!data)
return -ENOMEM;

- platform_set_drvdata(pdev, data);
-
data->little_endian = of_property_read_bool(np, "little-endian");

data->sensor_id = qoriq_tmu_get_sensor_id();
if (data->sensor_id < 0) {
dev_err(dev, "Failed to get sensor id\n");
- ret = -ENODEV;
- goto err_iomap;
+ return -ENODEV;
}

data->regs = of_iomap(np, 0);
if (!data->regs) {
dev_err(dev, "Failed to get memory region\n");
- ret = -ENODEV;
- goto err_iomap;
+ return -ENODEV;
}

qoriq_tmu_init_device(data); /* TMU initialization */

- ret = qoriq_tmu_calibration(pdev); /* TMU calibration */
+ ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
goto err_tmu;

@@ -229,6 +225,8 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
goto err_tmu;
}

+ platform_set_drvdata(pdev, data);
+
/* Enable monitoring */
site = 0x1 << (15 - data->sensor_id);
tmu_write(data, site | TMR_ME | TMR_ALPF, &data->regs->tmr);
@@ -238,9 +236,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
err_tmu:
iounmap(data->regs);

-err_iomap:
- platform_set_drvdata(pdev, NULL);
-
return ret;
}

--
2.20.1


2019-02-18 19:35:14

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 03/12] thermal: qoriq: Add local struct device pointer in qoriq_tmu_probe()

Use a local "struct device *dev" for brevity. No functional change
intended.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/thermal/qoriq_thermal.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 3e147f856bf5..c24629b1b8c2 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -187,9 +187,10 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
int ret;
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
u32 site;

- data = devm_kzalloc(&pdev->dev, sizeof(struct qoriq_tmu_data),
+ data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -200,14 +201,14 @@ static int qoriq_tmu_probe(struct platform_device *pdev)

data->sensor_id = qoriq_tmu_get_sensor_id();
if (data->sensor_id < 0) {
- dev_err(&pdev->dev, "Failed to get sensor id\n");
+ dev_err(dev, "Failed to get sensor id\n");
ret = -ENODEV;
goto err_iomap;
}

data->regs = of_iomap(np, 0);
if (!data->regs) {
- dev_err(&pdev->dev, "Failed to get memory region\n");
+ dev_err(dev, "Failed to get memory region\n");
ret = -ENODEV;
goto err_iomap;
}
@@ -218,13 +219,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (ret < 0)
goto err_tmu;

- data->tz = devm_thermal_zone_of_sensor_register(&pdev->dev,
+ data->tz = devm_thermal_zone_of_sensor_register(dev,
data->sensor_id,
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);
+ dev_err(dev, "Failed to register thermal zone device %d\n",
+ ret);
goto err_tmu;
}

--
2.20.1


2019-02-21 00:53:28

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCH 00/12] QorIQ TMU multi-sensor and HWMON support

Hey Andrey

On Mon, Feb 18, 2019 at 11:11:29AM -0800, Andrey Smirnov wrote:
> Everyone:
>
> This series contains patches adding support for HWMON integration,
> multi-sensor support as well as a small fix and general improvements
> (hopefully) for TMU driver I made while working on it on i.MX8MQ.
>
> Feedback is welcome!

What branch is this patch series based off?

Would you mind re-sending the series based off my -linus branch?
There has been an addition on qoriq driver to support multiple sensors
and the code has shifted and your series does not apply over there.

>
> Thanks,
> Andrey Smirnov
>
> Andrey Smirnov (12):
> thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()
> thermal: qoriq: Remove unnecessary DT node is NULL check
> thermal: qoriq: Add local struct device pointer in qoriq_tmu_probe()
> thermal: qoriq: Don't pass platform_device to qoriq_tmu_calibration()
> thermal: qoriq: Convert driver to use devm_ioremap()
> thermal: qoriq: Add hwmon support
> thermal: qoriq: Convert driver to use regmap API
> thermal: qoriq: Enable monitoring before querying temperature
> thermal: qoriq: Do not report invalid temperature reading
> thermal: qoriq: Simplify error handling in qoriq_tmu_get_sensor_id()
> thermal: qoriq: Be more strict when parsing "thermal-sensors"
> thermal: qoriq: Add support for multiple thremal sites
>
> drivers/thermal/qoriq_thermal.c | 326 ++++++++++++++++++--------------
> drivers/thermal/thermal_hwmon.c | 28 +++
> drivers/thermal/thermal_hwmon.h | 7 +
> 3 files changed, 214 insertions(+), 147 deletions(-)
>
> --
> 2.20.1
>

2019-02-21 00:58:00

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCH 12/12] thermal: qoriq: Add support for multiple thremal sites

On Mon, Feb 18, 2019 at 11:11:41AM -0800, Andrey Smirnov wrote:
> TMU IP block provides temerature measurement for up to 16 sites,
> current implementation of the driver however hardcodes a single site
> ID. Change the code so it would be possible to reference multiple
> sites as indivudial sensors and get their temperature readings.

small typo: indivudial/individual.

>
> Signed-off-by: Andrey Smirnov <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: Eduardo Valentin <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/thermal/qoriq_thermal.c | 132 ++++++++++++++++++++------------
> 1 file changed, 84 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index f746c62789b0..6cc6e6b36fb0 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -24,6 +24,8 @@
> #define TMR_DISABLE 0x0
> #define TMR_ME 0x80000000
> #define TMR_ALPF 0x0c000000
> +#define TMR_ALPF_MASK GENMASK(27, 26)
> +
>
> #define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
> #define TMTMIR_DEFAULT 0x0000000f
> @@ -41,21 +43,32 @@
> #define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
> * Control Register
> */
> +struct qoriq_tmu_sensor {
> + struct thermal_zone_device *tz;
> + int id;
> +};
> +
> /*
> * Thermal zone data
> */
> struct qoriq_tmu_data {
> struct thermal_zone_device *tz;
> struct regmap *regmap;
> - int sensor_id;
> + struct qoriq_tmu_sensor sensors[SITES_MAX];
> };
>
> +static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_tmu_sensor *s)
> +{
> + return container_of(s, struct qoriq_tmu_data, sensors[s->id]);
> +}
> +
> static int tmu_get_temp(void *p, int *temp)
> {
> u32 val;
> - struct qoriq_tmu_data *data = p;
> + struct qoriq_tmu_sensor *s = p;
> + struct qoriq_tmu_data *data = qoriq_sensor_to_data(s);
>
> - regmap_read(data->regmap, REGS_TRITSR(data->sensor_id), &val);
> + regmap_read(data->regmap, REGS_TRITSR(s->id), &val);
> if (!(val & TRITSR_V))
> return -ENODATA;
>
> @@ -63,38 +76,86 @@ static int tmu_get_temp(void *p, int *temp)
> return 0;
> }
>
> -static int qoriq_tmu_get_sensor_id(void)
> +static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> + .get_temp = tmu_get_temp,
> +};
> +
> +static int qoriq_tmu_populate_sensors(struct device *dev,
> + struct qoriq_tmu_data *data)
> {
> - int ret, id;
> + int ret, id, i, count = 0;
> struct of_phandle_args sensor_specs;
> struct device_node *np, *sensor_np;
> + struct qoriq_tmu_sensor *s;
>
> np = of_find_node_by_name(NULL, "thermal-zones");
> if (!np)
> return -ENODEV;
>
> - sensor_np = of_get_next_child(np, NULL);
> - ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> - "#thermal-sensor-cells",
> - 0, &sensor_specs);
> - if (ret) {
> - id = ret;
> - goto out;
> + for_each_child_of_node(np, sensor_np) {
> + u16 msite;
> +
> + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> + "#thermal-sensor-cells",
> + 0, &sensor_specs);
> + if (ret)
> + break;
> +
> + if (sensor_specs.args_count != 1) {
> + pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
> + sensor_specs.np, sensor_specs.args_count);
> + ret = -EINVAL;
> + break;
> + }
> +
> + id = sensor_specs.args[0];
> + if (id >= SITES_MAX) {
> + ret = -EINVAL;
> + dev_err(dev, "Sensor id %d is out of range\n", id);
> + break;
> + }
> +
> + msite = BIT(15 - id);
> + /* Enable monitoring of that particular sensor*/
> + regmap_update_bits(data->regmap, REGS_TMR, msite, msite);
> +
> + s = &data->sensors[id];
> + s->id = id;
> + count++;

Not sure I follow why you need to bother about thermal-sensor-cells..

> }
>
> - if (sensor_specs.args_count != 1) {
> - pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
> - sensor_specs.np, sensor_specs.args_count);
> - id = -EINVAL;
> - goto out;
> + of_node_put(np);
> + if (ret) {
> + /* We only need to "put" sensor_np if we exited the
> + * loop early with "break"
> + */
> + of_node_put(sensor_np);
> + return ret;
> }
>
> - id = sensor_specs.args[0];
> -out:
> - of_node_put(np);
> - of_node_put(sensor_np);
> + /* Enable monitoring */
> + regmap_update_bits(data->regmap, REGS_TMR, TMR_ME | TMR_ALPF_MASK,
> + TMR_ME | TMR_ALPF);
> +
> + for (i = 0; i < count; i++) {
> + s = &data->sensors[i];
> + s->tz = devm_thermal_zone_of_sensor_register(dev, s->id, s,
> + &tmu_tz_ops);
> + if (IS_ERR(s->tz)) {
> + ret = PTR_ERR(s->tz);
> + dev_err(dev,
> + "Failed to register thermal zone device %d\n",
> + ret);
> + break;
> + }
> +
> + ret = devm_thermal_add_hwmon_sysfs(s->tz);
> + if (ret)
> + break;
> +
> + }
>
> - return id;
> + return ret;
> }
>
> static int qoriq_tmu_calibration(struct device *dev,
> @@ -142,10 +203,6 @@ static void qoriq_tmu_init_device(struct qoriq_tmu_data *data)
> regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);
> }
>
> -static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> - .get_temp = tmu_get_temp,
> -};
> -
> static const struct regmap_range qiriq_wr_yes_ranges[] = {
> regmap_reg_range(REGS_TMR, REGS_TSCFGR),
> regmap_reg_range(REGS_TTRnCR(0), REGS_TTRnCR(3)),
> @@ -187,19 +244,12 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> .max_register = SZ_4K,
> };
> void __iomem *base;
> - u32 site;
>
> data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
> GFP_KERNEL);
> if (!data)
> return -ENOMEM;
>
> - data->sensor_id = qoriq_tmu_get_sensor_id();
> - if (data->sensor_id < 0) {
> - dev_err(dev, "Failed to get sensor id\n");
> - return -ENODEV;
> - }
> -
> io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!io) {
> dev_err(dev, "Failed to get memory region\n");
> @@ -225,21 +275,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - /* Enable monitoring */
> - site = 0x1 << (15 - data->sensor_id);
> - regmap_write(data->regmap, REGS_TMR, site | TMR_ME | TMR_ALPF);
> -
> - data->tz = devm_thermal_zone_of_sensor_register(dev,
> - data->sensor_id,
> - data, &tmu_tz_ops);
> - if (IS_ERR(data->tz)) {
> - ret = PTR_ERR(data->tz);
> - dev_err(dev, "Failed to register thermal zone device %d\n",
> - ret);
> - goto disable_monitoring;
> - }
> -
> - ret = devm_thermal_add_hwmon_sysfs(data->tz);
> + ret = qoriq_tmu_populate_sensors(dev, data);
> if (ret)
> goto disable_monitoring;
>
> --
> 2.20.1
>

2019-02-21 18:48:14

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH 00/12] QorIQ TMU multi-sensor and HWMON support

On Wed, Feb 20, 2019 at 4:52 PM Eduardo Valentin <[email protected]> wrote:
>
> Hey Andrey
>
> On Mon, Feb 18, 2019 at 11:11:29AM -0800, Andrey Smirnov wrote:
> > Everyone:
> >
> > This series contains patches adding support for HWMON integration,
> > multi-sensor support as well as a small fix and general improvements
> > (hopefully) for TMU driver I made while working on it on i.MX8MQ.
> >
> > Feedback is welcome!
>
> What branch is this patch series based off?
>

This was based on v5.0-rc6

> Would you mind re-sending the series based off my -linus branch?
> There has been an addition on qoriq driver to support multiple sensors
> and the code has shifted and your series does not apply over there.
>

Sure, will do in v2.

Thanks,
Andrey Smirnov

2019-02-21 18:54:46

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH 12/12] thermal: qoriq: Add support for multiple thremal sites

On Wed, Feb 20, 2019 at 4:57 PM Eduardo Valentin <[email protected]> wrote:
>
> On Mon, Feb 18, 2019 at 11:11:41AM -0800, Andrey Smirnov wrote:
> > TMU IP block provides temerature measurement for up to 16 sites,
> > current implementation of the driver however hardcodes a single site
> > ID. Change the code so it would be possible to reference multiple
> > sites as indivudial sensors and get their temperature readings.
>
> small typo: indivudial/individual.
>
> >
> > Signed-off-by: Andrey Smirnov <[email protected]>
> > Cc: Chris Healy <[email protected]>
> > Cc: Lucas Stach <[email protected]>
> > Cc: Zhang Rui <[email protected]>
> > Cc: Eduardo Valentin <[email protected]>
> > Cc: Daniel Lezcano <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/thermal/qoriq_thermal.c | 132 ++++++++++++++++++++------------
> > 1 file changed, 84 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index f746c62789b0..6cc6e6b36fb0 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -24,6 +24,8 @@
> > #define TMR_DISABLE 0x0
> > #define TMR_ME 0x80000000
> > #define TMR_ALPF 0x0c000000
> > +#define TMR_ALPF_MASK GENMASK(27, 26)
> > +
> >
> > #define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
> > #define TMTMIR_DEFAULT 0x0000000f
> > @@ -41,21 +43,32 @@
> > #define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
> > * Control Register
> > */
> > +struct qoriq_tmu_sensor {
> > + struct thermal_zone_device *tz;
> > + int id;
> > +};
> > +
> > /*
> > * Thermal zone data
> > */
> > struct qoriq_tmu_data {
> > struct thermal_zone_device *tz;
> > struct regmap *regmap;
> > - int sensor_id;
> > + struct qoriq_tmu_sensor sensors[SITES_MAX];
> > };
> >
> > +static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_tmu_sensor *s)
> > +{
> > + return container_of(s, struct qoriq_tmu_data, sensors[s->id]);
> > +}
> > +
> > static int tmu_get_temp(void *p, int *temp)
> > {
> > u32 val;
> > - struct qoriq_tmu_data *data = p;
> > + struct qoriq_tmu_sensor *s = p;
> > + struct qoriq_tmu_data *data = qoriq_sensor_to_data(s);
> >
> > - regmap_read(data->regmap, REGS_TRITSR(data->sensor_id), &val);
> > + regmap_read(data->regmap, REGS_TRITSR(s->id), &val);
> > if (!(val & TRITSR_V))
> > return -ENODATA;
> >
> > @@ -63,38 +76,86 @@ static int tmu_get_temp(void *p, int *temp)
> > return 0;
> > }
> >
> > -static int qoriq_tmu_get_sensor_id(void)
> > +static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> > + .get_temp = tmu_get_temp,
> > +};
> > +
> > +static int qoriq_tmu_populate_sensors(struct device *dev,
> > + struct qoriq_tmu_data *data)
> > {
> > - int ret, id;
> > + int ret, id, i, count = 0;
> > struct of_phandle_args sensor_specs;
> > struct device_node *np, *sensor_np;
> > + struct qoriq_tmu_sensor *s;
> >
> > np = of_find_node_by_name(NULL, "thermal-zones");
> > if (!np)
> > return -ENODEV;
> >
> > - sensor_np = of_get_next_child(np, NULL);
> > - ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > - "#thermal-sensor-cells",
> > - 0, &sensor_specs);
> > - if (ret) {
> > - id = ret;
> > - goto out;
> > + for_each_child_of_node(np, sensor_np) {
> > + u16 msite;
> > +
> > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > + "#thermal-sensor-cells",
> > + 0, &sensor_specs);
> > + if (ret)
> > + break;
> > +
> > + if (sensor_specs.args_count != 1) {
> > + pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
> > + sensor_specs.np, sensor_specs.args_count);
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + id = sensor_specs.args[0];
> > + if (id >= SITES_MAX) {
> > + ret = -EINVAL;
> > + dev_err(dev, "Sensor id %d is out of range\n", id);
> > + break;
> > + }
> > +
> > + msite = BIT(15 - id);
> > + /* Enable monitoring of that particular sensor*/
> > + regmap_update_bits(data->regmap, REGS_TMR, msite, msite);
> > +
> > + s = &data->sensors[id];
> > + s->id = id;
> > + count++;
>
> Not sure I follow why you need to bother about thermal-sensor-cells..
>

devm_thermal_zone_of_sensor_register() will call
.get_temp()/tmu_get_temp(), so all present sensors need to be enabled
before that to avoid returning bogus data. The first loop going over
thermal-sensor-cells does exactly that as well as count # of sensors
present to avoid having to loop over all 16 possible sensors in the
code that follow.

Given how there's already a patch adding multiple sensors support, not
a lot of this patch will probably survive, so we can pretty much
disregard it.

Thanks,
Andrey Smirnov