dm: usb: Move descriptor setup code into its own function

usb_new_device() is far too long and does far too much. As a first step, move the code that does initial setup and reads a descriptor into its own function called usb_setup_descriptor(). For XHCI the init order is different - we set up the device but don't actually read the descriptor until after we set an address. Support this option as a parameter to usb_setup_descriptor(). Avoid changing this torturous code more than necessary to make it easy to review. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Marek Vasut <marex@denx.de>

dm: usb: Move descriptor setup code into its own function
usb_new_device() is far too long and does far too much. As a first step, move the code that does initial setup and reads a descriptor into its own function called usb_setup_descriptor(). For XHCI the init order is different - we set up the device but don't actually read the descriptor until after we set an address. Support this option as a parameter to usb_setup_descriptor(). Avoid changing this torturous code more than necessary to make it easy to review. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Marek Vasut <marex@denx.de>
Simon Glass
1 parent 862e75c0db
Showing 1 changed file with 82 additions and 65 deletions Side-by-side Diff
common/usb.c
@@ -897,35 +897,12 @@
 	return 0;
 }
  
-/*
- * By the time we get here, the device has gotten a new device ID
- * and is in the default state. We need to identify the thing and
- * get the ball rolling..
- *
- * Returns 0 for success, != 0 for error.
- */
-int usb_new_device(struct usb_device *dev)
+static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
 {
-	int addr, err;
-	int tmp;
+	__maybe_unused struct usb_device_descriptor *desc;
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
  
 	/*
-	 * Allocate usb 3.0 device context.
-	 * USB 3.0 (xHCI) protocol tries to allocate device slot
-	 * and related data structures first. This call does that.
-	 * Refer to sec 4.3.2 in xHCI spec rev1.0
-	 */
-	if (usb_alloc_device(dev)) {
-		printf("Cannot allocate device context to get SLOT_ID\n");
-		return -EINVAL;
-	}
-
-	/* We still haven't set the Address yet */
-	addr = dev->devnum;
-	dev->devnum = 0;
-
-	/*
 	 * This is a Windows scheme of initialization sequence, with double
 	 * reset of the device (Linux uses the same sequence)
 	 * Some equipment is said to work only with such init sequence; this
  
  
  
@@ -940,55 +917,46 @@
 	 * the maxpacket size is 8 or 16 the device may be waiting to transmit
 	 * some more, or keeps on retransmitting the 8 byte header. */
  
+	desc = (struct usb_device_descriptor *)tmpbuf;
 	dev->descriptor.bMaxPacketSize0 = 64;	    /* Start off at 64 bytes  */
 	/* Default to 64 byte max packet size */
 	dev->maxpacketsize = PACKET_SIZE_64;
 	dev->epmaxpacketin[0] = 64;
 	dev->epmaxpacketout[0] = 64;
  
-	/*
-	 * XHCI needs to issue a Address device command to setup
-	 * proper device context structures, before it can interact
-	 * with the device. So a get_descriptor will fail before any
-	 * of that is done for XHCI unlike EHCI.
-	 */
-#ifndef CONFIG_USB_XHCI
-	struct usb_device_descriptor *desc;
+	if (do_read) {
+		int err;
  
-	desc = (struct usb_device_descriptor *)tmpbuf;
-	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
-	/*
-	 * Validate we've received only at least 8 bytes, not that we've
-	 * received the entire descriptor. The reasoning is:
-	 * - The code only uses fields in the first 8 bytes, so that's all we
-	 *   need to have fetched at this stage.
-	 * - The smallest maxpacket size is 8 bytes. Before we know the actual
-	 *   maxpacket the device uses, the USB controller may only accept a
-	 *   single packet. Consequently we are only guaranteed to receive 1
-	 *   packet (at least 8 bytes) even in a non-error case.
-	 *
-	 * At least the DWC2 controller needs to be programmed with the number
-	 * of packets in addition to the number of bytes. A request for 64
-	 * bytes of data with the maxpacket guessed as 64 (above) yields a
-	 * request for 1 packet.
-	 */
-	if (err < 8) {
-		debug("usb_new_device: usb_get_descriptor() failed\n");
-		return -EIO;
+		err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
+		/*
+		 * Validate we've received only at least 8 bytes, not that we've
+		 * received the entire descriptor. The reasoning is:
+		 * - The code only uses fields in the first 8 bytes, so that's all we
+		 *   need to have fetched at this stage.
+		 * - The smallest maxpacket size is 8 bytes. Before we know the actual
+		 *   maxpacket the device uses, the USB controller may only accept a
+		 *   single packet. Consequently we are only guaranteed to receive 1
+		 *   packet (at least 8 bytes) even in a non-error case.
+		 *
+		 * At least the DWC2 controller needs to be programmed with the number
+		 * of packets in addition to the number of bytes. A request for 64
+		 * bytes of data with the maxpacket guessed as 64 (above) yields a
+		 * request for 1 packet.
+		 */
+		if (err < 8) {
+			if (err < 0) {
+				printf("unable to get device descriptor (error=%d)\n",
+				       err);
+				return err;
+			} else {
+				printf("USB device descriptor short read (expected %i, got %i)\n",
+				       (int)sizeof(dev->descriptor), err);
+				return -EIO;
+			}
+		}
+		memcpy(&dev->descriptor, tmpbuf, sizeof(dev->descriptor));
 	}
  
-	dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
-	/*
-	 * Fetch the device class, driver can use this info
-	 * to differentiate between HUB and DEVICE.
-	 */
-	dev->descriptor.bDeviceClass = desc->bDeviceClass;
-#endif
-
-	err = usb_legacy_port_reset(dev->parent, dev->portnr);
-	if (err)
-		return err;
-
 	dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;
 	dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;
 	switch (dev->descriptor.bMaxPacketSize0) {
@@ -1008,6 +976,55 @@
 		printf("usb_new_device: invalid max packet size\n");
 		return -EIO;
 	}
+
+	return 0;
+}
+
+/*
+ * By the time we get here, the device has gotten a new device ID
+ * and is in the default state. We need to identify the thing and
+ * get the ball rolling..
+ *
+ * Returns 0 for success, != 0 for error.
+ */
+int usb_new_device(struct usb_device *dev)
+{
+	bool do_read = true;
+	int addr, err;
+	int tmp;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
+
+	/*
+	 * Allocate usb 3.0 device context.
+	 * USB 3.0 (xHCI) protocol tries to allocate device slot
+	 * and related data structures first. This call does that.
+	 * Refer to sec 4.3.2 in xHCI spec rev1.0
+	 */
+	if (usb_alloc_device(dev)) {
+		printf("Cannot allocate device context to get SLOT_ID\n");
+		return -1;
+	}
+
+	/* We still haven't set the Address yet */
+	addr = dev->devnum;
+	dev->devnum = 0;
+
+	/*
+	 * XHCI needs to issue a Address device command to setup
+	 * proper device context structures, before it can interact
+	 * with the device. So a get_descriptor will fail before any
+	 * of that is done for XHCI unlike EHCI.
+	 */
+#ifdef CONFIG_USB_XHCI
+	do_read = false;
+#endif
+	err = usb_setup_descriptor(dev, do_read);
+	if (err)
+		return err;
+	err = usb_legacy_port_reset(dev->parent, dev->portnr);
+	if (err)
+		return err;
+
 	dev->devnum = addr;
  
 	err = usb_set_address(dev); /* set address */
...	...	@@ -897,35 +897,12 @@
897	897	return 0;
898	898	}
899	899
900		-/*
901		- * By the time we get here, the device has gotten a new device ID
902		- * and is in the default state. We need to identify the thing and
903		- * get the ball rolling..
904		- *
905		- * Returns 0 for success, != 0 for error.
906		- */
907		-int usb_new_device(struct usb_device *dev)
	900	+static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
908	901	{
909		- int addr, err;
910		- int tmp;
	902	+ __maybe_unused struct usb_device_descriptor *desc;
911	903	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
912	904
913	905	/*
914		- * Allocate usb 3.0 device context.
915		- * USB 3.0 (xHCI) protocol tries to allocate device slot
916		- * and related data structures first. This call does that.
917		- * Refer to sec 4.3.2 in xHCI spec rev1.0
918		- */
919		- if (usb_alloc_device(dev)) {
920		- printf("Cannot allocate device context to get SLOT_ID\n");
921		- return -EINVAL;
922		- }
923		-
924		- /* We still haven't set the Address yet */
925		- addr = dev->devnum;
926		- dev->devnum = 0;
927		-
928		- /*
929	906	* This is a Windows scheme of initialization sequence, with double
930	907	* reset of the device (Linux uses the same sequence)
931	908	* Some equipment is said to work only with such init sequence; this
932	909
933	910
934	911
...	...	@@ -940,55 +917,46 @@
940	917	* the maxpacket size is 8 or 16 the device may be waiting to transmit
941	918	* some more, or keeps on retransmitting the 8 byte header. */
942	919
	920	+ desc = (struct usb_device_descriptor *)tmpbuf;
943	921	dev->descriptor.bMaxPacketSize0 = 64; /* Start off at 64 bytes */
944	922	/* Default to 64 byte max packet size */
945	923	dev->maxpacketsize = PACKET_SIZE_64;
946	924	dev->epmaxpacketin[0] = 64;
947	925	dev->epmaxpacketout[0] = 64;
948	926
949		- /*
950		- * XHCI needs to issue a Address device command to setup
951		- * proper device context structures, before it can interact
952		- * with the device. So a get_descriptor will fail before any
953		- * of that is done for XHCI unlike EHCI.
954		- */
955		-#ifndef CONFIG_USB_XHCI
956		- struct usb_device_descriptor *desc;
	927	+ if (do_read) {
	928	+ int err;
957	929
958		- desc = (struct usb_device_descriptor *)tmpbuf;
959		- err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
960		- /*
961		- * Validate we've received only at least 8 bytes, not that we've
962		- * received the entire descriptor. The reasoning is:
963		- * - The code only uses fields in the first 8 bytes, so that's all we
964		- * need to have fetched at this stage.
965		- * - The smallest maxpacket size is 8 bytes. Before we know the actual
966		- * maxpacket the device uses, the USB controller may only accept a
967		- * single packet. Consequently we are only guaranteed to receive 1
968		- * packet (at least 8 bytes) even in a non-error case.
969		- *
970		- * At least the DWC2 controller needs to be programmed with the number
971		- * of packets in addition to the number of bytes. A request for 64
972		- * bytes of data with the maxpacket guessed as 64 (above) yields a
973		- * request for 1 packet.
974		- */
975		- if (err < 8) {
976		- debug("usb_new_device: usb_get_descriptor() failed\n");
977		- return -EIO;
	930	+ err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
	931	+ /*
	932	+ * Validate we've received only at least 8 bytes, not that we've
	933	+ * received the entire descriptor. The reasoning is:
	934	+ * - The code only uses fields in the first 8 bytes, so that's all we
	935	+ * need to have fetched at this stage.
	936	+ * - The smallest maxpacket size is 8 bytes. Before we know the actual
	937	+ * maxpacket the device uses, the USB controller may only accept a
	938	+ * single packet. Consequently we are only guaranteed to receive 1
	939	+ * packet (at least 8 bytes) even in a non-error case.
	940	+ *
	941	+ * At least the DWC2 controller needs to be programmed with the number
	942	+ * of packets in addition to the number of bytes. A request for 64
	943	+ * bytes of data with the maxpacket guessed as 64 (above) yields a
	944	+ * request for 1 packet.
	945	+ */
	946	+ if (err < 8) {
	947	+ if (err < 0) {
	948	+ printf("unable to get device descriptor (error=%d)\n",
	949	+ err);
	950	+ return err;
	951	+ } else {
	952	+ printf("USB device descriptor short read (expected %i, got %i)\n",
	953	+ (int)sizeof(dev->descriptor), err);
	954	+ return -EIO;
	955	+ }
	956	+ }
	957	+ memcpy(&dev->descriptor, tmpbuf, sizeof(dev->descriptor));
978	958	}
979	959
980		- dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
981		- /*
982		- * Fetch the device class, driver can use this info
983		- * to differentiate between HUB and DEVICE.
984		- */
985		- dev->descriptor.bDeviceClass = desc->bDeviceClass;
986		-#endif
987		-
988		- err = usb_legacy_port_reset(dev->parent, dev->portnr);
989		- if (err)
990		- return err;
991		-
992	960	dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;
993	961	dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;
994	962	switch (dev->descriptor.bMaxPacketSize0) {
...	...	@@ -1008,6 +976,55 @@
1008	976	printf("usb_new_device: invalid max packet size\n");
1009	977	return -EIO;
1010	978	}
	979	+
	980	+ return 0;
	981	+}
	982	+
	983	+/*
	984	+ * By the time we get here, the device has gotten a new device ID
	985	+ * and is in the default state. We need to identify the thing and
	986	+ * get the ball rolling..
	987	+ *
	988	+ * Returns 0 for success, != 0 for error.
	989	+ */
	990	+int usb_new_device(struct usb_device *dev)
	991	+{
	992	+ bool do_read = true;
	993	+ int addr, err;
	994	+ int tmp;
	995	+ ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
	996	+
	997	+ /*
	998	+ * Allocate usb 3.0 device context.
	999	+ * USB 3.0 (xHCI) protocol tries to allocate device slot
	1000	+ * and related data structures first. This call does that.
	1001	+ * Refer to sec 4.3.2 in xHCI spec rev1.0
	1002	+ */
	1003	+ if (usb_alloc_device(dev)) {
	1004	+ printf("Cannot allocate device context to get SLOT_ID\n");
	1005	+ return -1;
	1006	+ }
	1007	+
	1008	+ /* We still haven't set the Address yet */
	1009	+ addr = dev->devnum;
	1010	+ dev->devnum = 0;
	1011	+
	1012	+ /*
	1013	+ * XHCI needs to issue a Address device command to setup
	1014	+ * proper device context structures, before it can interact
	1015	+ * with the device. So a get_descriptor will fail before any
	1016	+ * of that is done for XHCI unlike EHCI.
	1017	+ */
	1018	+#ifdef CONFIG_USB_XHCI
	1019	+ do_read = false;
	1020	+#endif
	1021	+ err = usb_setup_descriptor(dev, do_read);
	1022	+ if (err)
	1023	+ return err;
	1024	+ err = usb_legacy_port_reset(dev->parent, dev->portnr);
	1025	+ if (err)
	1026	+ return err;
	1027	+
1011	1028	dev->devnum = addr;
1012	1029
1013	1030	err = usb_set_address(dev); /* set address */