public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH V3 1/4] part: add partition number to disk_partition_t
@ 2012-10-10 18:13 Stephen Warren
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part Stephen Warren
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Stephen Warren @ 2012-10-10 18:13 UTC (permalink / raw)
  To: u-boot

From: Stephen Warren <swarren@nvidia.com>

The FAT filesystem code knows which partition ID it is operating on.
Currently, this is passed to fat_register_device() as a parameter.
In order to convert FAT to the more standardized fat_set_blk_dev(), the
information needs to come from somewhere else, and the partition
definition structure is the logical place.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: Set info->part in get_device_and_partition() when returning "whole
    disk" partition.
v2: No change.
---
 disk/part.c    |    2 ++
 include/part.h |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index 8ba3cde..6d8135d 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -391,6 +391,7 @@ int get_partition_info(block_dev_desc_t *dev_desc, int part
 	defined(CONFIG_MMC) || \
 	defined(CONFIG_SYSTEMACE)
 
+	info->part = part;
 #ifdef CONFIG_PARTITION_UUIDS
 	/* The common case is no UUID support */
 	info->uuid[0] = 0;
@@ -557,6 +558,7 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,
 			goto cleanup;
 		}
 
+		info->part = 0;
 		info->start = 0;
 		info->size = (*dev_desc)->lba;
 		info->blksz = (*dev_desc)->blksz;
diff --git a/include/part.h b/include/part.h
index 27ea283..ebdebd8 100644
--- a/include/part.h
+++ b/include/part.h
@@ -88,6 +88,7 @@ typedef struct block_dev_desc {
 #define DEV_TYPE_OPDISK		0x07	/* optical disk */
 
 typedef struct disk_partition {
+	int	part;		/* Partition number			*/
 	ulong	start;		/* # of first block in partition	*/
 	ulong	size;		/* number of blocks in partition	*/
 	ulong	blksz;		/* block size in bytes			*/
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part
  2012-10-10 18:13 [U-Boot] [PATCH V3 1/4] part: add partition number to disk_partition_t Stephen Warren
@ 2012-10-10 18:14 ` Stephen Warren
  2012-10-13 19:38   ` Pavel Herrmann
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 3/4] FAT: initialize all fields in cur_part_info, simplify init Stephen Warren
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 4/4] FAT: implement fat_set_blk_dev(), convert cmd_fat.c Stephen Warren
  2 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2012-10-10 18:14 UTC (permalink / raw)
  To: u-boot

From: Stephen Warren <swarren@nvidia.com>

This removes the standalone cur_part_nr variable, opening the way to
replacing fat_register_device() with fat_set_blk_dev().

Note that when get_partition_info() fails and we use the entire disk,
the correct partition number is 0 (whole disk) not 1 (first partition),
so that change is also made here.

An alternative to this (and the previous) patch might be to simply
remove the partition number from the printf(). I don't know how attached
people are to it.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: No change.
v2: No change.
---
 fs/fat/fat.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 80156c8..1e0d2a3 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
@@ -79,7 +78,6 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
 	/* Read the partition table, if present */
 	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
 		cur_dev = dev_desc;
-		cur_part_nr = part_no;
 	}
 #endif
 
@@ -92,7 +90,7 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
 		}
 
 		cur_dev = dev_desc;
-		cur_part_nr = 1;
+		cur_part_info.part = 0;
 		cur_part_info.start = 0;
 		cur_part_info.size = dev_desc->lba;
 		cur_part_info.blksz = dev_desc->blksz;
@@ -1235,7 +1233,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,
+	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_info.part,
 		volinfo.fs_type, vol_label);
 
 	return 0;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 3/4] FAT: initialize all fields in cur_part_info, simplify init
  2012-10-10 18:13 [U-Boot] [PATCH V3 1/4] part: add partition number to disk_partition_t Stephen Warren
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part Stephen Warren
@ 2012-10-10 18:14 ` Stephen Warren
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 4/4] FAT: implement fat_set_blk_dev(), convert cmd_fat.c Stephen Warren
  2 siblings, 0 replies; 9+ messages in thread
From: Stephen Warren @ 2012-10-10 18:14 UTC (permalink / raw)
  To: u-boot

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>
---
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 1e0d2a3..2863c4b 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -94,8 +94,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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 4/4] FAT: implement fat_set_blk_dev(), convert cmd_fat.c
  2012-10-10 18:13 [U-Boot] [PATCH V3 1/4] part: add partition number to disk_partition_t Stephen Warren
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part Stephen Warren
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 3/4] FAT: initialize all fields in cur_part_info, simplify init Stephen Warren
@ 2012-10-10 18:14 ` Stephen Warren
  2012-10-10 18:44   ` Benoît Thébaudeau
  2 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2012-10-10 18:14 UTC (permalink / raw)
  To: u-boot

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>
---
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     |   69 +++++++++++++++++++++++++-----------------------------
 include/fat.h    |    1 +
 3 files changed, 37 insertions(+), 41 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 2863c4b..a6fc07c 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -61,46 +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.part = 0;
-		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) {
@@ -124,6 +90,35 @@ 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.part = 0;
+		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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 4/4] FAT: implement fat_set_blk_dev(), convert cmd_fat.c
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 4/4] FAT: implement fat_set_blk_dev(), convert cmd_fat.c Stephen Warren
@ 2012-10-10 18:44   ` Benoît Thébaudeau
  0 siblings, 0 replies; 9+ messages in thread
From: Benoît Thébaudeau @ 2012-10-10 18:44 UTC (permalink / raw)
  To: u-boot

On Wednesday, October 10, 2012 8:14:02 PM, Stephen Warren wrote:
> 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>
> ---
> 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     |   69
>  +++++++++++++++++++++++++-----------------------------
>  include/fat.h    |    1 +
>  3 files changed, 37 insertions(+), 41 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 2863c4b..a6fc07c 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -61,46 +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.part = 0;
> -		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) {
> @@ -124,6 +90,35 @@ 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.part = 0;
> +		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);

For the V3 of this series:
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part
  2012-10-10 18:14 ` [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part Stephen Warren
@ 2012-10-13 19:38   ` Pavel Herrmann
  2012-10-15 16:40     ` Stephen Warren
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Herrmann @ 2012-10-13 19:38 UTC (permalink / raw)
  To: u-boot

Hi

On Wednesday 10 October 2012 12:14:00 Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> This removes the standalone cur_part_nr variable, opening the way to
> replacing fat_register_device() with fat_set_blk_dev().
> 
> Note that when get_partition_info() fails and we use the entire disk,
> the correct partition number is 0 (whole disk) not 1 (first partition),
> so that change is also made here.
> 
> An alternative to this (and the previous) patch might be to simply
> remove the partition number from the printf(). I don't know how attached
> people are to it.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Just as a heads up, in the DM any difference between a partition and a whole 
block device (also between different interfaces for disks) is hidden from any 
user code (code other than the one keeping track of partitions/disks, that 
only uses such information to determine whether to scan for partitions), you 
only get some object that can read/write blocks if you ask it nicely, and you 
have to make do with that (if you need more then you're probably doing 
something wrong).
As a result, there is no notion of partition number, and the string you are 
patching up here (along with many others, due to unification of disk 
interfaces) is changed.

Best Regards
Pavel Herrmann

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part
  2012-10-13 19:38   ` Pavel Herrmann
@ 2012-10-15 16:40     ` Stephen Warren
  2012-10-15 18:07       ` Pavel Herrmann
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2012-10-15 16:40 UTC (permalink / raw)
  To: u-boot

On 10/13/2012 01:38 PM, Pavel Herrmann wrote:
> Hi
> 
> On Wednesday 10 October 2012 12:14:00 Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> This removes the standalone cur_part_nr variable, opening the way to
>> replacing fat_register_device() with fat_set_blk_dev().
>>
>> Note that when get_partition_info() fails and we use the entire disk,
>> the correct partition number is 0 (whole disk) not 1 (first partition),
>> so that change is also made here.
>>
>> An alternative to this (and the previous) patch might be to simply
>> remove the partition number from the printf(). I don't know how attached
>> people are to it.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Just as a heads up, in the DM any difference between a partition and a whole 
> block device (also between different interfaces for disks) is hidden from any 
> user code (code other than the one keeping track of partitions/disks, that 
> only uses such information to determine whether to scan for partitions), you 
> only get some object that can read/write blocks if you ask it nicely, and you 
> have to make do with that (if you need more then you're probably doing 
> something wrong).
> As a result, there is no notion of partition number, and the string you are 
> patching up here (along with many others, due to unification of disk 
> interfaces) is changed.

OK, so do you think it'd be better right now to drop patches 1 and 2 in
this series, and just remove the partition number from fat.c's printf()
call? That'd certainly be simple to do.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part
  2012-10-15 16:40     ` Stephen Warren
@ 2012-10-15 18:07       ` Pavel Herrmann
  2012-10-17 16:23         ` Tom Rini
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Herrmann @ 2012-10-15 18:07 UTC (permalink / raw)
  To: u-boot

On Monday 15 of October 2012 10:40:25 Stephen Warren wrote:
> On 10/13/2012 01:38 PM, Pavel Herrmann wrote:
> > Hi
> > 
> > On Wednesday 10 October 2012 12:14:00 Stephen Warren wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >> 
> >> This removes the standalone cur_part_nr variable, opening the way to
> >> replacing fat_register_device() with fat_set_blk_dev().
> >> 
> >> Note that when get_partition_info() fails and we use the entire disk,
> >> the correct partition number is 0 (whole disk) not 1 (first partition),
> >> so that change is also made here.
> >> 
> >> An alternative to this (and the previous) patch might be to simply
> >> remove the partition number from the printf(). I don't know how attached
> >> people are to it.
> >> 
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > 
> > Just as a heads up, in the DM any difference between a partition and a
> > whole block device (also between different interfaces for disks) is
> > hidden from any user code (code other than the one keeping track of
> > partitions/disks, that only uses such information to determine whether to
> > scan for partitions), you only get some object that can read/write blocks
> > if you ask it nicely, and you have to make do with that (if you need more
> > then you're probably doing something wrong).
> > As a result, there is no notion of partition number, and the string you
> > are
> > patching up here (along with many others, due to unification of disk
> > interfaces) is changed.
> 
> OK, so do you think it'd be better right now to drop patches 1 and 2 in
> this series, and just remove the partition number from fat.c's printf()
> call? That'd certainly be simple to do.

Well, in my case I have done a broader abstraction, that could be used for 
non-continuous partitions as well (think LVM) with minimal effort (think extend 
identifier structure used for searching to more than 
interface:number:partnumber, no changes in FS code), and partition number 
loses any meaning in that context.
Whether dropping the number now is an acceptable change would be up to Tom 
Rini, I would vote for it though, if that meant anything around here.

Pavel Herrmann

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part
  2012-10-15 18:07       ` Pavel Herrmann
@ 2012-10-17 16:23         ` Tom Rini
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2012-10-17 16:23 UTC (permalink / raw)
  To: u-boot

On Mon, Oct 15, 2012 at 08:07:44PM +0200, Pavel Herrmann wrote:
> On Monday 15 of October 2012 10:40:25 Stephen Warren wrote:
> > On 10/13/2012 01:38 PM, Pavel Herrmann wrote:
> > > Hi
> > > 
> > > On Wednesday 10 October 2012 12:14:00 Stephen Warren wrote:
> > >> From: Stephen Warren <swarren@nvidia.com>
> > >> 
> > >> This removes the standalone cur_part_nr variable, opening the way to
> > >> replacing fat_register_device() with fat_set_blk_dev().
> > >> 
> > >> Note that when get_partition_info() fails and we use the entire disk,
> > >> the correct partition number is 0 (whole disk) not 1 (first partition),
> > >> so that change is also made here.
> > >> 
> > >> An alternative to this (and the previous) patch might be to simply
> > >> remove the partition number from the printf(). I don't know how attached
> > >> people are to it.
> > >> 
> > >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > > 
> > > Just as a heads up, in the DM any difference between a partition and a
> > > whole block device (also between different interfaces for disks) is
> > > hidden from any user code (code other than the one keeping track of
> > > partitions/disks, that only uses such information to determine whether to
> > > scan for partitions), you only get some object that can read/write blocks
> > > if you ask it nicely, and you have to make do with that (if you need more
> > > then you're probably doing something wrong).
> > > As a result, there is no notion of partition number, and the string you
> > > are
> > > patching up here (along with many others, due to unification of disk
> > > interfaces) is changed.
> > 
> > OK, so do you think it'd be better right now to drop patches 1 and 2 in
> > this series, and just remove the partition number from fat.c's printf()
> > call? That'd certainly be simple to do.
> 
> Well, in my case I have done a broader abstraction, that could be used for 
> non-continuous partitions as well (think LVM) with minimal effort (think extend 
> identifier structure used for searching to more than 
> interface:number:partnumber, no changes in FS code), and partition number 
> loses any meaning in that context.
> Whether dropping the number now is an acceptable change would be up to Tom 
> Rini, I would vote for it though, if that meant anything around here.

OK, lets rework this series as suggested here to make the DM work a
little easier.  Drop the print instead.

-- 
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/20121017/712e7b43/attachment.pgp>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-10-17 16:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-10 18:13 [U-Boot] [PATCH V3 1/4] part: add partition number to disk_partition_t Stephen Warren
2012-10-10 18:14 ` [U-Boot] [PATCH V3 2/4] FAT: make use of disk_partition_t.part Stephen Warren
2012-10-13 19:38   ` Pavel Herrmann
2012-10-15 16:40     ` Stephen Warren
2012-10-15 18:07       ` Pavel Herrmann
2012-10-17 16:23         ` Tom Rini
2012-10-10 18:14 ` [U-Boot] [PATCH V3 3/4] FAT: initialize all fields in cur_part_info, simplify init Stephen Warren
2012-10-10 18:14 ` [U-Boot] [PATCH V3 4/4] FAT: implement fat_set_blk_dev(), convert cmd_fat.c Stephen Warren
2012-10-10 18:44   ` Benoît Thébaudeau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox