On Mon, 5 Apr 2021 23:54:31 +0200
Marcus Glocker <[email protected]> wrote:
> 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.
While cleaning up my diffs I just found that this one hasn't been
committed. OK to get this in?
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);
