Re: [PATCH 00/18] Use usb_endpoint_descriptor inside the struct usb_ep

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

> Am Dienstag 22 Mai 2012, 19:53:03 schrieb Felipe Balbi:
>> On Mon, May 21, 2012 at 03:04:21PM +0800, Yu Xu wrote:
>
>> > mv_udc does not work with the patch.
>> > For example, mass storage cannot work over mv_udc. In function
>> > do_set_interface, config_ep_by_speed do "_ep->desc = chosen_desc". But
>> > after that, mv_ep_enable check the ep->ep.desc. It always return with
>> > failure. And in the function mv_ep_enable, it do the "ep->ep.desc =
>> > desc;" again. Can we remove it?
>> > And the issue exists on other controller driver as well.
>>
>> this patch was pending a looooooong time exactly because it was so big.
>> I even asked Ido to miss a merge window so we did things step-by-step.
>>
>> Just send a fix on during the -rc series and I will gladly apply. That's
>> why we have -rc cycle anyway.
>
>
> It seems I've got bitten by the same problem.
>
>> diff --git a/drivers/usb/gadget/s3c-hsudc.c
>> b/drivers/usb/gadget/s3c-hsudc.c index cef9b82..36c6836 100644
>> --- a/drivers/usb/gadget/s3c-hsudc.c
>> +++ b/drivers/usb/gadget/s3c-hsudc.c
>> @@ -761,7 +760,7 @@ static int s3c_hsudc_ep_enable(struct usb_ep *_ep,
>>  	u32 ecr = 0;
>>
>>  	hsep = our_ep(_ep);
>> -	if (!_ep || !desc || hsep->desc || _ep->name == ep0name
>> +	if (!_ep || !desc || hsep->ep.desc || _ep->name == ep0name
>
> In the s3c-hsudc driver the hsep->ep.desc gets set to something here and
> to
> NULL in disable_ep and initep. (the initep stuff runs
>
> But everytime a enable_ep reaches this check hsep->ep.desc is already set
> to
> something. If I remove the hsep->ep.desc check, the gadget of course
> works.
>
> So, either some other parts meddle with the ep.desc or something is really
> fishy in the driver.
>
> Any pointers in the right direction would be greatly appriciated.

The config_ep_by_speed sets the endpoint descriptor before enabling the
endpoint, so indeed the ep.desc will not be null.
The next step is to remove the "desc" argument from the enable function...

The correct fix is just to remove the check above, as you did.
This should also be applied to other UDCs, I will re-patch fixes.
Sorry for the inconvenience, but we were waiting for ACKs on this before
applaying... ;)

>
> Thanks
> Heiko
>

Thanks,
Ido
-- 
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux