hid-input: add support for HID devices reporting Battery Strength
Daniel Nicoletti [Fri, 2 Dec 2011 05:52:22 +0000 (03:52 -0200)]
I've sent an email earlier asking for help with a GetFeature code, and now I
have a second patch on top of Jeremy's to provide the battery functionality
for devices that support reporting it.

If I understood correctly when talking to Jeremy he said his device
never actually reported the status as an input event (sorry if I didn't
understand it correctly), and after reading HID specs I believe it's
really because it was meant to be probed, I have an Apple Keyboard and
Magic Trackpad both bluetooth batteries operated, so using PacketLogger
I saw that Mac OSX always ask the battery status using the so called
GetFeature.

What my patch does is basically:
- store the report id that matches the battery_strength
- setup the battery if 0x6.0x20 is found, even if that is reported as a feature
  (as it was meant to be but only the MagicTrackpad does)
- when upower or someone access /sys/class/power_supply/hid-*/capacity it
  will probe the device and return it's status.

It works great for both devices, but I have two concerns:
- the report_features function has a duplicated code
- it would be nice if it was possible for specific drivers to provide their own
  probe as there might be some strange devices... (but maybe it's
already possible)

I've talked to the upower dev and he fixed it to be able to show the
right percentage.

Here how the uevent file (in /sys/class/power_supply/hid-*/) looks like:
POWER_SUPPLY_NAME=hid-00:22:41:D9:18:E7-battery
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_CAPACITY=66
POWER_SUPPLY_MODEL_NAME=MacAdmin’s keyboard
POWER_SUPPLY_STATUS=Discharging

POWER_SUPPLY_NAME=hid-70:CD:60:F5:FF:3F-battery
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_CAPACITY=62
POWER_SUPPLY_MODEL_NAME=nexx’s Trackpad
POWER_SUPPLY_STATUS=Discharging

Signed-off-by: Daniel Nicoletti <dantti12@gmail.com>

drivers/hid/hid-input.c
include/linux/hid.h

index b9b8c75..8fac47c 100644 (file)
@@ -277,6 +277,7 @@ static enum power_supply_property hidinput_battery_props[] = {
        POWER_SUPPLY_PROP_ONLINE,
        POWER_SUPPLY_PROP_CAPACITY,
        POWER_SUPPLY_PROP_MODEL_NAME,
+       POWER_SUPPLY_PROP_STATUS
 };
 
 static int hidinput_get_battery_property(struct power_supply *psy,
@@ -285,6 +286,9 @@ static int hidinput_get_battery_property(struct power_supply *psy,
 {
        struct hid_device *dev = container_of(psy, struct hid_device, battery);
        int ret = 0;
+       int ret_rep;
+       __u8 *buf = NULL;
+       unsigned char report_number = dev->battery_report_id;
 
        switch (prop) {
        case POWER_SUPPLY_PROP_PRESENT:
@@ -293,28 +297,45 @@ static int hidinput_get_battery_property(struct power_supply *psy,
                break;
 
        case POWER_SUPPLY_PROP_CAPACITY:
-               if (dev->battery_min < dev->battery_max &&
-                   dev->battery_val >= dev->battery_min &&
-                   dev->battery_val <= dev->battery_max)
-                       val->intval = (100 * (dev->battery_val - dev->battery_min)) /
-                               (dev->battery_max - dev->battery_min);
-               else
+               buf = kmalloc(2 * sizeof(__u8), GFP_KERNEL);
+               if (!buf) {
+                       ret = -ENOMEM;
+                       break;
+               }
+
+               memset(buf, 0, sizeof(buf));
+               ret_rep = dev->hid_get_raw_report(dev, report_number, buf, sizeof(buf), HID_FEATURE_REPORT);
+               if (ret_rep != 2) {
                        ret = -EINVAL;
+                       break;
+               }
+
+               /* store the returned value */
+               /* I'm not calculating this using the logical_minimum and maximum */
+               /* because my device returns 0-100 even though the min and max are 0-255 */
+               val->intval = buf[1];
                break;
 
        case POWER_SUPPLY_PROP_MODEL_NAME:
                val->strval = dev->name;
                break;
 
+       case POWER_SUPPLY_PROP_STATUS:
+               val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+               break;
+
        default:
                ret = -EINVAL;
                break;
        }
 
+       if (buf) {
+               kfree(buf);
+       }
        return ret;
 }
 
-static void hidinput_setup_battery(struct hid_device *dev, s32 min, s32 max)
+static void hidinput_setup_battery(struct hid_device *dev, unsigned id, s32 min, s32 max)
 {
        struct power_supply *battery = &dev->battery;
        int ret;
@@ -326,7 +347,7 @@ static void hidinput_setup_battery(struct hid_device *dev, s32 min, s32 max)
        if (battery->name == NULL)
                return;
 
-       battery->type = POWER_SUPPLY_TYPE_BATTERY;
+       battery->type = POWER_SUPPLY_TYPE_USB;
        battery->properties = hidinput_battery_props;
        battery->num_properties = ARRAY_SIZE(hidinput_battery_props);
        battery->use_for_apm = 0;
@@ -334,6 +355,7 @@ static void hidinput_setup_battery(struct hid_device *dev, s32 min, s32 max)
 
        dev->battery_min = min;
        dev->battery_max = max;
+       dev->battery_report_id = id;
 
        ret = power_supply_register(&dev->dev, battery);
        if (ret != 0) {
@@ -353,7 +375,7 @@ static void hidinput_cleanup_battery(struct hid_device *dev)
        dev->battery.name = NULL;
 }
 #else  /* !CONFIG_HID_BATTERY_STRENGTH */
-static void hidinput_setup_battery(struct hid_device *dev, s32 min, s32 max)
+static void hidinput_setup_battery(struct hid_device *dev, unsigned id, s32 min, s32 max)
 {
 }
 
@@ -723,6 +745,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
        case HID_UP_GENDEVCTRLS:
                if ((usage->hid & HID_USAGE) == 0x20) { /* Battery Strength */
                        hidinput_setup_battery(device,
+                                              field->report->id,
                                               field->logical_minimum,
                                               field->logical_maximum);
                        goto ignore;
@@ -997,15 +1020,23 @@ static void report_features(struct hid_device *hid)
        struct hid_report *rep;
        int i, j;
 
-       if (!drv->feature_mapping)
-               return;
-
        rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
        list_for_each_entry(rep, &rep_enum->report_list, list)
                for (i = 0; i < rep->maxfield; i++)
-                       for (j = 0; j < rep->field[i]->maxusage; j++)
-                               drv->feature_mapping(hid, rep->field[i],
-                                                    rep->field[i]->usage + j);
+                       for (j = 0; j < rep->field[i]->maxusage; j++) {
+                               /* Verify if Battery Strength feature is available */
+                               if (((rep->field[i]->usage + j)->hid & HID_USAGE_PAGE) == HID_UP_GENDEVCTRLS &&
+                                       ((rep->field[i]->usage + j)->hid & HID_USAGE) == 0x20) {
+                                       hidinput_setup_battery(hid,
+                                                              rep->id,
+                                                              rep->field[i]->logical_minimum,
+                                                              rep->field[i]->logical_maximum);
+                               }
+
+                               if (drv->feature_mapping)
+                                       drv->feature_mapping(hid, rep->field[i],
+                                                            rep->field[i]->usage + j);
+                       }
 }
 
 /*
index 7f344c3..b5df198 100644 (file)
@@ -496,6 +496,7 @@ struct hid_device {                                                 /* device report descriptor */
        __s32 battery_min;
        __s32 battery_max;
        __s32 battery_val;
+       __s32 battery_report_id;
 #endif
 
        unsigned int status;                                            /* see STAT flags above */