On Thu, May 26, 2016 at 10:55:47PM +0200, Marcus Glocker wrote:
> On Tue, May 17, 2016 at 05:15:51PM -0700, patrick keshishian wrote:
>
> > Greetings,
> >
> > I have been looking at uvideo trying to model a new driver I'm
> > attempting to port over and found a few issues (or what I precive
> > as issues).
> >
> > Since the list likes separate diffs for easier discussion, Here
> > is my attempt to break them up in four emails. I think, with
> > exception of one, all should apply and compile individually.
> >
> > Here are description of patches in decreasing order of my
> > confidence in proposing them:
> >
> > 1/4: Incorrect enum used for v4l2_buf.flags.
> > This is a paste error I believe. Very simple diff
> >
> > 2/4: Assumption on endpoint index to use in uvideo_vs_open() vs
> > actual saved endpoint address.
>
> Makes sense. I also would change the following DPRINTF text to
> remove 'sc->sc_vs_cur->endpoint' since this will be same as
> 'ed->bEndpointAddress' and also remove 'sc->sc_vs_cur->psize'
> since this information is already printed in uvideo_vs_set_alt().
As long as it is understood that the actually packet size is
not the literal value of wMaxPacketSize, rather:
psize = UGETW(ed->wMaxPacketSize);
psize = UE_GET_SIZE(psize) * (1 + UE_GET_TRANS(psize));
Fancy stuff this USB.
--patrick
> I think this check was in from the very beginning before we had
> implemented a proper set alternate interface function.
>
>
> Index: uvideo.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> retrieving revision 1.187
> diff -u -p -u -p -r1.187 uvideo.c
> --- uvideo.c 26 May 2016 04:47:08 -0000 1.187
> +++ uvideo.c 26 May 2016 20:38:16 -0000
> @@ -1817,18 +1817,17 @@ uvideo_vs_open(struct uvideo_softc *sc)
> return (error);
> }
>
> - ed = usbd_interface2endpoint_descriptor(sc->sc_vs_cur->ifaceh, 0);
> + ed = usbd_get_endpoint_descriptor(sc->sc_vs_cur->ifaceh,
> + sc->sc_vs_cur->endpoint);
> if (ed == NULL) {
> printf("%s: no endpoint descriptor for VS iface\n",
> DEVNAME(sc));
> return (USBD_INVAL);
> }
> DPRINTF(1, "%s: open pipe for ", DEVNAME(sc));
> - DPRINTF(1, "bEndpointAddress=0x%02x (0x%02x), wMaxPacketSize=%d (%d)\n",
> + DPRINTF(1, "bEndpointAddress=0x%02x, wMaxPacketSize=%d\n",
> ed->bEndpointAddress,
> - sc->sc_vs_cur->endpoint,
> - UGETW(ed->wMaxPacketSize),
> - sc->sc_vs_cur->psize);
> + UGETW(ed->wMaxPacketSize));
>
> error = usbd_open_pipe(
> sc->sc_vs_cur->ifaceh,
>
