* [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar @ 2014-09-17 7:43 Bo Shen 2014-09-17 10:10 ` Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Bo Shen @ 2014-09-17 7:43 UTC (permalink / raw) To: u-boot 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.shen@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; printf("Starting download of %d bytes\n", download_size); -- 2.1.0.24.g4109c28 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar 2014-09-17 7:43 [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar Bo Shen @ 2014-09-17 10:10 ` Marek Vasut 2014-09-17 10:28 ` Bo Shen 0 siblings, 1 reply; 6+ messages in thread From: Marek Vasut @ 2014-09-17 10:10 UTC (permalink / raw) To: u-boot 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.shen@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar 2014-09-17 10:10 ` Marek Vasut @ 2014-09-17 10:28 ` Bo Shen 2014-09-17 11:16 ` Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Bo Shen @ 2014-09-17 10:28 UTC (permalink / raw) To: u-boot 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.shen@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar 2014-09-17 10:28 ` Bo Shen @ 2014-09-17 11:16 ` Marek Vasut 2014-09-18 1:34 ` Bo Shen 0 siblings, 1 reply; 6+ messages in thread From: Marek Vasut @ 2014-09-17 11:16 UTC (permalink / raw) To: u-boot 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.shen@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 ? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar 2014-09-17 11:16 ` Marek Vasut @ 2014-09-18 1:34 ` Bo Shen 2014-09-18 2:31 ` Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Bo Shen @ 2014-09-18 1:34 UTC (permalink / raw) To: u-boot 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.shen@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar 2014-09-18 1:34 ` Bo Shen @ 2014-09-18 2:31 ` Marek Vasut 0 siblings, 0 replies; 6+ messages in thread From: Marek Vasut @ 2014-09-18 2:31 UTC (permalink / raw) To: u-boot 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.shen@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? ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-18 2:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-17 7:43 [U-Boot] [PATCH] usb: gadget: fastboot: improve download progress bar Bo Shen 2014-09-17 10:10 ` Marek Vasut 2014-09-17 10:28 ` Bo Shen 2014-09-17 11:16 ` Marek Vasut 2014-09-18 1:34 ` Bo Shen 2014-09-18 2:31 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox