Messages in this thread | | | Date | Wed, 26 Jan 2022 20:49:18 +0800 | Subject | Re: [PATCH -next] xhci: fix two places when dealing with return value of function xhci_check_args | From | Hongyu Xie <> |
| |
Hi Greg,
On 2022/1/26 18:50, Greg KH wrote: > A: http://en.wikipedia.org/wiki/Top_post > Q: Were do I find info about this thing called top-posting? > A: Because it messes up the order in which people normally read text. > Q: Why is top-posting such a bad thing? > A: Top-posting. > Q: What is the most annoying thing in e-mail? > > A: No. > Q: Should I include quotations after my reply? > > http://daringfireball.net/2007/07/on_top > > On Wed, Jan 26, 2022 at 06:22:45PM +0800, 谢泓宇 wrote: >> 1."What problem? >> r8152_submit_rx needs to detach netdev if -ENODEV happened, but -ENODEV will >> never happen >> because xhci_urb_enqueue only returns -EINVAL if the return value of >> xhci_check_args <= 0. So >> r8152_submit_rx will will call napi_schedule to re-submit that urb, and this >> will cause infinite urb >> submission. > Odd line-wrapping... Sorry about my last reply. > > Anyway, why is this unique to this one driver? Why does it not show up > for any other driver? The whole thing is not about a particular driver. The thing is xhci_urb_enqueue shouldn't change the return value of xhci_check_args from -ENODEV to -EINVAL. Many other drivers only check if the return value of xchi_check_args is <= 0. > >> The whole point is, if xhci_check_args returns value A, xhci_urb_enqueque >> shouldn't return any >> other value, because that will change some driver's behavior(like r8152.c). > But you are changing how the code currently works. Are you sure you > want to have this "succeed" if this is on a root hub? Yes, I'm changing how the code currently works but not on a root hub. > >> 2."So if 0 is returned, you will now return that here, is that ok? >> That is a change in functionality. >> But this can only ever be the case for a root hub, is that ok?" >> >> It's the same logic, but now xhci_urb_enqueue can return -ENODEV if xHC is >> halted. >> If it happens on a root hub, xhci_urb_enqueue won't be called. >> >> 3."Again, this means all is good? Why is this being called for a root hub?" >> >> It is the same logic with the old one, but now xhci_check_streams_endpoint >> can return -ENODEV if xHC is halted. > This still feels wrong to me, but I'll let the maintainer decide, as I > don't understand why a root hub is special here.
Thanks please. usb_submit_urb will call usb_hcd_submit_urb. And usb_hcd_submit_urb will call rh_urb_enqueue if it's on a root hub instead of calling hcd->driver->urb_enqueue(which is xhci_urb_enqueue in this case).
> > thanks, > > greg k-h
thanks,
Hongyu Xie
| |