From: mpi <mpi@openbsd.org>
Date: Tue, 14 Apr 2015 14:57:05 +0000 (+0000)
Subject: Setting the configuration in *_attach() is a bad practise because if it
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4dd0bafb12a9eb1eeb6beb79bb6cfbcad2c28ec3;p=openbsd

Setting the configuration in *_attach() is a bad practise because if it
fails it's impossible to debug and you cannot use your device.

So instead of calling usbd_set_config_index(), match the right interface.

This is trivial with this device because it has only one configuration
and interface.
---

diff --git a/sys/dev/usb/umcs.c b/sys/dev/usb/umcs.c
index 50d477e163a..03a545814de 100644
--- a/sys/dev/usb/umcs.c
+++ b/sys/dev/usb/umcs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: umcs.c,v 1.3 2015/04/14 14:38:17 mpi Exp $ */
+/* $OpenBSD: umcs.c,v 1.4 2015/04/14 14:57:05 mpi Exp $ */
 /* $NetBSD: umcs.c,v 1.8 2014/08/23 21:37:56 martin Exp $ */
 /* $FreeBSD: head/sys/dev/usb/serial/umcs.c 260559 2014-01-12 11:44:28Z hselasky $ */
 
@@ -85,7 +85,6 @@ struct umcs_port {
 
 struct umcs_softc {
 	struct device		 sc_dev;
-	struct usbd_interface	*sc_iface;	/* the usb interface */
 	struct usbd_device	*sc_udev;	/* the usb device */
 	struct usbd_pipe	*sc_ipipe;	/* interrupt pipe */
 	uint8_t			*sc_ibuf;	/* buffer for interrupt xfer */
@@ -178,6 +177,9 @@ umcs_match(struct device *dev, void *match, void *aux)
 {
 	struct usb_attach_arg *uaa = aux;
 
+	if (uaa->iface == NULL || uaa->ifaceno != UMCS_IFACE_NO)
+		return (UMATCH_NONE);
+
 	return (usb_lookup(umcs_devs, uaa->vendor, uaa->product) != NULL) ?
 	    UMATCH_VENDOR_PRODUCT : UMATCH_NONE;
 }
@@ -195,21 +197,6 @@ umcs_attach(struct device *parent, struct device *self, void *aux)
 
 	sc->sc_udev = uaa->device;
 
-	if (usbd_set_config_index(sc->sc_udev, UMCS_CONFIG_NO, 1) != 0) {
-		printf("%s: could not set configuration no\n", DEVNAME(sc));
-		usbd_deactivate(sc->sc_udev);
-		return;
-	}
-
-	/* Get the first interface handle */
-	error = usbd_device2interface_handle(sc->sc_udev, UMCS_IFACE_NO,
-	    &sc->sc_iface);
-	if (error != 0) {
-		printf("%s: could not get interface handle\n", DEVNAME(sc));
-		usbd_deactivate(sc->sc_udev);
-		return;
-	}
-
 	/*
 	 * Get number of ports
 	 * Documentation (full datasheet) says, that number of ports is
@@ -246,10 +233,10 @@ umcs_attach(struct device *parent, struct device *self, void *aux)
 #endif
 
 	/* Set up the interrupt pipe */
-	id = usbd_get_interface_descriptor(sc->sc_iface);
+	id = usbd_get_interface_descriptor(uaa->iface);
 	intr_addr = -1;
 	for (i = 0 ; i < id->bNumEndpoints ; i++) {
-		ed = usbd_interface2endpoint_descriptor(sc->sc_iface, i);
+		ed = usbd_interface2endpoint_descriptor(uaa->iface, i);
 		if (ed == NULL) {
 			printf("%s: no endpoint descriptor found for %d\n",
 			    DEVNAME(sc), i);
@@ -271,7 +258,7 @@ umcs_attach(struct device *parent, struct device *self, void *aux)
 	}
 	sc->sc_ibuf = malloc(sc->sc_isize, M_USBDEV, M_WAITOK);
 
-	error = usbd_open_pipe_intr(sc->sc_iface, intr_addr,
+	error = usbd_open_pipe_intr(uaa->iface, intr_addr,
 		    USBD_SHORT_XFER_OK, &sc->sc_ipipe, sc, sc->sc_ibuf,
 		    sc->sc_isize, umcs_intr, 100 /* XXX */);
 	if (error) {
@@ -287,7 +274,7 @@ umcs_attach(struct device *parent, struct device *self, void *aux)
 	uca.ibufsizepad = 256;
 	uca.opkthdrlen = 0;
 	uca.device = sc->sc_udev;
-	uca.iface = sc->sc_iface;
+	uca.iface = uaa->iface;
 	uca.methods = &umcs_methods;
 	uca.arg = sc;
 
@@ -301,7 +288,7 @@ umcs_attach(struct device *parent, struct device *self, void *aux)
 		 */
 		int pn = i * (sc->sc_numports == 2 ? 2 : 1);
 
-		ed = usbd_interface2endpoint_descriptor(sc->sc_iface, pn*2);
+		ed = usbd_interface2endpoint_descriptor(uaa->iface, pn*2);
 		if (ed == NULL) {
 			printf("%s: no bulk in endpoint found for %d\n",
 			    DEVNAME(sc), i);
@@ -310,7 +297,7 @@ umcs_attach(struct device *parent, struct device *self, void *aux)
 		}
 		uca.bulkin = ed->bEndpointAddress;
 
-		ed = usbd_interface2endpoint_descriptor(sc->sc_iface, pn*2+1);
+		ed = usbd_interface2endpoint_descriptor(uaa->iface, pn*2+1);
 		if (ed == NULL) {
 			printf("%s: no bulk out endpoint found for %d\n",
 			    DEVNAME(sc), i);