[U-Boot] [PATCH v3 0/9] fs/fat: cleanups + readdir implementation

Introduce directory traversal iterators, and implement fs_readdir() which is needed by EFI_LOADER.
The part re-working fat.c to use the directory iterators itself is nearly a 2:1 negative diffstat, and a pretty big cleanup. I fixed one or two other small issues along the way.
v2: fix a couple issues (partition is whole disk, and traversing ".." up to root directory) v3: mostly addition of comments plus cleaning up open-coded sector <-> cluster conversions, and some FAT32 fixes for large directories
Rob Clark (9): fs/fat: split out helper to init fsdata fs/fat: introduce new director iterators fat/fs: convert to directory iterators fs: add fs_readdir() fs/fat: implement opendir/readdir/closedir fat/fs: remove a bunch of dead code fat/fs: move ls to generic implementation fs/fat: fix case for FAT shortnames fs/fat: Clean up open-coded sector <-> cluster conversions
disk/part.c | 31 +- fs/fat/Makefile | 4 - fs/fat/fat.c | 1062 ++++++++++++++++++++++------------------------------ fs/fat/fat_write.c | 12 +- fs/fat/file.c | 183 --------- fs/fs.c | 137 ++++++- include/fat.h | 49 ++- include/fs.h | 67 ++++ include/part.h | 9 + 9 files changed, 700 insertions(+), 854 deletions(-) delete mode 100644 fs/fat/file.c

Want to re-use this in fat dirent iterator in next patch.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org --- fs/fat/fat.c | 73 +++++++++++++++++++++++++++++++++++------------------------ include/fat.h | 1 + 2 files changed, 44 insertions(+), 30 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 465a6875ed..e1c0a15dc7 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -808,35 +808,17 @@ exit: return ret; }
-__u8 do_fat_read_at_block[MAX_CLUSTSIZE] - __aligned(ARCH_DMA_MINALIGN); - -int do_fat_read_at(const char *filename, loff_t pos, void *buffer, - loff_t maxsize, int dols, int dogetsize, loff_t *size) +static int get_fs_info(fsdata *mydata) { - char fnamecopy[2048]; boot_sector bs; volume_info volinfo; - fsdata datablock; - fsdata *mydata = &datablock; - dir_entry *dentptr = NULL; - __u16 prevcksum = 0xffff; - char *subname = ""; - __u32 cursect; - int idx, isdir = 0; - int files = 0, dirs = 0; - int ret = -1; - int firsttime; __u32 root_cluster = 0; - __u32 read_blk; - int rootdir_size = 0; - int buffer_blk_cnt; - int do_read; - __u8 *dir_ptr; + int ret;
- if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) { + ret = read_bootsectandvi(&bs, &volinfo, &mydata->fatsize); + if (ret) { debug("Error: reading boot sector\n"); - return -1; + return ret; }
if (mydata->fatsize == 32) { @@ -848,8 +830,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
mydata->fat_sect = bs.reserved;
- cursect = mydata->rootdir_sect - = mydata->fat_sect + mydata->fatlength * bs.fats; + mydata->rootdir_sect = mydata->fat_sect + mydata->fatlength * bs.fats;
mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0]; mydata->clust_size = bs.cluster_size; @@ -863,12 +844,12 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, mydata->data_begin = mydata->rootdir_sect - (mydata->clust_size * 2); } else { - rootdir_size = ((bs.dir_entries[1] * (int)256 + - bs.dir_entries[0]) * - sizeof(dir_entry)) / - mydata->sect_size; + mydata->rootdir_size = ((bs.dir_entries[1] * (int)256 + + bs.dir_entries[0]) * + sizeof(dir_entry)) / + mydata->sect_size; mydata->data_begin = mydata->rootdir_sect + - rootdir_size - + mydata->rootdir_size - (mydata->clust_size * 2); }
@@ -893,6 +874,38 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, debug("Sector size: %d, cluster size: %d\n", mydata->sect_size, mydata->clust_size);
+ return 0; +} + +__u8 do_fat_read_at_block[MAX_CLUSTSIZE] + __aligned(ARCH_DMA_MINALIGN); + +int do_fat_read_at(const char *filename, loff_t pos, void *buffer, + loff_t maxsize, int dols, int dogetsize, loff_t *size) +{ + char fnamecopy[2048]; + fsdata datablock; + fsdata *mydata = &datablock; + dir_entry *dentptr = NULL; + __u16 prevcksum = 0xffff; + char *subname = ""; + __u32 cursect; + int idx, isdir = 0; + int files = 0, dirs = 0; + int ret = -1; + int firsttime; + __u32 root_cluster = 0; + __u32 read_blk; + int rootdir_size = 0; + int buffer_blk_cnt; + int do_read; + __u8 *dir_ptr; + + if (get_fs_info(mydata)) + return -1; + + cursect = mydata->rootdir_sect; + /* "cwd" is always the root... */ while (ISDIRDELIM(*filename)) filename++; diff --git a/include/fat.h b/include/fat.h index 71879f01ca..b671ee8f81 100644 --- a/include/fat.h +++ b/include/fat.h @@ -174,6 +174,7 @@ typedef struct { __u16 clust_size; /* Size of clusters in sectors */ int data_begin; /* The sector of the first cluster, can be negative */ int fatbufnum; /* Used by get_fatent, init to -1 */ + int rootdir_size; /* Size of root dir for non-FAT32 */ } fsdata;
typedef int (file_detectfs_func)(void);

On Sat, Sep 09, 2017 at 01:15:52PM -0400, Rob Clark wrote:
Want to re-use this in fat dirent iterator in next patch.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Untangle directory traversal into a simple iterator, to replace the existing multi-purpose do_fat_read_at() + get_dentfromdir().
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de --- fs/fat/fat.c | 356 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- include/fat.h | 7 ++ 2 files changed, 360 insertions(+), 3 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index e1c0a15dc7..ee2bbe38f1 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -812,7 +812,6 @@ static int get_fs_info(fsdata *mydata) { boot_sector bs; volume_info volinfo; - __u32 root_cluster = 0; int ret;
ret = read_bootsectandvi(&bs, &volinfo, &mydata->fatsize); @@ -822,7 +821,6 @@ static int get_fs_info(fsdata *mydata) }
if (mydata->fatsize == 32) { - root_cluster = bs.root_cluster; mydata->fatlength = bs.fat32_length; } else { mydata->fatlength = bs.fat_length; @@ -843,6 +841,7 @@ static int get_fs_info(fsdata *mydata) if (mydata->fatsize == 32) { mydata->data_begin = mydata->rootdir_sect - (mydata->clust_size * 2); + mydata->root_cluster = bs.root_cluster; } else { mydata->rootdir_size = ((bs.dir_entries[1] * (int)256 + bs.dir_entries[0]) * @@ -851,6 +850,9 @@ static int get_fs_info(fsdata *mydata) mydata->data_begin = mydata->rootdir_sect + mydata->rootdir_size - (mydata->clust_size * 2); + mydata->root_cluster = (mydata->rootdir_sect - + mydata->data_begin) / + mydata->clust_size; }
mydata->fatbufnum = -1; @@ -868,7 +870,7 @@ static int get_fs_info(fsdata *mydata) mydata->fatsize, mydata->fat_sect, mydata->fatlength); debug("Rootdir begins at cluster: %d, sector: %d, offset: %x\n" "Data begins at: %d\n", - root_cluster, + mydata->root_cluster, mydata->rootdir_sect, mydata->rootdir_sect * mydata->sect_size, mydata->data_begin); debug("Sector size: %d, cluster size: %d\n", mydata->sect_size, @@ -1245,6 +1247,354 @@ exit: return ret; }
+ +/* + * Directory iterator, to simplify filesystem traversal + * + * Implements an iterator pattern to traverse directory tables, + * transparently handling directory tables split across multiple + * clusters, and the difference between FAT12/FAT16 root directory + * (contiguous) and subdirectories + FAT32 root (chained). + * + * Rough usage: + * + * for (fat_itr_root(&itr, fsdata); fat_itr_next(&itr); ) { + * // to traverse down to a subdirectory pointed to by + * // current iterator position: + * fat_itr_child(&itr, &itr); + * } + * + * For more complete example, see fat_itr_resolve() + */ + +typedef struct { + fsdata *fsdata; /* filesystem parameters */ + unsigned clust; /* current cluster */ + int last_cluster; /* set once we've read last cluster */ + int is_root; /* is iterator at root directory */ + int remaining; /* remaining dent's in current cluster */ + + /* current iterator position values: */ + dir_entry *dent; /* current directory entry */ + char l_name[VFAT_MAXLEN_BYTES]; /* long (vfat) name */ + char s_name[14]; /* short 8.3 name */ + char *name; /* l_name if there is one, else s_name */ + + /* storage for current cluster in memory: */ + u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); +} fat_itr; + +static int fat_itr_isdir(fat_itr *itr); + +/** + * fat_itr_root() - initialize an iterator to start at the root + * directory + * + * @itr: iterator to initialize + * @fsdata: filesystem data for the partition + * @return 0 on success, else -errno + */ +static int fat_itr_root(fat_itr *itr, fsdata *fsdata) +{ + if (get_fs_info(fsdata)) + return -ENXIO; + + itr->fsdata = fsdata; + itr->clust = fsdata->root_cluster; + itr->dent = NULL; + itr->remaining = 0; + itr->last_cluster = 0; + itr->is_root = 1; + + return 0; +} + +/** + * fat_itr_child() - initialize an iterator to descend into a sub- + * directory + * + * Initializes 'itr' to iterate the contents of the directory at + * the current cursor position of 'parent'. It is an error to + * call this if the current cursor of 'parent' is pointing at a + * regular file. + * + * Note that 'itr' and 'parent' can be the same pointer if you do + * not need to preserve 'parent' after this call, which is useful + * for traversing directory structure to resolve a file/directory. + * + * @itr: iterator to initialize + * @parent: the iterator pointing at a directory entry in the + * parent directory of the directory to iterate + */ +static void fat_itr_child(fat_itr *itr, fat_itr *parent) +{ + fsdata *mydata = parent->fsdata; /* for silly macros */ + unsigned clustnum = START(parent->dent); + + assert(fat_itr_isdir(parent)); + + itr->fsdata = parent->fsdata; + if (clustnum > 0) { + itr->clust = clustnum; + } else { + itr->clust = parent->fsdata->root_cluster; + } + itr->dent = NULL; + itr->remaining = 0; + itr->last_cluster = 0; + itr->is_root = 0; +} + +static void *next_cluster(fat_itr *itr) +{ + fsdata *mydata = itr->fsdata; /* for silly macros */ + int ret; + u32 sect; + + /* have we reached the end? */ + if (itr->last_cluster) + return NULL; + + sect = clust_to_sect(itr->fsdata, itr->clust); + + debug("FAT read(sect=%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n", + sect, itr->fsdata->clust_size, DIRENTSPERBLOCK); + + /* + * NOTE: do_fat_read_at() had complicated logic to deal w/ + * vfat names that span multiple clusters in the fat16 case, + * which get_dentfromdir() probably also needed (and was + * missing). And not entirely sure what fat32 didn't have + * the same issue.. We solve that by only caring about one + * dent at a time and iteratively constructing the vfat long + * name. + */ + ret = disk_read(sect, itr->fsdata->clust_size, + itr->block); + if (ret < 0) { + debug("Error: reading block\n"); + return NULL; + } + + if (itr->is_root && itr->fsdata->fatsize != 32) { + itr->clust++; + sect = clust_to_sect(itr->fsdata, itr->clust); + if (sect - itr->fsdata->rootdir_sect >= + itr->fsdata->rootdir_size) { + debug("cursect: 0x%x\n", itr->clust); + itr->last_cluster = 1; + } + } else { + itr->clust = get_fatent(itr->fsdata, itr->clust); + if (CHECK_CLUST(itr->clust, itr->fsdata->fatsize)) { + debug("cursect: 0x%x\n", itr->clust); + itr->last_cluster = 1; + } + } + + return itr->block; +} + +static dir_entry *next_dent(fat_itr *itr) +{ + if (itr->remaining == 0) { + struct dir_entry *dent = next_cluster(itr); + unsigned nbytes = itr->fsdata->sect_size * + itr->fsdata->clust_size; + + /* have we reached the last cluster? */ + if (!dent) + return NULL; + + itr->remaining = nbytes / sizeof(dir_entry) - 1; + itr->dent = dent; + } else { + itr->remaining--; + itr->dent++; + } + + /* have we reached the last valid entry? */ + if (itr->dent->name[0] == 0) + return NULL; + + return itr->dent; +} + +static dir_entry *extract_vfat_name(fat_itr *itr) +{ + struct dir_entry *dent = itr->dent; + int seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK; + u8 chksum, alias_checksum = ((dir_slot *)dent)->alias_checksum; + int n = 0; + + while (seqn--) { + char buf[13]; + int idx = 0; + + slot2str((dir_slot *)dent, buf, &idx); + + /* shift accumulated long-name up and copy new part in: */ + memmove(itr->l_name + idx, itr->l_name, n); + memcpy(itr->l_name, buf, idx); + n += idx; + + dent = next_dent(itr); + if (!dent) + return NULL; + } + + itr->l_name[n] = '\0'; + + chksum = mkcksum(dent->name, dent->ext); + + /* checksum mismatch could mean deleted file, etc.. skip it: */ + if (chksum != alias_checksum) { + debug("** chksum=%x, alias_checksum=%x, l_name=%s, s_name=%8s.%3s\n", + chksum, alias_checksum, itr->l_name, dent->name, dent->ext); + return NULL; + } + + return dent; +} + +/** + * fat_itr_next() - step to the next entry in a directory + * + * Must be called once on a new iterator before the cursor is valid. + * + * @itr: the iterator to iterate + * @return boolean, 1 if success or 0 if no more entries in the + * current directory + */ +static int fat_itr_next(fat_itr *itr) +{ + dir_entry *dent; + + itr->name = NULL; + + while (1) { + dent = next_dent(itr); + if (!dent) + return 0; + + if (dent->name[0] == DELETED_FLAG || + dent->name[0] == aRING) + continue; + + if (dent->attr & ATTR_VOLUME) { + if (vfat_enabled && + (dent->attr & ATTR_VFAT) == ATTR_VFAT && + (dent->name[0] & LAST_LONG_ENTRY_MASK)) { + dent = extract_vfat_name(itr); + if (!dent) + continue; + itr->name = itr->l_name; + break; + } else { + /* Volume label or VFAT entry, skip */ + continue; + } + } + + break; + } + + get_name(dent, itr->s_name); + if (!itr->name) + itr->name = itr->s_name; + + return 1; +} + +/** + * fat_itr_isdir() - is current cursor position pointing to a directory + * + * @itr: the iterator + * @return true if cursor is at a directory + */ +static int fat_itr_isdir(fat_itr *itr) +{ + return !!(itr->dent->attr & ATTR_DIR); +} + +/* + * Helpers: + */ + +#define TYPE_FILE 0x1 +#define TYPE_DIR 0x2 +#define TYPE_ANY (TYPE_FILE | TYPE_DIR) + +/** + * fat_itr_resolve() - traverse directory structure to resolve the + * requested path. + * + * Traverse directory structure to the requested path. If the specified + * path is to a directory, this will descend into the directory and + * leave it iterator at the start of the directory. If the path is to a + * file, it will leave the iterator in the parent directory with current + * cursor at file's entry in the directory. + * + * @itr: iterator initialized to root + * @path: the requested path + * @type: bitmask of allowable file types + * @return 0 on success or -errno + */ +static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type) +{ + const char *next; + + /* chomp any extra leading slashes: */ + while (path[0] && ISDIRDELIM(path[0])) + path++; + + /* are we at the end? */ + if (strlen(path) == 0) { + if (!(type & TYPE_DIR)) + return -ENOENT; + return 0; + } + + /* find length of next path entry: */ + next = path; + while (next[0] && !ISDIRDELIM(next[0])) + next++; + + while (fat_itr_next(itr)) { + int match = 0; + unsigned n = max(strlen(itr->name), (size_t)(next - path)); + + /* check both long and short name: */ + if (!strncasecmp(path, itr->name, n)) + match = 1; + else if (itr->name != itr->s_name && + !strncasecmp(path, itr->s_name, n)) + match = 1; + + if (!match) + continue; + + if (fat_itr_isdir(itr)) { + /* recurse into directory: */ + fat_itr_child(itr, itr); + return fat_itr_resolve(itr, next, type); + } else if (next[0]) { + /* + * If next is not empty then we have a case + * like: /path/to/realfile/nonsense + */ + debug("bad trailing path: %s\n", next); + return -ENOENT; + } else if (!(type & TYPE_FILE)) { + return -ENOTDIR; + } else { + return 0; + } + } + + return -ENOENT; +} + int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int dols, loff_t *actread) { diff --git a/include/fat.h b/include/fat.h index b671ee8f81..21bb6666cf 100644 --- a/include/fat.h +++ b/include/fat.h @@ -175,8 +175,15 @@ typedef struct { int data_begin; /* The sector of the first cluster, can be negative */ int fatbufnum; /* Used by get_fatent, init to -1 */ int rootdir_size; /* Size of root dir for non-FAT32 */ + __u32 root_cluster; /* First cluster of root dir for FAT32 */ } fsdata;
+/* TODO clean up places that are open-coding this: */ +static inline u32 clust_to_sect(fsdata *fsdata, u32 clust) +{ + return fsdata->data_begin + clust * fsdata->clust_size; +} + typedef int (file_detectfs_func)(void); typedef int (file_ls_func)(const char *dir); typedef int (file_read_func)(const char *filename, void *buffer,

On Sat, Sep 09, 2017 at 01:15:53PM -0400, Rob Clark wrote:
Untangle directory traversal into a simple iterator, to replace the existing multi-purpose do_fat_read_at() + get_dentfromdir().
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!

And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de --- fs/fat/fat.c | 722 +++++++--------------------------------------------------- include/fat.h | 6 - 2 files changed, 76 insertions(+), 652 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index ee2bbe38f1..bbba7947ee 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -119,22 +119,6 @@ int fat_register_device(struct blk_desc *dev_desc, int part_no) }
/* - * Get the first occurence of a directory delimiter ('/' or '') in a string. - * Return index into string if found, -1 otherwise. - */ -static int dirdelim(char *str) -{ - char *start = str; - - while (*str != '\0') { - if (ISDIRDELIM(*str)) - return str - start; - str++; - } - return -1; -} - -/* * Extract zero terminated short name from a directory entry. */ static void get_name(dir_entry *dirent, char *s_name) @@ -468,95 +452,6 @@ static int slot2str(dir_slot *slotptr, char *l_name, int *idx) return 0; }
-/* - * Extract the full long filename starting at 'retdent' (which is really - * a slot) into 'l_name'. If successful also copy the real directory entry - * into 'retdent' - * Return 0 on success, -1 otherwise. - */ -static int -get_vfatname(fsdata *mydata, int curclust, __u8 *cluster, - dir_entry *retdent, char *l_name) -{ - dir_entry *realdent; - dir_slot *slotptr = (dir_slot *)retdent; - __u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ? - PREFETCH_BLOCKS : - mydata->clust_size); - __u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff; - int idx = 0; - - if (counter > VFAT_MAXSEQ) { - debug("Error: VFAT name is too long\n"); - return -1; - } - - while ((__u8 *)slotptr < buflimit) { - if (counter == 0) - break; - if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) != counter) - return -1; - slotptr++; - counter--; - } - - if ((__u8 *)slotptr >= buflimit) { - dir_slot *slotptr2; - - if (curclust == 0) - return -1; - curclust = get_fatent(mydata, curclust); - if (CHECK_CLUST(curclust, mydata->fatsize)) { - debug("curclust: 0x%x\n", curclust); - printf("Invalid FAT entry\n"); - return -1; - } - - if (get_cluster(mydata, curclust, get_contents_vfatname_block, - mydata->clust_size * mydata->sect_size) != 0) { - debug("Error: reading directory block\n"); - return -1; - } - - slotptr2 = (dir_slot *)get_contents_vfatname_block; - while (counter > 0) { - if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK) - & 0xff) != counter) - return -1; - slotptr2++; - counter--; - } - - /* Save the real directory entry */ - realdent = (dir_entry *)slotptr2; - while ((__u8 *)slotptr2 > get_contents_vfatname_block) { - slotptr2--; - slot2str(slotptr2, l_name, &idx); - } - } else { - /* Save the real directory entry */ - realdent = (dir_entry *)slotptr; - } - - do { - slotptr--; - if (slot2str(slotptr, l_name, &idx)) - break; - } while (!(slotptr->id & LAST_LONG_ENTRY_MASK)); - - l_name[idx] = '\0'; - if (*l_name == DELETED_FLAG) - *l_name = '\0'; - else if (*l_name == aRING) - *l_name = DELETED_FLAG; - downcase(l_name); - - /* Return the real directory entry */ - memcpy(retdent, realdent, sizeof(dir_entry)); - - return 0; -} - /* Calculate short name checksum */ static __u8 mkcksum(const char name[8], const char ext[3]) { @@ -573,169 +468,13 @@ static __u8 mkcksum(const char name[8], const char ext[3]) }
/* - * Get the directory entry associated with 'filename' from the directory - * starting at 'startsect' + * TODO these should go away once fat_write is reworked to use the + * directory iterator */ __u8 get_dentfromdir_block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); - -static dir_entry *get_dentfromdir(fsdata *mydata, int startsect, - char *filename, dir_entry *retdent, - int dols) -{ - __u16 prevcksum = 0xffff; - __u32 curclust = START(retdent); - int files = 0, dirs = 0; - - debug("get_dentfromdir: %s\n", filename); - - while (1) { - dir_entry *dentptr; - - int i; - - if (get_cluster(mydata, curclust, get_dentfromdir_block, - mydata->clust_size * mydata->sect_size) != 0) { - debug("Error: reading directory block\n"); - return NULL; - } - - dentptr = (dir_entry *)get_dentfromdir_block; - - for (i = 0; i < DIRENTSPERCLUST; i++) { - char s_name[14], l_name[VFAT_MAXLEN_BYTES]; - - l_name[0] = '\0'; - if (dentptr->name[0] == DELETED_FLAG) { - dentptr++; - continue; - } - if ((dentptr->attr & ATTR_VOLUME)) { - if (vfat_enabled && - (dentptr->attr & ATTR_VFAT) == ATTR_VFAT && - (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) { - prevcksum = ((dir_slot *)dentptr)->alias_checksum; - get_vfatname(mydata, curclust, - get_dentfromdir_block, - dentptr, l_name); - if (dols) { - int isdir; - char dirc; - int doit = 0; - - isdir = (dentptr->attr & ATTR_DIR); - - if (isdir) { - dirs++; - dirc = '/'; - doit = 1; - } else { - dirc = ' '; - if (l_name[0] != 0) { - files++; - doit = 1; - } - } - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - l_name, - dirc); - } else { - printf(" %s%c\n", - l_name, - dirc); - } - } - dentptr++; - continue; - } - debug("vfatname: |%s|\n", l_name); - } else { - /* Volume label or VFAT entry */ - dentptr++; - continue; - } - } - if (dentptr->name[0] == 0) { - if (dols) { - printf("\n%d file(s), %d dir(s)\n\n", - files, dirs); - } - debug("Dentname == NULL - %d\n", i); - return NULL; - } - if (vfat_enabled) { - __u8 csum = mkcksum(dentptr->name, dentptr->ext); - if (dols && csum == prevcksum) { - prevcksum = 0xffff; - dentptr++; - continue; - } - } - - get_name(dentptr, s_name); - if (dols) { - int isdir = (dentptr->attr & ATTR_DIR); - char dirc; - int doit = 0; - - if (isdir) { - dirs++; - dirc = '/'; - doit = 1; - } else { - dirc = ' '; - if (s_name[0] != 0) { - files++; - doit = 1; - } - } - - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - s_name, dirc); - } else { - printf(" %s%c\n", - s_name, dirc); - } - } - - dentptr++; - continue; - } - - if (strcmp(filename, s_name) - && strcmp(filename, l_name)) { - debug("Mismatch: |%s|%s|\n", s_name, l_name); - dentptr++; - continue; - } - - memcpy(retdent, dentptr, sizeof(dir_entry)); - - debug("DentName: %s", s_name); - debug(", start: 0x%x", START(dentptr)); - debug(", size: 0x%x %s\n", - FAT2CPU32(dentptr->size), - (dentptr->attr & ATTR_DIR) ? "(DIR)" : ""); - - return retdent; - } - - curclust = get_fatent(mydata, curclust); - if (CHECK_CLUST(curclust, mydata->fatsize)) { - debug("curclust: 0x%x\n", curclust); - printf("Invalid FAT entry\n"); - return NULL; - } - } - - return NULL; -} +__u8 do_fat_read_at_block[MAX_CLUSTSIZE] + __aligned(ARCH_DMA_MINALIGN);
/* * Read boot sector and volume info from a FAT filesystem @@ -879,374 +618,6 @@ static int get_fs_info(fsdata *mydata) return 0; }
-__u8 do_fat_read_at_block[MAX_CLUSTSIZE] - __aligned(ARCH_DMA_MINALIGN); - -int do_fat_read_at(const char *filename, loff_t pos, void *buffer, - loff_t maxsize, int dols, int dogetsize, loff_t *size) -{ - char fnamecopy[2048]; - fsdata datablock; - fsdata *mydata = &datablock; - dir_entry *dentptr = NULL; - __u16 prevcksum = 0xffff; - char *subname = ""; - __u32 cursect; - int idx, isdir = 0; - int files = 0, dirs = 0; - int ret = -1; - int firsttime; - __u32 root_cluster = 0; - __u32 read_blk; - int rootdir_size = 0; - int buffer_blk_cnt; - int do_read; - __u8 *dir_ptr; - - if (get_fs_info(mydata)) - return -1; - - cursect = mydata->rootdir_sect; - - /* "cwd" is always the root... */ - while (ISDIRDELIM(*filename)) - filename++; - - /* Make a copy of the filename and convert it to lowercase */ - strcpy(fnamecopy, filename); - downcase(fnamecopy); - -root_reparse: - if (*fnamecopy == '\0') { - if (!dols) - goto exit; - - dols = LS_ROOT; - } else if ((idx = dirdelim(fnamecopy)) >= 0) { - isdir = 1; - fnamecopy[idx] = '\0'; - subname = fnamecopy + idx + 1; - - /* Handle multiple delimiters */ - while (ISDIRDELIM(*subname)) - subname++; - } else if (dols) { - isdir = 1; - } - - buffer_blk_cnt = 0; - firsttime = 1; - while (1) { - int i; - - if (mydata->fatsize == 32 || firsttime) { - dir_ptr = do_fat_read_at_block; - firsttime = 0; - } else { - /** - * FAT16 sector buffer modification: - * Each loop, the second buffered block is moved to - * the buffer begin, and two next sectors are read - * next to the previously moved one. So the sector - * buffer keeps always 3 sectors for fat16. - * And the current sector is the buffer second sector - * beside the "firsttime" read, when it is the first one. - * - * PREFETCH_BLOCKS is 2 for FAT16 == loop[0:1] - * n = computed root dir sector - * loop | cursect-1 | cursect | cursect+1 | - * 0 | sector n+0 | sector n+1 | none | - * 1 | none | sector n+0 | sector n+1 | - * 0 | sector n+1 | sector n+2 | sector n+3 | - * 1 | sector n+3 | ... - */ - dir_ptr = (do_fat_read_at_block + mydata->sect_size); - memcpy(do_fat_read_at_block, dir_ptr, mydata->sect_size); - } - - do_read = 1; - - if (mydata->fatsize == 32 && buffer_blk_cnt) - do_read = 0; - - if (do_read) { - read_blk = (mydata->fatsize == 32) ? - mydata->clust_size : PREFETCH_BLOCKS; - - debug("FAT read(sect=%d, cnt:%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n", - cursect, read_blk, mydata->clust_size, DIRENTSPERBLOCK); - - if (disk_read(cursect, read_blk, dir_ptr) < 0) { - debug("Error: reading rootdir block\n"); - goto exit; - } - - dentptr = (dir_entry *)dir_ptr; - } - - for (i = 0; i < DIRENTSPERBLOCK; i++) { - char s_name[14], l_name[VFAT_MAXLEN_BYTES]; - __u8 csum; - - l_name[0] = '\0'; - if (dentptr->name[0] == DELETED_FLAG) { - dentptr++; - continue; - } - - if (vfat_enabled) - csum = mkcksum(dentptr->name, dentptr->ext); - - if (dentptr->attr & ATTR_VOLUME) { - if (vfat_enabled && - (dentptr->attr & ATTR_VFAT) == ATTR_VFAT && - (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) { - prevcksum = - ((dir_slot *)dentptr)->alias_checksum; - - get_vfatname(mydata, - root_cluster, - dir_ptr, - dentptr, l_name); - - if (dols == LS_ROOT) { - char dirc; - int doit = 0; - int isdir = - (dentptr->attr & ATTR_DIR); - - if (isdir) { - dirs++; - dirc = '/'; - doit = 1; - } else { - dirc = ' '; - if (l_name[0] != 0) { - files++; - doit = 1; - } - } - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - l_name, - dirc); - } else { - printf(" %s%c\n", - l_name, - dirc); - } - } - dentptr++; - continue; - } - debug("Rootvfatname: |%s|\n", - l_name); - } else { - /* Volume label or VFAT entry */ - dentptr++; - continue; - } - } else if (dentptr->name[0] == 0) { - debug("RootDentname == NULL - %d\n", i); - if (dols == LS_ROOT) { - printf("\n%d file(s), %d dir(s)\n\n", - files, dirs); - ret = 0; - } - goto exit; - } - else if (vfat_enabled && - dols == LS_ROOT && csum == prevcksum) { - prevcksum = 0xffff; - dentptr++; - continue; - } - - get_name(dentptr, s_name); - - if (dols == LS_ROOT) { - int isdir = (dentptr->attr & ATTR_DIR); - char dirc; - int doit = 0; - - if (isdir) { - dirc = '/'; - if (s_name[0] != 0) { - dirs++; - doit = 1; - } - } else { - dirc = ' '; - if (s_name[0] != 0) { - files++; - doit = 1; - } - } - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - s_name, dirc); - } else { - printf(" %s%c\n", - s_name, dirc); - } - } - dentptr++; - continue; - } - - if (strcmp(fnamecopy, s_name) - && strcmp(fnamecopy, l_name)) { - debug("RootMismatch: |%s|%s|\n", s_name, - l_name); - dentptr++; - continue; - } - - if (isdir && !(dentptr->attr & ATTR_DIR)) - goto exit; - - debug("RootName: %s", s_name); - debug(", start: 0x%x", START(dentptr)); - debug(", size: 0x%x %s\n", - FAT2CPU32(dentptr->size), - isdir ? "(DIR)" : ""); - - goto rootdir_done; /* We got a match */ - } - debug("END LOOP: buffer_blk_cnt=%d clust_size=%d\n", buffer_blk_cnt, - mydata->clust_size); - - /* - * On FAT32 we must fetch the FAT entries for the next - * root directory clusters when a cluster has been - * completely processed. - */ - ++buffer_blk_cnt; - int rootdir_end = 0; - if (mydata->fatsize == 32) { - if (buffer_blk_cnt == mydata->clust_size) { - int nxtsect = 0; - int nxt_clust = 0; - - nxt_clust = get_fatent(mydata, root_cluster); - rootdir_end = CHECK_CLUST(nxt_clust, 32); - - nxtsect = mydata->data_begin + - (nxt_clust * mydata->clust_size); - - root_cluster = nxt_clust; - - cursect = nxtsect; - buffer_blk_cnt = 0; - } - } else { - if (buffer_blk_cnt == PREFETCH_BLOCKS) - buffer_blk_cnt = 0; - - rootdir_end = (++cursect - mydata->rootdir_sect >= - rootdir_size); - } - - /* If end of rootdir reached */ - if (rootdir_end) { - if (dols == LS_ROOT) { - printf("\n%d file(s), %d dir(s)\n\n", - files, dirs); - *size = 0; - } - goto exit; - } - } -rootdir_done: - - firsttime = 1; - - while (isdir) { - int startsect = mydata->data_begin - + START(dentptr) * mydata->clust_size; - dir_entry dent; - char *nextname = NULL; - - dent = *dentptr; - dentptr = &dent; - - idx = dirdelim(subname); - - if (idx >= 0) { - subname[idx] = '\0'; - nextname = subname + idx + 1; - /* Handle multiple delimiters */ - while (ISDIRDELIM(*nextname)) - nextname++; - if (dols && *nextname == '\0') - firsttime = 0; - } else { - if (dols && firsttime) { - firsttime = 0; - } else { - isdir = 0; - } - } - - if (get_dentfromdir(mydata, startsect, subname, dentptr, - isdir ? 0 : dols) == NULL) { - if (dols && !isdir) - *size = 0; - goto exit; - } - - if (isdir && !(dentptr->attr & ATTR_DIR)) - goto exit; - - /* - * If we are looking for a directory, and found a directory - * type entry, and the entry is for the root directory (as - * denoted by a cluster number of 0), jump back to the start - * of the function, since at least on FAT12/16, the root dir - * lives in a hard-coded location and needs special handling - * to parse, rather than simply following the cluster linked - * list in the FAT, like other directories. - */ - if (isdir && (dentptr->attr & ATTR_DIR) && !START(dentptr)) { - /* - * Modify the filename to remove the prefix that gets - * back to the root directory, so the initial root dir - * parsing code can continue from where we are without - * confusion. - */ - strcpy(fnamecopy, nextname ?: ""); - /* - * Set up state the same way as the function does when - * first started. This is required for the root dir - * parsing code operates in its expected environment. - */ - subname = ""; - cursect = mydata->rootdir_sect; - isdir = 0; - goto root_reparse; - } - - if (idx >= 0) - subname = nextname; - } - - if (dogetsize) { - *size = FAT2CPU32(dentptr->size); - ret = 0; - } else { - ret = get_contents(mydata, dentptr, pos, buffer, maxsize, size); - } - debug("Size: %u, got: %llu\n", FAT2CPU32(dentptr->size), *size); - -exit: - free(mydata->fatbuf); - return ret; -} -
/* * Directory iterator, to simplify filesystem traversal @@ -1595,12 +966,6 @@ static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type) return -ENOENT; }
-int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int dols, - loff_t *actread) -{ - return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0, actread); -} - int file_fat_detectfs(void) { boot_sector bs; @@ -1665,31 +1030,96 @@ int file_fat_detectfs(void)
int file_fat_ls(const char *dir) { - loff_t size; + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; + int files = 0, dirs = 0; + int ret; + + ret = fat_itr_root(itr, &fsdata); + if (ret) + return ret; + + ret = fat_itr_resolve(itr, dir, TYPE_DIR); + if (ret) + return ret; + + while (fat_itr_next(itr)) { + if (fat_itr_isdir(itr)) { + printf(" %s/\n", itr->name); + dirs++; + } else { + printf(" %8u %s\n", + FAT2CPU32(itr->dent->size), + itr->name); + files++; + } + }
- return do_fat_read(dir, NULL, 0, LS_YES, &size); + printf("\n%d file(s), %d dir(s)\n\n", files, dirs); + + return 0; }
int fat_exists(const char *filename) { + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; int ret; - loff_t size;
- ret = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, &size); + ret = fat_itr_root(itr, &fsdata); + if (ret) + return 0; + + ret = fat_itr_resolve(itr, filename, TYPE_ANY); return ret == 0; }
int fat_size(const char *filename, loff_t *size) { - return do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, size); + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; + int ret; + + ret = fat_itr_root(itr, &fsdata); + if (ret) + return ret; + + ret = fat_itr_resolve(itr, filename, TYPE_FILE); + if (ret) { + /* + * Directories don't have size, but fs_size() is not + * expected to fail if passed a directory path: + */ + fat_itr_root(itr, &fsdata); + if (!fat_itr_resolve(itr, filename, TYPE_DIR)) { + *size = 0; + return 0; + } + return ret; + } + + *size = FAT2CPU32(itr->dent->size); + + return 0; }
int file_fat_read_at(const char *filename, loff_t pos, void *buffer, loff_t maxsize, loff_t *actread) { + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; + int ret; + + ret = fat_itr_root(itr, &fsdata); + if (ret) + return ret; + + ret = fat_itr_resolve(itr, filename, TYPE_FILE); + if (ret) + return ret; + printf("reading %s\n", filename); - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0, - actread); + return get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread); }
int file_fat_read(const char *filename, void *buffer, int maxsize) diff --git a/include/fat.h b/include/fat.h index 21bb6666cf..18d8981c48 100644 --- a/include/fat.h +++ b/include/fat.h @@ -58,12 +58,6 @@ */ #define LAST_LONG_ENTRY_MASK 0x40
-/* Flags telling whether we should read a file or list a directory */ -#define LS_NO 0 -#define LS_YES 1 -#define LS_DIR 1 -#define LS_ROOT 2 - #define ISDIRDELIM(c) ((c) == '/' || (c) == '\')
#define FSTYPE_NONE (-1)

On 9 September 2017 at 11:15, Rob Clark robdclark@gmail.com wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de
fs/fat/fat.c | 722 +++++++--------------------------------------------------- include/fat.h | 6 - 2 files changed, 76 insertions(+), 652 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Sep 09, 2017 at 01:15:54PM -0400, Rob Clark wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

On Fri, Sep 15, 2017 at 9:32 PM, Tom Rini trini@konsulko.com wrote:
On Sat, Sep 09, 2017 at 01:15:54PM -0400, Rob Clark wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
According to git bisect, this patch seems to break the booting of am3517_evm
It just displays:
U-Boot SPL 2017.09-00135-g8eafae2 (Sep 16 2017 - 15:23:16) Trying to boot from MMC1
Then hangs.
It does, however the same patch boots just fine on omap3_logic_defconfig which is an OMAP3630/DM3730 board.
da850evm_defconfig (OMAP-L138) is also booting just fine.
I'm going to investigate it further to see why just 1/3 boards seems impacted.
adam
-- Tom
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Sat, Sep 16, 2017 at 4:32 PM, Adam Ford aford173@gmail.com wrote:
On Fri, Sep 15, 2017 at 9:32 PM, Tom Rini trini@konsulko.com wrote:
On Sat, Sep 09, 2017 at 01:15:54PM -0400, Rob Clark wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
According to git bisect, this patch seems to break the booting of am3517_evm
It just displays:
U-Boot SPL 2017.09-00135-g8eafae2 (Sep 16 2017 - 15:23:16) Trying to boot from MMC1
Then hangs.
It does, however the same patch boots just fine on omap3_logic_defconfig which is an OMAP3630/DM3730 board.
da850evm_defconfig (OMAP-L138) is also booting just fine.
I'm going to investigate it further to see why just 1/3 boards seems impacted.
Tom reported a similar fail.. although I am failing to reproduce it in sandbox w/ a copy of his partition, so maybe it is somehow hw specific? If you could, debug logs w/ https://hastebin.com/raw/fijazebiso applied in before/after case would be useful.. maybe I could spot something from that?
BR, -R

On Sat, Sep 16, 2017 at 3:42 PM, Rob Clark robdclark@gmail.com wrote:
On Sat, Sep 16, 2017 at 4:32 PM, Adam Ford aford173@gmail.com wrote:
On Fri, Sep 15, 2017 at 9:32 PM, Tom Rini trini@konsulko.com wrote:
On Sat, Sep 09, 2017 at 01:15:54PM -0400, Rob Clark wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
According to git bisect, this patch seems to break the booting of am3517_evm
It just displays:
U-Boot SPL 2017.09-00135-g8eafae2 (Sep 16 2017 - 15:23:16) Trying to boot from MMC1
Then hangs.
It does, however the same patch boots just fine on omap3_logic_defconfig which is an OMAP3630/DM3730 board.
da850evm_defconfig (OMAP-L138) is also booting just fine.
I'm going to investigate it further to see why just 1/3 boards seems impacted.
Tom reported a similar fail.. although I am failing to reproduce it in sandbox w/ a copy of his partition, so maybe it is somehow hw specific? If you could, debug logs w/ https://hastebin.com/raw/fijazebiso applied in before/after case would be useful.. maybe I could spot something from that?
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
Could there a be some memory overflow somewhere?
Adding the debug to the last working commit, yielded the following:
U-Boot SPL 2017.09-00134-ge71f88d-dirty (Sep 17 2017 - 06:03:28) Trying to boot from MMC1 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=d RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x98, size: 0x83708 Filesize: u bytes u bytes gc - clustnum: 152, startsect: 1633 Size: 538376, got: u reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=d RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x98, size: 0x83708 Filesize: u bytes u bytes
I truncated it, because after that, it seems to just be showing entries and offsets over and over again.
adam
BR, -R

On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
On Sat, Sep 16, 2017 at 3:42 PM, Rob Clark robdclark@gmail.com wrote:
On Sat, Sep 16, 2017 at 4:32 PM, Adam Ford aford173@gmail.com wrote:
On Fri, Sep 15, 2017 at 9:32 PM, Tom Rini trini@konsulko.com wrote:
On Sat, Sep 09, 2017 at 01:15:54PM -0400, Rob Clark wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
According to git bisect, this patch seems to break the booting of am3517_evm
It just displays:
U-Boot SPL 2017.09-00135-g8eafae2 (Sep 16 2017 - 15:23:16) Trying to boot from MMC1
Then hangs.
It does, however the same patch boots just fine on omap3_logic_defconfig which is an OMAP3630/DM3730 board.
da850evm_defconfig (OMAP-L138) is also booting just fine.
I'm going to investigate it further to see why just 1/3 boards seems impacted.
Tom reported a similar fail.. although I am failing to reproduce it in sandbox w/ a copy of his partition, so maybe it is somehow hw specific? If you could, debug logs w/ https://hastebin.com/raw/fijazebiso applied in before/after case would be useful.. maybe I could spot something from that?
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
Could there a be some memory overflow somewhere?
Adding the debug to the last working commit, yielded the following:
U-Boot SPL 2017.09-00134-ge71f88d-dirty (Sep 17 2017 - 06:03:28) Trying to boot from MMC1 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=d RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x98, size: 0x83708 Filesize: u bytes u bytes gc - clustnum: 152, startsect: 1633 Size: 538376, got: u reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=d RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x98, size: 0x83708 Filesize: u bytes u bytes
I truncated it, because after that, it seems to just be showing entries and offsets over and over again.
Thanks, this first part is what I was looking for.
Not sure why DEBUG would make it hang on master. I don't know much about spl, but is there a way you can get it to just do 'ls mmc 1:1' instead of loading u-boot.img? ls should give me the same information about the partition, without so many repeating entries/offsets. Alternately, is it possible to boot w/ old spl image but new u-boot.img to debug this once we get into main u-boot image??
BR, -R

On Sun, Sep 17, 2017 at 8:28 AM, Rob Clark robdclark@gmail.com wrote:
On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
On Sat, Sep 16, 2017 at 3:42 PM, Rob Clark robdclark@gmail.com wrote:
On Sat, Sep 16, 2017 at 4:32 PM, Adam Ford aford173@gmail.com wrote:
On Fri, Sep 15, 2017 at 9:32 PM, Tom Rini trini@konsulko.com wrote:
On Sat, Sep 09, 2017 at 01:15:54PM -0400, Rob Clark wrote:
And drop a whole lot of ugly code!
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
According to git bisect, this patch seems to break the booting of am3517_evm
It just displays:
U-Boot SPL 2017.09-00135-g8eafae2 (Sep 16 2017 - 15:23:16) Trying to boot from MMC1
Then hangs.
It does, however the same patch boots just fine on omap3_logic_defconfig which is an OMAP3630/DM3730 board.
da850evm_defconfig (OMAP-L138) is also booting just fine.
I'm going to investigate it further to see why just 1/3 boards seems impacted.
Tom reported a similar fail.. although I am failing to reproduce it in sandbox w/ a copy of his partition, so maybe it is somehow hw specific? If you could, debug logs w/ https://hastebin.com/raw/fijazebiso applied in before/after case would be useful.. maybe I could spot something from that?
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
Could there a be some memory overflow somewhere?
Adding the debug to the last working commit, yielded the following:
U-Boot SPL 2017.09-00134-ge71f88d-dirty (Sep 17 2017 - 06:03:28) Trying to boot from MMC1 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=d RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x98, size: 0x83708 Filesize: u bytes u bytes gc - clustnum: 152, startsect: 1633 Size: 538376, got: u reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=d RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x98, size: 0x83708 Filesize: u bytes u bytes
I truncated it, because after that, it seems to just be showing entries and offsets over and over again.
Thanks, this first part is what I was looking for.
Not sure why DEBUG would make it hang on master. I don't know much about spl, but is there a way you can get it to just do 'ls mmc 1:1' instead of loading u-boot.img? ls should give me the same information about the partition, without so many repeating entries/offsets. Alternately, is it possible to boot w/ old spl image but new u-boot.img to debug this once we get into main u-boot image??
I suppose another benefit if it is possible to use old spl and new u-boot.img is the presumably u-boot.img isn't using TINY_PRINTF so the debug prints about the partition parameters would be more complete..
BR, -R

On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
hmm, one thing I noticed in doc/README.SPL:
"When building SPL with DEBUG set you may also need to set CONFIG_PANIC_HANG as in most cases do_reset is not defined within SPL."
not sure if that would help.
Also, it has a section about estimating stack usage.. not sure if this implies that stack size is constrained in spl? If so, maybe that is related. The new directory iterators do move the buffer for current block of dir_entry's to the stack. Reducing CONFIG_FS_FAT_MAX_CLUSTSIZE might help.
BR, -R

On Sun, Sep 17, 2017 at 08:53:23AM -0400, Rob Clark wrote:
On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
hmm, one thing I noticed in doc/README.SPL:
"When building SPL with DEBUG set you may also need to set CONFIG_PANIC_HANG as in most cases do_reset is not defined within SPL."
not sure if that would help.
Also, it has a section about estimating stack usage.. not sure if this implies that stack size is constrained in spl? If so, maybe that is related. The new directory iterators do move the buffer for current block of dir_entry's to the stack. Reducing CONFIG_FS_FAT_MAX_CLUSTSIZE might help.
Yes, in SPL we don't require stack being in SDRAM and unless SPL_STACK_R is set we have a very limited stack available. And that looks to be the common thread between functional and non-functional platforms. In the case of what Adam and I both have, we could move to SPL_STACK_R easily enough. What needs to be done next 'tho would be to see if making SPL_FAT_SUPPORT depend on SPL_STACK_R, migrating the various TI platforms (and possibly others) that could butdon't use SPL_STACK_R, and seeing if we have platforms that have now lost FAT support.
Or, how hard would it be to make that code not use the stack so much?

On Sun, Sep 17, 2017 at 7:53 AM, Rob Clark robdclark@gmail.com wrote:
On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
With the TINY_PRINTF disabled (except in SPL), the log looks better:
Trying to boot from MMC1 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16 RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x12, size: 0x83708 Filesize: 538376 bytes 64 bytes gc - clustnum: 18, startsect: 561 Size: 538376, got: 64 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16 RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x12, size: 0x83708 Filesize: 538376 bytes
hmm, one thing I noticed in doc/README.SPL:
"When building SPL with DEBUG set you may also need to set CONFIG_PANIC_HANG as in most cases do_reset is not defined within SPL."
not sure if that would help.
I did try your experiment with keeping the working SPL and using the newer u-boot.img file and that worked, so I am guessing it's probably related to tight memory in SPL. I looked up the mapping in doc/SPL/README.omap3 and it shows:
0x40200800 - 0x4020BBFF: Area for SPL text, data and rodata 0x4020E000 - 0x4020FFFC: Area for the SPL stack. 0x80000000 - 0x8007FFFF: Area for the SPL BSS. 0x80100000: CONFIG_SYS_TEXT_BASE of U-Boot 0x80208000 - 0x80307FFF: malloc() pool available to SPL.
For this board, CONFIG_SYS_SPL_MALLOC_SIZE is set to 0x100000
I don't know if any of this is helpful information to you or not.
Also, it has a section about estimating stack usage.. not sure if this implies that stack size is constrained in spl? If so, maybe that is related. The new directory iterators do move the buffer for current block of dir_entry's to the stack. Reducing CONFIG_FS_FAT_MAX_CLUSTSIZE might help.
Any recommendations on how small this can go and still operate reliably? I don't want to just arbitrarily choose something.
BR, -R
adam

On Sun, Sep 17, 2017 at 9:42 AM, Adam Ford aford173@gmail.com wrote:
On Sun, Sep 17, 2017 at 7:53 AM, Rob Clark robdclark@gmail.com wrote:
On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
With the TINY_PRINTF disabled (except in SPL), the log looks better:
Trying to boot from MMC1 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16 RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x12, size: 0x83708 Filesize: 538376 bytes 64 bytes gc - clustnum: 18, startsect: 561 Size: 538376, got: 64 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16 RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x12, size: 0x83708 Filesize: 538376 bytes
hmm, one thing I noticed in doc/README.SPL:
"When building SPL with DEBUG set you may also need to set CONFIG_PANIC_HANG as in most cases do_reset is not defined within SPL."
not sure if that would help.
I did try your experiment with keeping the working SPL and using the newer u-boot.img file and that worked, so I am guessing it's probably related to tight memory in SPL. I looked up the mapping in doc/SPL/README.omap3 and it shows:
0x40200800 - 0x4020BBFF: Area for SPL text, data and rodata 0x4020E000 - 0x4020FFFC: Area for the SPL stack. 0x80000000 - 0x8007FFFF: Area for the SPL BSS. 0x80100000: CONFIG_SYS_TEXT_BASE of U-Boot 0x80208000 - 0x80307FFF: malloc() pool available to SPL.
For this board, CONFIG_SYS_SPL_MALLOC_SIZE is set to 0x100000
I don't know if any of this is helpful information to you or not.
I think we narrowed it down to stack usage.. Tom tested the "please test" patch I just sent and it works for him.
If reducing SPL_MALLOC_SIZE increases available stack size, that might also work. But I think for next release we can just go with my patch that moves itrblock off the stack.
Also, it has a section about estimating stack usage.. not sure if this implies that stack size is constrained in spl? If so, maybe that is related. The new directory iterators do move the buffer for current block of dir_entry's to the stack. Reducing CONFIG_FS_FAT_MAX_CLUSTSIZE might help.
Any recommendations on how small this can go and still operate reliably? I don't want to just arbitrarily choose something.
Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16
So I think 512 * 8 = 4096 would work for your filesys.. what I'm not entirely sure is what range of sector sizes and cluster sizes are possible. 64k seems *way* too much. Maybe in the end I should switch to malloc() so we can allocate just what is needed.
I'll give it some thought for my next batch of fs/fat work.. but suggestions welcome.
BR, -R

On Sun, Sep 17, 2017 at 8:50 AM, Rob Clark robdclark@gmail.com wrote:
On Sun, Sep 17, 2017 at 9:42 AM, Adam Ford aford173@gmail.com wrote:
On Sun, Sep 17, 2017 at 7:53 AM, Rob Clark robdclark@gmail.com wrote:
On Sun, Sep 17, 2017 at 7:08 AM, Adam Ford aford173@gmail.com wrote:
Adding the debug to master made no difference. No debug messages appeared. Interestingly enough, some junk appeard, and the name of U-Boot name wasn't correctly displayed:
**Sș017.09-00178-g08cebee-dirty (Sep 17 2017 - 06:01:07) Trying to boot from MMC1
(hang)
With the TINY_PRINTF disabled (except in SPL), the log looks better:
Trying to boot from MMC1 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16 RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x12, size: 0x83708 Filesize: 538376 bytes 64 bytes gc - clustnum: 18, startsect: 561 Size: 538376, got: 64 reading u-boot.img VFAT Support enabled FAT16, fat_sect: 1, fatlength: 200 Rootdir begins at cluster: 536870910, sector: 401, offset: 32200 Data begins at: 417 Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16 RootMismatch: |mlo|| Rootvfatname: |u-boot.img| RootName: u-boot.img, start: 0x12, size: 0x83708 Filesize: 538376 bytes
hmm, one thing I noticed in doc/README.SPL:
"When building SPL with DEBUG set you may also need to set CONFIG_PANIC_HANG as in most cases do_reset is not defined within SPL."
not sure if that would help.
I did try your experiment with keeping the working SPL and using the newer u-boot.img file and that worked, so I am guessing it's probably related to tight memory in SPL. I looked up the mapping in doc/SPL/README.omap3 and it shows:
0x40200800 - 0x4020BBFF: Area for SPL text, data and rodata 0x4020E000 - 0x4020FFFC: Area for the SPL stack. 0x80000000 - 0x8007FFFF: Area for the SPL BSS. 0x80100000: CONFIG_SYS_TEXT_BASE of U-Boot 0x80208000 - 0x80307FFF: malloc() pool available to SPL.
For this board, CONFIG_SYS_SPL_MALLOC_SIZE is set to 0x100000
I don't know if any of this is helpful information to you or not.
I think we narrowed it down to stack usage.. Tom tested the "please test" patch I just sent and it works for him.
If reducing SPL_MALLOC_SIZE increases available stack size, that might also work. But I think for next release we can just go with my patch that moves itrblock off the stack.
Also, it has a section about estimating stack usage.. not sure if this implies that stack size is constrained in spl? If so, maybe that is related. The new directory iterators do move the buffer for current block of dir_entry's to the stack. Reducing CONFIG_FS_FAT_MAX_CLUSTSIZE might help.
Any recommendations on how small this can go and still operate reliably? I don't want to just arbitrarily choose something.
Sector size: 512, cluster size: 8 FAT read(sect=401, cnt:2), clust_size=8, DIRENTSPERBLOCK=16
So I think 512 * 8 = 4096 would work for your filesys.. what I'm not entirely sure is what range of sector sizes and cluster sizes are possible. 64k seems *way* too much. Maybe in the end I should switch to malloc() so we can allocate just what is needed.
I'll give it some thought for my next batch of fs/fat work.. but suggestions welcome.
When I wrote that, I didn't see either Tom's comments or you updated patch yet, but you can go ahead and add my tested-by because I'm satisfied with that.
adam
BR, -R

Needed to support efi file protocol. The fallback.efi loader wants to be able to read the contents of the /EFI directory to find an OS to boot.
Modelled after POSIX opendir()/readdir()/closedir(). Unlike the other fs APIs, this is stateful (ie. state is held in the FS_DIR "directory stream"), to avoid re-traversing of the directory structure at each step. The directory stream must be released with closedir() when it is no longer needed.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de --- disk/part.c | 31 ++++++++++------- fs/fs.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs.h | 67 +++++++++++++++++++++++++++++++++++++ include/part.h | 9 +++++ 4 files changed, 199 insertions(+), 12 deletions(-)
diff --git a/disk/part.c b/disk/part.c index c67fdacc79..aa9183d696 100644 --- a/disk/part.c +++ b/disk/part.c @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -1; }
+int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info) +{ + info->start = 0; + info->size = dev_desc->lba; + info->blksz = dev_desc->blksz; + info->bootable = 0; + strcpy((char *)info->type, BOOT_PART_TYPE); + strcpy((char *)info->name, "Whole Disk"); +#if CONFIG_IS_ENABLED(PARTITION_UUIDS) + info->uuid[0] = 0; +#endif +#ifdef CONFIG_PARTITION_TYPE_GUID + info->type_guid[0] = 0; +#endif + + return 0; +} + int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, struct blk_desc **dev_desc) { @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
(*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);
- info->start = 0; - info->size = (*dev_desc)->lba; - info->blksz = (*dev_desc)->blksz; - info->bootable = 0; - strcpy((char *)info->type, BOOT_PART_TYPE); - strcpy((char *)info->name, "Whole Disk"); -#if CONFIG_IS_ENABLED(PARTITION_UUIDS) - info->uuid[0] = 0; -#endif -#ifdef CONFIG_PARTITION_TYPE_GUID - info->type_guid[0] = 0; -#endif + part_get_info_whole_disk(*dev_desc, info);
ret = 0; goto cleanup; diff --git a/fs/fs.c b/fs/fs.c index 13cd3626c6..fc0c953fcb 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -21,6 +21,7 @@ DECLARE_GLOBAL_DATA_PTR;
static struct blk_desc *fs_dev_desc; +static int fs_dev_part; static disk_partition_t fs_partition; static int fs_type = FS_TYPE_ANY;
@@ -69,6 +70,12 @@ static inline int fs_uuid_unsupported(char *uuid_str) return -1; }
+static inline int fs_opendir_unsupported(const char *filename, + struct fs_dir_stream **dirs) +{ + return -EACCES; +} + struct fstype_info { int fstype; char *name; @@ -92,6 +99,20 @@ struct fstype_info { loff_t len, loff_t *actwrite); void (*close)(void); int (*uuid)(char *uuid_str); + /* + * Open a directory stream. On success return 0 and directory + * stream pointer via 'dirsp'. On error, return -errno. See + * fs_opendir(). + */ + int (*opendir)(const char *filename, struct fs_dir_stream **dirsp); + /* + * Read next entry from directory stream. On success return 0 + * and directory entry pointer via 'dentp'. On error return + * -errno. See fs_readdir(). + */ + int (*readdir)(struct fs_dir_stream *dirs, struct fs_dirent **dentp); + /* see fs_closedir() */ + void (*closedir)(struct fs_dir_stream *dirs); };
static struct fstype_info fstypes[] = { @@ -112,6 +133,7 @@ static struct fstype_info fstypes[] = { .write = fs_write_unsupported, #endif .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, #endif #ifdef CONFIG_FS_EXT4 @@ -131,6 +153,7 @@ static struct fstype_info fstypes[] = { .write = fs_write_unsupported, #endif .uuid = ext4fs_uuid, + .opendir = fs_opendir_unsupported, }, #endif #ifdef CONFIG_SANDBOX @@ -146,6 +169,7 @@ static struct fstype_info fstypes[] = { .read = fs_read_sandbox, .write = fs_write_sandbox, .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, #endif #ifdef CONFIG_CMD_UBIFS @@ -161,6 +185,7 @@ static struct fstype_info fstypes[] = { .read = ubifs_read, .write = fs_write_unsupported, .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, #endif { @@ -175,6 +200,7 @@ static struct fstype_info fstypes[] = { .read = fs_read_unsupported, .write = fs_write_unsupported, .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, };
@@ -228,6 +254,31 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
if (!info->probe(fs_dev_desc, &fs_partition)) { fs_type = info->fstype; + fs_dev_part = part; + return 0; + } + } + + return -1; +} + +/* set current blk device w/ blk_desc + partition # */ +int fs_set_blk_dev_with_part(struct blk_desc *desc, int part) +{ + struct fstype_info *info; + int ret, i; + + if (part >= 1) + ret = part_get_info(desc, part, &fs_partition); + else + ret = part_get_info_whole_disk(desc, &fs_partition); + if (ret) + return ret; + fs_dev_desc = desc; + + for (i = 0, info = fstypes; i < ARRAY_SIZE(fstypes); i++, info++) { + if (!info->probe(fs_dev_desc, &fs_partition)) { + fs_type = info->fstype; return 0; } } @@ -334,6 +385,59 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, return ret; }
+struct fs_dir_stream *fs_opendir(const char *filename) +{ + struct fstype_info *info = fs_get_info(fs_type); + struct fs_dir_stream *dirs = NULL; + int ret; + + ret = info->opendir(filename, &dirs); + fs_close(); + if (ret) { + errno = -ret; + return NULL; + } + + dirs->desc = fs_dev_desc; + dirs->part = fs_dev_part; + + return dirs; +} + +struct fs_dirent *fs_readdir(struct fs_dir_stream *dirs) +{ + struct fstype_info *info; + struct fs_dirent *dirent; + int ret; + + fs_set_blk_dev_with_part(dirs->desc, dirs->part); + info = fs_get_info(fs_type); + + ret = info->readdir(dirs, &dirent); + fs_close(); + if (ret) { + errno = -ret; + return NULL; + } + + return dirent; +} + +void fs_closedir(struct fs_dir_stream *dirs) +{ + struct fstype_info *info; + + if (!dirs) + return; + + fs_set_blk_dev_with_part(dirs->desc, dirs->part); + info = fs_get_info(fs_type); + + info->closedir(dirs); + fs_close(); +} + + int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype) { diff --git a/include/fs.h b/include/fs.h index 2f2aca8378..0869ad6e80 100644 --- a/include/fs.h +++ b/include/fs.h @@ -27,6 +27,17 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);
/* + * fs_set_blk_dev_with_part - Set current block device + partition + * + * Similar to fs_set_blk_dev(), but useful for cases where you already + * know the blk_desc and part number. + * + * Returns 0 on success. + * Returns non-zero if invalid partition or error accessing the disk. + */ +int fs_set_blk_dev_with_part(struct blk_desc *desc, int part); + +/* * Print the list of files on the partition previously set by fs_set_blk_dev(), * in directory "dirname". * @@ -79,6 +90,62 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, loff_t *actwrite);
/* + * Directory entry types, matches the subset of DT_x in posix readdir() + * which apply to u-boot. + */ +#define FS_DT_DIR 4 /* directory */ +#define FS_DT_REG 8 /* regular file */ +#define FS_DT_LNK 10 /* symbolic link */ + +/* + * A directory entry, returned by fs_readdir(). Returns information + * about the file/directory at the current directory entry position. + */ +struct fs_dirent { + unsigned type; /* one of FS_DT_x (not a mask) */ + loff_t size; /* size in bytes */ + char name[256]; +}; + +/* Note: fs_dir_stream should be treated as opaque to the user of fs layer */ +struct fs_dir_stream { + /* private to fs. layer: */ + struct blk_desc *desc; + int part; +}; + +/* + * fs_opendir - Open a directory + * + * @filename: the path to directory to open + * @return a pointer to the directory stream or NULL on error and errno + * set appropriately + */ +struct fs_dir_stream *fs_opendir(const char *filename); + +/* + * fs_readdir - Read the next directory entry in the directory stream. + * + * Works in an analogous way to posix readdir(). The previously returned + * directory entry is no longer valid after calling fs_readdir() again. + * After fs_closedir() is called, the returned directory entry is no + * longer valid. + * + * @dirs: the directory stream + * @return the next directory entry (only valid until next fs_readdir() or + * fs_closedir() call, do not attempt to free()) or NULL if the end of + * the directory is reached. + */ +struct fs_dirent *fs_readdir(struct fs_dir_stream *dirs); + +/* + * fs_closedir - close a directory stream + * + * @dirs: the directory stream + */ +void fs_closedir(struct fs_dir_stream *dirs); + +/* * Common implementation for various filesystem commands, optionally limited * to a specific filesystem type via the fstype parameter. */ diff --git a/include/part.h b/include/part.h index 0d5c99836b..86117a7ce5 100644 --- a/include/part.h +++ b/include/part.h @@ -98,6 +98,12 @@ int host_get_dev_err(int dev, struct blk_desc **blk_devp);
/* disk/part.c */ int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info); +/** + * part_get_info_whole_disk() - get partition info for the special case of + * a partition occupying the entire disk. + */ +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info); + void part_print(struct blk_desc *dev_desc); void part_init(struct blk_desc *dev_desc); void dev_print(struct blk_desc *dev_desc); @@ -203,6 +209,9 @@ static inline struct blk_desc *mg_disk_get_dev(int dev) { return NULL; }
static inline int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info) { return -1; } +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc, + disk_partition_t *info) +{ return -1; } static inline void part_print(struct blk_desc *dev_desc) {} static inline void part_init(struct blk_desc *dev_desc) {} static inline void dev_print(struct blk_desc *dev_desc) {}

On 9 September 2017 at 11:15, Rob Clark robdclark@gmail.com wrote:
Needed to support efi file protocol. The fallback.efi loader wants to be able to read the contents of the /EFI directory to find an OS to boot.
Modelled after POSIX opendir()/readdir()/closedir(). Unlike the other fs APIs, this is stateful (ie. state is held in the FS_DIR "directory stream"), to avoid re-traversing of the directory structure at each step. The directory stream must be released with closedir() when it is no longer needed.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de
disk/part.c | 31 ++++++++++------- fs/fs.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs.h | 67 +++++++++++++++++++++++++++++++++++++ include/part.h | 9 +++++ 4 files changed, 199 insertions(+), 12 deletions(-)
I do think it is useful to see a change log on each patch.
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Sep 09, 2017 at 01:15:55PM -0400, Rob Clark wrote:
Needed to support efi file protocol. The fallback.efi loader wants to be able to read the contents of the /EFI directory to find an OS to boot.
Modelled after POSIX opendir()/readdir()/closedir(). Unlike the other fs APIs, this is stateful (ie. state is held in the FS_DIR "directory stream"), to avoid re-traversing of the directory structure at each step. The directory stream must be released with closedir() when it is no longer needed.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Implement the readdir interface using the directory iterators.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de --- fs/fat/fat.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index bbba7947ee..82ddb7eab1 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -14,6 +14,7 @@ #include <config.h> #include <exports.h> #include <fat.h> +#include <fs.h> #include <asm/byteorder.h> #include <part.h> #include <malloc.h> @@ -1146,6 +1147,66 @@ int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len, return ret; }
+typedef struct { + struct fs_dir_stream parent; + struct fs_dirent dirent; + fsdata fsdata; + fat_itr itr; +} fat_dir; + +int fat_opendir(const char *filename, struct fs_dir_stream **dirsp) +{ + fat_dir *dir = malloc(sizeof(*dir)); + int ret; + + if (!dir) + return -ENOMEM; + + ret = fat_itr_root(&dir->itr, &dir->fsdata); + if (ret) + goto fail; + + ret = fat_itr_resolve(&dir->itr, filename, TYPE_DIR); + if (ret) + goto fail; + + *dirsp = (struct fs_dir_stream *)dir; + return 0; + +fail: + free(dir); + return ret; +} + +int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp) +{ + fat_dir *dir = (fat_dir *)dirs; + struct fs_dirent *dent = &dir->dirent; + + if (!fat_itr_next(&dir->itr)) + return -ENOENT; + + memset(dent, 0, sizeof(*dent)); + strcpy(dent->name, dir->itr.name); + + if (fat_itr_isdir(&dir->itr)) { + dent->type = FS_DT_DIR; + } else { + dent->type = FS_DT_REG; + dent->size = FAT2CPU32(dir->itr.dent->size); + } + + *dentp = dent; + + return 0; +} + +void fat_closedir(struct fs_dir_stream *dirs) +{ + fat_dir *dir = (fat_dir *)dirs; + free(dir); +} + void fat_close(void) { }

On 9 September 2017 at 11:15, Rob Clark robdclark@gmail.com wrote:
Implement the readdir interface using the directory iterators.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de
fs/fat/fat.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But I still feel unconvinced that you need to add the typedef.

On Sat, Sep 09, 2017 at 01:15:56PM -0400, Rob Clark wrote:
Implement the readdir interface using the directory iterators.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Spotted by chance, when trying to remove file_fat_ls(), I noticed there were some dead users of the API.
Signed-off-by: Rob Clark robdclark@gmail.com Acked-by: Stefan Brüns stefan.bruens@rwth-aachen.de Reviewed-by: Simon Glass sjg@chromium.org --- fs/fat/Makefile | 4 -- fs/fat/file.c | 183 -------------------------------------------------------- include/fat.h | 20 ------- 3 files changed, 207 deletions(-) delete mode 100644 fs/fat/file.c
diff --git a/fs/fat/Makefile b/fs/fat/Makefile index b60e8486c4..3e2a6b01a8 100644 --- a/fs/fat/Makefile +++ b/fs/fat/Makefile @@ -5,7 +5,3 @@
obj-$(CONFIG_FS_FAT) := fat.o obj-$(CONFIG_FAT_WRITE):= fat_write.o - -ifndef CONFIG_SPL_BUILD -obj-$(CONFIG_FS_FAT) += file.o -endif diff --git a/fs/fat/file.c b/fs/fat/file.c deleted file mode 100644 index 89706117b9..0000000000 --- a/fs/fat/file.c +++ /dev/null @@ -1,183 +0,0 @@ -/* - * file.c - * - * Mini "VFS" by Marcus Sundberg - * - * 2002-07-28 - rjones@nexus-tech.net - ported to ppcboot v1.1.6 - * 2003-03-10 - kharris@nexus-tech.net - ported to uboot - * - * SPDX-License-Identifier: GPL-2.0+ - */ - -#include <common.h> -#include <config.h> -#include <malloc.h> -#include <fat.h> -#include <linux/stat.h> -#include <linux/time.h> - -/* Supported filesystems */ -static const struct filesystem filesystems[] = { - { file_fat_detectfs, file_fat_ls, file_fat_read, "FAT" }, -}; -#define NUM_FILESYS (sizeof(filesystems)/sizeof(struct filesystem)) - -/* The filesystem which was last detected */ -static int current_filesystem = FSTYPE_NONE; - -/* The current working directory */ -#define CWD_LEN 511 -char file_cwd[CWD_LEN+1] = "/"; - -const char * -file_getfsname(int idx) -{ - if (idx < 0 || idx >= NUM_FILESYS) - return NULL; - - return filesystems[idx].name; -} - -static void -pathcpy(char *dest, const char *src) -{ - char *origdest = dest; - - do { - if (dest-file_cwd >= CWD_LEN) { - *dest = '\0'; - return; - } - *(dest) = *(src); - if (*src == '\0') { - if (dest-- != origdest && ISDIRDELIM(*dest)) { - *dest = '\0'; - } - return; - } - ++dest; - - if (ISDIRDELIM(*src)) - while (ISDIRDELIM(*src)) src++; - else - src++; - } while (1); -} - -int -file_cd(const char *path) -{ - if (ISDIRDELIM(*path)) { - while (ISDIRDELIM(*path)) path++; - strncpy(file_cwd+1, path, CWD_LEN-1); - } else { - const char *origpath = path; - char *tmpstr = file_cwd; - int back = 0; - - while (*tmpstr != '\0') tmpstr++; - do { - tmpstr--; - } while (ISDIRDELIM(*tmpstr)); - - while (*path == '.') { - path++; - while (*path == '.') { - path++; - back++; - } - if (*path != '\0' && !ISDIRDELIM(*path)) { - path = origpath; - back = 0; - break; - } - while (ISDIRDELIM(*path)) path++; - origpath = path; - } - - while (back--) { - /* Strip off path component */ - while (!ISDIRDELIM(*tmpstr)) { - tmpstr--; - } - if (tmpstr == file_cwd) { - /* Incremented again right after the loop. */ - tmpstr--; - break; - } - /* Skip delimiters */ - while (ISDIRDELIM(*tmpstr)) tmpstr--; - } - tmpstr++; - if (*path == '\0') { - if (tmpstr == file_cwd) { - *tmpstr = '/'; - tmpstr++; - } - *tmpstr = '\0'; - return 0; - } - *tmpstr = '/'; - pathcpy(tmpstr+1, path); - } - - return 0; -} - -int -file_detectfs(void) -{ - int i; - - current_filesystem = FSTYPE_NONE; - - for (i = 0; i < NUM_FILESYS; i++) { - if (filesystems[i].detect() == 0) { - strcpy(file_cwd, "/"); - current_filesystem = i; - break; - } - } - - return current_filesystem; -} - -int -file_ls(const char *dir) -{ - char fullpath[1024]; - const char *arg; - - if (current_filesystem == FSTYPE_NONE) { - printf("Can't list files without a filesystem!\n"); - return -1; - } - - if (ISDIRDELIM(*dir)) { - arg = dir; - } else { - sprintf(fullpath, "%s/%s", file_cwd, dir); - arg = fullpath; - } - return filesystems[current_filesystem].ls(arg); -} - -int file_read(const char *filename, void *buffer, int maxsize) -{ - char fullpath[1024]; - const char *arg; - - if (current_filesystem == FSTYPE_NONE) { - printf("Can't load file without a filesystem!\n"); - return -1; - } - - if (ISDIRDELIM(*filename)) { - arg = filename; - } else { - sprintf(fullpath, "%s/%s", file_cwd, filename); - arg = fullpath; - } - - return filesystems[current_filesystem].read(arg, buffer, maxsize); -} diff --git a/include/fat.h b/include/fat.h index 18d8981c48..b255ce5337 100644 --- a/include/fat.h +++ b/include/fat.h @@ -178,25 +178,6 @@ static inline u32 clust_to_sect(fsdata *fsdata, u32 clust) return fsdata->data_begin + clust * fsdata->clust_size; }
-typedef int (file_detectfs_func)(void); -typedef int (file_ls_func)(const char *dir); -typedef int (file_read_func)(const char *filename, void *buffer, - int maxsize); - -struct filesystem { - file_detectfs_func *detect; - file_ls_func *ls; - file_read_func *read; - const char name[12]; -}; - -/* FAT tables */ -file_detectfs_func file_fat_detectfs; -file_ls_func file_fat_ls; -file_read_func file_fat_read; - -/* Currently this doesn't check if the dir exists or is valid... */ -int file_cd(const char *path); int file_fat_detectfs(void); int file_fat_ls(const char *dir); int fat_exists(const char *filename); @@ -204,7 +185,6 @@ int fat_size(const char *filename, loff_t *size); int file_fat_read_at(const char *filename, loff_t pos, void *buffer, loff_t maxsize, loff_t *actread); int file_fat_read(const char *filename, void *buffer, int maxsize); -const char *file_getfsname(int idx); int fat_set_blk_dev(struct blk_desc *rbdd, disk_partition_t *info); int fat_register_device(struct blk_desc *dev_desc, int part_no);

On Sat, Sep 09, 2017 at 01:15:57PM -0400, Rob Clark wrote:
Spotted by chance, when trying to remove file_fat_ls(), I noticed there were some dead users of the API.
Signed-off-by: Rob Clark robdclark@gmail.com Acked-by: Stefan Brüns stefan.bruens@rwth-aachen.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Add a generic implementation of 'ls' using opendir/readdir/closedir, and replace fat's custom implementation. Other filesystems should move to the generic implementation after they add opendir/readdir/closedir support.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org --- fs/fat/fat.c | 32 -------------------------------- fs/fs.c | 35 +++++++++++++++++++++++++++++++++-- include/fat.h | 5 ++++- 3 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 82ddb7eab1..c951d84f57 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -1029,38 +1029,6 @@ int file_fat_detectfs(void) return 0; }
-int file_fat_ls(const char *dir) -{ - fsdata fsdata; - fat_itr itrblock, *itr = &itrblock; - int files = 0, dirs = 0; - int ret; - - ret = fat_itr_root(itr, &fsdata); - if (ret) - return ret; - - ret = fat_itr_resolve(itr, dir, TYPE_DIR); - if (ret) - return ret; - - while (fat_itr_next(itr)) { - if (fat_itr_isdir(itr)) { - printf(" %s/\n", itr->name); - dirs++; - } else { - printf(" %8u %s\n", - FAT2CPU32(itr->dent->size), - itr->name); - files++; - } - } - - printf("\n%d file(s), %d dir(s)\n\n", files, dirs); - - return 0; -} - int fat_exists(const char *filename) { fsdata fsdata; diff --git a/fs/fs.c b/fs/fs.c index fc0c953fcb..3481229aa6 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -37,6 +37,35 @@ static inline int fs_ls_unsupported(const char *dirname) return -1; }
+/* generic implementation of ls in terms of opendir/readdir/closedir */ +__maybe_unused +static int fs_ls_generic(const char *dirname) +{ + struct fs_dir_stream *dirs; + struct fs_dirent *dent; + int nfiles = 0, ndirs = 0; + + dirs = fs_opendir(dirname); + if (!dirs) + return -errno; + + while ((dent = fs_readdir(dirs))) { + if (dent->type == FS_DT_DIR) { + printf(" %s/\n", dent->name); + ndirs++; + } else { + printf(" %8lld %s\n", dent->size, dent->name); + nfiles++; + } + } + + fs_closedir(dirs); + + printf("\n%d file(s), %d dir(s)\n\n", nfiles, ndirs); + + return 0; +} + static inline int fs_exists_unsupported(const char *filename) { return 0; @@ -123,7 +152,7 @@ static struct fstype_info fstypes[] = { .null_dev_desc_ok = false, .probe = fat_set_blk_dev, .close = fat_close, - .ls = file_fat_ls, + .ls = fs_ls_generic, .exists = fat_exists, .size = fat_size, .read = fat_read_file, @@ -133,7 +162,9 @@ static struct fstype_info fstypes[] = { .write = fs_write_unsupported, #endif .uuid = fs_uuid_unsupported, - .opendir = fs_opendir_unsupported, + .opendir = fat_opendir, + .readdir = fat_readdir, + .closedir = fat_closedir, }, #endif #ifdef CONFIG_FS_EXT4 diff --git a/include/fat.h b/include/fat.h index b255ce5337..83cd90017e 100644 --- a/include/fat.h +++ b/include/fat.h @@ -11,6 +11,7 @@ #define _FAT_H_
#include <asm/byteorder.h> +#include <fs.h>
#define CONFIG_SUPPORT_VFAT /* Maximum Long File Name length supported here is 128 UTF-16 code units */ @@ -179,7 +180,6 @@ static inline u32 clust_to_sect(fsdata *fsdata, u32 clust) }
int file_fat_detectfs(void); -int file_fat_ls(const char *dir); int fat_exists(const char *filename); int fat_size(const char *filename, loff_t *size); int file_fat_read_at(const char *filename, loff_t pos, void *buffer, @@ -192,5 +192,8 @@ int file_fat_write(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actwrite); int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actread); +int fat_opendir(const char *filename, struct fs_dir_stream **dirsp); +int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp); +void fat_closedir(struct fs_dir_stream *dirs); void fat_close(void); #endif /* _FAT_H_ */

On Sat, Sep 09, 2017 at 01:15:58PM -0400, Rob Clark wrote:
Add a generic implementation of 'ls' using opendir/readdir/closedir, and replace fat's custom implementation. Other filesystems should move to the generic implementation after they add opendir/readdir/closedir support.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Noticed when comparing our output to linux. There are some lcase bits which control whether filename and/or extension should be downcase'd.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de --- fs/fat/fat.c | 16 ++++++++++------ fs/fat/fat_write.c | 4 ++-- include/fat.h | 6 +++++- 3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index c951d84f57..65873a2c2a 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -29,11 +29,13 @@ static const int vfat_enabled = 0; #endif
/* - * Convert a string to lowercase. + * Convert a string to lowercase. Converts at most 'len' characters, + * 'len' may be larger than the length of 'str' if 'str' is NULL + * terminated. */ -static void downcase(char *str) +static void downcase(char *str, size_t len) { - while (*str != '\0') { + while (*str != '\0' && len--) { *str = tolower(*str); str++; } @@ -131,10 +133,13 @@ static void get_name(dir_entry *dirent, char *s_name) ptr = s_name; while (*ptr && *ptr != ' ') ptr++; + if (dirent->lcase & CASE_LOWER_BASE) + downcase(s_name, (unsigned)(ptr - s_name)); if (dirent->ext[0] && dirent->ext[0] != ' ') { - *ptr = '.'; - ptr++; + *ptr++ = '.'; memcpy(ptr, dirent->ext, 3); + if (dirent->lcase & CASE_LOWER_EXT) + downcase(ptr, 3); ptr[3] = '\0'; while (*ptr && *ptr != ' ') ptr++; @@ -144,7 +149,6 @@ static void get_name(dir_entry *dirent, char *s_name) *s_name = '\0'; else if (*s_name == aRING) *s_name = DELETED_FLAG; - downcase(s_name); }
static int flush_dirty_fat_buffer(fsdata *mydata); diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 4ca024c208..d0468baf8f 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster, *l_name = '\0'; else if (*l_name == aRING) *l_name = DELETED_FLAG; - downcase(l_name); + downcase(l_name, INT_MAX);
/* Return the real directory entry */ *retdent = realdent; @@ -981,7 +981,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
memcpy(l_filename, filename, name_len); l_filename[name_len] = 0; /* terminate the string */ - downcase(l_filename); + downcase(l_filename, INT_MAX);
startsect = mydata->rootdir_sect; retdent = find_directory_entry(mydata, startsect, diff --git a/include/fat.h b/include/fat.h index 83cd90017e..0f58939124 100644 --- a/include/fat.h +++ b/include/fat.h @@ -128,10 +128,14 @@ typedef struct volume_info /* Boot sign comes last, 2 bytes */ } volume_info;
+/* see dir_entry::lcase: */ +#define CASE_LOWER_BASE 8 /* base (name) is lower case */ +#define CASE_LOWER_EXT 16 /* extension is lower case */ + typedef struct dir_entry { char name[8],ext[3]; /* Name and extension */ __u8 attr; /* Attribute bits */ - __u8 lcase; /* Case for base and extension */ + __u8 lcase; /* Case for name and ext (CASE_LOWER_x) */ __u8 ctime_ms; /* Creation time, milliseconds */ __u16 ctime; /* Creation time */ __u16 cdate; /* Creation date */

On 9 September 2017 at 11:15, Rob Clark robdclark@gmail.com wrote:
Noticed when comparing our output to linux. There are some lcase bits which control whether filename and/or extension should be downcase'd.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de
fs/fat/fat.c | 16 ++++++++++------ fs/fat/fat_write.c | 4 ++-- include/fat.h | 6 +++++- 3 files changed, 17 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Sep 09, 2017 at 01:15:59PM -0400, Rob Clark wrote:
Noticed when comparing our output to linux. There are some lcase bits which control whether filename and/or extension should be downcase'd.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Łukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Use the clust_to_sect() helper that was introduced earlier, and add an inverse sect_to_clust(), plus update the various spots that open-coded this conversion previously.
Signed-off-by: Rob Clark robdclark@gmail.com --- fs/fat/fat.c | 8 +++----- fs/fat/fat_write.c | 8 +++----- include/fat.h | 6 +++++- 3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 65873a2c2a..f5f74c12ff 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -257,8 +257,7 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size) int ret;
if (clustnum > 0) { - startsect = mydata->data_begin + - clustnum * mydata->clust_size; + startsect = clust_to_sect(mydata, clustnum); } else { startsect = mydata->rootdir_sect; } @@ -594,9 +593,8 @@ static int get_fs_info(fsdata *mydata) mydata->data_begin = mydata->rootdir_sect + mydata->rootdir_size - (mydata->clust_size * 2); - mydata->root_cluster = (mydata->rootdir_sect - - mydata->data_begin) / - mydata->clust_size; + mydata->root_cluster = + sect_to_clust(mydata, mydata->rootdir_sect); }
mydata->fatbufnum = -1; diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index d0468baf8f..9d2e0ed74c 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -502,8 +502,7 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, int ret;
if (clustnum > 0) - startsect = mydata->data_begin + - clustnum * mydata->clust_size; + startsect = clust_to_sect(mydata, clustnum); else startsect = mydata->rootdir_sect;
@@ -751,8 +750,7 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) __u32 startsect, sect_num, offset;
if (clustnum > 0) { - startsect = mydata->data_begin + - clustnum * mydata->clust_size; + startsect = clust_to_sect(mydata, clustnum); } else { startsect = mydata->rootdir_sect; } @@ -791,7 +789,7 @@ static dir_entry *empty_dentptr; static dir_entry *find_directory_entry(fsdata *mydata, int startsect, char *filename, dir_entry *retdent, __u32 start) { - __u32 curclust = (startsect - mydata->data_begin) / mydata->clust_size; + __u32 curclust = sect_to_clust(mydata, startsect);
debug("get_dentfromdir: %s\n", filename);
diff --git a/include/fat.h b/include/fat.h index 0f58939124..bdeda95e6d 100644 --- a/include/fat.h +++ b/include/fat.h @@ -177,12 +177,16 @@ typedef struct { __u32 root_cluster; /* First cluster of root dir for FAT32 */ } fsdata;
-/* TODO clean up places that are open-coding this: */ static inline u32 clust_to_sect(fsdata *fsdata, u32 clust) { return fsdata->data_begin + clust * fsdata->clust_size; }
+static inline u32 sect_to_clust(fsdata *fsdata, u32 sect) +{ + return (sect - fsdata->data_begin) / fsdata->clust_size; +} + int file_fat_detectfs(void); int fat_exists(const char *filename); int fat_size(const char *filename, loff_t *size);

On 9 September 2017 at 11:16, Rob Clark robdclark@gmail.com wrote:
Use the clust_to_sect() helper that was introduced earlier, and add an inverse sect_to_clust(), plus update the various spots that open-coded this conversion previously.
Signed-off-by: Rob Clark robdclark@gmail.com
fs/fat/fat.c | 8 +++----- fs/fat/fat_write.c | 8 +++----- include/fat.h | 6 +++++- 3 files changed, 11 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Sep 09, 2017 at 01:16:00PM -0400, Rob Clark wrote:
Use the clust_to_sect() helper that was introduced earlier, and add an inverse sect_to_clust(), plus update the various spots that open-coded this conversion previously.
Signed-off-by: Rob Clark robdclark@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (4)
-
Adam Ford
-
Rob Clark
-
Simon Glass
-
Tom Rini