Re: [PATCH v5 1/5] gadget: Introduce the notifier functions

2015-11-08 Thread Baolin Wang
On 7 November 2015 at 00:56, Greg KH  wrote:
> On Fri, Nov 06, 2015 at 07:35:10PM +0800, Baolin Wang wrote:

>>  #ifdef   CONFIG_HAS_DMA
>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> index c14a69b..755e8bc 100644
>> --- a/include/linux/usb/gadget.h
>> +++ b/include/linux/usb/gadget.h
>> @@ -609,6 +609,8 @@ struct usb_gadget {
>>   unsignedout_epnum;
>>   unsignedin_epnum;
>>   struct usb_otg_caps *otg_caps;
>> + struct raw_notifier_headnh;
>> + struct mutexlock;
>
> You have to document what this lock protects.

OK.

>
>
>>
>>   unsignedsg_supported:1;
>>   unsignedis_otg:1;
>> @@ -1183,6 +1185,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
>> + */
>
> kerneldoc does not belong in a .h file.
>

I'll remove the comments.

> And the kbuild system found lots of problems with this series, please
> fix those at the very least :(

I'm sorry for that, I'll check the patches again. Thanks for your comments.

>
> thanks,
>
> greg k-h



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


Re: [PATCH v5 1/5] gadget: Introduce the notifier functions

2015-11-06 Thread Greg KH
On Fri, Nov 06, 2015 at 07:35:10PM +0800, Baolin Wang wrote:
> The usb charger framework is based on usb gadget. The usb charger
> need to be notified the state changing of usb gadget to confirm the
> usb charger state.
> 
> Thus this patch adds a notifier mechanism for usb gadget to report a
> event to usb charger when the usb gadget state is changed.

I thought we said we did not want another notifier chain in the previous
versions of this patch?

> 
> Signed-off-by: Baolin Wang 
> ---
>  drivers/usb/gadget/udc/udc-core.c |   32 
>  include/linux/usb/gadget.h|   18 ++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/udc-core.c 
> b/drivers/usb/gadget/udc/udc-core.c
> index f660afb..4238fc3 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -129,6 +129,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(>lock);
> + ret = raw_notifier_chain_register(>nh, nb);
> + mutex_unlock(>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(>lock);
> + ret = raw_notifier_chain_unregister(>nh, nb);
> + mutex_unlock(>lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_unregister_notify);
> +
>  /* - 
> */
>  
>  /**
> @@ -226,6 +252,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(>lock);
> + raw_notifier_call_chain(>nh, gadget->state, gadget);
> + mutex_unlock(>lock);
> +
>   if (udc)
>   sysfs_notify(>dev.kobj, NULL, "state");
>  }
> @@ -364,6 +394,8 @@ int usb_add_gadget_udc_release(struct device *parent, 
> struct usb_gadget *gadget,
>  
>   dev_set_name(>dev, "gadget");
>   INIT_WORK(>work, usb_gadget_state_work);
> + RAW_INIT_NOTIFIER_HEAD(>nh);
> + mutex_init(>lock);
>   gadget->dev.parent = parent;
>  
>  #ifdef   CONFIG_HAS_DMA
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index c14a69b..755e8bc 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -609,6 +609,8 @@ struct usb_gadget {
>   unsignedout_epnum;
>   unsignedin_epnum;
>   struct usb_otg_caps *otg_caps;
> + struct raw_notifier_headnh;
> + struct mutexlock;

You have to document what this lock protects.


>  
>   unsignedsg_supported:1;
>   unsignedis_otg:1;
> @@ -1183,6 +1185,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
> + */

kerneldoc does not belong in a .h file.

And the kbuild system found lots of problems with this series, please
fix those at the very least :(

thanks,

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


Re: [PATCH v5 1/5] gadget: Introduce the notifier functions

2015-11-06 Thread kbuild test robot
Hi Baolin,

[auto build test WARNING on v4.3-rc7]
[also build test WARNING on next-20151106]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-usb-charger-framework-to-deal-with-the-usb-gadget-power-negotation/20151106-194008
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/usb/gadget.h:226: warning: No description found for parameter 
'claimed'
>> include/linux/usb/gadget.h:628: warning: No description found for parameter 
>> 'nh'
>> include/linux/usb/gadget.h:628: warning: No description found for parameter 
>> 'lock'
   include/linux/usb/gadget.h:628: warning: No description found for parameter 
'quirk_altset_not_supp'
   include/linux/usb/gadget.h:628: warning: No description found for parameter 
'quirk_stall_not_supp'
   include/linux/usb/gadget.h:628: warning: No description found for parameter 
'quirk_zlp_not_supp'
>> include/linux/usb/gadget.h:1193: warning: No description found for parameter 
>> 'gadget'
>> include/linux/usb/gadget.h:1193: warning: No description found for parameter 
>> 'nb'
   include/linux/usb/composite.h:501: warning: Excess struct/union/enum/typedef 
member 'setup_pending' description in 'usb_composite_dev'
   include/linux/usb/composite.h:501: warning: Excess struct/union/enum/typedef 
member 'os_desc_pending' description in 'usb_composite_dev'
   drivers/usb/gadget/function/f_acm.c:1: warning: no structured comments found
   drivers/usb/gadget/function/f_ecm.c:1: warning: no structured comments found
   drivers/usb/gadget/function/f_subset.c:1: warning: no structured comments 
found
   drivers/usb/gadget/function/f_obex.c:1: warning: no structured comments found
   drivers/usb/gadget/function/f_serial.c:1: warning: no structured comments 
found

vim +/nh +628 include/linux/usb/gadget.h

a64cbb7e92 include/linux/usb/gadget.h Baolin Wang 2015-11-06  612   struct 
raw_notifier_headnh;
a64cbb7e92 include/linux/usb/gadget.h Baolin Wang 2015-11-06  613   struct 
mutexlock;
d8318d7f6b include/linux/usb/gadget.h David Cohen 2013-12-09  614  
898c608678 include/linux/usb/gadget.h Felipe Balbi2011-11-22  615   
unsignedsg_supported:1;
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  616   
unsignedis_otg:1;
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  617   
unsignedis_a_peripheral:1;
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  618   
unsignedb_hnp_enable:1;
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  619   
unsigneda_hnp_support:1;
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  620   
unsigneda_alt_hnp_support:1;
0b2d2bbade include/linux/usb/gadget.h David Cohen 2013-12-09  621   
unsignedquirk_ep_out_aligned_size:1;
ffd9a0fcbb include/linux/usb/gadget.h Robert Baldyga  2015-07-28  622   
unsignedquirk_altset_not_supp:1;
02ded1b0d8 include/linux/usb/gadget.h Robert Baldyga  2015-07-28  623   
unsignedquirk_stall_not_supp:1;
ca1023c81d include/linux/usb/gadget.h Robert Baldyga  2015-07-28  624   
unsignedquirk_zlp_not_supp:1;
80b2502cea include/linux/usb/gadget.h Peter Chen  2015-01-28  625   
unsignedis_selfpowered:1;
ccdf138fe3 include/linux/usb/gadget.h Robert Baldyga  2015-05-04  626   
unsigneddeactivated:1;
ccdf138fe3 include/linux/usb/gadget.h Robert Baldyga  2015-05-04  627   
unsignedconnected:1;
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16 @628  };
5702f75375 include/linux/usb/gadget.h Felipe Balbi2013-07-17  629  #define 
work_to_gadget(w)(container_of((w), struct usb_gadget, work))
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  630  
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  631  static 
inline void set_gadget_data(struct usb_gadget *gadget, void *data)
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  632   { 
dev_set_drvdata(>dev, data); }
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  633  static 
inline void *get_gadget_data(struct usb_gadget *gadget)
^1da177e4c include/linux/usb_gadget.h Linus Torvalds  2005-04-16  634   { 
return dev_get_drvdata(>dev); }
f48cf80f93 include/linux/usb/gadget.h Fabien Chouteau 2010-04-23  635  static 
inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
f48cf80f93 include/linux/usb/gadget.h Fabien Chouteau 2010-04-23  636  {

:: The code at line 628 was first introduced by commit
:: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:: TO: Linus Torvalds 
:: CC: Linus Torvalds 

---
0-DAY kernel test infrastructure

[PATCH v5 1/5] gadget: Introduce the notifier functions

2015-11-06 Thread Baolin Wang
The usb charger framework is based on usb gadget. The usb charger
need to be notified the state changing of usb gadget to confirm the
usb charger state.

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

Signed-off-by: Baolin Wang 
---
 drivers/usb/gadget/udc/udc-core.c |   32 
 include/linux/usb/gadget.h|   18 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index f660afb..4238fc3 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -129,6 +129,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(>lock);
+   ret = raw_notifier_chain_register(>nh, nb);
+   mutex_unlock(>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(>lock);
+   ret = raw_notifier_chain_unregister(>nh, nb);
+   mutex_unlock(>lock);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_unregister_notify);
+
 /* - */
 
 /**
@@ -226,6 +252,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(>lock);
+   raw_notifier_call_chain(>nh, gadget->state, gadget);
+   mutex_unlock(>lock);
+
if (udc)
sysfs_notify(>dev.kobj, NULL, "state");
 }
@@ -364,6 +394,8 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
 
dev_set_name(>dev, "gadget");
INIT_WORK(>work, usb_gadget_state_work);
+   RAW_INIT_NOTIFIER_HEAD(>nh);
+   mutex_init(>lock);
gadget->dev.parent = parent;
 
 #ifdef CONFIG_HAS_DMA
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c14a69b..755e8bc 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -609,6 +609,8 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
struct usb_otg_caps *otg_caps;
+   struct raw_notifier_headnh;
+   struct mutexlock;
 
unsignedsg_supported:1;
unsignedis_otg:1;
@@ -1183,6 +1185,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

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


Re: [PATCH v5 1/5] gadget: Introduce the notifier functions

2015-11-06 Thread Mark Brown
On Fri, Nov 06, 2015 at 08:56:44AM -0800, Greg KH wrote:
> On Fri, Nov 06, 2015 at 07:35:10PM +0800, Baolin Wang wrote:

> > Thus this patch adds a notifier mechanism for usb gadget to report a
> > event to usb charger when the usb gadget state is changed.

> I thought we said we did not want another notifier chain in the previous
> versions of this patch?

Did we come up with anything better?


signature.asc
Description: PGP signature