2015-12-04 15:00:01

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 00/16] Add generic PM domain support for Tegra

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

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

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

Series Summary:
Patch 1: Adds the new function of_reset_control_get_by_index().
Patch 2-9: PMC clean-up and fixes
Patch 10: Adds function to remove a generic PM domain
Patch 11: Updates DT documentation for PMC w.r.t Tegra210
Patch 12: Adds DT documentation for tegra generic PM domains
Patch 13: Adds PMC generic DM domains support
Patch 14: Adds audio clock for Tegra210 audio power-domain
Patch 15: Adds DT bindings for Tegra210 audio power-domain
Patch 16: Enable generic PM domains for tegra64

[0] http://comments.gmane.org/gmane.linux.ports.tegra/22944

Jon Hunter (15):
soc: tegra: pmc: Add missing structure members to kernel-doc
soc: tegra: pmc: Fix sparse warning for tegra_pmc_init_tsense_reset
soc: tegra: pmc: Remove debugfs entry on probe failure
soc: tegra: pmc: Avoid extra remapping of PMC registers
soc: tegra: pmc: Wait for powergate state to change
soc: tegra: pmc: Remove non-existing power partitions for T210
soc: tegra: pmc: Fix checking of valid partitions
soc: tegra: pmc: Ensure partitions can be toggled on/off by PMC
PM / Domains: Add function to remove a pm-domain
Documentation: DT: bindings: Update NVIDIA PMC for Tegra210
Documentation: DT: bindings: Add power domain info for NVIDIA PMC
soc: tegra: pmc: Add generic PM domain support
clk: tegra210: Add the APB2APE audio clock
ARM64: tegra: Add audio PM domain device node for Tegra210
ARM64: tegra: select PM_GENERIC_DOMAINS

Vince Hsu (1):
reset: add of_reset_control_get_by_index()

.../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 66 ++-
arch/arm/boot/dts/tegra124.dtsi | 1 +
arch/arm/mach-tegra/platsmp.c | 16 +-
arch/arm64/Kconfig.platforms | 2 +
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 11 +-
drivers/base/power/domain.c | 26 +
drivers/clk/tegra/clk-id.h | 1 +
drivers/clk/tegra/clk-tegra-periph.c | 1 +
drivers/clk/tegra/clk-tegra210.c | 1 +
drivers/reset/core.c | 40 +-
drivers/soc/tegra/pmc.c | 586 +++++++++++++++++++--
include/dt-bindings/clock/tegra210-car.h | 2 +-
include/dt-bindings/power/tegra-powergate.h | 35 ++
include/linux/pm_domain.h | 5 +
include/linux/reset.h | 9 +
include/soc/tegra/pmc.h | 38 +-
16 files changed, 720 insertions(+), 120 deletions(-)
create mode 100644 include/dt-bindings/power/tegra-powergate.h

--
2.1.4


2015-12-04 15:00:08

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 01/16] reset: add of_reset_control_get_by_index()

From: Vince Hsu <[email protected]>

Add of_reset_control_get_by_index() to allow the drivers to get reset
device without knowing its name.

Signed-off-by: Vince Hsu <[email protected]>
[[email protected]: Updated stub function to return -ENOTSUPP instead
of -ENOSYS which should only be used for system calls.]
Signed-off-by: Jon Hunter <[email protected]>

---
v3: addressed comments from Philipp
v2: minor changes according to Alex's comments
---
drivers/reset/core.c | 40 +++++++++++++++++++++++++++++-----------
include/linux/reset.h | 9 +++++++++
2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 7955e00d04d4..81ae17d15480 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -141,27 +141,24 @@ int reset_control_status(struct reset_control *rstc)
EXPORT_SYMBOL_GPL(reset_control_status);

/**
- * of_reset_control_get - Lookup and obtain a reference to a reset controller.
+ * of_reset_control_get_by_index - Lookup and obtain a reference to a reset
+ * controller by index.
* @node: device to be reset by the controller
- * @id: reset line name
- *
- * Returns a struct reset_control or IS_ERR() condition containing errno.
+ * @index: index of the reset controller
*
- * Use of id names is optional.
+ * This is to be used to perform a list of resets for a device or power domain
+ * in whatever order. Returns a struct reset_control or IS_ERR() condition
+ * containing errno.
*/
-struct reset_control *of_reset_control_get(struct device_node *node,
- const char *id)
+struct reset_control *of_reset_control_get_by_index(struct device_node *node,
+ int index)
{
struct reset_control *rstc = ERR_PTR(-EPROBE_DEFER);
struct reset_controller_dev *r, *rcdev;
struct of_phandle_args args;
- int index = 0;
int rstc_id;
int ret;

- if (id)
- index = of_property_match_string(node,
- "reset-names", id);
ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
index, &args);
if (ret)
@@ -202,6 +199,27 @@ struct reset_control *of_reset_control_get(struct device_node *node,

return rstc;
}
+EXPORT_SYMBOL_GPL(of_reset_control_get_by_index);
+
+/**
+ * of_reset_control_get - Lookup and obtain a reference to a reset controller.
+ * @node: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Returns a struct reset_control or IS_ERR() condition containing errno.
+ *
+ * Use of id names is optional.
+ */
+struct reset_control *of_reset_control_get(struct device_node *node,
+ const char *id)
+{
+ int index = 0;
+
+ if (id)
+ index = of_property_match_string(node,
+ "reset-names", id);
+ return of_reset_control_get_by_index(node, index);
+}
EXPORT_SYMBOL_GPL(of_reset_control_get);

/**
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 7f65f9cff951..6db74ad3dec7 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -38,6 +38,9 @@ static inline struct reset_control *devm_reset_control_get_optional(
struct reset_control *of_reset_control_get(struct device_node *node,
const char *id);

+struct reset_control *of_reset_control_get_by_index(
+ struct device_node *node, int index);
+
#else

static inline int reset_control_reset(struct reset_control *rstc)
@@ -106,6 +109,12 @@ static inline struct reset_control *of_reset_control_get(
return ERR_PTR(-ENOSYS);
}

+static inline struct reset_control *of_reset_control_get_by_index(
+ struct device_node *node, int index)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
#endif /* CONFIG_RESET_CONTROLLER */

#endif
--
2.1.4

2015-12-04 15:04:51

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 02/16] soc: tegra: pmc: Add missing structure members to kernel-doc

Some members of the tegra_pmc structure are missing from the kernel-doc
comment for this structure. Add the missing members.

Signed-off-by: Jon Hunter <[email protected]>
---
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 bc34cf7482fb..99ca91f09929 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -113,8 +113,10 @@ struct tegra_pmc_soc {

/**
* struct tegra_pmc - NVIDIA Tegra PMC
+ * @dev: pointer to PMC device structure
* @base: pointer to I/O remapped register region
* @clk: pointer to pclk clock
+ * @soc: pointer to SoC data structure
* @rate: currently configured rate of pclk
* @suspend_mode: lowest suspend mode available
* @cpu_good_time: CPU power good time (in microseconds)
--
2.1.4

2015-12-04 15:00:15

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 03/16] soc: tegra: pmc: Fix sparse warning for tegra_pmc_init_tsense_reset

Sparse reports the following warning for tegra_pmc_init_tsense_reset:

drivers/soc/tegra/pmc.c:741:6: warning: symbol
'tegra_pmc_init_tsense_reset' was not declared. Should it be static?

This function is only used internally by the PMC driver and so fix this
by making it static.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 99ca91f09929..5b6125ecd69a 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -729,7 +729,7 @@ static void tegra_pmc_init(struct tegra_pmc *pmc)
tegra_pmc_writel(value, PMC_CNTRL);
}

-void tegra_pmc_init_tsense_reset(struct tegra_pmc *pmc)
+static void tegra_pmc_init_tsense_reset(struct tegra_pmc *pmc)
{
static const char disabled[] = "emergency thermal reset disabled";
u32 pmu_addr, ctrl_id, reg_addr, reg_data, pinmux;
--
2.1.4

2015-12-04 15:00:24

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 04/16] soc: tegra: pmc: Remove debugfs entry on probe failure

The debugfs entry for the PMC device will not be removed if the probe of
the device fails on registering the restart handler and so fix this so
that it is removed.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 5b6125ecd69a..e60fc2d33c94 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -117,6 +117,7 @@ struct tegra_pmc_soc {
* @base: pointer to I/O remapped register region
* @clk: pointer to pclk clock
* @soc: pointer to SoC data structure
+ * @debugfs: pointer to debugfs entry
* @rate: currently configured rate of pclk
* @suspend_mode: lowest suspend mode available
* @cpu_good_time: CPU power good time (in microseconds)
@@ -136,6 +137,7 @@ struct tegra_pmc {
struct device *dev;
void __iomem *base;
struct clk *clk;
+ struct dentry *debugfs;

const struct tegra_pmc_soc *soc;

@@ -447,11 +449,9 @@ static const struct file_operations powergate_fops = {

static int tegra_powergate_debugfs_init(void)
{
- struct dentry *d;
-
- d = debugfs_create_file("powergate", S_IRUGO, NULL, NULL,
- &powergate_fops);
- if (!d)
+ pmc->debugfs = debugfs_create_file("powergate", S_IRUGO, NULL, NULL,
+ &powergate_fops);
+ if (!pmc->debugfs)
return -ENOMEM;

return 0;
@@ -844,6 +844,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)

err = register_restart_handler(&tegra_pmc_restart_handler);
if (err) {
+ debugfs_remove(pmc->debugfs);
dev_err(&pdev->dev, "unable to register restart handler, %d\n",
err);
return err;
--
2.1.4

2015-12-04 15:04:14

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 05/16] soc: tegra: pmc: Avoid extra remapping of PMC registers

During early initialisation, the PMC registers are mapped and the PMC SoC
data is populated in the PMC data structure. This allows other drivers
access the PMC register space, via the public tegra PMC APIs, prior to
probing the PMC device.

When the PMC device is probed, the PMC registers are mapped again and if
successful the initial mapping is freed. If the probing of the PMC device
fails after the registers are remapped, then the registers will be
unmapped and hence the pointer to the PMC registers will be invalid. This
could lead to a potential crash, because once the PMC SoC data pointer is
populated, the driver assumes that the PMC register mapping is also valid
and a user calling any of the public tegra PMC APIs could trigger an
exception because these APIs don't check that the mapping is still valid.

Rather than adding a test to see if the PMC register mapping is valid,
fix this by removing the second mapping of the PMC registers and reserve
the memory region for the PMC registers during early initialisation where
the initial mapping is created. During the probing of the PMC simply check
that the PMC registers have been mapped.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e60fc2d33c94..fdd1a8d0940f 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -807,22 +807,17 @@ out:

static int tegra_pmc_probe(struct platform_device *pdev)
{
- void __iomem *base = pmc->base;
- struct resource *res;
int err;

+ if (!pmc->base) {
+ dev_err(&pdev->dev, "base address is not configured\n");
+ return -ENXIO;
+ }
+
err = tegra_pmc_parse_dt(pmc, pdev->dev.of_node);
if (err < 0)
return err;

- /* take over the memory region from the early initialization */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pmc->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(pmc->base))
- return PTR_ERR(pmc->base);
-
- iounmap(base);
-
pmc->clk = devm_clk_get(&pdev->dev, "pclk");
if (IS_ERR(pmc->clk)) {
err = PTR_ERR(pmc->clk);
@@ -1126,8 +1121,12 @@ static int __init tegra_pmc_early_init(void)
pmc->soc = match->data;
}

+ if (!request_mem_region(regs.start, resource_size(&regs), regs.name))
+ return -ENOMEM;
+
pmc->base = ioremap_nocache(regs.start, resource_size(&regs));
if (!pmc->base) {
+ release_mem_region(regs.start, resource_size(&regs));
pr_err("failed to map PMC registers\n");
return -ENXIO;
}
--
2.1.4

2015-12-04 15:00:32

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 06/16] soc: tegra: pmc: Wait for powergate state to change

Currently, the function tegra_powergate_set() simply sets the desired
powergate state but does not wait for the state to change. In most cases
we should wait for the state to change before proceeding. Currently, there
is a case for tegra114 and tegra124 devices where we do not wait when
starting the secondary CPU as this is not necessary. However, this is only
done at boot time and so waiting here will only have a small impact on boot
time. Therefore, update tegra_powergate_set() to wait when setting the
powergate.

By adding this feature, we can also eliminate the polling loop from
tegra30_boot_secondary().

A macro has also been adding for checking the status of the powergate and
so update the tegra_powergate_is_powered() to use this macro as well.

Signed-off-by: Jon Hunter <[email protected]>
---
arch/arm/mach-tegra/platsmp.c | 16 +++-------------
drivers/soc/tegra/pmc.c | 21 +++++++++++++++------
2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
index b45086666648..40cb761e7c95 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/arch/arm/mach-tegra/platsmp.c
@@ -108,19 +108,9 @@ static int tegra30_boot_secondary(unsigned int cpu, struct task_struct *idle)
* be un-gated by un-toggling the power gate register
* manually.
*/
- if (!tegra_pmc_cpu_is_powered(cpu)) {
- ret = tegra_pmc_cpu_power_on(cpu);
- if (ret)
- return ret;
-
- /* Wait for the power to come up. */
- timeout = jiffies + msecs_to_jiffies(100);
- while (!tegra_pmc_cpu_is_powered(cpu)) {
- if (time_after(jiffies, timeout))
- return -ETIMEDOUT;
- udelay(10);
- }
- }
+ ret = tegra_pmc_cpu_power_on(cpu);
+ if (ret)
+ return ret;

remove_clamps:
/* CPU partition is powered. Enable the CPU clock. */
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index fdd1a8d0940f..f94d970089ce 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -28,6 +28,7 @@
#include <linux/export.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
@@ -101,6 +102,8 @@

#define GPU_RG_CNTRL 0x2d4

+#define PMC_PWRGATE_STATE(status, id) ((status & BIT(id)) != 0)
+
struct tegra_pmc_soc {
unsigned int num_powergates;
const char *const *powergates;
@@ -181,22 +184,27 @@ static void tegra_pmc_writel(u32 value, unsigned long offset)
*/
static int tegra_powergate_set(int id, bool new_state)
{
- bool status;
+ u32 status;
+ int err = 0;

mutex_lock(&pmc->powergates_lock);

- status = tegra_pmc_readl(PWRGATE_STATUS) & (1 << id);
+ status = tegra_pmc_readl(PWRGATE_STATUS);

- if (status == new_state) {
+ if (PMC_PWRGATE_STATE(status, id) == new_state) {
mutex_unlock(&pmc->powergates_lock);
return 0;
}

tegra_pmc_writel(PWRGATE_TOGGLE_START | id, PWRGATE_TOGGLE);

+ err = readx_poll_timeout(tegra_pmc_readl, PWRGATE_STATUS, status,
+ PMC_PWRGATE_STATE(status, id) == new_state,
+ 10, 100000);
+
mutex_unlock(&pmc->powergates_lock);

- return 0;
+ return err;
}

/**
@@ -235,8 +243,9 @@ int tegra_powergate_is_powered(int id)
if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
return -EINVAL;

- status = tegra_pmc_readl(PWRGATE_STATUS) & (1 << id);
- return !!status;
+ status = tegra_pmc_readl(PWRGATE_STATUS);
+
+ return PMC_PWRGATE_STATE(status, id);
}

/**
--
2.1.4

2015-12-04 15:00:38

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 07/16] soc: tegra: pmc: Remove non-existing power partitions for T210

The power partitions L2, HEG, CELP and C1NC do not exist for T210 but were
incorrectly documented in the TRM. These will be removed from the TRM and
so also remove their definitions for T210.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index f94d970089ce..28d3106d3add 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1013,17 +1013,13 @@ static const char * const tegra210_powergates[] = {
[TEGRA_POWERGATE_3D] = "3d",
[TEGRA_POWERGATE_VENC] = "venc",
[TEGRA_POWERGATE_PCIE] = "pcie",
- [TEGRA_POWERGATE_L2] = "l2",
[TEGRA_POWERGATE_MPE] = "mpe",
- [TEGRA_POWERGATE_HEG] = "heg",
[TEGRA_POWERGATE_SATA] = "sata",
[TEGRA_POWERGATE_CPU1] = "cpu1",
[TEGRA_POWERGATE_CPU2] = "cpu2",
[TEGRA_POWERGATE_CPU3] = "cpu3",
- [TEGRA_POWERGATE_CELP] = "celp",
[TEGRA_POWERGATE_CPU0] = "cpu0",
[TEGRA_POWERGATE_C0NC] = "c0nc",
- [TEGRA_POWERGATE_C1NC] = "c1nc",
[TEGRA_POWERGATE_SOR] = "sor",
[TEGRA_POWERGATE_DIS] = "dis",
[TEGRA_POWERGATE_DISB] = "disb",
--
2.1.4

2015-12-04 15:03:32

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 08/16] soc: tegra: pmc: Fix checking of valid partitions

The tegra power partitions are referenced by a numerical ID which are
the same values programmed into the PMC registers for controlling the
partition. For a given device, the valid partition IDs may not be
contiguous and so simply checking that an ID is not greater than the
maximum ID supported may not mean it is valid. Fix this by computing
a bit-mask of the valid partition IDs for a device and add a macro that
will test if the partition is valid based upon this mask.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 28d3106d3add..0967bba13947 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -103,6 +103,7 @@
#define GPU_RG_CNTRL 0x2d4

#define PMC_PWRGATE_STATE(status, id) ((status & BIT(id)) != 0)
+#define PMC_PWRGATE_IS_VALID(id) (pmc->powergates_mask & BIT(id))

struct tegra_pmc_soc {
unsigned int num_powergates;
@@ -134,6 +135,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_mask: Bit mask of valid power gates
* @powergates_lock: mutex for power gate register access
*/
struct tegra_pmc {
@@ -158,6 +160,7 @@ struct tegra_pmc {
bool cpu_pwr_good_en;
u32 lp0_vec_phys;
u32 lp0_vec_size;
+ u32 powergates_mask;

struct mutex powergates_lock;
};
@@ -213,7 +216,7 @@ static int tegra_powergate_set(int id, bool new_state)
*/
int tegra_powergate_power_on(int id)
{
- if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
+ if (!PMC_PWRGATE_IS_VALID(id))
return -EINVAL;

return tegra_powergate_set(id, true);
@@ -225,7 +228,7 @@ int tegra_powergate_power_on(int id)
*/
int tegra_powergate_power_off(int id)
{
- if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
+ if (!PMC_PWRGATE_IS_VALID(id))
return -EINVAL;

return tegra_powergate_set(id, false);
@@ -240,7 +243,7 @@ int tegra_powergate_is_powered(int id)
{
u32 status;

- if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
+ if (!PMC_PWRGATE_IS_VALID(id))
return -EINVAL;

status = tegra_pmc_readl(PWRGATE_STATUS);
@@ -256,7 +259,7 @@ int tegra_powergate_remove_clamping(int id)
{
u32 mask;

- if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
+ if (!PMC_PWRGATE_IS_VALID(id))
return -EINVAL;

/*
@@ -1084,7 +1087,7 @@ static int __init tegra_pmc_early_init(void)
struct device_node *np;
struct resource regs;
bool invert;
- u32 value;
+ u32 value, i;

np = of_find_matching_node_and_match(NULL, tegra_pmc_match, &match);
if (!np) {
@@ -1136,6 +1139,11 @@ static int __init tegra_pmc_early_init(void)
return -ENXIO;
}

+ /* Create a bit-mask of the valid partitions */
+ for (i = 0; i < pmc->soc->num_powergates; i++)
+ if (pmc->soc->powergates[i])
+ pmc->powergates_mask |= BIT(i);
+
mutex_init(&pmc->powergates_lock);

/*
--
2.1.4

2015-12-04 15:00:46

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 09/16] soc: tegra: pmc: Ensure partitions can be toggled on/off by PMC

For Tegra124 and Tegra210, the GPU partition cannot be toggled on and off
via the APBDEV_PMC_PWRGATE_TOGGLE_0 register. For these devices, the
partition is simply powered up and down via an external regulator.
Describe in the PMC SoC data in which devices the GPU partition can be
controlled via the APBDEV_PMC_PWRGATE_TOGGLE_0 register and ensure that
no one can incorrectly try to toggle the GPU partition via the
APBDEV_PMC_PWRGATE_TOGGLE_0 register.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 0967bba13947..b412098b8197 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -113,6 +113,7 @@ struct tegra_pmc_soc {

bool has_tsense_reset;
bool has_gpu_clamps;
+ bool has_gpu_toggle;
};

/**
@@ -190,6 +191,9 @@ static int tegra_powergate_set(int id, bool new_state)
u32 status;
int err = 0;

+ if (id == TEGRA_POWERGATE_3D && !pmc->soc->has_gpu_toggle)
+ return -EINVAL;
+
mutex_lock(&pmc->powergates_lock);

status = tegra_pmc_readl(PWRGATE_STATUS);
@@ -246,6 +250,9 @@ int tegra_powergate_is_powered(int id)
if (!PMC_PWRGATE_IS_VALID(id))
return -EINVAL;

+ if (id == TEGRA_POWERGATE_3D && !pmc->soc->has_gpu_toggle)
+ return -EINVAL;
+
status = tegra_pmc_readl(PWRGATE_STATUS);

return PMC_PWRGATE_STATE(status, id);
@@ -929,6 +936,7 @@ static const struct tegra_pmc_soc tegra30_pmc_soc = {
.cpu_powergates = tegra30_cpu_powergates,
.has_tsense_reset = true,
.has_gpu_clamps = false,
+ .has_gpu_toggle = true,
};

static const char * const tegra114_powergates[] = {
@@ -966,6 +974,7 @@ static const struct tegra_pmc_soc tegra114_pmc_soc = {
.cpu_powergates = tegra114_cpu_powergates,
.has_tsense_reset = true,
.has_gpu_clamps = false,
+ .has_gpu_toggle = true,
};

static const char * const tegra124_powergates[] = {
@@ -1009,6 +1018,7 @@ static const struct tegra_pmc_soc tegra124_pmc_soc = {
.cpu_powergates = tegra124_cpu_powergates,
.has_tsense_reset = true,
.has_gpu_clamps = true,
+ .has_gpu_toggle = false,
};

static const char * const tegra210_powergates[] = {
@@ -1052,6 +1062,7 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
.cpu_powergates = tegra210_cpu_powergates,
.has_tsense_reset = true,
.has_gpu_clamps = true,
+ .has_gpu_toggle = false,
};

static const struct of_device_id tegra_pmc_match[] = {
--
2.1.4

2015-12-04 15:00:50

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 10/16] PM / Domains: Add function to remove a pm-domain

The genpd framework allows users to add power-domains via the
pm_genpd_init() function, however, there is no corresponding function
to remove a power-domain. For most devices this may be fine as the power
domains are never removed, however, for devices that wish to populate
the power-domains from within a driver, having the ability to remove a
power domain if the probing of the device fails or the driver is unloaded
is necessary. Therefore, add a function to remove a power-domain. Please
note that the power domain can only be removed if there are no devices
using the power-domain and it is not linked to another domain.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/base/power/domain.c | 26 ++++++++++++++++++++++++++
include/linux/pm_domain.h | 5 +++++
2 files changed, 31 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e03b1ad25a90..137f29cde8a8 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1509,6 +1509,32 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
}
EXPORT_SYMBOL_GPL(pm_genpd_init);

+/**
+ * pm_genpd_remove - Remove a generic I/O PM domain object.
+ * @genpd: PM domain object to remove.
+ */
+int pm_genpd_remove(struct generic_pm_domain *genpd)
+{
+ if (IS_ERR_OR_NULL(genpd))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (!list_empty(&genpd->master_links)
+ || !list_empty(&genpd->slave_links) || genpd->device_count) {
+ mutex_unlock(&genpd->lock);
+ return -EBUSY;
+ }
+
+ mutex_lock_nested(&gpd_list_lock, SINGLE_DEPTH_NESTING);
+ list_del(&genpd->gpd_list_node);
+ mutex_unlock(&gpd_list_lock);
+ mutex_unlock(&genpd->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_remove);
+
#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
/*
* Device Tree based PM domain providers.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index ba4ced38efae..637699ff5a23 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -123,6 +123,7 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
struct generic_pm_domain *target);
extern void pm_genpd_init(struct generic_pm_domain *genpd,
struct dev_power_governor *gov, bool is_off);
+extern int pm_genpd_remove(struct generic_pm_domain *genpd);

extern struct dev_power_governor simple_qos_governor;
extern struct dev_power_governor pm_domain_always_on_gov;
@@ -161,6 +162,10 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd,
struct dev_power_governor *gov, bool is_off)
{
}
+static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
+{
+ return -ENOTSUPP;
+}
#endif

static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
--
2.1.4

2015-12-04 15:00:58

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 11/16] Documentation: DT: bindings: Update NVIDIA PMC for Tegra210

---
Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
index 02c27004d4a8..838e1a69ec0a 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
@@ -9,8 +9,9 @@ Required properties:
- compatible : For Tegra20, must contain "nvidia,tegra20-pmc". For Tegra30,
must contain "nvidia,tegra30-pmc". For Tegra114, must contain
"nvidia,tegra114-pmc". For Tegra124, must contain "nvidia,tegra124-pmc".
- Otherwise, must contain "nvidia,<chip>-pmc", plus at least one of the
- above, where <chip> is tegra132.
+ For Tegra210, must contain "nvidia,tegra210-pmc". Otherwise, must contain
+ "nvidia,<chip>-pmc", plus at least one of the above, where <chip> is
+ tegra132.
- reg : Offset and length of the register set for the device
- clocks : Must contain an entry for each entry in clock-names.
See ../clocks/clock-bindings.txt for details.
--
2.1.4

2015-12-04 15:01:05

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC

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

Signed-off-by: Jon Hunter <[email protected]>

---

Please note that I have been debating whether I add this
"nvidia,powergate-clock-disable" property or just leave the clocks
disabled by default. Some downstream kernels leave the clocks enabled
for the audio power-domain because the clocks required for powering up
the power-domain are needed by all modules within the power-domain.
However are the same time there are other power-domains that may need
to be on, but not always clocked and so having the ability to specify if
the clocks should be disabled seems useful. However, I can also remove
this and just have the appropriate devices turn on the clocks as well.
---
.../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
1 file changed, 61 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
index 838e1a69ec0a..8e4641db51a9 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.
@@ -69,6 +71,10 @@ 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:
+- pm-domains : This node contains a hierarchy of PM domain nodes, which should
+ match the power-domains on the Tegra SoC.
+
Example:

/ SoC dts including file
@@ -114,3 +120,58 @@ pmc@7000f400 {
};
...
};
+
+
+== PM Domain Nodes ==
+
+Each of the PM domain nodes represents a power-domain on the Tegra SoC
+that can be power-gated by the PMC and should be named appropriately.
+
+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.
+ - nvidia,powergate: Integer cell that contains an identifier for the PMC
+ power-gate that is associated with the power-domain. Please refer to
+ the Tegra TRM for more details.
+ - #power-domain-cells: Must be 0.
+
+Optional properties:
+ - nvidia,powergate-disable-clocks: Boolean property that if present
+ indicates that the clocks listed in the "clocks" property should be
+ disabled after turning on the power-domain. Otherwise the clocks will
+ be kept enabled.
+
+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";
+
+ pm-domains {
+ pd_audio: aud {
+ clocks = <&tegra_car TEGRA210_CLK_APE>,
+ <&tegra_car TEGRA210_CLK_APB2APE>;
+ resets = <&tegra_car 198>;
+ nvidia,powergate = <TEGRA_POWERGATE_AUD>;
+ #power-domain-cells = <0>;
+ };
+ };
+ };
+
+
+== PM Domain Consumers ==
+
+Hardware blocks belonging to a PM domain should contain a "power-domains"
+property that is a phandle pointing to the corresponding PM domain node.
+
+Example:
+
+ adma: adma@702e2000 {
+ ...
+ power-domains = <&pd_audio>;
+ ...
+ };
--
2.1.4

2015-12-04 15:01:13

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 13/16] soc: tegra: pmc: Add generic PM domain support

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.

This is based upon work by Thierry Reding <[email protected]>
and Vince Hsu <[email protected]>.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 504 ++++++++++++++++++++++++++--
include/dt-bindings/power/tegra-powergate.h | 35 ++
include/soc/tegra/pmc.h | 38 +--
3 files changed, 513 insertions(+), 64 deletions(-)
create mode 100644 include/dt-bindings/power/tegra-powergate.h

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index b412098b8197..76bee999c85b 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -19,6 +19,8 @@

#define pr_fmt(fmt) "tegra-pmc: " fmt

+#include <dt-bindings/power/tegra-powergate.h>
+
#include <linux/kernel.h>
#include <linux/clk.h>
#include <linux/clk/tegra.h>
@@ -31,7 +33,9 @@
#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>
@@ -105,6 +109,20 @@
#define PMC_PWRGATE_STATE(status, id) ((status & BIT(id)) != 0)
#define PMC_PWRGATE_IS_VALID(id) (pmc->powergates_mask & BIT(id))

+struct tegra_powergate {
+ struct generic_pm_domain genpd;
+ struct generic_pm_domain *parent;
+ struct tegra_pmc *pmc;
+ unsigned int id;
+ struct list_head node;
+ struct device_node *of_node;
+ struct clk **clks;
+ unsigned int num_clks;
+ struct reset_control **resets;
+ unsigned int num_resets;
+ bool disable_clocks;
+};
+
struct tegra_pmc_soc {
unsigned int num_powergates;
const char *const *powergates;
@@ -137,6 +155,7 @@ struct tegra_pmc_soc {
* @lp0_vec_phys: physical base address of the LP0 warm boot code
* @lp0_vec_size: size of the LP0 warm boot code
* @powergates_mask: Bit mask of valid power gates
+ * @powergates_list: list of power gates
* @powergates_lock: mutex for power gate register access
*/
struct tegra_pmc {
@@ -163,6 +182,7 @@ struct tegra_pmc {
u32 lp0_vec_size;
u32 powergates_mask;

+ struct list_head powergates_list;
struct mutex powergates_lock;
};

@@ -171,6 +191,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);
@@ -214,6 +240,177 @@ static int tegra_powergate_set(int id, bool new_state)
return err;
}

+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 err_clks;
+
+ usleep_range(10, 20);
+
+ tegra_powergate_remove_clamping(pg->id);
+
+ usleep_range(10, 20);
+
+ err = tegra_powergate_reset_deassert(pg);
+ if (err)
+ goto err_reset;
+
+ usleep_range(10, 20);
+
+ if (disable_clocks)
+ tegra_powergate_disable_clocks(pg);
+
+ return 0;
+
+err_reset:
+ tegra_powergate_disable_clocks(pg);
+ usleep_range(10, 20);
+err_clks:
+ tegra_powergate_set(pg->id, false);
+
+ return err;
+}
+
+static int tegra_powergate_power_down(struct tegra_powergate *pg,
+ bool enable_clocks)
+{
+ int err;
+
+ if (enable_clocks) {
+ err = tegra_powergate_enable_clocks(pg);
+ if (err)
+ return err;
+
+ usleep_range(10, 20);
+ }
+
+ err = tegra_powergate_reset_assert(pg);
+ if (err)
+ goto err_reset;
+
+ usleep_range(10, 20);
+
+ tegra_powergate_disable_clocks(pg);
+
+ usleep_range(10, 20);
+
+ err = tegra_powergate_set(pg->id, false);
+ if (err)
+ goto err_powergate;
+
+ return 0;
+
+err_powergate:
+ tegra_powergate_enable_clocks(pg);
+ usleep_range(10, 20);
+ tegra_powergate_reset_deassert(pg);
+ usleep_range(10, 20);
+err_reset:
+ 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, pg->disable_clocks);
+ if (err)
+ dev_err(pmc->dev, "failed to turn on PM domain %s: %d\n",
+ pg->of_node->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, !pg->disable_clocks);
+ if (err)
+ dev_err(pmc->dev, "failed to turn off PM domain %s: %d\n",
+ pg->of_node->name, err);
+
+ return err;
+}
+
/**
* tegra_powergate_power_on() - power on partition
* @id: partition ID
@@ -308,35 +505,20 @@ EXPORT_SYMBOL(tegra_powergate_remove_clamping);
int tegra_powergate_sequence_power_up(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);
-
- ret = tegra_powergate_remove_clamping(id);
- if (ret)
- goto err_clamp;
+ struct tegra_powergate pg;
+ int err;

- usleep_range(10, 20);
- reset_control_deassert(rst);
+ pg.id = id;
+ pg.clks = &clk;
+ pg.num_clks = 1;
+ pg.resets = &rst;
+ pg.num_resets = 1;

- 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);

@@ -476,6 +658,260 @@ static int tegra_powergate_debugfs_init(void)
return 0;
}

+static int tegra_powergate_of_get_clks(struct device *dev,
+ struct tegra_powergate *pg)
+{
+ struct clk *clk;
+ unsigned int i, count;
+ int err;
+
+ /*
+ * Determine number of clocks used by the powergate
+ */
+ for (count = 0; ; count++) {
+ clk = of_clk_get(pg->of_node, count);
+ if (IS_ERR(clk))
+ break;
+
+ clk_put(clk);
+ }
+
+ if (PTR_ERR(clk) != -ENOENT)
+ return PTR_ERR(clk);
+
+ if (count == 0)
+ return -ENODEV;
+
+ pg->clks = devm_kcalloc(dev, count, sizeof(clk), GFP_KERNEL);
+ if (!pg->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < count; i++) {
+ pg->clks[i] = of_clk_get(pg->of_node, 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]);
+
+ return err;
+}
+
+static int tegra_powergate_of_get_resets(struct device *dev,
+ struct tegra_powergate *pg)
+{
+ struct reset_control *rst;
+ unsigned int i, count;
+ int err;
+
+ /*
+ * Determine number of resets used by the powergate
+ */
+ for (count = 0; ; count++) {
+ rst = of_reset_control_get_by_index(pg->of_node, count);
+ if (IS_ERR(rst))
+ break;
+
+ reset_control_put(rst);
+ }
+
+ if (PTR_ERR(rst) != -ENOENT)
+ return PTR_ERR(rst);
+
+ if (count == 0)
+ return -ENODEV;
+
+ pg->resets = devm_kcalloc(dev, 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(pg->of_node, i);
+ if (IS_ERR(pg->resets[i])) {
+ err = PTR_ERR(pg->resets[i]);
+ goto err;
+ }
+ }
+
+ pg->num_resets = count;
+
+ return 0;
+
+err:
+ while (i--)
+ reset_control_put(pg->resets[i]);
+
+ return err;
+}
+
+static struct tegra_powergate *
+tegra_powergate_add_one(struct tegra_pmc *pmc, struct device_node *np,
+ struct generic_pm_domain *parent)
+{
+ struct tegra_powergate *pg;
+ unsigned int id;
+ bool off;
+ int err;
+
+ /*
+ * If the powergate ID is missing or invalid then return NULL
+ * to skip this one and allow any others to be created.
+ */
+ err = of_property_read_u32(np, "nvidia,powergate", &id);
+ if (err) {
+ dev_WARN(pmc->dev, "no powergate ID for node %s\n", np->name);
+ return NULL;
+ }
+
+ if (!PMC_PWRGATE_IS_VALID(id)) {
+ dev_WARN(pmc->dev, "powergate ID invalid for %s\n", np->name);
+ return NULL;
+ }
+
+ pg = devm_kzalloc(pmc->dev, sizeof(*pg), GFP_KERNEL);
+ if (!pg) {
+ err = -ENOMEM;
+ goto err_mem;
+ }
+
+ pg->id = id;
+ pg->of_node = np;
+ pg->parent = parent;
+ pg->genpd.name = np->name;
+ pg->genpd.power_off = tegra_genpd_power_off;
+ pg->genpd.power_on = tegra_genpd_power_on;
+ pg->pmc = pmc;
+
+ pg->disable_clocks = of_property_read_bool(np,
+ "nvidia,powergate-disable-clocks");
+
+ err = tegra_powergate_of_get_clks(pmc->dev, pg);
+ if (err)
+ goto err_mem;
+
+ err = tegra_powergate_of_get_resets(pmc->dev, pg);
+ if (err)
+ goto err_reset;
+
+ off = !tegra_powergate_is_powered(pg->id);
+
+ pm_genpd_init(&pg->genpd, NULL, off);
+
+ if (pg->parent) {
+ err = pm_genpd_add_subdomain(pg->parent, &pg->genpd);
+ if (err)
+ goto err_subdomain;
+ }
+
+ err = of_genpd_add_provider_simple(pg->of_node, &pg->genpd);
+ if (err)
+ goto err_provider;
+
+ list_add_tail(&pg->node, &pmc->powergates_list);
+
+ dev_info(pmc->dev, "added power domain %s\n", pg->genpd.name);
+
+ return pg;
+
+err_provider:
+ WARN_ON(pm_genpd_remove_subdomain(pg->parent, &pg->genpd));
+err_subdomain:
+ WARN_ON(pm_genpd_remove(&pg->genpd));
+ while (pg->num_resets--)
+ reset_control_put(pg->resets[pg->num_resets]);
+err_reset:
+ while (pg->num_clks--)
+ clk_put(pg->clks[pg->num_clks]);
+err_mem:
+ dev_err(pmc->dev, "failed to create power domain for node %s\n",
+ np->name);
+
+ return ERR_PTR(err);
+}
+
+static int tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np,
+ struct generic_pm_domain *parent)
+{
+ struct tegra_powergate *pg;
+ struct device_node *child;
+ int err = 0;
+
+ for_each_child_of_node(np, child) {
+ if (err)
+ goto err;
+
+ pg = tegra_powergate_add_one(pmc, child, parent);
+ if (IS_ERR(pg)) {
+ err = PTR_ERR(pg);
+ goto err;
+ }
+
+ if (pg)
+ err = tegra_powergate_add(pmc, child, pg->parent);
+
+err:
+ of_node_put(child);
+
+ if (err)
+ return err;
+ }
+
+ return err;
+}
+
+static void tegra_powergate_remove(struct tegra_pmc *pmc)
+{
+ struct tegra_powergate *pg, *n;
+
+ list_for_each_entry_safe(pg, n, &pmc->powergates_list, node) {
+ of_genpd_del_provider(pg->of_node);
+ if (pg->parent)
+ pm_genpd_remove_subdomain(pg->parent, &pg->genpd);
+ pm_genpd_remove(&pg->genpd);
+
+ while (pg->num_clks--)
+ clk_put(pg->clks[pg->num_clks]);
+
+ while (pg->num_resets--)
+ reset_control_put(pg->resets[pg->num_resets]);
+
+ list_del(&pg->node);
+ }
+}
+
+static int tegra_powergate_init(struct tegra_pmc *pmc)
+{
+ struct device_node *np;
+ int err;
+
+ INIT_LIST_HEAD(&pmc->powergates_list);
+
+ np = of_get_child_by_name(pmc->dev->of_node, "pm-domains");
+ if (!np) {
+ dev_dbg(pmc->dev, "pm-domains node not found\n");
+ return 0;
+ }
+
+ err = tegra_powergate_add(pmc, np, NULL);
+ if (err) {
+ tegra_powergate_remove(pmc);
+ of_node_put(np);
+ return err;
+ }
+
+ of_node_put(np);
+
+ return 0;
+}
+
static int tegra_io_rail_prepare(int id, unsigned long *request,
unsigned long *status, unsigned int *bit)
{
@@ -850,21 +1286,31 @@ static int tegra_pmc_probe(struct platform_device *pdev)

tegra_pmc_init_tsense_reset(pmc);

+ err = tegra_powergate_init(pmc);
+ if (err < 0)
+ return err;
+
if (IS_ENABLED(CONFIG_DEBUG_FS)) {
err = tegra_powergate_debugfs_init();
if (err < 0)
- return err;
+ goto err_debugfs;
}

err = register_restart_handler(&tegra_pmc_restart_handler);
if (err) {
- debugfs_remove(pmc->debugfs);
dev_err(&pdev->dev, "unable to register restart handler, %d\n",
err);
- return err;
+ goto err_restart;
}

return 0;
+
+err_restart:
+ debugfs_remove(pmc->debugfs);
+err_debugfs:
+ tegra_powergate_remove(pmc);
+
+ return err;
}

#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM)
diff --git a/include/dt-bindings/power/tegra-powergate.h b/include/dt-bindings/power/tegra-powergate.h
new file mode 100644
index 000000000000..12b72aa77d6a
--- /dev/null
+++ b/include/dt-bindings/power/tegra-powergate.h
@@ -0,0 +1,35 @@
+#ifndef _DT_BINDINGS_POWER_TEGRA_POWERGATE_H
+#define _DT_BINDINGS_POWER_TEGRA_POWERGATE_H
+
+#define TEGRA_POWERGATE_CPU 0
+#define TEGRA_POWERGATE_3D 1
+#define TEGRA_POWERGATE_VENC 2
+#define TEGRA_POWERGATE_PCIE 3
+#define TEGRA_POWERGATE_VDEC 4
+#define TEGRA_POWERGATE_L2 5
+#define TEGRA_POWERGATE_MPE 6
+#define TEGRA_POWERGATE_HEG 7
+#define TEGRA_POWERGATE_SATA 8
+#define TEGRA_POWERGATE_CPU1 9
+#define TEGRA_POWERGATE_CPU2 10
+#define TEGRA_POWERGATE_CPU3 11
+#define TEGRA_POWERGATE_CELP 12
+#define TEGRA_POWERGATE_3D1 13
+#define TEGRA_POWERGATE_CPU0 14
+#define TEGRA_POWERGATE_C0NC 15
+#define TEGRA_POWERGATE_C1NC 16
+#define TEGRA_POWERGATE_SOR 17
+#define TEGRA_POWERGATE_DIS 18
+#define TEGRA_POWERGATE_DISB 19
+#define TEGRA_POWERGATE_XUSBA 20
+#define TEGRA_POWERGATE_XUSBB 21
+#define TEGRA_POWERGATE_XUSBC 22
+#define TEGRA_POWERGATE_VIC 23
+#define TEGRA_POWERGATE_IRAM 24
+#define TEGRA_POWERGATE_NVDEC 25
+#define TEGRA_POWERGATE_NVJPG 26
+#define TEGRA_POWERGATE_AUD 27
+#define TEGRA_POWERGATE_DFD 28
+#define TEGRA_POWERGATE_VE2 29
+
+#endif
diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
index d18efe402ff1..61c9f847f2b2 100644
--- a/include/soc/tegra/pmc.h
+++ b/include/soc/tegra/pmc.h
@@ -19,6 +19,8 @@
#ifndef __SOC_TEGRA_PMC_H__
#define __SOC_TEGRA_PMC_H__

+#include <dt-bindings/power/tegra-powergate.h>
+
#include <linux/reboot.h>

#include <soc/tegra/pm.h>
@@ -39,42 +41,8 @@ int tegra_pmc_cpu_remove_clamping(int cpuid);
#endif /* CONFIG_SMP */

/*
- * powergate and I/O rail APIs
+ * I/O rail APIs
*/
-
-#define TEGRA_POWERGATE_CPU 0
-#define TEGRA_POWERGATE_3D 1
-#define TEGRA_POWERGATE_VENC 2
-#define TEGRA_POWERGATE_PCIE 3
-#define TEGRA_POWERGATE_VDEC 4
-#define TEGRA_POWERGATE_L2 5
-#define TEGRA_POWERGATE_MPE 6
-#define TEGRA_POWERGATE_HEG 7
-#define TEGRA_POWERGATE_SATA 8
-#define TEGRA_POWERGATE_CPU1 9
-#define TEGRA_POWERGATE_CPU2 10
-#define TEGRA_POWERGATE_CPU3 11
-#define TEGRA_POWERGATE_CELP 12
-#define TEGRA_POWERGATE_3D1 13
-#define TEGRA_POWERGATE_CPU0 14
-#define TEGRA_POWERGATE_C0NC 15
-#define TEGRA_POWERGATE_C1NC 16
-#define TEGRA_POWERGATE_SOR 17
-#define TEGRA_POWERGATE_DIS 18
-#define TEGRA_POWERGATE_DISB 19
-#define TEGRA_POWERGATE_XUSBA 20
-#define TEGRA_POWERGATE_XUSBB 21
-#define TEGRA_POWERGATE_XUSBC 22
-#define TEGRA_POWERGATE_VIC 23
-#define TEGRA_POWERGATE_IRAM 24
-#define TEGRA_POWERGATE_NVDEC 25
-#define TEGRA_POWERGATE_NVJPG 26
-#define TEGRA_POWERGATE_AUD 27
-#define TEGRA_POWERGATE_DFD 28
-#define TEGRA_POWERGATE_VE2 29
-
-#define TEGRA_POWERGATE_3D0 TEGRA_POWERGATE_3D
-
#define TEGRA_IO_RAIL_CSIA 0
#define TEGRA_IO_RAIL_CSIB 1
#define TEGRA_IO_RAIL_DSI 2
--
2.1.4

2015-12-04 15:01:18

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 14/16] clk: tegra210: Add the APB2APE audio clock

The APB2APE clock for the audio subsystem is required for powering up the
audio power domain and accessing the various modules in this subsystem on
the tegra210 device. Add this clock for tegra210.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/clk/tegra/clk-id.h | 1 +
drivers/clk/tegra/clk-tegra-periph.c | 1 +
drivers/clk/tegra/clk-tegra210.c | 1 +
include/dt-bindings/clock/tegra210-car.h | 2 +-
4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h
index 19ce0738ee76..62ea38187b71 100644
--- a/drivers/clk/tegra/clk-id.h
+++ b/drivers/clk/tegra/clk-id.h
@@ -11,6 +11,7 @@ enum clk_id {
tegra_clk_afi,
tegra_clk_amx,
tegra_clk_amx1,
+ tegra_clk_apb2ape,
tegra_clk_apbdma,
tegra_clk_apbif,
tegra_clk_ape,
diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 6ad381a888a6..f8360c850c69 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -829,6 +829,7 @@ static struct tegra_periph_init_data gate_clks[] = {
GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, 0),
GATE("pll_p_out_cpu", "pll_p", 223, 0, tegra_clk_pll_p_out_cpu, 0),
GATE("pll_p_out_adsp", "pll_p", 187, 0, tegra_clk_pll_p_out_adsp, 0),
+ GATE("apb2ape", "clk_m", 107, 0, tegra_clk_apb2ape, 0),
};

static struct tegra_periph_init_data div_clks[] = {
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 58514c44ea83..e2d44a6a6b96 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -2218,6 +2218,7 @@ static struct tegra_clk tegra210_clks[tegra_clk_max] __initdata = {
[tegra_clk_pll_c4_out1] = { .dt_id = TEGRA210_CLK_PLL_C4_OUT1, .present = true },
[tegra_clk_pll_c4_out2] = { .dt_id = TEGRA210_CLK_PLL_C4_OUT2, .present = true },
[tegra_clk_pll_c4_out3] = { .dt_id = TEGRA210_CLK_PLL_C4_OUT3, .present = true },
+ [tegra_clk_apb2ape] = { .dt_id = TEGRA210_CLK_APB2APE, .present = true },
};

static struct tegra_devclk devclks[] __initdata = {
diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
index 6f45aea49e4f..0a05b0d36ae7 100644
--- a/include/dt-bindings/clock/tegra210-car.h
+++ b/include/dt-bindings/clock/tegra210-car.h
@@ -126,7 +126,7 @@
/* 104 */
/* 105 */
#define TEGRA210_CLK_D_AUDIO 106
-/* 107 ( affects abp -> ape) */
+#define TEGRA210_CLK_APB2APE 107
/* 108 */
/* 109 */
/* 110 */
--
2.1.4

2015-12-04 15:01:54

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 15/16] ARM64: tegra: Add audio PM domain device node for Tegra210

Add the audio power-domain for tegra210. Note that this also removes the
existing "#power-domain-cells" which was incorrectly included by
commit e53095857166 ("arm64: tegra: Add Tegra210 support").

Signed-off-by: Jon Hunter <[email protected]>

---

So far I have only added the audio power-domain for tegra210 as this is
what I have been testing with to date. However, once this series is
accepted then we can begin to add more.
---
arch/arm/boot/dts/tegra124.dtsi | 1 +
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 68669f791c8b..d868f4452c4e 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -3,6 +3,7 @@
#include <dt-bindings/memory/tegra124-mc.h>
#include <dt-bindings/pinctrl/pinctrl-tegra.h>
#include <dt-bindings/pinctrl/pinctrl-tegra-xusb.h>
+#include <dt-bindings/power/tegra-powergate.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/reset/tegra124-car.h>
#include <dt-bindings/thermal/tegra124-soctherm.h>
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index bc23f4dea002..c5e9c320092b 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -2,6 +2,7 @@
#include <dt-bindings/gpio/tegra-gpio.h>
#include <dt-bindings/memory/tegra210-mc.h>
#include <dt-bindings/pinctrl/pinctrl-tegra.h>
+#include <dt-bindings/power/tegra-powergate.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>

/ {
@@ -581,7 +582,15 @@
clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
clock-names = "pclk", "clk32k_in";

- #power-domain-cells = <1>;
+ pm-domains {
+ pd_audio: aud {
+ clocks = <&tegra_car TEGRA210_CLK_APE>,
+ <&tegra_car TEGRA210_CLK_APB2APE>;
+ resets = <&tegra_car 198>;
+ nvidia,powergate = <TEGRA_POWERGATE_AUD>;
+ #power-domain-cells = <0>;
+ };
+ };
};

fuse@0,7000f800 {
--
2.1.4

2015-12-04 15:01:25

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS

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 <[email protected]>
---
arch/arm64/Kconfig.platforms | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 9806324fa215..e0b5bd0aff0f 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -93,6 +93,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

2015-12-06 00:31:14

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V4 11/16] Documentation: DT: bindings: Update NVIDIA PMC for Tegra210

On Fri, Dec 04, 2015 at 02:57:12PM +0000, Jon Hunter wrote:
> ---
> Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> index 02c27004d4a8..838e1a69ec0a 100644
> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> @@ -9,8 +9,9 @@ Required properties:
> - compatible : For Tegra20, must contain "nvidia,tegra20-pmc". For Tegra30,
> must contain "nvidia,tegra30-pmc". For Tegra114, must contain
> "nvidia,tegra114-pmc". For Tegra124, must contain "nvidia,tegra124-pmc".
> - Otherwise, must contain "nvidia,<chip>-pmc", plus at least one of the
> - above, where <chip> is tegra132.
> + For Tegra210, must contain "nvidia,tegra210-pmc". Otherwise, must contain
> + "nvidia,<chip>-pmc", plus at least one of the above, where <chip> is
> + tegra132.

Can you reorg this some so the compatible strings are a list rather than
a paragraph of text. I find the last sentence a bit confusing as well.
Seems like <chip> should be a list rather than just tegra132.


> - reg : Offset and length of the register set for the device
> - clocks : Must contain an entry for each entry in clock-names.
> See ../clocks/clock-bindings.txt for details.
> --
> 2.1.4
>

2015-12-06 00:38:03

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC

On Fri, Dec 04, 2015 at 02:57:13PM +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 <[email protected]>
>
> ---
>
> Please note that I have been debating whether I add this
> "nvidia,powergate-clock-disable" property or just leave the clocks
> disabled by default. Some downstream kernels leave the clocks enabled
> for the audio power-domain because the clocks required for powering up
> the power-domain are needed by all modules within the power-domain.
> However are the same time there are other power-domains that may need
> to be on, but not always clocked and so having the ability to specify if
> the clocks should be disabled seems useful. However, I can also remove
> this and just have the appropriate devices turn on the clocks as well.

Seems like that is just a failure of drivers to control all the clocks
they need IMO.

> ---
> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> index 838e1a69ec0a..8e4641db51a9 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.
> @@ -69,6 +71,10 @@ 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:
> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
> + match the power-domains on the Tegra SoC.
> +
> Example:
>
> / SoC dts including file
> @@ -114,3 +120,58 @@ pmc@7000f400 {
> };
> ...
> };
> +
> +
> +== PM Domain Nodes ==
> +
> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
> +that can be power-gated by the PMC and should be named appropriately.
> +
> +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.
> + - nvidia,powergate: Integer cell that contains an identifier for the PMC
> + power-gate that is associated with the power-domain. Please refer to
> + the Tegra TRM for more details.

"reg" does not work here?

Rob

2015-12-07 09:54:24

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 11/16] Documentation: DT: bindings: Update NVIDIA PMC for Tegra210


On 06/12/15 00:31, Rob Herring wrote:
> On Fri, Dec 04, 2015 at 02:57:12PM +0000, Jon Hunter wrote:
>> ---
>> Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> index 02c27004d4a8..838e1a69ec0a 100644
>> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> @@ -9,8 +9,9 @@ Required properties:
>> - compatible : For Tegra20, must contain "nvidia,tegra20-pmc". For Tegra30,
>> must contain "nvidia,tegra30-pmc". For Tegra114, must contain
>> "nvidia,tegra114-pmc". For Tegra124, must contain "nvidia,tegra124-pmc".
>> - Otherwise, must contain "nvidia,<chip>-pmc", plus at least one of the
>> - above, where <chip> is tegra132.
>> + For Tegra210, must contain "nvidia,tegra210-pmc". Otherwise, must contain
>> + "nvidia,<chip>-pmc", plus at least one of the above, where <chip> is
>> + tegra132.
>
> Can you reorg this some so the compatible strings are a list rather than
> a paragraph of text. I find the last sentence a bit confusing as well.
> Seems like <chip> should be a list rather than just tegra132.

Yes no problem.

Thanks
Jon

2015-12-07 09:57:05

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC


On 06/12/15 00:37, Rob Herring wrote:
> On Fri, Dec 04, 2015 at 02:57:13PM +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 <[email protected]>
>>
>> ---
>>
>> Please note that I have been debating whether I add this
>> "nvidia,powergate-clock-disable" property or just leave the clocks
>> disabled by default. Some downstream kernels leave the clocks enabled
>> for the audio power-domain because the clocks required for powering up
>> the power-domain are needed by all modules within the power-domain.
>> However are the same time there are other power-domains that may need
>> to be on, but not always clocked and so having the ability to specify if
>> the clocks should be disabled seems useful. However, I can also remove
>> this and just have the appropriate devices turn on the clocks as well.
>
> Seems like that is just a failure of drivers to control all the clocks
> they need IMO.

Right and that was why I was on the fence. However, I could see that
some domains may always need a bus clock on and so it did not seem
complete absurd. If the consensus is to drop it, I will.

>> ---
>> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> index 838e1a69ec0a..8e4641db51a9 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.
>> @@ -69,6 +71,10 @@ 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:
>> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
>> + match the power-domains on the Tegra SoC.
>> +
>> Example:
>>
>> / SoC dts including file
>> @@ -114,3 +120,58 @@ pmc@7000f400 {
>> };
>> ...
>> };
>> +
>> +
>> +== PM Domain Nodes ==
>> +
>> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
>> +that can be power-gated by the PMC and should be named appropriately.
>> +
>> +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.
>> + - nvidia,powergate: Integer cell that contains an identifier for the PMC
>> + power-gate that is associated with the power-domain. Please refer to
>> + the Tegra TRM for more details.
>
> "reg" does not work here?

Not really. This value is used to program a register to power-up/down
the particular power-domain you are interested in.

Cheers
Jon

2015-12-08 19:07:15

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC

Jon Hunter <[email protected]> writes:

> Add power-domain binding documentation for the NVIDIA PMC driver in
> order to support generic power-domains.
>
> Signed-off-by: Jon Hunter <[email protected]>
>
> ---
>
> Please note that I have been debating whether I add this
> "nvidia,powergate-clock-disable" property or just leave the clocks
> disabled by default. Some downstream kernels leave the clocks enabled
> for the audio power-domain because the clocks required for powering up
> the power-domain are needed by all modules within the power-domain.
> However are the same time there are other power-domains that may need
> to be on, but not always clocked and so having the ability to specify if
> the clocks should be disabled seems useful. However, I can also remove
> this and just have the appropriate devices turn on the clocks as well.
> ---
> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> index 838e1a69ec0a..8e4641db51a9 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.
> @@ -69,6 +71,10 @@ 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:
> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
> + match the power-domains on the Tegra SoC.
> +
> Example:
>
> / SoC dts including file
> @@ -114,3 +120,58 @@ pmc@7000f400 {
> };
> ...
> };
> +
> +
> +== PM Domain Nodes ==
> +
> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
> +that can be power-gated by the PMC and should be named appropriately.
> +
> +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.

We've had this discussiona for a couple of other SoCs, so I need to
ask...

Presumably these are not device clocks that a runtime PM enabled driver
should be managing for a device, right? IOW, We want to make sure that the
PM domain isn't managing clocks for drivers that should be doing it.

I understand there are legitimate reasons for the PM domain to manage
clocks in addition to device drivers (e.g. for synchronous reset), but
just want to be sure it's not a shortcut for having a proper driver.

Kevin

2015-12-09 12:23:56

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC


On 08/12/15 19:07, Kevin Hilman wrote:
> Jon Hunter <[email protected]> writes:
>
>> Add power-domain binding documentation for the NVIDIA PMC driver in
>> order to support generic power-domains.
>>
>> Signed-off-by: Jon Hunter <[email protected]>
>>
>> ---
>>
>> Please note that I have been debating whether I add this
>> "nvidia,powergate-clock-disable" property or just leave the clocks
>> disabled by default. Some downstream kernels leave the clocks enabled
>> for the audio power-domain because the clocks required for powering up
>> the power-domain are needed by all modules within the power-domain.
>> However are the same time there are other power-domains that may need
>> to be on, but not always clocked and so having the ability to specify if
>> the clocks should be disabled seems useful. However, I can also remove
>> this and just have the appropriate devices turn on the clocks as well.
>> ---
>> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>> index 838e1a69ec0a..8e4641db51a9 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.
>> @@ -69,6 +71,10 @@ 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:
>> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
>> + match the power-domains on the Tegra SoC.
>> +
>> Example:
>>
>> / SoC dts including file
>> @@ -114,3 +120,58 @@ pmc@7000f400 {
>> };
>> ...
>> };
>> +
>> +
>> +== PM Domain Nodes ==
>> +
>> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
>> +that can be power-gated by the PMC and should be named appropriately.
>> +
>> +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.
>
> We've had this discussiona for a couple of other SoCs, so I need to
> ask...
>
> Presumably these are not device clocks that a runtime PM enabled driver
> should be managing for a device, right? IOW, We want to make sure that the
> PM domain isn't managing clocks for drivers that should be doing it.
>
> I understand there are legitimate reasons for the PM domain to manage
> clocks in addition to device drivers (e.g. for synchronous reset), but
> just want to be sure it's not a shortcut for having a proper driver.

So some clocks may also be used by devices, but they are needed as part
of the power ungating/gating sequence. The general power-up sequence for
tegra is ...

1. Enable the power-domain
2. Enable the clock(s)
3. Remove signal clamps
4. De-assert reset(s)
5. Disable clocks (optional)

You may say we should only handle #1 above for the powering up sequence,
but we can't do this. The reason is that there is one bit for each
power-domain that controls the signaling clamps and so we need to turn
on all the clocks specified in the TRM before we do this. Once we have
done this and released the reset(s), we can then disable the clocks
again (shown above an optional as it is not mandatory from a design
perspective) and then the devices in the power-domain should enable the
clocks they need as and when they want them.

Please note that I 100% agree that all clocks required by a device are
handled by the device and we do not implement any short-cuts here. The
only question I had was if there are clocks that may be bus clocks in
the power-domain that are required by all device in the power-domain.
However, may be this should be represented as a bus driver and all the
devices are children of it?

Hope that helps.

Cheers
Jon

2015-12-09 12:33:43

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC


On 09/12/15 12:23, Jon Hunter wrote:
>
> On 08/12/15 19:07, Kevin Hilman wrote:
>> Jon Hunter <[email protected]> writes:
>>
>>> Add power-domain binding documentation for the NVIDIA PMC driver in
>>> order to support generic power-domains.
>>>
>>> Signed-off-by: Jon Hunter <[email protected]>
>>>
>>> ---
>>>
>>> Please note that I have been debating whether I add this
>>> "nvidia,powergate-clock-disable" property or just leave the clocks
>>> disabled by default. Some downstream kernels leave the clocks enabled
>>> for the audio power-domain because the clocks required for powering up
>>> the power-domain are needed by all modules within the power-domain.
>>> However are the same time there are other power-domains that may need
>>> to be on, but not always clocked and so having the ability to specify if
>>> the clocks should be disabled seems useful. However, I can also remove
>>> this and just have the appropriate devices turn on the clocks as well.
>>> ---
>>> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
>>> 1 file changed, 61 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>> index 838e1a69ec0a..8e4641db51a9 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.
>>> @@ -69,6 +71,10 @@ 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:
>>> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
>>> + match the power-domains on the Tegra SoC.
>>> +
>>> Example:
>>>
>>> / SoC dts including file
>>> @@ -114,3 +120,58 @@ pmc@7000f400 {
>>> };
>>> ...
>>> };
>>> +
>>> +
>>> +== PM Domain Nodes ==
>>> +
>>> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
>>> +that can be power-gated by the PMC and should be named appropriately.
>>> +
>>> +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.
>>
>> We've had this discussiona for a couple of other SoCs, so I need to
>> ask...
>>
>> Presumably these are not device clocks that a runtime PM enabled driver
>> should be managing for a device, right? IOW, We want to make sure that the
>> PM domain isn't managing clocks for drivers that should be doing it.
>>
>> I understand there are legitimate reasons for the PM domain to manage
>> clocks in addition to device drivers (e.g. for synchronous reset), but
>> just want to be sure it's not a shortcut for having a proper driver.
>
> So some clocks may also be used by devices, but they are needed as part
> of the power ungating/gating sequence. The general power-up sequence for
> tegra is ...
>
> 1. Enable the power-domain
> 2. Enable the clock(s)
> 3. Remove signal clamps
> 4. De-assert reset(s)
> 5. Disable clocks (optional)
>
> You may say we should only handle #1 above for the powering up sequence,
> but we can't do this. The reason is that there is one bit for each
> power-domain that controls the signaling clamps and so we need to turn
> on all the clocks specified in the TRM before we do this. Once we have
> done this and released the reset(s), we can then disable the clocks
> again (shown above an optional as it is not mandatory from a design
> perspective) and then the devices in the power-domain should enable the
> clocks they need as and when they want them.
>
> Please note that I 100% agree that all clocks required by a device are
> handled by the device and we do not implement any short-cuts here. The
> only question I had was if there are clocks that may be bus clocks in
> the power-domain that are required by all device in the power-domain.
> However, may be this should be represented as a bus driver and all the
> devices are children of it?

Although the "nvidia,powergate-disable-clocks" optional property I had
proposed could be seen as a bit of a short-cut, it is true :-)

May be I should make the disabling of clocks again mandatory for the
power-up sequence.

Jon

2015-12-15 00:34:42

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC

Jon Hunter <[email protected]> writes:

> On 08/12/15 19:07, Kevin Hilman wrote:
>> Jon Hunter <[email protected]> writes:
>>
>>> Add power-domain binding documentation for the NVIDIA PMC driver in
>>> order to support generic power-domains.
>>>
>>> Signed-off-by: Jon Hunter <[email protected]>
>>>
>>> ---
>>>
>>> Please note that I have been debating whether I add this
>>> "nvidia,powergate-clock-disable" property or just leave the clocks
>>> disabled by default. Some downstream kernels leave the clocks enabled
>>> for the audio power-domain because the clocks required for powering up
>>> the power-domain are needed by all modules within the power-domain.
>>> However are the same time there are other power-domains that may need
>>> to be on, but not always clocked and so having the ability to specify if
>>> the clocks should be disabled seems useful. However, I can also remove
>>> this and just have the appropriate devices turn on the clocks as well.
>>> ---
>>> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
>>> 1 file changed, 61 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>> index 838e1a69ec0a..8e4641db51a9 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.
>>> @@ -69,6 +71,10 @@ 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:
>>> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
>>> + match the power-domains on the Tegra SoC.
>>> +
>>> Example:
>>>
>>> / SoC dts including file
>>> @@ -114,3 +120,58 @@ pmc@7000f400 {
>>> };
>>> ...
>>> };
>>> +
>>> +
>>> +== PM Domain Nodes ==
>>> +
>>> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
>>> +that can be power-gated by the PMC and should be named appropriately.
>>> +
>>> +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.
>>
>> We've had this discussiona for a couple of other SoCs, so I need to
>> ask...
>>
>> Presumably these are not device clocks that a runtime PM enabled driver
>> should be managing for a device, right? IOW, We want to make sure that the
>> PM domain isn't managing clocks for drivers that should be doing it.
>>
>> I understand there are legitimate reasons for the PM domain to manage
>> clocks in addition to device drivers (e.g. for synchronous reset), but
>> just want to be sure it's not a shortcut for having a proper driver.
>
> So some clocks may also be used by devices, but they are needed as part
> of the power ungating/gating sequence. The general power-up sequence for
> tegra is ...
>
> 1. Enable the power-domain
> 2. Enable the clock(s)
> 3. Remove signal clamps
> 4. De-assert reset(s)
> 5. Disable clocks (optional)
>
> You may say we should only handle #1 above for the powering up sequence,
> but we can't do this. The reason is that there is one bit for each
> power-domain that controls the signaling clamps and so we need to turn
> on all the clocks specified in the TRM before we do this. Once we have
> done this and released the reset(s), we can then disable the clocks
> again (shown above an optional as it is not mandatory from a design
> perspective) and then the devices in the power-domain should enable the
> clocks they need as and when they want them.

Thanks for the clarification, that's what I expected and similar to what
I've seen on other SoCs. I just wanted to be sure that the power-domain
clk managent was "in addition" to the device drivers, not "instead of."

Kevin

2015-12-15 00:42:34

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH V4 12/16] Documentation: DT: bindings: Add power domain info for NVIDIA PMC

Jon Hunter <[email protected]> writes:

> On 09/12/15 12:23, Jon Hunter wrote:
>>
>> On 08/12/15 19:07, Kevin Hilman wrote:
>>> Jon Hunter <[email protected]> writes:
>>>
>>>> Add power-domain binding documentation for the NVIDIA PMC driver in
>>>> order to support generic power-domains.
>>>>
>>>> Signed-off-by: Jon Hunter <[email protected]>
>>>>
>>>> ---
>>>>
>>>> Please note that I have been debating whether I add this
>>>> "nvidia,powergate-clock-disable" property or just leave the clocks
>>>> disabled by default. Some downstream kernels leave the clocks enabled
>>>> for the audio power-domain because the clocks required for powering up
>>>> the power-domain are needed by all modules within the power-domain.
>>>> However are the same time there are other power-domains that may need
>>>> to be on, but not always clocked and so having the ability to specify if
>>>> the clocks should be disabled seems useful. However, I can also remove
>>>> this and just have the appropriate devices turn on the clocks as well.
>>>> ---
>>>> .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 61 ++++++++++++++++++++++
>>>> 1 file changed, 61 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>> index 838e1a69ec0a..8e4641db51a9 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.
>>>> @@ -69,6 +71,10 @@ 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:
>>>> +- pm-domains : This node contains a hierarchy of PM domain nodes, which should
>>>> + match the power-domains on the Tegra SoC.
>>>> +
>>>> Example:
>>>>
>>>> / SoC dts including file
>>>> @@ -114,3 +120,58 @@ pmc@7000f400 {
>>>> };
>>>> ...
>>>> };
>>>> +
>>>> +
>>>> +== PM Domain Nodes ==
>>>> +
>>>> +Each of the PM domain nodes represents a power-domain on the Tegra SoC
>>>> +that can be power-gated by the PMC and should be named appropriately.
>>>> +
>>>> +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.
>>>
>>> We've had this discussiona for a couple of other SoCs, so I need to
>>> ask...
>>>
>>> Presumably these are not device clocks that a runtime PM enabled driver
>>> should be managing for a device, right? IOW, We want to make sure that the
>>> PM domain isn't managing clocks for drivers that should be doing it.
>>>
>>> I understand there are legitimate reasons for the PM domain to manage
>>> clocks in addition to device drivers (e.g. for synchronous reset), but
>>> just want to be sure it's not a shortcut for having a proper driver.
>>
>> So some clocks may also be used by devices, but they are needed as part
>> of the power ungating/gating sequence. The general power-up sequence for
>> tegra is ...
>>
>> 1. Enable the power-domain
>> 2. Enable the clock(s)
>> 3. Remove signal clamps
>> 4. De-assert reset(s)
>> 5. Disable clocks (optional)
>>
>> You may say we should only handle #1 above for the powering up sequence,
>> but we can't do this. The reason is that there is one bit for each
>> power-domain that controls the signaling clamps and so we need to turn
>> on all the clocks specified in the TRM before we do this. Once we have
>> done this and released the reset(s), we can then disable the clocks
>> again (shown above an optional as it is not mandatory from a design
>> perspective) and then the devices in the power-domain should enable the
>> clocks they need as and when they want them.
>>
>> Please note that I 100% agree that all clocks required by a device are
>> handled by the device and we do not implement any short-cuts here. The
>> only question I had was if there are clocks that may be bus clocks in
>> the power-domain that are required by all device in the power-domain.
>> However, may be this should be represented as a bus driver and all the
>> devices are children of it?
>
> Although the "nvidia,powergate-disable-clocks" optional property I had
> proposed could be seen as a bit of a short-cut, it is true :-)
> May be I should make the disabling of clocks again mandatory for the
> power-up sequence.

IIUC, that looks like a flag that tells the power-domain to just leave
all the clocks enabled after the domain power up? Is that right?

To me that looks like possibly useful bringup hack, but a short-cut
that's ripe for abuse and would likely be used so that drivers don't
ever need to do their own clock management.

Kevin

2015-12-15 19:54:50

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS

On 4 December 2015 at 15:57, Jon Hunter <[email protected]> wrote:
> 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 <[email protected]>
> ---
> arch/arm64/Kconfig.platforms | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 9806324fa215..e0b5bd0aff0f 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -93,6 +93,8 @@ config ARCH_TEGRA
> select GENERIC_CLOCKEVENTS
> select HAVE_CLK
> select PINCTRL
> + select PM
> + select PM_GENERIC_DOMAINS

If you still want to allow ARCH_TEGRA to run without PM, you should
probably change to:

select PM_GENERIC_DOMAINS if PM

> select RESET_CONTROLLER
> help
> This enables support for the NVIDIA Tegra SoC family.
> --
> 2.1.4
>

Kind regards
Uffe

2015-12-16 09:41:01

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS

Hi Ulf,

On 15/12/15 19:54, Ulf Hansson wrote:
> On 4 December 2015 at 15:57, Jon Hunter <[email protected]> wrote:
>> 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 <[email protected]>
>> ---
>> arch/arm64/Kconfig.platforms | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index 9806324fa215..e0b5bd0aff0f 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -93,6 +93,8 @@ config ARCH_TEGRA
>> select GENERIC_CLOCKEVENTS
>> select HAVE_CLK
>> select PINCTRL
>> + select PM
>> + select PM_GENERIC_DOMAINS
>
> If you still want to allow ARCH_TEGRA to run without PM, you should
> probably change to:
>
> select PM_GENERIC_DOMAINS if PM

Per the changelog this is deliberate. If we allow !PM, then there is a
potential that you could probe a device when the power domain is not
powered on. I understand that some SoCs turn on all the power-domains
when !PM but this will not work for tegra because we don't register the
power domain until later in the boot and so we are relying upon probe
deferral to defer the probe of devices that use power-domains.

Cheers
Jon

2015-12-16 09:47:29

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS

On 16 December 2015 at 10:40, Jon Hunter <[email protected]> wrote:
> Hi Ulf,
>
> On 15/12/15 19:54, Ulf Hansson wrote:
>> On 4 December 2015 at 15:57, Jon Hunter <[email protected]> wrote:
>>> 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 <[email protected]>
>>> ---
>>> arch/arm64/Kconfig.platforms | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>> index 9806324fa215..e0b5bd0aff0f 100644
>>> --- a/arch/arm64/Kconfig.platforms
>>> +++ b/arch/arm64/Kconfig.platforms
>>> @@ -93,6 +93,8 @@ config ARCH_TEGRA
>>> select GENERIC_CLOCKEVENTS
>>> select HAVE_CLK
>>> select PINCTRL
>>> + select PM
>>> + select PM_GENERIC_DOMAINS
>>
>> If you still want to allow ARCH_TEGRA to run without PM, you should
>> probably change to:
>>
>> select PM_GENERIC_DOMAINS if PM
>
> Per the changelog this is deliberate. If we allow !PM, then there is a
> potential that you could probe a device when the power domain is not
> powered on. I understand that some SoCs turn on all the power-domains
> when !PM but this will not work for tegra because we don't register the
> power domain until later in the boot and so we are relying upon probe
> deferral to defer the probe of devices that use power-domains.

So what you are saying is that adding the PM domain support, will fix
some devices to become successfully probed as they were broken before?

If that's the case, this makes all good sense!

Kind regards
Uffe

2015-12-16 11:41:10

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS


On 16/12/15 09:47, Ulf Hansson wrote:
> On 16 December 2015 at 10:40, Jon Hunter <[email protected]> wrote:
>> Hi Ulf,
>>
>> On 15/12/15 19:54, Ulf Hansson wrote:
>>> On 4 December 2015 at 15:57, Jon Hunter <[email protected]> wrote:
>>>> 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 <[email protected]>
>>>> ---
>>>> arch/arm64/Kconfig.platforms | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>>> index 9806324fa215..e0b5bd0aff0f 100644
>>>> --- a/arch/arm64/Kconfig.platforms
>>>> +++ b/arch/arm64/Kconfig.platforms
>>>> @@ -93,6 +93,8 @@ config ARCH_TEGRA
>>>> select GENERIC_CLOCKEVENTS
>>>> select HAVE_CLK
>>>> select PINCTRL
>>>> + select PM
>>>> + select PM_GENERIC_DOMAINS
>>>
>>> If you still want to allow ARCH_TEGRA to run without PM, you should
>>> probably change to:
>>>
>>> select PM_GENERIC_DOMAINS if PM
>>
>> Per the changelog this is deliberate. If we allow !PM, then there is a
>> potential that you could probe a device when the power domain is not
>> powered on. I understand that some SoCs turn on all the power-domains
>> when !PM but this will not work for tegra because we don't register the
>> power domain until later in the boot and so we are relying upon probe
>> deferral to defer the probe of devices that use power-domains.
>
> So what you are saying is that adding the PM domain support, will fix
> some devices to become successfully probed as they were broken before?

Not exactly. There is a legacy tegra_powergate_sequence_power_up() that
has been used to date to get around this. However, by migrating to GENPD
we really need to make PM mandatory, otherwise you could attempt to
probe a device in a power domain that is not powered. In other words,
you are probing blindly. I know some SoCs do this, but that does not
seem very robust.

Cheers
Jon

2015-12-16 12:51:49

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS

On 16 December 2015 at 12:40, Jon Hunter <[email protected]> wrote:
>
> On 16/12/15 09:47, Ulf Hansson wrote:
>> On 16 December 2015 at 10:40, Jon Hunter <[email protected]> wrote:
>>> Hi Ulf,
>>>
>>> On 15/12/15 19:54, Ulf Hansson wrote:
>>>> On 4 December 2015 at 15:57, Jon Hunter <[email protected]> wrote:
>>>>> 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 <[email protected]>
>>>>> ---
>>>>> arch/arm64/Kconfig.platforms | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>>>> index 9806324fa215..e0b5bd0aff0f 100644
>>>>> --- a/arch/arm64/Kconfig.platforms
>>>>> +++ b/arch/arm64/Kconfig.platforms
>>>>> @@ -93,6 +93,8 @@ config ARCH_TEGRA
>>>>> select GENERIC_CLOCKEVENTS
>>>>> select HAVE_CLK
>>>>> select PINCTRL
>>>>> + select PM
>>>>> + select PM_GENERIC_DOMAINS
>>>>
>>>> If you still want to allow ARCH_TEGRA to run without PM, you should
>>>> probably change to:
>>>>
>>>> select PM_GENERIC_DOMAINS if PM
>>>
>>> Per the changelog this is deliberate. If we allow !PM, then there is a
>>> potential that you could probe a device when the power domain is not
>>> powered on. I understand that some SoCs turn on all the power-domains
>>> when !PM but this will not work for tegra because we don't register the
>>> power domain until later in the boot and so we are relying upon probe
>>> deferral to defer the probe of devices that use power-domains.
>>
>> So what you are saying is that adding the PM domain support, will fix
>> some devices to become successfully probed as they were broken before?
>
> Not exactly. There is a legacy tegra_powergate_sequence_power_up() that
> has been used to date to get around this. However, by migrating to GENPD
> we really need to make PM mandatory, otherwise you could attempt to
> probe a device in a power domain that is not powered. In other words,
> you are probing blindly. I know some SoCs do this, but that does not
> seem very robust.

Thank for the clarification. I agree.

You may add my:

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe