* [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers @ 2016-03-11 3:10 Wei Ni 2016-03-14 19:08 ` Eduardo Valentin 0 siblings, 1 reply; 3+ messages in thread From: Wei Ni @ 2016-03-11 3:10 UTC (permalink / raw) To: rui.zhang, edubezval, thierry.reding Cc: MLongnecker, swarren, mikko.perttunen, linux-tegra, linux-pm, linux-kernel, 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, ®s_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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers 2016-03-11 3:10 [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers Wei Ni @ 2016-03-14 19:08 ` Eduardo Valentin 2016-03-15 7:45 ` Wei Ni 0 siblings, 1 reply; 3+ messages in thread From: Eduardo Valentin @ 2016-03-14 19:08 UTC (permalink / raw) To: Wei Ni Cc: rui.zhang, thierry.reding, MLongnecker, swarren, mikko.perttunen, linux-tegra, linux-pm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 8240 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@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); 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, ®s_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 --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers 2016-03-14 19:08 ` Eduardo Valentin @ 2016-03-15 7:45 ` Wei Ni 0 siblings, 0 replies; 3+ messages in thread From: Wei Ni @ 2016-03-15 7:45 UTC (permalink / raw) To: Eduardo Valentin Cc: rui.zhang, thierry.reding, MLongnecker, swarren, mikko.perttunen, linux-tegra, linux-pm, linux-kernel 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@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); > > > 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, ®s_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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-15 7:45 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-11 3:10 [PATCH V7 06/12] thermal: tegra: add a debugfs to show registers Wei Ni 2016-03-14 19:08 ` Eduardo Valentin 2016-03-15 7:45 ` Wei Ni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox