From owner-p4-projects@FreeBSD.ORG Sun Nov 22 22:41:43 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id CB91E1065676; Sun, 22 Nov 2009 22:41:43 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 41D30106566C; Sun, 22 Nov 2009 22:41:43 +0000 (UTC) (envelope-from andy@fud.org.nz) Received: from mail-vw0-f173.google.com (mail-vw0-f173.google.com [209.85.212.173]) by mx1.freebsd.org (Postfix) with ESMTP id DF0678FC18; Sun, 22 Nov 2009 22:41:42 +0000 (UTC) Received: by vws3 with SMTP id 3so1376862vws.3 for ; Sun, 22 Nov 2009 14:41:42 -0800 (PST) MIME-Version: 1.0 Sender: andy@fud.org.nz Received: by 10.220.125.104 with SMTP id x40mr4863726vcr.101.1258928044501; Sun, 22 Nov 2009 14:14:04 -0800 (PST) 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> Date: Mon, 23 Nov 2009 11:14:04 +1300 X-Google-Sender-Auth: f25a4b788b932394 Message-ID: <1280352d0911221414i28e58b80o2c1a5958d3ce54ef@mail.gmail.com> From: Andrew Thompson To: Nathan Whitehorn Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Perforce Change Reviews , Hans Petter Selasky Subject: Re: PERFORCE change 170842 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 22 Nov 2009 22:41:44 -0000 2009/11/21 Nathan Whitehorn : > 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); }