| Message ID | 20190805192305.9692-1-kristian@klausen.dk (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: asus-wmi: Add support for charge threshold | expand |
On Tue, Aug 6, 2019 at 3:23 AM Kristian Klausen <kristian@klausen.dk> wrote: > > Most newer ASUS laptops supports limiting the battery charge level, which > help prolonging the battery life. > > Tested on a Zenbook UX430UNR. > > Signed-off-by: Kristian Klausen <kristian@klausen.dk> > --- > I'm not sure what to call it, is charge threshold good enough or do you > have any better ideas? The spec calls it "Maximum Charging Percentage". I think something along those lines would be clearer. It would be good to have a one line explanatory comment against where you introduce the variable, so that it's clear from glancing at the code (not just the commit message) what this functionality actually is. Also you need a Documentation/ entry for this new file where you can describe it a little more verbosely. The 0x00120057 device is officially called "Relative state of charge", not sure if you want to somehow follow a similar naming scheme there, I don't have a real preference. > /* Misc */ > #define ASUS_WMI_DEVID_CAMERA 0x00060013 > +#define ASUS_WMI_CHARGE_THRESHOLD 0x00120057 Please keep the devices ordered numerically by their ID Thanks Daniel
On 08.08.2019 10.13, Daniel Drake wrote: > On Tue, Aug 6, 2019 at 3:23 AM Kristian Klausen <kristian@klausen.dk> wrote: >> Most newer ASUS laptops supports limiting the battery charge level, which >> help prolonging the battery life. >> >> Tested on a Zenbook UX430UNR. >> >> Signed-off-by: Kristian Klausen <kristian@klausen.dk> >> --- >> I'm not sure what to call it, is charge threshold good enough or do you >> have any better ideas? > The spec calls it "Maximum Charging Percentage". I think something > along those lines would be clearer. I just sent a V2 patch. I'm using "maximum charging percentage" in the documentation/commit message, but the knob is still named "charge_threshold". > It would be good to have a one line explanatory comment against where > you introduce the variable, so that it's clear from glancing at the > code (not just the commit message) what this functionality actually > is.documentation Ack > Also you need a Documentation/ entry for this new file where you can > describe it a little more verbosely. Ack > The 0x00120057 device is officially called "Relative state of charge", > not sure if you want to somehow follow a similar naming scheme there, > I don't have a real preference. The same name is used in ASUS online documentation. I change it. https://www.asus.com/us/support/FAQ/1032726/ The current name also seems to lack DEVID. >> /* Misc */ >> #define ASUS_WMI_DEVID_CAMERA 0x00060013 >> +#define ASUS_WMI_CHARGE_THRESHOLD 0x00120057 > Please keep the devices ordered numerically by their ID Ack Kristian
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 34dfbed65332..22ae350e0a96 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -195,6 +195,8 @@ struct asus_wmi { u8 fan_boost_mode_mask; u8 fan_boost_mode; + int charge_threshold; + struct hotplug_slot hotplug_slot; struct mutex hotplug_lock; struct mutex wmi_lock; @@ -2075,6 +2077,43 @@ static ssize_t cpufv_store(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR_WO(cpufv); + +static ssize_t charge_threshold_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct asus_wmi *asus = dev_get_drvdata(dev); + 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_CHARGE_THRESHOLD, value, &rv); + + if (rv != 1) + return -EIO; + + /* There isn't any method in the DSDT to read the threshold, so we + * save the threshold. + */ + asus->charge_threshold = value; + return count; +} + +static ssize_t charge_threshold_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct asus_wmi *asus = dev_get_drvdata(dev); + + return sprintf(buf, "%d\n", asus->charge_threshold); +} + +static DEVICE_ATTR_RW(charge_threshold); + static struct attribute *platform_attributes[] = { &dev_attr_cpufv.attr, &dev_attr_camera.attr, @@ -2083,6 +2122,7 @@ static struct attribute *platform_attributes[] = { &dev_attr_lid_resume.attr, &dev_attr_als_enable.attr, &dev_attr_fan_boost_mode.attr, + &dev_attr_charge_threshold.attr, NULL }; @@ -2106,6 +2146,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj, devid = ASUS_WMI_DEVID_ALS_ENABLE; else if (attr == &dev_attr_fan_boost_mode.attr) ok = asus->fan_boost_mode_available; + else if (attr == &dev_attr_charge_threshold.attr) + devid = ASUS_WMI_CHARGE_THRESHOLD; if (devid != -1) ok = !(asus_wmi_get_devstate_simple(asus, devid) < 0); @@ -2434,6 +2476,12 @@ static int asus_wmi_add(struct platform_device *pdev) } asus_wmi_debugfs_init(asus); + /* 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% on + * module load. + */ + asus_wmi_set_devstate(ASUS_WMI_CHARGE_THRESHOLD, 100, NULL); + asus->charge_threshold = 100; return 0; diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h index 409e16064f4b..53934ef38d98 100644 --- a/include/linux/platform_data/x86/asus-wmi.h +++ b/include/linux/platform_data/x86/asus-wmi.h @@ -61,6 +61,7 @@ /* Misc */ #define ASUS_WMI_DEVID_CAMERA 0x00060013 +#define ASUS_WMI_CHARGE_THRESHOLD 0x00120057 /* Storage */ #define ASUS_WMI_DEVID_CARDREADER 0x00080013
Most newer ASUS laptops supports limiting the battery charge level, which help prolonging the battery life. Tested on a Zenbook UX430UNR. Signed-off-by: Kristian Klausen <kristian@klausen.dk> --- I'm not sure what to call it, is charge threshold good enough or do you have any better ideas? drivers/platform/x86/asus-wmi.c | 48 ++++++++++++++++++++++ include/linux/platform_data/x86/asus-wmi.h | 1 + 2 files changed, 49 insertions(+)