lkml.org 
[lkml]   [2017]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices
From
Date
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)
>

\
 
 \ /
  Last update: 2017-01-04 18:19    [W:0.088 / U:1.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site