[PATCH V7 06/12] thermal: tegra: add a debugfs to show registers

[PATCH V7 06/12] thermal: tegra: add a debugfs to show registers

[Wei Ni]

Add a debugfs interface to show register contents for debug.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/soctherm.c | 143 ++++++++++++++++++++++++++++++++++++++-
 drivers/thermal/tegra/soctherm.h |   2 +
 2 files changed, 142 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index 52a33760e8e8..02ac6d2e5a20 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -15,6 +15,7 @@
  *
  */
 
+#include <linux/debugfs.h>
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -33,14 +34,18 @@
 
 #define SENSOR_CONFIG0				0
 #define SENSOR_CONFIG0_STOP			BIT(0)
-#define SENSOR_CONFIG0_TALL_SHIFT		8
-#define SENSOR_CONFIG0_TCALC_OVER		BIT(4)
-#define SENSOR_CONFIG0_OVER			BIT(3)
 #define SENSOR_CONFIG0_CPTR_OVER		BIT(2)
+#define SENSOR_CONFIG0_OVER			BIT(3)
+#define SENSOR_CONFIG0_TCALC_OVER		BIT(4)
+#define SENSOR_CONFIG0_TALL_MASK		(0xfffff << 8)
+#define SENSOR_CONFIG0_TALL_SHIFT		8
 
 #define SENSOR_CONFIG1				4
+#define SENSOR_CONFIG1_TSAMPLE_MASK		0x3ff
 #define SENSOR_CONFIG1_TSAMPLE_SHIFT		0
+#define SENSOR_CONFIG1_TIDDQ_EN_MASK		(0x3f << 15)
 #define SENSOR_CONFIG1_TIDDQ_EN_SHIFT		15
+#define SENSOR_CONFIG1_TEN_COUNT_MASK		(0x3f << 24)
 #define SENSOR_CONFIG1_TEN_COUNT_SHIFT		24
 #define SENSOR_CONFIG1_TEMP_ENABLE		BIT(31)
 
@@ -49,6 +54,14 @@
  * because, it will be used by tegra_soctherm_fuse.c
  */
 
+#define SENSOR_STATUS0				0xc
+#define SENSOR_STATUS0_VALID_MASK		BIT(31)
+#define SENSOR_STATUS0_CAPTURE_MASK		0xffff
+
+#define SENSOR_STATUS1				0x10
+#define SENSOR_STATUS1_TEMP_VALID_MASK		BIT(31)
+#define SENSOR_STATUS1_TEMP_MASK		0xffff
+
 #define READBACK_VALUE_MASK			0xff00
 #define READBACK_VALUE_SHIFT			8
 #define READBACK_ADD_HALF			BIT(7)
@@ -73,6 +86,8 @@ struct tegra_soctherm {
 
 	u32 *calib;
 	struct tegra_soctherm_soc *soc;
+
+	struct dentry *debugfs_dir;
 };
 
 static int enable_tsensor(struct tegra_soctherm *tegra,
@@ -140,6 +155,124 @@ static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
 	.get_temp = tegra_thermctl_get_temp,
 };
 
+#ifdef CONFIG_DEBUG_FS
+static int regs_show(struct seq_file *s, void *data)
+{
+	struct platform_device *pdev = s->private;
+	struct tegra_soctherm *ts = platform_get_drvdata(pdev);
+	const struct tegra_tsensor *tsensors = ts->soc->tsensors;
+	u32 r, state;
+	int i;
+
+	seq_puts(s, "-----TSENSE (convert HW)-----\n");
+
+	for (i = 0; i < ts->soc->num_tsensors; i++) {
+		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG1);
+		state = REG_GET_MASK(r, SENSOR_CONFIG1_TEMP_ENABLE);
+		if (!state)
+			continue;
+
+		seq_printf(s, "%s: ", tsensors[i].name);
+
+		seq_printf(s, "En(%d) ", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG1_TIDDQ_EN_MASK);
+		seq_printf(s, "tiddq(%d) ", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG1_TEN_COUNT_MASK);
+		seq_printf(s, "ten_count(%d) ", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG1_TSAMPLE_MASK);
+		seq_printf(s, "tsample(%d) ", state + 1);
+
+		r = readl(ts->regs + tsensors[i].base + SENSOR_STATUS1);
+		state = REG_GET_MASK(r, SENSOR_STATUS1_TEMP_VALID_MASK);
+		seq_printf(s, "Temp(%d/", state);
+		state = REG_GET_MASK(r, SENSOR_STATUS1_TEMP_MASK);
+		seq_printf(s, "%d) ", translate_temp(state));
+
+		r = readl(ts->regs + tsensors[i].base + SENSOR_STATUS0);
+		state = REG_GET_MASK(r, SENSOR_STATUS0_VALID_MASK);
+		seq_printf(s, "Capture(%d/", state);
+		state = REG_GET_MASK(r, SENSOR_STATUS0_CAPTURE_MASK);
+		seq_printf(s, "%d) ", state);
+
+		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG0);
+		state = REG_GET_MASK(r, SENSOR_CONFIG0_STOP);
+		seq_printf(s, "Stop(%d) ", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG0_TALL_MASK);
+		seq_printf(s, "Tall(%d) ", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG0_TCALC_OVER);
+		seq_printf(s, "Over(%d/", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG0_OVER);
+		seq_printf(s, "%d/", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG0_CPTR_OVER);
+		seq_printf(s, "%d) ", state);
+
+		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG2);
+		state = REG_GET_MASK(r, SENSOR_CONFIG2_THERMA_MASK);
+		seq_printf(s, "Therm_A/B(%d/", state);
+		state = REG_GET_MASK(r, SENSOR_CONFIG2_THERMB_MASK);
+		seq_printf(s, "%d)\n", (s16)state);
+	}
+
+	r = readl(ts->regs + SENSOR_PDIV);
+	seq_printf(s, "PDIV: 0x%x\n", r);
+
+	r = readl(ts->regs + SENSOR_HOTSPOT_OFF);
+	seq_printf(s, "HOTSPOT: 0x%x\n", r);
+
+	seq_puts(s, "\n");
+	seq_puts(s, "-----SOC_THERM-----\n");
+
+	r = readl(ts->regs + SENSOR_TEMP1);
+	state = REG_GET_MASK(r, SENSOR_TEMP1_CPU_TEMP_MASK);
+	seq_printf(s, "Temperatures: CPU(%d) ", translate_temp(state));
+	state = REG_GET_MASK(r, SENSOR_TEMP1_GPU_TEMP_MASK);
+	seq_printf(s, " GPU(%d) ", translate_temp(state));
+	r = readl(ts->regs + SENSOR_TEMP2);
+	state = REG_GET_MASK(r, SENSOR_TEMP2_PLLX_TEMP_MASK);
+	seq_printf(s, " PLLX(%d) ", translate_temp(state));
+	state = REG_GET_MASK(r, SENSOR_TEMP2_MEM_TEMP_MASK);
+	seq_printf(s, " MEM(%d)\n", translate_temp(state));
+
+	return 0;
+}
+
+static int regs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, regs_show, inode->i_private);
+}
+
+static const struct file_operations regs_fops = {
+	.open		= regs_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static void soctherm_debug_init(struct platform_device *pdev)
+{
+	struct tegra_soctherm *tegra = platform_get_drvdata(pdev);
+	struct dentry *root, *file;
+
+	root = debugfs_create_dir("soctherm", NULL);
+	if (!root) {
+		dev_err(&pdev->dev, "failed to create debugfs directory\n");
+		return;
+	}
+
+	tegra->debugfs_dir = root;
+
+	file = debugfs_create_file("reg_contents", 0644, root,
+				   pdev, &regs_fops);
+	if (!file)
+		dev_err(&pdev->dev, "failed to create debugfs file\n");
+}
+#else
+static inline void soctherm_debug_init(struct platform_device *pdev)
+{
+	return 0;
+}
+#endif
+
 static const struct of_device_id tegra_soctherm_of_match[] = {
 #ifdef CONFIG_ARCH_TEGRA_124_SOC
 	{
@@ -279,6 +412,8 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 		}
 	}
 
+	soctherm_debug_init(pdev);
+
 	return 0;
 
 disable_clocks:
@@ -292,6 +427,8 @@ static int tegra_soctherm_remove(struct platform_device *pdev)
 {
 	struct tegra_soctherm *tegra = platform_get_drvdata(pdev);
 
+	debugfs_remove_recursive(tegra->debugfs_dir);
+
 	clk_disable_unprepare(tegra->clock_tsensor);
 	clk_disable_unprepare(tegra->clock_soctherm);
 
diff --git a/drivers/thermal/tegra/soctherm.h b/drivers/thermal/tegra/soctherm.h
index 69d317269af1..ec4b87616d01 100644
--- a/drivers/thermal/tegra/soctherm.h
+++ b/drivers/thermal/tegra/soctherm.h
@@ -16,7 +16,9 @@
 #define __DRIVERS_THERMAL_TEGRA_SOCTHERM_H
 
 #define SENSOR_CONFIG2                          8
+#define SENSOR_CONFIG2_THERMA_MASK		(0xffff << 16)
 #define SENSOR_CONFIG2_THERMA_SHIFT		16
+#define SENSOR_CONFIG2_THERMB_MASK		0xffff
 #define SENSOR_CONFIG2_THERMB_SHIFT		0
 
 #define SENSOR_PDIV				0x1c0
-- 
1.9.1

Re: [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 8269 bytes --]

On Fri, Mar 11, 2016 at 11:10:35AM +0800, Wei Ni wrote:
> Add a debugfs interface to show register contents for debug.
> 
> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/thermal/tegra/soctherm.c | 143 ++++++++++++++++++++++++++++++++++++++-
>  drivers/thermal/tegra/soctherm.h |   2 +
>  2 files changed, 142 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
> index 52a33760e8e8..02ac6d2e5a20 100644
> --- a/drivers/thermal/tegra/soctherm.c
> +++ b/drivers/thermal/tegra/soctherm.c
> @@ -15,6 +15,7 @@
>   *
>   */
>  
> +#include <linux/debugfs.h>
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> @@ -33,14 +34,18 @@
>  
>  #define SENSOR_CONFIG0				0
>  #define SENSOR_CONFIG0_STOP			BIT(0)
> -#define SENSOR_CONFIG0_TALL_SHIFT		8
> -#define SENSOR_CONFIG0_TCALC_OVER		BIT(4)
> -#define SENSOR_CONFIG0_OVER			BIT(3)
>  #define SENSOR_CONFIG0_CPTR_OVER		BIT(2)
> +#define SENSOR_CONFIG0_OVER			BIT(3)
> +#define SENSOR_CONFIG0_TCALC_OVER		BIT(4)
> +#define SENSOR_CONFIG0_TALL_MASK		(0xfffff << 8)
> +#define SENSOR_CONFIG0_TALL_SHIFT		8
>  
>  #define SENSOR_CONFIG1				4
> +#define SENSOR_CONFIG1_TSAMPLE_MASK		0x3ff
>  #define SENSOR_CONFIG1_TSAMPLE_SHIFT		0
> +#define SENSOR_CONFIG1_TIDDQ_EN_MASK		(0x3f << 15)
>  #define SENSOR_CONFIG1_TIDDQ_EN_SHIFT		15
> +#define SENSOR_CONFIG1_TEN_COUNT_MASK		(0x3f << 24)
>  #define SENSOR_CONFIG1_TEN_COUNT_SHIFT		24
>  #define SENSOR_CONFIG1_TEMP_ENABLE		BIT(31)
>  
> @@ -49,6 +54,14 @@
>   * because, it will be used by tegra_soctherm_fuse.c
>   */
>  
> +#define SENSOR_STATUS0				0xc
> +#define SENSOR_STATUS0_VALID_MASK		BIT(31)
> +#define SENSOR_STATUS0_CAPTURE_MASK		0xffff
> +
> +#define SENSOR_STATUS1				0x10
> +#define SENSOR_STATUS1_TEMP_VALID_MASK		BIT(31)
> +#define SENSOR_STATUS1_TEMP_MASK		0xffff
> +
>  #define READBACK_VALUE_MASK			0xff00
>  #define READBACK_VALUE_SHIFT			8
>  #define READBACK_ADD_HALF			BIT(7)
> @@ -73,6 +86,8 @@ struct tegra_soctherm {
>  
>  	u32 *calib;
>  	struct tegra_soctherm_soc *soc;
> +
> +	struct dentry *debugfs_dir;
>  };
>  
>  static int enable_tsensor(struct tegra_soctherm *tegra,
> @@ -140,6 +155,124 @@ static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
>  	.get_temp = tegra_thermctl_get_temp,
>  };
>  
> +#ifdef CONFIG_DEBUG_FS
> +static int regs_show(struct seq_file *s, void *data)
> +{
> +	struct platform_device *pdev = s->private;
> +	struct tegra_soctherm *ts = platform_get_drvdata(pdev);
> +	const struct tegra_tsensor *tsensors = ts->soc->tsensors;
> +	u32 r, state;
> +	int i;
> +
> +	seq_puts(s, "-----TSENSE (convert HW)-----\n");
> +
> +	for (i = 0; i < ts->soc->num_tsensors; i++) {
> +		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG1);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TEMP_ENABLE);
> +		if (!state)
> +			continue;
> +
> +		seq_printf(s, "%s: ", tsensors[i].name);
> +
> +		seq_printf(s, "En(%d) ", state);


Shouldnt the above if (!state) be place right here?


> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TIDDQ_EN_MASK);
> +		seq_printf(s, "tiddq(%d) ", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TEN_COUNT_MASK);
> +		seq_printf(s, "ten_count(%d) ", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TSAMPLE_MASK);
> +		seq_printf(s, "tsample(%d) ", state + 1);
> +
> +		r = readl(ts->regs + tsensors[i].base + SENSOR_STATUS1);
> +		state = REG_GET_MASK(r, SENSOR_STATUS1_TEMP_VALID_MASK);
> +		seq_printf(s, "Temp(%d/", state);
> +		state = REG_GET_MASK(r, SENSOR_STATUS1_TEMP_MASK);
> +		seq_printf(s, "%d) ", translate_temp(state));
> +
> +		r = readl(ts->regs + tsensors[i].base + SENSOR_STATUS0);
> +		state = REG_GET_MASK(r, SENSOR_STATUS0_VALID_MASK);
> +		seq_printf(s, "Capture(%d/", state);
> +		state = REG_GET_MASK(r, SENSOR_STATUS0_CAPTURE_MASK);
> +		seq_printf(s, "%d) ", state);
> +
> +		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG0);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_STOP);
> +		seq_printf(s, "Stop(%d) ", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_TALL_MASK);
> +		seq_printf(s, "Tall(%d) ", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_TCALC_OVER);
> +		seq_printf(s, "Over(%d/", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_OVER);
> +		seq_printf(s, "%d/", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_CPTR_OVER);
> +		seq_printf(s, "%d) ", state);
> +
> +		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG2);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG2_THERMA_MASK);
> +		seq_printf(s, "Therm_A/B(%d/", state);
> +		state = REG_GET_MASK(r, SENSOR_CONFIG2_THERMB_MASK);
> +		seq_printf(s, "%d)\n", (s16)state);
> +	}
> +
> +	r = readl(ts->regs + SENSOR_PDIV);
> +	seq_printf(s, "PDIV: 0x%x\n", r);

This is a matter of taste, but:

Why here %x but all the above %d?

Reg dumps are typically %x.
> +
> +	r = readl(ts->regs + SENSOR_HOTSPOT_OFF);
> +	seq_printf(s, "HOTSPOT: 0x%x\n", r);
> +
> +	seq_puts(s, "\n");
> +	seq_puts(s, "-----SOC_THERM-----\n");
> +
> +	r = readl(ts->regs + SENSOR_TEMP1);
> +	state = REG_GET_MASK(r, SENSOR_TEMP1_CPU_TEMP_MASK);
> +	seq_printf(s, "Temperatures: CPU(%d) ", translate_temp(state));

Maybe a line break before CPU, for consistency?

> +	state = REG_GET_MASK(r, SENSOR_TEMP1_GPU_TEMP_MASK);
> +	seq_printf(s, " GPU(%d) ", translate_temp(state));
> +	r = readl(ts->regs + SENSOR_TEMP2);
> +	state = REG_GET_MASK(r, SENSOR_TEMP2_PLLX_TEMP_MASK);
> +	seq_printf(s, " PLLX(%d) ", translate_temp(state));
> +	state = REG_GET_MASK(r, SENSOR_TEMP2_MEM_TEMP_MASK);
> +	seq_printf(s, " MEM(%d)\n", translate_temp(state));
> +

I got confused, I thought you would the above mapped as tsensors[i]. I
will check the code again.


> +	return 0;
> +}
> +
> +static int regs_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, regs_show, inode->i_private);
> +}
> +
> +static const struct file_operations regs_fops = {
> +	.open		= regs_open,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
> +static void soctherm_debug_init(struct platform_device *pdev)
> +{
> +	struct tegra_soctherm *tegra = platform_get_drvdata(pdev);
> +	struct dentry *root, *file;
> +
> +	root = debugfs_create_dir("soctherm", NULL);
> +	if (!root) {
> +		dev_err(&pdev->dev, "failed to create debugfs directory\n");
> +		return;
> +	}
> +
> +	tegra->debugfs_dir = root;
> +
> +	file = debugfs_create_file("reg_contents", 0644, root,
> +				   pdev, &regs_fops);
> +	if (!file)
> +		dev_err(&pdev->dev, "failed to create debugfs file\n");

Are we leaving the 'soctherm' dir left uncleaned in fail path here?

> +}
> +#else
> +static inline void soctherm_debug_init(struct platform_device *pdev)
> +{
> +	return 0;

I dont think we need this return for a void function.

> +}
> +#endif
> +
>  static const struct of_device_id tegra_soctherm_of_match[] = {
>  #ifdef CONFIG_ARCH_TEGRA_124_SOC
>  	{
> @@ -279,6 +412,8 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	soctherm_debug_init(pdev);
> +
>  	return 0;
>  
>  disable_clocks:
> @@ -292,6 +427,8 @@ static int tegra_soctherm_remove(struct platform_device *pdev)
>  {
>  	struct tegra_soctherm *tegra = platform_get_drvdata(pdev);
>  
> +	debugfs_remove_recursive(tegra->debugfs_dir);
> +
>  	clk_disable_unprepare(tegra->clock_tsensor);
>  	clk_disable_unprepare(tegra->clock_soctherm);
>  
> diff --git a/drivers/thermal/tegra/soctherm.h b/drivers/thermal/tegra/soctherm.h
> index 69d317269af1..ec4b87616d01 100644
> --- a/drivers/thermal/tegra/soctherm.h
> +++ b/drivers/thermal/tegra/soctherm.h
> @@ -16,7 +16,9 @@
>  #define __DRIVERS_THERMAL_TEGRA_SOCTHERM_H
>  
>  #define SENSOR_CONFIG2                          8
> +#define SENSOR_CONFIG2_THERMA_MASK		(0xffff << 16)
>  #define SENSOR_CONFIG2_THERMA_SHIFT		16
> +#define SENSOR_CONFIG2_THERMB_MASK		0xffff
>  #define SENSOR_CONFIG2_THERMB_SHIFT		0
>  
>  #define SENSOR_PDIV				0x1c0
> -- 
> 1.9.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers

[Wei Ni]

On 2016年03月15日 03:08, Eduardo Valentin wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Mar 11, 2016 at 11:10:35AM +0800, Wei Ni wrote:
>> Add a debugfs interface to show register contents for debug.
>>
>> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/thermal/tegra/soctherm.c | 143 ++++++++++++++++++++++++++++++++++++++-
>>  drivers/thermal/tegra/soctherm.h |   2 +
>>  2 files changed, 142 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
>> index 52a33760e8e8..02ac6d2e5a20 100644
>> --- a/drivers/thermal/tegra/soctherm.c
>> +++ b/drivers/thermal/tegra/soctherm.c
>> @@ -15,6 +15,7 @@
>>   *
>>   */
>>  
>> +#include <linux/debugfs.h>
>>  #include <linux/bitops.h>
>>  #include <linux/clk.h>
>>  #include <linux/delay.h>
>> @@ -33,14 +34,18 @@
>>  
>>  #define SENSOR_CONFIG0				0
>>  #define SENSOR_CONFIG0_STOP			BIT(0)
>> -#define SENSOR_CONFIG0_TALL_SHIFT		8
>> -#define SENSOR_CONFIG0_TCALC_OVER		BIT(4)
>> -#define SENSOR_CONFIG0_OVER			BIT(3)
>>  #define SENSOR_CONFIG0_CPTR_OVER		BIT(2)
>> +#define SENSOR_CONFIG0_OVER			BIT(3)
>> +#define SENSOR_CONFIG0_TCALC_OVER		BIT(4)
>> +#define SENSOR_CONFIG0_TALL_MASK		(0xfffff << 8)
>> +#define SENSOR_CONFIG0_TALL_SHIFT		8
>>  
>>  #define SENSOR_CONFIG1				4
>> +#define SENSOR_CONFIG1_TSAMPLE_MASK		0x3ff
>>  #define SENSOR_CONFIG1_TSAMPLE_SHIFT		0
>> +#define SENSOR_CONFIG1_TIDDQ_EN_MASK		(0x3f << 15)
>>  #define SENSOR_CONFIG1_TIDDQ_EN_SHIFT		15
>> +#define SENSOR_CONFIG1_TEN_COUNT_MASK		(0x3f << 24)
>>  #define SENSOR_CONFIG1_TEN_COUNT_SHIFT		24
>>  #define SENSOR_CONFIG1_TEMP_ENABLE		BIT(31)
>>  
>> @@ -49,6 +54,14 @@
>>   * because, it will be used by tegra_soctherm_fuse.c
>>   */
>>  
>> +#define SENSOR_STATUS0				0xc
>> +#define SENSOR_STATUS0_VALID_MASK		BIT(31)
>> +#define SENSOR_STATUS0_CAPTURE_MASK		0xffff
>> +
>> +#define SENSOR_STATUS1				0x10
>> +#define SENSOR_STATUS1_TEMP_VALID_MASK		BIT(31)
>> +#define SENSOR_STATUS1_TEMP_MASK		0xffff
>> +
>>  #define READBACK_VALUE_MASK			0xff00
>>  #define READBACK_VALUE_SHIFT			8
>>  #define READBACK_ADD_HALF			BIT(7)
>> @@ -73,6 +86,8 @@ struct tegra_soctherm {
>>  
>>  	u32 *calib;
>>  	struct tegra_soctherm_soc *soc;
>> +
>> +	struct dentry *debugfs_dir;
>>  };
>>  
>>  static int enable_tsensor(struct tegra_soctherm *tegra,
>> @@ -140,6 +155,124 @@ static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
>>  	.get_temp = tegra_thermctl_get_temp,
>>  };
>>  
>> +#ifdef CONFIG_DEBUG_FS
>> +static int regs_show(struct seq_file *s, void *data)
>> +{
>> +	struct platform_device *pdev = s->private;
>> +	struct tegra_soctherm *ts = platform_get_drvdata(pdev);
>> +	const struct tegra_tsensor *tsensors = ts->soc->tsensors;
>> +	u32 r, state;
>> +	int i;
>> +
>> +	seq_puts(s, "-----TSENSE (convert HW)-----\n");
>> +
>> +	for (i = 0; i < ts->soc->num_tsensors; i++) {
>> +		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG1);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TEMP_ENABLE);
>> +		if (!state)
>> +			continue;
>> +
>> +		seq_printf(s, "%s: ", tsensors[i].name);
>> +
>> +		seq_printf(s, "En(%d) ", state);
> 
> 
> Shouldnt the above if (!state) be place right here?

Yes, it make sense, will do it.

> 
> 
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TIDDQ_EN_MASK);
>> +		seq_printf(s, "tiddq(%d) ", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TEN_COUNT_MASK);
>> +		seq_printf(s, "ten_count(%d) ", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG1_TSAMPLE_MASK);
>> +		seq_printf(s, "tsample(%d) ", state + 1);
>> +
>> +		r = readl(ts->regs + tsensors[i].base + SENSOR_STATUS1);
>> +		state = REG_GET_MASK(r, SENSOR_STATUS1_TEMP_VALID_MASK);
>> +		seq_printf(s, "Temp(%d/", state);
>> +		state = REG_GET_MASK(r, SENSOR_STATUS1_TEMP_MASK);
>> +		seq_printf(s, "%d) ", translate_temp(state));
>> +
>> +		r = readl(ts->regs + tsensors[i].base + SENSOR_STATUS0);
>> +		state = REG_GET_MASK(r, SENSOR_STATUS0_VALID_MASK);
>> +		seq_printf(s, "Capture(%d/", state);
>> +		state = REG_GET_MASK(r, SENSOR_STATUS0_CAPTURE_MASK);
>> +		seq_printf(s, "%d) ", state);
>> +
>> +		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG0);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_STOP);
>> +		seq_printf(s, "Stop(%d) ", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_TALL_MASK);
>> +		seq_printf(s, "Tall(%d) ", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_TCALC_OVER);
>> +		seq_printf(s, "Over(%d/", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_OVER);
>> +		seq_printf(s, "%d/", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG0_CPTR_OVER);
>> +		seq_printf(s, "%d) ", state);
>> +
>> +		r = readl(ts->regs + tsensors[i].base + SENSOR_CONFIG2);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG2_THERMA_MASK);
>> +		seq_printf(s, "Therm_A/B(%d/", state);
>> +		state = REG_GET_MASK(r, SENSOR_CONFIG2_THERMB_MASK);
>> +		seq_printf(s, "%d)\n", (s16)state);
>> +	}
>> +
>> +	r = readl(ts->regs + SENSOR_PDIV);
>> +	seq_printf(s, "PDIV: 0x%x\n", r);
> 
> This is a matter of taste, but:
> 
> Why here %x but all the above %d?

This register value contain CPU/GPU/MEM/PLLX's pdiv values, and didn't parse
them, so we need to use %x to print.

For above messages, we doesn't dump registers simply, we parsed these Reg
values, so we use %d for them, so that they are more readable.

> 
> Reg dumps are typically %x.

See above explanation, and for this debugfs, we called it as "reg_context", not
"reg" simply.

>> +
>> +	r = readl(ts->regs + SENSOR_HOTSPOT_OFF);
>> +	seq_printf(s, "HOTSPOT: 0x%x\n", r);
>> +
>> +	seq_puts(s, "\n");
>> +	seq_puts(s, "-----SOC_THERM-----\n");
>> +
>> +	r = readl(ts->regs + SENSOR_TEMP1);
>> +	state = REG_GET_MASK(r, SENSOR_TEMP1_CPU_TEMP_MASK);
>> +	seq_printf(s, "Temperatures: CPU(%d) ", translate_temp(state));
> 
> Maybe a line break before CPU, for consistency?

-----TSENSE (convert HW)-----
cpu0: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/27500) Capture(1/8483)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1683/-1687)
cpu1: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/26500) Capture(1/8665)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1800/-1850)
cpu2: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/29000) Capture(1/8395)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1709/-1693)
cpu3: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/27500) Capture(1/8413)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1732/-1723)
mem0: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/28000) Capture(1/8563)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1670/-1689)
mem1: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/26500) Capture(1/8700)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1911/-1976)
gpu: En(1) tiddq(1) ten_count(1) tsample(120) Temp(0/0) Capture(0/0) Stop(0)
Tall(16300) Over(0/0/0) Therm_A/B(1714/-1730)
pllx: En(1) tiddq(1) ten_count(1) tsample(120) Temp(1/28000) Capture(1/8477)
Stop(0) Tall(16300) Over(1/1/1) Therm_A/B(1644/-1645)
PDIV: 0x8888
HOTSPOT: 0xa0500

-----SOC_THERM-----
Temperatures: CPU(29000)  GPU(30500)  PLLX(28000)  MEM(28000)
Thermtrip Any En(0)
     cpu En(1) Thresh(102500)
     gpu En(1) Thresh(103000)
     pll En(0) Thresh(105000)
     mem En(0) Thresh(103000)

I paste this debufs output to here, it looks it is consistency :)

> 
>> +	state = REG_GET_MASK(r, SENSOR_TEMP1_GPU_TEMP_MASK);
>> +	seq_printf(s, " GPU(%d) ", translate_temp(state));
>> +	r = readl(ts->regs + SENSOR_TEMP2);
>> +	state = REG_GET_MASK(r, SENSOR_TEMP2_PLLX_TEMP_MASK);
>> +	seq_printf(s, " PLLX(%d) ", translate_temp(state));
>> +	state = REG_GET_MASK(r, SENSOR_TEMP2_MEM_TEMP_MASK);
>> +	seq_printf(s, " MEM(%d)\n", translate_temp(state));
>> +
> 
> I got confused, I thought you would the above mapped as tsensors[i]. I
> will check the code again.

We have sensors of CPU0/1/2/3, MEM0/1, GPU and PLLX, and mapped as above
tsensors[i].
Here are the group temperatures of CPU, MEM, GPU, PLLX. These groups aggregate
the actual tsensors (count >= 1) and read out the max of all the sensors in a
particular group as ‘group temperature’ in TEMP1/2.

> 
> 
>> +	return 0;
>> +}
>> +
>> +static int regs_open(struct inode *inode, struct file *file)
>> +{
>> +	return single_open(file, regs_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations regs_fops = {
>> +	.open		= regs_open,
>> +	.read		= seq_read,
>> +	.llseek		= seq_lseek,
>> +	.release	= single_release,
>> +};
>> +
>> +static void soctherm_debug_init(struct platform_device *pdev)
>> +{
>> +	struct tegra_soctherm *tegra = platform_get_drvdata(pdev);
>> +	struct dentry *root, *file;
>> +
>> +	root = debugfs_create_dir("soctherm", NULL);
>> +	if (!root) {
>> +		dev_err(&pdev->dev, "failed to create debugfs directory\n");
>> +		return;
>> +	}
>> +
>> +	tegra->debugfs_dir = root;
>> +
>> +	file = debugfs_create_file("reg_contents", 0644, root,
>> +				   pdev, &regs_fops);
>> +	if (!file)
>> +		dev_err(&pdev->dev, "failed to create debugfs file\n");
> 
> Are we leaving the 'soctherm' dir left uncleaned in fail path here?

I will fix it.

> 
>> +}
>> +#else
>> +static inline void soctherm_debug_init(struct platform_device *pdev)
>> +{
>> +	return 0;
> 
> I dont think we need this return for a void function.

Yes, will fix it.

> 
>> +}
>> +#endif
>> +
>>  static const struct of_device_id tegra_soctherm_of_match[] = {
>>  #ifdef CONFIG_ARCH_TEGRA_124_SOC
>>  	{
>> @@ -279,6 +412,8 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> +	soctherm_debug_init(pdev);
>> +
>>  	return 0;
>>  
>>  disable_clocks:
>> @@ -292,6 +427,8 @@ static int tegra_soctherm_remove(struct platform_device *pdev)
>>  {
>>  	struct tegra_soctherm *tegra = platform_get_drvdata(pdev);
>>  
>> +	debugfs_remove_recursive(tegra->debugfs_dir);
>> +
>>  	clk_disable_unprepare(tegra->clock_tsensor);
>>  	clk_disable_unprepare(tegra->clock_soctherm);
>>  
>> diff --git a/drivers/thermal/tegra/soctherm.h b/drivers/thermal/tegra/soctherm.h
>> index 69d317269af1..ec4b87616d01 100644
>> --- a/drivers/thermal/tegra/soctherm.h
>> +++ b/drivers/thermal/tegra/soctherm.h
>> @@ -16,7 +16,9 @@
>>  #define __DRIVERS_THERMAL_TEGRA_SOCTHERM_H
>>  
>>  #define SENSOR_CONFIG2                          8
>> +#define SENSOR_CONFIG2_THERMA_MASK		(0xffff << 16)
>>  #define SENSOR_CONFIG2_THERMA_SHIFT		16
>> +#define SENSOR_CONFIG2_THERMB_MASK		0xffff
>>  #define SENSOR_CONFIG2_THERMB_SHIFT		0
>>  
>>  #define SENSOR_PDIV				0x1c0
>> -- 
>> 1.9.1
>>
> 
> * Unknown Key
> * 0x7DA4E256
>