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
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);
