From: Stuart Henderson <stu@spacehopper.org>
Subject: Re: Support Jabra PanaCast 20 webcam
To: me@xha.li, tech@openbsd.org, mglocker@openbsd.org
Date: Wed, 27 Nov 2024 18:09:47 +0000

On 2024/11/27 17:54, Kirill A. Korinsky wrote:
> On Wed, 27 Nov 2024 17:10:34 +0100,
> "Lorenz (xha)" <me@xha.li> wrote:
> > 
> > hi Kirill,
> > 
> > On Tue, Nov 26, 2024 at 11:51:56PM +0100, Kirill A. Korinsky wrote:
> > > tech@,
> > > 
> > > I had figured out why the Jabra PanaCast 20 wouldn't start streaming. It
> > > needs uvideo_vs_negotiation between usbd_set_interface and usbd_open_pipe.
> > > Once I'd added it, it just started working.
> > > 
> > > Putting it before usbd_set_interface or after usbd_open_pipe doesn't help.
> > > 
> > > I am not sure if I should enable it only for this webcam via quirks or keep
> > > it for everything. I had tested on Azurewave, USB camera which is embedded
> > > in my laptop, this diff doesn't affect it and it continues to work as if
> > > nothing had changed.
> > > 
> > > I guess it might fix some webcams that don't work today.
> > > 
> > > 
> > > Feedback? Tests? Ok?
> > >
> 
> +mglocker@ to CC
> 
> > > +	/* do device negotiation witht commit after switch interface */
> > 
> > there's a typo here :)
> >
> 
> Here a version with a bit better wording.
> 
> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
> diff -u -p -r1.223 uvideo.c
> --- sys/dev/usb/uvideo.c	27 Nov 2024 11:37:23 -0000	1.223
> +++ sys/dev/usb/uvideo.c	27 Nov 2024 16:50:14 -0000
> @@ -1899,6 +1899,12 @@ uvideo_vs_open(struct uvideo_softc *sc)
>  		return (USBD_INVAL);
>  	}
>  
> +	/* do device negotiation with commit after set alternate interface */

would "renegotiate after setting alternate interface" make sense?

> +	error = uvideo_vs_negotiation(sc, 1);
> +	if (error != USBD_NORMAL_COMPLETION)
> +		printf("%s: could not negotiate after setting alternate interface!\n",
> +		    DEVNAME(sc));
> +
>  	DPRINTF(1, "%s: open pipe for bEndpointAddress=0x%02x\n",
>  	    DEVNAME(sc), sc->sc_vs_cur->endpoint);

please stick to <80 columns (either shorter text, or split the
string across two lines)

should that return (error) too?

>  	error = usbd_open_pipe(
> 
> -- 
> wbr, Kirill
>