From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Tue, 15 Dec 2015 20:36:02 -0700 Subject: [U-Boot] [PATCH 5/6] usb: dwc2: add support for SPLIT transactions In-Reply-To: <1449980278-19881-6-git-send-email-stefan.bruens@rwth-aachen.de> References: <1449980278-19881-1-git-send-email-stefan.bruens@rwth-aachen.de> <1449980278-19881-6-git-send-email-stefan.bruens@rwth-aachen.de> Message-ID: <5670DC22.4030105@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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