Messages in this thread | | | Subject | Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices | From | Thierry Escande <> | Date | Wed, 4 Jan 2017 18:17:29 +0100 |
| |
Hi Lee,
On 04/01/2017 10:06, Lee Jones wrote: > On Fri, 16 Dec 2016, Thierry Escande wrote: > >> From: Archana Patni <archana.patni@intel.com> >> >> This patch installs an ACPI GPE handler for LID0 ACPI device to indicate >> ACPI core that this GPE should stay enabled for lid to work in suspend >> to idle path. >> >> Signed-off-by: Archana Patni <archana.patni@intel.com> >> Signed-off-by: Thierry Escande <thierry.escande@collabora.com> >> --- >> drivers/mfd/cros_ec.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 95 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c >> index b8a5080..628718e 100644 >> --- a/drivers/mfd/cros_ec.c >> +++ b/drivers/mfd/cros_ec.c >> @@ -17,6 +17,9 @@ >> * battery charging and regulator control, firmware update. >> */ >> >> +#ifdef CONFIG_ACPI >> +#include <linux/acpi.h> >> +#endif > > Please don't place #ifery in C files. > >> #include <linux/of_platform.h> >> #include <linux/interrupt.h> >> #include <linux/slab.h> >> @@ -29,6 +32,73 @@ >> #define CROS_EC_DEV_EC_INDEX 0 >> #define CROS_EC_DEV_PD_INDEX 1 >> >> +#ifdef CONFIG_ACPI > > Remove this. > >> +#define ACPI_LID_DEVICE "LID0" >> + >> +static int ec_wake_gpe = -EINVAL; >> + >> +/* >> + * This handler indicates to ACPI core that this GPE should stay enabled for >> + * lid to work in suspend to idle path. >> + */ >> +static u32 cros_ec_gpe_handler(acpi_handle gpe_device, u32 gpe_number, >> + void *data) >> +{ >> + return ACPI_INTERRUPT_HANDLED | ACPI_REENABLE_GPE; >> +} >> + >> +/* >> + * Get ACPI GPE for LID0 device. >> + */ >> +static int cros_ec_get_ec_wake_gpe(struct device *dev) >> +{ If it's ok for you, I'll keep one #ifdef CONFIG_ACPI around the body of this function. Otherwise it won't compile if CONFIG_ACPI is not set.
Regards, Thierry
>> + struct acpi_device *cros_acpi_dev; >> + struct acpi_device *adev; >> + acpi_handle handle; >> + acpi_status status; >> + int ret; >> + >> + cros_acpi_dev = ACPI_COMPANION(dev); >> + >> + if (!cros_acpi_dev || !cros_acpi_dev->parent || >> + !cros_acpi_dev->parent->handle) >> + return -EINVAL; >> + >> + status = acpi_get_handle(cros_acpi_dev->parent->handle, ACPI_LID_DEVICE, >> + &handle); >> + if (ACPI_FAILURE(status)) >> + return -EINVAL; >> + >> + ret = acpi_bus_get_device(handle, &adev); >> + if (ret) >> + return ret; >> + >> + return adev->wakeup.gpe_number; >> +} >> + >> +static int cros_ec_install_handler(struct device *dev) >> +{ >> + acpi_status status; >> + >> + ec_wake_gpe = cros_ec_get_ec_wake_gpe(dev); >> + >> + if (ec_wake_gpe < 0) >> + return ec_wake_gpe; >> + >> + status = acpi_install_gpe_handler(NULL, ec_wake_gpe, >> + ACPI_GPE_EDGE_TRIGGERED, >> + &cros_ec_gpe_handler, NULL); >> + if (ACPI_FAILURE(status)) >> + return -ENODEV; >> + >> + dev_info(dev, "Initialized, GPE = 0x%x\n", ec_wake_gpe); >> + >> + return 0; >> +} >> + >> +#endif >> + >> static struct cros_ec_platform ec_p = { >> .ec_name = CROS_EC_DEV_NAME, >> .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_EC_INDEX), >> @@ -166,6 +236,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev) >> >> dev_info(dev, "Chrome EC device registered\n"); >> >> +#ifdef CONFIG_ACPI >> + cros_ec_install_handler(dev); >> +#endif > > Here, just do: > > if (IS_ENABLED(CONFIG_ACPI)) > cros_ec_install_handler(dev); > > And let the compiler take care of the rest. > >> return 0; >> >> fail_mfd: >> @@ -179,6 +253,17 @@ int cros_ec_remove(struct cros_ec_device *ec_dev) >> { >> mfd_remove_devices(ec_dev->dev); >> >> +#ifdef CONFIG_ACPI >> + if (ec_wake_gpe >= 0) { > > if (IS_ENABLED(CONFIG_ACPI) && ec_wake_gpe >= 0) { > >> + acpi_status status; >> + >> + status = acpi_remove_gpe_handler(NULL, ec_wake_gpe, >> + &cros_ec_gpe_handler); >> + if (ACPI_FAILURE(status)) >> + pr_err("failed to remove gpe handler\n"); >> + } >> +#endif >> + >> return 0; >> } >> EXPORT_SYMBOL(cros_ec_remove); >> @@ -190,8 +275,16 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev) >> int ret; >> u8 sleep_event; >> >> - sleep_event = pm_suspend_via_firmware() ? HOST_SLEEP_EVENT_S3_RESUME : >> - HOST_SLEEP_EVENT_S0IX_RESUME; >> + if (!pm_suspend_via_firmware()) { >> + sleep_event = HOST_SLEEP_EVENT_S0IX_SUSPEND; >> +#ifdef CONFIG_ACPI >> + /* Clearing the GPE status for any pending event */ >> + if (ec_wake_gpe >= 0) > > As above. > >> + acpi_clear_gpe(NULL, ec_wake_gpe); >> +#endif >> + } else { >> + sleep_event = HOST_SLEEP_EVENT_S3_SUSPEND; >> + } >> >> ret = cros_ec_sleep_event(ec_dev, sleep_event); >> if (ret < 0) >
| |