[U-Boot] [PATCH V4 1/3] FAT: remove cur_part_nr

[U-Boot] [PATCH V4 1/3] FAT: remove cur_part_nr

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

A future patch will implement the more standard filesystem API
fat_set_blk_dev(). This API has no way to know which partition number
the partition represents. Equally, future DM rework will make the
concept of partition number harder to pass around.

So, simply remove cur_part_nr from fat.c; its only use is in a
diagnostic printf, and the context where it's printed should make it
obvious which partition is referred to anyway (since the partition ID
would come from the user command-line that caused it).

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v4: New patch: Dropped addition of part number of disk_partition_t, and
replaced it with this patch.
---
 fs/fat/fat.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 80156c8..b038baf 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -46,7 +46,6 @@ static void downcase(char *str)
 }
 
 static block_dev_desc_t *cur_dev;
-static unsigned int cur_part_nr;
 static disk_partition_t cur_part_info;
 
 #define DOS_BOOT_MAGIC_OFFSET	0x1fe
@@ -77,10 +76,8 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
      defined(CONFIG_SYSTEMACE) )
 
 	/* Read the partition table, if present */
-	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
+	if (!get_partition_info(dev_desc, part_no, &cur_part_info))
 		cur_dev = dev_desc;
-		cur_part_nr = part_no;
-	}
 #endif
 
 	/* Otherwise it might be a superfloppy (whole-disk FAT filesystem) */
@@ -92,7 +89,6 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
 		}
 
 		cur_dev = dev_desc;
-		cur_part_nr = 1;
 		cur_part_info.start = 0;
 		cur_part_info.size = dev_desc->lba;
 		cur_part_info.blksz = dev_desc->blksz;
@@ -1235,8 +1231,7 @@ int file_fat_detectfs(void)
 	vol_label[11] = '\0';
 	volinfo.fs_type[5] = '\0';
 
-	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
-		volinfo.fs_type, vol_label);
+	printf("Filesystem: %s \"%s\"\n", volinfo.fs_type, vol_label);
 
 	return 0;
 }
-- 
1.7.0.4

[U-Boot] [PATCH V4 2/3] FAT: initialize all fields in cur_part_info, simplify init

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

cur_part_info.{name,type} are strings. So, we don't need to memset()
the entire thing, just put the NULL-termination in the first byte.

Add missing initialization of the bootable and uuid fields.

None of these fields are actually used by fat.c. However, since it
stores the entire disk_partition_t, we should make sure that all fields
are valid.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
---
v4: No change (just rebased).
v3: New patch.
---
 fs/fat/fat.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index b038baf..425aec7 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -92,8 +92,12 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
 		cur_part_info.start = 0;
 		cur_part_info.size = dev_desc->lba;
 		cur_part_info.blksz = dev_desc->blksz;
-		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
-		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
+		cur_part_info.name[0] = 0;
+		cur_part_info.type[0] = 0;
+		cur_part_info.bootable = 0;
+#ifdef CONFIG_PARTITION_UUIDS
+		cur_part_info.uuid[0] = 0;
+#endif
 	}
 
 	/* Make sure it has a valid FAT header */
-- 
1.7.0.4

[U-Boot] [PATCH V4 3/3] FAT: implement fat_set_blk_dev(), convert cmd_fat.c

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

This makes the FAT filesystem API more consistent with other block-based
filesystems. If in the future standard multi-filesystem commands such as
"ls" or "load" are implemented, having FAT work the same way as other
filesystems will be necessary.

Convert cmd_fat.c to the new API, so the code looks more like other files
implementing the same commands for other filesystems.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
---
v4: No change (just rebased).
v3: Rebase on new fat.c cleanup patch.
v2: Fix inverted test of get_partition_info() result in fat_register_device().
---
 common/cmd_fat.c |    8 +++---
 fs/fat/fat.c     |   66 +++++++++++++++++++++++++----------------------------
 include/fat.h    |    1 +
 3 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index 5a5698b..c38302d 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -55,7 +55,7 @@ int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 
 	dev = dev_desc->dev;
-	if (fat_register_device(dev_desc,part)!=0) {
+	if (fat_set_blk_dev(dev_desc, &info) != 0) {
 		printf("\n** Unable to use %s %d:%d for fatload **\n",
 			argv[1], dev, part);
 		return 1;
@@ -111,7 +111,7 @@ int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 
 	dev = dev_desc->dev;
-	if (fat_register_device(dev_desc,part)!=0) {
+	if (fat_set_blk_dev(dev_desc, &info) != 0) {
 		printf("\n** Unable to use %s %d:%d for fatls **\n",
 			argv[1], dev, part);
 		return 1;
@@ -149,7 +149,7 @@ int do_fat_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 
 	dev = dev_desc->dev;
-	if (fat_register_device(dev_desc,part)!=0) {
+	if (fat_set_blk_dev(dev_desc, &info) != 0) {
 		printf("\n** Unable to use %s %d:%d for fatinfo **\n",
 			argv[1], dev, part);
 		return 1;
@@ -185,7 +185,7 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
 
 	dev = dev_desc->dev;
 
-	if (fat_register_device(dev_desc, part) != 0) {
+	if (fat_set_blk_dev(dev_desc, &info) != 0) {
 		printf("\n** Unable to use %s %d:%d for fatwrite **\n",
 			argv[1], dev, part);
 		return 1;
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 425aec7..b69a44f 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -61,44 +61,12 @@ static int disk_read(__u32 block, __u32 nr_blocks, void *buf)
 			cur_part_info.start + block, nr_blocks, buf);
 }
 
-int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
+int fat_set_blk_dev(block_dev_desc_t *dev_desc, disk_partition_t *info)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
 
-	/* First close any currently found FAT filesystem */
-	cur_dev = NULL;
-
-#if (defined(CONFIG_CMD_IDE) || \
-     defined(CONFIG_CMD_SATA) || \
-     defined(CONFIG_CMD_SCSI) || \
-     defined(CONFIG_CMD_USB) || \
-     defined(CONFIG_MMC) || \
-     defined(CONFIG_SYSTEMACE) )
-
-	/* Read the partition table, if present */
-	if (!get_partition_info(dev_desc, part_no, &cur_part_info))
-		cur_dev = dev_desc;
-#endif
-
-	/* Otherwise it might be a superfloppy (whole-disk FAT filesystem) */
-	if (!cur_dev) {
-		if (part_no != 0) {
-			printf("** Partition %d not valid on device %d **\n",
-					part_no, dev_desc->dev);
-			return -1;
-		}
-
-		cur_dev = dev_desc;
-		cur_part_info.start = 0;
-		cur_part_info.size = dev_desc->lba;
-		cur_part_info.blksz = dev_desc->blksz;
-		cur_part_info.name[0] = 0;
-		cur_part_info.type[0] = 0;
-		cur_part_info.bootable = 0;
-#ifdef CONFIG_PARTITION_UUIDS
-		cur_part_info.uuid[0] = 0;
-#endif
-	}
+	cur_dev = dev_desc;
+	cur_part_info = *info;
 
 	/* Make sure it has a valid FAT header */
 	if (disk_read(0, 1, buffer) != 1) {
@@ -122,6 +90,34 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
 	return -1;
 }
 
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no)
+{
+	disk_partition_t info;
+
+	/* First close any currently found FAT filesystem */
+	cur_dev = NULL;
+
+	/* Read the partition table, if present */
+	if (get_partition_info(dev_desc, part_no, &info)) {
+		if (part_no != 0) {
+			printf("** Partition %d not valid on device %d **\n",
+					part_no, dev_desc->dev);
+			return -1;
+		}
+
+		info.start = 0;
+		info.size = dev_desc->lba;
+		info.blksz = dev_desc->blksz;
+		info.name[0] = 0;
+		info.type[0] = 0;
+		info.bootable = 0;
+#ifdef CONFIG_PARTITION_UUIDS
+		info.uuid[0] = 0;
+#endif
+	}
+
+	return fat_set_blk_dev(dev_desc, &info);
+}
 
 /*
  * Get the first occurence of a directory delimiter ('/' or '\') in a string.
diff --git a/include/fat.h b/include/fat.h
index cc85b06..706cd7a 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -212,6 +212,7 @@ long file_fat_read_at(const char *filename, unsigned long pos, void *buffer,
 		      unsigned long maxsize);
 long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);
 const char *file_getfsname(int idx);
+int fat_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);
 int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
 
 int file_fat_write(const char *filename, void *buffer, unsigned long maxsize);
-- 
1.7.0.4

[U-Boot] [PATCH V4 3/3] FAT: implement fat_set_blk_dev(), convert cmd_fat.c

[Benoît Thébaudeau]

Hi Stephen,

On Wednesday, October 17, 2012 6:44:59 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> This makes the FAT filesystem API more consistent with other
> block-based
> filesystems. If in the future standard multi-filesystem commands such
> as
> "ls" or "load" are implemented, having FAT work the same way as other
> filesystems will be necessary.
> 
> Convert cmd_fat.c to the new API, so the code looks more like other
> files
> implementing the same commands for other filesystems.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> ---
> v4: No change (just rebased).
> v3: Rebase on new fat.c cleanup patch.
> v2: Fix inverted test of get_partition_info() result in
> fat_register_device().
> ---
[...]

I'm still fine with the V4 of this series:
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>

Best regards,
Beno?t

[U-Boot] [PATCH V4 1/3] FAT: remove cur_part_nr

[Tom Rini]

On Wed, Oct 17, 2012 at 10:44:57AM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> A future patch will implement the more standard filesystem API
> fat_set_blk_dev(). This API has no way to know which partition number
> the partition represents. Equally, future DM rework will make the
> concept of partition number harder to pass around.
> 
> So, simply remove cur_part_nr from fat.c; its only use is in a
> diagnostic printf, and the context where it's printed should make it
> obvious which partition is referred to anyway (since the partition ID
> would come from the user command-line that caused it).
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v4: New patch: Dropped addition of part number of disk_partition_t, and
> replaced it with this patch.

For the series, applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121025/a4060009/attachment.pgp>