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 v7:
- Add documentations for sysfs attributes.
Other prominent improvements since v5:
- Remove the notifier chain things from the gadget and introduce one callback
function to report to the usb charger when the gadget state is changed.
- Flesh out the port type detection which combines the USB negotiation and
PMICs detection.
- Supply the notification mechanism to userspace when charger state is changed.
- Integrate with the vbus staff in the gadget API.
Baolin Wang (4):
gadget: Introduce the usb charger framework
gadget: Support for the usb charger framework
gadget: Integrate with the usb gadget supporting for usb charger
power: wm831x_power: Support USB charger current limit management
drivers/power/wm831x_power.c | 69 ++++
drivers/usb/gadget/Kconfig | 7 +
drivers/usb/gadget/Makefile | 1 +
drivers/usb/gadget/charger.c | 714 +++++++++++++++++++++++++++++++++++++
drivers/usb/gadget/udc/udc-core.c | 11 +
include/linux/mfd/wm831x/pdata.h | 3 +
include/linux/usb/gadget.h | 11 +
include/linux/usb/usb_charger.h | 164 +++++++++
8 files changed, 980 insertions(+)
create mode 100644 drivers/usb/gadget/charger.c
create mode 100644 include/linux/usb/usb_charger.h
--
1.7.9.5
This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.
The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.
The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state. It also supplies the
notification mechanism to userspace When the usb charger state is changed.
Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.
This patch doesn't yet integrate with the gadget code, so some functions
which rely on the 'gadget' are not completed, that will be implemented
in the following patches.
Signed-off-by: Baolin Wang <[email protected]>
---
drivers/usb/gadget/Kconfig | 7 +
drivers/usb/gadget/Makefile | 1 +
drivers/usb/gadget/charger.c | 675 +++++++++++++++++++++++++++++++++++++++
include/linux/usb/usb_charger.h | 164 ++++++++++
4 files changed, 847 insertions(+)
create mode 100644 drivers/usb/gadget/charger.c
create mode 100644 include/linux/usb/usb_charger.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index af5d922..82a5b3c 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -133,6 +133,13 @@ config U_SERIAL_CONSOLE
help
It supports the serial gadget can be used as a console.
+config USB_CHARGER
+ bool "USB charger support"
+ help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
source "drivers/usb/gadget/udc/Kconfig"
#
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 598a67d..1e421c1 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -10,3 +10,4 @@ libcomposite-y := usbstring.o config.o epautoconf.o
libcomposite-y += composite.o functions.o configfs.o u_f.o
obj-$(CONFIG_USB_GADGET) += udc/ function/ legacy/
+obj-$(CONFIG_USB_CHARGER) += charger.o
diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
new file mode 100644
index 0000000..861c144
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,675 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/usb_charger.h>
+#include <linux/power_supply.h>
+
+#define DEFAULT_CUR_PROTECT (50)
+#define DEFAULT_SDP_CUR_LIMIT (500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_DCP_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_CDP_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_ACA_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define UCHGER_STATE_LENGTH (50)
+
+static DEFINE_IDA(usb_charger_ida);
+static struct bus_type usb_charger_subsys = {
+ .name = "usb-charger",
+ .dev_name = "usb-charger",
+};
+
+static struct usb_charger *dev_to_uchger(struct device *udev)
+{
+ return container_of(udev, struct usb_charger, dev);
+}
+
+/*
+ * Sysfs attributes:
+ *
+ * These sysfs attributes are used for showing and setting different type
+ * (SDP/DCP/CDP/ACA) chargers' current limitation.
+ */
+static ssize_t sdp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
+}
+
+static ssize_t sdp_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ unsigned int sdp_limit;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &sdp_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = usb_charger_set_cur_limit_by_type(uchger, SDP_TYPE, sdp_limit);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_RW(sdp_limit);
+
+static ssize_t dcp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.dcp_cur_limit);
+}
+
+static ssize_t dcp_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ unsigned int dcp_limit;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &dcp_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = usb_charger_set_cur_limit_by_type(uchger, DCP_TYPE, dcp_limit);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_RW(dcp_limit);
+
+static ssize_t cdp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.cdp_cur_limit);
+}
+
+static ssize_t cdp_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ unsigned int cdp_limit;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &cdp_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = usb_charger_set_cur_limit_by_type(uchger, CDP_TYPE, cdp_limit);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_RW(cdp_limit);
+
+static ssize_t aca_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.aca_cur_limit);
+}
+
+static ssize_t aca_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ unsigned int aca_limit;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &aca_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = usb_charger_set_cur_limit_by_type(uchger, ACA_TYPE, aca_limit);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_RW(aca_limit);
+
+static struct attribute *usb_charger_attrs[] = {
+ &dev_attr_sdp_limit.attr,
+ &dev_attr_dcp_limit.attr,
+ &dev_attr_cdp_limit.attr,
+ &dev_attr_aca_limit.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(usb_charger);
+
+/*
+ * usb_charger_find_by_name - Get the usb charger device by name.
+ * @name - usb charger device name.
+ *
+ * return the instance of usb charger device, the device must be
+ * released with usb_charger_put().
+ */
+struct usb_charger *usb_charger_find_by_name(const char *name)
+{
+ struct device *udev;
+
+ if (!name)
+ return ERR_PTR(-EINVAL);
+
+ udev = bus_find_device_by_name(&usb_charger_subsys, NULL, name);
+ if (!udev)
+ return ERR_PTR(-ENODEV);
+
+ return dev_to_uchger(udev);
+}
+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_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 (!uchger || !nb)
+ 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_detect_type(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 (!uchger || !nb)
+ 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_detect_type() - Get the usb charger type by the callback
+ * which is implemented by gadget operations.
+ * @uchger - the usb charger device.
+ *
+ * return the usb charger type.
+ */
+enum usb_charger_type
+usb_charger_detect_type(struct usb_charger *uchger)
+{
+ 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;
+}
+EXPORT_SYMBOL_GPL(usb_charger_detect_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 (!uchger)
+ return -EINVAL;
+
+ 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:
+ return -EINVAL;
+ }
+
+ 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 (!uchger || !cur_limit_set)
+ return -EINVAL;
+
+ 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;
+ 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.
+ */
+unsigned int
+usb_charger_get_cur_limit(struct usb_charger *uchger)
+{
+ enum usb_charger_type uchger_type = usb_charger_detect_type(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;
+}
+EXPORT_SYMBOL_GPL(usb_charger_get_cur_limit);
+
+/*
+ * 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[UCHGER_STATE_LENGTH];
+ char *envp[] = { uchger_state, NULL };
+
+ mutex_lock(&uchger->lock);
+ uchger->state = state;
+
+ switch (state) {
+ case USB_CHARGER_PRESENT:
+ usb_charger_detect_type(uchger);
+ raw_notifier_call_chain(&uchger->uchger_nh,
+ usb_charger_get_cur_limit(uchger),
+ uchger);
+ snprintf(uchger_state, UCHGER_STATE_LENGTH,
+ "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, UCHGER_STATE_LENGTH,
+ "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 (!uchger)
+ 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 (!uchger)
+ 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);
+}
+
+/*
+ * 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 (!uchger)
+ return -EINVAL;
+
+ uchger->dev.parent = parent;
+ uchger->dev.release = usb_charger_release;
+ uchger->dev.bus = &usb_charger_subsys;
+ 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;
+}
+
+int usb_charger_init(struct usb_gadget *ugadget)
+{
+ struct usb_charger *uchger;
+ struct extcon_dev *edev;
+ struct power_supply *psy;
+ int ret;
+
+ if (!ugadget)
+ return -EINVAL;
+
+ uchger = kzalloc(sizeof(struct usb_charger), GFP_KERNEL);
+ if (!uchger)
+ return -ENOMEM;
+
+ uchger->type = UNKNOWN_TYPE;
+ uchger->state = USB_CHARGER_DEFAULT;
+ uchger->id = -1;
+ 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;
+ uchger->get_charger_type = NULL;
+
+ 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_sysfs_init(void)
+{
+ return subsys_system_register(&usb_charger_subsys, NULL);
+}
+core_initcall(usb_charger_sysfs_init);
+
+MODULE_AUTHOR("Baolin Wang <[email protected]>");
+MODULE_DESCRIPTION("USB charger driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/usb_charger.h b/include/linux/usb/usb_charger.h
new file mode 100644
index 0000000..eed422f
--- /dev/null
+++ b/include/linux/usb/usb_charger.h
@@ -0,0 +1,164 @@
+#ifndef __LINUX_USB_CHARGER_H__
+#define __LINUX_USB_CHARGER_H__
+
+#include <uapi/linux/usb/ch9.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,
+};
+
+/* 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 */
+ 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 *);
+
+ /* 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_cur_limit(struct usb_charger *uchger);
+
+extern enum usb_charger_type usb_charger_detect_type(struct usb_charger *uchger);
+extern int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
+ unsigned long state);
+
+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_cur_limit(struct usb_charger *uchger)
+{
+ return 0;
+}
+
+static inline enum usb_charger_type
+usb_charger_detect_type(struct usb_charger *uchger)
+{
+ return UNKNOWN_TYPE;
+}
+
+static inline int
+usb_charger_plug_by_gadget(struct usb_gadget *gadget, unsigned long state)
+{
+ 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__ */
--
1.7.9.5
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 | 11 +++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b86a6f0..3d9a489 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
#include <linux/usb.h>
+#include <linux/usb/usb_charger.h>
/**
* struct usb_udc - describes one usb device controller
@@ -230,6 +231,9 @@ static void usb_gadget_state_work(struct work_struct *work)
struct usb_gadget *gadget = work_to_gadget(work);
struct usb_udc *udc = gadget->udc;
+ /* when the gadget state is changed, then report to USB charger */
+ usb_charger_plug_by_gadget(gadget, gadget->state);
+
if (udc)
sysfs_notify(&udc->dev.kobj, NULL, "state");
}
@@ -423,8 +427,14 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
mutex_unlock(&udc_lock);
+ ret = usb_charger_init(gadget);
+ if (ret)
+ goto err5;
+
return 0;
+err5:
+ device_del(&udc->dev);
err4:
list_del(&udc->list);
mutex_unlock(&udc_lock);
@@ -503,6 +513,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
flush_work(&gadget->work);
device_unregister(&udc->dev);
+ usb_charger_exit(gadget);
device_unregister(&gadget->dev);
}
EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..024b33d 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -24,6 +24,7 @@
#include <linux/types.h>
#include <linux/workqueue.h>
#include <linux/usb/ch9.h>
+#include <linux/usb/usb_charger.h>
struct usb_ep;
@@ -563,6 +564,7 @@ struct usb_gadget_ops {
struct usb_ep *(*match_ep)(struct usb_gadget *,
struct usb_endpoint_descriptor *,
struct usb_ss_ep_comp_descriptor *);
+ enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
};
/**
@@ -635,6 +637,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 +843,17 @@ static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
* reporting how much power the device may consume. For example, this
* could affect how quickly batteries are recharged.
*
+ * It will also notify the USB charger how much power the device may
+ * consume if there is a USB charger linking with the gadget.
+ *
* Returns zero on success, else negative errno.
*/
static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
{
+ if (gadget->charger)
+ usb_charger_set_cur_limit_by_type(gadget->charger,
+ usb_charger_detect_type(gadget->charger), mA);
+
if (!gadget->ops->vbus_draw)
return -EOPNOTSUPP;
return gadget->ops->vbus_draw(gadget, mA);
--
1.7.9.5
When the usb gadget supporting for usb charger is ready, the usb charger
should get the type by the 'get_charger_type' callback which is implemented
by the usb gadget operations, and get the usb charger pointer from struct
'usb_gadget'.
Signed-off-by: Baolin Wang <[email protected]>
---
drivers/usb/gadget/charger.c | 43 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
index 861c144..b02c842 100644
--- a/drivers/usb/gadget/charger.c
+++ b/drivers/usb/gadget/charger.c
@@ -278,7 +278,11 @@ EXPORT_SYMBOL_GPL(usb_charger_unregister_notify);
enum usb_charger_type
usb_charger_detect_type(struct usb_charger *uchger)
{
- if (uchger->psy) {
+ if (uchger->gadget && uchger->gadget->ops
+ && uchger->gadget->ops->get_charger_type) {
+ uchger->type =
+ uchger->gadget->ops->get_charger_type(uchger->gadget);
+ } else if (uchger->psy) {
union power_supply_propval val;
power_supply_get_property(uchger->psy,
@@ -485,6 +489,29 @@ usb_charger_plug_by_extcon(struct notifier_block *nb,
int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
unsigned long state)
{
+ struct usb_charger *uchger = gadget->charger;
+ enum usb_charger_state uchger_state;
+
+ if (!uchger)
+ return -EINVAL;
+
+ /* Report event to power to setting the current limitation
+ * for this usb charger when one usb charger state is changed
+ * with detecting by usb gadget state.
+ */
+ if (uchger->old_gadget_state != state) {
+ uchger->old_gadget_state = state;
+
+ if (state >= USB_STATE_ATTACHED)
+ uchger_state = USB_CHARGER_PRESENT;
+ else if (state == USB_STATE_NOTATTACHED)
+ uchger_state = USB_CHARGER_REMOVE;
+ else
+ uchger_state = USB_CHARGER_DEFAULT;
+
+ usb_charger_notify_others(uchger, uchger_state);
+ }
+
return 0;
}
EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
@@ -641,6 +668,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
/* register a notifier on a usb gadget device */
uchger->gadget = ugadget;
+ ugadget->charger = uchger;
uchger->old_gadget_state = ugadget->state;
/* register a new usb charger */
@@ -661,7 +689,18 @@ fail:
int usb_charger_exit(struct usb_gadget *ugadget)
{
- return 0;
+ struct usb_charger *uchger = ugadget->charger;
+
+ if (!uchger)
+ return -EINVAL;
+
+ if (uchger->extcon_dev)
+ extcon_unregister_notifier(uchger->extcon_dev,
+ EXTCON_USB, &uchger->extcon_nb.nb);
+
+ ida_simple_remove(&usb_charger_ida, uchger->id);
+
+ return usb_charger_unregister(uchger);
}
static int __init usb_charger_sysfs_init(void)
--
1.7.9.5
Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.
Signed-off-by: Mark Brown <[email protected]>
Signed-off-by: Baolin Wang <[email protected]>
Acked-by: Lee Jones <[email protected]>
Acked-by: Charles Keepax <[email protected]>
Acked-by: Peter Chen <[email protected]>
Acked-by: Sebastian Reichel <[email protected]>
---
drivers/power/wm831x_power.c | 69 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/wm831x/pdata.h | 3 ++
2 files changed, 72 insertions(+)
diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..043f1f4 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/slab.h>
+#include <linux/usb/usb_charger.h>
#include <linux/mfd/wm831x/core.h>
#include <linux/mfd/wm831x/auxadc.h>
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+ struct usb_charger *usb_charger;
+ struct notifier_block usb_notify;
};
static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
};
+/* In milliamps */
+static unsigned int wm831x_usb_limits[] = {
+ 0,
+ 2,
+ 100,
+ 500,
+ 900,
+ 1500,
+ 1800,
+ 550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+ unsigned long limit, void *data)
+{
+ struct wm831x_power *wm831x_power = container_of(nb,
+ struct wm831x_power,
+ usb_notify);
+ int i, best;
+
+ /* Find the highest supported limit */
+ best = 0;
+ for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+ if (limit >= wm831x_usb_limits[i] &&
+ wm831x_usb_limits[best] < wm831x_usb_limits[i])
+ best = i;
+ }
+
+ dev_dbg(wm831x_power->wm831x->dev,
+ "Limiting USB current to %dmA", wm831x_usb_limits[best]);
+
+ wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+ WM831X_USB_ILIM_MASK, best);
+
+ return 0;
+}
+
/*********************************************************************
* Battery properties
*********************************************************************/
@@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
+ if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+ power->usb_charger =
+ usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+ if (IS_ERR(power->usb_charger)) {
+ ret = PTR_ERR(power->usb_charger);
+ dev_err(&pdev->dev,
+ "Failed to find USB gadget: %d\n", ret);
+ goto err_bat_irq;
+ }
+
+ power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+ ret = usb_charger_register_notify(power->usb_charger,
+ &power->usb_notify);
+ if (ret != 0) {
+ dev_err(&pdev->dev,
+ "Failed to register notifier: %d\n", ret);
+ goto err_usb_charger;
+ }
+ }
+
return ret;
+err_usb_charger:
+ /* put_device on charger */
err_bat_irq:
--i;
for (; i >= 0; i--) {
@@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device *pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
+ if (wm831x_power->usb_charger) {
+ usb_charger_unregister_notify(wm831x_power->usb_charger,
+ &wm831x_power->usb_notify);
+ /* Free charger */
+ }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x,
platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
/** The driver should initiate a power off sequence during shutdown */
bool soft_shutdown;
+ /** dev_name of USB charger gadget to integrate with */
+ const char *usb_gadget;
+
int irq_base;
int gpio_base;
int gpio_defaults[WM831X_GPIO_NUM];
--
1.7.9.5
On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> Currently the Linux kernel does not provide any standard integration of this
> feature that integrates the USB subsystem with the system power regulation
> provided by PMICs meaning that either vendors must add this in their kernels
> or USB gadget devices based on Linux (such as mobile phones) may not behave
> as they should. Thus provide a standard framework for doing this in kernel.
>
> Now introduce one user with wm831x_power to support and test the usb charger,
> which is pending testing. Moreover there may be other potential users will use
> it in future.
>
I am afraid I still not find the user (udc driver) for this framework, I would
like to see how udc driver block the enumeration until the charger detection
has finished, or am I missing something?
--
Best Regards,
Peter Chen
Hi Baolin,
[auto build test ERROR on v4.5-rc7]
[also build test ERROR on next-20160324]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-usb-charger-framework-to-deal-with-the-usb-gadget-power-negotation/20160324-204018
config: m68k-allmodconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
>> ERROR: "usb_charger_init" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>> ERROR: "usb_charger_plug_by_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>> ERROR: "usb_charger_exit" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>> ERROR: "usb_charger_set_cur_limit_by_type" [drivers/usb/gadget/libcomposite.ko] undefined!
>> ERROR: "usb_charger_detect_type" [drivers/usb/gadget/libcomposite.ko] undefined!
>> ERROR: "usb_charger_set_cur_limit_by_type" [drivers/usb/gadget/legacy/gadgetfs.ko] undefined!
>> ERROR: "usb_charger_detect_type" [drivers/usb/gadget/legacy/gadgetfs.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Baolin,
[auto build test ERROR on v4.5-rc7]
[also build test ERROR on next-20160324]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-usb-charger-framework-to-deal-with-the-usb-gadget-power-negotation/20160324-204018
config: m68k-allmodconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
ERROR: "usb_charger_init" [drivers/usb/gadget/udc/udc-core.ko] undefined!
ERROR: "usb_charger_plug_by_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
ERROR: "usb_charger_exit" [drivers/usb/gadget/udc/udc-core.ko] undefined!
ERROR: "usb_charger_set_cur_limit_by_type" [drivers/usb/gadget/libcomposite.ko] undefined!
ERROR: "usb_charger_detect_type" [drivers/usb/gadget/libcomposite.ko] undefined!
ERROR: "usb_charger_set_cur_limit_by_type" [drivers/usb/gadget/legacy/gadgetfs.ko] undefined!
ERROR: "usb_charger_detect_type" [drivers/usb/gadget/legacy/gadgetfs.ko] undefined!
>> ERROR: "usb_charger_register_notify" [drivers/power/wm831x_power.ko] undefined!
>> ERROR: "usb_charger_find_by_name" [drivers/power/wm831x_power.ko] undefined!
>> ERROR: "usb_charger_unregister_notify" [drivers/power/wm831x_power.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Mar 24, 2016 at 1:35 PM, Baolin Wang <[email protected]> wrote:
> --- a/drivers/power/wm831x_power.c
> +++ b/drivers/power/wm831x_power.c
> @@ -13,6 +13,7 @@
> #include <linux/platform_device.h>
> #include <linux/power_supply.h>
> #include <linux/slab.h>
> +#include <linux/usb/usb_charger.h>
>
> #include <linux/mfd/wm831x/core.h>
> #include <linux/mfd/wm831x/auxadc.h>
> @@ -31,6 +32,8 @@ struct wm831x_power {
> char usb_name[20];
> char battery_name[20];
> bool have_battery;
> + struct usb_charger *usb_charger;
> + struct notifier_block usb_notify;
> };
>
> static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> @@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> };
>
> +/* In milliamps */
> +static unsigned int wm831x_usb_limits[] = {
const
> + 0,
> + 2,
> + 100,
> + 500,
> + 900,
> + 1500,
> + 1800,
> + 550,
> +};
> +
> +static int wm831x_usb_limit_change(struct notifier_block *nb,
> + unsigned long limit, void *data)
> +{
> + struct wm831x_power *wm831x_power = container_of(nb,
> + struct wm831x_power,
> + usb_notify);
> + int i, best;
unsigned int
> +
> + /* Find the highest supported limit */
> + best = 0;
> + for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
> + if (limit >= wm831x_usb_limits[i] &&
> + wm831x_usb_limits[best] < wm831x_usb_limits[i])
> + best = i;
> + }
> +
> + dev_dbg(wm831x_power->wm831x->dev,
> + "Limiting USB current to %dmA", wm831x_usb_limits[best]);
%u
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 27 March 2016 at 16:22, Geert Uytterhoeven <[email protected]> wrote:
> On Thu, Mar 24, 2016 at 1:35 PM, Baolin Wang <[email protected]> wrote:
>> --- a/drivers/power/wm831x_power.c
>> +++ b/drivers/power/wm831x_power.c
>> @@ -13,6 +13,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/power_supply.h>
>> #include <linux/slab.h>
>> +#include <linux/usb/usb_charger.h>
>>
>> #include <linux/mfd/wm831x/core.h>
>> #include <linux/mfd/wm831x/auxadc.h>
>> @@ -31,6 +32,8 @@ struct wm831x_power {
>> char usb_name[20];
>> char battery_name[20];
>> bool have_battery;
>> + struct usb_charger *usb_charger;
>> + struct notifier_block usb_notify;
>> };
>>
>> static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
>> @@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> };
>>
>> +/* In milliamps */
>> +static unsigned int wm831x_usb_limits[] = {
>
> const
OK.
>
>> + 0,
>> + 2,
>> + 100,
>> + 500,
>> + 900,
>> + 1500,
>> + 1800,
>> + 550,
>> +};
>> +
>> +static int wm831x_usb_limit_change(struct notifier_block *nb,
>> + unsigned long limit, void *data)
>> +{
>> + struct wm831x_power *wm831x_power = container_of(nb,
>> + struct wm831x_power,
>> + usb_notify);
>> + int i, best;
>
> unsigned int
OK.
>
>> +
>> + /* Find the highest supported limit */
>> + best = 0;
>> + for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
>> + if (limit >= wm831x_usb_limits[i] &&
>> + wm831x_usb_limits[best] < wm831x_usb_limits[i])
>> + best = i;
>> + }
>> +
>> + dev_dbg(wm831x_power->wm831x->dev,
>> + "Limiting USB current to %dmA", wm831x_usb_limits[best]);
>
> %u
OK. Thanks for your comments.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Baolin.wang
Best Regards
On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
> On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> Currently the Linux kernel does not provide any standard integration of this
>> feature that integrates the USB subsystem with the system power regulation
>> provided by PMICs meaning that either vendors must add this in their kernels
>> or USB gadget devices based on Linux (such as mobile phones) may not behave
>> as they should. Thus provide a standard framework for doing this in kernel.
>>
>> Now introduce one user with wm831x_power to support and test the usb charger,
>> which is pending testing. Moreover there may be other potential users will use
>> it in future.
>>
>
> I am afraid I still not find the user (udc driver) for this framework, I would
> like to see how udc driver block the enumeration until the charger detection
> has finished, or am I missing something?
It is not for udc driver but for power users who want to negotiate
with USB subsystem.
>
> --
> Best Regards,
> Peter Chen
--
Baolin.wang
Best Regards
On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> Currently the Linux kernel does not provide any standard integration of this
> >> feature that integrates the USB subsystem with the system power regulation
> >> provided by PMICs meaning that either vendors must add this in their kernels
> >> or USB gadget devices based on Linux (such as mobile phones) may not behave
> >> as they should. Thus provide a standard framework for doing this in kernel.
> >>
> >> Now introduce one user with wm831x_power to support and test the usb charger,
> >> which is pending testing. Moreover there may be other potential users will use
> >> it in future.
> >>
> >
> > I am afraid I still not find the user (udc driver) for this framework, I would
> > like to see how udc driver block the enumeration until the charger detection
> > has finished, or am I missing something?
>
> It is not for udc driver but for power users who want to negotiate
> with USB subsystem.
>
Then, where is the code the test user to decide what kinds of USB charger
(SDP, CDP, DCP) is connecting now?
--
Best Regards,
Peter Chen
On 28 March 2016 at 15:13, Peter Chen <[email protected]> wrote:
> On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
>> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> Currently the Linux kernel does not provide any standard integration of this
>> >> feature that integrates the USB subsystem with the system power regulation
>> >> provided by PMICs meaning that either vendors must add this in their kernels
>> >> or USB gadget devices based on Linux (such as mobile phones) may not behave
>> >> as they should. Thus provide a standard framework for doing this in kernel.
>> >>
>> >> Now introduce one user with wm831x_power to support and test the usb charger,
>> >> which is pending testing. Moreover there may be other potential users will use
>> >> it in future.
>> >>
>> >
>> > I am afraid I still not find the user (udc driver) for this framework, I would
>> > like to see how udc driver block the enumeration until the charger detection
>> > has finished, or am I missing something?
>>
>> It is not for udc driver but for power users who want to negotiate
>> with USB subsystem.
>>
>
> Then, where is the code the test user to decide what kinds of USB charger
> (SDP, CDP, DCP) is connecting now?
Users can choose to implement multiple ways to get the charger type In
'usb_charger_detect_type()' function.
>
> --
> Best Regards,
> Peter Chen
--
Baolin.wang
Best Regards
>
> On 28 March 2016 at 15:13, Peter Chen <[email protected]> wrote:
> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> >> Currently the Linux kernel does not provide any standard
> >> >> integration of this feature that integrates the USB subsystem with
> >> >> the system power regulation provided by PMICs meaning that either
> >> >> vendors must add this in their kernels or USB gadget devices based
> >> >> on Linux (such as mobile phones) may not behave as they should. Thus
> provide a standard framework for doing this in kernel.
> >> >>
> >> >> Now introduce one user with wm831x_power to support and test the
> >> >> usb charger, which is pending testing. Moreover there may be other
> >> >> potential users will use it in future.
> >> >>
> >> >
> >> > I am afraid I still not find the user (udc driver) for this
> >> > framework, I would like to see how udc driver block the enumeration
> >> > until the charger detection has finished, or am I missing something?
> >>
> >> It is not for udc driver but for power users who want to negotiate
> >> with USB subsystem.
> >>
> >
> > Then, where is the code the test user to decide what kinds of USB
> > charger (SDP, CDP, DCP) is connecting now?
>
> Users can choose to implement multiple ways to get the charger type In
> 'usb_charger_detect_type()' function.
>
> >
Then, how you test your code? When we introduce an API, at least, there is one
user for it.
Peter
On 29 March 2016 at 08:32, Peter Chen <[email protected]> wrote:
>
>>
>> On 28 March 2016 at 15:13, Peter Chen <[email protected]> wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
>> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> >> Currently the Linux kernel does not provide any standard
>> >> >> integration of this feature that integrates the USB subsystem with
>> >> >> the system power regulation provided by PMICs meaning that either
>> >> >> vendors must add this in their kernels or USB gadget devices based
>> >> >> on Linux (such as mobile phones) may not behave as they should. Thus
>> provide a standard framework for doing this in kernel.
>> >> >>
>> >> >> Now introduce one user with wm831x_power to support and test the
>> >> >> usb charger, which is pending testing. Moreover there may be other
>> >> >> potential users will use it in future.
>> >> >>
>> >> >
>> >> > I am afraid I still not find the user (udc driver) for this
>> >> > framework, I would like to see how udc driver block the enumeration
>> >> > until the charger detection has finished, or am I missing something?
>> >>
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Then, where is the code the test user to decide what kinds of USB
>> > charger (SDP, CDP, DCP) is connecting now?
>>
>> Users can choose to implement multiple ways to get the charger type In
>> 'usb_charger_detect_type()' function.
>>
>> >
>
> Then, how you test your code? When we introduce an API, at least, there is one
> user for it.
At first I've tested the API on my board to make sure it can work
well, another hand we introduce one user 'wm831x_power' to support and
test the usb charger.
Yes, The user 'wm831x_power' did not implement any callbacks in
'usb_charger_detect_type()' function, but in
'usb_charger_detect_type()' function it just supplies different
callbacks to get the charger type with simple logic. Anyway we can try
to implement one callback to get the charger type and test the API.
Thanks.
>
> Peter
--
Baolin.wang
Best Regards
> -----Original Message-----
> From: [email protected] [mailto:linux-usb-
> [email protected]] On Behalf Of Baolin Wang
> Sent: Monday, March 28, 2016 2:52 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 v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
>
> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> Currently the Linux kernel does not provide any standard integration
> >> of this feature that integrates the USB subsystem with the system
> >> power regulation provided by PMICs meaning that either vendors must
> >> add this in their kernels or USB gadget devices based on Linux (such
> >> as mobile phones) may not behave as they should. Thus provide a
> standard framework for doing this in kernel.
> >>
> >> Now introduce one user with wm831x_power to support and test the usb
> >> charger, which is pending testing. Moreover there may be other
> >> potential users will use it in future.
> >>
> >
> > I am afraid I still not find the user (udc driver) for this framework,
> > I would like to see how udc driver block the enumeration until the
> > charger detection has finished, or am I missing something?
>
> It is not for udc driver but for power users who want to negotiate with
> USB subsystem.
>
Seems you don't want to guarantee charger type detection is done before
gadget connection(pullup DP), right?
I see you call usb_charger_detect_type() in each gadget usb state changes.
Li Jun
> >
> > --
> > Best Regards,
> > Peter Chen
>
>
>
> --
> Baolin.wang
> Best Regards
> --
> 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 29 March 2016 at 16:45, Jun Li <[email protected]> wrote:
>
>
>> -----Original Message-----
>> From: [email protected] [mailto:linux-usb-
>> [email protected]] On Behalf Of Baolin Wang
>> Sent: Monday, March 28, 2016 2:52 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 v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
>> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> Currently the Linux kernel does not provide any standard integration
>> >> of this feature that integrates the USB subsystem with the system
>> >> power regulation provided by PMICs meaning that either vendors must
>> >> add this in their kernels or USB gadget devices based on Linux (such
>> >> as mobile phones) may not behave as they should. Thus provide a
>> standard framework for doing this in kernel.
>> >>
>> >> Now introduce one user with wm831x_power to support and test the usb
>> >> charger, which is pending testing. Moreover there may be other
>> >> potential users will use it in future.
>> >>
>> >
>> > I am afraid I still not find the user (udc driver) for this framework,
>> > I would like to see how udc driver block the enumeration until the
>> > charger detection has finished, or am I missing something?
>>
>> It is not for udc driver but for power users who want to negotiate with
>> USB subsystem.
>>
>
> Seems you don't want to guarantee charger type detection is done before
> gadget connection(pullup DP), right?
> I see you call usb_charger_detect_type() in each gadget usb state changes.
I am not sure I get your point correctly, please correct me if I
misunderstand you.
We need to check the charger type at every event comes from the usb
gadget state changes or the extcon device state changes, which means a
new charger plugin or pullup.
>
> Li Jun
>> >
>> > --
>> > Best Regards,
>> > Peter Chen
>>
>>
>>
>> --
>> Baolin.wang
>> Best Regards
>> --
>> 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
--
Baolin.wang
Best Regards
On Tue, Mar 29, 2016 at 10:05:23AM +0800, Baolin Wang wrote:
> Yes, The user 'wm831x_power' did not implement any callbacks in
> 'usb_charger_detect_type()' function, but in
> 'usb_charger_detect_type()' function it just supplies different
> callbacks to get the charger type with simple logic. Anyway we can try
> to implement one callback to get the charger type and test the API.
The wm831x doesn't have any charger detection features, what it does
have is hardware for implementing a current limit on the power drawn
from USB. What it's looking for from this framework is for something to
tell it how much current it should draw from USB so it can program the
hardware appropriately to impose that limit.
On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> > > I am afraid I still not find the user (udc driver) for this framework, I would
> > > like to see how udc driver block the enumeration until the charger detection
> > > has finished, or am I missing something?
> > It is not for udc driver but for power users who want to negotiate
> > with USB subsystem.
> Then, where is the code the test user to decide what kinds of USB charger
> (SDP, CDP, DCP) is connecting now?
Even without detection of CDP and DCP we have configurability within SDP
- there's the 2.5mA suspended limit, the 100mA default limit and the
higher 500mA limit which can be negotiated.
On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>
> > > > I am afraid I still not find the user (udc driver) for this framework, I would
> > > > like to see how udc driver block the enumeration until the charger detection
> > > > has finished, or am I missing something?
>
> > > It is not for udc driver but for power users who want to negotiate
> > > with USB subsystem.
>
> > Then, where is the code the test user to decide what kinds of USB charger
> > (SDP, CDP, DCP) is connecting now?
>
> Even without detection of CDP and DCP we have configurability within SDP
> - there's the 2.5mA suspended limit, the 100mA default limit and the
> higher 500mA limit which can be negotiated.
Well, things may be a little complicated.
- First, how to design get_charger_type for each udc driver?
Since the charger detection process affects dp/dm signal, it can't be done
during the enumeration or after that. So, the detection process can be only
done after vbus has detected and before udc pull up dp.
Then, when the get_charger_type do real charger detection? My suggestion is,
if the charger type is unknown, we do real one, else, we just return the
stored type value.
- Second, When to notify charger IC to charger:
For SDP and CDP, it needs to notify charger IC after set configuration
has finished.
For DCP (and ACA, I am not sure), can notify charger IC after charger
detection has finished or later.
So, when we get charger is present, we need to notify charger IC at once
for DCP (and ACA); But for SDP and CDP, we need to let the gadget
composite core to notify charger IC after set configuration has finished,
like the patch 2/4 does.
- Third, since composite driver covers 500mA (and more for CDP) after set
configuration and 2mA after suspend, and vbus handler covers connect
and disconnect. I can't see any reasons we need to notify gadget state
for power driver, do we really need to have usb_charger_plug_by_gadget?
--
Best Regards,
eter Chen
> -----Original Message-----
> From: Baolin Wang [mailto:[email protected]]
> Sent: Tuesday, March 29, 2016 5:49 PM
> To: Jun Li <[email protected]>
> Cc: Peter Chen <[email protected]>; 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 v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
>
> On 29 March 2016 at 16:45, Jun Li <[email protected]> wrote:
> >
> >
> >> -----Original Message-----
> >> From: [email protected] [mailto:linux-usb-
> >> [email protected]] On Behalf Of Baolin Wang
> >> Sent: Monday, March 28, 2016 2:52 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 v8 0/4] Introduce usb charger framework to deal
> >> with the usb gadget power negotation
> >>
> >> On 25 March 2016 at 15:09, Peter Chen <[email protected]> wrote:
> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> >> Currently the Linux kernel does not provide any standard
> >> >> integration of this feature that integrates the USB subsystem with
> >> >> the system power regulation provided by PMICs meaning that either
> >> >> vendors must add this in their kernels or USB gadget devices based
> >> >> on Linux (such as mobile phones) may not behave as they should.
> >> >> Thus provide a
> >> standard framework for doing this in kernel.
> >> >>
> >> >> Now introduce one user with wm831x_power to support and test the
> >> >> usb charger, which is pending testing. Moreover there may be other
> >> >> potential users will use it in future.
> >> >>
> >> >
> >> > I am afraid I still not find the user (udc driver) for this
> >> > framework, I would like to see how udc driver block the enumeration
> >> > until the charger detection has finished, or am I missing something?
> >>
> >> It is not for udc driver but for power users who want to negotiate
> >> with USB subsystem.
> >>
> >
> > Seems you don't want to guarantee charger type detection is done
> > before gadget connection(pullup DP), right?
> > I see you call usb_charger_detect_type() in each gadget usb state
> changes.
>
> I am not sure I get your point correctly, please correct me if I
> misunderstand you.
> We need to check the charger type at every event comes from the usb gadget
> state changes or the extcon device state changes, which means a new
> charger plugin or pullup.
>
According to usb charger spec, my understanding is you can't do real charger
detection procedure *after* gadget _connection_(pullup DP), also I don't
think it's necessary to check charger type at every event from usb gadget.
Something in gadget driver you can utilize is only vbus detection, and
report diff current by diff usb state if it's a SDP.
> >
> > Li Jun
> >> >
> >> > --
> >> > Best Regards,
> >> > Peter Chen
> >>
> >>
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
> >> --
> >> 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
>
>
>
> --
> Baolin.wang
> Best Regards
On 30 March 2016 at 10:54, Jun Li <[email protected]> wrote:
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Seems you don't want to guarantee charger type detection is done
>> > before gadget connection(pullup DP), right?
>> > I see you call usb_charger_detect_type() in each gadget usb state
>> changes.
>>
>> I am not sure I get your point correctly, please correct me if I
>> misunderstand you.
>> We need to check the charger type at every event comes from the usb gadget
>> state changes or the extcon device state changes, which means a new
>> charger plugin or pullup.
>>
>
> According to usb charger spec, my understanding is you can't do real charger
> detection procedure *after* gadget _connection_(pullup DP), also I don't
Why can not? Charger detection is usually from PMIC.
> think it's necessary to check charger type at every event from usb gadget.
My meaning is not every event from usb gadget. When the usb gadget
state changes or the extcon device (maybe GPIO detection) state
changes, which means charger plugin or pullup, we need to check the
charger type to set current.
> Something in gadget driver you can utilize is only vbus detection, and
> report diff current by diff usb state if it's a SDP.
--
Baolin.wang
Best Regards
On 30 March 2016 at 10:05, Peter Chen <[email protected]> wrote:
> On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>>
>> > > > I am afraid I still not find the user (udc driver) for this framework, I would
>> > > > like to see how udc driver block the enumeration until the charger detection
>> > > > has finished, or am I missing something?
>>
>> > > It is not for udc driver but for power users who want to negotiate
>> > > with USB subsystem.
>>
>> > Then, where is the code the test user to decide what kinds of USB charger
>> > (SDP, CDP, DCP) is connecting now?
>>
>> Even without detection of CDP and DCP we have configurability within SDP
>> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> higher 500mA limit which can be negotiated.
>
> Well, things may be a little complicated.
I try to address your comments, maybe Mark need to do some complements. Thanks.
>
> - First, how to design get_charger_type for each udc driver?
> Since the charger detection process affects dp/dm signal, it can't be done
> during the enumeration or after that. So, the detection process can be only
> done after vbus has detected and before udc pull up dp.
>
> Then, when the get_charger_type do real charger detection? My suggestion is,
> if the charger type is unknown, we do real one, else, we just return the
> stored type value.
I think that is why we let udc driver to implement the
'gadget->ops->get_charger_type()' callback, which can be controlled by
udc driver.
>
> - Second, When to notify charger IC to charger:
> For SDP and CDP, it needs to notify charger IC after set configuration
> has finished.
> For DCP (and ACA, I am not sure), can notify charger IC after charger
> detection has finished or later.
>
> So, when we get charger is present, we need to notify charger IC at once
> for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> composite core to notify charger IC after set configuration has finished,
> like the patch 2/4 does.
But mostly charger IC is separate with gadget, so the charger IC
doesn't need to worry about the gadget status.
>
> - Third, since composite driver covers 500mA (and more for CDP) after set
> configuration and 2mA after suspend, and vbus handler covers connect
> and disconnect. I can't see any reasons we need to notify gadget state
> for power driver, do we really need to have usb_charger_plug_by_gadget?
In some solutions, gadget does not negotiate with the current. They
just send out one signal to power driver to set the current when the
gadget state is changed (plugin or not). So we need to check the
charger state by the gadget state to notify the charger IC to set
current.
>
> --
> Best Regards,
> eter Chen
--
Baolin.wang
Best Regards
On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
> On 30 March 2016 at 10:05, Peter Chen <[email protected]> wrote:
> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >>
> >> > > > I am afraid I still not find the user (udc driver) for this framework, I would
> >> > > > like to see how udc driver block the enumeration until the charger detection
> >> > > > has finished, or am I missing something?
> >>
> >> > > It is not for udc driver but for power users who want to negotiate
> >> > > with USB subsystem.
> >>
> >> > Then, where is the code the test user to decide what kinds of USB charger
> >> > (SDP, CDP, DCP) is connecting now?
> >>
> >> Even without detection of CDP and DCP we have configurability within SDP
> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
> >> higher 500mA limit which can be negotiated.
> >
> > Well, things may be a little complicated.
>
> I try to address your comments, maybe Mark need to do some complements. Thanks.
>
> >
> > - First, how to design get_charger_type for each udc driver?
> > Since the charger detection process affects dp/dm signal, it can't be done
> > during the enumeration or after that. So, the detection process can be only
> > done after vbus has detected and before udc pull up dp.
> >
> > Then, when the get_charger_type do real charger detection? My suggestion is,
> > if the charger type is unknown, we do real one, else, we just return the
> > stored type value.
>
> I think that is why we let udc driver to implement the
> 'gadget->ops->get_charger_type()' callback, which can be controlled by
> udc driver.
>
> >
> > - Second, When to notify charger IC to charger:
> > For SDP and CDP, it needs to notify charger IC after set configuration
> > has finished.
> > For DCP (and ACA, I am not sure), can notify charger IC after charger
> > detection has finished or later.
> >
> > So, when we get charger is present, we need to notify charger IC at once
> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> > composite core to notify charger IC after set configuration has finished,
> > like the patch 2/4 does.
>
> But mostly charger IC is separate with gadget, so the charger IC
> doesn't need to worry about the gadget status.
>
The reason why we need to combine charger IC with USB gadget (charger)
driver is the charger IC should not charge as much as it wants, it
needs to consider USB charger style and USB connection
(un-configuration vs configuration) situation.
> >
> > - Third, since composite driver covers 500mA (and more for CDP) after set
> > configuration and 2mA after suspend, and vbus handler covers connect
> > and disconnect. I can't see any reasons we need to notify gadget state
> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>
> In some solutions, gadget does not negotiate with the current. They
> just send out one signal to power driver to set the current when the
> gadget state is changed (plugin or not). So we need to check the
> charger state by the gadget state to notify the charger IC to set
> current.
>
Would you give some examples?
--
Best Regards,
Peter Chen
On 30 March 2016 at 15:42, Peter Chen <[email protected]> wrote:
> On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
>> On 30 March 2016 at 10:05, Peter Chen <[email protected]> wrote:
>> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >>
>> >> > > > I am afraid I still not find the user (udc driver) for this framework, I would
>> >> > > > like to see how udc driver block the enumeration until the charger detection
>> >> > > > has finished, or am I missing something?
>> >>
>> >> > > It is not for udc driver but for power users who want to negotiate
>> >> > > with USB subsystem.
>> >>
>> >> > Then, where is the code the test user to decide what kinds of USB charger
>> >> > (SDP, CDP, DCP) is connecting now?
>> >>
>> >> Even without detection of CDP and DCP we have configurability within SDP
>> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> >> higher 500mA limit which can be negotiated.
>> >
>> > Well, things may be a little complicated.
>>
>> I try to address your comments, maybe Mark need to do some complements. Thanks.
>>
>> >
>> > - First, how to design get_charger_type for each udc driver?
>> > Since the charger detection process affects dp/dm signal, it can't be done
>> > during the enumeration or after that. So, the detection process can be only
>> > done after vbus has detected and before udc pull up dp.
>> >
>> > Then, when the get_charger_type do real charger detection? My suggestion is,
>> > if the charger type is unknown, we do real one, else, we just return the
>> > stored type value.
>>
>> I think that is why we let udc driver to implement the
>> 'gadget->ops->get_charger_type()' callback, which can be controlled by
>> udc driver.
>>
>> >
>> > - Second, When to notify charger IC to charger:
>> > For SDP and CDP, it needs to notify charger IC after set configuration
>> > has finished.
>> > For DCP (and ACA, I am not sure), can notify charger IC after charger
>> > detection has finished or later.
>> >
>> > So, when we get charger is present, we need to notify charger IC at once
>> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
>> > composite core to notify charger IC after set configuration has finished,
>> > like the patch 2/4 does.
>>
>> But mostly charger IC is separate with gadget, so the charger IC
>> doesn't need to worry about the gadget status.
>>
>
> The reason why we need to combine charger IC with USB gadget (charger)
> driver is the charger IC should not charge as much as it wants, it
> needs to consider USB charger style and USB connection
> (un-configuration vs configuration) situation.
Yes, I agree with the charger IC should not charge as much as it
wants, but we don't want to combine two separated things. Like you
said, udc driver can implement 'get_charger_type()' callback to check
the charger type at the right time, then set the current after
checking the charger type (now the gadget has been set configuration).
>
>> >
>> > - Third, since composite driver covers 500mA (and more for CDP) after set
>> > configuration and 2mA after suspend, and vbus handler covers connect
>> > and disconnect. I can't see any reasons we need to notify gadget state
>> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>>
>> In some solutions, gadget does not negotiate with the current. They
>> just send out one signal to power driver to set the current when the
>> gadget state is changed (plugin or not). So we need to check the
>> charger state by the gadget state to notify the charger IC to set
>> current.
>>
>
> Would you give some examples?
OK. I explain it in detail. Now charger detection can be from gadget
itself or PMIC, and we focus on gadget detection. Charger IC (charger
driver) is separate with gadget.
When the usb cable is plugin, we need to report the plugin event to
charger driver to set current after setting configuration for gadget.
The usb charger is responsible for reporting plugin event to charger
driver. But how usb charger get the plugin event? It can get the
plugin event from gadget state (if the gadget state is more than
'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
gadget state to usb charger.
>
> --
> Best Regards,
> Peter Chen
--
Baolin.wang
Best Regards
Hi
> -----Original Message-----
> From: Baolin Wang [mailto:[email protected]]
> Sent: Wednesday, March 30, 2016 2:15 PM
> To: Jun Li <[email protected]>
> Cc: Peter Chen <[email protected]>; 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 v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
>
> On 30 March 2016 at 10:54, Jun Li <[email protected]> wrote:
> >> >> It is not for udc driver but for power users who want to negotiate
> >> >> with USB subsystem.
> >> >>
> >> >
> >> > Seems you don't want to guarantee charger type detection is done
> >> > before gadget connection(pullup DP), right?
> >> > I see you call usb_charger_detect_type() in each gadget usb state
> >> changes.
> >>
> >> I am not sure I get your point correctly, please correct me if I
> >> misunderstand you.
> >> We need to check the charger type at every event comes from the usb
> >> gadget state changes or the extcon device state changes, which means
> >> a new charger plugin or pullup.
> >>
> >
> > According to usb charger spec, my understanding is you can't do real
> > charger detection procedure *after* gadget _connection_(pullup DP),
> > also I don't
>
> Why can not? Charger detection is usually from PMIC.
Charger detection process will impact DP/DM line state, see usb charger
spec v1.2 for detail detection process, section 4.6.3 says:
"A PD is allowed to *disconnect* and repeat the charger detection process
multiple times while attached. The PD is required to wait for a time of at
least TCP_VDM_EN max between disconnecting and restarting the charger
detection process."
As Peter mentioned, the charger detection should happen between VBUS
detection and gadget pull up DP for first plug in case. So when&after
gadget connect (pullup DP), you should already know the charger type.
Li Jun
>
> > think it's necessary to check charger type at every event from usb
> gadget.
>
> My meaning is not every event from usb gadget. When the usb gadget state
> changes or the extcon device (maybe GPIO detection) state changes, which
> means charger plugin or pullup, we need to check the charger type to set
> current.
>From your below code, you call usb_charger_notify_others() in
every state change.
if (uchger->old_gadget_state != state) {
uchger->old_gadget_state = state;
if (state >= USB_STATE_ATTACHED)
uchger_state = USB_CHARGER_PRESENT;
else if (state == USB_STATE_NOTATTACHED)
uchger_state = USB_CHARGER_REMOVE;
else
/* this else will never happen */
uchger_state = USB_CHARGER_DEFAULT;
usb_charger_notify_others(uchger, uchger_state);
}
>
> > Something in gadget driver you can utilize is only vbus detection, and
> > report diff current by diff usb state if it's a SDP.
>
> --
> Baolin.wang
> Best Regards
On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
> >> > - Third, since composite driver covers 500mA (and more for CDP) after set
> >> > configuration and 2mA after suspend, and vbus handler covers connect
> >> > and disconnect. I can't see any reasons we need to notify gadget state
> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> >>
> >> In some solutions, gadget does not negotiate with the current. They
> >> just send out one signal to power driver to set the current when the
> >> gadget state is changed (plugin or not). So we need to check the
> >> charger state by the gadget state to notify the charger IC to set
> >> current.
> >>
> >
> > Would you give some examples?
>
> OK. I explain it in detail. Now charger detection can be from gadget
> itself or PMIC, and we focus on gadget detection. Charger IC (charger
> driver) is separate with gadget.
> When the usb cable is plugin, we need to report the plugin event to
> charger driver to set current after setting configuration for gadget.
> The usb charger is responsible for reporting plugin event to charger
> driver. But how usb charger get the plugin event? It can get the
> plugin event from gadget state (if the gadget state is more than
> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
> gadget state to usb charger.
>
Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
in your patch 2/4. Then, we need to make sure usb_charger_set_cur_limit_by_type
is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).
It seems you have not implemented usb_charger_plug_by_gadget in your patch set.
--
Best Regards,
Peter Chen
On 30 March 2016 at 16:07, Jun Li <[email protected]> wrote:
> Hi
>> On 30 March 2016 at 10:54, Jun Li <[email protected]> wrote:
>> >> >> It is not for udc driver but for power users who want to negotiate
>> >> >> with USB subsystem.
>> >> >>
>> >> >
>> >> > Seems you don't want to guarantee charger type detection is done
>> >> > before gadget connection(pullup DP), right?
>> >> > I see you call usb_charger_detect_type() in each gadget usb state
>> >> changes.
>> >>
>> >> I am not sure I get your point correctly, please correct me if I
>> >> misunderstand you.
>> >> We need to check the charger type at every event comes from the usb
>> >> gadget state changes or the extcon device state changes, which means
>> >> a new charger plugin or pullup.
>> >>
>> >
>> > According to usb charger spec, my understanding is you can't do real
>> > charger detection procedure *after* gadget _connection_(pullup DP),
>> > also I don't
>>
>> Why can not? Charger detection is usually from PMIC.
>
> Charger detection process will impact DP/DM line state, see usb charger
> spec v1.2 for detail detection process, section 4.6.3 says:
>
> "A PD is allowed to *disconnect* and repeat the charger detection process
> multiple times while attached. The PD is required to wait for a time of at
> least TCP_VDM_EN max between disconnecting and restarting the charger
> detection process."
>
> As Peter mentioned, the charger detection should happen between VBUS
> detection and gadget pull up DP for first plug in case. So when&after
> gadget connect (pullup DP), you should already know the charger type.
Make sense. In our company's solution, charger detection can be done
by hardware from PMIC at first, then it will not affect the DP/DM line
when gadget starts to enumeration. In the 'usb_charger_detect_type()',
it usually get the charger type from type registers has been done by
hardware from PMIC, which can not affect the DP/DM line.
>
> Li Jun
>
>>
>> > think it's necessary to check charger type at every event from usb
>> gadget.
>>
>> My meaning is not every event from usb gadget. When the usb gadget state
>> changes or the extcon device (maybe GPIO detection) state changes, which
>> means charger plugin or pullup, we need to check the charger type to set
>> current.
>
> From your below code, you call usb_charger_notify_others() in
> every state change.
I think it does not matter. In case the usb charger missed some gadget
state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
Thanks.
>
> if (uchger->old_gadget_state != state) {
> uchger->old_gadget_state = state;
>
> if (state >= USB_STATE_ATTACHED)
> uchger_state = USB_CHARGER_PRESENT;
> else if (state == USB_STATE_NOTATTACHED)
> uchger_state = USB_CHARGER_REMOVE;
> else
> /* this else will never happen */
> uchger_state = USB_CHARGER_DEFAULT;
>
> usb_charger_notify_others(uchger, uchger_state);
> }
>
>>
>> > Something in gadget driver you can utilize is only vbus detection, and
>> > report diff current by diff usb state if it's a SDP.
>>
>> --
>> Baolin.wang
>> Best Regards
--
Baolin.wang
Best Regards
On 30 March 2016 at 17:19, Peter Chen <[email protected]> wrote:
> On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
>> >> > - Third, since composite driver covers 500mA (and more for CDP) after set
>> >> > configuration and 2mA after suspend, and vbus handler covers connect
>> >> > and disconnect. I can't see any reasons we need to notify gadget state
>> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>> >>
>> >> In some solutions, gadget does not negotiate with the current. They
>> >> just send out one signal to power driver to set the current when the
>> >> gadget state is changed (plugin or not). So we need to check the
>> >> charger state by the gadget state to notify the charger IC to set
>> >> current.
>> >>
>> >
>> > Would you give some examples?
>>
>> OK. I explain it in detail. Now charger detection can be from gadget
>> itself or PMIC, and we focus on gadget detection. Charger IC (charger
>> driver) is separate with gadget.
>> When the usb cable is plugin, we need to report the plugin event to
>> charger driver to set current after setting configuration for gadget.
>> The usb charger is responsible for reporting plugin event to charger
>> driver. But how usb charger get the plugin event? It can get the
>> plugin event from gadget state (if the gadget state is more than
>> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
>> gadget state to usb charger.
>>
>
> Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
> in your patch 2/4. Then, we need to make sure usb_charger_set_cur_limit_by_type
> is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).
That's right.
>
> It seems you have not implemented usb_charger_plug_by_gadget in your patch set.
It is implemented in patch 3/4.
>
> --
> Best Regards,
> Peter Chen
--
Baolin.wang
Best Regards
> -----Original Message-----
> From: Baolin Wang [mailto:[email protected]]
> Sent: Wednesday, March 30, 2016 5:31 PM
> To: Jun Li <[email protected]>
> Cc: Peter Chen <[email protected]>; 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 v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
>
> On 30 March 2016 at 16:07, Jun Li <[email protected]> wrote:
> > Hi
>
> >> On 30 March 2016 at 10:54, Jun Li <[email protected]> wrote:
> >> >> >> It is not for udc driver but for power users who want to
> >> >> >> negotiate with USB subsystem.
> >> >> >>
> >> >> >
> >> >> > Seems you don't want to guarantee charger type detection is done
> >> >> > before gadget connection(pullup DP), right?
> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> > state
> >> >> changes.
> >> >>
> >> >> I am not sure I get your point correctly, please correct me if I
> >> >> misunderstand you.
> >> >> We need to check the charger type at every event comes from the
> >> >> usb gadget state changes or the extcon device state changes, which
> >> >> means a new charger plugin or pullup.
> >> >>
> >> >
> >> > According to usb charger spec, my understanding is you can't do
> >> > real charger detection procedure *after* gadget _connection_(pullup
> >> > DP), also I don't
> >>
> >> Why can not? Charger detection is usually from PMIC.
> >
> > Charger detection process will impact DP/DM line state, see usb
> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >
> > "A PD is allowed to *disconnect* and repeat the charger detection
> > process multiple times while attached. The PD is required to wait for
> > a time of at least TCP_VDM_EN max between disconnecting and restarting
> > the charger detection process."
> >
> > As Peter mentioned, the charger detection should happen between VBUS
> > detection and gadget pull up DP for first plug in case. So when&after
> > gadget connect (pullup DP), you should already know the charger type.
>
> Make sense. In our company's solution, charger detection can be done by
> hardware from PMIC at first, then it will not affect the DP/DM line when
> gadget starts to enumeration.
I see, charger type detection is done automatically by PMIC when VBUS is
detected in your case, you just assume the process is complete before SW
do gadget connect. To make the framework common, you may do one time charger type check when vbus is on, and save it to avoid repeat charger type check.
> In the 'usb_charger_detect_type()', it
> usually get the charger type from type registers has been done by hardware
> from PMIC, which can not affect the DP/DM line.
>
> >
> > Li Jun
> >
> >>
> >> > think it's necessary to check charger type at every event from usb
> >> gadget.
> >>
> >> My meaning is not every event from usb gadget. When the usb gadget
> >> state changes or the extcon device (maybe GPIO detection) state
> >> changes, which means charger plugin or pullup, we need to check the
> >> charger type to set current.
> >
> > From your below code, you call usb_charger_notify_others() in every
> > state change.
>
> I think it does not matter. In case the usb charger missed some gadget
> state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
> Thanks.
>
> >
> > if (uchger->old_gadget_state != state) {
> > uchger->old_gadget_state = state;
> >
> > if (state >= USB_STATE_ATTACHED)
> > uchger_state = USB_CHARGER_PRESENT;
> > else if (state == USB_STATE_NOTATTACHED)
> > uchger_state = USB_CHARGER_REMOVE;
> > else
> > /* this else will never happen */
> > uchger_state = USB_CHARGER_DEFAULT;
> >
> > usb_charger_notify_others(uchger, uchger_state); }
> >
> >>
> >> > Something in gadget driver you can utilize is only vbus detection,
> >> > and report diff current by diff usb state if it's a SDP.
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
>
>
>
> --
> Baolin.wang
> Best Regards
Hi,
Jun Li <[email protected]> writes:
>> -----Original Message-----
>> From: Baolin Wang [mailto:[email protected]]
>> Sent: Wednesday, March 30, 2016 5:31 PM
>> To: Jun Li <[email protected]>
>> Cc: Peter Chen <[email protected]>; 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 v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 30 March 2016 at 16:07, Jun Li <[email protected]> wrote:
>> > Hi
>>
>> >> On 30 March 2016 at 10:54, Jun Li <[email protected]> wrote:
>> >> >> >> It is not for udc driver but for power users who want to
>> >> >> >> negotiate with USB subsystem.
>> >> >> >>
>> >> >> >
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when&after
>> > gadget connect (pullup DP), you should already know the charger type.
>>
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration.
>
> I see, charger type detection is done automatically by PMIC when VBUS
> is detected in your case, you just assume the process is complete
assuming this finishes before gadget starts is a bad idea. It would've
been much more robust to delay usb_gadget_connect() until we KNOW
charger detection has completed.
--
balbi
On 30 March 2016 at 18:58, Jun Li <[email protected]> wrote:
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when&after
>> > gadget connect (pullup DP), you should already know the charger type.
>>
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration.
>
> I see, charger type detection is done automatically by PMIC when VBUS is
> detected in your case, you just assume the process is complete before SW
> do gadget connect. To make the framework common, you may do one time charger type check when vbus is on, and save it to avoid repeat charger type check.
OK. I'll add one judgement to check if the charger type is set in
'usb_charger_detect_type()' function.
--
Baolin.wang
Best Regards
On 30 March 2016 at 19:24, Felipe Balbi <[email protected]> wrote:
>>> >> >> >
>>> >> >> > Seems you don't want to guarantee charger type detection is done
>>> >> >> > before gadget connection(pullup DP), right?
>>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>>> >> >> > state
>>> >> >> changes.
>>> >> >>
>>> >> >> I am not sure I get your point correctly, please correct me if I
>>> >> >> misunderstand you.
>>> >> >> We need to check the charger type at every event comes from the
>>> >> >> usb gadget state changes or the extcon device state changes, which
>>> >> >> means a new charger plugin or pullup.
>>> >> >>
>>> >> >
>>> >> > According to usb charger spec, my understanding is you can't do
>>> >> > real charger detection procedure *after* gadget _connection_(pullup
>>> >> > DP), also I don't
>>> >>
>>> >> Why can not? Charger detection is usually from PMIC.
>>> >
>>> > Charger detection process will impact DP/DM line state, see usb
>>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>>> >
>>> > "A PD is allowed to *disconnect* and repeat the charger detection
>>> > process multiple times while attached. The PD is required to wait for
>>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>>> > the charger detection process."
>>> >
>>> > As Peter mentioned, the charger detection should happen between VBUS
>>> > detection and gadget pull up DP for first plug in case. So when&after
>>> > gadget connect (pullup DP), you should already know the charger type.
>>>
>>> Make sense. In our company's solution, charger detection can be done by
>>> hardware from PMIC at first, then it will not affect the DP/DM line when
>>> gadget starts to enumeration.
>>
>> I see, charger type detection is done automatically by PMIC when VBUS
>> is detected in your case, you just assume the process is complete
>
> assuming this finishes before gadget starts is a bad idea. It would've
> been much more robust to delay usb_gadget_connect() until we KNOW
> charger detection has completed.
It is hardware action to detect the charger type quickly. It actually
*gets* the charger type and does not means *detect* charger type in
'usb_charger_detect_type()' function. Maybe I need to change the
function name as 'usb_charger_get_type()'.
If some udc drivers want to detect charger type in
'gadget->ops->get_charger_type()' callback, they should avoid
impacting DP/DM line state at the right gadget state. Thanks.
>
> --
> balbi
--
Baolin.wang
Best Regards
Hi
> -----Original Message-----
> From: Baolin Wang [mailto:[email protected]]
> Sent: Thursday, March 31, 2016 1:23 PM
> To: Jun Li <[email protected]>
> Cc: Peter Chen <[email protected]>; 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 v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
>
> On 30 March 2016 at 18:58, Jun Li <[email protected]> wrote:
> >> >> >> > Seems you don't want to guarantee charger type detection is
> >> >> >> > done before gadget connection(pullup DP), right?
> >> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> >> > state
> >> >> >> changes.
> >> >> >>
> >> >> >> I am not sure I get your point correctly, please correct me if
> >> >> >> I misunderstand you.
> >> >> >> We need to check the charger type at every event comes from the
> >> >> >> usb gadget state changes or the extcon device state changes,
> >> >> >> which means a new charger plugin or pullup.
> >> >> >>
> >> >> >
> >> >> > According to usb charger spec, my understanding is you can't do
> >> >> > real charger detection procedure *after* gadget
> >> >> > _connection_(pullup DP), also I don't
> >> >>
> >> >> Why can not? Charger detection is usually from PMIC.
> >> >
> >> > Charger detection process will impact DP/DM line state, see usb
> >> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >> >
> >> > "A PD is allowed to *disconnect* and repeat the charger detection
> >> > process multiple times while attached. The PD is required to wait
> >> > for a time of at least TCP_VDM_EN max between disconnecting and
> >> > restarting the charger detection process."
> >> >
> >> > As Peter mentioned, the charger detection should happen between
> >> > VBUS detection and gadget pull up DP for first plug in case. So
> >> > when&after gadget connect (pullup DP), you should already know the
> charger type.
> >>
> >> Make sense. In our company's solution, charger detection can be done
> >> by hardware from PMIC at first, then it will not affect the DP/DM
> >> line when gadget starts to enumeration.
> >
> > I see, charger type detection is done automatically by PMIC when VBUS
> > is detected in your case, you just assume the process is complete
> > before SW do gadget connect. To make the framework common, you may do
> one time charger type check when vbus is on, and save it to avoid repeat
> charger type check.
>
> OK. I'll add one judgement to check if the charger type is set in
> 'usb_charger_detect_type()' function.
Just adding a judgement isn't enough here, your framework should make sure
usb_charger_detect_type() is called before gadget connect, with that, the
existing caller place just gets the charger type from the saved value.
The real charger type detection done by usb_charger_detect_type() can
be called only when vbus is on.
e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>
> --
> Baolin.wang
> Best Regards
Hi,
Baolin Wang <[email protected]> writes:
> [ text/plain ]
> On 30 March 2016 at 19:24, Felipe Balbi <[email protected]> wrote:
>>>> >> >> >
>>>> >> >> > Seems you don't want to guarantee charger type detection is done
>>>> >> >> > before gadget connection(pullup DP), right?
>>>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>>>> >> >> > state
>>>> >> >> changes.
>>>> >> >>
>>>> >> >> I am not sure I get your point correctly, please correct me if I
>>>> >> >> misunderstand you.
>>>> >> >> We need to check the charger type at every event comes from the
>>>> >> >> usb gadget state changes or the extcon device state changes, which
>>>> >> >> means a new charger plugin or pullup.
>>>> >> >>
>>>> >> >
>>>> >> > According to usb charger spec, my understanding is you can't do
>>>> >> > real charger detection procedure *after* gadget _connection_(pullup
>>>> >> > DP), also I don't
>>>> >>
>>>> >> Why can not? Charger detection is usually from PMIC.
>>>> >
>>>> > Charger detection process will impact DP/DM line state, see usb
>>>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>>>> >
>>>> > "A PD is allowed to *disconnect* and repeat the charger detection
>>>> > process multiple times while attached. The PD is required to wait for
>>>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>>>> > the charger detection process."
>>>> >
>>>> > As Peter mentioned, the charger detection should happen between VBUS
>>>> > detection and gadget pull up DP for first plug in case. So when&after
>>>> > gadget connect (pullup DP), you should already know the charger type.
>>>>
>>>> Make sense. In our company's solution, charger detection can be done by
>>>> hardware from PMIC at first, then it will not affect the DP/DM line when
>>>> gadget starts to enumeration.
>>>
>>> I see, charger type detection is done automatically by PMIC when VBUS
>>> is detected in your case, you just assume the process is complete
>>
>> assuming this finishes before gadget starts is a bad idea. It would've
>> been much more robust to delay usb_gadget_connect() until we KNOW
>> charger detection has completed.
>
> It is hardware action to detect the charger type quickly. It actually
> *gets* the charger type and does not means *detect* charger type in
> 'usb_charger_detect_type()' function. Maybe I need to change the
> function name as 'usb_charger_get_type()'.
yes.
> If some udc drivers want to detect charger type in
> 'gadget->ops->get_charger_type()' callback, they should avoid
> impacting DP/DM line state at the right gadget state. Thanks.
they shouldn't detect is get_type(), the semantics doesn't work. If, at
some point, we have to do SW detection of the charger, then a new
->charger_detect() method will have to be added.
--
balbi
On 31 March 2016 at 14:18, Felipe Balbi <[email protected]> wrote:
>
> Hi,
>
> Baolin Wang <[email protected]> writes:
>> [ text/plain ]
>> On 30 March 2016 at 19:24, Felipe Balbi <[email protected]> wrote:
>>>>> >> >> >
>>>>> >> >> > Seems you don't want to guarantee charger type detection is done
>>>>> >> >> > before gadget connection(pullup DP), right?
>>>>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>>>>> >> >> > state
>>>>> >> >> changes.
>>>>> >> >>
>>>>> >> >> I am not sure I get your point correctly, please correct me if I
>>>>> >> >> misunderstand you.
>>>>> >> >> We need to check the charger type at every event comes from the
>>>>> >> >> usb gadget state changes or the extcon device state changes, which
>>>>> >> >> means a new charger plugin or pullup.
>>>>> >> >>
>>>>> >> >
>>>>> >> > According to usb charger spec, my understanding is you can't do
>>>>> >> > real charger detection procedure *after* gadget _connection_(pullup
>>>>> >> > DP), also I don't
>>>>> >>
>>>>> >> Why can not? Charger detection is usually from PMIC.
>>>>> >
>>>>> > Charger detection process will impact DP/DM line state, see usb
>>>>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>>>>> >
>>>>> > "A PD is allowed to *disconnect* and repeat the charger detection
>>>>> > process multiple times while attached. The PD is required to wait for
>>>>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>>>>> > the charger detection process."
>>>>> >
>>>>> > As Peter mentioned, the charger detection should happen between VBUS
>>>>> > detection and gadget pull up DP for first plug in case. So when&after
>>>>> > gadget connect (pullup DP), you should already know the charger type.
>>>>>
>>>>> Make sense. In our company's solution, charger detection can be done by
>>>>> hardware from PMIC at first, then it will not affect the DP/DM line when
>>>>> gadget starts to enumeration.
>>>>
>>>> I see, charger type detection is done automatically by PMIC when VBUS
>>>> is detected in your case, you just assume the process is complete
>>>
>>> assuming this finishes before gadget starts is a bad idea. It would've
>>> been much more robust to delay usb_gadget_connect() until we KNOW
>>> charger detection has completed.
>>
>> It is hardware action to detect the charger type quickly. It actually
>> *gets* the charger type and does not means *detect* charger type in
>> 'usb_charger_detect_type()' function. Maybe I need to change the
>> function name as 'usb_charger_get_type()'.
>
> yes.
>
>> If some udc drivers want to detect charger type in
>> 'gadget->ops->get_charger_type()' callback, they should avoid
>> impacting DP/DM line state at the right gadget state. Thanks.
>
> they shouldn't detect is get_type(), the semantics doesn't work. If, at
> some point, we have to do SW detection of the charger, then a new
> ->charger_detect() method will have to be added.
Make sense.
>
> --
> balbi
--
Baolin.wang
Best Regards
On 31 March 2016 at 14:12, Jun Li <[email protected]> wrote:
> Hi
>
>> -----Original Message-----
>> From: Baolin Wang [mailto:[email protected]]
>> Sent: Thursday, March 31, 2016 1:23 PM
>> To: Jun Li <[email protected]>
>> Cc: Peter Chen <[email protected]>; 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 v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 30 March 2016 at 18:58, Jun Li <[email protected]> wrote:
>> >> >> >> > Seems you don't want to guarantee charger type detection is
>> >> >> >> > done before gadget connection(pullup DP), right?
>> >> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> >> > state
>> >> >> >> changes.
>> >> >> >>
>> >> >> >> I am not sure I get your point correctly, please correct me if
>> >> >> >> I misunderstand you.
>> >> >> >> We need to check the charger type at every event comes from the
>> >> >> >> usb gadget state changes or the extcon device state changes,
>> >> >> >> which means a new charger plugin or pullup.
>> >> >> >>
>> >> >> >
>> >> >> > According to usb charger spec, my understanding is you can't do
>> >> >> > real charger detection procedure *after* gadget
>> >> >> > _connection_(pullup DP), also I don't
>> >> >>
>> >> >> Why can not? Charger detection is usually from PMIC.
>> >> >
>> >> > Charger detection process will impact DP/DM line state, see usb
>> >> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >> >
>> >> > "A PD is allowed to *disconnect* and repeat the charger detection
>> >> > process multiple times while attached. The PD is required to wait
>> >> > for a time of at least TCP_VDM_EN max between disconnecting and
>> >> > restarting the charger detection process."
>> >> >
>> >> > As Peter mentioned, the charger detection should happen between
>> >> > VBUS detection and gadget pull up DP for first plug in case. So
>> >> > when&after gadget connect (pullup DP), you should already know the
>> charger type.
>> >>
>> >> Make sense. In our company's solution, charger detection can be done
>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>> >> line when gadget starts to enumeration.
>> >
>> > I see, charger type detection is done automatically by PMIC when VBUS
>> > is detected in your case, you just assume the process is complete
>> > before SW do gadget connect. To make the framework common, you may do
>> one time charger type check when vbus is on, and save it to avoid repeat
>> charger type check.
>>
>> OK. I'll add one judgement to check if the charger type is set in
>> 'usb_charger_detect_type()' function.
>
> Just adding a judgement isn't enough here, your framework should make sure
> usb_charger_detect_type() is called before gadget connect, with that, the
> existing caller place just gets the charger type from the saved value.
> The real charger type detection done by usb_charger_detect_type() can
> be called only when vbus is on.
> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
Yeah, Like Felipe suggested, I think we need to introduce one
'charger_detect()' method to do the SW charger type detection at the
right gadget state. Thanks for your comments.
>>
>> --
>> Baolin.wang
>> Best Regards
--
Baolin.wang
Best Regards
Hi Baolin,
Baolin Wang <[email protected]> writes:
>>> >> Make sense. In our company's solution, charger detection can be done
>>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>>> >> line when gadget starts to enumeration.
>>> >
>>> > I see, charger type detection is done automatically by PMIC when VBUS
>>> > is detected in your case, you just assume the process is complete
>>> > before SW do gadget connect. To make the framework common, you may do
>>> one time charger type check when vbus is on, and save it to avoid repeat
>>> charger type check.
>>>
>>> OK. I'll add one judgement to check if the charger type is set in
>>> 'usb_charger_detect_type()' function.
>>
>> Just adding a judgement isn't enough here, your framework should make sure
>> usb_charger_detect_type() is called before gadget connect, with that, the
>> existing caller place just gets the charger type from the saved value.
>> The real charger type detection done by usb_charger_detect_type() can
>> be called only when vbus is on.
>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>
> Yeah, Like Felipe suggested, I think we need to introduce one
> 'charger_detect()' method to do the SW charger type detection at the
> right gadget state. Thanks for your comments.
Just to be clear, we add ->charger_detect() when we know of a platform
which needs to manually detect the charger type. Until then, we ignore
that situation. It might be a good idea, however, do document this in
comments on your structure definition stating that if we need to detect
charger type, a new method should be added ;-)
cheers
--
balbi
On 31 March 2016 at 16:15, Felipe Balbi <[email protected]> wrote:
>
> Hi Baolin,
>
> Baolin Wang <[email protected]> writes:
>>>> >> Make sense. In our company's solution, charger detection can be done
>>>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>>>> >> line when gadget starts to enumeration.
>>>> >
>>>> > I see, charger type detection is done automatically by PMIC when VBUS
>>>> > is detected in your case, you just assume the process is complete
>>>> > before SW do gadget connect. To make the framework common, you may do
>>>> one time charger type check when vbus is on, and save it to avoid repeat
>>>> charger type check.
>>>>
>>>> OK. I'll add one judgement to check if the charger type is set in
>>>> 'usb_charger_detect_type()' function.
>>>
>>> Just adding a judgement isn't enough here, your framework should make sure
>>> usb_charger_detect_type() is called before gadget connect, with that, the
>>> existing caller place just gets the charger type from the saved value.
>>> The real charger type detection done by usb_charger_detect_type() can
>>> be called only when vbus is on.
>>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>>
>> Yeah, Like Felipe suggested, I think we need to introduce one
>> 'charger_detect()' method to do the SW charger type detection at the
>> right gadget state. Thanks for your comments.
>
> Just to be clear, we add ->charger_detect() when we know of a platform
> which needs to manually detect the charger type. Until then, we ignore
> that situation. It might be a good idea, however, do document this in
> comments on your structure definition stating that if we need to detect
> charger type, a new method should be added ;-)
Make sense. Thanks.
>
> cheers
>
> --
> balbi
--
Baolin.wang
Best Regards
Hi!
> +/*
> + * Sysfs attributes:
> + *
> + * These sysfs attributes are used for showing and setting different type
> + * (SDP/DCP/CDP/ACA) chargers' current limitation.
> + */
> +static ssize_t sdp_limit_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb_charger *uchger = dev_to_uchger(dev);
> +
> + return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
> +}
> +
Sysfs attributes... can we get appropriate documentation for them?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
On 24 April 2016 at 03:53, Pavel Machek <[email protected]> wrote:
> Hi!
>
>> +/*
>> + * Sysfs attributes:
>> + *
>> + * These sysfs attributes are used for showing and setting different type
>> + * (SDP/DCP/CDP/ACA) chargers' current limitation.
>> + */
>> +static ssize_t sdp_limit_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct usb_charger *uchger = dev_to_uchger(dev);
>> +
>> + return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
>> +}
>> +
>
> Sysfs attributes... can we get appropriate documentation for them?
I've send out the v10 patchset which fixed this issue. Thanks.
>
> Thanks,
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
Baolin.wang
Best Regards