Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1550549 > unrolled thread

Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices

Started byLee Jones <lee.jones@linaro.org>
First post2017-01-04 10:10 +0100
Last post2017-01-05 17:40 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices Lee Jones <lee.jones@linaro.org> - 2017-01-04 10:10 +0100
    Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices Thierry Escande <thierry.escande@collabora.com> - 2017-01-04 18:20 +0100
      Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices Lee Jones <lee.jones@linaro.org> - 2017-01-05 09:00 +0100
        Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices Thierry Escande <thierry.escande@collabora.com> - 2017-01-05 11:40 +0100
          Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices Lee Jones <lee.jones@linaro.org> - 2017-01-05 15:40 +0100
            Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices Thierry Escande <thierry.escande@collabora.com> - 2017-01-05 17:40 +0100

#1550549 — Re: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices

FromLee Jones <lee.jones@linaro.org>
Date2017-01-04 10:10 +0100
SubjectRe: [PATCH 3/3] mfd: cros_ec: Add ACPI GPE handler for LID0 devices
Message-ID<sVPK2-2AQ-51@gated-at.bofh.it>
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)
> +{
> +	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)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [next] | [standalone]

#1550996

FromThierry Escande <thierry.escande@collabora.com>
Date2017-01-04 18:20 +0100
Message-ID<sVXod-7Er-9@gated-at.bofh.it>
In reply to#1550549
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)
>

[toc] | [prev] | [next] | [standalone]

#1551712

FromLee Jones <lee.jones@linaro.org>
Date2017-01-05 09:00 +0100
Message-ID<sWb7Q-8fR-21@gated-at.bofh.it>
In reply to#1550996
On Wed, 04 Jan 2017, Thierry Escande wrote:

> 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.

Can you try placing:

	if (IS_ENABLED(CONFIG_ACPI))

... before the call to cros_ec_get_ec_wake_gpe() and see if the
compiler will do-the-right-thing please?

> > > +	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)
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1551871

FromThierry Escande <thierry.escande@collabora.com>
Date2017-01-05 11:40 +0100
Message-ID<sWdCG-1IQ-11@gated-at.bofh.it>
In reply to#1551712
Hi Lee,

On 05/01/2017 08:54, Lee Jones wrote:
> On Wed, 04 Jan 2017, Thierry Escande wrote:
>
>> 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.
>
> Can you try placing:
>
> 	if (IS_ENABLED(CONFIG_ACPI))
>
> ... before the call to cros_ec_get_ec_wake_gpe() and see if the
> compiler will do-the-right-thing please?

With CONFIG_ACPI not defined, the acpi_dev structure is not defined as 
well as the acpi_bus_get_device() API. So no, the compiler is not smart 
enough even if the function body is skipped by the test:
	if (!IS_ENABLED(CONFIG_ACPI))
		return -ENODEV;

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)
>>>
>

[toc] | [prev] | [next] | [standalone]

#1552020

FromLee Jones <lee.jones@linaro.org>
Date2017-01-05 15:40 +0100
Message-ID<sWhmW-4hV-19@gated-at.bofh.it>
In reply to#1551871
Rafael,

On Thu, 05 Jan 2017, Thierry Escande wrote:

> Hi Lee,
> 
> On 05/01/2017 08:54, Lee Jones wrote:
> > On Wed, 04 Jan 2017, Thierry Escande wrote:
> > 
> > > 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.
> > 
> > Can you try placing:
> > 
> > 	if (IS_ENABLED(CONFIG_ACPI))
> > 
> > ... before the call to cros_ec_get_ec_wake_gpe() and see if the
> > compiler will do-the-right-thing please?
> 
> With CONFIG_ACPI not defined, the acpi_dev structure is not defined as well
> as the acpi_bus_get_device() API. So no, the compiler is not smart enough
> even if the function body is skipped by the test:
> 	if (!IS_ENABLED(CONFIG_ACPI))
> 		return -ENODEV;

Is there any way we can make ACPI play nicely if it's not enabled?

I'm thinking stubs.  Like we do for most other things.

> > > > > +	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)
> > > > 
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1552102

FromThierry Escande <thierry.escande@collabora.com>
Date2017-01-05 17:40 +0100
Message-ID<sWjf3-5uZ-21@gated-at.bofh.it>
In reply to#1552020
On 05/01/2017 15:43, Lee Jones wrote:
> Rafael,
>
> On Thu, 05 Jan 2017, Thierry Escande wrote:
>
>> Hi Lee,
>>
>> On 05/01/2017 08:54, Lee Jones wrote:
>>> On Wed, 04 Jan 2017, Thierry Escande wrote:
>>>
>>>> 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.
>>>
>>> Can you try placing:
>>>
>>> 	if (IS_ENABLED(CONFIG_ACPI))
>>>
>>> ... before the call to cros_ec_get_ec_wake_gpe() and see if the
>>> compiler will do-the-right-thing please?
>>
>> With CONFIG_ACPI not defined, the acpi_dev structure is not defined as well
>> as the acpi_bus_get_device() API. So no, the compiler is not smart enough
>> even if the function body is skipped by the test:
>> 	if (!IS_ENABLED(CONFIG_ACPI))
>> 		return -ENODEV;
>
> Is there any way we can make ACPI play nicely if it's not enabled?
>
> I'm thinking stubs.  Like we do for most other things.

Or I can move the 2 functions into a separate file compiled only if ACPI 
is enabled.

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)
>>>>>
>>>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web