[PATCH 0/5] Tegra PMC Updates

[PATCH 0/5] Tegra PMC Updates

[Jon Hunter]

Some updates and clean-ups for the Tegra PMC driver.

Jon Hunter (5):
  soc/tegra: pmc: Guard against uninitialised PMC clock
  soc/tegra: pmc: Simplify IO rail bit handling
  soc/tegra: Clean-up PMC rail IO error messages
  soc/tegra: pmc: Check return code for pm_genpd_init()
  soc/tegra: pmc: Remove genpd when adding provider fails

 drivers/soc/tegra/pmc.c | 56 +++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

-- 
1.9.1

[PATCH 1/5] soc/tegra: pmc: Guard against uninitialised PMC clock

[Jon Hunter]

It is possible for the public functions, tegra_io_rail_power_on/off()
to be called before the PMC device has been probed. If this happens
then the pmc->clk member will not be initialised and the call to
clk_get_rate() in tegra_io_rail_prepare() will return zero and lead
to a divide-by-zero exception. The function clk_get_rate() will return
zero if a NULl clk pointer is passed. Therefore, rather that checking
if pmc->clk is initialised, fix this by checking the return value for
clk_get_rate() to make sure it is not zero.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/soc/tegra/pmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 7792ed88d80b..c99580aabcf6 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -931,6 +931,8 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	}
 
 	rate = clk_get_rate(pmc->clk);
+	if (!rate)
+		return -ENODEV;
 
 	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
 
-- 
1.9.1

[PATCH 2/5] soc/tegra: pmc: Simplify IO rail bit handling

[Jon Hunter]

The function tegra_io_rail_prepare() converts the IO rail ID into a
bit position that is used to check the status and control the IO rail
in the PMC registers. However, rather than converting to a bit position
it is more useful to convert to a bit-mask because this is what is
actually used. By doing so the BIT() marco only needs to be used once
and we can use the IO_DPD_REQ_CODE_MASK when checking for erroneous rail
IDs.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/soc/tegra/pmc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index c99580aabcf6..6a6df6e8bfd6 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -913,13 +913,13 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 {
 	unsigned long rate, value;
 
-	*bit = id % 32;
+	*bit = BIT(id % 32);
 
 	/*
 	 * There are two sets of 30 bits to select IO rails, but bits 30 and
 	 * 31 are control bits rather than IO rail selection bits.
 	 */
-	if (id > 63 || *bit == 30 || *bit == 31)
+	if (id > 63 || *bit & IO_DPD_REQ_CODE_MASK)
 		return -EINVAL;
 
 	if (id < 32) {
@@ -979,9 +979,9 @@ int tegra_io_rail_power_on(unsigned int id)
 	if (err)
 		goto error;
 
-	tegra_pmc_writel(IO_DPD_REQ_CODE_OFF | BIT(bit), request);
+	tegra_pmc_writel(IO_DPD_REQ_CODE_OFF | bit, request);
 
-	err = tegra_io_rail_poll(status, BIT(bit), 0, 250);
+	err = tegra_io_rail_poll(status, bit, 0, 250);
 	if (err) {
 		pr_info("tegra_io_rail_poll() failed: %d\n", err);
 		goto error;
@@ -1010,9 +1010,9 @@ int tegra_io_rail_power_off(unsigned int id)
 		goto error;
 	}
 
-	tegra_pmc_writel(IO_DPD_REQ_CODE_ON | BIT(bit), request);
+	tegra_pmc_writel(IO_DPD_REQ_CODE_ON | bit, request);
 
-	err = tegra_io_rail_poll(status, BIT(bit), BIT(bit), 250);
+	err = tegra_io_rail_poll(status, bit, bit, 250);
 	if (err)
 		goto error;
 
-- 
1.9.1

[PATCH 3/5] soc/tegra: Clean-up PMC rail IO error messages

[Jon Hunter]

Clean-up the error messages in the PMC rail IO functions by:
1. Using pr_err instead of pr_info which is currently used.
2. Use the compiler __func__ marco for displaying the function name.
3. Add specific error messages to the function tegra_io_rail_prepare()
   instead of just reporting it failed.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/soc/tegra/pmc.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 6a6df6e8bfd6..8c9797379949 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -919,8 +919,10 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	 * There are two sets of 30 bits to select IO rails, but bits 30 and
 	 * 31 are control bits rather than IO rail selection bits.
 	 */
-	if (id > 63 || *bit & IO_DPD_REQ_CODE_MASK)
+	if (id > 63 || *bit & IO_DPD_REQ_CODE_MASK) {
+		pr_err("%s(): invalid rail ID %d\n", __func__, id);
 		return -EINVAL;
+	}
 
 	if (id < 32) {
 		*status = IO_DPD_STATUS;
@@ -931,8 +933,10 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	}
 
 	rate = clk_get_rate(pmc->clk);
-	if (!rate)
+	if (!rate) {
+		pr_err("%s(): failed to get clock rate\n", __func__);
 		return -ENODEV;
+	}
 
 	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
 
@@ -983,7 +987,7 @@ int tegra_io_rail_power_on(unsigned int id)
 
 	err = tegra_io_rail_poll(status, bit, 0, 250);
 	if (err) {
-		pr_info("tegra_io_rail_poll() failed: %d\n", err);
+		pr_err("%s(): failed to power on rail %d\n", __func__, id);
 		goto error;
 	}
 
@@ -1005,16 +1009,16 @@ int tegra_io_rail_power_off(unsigned int id)
 	mutex_lock(&pmc->powergates_lock);
 
 	err = tegra_io_rail_prepare(id, &request, &status, &bit);
-	if (err) {
-		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
+	if (err)
 		goto error;
-	}
 
 	tegra_pmc_writel(IO_DPD_REQ_CODE_ON | bit, request);
 
 	err = tegra_io_rail_poll(status, bit, bit, 250);
-	if (err)
+	if (err) {
+		pr_err("%s(): failed to power off rail %d\n", __func__, id);
 		goto error;
+	}
 
 	tegra_io_rail_unprepare();
 
-- 
1.9.1

[PATCH 4/5] soc/tegra: pmc: Check return code for pm_genpd_init()

[Jon Hunter]

Commit 7eb231c337e0 ("PM / Domains: Convert pm_genpd_init() to return
an error code") updated pm_genpd_init() to return an error code. Update
the Tegra PMC driver to check the return value from pm_genpd_init() and
handle any errors returned.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/soc/tegra/pmc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 8c9797379949..b6efb62ad1dd 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -849,7 +849,12 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	    (id == TEGRA_POWERGATE_XUSBA || id == TEGRA_POWERGATE_XUSBC))
 		goto power_on_cleanup;
 
-	pm_genpd_init(&pg->genpd, NULL, off);
+	err = pm_genpd_init(&pg->genpd, NULL, off);
+	if (err < 0) {
+		dev_err(pmc->dev, "failed to initialise genpd for %s: %d\n",
+			np->name, err);
+		goto remove_resets;
+	}
 
 	err = of_genpd_add_provider_simple(np, &pg->genpd);
 	if (err < 0) {
-- 
1.9.1

[PATCH 5/5] soc/tegra: pmc: Remove genpd when adding provider fails

[Jon Hunter]

Commit 3fe577107ccf ("PM / Domains: Add support for removing PM
domains") add support for removing PM domains. Update the Tegra PMC
driver to remove PM domains if we fail to add a provider for the PM
domain.

Please note that the code under 'power_on_cleanup' label does not
really belong in the clean-up error path for tegra_powergate_add().
To keep the error path simple, remove this label and move the
associated code to where it needs to be invoked.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/soc/tegra/pmc.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index b6efb62ad1dd..ec4a33f49ff8 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -834,8 +834,11 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 		goto remove_clks;
 	}
 
-	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
-		goto power_on_cleanup;
+	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		if (off)
+			WARN_ON(tegra_powergate_power_up(pg, true));
+		goto remove_resets;
+	}
 
 	/*
 	 * FIXME: If XHCI is enabled for Tegra, then power-up the XUSB
@@ -846,8 +849,11 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	 * to be unused.
 	 */
 	if (IS_ENABLED(CONFIG_USB_XHCI_TEGRA) &&
-	    (id == TEGRA_POWERGATE_XUSBA || id == TEGRA_POWERGATE_XUSBC))
-		goto power_on_cleanup;
+	    (id == TEGRA_POWERGATE_XUSBA || id == TEGRA_POWERGATE_XUSBC)) {
+		if (off)
+			WARN_ON(tegra_powergate_power_up(pg, true));
+		goto remove_resets;
+	}
 
 	err = pm_genpd_init(&pg->genpd, NULL, off);
 	if (err < 0) {
@@ -860,16 +866,15 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	if (err < 0) {
 		dev_err(pmc->dev, "failed to add genpd provider for %s: %d\n",
 			np->name, err);
-		goto remove_resets;
+		goto remove_genpd;
 	}
 
 	dev_dbg(pmc->dev, "added power domain %s\n", pg->genpd.name);
 
 	return;
 
-power_on_cleanup:
-	if (off)
-		WARN_ON(tegra_powergate_power_up(pg, true));
+remove_genpd:
+	pm_genpd_remove(&pg->genpd);
 
 remove_resets:
 	while (pg->num_resets--)
-- 
1.9.1

Re: [PATCH 0/5] Tegra PMC Updates

[Thierry Reding]

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

On Sat, Oct 22, 2016 at 08:23:51PM +0100, Jon Hunter wrote:
> Some updates and clean-ups for the Tegra PMC driver.
> 
> Jon Hunter (5):
>   soc/tegra: pmc: Guard against uninitialised PMC clock
>   soc/tegra: pmc: Simplify IO rail bit handling
>   soc/tegra: Clean-up PMC rail IO error messages
>   soc/tegra: pmc: Check return code for pm_genpd_init()
>   soc/tegra: pmc: Remove genpd when adding provider fails
> 
>  drivers/soc/tegra/pmc.c | 56 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 20 deletions(-)

I've applied this series, though I had to rebase most of the patches on
top of Laxman's I/O pad work. As a side-effect most of patch 3 did no
longer apply, so I changed it a little.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]