[U-Boot] [PATCH 5/7] FAT: get_cluster: Add buffer bouncing

[U-Boot] [PATCH 5/7] FAT: get_cluster: Add buffer bouncing

[Benoît Thébaudeau]

Add a buffer bouncing mechanism to get_cluster. This can be useful for
misaligned applicative buffers passed through get_contents. This is required for
the following patches in the case of data aligned differently relatively to
buffers and clusters.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
Cc: Wolfgang Denk <wd@denx.de>
---
 .../fs/fat/fat.c                                   |   42 ++++++++++++++------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git u-boot-66714b1.orig/fs/fat/fat.c u-boot-66714b1/fs/fat/fat.c
index 943cdb6..4c80820 100644
--- u-boot-66714b1.orig/fs/fat/fat.c
+++ u-boot-66714b1/fs/fat/fat.c
@@ -274,7 +274,6 @@ get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
 {
 	__u32 idx = 0;
 	__u32 startsect;
-	__u32 nr_sect;
 	int ret;
 
 	if (clustnum > 0) {
@@ -286,25 +285,44 @@ get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
 
 	debug("gc - clustnum: %d, startsect: %d\n", clustnum, startsect);
 
-	nr_sect = size / mydata->sect_size;
-	ret = disk_read(startsect, nr_sect, buffer);
-	if (ret != nr_sect) {
-		debug("Error reading data (got %d)\n", ret);
-		return -1;
-	}
-	if (size % mydata->sect_size) {
+	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
 
+		debug("FAT: Misaligned buffer address (%p)\n", buffer);
+
+		while (size >= mydata->sect_size) {
+			ret = disk_read(startsect++, 1, tmpbuf);
+			if (ret != 1) {
+				debug("Error reading data (got %d)\n", ret);
+				return -1;
+			}
+
+			memcpy(buffer, tmpbuf, mydata->sect_size);
+			buffer += mydata->sect_size;
+			size -= mydata->sect_size;
+		}
+	} else {
 		idx = size / mydata->sect_size;
-		ret = disk_read(startsect + idx, 1, tmpbuf);
+		ret = disk_read(startsect, idx, buffer);
+		if (ret != idx) {
+			debug("Error reading data (got %d)\n", ret);
+			return -1;
+		}
+		startsect += idx;
+		idx *= mydata->sect_size;
+		buffer += idx;
+		size -= idx;
+	}
+	if (size) {
+		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
+
+		ret = disk_read(startsect, 1, tmpbuf);
 		if (ret != 1) {
 			debug("Error reading data (got %d)\n", ret);
 			return -1;
 		}
-		buffer += idx * mydata->sect_size;
 
-		memcpy(buffer, tmpbuf, size % mydata->sect_size);
-		return 0;
+		memcpy(buffer, tmpbuf, size);
 	}
 
 	return 0;

[U-Boot] [PATCH 5/7] FAT: get_cluster: Add buffer bouncing

[Mike Frysinger]

On Thursday 19 July 2012 18:02:37 Beno?t Th?baudeau wrote:
> +	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
>  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
> 
> +		debug("FAT: Misaligned buffer address (%p)\n", buffer);

i'd suggest making this a printf.  bounce buffers are not cheap.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/15ecb8f4/attachment.pgp>

[U-Boot] [PATCH 5/7] FAT: get_cluster: Add buffer bouncing

[Benoît Thébaudeau]

On Friday 20 July 2012 00:48:26 Mike Frysinger wrote:
> On Thursday 19 July 2012 18:02:37 Beno?t Th?baudeau wrote:
> > +	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
> >  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
> > 
> > +		debug("FAT: Misaligned buffer address (%p)\n", buffer);
> 
> i'd suggest making this a printf.

OK. I chose debug to do like mmc.c. I'll switch to printf if you prefer.
However, debug is better here IMHO to avoid visual pollution on stdout because
this bouncing can still occur in normal conditions with perfectly aligned
buffers, in the case data is read from an unaligned position relative to the
file to an aligned offset relative to the buffer. This can be avoided by
improving the file format by aligning its data structures, but one can not
always change a file format. debug would allow to detect misaligned buffers or
to optimize file formats while not being bothering for the compelled file
formats containing misaligned data structures.

> bounce buffers are not cheap.

Indeed.

Anything else before I post a v2 of the series?

Regards,
Beno?t

[U-Boot] [PATCH v2 6/8] FAT: get_cluster: Add buffer bouncing

[Benoît Thébaudeau]

Add a buffer bouncing mechanism to get_cluster. This can be useful for
misaligned applicative buffers passed through get_contents. This is required for
the following patches in the case of data aligned differently relatively to
buffers and clusters.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes for v2:
 - Patch renumbering because of the new v2 1/8.
 - Possible code style changes due to the new v2 1/8.
 - Use printf instead of debug for the misalignment message.

 .../fs/fat/fat.c                                   |   42 ++++++++++++++------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git u-boot-66714b1.orig/fs/fat/fat.c u-boot-66714b1/fs/fat/fat.c
index 5d08948..101eb3a 100644
--- u-boot-66714b1.orig/fs/fat/fat.c
+++ u-boot-66714b1/fs/fat/fat.c
@@ -273,7 +273,6 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 {
 	__u32 idx = 0;
 	__u32 startsect;
-	__u32 nr_sect;
 	int ret;
 
 	if (clustnum > 0) {
@@ -285,25 +284,44 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 
 	debug("gc - clustnum: %d, startsect: %d\n", clustnum, startsect);
 
-	nr_sect = size / mydata->sect_size;
-	ret = disk_read(startsect, nr_sect, buffer);
-	if (ret != nr_sect) {
-		debug("Error reading data (got %d)\n", ret);
-		return -1;
-	}
-	if (size % mydata->sect_size) {
+	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
 
+		printf("FAT: Misaligned buffer address (%p)\n", buffer);
+
+		while (size >= mydata->sect_size) {
+			ret = disk_read(startsect++, 1, tmpbuf);
+			if (ret != 1) {
+				debug("Error reading data (got %d)\n", ret);
+				return -1;
+			}
+
+			memcpy(buffer, tmpbuf, mydata->sect_size);
+			buffer += mydata->sect_size;
+			size -= mydata->sect_size;
+		}
+	} else {
 		idx = size / mydata->sect_size;
-		ret = disk_read(startsect + idx, 1, tmpbuf);
+		ret = disk_read(startsect, idx, buffer);
+		if (ret != idx) {
+			debug("Error reading data (got %d)\n", ret);
+			return -1;
+		}
+		startsect += idx;
+		idx *= mydata->sect_size;
+		buffer += idx;
+		size -= idx;
+	}
+	if (size) {
+		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
+
+		ret = disk_read(startsect, 1, tmpbuf);
 		if (ret != 1) {
 			debug("Error reading data (got %d)\n", ret);
 			return -1;
 		}
-		buffer += idx * mydata->sect_size;
 
-		memcpy(buffer, tmpbuf, size % mydata->sect_size);
-		return 0;
+		memcpy(buffer, tmpbuf, size);
 	}
 
 	return 0;

[U-Boot] [PATCH v2 6/8] FAT: get_cluster: Add buffer bouncing

[Wolfgang Denk]

Dear Beno?t Th?baudeau,

In message <1423162220.332690.1342790468790.JavaMail.root@advansee.com> you wrote:
> Add a buffer bouncing mechanism to get_cluster. This can be useful for
> misaligned applicative buffers passed through get_contents. This is require=
> d for
> the following patches in the case of data aligned differently relatively to
> buffers and clusters.
> 
> Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit.thebaudeau@advansee.com>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes for v2:
>  - Patch renumbering because of the new v2 1/8.
>  - Possible code style changes due to the new v2 1/8.
>  - Use printf instead of debug for the misalignment message.
> 
>  .../fs/fat/fat.c                                   |   42 ++++++++++++++--=
> ----
>  1 file changed, 30 insertions(+), 12 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Life is a process, not a principle, a mystery  to  be  lived,  not  a
problem to be solved. - Gerard Straub, television producer and author
(stolen from Frank Herbert??)