greybus: power_supply: rework and operation changes

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

@@ -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;
 
 

drivers/staging/greybus/kernel_ver.h

@@ -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 */

drivers/staging/greybus/power_supply.c

 /*
  * 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);