* Re: [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void
[not found] ` <b361f4df33e414ecd2e8a25959746e84da789d6a.1380703248.git.viresh.kumar@linaro.org>
@ 2013-10-03 7:09 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:09 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> lock_policy_rwsem_{read|write}() currently has return type of int but it always
> return zero and hence its return type must be void instead. This patch makes its
> return type void and fixes all users of it as well.
>
> Reported-by: Jon Medhurst<tixy@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> drivers/cpufreq/cpufreq.c | 38 +++++++++++---------------------------
> 1 file changed, 11 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 04548f7..eb993d9 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -67,13 +67,11 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
> static DEFINE_PER_CPU(struct rw_semaphore, cpu_policy_rwsem);
>
> #define lock_policy_rwsem(mode, cpu) \
> -static int lock_policy_rwsem_##mode(int cpu) \
> +static void lock_policy_rwsem_##mode(int cpu) \
> { \
> struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
> BUG_ON(!policy); \
> down_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
> - \
> - return 0; \
> }
>
> lock_policy_rwsem(read, cpu);
> @@ -653,13 +651,12 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> {
> struct cpufreq_policy *policy = to_policy(kobj);
> struct freq_attr *fattr = to_attr(attr);
> - ssize_t ret = -EINVAL;
> + ssize_t ret;
>
> if (!down_read_trylock(&cpufreq_rwsem))
> - goto exit;
> + return -EINVAL;
>
> - if (lock_policy_rwsem_read(policy->cpu) < 0)
> - goto up_read;
> + lock_policy_rwsem_read(policy->cpu);
>
> if (fattr->show)
> ret = fattr->show(policy, buf);
> @@ -667,10 +664,8 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> ret = -EIO;
>
> unlock_policy_rwsem_read(policy->cpu);
> -
> -up_read:
> up_read(&cpufreq_rwsem);
> -exit:
> +
> return ret;
> }
>
> @@ -689,8 +684,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
> if (!down_read_trylock(&cpufreq_rwsem))
> goto unlock;
>
> - if (lock_policy_rwsem_write(policy->cpu) < 0)
> - goto up_read;
> + lock_policy_rwsem_write(policy->cpu);
>
> if (fattr->store)
> ret = fattr->store(policy, buf, count);
> @@ -699,7 +693,6 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
>
> unlock_policy_rwsem_write(policy->cpu);
>
> -up_read:
> up_read(&cpufreq_rwsem);
> unlock:
> put_online_cpus();
> @@ -1147,7 +1140,7 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
> if (ret) {
> pr_err("%s: Failed to move kobj: %d", __func__, ret);
>
> - WARN_ON(lock_policy_rwsem_write(old_cpu));
> + lock_policy_rwsem_write(old_cpu);
> cpumask_set_cpu(old_cpu, policy->cpus);
> unlock_policy_rwsem_write(old_cpu);
>
> @@ -1243,7 +1236,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> return -EINVAL;
> }
>
> - WARN_ON(lock_policy_rwsem_write(cpu));
> + lock_policy_rwsem_write(cpu);
> cpus = cpumask_weight(policy->cpus);
>
> if (cpus > 1)
> @@ -1466,14 +1459,11 @@ unsigned int cpufreq_get(unsigned int cpu)
> if (!down_read_trylock(&cpufreq_rwsem))
> return 0;
>
> - if (unlikely(lock_policy_rwsem_read(cpu)))
> - goto out_policy;
> + lock_policy_rwsem_read(cpu);
>
> ret_freq = __cpufreq_get(cpu);
>
> unlock_policy_rwsem_read(cpu);
> -
> -out_policy:
> up_read(&cpufreq_rwsem);
>
> return ret_freq;
> @@ -1697,14 +1687,12 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
> {
> int ret = -EINVAL;
>
> - if (unlikely(lock_policy_rwsem_write(policy->cpu)))
> - goto fail;
> + lock_policy_rwsem_write(policy->cpu);
>
> ret = __cpufreq_driver_target(policy, target_freq, relation);
>
> unlock_policy_rwsem_write(policy->cpu);
>
> -fail:
> return ret;
> }
> EXPORT_SYMBOL_GPL(cpufreq_driver_target);
> @@ -1995,10 +1983,7 @@ int cpufreq_update_policy(unsigned int cpu)
> goto no_policy;
> }
>
> - if (unlikely(lock_policy_rwsem_write(cpu))) {
> - ret = -EINVAL;
> - goto fail;
> - }
> + lock_policy_rwsem_write(cpu);
>
> pr_debug("updating policy for CPU %u\n", cpu);
> memcpy(&new_policy, policy, sizeof(*policy));
> @@ -2027,7 +2012,6 @@ int cpufreq_update_policy(unsigned int cpu)
>
> unlock_policy_rwsem_write(cpu);
>
> -fail:
> cpufreq_cpu_put(policy);
> no_policy:
> return ret;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem
[not found] ` <fef9eba980ca340b3f1b1a56e92c84f6bb0f5d81.1380703248.git.viresh.kumar@linaro.org>
@ 2013-10-03 7:23 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:23 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> We have per-cpu cpu_policy_rwsem for cpufreq core, but we never use all of them.
> We always use rwsem of policy->cpu and so we can actually make this rwsem per
> policy instead.
>
> This patch does this change. With this change other tricky situations are also
> avoided now, like which lock to take while we are changing policy->cpu, etc.
>
> Suggested-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
This is a very nice improvement!
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> drivers/cpufreq/cpufreq.c | 110 +++++++++++++---------------------------------
> include/linux/cpufreq.h | 14 ++++++
> 2 files changed, 45 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index eb993d9..ae866b1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -48,47 +48,6 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
> #endif
>
> /*
> - * cpu_policy_rwsem is a per CPU reader-writer semaphore designed to cure
> - * all cpufreq/hotplug/workqueue/etc related lock issues.
> - *
> - * The rules for this semaphore:
> - * - Any routine that wants to read from the policy structure will
> - * do a down_read on this semaphore.
> - * - Any routine that will write to the policy structure and/or may take away
> - * the policy altogether (eg. CPU hotplug), will hold this lock in write
> - * mode before doing so.
> - *
> - * Additional rules:
> - * - Governor routines that can be called in cpufreq hotplug path should not
> - * take this sem as top level hotplug notifier handler takes this.
> - * - Lock should not be held across
> - * __cpufreq_governor(data, CPUFREQ_GOV_STOP);
> - */
> -static DEFINE_PER_CPU(struct rw_semaphore, cpu_policy_rwsem);
> -
> -#define lock_policy_rwsem(mode, cpu) \
> -static void lock_policy_rwsem_##mode(int cpu) \
> -{ \
> - struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
> - BUG_ON(!policy); \
> - down_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
> -}
> -
> -lock_policy_rwsem(read, cpu);
> -lock_policy_rwsem(write, cpu);
> -
> -#define unlock_policy_rwsem(mode, cpu) \
> -static void unlock_policy_rwsem_##mode(int cpu) \
> -{ \
> - struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
> - BUG_ON(!policy); \
> - up_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
> -}
> -
> -unlock_policy_rwsem(read, cpu);
> -unlock_policy_rwsem(write, cpu);
> -
> -/*
> * rwsem to guarantee that cpufreq driver module doesn't unload during critical
> * sections
> */
> @@ -656,14 +615,14 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> if (!down_read_trylock(&cpufreq_rwsem))
> return -EINVAL;
>
> - lock_policy_rwsem_read(policy->cpu);
> + down_read(&policy->rwsem);
>
> if (fattr->show)
> ret = fattr->show(policy, buf);
> else
> ret = -EIO;
>
> - unlock_policy_rwsem_read(policy->cpu);
> + up_read(&policy->rwsem);
> up_read(&cpufreq_rwsem);
>
> return ret;
> @@ -684,14 +643,14 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
> if (!down_read_trylock(&cpufreq_rwsem))
> goto unlock;
>
> - lock_policy_rwsem_write(policy->cpu);
> + down_write(&policy->rwsem);
>
> if (fattr->store)
> ret = fattr->store(policy, buf, count);
> else
> ret = -EIO;
>
> - unlock_policy_rwsem_write(policy->cpu);
> + up_write(&policy->rwsem);
>
> up_read(&cpufreq_rwsem);
> unlock:
> @@ -868,7 +827,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
> }
> }
>
> - lock_policy_rwsem_write(policy->cpu);
> + down_write(&policy->rwsem);
>
> write_lock_irqsave(&cpufreq_driver_lock, flags);
>
> @@ -876,7 +835,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
> per_cpu(cpufreq_cpu_data, cpu) = policy;
> write_unlock_irqrestore(&cpufreq_driver_lock, flags);
>
> - unlock_policy_rwsem_write(policy->cpu);
> + up_write(&policy->rwsem);
>
> if (has_target) {
> if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
> @@ -923,6 +882,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void)
> goto err_free_cpumask;
>
> INIT_LIST_HEAD(&policy->policy_list);
> + init_rwsem(&policy->rwsem);
> +
> return policy;
>
> err_free_cpumask:
> @@ -945,19 +906,12 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
> if (cpu == policy->cpu)
> return;
>
> - /*
> - * Take direct locks as lock_policy_rwsem_write wouldn't work here.
> - * Also lock for last cpu is enough here as contention will happen only
> - * after policy->cpu is changed and after it is changed, other threads
> - * will try to acquire lock for new cpu. And policy is already updated
> - * by then.
> - */
> - down_write(&per_cpu(cpu_policy_rwsem, policy->cpu));
> + down_write(&policy->rwsem);
>
> policy->last_cpu = policy->cpu;
> policy->cpu = cpu;
>
> - up_write(&per_cpu(cpu_policy_rwsem, policy->last_cpu));
> + up_write(&policy->rwsem);
>
> #ifdef CONFIG_CPU_FREQ_TABLE
> cpufreq_frequency_table_update_policy_cpu(policy);
> @@ -1140,9 +1094,9 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
> if (ret) {
> pr_err("%s: Failed to move kobj: %d", __func__, ret);
>
> - lock_policy_rwsem_write(old_cpu);
> + down_write(&policy->rwsem);
> cpumask_set_cpu(old_cpu, policy->cpus);
> - unlock_policy_rwsem_write(old_cpu);
> + up_write(&policy->rwsem);
>
> ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
> "cpufreq");
> @@ -1193,9 +1147,9 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
> policy->governor->name, CPUFREQ_NAME_LEN);
> #endif
>
> - lock_policy_rwsem_read(cpu);
> + down_read(&policy->rwsem);
> cpus = cpumask_weight(policy->cpus);
> - unlock_policy_rwsem_read(cpu);
> + up_read(&policy->rwsem);
>
> if (cpu != policy->cpu) {
> if (!frozen)
> @@ -1236,12 +1190,12 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> return -EINVAL;
> }
>
> - lock_policy_rwsem_write(cpu);
> + down_write(&policy->rwsem);
> cpus = cpumask_weight(policy->cpus);
>
> if (cpus > 1)
> cpumask_clear_cpu(cpu, policy->cpus);
> - unlock_policy_rwsem_write(cpu);
> + up_write(&policy->rwsem);
>
> /* If cpu is last user of policy, free policy */
> if (cpus == 1) {
> @@ -1256,10 +1210,10 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> }
>
> if (!frozen) {
> - lock_policy_rwsem_read(cpu);
> + down_read(&policy->rwsem);
> kobj = &policy->kobj;
> cmp = &policy->kobj_unregister;
> - unlock_policy_rwsem_read(cpu);
> + up_read(&policy->rwsem);
> kobject_put(kobj);
>
> /*
> @@ -1451,19 +1405,22 @@ static unsigned int __cpufreq_get(unsigned int cpu)
> */
> unsigned int cpufreq_get(unsigned int cpu)
> {
> + struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
> unsigned int ret_freq = 0;
>
> if (cpufreq_disabled() || !cpufreq_driver)
> return -ENOENT;
>
> + BUG_ON(!policy);
> +
> if (!down_read_trylock(&cpufreq_rwsem))
> return 0;
>
> - lock_policy_rwsem_read(cpu);
> + down_read(&policy->rwsem);
>
> ret_freq = __cpufreq_get(cpu);
>
> - unlock_policy_rwsem_read(cpu);
> + up_read(&policy->rwsem);
> up_read(&cpufreq_rwsem);
>
> return ret_freq;
> @@ -1687,11 +1644,11 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
> {
> int ret = -EINVAL;
>
> - lock_policy_rwsem_write(policy->cpu);
> + down_write(&policy->rwsem);
>
> ret = __cpufreq_driver_target(policy, target_freq, relation);
>
> - unlock_policy_rwsem_write(policy->cpu);
> + up_write(&policy->rwsem);
>
> return ret;
> }
> @@ -1922,10 +1879,10 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
> /* end old governor */
> if (policy->governor) {
> __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> - unlock_policy_rwsem_write(new_policy->cpu);
> + up_write(&new_policy->rwsem);
> __cpufreq_governor(policy,
> CPUFREQ_GOV_POLICY_EXIT);
> - lock_policy_rwsem_write(new_policy->cpu);
> + down_write(&new_policy->rwsem);
> }
>
> /* start new governor */
> @@ -1934,10 +1891,10 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
> if (!__cpufreq_governor(policy, CPUFREQ_GOV_START)) {
> failed = 0;
> } else {
> - unlock_policy_rwsem_write(new_policy->cpu);
> + up_write(&new_policy->rwsem);
> __cpufreq_governor(policy,
> CPUFREQ_GOV_POLICY_EXIT);
> - lock_policy_rwsem_write(new_policy->cpu);
> + down_write(&new_policy->rwsem);
> }
> }
>
> @@ -1983,7 +1940,7 @@ int cpufreq_update_policy(unsigned int cpu)
> goto no_policy;
> }
>
> - lock_policy_rwsem_write(cpu);
> + down_write(&policy->rwsem);
>
> pr_debug("updating policy for CPU %u\n", cpu);
> memcpy(&new_policy, policy, sizeof(*policy));
> @@ -2010,7 +1967,7 @@ int cpufreq_update_policy(unsigned int cpu)
>
> ret = __cpufreq_set_policy(policy, &new_policy);
>
> - unlock_policy_rwsem_write(cpu);
> + up_write(&policy->rwsem);
>
> cpufreq_cpu_put(policy);
> no_policy:
> @@ -2167,14 +2124,9 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
>
> static int __init cpufreq_core_init(void)
> {
> - int cpu;
> -
> if (cpufreq_disabled())
> return -ENODEV;
>
> - for_each_possible_cpu(cpu)
> - init_rwsem(&per_cpu(cpu_policy_rwsem, cpu));
> -
> cpufreq_global_kobject = kobject_create();
> BUG_ON(!cpufreq_global_kobject);
> register_syscore_ops(&cpufreq_syscore_ops);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 6b199ed..a72bac2 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -85,6 +85,20 @@ struct cpufreq_policy {
> struct list_head policy_list;
> struct kobject kobj;
> struct completion kobj_unregister;
> +
> + /*
> + * The rules for this semaphore:
> + * - Any routine that wants to read from the policy structure will
> + * do a down_read on this semaphore.
> + * - Any routine that will write to the policy structure and/or may take away
> + * the policy altogether (eg. CPU hotplug), will hold this lock in write
> + * mode before doing so.
> + *
> + * Additional rules:
> + * - Lock should not be held across
> + * __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
> + */
> + struct rw_semaphore rwsem;
> };
>
> /* Only for ACPI */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev()
[not found] ` <0d25692a141042a3d4d6fab310029e96fbe57822.1380703248.git.viresh.kumar@linaro.org>
@ 2013-10-03 7:25 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:25 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> Some section of kerneldoc comment for __cpufreq_remove_dev() is invalid now.
> Remove it.
>
> Suggested-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> ---
> drivers/cpufreq/cpufreq.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index ae866b1..d80cbe6 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1260,8 +1260,6 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> * __cpufreq_remove_dev - remove a CPU device
> *
> * Removes the cpufreq interface for a CPU device.
> - * Caller should already have policy_rwsem in write mode for this CPU.
> - * This routine frees the rwsem before returning.
> */
> static inline int __cpufreq_remove_dev(struct device *dev,
> struct subsys_interface *sif,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 04/11] cpufreq: Remove extra blank line
[not found] ` <f70f2486e3107448d1d3b835b78ad20724fdd252.1380703248.git.viresh.kumar@linaro.org>
@ 2013-10-03 7:26 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:26 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> We don't need a blank line just at start of a block, lets remove it.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> ---
> drivers/cpufreq/cpufreq.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d80cbe6..d149caa 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1155,7 +1155,6 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
> if (!frozen)
> sysfs_remove_link(&dev->kobj, "cpufreq");
> } else if (cpus > 1) {
> -
> new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen);
> if (new_cpu >= 0) {
> update_policy_cpu(policy, new_cpu);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator
[not found] ` <7b34bf61846bed19e4a7e8ee18a55e717c570fef.1380703248.git.viresh.kumar@linaro.org>
@ 2013-10-03 7:32 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:32 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> Currently cpufreq_driver's flags are defined directly using 0x1, 0x2, 0x4, 0x8,
> etc.. As the list grows it doesn't stays much readable..
>
> Lets use bitwise shift operator << to generate these numbers for respective
> positions.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> include/linux/cpufreq.h | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index a72bac2..9321059 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -225,13 +225,14 @@ struct cpufreq_driver {
> };
>
> /* flags */
> -#define CPUFREQ_STICKY 0x01 /* the driver isn't removed even if
> - * all ->init() calls failed */
> -#define CPUFREQ_CONST_LOOPS 0x02 /* loops_per_jiffy or other kernel
> - * "constants" aren't affected by
> - * frequency transitions */
> -#define CPUFREQ_PM_NO_WARN 0x04 /* don't warn on suspend/resume speed
> - * mismatches */
> +#define CPUFREQ_STICKY (1 << 0) /* driver isn't removed even if
> + all ->init() calls failed */
> +#define CPUFREQ_CONST_LOOPS (1 << 1) /* loops_per_jiffy or other
> + kernel "constants" aren't
> + affected by frequency
> + transitions */
> +#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
> + speed mismatches */
>
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY
[not found] ` <4c9af8dda1c4f968e08815632cea3987d0426127.1380703248.git.viresh.kumar@linaro.org>
@ 2013-10-03 7:39 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:39 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> Lets use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY instead
> of a separate field within cpufreq_driver. This will save some bytes for us.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> drivers/cpufreq/arm_big_little.c | 4 ++--
> drivers/cpufreq/cpufreq.c | 2 +-
> drivers/cpufreq/cpufreq_governor.h | 5 ++++-
> include/linux/cpufreq.h | 15 ++++++++-------
> 4 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> index 9acfb82..2c7c134 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -213,13 +213,13 @@ static struct freq_attr *bL_cpufreq_attr[] = {
>
> static struct cpufreq_driver bL_cpufreq_driver = {
> .name = "arm-big-little",
> - .flags = CPUFREQ_STICKY,
> + .flags = CPUFREQ_STICKY |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> .verify = bL_cpufreq_verify_policy,
> .target = bL_cpufreq_set_target,
> .get = bL_cpufreq_get,
> .init = bL_cpufreq_init,
> .exit = bL_cpufreq_exit,
> - .have_governor_per_policy = true,
> .attr = bL_cpufreq_attr,
> };
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 267ae99..23e0a40 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -92,7 +92,7 @@ static DEFINE_MUTEX(cpufreq_governor_mutex);
>
> bool have_governor_per_policy(void)
> {
> - return cpufreq_driver->have_governor_per_policy;
> + return !!(cpufreq_driver->flags & CPUFREQ_HAVE_GOVERNOR_PER_POLICY);
> }
> EXPORT_SYMBOL_GPL(have_governor_per_policy);
>
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index 88cd39f..b5f2b86 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -191,7 +191,10 @@ struct common_dbs_data {
> struct attribute_group *attr_group_gov_sys; /* one governor - system */
> struct attribute_group *attr_group_gov_pol; /* one governor - policy */
>
> - /* Common data for platforms that don't set have_governor_per_policy */
> + /*
> + * Common data for platforms that don't set
> + * CPUFREQ_HAVE_GOVERNOR_PER_POLICY
> + */
> struct dbs_data *gdbs_data;
>
> struct cpu_dbs_common_info *(*get_cpu_cdbs)(int cpu);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 9321059..6c9c365 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -194,13 +194,6 @@ __ATTR(_name, 0644, show_##_name, store_##_name)
> struct cpufreq_driver {
> char name[CPUFREQ_NAME_LEN];
> u8 flags;
> - /*
> - * This should be set by platforms having multiple clock-domains, i.e.
> - * supporting multiple policies. With this sysfs directories of governor
> - * would be created in cpu/cpu<num>/cpufreq/ directory and so they can
> - * use the same governor with different tunables for different clusters.
> - */
> - bool have_governor_per_policy;
>
> /* needed by all drivers */
> int (*init) (struct cpufreq_policy *policy);
> @@ -234,6 +227,14 @@ struct cpufreq_driver {
> #define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
> speed mismatches */
>
> +/*
> + * This should be set by platforms having multiple clock-domains, i.e.
> + * supporting multiple policies. With this sysfs directories of governor would
> + * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
> + * governor with different tunables for different clusters.
> + */
> +#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY (1 << 3)
> +
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-03 7:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1380703248.git.viresh.kumar@linaro.org>
[not found] ` <b361f4df33e414ecd2e8a25959746e84da789d6a.1380703248.git.viresh.kumar@linaro.org>
2013-10-03 7:09 ` [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void Srivatsa S. Bhat
[not found] ` <fef9eba980ca340b3f1b1a56e92c84f6bb0f5d81.1380703248.git.viresh.kumar@linaro.org>
2013-10-03 7:23 ` [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem Srivatsa S. Bhat
[not found] ` <0d25692a141042a3d4d6fab310029e96fbe57822.1380703248.git.viresh.kumar@linaro.org>
2013-10-03 7:25 ` [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev() Srivatsa S. Bhat
[not found] ` <f70f2486e3107448d1d3b835b78ad20724fdd252.1380703248.git.viresh.kumar@linaro.org>
2013-10-03 7:26 ` [PATCH RESEND 04/11] cpufreq: Remove extra blank line Srivatsa S. Bhat
[not found] ` <7b34bf61846bed19e4a7e8ee18a55e717c570fef.1380703248.git.viresh.kumar@linaro.org>
2013-10-03 7:32 ` [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator Srivatsa S. Bhat
[not found] ` <4c9af8dda1c4f968e08815632cea3987d0426127.1380703248.git.viresh.kumar@linaro.org>
2013-10-03 7:39 ` [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY Srivatsa S. Bhat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox