On Monday, August 10, 2015 10:11:26 AM Chen Yu wrote:
> For ACPI compatible system, SCI(ACPI System Control
> Interrupt) is used to wake system up from suspend-to-idle.
> Once CPU is woken up by SCI, interrupt handler will
> firstly checks if current interrupt is legal to wake up
> the whole system, thus irq_pm_check_wakeup is invoked
> to validate the irq number. However, before suspend-to-idle,
> acpi_gbl_FADT.sci_interrupt is marked rather than actual
> irq number in acpi_freeze_prepare, this might lead to unable
> to wake up the system.
>
> This patch fixes this problem by marking the irq number
> return by acpi_gsi_to_irq as IRQD_WAKEUP_STATE, rather than
> marking the acpi_gbl_FADT.sci_interrupt.
>
> Signed-off-by: Chen Yu <[email protected]>
That would only really matter if GPE devices were used, but I've never seen
a system using them in practice, so this is more of a theoretical issue.
> ---
> drivers/acpi/osl.c | 5 ++++-
> drivers/acpi/sleep.c | 20 ++++++++++++++++++--
> drivers/acpi/sleep.h | 5 +++++
> 3 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3b8963f..8e1420a 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -49,6 +49,7 @@
> #include <asm/uaccess.h>
>
> #include "internal.h"
> +#include "sleep.h"
>
> #define _COMPONENT ACPI_OS_SERVICES
> ACPI_MODULE_NAME("osl");
> @@ -850,7 +851,9 @@ acpi_os_install_interrupt_handler(u32 gsi,
> acpi_osd_handler handler,
> gsi);
> return AE_OK;
> }
> -
> +#ifdef CONFIG_SUSPEND
> + set_wake_irq_freeze(irq);
> +#endif
Please don't use #ifdefs in function bodies. You can use IS_ENABLED() for that.
> acpi_irq_handler = handler;
> acpi_irq_context = context;
> if (request_irq(irq, acpi_irq, IRQF_SHARED, "acpi", acpi_irq)) {
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 2f0d4db..9e7b54e 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -620,6 +620,22 @@ static const struct platform_suspend_ops
> acpi_suspend_ops_old = {
> .end = acpi_pm_end,
> .recover = acpi_pm_finish,
> };
> +static int wake_irq_freeze = -EINVAL;
There may be more than one of these in theory.
> +
> +int get_wake_irq_freeze(void)
> +{
> + if (IS_ERR_VALUE(wake_irq_freeze))
> + return acpi_gbl_FADT.sci_interrupt;
> + else
> + return wake_irq_freeze;
That would look better this way IMO:
return IS_ERR_VALUE(wake_irq_freeze) ?
acpi_gbl_FADT.sci_interrupt : wake_irq_freeze;
> +}
> +EXPORT_SYMBOL_GPL(get_wake_irq_freeze);
> +
> +void set_wake_irq_freeze(unsigned int irq)
> +{
> + wake_irq_freeze = (int)irq;
> +}
> +EXPORT_SYMBOL_GPL(set_wake_irq_freeze);
>
> static int acpi_freeze_begin(void)
> {
> @@ -632,14 +648,14 @@ static int acpi_freeze_prepare(void)
> acpi_enable_wakeup_devices(ACPI_STATE_S0);
> acpi_enable_all_wakeup_gpes();
> acpi_os_wait_events_complete();
> - enable_irq_wake(acpi_gbl_FADT.sci_interrupt);
> + enable_irq_wake(get_wake_irq_freeze());
> return 0;
> }
>
> static void acpi_freeze_restore(void)
> {
> acpi_disable_wakeup_devices(ACPI_STATE_S0);
> - disable_irq_wake(acpi_gbl_FADT.sci_interrupt);
> + disable_irq_wake(get_wake_irq_freeze());
> acpi_enable_all_runtime_gpes();
> }
>
> diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h
> index c797ffa..eca4fda 100644
> --- a/drivers/acpi/sleep.h
> +++ b/drivers/acpi/sleep.h
> @@ -6,3 +6,8 @@ extern struct list_head acpi_wakeup_device_list;
> extern struct mutex acpi_device_lock;
>
> extern void acpi_resume_power_resources(void);
> +
> +#ifdef CONFIG_SUSPEND
> +extern int get_wake_irq_freeze(void);
> +extern void set_wake_irq_freeze(unsigned int irq);
> +#endif
Is the #ifdef needed here at all?
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
