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

[Bo Shen]

Hi Marek,

On 09/18/2014 10:32 AM, Marek Vasut wrote:
> 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 at 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 ?

No, I don't reset the "download_bytes" for each transfer. It reset the 
"download_bytes" before transfer start.
The "download_bytes" is increase in rx_handler_dl_image function.

> 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?

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.

Best Regards,
Bo Shen