On Tue, Dec 31, 2013 at 11:46:05PM +0100, Julian Andres Klode wrote:
> On Tue, Dec 31, 2013 at 10:12:31AM -0200, Henrique de Moraes Holschuh wrote:
> > On Tue, 31 Dec 2013, Julian Andres Klode wrote:
> > > We might be able to work around this by simple setting stop = start
> > > if a new write causes the stop threshold to be below the start
> > > threshold. But this also seems ugly.
> > 
> > It is the safest way, but the correct pseudo-code would be, assuiming
> > unsigned:
> > 
> > when someone changes start:
> > 
> > if (start > 99)
> >     start = 99;
> 
> I think we should just return -EINVAL in such cases. Allowing users to
> write larger percentages is a bit pointless (we don't allow them to write
> negative ones either). And other promiment code (the backlight drivers)
> seem to reject out-of-range values.
> 
> > set_thresholds(start, stop);
> 
> I think there should not be some common set_thresholds, because we also
> need to write things in different orders for start / stop then:
> 
>       DECREASE STOP  => Write new start if needed, then write stop
>       INCREASE START => Write new stop if needed, then write start
> 
> Otherwise we might have a very very very short time in which start
> is greater than stop.
> 
> I'll incorporate this in real code and test it tomorrow. 
> 

OK, tommorow is now, 4 months later. I was to busy with
lectures. An updated patch is below that passes my small
test suite.

The next step is to integrate this properly with power supply
and/or acpi battery. One way would be to add additional power
supply properties and then add get/set_property() pointers to
the acpi battery which it can fall back to if it does not support
a requested property (and we would locate the ACPI battery and
set those pointers to new thinkpad_acpi functions).

Full changelog:
* Changed stop interval to 1..100
* Fixed name of attributes (tresh => thresh)
* Made sure to keep start < stop

-- >8 --
Subject: [PATCH 1/4] thinkpad_acpi: Add support for controlling charge
 thresholds

Add support for battery charge thresholds in new Sandy Bridge and Ivy Bridge
ThinkPads. Based on the unofficial documentation in tpacpi-bat.

Signed-off-by: Julian Andres Klode <j...@jak-linux.org>
---
 Documentation/laptops/thinkpad-acpi.txt |  15 ++
 drivers/platform/x86/thinkpad_acpi.c    | 235 ++++++++++++++++++++++++++++++++
 2 files changed, 250 insertions(+)

diff --git a/Documentation/laptops/thinkpad-acpi.txt 
b/Documentation/laptops/thinkpad-acpi.txt
index 86c5236..9b8a637 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
@@ -46,6 +46,7 @@ detailed description):
        - Fan control and monitoring: fan speed, fan enable/disable
        - WAN enable and disable
        - UWB enable and disable
+       - Charging control
 
 A compatibility table by model and feature is maintained on the web
 site, http://ibm-acpi.sf.net/. I appreciate any success or failure
@@ -1352,6 +1353,20 @@ Sysfs notes:
        rfkill controller switch "tpacpi_uwb_sw": refer to
        Documentation/rfkill.txt for details.
 
+Charging control
+----------------
+sysfs attribute groups: BAT0, BAT1, and so on, depending on how many batteries
+are supported by the embedded controller.
+
+This feature controls the battery charging process.
+
+battery sysfs attribute: start_charge_thresh
+
+       A percentage value from 0-99%, controlling when charging should start
+
+battery sysfs attribute: stop_charge_thresh
+
+       A percentage value from 1-100%, controlling when charging should stop
 
 Multiple Commands, Module Parameters
 ------------------------------------
diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index 03ca6c1..fa29d82 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -323,6 +323,7 @@ static struct {
        u32 sensors_pdrv_attrs_registered:1;
        u32 sensors_pdev_attrs_registered:1;
        u32 hotkey_poll_active:1;
+       u32 battery:1;
 } tp_features;
 
 static struct {
@@ -8350,6 +8351,236 @@ static struct ibm_struct fan_driver_data = {
        .resume = fan_resume,
 };
 
+
+/*************************************************************************
+ * Battery subdriver
+ */
+
+/* Modify battery_init() if you modify them */
+#define BATTERY_MAX_COUNT 3
+#define BATTERY_MAX_ATTRS 2
+
+static struct battery {
+       char name[3 + 1 + 1];
+       struct attribute_set *set;
+       struct dev_ext_attribute attributes[BATTERY_MAX_ATTRS];
+} batteries[BATTERY_MAX_COUNT];
+
+static int battery_attribute_get_battery(struct device_attribute *attr)
+{
+       return (int) (unsigned long) container_of(attr,
+                                                 struct dev_ext_attribute,
+                                                 attr)->var;
+}
+
+static int battery_read_threshold(int bat, char *threshold)
+{
+       int result = 0;
+
+       if (!hkey_handle || !acpi_evalf(hkey_handle, &result, threshold,
+                                       "dd", bat) || result < 0)
+               return -EIO;
+
+       /* Translate 0 to 100 for stop threshold */
+       if (strcmp(threshold, "BCSG") == 0 && (result & 0xFF) == 0)
+               return 100;
+       /* Bits 0 - 7 contain the current threshold */
+       return result & 0xFF;
+}
+
+static int battery_write_stop_charge_thresh(int bat, int value,
+                                            bool adjust_start);
+
+static int battery_write_start_charge_thresh(int bat, int value,
+                                            bool adjust_stop)
+{
+       int res = 0;
+
+       if (value < 0 || value > 99)
+               return -EINVAL;
+
+       /* Adjust the stop value if stop < new start value */
+       if (adjust_stop) {
+               int stop = battery_read_threshold(bat, "BCSG");
+               if (stop < 0)
+                       return stop;
+               if (stop <= value)
+                       res = battery_write_stop_charge_thresh(bat, value + 1,
+                                                              FALSE);
+               if (res)
+                       return res;
+       }
+
+       if (!hkey_handle || !acpi_evalf(hkey_handle, &res, "BCCS", "dd",
+                                       value | (bat << 8)) || res < 0)
+               return -EIO;
+
+       return 0;
+}
+
+static int battery_write_stop_charge_thresh(int bat, int value,
+                                            bool adjust_start)
+{
+       int res = 0;
+
+       if (value < 1 || value > 100)
+               return -EINVAL;
+
+       /* Adjust the start value if start > new stop value */
+       if (adjust_start) {
+               int start = battery_read_threshold(bat, "BCTG");
+               if (start < 0)
+                       return start;
+               if (value != 0)
+                       res = battery_write_start_charge_thresh(bat, value - 1,
+                                                               FALSE);
+               if (res)
+                       return res;
+       }
+
+       /* 0 means default which seems to be 100%. */
+       if (value == 100)
+               value = 0;
+
+       if (!hkey_handle || !acpi_evalf(hkey_handle, &res, "BCSS", "dd",
+                                       value | (bat << 8)) || res < 0)
+               return -EIO;
+
+       return 0;
+}
+
+static ssize_t battery_start_charge_thresh_store(struct device *dev,
+                                                struct device_attribute *attr,
+                                                const char *buf, size_t count)
+{
+       int bat = battery_attribute_get_battery(attr);
+       int res = -EINVAL;
+       unsigned long value;
+
+       res = kstrtoul(buf, 0, &value);
+       if (res)
+               return res;
+       res = battery_write_start_charge_thresh(bat, value, TRUE);
+       return res ? res : count;
+}
+
+static ssize_t battery_stop_charge_thresh_store(struct device *dev,
+                                               struct device_attribute *attr,
+                                               const char *buf, size_t count)
+{
+       int bat = battery_attribute_get_battery(attr);
+       int res = -EINVAL;
+       unsigned long value;
+
+       res = kstrtoul(buf, 0, &value);
+       if (res)
+               return res;
+       res = battery_write_stop_charge_thresh(bat, value, TRUE);
+       return res ? res : count;
+}
+
+static ssize_t battery_start_charge_thresh_show(struct device *dev,
+                                               struct device_attribute *attr,
+                                               char *buf)
+{
+       int bat = battery_attribute_get_battery(attr);
+       int value = battery_read_threshold(bat, "BCTG");
+
+       return value < 0 ? value : snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static ssize_t battery_stop_charge_thresh_show(struct device *dev,
+                                              struct device_attribute *attr,
+                                              char *buf)
+{
+       int bat = battery_attribute_get_battery(attr);
+       int value = battery_read_threshold(bat, "BCSG");
+
+       return value < 0 ? value : snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static int __init battery_init(struct ibm_init_struct *iibm)
+{
+       int res;
+       int i;
+       int state;
+
+       vdbg_printk(TPACPI_DBG_INIT,
+                   "initializing battery commands subdriver\n");
+
+       TPACPI_ACPIHANDLE_INIT(hkey);
+
+       /* Check whether getter for start threshold exists */
+       tp_features.battery = hkey_handle &&
+           acpi_evalf(hkey_handle, &state, "BCTG", "qdd", 1);
+
+       vdbg_printk(TPACPI_DBG_INIT, "battery commands are %s\n",
+                   str_supported(tp_features.battery));
+
+       if (!tp_features.battery)
+               return 1;
+
+       for (i = 0; i < BATTERY_MAX_COUNT; i++) {
+               int j = 0;
+               if (!acpi_evalf(hkey_handle, &state, "BCTG", "qdd", i + 1))
+                       continue;
+               /* If the sign bit was set, we could not get the start charge
+                * threshold of that battery. Let's assume that this battery
+                * (and all following ones) do not exist */
+               if (state < 0)
+                       break;
+               /* Modify BATTERY_MAX_ATTRS if you add an attribute */
+               batteries[i].attributes[j++] = (struct dev_ext_attribute) {
+                       .attr = __ATTR(start_charge_thresh,
+                                      S_IWUSR | S_IRUGO,
+                                      battery_start_charge_thresh_show,
+                                      battery_start_charge_thresh_store),
+                       .var = (void *)(unsigned long) (i + 1)
+               };
+               batteries[i].attributes[j++] = (struct dev_ext_attribute) {
+                       .attr = __ATTR(stop_charge_thresh,
+                                      S_IWUSR | S_IRUGO,
+                                      battery_stop_charge_thresh_show,
+                                      battery_stop_charge_thresh_store),
+                       .var = (void *)(unsigned long) (i + 1)
+               };
+
+               strncpy(batteries[i].name, "BAT", 3);
+               batteries[i].name[3] = '0' + i;
+               batteries[i].name[4] = '\0';
+               batteries[i].set = create_attr_set(j, batteries[i].name);
+
+               for (j = j - 1; j >= 0; j--)
+                       add_to_attr_set(batteries[i].set,
+                                       &batteries[i].attributes[j].attr.attr);
+
+               res = register_attr_set_with_sysfs(batteries[i].set,
+                                                  &tpacpi_pdev->dev.kobj);
+
+               if (res)
+                       return res;
+       }
+
+       return 0;
+}
+
+static void battery_exit(void)
+{
+       int i;
+
+       for (i = 0; i < BATTERY_MAX_COUNT; i++) {
+               if (batteries[i].set != NULL) {
+                       delete_attr_set(batteries[i].set,
+                                       &tpacpi_pdev->dev.kobj);
+               }
+       }
+}
+
+static struct ibm_struct battery_driver_data = {
+       .name = "battery",
+       .exit = battery_exit,
+};
+
 /****************************************************************************
  ****************************************************************************
  *
@@ -8741,6 +8972,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
                .data = &light_driver_data,
        },
        {
+               .init = battery_init,
+               .data = &battery_driver_data,
+       },
+       {
                .init = cmos_init,
                .data = &cmos_driver_data,
        },
-- 
1.9.1



-- 
Julian Andres Klode  - Debian Developer, Ubuntu Member

See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.

Please do not top-post if possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to