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

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

[Baolin Wang]

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

Providing a standard framework for doing this in the 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 v2:
 - Use macro DEVICE_ATTR_RW()/ATTRIBUTE_GROUPS() and change the method to
   register the attruibutes.
 - Remove the 'name' and 'entry' member of struct 'usb_charger'.
 - Replace devm_kzalloc() with kzalloc() to follow the the lifetime rules.
 - Other modifications.

Baolin Wang (3):
  gadget: Support for the usb charger framework
  gadget: Introduce the usb charger framework
  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      |  538 +++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/udc/udc-core.c |   41 ++-
 include/linux/mfd/wm831x/pdata.h  |    3 +
 include/linux/usb/gadget.h        |   20 ++
 include/linux/usb/usb_charger.h   |  138 ++++++++++
 8 files changed, 816 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/gadget/charger.c
 create mode 100644 include/linux/usb/usb_charger.h

-- 
1.7.9.5

[PATCH v3 1/3] gadget: Support for the usb charger framework

[Baolin Wang]

The usb charger framework is based on usb gadget, and each usb gadget
can be one usb charger to set the current limitation.

This patch adds a notifier mechanism for usb charger to report to usb
charger when the usb gadget state is changed.

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

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

diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index f660afb..1971218 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
@@ -129,6 +130,32 @@ void usb_gadget_giveback_request(struct usb_ep *ep,
 }
 EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
 
+int usb_gadget_register_notify(struct usb_gadget *gadget,
+			       struct notifier_block *nb)
+{
+	int ret;
+
+	mutex_lock(&gadget->lock);
+	ret = raw_notifier_chain_register(&gadget->nh, nb);
+	mutex_unlock(&gadget->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_register_notify);
+
+int usb_gadget_unregister_notify(struct usb_gadget *gadget,
+				 struct notifier_block *nb)
+{
+	int ret;
+
+	mutex_lock(&gadget->lock);
+	ret = raw_notifier_chain_unregister(&gadget->nh, nb);
+	mutex_unlock(&gadget->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_unregister_notify);
+
 /* ------------------------------------------------------------------------- */
 
 /**
@@ -226,6 +253,10 @@ static void usb_gadget_state_work(struct work_struct *work)
 	struct usb_gadget *gadget = work_to_gadget(work);
 	struct usb_udc *udc = gadget->udc;
 
+	mutex_lock(&gadget->lock);
+	raw_notifier_call_chain(&gadget->nh, gadget->state, gadget);
+	mutex_unlock(&gadget->lock);
+
 	if (udc)
 		sysfs_notify(&udc->dev.kobj, NULL, "state");
 }
@@ -364,6 +395,8 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
 
 	dev_set_name(&gadget->dev, "gadget");
 	INIT_WORK(&gadget->work, usb_gadget_state_work);
+	RAW_INIT_NOTIFIER_HEAD(&gadget->nh);
+	mutex_init(&gadget->lock);
 	gadget->dev.parent = parent;
 
 #ifdef	CONFIG_HAS_DMA
@@ -405,8 +438,13 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
 
 	mutex_unlock(&udc_lock);
 
-	return 0;
+	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);
@@ -481,6 +519,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 c14a69b..78cc862 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -537,6 +537,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 *);
 };
 
 /**
@@ -609,6 +610,9 @@ struct usb_gadget {
 	unsigned			out_epnum;
 	unsigned			in_epnum;
 	struct usb_otg_caps		*otg_caps;
+	struct raw_notifier_head	nh;
+	struct usb_charger		*uchger;
+	struct mutex			lock;
 
 	unsigned			sg_supported:1;
 	unsigned			is_otg:1;
@@ -1183,6 +1187,22 @@ extern void usb_gadget_unmap_request(struct usb_gadget *gadget,
 
 /*-------------------------------------------------------------------------*/
 
+/**
+ * Register a notifiee to get notified by any attach status changes from
+ * the usb gadget
+ */
+int usb_gadget_register_notify(struct usb_gadget *gadget,
+			       struct notifier_block *nb);
+
+/*-------------------------------------------------------------------------*/
+
+
+/* Unregister a notifiee from the usb gadget */
+int usb_gadget_unregister_notify(struct usb_gadget *gadget,
+				 struct notifier_block *nb);
+
+/*-------------------------------------------------------------------------*/
+
 /* utility to set gadget state properly */
 
 extern void usb_gadget_set_state(struct usb_gadget *gadget,
-- 
1.7.9.5

Re: [PATCH v3 1/3] gadget: Support for the usb charger framework

[Greg KH]

On Tue, Aug 18, 2015 at 07:14:19PM +0800, Baolin Wang wrote:
> The usb charger framework is based on usb gadget, and each usb gadget
> can be one usb charger to set the current limitation.
> 
> This patch adds a notifier mechanism for usb charger to report to usb
> charger when the usb gadget state is changed.
> 
> Also we introduce a callback 'get_charger_type' which will implemented
> by user for usb gadget operations to get the usb charger type.
> 
> Signed-off-by: Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/usb/gadget/udc/udc-core.c |   41 ++++++++++++++++++++++++++++++++++++-
>  include/linux/usb/gadget.h        |   20 ++++++++++++++++++
>  2 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index f660afb..1971218 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>

You just broke the build, which proves you did not properly test this
patch series, so why should we even consider it?

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

Re: [PATCH v3 1/3] gadget: Support for the usb charger framework

[Baolin Wang]

On 19 August 2015 at 00:04, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 18, 2015 at 07:14:19PM +0800, Baolin Wang wrote:
>> The usb charger framework is based on usb gadget, and each usb gadget
>> can be one usb charger to set the current limitation.
>>
>> This patch adds a notifier mechanism for usb charger to report to usb
>> charger when the usb gadget state is changed.
>>
>> Also we introduce a callback 'get_charger_type' which will implemented
>> by user for usb gadget operations to get the usb charger type.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>>  drivers/usb/gadget/udc/udc-core.c |   41 ++++++++++++++++++++++++++++++++++++-
>>  include/linux/usb/gadget.h        |   20 ++++++++++++++++++
>>  2 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
>> index f660afb..1971218 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>
>
> You just broke the build, which proves you did not properly test this
> patch series, so why should we even consider it?
>

Oh, I'm very sorry about that. I'll re-check my patch and test it
carefully. Thanks for your comments.

> greg k-h



-- 
Baolin.wang
Best Regards

[PATCH v3 2/3] gadget: Introduce the usb charger framework

[Baolin Wang]

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

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

The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state.

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.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/usb/gadget/Kconfig      |    7 +
 drivers/usb/gadget/Makefile     |    1 +
 drivers/usb/gadget/charger.c    |  538 +++++++++++++++++++++++++++++++++++++++
 include/linux/usb/usb_charger.h |  138 ++++++++++
 4 files changed, 684 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 bcf83c0..3d2b959 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -127,6 +127,13 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
 	   a module parameter as well.
 	   If unsure, say 2.
 
+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..93fd36f
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,538 @@
+/*
+ * 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>
+
+#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)
+
+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);
+}
+
+static ssize_t cur_limit_show(struct device *dev,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%d %d %d %d\n",
+			 uchger->cur_limit.sdp_cur_limit,
+			 uchger->cur_limit.dcp_cur_limit,
+			 uchger->cur_limit.cdp_cur_limit,
+			 uchger->cur_limit.aca_cur_limit);
+}
+
+static ssize_t cur_limit_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct usb_charger *uchger = dev_to_uchger(dev);
+	struct usb_charger_cur_limit cur;
+	int ret;
+
+	ret = sscanf(buf, "%d %d %d %d",
+		     &cur.sdp_cur_limit, &cur.dcp_cur_limit,
+		     &cur.cdp_cur_limit, &cur.aca_cur_limit);
+	if (ret == 0)
+		return -EINVAL;
+
+	ret = usb_charger_set_cur_limit(uchger, &cur);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+static DEVICE_ATTR_RW(cur_limit);
+
+static struct attribute *usb_charger_attrs[] = {
+	&dev_attr_cur_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);
+}
+
+/*
+ * 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;
+}
+
+/*
+ * 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);
+}
+
+/*
+ * 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;
+}
+
+/*
+ * 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;
+}
+
+/*
+ * 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->gadget && uchger->gadget->ops
+	    && uchger->gadget->ops->get_charger_type)
+		uchger->type =
+			uchger->gadget->ops->get_charger_type(uchger->gadget);
+	else
+		uchger->type = UNKNOWN_TYPE;
+
+	return uchger->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;
+}
+
+/*
+ * 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 = uchger->type;
+	unsigned int cur_limit;
+
+	switch (uchger_type) {
+	case SDP_TYPE:
+		cur_limit = uchger->cur_limit.sdp_cur_limit;
+		break;
+	case DCP_TYPE:
+		cur_limit = uchger->cur_limit.dcp_cur_limit;
+		break;
+	case CDP_TYPE:
+		cur_limit = uchger->cur_limit.cdp_cur_limit;
+		break;
+	case ACA_TYPE:
+		cur_limit = uchger->cur_limit.aca_cur_limit;
+		break;
+	default:
+		return 0;
+	}
+
+	return cur_limit;
+}
+
+/*
+ * usb_charger_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)
+{
+	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);
+		break;
+	case USB_CHARGER_REMOVE:
+		uchger->type = UNKNOWN_TYPE;
+		raw_notifier_call_chain(&uchger->uchger_nh, 0, uchger);
+		break;
+	default:
+		dev_warn(&uchger->dev, "Unknown USB charger state: %d\n",
+			 state);
+		break;
+	}
+	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.
+ * @nb - the notifier block that notified by usb gadget device.
+ * @state - the usb gadget state.
+ * @data - here specify a usb gadget device.
+ */
+static int
+usb_charger_plug_by_gadget(struct notifier_block *nb,
+			   unsigned long state, void *data)
+{
+	struct usb_gadget *gadget = (struct usb_gadget *)data;
+	struct usb_charger *uchger = gadget->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 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 NOTIFY_OK;
+}
+
+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;
+	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;
+
+	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);
+	}
+
+	/* register a notifier on a usb gadget device */
+	uchger->gadget = ugadget;
+	ugadget->uchger = uchger;
+	uchger->old_gadget_state = ugadget->state;
+	uchger->gadget_nb.notifier_call = usb_charger_plug_by_gadget;
+	usb_gadget_register_notify(ugadget, &uchger->gadget_nb);
+
+	/* 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);
+
+	usb_gadget_unregister_notify(uchger->gadget, &uchger->gadget_nb);
+	kfree(uchger);
+	return ret;
+}
+
+int usb_charger_exit(struct usb_gadget *ugadget)
+{
+	struct usb_charger *uchger = ugadget->uchger;
+
+	if (!uchger)
+		return -EINVAL;
+
+	if (uchger->extcon_dev)
+		extcon_unregister_notifier(uchger->extcon_dev,
+					   EXTCON_USB, &uchger->extcon_nb.nb);
+
+	usb_gadget_unregister_notify(uchger->gadget, &uchger->gadget_nb);
+	ida_simple_remove(&usb_charger_ida, uchger->id);
+
+	return usb_charger_unregister(uchger);
+}
+
+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 <baolin.wang@linaro.org>");
+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..99bc26f
--- /dev/null
+++ b/include/linux/usb/usb_charger.h
@@ -0,0 +1,138 @@
+#ifndef __LINUX_USB_CHARGER_H__
+#define __LINUX_USB_CHARGER_H__
+
+/* USB charger type:
+ * SDP (Standard Downstream Port)
+ * DCP (Dedicated Charging Port)
+ * CDP (Charging Downstream Port)
+ * ACA (Accessory Charger Adapters)
+ */
+enum usb_charger_type {
+	UNKNOWN_TYPE,
+	SDP_TYPE,
+	DCP_TYPE,
+	CDP_TYPE,
+	ACA_TYPE,
+};
+
+/* USB charger state */
+enum usb_charger_state {
+	USB_CHARGER_DEFAULT,
+	USB_CHARGER_PRESENT,
+	USB_CHARGER_REMOVE,
+};
+
+/* 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;
+	struct notifier_block	gadget_nb;
+
+	/* 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 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_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 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_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

[PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

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

Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 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 db11ae6..72c661f 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 miliamps */
+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
  *********************************************************************/
@@ -606,8 +646,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:
+	usb_charger_put(power->usb_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);
+		usb_charger_put(wm831x_power->usb_charger);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
 		irq = wm831x_irq(wm831x, 
 				 platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
 	/** The driver should initiate a power off sequence during shutdown */
 	bool soft_shutdown;
 
+	/** dev_name of USB charger gadget to integrate with */
+	const char *usb_gadget;
+
 	int irq_base;
 	int gpio_base;
 	int gpio_defaults[WM831X_GPIO_NUM];
-- 
1.7.9.5

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Lee Jones]

On Tue, 18 Aug 2015, Baolin Wang wrote:

> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
> 
> Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/power/wm831x_power.c     |   69 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/wm831x/pdata.h |    3 ++

MFD:
  Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

>  2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
> index db11ae6..72c661f 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 miliamps */
> +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
>   *********************************************************************/
> @@ -606,8 +646,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:
> +	usb_charger_put(power->usb_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);
> +		usb_charger_put(wm831x_power->usb_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];

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Charles Keepax]

On Tue, Aug 18, 2015 at 07:14:21PM +0800, Baolin Wang wrote:
> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Sorry if you guys got a bunch of bounces, we had a bit of an
email fiasco at this end. Hopefully all fixed now.

Thanks,
Charles

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Lee Jones]

On Tue, 18 Aug 2015, Charles Keepax wrote:

> On Tue, Aug 18, 2015 at 07:14:21PM +0800, Baolin Wang wrote:
> > Integrate with the newly added USB charger interface to limit the current
> > we draw from the USB input based on the input device configuration
> > identified by the USB stack, allowing us to charge more quickly from high
> > current inputs without drawing more current than specified from others.
> > 
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > ---
> 
> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> 
> Sorry if you guys got a bunch of bounces, we had a bit of an
> email fiasco at this end. Hopefully all fixed now.

Yes, I sent you guys a mail about that.

Glad it's all sorted now.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 0/3] Introduce USB charger support in USB phy

[Baolin Wang]

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

Now introduce one user with wm831x_power to support and test the usb charger.
In future we will also cnvert below power drivers:
drivers/power/supply/axp288_charger.c
drivers/power/supply/bq24190_charger.c
drivers/power/supply/charger-manager.c
drivers/power/supply/qcom_smbb.c

Changes since v1:
 - Fix building errors.
Changes since v2:
 - Add DT binding documentation for wm831x_power driver.
 - Change 'usb-phy' as one optional property for wm831x_power driver.

Baolin Wang (3):
  include: uapi: usb: Introduce USB charger type and state definition
  usb: phy: Add USB charger support
  power: wm831x_power: Support USB charger current limit management

 Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +
 drivers/power/supply/wm831x_power.c              |   58 +++++
 drivers/usb/phy/phy.c                            |  272 ++++++++++++++++++++++
 include/linux/usb/phy.h                          |   49 ++++
 include/uapi/linux/usb/charger.h                 |   31 +++
 5 files changed, 411 insertions(+)
 create mode 100644 include/uapi/linux/usb/charger.h

-- 
1.7.9.5

[PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

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

Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +
 drivers/power/supply/wm831x_power.c              |   58 ++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt b/Documentation/devicetree/bindings/mfd/wm831x.txt
index 9f8b743..4e3bc07 100644
--- a/Documentation/devicetree/bindings/mfd/wm831x.txt
+++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
@@ -31,6 +31,7 @@ Required properties:
     ../interrupt-controller/interrupts.txt
 
 Optional sub-nodes:
+  - usb-phy : Contains a phandle to the USB PHY.
   - regulators : Contains sub-nodes for each of the regulators supplied by
     the device. The regulators are bound using their names listed below:
 
diff --git a/drivers/power/supply/wm831x_power.c b/drivers/power/supply/wm831x_power.c
index 7082301..d3948ab 100644
--- a/drivers/power/supply/wm831x_power.c
+++ b/drivers/power/supply/wm831x_power.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
+#include <linux/usb/phy.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_phy *usb_phy;
+	struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+	0,
+	2,
+	100,
+	500,
+	900,
+	1500,
+	1800,
+	550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+				   unsigned long limit, void *data)
+{
+	struct wm831x_power *wm831x_power = container_of(nb,
+							 struct wm831x_power,
+							 usb_notify);
+	unsigned int i, best;
+
+	/* Find the highest supported limit */
+	best = 0;
+	for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+		if (limit >= wm831x_usb_limits[i] &&
+		    wm831x_usb_limits[best] < wm831x_usb_limits[i])
+			best = i;
+	}
+
+	dev_dbg(wm831x_power->wm831x->dev,
+		"Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+	wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+		        WM831X_USB_ILIM_MASK, best);
+
+	return 0;
+}
+
 /*********************************************************************
  *		Battery properties
  *********************************************************************/
@@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device *pdev)
 		}
 	}
 
+	power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
+						     "usb-phy", 0);
+	if (!IS_ERR(power->usb_phy)) {
+		power->usb_notify.notifier_call = wm831x_usb_limit_change;
+		ret = usb_register_notifier(power->usb_phy,
+					    &power->usb_notify);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to register notifier: %d\n",
+				ret);
+			goto err_bat_irq;
+		}
+	}
+
 	return ret;
 
 err_bat_irq:
@@ -637,6 +690,11 @@ static int wm831x_power_remove(struct platform_device *pdev)
 	struct wm831x *wm831x = wm831x_power->wm831x;
 	int irq, i;
 
+	if (!IS_ERR(wm831x_power->usb_phy)) {
+		usb_unregister_notifier(wm831x_power->usb_phy,
+					&wm831x_power->usb_notify);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
 		irq = wm831x_irq(wm831x, 
 				 platform_get_irq_byname(pdev,
-- 
1.7.9.5

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Lee Jones]

On Tue, 25 Jul 2017, Baolin Wang wrote:

> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Charles Keepax]

On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Sebastian Reichel]

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

Hi,

On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +
>  drivers/power/supply/wm831x_power.c              |   58 ++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt b/Documentation/devicetree/bindings/mfd/wm831x.txt
> index 9f8b743..4e3bc07 100644
> --- a/Documentation/devicetree/bindings/mfd/wm831x.txt
> +++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
> @@ -31,6 +31,7 @@ Required properties:
>      ../interrupt-controller/interrupts.txt
>  
>  Optional sub-nodes:
> +  - usb-phy : Contains a phandle to the USB PHY.
>    - regulators : Contains sub-nodes for each of the regulators supplied by
>      the device. The regulators are bound using their names listed below:
>  
> diff --git a/drivers/power/supply/wm831x_power.c b/drivers/power/supply/wm831x_power.c
> index 7082301..d3948ab 100644
> --- a/drivers/power/supply/wm831x_power.c
> +++ b/drivers/power/supply/wm831x_power.c
> @@ -13,6 +13,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/slab.h>
> +#include <linux/usb/phy.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_phy *usb_phy;
> +	struct notifier_block usb_notify;
>  };
>  
>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> @@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
>  	POWER_SUPPLY_PROP_VOLTAGE_NOW,
>  };
>  
> +/* In milliamps */
> +static const unsigned int wm831x_usb_limits[] = {
> +	0,
> +	2,
> +	100,
> +	500,
> +	900,
> +	1500,
> +	1800,
> +	550,
> +};
> +
> +static int wm831x_usb_limit_change(struct notifier_block *nb,
> +				   unsigned long limit, void *data)
> +{
> +	struct wm831x_power *wm831x_power = container_of(nb,
> +							 struct wm831x_power,
> +							 usb_notify);
> +	unsigned int i, best;
> +
> +	/* Find the highest supported limit */
> +	best = 0;
> +	for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
> +		if (limit >= wm831x_usb_limits[i] &&
> +		    wm831x_usb_limits[best] < wm831x_usb_limits[i])
> +			best = i;
> +	}
> +
> +	dev_dbg(wm831x_power->wm831x->dev,
> +		"Limiting USB current to %umA", wm831x_usb_limits[best]);
> +
> +	wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
> +		        WM831X_USB_ILIM_MASK, best);
> +
> +	return 0;
> +}
> +
>  /*********************************************************************
>   *		Battery properties
>   *********************************************************************/
> @@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
> +						     "usb-phy", 0);
> +	if (!IS_ERR(power->usb_phy)) {
> +		power->usb_notify.notifier_call = wm831x_usb_limit_change;
> +		ret = usb_register_notifier(power->usb_phy,
> +					    &power->usb_notify);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to register notifier: %d\n",
> +				ret);
> +			goto err_bat_irq;
> +		}
> +	}

No error handling for power->usb_phy? I think you should bail out
for all errors except for "not defined in DT". Especially I would
expect probe defer handling in case the power supply driver is
loaded before the phy driver.

>  	return ret;
>  
>  err_bat_irq:
> @@ -637,6 +690,11 @@ static int wm831x_power_remove(struct platform_device *pdev)
>  	struct wm831x *wm831x = wm831x_power->wm831x;
>  	int irq, i;
>  
> +	if (!IS_ERR(wm831x_power->usb_phy)) {
> +		usb_unregister_notifier(wm831x_power->usb_phy,
> +					&wm831x_power->usb_notify);
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
>  		irq = wm831x_irq(wm831x, 
>  				 platform_get_irq_byname(pdev,

-- Sebastian

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

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Hi,

On 25 July 2017 at 17:59, Sebastian Reichel
<sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org> wrote:
> Hi,
>
> On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
>> Integrate with the newly added USB charger interface to limit the current
>> we draw from the USB input based on the input device configuration
>> identified by the USB stack, allowing us to charge more quickly from high
>> current inputs without drawing more current than specified from others.
>>
>> Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +
>>  drivers/power/supply/wm831x_power.c              |   58 ++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt b/Documentation/devicetree/bindings/mfd/wm831x.txt
>> index 9f8b743..4e3bc07 100644
>> --- a/Documentation/devicetree/bindings/mfd/wm831x.txt
>> +++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
>> @@ -31,6 +31,7 @@ Required properties:
>>      ../interrupt-controller/interrupts.txt
>>
>>  Optional sub-nodes:
>> +  - usb-phy : Contains a phandle to the USB PHY.
>>    - regulators : Contains sub-nodes for each of the regulators supplied by
>>      the device. The regulators are bound using their names listed below:
>>
>> diff --git a/drivers/power/supply/wm831x_power.c b/drivers/power/supply/wm831x_power.c
>> index 7082301..d3948ab 100644
>> --- a/drivers/power/supply/wm831x_power.c
>> +++ b/drivers/power/supply/wm831x_power.c
>> @@ -13,6 +13,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/power_supply.h>
>>  #include <linux/slab.h>
>> +#include <linux/usb/phy.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_phy *usb_phy;
>> +     struct notifier_block usb_notify;
>>  };
>>
>>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
>> @@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
>>       POWER_SUPPLY_PROP_VOLTAGE_NOW,
>>  };
>>
>> +/* In milliamps */
>> +static const unsigned int wm831x_usb_limits[] = {
>> +     0,
>> +     2,
>> +     100,
>> +     500,
>> +     900,
>> +     1500,
>> +     1800,
>> +     550,
>> +};
>> +
>> +static int wm831x_usb_limit_change(struct notifier_block *nb,
>> +                                unsigned long limit, void *data)
>> +{
>> +     struct wm831x_power *wm831x_power = container_of(nb,
>> +                                                      struct wm831x_power,
>> +                                                      usb_notify);
>> +     unsigned int i, best;
>> +
>> +     /* Find the highest supported limit */
>> +     best = 0;
>> +     for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
>> +             if (limit >= wm831x_usb_limits[i] &&
>> +                 wm831x_usb_limits[best] < wm831x_usb_limits[i])
>> +                     best = i;
>> +     }
>> +
>> +     dev_dbg(wm831x_power->wm831x->dev,
>> +             "Limiting USB current to %umA", wm831x_usb_limits[best]);
>> +
>> +     wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
>> +                     WM831X_USB_ILIM_MASK, best);
>> +
>> +     return 0;
>> +}
>> +
>>  /*********************************************************************
>>   *           Battery properties
>>   *********************************************************************/
>> @@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device *pdev)
>>               }
>>       }
>>
>> +     power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
>> +                                                  "usb-phy", 0);
>> +     if (!IS_ERR(power->usb_phy)) {
>> +             power->usb_notify.notifier_call = wm831x_usb_limit_change;
>> +             ret = usb_register_notifier(power->usb_phy,
>> +                                         &power->usb_notify);
>> +             if (ret) {
>> +                     dev_err(&pdev->dev, "Failed to register notifier: %d\n",
>> +                             ret);
>> +                     goto err_bat_irq;
>> +             }
>> +     }
>
> No error handling for power->usb_phy? I think you should bail out
> for all errors except for "not defined in DT". Especially I would
> expect probe defer handling in case the power supply driver is
> loaded before the phy driver.

Make sense. So I think I need to change like below:

power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
if (!IS_ERR(power->usb_phy)) {
        power->usb_notify.notifier_call = wm831x_usb_limit_change;
        ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
        if (ret) {
                dev_err(&pdev->dev, "Failed to register notifier: %d\n", ret);
                goto err_bat_irq;
        }
} else if (PTR_ERR(power->usb_phy) != -ENODEV &&
PTR_ERR(power->usb_phy) != -EINVAL) {
       ret = PTR_ERR(power->usb_phy);
       dev_err(&pdev->dev, "Failed to find USB phy: %d\n", ret);
       goto err_bat_irq;
}

The return value -EINVAL means the platform device does not have a
device node and -ENODEV  means we did not set the 'usb-phy' phandle,
other errors we should bail out including -PROBE_DEFER. Is it OK for
you?

>
>>       return ret;
>>
>>  err_bat_irq:
>> @@ -637,6 +690,11 @@ static int wm831x_power_remove(struct platform_device *pdev)
>>       struct wm831x *wm831x = wm831x_power->wm831x;
>>       int irq, i;
>>
>> +     if (!IS_ERR(wm831x_power->usb_phy)) {
>> +             usb_unregister_notifier(wm831x_power->usb_phy,
>> +                                     &wm831x_power->usb_notify);
>> +     }
>> +
>>       for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
>>               irq = wm831x_irq(wm831x,
>>                                platform_get_irq_byname(pdev,
>
> -- Sebastian



-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Sebastian Reichel]

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

Hi,

On Wed, Jul 26, 2017 at 11:05:25AM +0800, Baolin Wang wrote:
> On 25 July 2017 at 17:59, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
> > On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
> >> Integrate with the newly added USB charger interface to limit the current
> >> we draw from the USB input based on the input device configuration
> >> identified by the USB stack, allowing us to charge more quickly from high
> >> current inputs without drawing more current than specified from others.
> >>
> >> Signed-off-by: Mark Brown <broonie@kernel.org>
> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +
> >>  drivers/power/supply/wm831x_power.c              |   58 ++++++++++++++++++++++
> >>  2 files changed, 59 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt b/Documentation/devicetree/bindings/mfd/wm831x.txt
> >> index 9f8b743..4e3bc07 100644
> >> --- a/Documentation/devicetree/bindings/mfd/wm831x.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
> >> @@ -31,6 +31,7 @@ Required properties:
> >>      ../interrupt-controller/interrupts.txt
> >>
> >>  Optional sub-nodes:
> >> +  - usb-phy : Contains a phandle to the USB PHY.
> >>    - regulators : Contains sub-nodes for each of the regulators supplied by
> >>      the device. The regulators are bound using their names listed below:
> >>
> >> diff --git a/drivers/power/supply/wm831x_power.c b/drivers/power/supply/wm831x_power.c
> >> index 7082301..d3948ab 100644
> >> --- a/drivers/power/supply/wm831x_power.c
> >> +++ b/drivers/power/supply/wm831x_power.c
> >> @@ -13,6 +13,7 @@
> >>  #include <linux/platform_device.h>
> >>  #include <linux/power_supply.h>
> >>  #include <linux/slab.h>
> >> +#include <linux/usb/phy.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_phy *usb_phy;
> >> +     struct notifier_block usb_notify;
> >>  };
> >>
> >>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> >> @@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
> >>       POWER_SUPPLY_PROP_VOLTAGE_NOW,
> >>  };
> >>
> >> +/* In milliamps */
> >> +static const unsigned int wm831x_usb_limits[] = {
> >> +     0,
> >> +     2,
> >> +     100,
> >> +     500,
> >> +     900,
> >> +     1500,
> >> +     1800,
> >> +     550,
> >> +};
> >> +
> >> +static int wm831x_usb_limit_change(struct notifier_block *nb,
> >> +                                unsigned long limit, void *data)
> >> +{
> >> +     struct wm831x_power *wm831x_power = container_of(nb,
> >> +                                                      struct wm831x_power,
> >> +                                                      usb_notify);
> >> +     unsigned int i, best;
> >> +
> >> +     /* Find the highest supported limit */
> >> +     best = 0;
> >> +     for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
> >> +             if (limit >= wm831x_usb_limits[i] &&
> >> +                 wm831x_usb_limits[best] < wm831x_usb_limits[i])
> >> +                     best = i;
> >> +     }
> >> +
> >> +     dev_dbg(wm831x_power->wm831x->dev,
> >> +             "Limiting USB current to %umA", wm831x_usb_limits[best]);
> >> +
> >> +     wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
> >> +                     WM831X_USB_ILIM_MASK, best);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >>  /*********************************************************************
> >>   *           Battery properties
> >>   *********************************************************************/
> >> @@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device *pdev)
> >>               }
> >>       }
> >>
> >> +     power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
> >> +                                                  "usb-phy", 0);
> >> +     if (!IS_ERR(power->usb_phy)) {
> >> +             power->usb_notify.notifier_call = wm831x_usb_limit_change;
> >> +             ret = usb_register_notifier(power->usb_phy,
> >> +                                         &power->usb_notify);
> >> +             if (ret) {
> >> +                     dev_err(&pdev->dev, "Failed to register notifier: %d\n",
> >> +                             ret);
> >> +                     goto err_bat_irq;
> >> +             }
> >> +     }
> >
> > No error handling for power->usb_phy? I think you should bail out
> > for all errors except for "not defined in DT". Especially I would
> > expect probe defer handling in case the power supply driver is
> > loaded before the phy driver.
> 
> Make sense. So I think I need to change like below:
> 
> power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> if (!IS_ERR(power->usb_phy)) {
>         power->usb_notify.notifier_call = wm831x_usb_limit_change;
>         ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
>         if (ret) {
>                 dev_err(&pdev->dev, "Failed to register notifier: %d\n", ret);
>                 goto err_bat_irq;
>         }
> } else if (PTR_ERR(power->usb_phy) != -ENODEV &&
> PTR_ERR(power->usb_phy) != -EINVAL) {
>        ret = PTR_ERR(power->usb_phy);
>        dev_err(&pdev->dev, "Failed to find USB phy: %d\n", ret);
>        goto err_bat_irq;
> }
> 
> The return value -EINVAL means the platform device does not have a
> device node and -ENODEV  means we did not set the 'usb-phy' phandle,
> other errors we should bail out including -PROBE_DEFER. Is it OK for
> you?

Yes, but I think the following is better, which avoids
spamming the log with probe defer messages. 

power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
ret = PTR_ERR_OR_ZERO(power->usb_phy);

switch (ret) {
case 0:
    power->usb_notify.notifier_call = wm831x_usb_limit_change;
    ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
    if (ret) {
        dev_err(&pdev->dev, "Failed to register notifier: %d\n", ret);
        goto err_bat_irq;
    }
    break;
case -EINVAL:
case -ENODEV:
    /* ignore missing usb-phy, it's optional */
    power->usb_phy = NULL;
    break;
default:
    dev_err(&pdev->dev, "Failed to find USB phy: %d\n", ret);
    /* fall-through */
case -EPROBE_DEFER:
    goto err_bat_irq;
    break;
}

-- Sebastian

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

Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Hi,

On 26 July 2017 at 20:08, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> Hi,
>
> On Wed, Jul 26, 2017 at 11:05:25AM +0800, Baolin Wang wrote:
>> On 25 July 2017 at 17:59, Sebastian Reichel
>> <sebastian.reichel@collabora.co.uk> wrote:
>> > On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
>> >> Integrate with the newly added USB charger interface to limit the current
>> >> we draw from the USB input based on the input device configuration
>> >> identified by the USB stack, allowing us to charge more quickly from high
>> >> current inputs without drawing more current than specified from others.
>> >>
>> >> Signed-off-by: Mark Brown <broonie@kernel.org>
>> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> >> ---
>> >>  Documentation/devicetree/bindings/mfd/wm831x.txt |    1 +
>> >>  drivers/power/supply/wm831x_power.c              |   58 ++++++++++++++++++++++
>> >>  2 files changed, 59 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt b/Documentation/devicetree/bindings/mfd/wm831x.txt
>> >> index 9f8b743..4e3bc07 100644
>> >> --- a/Documentation/devicetree/bindings/mfd/wm831x.txt
>> >> +++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
>> >> @@ -31,6 +31,7 @@ Required properties:
>> >>      ../interrupt-controller/interrupts.txt
>> >>
>> >>  Optional sub-nodes:
>> >> +  - usb-phy : Contains a phandle to the USB PHY.
>> >>    - regulators : Contains sub-nodes for each of the regulators supplied by
>> >>      the device. The regulators are bound using their names listed below:
>> >>
>> >> diff --git a/drivers/power/supply/wm831x_power.c b/drivers/power/supply/wm831x_power.c
>> >> index 7082301..d3948ab 100644
>> >> --- a/drivers/power/supply/wm831x_power.c
>> >> +++ b/drivers/power/supply/wm831x_power.c
>> >> @@ -13,6 +13,7 @@
>> >>  #include <linux/platform_device.h>
>> >>  #include <linux/power_supply.h>
>> >>  #include <linux/slab.h>
>> >> +#include <linux/usb/phy.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_phy *usb_phy;
>> >> +     struct notifier_block usb_notify;
>> >>  };
>> >>
>> >>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
>> >> @@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
>> >>       POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> >>  };
>> >>
>> >> +/* In milliamps */
>> >> +static const unsigned int wm831x_usb_limits[] = {
>> >> +     0,
>> >> +     2,
>> >> +     100,
>> >> +     500,
>> >> +     900,
>> >> +     1500,
>> >> +     1800,
>> >> +     550,
>> >> +};
>> >> +
>> >> +static int wm831x_usb_limit_change(struct notifier_block *nb,
>> >> +                                unsigned long limit, void *data)
>> >> +{
>> >> +     struct wm831x_power *wm831x_power = container_of(nb,
>> >> +                                                      struct wm831x_power,
>> >> +                                                      usb_notify);
>> >> +     unsigned int i, best;
>> >> +
>> >> +     /* Find the highest supported limit */
>> >> +     best = 0;
>> >> +     for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
>> >> +             if (limit >= wm831x_usb_limits[i] &&
>> >> +                 wm831x_usb_limits[best] < wm831x_usb_limits[i])
>> >> +                     best = i;
>> >> +     }
>> >> +
>> >> +     dev_dbg(wm831x_power->wm831x->dev,
>> >> +             "Limiting USB current to %umA", wm831x_usb_limits[best]);
>> >> +
>> >> +     wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
>> >> +                     WM831X_USB_ILIM_MASK, best);
>> >> +
>> >> +     return 0;
>> >> +}
>> >> +
>> >>  /*********************************************************************
>> >>   *           Battery properties
>> >>   *********************************************************************/
>> >> @@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device *pdev)
>> >>               }
>> >>       }
>> >>
>> >> +     power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
>> >> +                                                  "usb-phy", 0);
>> >> +     if (!IS_ERR(power->usb_phy)) {
>> >> +             power->usb_notify.notifier_call = wm831x_usb_limit_change;
>> >> +             ret = usb_register_notifier(power->usb_phy,
>> >> +                                         &power->usb_notify);
>> >> +             if (ret) {
>> >> +                     dev_err(&pdev->dev, "Failed to register notifier: %d\n",
>> >> +                             ret);
>> >> +                     goto err_bat_irq;
>> >> +             }
>> >> +     }
>> >
>> > No error handling for power->usb_phy? I think you should bail out
>> > for all errors except for "not defined in DT". Especially I would
>> > expect probe defer handling in case the power supply driver is
>> > loaded before the phy driver.
>>
>> Make sense. So I think I need to change like below:
>>
>> power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
>> if (!IS_ERR(power->usb_phy)) {
>>         power->usb_notify.notifier_call = wm831x_usb_limit_change;
>>         ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
>>         if (ret) {
>>                 dev_err(&pdev->dev, "Failed to register notifier: %d\n", ret);
>>                 goto err_bat_irq;
>>         }
>> } else if (PTR_ERR(power->usb_phy) != -ENODEV &&
>> PTR_ERR(power->usb_phy) != -EINVAL) {
>>        ret = PTR_ERR(power->usb_phy);
>>        dev_err(&pdev->dev, "Failed to find USB phy: %d\n", ret);
>>        goto err_bat_irq;
>> }
>>
>> The return value -EINVAL means the platform device does not have a
>> device node and -ENODEV  means we did not set the 'usb-phy' phandle,
>> other errors we should bail out including -PROBE_DEFER. Is it OK for
>> you?
>
> Yes, but I think the following is better, which avoids
> spamming the log with probe defer messages.
>
> power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> ret = PTR_ERR_OR_ZERO(power->usb_phy);
>
> switch (ret) {
> case 0:
>     power->usb_notify.notifier_call = wm831x_usb_limit_change;
>     ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
>     if (ret) {
>         dev_err(&pdev->dev, "Failed to register notifier: %d\n", ret);
>         goto err_bat_irq;
>     }
>     break;
> case -EINVAL:
> case -ENODEV:
>     /* ignore missing usb-phy, it's optional */
>     power->usb_phy = NULL;
>     break;
> default:
>     dev_err(&pdev->dev, "Failed to find USB phy: %d\n", ret);
>     /* fall-through */
> case -EPROBE_DEFER:
>     goto err_bat_irq;
>     break;
> }

OK. Thanks.

-- 
Baolin.wang
Best Regards