[U-Boot] [PATCH 7/7] usb: dwc2: Do not mix data toggle for IN and OUT endpoints, check bounds

[U-Boot] [PATCH 7/7] usb: dwc2: Do not mix data toggle for IN and OUT endpoints, check bounds

[Stefan Brüns]

USB protocol allows for 16 IN and 16 OUT endpoints (USB 2.0 Spec,
8.3.2.2 Endpoint Field). A function may have an EP 1 for both IN and OUT,
so these two should be kept separate. As EPs are either BULK or INTERRUPT
(or ISO), it is fine to have one array per direction for all transfer
types (also see e236519b7365ef75c5da6a5623f0b03d9c00cfae).

USB device address is 7 bits, so a bus may have more than 16 devices.
Check the device number, as the DWC2 driver only supports BULK/ISO for
the first 16 devices.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/usb/host/dwc2.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 291e4a5..7c107bc 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -34,7 +34,8 @@ struct dwc2_priv {
 	uint8_t *aligned_buffer;
 	uint8_t *status_buffer;
 #endif
-	int bulk_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
+	uint8_t in_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
+	uint8_t out_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
 	struct dwc2_core_regs *regs;
 	int root_hub_devnum;
 };
@@ -739,7 +740,7 @@ static int dwc_otg_submit_rh_msg(struct dwc2_priv *priv, struct usb_device *dev,
 	return stat;
 }
 
-int wait_for_chhltd(struct dwc2_hc_regs *hc_regs, uint32_t *sub, int *toggle)
+int wait_for_chhltd(struct dwc2_hc_regs *hc_regs, uint32_t *sub, uint8_t *toggle)
 {
 	int ret;
 	uint32_t hcint, hctsiz;
@@ -775,7 +776,7 @@ static int dwc2_eptype[] = {
 };
 
 static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer,
-			  int *pid, int in, void *buffer, int num_packets,
+			  uint8_t *pid, int in, void *buffer, int num_packets,
 			  int xfer_len, int *actual_len, int odd_frame)
 {
 	int ret = 0;
@@ -829,7 +830,7 @@ static int transfer_chunk(struct dwc2_hc_regs *hc_regs, void *aligned_buffer,
 }
 
 int chunk_msg(struct dwc2_priv *priv, struct usb_device *dev,
-	      unsigned long pipe, int *pid, int in, void *buffer, int len)
+	      unsigned long pipe, uint8_t *pid, int in, void *buffer, int len)
 {
 	struct dwc2_core_regs *regs = priv->regs;
 	struct dwc2_hc_regs *hc_regs = &regs->hc_regs[DWC2_HC_CHANNEL];
@@ -960,14 +961,19 @@ int _submit_bulk_msg(struct dwc2_priv *priv, struct usb_device *dev,
 {
 	int devnum = usb_pipedevice(pipe);
 	int ep = usb_pipeendpoint(pipe);
+	uint8_t* pid;
 
-	if (devnum == priv->root_hub_devnum) {
+	if ((devnum >= MAX_DEVICE) || (devnum == priv->root_hub_devnum)) {
 		dev->status = 0;
 		return -EINVAL;
 	}
 
-	return chunk_msg(priv, dev, pipe, &priv->bulk_data_toggle[devnum][ep],
-			 usb_pipein(pipe), buffer, len);
+	if (usb_pipein(pipe))
+		pid = &priv->in_data_toggle[devnum][ep];
+	else
+		pid = &priv->out_data_toggle[devnum][ep];
+
+	return chunk_msg(priv, dev, pipe, pid, usb_pipein(pipe), buffer, len);
 }
 
 static int _submit_control_msg(struct dwc2_priv *priv, struct usb_device *dev,
@@ -975,7 +981,8 @@ static int _submit_control_msg(struct dwc2_priv *priv, struct usb_device *dev,
 			       struct devrequest *setup)
 {
 	int devnum = usb_pipedevice(pipe);
-	int pid, ret, act_len;
+	int ret, act_len;
+	uint8_t pid;
 	/* For CONTROL endpoint pid should start with DATA1 */
 	int status_direction;
 
@@ -1075,8 +1082,10 @@ static int dwc2_init_common(struct dwc2_priv *priv)
 		     DWC2_HPRT0_PRTRST);
 
 	for (i = 0; i < MAX_DEVICE; i++) {
-		for (j = 0; j < MAX_ENDPOINT; j++)
-			priv->bulk_data_toggle[i][j] = DWC2_HC_PID_DATA0;
+		for (j = 0; j < MAX_ENDPOINT; j++) {
+			priv->in_data_toggle[i][j] = DWC2_HC_PID_DATA0;
+			priv->out_data_toggle[i][j] = DWC2_HC_PID_DATA0;
+		}
 	}
 
 	return 0;
-- 
2.1.4

[U-Boot] [PATCH 7/7] usb: dwc2: Do not mix data toggle for IN and OUT endpoints, check bounds

[Stefan Bruens]

On Freitag, 22. Januar 2016 02:30:43 CET you wrote:
> USB protocol allows for 16 IN and 16 OUT endpoints (USB 2.0 Spec,
> 8.3.2.2 Endpoint Field). A function may have an EP 1 for both IN and OUT,
> so these two should be kept separate. As EPs are either BULK or INTERRUPT
> (or ISO), it is fine to have one array per direction for all transfer
> types (also see e236519b7365ef75c5da6a5623f0b03d9c00cfae).
> 
> USB device address is 7 bits, so a bus may have more than 16 devices.
> Check the device number, as the DWC2 driver only supports BULK/ISO for
> the first 16 devices.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/usb/host/dwc2.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)

This one goes on top of the SPLIT support patches, if anyone is wondering why 
it is labeled Patch 7/7.

Kind regards,

Stefan

-- 
Stefan Br?ns  /  Bergstra?e 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
work: +49 2405 49936-424

[U-Boot] [PATCH 7/7] usb: dwc2: Do not mix data toggle for IN and OUT endpoints, check bounds

[Marek Vasut]

On Friday, January 22, 2016 at 02:30:43 AM, Stefan Br?ns wrote:
> USB protocol allows for 16 IN and 16 OUT endpoints (USB 2.0 Spec,
> 8.3.2.2 Endpoint Field). A function may have an EP 1 for both IN and OUT,
> so these two should be kept separate. As EPs are either BULK or INTERRUPT
> (or ISO), it is fine to have one array per direction for all transfer
> types (also see e236519b7365ef75c5da6a5623f0b03d9c00cfae).
> 
> USB device address is 7 bits, so a bus may have more than 16 devices.
> Check the device number, as the DWC2 driver only supports BULK/ISO for
> the first 16 devices.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---

Since this is patch 7/7, where are the remaining 6 ?

>  drivers/usb/host/dwc2.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index 291e4a5..7c107bc 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -34,7 +34,8 @@ struct dwc2_priv {
>  	uint8_t *aligned_buffer;
>  	uint8_t *status_buffer;
>  #endif
> -	int bulk_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
> +	uint8_t in_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
> +	uint8_t out_data_toggle[MAX_DEVICE][MAX_ENDPOINT];

Use u8/u16/u32 please. All around the place .

>  	struct dwc2_core_regs *regs;
>  	int root_hub_devnum;
>  };

The rest is fine,

Acked-by: Marek Vasut <marex@denx.de>

Thanks!

Best regards,
Marek Vasut