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@wwwdotorg.org