Date: Mon, 23 Nov 2009 11:14:04 +1300 From: Andrew Thompson <thompsa@FreeBSD.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org>, Hans Petter Selasky <hselasky@freebsd.org> Subject: Re: PERFORCE change 170842 for review Message-ID: <1280352d0911221414i28e58b80o2c1a5958d3ce54ef@mail.gmail.com> In-Reply-To: <4B06B19F.7050501@freebsd.org> References: <200911192235.nAJMZ2XH072195@repoman.freebsd.org> <4B05CB1F.8020100@freebsd.org> <200911200846.54559.hselasky@freebsd.org> <4B06B19F.7050501@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
2009/11/21 Nathan Whitehorn <nwhitehorn@freebsd.org>: > Hans Petter Selasky wrote: >> >> On Thursday 19 November 2009 23:47:59 Nathan Whitehorn wrote: >>>> >>>> @@ -1530,7 +1574,7 @@ >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (ENXIO); >>>> >>>> =A0 =A0 =A0 =A0if (usbd_lookup_id_by_uaa(atp_devs, sizeof(atp_devs), u= aa) =3D=3D 0) >>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return BUS_PROBE_SPECIFIC; >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0; >>>> =A0 =A0 =A0 =A0else >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return ENXIO; >>>> =A0} >>>> >>> >>> Why are you replacing symbolic constants with less informative numeric >>> ones? -Nathan >>> >> >> Because returning zero in probe has special meaning and is hardcoded in >> the subr_bus.c code aswell. The other return values will not be changed. >> > > It's the same thing as far as the code is concerned, of course, my compla= int > was merely a style issue. Using the constant makes the meaning of the ret= urn > value clearer, especially since this driver uses this return value to > override the BUS_PROBE_GENERIC priority of ums(4). Changing it from the > constant that was already there seemed like a step backward in readabilit= y. I think we should really bring back the probe returns for usb, to allow drivers to be overridden. ie, UMATCH_VENDOR_PRODUCT vs UMATCH_IFACECLASS http://fxr.watson.org/fxr/source/dev/usb/usbdi.h?v=3DFREEBSD72#L236 Something like... Index: usb_lookup.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- usb_lookup.c (revision 199667) +++ usb_lookup.c (working copy) @@ -133,6 +133,33 @@ done: return (NULL); } +int +usbd_probe_priority(const struct usb_device_id *id) +{ + int pri; + + pri =3D UMATCH_GENERIC; + + /* Probe priority, lowest to highest */ + if (id->match_flag_int_class) + pri =3D UMATCH_IFACECLASS; + if (id->match_flag_int_subclass) + pri =3D UMATCH_IFACECLASS_IFACESUBCLASS; + if (id->match_flag_int_protocol) + pri =3D UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO; + if (id->match_flag_dev_protocol) + pri =3D UMATCH_DEVCLASS_DEVSUBCLASS_DEVPROTO; + if (id->match_flag_dev_class && id->match_flag_dev_subclass) + pri =3D UMATCH_DEVCLASS_DEVSUBCLASS; + if (id->match_flag_vendor && id->match_flag_product) + pri =3D UMATCH_VENDOR_PRODUCT; + if (id->match_flag_vendor && id->match_flag_product && + (id->match_flag_dev_lo || id->match_flag_dev_hi)) + pri =3D UMATCH_VENDOR_PRODUCT_REV; + + return (pri); +} + /*------------------------------------------------------------------------= * * usbd_lookup_id_by_uaa - factored out code * @@ -148,7 +175,7 @@ usbd_lookup_id_by_uaa(const struct usb_device_id * if (id) { /* copy driver info */ uaa->driver_info =3D id->driver_info; - return (0); + return (usbd_probe_priority(id)); } return (ENXIO); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1280352d0911221414i28e58b80o2c1a5958d3ce54ef>