[PATCH 0/2] Some corrections on archosG9

[PATCH 0/2] Some corrections on archosG9

[Vicente Bergas]

In the patch series for archosG9 there were an attempt to improve file
transfers by reading files at once, but finally this solution was discarded.
Today I've checked the repository and found this solution there, perhaps this
is a misunderstanding and it's corrected here.
Sorry for not checking before.

The other change is just the inverse: a submitted patch that was not applied.

Vicente Bergas (2):
  uimage: fix misunderstanding in common/uimage.c
  archosg9: improve configuration

 arch/arm/boards/archosg9/env/config       |  4 +++-
 arch/arm/boards/archosg9/env/init/usbboot |  9 +++++----
 arch/arm/configs/archosg9_defconfig       |  2 +-
 common/uimage.c                           | 24 ------------------------
 4 files changed, 9 insertions(+), 30 deletions(-)

-- 
1.8.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

[Vicente Bergas]

The option of reading the file at once was discarded because
the option of increasing the buffer size provided almost the
same benefit.

Signed-off-by: Vicente Bergas <vicencb@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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] archosg9: improve configuration

[Vicente Bergas]

Signed-off-by: Vicente Bergas <vicencb@gmail.com>
---
 arch/arm/boards/archosg9/env/config       | 4 +++-
 arch/arm/boards/archosg9/env/init/usbboot | 9 +++++----
 arch/arm/configs/archosg9_defconfig       | 2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boards/archosg9/env/config b/arch/arm/boards/archosg9/env/config
index 38fc51c..8f3edb0 100644
--- a/arch/arm/boards/archosg9/env/config
+++ b/arch/arm/boards/archosg9/env/config
@@ -1 +1,3 @@
-global.linux.bootargs.base="earlyprintk=serial console=ttyO1,1000000n8 keep_bootcon pm_disable initcall_debug ignore_loglevel no_console_suspend=1 root=/dev/ram0 init=/linuxrc"
+global.bootm.image="${TARGET_ROOT}/boot/zImage"
+global.bootm.initrd="${TARGET_ROOT}/boot/initrd"
+global.linux.bootargs.base="console=ttyO0,115200n8 root=/dev/ram0"
diff --git a/arch/arm/boards/archosg9/env/init/usbboot b/arch/arm/boards/archosg9/env/init/usbboot
index 25536e2..83dd19a 100644
--- a/arch/arm/boards/archosg9/env/init/usbboot
+++ b/arch/arm/boards/archosg9/env/init/usbboot
@@ -1,5 +1,6 @@
-mkdir /usb
-mount -t omap4_usbbootfs omap4_usbboot /usb
+TARGET_ROOT="/mnt/usb"
+mkdir ${TARGET_ROOT}
+mount -t omap4_usbbootfs omap4_usbboot ${TARGET_ROOT}
 . /env/config
-. /usb/boot/config
-bootm -r /usb/boot/initrd /usb/boot/zImage
+. ${TARGET_ROOT}/boot/config
+bootm
diff --git a/arch/arm/configs/archosg9_defconfig b/arch/arm/configs/archosg9_defconfig
index e598258..2a20dd7 100644
--- a/arch/arm/configs/archosg9_defconfig
+++ b/arch/arm/configs/archosg9_defconfig
@@ -55,7 +55,7 @@ CONFIG_CMD_I2C=y
 CONFIG_DRIVER_SERIAL_OMAP4_USBBOOT=y
 CONFIG_DRIVER_SERIAL_NS16550=y
 CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS=y
-CONFIG_BAUDRATE=1000000
+CONFIG_BAUDRATE=115200
 # CONFIG_SPI is not set
 CONFIG_I2C=y
 CONFIG_I2C_OMAP=y
-- 
1.8.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

[Jean-Christophe PLAGNIOL-VILLARD]

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.
> 
> Signed-off-by: Vicente Bergas <vicencb@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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [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@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@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@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] Some corrections on archosG9

[Sascha Hauer]

On Sat, Nov 24, 2012 at 04:24:22AM +0100, Vicente Bergas wrote:
> In the patch series for archosG9 there were an attempt to improve file
> transfers by reading files at once, but finally this solution was discarded.
> Today I've checked the repository and found this solution there, perhaps this
> is a misunderstanding and it's corrected here.
> Sorry for not checking before.
> 
> The other change is just the inverse: a submitted patch that was not applied.
> 
> Vicente Bergas (2):
>   uimage: fix misunderstanding in common/uimage.c
>   archosg9: improve configuration
> 
>  arch/arm/boards/archosg9/env/config       |  4 +++-
>  arch/arm/boards/archosg9/env/init/usbboot |  9 +++++----
>  arch/arm/configs/archosg9_defconfig       |  2 +-
>  common/uimage.c                           | 24 ------------------------
>  4 files changed, 9 insertions(+), 30 deletions(-)

Applied, thanks

As a side note, I still have this patch in my queue which fixes
read_file for tftp severs which do not pass the size. My tftp server at
home is one of these, I'll repost the patch once I tested it again.

Sascha


From 1508618b3bc53abf122b9c7473f468e67b391b19 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Sun, 18 Mar 2012 17:59:46 +0100
Subject: [PATCH] read_file: Make it work on tftp servers which do not pass
 size

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c      |   11 +++++++++++
 fs/tftp.c    |    5 ++++-
 include/fs.h |    2 ++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index db4621a..871dc26 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -41,10 +41,21 @@ void *read_file(const char *filename, size_t *size)
 	int fd;
 	struct stat s;
 	void *buf = NULL;
+	const char *tmpfile = "/_read_file_tmp";
+	int ret;
 
+again:
 	if (stat(filename, &s))
 		return NULL;
 
+	if (s.st_size == FILESIZE_MAX) {
+		ret = copy_file(filename, tmpfile, 0);
+		if (ret)
+			return NULL;
+		filename = tmpfile;
+		goto again;
+	}
+
 	buf = xzalloc(s.st_size + 1);
 
 	fd = open(filename, O_RDONLY);
diff --git a/fs/tftp.c b/fs/tftp.c
index b58d6fc..13e117a 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -590,7 +590,10 @@ static int tftp_stat(struct device_d *dev, const char *filename, struct stat *s)
 		return PTR_ERR(priv);
 
 	s->st_mode = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO;
-	s->st_size = priv->filesize;
+	if (priv->filesize)
+		s->st_size = priv->filesize;
+	else
+		s->st_size = FILESIZE_MAX;
 
 	tftp_do_close(priv);
 
diff --git a/include/fs.h b/include/fs.h
index 3d5714c..b6a46bb 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
 int protect_file(const char *file, int prot);
 void *memmap(int fd, int flags);
 
+#define FILESIZE_MAX	((size_t)-1)
+
 #define PROT_READ	1
 #define PROT_WRITE	2
 
-- 
1.7.10.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox