* Re: [PATCH] input: CPU frequency booster
From: Thomas Renninger @ 2012-01-19 22:52 UTC (permalink / raw)
To: Antti P Miettinen
Cc: davej, pavel, rjw, len.brown, khilman, j-pihet, markgross,
cpufreq, linux-pm
In-Reply-To: <1326977615-4387-1-git-send-email-amiettinen@nvidia.com>
Hi,
Not sure which email I should take to answer...
I can't see why pm_qos is needed at all, better use cpufreq
providing interface.
PPC (BIOS request to cut max_frequency) in
drivers/acpi/processor_perflib.c is a nice example
I'd also put this into drivers/cpufreq/cpufreq_input_boost.c.
Find below a re-written version of the booster.
Instead of pm_qos, it simply uses cpufreq interface:
/sys/devices/system/cpu/cpufreq/input_boost_freq
/sys/devices/system/cpu/cpufreq/input_boost_time
I started with global cpufreq variables, they could also
be declared per cpu like scaling_min_freq or others
(at least input_boost_freq, input_boost_time should probably
stay global). That would be:
/sys/devices/system/cpu/cpuX/cpufreq/input_boost_freq
A mechanism to set the right variables on the right HW
so that no userspace interaction is needed by default would be great.
Is there a PCI id, CPU family/model that could get checked
in the init func and automatically set the right values?
Now that this is part of cpufreq core, one could use these:
/* the following 3 funtions are for cpufreq core use only */
struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
void cpufreq_cpu_put(struct cpufreq_policy *data);
to find the 2nd lowest P-state or similar as default boost freq
A comment on which platforms this makes sense at all would be great.
Does any X86 machine need this?
Which ARM platforms you'd recommend to enable this in .config?
Could you give below a test, please.
Compile tested only.
Based on latest Linus git tree.
What do you think about it?
Thomas
---
drivers/cpufreq/Kconfig | 9 ++
drivers/cpufreq/Makefile | 3 +
drivers/cpufreq/cpufreq_input_boost.c | 238 +++++++++++++++++++++++++++++++++
3 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index e24a2a1..3f584d6 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -179,6 +179,15 @@ config CPU_FREQ_GOV_CONSERVATIVE
If in doubt, say N.
+config INPUT_CFBOOST
+ boolean "CPU frequency booster"
+ depends on INPUT
+ help
+ Say Y here if you want to boost frequency upon input events;
+
+ To compile this driver as a module, choose M here: the
+ module will be called input-cfboost.
+
menu "x86 CPU frequency scaling drivers"
depends on X86
source "drivers/cpufreq/Kconfig.x86"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index ac000fa..d083b9f 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -10,6 +10,9 @@ obj-$(CONFIG_CPU_FREQ_GOV_USERSPACE) += cpufreq_userspace.o
obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND) += cpufreq_ondemand.o
obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE) += cpufreq_conservative.o
+# CPUfreq misc
+obj-$(CONFIG_INPUT_CFBOOST) += cpufreq_input_boost.o
+
# CPUfreq cross-arch helpers
obj-$(CONFIG_CPU_FREQ_TABLE) += freq_table.o
diff --git a/drivers/cpufreq/cpufreq_input_boost.c b/drivers/cpufreq/cpufreq_input_boost.c
new file mode 100644
index 0000000..3ef8bca
--- /dev/null
+++ b/drivers/cpufreq/cpufreq_input_boost.c
@@ -0,0 +1,238 @@
+/*
+ * drivers/input/input-cfboost.c
+ *
+ * Copyright (C) 2012 NVIDIA Corporation
+ * Copyright (C) 2012 Thomas Renninger <trenn@suse.de>
+ * Better integration into cpufreq subsystem
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * 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.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/printk.h>
+#include <linux/workqueue.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/cpufreq.h>
+
+/* This module listens to input events and sets a temporary frequency
+ * floor upon input event detection. This is based on changes to
+ * cpufreq ondemand governor by:
+ *
+ * Tero Kristo <tero.kristo@nokia.com>
+ * Brian Steuer <bsteuer@codeaurora.org>
+ * David Ng <dave@codeaurora.org>
+ *
+ * at git://codeaurora.org/kernel/msm.git tree, commits:
+ *
+ * 2a6181bc76c6ce46ca0fa8e547be42acd534cf0e
+ * 1cca8861d8fda4e05f6b0c59c60003345c15454d
+ * 96a9aeb02bf5b3fbbef47e44460750eb275e9f1b
+ * b600449501cf15928440f87eff86b1f32d14214e
+ * 88a65c7ae04632ffee11f9fc628d7ab017c06b83
+ */
+
+MODULE_AUTHOR("Antti P Miettinen <amiettinen@nvidia.com>");
+MODULE_DESCRIPTION("Input event CPU frequency booster");
+MODULE_LICENSE("GPL v2");
+
+
+static struct work_struct boost;
+static struct delayed_work unboost;
+static struct workqueue_struct *cfb_wq;
+
+/* sysfs stuff **************************/
+static unsigned long boost_time;
+static unsigned int boost_freq = 500;
+
+static ssize_t show_input_boost_time (struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ return sprintf(buf, "%lu\n", boost_time);
+}
+static ssize_t show_input_boost_freq (struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ return sprintf(buf, "%u\n", boost_freq);
+}
+
+static ssize_t store_input_boost_time(struct kobject *a, struct attribute *b,
+ const char *buf, size_t count)
+{
+ /* TBD: Check input! */
+ int ret;
+ ret = sscanf(buf, "%lu", &boost_time);
+ if (ret != 1)
+ return -EINVAL;
+ return count;
+}
+
+static ssize_t store_input_boost_freq(struct kobject *a, struct attribute *b,
+ const char *buf, size_t count)
+{
+ /* TBD: Check input! */
+ int ret;
+ ret = sscanf(buf, "%u", &boost_freq);
+ if (ret != 1)
+ return -EINVAL;
+ return count;
+}
+define_one_global_rw(input_boost_freq);
+define_one_global_rw(input_boost_time);
+
+static const struct attribute *input_boost_attr[] = {
+ &input_boost_freq.attr,
+ &input_boost_time.attr,
+};
+
+/* sysfs stuff **************************/
+
+
+static void cfb_boost(struct work_struct *w)
+{
+ struct cpufreq_policy policy;
+ int ret;
+
+ cancel_delayed_work_sync(&unboost);
+ ret = cpufreq_get_policy(&policy, 0);
+ if (ret)
+ return;
+ cpufreq_verify_within_limits(&policy, boost_freq,
+ policy.cpuinfo.max_freq);
+ queue_delayed_work(cfb_wq, &unboost,
+ msecs_to_jiffies(boost_time));
+}
+
+static void cfb_unboost(struct work_struct *w)
+{
+ struct cpufreq_policy policy;
+ int ret;
+
+ ret = cpufreq_get_policy(&policy, 0);
+ if (ret)
+ return;
+
+ cpufreq_verify_within_limits(&policy, policy.cpuinfo.min_freq,
+ policy.cpuinfo.max_freq);
+}
+
+static void cfb_input_event(struct input_handle *handle, unsigned int type,
+ unsigned int code, int value)
+{
+ if (!work_pending(&boost))
+ queue_work(cfb_wq, &boost);
+}
+
+static int cfb_input_connect(struct input_handler *handler,
+ struct input_dev *dev,
+ const struct input_device_id *id)
+{
+ struct input_handle *handle;
+ int error;
+
+ handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+ if (!handle)
+ return -ENOMEM;
+
+ handle->dev = dev;
+ handle->handler = handler;
+ handle->name = "icfboost";
+
+ error = input_register_handle(handle);
+ if (error)
+ goto err2;
+
+ error = input_open_device(handle);
+ if (error)
+ goto err1;
+
+ return 0;
+err1:
+ input_unregister_handle(handle);
+err2:
+ kfree(handle);
+ return error;
+}
+
+static void cfb_input_disconnect(struct input_handle *handle)
+{
+ input_close_device(handle);
+ input_unregister_handle(handle);
+ kfree(handle);
+}
+
+static const struct input_device_id cfb_ids[] = {
+ { /* touch screen */
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_KEYBIT,
+ .evbit = { BIT_MASK(EV_ABS) },
+ .keybit = {[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH) },
+ },
+ { /* mouse */
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_KEYBIT,
+ .evbit = { BIT_MASK(EV_REL) },
+ .keybit = {[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_MOUSE) },
+ },
+ { },
+};
+
+static struct input_handler cfb_input_handler = {
+ .event = cfb_input_event,
+ .connect = cfb_input_connect,
+ .disconnect = cfb_input_disconnect,
+ .name = "icfboost",
+ .id_table = cfb_ids,
+};
+
+static int __init cfboost_init(void)
+{
+ /* TBD: Properly clean up in all cases */
+ int ret;
+
+ ret = sysfs_create_files(cpufreq_global_kobject,
+ input_boost_attr);
+
+ cfb_wq = create_workqueue("icfb-wq");
+ if (!cfb_wq)
+ return -ENOMEM;
+ INIT_WORK(&boost, cfb_boost);
+ INIT_DELAYED_WORK(&unboost, cfb_unboost);
+ ret = input_register_handler(&cfb_input_handler);
+ if (ret) {
+ destroy_workqueue(cfb_wq);
+ return ret;
+ }
+ return 0;
+}
+
+static void __exit cfboost_exit(void)
+{
+ sysfs_remove_files(cpufreq_global_kobject,
+ input_boost_attr);
+
+ /* stop input events */
+ input_unregister_handler(&cfb_input_handler);
+ /* cancel pending work requests */
+ cancel_work_sync(&boost);
+ cancel_delayed_work_sync(&unboost);
+ /* clean up */
+ destroy_workqueue(cfb_wq);
+}
+
+module_init(cfboost_init);
+module_exit(cfboost_exit);
^ permalink raw reply related
* Re: [PATCH] input: CPU frequency booster
From: Thomas Renninger @ 2012-01-19 23:10 UTC (permalink / raw)
To: Antti P Miettinen
Cc: davej, pavel, rjw, len.brown, khilman, j-pihet, markgross,
cpufreq, linux-pm
In-Reply-To: <201201192353.00761.trenn@suse.de>
On Thursday 19 January 2012 23:52:59 Thomas Renninger wrote:
> Hi,
...
> I'd also put this into drivers/cpufreq/cpufreq_input_boost.c.
>
> Find below a re-written version of the booster.
> Instead of pm_qos, it simply uses cpufreq interface:
> /sys/devices/system/cpu/cpufreq/input_boost_freq
> /sys/devices/system/cpu/cpufreq/input_boost_time
There was the same problem that scaling_max_freq showed a limit,
but one did not know whether it comes from:
- thermal
- PPC/BIOS
restrictions.
This got solved by introducing:
/sys/.../cpuX/cpufreq/bios_limit
which shows the PPC limitation
A file:
/sys/.../cpuX/cpufreq/input_boost_limit
could show the currently raised min_freq, caused by
the input booster.
Thomas
^ permalink raw reply
* Re: [linux-pm] [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Rafael J. Wysocki @ 2012-01-19 23:36 UTC (permalink / raw)
To: markgross; +Cc: linux-pm, Antti P Miettinen, cpufreq
In-Reply-To: <20120119164144.GA8757@mgross-G62>
On Thursday, January 19, 2012, mark gross wrote:
> On Thu, Jan 19, 2012 at 12:24:26AM +0100, Rafael J. Wysocki wrote:
> > On Wednesday, January 18, 2012, mark gross wrote:
> > > On Mon, Jan 16, 2012 at 10:38:57PM +0100, Rafael J. Wysocki wrote:
> > > > Hi,
> > > >
> > > > On Monday, January 16, 2012, Antti P Miettinen wrote:
> > > > > [did not reach linux-pm as I sent to wrong address, sorry for
> > > > > duplicates]
> > > > >
> > > > > The inspiration for this patch series is the N9 CPU frequency boost
> > > > > upon input events:
> > > > >
> > > > > http://www.spinics.net/lists/cpufreq/msg00667.html
> > > > >
> > > > > and the related changes in git://codeaurora.org/kernel/msm.git tree.
> > > > > Those patches modify the ondemand cpufreq governor. This patch series
> > > > > adds minimum and maximum CPU frequency as PM QoS parameters and
> > > > > modifies the cpufreq core to enforce the PM QoS limits.
> > > >
> > > > If that hasn't been clear enough so far, I'm still not convinced that using
> > > > PM QoS for that is a good idea.
> > > >
> > > > First off, frequency as a unit of throughput is questionable to say the least,
> > > > because it isn't portable from one system to another. Moreover, even on a
> > > > given system it isn't particularly clear what the exact correspondence
> > > > between frequency and throughput actually is.
> > >
> > > You are right. The notion of throughput of a CPU is really hard to
> > > quantify. Perhaps not using the term "throughput" would help?
> >
> > Yes, it would.
> >
> > > The base issue I see, the Intel platform, is needing is that sometimes
> > > we need to block the lowest P-states that the ondemand governor goes for
> > > because those P-states result in media / graphics workloads dropping
> > > frames. However; GPU intensive workloads do not stress the CPU so the
> > > ondemand governor goes for the low p-state.
> > >
> > > I could use some way of constraining the PM-throttling of the
> > > cpu-freq that can be hit from kernel or user mode. So the graphics
> > > driver can dynamically adjust the constraint request on the cpufreq
> > > subsystem.
> > >
> > > It is problematic that any driver requesting a given frequency request
> > > is not portable across ISA's or even processor families in the same ISA.
> > > But, maybe such a driver should use a module parameter to work around
> > > this lack of portability?
> >
> > Well, it seems to me that we're trying to add a backdoor to the (apparently
> > inadequate) governors here. Arguably, the governors should be able to
> > make the right decisions on the basis of the information they receive
> > through their own interfaces.
>
> the failings of governors to have the information needed is why pm_qos
> was created in the first place.
Well, that's interesting. :-)
> It can be seen as a limitation on the governor from some perspectives. But,
> I like to think of if as updating existing governors to account for new use
> case requirements as hardware get bigger power management / performance
> dynamic ranges.
The current patchset doesn't seem to update governors, though.
> > > > Second, it's not particularly clear what the meaning of the "min" frequency
> > > > is supposed to be in terms of throughput.
> > >
> > > It should mean "please cpufreq do not put the cpu into a state where its
> > > clock runs slower than min". I don't think we should talk about it as
> > > throughput because thats not what the cpufreq controls.
> >
> > Perhaps we need a new cpufreq governor that would take use PM QoS internally
> > to store requests from different sources, but that would work on a per-CPU
> > basis (not globally) and would provide a new interface for user space?
> >
>
> I don' think we need a new cpufreq governor, the parts of this patchset
> that I agree with evolve the governor to account for pm-qos requests
> but, globally for all cpu's.
As I said, it doesn't really evolve governors. It adds a mechanism for
influencing policy limits in a kind of convoluted fashion.
I mean, there are scaling_min_freq and scaling_max_freq in the per-CPU
cpufreq's sysfs interface that can be used to set min/max policy
limits. The only problem with those I see, which the current patchset
is kind of trying to address, is that they don't contain information
about who requested those limits.
> Hmm, your right this patch set is global in its request and not
> "per-cpu". I need to think on that. Making it per-cpu would likely
> infer we need to make the qos request per cpu as well.
>
> Do you think it needs to be per-cpu? (I'm starting to think "yes" it
> does)
>
> How do we scale the pm_qos ABI to support per/cpu? (maybe we don't
> export those types of qos classes to the user mode?)
I think we should focus on iproving the existing cpufreq inteface in the
first place.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] input: CPU frequency booster
From: Antti P Miettinen @ 2012-01-20 8:36 UTC (permalink / raw)
To: linux-pm; +Cc: cpufreq
In-Reply-To: <201201192353.00761.trenn@suse.de>
Thomas Renninger <trenn@suse.de> writes:
> I can't see why pm_qos is needed at all, better use cpufreq
> providing interface.
One reason for using PM QoS is that it provides an interface for user
space. We want to be able to control frequency also from user space.
--Antti
^ permalink raw reply
* Re: [PATCH 0/3] coupled cpuidle state support
From: Daniel Lezcano @ 2012-01-20 8:46 UTC (permalink / raw)
To: Colin Cross
Cc: linux-kernel, linux-arm-kernel, linux-pm, Len Brown, Kevin Hilman,
Santosh Shilimkar, Amit Kucheria, Arjan van de Ven, Trinabh Gupta,
Deepthi Dharwar, linux-omap, linux-tegra
In-Reply-To: <1324426147-16735-1-git-send-email-ccross@android.com>
On 12/21/2011 01:09 AM, Colin Cross wrote:
> On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
> cpus cannot be independently powered down, either due to
> sequencing restrictions (on Tegra 2, cpu 0 must be the last to
> power down), or due to HW bugs (on OMAP4460, a cpu powering up
> will corrupt the gic state unless the other cpu runs a work
> around). Each cpu has a power state that it can enter without
> coordinating with the other cpu (usually Wait For Interrupt, or
> WFI), and one or more "coupled" power states that affect blocks
> shared between the cpus (L2 cache, interrupt controller, and
> sometimes the whole SoC). Entering a coupled power state must
> be tightly controlled on both cpus.
>
> The easiest solution to implementing coupled cpu power states is
> to hotplug all but one cpu whenever possible, usually using a
> cpufreq governor that looks at cpu load to determine when to
> enable the secondary cpus. This causes problems, as hotplug is an
> expensive operation, so the number of hotplug transitions must be
> minimized, leading to very slow response to loads, often on the
> order of seconds.
>
> This patch series implements an alternative solution, where each
> cpu will wait in the WFI state until all cpus are ready to enter
> a coupled state, at which point the coupled state function will
> be called on all cpus at approximately the same time.
>
> Once all cpus are ready to enter idle, they are woken by an smp
> cross call. At this point, there is a chance that one of the
> cpus will find work to do, and choose not to enter suspend. A
> final pass is needed to guarantee that all cpus will call the
> power state enter function at the same time. During this pass,
> each cpu will increment the ready counter, and continue once the
> ready counter matches the number of online coupled cpus. If any
> cpu exits idle, the other cpus will decrement their counter and
> retry.
>
> To use coupled cpuidle states, a cpuidle driver must:
>
> Set struct cpuidle_device.coupled_cpus to the mask of all
> coupled cpus, usually the same as cpu_possible_mask if all cpus
> are part of the same cluster. The coupled_cpus mask must be
> set in the struct cpuidle_device for each cpu.
>
> Set struct cpuidle_device.safe_state to a state that is not a
> coupled state. This is usually WFI.
>
> Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
> state that affects multiple cpus.
>
> Provide a struct cpuidle_state.enter function for each state
> that affects multiple cpus. This function is guaranteed to be
> called on all cpus at approximately the same time. The driver
> should ensure that the cpus all abort together if any cpu tries
> to abort once the function is called.
>
> This series was functionally tested on v3.0, but has only been
> compile-tested on v3.2 after the removal of per-cpu state fields.
Hi Colin,
this patchset could be interesting to resolve in a generic way the cpu
dependencies.
What is the status of this patchset ?
Did you have the opportunity to measure the power consumption with and
without this patchset ?
Thanks
-- Daniel
--
<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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] input: CPU frequency booster
From: Thomas Renninger @ 2012-01-20 10:50 UTC (permalink / raw)
To: Antti P Miettinen
Cc: khilman, len.brown, markgross, cpufreq, linux-pm, j-pihet
In-Reply-To: <201201192353.00761.trenn@suse.de>
[-- Attachment #1.1: Type: text/plain, Size: 938 bytes --]
On Thursday, January 19, 2012 11:52:59 PM Thomas Renninger wrote:
> Hi,
>
...
I forgot to activate the newly set limits:
> +/* sysfs stuff **************************/
> +
> +
> +static void cfb_boost(struct work_struct *w)
> +{
> + struct cpufreq_policy policy;
> + int ret;
> +
> + cancel_delayed_work_sync(&unboost);
> + ret = cpufreq_get_policy(&policy, 0);
> + if (ret)
> + return;
> + cpufreq_verify_within_limits(&policy, boost_freq,
> + policy.cpuinfo.max_freq);
cpufreq_update_policy(0);
> + queue_delayed_work(cfb_wq, &unboost,
> + msecs_to_jiffies(boost_time));
> +}
> +
> +static void cfb_unboost(struct work_struct *w)
> +{
> + struct cpufreq_policy policy;
> + int ret;
> +
> + ret = cpufreq_get_policy(&policy, 0);
> + if (ret)
> + return;
> +
> + cpufreq_verify_within_limits(&policy, policy.cpuinfo.min_freq,
> + policy.cpuinfo.max_freq);
cpufreq_update_policy(0);
Hope it works now?
Thomas
[-- Attachment #1.2: Type: text/html, Size: 7251 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] coupled cpuidle state support
From: Colin Cross @ 2012-01-20 20:40 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
linux-pm, linux-omap, Arjan van de Ven, linux-arm-kernel
In-Reply-To: <4F1929E9.7070707@linaro.org>
On Fri, Jan 20, 2012 at 12:46 AM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> Hi Colin,
>
> this patchset could be interesting to resolve in a generic way the cpu
> dependencies.
> What is the status of this patchset ?
I can't do much with it right now, because I don't have any devices
that can do SMP idle with a v3.2 kernel. I've started working on an
updated version that avoids the spinlock, but it might be a while
before I can test and post it. I'm mostly looking for feedback on the
approach taken in this patch, and whether it will be useful for other
SoCs besides Tegra and OMAP4.
> Did you have the opportunity to measure the power consumption with and
> without this patchset ?
Power consumption will be very dependent on the specific SoC in
question. The most important factors are the power savings of the
independent cpuidle state (normally WFI) vs. the hotplug state
(normally 1 cpu in OFF), and the workload being tested.
On a very idle system, these patches result in the same total power as
hotplugging one cpu and letting the other idle normally. On a 25%
busy system, you might see a slight increase in power, as the best
independent cpuidle state might be WFI, vs 1 cpu in OFF mode in
hotplug. On OMAP4, that difference is small, on the order of 10 mW.
Once you hit the threshold where a hotplug governor would have
hotplugged in the second cpu (lets say 40%), the savings from these
patches are enormous, as you can hit the lowest power state up to 60%
of the time, where the hotplug solution would never be going below WFI
on both cpus. On OMAP4, that can be well over 100 mW.
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Antti P Miettinen @ 2012-01-22 9:55 UTC (permalink / raw)
To: linux-pm; +Cc: cpufreq
In-Reply-To: <201201192340.24503.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
> Well, while it might be easier, the _whole_ _point_ of governors is to make
> decisions on the basis of available data. By introducing an independent
> mechanism for that we're bypassing governors in a sense.
I see this as modular design: governor defines the overall CPU frequency
control policy and PM QoS defines constraints. I think those things can
and probably should be somewhat independent.
--Antti
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Antti P Miettinen @ 2012-01-22 10:35 UTC (permalink / raw)
To: linux-pm; +Cc: cpufreq
In-Reply-To: <201201192315.22590.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
> On Thursday, January 19, 2012, Antti P Miettinen wrote:
>> I think we need both, global and per cpu.
>
> Well, if we have that per CPU, it will be easy to create a global
> setting: set all of the per-CPU settings to the same values. :-)
That is true. Currently the device specific constraints do not have user
space interface. The pm_qos_interface.txt refers to ongoing
discussion. Is there a thread I should read?
Seems that the device specific constraints are not yet in use in
3.3-rc1, or am I not looking hard enough?
--Antti
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Fix linux-pm address
From: Antti P Miettinen @ 2012-01-22 10:50 UTC (permalink / raw)
To: linux-pm
In-Reply-To: <201201192256.41151.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
> On Thursday, January 19, 2012, Alan Stern wrote:
>> On Thu, 19 Jan 2012, Srivatsa S. Bhat wrote:
>>
>> > On 01/19/2012 06:48 PM, Antti P Miettinen wrote:
>> >
>> > > Current linux-pm mailing list address seems to be
>> > > linux-pm@lists.linux-foundation.org.
>> > >
>> >
>> >
>> > Actually now both the mailing list addresses work.
>>
>> The lists.linux-foundation.org address was linux-pm's original home.
>> The vger.kernel.org addresss was created when the Linux Foundation's
>> listserv went down last summer; it is now regarded as linux-pm's
>> primary address even though both addresses are working.
>>
>> See commit bf1c138e350155edb06709c7fbf0946f252b257c (MAINTAINERS:
>> Update linux-pm list address).
>
> That's correct.
>
> Thanks,
> Rafael
OK - I was probably seeing just a transient problem. Sorry for the
noise.
--Antti
^ permalink raw reply
* Re: [PATCH] input: CPU frequency booster
From: Antti P Miettinen @ 2012-01-22 10:55 UTC (permalink / raw)
To: linux-pm; +Cc: cpufreq
In-Reply-To: <20120119174459.GB25397@atrey.karlin.mff.cuni.cz>
Pavel Machek <pavel@ucw.cz> writes:
> Does it need to be configurable? Going to max frequency for user
> interaction sounds like good idea... Probably should include keyboard,
> too.
Boosting upon all input events might be wasteful. Configurability would
allow tuning based on platform characteristics.
--Antti
^ permalink raw reply
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Sylwester Nawrocki @ 2012-01-22 12:59 UTC (permalink / raw)
To: Mark Brown
Cc: Wolfram Sang, Ben Dooks, linux-samsung-soc, linux-i2c, linux-pm
In-Reply-To: <1327171600-5489-1-git-send-email-broonie@opensource.wolfsonmicro.com>
On 01/21/2012 07:46 PM, Mark Brown wrote:
> Add stub runtime_pm calls which go through the flow of enabling and
> disabling but don't actually do anything with the device itself as
> there's nothing useful we can do. This provides the core PM framework
> with information about when the device is idle, enabling chip wide
> power savings.
>
> Signed-off-by: Mark Brown<broonie@opensource.wolfsonmicro.com>
> Acked-by: Heiko Stuebner<heiko@sntech.de>
> ---
> drivers/i2c/busses/i2c-s3c2410.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index e6f982b..3d80bab 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -31,6 +31,7 @@
> #include<linux/errno.h>
> #include<linux/err.h>
> #include<linux/platform_device.h>
> +#include<linux/pm_runtime.h>
> #include<linux/clk.h>
> #include<linux/cpufreq.h>
> #include<linux/slab.h>
> @@ -564,6 +565,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
> int retry;
> int ret;
>
> + pm_runtime_get_sync(&adap->dev);
IMHO it's more appropriate to use i2c->dev here instead, i.e. to reference
the platform device we've enabled runtime PM for.
> clk_enable(i2c->clk);
>
> for (retry = 0; retry< adap->retries; retry++) {
> @@ -572,6 +574,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
>
> if (ret != -EAGAIN) {
> clk_disable(i2c->clk);
> + pm_runtime_put(&adap->dev);
Ditto.
> return ret;
> }
>
> @@ -581,6 +584,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
> }
>
> clk_disable(i2c->clk);
> + pm_runtime_put(&adap->dev);
Ditto.
> return -EREMOTEIO;
> }
>
> @@ -1013,6 +1017,8 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
> of_i2c_register_devices(&i2c->adap);
> platform_set_drvdata(pdev, i2c);
>
> + pm_runtime_enable(&pdev->dev);
> +
> dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
> clk_disable(i2c->clk);
> return 0;
> @@ -1047,6 +1053,8 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
> {
> struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
>
> + pm_runtime_disable(&pdev->dev);
> +
> s3c24xx_i2c_deregister_cpufreq(i2c);
>
> i2c_del_adapter(&i2c->adap);
How about the following patch (untested) ? It might be a better start for
proper power management implementation and would still allow the driver
to work on platforms that don't support runtime PM.
8<----------------------------------------------
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 4c17180..0f588bb 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -31,6 +31,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/clk.h>
#include <linux/cpufreq.h>
#include <linux/slab.h>
@@ -551,6 +552,26 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
return ret;
}
+static inline int s3c24xx_pm_runtime_get(struct s3c24xx_i2c *i2c)
+{
+ if (!pm_runtime_enabled(i2c->dev)) {
+ clk_enable(i2c->clk);
+ return 0;
+ }
+
+ return pm_runtime_get_sync(i2c->dev);
+}
+
+static inline int s3c24xx_pm_runtime_put(struct s3c24xx_i2c *i2c)
+{
+ if (!pm_runtime_enabled(i2c->dev)) {
+ clk_disable(i2c->clk);
+ return 0;
+ }
+
+ return pm_runtime_put(i2c->dev);
+}
+
/* s3c24xx_i2c_xfer
*
* first port of call from the i2c bus code when an message needs
@@ -564,14 +585,14 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
int retry;
int ret;
- clk_enable(i2c->clk);
+ s3c24xx_pm_runtime_get(i2c);
for (retry = 0; retry < adap->retries; retry++) {
ret = s3c24xx_i2c_doxfer(i2c, msgs, num);
if (ret != -EAGAIN) {
- clk_disable(i2c->clk);
+ s3c24xx_pm_runtime_put(i2c);
return ret;
}
@@ -580,7 +601,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
udelay(100);
}
- clk_disable(i2c->clk);
+ s3c24xx_pm_runtime_put(i2c);
return -EREMOTEIO;
}
@@ -929,7 +950,9 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
dev_dbg(&pdev->dev, "clock source %p\n", i2c->clk);
- clk_enable(i2c->clk);
+ platform_set_drvdata(pdev, i2c);
+ pm_runtime_enable(i2c->dev);
+ s3c24xx_pm_runtime_get(i2c);
/* map the registers */
@@ -1011,10 +1034,9 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
}
of_i2c_register_devices(&i2c->adap);
- platform_set_drvdata(pdev, i2c);
dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
- clk_disable(i2c->clk);
+ s3c24xx_pm_runtime_put(i2c);
return 0;
err_cpufreq:
@@ -1048,6 +1070,8 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
{
struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
+ pm_runtime_disable(&pdev->dev);
+
s3c24xx_i2c_deregister_cpufreq(i2c);
i2c_del_adapter(&i2c->adap);
@@ -1066,7 +1090,28 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_RUNTIME
+static int s3c24xx_i2c_runtime_suspend(struct device *dev)
+{
+ struct s3c24xx_i2c *i2c = dev_get_drvdata(dev);
+
+ clk_disable(i2c->clk);
+ return 0;
+}
+
+static int s3c24xx_i2c_runtime_resume(struct device *dev)
+{
+ struct s3c24xx_i2c *i2c = dev_get_drvdata(dev);
+
+ clk_enable(i2c->clk);
+ return 0;
+}
+#else
+#define s3c24xx_i2c_runtime_suspend NULL
+#define s3c24xx_i2c_runtime_resume NULL
+#endif
+
+#ifdef CONFIG_PM_SLEEP
static int s3c24xx_i2c_suspend_noirq(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -1089,17 +1134,18 @@ static int s3c24xx_i2c_resume(struct device *dev)
return 0;
}
+#else
+#define s3c24xx_i2c_suspend_noirq NULL
+#define s3c24xx_i2c_resume NULL
+#endif
static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
.suspend_noirq = s3c24xx_i2c_suspend_noirq,
.resume = s3c24xx_i2c_resume,
+ .runtime_suspend = s3c24xx_i2c_runtime_suspend,
+ .runtime_resume = s3c24xx_i2c_runtime_resume,
};
-#define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops)
-#else
-#define S3C24XX_DEV_PM_OPS NULL
-#endif
-
/* device driver for platform bus bits */
static struct platform_device_id s3c24xx_driver_ids[] = {
@@ -1131,7 +1177,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "s3c-i2c",
- .pm = S3C24XX_DEV_PM_OPS,
+ .pm = &s3c24xx_i2c_dev_pm_ops,
.of_match_table = s3c24xx_i2c_match,
},
};
8>----------------------------------------------
--
Thanks,
Sylwester
^ permalink raw reply related
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Mark Brown @ 2012-01-22 15:22 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Wolfram Sang, Ben Dooks, linux-samsung-soc, linux-i2c, linux-pm
In-Reply-To: <4F1C082D.2080005@gmail.com>
On Sun, Jan 22, 2012 at 01:59:25PM +0100, Sylwester Nawrocki wrote:
> On 01/21/2012 07:46 PM, Mark Brown wrote:
> > + pm_runtime_get_sync(&adap->dev);
> IMHO it's more appropriate to use i2c->dev here instead, i.e. to reference
> the platform device we've enabled runtime PM for.
Oh, bah humbug. Now that I notice this I think that's the reason I
enabled runtime PM for the adaptor - it's easier to get hold of.
> How about the following patch (untested) ? It might be a better start for
> proper power management implementation and would still allow the driver
> to work on platforms that don't support runtime PM.
It's not really a platform issue - the platform bus by default does the
right thing and the runtime PM core is pure software, the only platform
dependency is if users have bothered truning runtime PM on.
> +static inline int s3c24xx_pm_runtime_put(struct s3c24xx_i2c *i2c)
> +{
> + if (!pm_runtime_enabled(i2c->dev)) {
> + clk_disable(i2c->clk);
> + return 0;
> + }
> +
> + return pm_runtime_put(i2c->dev);
> +}
To be honest I don't think this is worth it. Either we just keep the
clock management outside of runtime PM or we push it in but trying to
support both simultaneously adds complication and doesn't actually do
all that much in practical terms.
^ permalink raw reply
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Sylwester Nawrocki @ 2012-01-22 17:22 UTC (permalink / raw)
To: Mark Brown
Cc: Wolfram Sang, Ben Dooks, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
In-Reply-To: <20120122152234.GA2915-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
On 01/22/2012 04:22 PM, Mark Brown wrote:
> On Sun, Jan 22, 2012 at 01:59:25PM +0100, Sylwester Nawrocki wrote:
>> How about the following patch (untested) ? It might be a better start for
>> proper power management implementation and would still allow the driver
>> to work on platforms that don't support runtime PM.
>
> It's not really a platform issue - the platform bus by default does the
> right thing and the runtime PM core is pure software, the only platform
> dependency is if users have bothered truning runtime PM on.
It is a platform (SoC) issue in terms of enabling PM_RUNTIME, which is
pure software, as you're pointing out. I think we agree here that the
only issue is that some (ill) platforms refuse to enable PM_RUNTIME.
And clock gating belongs to runtime PM, which this I2C controller driver
chooses to implement without runtime PM awareness.
>> +static inline int s3c24xx_pm_runtime_put(struct s3c24xx_i2c *i2c)
>> +{
>> + if (!pm_runtime_enabled(i2c->dev)) {
>> + clk_disable(i2c->clk);
>> + return 0;
>> + }
>> +
>> + return pm_runtime_put(i2c->dev);
>> +}
>
> To be honest I don't think this is worth it. Either we just keep the
> clock management outside of runtime PM or we push it in but trying to
> support both simultaneously adds complication and doesn't actually do
> all that much in practical terms.
It's not pretty, I agree. However it is supposed to be more a temporary
solution, rather than anything final. And it probably is better at
avoiding any races and mismatch between real device state and the PM core
knowledge of it. I'm not going to persuade my version too much, it's far
from prefect, similarly as the original patch in this thread. The best
solution would be to force platform to enable PM_RUNTIME if they want to
use a driver which does runtime PM. But this has to wait, yet, probably
infinitely.. :)
^ permalink raw reply
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Bill Gatliff @ 2012-01-22 17:27 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Mark Brown, Wolfram Sang, Ben Dooks, linux-samsung-soc, linux-i2c,
linux-pm
In-Reply-To: <4F1C45C6.1040003@gmail.com>
Guys:
On Sun, Jan 22, 2012 at 6:22 PM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
> The best solution would be to force platform to enable PM_RUNTIME if they want to
> use a driver which does runtime PM. But this has to wait, yet, probably
> infinitely.. :)
Probably easier to motivate platforms to enable PM_RUNTIME if there
are drivers that require it. :)
I for one would rather see in-kernel drivers that require it, and then
patches in mailing lists that show how to un-require it. Make it more
painful to avoid PM_RUNTIME, and less painful to use it.
b.g.
--
Bill Gatliff
bgat@billgatliff.com
^ permalink raw reply
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Sylwester Nawrocki @ 2012-01-22 17:48 UTC (permalink / raw)
To: Bill Gatliff
Cc: Mark Brown, Wolfram Sang, Ben Dooks, linux-samsung-soc, linux-i2c,
linux-pm
In-Reply-To: <CADkCAusScak6gH4BBowrOpveGG=eDjZdj+X8vF4D5+yi8gv5PA@mail.gmail.com>
Hi,
On 01/22/2012 06:27 PM, Bill Gatliff wrote:
> On Sun, Jan 22, 2012 at 6:22 PM, Sylwester Nawrocki
> <sylvester.nawrocki@gmail.com> wrote:
>> The best solution would be to force platform to enable PM_RUNTIME if
>> they want to use a driver which does runtime PM. But this has to wait,
>> yet, probably infinitely.. :)
>
> Probably easier to motivate platforms to enable PM_RUNTIME if there
> are drivers that require it. :)
>
> I for one would rather see in-kernel drivers that require it, and then
In fact we have to deal with the opposite now, as some existing drivers
have been used for multiple generations of SoC, where almost unchanged
device IPs are deployed. Those drivers were originally written for the
simplest SoCs.
> patches in mailing lists that show how to un-require it. Make it more
> painful to avoid PM_RUNTIME, and less painful to use it.
Yeah, makes a lot of sense. With new code there is no issue, only the code
with long history is sort of troublesome.
--
Regards,
Sylwester
^ permalink raw reply
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Mark Brown @ 2012-01-22 21:39 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Bill Gatliff, Wolfram Sang, Ben Dooks, linux-samsung-soc,
linux-i2c, linux-pm
In-Reply-To: <4F1C4BF4.5040409@gmail.com>
On Sun, Jan 22, 2012 at 06:48:36PM +0100, Sylwester Nawrocki wrote:
> On 01/22/2012 06:27 PM, Bill Gatliff wrote:
> > I for one would rather see in-kernel drivers that require it, and then
> In fact we have to deal with the opposite now, as some existing drivers
> have been used for multiple generations of SoC, where almost unchanged
> device IPs are deployed. Those drivers were originally written for the
> simplest SoCs.
TBH I think most of the devices for which people are running these days
will be able to get some win from the system wide stuff - the WFI modes
aren't exactly the latest thing in hardware terms, it's just been a long
road to getting them supported. Infrastructure like Mark's PM QoS work
and Raphael's PM domains work has really helped a lot here.
> > patches in mailing lists that show how to un-require it. Make it more
> > painful to avoid PM_RUNTIME, and less painful to use it.
> Yeah, makes a lot of sense. With new code there is no issue, only the code
> with long history is sort of troublesome.
It's mostly a transition management issue I think. When I repost I'll
add an additional patch on top which moves the clock gating into the
runtime PM callbacks, that way the decision on that doesn't block the
system wide work.
^ permalink raw reply
* Re: [linux-pm] [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Rafael J. Wysocki @ 2012-01-22 23:43 UTC (permalink / raw)
To: Antti P Miettinen; +Cc: linux-pm, cpufreq
In-Reply-To: <87d3acf2jh.fsf@amiettinen-lnx.nvidia.com>
On Sunday, January 22, 2012, Antti P Miettinen wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> > On Thursday, January 19, 2012, Antti P Miettinen wrote:
> >> I think we need both, global and per cpu.
> >
> > Well, if we have that per CPU, it will be easy to create a global
> > setting: set all of the per-CPU settings to the same values. :-)
>
> That is true. Currently the device specific constraints do not have user
> space interface. The pm_qos_interface.txt refers to ongoing
> discussion. Is there a thread I should read?
>
> Seems that the device specific constraints are not yet in use in
> 3.3-rc1, or am I not looking hard enough?
They are in use through generic PM domains (drivers/base/power/domain*.c
and friends) and ARM/shmobile uses those.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] i2c-s3c2410: Add stub runtime power management
From: Sylwester Nawrocki @ 2012-01-23 20:19 UTC (permalink / raw)
To: Mark Brown
Cc: Bill Gatliff, Wolfram Sang, Ben Dooks, linux-samsung-soc,
linux-i2c, linux-pm
In-Reply-To: <20120122213952.GA29022@opensource.wolfsonmicro.com>
On 01/22/2012 10:39 PM, Mark Brown wrote:
> On Sun, Jan 22, 2012 at 06:48:36PM +0100, Sylwester Nawrocki wrote:
>> On 01/22/2012 06:27 PM, Bill Gatliff wrote:
>
>>> I for one would rather see in-kernel drivers that require it, and then
>
>> In fact we have to deal with the opposite now, as some existing drivers
>> have been used for multiple generations of SoC, where almost unchanged
>> device IPs are deployed. Those drivers were originally written for the
>> simplest SoCs.
>
> TBH I think most of the devices for which people are running these days
> will be able to get some win from the system wide stuff - the WFI modes
> aren't exactly the latest thing in hardware terms, it's just been a long
> road to getting them supported. Infrastructure like Mark's PM QoS work
> and Raphael's PM domains work has really helped a lot here.
Indeed, now that I checked some earliest SoC TRMs even the simplest systems
have some sort of intermediate Idle states.
>>> patches in mailing lists that show how to un-require it. Make it more
>>> painful to avoid PM_RUNTIME, and less painful to use it.
>
>> Yeah, makes a lot of sense. With new code there is no issue, only the code
>> with long history is sort of troublesome.
>
> It's mostly a transition management issue I think. When I repost I'll
> add an additional patch on top which moves the clock gating into the
> runtime PM callbacks, that way the decision on that doesn't block the
> system wide work.
Sounds like a great resolution to the problem, thanks for considering it.
And sorry for disturbing. Let's see what's Kukjin's opinion on that.
^ permalink raw reply
* ACPI & Power Management Patches for Linux 3.3-rc1
From: Len Brown @ 2012-01-24 1:29 UTC (permalink / raw)
To: linux-acpi, linux-pm
This fixes a merge-window regression due to a conflict
between error injection and preparation to remove atomicio.c
Here we fix that regression and complete the removal
of atomicio.c.
This also re-orders some idle initializtion code to
complete the merge window series that allows cpuidle
to cope with bringing processors on-line after boot.
Please let me know if you see issues with any of these patches.
thanks,
Len Brown, Intel Open Source Technolgy Center
^ permalink raw reply
* [PATCH 01/10] ACPI processor hotplug: Split up acpi_processor_add
From: Len Brown @ 2012-01-24 1:30 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Thomas Renninger, Len Brown
In-Reply-To: <1327368609-28409-1-git-send-email-lenb@kernel.org>
From: Thomas Renninger <trenn@suse.de>
No functional change.
This is needed because:
When a CPU gets hotplugged, it's totally uninitialized
and offline. cpuinfo_x86 struct (cpu_data(cpu)) is mostly
zero (CPU feature flags, model, family,..).
When a CPU gets hotplugged, struct processor is alloc'd,
some sysfs files are set up but acpi_processor_add()
must not try to access a MSR on this CPU or try to read
out CPU feature,family, etc.
This must be done in acpi_processor_start().
The next patch will delay the call of acpi_processor_start()
for physically hotpluggedcores, to the time when they are onlined
the first time. There it is safe then to access cpu_data(cpu)
cpuinfo_x86 struct or access MSRs which is needed to
set up cpuidle, throttling and other features.
Tested and
Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/processor_driver.c | 92 +++++++++++++++++++++++----------------
1 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 0034ede..bec5593 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -440,6 +440,58 @@ static struct notifier_block acpi_cpu_notifier =
.notifier_call = acpi_cpu_soft_notify,
};
+static int __cpuinit acpi_processor_start(struct acpi_processor *pr)
+{
+ struct acpi_device *device = per_cpu(processor_device_array, pr->id);
+ int result = 0;
+
+#ifdef CONFIG_CPU_FREQ
+ acpi_processor_ppc_has_changed(pr, 0);
+#endif
+ acpi_processor_get_throttling_info(pr);
+ acpi_processor_get_limit_info(pr);
+
+ if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
+ acpi_processor_power_init(pr, device);
+
+ pr->cdev = thermal_cooling_device_register("Processor", device,
+ &processor_cooling_ops);
+ if (IS_ERR(pr->cdev)) {
+ result = PTR_ERR(pr->cdev);
+ goto err_power_exit;
+ }
+
+ dev_dbg(&device->dev, "registered as cooling_device%d\n",
+ pr->cdev->id);
+
+ result = sysfs_create_link(&device->dev.kobj,
+ &pr->cdev->device.kobj,
+ "thermal_cooling");
+ if (result) {
+ printk(KERN_ERR PREFIX "Create sysfs link\n");
+ goto err_thermal_unregister;
+ }
+ result = sysfs_create_link(&pr->cdev->device.kobj,
+ &device->dev.kobj,
+ "device");
+ if (result) {
+ printk(KERN_ERR PREFIX "Create sysfs link\n");
+ goto err_remove_sysfs_thermal;
+ }
+
+ return 0;
+
+err_remove_sysfs_thermal:
+ sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
+err_thermal_unregister:
+ thermal_cooling_device_unregister(pr->cdev);
+err_power_exit:
+ acpi_processor_power_exit(pr, device);
+
+ return result;
+}
+
+
static int __cpuinit acpi_processor_add(struct acpi_device *device)
{
struct acpi_processor *pr = NULL;
@@ -494,49 +546,13 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
result = -EFAULT;
goto err_free_cpumask;
}
-
-#ifdef CONFIG_CPU_FREQ
- acpi_processor_ppc_has_changed(pr, 0);
-#endif
- acpi_processor_get_throttling_info(pr);
- acpi_processor_get_limit_info(pr);
-
- if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
- acpi_processor_power_init(pr, device);
-
- pr->cdev = thermal_cooling_device_register("Processor", device,
- &processor_cooling_ops);
- if (IS_ERR(pr->cdev)) {
- result = PTR_ERR(pr->cdev);
- goto err_power_exit;
- }
-
- dev_dbg(&device->dev, "registered as cooling_device%d\n",
- pr->cdev->id);
-
- result = sysfs_create_link(&device->dev.kobj,
- &pr->cdev->device.kobj,
- "thermal_cooling");
- if (result) {
- printk(KERN_ERR PREFIX "Create sysfs link\n");
- goto err_thermal_unregister;
- }
- result = sysfs_create_link(&pr->cdev->device.kobj,
- &device->dev.kobj,
- "device");
- if (result) {
- printk(KERN_ERR PREFIX "Create sysfs link\n");
+ result = acpi_processor_start(pr);
+ if (result)
goto err_remove_sysfs;
- }
return 0;
err_remove_sysfs:
- sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
-err_thermal_unregister:
- thermal_cooling_device_unregister(pr->cdev);
-err_power_exit:
- acpi_processor_power_exit(pr, device);
sysfs_remove_link(&device->dev.kobj, "sysdev");
err_free_cpumask:
free_cpumask_var(pr->throttling.shared_cpu_map);
--
1.7.9.rc2
^ permalink raw reply related
* [PATCH 02/10] ACPI processor hotplug: Delay acpi_processor_start() call for hotplugged cores
From: Len Brown @ 2012-01-24 1:30 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Thomas Renninger, Len Brown
In-Reply-To: <54d5dcc45af7adbb907072d042bbece4c2b4de6e.1327368464.git.len.brown@intel.com>
From: Thomas Renninger <trenn@suse.de>
Delay the setting up of features (cpuidle, throttling by calling
acpi_processor_start()) to the time when the hotplugged
core got onlined the first time and got fully
initialized.
Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/processor_driver.c | 72 +++++++++++++++++++++++++++++++++++----
drivers/idle/intel_idle.c | 2 +-
include/acpi/processor.h | 1 +
3 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index bec5593..2b805d7 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -84,7 +84,7 @@ static int acpi_processor_remove(struct acpi_device *device, int type);
static void acpi_processor_notify(struct acpi_device *device, u32 event);
static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr);
static int acpi_processor_handle_eject(struct acpi_processor *pr);
-
+static int acpi_processor_start(struct acpi_processor *pr);
static const struct acpi_device_id processor_device_ids[] = {
{ACPI_PROCESSOR_OBJECT_HID, 0},
@@ -423,10 +423,29 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb,
struct acpi_processor *pr = per_cpu(processors, cpu);
if (action == CPU_ONLINE && pr) {
- acpi_processor_ppc_has_changed(pr, 0);
- acpi_processor_hotplug(pr);
- acpi_processor_reevaluate_tstate(pr, action);
- acpi_processor_tstate_has_changed(pr);
+ /* CPU got physically hotplugged and onlined the first time:
+ * Initialize missing things
+ */
+ if (pr->flags.need_hotplug_init) {
+ struct cpuidle_driver *idle_driver =
+ cpuidle_get_driver();
+
+ printk(KERN_INFO "Will online and init hotplugged "
+ "CPU: %d\n", pr->id);
+ WARN(acpi_processor_start(pr), "Failed to start CPU:"
+ " %d\n", pr->id);
+ pr->flags.need_hotplug_init = 0;
+ if (idle_driver && !strcmp(idle_driver->name,
+ "intel_idle")) {
+ intel_idle_cpu_init(pr->id);
+ }
+ /* Normal CPU soft online event */
+ } else {
+ acpi_processor_ppc_has_changed(pr, 0);
+ acpi_processor_cst_has_changed(pr);
+ acpi_processor_reevaluate_tstate(pr, action);
+ acpi_processor_tstate_has_changed(pr);
+ }
}
if (action == CPU_DEAD && pr) {
/* invalidate the flag.throttling after one CPU is offline */
@@ -440,7 +459,15 @@ static struct notifier_block acpi_cpu_notifier =
.notifier_call = acpi_cpu_soft_notify,
};
-static int __cpuinit acpi_processor_start(struct acpi_processor *pr)
+/*
+ * acpi_processor_start() is called by the cpu_hotplug_notifier func:
+ * acpi_cpu_soft_notify(). Getting it __cpuinit{data} is difficult, the
+ * root cause seem to be that acpi_processor_uninstall_hotplug_notify()
+ * is in the module_exit (__exit) func. Allowing acpi_processor_start()
+ * to not be in __cpuinit section, but being called from __cpuinit funcs
+ * via __ref looks like the right thing to do here.
+ */
+static __ref int acpi_processor_start(struct acpi_processor *pr)
{
struct acpi_device *device = per_cpu(processor_device_array, pr->id);
int result = 0;
@@ -491,7 +518,12 @@ err_power_exit:
return result;
}
-
+/*
+ * Do not put anything in here which needs the core to be online.
+ * For example MSR access or setting up things which check for cpuinfo_x86
+ * (cpu_data(cpu)) values, like CPU feature flags, family, model, etc.
+ * Such things have to be put in and set up above in acpi_processor_start()
+ */
static int __cpuinit acpi_processor_add(struct acpi_device *device)
{
struct acpi_processor *pr = NULL;
@@ -546,6 +578,21 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
result = -EFAULT;
goto err_free_cpumask;
}
+
+ /*
+ * Do not start hotplugged CPUs now, but when they
+ * are onlined the first time
+ */
+ if (pr->flags.need_hotplug_init)
+ return 0;
+
+ /*
+ * Do not start hotplugged CPUs now, but when they
+ * are onlined the first time
+ */
+ if (pr->flags.need_hotplug_init)
+ return 0;
+
result = acpi_processor_start(pr);
if (result)
goto err_remove_sysfs;
@@ -751,6 +798,17 @@ static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
return AE_ERROR;
}
+ /* CPU got hot-plugged, but cpu_data is not initialized yet
+ * Set flag to delay cpu_idle/throttling initialization
+ * in:
+ * acpi_processor_add()
+ * acpi_processor_get_info()
+ * and do it when the CPU gets online the first time
+ * TBD: Cleanup above functions and try to do this more elegant.
+ */
+ printk(KERN_INFO "CPU %d got hotplugged\n", pr->id);
+ pr->flags.need_hotplug_init = 1;
+
return AE_OK;
}
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 20bce51..54ab97b 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -527,7 +527,7 @@ int intel_idle_cpu_init(int cpu)
return 0;
}
-
+EXPORT_SYMBOL_GPL(intel_idle_cpu_init);
static int __init intel_idle_init(void)
{
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 610f6fb..8cf7e98 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -195,6 +195,7 @@ struct acpi_processor_flags {
u8 has_cst:1;
u8 power_setup_done:1;
u8 bm_rld_set:1;
+ u8 need_hotplug_init:1;
};
struct acpi_processor {
--
1.7.9.rc2
^ permalink raw reply related
* [PATCH 03/10] ACPI, APEI: Add 64-bit read/write support for APEI on i386
From: Len Brown @ 2012-01-24 1:30 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Myron Stowe, Myron Stowe, Len Brown
In-Reply-To: <54d5dcc45af7adbb907072d042bbece4c2b4de6e.1327368464.git.len.brown@intel.com>
From: Myron Stowe <mstowe@redhat.com>
Base ACPI (CA) currently does not support atomic 64-bit reads and writes
(acpi_read() and acpi_write() split 64-bit loads/stores into two
32-bit transfers) yet APEI expects 64-bit transfer capability, even
when running on 32-bit systems.
This patch implements 64-bit read and write routines for APEI usage.
This patch re-factors similar functionality introduced in commit
04c25997c97, bringing it into the ACPI subsystem in preparation for
removing ./drivers/acpi/atomicio.[ch]. In the implementation I have
replicated acpi_os_read_memory() and acpi_os_write_memory(), creating
64-bit versions for APEI to utilize, as opposed to something more
elegant. My thinking is that we should attempt to see if we can get
ACPI's CA/OSL changed so that the existing acpi_read() and acpi_write()
interfaces are natively 64-bit capable and then subsequently remove the
replication.
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/apei/apei-base.c | 35 ++-----------
drivers/acpi/osl.c | 116 +++++++++++++++++++++++++++++++++++++++++
include/acpi/acpiosxf.h | 4 ++
3 files changed, 124 insertions(+), 31 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index e45350c..e5d53b7 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -596,33 +596,19 @@ int apei_read(u64 *val, struct acpi_generic_address *reg)
{
int rc;
u64 address;
- u32 tmp, width = reg->bit_width;
acpi_status status;
rc = apei_check_gar(reg, &address);
if (rc)
return rc;
- if (width == 64)
- width = 32; /* Break into two 32-bit transfers */
-
*val = 0;
switch(reg->space_id) {
case ACPI_ADR_SPACE_SYSTEM_MEMORY:
- status = acpi_os_read_memory((acpi_physical_address)
- address, &tmp, width);
+ status = acpi_os_read_memory64((acpi_physical_address)
+ address, val, reg->bit_width);
if (ACPI_FAILURE(status))
return -EIO;
- *val = tmp;
-
- if (reg->bit_width == 64) {
- /* Read the top 32 bits */
- status = acpi_os_read_memory((acpi_physical_address)
- (address + 4), &tmp, 32);
- if (ACPI_FAILURE(status))
- return -EIO;
- *val |= ((u64)tmp << 32);
- }
break;
case ACPI_ADR_SPACE_SYSTEM_IO:
status = acpi_os_read_port(address, (u32 *)val, reg->bit_width);
@@ -642,31 +628,18 @@ int apei_write(u64 val, struct acpi_generic_address *reg)
{
int rc;
u64 address;
- u32 width = reg->bit_width;
acpi_status status;
rc = apei_check_gar(reg, &address);
if (rc)
return rc;
- if (width == 64)
- width = 32; /* Break into two 32-bit transfers */
-
switch (reg->space_id) {
case ACPI_ADR_SPACE_SYSTEM_MEMORY:
- status = acpi_os_write_memory((acpi_physical_address)
- address, ACPI_LODWORD(val),
- width);
+ status = acpi_os_write_memory64((acpi_physical_address)
+ address, val, reg->bit_width);
if (ACPI_FAILURE(status))
return -EIO;
-
- if (reg->bit_width == 64) {
- status = acpi_os_write_memory((acpi_physical_address)
- (address + 4),
- ACPI_HIDWORD(val), 32);
- if (ACPI_FAILURE(status))
- return -EIO;
- }
break;
case ACPI_ADR_SPACE_SYSTEM_IO:
status = acpi_os_write_port(address, val, reg->bit_width);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fcc12d8..5498a6d 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -710,6 +710,67 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u32 * value, u32 width)
return AE_OK;
}
+#ifdef readq
+static inline u64 read64(const volatile void __iomem *addr)
+{
+ return readq(addr);
+}
+#else
+static inline u64 read64(const volatile void __iomem *addr)
+{
+ u64 l, h;
+ l = readl(addr);
+ h = readl(addr+4);
+ return l | (h << 32);
+}
+#endif
+
+acpi_status
+acpi_os_read_memory64(acpi_physical_address phys_addr, u64 *value, u32 width)
+{
+ void __iomem *virt_addr;
+ unsigned int size = width / 8;
+ bool unmap = false;
+ u64 dummy;
+
+ rcu_read_lock();
+ virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
+ if (!virt_addr) {
+ rcu_read_unlock();
+ virt_addr = acpi_os_ioremap(phys_addr, size);
+ if (!virt_addr)
+ return AE_BAD_ADDRESS;
+ unmap = true;
+ }
+
+ if (!value)
+ value = &dummy;
+
+ switch (width) {
+ case 8:
+ *(u8 *) value = readb(virt_addr);
+ break;
+ case 16:
+ *(u16 *) value = readw(virt_addr);
+ break;
+ case 32:
+ *(u32 *) value = readl(virt_addr);
+ break;
+ case 64:
+ *(u64 *) value = read64(virt_addr);
+ break;
+ default:
+ BUG();
+ }
+
+ if (unmap)
+ iounmap(virt_addr);
+ else
+ rcu_read_unlock();
+
+ return AE_OK;
+}
+
acpi_status
acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
{
@@ -749,6 +810,61 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
return AE_OK;
}
+#ifdef writeq
+static inline void write64(u64 val, volatile void __iomem *addr)
+{
+ writeq(val, addr);
+}
+#else
+static inline void write64(u64 val, volatile void __iomem *addr)
+{
+ writel(val, addr);
+ writel(val>>32, addr+4);
+}
+#endif
+
+acpi_status
+acpi_os_write_memory64(acpi_physical_address phys_addr, u64 value, u32 width)
+{
+ void __iomem *virt_addr;
+ unsigned int size = width / 8;
+ bool unmap = false;
+
+ rcu_read_lock();
+ virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
+ if (!virt_addr) {
+ rcu_read_unlock();
+ virt_addr = acpi_os_ioremap(phys_addr, size);
+ if (!virt_addr)
+ return AE_BAD_ADDRESS;
+ unmap = true;
+ }
+
+ switch (width) {
+ case 8:
+ writeb(value, virt_addr);
+ break;
+ case 16:
+ writew(value, virt_addr);
+ break;
+ case 32:
+ writel(value, virt_addr);
+ break;
+ case 64:
+ write64(value, virt_addr);
+ break;
+ default:
+ BUG();
+ }
+
+ if (unmap)
+ iounmap(virt_addr);
+ else
+ rcu_read_unlock();
+
+ return AE_OK;
+}
+
acpi_status
acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
u64 *value, u32 width)
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 2fe8639..7c9aebe 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -218,9 +218,13 @@ acpi_status acpi_os_write_port(acpi_io_address address, u32 value, u32 width);
*/
acpi_status
acpi_os_read_memory(acpi_physical_address address, u32 * value, u32 width);
+acpi_status
+acpi_os_read_memory64(acpi_physical_address address, u64 *value, u32 width);
acpi_status
acpi_os_write_memory(acpi_physical_address address, u32 value, u32 width);
+acpi_status
+acpi_os_write_memory64(acpi_physical_address address, u64 value, u32 width);
/*
* Platform and hardware-independent PCI configuration space access
--
1.7.9.rc2
^ permalink raw reply related
* [PATCH 04/10] ACPI, APEI: Add RAM mapping support to ACPI
From: Len Brown @ 2012-01-24 1:30 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Myron Stowe, Myron Stowe, Len Brown
In-Reply-To: <54d5dcc45af7adbb907072d042bbece4c2b4de6e.1327368464.git.len.brown@intel.com>
From: Myron Stowe <mstowe@redhat.com>
This patch adds support for RAM to ACPI's mapping capabilities in order
to support APEI error injection (EINJ) actions.
This patch re-factors similar functionality introduced in commit
76da3fb3575, bringing it into osl.c in preparation for removing
./drivers/acpi/atomicio.[ch].
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/osl.c | 36 ++++++++++++++++++++++++++++++++++--
1 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 5498a6d..412a1e0 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -31,6 +31,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/mm.h>
+#include <linux/highmem.h>
#include <linux/pci.h>
#include <linux/interrupt.h>
#include <linux/kmod.h>
@@ -321,6 +322,37 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
return NULL;
}
+#ifndef CONFIG_IA64
+#define should_use_kmap(pfn) page_is_ram(pfn)
+#else
+/* ioremap will take care of cache attributes */
+#define should_use_kmap(pfn) 0
+#endif
+
+static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
+{
+ unsigned long pfn;
+
+ pfn = pg_off >> PAGE_SHIFT;
+ if (should_use_kmap(pfn)) {
+ if (pg_sz > PAGE_SIZE)
+ return NULL;
+ return (void __iomem __force *)kmap(pfn_to_page(pfn));
+ } else
+ return acpi_os_ioremap(pg_off, pg_sz);
+}
+
+static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
+{
+ unsigned long pfn;
+
+ pfn = pg_off >> PAGE_SHIFT;
+ if (page_is_ram(pfn))
+ kunmap(pfn_to_page(pfn));
+ else
+ iounmap(vaddr);
+}
+
void __iomem *__init_refok
acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
{
@@ -353,7 +385,7 @@ acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
pg_off = round_down(phys, PAGE_SIZE);
pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
- virt = acpi_os_ioremap(pg_off, pg_sz);
+ virt = acpi_map(pg_off, pg_sz);
if (!virt) {
mutex_unlock(&acpi_ioremap_lock);
kfree(map);
@@ -384,7 +416,7 @@ static void acpi_os_map_cleanup(struct acpi_ioremap *map)
{
if (!map->refcount) {
synchronize_rcu();
- iounmap(map->virt);
+ acpi_unmap(map->phys, map->virt);
kfree(map);
}
}
--
1.7.9.rc2
^ permalink raw reply related
* [PATCH 05/10] ACPI: Remove ./drivers/acpi/atomicio.[ch]
From: Len Brown @ 2012-01-24 1:30 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Myron Stowe, Myron Stowe, Len Brown
In-Reply-To: <54d5dcc45af7adbb907072d042bbece4c2b4de6e.1327368464.git.len.brown@intel.com>
From: Myron Stowe <mstowe@redhat.com>
With the conversion of atomicio's routines in place (see commits
6f68c91c55e and 700130b41f4), atomicio.[ch] can be removed, replacing
the APEI specific pre-mapping capabilities with the more generalized
versions that drivers/acpi/osl.c provides.
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/Makefile | 1 -
drivers/acpi/atomicio.c | 422 -----------------------------------------------
include/acpi/atomicio.h | 10 -
3 files changed, 0 insertions(+), 433 deletions(-)
delete mode 100644 drivers/acpi/atomicio.c
delete mode 100644 include/acpi/atomicio.h
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c07f44f..1567028 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -19,7 +19,6 @@ obj-y += acpi.o \
# All the builtin files are in the "acpi." module_param namespace.
acpi-y += osl.o utils.o reboot.o
-acpi-y += atomicio.o
acpi-y += nvs.o
# sleep related files
diff --git a/drivers/acpi/atomicio.c b/drivers/acpi/atomicio.c
deleted file mode 100644
index d4a5b3d..0000000
--- a/drivers/acpi/atomicio.c
+++ /dev/null
@@ -1,422 +0,0 @@
-/*
- * atomicio.c - ACPI IO memory pre-mapping/post-unmapping, then
- * accessing in atomic context.
- *
- * This is used for NMI handler to access IO memory area, because
- * ioremap/iounmap can not be used in NMI handler. The IO memory area
- * is pre-mapped in process context and accessed in NMI handler.
- *
- * Copyright (C) 2009-2010, Intel Corp.
- * Author: Huang Ying <ying.huang@intel.com>
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * 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
- */
-
-#include <linux/kernel.h>
-#include <linux/export.h>
-#include <linux/init.h>
-#include <linux/acpi.h>
-#include <linux/io.h>
-#include <linux/kref.h>
-#include <linux/rculist.h>
-#include <linux/interrupt.h>
-#include <linux/slab.h>
-#include <linux/mm.h>
-#include <linux/highmem.h>
-#include <acpi/atomicio.h>
-
-#define ACPI_PFX "ACPI: "
-
-static LIST_HEAD(acpi_iomaps);
-/*
- * Used for mutual exclusion between writers of acpi_iomaps list, for
- * synchronization between readers and writer, RCU is used.
- */
-static DEFINE_SPINLOCK(acpi_iomaps_lock);
-
-struct acpi_iomap {
- struct list_head list;
- void __iomem *vaddr;
- unsigned long size;
- phys_addr_t paddr;
- struct kref ref;
-};
-
-/* acpi_iomaps_lock or RCU read lock must be held before calling */
-static struct acpi_iomap *__acpi_find_iomap(phys_addr_t paddr,
- unsigned long size)
-{
- struct acpi_iomap *map;
-
- list_for_each_entry_rcu(map, &acpi_iomaps, list) {
- if (map->paddr + map->size >= paddr + size &&
- map->paddr <= paddr)
- return map;
- }
- return NULL;
-}
-
-/*
- * Atomic "ioremap" used by NMI handler, if the specified IO memory
- * area is not pre-mapped, NULL will be returned.
- *
- * acpi_iomaps_lock or RCU read lock must be held before calling
- */
-static void __iomem *__acpi_ioremap_fast(phys_addr_t paddr,
- unsigned long size)
-{
- struct acpi_iomap *map;
-
- map = __acpi_find_iomap(paddr, size/8);
- if (map)
- return map->vaddr + (paddr - map->paddr);
- else
- return NULL;
-}
-
-/* acpi_iomaps_lock must be held before calling */
-static void __iomem *__acpi_try_ioremap(phys_addr_t paddr,
- unsigned long size)
-{
- struct acpi_iomap *map;
-
- map = __acpi_find_iomap(paddr, size);
- if (map) {
- kref_get(&map->ref);
- return map->vaddr + (paddr - map->paddr);
- } else
- return NULL;
-}
-
-#ifndef CONFIG_IA64
-#define should_use_kmap(pfn) page_is_ram(pfn)
-#else
-/* ioremap will take care of cache attributes */
-#define should_use_kmap(pfn) 0
-#endif
-
-static void __iomem *acpi_map(phys_addr_t pg_off, unsigned long pg_sz)
-{
- unsigned long pfn;
-
- pfn = pg_off >> PAGE_SHIFT;
- if (should_use_kmap(pfn)) {
- if (pg_sz > PAGE_SIZE)
- return NULL;
- return (void __iomem __force *)kmap(pfn_to_page(pfn));
- } else
- return ioremap(pg_off, pg_sz);
-}
-
-static void acpi_unmap(phys_addr_t pg_off, void __iomem *vaddr)
-{
- unsigned long pfn;
-
- pfn = pg_off >> PAGE_SHIFT;
- if (page_is_ram(pfn))
- kunmap(pfn_to_page(pfn));
- else
- iounmap(vaddr);
-}
-
-/*
- * Used to pre-map the specified IO memory area. First try to find
- * whether the area is already pre-mapped, if it is, increase the
- * reference count (in __acpi_try_ioremap) and return; otherwise, do
- * the real ioremap, and add the mapping into acpi_iomaps list.
- */
-static void __iomem *acpi_pre_map(phys_addr_t paddr,
- unsigned long size)
-{
- void __iomem *vaddr;
- struct acpi_iomap *map;
- unsigned long pg_sz, flags;
- phys_addr_t pg_off;
-
- spin_lock_irqsave(&acpi_iomaps_lock, flags);
- vaddr = __acpi_try_ioremap(paddr, size);
- spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
- if (vaddr)
- return vaddr;
-
- pg_off = paddr & PAGE_MASK;
- pg_sz = ((paddr + size + PAGE_SIZE - 1) & PAGE_MASK) - pg_off;
- vaddr = acpi_map(pg_off, pg_sz);
- if (!vaddr)
- return NULL;
- map = kmalloc(sizeof(*map), GFP_KERNEL);
- if (!map)
- goto err_unmap;
- INIT_LIST_HEAD(&map->list);
- map->paddr = pg_off;
- map->size = pg_sz;
- map->vaddr = vaddr;
- kref_init(&map->ref);
-
- spin_lock_irqsave(&acpi_iomaps_lock, flags);
- vaddr = __acpi_try_ioremap(paddr, size);
- if (vaddr) {
- spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
- acpi_unmap(pg_off, map->vaddr);
- kfree(map);
- return vaddr;
- }
- list_add_tail_rcu(&map->list, &acpi_iomaps);
- spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-
- return map->vaddr + (paddr - map->paddr);
-err_unmap:
- acpi_unmap(pg_off, vaddr);
- return NULL;
-}
-
-/* acpi_iomaps_lock must be held before calling */
-static void __acpi_kref_del_iomap(struct kref *ref)
-{
- struct acpi_iomap *map;
-
- map = container_of(ref, struct acpi_iomap, ref);
- list_del_rcu(&map->list);
-}
-
-/*
- * Used to post-unmap the specified IO memory area. The iounmap is
- * done only if the reference count goes zero.
- */
-static void acpi_post_unmap(phys_addr_t paddr, unsigned long size)
-{
- struct acpi_iomap *map;
- unsigned long flags;
- int del;
-
- spin_lock_irqsave(&acpi_iomaps_lock, flags);
- map = __acpi_find_iomap(paddr, size);
- BUG_ON(!map);
- del = kref_put(&map->ref, __acpi_kref_del_iomap);
- spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-
- if (!del)
- return;
-
- synchronize_rcu();
- acpi_unmap(map->paddr, map->vaddr);
- kfree(map);
-}
-
-/* In NMI handler, should set silent = 1 */
-static int acpi_check_gar(struct acpi_generic_address *reg,
- u64 *paddr, int silent)
-{
- u32 width, space_id;
-
- width = reg->bit_width;
- space_id = reg->space_id;
- /* Handle possible alignment issues */
- memcpy(paddr, ®->address, sizeof(*paddr));
- if (!*paddr) {
- if (!silent)
- pr_warning(FW_BUG ACPI_PFX
- "Invalid physical address in GAR [0x%llx/%u/%u]\n",
- *paddr, width, space_id);
- return -EINVAL;
- }
-
- if ((width != 8) && (width != 16) && (width != 32) && (width != 64)) {
- if (!silent)
- pr_warning(FW_BUG ACPI_PFX
- "Invalid bit width in GAR [0x%llx/%u/%u]\n",
- *paddr, width, space_id);
- return -EINVAL;
- }
-
- if (space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
- space_id != ACPI_ADR_SPACE_SYSTEM_IO) {
- if (!silent)
- pr_warning(FW_BUG ACPI_PFX
- "Invalid address space type in GAR [0x%llx/%u/%u]\n",
- *paddr, width, space_id);
- return -EINVAL;
- }
-
- return 0;
-}
-
-/* Pre-map, working on GAR */
-int acpi_pre_map_gar(struct acpi_generic_address *reg)
-{
- u64 paddr;
- void __iomem *vaddr;
- int rc;
-
- if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
- return 0;
-
- rc = acpi_check_gar(reg, &paddr, 0);
- if (rc)
- return rc;
-
- vaddr = acpi_pre_map(paddr, reg->bit_width / 8);
- if (!vaddr)
- return -EIO;
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(acpi_pre_map_gar);
-
-/* Post-unmap, working on GAR */
-int acpi_post_unmap_gar(struct acpi_generic_address *reg)
-{
- u64 paddr;
- int rc;
-
- if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
- return 0;
-
- rc = acpi_check_gar(reg, &paddr, 0);
- if (rc)
- return rc;
-
- acpi_post_unmap(paddr, reg->bit_width / 8);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(acpi_post_unmap_gar);
-
-#ifdef readq
-static inline u64 read64(const volatile void __iomem *addr)
-{
- return readq(addr);
-}
-#else
-static inline u64 read64(const volatile void __iomem *addr)
-{
- u64 l, h;
- l = readl(addr);
- h = readl(addr+4);
- return l | (h << 32);
-}
-#endif
-
-/*
- * Can be used in atomic (including NMI) or process context. RCU read
- * lock can only be released after the IO memory area accessing.
- */
-static int acpi_atomic_read_mem(u64 paddr, u64 *val, u32 width)
-{
- void __iomem *addr;
-
- rcu_read_lock();
- addr = __acpi_ioremap_fast(paddr, width);
- switch (width) {
- case 8:
- *val = readb(addr);
- break;
- case 16:
- *val = readw(addr);
- break;
- case 32:
- *val = readl(addr);
- break;
- case 64:
- *val = read64(addr);
- break;
- default:
- return -EINVAL;
- }
- rcu_read_unlock();
-
- return 0;
-}
-
-#ifdef writeq
-static inline void write64(u64 val, volatile void __iomem *addr)
-{
- writeq(val, addr);
-}
-#else
-static inline void write64(u64 val, volatile void __iomem *addr)
-{
- writel(val, addr);
- writel(val>>32, addr+4);
-}
-#endif
-
-static int acpi_atomic_write_mem(u64 paddr, u64 val, u32 width)
-{
- void __iomem *addr;
-
- rcu_read_lock();
- addr = __acpi_ioremap_fast(paddr, width);
- switch (width) {
- case 8:
- writeb(val, addr);
- break;
- case 16:
- writew(val, addr);
- break;
- case 32:
- writel(val, addr);
- break;
- case 64:
- write64(val, addr);
- break;
- default:
- return -EINVAL;
- }
- rcu_read_unlock();
-
- return 0;
-}
-
-/* GAR accessing in atomic (including NMI) or process context */
-int acpi_atomic_read(u64 *val, struct acpi_generic_address *reg)
-{
- u64 paddr;
- int rc;
-
- rc = acpi_check_gar(reg, &paddr, 1);
- if (rc)
- return rc;
-
- *val = 0;
- switch (reg->space_id) {
- case ACPI_ADR_SPACE_SYSTEM_MEMORY:
- return acpi_atomic_read_mem(paddr, val, reg->bit_width);
- case ACPI_ADR_SPACE_SYSTEM_IO:
- return acpi_os_read_port(paddr, (u32 *)val, reg->bit_width);
- default:
- return -EINVAL;
- }
-}
-EXPORT_SYMBOL_GPL(acpi_atomic_read);
-
-int acpi_atomic_write(u64 val, struct acpi_generic_address *reg)
-{
- u64 paddr;
- int rc;
-
- rc = acpi_check_gar(reg, &paddr, 1);
- if (rc)
- return rc;
-
- switch (reg->space_id) {
- case ACPI_ADR_SPACE_SYSTEM_MEMORY:
- return acpi_atomic_write_mem(paddr, val, reg->bit_width);
- case ACPI_ADR_SPACE_SYSTEM_IO:
- return acpi_os_write_port(paddr, val, reg->bit_width);
- default:
- return -EINVAL;
- }
-}
-EXPORT_SYMBOL_GPL(acpi_atomic_write);
diff --git a/include/acpi/atomicio.h b/include/acpi/atomicio.h
deleted file mode 100644
index 8b9fb4b..0000000
--- a/include/acpi/atomicio.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef ACPI_ATOMIC_IO_H
-#define ACPI_ATOMIC_IO_H
-
-int acpi_pre_map_gar(struct acpi_generic_address *reg);
-int acpi_post_unmap_gar(struct acpi_generic_address *reg);
-
-int acpi_atomic_read(u64 *val, struct acpi_generic_address *reg);
-int acpi_atomic_write(u64 val, struct acpi_generic_address *reg);
-
-#endif
--
1.7.9.rc2
^ permalink raw reply related
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