2017-03-28 14:17:47

by Jon Hunter

[permalink] [raw]
Subject: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

The current generic PM domain framework (GenDP) only allows a single
PM domain to be associated with a given device. There are several
use-cases for various system-on-chip devices where it is necessary for
a PM domain consumer to control more than one PM domain where the PM
domains:
i). Do not conform to a parent-child relationship so are not nested
ii). May not be powered on and off at the same time so need independent
control.

The solution proposed in this RFC is to allow consumers to explictly
control PM domains, by getting a handle to a PM domain and explicitly
making calls to power on and off the PM domain. Note that referencing
counting is used to ensure that a PM domain shared between consumers
is not powered off incorrectly.

The Tegra124/210 XUSB subsystem (that consists of both host and device
controllers) is an example of a consumer that needs to control more than
one PM domain because the logic is partitioned across 3 PM domains which
are:
- XUSBA: Superspeed logic (for USB 3.0)
- XUSBB: Device controller
- XUSBC: Host controller

These power domains are not nested and can be powered-up and down
independently of one another. In practice different scenarios require
different combinations of the power domains, for example:
- Superspeed host: XUSBA and XUSBC
- Superspeed device: XUSBA and XUSBB

Although it could be possible to logically nest both the XUSBB and XUSBC
domains under the XUSBA, superspeed may not always be used/required and
so this would keep it on unnecessarily.

Given that Tegra uses device-tree for describing the hardware, it would
be ideal that the device-tree 'power-domains' property for generic PM
domains could be extended to allow more than one PM domain to be
specified. For example, define the following the Tegra210 xHCI device ...

usb@70090000 {
compatible = "nvidia,tegra210-xusb";
...
power-domains = <&pd_xusbhost>, <&pd_xusbss>;
power-domain-names = "host", "superspeed";
};

This RFC extends the generic PM domain framework to allow a device to
define more than one PM domain in the device-tree 'power-domains'
property. If there is more than one then the assumption is that these
PM domains will be controlled explicitly by the consumer and the device
will not be automatically bound to any PM domain.

This RFC is a follow-up to the following RFC but because it is a
completely different approach has not been titled V2.

https://lkml.org/lkml/2016/9/20/173

Jon Hunter (4):
PM / Domains: Prepare for supporting explicit PM domain control
PM / Domains: Add support for explicit control of PM domains
PM / Domains: Add OF helpers for getting PM domains
dt-bindings: Add support for devices with multiple PM domains

.../devicetree/bindings/power/power_domain.txt | 11 +-
drivers/base/power/domain.c | 203 ++++++++++++++++++++-
include/linux/pm_domain.h | 35 ++++
3 files changed, 246 insertions(+), 3 deletions(-)

--
2.7.4


2017-03-28 14:15:51

by Jon Hunter

[permalink] [raw]
Subject: [RFC PATCH 3/4] PM / Domains: Add OF helpers for getting PM domains

Add helper functions for getting PM domains via device-tree that are to
be controlled explicitly via the pm_genpd_poweron/off() APIs. PM domains
can be retrieved by either index or name. Retrieving a PM domain by name
requires that the 'power-domain-names' property is present for the
consumer.

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

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 4980ec157750..77516b2b3e58 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2285,6 +2285,78 @@ int of_genpd_parse_idle_states(struct device_node *dn,
return 0;
}
EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
+
+static struct generic_pm_domain *genpd_get(struct device_node *np, int index)
+{
+ struct of_phandle_args genpdspec;
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ if (index < 0)
+ return ERR_PTR(-EINVAL);
+
+ ret = of_parse_phandle_with_args(np, "power-domains",
+ "#power-domain-cells", index,
+ &genpdspec);
+ if (ret)
+ return ERR_PTR(ret);
+
+ mutex_lock(&gpd_list_lock);
+
+ genpd = genpd_get_from_provider(&genpdspec);
+ of_node_put(genpdspec.np);
+
+ if (!IS_ERR(genpd)) {
+ genpd_lock(genpd);
+ genpd->device_count++;
+ genpd->suspended_count++;
+ genpd_unlock(genpd);
+ }
+
+ mutex_unlock(&gpd_list_lock);
+
+ return genpd;
+}
+
+/**
+ * of_genpd_get() - Get a PM domain by index using a device node
+ * @np: pointer to PM domain consumer node
+ * @index: index reference for a PM domain in the consumer node
+ *
+ * This function parses the 'power-domains' property using the index
+ * provided to look up a PM domain from the registered list of PM domain
+ * providers.
+ */
+struct generic_pm_domain *of_genpd_get(struct device_node *np, int index)
+{
+ return genpd_get(np, index);
+}
+EXPORT_SYMBOL(of_genpd_get);
+
+/**
+ * of_genpd_get_by_name() - Get a PM domain by name using a device node
+ * @np: pointer to PM domain consumer node
+ * @name: name reference for a PM domain in the consumer node
+ *
+ * This function parses the 'power-domains' and 'power-domain-names'
+ * properties, and uses them to look up a PM domain from the registered
+ * list of PM domain providers.
+ */
+struct generic_pm_domain *of_genpd_get_by_name(struct device_node *np,
+ const char *name)
+{
+ int index;
+
+ if (!np || !name)
+ return ERR_PTR(-EINVAL);
+
+ index = of_property_match_string(np, "power-domain-names", name);
+ if (index < 0)
+ return ERR_PTR(index);
+
+ return genpd_get(np, index);
+}
+EXPORT_SYMBOL(of_genpd_get_by_name);
#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */


diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index b3aa1f237d96..d0183d96a1b3 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -240,6 +240,10 @@ extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
extern int of_genpd_parse_idle_states(struct device_node *dn,
struct genpd_power_state **states, int *n);
+extern struct generic_pm_domain *of_genpd_get(struct device_node *np,
+ int index);
+extern struct generic_pm_domain *of_genpd_get_by_name(struct device_node *np,
+ const char *name);

int genpd_dev_pm_attach(struct device *dev);
#else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
@@ -285,6 +289,19 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
{
return ERR_PTR(-ENOTSUPP);
}
+
+static inline
+struct generic_pm_domain *of_genpd_get(struct device_node *np, int index)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
+static inline
+struct generic_pm_domain *of_genpd_get_by_name(struct device_node *np,
+ const char *name)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */

#ifdef CONFIG_PM
--
2.7.4

2017-03-28 14:16:02

by Jon Hunter

[permalink] [raw]
Subject: [RFC PATCH 1/4] PM / Domains: Prepare for supporting explicit PM domain control

The generic PM domain framework only supports consumers that require a
single PM domain. In order to extend the framework so that consumers can
explicitly control more than one PM domain, detect if the consumers
specifies more than one PM domain and if it does then don't
automatically bind the device with any of the PM domains and (leap of
faith!) assume the consumer knows what to do!

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/base/power/domain.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e697dec9d25b..0eb75954c087 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2011,6 +2011,16 @@ int genpd_dev_pm_attach(struct device *dev)
if (dev->pm_domain)
return -EEXIST;

+ /*
+ * If there are more than one PM domain defined for a device,
+ * then these need to be manually controlled by the driver
+ * that device, because the genpd core cannot bind a device
+ * with more than one PM domain.
+ */
+ if (of_count_phandle_with_args(dev->of_node, "power-domains",
+ "#power-domain-cells") > 1)
+ return 0;
+
ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
"#power-domain-cells", 0, &pd_args);
if (ret < 0) {
--
2.7.4

2017-03-28 14:18:09

by Jon Hunter

[permalink] [raw]
Subject: [RFC PATCH 2/4] PM / Domains: Add support for explicit control of PM domains

The current generic PM domain framework (GenDP) only allows a single
PM domain to be associated with a given device. There are several
use-cases for various system-on-chip devices where it is necessary for
a PM domain consumer to control more than one PM domain where the PM
domains:
i). Do not conform to a parent-child relationship so are not nested
ii). May not be powered on and off at the same time so need independent
control.

To support the above, add new APIs for GenPD to allow consumers to get,
power-on, power-off and put PM domains so that they can be explicitly
controlled by the consumer.

These new APIs for powering on and off the PM domains, call into the
existing internal functions, genpd_sync_power_on/off(), to power them
on and off. To ensure that PM domains that are both controlled
explicitly (via these new APIs) and implicitly (via runtime-pm
callbacks) do not conflict, the PM domain 'device_count' and
'suspended_count' counters are used to ensure the PM domain is in the
correct state.

For PM domains that are controlled explicitly, the debugfs 'summary'
node for GenPD will display an 'unknown (X)' under the 'device' column
to indicate that 'X' unknown device(s) are controlling the PM domain.

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

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 0eb75954c087..4980ec157750 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -303,6 +303,9 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
if (atomic_read(&genpd->sd_count) > 0)
return -EBUSY;

+ if (genpd->device_count > genpd->suspended_count)
+ return -EBUSY;
+
list_for_each_entry(pdd, &genpd->dev_list, list_node) {
enum pm_qos_flags_status stat;

@@ -1568,6 +1571,117 @@ int pm_genpd_remove(struct generic_pm_domain *genpd)
}
EXPORT_SYMBOL_GPL(pm_genpd_remove);

+/**
+ * pm_genpd_get - Get a generic I/O PM domain by name
+ * @name: Name of the PM domain.
+ *
+ * Look-ups a PM domain by name. If found, increment the device
+ * count for PM domain to ensure that the PM domain cannot be
+ * removed, increment the suspended count so that it can still
+ * be turned off (when not in-use) and return a pointer to its
+ * generic_pm_domain structure. If not found return ERR_PTR().
+ */
+struct generic_pm_domain *pm_genpd_get(const char *name)
+{
+ struct generic_pm_domain *gpd, *genpd = ERR_PTR(-EEXIST);
+
+ if (!name)
+ return ERR_PTR(-EINVAL);
+
+ mutex_lock(&gpd_list_lock);
+ list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
+ if (!strcmp(gpd->name, name)) {
+ genpd_lock(gpd);
+ gpd->device_count++;
+ gpd->suspended_count++;
+ genpd_unlock(gpd);
+ genpd = gpd;
+ break;
+ }
+ }
+ mutex_unlock(&gpd_list_lock);
+
+ return genpd;
+}
+EXPORT_SYMBOL(pm_genpd_get);
+
+/**
+ * pm_genpd_put - Put a generic I/O PM domain
+ * @genpd: Pointer to a PM domain.
+ */
+void pm_genpd_put(struct generic_pm_domain *genpd)
+{
+ if (!genpd)
+ return;
+
+ genpd_lock(genpd);
+
+ if (WARN_ON(!genpd->device_count || !genpd->suspended_count))
+ goto out;
+
+ genpd->suspended_count--;
+ genpd->device_count--;
+
+out:
+ genpd_unlock(genpd);
+}
+EXPORT_SYMBOL(pm_genpd_put);
+
+/**
+ * pm_genpd_poweron - Power on a generic I/O PM domain
+ * @genpd: Pointer to a PM domain.
+ *
+ * Powers on a PM domain, if not already on, and decrements the
+ * 'suspended_count' to prevent the PM domain from being powered off.
+ */
+int pm_genpd_poweron(struct generic_pm_domain *genpd)
+{
+ if (!genpd)
+ return -EINVAL;
+
+ genpd_lock(genpd);
+
+ if (WARN_ON(!genpd->suspended_count))
+ goto out;
+
+ genpd->suspended_count--;
+ genpd_sync_power_on(genpd, true, 0);
+
+out:
+ genpd_unlock(genpd);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_poweron);
+
+/**
+ * pm_genpd_poweroff - Power off a generic I/O PM domain
+ * @genpd: Pointer to a PM domain.
+ *
+ * Increments the 'suspended_count' for a PM domain and if the
+ * 'suspended_count' equals the 'device_count' then will power
+ * off the PM domain.
+ */
+int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+{
+ if (!genpd)
+ return -EINVAL;
+
+ genpd_lock(genpd);
+
+ if (WARN_ON(genpd->suspended_count >= genpd->device_count))
+ goto out;
+
+ genpd->suspended_count++;
+ genpd_sync_power_off(genpd, false, 0);
+
+out:
+ genpd_unlock(genpd);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_poweroff);
+
#ifdef CONFIG_PM_GENERIC_DOMAINS_OF

typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
@@ -2171,7 +2285,6 @@ int of_genpd_parse_idle_states(struct device_node *dn,
return 0;
}
EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
-
#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */


@@ -2223,7 +2336,7 @@ static int pm_genpd_summary_one(struct seq_file *s,
const char *kobj_path;
struct gpd_link *link;
char state[16];
- int ret;
+ int ret, count;

ret = genpd_lock_interruptible(genpd);
if (ret)
@@ -2250,6 +2363,8 @@ static int pm_genpd_summary_one(struct seq_file *s,
seq_puts(s, ", ");
}

+ count = genpd->device_count;
+
list_for_each_entry(pm_data, &genpd->dev_list, list_node) {
kobj_path = kobject_get_path(&pm_data->dev->kobj,
genpd_is_irq_safe(genpd) ?
@@ -2260,8 +2375,12 @@ static int pm_genpd_summary_one(struct seq_file *s,
seq_printf(s, "\n %-50s ", kobj_path);
rtpm_status_str(s, pm_data->dev);
kfree(kobj_path);
+ count--;
}

+ if (count > 0)
+ seq_printf(s, "\n unknown (%d)", count);
+
seq_puts(s, "\n");
exit:
genpd_unlock(genpd);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 5339ed5bd6f9..b3aa1f237d96 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -143,6 +143,10 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
extern int 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 generic_pm_domain *pm_genpd_get(const char *name);
+extern void pm_genpd_put(struct generic_pm_domain *genpd);
+extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
+extern int pm_genpd_poweroff(struct generic_pm_domain *genpd);

extern struct dev_power_governor simple_qos_governor;
extern struct dev_power_governor pm_domain_always_on_gov;
@@ -182,6 +186,20 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
{
return -ENOTSUPP;
}
+static inline struct generic_pm_domain *pm_genpd_get(const char *name)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
+static inline void pm_genpd_put(struct generic_pm_domain *genpd) {}
+static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
+{
+ return -ENOTSUPP;
+}
+static inline int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+{
+ return -ENOTSUPP;
+}

#define simple_qos_governor (*(struct dev_power_governor *)(NULL))
#define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
--
2.7.4

2017-03-28 14:18:07

by Jon Hunter

[permalink] [raw]
Subject: [RFC PATCH 4/4] dt-bindings: Add support for devices with multiple PM domains

Now that the generic PM domain framework supports consumers that can
control multiple PM domains, update the device-tree binding for generic
PM domains to state that one or more PM domain is permitted for a
device.

Signed-off-by: Jon Hunter <[email protected]>
---
Documentation/devicetree/bindings/power/power_domain.txt | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 723e1ad937da..fb28d37f9e1f 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -20,8 +20,15 @@ Required properties:
as specified by device tree binding documentation of particular provider.

Optional properties:
- - power-domains : A phandle and PM domain specifier as defined by bindings of
- the power controller specified by phandle.
+ - power-domains : An array of one or more PM domain specifiers (defined by the
+ bindings of the PM domain provider) for each PM domain that
+ is required by the device.
+ - power-domain-names: A list of strings of PM domain names. The list must have
+ a name for each PM domain specifier in the
+ 'power-domains' property and these names must be unique
+ within the context of this property. The names must be
+ indexed so that the first name corresponds to the first
+ PM domain specifier and so on.
Some power domains might be powered from another power domain (or have
other hardware specific dependencies). For representing such dependency
a standard PM domain consumer binding is used. When provided, all domains
--
2.7.4

2017-04-10 04:09:26

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] PM / Domains: Add support for explicit control of PM domains

Hey Jon,

On 03/28/2017 07:44 PM, Jon Hunter wrote:
> The current generic PM domain framework (GenDP) only allows a single
> PM domain to be associated with a given device. There are several
> use-cases for various system-on-chip devices where it is necessary for
> a PM domain consumer to control more than one PM domain where the PM
> domains:
> i). Do not conform to a parent-child relationship so are not nested
> ii). May not be powered on and off at the same time so need independent
> control.
>
> To support the above, add new APIs for GenPD to allow consumers to get,
> power-on, power-off and put PM domains so that they can be explicitly
> controlled by the consumer.

thanks for working on this RFC.

[]..

> +/**
> + * pm_genpd_get - Get a generic I/O PM domain by name
> + * @name: Name of the PM domain.
> + *
> + * Look-ups a PM domain by name. If found, increment the device
> + * count for PM domain to ensure that the PM domain cannot be
> + * removed, increment the suspended count so that it can still
> + * be turned off (when not in-use) and return a pointer to its
> + * generic_pm_domain structure. If not found return ERR_PTR().
> + */
> +struct generic_pm_domain *pm_genpd_get(const char *name)
> +{
> + struct generic_pm_domain *gpd, *genpd = ERR_PTR(-EEXIST);
> +
> + if (!name)
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(&gpd_list_lock);
> + list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
> + if (!strcmp(gpd->name, name)) {
> + genpd_lock(gpd);
> + gpd->device_count++;

There apis' should also take a device pointer as a parameter,
so we can track all the devices belonging to a powerdomain.
That would also mean keeping the genpd->dev_list updated instead of
only incrementing the device_count here.

> + gpd->suspended_count++;
> + genpd_unlock(gpd);
> + genpd = gpd;
> + break;
> + }
> + }
> + mutex_unlock(&gpd_list_lock);
> +
> + return genpd;

Instead of returning a pointer to generic_pm_domain to all
consumers (who are then free to poke around it) we should hide
all internal structures handled by the framework and only expose
some kind of a handle to all the consumers.
That would also mean having a clear split of the headers to
distinguish between what's accessible to consumers vs providers.

regards
Rajendra

> +}
> +EXPORT_SYMBOL(pm_genpd_get);
> +
> +/**
> + * pm_genpd_put - Put a generic I/O PM domain
> + * @genpd: Pointer to a PM domain.
> + */
> +void pm_genpd_put(struct generic_pm_domain *genpd)
> +{
> + if (!genpd)
> + return;
> +
> + genpd_lock(genpd);
> +
> + if (WARN_ON(!genpd->device_count || !genpd->suspended_count))
> + goto out;
> +
> + genpd->suspended_count--;
> + genpd->device_count--;
> +
> +out:
> + genpd_unlock(genpd);
> +}
> +EXPORT_SYMBOL(pm_genpd_put);
> +
> +/**
> + * pm_genpd_poweron - Power on a generic I/O PM domain
> + * @genpd: Pointer to a PM domain.
> + *
> + * Powers on a PM domain, if not already on, and decrements the
> + * 'suspended_count' to prevent the PM domain from being powered off.
> + */
> +int pm_genpd_poweron(struct generic_pm_domain *genpd)
> +{
> + if (!genpd)
> + return -EINVAL;
> +
> + genpd_lock(genpd);
> +
> + if (WARN_ON(!genpd->suspended_count))
> + goto out;
> +
> + genpd->suspended_count--;
> + genpd_sync_power_on(genpd, true, 0);
> +
> +out:
> + genpd_unlock(genpd);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_poweron);
> +
> +/**
> + * pm_genpd_poweroff - Power off a generic I/O PM domain
> + * @genpd: Pointer to a PM domain.
> + *
> + * Increments the 'suspended_count' for a PM domain and if the
> + * 'suspended_count' equals the 'device_count' then will power
> + * off the PM domain.
> + */
> +int pm_genpd_poweroff(struct generic_pm_domain *genpd)
> +{
> + if (!genpd)
> + return -EINVAL;
> +
> + genpd_lock(genpd);
> +
> + if (WARN_ON(genpd->suspended_count >= genpd->device_count))
> + goto out;
> +
> + genpd->suspended_count++;
> + genpd_sync_power_off(genpd, false, 0);
> +
> +out:
> + genpd_unlock(genpd);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_poweroff);
> +
> #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>
> typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
> @@ -2171,7 +2285,6 @@ int of_genpd_parse_idle_states(struct device_node *dn,
> return 0;
> }
> EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
> -
> #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
>
>
> @@ -2223,7 +2336,7 @@ static int pm_genpd_summary_one(struct seq_file *s,
> const char *kobj_path;
> struct gpd_link *link;
> char state[16];
> - int ret;
> + int ret, count;
>
> ret = genpd_lock_interruptible(genpd);
> if (ret)
> @@ -2250,6 +2363,8 @@ static int pm_genpd_summary_one(struct seq_file *s,
> seq_puts(s, ", ");
> }
>
> + count = genpd->device_count;
> +
> list_for_each_entry(pm_data, &genpd->dev_list, list_node) {
> kobj_path = kobject_get_path(&pm_data->dev->kobj,
> genpd_is_irq_safe(genpd) ?
> @@ -2260,8 +2375,12 @@ static int pm_genpd_summary_one(struct seq_file *s,
> seq_printf(s, "\n %-50s ", kobj_path);
> rtpm_status_str(s, pm_data->dev);
> kfree(kobj_path);
> + count--;
> }
>
> + if (count > 0)
> + seq_printf(s, "\n unknown (%d)", count);
> +
> seq_puts(s, "\n");
> exit:
> genpd_unlock(genpd);
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 5339ed5bd6f9..b3aa1f237d96 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -143,6 +143,10 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
> extern int 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 generic_pm_domain *pm_genpd_get(const char *name);
> +extern void pm_genpd_put(struct generic_pm_domain *genpd);
> +extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
> +extern int pm_genpd_poweroff(struct generic_pm_domain *genpd);
>
> extern struct dev_power_governor simple_qos_governor;
> extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -182,6 +186,20 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
> {
> return -ENOTSUPP;
> }
> +static inline struct generic_pm_domain *pm_genpd_get(const char *name)
> +{
> + return ERR_PTR(-ENOTSUPP);
> +}
> +
> +static inline void pm_genpd_put(struct generic_pm_domain *genpd) {}
> +static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
> +{
> + return -ENOTSUPP;
> +}
> +static inline int pm_genpd_poweroff(struct generic_pm_domain *genpd)
> +{
> + return -ENOTSUPP;
> +}
>
> #define simple_qos_governor (*(struct dev_power_governor *)(NULL))
> #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2017-04-10 04:12:52

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] dt-bindings: Add support for devices with multiple PM domains



On 03/28/2017 07:44 PM, Jon Hunter wrote:
> Now that the generic PM domain framework supports consumers that can
> control multiple PM domains, update the device-tree binding for generic
> PM domains to state that one or more PM domain is permitted for a
> device.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> Documentation/devicetree/bindings/power/power_domain.txt | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 723e1ad937da..fb28d37f9e1f 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -20,8 +20,15 @@ Required properties:
> as specified by device tree binding documentation of particular provider.
>
> Optional properties:
> - - power-domains : A phandle and PM domain specifier as defined by bindings of
> - the power controller specified by phandle.
> + - power-domains : An array of one or more PM domain specifiers (defined by the
> + bindings of the PM domain provider) for each PM domain that
> + is required by the device.
> + - power-domain-names: A list of strings of PM domain names. The list must have
> + a name for each PM domain specifier in the
> + 'power-domains' property and these names must be unique
> + within the context of this property. The names must be
> + indexed so that the first name corresponds to the first
> + PM domain specifier and so on.

These bindings are for power-domain providers. We also need to update the bindings
for the consumers (look further down in the same file)

> Some power domains might be powered from another power domain (or have
> other hardware specific dependencies). For representing such dependency
> a standard PM domain consumer binding is used. When provided, all domains
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2017-04-10 08:24:23

by Jon Hunter

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] PM / Domains: Add support for explicit control of PM domains


On 10/04/17 05:09, Rajendra Nayak wrote:
> Hey Jon,
>
> On 03/28/2017 07:44 PM, Jon Hunter wrote:
>> The current generic PM domain framework (GenDP) only allows a single
>> PM domain to be associated with a given device. There are several
>> use-cases for various system-on-chip devices where it is necessary for
>> a PM domain consumer to control more than one PM domain where the PM
>> domains:
>> i). Do not conform to a parent-child relationship so are not nested
>> ii). May not be powered on and off at the same time so need independent
>> control.
>>
>> To support the above, add new APIs for GenPD to allow consumers to get,
>> power-on, power-off and put PM domains so that they can be explicitly
>> controlled by the consumer.
>
> thanks for working on this RFC.
>
> []..
>
>> +/**
>> + * pm_genpd_get - Get a generic I/O PM domain by name
>> + * @name: Name of the PM domain.
>> + *
>> + * Look-ups a PM domain by name. If found, increment the device
>> + * count for PM domain to ensure that the PM domain cannot be
>> + * removed, increment the suspended count so that it can still
>> + * be turned off (when not in-use) and return a pointer to its
>> + * generic_pm_domain structure. If not found return ERR_PTR().
>> + */
>> +struct generic_pm_domain *pm_genpd_get(const char *name)
>> +{
>> + struct generic_pm_domain *gpd, *genpd = ERR_PTR(-EEXIST);
>> +
>> + if (!name)
>> + return ERR_PTR(-EINVAL);
>> +
>> + mutex_lock(&gpd_list_lock);
>> + list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
>> + if (!strcmp(gpd->name, name)) {
>> + genpd_lock(gpd);
>> + gpd->device_count++;
>
> There apis' should also take a device pointer as a parameter,
> so we can track all the devices belonging to a powerdomain.
> That would also mean keeping the genpd->dev_list updated instead of
> only incrementing the device_count here.

I had contemplated that and I am happy to do that if that is what the
consensus wants. However, my only reservation about doing that was it
only allows devices to call the APIs, but maybe that is ok. I was trying
to keep it similar to the clk and regulator APIs.

>> + gpd->suspended_count++;
>> + genpd_unlock(gpd);
>> + genpd = gpd;
>> + break;
>> + }
>> + }
>> + mutex_unlock(&gpd_list_lock);
>> +
>> + return genpd;
>
> Instead of returning a pointer to generic_pm_domain to all
> consumers (who are then free to poke around it) we should hide
> all internal structures handled by the framework and only expose
> some kind of a handle to all the consumers.
> That would also mean having a clear split of the headers to
> distinguish between what's accessible to consumers vs providers.

OK, I will take a look at that.

Cheers
Jon

--
nvpublic

2017-04-10 08:25:59

by Jon Hunter

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] dt-bindings: Add support for devices with multiple PM domains


On 10/04/17 05:12, Rajendra Nayak wrote:
>
>
> On 03/28/2017 07:44 PM, Jon Hunter wrote:
>> Now that the generic PM domain framework supports consumers that can
>> control multiple PM domains, update the device-tree binding for generic
>> PM domains to state that one or more PM domain is permitted for a
>> device.
>>
>> Signed-off-by: Jon Hunter <[email protected]>
>> ---
>> Documentation/devicetree/bindings/power/power_domain.txt | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>> index 723e1ad937da..fb28d37f9e1f 100644
>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> @@ -20,8 +20,15 @@ Required properties:
>> as specified by device tree binding documentation of particular provider.
>>
>> Optional properties:
>> - - power-domains : A phandle and PM domain specifier as defined by bindings of
>> - the power controller specified by phandle.
>> + - power-domains : An array of one or more PM domain specifiers (defined by the
>> + bindings of the PM domain provider) for each PM domain that
>> + is required by the device.
>> + - power-domain-names: A list of strings of PM domain names. The list must have
>> + a name for each PM domain specifier in the
>> + 'power-domains' property and these names must be unique
>> + within the context of this property. The names must be
>> + indexed so that the first name corresponds to the first
>> + PM domain specifier and so on.
>
> These bindings are for power-domain providers. We also need to update the bindings
> for the consumers (look further down in the same file)

Thanks, will take a look at that.

Cheers
Jon

--
nvpublic

2017-04-10 10:02:41

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] PM / Domains: Add support for explicit control of PM domains



On 04/10/2017 01:54 PM, Jon Hunter wrote:
>
> On 10/04/17 05:09, Rajendra Nayak wrote:
>> Hey Jon,
>>
>> On 03/28/2017 07:44 PM, Jon Hunter wrote:
>>> The current generic PM domain framework (GenDP) only allows a single
>>> PM domain to be associated with a given device. There are several
>>> use-cases for various system-on-chip devices where it is necessary for
>>> a PM domain consumer to control more than one PM domain where the PM
>>> domains:
>>> i). Do not conform to a parent-child relationship so are not nested
>>> ii). May not be powered on and off at the same time so need independent
>>> control.
>>>
>>> To support the above, add new APIs for GenPD to allow consumers to get,
>>> power-on, power-off and put PM domains so that they can be explicitly
>>> controlled by the consumer.
>>
>> thanks for working on this RFC.
>>
>> []..
>>
>>> +/**
>>> + * pm_genpd_get - Get a generic I/O PM domain by name
>>> + * @name: Name of the PM domain.
>>> + *
>>> + * Look-ups a PM domain by name. If found, increment the device
>>> + * count for PM domain to ensure that the PM domain cannot be
>>> + * removed, increment the suspended count so that it can still
>>> + * be turned off (when not in-use) and return a pointer to its
>>> + * generic_pm_domain structure. If not found return ERR_PTR().
>>> + */
>>> +struct generic_pm_domain *pm_genpd_get(const char *name)
>>> +{
>>> + struct generic_pm_domain *gpd, *genpd = ERR_PTR(-EEXIST);
>>> +
>>> + if (!name)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + mutex_lock(&gpd_list_lock);
>>> + list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
>>> + if (!strcmp(gpd->name, name)) {

Also looking up the powerdomain this way means the consumers need
to know the _exact_ name with which the providers have registered
the powerdomains?

>>> + genpd_lock(gpd);
>>> + gpd->device_count++;
>>
>> There apis' should also take a device pointer as a parameter,
>> so we can track all the devices belonging to a powerdomain.
>> That would also mean keeping the genpd->dev_list updated instead of
>> only incrementing the device_count here.
>
> I had contemplated that and I am happy to do that if that is what the
> consensus wants. However, my only reservation about doing that was it
> only allows devices to call the APIs, but maybe that is ok. I was trying
> to keep it similar to the clk and regulator APIs.
>
>>> + gpd->suspended_count++;
>>> + genpd_unlock(gpd);
>>> + genpd = gpd;
>>> + break;
>>> + }
>>> + }
>>> + mutex_unlock(&gpd_list_lock);
>>> +
>>> + return genpd;
>>
>> Instead of returning a pointer to generic_pm_domain to all
>> consumers (who are then free to poke around it) we should hide
>> all internal structures handled by the framework and only expose
>> some kind of a handle to all the consumers.
>> That would also mean having a clear split of the headers to
>> distinguish between what's accessible to consumers vs providers.
>
> OK, I will take a look at that.
>
> Cheers
> Jon
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2017-04-10 19:53:25

by Jon Hunter

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] PM / Domains: Add support for explicit control of PM domains


On 10/04/17 11:02, Rajendra Nayak wrote:
>
>
> On 04/10/2017 01:54 PM, Jon Hunter wrote:
>>
>> On 10/04/17 05:09, Rajendra Nayak wrote:
>>> Hey Jon,
>>>
>>> On 03/28/2017 07:44 PM, Jon Hunter wrote:
>>>> The current generic PM domain framework (GenDP) only allows a single
>>>> PM domain to be associated with a given device. There are several
>>>> use-cases for various system-on-chip devices where it is necessary for
>>>> a PM domain consumer to control more than one PM domain where the PM
>>>> domains:
>>>> i). Do not conform to a parent-child relationship so are not nested
>>>> ii). May not be powered on and off at the same time so need independent
>>>> control.
>>>>
>>>> To support the above, add new APIs for GenPD to allow consumers to get,
>>>> power-on, power-off and put PM domains so that they can be explicitly
>>>> controlled by the consumer.
>>>
>>> thanks for working on this RFC.
>>>
>>> []..
>>>
>>>> +/**
>>>> + * pm_genpd_get - Get a generic I/O PM domain by name
>>>> + * @name: Name of the PM domain.
>>>> + *
>>>> + * Look-ups a PM domain by name. If found, increment the device
>>>> + * count for PM domain to ensure that the PM domain cannot be
>>>> + * removed, increment the suspended count so that it can still
>>>> + * be turned off (when not in-use) and return a pointer to its
>>>> + * generic_pm_domain structure. If not found return ERR_PTR().
>>>> + */
>>>> +struct generic_pm_domain *pm_genpd_get(const char *name)
>>>> +{
>>>> + struct generic_pm_domain *gpd, *genpd = ERR_PTR(-EEXIST);
>>>> +
>>>> + if (!name)
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + mutex_lock(&gpd_list_lock);
>>>> + list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
>>>> + if (!strcmp(gpd->name, name)) {
>
> Also looking up the powerdomain this way means the consumers need
> to know the _exact_ name with which the providers have registered
> the powerdomains?

Yes, this provides a means for someone not using DT to lookup a PM
domain. How else would you do it?

Patch 3/4 allows you to use DT instead, which I imagine anyone using DT
would use.

Jon

--
nvpublic

2017-04-25 11:13:55

by Jon Hunter

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains


On 28/03/17 15:13, Jon Hunter wrote:
> The current generic PM domain framework (GenDP) only allows a single
> PM domain to be associated with a given device. There are several
> use-cases for various system-on-chip devices where it is necessary for
> a PM domain consumer to control more than one PM domain where the PM
> domains:
> i). Do not conform to a parent-child relationship so are not nested
> ii). May not be powered on and off at the same time so need independent
> control.
>
> The solution proposed in this RFC is to allow consumers to explictly
> control PM domains, by getting a handle to a PM domain and explicitly
> making calls to power on and off the PM domain. Note that referencing
> counting is used to ensure that a PM domain shared between consumers
> is not powered off incorrectly.
>
> The Tegra124/210 XUSB subsystem (that consists of both host and device
> controllers) is an example of a consumer that needs to control more than
> one PM domain because the logic is partitioned across 3 PM domains which
> are:
> - XUSBA: Superspeed logic (for USB 3.0)
> - XUSBB: Device controller
> - XUSBC: Host controller
>
> These power domains are not nested and can be powered-up and down
> independently of one another. In practice different scenarios require
> different combinations of the power domains, for example:
> - Superspeed host: XUSBA and XUSBC
> - Superspeed device: XUSBA and XUSBB
>
> Although it could be possible to logically nest both the XUSBB and XUSBC
> domains under the XUSBA, superspeed may not always be used/required and
> so this would keep it on unnecessarily.
>
> Given that Tegra uses device-tree for describing the hardware, it would
> be ideal that the device-tree 'power-domains' property for generic PM
> domains could be extended to allow more than one PM domain to be
> specified. For example, define the following the Tegra210 xHCI device ...
>
> usb@70090000 {
> compatible = "nvidia,tegra210-xusb";
> ...
> power-domains = <&pd_xusbhost>, <&pd_xusbss>;
> power-domain-names = "host", "superspeed";
> };
>
> This RFC extends the generic PM domain framework to allow a device to
> define more than one PM domain in the device-tree 'power-domains'
> property. If there is more than one then the assumption is that these
> PM domains will be controlled explicitly by the consumer and the device
> will not be automatically bound to any PM domain.

Any more comments/inputs on this? I can address Rajendra's feedback, but
before I did I wanted to see if this is along the right lines or not?

Cheers
Jon

--
nvpublic

2017-04-25 19:34:55

by Ulf Hansson

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

On 25 April 2017 at 13:13, Jon Hunter <[email protected]> wrote:
>
> On 28/03/17 15:13, Jon Hunter wrote:
>> The current generic PM domain framework (GenDP) only allows a single
>> PM domain to be associated with a given device. There are several
>> use-cases for various system-on-chip devices where it is necessary for
>> a PM domain consumer to control more than one PM domain where the PM
>> domains:
>> i). Do not conform to a parent-child relationship so are not nested
>> ii). May not be powered on and off at the same time so need independent
>> control.
>>
>> The solution proposed in this RFC is to allow consumers to explictly
>> control PM domains, by getting a handle to a PM domain and explicitly
>> making calls to power on and off the PM domain. Note that referencing
>> counting is used to ensure that a PM domain shared between consumers
>> is not powered off incorrectly.
>>
>> The Tegra124/210 XUSB subsystem (that consists of both host and device
>> controllers) is an example of a consumer that needs to control more than
>> one PM domain because the logic is partitioned across 3 PM domains which
>> are:
>> - XUSBA: Superspeed logic (for USB 3.0)
>> - XUSBB: Device controller
>> - XUSBC: Host controller
>>
>> These power domains are not nested and can be powered-up and down
>> independently of one another. In practice different scenarios require
>> different combinations of the power domains, for example:
>> - Superspeed host: XUSBA and XUSBC
>> - Superspeed device: XUSBA and XUSBB
>>
>> Although it could be possible to logically nest both the XUSBB and XUSBC
>> domains under the XUSBA, superspeed may not always be used/required and
>> so this would keep it on unnecessarily.
>>
>> Given that Tegra uses device-tree for describing the hardware, it would
>> be ideal that the device-tree 'power-domains' property for generic PM
>> domains could be extended to allow more than one PM domain to be
>> specified. For example, define the following the Tegra210 xHCI device ...
>>
>> usb@70090000 {
>> compatible = "nvidia,tegra210-xusb";
>> ...
>> power-domains = <&pd_xusbhost>, <&pd_xusbss>;
>> power-domain-names = "host", "superspeed";
>> };
>>
>> This RFC extends the generic PM domain framework to allow a device to
>> define more than one PM domain in the device-tree 'power-domains'
>> property. If there is more than one then the assumption is that these
>> PM domains will be controlled explicitly by the consumer and the device
>> will not be automatically bound to any PM domain.
>
> Any more comments/inputs on this? I can address Rajendra's feedback, but
> before I did I wanted to see if this is along the right lines or not?

I discussed this with Rafael at the OSPM summit in Pisa a couple of
weeks ago. Apologize for the delay in providing additional feedback.

First, whether the problem is really rare, perhaps adding a new
API/framework can't be justified - then it may be better to add some
kind of aggregation layer on top of the current PM domain
infrastructure (something along the first attempt you made for genpd).
That was kind of Rafael's thoughts (Rafael, please correct me if I am
wrong).

However, we currently know about at least two different SoCs that need
this. Perhaps we can extend the below list to justify adding a new
framework/APIs. Something along the lines what you propose in $subject
patchset.

1) Nvidia; to solve the USB super-speed host/device problem.
2) QCOM, which has pointed to several cases where the PM topology is
laid out like devices having two PM domains..
3?) I don't fully remember - but I think Geert also pointed to some
examples where a device could reside in a clock domain but also in
power domain for a Renesas SoC!?
4) ?

Moreover, perhaps this could also be useful for SoCs using ACPI? If
so, we shouldn't tie this immediately to genpd, but in the layer above
genpd/ACPI PM domain. Something like dev_pm_domain_get(), which would
be implemented similar as the dev_pm_domain_attach() (which calls both
the ACPI PM domain and genpd to find a match).

Kind regards
Uffe

2017-04-25 21:18:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

On Tue, Apr 25, 2017 at 9:34 PM, Ulf Hansson <[email protected]> wrote:
> On 25 April 2017 at 13:13, Jon Hunter <[email protected]> wrote:
>>
>> On 28/03/17 15:13, Jon Hunter wrote:
>>> The current generic PM domain framework (GenDP) only allows a single
>>> PM domain to be associated with a given device. There are several
>>> use-cases for various system-on-chip devices where it is necessary for
>>> a PM domain consumer to control more than one PM domain where the PM
>>> domains:
>>> i). Do not conform to a parent-child relationship so are not nested
>>> ii). May not be powered on and off at the same time so need independent
>>> control.
>>>
>>> The solution proposed in this RFC is to allow consumers to explictly
>>> control PM domains, by getting a handle to a PM domain and explicitly
>>> making calls to power on and off the PM domain. Note that referencing
>>> counting is used to ensure that a PM domain shared between consumers
>>> is not powered off incorrectly.
>>>
>>> The Tegra124/210 XUSB subsystem (that consists of both host and device
>>> controllers) is an example of a consumer that needs to control more than
>>> one PM domain because the logic is partitioned across 3 PM domains which
>>> are:
>>> - XUSBA: Superspeed logic (for USB 3.0)
>>> - XUSBB: Device controller
>>> - XUSBC: Host controller
>>>
>>> These power domains are not nested and can be powered-up and down
>>> independently of one another. In practice different scenarios require
>>> different combinations of the power domains, for example:
>>> - Superspeed host: XUSBA and XUSBC
>>> - Superspeed device: XUSBA and XUSBB
>>>
>>> Although it could be possible to logically nest both the XUSBB and XUSBC
>>> domains under the XUSBA, superspeed may not always be used/required and
>>> so this would keep it on unnecessarily.
>>>
>>> Given that Tegra uses device-tree for describing the hardware, it would
>>> be ideal that the device-tree 'power-domains' property for generic PM
>>> domains could be extended to allow more than one PM domain to be
>>> specified. For example, define the following the Tegra210 xHCI device ...
>>>
>>> usb@70090000 {
>>> compatible = "nvidia,tegra210-xusb";
>>> ...
>>> power-domains = <&pd_xusbhost>, <&pd_xusbss>;
>>> power-domain-names = "host", "superspeed";
>>> };
>>>
>>> This RFC extends the generic PM domain framework to allow a device to
>>> define more than one PM domain in the device-tree 'power-domains'
>>> property. If there is more than one then the assumption is that these
>>> PM domains will be controlled explicitly by the consumer and the device
>>> will not be automatically bound to any PM domain.
>>
>> Any more comments/inputs on this? I can address Rajendra's feedback, but
>> before I did I wanted to see if this is along the right lines or not?
>
> I discussed this with Rafael at the OSPM summit in Pisa a couple of
> weeks ago. Apologize for the delay in providing additional feedback.
>
> First, whether the problem is really rare, perhaps adding a new
> API/framework can't be justified - then it may be better to add some
> kind of aggregation layer on top of the current PM domain
> infrastructure (something along the first attempt you made for genpd).
> That was kind of Rafael's thoughts (Rafael, please correct me if I am
> wrong).

We were talking about the original idea behind the pm_domain pointer
concept, which was about adding a set of PM operations above the bus
type/class layer, which could be used for intercepting bus-type PM
operations and providing some common handling above them. This is
still relevant IMO.

The basic observation here is that the PM core takes only one set of
PM operation per device into account and therefore, in every stage of
system suspend, for example, the callback invoked by it has to take
care of all actions that need to be carried out for the given device,
possibly by invoking callbacks from other code layers. That
limitation cannot be removed easily, because it is built into the PM
core design quite fundamentally.

However, this series seems to be about controlling power resources
represented by power domain objects rather than about PM operations.
In ACPI there is a power resource concept which seems to be quite
similar to this, so it is not entirely new. :-)

Of course, question is whether or not to extend genpd this way and I'm
not really sure. I actually probably wouldn't do that, because
poweron/poweroff operations used by genpd can be implemeted in terms
of lower-level power resource control and I don't see the reason for
mixing the two in one framework.

> However, we currently know about at least two different SoCs that need
> this. Perhaps we can extend the below list to justify adding a new
> framework/APIs. Something along the lines what you propose in $subject
> patchset.
>
> 1) Nvidia; to solve the USB super-speed host/device problem.
> 2) QCOM, which has pointed to several cases where the PM topology is
> laid out like devices having two PM domains..
> 3?) I don't fully remember - but I think Geert also pointed to some
> examples where a device could reside in a clock domain but also in
> power domain for a Renesas SoC!?
> 4) ?
>
> Moreover, perhaps this could also be useful for SoCs using ACPI?

No, it couldn't. At least not in general.

Thanks,
Rafael

2017-04-26 08:07:32

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

Hi Ulf,

On Tue, Apr 25, 2017 at 9:34 PM, Ulf Hansson <[email protected]> wrote:
> However, we currently know about at least two different SoCs that need
> this. Perhaps we can extend the below list to justify adding a new
> framework/APIs. Something along the lines what you propose in $subject
> patchset.
>
> 1) Nvidia; to solve the USB super-speed host/device problem.
> 2) QCOM, which has pointed to several cases where the PM topology is
> laid out like devices having two PM domains..
> 3?) I don't fully remember - but I think Geert also pointed to some
> examples where a device could reside in a clock domain but also in
> power domain for a Renesas SoC!?
> 4) ?

Most Renesas SoCs have module clocks, which we model as a clock domain.
Some Renesas SoCs have power domains for CPUs, others have them for
devices as well.
As we always provide a virtual "always-on" power domain in the power domain
controller, all devices can refer to it using "power-domains" properties,
and the driver for the power domain controller can just forward the clock
domain operations to the clock driver.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2017-04-26 09:29:01

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

Hi Ulf,

On Wed, Apr 26, 2017 at 11:04 AM, Ulf Hansson <[email protected]> wrote:
> On 26 April 2017 at 10:06, Geert Uytterhoeven <[email protected]> wrote:
>> On Tue, Apr 25, 2017 at 9:34 PM, Ulf Hansson <[email protected]> wrote:
>>> However, we currently know about at least two different SoCs that need
>>> this. Perhaps we can extend the below list to justify adding a new
>>> framework/APIs. Something along the lines what you propose in $subject
>>> patchset.
>>>
>>> 1) Nvidia; to solve the USB super-speed host/device problem.
>>> 2) QCOM, which has pointed to several cases where the PM topology is
>>> laid out like devices having two PM domains..
>>> 3?) I don't fully remember - but I think Geert also pointed to some
>>> examples where a device could reside in a clock domain but also in
>>> power domain for a Renesas SoC!?
>>> 4) ?
>>
>> Most Renesas SoCs have module clocks, which we model as a clock domain.
>> Some Renesas SoCs have power domains for CPUs, others have them for
>> devices as well.
>> As we always provide a virtual "always-on" power domain in the power domain
>> controller, all devices can refer to it using "power-domains" properties,
>> and the driver for the power domain controller can just forward the clock
>> domain operations to the clock driver.
>
> Okay, thanks for clarifying this.
>
> Thinking about this as bit more, when I realized that *if* we would
> add a new PM domain framework for explicit control of PM domains, that
> would mean you need to deploy support for that in the drivers.

Correct. And we have to update DT bindings and DTS.

> On the other hand, as you anyway would need to change the drivers, you
> could instead deploy clock support in the drivers, which would avoid
> using the clock domain. In that way, you could still stay with one PM
> domain pointer per device, used to control the power domains instead.
> Right? Or would that have other implications?

That's exactly what we're doing already.
Which means that if you allow multiple entries in power-domains, we
have to change drivers, DT bindings, and DTS again (which we may
decide not to do ;-)

On SH/R-Mobile, we always did it that way, as the user manual had an
explicit "always-on" power domain.

On R-Car Gen2, the power domains contain CPU and L2 and GPU only,
so devices had their power-domains pointing to the clock controller.

On R-Car Gen3, some devices were moved into power domains, so we
generalized this by creating a virtual "always-on" power domain, and
letting all devices point their power-domains properties to the power
domain controller, which forwards clock handling to the clock controller.
For consistency, this was applied to R-Car Gen2 as well.

Cfr. some late relics fixed in e.g. commit 24b2d930a50662c1
('ARM: dts: r8a7794: Use SYSC "always-on" PM Domain for sound'),
but technically the fix was not needed, as it worked fine without.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2017-04-26 09:24:48

by Ulf Hansson

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

On 26 April 2017 at 10:06, Geert Uytterhoeven <[email protected]> wrote:
> Hi Ulf,
>
> On Tue, Apr 25, 2017 at 9:34 PM, Ulf Hansson <[email protected]> wrote:
>> However, we currently know about at least two different SoCs that need
>> this. Perhaps we can extend the below list to justify adding a new
>> framework/APIs. Something along the lines what you propose in $subject
>> patchset.
>>
>> 1) Nvidia; to solve the USB super-speed host/device problem.
>> 2) QCOM, which has pointed to several cases where the PM topology is
>> laid out like devices having two PM domains..
>> 3?) I don't fully remember - but I think Geert also pointed to some
>> examples where a device could reside in a clock domain but also in
>> power domain for a Renesas SoC!?
>> 4) ?
>
> Most Renesas SoCs have module clocks, which we model as a clock domain.
> Some Renesas SoCs have power domains for CPUs, others have them for
> devices as well.
> As we always provide a virtual "always-on" power domain in the power domain
> controller, all devices can refer to it using "power-domains" properties,
> and the driver for the power domain controller can just forward the clock
> domain operations to the clock driver.

Okay, thanks for clarifying this.

Thinking about this as bit more, when I realized that *if* we would
add a new PM domain framework for explicit control of PM domains, that
would mean you need to deploy support for that in the drivers.

On the other hand, as you anyway would need to change the drivers, you
could instead deploy clock support in the drivers, which would avoid
using the clock domain. In that way, you could still stay with one PM
domain pointer per device, used to control the power domains instead.
Right? Or would that have other implications?

Kind regards
Uffe

2017-04-26 09:56:02

by Ulf Hansson

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

On 26 April 2017 at 11:17, Geert Uytterhoeven <[email protected]> wrote:
> Hi Ulf,
>
> On Wed, Apr 26, 2017 at 11:04 AM, Ulf Hansson <[email protected]> wrote:
>> On 26 April 2017 at 10:06, Geert Uytterhoeven <[email protected]> wrote:
>>> On Tue, Apr 25, 2017 at 9:34 PM, Ulf Hansson <[email protected]> wrote:
>>>> However, we currently know about at least two different SoCs that need
>>>> this. Perhaps we can extend the below list to justify adding a new
>>>> framework/APIs. Something along the lines what you propose in $subject
>>>> patchset.
>>>>
>>>> 1) Nvidia; to solve the USB super-speed host/device problem.
>>>> 2) QCOM, which has pointed to several cases where the PM topology is
>>>> laid out like devices having two PM domains..
>>>> 3?) I don't fully remember - but I think Geert also pointed to some
>>>> examples where a device could reside in a clock domain but also in
>>>> power domain for a Renesas SoC!?
>>>> 4) ?
>>>
>>> Most Renesas SoCs have module clocks, which we model as a clock domain.
>>> Some Renesas SoCs have power domains for CPUs, others have them for
>>> devices as well.
>>> As we always provide a virtual "always-on" power domain in the power domain
>>> controller, all devices can refer to it using "power-domains" properties,
>>> and the driver for the power domain controller can just forward the clock
>>> domain operations to the clock driver.
>>
>> Okay, thanks for clarifying this.
>>
>> Thinking about this as bit more, when I realized that *if* we would
>> add a new PM domain framework for explicit control of PM domains, that
>> would mean you need to deploy support for that in the drivers.
>
> Correct. And we have to update DT bindings and DTS.
>
>> On the other hand, as you anyway would need to change the drivers, you
>> could instead deploy clock support in the drivers, which would avoid
>> using the clock domain. In that way, you could still stay with one PM
>> domain pointer per device, used to control the power domains instead.
>> Right? Or would that have other implications?
>
> That's exactly what we're doing already.

No really, but perhaps I was not clear enough.

Currently you deploy only runtime PM support in the driver and don't
do any clk_get() etc. Then you have a PM domain (genpd) attached to
the device and makes use of genpd's device specific callbacks, in
struct gpd_dev_ops ->start|stop(), which allows you to control clocks
for each device. Of course this is perfectly okay.

So then my question is/was; does there exist cases when these devices
(already attached to a PM domain) would needed to be attach to yet
another separate PM domain? From the nicely detailed description
below, I find the answer to be *no*!?

> Which means that if you allow multiple entries in power-domains, we
> have to change drivers, DT bindings, and DTS again (which we may
> decide not to do ;-)
>
> On SH/R-Mobile, we always did it that way, as the user manual had an
> explicit "always-on" power domain.
>
> On R-Car Gen2, the power domains contain CPU and L2 and GPU only,
> so devices had their power-domains pointing to the clock controller.
>
> On R-Car Gen3, some devices were moved into power domains, so we
> generalized this by creating a virtual "always-on" power domain, and
> letting all devices point their power-domains properties to the power
> domain controller, which forwards clock handling to the clock controller.
> For consistency, this was applied to R-Car Gen2 as well.
>
> Cfr. some late relics fixed in e.g. commit 24b2d930a50662c1
> ('ARM: dts: r8a7794: Use SYSC "always-on" PM Domain for sound'),
> but technically the fix was not needed, as it worked fine without.

Thanks for the detailed summary!

Kind regards
Uffe