Skip to contentSkip to conversation

NVIDIA Tegra core power domain follow up

PATCH In review Dmitry Osipenko 3 participants 6 messages Jun 1, 2021 – Jun 2, 2021 Tegra architecture support

lore
Share & tools
lore
#0 lore raw

cover NVIDIA Tegra core power domain follow up

Remove the lockdep_set_class(), which Ulf Hansson asked for. And prevent core domain syncing if domain node is missing in device-tree, which I accidentally missed to add after squashing the standalone domain driver into the PMC driver.

Dmitry Osipenko (2):
soc/tegra: pmc: Don't sync core domain if it's missing in device-tree soc/tegra: pmc: Remove usage of lockdep_set_class()

 drivers/soc/tegra/pmc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
Signature · 1 line
2.30.2
#1 lore raw

PATCH 1/2 soc/tegra: pmc: Don't sync core domain if it's missing in device-tree

Older device-trees don't have core domain node. In this case we shouldn't mark the domain as synced, allowing to begin core voltage scaling.

---
 drivers/soc/tegra/pmc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/soc/tegra/pmc.c +11 -0
index d8eee285fd58..d317532130c3 100644
@@ -431,6 +431,7 @@ struct tegra_pmc {
 	struct notifier_block clk_nb;
 
 	bool core_domain_state_synced;
+	bool core_domain_registered;
 };
 
 static struct tegra_pmc *pmc = &(struct tegra_pmc) {
@@ -1382,6 +1383,8 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
 		goto remove_genpd;
 	}
 
+	pmc->core_domain_registered = true;
+
 	return 0;
 
 remove_genpd:
@@ -3798,6 +3801,14 @@ static void tegra_pmc_sync_state(struct device *dev)
 {
 	int err;
 
+	/*
+	 * Older device-trees don't have core PD, and thus, there are
+	 * no dependencies that will block the state syncing. We shouldn't
+	 * mark the domain as synced in this case.
+	 */
+	if (!pmc->core_domain_registered)
+		return;
+
 	pmc->core_domain_state_synced = true;
 
 	/* this is a no-op if core regulator isn't used */
Signature · 1 line
2.30.2
#2 lore raw

PATCH 2/2 soc/tegra: pmc: Remove usage of lockdep_set_class()

In reply to Dmitry Osipenko

The lockdep_set_class() will become necessary only if we will have a nested toggling of core / PMC power domains that will happen if we will add RPM and OPP support to Tegra CCF driver. Ulf Hansson has concerns about whether this is the best approach. He suggested to remove the lockdep class annotation until we will agree on how the OPP support of clocks should be implemented, hence remove it for now.

---
 drivers/soc/tegra/pmc.c | 10 ----------
 1 file changed, 10 deletions(-)
drivers/soc/tegra/pmc.c +0 -10
index d317532130c3..ea62f84d1c8b 100644
@@ -1344,7 +1344,6 @@ tegra_pmc_core_pd_opp_to_performance_state(struct generic_pm_domain *genpd,
 
 static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
 {
-	static struct lock_class_key tegra_core_domain_lock_class;
 	struct generic_pm_domain *genpd;
 	const char *rname = "core";
 	int err;
@@ -1368,15 +1367,6 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
 		return err;
 	}
 
-	/*
-	 * We have a "PMC pwrgate -> Core" hierarchy of the power domains
-	 * where PMC needs to resume and change performance (voltage) of the
-	 * Core domain from the PMC GENPD on/off callbacks, hence we need
-	 * to annotate the lock in order to remove confusion from the
-	 * lockdep checker when a nested access happens.
-	 */
-	lockdep_set_class(&genpd->mlock, &tegra_core_domain_lock_class);
-
 	err = of_genpd_add_provider_simple(np, genpd);
 	if (err) {
 		dev_err(pmc->dev, "failed to add core genpd: %d\n", err);
Signature · 1 line
2.30.2
Reviewed-by on the v1 series
#3 lore raw
In reply to Dmitry Osipenko
Quoted 6 lines · Dmitry Osipenko

Remove the lockdep_set_class(), which Ulf Hansson asked for. And prevent core domain syncing if domain node is missing in device-tree, which I accidentally missed to add after squashing the standalone domain driver into the PMC driver.

Quoted 7 lines

Dmitry Osipenko (2):
soc/tegra: pmc: Don't sync core domain if it's missing in device-tree soc/tegra: pmc: Remove usage of lockdep_set_class()

 drivers/soc/tegra/pmc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Kind regards
Uffe

#4 lore raw
In reply to Dmitry Osipenko
Quoted 11 lines · Dmitry Osipenko

Remove the lockdep_set_class(), which Ulf Hansson asked for. And prevent core domain syncing if domain node is missing in device-tree, which I accidentally missed to add after squashing the standalone domain driver into the PMC driver.

Dmitry Osipenko (2):
soc/tegra: pmc: Don't sync core domain if it's missing in device-tree soc/tegra: pmc: Remove usage of lockdep_set_class()

 drivers/soc/tegra/pmc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

I've squashed these into the corresponding patches of you v6 series and added Ulf's reviewed-by to them.

Ulf, let me know if I misinterpreted the discussion and your reviewed-by didn't extend to the original patches.

Thierry

#5 lore raw
Quoted 11 lines · Thierry Reding
Quoted 9 lines · Dmitry Osipenko

Remove the lockdep_set_class(), which Ulf Hansson asked for. And prevent core domain syncing if domain node is missing in device-tree, which I accidentally missed to add after squashing the standalone domain driver into the PMC driver.

Dmitry Osipenko (2):
soc/tegra: pmc: Don't sync core domain if it's missing in device-tree soc/tegra: pmc: Remove usage of lockdep_set_class()

Quoted 8 lines
Quoted 2 lines
 drivers/soc/tegra/pmc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

I've squashed these into the corresponding patches of you v6 series and added Ulf's reviewed-by to them.

Ulf, let me know if I misinterpreted the discussion and your reviewed-by didn't extend to the original patches.

Nope, looks good to me! Thanks!

Kind regards
Uffe