Patchwork [U-Boot,V3,2/4] FAT: make use of disk_partition_t.part

login
register
mail settings
Submitter Stephen Warren
Date Oct. 10, 2012, 6:14 p.m.
Message ID <1349892842-11994-2-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/190802/
State Changes Requested
Delegated to: Tom Rini
Headers show

Comments

Stephen Warren - Oct. 10, 2012, 6:14 p.m.
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(-)
Pavel Herrmann - Oct. 13, 2012, 7:38 p.m.
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
Stephen Warren - Oct. 15, 2012, 4:40 p.m.
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.
Pavel Herrmann - Oct. 15, 2012, 6:07 p.m.
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
Tom Rini - Oct. 17, 2012, 4:23 p.m.
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.

Patch

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;