public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] FW: [PATCH] usb: gadget: fastboot: improve download progress bar
Date: Fri, 19 Sep 2014 06:24:32 +0200	[thread overview]
Message-ID: <201409190624.32446.marex@denx.de> (raw)
In-Reply-To: <541BA289.1080507@atmel.com>

On Friday, September 19, 2014 at 05:27:05 AM, Bo Shen wrote:
> Hi Marek,
[...]
> >> Transfer I mean here is a usb request, which trying to transfer
> >> EP_BUFFER_SIZE at one time.
> >> In my test case, sometime it transfer less than EP_BUFFER_SIZE in a usb
> >> request. So, it cause dot won't print the dot, and seems stuck. However,
> >> it will finish transfer after some time.
> > 
> > I see now. This code is really weird.
> > 
> > What would happen if the following condition is met in the code for k>0 ?
> > (download_bytes == download_size) AND (download_bytes = k *
> > BYTES_PER_DOT)
> 
> I am not fully understand what you plan to present here.
> 
> > I think the original code would happily print a dot after printing this
> > output: printf("\ndownloading of %d bytes finished\n", download_bytes);
> > 
> > Do you agree ? If yes, then I believe this code should go into the else
> > branch only.
> 
> Yes, I agree. This may happen, if the (download_bytes % BTYES_PER_DOT)
> equals to 0.
> 
> > Also, you can probably avoid the counting variable if you do something
> > like:
> > 
> > if (download_bytes / CONST != (download_bytes + transfer_size) / CONST) {
> > 
> > 	print(dot);
> > 	if (download_bytes / (74 * CONST) != ((download_bytes + transfer_size) /
> > 
> > (74 * CONST))
> > 
> > 		print(\n);
> > 
> > }
> > 
> > Surely, the code can be simplified . You would also need to be careful
> > about this assignment at the top of the function : download_bytes +=
> > transfer_size;
> > 
> > What do you think ?
> 
> I think this piece of code is better, which won't introduce new variable.
> 
> If no other comments, I will modify the code like this and send the v2
> patch.

Well OK then. Thanks!

      reply	other threads:[~2014-09-19  4:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <ABFEDD1A8D2AAF42A54E3C074D49DC729DB2AD9B@penmbx01>
2014-09-18  8:11 ` [U-Boot] FW: [PATCH] usb: gadget: fastboot: improve download progress bar Bo Shen
2014-09-18  9:59   ` Marek Vasut
2014-09-19  3:27     ` Bo Shen
2014-09-19  4:24       ` Marek Vasut [this message]

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=201409190624.32446.marex@denx.de \
    --to=marex@denx.de \
    --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