public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 5/6] usb: dwc2: add support for SPLIT transactions
Date: Tue, 15 Dec 2015 20:36:02 -0700	[thread overview]
Message-ID: <5670DC22.4030105@wwwdotorg.org> (raw)
In-Reply-To: <1449980278-19881-6-git-send-email-stefan.bruens@rwth-aachen.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 <swarren@wwwdotorg.org>

  parent reply	other threads:[~2015-12-16  3:36 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-13  4:17 [U-Boot] [PATCH 0/6] usb: dwc2: add SPLIT transaction support Stefan Brüns
2015-12-13  4:17 ` [U-Boot] [PATCH 1/6] usb: dwc2: avoid out of bounds access Stefan Brüns
2015-12-13  4:41   ` Marek Vasut
2015-12-16  2:58   ` Stephen Warren
2015-12-16 10:29     ` Marek Vasut
2015-12-17  1:44       ` Stefan Bruens
2015-12-13  4:17 ` [U-Boot] [PATCH 2/6] usb: dwc2: Handle NAK during CONTROL DATA and STATUS stage Stefan Brüns
2015-12-13  4:46   ` Marek Vasut
2015-12-16  3:07   ` Stephen Warren
2015-12-17  3:09     ` Stefan Bruens
2015-12-13  4:17 ` [U-Boot] [PATCH 3/6] usb: dwc2: determine TT hub address and port for split transactions Stefan Brüns
2015-12-13  4:43   ` Marek Vasut
2015-12-16  3:17   ` Stephen Warren
2015-12-17  3:16     ` Stefan Bruens
2015-12-18  1:11       ` [U-Boot] [PATCH] usb: Move determination of TT hub address/port into seperate function Stefan Brüns
2015-12-18  2:35         ` Marek Vasut
2015-12-18 10:00         ` Hans de Goede
2015-12-19 17:17           ` Stefan Bruens
2015-12-19 18:27             ` Hans de Goede
2015-12-19 20:16               ` [U-Boot] [PATCH 1/2 v2] " Stefan Brüns
2015-12-19 20:16                 ` [U-Boot] [PATCH 2/2 v2] usb: musb: Fix hub port number for SPLIT transactions Stefan Brüns
2015-12-21 19:33                   ` Hans de Goede
2015-12-21 20:27                     ` Marek Vasut
2015-12-21 23:02                       ` Stefan Bruens
2015-12-21 23:08                         ` Marek Vasut
2015-12-19 20:26               ` [U-Boot] [PATCH 1/2 v3] usb: Move determination of TT hub address/port into seperate function Stefan Brüns
2015-12-20 19:12                 ` Hans de Goede
2015-12-21 19:32                   ` Hans de Goede
2015-12-13  4:17 ` [U-Boot] [PATCH 4/6] usb: dwc2: add helper function for setting SPLIT HC registers Stefan Brüns
2015-12-13  4:44   ` Marek Vasut
2015-12-16  3:19   ` Stephen Warren
2015-12-13  4:17 ` [U-Boot] [PATCH 5/6] usb: dwc2: add support for SPLIT transactions Stefan Brüns
2015-12-13  4:48   ` Marek Vasut
2015-12-16  3:36   ` Stephen Warren [this message]
2015-12-20  0:17     ` Stefan Bruens
2015-12-20  0:41       ` Marek Vasut
2015-12-13  4:17 ` [U-Boot] [PATCH 6/6] usb: dwc2: remove no longer used wait_for_chhltd() Stefan Brüns
2015-12-13  4:48   ` Marek Vasut
2015-12-16  3:36   ` Stephen Warren
2015-12-13  4:41 ` [U-Boot] [PATCH 0/6] usb: dwc2: add SPLIT transaction support Marek Vasut
2015-12-16  2:53 ` Stephen Warren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5670DC22.4030105@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox