Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
On 08/20/2015 06:48 PM, Felipe Balbi wrote: On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote: Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; - ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. I was considering the same thing, but the lifetime of -claimed doesn't look correct to me either way. Note that once the flag is enabled, it won't get disabled by most gadget drivers. And it should not be. This flag is indicator, that endpoint is used by some function. It should be set once by usb_ep_autoconfig() and cleared by usb_ep_autoconfig_reset(). I wonder what is reason of this enable/disable regression. Maybe the problem is that we don't set ep-driver_data to NULL in usb_ep_autoconfig_reset() (so far it was done). Does this problem occur while gadget is binded to UDC for the first time, or at any next time? Unfortunately at this moment I don't have access to my hardware, so it will take a moment before I will setup some testing environment. Thanks, Robert -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
On Thu, Aug 20, 2015 at 10:51:46PM +0200, Robert Baldyga wrote: On 08/20/2015 10:23 PM, Alan Stern wrote: [CC: list drastically trimmed] On Thu, 20 Aug 2015, Robert Baldyga wrote: On 08/20/2015 06:48 PM, Felipe Balbi wrote: On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote: Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; - ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. I was considering the same thing, but the lifetime of -claimed doesn't look correct to me either way. Note that once the flag is enabled, it won't get disabled by most gadget drivers. And it should not be. This flag is indicator, that endpoint is used by some function. It should be set once by usb_ep_autoconfig() and cleared by usb_ep_autoconfig_reset(). I wonder what is reason of this enable/disable regression. Maybe the problem is that we don't set ep-driver_data to NULL in usb_ep_autoconfig_reset() (so far it was done). Does this problem occur while gadget is binded to UDC for the first time, or at any next time? Unfortunately at this moment I don't have access to my hardware, so it will take a moment before I will setup some testing environment. To understand this, you have to think back over the history. Originally there was no composite gadget driver; each function was its own gadget. The driver would allocate endpoints only once, when starting up. ep-claimed was used for telling whether or not ep had , already been allocated, not for whether ep was enabled or anything like that (none of the endpoints were enabled at this stage). The only thing the driver had to be careful about was clearing all the -claimed flags initially, in case some other gadget driver had been loaded earlier and left the flags set. Things are different now with the composite driver. I don't know the details of how it works, but some things are clear. One function driver must not be allowed to interfere with the endpoints allocated by another. A function driver can't allocate all the endpoints it needs for all configs in one go; instead the configs have to be handled independently and each function belonging to the config must allocate all the endpoints it needs before another config is handled. The only time usb_ep_autoconfig_reset() should be called is when the composite core is ready to start allocating endpoints for a new config. At any rate, I don't see how ep-claimed is related in any way to whether or not the endpoint is enabled. Claiming endpoints takes place when the physical endpoints are allocated for use as the functions' logical endpoints. This has to happen before the host reads the config descriptors -- before any endpoints get enabled. Thats right. My intention was to introduce flag which can be used only for marking endpoints as claimed during functions bind. It should definitely not have anything to do with endpoint enable/disable. It looks like before ep-claimed flag was added, ep-driver_data was used for both, marking endpoint as claimed during bind and marking endpoints as enabled/disabled after gadget enumeration. My patch slightly modified ep-driver_data handling during bind/unbind process,
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
Hi, On Thu, Aug 20, 2015 at 07:16:48PM +0200, Robert Baldyga wrote: On 08/20/2015 06:48 PM, Felipe Balbi wrote: On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote: Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; - ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. I was considering the same thing, but the lifetime of -claimed doesn't look correct to me either way. Note that once the flag is enabled, it won't get disabled by most gadget drivers. And it should not be. This flag is indicator, that endpoint is used by some function. It should be set once by usb_ep_autoconfig() and cleared by usb_ep_autoconfig_reset(). have you considered switching interfaces and/or alternate settings ? I wonder what is reason of this enable/disable regression. Maybe the problem is that we don't set ep-driver_data to NULL in usb_ep_autoconfig_reset() (so far it was done). Does this problem occur while gadget is binded to UDC for the first time, or at any next time? Unfortunately at this moment I don't have access to my hardware, so it will take a moment before I will setup some testing environment. yeah, that's okay. We've got time. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
On 08/20/2015 10:23 PM, Alan Stern wrote: [CC: list drastically trimmed] On Thu, 20 Aug 2015, Robert Baldyga wrote: On 08/20/2015 06:48 PM, Felipe Balbi wrote: On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote: Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; - ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. I was considering the same thing, but the lifetime of -claimed doesn't look correct to me either way. Note that once the flag is enabled, it won't get disabled by most gadget drivers. And it should not be. This flag is indicator, that endpoint is used by some function. It should be set once by usb_ep_autoconfig() and cleared by usb_ep_autoconfig_reset(). I wonder what is reason of this enable/disable regression. Maybe the problem is that we don't set ep-driver_data to NULL in usb_ep_autoconfig_reset() (so far it was done). Does this problem occur while gadget is binded to UDC for the first time, or at any next time? Unfortunately at this moment I don't have access to my hardware, so it will take a moment before I will setup some testing environment. To understand this, you have to think back over the history. Originally there was no composite gadget driver; each function was its own gadget. The driver would allocate endpoints only once, when starting up. ep-claimed was used for telling whether or not ep had already been allocated, not for whether ep was enabled or anything like that (none of the endpoints were enabled at this stage). The only thing the driver had to be careful about was clearing all the -claimed flags initially, in case some other gadget driver had been loaded earlier and left the flags set. Things are different now with the composite driver. I don't know the details of how it works, but some things are clear. One function driver must not be allowed to interfere with the endpoints allocated by another. A function driver can't allocate all the endpoints it needs for all configs in one go; instead the configs have to be handled independently and each function belonging to the config must allocate all the endpoints it needs before another config is handled. The only time usb_ep_autoconfig_reset() should be called is when the composite core is ready to start allocating endpoints for a new config. At any rate, I don't see how ep-claimed is related in any way to whether or not the endpoint is enabled. Claiming endpoints takes place when the physical endpoints are allocated for use as the functions' logical endpoints. This has to happen before the host reads the config descriptors -- before any endpoints get enabled. Thats right. My intention was to introduce flag which can be used only for marking endpoints as claimed during functions bind. It should definitely not have anything to do with endpoint enable/disable. It looks like before ep-claimed flag was added, ep-driver_data was used for both, marking endpoint as claimed during bind and marking endpoints as enabled/disabled after gadget enumeration. My patch slightly modified ep-driver_data handling during bind/unbind process, and that is probably cause of problems with enable/disable. However using ep-driver_data field to marking endpoint as enabled/disabled is not the most fortunate solution.
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
[CC: list drastically trimmed] On Thu, 20 Aug 2015, Robert Baldyga wrote: On 08/20/2015 06:48 PM, Felipe Balbi wrote: On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote: Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; - ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. I was considering the same thing, but the lifetime of -claimed doesn't look correct to me either way. Note that once the flag is enabled, it won't get disabled by most gadget drivers. And it should not be. This flag is indicator, that endpoint is used by some function. It should be set once by usb_ep_autoconfig() and cleared by usb_ep_autoconfig_reset(). I wonder what is reason of this enable/disable regression. Maybe the problem is that we don't set ep-driver_data to NULL in usb_ep_autoconfig_reset() (so far it was done). Does this problem occur while gadget is binded to UDC for the first time, or at any next time? Unfortunately at this moment I don't have access to my hardware, so it will take a moment before I will setup some testing environment. To understand this, you have to think back over the history. Originally there was no composite gadget driver; each function was its own gadget. The driver would allocate endpoints only once, when starting up. ep-claimed was used for telling whether or not ep had already been allocated, not for whether ep was enabled or anything like that (none of the endpoints were enabled at this stage). The only thing the driver had to be careful about was clearing all the -claimed flags initially, in case some other gadget driver had been loaded earlier and left the flags set. Things are different now with the composite driver. I don't know the details of how it works, but some things are clear. One function driver must not be allowed to interfere with the endpoints allocated by another. A function driver can't allocate all the endpoints it needs for all configs in one go; instead the configs have to be handled independently and each function belonging to the config must allocate all the endpoints it needs before another config is handled. The only time usb_ep_autoconfig_reset() should be called is when the composite core is ready to start allocating endpoints for a new config. At any rate, I don't see how ep-claimed is related in any way to whether or not the endpoint is enabled. Claiming endpoints takes place when the physical endpoints are allocated for use as the functions' logical endpoints. This has to happen before the host reads the config descriptors -- before any endpoints get enabled. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
Hi,
On Fri, Jul 31, 2015 at 04:00:13PM +0200, Robert Baldyga wrote:
So far it was necessary for usb functions to set ep-driver_data in
endpoint obtained from autoconfig to non-null value, to indicate that
endpoint is claimed by function (in autoconfig it was checked if endpoint
has set this field to non-null value, and if it has, it was assumed that
it is claimed). It could cause bugs because if some function doesn't
set this field autoconfig could return the same endpoint more than one
time.
To help to avoid such bugs this patch adds claimed flag to struct usb_ep,
and encapsulates endpoint claiming mechanism inside usb_ep_autoconfig_ss()
and usb_ep_autoconfig_reset(), so now usb functions don't need to perform
any additional actions to mark endpoint obtained from autoconfig as claimed.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.
I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi ba...@ti.com
Date: Wed Aug 19 18:05:27 2015 -0500
usb: gadget: fix ep-claimed lifetime
In order to fix a regression introduced by commit
cc476b42a39d (usb: gadget: encapsulate endpoint
claiming mechanism) we have to introduce a simple
helper to check if a particular is enabled or not.
After that, we need to move ep-claimed lifetime to
usb_ep_enable() and usb_ep_disable() since those
are the only functions which actually enable and
disable endpoints.
A follow-up patch will come to drop all driver_data
checks from function drivers, since those are, now,
pointless.
Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint
claiming mechanism)
Cc: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 978435a51038..ad45070cd76f 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -126,7 +126,6 @@ found_ep:
ep-address = desc-bEndpointAddress;
ep-desc = NULL;
ep-comp_desc = NULL;
- ep-claimed = true;
return ep;
}
EXPORT_SYMBOL_GPL(usb_ep_autoconfig_ss);
@@ -182,11 +181,6 @@ EXPORT_SYMBOL_GPL(usb_ep_autoconfig);
*/
void usb_ep_autoconfig_reset (struct usb_gadget *gadget)
{
- struct usb_ep *ep;
-
- list_for_each_entry (ep, gadget-ep_list, ep_list) {
- ep-claimed = false;
- }
gadget-in_epnum = 0;
gadget-out_epnum = 0;
}
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c14a69b36d27..9b3d60c1cf9f 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -243,6 +243,22 @@ static inline void usb_ep_set_maxpacket_limit(struct
usb_ep *ep,
}
/**
+ * usb_ep_enabled - is endpoint enabled ?
+ * @ep: the endpoint being checked. may not be the endpoint named ep0.
+ *
+ * Whenever a function driver wants to check if a particular endpoint is
+ * enabled or not, it must check using this helper function. This will
+ * encapsulate details about how the endpoint is checked, saving the function
+ * driver from using private methods for doing so.
+ *
+ * return true if endpoint is enabled, false otherwise.
+ */
+static inline bool usb_ep_enabled(struct usb_ep *ep)
+{
+ return ep-claimed;
+}
+
+/**
* usb_ep_enable - configure endpoint, making it usable
* @ep:the endpoint being configured. may not be the endpoint named ep0.
* drivers discover endpoints through the ep_list of a usb_gadget.
@@ -264,7 +280,18 @@ 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;
+
+ if (usb_ep_enabled(ep))
+ return 0;
+
+ ret = ep-ops-enable(ep, ep-desc);
+ if (ret)
+ return ret;
+
+ ep-claimed = true;
+
+ return 0;
}
/**
@@ -281,7 +308,18 @@ 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;
+
+ if (!usb_ep_enabled(ep))
+ return 0;
+
+ ret = ep-ops-disable(ep);
+ if (ret)
+ return ret;
+
+ ep-claimed = false;
+
+ return 0;
}
/**
[ 73.290345] WARNING: CPU: 0 PID: 300 at lib/kobject.c:240
kobject_add_internal+0x25c/0x2d8()
[ 73.299172] kobject_add_internal failed for ep_81 with -EEXIST, don't try to
register things with the same name in the same directory.
[ 73.311825] Modules linked in: usbtest usb_f_ss_lb g_zero libcomposite
xhci_plat_hcd xhci_hcd usbcore joydev dwc3 udc_core usb_common m25p80
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; - ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. Thanks, Robert -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/46] usb: gadget: encapsulate endpoint claiming mechanism
On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote: Hi Felipe, On 08/20/2015 05:35 PM, Felipe Balbi wrote: [...] just letting you know that this regresses all gadget drivers making them try to disable previously disabled endpoints and enable previously enabled endpoints. I have a possible fix (see below) but then it shows a problem on the host side when using with g_zero (see further below): commit 3b8932100aacb6cfbffe288ca93025d8b8430c00 Author: Felipe Balbi ba...@ti.com Date: Wed Aug 19 18:05:27 2015 -0500 usb: gadget: fix ep-claimed lifetime In order to fix a regression introduced by commit cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) we have to introduce a simple helper to check if a particular is enabled or not. After that, we need to move ep-claimed lifetime to usb_ep_enable() and usb_ep_disable() since those are the only functions which actually enable and disable endpoints. A follow-up patch will come to drop all driver_data checks from function drivers, since those are, now, pointless. Fixes: cc476b42a39d (usb: gadget: encapsulate endpoint claiming mechanism) Cc: Robert Baldyga r.bald...@samsung.com Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 978435a51038..ad45070cd76f 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -126,7 +126,6 @@ found_ep: ep-address = desc-bEndpointAddress; ep-desc = NULL; ep-comp_desc = NULL; -ep-claimed = true; Removing this line causes autoconfig can return the same endpoint many times. This probably causes problems with g_zero. I will try to fix it ASAP. I was considering the same thing, but the lifetime of -claimed doesn't look correct to me either way. Note that once the flag is enabled, it won't get disabled by most gadget drivers. -- balbi signature.asc Description: Digital signature
