* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
@ 2012-04-09 6:45 Marek Vasut
2012-04-09 8:20 ` Dirk Behme
2012-04-10 4:36 ` Mike Frysinger
0 siblings, 2 replies; 12+ messages in thread
From: Marek Vasut @ 2012-04-09 6:45 UTC (permalink / raw)
To: u-boot
Align the FAT FS buffers so DMA on various systems can directly pick them.
Signed-off-by: Marek Vasut <marex@denx.de>
---
fs/fat/fat.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 1f95eb4..d709e59 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -31,6 +31,7 @@
#include <fat.h>
#include <asm/byteorder.h>
#include <part.h>
+#include <malloc.h>
/*
* Convert a string to lowercase.
@@ -62,7 +63,7 @@ static int disk_read(__u32 block, __u32 nr_blocks, void *buf)
int fat_register_device (block_dev_desc_t * dev_desc, int part_no)
{
- unsigned char buffer[dev_desc->blksz];
+ ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
/* First close any currently found FAT filesystem */
cur_dev = NULL;
@@ -293,9 +294,10 @@ get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
return -1;
}
if (size % mydata->sect_size) {
- __u8 tmpbuf[mydata->sect_size];
+ ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, mydata->sect_size);
idx = size / mydata->sect_size;
+
ret = disk_read(startsect + idx, 1, tmpbuf);
if (ret != 1) {
debug("Error reading data (got %d)\n", ret);
@@ -709,7 +711,7 @@ read_bootsectandvi (boot_sector *bs, volume_info *volinfo, int *fatsize)
return -1;
}
- block = malloc(cur_dev->blksz);
+ block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz);
if (block == NULL) {
debug("Error: allocating block\n");
return -1;
@@ -765,9 +767,6 @@ exit:
return ret;
}
-__attribute__ ((__aligned__ (__alignof__ (dir_entry))))
-__u8 do_fat_read_block[MAX_CLUSTSIZE];
-
long
do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
int dols)
@@ -788,6 +787,7 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
__u32 root_cluster = 0;
int rootdir_size = 0;
int j;
+ uint8_t do_fat_read_block[MAX_CLUSTSIZE] __attribute__((aligned(32)));
if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
debug("Error: reading boot sector\n");
--
1.7.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-09 6:45 [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations Marek Vasut
@ 2012-04-09 8:20 ` Dirk Behme
2012-04-09 8:26 ` Marek Vasut
2012-04-10 4:36 ` Mike Frysinger
1 sibling, 1 reply; 12+ messages in thread
From: Dirk Behme @ 2012-04-09 8:20 UTC (permalink / raw)
To: u-boot
On 09.04.2012 08:45, Marek Vasut wrote:
> Align the FAT FS buffers so DMA on various systems can directly pick them.
Just fyi:
http://lists.denx.de/pipermail/u-boot/2012-March/119311.html
http://lists.denx.de/pipermail/u-boot/2012-March/119309.html
Best regards
Dirk
> Signed-off-by: Marek Vasut<marex@denx.de>
> ---
> fs/fat/fat.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 1f95eb4..d709e59 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -31,6 +31,7 @@
> #include<fat.h>
> #include<asm/byteorder.h>
> #include<part.h>
> +#include<malloc.h>
>
> /*
> * Convert a string to lowercase.
> @@ -62,7 +63,7 @@ static int disk_read(__u32 block, __u32 nr_blocks, void *buf)
>
> int fat_register_device (block_dev_desc_t * dev_desc, int part_no)
> {
> - unsigned char buffer[dev_desc->blksz];
> + ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
>
> /* First close any currently found FAT filesystem */
> cur_dev = NULL;
> @@ -293,9 +294,10 @@ get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
> return -1;
> }
> if (size % mydata->sect_size) {
> - __u8 tmpbuf[mydata->sect_size];
> + ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, mydata->sect_size);
>
> idx = size / mydata->sect_size;
> +
> ret = disk_read(startsect + idx, 1, tmpbuf);
> if (ret != 1) {
> debug("Error reading data (got %d)\n", ret);
> @@ -709,7 +711,7 @@ read_bootsectandvi (boot_sector *bs, volume_info *volinfo, int *fatsize)
> return -1;
> }
>
> - block = malloc(cur_dev->blksz);
> + block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz);
> if (block == NULL) {
> debug("Error: allocating block\n");
> return -1;
> @@ -765,9 +767,6 @@ exit:
> return ret;
> }
>
> -__attribute__ ((__aligned__ (__alignof__ (dir_entry))))
> -__u8 do_fat_read_block[MAX_CLUSTSIZE];
> -
> long
> do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
> int dols)
> @@ -788,6 +787,7 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
> __u32 root_cluster = 0;
> int rootdir_size = 0;
> int j;
> + uint8_t do_fat_read_block[MAX_CLUSTSIZE] __attribute__((aligned(32)));
>
> if (read_bootsectandvi(&bs,&volinfo,&mydata->fatsize)) {
> debug("Error: reading boot sector\n");
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-09 8:20 ` Dirk Behme
@ 2012-04-09 8:26 ` Marek Vasut
2012-04-09 10:28 ` Dirk Behme
2012-04-09 13:35 ` Eric Nelson
0 siblings, 2 replies; 12+ messages in thread
From: Marek Vasut @ 2012-04-09 8:26 UTC (permalink / raw)
To: u-boot
Dear Dirk Behme,
> On 09.04.2012 08:45, Marek Vasut wrote:
> > Align the FAT FS buffers so DMA on various systems can directly pick
> > them.
>
> Just fyi:
>
> http://lists.denx.de/pipermail/u-boot/2012-March/119311.html
>
> http://lists.denx.de/pipermail/u-boot/2012-March/119309.html
Heh, nice! :-)
I've been so dug up in debugging the USB cache issues I didn't bother to look
around the list for previous efforts. So obviously, apply Eric's patch! :-)
> Best regards
>
> Dirk
>
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-09 8:26 ` Marek Vasut
@ 2012-04-09 10:28 ` Dirk Behme
2012-04-09 10:44 ` Marek Vasut
2012-04-09 13:35 ` Eric Nelson
1 sibling, 1 reply; 12+ messages in thread
From: Dirk Behme @ 2012-04-09 10:28 UTC (permalink / raw)
To: u-boot
On 09.04.2012 10:26, Marek Vasut wrote:
> Dear Dirk Behme,
>
>> On 09.04.2012 08:45, Marek Vasut wrote:
>>> Align the FAT FS buffers so DMA on various systems can directly pick
>>> them.
>>
>> Just fyi:
>>
>> http://lists.denx.de/pipermail/u-boot/2012-March/119311.html
>>
>> http://lists.denx.de/pipermail/u-boot/2012-March/119309.html
>
> Heh, nice! :-)
>
> I've been so dug up in debugging the USB cache issues I didn't bother to look
> around the list for previous efforts. So obviously, apply Eric's patch! :-)
Do you need this for 2012.04? Or is the stuff which needs this
scheduled for -next?
Best regards
Dirk
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-09 10:28 ` Dirk Behme
@ 2012-04-09 10:44 ` Marek Vasut
0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2012-04-09 10:44 UTC (permalink / raw)
To: u-boot
Dear Dirk Behme,
> On 09.04.2012 10:26, Marek Vasut wrote:
> > Dear Dirk Behme,
> >
> >> On 09.04.2012 08:45, Marek Vasut wrote:
> >>> Align the FAT FS buffers so DMA on various systems can directly pick
> >>> them.
> >>
> >> Just fyi:
> >>
> >> http://lists.denx.de/pipermail/u-boot/2012-March/119311.html
> >>
> >> http://lists.denx.de/pipermail/u-boot/2012-March/119309.html
> >
> > Heh, nice! :-)
> >
> > I've been so dug up in debugging the USB cache issues I didn't bother to
> > look around the list for previous efforts. So obviously, apply Eric's
> > patch! :-)
>
> Do you need this for 2012.04? Or is the stuff which needs this
> scheduled for -next?
Definitelly -next, we're on -RC1 now, close to RC2.
>
> Best regards
>
> Dirk
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-09 8:26 ` Marek Vasut
2012-04-09 10:28 ` Dirk Behme
@ 2012-04-09 13:35 ` Eric Nelson
1 sibling, 0 replies; 12+ messages in thread
From: Eric Nelson @ 2012-04-09 13:35 UTC (permalink / raw)
To: u-boot
On 04/09/2012 01:26 AM, Marek Vasut wrote:
> Dear Dirk Behme,
>
>> On 09.04.2012 08:45, Marek Vasut wrote:
>>> Align the FAT FS buffers so DMA on various systems can directly pick
>>> them.
>>
>> Just fyi:
>>
>> http://lists.denx.de/pipermail/u-boot/2012-March/119311.html
>>
>> http://lists.denx.de/pipermail/u-boot/2012-March/119309.html
>
> Heh, nice! :-)
>
> I've been so dug up in debugging the USB cache issues I didn't bother to look
> around the list for previous efforts. So obviously, apply Eric's patch! :-)
>
Hi guys,
It looks like I missed this bit though:
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 1f95eb4..d709e59 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -709,7 +711,7 @@ read_bootsectandvi (boot_sector *bs, volume_info *volinfo,
int *fatsize)
return -1;
}
- block = malloc(cur_dev->blksz);
+ block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz);
if (block == NULL) {
debug("Error: allocating block\n");
return -1;
Perhaps I got lucky in my testing!
Regards,
Eric
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-09 6:45 [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations Marek Vasut
2012-04-09 8:20 ` Dirk Behme
@ 2012-04-10 4:36 ` Mike Frysinger
2012-04-10 5:00 ` Marek Vasut
1 sibling, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2012-04-10 4:36 UTC (permalink / raw)
To: u-boot
On Monday 09 April 2012 02:45:03 Marek Vasut wrote:
> @@ -765,9 +767,6 @@
>
> -__attribute__ ((__aligned__ (__alignof__ (dir_entry))))
> -__u8 do_fat_read_block[MAX_CLUSTSIZE];
> -
>
> @@ -788,6 +787,7 @@ do_fat_read (const char *filename, void *buffer,
>
> int rootdir_size = 0;
> int j;
> + uint8_t do_fat_read_block[MAX_CLUSTSIZE] __attribute__((aligned(32)));
what's going on here exactly ? the old code had the advantage of being in the
bss and the linker taking care of the alignment. this new code has an
incorrectly hard-coded "32", and puts a 64KiB array onto the *stack*.
-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/20120410/dc01d27d/attachment.pgp>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-10 4:36 ` Mike Frysinger
@ 2012-04-10 5:00 ` Marek Vasut
2012-04-10 15:07 ` Eric Nelson
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2012-04-10 5:00 UTC (permalink / raw)
To: u-boot
Dear Mike Frysinger,
> On Monday 09 April 2012 02:45:03 Marek Vasut wrote:
> > @@ -765,9 +767,6 @@
> >
> > -__attribute__ ((__aligned__ (__alignof__ (dir_entry))))
> > -__u8 do_fat_read_block[MAX_CLUSTSIZE];
> > -
> >
> > @@ -788,6 +787,7 @@ do_fat_read (const char *filename, void *buffer,
> >
> > int rootdir_size = 0;
> > int j;
> >
> > + uint8_t do_fat_read_block[MAX_CLUSTSIZE] __attribute__((aligned(32)));
>
> what's going on here exactly ? the old code had the advantage of being in
> the bss and the linker taking care of the alignment. this new code has an
> incorrectly hard-coded "32", and puts a 64KiB array onto the *stack*.
This will be probably fixed in Eric's patch
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-10 5:00 ` Marek Vasut
@ 2012-04-10 15:07 ` Eric Nelson
2012-04-10 16:20 ` Dirk Behme
0 siblings, 1 reply; 12+ messages in thread
From: Eric Nelson @ 2012-04-10 15:07 UTC (permalink / raw)
To: u-boot
On 04/09/2012 10:00 PM, Marek Vasut wrote:
> Dear Mike Frysinger,
>
>> On Monday 09 April 2012 02:45:03 Marek Vasut wrote:
>>> @@ -765,9 +767,6 @@
>>>
>>> -__attribute__ ((__aligned__ (__alignof__ (dir_entry))))
>>> -__u8 do_fat_read_block[MAX_CLUSTSIZE];
>>> -
>>>
>>> @@ -788,6 +787,7 @@ do_fat_read (const char *filename, void *buffer,
>>>
>>> int rootdir_size = 0;
>>> int j;
>>>
>>> + uint8_t do_fat_read_block[MAX_CLUSTSIZE] __attribute__((aligned(32)));
>>
>> what's going on here exactly ? the old code had the advantage of being in
>> the bss and the linker taking care of the alignment. this new code has an
>> incorrectly hard-coded "32", and puts a 64KiB array onto the *stack*.
>
> This will be probably fixed in Eric's patch
>
Yep. I left it in bss space.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-10 15:07 ` Eric Nelson
@ 2012-04-10 16:20 ` Dirk Behme
2012-04-10 20:34 ` Wolfgang Denk
0 siblings, 1 reply; 12+ messages in thread
From: Dirk Behme @ 2012-04-10 16:20 UTC (permalink / raw)
To: u-boot
On 10.04.2012 17:07, Eric Nelson wrote:
> On 04/09/2012 10:00 PM, Marek Vasut wrote:
>> Dear Mike Frysinger,
>>
>>> On Monday 09 April 2012 02:45:03 Marek Vasut wrote:
>>>> @@ -765,9 +767,6 @@
>>>>
>>>> -__attribute__ ((__aligned__ (__alignof__ (dir_entry))))
>>>> -__u8 do_fat_read_block[MAX_CLUSTSIZE];
>>>> -
>>>>
>>>> @@ -788,6 +787,7 @@ do_fat_read (const char *filename, void *buffer,
>>>>
>>>> int rootdir_size = 0;
>>>> int j;
>>>>
>>>> + uint8_t do_fat_read_block[MAX_CLUSTSIZE]
>>>> __attribute__((aligned(32)));
>>>
>>> what's going on here exactly ? the old code had the advantage of
>>> being in
>>> the bss and the linker taking care of the alignment. this new code
>>> has an
>>> incorrectly hard-coded "32", and puts a 64KiB array onto the *stack*.
>>
>> This will be probably fixed in Eric's patch
>>
> Yep. I left it in bss space.
Sorry guys, but I'm confused now.
We have two patches, Eric's [1] and Marek's [2]. Which one should we
take? With the discussion here and [3] I'm somehow under the
impression that both patches [1] [2] are not complete?
Do I miss anything? Or do we need a new version ("best of [1] & [2]")?
Best regards
Dirk
[1] http://lists.denx.de/pipermail/u-boot/2012-March/119311.html
[2] http://lists.denx.de/pipermail/u-boot/2012-April/122100.html
[3] http://lists.denx.de/pipermail/u-boot/2012-April/122126.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-10 16:20 ` Dirk Behme
@ 2012-04-10 20:34 ` Wolfgang Denk
2012-04-10 23:13 ` Eric Nelson
0 siblings, 1 reply; 12+ messages in thread
From: Wolfgang Denk @ 2012-04-10 20:34 UTC (permalink / raw)
To: u-boot
Dear Dirk,
In message <4F845DB6.7050002@googlemail.com> you wrote:
>
> Sorry guys, but I'm confused now.
Heh. Me too ;-)
> We have two patches, Eric's [1] and Marek's [2]. Which one should we
> take? With the discussion here and [3] I'm somehow under the
> impression that both patches [1] [2] are not complete?
>
> Do I miss anything? Or do we need a new version ("best of [1] & [2]")?
I'm waiting for Eric to post an updated patch that includes 1+2+3.
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
Any sufficiently advanced technology is indistinguishable from magic.
Clarke's Third Law - _Profiles of the Future_ (1962; rev. 1973)
``Hazards of Prophecy: The Failure of Imagination''
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations
2012-04-10 20:34 ` Wolfgang Denk
@ 2012-04-10 23:13 ` Eric Nelson
0 siblings, 0 replies; 12+ messages in thread
From: Eric Nelson @ 2012-04-10 23:13 UTC (permalink / raw)
To: u-boot
On 04/10/2012 01:34 PM, Wolfgang Denk wrote:
> Dear Dirk,
>
> In message<4F845DB6.7050002@googlemail.com> you wrote:
>>
>> Sorry guys, but I'm confused now.
>
> Heh. Me too ;-)
>
>> We have two patches, Eric's [1] and Marek's [2]. Which one should we
>> take? With the discussion here and [3] I'm somehow under the
>> impression that both patches [1] [2] are not complete?
>>
>> Do I miss anything? Or do we need a new version ("best of [1]& [2]")?
>
> I'm waiting for Eric to post an updated patch that includes 1+2+3.
>
I guess I had better get busy ;)
I can probably handle 1 and 2 and my guess is that #3 will appear with
a fresh re-read of the code.
I should be able to get this out in the A.M. (Arizona time).
Regards,
Eric
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-04-10 23:13 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-09 6:45 [U-Boot] [PATCH] FAT: Properly align buffers to allow cache operations Marek Vasut
2012-04-09 8:20 ` Dirk Behme
2012-04-09 8:26 ` Marek Vasut
2012-04-09 10:28 ` Dirk Behme
2012-04-09 10:44 ` Marek Vasut
2012-04-09 13:35 ` Eric Nelson
2012-04-10 4:36 ` Mike Frysinger
2012-04-10 5:00 ` Marek Vasut
2012-04-10 15:07 ` Eric Nelson
2012-04-10 16:20 ` Dirk Behme
2012-04-10 20:34 ` Wolfgang Denk
2012-04-10 23:13 ` Eric Nelson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.