* Re: [PATCH v11 6/9] libata: handle power transition of ODD
From: Aaron Lu @ 2013-01-08 9:09 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20130107184245.GS3926@htj.dyndns.org>
On 01/08/2013 02:42 AM, Tejun Heo wrote:
> On Sun, Jan 06, 2013 at 10:48:26AM +0800, Aaron Lu wrote:
>> +bool zpodd_zpready(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> + return zpodd->zp_ready;
>> +}
>> +
>> +void zpodd_pre_poweroff(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> +
>> + zpodd->powered_off = true;
>> + device_set_run_wake(&dev->sdev->sdev_gendev, true);
>> + acpi_pm_device_run_wake(&dev->sdev->sdev_gendev, true);
>> +}
>> +
>> +void zpodd_pre_poweron(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> +
>> + if (zpodd->powered_off) {
>> + acpi_pm_device_run_wake(&dev->sdev->sdev_gendev, false);
>> + device_set_run_wake(&dev->sdev->sdev_gendev, false);
>> + }
>> +}
>> +
>> +void zpodd_post_resume(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> +
>> + if (!zpodd->powered_off)
>> + return;
>> +
>> + zpodd->powered_off = false;
>> +
>> + if (zpodd->from_notify) {
>> + zpodd->from_notify = false;
>> + if (zpodd->drawer)
>> + eject_tray(dev);
>> + }
>> +
>> + zpodd->last_ready = 0;
>> + zpodd->zp_ready = false;
>> +}
>
> I would really appreciate some comments at least on functions visible
> outside zpodd.c. Please add proper function comments explaining what
> they're doing to achieve what.
Will add them in next version, thanks.
-Aaron
^ permalink raw reply
* Re: [PATCH v11 5/9] libata: check zero power ready status for ZPODD
From: Aaron Lu @ 2013-01-08 9:09 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20130107183610.GR3926@htj.dyndns.org>
On 01/08/2013 02:36 AM, Tejun Heo wrote:
> Hello, Aaron.
>
> On Sun, Jan 06, 2013 at 10:48:25AM +0800, Aaron Lu wrote:
>> +/* Check zero power ready status */
>> +void zpodd_on_suspend(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> + unsigned long expires;
>> +
>> + if (!zpready(dev)) {
>> + zpodd->zp_ready = false;
>> + zpodd->last_ready = 0;
>> + return;
>> + }
>> +
>> + if (!zpodd->last_ready) {
>> + zpodd->last_ready = jiffies;
>> + return;
>> + }
>> +
>> + expires = zpodd->last_ready +
>> + msecs_to_jiffies(zpodd_poweroff_delay * 1000);
>> + if (time_before(jiffies, expires))
>> + return;
>> +
>> + zpodd->zp_ready = true;
>> +}
>
> Using 0 jiffies as special value is generally considered a bad form.
> It should be mostly ok here but it's not like avoiding that is
> difficult, so let's please not use 0 jiffies as special value. If you
> have to, add another variable zp_sample_cnt or whatever.
No problem, thanks!
-Aaron
^ permalink raw reply
* Re: [PATCH v11 3/9] libata: identify and init ZPODD devices
From: Aaron Lu @ 2013-01-08 9:07 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20130107182052.GP3926@htj.dyndns.org>
On 01/08/2013 02:20 AM, Tejun Heo wrote:
> On Sun, Jan 06, 2013 at 10:48:23AM +0800, Aaron Lu wrote:
>> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
>> index ef01ac0..5aa7322 100644
>> --- a/drivers/ata/libata-acpi.c
>> +++ b/drivers/ata/libata-acpi.c
>> @@ -1063,6 +1063,8 @@ void ata_acpi_bind(struct ata_device *dev)
>>
>> void ata_acpi_unbind(struct ata_device *dev)
>> {
>> + if (zpodd_dev_enabled(dev))
>> + zpodd_exit(dev);
>> ata_acpi_remove_pm_notifier(dev);
>> ata_acpi_unregister_power_resource(dev);
>> }
>
> Wouldn't it make more sense to invoke zpodd_exit() from
> ata_scsi_remove_dev() which is approximate counterpart of
> dev_configure?
Yes, I agree.
>
>> +struct zpodd {
>> + bool slot:1;
>> + bool drawer:1;
>> +
>> + struct ata_device *dev;
>> +};
>
> Field names are usually indented. It would be nice to have a comment
checkscript.sh doesn't seem like this if I put a tab around the ':'
ERROR: spaces prohibited around that ':' (ctx:VxW)
#222: FILE: include/uapi/linux/cdrom.h:915:
+ __u8 reserved1: 2;
^
Which style should I follow?
> explaining synchronization. Bitfields w/ their implicit RMW ops tend
> to make people wonder about what the access rules are.
The slot and drawer bit field is assigned only once during ata probe
time in EH context, and accessed later in PM's callback context.
Not sure what access rule should I describe...
>
>> +static int run_atapi_cmd(struct ata_device *dev, const char *cdb,
>> + unsigned short cdb_len, char *buf, unsigned int buf_len)
>> +{
>> + struct ata_taskfile tf = {0};
>
> No need for 0. { } is enough and more generic.
Thanks for the info.
>
>> +
>> + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
>> + tf.command = ATA_CMD_PACKET;
>> +
>> + if (buf) {
>> + tf.protocol = ATAPI_PROT_PIO;
>> + tf.lbam = buf_len;
>> + } else {
>> + tf.protocol = ATAPI_PROT_NODATA;
>> + }
>> +
>> + return ata_exec_internal(dev, &tf, cdb,
>> + buf ? DMA_FROM_DEVICE : DMA_NONE, buf, buf_len, 0);
>> +}
>
> So, the function name is a bit of misnomer given that ATAPI commands
> are not limited to PIO or DMA_FROM_DEVICE. Also, this function ends
> up being used twice - once w/ read buffer and once w/o. Do we really
> want this function? It's not like exec_internal is difficult to use.
Then I'll remove this function.
>
>> +/*
>> + * Per the spec, only slot type and drawer type ODD can be supported
>> + *
>> + * Return 0 for slot type, 1 for drawer, -ENODEV for other types or on error.
>> + */
>
> Maybe bool odd_has_drawer() is better?
There are other types of ODD other than slot and drawer, and both slot
and drawer type ODDs can be supported for ZPODD. So a bool can't convey
such information :-)
>
>> +static int check_loading_mechanism(struct ata_device *dev)
>> +{
>> + char buf[16];
>> + unsigned int ret;
>> + struct rm_feature_desc *desc = (void *)(buf + 8);
>> +
>> + char cdb[] = { GPCMD_GET_CONFIGURATION,
>> + 2, /* only 1 feature descriptor requested */
>> + 0, 3, /* 3, removable medium feature */
>> + 0, 0, 0,/* reserved */
>> + 0, sizeof(buf),
>> + 0, 0, 0,
>> + };
>> +
>> + ret = run_atapi_cmd(dev, cdb, sizeof(cdb), buf, sizeof(buf));
>> + if (ret)
>> + return -ENODEV;
>> +
>> + if (be16_to_cpu(desc->feature_code) != 3)
>> + return -ENODEV;
>> +
>> + if (desc->mech_type == 0 && desc->load == 0 && desc->eject == 1)
>> + return 0; /* slot */
>> + else if (desc->mech_type == 1 && desc->load == 0 && desc->eject == 1)
>> + return 1; /* drawer */
>> + else
>> + return -ENODEV;
>> +}
>> +
>> +static bool odd_can_poweroff(struct ata_device *ata_dev)
>> +{
>> + acpi_handle handle;
>> + acpi_status status;
>> + struct acpi_device *acpi_dev;
>> +
>> + handle = ata_dev_acpi_handle(ata_dev);
>> + if (!handle)
>> + return false;
>> +
>> + status = acpi_bus_get_device(handle, &acpi_dev);
>> + if (ACPI_FAILURE(status))
>> + return false;
>> +
>> + return acpi_device_can_poweroff(acpi_dev);
>> +}
>> +
>> +void zpodd_init(struct ata_device *dev)
>> +{
>> + int ret;
>> + struct zpodd *zpodd;
>> +
>> + if (dev->zpodd)
>> + return;
>> +
>> + if (!odd_can_poweroff(dev))
>> + return;
>> +
>> + if ((ret = check_loading_mechanism(dev)) == -ENODEV)
>> + return;
>> +
>> + zpodd = kzalloc(sizeof(struct zpodd), GFP_KERNEL);
>> + if (!zpodd)
>> + return;
>> +
>> + if (ret)
>> + zpodd->drawer = true;
>> + else
>> + zpodd->slot = true;
>> +
>> + zpodd->dev = dev;
>> + dev->zpodd = zpodd;
>> +}
>> +
>> +void zpodd_exit(struct ata_device *dev)
>> +{
>> + kfree(dev->zpodd);
>> + dev->zpodd = NULL;
>> +}
>> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
>> index 7148a58..8cb4372 100644
>> --- a/drivers/ata/libata.h
>> +++ b/drivers/ata/libata.h
>> @@ -230,4 +230,18 @@ static inline void ata_sff_exit(void)
>> { }
>> #endif /* CONFIG_ATA_SFF */
>>
>> +/* libata-zpodd.c */
>> +#ifdef CONFIG_SATA_ZPODD
>> +void zpodd_init(struct ata_device *dev);
>> +void zpodd_exit(struct ata_device *dev);
>> +static inline bool zpodd_dev_enabled(struct ata_device *dev)
>> +{
>> + return dev->zpodd ? true : false;
>
> return dev->zpodd or return dev->zpodd != NULL?
>
> Other than the above nits, looks okay to me.
Thanks a lot for the review.
-Aaron
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Sascha Hauer @ 2013-01-08 8:57 UTC (permalink / raw)
To: Shawn Guo; +Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki
In-Reply-To: <1357628043-19256-3-git-send-email-shawn.guo@linaro.org>
On Tue, Jan 08, 2013 at 02:54:02PM +0800, Shawn Guo wrote:
> Add an imx6q-cpufreq for Freescale i.MX6Q SoC to handle the hardware
> specific frequency and voltage scaling requirements.
>
> The driver supports module build and is instantiated by the platform
> device/driver mechanism, so that it will be instantiated on other
> platform, as IMX is built with multiplatform support.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/cpufreq/Kconfig.arm | 9 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/imx6q-cpufreq.c | 325 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 335 insertions(+)
> create mode 100644 drivers/cpufreq/imx6q-cpufreq.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index a0b3661..9e628ba 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -77,6 +77,15 @@ config ARM_EXYNOS5250_CPUFREQ
> This adds the CPUFreq driver for Samsung EXYNOS5250
> SoC.
>
> +config ARM_IMX6Q_CPUFREQ
> + tristate "Freescale i.MX6Q cpufreq support"
> + depends on SOC_IMX6Q
> + depends on REGULATOR_ANATOP
> + help
> + This adds cpufreq driver support for Freescale i.MX6Q SOC.
> +
> + If in doubt, say N.
> +
> config ARM_SPEAR_CPUFREQ
> bool "SPEAr CPUFreq support"
> depends on PLAT_SPEAR
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 1f254ec0..31699a0 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += exynos-cpufreq.o
> obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o
> obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o
> obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o
> +obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
> obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
> obj-$(CONFIG_ARM_SPEAR_CPUFREQ) += spear-cpufreq.o
>
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> new file mode 100644
> index 0000000..8b3db8c
> --- /dev/null
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -0,0 +1,325 @@
> +/*
> + * Copyright (C) 2013 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/opp.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +#define PU_SOC_VOLTAGE_NORMAL 1250000
> +#define PU_SOC_VOLTAGE_HIGH 1275000
> +#define FREQ_1P2_GHZ 1200000000
> +
> +static struct regulator *arm_reg;
> +static struct regulator *pu_reg;
> +static struct regulator *soc_reg;
> +
> +static struct clk *arm_clk;
> +static struct clk *pll1_sys_clk;
> +static struct clk *pll1_sw_clk;
> +static struct clk *step_clk;
> +static struct clk *pll2_pfd2_396m_clk;
> +
> +static struct device *cpu_dev;
> +static struct cpufreq_frequency_table *freq_table;
> +static unsigned int transition_latency;
> +
> +static int imx6q_verify_speed(struct cpufreq_policy *policy)
> +{
> + return cpufreq_frequency_table_verify(policy, freq_table);
> +}
> +
> +static unsigned int imx6q_get_speed(unsigned int cpu)
> +{
> + return clk_get_rate(arm_clk) / 1000;
> +}
> +
> +static int imx6q_set_target(struct cpufreq_policy *policy,
> + unsigned int target_freq, unsigned int relation)
> +{
> + struct cpufreq_freqs freqs;
> + struct opp *opp;
> + unsigned long freq_hz, volt, volt_old;
> + unsigned int index, cpu;
> + int ret;
> +
> + ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> + relation, &index);
> + if (ret) {
> + pr_err("failed to match target freqency %d: %d\n",
> + target_freq, ret);
> + return ret;
> + }
> +
> + freq_hz = freq_table[index].frequency * 1000;
> + freqs.new = freq_hz / 1000;
> + freqs.old = clk_get_rate(arm_clk) / 1000;
> +
> + if (freqs.old == freqs.new)
> + return 0;
> +
> + for_each_online_cpu(cpu) {
> + freqs.cpu = cpu;
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> + }
> +
> + opp = opp_find_freq_ceil(cpu_dev, &freq_hz);
> + if (IS_ERR(opp)) {
> + pr_err("failed to find OPP for %ld\n", freq_hz);
> + return PTR_ERR(opp);
> + }
> +
> + volt = opp_get_voltage(opp);
> + volt_old = regulator_get_voltage(arm_reg);
> +
> + pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> + freqs.old / 1000, volt_old / 1000,
> + freqs.new / 1000, volt / 1000);
> +
> + /* scaling up? scale voltage before frequency */
> + if (freqs.new > freqs.old) {
> + ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> + if (ret) {
> + pr_err("failed to scale voltage up: %d\n", ret);
> + freqs.new = freqs.old;
> + return ret;
> + }
So this regulator_set_voltage_tol can fail, ...
> +
> + /*
> + * Need to increase vddpu and vddsoc for safety
> + * if we are about to run at 1.2 GHz.
> + */
> + if (freqs.new == FREQ_1P2_GHZ / 1000) {
> + regulator_set_voltage_tol(pu_reg,
> + PU_SOC_VOLTAGE_HIGH, 0);
> + regulator_set_voltage_tol(soc_reg,
> + PU_SOC_VOLTAGE_HIGH, 0);
... but these can't?
> + }
> + }
> +
> + /*
> + * The setpoints are selected per PLL/PDF frequencies, so we need to
> + * reprogram PLL for frequency scaling. The procedure of reprogramming
> + * PLL1 is as blow.
s/blow/below/
> + *
> + * - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
> + * - Disable pll1_sys_clk and reprogram it
> + * - Enable pll1_sys_clk and reparent pll1_sw_clk back to it
> + * - Disable pll2_pfd2_396m_clk
> + */
[...]
> +
> +static struct cpufreq_driver imx6q_cpufreq_driver = {
> + .verify = imx6q_verify_speed,
> + .target = imx6q_set_target,
> + .get = imx6q_get_speed,
> + .init = imx6q_cpufreq_init,
> + .exit = imx6q_cpufreq_exit,
> + .name = "imx6q-cpufreq",
> + .attr = imx6q_cpufreq_attr,
> +};
> +
> +static int imx6q_cpufreq_probe(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + int ret;
> +
> + np = of_find_node_by_path("/cpus/cpu@0");
> + if (!np) {
> + pr_err("failed to find cpu0 node\n");
> + return -ENOENT;
> + }
> +
> + cpu_dev = get_cpu_device(0);
> + if (!cpu_dev) {
> + pr_err("failed to get cpu0 device\n");
Since you have a struct device * you should use it throughout the driver
to give the messages a bit more context.
> + ret = -ENODEV;
> + goto put_node;
> + }
> +
> + cpu_dev->of_node = np;
> +
> + arm_clk = clk_get(cpu_dev, "arm");
> + pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
> + pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
> + step_clk = clk_get(cpu_dev, "step");
> + pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
devm_*?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v6 3/4] block: implement runtime pm strategy
From: Aaron Lu @ 2013-01-08 7:36 UTC (permalink / raw)
To: Alan Stern
Cc: Jens Axboe, Rafael J. Wysocki, James Bottomley, linux-pm,
linux-scsi, linux-kernel, Aaron Lu, Shane Huang
In-Reply-To: <Pine.LNX.4.44L0.1301071219540.1658-100000@iolanthe.rowland.org>
On 01/08/2013 01:21 AM, Alan Stern wrote:
> On Sun, 6 Jan 2013, Aaron Lu wrote:
>
>> From: Lin Ming <ming.m.lin@intel.com>
>>
>> When a request is added:
>> If device is suspended or is suspending and the request is not a
>> PM request, resume the device.
>>
>> When the last request finishes:
>> Call pm_runtime_mark_last_busy() and pm_runtime_autosuspend().
>>
>> When pick a request:
>> If device is resuming/suspending, then only PM request is allowed to go.
>> Return NULL for other cases.
>>
>> [aaron.lu@intel.com: PM request does not involve nr_pending counting]
>> [aaron.lu@intel.com: No need to check q->dev]
>> [aaron.lu@intel.com: Autosuspend when the last request finished]
>> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -974,6 +974,40 @@ extern int blk_pre_runtime_suspend(struct request_queue *q);
>> extern void blk_post_runtime_suspend(struct request_queue *q, int err);
>> extern void blk_pre_runtime_resume(struct request_queue *q);
>> extern void blk_post_runtime_resume(struct request_queue *q, int err);
>> +
>> +static inline void blk_pm_put_request(struct request *rq)
>> +{
>> + if (!(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) {
>> + pm_runtime_mark_last_busy(rq->q->dev);
>> + pm_runtime_autosuspend(rq->q->dev);
>> + }
>> +}
>> +
>> +static inline struct request *blk_pm_peek_request(
>> + struct request_queue *q, struct request *rq)
>> +{
>> + if (q->rpm_status == RPM_SUSPENDED ||
>> + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM)))
>> + return NULL;
>> + else
>> + return rq;
>> +}
>> +
>> +static inline void blk_pm_requeue_request(struct request *rq)
>> +{
>> + if (!(rq->cmd_flags & REQ_PM))
>> + rq->q->nr_pending--;
>> +}
>> +
>> +static inline void blk_pm_add_request(struct request_queue *q,
>> + struct request *rq)
>> +{
>> + if (!(rq->cmd_flags & REQ_PM) &&
>> + q->nr_pending++ == 0 &&
>> + (q->rpm_status == RPM_SUSPENDED ||
>> + q->rpm_status == RPM_SUSPENDING))
>> + pm_request_resume(q->dev);
>> +}
>
> These routines also don't belong in include/linux. And they don't need
> to be marked inline.
OK, will move them.
What about create a new file blk-pm.c for all these block pm related
code?
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCH v6 0/4] block layer runtime pm
From: Aaron Lu @ 2013-01-08 7:33 UTC (permalink / raw)
To: Alan Stern
Cc: Jens Axboe, Rafael J. Wysocki, James Bottomley, linux-pm,
linux-scsi, linux-kernel, Aaron Lu, Shane Huang
In-Reply-To: <Pine.LNX.4.44L0.1301071200420.1658-100000@iolanthe.rowland.org>
On 01/08/2013 01:11 AM, Alan Stern wrote:
> On Sun, 6 Jan 2013, Aaron Lu wrote:
>
>> In August 2010, Jens and Alan discussed about "Runtime PM and the block
>> layer". http://marc.info/?t=128259108400001&r=1&w=2
>> And then Alan has given a detailed implementation guide:
>> http://marc.info/?l=linux-scsi&m=133727953625963&w=2
>>
>> To test:
>> # ls -l /sys/block/sda
>> /sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda
>>
>> # echo 10000 > /sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/power/autosuspend_delay_ms
>> # echo auto > /sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/power/control
>> Then you'll see sda is suspended after 10secs idle.
>>
>> [ 1767.680192] sd 2:0:0:0: [sda] Synchronizing SCSI cache
>> [ 1767.680317] sd 2:0:0:0: [sda] Stopping disk
>>
>> And if you do some IO, it will resume immediately.
>> [ 1791.052438] sd 2:0:0:0: [sda] Starting disk
>>
>> For test, I often set the autosuspend time to 1 second. If you are using
>> a GUI, the 10 seconds delay may be too long that the disk can not enter
>> runtime suspended state.
>>
>> Note that sd's runtime suspend callback will dump some kernel messages
>> and the syslog daemon will write kernel message to /var/log/messages,
>> making the disk instantly resume after suspended. So for test, the
>> syslog daemon should better be temporarily stopped.
>>
>> v6:
>> Take over from Lin Ming.
>>
>> - Instead of put the device into autosuspend state in
>> blk_post_runtime_suspend, do it when the last request is finished.
>> This can also solve the problem illustrated below:
>>
>> thread A thread B
>> |suspend timer expired |
>> | ... ... |a new request comes in,
>> | ... ... |blk_pm_add_request
>> | ... ... |skip request_resume due to
>> | ... ... |q->status is still RPM_ACTIVE
>> | rpm_suspend | ... ...
>> | scsi_runtime_suspend | ... ...
>> | blk_pre_runtime_suspend | ... ...
>> | return -EBUSY due to nr_pending | ... ...
>> | rpm_suspend done | ... ...
>> | | blk_pm_put_request, mark last busy
>>
>> But no more trigger point, and the device will stay at RPM_ACTIVE state.
>> Run pm_runtime_autosuspend after the last request is finished solved
>> this problem.
>
> This doesn't look like the best solution, because it involves adding a
> nontrivial routine (pm_runtime_autosuspend) to a hot path.
Oh right, I didn't realize this. Thanks for pointing this out.
>
> How about this instead? When blk_pre_runtime_suspend returns -EBUSY,
> have it do a mark-last-busy. Then rpm_suspend will automatically
> reschedule the autosuspend for later.
Yes, this is better.
>
>> - Requests which have the REQ_PM flag should not involve nr_pending
>> counting, or we may lose the condition to resume the device:
>> Suppose queue is active and nr_pending is 0. Then a REQ_PM request
>> comes and nr_pending will be increased to 1, but since the request has
>> REQ_PM flag, it will not cause resume. Before it is finished, a normal
>> request comes in, and since nr_pending is 1 now, it will not trigger
>> the resume of the device either. Bug.
>>
>> - Do not quiesce the device in scsi bus level runtime suspend callback.
>> Since the only reason the device is to be runtime suspended is due to
>> no more requests pending for it, quiesce it is pointless.
>>
>> - Remove scsi_autopm_* from sd_check_events as we are request driven.
>>
>> - Call blk_pm_runtime_init in scsi_sysfs_initialize_dev, so that we do
>> not need to check queue's device in blk_pm_add/put_request.
>
> I think you still need to have that check. After all, the block layer
> has other users besides the SCSI stack, and those users don't call
> blk_pm_runtime_init.
Right...
So this also reminds me that as long as CONFIG_PM_RUNTIME is selected,
the blk_pm_add/put/peek_request functions will be in the block IO path.
Shall we introduce a new config option to selectively build block
runtime PM functionality? something like CONFIG_BLK_PM_RUNTIME perhaps?
Just some condition checks in those functions, not sure if it is worth a
new config though. Please suggest, thanks.
>
>> - Do not mark last busy and initiate an autosuspend for the device in
>> blk_pm_runtime_init function.
>>
>> - Do not mark last busy and initiate an autosuspend for the device in
>> block_post_runtime_resume, as when the request that triggered the
>> resume finished, the blk_pm_put_request will mark last busy and
>> initiate an autosuspend.
>
> If you make the change that I recommended above then this is still
> necessary.
Yes, they are needed. Thanks!
-Aaron
^ permalink raw reply
* [PATCH 3/3] ARM: imx: enable imx6q-cpufreq support
From: Shawn Guo @ 2013-01-08 6:54 UTC (permalink / raw)
To: cpufreq, linux-pm, linux-arm-kernel
Cc: Rafael J. Wysocki, Sascha Hauer, Shawn Guo
In-Reply-To: <1357628043-19256-1-git-send-email-shawn.guo@linaro.org>
Update operating-points per hardware document and add support for
1 GHz and 1.2 GHz frequencies.
400 MHz, 800 MHz and 1 GHz should be supported by all i.MX6Q chips,
while 1.2 GHz support needs to know from OTP fuse bit.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
arch/arm/boot/dts/imx6q.dtsi | 19 ++++++++----
arch/arm/mach-imx/mach-imx6q.c | 64 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index d6265ca..17c5618 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -38,12 +38,18 @@
next-level-cache = <&L2>;
operating-points = <
/* kHz uV */
- 792000 1100000
+ 996000 1250000
+ 792000 1150000
396000 950000
- 198000 850000
>;
clock-latency = <61036>; /* two CLK32 periods */
- cpu0-supply = <®_cpu>;
+ clocks = <&clks 104>, <&clks 6>, <&clks 16>,
+ <&clks 17>, <&clks 170>;
+ clock-names = "arm", "pll2_pfd2_396m", "step",
+ "pll1_sw", "pll1_sys";
+ arm-supply = <®_arm>;
+ pu-supply = <®_pu>;
+ soc-supply = <®_soc>;
};
cpu@1 {
@@ -471,7 +477,7 @@
anatop-max-voltage = <2750000>;
};
- reg_cpu: regulator-vddcore@140 {
+ reg_arm: regulator-vddcore@140 {
compatible = "fsl,anatop-regulator";
regulator-name = "cpu";
regulator-min-microvolt = <725000>;
@@ -485,7 +491,7 @@
anatop-max-voltage = <1450000>;
};
- regulator-vddpu@140 {
+ reg_pu: regulator-vddpu@140 {
compatible = "fsl,anatop-regulator";
regulator-name = "vddpu";
regulator-min-microvolt = <725000>;
@@ -499,7 +505,7 @@
anatop-max-voltage = <1450000>;
};
- regulator-vddsoc@140 {
+ reg_soc: regulator-vddsoc@140 {
compatible = "fsl,anatop-regulator";
regulator-name = "vddsoc";
regulator-min-microvolt = <725000>;
@@ -965,6 +971,7 @@
};
ocotp@021bc000 {
+ compatible = "fsl,imx6q-ocotp";
reg = <0x021bc000 0x4000>;
};
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 4eb1b3a..26f63f28 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -12,6 +12,7 @@
#include <linux/clk.h>
#include <linux/clkdev.h>
+#include <linux/cpu.h>
#include <linux/cpuidle.h>
#include <linux/delay.h>
#include <linux/export.h>
@@ -22,6 +23,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/opp.h>
#include <linux/phy.h>
#include <linux/regmap.h>
#include <linux/micrel_phy.h>
@@ -209,9 +211,71 @@ static struct cpuidle_driver imx6q_cpuidle_driver = {
.state_count = 1,
};
+#define OCOTP_CFG3 0x440
+#define OCOTP_CFG3_SPEED_SHIFT 16
+#define OCOTP_CFG3_SPEED_1P2GHZ 0x3
+
+static void __init imx6q_opp_check_1p2ghz(struct device *cpu_dev)
+{
+ struct device_node *np;
+ void __iomem *base;
+ u32 val;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp");
+ base = of_iomap(np, 0);
+ if (!base) {
+ pr_warn("failed to map ocotp\n");
+ goto put_node;
+ }
+
+ val = readl_relaxed(base + OCOTP_CFG3);
+ val >>= OCOTP_CFG3_SPEED_SHIFT;
+ if ((val & 0x3) == OCOTP_CFG3_SPEED_1P2GHZ)
+ if (opp_add(cpu_dev, 1200000000, 1275000))
+ pr_warn("failed to add 1.2 GHz operating point\n");
+
+put_node:
+ of_node_put(np);
+}
+
+static void __init imx6q_opp_init(void)
+{
+ struct device *cpu_dev;
+ struct device_node *np;
+
+ np = of_find_node_by_path("/cpus/cpu@0");
+ if (!np) {
+ pr_warn("failed to find cpu0 node\n");
+ return;
+ }
+
+ cpu_dev = get_cpu_device(0);
+ if (!cpu_dev) {
+ pr_warn("failed to get cpu0 device\n");
+ goto put_node;
+ }
+
+ cpu_dev->of_node = np;
+ if (of_init_opp_table(cpu_dev))
+ pr_warn("failed to init OPP table\n");
+
+ imx6q_opp_check_1p2ghz(cpu_dev);
+
+put_node:
+ of_node_put(np);
+}
+
static void __init imx6q_init_late(void)
{
imx_cpuidle_init(&imx6q_cpuidle_driver);
+
+ if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+ struct platform_device_info pdevinfo = {
+ .name = "imx6q_cpufreq",
+ };
+ imx6q_opp_init();
+ platform_device_register_full(&pdevinfo);
+ }
}
static void __init imx6q_map_io(void)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Shawn Guo @ 2013-01-08 6:54 UTC (permalink / raw)
To: cpufreq, linux-pm, linux-arm-kernel
Cc: Rafael J. Wysocki, Sascha Hauer, Shawn Guo
In-Reply-To: <1357628043-19256-1-git-send-email-shawn.guo@linaro.org>
Add an imx6q-cpufreq for Freescale i.MX6Q SoC to handle the hardware
specific frequency and voltage scaling requirements.
The driver supports module build and is instantiated by the platform
device/driver mechanism, so that it will be instantiated on other
platform, as IMX is built with multiplatform support.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/cpufreq/Kconfig.arm | 9 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/imx6q-cpufreq.c | 325 +++++++++++++++++++++++++++++++++++++++
3 files changed, 335 insertions(+)
create mode 100644 drivers/cpufreq/imx6q-cpufreq.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index a0b3661..9e628ba 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -77,6 +77,15 @@ config ARM_EXYNOS5250_CPUFREQ
This adds the CPUFreq driver for Samsung EXYNOS5250
SoC.
+config ARM_IMX6Q_CPUFREQ
+ tristate "Freescale i.MX6Q cpufreq support"
+ depends on SOC_IMX6Q
+ depends on REGULATOR_ANATOP
+ help
+ This adds cpufreq driver support for Freescale i.MX6Q SOC.
+
+ If in doubt, say N.
+
config ARM_SPEAR_CPUFREQ
bool "SPEAr CPUFreq support"
depends on PLAT_SPEAR
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 1f254ec0..31699a0 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += exynos-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o
+obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ) += spear-cpufreq.o
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
new file mode 100644
index 0000000..8b3db8c
--- /dev/null
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/opp.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#define PU_SOC_VOLTAGE_NORMAL 1250000
+#define PU_SOC_VOLTAGE_HIGH 1275000
+#define FREQ_1P2_GHZ 1200000000
+
+static struct regulator *arm_reg;
+static struct regulator *pu_reg;
+static struct regulator *soc_reg;
+
+static struct clk *arm_clk;
+static struct clk *pll1_sys_clk;
+static struct clk *pll1_sw_clk;
+static struct clk *step_clk;
+static struct clk *pll2_pfd2_396m_clk;
+
+static struct device *cpu_dev;
+static struct cpufreq_frequency_table *freq_table;
+static unsigned int transition_latency;
+
+static int imx6q_verify_speed(struct cpufreq_policy *policy)
+{
+ return cpufreq_frequency_table_verify(policy, freq_table);
+}
+
+static unsigned int imx6q_get_speed(unsigned int cpu)
+{
+ return clk_get_rate(arm_clk) / 1000;
+}
+
+static int imx6q_set_target(struct cpufreq_policy *policy,
+ unsigned int target_freq, unsigned int relation)
+{
+ struct cpufreq_freqs freqs;
+ struct opp *opp;
+ unsigned long freq_hz, volt, volt_old;
+ unsigned int index, cpu;
+ int ret;
+
+ ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
+ relation, &index);
+ if (ret) {
+ pr_err("failed to match target freqency %d: %d\n",
+ target_freq, ret);
+ return ret;
+ }
+
+ freq_hz = freq_table[index].frequency * 1000;
+ freqs.new = freq_hz / 1000;
+ freqs.old = clk_get_rate(arm_clk) / 1000;
+
+ if (freqs.old == freqs.new)
+ return 0;
+
+ for_each_online_cpu(cpu) {
+ freqs.cpu = cpu;
+ cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+ }
+
+ opp = opp_find_freq_ceil(cpu_dev, &freq_hz);
+ if (IS_ERR(opp)) {
+ pr_err("failed to find OPP for %ld\n", freq_hz);
+ return PTR_ERR(opp);
+ }
+
+ volt = opp_get_voltage(opp);
+ volt_old = regulator_get_voltage(arm_reg);
+
+ pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
+ freqs.old / 1000, volt_old / 1000,
+ freqs.new / 1000, volt / 1000);
+
+ /* scaling up? scale voltage before frequency */
+ if (freqs.new > freqs.old) {
+ ret = regulator_set_voltage_tol(arm_reg, volt, 0);
+ if (ret) {
+ pr_err("failed to scale voltage up: %d\n", ret);
+ freqs.new = freqs.old;
+ return ret;
+ }
+
+ /*
+ * Need to increase vddpu and vddsoc for safety
+ * if we are about to run at 1.2 GHz.
+ */
+ if (freqs.new == FREQ_1P2_GHZ / 1000) {
+ regulator_set_voltage_tol(pu_reg,
+ PU_SOC_VOLTAGE_HIGH, 0);
+ regulator_set_voltage_tol(soc_reg,
+ PU_SOC_VOLTAGE_HIGH, 0);
+ }
+ }
+
+ /*
+ * The setpoints are selected per PLL/PDF frequencies, so we need to
+ * reprogram PLL for frequency scaling. The procedure of reprogramming
+ * PLL1 is as blow.
+ *
+ * - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
+ * - Disable pll1_sys_clk and reprogram it
+ * - Enable pll1_sys_clk and reparent pll1_sw_clk back to it
+ * - Disable pll2_pfd2_396m_clk
+ */
+ clk_prepare_enable(pll2_pfd2_396m_clk);
+ clk_set_parent(step_clk, pll2_pfd2_396m_clk);
+ clk_set_parent(pll1_sw_clk, step_clk);
+ clk_prepare_enable(pll1_sys_clk);
+ if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) {
+ clk_disable_unprepare(pll1_sys_clk);
+ clk_set_rate(pll1_sys_clk, freqs.new * 1000);
+ clk_prepare_enable(pll1_sys_clk);
+ clk_set_parent(pll1_sw_clk, pll1_sys_clk);
+ clk_disable_unprepare(pll2_pfd2_396m_clk);
+ } else {
+ /*
+ * Disable pll1_sys_clk if pll2_pfd2_396m_clk is sufficient
+ * to provide the frequency.
+ */
+ clk_disable_unprepare(pll1_sys_clk);
+ }
+
+ /* Ensure the arm clock divider is what we expect */
+ ret = clk_set_rate(arm_clk, freqs.new * 1000);
+ if (ret) {
+ pr_err("failed to set clock rate: %d\n", ret);
+ regulator_set_voltage_tol(arm_reg, volt_old, 0);
+ return ret;
+ }
+
+ /* scaling down? scale voltage after frequency */
+ if (freqs.new < freqs.old) {
+ ret = regulator_set_voltage_tol(arm_reg, volt, 0);
+ if (ret)
+ pr_warn("failed to scale voltage down: %d\n", ret);
+
+ if (freqs.old == FREQ_1P2_GHZ / 1000) {
+ regulator_set_voltage_tol(pu_reg,
+ PU_SOC_VOLTAGE_NORMAL, 0);
+ regulator_set_voltage_tol(soc_reg,
+ PU_SOC_VOLTAGE_NORMAL, 0);
+ }
+ }
+
+ for_each_online_cpu(cpu) {
+ freqs.cpu = cpu;
+ cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+ }
+
+ return 0;
+}
+
+static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
+{
+ int ret;
+
+ if (policy->cpu != 0)
+ return -EINVAL;
+
+ ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+ if (ret) {
+ pr_err("invalid frequency table: %d\n", ret);
+ return ret;
+ }
+
+ policy->cpuinfo.transition_latency = transition_latency;
+ policy->cur = clk_get_rate(arm_clk) / 1000;
+ policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
+ cpumask_setall(policy->cpus);
+ cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
+ return 0;
+}
+
+static int imx6q_cpufreq_exit(struct cpufreq_policy *policy)
+{
+ cpufreq_frequency_table_put_attr(policy->cpu);
+ return 0;
+}
+
+static struct freq_attr *imx6q_cpufreq_attr[] = {
+ &cpufreq_freq_attr_scaling_available_freqs,
+ NULL,
+};
+
+static struct cpufreq_driver imx6q_cpufreq_driver = {
+ .verify = imx6q_verify_speed,
+ .target = imx6q_set_target,
+ .get = imx6q_get_speed,
+ .init = imx6q_cpufreq_init,
+ .exit = imx6q_cpufreq_exit,
+ .name = "imx6q-cpufreq",
+ .attr = imx6q_cpufreq_attr,
+};
+
+static int imx6q_cpufreq_probe(struct platform_device *pdev)
+{
+ struct device_node *np;
+ int ret;
+
+ np = of_find_node_by_path("/cpus/cpu@0");
+ if (!np) {
+ pr_err("failed to find cpu0 node\n");
+ return -ENOENT;
+ }
+
+ cpu_dev = get_cpu_device(0);
+ if (!cpu_dev) {
+ pr_err("failed to get cpu0 device\n");
+ ret = -ENODEV;
+ goto put_node;
+ }
+
+ cpu_dev->of_node = np;
+
+ arm_clk = clk_get(cpu_dev, "arm");
+ pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
+ pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
+ step_clk = clk_get(cpu_dev, "step");
+ pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
+ if (IS_ERR(arm_clk) || IS_ERR(pll1_sys_clk) || IS_ERR(pll1_sw_clk) ||
+ IS_ERR(step_clk) || IS_ERR(pll2_pfd2_396m_clk)) {
+ pr_err("failed to get clocks\n");
+ ret = -ENOENT;
+ goto put_node;
+ }
+
+ arm_reg = regulator_get(cpu_dev, "arm");
+ pu_reg = regulator_get(cpu_dev, "pu");
+ soc_reg = regulator_get(cpu_dev, "soc");
+ if (!arm_reg || !pu_reg || !soc_reg) {
+ pr_err("failed to get regulators\n");
+ ret = -ENOENT;
+ goto put_clk;
+ }
+
+ /* We expect an OPP table supplied by platform */
+ ret = opp_get_opp_count(cpu_dev);
+ if (ret < 0) {
+ pr_err("no OPP table is found: %d\n", ret);
+ goto put_reg;
+ }
+
+ ret = opp_init_cpufreq_table(cpu_dev, &freq_table);
+ if (ret) {
+ pr_err("failed to init cpufreq table: %d\n", ret);
+ goto put_reg;
+ }
+
+ if (of_property_read_u32(np, "clock-latency", &transition_latency))
+ transition_latency = CPUFREQ_ETERNAL;
+
+ ret = cpufreq_register_driver(&imx6q_cpufreq_driver);
+ if (ret) {
+ pr_err("failed register driver: %d\n", ret);
+ goto free_freq_table;
+ }
+
+ of_node_put(np);
+ return 0;
+
+free_freq_table:
+ opp_free_cpufreq_table(cpu_dev, &freq_table);
+put_reg:
+ regulator_put(soc_reg);
+ regulator_put(pu_reg);
+ regulator_put(arm_reg);
+put_clk:
+ clk_put(pll2_pfd2_396m_clk);
+ clk_put(step_clk);
+ clk_put(pll1_sw_clk);
+ clk_put(pll1_sys_clk);
+ clk_put(arm_clk);
+put_node:
+ of_node_put(np);
+ return ret;
+}
+
+static int imx6q_cpufreq_remove(struct platform_device *pdev)
+{
+ cpufreq_unregister_driver(&imx6q_cpufreq_driver);
+ opp_free_cpufreq_table(cpu_dev, &freq_table);
+ regulator_put(soc_reg);
+ regulator_put(pu_reg);
+ regulator_put(arm_reg);
+ clk_put(pll2_pfd2_396m_clk);
+ clk_put(step_clk);
+ clk_put(pll1_sw_clk);
+ clk_put(pll1_sys_clk);
+ clk_put(arm_clk);
+
+ return 0;
+}
+
+static struct platform_driver imx6q_cpufreq_platdrv = {
+ .driver = {
+ .name = "imx6q_cpufreq",
+ .owner = THIS_MODULE,
+ },
+ .probe = imx6q_cpufreq_probe,
+ .remove = imx6q_cpufreq_remove,
+};
+module_platform_driver(imx6q_cpufreq_platdrv);
+
+MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
+MODULE_DESCRIPTION("Freescale i.MX6Q cpufreq driver");
+MODULE_LICENSE("GPL");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/3] PM / OPP: Export more symbols for module usage
From: Shawn Guo @ 2013-01-08 6:54 UTC (permalink / raw)
To: cpufreq, linux-pm, linux-arm-kernel
Cc: Rafael J. Wysocki, Sascha Hauer, Shawn Guo
In-Reply-To: <1357628043-19256-1-git-send-email-shawn.guo@linaro.org>
Export opp_init_cpufreq_table and opp_free_cpufreq_table for module
usage.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/base/power/opp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 50b2831..d16db9a 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -661,6 +661,7 @@ int opp_init_cpufreq_table(struct device *dev,
return 0;
}
+EXPORT_SYMBOL(opp_init_cpufreq_table);
/**
* opp_free_cpufreq_table() - free the cpufreq table
@@ -678,6 +679,7 @@ void opp_free_cpufreq_table(struct device *dev,
kfree(*table);
*table = NULL;
}
+EXPORT_SYMBOL(opp_free_cpufreq_table);
#endif /* CONFIG_CPU_FREQ */
/**
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/3] Add imx6q-cpufreq driver support
From: Shawn Guo @ 2013-01-08 6:54 UTC (permalink / raw)
To: cpufreq, linux-pm, linux-arm-kernel
Cc: Rafael J. Wysocki, Sascha Hauer, Shawn Guo
The generic cpufreq-cpu0 driver works well for imx6q before. But it
does not work for 1 GHz and 1.2 GHz operating points the series adds
here. We ended up with having to add an imx6q specific cpufreq driver
to handle those special frequency and voltage scaling requirements for
the new operating points.
I expect patches #1 and #2 go via cpufreq tree and the #3 through
arm-soc tree.
Shawn Guo (3):
PM / OPP: Export more symbols for module usage
cpufreq: add imx6q-cpufreq driver
ARM: imx: enable imx6q-cpufreq support
arch/arm/boot/dts/imx6q.dtsi | 19 ++-
arch/arm/mach-imx/mach-imx6q.c | 64 ++++++++
drivers/base/power/opp.c | 2 +
drivers/cpufreq/Kconfig.arm | 9 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/imx6q-cpufreq.c | 325 +++++++++++++++++++++++++++++++++++++++
6 files changed, 414 insertions(+), 6 deletions(-)
create mode 100644 drivers/cpufreq/imx6q-cpufreq.c
--
1.7.9.5
^ permalink raw reply
* [RFC PATCH] PM / devfreq: Add runtime-pm support
From: Rajagopal Venkat @ 2013-01-08 6:40 UTC (permalink / raw)
To: myungjoo.ham, kyungmin.park, mturquette, rjw
Cc: patches, linaro-dev, linux-pm, linux-kernel, Rajagopal Venkat
Instead of devfreq device driver explicitly calling devfreq suspend
and resume apis perhaps from runtime-pm suspend and resume callbacks,
let devfreq core handle it automatically.
Attach devfreq core to runtime-pm framework so that, devfreq device
driver pm_runtime_suspend() will automatically suspend the devfreq
and pm_runtime_resume() will resume the devfreq.
Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
---
drivers/devfreq/devfreq.c | 89 ++++++++++++++++++++++++++++++++++++++-------
include/linux/devfreq.h | 12 ------
2 files changed, 76 insertions(+), 25 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 4c50235..781ea47 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -25,10 +25,9 @@
#include <linux/list.h>
#include <linux/printk.h>
#include <linux/hrtimer.h>
+#include <linux/pm_runtime.h>
#include "governor.h"
-static struct class *devfreq_class;
-
/*
* devfreq core provides delayed work based load monitoring helper
* functions. Governors can use these or can implement their own
@@ -42,6 +41,9 @@ static LIST_HEAD(devfreq_governor_list);
static LIST_HEAD(devfreq_list);
static DEFINE_MUTEX(devfreq_list_lock);
+static int devfreq_suspend_device(struct devfreq *devfreq);
+static int devfreq_resume_device(struct devfreq *devfreq);
+
/**
* find_device_devfreq() - find devfreq struct using device pointer
* @dev: device pointer used to lookup device devfreq.
@@ -453,6 +455,61 @@ static void devfreq_dev_release(struct device *dev)
_remove_devfreq(devfreq, true);
}
+static int devfreq_runtime_suspend(struct device *dev)
+{
+ int ret;
+ struct devfreq *devfreq;
+
+ mutex_lock(&devfreq_list_lock);
+ devfreq = find_device_devfreq(dev);
+ mutex_unlock(&devfreq_list_lock);
+
+ ret = devfreq_suspend_device(devfreq);
+ if (ret < 0)
+ goto out;
+
+ ret = pm_generic_runtime_suspend(dev);
+out:
+ return ret;
+}
+
+static int devfreq_runtime_resume(struct device *dev)
+{
+ int ret;
+ struct devfreq *devfreq;
+
+ mutex_lock(&devfreq_list_lock);
+ devfreq = find_device_devfreq(dev);
+ mutex_unlock(&devfreq_list_lock);
+
+ ret = devfreq_resume_device(devfreq);
+ if (ret < 0)
+ goto out;
+
+ ret = pm_generic_runtime_resume(dev);
+out:
+ return ret;
+}
+
+static int devfreq_runtime_idle(struct device *dev)
+{
+ return pm_generic_runtime_idle(dev);
+}
+
+static const struct dev_pm_ops devfreq_pm_ops = {
+ SET_RUNTIME_PM_OPS(
+ devfreq_runtime_suspend,
+ devfreq_runtime_resume,
+ devfreq_runtime_idle
+ )
+};
+
+static struct class devfreq_class = {
+ .name = "devfreq",
+ .owner = THIS_MODULE,
+ .pm = &devfreq_pm_ops,
+};
+
/**
* devfreq_add_device() - Add devfreq feature to the device
* @dev: the device to add devfreq feature.
@@ -494,8 +551,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_init(&devfreq->lock);
mutex_lock(&devfreq->lock);
+ dev->class = &devfreq_class;
devfreq->dev.parent = dev;
- devfreq->dev.class = devfreq_class;
+ devfreq->dev.class = &devfreq_class;
devfreq->dev.release = devfreq_dev_release;
devfreq->profile = profile;
strncpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
@@ -538,6 +596,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
goto err_init;
}
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+
return devfreq;
err_init:
@@ -569,7 +630,7 @@ EXPORT_SYMBOL(devfreq_remove_device);
* devfreq_suspend_device() - Suspend devfreq of a device.
* @devfreq: the devfreq instance to be suspended
*/
-int devfreq_suspend_device(struct devfreq *devfreq)
+static int devfreq_suspend_device(struct devfreq *devfreq)
{
if (!devfreq)
return -EINVAL;
@@ -580,13 +641,12 @@ int devfreq_suspend_device(struct devfreq *devfreq)
return devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_SUSPEND, NULL);
}
-EXPORT_SYMBOL(devfreq_suspend_device);
/**
* devfreq_resume_device() - Resume devfreq of a device.
* @devfreq: the devfreq instance to be resumed
*/
-int devfreq_resume_device(struct devfreq *devfreq)
+static int devfreq_resume_device(struct devfreq *devfreq)
{
if (!devfreq)
return -EINVAL;
@@ -597,12 +657,12 @@ int devfreq_resume_device(struct devfreq *devfreq)
return devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_RESUME, NULL);
}
-EXPORT_SYMBOL(devfreq_resume_device);
/**
* devfreq_add_governor() - Add devfreq governor
* @governor: the devfreq governor to be added
*/
+
int devfreq_add_governor(struct devfreq_governor *governor)
{
struct devfreq_governor *g;
@@ -770,6 +830,7 @@ out:
ret = count;
return ret;
}
+
static ssize_t show_available_governors(struct device *d,
struct device_attribute *attr,
char *buf)
@@ -992,19 +1053,21 @@ static struct device_attribute devfreq_attrs[] = {
static int __init devfreq_init(void)
{
- devfreq_class = class_create(THIS_MODULE, "devfreq");
- if (IS_ERR(devfreq_class)) {
+ int err;
+
+ err = class_register(&devfreq_class);
+ if (err < 0) {
pr_err("%s: couldn't create class\n", __FILE__);
- return PTR_ERR(devfreq_class);
+ return err;
}
devfreq_wq = create_freezable_workqueue("devfreq_wq");
if (IS_ERR(devfreq_wq)) {
- class_destroy(devfreq_class);
+ class_unregister(&devfreq_class);
pr_err("%s: couldn't create workqueue\n", __FILE__);
return PTR_ERR(devfreq_wq);
}
- devfreq_class->dev_attrs = devfreq_attrs;
+ devfreq_class.dev_attrs = devfreq_attrs;
return 0;
}
@@ -1012,7 +1075,7 @@ subsys_initcall(devfreq_init);
static void __exit devfreq_exit(void)
{
- class_destroy(devfreq_class);
+ class_unregister(&devfreq_class);
destroy_workqueue(devfreq_wq);
}
module_exit(devfreq_exit);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index e83ef39..b84840c 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -181,8 +181,6 @@ extern struct devfreq *devfreq_add_device(struct device *dev,
const char *governor_name,
void *data);
extern int devfreq_remove_device(struct devfreq *devfreq);
-extern int devfreq_suspend_device(struct devfreq *devfreq);
-extern int devfreq_resume_device(struct devfreq *devfreq);
/* Helper functions for devfreq user device driver with OPP. */
extern struct opp *devfreq_recommended_opp(struct device *dev,
@@ -226,16 +224,6 @@ static int devfreq_remove_device(struct devfreq *devfreq)
return 0;
}
-static int devfreq_suspend_device(struct devfreq *devfreq)
-{
- return 0;
-}
-
-static int devfreq_resume_device(struct devfreq *devfreq)
-{
- return 0;
-}
-
static struct opp *devfreq_recommended_opp(struct device *dev,
unsigned long *freq, u32 flags)
{
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/3] PM / devfreq: set min/max freq limit from freq table
From: Rajagopal Venkat @ 2013-01-08 5:50 UTC (permalink / raw)
To: myungjoo.ham, kyungmin.park, mturquette, rjw
Cc: patches, linaro-dev, linux-pm, linux-kernel, Rajagopal Venkat
Set devfreq device min and max frequency limits when device
is added to devfreq, provided frequency table is supplied.
This helps governors to suggest target frequency with in
limits.
Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
---
drivers/devfreq/devfreq.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a8f0173..5782c9b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -69,6 +69,29 @@ static struct devfreq *find_device_devfreq(struct device *dev)
}
/**
+ * devfreq_set_freq_limits() - Set min and max frequency from freq_table
+ * @devfreq: the devfreq instance
+ */
+static void devfreq_set_freq_limits(struct devfreq *devfreq)
+{
+ int idx;
+ unsigned long min = ~0, max = 0;
+
+ if (!devfreq->profile->freq_table)
+ return;
+
+ for (idx = 0; idx < devfreq->profile->max_state; idx++) {
+ if (min > devfreq->profile->freq_table[idx])
+ min = devfreq->profile->freq_table[idx];
+ if (max < devfreq->profile->freq_table[idx])
+ max = devfreq->profile->freq_table[idx];
+ }
+
+ devfreq->min_freq = min;
+ devfreq->max_freq = max;
+}
+
+/**
* devfreq_get_freq_level() - Lookup freq_table for the frequency
* @devfreq: the devfreq instance
* @freq: the target frequency
@@ -476,6 +499,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->profile->max_state,
GFP_KERNEL);
devfreq->last_stat_updated = jiffies;
+ devfreq_set_freq_limits(devfreq);
dev_set_name(&devfreq->dev, dev_name(dev));
err = device_register(&devfreq->dev);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/3] PM / devfreq: account suspend/resume for stats
From: Rajagopal Venkat @ 2013-01-08 5:50 UTC (permalink / raw)
To: myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
mturquette-QSEj5FYQhm4dnm+yROfE0A, rjw-KKrjLPT3xs0
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
patches-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1357624239-13938-1-git-send-email-rajagopal.venkat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
devfreq stats is not taking device suspend and resume into
account. Fix it.
Signed-off-by: Rajagopal Venkat <rajagopal.venkat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/devfreq/devfreq.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2843a22..4c50235 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -297,6 +297,7 @@ void devfreq_monitor_suspend(struct devfreq *devfreq)
return;
}
+ devfreq_update_status(devfreq, devfreq->previous_freq);
devfreq->stop_polling = true;
mutex_unlock(&devfreq->lock);
cancel_delayed_work_sync(&devfreq->work);
@@ -313,6 +314,8 @@ EXPORT_SYMBOL(devfreq_monitor_suspend);
*/
void devfreq_monitor_resume(struct devfreq *devfreq)
{
+ unsigned long freq;
+
mutex_lock(&devfreq->lock);
if (!devfreq->stop_polling)
goto out;
@@ -321,8 +324,14 @@ void devfreq_monitor_resume(struct devfreq *devfreq)
devfreq->profile->polling_ms)
queue_delayed_work(devfreq_wq, &devfreq->work,
msecs_to_jiffies(devfreq->profile->polling_ms));
+
+ devfreq->last_stat_updated = jiffies;
devfreq->stop_polling = false;
+ if (devfreq->profile->get_cur_freq &&
+ !devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
+ devfreq->previous_freq = freq;
+
out:
mutex_unlock(&devfreq->lock);
}
@@ -931,11 +940,11 @@ static ssize_t show_trans_table(struct device *dev, struct device_attribute *att
{
struct devfreq *devfreq = to_devfreq(dev);
ssize_t len;
- int i, j, err;
+ int i, j;
unsigned int max_state = devfreq->profile->max_state;
- err = devfreq_update_status(devfreq, devfreq->previous_freq);
- if (err)
+ if (!devfreq->stop_polling &&
+ devfreq_update_status(devfreq, devfreq->previous_freq))
return 0;
len = sprintf(buf, " From : To\n");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/3] PM / devfreq: fix stats start time stamp
From: Rajagopal Venkat @ 2013-01-08 5:50 UTC (permalink / raw)
To: myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
mturquette-QSEj5FYQhm4dnm+yROfE0A, rjw-KKrjLPT3xs0
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
patches-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1357624239-13938-1-git-send-email-rajagopal.venkat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Mark the stats start time stamp when actual load monitoring is
started for accuracy.
Signed-off-by: Rajagopal Venkat <rajagopal.venkat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/devfreq/devfreq.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 5782c9b..2843a22 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -254,9 +254,12 @@ static void devfreq_monitor(struct work_struct *work)
void devfreq_monitor_start(struct devfreq *devfreq)
{
INIT_DEFERRABLE_WORK(&devfreq->work, devfreq_monitor);
- if (devfreq->profile->polling_ms)
+ if (devfreq->profile->polling_ms) {
queue_delayed_work(devfreq_wq, &devfreq->work,
msecs_to_jiffies(devfreq->profile->polling_ms));
+
+ devfreq->last_stat_updated = jiffies;
+ }
}
EXPORT_SYMBOL(devfreq_monitor_start);
@@ -498,7 +501,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned int) *
devfreq->profile->max_state,
GFP_KERNEL);
- devfreq->last_stat_updated = jiffies;
devfreq_set_freq_limits(devfreq);
dev_set_name(&devfreq->dev, dev_name(dev));
--
1.7.10.4
^ permalink raw reply related
* [GIT PULL] ACPI and power management fixes for v3.8-rc3
From: Rafael J. Wysocki @ 2013-01-07 23:53 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Len Brown, ACPI Devel Maling List, LKML, Linux PM list
Hi Linus,
Please pull from the git repository at
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm+acpi-for-3.8-rc3
to receive ACPI and power management fixes for v3.8-rc3 with top-most commit
f67ffa95836b31be5d8fe336aee3bfc6412c5696
Merge branch 'pm-sleep'
on top of commit d1c3ed669a2d452cacfb48c2d171a1f364dae2ed
Linux 3.8-rc2
Included are:
* Removal of some ACPICA code that the kernel will never use from Lv Zheng.
* APEI fix for erst_dbg_read() return value when persistent store is empty
initially from Adrian Huang.
* Removal of some unnecessary code from ACPI memory hotplug driver from
Liu Jinsong.
* ACPI power management fixes preventing useless messages from being printed
and correcting acpi_bus_get_device() result check.
* ACPI debug code fix to prevent some debug messages from being printed
with levels greater than KERN_DEBUG from Joe Perches.
* ACPI namespace scanning code fix to make system bus device nodes (like _SB)
get the right names (-stable material).
* PNP resources handling fixes from Witold Szczeponik.
* cpuidle fix for a recent regression stalling boot on systems with great
numbers (like hundreds) of CPUs from Daniel Lezcano.
* cpuidle fixes (one of them being -stable material) from Sivaram Nair.
* intel_idle debug message fix from Youquan Song.
* cpufreq build regression fix from Larry Finger.
* cpufreq fix for an obscure initialization race related to statistics from
Konstantin Khlebnikov.
* cpufreq change disabling the Longhaul driver by default (which apparently
is hopless to debug and fix) from Rafał Bilski.
* PM core fix preventing device suspend errors from happening during system
suspend due to obscure race conditions (-stable material).
* PM QoS local variable name cleanup.
Thanks!
Documentation/power/runtime_pm.txt | 9 +-
drivers/acpi/acpi_memhotplug.c | 18 --
drivers/acpi/acpica/Makefile | 2 +-
drivers/acpi/acpica/utclib.c | 749 ---------------------------------------------
drivers/acpi/apei/erst-dbg.c | 11 +-
drivers/acpi/device_pm.c | 3 +-
drivers/acpi/glue.c | 9 +-
drivers/acpi/power.c | 11 +-
drivers/acpi/scan.c | 2 +-
drivers/base/power/main.c | 9 +-
drivers/base/power/qos.c | 10 +-
drivers/cpufreq/Kconfig | 5 +
drivers/cpufreq/Makefile | 5 +-
drivers/cpufreq/cpufreq_stats.c | 11 +-
drivers/cpufreq/longhaul.c | 10 +-
drivers/cpuidle/coupled.c | 2 +-
drivers/cpuidle/cpuidle.c | 2 +-
drivers/cpuidle/driver.c | 8 +-
drivers/cpuidle/governors/menu.c | 2 +-
drivers/idle/intel_idle.c | 2 +-
drivers/pnp/interface.c | 105 ++++---
drivers/pnp/manager.c | 25 +-
22 files changed, 154 insertions(+), 856 deletions(-)
---------------
Adrian Huang (1):
ACPI / APEI: Fix the returned value in erst_dbg_read
Daniel Lezcano (1):
cpuidle: fix lock contention in the idle path
Joe Perches (1):
ACPI / glue: Update DBG macro to include KERN_DEBUG
Konstantin Khlebnikov (1):
cpufreq / stats: fix race between stats allocation and first usage
Larry Finger (1):
cpufreq / governor: Fix problem with cpufreq_ondemand or cpufreq_conservative
Liu Jinsong (1):
ACPI / memhotplug: remove redundant logic of acpi memory hotadd
Lv Zheng (1):
ACPICA: Remove useless mini-C library.
Rafael J. Wysocki (5):
ACPI / PM: Do not apply ACPI_SUCCESS() to acpi_bus_get_device() result
ACPI / power: Remove useless message from device registering routine
ACPI / scan: Do not use dummy HID for system bus ACPI nodes
PM / QoS: Rename local variable in dev_pm_qos_add_ancestor_request()
PM: Move disabling/enabling runtime PM to late suspend/early resume
Rafał Bilski (1):
cpufreq / Longhaul: Disable driver by default
Sivaram Nair (2):
cpuidle: Fix finding state with min power_usage
cpuidle / coupled: fix ready counter decrement
Witold Szczeponik (2):
PNP: Simplify setting of resources
PNP: Handle IORESOURCE_BITS in resource allocation
Youquan Song (1):
intel_idle: pr_debug information need separated
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] cpuidle: fix number of initialized/destroyed states
From: Rafael J. Wysocki @ 2013-01-07 23:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Krzysztof Mazur, linux-kernel, Linux PM list, Lists Linaro-dev
In-Reply-To: <50EB52A4.10307@linaro.org>
On Monday, January 07, 2013 11:56:36 PM Daniel Lezcano wrote:
> On 01/07/2013 10:58 PM, Rafael J. Wysocki wrote:
> > Hi,
> >
> > Thanks for the patch!
> >
> > I'd like Daniel to have a look at it still.
>
> I agree with this patch. I was about to send exactly the same.
>
> Thanks Krzysztof for fixing this.
>
>
> > On Monday, January 07, 2013 08:12:01 PM Krzysztof Mazur wrote:
> >> Commit bf4d1b5ddb78f86078ac6ae0415802d5f0c68f92 (cpuidle: support
> >> multiple drivers) changed the number of initialized state kobjects
> >> in cpuidle_add_state_sysfs() from device->state_count to drv->state_count,
> >> but leaved device->state_count in cpuidle_remove_state_sysfs().
> >> Those two values might have different values, causing for instance
> >> NULL pointer dereference in cpuidle_remove_state_sysfs().
> >>
> >> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
OK, thanks.
I have a pull request for 3.8-rc3 ready, so I'll send this one in the next
round.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] cpuidle: fix number of initialized/destroyed states
From: Daniel Lezcano @ 2013-01-07 22:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Krzysztof Mazur, linux-kernel, Linux PM list, Lists Linaro-dev
In-Reply-To: <339419841.p0RXgOP6cW@vostro.rjw.lan>
On 01/07/2013 10:58 PM, Rafael J. Wysocki wrote:
> Hi,
>
> Thanks for the patch!
>
> I'd like Daniel to have a look at it still.
I agree with this patch. I was about to send exactly the same.
Thanks Krzysztof for fixing this.
> On Monday, January 07, 2013 08:12:01 PM Krzysztof Mazur wrote:
>> Commit bf4d1b5ddb78f86078ac6ae0415802d5f0c68f92 (cpuidle: support
>> multiple drivers) changed the number of initialized state kobjects
>> in cpuidle_add_state_sysfs() from device->state_count to drv->state_count,
>> but leaved device->state_count in cpuidle_remove_state_sysfs().
>> Those two values might have different values, causing for instance
>> NULL pointer dereference in cpuidle_remove_state_sysfs().
>>
>> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<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
^ permalink raw reply
* Re: [PATCH] cpuidle: fix number of initialized/destroyed states
From: Rafael J. Wysocki @ 2013-01-07 21:58 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: daniel.lezcano, linux-kernel, Linux PM list
In-Reply-To: <1357585921-3391-1-git-send-email-krzysiek@podlesie.net>
Hi,
Thanks for the patch!
I'd like Daniel to have a look at it still.
Thanks,
Rafael
On Monday, January 07, 2013 08:12:01 PM Krzysztof Mazur wrote:
> Commit bf4d1b5ddb78f86078ac6ae0415802d5f0c68f92 (cpuidle: support
> multiple drivers) changed the number of initialized state kobjects
> in cpuidle_add_state_sysfs() from device->state_count to drv->state_count,
> but leaved device->state_count in cpuidle_remove_state_sysfs().
> Those two values might have different values, causing for instance
> NULL pointer dereference in cpuidle_remove_state_sysfs().
>
> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
> ---
> Hi,
>
> commit bf4d1b5ddb78f86078ac6ae0415802d5f0c68f92
> (cpuidle: support multiple drivers, merged in v3.8-rc1) causes NULL pointer
> dereference in cpuidle_remove_state_sysfs() when I plug the AC line to my
> laptop. I'm using the acpi_idle cpuidle driver and the C4 state is
> available only on when the system runs from battery. The problem still
> exists in v3.8-rc2 and f243b9b46a22e5790dbbc36f574c2417af49a41.
>
> I noticed that the commit bf4d1b5ddb78f86078ac6ae0415802d5f0c68f92
> (merged in v3.8-rc1) changed device->state_count to drv->state_count
> in only one of two places, which seems to be incorrect. This patch restores
> device->state_count in both places. It fixes the problem on my system.
>
> Krzysiek
>
> drivers/cpuidle/sysfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 3409429..428754a 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -374,7 +374,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
>
> /* state statistics */
> - for (i = 0; i < drv->state_count; i++) {
> + for (i = 0; i < device->state_count; i++) {
> kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
> if (!kobj)
> goto error_state;
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 3/9] Thermal: Add APIs to bind cdev to new zone structure
From: Greg KH @ 2013-01-07 19:26 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, linux-kernel, eduardo.valentin, hongbo.zhang,
wni
In-Reply-To: <1357542806-20449-4-git-send-email-durgadoss.r@intel.com>
On Mon, Jan 07, 2013 at 12:43:20PM +0530, Durgadoss R wrote:
> +struct thermal_cooling_device *get_cdev_by_name(const char *name)
> +{
> + struct thermal_cooling_device *pos;
> + struct thermal_cooling_device *cdev = NULL;
> +
> + mutex_lock(&cdev_list_lock);
> + for_each_cdev(pos) {
> + if (!strnicmp(pos->type, name, THERMAL_NAME_LENGTH)) {
> + cdev = pos;
> + break;
> + }
> + }
> + mutex_unlock(&cdev_list_lock);
> + return cdev;
> +}
> +EXPORT_SYMBOL(get_cdev_by_name);
EXPORT_SYMBOL_GPL?
You also forgot to increment the reference count, which is required for
all reference counted objects.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
From: Greg KH @ 2013-01-07 19:24 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, linux-kernel, eduardo.valentin, hongbo.zhang,
wni
In-Reply-To: <1357542806-20449-8-git-send-email-durgadoss.r@intel.com>
On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> This patch makes MAX_SENSORS_PER_ZONE and
> MAX_CDEVS_PER_ZONE values configurable. The
> default value is 1, and range is 1-12.
Why would we ever want to change this? Why make this configurable at
all, how is a distro supposed to set this value?
Shouldn't it be specified from the driver itself?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 9/9] Thermal: Dummy driver used for testing
From: Greg KH @ 2013-01-07 19:23 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, linux-kernel, eduardo.valentin, hongbo.zhang,
wni
In-Reply-To: <1357542806-20449-10-git-send-email-durgadoss.r@intel.com>
On Mon, Jan 07, 2013 at 12:43:26PM +0530, Durgadoss R wrote:
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
Unless you wish to track the office movements of the FSF for the next
40+ years, never include this in the code, it's useless.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 5/9] Thermal: Create 'mapX' sysfs node for a zone
From: Greg KH @ 2013-01-07 19:21 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, linux-kernel, eduardo.valentin, hongbo.zhang,
wni
In-Reply-To: <1357542806-20449-6-git-send-email-durgadoss.r@intel.com>
On Mon, Jan 07, 2013 at 12:43:22PM +0530, Durgadoss R wrote:
> This patch creates a thermal map sysfs node under
> /sys/class/thermal/zoneX/. This contains
> entries named map0, map1 .. mapN. Each map has the
> following space separated values:
> trip_type sensor_name cdev_name trip_mask weights
sysfs file are always "one value per file". This seems to violate that
rule, so please rework this.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v11 0/9] ZPODD Patches
From: Tejun Heo @ 2013-01-07 18:49 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <1357440509-28108-1-git-send-email-aaron.lu@intel.com>
Hello, Aaron.
On Sun, Jan 06, 2013 at 10:48:20AM +0800, Aaron Lu wrote:
> v11:
> Introduce event_driven flag in scsi_device to silence the media event
> poll after ODD is powered off;
> Removed ata layer PM QOS control, instead, simply limit ACPI state to
> D3_HOT when choosing state;
> Make the power off delay a module param named zpodd_poweroff_delay,
> defaults to 30 seconds.
Other than the nitpicks. The series generally looks good to me.
Thanks a lot!
--
tejun
^ permalink raw reply
* Re: [PATCH v11 8/9] libata: no poll when ODD is powered off
From: Tejun Heo @ 2013-01-07 18:45 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <1357440509-28108-9-git-send-email-aaron.lu@intel.com>
On Sun, Jan 06, 2013 at 10:48:28AM +0800, Aaron Lu wrote:
> When the ODD is powered off, any action the user did to the ODD that
> would generate a media event will trigger an ACPI interrupt, so the
> poll for media event is no longer necessary. And the poll will also
> cause a runtime status change, which will stop the ODD from staying in
> powered off state, so the poll should better be stopped.
>
> But since we don't have access to the gendisk structure in LLDs, here
> comes the event_driven flag for scsi device. This flag serves as a
> capability of the device, conveyed by the LLDs to upper layer. It is set
> when LLDs know that this device will no longer generate any media
> related events, so that the check_events can simply return 0 without
> bothering the device, effectively silence the poll.
>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> @@ -160,6 +160,7 @@ struct scsi_device {
> unsigned can_power_off:1; /* Device supports runtime power off */
> unsigned wce_default_on:1; /* Cache is ON by default */
> unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
> + unsigned event_driven:1; /* No need to poll the device */
Again, synchronization. Also, wouldn't something more explicit like
disable_disk_events better suited?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v11 7/9] libata: expose pm qos flags for ata device
From: Tejun Heo @ 2013-01-07 18:43 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <1357440509-28108-8-git-send-email-aaron.lu@intel.com>
On Sun, Jan 06, 2013 at 10:48:27AM +0800, Aaron Lu wrote:
> Expose pm qos flags to user space so that user has a chance to disable
> pm features like power off, if he/she has a broken platform or devices
> or simply does not like this pm feature.
>
> This flag is exposed to user space only for ata device or atapi device
> that is zero power capable. For normal atapi device, it will never be
> powered off.
>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
> drivers/ata/libata-acpi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index e832bf6..5b67be3 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -17,6 +17,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/pm_runtime.h>
> +#include <linux/pm_qos.h>
> #include <scsi/scsi_device.h>
> #include "libata.h"
>
> @@ -1022,6 +1023,8 @@ static void ata_acpi_unregister_power_resource(struct ata_device *dev)
> void ata_acpi_bind(struct ata_device *dev)
> {
> ata_acpi_register_power_resource(dev);
> + if (dev->class == ATA_DEV_ATA || zpodd_dev_enabled(dev))
> + dev_pm_qos_expose_flags(&dev->sdev->sdev_gendev, 0);
> }
Why from ata_acpi_bind()?
--
tejun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox