greybus: power_supply: rework and operation changes
authorRui Miguel Silva <rui.silva@linaro.org>
Thu, 12 Nov 2015 15:36:02 +0000 (15:36 +0000)
committerGreg Kroah-Hartman <gregkh@google.com>
Fri, 13 Nov 2015 00:12:55 +0000 (16:12 -0800)
This is a major rework and changes to the current implementation of the
battery protocol. The previous implementation lack the support of a more
dynamic handle of power supply properties and updating of status. Also,
reflect the actual state of the greybus specification

So, with this new approach a set of operations to fetch the battery
module configuration and properties is add, new methods to cache and
update the values of properties, new operation to set properties if
declared writable and an event operation that can be triggered by the
module to force an update read on the properties values.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
drivers/staging/greybus/greybus_protocols.h
drivers/staging/greybus/kernel_ver.h
drivers/staging/greybus/power_supply.c

index b2380403edc64cb5ace309895e107cc0cf93c360..9fcbbe4eb6363dfb18aa4c23a1a250015d2eeecf 100644 (file)
@@ -205,17 +205,14 @@ struct gb_firmware_ready_to_boot_request {
 #define GB_POWER_SUPPLY_VERSION_MINOR          0x01
 
 /* Greybus power supply request types */
-#define GB_POWER_SUPPLY_TYPE_TECHNOLOGY                        0x02
-#define GB_POWER_SUPPLY_TYPE_STATUS                    0x03
-#define GB_POWER_SUPPLY_TYPE_MAX_VOLTAGE               0x04
-#define GB_POWER_SUPPLY_TYPE_PERCENT_CAPACITY          0x05
-#define GB_POWER_SUPPLY_TYPE_TEMPERATURE               0x06
-#define GB_POWER_SUPPLY_TYPE_VOLTAGE                   0x07
-#define GB_POWER_SUPPLY_TYPE_CURRENT                   0x08
-#define GB_POWER_SUPPLY_TYPE_CAPACITY                  0x09    // TODO - POWER_SUPPLY_PROP_CURRENT_MAX
-#define GB_POWER_SUPPLY_TYPE_SHUTDOWN_TEMP             0x0a    // TODO - POWER_SUPPLY_PROP_TEMP_ALERT_MAX
-
-/* Should match up with battery technology types in linux/power_supply.h */
+#define GB_POWER_SUPPLY_TYPE_GET_SUPPLIES              0x02
+#define GB_POWER_SUPPLY_TYPE_GET_DESCRIPTION           0x03
+#define GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS      0x04
+#define GB_POWER_SUPPLY_TYPE_GET_PROPERTY              0x05
+#define GB_POWER_SUPPLY_TYPE_SET_PROPERTY              0x06
+#define GB_POWER_SUPPLY_TYPE_EVENT                     0x07
+
+/* Should match up with battery technologies in linux/power_supply.h */
 #define GB_POWER_SUPPLY_TECH_UNKNOWN                   0x0000
 #define GB_POWER_SUPPLY_TECH_NiMH                      0x0001
 #define GB_POWER_SUPPLY_TECH_LION                      0x0002
@@ -224,35 +221,145 @@ struct gb_firmware_ready_to_boot_request {
 #define GB_POWER_SUPPLY_TECH_NiCd                      0x0005
 #define GB_POWER_SUPPLY_TECH_LiMn                      0x0006
 
-struct gb_power_supply_technology_response {
-       __le32  technology;
-} __packed;
-
-/* Should match up with power supply status in linux/power_supply.h */
-#define GB_POWER_SUPPLY_STATUS_UNKNOWN                 0x0000
-#define GB_POWER_SUPPLY_STATUS_CHARGING                        0x0001
-#define GB_POWER_SUPPLY_STATUS_DISCHARGING             0x0002
-#define GB_POWER_SUPPLY_STATUS_NOT_CHARGING            0x0003
-#define GB_POWER_SUPPLY_STATUS_FULL                    0x0004
-
-struct gb_power_supply_status_response {
-       __le16  psy_status;
-} __packed;
-
-struct gb_power_supply_max_voltage_response {
-       __le32  max_voltage;
-} __packed;
-
-struct gb_power_supply_capacity_response {
-       __le32  capacity;
-} __packed;
-
-struct gb_power_supply_temperature_response {
-       __le32  temperature;
-} __packed;
-
-struct gb_power_supply_voltage_response {
-       __le32  voltage;
+/* Should match up with power supply types in linux/power_supply.h */
+#define GB_POWER_SUPPLY_UNKNOWN_TYPE                   0x0000
+#define GB_POWER_SUPPLY_BATTERY_TYPE                   0x0001
+#define GB_POWER_SUPPLY_UPS_TYPE                       0x0002
+#define GB_POWER_SUPPLY_MAINS_TYPE                     0x0003
+#define GB_POWER_SUPPLY_USB_TYPE                       0x0004
+#define GB_POWER_SUPPLY_USB_DCP_TYPE                   0x0005
+#define GB_POWER_SUPPLY_USB_CDP_TYPE                   0x0006
+#define GB_POWER_SUPPLY_USB_ACA_TYPE                   0x0007
+
+/* Should match up with power supply health in linux/power_supply.h */
+#define GB_POWER_SUPPLY_HEALTH_UNKNOWN                 0x0000
+#define GB_POWER_SUPPLY_HEALTH_GOOD                    0x0001
+#define GB_POWER_SUPPLY_HEALTH_OVERHEAT                        0x0002
+#define GB_POWER_SUPPLY_HEALTH_DEAD                    0x0003
+#define GB_POWER_SUPPLY_HEALTH_OVERVOLTAGE             0x0004
+#define GB_POWER_SUPPLY_HEALTH_UNSPEC_FAILURE          0x0005
+#define GB_POWER_SUPPLY_HEALTH_COLD                    0x0006
+#define GB_POWER_SUPPLY_HEALTH_WATCHDOG_TIMER_EXPIRE   0x0007
+#define GB_POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE     0x0008
+
+/* Should match up with battery status in linux/power_supply.h */
+#define GB_POWER_SUPPLY_STATUS_UNKNOWN         0x0000
+#define GB_POWER_SUPPLY_STATUS_CHARGING                0x0001
+#define GB_POWER_SUPPLY_STATUS_DISCHARGING     0x0002
+#define GB_POWER_SUPPLY_STATUS_NOT_CHARGING    0x0003
+#define GB_POWER_SUPPLY_STATUS_FULL            0x0004
+
+struct gb_power_supply_get_supplies_response {
+       __u8    supplies_count;
+} __packed;
+
+struct gb_power_supply_get_description_request {
+       __u8    psy_id;
+} __packed;
+
+struct gb_power_supply_get_description_response {
+       __u8    manufacturer[32];
+       __u8    model[32];
+       __u8    serial_number[32];
+       __le16  type;
+       __u8    properties_count;
+} __packed;
+
+struct gb_power_supply_props_desc {
+       __u8    property;
+#define GB_POWER_SUPPLY_PROP_STATUS                            0x00
+#define GB_POWER_SUPPLY_PROP_CHARGE_TYPE                       0x01
+#define GB_POWER_SUPPLY_PROP_HEALTH                            0x02
+#define GB_POWER_SUPPLY_PROP_PRESENT                           0x03
+#define GB_POWER_SUPPLY_PROP_ONLINE                            0x04
+#define GB_POWER_SUPPLY_PROP_AUTHENTIC                         0x05
+#define GB_POWER_SUPPLY_PROP_TECHNOLOGY                                0x06
+#define GB_POWER_SUPPLY_PROP_CYCLE_COUNT                       0x07
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_MAX                       0x08
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_MIN                       0x09
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN                        0x0A
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN                        0x0B
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_NOW                       0x0C
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_AVG                       0x0D
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_OCV                       0x0E
+#define GB_POWER_SUPPLY_PROP_VOLTAGE_BOOT                      0x0F
+#define GB_POWER_SUPPLY_PROP_CURRENT_MAX                       0x10
+#define GB_POWER_SUPPLY_PROP_CURRENT_NOW                       0x11
+#define GB_POWER_SUPPLY_PROP_CURRENT_AVG                       0x12
+#define GB_POWER_SUPPLY_PROP_CURRENT_BOOT                      0x13
+#define GB_POWER_SUPPLY_PROP_POWER_NOW                         0x14
+#define GB_POWER_SUPPLY_PROP_POWER_AVG                         0x15
+#define GB_POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN                        0x16
+#define GB_POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN               0x17
+#define GB_POWER_SUPPLY_PROP_CHARGE_FULL                       0x18
+#define GB_POWER_SUPPLY_PROP_CHARGE_EMPTY                      0x19
+#define GB_POWER_SUPPLY_PROP_CHARGE_NOW                                0x1A
+#define GB_POWER_SUPPLY_PROP_CHARGE_AVG                                0x1B
+#define GB_POWER_SUPPLY_PROP_CHARGE_COUNTER                    0x1C
+#define GB_POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT           0x1D
+#define GB_POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX       0x1E
+#define GB_POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE           0x1F
+#define GB_POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX       0x20
+#define GB_POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT              0x21
+#define GB_POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX          0x22
+#define GB_POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT               0x23
+#define GB_POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN                        0x24
+#define GB_POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN               0x25
+#define GB_POWER_SUPPLY_PROP_ENERGY_FULL                       0x26
+#define GB_POWER_SUPPLY_PROP_ENERGY_EMPTY                      0x27
+#define GB_POWER_SUPPLY_PROP_ENERGY_NOW                                0x28
+#define GB_POWER_SUPPLY_PROP_ENERGY_AVG                                0x29
+#define GB_POWER_SUPPLY_PROP_CAPACITY                          0x2A
+#define GB_POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN                        0x2B
+#define GB_POWER_SUPPLY_PROP_CAPACITY_ALERT_MAX                        0x2C
+#define GB_POWER_SUPPLY_PROP_CAPACITY_LEVEL                    0x2D
+#define GB_POWER_SUPPLY_PROP_TEMP                              0x2E
+#define GB_POWER_SUPPLY_PROP_TEMP_MAX                          0x2F
+#define GB_POWER_SUPPLY_PROP_TEMP_MIN                          0x30
+#define GB_POWER_SUPPLY_PROP_TEMP_ALERT_MIN                    0x31
+#define GB_POWER_SUPPLY_PROP_TEMP_ALERT_MAX                    0x32
+#define GB_POWER_SUPPLY_PROP_TEMP_AMBIENT                      0x33
+#define GB_POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN            0x34
+#define GB_POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX            0x35
+#define GB_POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW                 0x36
+#define GB_POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG                 0x37
+#define GB_POWER_SUPPLY_PROP_TIME_TO_FULL_NOW                  0x38
+#define GB_POWER_SUPPLY_PROP_TIME_TO_FULL_AVG                  0x39
+#define GB_POWER_SUPPLY_PROP_TYPE                              0x3A
+#define GB_POWER_SUPPLY_PROP_SCOPE                             0x3B
+#define GB_POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT               0x3C
+#define GB_POWER_SUPPLY_PROP_CALIBRATE                         0x3D
+       __u8    is_writeable;
+} __packed;
+
+struct gb_power_supply_get_property_descriptors_request {
+       __u8    psy_id;
+} __packed;
+
+struct gb_power_supply_get_property_descriptors_response {
+       __u8    properties_count;
+       struct gb_power_supply_props_desc props[];
+} __packed;
+
+struct gb_power_supply_get_property_request {
+       __u8    psy_id;
+       __u8    property;
+} __packed;
+
+struct gb_power_supply_get_property_response {
+       __le32  prop_val;
+};
+
+struct gb_power_supply_set_property_request {
+       __u8    psy_id;
+       __u8    property;
+       __le32  prop_val;
+} __packed;
+
+struct gb_power_supply_event_request {
+       __u8    psy_id;
+       __u8    event;
+#define GB_POWER_SUPPLY_UPDATE         0x01
 } __packed;
 
 
index f3cb1895c092e40660ddb1382bd5b89832be1e1c..d9bf159329e5c63019e3f1e8baab9b9003d92035 100644 (file)
@@ -289,4 +289,11 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
 #include <media/v4l2-flash-led-class.h>
 #endif
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 1, 0)
+/*
+ * Power supply get by name need to drop reference after call
+ */
+#define PSY_HAVE_PUT
+#endif
+
 #endif /* __GREYBUS_KERNEL_VER_H */
index d7797a24e00dd27bc04da2c3f4b0581abaff15a6..4a7381650bb7321a801f6ad8ebbe147d76e2d37a 100644 (file)
 /*
  * Power Supply driver for a Greybus module.
  *
- * Copyright 2014 Google Inc.
- * Copyright 2014 Linaro Ltd.
+ * Copyright 2014-2015 Google Inc.
+ * Copyright 2014-2015 Linaro Ltd.
  *
  * Released under the GPLv2 only.
  */
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/slab.h>
 #include <linux/power_supply.h>
+#include <linux/slab.h>
+
 #include "greybus.h"
 
+#define PROP_MAX 32
+
+struct gb_power_supply_prop {
+       enum power_supply_property      prop;
+       u32                             val;
+       u32                             previous_val;
+       bool                            is_writeable;
+};
+
 struct gb_power_supply {
-       /*
-        * The power supply api changed in 4.1, so handle both the old
-        * and new apis in the same driver for now, until this is merged
-        * upstream, when all of these version checks can be removed.
-        */
+       u8                              id;
 #ifdef DRIVER_OWNS_PSY_STRUCT
-       struct power_supply psy;
+       struct power_supply             psy;
 #define to_gb_power_supply(x) container_of(x, struct gb_power_supply, psy)
 #else
-       struct power_supply *psy;
-       struct power_supply_desc desc;
+       struct power_supply             *psy;
+       struct power_supply_desc        desc;
 #define to_gb_power_supply(x) power_supply_get_drvdata(x)
 #endif
-       // FIXME
-       // we will want to keep the power supply stats in here as we will be
-       // getting updates from the SVC "on the fly" so we don't have to always
-       // go ask the power supply for some information. Hopefully...
-       struct gb_connection *connection;
+       char                            name[64];
+       struct gb_power_supplies        *supplies;
+       struct delayed_work             work;
+       char                            *manufacturer;
+       char                            *model_name;
+       char                            *serial_number;
+       u8                              type;
+       u8                              properties_count;
+       u8                              properties_count_str;
+       unsigned long                   last_update;
+       unsigned int                    update_interval;
+       bool                            changed;
+       struct gb_power_supply_prop     *props;
+       enum power_supply_property      *props_raw;
+};
+
+struct gb_power_supplies {
+       struct gb_connection    *connection;
+       u8                      supplies_count;
+       struct gb_power_supply  *supply;
+       struct mutex            supplies_lock;
+};
+
+/* cache time in milliseconds, if cache_time is set to 0 cache is disable */
+static unsigned int cache_time = 1000;
+/*
+ * update interval initial and maximum value, between the two will
+ * back-off exponential
+ */
+static unsigned int update_interval_init = 1 * HZ;
+static unsigned int update_interval_max = 30 * HZ;
+
+struct gb_power_supply_changes {
+       enum power_supply_property      prop;
+       u32                             tolerance_change;
+};
 
+static const struct gb_power_supply_changes psy_props_changes[] = {
+       {       .prop =                 GB_POWER_SUPPLY_PROP_STATUS,
+               .tolerance_change =     0,
+       },
+       {       .prop =                 GB_POWER_SUPPLY_PROP_TEMP,
+               .tolerance_change =     500,
+       },
+       {       .prop =                 GB_POWER_SUPPLY_PROP_ONLINE,
+               .tolerance_change =     0,
+       },
 };
 
-static int get_tech(struct gb_power_supply *gb)
+static struct gb_connection *get_conn_from_psy(struct gb_power_supply *gbpsy)
 {
-       struct gb_power_supply_technology_response tech_response;
-       u32 technology;
-       int retval;
+       return gbpsy->supplies->connection;
+}
 
-       retval = gb_operation_sync(gb->connection,
-                                  GB_POWER_SUPPLY_TYPE_TECHNOLOGY,
-                                  NULL, 0,
-                                  &tech_response, sizeof(tech_response));
-       if (retval)
-               return retval;
+static struct gb_power_supply_prop *get_psy_prop(struct gb_power_supply *gbpsy,
+                                                enum power_supply_property psp)
+{
+       int i;
 
-       /*
-        * Map greybus values to power_supply values.  Hopefully these are
-        * "identical" which should allow gcc to optimize the code away to
-        * nothing.
-        */
-       technology = le32_to_cpu(tech_response.technology);
-       switch (technology) {
-       case GB_POWER_SUPPLY_TECH_NiMH:
-               technology = POWER_SUPPLY_TECHNOLOGY_NiMH;
-               break;
-       case GB_POWER_SUPPLY_TECH_LION:
-               technology = POWER_SUPPLY_TECHNOLOGY_LION;
-               break;
-       case GB_POWER_SUPPLY_TECH_LIPO:
-               technology = POWER_SUPPLY_TECHNOLOGY_LIPO;
-               break;
-       case GB_POWER_SUPPLY_TECH_LiFe:
-               technology = POWER_SUPPLY_TECHNOLOGY_LiFe;
-               break;
-       case GB_POWER_SUPPLY_TECH_NiCd:
-               technology = POWER_SUPPLY_TECHNOLOGY_NiCd;
-               break;
-       case GB_POWER_SUPPLY_TECH_LiMn:
-               technology = POWER_SUPPLY_TECHNOLOGY_LiMn;
-               break;
-       case GB_POWER_SUPPLY_TECH_UNKNOWN:
-       default:
-               technology = POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
-               break;
+       for (i = 0; i < gbpsy->properties_count; i++)
+               if (gbpsy->props[i].prop == psp)
+                       return &gbpsy->props[i];
+       return NULL;
+}
+
+static int is_psy_prop_writeable(struct gb_power_supply *gbpsy,
+                                    enum power_supply_property psp)
+{
+       struct gb_power_supply_prop *prop;
+
+       prop = get_psy_prop(gbpsy, psp);
+       if (!prop)
+               return -ENOENT;
+       return prop->is_writeable ? 1 : 0;
+}
+
+static int is_prop_valint(enum power_supply_property psp)
+{
+       return ((psp < POWER_SUPPLY_PROP_MODEL_NAME) ? 1 : 0);
+}
+
+static void next_interval(struct gb_power_supply *gbpsy)
+{
+       if (gbpsy->update_interval == update_interval_max)
+               return;
+
+       /* do some exponential back-off in the update interval */
+       gbpsy->update_interval *= 2;
+       if (gbpsy->update_interval > update_interval_max)
+               gbpsy->update_interval = update_interval_max;
+}
+
+#ifdef DRIVER_OWNS_PSY_STRUCT
+static void __gb_power_supply_changed(struct gb_power_supply *gbpsy)
+{
+       power_supply_changed(&gbpsy->psy);
+}
+#else
+static void __gb_power_supply_changed(struct gb_power_supply *gbpsy)
+{
+       power_supply_changed(gbpsy->psy);
+}
+#endif
+
+static void check_changed(struct gb_power_supply *gbpsy,
+                         struct gb_power_supply_prop *prop)
+{
+       const struct gb_power_supply_changes *psyc;
+       u32 val = prop->val;
+       u32 prev_val = prop->previous_val;
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(psy_props_changes); i++) {
+               psyc = &psy_props_changes[i];
+               if (prop->prop == psyc->prop) {
+                       if (!psyc->tolerance_change)
+                               gbpsy->changed = true;
+                       else if (val < prev_val &&
+                                prev_val - val > psyc->tolerance_change)
+                               gbpsy->changed = true;
+                       else if (val > prev_val &&
+                                val - prev_val > psyc->tolerance_change)
+                               gbpsy->changed = true;
+                       break;
+               }
        }
-       return technology;
 }
 
-static int get_status(struct gb_power_supply *gb)
+static int total_props(struct gb_power_supply *gbpsy)
 {
-       struct gb_power_supply_status_response status_response;
-       u16 psy_status;
-       int retval;
+       /* this return the intval plus the strval properties */
+       return (gbpsy->properties_count + gbpsy->properties_count_str);
+}
 
-       retval = gb_operation_sync(gb->connection, GB_POWER_SUPPLY_TYPE_STATUS,
-                                  NULL, 0,
-                                  &status_response, sizeof(status_response));
-       if (retval)
-               return retval;
+static void prop_append(struct gb_power_supply *gbpsy,
+                       enum power_supply_property prop)
+{
+       enum power_supply_property *new_props_raw;
+
+       gbpsy->properties_count_str++;
+       new_props_raw = krealloc(gbpsy->props_raw, total_props(gbpsy) *
+                                sizeof(enum power_supply_property),
+                                GFP_KERNEL);
+       if (!new_props_raw)
+               return;
+       gbpsy->props_raw = new_props_raw;
+       gbpsy->props_raw[total_props(gbpsy) - 1] = prop;
+}
+
+static int __gb_power_supply_set_name(char *init_name, char *name, size_t len)
+{
+       unsigned int i = 0;
+       int ret = 0;
+       struct power_supply *psy;
+
+       if (!strlen(init_name))
+               init_name = "gb_power_supply";
+       strlcpy(name, init_name, len);
+
+       while ((ret < len) && (psy = power_supply_get_by_name(name))) {
+#ifdef PSY_HAVE_PUT
+               power_supply_put(psy);
+#endif
+               ret = snprintf(name, len, "%s_%u", init_name, ++i);
+       }
+       if (ret >= len)
+               return -ENOMEM;
+       return i;
+}
+
+static void _gb_power_supply_append_props(struct gb_power_supply *gbpsy)
+{
+       if (strlen(gbpsy->manufacturer))
+               prop_append(gbpsy, POWER_SUPPLY_PROP_MANUFACTURER);
+       if (strlen(gbpsy->model_name))
+               prop_append(gbpsy, POWER_SUPPLY_PROP_MODEL_NAME);
+       if (strlen(gbpsy->serial_number))
+               prop_append(gbpsy, POWER_SUPPLY_PROP_SERIAL_NUMBER);
+}
+
+static int gb_power_supply_description_get(struct gb_power_supply *gbpsy)
+{
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
+       struct gb_power_supply_get_description_request req;
+       struct gb_power_supply_get_description_response resp;
+       int ret;
+
+       req.psy_id = gbpsy->id;
+
+       ret = gb_operation_sync(connection,
+                               GB_POWER_SUPPLY_TYPE_GET_DESCRIPTION,
+                               &req, sizeof(req), &resp, sizeof(resp));
+       if (ret < 0)
+               return ret;
+
+       gbpsy->manufacturer = kstrndup(resp.manufacturer, PROP_MAX, GFP_KERNEL);
+       if (!gbpsy->manufacturer)
+               return -ENOMEM;
+       gbpsy->model_name = kstrndup(resp.model, PROP_MAX, GFP_KERNEL);
+       if (!gbpsy->model_name)
+               return -ENOMEM;
+       gbpsy->serial_number = kstrndup(resp.serial_number, PROP_MAX,
+                                      GFP_KERNEL);
+       if (!gbpsy->serial_number)
+               return -ENOMEM;
+
+       gbpsy->type = le16_to_cpu(resp.type);
+       gbpsy->properties_count = resp.properties_count;
+
+       return 0;
+}
+
+static int gb_power_supply_prop_descriptors_get(struct gb_power_supply *gbpsy)
+{
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
+       struct gb_power_supply_get_property_descriptors_request req;
+       struct gb_power_supply_get_property_descriptors_response resp;
+       int ret;
+       int i;
+
+       if (gbpsy->properties_count == 0)
+               return 0;
+
+       req.psy_id = gbpsy->id;
+
+       ret = gb_operation_sync(connection,
+                               GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS,
+                               &req, sizeof(req), &resp,
+                               sizeof(resp) + gbpsy->properties_count *
+                               sizeof(struct gb_power_supply_props_desc));
+       if (ret < 0)
+               return ret;
+
+       gbpsy->props = kcalloc(gbpsy->properties_count, sizeof(*gbpsy->props),
+                             GFP_KERNEL);
+       if (!gbpsy->props)
+               return -ENOMEM;
+
+       gbpsy->props_raw = kzalloc(gbpsy->properties_count *
+                                 sizeof(*gbpsy->props_raw), GFP_KERNEL);
+       if (!gbpsy->props_raw)
+               return -ENOMEM;
+
+       /* Store available properties */
+       for (i = 0; i < gbpsy->properties_count; i++) {
+               gbpsy->props[i].prop = resp.props[i].property;
+               gbpsy->props_raw[i] = resp.props[i].property;
+               if (resp.props[i].is_writeable)
+                       gbpsy->props[i].is_writeable = true;
+       }
 
        /*
-        * Map greybus values to power_supply values.  Hopefully these are
-        * "identical" which should allow gcc to optimize the code away to
-        * nothing.
+        * now append the properties that we already got information in the
+        * get_description operation. (char * ones)
         */
-       psy_status = le16_to_cpu(status_response.psy_status);
-       switch (psy_status) {
-       case GB_POWER_SUPPLY_STATUS_CHARGING:
-               psy_status = POWER_SUPPLY_STATUS_CHARGING;
-               break;
-       case GB_POWER_SUPPLY_STATUS_DISCHARGING:
-               psy_status = POWER_SUPPLY_STATUS_DISCHARGING;
+       _gb_power_supply_append_props(gbpsy);
+
+       return 0;
+}
+
+static int __gb_power_supply_property_update(struct gb_power_supply *gbpsy,
+                                            enum power_supply_property psp)
+{
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
+       struct gb_power_supply_prop *prop;
+       struct gb_power_supply_get_property_request req;
+       struct gb_power_supply_get_property_response resp;
+       u32 val;
+       int ret;
+
+       prop = get_psy_prop(gbpsy, psp);
+       if (!prop)
+               return -EINVAL;
+       req.psy_id = gbpsy->id;
+       req.property = (u8)psp;
+
+       ret = gb_operation_sync(connection, GB_POWER_SUPPLY_TYPE_GET_PROPERTY,
+                               &req, sizeof(req), &resp, sizeof(resp));
+       if (ret < 0)
+               return ret;
+
+       val = le32_to_cpu(resp.prop_val);
+       if (val == prop->val)
+               return 0;
+
+       prop->previous_val = prop->val;
+       prop->val = val;
+
+       check_changed(gbpsy, prop);
+
+       return 0;
+}
+
+static int __gb_power_supply_property_get(struct gb_power_supply *gbpsy,
+                                         enum power_supply_property psp,
+                                         union power_supply_propval *val)
+{
+       struct gb_power_supply_prop *prop;
+
+       prop = get_psy_prop(gbpsy, psp);
+       if (!prop)
+               return -EINVAL;
+
+       val->intval = prop->val;
+       return 0;
+}
+
+static int __gb_power_supply_property_strval_get(struct gb_power_supply *gbpsy,
+                                               enum power_supply_property psp,
+                                               union power_supply_propval *val)
+{
+       switch (psp) {
+       case POWER_SUPPLY_PROP_MODEL_NAME:
+               val->strval = kstrndup(gbpsy->model_name, PROP_MAX, GFP_KERNEL);
                break;
-       case GB_POWER_SUPPLY_STATUS_NOT_CHARGING:
-               psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+       case POWER_SUPPLY_PROP_MANUFACTURER:
+               val->strval = kstrndup(gbpsy->manufacturer, PROP_MAX,
+                                      GFP_KERNEL);
                break;
-       case GB_POWER_SUPPLY_STATUS_FULL:
-               psy_status = POWER_SUPPLY_STATUS_FULL;
+       case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+               val->strval = kstrndup(gbpsy->serial_number, PROP_MAX,
+                                      GFP_KERNEL);
                break;
-       case GB_POWER_SUPPLY_STATUS_UNKNOWN:
        default:
-               psy_status = POWER_SUPPLY_STATUS_UNKNOWN;
                break;
        }
-       return psy_status;
+
+       return 0;
 }
 
-static int get_max_voltage(struct gb_power_supply *gb)
+static int _gb_power_supply_property_get(struct gb_power_supply *gbpsy,
+                                        enum power_supply_property psp,
+                                        union power_supply_propval *val)
 {
-       struct gb_power_supply_max_voltage_response volt_response;
-       u32 max_voltage;
-       int retval;
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
+       int ret;
+
+       /*
+        * Properties of type const char *, were already fetched on
+        * get_description operation and should be cached in gb
+        */
+       if (is_prop_valint(psp))
+               ret = __gb_power_supply_property_get(gbpsy, psp, val);
+       else
+               ret = __gb_power_supply_property_strval_get(gbpsy, psp, val);
 
-       retval = gb_operation_sync(gb->connection,
-                                  GB_POWER_SUPPLY_TYPE_MAX_VOLTAGE,
-                                  NULL, 0,
-                                  &volt_response, sizeof(volt_response));
-       if (retval)
-               return retval;
+       if (ret < 0)
+               dev_err(&connection->bundle->dev, "get property %u\n", psp);
 
-       max_voltage = le32_to_cpu(volt_response.max_voltage);
-       return max_voltage;
+       return 0;
 }
 
-static int get_percent_capacity(struct gb_power_supply *gb)
+static int gb_power_supply_status_get(struct gb_power_supply *gbpsy)
 {
-       struct gb_power_supply_capacity_response capacity_response;
-       u32 capacity;
-       int retval;
+       int ret = 0;
+       int i;
+
+       /* check if cache is good enough */
+       if (gbpsy->last_update &&
+           time_is_after_jiffies(gbpsy->last_update +
+                                 msecs_to_jiffies(cache_time)))
+               return 0;
+
+       for (i = 0; i < gbpsy->properties_count; i++) {
+               ret = __gb_power_supply_property_update(gbpsy,
+                                                       gbpsy->props[i].prop);
+               if (ret < 0)
+                       break;
+       }
 
-       retval = gb_operation_sync(gb->connection,
-                                  GB_POWER_SUPPLY_TYPE_PERCENT_CAPACITY,
-                                  NULL, 0, &capacity_response,
-                                  sizeof(capacity_response));
-       if (retval)
-               return retval;
+       if (ret == 0)
+               gbpsy->last_update = jiffies;
 
-       capacity = le32_to_cpu(capacity_response.capacity);
-       return capacity;
+       return ret;
 }
 
-static int get_temp(struct gb_power_supply *gb)
+static void gb_power_supply_status_update(struct gb_power_supply *gbpsy)
 {
-       struct gb_power_supply_temperature_response temp_response;
-       u32 temperature;
-       int retval;
+       /* check if there a change that need to be reported */
+       gb_power_supply_status_get(gbpsy);
 
-       retval = gb_operation_sync(gb->connection,
-                                  GB_POWER_SUPPLY_TYPE_TEMPERATURE,
-                                  NULL, 0,
-                                  &temp_response, sizeof(temp_response));
-       if (retval)
-               return retval;
+       if (!gbpsy->changed)
+               return;
 
-       temperature = le32_to_cpu(temp_response.temperature);
-       return temperature;
+       gbpsy->update_interval = update_interval_init;
+       __gb_power_supply_changed(gbpsy);
+       gbpsy->changed = false;
 }
 
-static int get_voltage(struct gb_power_supply *gb)
+static void gb_power_supply_work(struct work_struct *work)
 {
-       struct gb_power_supply_voltage_response voltage_response;
-       u32 voltage;
-       int retval;
+       struct gb_power_supply *gbpsy = container_of(work,
+                                                    struct gb_power_supply,
+                                                    work.work);
 
-       retval = gb_operation_sync(gb->connection, GB_POWER_SUPPLY_TYPE_VOLTAGE,
-                                  NULL, 0,
-                                  &voltage_response, sizeof(voltage_response));
-       if (retval)
-               return retval;
+       /*
+        * if the poll interval is not set, disable polling, this is helpful
+        * specially at unregister time.
+        */
+       if (!gbpsy->update_interval)
+               return;
 
-       voltage = le32_to_cpu(voltage_response.voltage);
-       return voltage;
+       gb_power_supply_status_update(gbpsy);
+       next_interval(gbpsy);
+       schedule_delayed_work(&gbpsy->work, gbpsy->update_interval);
 }
 
 static int get_property(struct power_supply *b,
                        enum power_supply_property psp,
                        union power_supply_propval *val)
 {
-       struct gb_power_supply *gb = to_gb_power_supply(b);
+       struct gb_power_supply *gbpsy = to_gb_power_supply(b);
 
-       switch (psp) {
-       case POWER_SUPPLY_PROP_TECHNOLOGY:
-               val->intval = get_tech(gb);
-               break;
+       gb_power_supply_status_get(gbpsy);
 
-       case POWER_SUPPLY_PROP_STATUS:
-               val->intval = get_status(gb);
-               break;
+       return _gb_power_supply_property_get(gbpsy, psp, val);
+}
 
-       case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
-               val->intval = get_max_voltage(gb);
-               break;
+static int gb_power_supply_property_set(struct gb_power_supply *gbpsy,
+                                       enum power_supply_property psp,
+                                       int val)
+{
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
+       struct gb_power_supply_prop *prop;
+       struct gb_power_supply_set_property_request req;
+       int ret;
 
-       case POWER_SUPPLY_PROP_CAPACITY:
-               val->intval = get_percent_capacity(gb);
-               break;
+       prop = get_psy_prop(gbpsy, psp);
+       if (!prop)
+               return -EINVAL;
+       req.psy_id = gbpsy->id;
+       req.property = (u8)psp;
+       req.prop_val = cpu_to_le32(val);
 
-       case POWER_SUPPLY_PROP_TEMP:
-               val->intval = get_temp(gb);
-               break;
+       ret = gb_operation_sync(connection, GB_POWER_SUPPLY_TYPE_SET_PROPERTY,
+                               &req, sizeof(req), NULL, 0);
+       if (ret < 0)
+               goto out;
 
-       case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-               val->intval = get_voltage(gb);
-               break;
+       /* cache immediately the new value */
+       prop->val = val;
 
-       default:
-               return -EINVAL;
-       }
+out:
+       return ret;
+}
 
-       return (val->intval < 0) ? val->intval : 0;
+static int set_property(struct power_supply *b,
+                       enum power_supply_property psp,
+                       const union power_supply_propval *val)
+{
+       struct gb_power_supply *gbpsy = to_gb_power_supply(b);
+
+       return gb_power_supply_property_set(gbpsy, psp, val->intval);
+}
+
+static int property_is_writeable(struct power_supply *b,
+                                enum power_supply_property psp)
+{
+       struct gb_power_supply *gbpsy = to_gb_power_supply(b);
+
+       return is_psy_prop_writeable(gbpsy, psp);
 }
 
-// FIXME - verify this list, odds are some can be removed and others added.
-static enum power_supply_property psy_props[] = {
-       POWER_SUPPLY_PROP_TECHNOLOGY,
-       POWER_SUPPLY_PROP_STATUS,
-       POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
-       POWER_SUPPLY_PROP_CAPACITY,
-       POWER_SUPPLY_PROP_TEMP,
-       POWER_SUPPLY_PROP_VOLTAGE_NOW,
-};
 
 #ifdef DRIVER_OWNS_PSY_STRUCT
-static int init_and_register(struct gb_connection *connection,
-                            struct gb_battery *gb)
+static int gb_power_supply_register(struct gb_power_supply *gbpsy)
 {
-       // FIXME - get a better (i.e. unique) name
-       // FIXME - anything else needs to be set?
-       gb->psy.name            = "gb_battery";
-       gb->psy.type            = POWER_SUPPLY_TYPE_BATTERY;
-       gb->psy.properties      = psy_props;
-       gb->psy.num_properties  = ARRAY_SIZE(psy_props);
-       gb->psy.get_property    = get_property;
-
-       return power_supply_register(&connection->bundle->dev, &gb->psy);
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
+
+       gbpsy->psy.name                 = gbpsy->name;
+       gbpsy->psy.type                 = gbpsy->type;
+       gbpsy->psy.properties           = gbpsy->props_raw;
+       gbpsy->psy.num_properties       = total_props(gbpsy);
+       gbpsy->psy.get_property         = get_property;
+       gbpsy->psy.set_property         = set_property;
+       gbpsy->psy.property_is_writeable = property_is_writeable;
+
+       return power_supply_register(&connection->bundle->dev,
+                                    &gbpsy->psy);
 }
 #else
-static int init_and_register(struct gb_connection *connection,
-                            struct gb_power_supply *gb)
+static int gb_power_supply_register(struct gb_power_supply *gbpsy)
 {
+       struct gb_connection *connection = get_conn_from_psy(gbpsy);
        struct power_supply_config cfg = {};
 
-       cfg.drv_data = gb;
+       cfg.drv_data = gbpsy;
 
-       // FIXME - get a better (i.e. unique) name
-       // FIXME - anything else needs to be set?
-       gb->desc.name           = "gb_battery";
-       gb->desc.type           = POWER_SUPPLY_TYPE_BATTERY;
-       gb->desc.properties     = psy_props;
-       gb->desc.num_properties = ARRAY_SIZE(psy_props);
-       gb->desc.get_property   = get_property;
+       gbpsy->desc.name                = gbpsy->name;
+       gbpsy->desc.type                = gbpsy->type;
+       gbpsy->desc.properties          = gbpsy->props_raw;
+       gbpsy->desc.num_properties      = total_props(gbpsy);
+       gbpsy->desc.get_property        = get_property;
+       gbpsy->desc.set_property        = set_property;
+       gbpsy->desc.property_is_writeable = property_is_writeable;
 
-       gb->psy = power_supply_register(&connection->bundle->dev,
-                                       &gb->desc, &cfg);
-       if (IS_ERR(gb->psy))
-               return PTR_ERR(gb->psy);
+       gbpsy->psy = power_supply_register(&connection->bundle->dev,
+                                          &gbpsy->desc, &cfg);
+       if (IS_ERR(gbpsy->psy))
+               return PTR_ERR(gbpsy->psy);
 
        return 0;
 }
 #endif
 
+static void _gb_power_supply_free(struct gb_power_supply *gbpsy)
+{
+       kfree(gbpsy->serial_number);
+       kfree(gbpsy->model_name);
+       kfree(gbpsy->manufacturer);
+       kfree(gbpsy->props_raw);
+       kfree(gbpsy->props);
+       kfree(gbpsy);
+}
+
+static void _gb_power_supply_release(struct gb_power_supply *gbpsy)
+{
+       if (!gbpsy)
+               return;
+
+       gbpsy->update_interval = 0;
+
+       cancel_delayed_work_sync(&gbpsy->work);
+#ifdef DRIVER_OWNS_PSY_STRUCT
+       power_supply_unregister(&gbpsy->psy);
+#else
+       power_supply_unregister(gbpsy->psy);
+#endif
+
+       _gb_power_supply_free(gbpsy);
+}
+
+static void _gb_power_supplies_release(struct gb_power_supplies *supplies)
+{
+       int i;
+
+       mutex_lock(&supplies->supplies_lock);
+       for (i = 0; i < supplies->supplies_count; i++)
+               _gb_power_supply_release(&supplies->supply[i]);
+       mutex_unlock(&supplies->supplies_lock);
+}
+
+static int gb_power_supplies_get_count(struct gb_power_supplies *supplies)
+{
+       struct gb_power_supply_get_supplies_response resp;
+       int ret;
+
+       ret = gb_operation_sync(supplies->connection,
+                               GB_POWER_SUPPLY_TYPE_GET_SUPPLIES,
+                               NULL, 0, &resp, sizeof(resp));
+       if (ret < 0)
+               return ret;
+
+       if  (!resp.supplies_count)
+               return -EINVAL;
+
+       supplies->supplies_count = resp.supplies_count;
+
+       return ret;
+}
+
+static int gb_power_supply_config(struct gb_power_supplies *supplies, int id)
+{
+       struct gb_power_supply *gbpsy = &supplies->supply[id];
+       int ret;
+
+       gbpsy->supplies = supplies;
+       gbpsy->id = id;
+
+       ret = gb_power_supply_description_get(gbpsy);
+       if (ret < 0)
+               goto out;
+
+       ret = gb_power_supply_prop_descriptors_get(gbpsy);
+       if (ret < 0)
+               goto out;
+
+       /* guarantee that we have an unique name, before register */
+       ret = __gb_power_supply_set_name(gbpsy->model_name, gbpsy->name,
+                                        sizeof(gbpsy->name));
+       if (ret < 0)
+               goto out;
+
+       ret = gb_power_supply_register(gbpsy);
+       if (ret < 0)
+               goto out;
+
+       gbpsy->update_interval = update_interval_init;
+       INIT_DELAYED_WORK(&gbpsy->work, gb_power_supply_work);
+       schedule_delayed_work(&gbpsy->work, 0);
+
+out:
+       return ret;
+}
+
+static int gb_power_supplies_setup(struct gb_power_supplies *supplies)
+{
+       struct gb_connection *connection = supplies->connection;
+       int ret;
+       int i;
+
+       mutex_lock(&supplies->supplies_lock);
+
+       ret = gb_power_supplies_get_count(supplies);
+       if (ret < 0)
+               goto out;
+
+       supplies->supply = kzalloc(supplies->supplies_count *
+                                    sizeof(struct gb_power_supply),
+                                    GFP_KERNEL);
+
+       if (!supplies->supply)
+               return -ENOMEM;
+
+       for (i = 0; i < supplies->supplies_count; i++) {
+               ret = gb_power_supply_config(supplies, i);
+               if (ret < 0) {
+                       dev_err(&connection->bundle->dev,
+                               "Fail to configure supplies devices\n");
+                       goto out;
+               }
+       }
+out:
+       mutex_unlock(&supplies->supplies_lock);
+       return ret;
+}
+
+static int gb_power_supply_event_recv(u8 type, struct gb_operation *op)
+{
+       struct gb_connection *connection = op->connection;
+       struct gb_power_supplies *supplies = connection->private;
+       struct gb_power_supply *gbpsy;
+       struct gb_message *request;
+       struct gb_power_supply_event_request *payload;
+       u8 psy_id;
+       u8 event;
+       int ret = 0;
+
+       if (type != GB_POWER_SUPPLY_TYPE_EVENT) {
+               dev_err(&connection->bundle->dev,
+                       "Unsupported unsolicited event: %u\n", type);
+               return -EINVAL;
+       }
+
+       request = op->request;
+
+       if (request->payload_size < sizeof(*payload)) {
+               dev_err(&connection->bundle->dev,
+                       "Wrong event size received (%zu < %zu)\n",
+                       request->payload_size, sizeof(*payload));
+               return -EINVAL;
+       }
+
+       payload = request->payload;
+       psy_id = payload->psy_id;
+       mutex_lock(&supplies->supplies_lock);
+       if (psy_id >= supplies->supplies_count || !&supplies->supply[psy_id]) {
+               dev_err(&connection->bundle->dev,
+                       "Event received for unconfigured power_supply id: %d\n",
+                       psy_id);
+               ret = -EINVAL;
+               goto out_unlock;
+       }
+
+       event = payload->event;
+       /*
+        * we will only handle events after setup is done and before release is
+        * running. For that just check update_interval.
+        */
+       gbpsy = &supplies->supply[psy_id];
+       if (gbpsy->update_interval) {
+               ret = -ESHUTDOWN;
+               goto out_unlock;
+       }
+
+       if (event & GB_POWER_SUPPLY_UPDATE)
+               gb_power_supply_status_update(gbpsy);
+
+out_unlock:
+       mutex_unlock(&supplies->supplies_lock);
+       return ret;
+}
+
 static int gb_power_supply_connection_init(struct gb_connection *connection)
 {
-       struct gb_power_supply *gb;
-       int retval;
+       struct gb_power_supplies *supplies;
 
-       gb = kzalloc(sizeof(*gb), GFP_KERNEL);
-       if (!gb)
+       supplies = kzalloc(sizeof(*supplies), GFP_KERNEL);
+       if (!supplies)
                return -ENOMEM;
 
-       gb->connection = connection;
-       connection->private = gb;
+       supplies->connection = connection;
+       connection->private = supplies;
 
-       retval = init_and_register(connection, gb);
-       if (retval)
-               kfree(gb);
+       mutex_init(&supplies->supplies_lock);
 
-       return retval;
+       return gb_power_supplies_setup(supplies);
 }
 
 static void gb_power_supply_connection_exit(struct gb_connection *connection)
 {
-       struct gb_power_supply *gb = connection->private;
+       struct gb_power_supplies *supplies = connection->private;
 
-#ifdef DRIVER_OWNS_PSY_STRUCT
-       power_supply_unregister(&gb->psy);
-#else
-       power_supply_unregister(gb->psy);
-#endif
-       kfree(gb);
+       _gb_power_supplies_release(supplies);
 }
 
 static struct gb_protocol power_supply_protocol = {
@@ -312,7 +746,7 @@ static struct gb_protocol power_supply_protocol = {
        .minor                  = GB_POWER_SUPPLY_VERSION_MINOR,
        .connection_init        = gb_power_supply_connection_init,
        .connection_exit        = gb_power_supply_connection_exit,
-       .request_recv           = NULL, /* no incoming requests */
+       .request_recv           = gb_power_supply_event_recv,
 };
 
 gb_protocol_driver(&power_supply_protocol);