Hi,
prompted by mpi@ i tried simplifying the way we get the
vendor/product/serial info from the device. Currently this is done
lazily, and sometimes several times. Instead of this, let's query the
device in usbd_new_device(), fallback to the global known lists then to
the USB IDs, cache the info in struct usbd_device and reuse it where
needed instead of pollin the device again.
The comment about vendor/product being possibly NULL in usbdivar.h might
need to be amended, since this is no longer the case.
As a bonus it allows us to remove the usedev parameter that was being
passed around.
Looking for testers, working fine here on my x1, with devices providing
the information directly, devices gettting the info from the global
usb_known_product/usb_known_vendor lists, and devices defaulting to
print the USB IDs as a fallback (which is the case within bsd.rd)
To look for regressions, compare usbdevs -vd and lsusb -v (from usbutils
pkg) outputs with and without the diff.
Landry
? usb_subr.c.dbg
Index: ugen.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.97
diff -u -r1.97 ugen.c
--- ugen.c 30 Dec 2017 23:08:29 -0000 1.97
+++ ugen.c 25 Apr 2018 18:32:36 -0000
@@ -1202,7 +1202,7 @@
}
case USB_GET_DEVICEINFO:
usbd_fill_deviceinfo(sc->sc_udev,
- (struct usb_device_info *)addr, 1);
+ (struct usb_device_info *)addr);
break;
default:
return (EINVAL);
Index: uhid.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhid.c,v
retrieving revision 1.70
diff -u -r1.70 uhid.c
--- uhid.c 30 Dec 2017 20:46:59 -0000 1.70
+++ uhid.c 25 Apr 2018 18:32:36 -0000
@@ -364,7 +364,7 @@
case USB_GET_DEVICEINFO:
usbd_fill_deviceinfo(sc->sc_hdev.sc_udev,
- (struct usb_device_info *)addr, 1);
+ (struct usb_device_info *)addr);
break;
case USB_GET_REPORT_DESC:
Index: umass_scsi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/umass_scsi.c,v
retrieving revision 1.45
diff -u -r1.45 umass_scsi.c
--- umass_scsi.c 3 Aug 2016 13:44:49 -0000 1.45
+++ umass_scsi.c 25 Apr 2018 18:32:36 -0000
@@ -160,7 +160,7 @@
if (sc->maxlun > 0)
return (0);
- usbd_fill_deviceinfo(sc->sc_udev, &udi, 1);
+ usbd_fill_deviceinfo(sc->sc_udev, &udi);
/*
* Create a fake devid using the vendor and product ids and the last
Index: uoak_subr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uoak_subr.c,v
retrieving revision 1.8
diff -u -r1.8 uoak_subr.c
--- uoak_subr.c 9 Jan 2017 14:44:28 -0000 1.8
+++ uoak_subr.c 25 Apr 2018 18:32:36 -0000
@@ -231,7 +231,7 @@
uoak_get_devinfo(struct uoak_softc *sc)
{
/* get device serial# */
- usbd_fill_deviceinfo(sc->sc_udev, &sc->sc_udi, 1);
+ usbd_fill_deviceinfo(sc->sc_udev, &sc->sc_udi);
}
void
Index: usb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.118
diff -u -r1.118 usb.c
--- usb.c 26 Feb 2018 13:06:49 -0000 1.118
+++ usb.c 25 Apr 2018 18:32:36 -0000
@@ -527,7 +527,7 @@
if (dev == NULL)
return;
- usbd_fill_deviceinfo(dev, di, 0);
+ usbd_fill_deviceinfo(dev, di);
}
void
Index: usb_subr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.135
diff -u -r1.135 usb_subr.c
--- usb_subr.c 24 Apr 2018 17:22:33 -0000 1.135
+++ usb_subr.c 25 Apr 2018 18:32:37 -0000
@@ -66,6 +66,7 @@
int usbd_getnewaddr(struct usbd_bus *);
int usbd_print(void *, const char *);
void usbd_free_iface_data(struct usbd_device *, int);
+int usbd_cache_devinfo(struct usbd_device *);
usbd_status usbd_probe_and_attach(struct device *,
struct usbd_device *, int, int);
@@ -230,70 +231,70 @@
return (buf);
}
-void
-usbd_devinfo_vp(struct usbd_device *dev, char *v, size_t vl,
- char *p, size_t pl, int usedev)
+int
+usbd_cache_devinfo(struct usbd_device *dev)
{
usb_device_descriptor_t *udd = &dev->ddesc;
- char *vendor = NULL, *product = NULL;
-#ifdef USBVERBOSE
- const struct usb_known_vendor *ukv;
- const struct usb_known_product *ukp;
-#endif
- if (dev == NULL) {
- v[0] = p[0] = '\0';
- return;
- }
+ dev->serial = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+ if (dev->serial == NULL)
+ return (ENOMEM);
- if (usedev) {
- vendor = usbd_get_string(dev, udd->iManufacturer, v, vl);
- usbd_trim_spaces(vendor);
- product = usbd_get_string(dev, udd->iProduct, p, pl);
- usbd_trim_spaces(product);
+ if (usbd_get_string(dev, udd->iSerialNumber, dev->serial,
USB_MAX_STRING_LEN) != NULL) {
+ usbd_trim_spaces(dev->serial);
} else {
- if (dev->vendor != NULL)
- vendor = dev->vendor;
- if (dev->product != NULL)
- product = dev->product;
+ free(dev->serial, M_USB, USB_MAX_STRING_LEN);
+ dev->serial = NULL;
}
+
+ dev->vendor = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+ if (dev->vendor == NULL)
+ return (ENOMEM);
+
+ if (usbd_get_string(dev, udd->iManufacturer, dev->vendor,
USB_MAX_STRING_LEN) != NULL) {
+ usbd_trim_spaces(dev->vendor);
+ } else {
#ifdef USBVERBOSE
- if (vendor == NULL || product == NULL) {
- for (ukv = usb_known_vendors;
- ukv->vendorname != NULL;
- ukv++) {
+ const struct usb_known_vendor *ukv;
+
+ for (ukv = usb_known_vendors; ukv->vendorname != NULL; ukv++) {
if (ukv->vendor == UGETW(udd->idVendor)) {
- vendor = ukv->vendorname;
+ strlcpy(dev->vendor, ukv->vendorname,
+ USB_MAX_STRING_LEN);
break;
}
}
- if (vendor != NULL) {
- for (ukp = usb_known_products;
- ukp->productname != NULL;
- ukp++) {
- if (ukp->vendor == UGETW(udd->idVendor) &&
- (ukp->product == UGETW(udd->idProduct))) {
- product = ukp->productname;
- break;
- }
+ if (ukv->vendorname == NULL)
+#endif
+ snprintf(dev->vendor, USB_MAX_STRING_LEN, "vendor
0x%04x",
+ UGETW(udd->idVendor));
+ }
+
+ dev->product = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+ if (dev->product == NULL)
+ return (ENOMEM);
+
+ if (usbd_get_string(dev, udd->iProduct, dev->product,
USB_MAX_STRING_LEN) != NULL) {
+ usbd_trim_spaces(dev->product);
+ } else {
+#ifdef USBVERBOSE
+ const struct usb_known_product *ukp;
+
+ for (ukp = usb_known_products; ukp->productname != NULL; ukp++)
{
+ if (ukp->vendor == UGETW(udd->idVendor) &&
+ (ukp->product == UGETW(udd->idProduct))) {
+ strlcpy(dev->product, ukp->productname,
+ USB_MAX_STRING_LEN);
+ break;
}
}
- }
+ if (ukp->productname == NULL)
#endif
+ snprintf(dev->product, USB_MAX_STRING_LEN, "product
0x%04x",
+ UGETW(udd->idProduct));
+ }
- if (v == vendor)
- ;
- else if (vendor != NULL && *vendor)
- strlcpy(v, vendor, vl);
- else
- snprintf(v, vl, "vendor 0x%04x", UGETW(udd->idVendor));
-
- if (p == product)
- ;
- else if (product != NULL && *product)
- strlcpy(p, product, pl);
- else
- snprintf(p, pl, "product 0x%04x", UGETW(udd->idProduct));
+ return (0);
}
int
@@ -313,13 +314,10 @@
usbd_devinfo(struct usbd_device *dev, int showclass, char *base, size_t len)
{
usb_device_descriptor_t *udd = &dev->ddesc;
- char vendor[USB_MAX_STRING_LEN];
- char product[USB_MAX_STRING_LEN];
char *cp = base;
int bcdDevice, bcdUSB;
- usbd_devinfo_vp(dev, vendor, sizeof vendor, product, sizeof product, 0);
- snprintf(cp, len, "\"%s %s\"", vendor, product);
+ snprintf(cp, len, "\"%s %s\"", dev->vendor, dev->product);
cp += strlen(cp);
if (showclass) {
snprintf(cp, base + len - cp, ", class %d/%d",
@@ -1237,10 +1235,13 @@
DPRINTF(("usbd_new_device: new dev (addr %d), dev=%p, parent=%p\n",
addr, dev, parent));
- /* Cache some strings if possible. */
- dev->vendor = usbd_get_device_string(dev, dev->ddesc.iManufacturer);
- dev->product = usbd_get_device_string(dev, dev->ddesc.iProduct);
- dev->serial = usbd_get_device_string(dev, dev->ddesc.iSerialNumber);
+ /* Get device info and cache it */
+ err = usbd_cache_devinfo(dev);
+ if (err) {
+ usb_free_device(dev);
+ up->device = NULL;
+ return (err);
+ }
err = usbd_probe_and_attach(parent, dev, port, addr);
if (err) {
@@ -1300,16 +1301,15 @@
}
void
-usbd_fill_deviceinfo(struct usbd_device *dev, struct usb_device_info *di,
- int usedev)
+usbd_fill_deviceinfo(struct usbd_device *dev, struct usb_device_info *di)
{
struct usbd_port *p;
int i, err, s;
di->udi_bus = dev->bus->usbctl->dv_unit;
di->udi_addr = dev->address;
- usbd_devinfo_vp(dev, di->udi_vendor, sizeof(di->udi_vendor),
- di->udi_product, sizeof(di->udi_product), usedev);
+ strlcpy(di->udi_vendor, dev->vendor, sizeof(di->udi_vendor));
+ strlcpy(di->udi_product, dev->product, sizeof(di->udi_product));
usbd_printBCD(di->udi_release, sizeof di->udi_release,
UGETW(dev->ddesc.bcdDevice));
di->udi_vendorNo = UGETW(dev->ddesc.idVendor);
@@ -1358,13 +1358,9 @@
di->udi_nports = 0;
bzero(di->udi_serial, sizeof(di->udi_serial));
- if (!usedev && dev->serial != NULL) {
+ if (dev->serial != NULL)
strlcpy(di->udi_serial, dev->serial,
sizeof(di->udi_serial));
- } else {
- usbd_get_string(dev, dev->ddesc.iSerialNumber,
- di->udi_serial, sizeof(di->udi_serial));
- }
}
/* Retrieve a complete descriptor for a certain device and index. */
Index: usbdi.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.h,v
retrieving revision 1.69
diff -u -r1.69 usbdi.h
--- usbdi.h 15 May 2017 11:05:51 -0000 1.69
+++ usbdi.h 25 Apr 2018 18:32:37 -0000
@@ -129,7 +129,7 @@
usb_device_descriptor_t *usbd_get_device_descriptor(struct usbd_device *dev);
usbd_status usbd_set_interface(struct usbd_interface *, int);
int usbd_get_no_alts(usb_config_descriptor_t *, int);
-void usbd_fill_deviceinfo(struct usbd_device *, struct usb_device_info *, int);
+void usbd_fill_deviceinfo(struct usbd_device *, struct usb_device_info *);
usb_config_descriptor_t *usbd_get_cdesc(struct usbd_device *, int, u_int *);
int usbd_get_interface_altindex(struct usbd_interface *iface);
Index: usbdivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.74
diff -u -r1.74 usbdivar.h
--- usbdivar.h 24 Apr 2018 17:22:33 -0000 1.74
+++ usbdivar.h 25 Apr 2018 18:32:37 -0000
@@ -257,8 +257,6 @@
usbd_status usb_insert_transfer(struct usbd_xfer *);
void usb_transfer_complete(struct usbd_xfer *);
int usbd_detach(struct usbd_device *, struct device *);
-void usbd_devinfo_vp(struct usbd_device *, char *, size_t,
- char *, size_t, int);
/* Routines from usb.c */
void usb_needs_explore(struct usbd_device *, int);
Index: uvideo.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.197
diff -u -r1.197 uvideo.c
--- uvideo.c 24 Apr 2018 17:22:33 -0000 1.197
+++ uvideo.c 25 Apr 2018 18:32:38 -0000
@@ -2781,14 +2781,10 @@
uvideo_querycap(void *v, struct v4l2_capability *caps)
{
struct uvideo_softc *sc = v;
- char vendor[USB_MAX_STRING_LEN];
- char product[USB_MAX_STRING_LEN];
bzero(caps, sizeof(*caps));
strlcpy(caps->driver, DEVNAME(sc), sizeof(caps->driver));
- usbd_devinfo_vp(sc->sc_udev, vendor, sizeof (vendor), product,
- sizeof (product), 0);
- strlcpy(caps->card, product, sizeof(caps->card));
+ strlcpy(caps->card, sc->sc_udev->product, sizeof(caps->card));
strlcpy(caps->bus_info, "usb", sizeof(caps->bus_info));
caps->version = 1;
