On Mon, 11 Dec 2017, Shuah Khan wrote:
> On 12/11/2017 09:58 AM, Alan Stern wrote:
> > The URB_NO_FSBR flag has never really been used. It was introduced as
> > a potential way for UHCI to minimize PCI bus usage (by not attempting
> > full-speed bulk and control transfers more than once per frame), but
> > the flag was not set by any drivers.
> >
> > There's no point in keeping it around. This patch simplifies the API
> > by removing it. Unfortunately, it does have to be kept as part of the
> > usbfs ABI, but at least we can document in
> > include/uapi/linux/usbdevice_fs.h that it doesn't do anything.
> >
> > Signed-off-by: Alan Stern <[email protected]>
> >
> > ---
> >
> > Shuah, I'm not sure if this will cause an interoperability problem for
> > usbip. In theory, a machine running an older kernel might set the
> > URB_NO_FSBR flag when communicating with a new kernel, which would
> > cause an error. But I don't know any reason why even an old kernel
> > would ever set the flag.
>
> Alan, I am not sure how the interoperability will manifest.
I was worried that a newer kernel might get an URB from an older kernel
that had the NO_FSBR bit set. The newer kernel wouldn't understand
the bit and might reject the URB because of it.
> > Index: usb-4.x/drivers/usb/host/uhci-q.c
> > ===================================================================
> > --- usb-4.x.orig/drivers/usb/host/uhci-q.c
> > +++ usb-4.x/drivers/usb/host/uhci-q.c
> > @@ -73,8 +73,7 @@ static void uhci_add_fsbr(struct uhci_hc
> > {
> > struct urb_priv *urbp = urb->hcpriv;
> >
> > - if (!(urb->transfer_flags & URB_NO_FSBR))
> > - urbp->fsbr = 1;
> > + urbp->fsbr = 1;
>
> So essentially fsbr gets set always. URB_NO_FSBR has no effect.
Correct.
> > Index: usb-4.x/drivers/usb/usbip/stub_rx.c
> > ===================================================================
> > --- usb-4.x.orig/drivers/usb/usbip/stub_rx.c
> > +++ usb-4.x/drivers/usb/usbip/stub_rx.c
> > @@ -412,9 +412,6 @@ static void masking_bogus_flags(struct u
> > if (is_out)
> > allowed |= URB_ZERO_PACKET;
> > /* FALLTHROUGH */
> > - case USB_ENDPOINT_XFER_CONTROL:
> > - allowed |= URB_NO_FSBR; /* only affects UHCI */
> > - /* FALLTHROUGH */
>
> usbip doesn't really do anything special for URB_NO_FSBR, other
> than just setting the flag here. This is followed by usb_submit_urb().
> Probably there is no need to set URB_NO_FSBR in USBIP to begin with,
> considering usb_submit_urb() handled that before this patch.
Agreed, I can't think of any reasonable way this would happen.
> I think it is safe to make the change.
>
> Acked-by: Shuah Khan <[email protected]>
Okay, thanks.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
