[PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

Currently the Linux kernel does not provide any standard integration of this
feature that integrates the USB subsystem with the system power regulation
provided by PMICs meaning that either vendors must add this in their kernels
or USB gadget devices based on Linux (such as mobile phones) may not behave
as they should. Thus provide a standard framework for doing this in kernel.

Now introduce one user with wm831x_power to support and test the usb charger,
which is pending testing. Moreover there may be other potential users will use
it in future.

Changes since v8:
 - Remove the bus type things.
 - Introduce one charger_detect() callback to detect charger type if it is needed.
 - Add some sysfs attributes for showing the charger type and state.
 - Change the charger current attributes to read only.
 - Move charger state and type definition to include/uapi.
 - Some other optimiazations.

Baolin Wang (4):
  gadget: Introduce the usb charger framework
  gadget: Support for the usb charger framework
  gadget: Integrate with the usb gadget supporting for usb charger
  power: wm831x_power: Support USB charger current limit management

 drivers/power/wm831x_power.c      |   69 ++++
 drivers/usb/gadget/Kconfig        |    7 +
 drivers/usb/gadget/udc/Makefile   |    1 +
 drivers/usb/gadget/udc/charger.c  |  798 +++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/udc/udc-core.c |   11 +
 include/linux/mfd/wm831x/pdata.h  |    3 +
 include/linux/usb/charger.h       |  171 ++++++++
 include/linux/usb/gadget.h        |   15 +
 include/uapi/linux/usb/charger.h  |   31 ++
 9 files changed, 1106 insertions(+)
 create mode 100644 drivers/usb/gadget/udc/charger.c
 create mode 100644 include/linux/usb/charger.h
 create mode 100644 include/uapi/linux/usb/charger.h

-- 
1.7.9.5

[PATCH v9 1/4] gadget: Introduce the usb charger framework

[Baolin Wang]

This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.

The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.

The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state. It also supplies the
notification mechanism to userspace When the usb charger state is changed.

Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.

This patch doesn't yet integrate with the gadget code, so some functions
which rely on the 'gadget' are not completed, that will be implemented
in the following patches.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/usb/gadget/Kconfig       |    7 +
 drivers/usb/gadget/udc/Makefile  |    1 +
 drivers/usb/gadget/udc/charger.c |  758 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb/charger.h      |  171 +++++++++
 include/uapi/linux/usb/charger.h |   31 ++
 5 files changed, 968 insertions(+)
 create mode 100644 drivers/usb/gadget/udc/charger.c
 create mode 100644 include/linux/usb/charger.h
 create mode 100644 include/uapi/linux/usb/charger.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index af5d922..82a5b3c 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -133,6 +133,13 @@ config U_SERIAL_CONSOLE
 	help
 	   It supports the serial gadget can be used as a console.
 
+config USB_CHARGER
+	bool "USB charger support"
+	help
+	  The usb charger driver based on the usb gadget that makes an
+	  enhancement to a power driver which can set the current limitation
+	  when the usb charger is added or removed.
+
 source "drivers/usb/gadget/udc/Kconfig"
 
 #
diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
index dfee534..0e9564c 100644
--- a/drivers/usb/gadget/udc/Makefile
+++ b/drivers/usb/gadget/udc/Makefile
@@ -2,6 +2,7 @@
 # USB peripheral controller drivers
 #
 obj-$(CONFIG_USB_GADGET)	+= udc-core.o
+obj-$(CONFIG_USB_CHARGER)	+= charger.o
 obj-$(CONFIG_USB_DUMMY_HCD)	+= dummy_hcd.o
 obj-$(CONFIG_USB_NET2272)	+= net2272.o
 obj-$(CONFIG_USB_NET2280)	+= net2280.o
diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
new file mode 100644
index 0000000..251f42b
--- /dev/null
+++ b/drivers/usb/gadget/udc/charger.c
@@ -0,0 +1,758 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/charger.h>
+#include <linux/power_supply.h>
+
+#define DEFAULT_SDP_CUR_LIMIT		500
+#define DEFAULT_SDP_CUR_LIMIT_SS	900
+#define DEFAULT_DCP_CUR_LIMIT		1500
+#define DEFAULT_CDP_CUR_LIMIT		1500
+#define DEFAULT_ACA_CUR_LIMIT		1500
+
+static DEFINE_IDA(usb_charger_ida);
+struct class *usb_charger_class;
+
+static struct usb_charger *dev_to_uchger(struct device *dev)
+{
+	return container_of(dev, struct usb_charger, dev);
+}
+
+/*
+ * sdp_current_show() - Show the SDP type charger current limit.
+ */
+static ssize_t sdp_current_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+
+	return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
+}
+static DEVICE_ATTR_RO(sdp_current);
+
+/*
+ * dcp_current_show() - Show the DCP type charger current limit.
+ */
+static ssize_t dcp_current_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+
+	return sprintf(buf, "%d\n", uchger->cur_limit.dcp_cur_limit);
+}
+static DEVICE_ATTR_RO(dcp_current);
+
+/*
+ * cdp_current_show() - Show the CDP type charger current limit.
+ */
+static ssize_t cdp_current_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+
+	return sprintf(buf, "%d\n", uchger->cur_limit.cdp_cur_limit);
+}
+static DEVICE_ATTR_RO(cdp_current);
+
+/*
+ * aca_current_show() - Show the ACA type charger current limit.
+ */
+static ssize_t aca_current_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+
+	return sprintf(buf, "%d\n", uchger->cur_limit.aca_cur_limit);
+}
+static DEVICE_ATTR_RO(aca_current);
+
+/*
+ * charger_type_show() - Show the charger type.
+ *
+ * It can be SDP/DCP/CDP/ACA type, else for unknown type.
+ */
+static ssize_t charger_type_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+	int cnt;
+
+	switch (uchger->type) {
+	case SDP_TYPE:
+		cnt = sprintf(buf, "%s\n", "SDP");
+		break;
+	case DCP_TYPE:
+		cnt = sprintf(buf, "%s\n", "DCP");
+		break;
+	case CDP_TYPE:
+		cnt = sprintf(buf, "%s\n", "CDP");
+		break;
+	case ACA_TYPE:
+		cnt = sprintf(buf, "%s\n", "ACA");
+		break;
+	default:
+		cnt = sprintf(buf, "%s\n", "UNKNOWN");
+		break;
+	}
+
+	return cnt;
+}
+static DEVICE_ATTR_RO(charger_type);
+
+/*
+ * charger_state_show() - Show the charger state.
+ *
+ * Charger state can be present or removed.
+ */
+static ssize_t charger_state_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+	int cnt;
+
+	switch (uchger->state) {
+	case USB_CHARGER_PRESENT:
+		cnt = sprintf(buf, "%s\n", "PRESENT");
+		break;
+	case USB_CHARGER_REMOVE:
+		cnt = sprintf(buf, "%s\n", "REMOVE");
+		break;
+	default:
+		cnt = sprintf(buf, "%s\n", "UNKNOWN");
+		break;
+	}
+
+	return cnt;
+}
+static DEVICE_ATTR_RO(charger_state);
+
+static struct attribute *usb_charger_attrs[] = {
+	&dev_attr_sdp_current.attr,
+	&dev_attr_dcp_current.attr,
+	&dev_attr_cdp_current.attr,
+	&dev_attr_aca_current.attr,
+	&dev_attr_charger_type.attr,
+	&dev_attr_charger_state.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(usb_charger);
+
+/*
+ * usb_charger_find_by_name() - Get the usb charger device by name.
+ * @name - usb charger device name.
+ *
+ * return the instance of usb charger device, the device must be
+ * released with usb_charger_put().
+ */
+struct usb_charger *usb_charger_find_by_name(const char *name)
+{
+	struct class_dev_iter iter;
+	struct device *dev;
+
+	if (WARN(!name, "can't work with NULL name"))
+		return ERR_PTR(-EINVAL);
+
+	/* Iterate all usb charger devices in the class */
+	class_dev_iter_init(&iter, usb_charger_class, NULL, NULL);
+	while ((dev = class_dev_iter_next(&iter))) {
+		if (!strcmp(dev_name(dev), name))
+			break;
+	}
+	class_dev_iter_exit(&iter);
+
+	if (WARN(!dev, "can't find usb charger device"))
+		return ERR_PTR(-ENODEV);
+
+	return dev_to_uchger(dev);
+}
+EXPORT_SYMBOL_GPL(usb_charger_find_by_name);
+
+/*
+ * usb_charger_get() - Reference a usb charger.
+ * @uchger - usb charger
+ */
+struct usb_charger *usb_charger_get(struct usb_charger *uchger)
+{
+	return (uchger && get_device(&uchger->dev)) ? uchger : NULL;
+}
+EXPORT_SYMBOL_GPL(usb_charger_get);
+
+/*
+ * usb_charger_put() - Dereference a usb charger.
+ * @uchger - charger to release
+ */
+void usb_charger_put(struct usb_charger *uchger)
+{
+	if (uchger)
+		put_device(&uchger->dev);
+}
+EXPORT_SYMBOL_GPL(usb_charger_put);
+
+/*
+ * usb_charger_get_type() - get the usb charger type with lock protection.
+ * @uchger - usb charger device
+ *
+ * Users can get the charger type by this safe API, rather than using the
+ * usb_charger structure directly.
+ */
+enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger)
+{
+	enum usb_charger_type type;
+
+	mutex_lock(&uchger->lock);
+	type = uchger->type;
+	mutex_unlock(&uchger->lock);
+
+	return type;
+}
+EXPORT_SYMBOL_GPL(usb_charger_get_type);
+
+/*
+ * usb_charger_detect_type() - detect the charger type manually.
+ * @uchger - usb charger device
+ *
+ * Note: You should ensure you need to detect the charger type manually on your
+ * platform.
+ * You should call it at the right gadget state to avoid affecting gadget
+ * enumeration.
+ */
+int usb_charger_detect_type(struct usb_charger *uchger)
+{
+	enum usb_charger_type type;
+
+	if (WARN(!uchger->charger_detect,
+		 "charger detection method should not be NULL"))
+		return -EINVAL;
+
+	type = uchger->charger_detect(uchger);
+
+	mutex_lock(&uchger->lock);
+	uchger->type = type;
+	mutex_unlock(&uchger->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_charger_detect_type);
+
+/*
+ * usb_charger_get_type_by_others() - Get the usb charger type by the callback
+ * which is implemented by users.
+ * @uchger - the usb charger device.
+ *
+ * Note: This function is just used for getting the charger type, not for
+ * detecting charger type which might affect the DP/DM line when gadget is on
+ * enumeration state.
+ */
+static enum usb_charger_type
+usb_charger_get_type_by_others(struct usb_charger *uchger)
+{
+	if (uchger->type != UNKNOWN_TYPE)
+		return uchger->type;
+
+	if (uchger->psy) {
+		union power_supply_propval val;
+
+		power_supply_get_property(uchger->psy,
+					  POWER_SUPPLY_PROP_CHARGE_TYPE,
+					  &val);
+		switch (val.intval) {
+		case POWER_SUPPLY_TYPE_USB:
+			uchger->type = SDP_TYPE;
+			break;
+		case POWER_SUPPLY_TYPE_USB_DCP:
+			uchger->type = DCP_TYPE;
+			break;
+		case POWER_SUPPLY_TYPE_USB_CDP:
+			uchger->type = CDP_TYPE;
+			break;
+		case POWER_SUPPLY_TYPE_USB_ACA:
+			uchger->type = ACA_TYPE;
+			break;
+		default:
+			uchger->type = UNKNOWN_TYPE;
+			break;
+		}
+	} else if (uchger->get_charger_type) {
+		uchger->type = uchger->get_charger_type(uchger);
+	} else {
+		uchger->type = UNKNOWN_TYPE;
+	}
+
+	return uchger->type;
+}
+
+/*
+ * usb_charger_set_cur_limit_by_type() - Set the current limitation
+ * by charger type.
+ * @uchger - the usb charger device.
+ * @type - the usb charger type.
+ * @cur_limit - the current limitation.
+ */
+int usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
+				      enum usb_charger_type type,
+				      unsigned int cur_limit)
+{
+	if (WARN(!uchger, "charger can not be NULL"))
+		return -EINVAL;
+
+	mutex_lock(&uchger->lock);
+	switch (type) {
+	case SDP_TYPE:
+		uchger->cur_limit.sdp_cur_limit = cur_limit;
+		break;
+	case DCP_TYPE:
+		uchger->cur_limit.dcp_cur_limit = cur_limit;
+		break;
+	case CDP_TYPE:
+		uchger->cur_limit.cdp_cur_limit	= cur_limit;
+		break;
+	case ACA_TYPE:
+		uchger->cur_limit.aca_cur_limit	= cur_limit;
+		break;
+	default:
+		mutex_unlock(&uchger->lock);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&uchger->lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_charger_set_cur_limit_by_type);
+
+/*
+ * usb_charger_set_cur_limit() - Set the current limitation.
+ * @uchger - the usb charger device.
+ * @cur_limit_set - the current limitation.
+ */
+int usb_charger_set_cur_limit(struct usb_charger *uchger,
+			      struct usb_charger_cur_limit *cur_limit_set)
+{
+	if (WARN(!uchger || !cur_limit_set, "charger or setting can't be NULL"))
+		return -EINVAL;
+
+	mutex_lock(&uchger->lock);
+	uchger->cur_limit.sdp_cur_limit = cur_limit_set->sdp_cur_limit;
+	uchger->cur_limit.dcp_cur_limit = cur_limit_set->dcp_cur_limit;
+	uchger->cur_limit.cdp_cur_limit = cur_limit_set->cdp_cur_limit;
+	uchger->cur_limit.aca_cur_limit = cur_limit_set->aca_cur_limit;
+	mutex_unlock(&uchger->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_charger_set_cur_limit);
+
+/*
+ * usb_charger_get_cur_limit() - Get the current limitation by
+ * different usb charger type.
+ * @uchger - the usb charger device.
+ *
+ * return the current limitation to set.
+ */
+static unsigned int
+usb_charger_get_cur_limit(struct usb_charger *uchger)
+{
+	enum usb_charger_type uchger_type =
+		usb_charger_get_type_by_others(uchger);
+	unsigned int cur_limit;
+
+	switch (uchger_type) {
+	case SDP_TYPE:
+		cur_limit = uchger->cur_limit.sdp_cur_limit;
+		break;
+	case DCP_TYPE:
+		cur_limit = uchger->cur_limit.dcp_cur_limit;
+		break;
+	case CDP_TYPE:
+		cur_limit = uchger->cur_limit.cdp_cur_limit;
+		break;
+	case ACA_TYPE:
+		cur_limit = uchger->cur_limit.aca_cur_limit;
+		break;
+	default:
+		return 0;
+	}
+
+	return cur_limit;
+}
+
+/*
+ * usb_charger_get_current() - Get the charger current with lock protection.
+ * @uchger - the usb charger device.
+ *
+ * Users should get the charger current by this safe API.
+ */
+unsigned int usb_charger_get_current(struct usb_charger *uchger)
+{
+	unsigned int cur;
+
+	mutex_lock(&uchger->lock);
+	cur = usb_charger_get_cur_limit(uchger);
+	mutex_unlock(&uchger->lock);
+
+	return cur;
+}
+EXPORT_SYMBOL_GPL(usb_charger_get_current);
+
+/*
+ * usb_charger_register_notify() - Register a notifiee to get notified by
+ * any attach status changes from the usb charger detection.
+ * @uchger - the usb charger device which is monitored.
+ * @nb - a notifier block to be registered.
+ */
+int usb_charger_register_notify(struct usb_charger *uchger,
+				struct notifier_block *nb)
+{
+	int ret;
+
+	if (WARN(!uchger || !nb, "charger or nb can not be NULL"))
+		return -EINVAL;
+
+	mutex_lock(&uchger->lock);
+	ret = raw_notifier_chain_register(&uchger->uchger_nh, nb);
+
+	/* Generate an initial notify so users start in the right state */
+	if (!ret) {
+		usb_charger_get_type_by_others(uchger);
+		raw_notifier_call_chain(&uchger->uchger_nh,
+					usb_charger_get_cur_limit(uchger),
+					uchger);
+	}
+	mutex_unlock(&uchger->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_charger_register_notify);
+
+/*
+ * usb_charger_unregister_notify() - Unregister a notifiee from the usb charger.
+ * @uchger - the usb charger device which is monitored.
+ * @nb - a notifier block to be unregistered.
+ */
+int usb_charger_unregister_notify(struct usb_charger *uchger,
+				  struct notifier_block *nb)
+{
+	int ret;
+
+	if (WARN(!uchger || !nb, "charger or nb can not be NULL"))
+		return -EINVAL;
+
+	mutex_lock(&uchger->lock);
+	ret = raw_notifier_chain_unregister(&uchger->uchger_nh, nb);
+	mutex_unlock(&uchger->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_charger_unregister_notify);
+
+/*
+ * usb_charger_notifier_others() - It will notify other device registered
+ * on usb charger when the usb charger state is changed.
+ * @uchger - the usb charger device.
+ * @state - the state of the usb charger.
+ */
+static void
+usb_charger_notify_others(struct usb_charger *uchger,
+			  enum usb_charger_state state)
+{
+	char uchger_state[50] = { 0 };
+	char *envp[] = { uchger_state, NULL };
+
+	mutex_lock(&uchger->lock);
+	uchger->state = state;
+
+	switch (state) {
+	case USB_CHARGER_PRESENT:
+		usb_charger_get_type_by_others(uchger);
+		raw_notifier_call_chain(&uchger->uchger_nh,
+			usb_charger_get_cur_limit(uchger),
+			uchger);
+		snprintf(uchger_state, ARRAY_SIZE(uchger_state),
+			 "USB_CHARGER_STATE=%s", "USB_CHARGER_PRESENT");
+		break;
+	case USB_CHARGER_REMOVE:
+		uchger->type = UNKNOWN_TYPE;
+		raw_notifier_call_chain(&uchger->uchger_nh, 0, uchger);
+		snprintf(uchger_state, ARRAY_SIZE(uchger_state),
+			 "USB_CHARGER_STATE=%s", "USB_CHARGER_REMOVE");
+		break;
+	default:
+		dev_warn(&uchger->dev, "Unknown USB charger state: %d\n",
+			 state);
+		mutex_unlock(&uchger->lock);
+		return;
+	}
+
+	kobject_uevent_env(&uchger->dev.kobj, KOBJ_CHANGE, envp);
+	mutex_unlock(&uchger->lock);
+}
+
+/*
+ * usb_charger_plug_by_extcon() - The notifier call function which is registered
+ * on the extcon device.
+ * @nb - the notifier block that notified by extcon device.
+ * @state - the extcon device state.
+ * @data - here specify a extcon device.
+ *
+ * return the notify flag.
+ */
+static int
+usb_charger_plug_by_extcon(struct notifier_block *nb,
+			   unsigned long state, void *data)
+{
+	struct usb_charger_nb *extcon_nb =
+		container_of(nb, struct usb_charger_nb, nb);
+	struct usb_charger *uchger = extcon_nb->uchger;
+	enum usb_charger_state uchger_state;
+
+	if (WARN(!uchger, "charger can not be NULL"))
+		return NOTIFY_BAD;
+
+	/*
+	 * Report event to power to setting the current limitation
+	 * for this usb charger when one usb charger is added or removed
+	 * with detecting by extcon device.
+	 */
+	if (state)
+		uchger_state = USB_CHARGER_PRESENT;
+	else
+		uchger_state = USB_CHARGER_REMOVE;
+
+	usb_charger_notify_others(uchger, uchger_state);
+
+	return NOTIFY_OK;
+}
+
+/*
+ * usb_charger_plug_by_gadget() - Set the usb charger current limitation
+ * according to the usb gadget device state.
+ * @gadget - the usb gadget device.
+ * @state - the usb gadget state.
+ */
+int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
+			       unsigned long state)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
+
+static int devm_uchger_dev_match(struct device *dev, void *res, void *data)
+{
+	struct usb_charger **r = res;
+
+	if (WARN_ON(!r || !*r))
+		return 0;
+
+	return *r == data;
+}
+
+static void usb_charger_release(struct device *dev)
+{
+	struct usb_charger *uchger = dev_get_drvdata(dev);
+
+	kfree(uchger);
+}
+
+/*
+ * usb_charger_unregister() - Unregister a usb charger device.
+ * @uchger - the usb charger to be unregistered.
+ */
+static int usb_charger_unregister(struct usb_charger *uchger)
+{
+	if (WARN(!uchger, "charger can not be NULL"))
+		return -EINVAL;
+
+	device_unregister(&uchger->dev);
+	return 0;
+}
+
+static void devm_uchger_dev_unreg(struct device *dev, void *res)
+{
+	usb_charger_unregister(*(struct usb_charger **)res);
+}
+
+void devm_usb_charger_unregister(struct device *dev,
+				 struct usb_charger *uchger)
+{
+	devres_release(dev, devm_uchger_dev_unreg,
+		       devm_uchger_dev_match, uchger);
+}
+EXPORT_SYMBOL_GPL(devm_usb_charger_unregister);
+
+/*
+ * usb_charger_register() - Register a new usb charger device
+ * which is created by the usb charger framework.
+ * @parent - the parent device of the new usb charger.
+ * @uchger - the new usb charger device.
+ */
+static int usb_charger_register(struct device *parent,
+				struct usb_charger *uchger)
+{
+	int ret;
+
+	if (WARN(!uchger, "charger can not be NULL"))
+		return -EINVAL;
+
+	uchger->dev.parent = parent;
+	uchger->dev.release = usb_charger_release;
+	uchger->dev.class = usb_charger_class;
+	uchger->dev.groups = usb_charger_groups;
+
+	ret = ida_simple_get(&usb_charger_ida, 0, 0, GFP_KERNEL);
+	if (ret < 0)
+		goto fail_ida;
+
+	uchger->id = ret;
+	dev_set_name(&uchger->dev, "usb-charger.%d", uchger->id);
+	dev_set_drvdata(&uchger->dev, uchger);
+
+	ret = device_register(&uchger->dev);
+	if (ret)
+		goto fail_register;
+
+	return 0;
+
+fail_register:
+	put_device(&uchger->dev);
+	ida_simple_remove(&usb_charger_ida, uchger->id);
+	uchger->id = -1;
+fail_ida:
+	dev_err(parent, "Failed to register usb charger: %d\n", ret);
+	return ret;
+}
+
+int devm_usb_charger_register(struct device *dev,
+			      struct usb_charger *uchger)
+{
+	struct usb_charger **ptr;
+	int ret;
+
+	ptr = devres_alloc(devm_uchger_dev_unreg, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	ret = usb_charger_register(dev, uchger);
+	if (ret) {
+		devres_free(ptr);
+		return ret;
+	}
+
+	*ptr = uchger;
+	devres_add(dev, ptr);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_usb_charger_register);
+
+int usb_charger_init(struct usb_gadget *ugadget)
+{
+	struct usb_charger *uchger;
+	struct extcon_dev *edev;
+	struct power_supply *psy;
+	int ret;
+
+	uchger = kzalloc(sizeof(struct usb_charger), GFP_KERNEL);
+	if (!uchger)
+		return -ENOMEM;
+
+	uchger->type = UNKNOWN_TYPE;
+	uchger->state = USB_CHARGER_DEFAULT;
+	uchger->id = -1;
+
+	if (ugadget->speed >= USB_SPEED_SUPER)
+		uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT_SS;
+	else
+		uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT;
+	uchger->cur_limit.dcp_cur_limit = DEFAULT_DCP_CUR_LIMIT;
+	uchger->cur_limit.cdp_cur_limit = DEFAULT_CDP_CUR_LIMIT;
+	uchger->cur_limit.aca_cur_limit = DEFAULT_ACA_CUR_LIMIT;
+
+	mutex_init(&uchger->lock);
+	RAW_INIT_NOTIFIER_HEAD(&uchger->uchger_nh);
+
+	/* register a notifier on a extcon device if it is exsited */
+	edev = extcon_get_edev_by_phandle(ugadget->dev.parent, 0);
+	if (!IS_ERR_OR_NULL(edev)) {
+		uchger->extcon_dev = edev;
+		uchger->extcon_nb.nb.notifier_call = usb_charger_plug_by_extcon;
+		uchger->extcon_nb.uchger = uchger;
+		extcon_register_notifier(edev, EXTCON_USB,
+					 &uchger->extcon_nb.nb);
+	}
+
+	/* to check if the usb charger is link to a power supply */
+	psy = devm_power_supply_get_by_phandle(ugadget->dev.parent,
+					       "power-supplies");
+	if (!IS_ERR_OR_NULL(psy))
+		uchger->psy = psy;
+	else
+		uchger->psy = NULL;
+
+	/* register a notifier on a usb gadget device */
+	uchger->gadget = ugadget;
+	uchger->old_gadget_state = ugadget->state;
+
+	/* register a new usb charger */
+	ret = usb_charger_register(&ugadget->dev, uchger);
+	if (ret)
+		goto fail;
+
+	return 0;
+
+fail:
+	if (uchger->extcon_dev)
+		extcon_unregister_notifier(uchger->extcon_dev,
+					   EXTCON_USB, &uchger->extcon_nb.nb);
+
+	kfree(uchger);
+	return ret;
+}
+
+int usb_charger_exit(struct usb_gadget *ugadget)
+{
+	return 0;
+}
+
+static int __init usb_charger_class_init(void)
+{
+	usb_charger_class = class_create(THIS_MODULE, "usb_charger");
+	if (IS_ERR(usb_charger_class)) {
+		pr_err("couldn't create class\n");
+		return PTR_ERR(usb_charger_class);
+	}
+
+	return 0;
+}
+
+static void __exit usb_charger_class_exit(void)
+{
+	class_destroy(usb_charger_class);
+}
+subsys_initcall(usb_charger_class_init);
+module_exit(usb_charger_class_exit);
+
+MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org>");
+MODULE_DESCRIPTION("USB charger driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/usb/charger.h b/include/linux/usb/charger.h
new file mode 100644
index 0000000..1bf1d55
--- /dev/null
+++ b/include/linux/usb/charger.h
@@ -0,0 +1,171 @@
+#ifndef __LINUX_USB_CHARGER_H__
+#define __LINUX_USB_CHARGER_H__
+
+#include <uapi/linux/usb/ch9.h>
+#include <uapi/linux/usb/charger.h>
+
+/* Current limitation by charger type */
+struct usb_charger_cur_limit {
+	unsigned int sdp_cur_limit;
+	unsigned int dcp_cur_limit;
+	unsigned int cdp_cur_limit;
+	unsigned int aca_cur_limit;
+};
+
+struct usb_charger_nb {
+	struct notifier_block	nb;
+	struct usb_charger	*uchger;
+};
+
+struct usb_charger {
+	struct device		dev;
+	struct raw_notifier_head	uchger_nh;
+	/* protect the notifier head and charger */
+	struct mutex		lock;
+	int			id;
+	enum usb_charger_type	type;
+	enum usb_charger_state	state;
+
+	/* for supporting extcon usb gpio */
+	struct extcon_dev	*extcon_dev;
+	struct usb_charger_nb	extcon_nb;
+
+	/* for supporting usb gadget */
+	struct usb_gadget	*gadget;
+	enum usb_device_state	old_gadget_state;
+
+	/* for supporting power supply */
+	struct power_supply	*psy;
+
+	/* user can get charger type by implementing this callback */
+	enum usb_charger_type	(*get_charger_type)(struct usb_charger *);
+	/*
+	 * charger detection method can be implemented if you need to
+	 * manually detect the charger type.
+	 */
+	enum usb_charger_type	(*charger_detect)(struct usb_charger *);
+
+	/* current limitation */
+	struct usb_charger_cur_limit	cur_limit;
+};
+
+#ifdef CONFIG_USB_CHARGER
+extern struct usb_charger *usb_charger_find_by_name(const char *name);
+
+extern struct usb_charger *usb_charger_get(struct usb_charger *uchger);
+extern void usb_charger_put(struct usb_charger *uchger);
+
+extern int usb_charger_register_notify(struct usb_charger *uchger,
+				       struct notifier_block *nb);
+extern int usb_charger_unregister_notify(struct usb_charger *uchger,
+					 struct notifier_block *nb);
+
+extern int usb_charger_set_cur_limit(struct usb_charger *uchger,
+				struct usb_charger_cur_limit *cur_limit_set);
+extern int usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
+					     enum usb_charger_type type,
+					     unsigned int cur_limit);
+extern unsigned int usb_charger_get_current(struct usb_charger *uchger);
+
+extern int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
+				      unsigned long state);
+extern enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger);
+extern int usb_charger_detect_type(struct usb_charger *uchger);
+
+extern void devm_usb_charger_unregister(struct device *dev,
+					struct usb_charger *uchger);
+extern int devm_usb_charger_register(struct device *dev,
+				     struct usb_charger *uchger);
+
+extern int usb_charger_init(struct usb_gadget *ugadget);
+extern int usb_charger_exit(struct usb_gadget *ugadget);
+#else
+static inline struct usb_charger *usb_charger_find_by_name(const char *name)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline struct usb_charger *usb_charger_get(struct usb_charger *uchger)
+{
+	return NULL;
+}
+
+static inline void usb_charger_put(struct usb_charger *uchger)
+{
+}
+
+static inline int
+usb_charger_register_notify(struct usb_charger *uchger,
+			    struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline int
+usb_charger_unregister_notify(struct usb_charger *uchger,
+			      struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline int
+usb_charger_set_cur_limit(struct usb_charger *uchger,
+			  struct usb_charger_cur_limit *cur_limit_set)
+{
+	return 0;
+}
+
+static inline int
+usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
+				  enum usb_charger_type type,
+				  unsigned int cur_limit)
+{
+	return 0;
+}
+
+static inline unsigned int
+usb_charger_get_current(struct usb_charger *uchger)
+{
+	return 0;
+}
+
+static inline enum usb_charger_type
+usb_charger_get_type(struct usb_charger *uchger)
+{
+	return UNKNOWN_TYPE;
+}
+
+static inline int usb_charger_detect_type(struct usb_charger *uchger)
+{
+	return 0;
+}
+
+static inline int
+usb_charger_plug_by_gadget(struct usb_gadget *gadget, unsigned long state)
+{
+	return 0;
+}
+
+static inline void devm_usb_charger_unregister(struct device *dev,
+					       struct usb_charger *uchger)
+{
+}
+
+static inline int devm_usb_charger_register(struct device *dev,
+					    struct usb_charger *uchger)
+{
+	return 0;
+}
+
+static inline int usb_charger_init(struct usb_gadget *ugadget)
+{
+	return 0;
+}
+
+static inline int usb_charger_exit(struct usb_gadget *ugadget)
+{
+	return 0;
+}
+#endif
+
+#endif /* __LINUX_USB_CHARGER_H__ */
diff --git a/include/uapi/linux/usb/charger.h b/include/uapi/linux/usb/charger.h
new file mode 100644
index 0000000..3c56ec4
--- /dev/null
+++ b/include/uapi/linux/usb/charger.h
@@ -0,0 +1,31 @@
+/*
+ * This file defines the USB charger type and state that are needed for
+ * USB device APIs.
+ */
+
+#ifndef _UAPI__LINUX_USB_CHARGER_H
+#define _UAPI__LINUX_USB_CHARGER_H
+
+/*
+ * USB charger type:
+ * SDP (Standard Downstream Port)
+ * DCP (Dedicated Charging Port)
+ * CDP (Charging Downstream Port)
+ * ACA (Accessory Charger Adapters)
+ */
+enum usb_charger_type {
+	UNKNOWN_TYPE,
+	SDP_TYPE,
+	DCP_TYPE,
+	CDP_TYPE,
+	ACA_TYPE,
+};
+
+/* USB charger state */
+enum usb_charger_state {
+	USB_CHARGER_DEFAULT,
+	USB_CHARGER_PRESENT,
+	USB_CHARGER_REMOVE,
+};
+
+#endif /* _UAPI__LINUX_USB_CHARGER_H */
-- 
1.7.9.5

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Peter Chen]

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
> +
> +int devm_usb_charger_register(struct device *dev,
> +			      struct usb_charger *uchger)
> +{
> +	struct usb_charger **ptr;
> +	int ret;
> +
> +	ptr = devres_alloc(devm_uchger_dev_unreg, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ret = usb_charger_register(dev, uchger);
> +	if (ret) {
> +		devres_free(ptr);
> +		return ret;
> +	}
> +
> +	*ptr = uchger;
> +	devres_add(dev, ptr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_usb_charger_register);

When the above API is expected to call? Can we use the USB charger
without USB gadget?

> +
> +int usb_charger_init(struct usb_gadget *ugadget)
> +{
> +	struct usb_charger *uchger;
> +	struct extcon_dev *edev;
> +	struct power_supply *psy;
> +	int ret;
> +
> +	uchger = kzalloc(sizeof(struct usb_charger), GFP_KERNEL);
> +	if (!uchger)
> +		return -ENOMEM;
> +
> +	uchger->type = UNKNOWN_TYPE;
> +	uchger->state = USB_CHARGER_DEFAULT;
> +	uchger->id = -1;
> +
> +	if (ugadget->speed >= USB_SPEED_SUPER)
> +		uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT_SS;
> +	else
> +		uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT;

We still haven't known bus speed here, it is better do it after
setting configuration has finished.

Peter

> +	uchger->cur_limit.dcp_cur_limit = DEFAULT_DCP_CUR_LIMIT;
> +	uchger->cur_limit.cdp_cur_limit = DEFAULT_CDP_CUR_LIMIT;
> +	uchger->cur_limit.aca_cur_limit = DEFAULT_ACA_CUR_LIMIT;
> +
> +	mutex_init(&uchger->lock);
> +	RAW_INIT_NOTIFIER_HEAD(&uchger->uchger_nh);
> +
> +	/* register a notifier on a extcon device if it is exsited */
> +	edev = extcon_get_edev_by_phandle(ugadget->dev.parent, 0);
> +	if (!IS_ERR_OR_NULL(edev)) {
> +		uchger->extcon_dev = edev;
> +		uchger->extcon_nb.nb.notifier_call = usb_charger_plug_by_extcon;
> +		uchger->extcon_nb.uchger = uchger;
> +		extcon_register_notifier(edev, EXTCON_USB,
> +					 &uchger->extcon_nb.nb);
> +	}
> +
> +	/* to check if the usb charger is link to a power supply */
> +	psy = devm_power_supply_get_by_phandle(ugadget->dev.parent,
> +					       "power-supplies");
> +	if (!IS_ERR_OR_NULL(psy))
> +		uchger->psy = psy;
> +	else
> +		uchger->psy = NULL;
> +
> +	/* register a notifier on a usb gadget device */
> +	uchger->gadget = ugadget;
> +	uchger->old_gadget_state = ugadget->state;
> +
> +	/* register a new usb charger */
> +	ret = usb_charger_register(&ugadget->dev, uchger);
> +	if (ret)
> +		goto fail;
> +
> +	return 0;
> +
> +fail:
> +	if (uchger->extcon_dev)
> +		extcon_unregister_notifier(uchger->extcon_dev,
> +					   EXTCON_USB, &uchger->extcon_nb.nb);
> +
> +	kfree(uchger);
> +	return ret;
> +}
> +
> +int usb_charger_exit(struct usb_gadget *ugadget)
> +{
> +	return 0;
> +}
> +
> +static int __init usb_charger_class_init(void)
> +{
> +	usb_charger_class = class_create(THIS_MODULE, "usb_charger");
> +	if (IS_ERR(usb_charger_class)) {
> +		pr_err("couldn't create class\n");
> +		return PTR_ERR(usb_charger_class);
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit usb_charger_class_exit(void)
> +{
> +	class_destroy(usb_charger_class);
> +}
> +subsys_initcall(usb_charger_class_init);
> +module_exit(usb_charger_class_exit);
> +
> +MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org>");
> +MODULE_DESCRIPTION("USB charger driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/usb/charger.h b/include/linux/usb/charger.h
> new file mode 100644
> index 0000000..1bf1d55
> --- /dev/null
> +++ b/include/linux/usb/charger.h
> @@ -0,0 +1,171 @@
> +#ifndef __LINUX_USB_CHARGER_H__
> +#define __LINUX_USB_CHARGER_H__
> +
> +#include <uapi/linux/usb/ch9.h>
> +#include <uapi/linux/usb/charger.h>
> +
> +/* Current limitation by charger type */
> +struct usb_charger_cur_limit {
> +	unsigned int sdp_cur_limit;
> +	unsigned int dcp_cur_limit;
> +	unsigned int cdp_cur_limit;
> +	unsigned int aca_cur_limit;
> +};
> +
> +struct usb_charger_nb {
> +	struct notifier_block	nb;
> +	struct usb_charger	*uchger;
> +};
> +
> +struct usb_charger {
> +	struct device		dev;
> +	struct raw_notifier_head	uchger_nh;
> +	/* protect the notifier head and charger */
> +	struct mutex		lock;
> +	int			id;
> +	enum usb_charger_type	type;
> +	enum usb_charger_state	state;
> +
> +	/* for supporting extcon usb gpio */
> +	struct extcon_dev	*extcon_dev;
> +	struct usb_charger_nb	extcon_nb;
> +
> +	/* for supporting usb gadget */
> +	struct usb_gadget	*gadget;
> +	enum usb_device_state	old_gadget_state;
> +
> +	/* for supporting power supply */
> +	struct power_supply	*psy;
> +
> +	/* user can get charger type by implementing this callback */
> +	enum usb_charger_type	(*get_charger_type)(struct usb_charger *);
> +	/*
> +	 * charger detection method can be implemented if you need to
> +	 * manually detect the charger type.
> +	 */
> +	enum usb_charger_type	(*charger_detect)(struct usb_charger *);
> +
> +	/* current limitation */
> +	struct usb_charger_cur_limit	cur_limit;
> +};
> +
> +#ifdef CONFIG_USB_CHARGER
> +extern struct usb_charger *usb_charger_find_by_name(const char *name);
> +
> +extern struct usb_charger *usb_charger_get(struct usb_charger *uchger);
> +extern void usb_charger_put(struct usb_charger *uchger);
> +
> +extern int usb_charger_register_notify(struct usb_charger *uchger,
> +				       struct notifier_block *nb);
> +extern int usb_charger_unregister_notify(struct usb_charger *uchger,
> +					 struct notifier_block *nb);
> +
> +extern int usb_charger_set_cur_limit(struct usb_charger *uchger,
> +				struct usb_charger_cur_limit *cur_limit_set);
> +extern int usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
> +					     enum usb_charger_type type,
> +					     unsigned int cur_limit);
> +extern unsigned int usb_charger_get_current(struct usb_charger *uchger);
> +
> +extern int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
> +				      unsigned long state);
> +extern enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger);
> +extern int usb_charger_detect_type(struct usb_charger *uchger);
> +
> +extern void devm_usb_charger_unregister(struct device *dev,
> +					struct usb_charger *uchger);
> +extern int devm_usb_charger_register(struct device *dev,
> +				     struct usb_charger *uchger);
> +
> +extern int usb_charger_init(struct usb_gadget *ugadget);
> +extern int usb_charger_exit(struct usb_gadget *ugadget);
> +#else
> +static inline struct usb_charger *usb_charger_find_by_name(const char *name)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +static inline struct usb_charger *usb_charger_get(struct usb_charger *uchger)
> +{
> +	return NULL;
> +}
> +
> +static inline void usb_charger_put(struct usb_charger *uchger)
> +{
> +}
> +
> +static inline int
> +usb_charger_register_notify(struct usb_charger *uchger,
> +			    struct notifier_block *nb)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_unregister_notify(struct usb_charger *uchger,
> +			      struct notifier_block *nb)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_set_cur_limit(struct usb_charger *uchger,
> +			  struct usb_charger_cur_limit *cur_limit_set)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
> +				  enum usb_charger_type type,
> +				  unsigned int cur_limit)
> +{
> +	return 0;
> +}
> +
> +static inline unsigned int
> +usb_charger_get_current(struct usb_charger *uchger)
> +{
> +	return 0;
> +}
> +
> +static inline enum usb_charger_type
> +usb_charger_get_type(struct usb_charger *uchger)
> +{
> +	return UNKNOWN_TYPE;
> +}
> +
> +static inline int usb_charger_detect_type(struct usb_charger *uchger)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_plug_by_gadget(struct usb_gadget *gadget, unsigned long state)
> +{
> +	return 0;
> +}
> +
> +static inline void devm_usb_charger_unregister(struct device *dev,
> +					       struct usb_charger *uchger)
> +{
> +}
> +
> +static inline int devm_usb_charger_register(struct device *dev,
> +					    struct usb_charger *uchger)
> +{
> +	return 0;
> +}
> +
> +static inline int usb_charger_init(struct usb_gadget *ugadget)
> +{
> +	return 0;
> +}
> +
> +static inline int usb_charger_exit(struct usb_gadget *ugadget)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* __LINUX_USB_CHARGER_H__ */
> diff --git a/include/uapi/linux/usb/charger.h b/include/uapi/linux/usb/charger.h
> new file mode 100644
> index 0000000..3c56ec4
> --- /dev/null
> +++ b/include/uapi/linux/usb/charger.h
> @@ -0,0 +1,31 @@
> +/*
> + * This file defines the USB charger type and state that are needed for
> + * USB device APIs.
> + */
> +
> +#ifndef _UAPI__LINUX_USB_CHARGER_H
> +#define _UAPI__LINUX_USB_CHARGER_H
> +
> +/*
> + * USB charger type:
> + * SDP (Standard Downstream Port)
> + * DCP (Dedicated Charging Port)
> + * CDP (Charging Downstream Port)
> + * ACA (Accessory Charger Adapters)
> + */
> +enum usb_charger_type {
> +	UNKNOWN_TYPE,
> +	SDP_TYPE,
> +	DCP_TYPE,
> +	CDP_TYPE,
> +	ACA_TYPE,
> +};
> +
> +/* USB charger state */
> +enum usb_charger_state {
> +	USB_CHARGER_DEFAULT,
> +	USB_CHARGER_PRESENT,
> +	USB_CHARGER_REMOVE,
> +};
> +
> +#endif /* _UAPI__LINUX_USB_CHARGER_H */
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Baolin Wang]

On 5 April 2016 at 15:56, Peter Chen <hzpeterchen@gmail.com> wrote:
> On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>> +
>> +int devm_usb_charger_register(struct device *dev,
>> +                           struct usb_charger *uchger)
>> +{
>> +     struct usb_charger **ptr;
>> +     int ret;
>> +
>> +     ptr = devres_alloc(devm_uchger_dev_unreg, sizeof(*ptr), GFP_KERNEL);
>> +     if (!ptr)
>> +             return -ENOMEM;
>> +
>> +     ret = usb_charger_register(dev, uchger);
>> +     if (ret) {
>> +             devres_free(ptr);
>> +             return ret;
>> +     }
>> +
>> +     *ptr = uchger;
>> +     devres_add(dev, ptr);
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_usb_charger_register);
>
> When the above API is expected to call? Can we use the USB charger
> without USB gadget?

I think this is open for user to do their specific initialization for
usb charger. That depends on how to initialize your usb charger
structure.

>
>> +
>> +int usb_charger_init(struct usb_gadget *ugadget)
>> +{
>> +     struct usb_charger *uchger;
>> +     struct extcon_dev *edev;
>> +     struct power_supply *psy;
>> +     int ret;
>> +
>> +     uchger = kzalloc(sizeof(struct usb_charger), GFP_KERNEL);
>> +     if (!uchger)
>> +             return -ENOMEM;
>> +
>> +     uchger->type = UNKNOWN_TYPE;
>> +     uchger->state = USB_CHARGER_DEFAULT;
>> +     uchger->id = -1;
>> +
>> +     if (ugadget->speed >= USB_SPEED_SUPER)
>> +             uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT_SS;
>> +     else
>> +             uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT;
>
> We still haven't known bus speed here, it is better do it after
> setting configuration has finished.

OK. Make sense.

-- 
Baolin.wang
Best Regards

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Peter Chen]

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
 +
> +static struct attribute *usb_charger_attrs[] = {
> +	&dev_attr_sdp_current.attr,
> +	&dev_attr_dcp_current.attr,
> +	&dev_attr_cdp_current.attr,
> +	&dev_attr_aca_current.attr,
> +	&dev_attr_charger_type.attr,
> +	&dev_attr_charger_state.attr,
> +	NULL
> +};

The user may only care about current limit, type and state, why they
need to care what type's current limit, it is the usb charger
framework handles, the framework judge the current according to
charger type and USB state (connect/configured/suspended).

Peter

> +ATTRIBUTE_GROUPS(usb_charger);
> +
> +/*
> + * usb_charger_find_by_name() - Get the usb charger device by name.
> + * @name - usb charger device name.
> + *
> + * return the instance of usb charger device, the device must be
> + * released with usb_charger_put().
> + */
> +struct usb_charger *usb_charger_find_by_name(const char *name)
> +{
> +	struct class_dev_iter iter;
> +	struct device *dev;
> +
> +	if (WARN(!name, "can't work with NULL name"))
> +		return ERR_PTR(-EINVAL);
> +
> +	/* Iterate all usb charger devices in the class */
> +	class_dev_iter_init(&iter, usb_charger_class, NULL, NULL);
> +	while ((dev = class_dev_iter_next(&iter))) {
> +		if (!strcmp(dev_name(dev), name))
> +			break;
> +	}
> +	class_dev_iter_exit(&iter);
> +
> +	if (WARN(!dev, "can't find usb charger device"))
> +		return ERR_PTR(-ENODEV);
> +
> +	return dev_to_uchger(dev);
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_find_by_name);
> +
> +/*
> + * usb_charger_get() - Reference a usb charger.
> + * @uchger - usb charger
> + */
> +struct usb_charger *usb_charger_get(struct usb_charger *uchger)
> +{
> +	return (uchger && get_device(&uchger->dev)) ? uchger : NULL;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_get);
> +
> +/*
> + * usb_charger_put() - Dereference a usb charger.
> + * @uchger - charger to release
> + */
> +void usb_charger_put(struct usb_charger *uchger)
> +{
> +	if (uchger)
> +		put_device(&uchger->dev);
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_put);
> +
> +/*
> + * usb_charger_get_type() - get the usb charger type with lock protection.
> + * @uchger - usb charger device
> + *
> + * Users can get the charger type by this safe API, rather than using the
> + * usb_charger structure directly.
> + */
> +enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger)
> +{
> +	enum usb_charger_type type;
> +
> +	mutex_lock(&uchger->lock);
> +	type = uchger->type;
> +	mutex_unlock(&uchger->lock);
> +
> +	return type;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_get_type);
> +
> +/*
> + * usb_charger_detect_type() - detect the charger type manually.
> + * @uchger - usb charger device
> + *
> + * Note: You should ensure you need to detect the charger type manually on your
> + * platform.
> + * You should call it at the right gadget state to avoid affecting gadget
> + * enumeration.
> + */
> +int usb_charger_detect_type(struct usb_charger *uchger)
> +{
> +	enum usb_charger_type type;
> +
> +	if (WARN(!uchger->charger_detect,
> +		 "charger detection method should not be NULL"))
> +		return -EINVAL;
> +
> +	type = uchger->charger_detect(uchger);
> +
> +	mutex_lock(&uchger->lock);
> +	uchger->type = type;
> +	mutex_unlock(&uchger->lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_detect_type);
> +
> +/*
> + * usb_charger_get_type_by_others() - Get the usb charger type by the callback
> + * which is implemented by users.
> + * @uchger - the usb charger device.
> + *
> + * Note: This function is just used for getting the charger type, not for
> + * detecting charger type which might affect the DP/DM line when gadget is on
> + * enumeration state.
> + */
> +static enum usb_charger_type
> +usb_charger_get_type_by_others(struct usb_charger *uchger)
> +{
> +	if (uchger->type != UNKNOWN_TYPE)
> +		return uchger->type;
> +
> +	if (uchger->psy) {
> +		union power_supply_propval val;
> +
> +		power_supply_get_property(uchger->psy,
> +					  POWER_SUPPLY_PROP_CHARGE_TYPE,
> +					  &val);
> +		switch (val.intval) {
> +		case POWER_SUPPLY_TYPE_USB:
> +			uchger->type = SDP_TYPE;
> +			break;
> +		case POWER_SUPPLY_TYPE_USB_DCP:
> +			uchger->type = DCP_TYPE;
> +			break;
> +		case POWER_SUPPLY_TYPE_USB_CDP:
> +			uchger->type = CDP_TYPE;
> +			break;
> +		case POWER_SUPPLY_TYPE_USB_ACA:
> +			uchger->type = ACA_TYPE;
> +			break;
> +		default:
> +			uchger->type = UNKNOWN_TYPE;
> +			break;
> +		}
> +	} else if (uchger->get_charger_type) {
> +		uchger->type = uchger->get_charger_type(uchger);
> +	} else {
> +		uchger->type = UNKNOWN_TYPE;
> +	}
> +
> +	return uchger->type;
> +}
> +
> +/*
> + * usb_charger_set_cur_limit_by_type() - Set the current limitation
> + * by charger type.
> + * @uchger - the usb charger device.
> + * @type - the usb charger type.
> + * @cur_limit - the current limitation.
> + */
> +int usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
> +				      enum usb_charger_type type,
> +				      unsigned int cur_limit)
> +{
> +	if (WARN(!uchger, "charger can not be NULL"))
> +		return -EINVAL;
> +
> +	mutex_lock(&uchger->lock);
> +	switch (type) {
> +	case SDP_TYPE:
> +		uchger->cur_limit.sdp_cur_limit = cur_limit;
> +		break;
> +	case DCP_TYPE:
> +		uchger->cur_limit.dcp_cur_limit = cur_limit;
> +		break;
> +	case CDP_TYPE:
> +		uchger->cur_limit.cdp_cur_limit	= cur_limit;
> +		break;
> +	case ACA_TYPE:
> +		uchger->cur_limit.aca_cur_limit	= cur_limit;
> +		break;
> +	default:
> +		mutex_unlock(&uchger->lock);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&uchger->lock);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_set_cur_limit_by_type);
> +
> +/*
> + * usb_charger_set_cur_limit() - Set the current limitation.
> + * @uchger - the usb charger device.
> + * @cur_limit_set - the current limitation.
> + */
> +int usb_charger_set_cur_limit(struct usb_charger *uchger,
> +			      struct usb_charger_cur_limit *cur_limit_set)
> +{
> +	if (WARN(!uchger || !cur_limit_set, "charger or setting can't be NULL"))
> +		return -EINVAL;
> +
> +	mutex_lock(&uchger->lock);
> +	uchger->cur_limit.sdp_cur_limit = cur_limit_set->sdp_cur_limit;
> +	uchger->cur_limit.dcp_cur_limit = cur_limit_set->dcp_cur_limit;
> +	uchger->cur_limit.cdp_cur_limit = cur_limit_set->cdp_cur_limit;
> +	uchger->cur_limit.aca_cur_limit = cur_limit_set->aca_cur_limit;
> +	mutex_unlock(&uchger->lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_set_cur_limit);
> +
> +/*
> + * usb_charger_get_cur_limit() - Get the current limitation by
> + * different usb charger type.
> + * @uchger - the usb charger device.
> + *
> + * return the current limitation to set.
> + */
> +static unsigned int
> +usb_charger_get_cur_limit(struct usb_charger *uchger)
> +{
> +	enum usb_charger_type uchger_type =
> +		usb_charger_get_type_by_others(uchger);
> +	unsigned int cur_limit;
> +
> +	switch (uchger_type) {
> +	case SDP_TYPE:
> +		cur_limit = uchger->cur_limit.sdp_cur_limit;
> +		break;
> +	case DCP_TYPE:
> +		cur_limit = uchger->cur_limit.dcp_cur_limit;
> +		break;
> +	case CDP_TYPE:
> +		cur_limit = uchger->cur_limit.cdp_cur_limit;
> +		break;
> +	case ACA_TYPE:
> +		cur_limit = uchger->cur_limit.aca_cur_limit;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return cur_limit;
> +}
> +
> +/*
> + * usb_charger_get_current() - Get the charger current with lock protection.
> + * @uchger - the usb charger device.
> + *
> + * Users should get the charger current by this safe API.
> + */
> +unsigned int usb_charger_get_current(struct usb_charger *uchger)
> +{
> +	unsigned int cur;
> +
> +	mutex_lock(&uchger->lock);
> +	cur = usb_charger_get_cur_limit(uchger);
> +	mutex_unlock(&uchger->lock);
> +
> +	return cur;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_get_current);
> +
> +/*
> + * usb_charger_register_notify() - Register a notifiee to get notified by
> + * any attach status changes from the usb charger detection.
> + * @uchger - the usb charger device which is monitored.
> + * @nb - a notifier block to be registered.
> + */
> +int usb_charger_register_notify(struct usb_charger *uchger,
> +				struct notifier_block *nb)
> +{
> +	int ret;
> +
> +	if (WARN(!uchger || !nb, "charger or nb can not be NULL"))
> +		return -EINVAL;
> +
> +	mutex_lock(&uchger->lock);
> +	ret = raw_notifier_chain_register(&uchger->uchger_nh, nb);
> +
> +	/* Generate an initial notify so users start in the right state */
> +	if (!ret) {
> +		usb_charger_get_type_by_others(uchger);
> +		raw_notifier_call_chain(&uchger->uchger_nh,
> +					usb_charger_get_cur_limit(uchger),
> +					uchger);
> +	}
> +	mutex_unlock(&uchger->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_register_notify);
> +
> +/*
> + * usb_charger_unregister_notify() - Unregister a notifiee from the usb charger.
> + * @uchger - the usb charger device which is monitored.
> + * @nb - a notifier block to be unregistered.
> + */
> +int usb_charger_unregister_notify(struct usb_charger *uchger,
> +				  struct notifier_block *nb)
> +{
> +	int ret;
> +
> +	if (WARN(!uchger || !nb, "charger or nb can not be NULL"))
> +		return -EINVAL;
> +
> +	mutex_lock(&uchger->lock);
> +	ret = raw_notifier_chain_unregister(&uchger->uchger_nh, nb);
> +	mutex_unlock(&uchger->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_unregister_notify);
> +
> +/*
> + * usb_charger_notifier_others() - It will notify other device registered
> + * on usb charger when the usb charger state is changed.
> + * @uchger - the usb charger device.
> + * @state - the state of the usb charger.
> + */
> +static void
> +usb_charger_notify_others(struct usb_charger *uchger,
> +			  enum usb_charger_state state)
> +{
> +	char uchger_state[50] = { 0 };
> +	char *envp[] = { uchger_state, NULL };
> +
> +	mutex_lock(&uchger->lock);
> +	uchger->state = state;
> +
> +	switch (state) {
> +	case USB_CHARGER_PRESENT:
> +		usb_charger_get_type_by_others(uchger);
> +		raw_notifier_call_chain(&uchger->uchger_nh,
> +			usb_charger_get_cur_limit(uchger),
> +			uchger);
> +		snprintf(uchger_state, ARRAY_SIZE(uchger_state),
> +			 "USB_CHARGER_STATE=%s", "USB_CHARGER_PRESENT");
> +		break;
> +	case USB_CHARGER_REMOVE:
> +		uchger->type = UNKNOWN_TYPE;
> +		raw_notifier_call_chain(&uchger->uchger_nh, 0, uchger);
> +		snprintf(uchger_state, ARRAY_SIZE(uchger_state),
> +			 "USB_CHARGER_STATE=%s", "USB_CHARGER_REMOVE");
> +		break;
> +	default:
> +		dev_warn(&uchger->dev, "Unknown USB charger state: %d\n",
> +			 state);
> +		mutex_unlock(&uchger->lock);
> +		return;
> +	}
> +
> +	kobject_uevent_env(&uchger->dev.kobj, KOBJ_CHANGE, envp);
> +	mutex_unlock(&uchger->lock);
> +}
> +
> +/*
> + * usb_charger_plug_by_extcon() - The notifier call function which is registered
> + * on the extcon device.
> + * @nb - the notifier block that notified by extcon device.
> + * @state - the extcon device state.
> + * @data - here specify a extcon device.
> + *
> + * return the notify flag.
> + */
> +static int
> +usb_charger_plug_by_extcon(struct notifier_block *nb,
> +			   unsigned long state, void *data)
> +{
> +	struct usb_charger_nb *extcon_nb =
> +		container_of(nb, struct usb_charger_nb, nb);
> +	struct usb_charger *uchger = extcon_nb->uchger;
> +	enum usb_charger_state uchger_state;
> +
> +	if (WARN(!uchger, "charger can not be NULL"))
> +		return NOTIFY_BAD;
> +
> +	/*
> +	 * Report event to power to setting the current limitation
> +	 * for this usb charger when one usb charger is added or removed
> +	 * with detecting by extcon device.
> +	 */
> +	if (state)
> +		uchger_state = USB_CHARGER_PRESENT;
> +	else
> +		uchger_state = USB_CHARGER_REMOVE;
> +
> +	usb_charger_notify_others(uchger, uchger_state);
> +
> +	return NOTIFY_OK;
> +}
> +
> +/*
> + * usb_charger_plug_by_gadget() - Set the usb charger current limitation
> + * according to the usb gadget device state.
> + * @gadget - the usb gadget device.
> + * @state - the usb gadget state.
> + */
> +int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
> +			       unsigned long state)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
> +
> +static int devm_uchger_dev_match(struct device *dev, void *res, void *data)
> +{
> +	struct usb_charger **r = res;
> +
> +	if (WARN_ON(!r || !*r))
> +		return 0;
> +
> +	return *r == data;
> +}
> +
> +static void usb_charger_release(struct device *dev)
> +{
> +	struct usb_charger *uchger = dev_get_drvdata(dev);
> +
> +	kfree(uchger);
> +}
> +
> +/*
> + * usb_charger_unregister() - Unregister a usb charger device.
> + * @uchger - the usb charger to be unregistered.
> + */
> +static int usb_charger_unregister(struct usb_charger *uchger)
> +{
> +	if (WARN(!uchger, "charger can not be NULL"))
> +		return -EINVAL;
> +
> +	device_unregister(&uchger->dev);
> +	return 0;
> +}
> +
> +static void devm_uchger_dev_unreg(struct device *dev, void *res)
> +{
> +	usb_charger_unregister(*(struct usb_charger **)res);
> +}
> +
> +void devm_usb_charger_unregister(struct device *dev,
> +				 struct usb_charger *uchger)
> +{
> +	devres_release(dev, devm_uchger_dev_unreg,
> +		       devm_uchger_dev_match, uchger);
> +}
> +EXPORT_SYMBOL_GPL(devm_usb_charger_unregister);
> +
> +/*
> + * usb_charger_register() - Register a new usb charger device
> + * which is created by the usb charger framework.
> + * @parent - the parent device of the new usb charger.
> + * @uchger - the new usb charger device.
> + */
> +static int usb_charger_register(struct device *parent,
> +				struct usb_charger *uchger)
> +{
> +	int ret;
> +
> +	if (WARN(!uchger, "charger can not be NULL"))
> +		return -EINVAL;
> +
> +	uchger->dev.parent = parent;
> +	uchger->dev.release = usb_charger_release;
> +	uchger->dev.class = usb_charger_class;
> +	uchger->dev.groups = usb_charger_groups;
> +
> +	ret = ida_simple_get(&usb_charger_ida, 0, 0, GFP_KERNEL);
> +	if (ret < 0)
> +		goto fail_ida;
> +
> +	uchger->id = ret;
> +	dev_set_name(&uchger->dev, "usb-charger.%d", uchger->id);
> +	dev_set_drvdata(&uchger->dev, uchger);
> +
> +	ret = device_register(&uchger->dev);
> +	if (ret)
> +		goto fail_register;
> +
> +	return 0;
> +
> +fail_register:
> +	put_device(&uchger->dev);
> +	ida_simple_remove(&usb_charger_ida, uchger->id);
> +	uchger->id = -1;
> +fail_ida:
> +	dev_err(parent, "Failed to register usb charger: %d\n", ret);
> +	return ret;
> +}
> +
> +int devm_usb_charger_register(struct device *dev,
> +			      struct usb_charger *uchger)
> +{
> +	struct usb_charger **ptr;
> +	int ret;
> +
> +	ptr = devres_alloc(devm_uchger_dev_unreg, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ret = usb_charger_register(dev, uchger);
> +	if (ret) {
> +		devres_free(ptr);
> +		return ret;
> +	}
> +
> +	*ptr = uchger;
> +	devres_add(dev, ptr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_usb_charger_register);
> +
> +int usb_charger_init(struct usb_gadget *ugadget)
> +{
> +	struct usb_charger *uchger;
> +	struct extcon_dev *edev;
> +	struct power_supply *psy;
> +	int ret;
> +
> +	uchger = kzalloc(sizeof(struct usb_charger), GFP_KERNEL);
> +	if (!uchger)
> +		return -ENOMEM;
> +
> +	uchger->type = UNKNOWN_TYPE;
> +	uchger->state = USB_CHARGER_DEFAULT;
> +	uchger->id = -1;
> +
> +	if (ugadget->speed >= USB_SPEED_SUPER)
> +		uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT_SS;
> +	else
> +		uchger->cur_limit.sdp_cur_limit = DEFAULT_SDP_CUR_LIMIT;
> +	uchger->cur_limit.dcp_cur_limit = DEFAULT_DCP_CUR_LIMIT;
> +	uchger->cur_limit.cdp_cur_limit = DEFAULT_CDP_CUR_LIMIT;
> +	uchger->cur_limit.aca_cur_limit = DEFAULT_ACA_CUR_LIMIT;
> +
> +	mutex_init(&uchger->lock);
> +	RAW_INIT_NOTIFIER_HEAD(&uchger->uchger_nh);
> +
> +	/* register a notifier on a extcon device if it is exsited */
> +	edev = extcon_get_edev_by_phandle(ugadget->dev.parent, 0);
> +	if (!IS_ERR_OR_NULL(edev)) {
> +		uchger->extcon_dev = edev;
> +		uchger->extcon_nb.nb.notifier_call = usb_charger_plug_by_extcon;
> +		uchger->extcon_nb.uchger = uchger;
> +		extcon_register_notifier(edev, EXTCON_USB,
> +					 &uchger->extcon_nb.nb);
> +	}
> +
> +	/* to check if the usb charger is link to a power supply */
> +	psy = devm_power_supply_get_by_phandle(ugadget->dev.parent,
> +					       "power-supplies");
> +	if (!IS_ERR_OR_NULL(psy))
> +		uchger->psy = psy;
> +	else
> +		uchger->psy = NULL;
> +
> +	/* register a notifier on a usb gadget device */
> +	uchger->gadget = ugadget;
> +	uchger->old_gadget_state = ugadget->state;
> +
> +	/* register a new usb charger */
> +	ret = usb_charger_register(&ugadget->dev, uchger);
> +	if (ret)
> +		goto fail;
> +
> +	return 0;
> +
> +fail:
> +	if (uchger->extcon_dev)
> +		extcon_unregister_notifier(uchger->extcon_dev,
> +					   EXTCON_USB, &uchger->extcon_nb.nb);
> +
> +	kfree(uchger);
> +	return ret;
> +}
> +
> +int usb_charger_exit(struct usb_gadget *ugadget)
> +{
> +	return 0;
> +}
> +
> +static int __init usb_charger_class_init(void)
> +{
> +	usb_charger_class = class_create(THIS_MODULE, "usb_charger");
> +	if (IS_ERR(usb_charger_class)) {
> +		pr_err("couldn't create class\n");
> +		return PTR_ERR(usb_charger_class);
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit usb_charger_class_exit(void)
> +{
> +	class_destroy(usb_charger_class);
> +}
> +subsys_initcall(usb_charger_class_init);
> +module_exit(usb_charger_class_exit);
> +
> +MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org>");
> +MODULE_DESCRIPTION("USB charger driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/usb/charger.h b/include/linux/usb/charger.h
> new file mode 100644
> index 0000000..1bf1d55
> --- /dev/null
> +++ b/include/linux/usb/charger.h
> @@ -0,0 +1,171 @@
> +#ifndef __LINUX_USB_CHARGER_H__
> +#define __LINUX_USB_CHARGER_H__
> +
> +#include <uapi/linux/usb/ch9.h>
> +#include <uapi/linux/usb/charger.h>
> +
> +/* Current limitation by charger type */
> +struct usb_charger_cur_limit {
> +	unsigned int sdp_cur_limit;
> +	unsigned int dcp_cur_limit;
> +	unsigned int cdp_cur_limit;
> +	unsigned int aca_cur_limit;
> +};
> +
> +struct usb_charger_nb {
> +	struct notifier_block	nb;
> +	struct usb_charger	*uchger;
> +};
> +
> +struct usb_charger {
> +	struct device		dev;
> +	struct raw_notifier_head	uchger_nh;
> +	/* protect the notifier head and charger */
> +	struct mutex		lock;
> +	int			id;
> +	enum usb_charger_type	type;
> +	enum usb_charger_state	state;
> +
> +	/* for supporting extcon usb gpio */
> +	struct extcon_dev	*extcon_dev;
> +	struct usb_charger_nb	extcon_nb;
> +
> +	/* for supporting usb gadget */
> +	struct usb_gadget	*gadget;
> +	enum usb_device_state	old_gadget_state;
> +
> +	/* for supporting power supply */
> +	struct power_supply	*psy;
> +
> +	/* user can get charger type by implementing this callback */
> +	enum usb_charger_type	(*get_charger_type)(struct usb_charger *);
> +	/*
> +	 * charger detection method can be implemented if you need to
> +	 * manually detect the charger type.
> +	 */
> +	enum usb_charger_type	(*charger_detect)(struct usb_charger *);
> +
> +	/* current limitation */
> +	struct usb_charger_cur_limit	cur_limit;
> +};
> +
> +#ifdef CONFIG_USB_CHARGER
> +extern struct usb_charger *usb_charger_find_by_name(const char *name);
> +
> +extern struct usb_charger *usb_charger_get(struct usb_charger *uchger);
> +extern void usb_charger_put(struct usb_charger *uchger);
> +
> +extern int usb_charger_register_notify(struct usb_charger *uchger,
> +				       struct notifier_block *nb);
> +extern int usb_charger_unregister_notify(struct usb_charger *uchger,
> +					 struct notifier_block *nb);
> +
> +extern int usb_charger_set_cur_limit(struct usb_charger *uchger,
> +				struct usb_charger_cur_limit *cur_limit_set);
> +extern int usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
> +					     enum usb_charger_type type,
> +					     unsigned int cur_limit);
> +extern unsigned int usb_charger_get_current(struct usb_charger *uchger);
> +
> +extern int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
> +				      unsigned long state);
> +extern enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger);
> +extern int usb_charger_detect_type(struct usb_charger *uchger);
> +
> +extern void devm_usb_charger_unregister(struct device *dev,
> +					struct usb_charger *uchger);
> +extern int devm_usb_charger_register(struct device *dev,
> +				     struct usb_charger *uchger);
> +
> +extern int usb_charger_init(struct usb_gadget *ugadget);
> +extern int usb_charger_exit(struct usb_gadget *ugadget);
> +#else
> +static inline struct usb_charger *usb_charger_find_by_name(const char *name)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +static inline struct usb_charger *usb_charger_get(struct usb_charger *uchger)
> +{
> +	return NULL;
> +}
> +
> +static inline void usb_charger_put(struct usb_charger *uchger)
> +{
> +}
> +
> +static inline int
> +usb_charger_register_notify(struct usb_charger *uchger,
> +			    struct notifier_block *nb)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_unregister_notify(struct usb_charger *uchger,
> +			      struct notifier_block *nb)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_set_cur_limit(struct usb_charger *uchger,
> +			  struct usb_charger_cur_limit *cur_limit_set)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_set_cur_limit_by_type(struct usb_charger *uchger,
> +				  enum usb_charger_type type,
> +				  unsigned int cur_limit)
> +{
> +	return 0;
> +}
> +
> +static inline unsigned int
> +usb_charger_get_current(struct usb_charger *uchger)
> +{
> +	return 0;
> +}
> +
> +static inline enum usb_charger_type
> +usb_charger_get_type(struct usb_charger *uchger)
> +{
> +	return UNKNOWN_TYPE;
> +}
> +
> +static inline int usb_charger_detect_type(struct usb_charger *uchger)
> +{
> +	return 0;
> +}
> +
> +static inline int
> +usb_charger_plug_by_gadget(struct usb_gadget *gadget, unsigned long state)
> +{
> +	return 0;
> +}
> +
> +static inline void devm_usb_charger_unregister(struct device *dev,
> +					       struct usb_charger *uchger)
> +{
> +}
> +
> +static inline int devm_usb_charger_register(struct device *dev,
> +					    struct usb_charger *uchger)
> +{
> +	return 0;
> +}
> +
> +static inline int usb_charger_init(struct usb_gadget *ugadget)
> +{
> +	return 0;
> +}
> +
> +static inline int usb_charger_exit(struct usb_gadget *ugadget)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* __LINUX_USB_CHARGER_H__ */
> diff --git a/include/uapi/linux/usb/charger.h b/include/uapi/linux/usb/charger.h
> new file mode 100644
> index 0000000..3c56ec4
> --- /dev/null
> +++ b/include/uapi/linux/usb/charger.h
> @@ -0,0 +1,31 @@
> +/*
> + * This file defines the USB charger type and state that are needed for
> + * USB device APIs.
> + */
> +
> +#ifndef _UAPI__LINUX_USB_CHARGER_H
> +#define _UAPI__LINUX_USB_CHARGER_H
> +
> +/*
> + * USB charger type:
> + * SDP (Standard Downstream Port)
> + * DCP (Dedicated Charging Port)
> + * CDP (Charging Downstream Port)
> + * ACA (Accessory Charger Adapters)
> + */
> +enum usb_charger_type {
> +	UNKNOWN_TYPE,
> +	SDP_TYPE,
> +	DCP_TYPE,
> +	CDP_TYPE,
> +	ACA_TYPE,
> +};
> +
> +/* USB charger state */
> +enum usb_charger_state {
> +	USB_CHARGER_DEFAULT,
> +	USB_CHARGER_PRESENT,
> +	USB_CHARGER_REMOVE,
> +};
> +
> +#endif /* _UAPI__LINUX_USB_CHARGER_H */
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

Peter Chen <hzpeterchen@gmail.com> writes:
> On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>  +
>> +static struct attribute *usb_charger_attrs[] = {
>> +	&dev_attr_sdp_current.attr,
>> +	&dev_attr_dcp_current.attr,
>> +	&dev_attr_cdp_current.attr,
>> +	&dev_attr_aca_current.attr,
>> +	&dev_attr_charger_type.attr,
>> +	&dev_attr_charger_state.attr,
>> +	NULL
>> +};
>
> The user may only care about current limit, type and state, why they
> need to care what type's current limit, it is the usb charger
> framework handles, the framework judge the current according to
> charger type and USB state (connect/configured/suspended).

it might be useful if we want to know that $this charger doesn't really
give us as much current as it advertises.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Peter Chen]

On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
> Peter Chen <hzpeterchen@gmail.com> writes:
> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
> >  +
> >> +static struct attribute *usb_charger_attrs[] = {
> >> +	&dev_attr_sdp_current.attr,
> >> +	&dev_attr_dcp_current.attr,
> >> +	&dev_attr_cdp_current.attr,
> >> +	&dev_attr_aca_current.attr,
> >> +	&dev_attr_charger_type.attr,
> >> +	&dev_attr_charger_state.attr,
> >> +	NULL
> >> +};
> >
> > The user may only care about current limit, type and state, why they
> > need to care what type's current limit, it is the usb charger
> > framework handles, the framework judge the current according to
> > charger type and USB state (connect/configured/suspended).
> 
> it might be useful if we want to know that $this charger doesn't really
> give us as much current as it advertises.
> 

As my understanding, the current limit is dynamic value, it should
report the value the charger supports now, eg, it connects SDP, but
the host is suspended now, then the value should be 2mA.

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

Peter Chen <hzpeterchen@gmail.com> writes:
> On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
>> Peter Chen <hzpeterchen@gmail.com> writes:
>> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>> >  +
>> >> +static struct attribute *usb_charger_attrs[] = {
>> >> +	&dev_attr_sdp_current.attr,
>> >> +	&dev_attr_dcp_current.attr,
>> >> +	&dev_attr_cdp_current.attr,
>> >> +	&dev_attr_aca_current.attr,
>> >> +	&dev_attr_charger_type.attr,
>> >> +	&dev_attr_charger_state.attr,
>> >> +	NULL
>> >> +};
>> >
>> > The user may only care about current limit, type and state, why they
>> > need to care what type's current limit, it is the usb charger
>> > framework handles, the framework judge the current according to
>> > charger type and USB state (connect/configured/suspended).
>> 
>> it might be useful if we want to know that $this charger doesn't really
>> give us as much current as it advertises.
>> 
>
> As my understanding, the current limit is dynamic value, it should
> report the value the charger supports now, eg, it connects SDP, but
> the host is suspended now, then the value should be 2mA.

yes, and that's the limit. Now consider we connect to DCP or CDP and
limit is 2000mA but we're charging at 1000mA ;-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Peter Chen]

On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
> Peter Chen <hzpeterchen@gmail.com> writes:
> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
> >> Peter Chen <hzpeterchen@gmail.com> writes:
> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
> >> >  +
> >> >> +static struct attribute *usb_charger_attrs[] = {
> >> >> +	&dev_attr_sdp_current.attr,
> >> >> +	&dev_attr_dcp_current.attr,
> >> >> +	&dev_attr_cdp_current.attr,
> >> >> +	&dev_attr_aca_current.attr,
> >> >> +	&dev_attr_charger_type.attr,
> >> >> +	&dev_attr_charger_state.attr,
> >> >> +	NULL
> >> >> +};
> >> >
> >> > The user may only care about current limit, type and state, why they
> >> > need to care what type's current limit, it is the usb charger
> >> > framework handles, the framework judge the current according to
> >> > charger type and USB state (connect/configured/suspended).
> >> 
> >> it might be useful if we want to know that $this charger doesn't really
> >> give us as much current as it advertises.
> >> 
> >
> > As my understanding, the current limit is dynamic value, it should
> > report the value the charger supports now, eg, it connects SDP, but
> > the host is suspended now, then the value should be 2mA.
> 
> yes, and that's the limit. Now consider we connect to DCP or CDP and
> limit is 2000mA but we're charging at 1000mA ;-)
> 

Does the user need to know the $this charger limit? Don't they only
care about the current charging value? I have a USB cable which can
show charging current value, it changes from time to time, when it
connects to host pc, it shows 430mA; when it connects to dedicated
charger, it shows 1000mA.

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2085 bytes --]


Hi,

Peter Chen <hzpeterchen@gmail.com> writes:
> On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
>> Peter Chen <hzpeterchen@gmail.com> writes:
>> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
>> >> Peter Chen <hzpeterchen@gmail.com> writes:
>> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>> >> >  +
>> >> >> +static struct attribute *usb_charger_attrs[] = {
>> >> >> +	&dev_attr_sdp_current.attr,
>> >> >> +	&dev_attr_dcp_current.attr,
>> >> >> +	&dev_attr_cdp_current.attr,
>> >> >> +	&dev_attr_aca_current.attr,
>> >> >> +	&dev_attr_charger_type.attr,
>> >> >> +	&dev_attr_charger_state.attr,
>> >> >> +	NULL
>> >> >> +};
>> >> >
>> >> > The user may only care about current limit, type and state, why they
>> >> > need to care what type's current limit, it is the usb charger
>> >> > framework handles, the framework judge the current according to
>> >> > charger type and USB state (connect/configured/suspended).
>> >> 
>> >> it might be useful if we want to know that $this charger doesn't really
>> >> give us as much current as it advertises.
>> >> 
>> >
>> > As my understanding, the current limit is dynamic value, it should
>> > report the value the charger supports now, eg, it connects SDP, but
>> > the host is suspended now, then the value should be 2mA.
>> 
>> yes, and that's the limit. Now consider we connect to DCP or CDP and
>> limit is 2000mA but we're charging at 1000mA ;-)
>> 
>
> Does the user need to know the $this charger limit? Don't they only
> care about the current charging value? I have a USB cable which can

Why not ? UI might want to change the color of the battery charging icon
if we're charging @ 2000mA or @ 1000mA to give some visual feedback as
to "how fast" battery is supposed to be charged.

> show charging current value, it changes from time to time, when it
> connects to host pc, it shows 430mA; when it connects to dedicated
> charger, it shows 1000mA.

good for you, now what does that have to do with $subject ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Peter Chen]

On Wed, Apr 06, 2016 at 01:25:06PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen <hzpeterchen@gmail.com> writes:
> > On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
> >> Peter Chen <hzpeterchen@gmail.com> writes:
> >> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
> >> >> Peter Chen <hzpeterchen@gmail.com> writes:
> >> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
> >> >> >  +
> >> >> >> +static struct attribute *usb_charger_attrs[] = {
> >> >> >> +	&dev_attr_sdp_current.attr,
> >> >> >> +	&dev_attr_dcp_current.attr,
> >> >> >> +	&dev_attr_cdp_current.attr,
> >> >> >> +	&dev_attr_aca_current.attr,
> >> >> >> +	&dev_attr_charger_type.attr,
> >> >> >> +	&dev_attr_charger_state.attr,
> >> >> >> +	NULL
> >> >> >> +};
> >> >> >
> >> >> > The user may only care about current limit, type and state, why they
> >> >> > need to care what type's current limit, it is the usb charger
> >> >> > framework handles, the framework judge the current according to
> >> >> > charger type and USB state (connect/configured/suspended).
> >> >> 
> >> >> it might be useful if we want to know that $this charger doesn't really
> >> >> give us as much current as it advertises.
> >> >> 
> >> >
> >> > As my understanding, the current limit is dynamic value, it should
> >> > report the value the charger supports now, eg, it connects SDP, but
> >> > the host is suspended now, then the value should be 2mA.
> >> 
> >> yes, and that's the limit. Now consider we connect to DCP or CDP and
> >> limit is 2000mA but we're charging at 1000mA ;-)
> >> 
> >
> > Does the user need to know the $this charger limit? Don't they only
> > care about the current charging value? I have a USB cable which can
> 
> Why not ? UI might want to change the color of the battery charging icon
> if we're charging @ 2000mA or @ 1000mA to give some visual feedback as
> to "how fast" battery is supposed to be charged.
> 
> > show charging current value, it changes from time to time, when it
> > connects to host pc, it shows 430mA; when it connects to dedicated
> > charger, it shows 1000mA.
> 
> good for you, now what does that have to do with $subject ?
> 

+static struct attribute *usb_charger_attrs[] = {
+	&dev_attr_sdp_current.attr,
+	&dev_attr_dcp_current.attr,
+	&dev_attr_cdp_current.attr,
+	&dev_attr_aca_current.attr,
+	&dev_attr_charger_type.attr,
+	&dev_attr_charger_state.attr,
+	NULL
+};

Ok, even the users are interesting in current limit, we still have no
necessary to know all kinds of chargers limit and current value, since
we already have charger type user interface, the framework can show
limit according to charger type.

I think below user interfaces are enough, who do you think?

+static struct attribute *usb_charger_attrs[] = {
+	&dev_attr_current.attr,
+	&dev_attr_current_limit.attr,
+	&dev_attr_charger_type.attr,
+	&dev_attr_charger_state.attr,
+	NULL
+};

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3317 bytes --]


Hi,

Peter Chen <hzpeterchen@gmail.com> writes:
> On Wed, Apr 06, 2016 at 01:25:06PM +0300, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Peter Chen <hzpeterchen@gmail.com> writes:
>> > On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
>> >> Peter Chen <hzpeterchen@gmail.com> writes:
>> >> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
>> >> >> Peter Chen <hzpeterchen@gmail.com> writes:
>> >> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>> >> >> >  +
>> >> >> >> +static struct attribute *usb_charger_attrs[] = {
>> >> >> >> +	&dev_attr_sdp_current.attr,
>> >> >> >> +	&dev_attr_dcp_current.attr,
>> >> >> >> +	&dev_attr_cdp_current.attr,
>> >> >> >> +	&dev_attr_aca_current.attr,
>> >> >> >> +	&dev_attr_charger_type.attr,
>> >> >> >> +	&dev_attr_charger_state.attr,
>> >> >> >> +	NULL
>> >> >> >> +};
>> >> >> >
>> >> >> > The user may only care about current limit, type and state, why they
>> >> >> > need to care what type's current limit, it is the usb charger
>> >> >> > framework handles, the framework judge the current according to
>> >> >> > charger type and USB state (connect/configured/suspended).
>> >> >> 
>> >> >> it might be useful if we want to know that $this charger doesn't really
>> >> >> give us as much current as it advertises.
>> >> >> 
>> >> >
>> >> > As my understanding, the current limit is dynamic value, it should
>> >> > report the value the charger supports now, eg, it connects SDP, but
>> >> > the host is suspended now, then the value should be 2mA.
>> >> 
>> >> yes, and that's the limit. Now consider we connect to DCP or CDP and
>> >> limit is 2000mA but we're charging at 1000mA ;-)
>> >> 
>> >
>> > Does the user need to know the $this charger limit? Don't they only
>> > care about the current charging value? I have a USB cable which can
>> 
>> Why not ? UI might want to change the color of the battery charging icon
>> if we're charging @ 2000mA or @ 1000mA to give some visual feedback as
>> to "how fast" battery is supposed to be charged.
>> 
>> > show charging current value, it changes from time to time, when it
>> > connects to host pc, it shows 430mA; when it connects to dedicated
>> > charger, it shows 1000mA.
>> 
>> good for you, now what does that have to do with $subject ?
>> 
>
> +static struct attribute *usb_charger_attrs[] = {
> +	&dev_attr_sdp_current.attr,
> +	&dev_attr_dcp_current.attr,
> +	&dev_attr_cdp_current.attr,
> +	&dev_attr_aca_current.attr,
> +	&dev_attr_charger_type.attr,
> +	&dev_attr_charger_state.attr,
> +	NULL
> +};
>
> Ok, even the users are interesting in current limit, we still have no
> necessary to know all kinds of chargers limit and current value, since
> we already have charger type user interface, the framework can show
> limit according to charger type.

Oh, now I get your comment and I totally agree. We already *know* the
detected charger type, there's no point in showing them all.

> I think below user interfaces are enough, who do you think?
>
> +static struct attribute *usb_charger_attrs[] = {
> +	&dev_attr_current.attr,
> +	&dev_attr_current_limit.attr,
> +	&dev_attr_charger_type.attr,
> +	&dev_attr_charger_state.attr,
> +	NULL
> +};

agreed, const though.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Baolin Wang]

On 7 April 2016 at 12:56, Felipe Balbi <balbi@kernel.org> wrote:
>
> Hi,
>
> Peter Chen <hzpeterchen@gmail.com> writes:
>> On Wed, Apr 06, 2016 at 01:25:06PM +0300, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Peter Chen <hzpeterchen@gmail.com> writes:
>>> > On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
>>> >> Peter Chen <hzpeterchen@gmail.com> writes:
>>> >> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
>>> >> >> Peter Chen <hzpeterchen@gmail.com> writes:
>>> >> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>>> >> >> >  +
>>> >> >> >> +static struct attribute *usb_charger_attrs[] = {
>>> >> >> >> +   &dev_attr_sdp_current.attr,
>>> >> >> >> +   &dev_attr_dcp_current.attr,
>>> >> >> >> +   &dev_attr_cdp_current.attr,
>>> >> >> >> +   &dev_attr_aca_current.attr,
>>> >> >> >> +   &dev_attr_charger_type.attr,
>>> >> >> >> +   &dev_attr_charger_state.attr,
>>> >> >> >> +   NULL
>>> >> >> >> +};
>>> >> >> >
>>> >> >> > The user may only care about current limit, type and state, why they
>>> >> >> > need to care what type's current limit, it is the usb charger
>>> >> >> > framework handles, the framework judge the current according to
>>> >> >> > charger type and USB state (connect/configured/suspended).
>>> >> >>
>>> >> >> it might be useful if we want to know that $this charger doesn't really
>>> >> >> give us as much current as it advertises.
>>> >> >>
>>> >> >
>>> >> > As my understanding, the current limit is dynamic value, it should
>>> >> > report the value the charger supports now, eg, it connects SDP, but
>>> >> > the host is suspended now, then the value should be 2mA.
>>> >>
>>> >> yes, and that's the limit. Now consider we connect to DCP or CDP and
>>> >> limit is 2000mA but we're charging at 1000mA ;-)
>>> >>
>>> >
>>> > Does the user need to know the $this charger limit? Don't they only
>>> > care about the current charging value? I have a USB cable which can
>>>
>>> Why not ? UI might want to change the color of the battery charging icon
>>> if we're charging @ 2000mA or @ 1000mA to give some visual feedback as
>>> to "how fast" battery is supposed to be charged.
>>>
>>> > show charging current value, it changes from time to time, when it
>>> > connects to host pc, it shows 430mA; when it connects to dedicated
>>> > charger, it shows 1000mA.
>>>
>>> good for you, now what does that have to do with $subject ?
>>>
>>
>> +static struct attribute *usb_charger_attrs[] = {
>> +     &dev_attr_sdp_current.attr,
>> +     &dev_attr_dcp_current.attr,
>> +     &dev_attr_cdp_current.attr,
>> +     &dev_attr_aca_current.attr,
>> +     &dev_attr_charger_type.attr,
>> +     &dev_attr_charger_state.attr,
>> +     NULL
>> +};
>>
>> Ok, even the users are interesting in current limit, we still have no
>> necessary to know all kinds of chargers limit and current value, since
>> we already have charger type user interface, the framework can show
>> limit according to charger type.
>
> Oh, now I get your comment and I totally agree. We already *know* the
> detected charger type, there's no point in showing them all.
>
>> I think below user interfaces are enough, who do you think?
>>
>> +static struct attribute *usb_charger_attrs[] = {
>> +     &dev_attr_current.attr,
>> +     &dev_attr_current_limit.attr,
>> +     &dev_attr_charger_type.attr,
>> +     &dev_attr_charger_state.attr,
>> +     NULL
>> +};
>
> agreed, const though.

OK. Thanks for Felipe, Peter and Jun's suggestion. I'll modify it in
next version.

>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Peter Chen]

On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
> Peter Chen <hzpeterchen@gmail.com> writes:
> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
> >> Peter Chen <hzpeterchen@gmail.com> writes:
> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
> >> >  +
> >> >> +static struct attribute *usb_charger_attrs[] = {
> >> >> +	&dev_attr_sdp_current.attr,
> >> >> +	&dev_attr_dcp_current.attr,
> >> >> +	&dev_attr_cdp_current.attr,
> >> >> +	&dev_attr_aca_current.attr,
> >> >> +	&dev_attr_charger_type.attr,
> >> >> +	&dev_attr_charger_state.attr,
> >> >> +	NULL
> >> >> +};
> >> >
> >> > The user may only care about current limit, type and state, why they
> >> > need to care what type's current limit, it is the usb charger
> >> > framework handles, the framework judge the current according to
> >> > charger type and USB state (connect/configured/suspended).
> >> 
> >> it might be useful if we want to know that $this charger doesn't really
> >> give us as much current as it advertises.
> >> 
> >
> > As my understanding, the current limit is dynamic value, it should
> > report the value the charger supports now, eg, it connects SDP, but
> > the host is suspended now, then the value should be 2mA.
> 
> yes, and that's the limit. Now consider we connect to DCP or CDP and
> limit is 2000mA but we're charging at 1000mA ;-)
> 

The user doesn't need to know the value which spec designs.

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]


Hi,

Peter Chen <hzpeterchen@gmail.com> writes:
> On Wed, Apr 06, 2016 at 11:05:26AM +0300, Felipe Balbi wrote:
>> Peter Chen <hzpeterchen@gmail.com> writes:
>> > On Wed, Apr 06, 2016 at 10:38:23AM +0300, Felipe Balbi wrote:
>> >> Peter Chen <hzpeterchen@gmail.com> writes:
>> >> > On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
>> >> >  +
>> >> >> +static struct attribute *usb_charger_attrs[] = {
>> >> >> +	&dev_attr_sdp_current.attr,
>> >> >> +	&dev_attr_dcp_current.attr,
>> >> >> +	&dev_attr_cdp_current.attr,
>> >> >> +	&dev_attr_aca_current.attr,
>> >> >> +	&dev_attr_charger_type.attr,
>> >> >> +	&dev_attr_charger_state.attr,
>> >> >> +	NULL
>> >> >> +};
>> >> >
>> >> > The user may only care about current limit, type and state, why they
>> >> > need to care what type's current limit, it is the usb charger
>> >> > framework handles, the framework judge the current according to
>> >> > charger type and USB state (connect/configured/suspended).
>> >> 
>> >> it might be useful if we want to know that $this charger doesn't really
>> >> give us as much current as it advertises.
>> >> 
>> >
>> > As my understanding, the current limit is dynamic value, it should
>> > report the value the charger supports now, eg, it connects SDP, but
>> > the host is suspended now, then the value should be 2mA.
>> 
>> yes, and that's the limit. Now consider we connect to DCP or CDP and
>> limit is 2000mA but we're charging at 1000mA ;-)
>> 
>
> The user doesn't need to know the value which spec designs.

because... ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

RE: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Jun Li]

Hi

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Baolin Wang
> Sent: Friday, April 01, 2016 3:22 PM
> To: balbi@kernel.org; gregkh@linuxfoundation.org; sre@kernel.org;
> dbaryshkov@gmail.com; dwmw2@infradead.org
> Cc: peter.chen@freescale.com; stern@rowland.harvard.edu;
> r.baldyga@samsung.com; yoshihiro.shimoda.uh@renesas.com;
> lee.jones@linaro.org; broonie@kernel.org;
> ckeepax@opensource.wolfsonmicro.com; patches@opensource.wolfsonmicro.com;
> baolin.wang@linaro.org; linux-pm@vger.kernel.org; linux-
> usb@vger.kernel.org; device-mainlining@lists.linuxfoundation.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH v9 1/4] gadget: Introduce the usb charger framework
> 

...

> +/*
> + * usb_charger_get_type() - get the usb charger type with lock protection.
> + * @uchger - usb charger device
> + *
> + * Users can get the charger type by this safe API, rather than using
> +the
> + * usb_charger structure directly.
> + */
> +enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger)
> +{
> +	enum usb_charger_type type;
> +
> +	mutex_lock(&uchger->lock);
> +	type = uchger->type;
> +	mutex_unlock(&uchger->lock);
> +
> +	return type;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_get_type);
> +
> +/*
> + * usb_charger_detect_type() - detect the charger type manually.
> + * @uchger - usb charger device
> + *
> + * Note: You should ensure you need to detect the charger type manually
> +on your
> + * platform.
> + * You should call it at the right gadget state to avoid affecting
> +gadget
> + * enumeration.
> + */
> +int usb_charger_detect_type(struct usb_charger *uchger) {
> +	enum usb_charger_type type;
> +
> +	if (WARN(!uchger->charger_detect,
> +		 "charger detection method should not be NULL"))
> +		return -EINVAL;
> +
> +	type = uchger->charger_detect(uchger);
> +
> +	mutex_lock(&uchger->lock);
> +	uchger->type = type;
> +	mutex_unlock(&uchger->lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_charger_detect_type);
> +
> +/*
> + * usb_charger_get_type_by_others() - Get the usb charger type by the
> +callback
> + * which is implemented by users.
> + * @uchger - the usb charger device.
> + *
> + * Note: This function is just used for getting the charger type, not
> +for
> + * detecting charger type which might affect the DP/DM line when gadget
> +is on
> + * enumeration state.
> + */
> +static enum usb_charger_type
> +usb_charger_get_type_by_others(struct usb_charger *uchger) {
> +	if (uchger->type != UNKNOWN_TYPE)
> +		return uchger->type;
> +
> +	if (uchger->psy) {
> +		union power_supply_propval val;
> +
> +		power_supply_get_property(uchger->psy,
> +					  POWER_SUPPLY_PROP_CHARGE_TYPE,
> +					  &val);
> +		switch (val.intval) {
> +		case POWER_SUPPLY_TYPE_USB:
> +			uchger->type = SDP_TYPE;
> +			break;
> +		case POWER_SUPPLY_TYPE_USB_DCP:
> +			uchger->type = DCP_TYPE;
> +			break;
> +		case POWER_SUPPLY_TYPE_USB_CDP:
> +			uchger->type = CDP_TYPE;
> +			break;
> +		case POWER_SUPPLY_TYPE_USB_ACA:
> +			uchger->type = ACA_TYPE;
> +			break;
> +		default:
> +			uchger->type = UNKNOWN_TYPE;
> +			break;
> +		}
> +	} else if (uchger->get_charger_type) {
> +		uchger->type = uchger->get_charger_type(uchger);
> +	} else {
> +		uchger->type = UNKNOWN_TYPE;
> +	}
> +
> +	return uchger->type;
> +}
> +

I think we may don't need this usb_charger_get_type_by_others().
"uchger->type" is set in one place is enough, that is: by
uchger->charger_detect() in usb_charger_detect_type(), then
usb_charger_get_type_by_others() is replaced by usb_charger_get_type().

uchger->charger_detect() can have diff implementations no matter
what kind of mechanism is used, for your PMIC case, you can just
directly get the type value by power_supply_get_property();
with that, we can have one central place to set uchger->type.
After uchger->type is set, charger type detection is no need to be
involved until charger type changes.

Then next question is where is to call usb_charger_detect_type(),
We need make sure it finished before usb gadget connect.

Ideal is with your framework, diff users only need implement
uchger->charger_detect(). :)

Li Jun

> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Baolin Wang]

On 6 April 2016 at 16:26, Jun Li <jun.li@nxp.com> wrote:
> Hi
>
>> + */
>> +static enum usb_charger_type
>> +usb_charger_get_type_by_others(struct usb_charger *uchger) {
>> +     if (uchger->type != UNKNOWN_TYPE)
>> +             return uchger->type;
>> +
>> +     if (uchger->psy) {
>> +             union power_supply_propval val;
>> +
>> +             power_supply_get_property(uchger->psy,
>> +                                       POWER_SUPPLY_PROP_CHARGE_TYPE,
>> +                                       &val);
>> +             switch (val.intval) {
>> +             case POWER_SUPPLY_TYPE_USB:
>> +                     uchger->type = SDP_TYPE;
>> +                     break;
>> +             case POWER_SUPPLY_TYPE_USB_DCP:
>> +                     uchger->type = DCP_TYPE;
>> +                     break;
>> +             case POWER_SUPPLY_TYPE_USB_CDP:
>> +                     uchger->type = CDP_TYPE;
>> +                     break;
>> +             case POWER_SUPPLY_TYPE_USB_ACA:
>> +                     uchger->type = ACA_TYPE;
>> +                     break;
>> +             default:
>> +                     uchger->type = UNKNOWN_TYPE;
>> +                     break;
>> +             }
>> +     } else if (uchger->get_charger_type) {
>> +             uchger->type = uchger->get_charger_type(uchger);
>> +     } else {
>> +             uchger->type = UNKNOWN_TYPE;
>> +     }
>> +
>> +     return uchger->type;
>> +}
>> +
>
> I think we may don't need this usb_charger_get_type_by_others().
> "uchger->type" is set in one place is enough, that is: by
> uchger->charger_detect() in usb_charger_detect_type(), then
> usb_charger_get_type_by_others() is replaced by usb_charger_get_type().
>
> uchger->charger_detect() can have diff implementations no matter
> what kind of mechanism is used, for your PMIC case, you can just
> directly get the type value by power_supply_get_property();
> with that, we can have one central place to set uchger->type.
> After uchger->type is set, charger type detection is no need to be
> involved until charger type changes.
>
> Then next question is where is to call usb_charger_detect_type(),
> We need make sure it finished before usb gadget connect.

Yeah, that's the point: where? It is hard for usb charger framework to
control, which will make it more complicated. The
'usb_charger_detect_type()' is used for detecting the charger type
manually on user's platform, and user should call it at the right time
to avoid affecting gadget enumeration. Otherwise user can implement
some callbacks showed in 'usb_charger_get_type_by_others()' function
to get charger type. I think this is controllable and simple for the
usb charger framework.

>
> Ideal is with your framework, diff users only need implement
> uchger->charger_detect(). :)
>

-- 
Baolin.wang
Best Regards

RE: [PATCH v9 1/4] gadget: Introduce the usb charger framework

[Jun Li]

Hi

> -----Original Message-----
> From: Baolin Wang [mailto:baolin.wang@linaro.org]
> Sent: Wednesday, April 06, 2016 7:31 PM
> To: Jun Li <jun.li@nxp.com>
> Cc: balbi@kernel.org; gregkh@linuxfoundation.org; sre@kernel.org;
> dbaryshkov@gmail.com; dwmw2@infradead.org; peter.chen@freescale.com;
> stern@rowland.harvard.edu; r.baldyga@samsung.com;
> yoshihiro.shimoda.uh@renesas.com; lee.jones@linaro.org; broonie@kernel.org;
> ckeepax@opensource.wolfsonmicro.com; patches@opensource.wolfsonmicro.com;
> linux-pm@vger.kernel.org; linux-usb@vger.kernel.org; device-
> mainlining@lists.linuxfoundation.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework
> 
> On 6 April 2016 at 16:26, Jun Li <jun.li@nxp.com> wrote:
> > Hi
> >
> >> + */
> >> +static enum usb_charger_type
> >> +usb_charger_get_type_by_others(struct usb_charger *uchger) {
> >> +     if (uchger->type != UNKNOWN_TYPE)
> >> +             return uchger->type;
> >> +
> >> +     if (uchger->psy) {
> >> +             union power_supply_propval val;
> >> +
> >> +             power_supply_get_property(uchger->psy,
> >> +                                       POWER_SUPPLY_PROP_CHARGE_TYPE,
> >> +                                       &val);
> >> +             switch (val.intval) {
> >> +             case POWER_SUPPLY_TYPE_USB:
> >> +                     uchger->type = SDP_TYPE;
> >> +                     break;
> >> +             case POWER_SUPPLY_TYPE_USB_DCP:
> >> +                     uchger->type = DCP_TYPE;
> >> +                     break;
> >> +             case POWER_SUPPLY_TYPE_USB_CDP:
> >> +                     uchger->type = CDP_TYPE;
> >> +                     break;
> >> +             case POWER_SUPPLY_TYPE_USB_ACA:
> >> +                     uchger->type = ACA_TYPE;
> >> +                     break;
> >> +             default:
> >> +                     uchger->type = UNKNOWN_TYPE;
> >> +                     break;
> >> +             }
> >> +     } else if (uchger->get_charger_type) {
> >> +             uchger->type = uchger->get_charger_type(uchger);
> >> +     } else {
> >> +             uchger->type = UNKNOWN_TYPE;
> >> +     }
> >> +
> >> +     return uchger->type;
> >> +}
> >> +
> >
> > I think we may don't need this usb_charger_get_type_by_others().
> > "uchger->type" is set in one place is enough, that is: by
> > uchger->charger_detect() in usb_charger_detect_type(), then
> > usb_charger_get_type_by_others() is replaced by usb_charger_get_type().
> >
> > uchger->charger_detect() can have diff implementations no matter
> > what kind of mechanism is used, for your PMIC case, you can just
> > directly get the type value by power_supply_get_property(); with that,
> > we can have one central place to set uchger->type.
> > After uchger->type is set, charger type detection is no need to be
> > involved until charger type changes.
> >
> > Then next question is where is to call usb_charger_detect_type(), We
> > need make sure it finished before usb gadget connect.
> 
> Yeah, that's the point: where? It is hard for usb charger framework to
> control, which will make it more complicated. The
> 'usb_charger_detect_type()' is used for detecting the charger type
> manually on user's platform, and user should call it at the right time to
> avoid affecting gadget enumeration. Otherwise user can implement some
> callbacks showed in 'usb_charger_get_type_by_others()' function to get
> charger type. I think this is controllable and simple for the usb charger
> framework.

As my suggested, Can this detection be in usb_udc_vbus_handler(), and
before usb_udc_connect_control()? We should be able to find some central
place where it is between vbus detection and gadget connect. 

> 
> >
> > Ideal is with your framework, diff users only need implement
> > uchger->charger_detect(). :)
> >
> 
> --
> Baolin.wang
> Best Regards

[PATCH v9 2/4] gadget: Support for the usb charger framework

[Baolin Wang]

For supporting the usb charger, it adds the usb_charger_init() and
usb_charger_exit() functions for usb charger initialization and exit.

It will report to the usb charger when the gadget state is changed,
then the usb charger can do the power things.

Introduce a callback 'get_charger_type' which will implemented by
user for usb gadget operations to get the usb charger type.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/usb/gadget/udc/udc-core.c |   11 +++++++++++
 include/linux/usb/gadget.h        |   15 +++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b86a6f0..8d98c6b 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb.h>
+#include <linux/usb/charger.h>
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -230,6 +231,9 @@ static void usb_gadget_state_work(struct work_struct *work)
 	struct usb_gadget *gadget = work_to_gadget(work);
 	struct usb_udc *udc = gadget->udc;
 
+	/* when the gadget state is changed, then report to USB charger */
+	usb_charger_plug_by_gadget(gadget, gadget->state);
+
 	if (udc)
 		sysfs_notify(&udc->dev.kobj, NULL, "state");
 }
@@ -423,8 +427,14 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
 
 	mutex_unlock(&udc_lock);
 
+	ret = usb_charger_init(gadget);
+	if (ret)
+		goto err5;
+
 	return 0;
 
+err5:
+	device_del(&udc->dev);
 err4:
 	list_del(&udc->list);
 	mutex_unlock(&udc_lock);
@@ -503,6 +513,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
 	kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
 	flush_work(&gadget->work);
 	device_unregister(&udc->dev);
+	usb_charger_exit(gadget);
 	device_unregister(&gadget->dev);
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..054488a 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -24,6 +24,7 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 #include <linux/usb/ch9.h>
+#include <linux/usb/charger.h>
 
 struct usb_ep;
 
@@ -563,6 +564,8 @@ struct usb_gadget_ops {
 	struct usb_ep *(*match_ep)(struct usb_gadget *,
 			struct usb_endpoint_descriptor *,
 			struct usb_ss_ep_comp_descriptor *);
+	/* get the charger type */
+	enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
 };
 
 /**
@@ -635,6 +638,8 @@ struct usb_gadget {
 	unsigned			out_epnum;
 	unsigned			in_epnum;
 	struct usb_otg_caps		*otg_caps;
+	/* negotiate the power with the usb charger */
+	struct usb_charger		*charger;
 
 	unsigned			sg_supported:1;
 	unsigned			is_otg:1;
@@ -839,10 +844,20 @@ static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
  * reporting how much power the device may consume.  For example, this
  * could affect how quickly batteries are recharged.
  *
+ * It will also notify the USB charger how much power the device may
+ * consume if there is a USB charger linking with the gadget.
+ *
  * Returns zero on success, else negative errno.
  */
 static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 {
+	enum usb_charger_type type;
+
+	if (gadget->charger) {
+		type = usb_charger_get_type(gadget->charger);
+		usb_charger_set_cur_limit_by_type(gadget->charger, type, mA);
+	}
+
 	if (!gadget->ops->vbus_draw)
 		return -EOPNOTSUPP;
 	return gadget->ops->vbus_draw(gadget, mA);
-- 
1.7.9.5

Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Peter Chen]

On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
> For supporting the usb charger, it adds the usb_charger_init() and
> usb_charger_exit() functions for usb charger initialization and exit.
> 
> It will report to the usb charger when the gadget state is changed,
> then the usb charger can do the power things.
> 
> Introduce a callback 'get_charger_type' which will implemented by
> user for usb gadget operations to get the usb charger type.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  drivers/usb/gadget/udc/udc-core.c |   11 +++++++++++
>  include/linux/usb/gadget.h        |   15 +++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b86a6f0..8d98c6b 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -28,6 +28,7 @@
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
>  #include <linux/usb.h>
> +#include <linux/usb/charger.h>
>  
>  /**
>   * struct usb_udc - describes one usb device controller
> @@ -230,6 +231,9 @@ static void usb_gadget_state_work(struct work_struct *work)
>  	struct usb_gadget *gadget = work_to_gadget(work);
>  	struct usb_udc *udc = gadget->udc;
>  
> +	/* when the gadget state is changed, then report to USB charger */
> +	usb_charger_plug_by_gadget(gadget, gadget->state);
> +
>  	if (udc)
>  		sysfs_notify(&udc->dev.kobj, NULL, "state");
>  }
> @@ -423,8 +427,14 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
>  
>  	mutex_unlock(&udc_lock);
>  
> +	ret = usb_charger_init(gadget);
> +	if (ret)
> +		goto err5;
> +
>  	return 0;
>  
> +err5:
> +	device_del(&udc->dev);
>  err4:
>  	list_del(&udc->list);
>  	mutex_unlock(&udc_lock);
> @@ -503,6 +513,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
>  	kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
>  	flush_work(&gadget->work);
>  	device_unregister(&udc->dev);
> +	usb_charger_exit(gadget);
>  	device_unregister(&gadget->dev);
>  }
>  EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index d82d006..054488a 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -24,6 +24,7 @@
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
>  #include <linux/usb/ch9.h>
> +#include <linux/usb/charger.h>
>  
>  struct usb_ep;
>  
> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
>  	struct usb_ep *(*match_ep)(struct usb_gadget *,
>  			struct usb_endpoint_descriptor *,
>  			struct usb_ss_ep_comp_descriptor *);
> +	/* get the charger type */
> +	enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
>  };

Since we already have get_charger_type callback at usb_charger
structure, why we still need this API at usb_gadget_ops?

>  
>  /**
> @@ -635,6 +638,8 @@ struct usb_gadget {
>  	unsigned			out_epnum;
>  	unsigned			in_epnum;
>  	struct usb_otg_caps		*otg_caps;
> +	/* negotiate the power with the usb charger */
> +	struct usb_charger		*charger;
>  
>  	unsigned			sg_supported:1;
>  	unsigned			is_otg:1;
> @@ -839,10 +844,20 @@ static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
>   * reporting how much power the device may consume.  For example, this
>   * could affect how quickly batteries are recharged.
>   *
> + * It will also notify the USB charger how much power the device may
> + * consume if there is a USB charger linking with the gadget.
> + *
>   * Returns zero on success, else negative errno.
>   */
>  static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
>  {
> +	enum usb_charger_type type;
> +
> +	if (gadget->charger) {
> +		type = usb_charger_get_type(gadget->charger);
> +		usb_charger_set_cur_limit_by_type(gadget->charger, type, mA);
> +	}
> +

You may do something redundant.

- Charger detection only occurs at connection period, at other periods,
we only change the current limit, and notify charger IC. That is to
say, we may only need to save charger type and current limit at
usb_charger structure, we don't need to distinguish all chargers
type from time to time.

- The purpose of usb_gadget_vbus_draw design is notify charger IC too,
so you can do set current limit and notify charger IC together at this
API together, it has already covered all situations. We don't need to
notify charger IC again when the gadget status has changed again.

>  	if (!gadget->ops->vbus_draw)
>  		return -EOPNOTSUPP;
>  	return gadget->ops->vbus_draw(gadget, mA);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Baolin Wang]

On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@gmail.com> wrote:
> On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
>>
>> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
>>       struct usb_ep *(*match_ep)(struct usb_gadget *,
>>                       struct usb_endpoint_descriptor *,
>>                       struct usb_ss_ep_comp_descriptor *);
>> +     /* get the charger type */
>> +     enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
>>  };
>
> Since we already have get_charger_type callback at usb_charger
> structure, why we still need this API at usb_gadget_ops?

In case some users want to get charger type at gadget level.

>
>>
>>  /**
>> @@ -635,6 +638,8 @@ struct usb_gadget {
>>       unsigned                        out_epnum;
>>       unsigned                        in_epnum;
>>       struct usb_otg_caps             *otg_caps;
>> +     /* negotiate the power with the usb charger */
>> +     struct usb_charger              *charger;
>>
>>       unsigned                        sg_supported:1;
>>       unsigned                        is_otg:1;
>> @@ -839,10 +844,20 @@ static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
>>   * reporting how much power the device may consume.  For example, this
>>   * could affect how quickly batteries are recharged.
>>   *
>> + * It will also notify the USB charger how much power the device may
>> + * consume if there is a USB charger linking with the gadget.
>> + *
>>   * Returns zero on success, else negative errno.
>>   */
>>  static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
>>  {
>> +     enum usb_charger_type type;
>> +
>> +     if (gadget->charger) {
>> +             type = usb_charger_get_type(gadget->charger);
>> +             usb_charger_set_cur_limit_by_type(gadget->charger, type, mA);
>> +     }
>> +
>
> You may do something redundant.
>
> - Charger detection only occurs at connection period, at other periods,
> we only change the current limit, and notify charger IC. That is to
> say, we may only need to save charger type and current limit at
> usb_charger structure, we don't need to distinguish all chargers
> type from time to time.

That's right. I just want to get the charger type as one parameter to
set current. The function is implemented as below:

enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger)
{
        enum usb_charger_type type;

        mutex_lock(&uchger->lock);
        type = uchger->type;
        mutex_unlock(&uchger->lock);

        return type;
}

>
> - The purpose of usb_gadget_vbus_draw design is notify charger IC too,
> so you can do set current limit and notify charger IC together at this
> API together, it has already covered all situations. We don't need to
> notify charger IC again when the gadget status has changed again.

It did not notify charger IC again. You are right,
usb_gadget_vbus_draw design will notify charger IC, so we want to
record the current in usb charger framework at the same time.

>
>>       if (!gadget->ops->vbus_draw)
>>               return -EOPNOTSUPP;
>>       return gadget->ops->vbus_draw(gadget, mA);
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
>
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Jun Li]

Hi

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Baolin Wang
> Sent: Wednesday, April 06, 2016 6:47 PM
> To: Peter Chen <hzpeterchen@gmail.com>
> Cc: Felipe Balbi <balbi@kernel.org>; Greg KH <gregkh@linuxfoundation.org>;
> Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Peter Chen
> <peter.chen@freescale.com>; Alan Stern <stern@rowland.harvard.edu>;
> r.baldyga@samsung.com; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; Lee Jones <lee.jones@linaro.org>; Mark
> Brown <broonie@kernel.org>; Charles Keepax
> <ckeepax@opensource.wolfsonmicro.com>; patches@opensource.wolfsonmicro.com;
> Linux PM list <linux-pm@vger.kernel.org>; USB <linux-usb@vger.kernel.org>;
> device-mainlining@lists.linuxfoundation.org; LKML <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger framework
> 
> On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@gmail.com> wrote:
> > On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
> >>
> >> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
> >>       struct usb_ep *(*match_ep)(struct usb_gadget *,
> >>                       struct usb_endpoint_descriptor *,
> >>                       struct usb_ss_ep_comp_descriptor *);
> >> +     /* get the charger type */
> >> +     enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
> >>  };
> >
> > Since we already have get_charger_type callback at usb_charger
> > structure, why we still need this API at usb_gadget_ops?
> 
> In case some users want to get charger type at gadget level.
> 
Why gadget needs to know charger type? I also don't catch the intent of
This api, as my understanding, gadget only need report gadget state changes.
All information required for usb charger is charger type and gadget state.

Li Jun

RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]


Hi,

Jun Li <jun.li@nxp.com> writes:
>> -----Original Message-----
>> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
>> owner@vger.kernel.org] On Behalf Of Baolin Wang
>> Sent: Wednesday, April 06, 2016 6:47 PM
>> To: Peter Chen <hzpeterchen@gmail.com>
>> Cc: Felipe Balbi <balbi@kernel.org>; Greg KH <gregkh@linuxfoundation.org>;
>> Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Peter Chen
>> <peter.chen@freescale.com>; Alan Stern <stern@rowland.harvard.edu>;
>> r.baldyga@samsung.com; Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh@renesas.com>; Lee Jones <lee.jones@linaro.org>; Mark
>> Brown <broonie@kernel.org>; Charles Keepax
>> <ckeepax@opensource.wolfsonmicro.com>; patches@opensource.wolfsonmicro.com;
>> Linux PM list <linux-pm@vger.kernel.org>; USB <linux-usb@vger.kernel.org>;
>> device-mainlining@lists.linuxfoundation.org; LKML <linux-
>> kernel@vger.kernel.org>
>> Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger framework
>> 
>> On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@gmail.com> wrote:
>> > On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
>> >>
>> >> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
>> >>       struct usb_ep *(*match_ep)(struct usb_gadget *,
>> >>                       struct usb_endpoint_descriptor *,
>> >>                       struct usb_ss_ep_comp_descriptor *);
>> >> +     /* get the charger type */
>> >> +     enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
>> >>  };
>> >
>> > Since we already have get_charger_type callback at usb_charger
>> > structure, why we still need this API at usb_gadget_ops?
>> 
>> In case some users want to get charger type at gadget level.
>> 
> Why gadget needs to know charger type? I also don't catch the intent of

because some gadgets need to call usb_gadget_vbus_draw(), although for
that they need power in mA rather.

> This api, as my understanding, gadget only need report gadget state changes.
> All information required for usb charger is charger type and gadget state.

you're making an assumption about how the HW is laid out which might not
be true.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Jun Li]

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: Wednesday, April 06, 2016 8:22 PM
> To: Jun Li <jun.li@nxp.com>; Baolin Wang <baolin.wang@linaro.org>; Peter
> Chen <hzpeterchen@gmail.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>; Sebastian Reichel
> <sre@kernel.org>; Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>; David
> Woodhouse <dwmw2@infradead.org>; Peter Chen <peter.chen@freescale.com>;
> Alan Stern <stern@rowland.harvard.edu>; r.baldyga@samsung.com; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@renesas.com>; Lee Jones
> <lee.jones@linaro.org>; Mark Brown <broonie@kernel.org>; Charles Keepax
> <ckeepax@opensource.wolfsonmicro.com>; patches@opensource.wolfsonmicro.com;
> Linux PM list <linux-pm@vger.kernel.org>; USB <linux-usb@vger.kernel.org>;
> device-mainlining@lists.linuxfoundation.org; LKML <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH v9 2/4] gadget: Support for the usb charger framework
> 
> 
> Hi,
> 
> Jun Li <jun.li@nxp.com> writes:
> >> -----Original Message-----
> >> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> >> owner@vger.kernel.org] On Behalf Of Baolin Wang
> >> Sent: Wednesday, April 06, 2016 6:47 PM
> >> To: Peter Chen <hzpeterchen@gmail.com>
> >> Cc: Felipe Balbi <balbi@kernel.org>; Greg KH
> >> <gregkh@linuxfoundation.org>; Sebastian Reichel <sre@kernel.org>;
> >> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>; David Woodhouse
> >> <dwmw2@infradead.org>; Peter Chen <peter.chen@freescale.com>; Alan
> >> Stern <stern@rowland.harvard.edu>; r.baldyga@samsung.com; Yoshihiro
> >> Shimoda <yoshihiro.shimoda.uh@renesas.com>; Lee Jones
> >> <lee.jones@linaro.org>; Mark Brown <broonie@kernel.org>; Charles
> >> Keepax <ckeepax@opensource.wolfsonmicro.com>;
> >> patches@opensource.wolfsonmicro.com;
> >> Linux PM list <linux-pm@vger.kernel.org>; USB
> >> <linux-usb@vger.kernel.org>;
> >> device-mainlining@lists.linuxfoundation.org; LKML <linux-
> >> kernel@vger.kernel.org>
> >> Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger
> >> framework
> >>
> >> On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@gmail.com> wrote:
> >> > On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
> >> >>
> >> >> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
> >> >>       struct usb_ep *(*match_ep)(struct usb_gadget *,
> >> >>                       struct usb_endpoint_descriptor *,
> >> >>                       struct usb_ss_ep_comp_descriptor *);
> >> >> +     /* get the charger type */
> >> >> +     enum usb_charger_type (*get_charger_type)(struct usb_gadget
> >> >> + *);
> >> >>  };
> >> >
> >> > Since we already have get_charger_type callback at usb_charger
> >> > structure, why we still need this API at usb_gadget_ops?
> >>
> >> In case some users want to get charger type at gadget level.
> >>
> > Why gadget needs to know charger type? I also don't catch the intent
> > of
> 
> because some gadgets need to call usb_gadget_vbus_draw(), although for
> that they need power in mA rather.

In below change of usb_gadget_vbus_draw(), already can get charger type
via usb_charger_get_type().

static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 {
+       enum usb_charger_type type;
+
+       if (gadget->charger) {
+               type = usb_charger_get_type(gadget->charger);
+               usb_charger_set_cur_limit_by_type(gadget->charger, type, mA);
+       }
+
        if (!gadget->ops->vbus_draw)
                return -EOPNOTSUPP;
        return gadget->ops->vbus_draw(gadget, mA);

Could you detail in what situation gadget->ops-> get_charger_type() is used?

> 
> > This api, as my understanding, gadget only need report gadget state
> changes.
> > All information required for usb charger is charger type and gadget
> state.
> 
> you're making an assumption about how the HW is laid out which might not
> be true.
> 

What other information you refer to here? Or what I am not aware of?

Thanks.
Li Jun  

> --
> balbi

RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2643 bytes --]


Hi,

Jun Li <jun.li@nxp.com> writes:
>> >> On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@gmail.com> wrote:
>> >> > On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
>> >> >>
>> >> >> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
>> >> >>       struct usb_ep *(*match_ep)(struct usb_gadget *,
>> >> >>                       struct usb_endpoint_descriptor *,
>> >> >>                       struct usb_ss_ep_comp_descriptor *);
>> >> >> +     /* get the charger type */
>> >> >> +     enum usb_charger_type (*get_charger_type)(struct usb_gadget
>> >> >> + *);
>> >> >>  };
>> >> >
>> >> > Since we already have get_charger_type callback at usb_charger
>> >> > structure, why we still need this API at usb_gadget_ops?
>> >>
>> >> In case some users want to get charger type at gadget level.
>> >>
>> > Why gadget needs to know charger type? I also don't catch the intent
>> > of
>> 
>> because some gadgets need to call usb_gadget_vbus_draw(), although for
>> that they need power in mA rather.
>
> In below change of usb_gadget_vbus_draw(), already can get charger type
> via usb_charger_get_type().
>
> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
>  {
> +       enum usb_charger_type type;
> +
> +       if (gadget->charger) {
> +               type = usb_charger_get_type(gadget->charger);
> +               usb_charger_set_cur_limit_by_type(gadget->charger, type, mA);
> +       }
> +
>         if (!gadget->ops->vbus_draw)
>                 return -EOPNOTSUPP;
>         return gadget->ops->vbus_draw(gadget, mA);
>
> Could you detail in what situation gadget->ops-> get_charger_type() is used?

isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling
it ? What did I miss here ?

>> > This api, as my understanding, gadget only need report gadget state
>> changes.
>> > All information required for usb charger is charger type and gadget
>> state.
>> 
>> you're making an assumption about how the HW is laid out which might not
>> be true.
>> 
>
> What other information you refer to here? Or what I am not aware of?

what I'm trying to say is that you're assuming gadgets don't need to
know anything other than charger type and gadget state (suspended,
resume, enumerated, default state, addressed, etc), but that might not
be true for all UDCs. You can't make that assumption that charger type
and gadget state is enough. The real question is what do we need *now*,
but still keep in mind that what we need *now* might be valid 2 years
from now, so API needs to be a little flexible.

cheers

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Jun Li]

Hi

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: Wednesday, April 06, 2016 8:56 PM
> To: Jun Li <jun.li@nxp.com>; Baolin Wang <baolin.wang@linaro.org>; Peter
> Chen <hzpeterchen@gmail.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>; Sebastian Reichel
> <sre@kernel.org>; Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>; David
> Woodhouse <dwmw2@infradead.org>; Peter Chen <peter.chen@freescale.com>;
> Alan Stern <stern@rowland.harvard.edu>; r.baldyga@samsung.com; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@renesas.com>; Lee Jones
> <lee.jones@linaro.org>; Mark Brown <broonie@kernel.org>; Charles Keepax
> <ckeepax@opensource.wolfsonmicro.com>; patches@opensource.wolfsonmicro.com;
> Linux PM list <linux-pm@vger.kernel.org>; USB <linux-usb@vger.kernel.org>;
> device-mainlining@lists.linuxfoundation.org; LKML <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH v9 2/4] gadget: Support for the usb charger framework
> 
> 
> Hi,
> 
> Jun Li <jun.li@nxp.com> writes:
> >> >> On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@gmail.com> wrote:
> >> >> > On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
> >> >> >>
> >> >> >> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
> >> >> >>       struct usb_ep *(*match_ep)(struct usb_gadget *,
> >> >> >>                       struct usb_endpoint_descriptor *,
> >> >> >>                       struct usb_ss_ep_comp_descriptor *);
> >> >> >> +     /* get the charger type */
> >> >> >> +     enum usb_charger_type (*get_charger_type)(struct
> >> >> >> + usb_gadget *);
> >> >> >>  };
> >> >> >
> >> >> > Since we already have get_charger_type callback at usb_charger
> >> >> > structure, why we still need this API at usb_gadget_ops?
> >> >>
> >> >> In case some users want to get charger type at gadget level.
> >> >>
> >> > Why gadget needs to know charger type? I also don't catch the
> >> > intent of
> >>
> >> because some gadgets need to call usb_gadget_vbus_draw(), although
> >> for that they need power in mA rather.
> >
> > In below change of usb_gadget_vbus_draw(), already can get charger
> > type via usb_charger_get_type().
> >
> > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
> > unsigned mA)  {
> > +       enum usb_charger_type type;
> > +
> > +       if (gadget->charger) {
> > +               type = usb_charger_get_type(gadget->charger);
> > +               usb_charger_set_cur_limit_by_type(gadget->charger, type,
> mA);
> > +       }
> > +
> >         if (!gadget->ops->vbus_draw)
> >                 return -EOPNOTSUPP;
> >         return gadget->ops->vbus_draw(gadget, mA);
> >
> > Could you detail in what situation gadget->ops-> get_charger_type() is
> used?
> 
> isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling
> it ? What did I miss here ?

Well, that's true, so my real meaning is why gadget need get charger type
via another new api gadget->ops->get_charger_type().
 
> 
> >> > This api, as my understanding, gadget only need report gadget state
> >> changes.
> >> > All information required for usb charger is charger type and gadget
> >> state.
> >>
> >> you're making an assumption about how the HW is laid out which might
> >> not be true.
> >>
> >
> > What other information you refer to here? Or what I am not aware of?
> 
> what I'm trying to say is that you're assuming gadgets don't need to know
> anything other than charger type and gadget state (suspended, resume,
> enumerated, default state, addressed, etc), but that might not be true for
> all UDCs. You can't make that assumption that charger type and gadget
> state is enough. The real question is what do we need *now*, but still
> keep in mind that what we need *now* might be valid 2 years from now, so
> API needs to be a little flexible.

Get your point, flexible, I just thought create an api without any user
for existing case/spec, wouldn't it be better to let the real user add it
later when it's needed.

> 
> cheers
> 
> --
> balbi

RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2978 bytes --]


Hi,

Jun Li <jun.li@nxp.com> writes:
>> >> >> > Since we already have get_charger_type callback at usb_charger
>> >> >> > structure, why we still need this API at usb_gadget_ops?
>> >> >>
>> >> >> In case some users want to get charger type at gadget level.
>> >> >>
>> >> > Why gadget needs to know charger type? I also don't catch the
>> >> > intent of
>> >>
>> >> because some gadgets need to call usb_gadget_vbus_draw(), although
>> >> for that they need power in mA rather.
>> >
>> > In below change of usb_gadget_vbus_draw(), already can get charger
>> > type via usb_charger_get_type().
>> >
>> > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
>> > unsigned mA)  {
>> > +       enum usb_charger_type type;
>> > +
>> > +       if (gadget->charger) {
>> > +               type = usb_charger_get_type(gadget->charger);
>> > +               usb_charger_set_cur_limit_by_type(gadget->charger, type,
>> mA);
>> > +       }
>> > +
>> >         if (!gadget->ops->vbus_draw)
>> >                 return -EOPNOTSUPP;
>> >         return gadget->ops->vbus_draw(gadget, mA);
>> >
>> > Could you detail in what situation gadget->ops-> get_charger_type() is
>> used?
>> 
>> isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling
>> it ? What did I miss here ?
>
> Well, that's true, so my real meaning is why gadget need get charger type
> via another new api gadget->ops->get_charger_type().

because of semantics. usb_gadget_vbus_draw() is *only* supposed to
connect a load across vbus and gnd so some battery can be charged. Also,
we need to abstract away this ->get_charger_type() operation because it
might be different for each UDC.

$subject has a fragility, however: It's assuming that we should always
call ->get_charger_type() before ->vbus_draw(), but that's a good
default, I'd say.

>> >> > This api, as my understanding, gadget only need report gadget state
>> >> changes.
>> >> > All information required for usb charger is charger type and gadget
>> >> state.
>> >>
>> >> you're making an assumption about how the HW is laid out which might
>> >> not be true.
>> >>
>> >
>> > What other information you refer to here? Or what I am not aware of?
>> 
>> what I'm trying to say is that you're assuming gadgets don't need to know
>> anything other than charger type and gadget state (suspended, resume,
>> enumerated, default state, addressed, etc), but that might not be true for
>> all UDCs. You can't make that assumption that charger type and gadget
>> state is enough. The real question is what do we need *now*, but still
>> keep in mind that what we need *now* might be valid 2 years from now, so
>> API needs to be a little flexible.
>
> Get your point, flexible, I just thought create an api without any user
> for existing case/spec, wouldn't it be better to let the real user add it
> later when it's needed.

that sure is a fair point.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Peter Chen]

On Wed, Apr 06, 2016 at 04:58:03PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Jun Li <jun.li@nxp.com> writes:
> >> >> >> > Since we already have get_charger_type callback at usb_charger
> >> >> >> > structure, why we still need this API at usb_gadget_ops?
> >> >> >>
> >> >> >> In case some users want to get charger type at gadget level.
> >> >> >>
> >> >> > Why gadget needs to know charger type? I also don't catch the
> >> >> > intent of
> >> >>
> >> >> because some gadgets need to call usb_gadget_vbus_draw(), although
> >> >> for that they need power in mA rather.
> >> >
> >> > In below change of usb_gadget_vbus_draw(), already can get charger
> >> > type via usb_charger_get_type().
> >> >
> >> > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
> >> > unsigned mA)  {
> >> > +       enum usb_charger_type type;
> >> > +
> >> > +       if (gadget->charger) {
> >> > +               type = usb_charger_get_type(gadget->charger);
> >> > +               usb_charger_set_cur_limit_by_type(gadget->charger, type,
> >> mA);
> >> > +       }
> >> > +
> >> >         if (!gadget->ops->vbus_draw)
> >> >                 return -EOPNOTSUPP;
> >> >         return gadget->ops->vbus_draw(gadget, mA);
> >> >
> >> > Could you detail in what situation gadget->ops-> get_charger_type() is
> >> used?
> >> 
> >> isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling
> >> it ? What did I miss here ?
> >
> > Well, that's true, so my real meaning is why gadget need get charger type
> > via another new api gadget->ops->get_charger_type().
> 
> because of semantics. usb_gadget_vbus_draw() is *only* supposed to
> connect a load across vbus and gnd so some battery can be charged. Also,
> we need to abstract away this ->get_charger_type() operation because it
> might be different for each UDC.

In this patch set, there are two ->get_charger_type in below two
structures, as my understanding, get_charger_type at struct usb_charger
can be implemented at UDC drivers; But I don't see necessary we
need to implement get_charger_type for usb_gadget_ops at UDC drivers
again. What do you think?

struct usb_gadget_ops {
	....
	        struct usb_ep *(*match_ep)(struct usb_gadget *,
	+       /* get the charger type */
	+       enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
};

struct usb_charger {
	...
+       /* user can get charger type by implementing this callback */
+       enum usb_charger_type   (*get_charger_type)(struct usb_charger
		*);
}
> 
> $subject has a fragility, however: It's assuming that we should always
> call ->get_charger_type() before ->vbus_draw(), but that's a good
> default, I'd say.
> 

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]


Hi,

Peter Chen <hzpeterchen@gmail.com> writes:
>> >> > In below change of usb_gadget_vbus_draw(), already can get charger
>> >> > type via usb_charger_get_type().
>> >> >
>> >> > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget,
>> >> > unsigned mA)  {
>> >> > +       enum usb_charger_type type;
>> >> > +
>> >> > +       if (gadget->charger) {
>> >> > +               type = usb_charger_get_type(gadget->charger);
>> >> > +               usb_charger_set_cur_limit_by_type(gadget->charger, type,
>> >> mA);
>> >> > +       }
>> >> > +
>> >> >         if (!gadget->ops->vbus_draw)
>> >> >                 return -EOPNOTSUPP;
>> >> >         return gadget->ops->vbus_draw(gadget, mA);
>> >> >
>> >> > Could you detail in what situation gadget->ops-> get_charger_type() is
>> >> used?
>> >> 
>> >> isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling
>> >> it ? What did I miss here ?
>> >
>> > Well, that's true, so my real meaning is why gadget need get charger type
>> > via another new api gadget->ops->get_charger_type().
>> 
>> because of semantics. usb_gadget_vbus_draw() is *only* supposed to
>> connect a load across vbus and gnd so some battery can be charged. Also,
>> we need to abstract away this ->get_charger_type() operation because it
>> might be different for each UDC.
>
> In this patch set, there are two ->get_charger_type in below two
> structures, as my understanding, get_charger_type at struct usb_charger
> can be implemented at UDC drivers; But I don't see necessary we
> need to implement get_charger_type for usb_gadget_ops at UDC drivers
> again. What do you think?

I had missed that completely, nice catch. I agree with you, there should
be one place where this is implemented and struct usb_charger sounds
like it is that place.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH v9 3/4] gadget: Integrate with the usb gadget supporting for usb charger

[Baolin Wang]

When the usb gadget supporting for usb charger is ready, the usb charger
should get the type by the 'get_charger_type' callback which is implemented
by the usb gadget operations, and get the usb charger pointer from struct
'usb_gadget'.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/usb/gadget/udc/charger.c |   44 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
index 251f42b..73f0531 100644
--- a/drivers/usb/gadget/udc/charger.c
+++ b/drivers/usb/gadget/udc/charger.c
@@ -272,7 +272,10 @@ usb_charger_get_type_by_others(struct usb_charger *uchger)
 	if (uchger->type != UNKNOWN_TYPE)
 		return uchger->type;
 
-	if (uchger->psy) {
+	if (uchger->gadget->ops && uchger->gadget->ops->get_charger_type) {
+		uchger->type =
+			uchger->gadget->ops->get_charger_type(uchger->gadget);
+	} else if (uchger->psy) {
 		union power_supply_propval val;
 
 		power_supply_get_property(uchger->psy,
@@ -554,6 +557,30 @@ usb_charger_plug_by_extcon(struct notifier_block *nb,
 int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
 			       unsigned long state)
 {
+	struct usb_charger *uchger = gadget->charger;
+	enum usb_charger_state uchger_state;
+
+	if (WARN(!uchger, "charger can not be NULL"))
+		return -EINVAL;
+
+	/*
+	 * Report event to power to setting the current limitation
+	 * for this usb charger when one usb charger state is changed
+	 * with detecting by usb gadget state.
+	 */
+	if (uchger->old_gadget_state != state) {
+		uchger->old_gadget_state = state;
+
+		if (state >= USB_STATE_ATTACHED)
+			uchger_state = USB_CHARGER_PRESENT;
+		else if (state == USB_STATE_NOTATTACHED)
+			uchger_state = USB_CHARGER_REMOVE;
+		else
+			uchger_state = USB_CHARGER_DEFAULT;
+
+		usb_charger_notify_others(uchger, uchger_state);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
@@ -712,6 +739,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
 
 	/* register a notifier on a usb gadget device */
 	uchger->gadget = ugadget;
+	ugadget->charger = uchger;
 	uchger->old_gadget_state = ugadget->state;
 
 	/* register a new usb charger */
@@ -732,7 +760,19 @@ fail:
 
 int usb_charger_exit(struct usb_gadget *ugadget)
 {
-	return 0;
+	struct usb_charger *uchger = ugadget->charger;
+
+	if (WARN(!uchger, "charger can not be NULL"))
+		return -EINVAL;
+
+	if (uchger->extcon_dev)
+		extcon_unregister_notifier(uchger->extcon_dev,
+					   EXTCON_USB,
+					   &uchger->extcon_nb.nb);
+
+	ida_simple_remove(&usb_charger_ida, uchger->id);
+
+	return usb_charger_unregister(uchger);
 }
 
 static int __init usb_charger_class_init(void)
-- 
1.7.9.5

[PATCH v9 4/4] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.

Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: Peter Chen <peter.chen@freescale.com>
Acked-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/power/wm831x_power.c     |   69 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/wm831x/pdata.h |    3 ++
 2 files changed, 72 insertions(+)

diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..cef1812 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
+#include <linux/usb/charger.h>
 
 #include <linux/mfd/wm831x/core.h>
 #include <linux/mfd/wm831x/auxadc.h>
@@ -31,6 +32,8 @@ struct wm831x_power {
 	char usb_name[20];
 	char battery_name[20];
 	bool have_battery;
+	struct usb_charger *usb_charger;
+	struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+	0,
+	2,
+	100,
+	500,
+	900,
+	1500,
+	1800,
+	550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+				   unsigned long limit, void *data)
+{
+	struct wm831x_power *wm831x_power = container_of(nb,
+							 struct wm831x_power,
+							 usb_notify);
+	unsigned int i, best;
+
+	/* Find the highest supported limit */
+	best = 0;
+	for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+		if (limit >= wm831x_usb_limits[i] &&
+		    wm831x_usb_limits[best] < wm831x_usb_limits[i])
+			best = i;
+	}
+
+	dev_dbg(wm831x_power->wm831x->dev,
+		"Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+	wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+		        WM831X_USB_ILIM_MASK, best);
+
+	return 0;
+}
+
 /*********************************************************************
  *		Battery properties
  *********************************************************************/
@@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+		power->usb_charger =
+			usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+		if (IS_ERR(power->usb_charger)) {
+			ret = PTR_ERR(power->usb_charger);
+			dev_err(&pdev->dev,
+				"Failed to find USB gadget: %d\n", ret);
+			goto err_bat_irq;
+		}
+
+		power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+		ret = usb_charger_register_notify(power->usb_charger,
+						  &power->usb_notify);
+		if (ret != 0) {
+			dev_err(&pdev->dev,
+				"Failed to register notifier: %d\n", ret);
+			goto err_usb_charger;
+		}
+	}
+
 	return ret;
 
+err_usb_charger:
+	/* put_device on charger */
 err_bat_irq:
 	--i;
 	for (; i >= 0; i--) {
@@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device *pdev)
 	struct wm831x *wm831x = wm831x_power->wm831x;
 	int irq, i;
 
+	if (wm831x_power->usb_charger) {
+		usb_charger_unregister_notify(wm831x_power->usb_charger,
+					      &wm831x_power->usb_notify);
+		/* Free charger */
+	}
+
 	for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
 		irq = wm831x_irq(wm831x, 
 				 platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
 	/** The driver should initiate a power off sequence during shutdown */
 	bool soft_shutdown;
 
+	/** dev_name of USB charger gadget to integrate with */
+	const char *usb_gadget;
+
 	int irq_base;
 	int gpio_base;
 	int gpio_defaults[WM831X_GPIO_NUM];
-- 
1.7.9.5

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Fri, Apr 01, 2016 at 03:21:48PM +0800, Baolin Wang wrote:
> Currently the Linux kernel does not provide any standard integration of this
> feature that integrates the USB subsystem with the system power regulation
> provided by PMICs meaning that either vendors must add this in their kernels
> or USB gadget devices based on Linux (such as mobile phones) may not behave
> as they should. Thus provide a standard framework for doing this in kernel.
> 
> Now introduce one user with wm831x_power to support and test the usb charger,
> which is pending testing. Moreover there may be other potential users will use
> it in future.
> 
> Changes since v8:
>  - Remove the bus type things.
>  - Introduce one charger_detect() callback to detect charger type if it is needed.
>  - Add some sysfs attributes for showing the charger type and state.
>  - Change the charger current attributes to read only.
>  - Move charger state and type definition to include/uapi.
>  - Some other optimiazations.
> 
> Baolin Wang (4):
>   gadget: Introduce the usb charger framework
>   gadget: Support for the usb charger framework
>   gadget: Integrate with the usb gadget supporting for usb charger
>   power: wm831x_power: Support USB charger current limit management
> 

We are thinking USB charger framework for Freescale i.mx SoC series,
since our internal framework is not good enough.
So I have more questions for your framework since there are many
different USB charger designs, and I hope it is universal.

- I would like to see your all code to let the charger work, eg
you have said the charger detection is done by PMIC automatically,
but I did not find your PMIC code to read charger type.

Besides, how you can make sure the charger detection has finished
before the framework handles USB_CHARGER_PRESENT event?

- I commented the current limit at different situations for USB
charger last time, but I have not seen your further comments.
I would like give it again. For DCP, you can notify charger IC
once you get the charger type. But for CDP/SDP, you need to
notify charger IC after set configuration has finished, since
the host may still not be ready to give high current.

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

Hi Peter,

On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@gmail.com> wrote:
>
> We are thinking USB charger framework for Freescale i.mx SoC series,
> since our internal framework is not good enough.
> So I have more questions for your framework since there are many
> different USB charger designs, and I hope it is universal.

Great, thanks for your attention and suggestions.

>
> - I would like to see your all code to let the charger work, eg
> you have said the charger detection is done by PMIC automatically,
> but I did not find your PMIC code to read charger type.

Yeah, this patchset did not give an example to read charger type from
PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
But I think user can get it easily by implementing below callbacks.
(1) gadget->ops->get_charger_type();
(2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
(3) uchger->get_charger_type();

In some solutions, when one cable is plugin, the PMIC will detect the
charger type automatically and you can read it from PMIC registers
then. Then maybe one interrupt or other ways to report to gadget to do
enumeration. So user can read the PMIC registers to get charger type
by implementing any above methods.

>
> Besides, how you can make sure the charger detection has finished
> before the framework handles USB_CHARGER_PRESENT event?

I think we don't need to care about this situation. If the charger
type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
the charger framework will not set current (current is 0) for power
driver.

>
> - I commented the current limit at different situations for USB
> charger last time, but I have not seen your further comments.
> I would like give it again. For DCP, you can notify charger IC
> once you get the charger type. But for CDP/SDP, you need to
> notify charger IC after set configuration has finished, since
> the host may still not be ready to give high current.

As my understanding, if the usb charger framework get the charger
type, it means we can notify the power driver to set the current. If
you don't ready for setting current, please don't give the charger
type to usb charger framework.

The framework does not want to focus on charger detection too much,
and just supplies one callback '->charger_detect()' for user to be
implemented if they ensure they need to do the SW charger detection
manually (Note: must at the right time to do the SW detection.). So
the usb charger just focus on dealing with the usb gadget power
negotiation, and it does not need to care much how to do charger
detection on your platform.

>
> --
>
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
> Hi Peter,
> 
> On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@gmail.com> wrote:
> >
> > We are thinking USB charger framework for Freescale i.mx SoC series,
> > since our internal framework is not good enough.
> > So I have more questions for your framework since there are many
> > different USB charger designs, and I hope it is universal.
> 
> Great, thanks for your attention and suggestions.
> 
> >
> > - I would like to see your all code to let the charger work, eg
> > you have said the charger detection is done by PMIC automatically,
> > but I did not find your PMIC code to read charger type.
> 
> Yeah, this patchset did not give an example to read charger type from
> PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
> But I think user can get it easily by implementing below callbacks.
> (1) gadget->ops->get_charger_type();
> (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
> (3) uchger->get_charger_type();
> 

I just would like if you can have this, then, we (you) can test it, eg,
you can test if the wm831x can charge more than 1500mA for DCP.

> In some solutions, when one cable is plugin, the PMIC will detect the
> charger type automatically and you can read it from PMIC registers
> then. Then maybe one interrupt or other ways to report to gadget to do
> enumeration. So user can read the PMIC registers to get charger type
> by implementing any above methods.
> 

Wait. When the USB cable is plugging in, which module will get
interrupt? Only PMIC or Both PMIC and USB (or extcon)? If PMIC
can trigger USB "connect" interrupt, then it is ok.

> >
> > Besides, how you can make sure the charger detection has finished
> > before the framework handles USB_CHARGER_PRESENT event?
> 
> I think we don't need to care about this situation. If the charger
> type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
> the charger framework will not set current (current is 0) for power
> driver.

Then, when we notify the charger IC for larger current, eg, for DCP.

> 
> >
> > - I commented the current limit at different situations for USB
> > charger last time, but I have not seen your further comments.
> > I would like give it again. For DCP, you can notify charger IC
> > once you get the charger type. But for CDP/SDP, you need to
> > notify charger IC after set configuration has finished, since
> > the host may still not be ready to give high current.
> 
> As my understanding, if the usb charger framework get the charger
> type, it means we can notify the power driver to set the current. If
> you don't ready for setting current, please don't give the charger
> type to usb charger framework.
> 
> The framework does not want to focus on charger detection too much,
> and just supplies one callback '->charger_detect()' for user to be
> implemented if they ensure they need to do the SW charger detection
> manually (Note: must at the right time to do the SW detection.). So
> the usb charger just focus on dealing with the usb gadget power
> negotiation, and it does not need to care much how to do charger
> detection on your platform.

No, this comment is common one, but only for SW detection. Eg, when
the PMIC tells you it is a SDP, you can't notify to charger IC about
500mA at once, you need to do it after host allows you to do it.

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 5 April 2016 at 16:12, Peter Chen <hzpeterchen@gmail.com> wrote:
> On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
>> Hi Peter,
>>
>> On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@gmail.com> wrote:
>> >
>> > We are thinking USB charger framework for Freescale i.mx SoC series,
>> > since our internal framework is not good enough.
>> > So I have more questions for your framework since there are many
>> > different USB charger designs, and I hope it is universal.
>>
>> Great, thanks for your attention and suggestions.
>>
>> >
>> > - I would like to see your all code to let the charger work, eg
>> > you have said the charger detection is done by PMIC automatically,
>> > but I did not find your PMIC code to read charger type.
>>
>> Yeah, this patchset did not give an example to read charger type from
>> PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
>> But I think user can get it easily by implementing below callbacks.
>> (1) gadget->ops->get_charger_type();
>> (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
>> (3) uchger->get_charger_type();
>>
>
> I just would like if you can have this, then, we (you) can test it, eg,
> you can test if the wm831x can charge more than 1500mA for DCP.

Mark, could you please address Peter's comments about if the the
wm831x can charge more than 1500mA for DCP? (I have no environment to
test wm831x) Thanks.

>
>> In some solutions, when one cable is plugin, the PMIC will detect the
>> charger type automatically and you can read it from PMIC registers
>> then. Then maybe one interrupt or other ways to report to gadget to do
>> enumeration. So user can read the PMIC registers to get charger type
>> by implementing any above methods.
>>
>
> Wait. When the USB cable is plugging in, which module will get
> interrupt? Only PMIC or Both PMIC and USB (or extcon)? If PMIC
> can trigger USB "connect" interrupt, then it is ok.

Sorry for confusing you. What I mean is this is one hardware design,
when USB cable is plugging in, the PMIC will detect the charger type
automatically and the gadget does not know any plugin event at the
same time. Then after the hardware detection, system will create one
VBUS interrupt to gadget to do enumeration. Anyway, no matter how you
design your hardware detection method, it should guarantee the charger
detection action safely.

>
>> >
>> > Besides, how you can make sure the charger detection has finished
>> > before the framework handles USB_CHARGER_PRESENT event?
>>
>> I think we don't need to care about this situation. If the charger
>> type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
>> the charger framework will not set current (current is 0) for power
>> driver.
>
> Then, when we notify the charger IC for larger current, eg, for DCP.

I suppose If the usb charger framework gets the charger type, then
notify the charger IC for larger current.

>
>>
>> >
>> > - I commented the current limit at different situations for USB
>> > charger last time, but I have not seen your further comments.
>> > I would like give it again. For DCP, you can notify charger IC
>> > once you get the charger type. But for CDP/SDP, you need to
>> > notify charger IC after set configuration has finished, since
>> > the host may still not be ready to give high current.
>>
>> As my understanding, if the usb charger framework get the charger
>> type, it means we can notify the power driver to set the current. If
>> you don't ready for setting current, please don't give the charger
>> type to usb charger framework.
>>
>> The framework does not want to focus on charger detection too much,
>> and just supplies one callback '->charger_detect()' for user to be
>> implemented if they ensure they need to do the SW charger detection
>> manually (Note: must at the right time to do the SW detection.). So
>> the usb charger just focus on dealing with the usb gadget power
>> negotiation, and it does not need to care much how to do charger
>> detection on your platform.
>
> No, this comment is common one, but only for SW detection. Eg, when
> the PMIC tells you it is a SDP, you can't notify to charger IC about
> 500mA at once, you need to do it after host allows you to do it.

Well. Sounds reasonable. I just give an ugly example to implement the
'gadget->ops->get_charger_type()' method to get the charger type.

enum usb_charger_type get_charger_type(struct usb_gadget *gadget)
{
        if (host is allowed)
                read charger type from PMIC;
        else
                return UNKNOWN_TYPE;
}

So that will makes usb charger do not need to care about too much to
make things more complicated. Or do you have any other good
suggestions? Thanks.

>
> --
>
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Tue, Apr 05, 2016 at 05:34:02PM +0800, Baolin Wang wrote:
> On 5 April 2016 at 16:12, Peter Chen <hzpeterchen@gmail.com> wrote:
> > On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
> >> Hi Peter,
> >>
> >> On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@gmail.com> wrote:
> >> >
> >> > We are thinking USB charger framework for Freescale i.mx SoC series,
> >> > since our internal framework is not good enough.
> >> > So I have more questions for your framework since there are many
> >> > different USB charger designs, and I hope it is universal.
> >>
> >> Great, thanks for your attention and suggestions.
> >>
> >> >
> >> > - I would like to see your all code to let the charger work, eg
> >> > you have said the charger detection is done by PMIC automatically,
> >> > but I did not find your PMIC code to read charger type.
> >>
> >> Yeah, this patchset did not give an example to read charger type from
> >> PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
> >> But I think user can get it easily by implementing below callbacks.
> >> (1) gadget->ops->get_charger_type();
> >> (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
> >> (3) uchger->get_charger_type();
> >>
> >
> > I just would like if you can have this, then, we (you) can test it, eg,
> > you can test if the wm831x can charge more than 1500mA for DCP.
> 
> Mark, could you please address Peter's comments about if the the
> wm831x can charge more than 1500mA for DCP? (I have no environment to
> test wm831x) Thanks.
> 

I don't want you or Mark to test at hardware, I just would like to see
some code that how PMIC, wm831x, and USB gadget driver work together.

> >> >
> >> > Besides, how you can make sure the charger detection has finished
> >> > before the framework handles USB_CHARGER_PRESENT event?
> >>
> >> I think we don't need to care about this situation. If the charger
> >> type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
> >> the charger framework will not set current (current is 0) for power
> >> driver.
> >
> > Then, when we notify the charger IC for larger current, eg, for DCP.
> 
> I suppose If the usb charger framework gets the charger type, then
> notify the charger IC for larger current.

It is no problem for software detection, but for hardware one, it must
make sure it sends "USB_CHARGER_PRESENT" event after detection has
finished, otherwise no one will notify charger framework for DCP.

> 
> >
> >>
> >> >
> >> > - I commented the current limit at different situations for USB
> >> > charger last time, but I have not seen your further comments.
> >> > I would like give it again. For DCP, you can notify charger IC
> >> > once you get the charger type. But for CDP/SDP, you need to
> >> > notify charger IC after set configuration has finished, since
> >> > the host may still not be ready to give high current.
> >>
> >> As my understanding, if the usb charger framework get the charger
> >> type, it means we can notify the power driver to set the current. If
> >> you don't ready for setting current, please don't give the charger
> >> type to usb charger framework.
> >>
> >> The framework does not want to focus on charger detection too much,
> >> and just supplies one callback '->charger_detect()' for user to be
> >> implemented if they ensure they need to do the SW charger detection
> >> manually (Note: must at the right time to do the SW detection.). So
> >> the usb charger just focus on dealing with the usb gadget power
> >> negotiation, and it does not need to care much how to do charger
> >> detection on your platform.
> >
> > No, this comment is common one, but only for SW detection. Eg, when
> > the PMIC tells you it is a SDP, you can't notify to charger IC about
> > 500mA at once, you need to do it after host allows you to do it.
> 
> Well. Sounds reasonable. I just give an ugly example to implement the
> 'gadget->ops->get_charger_type()' method to get the charger type.
> 
> enum usb_charger_type get_charger_type(struct usb_gadget *gadget)
> {
>         if (host is allowed)
>                 read charger type from PMIC;
>         else
>                 return UNKNOWN_TYPE;
> }
> 
> So that will makes usb charger do not need to care about too much to
> make things more complicated. Or do you have any other good
> suggestions? Thanks.
> 

Since it is a USB charger, you had to be involved with USB stuffs:).

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 5 April 2016 at 17:43, Peter Chen <hzpeterchen@gmail.com> wrote:
> On Tue, Apr 05, 2016 at 05:34:02PM +0800, Baolin Wang wrote:
>> On 5 April 2016 at 16:12, Peter Chen <hzpeterchen@gmail.com> wrote:
>> > On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
>> >> Hi Peter,
>> >>
>> >> On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@gmail.com> wrote:
>> >> >
>> >> > We are thinking USB charger framework for Freescale i.mx SoC series,
>> >> > since our internal framework is not good enough.
>> >> > So I have more questions for your framework since there are many
>> >> > different USB charger designs, and I hope it is universal.
>> >>
>> >> Great, thanks for your attention and suggestions.
>> >>
>> >> >
>> >> > - I would like to see your all code to let the charger work, eg
>> >> > you have said the charger detection is done by PMIC automatically,
>> >> > but I did not find your PMIC code to read charger type.
>> >>
>> >> Yeah, this patchset did not give an example to read charger type from
>> >> PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
>> >> But I think user can get it easily by implementing below callbacks.
>> >> (1) gadget->ops->get_charger_type();
>> >> (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
>> >> (3) uchger->get_charger_type();
>> >>
>> >
>> > I just would like if you can have this, then, we (you) can test it, eg,
>> > you can test if the wm831x can charge more than 1500mA for DCP.
>>
>> Mark, could you please address Peter's comments about if the the
>> wm831x can charge more than 1500mA for DCP? (I have no environment to
>> test wm831x) Thanks.
>>
>
> I don't want you or Mark to test at hardware, I just would like to see
> some code that how PMIC, wm831x, and USB gadget driver work together.

The whole code are in this patchset.

>
>> >> >
>> >> > Besides, how you can make sure the charger detection has finished
>> >> > before the framework handles USB_CHARGER_PRESENT event?
>> >>
>> >> I think we don't need to care about this situation. If the charger
>> >> type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
>> >> the charger framework will not set current (current is 0) for power
>> >> driver.
>> >
>> > Then, when we notify the charger IC for larger current, eg, for DCP.
>>
>> I suppose If the usb charger framework gets the charger type, then
>> notify the charger IC for larger current.
>
> It is no problem for software detection, but for hardware one, it must
> make sure it sends "USB_CHARGER_PRESENT" event after detection has
> finished, otherwise no one will notify charger framework for DCP.

Yeah, the hardware detection should guarantee charger detection will
be finished firstly.

>
>>
>> >
>> >>
>> >> >
>> >> > - I commented the current limit at different situations for USB
>> >> > charger last time, but I have not seen your further comments.
>> >> > I would like give it again. For DCP, you can notify charger IC
>> >> > once you get the charger type. But for CDP/SDP, you need to
>> >> > notify charger IC after set configuration has finished, since
>> >> > the host may still not be ready to give high current.
>> >>
>> >> As my understanding, if the usb charger framework get the charger
>> >> type, it means we can notify the power driver to set the current. If
>> >> you don't ready for setting current, please don't give the charger
>> >> type to usb charger framework.
>> >>
>> >> The framework does not want to focus on charger detection too much,
>> >> and just supplies one callback '->charger_detect()' for user to be
>> >> implemented if they ensure they need to do the SW charger detection
>> >> manually (Note: must at the right time to do the SW detection.). So
>> >> the usb charger just focus on dealing with the usb gadget power
>> >> negotiation, and it does not need to care much how to do charger
>> >> detection on your platform.
>> >
>> > No, this comment is common one, but only for SW detection. Eg, when
>> > the PMIC tells you it is a SDP, you can't notify to charger IC about
>> > 500mA at once, you need to do it after host allows you to do it.
>>
>> Well. Sounds reasonable. I just give an ugly example to implement the
>> 'gadget->ops->get_charger_type()' method to get the charger type.
>>
>> enum usb_charger_type get_charger_type(struct usb_gadget *gadget)
>> {
>>         if (host is allowed)
>>                 read charger type from PMIC;
>>         else
>>                 return UNKNOWN_TYPE;
>> }
>>
>> So that will makes usb charger do not need to care about too much to
>> make things more complicated. Or do you have any other good
>> suggestions? Thanks.
>>
>
> Since it is a USB charger, you had to be involved with USB stuffs:).

OK. Maybe I need to do some investigation about that. Thanks.

>
> --
>
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

On Tue, Apr 05, 2016 at 05:43:20PM +0800, Peter Chen wrote:
> On Tue, Apr 05, 2016 at 05:34:02PM +0800, Baolin Wang wrote:

> > Mark, could you please address Peter's comments about if the the
> > wm831x can charge more than 1500mA for DCP? (I have no environment to
> > test wm831x) Thanks.

> I don't want you or Mark to test at hardware, I just would like to see
> some code that how PMIC, wm831x, and USB gadget driver work together.

If you want to see this running it's going to be easier for you to just
write an equivalent driver that just does a print instead of writing to
a register (which is all that the wm831x driver boils down to) and test
on hardware you have in front of you.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

On Tue, Apr 05, 2016 at 04:12:22PM +0800, Peter Chen wrote:
> On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
> > Hi Peter,

> > Yeah, this patchset did not give an example to read charger type from
> > PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
> > But I think user can get it easily by implementing below callbacks.
> > (1) gadget->ops->get_charger_type();
> > (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
> > (3) uchger->get_charger_type();

> I just would like if you can have this, then, we (you) can test it, eg,
> you can test if the wm831x can charge more than 1500mA for DCP.

The hardware in the wm831x is extremely simple here - all it does is
take in the power rail from USB, apply a current limit to it and feed
it into the power source selection circuitry it has.  It doesn't see any
of the USB data signals, it relies on the rest of the system to identify
and configure the limit by writing a register.  The highest limit it
supports is 1.8A.

> > The framework does not want to focus on charger detection too much,
> > and just supplies one callback '->charger_detect()' for user to be
> > implemented if they ensure they need to do the SW charger detection
> > manually (Note: must at the right time to do the SW detection.). So
> > the usb charger just focus on dealing with the usb gadget power
> > negotiation, and it does not need to care much how to do charger
> > detection on your platform.

> No, this comment is common one, but only for SW detection. Eg, when
> the PMIC tells you it is a SDP, you can't notify to charger IC about
> 500mA at once, you need to do it after host allows you to do it.

Note that this isn't just the charger device that needs to constrain
current consumption - it's the entire system.  You can't charge to the
limit for system power draw if the USB controller is supplying the main
system rail.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Tue, Apr 05, 2016 at 09:53:05AM -0700, Mark Brown wrote:
> 
> > > The framework does not want to focus on charger detection too much,
> > > and just supplies one callback '->charger_detect()' for user to be
> > > implemented if they ensure they need to do the SW charger detection
> > > manually (Note: must at the right time to do the SW detection.). So
> > > the usb charger just focus on dealing with the usb gadget power
> > > negotiation, and it does not need to care much how to do charger
> > > detection on your platform.
> 
> > No, this comment is common one, but only for SW detection. Eg, when
> > the PMIC tells you it is a SDP, you can't notify to charger IC about
> > 500mA at once, you need to do it after host allows you to do it.
> 
> Note that this isn't just the charger device that needs to constrain
> current consumption - it's the entire system.  You can't charge to the
> limit for system power draw if the USB controller is supplying the main
> system rail.

Sorry, I can't catch up you. USB Host (SDP or CDP) supplies power for
USB device (not only USB controller, it is the whole system), it can
supply more power after set configuration. See 1.4.13 from BC 1.2.

An SDP expects a downstream device with a good battery to draw less than
2.5mA average when unconnected or suspended, up to 100mA maximum when
connected and not configured and not suspended, and up to 500mA maximum
if so configured and not suspended.

-- 

Best Regards,
Peter Chen

Re: [PATCH v9 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On Wed, Apr 06, 2016 at 09:15:26AM +0800, Peter Chen wrote:

> > > No, this comment is common one, but only for SW detection. Eg, when
> > > the PMIC tells you it is a SDP, you can't notify to charger IC about
> > > 500mA at once, you need to do it after host allows you to do it.

> > Note that this isn't just the charger device that needs to constrain
> > current consumption - it's the entire system.  You can't charge to the
> > limit for system power draw if the USB controller is supplying the main
> > system rail.

> Sorry, I can't catch up you. USB Host (SDP or CDP) supplies power for
> USB device (not only USB controller, it is the whole system), it can
> supply more power after set configuration. See 1.4.13 from BC 1.2.

You're saying we need to notify the charger.  The charger does not in
general control the overall system current draw.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]