Re: [Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online
Hi Viresh, On Tue, Jul 28, 2015 at 4:20 AM, Viresh Kumar wrote: > On 27-07-15, 23:55, Rafael J. Wysocki wrote: >> +static void cpufreq_online(unsigned int cpu) > > As I said in the earlier message, I need the return value of this to > be used for add_dev() callback. Which is required to retry probing the > device if it wasn't ready on a earlier call, i.e. it returns > -EPROBE_DEFER. My other patch already fixes the subsys layer for this. If this is the patch I've looked at, it doesn't fix this. It only fixes one case. It also doesn't fix the hotplug notifier failure case. I can retain the return value, but we need to be consistent about using it. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online
Hi Viresh, On Tue, Jul 28, 2015 at 4:20 AM, Viresh Kumar viresh.ku...@linaro.org wrote: On 27-07-15, 23:55, Rafael J. Wysocki wrote: +static void cpufreq_online(unsigned int cpu) As I said in the earlier message, I need the return value of this to be used for add_dev() callback. Which is required to retry probing the device if it wasn't ready on a earlier call, i.e. it returns -EPROBE_DEFER. My other patch already fixes the subsys layer for this. If this is the patch I've looked at, it doesn't fix this. It only fixes one case. It also doesn't fix the hotplug notifier failure case. I can retain the return value, but we need to be consistent about using it. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online
On 27-07-15, 23:55, Rafael J. Wysocki wrote: > +static void cpufreq_online(unsigned int cpu) As I said in the earlier message, I need the return value of this to be used for add_dev() callback. Which is required to retry probing the device if it wasn't ready on a earlier call, i.e. it returns -EPROBE_DEFER. My other patch already fixes the subsys layer for this. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online
From: Rafael J. Wysocki
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
Suggested-by: Russell King
---
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, _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(>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
[Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online
From: Rafael J. Wysocki rafael.j.wyso...@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.wyso...@intel.com
Suggested-by: Russell King li...@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:
Re: [Update][PATCH 7/7] cpufreq: Separate CPU device registration from CPU online
On 27-07-15, 23:55, Rafael J. Wysocki wrote: +static void cpufreq_online(unsigned int cpu) As I said in the earlier message, I need the return value of this to be used for add_dev() callback. Which is required to retry probing the device if it wasn't ready on a earlier call, i.e. it returns -EPROBE_DEFER. My other patch already fixes the subsys layer for this. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
