[PATCH 00/26] usb: gadget: encapsulate ep enable/disable

[PATCH 00/26] usb: gadget: encapsulate ep enable/disable

[Robert Baldyga]

Hi Felipe,

There is my next patches series containing few fixes and slightly
reworking USB gadget function API. It introduces ep->enabled flag which
indicates whether endpointis enabled or not, and encapsulates its checking
and setting into usb_ep_enable() and usb_ep_disable() functions. So now
these functions can be used more safely.

Why is that needed?

For example, it's very common pattern in USB functions to re-enable
endpoints in set_alt(). Usually it looks like:

 usb_ep_disable()
 config_ep_by_speed()
 usb_ep_enable()

So far to avoid disabling endpoint which was already disabled there was
need to remember somehow which endpoints are enabled. Unfortunately for
this purpose no new flag in struct usb_ep was introduced, but instead
ep->driver_data was set and cleared to mark endpoints as enabled/disabled.
This made code a little messy considering that the same ep->driver_data
was used before in bind() to claim endpoints obtained from autoconfig,
and moreover many functions use the same ep->driver_data to save some
private pointer, accessible for example in complete() callback (what is,
I believe, driver_data was designed for).

So as now we have ep->claimed flag for marking claimed endpoints, and
ep->enabled flag to remember its enabled/disabled state, we can finally
use ep->driver_data to only contain pointer to private data.

To achieve this, this patch set modifies all USB functions where abuse
ep->driver_data had place, and leaves them in form, where internal endpoint
state is handled by gadget framework functions, and ep->driver_data has
no additional purpose over containing driver data pointer.

Best regards,
Robert Baldyga

Robert Baldyga (26):
  usb: gadget: fix few outdated comments
  usb: gadget: f_ncm: obtain cdev from function instead of driver_data
  usb: gadget: epautoconf: add usb_ep_autoconfig_release() function
  usb: gadget: introduce 'enabled' flag in struct usb_ep
  usb: gadget: f_ecm: eliminate abuse of ep->driver data
  usb: gadget: f_acm: eliminate abuse of ep->driver data
  usb: gadget: f_eem: eliminate abuse of ep->driver data
  usb: gadget: f_hid: eliminate abuse of ep->driver data
  usb: gadget: f_loopback: eliminate abuse of ep->driver data
  usb: gadget: f_mass_storage: eliminate abuse of ep->driver data
  usb: gadget: f_midi: eliminate abuse of ep->driver data
  usb: gadget: f_ncm: eliminate abuse of ep->driver data
  usb: gadget: f_obex: eliminate abuse of ep->driver data
  usb: gadget: f_phonet: eliminate abuse of ep->driver data
  usb: gadget: f_printer: eliminate abuse of ep->driver data
  usb: gadget: f_rndis: eliminate abuse of ep->driver data
  usb: gadget: f_serial: eliminate abuse of ep->driver data
  usb: gadget: f_sourcesink: eliminate abuse of ep->driver data
  usb: gadget: f_subset: eliminate abuse of ep->driver data
  usb: gadget: f_uac1: eliminate abuse of ep->driver data
  usb: gadget: f_uac2: eliminate abuse of ep->driver data
  usb: gadget: f_uvc: eliminate abuse of ep->driver data
  usb: gadget: u_ether: eliminate abuse of ep->driver data
  usb: gadget: u_serial: eliminate abuse of ep->driver data
  usb: gadget: legacy: dbgp: eliminate abuse of ep->driver data
  usb: gadget: legacy: tcm: eliminate abuse of ep->driver data

 drivers/usb/gadget/composite.c               |  4 +--
 drivers/usb/gadget/epautoconf.c              | 25 ++++++++++++++---
 drivers/usb/gadget/function/f_acm.c          | 23 +++------------
 drivers/usb/gadget/function/f_ecm.c          | 31 +++++---------------
 drivers/usb/gadget/function/f_eem.c          | 16 ++---------
 drivers/usb/gadget/function/f_hid.c          | 12 ++------
 drivers/usb/gadget/function/f_loopback.c     |  5 +---
 drivers/usb/gadget/function/f_mass_storage.c |  4 ---
 drivers/usb/gadget/function/f_midi.c         | 14 ++--------
 drivers/usb/gadget/function/f_ncm.c          | 29 +++++--------------
 drivers/usb/gadget/function/f_obex.c         | 10 +------
 drivers/usb/gadget/function/f_phonet.c       |  8 +-----
 drivers/usb/gadget/function/f_printer.c      |  2 --
 drivers/usb/gadget/function/f_rndis.c        | 24 ++++------------
 drivers/usb/gadget/function/f_serial.c       | 10 +------
 drivers/usb/gadget/function/f_sourcesink.c   | 29 +++++--------------
 drivers/usb/gadget/function/f_subset.c       | 10 +------
 drivers/usb/gadget/function/f_uac1.c         |  4 ---
 drivers/usb/gadget/function/f_uac2.c         | 11 --------
 drivers/usb/gadget/function/f_uvc.c          | 42 ++++++----------------------
 drivers/usb/gadget/function/u_ether.c        |  2 --
 drivers/usb/gadget/function/u_serial.c       |  5 ----
 drivers/usb/gadget/legacy/dbgp.c             | 18 ++----------
 drivers/usb/gadget/legacy/tcm_usb_gadget.c   | 18 ------------
 include/linux/usb/gadget.h                   | 23 +++++++++++++--
 25 files changed, 95 insertions(+), 284 deletions(-)

-- 
1.9.1

[PATCH 01/26] usb: gadget: fix few outdated comments

[Robert Baldyga]

Fix comments in code to make them up to date.

composite: claiming endpoint is now done by setting ep->claimed flag,
not ep->driver_data.

epautoconf: usb_ep_autoconfig() and usb_ep_autoconfig_ss() return
claimed endpoint with ep->claimed flag already set.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/composite.c  | 4 +---
 drivers/usb/gadget/epautoconf.c | 8 ++++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index b474499..3e95c0e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -839,9 +839,7 @@ int usb_add_config(struct usb_composite_dev *cdev,
 		}
 	}
 
-	/* set_alt(), or next bind(), sets up
-	 * ep->driver_data as needed.
-	 */
+	/* set_alt(), or next bind(), sets up ep->claimed as needed */
 	usb_ep_autoconfig_reset(cdev->gadget);
 
 done:
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 6399c10..0f4ece4 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -53,13 +53,13 @@
  * the restrictions that may apply. Some combinations of driver
  * and hardware won't be able to autoconfigure.
  *
- * On success, this returns an un-claimed usb_ep, and modifies the endpoint
+ * On success, this returns an claimed usb_ep, and modifies the endpoint
  * descriptor bEndpointAddress.  For bulk endpoints, the wMaxPacket value
  * is initialized as if the endpoint were used at full speed and
  * the bmAttribute field in the ep companion descriptor is
  * updated with the assigned number of streams if it is
  * different from the original value. To prevent the endpoint
- * from being returned by a later autoconfig call, claim it by
+ * from being returned by a later autoconfig call, claims it by
  * assigning ep->claimed to true.
  *
  * On failure, this returns a null endpoint descriptor.
@@ -154,10 +154,10 @@ EXPORT_SYMBOL_GPL(usb_ep_autoconfig_ss);
  * USB controller, and it can't know all the restrictions that may apply.
  * Some combinations of driver and hardware won't be able to autoconfigure.
  *
- * On success, this returns an un-claimed usb_ep, and modifies the endpoint
+ * On success, this returns an claimed usb_ep, and modifies the endpoint
  * descriptor bEndpointAddress.  For bulk endpoints, the wMaxPacket value
  * is initialized as if the endpoint were used at full speed.  To prevent
- * the endpoint from being returned by a later autoconfig call, claim it
+ * the endpoint from being returned by a later autoconfig call, claims it
  * by assigning ep->claimed to true.
  *
  * On failure, this returns a null endpoint descriptor.
-- 
1.9.1

[PATCH 02/26] usb: gadget: f_ncm: obtain cdev from function instead of driver_data

[Robert Baldyga]

The 'driver_data' field in ep0 is never set to pointer to cdev, so we
have to obtain it from another source as in this context ep->driver_data
contains invalid data.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_ncm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 3f05c6bd..394cdbf 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -586,7 +586,7 @@ static void ncm_ep0out_complete(struct usb_ep *ep, struct usb_request *req)
 	unsigned		in_size;
 	struct usb_function	*f = req->context;
 	struct f_ncm		*ncm = func_to_ncm(f);
-	struct usb_composite_dev *cdev = ep->driver_data;
+	struct usb_composite_dev *cdev = f->config->cdev;
 
 	req->context = NULL;
 	if (req->status || req->actual != req->length) {
-- 
1.9.1

[PATCH 03/26] usb: gadget: epautoconf: add usb_ep_autoconfig_release() function

[Robert Baldyga]

This patch introduces usb_ep_autoconfig_release() function which allows
to release endpoint previously obtained from usb_ep_autoconfig() during
USB function bind.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/epautoconf.c | 17 +++++++++++++++++
 include/linux/usb/gadget.h      |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 0f4ece4..30fdab0 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -172,6 +172,23 @@ struct usb_ep *usb_ep_autoconfig(
 EXPORT_SYMBOL_GPL(usb_ep_autoconfig);
 
 /**
+ * usb_ep_autoconfig_release - releases endpoint and set it to initial state
+ * @ep: endpoint which should be released
+ *
+ * This function can be used during function bind for endpoints obtained
+ * from usb_ep_autoconfig(). It unclaims endpoint claimed by
+ * usb_ep_autoconfig() to make it available for other functions. Endpoint
+ * which was released is no longer invalid and shouldn't be used in
+ * context of function which released it.
+ */
+void usb_ep_autoconfig_release(struct usb_ep *ep)
+{
+	ep->claimed = false;
+	ep->driver_data = NULL;
+}
+EXPORT_SYMBOL_GPL(usb_ep_autoconfig_release);
+
+/**
  * usb_ep_autoconfig_reset - reset endpoint autoconfig state
  * @gadget: device for which autoconfig state will be reset
  *
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c14a69b3..3f299e2 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1233,6 +1233,8 @@ extern struct usb_ep *usb_ep_autoconfig_ss(struct usb_gadget *,
 			struct usb_endpoint_descriptor *,
 			struct usb_ss_ep_comp_descriptor *);
 
+extern void usb_ep_autoconfig_release(struct usb_ep *);
+
 extern void usb_ep_autoconfig_reset(struct usb_gadget *);
 
 #endif /* __LINUX_USB_GADGET_H */
-- 
1.9.1

[PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Robert Baldyga]

This patch introduces 'enabled' flag in struct usb_ep, and modifies
usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
enabled/disabled state. It helps to avoid enabling endpoints which are
already enabled, and disabling endpoints which are already disables.

>From now USB functions don't have to remember current endpoint
enable/disable state, as this state is now handled automatically which
makes this API less bug-prone.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 include/linux/usb/gadget.h | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 3f299e2..63375cd 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -215,6 +215,7 @@ struct usb_ep {
 	struct list_head	ep_list;
 	struct usb_ep_caps	caps;
 	bool			claimed;
+	bool			enabled;
 	unsigned		maxpacket:16;
 	unsigned		maxpacket_limit:16;
 	unsigned		max_streams:16;
@@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
  */
 static inline int usb_ep_enable(struct usb_ep *ep)
 {
-	return ep->ops->enable(ep, ep->desc);
+	int ret = 0;
+
+	if (!ep->enabled) {
+		ret = ep->ops->enable(ep, ep->desc);
+		if (!ret)
+			ep->enabled = true;
+	}
+
+	return ret;
 }
 
 /**
@@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep)
  */
 static inline int usb_ep_disable(struct usb_ep *ep)
 {
-	return ep->ops->disable(ep);
+	int ret = 0;
+
+	if (ep->enabled) {
+		ret = ep->ops->disable(ep);
+		if (!ret)
+			ep->enabled = false;
+	}
+
+	return ret;
 }
 
 /**
-- 
1.9.1

[PATCH 05/26] usb: gadget: f_ecm: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_ecm, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_ecm.c | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 7b7424f..4abca70 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -541,24 +541,21 @@ static int ecm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		if (alt != 0)
 			goto fail;
 
-		if (ecm->notify->driver_data) {
-			VDBG(cdev, "reset ecm control %d\n", intf);
-			usb_ep_disable(ecm->notify);
-		}
+		VDBG(cdev, "reset ecm control %d\n", intf);
+		usb_ep_disable(ecm->notify);
 		if (!(ecm->notify->desc)) {
 			VDBG(cdev, "init ecm ctrl %d\n", intf);
 			if (config_ep_by_speed(cdev->gadget, f, ecm->notify))
 				goto fail;
 		}
 		usb_ep_enable(ecm->notify);
-		ecm->notify->driver_data = ecm;
 
 	/* Data interface has two altsettings, 0 and 1 */
 	} else if (intf == ecm->data_id) {
 		if (alt > 1)
 			goto fail;
 
-		if (ecm->port.in_ep->driver_data) {
+		if (ecm->port.in_ep->enabled) {
 			DBG(cdev, "reset ecm\n");
 			gether_disconnect(&ecm->port);
 		}
@@ -618,7 +615,7 @@ static int ecm_get_alt(struct usb_function *f, unsigned intf)
 
 	if (intf == ecm->ctrl_id)
 		return 0;
-	return ecm->port.in_ep->driver_data ? 1 : 0;
+	return ecm->port.in_ep->enabled ? 1 : 0;
 }
 
 static void ecm_disable(struct usb_function *f)
@@ -628,14 +625,11 @@ static void ecm_disable(struct usb_function *f)
 
 	DBG(cdev, "ecm deactivated\n");
 
-	if (ecm->port.in_ep->driver_data)
+	if (ecm->port.in_ep->enabled)
 		gether_disconnect(&ecm->port);
 
-	if (ecm->notify->driver_data) {
-		usb_ep_disable(ecm->notify);
-		ecm->notify->driver_data = NULL;
-		ecm->notify->desc = NULL;
-	}
+	usb_ep_disable(ecm->notify);
+	ecm->notify->desc = NULL;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -750,13 +744,11 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	ecm->port.in_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_out_desc);
 	if (!ep)
 		goto fail;
 	ecm->port.out_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	/* NOTE:  a status/notification endpoint is *OPTIONAL* but we
 	 * don't treat it that way.  It's simpler, and some newer CDC
@@ -766,7 +758,6 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	ecm->notify = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	status = -ENOMEM;
 
@@ -820,14 +811,6 @@ fail:
 		usb_ep_free_request(ecm->notify, ecm->notify_req);
 	}
 
-	/* we might as well release our claims on endpoints */
-	if (ecm->notify)
-		ecm->notify->driver_data = NULL;
-	if (ecm->port.out_ep)
-		ecm->port.out_ep->driver_data = NULL;
-	if (ecm->port.in_ep)
-		ecm->port.in_ep->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 06/26] usb: gadget: f_acm: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_acm we only need to store in ep->driver_data pointer to
struct f_acm, as it's used in acm_complete_set_line_coding() callback.
All other uses of ep->driver_data are now meaningless and can be safely
removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_acm.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index be9df09..22e723d 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -428,21 +428,18 @@ static int acm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	/* we know alt == 0, so this is an activation or a reset */
 
 	if (intf == acm->ctrl_id) {
-		if (acm->notify->driver_data) {
-			dev_vdbg(&cdev->gadget->dev,
-				 "reset acm control interface %d\n", intf);
-			usb_ep_disable(acm->notify);
-		}
+		dev_vdbg(&cdev->gadget->dev,
+				"reset acm control interface %d\n", intf);
+		usb_ep_disable(acm->notify);
 
 		if (!acm->notify->desc)
 			if (config_ep_by_speed(cdev->gadget, f, acm->notify))
 				return -EINVAL;
 
 		usb_ep_enable(acm->notify);
-		acm->notify->driver_data = acm;
 
 	} else if (intf == acm->data_id) {
-		if (acm->port.in->driver_data) {
+		if (acm->notify->enabled) {
 			dev_dbg(&cdev->gadget->dev,
 				"reset acm ttyGS%d\n", acm->port_num);
 			gserial_disconnect(&acm->port);
@@ -475,7 +472,6 @@ static void acm_disable(struct usb_function *f)
 	dev_dbg(&cdev->gadget->dev, "acm ttyGS%d deactivated\n", acm->port_num);
 	gserial_disconnect(&acm->port);
 	usb_ep_disable(acm->notify);
-	acm->notify->driver_data = NULL;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -655,19 +651,16 @@ acm_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	acm->port.in = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &acm_fs_out_desc);
 	if (!ep)
 		goto fail;
 	acm->port.out = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &acm_fs_notify_desc);
 	if (!ep)
 		goto fail;
 	acm->notify = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	/* allocate notification */
 	acm->notify_req = gs_alloc_req(ep,
@@ -709,14 +702,6 @@ fail:
 	if (acm->notify_req)
 		gs_free_req(acm->notify, acm->notify_req);
 
-	/* we might as well release our claims on endpoints */
-	if (acm->notify)
-		acm->notify->driver_data = NULL;
-	if (acm->port.out)
-		acm->port.out->driver_data = NULL;
-	if (acm->port.in)
-		acm->port.in->driver_data = NULL;
-
 	ERROR(cdev, "%s/%p: can't bind, err %d\n", f->name, f, status);
 
 	return status;
-- 
1.9.1

[PATCH 07/26] usb: gadget: f_eem: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_ecm, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_eem.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
index c9e90de..9a55757 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -195,11 +195,8 @@ static int eem_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		goto fail;
 
 	if (intf == eem->ctrl_id) {
-
-		if (eem->port.in_ep->driver_data) {
-			DBG(cdev, "reset eem\n");
-			gether_disconnect(&eem->port);
-		}
+		DBG(cdev, "reset eem\n");
+		gether_disconnect(&eem->port);
 
 		if (!eem->port.in_ep->desc || !eem->port.out_ep->desc) {
 			DBG(cdev, "init eem\n");
@@ -237,7 +234,7 @@ static void eem_disable(struct usb_function *f)
 
 	DBG(cdev, "eem deactivated\n");
 
-	if (eem->port.in_ep->driver_data)
+	if (eem->port.in_ep->enabled)
 		gether_disconnect(&eem->port);
 }
 
@@ -293,13 +290,11 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	eem->port.in_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &eem_fs_out_desc);
 	if (!ep)
 		goto fail;
 	eem->port.out_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	status = -ENOMEM;
 
@@ -325,11 +320,6 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 fail:
-	if (eem->port.out_ep)
-		eem->port.out_ep->driver_data = NULL;
-	if (eem->port.in_ep)
-		eem->port.in_ep->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 08/26] usb: gadget: f_hid: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_hid we only need to store in ep->driver_data pointer to
struct f_hidg, as it's used in f_hidg_req_complete() callback. All
other uses of ep->driver_data are now meaningless and can be safely
removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_hid.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 6df9715..21fcf18 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -492,10 +492,7 @@ static void hidg_disable(struct usb_function *f)
 	struct f_hidg_req_list *list, *next;
 
 	usb_ep_disable(hidg->in_ep);
-	hidg->in_ep->driver_data = NULL;
-
 	usb_ep_disable(hidg->out_ep);
-	hidg->out_ep->driver_data = NULL;
 
 	list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) {
 		list_del(&list->list);
@@ -513,8 +510,7 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 	if (hidg->in_ep != NULL) {
 		/* restart endpoint */
-		if (hidg->in_ep->driver_data != NULL)
-			usb_ep_disable(hidg->in_ep);
+		usb_ep_disable(hidg->in_ep);
 
 		status = config_ep_by_speed(f->config->cdev->gadget, f,
 					    hidg->in_ep);
@@ -533,8 +529,7 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 	if (hidg->out_ep != NULL) {
 		/* restart endpoint */
-		if (hidg->out_ep->driver_data != NULL)
-			usb_ep_disable(hidg->out_ep);
+		usb_ep_disable(hidg->out_ep);
 
 		status = config_ep_by_speed(f->config->cdev->gadget, f,
 					    hidg->out_ep);
@@ -566,7 +561,6 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 						hidg->out_ep->name, status);
 			} else {
 				usb_ep_disable(hidg->out_ep);
-				hidg->out_ep->driver_data = NULL;
 				status = -ENOMEM;
 				goto fail;
 			}
@@ -614,13 +608,11 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	ep = usb_ep_autoconfig(c->cdev->gadget, &hidg_fs_in_ep_desc);
 	if (!ep)
 		goto fail;
-	ep->driver_data = c->cdev;	/* claim */
 	hidg->in_ep = ep;
 
 	ep = usb_ep_autoconfig(c->cdev->gadget, &hidg_fs_out_ep_desc);
 	if (!ep)
 		goto fail;
-	ep->driver_data = c->cdev;	/* claim */
 	hidg->out_ep = ep;
 
 	/* preallocate request and buffer */
-- 
1.9.1

[PATCH 09/26] usb: gadget: f_loopback: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_hid we only need to store in ep->driver_data pointer to
struct f_loopback, as it's used in loopback_complete() callback. All
other uses of ep->driver_data are now meaningless and can be safely
removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_loopback.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
index 6e2fe63..300e601 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -195,12 +195,10 @@ autoconf_fail:
 			f->name, cdev->gadget->name);
 		return -ENODEV;
 	}
-	loop->in_ep->driver_data = cdev;	/* claim */
 
 	loop->out_ep = usb_ep_autoconfig(cdev->gadget, &fs_loop_sink_desc);
 	if (!loop->out_ep)
 		goto autoconf_fail;
-	loop->out_ep->driver_data = cdev;	/* claim */
 
 	/* support high speed hardware */
 	hs_loop_source_desc.bEndpointAddress =
@@ -364,8 +362,7 @@ static int loopback_set_alt(struct usb_function *f,
 	struct usb_composite_dev *cdev = f->config->cdev;
 
 	/* we know alt is zero */
-	if (loop->in_ep->driver_data)
-		disable_loopback(loop);
+	disable_loopback(loop);
 	return enable_loopback(cdev, loop);
 }
 
-- 
1.9.1

[PATCH 10/26] usb: gadget: f_mass_storage: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_mass_storage we only need to store in ep->driver_data
pointer to struct fsg_common, which is used in bulk_in_complete() and
bulk_out_complete() callbacks. All other uses of ep->driver_data are now
meaningless and can be safely removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index a6eb537..5ae7473 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2258,12 +2258,10 @@ reset:
 		/* Disable the endpoints */
 		if (fsg->bulk_in_enabled) {
 			usb_ep_disable(fsg->bulk_in);
-			fsg->bulk_in->driver_data = NULL;
 			fsg->bulk_in_enabled = 0;
 		}
 		if (fsg->bulk_out_enabled) {
 			usb_ep_disable(fsg->bulk_out);
-			fsg->bulk_out->driver_data = NULL;
 			fsg->bulk_out_enabled = 0;
 		}
 
@@ -3070,13 +3068,11 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
 	ep = usb_ep_autoconfig(gadget, &fsg_fs_bulk_in_desc);
 	if (!ep)
 		goto autoconf_fail;
-	ep->driver_data = fsg->common;	/* claim the endpoint */
 	fsg->bulk_in = ep;
 
 	ep = usb_ep_autoconfig(gadget, &fsg_fs_bulk_out_desc);
 	if (!ep)
 		goto autoconf_fail;
-	ep->driver_data = fsg->common;	/* claim the endpoint */
 	fsg->bulk_out = ep;
 
 	/* Assume endpoint addresses are the same for both speeds */
-- 
1.9.1

[PATCH 11/26] usb: gadget: f_midi: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_midi we only need to store in ep->driver_data pointer to
struct f_midi, as it's used in f_midi_complete() callback and related
functions. All other uses of ep->driver_data are now meaningless and
can be safely removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_midi.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index a287a48..3e6d1da 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -302,8 +302,7 @@ static int f_midi_start_ep(struct f_midi *midi,
 	int err;
 	struct usb_composite_dev *cdev = f->config->cdev;
 
-	if (ep->driver_data)
-		usb_ep_disable(ep);
+	usb_ep_disable(ep);
 
 	err = config_ep_by_speed(midi->gadget, f, ep);
 	if (err) {
@@ -341,8 +340,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	if (err)
 		return err;
 
-	if (midi->out_ep->driver_data)
-		usb_ep_disable(midi->out_ep);
+	usb_ep_disable(midi->out_ep);
 
 	err = config_ep_by_speed(midi->gadget, f, midi->out_ep);
 	if (err) {
@@ -757,12 +755,10 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
 	midi->in_ep = usb_ep_autoconfig(cdev->gadget, &bulk_in_desc);
 	if (!midi->in_ep)
 		goto fail;
-	midi->in_ep->driver_data = cdev;	/* claim */
 
 	midi->out_ep = usb_ep_autoconfig(cdev->gadget, &bulk_out_desc);
 	if (!midi->out_ep)
 		goto fail;
-	midi->out_ep->driver_data = cdev;	/* claim */
 
 	/* allocate temporary function list */
 	midi_function = kcalloc((MAX_PORTS * 4) + 9, sizeof(*midi_function),
@@ -889,12 +885,6 @@ fail_f_midi:
 fail:
 	f_midi_unregister_card(midi);
 fail_register:
-	/* we might as well release our claims on endpoints */
-	if (midi->out_ep)
-		midi->out_ep->driver_data = NULL;
-	if (midi->in_ep)
-		midi->in_ep->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 12/26] usb: gadget: f_ncm: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_ncm, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_ncm.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 394cdbf..b6f7ed7 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -803,10 +803,8 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		if (alt != 0)
 			goto fail;
 
-		if (ncm->notify->driver_data) {
-			DBG(cdev, "reset ncm control %d\n", intf);
-			usb_ep_disable(ncm->notify);
-		}
+		DBG(cdev, "reset ncm control %d\n", intf);
+		usb_ep_disable(ncm->notify);
 
 		if (!(ncm->notify->desc)) {
 			DBG(cdev, "init ncm ctrl %d\n", intf);
@@ -814,14 +812,13 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 				goto fail;
 		}
 		usb_ep_enable(ncm->notify);
-		ncm->notify->driver_data = ncm;
 
 	/* Data interface has two altsettings, 0 and 1 */
 	} else if (intf == ncm->data_id) {
 		if (alt > 1)
 			goto fail;
 
-		if (ncm->port.in_ep->driver_data) {
+		if (ncm->port.in_ep->enabled) {
 			DBG(cdev, "reset ncm\n");
 			ncm->timer_stopping = true;
 			ncm->netdev = NULL;
@@ -885,7 +882,7 @@ static int ncm_get_alt(struct usb_function *f, unsigned intf)
 
 	if (intf == ncm->ctrl_id)
 		return 0;
-	return ncm->port.in_ep->driver_data ? 1 : 0;
+	return ncm->port.in_ep->enabled ? 1 : 0;
 }
 
 static struct sk_buff *package_for_tx(struct f_ncm *ncm)
@@ -1276,15 +1273,14 @@ static void ncm_disable(struct usb_function *f)
 
 	DBG(cdev, "ncm deactivated\n");
 
-	if (ncm->port.in_ep->driver_data) {
+	if (ncm->port.in_ep->enabled) {
 		ncm->timer_stopping = true;
 		ncm->netdev = NULL;
 		gether_disconnect(&ncm->port);
 	}
 
-	if (ncm->notify->driver_data) {
+	if (ncm->notify->enabled) {
 		usb_ep_disable(ncm->notify);
-		ncm->notify->driver_data = NULL;
 		ncm->notify->desc = NULL;
 	}
 }
@@ -1402,19 +1398,16 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	ncm->port.in_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_out_desc);
 	if (!ep)
 		goto fail;
 	ncm->port.out_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_notify_desc);
 	if (!ep)
 		goto fail;
 	ncm->notify = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	status = -ENOMEM;
 
@@ -1468,14 +1461,6 @@ fail:
 		usb_ep_free_request(ncm->notify, ncm->notify_req);
 	}
 
-	/* we might as well release our claims on endpoints */
-	if (ncm->notify)
-		ncm->notify->driver_data = NULL;
-	if (ncm->port.out_ep)
-		ncm->port.out_ep->driver_data = NULL;
-	if (ncm->port.in_ep)
-		ncm->port.in_ep->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 13/26] usb: gadget: f_obex: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_obex, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_obex.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/function/f_obex.c b/drivers/usb/gadget/function/f_obex.c
index 5460426..1c3d30a 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -206,7 +206,7 @@ static int obex_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		if (alt > 1)
 			goto fail;
 
-		if (obex->port.in->driver_data) {
+		if (obex->port.in->enabled) {
 			dev_dbg(&cdev->gadget->dev,
 				"reset obex ttyGS%d\n", obex->port_num);
 			gserial_disconnect(&obex->port);
@@ -348,13 +348,11 @@ static int obex_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	obex->port.in = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &obex_fs_ep_out_desc);
 	if (!ep)
 		goto fail;
 	obex->port.out = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	/* support all relevant hardware speeds... we expect that when
 	 * hardware is dual speed, all bulk-capable endpoints work at
@@ -378,12 +376,6 @@ static int obex_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 fail:
-	/* we might as well release our claims on endpoints */
-	if (obex->port.out)
-		obex->port.out->driver_data = NULL;
-	if (obex->port.in)
-		obex->port.in->driver_data = NULL;
-
 	ERROR(cdev, "%s/%p: can't bind, err %d\n", f->name, f, status);
 
 	return status;
-- 
1.9.1

[PATCH 14/26] usb: gadget: f_phonet: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_phonet we only need to store in ep->driver_data pointer to
struct f_phonet, as it's used in pn_tx_complete() and pn_rx_complete()
callbacks. All other uses of ep->driver_data are now meaningless and can
be safely removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_phonet.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index c0c3ef2..62a1987 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -418,7 +418,7 @@ static int pn_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 		spin_lock(&port->lock);
 
-		if (fp->in_ep->driver_data)
+		if (fp->in_ep->enabled)
 			__pn_reset(f);
 
 		if (alt == 1) {
@@ -530,13 +530,11 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto err;
 	fp->out_ep = ep;
-	ep->driver_data = fp; /* Claim */
 
 	ep = usb_ep_autoconfig(gadget, &pn_fs_source_desc);
 	if (!ep)
 		goto err;
 	fp->in_ep = ep;
-	ep->driver_data = fp; /* Claim */
 
 	pn_hs_sink_desc.bEndpointAddress = pn_fs_sink_desc.bEndpointAddress;
 	pn_hs_source_desc.bEndpointAddress = pn_fs_source_desc.bEndpointAddress;
@@ -575,10 +573,6 @@ err_req:
 		usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
 	usb_free_all_descriptors(f);
 err:
-	if (fp->out_ep)
-		fp->out_ep->driver_data = NULL;
-	if (fp->in_ep)
-		fp->in_ep->driver_data = NULL;
 	ERROR(cdev, "USB CDC Phonet: cannot autoconfigure\n");
 	return status;
 }
-- 
1.9.1

[PATCH 15/26] usb: gadget: f_printer: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_printer we only need to store in ep->driver_data pointer to
struct printer_dev, as it's used in rx_complete() and tx_complete()
callbacks. All other uses of ep->driver_data are now meaningless and can
be safely removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_printer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 8e2b6be..7fb3209 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1039,12 +1039,10 @@ autoconf_fail:
 			cdev->gadget->name);
 		return -ENODEV;
 	}
-	in_ep->driver_data = in_ep;	/* claim */
 
 	out_ep = usb_ep_autoconfig(cdev->gadget, &fs_ep_out_desc);
 	if (!out_ep)
 		goto autoconf_fail;
-	out_ep->driver_data = out_ep;	/* claim */
 
 	/* assumes that all endpoints are dual-speed */
 	hs_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
-- 
1.9.1

[PATCH 16/26] usb: gadget: f_rndis: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_rndis, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_rndis.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index 32985da..fd301ed 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -543,22 +543,20 @@ static int rndis_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	/* we know alt == 0 */
 
 	if (intf == rndis->ctrl_id) {
-		if (rndis->notify->driver_data) {
-			VDBG(cdev, "reset rndis control %d\n", intf);
-			usb_ep_disable(rndis->notify);
-		}
+		VDBG(cdev, "reset rndis control %d\n", intf);
+		usb_ep_disable(rndis->notify);
+
 		if (!rndis->notify->desc) {
 			VDBG(cdev, "init rndis ctrl %d\n", intf);
 			if (config_ep_by_speed(cdev->gadget, f, rndis->notify))
 				goto fail;
 		}
 		usb_ep_enable(rndis->notify);
-		rndis->notify->driver_data = rndis;
 
 	} else if (intf == rndis->data_id) {
 		struct net_device	*net;
 
-		if (rndis->port.in_ep->driver_data) {
+		if (rndis->port.in_ep->enabled) {
 			DBG(cdev, "reset rndis\n");
 			gether_disconnect(&rndis->port);
 		}
@@ -612,7 +610,7 @@ static void rndis_disable(struct usb_function *f)
 	struct f_rndis		*rndis = func_to_rndis(f);
 	struct usb_composite_dev *cdev = f->config->cdev;
 
-	if (!rndis->notify->driver_data)
+	if (!rndis->notify->enabled)
 		return;
 
 	DBG(cdev, "rndis deactivated\n");
@@ -621,7 +619,6 @@ static void rndis_disable(struct usb_function *f)
 	gether_disconnect(&rndis->port);
 
 	usb_ep_disable(rndis->notify);
-	rndis->notify->driver_data = NULL;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -745,13 +742,11 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	rndis->port.in_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &fs_out_desc);
 	if (!ep)
 		goto fail;
 	rndis->port.out_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	/* NOTE:  a status/notification endpoint is, strictly speaking,
 	 * optional.  We don't treat it that way though!  It's simpler,
@@ -761,7 +756,6 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	rndis->notify = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	status = -ENOMEM;
 
@@ -829,14 +823,6 @@ fail:
 		usb_ep_free_request(rndis->notify, rndis->notify_req);
 	}
 
-	/* we might as well release our claims on endpoints */
-	if (rndis->notify)
-		rndis->notify->driver_data = NULL;
-	if (rndis->port.out_ep)
-		rndis->port.out_ep->driver_data = NULL;
-	if (rndis->port.in_ep)
-		rndis->port.in_ep->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 17/26] usb: gadget: f_serial: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_serial, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_serial.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/function/f_serial.c b/drivers/usb/gadget/function/f_serial.c
index 1d162e2..ba705e0 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -153,7 +153,7 @@ static int gser_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 	/* we know alt == 0, so this is an activation or a reset */
 
-	if (gser->port.in->driver_data) {
+	if (gser->port.in->enabled) {
 		dev_dbg(&cdev->gadget->dev,
 			"reset generic ttyGS%d\n", gser->port_num);
 		gserial_disconnect(&gser->port);
@@ -219,13 +219,11 @@ static int gser_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	gser->port.in = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &gser_fs_out_desc);
 	if (!ep)
 		goto fail;
 	gser->port.out = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	/* support all relevant hardware speeds... we expect that when
 	 * hardware is dual speed, all bulk-capable endpoints work at
@@ -249,12 +247,6 @@ static int gser_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 fail:
-	/* we might as well release our claims on endpoints */
-	if (gser->port.out)
-		gser->port.out->driver_data = NULL;
-	if (gser->port.in)
-		gser->port.in->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 18/26] usb: gadget: f_sourcesink: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_sourcesink we only need to store in ep->driver_data pointer
to struct f_sourcesink, as it's used in source_sink_complete() callback.
All other uses of ep->driver_data are now meaningless and can be safely
removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_sourcesink.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index cbfaf86..87817de 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -311,13 +311,9 @@ static void disable_ep(struct usb_composite_dev *cdev, struct usb_ep *ep)
 {
 	int			value;
 
-	if (ep->driver_data) {
-		value = usb_ep_disable(ep);
-		if (value < 0)
-			DBG(cdev, "disable %s --> %d\n",
-					ep->name, value);
-		ep->driver_data = NULL;
-	}
+	value = usb_ep_disable(ep);
+	if (value < 0)
+		DBG(cdev, "disable %s --> %d\n", ep->name, value);
 }
 
 void disable_endpoints(struct usb_composite_dev *cdev,
@@ -355,12 +351,10 @@ autoconf_fail:
 			f->name, cdev->gadget->name);
 		return -ENODEV;
 	}
-	ss->in_ep->driver_data = cdev;	/* claim */
 
 	ss->out_ep = usb_ep_autoconfig(cdev->gadget, &fs_sink_desc);
 	if (!ss->out_ep)
 		goto autoconf_fail;
-	ss->out_ep->driver_data = cdev;	/* claim */
 
 	/* sanity check the isoc module parameters */
 	if (isoc_interval < 1)
@@ -384,13 +378,10 @@ autoconf_fail:
 	ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc);
 	if (!ss->iso_in_ep)
 		goto no_iso;
-	ss->iso_in_ep->driver_data = cdev;	/* claim */
 
 	ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_sink_desc);
-	if (ss->iso_out_ep) {
-		ss->iso_out_ep->driver_data = cdev;	/* claim */
-	} else {
-		ss->iso_in_ep->driver_data = NULL;
+	if (!ss->iso_out_ep) {
+		usb_ep_autoconfig_release(ss->iso_in_ep);
 		ss->iso_in_ep = NULL;
 no_iso:
 		/*
@@ -683,7 +674,6 @@ enable_source_sink(struct usb_composite_dev *cdev, struct f_sourcesink *ss,
 fail:
 		ep = ss->in_ep;
 		usb_ep_disable(ep);
-		ep->driver_data = NULL;
 		return result;
 	}
 
@@ -702,7 +692,6 @@ fail:
 fail2:
 		ep = ss->out_ep;
 		usb_ep_disable(ep);
-		ep->driver_data = NULL;
 		goto fail;
 	}
 
@@ -724,10 +713,8 @@ fail2:
 		if (result < 0) {
 fail3:
 			ep = ss->iso_in_ep;
-			if (ep) {
+			if (ep)
 				usb_ep_disable(ep);
-				ep->driver_data = NULL;
-			}
 			goto fail2;
 		}
 	}
@@ -746,7 +733,6 @@ fail3:
 		result = source_sink_start_ep(ss, false, true, speed);
 		if (result < 0) {
 			usb_ep_disable(ep);
-			ep->driver_data = NULL;
 			goto fail3;
 		}
 	}
@@ -763,8 +749,7 @@ static int sourcesink_set_alt(struct usb_function *f,
 	struct f_sourcesink		*ss = func_to_ss(f);
 	struct usb_composite_dev	*cdev = f->config->cdev;
 
-	if (ss->in_ep->driver_data)
-		disable_source_sink(ss);
+	disable_source_sink(ss);
 	return enable_source_sink(cdev, ss, alt);
 }
 
-- 
1.9.1

[PATCH 19/26] usb: gadget: f_subset: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_subset, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_subset.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
index e3dfa67..2e66e62 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -262,7 +262,7 @@ static int geth_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 	/* we know alt == 0, so this is an activation or a reset */
 
-	if (geth->port.in_ep->driver_data) {
+	if (geth->port.in_ep->enabled) {
 		DBG(cdev, "reset cdc subset\n");
 		gether_disconnect(&geth->port);
 	}
@@ -343,13 +343,11 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!ep)
 		goto fail;
 	geth->port.in_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	ep = usb_ep_autoconfig(cdev->gadget, &fs_subset_out_desc);
 	if (!ep)
 		goto fail;
 	geth->port.out_ep = ep;
-	ep->driver_data = cdev;	/* claim */
 
 	/* support all relevant hardware speeds... we expect that when
 	 * hardware is dual speed, all bulk-capable endpoints work at
@@ -380,12 +378,6 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 fail:
-	/* we might as well release our claims on endpoints */
-	if (geth->port.out_ep)
-		geth->port.out_ep->driver_data = NULL;
-	if (geth->port.in_ep)
-		geth->port.in_ep->driver_data = NULL;
-
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
 
 	return status;
-- 
1.9.1

[PATCH 20/26] usb: gadget: f_uac1: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_uac1, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_uac1.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
index 7856b33..8ee7019 100644
--- a/drivers/usb/gadget/function/f_uac1.c
+++ b/drivers/usb/gadget/function/f_uac1.c
@@ -593,7 +593,6 @@ static int f_audio_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 				return err;
 
 			usb_ep_enable(out_ep);
-			out_ep->driver_data = audio;
 			audio->copy_buf = f_audio_buffer_alloc(audio_buf_size);
 			if (IS_ERR(audio->copy_buf))
 				return -ENOMEM;
@@ -718,7 +717,6 @@ f_audio_bind(struct usb_configuration *c, struct usb_function *f)
 		goto fail;
 	audio->out_ep = ep;
 	audio->out_ep->desc = &as_out_ep_desc;
-	ep->driver_data = cdev;	/* claim */
 
 	status = -ENOMEM;
 
@@ -730,8 +728,6 @@ f_audio_bind(struct usb_configuration *c, struct usb_function *f)
 
 fail:
 	gaudio_cleanup(&audio->card);
-	if (ep)
-		ep->driver_data = NULL;
 	return status;
 }
 
-- 
1.9.1

[PATCH 21/26] usb: gadget: f_uac2: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_uac2, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_uac2.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index f8de7ea..63336e26 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1081,14 +1081,12 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
 		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
 		goto err;
 	}
-	agdev->out_ep->driver_data = agdev;
 
 	agdev->in_ep = usb_ep_autoconfig(gadget, &fs_epin_desc);
 	if (!agdev->in_ep) {
 		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
 		goto err;
 	}
-	agdev->in_ep->driver_data = agdev;
 
 	uac2->p_prm.uac2 = uac2;
 	uac2->c_prm.uac2 = uac2;
@@ -1132,10 +1130,6 @@ err_free_descs:
 err:
 	kfree(agdev->uac2.p_prm.rbuf);
 	kfree(agdev->uac2.c_prm.rbuf);
-	if (agdev->in_ep)
-		agdev->in_ep->driver_data = NULL;
-	if (agdev->out_ep)
-		agdev->out_ep->driver_data = NULL;
 	return -EINVAL;
 }
 
@@ -1583,11 +1577,6 @@ static void afunc_unbind(struct usb_configuration *c, struct usb_function *f)
 	prm = &agdev->uac2.c_prm;
 	kfree(prm->rbuf);
 	usb_free_all_descriptors(f);
-
-	if (agdev->in_ep)
-		agdev->in_ep->driver_data = NULL;
-	if (agdev->out_ep)
-		agdev->out_ep->driver_data = NULL;
 }
 
 static struct usb_function *afunc_alloc(struct usb_function_instance *fi)
-- 
1.9.1

[PATCH 22/26] usb: gadget: f_uvc: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of f_uvc, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_uvc.c | 42 +++++++------------------------------
 1 file changed, 8 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 743be34..29b41b5 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -280,7 +280,7 @@ uvc_function_get_alt(struct usb_function *f, unsigned interface)
 	else if (interface != uvc->streaming_intf)
 		return -EINVAL;
 	else
-		return uvc->video.ep->driver_data ? 1 : 0;
+		return uvc->video.ep->enabled ? 1 : 0;
 }
 
 static int
@@ -298,18 +298,14 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		if (alt)
 			return -EINVAL;
 
-		if (uvc->control_ep->driver_data) {
-			INFO(cdev, "reset UVC Control\n");
-			usb_ep_disable(uvc->control_ep);
-			uvc->control_ep->driver_data = NULL;
-		}
+		INFO(cdev, "reset UVC Control\n");
+		usb_ep_disable(uvc->control_ep);
 
 		if (!uvc->control_ep->desc)
 			if (config_ep_by_speed(cdev->gadget, f, uvc->control_ep))
 				return -EINVAL;
 
 		usb_ep_enable(uvc->control_ep);
-		uvc->control_ep->driver_data = uvc;
 
 		if (uvc->state == UVC_STATE_DISCONNECTED) {
 			memset(&v4l2_event, 0, sizeof(v4l2_event));
@@ -336,10 +332,8 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		if (uvc->state != UVC_STATE_STREAMING)
 			return 0;
 
-		if (uvc->video.ep) {
+		if (uvc->video.ep)
 			usb_ep_disable(uvc->video.ep);
-			uvc->video.ep->driver_data = NULL;
-		}
 
 		memset(&v4l2_event, 0, sizeof(v4l2_event));
 		v4l2_event.type = UVC_EVENT_STREAMOFF;
@@ -355,18 +349,14 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		if (!uvc->video.ep)
 			return -EINVAL;
 
-		if (uvc->video.ep->driver_data) {
-			INFO(cdev, "reset UVC\n");
-			usb_ep_disable(uvc->video.ep);
-			uvc->video.ep->driver_data = NULL;
-		}
+		INFO(cdev, "reset UVC\n");
+		usb_ep_disable(uvc->video.ep);
 
 		ret = config_ep_by_speed(f->config->cdev->gadget,
 				&(uvc->func), uvc->video.ep);
 		if (ret)
 			return ret;
 		usb_ep_enable(uvc->video.ep);
-		uvc->video.ep->driver_data = uvc;
 
 		memset(&v4l2_event, 0, sizeof(v4l2_event));
 		v4l2_event.type = UVC_EVENT_STREAMON;
@@ -392,15 +382,8 @@ uvc_function_disable(struct usb_function *f)
 
 	uvc->state = UVC_STATE_DISCONNECTED;
 
-	if (uvc->video.ep->driver_data) {
-		usb_ep_disable(uvc->video.ep);
-		uvc->video.ep->driver_data = NULL;
-	}
-
-	if (uvc->control_ep->driver_data) {
-		usb_ep_disable(uvc->control_ep);
-		uvc->control_ep->driver_data = NULL;
-	}
+	usb_ep_disable(uvc->video.ep);
+	usb_ep_disable(uvc->control_ep);
 }
 
 /* --------------------------------------------------------------------------
@@ -651,7 +634,6 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		goto error;
 	}
 	uvc->control_ep = ep;
-	ep->driver_data = uvc;
 
 	if (gadget_is_superspeed(c->cdev->gadget))
 		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
@@ -666,7 +648,6 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		goto error;
 	}
 	uvc->video.ep = ep;
-	ep->driver_data = uvc;
 
 	uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
@@ -755,11 +736,6 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 error:
 	v4l2_device_unregister(&uvc->v4l2_dev);
 
-	if (uvc->control_ep)
-		uvc->control_ep->driver_data = NULL;
-	if (uvc->video.ep)
-		uvc->video.ep->driver_data = NULL;
-
 	if (uvc->control_req)
 		usb_ep_free_request(cdev->gadget->ep0, uvc->control_req);
 	kfree(uvc->control_buf);
@@ -886,8 +862,6 @@ static void uvc_unbind(struct usb_configuration *c, struct usb_function *f)
 
 	video_unregister_device(&uvc->vdev);
 	v4l2_device_unregister(&uvc->v4l2_dev);
-	uvc->control_ep->driver_data = NULL;
-	uvc->video.ep->driver_data = NULL;
 
 	usb_ep_free_request(cdev->gadget->ep0, uvc->control_req);
 	kfree(uvc->control_buf);
-- 
1.9.1

[PATCH 23/26] usb: gadget: u_ether: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of u_ether we only need to store in ep->driver_data pointer to
struct eth_dev, as it's used in rx_complete() and tx_complete() callbacks.
All other uses of ep->driver_data are now meaningless and can be safely
removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/u_ether.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index f1fd777..eccde2f 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -1144,7 +1144,6 @@ void gether_disconnect(struct gether *link)
 		spin_lock(&dev->req_lock);
 	}
 	spin_unlock(&dev->req_lock);
-	link->in_ep->driver_data = NULL;
 	link->in_ep->desc = NULL;
 
 	usb_ep_disable(link->out_ep);
@@ -1159,7 +1158,6 @@ void gether_disconnect(struct gether *link)
 		spin_lock(&dev->req_lock);
 	}
 	spin_unlock(&dev->req_lock);
-	link->out_ep->driver_data = NULL;
 	link->out_ep->desc = NULL;
 
 	/* finish forgetting about this USB link episode */
-- 
1.9.1

[PATCH 24/26] usb: gadget: u_serial: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of u_serial ep->driver_data stores pointer to struct gs_port,
which is referenced in many places in code. Code using ep->driver_data
to mark endpoint as enabled/disabled has been removed.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/u_serial.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 7ee05793..9cc6a13 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -876,7 +876,6 @@ static void gs_close(struct tty_struct *tty, struct file *file)
 	else
 		gs_buf_clear(&port->port_write_buf);
 
-	tty->driver_data = NULL;
 	port->port.tty = NULL;
 
 	port->openclose = false;
@@ -1224,7 +1223,6 @@ int gserial_connect(struct gserial *gser, u8 port_num)
 
 fail_out:
 	usb_ep_disable(gser->in);
-	gser->in->driver_data = NULL;
 	return status;
 }
 EXPORT_SYMBOL_GPL(gserial_connect);
@@ -1264,10 +1262,7 @@ void gserial_disconnect(struct gserial *gser)
 
 	/* disable endpoints, aborting down any active I/O */
 	usb_ep_disable(gser->out);
-	gser->out->driver_data = NULL;
-
 	usb_ep_disable(gser->in);
-	gser->in->driver_data = NULL;
 
 	/* finally, free any unused/unusable I/O buffers */
 	spin_lock_irqsave(&port->port_lock, flags);
-- 
1.9.1

[PATCH 25/26] usb: gadget: legacy: dbgp: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of dbgp, ep->driver_data was used only for endpoint claiming
and marking endpoints as enabled, so we can simplify code by reducing
it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/legacy/dbgp.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 5231a32..99ca3da 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -79,10 +79,7 @@ static int dbgp_consume(char *buf, unsigned len)
 
 static void __disable_ep(struct usb_ep *ep)
 {
-	if (ep && ep->driver_data == dbgp.gadget) {
-		usb_ep_disable(ep);
-		ep->driver_data = NULL;
-	}
+	usb_ep_disable(ep);
 }
 
 static void dbgp_disable_ep(void)
@@ -171,7 +168,6 @@ static int __enable_ep(struct usb_ep *ep, struct usb_endpoint_descriptor *desc)
 	int err;
 	ep->desc = desc;
 	err = usb_ep_enable(ep);
-	ep->driver_data = dbgp.gadget;
 	return err;
 }
 
@@ -229,8 +225,6 @@ static void dbgp_unbind(struct usb_gadget *gadget)
 		usb_ep_free_request(gadget->ep0, dbgp.req);
 		dbgp.req = NULL;
 	}
-
-	gadget->ep0->driver_data = NULL;
 }
 
 #ifdef CONFIG_USB_G_DBGP_SERIAL
@@ -249,18 +243,15 @@ static int dbgp_configure_endpoints(struct usb_gadget *gadget)
 		goto fail_1;
 	}
 
-	dbgp.i_ep->driver_data = gadget;
 	i_desc.wMaxPacketSize =
 		cpu_to_le16(USB_DEBUG_MAX_PACKET_SIZE);
 
 	dbgp.o_ep = usb_ep_autoconfig(gadget, &o_desc);
 	if (!dbgp.o_ep) {
-		dbgp.i_ep->driver_data = NULL;
 		stp = 2;
-		goto fail_2;
+		goto fail_1;
 	}
 
-	dbgp.o_ep->driver_data = gadget;
 	o_desc.wMaxPacketSize =
 		cpu_to_le16(USB_DEBUG_MAX_PACKET_SIZE);
 
@@ -277,8 +268,6 @@ static int dbgp_configure_endpoints(struct usb_gadget *gadget)
 
 	return 0;
 
-fail_2:
-	dbgp.i_ep->driver_data = NULL;
 fail_1:
 	dev_dbg(&dbgp.gadget->dev, "ep config: failure (%d)\n", stp);
 	return -ENODEV;
@@ -306,7 +295,6 @@ static int dbgp_bind(struct usb_gadget *gadget,
 	}
 
 	dbgp.req->length = DBGP_REQ_EP0_LEN;
-	gadget->ep0->driver_data = gadget;
 
 #ifdef CONFIG_USB_G_DBGP_SERIAL
 	dbgp.serial = kzalloc(sizeof(struct gserial), GFP_KERNEL);
@@ -356,8 +344,6 @@ static int dbgp_setup(struct usb_gadget *gadget,
 	void *data = NULL;
 	u16 len = 0;
 
-	gadget->ep0->driver_data = gadget;
-
 	if (request == USB_REQ_GET_DESCRIPTOR) {
 		switch (value>>8) {
 		case USB_DT_DEVICE:
-- 
1.9.1

[PATCH 26/26] usb: gadget: legacy: tcm: eliminate abuse of ep->driver data

[Robert Baldyga]

Since ep->driver_data is not used for endpoint claiming, neither for
enabled/disabled state storing, we can reduce number of places where
we read or modify it's value, as now it has no particular meaning for
function or framework logic.

In case of tcm, ep->driver_data was used only for endpoint claiming so
we can simplify code by reducing it. We also remove give_back_ep()
function which is not needed after all - when error code is returned
from bind() function, composite will release all endpoints anyway.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/legacy/tcm_usb_gadget.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/usb/gadget/legacy/tcm_usb_gadget.c b/drivers/usb/gadget/legacy/tcm_usb_gadget.c
index c3c4808..778e42a 100644
--- a/drivers/usb/gadget/legacy/tcm_usb_gadget.c
+++ b/drivers/usb/gadget/legacy/tcm_usb_gadget.c
@@ -2018,14 +2018,6 @@ static struct usb_configuration usbg_config_driver = {
 	.bmAttributes           = USB_CONFIG_ATT_SELFPOWER,
 };
 
-static void give_back_ep(struct usb_ep **pep)
-{
-	struct usb_ep *ep = *pep;
-	if (!ep)
-		return;
-	ep->driver_data = NULL;
-}
-
 static int usbg_bind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct f_uas		*fu = to_f_uas(f);
@@ -2045,29 +2037,24 @@ static int usbg_bind(struct usb_configuration *c, struct usb_function *f)
 			&uasp_bi_ep_comp_desc);
 	if (!ep)
 		goto ep_fail;
-
-	ep->driver_data = fu;
 	fu->ep_in = ep;
 
 	ep = usb_ep_autoconfig_ss(gadget, &uasp_ss_bo_desc,
 			&uasp_bo_ep_comp_desc);
 	if (!ep)
 		goto ep_fail;
-	ep->driver_data = fu;
 	fu->ep_out = ep;
 
 	ep = usb_ep_autoconfig_ss(gadget, &uasp_ss_status_desc,
 			&uasp_status_in_ep_comp_desc);
 	if (!ep)
 		goto ep_fail;
-	ep->driver_data = fu;
 	fu->ep_status = ep;
 
 	ep = usb_ep_autoconfig_ss(gadget, &uasp_ss_cmd_desc,
 			&uasp_cmd_comp_desc);
 	if (!ep)
 		goto ep_fail;
-	ep->driver_data = fu;
 	fu->ep_cmd = ep;
 
 	/* Assume endpoint addresses are the same for both speeds */
@@ -2091,11 +2078,6 @@ static int usbg_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 ep_fail:
 	pr_err("Can't claim all required eps\n");
-
-	give_back_ep(&fu->ep_in);
-	give_back_ep(&fu->ep_out);
-	give_back_ep(&fu->ep_status);
-	give_back_ep(&fu->ep_cmd);
 	return -ENOTSUPP;
 }
 
-- 
1.9.1

Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Krzysztof Opasiak]

Hello,

On 09/15/2015 04:26 PM, Robert Baldyga wrote:
> This patch introduces 'enabled' flag in struct usb_ep, and modifies
> usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
> enabled/disabled state. It helps to avoid enabling endpoints which are
> already enabled, and disabling endpoints which are already disables.
>
>>From now USB functions don't have to remember current endpoint
> enable/disable state, as this state is now handled automatically which
> makes this API less bug-prone.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> ---
>   include/linux/usb/gadget.h | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 3f299e2..63375cd 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -215,6 +215,7 @@ struct usb_ep {
>   	struct list_head	ep_list;
>   	struct usb_ep_caps	caps;
>   	bool			claimed;
> +	bool			enabled;
>   	unsigned		maxpacket:16;
>   	unsigned		maxpacket_limit:16;
>   	unsigned		max_streams:16;
> @@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
>    */
>   static inline int usb_ep_enable(struct usb_ep *ep)
>   {
> -	return ep->ops->enable(ep, ep->desc);
> +	int ret = 0;
> +
> +	if (!ep->enabled) {
> +		ret = ep->ops->enable(ep, ep->desc);
> +		if (!ret)
> +			ep->enabled = true;
> +	}
> +
> +	return ret;
>   }
>
>   /**
> @@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep)
>    */
>   static inline int usb_ep_disable(struct usb_ep *ep)
>   {
> -	return ep->ops->disable(ep);
> +	int ret = 0;
> +
> +	if (ep->enabled) {
> +		ret = ep->ops->disable(ep);
> +		if (!ret)
> +			ep->enabled = false;
> +	}
> +
> +	return ret;
>   }
>

Personally I don't like this convention. In my opinion usb_ep_disable() 
& usb_ep_enable() should fail if ep is already disabled/enabled. Then in 
function code we should check if endpoint is enabled (maybe even we 
should have usb_ep_is_enabled()) and call disable only when it is really 
enabled.

Best Regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Felipe Balbi]

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

On Tue, Sep 15, 2015 at 05:37:27PM +0200, Krzysztof Opasiak wrote:
> Hello,
> 
> On 09/15/2015 04:26 PM, Robert Baldyga wrote:
> >This patch introduces 'enabled' flag in struct usb_ep, and modifies
> >usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
> >enabled/disabled state. It helps to avoid enabling endpoints which are
> >already enabled, and disabling endpoints which are already disables.
> >
> >>From now USB functions don't have to remember current endpoint
> >enable/disable state, as this state is now handled automatically which
> >makes this API less bug-prone.
> >
> >Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> >---
> >  include/linux/usb/gadget.h | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> >diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> >index 3f299e2..63375cd 100644
> >--- a/include/linux/usb/gadget.h
> >+++ b/include/linux/usb/gadget.h
> >@@ -215,6 +215,7 @@ struct usb_ep {
> >  	struct list_head	ep_list;
> >  	struct usb_ep_caps	caps;
> >  	bool			claimed;
> >+	bool			enabled;
> >  	unsigned		maxpacket:16;
> >  	unsigned		maxpacket_limit:16;
> >  	unsigned		max_streams:16;
> >@@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
> >   */
> >  static inline int usb_ep_enable(struct usb_ep *ep)
> >  {
> >-	return ep->ops->enable(ep, ep->desc);
> >+	int ret = 0;
> >+
> >+	if (!ep->enabled) {
> >+		ret = ep->ops->enable(ep, ep->desc);
> >+		if (!ret)
> >+			ep->enabled = true;
> >+	}
> >+
> >+	return ret;
> >  }
> >
> >  /**
> >@@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep)
> >   */
> >  static inline int usb_ep_disable(struct usb_ep *ep)
> >  {
> >-	return ep->ops->disable(ep);
> >+	int ret = 0;
> >+
> >+	if (ep->enabled) {
> >+		ret = ep->ops->disable(ep);
> >+		if (!ret)
> >+			ep->enabled = false;
> >+	}
> >+
> >+	return ret;
> >  }
> >
> 
> Personally I don't like this convention. In my opinion usb_ep_disable() &
> usb_ep_enable() should fail if ep is already disabled/enabled. Then in
> function code we should check if endpoint is enabled (maybe even we should
> have usb_ep_is_enabled()) and call disable only when it is really enabled.

usb_ep_is_enabled() should be a good addition but I don't see an issue
ignoring usb_ep_enabled() for something that's already enabled.

Imagine if you got an error when you tried to push the light switch to
the 'on' position while the light was already on :-p

I do think, though, that this can be simplified by returning early if
already enabled:

usb_ep_enable()
{
	if (ep->enabled)
		return 0;

	return ep->ops->enable(ep, ep->desc);
}

and likewise for usb_ep_disable()

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 05/26] usb: gadget: f_ecm: eliminate abuse of ep->driver data

[Krzysztof Opasiak]

On 09/15/2015 04:26 PM, Robert Baldyga wrote:
> Since ep->driver_data is not used for endpoint claiming, neither for
> enabled/disabled state storing, we can reduce number of places where
> we read or modify it's value, as now it has no particular meaning for
> function or framework logic.
>
> In case of f_ecm, ep->driver_data was used only for endpoint claiming
> and marking endpoints as enabled, so we can simplify code by reducing
> it.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>

( ... )

>
> @@ -820,14 +811,6 @@ fail:
>   		usb_ep_free_request(ecm->notify, ecm->notify_req);
>   	}
>
> -	/* we might as well release our claims on endpoints */
> -	if (ecm->notify)
> -		ecm->notify->driver_data = NULL;
> -	if (ecm->port.out_ep)
> -		ecm->port.out_ep->driver_data = NULL;
> -	if (ecm->port.in_ep)
> -		ecm->port.in_ep->driver_data = NULL;
> -
>   	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
>
>   	return status;
>

You have done this in almost all functions but personally I'm really 
concern about this change.

By convention function should free all allocated resources when exiting 
with non 0 code. Endpoints are some kind of resources, they are 
"allocated" using usb_ep_autoconfig() and if you are not going to use 
them because error occurred you should free the using 
usb_ep_autoconfig_release(). Moreover, you have done this in source sink 
function so why not do this in all other?


Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Robert Baldyga]

On 09/15/2015 05:43 PM, Felipe Balbi wrote:
> On Tue, Sep 15, 2015 at 05:37:27PM +0200, Krzysztof Opasiak wrote:
>> Hello,
>>
>> On 09/15/2015 04:26 PM, Robert Baldyga wrote:
>>> This patch introduces 'enabled' flag in struct usb_ep, and modifies
>>> usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
>>> enabled/disabled state. It helps to avoid enabling endpoints which are
>>> already enabled, and disabling endpoints which are already disables.
>>>
>>> >From now USB functions don't have to remember current endpoint
>>> enable/disable state, as this state is now handled automatically which
>>> makes this API less bug-prone.
>>>
>>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>>> ---
>>>  include/linux/usb/gadget.h | 21 +++++++++++++++++++--
>>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>> index 3f299e2..63375cd 100644
>>> --- a/include/linux/usb/gadget.h
>>> +++ b/include/linux/usb/gadget.h
>>> @@ -215,6 +215,7 @@ struct usb_ep {
>>>  	struct list_head	ep_list;
>>>  	struct usb_ep_caps	caps;
>>>  	bool			claimed;
>>> +	bool			enabled;
>>>  	unsigned		maxpacket:16;
>>>  	unsigned		maxpacket_limit:16;
>>>  	unsigned		max_streams:16;
>>> @@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
>>>   */
>>>  static inline int usb_ep_enable(struct usb_ep *ep)
>>>  {
>>> -	return ep->ops->enable(ep, ep->desc);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep->enabled) {
>>> +		ret = ep->ops->enable(ep, ep->desc);
>>> +		if (!ret)
>>> +			ep->enabled = true;
>>> +	}
>>> +
>>> +	return ret;
>>>  }
>>>
>>>  /**
>>> @@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep)
>>>   */
>>>  static inline int usb_ep_disable(struct usb_ep *ep)
>>>  {
>>> -	return ep->ops->disable(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (ep->enabled) {
>>> +		ret = ep->ops->disable(ep);
>>> +		if (!ret)
>>> +			ep->enabled = false;
>>> +	}
>>> +
>>> +	return ret;
>>>  }
>>>
>>
>> Personally I don't like this convention. In my opinion usb_ep_disable() &
>> usb_ep_enable() should fail if ep is already disabled/enabled. Then in
>> function code we should check if endpoint is enabled (maybe even we should
>> have usb_ep_is_enabled()) and call disable only when it is really enabled.
> 
> usb_ep_is_enabled() should be a good addition but I don't see an issue
> ignoring usb_ep_enabled() for something that's already enabled.
> 
> Imagine if you got an error when you tried to push the light switch to
> the 'on' position while the light was already on :-p
> 
> I do think, though, that this can be simplified by returning early if
> already enabled:
> 
> usb_ep_enable()
> {
> 	if (ep->enabled)
> 		return 0;
> 
> 	return ep->ops->enable(ep, ep->desc);
> }
> 
> and likewise for usb_ep_disable()

We can't do that, because we need to toggle ep->enable flag.

Thanks,
Robert

Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Felipe Balbi]

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

Hi,

On Tue, Sep 15, 2015 at 05:57:53PM +0200, Robert Baldyga wrote:
> On 09/15/2015 05:43 PM, Felipe Balbi wrote:
> > On Tue, Sep 15, 2015 at 05:37:27PM +0200, Krzysztof Opasiak wrote:
> >> Hello,
> >>
> >> On 09/15/2015 04:26 PM, Robert Baldyga wrote:
> >>> This patch introduces 'enabled' flag in struct usb_ep, and modifies
> >>> usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
> >>> enabled/disabled state. It helps to avoid enabling endpoints which are
> >>> already enabled, and disabling endpoints which are already disables.
> >>>
> >>> >From now USB functions don't have to remember current endpoint
> >>> enable/disable state, as this state is now handled automatically which
> >>> makes this API less bug-prone.
> >>>
> >>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> >>> ---
> >>>  include/linux/usb/gadget.h | 21 +++++++++++++++++++--
> >>>  1 file changed, 19 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> >>> index 3f299e2..63375cd 100644
> >>> --- a/include/linux/usb/gadget.h
> >>> +++ b/include/linux/usb/gadget.h
> >>> @@ -215,6 +215,7 @@ struct usb_ep {
> >>>  	struct list_head	ep_list;
> >>>  	struct usb_ep_caps	caps;
> >>>  	bool			claimed;
> >>> +	bool			enabled;
> >>>  	unsigned		maxpacket:16;
> >>>  	unsigned		maxpacket_limit:16;
> >>>  	unsigned		max_streams:16;
> >>> @@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
> >>>   */
> >>>  static inline int usb_ep_enable(struct usb_ep *ep)
> >>>  {
> >>> -	return ep->ops->enable(ep, ep->desc);
> >>> +	int ret = 0;
> >>> +
> >>> +	if (!ep->enabled) {
> >>> +		ret = ep->ops->enable(ep, ep->desc);
> >>> +		if (!ret)
> >>> +			ep->enabled = true;
> >>> +	}
> >>> +
> >>> +	return ret;
> >>>  }
> >>>
> >>>  /**
> >>> @@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep)
> >>>   */
> >>>  static inline int usb_ep_disable(struct usb_ep *ep)
> >>>  {
> >>> -	return ep->ops->disable(ep);
> >>> +	int ret = 0;
> >>> +
> >>> +	if (ep->enabled) {
> >>> +		ret = ep->ops->disable(ep);
> >>> +		if (!ret)
> >>> +			ep->enabled = false;
> >>> +	}
> >>> +
> >>> +	return ret;
> >>>  }
> >>>
> >>
> >> Personally I don't like this convention. In my opinion usb_ep_disable() &
> >> usb_ep_enable() should fail if ep is already disabled/enabled. Then in
> >> function code we should check if endpoint is enabled (maybe even we should
> >> have usb_ep_is_enabled()) and call disable only when it is really enabled.
> > 
> > usb_ep_is_enabled() should be a good addition but I don't see an issue
> > ignoring usb_ep_enabled() for something that's already enabled.
> > 
> > Imagine if you got an error when you tried to push the light switch to
> > the 'on' position while the light was already on :-p
> > 
> > I do think, though, that this can be simplified by returning early if
> > already enabled:
> > 
> > usb_ep_enable()
> > {
> > 	if (ep->enabled)
> > 		return 0;
> > 
> > 	return ep->ops->enable(ep, ep->desc);
> > }
> > 
> > and likewise for usb_ep_disable()
> 
> We can't do that, because we need to toggle ep->enable flag.

man, things have to be spelled out to the last comma... The point was to
avoid the extra identation level

usb_ep_enable()
{
	int ret;

	if (ep->enabled)
		return 0;

	ret = ep->ops->enable(ep, ep->desc);
	if (ret)
		return ret;

	ep->enabled = true;

	return 0;
}

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Krzysztof Opasiak]

On 09/15/2015 05:43 PM, Felipe Balbi wrote:
> On Tue, Sep 15, 2015 at 05:37:27PM +0200, Krzysztof Opasiak wrote:
>> Hello,
>>
>> On 09/15/2015 04:26 PM, Robert Baldyga wrote:
>>> This patch introduces 'enabled' flag in struct usb_ep, and modifies
>>> usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
>>> enabled/disabled state. It helps to avoid enabling endpoints which are
>>> already enabled, and disabling endpoints which are already disables.
>>>
>>> >From now USB functions don't have to remember current endpoint
>>> enable/disable state, as this state is now handled automatically which
>>> makes this API less bug-prone.
>>>
>>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>>> ---
>>>   include/linux/usb/gadget.h | 21 +++++++++++++++++++--
>>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>> index 3f299e2..63375cd 100644
>>> --- a/include/linux/usb/gadget.h
>>> +++ b/include/linux/usb/gadget.h
>>> @@ -215,6 +215,7 @@ struct usb_ep {
>>>   	struct list_head	ep_list;
>>>   	struct usb_ep_caps	caps;
>>>   	bool			claimed;
>>> +	bool			enabled;
>>>   	unsigned		maxpacket:16;
>>>   	unsigned		maxpacket_limit:16;
>>>   	unsigned		max_streams:16;
>>> @@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
>>>    */
>>>   static inline int usb_ep_enable(struct usb_ep *ep)
>>>   {
>>> -	return ep->ops->enable(ep, ep->desc);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep->enabled) {
>>> +		ret = ep->ops->enable(ep, ep->desc);
>>> +		if (!ret)
>>> +			ep->enabled = true;
>>> +	}
>>> +
>>> +	return ret;
>>>   }
>>>
>>>   /**
>>> @@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep)
>>>    */
>>>   static inline int usb_ep_disable(struct usb_ep *ep)
>>>   {
>>> -	return ep->ops->disable(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (ep->enabled) {
>>> +		ret = ep->ops->disable(ep);
>>> +		if (!ret)
>>> +			ep->enabled = false;
>>> +	}
>>> +
>>> +	return ret;
>>>   }
>>>
>>
>> Personally I don't like this convention. In my opinion usb_ep_disable() &
>> usb_ep_enable() should fail if ep is already disabled/enabled. Then in
>> function code we should check if endpoint is enabled (maybe even we should
>> have usb_ep_is_enabled()) and call disable only when it is really enabled.
>
> usb_ep_is_enabled() should be a good addition but I don't see an issue
> ignoring usb_ep_enabled() for something that's already enabled.
>
> Imagine if you got an error when you tried to push the light switch to
> the 'on' position while the light was already on :-p
>

Hmmm not sure right now, didn't test this recently :D as usually I check 
if light isn't already "on" before I touch the switch to turn it on:P

Just joking. Personally I just prefer to don't touch things which are 
already in desired condition. Let's take close() as example which could 
be a little bit equivalent of our usb_ep_disable(). It is not legal to 
call it twice on some fd and second call ends up with error.

Best regards,

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Felipe Balbi]

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

Hi,

On Tue, Sep 15, 2015 at 06:15:25PM +0200, Krzysztof Opasiak wrote:
> >>>+	}
> >>>+
> >>>+	return ret;
> >>>  }
> >>>
> >>
> >>Personally I don't like this convention. In my opinion usb_ep_disable() &
> >>usb_ep_enable() should fail if ep is already disabled/enabled. Then in
> >>function code we should check if endpoint is enabled (maybe even we should
> >>have usb_ep_is_enabled()) and call disable only when it is really enabled.
> >
> >usb_ep_is_enabled() should be a good addition but I don't see an issue
> >ignoring usb_ep_enabled() for something that's already enabled.
> >
> >Imagine if you got an error when you tried to push the light switch to
> >the 'on' position while the light was already on :-p
> >
> 
> Hmmm not sure right now, didn't test this recently :D as usually I check if
> light isn't already "on" before I touch the switch to turn it on:P

not my best analogy :)

> Just joking. Personally I just prefer to don't touch things which are
> already in desired condition. Let's take close() as example which
> could be a little bit equivalent of our usb_ep_disable(). It is not
> legal to call it twice on some fd and second call ends up with error.

I understand that, it's just the difference between adding
usb_ep_is_enabled() to every single gadget/function driver or adding it
to usb_ep_enable() itself to keep gadget/function drivers cleaner.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 05/26] usb: gadget: f_ecm: eliminate abuse of ep->driver data

[Robert Baldyga]

On 09/15/2015 05:45 PM, Krzysztof Opasiak wrote:
> 
> 
> On 09/15/2015 04:26 PM, Robert Baldyga wrote:
>> Since ep->driver_data is not used for endpoint claiming, neither for
>> enabled/disabled state storing, we can reduce number of places where
>> we read or modify it's value, as now it has no particular meaning for
>> function or framework logic.
>>
>> In case of f_ecm, ep->driver_data was used only for endpoint claiming
>> and marking endpoints as enabled, so we can simplify code by reducing
>> it.
>>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> 
> ( ... )
> 
>>
>> @@ -820,14 +811,6 @@ fail:
>>   		usb_ep_free_request(ecm->notify, ecm->notify_req);
>>   	}
>>
>> -	/* we might as well release our claims on endpoints */
>> -	if (ecm->notify)
>> -		ecm->notify->driver_data = NULL;
>> -	if (ecm->port.out_ep)
>> -		ecm->port.out_ep->driver_data = NULL;
>> -	if (ecm->port.in_ep)
>> -		ecm->port.in_ep->driver_data = NULL;
>> -
>>   	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
>>
>>   	return status;
>>
> 
> You have done this in almost all functions but personally I'm really 
> concern about this change.
> 
> By convention function should free all allocated resources when exiting 
> with non 0 code. Endpoints are some kind of resources, they are 
> "allocated" using usb_ep_autoconfig() and if you are not going to use 
> them because error occurred you should free the using 
> usb_ep_autoconfig_release(). Moreover, you have done this in source sink 
> function so why not do this in all other?


Well, if bind() returns an error code, composite calls
usb_ep_autoconfig_reset() anyway, so manual endpoint releasing in error
path isn't necessary. We can compare the way it works with devm_*
functions family - you don't need to care about resource lifetime as
it's managed for you.

The function f_sourcesink is a bit different, because it can work with
two different endpoint configurations {bulk-in, bulk-out} or {bulk-in,
bulk-out, iso-in, iso-out}. When bulk endpoints are obtained
successfully it tries to get iso endpoints, but if one if them is not
available it releases previously obtained iso and ends with success. In
such case it simply starts in bulk-only configuration.

Best regards,
Robert