On Mon, Apr 05, 2021 at 11:27:10PM +0200, Mark Kettenis wrote:
[...]
> > > > 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.
>
> If you really wanted to print a message, you'll need to let uvideo(4)
> attach and then print a "not supported" message early on in the attach
> function and return.
Right. Despite the print line, maybe this way is anyway better than
doing this check in the match function.
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:51:11 -0000
@@ -490,9 +490,6 @@ uvideo_match(struct device *parent, void
/* quirk devices */
quirk = uvideo_lookup(uaa->vendor, uaa->product);
if (quirk != NULL) {
- if (quirk->flags & UVIDEO_FLAG_NOATTACH)
- return (UMATCH_NONE);
-
if (quirk->flags & UVIDEO_FLAG_REATTACH)
return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
@@ -577,6 +574,11 @@ uvideo_attach(struct device *parent, str
/* maybe the device has quirks */
sc->sc_quirk = uvideo_lookup(uaa->vendor, uaa->product);
+
+ if (sc->sc_quirk && sc->sc_quirk->flags & UVIDEO_FLAG_NOATTACH) {
+ printf("%s: device not supported\n", DEVNAME(sc));
+ return;
+ }
if (sc->sc_quirk && sc->sc_quirk->ucode_name)
config_mountroot(self, uvideo_attach_hook);
