diff mbox series

[1/6] USB: UDC: Expand device model API interface

Message ID 20200807094151.13526-2-peter.chen@nxp.com (mailing list archive)
State Superseded
Headers show
Series USB: UDC: Fix memory leaks by expanding the API | expand

Commit Message

Peter Chen Aug. 7, 2020, 9:41 a.m. UTC 
From: Alan Stern <stern@rowland.harvard.edu>

The routines used by the UDC core to interface with the kernel's
device model, namely usb_add_gadget_udc(),
usb_add_gadget_udc_release(), and usb_del_gadget_udc(), provide access
to only a subset of the device model's full API.  They include
functionality equivalent to device_register() and device_unregister()
for gadgets, but they omit device_initialize(), device_add(),
device_del(), get_device(), and put_device().

This patch expands the UDC API by adding usb_initialize_gadget(),
usb_add_gadget(), usb_del_gadget(), usb_get_gadget(), and
usb_put_gadget() to fill in the gap.  It rewrites the existing
routines to call the new ones.

CC: Anton Vasilyev <vasilyev@ispras.ru>
CC: Evgeny Novikov <novikov@ispras.ru>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes for RFC
- %s/intialize/initialize
- Delete the net2272 and 2280 description at commit log

 drivers/usb/gadget/udc/core.c | 78 ++++++++++++++++++++++++++++-------
 include/linux/usb/gadget.h    | 27 +++++++++---
 2 files changed, 84 insertions(+), 21 deletions(-)

Comments

Greg Kroah-Hartman Aug. 7, 2020, 9:52 a.m. UTC | #1 
On Fri, Aug 07, 2020 at 05:41:46PM +0800, Peter Chen wrote:
> From: Alan Stern <stern@rowland.harvard.edu>
> 
> The routines used by the UDC core to interface with the kernel's
> device model, namely usb_add_gadget_udc(),
> usb_add_gadget_udc_release(), and usb_del_gadget_udc(), provide access
> to only a subset of the device model's full API.  They include
> functionality equivalent to device_register() and device_unregister()
> for gadgets, but they omit device_initialize(), device_add(),
> device_del(), get_device(), and put_device().
> 
> This patch expands the UDC API by adding usb_initialize_gadget(),
> usb_add_gadget(), usb_del_gadget(), usb_get_gadget(), and
> usb_put_gadget() to fill in the gap.  It rewrites the existing
> routines to call the new ones.
> 
> CC: Anton Vasilyev <vasilyev@ispras.ru>
> CC: Evgeny Novikov <novikov@ispras.ru>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

You can not forward on patches from someone else without also signing
off on them.

Please fix this series up to do so.

thanks,

greg k-h
Peter Chen Aug. 7, 2020, 10:21 a.m. UTC | #2 
> 
> On Fri, Aug 07, 2020 at 05:41:46PM +0800, Peter Chen wrote:
> > From: Alan Stern <stern@rowland.harvard.edu>
> >
> > The routines used by the UDC core to interface with the kernel's
> > device model, namely usb_add_gadget_udc(),
> > usb_add_gadget_udc_release(), and usb_del_gadget_udc(), provide access
> > to only a subset of the device model's full API.  They include
> > functionality equivalent to device_register() and device_unregister()
> > for gadgets, but they omit device_initialize(), device_add(),
> > device_del(), get_device(), and put_device().
> >
> > This patch expands the UDC API by adding usb_initialize_gadget(),
> > usb_add_gadget(), usb_del_gadget(), usb_get_gadget(), and
> > usb_put_gadget() to fill in the gap.  It rewrites the existing
> > routines to call the new ones.
> >
> > CC: Anton Vasilyev <vasilyev@ispras.ru>
> > CC: Evgeny Novikov <novikov@ispras.ru>
> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> 
> You can not forward on patches from someone else without also signing off on
> them.
> 
 
So, even without contribution for the patch, we also need to add signing off when
submit?

Peter
Greg Kroah-Hartman Aug. 7, 2020, 10:37 a.m. UTC | #3 
On Fri, Aug 07, 2020 at 10:21:53AM +0000, Peter Chen wrote:
>  
> > 
> > On Fri, Aug 07, 2020 at 05:41:46PM +0800, Peter Chen wrote:
> > > From: Alan Stern <stern@rowland.harvard.edu>
> > >
> > > The routines used by the UDC core to interface with the kernel's
> > > device model, namely usb_add_gadget_udc(),
> > > usb_add_gadget_udc_release(), and usb_del_gadget_udc(), provide access
> > > to only a subset of the device model's full API.  They include
> > > functionality equivalent to device_register() and device_unregister()
> > > for gadgets, but they omit device_initialize(), device_add(),
> > > device_del(), get_device(), and put_device().
> > >
> > > This patch expands the UDC API by adding usb_initialize_gadget(),
> > > usb_add_gadget(), usb_del_gadget(), usb_get_gadget(), and
> > > usb_put_gadget() to fill in the gap.  It rewrites the existing
> > > routines to call the new ones.
> > >
> > > CC: Anton Vasilyev <vasilyev@ispras.ru>
> > > CC: Evgeny Novikov <novikov@ispras.ru>
> > > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > 
> > You can not forward on patches from someone else without also signing off on
> > them.
> > 
>  
> So, even without contribution for the patch, we also need to add signing off when
> submit?

Please read the DCO, you are submitting some else's work here, it is
passing through you, you need to add your s-o-b to it.  It's only been
this way since the very beginning :)

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index ee226ad802a4..473e74088b1f 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1162,21 +1162,18 @@  static int check_pending_gadget_drivers(struct usb_udc *udc)
 }
 
 /**
- * usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
+ * usb_initialize_gadget - initialize a gadget and its embedded struct device
  * @parent: the parent device to this udc. Usually the controller driver's
  * device.
- * @gadget: the gadget to be added to the list.
+ * @gadget: the gadget to be initialized.
  * @release: a gadget release function.
  *
  * Returns zero on success, negative errno otherwise.
  * Calls the gadget release function in the latter case.
  */
-int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
+void usb_initialize_gadget(struct device *parent, struct usb_gadget *gadget,
 		void (*release)(struct device *dev))
 {
-	struct usb_udc		*udc;
-	int			ret = -ENOMEM;
-
 	dev_set_name(&gadget->dev, "gadget");
 	INIT_WORK(&gadget->work, usb_gadget_state_work);
 	gadget->dev.parent = parent;
@@ -1187,17 +1184,32 @@  int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
 		gadget->dev.release = usb_udc_nop_release;
 
 	device_initialize(&gadget->dev);
+}
+EXPORT_SYMBOL_GPL(usb_initialize_gadget);
+
+/**
+ * usb_add_gadget - adds a new gadget to the udc class driver list
+ * @gadget: the gadget to be added to the list.
+ *
+ * Returns zero on success, negative errno otherwise.
+ * Does not do a final usb_put_gadget() if an error occurs.
+ */
+int usb_add_gadget(struct usb_gadget *gadget)
+{
+	struct usb_udc		*udc;
+	int			ret = -ENOMEM;
 
 	udc = kzalloc(sizeof(*udc), GFP_KERNEL);
 	if (!udc)
-		goto err_put_gadget;
+		goto error;
 
 	device_initialize(&udc->dev);
 	udc->dev.release = usb_udc_release;
 	udc->dev.class = udc_class;
 	udc->dev.groups = usb_udc_attr_groups;
-	udc->dev.parent = parent;
-	ret = dev_set_name(&udc->dev, "%s", kobject_name(&parent->kobj));
+	udc->dev.parent = gadget->dev.parent;
+	ret = dev_set_name(&udc->dev, "%s",
+			kobject_name(&gadget->dev.parent->kobj));
 	if (ret)
 		goto err_put_udc;
 
@@ -1239,8 +1251,30 @@  int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
  err_put_udc:
 	put_device(&udc->dev);
 
- err_put_gadget:
-	put_device(&gadget->dev);
+ error:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_add_gadget);
+
+/**
+ * usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
+ * @parent: the parent device to this udc. Usually the controller driver's
+ * device.
+ * @gadget: the gadget to be added to the list.
+ * @release: a gadget release function.
+ *
+ * Returns zero on success, negative errno otherwise.
+ * Calls the gadget release function in the latter case.
+ */
+int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
+		void (*release)(struct device *dev))
+{
+	int	ret;
+
+	usb_initialize_gadget(parent, gadget, release);
+	ret = usb_add_gadget(gadget);
+	if (ret)
+		usb_put_gadget(gadget);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
@@ -1308,13 +1342,14 @@  static void usb_gadget_remove_driver(struct usb_udc *udc)
 }
 
 /**
- * usb_del_gadget_udc - deletes @udc from udc_list
+ * usb_del_gadget - deletes @udc from udc_list
  * @gadget: the gadget to be removed.
  *
- * This, will call usb_gadget_unregister_driver() if
+ * This will call usb_gadget_unregister_driver() if
  * the @udc is still busy.
+ * It will not do a final usb_put_gadget().
  */
-void usb_del_gadget_udc(struct usb_gadget *gadget)
+void usb_del_gadget(struct usb_gadget *gadget)
 {
 	struct usb_udc *udc = gadget->udc;
 
@@ -1337,7 +1372,20 @@  void usb_del_gadget_udc(struct usb_gadget *gadget)
 	kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
 	flush_work(&gadget->work);
 	device_unregister(&udc->dev);
-	device_unregister(&gadget->dev);
+	device_del(&gadget->dev);
+}
+EXPORT_SYMBOL_GPL(usb_del_gadget);
+
+/**
+ * usb_del_gadget_udc - deletes @udc from udc_list
+ * @gadget: the gadget to be removed.
+ *
+ * Calls usb_del_gadget() and does a final usb_put_gadget().
+ */
+void usb_del_gadget_udc(struct usb_gadget *gadget)
+{
+	usb_del_gadget(gadget);
+	usb_put_gadget(gadget);
 	memset(&gadget->dev, 0x00, sizeof(gadget->dev));
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 298b334e2951..791571f5191e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -436,6 +436,7 @@  struct usb_gadget {
 };
 #define work_to_gadget(w)	(container_of((w), struct usb_gadget, work))
 
+/* Interface to the device model */
 static inline void set_gadget_data(struct usb_gadget *gadget, void *data)
 	{ dev_set_drvdata(&gadget->dev, data); }
 static inline void *get_gadget_data(struct usb_gadget *gadget)
@@ -444,6 +445,26 @@  static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
 {
 	return container_of(dev, struct usb_gadget, dev);
 }
+static inline struct usb_gadget *usb_get_gadget(struct usb_gadget *gadget)
+{
+	get_device(&gadget->dev);
+	return gadget;
+}
+static inline void usb_put_gadget(struct usb_gadget *gadget)
+{
+	put_device(&gadget->dev);
+}
+extern void usb_initialize_gadget(struct device *parent,
+		struct usb_gadget *gadget, void (*release)(struct device *dev));
+extern int usb_add_gadget(struct usb_gadget *gadget);
+extern void usb_del_gadget(struct usb_gadget *gadget);
+
+/* Legacy device-model interface */
+extern int usb_add_gadget_udc_release(struct device *parent,
+		struct usb_gadget *gadget, void (*release)(struct device *dev));
+extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget);
+extern void usb_del_gadget_udc(struct usb_gadget *gadget);
+extern char *usb_get_gadget_udc_name(void);
 
 /* iterates the non-control endpoints; 'tmp' is a struct usb_ep pointer */
 #define gadget_for_each_ep(tmp, gadget) \
@@ -735,12 +756,6 @@  int usb_gadget_probe_driver(struct usb_gadget_driver *driver);
  */
 int usb_gadget_unregister_driver(struct usb_gadget_driver *driver);
 
-extern int usb_add_gadget_udc_release(struct device *parent,
-		struct usb_gadget *gadget, void (*release)(struct device *dev));
-extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget);
-extern void usb_del_gadget_udc(struct usb_gadget *gadget);
-extern char *usb_get_gadget_udc_name(void);
-
 /*-------------------------------------------------------------------------*/
 
 /* utility to simplify dealing with string descriptors */