Date: Sat, 17 Feb 2018 07:56:51 -0700 From: Warner Losh <imp@bsdimp.com> To: Hans Petter Selasky <hselasky@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r329458 - head/sbin/devmatch Message-ID: <CANCZdfrevbHxoRmV4HyjR9j%2B4%2BHbCU8H-aUhuhHjaySA=hwrsA@mail.gmail.com> In-Reply-To: <201802171434.w1HEYl8I063603@repo.freebsd.org> References: <201802171434.w1HEYl8I063603@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Feb 17, 2018 at 7:34 AM, Hans Petter Selasky <hselasky@freebsd.org> wrote: > Author: hselasky > Date: Sat Feb 17 14:34:47 2018 > New Revision: 329458 > URL: https://svnweb.freebsd.org/changeset/base/329458 > > Log: > Fix USB driver matching in devmatch(8). > > Multiple drivers can match on the same USB device and the order of > loading > decides which driver gets the device. Use the supplied mask value as an > indication of priority, so that vendor specific device drivers are loaded > before more generic ones. > This change isn't right. I mean, it might cause the special case of USB to get the driver you want loaded, but using the MASK this way is wrong. It's fixing the wrong problem. devmatch can't know who the winners will be and has to recommend all the drivers to load and those drivers have to cope with that. The right fix there, I think, is to load them all at once, in one kldload operation and not loop in /etc/rc.d/devmatch. I'd request you make no further changes to devmatch without a review. Warner > > Sponsored by: Mellanox Technologies > > Modified: > head/sbin/devmatch/devmatch.c > > Modified: head/sbin/devmatch/devmatch.c > ============================================================ > ================== > --- head/sbin/devmatch/devmatch.c Sat Feb 17 14:30:39 2018 > (r329457) > +++ head/sbin/devmatch/devmatch.c Sat Feb 17 14:34:47 2018 > (r329458) > @@ -54,6 +54,14 @@ static struct option longopts[] = { > { NULL, 0, NULL, 0 } > }; > > +#define DEVMATCH_MAX_HITS 256 > + > +static struct match_data { > + char *descr; > + int priority; > +} match_data[DEVMATCH_MAX_HITS]; > + > +static int hit_index; > static int all_flag; > static int dump_flag; > static char *linker_hints; > @@ -236,6 +244,35 @@ pnpval_as_str(const char *val, const char *pnpinfo) > return retval; > } > > +static int > +match_data_compare(const void *_pa, const void *_pb) > +{ > + const struct match_data *pa = _pa; > + const struct match_data *pb = _pb; > + > + /* biggest value first */ > + if (pa->priority > pb->priority) > + return (-1); > + else if (pa->priority < pb->priority) > + return (1); > + > + /* then sort by string */ > + return (strcmp(pa->descr, pb->descr)); > +} > + > +static int > +bitrev16(int input) > +{ > + int retval = 0; > + int x; > + > + for (x = 0; x != 16; x++) { > + if ((input >> x) & 1) > + retval |= (0x8000 >> x); > + } > + return (retval); > +} > + > static void > search_hints(const char *bus, const char *dev, const char *pnpinfo) > { > @@ -367,9 +404,20 @@ search_hints(const char *bus, const char *dev, const c > printf("\n"); > else if (!notme) { > if (!unbound_flag) { > + char *descr = NULL; > + > if (all_flag) > - printf("%s: ", > *dev ? dev : "unattached" ); > - printf("%s\n", lastmod); > + asprintf(&descr, > "%s: %s", *dev ? dev : "unattached", lastmod); > + else > + asprintf(&descr, > "%s", lastmod); > + > + if (descr != NULL && > hit_index < DEVMATCH_MAX_HITS) { > + > match_data[hit_index].descr = descr; > + > match_data[hit_index].priority = bitrev16(mask); > + hit_index++; > + } else { > + free(descr); > + } > } > found++; > } > @@ -382,6 +430,19 @@ search_hints(const char *bus, const char *dev, const c > } > walker = (void *)(len - sizeof(int) + (intptr_t)walker); > } > + if (hit_index != 0) { > + /* sort hits by priority */ > + mergesort(match_data, hit_index, sizeof(match_data[0]), > &match_data_compare); > + > + /* printout */ > + for (i = 0; i != hit_index; i++) { > + puts(match_data[i].descr); > + free(match_data[i].descr); > + } > + > + /* reset hit_index */ > + hit_index = 0; > + } > if (unbound_flag && found == 0 && *pnpinfo) { > if (verbose_flag) > printf("------------------------- "); > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrevbHxoRmV4HyjR9j%2B4%2BHbCU8H-aUhuhHjaySA=hwrsA>