linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus
@ 2013-01-29  4:39 Viresh Kumar
  2013-01-29  4:40 ` [PATCH 2/2] Revert "cpufreq: Don't use cpu removed during cpufreq_driver_unregister" Viresh Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Viresh Kumar @ 2013-01-29  4:39 UTC (permalink / raw)
  To: rjw, inderpal.singh
  Cc: cpufreq, linux-pm, linux-kernel, linaro-dev, patches,
	robin.randhawa, Steve.Bannister, Liviu.Dudau, Viresh Kumar

With the addition of following patch, related_cpus is required to be set by
cpufreq platform drivers:

commit c1070fd743533efb54e98142252283583f379190
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Mon Jan 14 13:23:04 2013 +0000

    cpufreq: Simplify cpufreq_add_dev()

Because this change is required by all platform drivers, why not do this in the
core itself. Hence, this patch is an attempt towards fixing all broken drivers.

>From now on, platforms don't really need to set related_cpus from their init()
routines, as the same work is done by core too.

If platform driver needs to set the related_cpus mask with some additional cpus,
other than cpus present in policy->cpus, they are free to do it as we aren't
overriding anything.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Inderpal,

Can you please add your tested-by for this patch? And this will require you to
drop your patch for exynos-cpufreq.c :)

 drivers/cpufreq/cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f5dc02b..db81382 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -554,8 +554,6 @@ static ssize_t show_cpus(const struct cpumask *mask, char *buf)
  */
 static ssize_t show_related_cpus(struct cpufreq_policy *policy, char *buf)
 {
-	if (cpumask_empty(policy->related_cpus))
-		return show_cpus(policy->cpus, buf);
 	return show_cpus(policy->related_cpus, buf);
 }
 
@@ -945,6 +943,9 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
 		goto err_unlock_policy;
 	}
 
+	/* related cpus should atleast have policy->cpus */
+	cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
+
 	/*
 	 * affected cpus must always be the one, which are online. We aren't
 	 * managing offline cpus here.
-- 
1.7.12.rc2.18.g61b472e



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] Revert "cpufreq: Don't use cpu removed during cpufreq_driver_unregister"
  2013-01-29  4:39 [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
@ 2013-01-29  4:40 ` Viresh Kumar
  2013-01-29  4:41 ` [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
  2013-01-29 11:51 ` Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2013-01-29  4:40 UTC (permalink / raw)
  To: rjw, inderpal.singh
  Cc: cpufreq, linux-pm, linux-kernel, linaro-dev, patches,
	robin.randhawa, Steve.Bannister, Liviu.Dudau, Viresh Kumar

This reverts commit 956f33948b95aa91f6cbc6860087671c6ac1de4b.

With the addition of following patch, this change/variable is not required:

commit b9ba2725343ae57add3f324dfa5074167f48de96
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Mon Jan 14 13:23:03 2013 +0000

    cpufreq: Simplify __cpufreq_remove_dev()

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db81382..8d521422 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -47,9 +47,6 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
 #endif
 static DEFINE_SPINLOCK(cpufreq_driver_lock);
 
-/* Used when we unregister cpufreq driver */
-static struct cpumask cpufreq_online_mask;
-
 /*
  * cpu_policy_rwsem is a per CPU reader-writer semaphore designed to cure
  * all cpufreq/hotplug/workqueue/etc related lock issues.
@@ -951,7 +948,6 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
 	 * managing offline cpus here.
 	 */
 	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
-	cpumask_and(policy->cpus, policy->cpus, &cpufreq_online_mask);
 
 	policy->user_policy.min = policy->min;
 	policy->user_policy.max = policy->max;
@@ -1133,7 +1129,6 @@ static int cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
 	if (unlikely(lock_policy_rwsem_write(cpu)))
 		BUG();
 
-	cpumask_clear_cpu(cpu, &cpufreq_online_mask);
 	retval = __cpufreq_remove_dev(dev, sif);
 	return retval;
 }
@@ -1875,8 +1870,6 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 	cpufreq_driver = driver_data;
 	spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
-	cpumask_setall(&cpufreq_online_mask);
-
 	ret = subsys_interface_register(&cpufreq_interface);
 	if (ret)
 		goto err_null_driver;
-- 
1.7.12.rc2.18.g61b472e



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus
  2013-01-29  4:39 [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
  2013-01-29  4:40 ` [PATCH 2/2] Revert "cpufreq: Don't use cpu removed during cpufreq_driver_unregister" Viresh Kumar
@ 2013-01-29  4:41 ` Viresh Kumar
  2013-01-29 11:51 ` Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2013-01-29  4:41 UTC (permalink / raw)
  To: rjw, inderpal.singh
  Cc: cpufreq, linux-pm, linux-kernel, linaro-dev, patches,
	robin.randhawa, Steve.Bannister, Liviu.Dudau, Viresh Kumar

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On 29 January 2013 10:09, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> With the addition of following patch, related_cpus is required to be set by
> cpufreq platform drivers:
>
> commit c1070fd743533efb54e98142252283583f379190
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> Date:   Mon Jan 14 13:23:04 2013 +0000
>
>     cpufreq: Simplify cpufreq_add_dev()
>
> Because this change is required by all platform drivers, why not do this in the
> core itself. Hence, this patch is an attempt towards fixing all broken drivers.
>
> From now on, platforms don't really need to set related_cpus from their init()
> routines, as the same work is done by core too.
>
> If platform driver needs to set the related_cpus mask with some additional cpus,
> other than cpus present in policy->cpus, they are free to do it as we aren't
> overriding anything.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Inderpal,
>
> Can you please add your tested-by for this patch? And this will require you to
> drop your patch for exynos-cpufreq.c :)

ARM mail servers are broken. Patches attached.

[-- Attachment #2: 0001-cpufreq-Set-policy-related_cpus-to-atleast-policy-cp.patch --]
[-- Type: application/octet-stream, Size: 2212 bytes --]

From d6827976a56d429848ab8ababa2779a21c68d65f Mon Sep 17 00:00:00 2001
Message-Id: <d6827976a56d429848ab8ababa2779a21c68d65f.1359434113.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 29 Jan 2013 09:55:00 +0530
Subject: [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast
 policy->cpus

With the addition of following patch, related_cpus is required to be set by
cpufreq platform drivers:

commit c1070fd743533efb54e98142252283583f379190
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Mon Jan 14 13:23:04 2013 +0000

    cpufreq: Simplify cpufreq_add_dev()

Because this change is required by all platform drivers, why not do this in the
core itself. Hence, this patch is an attempt towards fixing all broken drivers.

From now on, platforms don't really need to set related_cpus from their init()
routines, as the same work is done by core too.

If platform driver needs to set the related_cpus mask with some additional cpus,
other than cpus present in policy->cpus, they are free to do it as we aren't
overriding anything.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Inderpal,

Can you please add your tested-by for this patch? And this will require you to
drop your patch for exynos-cpufreq.c :)

 drivers/cpufreq/cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f5dc02b..db81382 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -554,8 +554,6 @@ static ssize_t show_cpus(const struct cpumask *mask, char *buf)
  */
 static ssize_t show_related_cpus(struct cpufreq_policy *policy, char *buf)
 {
-	if (cpumask_empty(policy->related_cpus))
-		return show_cpus(policy->cpus, buf);
 	return show_cpus(policy->related_cpus, buf);
 }
 
@@ -945,6 +943,9 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
 		goto err_unlock_policy;
 	}
 
+	/* related cpus should atleast have policy->cpus */
+	cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
+
 	/*
 	 * affected cpus must always be the one, which are online. We aren't
 	 * managing offline cpus here.
-- 
1.7.12.rc2.18.g61b472e


[-- Attachment #3: 0002-Revert-cpufreq-Don-t-use-cpu-removed-during-cpufreq_.patch --]
[-- Type: application/octet-stream, Size: 2471 bytes --]

From c5f698edde8215a706fb82c158fb0050798797f2 Mon Sep 17 00:00:00 2001
Message-Id: <c5f698edde8215a706fb82c158fb0050798797f2.1359434113.git.viresh.kumar@linaro.org>
In-Reply-To: <d6827976a56d429848ab8ababa2779a21c68d65f.1359434113.git.viresh.kumar@linaro.org>
References: <d6827976a56d429848ab8ababa2779a21c68d65f.1359434113.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 29 Jan 2013 10:01:19 +0530
Subject: [PATCH 2/2] Revert "cpufreq: Don't use cpu removed during
 cpufreq_driver_unregister"

This reverts commit 956f33948b95aa91f6cbc6860087671c6ac1de4b.

With the addition of following patch, this change/variable is not required:

commit b9ba2725343ae57add3f324dfa5074167f48de96
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Mon Jan 14 13:23:03 2013 +0000

    cpufreq: Simplify __cpufreq_remove_dev()

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db81382..8d521422 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -47,9 +47,6 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
 #endif
 static DEFINE_SPINLOCK(cpufreq_driver_lock);
 
-/* Used when we unregister cpufreq driver */
-static struct cpumask cpufreq_online_mask;
-
 /*
  * cpu_policy_rwsem is a per CPU reader-writer semaphore designed to cure
  * all cpufreq/hotplug/workqueue/etc related lock issues.
@@ -951,7 +948,6 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
 	 * managing offline cpus here.
 	 */
 	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
-	cpumask_and(policy->cpus, policy->cpus, &cpufreq_online_mask);
 
 	policy->user_policy.min = policy->min;
 	policy->user_policy.max = policy->max;
@@ -1133,7 +1129,6 @@ static int cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
 	if (unlikely(lock_policy_rwsem_write(cpu)))
 		BUG();
 
-	cpumask_clear_cpu(cpu, &cpufreq_online_mask);
 	retval = __cpufreq_remove_dev(dev, sif);
 	return retval;
 }
@@ -1875,8 +1870,6 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 	cpufreq_driver = driver_data;
 	spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
-	cpumask_setall(&cpufreq_online_mask);
-
 	ret = subsys_interface_register(&cpufreq_interface);
 	if (ret)
 		goto err_null_driver;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus
  2013-01-29  4:39 [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
  2013-01-29  4:40 ` [PATCH 2/2] Revert "cpufreq: Don't use cpu removed during cpufreq_driver_unregister" Viresh Kumar
  2013-01-29  4:41 ` [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
@ 2013-01-29 11:51 ` Rafael J. Wysocki
  2013-01-29 14:30   ` Viresh Kumar
  2 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-01-29 11:51 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: inderpal.singh, cpufreq, linux-pm, linux-kernel, linaro-dev,
	patches, robin.randhawa, Steve.Bannister, Liviu.Dudau

On Tuesday, January 29, 2013 10:09:59 AM Viresh Kumar wrote:
> With the addition of following patch, related_cpus is required to be set by
> cpufreq platform drivers:
> 
> commit c1070fd743533efb54e98142252283583f379190
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> Date:   Mon Jan 14 13:23:04 2013 +0000
> 
>     cpufreq: Simplify cpufreq_add_dev()
> 

I've dropped this one in the meantime.

Can you please fold the $subject patch into "cpufreq: Simplify cpufreq_add_dev()"
and post the result instead?  That surely will be less confusing?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus
  2013-01-29 11:51 ` Rafael J. Wysocki
@ 2013-01-29 14:30   ` Viresh Kumar
  2013-01-29 21:50     ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2013-01-29 14:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: inderpal.singh, cpufreq, linux-pm, linux-kernel, linaro-dev,
	patches, robin.randhawa, Steve.Bannister, Liviu.Dudau

On 29 January 2013 17:21, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, January 29, 2013 10:09:59 AM Viresh Kumar wrote:
>> With the addition of following patch, related_cpus is required to be set by
>> cpufreq platform drivers:
>>
>> commit c1070fd743533efb54e98142252283583f379190
>> Author: Viresh Kumar <viresh.kumar@linaro.org>
>> Date:   Mon Jan 14 13:23:04 2013 +0000
>>
>>     cpufreq: Simplify cpufreq_add_dev()
>>
>
> I've dropped this one in the meantime.
>
> Can you please fold the $subject patch into "cpufreq: Simplify cpufreq_add_dev()"
> and post the result instead?  That surely will be less confusing?

Okay. I will squash this one with cpufreq_add_dev() one + following line:

diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c
index 8ff26af..fc714a6 100644
--- a/drivers/cpufreq/spear-cpufreq.c
+++ b/drivers/cpufreq/spear-cpufreq.c
@@ -189,7 +189,6 @@ static int spear_cpufreq_init(struct cpufreq_policy *policy)
        policy->cur = spear_cpufreq_get(0);

        cpumask_copy(policy->cpus, topology_core_cpumask(policy->cpu));
-       cpumask_copy(policy->related_cpus, policy->cpus);

        return 0;
 }


Also, because you are happy loosing your commit history in linux-next,
you can drop
the patch that i have reverted as 2/2 of this set.

I will resend the rest again.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus
  2013-01-29 14:30   ` Viresh Kumar
@ 2013-01-29 21:50     ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-01-29 21:50 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: inderpal.singh, cpufreq, linux-pm, linux-kernel, linaro-dev,
	patches, robin.randhawa, Steve.Bannister, Liviu.Dudau

On Tuesday, January 29, 2013 08:00:23 PM Viresh Kumar wrote:
> On 29 January 2013 17:21, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday, January 29, 2013 10:09:59 AM Viresh Kumar wrote:
> >> With the addition of following patch, related_cpus is required to be set by
> >> cpufreq platform drivers:
> >>
> >> commit c1070fd743533efb54e98142252283583f379190
> >> Author: Viresh Kumar <viresh.kumar@linaro.org>
> >> Date:   Mon Jan 14 13:23:04 2013 +0000
> >>
> >>     cpufreq: Simplify cpufreq_add_dev()
> >>
> >
> > I've dropped this one in the meantime.
> >
> > Can you please fold the $subject patch into "cpufreq: Simplify cpufreq_add_dev()"
> > and post the result instead?  That surely will be less confusing?
> 
> Okay. I will squash this one with cpufreq_add_dev() one + following line:
> 
> diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c
> index 8ff26af..fc714a6 100644
> --- a/drivers/cpufreq/spear-cpufreq.c
> +++ b/drivers/cpufreq/spear-cpufreq.c
> @@ -189,7 +189,6 @@ static int spear_cpufreq_init(struct cpufreq_policy *policy)
>         policy->cur = spear_cpufreq_get(0);
> 
>         cpumask_copy(policy->cpus, topology_core_cpumask(policy->cpu));
> -       cpumask_copy(policy->related_cpus, policy->cpus);
> 
>         return 0;
>  }
> 
> 
> Also, because you are happy loosing your commit history in linux-next,
> you can drop
> the patch that i have reverted as 2/2 of this set.

Well, I'm not attached to the linux-next commit history, but also it's a
pain to change it too oftern. :-)

I generally avoid changing it unless there are build issues and such that
would cause pain to people doing bisection, for example.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-01-29 21:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-29  4:39 [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
2013-01-29  4:40 ` [PATCH 2/2] Revert "cpufreq: Don't use cpu removed during cpufreq_driver_unregister" Viresh Kumar
2013-01-29  4:41 ` [PATCH 1/2] cpufreq: Set policy->related_cpus to atleast policy->cpus Viresh Kumar
2013-01-29 11:51 ` Rafael J. Wysocki
2013-01-29 14:30   ` Viresh Kumar
2013-01-29 21:50     ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).