* Re: [PATCH v3 4/5] PM / Domains: Add support for multi PM domains per device to genpd
From: Lucas Stach @ 2018-05-31 11:40 UTC (permalink / raw)
To: Ulf Hansson, Rafael J . Wysocki, linux-pm
Cc: Greg Kroah-Hartman, Jon Hunter, Geert Uytterhoeven, Todor Tomov,
Rajendra Nayak, Viresh Kumar, Vincent Guittot, Kevin Hilman,
linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180531105959.14843-5-ulf.hansson@linaro.org>
Hi Ulf,
Am Donnerstag, den 31.05.2018, 12:59 +0200 schrieb Ulf Hansson:
> To support devices being partitioned across multiple PM domains, let's
> begin with extending genpd to cope with these kind of configurations.
>
> Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
> is similar to the existing genpd_dev_pm_attach(), but with the difference
> that it allows its callers to provide an index to the PM domain that it
> wants to attach.
>
> Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
> core / PM core, similar to how the existing dev_pm_domain_attach() makes
> use of genpd_dev_pm_attach(). However, this is implemented by following
> changes on top.
by_id() APIs are not really intuitive to use for driver writers. Other
subsystems have solved this by providing a "-names" property to give
the phandles a bit more meaning and then providing a by_name API. I
would really appreciate if PM domains could move in the same direction.
Regards,
Lucas
> Because, only one PM domain can be attached per device, genpd needs to
> create a virtual device that it can attach/detach instead. More precisely,
> let the new function genpd_dev_pm_attach_by_id() register a virtual struct
> device via calling device_register(). Then let it attach this device to the
> corresponding PM domain, rather than the one that is provided by the
> caller. The actual attaching is done via re-using the existing genpd OF
> functions.
>
> At successful attachment, genpd_dev_pm_attach_by_id() returns the created
> virtual device, which allows the caller to operate on it to deal with power
> management. Following changes on top, provides more details in this
> regards.
>
> To deal with detaching of a PM domain for the multiple PM domains case,
> let's also extend the existing genpd_dev_pm_detach() function, to cover the
> cleanup of the created virtual device, via make it call device_unregister()
> on it. In this way, there is no need to introduce a new function to deal
> with detach for the multiple PM domain case, but instead the existing one
> is re-used.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 8 ++++
> 2 files changed, 88 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b1fcbf917974..4925af5c4cf0 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
> }
> EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>
> +static void genpd_release_dev(struct device *dev)
> +{
> + kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> + .name = "genpd",
> +};
> +
> /**
> * genpd_dev_pm_detach - Detach a device from its PM domain.
> * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>
> /* Check if PM domain can be powered off after removing this device. */
> genpd_queue_power_off_work(pd);
> +
> + /* Unregister the device if it was created by genpd. */
> + if (dev->bus == &genpd_bus_type)
> + device_unregister(dev);
> }
>
> static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> +/**
> + * genpd_dev_pm_attach_by_id - Associate a device with one of its PM domains.
> + * @dev: The device used to lookup the PM domain.
> + * @index: The index of the PM domain.
> + *
> + * Parse device's OF node to find a PM domain specifier at the provided @index.
> + * If such is found, creates a virtual device and attaches it to the retrieved
> + * pm_domain ops. To deal with detaching of the virtual device, the ->detach()
> + * callback in the struct dev_pm_domain are assigned to genpd_dev_pm_detach().
> + *
> + * Returns the created virtual device if successfully attached PM domain, NULL
> + * when the device don't need a PM domain, else an ERR_PTR() in case of
> + * failures. If a power-domain exists for the device, but cannot be found or
> + * turned on, then ERR_PTR(-EPROBE_DEFER) is returned to ensure that the device
> + * is not probed and to re-try again later.
> + */
> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
> + unsigned int index)
> +{
> + struct device *genpd_dev;
> + int num_domains;
> + int ret;
> +
> + if (!dev->of_node)
> + return NULL;
> +
> + /* Deal only with devices using multiple PM domains. */
> + num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
> + "#power-domain-cells");
> + if (num_domains < 2 || index >= num_domains)
> + return NULL;
> +
> + /* Allocate and register device on the genpd bus. */
> + genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
> + if (!genpd_dev)
> + return ERR_PTR(-ENOMEM);
> +
> + dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
> + genpd_dev->bus = &genpd_bus_type;
> + genpd_dev->release = genpd_release_dev;
> +
> + ret = device_register(genpd_dev);
> + if (ret) {
> + kfree(genpd_dev);
> + return ERR_PTR(ret);
> + }
> +
> + /* Try to attach the device to the PM domain at the specified index. */
> + ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
> + if (ret < 1) {
> + device_unregister(genpd_dev);
> + return ret ? ERR_PTR(ret) : NULL;
> + }
> +
> + pm_runtime_set_active(genpd_dev);
> + pm_runtime_enable(genpd_dev);
> +
> + return genpd_dev;
> +}
> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
> +
> static const struct of_device_id idle_state_match[] = {
> { .compatible = "domain-idle-state", },
> { }
> @@ -2457,6 +2531,12 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(of_genpd_opp_to_performance_state);
>
> +static int __init genpd_bus_init(void)
> +{
> + return bus_register(&genpd_bus_type);
> +}
> +core_initcall(genpd_bus_init);
> +
> #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
>
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 42e0d649e653..82458e8e2e01 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -237,6 +237,8 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
> struct device_node *opp_node);
>
> int genpd_dev_pm_attach(struct device *dev);
> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
> + unsigned int index);
> #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
> static inline int of_genpd_add_provider_simple(struct device_node *np,
> struct generic_pm_domain *genpd)
> @@ -282,6 +284,12 @@ static inline int genpd_dev_pm_attach(struct device *dev)
> return 0;
> }
>
> +static inline struct device *genpd_dev_pm_attach_by_id(struct device *dev,
> + unsigned int index)
> +{
> + return NULL;
> +}
> +
> static inline
> struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
> {
^ permalink raw reply
* [PATCH v2 10/21] cpufreq: intel_pstate: use match_string() helper
From: Yisheng Xie @ 2018-05-31 11:11 UTC (permalink / raw)
To: linux-kernel
Cc: andy.shevchenko, Yisheng Xie, Srinivas Pandruvada, Len Brown,
Rafael J. Wysocki, Viresh Kumar, linux-pm
In-Reply-To: <1527765086-19873-1-git-send-email-xieyisheng1@huawei.com>
match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
- add Reviewed-by tag.
drivers/cpufreq/intel_pstate.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 17e566af..d701e26 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -645,21 +645,18 @@ static ssize_t store_energy_performance_preference(
{
struct cpudata *cpu_data = all_cpu_data[policy->cpu];
char str_preference[21];
- int ret, i = 0;
+ int ret;
ret = sscanf(buf, "%20s", str_preference);
if (ret != 1)
return -EINVAL;
- while (energy_perf_strings[i] != NULL) {
- if (!strcmp(str_preference, energy_perf_strings[i])) {
- intel_pstate_set_energy_pref_index(cpu_data, i);
- return count;
- }
- ++i;
- }
+ ret = match_string(energy_perf_strings, -1, str_preference);
+ if (ret < 0)
+ return ret;
- return -EINVAL;
+ intel_pstate_set_energy_pref_index(cpu_data, ret);
+ return count;
}
static ssize_t show_energy_performance_preference(
--
1.7.12.4
^ permalink raw reply related
* [PATCH v3 5/5] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains
From: Ulf Hansson @ 2018-05-31 10:59 UTC (permalink / raw)
To: Rafael J . Wysocki, linux-pm
Cc: Ulf Hansson, Greg Kroah-Hartman, Jon Hunter, Geert Uytterhoeven,
Todor Tomov, Rajendra Nayak, Viresh Kumar, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180531105959.14843-1-ulf.hansson@linaro.org>
The existing dev_pm_domain_attach() function, allows a single PM domain to
be attached per device. To be able to support devices that are partitioned
across multiple PM domains, let's introduce a new interface,
dev_pm_domain_attach_by_id().
The dev_pm_domain_attach_by_id() returns a new allocated struct device with
the corresponding attached PM domain. This enables for example a driver to
operate on the new device from a power management point of view. The driver
may then also benefit from using the received device, to set up so called
device-links towards its original device. Depending on the situation, these
links may then be dynamically changed.
The new interface is typically called by drivers during their probe phase,
in case they manages devices which uses multiple PM domains. If that is the
case, the driver also becomes responsible of managing the detaching of the
PM domains, which typically should be done at the remove phase. Detaching
is done by calling the existing dev_pm_domain_detach() function and for
each of the received devices from dev_pm_domain_attach_by_id().
Note, currently its only genpd that supports multiple PM domains per
device, but dev_pm_domain_attach_by_id() can easily by extended to cover
other PM domain types, if/when needed.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
include/linux/pm_domain.h | 7 ++++++
2 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index 7ae62b6355b8..df41b4780b3b 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
}
EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
+/**
+ * dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
+ * @dev: The device used to lookup the PM domain.
+ * @index: The index of the PM domain.
+ *
+ * As @dev may only be attached to a single PM domain, the backend PM domain
+ * provider creates a virtual device to attach instead. If attachment succeeds,
+ * the ->detach() callback in the struct dev_pm_domain are assigned by the
+ * corresponding backend attach function, as to deal with detaching of the
+ * created virtual device.
+ *
+ * This function should typically be invoked by a driver during the probe phase,
+ * in case its device requires power management through multiple PM domains. The
+ * driver may benefit from using the received device, to configure device-links
+ * towards its original device. Depending on the use-case and if needed, the
+ * links may be dynamically changed by the driver, which allows it to control
+ * the power to the PM domains independently from each other.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ *
+ * Returns the virtual created device when successfully attached to its PM
+ * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
+ * Note that, to detach the returned virtual device, the driver shall call
+ * dev_pm_domain_detach() on it, typically during the remove phase.
+ */
+struct device *dev_pm_domain_attach_by_id(struct device *dev,
+ unsigned int index)
+{
+ if (dev->pm_domain)
+ return ERR_PTR(-EEXIST);
+
+ return genpd_dev_pm_attach_by_id(dev, index);
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
+
/**
* dev_pm_domain_detach - Detach a device from its PM domain.
* @dev: Device to detach.
* @power_off: Used to indicate whether we should power off the device.
*
- * This functions will reverse the actions from dev_pm_domain_attach() and thus
- * try to detach the @dev from its PM domain. Typically it should be invoked
- * from subsystem level code during the remove phase.
+ * This functions will reverse the actions from dev_pm_domain_attach() and
+ * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
+ * Typically it should be invoked during the remove phase, either from
+ * subsystem level code or from drivers.
*
* Callers must ensure proper synchronization of this function with power
* management callbacks.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 82458e8e2e01..9206a4fef9ac 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
#ifdef CONFIG_PM
int dev_pm_domain_attach(struct device *dev, bool power_on);
+struct device *dev_pm_domain_attach_by_id(struct device *dev,
+ unsigned int index);
void dev_pm_domain_detach(struct device *dev, bool power_off);
void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
#else
@@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
{
return 0;
}
+static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
+ unsigned int index)
+{
+ return NULL;
+}
static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
static inline void dev_pm_domain_set(struct device *dev,
struct dev_pm_domain *pd) {}
--
2.17.0
^ permalink raw reply related
* [PATCH v3 4/5] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-31 10:59 UTC (permalink / raw)
To: Rafael J . Wysocki, linux-pm
Cc: Ulf Hansson, Rajendra Nayak, Geert Uytterhoeven, Kevin Hilman,
Greg Kroah-Hartman, linux-kernel, Jon Hunter, Todor Tomov,
Viresh Kumar, linux-tegra, Vincent Guittot, linux-arm-kernel
In-Reply-To: <20180531105959.14843-1-ulf.hansson@linaro.org>
To support devices being partitioned across multiple PM domains, let's
begin with extending genpd to cope with these kind of configurations.
Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
is similar to the existing genpd_dev_pm_attach(), but with the difference
that it allows its callers to provide an index to the PM domain that it
wants to attach.
Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
core / PM core, similar to how the existing dev_pm_domain_attach() makes
use of genpd_dev_pm_attach(). However, this is implemented by following
changes on top.
Because, only one PM domain can be attached per device, genpd needs to
create a virtual device that it can attach/detach instead. More precisely,
let the new function genpd_dev_pm_attach_by_id() register a virtual struct
device via calling device_register(). Then let it attach this device to the
corresponding PM domain, rather than the one that is provided by the
caller. The actual attaching is done via re-using the existing genpd OF
functions.
At successful attachment, genpd_dev_pm_attach_by_id() returns the created
virtual device, which allows the caller to operate on it to deal with power
management. Following changes on top, provides more details in this
regards.
To deal with detaching of a PM domain for the multiple PM domains case,
let's also extend the existing genpd_dev_pm_detach() function, to cover the
cleanup of the created virtual device, via make it call device_unregister()
on it. In this way, there is no need to introduce a new function to deal
with detach for the multiple PM domain case, but instead the existing one
is re-used.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 8 ++++
2 files changed, 88 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index b1fcbf917974..4925af5c4cf0 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
}
EXPORT_SYMBOL_GPL(of_genpd_remove_last);
+static void genpd_release_dev(struct device *dev)
+{
+ kfree(dev);
+}
+
+static struct bus_type genpd_bus_type = {
+ .name = "genpd",
+};
+
/**
* genpd_dev_pm_detach - Detach a device from its PM domain.
* @dev: Device to detach.
@@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
/* Check if PM domain can be powered off after removing this device. */
genpd_queue_power_off_work(pd);
+
+ /* Unregister the device if it was created by genpd. */
+ if (dev->bus == &genpd_bus_type)
+ device_unregister(dev);
}
static void genpd_dev_pm_sync(struct device *dev)
@@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
}
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
+/**
+ * genpd_dev_pm_attach_by_id - Associate a device with one of its PM domains.
+ * @dev: The device used to lookup the PM domain.
+ * @index: The index of the PM domain.
+ *
+ * Parse device's OF node to find a PM domain specifier at the provided @index.
+ * If such is found, creates a virtual device and attaches it to the retrieved
+ * pm_domain ops. To deal with detaching of the virtual device, the ->detach()
+ * callback in the struct dev_pm_domain are assigned to genpd_dev_pm_detach().
+ *
+ * Returns the created virtual device if successfully attached PM domain, NULL
+ * when the device don't need a PM domain, else an ERR_PTR() in case of
+ * failures. If a power-domain exists for the device, but cannot be found or
+ * turned on, then ERR_PTR(-EPROBE_DEFER) is returned to ensure that the device
+ * is not probed and to re-try again later.
+ */
+struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+ unsigned int index)
+{
+ struct device *genpd_dev;
+ int num_domains;
+ int ret;
+
+ if (!dev->of_node)
+ return NULL;
+
+ /* Deal only with devices using multiple PM domains. */
+ num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
+ "#power-domain-cells");
+ if (num_domains < 2 || index >= num_domains)
+ return NULL;
+
+ /* Allocate and register device on the genpd bus. */
+ genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
+ if (!genpd_dev)
+ return ERR_PTR(-ENOMEM);
+
+ dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
+ genpd_dev->bus = &genpd_bus_type;
+ genpd_dev->release = genpd_release_dev;
+
+ ret = device_register(genpd_dev);
+ if (ret) {
+ kfree(genpd_dev);
+ return ERR_PTR(ret);
+ }
+
+ /* Try to attach the device to the PM domain at the specified index. */
+ ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
+ if (ret < 1) {
+ device_unregister(genpd_dev);
+ return ret ? ERR_PTR(ret) : NULL;
+ }
+
+ pm_runtime_set_active(genpd_dev);
+ pm_runtime_enable(genpd_dev);
+
+ return genpd_dev;
+}
+EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
+
static const struct of_device_id idle_state_match[] = {
{ .compatible = "domain-idle-state", },
{ }
@@ -2457,6 +2531,12 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
}
EXPORT_SYMBOL_GPL(of_genpd_opp_to_performance_state);
+static int __init genpd_bus_init(void)
+{
+ return bus_register(&genpd_bus_type);
+}
+core_initcall(genpd_bus_init);
+
#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 42e0d649e653..82458e8e2e01 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -237,6 +237,8 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
struct device_node *opp_node);
int genpd_dev_pm_attach(struct device *dev);
+struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+ unsigned int index);
#else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
static inline int of_genpd_add_provider_simple(struct device_node *np,
struct generic_pm_domain *genpd)
@@ -282,6 +284,12 @@ static inline int genpd_dev_pm_attach(struct device *dev)
return 0;
}
+static inline struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+ unsigned int index)
+{
+ return NULL;
+}
+
static inline
struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
{
--
2.17.0
^ permalink raw reply related
* [PATCH v3 3/5] PM / Domains: Split genpd_dev_pm_attach()
From: Ulf Hansson @ 2018-05-31 10:59 UTC (permalink / raw)
To: Rafael J . Wysocki, linux-pm
Cc: Ulf Hansson, Greg Kroah-Hartman, Jon Hunter, Geert Uytterhoeven,
Todor Tomov, Rajendra Nayak, Viresh Kumar, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180531105959.14843-1-ulf.hansson@linaro.org>
To extend genpd to deal with allowing multiple PM domains per device, some
of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
by moving some of the code into a sub-function.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 908c44779ae7..b1fcbf917974 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
genpd_queue_power_off_work(pd);
}
-/**
- * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
- * @dev: Device to attach.
- *
- * Parse device's OF node to find a PM domain specifier. If such is found,
- * attaches the device to retrieved pm_domain ops.
- *
- * Returns 1 on successfully attached PM domain, 0 when the device don't need a
- * PM domain or when multiple power-domains exists for it, else a negative error
- * code. Note that if a power-domain exists for the device, but it cannot be
- * found or turned on, then return -EPROBE_DEFER to ensure that the device is
- * not probed and to re-try again later.
- */
-int genpd_dev_pm_attach(struct device *dev)
+static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
+ unsigned int index)
{
struct of_phandle_args pd_args;
struct generic_pm_domain *pd;
int ret;
- if (!dev->of_node)
- return 0;
-
- /*
- * Devices with multiple PM domains must be attached separately, as we
- * can only attach one PM domain per device.
- */
- 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);
+ ret = of_parse_phandle_with_args(np, "power-domains",
+ "#power-domain-cells", index, &pd_args);
if (ret < 0)
return ret;
@@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
return ret ? -EPROBE_DEFER : 1;
}
+
+/**
+ * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
+ * @dev: Device to attach.
+ *
+ * Parse device's OF node to find a PM domain specifier. If such is found,
+ * attaches the device to retrieved pm_domain ops.
+ *
+ * Returns 1 on successfully attached PM domain, 0 when the device don't need a
+ * PM domain or when multiple power-domains exists for it, else a negative error
+ * code. Note that if a power-domain exists for the device, but it cannot be
+ * found or turned on, then return -EPROBE_DEFER to ensure that the device is
+ * not probed and to re-try again later.
+ */
+int genpd_dev_pm_attach(struct device *dev)
+{
+ if (!dev->of_node)
+ return 0;
+
+ /*
+ * Devices with multiple PM domains must be attached separately, as we
+ * can only attach one PM domain per device.
+ */
+ if (of_count_phandle_with_args(dev->of_node, "power-domains",
+ "#power-domain-cells") != 1)
+ return 0;
+
+ return __genpd_dev_pm_attach(dev, dev->of_node, 0);
+}
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
static const struct of_device_id idle_state_match[] = {
--
2.17.0
^ permalink raw reply related
* [PATCH v3 2/5] PM / Domains: Don't attach devices in genpd with multi PM domains
From: Ulf Hansson @ 2018-05-31 10:59 UTC (permalink / raw)
To: Rafael J . Wysocki, linux-pm
Cc: Ulf Hansson, Greg Kroah-Hartman, Jon Hunter, Geert Uytterhoeven,
Todor Tomov, Rajendra Nayak, Viresh Kumar, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra,
Rob Herring, devicetree
In-Reply-To: <20180531105959.14843-1-ulf.hansson@linaro.org>
The power-domain DT property may now contain a list of PM domain
specifiers, which represents that a device are partitioned across multiple
PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
one PM domain can be attached per device.
To remain things simple for the most common configuration, when a single PM
domain is used, let's treat the multiple PM domain case as being specific.
In other words, let's change genpd_dev_pm_attach() to check for multiple PM
domains and prevent it from attach any PM domain for this case. Instead,
leave this to be managed separately, from following changes to genpd.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/domain.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6f403d6fccb2..908c44779ae7 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
* attaches the device to retrieved pm_domain ops.
*
* Returns 1 on successfully attached PM domain, 0 when the device don't need a
- * PM domain or a negative error code in case of failures. Note that if a
- * power-domain exists for the device, but it cannot be found or turned on,
- * then return -EPROBE_DEFER to ensure that the device is not probed and to
- * re-try again later.
+ * PM domain or when multiple power-domains exists for it, else a negative error
+ * code. Note that if a power-domain exists for the device, but it cannot be
+ * found or turned on, then return -EPROBE_DEFER to ensure that the device is
+ * not probed and to re-try again later.
*/
int genpd_dev_pm_attach(struct device *dev)
{
@@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
if (!dev->of_node)
return 0;
+ /*
+ * Devices with multiple PM domains must be attached separately, as we
+ * can only attach one PM domain per device.
+ */
+ 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)
- return 0;
+ return ret;
mutex_lock(&gpd_list_lock);
pd = genpd_get_from_provider(&pd_args);
--
2.17.0
^ permalink raw reply related
* [PATCH v3 1/5] PM / Domains: dt: Allow power-domain property to be a list of specifiers
From: Ulf Hansson @ 2018-05-31 10:59 UTC (permalink / raw)
To: Rafael J . Wysocki, linux-pm
Cc: Ulf Hansson, Greg Kroah-Hartman, Jon Hunter, Geert Uytterhoeven,
Todor Tomov, Rajendra Nayak, Viresh Kumar, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra,
Rob Herring, devicetree
In-Reply-To: <20180531105959.14843-1-ulf.hansson@linaro.org>
To be able to describe topologies where devices are partitioned across
multiple power domains, let's extend the power-domain property to allow
being a list of PM domain specifiers.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
---
.../bindings/power/power_domain.txt | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 4733f76cbe48..9b387f861aed 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -111,8 +111,8 @@ Example 3:
==PM domain consumers==
Required properties:
- - power-domains : A phandle and PM domain specifier as defined by bindings of
- the power controller specified by phandle.
+ - power-domains : A list of PM domain specifiers, as defined by bindings of
+ the power controller that is the PM domain provider.
Example:
@@ -122,9 +122,18 @@ Example:
power-domains = <&power 0>;
};
-The node above defines a typical PM domain consumer device, which is located
-inside a PM domain with index 0 of a power controller represented by a node
-with the label "power".
+ leaky-device@12351000 {
+ compatible = "foo,i-leak-current";
+ reg = <0x12351000 0x1000>;
+ power-domains = <&power 0>, <&power 1> ;
+ };
+
+The first example above defines a typical PM domain consumer device, which is
+located inside a PM domain with index 0 of a power controller represented by a
+node with the label "power".
+In the second example the consumer device are partitioned across two PM domains,
+the first with index 0 and the second with index 1, of a power controller that
+is represented by a node with the label "power.
Optional properties:
- required-opps: This contains phandle to an OPP node in another device's OPP
--
2.17.0
^ permalink raw reply related
* [PATCH v3 0/5] PM / Domains: Add support for multi PM domains per device
From: Ulf Hansson @ 2018-05-31 10:59 UTC (permalink / raw)
To: Rafael J . Wysocki, linux-pm
Cc: Ulf Hansson, Greg Kroah-Hartman, Jon Hunter, Geert Uytterhoeven,
Todor Tomov, Rajendra Nayak, Viresh Kumar, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
Changes in v3:
- Drop patch 1->4 as they have already been applied.
- Collected tags, for tests and reviews.
- Minor update to function descriptions in patch 4 (earlier 8) and 5
(earlier9).
- Note, because of the minor changes, no history is provided per patch.
Changes in v2:
- Addressed comments from Geert around DT doc.
- Addressed comments from Jon around clarification of how to use this
and changes to returned error codes.
- Fixed build error in case CONFIG_PM was unset.
There are devices that are partitioned across multiple PM domains. Currently
these can't be supported well by the available PM infrastructures we have in
the kernel. This series is an attempt to address this.
One existing case where devices are partitioned across multiple PM domains, is
the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
series, trying to address these issues, however this is a new approach, while
it re-uses the same concepts from DT point of view.
The Tegra 124/210 X-USB subsystem contains of a host controller and a device
controller. Each controller have its own independent PM domain, but are being
partitioned across another shared PM domain for the USB super-speed logic.
Currently to make the drivers work, either the related PM domains needs to stay
powered on always or the PM domain topology needs to be in-correctly modelled
through sub-domains. In both cases PM domains may be powered on while they
don't need to be, so in the end this means - wasting power -.
As stated above, this series intends to address these problem from a PM
infrastructure point of view. More details are available in each changelog.
Kind regards
Ulf Hansson
Ulf Hansson (5):
PM / Domains: dt: Allow power-domain property to be a list of
specifiers
PM / Domains: Don't attach devices in genpd with multi PM domains
PM / Domains: Split genpd_dev_pm_attach()
PM / Domains: Add support for multi PM domains per device to genpd
PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
domains
.../bindings/power/power_domain.txt | 19 ++-
drivers/base/power/common.c | 43 +++++-
drivers/base/power/domain.c | 134 +++++++++++++++---
include/linux/pm_domain.h | 15 ++
4 files changed, 183 insertions(+), 28 deletions(-)
--
2.17.0
^ permalink raw reply
* Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-31 10:48 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, Linux PM, Greg Kroah-Hartman,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Viresh Kumar,
Vincent Guittot, Kevin Hilman, Linux Kernel Mailing List,
Linux ARM, linux-tegra
In-Reply-To: <CAPDyKFrtSGF79pFf-sx41ru_EVLk3BMg_f_5rK64k==dW=OXPA@mail.gmail.com>
On 31/05/18 11:44, Ulf Hansson wrote:
> On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> On 31/05/18 07:17, Ulf Hansson wrote:
>>> [...]
>>>
>>>>> +/**
>>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>>> + * @dev: Device to attach.
>>>>
>>>> Can you update the description of the above as well?
>>>
>>> Yes, like below?
>>>
>>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>>> @dev: The device used to lookup the PM domain.
>>
>> Yes perfect.
>
> I assume I can consider this as an ack and tested by tag, both for
> patch 8 and 9?
Yes you can.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device
From: Ulf Hansson @ 2018-05-31 10:47 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J . Wysocki, Linux PM, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, Linux Kernel Mailing List, Linux ARM, linux-tegra
In-Reply-To: <20180531091410.fl5f42zmyqi6zj5a@vireshk-i7>
On 31 May 2018 at 11:14, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-05-18, 12:04, Ulf Hansson wrote:
>> Changes in v2:
>> - Addressed comments from Geert around DT doc.
>> - Addressed comments from Jon around clarification of how to use this
>> and changes to returned error codes.
>> - Fixed build error in case CONFIG_PM was unset.
>>
>> There are devices that are partitioned across multiple PM domains. Currently
>> these can't be supported well by the available PM infrastructures we have in
>> the kernel. This series is an attempt to address this.
>>
>> The interesting parts happens from patch 5 an onwards, including a minor DT
>> update to the existing power-domain bindings, the 4 earlier are just trivial
>> clean-ups of some related code in genpd, which I happened to stumble over.
>>
>> Some additional background:
>>
>> One existing case where devices are partitioned across multiple PM domains, is
>> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
>> series, trying to address these issues, however this is a new approach, while
>> it re-uses the same concepts from DT point of view.
>>
>> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
>> controller. Each controller have its own independent PM domain, but are being
>> partitioned across another shared PM domain for the USB super-speed logic.
>>
>> Currently to make the drivers work, either the related PM domains needs to stay
>> powered on always or the PM domain topology needs to be in-correctly modelled
>> through sub-domains. In both cases PM domains may be powered on while they
>> don't need to be, so in the end this means - wasting power -.
>>
>> As stated above, this series intends to address these problem from a PM
>> infrastructure point of view. More details are available in each changelog.
>>
>> It should be noted that this series has been tested on HW, however only by using
>> a home-cooked test PM domain driver for genpd and together with a test driver.
>> This allowed me to play with PM domain (genpd), runtime PM and device links.
>>
>> Any further deployment for real use cases are greatly appreciated. I am happy to
>> to help, if needed!
>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Thanks!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v3 1/2] sched/cpufreq: always consider blocked FAIR utilization
From: Rafael J. Wysocki @ 2018-05-31 10:46 UTC (permalink / raw)
To: Patrick Bellasi
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Linux PM,
Ingo Molnar, Peter Zijlstra, Rafael J . Wysocki, Viresh Kumar,
Vincent Guittot, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Joel Fernandes, Steve Muckle
In-Reply-To: <20180530165010.GK30654@e110439-lin>
On Wednesday, May 30, 2018 6:50:10 PM CEST Patrick Bellasi wrote:
> On 27-May 11:50, Rafael J. Wysocki wrote:
> > On Thu, May 24, 2018 at 4:10 PM, Patrick Bellasi
> > <patrick.bellasi@arm.com> wrote:
> > > Since the refactoring introduced by:
> > >
> > > commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
> > >
> > > we aggregate FAIR utilization only if this class has runnable tasks.
> > > This was mainly due to avoid the risk to stay on an high frequency just
> > > because of the blocked utilization of a CPU not being properly decayed
> > > while the CPU was idle.
> > >
> > > However, since:
> > >
> > > commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle")
> > >
> > > the FAIR blocked utilization is properly decayed also for IDLE CPUs.
> > >
> > > This allows us to use the FAIR blocked utilization as a safe mechanism
> > > to gracefully reduce the frequency only if no FAIR tasks show up on a
> > > CPU for a reasonable period of time.
> > >
> > > Moreover, we also reduce the frequency drops of CPUs running periodic
> > > tasks which, depending on the task periodicity and the time required
> > > for a frequency switch, was increasing the chances to introduce some
> > > undesirable performance variations.
> > >
> > > Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
> > > Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > Acked-by: Vincent Guittot <vincent.guittot@linaro.org>
> > > Tested-by: Vincent Guittot <vincent.guittot@linaro.org>
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Cc: Vincent Guittot <vincent.guittot@linaro.org>
> > > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > > Cc: Joel Fernandes <joelaf@google.com>
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: linux-pm@vger.kernel.org
> >
> > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Or please let me know if you want me to apply this one.
>
> Hi Rafael, seems this patch has already been applied in tip/sched/core.
> However is missing your tag above. :/
That's OK.
I just wanted to let people know my opinion. :-)
^ permalink raw reply
* Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-31 10:44 UTC (permalink / raw)
To: Jon Hunter
Cc: Rafael J . Wysocki, Linux PM, Greg Kroah-Hartman,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Viresh Kumar,
Vincent Guittot, Kevin Hilman, Linux Kernel Mailing List,
Linux ARM, linux-tegra
In-Reply-To: <e313f870-ef27-7721-9395-31671f00ef4a@nvidia.com>
On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 31/05/18 07:17, Ulf Hansson wrote:
>> [...]
>>
>>>> +/**
>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>> + * @dev: Device to attach.
>>>
>>> Can you update the description of the above as well?
>>
>> Yes, like below?
>>
>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>> @dev: The device used to lookup the PM domain.
>
> Yes perfect.
I assume I can consider this as an ack and tested by tag, both for
patch 8 and 9?
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device
From: Viresh Kumar @ 2018-05-31 9:14 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180529100421.31022-1-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> Changes in v2:
> - Addressed comments from Geert around DT doc.
> - Addressed comments from Jon around clarification of how to use this
> and changes to returned error codes.
> - Fixed build error in case CONFIG_PM was unset.
>
> There are devices that are partitioned across multiple PM domains. Currently
> these can't be supported well by the available PM infrastructures we have in
> the kernel. This series is an attempt to address this.
>
> The interesting parts happens from patch 5 an onwards, including a minor DT
> update to the existing power-domain bindings, the 4 earlier are just trivial
> clean-ups of some related code in genpd, which I happened to stumble over.
>
> Some additional background:
>
> One existing case where devices are partitioned across multiple PM domains, is
> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
> series, trying to address these issues, however this is a new approach, while
> it re-uses the same concepts from DT point of view.
>
> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
> controller. Each controller have its own independent PM domain, but are being
> partitioned across another shared PM domain for the USB super-speed logic.
>
> Currently to make the drivers work, either the related PM domains needs to stay
> powered on always or the PM domain topology needs to be in-correctly modelled
> through sub-domains. In both cases PM domains may be powered on while they
> don't need to be, so in the end this means - wasting power -.
>
> As stated above, this series intends to address these problem from a PM
> infrastructure point of view. More details are available in each changelog.
>
> It should be noted that this series has been tested on HW, however only by using
> a home-cooked test PM domain driver for genpd and together with a test driver.
> This allowed me to play with PM domain (genpd), runtime PM and device links.
>
> Any further deployment for real use cases are greatly appreciated. I am happy to
> to help, if needed!
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH 11/11] misc/throttler: Add Chrome OS EC throttler
From: Enric Balletbo Serra @ 2018-05-31 9:05 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Arnd Bergmann,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Linux PM list,
devicetree@vger.kernel.org, linux-kernel, Brian Norris,
Douglas Anderson
In-Reply-To: <20180525203043.249193-12-mka@chromium.org>
Hi Matthias,
The patch looks good to me, just three minor comments to be more
coherent with other cros-ec drivers.
2018-05-25 22:30 GMT+02:00 Matthias Kaehlcke <mka@chromium.org>:
> The driver subscribes to throttling events from the Chrome OS
> embedded controller and enables/disables system throttling based
> on these events.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/misc/throttler/Kconfig | 15 +++
> drivers/misc/throttler/Makefile | 1 +
> drivers/misc/throttler/cros_ec_throttler.c | 122 +++++++++++++++++++++
> 3 files changed, 138 insertions(+)
> create mode 100644 drivers/misc/throttler/cros_ec_throttler.c
>
> diff --git a/drivers/misc/throttler/Kconfig b/drivers/misc/throttler/Kconfig
> index ef8388f6bc0a..652b6817b75c 100644
> --- a/drivers/misc/throttler/Kconfig
> +++ b/drivers/misc/throttler/Kconfig
> @@ -11,3 +11,18 @@ menuconfig THROTTLER
> Note that you also need a event monitor module usually called
> *_throttler.
>
> +if THROTTLER
> +
> +config CROS_EC_THROTTLER
> + tristate "Throttler event monitor for the Chrome OS Embedded Controller"
s/Chrome OS/ChromeOS/ This is how other cros-ec drivers refer to the
embedded controller, so will be good if you could maintain this
denomination.
> + default n
> + depends on MFD_CROS_EC
> + ---help---
> + This driver adds support to throttle the system in reaction to
> + Chrome OS EC events.
> +
ditto
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_throttler.
> +
> +endif # THROTTLER
> +
> diff --git a/drivers/misc/throttler/Makefile b/drivers/misc/throttler/Makefile
> index c8d920cee315..d9b2a77dabc9 100644
> --- a/drivers/misc/throttler/Makefile
> +++ b/drivers/misc/throttler/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_THROTTLER) += core.o
> +obj-$(CONFIG_CROS_EC_THROTTLER) += cros_ec_throttler.o
> diff --git a/drivers/misc/throttler/cros_ec_throttler.c b/drivers/misc/throttler/cros_ec_throttler.c
> new file mode 100644
> index 000000000000..ea6bc002d49c
> --- /dev/null
> +++ b/drivers/misc/throttler/cros_ec_throttler.c
> @@ -0,0 +1,122 @@
> +/*
> + * Driver for throttling triggered by EC events.
> + *
> + * Copyright (C) 2018 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
Replace this for the SPDX header format
// SPDX-License-Identifier: GPL-2.0+
// Driver for throttling triggered by EC events.
//
// Copyright (C) 2018 Google, Inc.
// Author: Matthias Kaehlcke <mka@chromium.org>
> +#include <linux/kernel.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/throttler.h>
> +
> +struct cros_ec_throttler {
> + struct cros_ec_device *ec;
> + struct throttler *throttler;
> + struct notifier_block nb;
> +};
> +
> +static int cros_ec_throttler_event(struct notifier_block *nb,
> + unsigned long queued_during_suspend, void *_notify)
> +{
> + struct cros_ec_throttler *cte =
> + container_of(nb, struct cros_ec_throttler, nb);
nit: Better add a define here instead of split the line like this ?
> + u32 host_event;
> +
> + host_event = cros_ec_get_host_event(cte->ec);
> + if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_THROTTLE_START)) {
> + throttler_set_level(cte->throttler, 1);
> +
> + return NOTIFY_OK;
> + } else if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_THROTTLE_STOP)) {
> + throttler_set_level(cte->throttler, 0);
> +
> + return NOTIFY_OK;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int cros_ec_throttler_probe(struct platform_device *pdev)
> +{
> + struct cros_ec_throttler *cte;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + int ret;
> +
> + if (!np) {
> + /* should never happen */
> + return -EINVAL;
> + }
> +
> + cte = devm_kzalloc(dev, sizeof(*cte), GFP_KERNEL);
> + if (!cte)
> + return -ENOMEM;
> +
> + cte->ec = dev_get_drvdata(pdev->dev.parent);
> +
> + cte->throttler = throttler_setup(dev);
> + if (IS_ERR(cte->throttler))
> + return PTR_ERR(cte->throttler);
> +
> + dev_set_drvdata(dev, cte);
> +
> + cte->nb.notifier_call = cros_ec_throttler_event;
> + ret = blocking_notifier_chain_register(&cte->ec->event_notifier,
> + &cte->nb);
> + if (ret < 0) {
> + dev_err(dev, "failed to register notifier\n");
> + throttler_teardown(cte->throttler);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int cros_ec_throttler_remove(struct platform_device *pdev)
> +{
> + struct cros_ec_throttler *cte = platform_get_drvdata(pdev);
> +
> + blocking_notifier_chain_unregister(&cte->ec->event_notifier,
> + &cte->nb);
> +
> + throttler_teardown(cte->throttler);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id cros_ec_throttler_of_match[] = {
> + { .compatible = "google,cros-ec-throttler" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_throttler_of_match);
> +#endif /* CONFIG_OF */
> +
> +static struct platform_driver cros_ec_throttler_driver = {
> + .driver = {
> + .name = "cros-ec-throttler",
> + .of_match_table = of_match_ptr(cros_ec_throttler_of_match),
> + },
> + .probe = cros_ec_throttler_probe,
> + .remove = cros_ec_throttler_remove,
> +};
> +
> +module_platform_driver(cros_ec_throttler_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Matthias Kaehlcke <mka@chromium.org>");
> +MODULE_DESCRIPTION("Chrome OS EC Throttler");
s/Chrome OS/ChromeOS/
> --
> 2.17.0.921.gf22659ad46-goog
>
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Thanks,
Enric
^ permalink raw reply
* Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-31 8:03 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, Linux PM, Greg Kroah-Hartman,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Viresh Kumar,
Vincent Guittot, Kevin Hilman, Linux Kernel Mailing List,
Linux ARM, linux-tegra
In-Reply-To: <CAPDyKFrV5fosZ1QZ4un+okzpeQDM3QSCBWJB1YZH4WNxKf_FVQ@mail.gmail.com>
On 31/05/18 07:17, Ulf Hansson wrote:
> [...]
>
>>> +/**
>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> + * @dev: Device to attach.
>>
>> Can you update the description of the above as well?
>
> Yes, like below?
>
> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.
Yes perfect.
Thanks
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Stephen Boyd @ 2018-05-31 7:41 UTC (permalink / raw)
To: Bjorn Andersson, Sricharan R
Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
linux
In-Reply-To: <9f2e1aa8-21c1-10b0-6193-e6bc16993a0d@codeaurora.org>
Quoting Sricharan R (2018-05-30 21:57:20)
> Hi Stephen,
>
> On 5/30/2018 9:25 PM, Stephen Boyd wrote:
> > Quoting Sricharan R (2018-05-24 22:40:11)
> >> Hi Bjorn,
> >>
> >> On 5/24/2018 11:09 PM, Bjorn Andersson wrote:
> >>> On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> >>>
> >>>> From: Stephen Boyd <sboyd@codeaurora.org>
> >>>>
> >>>> Krait CPUs have a handful of L2 cache controller registers that
> >>>> live behind a cp15 based indirection register. First you program
> >>>> the indirection register (l2cpselr) to point the L2 'window'
> >>>> register (l2cpdr) at what you want to read/write. Then you
> >>>> read/write the 'window' register to do what you want. The
> >>>> l2cpselr register is not banked per-cpu so we must lock around
> >>>> accesses to it to prevent other CPUs from re-pointing l2cpdr
> >>>> underneath us.
> >>>>
> >>>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>>> Cc: Russell King <linux@arm.linux.org.uk>
> >>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> >>>
> >>> This should have your signed-off-by here as well.
> >>>
> >>
> >> ok.
> >>
> >>> Apart from that:
> >>>
> >>> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >>>
> >>
> >
> > Will these patches come around again? I'll do a quick sweep on them
> > today but I expect them to be resent.
>
> Sure, i will have to resend them again, fixing couple of Bjorn's
> minor comments. Will address your comments that you would give
> as well along with that.
>
Ok. One general comment is that it would be nice if the bindings for all
the nodes that are introduced included 'clocks' properties and also
maybe 'clock-names' properties for the clocks that are consumed by each
node. Right now, we hide those details from DT and rely on the string
names to hook the clk tree up for us. That sort of prevents us from
moving away from string easily, so I would just throw the clocks into
the binding right now and always have them there just in case we want to
use the binding to figure out the hierarchy in the future.
I've been thinking we need to do something similar for the gcc and other
nodes for any clks they use, but I haven't gotten around to it.
Otherwise the patches look mostly ok to me. Not sure I'll have any other
comments.
^ permalink raw reply
* Re: [PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()
From: Viresh Kumar @ 2018-05-31 7:04 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180529100421.31022-8-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH] cpufreq / CPPC: Add cpuinfo_cur_freq support for CPPC
From: George Cherian @ 2018-05-31 6:33 UTC (permalink / raw)
To: Prakash, Prashanth, George Cherian, linux-kernel, linux-pm
Cc: rjw, viresh.kumar
In-Reply-To: <955aeba5-bd5e-6051-b283-ad544327ce64@codeaurora.org>
Hi Prashanth,
On 05/29/2018 09:14 PM, Prakash, Prashanth wrote:
>
> On 5/28/2018 1:09 AM, George Cherian wrote:
>> Hi Prashanth,
>>
>> On 05/26/2018 02:30 AM, Prakash, Prashanth wrote:
>>>
>>> On 5/25/2018 12:27 AM, George Cherian wrote:
>>>> Hi Prashanth,
>>>>
>>>> On 05/25/2018 12:55 AM, Prakash, Prashanth wrote:
>>>>> Hi George,
>>>>>
>>>>> On 5/22/2018 5:42 AM, George Cherian wrote:
>>>>>> Per Section 8.4.7.1.3 of ACPI 6.2, The platform provides performance
>>>>>> feedback via set of performance counters. To determine the actual
>>>>>> performance level delivered over time, OSPM may read a set of
>>>>>> performance counters from the Reference Performance Counter Register
>>>>>> and the Delivered Performance Counter Register.
>>>>>>
>>>>>> OSPM calculates the delivered performance over a given time period by
>>>>>> taking a beginning and ending snapshot of both the reference and
>>>>>> delivered performance counters, and calculating:
>>>>>>
>>>>>> delivered_perf = reference_perf X (delta of delivered_perf counter / delta of reference_perf counter).
>>>>>>
>>>>>> Implement the above and hook this to the cpufreq->get method.
>>>>>>
>>>>>> Signed-off-by: George Cherian <george.cherian@cavium.com>
>>>>>> ---
>>>>>> drivers/cpufreq/cppc_cpufreq.c | 44 ++++++++++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 44 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>>>>> index b15115a..a046915 100644
>>>>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>>>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>>>>> @@ -240,10 +240,54 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>>>> return ret;
>>>>>> }
>>>>>> +static int cppc_get_rate_from_fbctrs(struct cppc_perf_fb_ctrs fb_ctrs_t0,
>>>>>> + struct cppc_perf_fb_ctrs fb_ctrs_t1)
>>>>>> +{
>>>>>> + u64 delta_reference, delta_delivered;
>>>>>> + u64 reference_perf, ratio;
>>>>>> +
>>>>>> + reference_perf = fb_ctrs_t0.reference_perf;
>>>>>> + if (fb_ctrs_t1.reference > fb_ctrs_t0.reference)
>>>>>> + delta_reference = fb_ctrs_t1.reference - fb_ctrs_t0.reference;
>>>>>> + else /* Counters would have wrapped-around */
>>>>>> + delta_reference = ((u64)(~((u64)0)) - fb_ctrs_t0.reference) +
>>>>>> + fb_ctrs_t1.reference;
>>>>>> +
>>>>>> + if (fb_ctrs_t1.delivered > fb_ctrs_t0.delivered)
>>>>>> + delta_delivered = fb_ctrs_t1.delivered - fb_ctrs_t0.delivered;
>>>>>> + else /* Counters would have wrapped-around */
>>>>>> + delta_delivered = ((u64)(~((u64)0)) - fb_ctrs_t0.delivered) +
>>>>>> + fb_ctrs_t1.delivered;
>>>>> We need to check that the wraparound time is long enough to make sure that
>>>>> the counters cannot wrap around more than once. We can register a get() api
>>>>> only after checking that wraparound time value is reasonably high.
>>>>>
>>>>> I am not aware of any platforms where wraparound time is soo short, but
>>>>> wouldn't hurt to check once during init.
>>>> By design the wraparound time is a 64 bit counter, for that matter even
>>>> all the feedback counters too are 64 bit counters. I don't see any
>>>> chance in which the counters can wraparound twice in back to back reads.
>>>> The only situation is in which system itself is running at a really high
>>>> frequency. Even in that case today's spec is not sufficient to support the same.
>>>
>>> The spec doesn't say these have to be 64bit registers. The wraparound
>>> counter register is in spec to communicate the worst case(shortest)
>>> counter rollover time.
>>
>> Spec says these are 32 or 64 bit registers. Spec also defines counter
>> wraparound time in seconds. The minimum value possible is 1 as zero means the counters are never assumed to wrap around. Even in platforms with value set as 1 (1 sec) I dont really see a situation in which
>> the counter can wraparound twice if we are putting a delay of 2usec
>> between sampling.
> ok.
Thanks
>>
>>>
>>> As as mentioned before this is just a defensive check to make sure that
>>> the platform has not set it to some very low number (which is allowed
>>> by the spec).
>> It might be unnecessary to have a check like this.
>>>
>>>>
>>>>>> +
>>>>>> + if (delta_reference) /* Check to avoid divide-by zero */
>>>>>> + ratio = (delta_delivered * 1000) / delta_reference;
>>>>> Why not just return the computed value here instead of *1000 and later /1000?
>>>>> return (ref_per * delta_del) / delta_ref;
>>>> Yes.
>>>>>> + else
>>>>>> + return -EINVAL;
>>>>> Instead of EINVAL, i think we should return current frequency.
>>>>>
>>>> Sorry, I didn't get you, How do you calculate the current frequency?
>>>> Did you mean reference performance?
>>> I mean the performance that OSPM/Linux had requested earlier.
>>> i.e the desired_perf
>> Okay, I will make necessary changes for this in v2.
>>
>>>>
>>>>> The counters can pause if CPUs are in idle state during our sampling interval, so
>>>>> If the counters did not progress, it is reasonable to assume the delivered perf was
>>>>> equal to desired perf.
>>>> No, that is wrong. Here the check is for reference performance delta.
>>>> This counter can never pause. In case of cpuidle only the delivered counters could pause. Delivered counters will pause only if the particular core enters power down mode, Otherwise we would be still clocking the core and we should be getting a delta across 2 sampling periods. In case if the reference counter is paused which by design is not correct then there is no point in returning reference performance numbers. That too is wrong. In case the low level FW is not updating the
>>>> counters properly then it should be evident till Linux, instead of returning a bogus frequency.
>>>
>>> Again you are describing how it works on a specific platform and not
>>> how it is described in spec. Section 8.4.7.1.3.1.1 of ACPI 6.2 states
>>> "The Reference Performance Counter Register counts at a fixed rate
>>> any time the processor is active."
>>> > Implies the counters *may* pause in idle state -I can imagine an
>>> implementation where you can keep this counter running and
>>> account for it via delivered counter, but we cannot make any
>>> assumptions outside of what the spec describes.
>>>
>>>>>
>>>>> Even if platform wanted to limit, since the CPUs were asleep(idle) we could not have
>>>>> observed lower performance, so we will not throw off any logic that could be driven
>>>>> using the returned value.
>>>>>> +
>>>>>> + return (reference_perf * ratio) / 1000;
>>>>> This should be converted to KHz as cpufreq is not aware of CPPC abstract scale
>>>> In our platform all performance registers are implemented in KHz. Because of which we never had an issue with conversion. I am not
>>>> aware whether ACPI mandates to use any particular unit. How is that
>>>> implemented in your platform? Just to avoid any extra conversion don't
>>>> you feel it is better to always report in KHz from firmware.
>>> Again think of spec not a specific platform :)
>>> - The CPPC spec works on abstract scale and cpufreq works in KHz.
>>> - The above computed value is in abstract scale
>>> - The abstarct scale may be in KHz on your platform, but we cannot assume the
>>> same about all the platforms
>> For now can I assume it to be in KHz only?
>
> No, it will break platforms where abstract scale is not in KHz.
>
>> I am not sure how to convert the abstract scale to Khz.
>> Can you please give me some pointers on the same?
>
> Take a look at cppc_cpufreq_perf_to_khz and cppc_cpufreq_khz_to_perf
> in the same file (cppc_cpufreq.c). We use this in almost every function
> registered with core cpufreq.
> ||
>> In spec there is currently no interface which tells what is the abstract
>> scale!!
>
> CPPC v3 adds some additional hooks for this. On CPPC v2, we try to use
> few DMI table entries to get the ratio between abstract scale and KHz.
Okay I will address these in v2.
>
>>>>
>>>>>> +}
>>>>>> +
>>>>>> +static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
>>>>>> +{
>>>>>> + struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
>>>>>> + int ret;
>>>>>> +
>>>>>> + ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t0);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t1);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + return cppc_get_rate_from_fbctrs(fb_ctrs_t0, fb_ctrs_t1);
>>>>>> +}
>>>>> We need to make sure that we get a reasonably sample so make sure the reported
>>>>> performance is accurate.
>>>>> The counters can capture short transient throttling/limiting, so by sampling a really
>>>>> short duration of time we could return quite inaccurate measure of performance.
>>>>>
>>>> I would say it as a momentary thing only when the frequency is being ramped up/down.
>>> This exact behavior would depend on how different limiting functions are implemented.
>>> So this would vary from one platform to another.
>>>>
>>>>> We need to include some reasonable delay between the two calls. What is reasonable
>>>>> is debatable - so this can be few(2-10) microseconds defined as default. If the same value
>>>>> doesn't work for all the platforms, we might need to add a platform specific value.
>>>>>
>>>> cppc_get_perf_ctrs itself is a slow call, we have to format the CPC packet and ring a doorbell and then the response to be read from the shared registers. My initial implementation had a delay but in testing,
>>>> I found that it was unnecessary to have such a delay. Can you please
>>>> let me know whether it works without delay in your platform?
>>>>
>>>> Or else let me know whether udelay(10) is sufficient in between the
>>>> calls.
>>> Feedback counters need not be in PCC .
>>> 2us should be sufficient.
>> Yes I will add this to v2.
>>>>>> +
>>>>>> static struct cpufreq_driver cppc_cpufreq_driver = {
>>>>>> .flags = CPUFREQ_CONST_LOOPS,
>>>>>> .verify = cppc_verify_policy,
>>>>>> .target = cppc_cpufreq_set_target,
>>>>>> + .get = cppc_cpufreq_get_rate,
>>>>>> .init = cppc_cpufreq_cpu_init,
>>>>>> .stop_cpu = cppc_cpufreq_stop_cpu,
>>>>>> .name = "cppc_cpufreq",
>>>>>
>>>
>
^ permalink raw reply
* Re: [PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers
From: Viresh Kumar @ 2018-05-31 6:18 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra,
Rob Herring, devicetree
In-Reply-To: <20180529100421.31022-6-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> To be able to describe topologies where devices are partitioned across
> multiple power domains, let's extend the power-domain property to allow
> being a list of PM domain specifiers.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* RE: [RFC] PM / devfreq: Add support for alerts
From: MyungJoo Ham @ 2018-05-31 6:17 UTC (permalink / raw)
To: Akhil P Oommen, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, mka@chromium.org
Cc: jcrouse@codeaurora.org, Kyungmin Park, Chanwoo Choi,
linux-arm-msm@vger.kernel.org
In-Reply-To: <1527745019-25155-1-git-send-email-akhilpo@codeaurora.org>
> Currently, DEVFREQ reevaluates the device state periodically and/or
> based on the OPP list changes. Private API has to be exposed to allow
> the device driver to alert/notify the governor to reevaluate when a new
> set of data is available. This makes the governor more coupled to a
> particular device driver. We can improve here by exposing a DEVFREQ API
> to allow the device drivers to send generic alerts to the governor.
>
> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> ---
> drivers/devfreq/devfreq.c | 21 +++++++++++++++++++++
> drivers/devfreq/governor.h | 1 +
> include/linux/devfreq.h | 5 +++++
> 3 files changed, 27 insertions(+)
>
Hello Akhil,
It appears that this will have the same effect with
"[PATCH 08/11] PM / devfreq: Make update_devfreq() public" from Matthias Kaehlcke, doesn't it?
Cheers,
MyungJoo
^ permalink raw reply
* Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-31 6:17 UTC (permalink / raw)
To: Jon Hunter
Cc: Rafael J . Wysocki, Linux PM, Greg Kroah-Hartman,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Viresh Kumar,
Vincent Guittot, Kevin Hilman, Linux Kernel Mailing List,
Linux ARM, linux-tegra
In-Reply-To: <a53f0970-22c4-444f-10e6-c115a21d05ea@nvidia.com>
[...]
>> +/**
>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>> + * @dev: Device to attach.
>
> Can you update the description of the above as well?
Yes, like below?
genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
@dev: The device used to lookup the PM domain.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 4/9] PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
From: Viresh Kumar @ 2018-05-31 6:16 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180529100421.31022-5-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> The in-parameter struct generic_pm_domain *genpd to
> genpd_allocate_dev_data() is unused, so let's drop it.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH v2 3/9] PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
From: Viresh Kumar @ 2018-05-31 6:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra,
Alex Deucher, Christian König, David Zhou
In-Reply-To: <20180529100421.31022-4-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> There is no need to pass a genpd struct to pm_genpd_remove_device(), as we
> already have the information about the PM domain (genpd) through the device
> structure.
>
> Additionally, we don't allow to remove a PM domain from a device, other
> than the one it may have assigned to it, so really it does not make sense
> to have a separate in-param for it.
>
> For these reason, drop it and update the current only call to
> pm_genpd_remove_device() from amdgpu_acp.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 8 ++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 2 +-
> include/linux/pm_domain.h | 5 ++---
> 3 files changed, 7 insertions(+), 8 deletions(-)
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH v2 2/9] PM / Domains: Drop __pm_genpd_add_device()
From: Viresh Kumar @ 2018-05-31 6:13 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180529100421.31022-3-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> There are still a few non-DT existing users of genpd, however neither of
> them uses __pm_genpd_add_device(), hence let's drop it.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 10 ++++------
> include/linux/pm_domain.h | 14 +++-----------
> 2 files changed, 7 insertions(+), 17 deletions(-)
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH v2 1/9] PM / Domains: Drop extern declarations of functions in pm_domain.h
From: Viresh Kumar @ 2018-05-31 6:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <20180529100421.31022-2-ulf.hansson@linaro.org>
On 29-05-18, 12:04, Ulf Hansson wrote:
> Using "extern" to declare a function in a public header file is somewhat
> pointless, but also doesn't hurt. However, to make all the function
> declarations in pm_domain.h to be consistent, let's drop the use of
> "extern".
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> include/linux/pm_domain.h | 51 ++++++++++++++++++---------------------
> 1 file changed, 23 insertions(+), 28 deletions(-)
Finally.
I wanted to do that earlier as checkpatch complained non stop about
such declarations :)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox