[PATCH 1/2] uimage: fix misunderstanding in common/uimage.c

[vj]

On Sat, Nov 24, 2012 at 9:45 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj at jcrosoft.com> wrote:
> On 04:24 Sat 24 Nov     , Vicente Bergas wrote:
>> The option of reading the file at once was discarded because
>> the option of increasing the buffer size provided almost the
>> same benefit.
> nack
>
> this is mandatory for tftp fs support
>
> Best Regarfd,
> J.

I'll try to explain it more clearly:
Firstly file_to_sdram read files a chunck of 2 pages at a time because
not all file systems (e.g. TFTP) didn't report the file size.
This provided a slow transfer speed, because the overhead of reading
each chunck is nonzero.
So I proposed a First patch to file_to_sdram which first checked for a
known/correct file size. If the file size was unknown/incorrect the
original method was still used (so TFTP worked), else the whole file
was read at once.
Sascha Hauer proposed an alternative to this First patch of only
increase the chunck size, which, after benchmarking, was set to 32
pages and was considered a reasonable size. This was the Second patch
to file_to_sdram.
The performance between the First and the Second patches was
comparable, so the First was discarded in favor of the Second, which
is much more simple.
But yesterday I checked and found both patches were applied in git master.
Hope this clarifies the issue.

Best Regards,
  Vicente.

>>
>> Signed-off-by: Vicente Bergas <vicencb at gmail.com>
>> ---
>>  common/uimage.c | 24 ------------------------
>>  1 file changed, 24 deletions(-)
>>
>> diff --git a/common/uimage.c b/common/uimage.c
>> index 3f5a3d5..8bcbfdd 100644
>> --- a/common/uimage.c
>> +++ b/common/uimage.c
>> @@ -28,8 +28,6 @@
>>  #include <rtc.h>
>>  #include <filetype.h>
>>  #include <memory.h>
>> -#include <linux/stat.h>
>> -#include <sizes.h>
>>
>>  #ifdef CONFIG_UIMAGE_MULTI
>>  static inline int uimage_is_multi_image(struct uimage_handle *handle)
>> @@ -384,33 +382,11 @@ struct resource *file_to_sdram(const char *filename, unsigned long adr)
>>       size_t ofs = 0;
>>       size_t now;
>>       int fd;
>> -     struct stat s;
>>
>>       fd = open(filename, O_RDONLY);
>>       if (fd < 0)
>>               return NULL;
>>
>> -     /* TODO: use fstat(fd, &s) when implemented and avoid reopening file */
>> -     if (stat(filename, &s) == 0 && s.st_size <= SZ_1G) {
>> -             /*
>> -              * As the file size is known we can read it at once and improve
>> -              * transfer speed.
>> -              */
>> -             size = s.st_size;
>> -             res = request_sdram_region("image", adr, size);
>> -             if (!res) {
>> -                     printf("unable to request SDRAM 0x%08lx-0x%08lx\n",
>> -                             adr, adr + size - 1);
>> -                     goto out;
>> -             }
>> -
>> -             now = read_full(fd, (void *)(res->start), size);
>> -             if (now < size) {
>> -                     release_sdram_region(res);
>> -                     res = NULL;
>> -             }
>> -             goto out;
>> -     }
>>       while (1) {
>>               res = request_sdram_region("image", adr, size);
>>               if (!res) {
>> --
>> 1.8.0
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox