Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH 0/8] clk: ux500: Fixup smp_twd clk for clk notifiers
From: Ulf Hansson @ 2012-10-26  8:19 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Linus Walleij, Ulf Hansson, linux-arm-kernel, Mike Turquette,
	Mike Turquette, Rafael J. Wysocki, cpufreq, linux-pm,
	Philippe Begnic, Rickard Andersson, Jonas Aberg, Vincent Guittot
In-Reply-To: <20121011151947.GD15428@gmail.com>

On 11 October 2012 17:19, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 11 Oct 2012, Linus Walleij wrote:
>
>> On Thu, Oct 11, 2012 at 3:44 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> >> Patches are based on Linus Torvalds tree with latest commit as of okt 10.
>> >
>> > Hmm... I get:
>> >
>> > Applying: clk: ux500: Support for prcmu_scalable_rate clock
>> > error: drivers/clk/ux500/clk-prcmu.c: does not exist in index
>> > error: drivers/clk/ux500/clk.h: does not exist in index
>> > Patch failed at 0001 clk: ux500: Support for prcmu_scalable_rate clock
>> >
>> > So when did drivers/clk/ux500/* arrive?
>>
>> Exactly here, 10 days ago on Torvalds' master branch:
>
> Ah I see. Basing patches on commits half way through the merge
> window. Good move! ;)
>
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Samuel, a kind reminder on this, trying to collect acks to be able to
advise Mike to take this series through his clk tree. There are two
mfd patches.
[PATCH 2/8] mfd: db8500: Provide cpufreq table as platform data
[PATCH 5/8] mfd: db8500: Connect ARMSS clk to ARM OPP


Kind regards
Ulf Hansson

^ permalink raw reply

* Re: [PATCH 8/8] cpufreq: db8500: Use armss clk to update frequency
From: Ulf Hansson @ 2012-10-26  8:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-arm-kernel, Mike Turquette, Mike Turquette, Samuel Ortiz,
	cpufreq, linux-pm, Linus Walleij, Lee Jones, Philippe Begnic,
	Rickard Andersson, Jonas Aberg, Vincent Guittot
In-Reply-To: <1349869349-8070-9-git-send-email-ulf.hansson@stericsson.com>

On 10 October 2012 13:42, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> Using the armss clk to update the frequency makes the driver no more
> directly dependant on the prmcu API.
>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/cpufreq/db8500-cpufreq.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/cpufreq/db8500-cpufreq.c b/drivers/cpufreq/db8500-cpufreq.c
> index dea9a49..4f154bc 100644
> --- a/drivers/cpufreq/db8500-cpufreq.c
> +++ b/drivers/cpufreq/db8500-cpufreq.c
> @@ -14,10 +14,11 @@
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/platform_device.h>
> -#include <linux/mfd/dbx500-prcmu.h>
> +#include <linux/clk.h>
>  #include <mach/id.h>
>
>  static struct cpufreq_frequency_table *freq_table;
> +static struct clk *armss_clk;
>
>  static struct freq_attr *db8500_cpufreq_attr[] = {
>         &cpufreq_freq_attr_scaling_available_freqs,
> @@ -58,9 +59,9 @@ static int db8500_cpufreq_target(struct cpufreq_policy *policy,
>         for_each_cpu(freqs.cpu, policy->cpus)
>                 cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
>
> -       /* request the PRCM unit for opp change */
> -       if (prcmu_set_arm_opp(freq_table[idx].index)) {
> -               pr_err("db8500-cpufreq:  Failed to set OPP level\n");
> +       /* update armss clk frequency */
> +       if (clk_set_rate(armss_clk, freq_table[idx].frequency * 1000)) {
> +               pr_err("db8500-cpufreq: Failed to update armss clk\n");
>                 return -EINVAL;
>         }
>
> @@ -74,16 +75,16 @@ static int db8500_cpufreq_target(struct cpufreq_policy *policy,
>  static unsigned int db8500_cpufreq_getspeed(unsigned int cpu)
>  {
>         int i = 0;
> -       /* request the prcm to get the current ARM opp */
> -       int opp = prcmu_get_arm_opp();
> +       unsigned long freq = clk_get_rate(armss_clk) / 1000;
>
>         while (freq_table[i].frequency != CPUFREQ_TABLE_END) {
> -               if (opp == freq_table[i].index)
> +               if (freq <= freq_table[i].frequency)
>                         return freq_table[i].frequency;
>                 i++;
>         }
>
> -       /* We could not find a corresponding opp frequency. */
> +       /* We could not find a corresponding frequency. */
> +       pr_err("db8500-cpufreq: Failed to find cpufreq speed\n");
>         return 0;
>  }
>
> @@ -92,6 +93,12 @@ static int __cpuinit db8500_cpufreq_init(struct cpufreq_policy *policy)
>         int i = 0;
>         int res;
>
> +       armss_clk = clk_get(NULL, "armss");
> +       if (IS_ERR(armss_clk)) {
> +               pr_err("db8500-cpufreq : Failed to get armss clk\n");
> +               return PTR_ERR(armss_clk);
> +       }
> +
>         pr_info("db8500-cpufreq : Available frequencies:\n");
>         while (freq_table[i].frequency != CPUFREQ_TABLE_END) {
>                 pr_info("  %d Mhz\n", freq_table[i].frequency/1000);
> @@ -104,6 +111,7 @@ static int __cpuinit db8500_cpufreq_init(struct cpufreq_policy *policy)
>                 cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
>         else {
>                 pr_err("db8500-cpufreq : Failed to read policy table\n");
> +               clk_put(armss_clk);
>                 return res;
>         }
>
> --
> 1.7.10
>

Just a kind reminder on this Rafael; trying to collect acks, do you
think we can advise Mike to merge this though his clk tree?

Kind regards
Ulf Hansson

^ permalink raw reply

* [PATCH V2 4/6] Thermal: Remove the cooling_cpufreq_list.
From: hongbo.zhang @ 2012-10-26  7:09 UTC (permalink / raw)
  To: linaro-dev, linux-kernel, linux-pm
  Cc: patches, linaro-kernel, STEricsson_nomadik_linux, kernel,
	hongbo.zhang
In-Reply-To: <50898F97.3000202@gmail.com>

From: "hongbo.zhang" <hongbo.zhang@linaro.com>

Problem of using this list is that the cpufreq_get_max_state callback will be
called when register cooling device by thermal_cooling_device_register, but
this list isn't ready at this moment. What's more, there is no need to maintain
such a list, we can get cpufreq_cooling_device instance by the private
thermal_cooling_device.devdata.

Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
---
 drivers/thermal/cpu_cooling.c | 91 +++++++++----------------------------------
 1 file changed, 19 insertions(+), 72 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 415b041..2ffd12c 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -58,8 +58,9 @@ struct cpufreq_cooling_device {
 };
 static LIST_HEAD(cooling_cpufreq_list);
 static DEFINE_IDR(cpufreq_idr);
+static DEFINE_MUTEX(cooling_cpufreq_lock);
 
-static struct mutex cooling_cpufreq_lock;
+static unsigned int cpufreq_dev_count;
 
 /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
 #define NOTIFY_INVALID NULL
@@ -240,28 +241,18 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
 static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
 				 unsigned long *state)
 {
-	int ret = -EINVAL, i = 0;
-	struct cpufreq_cooling_device *cpufreq_device;
-	struct cpumask *maskPtr;
+	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
+	struct cpumask *maskPtr = &cpufreq_device->allowed_cpus;
 	unsigned int cpu;
 	struct cpufreq_frequency_table *table;
 	unsigned long count = 0;
+	int i = 0;
 
-	mutex_lock(&cooling_cpufreq_lock);
-	list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
-		if (cpufreq_device && cpufreq_device->cool_dev == cdev)
-			break;
-	}
-	if (cpufreq_device == NULL)
-		goto return_get_max_state;
-
-	maskPtr = &cpufreq_device->allowed_cpus;
 	cpu = cpumask_any(maskPtr);
 	table = cpufreq_frequency_get_table(cpu);
 	if (!table) {
 		*state = 0;
-		ret = 0;
-		goto return_get_max_state;
+		return 0;
 	}
 
 	for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
@@ -272,12 +263,10 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
 
 	if (count > 0) {
 		*state = --count;
-		ret = 0;
+		return 0;
 	}
 
-return_get_max_state:
-	mutex_unlock(&cooling_cpufreq_lock);
-	return ret;
+	return -EINVAL;
 }
 
 /**
@@ -288,20 +277,10 @@ return_get_max_state:
 static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
 				 unsigned long *state)
 {
-	int ret = -EINVAL;
-	struct cpufreq_cooling_device *cpufreq_device;
+	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
 
-	mutex_lock(&cooling_cpufreq_lock);
-	list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
-		if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
-			*state = cpufreq_device->cpufreq_state;
-			ret = 0;
-			break;
-		}
-	}
-	mutex_unlock(&cooling_cpufreq_lock);
-
-	return ret;
+	*state = cpufreq_device->cpufreq_state;
+	return 0;
 }
 
 /**
@@ -312,22 +291,9 @@ static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
 static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
 				 unsigned long state)
 {
-	int ret = -EINVAL;
-	struct cpufreq_cooling_device *cpufreq_device;
+	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
 
-	mutex_lock(&cooling_cpufreq_lock);
-	list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
-		if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
-			ret = 0;
-			break;
-		}
-	}
-	if (!ret)
-		ret = cpufreq_apply_cooling(cpufreq_device, state);
-
-	mutex_unlock(&cooling_cpufreq_lock);
-
-	return ret;
+	return cpufreq_apply_cooling(cpufreq_device, state);
 }
 
 /* Bind cpufreq callbacks to thermal cooling device ops */
@@ -351,7 +317,7 @@ struct thermal_cooling_device *cpufreq_cooling_register(
 {
 	struct thermal_cooling_device *cool_dev;
 	struct cpufreq_cooling_device *cpufreq_dev = NULL;
-	unsigned int cpufreq_dev_count = 0, min = 0, max = 0;
+	unsigned int min = 0, max = 0;
 	char dev_name[THERMAL_NAME_LENGTH];
 	int ret = 0, i;
 	struct cpufreq_policy policy;
@@ -360,9 +326,6 @@ struct thermal_cooling_device *cpufreq_cooling_register(
 	if (!cpufreq_frequency_get_table(cpumask_any(clip_cpus)))
 		return ERR_PTR(-EPROBE_DEFER);
 
-	list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node)
-		cpufreq_dev_count++;
-
 	/*Verify that all the clip cpus have same freq_min, freq_max limit*/
 	for_each_cpu(i, clip_cpus) {
 		/*continue if cpufreq policy not found and not return error*/
@@ -384,9 +347,6 @@ struct thermal_cooling_device *cpufreq_cooling_register(
 
 	cpumask_copy(&cpufreq_dev->allowed_cpus, clip_cpus);
 
-	if (cpufreq_dev_count == 0)
-		mutex_init(&cooling_cpufreq_lock);
-
 	ret = get_idr(&cpufreq_idr, &cpufreq_dev->id);
 	if (ret) {
 		kfree(cpufreq_dev);
@@ -405,12 +365,12 @@ struct thermal_cooling_device *cpufreq_cooling_register(
 	cpufreq_dev->cool_dev = cool_dev;
 	cpufreq_dev->cpufreq_state = 0;
 	mutex_lock(&cooling_cpufreq_lock);
-	list_add_tail(&cpufreq_dev->node, &cooling_cpufreq_list);
 
 	/* Register the notifier for first cpufreq cooling device */
 	if (cpufreq_dev_count == 0)
 		cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
 						CPUFREQ_POLICY_NOTIFIER);
+	cpufreq_dev_count++;
 
 	mutex_unlock(&cooling_cpufreq_lock);
 	return cool_dev;
@@ -423,33 +383,20 @@ EXPORT_SYMBOL(cpufreq_cooling_register);
  */
 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 {
-	struct cpufreq_cooling_device *cpufreq_dev = NULL;
-	unsigned int cpufreq_dev_count = 0;
+	struct cpufreq_cooling_device *cpufreq_dev = cdev->devdata;
 
 	mutex_lock(&cooling_cpufreq_lock);
-	list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
-		if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
-			break;
-		cpufreq_dev_count++;
-	}
-
-	if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
-		mutex_unlock(&cooling_cpufreq_lock);
-		return;
-	}
-
-	list_del(&cpufreq_dev->node);
+	cpufreq_dev_count--;
 
 	/* Unregister the notifier for the last cpufreq cooling device */
-	if (cpufreq_dev_count == 1) {
+	if (cpufreq_dev_count == 0) {
 		cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
 					CPUFREQ_POLICY_NOTIFIER);
 	}
 	mutex_unlock(&cooling_cpufreq_lock);
+
 	thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
 	release_idr(&cpufreq_idr, cpufreq_dev->id);
-	if (cpufreq_dev_count == 1)
-		mutex_destroy(&cooling_cpufreq_lock);
 	kfree(cpufreq_dev);
 }
 EXPORT_SYMBOL(cpufreq_cooling_unregister);
-- 
1.7.11.3


^ permalink raw reply related

* Re: [for-next PATCH V2] PM / devfreq: Add sysfs node to expose available frequencies
From: MyungJoo Ham @ 2012-10-26  6:16 UTC (permalink / raw)
  To: Nishanth Menon, linux-pm
  Cc: Rajagopal Venkat, 박경민, Rafael J. Wysocki,
	Kevin Hilman, linux-kernel@vger.kernel.org

> devfreq governors such as ondemand are controlled by a min and
> max frequency, while governors like userspace governor allow us
> to set a specific frequency.
> However, for the same specific device, depending on the SoC, the
> available frequencies can vary.
> 
> So expose the available frequencies as a snapshot over sysfs to
> allow informed decisions.
> 
> This was inspired by cpufreq framework's equivalent for similar
> usage sysfs node: scaling_available_frequencies.
> 
> Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>


^ permalink raw reply

* PCI/PM: Add comments for PME poll support for PCIe
From: Huang Ying @ 2012-10-26  5:07 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki, Huang Ying,
	Rafael J. Wysocki

There are comments on why PME poll support is necessary for PCI
devices, but not for PCIe devices.  That may lead to misunderstanding
that PME poll is only necessary for PCI devices.  So add comments
related to PCIe PME poll to make it more clear.

The content of comments comes from the changelog of commit:

379021d5c0899fcf9410cae4ca7a59a5a94ca769

Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/pci/pci.c |   28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1578,15 +1578,25 @@ void pci_pme_active(struct pci_dev *dev,
 
 	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 
-	/* PCI (as opposed to PCIe) PME requires that the device have
-	   its PME# line hooked up correctly. Not all hardware vendors
-	   do this, so the PME never gets delivered and the device
-	   remains asleep. The easiest way around this is to
-	   periodically walk the list of suspended devices and check
-	   whether any have their PME flag set. The assumption is that
-	   we'll wake up often enough anyway that this won't be a huge
-	   hit, and the power savings from the devices will still be a
-	   win. */
+	/*
+	 * PCI (as opposed to PCIe) PME requires that the device have
+	 * its PME# line hooked up correctly. Not all hardware vendors
+	 * do this, so the PME never gets delivered and the device
+	 * remains asleep. The easiest way around this is to
+	 * periodically walk the list of suspended devices and check
+	 * whether any have their PME flag set. The assumption is that
+	 * we'll wake up often enough anyway that this won't be a huge
+	 * hit, and the power savings from the devices will still be a
+	 * win.
+	 *
+	 * Although PCIe uses in-band PME message instead of PME# line
+	 * to report PME, PME does not work for some PCIe devices in
+	 * reality.  For example, there are devices that set their PME
+	 * status bits, but don't really bother to send a PME message;
+	 * there are PCI Express Root Ports that don't bother to
+	 * trigger interrupts when they receive PME messages from the
+	 * devices below.  So PME poll is used for PCIe devices too.
+	 */
 
 	if (dev->pme_poll) {
 		struct pci_pme_device *pme_dev;

^ permalink raw reply

* [PATCH] cpuidle: add missing header include
From: Jingoo Han @ 2012-10-26  4:30 UTC (permalink / raw)
  To: 'Rafael J. Wysocki'
  Cc: linux-pm, linux-kernel, 'Daniel Lezcano',
	'Jingoo Han'

This patch adds missing device.h header to fix build warnings as below:

drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
[enabled by default]
drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
In file included from drivers/cpuidle/driver.c:15:0:
drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
[enabled by default]
drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]

This build warning is introduced by commit efeca1b
"cpuidle / sysfs: change function parameter".

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/cpuidle/cpuidle.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
index a5bbd1c..2120d9e 100644
--- a/drivers/cpuidle/cpuidle.h
+++ b/drivers/cpuidle/cpuidle.h
@@ -5,6 +5,8 @@
 #ifndef __DRIVER_CPUIDLE_H
 #define __DRIVER_CPUIDLE_H
 
+#include <linux/device.h>
+
 /* For internal use only */
 extern struct cpuidle_governor *cpuidle_curr_governor;
 extern struct list_head cpuidle_governors;
-- 
1.7.1



^ permalink raw reply related

* Re: [PATCH 2/2] cpufreq: governors: remove redundant code
From: Viresh Kumar @ 2012-10-26  3:28 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, linux-kernel, linux-arm-kernel, linaro-dev,
	patches, pdsw-power-team, arvind.chauhan
In-Reply-To: <1707056.POnhGSP0pg@vostro.rjw.lan>

On 26 October 2012 05:43, Rafael J. Wysocki <rjw@sisk.pl> wrote:

> I have applied this patch only because of the fixes on top of it.  It broke
> kernel compliation due to some missing EXPORT_SYMBOL_GPLs in cpufreq_governor.c,
> so I woulnd't have applied it otherwise.

Hi Rafael,

So sorry for this. I am really feeling bad for that. I should have
tried compiling
them as modules too. I had that in mind while coding it, but forgot it later.

Thanks for making changes on my behalf.

--
viresh

^ permalink raw reply

* Re: [PATCH V2 4/6] Thermal: Remove the cooling_cpufreq_list
From: Hongbo Zhang @ 2012-10-26  2:59 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: linaro-dev@lists.linaro.org, linux-kernel, linux-pm,
	STEricsson_nomadik_linux, kernel, linaro-kernel, Patch Tracking
In-Reply-To: <50898F97.3000202@gmail.com>

On 26 October 2012 03:14, Francesco Lavra <francescolavra.fl@gmail.com> wrote:
> Hi,
> Hongbo Zhang wrote:
>> Problem of using this list is that the cpufreq_get_max_state callback will be
>> called when register cooling device by thermal_cooling_device_register, but
>> this list isn't ready at this moment. What's more, there is no need to maintain
>> such a list, we can get cpufreq_cooling_device instance by the private
>> thermal_cooling_device.devdata.
>>
>> Signed-off-by: hongbo.zhang <hongbo.zhang at linaro.com>
>> ---
>>  drivers/thermal/cpu_cooling.c | 81 +++++++++----------------------------------
>>  1 file changed, 16 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> index 415b041..cc80d29 100644
>> --- a/drivers/thermal/cpu_cooling.c
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -58,8 +58,9 @@ struct cpufreq_cooling_device {
>>  };
>>  static LIST_HEAD(cooling_cpufreq_list);
>>  static DEFINE_IDR(cpufreq_idr);
>> +static DEFINE_MUTEX(cooling_cpufreq_lock);
>>
>> -static struct mutex cooling_cpufreq_lock;
>> +static unsigned int cpufreq_dev_count;
>>
>>  /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
>>  #define NOTIFY_INVALID NULL
>> @@ -241,20 +242,12 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>>                                unsigned long *state)
>>  {
>>       int ret = -EINVAL, i = 0;
>> -     struct cpufreq_cooling_device *cpufreq_device;
>> +     struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
>>       struct cpumask *maskPtr;
>>       unsigned int cpu;
>>       struct cpufreq_frequency_table *table;
>>       unsigned long count = 0;
>>
>> -     mutex_lock(&cooling_cpufreq_lock);
>> -     list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>> -             if (cpufreq_device && cpufreq_device->cool_dev == cdev)
>> -                     break;
>> -     }
>> -     if (cpufreq_device == NULL)
>> -             goto return_get_max_state;
>> -
>>       maskPtr = &cpufreq_device->allowed_cpus;
>>       cpu = cpumask_any(maskPtr);
>>       table = cpufreq_frequency_get_table(cpu);
>> @@ -276,7 +269,6 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>>       }
>>
>>  return_get_max_state:
>> -     mutex_unlock(&cooling_cpufreq_lock);
>>       return ret;
>
> Since there is no mutex locking/unlocking anymore, I'd say the goto
> label should be removed.
Good.
>
> [...]
>>  void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>  {
>> -     struct cpufreq_cooling_device *cpufreq_dev = NULL;
>> -     unsigned int cpufreq_dev_count = 0;
>> +     struct cpufreq_cooling_device *cpufreq_dev = cdev->devdata;
>>
>> -     mutex_lock(&cooling_cpufreq_lock);
>> -     list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
>> -             if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
>> -                     break;
>> -             cpufreq_dev_count++;
>> -     }
>> -
>> -     if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
>> -             mutex_unlock(&cooling_cpufreq_lock);
>> -             return;
>> -     }
>> +     thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
>>
>> -     list_del(&cpufreq_dev->node);
>> +     mutex_lock(&cooling_cpufreq_lock);
>> +     cpufreq_dev_count--;
>>
>>       /* Unregister the notifier for the last cpufreq cooling device */
>> -     if (cpufreq_dev_count == 1) {
>> +     if (cpufreq_dev_count == 0) {
>>               cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
>>                                       CPUFREQ_POLICY_NOTIFIER);
>>       }
>>       mutex_unlock(&cooling_cpufreq_lock);
>> -     thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
>
> Why did you move the call to thermal_cooling_device_unregister() from
> here? I don't see any reason for moving it.
In common sense, usually unregister first and then count--;
But here it should be opposite sequence of cpufreq_cooling_register,
will update it.

>
>> +
>>       release_idr(&cpufreq_idr, cpufreq_dev->id);
>> -     if (cpufreq_dev_count == 1)
>> -             mutex_destroy(&cooling_cpufreq_lock);
>>       kfree(cpufreq_dev);
>>  }
>>  EXPORT_SYMBOL(cpufreq_cooling_unregister);
>> --
>> 1.7.11.3
>
> --
> Francesco

^ permalink raw reply

* [for-next PATCH V2] PM / devfreq: Add sysfs node to expose available frequencies
From: Nishanth Menon @ 2012-10-26  0:48 UTC (permalink / raw)
  To: linux-pm
  Cc: Nishanth Menon, Rajagopal Venkat, MyungJoo Ham, Kyungmin Park,
	Rafael J. Wysocki, Kevin Hilman, linux-kernel
In-Reply-To: <[for-next PATCH] PM / devfreq: Add sysfs node to expose available frequencies>

devfreq governors such as ondemand are controlled by a min and
max frequency, while governors like userspace governor allow us
to set a specific frequency.
However, for the same specific device, depending on the SoC, the
available frequencies can vary.

So expose the available frequencies as a snapshot over sysfs to
allow informed decisions.

This was inspired by cpufreq framework's equivalent for similar
usage sysfs node: scaling_available_frequencies.

Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Kevin Hilman <khilman@ti.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
Applies on top of Rafael's linux-next branch:
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
linux-next                b960e9a Merge branch 'pm-sleep-next' into linux-next

Example output from Beagleboard XM (3730) using a dummy test driver
http://pastebin.pandaboard.org/index.php/view/85100576 :

/sys/devices/platform/iva.0/devfreq/iva.0 # cat available_frequencies 
260000000 520000000 660000000
/sys/devices/platform/iva.0/devfreq/iva.0 # cat available_frequencies|tr ' ' '-'
260000000-520000000-660000000

V2 :
	- review comment update from v1
	- protected the sysfs from buffer overflow - just in case..
V1: https://patchwork.kernel.org/patch/1648001/

 Documentation/ABI/testing/sysfs-class-devfreq |    9 +++++++
 drivers/devfreq/devfreq.c                     |   32 +++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index e6cf08e..e672ccb 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -51,3 +51,12 @@ Description:
 		The /sys/class/devfreq/.../userspace/set_freq shows and
 		sets the requested frequency for the devfreq object if
 		userspace governor is in effect.
+
+What:		/sys/class/devfreq/.../available_frequencies
+Date:		October 2012
+Contact:	Nishanth Menon <nm@ti.com>
+Description:
+		The /sys/class/devfreq/.../available_frequencies shows
+		the available frequencies of the corresponding devfreq object.
+		This is a snapshot of available frequencies and not limited
+		by the min/max frequency restrictions.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index d02ee7e..104018e 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -571,9 +571,41 @@ static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
 	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
 }
 
+static ssize_t show_available_freqs(struct device *d,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct devfreq *df = to_devfreq(d);
+	struct device *dev = df->dev.parent;
+	struct opp *opp;
+	ssize_t count = 0;
+	unsigned long freq = 0;
+
+	rcu_read_lock();
+	do {
+		opp = opp_find_freq_ceil(dev, &freq);
+		if (IS_ERR(opp))
+			break;
+
+		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
+				   "%lu ", freq);
+		freq++;
+	} while (1);
+	rcu_read_unlock();
+
+	/* Truncate the trailing space */
+	if (count)
+		count--;
+
+	count += sprintf(&buf[count], "\n");
+
+	return count;
+}
+
 static struct device_attribute devfreq_attrs[] = {
 	__ATTR(governor, S_IRUGO, show_governor, NULL),
 	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
+	__ATTR(available_frequencies, S_IRUGO, show_available_freqs, NULL),
 	__ATTR(target_freq, S_IRUGO, show_target_freq, NULL),
 	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
 	       store_polling_interval),
-- 
1.7.9.5

^ permalink raw reply related

* Re: [for-next PATCH] PM / devfreq: Add sysfs node to expose available frequencies
From: Rafael J. Wysocki @ 2012-10-26  0:22 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-pm, Rajagopal Venkat, MyungJoo Ham, Kyungmin Park,
	Kevin Hilman, linux-kernel
In-Reply-To: <20121026000321.GA4190@kahuna>

On Thursday, October 25, 2012 07:03:21 PM Nishanth Menon wrote:
> On 01:56-20121026, Rafael J. Wysocki wrote:
> > On Thursday, October 25, 2012 06:32:43 PM Nishanth Menon wrote:
> [..]
> > > +static ssize_t show_available_freqs(struct device *d,
> > > +				    struct device_attribute *attr,
> > > +				    char *buf)
> > > +{
> > > +	struct devfreq *df = to_devfreq(d);
> > > +	struct device *dev = df->dev.parent;
> > > +	struct opp *opp;
> > > +	ssize_t count = 0;
> > > +	unsigned long freq = 0;
> > > +
> > > +	rcu_read_lock();
> > > +	do {
> > > +		opp = opp_find_freq_ceil(dev, &freq);
> > > +		if (IS_ERR(opp))
> > > +			break;
> > > +
> > > +		count += sprintf(&buf[count], "%lu ", freq);
> > > +		freq++;
> > > +	} while (1);
> > > +	rcu_read_unlock();
> > > +	count += sprintf(&buf[count], "\n");
> > 
> > Care to avoid printing the tailing space?
> count -= count ? 1 : 0;

What about

if (count)
  count--;

instead?

> count += sprintf(&buf[count], "\n");
> should take care of empty list and the trailing space. Sounds reasonable?

Thanks,
Rafael


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

^ permalink raw reply

* Re: [PATCH 2/2] cpufreq: governors: remove redundant code
From: Rafael J. Wysocki @ 2012-10-26  0:13 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw, patches-QSEj5FYQhm4dnm+yROfE0A,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, pdsw-power-team-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAKohpo=Nv6DfXqyv=v-1Eo=pvx25qyrQ2qs+pGqJvbJu2bwWaw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thursday, October 25, 2012 08:59:11 AM Viresh Kumar wrote:
> On 25 October 2012 02:42, Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> > On Wednesday 24 of October 2012 21:43:46 Rafael J. Wysocki wrote:
> >> On Wednesday 24 of October 2012 11:37:13 Viresh Kumar wrote:
> >> > On 22 October 2012 14:16, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > > On 20 October 2012 01:42, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > >> Initially ondemand governor was written and then using its code conservative
> >> > >> governor is written. It used a lot of code from ondemand governor, but copy of
> >> > >> code was created instead of using the same routines from both governors. Which
> >> > >> increased code redundancy, which is difficult to manage.
> >> > >>
> >> > >> This patch is an attempt to move common part of both the governors to
> >> > >> cpufreq_governor.c file to come over above mentioned issues.
> >> > >>
> >> > >> This shouldn't change anything from functionality point of view.
> >> > >>
> >> > >> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> >
> >> > For everybody else, this patch is already pushed by Rafael in his linux-next
> >> > branch.
> >>
> >> Well, not yet, although I'm going to do that.
> >
> > Or I would if it still applied.  Unfortunately, though, it doesn't apply any
> > more to my linux-next branch due to some previous changes in it.
> >
> > Care to rebase?
> 
> Ahh.. I got confused by the following patch:
> 
> commit 83a73f712f2275033b2dc7f5c664988a1823ebc7
> Author: viresh kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Date:   Tue Oct 23 01:28:05 2012 +0200
> 
>     cpufreq: Move common part from governors to separate file, v2
> 
>     Multiple cpufreq governers have defined similar get_cpu_idle_time_***()
>     routines. These routines must be moved to some common place, so that all
>     governors can use them.
> 
>     So moving them to cpufreq_governor.c, which seems to be a better place for
>     keeping these routines.
> 
>     Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> 
> Actually, i should i have replied on this patch (and i forgot). I
> wanted you to skip
> this patch, as the latest patch already had this change.
> 
> But now i see commits from others on cpufreq_governor.c file.
> 
> Hmm... So you can keep your tree as it is and apply the attached
> patch. It is the
> same patch getting discussed in this thread. Just rebased over your latest next.

I have applied this patch only because of the fixes on top of it.  It broke
kernel compliation due to some missing EXPORT_SYMBOL_GPLs in cpufreq_governor.c,
so I woulnd't have applied it otherwise.

Thanks,
Rafael


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

^ permalink raw reply

* Re: [PATCH 1/2] cpufreq: Fix sparse warnings by updating cputime64_t to u64
From: Rafael J. Wysocki @ 2012-10-26  0:10 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: fengguang.wu-ral2JQCrhuEAvxtiuMwx3w,
	linaro-dev-cunTk1MwBs8s++Sfvej+rw, patches-QSEj5FYQhm4dnm+yROfE0A,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, cpufreq-u79uwXL29TY76Z2rM5mHXA,
	pdsw-power-team-5wv7dgnIgG8
In-Reply-To: <CAKohpo=Sw6eUP6Xk3=a=rk6WMJQta=T7ReBPM+7e9E5PsvMJgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thursday, October 25, 2012 09:00:22 AM Viresh Kumar wrote:
> On 25 October 2012 02:44, Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> > On Wednesday 24 of October 2012 13:15:58 Viresh Kumar wrote:
> >> There were few sparse warnings due to mismatch of type on function arguments.
> >> Two types were used u64 and cputime64_t. Both are actually u64, so use u64 only.
> >>
> >> Reported-by: Fengguang Wu <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >
> > This series appears to be based on your "cpufreq: governors: remove
> > redundant code" patch that hasn't been applied yet.
> >
> > Please rebase it on top of linux-pm/linux-next or on top of v3.7-rc2,
> > whichever is more convenient, and resend.
> 
> Please apply it after applying the latest cpufreq: governors patch i have sent.

Done.

Thanks,
Rafael


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

^ permalink raw reply

* Re: [for-next PATCH] PM / devfreq: Add sysfs node to expose available frequencies
From: Nishanth Menon @ 2012-10-26  0:03 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-pm, Rajagopal Venkat, MyungJoo Ham, Kyungmin Park,
	Kevin Hilman, linux-kernel
In-Reply-To: <1639482.oGax1ZB8vZ@vostro.rjw.lan>

On 01:56-20121026, Rafael J. Wysocki wrote:
> On Thursday, October 25, 2012 06:32:43 PM Nishanth Menon wrote:
[..]
> > +static ssize_t show_available_freqs(struct device *d,
> > +				    struct device_attribute *attr,
> > +				    char *buf)
> > +{
> > +	struct devfreq *df = to_devfreq(d);
> > +	struct device *dev = df->dev.parent;
> > +	struct opp *opp;
> > +	ssize_t count = 0;
> > +	unsigned long freq = 0;
> > +
> > +	rcu_read_lock();
> > +	do {
> > +		opp = opp_find_freq_ceil(dev, &freq);
> > +		if (IS_ERR(opp))
> > +			break;
> > +
> > +		count += sprintf(&buf[count], "%lu ", freq);
> > +		freq++;
> > +	} while (1);
> > +	rcu_read_unlock();
> > +	count += sprintf(&buf[count], "\n");
> 
> Care to avoid printing the tailing space?
count -= count ? 1 : 0;
count += sprintf(&buf[count], "\n");
should take care of empty list and the trailing space. Sounds reasonable?

-- 
Regards,
Nishanth Menon

^ permalink raw reply

* Re: [for-next PATCH 2/2] PM / devfreq: more documentation warning fixes next set
From: Rafael J. Wysocki @ 2012-10-26  0:05 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-pm, Rajagopal Venkat, MyungJoo Ham, Rafael J. Wysocki,
	Kyungmin Park
In-Reply-To: <1351206614-23993-3-git-send-email-nm@ti.com>

On Thursday, October 25, 2012 06:10:14 PM Nishanth Menon wrote:
> commit cfd5194aecd08cd8fbfbf1534a0209b95bc6fcdf
> (PM / devfreq: Core updates to support devices which can idle)
> in Rafael's pm-devfreq-next branch on
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
> 
> Patchworks: https://patchwork.kernel.org/patch/1545751/
> 
> introduced a kernel documentation warning:
> Warning(drivers/devfreq/devfreq.c:289): bad line: 		release its resources.
> 
> This is due to lack of '*' at documentation start messing
> up poor scripts/kernel-doc 's mind. I suggest squashing
> this to original commit given not-yet-upstream status :).
> 
> Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

I fixed up the original patch in a slightly different way.

Thanks,
Rafael


> ---
>  drivers/devfreq/devfreq.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index cdc3e2d..2f6ad6b 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -286,7 +286,7 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>  
>  /**
>   * _remove_devfreq() - Remove devfreq from the devfreq list and
> -		release its resources.
> + *			release its resources.
>   * @devfreq:	the devfreq struct
>   * @skip:	skip calling device_unregister().
>   */
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [for-next PATCH 1/2] PM / devfreq: more documentation warning fixes
From: Rafael J. Wysocki @ 2012-10-26  0:05 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-pm, Rajagopal Venkat, MyungJoo Ham, Rafael J. Wysocki,
	Kyungmin Park
In-Reply-To: <1351206614-23993-2-git-send-email-nm@ti.com>

On Thursday, October 25, 2012 06:10:13 PM Nishanth Menon wrote:
> Commit 34b53237e9ceb141e13e846baed2a282461f4a01
> (PM / devfreq: Add suspend and resume apis)
> in Rafael's pm-devfreq-next branch on
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
> 
> Patchworks: https://patchwork.kernel.org/patch/1545791/
> 
> Warning(drivers/devfreq/devfreq.c:436): No description found for parameter 'devfreq'
> Warning(drivers/devfreq/devfreq.c:450): No description found for parameter 'devfreq'
> 
> Yet again, a missing ':' causes the kernel-doc to cry
> 
> Since it is not merged to upstream, I suggest squashing it
> 
> Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

I fixed up the original patch in a slightly different way.

Thanks,
Rafael


> ---
>  drivers/devfreq/devfreq.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 32e9b48..cdc3e2d 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -430,7 +430,7 @@ EXPORT_SYMBOL(devfreq_remove_device);
>  
>  /**
>   * devfreq_suspend_device() - Suspend devfreq of a device.
> - * @devfreq	the devfreq instance to be suspended
> + * @devfreq:	the devfreq instance to be suspended
>   */
>  int devfreq_suspend_device(struct devfreq *devfreq)
>  {
> @@ -444,7 +444,7 @@ EXPORT_SYMBOL(devfreq_suspend_device);
>  
>  /**
>   * devfreq_resume_device() - Resume devfreq of a device.
> - * @devfreq	the devfreq instance to be resumed
> + * @devfreq:	the devfreq instance to be resumed
>   */
>  int devfreq_resume_device(struct devfreq *devfreq)
>  {
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [for-next PATCH] PM / devfreq: Add sysfs node to expose available frequencies
From: Rafael J. Wysocki @ 2012-10-25 23:56 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-pm, Rajagopal Venkat, MyungJoo Ham, Kyungmin Park,
	Kevin Hilman, linux-kernel
In-Reply-To: <1351207963-30075-1-git-send-email-nm@ti.com>

On Thursday, October 25, 2012 06:32:43 PM Nishanth Menon wrote:
> devfreq governors such as ondemand are controlled by a min and
> max frequency, while governors like userspace governor allow us
> to set a specific frequency.
> However, for the same specific device, depending on the SoC, the
> available frequencies can vary.
> 
> So expose the available frequencies as a snapshot over sysfs to
> allow informed decisions.
> 
> This was inspired by cpufreq framework's equivalent for similar
> usage sysfs node: scaling_available_frequencies.
> 
> Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> Applies on top of Rafael's linux-next branch:
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
>  linux-next                                   b960e9a Merge branch 'pm-sleep-next' into linux-next
> 
> Example output from Beagleboard XM (3730) using a dummy test driver
> http://pastebin.pandaboard.org/index.php/view/85100576 :
> 
> /sys/devices/platform/iva.0/devfreq/iva.0 # cat available_frequencies 
> 260000000 520000000 660000000 
> 
>  Documentation/ABI/testing/sysfs-class-devfreq |    9 +++++++++
>  drivers/devfreq/devfreq.c                     |   26 +++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
> index e6cf08e..e672ccb 100644
> --- a/Documentation/ABI/testing/sysfs-class-devfreq
> +++ b/Documentation/ABI/testing/sysfs-class-devfreq
> @@ -51,3 +51,12 @@ Description:
>  		The /sys/class/devfreq/.../userspace/set_freq shows and
>  		sets the requested frequency for the devfreq object if
>  		userspace governor is in effect.
> +
> +What:		/sys/class/devfreq/.../available_frequencies
> +Date:		October 2012
> +Contact:	Nishanth Menon <nm@ti.com>
> +Description:
> +		The /sys/class/devfreq/.../available_frequencies shows
> +		the available frequencies of the corresponding devfreq object.
> +		This is a snapshot of available frequencies and not limited
> +		by the min/max frequency restrictions.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index d02ee7e..2f6ad6b 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -571,9 +571,35 @@ static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
>  	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
>  }
>  
> +static ssize_t show_available_freqs(struct device *d,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct devfreq *df = to_devfreq(d);
> +	struct device *dev = df->dev.parent;
> +	struct opp *opp;
> +	ssize_t count = 0;
> +	unsigned long freq = 0;
> +
> +	rcu_read_lock();
> +	do {
> +		opp = opp_find_freq_ceil(dev, &freq);
> +		if (IS_ERR(opp))
> +			break;
> +
> +		count += sprintf(&buf[count], "%lu ", freq);
> +		freq++;
> +	} while (1);
> +	rcu_read_unlock();
> +	count += sprintf(&buf[count], "\n");

Care to avoid printing the tailing space?

Rafael


> +
> +	return count;
> +}
> +
>  static struct device_attribute devfreq_attrs[] = {
>  	__ATTR(governor, S_IRUGO, show_governor, NULL),
>  	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
> +	__ATTR(available_frequencies, S_IRUGO, show_available_freqs, NULL),
>  	__ATTR(target_freq, S_IRUGO, show_target_freq, NULL),
>  	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
>  	       store_polling_interval),
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* [for-next PATCH] PM / devfreq: Add sysfs node to expose available frequencies
From: Nishanth Menon @ 2012-10-25 23:32 UTC (permalink / raw)
  To: linux-pm
  Cc: Nishanth Menon, Rajagopal Venkat, MyungJoo Ham, Kyungmin Park,
	Rafael J. Wysocki, Kevin Hilman, linux-kernel

devfreq governors such as ondemand are controlled by a min and
max frequency, while governors like userspace governor allow us
to set a specific frequency.
However, for the same specific device, depending on the SoC, the
available frequencies can vary.

So expose the available frequencies as a snapshot over sysfs to
allow informed decisions.

This was inspired by cpufreq framework's equivalent for similar
usage sysfs node: scaling_available_frequencies.

Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Kevin Hilman <khilman@ti.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
Applies on top of Rafael's linux-next branch:
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
 linux-next                                   b960e9a Merge branch 'pm-sleep-next' into linux-next

Example output from Beagleboard XM (3730) using a dummy test driver
http://pastebin.pandaboard.org/index.php/view/85100576 :

/sys/devices/platform/iva.0/devfreq/iva.0 # cat available_frequencies 
260000000 520000000 660000000 

 Documentation/ABI/testing/sysfs-class-devfreq |    9 +++++++++
 drivers/devfreq/devfreq.c                     |   26 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index e6cf08e..e672ccb 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -51,3 +51,12 @@ Description:
 		The /sys/class/devfreq/.../userspace/set_freq shows and
 		sets the requested frequency for the devfreq object if
 		userspace governor is in effect.
+
+What:		/sys/class/devfreq/.../available_frequencies
+Date:		October 2012
+Contact:	Nishanth Menon <nm@ti.com>
+Description:
+		The /sys/class/devfreq/.../available_frequencies shows
+		the available frequencies of the corresponding devfreq object.
+		This is a snapshot of available frequencies and not limited
+		by the min/max frequency restrictions.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index d02ee7e..2f6ad6b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -571,9 +571,35 @@ static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
 	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
 }
 
+static ssize_t show_available_freqs(struct device *d,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct devfreq *df = to_devfreq(d);
+	struct device *dev = df->dev.parent;
+	struct opp *opp;
+	ssize_t count = 0;
+	unsigned long freq = 0;
+
+	rcu_read_lock();
+	do {
+		opp = opp_find_freq_ceil(dev, &freq);
+		if (IS_ERR(opp))
+			break;
+
+		count += sprintf(&buf[count], "%lu ", freq);
+		freq++;
+	} while (1);
+	rcu_read_unlock();
+	count += sprintf(&buf[count], "\n");
+
+	return count;
+}
+
 static struct device_attribute devfreq_attrs[] = {
 	__ATTR(governor, S_IRUGO, show_governor, NULL),
 	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
+	__ATTR(available_frequencies, S_IRUGO, show_available_freqs, NULL),
 	__ATTR(target_freq, S_IRUGO, show_target_freq, NULL),
 	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
 	       store_polling_interval),
-- 
1.7.9.5


^ permalink raw reply related

* [for-next PATCH 0/2] PM / devfreq: more documentation fixes
From: Nishanth Menon @ 2012-10-25 23:10 UTC (permalink / raw)
  To: linux-pm
  Cc: Nishanth Menon, Rajagopal Venkat, MyungJoo Ham, Rafael J. Wysocki,
	Kyungmin Park

The following patches apply on top of
Rafael's pm-devfreq-next branch on
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

and introduced due to warnings of accepted patches.
I dont seem to have the original emails in my mailbox anymore
So apologies of not replying inline to the original patches.
I have tried to provide link to the relevant patchworks link
of the original patch if I could find them.

I hope we will keep the documentation clean ahead :)

Nishanth Menon (2):
  PM / devfreq: more documentation warning fixes
  PM / devfreq: more documentation warning fixes next set

 drivers/devfreq/devfreq.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>

Regards,
Nishanth Menon
-- 
1.7.9.5


^ permalink raw reply

* [for-next PATCH 1/2] PM / devfreq: more documentation warning fixes
From: Nishanth Menon @ 2012-10-25 23:10 UTC (permalink / raw)
  To: linux-pm
  Cc: Nishanth Menon, Rajagopal Venkat, MyungJoo Ham, Rafael J. Wysocki,
	Kyungmin Park
In-Reply-To: <1351206614-23993-1-git-send-email-nm@ti.com>

Commit 34b53237e9ceb141e13e846baed2a282461f4a01
(PM / devfreq: Add suspend and resume apis)
in Rafael's pm-devfreq-next branch on
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

Patchworks: https://patchwork.kernel.org/patch/1545791/

Warning(drivers/devfreq/devfreq.c:436): No description found for parameter 'devfreq'
Warning(drivers/devfreq/devfreq.c:450): No description found for parameter 'devfreq'

Yet again, a missing ':' causes the kernel-doc to cry

Since it is not merged to upstream, I suggest squashing it

Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/devfreq/devfreq.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 32e9b48..cdc3e2d 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -430,7 +430,7 @@ EXPORT_SYMBOL(devfreq_remove_device);
 
 /**
  * devfreq_suspend_device() - Suspend devfreq of a device.
- * @devfreq	the devfreq instance to be suspended
+ * @devfreq:	the devfreq instance to be suspended
  */
 int devfreq_suspend_device(struct devfreq *devfreq)
 {
@@ -444,7 +444,7 @@ EXPORT_SYMBOL(devfreq_suspend_device);
 
 /**
  * devfreq_resume_device() - Resume devfreq of a device.
- * @devfreq	the devfreq instance to be resumed
+ * @devfreq:	the devfreq instance to be resumed
  */
 int devfreq_resume_device(struct devfreq *devfreq)
 {
-- 
1.7.9.5


^ permalink raw reply related

* [for-next PATCH 2/2] PM / devfreq: more documentation warning fixes next set
From: Nishanth Menon @ 2012-10-25 23:10 UTC (permalink / raw)
  To: linux-pm
  Cc: Nishanth Menon, Rajagopal Venkat, MyungJoo Ham, Rafael J. Wysocki,
	Kyungmin Park
In-Reply-To: <1351206614-23993-1-git-send-email-nm@ti.com>

commit cfd5194aecd08cd8fbfbf1534a0209b95bc6fcdf
(PM / devfreq: Core updates to support devices which can idle)
in Rafael's pm-devfreq-next branch on
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

Patchworks: https://patchwork.kernel.org/patch/1545751/

introduced a kernel documentation warning:
Warning(drivers/devfreq/devfreq.c:289): bad line: 		release its resources.

This is due to lack of '*' at documentation start messing
up poor scripts/kernel-doc 's mind. I suggest squashing
this to original commit given not-yet-upstream status :).

Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/devfreq/devfreq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index cdc3e2d..2f6ad6b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -286,7 +286,7 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 
 /**
  * _remove_devfreq() - Remove devfreq from the devfreq list and
-		release its resources.
+ *			release its resources.
  * @devfreq:	the devfreq struct
  * @skip:	skip calling device_unregister().
  */
-- 
1.7.9.5


^ permalink raw reply related

* Re: [BUGFIX] PCI/PM: Fix proc config reg access for D3cold and bridge suspending
From: Rafael J. Wysocki @ 2012-10-25 20:27 UTC (permalink / raw)
  To: Huang Ying; +Cc: Bjorn Helgaas, linux-kernel, linux-pci, linux-pm, stable
In-Reply-To: <1351128963-10347-1-git-send-email-ying.huang@intel.com>

On Thursday, October 25, 2012 09:36:03 AM Huang Ying wrote:
> In
> 
>   https://bugzilla.kernel.org/show_bug.cgi?id=48981
> 
> Peter reported that /proc/bus/pci/??/??.? does not works for 3.6.
> This is This is because the device configuration space registers will
> be not accessible if the corresponding parent bridge is suspended or
> the device is put into D3cold state.
> 
> This is the same as /sys/bus/pci/devices/0000:??:??.?/config access
> issue.  So the function used to solve sysfs issue is used to solve
> this issue.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Peter <lekensteyn@gmail.com>
> Signed-off-by: Huang Ying <ying.huang@intel.com>

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

> ---
>  drivers/pci/pci-sysfs.c |   34 ----------------------------------
>  drivers/pci/pci.c       |   32 ++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h       |    2 ++
>  drivers/pci/proc.c      |    8 ++++++++
>  4 files changed, 42 insertions(+), 34 deletions(-)
> 
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -458,40 +458,6 @@ boot_vga_show(struct device *dev, struct
>  }
>  struct device_attribute vga_attr = __ATTR_RO(boot_vga);
>  
> -static void
> -pci_config_pm_runtime_get(struct pci_dev *pdev)
> -{
> -	struct device *dev = &pdev->dev;
> -	struct device *parent = dev->parent;
> -
> -	if (parent)
> -		pm_runtime_get_sync(parent);
> -	pm_runtime_get_noresume(dev);
> -	/*
> -	 * pdev->current_state is set to PCI_D3cold during suspending,
> -	 * so wait until suspending completes
> -	 */
> -	pm_runtime_barrier(dev);
> -	/*
> -	 * Only need to resume devices in D3cold, because config
> -	 * registers are still accessible for devices suspended but
> -	 * not in D3cold.
> -	 */
> -	if (pdev->current_state == PCI_D3cold)
> -		pm_runtime_resume(dev);
> -}
> -
> -static void
> -pci_config_pm_runtime_put(struct pci_dev *pdev)
> -{
> -	struct device *dev = &pdev->dev;
> -	struct device *parent = dev->parent;
> -
> -	pm_runtime_put(dev);
> -	if (parent)
> -		pm_runtime_put_sync(parent);
> -}
> -
>  static ssize_t
>  pci_read_config(struct file *filp, struct kobject *kobj,
>  		struct bin_attribute *bin_attr,
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1858,6 +1858,38 @@ bool pci_dev_run_wake(struct pci_dev *de
>  }
>  EXPORT_SYMBOL_GPL(pci_dev_run_wake);
>  
> +void pci_config_pm_runtime_get(struct pci_dev *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device *parent = dev->parent;
> +
> +	if (parent)
> +		pm_runtime_get_sync(parent);
> +	pm_runtime_get_noresume(dev);
> +	/*
> +	 * pdev->current_state is set to PCI_D3cold during suspending,
> +	 * so wait until suspending completes
> +	 */
> +	pm_runtime_barrier(dev);
> +	/*
> +	 * Only need to resume devices in D3cold, because config
> +	 * registers are still accessible for devices suspended but
> +	 * not in D3cold.
> +	 */
> +	if (pdev->current_state == PCI_D3cold)
> +		pm_runtime_resume(dev);
> +}
> +
> +void pci_config_pm_runtime_put(struct pci_dev *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device *parent = dev->parent;
> +
> +	pm_runtime_put(dev);
> +	if (parent)
> +		pm_runtime_put_sync(parent);
> +}
> +
>  /**
>   * pci_pm_init - Initialize PM functions of given PCI device
>   * @dev: PCI device to handle.
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -72,6 +72,8 @@ extern void pci_disable_enabled_device(s
>  extern int pci_finish_runtime_suspend(struct pci_dev *dev);
>  extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
>  extern void pci_wakeup_bus(struct pci_bus *bus);
> +extern void pci_config_pm_runtime_get(struct pci_dev *dev);
> +extern void pci_config_pm_runtime_put(struct pci_dev *dev);
>  extern void pci_pm_init(struct pci_dev *dev);
>  extern void platform_pci_wakeup_init(struct pci_dev *dev);
>  extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -76,6 +76,8 @@ proc_bus_pci_read(struct file *file, cha
>  	if (!access_ok(VERIFY_WRITE, buf, cnt))
>  		return -EINVAL;
>  
> +	pci_config_pm_runtime_get(dev);
> +
>  	if ((pos & 1) && cnt) {
>  		unsigned char val;
>  		pci_user_read_config_byte(dev, pos, &val);
> @@ -121,6 +123,8 @@ proc_bus_pci_read(struct file *file, cha
>  		cnt--;
>  	}
>  
> +	pci_config_pm_runtime_put(dev);
> +
>  	*ppos = pos;
>  	return nbytes;
>  }
> @@ -146,6 +150,8 @@ proc_bus_pci_write(struct file *file, co
>  	if (!access_ok(VERIFY_READ, buf, cnt))
>  		return -EINVAL;
>  
> +	pci_config_pm_runtime_get(dev);
> +
>  	if ((pos & 1) && cnt) {
>  		unsigned char val;
>  		__get_user(val, buf);
> @@ -191,6 +197,8 @@ proc_bus_pci_write(struct file *file, co
>  		cnt--;
>  	}
>  
> +	pci_config_pm_runtime_put(dev);
> +
>  	*ppos = pos;
>  	i_size_write(ino, dp->size);
>  	return nbytes;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 0/4][V2] cpuidle : multiple drivers support
From: Rafael J. Wysocki @ 2012-10-25 20:29 UTC (permalink / raw)
  To: Peter De Schrijver
  Cc: Daniel Lezcano, linux-pm@vger.kernel.org,
	lorenzo.pieralisi@arm.com, patches@linaro.org,
	linaro-dev@lists.linaro.org
In-Reply-To: <20121025134933.GA1962@tbergstrom-lnx.Nvidia.com>

On Thursday, October 25, 2012 04:49:33 PM Peter De Schrijver wrote:
> On Fri, Oct 19, 2012 at 12:10:45PM +0200, Daniel Lezcano wrote:
> > The discussion about having different cpus on the system with
> > different latencies bring us to a first attemp by adding a
> > pointer in the cpuidle_device to the states array.
> > 
> > But as Rafael suggested, it would make more sense to create a
> > driver per cpu [1].
> > 
> > This patch adds support for multiple cpuidle drivers.
> > 
> > It creates a per cpu cpuidle driver pointer.
> > 
> > In order to not break the different drivers, the function cpuidle_register_driver
> > assign for each cpu, the driver.
> > 
> > The multiple driver support is optional and if it is not set, the cpuide driver
> > core code remains the same (except some code reorganisation).
> > 
> > I did the following tests compiled, booted, tested without/with CONFIG_CPU_IDLE,
> > with/without CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
> > 
> > Tested on Core2 Duo T9500 with acpi_idle [and intel_idle]
> > Tested on ARM Dual Cortex-A9 U8500 (aka Snowball)
> > 
> > V1 tested on Tegra3 and Vexpress TC2
> > 
> 
> V2 tested on Tegra3.

Do I assume correctly that Tested-by applies?

Rafael


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

^ permalink raw reply

* Re: [PATCH V2 4/6] Thermal: Remove the cooling_cpufreq_list
From: Francesco Lavra @ 2012-10-25 19:14 UTC (permalink / raw)
  To: hongbo.zhang, linaro-dev@lists.linaro.org, linux-kernel, linux-pm
  Cc: STEricsson_nomadik_linux, kernel, linaro-kernel, Patch Tracking
In-Reply-To: <1351079900-32236-5-git-send-email-hongbo.zhang@linaro.com>

Hi,
Hongbo Zhang wrote:
> Problem of using this list is that the cpufreq_get_max_state callback will be
> called when register cooling device by thermal_cooling_device_register, but
> this list isn't ready at this moment. What's more, there is no need to maintain
> such a list, we can get cpufreq_cooling_device instance by the private
> thermal_cooling_device.devdata.
> 
> Signed-off-by: hongbo.zhang <hongbo.zhang at linaro.com>
> ---
>  drivers/thermal/cpu_cooling.c | 81 +++++++++----------------------------------
>  1 file changed, 16 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 415b041..cc80d29 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -58,8 +58,9 @@ struct cpufreq_cooling_device {
>  };
>  static LIST_HEAD(cooling_cpufreq_list);
>  static DEFINE_IDR(cpufreq_idr);
> +static DEFINE_MUTEX(cooling_cpufreq_lock);
>  
> -static struct mutex cooling_cpufreq_lock;
> +static unsigned int cpufreq_dev_count;
>  
>  /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
>  #define NOTIFY_INVALID NULL
> @@ -241,20 +242,12 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>  				 unsigned long *state)
>  {
>  	int ret = -EINVAL, i = 0;
> -	struct cpufreq_cooling_device *cpufreq_device;
> +	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
>  	struct cpumask *maskPtr;
>  	unsigned int cpu;
>  	struct cpufreq_frequency_table *table;
>  	unsigned long count = 0;
>  
> -	mutex_lock(&cooling_cpufreq_lock);
> -	list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
> -		if (cpufreq_device && cpufreq_device->cool_dev == cdev)
> -			break;
> -	}
> -	if (cpufreq_device == NULL)
> -		goto return_get_max_state;
> -
>  	maskPtr = &cpufreq_device->allowed_cpus;
>  	cpu = cpumask_any(maskPtr);
>  	table = cpufreq_frequency_get_table(cpu);
> @@ -276,7 +269,6 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>  	}
>  
>  return_get_max_state:
> -	mutex_unlock(&cooling_cpufreq_lock);
>  	return ret;

Since there is no mutex locking/unlocking anymore, I'd say the goto
label should be removed.

[...]
>  void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>  {
> -	struct cpufreq_cooling_device *cpufreq_dev = NULL;
> -	unsigned int cpufreq_dev_count = 0;
> +	struct cpufreq_cooling_device *cpufreq_dev = cdev->devdata;
>  
> -	mutex_lock(&cooling_cpufreq_lock);
> -	list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
> -		if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
> -			break;
> -		cpufreq_dev_count++;
> -	}
> -
> -	if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
> -		mutex_unlock(&cooling_cpufreq_lock);
> -		return;
> -	}
> +	thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
>  
> -	list_del(&cpufreq_dev->node);
> +	mutex_lock(&cooling_cpufreq_lock);
> +	cpufreq_dev_count--;
>  
>  	/* Unregister the notifier for the last cpufreq cooling device */
> -	if (cpufreq_dev_count == 1) {
> +	if (cpufreq_dev_count == 0) {
>  		cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
>  					CPUFREQ_POLICY_NOTIFIER);
>  	}
>  	mutex_unlock(&cooling_cpufreq_lock);
> -	thermal_cooling_device_unregister(cpufreq_dev->cool_dev);

Why did you move the call to thermal_cooling_device_unregister() from
here? I don't see any reason for moving it.

> +
>  	release_idr(&cpufreq_idr, cpufreq_dev->id);
> -	if (cpufreq_dev_count == 1)
> -		mutex_destroy(&cooling_cpufreq_lock);
>  	kfree(cpufreq_dev);
>  }
>  EXPORT_SYMBOL(cpufreq_cooling_unregister);
> -- 
> 1.7.11.3

--
Francesco

^ permalink raw reply

* Re: [PATCH 0/4][V2] cpuidle : multiple drivers support
From: Daniel Lezcano @ 2012-10-25 14:11 UTC (permalink / raw)
  To: Peter De Schrijver
  Cc: rjw-KKrjLPT3xs0@public.gmane.org,
	linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
	patches-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20121025134933.GA1962-Rysk9IDjsxmJz7etNGeUX8VPkgjIgRvpAL8bYrjMMd8@public.gmane.org>

On 10/25/2012 03:49 PM, Peter De Schrijver wrote:
> On Fri, Oct 19, 2012 at 12:10:45PM +0200, Daniel Lezcano wrote:
>> The discussion about having different cpus on the system with
>> different latencies bring us to a first attemp by adding a
>> pointer in the cpuidle_device to the states array.
>>
>> But as Rafael suggested, it would make more sense to create a
>> driver per cpu [1].
>>
>> This patch adds support for multiple cpuidle drivers.
>>
>> It creates a per cpu cpuidle driver pointer.
>>
>> In order to not break the different drivers, the function cpuidle_register_driver
>> assign for each cpu, the driver.
>>
>> The multiple driver support is optional and if it is not set, the cpuide driver
>> core code remains the same (except some code reorganisation).
>>
>> I did the following tests compiled, booted, tested without/with CONFIG_CPU_IDLE,
>> with/without CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
>>
>> Tested on Core2 Duo T9500 with acpi_idle [and intel_idle]
>> Tested on ARM Dual Cortex-A9 U8500 (aka Snowball)
>>
>> V1 tested on Tegra3 and Vexpress TC2
>>
> 
> V2 tested on Tegra3.

Cool thanks !

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

^ permalink raw reply

* Re: [PATCH 0/4][V2] cpuidle : multiple drivers support
From: Peter De Schrijver @ 2012-10-25 13:49 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: rjw@sisk.pl, linux-pm@vger.kernel.org, lorenzo.pieralisi@arm.com,
	patches@linaro.org, linaro-dev@lists.linaro.org
In-Reply-To: <1350641449-22863-1-git-send-email-daniel.lezcano@linaro.org>

On Fri, Oct 19, 2012 at 12:10:45PM +0200, Daniel Lezcano wrote:
> The discussion about having different cpus on the system with
> different latencies bring us to a first attemp by adding a
> pointer in the cpuidle_device to the states array.
> 
> But as Rafael suggested, it would make more sense to create a
> driver per cpu [1].
> 
> This patch adds support for multiple cpuidle drivers.
> 
> It creates a per cpu cpuidle driver pointer.
> 
> In order to not break the different drivers, the function cpuidle_register_driver
> assign for each cpu, the driver.
> 
> The multiple driver support is optional and if it is not set, the cpuide driver
> core code remains the same (except some code reorganisation).
> 
> I did the following tests compiled, booted, tested without/with CONFIG_CPU_IDLE,
> with/without CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
> 
> Tested on Core2 Duo T9500 with acpi_idle [and intel_idle]
> Tested on ARM Dual Cortex-A9 U8500 (aka Snowball)
> 
> V1 tested on Tegra3 and Vexpress TC2
> 

V2 tested on Tegra3.

Cheers,

Peter.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox