Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
On Mon, 5 Apr 2021 23:54:31 +0200
Marcus Glocker 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.c5 Apr 2021 20:45:49 - 1.212
+++ sys/dev/usb/uvideo.c5 Apr 2021 21:51:11 -
@@ -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);
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
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.c5 Apr 2021 20:45:49 - 1.212
+++ sys/dev/usb/uvideo.c5 Apr 2021 21:51:11 -
@@ -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);
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
> Date: Mon, 5 Apr 2021 23:19:02 +0200 (CEST)
> From: Mark Kettenis
>
> > Date: Mon, 5 Apr 2021 23:15:23 +0200
> > From: Marcus Glocker
> >
> > 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 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.c27 Jan 2021 17:28:19 - 1.211
> > > > +++ sys/dev/usb/uvideo.c8 Mar 2021 22:06:51 -
> > > > @@ -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.
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.
> > 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.c5 Apr 2021 20:45:49 - 1.212
> > +++ sys/dev/usb/uvideo.c5 Apr 2021 21:09:36 -
> > @@ -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);
> >
> >
>
>
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
Am Mon, Apr 05, 2021 at 11:19:02PM +0200 schrieb Mark Kettenis:
> > Date: Mon, 5 Apr 2021 23:15:23 +0200
> > From: Marcus Glocker
> >
> > 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 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.c27 Jan 2021 17:28:19 - 1.211
> > > > +++ sys/dev/usb/uvideo.c8 Mar 2021 22:06:51 -
> > > > @@ -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.
Agreed. If something like that was wanted, I'd rather have uvideo(4)
attach, but print that it's not supported, and then not attach video(4)
to it.
>
> > 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.c5 Apr 2021 20:45:49 - 1.212
> > +++ sys/dev/usb/uvideo.c5 Apr 2021 21:09:36 -
> > @@ -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);
> >
> >
>
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
> Date: Mon, 5 Apr 2021 23:15:23 +0200
> From: Marcus Glocker
>
> 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 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 - 1.211
> > > +++ sys/dev/usb/uvideo.c 8 Mar 2021 22:06:51 -
> > > @@ -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 - 1.212
> +++ sys/dev/usb/uvideo.c 5 Apr 2021 21:09:36 -
> @@ -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);
>
>
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
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 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.c27 Jan 2021 17:28:19 - 1.211
> > +++ sys/dev/usb/uvideo.c8 Mar 2021 22:06:51 -
> > @@ -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.c5 Apr 2021 20:45:49 - 1.212
+++ sys/dev/usb/uvideo.c5 Apr 2021 21:09:36 -
@@ -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);
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
OK gnezdo with a usability question inline.
Marcus Glocker 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 - 1.211
> +++ sys/dev/usb/uvideo.c 8 Mar 2021 22:06:51 -
> @@ -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?
Thanks
Greg
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
This diff shouldn't impact any other uvideo(4) devices so I would
welcome somebody brave to OK this :-)
Thanks,
Marcus
On Fri, 26 Mar 2021 10:15:58 +0100
Martijn van Duren wrote:
> I'm in no way qualified to OK this, but I'd like to see this get in,
> because it does help me out :-)
>
> martijn@
>
> On Mon, 2021-03-15 at 08:35 +0100, Marcus Glocker wrote:
> > 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.c27 Jan 2021 17:28:19 -
> > 1.211 +++ sys/dev/usb/uvideo.c8 Mar 2021 22:06:51 -
> > @@ -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)
> > + return (UMATCH_NONE);
> > +
> > if (quirk->flags & UVIDEO_FLAG_REATTACH)
> > return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
> >
> > @@ -495,6 +501,10 @@ uvideo_match(struct device *parent, void
> > id->bInterfaceSubClass ==
> > UISUBCLASS_VIDEOCONTROL) return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
> > }
> > +
> > + if (id->bInterfaceClass == UICLASS_VIDEO &&
> > + id->bInterfaceSubClass == UISUBCLASS_VIDEOCONTROL)
> > + return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
> >
> > return (UMATCH_NONE);
> > }
> > Index: sys/dev/usb/usbdevs
> > ===
> > RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> > retrieving revision 1.731
> > diff -u -p -u -p -r1.731 usbdevs
> > --- sys/dev/usb/usbdevs 27 Feb 2021 03:03:40 - 1.731
> > +++ sys/dev/usb/usbdevs 8 Mar 2021 22:06:53 -
> > @@ -1336,6 +1336,7 @@ product CHICONY RTL8188CUS_3 0xaff9 RTL8
> > product CHICONY RTL8188CUS_4 0xaffa RTL8188CUS
> > product CHICONY RTL8188CUS_5 0xaffb RTL8188CUS
> > product CHICONY RTL8188CUS_6 0xaffc RTL8188CUS
> > +product CHICONY IRCAMERA 0xb615 Integrated IR Camera
> >
> > /* CH Products */
> > product CHPRODUCTS PROTHROTTLE 0x00f1 Pro Throttle
> >
>
>
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
I'm in no way qualified to OK this, but I'd like to see this get in,
because it does help me out :-)
martijn@
On Mon, 2021-03-15 at 08:35 +0100, Marcus Glocker wrote:
> 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.c27 Jan 2021 17:28:19 - 1.211
> +++ sys/dev/usb/uvideo.c8 Mar 2021 22:06:51 -
> @@ -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)
> + return (UMATCH_NONE);
> +
> if (quirk->flags & UVIDEO_FLAG_REATTACH)
> return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
>
> @@ -495,6 +501,10 @@ uvideo_match(struct device *parent, void
> id->bInterfaceSubClass == UISUBCLASS_VIDEOCONTROL)
> return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
> }
> +
> + if (id->bInterfaceClass == UICLASS_VIDEO &&
> + id->bInterfaceSubClass == UISUBCLASS_VIDEOCONTROL)
> + return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
>
> return (UMATCH_NONE);
> }
> Index: sys/dev/usb/usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.731
> diff -u -p -u -p -r1.731 usbdevs
> --- sys/dev/usb/usbdevs 27 Feb 2021 03:03:40 - 1.731
> +++ sys/dev/usb/usbdevs 8 Mar 2021 22:06:53 -
> @@ -1336,6 +1336,7 @@ product CHICONY RTL8188CUS_3 0xaff9 RTL8
> product CHICONY RTL8188CUS_4 0xaffa RTL8188CUS
> product CHICONY RTL8188CUS_5 0xaffb RTL8188CUS
> product CHICONY RTL8188CUS_6 0xaffc RTL8188CUS
> +product CHICONY IRCAMERA 0xb615 Integrated IR Camera
>
> /* CH Products */
> product CHPRODUCTS PROTHROTTLE 0x00f1 Pro Throttle
>
