> Date: Mon, 5 Apr 2021 23:15:23 +0200
> From: Marcus Glocker <mar...@nazgul.ch>
>
> On Mon, Apr 05, 2021 at 07:30:43AM -0700, Greg Steuck wrote:
>
> > OK gnezdo with a usability question inline.
>
> Thanks. See below.
>
> > Marcus Glocker <mar...@nazgul.ch> writes:
> >
> > > martijn@ has recently reported that in his machine he has two cams
> > > of which one is doing IR, which isn't really supported by uvideo(4).
> > > This IR device attaches always first as uvideo0, so he needs to swap
> > > that regularly with his working cam which by default attaches to uvideo1.
> > >
> > > I came up with a new quirk flag to *not* attach certain devices. Tested
> > > successfully by martijn@, the IR cam attaches to ugen0 and the supported
> > > cam to uvideo0.
> > >
> > > This patch shouldn't affect any supported uvideo(4) devices.
> > >
> > > OK?
> > >
> > > Index: sys/dev/usb/uvideo.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> > > retrieving revision 1.211
> > > diff -u -p -u -p -r1.211 uvideo.c
> > > --- sys/dev/usb/uvideo.c 27 Jan 2021 17:28:19 -0000 1.211
> > > +++ sys/dev/usb/uvideo.c 8 Mar 2021 22:06:51 -0000
> > > @@ -307,6 +307,7 @@ struct video_hw_if uvideo_hw_if = {
> > > #define UVIDEO_FLAG_ISIGHT_STREAM_HEADER 0x1
> > > #define UVIDEO_FLAG_REATTACH 0x2
> > > #define UVIDEO_FLAG_VENDOR_CLASS 0x4
> > > +#define UVIDEO_FLAG_NOATTACH 0x8
> > > struct uvideo_devs {
> > > struct usb_devno uv_dev;
> > > char *ucode_name;
> > > @@ -382,6 +383,12 @@ struct uvideo_devs {
> > > NULL,
> > > UVIDEO_FLAG_VENDOR_CLASS
> > > },
> > > + { /* Infrared camera not supported */
> > > + { USB_VENDOR_CHICONY, USB_PRODUCT_CHICONY_IRCAMERA },
> > > + NULL,
> > > + NULL,
> > > + UVIDEO_FLAG_NOATTACH
> > > + },
> > > };
> > > #define uvideo_lookup(v, p) \
> > > ((struct uvideo_devs *)usb_lookup(uvideo_devs, v, p))
> > > @@ -480,13 +487,12 @@ uvideo_match(struct device *parent, void
> > > if (id == NULL)
> > > return (UMATCH_NONE);
> > >
> > > - if (id->bInterfaceClass == UICLASS_VIDEO &&
> > > - id->bInterfaceSubClass == UISUBCLASS_VIDEOCONTROL)
> > > - return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
> > > -
> > > - /* quirk devices which we want to attach */
> > > + /* quirk devices */
> > > quirk = uvideo_lookup(uaa->vendor, uaa->product);
> > > if (quirk != NULL) {
> > > + if (quirk->flags & UVIDEO_FLAG_NOATTACH)
> >
> > How common is it to explain the system behavior in cases like this?
> > Would it be less surprising (generate less misc@ traffic) if we printed
> > a note explaining why the camera was skipped?
>
> I wouldn't print a specific message per unsupported device, but I think
> a generic message that the video device isn't supported would make
> sense. Something like that maybe? Obviously this can print more than
> once, but I'm not sure if it's worth to make that a unique print.
>
> E.g.:
>
> vmm0 at mainbus0: VMX/EPT
> uvideo: device 13d3:56b2 isn't supported
> uvideo: device 13d3:56b2 isn't supported
> ugen0 at uhub0 port 8 "SunplusIT Inc Integrated Camera" rev 2.01/17.11 addr 2
No; match functions shouldn't print stuff.
> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> retrieving revision 1.212
> diff -u -p -u -p -r1.212 uvideo.c
> --- sys/dev/usb/uvideo.c 5 Apr 2021 20:45:49 -0000 1.212
> +++ sys/dev/usb/uvideo.c 5 Apr 2021 21:09:36 -0000
> @@ -490,8 +490,11 @@ uvideo_match(struct device *parent, void
> /* quirk devices */
> quirk = uvideo_lookup(uaa->vendor, uaa->product);
> if (quirk != NULL) {
> - if (quirk->flags & UVIDEO_FLAG_NOATTACH)
> + if (quirk->flags & UVIDEO_FLAG_NOATTACH) {
> + printf("uvideo: device %x:%x isn't supported\n",
> + uaa->vendor, uaa->product);
> return (UMATCH_NONE);
> + }
>
> if (quirk->flags & UVIDEO_FLAG_REATTACH)
> return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
>
>
