Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1190360 > unrolled thread

[PATCH 0/2] cpufreq: Better separation of device addition/removal and online/offline paths

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] cpufreq: Better separation of device addition/removal and online/offline paths "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-23 01:40 +0200
    [PATCH 1/2] cpufreq: Rename two functions related to CPU offline "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-23 01:40 +0200
      Re: [PATCH 1/2] cpufreq: Rename two functions related to CPU offline Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-23 08:50 +0200
    [PATCH 1/7] cpufreq: Rework two functions related to CPU offline "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
      Re: [PATCH 1/7] cpufreq: Rework two functions related to CPU offline Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 16:50 +0200
    [PATCH 0/7] cpufreq: Better separation of device addition/removal and online/offline paths "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
      [PATCH 6/7] cpufreq: Pass CPU number to cpufreq_policy_alloc() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
        Re: [PATCH 6/7] cpufreq: Pass CPU number to cpufreq_policy_alloc() Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 17:00 +0200
      [PATCH 3/7] cpufreq: Drop unnecessary label from cpufreq_add_dev() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
        Re: [PATCH 3/7] cpufreq: Drop unnecessary label from  cpufreq_add_dev() Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 17:00 +0200
      [PATCH 7/7] cpufreq: Separate CPU device removal from CPU online "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
        Re: [PATCH 7/7] cpufreq: Separate CPU device removal from CPU online Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 17:10 +0200
          Re: [PATCH 7/7] cpufreq: Separate CPU device removal from CPU online "Rafael J. Wysocki" <rafael@kernel.org> - 2015-07-28 00:00 +0200
        [Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 23:30 +0200
          [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-29 02:40 +0200
            [PATCH] cpufreq: Replace recover_policy with new_policy in cpufreq_online() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-29 02:50 +0200
              Re: [PATCH] cpufreq: Replace recover_policy with new_policy in  cpufreq_online() Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-29 07:40 +0200
            Re: [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration  from CPU online Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-29 07:40 +0200
              Re: [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration  from CPU online Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-29 16:10 +0200
              Re: [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration  from CPU online "Rafael J. Wysocki" <rafael@kernel.org> - 2015-07-29 16:10 +0200
      [PATCH 4/7] cpufreq: Drop unused dev argument from two functions "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
        Re: [PATCH 4/7] cpufreq: Drop unused dev argument from two functions Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 17:00 +0200
      [PATCH 5/7] cpufreq: Do not update related_cpus on every policy activation "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
        Re: [PATCH 5/7] cpufreq: Do not update related_cpus on every policy  activation Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 17:00 +0200
      [PATCH 2/7] cpufreq: Drop cpufreq_policy_restore() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:50 +0200
        Re: [PATCH 2/7] cpufreq: Drop cpufreq_policy_restore() Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 16:50 +0200

Page 1 of 2  [1] 2  Next page →

#1190360 — [PATCH 0/2] cpufreq: Better separation of device addition/removal and online/offline paths

Hi,

As per the recent discussion (http://marc.info/?l=linux-pm&m=143758336123670&w=4),
separete CPU device addition/removal code paths from the CPU online/offline code
paths in the cpufreq core a bit better.

Both on top of http://marc.info/?l=linux-pm&m=143760250929586&w=4.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1190361 — [PATCH 1/2] cpufreq: Rename two functions related to CPU offline

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since __cpufreq_remove_dev_prepare() and __cpufreq_remove_dev_finish()
are about CPU offline rather than about CPU removal, rename them to
cpufreq_offline_prepare() and cpufreq_offline_finish(), respectively.

Also change their argument from a struct device pointer to a CPU
number, because they use the CPU number only internally anyway.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1376,9 +1376,8 @@ out_release_rwsem:
 	return ret;
 }
 
-static void __cpufreq_remove_dev_prepare(struct device *dev)
+static void cpufreq_offline_prepare(unsigned int cpu)
 {
-	unsigned int cpu = dev->id;
 	struct cpufreq_policy *policy;
 
 	pr_debug("%s: unregistering CPU %u\n", __func__, cpu);
@@ -1423,9 +1422,8 @@ static void __cpufreq_remove_dev_prepare
 	}
 }
 
-static void __cpufreq_remove_dev_finish(struct device *dev)
+static void cpufreq_offline_finish(unsigned int cpu)
 {
-	unsigned int cpu = dev->id;
 	struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
 
 	if (!policy) {
@@ -1467,8 +1465,8 @@ static int cpufreq_remove_dev(struct dev
 		return 0;
 
 	if (cpu_online(cpu)) {
-		__cpufreq_remove_dev_prepare(dev);
-		__cpufreq_remove_dev_finish(dev);
+		cpufreq_offline_prepare(cpu);
+		cpufreq_offline_finish(cpu);
 	}
 
 	/* sysfs links are removed only on subsys callback */
@@ -2360,11 +2358,11 @@ static int cpufreq_cpu_callback(struct n
 			break;
 
 		case CPU_DOWN_PREPARE:
-			__cpufreq_remove_dev_prepare(dev);
+			cpufreq_offline_prepare(cpu);
 			break;
 
 		case CPU_POST_DEAD:
-			__cpufreq_remove_dev_finish(dev);
+			cpufreq_offline_finish(cpu);
 			break;
 
 		case CPU_DOWN_FAILED:

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1190655 — Re: [PATCH 1/2] cpufreq: Rename two functions related to CPU offline

On 23-07-15, 02:01, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Since __cpufreq_remove_dev_prepare() and __cpufreq_remove_dev_finish()
> are about CPU offline rather than about CPU removal, rename them to
> cpufreq_offline_prepare() and cpufreq_offline_finish(), respectively.
> 
> Also change their argument from a struct device pointer to a CPU
> number, because they use the CPU number only internally anyway.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |   14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193097 — [PATCH 1/7] cpufreq: Rework two functions related to CPU offline

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since __cpufreq_remove_dev_prepare() and __cpufreq_remove_dev_finish()
are about CPU offline rather than about CPU removal, rename them to
cpufreq_offline_prepare() and cpufreq_offline_finish(), respectively.

Also change their argument from a struct device pointer to a CPU
number, because they use the CPU number only internally anyway
and make them void as their return values are ignored.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c |   32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1398,10 +1398,8 @@ out_release_rwsem:
 	return ret;
 }
 
-static int __cpufreq_remove_dev_prepare(struct device *dev)
+static void cpufreq_offline_prepare(unsigned int cpu)
 {
-	unsigned int cpu = dev->id;
-	int ret = 0;
 	struct cpufreq_policy *policy;
 
 	pr_debug("%s: unregistering CPU %u\n", __func__, cpu);
@@ -1409,11 +1407,11 @@ static int __cpufreq_remove_dev_prepare(
 	policy = cpufreq_cpu_get_raw(cpu);
 	if (!policy) {
 		pr_debug("%s: No cpu_data found\n", __func__);
-		return -EINVAL;
+		return;
 	}
 
 	if (has_target()) {
-		ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+		int ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 		if (ret)
 			pr_err("%s: Failed to stop governor\n", __func__);
 	}
@@ -1434,7 +1432,7 @@ static int __cpufreq_remove_dev_prepare(
 	/* Start governor again for active policy */
 	if (!policy_is_inactive(policy)) {
 		if (has_target()) {
-			ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
+			int ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
 			if (!ret)
 				ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
 
@@ -1444,28 +1442,24 @@ static int __cpufreq_remove_dev_prepare(
 	} else if (cpufreq_driver->stop_cpu) {
 		cpufreq_driver->stop_cpu(policy);
 	}
-
-	return ret;
 }
 
-static int __cpufreq_remove_dev_finish(struct device *dev)
+static void cpufreq_offline_finish(unsigned int cpu)
 {
-	unsigned int cpu = dev->id;
-	int ret;
 	struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
 
 	if (!policy) {
 		pr_debug("%s: No cpu_data found\n", __func__);
-		return -EINVAL;
+		return;
 	}
 
 	/* Only proceed for inactive policies */
 	if (!policy_is_inactive(policy))
-		return 0;
+		return;
 
 	/* If cpu is last user of policy, free policy */
 	if (has_target()) {
-		ret = __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
+		int ret = __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
 		if (ret)
 			pr_err("%s: Failed to exit governor\n", __func__);
 	}
@@ -1477,8 +1471,6 @@ static int __cpufreq_remove_dev_finish(s
 	 */
 	if (cpufreq_driver->exit)
 		cpufreq_driver->exit(policy);
-
-	return 0;
 }
 
 /**
@@ -1495,8 +1487,8 @@ static int cpufreq_remove_dev(struct dev
 		return 0;
 
 	if (cpu_online(cpu)) {
-		__cpufreq_remove_dev_prepare(dev);
-		__cpufreq_remove_dev_finish(dev);
+		cpufreq_offline_prepare(cpu);
+		cpufreq_offline_finish(cpu);
 	}
 
 	cpumask_clear_cpu(cpu, policy->real_cpus);
@@ -2379,11 +2371,11 @@ static int cpufreq_cpu_callback(struct n
 			break;
 
 		case CPU_DOWN_PREPARE:
-			__cpufreq_remove_dev_prepare(dev);
+			cpufreq_offline_prepare(cpu);
 			break;
 
 		case CPU_POST_DEAD:
-			__cpufreq_remove_dev_finish(dev);
+			cpufreq_offline_finish(cpu);
 			break;
 
 		case CPU_DOWN_FAILED:

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193202 — Re: [PATCH 1/7] cpufreq: Rework two functions related to CPU offline

On 27-07-15, 16:03, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Since __cpufreq_remove_dev_prepare() and __cpufreq_remove_dev_finish()
> are about CPU offline rather than about CPU removal, rename them to
> cpufreq_offline_prepare() and cpufreq_offline_finish(), respectively.
> 
> Also change their argument from a struct device pointer to a CPU
> number, because they use the CPU number only internally anyway
> and make them void as their return values are ignored.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |   32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193099 — [PATCH 0/7] cpufreq: Better separation of device addition/removal and online/offline paths

Hi,

On Thursday, July 23, 2015 02:00:23 AM Rafael J. Wysocki wrote:
> Hi,
> 
> As per the recent discussion (http://marc.info/?l=linux-pm&m=143758336123670&w=4),
> separete CPU device addition/removal code paths from the CPU online/offline code
> paths in the cpufreq core a bit better.

A new version of this, with more cleanups included, on top of the linux-next
branch of the linux-pm.git tree (from today).

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193101 — [PATCH 6/7] cpufreq: Pass CPU number to cpufreq_policy_alloc()

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Change cpufreq_policy_alloc() to take a CPU number instead of a CPU
device pointer as its argument, as it is the only function called by
cpufreq_add_dev() taking a device pointer argument at this point.

That will allow us to split the CPU online part from cpufreq_add_dev()
more cleanly going forward.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1090,11 +1090,15 @@ static int cpufreq_add_policy_cpu(struct
 	return 0;
 }
 
-static struct cpufreq_policy *cpufreq_policy_alloc(struct device *dev)
+static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
 {
+	struct device *dev = get_cpu_device(cpu);
 	struct cpufreq_policy *policy;
 	int ret;
 
+	if (WARN_ON(!dev))
+		return NULL;
+
 	policy = kzalloc(sizeof(*policy), GFP_KERNEL);
 	if (!policy)
 		return NULL;
@@ -1122,10 +1126,10 @@ static struct cpufreq_policy *cpufreq_po
 	init_completion(&policy->kobj_unregister);
 	INIT_WORK(&policy->update, handle_update);
 
-	policy->cpu = dev->id;
+	policy->cpu = cpu;
 
 	/* Set this once on allocation */
-	policy->kobj_cpu = dev->id;
+	policy->kobj_cpu = cpu;
 
 	return policy;
 
@@ -1233,7 +1237,7 @@ static int cpufreq_add_dev(struct device
 		up_write(&policy->rwsem);
 	} else {
 		recover_policy = false;
-		policy = cpufreq_policy_alloc(dev);
+		policy = cpufreq_policy_alloc(cpu);
 		if (!policy)
 			return -ENOMEM;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193217 — Re: [PATCH 6/7] cpufreq: Pass CPU number to cpufreq_policy_alloc()

On 27-07-15, 16:07, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Change cpufreq_policy_alloc() to take a CPU number instead of a CPU
> device pointer as its argument, as it is the only function called by
> cpufreq_add_dev() taking a device pointer argument at this point.
> 
> That will allow us to split the CPU online part from cpufreq_add_dev()
> more cleanly going forward.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193103 — [PATCH 3/7] cpufreq: Drop unnecessary label from cpufreq_add_dev()

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The leftover out_release_rwsem label in cpufreq_add_dev() is not
necessary any more and confusing, so drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1237,7 +1237,7 @@ static int cpufreq_add_dev(struct device
 		recover_policy = false;
 		policy = cpufreq_policy_alloc(dev);
 		if (!policy)
-			goto out_release_rwsem;
+			return -ENOMEM;
 	}
 
 	cpumask_copy(policy->cpus, cpumask_of(cpu));
@@ -1372,7 +1372,6 @@ out_exit_policy:
 		cpufreq_driver->exit(policy);
 out_free_policy:
 	cpufreq_policy_free(policy, recover_policy);
-out_release_rwsem:
 	return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193212 — Re: [PATCH 3/7] cpufreq: Drop unnecessary label from cpufreq_add_dev()

On 27-07-15, 16:04, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The leftover out_release_rwsem label in cpufreq_add_dev() is not
> necessary any more and confusing, so drop it.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -1237,7 +1237,7 @@ static int cpufreq_add_dev(struct device
>  		recover_policy = false;
>  		policy = cpufreq_policy_alloc(dev);
>  		if (!policy)
> -			goto out_release_rwsem;
> +			return -ENOMEM;
>  	}
>  
>  	cpumask_copy(policy->cpus, cpumask_of(cpu));
> @@ -1372,7 +1372,6 @@ out_exit_policy:
>  		cpufreq_driver->exit(policy);
>  out_free_policy:
>  	cpufreq_policy_free(policy, recover_policy);
> -out_release_rwsem:
>  	return ret;

There is no need to initialize ret to -ENOMEM now, please get rid of
that as well and add my

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193105 — [PATCH 7/7] cpufreq: Separate CPU device removal from CPU online

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

To separate the CPU online interface from the CPU device removal
one, split cpufreq_online() out of cpufreq_add_dev() and make
cpufreq_cpu_callback() call the former, while the latter will only
be used as the CPU device removal subsystem interface callback.

While at it, notice that the return value of sif->add_dev() is
ignored in bus_probe_device(), so (the new) cpufreq_add_dev()
doesn't need to bother with returning anything different from 0
and cpufreq_online() may be a void function.

Moreover, since the return value of cpufreq_add_policy_cpu() is
going to be ignored now too, make a void function of it as well.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Suggested-by: Russell King <linux@arm.linux.org.uk>
---
 drivers/cpufreq/cpufreq.c |  125 ++++++++++++++++++++++------------------------
 1 file changed, 61 insertions(+), 64 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1056,19 +1056,17 @@ static int cpufreq_init_policy(struct cp
 	return cpufreq_set_policy(policy, &new_policy);
 }
 
-static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
+static void cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
 {
-	int ret = 0;
-
 	/* Has this CPU been taken care of already? */
 	if (cpumask_test_cpu(cpu, policy->cpus))
-		return 0;
+		return;
 
 	if (has_target()) {
-		ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+		int ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 		if (ret) {
 			pr_err("%s: Failed to stop governor\n", __func__);
-			return ret;
+			return;
 		}
 	}
 
@@ -1077,17 +1075,13 @@ static int cpufreq_add_policy_cpu(struct
 	up_write(&policy->rwsem);
 
 	if (has_target()) {
-		ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
+		int ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
 		if (!ret)
 			ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
 
-		if (ret) {
+		if (ret)
 			pr_err("%s: Failed to start governor\n", __func__);
-			return ret;
-		}
 	}
-
-	return 0;
 }
 
 static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
@@ -1191,44 +1185,24 @@ static void cpufreq_policy_free(struct c
 	kfree(policy);
 }
 
-/**
- * cpufreq_add_dev - add a CPU device
- *
- * Adds the cpufreq interface for a CPU device.
- *
- * The Oracle says: try running cpufreq registration/unregistration concurrently
- * with with cpu hotplugging and all hell will break loose. Tried to clean this
- * mess up, but more thorough testing is needed. - Mathieu
- */
-static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
+static void cpufreq_online(unsigned int cpu)
 {
-	unsigned int j, cpu = dev->id;
-	int ret = -ENOMEM;
 	struct cpufreq_policy *policy;
-	unsigned long flags;
 	bool recover_policy;
+	unsigned long flags;
+	unsigned int j;
+	int ret;
 
-	pr_debug("adding CPU %u\n", cpu);
-
-	if (cpu_is_offline(cpu)) {
-		/*
-		 * Only possible if we are here from the subsys_interface add
-		 * callback.  A hotplug notifier will follow and we will handle
-		 * it as CPU online then.  For now, just create the sysfs link,
-		 * unless there is no policy or the link is already present.
-		 */
-		policy = per_cpu(cpufreq_cpu_data, cpu);
-		return policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
-			? add_cpu_dev_symlink(policy, cpu) : 0;
-	}
+	pr_debug("%s: bringing CPU%u online\n", __func__, cpu);
 
 	/* Check if this CPU already has a policy to manage it */
 	policy = per_cpu(cpufreq_cpu_data, cpu);
 	if (policy) {
 		WARN_ON(!cpumask_test_cpu(cpu, policy->related_cpus));
-		if (!policy_is_inactive(policy))
-			return cpufreq_add_policy_cpu(policy, cpu);
-
+		if (!policy_is_inactive(policy)) {
+			cpufreq_add_policy_cpu(policy, cpu);
+			return;
+		}
 		/* This is the only online CPU for the policy.  Start over. */
 		recover_policy = true;
 		down_write(&policy->rwsem);
@@ -1239,7 +1213,7 @@ static int cpufreq_add_dev(struct device
 		recover_policy = false;
 		policy = cpufreq_policy_alloc(cpu);
 		if (!policy)
-			return -ENOMEM;
+			return;
 	}
 
 	cpumask_copy(policy->cpus, cpumask_of(cpu));
@@ -1362,7 +1336,7 @@ static int cpufreq_add_dev(struct device
 
 	pr_debug("initialization complete\n");
 
-	return 0;
+	return;
 
 out_remove_policy_notify:
 	/* cpufreq_policy_free() will notify based on this */
@@ -1374,7 +1348,34 @@ out_exit_policy:
 		cpufreq_driver->exit(policy);
 out_free_policy:
 	cpufreq_policy_free(policy, recover_policy);
-	return ret;
+}
+
+/**
+ * cpufreq_add_dev - the cpufreq interface for a CPU device.
+ * @dev: CPU device.
+ * @sif: Subsystem interface structure pointer (not used)
+ */
+static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
+{
+	unsigned cpu = dev->id;
+
+	dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
+
+	if (cpu_online(cpu)) {
+		cpufreq_online(cpu);
+	} else {
+		/*
+		 * A hotplug notifier will follow and we will handle it as CPU
+		 * online then.  For now, just create the sysfs link, unless
+		 * there is no policy or the link is already present.
+		 */
+		struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
+
+		if (policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus))
+			WARN_ON(add_cpu_dev_symlink(policy, cpu));
+	}
+
+	return 0;
 }
 
 static void cpufreq_offline_prepare(unsigned int cpu)
@@ -2340,27 +2341,23 @@ static int cpufreq_cpu_callback(struct n
 					unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
-	struct device *dev;
 
-	dev = get_cpu_device(cpu);
-	if (dev) {
-		switch (action & ~CPU_TASKS_FROZEN) {
-		case CPU_ONLINE:
-			cpufreq_add_dev(dev, NULL);
-			break;
-
-		case CPU_DOWN_PREPARE:
-			cpufreq_offline_prepare(cpu);
-			break;
-
-		case CPU_POST_DEAD:
-			cpufreq_offline_finish(cpu);
-			break;
-
-		case CPU_DOWN_FAILED:
-			cpufreq_add_dev(dev, NULL);
-			break;
-		}
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_ONLINE:
+		cpufreq_online(cpu);
+		break;
+
+	case CPU_DOWN_PREPARE:
+		cpufreq_offline_prepare(cpu);
+		break;
+
+	case CPU_POST_DEAD:
+		cpufreq_offline_finish(cpu);
+		break;
+
+	case CPU_DOWN_FAILED:
+		cpufreq_online(cpu);
+		break;
 	}
 	return NOTIFY_OK;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193222 — Re: [PATCH 7/7] cpufreq: Separate CPU device removal from CPU online

On 27-07-15, 16:09, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> To separate the CPU online interface from the CPU device removal
> one,

Why do you call this cpu device removal code?

> split cpufreq_online() out of cpufreq_add_dev() and make
> cpufreq_cpu_callback() call the former, while the latter will only
> be used as the CPU device removal subsystem interface callback.
> 
> While at it, notice that the return value of sif->add_dev() is
> ignored in bus_probe_device(), so (the new) cpufreq_add_dev()
> doesn't need to bother with returning anything different from 0
> and cpufreq_online() may be a void function.

That is going to change in 4.3:

https://lkml.org/lkml/2015/6/26/132

> 
> Moreover, since the return value of cpufreq_add_policy_cpu() is
> going to be ignored now too, make a void function of it as well.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Suggested-by: Russell King <linux@arm.linux.org.uk>
> ---
>  drivers/cpufreq/cpufreq.c |  125 ++++++++++++++++++++++------------------------
>  1 file changed, 61 insertions(+), 64 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -1056,19 +1056,17 @@ static int cpufreq_init_policy(struct cp
>  	return cpufreq_set_policy(policy, &new_policy);
>  }
>  
> -static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
> +static void cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
>  {
> -	int ret = 0;
> -
>  	/* Has this CPU been taken care of already? */
>  	if (cpumask_test_cpu(cpu, policy->cpus))
> -		return 0;
> +		return;
>  
>  	if (has_target()) {
> -		ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> +		int ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);

Why should we move the definition of ret here and ...

>  		if (ret) {
>  			pr_err("%s: Failed to stop governor\n", __func__);
> -			return ret;
> +			return;
>  		}
>  	}

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193520 — Re: [PATCH 7/7] cpufreq: Separate CPU device removal from CPU online

On Mon, Jul 27, 2015 at 10:56 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Mon, Jul 27, 2015 at 5:06 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 27-07-15, 16:09, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> To separate the CPU online interface from the CPU device removal
>>> one,
>>
>> Why do you call this cpu device removal code?
>
> By mistake.
>
> Of course, that should be addition/registration.
>
>>> split cpufreq_online() out of cpufreq_add_dev() and make
>>> cpufreq_cpu_callback() call the former, while the latter will only
>>> be used as the CPU device removal subsystem interface callback.
>>>
>>> While at it, notice that the return value of sif->add_dev() is
>>> ignored in bus_probe_device(), so (the new) cpufreq_add_dev()
>>> doesn't need to bother with returning anything different from 0
>>> and cpufreq_online() may be a void function.
>>
>> That is going to change in 4.3:
>>
>> https://lkml.org/lkml/2015/6/26/132
>
> There are some problems with access to klml.org today and I'm not sure
> what you mean.
>
> Can you explain your points in addition to sending links to stuff, please?

OK, I've just seen that patch, but it doesn't modify bus_probe_device() AFAICS.

Plus we also ignore the return value of cpufreq_add_dev() in the
hotplug notifier callback.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193504 — [Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

To separate the CPU online interface from the CPU device
registration, split cpufreq_online() out of cpufreq_add_dev()
and make cpufreq_cpu_callback() call the former, while
cpufreq_add_dev() itself will only be used as the CPU device
addition subsystem interface callback.

While at it, notice that the return value of sif->add_dev() is
ignored in bus_probe_device(), so (the new) cpufreq_add_dev()
doesn't need to bother with returning anything different from 0
and cpufreq_online() may be a void function.

Moreover, since the return value of cpufreq_add_policy_cpu() is
going to be ignored now too, make a void function of it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Suggested-by: Russell King <linux@arm.linux.org.uk>
---
 drivers/cpufreq/cpufreq.c |  121 ++++++++++++++++++++++------------------------
 1 file changed, 60 insertions(+), 61 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1056,19 +1056,19 @@ static int cpufreq_init_policy(struct cp
 	return cpufreq_set_policy(policy, &new_policy);
 }
 
-static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
+static void cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
 {
-	int ret = 0;
+	int ret;
 
 	/* Has this CPU been taken care of already? */
 	if (cpumask_test_cpu(cpu, policy->cpus))
-		return 0;
+		return;
 
 	if (has_target()) {
 		ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 		if (ret) {
 			pr_err("%s: Failed to stop governor\n", __func__);
-			return ret;
+			return;
 		}
 	}
 
@@ -1081,13 +1081,9 @@ static int cpufreq_add_policy_cpu(struct
 		if (!ret)
 			ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
 
-		if (ret) {
+		if (ret)
 			pr_err("%s: Failed to start governor\n", __func__);
-			return ret;
-		}
 	}
-
-	return 0;
 }
 
 static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
@@ -1191,44 +1187,24 @@ static void cpufreq_policy_free(struct c
 	kfree(policy);
 }
 
-/**
- * cpufreq_add_dev - add a CPU device
- *
- * Adds the cpufreq interface for a CPU device.
- *
- * The Oracle says: try running cpufreq registration/unregistration concurrently
- * with with cpu hotplugging and all hell will break loose. Tried to clean this
- * mess up, but more thorough testing is needed. - Mathieu
- */
-static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
+static void cpufreq_online(unsigned int cpu)
 {
-	unsigned int j, cpu = dev->id;
-	int ret;
 	struct cpufreq_policy *policy;
-	unsigned long flags;
 	bool recover_policy;
+	unsigned long flags;
+	unsigned int j;
+	int ret;
 
-	pr_debug("adding CPU %u\n", cpu);
-
-	if (cpu_is_offline(cpu)) {
-		/*
-		 * Only possible if we are here from the subsys_interface add
-		 * callback.  A hotplug notifier will follow and we will handle
-		 * it as CPU online then.  For now, just create the sysfs link,
-		 * unless there is no policy or the link is already present.
-		 */
-		policy = per_cpu(cpufreq_cpu_data, cpu);
-		return policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
-			? add_cpu_dev_symlink(policy, cpu) : 0;
-	}
+	pr_debug("%s: bringing CPU%u online\n", __func__, cpu);
 
 	/* Check if this CPU already has a policy to manage it */
 	policy = per_cpu(cpufreq_cpu_data, cpu);
 	if (policy) {
 		WARN_ON(!cpumask_test_cpu(cpu, policy->related_cpus));
-		if (!policy_is_inactive(policy))
-			return cpufreq_add_policy_cpu(policy, cpu);
-
+		if (!policy_is_inactive(policy)) {
+			cpufreq_add_policy_cpu(policy, cpu);
+			return;
+		}
 		/* This is the only online CPU for the policy.  Start over. */
 		recover_policy = true;
 		down_write(&policy->rwsem);
@@ -1239,7 +1215,7 @@ static int cpufreq_add_dev(struct device
 		recover_policy = false;
 		policy = cpufreq_policy_alloc(cpu);
 		if (!policy)
-			return -ENOMEM;
+			return;
 	}
 
 	cpumask_copy(policy->cpus, cpumask_of(cpu));
@@ -1362,7 +1338,7 @@ static int cpufreq_add_dev(struct device
 
 	pr_debug("initialization complete\n");
 
-	return 0;
+	return;
 
 out_remove_policy_notify:
 	/* cpufreq_policy_free() will notify based on this */
@@ -1374,7 +1350,34 @@ out_exit_policy:
 		cpufreq_driver->exit(policy);
 out_free_policy:
 	cpufreq_policy_free(policy, recover_policy);
-	return ret;
+}
+
+/**
+ * cpufreq_add_dev - the cpufreq interface for a CPU device.
+ * @dev: CPU device.
+ * @sif: Subsystem interface structure pointer (not used)
+ */
+static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
+{
+	unsigned cpu = dev->id;
+
+	dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
+
+	if (cpu_online(cpu)) {
+		cpufreq_online(cpu);
+	} else {
+		/*
+		 * A hotplug notifier will follow and we will handle it as CPU
+		 * online then.  For now, just create the sysfs link, unless
+		 * there is no policy or the link is already present.
+		 */
+		struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
+
+		if (policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus))
+			WARN_ON(add_cpu_dev_symlink(policy, cpu));
+	}
+
+	return 0;
 }
 
 static void cpufreq_offline_prepare(unsigned int cpu)
@@ -2340,27 +2343,23 @@ static int cpufreq_cpu_callback(struct n
 					unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
-	struct device *dev;
 
-	dev = get_cpu_device(cpu);
-	if (dev) {
-		switch (action & ~CPU_TASKS_FROZEN) {
-		case CPU_ONLINE:
-			cpufreq_add_dev(dev, NULL);
-			break;
-
-		case CPU_DOWN_PREPARE:
-			cpufreq_offline_prepare(cpu);
-			break;
-
-		case CPU_POST_DEAD:
-			cpufreq_offline_finish(cpu);
-			break;
-
-		case CPU_DOWN_FAILED:
-			cpufreq_add_dev(dev, NULL);
-			break;
-		}
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_ONLINE:
+		cpufreq_online(cpu);
+		break;
+
+	case CPU_DOWN_PREPARE:
+		cpufreq_offline_prepare(cpu);
+		break;
+
+	case CPU_POST_DEAD:
+		cpufreq_offline_finish(cpu);
+		break;
+
+	case CPU_DOWN_FAILED:
+		cpufreq_online(cpu);
+		break;
 	}
 	return NOTIFY_OK;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194745 — [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

To separate the CPU online interface from the CPU device
registration, split cpufreq_online() out of cpufreq_add_dev()
and make cpufreq_cpu_callback() call the former, while
cpufreq_add_dev() itself will only be used as the CPU device
addition subsystem interface callback.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Suggested-by: Russell King <linux@arm.linux.org.uk>
---

cpufreq_online() returns an int and cpufreq_add_dev() propagates that to the
caller in this version.

That said I don't agree with using that to defer the platform device probing
in cpufreq-dt.  That's just over the top to me.

---
 drivers/cpufreq/cpufreq.c |   96 +++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 46 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1191,36 +1191,15 @@ static void cpufreq_policy_free(struct c
 	kfree(policy);
 }
 
-/**
- * cpufreq_add_dev - add a CPU device
- *
- * Adds the cpufreq interface for a CPU device.
- *
- * The Oracle says: try running cpufreq registration/unregistration concurrently
- * with with cpu hotplugging and all hell will break loose. Tried to clean this
- * mess up, but more thorough testing is needed. - Mathieu
- */
-static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
+static int cpufreq_online(unsigned int cpu)
 {
-	unsigned int j, cpu = dev->id;
-	int ret;
 	struct cpufreq_policy *policy;
-	unsigned long flags;
 	bool recover_policy;
+	unsigned long flags;
+	unsigned int j;
+	int ret;
 
-	pr_debug("adding CPU %u\n", cpu);
-
-	if (cpu_is_offline(cpu)) {
-		/*
-		 * Only possible if we are here from the subsys_interface add
-		 * callback.  A hotplug notifier will follow and we will handle
-		 * it as CPU online then.  For now, just create the sysfs link,
-		 * unless there is no policy or the link is already present.
-		 */
-		policy = per_cpu(cpufreq_cpu_data, cpu);
-		return policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
-			? add_cpu_dev_symlink(policy, cpu) : 0;
-	}
+	pr_debug("%s: bringing CPU%u online\n", __func__, cpu);
 
 	/* Check if this CPU already has a policy to manage it */
 	policy = per_cpu(cpufreq_cpu_data, cpu);
@@ -1377,6 +1356,35 @@ out_free_policy:
 	return ret;
 }
 
+/**
+ * cpufreq_add_dev - the cpufreq interface for a CPU device.
+ * @dev: CPU device.
+ * @sif: Subsystem interface structure pointer (not used)
+ */
+static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
+{
+	unsigned cpu = dev->id;
+	int ret;
+
+	dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
+
+	if (cpu_online(cpu)) {
+		ret = cpufreq_online(cpu);
+	} else {
+		/*
+		 * A hotplug notifier will follow and we will handle it as CPU
+		 * online then.  For now, just create the sysfs link, unless
+		 * there is no policy or the link is already present.
+		 */
+		struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
+
+		ret = policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
+			? add_cpu_dev_symlink(policy, cpu) : 0;
+	}
+
+	return ret;
+}
+
 static void cpufreq_offline_prepare(unsigned int cpu)
 {
 	struct cpufreq_policy *policy;
@@ -2340,27 +2348,23 @@ static int cpufreq_cpu_callback(struct n
 					unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
-	struct device *dev;
 
-	dev = get_cpu_device(cpu);
-	if (dev) {
-		switch (action & ~CPU_TASKS_FROZEN) {
-		case CPU_ONLINE:
-			cpufreq_add_dev(dev, NULL);
-			break;
-
-		case CPU_DOWN_PREPARE:
-			cpufreq_offline_prepare(cpu);
-			break;
-
-		case CPU_POST_DEAD:
-			cpufreq_offline_finish(cpu);
-			break;
-
-		case CPU_DOWN_FAILED:
-			cpufreq_add_dev(dev, NULL);
-			break;
-		}
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_ONLINE:
+		cpufreq_online(cpu);
+		break;
+
+	case CPU_DOWN_PREPARE:
+		cpufreq_offline_prepare(cpu);
+		break;
+
+	case CPU_POST_DEAD:
+		cpufreq_offline_finish(cpu);
+		break;
+
+	case CPU_DOWN_FAILED:
+		cpufreq_online(cpu);
+		break;
 	}
 	return NOTIFY_OK;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194747 — [PATCH] cpufreq: Replace recover_policy with new_policy in cpufreq_online()

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The recover_policy is unsed in cpufreq_online() to indicate whether
a new policy object is created or an existing one is reinitialized.

The "recover" part of the name is slightly confusing (it should be
"reinitialization" rather than "recovery") and the logical not (!)
operator is applied to it in almost all of the checks it is used in,
so replace that variable with a new one called "new_policy" that
will be true in the case of a new policy creation.

While at it, drop one of the labels that is jumped to from only
one spot.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

One extra cleanup on top of https://patchwork.kernel.org/patch/6888751/

---
 drivers/cpufreq/cpufreq.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1194,7 +1194,7 @@ static void cpufreq_policy_free(struct c
 static int cpufreq_online(unsigned int cpu)
 {
 	struct cpufreq_policy *policy;
-	bool recover_policy;
+	bool new_policy;
 	unsigned long flags;
 	unsigned int j;
 	int ret;
@@ -1209,13 +1209,13 @@ static int cpufreq_online(unsigned int c
 			return cpufreq_add_policy_cpu(policy, cpu);
 
 		/* This is the only online CPU for the policy.  Start over. */
-		recover_policy = true;
+		new_policy = false;
 		down_write(&policy->rwsem);
 		policy->cpu = cpu;
 		policy->governor = NULL;
 		up_write(&policy->rwsem);
 	} else {
-		recover_policy = false;
+		new_policy = true;
 		policy = cpufreq_policy_alloc(cpu);
 		if (!policy)
 			return -ENOMEM;
@@ -1234,7 +1234,7 @@ static int cpufreq_online(unsigned int c
 
 	down_write(&policy->rwsem);
 
-	if (!recover_policy) {
+	if (new_policy) {
 		/* related_cpus should at least include policy->cpus. */
 		cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
 		/* Remember CPUs present at the policy creation time. */
@@ -1247,7 +1247,7 @@ static int cpufreq_online(unsigned int c
 	 */
 	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
 
-	if (!recover_policy) {
+	if (new_policy) {
 		policy->user_policy.min = policy->min;
 		policy->user_policy.max = policy->max;
 
@@ -1308,7 +1308,7 @@ static int cpufreq_online(unsigned int c
 	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				     CPUFREQ_START, policy);
 
-	if (!recover_policy) {
+	if (new_policy) {
 		ret = cpufreq_add_dev_interface(policy);
 		if (ret)
 			goto out_exit_policy;
@@ -1324,10 +1324,12 @@ static int cpufreq_online(unsigned int c
 	if (ret) {
 		pr_err("%s: Failed to initialize policy for cpu: %d (%d)\n",
 		       __func__, cpu, ret);
-		goto out_remove_policy_notify;
+		/* cpufreq_policy_free() will notify based on this */
+		new_policy = false;
+		goto out_exit_policy;
 	}
 
-	if (!recover_policy) {
+	if (new_policy) {
 		policy->user_policy.policy = policy->policy;
 		policy->user_policy.governor = policy->governor;
 	}
@@ -1343,16 +1345,13 @@ static int cpufreq_online(unsigned int c
 
 	return 0;
 
-out_remove_policy_notify:
-	/* cpufreq_policy_free() will notify based on this */
-	recover_policy = true;
 out_exit_policy:
 	up_write(&policy->rwsem);
 
 	if (cpufreq_driver->exit)
 		cpufreq_driver->exit(policy);
 out_free_policy:
-	cpufreq_policy_free(policy, recover_policy);
+	cpufreq_policy_free(policy, !new_policy);
 	return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194812 — Re: [PATCH] cpufreq: Replace recover_policy with new_policy in cpufreq_online()

On 29-07-15, 03:08, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The recover_policy is unsed in cpufreq_online() to indicate whether
> a new policy object is created or an existing one is reinitialized.
> 
> The "recover" part of the name is slightly confusing (it should be
> "reinitialization" rather than "recovery") and the logical not (!)
> operator is applied to it in almost all of the checks it is used in,
> so replace that variable with a new one called "new_policy" that
> will be true in the case of a new policy creation.
> 
> While at it, drop one of the labels that is jumped to from only
> one spot.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> One extra cleanup on top of https://patchwork.kernel.org/patch/6888751/
> 
> ---
>  drivers/cpufreq/cpufreq.c |   23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194808 — Re: [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online

On 29-07-15, 03:03, Rafael J. Wysocki wrote:
> +static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
> +{
> +	unsigned cpu = dev->id;
> +	int ret;
> +
> +	dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
> +
> +	if (cpu_online(cpu)) {
> +		ret = cpufreq_online(cpu);

I will do return right here ...

> +	} else {

... and this else will not be required anymore.

> +		/*
> +		 * A hotplug notifier will follow and we will handle it as CPU
> +		 * online then.  For now, just create the sysfs link, unless
> +		 * there is no policy or the link is already present.
> +		 */
> +		struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
> +
> +		ret = policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
> +			? add_cpu_dev_symlink(policy, cpu) : 0;
> +	}
> +
> +	return ret;
> +}

Looks good otherwise.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195225 — Re: [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online

On 29-07-15, 16:02, Rafael J. Wysocki wrote:
> You'd still need policy, so its definition and initialization would stay there.
> 
> The only thing you can save by doing that change is the ret variable,
> but I like the code more the way it is.

Okay, that's fine.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195230 — Re: [Update 2x][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online

Hi Viresh,

On Wed, Jul 29, 2015 at 7:32 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-07-15, 03:03, Rafael J. Wysocki wrote:
>> +static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
>> +{
>> +     unsigned cpu = dev->id;
>> +     int ret;
>> +
>> +     dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
>> +
>> +     if (cpu_online(cpu)) {
>> +             ret = cpufreq_online(cpu);
>
> I will do return right here ...
>
>> +     } else {
>
> ... and this else will not be required anymore.

No.

You'd still need policy, so its definition and initialization would stay there.

The only thing you can save by doing that change is the ret variable,
but I like the code more the way it is.

>> +             /*
>> +              * A hotplug notifier will follow and we will handle it as CPU
>> +              * online then.  For now, just create the sysfs link, unless
>> +              * there is no policy or the link is already present.
>> +              */
>> +             struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
>> +
>> +             ret = policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
>> +                     ? add_cpu_dev_symlink(policy, cpu) : 0;
>> +     }
>> +
>> +     return ret;
>> +}
>
> Looks good otherwise.
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web