Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311437 > unrolled thread

[PATCH V3 00/11] Add T210 support in Tegra soctherm

Back to article view | Back to linux.kernel

Contents

  [PATCH V3 00/11] Add T210 support in Tegra soctherm Wei Ni <wni@nvidia.com> - 2016-01-18 11:10 +0100
    [PATCH V3 02/11] thermal: tegra: combine sensor group-related data Wei Ni <wni@nvidia.com> - 2016-01-18 11:10 +0100
    [PATCH V3 03/11] thermal: tegra: get rid of PDIV/HOTSPOT hack Wei Ni <wni@nvidia.com> - 2016-01-18 11:10 +0100
    [PATCH V3 01/11] thermal: tegra: move tegra thermal files into tegra directory Wei Ni <wni@nvidia.com> - 2016-01-18 11:10 +0100
    Re: [PATCH V3 04/11] thermal: tegra: split tegra_soctherm driver Thierry Reding <thierry.reding@gmail.com> - 2016-01-21 15:50 +0100
      Re: [PATCH V3 04/11] thermal: tegra: split tegra_soctherm driver Wei Ni <wni@nvidia.com> - 2016-01-22 10:00 +0100
    Re: [PATCH V3 00/11] Add T210 support in Tegra soctherm Thierry Reding <thierry.reding@gmail.com> - 2016-01-21 16:00 +0100
      Re: [PATCH V3 00/11] Add T210 support in Tegra soctherm Wei Ni <wni@nvidia.com> - 2016-01-22 08:30 +0100

#1311437 — [PATCH V3 00/11] Add T210 support in Tegra soctherm

This patchset adds following functions for tegra_soctherm driver:
1. add T210 support.
2. export debugfs to show some registers.
3. add thermtrip funciton.
4. add suspend/resume function.

The V1 serial is in:
http://www.spinics.net/lists/linux-tegra/msg24808.html
The V2 serial is in:
http://www.spinics.net/lists/linux-tegra/msg24901.html

Main changes from V2:
1. Fix build error in patch [1/11].
2. Use of_get_child_by_name instead of of_find_node_by_name in patch [8/11].
3. Use debugfs_remove_recursive to remove debugfs in patch [6/11].

Main changes from V1:
1. Use the new type to handl different Tegra chips in one driver,
which suggested by Thierry.
2. Changes per Thieery's other comments.

Wei Ni (11):
  thermal: tegra: move tegra thermal files into tegra directory
  thermal: tegra: combine sensor group-related data
  thermal: tegra: get rid of PDIV/HOTSPOT hack
  thermal: tegra: split tegra_soctherm driver
  thermal: tegra: add T210-specific SOC_THERM driver
  thermal: tegra: add a debugfs to show registers
  of: Add bindings of hw-trips for soctherm
  thermal: tegra: add thermtrip function
  thermal: tegra: add PM support
  arm64: tegra: add soctherm node for Tegra210
  ARM: tegra: set hw trips for Tegra124

 .../devicetree/bindings/thermal/tegra-soctherm.txt |  24 +
 arch/arm/boot/dts/tegra124.dtsi                    |   9 +
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  53 ++
 drivers/thermal/Kconfig                            |  12 +-
 drivers/thermal/Makefile                           |   2 +-
 drivers/thermal/tegra/Kconfig                      |  13 +
 drivers/thermal/tegra/Makefile                     |   5 +
 drivers/thermal/tegra/soctherm-fuse.c              | 155 ++++
 drivers/thermal/tegra/soctherm.c                   | 789 +++++++++++++++++++++
 drivers/thermal/tegra/soctherm.h                   | 125 ++++
 drivers/thermal/tegra/tegra124-soctherm.c          | 207 ++++++
 drivers/thermal/tegra/tegra210-soctherm.c          | 208 ++++++
 drivers/thermal/tegra_soctherm.c                   | 476 -------------
 include/dt-bindings/thermal/tegra124-soctherm.h    |   1 +
 14 files changed, 1592 insertions(+), 487 deletions(-)
 create mode 100644 drivers/thermal/tegra/Kconfig
 create mode 100644 drivers/thermal/tegra/Makefile
 create mode 100644 drivers/thermal/tegra/soctherm-fuse.c
 create mode 100644 drivers/thermal/tegra/soctherm.c
 create mode 100644 drivers/thermal/tegra/soctherm.h
 create mode 100644 drivers/thermal/tegra/tegra124-soctherm.c
 create mode 100644 drivers/thermal/tegra/tegra210-soctherm.c
 delete mode 100644 drivers/thermal/tegra_soctherm.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1311446 — [PATCH V3 02/11] thermal: tegra: combine sensor group-related data

Combine sensor group-related data structures into struct
tegra_tsensor_group. This provides a single location for
sensor group data storage.
More sensor group data will be added in subsequent patches.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/tegra-soctherm.c          | 147 +++++++++++++++++++-----
 include/dt-bindings/thermal/tegra124-soctherm.h |   1 +
 2 files changed, 121 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/tegra/tegra-soctherm.c b/drivers/thermal/tegra/tegra-soctherm.c
index 74ea5765938b..6ef3a7dec1d8 100644
--- a/drivers/thermal/tegra/tegra-soctherm.c
+++ b/drivers/thermal/tegra/tegra-soctherm.c
@@ -28,6 +28,7 @@
 #include <linux/thermal.h>
 
 #include <soc/tegra/fuse.h>
+#include <dt-bindings/thermal/tegra124-soctherm.h>
 
 #define SENSOR_CONFIG0				0
 #define SENSOR_CONFIG0_STOP			BIT(0)
@@ -48,12 +49,24 @@
 
 #define SENSOR_PDIV				0x1c0
 #define SENSOR_PDIV_T124			0x8888
+#define SENSOR_PDIV_CPU_MASK			(0xf << 12)
+#define SENSOR_PDIV_GPU_MASK			(0xf << 8)
+#define SENSOR_PDIV_MEM_MASK			(0xf << 4)
+#define SENSOR_PDIV_PLLX_MASK			(0xf << 0)
+
 #define SENSOR_HOTSPOT_OFF			0x1c4
 #define SENSOR_HOTSPOT_OFF_T124			0x00060600
+#define SENSOR_HOTSPOT_CPU_MASK			(0xff << 16)
+#define SENSOR_HOTSPOT_GPU_MASK			(0xff << 8)
+#define SENSOR_HOTSPOT_MEM_MASK			(0xff << 0)
+
 #define SENSOR_TEMP1				0x1c8
+#define SENSOR_TEMP1_CPU_TEMP_MASK		(0xffff << 16)
+#define SENSOR_TEMP1_GPU_TEMP_MASK		0xffff
 #define SENSOR_TEMP2				0x1cc
+#define SENSOR_TEMP2_MEM_TEMP_MASK		(0xffff << 16)
+#define SENSOR_TEMP2_PLLX_TEMP_MASK		0xffff
 
-#define SENSOR_TEMP_MASK			0xffff
 #define READBACK_VALUE_MASK			0xff00
 #define READBACK_VALUE_SHIFT			8
 #define READBACK_ADD_HALF			BIT(7)
@@ -77,8 +90,36 @@
 #define NOMINAL_CALIB_FT_T124			105
 #define NOMINAL_CALIB_CP_T124			25
 
+/* get val from register(r) mask bits(m) */
+#define REG_GET_MASK(r, m)	(((r) & (m)) >> (ffs(m) - 1))
+/* set val(v) to mask bits(m) of register(r) */
+#define REG_SET_MASK(r, m, v)	(((r) & ~(m)) | \
+				 (((v) & (m >> (ffs(m) - 1))) << (ffs(m) - 1)))
+
+/**
+ * struct tegra_tsensor_group - SOC_THERM sensor group data
+ * @name: short name of the temperature sensor group
+ * @id: numeric ID of the temperature sensor group
+ * @sensor_temp_offset: offset of the SENSOR_TEMP* register
+ * @sensor_temp_mask: bit mask for this sensor group in SENSOR_TEMP* register
+ * @pdiv: the sensor count post-divider to use during runtime
+ * @pdiv_ate: the sensor count post-divider used during automated test
+ * @pdiv_mask: register bitfield mask for the PDIV field for this sensor
+ * @pllx_hotspot_diff: hotspot offset from the PLLX sensor, must be 0 for
+    PLLX sensor group
+ * @pllx_hotspot_mask: register bitfield mask for the HOTSPOT field
+ */
+struct tegra_tsensor_group {
+	const char	*name;
+	u8		id;
+	u16		sensor_temp_offset;
+	u32		sensor_temp_mask;
+	u32		pdiv, pdiv_ate, pdiv_mask;
+	u32		pllx_hotspot_diff, pllx_hotspot_mask;
+};
+
 struct tegra_tsensor_configuration {
-	u32 tall, tsample, tiddq_en, ten_count, pdiv, tsample_ate, pdiv_ate;
+	u32 tall, tiddq_en, ten_count, tsample, tsample_ate;
 };
 
 struct tegra_tsensor {
@@ -86,21 +127,74 @@ struct tegra_tsensor {
 	u32 base, calib_fuse_offset;
 	/* Correction values used to modify values read from calibration fuses */
 	s32 fuse_corr_alpha, fuse_corr_beta;
+	const struct tegra_tsensor_group *group;
 };
 
 struct tegra_thermctl_zone {
 	void __iomem *reg;
-	unsigned int shift;
+	u32 mask;
 };
 
 static const struct tegra_tsensor_configuration t124_tsensor_config = {
 	.tall = 16300,
-	.tsample = 120,
 	.tiddq_en = 1,
 	.ten_count = 1,
-	.pdiv = 8,
+	.tsample = 120,
 	.tsample_ate = 480,
-	.pdiv_ate = 8
+};
+
+static const struct tegra_tsensor_group tegra124_tsensor_group_cpu = {
+	.id				= TEGRA124_SOCTHERM_SENSOR_CPU,
+	.name				= "cpu",
+	.sensor_temp_offset		= SENSOR_TEMP1,
+	.sensor_temp_mask		= SENSOR_TEMP1_CPU_TEMP_MASK,
+	.pdiv				= 8,
+	.pdiv_ate			= 8,
+	.pdiv_mask			= SENSOR_PDIV_CPU_MASK,
+	.pllx_hotspot_diff		= 10,
+	.pllx_hotspot_mask		= SENSOR_HOTSPOT_CPU_MASK,
+};
+
+static const struct tegra_tsensor_group tegra124_tsensor_group_gpu = {
+	.id				= TEGRA124_SOCTHERM_SENSOR_GPU,
+	.name				= "gpu",
+	.sensor_temp_offset		= SENSOR_TEMP1,
+	.sensor_temp_mask		= SENSOR_TEMP1_GPU_TEMP_MASK,
+	.pdiv				= 8,
+	.pdiv_ate			= 8,
+	.pdiv_mask			= SENSOR_PDIV_GPU_MASK,
+	.pllx_hotspot_diff		= 5,
+	.pllx_hotspot_mask		= SENSOR_HOTSPOT_GPU_MASK,
+};
+
+static const struct tegra_tsensor_group tegra124_tsensor_group_pll = {
+	.id				= TEGRA124_SOCTHERM_SENSOR_PLLX,
+	.name				= "pll",
+	.sensor_temp_offset		= SENSOR_TEMP2,
+	.sensor_temp_mask		= SENSOR_TEMP2_PLLX_TEMP_MASK,
+	.pdiv				= 8,
+	.pdiv_ate			= 8,
+	.pdiv_mask			= SENSOR_PDIV_PLLX_MASK,
+	.pllx_hotspot_diff		= 0,
+	.pllx_hotspot_mask		= SENSOR_HOTSPOT_MEM_MASK,
+};
+
+static const struct tegra_tsensor_group tegra124_tsensor_group_mem = {
+	.id				= TEGRA124_SOCTHERM_SENSOR_MEM,
+	.name				= "mem",
+	.sensor_temp_offset		= SENSOR_TEMP2,
+	.sensor_temp_mask		= SENSOR_TEMP2_MEM_TEMP_MASK,
+	.pdiv				= 8,
+	.pdiv_ate			= 8,
+	.pdiv_mask			= SENSOR_PDIV_MEM_MASK,
+};
+
+static const struct tegra_tsensor_group *
+tegra124_tsensor_groups[TEGRA124_SOCTHERM_SENSOR_NUM] = {
+	&tegra124_tsensor_group_cpu,
+	&tegra124_tsensor_group_gpu,
+	&tegra124_tsensor_group_pll,
+	&tegra124_tsensor_group_mem,
 };
 
 static const struct tegra_tsensor t124_tsensors[] = {
@@ -110,6 +204,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x098,
 		.fuse_corr_alpha = 1135400,
 		.fuse_corr_beta = -6266900,
+		.group = &tegra124_tsensor_group_cpu,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -117,6 +212,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x084,
 		.fuse_corr_alpha = 1122220,
 		.fuse_corr_beta = -5700700,
+		.group = &tegra124_tsensor_group_cpu,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -124,6 +220,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x088,
 		.fuse_corr_alpha = 1127000,
 		.fuse_corr_beta = -6768200,
+		.group = &tegra124_tsensor_group_cpu,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -131,6 +228,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x12c,
 		.fuse_corr_alpha = 1110900,
 		.fuse_corr_beta = -6232000,
+		.group = &tegra124_tsensor_group_cpu,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -138,6 +236,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x158,
 		.fuse_corr_alpha = 1122300,
 		.fuse_corr_beta = -5936400,
+		.group = &tegra124_tsensor_group_mem,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -145,6 +244,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x15c,
 		.fuse_corr_alpha = 1145700,
 		.fuse_corr_beta = -7124600,
+		.group = &tegra124_tsensor_group_mem,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -152,6 +252,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x154,
 		.fuse_corr_alpha = 1120100,
 		.fuse_corr_beta = -6000500,
+		.group = &tegra124_tsensor_group_gpu,
 	},
 	{
 		.config = &t124_tsensor_config,
@@ -159,6 +260,7 @@ static const struct tegra_tsensor t124_tsensors[] = {
 		.calib_fuse_offset = 0x160,
 		.fuse_corr_alpha = 1106500,
 		.fuse_corr_beta = -6729300,
+		.group = &tegra124_tsensor_group_pll,
 	},
 };
 
@@ -168,7 +270,7 @@ struct tegra_soctherm {
 	struct clk *clock_soctherm;
 	void __iomem *regs;
 
-	struct thermal_zone_device *thermctl_tzs[4];
+	struct thermal_zone_device *thermctl_tzs[TEGRA124_SOCTHERM_SENSOR_NUM];
 };
 
 struct tsensor_shared_calibration {
@@ -237,8 +339,8 @@ calculate_tsensor_calibration(const struct tegra_tsensor *sensor,
 	delta_sens = actual_tsensor_ft - actual_tsensor_cp;
 	delta_temp = shared->actual_temp_ft - shared->actual_temp_cp;
 
-	mult = sensor->config->pdiv * sensor->config->tsample_ate;
-	div = sensor->config->tsample * sensor->config->pdiv_ate;
+	mult = sensor->group->pdiv * sensor->config->tsample_ate;
+	div = sensor->config->tsample * sensor->group->pdiv_ate;
 
 	therma = div64_s64_precise((s64) delta_temp * (1LL << 13) * mult,
 				   (s64) delta_sens * div);
@@ -311,7 +413,8 @@ static int tegra_thermctl_get_temp(void *data, int *out_temp)
 	struct tegra_thermctl_zone *zone = data;
 	u32 val;
 
-	val = (readl(zone->reg) >> zone->shift) & SENSOR_TEMP_MASK;
+	val = readl(zone->reg);
+	val = REG_GET_MASK(val, zone->mask);
 	*out_temp = translate_temp(val);
 
 	return 0;
@@ -327,18 +430,6 @@ static const struct of_device_id tegra_soctherm_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra_soctherm_of_match);
 
-struct thermctl_zone_desc {
-	unsigned int offset;
-	unsigned int shift;
-};
-
-static const struct thermctl_zone_desc t124_thermctl_temp_zones[] = {
-	{ SENSOR_TEMP1, 16 },
-	{ SENSOR_TEMP2, 16 },
-	{ SENSOR_TEMP1, 0 },
-	{ SENSOR_TEMP2, 0 }
-};
-
 static int tegra_soctherm_probe(struct platform_device *pdev)
 {
 	struct tegra_soctherm *tegra;
@@ -349,6 +440,7 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 	int err;
 
 	const struct tegra_tsensor *tsensors = t124_tsensors;
+	const struct tegra_tsensor_group **ttgs = tegra124_tsensor_groups;
 
 	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
 	if (!tegra)
@@ -408,7 +500,7 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 
 	/* Initialize thermctl sensors */
 
-	for (i = 0; i < ARRAY_SIZE(tegra->thermctl_tzs); ++i) {
+	for (i = 0; i < TEGRA124_SOCTHERM_SENSOR_NUM; ++i) {
 		struct tegra_thermctl_zone *zone =
 			devm_kzalloc(&pdev->dev, sizeof(*zone), GFP_KERNEL);
 		if (!zone) {
@@ -416,10 +508,11 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 			goto unregister_tzs;
 		}
 
-		zone->reg = tegra->regs + t124_thermctl_temp_zones[i].offset;
-		zone->shift = t124_thermctl_temp_zones[i].shift;
+		zone->reg = tegra->regs + ttgs[i]->sensor_temp_offset;
+		zone->mask = ttgs[i]->sensor_temp_mask;
 
-		tz = thermal_zone_of_sensor_register(&pdev->dev, i, zone,
+		tz = thermal_zone_of_sensor_register(&pdev->dev,
+						     ttgs[i]->id, zone,
 						     &tegra_of_thermal_ops);
 		if (IS_ERR(tz)) {
 			err = PTR_ERR(tz);
@@ -428,7 +521,7 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 			goto unregister_tzs;
 		}
 
-		tegra->thermctl_tzs[i] = tz;
+		tegra->thermctl_tzs[ttgs[i]->id] = tz;
 	}
 
 	return 0;
diff --git a/include/dt-bindings/thermal/tegra124-soctherm.h b/include/dt-bindings/thermal/tegra124-soctherm.h
index 85aaf66690f9..729ab9fc325e 100644
--- a/include/dt-bindings/thermal/tegra124-soctherm.h
+++ b/include/dt-bindings/thermal/tegra124-soctherm.h
@@ -9,5 +9,6 @@
 #define TEGRA124_SOCTHERM_SENSOR_MEM 1
 #define TEGRA124_SOCTHERM_SENSOR_GPU 2
 #define TEGRA124_SOCTHERM_SENSOR_PLLX 3
+#define TEGRA124_SOCTHERM_SENSOR_NUM 4
 
 #endif
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1311448 — [PATCH V3 03/11] thermal: tegra: get rid of PDIV/HOTSPOT hack

Get rid of T124-specific PDIV/HOTSPOT hack.
tegra-soctherm.c contained a hack to set the SENSOR_PDIV and
SENSOR_HOTSPOT_OFFSET registers - it just did two writes of
T124-specific opaque values.  Convert these into a form that can be
substituted on a per-chip basis, and into structure fields that have
at least some independent meaning.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/tegra-soctherm.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/tegra/tegra-soctherm.c b/drivers/thermal/tegra/tegra-soctherm.c
index 6ef3a7dec1d8..6c3d508929d7 100644
--- a/drivers/thermal/tegra/tegra-soctherm.c
+++ b/drivers/thermal/tegra/tegra-soctherm.c
@@ -48,14 +48,12 @@
 #define SENSOR_CONFIG2_THERMB_SHIFT		0
 
 #define SENSOR_PDIV				0x1c0
-#define SENSOR_PDIV_T124			0x8888
 #define SENSOR_PDIV_CPU_MASK			(0xf << 12)
 #define SENSOR_PDIV_GPU_MASK			(0xf << 8)
 #define SENSOR_PDIV_MEM_MASK			(0xf << 4)
 #define SENSOR_PDIV_PLLX_MASK			(0xf << 0)
 
 #define SENSOR_HOTSPOT_OFF			0x1c4
-#define SENSOR_HOTSPOT_OFF_T124			0x00060600
 #define SENSOR_HOTSPOT_CPU_MASK			(0xff << 16)
 #define SENSOR_HOTSPOT_GPU_MASK			(0xff << 8)
 #define SENSOR_HOTSPOT_MEM_MASK			(0xff << 0)
@@ -438,6 +436,7 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 	struct resource *res;
 	unsigned int i;
 	int err;
+	u32 pdiv, hotspot;
 
 	const struct tegra_tsensor *tsensors = t124_tsensors;
 	const struct tegra_tsensor_group **ttgs = tegra124_tsensor_groups;
@@ -495,8 +494,20 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 			goto disable_clocks;
 	}
 
-	writel(SENSOR_PDIV_T124, tegra->regs + SENSOR_PDIV);
-	writel(SENSOR_HOTSPOT_OFF_T124, tegra->regs + SENSOR_HOTSPOT_OFF);
+
+	/* program pdiv and hotspot offsets per THERM */
+	pdiv = readl(tegra->regs + SENSOR_PDIV);
+	hotspot = readl(tegra->regs + SENSOR_HOTSPOT_OFF);
+	for (i = 0; i < TEGRA124_SOCTHERM_SENSOR_NUM; ++i) {
+		pdiv = REG_SET_MASK(pdiv, ttgs[i]->pdiv_mask,
+				    ttgs[i]->pdiv);
+		if (ttgs[i]->id != TEGRA124_SOCTHERM_SENSOR_PLLX)
+			hotspot =  REG_SET_MASK(hotspot,
+						ttgs[i]->pllx_hotspot_mask,
+						ttgs[i]->pllx_hotspot_diff);
+	}
+	writel(pdiv, tegra->regs + SENSOR_PDIV);
+	writel(hotspot, tegra->regs + SENSOR_HOTSPOT_OFF);
 
 	/* Initialize thermctl sensors */
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1311449 — [PATCH V3 01/11] thermal: tegra: move tegra thermal files into tegra directory

Move Tegra soctherm driver to tegra directory, it's easy to maintain
and add more new function support for Tegra platforms.
This will also help to split soctherm driver into common parts and
chip specific data related parts.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/Kconfig                                     | 12 ++----------
 drivers/thermal/Makefile                                    |  2 +-
 drivers/thermal/tegra/Kconfig                               | 13 +++++++++++++
 drivers/thermal/tegra/Makefile                              |  1 +
 .../thermal/{tegra_soctherm.c => tegra/tegra-soctherm.c}    |  0
 5 files changed, 17 insertions(+), 11 deletions(-)
 create mode 100644 drivers/thermal/tegra/Kconfig
 create mode 100644 drivers/thermal/tegra/Makefile
 rename drivers/thermal/{tegra_soctherm.c => tegra/tegra-soctherm.c} (100%)

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index c463c89b90ef..e0577b15cbff 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -254,16 +254,6 @@ config ARMADA_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Armada 370 and Armada XP SoC.
 
-config TEGRA_SOCTHERM
-	tristate "Tegra SOCTHERM thermal management"
-	depends on ARCH_TEGRA
-	help
-	  Enable this option for integrated thermal management support on NVIDIA
-	  Tegra124 systems-on-chip. The driver supports four thermal zones
-	  (CPU, GPU, MEM, PLLX). Cooling devices can be bound to the thermal
-	  zones to manage temperatures. This option is also required for the
-	  emergency thermal reset (thermtrip) feature to function.
-
 config DB8500_CPUFREQ_COOLING
 	tristate "DB8500 cpufreq cooling"
 	depends on ARCH_U8500
@@ -380,6 +370,8 @@ depends on ARCH_STI && OF
 source "drivers/thermal/st/Kconfig"
 endmenu
 
+source "drivers/thermal/tegra/Kconfig"
+
 config QCOM_SPMI_TEMP_ALARM
 	tristate "Qualcomm SPMI PMIC Temperature Alarm"
 	depends on OF && (SPMI || COMPILE_TEST) && IIO
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index cfae6a654793..119e25cdcc66 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -46,5 +46,5 @@ obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
 obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
 obj-$(CONFIG_INTEL_PCH_THERMAL)	+= intel_pch_thermal.o
 obj-$(CONFIG_ST_THERMAL)	+= st/
-obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o
+obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra/
 obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
diff --git a/drivers/thermal/tegra/Kconfig b/drivers/thermal/tegra/Kconfig
new file mode 100644
index 000000000000..0b719d8b629b
--- /dev/null
+++ b/drivers/thermal/tegra/Kconfig
@@ -0,0 +1,13 @@
+menu "NVIDIA Tegra thermal drivers"
+
+config TEGRA_SOCTHERM
+	tristate "Tegra SOCTHERM thermal management"
+	depends on ARCH_TEGRA
+	help
+	  Enable this option for integrated thermal management support on NVIDIA
+	  Tegra124 systems-on-chip. The driver supports four thermal zones
+	  (CPU, GPU, MEM, PLLX). Cooling devices can be bound to the thermal
+	  zones to manage temperatures. This option is also required for the
+	  emergency thermal reset (thermtrip) feature to function.
+
+endmenu
diff --git a/drivers/thermal/tegra/Makefile b/drivers/thermal/tegra/Makefile
new file mode 100644
index 000000000000..d4dc4e7f279e
--- /dev/null
+++ b/drivers/thermal/tegra/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra-soctherm.o
diff --git a/drivers/thermal/tegra_soctherm.c b/drivers/thermal/tegra/tegra-soctherm.c
similarity index 100%
rename from drivers/thermal/tegra_soctherm.c
rename to drivers/thermal/tegra/tegra-soctherm.c
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1314236 — Re: [PATCH V3 04/11] thermal: tegra: split tegra_soctherm driver

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 18, 2016 at 06:02:29PM +0800, Wei Ni wrote:
[...]
> +int tegra_soctherm_calculate_tsensor_calibration(
> +				struct tegra_tsensor *sensor,
> +				const struct tsensor_shared_calibration *shared)

The need to ident weirdly here should be an indication that the function
name is too long, how about:

int tegra_tsensor_calc_calib(struct tegra_tsensor *sensor,
			     const struct tsensor_shared_calibration *shared)

?

> +{
> +	const struct tegra_tsensor_group *sensor_group;
> +	u32 val, calib;
> +	s32 actual_tsensor_ft, actual_tsensor_cp;
> +	s32 delta_sens, delta_temp;
> +	s32 mult, div;
> +	s16 therma, thermb;
> +	int err;
> +
> +	sensor_group = sensor->group;
> +
> +	err = tegra_fuse_readl(sensor->calib_fuse_offset, &val);
> +	if (err)
> +		return err;
> +
> +	actual_tsensor_cp = (shared->base_cp * 64) + sign_extend32(val, 12);
> +	val = (val & FUSE_TSENSOR_CALIB_FT_TS_BASE_MASK)
> +		>> FUSE_TSENSOR_CALIB_FT_TS_BASE_SHIFT;

I think it's more canonical to put the >> on the first line line.

> +	actual_tsensor_ft = (shared->base_ft * 32) + sign_extend32(val, 12);
> +
> +	delta_sens = actual_tsensor_ft - actual_tsensor_cp;
> +	delta_temp = shared->actual_temp_ft - shared->actual_temp_cp;
> +
> +	mult = sensor_group->pdiv * sensor->config->tsample_ate;
> +	div = sensor->config->tsample * sensor_group->pdiv_ate;
> +
> +	therma = div64_s64_precise((s64)delta_temp * (1LL << 13) * mult,
> +			(s64)delta_sens * div);

Are the explicit casts necessary? Shouldn't an s32 be automatically
promoted to s64? Also arguments on subsequent lines should be aligned
with the first argument on the first line.

> +	thermb = div64_s64_precise(
> +			((s64)actual_tsensor_ft * shared->actual_temp_cp) -
> +			((s64)actual_tsensor_cp * shared->actual_temp_ft),
> +			(s64)delta_sens);

Perhaps add a temporary variable for the first parameter here for
readability?

> +
> +	therma = div64_s64_precise((s64)therma * sensor->fuse_corr_alpha,
> +			(s64)1000000LL);
> +	thermb = div64_s64_precise((s64)thermb * sensor->fuse_corr_alpha +
> +			sensor->fuse_corr_beta,
> +			(s64)1000000LL);

What are the 1000000LL? Does it perhaps make sense to have a macro for
it, or perhaps a comment would help.

> +	calib = ((u16)therma << SENSOR_CONFIG2_THERMA_SHIFT) |
> +		 ((u16)thermb << SENSOR_CONFIG2_THERMB_SHIFT);

Alignment here isn't right.

> diff --git a/drivers/thermal/tegra/soctherm.h b/drivers/thermal/tegra/soctherm.h
[...]
> +struct tegra_soctherm_soc {
> +	struct tegra_tsensor *tsensors;

Can't these be const? Do they ever need to be modified? If so, they
should probably not be part of this structure. Or at least only part of
them should be. The invariant part.

The reason is that if ever a second instance of this device was present
both instances would share this same data. I know it's unlikely to
happen, but setting a bad example would be... well... bad.

Instead I think if you need to have non-const fields you could separate
this further into struct tegra_tsensor_soc, with only the static
information about the sensor, and make struct tegra_tsensor contain a
pointer to that SoC structure and provide the variable fields in
addition. That way you can create a struct tegra_tsensor for each struct
tegra_tsensor_soc and store those per-instance.

> diff --git a/drivers/thermal/tegra/tegra124-soctherm.c b/drivers/thermal/tegra/tegra124-soctherm.c
[...]
> +static struct tegra_tsensor tegra124_tsensors[] = {

Can this be "static const" instead?

> +	{
> +		.name = "cpu0",
> +		.base = 0xc0,
> +		.config = &t124_tsensor_config,
> +		.calib_fuse_offset = 0x098,
> +		.fuse_corr_alpha = 1135400,
> +		.fuse_corr_beta = -6266900,
> +		.group = &tegra124_tsensor_group_cpu,
> +	},
> +	{

"}," and "{" can go on the same line.

> +struct tegra_soctherm_soc tegra124_soctherm = {

"const"?

Thierry

[toc] | [prev] | [next] | [standalone]

#1314831 — Re: [PATCH V3 04/11] thermal: tegra: split tegra_soctherm driver

On 2016年01月21日 22:46, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Mon, Jan 18, 2016 at 06:02:29PM +0800, Wei Ni wrote:
> [...]
>> +int tegra_soctherm_calculate_tsensor_calibration(
>> +				struct tegra_tsensor *sensor,
>> +				const struct tsensor_shared_calibration *shared)
> 
> The need to ident weirdly here should be an indication that the function
> name is too long, how about:

Hmm, yes, it's too long.

> 
> int tegra_tsensor_calc_calib(struct tegra_tsensor *sensor,
> 			     const struct tsensor_shared_calibration *shared)
> 
> ?

There have two functions about calibration, I prefer to name them as:
tegra_calc_tsensor_calib() and tegra_calc_shared_calib().

> 
>> +{
>> +	const struct tegra_tsensor_group *sensor_group;
>> +	u32 val, calib;
>> +	s32 actual_tsensor_ft, actual_tsensor_cp;
>> +	s32 delta_sens, delta_temp;
>> +	s32 mult, div;
>> +	s16 therma, thermb;
>> +	int err;
>> +
>> +	sensor_group = sensor->group;
>> +
>> +	err = tegra_fuse_readl(sensor->calib_fuse_offset, &val);
>> +	if (err)
>> +		return err;
>> +
>> +	actual_tsensor_cp = (shared->base_cp * 64) + sign_extend32(val, 12);
>> +	val = (val & FUSE_TSENSOR_CALIB_FT_TS_BASE_MASK)
>> +		>> FUSE_TSENSOR_CALIB_FT_TS_BASE_SHIFT;
> 
> I think it's more canonical to put the >> on the first line line.

Ok, will fix it.

> 
>> +	actual_tsensor_ft = (shared->base_ft * 32) + sign_extend32(val, 12);
>> +
>> +	delta_sens = actual_tsensor_ft - actual_tsensor_cp;
>> +	delta_temp = shared->actual_temp_ft - shared->actual_temp_cp;
>> +
>> +	mult = sensor_group->pdiv * sensor->config->tsample_ate;
>> +	div = sensor->config->tsample * sensor_group->pdiv_ate;
>> +
>> +	therma = div64_s64_precise((s64)delta_temp * (1LL << 13) * mult,
>> +			(s64)delta_sens * div);
> 
> Are the explicit casts necessary? Shouldn't an s32 be automatically
> promoted to s64? Also arguments on subsequent lines should be aligned
> with the first argument on the first line.

I made a mistake, the div64_s64_precise(s64 a, s64 b) should be
div64_s64_precise(s64 a, s32b), so to make the value more precise, I added (s64)
cast in here.
And I will use temporary variable, and align the arguments.

> 
>> +	thermb = div64_s64_precise(
>> +			((s64)actual_tsensor_ft * shared->actual_temp_cp) -
>> +			((s64)actual_tsensor_cp * shared->actual_temp_ft),
>> +			(s64)delta_sens);
> 
> Perhaps add a temporary variable for the first parameter here for
> readability?

Yes, will use temporary variable, and remove the cast for delta_sens.

> 
>> +
>> +	therma = div64_s64_precise((s64)therma * sensor->fuse_corr_alpha,
>> +			(s64)1000000LL);
>> +	thermb = div64_s64_precise((s64)thermb * sensor->fuse_corr_alpha +
>> +			sensor->fuse_corr_beta,
>> +			(s64)1000000LL);
> 
> What are the 1000000LL? Does it perhaps make sense to have a macro for
> it, or perhaps a comment would help.

It's the coefficient data for the therma and thermb. I will use macro for it.
#define CALIB_COEFFICIENT 1000000LL

> 
>> +	calib = ((u16)therma << SENSOR_CONFIG2_THERMA_SHIFT) |
>> +		 ((u16)thermb << SENSOR_CONFIG2_THERMB_SHIFT);
> 
> Alignment here isn't right.

will do it.

> 
>> diff --git a/drivers/thermal/tegra/soctherm.h b/drivers/thermal/tegra/soctherm.h
> [...]
>> +struct tegra_soctherm_soc {
>> +	struct tegra_tsensor *tsensors;
> 
> Can't these be const? Do they ever need to be modified? If so, they
> should probably not be part of this structure. Or at least only part of
> them should be. The invariant part.
> 
> The reason is that if ever a second instance of this device was present
> both instances would share this same data. I know it's unlikely to
> happen, but setting a bad example would be... well... bad.
> 
> Instead I think if you need to have non-const fields you could separate
> this further into struct tegra_tsensor_soc, with only the static
> information about the sensor, and make struct tegra_tsensor contain a
> pointer to that SoC structure and provide the variable fields in
> addition. That way you can create a struct tegra_tsensor for each struct
> tegra_tsensor_soc and store those per-instance.

There has a member "calib" in the tsensors which will be written in the driver,
so I didn't make it as const.
But you are right, I need to consider the risk that the system have two more SOCs.
I will remove the "calib" to struct tegra_soctherm, then can fix this issue, and
can make all data to "const" in the chip-specific file.

> 
>> diff --git a/drivers/thermal/tegra/tegra124-soctherm.c b/drivers/thermal/tegra/tegra124-soctherm.c
> [...]
>> +static struct tegra_tsensor tegra124_tsensors[] = {
> 
> Can this be "static const" instead?

Will fix this one.

> 
>> +	{
>> +		.name = "cpu0",
>> +		.base = 0xc0,
>> +		.config = &t124_tsensor_config,
>> +		.calib_fuse_offset = 0x098,
>> +		.fuse_corr_alpha = 1135400,
>> +		.fuse_corr_beta = -6266900,
>> +		.group = &tegra124_tsensor_group_cpu,
>> +	},
>> +	{
> 
> "}," and "{" can go on the same line.

Got it, will change it.

> 
>> +struct tegra_soctherm_soc tegra124_soctherm = {
> 
> "const"?

Will fix it.

> 
> Thierry
> 
> * Unknown Key
> * 0x7F3EB3A1
> 

[toc] | [prev] | [next] | [standalone]

#1314242

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 18, 2016 at 06:02:25PM +0800, Wei Ni wrote:
> This patchset adds following functions for tegra_soctherm driver:
> 1. add T210 support.
> 2. export debugfs to show some registers.
> 3. add thermtrip funciton.
> 4. add suspend/resume function.
> 
> The V1 serial is in:
> http://www.spinics.net/lists/linux-tegra/msg24808.html
> The V2 serial is in:
> http://www.spinics.net/lists/linux-tegra/msg24901.html
> 
> Main changes from V2:
> 1. Fix build error in patch [1/11].
> 2. Use of_get_child_by_name instead of of_find_node_by_name in patch [8/11].
> 3. Use debugfs_remove_recursive to remove debugfs in patch [6/11].
> 
> Main changes from V1:
> 1. Use the new type to handl different Tegra chips in one driver,
> which suggested by Thierry.
> 2. Changes per Thieery's other comments.
> 
> Wei Ni (11):
>   thermal: tegra: move tegra thermal files into tegra directory
>   thermal: tegra: combine sensor group-related data
>   thermal: tegra: get rid of PDIV/HOTSPOT hack
>   thermal: tegra: split tegra_soctherm driver
>   thermal: tegra: add T210-specific SOC_THERM driver
>   thermal: tegra: add a debugfs to show registers
>   of: Add bindings of hw-trips for soctherm
>   thermal: tegra: add thermtrip function
>   thermal: tegra: add PM support
>   arm64: tegra: add soctherm node for Tegra210
>   ARM: tegra: set hw trips for Tegra124

Hi Wei,

This series looks mostly good to me. I've commented on a couple of minor
things as replies to the individual patches.

On a higher level, what's the test procedure that we can use to validate
that this code works?

Thierry

[toc] | [prev] | [next] | [standalone]

#1314793

On 2016年01月21日 22:56, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Mon, Jan 18, 2016 at 06:02:25PM +0800, Wei Ni wrote:
>> This patchset adds following functions for tegra_soctherm driver:
>> 1. add T210 support.
>> 2. export debugfs to show some registers.
>> 3. add thermtrip funciton.
>> 4. add suspend/resume function.
>>
>> The V1 serial is in:
>> http://www.spinics.net/lists/linux-tegra/msg24808.html
>> The V2 serial is in:
>> http://www.spinics.net/lists/linux-tegra/msg24901.html
>>
>> Main changes from V2:
>> 1. Fix build error in patch [1/11].
>> 2. Use of_get_child_by_name instead of of_find_node_by_name in patch [8/11].
>> 3. Use debugfs_remove_recursive to remove debugfs in patch [6/11].
>>
>> Main changes from V1:
>> 1. Use the new type to handl different Tegra chips in one driver,
>> which suggested by Thierry.
>> 2. Changes per Thieery's other comments.
>>
>> Wei Ni (11):
>>   thermal: tegra: move tegra thermal files into tegra directory
>>   thermal: tegra: combine sensor group-related data
>>   thermal: tegra: get rid of PDIV/HOTSPOT hack
>>   thermal: tegra: split tegra_soctherm driver
>>   thermal: tegra: add T210-specific SOC_THERM driver
>>   thermal: tegra: add a debugfs to show registers
>>   of: Add bindings of hw-trips for soctherm
>>   thermal: tegra: add thermtrip function
>>   thermal: tegra: add PM support
>>   arm64: tegra: add soctherm node for Tegra210
>>   ARM: tegra: set hw trips for Tegra124
> 
> Hi Wei,
> 
> This series looks mostly good to me. I've commented on a couple of minor
> things as replies to the individual patches.

Thanks for your review, I will check your comments and send out next version in
next few days.

> 
> On a higher level, what's the test procedure that we can use to validate
> that this code works?

You can check following files:
1. Run "cat /sys/class/thermal/thermal*/temp" to read temperature
This driver will register four thermal zones: cpu, gpu, mem and pll.
2. Run "cat /sys/kernel/debug/tegra_soctherm/regs" to show the register contents.
3. Write a low temperature value to
/sys/kernel/debug/tegra_soctherm/thermtrip/xxx to trigger the thermtrip function.
For example, if the cpu temperature is 30000 now, you can read it from above
thermal zones, then "echo 25000 >
/sys/kernel/debug/tegra_soctherm/thermtrip/cpu", the system will be shutdown
immediately.

> 
> Thierry
> 
> * Unknown Key
> * 0x7F3EB3A1
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web