All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V7 0/4] Add generic PM domain support for Tegra
@ 2016-03-04 12:23 Jon Hunter
  2016-03-04 12:23 ` [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC Jon Hunter
       [not found] ` <1457094186-15786-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 2 replies; 15+ messages in thread
From: Jon Hunter @ 2016-03-04 12:23 UTC (permalink / raw)
  To: Stephen Warren, Thierry Reding, Alexandre Courbot
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Jon Hunter

Adds generic PM domain support for Tegra SoCs but this series only
enables support for it on Tegra 64-bit devices. There is no reason why
this cannot be enable for Tegra 32-bit devices, but to keep the patch
series to a minimum only 64-bit devices are enabled so far.

This series has been boot tested on Tegra210 as well as various 32-bit
Tegra platforms.

Summary of changes since V6:
- Dropped removal of PM domains, while infrastructure for removing PM
  domains has been sorted out [4]. Please note that because the PMC driver
  is never removed, we can simply create the PM domains at the very end
  of the PMC probe and WARN if any of the PM domains cannot be
  instantiated.
- Dropped support for subdomains because there is currently no need to
  support subdomains for Tegra and this simplifies the code substantially.
- Updated DT bindings for Tegra powergates based upon feedback from
  Thierry Reding.
- Added patch to remove "#power-domain-cells" that in the Tegra210
  device-tree source that was added incorrectly.

Summary of changes since V5 [2]:
- Split series into 2 series, separating PMC fixes [3] from genpd changes
- This series is based upon fixes series (which is now in -next)
- Added patch to fix removal of genpd subdomain
- Added patch to get last genpd that was added
- Fixed locking in function to remove genpd
- Updated Tegra PMC driver to ensure power-domains cannot be controlled
  by both legacy Tegra APIs and generic PM domain framework
- Removed powergate list from Tegra PMC driver and updated removal of
  power domains to use new genpd APIs.

Summary of changes since V4 [1]:
- Re-worked fix to handle base address on probe failure
- Added patch to lock around simultaneuous accesses to PMC registers
- Added patch to change powergate and rail IDs to unsigned type
- Added patch to fix testing of powergate state
- Updated patch to check for valid powergates to use a bitmap
- Updated Tegra DT PMC bindings per Rob H's feedback
- Updated Tegra power domains binding per Thierry's feedback
- Updated Tegra generic power domain support per Thierry's feedback

Summary of changes since V3 [0]:
- Dropped tegra124 support for now
- Removed MC flush support per feedback from Thierry
- Cleaned up the PMC changes per feedback from Thierry
- Added support for tegra210

[0] http://comments.gmane.org/gmane.linux.ports.tegra/22944
[1] http://marc.info/?l=linux-tegra&m=144924153600529&w=2
[2] http://marc.info/?l=linux-tegra&m=145399885003830&w=2
[3] http://marc.info/?l=linux-tegra&m=145521381331144&w=2
[4] http://marc.info/?l=linux-pm&m=145709064407085&w=2

Jon Hunter (4):
  ARM64: tegra: Remove unused #power-domain-cells property
  Documentation: DT: bindings: Add power domain info for NVIDIA PMC
  soc: tegra: pmc: Add generic PM domain support
  ARM64: tegra: select PM_GENERIC_DOMAINS

 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  80 ++++
 arch/arm64/Kconfig.platforms                       |   2 +
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   2 -
 drivers/soc/tegra/pmc.c                            | 485 ++++++++++++++++++---
 include/soc/tegra/pmc.h                            |   1 +
 5 files changed, 507 insertions(+), 63 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property
       [not found] ` <1457094186-15786-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2016-03-04 12:23   ` Jon Hunter
  2016-03-04 14:13     ` Thierry Reding
  2016-03-04 12:23   ` [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support Jon Hunter
  2016-03-04 12:23   ` [PATCH V7 4/4] ARM64: tegra: select PM_GENERIC_DOMAINS Jon Hunter
  2 siblings, 1 reply; 15+ messages in thread
From: Jon Hunter @ 2016-03-04 12:23 UTC (permalink / raw)
  To: Stephen Warren, Thierry Reding, Alexandre Courbot
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Jon Hunter

Remove the "#power-domain-cells" property which was incorrectly
included by commit e53095857166 ("arm64: tegra: Add Tegra210
support").

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index cd4f45ccd6a7..6cd2048defe2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -580,8 +580,6 @@
 		reg = <0x0 0x7000e400 0x0 0x400>;
 		clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
 		clock-names = "pclk", "clk32k_in";
-
-		#power-domain-cells = <1>;
 	};
 
 	fuse@0,7000f800 {
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
  2016-03-04 12:23 [PATCH V7 0/4] Add generic PM domain support for Tegra Jon Hunter
@ 2016-03-04 12:23 ` Jon Hunter
  2016-03-05  4:31   ` Rob Herring
       [not found] ` <1457094186-15786-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: Jon Hunter @ 2016-03-04 12:23 UTC (permalink / raw)
  To: Stephen Warren, Thierry Reding, Alexandre Courbot
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, linux-tegra,
	devicetree, linux-pm, Jon Hunter

Add power-domain binding documentation for the NVIDIA PMC driver in
order to support generic power-domains.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80 ++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
index 53aa5496c5cf..7d52bbe99709 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
@@ -1,5 +1,7 @@
 NVIDIA Tegra Power Management Controller (PMC)
 
+== Power Management Controller Node ==
+
 The PMC block interacts with an external Power Management Unit. The PMC
 mostly controls the entry and exit of the system from different sleep
 modes. It provides power-gating controllers for SoC and CPU power-islands.
@@ -70,6 +72,11 @@ Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'
                      Defaults to 0. Valid values are described in section 12.5.2
                      "Pinmux Support" of the Tegra4 Technical Reference Manual.
 
+Optional nodes:
+- powergates : This node contains a hierarchy of power domain nodes, which
+	       should match the powergates on the Tegra SoC. See "Powergate
+	       Nodes" below.
+
 Example:
 
 / SoC dts including file
@@ -115,3 +122,76 @@ pmc@7000f400 {
 	};
 	...
 };
+
+
+== Powergate Nodes ==
+
+Each of the powergate nodes represent a power-domain on the Tegra SoC
+that can be power-gated by the Tegra PMC. The name of the powergate node
+should be one of the below. Note that not every powergate is applicable
+to all Tegra devices and the following list shows which powergates are
+applicable to which devices. Please refer to the Tegra TRM for more
+details on the various powergates.
+
+ Name		Description			Devices Applicable
+ 3d		3D Graphics			Tegra20/114/124/210
+ 3d0		3D Graphics 0			Tegra30
+ 3d1		3D Graphics 1			Tegra30
+ aud		Audio				Tegra210
+ dfd		Debug				Tegra210
+ dis		Display A			Tegra114/124/210
+ disb		Display B			Tegra114/124/210
+ heg		2D Graphics			Tegra30/114/124/210
+ iram		Internal RAM			Tegra124/210
+ mpe		MPEG Encode			All
+ nvdec		NVIDIA Video Decode Engine	Tegra210
+ nvjpg		NVIDIA JPEG Engine		Tegra210
+ pcie		PCIE				Tegra20/30/124/210
+ sata		SATA				Tegra30/124/210
+ sor		Display interfaces		Tegra124/210
+ ve2		Video Encode Engine 2		Tegra210
+ venc		Video Encode Engine		All
+ vdec		Video Decode Engine		Tegra20/30/114/124
+ vic		Video Imaging Compositor	Tegra124/210
+ xusba		USB Partition A			Tegra114/124/210
+ xusbb		USB Partition B 		Tegra114/124/210
+ xusbc		USB Partition C			Tegra114/124/210
+
+Required properties:
+  - clocks: Must contain an entry for each clock required by the PMC for
+    controlling a power-gate. See ../clocks/clock-bindings.txt for details.
+  - resets: Must contain an entry for each reset required by the PMC for
+    controlling a power-gate. See ../reset/reset.txt for details.
+  - #power-domain-cells: Must be 0.
+
+Example:
+
+	pmc: pmc@0,7000e400 {
+		compatible = "nvidia,tegra210-pmc";
+		reg = <0x0 0x7000e400 0x0 0x400>;
+		clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
+		clock-names = "pclk", "clk32k_in";
+
+		powergates {
+			pd_audio: aud {
+				clocks = <&tegra_car TEGRA210_CLK_APE>,
+					 <&tegra_car TEGRA210_CLK_APB2APE>;
+				resets = <&tegra_car 198>;
+				#power-domain-cells = <0>;
+			};
+		};
+	};
+
+
+== Powergate Clients ==
+
+Hardware blocks belonging to a power domain should contain a "power-domains"
+property that is a phandle pointing to the corresponding powergate node.
+
+Example:
+
+	adma: adma@702e2000 {
+		...
+		power-domains = <&pd_audio>;
+		...
+	};
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support
       [not found] ` <1457094186-15786-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2016-03-04 12:23   ` [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property Jon Hunter
@ 2016-03-04 12:23   ` Jon Hunter
  2016-03-08 21:28     ` Ulf Hansson
  2016-03-04 12:23   ` [PATCH V7 4/4] ARM64: tegra: select PM_GENERIC_DOMAINS Jon Hunter
  2 siblings, 1 reply; 15+ messages in thread
From: Jon Hunter @ 2016-03-04 12:23 UTC (permalink / raw)
  To: Stephen Warren, Thierry Reding, Alexandre Courbot
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Jon Hunter

Adds generic PM support to the PMC driver where the PM domains are
populated from device-tree and the PM domain consumer devices are
bound to their relevant PM domains via device-tree as well.

Update the tegra_powergate_sequence_power_up() API so that internally
it calls the same tegra_powergate_xxx functions that are used by the
tegra generic power domain code for consistency.

To ensure that the Tegra power domains (a.k.a powergates) cannot be
controlled via both the legacy tegra_powergate_xxx functions as well
as the generic PM domain framework, add a bit map for available
powergates that can be controlled via the legacy powergate functions.

Move the majority of the tegra_powergate_remove_clamping() function
to a sub-function, so that this can be used by both the legacy and
generic power domain code.

Currently, the power domains are not removed once added because this
is not yet supported by the PM domains framework. An error message
will be displayed if we are unable to instantiate a power domain.

This is based upon work by Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
and Vince Hsu <vinceh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>.

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

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 08966c26d65c..bb173456bbff 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -31,10 +31,13 @@
 #include <linux/iopoll.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/reboot.h>
 #include <linux/reset.h>
 #include <linux/seq_file.h>
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 
 #include <soc/tegra/common.h>
@@ -102,6 +105,16 @@
 
 #define GPU_RG_CNTRL			0x2d4
 
+struct tegra_powergate {
+	struct generic_pm_domain genpd;
+	struct tegra_pmc *pmc;
+	unsigned int id;
+	struct clk **clks;
+	unsigned int num_clks;
+	struct reset_control **resets;
+	unsigned int num_resets;
+};
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
@@ -132,6 +145,7 @@ struct tegra_pmc_soc {
  * @cpu_pwr_good_en: CPU power good signal is enabled
  * @lp0_vec_phys: physical base address of the LP0 warm boot code
  * @lp0_vec_size: size of the LP0 warm boot code
+ * @powergates_available: Bitmap of available power gates
  * @powergates_lock: mutex for power gate register access
  */
 struct tegra_pmc {
@@ -156,6 +170,7 @@ struct tegra_pmc {
 	bool cpu_pwr_good_en;
 	u32 lp0_vec_phys;
 	u32 lp0_vec_size;
+	DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
 
 	struct mutex powergates_lock;
 };
@@ -165,6 +180,12 @@ static struct tegra_pmc *pmc = &(struct tegra_pmc) {
 	.suspend_mode = TEGRA_SUSPEND_NONE,
 };
 
+static inline struct tegra_powergate *
+to_powergate(struct generic_pm_domain *domain)
+{
+	return container_of(domain, struct tegra_powergate, genpd);
+}
+
 static u32 tegra_pmc_readl(unsigned long offset)
 {
 	return readl(pmc->base + offset);
@@ -188,6 +209,31 @@ static inline bool tegra_powergate_is_valid(int id)
 	return (pmc->soc && pmc->soc->powergates[id]);
 }
 
+static inline bool tegra_powergate_is_available(int id)
+{
+	return test_bit(id, pmc->powergates_available);
+}
+
+static int tegra_powergate_lookup(struct tegra_pmc *pmc, const char *name)
+{
+	unsigned int i;
+
+	if (!pmc || !pmc->soc || !name)
+		return -EINVAL;
+
+	for (i = 0; i < pmc->soc->num_powergates; i++) {
+		if (!tegra_powergate_is_valid(i))
+			continue;
+
+		if (!strcmp(name, pmc->soc->powergates[i]))
+			return i;
+	}
+
+	dev_err(pmc->dev, "powergate %s not found\n", name);
+
+	return -ENODEV;
+}
+
 /**
  * tegra_powergate_set() - set the state of a partition
  * @id: partition ID
@@ -218,13 +264,219 @@ static int tegra_powergate_set(unsigned int id, bool new_state)
 	return err;
 }
 
+static int __tegra_powergate_remove_clamping(unsigned int id)
+{
+	u32 mask;
+
+	mutex_lock(&pmc->powergates_lock);
+
+	/*
+	 * On Tegra124 and later, the clamps for the GPU are controlled by a
+	 * separate register (with different semantics).
+	 */
+	if (id == TEGRA_POWERGATE_3D) {
+		if (pmc->soc->has_gpu_clamps) {
+			tegra_pmc_writel(0, GPU_RG_CNTRL);
+			goto out;
+		}
+	}
+
+	/*
+	 * Tegra 2 has a bug where PCIE and VDE clamping masks are
+	 * swapped relatively to the partition ids
+	 */
+	if (id == TEGRA_POWERGATE_VDEC)
+		mask = (1 << TEGRA_POWERGATE_PCIE);
+	else if (id == TEGRA_POWERGATE_PCIE)
+		mask = (1 << TEGRA_POWERGATE_VDEC);
+	else
+		mask = (1 << id);
+
+	tegra_pmc_writel(mask, REMOVE_CLAMPING);
+
+out:
+	mutex_unlock(&pmc->powergates_lock);
+
+	return 0;
+}
+
+static void tegra_powergate_disable_clocks(struct tegra_powergate *pg)
+{
+	unsigned int i;
+
+	for (i = 0; i < pg->num_clks; i++)
+		clk_disable_unprepare(pg->clks[i]);
+}
+
+static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
+{
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < pg->num_clks; i++) {
+		err = clk_prepare_enable(pg->clks[i]);
+		if (err)
+			goto out;
+	}
+
+	return 0;
+
+out:
+	while (i--)
+		clk_disable_unprepare(pg->clks[i]);
+
+	return err;
+}
+
+static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
+{
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < pg->num_resets; i++) {
+		err = reset_control_assert(pg->resets[i]);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
+{
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < pg->num_resets; i++) {
+		err = reset_control_deassert(pg->resets[i]);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int tegra_powergate_power_up(struct tegra_powergate *pg,
+				    bool disable_clocks)
+{
+	int err;
+
+	err = tegra_powergate_reset_assert(pg);
+	if (err)
+		return err;
+
+	usleep_range(10, 20);
+
+	err = tegra_powergate_set(pg->id, true);
+	if (err < 0)
+		return err;
+
+	usleep_range(10, 20);
+
+	err = tegra_powergate_enable_clocks(pg);
+	if (err)
+		goto disable_clks;
+
+	usleep_range(10, 20);
+
+	err = __tegra_powergate_remove_clamping(pg->id);
+	if (err)
+		goto disable_clks;
+
+	usleep_range(10, 20);
+
+	err = tegra_powergate_reset_deassert(pg);
+	if (err)
+		goto powergate_off;
+
+	usleep_range(10, 20);
+
+	if (disable_clocks)
+		tegra_powergate_disable_clocks(pg);
+
+	return 0;
+
+disable_clks:
+	tegra_powergate_disable_clocks(pg);
+	usleep_range(10, 20);
+powergate_off:
+	tegra_powergate_set(pg->id, false);
+
+	return err;
+}
+
+static int tegra_powergate_power_down(struct tegra_powergate *pg)
+{
+	int err;
+
+	err = tegra_powergate_enable_clocks(pg);
+	if (err)
+		return err;
+
+	usleep_range(10, 20);
+
+	err = tegra_powergate_reset_assert(pg);
+	if (err)
+		goto disable_clks;
+
+	usleep_range(10, 20);
+
+	tegra_powergate_disable_clocks(pg);
+
+	usleep_range(10, 20);
+
+	err = tegra_powergate_set(pg->id, false);
+	if (err)
+		goto assert_resets;
+
+	return 0;
+
+assert_resets:
+	tegra_powergate_enable_clocks(pg);
+	usleep_range(10, 20);
+	tegra_powergate_reset_deassert(pg);
+	usleep_range(10, 20);
+disable_clks:
+	tegra_powergate_disable_clocks(pg);
+
+	return err;
+}
+
+static int tegra_genpd_power_on(struct generic_pm_domain *domain)
+{
+	struct tegra_powergate *pg = to_powergate(domain);
+	struct tegra_pmc *pmc = pg->pmc;
+	int err;
+
+	err = tegra_powergate_power_up(pg, true);
+	if (err)
+		dev_err(pmc->dev, "failed to turn on PM domain %s: %d\n",
+			pg->genpd.name, err);
+
+	return err;
+}
+
+static int tegra_genpd_power_off(struct generic_pm_domain *domain)
+{
+	struct tegra_powergate *pg = to_powergate(domain);
+	struct tegra_pmc *pmc = pg->pmc;
+	int err;
+
+	err = tegra_powergate_power_down(pg);
+	if (err)
+		dev_err(pmc->dev, "failed to turn off PM domain %s: %d\n",
+			pg->genpd.name, err);
+
+	return err;
+}
+
 /**
  * tegra_powergate_power_on() - power on partition
  * @id: partition ID
  */
 int tegra_powergate_power_on(unsigned int id)
 {
-	if (!tegra_powergate_is_valid(id))
+	if (!tegra_powergate_is_available(id))
 		return -EINVAL;
 
 	return tegra_powergate_set(id, true);
@@ -236,7 +488,7 @@ int tegra_powergate_power_on(unsigned int id)
  */
 int tegra_powergate_power_off(unsigned int id)
 {
-	if (!tegra_powergate_is_valid(id))
+	if (!tegra_powergate_is_available(id))
 		return -EINVAL;
 
 	return tegra_powergate_set(id, false);
@@ -267,41 +519,10 @@ int tegra_powergate_is_powered(unsigned int id)
  */
 int tegra_powergate_remove_clamping(unsigned int id)
 {
-	u32 mask;
-
-	if (!tegra_powergate_is_valid(id))
+	if (!tegra_powergate_is_available(id))
 		return -EINVAL;
 
-	mutex_lock(&pmc->powergates_lock);
-
-	/*
-	 * On Tegra124 and later, the clamps for the GPU are controlled by a
-	 * separate register (with different semantics).
-	 */
-	if (id == TEGRA_POWERGATE_3D) {
-		if (pmc->soc->has_gpu_clamps) {
-			tegra_pmc_writel(0, GPU_RG_CNTRL);
-			goto out;
-		}
-	}
-
-	/*
-	 * Tegra 2 has a bug where PCIE and VDE clamping masks are
-	 * swapped relatively to the partition ids
-	 */
-	if (id == TEGRA_POWERGATE_VDEC)
-		mask = (1 << TEGRA_POWERGATE_PCIE);
-	else if (id == TEGRA_POWERGATE_PCIE)
-		mask = (1 << TEGRA_POWERGATE_VDEC);
-	else
-		mask = (1 << id);
-
-	tegra_pmc_writel(mask, REMOVE_CLAMPING);
-
-out:
-	mutex_unlock(&pmc->powergates_lock);
-
-	return 0;
+	return __tegra_powergate_remove_clamping(id);
 }
 EXPORT_SYMBOL(tegra_powergate_remove_clamping);
 
@@ -316,35 +537,20 @@ EXPORT_SYMBOL(tegra_powergate_remove_clamping);
 int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 				      struct reset_control *rst)
 {
-	int ret;
-
-	reset_control_assert(rst);
-
-	ret = tegra_powergate_power_on(id);
-	if (ret)
-		goto err_power;
-
-	ret = clk_prepare_enable(clk);
-	if (ret)
-		goto err_clk;
-
-	usleep_range(10, 20);
+	struct tegra_powergate pg;
+	int err;
 
-	ret = tegra_powergate_remove_clamping(id);
-	if (ret)
-		goto err_clamp;
+	pg.id = id;
+	pg.clks = &clk;
+	pg.num_clks = 1;
+	pg.resets = &rst;
+	pg.num_resets = 1;
 
-	usleep_range(10, 20);
-	reset_control_deassert(rst);
-
-	return 0;
+	err = tegra_powergate_power_up(&pg, false);
+	if (err)
+		pr_err("failed to turn on partition %d: %d\n", id, err);
 
-err_clamp:
-	clk_disable_unprepare(clk);
-err_clk:
-	tegra_powergate_power_off(id);
-err_power:
-	return ret;
+	return err;
 }
 EXPORT_SYMBOL(tegra_powergate_sequence_power_up);
 
@@ -486,6 +692,155 @@ static int tegra_powergate_debugfs_init(void)
 	return 0;
 }
 
+static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
+				       struct device_node *np)
+{
+	struct clk *clk;
+	unsigned int i, count;
+	int err;
+
+	count = of_count_phandle_with_args(np, "clocks", "#clock-cells");
+	if (count == 0)
+		return -ENODEV;
+
+	pg->clks = kcalloc(count, sizeof(clk), GFP_KERNEL);
+	if (!pg->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		pg->clks[i] = of_clk_get(np, i);
+		if (IS_ERR(pg->clks[i])) {
+			err = PTR_ERR(pg->clks[i]);
+			goto err;
+		}
+	}
+
+	pg->num_clks = count;
+
+	return 0;
+
+err:
+	while (i--)
+		clk_put(pg->clks[i]);
+	kfree(pg->clks);
+
+	return err;
+}
+
+static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
+					 struct device_node *np)
+{
+	struct reset_control *rst;
+	unsigned int i, count;
+	int err;
+
+	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
+	if (count == 0)
+		return -ENODEV;
+
+	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
+	if (!pg->resets)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		pg->resets[i] = of_reset_control_get_by_index(np, i);
+		if (IS_ERR(pg->resets[i])) {
+			err = PTR_ERR(pg->resets[i]);
+			goto error;
+		}
+	}
+
+	pg->num_resets = count;
+
+	return 0;
+
+error:
+	while (i--)
+		reset_control_put(pg->resets[i]);
+	kfree(pg->resets);
+
+	return err;
+}
+
+static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
+{
+	struct tegra_powergate *pg;
+	bool off;
+	int id;
+
+	pg = kzalloc(sizeof(*pg), GFP_KERNEL);
+	if (!pg)
+		goto error;
+
+	id = tegra_powergate_lookup(pmc, np->name);
+	if (id < 0)
+		goto free_mem;
+
+	/*
+	 * Clear the bit for this powergate so it cannot be managed
+	 * directly via the legacy APIs for controlling powergates.
+	 */
+	clear_bit(id, pmc->powergates_available);
+
+	pg->id = id;
+	pg->genpd.name = np->name;
+	pg->genpd.power_off = tegra_genpd_power_off;
+	pg->genpd.power_on = tegra_genpd_power_on;
+	pg->pmc = pmc;
+
+	if (tegra_powergate_of_get_clks(pg, np))
+		goto set_available;
+
+	if (tegra_powergate_of_get_resets(pg, np))
+		goto remove_clks;
+
+	off = !tegra_powergate_is_powered(pg->id);
+
+	pm_genpd_init(&pg->genpd, NULL, off);
+
+	if (of_genpd_add_provider_simple(np, &pg->genpd))
+		goto remove_resets;
+
+	dev_dbg(pmc->dev, "added power domain %s\n", pg->genpd.name);
+
+	return;
+
+remove_resets:
+	while (pg->num_resets--)
+		reset_control_put(pg->resets[pg->num_resets]);
+	kfree(pg->resets);
+
+remove_clks:
+	while (pg->num_clks--)
+		clk_put(pg->clks[pg->num_clks]);
+	kfree(pg->clks);
+
+set_available:
+	set_bit(id, pmc->powergates_available);
+
+free_mem:
+	kfree(pg);
+
+error:
+	dev_err(pmc->dev, "failed to create power domain for %s\n", np->name);
+}
+
+static void tegra_powergate_init(struct tegra_pmc *pmc)
+{
+	struct device_node *np, *child;
+
+	np = of_get_child_by_name(pmc->dev->of_node, "powergates");
+	if (!np)
+		return;
+
+	for_each_child_of_node(np, child) {
+		tegra_powergate_add(pmc, child);
+		of_node_put(child);
+	}
+
+	of_node_put(np);
+}
+
 static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 				 unsigned long *status, unsigned int *bit)
 {
@@ -887,6 +1242,8 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	tegra_powergate_init(pmc);
+
 	mutex_lock(&pmc->powergates_lock);
 	iounmap(pmc->base);
 	pmc->base = base;
@@ -1120,6 +1477,7 @@ static int __init tegra_pmc_early_init(void)
 	const struct of_device_id *match;
 	struct device_node *np;
 	struct resource regs;
+	unsigned int i;
 	bool invert;
 	u32 value;
 
@@ -1169,6 +1527,11 @@ static int __init tegra_pmc_early_init(void)
 		return -ENXIO;
 	}
 
+	/* Create a bit-map of the available and valid partitions */
+	for (i = 0; i < pmc->soc->num_powergates; i++)
+		if (pmc->soc->powergates[i])
+			set_bit(i, pmc->powergates_available);
+
 	mutex_init(&pmc->powergates_lock);
 
 	/*
diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
index 07e332dd44fb..e9e53473a63e 100644
--- a/include/soc/tegra/pmc.h
+++ b/include/soc/tegra/pmc.h
@@ -72,6 +72,7 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
 #define TEGRA_POWERGATE_AUD	27
 #define TEGRA_POWERGATE_DFD	28
 #define TEGRA_POWERGATE_VE2	29
+#define TEGRA_POWERGATE_MAX	TEGRA_POWERGATE_VE2
 
 #define TEGRA_POWERGATE_3D0	TEGRA_POWERGATE_3D
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V7 4/4] ARM64: tegra: select PM_GENERIC_DOMAINS
       [not found] ` <1457094186-15786-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2016-03-04 12:23   ` [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property Jon Hunter
  2016-03-04 12:23   ` [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support Jon Hunter
@ 2016-03-04 12:23   ` Jon Hunter
  2 siblings, 0 replies; 15+ messages in thread
From: Jon Hunter @ 2016-03-04 12:23 UTC (permalink / raw)
  To: Stephen Warren, Thierry Reding, Alexandre Courbot
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Jon Hunter

Enable PM_GENERIC_DOMAINS for tegra 64-bit devices. To ensure that devices
dependent upon a particular power-domain are only probed when that power
domain has been powered up, requires that PM is made mandatory for tegra
64-bit devices and so select this option for tegra as well.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/Kconfig.platforms | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index c2d621fc7e01..9da2a0952c7d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -123,6 +123,8 @@ config ARCH_TEGRA
 	select GENERIC_CLOCKEVENTS
 	select HAVE_CLK
 	select PINCTRL
+	select PM
+	select PM_GENERIC_DOMAINS
 	select RESET_CONTROLLER
 	help
 	  This enables support for the NVIDIA Tegra SoC family.
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property
  2016-03-04 12:23   ` [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property Jon Hunter
@ 2016-03-04 14:13     ` Thierry Reding
  2016-03-04 14:19       ` Jon Hunter
  0 siblings, 1 reply; 15+ messages in thread
From: Thierry Reding @ 2016-03-04 14:13 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Stephen Warren, Alexandre Courbot, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Rafael J. Wysocki,
	Kevin Hilman, Ulf Hansson, linux-tegra, devicetree, linux-pm

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

On Fri, Mar 04, 2016 at 12:23:03PM +0000, Jon Hunter wrote:
> Remove the "#power-domain-cells" property which was incorrectly
> included by commit e53095857166 ("arm64: tegra: Add Tegra210
> support").
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 --
>  1 file changed, 2 deletions(-)

Jon, can you resend this to arm@kernel.org to get this applied before
the final v4.5? Feel free to add:

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property
  2016-03-04 14:13     ` Thierry Reding
@ 2016-03-04 14:19       ` Jon Hunter
  0 siblings, 0 replies; 15+ messages in thread
From: Jon Hunter @ 2016-03-04 14:19 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Stephen Warren, Alexandre Courbot, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Rafael J. Wysocki,
	Kevin Hilman, Ulf Hansson, linux-tegra, devicetree, linux-pm


On 04/03/16 14:13, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Mar 04, 2016 at 12:23:03PM +0000, Jon Hunter wrote:
>> Remove the "#power-domain-cells" property which was incorrectly
>> included by commit e53095857166 ("arm64: tegra: Add Tegra210
>> support").
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 --
>>  1 file changed, 2 deletions(-)
> 
> Jon, can you resend this to arm@kernel.org to get this applied before
> the final v4.5? Feel free to add:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>

Yes, no problem.

Jon

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
  2016-03-04 12:23 ` [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC Jon Hunter
@ 2016-03-05  4:31   ` Rob Herring
  2016-03-07 10:00     ` Jon Hunter
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2016-03-05  4:31 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Rafael J. Wysocki,
	Kevin Hilman, Ulf Hansson, linux-tegra, devicetree, linux-pm

On Fri, Mar 04, 2016 at 12:23:04PM +0000, Jon Hunter wrote:
> Add power-domain binding documentation for the NVIDIA PMC driver in
> order to support generic power-domains.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80 ++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> index 53aa5496c5cf..7d52bbe99709 100644
> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> @@ -1,5 +1,7 @@
>  NVIDIA Tegra Power Management Controller (PMC)
>  
> +== Power Management Controller Node ==
> +
>  The PMC block interacts with an external Power Management Unit. The PMC
>  mostly controls the entry and exit of the system from different sleep
>  modes. It provides power-gating controllers for SoC and CPU power-islands.
> @@ -70,6 +72,11 @@ Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'
>                       Defaults to 0. Valid values are described in section 12.5.2
>                       "Pinmux Support" of the Tegra4 Technical Reference Manual.
>  
> +Optional nodes:
> +- powergates : This node contains a hierarchy of power domain nodes, which
> +	       should match the powergates on the Tegra SoC. See "Powergate
> +	       Nodes" below.
> +
>  Example:
>  
>  / SoC dts including file
> @@ -115,3 +122,76 @@ pmc@7000f400 {
>  	};
>  	...
>  };
> +
> +
> +== Powergate Nodes ==
> +
> +Each of the powergate nodes represent a power-domain on the Tegra SoC
> +that can be power-gated by the Tegra PMC. The name of the powergate node
> +should be one of the below. Note that not every powergate is applicable
> +to all Tegra devices and the following list shows which powergates are
> +applicable to which devices. Please refer to the Tegra TRM for more
> +details on the various powergates.
> +
> + Name		Description			Devices Applicable
> + 3d		3D Graphics			Tegra20/114/124/210
> + 3d0		3D Graphics 0			Tegra30
> + 3d1		3D Graphics 1			Tegra30
> + aud		Audio				Tegra210
> + dfd		Debug				Tegra210
> + dis		Display A			Tegra114/124/210
> + disb		Display B			Tegra114/124/210
> + heg		2D Graphics			Tegra30/114/124/210
> + iram		Internal RAM			Tegra124/210
> + mpe		MPEG Encode			All
> + nvdec		NVIDIA Video Decode Engine	Tegra210
> + nvjpg		NVIDIA JPEG Engine		Tegra210
> + pcie		PCIE				Tegra20/30/124/210
> + sata		SATA				Tegra30/124/210
> + sor		Display interfaces		Tegra124/210
> + ve2		Video Encode Engine 2		Tegra210
> + venc		Video Encode Engine		All
> + vdec		Video Decode Engine		Tegra20/30/114/124
> + vic		Video Imaging Compositor	Tegra124/210
> + xusba		USB Partition A			Tegra114/124/210
> + xusbb		USB Partition B 		Tegra114/124/210
> + xusbc		USB Partition C			Tegra114/124/210
> +
> +Required properties:
> +  - clocks: Must contain an entry for each clock required by the PMC for
> +    controlling a power-gate. See ../clocks/clock-bindings.txt for details.
> +  - resets: Must contain an entry for each reset required by the PMC for
> +    controlling a power-gate. See ../reset/reset.txt for details.
> +  - #power-domain-cells: Must be 0.
> +
> +Example:
> +
> +	pmc: pmc@0,7000e400 {

Just pmc@7000e400. We were erronously using commas for 64-bit addresses 
briefly, but the correct usage is when there are distinct fields in reg 
like PCI bus,dev,func.

Otherwise:

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
  2016-03-05  4:31   ` Rob Herring
@ 2016-03-07 10:00     ` Jon Hunter
       [not found]       ` <56DD515A.4020207-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2016-03-07 17:47       ` Stephen Warren
  0 siblings, 2 replies; 15+ messages in thread
From: Jon Hunter @ 2016-03-07 10:00 UTC (permalink / raw)
  To: Rob Herring, Stephen Warren, Thierry Reding
  Cc: Alexandre Courbot, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA


On 05/03/16 04:31, Rob Herring wrote:
> On Fri, Mar 04, 2016 at 12:23:04PM +0000, Jon Hunter wrote:
>> Add power-domain binding documentation for the NVIDIA PMC driver in
>> order to support generic power-domains.
>>
>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80 ++++++++++++++++++++++
>>  1 file changed, 80 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> index 53aa5496c5cf..7d52bbe99709 100644
>> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> @@ -1,5 +1,7 @@
>>  NVIDIA Tegra Power Management Controller (PMC)
>>  
>> +== Power Management Controller Node ==
>> +
>>  The PMC block interacts with an external Power Management Unit. The PMC
>>  mostly controls the entry and exit of the system from different sleep
>>  modes. It provides power-gating controllers for SoC and CPU power-islands.
>> @@ -70,6 +72,11 @@ Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'
>>                       Defaults to 0. Valid values are described in section 12.5.2
>>                       "Pinmux Support" of the Tegra4 Technical Reference Manual.
>>  
>> +Optional nodes:
>> +- powergates : This node contains a hierarchy of power domain nodes, which
>> +	       should match the powergates on the Tegra SoC. See "Powergate
>> +	       Nodes" below.
>> +
>>  Example:
>>  
>>  / SoC dts including file
>> @@ -115,3 +122,76 @@ pmc@7000f400 {
>>  	};
>>  	...
>>  };
>> +
>> +
>> +== Powergate Nodes ==
>> +
>> +Each of the powergate nodes represent a power-domain on the Tegra SoC
>> +that can be power-gated by the Tegra PMC. The name of the powergate node
>> +should be one of the below. Note that not every powergate is applicable
>> +to all Tegra devices and the following list shows which powergates are
>> +applicable to which devices. Please refer to the Tegra TRM for more
>> +details on the various powergates.
>> +
>> + Name		Description			Devices Applicable
>> + 3d		3D Graphics			Tegra20/114/124/210
>> + 3d0		3D Graphics 0			Tegra30
>> + 3d1		3D Graphics 1			Tegra30
>> + aud		Audio				Tegra210
>> + dfd		Debug				Tegra210
>> + dis		Display A			Tegra114/124/210
>> + disb		Display B			Tegra114/124/210
>> + heg		2D Graphics			Tegra30/114/124/210
>> + iram		Internal RAM			Tegra124/210
>> + mpe		MPEG Encode			All
>> + nvdec		NVIDIA Video Decode Engine	Tegra210
>> + nvjpg		NVIDIA JPEG Engine		Tegra210
>> + pcie		PCIE				Tegra20/30/124/210
>> + sata		SATA				Tegra30/124/210
>> + sor		Display interfaces		Tegra124/210
>> + ve2		Video Encode Engine 2		Tegra210
>> + venc		Video Encode Engine		All
>> + vdec		Video Decode Engine		Tegra20/30/114/124
>> + vic		Video Imaging Compositor	Tegra124/210
>> + xusba		USB Partition A			Tegra114/124/210
>> + xusbb		USB Partition B 		Tegra114/124/210
>> + xusbc		USB Partition C			Tegra114/124/210
>> +
>> +Required properties:
>> +  - clocks: Must contain an entry for each clock required by the PMC for
>> +    controlling a power-gate. See ../clocks/clock-bindings.txt for details.
>> +  - resets: Must contain an entry for each reset required by the PMC for
>> +    controlling a power-gate. See ../reset/reset.txt for details.
>> +  - #power-domain-cells: Must be 0.
>> +
>> +Example:
>> +
>> +	pmc: pmc@0,7000e400 {
> 
> Just pmc@7000e400. We were erronously using commas for 64-bit addresses 
> briefly, but the correct usage is when there are distinct fields in reg 
> like PCI bus,dev,func.

Ok. Looks like we use this style quite widely across all Tegra 32-bit
and 64-bit devices for all peripheral devices.

Thierry, Stephen,

Do we need to correct this for existing devices?

> Otherwise:
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Thanks
Jon

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
       [not found]       ` <56DD515A.4020207-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2016-03-07 10:28         ` Thierry Reding
  0 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2016-03-07 10:28 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Rob Herring, Stephen Warren, Alexandre Courbot, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Rafael J. Wysocki,
	Kevin Hilman, Ulf Hansson, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Mar 07, 2016 at 10:00:58AM +0000, Jon Hunter wrote:
> 
> On 05/03/16 04:31, Rob Herring wrote:
> > On Fri, Mar 04, 2016 at 12:23:04PM +0000, Jon Hunter wrote:
> >> Add power-domain binding documentation for the NVIDIA PMC driver in
> >> order to support generic power-domains.
> >>
> >> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >> ---
> >>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80 ++++++++++++++++++++++
> >>  1 file changed, 80 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> >> index 53aa5496c5cf..7d52bbe99709 100644
> >> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> >> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> >> @@ -1,5 +1,7 @@
> >>  NVIDIA Tegra Power Management Controller (PMC)
> >>  
> >> +== Power Management Controller Node ==
> >> +
> >>  The PMC block interacts with an external Power Management Unit. The PMC
> >>  mostly controls the entry and exit of the system from different sleep
> >>  modes. It provides power-gating controllers for SoC and CPU power-islands.
> >> @@ -70,6 +72,11 @@ Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'
> >>                       Defaults to 0. Valid values are described in section 12.5.2
> >>                       "Pinmux Support" of the Tegra4 Technical Reference Manual.
> >>  
> >> +Optional nodes:
> >> +- powergates : This node contains a hierarchy of power domain nodes, which
> >> +	       should match the powergates on the Tegra SoC. See "Powergate
> >> +	       Nodes" below.
> >> +
> >>  Example:
> >>  
> >>  / SoC dts including file
> >> @@ -115,3 +122,76 @@ pmc@7000f400 {
> >>  	};
> >>  	...
> >>  };
> >> +
> >> +
> >> +== Powergate Nodes ==
> >> +
> >> +Each of the powergate nodes represent a power-domain on the Tegra SoC
> >> +that can be power-gated by the Tegra PMC. The name of the powergate node
> >> +should be one of the below. Note that not every powergate is applicable
> >> +to all Tegra devices and the following list shows which powergates are
> >> +applicable to which devices. Please refer to the Tegra TRM for more
> >> +details on the various powergates.
> >> +
> >> + Name		Description			Devices Applicable
> >> + 3d		3D Graphics			Tegra20/114/124/210
> >> + 3d0		3D Graphics 0			Tegra30
> >> + 3d1		3D Graphics 1			Tegra30
> >> + aud		Audio				Tegra210
> >> + dfd		Debug				Tegra210
> >> + dis		Display A			Tegra114/124/210
> >> + disb		Display B			Tegra114/124/210
> >> + heg		2D Graphics			Tegra30/114/124/210
> >> + iram		Internal RAM			Tegra124/210
> >> + mpe		MPEG Encode			All
> >> + nvdec		NVIDIA Video Decode Engine	Tegra210
> >> + nvjpg		NVIDIA JPEG Engine		Tegra210
> >> + pcie		PCIE				Tegra20/30/124/210
> >> + sata		SATA				Tegra30/124/210
> >> + sor		Display interfaces		Tegra124/210
> >> + ve2		Video Encode Engine 2		Tegra210
> >> + venc		Video Encode Engine		All
> >> + vdec		Video Decode Engine		Tegra20/30/114/124
> >> + vic		Video Imaging Compositor	Tegra124/210
> >> + xusba		USB Partition A			Tegra114/124/210
> >> + xusbb		USB Partition B 		Tegra114/124/210
> >> + xusbc		USB Partition C			Tegra114/124/210
> >> +
> >> +Required properties:
> >> +  - clocks: Must contain an entry for each clock required by the PMC for
> >> +    controlling a power-gate. See ../clocks/clock-bindings.txt for details.
> >> +  - resets: Must contain an entry for each reset required by the PMC for
> >> +    controlling a power-gate. See ../reset/reset.txt for details.
> >> +  - #power-domain-cells: Must be 0.
> >> +
> >> +Example:
> >> +
> >> +	pmc: pmc@0,7000e400 {
> > 
> > Just pmc@7000e400. We were erronously using commas for 64-bit addresses 
> > briefly, but the correct usage is when there are distinct fields in reg 
> > like PCI bus,dev,func.
> 
> Ok. Looks like we use this style quite widely across all Tegra 32-bit
> and 64-bit devices for all peripheral devices.
> 
> Thierry, Stephen,
> 
> Do we need to correct this for existing devices?

"Need" is perhaps a little strong. It's unfortunate that back at the
time the existing format was deemed best practice, and now we're told
to have been doing it wrong all along. There's also the issue that in
practice somebody might use the node name for some sort of lookup in
userspace or whatever, and they'd have every right to do so, because
the DT is an ABI after all.

Technically removing the 0, from the unit address would be breaking
that ABI, but in practice I'd hope that we wouldn't really break the
world for any users. If this gets us on the same page as everyone
else, I'm fine with taking this risk. I've already begun to convert
existing device trees.

Fingers crossed that best practices won't change again...

Thierry

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
  2016-03-07 10:00     ` Jon Hunter
       [not found]       ` <56DD515A.4020207-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2016-03-07 17:47       ` Stephen Warren
       [not found]         ` <56DDBEC6.7080102-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: Stephen Warren @ 2016-03-07 17:47 UTC (permalink / raw)
  To: Jon Hunter, Rob Herring, Thierry Reding
  Cc: Alexandre Courbot, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra, devicetree, linux-pm

On 03/07/2016 03:00 AM, Jon Hunter wrote:
>
> On 05/03/16 04:31, Rob Herring wrote:
>> On Fri, Mar 04, 2016 at 12:23:04PM +0000, Jon Hunter wrote:
>>> Add power-domain binding documentation for the NVIDIA PMC driver in
>>> order to support generic power-domains.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>   .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80 ++++++++++++++++++++++
>>>   1 file changed, 80 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>> index 53aa5496c5cf..7d52bbe99709 100644
>>> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>> @@ -1,5 +1,7 @@
>>>   NVIDIA Tegra Power Management Controller (PMC)
>>>
>>> +== Power Management Controller Node ==
>>> +
>>>   The PMC block interacts with an external Power Management Unit. The PMC
>>>   mostly controls the entry and exit of the system from different sleep
>>>   modes. It provides power-gating controllers for SoC and CPU power-islands.
>>> @@ -70,6 +72,11 @@ Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'
>>>                        Defaults to 0. Valid values are described in section 12.5.2
>>>                        "Pinmux Support" of the Tegra4 Technical Reference Manual.
>>>
>>> +Optional nodes:
>>> +- powergates : This node contains a hierarchy of power domain nodes, which
>>> +	       should match the powergates on the Tegra SoC. See "Powergate
>>> +	       Nodes" below.
>>> +
>>>   Example:
>>>
>>>   / SoC dts including file
>>> @@ -115,3 +122,76 @@ pmc@7000f400 {
>>>   	};
>>>   	...
>>>   };
>>> +
>>> +
>>> +== Powergate Nodes ==
>>> +
>>> +Each of the powergate nodes represent a power-domain on the Tegra SoC
>>> +that can be power-gated by the Tegra PMC. The name of the powergate node
>>> +should be one of the below. Note that not every powergate is applicable
>>> +to all Tegra devices and the following list shows which powergates are
>>> +applicable to which devices. Please refer to the Tegra TRM for more
>>> +details on the various powergates.
>>> +
>>> + Name		Description			Devices Applicable
>>> + 3d		3D Graphics			Tegra20/114/124/210
>>> + 3d0		3D Graphics 0			Tegra30
>>> + 3d1		3D Graphics 1			Tegra30
>>> + aud		Audio				Tegra210
>>> + dfd		Debug				Tegra210
>>> + dis		Display A			Tegra114/124/210
>>> + disb		Display B			Tegra114/124/210
>>> + heg		2D Graphics			Tegra30/114/124/210
>>> + iram		Internal RAM			Tegra124/210
>>> + mpe		MPEG Encode			All
>>> + nvdec		NVIDIA Video Decode Engine	Tegra210
>>> + nvjpg		NVIDIA JPEG Engine		Tegra210
>>> + pcie		PCIE				Tegra20/30/124/210
>>> + sata		SATA				Tegra30/124/210
>>> + sor		Display interfaces		Tegra124/210
>>> + ve2		Video Encode Engine 2		Tegra210
>>> + venc		Video Encode Engine		All
>>> + vdec		Video Decode Engine		Tegra20/30/114/124
>>> + vic		Video Imaging Compositor	Tegra124/210
>>> + xusba		USB Partition A			Tegra114/124/210
>>> + xusbb		USB Partition B 		Tegra114/124/210
>>> + xusbc		USB Partition C			Tegra114/124/210
>>> +
>>> +Required properties:
>>> +  - clocks: Must contain an entry for each clock required by the PMC for
>>> +    controlling a power-gate. See ../clocks/clock-bindings.txt for details.
>>> +  - resets: Must contain an entry for each reset required by the PMC for
>>> +    controlling a power-gate. See ../reset/reset.txt for details.
>>> +  - #power-domain-cells: Must be 0.
>>> +
>>> +Example:
>>> +
>>> +	pmc: pmc@0,7000e400 {
>>
>> Just pmc@7000e400. We were erronously using commas for 64-bit addresses
>> briefly, but the correct usage is when there are distinct fields in reg
>> like PCI bus,dev,func.
>
> Ok. Looks like we use this style quite widely across all Tegra 32-bit
> and 64-bit devices for all peripheral devices.
>
> Thierry, Stephen,
>
> Do we need to correct this for existing devices?

I don't think so; my understanding is that where reg has multiple 
address cells, they all get represented in the unit address. Otherwise, 
the node names couldn't be guaranteed unique.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
       [not found]         ` <56DDBEC6.7080102-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-03-07 19:33           ` Jon Hunter
  2016-03-07 20:29             ` Stephen Warren
  0 siblings, 1 reply; 15+ messages in thread
From: Jon Hunter @ 2016-03-07 19:33 UTC (permalink / raw)
  To: Stephen Warren, Rob Herring, Thierry Reding
  Cc: Alexandre Courbot, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA


On 07/03/16 17:47, Stephen Warren wrote:
> On 03/07/2016 03:00 AM, Jon Hunter wrote:
>>
>> On 05/03/16 04:31, Rob Herring wrote:
>>> On Fri, Mar 04, 2016 at 12:23:04PM +0000, Jon Hunter wrote:
>>>> Add power-domain binding documentation for the NVIDIA PMC driver in
>>>> order to support generic power-domains.
>>>>
>>>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>   .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80
>>>> ++++++++++++++++++++++
>>>>   1 file changed, 80 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>> index 53aa5496c5cf..7d52bbe99709 100644
>>>> ---
>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>> +++
>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>> @@ -1,5 +1,7 @@
>>>>   NVIDIA Tegra Power Management Controller (PMC)
>>>>
>>>> +== Power Management Controller Node ==
>>>> +
>>>>   The PMC block interacts with an external Power Management Unit.
>>>> The PMC
>>>>   mostly controls the entry and exit of the system from different sleep
>>>>   modes. It provides power-gating controllers for SoC and CPU
>>>> power-islands.
>>>> @@ -70,6 +72,11 @@ Optional properties for hardware-triggered
>>>> thermal reset (inside 'i2c-thermtrip'
>>>>                        Defaults to 0. Valid values are described in
>>>> section 12.5.2
>>>>                        "Pinmux Support" of the Tegra4 Technical
>>>> Reference Manual.
>>>>
>>>> +Optional nodes:
>>>> +- powergates : This node contains a hierarchy of power domain
>>>> nodes, which
>>>> +           should match the powergates on the Tegra SoC. See
>>>> "Powergate
>>>> +           Nodes" below.
>>>> +
>>>>   Example:
>>>>
>>>>   / SoC dts including file
>>>> @@ -115,3 +122,76 @@ pmc@7000f400 {
>>>>       };
>>>>       ...
>>>>   };
>>>> +
>>>> +
>>>> +== Powergate Nodes ==
>>>> +
>>>> +Each of the powergate nodes represent a power-domain on the Tegra SoC
>>>> +that can be power-gated by the Tegra PMC. The name of the powergate
>>>> node
>>>> +should be one of the below. Note that not every powergate is
>>>> applicable
>>>> +to all Tegra devices and the following list shows which powergates are
>>>> +applicable to which devices. Please refer to the Tegra TRM for more
>>>> +details on the various powergates.
>>>> +
>>>> + Name        Description            Devices Applicable
>>>> + 3d        3D Graphics            Tegra20/114/124/210
>>>> + 3d0        3D Graphics 0            Tegra30
>>>> + 3d1        3D Graphics 1            Tegra30
>>>> + aud        Audio                Tegra210
>>>> + dfd        Debug                Tegra210
>>>> + dis        Display A            Tegra114/124/210
>>>> + disb        Display B            Tegra114/124/210
>>>> + heg        2D Graphics            Tegra30/114/124/210
>>>> + iram        Internal RAM            Tegra124/210
>>>> + mpe        MPEG Encode            All
>>>> + nvdec        NVIDIA Video Decode Engine    Tegra210
>>>> + nvjpg        NVIDIA JPEG Engine        Tegra210
>>>> + pcie        PCIE                Tegra20/30/124/210
>>>> + sata        SATA                Tegra30/124/210
>>>> + sor        Display interfaces        Tegra124/210
>>>> + ve2        Video Encode Engine 2        Tegra210
>>>> + venc        Video Encode Engine        All
>>>> + vdec        Video Decode Engine        Tegra20/30/114/124
>>>> + vic        Video Imaging Compositor    Tegra124/210
>>>> + xusba        USB Partition A            Tegra114/124/210
>>>> + xusbb        USB Partition B         Tegra114/124/210
>>>> + xusbc        USB Partition C            Tegra114/124/210
>>>> +
>>>> +Required properties:
>>>> +  - clocks: Must contain an entry for each clock required by the
>>>> PMC for
>>>> +    controlling a power-gate. See ../clocks/clock-bindings.txt for
>>>> details.
>>>> +  - resets: Must contain an entry for each reset required by the
>>>> PMC for
>>>> +    controlling a power-gate. See ../reset/reset.txt for details.
>>>> +  - #power-domain-cells: Must be 0.
>>>> +
>>>> +Example:
>>>> +
>>>> +    pmc: pmc@0,7000e400 {
>>>
>>> Just pmc@7000e400. We were erronously using commas for 64-bit addresses
>>> briefly, but the correct usage is when there are distinct fields in reg
>>> like PCI bus,dev,func.
>>
>> Ok. Looks like we use this style quite widely across all Tegra 32-bit
>> and 64-bit devices for all peripheral devices.
>>
>> Thierry, Stephen,
>>
>> Do we need to correct this for existing devices?
> 
> I don't think so; my understanding is that where reg has multiple
> address cells, they all get represented in the unit address. Otherwise,
> the node names couldn't be guaranteed unique.

OK, but I am a bit confused as that does not sound exactly the same as
what Rob is saying?

Also, I see that starting with tegra124 we have #address-cells = <2> and
#size-cells = <2>, which I also don't understand either. Are the bus
addresses really 64-bit for tegra124?

Cheers
Jon

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC
  2016-03-07 19:33           ` Jon Hunter
@ 2016-03-07 20:29             ` Stephen Warren
  0 siblings, 0 replies; 15+ messages in thread
From: Stephen Warren @ 2016-03-07 20:29 UTC (permalink / raw)
  To: Jon Hunter, Rob Herring
  Cc: Thierry Reding, Alexandre Courbot, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rafael J. Wysocki, Kevin Hilman,
	Ulf Hansson, linux-tegra, devicetree, linux-pm

On 03/07/2016 12:33 PM, Jon Hunter wrote:
>
> On 07/03/16 17:47, Stephen Warren wrote:
>> On 03/07/2016 03:00 AM, Jon Hunter wrote:
>>>
>>> On 05/03/16 04:31, Rob Herring wrote:
>>>> On Fri, Mar 04, 2016 at 12:23:04PM +0000, Jon Hunter wrote:
>>>>> Add power-domain binding documentation for the NVIDIA PMC driver in
>>>>> order to support generic power-domains.
>>>>>
>>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>>> ---
>>>>>    .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 80
>>>>> ++++++++++++++++++++++
>>>>>    1 file changed, 80 insertions(+)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>> index 53aa5496c5cf..7d52bbe99709 100644
>>>>> ---
>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>> @@ -1,5 +1,7 @@
>>>>>    NVIDIA Tegra Power Management Controller (PMC)
>>>>>
>>>>> +== Power Management Controller Node ==
>>>>> +
>>>>>    The PMC block interacts with an external Power Management Unit.
>>>>> The PMC
>>>>>    mostly controls the entry and exit of the system from different sleep
>>>>>    modes. It provides power-gating controllers for SoC and CPU
>>>>> power-islands.
>>>>> @@ -70,6 +72,11 @@ Optional properties for hardware-triggered
>>>>> thermal reset (inside 'i2c-thermtrip'
>>>>>                         Defaults to 0. Valid values are described in
>>>>> section 12.5.2
>>>>>                         "Pinmux Support" of the Tegra4 Technical
>>>>> Reference Manual.
>>>>>
>>>>> +Optional nodes:
>>>>> +- powergates : This node contains a hierarchy of power domain
>>>>> nodes, which
>>>>> +           should match the powergates on the Tegra SoC. See
>>>>> "Powergate
>>>>> +           Nodes" below.
>>>>> +
>>>>>    Example:
>>>>>
>>>>>    / SoC dts including file
>>>>> @@ -115,3 +122,76 @@ pmc@7000f400 {
>>>>>        };
>>>>>        ...
>>>>>    };
>>>>> +
>>>>> +
>>>>> +== Powergate Nodes ==
>>>>> +
>>>>> +Each of the powergate nodes represent a power-domain on the Tegra SoC
>>>>> +that can be power-gated by the Tegra PMC. The name of the powergate
>>>>> node
>>>>> +should be one of the below. Note that not every powergate is
>>>>> applicable
>>>>> +to all Tegra devices and the following list shows which powergates are
>>>>> +applicable to which devices. Please refer to the Tegra TRM for more
>>>>> +details on the various powergates.
>>>>> +
>>>>> + Name        Description            Devices Applicable
>>>>> + 3d        3D Graphics            Tegra20/114/124/210
>>>>> + 3d0        3D Graphics 0            Tegra30
>>>>> + 3d1        3D Graphics 1            Tegra30
>>>>> + aud        Audio                Tegra210
>>>>> + dfd        Debug                Tegra210
>>>>> + dis        Display A            Tegra114/124/210
>>>>> + disb        Display B            Tegra114/124/210
>>>>> + heg        2D Graphics            Tegra30/114/124/210
>>>>> + iram        Internal RAM            Tegra124/210
>>>>> + mpe        MPEG Encode            All
>>>>> + nvdec        NVIDIA Video Decode Engine    Tegra210
>>>>> + nvjpg        NVIDIA JPEG Engine        Tegra210
>>>>> + pcie        PCIE                Tegra20/30/124/210
>>>>> + sata        SATA                Tegra30/124/210
>>>>> + sor        Display interfaces        Tegra124/210
>>>>> + ve2        Video Encode Engine 2        Tegra210
>>>>> + venc        Video Encode Engine        All
>>>>> + vdec        Video Decode Engine        Tegra20/30/114/124
>>>>> + vic        Video Imaging Compositor    Tegra124/210
>>>>> + xusba        USB Partition A            Tegra114/124/210
>>>>> + xusbb        USB Partition B         Tegra114/124/210
>>>>> + xusbc        USB Partition C            Tegra114/124/210
>>>>> +
>>>>> +Required properties:
>>>>> +  - clocks: Must contain an entry for each clock required by the
>>>>> PMC for
>>>>> +    controlling a power-gate. See ../clocks/clock-bindings.txt for
>>>>> details.
>>>>> +  - resets: Must contain an entry for each reset required by the
>>>>> PMC for
>>>>> +    controlling a power-gate. See ../reset/reset.txt for details.
>>>>> +  - #power-domain-cells: Must be 0.
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +    pmc: pmc@0,7000e400 {
>>>>
>>>> Just pmc@7000e400. We were erronously using commas for 64-bit addresses
>>>> briefly, but the correct usage is when there are distinct fields in reg
>>>> like PCI bus,dev,func.
>>>
>>> Ok. Looks like we use this style quite widely across all Tegra 32-bit
>>> and 64-bit devices for all peripheral devices.
>>>
>>> Thierry, Stephen,
>>>
>>> Do we need to correct this for existing devices?
>>
>> I don't think so; my understanding is that where reg has multiple
>> address cells, they all get represented in the unit address. Otherwise,
>> the node names couldn't be guaranteed unique.
>
> OK, but I am a bit confused as that does not sound exactly the same as
> what Rob is saying?

Consider a SoC with two I2C ports at address 1_00000000 and 2_00000000 
physical. The top 32-bits (2nd cell) of the address must be represented 
in the unit address, or the nodes won't have a unique name.

Perhaps Rob is simply saying that multi-cell values should be treated as 
a single large integer, rather than comma-separating each cell value? I 
perhaps mistakenly took his comment to mean that only a single address 
cell should be used in the unit address.

> Also, I see that starting with tegra124 we have #address-cells = <2> and
> #size-cells = <2>, which I also don't understand either. Are the bus
> addresses really 64-bit for tegra124?

T124 has a >32-bit bus since it supports >2GB RAM (via LPAE) (although 
not a full 64-bit bus I would imagine). That alone means we need two 
cells in DT to represent large RAM layouts.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support
  2016-03-04 12:23   ` [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support Jon Hunter
@ 2016-03-08 21:28     ` Ulf Hansson
       [not found]       ` <CAPDyKFooBRk4Od=pgEnK8Uvq6Y+5O9uK5Ej9Z4gJn1W=imPvdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Ulf Hansson @ 2016-03-08 21:28 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman, linux-tegra, devicetree,
	linux-pm

On 4 March 2016 at 13:23, Jon Hunter <jonathanh@nvidia.com> wrote:
> Adds generic PM support to the PMC driver where the PM domains are
> populated from device-tree and the PM domain consumer devices are
> bound to their relevant PM domains via device-tree as well.
>
> Update the tegra_powergate_sequence_power_up() API so that internally
> it calls the same tegra_powergate_xxx functions that are used by the
> tegra generic power domain code for consistency.
>
> To ensure that the Tegra power domains (a.k.a powergates) cannot be
> controlled via both the legacy tegra_powergate_xxx functions as well
> as the generic PM domain framework, add a bit map for available
> powergates that can be controlled via the legacy powergate functions.
>
> Move the majority of the tegra_powergate_remove_clamping() function
> to a sub-function, so that this can be used by both the legacy and
> generic power domain code.
>
> Currently, the power domains are not removed once added because this
> is not yet supported by the PM domains framework. An error message
> will be displayed if we are unable to instantiate a power domain.
>
> This is based upon work by Thierry Reding <treding@nvidia.com>
> and Vince Hsu <vinceh@nvidia.com>.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 485 ++++++++++++++++++++++++++++++++++++++++++------
>  include/soc/tegra/pmc.h |   1 +
>  2 files changed, 425 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 08966c26d65c..bb173456bbff 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -31,10 +31,13 @@
>  #include <linux/iopoll.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/reboot.h>
>  #include <linux/reset.h>
>  #include <linux/seq_file.h>
> +#include <linux/slab.h>
>  #include <linux/spinlock.h>
>
>  #include <soc/tegra/common.h>
> @@ -102,6 +105,16 @@
>
>  #define GPU_RG_CNTRL                   0x2d4
>
> +struct tegra_powergate {
> +       struct generic_pm_domain genpd;
> +       struct tegra_pmc *pmc;
> +       unsigned int id;
> +       struct clk **clks;
> +       unsigned int num_clks;
> +       struct reset_control **resets;
> +       unsigned int num_resets;
> +};
> +
>  struct tegra_pmc_soc {
>         unsigned int num_powergates;
>         const char *const *powergates;
> @@ -132,6 +145,7 @@ struct tegra_pmc_soc {
>   * @cpu_pwr_good_en: CPU power good signal is enabled
>   * @lp0_vec_phys: physical base address of the LP0 warm boot code
>   * @lp0_vec_size: size of the LP0 warm boot code
> + * @powergates_available: Bitmap of available power gates
>   * @powergates_lock: mutex for power gate register access
>   */
>  struct tegra_pmc {
> @@ -156,6 +170,7 @@ struct tegra_pmc {
>         bool cpu_pwr_good_en;
>         u32 lp0_vec_phys;
>         u32 lp0_vec_size;
> +       DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
>
>         struct mutex powergates_lock;
>  };
> @@ -165,6 +180,12 @@ static struct tegra_pmc *pmc = &(struct tegra_pmc) {
>         .suspend_mode = TEGRA_SUSPEND_NONE,
>  };
>
> +static inline struct tegra_powergate *
> +to_powergate(struct generic_pm_domain *domain)
> +{
> +       return container_of(domain, struct tegra_powergate, genpd);
> +}
> +
>  static u32 tegra_pmc_readl(unsigned long offset)
>  {
>         return readl(pmc->base + offset);
> @@ -188,6 +209,31 @@ static inline bool tegra_powergate_is_valid(int id)
>         return (pmc->soc && pmc->soc->powergates[id]);
>  }
>
> +static inline bool tegra_powergate_is_available(int id)
> +{
> +       return test_bit(id, pmc->powergates_available);
> +}
> +
> +static int tegra_powergate_lookup(struct tegra_pmc *pmc, const char *name)
> +{
> +       unsigned int i;
> +
> +       if (!pmc || !pmc->soc || !name)
> +               return -EINVAL;
> +
> +       for (i = 0; i < pmc->soc->num_powergates; i++) {
> +               if (!tegra_powergate_is_valid(i))
> +                       continue;
> +
> +               if (!strcmp(name, pmc->soc->powergates[i]))
> +                       return i;

Instead of having this name based lookup, why not provide the id using
DT instead?

In other words use of_genpd_add_provider_onecell() when adding the OF
genpd provider. In that way I think you will get a bit simplier code
dealing with the error path when initialzing the genpds, won't you?

Anyway, I guess it's more a matter of taste, so feel free to keep as is.

[...]

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support
       [not found]       ` <CAPDyKFooBRk4Od=pgEnK8Uvq6Y+5O9uK5Ej9Z4gJn1W=imPvdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-03-09 10:22         ` Jon Hunter
  0 siblings, 0 replies; 15+ messages in thread
From: Jon Hunter @ 2016-03-09 10:22 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rafael J. Wysocki, Kevin Hilman,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA

Hi Uffe,

On 08/03/16 21:28, Ulf Hansson wrote:
> On 4 March 2016 at 13:23, Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> Adds generic PM support to the PMC driver where the PM domains are
>> populated from device-tree and the PM domain consumer devices are
>> bound to their relevant PM domains via device-tree as well.
>>
>> Update the tegra_powergate_sequence_power_up() API so that internally
>> it calls the same tegra_powergate_xxx functions that are used by the
>> tegra generic power domain code for consistency.
>>
>> To ensure that the Tegra power domains (a.k.a powergates) cannot be
>> controlled via both the legacy tegra_powergate_xxx functions as well
>> as the generic PM domain framework, add a bit map for available
>> powergates that can be controlled via the legacy powergate functions.
>>
>> Move the majority of the tegra_powergate_remove_clamping() function
>> to a sub-function, so that this can be used by both the legacy and
>> generic power domain code.
>>
>> Currently, the power domains are not removed once added because this
>> is not yet supported by the PM domains framework. An error message
>> will be displayed if we are unable to instantiate a power domain.
>>
>> This is based upon work by Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> and Vince Hsu <vinceh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>.
>>
>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/soc/tegra/pmc.c | 485 ++++++++++++++++++++++++++++++++++++++++++------
>>  include/soc/tegra/pmc.h |   1 +
>>  2 files changed, 425 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 08966c26d65c..bb173456bbff 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -31,10 +31,13 @@
>>  #include <linux/iopoll.h>
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>>  #include <linux/reboot.h>
>>  #include <linux/reset.h>
>>  #include <linux/seq_file.h>
>> +#include <linux/slab.h>
>>  #include <linux/spinlock.h>
>>
>>  #include <soc/tegra/common.h>
>> @@ -102,6 +105,16 @@
>>
>>  #define GPU_RG_CNTRL                   0x2d4
>>
>> +struct tegra_powergate {
>> +       struct generic_pm_domain genpd;
>> +       struct tegra_pmc *pmc;
>> +       unsigned int id;
>> +       struct clk **clks;
>> +       unsigned int num_clks;
>> +       struct reset_control **resets;
>> +       unsigned int num_resets;
>> +};
>> +
>>  struct tegra_pmc_soc {
>>         unsigned int num_powergates;
>>         const char *const *powergates;
>> @@ -132,6 +145,7 @@ struct tegra_pmc_soc {
>>   * @cpu_pwr_good_en: CPU power good signal is enabled
>>   * @lp0_vec_phys: physical base address of the LP0 warm boot code
>>   * @lp0_vec_size: size of the LP0 warm boot code
>> + * @powergates_available: Bitmap of available power gates
>>   * @powergates_lock: mutex for power gate register access
>>   */
>>  struct tegra_pmc {
>> @@ -156,6 +170,7 @@ struct tegra_pmc {
>>         bool cpu_pwr_good_en;
>>         u32 lp0_vec_phys;
>>         u32 lp0_vec_size;
>> +       DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
>>
>>         struct mutex powergates_lock;
>>  };
>> @@ -165,6 +180,12 @@ static struct tegra_pmc *pmc = &(struct tegra_pmc) {
>>         .suspend_mode = TEGRA_SUSPEND_NONE,
>>  };
>>
>> +static inline struct tegra_powergate *
>> +to_powergate(struct generic_pm_domain *domain)
>> +{
>> +       return container_of(domain, struct tegra_powergate, genpd);
>> +}
>> +
>>  static u32 tegra_pmc_readl(unsigned long offset)
>>  {
>>         return readl(pmc->base + offset);
>> @@ -188,6 +209,31 @@ static inline bool tegra_powergate_is_valid(int id)
>>         return (pmc->soc && pmc->soc->powergates[id]);
>>  }
>>
>> +static inline bool tegra_powergate_is_available(int id)
>> +{
>> +       return test_bit(id, pmc->powergates_available);
>> +}
>> +
>> +static int tegra_powergate_lookup(struct tegra_pmc *pmc, const char *name)
>> +{
>> +       unsigned int i;
>> +
>> +       if (!pmc || !pmc->soc || !name)
>> +               return -EINVAL;
>> +
>> +       for (i = 0; i < pmc->soc->num_powergates; i++) {
>> +               if (!tegra_powergate_is_valid(i))
>> +                       continue;
>> +
>> +               if (!strcmp(name, pmc->soc->powergates[i]))
>> +                       return i;
> 
> Instead of having this name based lookup, why not provide the id using
> DT instead?
> 
> In other words use of_genpd_add_provider_onecell() when adding the OF
> genpd provider. In that way I think you will get a bit simplier code
> dealing with the error path when initialzing the genpds, won't you?

The above is simply used when creating the PM domains and not when
calling the xlate to lookup the PM domain (which I believe you are
referring to).

Originally, the DT node for the PM domain included an ID for the PM
domain, but when discussing with Thierry we thought there was no need to
have both a name and ID. So now the DT node for the PM domain look like ...

	powergates {
		pd_audio: aud {
			clocks = <&tegra_car TEGRA210_CLK_APE>,
			         <&tegra_car TEGRA210_CLK_APB2APE>;
			resets = <&tegra_car 198>;
			#power-domain-cells = <0>;
		};
	};

And the client binding ...

	adma: adma@702e2000 {
		...
		power-domains = <&pd_audio>;
		...
	};

> Anyway, I guess it's more a matter of taste, so feel free to keep as is.

I prefer not to use of_genpd_add_provider_onecell() for tegra as this
appears to be used for devices with a static list of PM domains. Yes,
the Tegra PMC driver does have a static list of PM domain names (per
pmc->soc->powergates[i]), however, I prefer that all the clock and reset
information resides in the DT blob.

Cheers
Jon

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-03-09 10:22 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-04 12:23 [PATCH V7 0/4] Add generic PM domain support for Tegra Jon Hunter
2016-03-04 12:23 ` [PATCH V7 2/4] Documentation: DT: bindings: Add power domain info for NVIDIA PMC Jon Hunter
2016-03-05  4:31   ` Rob Herring
2016-03-07 10:00     ` Jon Hunter
     [not found]       ` <56DD515A.4020207-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-03-07 10:28         ` Thierry Reding
2016-03-07 17:47       ` Stephen Warren
     [not found]         ` <56DDBEC6.7080102-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-03-07 19:33           ` Jon Hunter
2016-03-07 20:29             ` Stephen Warren
     [not found] ` <1457094186-15786-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-03-04 12:23   ` [PATCH V7 1/4] ARM64: tegra: Remove unused #power-domain-cells property Jon Hunter
2016-03-04 14:13     ` Thierry Reding
2016-03-04 14:19       ` Jon Hunter
2016-03-04 12:23   ` [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support Jon Hunter
2016-03-08 21:28     ` Ulf Hansson
     [not found]       ` <CAPDyKFooBRk4Od=pgEnK8Uvq6Y+5O9uK5Ej9Z4gJn1W=imPvdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-09 10:22         ` Jon Hunter
2016-03-04 12:23   ` [PATCH V7 4/4] ARM64: tegra: select PM_GENERIC_DOMAINS Jon Hunter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.