Patchwork [U-Boot] USB-CDC: wrong ep status used

login
register
mail settings
Submitter Stefano Babic
Date Aug. 11, 2010, 9:41 p.m.
Message ID <1281562865-6586-1-git-send-email-sbabic@denx.de>
Download mbox | patch
Permalink /patch/71809/
State Superseded
Headers show

Comments

Stefano Babic - Aug. 11, 2010, 9:41 p.m.
In case a status ep is requested, it is always allocated
a request for the ep0, instead of the correct one saved
in the dev structure.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 drivers/usb/gadget/ether.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Vitaly Kuzmichev - Aug. 12, 2010, 10:13 a.m.
Hi Stefano,

On 08/12/2010 01:41 AM, Stefano Babic wrote:
>  #if defined(DEV_CONFIG_CDC)
>  	if (dev->status_ep) {
> -		dev->stat_req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
> -		dev->stat_req->buf = status_req;
> +		dev->stat_req = usb_ep_alloc_request(dev->status_ep, GFP_KERNEL);
>  		if (!dev->stat_req) {
>  			dev->stat_req->buf=NULL;
We get oops here!

> -			usb_ep_free_request (gadget->ep0, dev->req);
> +			usb_ep_free_request (dev->status_ep, dev->req);
>  
>  			goto fail;
>  		}
> +		dev->stat_req->buf = status_req;
>  		dev->stat_req->context = NULL;
>  	}
>  #endif
Stefano Babic - Aug. 12, 2010, 12:28 p.m.
Vitaly Kuzmichev wrote:
> Hi Stefano,
> 
> On 08/12/2010 01:41 AM, Stefano Babic wrote:
>>  #if defined(DEV_CONFIG_CDC)
>>  	if (dev->status_ep) {
>> -		dev->stat_req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
>> -		dev->stat_req->buf = status_req;
>> +		dev->stat_req = usb_ep_alloc_request(dev->status_ep, GFP_KERNEL);
>>  		if (!dev->stat_req) {
>>  			dev->stat_req->buf=NULL;
> We get oops here!

Agree, and the issue is not related to this patch, I missed to correct
it, thanks. If no one complains, I will send a single patch to fix both
problems (wrong ep status + null pointer access).

Best regards,
Stefano
Sergei Shtylyov - Aug. 12, 2010, 5:05 p.m.
Hello.

Stefano Babic wrote:

>> On 08/12/2010 01:41 AM, Stefano Babic wrote:
>>>  #if defined(DEV_CONFIG_CDC)
>>>  	if (dev->status_ep) {
>>> -		dev->stat_req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
>>> -		dev->stat_req->buf = status_req;
>>> +		dev->stat_req = usb_ep_alloc_request(dev->status_ep, GFP_KERNEL);
>>>  		if (!dev->stat_req) {
>>>  			dev->stat_req->buf=NULL;
>> We get oops here!

> Agree, and the issue is not related to this patch, I missed to correct
> it, thanks. If no one complains, I will send a single patch to fix both
> problems (wrong ep status + null pointer access).

    Looks like Vitaly has done the same already:

http://lists.denx.de/pipermail/u-boot/2010-August/075419.html

> Best regards,
> Stefano

WBR, Sergei
Vitaly Kuzmichev - Aug. 13, 2010, 9:16 a.m.
Hi Sergei,

On 08/12/2010 09:05 PM, Sergei Shtylyov wrote:
> Hello.
> 
> Stefano Babic wrote:
> 
>>> We get oops here!
> 
>> Agree, and the issue is not related to this patch, I missed to correct
>> it, thanks. If no one complains, I will send a single patch to fix both
>> problems (wrong ep status + null pointer access).
> 
>    Looks like Vitaly has done the same already:
> 
Yes, but I have not seen Stefano's comment when sent.

Stefano,
I would like to add you 'signed-off' in this patch because I took this
part from your patch:
-			usb_ep_free_request (gadget->ep0, dev->req);
+			usb_ep_free_request (dev->status_ep, dev->req);

Do you allow me to add your signature?
Stefano Babic - Aug. 13, 2010, 9:19 a.m.
Vitaly Kuzmichev wrote:
> Hi Sergei,
> 
> On 08/12/2010 09:05 PM, Sergei Shtylyov wrote:
>> Hello.
>>
>> Stefano Babic wrote:
>>
>>>> We get oops here!
>>> Agree, and the issue is not related to this patch, I missed to correct
>>> it, thanks. If no one complains, I will send a single patch to fix both
>>> problems (wrong ep status + null pointer access).
>>    Looks like Vitaly has done the same already:
>>
> Yes, but I have not seen Stefano's comment when sent.
> 
> Stefano,
> I would like to add you 'signed-off' in this patch because I took this
> part from your patch:
> -			usb_ep_free_request (gadget->ep0, dev->req);
> +			usb_ep_free_request (dev->status_ep, dev->req);
> 
> Do you allow me to add your signature?
> 

Yes, of course !

Stefano

Patch

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 9f9b093..1481d76 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1731,14 +1731,14 @@  autoconf_fail:
 	/* ... and maybe likewise for status transfer */
 #if defined(DEV_CONFIG_CDC)
 	if (dev->status_ep) {
-		dev->stat_req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
-		dev->stat_req->buf = status_req;
+		dev->stat_req = usb_ep_alloc_request(dev->status_ep, GFP_KERNEL);
 		if (!dev->stat_req) {
 			dev->stat_req->buf=NULL;
-			usb_ep_free_request (gadget->ep0, dev->req);
+			usb_ep_free_request (dev->status_ep, dev->req);
 
 			goto fail;
 		}
+		dev->stat_req->buf = status_req;
 		dev->stat_req->context = NULL;
 	}
 #endif