diff mbox series

[V2,1/3] soc/tegra: pmc: Fix reset sources and levels

Message ID 1555433288-16967-1-git-send-email-jonathanh@nvidia.com
State Accepted
Headers show
Series [V2,1/3] soc/tegra: pmc: Fix reset sources and levels | expand

Commit Message

Jon Hunter April 16, 2019, 4:48 p.m. UTC 
Commit 5f84bb1a4099 ("soc/tegra: pmc: Add sysfs entries for reset info")
added support for reading the Tegra reset source and level from sysfs.
However, there are a few issues with this commit which are ...
1. The number of reset sources for Tegra210 is defined as 5 but it
   should be 6.
2. The number of reset sources for Tegra186 is defined as 13 but it
   should be 15.
3. The SoC data variables num_reset_sources and num_reset_levels are
   defined but never used.

Fix the above by ...

1. Removing the reset source 'AOTAG' from the tegra30_reset_sources
   because this is only applicable for Tegra210.
2. Adding a new tegra210_reset_sources structure for Tegra210 reset
   sources.
3. Correct the number of reset sources for Tegra210 and Tegra186 by
   using the ARRAY_SIZE macro.
4. Updating the functions reset_reason_show() and reset_level_show()
   to check whether the value read is valid. While we are at it
   clean-up these functions to remove an unnecessary u32 variable.

Fixes: 5f84bb1a4099 ("soc/tegra: pmc: Add sysfs entries for reset info")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
Changes since V1:
- Reverted removal of num_reset_sources/levels
- Used ARRAY_SIZE macro to populate num_reset_sources/levels
- Added test to ensure that reset source/level is valid

 drivers/soc/tegra/pmc.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

Comments

Thierry Reding April 17, 2019, 8:34 a.m. UTC | #1 
On Tue, Apr 16, 2019 at 05:48:06PM +0100, Jon Hunter wrote:
> Commit 5f84bb1a4099 ("soc/tegra: pmc: Add sysfs entries for reset info")
> added support for reading the Tegra reset source and level from sysfs.
> However, there are a few issues with this commit which are ...
> 1. The number of reset sources for Tegra210 is defined as 5 but it
>    should be 6.
> 2. The number of reset sources for Tegra186 is defined as 13 but it
>    should be 15.
> 3. The SoC data variables num_reset_sources and num_reset_levels are
>    defined but never used.
> 
> Fix the above by ...
> 
> 1. Removing the reset source 'AOTAG' from the tegra30_reset_sources
>    because this is only applicable for Tegra210.
> 2. Adding a new tegra210_reset_sources structure for Tegra210 reset
>    sources.
> 3. Correct the number of reset sources for Tegra210 and Tegra186 by
>    using the ARRAY_SIZE macro.
> 4. Updating the functions reset_reason_show() and reset_level_show()
>    to check whether the value read is valid. While we are at it
>    clean-up these functions to remove an unnecessary u32 variable.
> 
> Fixes: 5f84bb1a4099 ("soc/tegra: pmc: Add sysfs entries for reset info")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> Changes since V1:
> - Reverted removal of num_reset_sources/levels
> - Used ARRAY_SIZE macro to populate num_reset_sources/levels
> - Added test to ensure that reset source/level is valid
> 
>  drivers/soc/tegra/pmc.c | 44 +++++++++++++++++++++++++++++---------------
>  1 file changed, 29 insertions(+), 15 deletions(-)

Applied to for-5.2/soc, thanks.

Thierry
diff mbox series

Patch

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 0c5f79528e5f..7953aa7b11df 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -272,6 +272,14 @@  static const char * const tegra30_reset_sources[] = {
 	"WATCHDOG",
 	"SENSOR",
 	"SW_MAIN",
+	"LP0"
+};
+
+static const char * const tegra210_reset_sources[] = {
+	"POWER_ON_RESET",
+	"WATCHDOG",
+	"SENSOR",
+	"SW_MAIN",
 	"LP0",
 	"AOTAG"
 };
@@ -1736,13 +1744,16 @@  static int tegra_pmc_pinctrl_init(struct tegra_pmc *pmc)
 static ssize_t reset_reason_show(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
-	u32 value, rst_src;
+	u32 value;
 
 	value = tegra_pmc_readl(pmc, pmc->soc->regs->rst_status);
-	rst_src = (value & pmc->soc->regs->rst_source_mask) >>
-			pmc->soc->regs->rst_source_shift;
+	value &= pmc->soc->regs->rst_source_mask;
+	value >>= pmc->soc->regs->rst_source_shift;
+
+	if (WARN_ON(value >= pmc->soc->num_reset_sources))
+		return sprintf(buf, "%s\n", "UNKNOWN");
 
-	return sprintf(buf, "%s\n", pmc->soc->reset_sources[rst_src]);
+	return sprintf(buf, "%s\n", pmc->soc->reset_sources[value]);
 }
 
 static DEVICE_ATTR_RO(reset_reason);
@@ -1750,13 +1761,16 @@  static DEVICE_ATTR_RO(reset_reason);
 static ssize_t reset_level_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
-	u32 value, rst_lvl;
+	u32 value;
 
 	value = tegra_pmc_readl(pmc, pmc->soc->regs->rst_status);
-	rst_lvl = (value & pmc->soc->regs->rst_level_mask) >>
-			pmc->soc->regs->rst_level_shift;
+	value &= pmc->soc->regs->rst_level_mask;
+	value >>= pmc->soc->regs->rst_level_shift;
 
-	return sprintf(buf, "%s\n", pmc->soc->reset_levels[rst_lvl]);
+	if (WARN_ON(value >= pmc->soc->num_reset_levels))
+		return sprintf(buf, "%s\n", "UNKNOWN");
+
+	return sprintf(buf, "%s\n", pmc->soc->reset_levels[value]);
 }
 
 static DEVICE_ATTR_RO(reset_level);
@@ -2212,7 +2226,7 @@  static const struct tegra_pmc_soc tegra30_pmc_soc = {
 	.init = tegra20_pmc_init,
 	.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
 	.reset_sources = tegra30_reset_sources,
-	.num_reset_sources = 5,
+	.num_reset_sources = ARRAY_SIZE(tegra30_reset_sources),
 	.reset_levels = NULL,
 	.num_reset_levels = 0,
 };
@@ -2263,7 +2277,7 @@  static const struct tegra_pmc_soc tegra114_pmc_soc = {
 	.init = tegra20_pmc_init,
 	.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
 	.reset_sources = tegra30_reset_sources,
-	.num_reset_sources = 5,
+	.num_reset_sources = ARRAY_SIZE(tegra30_reset_sources),
 	.reset_levels = NULL,
 	.num_reset_levels = 0,
 };
@@ -2374,7 +2388,7 @@  static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.init = tegra20_pmc_init,
 	.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
 	.reset_sources = tegra30_reset_sources,
-	.num_reset_sources = 5,
+	.num_reset_sources = ARRAY_SIZE(tegra30_reset_sources),
 	.reset_levels = NULL,
 	.num_reset_levels = 0,
 };
@@ -2479,8 +2493,8 @@  static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.regs = &tegra20_pmc_regs,
 	.init = tegra20_pmc_init,
 	.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
-	.reset_sources = tegra30_reset_sources,
-	.num_reset_sources = 5,
+	.reset_sources = tegra210_reset_sources,
+	.num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
 	.reset_levels = NULL,
 	.num_reset_levels = 0,
 };
@@ -2605,9 +2619,9 @@  static const struct tegra_pmc_soc tegra186_pmc_soc = {
 	.init = NULL,
 	.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
 	.reset_sources = tegra186_reset_sources,
-	.num_reset_sources = 14,
+	.num_reset_sources = ARRAY_SIZE(tegra186_reset_sources),
 	.reset_levels = tegra186_reset_levels,
-	.num_reset_levels = 3,
+	.num_reset_levels = ARRAY_SIZE(tegra186_reset_levels),
 	.num_wake_events = ARRAY_SIZE(tegra186_wake_events),
 	.wake_events = tegra186_wake_events,
 };