All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.