[U-Boot] [PATCH 5/6] usb: dwc2: add support for SPLIT transactions

[Stephen Warren]

On 12/12/2015 09:17 PM, Stefan Brüns wrote:
> In contrast to non-SPLIT transfers each transaction has to be submitted
> as an individual chunk. Handling of ACK/NAk/NYET handshakes depends on
> transaction (non-SPLIT/SSPLIT/CSPLIT), thus inline the HCINT flag handling.

> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c

>  int chunk_msg(struct dwc2_priv *priv, struct usb_device *dev,
> -	      unsigned long pipe, int *pid, int in, void *buffer, int len,
> -	      bool ignore_ack)
> +	      unsigned long pipe, int *pid, int in, void *buffer, int len)
...
> +	uint32_t hctsiz;
> +	uint32_t hcint;
> +	uint32_t hcint_rem;
> +	uint8_t do_split = 0;
> +	uint8_t complete_split = 0;
> +	uint8_t start_again = 0;
> +	uint8_t hub_addr = 0;
> +	uint8_t hub_port = 0;

Rather than inlining all this stuff into chunk_msg, I had always
intended to move the body of chunk_msg() into a new function e.g.
split_msg() that chunk_msg() called repeatedly for each chunk, with
split_msg() either performing just a single transaction, or performing
both a start/complete-split. That would keep the functions a bit simpler
and more focused.

Still, you've implemented this, so you get to choose how it works:-)

> +	/* Initialize channel */
> +	dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, dev, devnum, ep, in,
> +			eptype, max);

I'd suggest calling that for each transaction. Sure there's some
redundancy, but I think it's better than duplicating the write of 0x3fff
to hc_regs->hc_int ther and within the loop here. If you disagree, I'd
suggest at least pulling that write out of dwc_otg_hc_init() so it's
only in one place.

>  	do {
...
> -	} while ((done < len) && !stop_transfer);
> +	} while (((done < len) && !stop_transfer) || start_again);

Perhaps just clear start_again if stop_transfer is set, or something
like that, to avoid the more complex condition?

Overall this looks reasonable though so,
Acked-by: Stephen Warren <swarren at wwwdotorg.org>