| Message ID | 1333953903-26335-1-git-send-email-marex@denx.de |
|---|---|
| State | Rejected |
| Headers | show |
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");
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
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
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
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
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
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.
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
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
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
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");
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(-)