On Sun, 8 Aug 2021 14:38:57 +1000
Jonathan Matthew <[email protected]> wrote:
> While working on a new driver, I noticed we have a few places where we
> pass USBD_EXCLUSIVE_USE as the flags parameter to
> usbd_open_pipe_intr(), which is wrong.
>
> The interrupt pipe is always opened exclusively, and the flags
> parameter is actually passed to usbd_setup_xfer(), where it means
> USBD_NO_COPY, so any data written by the transfer is not copied to
> the buffer where the driver expects it.
>
> I don't have hardware supported by any these drivers, but most of them
> don't look at the transferred data, and in a couple of them, the
> interrupt pipe code is #if 0'd out, so I think there is little chance
> this changes anything.
>
> ok?
Yes, agree, that's wrong, and it's documented in the according man
pages of usbd_open_pipe_intr(9) and usbd_open_pipe(9) as well. If it
should break one of those drivers because they really need
'USBD_NO_COPY', I would suggest to set that explicitly in
usbd_open_pipe_intr(9). But I doubt.
So OK.
> Index: if_aue.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_aue.c,v
> retrieving revision 1.111
> diff -u -p -u -p -r1.111 if_aue.c
> --- if_aue.c 31 Jul 2020 10:49:32 -0000 1.111
> +++ if_aue.c 8 Aug 2021 03:25:19 -0000
> @@ -1355,7 +1355,7 @@ aue_openpipes(struct aue_softc *sc)
> return (EIO);
> }
> err = usbd_open_pipe_intr(sc->aue_iface,
> sc->aue_ed[AUE_ENDPT_INTR],
> - USBD_EXCLUSIVE_USE, &sc->aue_ep[AUE_ENDPT_INTR], sc,
> + 0, &sc->aue_ep[AUE_ENDPT_INTR], sc,
> &sc->aue_cdata.aue_ibuf, AUE_INTR_PKTLEN, aue_intr,
> AUE_INTR_INTERVAL);
> if (err) {
> Index: if_udav.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_udav.c,v
> retrieving revision 1.84
> diff -u -p -u -p -r1.84 if_udav.c
> --- if_udav.c 31 Jul 2020 10:49:32 -0000 1.84
> +++ if_udav.c 8 Aug 2021 03:25:19 -0000
> @@ -769,7 +769,7 @@ udav_openpipes(struct udav_softc *sc)
> /* XXX: interrupt endpoint is not yet supported */
> /* Open Interrupt pipe */
> err = usbd_open_pipe_intr(sc->sc_ctl_iface, sc->sc_intrin_no,
> - USBD_EXCLUSIVE_USE,
> &sc->sc_pipe_intr, sc,
> + 0, &sc->sc_pipe_intr, sc,
> &sc->sc_cdata.udav_ibuf,
> UDAV_INTR_PKGLEN, udav_intr, UDAV_INTR_INTERVAL);
> if (err) {
> Index: if_ugl.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_ugl.c,v
> retrieving revision 1.26
> diff -u -p -u -p -r1.26 if_ugl.c
> --- if_ugl.c 31 Jul 2020 10:49:32 -0000 1.26
> +++ if_ugl.c 8 Aug 2021 03:25:20 -0000
> @@ -681,7 +681,7 @@ ugl_openpipes(struct ugl_softc *sc)
> return (EIO);
> }
> err = usbd_open_pipe_intr(sc->sc_iface,
> sc->sc_ed[UGL_ENDPT_INTR],
> - USBD_EXCLUSIVE_USE, &sc->sc_ep[UGL_ENDPT_INTR], sc,
> + 0, &sc->sc_ep[UGL_ENDPT_INTR], sc,
> sc->sc_ibuf, UGL_INTR_PKTLEN, ugl_intr,
> UGL_INTR_INTERVAL);
> if (err) {
> Index: if_upl.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_upl.c,v
> retrieving revision 1.78
> diff -u -p -u -p -r1.78 if_upl.c
> --- if_upl.c 31 Jul 2020 10:49:32 -0000 1.78
> +++ if_upl.c 8 Aug 2021 03:25:20 -0000
> @@ -661,7 +661,7 @@ upl_openpipes(struct upl_softc *sc)
> return (EIO);
> }
> err = usbd_open_pipe_intr(sc->sc_iface,
> sc->sc_ed[UPL_ENDPT_INTR],
> - USBD_EXCLUSIVE_USE, &sc->sc_ep[UPL_ENDPT_INTR], sc,
> + 0, &sc->sc_ep[UPL_ENDPT_INTR], sc,
> &sc->sc_ibuf, UPL_INTR_PKTLEN, upl_intr,
> UPL_INTR_INTERVAL);
> if (err) {
> Index: if_url.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_url.c,v
> retrieving revision 1.88
> diff -u -p -u -p -r1.88 if_url.c
> --- if_url.c 31 Jul 2020 10:49:33 -0000 1.88
> +++ if_url.c 8 Aug 2021 03:25:20 -0000
> @@ -635,7 +635,7 @@ url_openpipes(struct url_softc *sc)
> /* XXX: interrupt endpoint is not yet supported */
> /* Open Interrupt pipe */
> err = usbd_open_pipe_intr(sc->sc_ctl_iface, sc->sc_intrin_no,
> - USBD_EXCLUSIVE_USE,
> &sc->sc_pipe_intr, sc,
> + 0, &sc->sc_pipe_intr, sc,
> &sc->sc_cdata.url_ibuf,
> URL_INTR_PKGLEN, url_intr, URL_INTR_INTERVAL);
> if (err) {
> Index: if_wi_usb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_wi_usb.c,v
> retrieving revision 1.73
> diff -u -p -u -p -r1.73 if_wi_usb.c
> --- if_wi_usb.c 31 Jul 2020 10:49:33 -0000 1.73
> +++ if_wi_usb.c 8 Aug 2021 03:25:21 -0000
> @@ -1233,7 +1233,7 @@ wi_usb_open_pipes(struct wi_usb_softc *s
>
> /* is this used? */
> err = usbd_open_pipe_intr(sc->wi_usb_iface,
> - sc->wi_usb_ed[WI_USB_ENDPT_INTR], USBD_EXCLUSIVE_USE,
> + sc->wi_usb_ed[WI_USB_ENDPT_INTR], 0,
> &sc->wi_usb_ep[WI_USB_ENDPT_INTR], sc, &sc->wi_usb_ibuf,
> WI_USB_INTR_PKTLEN, wi_usb_intr, WI_USB_INTR_INTERVAL);
> if (err) {
>
