thinkpad_acpi: Add support for battery thresholds
Subsystems (3)
PATCH v11 3/5 thinkpad_acpi: Add support for battery thresholds
thinkpad_acpi registers two new attributes for each battery:
1) Charge start threshold /sys/class/power_supply/BATN/charge_start_threshold
Valid values are [0, 99]. A value of 0 turns off the start threshold wear control.
2) Charge stop threshold /sys/class/power_supply/BATN/charge_stop_threshold
Valid values are [1, 100]. A value of 100 turns off the stop threshold wear control. This must be configured first.
This patch depends on the following patches:
"battery: Add the battery hooking API"
v2:
* Re-write the patch to make the changes inbattery.c generic as suggested by Rafael
v3:
* Fixed a bug where you could not set the stop threshold to 100% when the start threshold is 0%
* Fixed the "Not Charging" quirk to match Lenovo's BIOS Firmware specification
v4:
* Fixed a bug where you could not set the start
threshold to >stop if stop was 100%
v5:
* Migrated from symbol_get to native linking,
to fix module dependencies
* Fixed a bug where unloading the module would
cause a BUG inside battery
* Fixed a bug where you could unload battery
before unloading thinkpad_acpi
v6:
* Fixed all the style and naming issues pointed
out by Andy Shevchenko
v7:
* No changes in this patch in v7
v8:
* No changes in this patch in v8
v9:
* Use DEVICE_ATTR_RW instead of DEVICE_ATTR
* Use bitopts.h instead of raw operators
* Remove redundant whitespaces
* Remove redundant comments
* Move the power_supply changes to separate patch
* Fix other various styling issues pointed out by
Andy Shevchenko
v10:
* Fix a pasting error from v6 that prevents the setting of the start threshold with EINVAL
v11:
* Fix formatting of changelog
--- drivers/platform/x86/Kconfig | 1 + drivers/platform/x86/thinkpad_acpi.c | 440 ++++++++++++++++++++++++++++++++++- 2 files changed, 440 insertions(+), 1 deletion(-)
drivers/platform/x86/Kconfig +1 -0
@@ -399,6 +399,7 @@ config SURFACE3_WMI config THINKPAD_ACPI tristate "ThinkPad ACPI Laptop Extras" depends on ACPI + depends on ACPI_BATTERY depends on INPUT depends on RFKILL || RFKILL = n depends on ACPI_VIDEO || ACPI_VIDEO = n
drivers/platform/x86/thinkpad_acpi.c +439 -1
481 lines not on this page · raw message
Signature · 1 line
2.7.4
Andy Shevchenko
Quoted 11 lines · Ognjen Galic
thinkpad_acpiregisters two new attributes for each battery:1) Charge start threshold
/sys/class/power_supply/BATN/charge_start_thresholdValid values are [0, 99]. A value of 0 turns off the start threshold wear control.
2) Charge stop threshold
/sys/class/power_supply/BATN/charge_stop_threshold
Quoted 8 lines
Valid values are [1, 100]. A value of 100 turns off the stop threshold wear control. This must be configured first.
This patch depends on the following patches:
"battery: Add the battery hooking API"
Since this is series, no need to put above into changelog.
AFAICS it's not going to be backported either.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9261
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+/** +* This evaluates a ACPI method call specific to the battery +* ACPI extension. The specifics are that an error is marked +* in the 32rd bit of the response, so we just check that here. +* +* Returns 0 on success +*/ +static int tpacpi_battery_acpi_eval (char *method,int *ret,int param) +{
One problem and one trick you may do.
A problem: you return ACPI status as int. We have special type for that. So, be consistent with it.
Looking to acpi_evalf() which is private to the module, I think you need to get rid of ACPI return codes here.
A trick: since you are using only least significant byte of the response, you can declare it as u8 *value (yeah, ret is not best name here I think).
So, something like
..._eval(..., u8 *value, ...)
{
int response;
if (!..._evalf(..., &response, ...)) {
...drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9267
Quoted 6 lines
drivers/platform/x86/thinkpad_acpi.c +6
-0+if (!acpi_evalf (hkey_handle, ret, method,"dd" , param)) { +acpi_handle_err (hkey_handle,"%s: evaluate failed" , method); +return AE_ERROR ; + } + +if (*ret &METHOD_ERR ) {
if (response & ...) {
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9271
Quoted 4 lines
drivers/platform/x86/thinkpad_acpi.c +4
-0+acpi_handle_err (hkey_handle, +"%s evaluated but flagged as error" , method); +return AE_ERROR ; + }
*value = response;
Quoted 6 lines
drivers/platform/x86/thinkpad_acpi.c +5
-0+ +return AE_OK ; +} +static int tpacpi_battery_get (int what,int battery,int *ret) +{
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9286
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+switch (what) { + +case THRESHOLD_START : + +if (tpacpi_battery_acpi_eval (GET_START , ret, battery)) +return -ENODEV ; + +/* The value is in the low 8 bits of the response */ + *ret = *ret &0xFF ;
So, this will gone with a trick above.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9295
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+return 0 ; + +case THRESHOLD_STOP : + +if (tpacpi_battery_acpi_eval (GET_STOP , ret, battery)) +return -ENODEV ; + +/* Value is in lower 8 bits */ + *ret = *ret &0xFF ;
Ditto.
Quoted 46 lines
drivers/platform/x86/thinkpad_acpi.c +45
-0+ +/* +* On the stop value, if we return 0 that +* does not make any sense. 0 means Default, which +* means that charging stops at 100%, so we return +* that. +*/ +if (*ret ==0 ) + *ret =100 ; + +return 0 ; + +default : +pr_crit ("wrong parameter: %d" , what); +return -EINVAL ; + } + +} +static int tpacpi_battery_probe (int battery) +{ +int ret =0 ; + +memset (&battery_info,0 ,sizeof (struct tpacpi_battery_driver_data )); + +/* +* 1) Get the current start threshold +* 2) Check for support +* 3) Get the current stop threshold +* 4) Check for support +*/ + +if (acpi_has_method (hkey_handle,GET_START )) { + +if (tpacpi_battery_acpi_eval (GET_START , &ret, battery)) { +pr_err ("Error probing battery %d\n" , battery); +return -ENODEV ; + } + +/* Individual addressing is in bit 9 */ +if (ret &BIT (9 )) + battery_info.individual_addressing = true; + +/* Support is marked in bit 8 */ +if (ret &BIT (8 )) + battery_info.batteries[battery].start_support =1 ;
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9386
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+else +return -ENODEV ; + +if (tpacpi_battery_get (THRESHOLD_START , battery, + &battery_info.batteries[battery].charge_start)) { +pr_err ("Error probing battery %d\n" , battery); +return -ENODEV ; + } +
Please, get rid of all useless empty lines like this one, or above (in between of two if:s).
Quoted 24 lines
drivers/platform/x86/thinkpad_acpi.c +23
-0+ } +/* General helper functions */ + +static int tpacpi_battery_get_id (const char *battery_name) +{ + +if (strcmp (battery_name,"BAT0" ) ==0 ) +return BAT_PRIMARY ; +if (strcmp (battery_name,"BAT1" ) ==0 ) +return BAT_SECONDARY ; + +/* +* If for some reason the battery is not BAT0 nor is it +* BAT1, we will assume it's the default, first battery, +* AKA primary. +*/ +pr_warn ("unknown battery %s, assuming primary" , battery_name); +return BAT_PRIMARY ; +} + +/* sysfs interface */ + +static ssize_t tpacpi_battery_store(int what,
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9464
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +8
-0+struct device *dev, + const char *buf, size_t count) +{ + struct power_supply *supply = to_power_supply(dev); +unsigned long value; +int battery, errno; + errno =kstrtoul (buf,10 , &value); +
Dont' split lines like these where one returns some error code followed by conditional check.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9466
Quoted 3 lines
drivers/platform/x86/thinkpad_acpi.c +2
-0+if (errno) +return errno;
Besides, errno is a bad name, please consider what is used elsewhere in this module (rc, ret, rval, ...?).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 38 lines · Andy Shevchenko
Quoted 11 lines · Ognjen Galic
thinkpad_acpiregisters two new attributes for each battery:1) Charge start threshold
/sys/class/power_supply/BATN/charge_start_thresholdValid values are [0, 99]. A value of 0 turns off the start threshold wear control.
2) Charge stop threshold
/sys/class/power_supply/BATN/charge_stop_thresholdQuoted 8 lines
Valid values are [1, 100]. A value of 100 turns off the stop threshold wear control. This must be configured first.
This patch depends on the following patches:
"battery: Add the battery hooking API"
Since this is series, no need to put above into changelog.
AFAICS it's not going to be backported either.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9261
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+/** +* This evaluates a ACPI method call specific to the battery +* ACPI extension. The specifics are that an error is marked +* in the 32rd bit of the response, so we just check that here. +* +* Returns 0 on success +*/ +static int tpacpi_battery_acpi_eval (char *method,int *ret,int param) +{One problem and one trick you may do.
Quoted 4 lines
A problem: you return ACPI status as int. We have special type for that. So, be consistent with it.
Looking toacpi_evalf()which is private to the module, I think you need to get rid of ACPI return codes here.
What do you mean by "get rid of ACPI return codes"? Are you suggesting this?
static void tpacpi_battery_ac....
How would I check if the ACPI call failed if I made the function return void?
I use a function return value to check if the ACPI call failed, and a return integer pointer to which I write the return data to.
Quoted 4 lines
A trick: since you are using only least significant byte of the response, you can declare it as u8 *value (yeah, ret is not best name here I think).
Neat trick.
Quoted 165 lines
So, something like
..._eval(..., u8 *value, ...)
{
int response;if (!..._evalf(..., &response, ...)) { ...drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9267
Quoted 6 lines
drivers/platform/x86/thinkpad_acpi.c +6
-0+if (!acpi_evalf (hkey_handle, ret, method,"dd" , param)) { +acpi_handle_err (hkey_handle,"%s: evaluate failed" , method); +return AE_ERROR ; + } + +if (*ret &METHOD_ERR ) {if (response & ...) {
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9271
Quoted 4 lines
drivers/platform/x86/thinkpad_acpi.c +4
-0+acpi_handle_err (hkey_handle, +"%s evaluated but flagged as error" , method); +return AE_ERROR ; + }*value = response;
Quoted 6 lines
drivers/platform/x86/thinkpad_acpi.c +5
-0+ +return AE_OK ; +} +static int tpacpi_battery_get (int what,int battery,int *ret) +{drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9286
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+switch (what) { + +case THRESHOLD_START : + +if (tpacpi_battery_acpi_eval (GET_START , ret, battery)) +return -ENODEV ; + +/* The value is in the low 8 bits of the response */ + *ret = *ret &0xFF ;So, this will gone with a trick above.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9295
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+return 0 ; + +case THRESHOLD_STOP : + +if (tpacpi_battery_acpi_eval (GET_STOP , ret, battery)) +return -ENODEV ; + +/* Value is in lower 8 bits */ + *ret = *ret &0xFF ;Ditto.
Quoted 46 lines
drivers/platform/x86/thinkpad_acpi.c +45
-0+ +/* +* On the stop value, if we return 0 that +* does not make any sense. 0 means Default, which +* means that charging stops at 100%, so we return +* that. +*/ +if (*ret ==0 ) + *ret =100 ; + +return 0 ; + +default : +pr_crit ("wrong parameter: %d" , what); +return -EINVAL ; + } + +} +static int tpacpi_battery_probe (int battery) +{ +int ret =0 ; + +memset (&battery_info,0 ,sizeof (struct tpacpi_battery_driver_data )); + +/* +* 1) Get the current start threshold +* 2) Check for support +* 3) Get the current stop threshold +* 4) Check for support +*/ + +if (acpi_has_method (hkey_handle,GET_START )) { + +if (tpacpi_battery_acpi_eval (GET_START , &ret, battery)) { +pr_err ("Error probing battery %d\n" , battery); +return -ENODEV ; + } + +/* Individual addressing is in bit 9 */ +if (ret &BIT (9 )) + battery_info.individual_addressing = true; + +/* Support is marked in bit 8 */ +if (ret &BIT (8 )) + battery_info.batteries[battery].start_support =1 ;drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9386
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+else +return -ENODEV ; + +if (tpacpi_battery_get (THRESHOLD_START , battery, + &battery_info.batteries[battery].charge_start)) { +pr_err ("Error probing battery %d\n" , battery); +return -ENODEV ; + } +Please, get rid of all useless empty lines like this one, or above (in between of two if:s).
Quoted 24 lines
drivers/platform/x86/thinkpad_acpi.c +23
-0+ } +/* General helper functions */ + +static int tpacpi_battery_get_id (const char *battery_name) +{ + +if (strcmp (battery_name,"BAT0" ) ==0 ) +return BAT_PRIMARY ; +if (strcmp (battery_name,"BAT1" ) ==0 ) +return BAT_SECONDARY ; + +/* +* If for some reason the battery is not BAT0 nor is it +* BAT1, we will assume it's the default, first battery, +* AKA primary. +*/ +pr_warn ("unknown battery %s, assuming primary" , battery_name); +return BAT_PRIMARY ; +} + +/* sysfs interface */ + +static ssize_t tpacpi_battery_store(int what,drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9464
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +8
-0+struct device *dev, + const char *buf, size_t count) +{ + struct power_supply *supply = to_power_supply(dev); +unsigned long value; +int battery, errno; + errno =kstrtoul (buf,10 , &value); +Dont' split lines like these where one returns some error code followed by conditional check.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9466
Quoted 3 lines
drivers/platform/x86/thinkpad_acpi.c +2
-0+if (errno) +return errno;Besides, errno is a bad name, please consider what is used elsewhere in this module (rc, ret, rval, ...?).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Rafael J. Wysocki
Quoted 44 lines · Ognjen Galić
Quoted 38 lines · Andy Shevchenko
Quoted 11 lines · Ognjen Galic
thinkpad_acpiregisters two new attributes for each battery:1) Charge start threshold
/sys/class/power_supply/BATN/charge_start_thresholdValid values are [0, 99]. A value of 0 turns off the start threshold wear control.
2) Charge stop threshold
/sys/class/power_supply/BATN/charge_stop_thresholdQuoted 8 lines
Valid values are [1, 100]. A value of 100 turns off the stop threshold wear control. This must be configured first.
This patch depends on the following patches:
"battery: Add the battery hooking API"
Since this is series, no need to put above into changelog.
AFAICS it's not going to be backported either.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9261
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+/** +* This evaluates a ACPI method call specific to the battery +* ACPI extension. The specifics are that an error is marked +* in the 32rd bit of the response, so we just check that here. +* +* Returns 0 on success +*/ +static int tpacpi_battery_acpi_eval (char *method,int *ret,int param) +{One problem and one trick you may do.
Quoted 4 lines
A problem: you return ACPI status as int. We have special type for that. So, be consistent with it.
Looking toacpi_evalf()which is private to the module, I think you need to get rid of ACPI return codes here.
Quoted 4 lines
What do you mean by "get rid of ACPI return codes"? Are you suggesting this?
static void
tpacpi_battery_ac....
I think that Andy was talking about returning things like AE_ERROR, which are of type acpi_status, as int.
You should return either the Linux error codes (in which case int is the type to use) or ACPI error codes (in which the function should be defined to return acpi_status). Do not mix them.
Quoted 7 lines
How would I check if the ACPI call failed if I made the function return void?
I use a function return value to check if the ACPI call failed, and a return integer pointer to which I write the return data to.
Quoted 1 line
Quoted 5 lines
Quoted 3 lines
A trick: since you are using only least significant byte of the response, you can declare it as u8 *value (yeah, ret is not best name here I think).
Neat trick.
Little-endian only though. Don't do that unless your code is guaranteed to always (now and in the future) run on little-endian.
Thanks,
Rafael
Quoted 38 lines · Andy Shevchenko
Quoted 11 lines · Ognjen Galic
thinkpad_acpiregisters two new attributes for each battery:1) Charge start threshold
/sys/class/power_supply/BATN/charge_start_thresholdValid values are [0, 99]. A value of 0 turns off the start threshold wear control.
2) Charge stop threshold
/sys/class/power_supply/BATN/charge_stop_thresholdQuoted 8 lines
Valid values are [1, 100]. A value of 100 turns off the stop threshold wear control. This must be configured first.
This patch depends on the following patches:
"battery: Add the battery hooking API"
Since this is series, no need to put above into changelog.
AFAICS it's not going to be backported either.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9261
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+/** +* This evaluates a ACPI method call specific to the battery +* ACPI extension. The specifics are that an error is marked +* in the 32rd bit of the response, so we just check that here. +* +* Returns 0 on success +*/ +static int tpacpi_battery_acpi_eval (char *method,int *ret,int param) +{One problem and one trick you may do.
Quoted 8 lines
A problem: you return ACPI status as int. We have special type for that. So, be consistent with it.
Looking toacpi_evalf()which is private to the module, I think you need to get rid of ACPI return codes here.A trick: since you are using only least significant byte of the response, you can declare it as u8 *value (yeah, ret is not best name here I think).
Not really. In one place I need bit #9, so that will stay a int. I have changed the acpi return value to acpi_status tho.
Quoted 165 lines
So, something like
..._eval(..., u8 *value, ...)
{
int response;if (!..._evalf(..., &response, ...)) { ...drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9267
Quoted 6 lines
drivers/platform/x86/thinkpad_acpi.c +6
-0+if (!acpi_evalf (hkey_handle, ret, method,"dd" , param)) { +acpi_handle_err (hkey_handle,"%s: evaluate failed" , method); +return AE_ERROR ; + } + +if (*ret &METHOD_ERR ) {if (response & ...) {
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9271
Quoted 4 lines
drivers/platform/x86/thinkpad_acpi.c +4
-0+acpi_handle_err (hkey_handle, +"%s evaluated but flagged as error" , method); +return AE_ERROR ; + }*value = response;
Quoted 6 lines
drivers/platform/x86/thinkpad_acpi.c +5
-0+ +return AE_OK ; +} +static int tpacpi_battery_get (int what,int battery,int *ret) +{drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9286
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+switch (what) { + +case THRESHOLD_START : + +if (tpacpi_battery_acpi_eval (GET_START , ret, battery)) +return -ENODEV ; + +/* The value is in the low 8 bits of the response */ + *ret = *ret &0xFF ;So, this will gone with a trick above.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9295
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+return 0 ; + +case THRESHOLD_STOP : + +if (tpacpi_battery_acpi_eval (GET_STOP , ret, battery)) +return -ENODEV ; + +/* Value is in lower 8 bits */ + *ret = *ret &0xFF ;Ditto.
Quoted 46 lines
drivers/platform/x86/thinkpad_acpi.c +45
-0+ +/* +* On the stop value, if we return 0 that +* does not make any sense. 0 means Default, which +* means that charging stops at 100%, so we return +* that. +*/ +if (*ret ==0 ) + *ret =100 ; + +return 0 ; + +default : +pr_crit ("wrong parameter: %d" , what); +return -EINVAL ; + } + +} +static int tpacpi_battery_probe (int battery) +{ +int ret =0 ; + +memset (&battery_info,0 ,sizeof (struct tpacpi_battery_driver_data )); + +/* +* 1) Get the current start threshold +* 2) Check for support +* 3) Get the current stop threshold +* 4) Check for support +*/ + +if (acpi_has_method (hkey_handle,GET_START )) { + +if (tpacpi_battery_acpi_eval (GET_START , &ret, battery)) { +pr_err ("Error probing battery %d\n" , battery); +return -ENODEV ; + } + +/* Individual addressing is in bit 9 */ +if (ret &BIT (9 )) + battery_info.individual_addressing = true; + +/* Support is marked in bit 8 */ +if (ret &BIT (8 )) + battery_info.batteries[battery].start_support =1 ;drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9386
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+else +return -ENODEV ; + +if (tpacpi_battery_get (THRESHOLD_START , battery, + &battery_info.batteries[battery].charge_start)) { +pr_err ("Error probing battery %d\n" , battery); +return -ENODEV ; + } +Please, get rid of all useless empty lines like this one, or above (in between of two if:s).
Quoted 24 lines
drivers/platform/x86/thinkpad_acpi.c +23
-0+ } +/* General helper functions */ + +static int tpacpi_battery_get_id (const char *battery_name) +{ + +if (strcmp (battery_name,"BAT0" ) ==0 ) +return BAT_PRIMARY ; +if (strcmp (battery_name,"BAT1" ) ==0 ) +return BAT_SECONDARY ; + +/* +* If for some reason the battery is not BAT0 nor is it +* BAT1, we will assume it's the default, first battery, +* AKA primary. +*/ +pr_warn ("unknown battery %s, assuming primary" , battery_name); +return BAT_PRIMARY ; +} + +/* sysfs interface */ + +static ssize_t tpacpi_battery_store(int what,drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9464
Quoted 10 lines
drivers/platform/x86/thinkpad_acpi.c +8
-0+struct device *dev, + const char *buf, size_t count) +{ + struct power_supply *supply = to_power_supply(dev); +unsigned long value; +int battery, errno; + errno =kstrtoul (buf,10 , &value); +Dont' split lines like these where one returns some error code followed by conditional check.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9466
Quoted 3 lines
drivers/platform/x86/thinkpad_acpi.c +2
-0+if (errno) +return errno;Besides, errno is a bad name, please consider what is used elsewhere in this module (rc, ret, rval, ...?).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Rafael J. Wysocki
Quoted 44 lines · CET Ognjen Galić
Quoted 43 lines · Andy Shevchenko
Quoted 11 lines · Ognjen Galic
thinkpad_acpiregisters two new attributes for each battery:1) Charge start threshold
/sys/class/power_supply/BATN/charge_start_thresholdValid values are [0, 99]. A value of 0 turns off the start threshold wear control.
2) Charge stop threshold
/sys/class/power_supply/BATN/charge_stop_thresholdQuoted 8 lines
Valid values are [1, 100]. A value of 100 turns off the stop threshold wear control. This must be configured first.
This patch depends on the following patches:
"battery: Add the battery hooking API"
Since this is series, no need to put above into changelog.
AFAICS it's not going to be backported either.
drivers/platform/x86/thinkpad_acpi.c static struct ibm_struct mute_led_driver_data = { · line 9261
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +9
-0+/** +* This evaluates a ACPI method call specific to the battery +* ACPI extension. The specifics are that an error is marked +* in the 32rd bit of the response, so we just check that here. +* +* Returns 0 on success +*/ +static int tpacpi_battery_acpi_eval (char *method,int *ret,int param) +{One problem and one trick you may do.
A problem: you return ACPI status as int. We have special type for that. So, be consistent with it.
Looking toacpi_evalf()which is private to the module, I think you need to get rid of ACPI return codes here.
Quoted 6 lines
Quoted 3 lines
A trick: since you are using only least significant byte of the response, you can declare it as u8 *value (yeah, ret is not best name here I think).
Not really. In one place I need bit #9, so that will stay a int. I have changed the acpi return value to
acpi_statustho.
OK, you can resend the updated series when you're ready.
Thanks,
Rafael