diff mbox

[v2,3/5] PM / OPP: check for existing OPP list when initialising from device tree

Message ID 1380634382-15609-4-git-send-email-Sudeep.KarkadaNagesha@arm.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Sudeep KarkadaNagesha Oct. 1, 2013, 1:33 p.m. UTC
From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

CPUs are registered as devices and their OPPs can be initialised from
the device tree. Whenever CPUs are hotplugged out, the corresponding
cpu devices are not removed. As a result all their OPPs remain intact
even when they are offlined.

But when they are hotplugged back-in, the cpufreq along with other cpu
related subsystem gets re-initialised. Since its almost same as secondary
cpu being brought up, no special consideration is taken in the hotplug
path. This may result in cpufreq trying to initialise the OPPs again though
the cpu device already contains the OPPs.

This patch checks if there exist an OPP list associated with the device,
before attempting to initialise it.

Cc: Nishanth Menon <nm@ti.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 drivers/base/power/opp.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Viresh Kumar Oct. 3, 2013, 4:54 a.m. UTC | #1
On 1 October 2013 19:03, Sudeep KarkadaNagesha
<Sudeep.KarkadaNagesha@arm.com> wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>
> CPUs are registered as devices and their OPPs can be initialised from
> the device tree. Whenever CPUs are hotplugged out, the corresponding
> cpu devices are not removed. As a result all their OPPs remain intact
> even when they are offlined.
>
> But when they are hotplugged back-in, the cpufreq along with other cpu
> related subsystem gets re-initialised. Since its almost same as secondary
> cpu being brought up, no special consideration is taken in the hotplug
> path. This may result in cpufreq trying to initialise the OPPs again though
> the cpu device already contains the OPPs.
>
> This patch checks if there exist an OPP list associated with the device,
> before attempting to initialise it.
>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  drivers/base/power/opp.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nishanth Menon Oct. 3, 2013, 2:25 p.m. UTC | #2
On 10/01/2013 08:33 AM, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> CPUs are registered as devices and their OPPs can be initialised from
> the device tree. Whenever CPUs are hotplugged out, the corresponding
> cpu devices are not removed. As a result all their OPPs remain intact
> even when they are offlined.
> 
> But when they are hotplugged back-in, the cpufreq along with other cpu
> related subsystem gets re-initialised. Since its almost same as secondary
> cpu being brought up, no special consideration is taken in the hotplug
> path. This may result in cpufreq trying to initialise the OPPs again though
> the cpu device already contains the OPPs.
> 
> This patch checks if there exist an OPP list associated with the device,
> before attempting to initialise it.
> 
> Cc: Nishanth Menon <nm@ti.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  drivers/base/power/opp.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index bb6ff64..05b2ebf 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -709,9 +709,15 @@ int of_init_opp_table(struct device *dev)
>  {
>  	const struct property *prop;
>  	struct device_node *opp_node;
> +	struct device_opp *dev_opp;
>  	const __be32 *val;
>  	int nr;
>  
> +	/* Check for existing list for 'dev' */
> +	dev_opp = find_device_opp(dev);
> +	if (!IS_ERR(dev_opp))
> +		return -EEXIST; /* Device OPP already initialized */
in the interest of remaining code, Prefer comments not inline with
code -> example if additional logic gets added later on in the if
condition, someone'd have to move that comment over, instead, prefer
it embedded before the if check as a line of it's own.
> +
>  	opp_node = of_parse_phandle(dev->of_node,
>  					"operating-points-phandle", 0);
>  	if (!opp_node) /* if no OPP phandle, search for OPPs in current node */
> 

other than that,
Acked-by: Nishanth Menon <nm@ti.com>
diff mbox

Patch

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index bb6ff64..05b2ebf 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -709,9 +709,15 @@  int of_init_opp_table(struct device *dev)
 {
 	const struct property *prop;
 	struct device_node *opp_node;
+	struct device_opp *dev_opp;
 	const __be32 *val;
 	int nr;
 
+	/* Check for existing list for 'dev' */
+	dev_opp = find_device_opp(dev);
+	if (!IS_ERR(dev_opp))
+		return -EEXIST; /* Device OPP already initialized */
+
 	opp_node = of_parse_phandle(dev->of_node,
 					"operating-points-phandle", 0);
 	if (!opp_node) /* if no OPP phandle, search for OPPs in current node */