Introduce usb charger framework to deal with the usb gadget power negotation

drivers/usb/gadget/udc/charger.c new file +13 -0

+
+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) {

drivers/usb/gadget/udc/charger.c new file +10 -0

+
+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)

drivers/usb/gadget/udc/charger.c new file +71 -0

+	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 <[email protected]>");
+MODULE_DESCRIPTION("USB charger driver");
+MODULE_LICENSE("GPL v2");

include/linux/usb/charger.h new file +171 -0

new file mode 100644
index 0000000..1bf1d55
@@ -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__ */

include/uapi/linux/usb/charger.h new file +31 -0

new file mode 100644
index 0000000..3c56ec4
@@ -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 */

Signature · 6 lines

1.7.9.5

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

Quoted 18 lines · Baolin Wang

drivers/usb/gadget/udc/charger.c new file +18 -0

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

drivers/usb/gadget/udc/charger.c line 667

Quoted 5 lines

drivers/usb/gadget/udc/charger.c new file +5 -0

+     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?

Quoted 16 lines

drivers/usb/gadget/udc/charger.c new file +15 -0

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

drivers/usb/gadget/udc/charger.c line 687

Quoted 5 lines

drivers/usb/gadget/udc/charger.c new file +5 -0

+
+     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.

drivers/usb/gadget/udc/charger.c new file +598 -0

+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 <[email protected]>");
+MODULE_DESCRIPTION("USB charger driver");
+MODULE_LICENSE("GPL v2");

include/linux/usb/charger.h new file +171 -0

new file mode 100644
index 0000000..1bf1d55
@@ -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__ */

include/uapi/linux/usb/charger.h new file +31 -0

new file mode 100644
index 0000000..3c56ec4
@@ -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 */

Signature · 6 lines

1.7.9.5

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

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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).

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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.

Quoted 18 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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).

Quoted 3 lines

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.

Quoted 23 lines · Peter Chen

Quoted 21 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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.

Quoted 3 lines

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

Quoted 28 lines · Felipe Balbi

Quoted 26 lines · Peter Chen

Quoted 18 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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).

Quoted 3 lines

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.

Quoted 3 lines

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.

Hi,

Quoted 29 lines · Peter Chen

Quoted 28 lines · Felipe Balbi

Quoted 26 lines · Peter Chen

Quoted 18 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

Review

Quoted 9 lines

(quoted diff) +9 -0

+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).

Quoted 3 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

Quoted 3 lines

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 ?

Quoted 50 lines · Felipe Balbi

Hi,

Quoted 29 lines · Peter Chen

Quoted 28 lines · Felipe Balbi

Quoted 26 lines · Peter Chen

Quoted 18 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

Review

Quoted 9 lines

(quoted diff) +9 -0

+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).

Quoted 3 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

Quoted 3 lines

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 ?

(quoted diff) +9 -0

+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.

Hi,

Quoted 62 lines · Peter Chen

Quoted 50 lines · Felipe Balbi

Hi,

Quoted 29 lines · Peter Chen

Quoted 28 lines · Felipe Balbi

Quoted 26 lines · Peter Chen

Quoted 18 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

Review

Quoted 9 lines

(quoted diff) +9 -0

+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).

Quoted 3 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

Quoted 3 lines

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 ?

(quoted diff) +9 -0

+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
+};

Quoted 4 lines

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.

Quoted 2 lines

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

Review

Quoted 7 lines

(quoted diff) +7 -0

+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

Quoted 23 lines · Peter Chen

Quoted 21 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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.

Quoted 3 lines

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

Quoted 28 lines · Felipe Balbi

Quoted 26 lines · Peter Chen

Quoted 18 lines · Felipe Balbi

Quoted 16 lines · Peter Chen

On Fri, Apr 01, 2016 at 03:21:49PM +0800, Baolin Wang wrote:
+

drivers/usb/gadget/udc/charger.c line 160

Quoted 9 lines

drivers/usb/gadget/udc/charger.c new file +9 -0

+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).

Quoted 3 lines

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.

Quoted 3 lines

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.

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Baolin Wang
Sent: Friday, April 01, 2016 3:22 PM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux- [email protected]
Subject: [PATCH v9 1/4] gadget: Introduce the usb charger framework

drivers/usb/gadget/udc/charger.c new file +87 -0

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

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

Hi

Quoted 28 lines

+ */

drivers/usb/gadget/udc/charger.c new file +27 -0

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

drivers/usb/gadget/udc/charger.c line 306

Quoted 10 lines

drivers/usb/gadget/udc/charger.c new file +10 -0

+             }
+     } 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(). :)

-----Original Message-----
From: Baolin Wang [mailto:[email protected]]
Sent: Wednesday, April 06, 2016 7:31 PM
To: Jun Li <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; device- [email protected]; [email protected]
Subject: Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework

Quoted 53 lines · Jun Li

Hi

Quoted 28 lines

+ */

drivers/usb/gadget/udc/charger.c new file +27 -0

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

drivers/usb/gadget/udc/charger.c line 306

Quoted 10 lines

drivers/usb/gadget/udc/charger.c new file +10 -0

+             }
+     } 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.

Quoted 2 lines

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.

Quoted 5 lines

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

--
Baolin.wang
Best Regards

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 <[email protected]>

---
 drivers/usb/gadget/udc/udc-core.c |   11 +++++++++++
 include/linux/usb/gadget.h        |   15 +++++++++++++++
 2 files changed, 26 insertions(+)

drivers/usb/gadget/udc/udc-core.c +11 -0

index b86a6f0..8d98c6b 100644
@@ -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);

include/linux/usb/gadget.h +1 -0

index d82d006..054488a 100644
@@ -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;
 

include/linux/usb/gadget.h +5 -0

 /**

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

include/linux/usb/gadget.h +0 -0

 	if (!gadget->ops->vbus_draw)
 		return -EOPNOTSUPP;
 	return gadget->ops->vbus_draw(gadget, mA);

Signature · 6 lines

1.7.9.5

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

include/linux/usb/gadget.h struct usb_gadget_ops { · line 569

Quoted 8 lines · Baolin Wang

include/linux/usb/gadget.h +2 -0

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

Quoted 20 lines

include/linux/usb/gadget.h +5 -0

 /**

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

include/linux/usb/gadget.h static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget) · line 860

Quoted 10 lines

include/linux/usb/gadget.h +7 -0

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

Quoted 11 lines

include/linux/usb/gadget.h +0 -1

      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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--

Best Regards,
Peter Chen

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Baolin Wang
Sent: Wednesday, April 06, 2016 6:47 PM
To: Peter Chen <[email protected]>
Cc: Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

Quoted 12 lines · Peter Chen

include/linux/usb/gadget.h struct usb_gadget_ops { · line 569

Quoted 8 lines · Baolin Wang

include/linux/usb/gadget.h +2 -0

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

Quoted 32 lines

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Baolin Wang
Sent: Wednesday, April 06, 2016 6:47 PM
To: Peter Chen <[email protected]>
Cc: Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <stern-nwvwT67g6+6dFdvTe/[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <ckeepax-yzvPICuk2AATkU/[email protected]>; patches-yzvPICuk2AATkU/dhu1WVueM+bqZidxxxddQ6fo/[email protected]; Linux PM list <[email protected]>; USB <[email protected]>; device-mainlining-cunTk1MwBs98uUxBSJOaYoYkZiVZrdSR2LY78lusg7I@public.gmane.org; LKML <linux- [email protected]>
Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

Quoted 12 lines · Peter Chen

include/linux/usb/gadget.h struct usb_gadget_ops { · line 569

Quoted 8 lines · Baolin Wang

include/linux/usb/gadget.h +2 -0

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

Quoted 2 lines

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.

-----Original Message-----
From: Felipe Balbi [mailto:[email protected]]
Sent: Wednesday, April 06, 2016 8:22 PM
To: Jun Li <[email protected]>; Baolin Wang <[email protected]>; Peter Chen <[email protected]>
Cc: Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

Hi,

Quoted 36 lines · Jun Li

Quoted 36 lines

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Baolin Wang
Sent: Wednesday, April 06, 2016 6:47 PM
To: Peter Chen <[email protected]>
Cc: Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected];
Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

Quoted 13 lines · Peter Chen

include/linux/usb/gadget.h struct usb_gadget_ops { · line 570

Quoted 9 lines · Baolin Wang

include/linux/usb/gadget.h +3 -0

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

Quoted 4 lines

Quoted 2 lines

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.

Quoted 2 lines

This api, as my understanding, gadget only need report gadget state

changes.

Quoted 1 line

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

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines · Peter Chen

Quoted 13 lines · Baolin Wang

include/linux/usb/gadget.h struct usb_gadget_ops { · line 570

Quoted 9 lines

include/linux/usb/gadget.h +3 -0

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

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

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)
{

include/linux/usb/gadget.h +7 -0

+       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?

Quoted 8 lines

Quoted 1 line

This api, as my understanding, gadget only need report gadget state

changes.

Quoted 1 line

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?

-----Original Message-----
From: Felipe Balbi [mailto:[email protected]]
Sent: Wednesday, April 06, 2016 8:56 PM
To: Jun Li <[email protected]>; Baolin Wang <[email protected]>; Peter Chen <[email protected]>
Cc: Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <stern-nwvwT67g6+6dFdvTe/[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <ckeepax-yzvPICuk2AATkU/[email protected]>; patches-yzvPICuk2AATkU/[email protected]; Linux PM list <[email protected]>; USB <[email protected]>; device-mainlining-cunTk1MwBs98uUxBSJOaYoYkZiVZrdSR2LY78lusg7I@public.gmane.org; LKML <linux- [email protected]>
Subject: RE: [PATCH v9 2/4] gadget: Support for the usb charger framework

Hi,

Quoted 28 lines · Jun Li

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines · Peter Chen

Quoted 13 lines · Baolin Wang

Review struct usb_gadget_ops { · line 570

Quoted 9 lines

(quoted diff) +3 -0

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

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

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) {

Review

Quoted 5 lines

(quoted diff) +5 -0

+       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);

Quoted 6 lines

(quoted diff) +2 -0

+       }
+
        if (!gadget->ops->vbus_draw)
                return -EOPNOTSUPP;
        return gadget->ops->vbus_draw(gadget, mA);

Quoted 1 line

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 ?

Quoted 11 lines

Quoted 9 lines

Quoted 2 lines

This api, as my understanding, gadget only need report gadget state

changes.

Quoted 1 line

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

Quoted 28 lines

Quoted 16 lines

Quoted 10 lines

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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) {

Review

Quoted 5 lines

(quoted diff) +5 -0

+       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);

Quoted 6 lines

(quoted diff) +2 -0

+       }
+
        if (!gadget->ops->vbus_draw)
                return -EOPNOTSUPP;
        return gadget->ops->vbus_draw(gadget, mA);

Quoted 5 lines

Quoted 1 line

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().

Quoted 11 lines

Quoted 10 lines

Quoted 8 lines

Quoted 1 line

This api, as my understanding, gadget only need report gadget state

changes.

Quoted 1 line

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?

Quoted 7 lines

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.

Hi,

Quoted 34 lines · Jun Li

Quoted 33 lines

Quoted 16 lines

Quoted 10 lines

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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) {

Review

Quoted 5 lines

(quoted diff) +5 -0

+       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);

Quoted 7 lines

(quoted diff) +2 -0

+       }
+
        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 ?

Quoted 2 lines

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.

Quoted 23 lines

Quoted 23 lines

Quoted 22 lines

Review

Quoted 10 lines

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) {

(quoted diff) +5 -0

+       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);

Quoted 7 lines

(quoted diff) +2 -0

+       }
+
        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 ?

Quoted 7 lines

Quoted 2 lines

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?

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

Hi Peter,

Quoted 5 lines · Peter Chen

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.

Quoted 4 lines

- 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.

Quoted 3 lines

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.

Quoted 8 lines

- 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.

Quoted 16 lines · Baolin Wang

Hi Peter,

Quoted 5 lines · Peter Chen

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.

Quoted 4 lines

- 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.

Quoted 7 lines

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.

Quoted 7 lines

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.

Quoted 9 lines

Quoted 4 lines

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.

Quoted 15 lines

Quoted 9 lines

- 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.

Quoted 7 lines

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

Quoted 25 lines · Peter Chen

Quoted 23 lines · Baolin Wang

Hi Peter,

Quoted 5 lines · Peter Chen

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.

Quoted 4 lines

- 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();

Quoted 2 lines

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.

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

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.

Quoted 1 line

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.

Quoted 24 lines

Quoted 16 lines

Quoted 10 lines

- 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.

Quoted 7 lines

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.

Quoted 3 lines

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.

Quoted 29 lines · Baolin Wang

Quoted 27 lines · Peter Chen

Quoted 16 lines · Baolin Wang

Hi Peter,

Quoted 5 lines · Peter Chen

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.

Quoted 4 lines

- 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.

Quoted 7 lines

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.

Quoted 4 lines

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.

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

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.

Quoted 4 lines

Quoted 1 line

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.

Quoted 40 lines

Quoted 25 lines

Quoted 17 lines

Quoted 11 lines

- 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.

Quoted 7 lines

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.

Quoted 3 lines

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

Quoted 4 lines

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

Quoted 3 lines · Baolin Wang

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.

Quoted 8 lines · Baolin Wang

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.

Quoted 7 lines

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.

Quoted 8 lines

Quoted 8 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

Quoted 3 lines

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.

Quoted 4 lines

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.