[{"id":1762281,"web_url":"http://patchwork.ozlabs.org/comment/1762281/","msgid":"","list_archive_url":null,"date":"2017-09-03T15:16:07","subject":"Re: [U-Boot] [PATCH v2 4/8] fs: add fs_readdir()","submitter":{"id":70701,"url":"http://patchwork.ozlabs.org/api/people/70701/","name":"Lukasz Majewski","email":"lukma@denx.de"},"content":"On 09/02/2017 06:37 PM, Rob Clark wrote:\n> Needed to support efi file protocol. The fallback.efi loader wants\n> to be able to read the contents of the /EFI directory to find an OS\n> to boot.\n> \n> Modelled after POSIX opendir()/readdir()/closedir(). Unlike the other\n> fs APIs, this is stateful (ie. state is held in the FS_DIR \"directory\n> stream\"), to avoid re-traversing of the directory structure at each\n> step. The directory stream must be released with closedir() when it\n> is no longer needed.\n> \n\nReviewed-by: Łukasz Majewski \n\n> Signed-off-by: Rob Clark \n> ---\n> disk/part.c | 31 ++++++++++++--------\n> fs/fs.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> include/fs.h | 55 +++++++++++++++++++++++++++++++++++\n> include/part.h | 4 +++\n> 4 files changed, 169 insertions(+), 12 deletions(-)\n> \n> diff --git a/disk/part.c b/disk/part.c\n> index c67fdacc79..aa9183d696 100644\n> --- a/disk/part.c\n> +++ b/disk/part.c\n> @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int part,\n> \treturn -1;\n> }\n> \n> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info)\n> +{\n> +\tinfo->start = 0;\n> +\tinfo->size = dev_desc->lba;\n> +\tinfo->blksz = dev_desc->blksz;\n> +\tinfo->bootable = 0;\n> +\tstrcpy((char *)info->type, BOOT_PART_TYPE);\n> +\tstrcpy((char *)info->name, \"Whole Disk\");\n> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)\n> +\tinfo->uuid[0] = 0;\n> +#endif\n> +#ifdef CONFIG_PARTITION_TYPE_GUID\n> +\tinfo->type_guid[0] = 0;\n> +#endif\n> +\n> +\treturn 0;\n> +}\n> +\n> int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str,\n> \t\t\t struct blk_desc **dev_desc)\n> {\n> @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,\n> \n> \t\t(*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);\n> \n> -\t\tinfo->start = 0;\n> -\t\tinfo->size = (*dev_desc)->lba;\n> -\t\tinfo->blksz = (*dev_desc)->blksz;\n> -\t\tinfo->bootable = 0;\n> -\t\tstrcpy((char *)info->type, BOOT_PART_TYPE);\n> -\t\tstrcpy((char *)info->name, \"Whole Disk\");\n> -#if CONFIG_IS_ENABLED(PARTITION_UUIDS)\n> -\t\tinfo->uuid[0] = 0;\n> -#endif\n> -#ifdef CONFIG_PARTITION_TYPE_GUID\n> -\t\tinfo->type_guid[0] = 0;\n> -#endif\n> +\t\tpart_get_info_whole_disk(*dev_desc, info);\n> \n> \t\tret = 0;\n> \t\tgoto cleanup;\n> diff --git a/fs/fs.c b/fs/fs.c\n> index 13cd3626c6..441c880654 100644\n> --- a/fs/fs.c\n> +++ b/fs/fs.c\n> @@ -21,6 +21,7 @@\n> DECLARE_GLOBAL_DATA_PTR;\n> \n> static struct blk_desc *fs_dev_desc;\n> +static int fs_dev_part;\n> static disk_partition_t fs_partition;\n> static int fs_type = FS_TYPE_ANY;\n> \n> @@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)\n> \treturn -1;\n> }\n> \n> +static inline int fs_opendir_unsupported(const char *filename, FS_DIR **dirp)\n> +{\n> +\treturn -EACCES;\n> +}\n> +\n> struct fstype_info {\n> \tint fstype;\n> \tchar *name;\n> @@ -92,6 +98,9 @@ struct fstype_info {\n> \t\t loff_t len, loff_t *actwrite);\n> \tvoid (*close)(void);\n> \tint (*uuid)(char *uuid_str);\n> +\tint (*opendir)(const char *filename, FS_DIR **dirp);\n> +\tint (*readdir)(FS_DIR *dirp);\n> +\tvoid (*closedir)(FS_DIR *dirp);\n> };\n> \n> static struct fstype_info fstypes[] = {\n> @@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {\n> \t\t.write = fs_write_unsupported,\n> #endif\n> \t\t.uuid = fs_uuid_unsupported,\n> +\t\t.opendir = fs_opendir_unsupported,\n> \t},\n> #endif\n> #ifdef CONFIG_FS_EXT4\n> @@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {\n> \t\t.write = fs_write_unsupported,\n> #endif\n> \t\t.uuid = ext4fs_uuid,\n> +\t\t.opendir = fs_opendir_unsupported,\n> \t},\n> #endif\n> #ifdef CONFIG_SANDBOX\n> @@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {\n> \t\t.read = fs_read_sandbox,\n> \t\t.write = fs_write_sandbox,\n> \t\t.uuid = fs_uuid_unsupported,\n> +\t\t.opendir = fs_opendir_unsupported,\n> \t},\n> #endif\n> #ifdef CONFIG_CMD_UBIFS\n> @@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {\n> \t\t.read = ubifs_read,\n> \t\t.write = fs_write_unsupported,\n> \t\t.uuid = fs_uuid_unsupported,\n> +\t\t.opendir = fs_opendir_unsupported,\n> \t},\n> #endif\n> \t{\n> @@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {\n> \t\t.read = fs_read_unsupported,\n> \t\t.write = fs_write_unsupported,\n> \t\t.uuid = fs_uuid_unsupported,\n> +\t\t.opendir = fs_opendir_unsupported,\n> \t},\n> };\n> \n> @@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)\n> \n> \t\tif (!info->probe(fs_dev_desc, &fs_partition)) {\n> \t\t\tfs_type = info->fstype;\n> +\t\t\tfs_dev_part = part;\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t}\n> +\n> +\treturn -1;\n> +}\n> +\n> +/* set current blk device w/ blk_desc + partition # */\n> +int fs_set_blk_dev2(struct blk_desc *desc, int part)\n> +{\n> +\tstruct fstype_info *info;\n> +\tint ret, i;\n> +\n> +\tif (part >= 1)\n> +\t\tret = part_get_info(desc, part, &fs_partition);\n> +\telse\n> +\t\tret = part_get_info_whole_disk(desc, &fs_partition);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\tfs_dev_desc = desc;\n> +\n> +\tfor (i = 0, info = fstypes; i < ARRAY_SIZE(fstypes); i++, info++) {\n> +\t\tif (!info->probe(fs_dev_desc, &fs_partition)) {\n> +\t\t\tfs_type = info->fstype;\n> \t\t\treturn 0;\n> \t\t}\n> \t}\n> @@ -334,6 +373,58 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,\n> \treturn ret;\n> }\n> \n> +FS_DIR *fs_opendir(const char *filename)\n> +{\n> +\tstruct fstype_info *info = fs_get_info(fs_type);\n> +\tFS_DIR *dirp = NULL;\n> +\tint ret;\n> +\n> +\tret = info->opendir(filename, &dirp);\n> +\tfs_close();\n> +\tif (ret) {\n> +\t\terrno = -ret;\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\tdirp->desc = fs_dev_desc;\n> +\tdirp->part = fs_dev_part;\n> +\n> +\treturn dirp;\n> +}\n> +\n> +struct fs_dirent *fs_readdir(FS_DIR *dirp)\n> +{\n> +\tstruct fstype_info *info;\n> +\tint ret;\n> +\n> +\tfs_set_blk_dev2(dirp->desc, dirp->part);\n> +\tinfo = fs_get_info(fs_type);\n> +\n> +\tmemset(&dirp->dirent, 0, sizeof(dirp->dirent));\n> +\n> +\tret = info->readdir(dirp);\n> +\tfs_close();\n> +\tif (ret)\n> +\t\treturn NULL;\n> +\n> +\treturn &dirp->dirent;;\n> +}\n> +\n> +void fs_closedir(FS_DIR *dirp)\n> +{\n> +\tstruct fstype_info *info;\n> +\n> +\tif (!dirp)\n> +\t\treturn;\n> +\n> +\tfs_set_blk_dev2(dirp->desc, dirp->part);\n> +\tinfo = fs_get_info(fs_type);\n> +\n> +\tinfo->closedir(dirp);\n> +\tfs_close();\n> +}\n> +\n> +\n> int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],\n> \t\tint fstype)\n> {\n> diff --git a/include/fs.h b/include/fs.h\n> index 2f2aca8378..0a6a366078 100644\n> --- a/include/fs.h\n> +++ b/include/fs.h\n> @@ -26,6 +26,8 @@\n> */\n> int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);\n> \n> +int fs_set_blk_dev2(struct blk_desc *desc, int part);\n> +\n> /*\n> * Print the list of files on the partition previously set by fs_set_blk_dev(),\n> * in directory \"dirname\".\n> @@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,\n> int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,\n> \t loff_t *actwrite);\n> \n> +/* Add additional FS_DT_* as supported by additional filesystems:*/\n> +#define FS_DT_DIR 0x4 /* directory */\n> +#define FS_DT_REG 0x8 /* regular file */\n> +\n> +/*\n> + * A directory entry.\n> + */\n> +struct fs_dirent {\n> +\tunsigned type; /* one of FS_DT_* */\n> +\tloff_t size;\n> +\tchar name[256];\n> +};\n> +\n> +typedef struct _FS_DIR FS_DIR;\n> +\n> +/*\n> + * fs_opendir - Open a directory\n> + *\n> + * @filename: the path to directory to open\n> + * @return a pointer to the directory stream or NULL on error and errno\n> + * set appropriately\n> + */\n> +FS_DIR *fs_opendir(const char *filename);\n> +\n> +/*\n> + * fs_readdir - Read the next directory entry in the directory stream.\n> + *\n> + * @dirp: the directory stream\n> + * @return the next directory entry (only valid until next fs_readdir() or\n> + * fs_closedir() call, do not attempt to free()) or NULL if the end of\n> + * the directory is reached.\n> + */\n> +struct fs_dirent *fs_readdir(FS_DIR *dirp);\n> +\n> +/*\n> + * fs_closedir - close a directory stream\n> + *\n> + * @dirp: the directory stream\n> + */\n> +void fs_closedir(FS_DIR *dirp);\n> +\n> +/*\n> + * private to fs implementations, would be in fs.c but we need to let\n> + * implementations subclass:\n> + */\n> +\n> +struct _FS_DIR {\n> +\tstruct fs_dirent dirent;\n> +\t/* private to fs layer: */\n> +\tstruct blk_desc *desc;\n> +\tint part;\n> +};\n> +\n> /*\n> * Common implementation for various filesystem commands, optionally limited\n> * to a specific filesystem type via the fstype parameter.\n> diff --git a/include/part.h b/include/part.h\n> index 0cd803a933..48e8ff6d8a 100644\n> --- a/include/part.h\n> +++ b/include/part.h\n> @@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc **blk_devp);\n> \n> /* disk/part.c */\n> int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info);\n> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info);\n> void part_print(struct blk_desc *dev_desc);\n> void part_init(struct blk_desc *dev_desc);\n> void dev_print(struct blk_desc *dev_desc);\n> @@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int dev) { return NULL; }\n> \n> static inline int part_get_info(struct blk_desc *dev_desc, int part,\n> \t\t\t\tdisk_partition_t *info) { return -1; }\n> +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,\n> +\t\t\t\t\t disk_partition_t *info)\n> +{ return -1; }\n> static inline void part_print(struct blk_desc *dev_desc) {}\n> static inline void part_init(struct blk_desc *dev_desc) {}\n> static inline void dev_print(struct blk_desc *dev_desc) {}\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xlc4y3cx1z9t2y\n\tfor ;\n\tMon, 4 Sep 2017 01:16:18 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 32DDDC21EB4; Sun, 3 Sep 2017 15:16:15 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 0FCA2C21DBD;\n\tSun, 3 Sep 2017 15:16:12 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 23FACC21D79; Sun, 3 Sep 2017 15:16:11 +0000 (UTC)","from mail-out.m-online.net (mail-out.m-online.net [212.18.0.9])\n\tby lists.denx.de (Postfix) with ESMTPS id DD56CC21DBD\n\tfor ; Sun, 3 Sep 2017 15:16:10 +0000 (UTC)","from frontend01.mail.m-online.net (unknown [192.168.8.182])\n\tby mail-out.m-online.net (Postfix) with ESMTP id 3xlc4p37mlz1r2fc;\n\tSun, 3 Sep 2017 17:16:10 +0200 (CEST)","from localhost (dynscan1.mnet-online.de [192.168.6.70])\n\tby mail.m-online.net (Postfix) with ESMTP id 3xlc4p1lP4z3hjkw;\n\tSun, 3 Sep 2017 17:16:10 +0200 (CEST)","from mail.mnet-online.de ([192.168.8.182])\n\tby localhost (dynscan1.mail.m-online.net [192.168.6.70]) (amavisd-new,\n\tport 10024)\n\twith ESMTP id kqcrzHrNX47t; Sun, 3 Sep 2017 17:16:08 +0200 (CEST)","from [192.168.2.222] (89-77-92-62.dynamic.chello.pl [89.77.92.62])\n\t(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mail.mnet-online.de (Postfix) with ESMTPSA;\n\tSun, 3 Sep 2017 17:16:08 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.7 required=5.0 tests=RCVD_IN_DNSWL_LOW,\n\tRCVD_IN_MSPIKE_H3,\n\tRCVD_IN_MSPIKE_WL autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","X-Virus-Scanned":"amavisd-new at mnet-online.de","X-Auth-Info":"/cvJW6B2eqFyUFHLqcuXQ3yQHv3HR0b/aW/fjR5RwaI=","To":"Rob Clark ,\n\tU-Boot Mailing List ","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-5-robdclark@gmail.com>","From":"=?utf-8?q?=C5=81ukasz_Majewski?= ","Organization":"DENX","Message-ID":"","Date":"Sun, 3 Sep 2017 17:16:07 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170902163806.27265-5-robdclark@gmail.com>","Content-Language":"en-US","Cc":"Alison Chaiken ,\n\tSteve Rae ,\n\tZhikang Zhang , Petr Kulhavy ","Subject":"Re: [U-Boot] [PATCH v2 4/8] fs: add fs_readdir()","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1763084,"web_url":"http://patchwork.ozlabs.org/comment/1763084/","msgid":"","list_archive_url":null,"date":"2017-09-05T08:56:22","subject":"Re: [U-Boot] [PATCH v2 4/8] fs: add fs_readdir()","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"Hi Rob,\n\nOn 3 September 2017 at 23:16, Łukasz Majewski wrote:\n> On 09/02/2017 06:37 PM, Rob Clark wrote:\n>>\n>> Needed to support efi file protocol. The fallback.efi loader wants\n>> to be able to read the contents of the /EFI directory to find an OS\n>> to boot.\n>>\n>> Modelled after POSIX opendir()/readdir()/closedir(). Unlike the other\n>> fs APIs, this is stateful (ie. state is held in the FS_DIR \"directory\n>> stream\"), to avoid re-traversing of the directory structure at each\n>> step. The directory stream must be released with closedir() when it\n>> is no longer needed.\n>>\n>\n> Reviewed-by: Łukasz Majewski \n>\n>\n>> Signed-off-by: Rob Clark \n>> ---\n>> disk/part.c | 31 ++++++++++++--------\n>> fs/fs.c | 91\n>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>> include/fs.h | 55 +++++++++++++++++++++++++++++++++++\n>> include/part.h | 4 +++\n>> 4 files changed, 169 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/disk/part.c b/disk/part.c\n>> index c67fdacc79..aa9183d696 100644\n>> --- a/disk/part.c\n>> +++ b/disk/part.c\n>> @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int\n>> part,\n>> return -1;\n>> }\n>> +int part_get_info_whole_disk(struct blk_desc *dev_desc,\n>> disk_partition_t *info)\n>> +{\n>> + info->start = 0;\n>> + info->size = dev_desc->lba;\n>> + info->blksz = dev_desc->blksz;\n>> + info->bootable = 0;\n>> + strcpy((char *)info->type, BOOT_PART_TYPE);\n>> + strcpy((char *)info->name, \"Whole Disk\");\n>> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)\n\nCan you use if() instead of #if for this one? And below. It helps to\nreduce the number of code paths at build-time.\n\n>> + info->uuid[0] = 0;\n>> +#endif\n>> +#ifdef CONFIG_PARTITION_TYPE_GUID\n\nHere too. And below.\n\n>> + info->type_guid[0] = 0;\n>> +#endif\n>> +\n>> + return 0;\n>> +}\n>> +\n>> int blk_get_device_by_str(const char *ifname, const char\n>> *dev_hwpart_str,\n>> struct blk_desc **dev_desc)\n>> {\n>> @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const\n>> char *dev_part_str,\n>> (*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);\n>> - info->start = 0;\n>> - info->size = (*dev_desc)->lba;\n>> - info->blksz = (*dev_desc)->blksz;\n>> - info->bootable = 0;\n>> - strcpy((char *)info->type, BOOT_PART_TYPE);\n>> - strcpy((char *)info->name, \"Whole Disk\");\n>> -#if CONFIG_IS_ENABLED(PARTITION_UUIDS)\n>> - info->uuid[0] = 0;\n>> -#endif\n>> -#ifdef CONFIG_PARTITION_TYPE_GUID\n>> - info->type_guid[0] = 0;\n>> -#endif\n>> + part_get_info_whole_disk(*dev_desc, info);\n>> ret = 0;\n>> goto cleanup;\n>> diff --git a/fs/fs.c b/fs/fs.c\n>> index 13cd3626c6..441c880654 100644\n>> --- a/fs/fs.c\n>> +++ b/fs/fs.c\n>> @@ -21,6 +21,7 @@\n>> DECLARE_GLOBAL_DATA_PTR;\n>> static struct blk_desc *fs_dev_desc;\n>> +static int fs_dev_part;\n>> static disk_partition_t fs_partition;\n>> static int fs_type = FS_TYPE_ANY;\n>> @@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)\n>> return -1;\n>> }\n>> +static inline int fs_opendir_unsupported(const char *filename, FS_DIR\n>> **dirp)\n>> +{\n>> + return -EACCES;\n>> +}\n>> +\n>> struct fstype_info {\n>> int fstype;\n>> char *name;\n>> @@ -92,6 +98,9 @@ struct fstype_info {\n>> loff_t len, loff_t *actwrite);\n>> void (*close)(void);\n>> int (*uuid)(char *uuid_str);\n>> + int (*opendir)(const char *filename, FS_DIR **dirp);\n>> + int (*readdir)(FS_DIR *dirp);\n>> + void (*closedir)(FS_DIR *dirp);\n\nPlease comment these. Also can you use struct instead of typedef?\n\n>> };\n>> static struct fstype_info fstypes[] = {\n>> @@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {\n>> .write = fs_write_unsupported,\n>> #endif\n>> .uuid = fs_uuid_unsupported,\n>> + .opendir = fs_opendir_unsupported,\n>> },\n>> #endif\n>> #ifdef CONFIG_FS_EXT4\n>> @@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {\n>> .write = fs_write_unsupported,\n>> #endif\n>> .uuid = ext4fs_uuid,\n>> + .opendir = fs_opendir_unsupported,\n>> },\n>> #endif\n>> #ifdef CONFIG_SANDBOX\n>> @@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {\n>> .read = fs_read_sandbox,\n>> .write = fs_write_sandbox,\n>> .uuid = fs_uuid_unsupported,\n>> + .opendir = fs_opendir_unsupported,\n>> },\n>> #endif\n>> #ifdef CONFIG_CMD_UBIFS\n>> @@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {\n>> .read = ubifs_read,\n>> .write = fs_write_unsupported,\n>> .uuid = fs_uuid_unsupported,\n>> + .opendir = fs_opendir_unsupported,\n>> },\n>> #endif\n>> {\n>> @@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {\n>> .read = fs_read_unsupported,\n>> .write = fs_write_unsupported,\n>> .uuid = fs_uuid_unsupported,\n>> + .opendir = fs_opendir_unsupported,\n>> },\n>> };\n>> @@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char\n>> *dev_part_str, int fstype)\n>> if (!info->probe(fs_dev_desc, &fs_partition)) {\n>> fs_type = info->fstype;\n>> + fs_dev_part = part;\n>> + return 0;\n>> + }\n>> + }\n>> +\n>> + return -1;\n>> +}\n>> +\n>> +/* set current blk device w/ blk_desc + partition # */\n>> +int fs_set_blk_dev2(struct blk_desc *desc, int part)\n\nPlease add a full function comment in the header file. See\nfs_set_blk() which has a comment.\n\nAlso '2' is a bad name. How about a _with_part suffix, or something like that?\n\n[...]\n\n>> diff --git a/include/fs.h b/include/fs.h\n>> index 2f2aca8378..0a6a366078 100644\n>> --- a/include/fs.h\n>> +++ b/include/fs.h\n>> @@ -26,6 +26,8 @@\n>> */\n>> int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int\n>> fstype);\n>> +int fs_set_blk_dev2(struct blk_desc *desc, int part);\n\nComment goes above this.\n\n>> +\n>> /*\n>> * Print the list of files on the partition previously set by\n>> fs_set_blk_dev(),\n>> * in directory \"dirname\".\n>> @@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t\n>> offset, loff_t len,\n>> int fs_write(const char *filename, ulong addr, loff_t offset, loff_t\n>> len,\n>> loff_t *actwrite);\n>> +/* Add additional FS_DT_* as supported by additional filesystems:*/\n>> +#define FS_DT_DIR 0x4 /* directory */\n>> +#define FS_DT_REG 0x8 /* regular file */\n>> +\n>> +/*\n>> + * A directory entry.\n>> + */\n>> +struct fs_dirent {\n>> + unsigned type; /* one of FS_DT_* */\n\nIs that a mask (so both can be set) or something else? If it is a mask\nplease say so.\n\n>> + loff_t size;\n\ncomment for this. Is it size of the file in bytes?\n\n>> + char name[256];\n>> +};\n>> +\n>> +typedef struct _FS_DIR FS_DIR;\n>> +\n\nPlease drop the typedef as it doesn't seem necessary\n\n>> +/*\n>> + * fs_opendir - Open a directory\n>> + *\n>> + * @filename: the path to directory to open\n>> + * @return a pointer to the directory stream or NULL on error and errno\n>> + * set appropriately\n>> + */\n>> +FS_DIR *fs_opendir(const char *filename);\n>> +\n>> +/*\n>> + * fs_readdir - Read the next directory entry in the directory stream.\n>> + *\n>> + * @dirp: the directory stream\n>> + * @return the next directory entry (only valid until next fs_readdir()\n>> or\n>> + * fs_closedir() call, do not attempt to free()) or NULL if the end of\n>> + * the directory is reached.\n>> + */\n>> +struct fs_dirent *fs_readdir(FS_DIR *dirp);\n>> +\n>> +/*\n>> + * fs_closedir - close a directory stream\n>> + *\n>> + * @dirp: the directory stream\n>> + */\n>> +void fs_closedir(FS_DIR *dirp);\n>> +\n>> +/*\n>> + * private to fs implementations, would be in fs.c but we need to let\n>> + * implementations subclass:\n>> + */\n>> +\n>> +struct _FS_DIR {\n\nLower case for struct names. Also please add struct member comments.\n\n>> + struct fs_dirent dirent;\n>> + /* private to fs layer: */\n>> + struct blk_desc *desc;\n>> + int part;\n>> +};\n>> +\n>> /*\n>> * Common implementation for various filesystem commands, optionally\n>> limited\n>> * to a specific filesystem type via the fstype parameter.\n>> diff --git a/include/part.h b/include/part.h\n>> index 0cd803a933..48e8ff6d8a 100644\n>> --- a/include/part.h\n>> +++ b/include/part.h\n>> @@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc\n>> **blk_devp);\n>> /* disk/part.c */\n>> int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t\n>> *info);\n>> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t\n>> *info);\n\nPlease fully comment new functions here.\n\n>> void part_print(struct blk_desc *dev_desc);\n>> void part_init(struct blk_desc *dev_desc);\n>> void dev_print(struct blk_desc *dev_desc);\n>> @@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int\n>> dev) { return NULL; }\n>> static inline int part_get_info(struct blk_desc *dev_desc, int part,\n>> disk_partition_t *info) { return -1; }\n>> +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,\n>> + disk_partition_t *info)\n>> +{ return -1; }\n>> static inline void part_print(struct blk_desc *dev_desc) {}\n>> static inline void part_init(struct blk_desc *dev_desc) {}\n>> static inline void dev_print(struct blk_desc *dev_desc) {}\n>>\n\nRegards,\nSimon","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"HpBXIiMS\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"BzQ+prOE\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmgc91kh3z9s72\n\tfor ;\n\tTue, 5 Sep 2017 18:58:32 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid F336BC21DD7; Tue, 5 Sep 2017 08:57:49 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 377A0C21EF3;\n\tTue, 5 Sep 2017 08:57:22 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 2CB86C21E1C; Tue, 5 Sep 2017 08:56:48 +0000 (UTC)","from mail-qk0-f180.google.com (mail-qk0-f180.google.com\n\t[209.85.220.180])\n\tby lists.denx.de (Postfix) with ESMTPS id 9C2BDC21DC5\n\tfor ; Tue, 5 Sep 2017 08:56:44 +0000 (UTC)","by mail-qk0-f180.google.com with SMTP id r141so9516306qke.2\n\tfor ; Tue, 05 Sep 2017 01:56:44 -0700 (PDT)","by 10.200.28.108 with HTTP; Tue, 5 Sep 2017 01:56:22 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-transfer-encoding;\n\tbh=xzyn6/SASJsh43x62TJKkDo0Y9rBptXXAugf02oZl0M=;\n\tb=HpBXIiMSNnmGUk5VbOLJ0xIJM4wFvgtBEbeo00hWISBuALRhFHzYXSVDPK+6H+8j/O\n\thJ2TWhTF39H4l6473qqjArm3xmV3qRn3Yhjq+YVPIDhG3IIP6AAd2XLvqjaGuMN492lp\n\tJ3xi7yDebXNX+O6pt48KnFHPucGj8c0w1u/2uYdYlFejhKv1/wlqPOMriqRcqgSR2ntr\n\tP09C8Ob8mGo/TIePGWjoQH1GPtJjK0FPc1sX7+wjJ8OYI/yfu+YvuIljDF/Z6fdj2/e+\n\t1ovo9wHU8264z83O/xBOQN4n15bTC3K5G3sHpjts3EEpS+hKW43hQlZGf3jX+0BVt845\n\tSnHA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-transfer-encoding;\n\tbh=xzyn6/SASJsh43x62TJKkDo0Y9rBptXXAugf02oZl0M=;\n\tb=BzQ+prOEijE8ssDtxnDAipdimFCLfuFd4IxAm4WqlCZ0eBfclt0XYBw4VGfHN4JkfK\n\tgaBq1JRlc9ez5TUDgPPQrf9noT7qNXjVTw0bunXCMzMWDF1Kk+tUYgfYmGd93e5yM0ZS\n\tq8u7Iqt0G5On08c4pgd5SONg9tdVpdz19TF0U="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=xzyn6/SASJsh43x62TJKkDo0Y9rBptXXAugf02oZl0M=;\n\tb=lvNnfwzeByH01dkG1BWrL3p82aTazl2g/Xfvgxl/hHhAL65SweJpXxTDlKD0lbCWse\n\tOsldbj4OW1WPbRZiiCM35m0P5K0Sp9dKg3rNgAtAmMKhDGkXjldau4XNyOF/x7LmaRcj\n\tMoDzjBd4dENOgUjbhynLciTVwlXH6lYptH7XhkBAYSzro2MaatyGTCamDA/yyN70OAZP\n\tZSUPIfhZdU4QyE/yW7XvShGbHTHkp2lfHkaW0qJyGjSw4rjyv8bb4W/kjWG7dTYh9Rwi\n\tTPkQGwkQBbYEtUSlWih8D7SZjvo63K3+S1058rJpSsXuMz2Jhk9jGczKOtsKFAHgKF2b\n\tqccg==","X-Gm-Message-State":"AHPjjUg5pnn+dsnu0W3wkp64PO1UqoVaf8s/CduHFqBNCWYX8yCueDK9\n\tn5QSrEDZwXT52A1Vx5Ha+HWfOlwY+37M","X-Google-Smtp-Source":"ADKCNb6dzilBYHXjWSP/A9qrSvyzcEinmspXl9HAy4ID2oAp7Marmn20vu3ad+N4WrEQP6vAmVV6Us62L20YJoBdP0Y=","X-Received":"by 10.55.92.7 with SMTP id q7mr4012962qkb.236.1504601803190; Tue,\n\t05 Sep 2017 01:56:43 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-5-robdclark@gmail.com>\n\t","From":"Simon Glass ","Date":"Tue, 5 Sep 2017 16:56:22 +0800","X-Google-Sender-Auth":"aYXie69RqinWJwtM6ZWfli3Ewlk","Message-ID":"","To":"=?utf-8?q?=C5=81ukasz_Majewski?= ","Cc":"Petr Kulhavy , Zhikang Zhang ,\n\tAlison Chaiken ,\n\tU-Boot Mailing List ,\n\tSteve Rae ","Subject":"Re: [U-Boot] [PATCH v2 4/8] fs: add fs_readdir()","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1763204,"web_url":"http://patchwork.ozlabs.org/comment/1763204/","msgid":"","list_archive_url":null,"date":"2017-09-05T10:48:59","subject":"Re: [U-Boot] [PATCH v2 4/8] fs: add fs_readdir()","submitter":{"id":18760,"url":"http://patchwork.ozlabs.org/api/people/18760/","name":"Rob Clark","email":"robdclark@gmail.com"},"content":"On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass wrote:\n> Hi Rob,\n>\n> On 3 September 2017 at 23:16, Łukasz Majewski wrote:\n>> On 09/02/2017 06:37 PM, Rob Clark wrote:\n>>>\n>>> Needed to support efi file protocol. The fallback.efi loader wants\n>>> to be able to read the contents of the /EFI directory to find an OS\n>>> to boot.\n>>>\n>>> Modelled after POSIX opendir()/readdir()/closedir(). Unlike the other\n>>> fs APIs, this is stateful (ie. state is held in the FS_DIR \"directory\n>>> stream\"), to avoid re-traversing of the directory structure at each\n>>> step. The directory stream must be released with closedir() when it\n>>> is no longer needed.\n>>>\n>>\n>> Reviewed-by: Łukasz Majewski \n>>\n>>\n>>> Signed-off-by: Rob Clark \n>>> ---\n>>> disk/part.c | 31 ++++++++++++--------\n>>> fs/fs.c | 91\n>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>> include/fs.h | 55 +++++++++++++++++++++++++++++++++++\n>>> include/part.h | 4 +++\n>>> 4 files changed, 169 insertions(+), 12 deletions(-)\n>>>\n>>> diff --git a/disk/part.c b/disk/part.c\n>>> index c67fdacc79..aa9183d696 100644\n>>> --- a/disk/part.c\n>>> +++ b/disk/part.c\n>>> @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int\n>>> part,\n>>> return -1;\n>>> }\n>>> +int part_get_info_whole_disk(struct blk_desc *dev_desc,\n>>> disk_partition_t *info)\n>>> +{\n>>> + info->start = 0;\n>>> + info->size = dev_desc->lba;\n>>> + info->blksz = dev_desc->blksz;\n>>> + info->bootable = 0;\n>>> + strcpy((char *)info->type, BOOT_PART_TYPE);\n>>> + strcpy((char *)info->name, \"Whole Disk\");\n>>> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)\n>\n> Can you use if() instead of #if for this one? And below. It helps to\n> reduce the number of code paths at build-time.\n\nI don't think so, at least not without dropping the corresponding\n#ifdef in disk_partition_t\n\n>>> + info->uuid[0] = 0;\n>>> +#endif\n>>> +#ifdef CONFIG_PARTITION_TYPE_GUID\n>\n> Here too. And below.\n\nsame thing\n\n>>> + info->type_guid[0] = 0;\n>>> +#endif\n>>> +\n>>> + return 0;\n>>> +}\n>>> +\n>>> int blk_get_device_by_str(const char *ifname, const char\n>>> *dev_hwpart_str,\n>>> struct blk_desc **dev_desc)\n>>> {\n>>> @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const\n>>> char *dev_part_str,\n>>> (*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);\n>>> - info->start = 0;\n>>> - info->size = (*dev_desc)->lba;\n>>> - info->blksz = (*dev_desc)->blksz;\n>>> - info->bootable = 0;\n>>> - strcpy((char *)info->type, BOOT_PART_TYPE);\n>>> - strcpy((char *)info->name, \"Whole Disk\");\n>>> -#if CONFIG_IS_ENABLED(PARTITION_UUIDS)\n>>> - info->uuid[0] = 0;\n>>> -#endif\n>>> -#ifdef CONFIG_PARTITION_TYPE_GUID\n>>> - info->type_guid[0] = 0;\n>>> -#endif\n>>> + part_get_info_whole_disk(*dev_desc, info);\n>>> ret = 0;\n>>> goto cleanup;\n>>> diff --git a/fs/fs.c b/fs/fs.c\n>>> index 13cd3626c6..441c880654 100644\n>>> --- a/fs/fs.c\n>>> +++ b/fs/fs.c\n>>> @@ -21,6 +21,7 @@\n>>> DECLARE_GLOBAL_DATA_PTR;\n>>> static struct blk_desc *fs_dev_desc;\n>>> +static int fs_dev_part;\n>>> static disk_partition_t fs_partition;\n>>> static int fs_type = FS_TYPE_ANY;\n>>> @@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)\n>>> return -1;\n>>> }\n>>> +static inline int fs_opendir_unsupported(const char *filename, FS_DIR\n>>> **dirp)\n>>> +{\n>>> + return -EACCES;\n>>> +}\n>>> +\n>>> struct fstype_info {\n>>> int fstype;\n>>> char *name;\n>>> @@ -92,6 +98,9 @@ struct fstype_info {\n>>> loff_t len, loff_t *actwrite);\n>>> void (*close)(void);\n>>> int (*uuid)(char *uuid_str);\n>>> + int (*opendir)(const char *filename, FS_DIR **dirp);\n>>> + int (*readdir)(FS_DIR *dirp);\n>>> + void (*closedir)(FS_DIR *dirp);\n>\n> Please comment these. Also can you use struct instead of typedef?\n\ntypedef-struct-caps here was based on how posix readdir() works. I\nguess we can deviate, it isn't 100% clone of readdir().. but I figured\nit was more clear to be more similar to posix.\n\n>>> };\n>>> static struct fstype_info fstypes[] = {\n>>> @@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {\n>>> .write = fs_write_unsupported,\n>>> #endif\n>>> .uuid = fs_uuid_unsupported,\n>>> + .opendir = fs_opendir_unsupported,\n>>> },\n>>> #endif\n>>> #ifdef CONFIG_FS_EXT4\n>>> @@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {\n>>> .write = fs_write_unsupported,\n>>> #endif\n>>> .uuid = ext4fs_uuid,\n>>> + .opendir = fs_opendir_unsupported,\n>>> },\n>>> #endif\n>>> #ifdef CONFIG_SANDBOX\n>>> @@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {\n>>> .read = fs_read_sandbox,\n>>> .write = fs_write_sandbox,\n>>> .uuid = fs_uuid_unsupported,\n>>> + .opendir = fs_opendir_unsupported,\n>>> },\n>>> #endif\n>>> #ifdef CONFIG_CMD_UBIFS\n>>> @@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {\n>>> .read = ubifs_read,\n>>> .write = fs_write_unsupported,\n>>> .uuid = fs_uuid_unsupported,\n>>> + .opendir = fs_opendir_unsupported,\n>>> },\n>>> #endif\n>>> {\n>>> @@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {\n>>> .read = fs_read_unsupported,\n>>> .write = fs_write_unsupported,\n>>> .uuid = fs_uuid_unsupported,\n>>> + .opendir = fs_opendir_unsupported,\n>>> },\n>>> };\n>>> @@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char\n>>> *dev_part_str, int fstype)\n>>> if (!info->probe(fs_dev_desc, &fs_partition)) {\n>>> fs_type = info->fstype;\n>>> + fs_dev_part = part;\n>>> + return 0;\n>>> + }\n>>> + }\n>>> +\n>>> + return -1;\n>>> +}\n>>> +\n>>> +/* set current blk device w/ blk_desc + partition # */\n>>> +int fs_set_blk_dev2(struct blk_desc *desc, int part)\n>\n> Please add a full function comment in the header file. See\n> fs_set_blk() which has a comment.\n>\n> Also '2' is a bad name. How about a _with_part suffix, or something like that?\n\nyeah, \"2\" was me running out of creativity.. _with_part sounds reasonable.\n\n> [...]\n>\n>>> diff --git a/include/fs.h b/include/fs.h\n>>> index 2f2aca8378..0a6a366078 100644\n>>> --- a/include/fs.h\n>>> +++ b/include/fs.h\n>>> @@ -26,6 +26,8 @@\n>>> */\n>>> int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int\n>>> fstype);\n>>> +int fs_set_blk_dev2(struct blk_desc *desc, int part);\n>\n> Comment goes above this.\n>\n>>> +\n>>> /*\n>>> * Print the list of files on the partition previously set by\n>>> fs_set_blk_dev(),\n>>> * in directory \"dirname\".\n>>> @@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t\n>>> offset, loff_t len,\n>>> int fs_write(const char *filename, ulong addr, loff_t offset, loff_t\n>>> len,\n>>> loff_t *actwrite);\n>>> +/* Add additional FS_DT_* as supported by additional filesystems:*/\n>>> +#define FS_DT_DIR 0x4 /* directory */\n>>> +#define FS_DT_REG 0x8 /* regular file */\n>>> +\n>>> +/*\n>>> + * A directory entry.\n>>> + */\n>>> +struct fs_dirent {\n>>> + unsigned type; /* one of FS_DT_* */\n>\n> Is that a mask (so both can be set) or something else? If it is a mask\n> please say so.\n\nIt is not a mask, despite how the corresponding readdir() DT_* are\ndefined. They match DT_* constants, or rather the subset of them that\nmake sense (ie. we don't have pipes or sockets.. maybe at some point\nwhen someone implements this for ext4 it would be worth adding\nFS_DT_LNK for symlinks)\n\nBR,\n-R\n\n\n>>> + loff_t size;\n>\n> comment for this. Is it size of the file in bytes?\n>\n>>> + char name[256];\n>>> +};\n>>> +\n>>> +typedef struct _FS_DIR FS_DIR;\n>>> +\n>\n> Please drop the typedef as it doesn't seem necessary\n>\n>>> +/*\n>>> + * fs_opendir - Open a directory\n>>> + *\n>>> + * @filename: the path to directory to open\n>>> + * @return a pointer to the directory stream or NULL on error and errno\n>>> + * set appropriately\n>>> + */\n>>> +FS_DIR *fs_opendir(const char *filename);\n>>> +\n>>> +/*\n>>> + * fs_readdir - Read the next directory entry in the directory stream.\n>>> + *\n>>> + * @dirp: the directory stream\n>>> + * @return the next directory entry (only valid until next fs_readdir()\n>>> or\n>>> + * fs_closedir() call, do not attempt to free()) or NULL if the end of\n>>> + * the directory is reached.\n>>> + */\n>>> +struct fs_dirent *fs_readdir(FS_DIR *dirp);\n>>> +\n>>> +/*\n>>> + * fs_closedir - close a directory stream\n>>> + *\n>>> + * @dirp: the directory stream\n>>> + */\n>>> +void fs_closedir(FS_DIR *dirp);\n>>> +\n>>> +/*\n>>> + * private to fs implementations, would be in fs.c but we need to let\n>>> + * implementations subclass:\n>>> + */\n>>> +\n>>> +struct _FS_DIR {\n>\n> Lower case for struct names. Also please add struct member comments.\n>\n>>> + struct fs_dirent dirent;\n>>> + /* private to fs layer: */\n>>> + struct blk_desc *desc;\n>>> + int part;\n>>> +};\n>>> +\n>>> /*\n>>> * Common implementation for various filesystem commands, optionally\n>>> limited\n>>> * to a specific filesystem type via the fstype parameter.\n>>> diff --git a/include/part.h b/include/part.h\n>>> index 0cd803a933..48e8ff6d8a 100644\n>>> --- a/include/part.h\n>>> +++ b/include/part.h\n>>> @@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc\n>>> **blk_devp);\n>>> /* disk/part.c */\n>>> int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t\n>>> *info);\n>>> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t\n>>> *info);\n>\n> Please fully comment new functions here.\n>\n>>> void part_print(struct blk_desc *dev_desc);\n>>> void part_init(struct blk_desc *dev_desc);\n>>> void dev_print(struct blk_desc *dev_desc);\n>>> @@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int\n>>> dev) { return NULL; }\n>>> static inline int part_get_info(struct blk_desc *dev_desc, int part,\n>>> disk_partition_t *info) { return -1; }\n>>> +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,\n>>> + disk_partition_t *info)\n>>> +{ return -1; }\n>>> static inline void part_print(struct blk_desc *dev_desc) {}\n>>> static inline void part_init(struct blk_desc *dev_desc) {}\n>>> static inline void dev_print(struct blk_desc *dev_desc) {}\n>>>\n>\n> Regards,\n> Simon","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Ss3jvnD9\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmk3p0J0bz9sP3\n\tfor ;\n\tTue, 5 Sep 2017 20:49:09 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid A6EDDC21F29; Tue, 5 Sep 2017 10:49:06 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 1CEEFC21D7E;\n\tTue, 5 Sep 2017 10:49:02 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid D32F4C21D7E; Tue, 5 Sep 2017 10:49:00 +0000 (UTC)","from mail-lf0-f68.google.com (mail-lf0-f68.google.com\n\t[209.85.215.68])\n\tby lists.denx.de (Postfix) with ESMTPS id 5C739C21C97\n\tfor ; Tue, 5 Sep 2017 10:49:00 +0000 (UTC)","by mail-lf0-f68.google.com with SMTP id q132so816564lfe.4\n\tfor ; Tue, 05 Sep 2017 03:49:00 -0700 (PDT)","by 10.46.82.27 with HTTP; Tue, 5 Sep 2017 03:48:59 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID\n\tautolearn=unavailable autolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=uKatTL1sCrSEzcIs3gLospUsH7gObBuV/6i5xCJEsO8=;\n\tb=Ss3jvnD9FCDt6uJlC+KHuAgSecIMZg5hVzOVSuVPn5IgWhTYOSr90eCGx/51z68HT9\n\tHTVfreRH7Alglv8L17/hwmy6DuPA1XTMD7yAruuimJVt7NYb3LRR1e/xd3SvzPYe4Mje\n\tgaqCyI9d9/sdsa40FyppBY1Vj5WlW8JGF1aDMsD5lO6h7NdoIoFCNwo7amaTmgOMeaEU\n\t4EwE3XS05I3BzU4+G57NP7cG7+U4HPVhL/sWa7M3ulARHCFeP/Dovo1k4xOfurhGn5o0\n\txc88r772WYnFQlEgDSNTkrxWoOwmmQO0qN7x3DAlsfcu2EMoyMIauJoSX5QGNgOJhr1A\n\tdZoQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=uKatTL1sCrSEzcIs3gLospUsH7gObBuV/6i5xCJEsO8=;\n\tb=DW35zE74XCIg1ojAw7WQGnqorOcyTaroFKE08ywO6BkRU0BCfjIEuuS7k3GIUKB0Gd\n\t7cEunD7ZZUOZHT0jlRbspoxFSfMyounXZQkqPFEK6e9pRAQiWRer6YvsFZb57Ke+/V9b\n\t7NfFmDwyYNI3QzI09CSQyF2OrzDrIkKEfr5S2m9AxpRUX7jOfRcLQx7OWHWR32yiWWH0\n\tJv8b/GZVo9a6Hef49hs+/1hkTcOYVo4vjP6m2NRZamhz/N8D9Mct+WZj5S7lAFVuhzCJ\n\tiIlJUOJKYBQHiWXxe3vLfRYJN0BH68xDVtBnO1TXNU4w2j6sB8WLaP3MERFnWOo1XkUu\n\tyFWQ==","X-Gm-Message-State":"AHPjjUgD9FzE3ZVH8dIU1ecJROREzDhNj72Ycu6LORtZryFY4o4ilVnD\n\t0JRTF4qHrZjO8GvAcHyE4QHOpIxWre/I","X-Google-Smtp-Source":"ADKCNb4vnZexxuTGTfk4F/oyn8HqJes73N1jBOiv1GGGgb6Hvdj9007AbnxLTuD/Q4L6URw1o9IFRRMcKsXaWBqQ9T8=","X-Received":"by 10.46.25.136 with SMTP id 8mr1315288ljz.122.1504608539757;\n\tTue, 05 Sep 2017 03:48:59 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170902163806.27265-1-robdclark@gmail.com>\n\t<20170902163806.27265-5-robdclark@gmail.com>\n\t\n\t","From":"Rob Clark ","Date":"Tue, 5 Sep 2017 06:48:59 -0400","Message-ID":"","To":"Simon Glass ","Cc":"Petr Kulhavy , Zhikang Zhang ,\n\tAlison Chaiken ,\n\tU-Boot Mailing List ,\n\tSteve Rae ","Subject":"Re: [U-Boot] [PATCH v2 4/8] fs: add fs_readdir()","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]