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

2015-08-07 Thread Peter Chen
On Thu, Aug 06, 2015 at 03:03:48PM +0800, Baolin Wang wrote:
 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.w...@linaro.org
 ---
  drivers/usb/gadget/charger.c|  547 
 +++
  include/linux/usb/usb_charger.h |  101 
  2 files changed, 648 insertions(+)
  create mode 100644 drivers/usb/gadget/charger.c
  create mode 100644 include/linux/usb/usb_charger.h
 
 diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
 new file mode 100644
 index 000..3ca0180
 --- /dev/null
 +++ b/drivers/usb/gadget/charger.c
 @@ -0,0 +1,547 @@
 +/*
 + * usb charger.c -- USB charger driver
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +
 +#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 LIST_HEAD(usb_charger_list);
 +static DEFINE_MUTEX(usb_charger_list_lock);
 +
 +/*
 + * usb_charger_find_by_name - Get the usb charger device by name.
 + * @name - usb charger device name.
 + *
 + * notes: when this function walks the list and returns a charger
 + * it's dropped the lock which means that something else could come
 + * along and delete the charger before we dereference the pointer.
 + * It's very unlikely but it's a possibility so you should take care
 + * of it.
 + * Thus when you get the usb charger by name, you should call
 + * put_usb_charger() to derease the reference count of the usb charger.
 + *
 + * return the instance of usb charger device.
 + */
 +struct usb_charger *usb_charger_find_by_name(char *name)
 +{
 + struct usb_charger *uchger;
 +
 + if (!name)
 + return ERR_PTR(-EINVAL);
 +
 + mutex_lock(usb_charger_list_lock);
 + list_for_each_entry(uchger, usb_charger_list, entry) {
 + if (!strcmp(uchger-name, name)) {
 + get_usb_charger(uchger);
 + mutex_unlock(usb_charger_list_lock);
 + return uchger;
 + }
 + }
 + mutex_unlock(usb_charger_list_lock);
 +
 + return NULL;
 +}
 +
 +/*
 + * usb_charger_register_notify() - Register a notifiee to get notified by
 + *   any attach status changes from the usb charger type detection.
 + * @uchger - the usb charger device which is monitored.
 + * @nb - a notifier block to be registered.
 + */
 +void usb_charger_register_notify(struct usb_charger *uchger,
 +  struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_register(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * 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.
 + */
 +void usb_charger_unregister_notify(struct usb_charger *uchger,
 +struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_unregister(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * usb_charger_register_extcon_notifier() - Register a notifiee of the usb
 + *   charger to get notified by any attach status changes from
 + *   

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

2015-08-07 Thread Baolin Wang
On 7 August 2015 at 13:41, Peter Chen peter.c...@freescale.com wrote:
 On Thu, Aug 06, 2015 at 03:03:48PM +0800, Baolin Wang wrote:
 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.w...@linaro.org
 ---
  drivers/usb/gadget/charger.c|  547 
 +++
  include/linux/usb/usb_charger.h |  101 
  2 files changed, 648 insertions(+)
  create mode 100644 drivers/usb/gadget/charger.c
  create mode 100644 include/linux/usb/usb_charger.h

 diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
 new file mode 100644
 index 000..3ca0180
 --- /dev/null
 +++ b/drivers/usb/gadget/charger.c
 @@ -0,0 +1,547 @@
 +/*
 + * usb charger.c -- USB charger driver
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +
 +#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 LIST_HEAD(usb_charger_list);
 +static DEFINE_MUTEX(usb_charger_list_lock);
 +
 +/*
 + * usb_charger_find_by_name - Get the usb charger device by name.
 + * @name - usb charger device name.
 + *
 + * notes: when this function walks the list and returns a charger
 + * it's dropped the lock which means that something else could come
 + * along and delete the charger before we dereference the pointer.
 + * It's very unlikely but it's a possibility so you should take care
 + * of it.
 + * Thus when you get the usb charger by name, you should call
 + * put_usb_charger() to derease the reference count of the usb charger.
 + *
 + * return the instance of usb charger device.
 + */
 +struct usb_charger *usb_charger_find_by_name(char *name)
 +{
 + struct usb_charger *uchger;
 +
 + if (!name)
 + return ERR_PTR(-EINVAL);
 +
 + mutex_lock(usb_charger_list_lock);
 + list_for_each_entry(uchger, usb_charger_list, entry) {
 + if (!strcmp(uchger-name, name)) {
 + get_usb_charger(uchger);
 + mutex_unlock(usb_charger_list_lock);
 + return uchger;
 + }
 + }
 + mutex_unlock(usb_charger_list_lock);
 +
 + return NULL;
 +}
 +
 +/*
 + * usb_charger_register_notify() - Register a notifiee to get notified by
 + *   any attach status changes from the usb charger type detection.
 + * @uchger - the usb charger device which is monitored.
 + * @nb - a notifier block to be registered.
 + */
 +void usb_charger_register_notify(struct usb_charger *uchger,
 +  struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_register(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * 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.
 + */
 +void usb_charger_unregister_notify(struct usb_charger *uchger,
 +struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_unregister(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * usb_charger_register_extcon_notifier() - Register a notifiee of the usb
 + *   charger 

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

2015-08-06 Thread Greg KH
On Thu, Aug 06, 2015 at 03:03:48PM +0800, Baolin Wang wrote:
 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.w...@linaro.org
 ---
  drivers/usb/gadget/charger.c|  547 
 +++
  include/linux/usb/usb_charger.h |  101 
  2 files changed, 648 insertions(+)
  create mode 100644 drivers/usb/gadget/charger.c
  create mode 100644 include/linux/usb/usb_charger.h
 
 diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
 new file mode 100644
 index 000..3ca0180
 --- /dev/null
 +++ b/drivers/usb/gadget/charger.c
 @@ -0,0 +1,547 @@
 +/*
 + * usb charger.c -- USB charger driver
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.

I have to ask, do you really mean any later version?

 + */
 +
 +#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 LIST_HEAD(usb_charger_list);
 +static DEFINE_MUTEX(usb_charger_list_lock);
 +
 +/*
 + * usb_charger_find_by_name - Get the usb charger device by name.
 + * @name - usb charger device name.
 + *
 + * notes: when this function walks the list and returns a charger
 + * it's dropped the lock which means that something else could come
 + * along and delete the charger before we dereference the pointer.
 + * It's very unlikely but it's a possibility so you should take care
 + * of it.
 + * Thus when you get the usb charger by name, you should call
 + * put_usb_charger() to derease the reference count of the usb charger.
 + *
 + * return the instance of usb charger device.
 + */
 +struct usb_charger *usb_charger_find_by_name(char *name)
 +{
 + struct usb_charger *uchger;
 +
 + if (!name)
 + return ERR_PTR(-EINVAL);
 +
 + mutex_lock(usb_charger_list_lock);
 + list_for_each_entry(uchger, usb_charger_list, entry) {
 + if (!strcmp(uchger-name, name)) {
 + get_usb_charger(uchger);
 + mutex_unlock(usb_charger_list_lock);
 + return uchger;
 + }
 + }
 + mutex_unlock(usb_charger_list_lock);
 +
 + return NULL;
 +}
 +
 +/*
 + * usb_charger_register_notify() - Register a notifiee to get notified by
 + *   any attach status changes from the usb charger type detection.
 + * @uchger - the usb charger device which is monitored.
 + * @nb - a notifier block to be registered.
 + */
 +void usb_charger_register_notify(struct usb_charger *uchger,
 +  struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_register(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * 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.
 + */
 +void usb_charger_unregister_notify(struct usb_charger *uchger,
 +struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_unregister(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * usb_charger_register_extcon_notifier() - Register a notifiee of the usb
 + *   charger to get 

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

2015-08-06 Thread 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.w...@linaro.org
---
 drivers/usb/gadget/charger.c|  547 +++
 include/linux/usb/usb_charger.h |  101 
 2 files changed, 648 insertions(+)
 create mode 100644 drivers/usb/gadget/charger.c
 create mode 100644 include/linux/usb/usb_charger.h

diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
new file mode 100644
index 000..3ca0180
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,547 @@
+/*
+ * usb charger.c -- USB charger driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#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 LIST_HEAD(usb_charger_list);
+static DEFINE_MUTEX(usb_charger_list_lock);
+
+/*
+ * usb_charger_find_by_name - Get the usb charger device by name.
+ * @name - usb charger device name.
+ *
+ * notes: when this function walks the list and returns a charger
+ * it's dropped the lock which means that something else could come
+ * along and delete the charger before we dereference the pointer.
+ * It's very unlikely but it's a possibility so you should take care
+ * of it.
+ * Thus when you get the usb charger by name, you should call
+ * put_usb_charger() to derease the reference count of the usb charger.
+ *
+ * return the instance of usb charger device.
+ */
+struct usb_charger *usb_charger_find_by_name(char *name)
+{
+   struct usb_charger *uchger;
+
+   if (!name)
+   return ERR_PTR(-EINVAL);
+
+   mutex_lock(usb_charger_list_lock);
+   list_for_each_entry(uchger, usb_charger_list, entry) {
+   if (!strcmp(uchger-name, name)) {
+   get_usb_charger(uchger);
+   mutex_unlock(usb_charger_list_lock);
+   return uchger;
+   }
+   }
+   mutex_unlock(usb_charger_list_lock);
+
+   return NULL;
+}
+
+/*
+ * usb_charger_register_notify() - Register a notifiee to get notified by
+ * any attach status changes from the usb charger type detection.
+ * @uchger - the usb charger device which is monitored.
+ * @nb - a notifier block to be registered.
+ */
+void usb_charger_register_notify(struct usb_charger *uchger,
+struct notifier_block *nb)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(uchger-lock, flags);
+   raw_notifier_chain_register(uchger-uchger_nh, nb);
+   spin_unlock_irqrestore(uchger-lock, flags);
+}
+
+/*
+ * 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.
+ */
+void usb_charger_unregister_notify(struct usb_charger *uchger,
+  struct notifier_block *nb)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(uchger-lock, flags);
+   raw_notifier_chain_unregister(uchger-uchger_nh, nb);
+   spin_unlock_irqrestore(uchger-lock, flags);
+}
+
+/*
+ * usb_charger_register_extcon_notifier() - Register a notifiee of the usb
+ * charger to get notified by any attach status changes from
+ * the extcon device.
+ * @uchger - the usb charger device.
+ * @edev - the extcon device.
+ * @extcon_id - extcon id.
+ */
+int usb_charger_register_extcon_notifier(struct 

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

2015-08-06 Thread Baolin Wang
On 7 August 2015 at 00:39, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, Aug 06, 2015 at 03:03:48PM +0800, Baolin Wang wrote:
 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.w...@linaro.org
 ---
  drivers/usb/gadget/charger.c|  547 
 +++
  include/linux/usb/usb_charger.h |  101 
  2 files changed, 648 insertions(+)
  create mode 100644 drivers/usb/gadget/charger.c
  create mode 100644 include/linux/usb/usb_charger.h

 diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
 new file mode 100644
 index 000..3ca0180
 --- /dev/null
 +++ b/drivers/usb/gadget/charger.c
 @@ -0,0 +1,547 @@
 +/*
 + * usb charger.c -- USB charger driver
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.

 I have to ask, do you really mean any later version?


I'll think about this and modify it.

 + */
 +
 +#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 LIST_HEAD(usb_charger_list);
 +static DEFINE_MUTEX(usb_charger_list_lock);
 +
 +/*
 + * usb_charger_find_by_name - Get the usb charger device by name.
 + * @name - usb charger device name.
 + *
 + * notes: when this function walks the list and returns a charger
 + * it's dropped the lock which means that something else could come
 + * along and delete the charger before we dereference the pointer.
 + * It's very unlikely but it's a possibility so you should take care
 + * of it.
 + * Thus when you get the usb charger by name, you should call
 + * put_usb_charger() to derease the reference count of the usb charger.
 + *
 + * return the instance of usb charger device.
 + */
 +struct usb_charger *usb_charger_find_by_name(char *name)
 +{
 + struct usb_charger *uchger;
 +
 + if (!name)
 + return ERR_PTR(-EINVAL);
 +
 + mutex_lock(usb_charger_list_lock);
 + list_for_each_entry(uchger, usb_charger_list, entry) {
 + if (!strcmp(uchger-name, name)) {
 + get_usb_charger(uchger);
 + mutex_unlock(usb_charger_list_lock);
 + return uchger;
 + }
 + }
 + mutex_unlock(usb_charger_list_lock);
 +
 + return NULL;
 +}
 +
 +/*
 + * usb_charger_register_notify() - Register a notifiee to get notified by
 + *   any attach status changes from the usb charger type detection.
 + * @uchger - the usb charger device which is monitored.
 + * @nb - a notifier block to be registered.
 + */
 +void usb_charger_register_notify(struct usb_charger *uchger,
 +  struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_register(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * 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.
 + */
 +void usb_charger_unregister_notify(struct usb_charger *uchger,
 +struct notifier_block *nb)
 +{
 + unsigned long flags;
 +
 + spin_lock_irqsave(uchger-lock, flags);
 + raw_notifier_chain_unregister(uchger-uchger_nh, nb);
 + spin_unlock_irqrestore(uchger-lock, flags);
 +}
 +
 +/*
 + * 

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

2015-08-06 Thread Mark Brown
On Thu, Aug 06, 2015 at 09:39:05AM -0700, Greg KH wrote:
 On Thu, Aug 06, 2015 at 03:03:48PM +0800, Baolin Wang wrote:

  +static void usb_charger_release(struct device *dev)
  +{
  +   struct usb_charger *uchger = dev_get_drvdata(dev);

  +   if (!atomic_dec_and_test(uchger-count)) {
  +   dev_err(dev, The usb charger is still in use\n);

 Why is the count different from the reference count?  You shouldn't be
 in this function if the reference count is not 0, so tie your user
 count to this one.  Having two different reference counts is a nightmare
 and almost impossible to get right.  And a huge red flag that the design
 is incorrect.

  +   return;

 You can't fail a release call, so you just leaked memory all over the
 floor here :(

Indeed.  I did discuss this with Baolin off list but I'd missed the
dynamic allocation of devices for some reason.

  +   mutex_lock(usb_charger_list_lock);
  +   list_for_each_entry(tmp, usb_charger_list, entry) {
  +   if (!(strcmp(tmp-name, uchger-name))) {
  +   mutex_unlock(usb_charger_list_lock);
  +   ret = -EEXIST;
  +   goto out;
  +   }
  +   }
  +   list_add_tail(uchger-entry, usb_charger_list);

 Why do you need a separate list?  This subsystem's bus structure should
 own that list of devices, no need for a separate one (again, a huge red
 flag that the design is not correct.)

Right, if we dynamically allocate a device per charger then the lifetime
issues should go away and we get a list for free.


signature.asc
Description: Digital signature