[Trusty,SRU] ACPI / battery: Accelerate battery resume callback

From: Lan Tianyu <tianyu.lan@intel.com>

BugLink: https://bugs.launchpad.net/bugs/838543

Most time of battery resume callback is spent on executing AML code
_BTP, _BIF and _BIF to get battery info, status and set alarm. These
AML methods may access EC operation regions several times and consumes
time.

These operations are not necessary during devices resume and can run
during POST_SUSPEND/HIBERNATION event when all processes are thawed.

This also can avoid removing and adding battery sysfs nodes every system
resume even if the battery unit is not actually changed. The original code
updates sysfs nodes without check and this seems not reasonable.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
(backported from commit 9e50bc14a7f58b5d8a55973b2d69355852ae2dae)
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>

Conflicts:
	drivers/acpi/battery.c
---
 drivers/acpi/battery.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

On 11/06/2014 01:15 AM, AceLan Kao wrote:
> From: Lan Tianyu <tianyu.lan@intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/838543
> 
> Most time of battery resume callback is spent on executing AML code
> _BTP, _BIF and _BIF to get battery info, status and set alarm. These
> AML methods may access EC operation regions several times and consumes
> time.
> 
> These operations are not necessary during devices resume and can run
> during POST_SUSPEND/HIBERNATION event when all processes are thawed.
> 
> This also can avoid removing and adding battery sysfs nodes every system
> resume even if the battery unit is not actually changed. The original code
> updates sysfs nodes without check and this seems not reasonable.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> (backported from commit 9e50bc14a7f58b5d8a55973b2d69355852ae2dae)
> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
> 
> Conflicts:
> 	drivers/acpi/battery.c
> ---
>  drivers/acpi/battery.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 2928e95..fabd751 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -700,7 +700,7 @@ static void acpi_battery_quirks(struct acpi_battery *battery)
>  	}
>  }
>  
> -static int acpi_battery_update(struct acpi_battery *battery)
> +static int acpi_battery_update(struct acpi_battery *battery, bool resume)
>  {
>  	int result, old_present = acpi_battery_present(battery);
>  	result = acpi_battery_get_status(battery);
> @@ -711,6 +711,10 @@ static int acpi_battery_update(struct acpi_battery *battery)
>  		battery->update_time = 0;
>  		return 0;
>  	}
> +
> +	if (resume)
> +		return 0;
> +
>  	if (!battery->update_time ||
>  	    old_present != acpi_battery_present(battery)) {
>  		result = acpi_battery_get_info(battery);
> @@ -919,7 +923,7 @@ static print_func acpi_print_funcs[ACPI_BATTERY_NUMFILES] = {
>  static int acpi_battery_read(int fid, struct seq_file *seq)
>  {
>  	struct acpi_battery *battery = seq->private;
> -	int result = acpi_battery_update(battery);
> +	int result = acpi_battery_update(battery, false);
>  	return acpi_print_funcs[fid](seq, result);
>  }
>  
> @@ -1034,7 +1038,7 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
>  	old = battery->bat.dev;
>  	if (event == ACPI_BATTERY_NOTIFY_INFO)
>  		acpi_battery_refresh(battery);
> -	acpi_battery_update(battery);
> +	acpi_battery_update(battery, false);
>  	acpi_bus_generate_netlink_event(device->pnp.device_class,
>  					dev_name(&device->dev), event,
>  					acpi_battery_present(battery));
> @@ -1048,13 +1052,27 @@ static int battery_notify(struct notifier_block *nb,
>  {
>  	struct acpi_battery *battery = container_of(nb, struct acpi_battery,
>  						    pm_nb);
> +	int result;
> +
>  	switch (mode) {
>  	case PM_POST_HIBERNATION:
>  	case PM_POST_SUSPEND:
> -		if (battery->bat.dev) {
> -			sysfs_remove_battery(battery);
> -			sysfs_add_battery(battery);
> -		}
> +		if (!acpi_battery_present(battery))
> +			return 0;
> +
> +		if (!battery->bat.dev) {
> +			result = acpi_battery_get_info(battery);
> +			if (result)
> +				return result;
> +
> +			result = sysfs_add_battery(battery);
> +			if (result)
> +				return result;
> +		} else
> +			acpi_battery_refresh(battery);
> +
> +		acpi_battery_init_alarm(battery);
> +		acpi_battery_get_state(battery);
>  		break;
>  	}
>  
> @@ -1085,7 +1103,7 @@ static int acpi_battery_update_retry(struct acpi_battery *battery)
>  	int retry, ret;
>  
>  	for (retry = 5; retry; retry--) {
> -		ret = acpi_battery_update(battery);
> +		ret = acpi_battery_update(battery, false);
>  		if (!ret)
>  			break;
>  
> @@ -1176,7 +1194,7 @@ static int acpi_battery_resume(struct device *dev)
>  		return -EINVAL;
>  
>  	battery->update_time = 0;
> -	acpi_battery_update(battery);
> +	acpi_battery_update(battery, true);
>  	return 0;
>  }
>  #endif
>

On Thu, Nov 06, 2014 at 03:15:34PM +0800, AceLan Kao wrote:
> From: Lan Tianyu <tianyu.lan@intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/838543
> 
> Most time of battery resume callback is spent on executing AML code
> _BTP, _BIF and _BIF to get battery info, status and set alarm. These
> AML methods may access EC operation regions several times and consumes
> time.
> 
> These operations are not necessary during devices resume and can run
> during POST_SUSPEND/HIBERNATION event when all processes are thawed.
> 
> This also can avoid removing and adding battery sysfs nodes every system
> resume even if the battery unit is not actually changed. The original code
> updates sysfs nodes without check and this seems not reasonable.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> (backported from commit 9e50bc14a7f58b5d8a55973b2d69355852ae2dae)
> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
> 
> Conflicts:
> 	drivers/acpi/battery.c
> ---
>  drivers/acpi/battery.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 2928e95..fabd751 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -700,7 +700,7 @@ static void acpi_battery_quirks(struct acpi_battery *battery)
>  	}
>  }
>  
> -static int acpi_battery_update(struct acpi_battery *battery)
> +static int acpi_battery_update(struct acpi_battery *battery, bool resume)
>  {
>  	int result, old_present = acpi_battery_present(battery);
>  	result = acpi_battery_get_status(battery);
> @@ -711,6 +711,10 @@ static int acpi_battery_update(struct acpi_battery *battery)
>  		battery->update_time = 0;
>  		return 0;
>  	}
> +
> +	if (resume)
> +		return 0;
> +
>  	if (!battery->update_time ||
>  	    old_present != acpi_battery_present(battery)) {
>  		result = acpi_battery_get_info(battery);
> @@ -919,7 +923,7 @@ static print_func acpi_print_funcs[ACPI_BATTERY_NUMFILES] = {
>  static int acpi_battery_read(int fid, struct seq_file *seq)
>  {
>  	struct acpi_battery *battery = seq->private;
> -	int result = acpi_battery_update(battery);
> +	int result = acpi_battery_update(battery, false);
>  	return acpi_print_funcs[fid](seq, result);
>  }
>  
> @@ -1034,7 +1038,7 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
>  	old = battery->bat.dev;
>  	if (event == ACPI_BATTERY_NOTIFY_INFO)
>  		acpi_battery_refresh(battery);
> -	acpi_battery_update(battery);
> +	acpi_battery_update(battery, false);
>  	acpi_bus_generate_netlink_event(device->pnp.device_class,
>  					dev_name(&device->dev), event,
>  					acpi_battery_present(battery));
> @@ -1048,13 +1052,27 @@ static int battery_notify(struct notifier_block *nb,
>  {
>  	struct acpi_battery *battery = container_of(nb, struct acpi_battery,
>  						    pm_nb);
> +	int result;
> +
>  	switch (mode) {
>  	case PM_POST_HIBERNATION:
>  	case PM_POST_SUSPEND:
> -		if (battery->bat.dev) {
> -			sysfs_remove_battery(battery);
> -			sysfs_add_battery(battery);
> -		}
> +		if (!acpi_battery_present(battery))
> +			return 0;
> +
> +		if (!battery->bat.dev) {
> +			result = acpi_battery_get_info(battery);
> +			if (result)
> +				return result;
> +
> +			result = sysfs_add_battery(battery);
> +			if (result)
> +				return result;
> +		} else
> +			acpi_battery_refresh(battery);
> +
> +		acpi_battery_init_alarm(battery);
> +		acpi_battery_get_state(battery);
>  		break;
>  	}
>  
> @@ -1085,7 +1103,7 @@ static int acpi_battery_update_retry(struct acpi_battery *battery)
>  	int retry, ret;
>  
>  	for (retry = 5; retry; retry--) {
> -		ret = acpi_battery_update(battery);
> +		ret = acpi_battery_update(battery, false);
>  		if (!ret)
>  			break;
>  
> @@ -1176,7 +1194,7 @@ static int acpi_battery_resume(struct device *dev)
>  		return -EINVAL;
>  
>  	battery->update_time = 0;
> -	acpi_battery_update(battery);
> +	acpi_battery_update(battery, true);
>  	return 0;
>  }
>  #endif

Looks reasonable.  Cirtainly ripping out and shoving back the battery
every time seems excessive.

Acked-by: Andy Whitcroft <apw@canonical.com>

As this is generic we do want to make sure this is tested on a variety
of other laptops.

-apw