lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH -next] xhci: fix two places when dealing with return value of function xhci_check_args
    Date
    On 26.1.2022 14.49, Hongyu Xie wrote:

    >> 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.

    Agree, lets return -ENODEV when appropriate.

    >>
    >>> 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).

    xhci_urb_enqueue() shouldn't be called for roothub urbs, but if it is then we
    should continue to return -EINVAL

    xhci_check_args() should be rewritten later, but first we want a targeted fix
    that can go to stable.

    Your original patch would be ok after following modification:
    if (ret <= 0)
    return ret ? ret : -EINVAL;

    Thanks
    -Mathias

    \
     
     \ /
      Last update: 2022-01-27 10:42    [W:2.624 / U:0.332 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site