* Re: [PATCH v2] acpi: intel_idle : break dependency between modules
From: Srivatsa S. Bhat @ 2012-06-28 11:24 UTC (permalink / raw)
To: Daniel Lezcano
Cc: trenn, deepthi, linux-acpi, linux-pm, linux-pm, lenb, rjw, x86,
linux-kernel, linaro-dev
In-Reply-To: <1340873202-2476-1-git-send-email-daniel.lezcano@linaro.org>
On 06/28/2012 02:16 PM, Daniel Lezcano wrote:
> When the system is booted with some cpus offline, the idle
> driver is not initialized. When a cpu is set online, the
> acpi code call the intel idle init function. Unfortunately
> this code introduce a dependency between intel_idle and acpi.
>
> This patch is intended to remove this dependency by using the
> notifier of intel_idle. This patch has the benefit of
> encapsulating the intel_idle driver and remove some exported
> functions.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Looks good to me.
Regards,
Srivatsa S. Bhat
> ---
> drivers/acpi/processor_driver.c | 7 ------
> drivers/idle/intel_idle.c | 41 +++++++++++++++++++++++++-------------
> include/linux/cpuidle.h | 7 ------
> 3 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 0734086..8648b29 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -427,18 +427,11 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb,
> * 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);
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index d0f59c3..fe95d54 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -96,6 +96,7 @@ static const struct idle_cpu *icpu;
> static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
> static int intel_idle(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index);
> +static int intel_idle_cpu_init(int cpu);
>
> static struct cpuidle_state *cpuidle_state_table;
>
> @@ -302,22 +303,35 @@ static void __setup_broadcast_timer(void *arg)
> clockevents_notify(reason, &cpu);
> }
>
> -static int setup_broadcast_cpuhp_notify(struct notifier_block *n,
> - unsigned long action, void *hcpu)
> +static int cpu_hotplug_notify(struct notifier_block *n,
> + unsigned long action, void *hcpu)
> {
> int hotcpu = (unsigned long)hcpu;
> + struct cpuidle_device *dev;
>
> switch (action & 0xf) {
> case CPU_ONLINE:
> - smp_call_function_single(hotcpu, __setup_broadcast_timer,
> - (void *)true, 1);
> +
> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
> + smp_call_function_single(hotcpu, __setup_broadcast_timer,
> + (void *)true, 1);
> +
> + /*
> + * Some systems can hotplug a cpu at runtime after
> + * the kernel has booted, we have to initialize the
> + * driver in this case
> + */
> + dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
> + if (!dev->registered)
> + intel_idle_cpu_init(hotcpu);
> +
> break;
> }
> return NOTIFY_OK;
> }
>
> -static struct notifier_block setup_broadcast_notifier = {
> - .notifier_call = setup_broadcast_cpuhp_notify,
> +static struct notifier_block cpu_hotplug_notifier = {
> + .notifier_call = cpu_hotplug_notify,
> };
>
> static void auto_demotion_disable(void *dummy)
> @@ -405,10 +419,10 @@ static int intel_idle_probe(void)
>
> if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */
> lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
> - else {
> + else
> on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
> - register_cpu_notifier(&setup_broadcast_notifier);
> - }
> +
> + register_cpu_notifier(&cpu_hotplug_notifier);
>
> pr_debug(PREFIX "v" INTEL_IDLE_VERSION
> " model 0x%X\n", boot_cpu_data.x86_model);
> @@ -494,7 +508,7 @@ static int intel_idle_cpuidle_driver_init(void)
> * allocate, initialize, register cpuidle_devices
> * @cpu: cpu/core to initialize
> */
> -int intel_idle_cpu_init(int cpu)
> +static int intel_idle_cpu_init(int cpu)
> {
> int cstate;
> struct cpuidle_device *dev;
> @@ -539,7 +553,6 @@ int intel_idle_cpu_init(int cpu)
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(intel_idle_cpu_init);
>
> static int __init intel_idle_init(void)
> {
> @@ -581,10 +594,10 @@ static void __exit intel_idle_exit(void)
> intel_idle_cpuidle_devices_uninit();
> cpuidle_unregister_driver(&intel_idle_driver);
>
> - if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
> +
> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
> on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
> - unregister_cpu_notifier(&setup_broadcast_notifier);
> - }
> + unregister_cpu_notifier(&cpu_hotplug_notifier);
>
> return;
> }
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 5ab7183..66d7e0d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -213,14 +213,7 @@ struct cpuidle_governor {
> extern int cpuidle_register_governor(struct cpuidle_governor *gov);
> extern void cpuidle_unregister_governor(struct cpuidle_governor *gov);
>
> -#ifdef CONFIG_INTEL_IDLE
> -extern int intel_idle_cpu_init(int cpu);
> #else
> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
> -#endif
> -
> -#else
> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
>
> static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
> {return 0;}
>
^ permalink raw reply
* Re: [PATCH v2] acpi: intel_idle : break dependency between modules
From: Daniel Lezcano @ 2012-06-28 11:27 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: linaro-dev, linux-pm, x86, linux-kernel, linux-acpi, linux-pm
In-Reply-To: <4FEC3EF7.5020208@linux.vnet.ibm.com>
On 06/28/2012 01:24 PM, Srivatsa S. Bhat wrote:
> On 06/28/2012 02:16 PM, Daniel Lezcano wrote:
>> When the system is booted with some cpus offline, the idle
>> driver is not initialized. When a cpu is set online, the
>> acpi code call the intel idle init function. Unfortunately
>> this code introduce a dependency between intel_idle and acpi.
>>
>> This patch is intended to remove this dependency by using the
>> notifier of intel_idle. This patch has the benefit of
>> encapsulating the intel_idle driver and remove some exported
>> functions.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
>
> Looks good to me.
Thanks for the review Srivatsa.
Shall I consider it as an acked-by ?
-- Daniel
>
> Regards,
> Srivatsa S. Bhat
>
>> ---
>> drivers/acpi/processor_driver.c | 7 ------
>> drivers/idle/intel_idle.c | 41 +++++++++++++++++++++++++-------------
>> include/linux/cpuidle.h | 7 ------
>> 3 files changed, 27 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 0734086..8648b29 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -427,18 +427,11 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb,
>> * 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);
>> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
>> index d0f59c3..fe95d54 100644
>> --- a/drivers/idle/intel_idle.c
>> +++ b/drivers/idle/intel_idle.c
>> @@ -96,6 +96,7 @@ static const struct idle_cpu *icpu;
>> static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
>> static int intel_idle(struct cpuidle_device *dev,
>> struct cpuidle_driver *drv, int index);
>> +static int intel_idle_cpu_init(int cpu);
>>
>> static struct cpuidle_state *cpuidle_state_table;
>>
>> @@ -302,22 +303,35 @@ static void __setup_broadcast_timer(void *arg)
>> clockevents_notify(reason, &cpu);
>> }
>>
>> -static int setup_broadcast_cpuhp_notify(struct notifier_block *n,
>> - unsigned long action, void *hcpu)
>> +static int cpu_hotplug_notify(struct notifier_block *n,
>> + unsigned long action, void *hcpu)
>> {
>> int hotcpu = (unsigned long)hcpu;
>> + struct cpuidle_device *dev;
>>
>> switch (action & 0xf) {
>> case CPU_ONLINE:
>> - smp_call_function_single(hotcpu, __setup_broadcast_timer,
>> - (void *)true, 1);
>> +
>> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
>> + smp_call_function_single(hotcpu, __setup_broadcast_timer,
>> + (void *)true, 1);
>> +
>> + /*
>> + * Some systems can hotplug a cpu at runtime after
>> + * the kernel has booted, we have to initialize the
>> + * driver in this case
>> + */
>> + dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
>> + if (!dev->registered)
>> + intel_idle_cpu_init(hotcpu);
>> +
>> break;
>> }
>> return NOTIFY_OK;
>> }
>>
>> -static struct notifier_block setup_broadcast_notifier = {
>> - .notifier_call = setup_broadcast_cpuhp_notify,
>> +static struct notifier_block cpu_hotplug_notifier = {
>> + .notifier_call = cpu_hotplug_notify,
>> };
>>
>> static void auto_demotion_disable(void *dummy)
>> @@ -405,10 +419,10 @@ static int intel_idle_probe(void)
>>
>> if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */
>> lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
>> - else {
>> + else
>> on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
>> - register_cpu_notifier(&setup_broadcast_notifier);
>> - }
>> +
>> + register_cpu_notifier(&cpu_hotplug_notifier);
>>
>> pr_debug(PREFIX "v" INTEL_IDLE_VERSION
>> " model 0x%X\n", boot_cpu_data.x86_model);
>> @@ -494,7 +508,7 @@ static int intel_idle_cpuidle_driver_init(void)
>> * allocate, initialize, register cpuidle_devices
>> * @cpu: cpu/core to initialize
>> */
>> -int intel_idle_cpu_init(int cpu)
>> +static int intel_idle_cpu_init(int cpu)
>> {
>> int cstate;
>> struct cpuidle_device *dev;
>> @@ -539,7 +553,6 @@ int intel_idle_cpu_init(int cpu)
>>
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(intel_idle_cpu_init);
>>
>> static int __init intel_idle_init(void)
>> {
>> @@ -581,10 +594,10 @@ static void __exit intel_idle_exit(void)
>> intel_idle_cpuidle_devices_uninit();
>> cpuidle_unregister_driver(&intel_idle_driver);
>>
>> - if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
>> +
>> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
>> on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
>> - unregister_cpu_notifier(&setup_broadcast_notifier);
>> - }
>> + unregister_cpu_notifier(&cpu_hotplug_notifier);
>>
>> return;
>> }
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 5ab7183..66d7e0d 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -213,14 +213,7 @@ struct cpuidle_governor {
>> extern int cpuidle_register_governor(struct cpuidle_governor *gov);
>> extern void cpuidle_unregister_governor(struct cpuidle_governor *gov);
>>
>> -#ifdef CONFIG_INTEL_IDLE
>> -extern int intel_idle_cpu_init(int cpu);
>> #else
>> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
>> -#endif
>> -
>> -#else
>> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
>>
>> static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>> {return 0;}
>>
>
--
<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 v2] acpi: intel_idle : break dependency between modules
From: Srivatsa S. Bhat @ 2012-06-28 11:56 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linaro-dev, linux-pm, x86, linux-kernel, linux-acpi, linux-pm
In-Reply-To: <4FEC3FAB.20706@linaro.org>
On 06/28/2012 04:57 PM, Daniel Lezcano wrote:
> On 06/28/2012 01:24 PM, Srivatsa S. Bhat wrote:
>> On 06/28/2012 02:16 PM, Daniel Lezcano wrote:
>>> When the system is booted with some cpus offline, the idle
>>> driver is not initialized. When a cpu is set online, the
>>> acpi code call the intel idle init function. Unfortunately
>>> this code introduce a dependency between intel_idle and acpi.
>>>
>>> This patch is intended to remove this dependency by using the
>>> notifier of intel_idle. This patch has the benefit of
>>> encapsulating the intel_idle driver and remove some exported
>>> functions.
>>>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>
>>
>> Looks good to me.
>
> Thanks for the review Srivatsa.
>
> Shall I consider it as an acked-by ?
>
Sure, go ahead! :-)
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH v3 0/7] OMAP System Control Module
From: Valentin, Eduardo @ 2012-06-28 13:48 UTC (permalink / raw)
To: Konstantin Baydarov
Cc: b-cousson, kishon, santosh.shilimkar, tony, paul, balbi,
amit.kucheria, linux-pm, linux-arm-kernel, linux-omap,
amit.kachhap, Eduardo Valentin
In-Reply-To: <CAGF5oy925c96QqwjqMEaUYDon+mSVGd2dtgnWrzwnrq0rN14Ng@mail.gmail.com>
Hello again,
On Thu, Jun 28, 2012 at 9:29 AM, Valentin, Eduardo
<eduardo.valentin@ti.com> wrote:
> Hello Konstantin,
>
> On Thu, Jun 28, 2012 at 7:43 AM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
>> Hello,
>>
>> On Wed, Jun 27, 2012 at 10:04:32PM +0400, Konstantin Baydarov wrote:
>>> Hello.
>>>
>>> This is a next version of series of patches(based on Eduardo Valentin's patch set) adding a basic support for system control module, on OMAP4+ context. It is a working in progress.
>>>
>>> Main changes since previous patch set version:
>>> - Bandgap and usb phy: drivers are now independent from control module driver, they use their own functions to acess scm registers.
>>
>> Well, I believe the idea of having them with their own io resource was to avoid
>> children drivers accessing each other io areas. Is this now working?
>>
>>> - omap-control-core: resources aren't hardcoded, they are specified in dts file.
>>> - omap-control-core: Control module is a built-in driver - added control module select to ARCH_HAS_CONTROL_MODULE and ARCH_OMAP4.
>>> Probably, no configuration option is required!
>>
>> Why is this? I suppose the idea is to have the arch config selecting that flag.
>>
>>> - omap-control-core: Added early init call that ioremaps control module IOMEM window, this allows access of SCM registers very early, for example from omap_type()
>>
>> Is this going to be reserved as well? if yes, how children are going to have
>> their own access functions?
>>
>>> - omap-control-core: Removed device pointer from omap-control-core API arguments, becuase there can be only one instance control
>>> module device.
>>> - omap-control-core: removed omap_control_get, omap_control_readl, omap_control_writel
>>
>> fine, assuming the io split works...
>>
>>> - omap-control-core: added omap_control_status_read that is used early in omap_type
>>> - Bandgap and usb phy: Added private spinlocks for bandgap and usb drivers.
>>> - Bandgap: Check the type of bandgap dynamically in bandgap driver probe function by reading
>>> omap core control module revision register CONTROL_GEN_CORE_REVISION.
>>
>> this has some issue... I will post comment on the patch
>>
>>> - Bandgap and usb phy: Parent SCM platform device IOMEM resources is used to get the base address of SCM window.
>>
>> Ohh.. Then what is the point of having them using their own io access functions,
>> nothing is again protected as you have only 1 io resource which is shared.
>> And now is even worse because you have several io access function..
>> I think this is moving backwards.
>>
>>> - Bandgap masks defines were moved to drivers/thermal/omap-bandgap.c.
>>>
>>> TODO list for bandgap driver:
>>> - Reserve omap-control-core IOMEM window.
>>> - Improve thermal zone definition for OMAP4
>>> - Introduce the thermal zones for OMAP5
>>
>> Based on the review comments on RFC patch series, there are more
>> things to be done.
>>
>> I have the O4 and O5 zone definition ready. But that work depends
>> on generic CPU cooling patches. I have also looked the driver support
>> for 4430. But the probing and io resource split is still based on
>> previous design. I have also a patch which does the remapping in the
>> bandgap driver. But really, for this to work it would require to have
>> several entries in the reg property. And that is going to change from
>> BG version to BG version.
>>
>> I can prepare some patches for this.
>
> You can find the updated patches here:
> git@gitorious.org:thermal-framework/thermal-framework.git
> thermal_work/omap/bandgap_clean
>
> But, as I said they are based on the generic cpu cooling. And the zone
> registration follows the API as in linux-next.
FYI, I just updated the branches on that repo. The branch
thermal_work/omap/pu has all required changes in order to get BG
driver working on 4430/4460/4470 and O5.
It relies on Rui's patch set sent to linux-pm, which reorgs the
thermal fw a bit, combined with the generic cpu cooling device by
Amit. I also pushed in the 'pu' branch the APIs changes mentioned by
Durga on other thread.
While checking the pu branch you should be able to get a thermal zone
with cpu cooling for OMAP on the mentioned omap versions.
Once there is an agreement about the APIs between SCM core driver and
its children, I can send out the BG driver patches and this initial
omap thermal support using the generic thermal layer.
There are some changes in the core thermal layer in the
thermal_work/eduardo_thermal_upgrade. But there could be probably some
extra changes in the core layer to finalize the omap thermal support.
That's a work in progress :-)
>
>
> --
>
> Eduardo Valentin
--
Eduardo Valentin
--
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: a few questions about Runtime PM
From: Alan Stern @ 2012-06-28 13:58 UTC (permalink / raw)
To: Zhang, QianFeng; +Cc: linux-pm@lists.linux-foundation.org
In-Reply-To: <4D0BDB1B0D72DF43A9D4C0B62FC1DDDE028423D3@SCYBEXDAG03.amd.com>
On Thu, 28 Jun 2012, Zhang, QianFeng wrote:
> I am looking through the IO device Runtime PM implementation on Linux (Fedora 16). There are some questions that
> need you help to clarify for me, here they are
Please tell your email client to wrap lines after 72 columns or so.
> 1) For those IO devices that could not be waked up through ACPI GPEs or native PCI Express PME , can Runtime suspend/resume be used ?
Yes, it can be used. If the drivers need wakeup support then the
devices have to have some other wakeup mechanism, such as an IRQ. If
the drivers don't need wakeup support then obviously the device doesn't
have to provide it.
> 2) In rtl8169's implementation of Runtime suspend/resume, there is a routine rtl8169_check_link_status(), which always invokes
> pm_schedule_suspend() if the Link line is dis-connected, pm_request_resume() if the Link line is connected. And rtl8169_check_link_status
> itself is called through the transmit time out handler, which invokes rtl8169_reset_task. So rtl8169_check_link_status, as an opportunity point
> for Runtime PM for its network interface , does not require any wake-up signaling, right? The only Hardware Level requirement for rtl1869_check_link_status
> to work in this way is that the link status register can always be read no matter the network interface is in off or on status.
I don't know the answers to these questions.
> 3) How should the Block IO adapter devices like the SCSI Controller or FC Adapters should do Runtime PM ? In my understand, these
> Devices should be auto-suspended when there is no IO request pending for a predefined duration, and they should be resumed when
> a new IO request issued by the block layer. So these devices may use a different wake up method that has no dependency on PCI Express PME or
> ACPI GPE.
I don't know about FC adapters. SCSI controllers do not need wakeup.
Alan Stern
^ permalink raw reply
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: preeti @ 2012-06-28 16:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Linux PM mailing list, linux-pm, linux-acpi,
Srivatsa.S.Bhat, Deepthi Dharwar
In-Reply-To: <201206281153.25310.rjw@sisk.pl>
On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
> On Thursday, June 28, 2012, preeti wrote:
>>
>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>
>> Dave Hansen reported problems with suspend/resume when used
>> with intel_idle driver.More such problems were noticed.
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/674075.
>>
>> The reason for this could not be suspected till he reported
>> resume hang issue when used with acpi idle driver on his Lenovo-S10-3
>> Atom netbook.
>>
>> The patch-[PM / ACPI: Fix suspend/resume regression caused by cpuidle
>> cleanup],fixed this issue by ensuring that acpi idle drivers prevent
>> cpus from going into deeper sleep states during suspend to prevent
>> resume hang on certain bios.
>> http://marc.info/?l=linux-pm&m=133958534231884&w=2
>>
>> Commit b04e7bdb984e3b7f62fb7f44146a529f88cc7639
>> (ACPI: disable lower idle C-states across suspend/resume) throws
>> light on the resume hang issue on certain specific bios.
>>
>> Also the following lines in drivers/idle/intel_idle.c suggest intel_idle
>> drivers should also ensure cpus are prevented from entering idle states
>> during suspend to avoid a resume hang.
>>
>> /*
>> * Known limitations
>> * [..]
>> *
>> * ACPI has a .suspend hack to turn off deep c-states during suspend
>> * to avoid complications with the lapic timer workaround.
>> * Have not seen issues with suspend, but may need same workaround here.
>> *
>> */
>> This patch aims at having this fix in a place common to both the idle
>> drivers.
>>
>> Suspend is enabled only if ACPI is active on x86.Thus the setting of
>> acpi_idle_suspend during suspend is moved up to ACPI specific code with
>> both acpi and intel idle drivers checking if it is valid to enter deeper
>> idle states.The setting of acpi_idle_suspend is done via PM_SUSPEND_PREPARE
>> notifiers to avoid race conditions between processors entering idle states
>> and the ongoing process of suspend.
>>
>> The check on acpi_idle_suspend is included in the most appropriate header
>> so as to be visible to both the idle drivers irrespective of the
>> different configurations.Even if ACPI is disabled intel idle drivers can
>> still carry out the acpi_idle_suspend check.
>>
>> Reported-by: Dave Hansen <dave@linux.vnet.ibm.com>
>> Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>> Reviewed-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>> ---
>> Dave, does this patch ensure suspend/resume works
>> fine on your netbook if intel_idle driver is enabled?
>>
>> drivers/acpi/processor_idle.c | 46 +++++++++++++++++++----------------------
>> drivers/acpi/sleep.c | 38 ++++++++++++++++++++++++++++++++++
>> drivers/idle/intel_idle.c | 10 +++++++++
>> include/linux/suspend.h | 24 +++++++++++++++++++++
>> 4 files changed, 93 insertions(+), 25 deletions(-)
>>
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index c2ffd84..3ab0963 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -41,7 +41,7 @@
>> #include <linux/clockchips.h>
>> #include <linux/cpuidle.h>
>> #include <linux/irqflags.h>
>> -
>> +#include <linux/suspend.h>
>> /*
>> * Include the apic definitions for x86 to have the APIC timer related defines
>> * available also for UP (on SMP it gets magically included via linux/smp.h).
>> @@ -221,10 +221,6 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr,
>>
>> #endif
>>
>> -/*
>> - * Suspend / resume control
>> - */
>> -static int acpi_idle_suspend;
>> static u32 saved_bm_rld;
>>
>> static void acpi_idle_bm_rld_save(void)
>> @@ -243,21 +239,13 @@ static void acpi_idle_bm_rld_restore(void)
>>
>> int acpi_processor_suspend(struct acpi_device * device, pm_message_t state)
>> {
>> - if (acpi_idle_suspend == 1)
>> - return 0;
>> -
>> acpi_idle_bm_rld_save();
>> - acpi_idle_suspend = 1;
>> return 0;
>> }
>
> So, this seems to be on top of the "PM / ACPI: Fix suspend/resume regression
> caused by cpuidle" patch, right?
>
This patch is on top of the PM/ACPI fix which is posted in the below link
http://marc.info/?l=linux-pm&m=133958534231884&w=2 and adhering to the
suggestions posted in reply to it.
Very sorry about the patch not being applied on the latest
linux-pm/linux-next tree.If this code structure is agreed upon,
I will post the patch as relevant to the latest linux-pm/linux-next tree.
>> int acpi_processor_resume(struct acpi_device * device)
>> {
>> - if (acpi_idle_suspend == 0)
>> - return 0;
>> -
>> acpi_idle_bm_rld_restore();
>> - acpi_idle_suspend = 0;
>> return 0;
>> }
>>
>> @@ -762,11 +750,13 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
>> return -EINVAL;
>>
>> local_irq_disable();
>> -
>> - if (acpi_idle_suspend) {
>> + /*
>> + * Do not enter idle states if you are in suspend path
>> + */
>> + if (in_suspend_path()) {
>
> This is more than ugly. Can't you simply use the acpi_idle_suspend _variable_
> here?
>
> Besides, why the name of the variable is acpi_idle_suspend, if it is used
> by the Intel driver too?
>
The acpi_idle_suspend is defined in ACPI specific code.So if ACPI
is commented out in the configs ,then intel_idle driver cannot reference
acpi_idle_suspend.
Yes the variable can be named better.I made an attempt to retain the variable
name from the PM/ACPI patch.This will be corrected.
>> local_irq_enable();
>> cpu_relax();
>> - return -EINVAL;
>> + return -EBUSY;
>
> That change is made by the "PM / ACPI: Fix suspend/resume regression
> caused by cpuidle" patch already.
This confusion,which i regret, has been addressed above.
>
>> }
>>
>> lapic_timer_state_broadcast(pr, cx, 1);
>> @@ -838,10 +828,13 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
>>
>> local_irq_disable();
>>
>> - if (acpi_idle_suspend) {
>> + /*
>> + * Do not enter idle states if you are in suspend path
>> + */
>> + if (in_suspend_path()) {
>> local_irq_enable();
>> cpu_relax();
>> - return -EINVAL;
>> + return -EBUSY;
>> }
>>
>> if (cx->entry_method != ACPI_CSTATE_FFH) {
>> @@ -922,12 +915,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>> if (unlikely(!pr))
>> return -EINVAL;
>>
>> - if (acpi_idle_suspend) {
>> - if (irqs_disabled())
>> - local_irq_enable();
>> - cpu_relax();
>> - return -EINVAL;
>> - }
>
> The last version of the "PM / ACPI: Fix suspend/resume regression
> caused by cpuidle" patch didn't contain this hunk.
>
Addressed above.
>>
>> if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
>> if (drv->safe_state_index >= 0) {
>> @@ -935,13 +922,22 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>> drv, drv->safe_state_index);
>> } else {
>> local_irq_disable();
>> - acpi_safe_halt();
>> + if (!(in_suspend_path()))
>> + acpi_safe_halt();
>> local_irq_enable();
>> return -EINVAL;
>> }
>> }
>>
>> local_irq_disable();
>> + /*
>> + * Do not enter idle states if you are in suspend path
>> + */
>> + if (in_suspend_path()) {
>> + local_irq_enable();
>> + cpu_relax();
>> + return -EBUSY;
>> + }
>>
>> if (cx->entry_method != ACPI_CSTATE_FFH) {
>> current_thread_info()->status &= ~TS_POLLING;
>> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
>> index 8856102..4ec77db 100644
>> --- a/drivers/acpi/sleep.c
>> +++ b/drivers/acpi/sleep.c
>> @@ -28,6 +28,11 @@
>> #include "internal.h"
>> #include "sleep.h"
>>
>> +/*
>> + * Suspend/Resume control
>> + */
>> +int acpi_idle_suspend;
>> +
>> u8 wake_sleep_flags = ACPI_NO_OPTIONAL_METHODS;
>> static unsigned int gts, bfs;
>> static int set_param_wake_flag(const char *val, struct kernel_param *kp)
>> @@ -905,6 +910,36 @@ static void __init acpi_gts_bfs_check(void)
>> }
>> }
>>
>> +/**
>> + * cpuidle_pm_callback - On some bios, resume hangs
>> + * if idle states are entered during suspend.
>> + *
>> + * acpi_idle_suspend is used by the x86 idle drivers
>> + * to decide whether to go into idle states or not.
>> + */
>> +static int
>> +cpuidle_pm_callback(struct notifier_block *nb,
>> + unsigned long action, void *ptr)
>> +{
>> + switch (action) {
>> +
>> + case PM_SUSPEND_PREPARE:
>> + if (acpi_idle_suspend == 0)
>> + acpi_idle_suspend = 1;
>> + break;
>> +
>> + case PM_POST_SUSPEND:
>> + if (acpi_idle_suspend == 1)
>> + acpi_idle_suspend = 0;
>> + break;
>> +
>> + default:
>> + return NOTIFY_DONE;
>> + }
>> +
>> + return NOTIFY_OK;
>> +}
>
> Why don't you put this notifier into cpuidle instead?
cpuidle is an architecture independent part of the kernel code.Since
this patch aims at x86 architecture in specific,I considered it
inappropriate.
In addition to this,suspend happens on x86 only if ACPI is configured.
Therefore it seemed right to put the callback in ACPI specific code
which deals with ACPI sleep support.
>
>> +
>> int __init acpi_sleep_init(void)
>> {
>> acpi_status status;
>> @@ -932,6 +967,9 @@ int __init acpi_sleep_init(void)
>>
>> suspend_set_ops(old_suspend_ordering ?
>> &acpi_suspend_ops_old : &acpi_suspend_ops);
>> +
>> + pm_notifier(cpuidle_pm_callback, 0);
>> +
>> #endif
>>
>> #ifdef CONFIG_HIBERNATION
>> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
>> index d0f59c3..a595207 100644
>> --- a/drivers/idle/intel_idle.c
>> +++ b/drivers/idle/intel_idle.c
>> @@ -65,6 +65,7 @@
>> #include <asm/cpu_device_id.h>
>> #include <asm/mwait.h>
>> #include <asm/msr.h>
>> +#include <linux/suspend.h>
>>
>> #define INTEL_IDLE_VERSION "0.4"
>> #define PREFIX "intel_idle: "
>> @@ -255,6 +256,15 @@ static int intel_idle(struct cpuidle_device *dev,
>> cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
>>
>> /*
>> + * Do not enter idle states if you are in suspend path
>> + */
>> + if (in_suspend_path()) {
>> + local_irq_enable();
>> + cpu_relax();
>> + return -EBUSY;
>> + }
>> +
>> + /*
>> * leave_mm() to avoid costly and often unnecessary wakeups
>> * for flushing the user TLB's associated with the active mm.
>> */
>> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
>> index 0c808d7..eb96670 100644
>> --- a/include/linux/suspend.h
>> +++ b/include/linux/suspend.h
>> @@ -38,6 +38,30 @@ typedef int __bitwise suspend_state_t;
>> #define PM_SUSPEND_MEM ((__force suspend_state_t) 3)
>> #define PM_SUSPEND_MAX ((__force suspend_state_t) 4)
>>
>> +extern int acpi_idle_suspend;
>> +
>> +/**
>> + * in_suspend_path - X86 idle drivers make a call
>> + * to this function before entering idle states.
>> + *
>> + * Entering idle states is prevented if it is in suspend
>> + * path.
>> + */
>> +#ifdef CONFIG_ACPI
>
> Why #ifdef CONFIG_ACPI?
As mentioned above suspend happens on x86 only if ACPI is configured.
The setting of acpi_idle_suspend occurs in ACPI specific part of the code,
which will not be present if ACPI is configured out. Hence this check.
>
>> +static inline int in_suspend_path(void)
>> +{
>> + if (acpi_idle_suspend == 1)
>> + return 1;
>> + else
>> + return 0;
>> +}
>> +#else
>> +static inline int in_suspend_path(void)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> enum suspend_stat_step {
>> SUSPEND_FREEZE = 1,
>> SUSPEND_PREPARE,
>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Regards
Preeti
^ permalink raw reply
* Re: Cpuidle drivers, Suspend : Fix suspend/resume hang with intel_idle driver
From: Rafael J. Wysocki @ 2012-06-28 19:11 UTC (permalink / raw)
To: preeti
Cc: Linux PM mailing list, Dave Hansen, linux-acpi, Srivatsa.S.Bhat,
linux-pm
In-Reply-To: <4FEC84A4.1000605@linux.vnet.ibm.com>
On Thursday, June 28, 2012, preeti wrote:
> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
> > On Thursday, June 28, 2012, preeti wrote:
> >>
> >> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
[...]
> cpuidle is an architecture independent part of the kernel code.Since
> this patch aims at x86 architecture in specific,I considered it
> inappropriate.
>
> In addition to this,suspend happens on x86 only if ACPI is configured.
But that is not required for intel_idle, so if it hangs with intel_idle,
then it is not dependent on ACPI after all.
> Therefore it seemed right to put the callback in ACPI specific code
> which deals with ACPI sleep support.
I wonder if we can address this issue correctly. That is, in a non-racy
way and in a better place.
First, I really don't think it is necessary to "suspend" cpuidle (be it
ACPI or any other) when device drivers' suspend routines are being
executed (which also is racy, because the cpuidle "suspend" may be running
concurrently with cpuidle on another CPU) or earlier. We really may want
to disable the deeper C-states when we're about to execute
suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
we've run dpm_suspend_end() successfully.
So, it seems, it might be a good idea to place a cpuidle driver suspend
(and/or hibernation) hook at the end of dpm_suspend_end() and make ACPI
and intel_idle drivers implement that hook.
That would be much more deterministic than your patch I think.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v2] acpi: intel_idle : break dependency between modules
From: Rafael J. Wysocki @ 2012-06-28 19:24 UTC (permalink / raw)
To: Daniel Lezcano, lenb
Cc: trenn, srivatsa.bhat, deepthi, linux-acpi, linux-pm, linux-pm,
x86, linux-kernel, linaro-dev
In-Reply-To: <1340873202-2476-1-git-send-email-daniel.lezcano@linaro.org>
On Thursday, June 28, 2012, Daniel Lezcano wrote:
> When the system is booted with some cpus offline, the idle
> driver is not initialized. When a cpu is set online, the
> acpi code call the intel idle init function. Unfortunately
> this code introduce a dependency between intel_idle and acpi.
>
> This patch is intended to remove this dependency by using the
> notifier of intel_idle. This patch has the benefit of
> encapsulating the intel_idle driver and remove some exported
> functions.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
This one looks good to me too.
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Len, are you going to take it?
Rafael
> ---
> drivers/acpi/processor_driver.c | 7 ------
> drivers/idle/intel_idle.c | 41 +++++++++++++++++++++++++-------------
> include/linux/cpuidle.h | 7 ------
> 3 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 0734086..8648b29 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -427,18 +427,11 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb,
> * 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);
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index d0f59c3..fe95d54 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -96,6 +96,7 @@ static const struct idle_cpu *icpu;
> static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
> static int intel_idle(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index);
> +static int intel_idle_cpu_init(int cpu);
>
> static struct cpuidle_state *cpuidle_state_table;
>
> @@ -302,22 +303,35 @@ static void __setup_broadcast_timer(void *arg)
> clockevents_notify(reason, &cpu);
> }
>
> -static int setup_broadcast_cpuhp_notify(struct notifier_block *n,
> - unsigned long action, void *hcpu)
> +static int cpu_hotplug_notify(struct notifier_block *n,
> + unsigned long action, void *hcpu)
> {
> int hotcpu = (unsigned long)hcpu;
> + struct cpuidle_device *dev;
>
> switch (action & 0xf) {
> case CPU_ONLINE:
> - smp_call_function_single(hotcpu, __setup_broadcast_timer,
> - (void *)true, 1);
> +
> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
> + smp_call_function_single(hotcpu, __setup_broadcast_timer,
> + (void *)true, 1);
> +
> + /*
> + * Some systems can hotplug a cpu at runtime after
> + * the kernel has booted, we have to initialize the
> + * driver in this case
> + */
> + dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
> + if (!dev->registered)
> + intel_idle_cpu_init(hotcpu);
> +
> break;
> }
> return NOTIFY_OK;
> }
>
> -static struct notifier_block setup_broadcast_notifier = {
> - .notifier_call = setup_broadcast_cpuhp_notify,
> +static struct notifier_block cpu_hotplug_notifier = {
> + .notifier_call = cpu_hotplug_notify,
> };
>
> static void auto_demotion_disable(void *dummy)
> @@ -405,10 +419,10 @@ static int intel_idle_probe(void)
>
> if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */
> lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
> - else {
> + else
> on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
> - register_cpu_notifier(&setup_broadcast_notifier);
> - }
> +
> + register_cpu_notifier(&cpu_hotplug_notifier);
>
> pr_debug(PREFIX "v" INTEL_IDLE_VERSION
> " model 0x%X\n", boot_cpu_data.x86_model);
> @@ -494,7 +508,7 @@ static int intel_idle_cpuidle_driver_init(void)
> * allocate, initialize, register cpuidle_devices
> * @cpu: cpu/core to initialize
> */
> -int intel_idle_cpu_init(int cpu)
> +static int intel_idle_cpu_init(int cpu)
> {
> int cstate;
> struct cpuidle_device *dev;
> @@ -539,7 +553,6 @@ int intel_idle_cpu_init(int cpu)
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(intel_idle_cpu_init);
>
> static int __init intel_idle_init(void)
> {
> @@ -581,10 +594,10 @@ static void __exit intel_idle_exit(void)
> intel_idle_cpuidle_devices_uninit();
> cpuidle_unregister_driver(&intel_idle_driver);
>
> - if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
> +
> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
> on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
> - unregister_cpu_notifier(&setup_broadcast_notifier);
> - }
> + unregister_cpu_notifier(&cpu_hotplug_notifier);
>
> return;
> }
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 5ab7183..66d7e0d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -213,14 +213,7 @@ struct cpuidle_governor {
> extern int cpuidle_register_governor(struct cpuidle_governor *gov);
> extern void cpuidle_unregister_governor(struct cpuidle_governor *gov);
>
> -#ifdef CONFIG_INTEL_IDLE
> -extern int intel_idle_cpu_init(int cpu);
> #else
> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
> -#endif
> -
> -#else
> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
>
> static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
> {return 0;}
>
^ permalink raw reply
* Re: [PATCH -v4 3/6] PM: PM notifier error injection module
From: Rafael J. Wysocki @ 2012-06-28 21:45 UTC (permalink / raw)
To: Akinobu Mita; +Cc: akpm, linux-kernel, linux-pm
In-Reply-To: <1340463502-15341-4-git-send-email-akinobu.mita@gmail.com>
On Saturday, June 23, 2012, Akinobu Mita wrote:
> This provides the ability to inject artifical errors to PM notifier chain
> callbacks. It is controlled through debugfs interface under
> /sys/kernel/debug/notifier-error-inject/pm
>
> Each of the files in "error" directory represents an event which can be
> failed and contains the error code. If the notifier call chain should
> be failed with some events notified, write the error code to the files.
>
> If the notifier call chain should be failed with some events notified,
> write the error code to "actions/<notifier event>/error".
>
> Example: Inject PM suspend error (-12 = -ENOMEM)
>
> # cd /sys/kernel/debug/notifier-error-inject/pm
> # echo -12 > actions/PM_SUSPEND_PREPARE/error
> # echo mem > /sys/power/state
> bash: echo: write error: Cannot allocate memory
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-pm@lists.linux-foundation.org
OK, I have no objections.
Thanks,
Rafael
> ---
> * v4
> - update modules to follow new interface
>
> lib/Kconfig.debug | 24 ++++++++++++++++++++
> lib/Makefile | 1 +
> lib/pm-notifier-error-inject.c | 49 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 74 insertions(+)
> create mode 100644 lib/pm-notifier-error-inject.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index be0c197..246cea6 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1119,6 +1119,30 @@ config CPU_NOTIFIER_ERROR_INJECT
>
> If unsure, say N.
>
> +config PM_NOTIFIER_ERROR_INJECT
> + tristate "PM notifier error injection module"
> + depends on PM && NOTIFIER_ERROR_INJECTION
> + default m if PM_DEBUG
> + help
> + This option provides the ability to inject artifical errors to
> + PM notifier chain callbacks. It is controlled through debugfs
> + interface /sys/kernel/debug/notifier-error-inject/pm
> +
> + If the notifier call chain should be failed with some events
> + notified, write the error code to "actions/<notifier event>/error".
> +
> + Example: Inject PM suspend error (-12 = -ENOMEM)
> +
> + # cd /sys/kernel/debug/notifier-error-inject/pm/
> + # echo -12 > actions/PM_SUSPEND_PREPARE/error
> + # echo mem > /sys/power/state
> + bash: echo: write error: Cannot allocate memory
> +
> + To compile this code as a module, choose M here: the module will
> + be called pm-notifier-error-inject.
> +
> + If unsure, say N.
> +
> config FAULT_INJECTION
> bool "Fault-injection framework"
> depends on DEBUG_KERNEL
> diff --git a/lib/Makefile b/lib/Makefile
> index 23fba9e..230a949 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -92,6 +92,7 @@ obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o
> obj-$(CONFIG_FAULT_INJECTION) += fault-inject.o
> obj-$(CONFIG_NOTIFIER_ERROR_INJECTION) += notifier-error-inject.o
> obj-$(CONFIG_CPU_NOTIFIER_ERROR_INJECT) += cpu-notifier-error-inject.o
> +obj-$(CONFIG_PM_NOTIFIER_ERROR_INJECT) += pm-notifier-error-inject.o
>
> lib-$(CONFIG_GENERIC_BUG) += bug.o
>
> diff --git a/lib/pm-notifier-error-inject.c b/lib/pm-notifier-error-inject.c
> new file mode 100644
> index 0000000..c094b2d
> --- /dev/null
> +++ b/lib/pm-notifier-error-inject.c
> @@ -0,0 +1,49 @@
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/suspend.h>
> +
> +#include "notifier-error-inject.h"
> +
> +static int priority;
> +module_param(priority, int, 0);
> +MODULE_PARM_DESC(priority, "specify PM notifier priority");
> +
> +static struct notifier_err_inject pm_notifier_err_inject = {
> + .actions = {
> + { NOTIFIER_ERR_INJECT_ACTION(PM_HIBERNATION_PREPARE) },
> + { NOTIFIER_ERR_INJECT_ACTION(PM_SUSPEND_PREPARE) },
> + { NOTIFIER_ERR_INJECT_ACTION(PM_RESTORE_PREPARE) },
> + {}
> + }
> +};
> +
> +static struct dentry *dir;
> +
> +static int err_inject_init(void)
> +{
> + int err;
> +
> + dir = notifier_err_inject_init("pm", notifier_err_inject_dir,
> + &pm_notifier_err_inject, priority);
> + if (IS_ERR(dir))
> + return PTR_ERR(dir);
> +
> + err = register_pm_notifier(&pm_notifier_err_inject.nb);
> + if (err)
> + debugfs_remove_recursive(dir);
> +
> + return err;
> +}
> +
> +static void err_inject_exit(void)
> +{
> + unregister_pm_notifier(&pm_notifier_err_inject.nb);
> + debugfs_remove_recursive(dir);
> +}
> +
> +module_init(err_inject_init);
> +module_exit(err_inject_exit);
> +
> +MODULE_DESCRIPTION("PM notifier error injection module");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Akinobu Mita <akinobu.mita@gmail.com>");
>
^ permalink raw reply
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: preeti @ 2012-06-29 4:11 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Linux PM mailing list, linux-pm, linux-acpi,
Srivatsa.S.Bhat, Deepthi Dharwar
In-Reply-To: <201206282111.03992.rjw@sisk.pl>
On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
> On Thursday, June 28, 2012, preeti wrote:
>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
>>> On Thursday, June 28, 2012, preeti wrote:
>>>>
>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> [...]
>> cpuidle is an architecture independent part of the kernel code.Since
>> this patch aims at x86 architecture in specific,I considered it
>> inappropriate.
>>
>> In addition to this,suspend happens on x86 only if ACPI is configured.
>
> But that is not required for intel_idle, so if it hangs with intel_idle,
> then it is not dependent on ACPI after all.
True intel_idle does not need ACPI to be configured,but that also means
that the kernel will not provide you the means to suspend.There is no
question of resume hang here at all as you cannot suspend in the first
place.
The issue is when ACPI is configured,and intel_idle is chosen to be the
cpuidle driver.In this situation when the user suspends,cpus can enter
deep sleep states as intel_idle driver does not prevent then from doing so.
This is when resume hangs.
>
>> Therefore it seemed right to put the callback in ACPI specific code
>> which deals with ACPI sleep support.
>
> I wonder if we can address this issue correctly. That is, in a non-racy
> way and in a better place.
>
> First, I really don't think it is necessary to "suspend" cpuidle (be it
> ACPI or any other) when device drivers' suspend routines are being
> executed (which also is racy, because the cpuidle "suspend" may be running
> concurrently with cpuidle on another CPU) or earlier. We really may want
> to disable the deeper C-states when we're about to execute
> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
> we've run dpm_suspend_end() successfully.
The commit "ACPI:disable lower idle C-states across suspend/resume"
states that device_suspend() calls ACPI suspend functions which cause
side effects on the lower idle C-states.This means we need to disable
entry into deeper C-states even before dpm_suspend_start(),but how much
before?
The commit answers this too.It says removing the functionality of
entering deep C-states before suspend removed the side effects.Besides,
the commit title says'across suspend/resume'.So I think to address the
resume hang effectively,it is desirable to disable entry into deeper
C-states during suspend_prepare operations.
>
> So, it seems, it might be a good idea to place a cpuidle driver suspend
> (and/or hibernation) hook at the end of dpm_suspend_end() and make ACPI
> and intel_idle drivers implement that hook.
>
Dont you think this patch is meant to fix a very ACPI specific problem?
Which is why I think the call backs or any hook meant to fix this should
go into ACPI specific code.
Else it will seem irrelevant to all other architectures that implement
suspend.
> That would be much more deterministic than your patch I think.
>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Regards
Preeti
^ permalink raw reply
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: preeti @ 2012-06-29 5:26 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Linux PM mailing list, linux-pm, linux-acpi,
Srivatsa.S.Bhat, Deepthi Dharwar
In-Reply-To: <4FED2AF2.4080900@linux.vnet.ibm.com>
On 06/29/2012 09:41 AM, preeti wrote:
> On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
>> On Thursday, June 28, 2012, preeti wrote:
>>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>
>>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>> [...]
>>> cpuidle is an architecture independent part of the kernel code.Since
>>> this patch aims at x86 architecture in specific,I considered it
>>> inappropriate.
>>>
>>> In addition to this,suspend happens on x86 only if ACPI is configured.
>>
>> But that is not required for intel_idle, so if it hangs with intel_idle,
>> then it is not dependent on ACPI after all.
> True intel_idle does not need ACPI to be configured,but that also means
> that the kernel will not provide you the means to suspend.There is no
> question of resume hang here at all as you cannot suspend in the first
> place.
>
> The issue is when ACPI is configured,and intel_idle is chosen to be the
> cpuidle driver.In this situation when the user suspends,cpus can enter
> deep sleep states as intel_idle driver does not prevent then from doing so.
> This is when resume hangs.
>>
>>> Therefore it seemed right to put the callback in ACPI specific code
>>> which deals with ACPI sleep support.
>>
>> I wonder if we can address this issue correctly. That is, in a non-racy
>> way and in a better place.
>>
>> First, I really don't think it is necessary to "suspend" cpuidle (be it
>> ACPI or any other) when device drivers' suspend routines are being
>> executed (which also is racy, because the cpuidle "suspend" may be running
>> concurrently with cpuidle on another CPU) or earlier. We really may want
>> to disable the deeper C-states when we're about to execute
>> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
>> we've run dpm_suspend_end() successfully.
>
> The commit "ACPI:disable lower idle C-states across suspend/resume"
> states that device_suspend() calls ACPI suspend functions which cause
> side effects on the lower idle C-states.This means we need to disable
> entry into deeper C-states even before dpm_suspend_start(),but how much
> before?
>
> The commit answers this too.It says removing the functionality of
> entering deep C-states before suspend removed the side effects.Besides,
> the commit title says'across suspend/resume'.So I think to address the
> resume hang effectively,it is desirable to disable entry into deeper
> C-states during suspend_prepare operations.
To clarify this further,since we take action upon PM_SUSPEND_PREPARE
notification,which is called before suspend begins,we avoid race
condition between suspend operations and disabling entry into deeper
c-states altogether.
>>
>> So, it seems, it might be a good idea to place a cpuidle driver suspend
>> (and/or hibernation) hook at the end of dpm_suspend_end() and make ACPI
>> and intel_idle drivers implement that hook.
>>
> Dont you think this patch is meant to fix a very ACPI specific problem?
> Which is why I think the call backs or any hook meant to fix this should
> go into ACPI specific code.
> Else it will seem irrelevant to all other architectures that implement
> suspend.
>> That would be much more deterministic than your patch I think.
>>
>> Thanks,
>> Rafael
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> Regards
> Preeti
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v2] acpi: intel_idle : break dependency between modules
From: Daniel Lezcano @ 2012-06-29 8:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linaro-dev, linux-pm, x86, linux-kernel, linux-acpi,
srivatsa.bhat, Colin Cross, linux-pm
In-Reply-To: <201206282124.09857.rjw@sisk.pl>
On 06/28/2012 09:24 PM, Rafael J. Wysocki wrote:
> On Thursday, June 28, 2012, Daniel Lezcano wrote:
>> When the system is booted with some cpus offline, the idle
>> driver is not initialized. When a cpu is set online, the
>> acpi code call the intel idle init function. Unfortunately
>> this code introduce a dependency between intel_idle and acpi.
>>
>> This patch is intended to remove this dependency by using the
>> notifier of intel_idle. This patch has the benefit of
>> encapsulating the intel_idle driver and remove some exported
>> functions.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> This one looks good to me too.
>
> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Thanks for the review Rafael.
> Len, are you going to take it?
Rafael, Len,
After the discussion [1], I put in place a tree at:
ssh://git.linaro.org/srv/git.linaro.org/git/people/dlezcano/cpuidle-next.git
#cpuidle-next
I proposed this tree to group the cpuidle modifications and prevent the
last minutes conflict when Len will apply them.
I also included the tree into linux-next for wider testing.
If you want I can take this patch even if it is related to acpi also and
in the future if you agree, Len or you could pull from there.
Thanks
-- Daniel
[1] https://lkml.org/lkml/2012/6/18/113
>
> Rafael
>
>
>> ---
>> drivers/acpi/processor_driver.c | 7 ------
>> drivers/idle/intel_idle.c | 41 +++++++++++++++++++++++++-------------
>> include/linux/cpuidle.h | 7 ------
>> 3 files changed, 27 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 0734086..8648b29 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -427,18 +427,11 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb,
>> * 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);
>> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
>> index d0f59c3..fe95d54 100644
>> --- a/drivers/idle/intel_idle.c
>> +++ b/drivers/idle/intel_idle.c
>> @@ -96,6 +96,7 @@ static const struct idle_cpu *icpu;
>> static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
>> static int intel_idle(struct cpuidle_device *dev,
>> struct cpuidle_driver *drv, int index);
>> +static int intel_idle_cpu_init(int cpu);
>>
>> static struct cpuidle_state *cpuidle_state_table;
>>
>> @@ -302,22 +303,35 @@ static void __setup_broadcast_timer(void *arg)
>> clockevents_notify(reason, &cpu);
>> }
>>
>> -static int setup_broadcast_cpuhp_notify(struct notifier_block *n,
>> - unsigned long action, void *hcpu)
>> +static int cpu_hotplug_notify(struct notifier_block *n,
>> + unsigned long action, void *hcpu)
>> {
>> int hotcpu = (unsigned long)hcpu;
>> + struct cpuidle_device *dev;
>>
>> switch (action & 0xf) {
>> case CPU_ONLINE:
>> - smp_call_function_single(hotcpu, __setup_broadcast_timer,
>> - (void *)true, 1);
>> +
>> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
>> + smp_call_function_single(hotcpu, __setup_broadcast_timer,
>> + (void *)true, 1);
>> +
>> + /*
>> + * Some systems can hotplug a cpu at runtime after
>> + * the kernel has booted, we have to initialize the
>> + * driver in this case
>> + */
>> + dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
>> + if (!dev->registered)
>> + intel_idle_cpu_init(hotcpu);
>> +
>> break;
>> }
>> return NOTIFY_OK;
>> }
>>
>> -static struct notifier_block setup_broadcast_notifier = {
>> - .notifier_call = setup_broadcast_cpuhp_notify,
>> +static struct notifier_block cpu_hotplug_notifier = {
>> + .notifier_call = cpu_hotplug_notify,
>> };
>>
>> static void auto_demotion_disable(void *dummy)
>> @@ -405,10 +419,10 @@ static int intel_idle_probe(void)
>>
>> if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */
>> lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
>> - else {
>> + else
>> on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
>> - register_cpu_notifier(&setup_broadcast_notifier);
>> - }
>> +
>> + register_cpu_notifier(&cpu_hotplug_notifier);
>>
>> pr_debug(PREFIX "v" INTEL_IDLE_VERSION
>> " model 0x%X\n", boot_cpu_data.x86_model);
>> @@ -494,7 +508,7 @@ static int intel_idle_cpuidle_driver_init(void)
>> * allocate, initialize, register cpuidle_devices
>> * @cpu: cpu/core to initialize
>> */
>> -int intel_idle_cpu_init(int cpu)
>> +static int intel_idle_cpu_init(int cpu)
>> {
>> int cstate;
>> struct cpuidle_device *dev;
>> @@ -539,7 +553,6 @@ int intel_idle_cpu_init(int cpu)
>>
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(intel_idle_cpu_init);
>>
>> static int __init intel_idle_init(void)
>> {
>> @@ -581,10 +594,10 @@ static void __exit intel_idle_exit(void)
>> intel_idle_cpuidle_devices_uninit();
>> cpuidle_unregister_driver(&intel_idle_driver);
>>
>> - if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
>> +
>> + if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
>> on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
>> - unregister_cpu_notifier(&setup_broadcast_notifier);
>> - }
>> + unregister_cpu_notifier(&cpu_hotplug_notifier);
>>
>> return;
>> }
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 5ab7183..66d7e0d 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -213,14 +213,7 @@ struct cpuidle_governor {
>> extern int cpuidle_register_governor(struct cpuidle_governor *gov);
>> extern void cpuidle_unregister_governor(struct cpuidle_governor *gov);
>>
>> -#ifdef CONFIG_INTEL_IDLE
>> -extern int intel_idle_cpu_init(int cpu);
>> #else
>> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
>> -#endif
>> -
>> -#else
>> -static inline int intel_idle_cpu_init(int cpu) { return -1; }
>>
>> static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>> {return 0;}
>>
>
--
<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
* [PATCH] Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: preeti @ 2012-06-29 8:53 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Linux PM mailing list, linux-pm, linux-acpi,
Srivatsa.S.Bhat, Deepthi Dharwar
In-Reply-To: <4FED3CA0.3050909@linux.vnet.ibm.com>
From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Dave Hansen reported problems with suspend/resume when used
with intel_idle driver.More such problems were noticed.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/674075.
The reason for this could not be suspected till he reported
resume hang issue when used with acpi idle driver on his Lenovo-S10-3
Atom netbook.
The patch-[PM / ACPI: Fix suspend/resume regression caused by cpuidle
cleanup],fixed this issue by ensuring that acpi idle drivers prevent
cpus from going into deeper sleep states during suspend to prevent
resume hang on certain bios.
http://marc.info/?l=linux-pm&m=133958534231884&w=2
Commit b04e7bdb984e3b7f62fb7f44146a529f88cc7639
(ACPI: disable lower idle C-states across suspend/resume) throws
light on the resume hang issue on certain specific bios.
Also the following lines in drivers/idle/intel_idle.c suggest intel_idle
drivers should also ensure cpus are prevented from entering idle states
during suspend to avoid a resume hang.
/*
* Known limitations
* [..]
*
* ACPI has a .suspend hack to turn off deep c-states during suspend
* to avoid complications with the lapic timer workaround.
* Have not seen issues with suspend, but may need same workaround here.
*
*/
This patch aims at having this fix in a place common to both the idle
drivers.
Suspend is enabled only if ACPI is active on x86.Thus the setting of
acpi_idle_suspend during suspend is moved up to ACPI specific code with
both acpi and intel idle drivers checking if it is valid to enter deeper
idle states.The setting of acpi_idle_suspend is done via PM_SUSPEND_PREPARE
notifiers to avoid race conditions between processors entering idle states
and the ongoing process of suspend.
The check on acpi_idle_suspend is included in the most appropriate header
so as to be visible to both the idle drivers irrespective of the
different configurations.Even if ACPI is disabled intel idle drivers can
still carry out the acpi_idle_suspend check.
Reported-by: Dave Hansen <dave@linux.vnet.ibm.com>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Reviewed-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---
The patch has been applied on the latest linux-pm/linux-next
tree due to the confusion that was present earlier.Also
acpi_idle_suspend has been changed to idle_suspend.Rest of the
code structure is the same.
drivers/acpi/processor_idle.c | 22 +++++-----------------
drivers/acpi/sleep.c | 34 ++++++++++++++++++++++++++++++++++
drivers/idle/intel_idle.c | 7 +++++++
include/linux/suspend.h | 24 ++++++++++++++++++++++++
4 files changed, 70 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 47a8caa..d6704a7 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -41,7 +41,7 @@
#include <linux/clockchips.h>
#include <linux/cpuidle.h>
#include <linux/irqflags.h>
-
+#include <linux/suspend.h>
/*
* Include the apic definitions for x86 to have the APIC timer related
defines
* available also for UP (on SMP it gets magically included via
linux/smp.h).
@@ -221,10 +221,6 @@ static void lapic_timer_state_broadcast(struct
acpi_processor *pr,
#endif
-/*
- * Suspend / resume control
- */
-static int acpi_idle_suspend;
static u32 saved_bm_rld;
static void acpi_idle_bm_rld_save(void)
@@ -243,21 +239,13 @@ static void acpi_idle_bm_rld_restore(void)
int acpi_processor_suspend(struct acpi_device * device, pm_message_t state)
{
- if (acpi_idle_suspend == 1)
- return 0;
-
acpi_idle_bm_rld_save();
- acpi_idle_suspend = 1;
return 0;
}
int acpi_processor_resume(struct acpi_device * device)
{
- if (acpi_idle_suspend == 0)
- return 0;
-
acpi_idle_bm_rld_restore();
- acpi_idle_suspend = 0;
return 0;
}
@@ -763,7 +751,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device
*dev,
local_irq_disable();
- if (acpi_idle_suspend) {
+ if (in_suspend_path()) {
local_irq_enable();
cpu_relax();
return -EBUSY;
@@ -838,7 +826,7 @@ static int acpi_idle_enter_simple(struct
cpuidle_device *dev,
local_irq_disable();
- if (acpi_idle_suspend) {
+ if (in_suspend_path()) {
local_irq_enable();
cpu_relax();
return -EBUSY;
@@ -928,7 +916,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device
*dev,
drv, drv->safe_state_index);
} else {
local_irq_disable();
- if (!acpi_idle_suspend)
+ if (!(in_suspend_path()))
acpi_safe_halt();
local_irq_enable();
return -EBUSY;
@@ -937,7 +925,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device
*dev,
local_irq_disable();
- if (acpi_idle_suspend) {
+ if (in_suspend_path()) {
local_irq_enable();
cpu_relax();
return -EBUSY;
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 8856102..c052e7e 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -28,6 +28,9 @@
#include "internal.h"
#include "sleep.h"
+/* Suspend/Resume control */
+int idle_suspend;
+
u8 wake_sleep_flags = ACPI_NO_OPTIONAL_METHODS;
static unsigned int gts, bfs;
static int set_param_wake_flag(const char *val, struct kernel_param *kp)
@@ -904,6 +907,36 @@ static void __init acpi_gts_bfs_check(void)
"please notify linux-acpi@vger.kernel.org\n");
}
}
+/**
+ * cpuidle_pm_callback - On some bios, resume hangs
+ * if idle states are entered during suspend.
+ *
+ * acpi_idle_suspend is used by the x86 idle drivers
+ * to decide whether to go into idle states or not.
+ */
+static int
+cpuidle_pm_callback(struct notifier_block *nb,
+ unsigned long action, void *ptr)
+{
+ switch (action) {
+
+ case PM_SUSPEND_PREPARE:
+ if (idle_suspend == 0)
+ idle_suspend = 1;
+ break;
+
+ case PM_POST_SUSPEND:
+ if (idle_suspend == 1)
+ idle_suspend = 0;
+ break;
+
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
int __init acpi_sleep_init(void)
{
@@ -932,6 +965,7 @@ int __init acpi_sleep_init(void)
suspend_set_ops(old_suspend_ordering ?
&acpi_suspend_ops_old : &acpi_suspend_ops);
+ pm_notifier(cpuidle_pm_callback, 0);
#endif
#ifdef CONFIG_HIBERNATION
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index d0f59c3..10555f8 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -62,6 +62,7 @@
#include <linux/notifier.h>
#include <linux/cpu.h>
#include <linux/module.h>
+#include <linux/suspend.h>
#include <asm/cpu_device_id.h>
#include <asm/mwait.h>
#include <asm/msr.h>
@@ -254,6 +255,12 @@ static int intel_idle(struct cpuidle_device *dev,
cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
+ if (in_suspend_path()) {
+ local_irq_enable();
+ cpu_relax();
+ return -EBUSY;
+ }
+
/*
* leave_mm() to avoid costly and often unnecessary wakeups
* for flushing the user TLB's associated with the active mm.
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 0c808d7..8d39a1a 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -38,6 +38,30 @@ typedef int __bitwise suspend_state_t;
#define PM_SUSPEND_MEM ((__force suspend_state_t) 3)
#define PM_SUSPEND_MAX ((__force suspend_state_t) 4)
+extern int idle_suspend;
+
+/**
+ * in_suspend_path - X86 idle drivers make a call
+ * to this function before entering idle states.
+ *
+ * Entering idle states is prevented if it is in suspend
+ * path.
+ */
+#ifdef CONFIG_ACPI
+static inline int in_suspend_path(void)
+{
+ if (idle_suspend == 1)
+ return 1;
+ else
+ return 0;
+}
+#else
+static inline int in_suspend_path(void)
+{
+ return 0;
+}
+#endif
+
enum suspend_stat_step {
SUSPEND_FREEZE = 1,
SUSPEND_PREPARE,
^ permalink raw reply related
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: Rafael J. Wysocki @ 2012-06-29 22:11 UTC (permalink / raw)
To: preeti
Cc: Dave Hansen, Linux PM mailing list, linux-pm, linux-acpi,
Srivatsa.S.Bhat, Deepthi Dharwar
In-Reply-To: <4FED3CA0.3050909@linux.vnet.ibm.com>
On Friday, June 29, 2012, preeti wrote:
> On 06/29/2012 09:41 AM, preeti wrote:
> > On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
> >> On Thursday, June 28, 2012, preeti wrote:
> >>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
> >>>> On Thursday, June 28, 2012, preeti wrote:
> >>>>>
> >>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> >> [...]
> >>> cpuidle is an architecture independent part of the kernel code.Since
> >>> this patch aims at x86 architecture in specific,I considered it
> >>> inappropriate.
> >>>
> >>> In addition to this,suspend happens on x86 only if ACPI is configured.
> >>
> >> But that is not required for intel_idle, so if it hangs with intel_idle,
> >> then it is not dependent on ACPI after all.
> > True intel_idle does not need ACPI to be configured,but that also means
> > that the kernel will not provide you the means to suspend.There is no
> > question of resume hang here at all as you cannot suspend in the first
> > place.
> >
> > The issue is when ACPI is configured,and intel_idle is chosen to be the
> > cpuidle driver.In this situation when the user suspends,cpus can enter
> > deep sleep states as intel_idle driver does not prevent then from doing so.
> > This is when resume hangs.
> >>
> >>> Therefore it seemed right to put the callback in ACPI specific code
> >>> which deals with ACPI sleep support.
> >>
> >> I wonder if we can address this issue correctly. That is, in a non-racy
> >> way and in a better place.
> >>
> >> First, I really don't think it is necessary to "suspend" cpuidle (be it
> >> ACPI or any other) when device drivers' suspend routines are being
> >> executed (which also is racy, because the cpuidle "suspend" may be running
> >> concurrently with cpuidle on another CPU) or earlier. We really may want
> >> to disable the deeper C-states when we're about to execute
> >> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
> >> we've run dpm_suspend_end() successfully.
> >
> > The commit "ACPI:disable lower idle C-states across suspend/resume"
> > states that device_suspend() calls ACPI suspend functions which cause
> > side effects on the lower idle C-states.This means we need to disable
> > entry into deeper C-states even before dpm_suspend_start(),but how much
> > before?
> >
> > The commit answers this too.It says removing the functionality of
> > entering deep C-states before suspend removed the side effects.Besides,
> > the commit title says'across suspend/resume'.So I think to address the
> > resume hang effectively,it is desirable to disable entry into deeper
> > C-states during suspend_prepare operations.
>
> To clarify this further,since we take action upon PM_SUSPEND_PREPARE
> notification,which is called before suspend begins,we avoid race
> condition between suspend operations and disabling entry into deeper
> c-states altogether.
Well, what about races between disabling deeper C-states and cpuidle?
Rafael
^ permalink raw reply
* Re: [PATCH] Cpuidle drivers, Suspend : Fix suspend/resume hang with intel_idle driver
From: Rafael J. Wysocki @ 2012-06-29 22:13 UTC (permalink / raw)
To: preeti
Cc: Linux PM mailing list, Dave Hansen, linux-acpi, Srivatsa.S.Bhat,
linux-pm
In-Reply-To: <4FED6CED.2060908@linux.vnet.ibm.com>
On Friday, June 29, 2012, preeti wrote:
>
> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>
> Dave Hansen reported problems with suspend/resume when used
> with intel_idle driver.More such problems were noticed.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/674075.
>
> The reason for this could not be suspected till he reported
> resume hang issue when used with acpi idle driver on his Lenovo-S10-3
> Atom netbook.
>
> The patch-[PM / ACPI: Fix suspend/resume regression caused by cpuidle
> cleanup],fixed this issue by ensuring that acpi idle drivers prevent
> cpus from going into deeper sleep states during suspend to prevent
> resume hang on certain bios.
> http://marc.info/?l=linux-pm&m=133958534231884&w=2
>
> Commit b04e7bdb984e3b7f62fb7f44146a529f88cc7639
> (ACPI: disable lower idle C-states across suspend/resume) throws
> light on the resume hang issue on certain specific bios.
>
> Also the following lines in drivers/idle/intel_idle.c suggest intel_idle
> drivers should also ensure cpus are prevented from entering idle states
> during suspend to avoid a resume hang.
>
> /*
> * Known limitations
> * [..]
> *
> * ACPI has a .suspend hack to turn off deep c-states during suspend
> * to avoid complications with the lapic timer workaround.
> * Have not seen issues with suspend, but may need same workaround here.
> *
> */
> This patch aims at having this fix in a place common to both the idle
> drivers.
>
> Suspend is enabled only if ACPI is active on x86.Thus the setting of
> acpi_idle_suspend during suspend is moved up to ACPI specific code with
> both acpi and intel idle drivers checking if it is valid to enter deeper
> idle states.The setting of acpi_idle_suspend is done via PM_SUSPEND_PREPARE
> notifiers to avoid race conditions between processors entering idle states
> and the ongoing process of suspend.
>
> The check on acpi_idle_suspend is included in the most appropriate header
> so as to be visible to both the idle drivers irrespective of the
> different configurations.Even if ACPI is disabled intel idle drivers can
> still carry out the acpi_idle_suspend check.
>
> Reported-by: Dave Hansen <dave@linux.vnet.ibm.com>
> Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Reviewed-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
The patch didn't get any better as a result of modifying the changelog.
Please consider it as rejected.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v2] acpi: intel_idle : break dependency between modules
From: Rafael J. Wysocki @ 2012-06-29 22:27 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linaro-dev, linux-pm, x86, linux-kernel, linux-acpi,
srivatsa.bhat, Colin Cross, linux-pm
In-Reply-To: <4FED69BE.20707@linaro.org>
On Friday, June 29, 2012, Daniel Lezcano wrote:
> On 06/28/2012 09:24 PM, Rafael J. Wysocki wrote:
> > On Thursday, June 28, 2012, Daniel Lezcano wrote:
> >> When the system is booted with some cpus offline, the idle
> >> driver is not initialized. When a cpu is set online, the
> >> acpi code call the intel idle init function. Unfortunately
> >> this code introduce a dependency between intel_idle and acpi.
> >>
> >> This patch is intended to remove this dependency by using the
> >> notifier of intel_idle. This patch has the benefit of
> >> encapsulating the intel_idle driver and remove some exported
> >> functions.
> >>
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >
> > This one looks good to me too.
> >
> > Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> Thanks for the review Rafael.
>
> > Len, are you going to take it?
>
> Rafael, Len,
>
> After the discussion [1], I put in place a tree at:
>
> ssh://git.linaro.org/srv/git.linaro.org/git/people/dlezcano/cpuidle-next.git
> #cpuidle-next
>
> I proposed this tree to group the cpuidle modifications and prevent the
> last minutes conflict when Len will apply them.
>
> I also included the tree into linux-next for wider testing.
I can't speak for Len, but I'm not sure he'll like this.
Anyway, you seem to have the same material as Len in that tree, won't there
be any conflicts in linux-next?
Rafael
^ permalink raw reply
* ACPI & Power Management Patches for Linux 3.5-rc4
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: linux-kernel
Here are some bug fixes queued for 3.5-rc4
Please let me know if you see troubles with any of them.
thanks,
Len Brown, Intel Open Source Technology Center
^ permalink raw reply
* [PATCH 1/8] ACPI sysfs.c strlen fix
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: linux-kernel, Pavel Vasilyev, Len Brown
In-Reply-To: <1341032855-27139-1-git-send-email-lenb@kernel.org>
From: Pavel Vasilyev <pavel@pavlinux.ru>
Current code is ignoring the last character of "enable" and "disable"
in comparisons.
https://bugzilla.kernel.org/show_bug.cgi?id=33732
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/sysfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 9f66181..240a244 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -173,7 +173,7 @@ static int param_set_trace_state(const char *val, struct kernel_param *kp)
{
int result = 0;
- if (!strncmp(val, "enable", strlen("enable") - 1)) {
+ if (!strncmp(val, "enable", strlen("enable"))) {
result = acpi_debug_trace(trace_method_name, trace_debug_level,
trace_debug_layer, 0);
if (result)
@@ -181,7 +181,7 @@ static int param_set_trace_state(const char *val, struct kernel_param *kp)
goto exit;
}
- if (!strncmp(val, "disable", strlen("disable") - 1)) {
+ if (!strncmp(val, "disable", strlen("disable"))) {
int name = 0;
result = acpi_debug_trace((char *)&name, trace_debug_level,
trace_debug_layer, 0);
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 2/8] ACPI, x86: fix Dell M6600 ACPI reboot regression via DMI
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, x86, linux-kernel
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Zhang Rui <rui.zhang@intel.com>
Dell Precision M6600 is known to require PCI reboot, so add it to
the reboot blacklist in pci_reboot_dmi_table[].
https://bugzilla.kernel.org/show_bug.cgi?id=42749
cc: x86@kernel.org
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/reboot.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 79c45af..412db57 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -451,6 +451,14 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "OptiPlex 990"),
},
},
+ { /* Handle problems with rebooting on the Precision M6600. */
+ .callback = set_pci_reboot,
+ .ident = "Dell OptiPlex 990",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Precision M6600"),
+ },
+ },
{ }
};
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 3/8] ACPI: Make acpi_skip_timer_override cover all source_irq==0 cases
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Feng Tang <feng.tang@intel.com>
Currently when acpi_skip_timer_override is set, it only cover the
(source_irq == 0 && global_irq == 2) cases. While there is also
platform which need use this option and its global_irq is not 2.
This patch will extend acpi_skip_timer_override to cover all
timer overriding cases as long as the source irq is 0.
This is the first part of a fix to kernel bug bugzilla 40002:
"IRQ 0 assigned to VGA"
https://bugzilla.kernel.org/show_bug.cgi?id=40002
Reported-and-tested-by: Szymon Kowalczyk <fazerxlo@o2.pl>
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/acpi/boot.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 8afb693..e7c698e 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -422,12 +422,14 @@ acpi_parse_int_src_ovr(struct acpi_subtable_header * header,
return 0;
}
- if (intsrc->source_irq == 0 && intsrc->global_irq == 2) {
+ if (intsrc->source_irq == 0) {
if (acpi_skip_timer_override) {
- printk(PREFIX "BIOS IRQ0 pin2 override ignored.\n");
+ printk(PREFIX "BIOS IRQ0 override ignored.\n");
return 0;
}
- if (acpi_fix_pin2_polarity && (intsrc->inti_flags & ACPI_MADT_POLARITY_MASK)) {
+
+ if ((intsrc->global_irq == 2) && acpi_fix_pin2_polarity
+ && (intsrc->inti_flags & ACPI_MADT_POLARITY_MASK)) {
intsrc->inti_flags &= ~ACPI_MADT_POLARITY_MASK;
printk(PREFIX "BIOS IRQ0 pin2 override: forcing polarity to high active.\n");
}
@@ -1334,7 +1336,7 @@ static int __init dmi_disable_acpi(const struct dmi_system_id *d)
}
/*
- * Force ignoring BIOS IRQ0 pin2 override
+ * Force ignoring BIOS IRQ0 override
*/
static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
{
@@ -1344,7 +1346,7 @@ static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
*/
if (!acpi_skip_timer_override) {
WARN(1, KERN_ERR "ati_ixp4x0 quirk not complete.\n");
- pr_notice("%s detected: Ignoring BIOS IRQ0 pin2 override\n",
+ pr_notice("%s detected: Ignoring BIOS IRQ0 override\n",
d->ident);
acpi_skip_timer_override = 1;
}
@@ -1438,7 +1440,7 @@ static struct dmi_system_id __initdata acpi_dmi_table_late[] = {
* is enabled. This input is incorrectly designated the
* ISA IRQ 0 via an interrupt source override even though
* it is wired to the output of the master 8259A and INTIN0
- * is not connected at all. Force ignoring BIOS IRQ0 pin2
+ * is not connected at all. Force ignoring BIOS IRQ0
* override in that cases.
*/
{
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 4/8] ACPI: Remove one board specific WARN when ignoring timer overriding
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Feng Tang <feng.tang@intel.com>
Current WARN msg is only for the ati_ixp4x0 board, while this function
is used by mulitple platforms. So this one board specific warning
is not appropriate any more.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/acpi/boot.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e7c698e..3a6afba 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1340,12 +1340,7 @@ static int __init dmi_disable_acpi(const struct dmi_system_id *d)
*/
static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
{
- /*
- * The ati_ixp4x0_rev() early PCI quirk should have set
- * the acpi_skip_timer_override flag already:
- */
if (!acpi_skip_timer_override) {
- WARN(1, KERN_ERR "ati_ixp4x0 quirk not complete.\n");
pr_notice("%s detected: Ignoring BIOS IRQ0 override\n",
d->ident);
acpi_skip_timer_override = 1;
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 5/8] ACPI: Add a quirk for "AMILO PRO V2030" to ignore the timer overriding
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Feng Tang <feng.tang@intel.com>
This is the 2nd part of fix for kernel bugzilla 40002:
"IRQ 0 assigned to VGA"
https://bugzilla.kernel.org/show_bug.cgi?id=40002
The root cause is the buggy FW, whose ACPI tables assign the GSI 16
to 2 irqs 0 and 16(VGA), and the VGA is the right owner of GSI 16.
So add a quirk to ignore the irq0 overriding GSI 16 for the
FUJITSU SIEMENS AMILO PRO V2030 platform will solve this issue.
Reported-and-tested-by: Szymon Kowalczyk <fazerxlo@o2.pl>
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/acpi/boot.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 3a6afba..b2297e5 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1470,6 +1470,14 @@ static struct dmi_system_id __initdata acpi_dmi_table_late[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq 6715b"),
},
},
+ {
+ .callback = dmi_ignore_irq0_timer_override,
+ .ident = "FUJITSU SIEMENS",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "AMILO PRO V2030"),
+ },
+ },
{}
};
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 6/8] ACPI, APEI, Avoid too much error reporting in runtime
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, stable, Huang Ying
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Huang Ying <ying.huang@intel.com>
This patch fixed the following bug.
https://bugzilla.kernel.org/show_bug.cgi?id=43282
This is caused by a firmware bug checking (checking generic address
register provided by firmware) in runtime. The checking should be
done in address mapping time instead of runtime to avoid too much
error reporting in runtime.
Reported-by: Pawel Sikora <pluto@agmk.net>
Signed-off-by: Huang Ying <ying.huang@intel.com>
Tested-by: Jean Delvare <khali@linux-fr.org>
Cc: stable@vger.kernel.org
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/apei/apei-base.c | 17 +++++++++++++++--
drivers/acpi/apei/apei-internal.h | 9 +++++++++
drivers/acpi/apei/ghes.c | 6 +++---
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 5577762..6686b1e 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -243,7 +243,7 @@ static int pre_map_gar_callback(struct apei_exec_context *ctx,
u8 ins = entry->instruction;
if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
- return acpi_os_map_generic_address(&entry->register_region);
+ return apei_map_generic_address(&entry->register_region);
return 0;
}
@@ -276,7 +276,7 @@ static int post_unmap_gar_callback(struct apei_exec_context *ctx,
u8 ins = entry->instruction;
if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
- acpi_os_unmap_generic_address(&entry->register_region);
+ apei_unmap_generic_address(&entry->register_region);
return 0;
}
@@ -606,6 +606,19 @@ static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr,
return 0;
}
+int apei_map_generic_address(struct acpi_generic_address *reg)
+{
+ int rc;
+ u32 access_bit_width;
+ u64 address;
+
+ rc = apei_check_gar(reg, &address, &access_bit_width);
+ if (rc)
+ return rc;
+ return acpi_os_map_generic_address(reg);
+}
+EXPORT_SYMBOL_GPL(apei_map_generic_address);
+
/* read GAR in interrupt (including NMI) or process context */
int apei_read(u64 *val, struct acpi_generic_address *reg)
{
diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index cca240a..f220d64 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -7,6 +7,8 @@
#define APEI_INTERNAL_H
#include <linux/cper.h>
+#include <linux/acpi.h>
+#include <linux/acpi_io.h>
struct apei_exec_context;
@@ -68,6 +70,13 @@ static inline int apei_exec_run_optional(struct apei_exec_context *ctx, u8 actio
/* IP has been set in instruction function */
#define APEI_EXEC_SET_IP 1
+int apei_map_generic_address(struct acpi_generic_address *reg);
+
+static inline void apei_unmap_generic_address(struct acpi_generic_address *reg)
+{
+ acpi_os_unmap_generic_address(reg);
+}
+
int apei_read(u64 *val, struct acpi_generic_address *reg);
int apei_write(u64 val, struct acpi_generic_address *reg);
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 9b3cac0..1599566 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -301,7 +301,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
if (!ghes)
return ERR_PTR(-ENOMEM);
ghes->generic = generic;
- rc = acpi_os_map_generic_address(&generic->error_status_address);
+ rc = apei_map_generic_address(&generic->error_status_address);
if (rc)
goto err_free;
error_block_length = generic->error_block_length;
@@ -321,7 +321,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
return ghes;
err_unmap:
- acpi_os_unmap_generic_address(&generic->error_status_address);
+ apei_unmap_generic_address(&generic->error_status_address);
err_free:
kfree(ghes);
return ERR_PTR(rc);
@@ -330,7 +330,7 @@ err_free:
static void ghes_fini(struct ghes *ghes)
{
kfree(ghes->estatus);
- acpi_os_unmap_generic_address(&ghes->generic->error_status_address);
+ apei_unmap_generic_address(&ghes->generic->error_status_address);
}
enum {
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 7/8] ACPI video: Still use ACPI backlight control if _DOS doesn't exist
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm
Cc: linux-kernel, Zhang Rui, Igor Murzov, stable, Len Brown
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Zhang Rui <rui.zhang@intel.com>
This fixes a regression in 3.4-rc1 caused by commit
ea9f8856bd6d4ed45885b06a338f7362cd6c60e5
(ACPI video: Harden video bus adding.)
Some platforms don't have _DOS control method, but the ACPI
backlight still works.
We should not invoke _DOS for these platforms.
https://bugzilla.kernel.org/show_bug.cgi?id=43168
Cc: Igor Murzov <intergalactic.anonymous@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/video.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 9577b6f..4134b30 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -558,6 +558,8 @@ acpi_video_bus_DOS(struct acpi_video_bus *video, int bios_flag, int lcd_flag)
union acpi_object arg0 = { ACPI_TYPE_INTEGER };
struct acpi_object_list args = { 1, &arg0 };
+ if (!video->cap._DOS)
+ return 0;
if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1)
return -EINVAL;
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
* [PATCH 8/8] acpi_pad: fix power_saving thread deadlock
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, stable, Stuart Hayes
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Stuart Hayes <Stuart_Hayes@Dell.com>
The acpi_pad driver can get stuck in destroy_power_saving_task()
waiting for kthread_stop() to stop a power_saving thread. The problem
is that the isolated_cpus_lock mutex is owned when
destroy_power_saving_task() calls kthread_stop(), which waits for a
power_saving thread to end, and the power_saving thread tries to
acquire the isolated_cpus_lock when it calls round_robin_cpu(). This
patch fixes the issue by making round_robin_cpu() use its own mutex.
https://bugzilla.kernel.org/show_bug.cgi?id=42981
Cc: stable@vger.kernel.org
Signed-off-by: Stuart Hayes <Stuart_Hayes@Dell.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/acpi_pad.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index a43fa1a..1502c502 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -36,6 +36,7 @@
#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
static DEFINE_MUTEX(isolated_cpus_lock);
+static DEFINE_MUTEX(round_robin_lock);
static unsigned long power_saving_mwait_eax;
@@ -107,7 +108,7 @@ static void round_robin_cpu(unsigned int tsk_index)
if (!alloc_cpumask_var(&tmp, GFP_KERNEL))
return;
- mutex_lock(&isolated_cpus_lock);
+ mutex_lock(&round_robin_lock);
cpumask_clear(tmp);
for_each_cpu(cpu, pad_busy_cpus)
cpumask_or(tmp, tmp, topology_thread_cpumask(cpu));
@@ -116,7 +117,7 @@ static void round_robin_cpu(unsigned int tsk_index)
if (cpumask_empty(tmp))
cpumask_andnot(tmp, cpu_online_mask, pad_busy_cpus);
if (cpumask_empty(tmp)) {
- mutex_unlock(&isolated_cpus_lock);
+ mutex_unlock(&round_robin_lock);
return;
}
for_each_cpu(cpu, tmp) {
@@ -131,7 +132,7 @@ static void round_robin_cpu(unsigned int tsk_index)
tsk_in_cpu[tsk_index] = preferred_cpu;
cpumask_set_cpu(preferred_cpu, pad_busy_cpus);
cpu_weight[preferred_cpu]++;
- mutex_unlock(&isolated_cpus_lock);
+ mutex_unlock(&round_robin_lock);
set_cpus_allowed_ptr(current, cpumask_of(preferred_cpu));
}
--
1.7.11.1.104.ge7b44f1
^ 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