Upgraded HCD transfers

Types for data request are fixed and commented.

Control transfer has additional call for proper completion validation.

drivers/usbd/hcd/hcd.c

@@ -33,6 +33,7 @@ static int hcd_non_control_urb(hcd_device_state *, hcd_urb *);
 
 /* For internal use by more general methods */
 static int hcd_setup_packet(hcd_device_state *, hcd_ctrlrequest *, hcd_reg1);
+static int hcd_finish_setup(hcd_device_state *, void *, hcd_reg4);
 static int hcd_data_transfer(hcd_device_state *, hcd_datarequest *);
 
 
@@ -45,6 +46,10 @@ static int hcd_data_transfer(hcd_device_state *, hcd_datarequest *);
  * have to be altered to allow that */
 static hcd_device_state hcd_device[1];
 
+/* TODO: This was added for compatibility with DDELinux drivers that
+ * allow receiving less data than expected in URB, without error */
+#define HCD_ANY_LENGTH 0xFFFFFFFFu
+
 
 /*===========================================================================*
  *    hcd_handle_event                                                       *
@@ -267,8 +272,10 @@ hcd_get_device_descriptor(hcd_device_state * this_device)
 	}
 
 	/* Put what was read in device descriptor */
-	memcpy(&(this_device->device_desc), this_device->buffer,
+	if (EXIT_SUCCESS != hcd_finish_setup(this_device,
-		sizeof(this_device->device_desc));
+					&(this_device->device_desc),
+					sizeof(this_device->device_desc)))
+		return EXIT_FAILURE;
 
 	/* Remember max packet size from device descriptor */
 	this_device->max_packet_size = this_device->device_desc.bMaxPacketSize;
@@ -378,9 +385,12 @@ hcd_get_descriptor_tree(hcd_device_state * this_device)
 		 * then ask again for other descriptors */
 		if (sizeof(config_descriptor) == buffer_length) {
 
-			/* Put what was read in configuration descriptor */
+			/* Put what was already read in configuration
-			memcpy(&config_descriptor, this_device->buffer,
+			 * descriptor for analysis */
-				sizeof(config_descriptor));
+			if (EXIT_SUCCESS != hcd_finish_setup(this_device,
+							&config_descriptor,
+							buffer_length))
+				return EXIT_FAILURE;
 
 			/* Continue only if there is more data */
 			total_length = UGETW(config_descriptor.wTotalLength);
@@ -404,9 +414,13 @@ hcd_get_descriptor_tree(hcd_device_state * this_device)
 	}
 	while (!completed);
 
-	/* Create tree based on received buffer */
+	/* Validate... */
-	if (EXIT_SUCCESS != hcd_buffer_to_tree(this_device->buffer,
+	if (EXIT_SUCCESS != hcd_finish_setup(this_device, NULL, total_length))
-						this_device->data_len,
+		return EXIT_FAILURE;
+
+	/* ... and create tree based on received buffer */
+	if (EXIT_SUCCESS != hcd_buffer_to_tree(this_device->control_data,
+						this_device->control_len,
 						&(this_device->config_tree))) {
 		/* This should never happen for a fine device */
 		USB_MSG("Illegal descriptor values");
@@ -532,10 +546,14 @@ hcd_control_urb(hcd_device_state * this_device, hcd_urb * urb)
 		return EXIT_FAILURE;
 	}
 
-	/* TODO: Calling memcpy may be removed when writing directly to URB */
 	/* Put what was read back into URB */
-	memcpy(urb->inout_data, this_device->buffer, this_device->data_len);
+	if (EXIT_SUCCESS != hcd_finish_setup(this_device,
-	urb->out_size = this_device->data_len;
+					urb->inout_data,
+					HCD_ANY_LENGTH))
+		return EXIT_FAILURE;
+
+	/* Write transfer output info to URB */
+	urb->out_size = (hcd_reg4)this_device->control_len;
 	urb->inout_status = EXIT_SUCCESS;
 
 	return EXIT_SUCCESS;
@@ -591,7 +609,7 @@ hcd_non_control_urb(hcd_device_state * this_device, hcd_urb * urb)
 	request.type = urb->type;
 	request.endpoint = urb->endpoint;
 	request.direction = urb->direction;
-	request.data_left = urb->in_size;
+	request.data_left = (int)urb->in_size;
 	request.data = urb->inout_data;
 	request.interval = urb->interval;
 
@@ -608,8 +626,10 @@ hcd_non_control_urb(hcd_device_state * this_device, hcd_urb * urb)
 		return EXIT_FAILURE;
 	}
 
-	/* Transfer successfully completed */
+	/* Transfer successfully completed update URB */
-	urb->out_size = urb->in_size - request.data_left;
+	USB_ASSERT(request.data_left >= 0,
+		"Negative amount of transfer data remains");
+	urb->out_size = urb->in_size - (hcd_reg4)request.data_left;
 	urb->inout_status = EXIT_SUCCESS;
 
 	return EXIT_SUCCESS;
@@ -638,8 +658,8 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
 
 	/* Initially... */
 	d = this_device->driver;
-	current_byte = this_device->buffer;	/* Start reading into this */
+	current_byte = this_device->control_data;/* Start reading into this */
-	this_device->data_len = 0;		/* Nothing read yet */
+	this_device->control_len = 0;		/* Nothing read yet */
 
 	/* Set parameters for further communication */
 	d->setup_device(d->private_data, ep, this_device->address);
@@ -685,7 +705,7 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
 
 				/* Increment */
 				current_byte += rx_len;
-				this_device->data_len += rx_len;
+				this_device->control_len += rx_len;
 
 				/* If full max sized packet was read... */
 				if (rx_len == (int)this_device->max_packet_size)
@@ -752,6 +772,45 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
 }
 
 
+/*===========================================================================*
+ *    hcd_finish_setup                                                       *
+ *===========================================================================*/
+static int
+hcd_finish_setup(hcd_device_state * this_device, void * output,
+		hcd_reg4 expected)
+{
+	DEBUG_DUMP;
+
+	/* Validate setup transfer output length */
+	if (this_device->control_len < 0) {
+		USB_MSG("Negative control transfer output length");
+		return EXIT_FAILURE;
+	}
+
+	/* In case it is required... */
+	if (HCD_ANY_LENGTH != expected) {
+		/* ...check for expected length */
+		if ((hcd_reg4)this_device->control_len != expected) {
+			USB_MSG("Control transfer output length mismatch");
+			return EXIT_FAILURE;
+		}
+
+		/* Valid but there is no need to copy anything */
+		if (0u == expected)
+			return EXIT_SUCCESS;
+	}
+
+	/* Length is valid but output not supplied */
+	if (NULL == output)
+		return EXIT_SUCCESS;
+
+	/* Finally, copy when needed */
+	memcpy(output, this_device->control_data, this_device->control_len);
+
+	return EXIT_SUCCESS;
+}
+
+
 /*===========================================================================*
  *    hcd_data_transfer                                                      *
  *===========================================================================*/
@@ -764,21 +823,22 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
 
 	DEBUG_DUMP;
 
-	USB_ASSERT((hcd_reg1)(UE_GET_ADDR(request->endpoint)) <= HCD_LAST_EP,
+	USB_ASSERT((request->endpoint <= HCD_LAST_EP) &&
+		(request->endpoint > HCD_DEFAULT_EP),
 		"Invalid EP number");
-	USB_ASSERT(this_device->address <= HCD_LAST_ADDR,
+	USB_ASSERT((this_device->address <= HCD_LAST_ADDR) &&
+		(this_device->address > HCD_DEFAULT_ADDR),
 		"Invalid device address");
 
 	/* Initially... */
 	d = this_device->driver;
 
 	/* Set parameters for further communication */
-	d->setup_device(d->private_data,
+	d->setup_device(d->private_data, request->endpoint,
-			(hcd_reg1)request->endpoint,
 			this_device->address);
 
-	/* TODO: broken USB_IN... constants */
+	/* Check transfer direction first */
-	if (1 == request->direction) {
+	if (HCD_DIRECTION_IN == request->direction) {
 
 		do {
 			/* Start actual data transfer */
@@ -786,7 +846,7 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
 
 			/* Wait for response */
 			hcd_device_wait(this_device, HCD_EVENT_ENDPOINT,
-					(hcd_reg1)request->endpoint);
+					request->endpoint);
 
 			/* Check response */
 			if (EXIT_SUCCESS != d->check_error(d->private_data,
@@ -796,8 +856,8 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
 
 			/* Read data received as response */
 			transfer_len = d->read_data(d->private_data,
-						(hcd_reg1 *)request->data,
+						request->data,
-						(hcd_reg1)request->endpoint);
+						request->endpoint);
 
 			request->data_left -= transfer_len;
 			request->data += transfer_len;
@@ -810,14 +870,15 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
 
 		} while (0 != request->data_left);
 
-	} else if (0 == request->direction) {
+	} else if (HCD_DIRECTION_OUT == request->direction) {
 
 		do {
 			temp_req = *request;
 
 			/* Decide temporary transfer size */
 			if (temp_req.data_left > (int)temp_req.max_packet_size)
-				temp_req.data_left = temp_req.max_packet_size;
+				temp_req.data_left =
+					(int)temp_req.max_packet_size;
 
 			/* Alter actual transfer size */
 			request->data += temp_req.data_left;
@@ -832,7 +893,7 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
 
 			/* Wait for response */
 			hcd_device_wait(this_device, HCD_EVENT_ENDPOINT,
-					(hcd_reg1)request->endpoint);
+					request->endpoint);
 
 			/* Check response */
 			if (EXIT_SUCCESS != d->check_error(d->private_data,

drivers/usbd/hcd/hcd_ddekit.c

@@ -352,7 +352,7 @@ hcd_decode_urb(hcd_urb * urb, struct ddekit_usb_urb * dde_urb)
 
 	/* TODO: Alignment of setup packet. Can DDE client guarantee that? */
 	/* Transfer data assignment */
-	urb->inout_data = (void *)dde_urb->data;
+	urb->inout_data = (hcd_reg1 *)dde_urb->data;
 	urb->in_setup = (hcd_ctrlrequest *)dde_urb->setup_packet;
 
 	/* TODO: Sane size check? */

drivers/usbd/hcd/musb/musb_core.c

@@ -470,7 +470,7 @@ musb_rx_stage(void * cfg, hcd_datarequest * request)
 	core = (musb_core_config *)cfg;
 	r = core->regs;
 
-	USB_ASSERT(request->max_packet_size <= 1024,
+	USB_ASSERT(request->max_packet_size <= HCD_MAX_MAXPACKETSIZE,
 			"Invalid wMaxPacketSize");
 	USB_ASSERT((core->ep <= HCD_LAST_EP) && (core->ep > HCD_DEFAULT_EP),
 			"Invalid bulk EP supplied");
@@ -564,7 +564,7 @@ musb_tx_stage(void * cfg, hcd_datarequest * request)
 	core = (musb_core_config *)cfg;
 	r = core->regs;
 
-	USB_ASSERT(request->max_packet_size <= 1024,
+	USB_ASSERT(request->max_packet_size <= HCD_MAX_MAXPACKETSIZE,
 			"Invalid wMaxPacketSize");
 	USB_ASSERT((core->ep <= HCD_LAST_EP) && (core->ep > HCD_DEFAULT_EP),
 			"Invalid bulk EP supplied");

drivers/usbd/include/usb/hcd_common.h

@@ -171,14 +171,16 @@ typedef struct hcd_device_state		hcd_device_state;
 /* Non-control transfer request structure */
 struct hcd_datarequest {
 
-	char * data;
+	hcd_reg1 * data;		/* RX/TX buffer */
-	int data_left;
+	int data_left;			/* Amount of data to transfer (may
-	int endpoint;
+					 * become negative in case of error
-	int direction;
+					 * thus 'int') */
-	unsigned int max_packet_size;
+	hcd_reg2 max_packet_size;	/* Read from EP descriptor */
-	unsigned int interval;
+	hcd_reg1 endpoint;		/* EP number */
-	hcd_speed speed;
+	hcd_reg1 interval;		/* Should match one in EP descriptor */
-	hcd_transfer type;
+	hcd_direction direction;	/* Should match one in EP descriptor */
+	hcd_speed speed;		/* Decided during device reset */
+	hcd_transfer type;		/* Should match one in EP descriptor */
 };
 
 /* HCD's URB structure */
@@ -190,9 +192,9 @@ struct hcd_urb {
 
 	/* Transfer (in/out signifies what may be overwritten by HCD) */
 	hcd_ctrlrequest * in_setup;
-	void * inout_data;
+	hcd_reg1 * inout_data;
 	hcd_reg4 in_size;
-	int out_size;
+	hcd_reg4 out_size;
 	int inout_status;	/* URB submission/validity status */
 
 	/* Transfer control */
@@ -218,11 +220,16 @@ struct hcd_device_state {
 	hcd_state state;
 	hcd_reg1 address;
 
-	/* Number of bytes received/transmitted in last transfer */
+	/*
-	int data_len;
+	 * Control transfer's local data:
+	 */
+
+	/* Number of bytes received/transmitted in last control transfer (may
+	 * become negative in case of error thus 'int') */
+	int control_len;
 
 	/* Word aligned buffer for each device to hold transfered data */
-	hcd_reg1 buffer[MAX_WTOTALLENGTH] __aligned(sizeof(hcd_reg4));
+	hcd_reg1 control_data[MAX_WTOTALLENGTH] __aligned(sizeof(hcd_reg4));
 };
 
 
@@ -258,6 +265,7 @@ struct hcd_device_state {
 /* Default MaxPacketSize for control transfer */
 #define HCD_LS_MAXPACKETSIZE		8u
 #define HCD_HS_MAXPACKETSIZE		64u
+#define HCD_MAX_MAXPACKETSIZE		1024u
 
 
 /*===========================================================================*