Re: [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar

2014-09-18 Thread Marek Vasut
On Thursday, September 18, 2014 at 03:34:18 AM, Bo Shen wrote:
 Hi Marek,
 
 On 09/17/2014 07:16 PM, Marek Vasut wrote:
  On Wednesday, September 17, 2014 at 12:28:57 PM, Bo Shen wrote:
  Hi Marek,
  
  On 09/17/2014 06:10 PM, Marek Vasut wrote:
  On Wednesday, September 17, 2014 at 09:43:56 AM, Bo Shen wrote:
  
  +CC Lukasz, this is his turf.
  
  When download is ongoing, if the actual size of one transfer
  is not the same as BTYES_PER_DOT, which will cause the dot
  won't print anymore. Then it will let the user thinking it
  is stuck, actually it is transfering without dot printed.
  
  So, improve the method to show the progress bar (print dot).
  
  Signed-off-by: Bo Shen voice.s...@atmel.com
  ---
  
 drivers/usb/gadget/f_fastboot.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/usb/gadget/f_fastboot.c
  b/drivers/usb/gadget/f_fastboot.c index 7a1acb9..2f13bf0 100644
  --- a/drivers/usb/gadget/f_fastboot.c
  +++ b/drivers/usb/gadget/f_fastboot.c
  @@ -51,6 +51,7 @@ static inline struct f_fastboot
  *func_to_fastboot(struct usb_function *f) static struct f_fastboot
  *fastboot_func;
  
 static unsigned int download_size;
 static unsigned int download_bytes;
  
  +static unsigned int num_of_dot;
  
 static struct usb_endpoint_descriptor fs_ep_in = {
 
   .bLength= USB_DT_ENDPOINT_SIZE,
  
  @@ -414,9 +415,10 @@ static void rx_handler_dl_image(struct usb_ep
  *ep, struct usb_request *req) req-length = ep-maxpacket;
  
   }
  
  -if (download_bytes  !(download_bytes % BYTES_PER_DOT)) {
  +if (download_bytes  ((download_bytes / BYTES_PER_DOT) 
  num_of_dot)) { + num_of_dot = download_bytes / BYTES_PER_DOT;
  
   putc('.');
  
  -if (!(download_bytes % (74 * BYTES_PER_DOT)))
  +if (!(num_of_dot % 74))
  
   putc('\n');
   
   }
   req-actual = 0;
  
  @@ -431,6 +433,7 @@ static void cb_download(struct usb_ep *ep, struct
  usb_request *req) strsep(cmd, :);
  
   download_size = simple_strtoul(cmd, NULL, 16);
   download_bytes = 0;
  
  +num_of_dot = 0;
  
  Make it a 'download_total' and log the total amount of bytes
  transferred please, that way it can be re-used for other purposes in
  the future ; for example for printing how much data were already
  transferred ;-)
  
  The download_bytes record the total amount of bytes transferred.
  And the download_bytes will print after finishing transfer.
  
  So why can this not be used to indicate the total progress ? Because the
  transfeer speed is variating too much ?
 
 As I described in the commit message. If the transfer length is not
 exactly the same as the request length, then the old method
download_bytes % BYTES_PER_DOT
 won't be 0 anymore, so for the following transfer, it won't print dot
 anymore.

And can you not reset the download_bytes for each transfer ?

Maybe we're not even aligned on what transfer means, so we might want to sync 
on this word first. What does transfer mean in this case?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar

2014-09-17 Thread Marek Vasut
On Wednesday, September 17, 2014 at 09:43:56 AM, Bo Shen wrote:

+CC Lukasz, this is his turf.

 When download is ongoing, if the actual size of one transfer
 is not the same as BTYES_PER_DOT, which will cause the dot
 won't print anymore. Then it will let the user thinking it
 is stuck, actually it is transfering without dot printed.
 
 So, improve the method to show the progress bar (print dot).
 
 Signed-off-by: Bo Shen voice.s...@atmel.com
 ---
 
  drivers/usb/gadget/f_fastboot.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/gadget/f_fastboot.c
 b/drivers/usb/gadget/f_fastboot.c index 7a1acb9..2f13bf0 100644
 --- a/drivers/usb/gadget/f_fastboot.c
 +++ b/drivers/usb/gadget/f_fastboot.c
 @@ -51,6 +51,7 @@ static inline struct f_fastboot *func_to_fastboot(struct
 usb_function *f) static struct f_fastboot *fastboot_func;
  static unsigned int download_size;
  static unsigned int download_bytes;
 +static unsigned int num_of_dot;
 
  static struct usb_endpoint_descriptor fs_ep_in = {
   .bLength= USB_DT_ENDPOINT_SIZE,
 @@ -414,9 +415,10 @@ static void rx_handler_dl_image(struct usb_ep *ep,
 struct usb_request *req) req-length = ep-maxpacket;
   }
 
 - if (download_bytes  !(download_bytes % BYTES_PER_DOT)) {
 + if (download_bytes  ((download_bytes / BYTES_PER_DOT)  num_of_dot)) {
 + num_of_dot = download_bytes / BYTES_PER_DOT;
   putc('.');
 - if (!(download_bytes % (74 * BYTES_PER_DOT)))
 + if (!(num_of_dot % 74))
   putc('\n');
   }
   req-actual = 0;
 @@ -431,6 +433,7 @@ static void cb_download(struct usb_ep *ep, struct
 usb_request *req) strsep(cmd, :);
   download_size = simple_strtoul(cmd, NULL, 16);
   download_bytes = 0;
 + num_of_dot = 0;

Make it a 'download_total' and log the total amount of bytes transferred 
please, 
that way it can be re-used for other purposes in the future ; for example for 
printing how much data were already transferred ;-)

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar

2014-09-17 Thread Bo Shen

Hi Marek,

On 09/17/2014 06:10 PM, Marek Vasut wrote:

On Wednesday, September 17, 2014 at 09:43:56 AM, Bo Shen wrote:

+CC Lukasz, this is his turf.


When download is ongoing, if the actual size of one transfer
is not the same as BTYES_PER_DOT, which will cause the dot
won't print anymore. Then it will let the user thinking it
is stuck, actually it is transfering without dot printed.

So, improve the method to show the progress bar (print dot).

Signed-off-by: Bo Shen voice.s...@atmel.com
---

  drivers/usb/gadget/f_fastboot.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c
b/drivers/usb/gadget/f_fastboot.c index 7a1acb9..2f13bf0 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -51,6 +51,7 @@ static inline struct f_fastboot *func_to_fastboot(struct
usb_function *f) static struct f_fastboot *fastboot_func;
  static unsigned int download_size;
  static unsigned int download_bytes;
+static unsigned int num_of_dot;

  static struct usb_endpoint_descriptor fs_ep_in = {
.bLength= USB_DT_ENDPOINT_SIZE,
@@ -414,9 +415,10 @@ static void rx_handler_dl_image(struct usb_ep *ep,
struct usb_request *req) req-length = ep-maxpacket;
}

-   if (download_bytes  !(download_bytes % BYTES_PER_DOT)) {
+   if (download_bytes  ((download_bytes / BYTES_PER_DOT)  num_of_dot)) {
+   num_of_dot = download_bytes / BYTES_PER_DOT;
putc('.');
-   if (!(download_bytes % (74 * BYTES_PER_DOT)))
+   if (!(num_of_dot % 74))
putc('\n');
}
req-actual = 0;
@@ -431,6 +433,7 @@ static void cb_download(struct usb_ep *ep, struct
usb_request *req) strsep(cmd, :);
download_size = simple_strtoul(cmd, NULL, 16);
download_bytes = 0;
+   num_of_dot = 0;


Make it a 'download_total' and log the total amount of bytes transferred please,
that way it can be re-used for other purposes in the future ; for example for
printing how much data were already transferred ;-)


The download_bytes record the total amount of bytes transferred.
And the download_bytes will print after finishing transfer.


Best regards,
Marek Vasut



Best Regards,
Bo Shen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar

2014-09-17 Thread Marek Vasut
On Wednesday, September 17, 2014 at 12:28:57 PM, Bo Shen wrote:
 Hi Marek,
 
 On 09/17/2014 06:10 PM, Marek Vasut wrote:
  On Wednesday, September 17, 2014 at 09:43:56 AM, Bo Shen wrote:
  
  +CC Lukasz, this is his turf.
  
  When download is ongoing, if the actual size of one transfer
  is not the same as BTYES_PER_DOT, which will cause the dot
  won't print anymore. Then it will let the user thinking it
  is stuck, actually it is transfering without dot printed.
  
  So, improve the method to show the progress bar (print dot).
  
  Signed-off-by: Bo Shen voice.s...@atmel.com
  ---
  
drivers/usb/gadget/f_fastboot.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/usb/gadget/f_fastboot.c
  b/drivers/usb/gadget/f_fastboot.c index 7a1acb9..2f13bf0 100644
  --- a/drivers/usb/gadget/f_fastboot.c
  +++ b/drivers/usb/gadget/f_fastboot.c
  @@ -51,6 +51,7 @@ static inline struct f_fastboot
  *func_to_fastboot(struct usb_function *f) static struct f_fastboot
  *fastboot_func;
  
static unsigned int download_size;
static unsigned int download_bytes;
  
  +static unsigned int num_of_dot;
  
static struct usb_endpoint_descriptor fs_ep_in = {

 .bLength= USB_DT_ENDPOINT_SIZE,
  
  @@ -414,9 +415,10 @@ static void rx_handler_dl_image(struct usb_ep *ep,
  struct usb_request *req) req-length = ep-maxpacket;
  
 }
  
  -  if (download_bytes  !(download_bytes % BYTES_PER_DOT)) {
  +  if (download_bytes  ((download_bytes / BYTES_PER_DOT)  num_of_dot))
  { +num_of_dot = download_bytes / BYTES_PER_DOT;
  
 putc('.');
  
  -  if (!(download_bytes % (74 * BYTES_PER_DOT)))
  +  if (!(num_of_dot % 74))
  
 putc('\n');
 
 }
 req-actual = 0;
  
  @@ -431,6 +433,7 @@ static void cb_download(struct usb_ep *ep, struct
  usb_request *req) strsep(cmd, :);
  
 download_size = simple_strtoul(cmd, NULL, 16);
 download_bytes = 0;
  
  +  num_of_dot = 0;
  
  Make it a 'download_total' and log the total amount of bytes transferred
  please, that way it can be re-used for other purposes in the future ;
  for example for printing how much data were already transferred ;-)
 
 The download_bytes record the total amount of bytes transferred.
 And the download_bytes will print after finishing transfer.

So why can this not be used to indicate the total progress ? Because the 
transfeer speed is variating too much ?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar

2014-09-17 Thread Bo Shen

Hi Marek,

On 09/17/2014 07:16 PM, Marek Vasut wrote:

On Wednesday, September 17, 2014 at 12:28:57 PM, Bo Shen wrote:

Hi Marek,

On 09/17/2014 06:10 PM, Marek Vasut wrote:

On Wednesday, September 17, 2014 at 09:43:56 AM, Bo Shen wrote:

+CC Lukasz, this is his turf.


When download is ongoing, if the actual size of one transfer
is not the same as BTYES_PER_DOT, which will cause the dot
won't print anymore. Then it will let the user thinking it
is stuck, actually it is transfering without dot printed.

So, improve the method to show the progress bar (print dot).

Signed-off-by: Bo Shen voice.s...@atmel.com
---

   drivers/usb/gadget/f_fastboot.c | 7 +--
   1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c
b/drivers/usb/gadget/f_fastboot.c index 7a1acb9..2f13bf0 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -51,6 +51,7 @@ static inline struct f_fastboot
*func_to_fastboot(struct usb_function *f) static struct f_fastboot
*fastboot_func;

   static unsigned int download_size;
   static unsigned int download_bytes;

+static unsigned int num_of_dot;

   static struct usb_endpoint_descriptor fs_ep_in = {

.bLength= USB_DT_ENDPOINT_SIZE,

@@ -414,9 +415,10 @@ static void rx_handler_dl_image(struct usb_ep *ep,
struct usb_request *req) req-length = ep-maxpacket;

}

-   if (download_bytes  !(download_bytes % BYTES_PER_DOT)) {
+   if (download_bytes  ((download_bytes / BYTES_PER_DOT)  num_of_dot))
{ + num_of_dot = download_bytes / BYTES_PER_DOT;

putc('.');

-   if (!(download_bytes % (74 * BYTES_PER_DOT)))
+   if (!(num_of_dot % 74))

putc('\n');

}
req-actual = 0;

@@ -431,6 +433,7 @@ static void cb_download(struct usb_ep *ep, struct
usb_request *req) strsep(cmd, :);

download_size = simple_strtoul(cmd, NULL, 16);
download_bytes = 0;

+   num_of_dot = 0;


Make it a 'download_total' and log the total amount of bytes transferred
please, that way it can be re-used for other purposes in the future ;
for example for printing how much data were already transferred ;-)


The download_bytes record the total amount of bytes transferred.
And the download_bytes will print after finishing transfer.


So why can this not be used to indicate the total progress ? Because the
transfeer speed is variating too much ?


As I described in the commit message. If the transfer length is not 
exactly the same as the request length, then the old method

  download_bytes % BYTES_PER_DOT
won't be 0 anymore, so for the following transfer, it won't print dot 
anymore.


Best Regards,
Bo Shen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot