On 07/18/2016 09:53 PM, Tien Fong Chee wrote:
On Fri, 2016-07-15 at 01:37 +0200, Benoît Thébaudeau wrote:
Dear Tien Fong,

On Thu, Jul 14, 2016 at 12:48 PM, Tien Fong Chee <tfc...@altera.com>
wrote:
Dear Benoît,

On Wed, 2016-07-13 at 12:56 +0200, Benoît Thébaudeau wrote:
Dear Tien Fong Chee,

On Jul 13, 2016 at 11:01 AM, Tien Fong Chee wrote:
Single 64KB get_contents_vfatname_block global variable would
be
used for
all FAT implementation instead of allocating additional two
global
variables
which are get_denfromdir_block and do_fat_read_at_block. This
implementation
can help in saving up 128KB memory space.

Signed-off-by: Tien Fong Chee <tfc...@altera.com>
Cc: Dinh Nguyen <dingu...@opensource.altera.com>
Cc: Dinh Nguyen <dinh.li...@gmail.com>
Cc: ChinLiang <cl...@altera.com>
Cc: Vagrant Cascadian <vagr...@debian.org>
Cc: Simon Glass <s...@chromium.org>
Cc: Stephen Warren <swar...@nvidia.com>
Cc: Benoît Thébaudeau <ben...@wsystem.com>
---
  fs/fat/fat.c |    6 ++----
  1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 826bd85..5d1afe6 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -579,8 +579,7 @@ static __u8 mkcksum(const char name[8],
const
char ext[3])
   * Get the directory entry associated with 'filename' from the
directory
   * starting at 'startsect'
   */
-__u8 get_dentfromdir_block[MAX_CLUSTSIZE]
-   __aligned(ARCH_DMA_MINALIGN);
+__u8 *get_dentfromdir_block = get_contents_vfatname_block;

  static dir_entry *get_dentfromdir(fsdata *mydata, int
startsect,
                               char *filename, dir_entry
*retdent,
@@ -811,8 +810,7 @@ exit:
     return ret;
  }

-__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
-   __aligned(ARCH_DMA_MINALIGN);
+__u8 *do_fat_read_at_block = get_contents_vfatname_block;

  int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
                loff_t maxsize, int dols, int dogetsize, loff_t
*size)

This probably breaks at least fat_write.c, which uses:
   memcpy(get_dentfromdir_block, get_contents_vfatname_block,

With this patch, this line code "memcpy(get_dentfromdir_block,
get_contents_vfatname_block," is not required anymore since both
  get_dentfromdir_block and get_contents_vfatname_block are sharing
the
same content and memory. So, this line code can be removed or
leaving
in there. However, there is only one place within
fill_dir_slot_buffer
function where it can corrupt the the global memory, and it is
fixed by
replacing with local buffer. This was sent out with another patch
called "[PATCH 1/2] fs/fat/fatwrite: Local variable as buffer to
store
dir_slot entries". Overall, applying these two patches, a lot
memory
space can be saved and fitting into small OCRAM, but need to be
very
careful when modifying the code related to global memory.

I get the point, but I am a bit concerned because these changes make
the code even more fragile and hard to maintain than it currently is.
Yeah, i agree with you, and there is trade-off in saving the memory
space.
Perhaps it would be time to switch to FatFs as previously suggested.
Here is its memory usage:
http://elm-chan.org/fsw/ff/en/appnote.html#memory

Cool. If i am not mistaken, this implementation would impact a lot of
areas, especially interface level. What's about the testing? I am still
voting for this simple patch changes, until we have enough resources to
do the switching.

I think switching to the FF library is a non-starter. It's excruciatingly slow since it always reads even contiguous files one cluster at a time. I did propose a change to the library, but the maintainer didn't seem interested in fixing the problem. If we were to switch, the Tianocore implementation might be worth looking at, now they've fixed the license of the FAT code to remove the requirement that it only be used in EFI implementations. I haven't looked the code to know whether it'd be possible/good to switch though.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to