Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 7, 2018 at 3:58 PM, Ognjen Galic <smclt30p@xxxxxxxxx> wrote:
> 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.
>
> Signed-off-by: Ognjen Galic <smclt30p@xxxxxxxxx>

Andy, Darren, Henrique,

I'm assuming no concerns about this you from you, please let me know
if that's not correct.

> ---
>
> Notes:
>     v2:
>     * Re-write the patch to make the changes in
>     battery.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
>
>     v12:
>     * Remove most whitespaces between ifs
>     * Change int to acpi_status in tpacpi_battery_acpi_eval
>
>     v13:
>     * Change if (!tpacpi_... to if ACPI_FAILURE(...
>
>  drivers/platform/x86/Kconfig         |   1 +
>  drivers/platform/x86/thinkpad_acpi.c | 389 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 389 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f96465..0d13d30c1 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -425,6 +425,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
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index d5eaf3b1e..1c57ee2b6 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -23,7 +23,7 @@
>
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> -#define TPACPI_VERSION "0.25"
> +#define TPACPI_VERSION "0.26"
>  #define TPACPI_SYSFS_VERSION 0x030000
>
>  /*
> @@ -66,6 +66,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/sysfs.h>
>  #include <linux/backlight.h>
> +#include <linux/bitops.h>
>  #include <linux/fb.h>
>  #include <linux/platform_device.h>
>  #include <linux/hwmon.h>
> @@ -78,11 +79,13 @@
>  #include <linux/workqueue.h>
>  #include <linux/acpi.h>
>  #include <linux/pci_ids.h>
> +#include <linux/power_supply.h>
>  #include <linux/thinkpad_acpi.h>
>  #include <sound/core.h>
>  #include <sound/control.h>
>  #include <sound/initval.h>
>  #include <linux/uaccess.h>
> +#include <acpi/battery.h>
>  #include <acpi/video.h>
>
>  /* ThinkPad CMOS commands */
> @@ -335,6 +338,7 @@ static struct {
>         u32 sensors_pdev_attrs_registered:1;
>         u32 hotkey_poll_active:1;
>         u32 has_adaptive_kbd:1;
> +       u32 battery:1;
>  } tp_features;
>
>  static struct {
> @@ -9209,6 +9213,385 @@ static struct ibm_struct mute_led_driver_data = {
>         .resume = mute_led_resume,
>  };
>
> +/*
> + * Battery Wear Control Driver
> + * Contact: Ognjen Galic <smclt30p@xxxxxxxxx>
> + */
> +
> +/* Metadata */
> +
> +#define GET_START      "BCTG"
> +#define SET_START      "BCCS"
> +#define GET_STOP       "BCSG"
> +#define SET_STOP       "BCSS"
> +
> +#define START_ATTR "charge_start_threshold"
> +#define STOP_ATTR  "charge_stop_threshold"
> +
> +enum {
> +       BAT_ANY = 0,
> +       BAT_PRIMARY = 1,
> +       BAT_SECONDARY = 2
> +};
> +
> +enum {
> +       /* Error condition bit */
> +       METHOD_ERR = BIT(31),
> +};
> +
> +enum {
> +       /* This is used in the get/set helpers */
> +       THRESHOLD_START,
> +       THRESHOLD_STOP,
> +};
> +
> +struct tpacpi_battery_data {
> +       int charge_start;
> +       int start_support;
> +       int charge_stop;
> +       int stop_support;
> +};
> +
> +struct tpacpi_battery_driver_data {
> +       struct tpacpi_battery_data batteries[3];
> +       int individual_addressing;
> +};
> +
> +static struct tpacpi_battery_driver_data battery_info;
> +
> +/* ACPI helpers/functions/probes */
> +
> +/**
> + * 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.
> + */
> +static acpi_status tpacpi_battery_acpi_eval(char *method, int *ret, int param)
> +{
> +       int response;
> +
> +       if (!acpi_evalf(hkey_handle, &response, method, "dd", param)) {
> +               acpi_handle_err(hkey_handle, "%s: evaluate failed", method);
> +               return AE_ERROR;
> +       }
> +       if (response & METHOD_ERR) {
> +               acpi_handle_err(hkey_handle,
> +                               "%s evaluated but flagged as error", method);
> +               return AE_ERROR;
> +       }
> +       *ret = response;
> +       return AE_OK;
> +}
> +
> +static int tpacpi_battery_get(int what, int battery, int *ret)
> +{
> +       switch (what) {
> +       case THRESHOLD_START:
> +               if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_START, ret, battery))
> +                       return -ENODEV;
> +
> +               /* The value is in the low 8 bits of the response */
> +               *ret = *ret & 0xFF;
> +               return 0;
> +       case THRESHOLD_STOP:
> +               if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_STOP, ret, battery))
> +                       return -ENODEV;
> +               /* Value is in lower 8 bits */
> +               *ret = *ret & 0xFF;
> +               /*
> +                * 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_set(int what, int battery, int value)
> +{
> +       int param, ret;
> +       /* The first 8 bits are the value of the threshold */
> +       param = value;
> +       /* The battery ID is in bits 8-9, 2 bits */
> +       param |= battery << 8;
> +
> +       switch (what) {
> +       case THRESHOLD_START:
> +               if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_START, &ret, param)) {
> +                       pr_err("failed to set charge threshold on battery %d",
> +                                       battery);
> +                       return -ENODEV;
> +               }
> +               return 0;
> +       case THRESHOLD_STOP:
> +               if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_STOP, &ret, param)) {
> +                       pr_err("failed to set stop threshold: %d", battery);
> +                       return -ENODEV;
> +               }
> +               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 ACPI_FAILURE(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;
> +               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;
> +               }
> +       }
> +       if (acpi_has_method(hkey_handle, GET_STOP)) {
> +               if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_STOP, &ret, battery)) {
> +                       pr_err("Error probing battery stop; %d\n", battery);
> +                       return -ENODEV;
> +               }
> +               /* Support is marked in bit 8 */
> +               if (ret & BIT(8))
> +                       battery_info.batteries[battery].stop_support = 1;
> +               else
> +                       return -ENODEV;
> +               if (tpacpi_battery_get(THRESHOLD_STOP, battery,
> +                       &battery_info.batteries[battery].charge_stop)) {
> +                       pr_err("Error probing battery stop: %d\n", battery);
> +                       return -ENODEV;
> +               }
> +       }
> +       pr_info("battery %d registered (start %d, stop %d)",
> +                       battery,
> +                       battery_info.batteries[battery].charge_start,
> +                       battery_info.batteries[battery].charge_stop);
> +
> +       return 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,
> +                                   struct device *dev,
> +                                   const char *buf, size_t count)
> +{
> +       struct power_supply *supply = to_power_supply(dev);
> +       unsigned long value;
> +       int battery, rval;
> +       /*
> +        * Some systems have support for more than
> +        * one battery. If that is the case,
> +        * tpacpi_battery_probe marked that addressing
> +        * them individually is supported, so we do that
> +        * based on the device struct.
> +        *
> +        * On systems that are not supported, we assume
> +        * the primary as most of the ACPI calls fail
> +        * with "Any Battery" as the parameter.
> +        */
> +       if (battery_info.individual_addressing)
> +               /* BAT_PRIMARY or BAT_SECONDARY */
> +               battery = tpacpi_battery_get_id(supply->desc->name);
> +       else
> +               battery = BAT_PRIMARY;
> +
> +       rval = kstrtoul(buf, 10, &value);
> +       if (rval)
> +               return rval;
> +
> +       switch (what) {
> +       case THRESHOLD_START:
> +               if (!battery_info.batteries[battery].start_support)
> +                       return -ENODEV;
> +               /* valid values are [0, 99] */
> +               if (value < 0 || value > 99)
> +                       return -EINVAL;
> +               if (value > battery_info.batteries[battery].charge_stop)
> +                       return -EINVAL;
> +               if (tpacpi_battery_set(THRESHOLD_START, battery, value))
> +                       return -ENODEV;
> +               battery_info.batteries[battery].charge_start = value;
> +               return count;
> +
> +       case THRESHOLD_STOP:
> +               if (!battery_info.batteries[battery].stop_support)
> +                       return -ENODEV;
> +               /* valid values are [1, 100] */
> +               if (value < 1 || value > 100)
> +                       return -EINVAL;
> +               if (value < battery_info.batteries[battery].charge_start)
> +                       return -EINVAL;
> +               battery_info.batteries[battery].charge_stop = value;
> +               /*
> +                * When 100 is passed to stop, we need to flip
> +                * it to 0 as that the EC understands that as
> +                * "Default", which will charge to 100%
> +                */
> +               if (value == 100)
> +                       value = 0;
> +               if (tpacpi_battery_set(THRESHOLD_STOP, battery, value))
> +                       return -EINVAL;
> +               return count;
> +       default:
> +               pr_crit("Wrong parameter: %d", what);
> +               return -EINVAL;
> +       }
> +       return count;
> +}
> +
> +static ssize_t tpacpi_battery_show(int what,
> +                                  struct device *dev,
> +                                  char *buf)
> +{
> +       struct power_supply *supply = to_power_supply(dev);
> +       int ret, battery;
> +       /*
> +        * Some systems have support for more than
> +        * one battery. If that is the case,
> +        * tpacpi_battery_probe marked that addressing
> +        * them individually is supported, so we;
> +        * based on the device struct.
> +        *
> +        * On systems that are not supported, we assume
> +        * the primary as most of the ACPI calls fail
> +        * with "Any Battery" as the parameter.
> +        */
> +       if (battery_info.individual_addressing)
> +               /* BAT_PRIMARY or BAT_SECONDARY */
> +               battery = tpacpi_battery_get_id(supply->desc->name);
> +       else
> +               battery = BAT_PRIMARY;
> +       if (tpacpi_battery_get(what, battery, &ret))
> +               return -ENODEV;
> +       return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t charge_start_threshold_show(struct device *device,
> +                               struct device_attribute *attr,
> +                               char *buf)
> +{
> +       return tpacpi_battery_show(THRESHOLD_START, device, buf);
> +}
> +
> +static ssize_t charge_stop_threshold_show(struct device *device,
> +                               struct device_attribute *attr,
> +                               char *buf)
> +{
> +       return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
> +}
> +
> +static ssize_t charge_start_threshold_store(struct device *dev,
> +                               struct device_attribute *attr,
> +                               const char *buf, size_t count)
> +{
> +       return tpacpi_battery_store(THRESHOLD_START, dev, buf, count);
> +}
> +
> +static ssize_t charge_stop_threshold_store(struct device *dev,
> +                               struct device_attribute *attr,
> +                               const char *buf, size_t count)
> +{
> +       return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
> +}
> +
> +static DEVICE_ATTR_RW(charge_start_threshold);
> +static DEVICE_ATTR_RW(charge_stop_threshold);
> +
> +static struct attribute *tpacpi_battery_attrs[] = {
> +       &dev_attr_charge_start_threshold.attr,
> +       &dev_attr_charge_stop_threshold.attr,
> +       NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(tpacpi_battery);
> +
> +/* ACPI battery hooking */
> +
> +static int tpacpi_battery_add(struct power_supply *battery)
> +{
> +       int batteryid = tpacpi_battery_get_id(battery->desc->name);
> +
> +       if (tpacpi_battery_probe(batteryid))
> +               return -ENODEV;
> +       if (device_add_groups(&battery->dev, tpacpi_battery_groups))
> +               return -ENODEV;
> +       return 0;
> +}
> +
> +static int tpacpi_battery_remove(struct power_supply *battery)
> +{
> +       device_remove_groups(&battery->dev, tpacpi_battery_groups);
> +       return 0;
> +}
> +
> +static struct acpi_battery_hook battery_hook = {
> +       .add_battery = tpacpi_battery_add,
> +       .remove_battery = tpacpi_battery_remove,
> +       .name = "ThinkPad Battery Extension",
> +};
> +
> +/* Subdriver init/exit */
> +
> +static int __init tpacpi_battery_init(struct ibm_init_struct *ibm)
> +{
> +       battery_hook_register(&battery_hook);
> +       return 0;
> +}
> +
> +static void tpacpi_battery_exit(void)
> +{
> +       battery_hook_unregister(&battery_hook);
> +}
> +
> +static struct ibm_struct battery_driver_data = {
> +       .name = "battery",
> +       .exit = tpacpi_battery_exit,
> +};
> +
>  /****************************************************************************
>   ****************************************************************************
>   *
> @@ -9655,6 +10038,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
>                 .init = mute_led_init,
>                 .data = &mute_led_driver_data,
>         },
> +       {
> +               .init = tpacpi_battery_init,
> +               .data = &battery_driver_data,
> +       },
>  };
>
>  static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
> --
> 2.14.1
>



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux