On Tue, Aug 13, 2019 at 8:30 AM Kristian Klausen <kristian@xxxxxxxxxx> wrote:
>
> Most newer ASUS laptops supports settings a maximum charging percentage,
> which help prolonging the battery life.
>
> Tested on a Zenbook UX430UNR.
>
> Signed-off-by: Kristian Klausen <kristian@xxxxxxxxxx>
> ---
> I can't pass the asus struct to asus_wmi_battery_{add,remove}, so I use a
> global variable. Is there any better way to do it?
> I think the implementation of asus_wmi_battery_{init,exit} could be
> improved, any ideas?
Added Ognjen and Rafael.
Is there a better way to have the sysfs file handlers of files added
via the battery hooking API reach back to the device that hooked in,
in order to access it's private data?
I see that thinkpad_acpi also uses global variables for this :(
> V3:
> Refactor to use the new battery hooking API[1] and knobs[2].
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fa93854f7a7ed63d054405bf3779247d5300edd3
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=813cab8f3994250e136819ae48fbd1c95d980466
>
> V2:
> Add sysfs documentation.
> Reorder ASUS_WMI_CHARGE_THRESHOLD and rename to ASUS_WMI_DEVID_RSOC.
> Add a comment explaining the charge_threshold variable.
> Rephrase the commit message (charge threshold -> maximum charging
> percentage).
>
> The sysfs knob is still called "charge_threshold", as
> maximum_charging_percentage seems a bit long.
> I did look on some of the other platform modules, the LG module
> use battery_care_limit and the Samsung module use
> battery_life_extender.
>
> drivers/platform/x86/asus-wmi.c | 91 ++++++++++++++++++++++
> include/linux/platform_data/x86/asus-wmi.h | 3 +
> 2 files changed, 94 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 34dfbed65332..06c830c1c04f 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -26,6 +26,7 @@
> #include <linux/rfkill.h>
> #include <linux/pci.h>
> #include <linux/pci_hotplug.h>
> +#include <linux/power_supply.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> #include <linux/debugfs.h>
> @@ -36,6 +37,7 @@
> #include <linux/acpi.h>
> #include <linux/dmi.h>
> #include <acpi/video.h>
> +#include <acpi/battery.h>
>
> #include "asus-wmi.h"
>
> @@ -368,6 +370,92 @@ static bool asus_wmi_dev_is_present(struct asus_wmi *asus, u32 dev_id)
> return status == 0 && (retval & ASUS_WMI_DSTS_PRESENCE_BIT);
> }
>
> +/* Battery ********************************************************************/
> +
> +/* The battery maximum charging percentage */
> +static int charge_end_threshold;
> +
> +static ssize_t charge_control_end_threshold_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int value, ret, rv;
> +
> + ret = kstrtouint(buf, 10, &value);
> +
> + if (!count || ret != 0)
> + return -EINVAL;
> + if (value < 0 || value > 100)
> + return -EINVAL;
> +
> + asus_wmi_set_devstate(ASUS_WMI_DEVID_RSOC, value, &rv);
> +
> + if (rv != 1)
> + return -EIO;
> +
> + /* There isn't any method in the DSDT to read the threshold, so we
> + * save the threshold.
> + */
> + charge_end_threshold = value;
> + return count;
> +}
> +
> +static ssize_t charge_control_end_threshold_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%d\n", charge_end_threshold);
> +}
> +
> +static DEVICE_ATTR_RW(charge_control_end_threshold);
> +
> +static int asus_wmi_battery_add(struct power_supply *battery)
> +{
> + /* The WMI method does not provide a way to specific a battery, so we
> + * just assume it is the first battery.
> + */
> + if (!strcmp(battery->desc->name, "BAT0") == 0)
> + return -ENODEV;
> +
> + if (device_create_file(&battery->dev,
> + &dev_attr_charge_control_end_threshold))
> + return -ENODEV;
> +
> + /* The charge threshold is only reset when the system is power cycled,
> + * and we can't get the current threshold so let set it to 100% when
> + * a battery is added.
> + */
> + asus_wmi_set_devstate(ASUS_WMI_DEVID_RSOC, 100, NULL);
> + charge_end_threshold = 100;
> +
> + return 0;
> +}
> +
> +static int asus_wmi_battery_remove(struct power_supply *battery)
> +{
> + device_remove_file(&battery->dev,
> + &dev_attr_charge_control_end_threshold);
> + return 0;
> +}
> +
> +static struct acpi_battery_hook battery_hook = {
> + .add_battery = asus_wmi_battery_add,
> + .remove_battery = asus_wmi_battery_remove,
> + .name = "ASUS Battery Extension",
> +};
> +
> +static void asus_wmi_battery_init(struct asus_wmi *asus)
> +{
> + if (asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_RSOC) >= 0)
> + battery_hook_register(&battery_hook);
> +}
asus_wmi_get_devstate_simple() checks ASUS_WMI_DSTS_STATUS_BIT and
ASUS_WMI_DSTS_UNKNOWN_BIT for this device. However the spec makes no
mentions of those bits being valid in this case.
Your code probably works anyway but I think it would be better to use
asus_wmi_dev_is_present() instead.
> +static void asus_wmi_battery_exit(struct asus_wmi *asus)
> +{
> + if (asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_RSOC) >= 0)
> + battery_hook_unregister(&battery_hook);
> +}
The unregister here should probably happen on the condition that we
previously registered the hook.
Don't rely on the devstate bits, since they might have changed
unpredictably (who knows), there might be a problem communicating with
the hardware, etc.
> /* LEDs ***********************************************************************/
>
> /*
> @@ -2433,6 +2521,8 @@ static int asus_wmi_add(struct platform_device *pdev)
> goto fail_wmi_handler;
> }
>
> + asus_wmi_battery_init(asus);
> +
> asus_wmi_debugfs_init(asus);
>
> return 0;
> @@ -2468,6 +2558,7 @@ static int asus_wmi_remove(struct platform_device *device)
> asus_wmi_debugfs_exit(asus);
> asus_wmi_sysfs_exit(asus->platform_device);
> asus_fan_set_auto(asus);
> + asus_wmi_battery_exit(asus);
>
> kfree(asus);
> return 0;
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 409e16064f4b..60249e22e844 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -81,6 +81,9 @@
> /* Deep S3 / Resume on LID open */
> #define ASUS_WMI_DEVID_LID_RESUME 0x00120031
>
> +/* Maximum charging percentage */
> +#define ASUS_WMI_DEVID_RSOC 0x00120057
> +
> /* DSTS masks */
> #define ASUS_WMI_DSTS_STATUS_BIT 0x00000001
> #define ASUS_WMI_DSTS_UNKNOWN_BIT 0x00000002
> --
> 2.22.0
>
