Messages in this thread | | | Subject | Re: [PATCH] usb: dwc2: host: fix isoc urb actual length | From | wlf <> | Date | Wed, 8 Nov 2017 10:58:08 +0800 |
| |
Hi Alan,
在 2017年11月07日 23:18, Alan Stern 写道: > On Tue, 7 Nov 2017, wlf wrote: > >>> That sounds like a good idea. Minas, does the following patch fix your >>> problem? >>> >>> In theory we could do this calculation for every isochronous URB, not >>> just those coming from usbfs. But I don't think there's any point, >>> since the USB class drivers don't use it. >>> >>> Alan Stern >>> >>> >>> >>> Index: usb-4.x/drivers/usb/core/devio.c >>> =================================================================== >>> --- usb-4.x.orig/drivers/usb/core/devio.c >>> +++ usb-4.x/drivers/usb/core/devio.c >>> @@ -1828,6 +1828,18 @@ static int proc_unlinkurb(struct usb_dev >>> return 0; >>> } >>> >>> +static void compute_isochronous_actual_length(struct urb *urb) >>> +{ >>> + unsigned i; >>> + >>> + if (urb->number_of_packets > 0) { >>> + urb->actual_length = 0; >>> + for (i = 0; i < urb->number_of_packets; i++) >>> + urb->actual_length += >>> + urb->iso_frame_desc[i].actual_length; >>> + } >>> +} >>> + >>> static int processcompl(struct async *as, void __user * __user *arg) >>> { >>> struct urb *urb = as->urb; >>> @@ -1835,6 +1847,8 @@ static int processcompl(struct async *as >>> void __user *addr = as->userurb; >>> unsigned int i; >>> >>> + compute_isochronous_actual_length(urb); >>> + >>> if (as->userbuffer && urb->actual_length) { >>> if (copy_urb_data_to_user(as->userbuffer, urb)) >>> goto err_out; >>> @@ -2003,6 +2017,8 @@ static int processcompl_compat(struct as >>> void __user *addr = as->userurb; >>> unsigned int i; >>> >>> + compute_isochronous_actual_length(urb); >>> + >>> if (as->userbuffer && urb->actual_length) { >>> if (copy_urb_data_to_user(as->userbuffer, urb)) >>> return -EFAULT; >>> >>> >> Yeah, it's a good idea to calculate the urb actual length in the devio >> driver. >> Your patch seems good, and I think we can do a small optimization base >> your patch, >> like the following patch: >> >> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c >> index bd94192..a2e7b97 100644 >> --- a/drivers/usb/core/devio.c >> +++ b/drivers/usb/core/devio.c >> @@ -1664,6 +1664,9 @@ static int processcompl(struct async *as, void >> __user * __user *arg) >> void __user *addr = as->userurb; >> unsigned int i; >> >> + if (usb_endpoint_xfer_isoc(&urb->ep->desc)) >> + compute_isochronous_actual_length(urb); >> + >> if (as->userbuffer && urb->actual_length) { >> if (copy_urb_data_to_user(as->userbuffer, urb)) >> goto err_out; >> @@ -1833,6 +1836,9 @@ static int processcompl_compat(struct async *as, >> void __user * __user *arg) >> void __user *addr = as->userurb; >> unsigned int i; >> >> + if (usb_endpoint_xfer_isoc(&urb->ep->desc)) >> + compute_isochronous_actual_length(urb); >> + > Well, this depends on whether you want to optimize for space or for > speed. I've been going for space. And since usbfs is inherently > rather slow, I don't think this will make any significant speed > difference. So I don't think adding the extra tests is worthwhile. > > (Besides, if you really wanted to do it this way, you should have moved > the test for "urb->number_of_packets > 0" into the callers instead of > adding an additional test of the endpoint type.) Yes, agree with you.
> > However, nobody has answered my original question: Does the patch > actually fix the problem? I test your patch on Rockchip RK3188 platform, use UsbWebCamera APP by Serenegiant (libusb + devio) with usb camera, it work well.
> > Alan Stern > > > >
-- 吴良峰 William.Wu 福建省福州市铜盘路软件大道89号软件园A区21号楼 No.21 Building, A District, No.89,software Boulevard Fuzhou,Fujian, PRC 手机: 13685012275 座机: 0591-83991906-8520 邮件:wulf@rock-chips.com
| |