Skip to contentSkip to conversation

Auto partition selection and fs partition consolidation

PATCH Superseded Rob Herring 3 participants 20 messages Aug 23, 2012 – Sep 25, 2012 ext4 file system

lore
Share & tools
lore
#0 lore raw

cover Auto partition selection and fs partition consolidation

From: Rob Herring <[email protected]>

The primary goal of this series is to enable auto selection of a partition using the 1st bootable partition as the default partition for disk based boot commands. If a bootable partition is not found and partition is not specified, then the first valid partition is used.

Every command that takes a "<dev>[:<part>]" option duplicates the same parsing code, so this series consolidates the parsing code to a single function and converts all block based filesystem code over to use th

This is based on Wolfgang's ext4 branch.

Rob

Rob Herring (9):
combine block device load commands into common function disk/part: check bootable flag for DOS partitions disk/part: introduce get_device_and_partition
ext4: remove init_fs/deinit_fs
cmd_extX: use common get_device_and_partition function
cmd_fat: use common get_device_and_partition function
cmd_disk: use common get_device_and_partition function
cmd_zfs: use common get_device_and_partition function
cmd_reiser: use common get_device_and_partition function

 common/Makefile         |    1 +
 common/cmd_disk.c       |  122 ++++++++++++++++++++++++++++++++++++++
 common/cmd_ext4.c       |  106 +++------------------------------
 common/cmd_ext_common.c |  108 ++++++++-------------------------
 common/cmd_fat.c        |  100 +++++++++++--------------------
 common/cmd_ide.c        |  151 +----------------------------------------------
 common/cmd_reiser.c     |   81 ++++++-------------------
 common/cmd_scsi.c       |  123 +-------------------------------------
 common/cmd_usb.c        |  138 +------------------------------------------
 common/cmd_zfs.c        |   88 ++++++---------------------
 disk/part.c             |   90 +++++++++++++++++++++++++++-
 disk/part_dos.c         |   11 +++-
 fs/ext4/dev.c           |   32 ++++------
 fs/ext4/ext4_common.h   |    1 -
 fs/ext4/ext4fs.c        |   37 +-----------
 fs/reiserfs/dev.c       |   29 ++++-----
 fs/zfs/dev.c            |   35 ++++-------
 include/command.h       |    4 ++
 include/ext4fs.h        |    5 +-
 include/ext_common.h    |    2 +
 include/part.h          |   14 ++++-
 include/reiserfs.h      |    2 +-
 include/zfs_common.h    |    7 +--
 23 files changed, 380 insertions(+), 907 deletions(-)
 create mode 100644 common/cmd_disk.c
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/9 combine block device load commands into common function

From: Rob Herring <[email protected]>

All the raw block load commands duplicate the same code. Starting with the ide version as it has progress updates convert ide, usb, and scsi boot commands to all use a common version.

---
 common/Makefile   |    1 +
 common/cmd_disk.c |  161 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/cmd_ide.c  |  151 +------------------------------------------------
 common/cmd_scsi.c |  123 +---------------------------------------
 common/cmd_usb.c  |  138 +--------------------------------------------
 include/command.h |    4 ++
 6 files changed, 169 insertions(+), 409 deletions(-)
 create mode 100644 common/cmd_disk.c
common/Makefile +1 -0
index 4273484..0fac6d0 100644
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_SYS_HUSH_PARSER) += hush.o
 COBJS-y += s_record.o
 COBJS-$(CONFIG_SERIAL_MULTI) += serial.o
 COBJS-y += xyzModem.o
+COBJS-y += cmd_disk.o
 
 # core command
 COBJS-y += cmd_boot.o
common/cmd_disk.c new file +161 -0

161 lines not on this page · raw message

common/cmd_ide.c +1 -150

157 lines not on this page · raw message

common/cmd_scsi.c +1 -122

129 lines not on this page · raw message

common/cmd_usb.c +1 -137

144 lines not on this page · raw message

include/command.h +4 -0
index 6e1bdc2..1f06aa1 100644
@@ -110,6 +110,10 @@ static inline int bootm_maybe_autostart(cmd_tbl_t *cmdtp, const char *cmd)
 	return 0;
 }
 #endif
+
+extern int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
+			   char *const argv[]);
+
 extern int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 
 /*
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 1/9 combine block device load commands into common function

Quoted 7 lines · Rob Herring

From: Rob Herring <[email protected]>

All the raw block load commands duplicate the same code. Starting with the ide version as it has progress updates convert ide, usb, and scsi boot commands to all use a common version.

[snip]

common/cmd_disk.c line 2

Quoted 8 lines
common/cmd_disk.c new file +2 -0
new file mode 100644
index 0000000..38420dc
@@ -0,0 +1,161 @@
+#include <common.h>
+#include <command.h>

Many companies have legal departments that get upset by files without license information. Please add a GPLv2+ message, thanks.

Signature · 1 line
Tom
#3 lore raw

PATCH 1/9 combine block device load commands into common function

Quoted 10 lines · Tom Rini
Quoted 7 lines · Rob Herring

From: Rob Herring <[email protected]>

All the raw block load commands duplicate the same code. Starting with the ide version as it has progress updates convert ide, usb, and scsi boot commands to all use a common version.

[snip]

Quoted 11 lines

common/cmd_disk.c line 2

Quoted 8 lines
common/cmd_disk.c new file +2 -0
new file mode 100644
index 0000000..38420dc
@@ -0,0 +1,161 @@
+#include <common.h>
+#include <command.h>

Many companies have legal departments that get upset by files without license information. Please add a GPLv2+ message, thanks.

Probably whatever is on the ide code I copied this from is more appropriate, but yes I need to add that.

Rob

#4 lore raw

PATCH 1/9 combine block device load commands into common function

Quoted 25 lines · Rob Herring
Quoted 10 lines · Tom Rini
Quoted 7 lines · Rob Herring

From: Rob Herring <[email protected]>

All the raw block load commands duplicate the same code. Starting with the ide version as it has progress updates convert ide, usb, and scsi boot commands to all use a common version.

[snip]

Quoted 11 lines

common/cmd_disk.c line 2

Quoted 8 lines
common/cmd_disk.c new file +2 -0
new file mode 100644
index 0000000..38420dc
@@ -0,0 +1,161 @@
+#include <common.h>
+#include <command.h>

Many companies have legal departments that get upset by files without license information. Please add a GPLv2+ message, thanks.

Quoted 2 lines

Probably whatever is on the ide code I copied this from is more appropriate, but yes I need to add that.

Yes, sorry, I meant the GPLv2+ message that all of the other files have some variant of. Copying the cmd_ide.c one on top sounds best.

Signature · 1 line
Tom
#5 lore raw

PATCH v2 1/9 combine block device load commands into common function

In reply to Rob Herring

From: Rob Herring <[email protected]>

All the raw block load commands duplicate the same code. Starting with the ide version as it has progress updates convert ide, usb, and scsi boot commands to all use a common version.

---
v2:
- Add copyright to cmd_disk.c

 common/Makefile   |    1 +
 common/cmd_disk.c |  183 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/cmd_ide.c  |  151 +------------------------------------------
 common/cmd_scsi.c |  123 +----------------------------------
 common/cmd_usb.c  |  138 +---------------------------------------
 include/command.h |    4 ++
 6 files changed, 191 insertions(+), 409 deletions(-)
 create mode 100644 common/cmd_disk.c
common/Makefile +1 -0
index 4273484..0fac6d0 100644
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_SYS_HUSH_PARSER) += hush.o
 COBJS-y += s_record.o
 COBJS-$(CONFIG_SERIAL_MULTI) += serial.o
 COBJS-y += xyzModem.o
+COBJS-y += cmd_disk.o
 
 # core command
 COBJS-y += cmd_boot.o
common/cmd_disk.c new file +183 -0

183 lines not on this page · raw message

common/cmd_ide.c +1 -150

157 lines not on this page · raw message

common/cmd_scsi.c +1 -122

129 lines not on this page · raw message

common/cmd_usb.c +1 -137

144 lines not on this page · raw message

include/command.h +4 -0
index 6e1bdc2..1f06aa1 100644
@@ -110,6 +110,10 @@ static inline int bootm_maybe_autostart(cmd_tbl_t *cmdtp, const char *cmd)
 	return 0;
 }
 #endif
+
+extern int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
+			   char *const argv[]);
+
 extern int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 
 /*
Signature · 1 line
1.7.9.5
#6 lore raw

PATCH v2 1/9 combine block device load commands into common function

Quoted 7 lines · Rob Herring

From: Rob Herring <[email protected]>

All the raw block load commands duplicate the same code. Starting with the ide version as it has progress updates convert ide, usb, and scsi boot commands to all use a common version.

This along with the rest of the series (v2 on 3/9), are now applied to u-boot/master, thanks!

Signature · 8 lines
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/20120925/67c906b3/attachment.pgp>
#7 lore raw

PATCH 2/9 disk/part: check bootable flag for DOS partitions

In reply to Rob Herring

From: Rob Herring <[email protected]>

Determine which partitions are bootable/active. In the partition listing, print "Boot" for partitions with the bootable/active flag set.

---
 disk/part_dos.c |   11 +++++++++--
 include/part.h  |    1 +
 2 files changed, 10 insertions(+), 2 deletions(-)
disk/part_dos.c +9 -2
index a43dd9c..24ac00c 100644
@@ -60,14 +60,20 @@ static inline int is_extended(int part_type)
 	    part_type == 0x85);
 }
 
+static inline int is_bootable(dos_partition_t *p)
+{
+	return p->boot_ind == 0x80;
+}
+
 static void print_one_part (dos_partition_t *p, int ext_part_sector, int part_num)
 {
 	int lba_start = ext_part_sector + le32_to_int (p->start4);
 	int lba_size  = le32_to_int (p->size4);
 
-	printf ("%5d\t\t%10d\t%10d\t%2x%s\n",
+	printf("%5d\t\t%10d\t%10d\t%2x%s%s\n",
 		part_num, lba_start, lba_size, p->sys_ind,
-		(is_extended (p->sys_ind) ? " Extd" : ""));
+		(is_extended(p->sys_ind) ? " Extd" : ""),
+		(is_bootable(p) ? " Boot" : ""));
 }
 
 static int test_block_type(unsigned char *buffer)
@@ -222,6 +228,7 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
 			}
 			/* sprintf(info->type, "%d, pt->sys_ind); */
 			sprintf ((char *)info->type, "U-Boot");
+			info->bootable = is_bootable(pt);
 			return 0;
 		}
 
include/part.h +1 -0
index e1478f4..447f69d 100644
@@ -93,6 +93,7 @@ typedef struct disk_partition {
 	ulong	blksz;		/* block size in bytes			*/
 	uchar	name[32];	/* partition name			*/
 	uchar	type[32];	/* string type description		*/
+	int	bootable;	/* Active/Bootable flag is set		*/
 } disk_partition_t;
 
 /* Misc _get_dev functions */
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH 3/9 disk/part: introduce get_device_and_partition

In reply to Rob Herring

From: Rob Herring <[email protected]>

All block device related commands (scsiboot, fatload, ext2ls, etc.) have simliar duplicated device and partition parsing and selection code. This adds a common function to replace various implementations.

The new function has some enhancements over current versions. If no device or partition is specified on the command line, the bootdevice env variable will be used (scsiboot does this). If the partition is not specified and the device has partitions, then the first bootable partition will be used. If a bootable partition is not found, the first valid partition is used. The ret value is not needed since part will be zero when no partition is found.

---
 disk/part.c    |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/part.h |   13 ++++++--
 2 files changed, 99 insertions(+), 4 deletions(-)
disk/part.c +88 -2
index 76f3939..1284e1a 100644
@@ -70,7 +70,7 @@ static const struct block_drvr block_drvr[] = {
 
 DECLARE_GLOBAL_DATA_PTR;
 
-block_dev_desc_t *get_dev(char* ifname, int dev)
+block_dev_desc_t *get_dev(const char *ifname, int dev)
 {
 	const struct block_drvr *drvr = block_drvr;
 	block_dev_desc_t* (*reloc_get_dev)(int dev);
@@ -97,7 +97,7 @@ block_dev_desc_t *get_dev(char* ifname, int dev)
 	return NULL;
 }
 #else
-block_dev_desc_t *get_dev(char* ifname, int dev)
+block_dev_desc_t *get_dev(const char *ifname, int dev)
 {
 	return NULL;
 }
@@ -288,6 +288,7 @@ void init_part (block_dev_desc_t * dev_desc)
 	    return;
 	}
 #endif
+	dev_desc->part_type = PART_TYPE_UNKNOWN;
 }
 
 
@@ -433,3 +434,88 @@ void print_part (block_dev_desc_t * dev_desc)
 #endif
 
 #endif
+
+int get_device_and_partition(const char *ifname, const char *dev_str,
+			     block_dev_desc_t **dev_desc,
+			     disk_partition_t *info)
+{
+	int ret;
+	char *ep;
+	int dev;
+	block_dev_desc_t *desc;
+	int p;
+	int part = 0;
+	char *part_str;
+	disk_partition_t *best_part = NULL;
+
+	if (dev_str)
+		dev = simple_strtoul(dev_str, &ep, 16);
+
+	if (!dev_str || (dev_str == ep)) {
+		dev_str = getenv("bootdevice");
+		if (dev_str)
+			dev = simple_strtoul(dev_str, &ep, 16);
+		if (!dev_str || (dev_str == ep))
+			goto err;
+	}
+
+	desc = get_dev(ifname, dev);
+	if (!desc || (desc->type == DEV_TYPE_UNKNOWN))
+		goto err;
+
+	if (desc->part_type == PART_TYPE_UNKNOWN) {
+		/* disk doesn't use partition table */
+		if (!desc->lba) {
+			printf("**Bad disk size - %s %d:0 **\n", ifname, dev);
+			return -1;
+		}
+		info->start = 0;
+		info->size = desc->lba;
+		info->blksz = desc->blksz;
+
+		*dev_desc = desc;
+		return 0;
+	}
+
+	part_str = strchr(dev_str, ':');
+	if (part_str) {
+		part = (int)simple_strtoul(++part_str, NULL, 16);
+		ret = get_partition_info(desc, part, info);
+	} else {
+		/* find the first bootable partition. If none are bootable,
+		 * fall back to the first valid partition */
+		for (p = 1; p < 16; p++) {
+			ret = get_partition_info(desc, p, info);
+			if (ret)
+				continue;
+
+			if (!best_part || info->bootable) {
+				best_part = info;
+				part = p;
+			}
+
+			if (info->bootable)
+				break;
+		}
+		info = best_part;
+		if (part)
+			ret = 0;
+	}
+	if (ret) {
+		printf("** Invalid partition %d, use `dev[:part]' **\n", part);
+		return -1;
+	}
+	if (strncmp((char *)info->type, BOOT_PART_TYPE, sizeof(info->type)) != 0) {
+		printf("** Invalid partition type \"%.32s\""
+			" (expect \"" BOOT_PART_TYPE "\")\n",
+			info->type);
+		return -1;
+	}
+
+	*dev_desc = desc;
+	return part;
+
+ err:
+	puts("** Invalid boot device, use `dev[:part]' **\n");
+	return -1;
+}
include/part.h +11 -2
index 447f69d..a6d06f3 100644
@@ -98,7 +98,7 @@ typedef struct disk_partition {
 
 /* Misc _get_dev functions */
 #ifdef CONFIG_PARTITIONS
-block_dev_desc_t* get_dev(char* ifname, int dev);
+block_dev_desc_t *get_dev(const char *ifname, int dev);
 block_dev_desc_t* ide_get_dev(int dev);
 block_dev_desc_t* sata_get_dev(int dev);
 block_dev_desc_t* scsi_get_dev(int dev);
@@ -112,8 +112,12 @@ int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t
 void print_part (block_dev_desc_t *dev_desc);
 void  init_part (block_dev_desc_t *dev_desc);
 void dev_print(block_dev_desc_t *dev_desc);
+int get_device_and_partition(const char *ifname, const char *dev_str,
+			     block_dev_desc_t **dev_desc,
+			     disk_partition_t *info);
 #else
-static inline block_dev_desc_t* get_dev(char* ifname, int dev) { return NULL; }
+static inline block_dev_desc_t *get_dev(const char *ifname, int dev)
+{ return NULL; }
 static inline block_dev_desc_t* ide_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* sata_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* scsi_get_dev(int dev) { return NULL; }
@@ -127,6 +131,11 @@ static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
 static inline void print_part (block_dev_desc_t *dev_desc) {}
 static inline void  init_part (block_dev_desc_t *dev_desc) {}
 static inline void dev_print(block_dev_desc_t *dev_desc) {}
+static inline int get_device_and_partition(const char *ifname,
+					   const char *dev_str,
+					   block_dev_desc_t **dev_desc,
+					   disk_partition_t *info)
+{ *dev_desc = NULL; return -1; }
 #endif
 
 #ifdef CONFIG_MAC_PARTITION
Signature · 1 line
1.7.9.5
#9 lore raw

PATCH 3/9 disk/part: introduce get_device_and_partition

Quoted 6 lines · Rob Herring

From: Rob Herring <[email protected]>

All block device related commands (scsiboot, fatload, ext2ls, etc.) have simliar duplicated device and partition parsing and selection code. This adds a common function to replace various implementations.

Quoted 7 lines

The new function has some enhancements over current versions. If no device or partition is specified on the command line, the bootdevice env variable will be used (scsiboot does this). If the partition is not specified and the device has partitions, then the first bootable partition will be used. If a bootable partition is not found, the first valid partition is used. The ret value is not needed since part will be zero when no partition is found.

Two thoughts on this patch:

First, if I write "mmc 0" right now, command will always attempt to access precisely partion 1, whereas after this patch, they will search for the first bootable, or valid, partition. This is a change in behavior. It's a pretty reasonable change, but I wonder if it might cause problems somewhere.

Instead, perhaps this new feature should be explicitly requested, supporting the following device/partition specifications:

# existing:

dev 0:0        # whole device
dev 0:n        # n >= 1: explicit partition
dev 0          # partition 1

# new:
dev 0:valid # first valid partition
dev 0:bootable # first bootable partition
dev 0:default # first bootable partition if there is one,

               # else first valid

That would allow scripts to be very explicit about whether they wanted this new functionality.

Second, if I run a slew of ext2load commands:

ext2load mmc 0:bootable ${scriptaddr} boot.scr
source ${scriptaddr}

# script does:
ext2load mmc 0:bootable ${kernel_addr} zImage
ext2load mmc 0:bootable ${initrd_addr} initrd.bin
ext2load mmc 0:bootable ${fdt_addr} foo.dtb

Then there are two disadvantages:

1) I believe the partition table is read and decoded and search for every one of those ext2load commands. Slightly inefficient.

2) There's no permanent record of the partition number, so this couldn't be e.g. used to construct a kernel command-line etc.

Instead, I wonder if get_device_and_partition() should just support the existing 3 device specification options, and we introduce a new command to determine which partition to boot from, e.g.:

# writes result to "bootpart" variable
# or get-default or get-first-valid
part get-first-bootable mmc 0 bootpart

ext2load mmc 0:${bootpart} ${scriptaddr} boot.scr
source ${scriptaddr}

# script does:
ext2load mmc 0:${bootpart} ${kernel_addr} zImage
ext2load mmc 0:${bootpart} ${initrd_addr} initrd.bin
ext2load mmc 0:${bootpart} ${fdt_addr} foo.dtb

That solves those issues. Does anyone have any comment on the two approaches?

(although perhaps e.g. ext2load always re-reads the partition table anyway, so perhaps that advantage is moot?)

Aside from that, this series looks conceptually reasonable at a quick glance. I'd be happy to provide an equivalent to patch 2 for GPT/EFI partition tables.

#10 lore raw

PATCH 3/9 disk/part: introduce get_device_and_partition

Quoted 27 lines · Stephen Warren
Quoted 6 lines · Rob Herring

From: Rob Herring <[email protected]>

All block device related commands (scsiboot, fatload, ext2ls, etc.) have simliar duplicated device and partition parsing and selection code. This adds a common function to replace various implementations.

Quoted 7 lines

The new function has some enhancements over current versions. If no device or partition is specified on the command line, the bootdevice env variable will be used (scsiboot does this). If the partition is not specified and the device has partitions, then the first bootable partition will be used. If a bootable partition is not found, the first valid partition is used. The ret value is not needed since part will be zero when no partition is found.

Two thoughts on this patch:

First, if I write "mmc 0" right now, command will always attempt to access precisely partion 1, whereas after this patch, they will search for the first bootable, or valid, partition. This is a change in behavior. It's a pretty reasonable change, but I wonder if it might cause problems somewhere.

Instead, perhaps this new feature should be explicitly requested, supporting the following device/partition specifications:

# existing:

Quoted 8 lines
dev 0:0        # whole device
dev 0:n        # n >= 1: explicit partition
dev 0          # partition 1

# new:
dev 0:valid # first valid partition
dev 0:bootable # first bootable partition
dev 0:default # first bootable partition if there is one,

               # else first valid

I'm not sure we need to distinguish valid vs. bootable. Returning the first valid partition was really just to maintain somewhat backwards compatible behavior.

Perhaps just "0:-" would be sufficient.

Quoted 13 lines

That would allow scripts to be very explicit about whether they wanted this new functionality.

Second, if I run a slew of ext2load commands:

ext2load mmc 0:bootable ${scriptaddr} boot.scr
source ${scriptaddr}

# script does:
ext2load mmc 0:bootable ${kernel_addr} zImage
ext2load mmc 0:bootable ${initrd_addr} initrd.bin
ext2load mmc 0:bootable ${fdt_addr} foo.dtb
Quoted 4 lines

Then there are two disadvantages:

1) I believe the partition table is read and decoded and search for every one of those ext2load commands. Slightly inefficient.

It was already multiple times per command with the command function calling get_partition_info and then the filesystem code calling it again internally as well. Now it is only 1 time at least. I would think the 1st partition being bootable is the common case.

Quoted 2 lines

2) There's no permanent record of the partition number, so this couldn't be e.g. used to construct a kernel command-line etc.

You mean to setup rootfs? I don't think we want u-boot to do that. Or what would be the use?

Quoted 10 lines

Instead, I wonder if get_device_and_partition() should just support the existing 3 device specification options, and we introduce a new command to determine which partition to boot from, e.g.:

# writes result to "bootpart" variable
# or get-default or get-first-valid
part get-first-bootable mmc 0 bootpart

ext2load mmc 0:${bootpart} ${scriptaddr} boot.scr
source ${scriptaddr}

Quoted 7 lines
# script does:
ext2load mmc 0:${bootpart} ${kernel_addr} zImage
ext2load mmc 0:${bootpart} ${initrd_addr} initrd.bin
ext2load mmc 0:${bootpart} ${fdt_addr} foo.dtb

That solves those issues. Does anyone have any comment on the two approaches?

I'm really open to either way.

Another option would be for the first command run to set bootpart and then re-use that value on subsequent commands.

Rob

Quoted 8 lines

(although perhaps e.g. ext2load always re-reads the partition table anyway, so perhaps that advantage is moot?)

Aside from that, this series looks conceptually reasonable at a quick glance. I'd be happy to provide an equivalent to patch 2 for GPT/EFI partition tables.

#11 lore raw

PATCH 3/9 disk/part: introduce get_device_and_partition

Quoted 37 lines · Rob Herring
Quoted 27 lines · Stephen Warren
Quoted 6 lines · Rob Herring

From: Rob Herring <[email protected]>

All block device related commands (scsiboot, fatload, ext2ls, etc.) have simliar duplicated device and partition parsing and selection code. This adds a common function to replace various implementations.

Quoted 7 lines

The new function has some enhancements over current versions. If no device or partition is specified on the command line, the bootdevice env variable will be used (scsiboot does this). If the partition is not specified and the device has partitions, then the first bootable partition will be used. If a bootable partition is not found, the first valid partition is used. The ret value is not needed since part will be zero when no partition is found.

Two thoughts on this patch:

First, if I write "mmc 0" right now, command will always attempt to access precisely partion 1, whereas after this patch, they will search for the first bootable, or valid, partition. This is a change in behavior. It's a pretty reasonable change, but I wonder if it might cause problems somewhere.

Instead, perhaps this new feature should be explicitly requested, supporting the following device/partition specifications:

# existing:

Quoted 8 lines
dev 0:0        # whole device
dev 0:n        # n >= 1: explicit partition
dev 0          # partition 1

# new:
dev 0:valid # first valid partition
dev 0:bootable # first bootable partition
dev 0:default # first bootable partition if there is one,

               # else first valid
Quoted 5 lines

I'm not sure we need to distinguish valid vs. bootable. Returning the first valid partition was really just to maintain somewhat backwards compatible behavior.

Perhaps just "0:-" would be sufficient.

I guess that syntax would be fine if we don't need to distinguish all the cases. "-" isn't that descriptive though, and I've only seen it mean "nothing" in U-Boot commands. So, bike-shedding a bit, it doesn't seem exactly correct. Perhaps just "auto"?

Quoted 22 lines
Quoted 12 lines

That would allow scripts to be very explicit about whether they wanted this new functionality.

Second, if I run a slew of ext2load commands:

ext2load mmc 0:bootable ${scriptaddr} boot.scr
source ${scriptaddr}

# script does:
ext2load mmc 0:bootable ${kernel_addr} zImage
ext2load mmc 0:bootable ${initrd_addr} initrd.bin
ext2load mmc 0:bootable ${fdt_addr} foo.dtb
Quoted 4 lines

Then there are two disadvantages:

1) I believe the partition table is read and decoded and search for every one of those ext2load commands. Slightly inefficient.

It was already multiple times per command with the command function calling get_partition_info and then the filesystem code calling it again internally as well. Now it is only 1 time at least. I would think the 1st partition being bootable is the common case.

Quoted 5 lines
Quoted 2 lines

2) There's no permanent record of the partition number, so this couldn't be e.g. used to construct a kernel command-line etc.

You mean to setup rootfs? I don't think we want u-boot to do that. Or what would be the use?

I can imagine a boot.scr that does:

setenv bootargs root=/dev/mmcblk0p${bootpart}

But then, you may as well use the partition UUID feature instead of that, so that boot.scr doesn't need to know the kernel's device name.

Quoted 15 lines
Quoted 15 lines

Instead, I wonder if get_device_and_partition() should just support the existing 3 device specification options, and we introduce a new command to determine which partition to boot from, e.g.:

# writes result to "bootpart" variable
# or get-default or get-first-valid
part get-first-bootable mmc 0 bootpart

ext2load mmc 0:${bootpart} ${scriptaddr} boot.scr
source ${scriptaddr}

# script does:
ext2load mmc 0:${bootpart} ${kernel_addr} zImage
ext2load mmc 0:${bootpart} ${initrd_addr} initrd.bin
ext2load mmc 0:${bootpart} ${fdt_addr} foo.dtb
Quoted 7 lines
Quoted 2 lines

That solves those issues. Does anyone have any comment on the two approaches?

I'm really open to either way.

Another option would be for the first command run to set bootpart and then re-use that value on subsequent commands.

That could work too, although commands using environment variables seems a little implicit/hidden.

#12 lore raw

PATCH 3/9 disk/part: introduce get_device_and_partition

Quoted 42 lines · Stephen Warren
Quoted 41 lines · Rob Herring
Quoted 27 lines · Stephen Warren
Quoted 6 lines · Rob Herring

From: Rob Herring <[email protected]>

All block device related commands (scsiboot, fatload, ext2ls, etc.) have simliar duplicated device and partition parsing and selection code. This adds a common function to replace various implementations.

Quoted 7 lines

The new function has some enhancements over current versions. If no device or partition is specified on the command line, the bootdevice env variable will be used (scsiboot does this). If the partition is not specified and the device has partitions, then the first bootable partition will be used. If a bootable partition is not found, the first valid partition is used. The ret value is not needed since part will be zero when no partition is found.

Two thoughts on this patch:

First, if I write "mmc 0" right now, command will always attempt to access precisely partion 1, whereas after this patch, they will search for the first bootable, or valid, partition. This is a change in behavior. It's a pretty reasonable change, but I wonder if it might cause problems somewhere.

Instead, perhaps this new feature should be explicitly requested, supporting the following device/partition specifications:

# existing:

Quoted 8 lines
dev 0:0        # whole device
dev 0:n        # n >= 1: explicit partition
dev 0          # partition 1

# new:
dev 0:valid # first valid partition
dev 0:bootable # first bootable partition
dev 0:default # first bootable partition if there is one,

               # else first valid

I'm not sure we need to distinguish valid vs. bootable. Returning the first valid partition was really just to maintain somewhat backwards compatible behavior.

Quoted 6 lines
Quoted 1 line

Perhaps just "0:-" would be sufficient.

I guess that syntax would be fine if we don't need to distinguish all the cases. "-" isn't that descriptive though, and I've only seen it mean "nothing" in U-Boot commands. So, bike-shedding a bit, it doesn't seem exactly correct. Perhaps just "auto"?

"auto" sounds like a good idea to me as well.

[snip]

Quoted 18 lines
Quoted 18 lines
Quoted 17 lines

Instead, I wonder if get_device_and_partition() should just support the existing 3 device specification options, and we introduce a new command to determine which partition to boot from, e.g.:

# writes result to "bootpart" variable
# or get-default or get-first-valid
part get-first-bootable mmc 0 bootpart

ext2load mmc 0:${bootpart} ${scriptaddr} boot.scr
source ${scriptaddr}

# script does:
ext2load mmc 0:${bootpart} ${kernel_addr} zImage
ext2load mmc 0:${bootpart} ${initrd_addr} initrd.bin
ext2load mmc 0:${bootpart} ${fdt_addr} foo.dtb

That solves those issues. Does anyone have any comment on the two approaches?

Quoted 7 lines
Quoted 4 lines

I'm really open to either way.

Another option would be for the first command run to set bootpart and then re-use that value on subsequent commands.

That could work too, although commands using environment variables seems a little implicit/hidden.

Agreed. We should be very careful when changing behavior. Adding a new command so that folks can use this as they see fit sounds like the best idea. I shudder to think what the partition table on some SD cards I have around looks like.

Signature · 1 line
Tom
#13 lore raw

PATCH v2 3/9 disk/part: introduce get_device_and_partition

In reply to Rob Herring

From: Rob Herring <[email protected]>

All block device related commands (scsiboot, fatload, ext2ls, etc.) have simliar duplicated device and partition parsing and selection code. This adds a common function to replace various implementations.

The new function has an enhancement over current versions. If no device or partition is specified on the command line, the bootdevice env variable will be used (scsiboot does this).

---
v2:
- Keep current behavior removing the bootable partition scanning. This will be added in follow-up series from Stephen Warren.

 disk/part.c    |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/part.h |   13 +++++++++--
 2 files changed, 78 insertions(+), 4 deletions(-)
disk/part.c +67 -2
index 76f3939..958656c 100644
@@ -70,7 +70,7 @@ static const struct block_drvr block_drvr[] = {
 
 DECLARE_GLOBAL_DATA_PTR;
 
-block_dev_desc_t *get_dev(char* ifname, int dev)
+block_dev_desc_t *get_dev(const char *ifname, int dev)
 {
 	const struct block_drvr *drvr = block_drvr;
 	block_dev_desc_t* (*reloc_get_dev)(int dev);
@@ -97,7 +97,7 @@ block_dev_desc_t *get_dev(char* ifname, int dev)
 	return NULL;
 }
 #else
-block_dev_desc_t *get_dev(char* ifname, int dev)
+block_dev_desc_t *get_dev(const char *ifname, int dev)
 {
 	return NULL;
 }
@@ -288,6 +288,7 @@ void init_part (block_dev_desc_t * dev_desc)
 	    return;
 	}
 #endif
+	dev_desc->part_type = PART_TYPE_UNKNOWN;
 }
 
 
@@ -433,3 +434,67 @@ void print_part (block_dev_desc_t * dev_desc)
 #endif
 
 #endif
+
+int get_device_and_partition(const char *ifname, const char *dev_str,
+			     block_dev_desc_t **dev_desc,
+			     disk_partition_t *info)
+{
+	int ret;
+	char *ep;
+	int dev;
+	block_dev_desc_t *desc;
+	int part = 0;
+	char *part_str;
+
+	if (dev_str)
+		dev = simple_strtoul(dev_str, &ep, 16);
+
+	if (!dev_str || (dev_str == ep)) {
+		dev_str = getenv("bootdevice");
+		if (dev_str)
+			dev = simple_strtoul(dev_str, &ep, 16);
+		if (!dev_str || (dev_str == ep))
+			goto err;
+	}
+
+	desc = get_dev(ifname, dev);
+	if (!desc || (desc->type == DEV_TYPE_UNKNOWN))
+		goto err;
+
+	if (desc->part_type == PART_TYPE_UNKNOWN) {
+		/* disk doesn't use partition table */
+		if (!desc->lba) {
+			printf("**Bad disk size - %s %d:0 **\n", ifname, dev);
+			return -1;
+		}
+		info->start = 0;
+		info->size = desc->lba;
+		info->blksz = desc->blksz;
+
+		*dev_desc = desc;
+		return 0;
+	}
+
+	part_str = strchr(dev_str, ':');
+	if (part_str)
+		part = (int)simple_strtoul(++part_str, NULL, 16);
+
+	ret = get_partition_info(desc, part, info);
+	if (ret) {
+		printf("** Invalid partition %d, use `dev[:part]' **\n", part);
+		return -1;
+	}
+	if (strncmp((char *)info->type, BOOT_PART_TYPE, sizeof(info->type)) != 0) {
+		printf("** Invalid partition type \"%.32s\""
+			" (expect \"" BOOT_PART_TYPE "\")\n",
+			info->type);
+		return -1;
+	}
+
+	*dev_desc = desc;
+	return part;
+
+ err:
+	puts("** Invalid boot device, use `dev[:part]' **\n");
+	return -1;
+}
include/part.h +11 -2
index 447f69d..a6d06f3 100644
@@ -98,7 +98,7 @@ typedef struct disk_partition {
 
 /* Misc _get_dev functions */
 #ifdef CONFIG_PARTITIONS
-block_dev_desc_t* get_dev(char* ifname, int dev);
+block_dev_desc_t *get_dev(const char *ifname, int dev);
 block_dev_desc_t* ide_get_dev(int dev);
 block_dev_desc_t* sata_get_dev(int dev);
 block_dev_desc_t* scsi_get_dev(int dev);
@@ -112,8 +112,12 @@ int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t
 void print_part (block_dev_desc_t *dev_desc);
 void  init_part (block_dev_desc_t *dev_desc);
 void dev_print(block_dev_desc_t *dev_desc);
+int get_device_and_partition(const char *ifname, const char *dev_str,
+			     block_dev_desc_t **dev_desc,
+			     disk_partition_t *info);
 #else
-static inline block_dev_desc_t* get_dev(char* ifname, int dev) { return NULL; }
+static inline block_dev_desc_t *get_dev(const char *ifname, int dev)
+{ return NULL; }
 static inline block_dev_desc_t* ide_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* sata_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* scsi_get_dev(int dev) { return NULL; }
@@ -127,6 +131,11 @@ static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
 static inline void print_part (block_dev_desc_t *dev_desc) {}
 static inline void  init_part (block_dev_desc_t *dev_desc) {}
 static inline void dev_print(block_dev_desc_t *dev_desc) {}
+static inline int get_device_and_partition(const char *ifname,
+					   const char *dev_str,
+					   block_dev_desc_t **dev_desc,
+					   disk_partition_t *info)
+{ *dev_desc = NULL; return -1; }
 #endif
 
 #ifdef CONFIG_MAC_PARTITION
Signature · 1 line
1.7.9.5
#14 lore raw

PATCH 4/9 ext4: remove init_fs/deinit_fs

In reply to Rob Herring

From: Rob Herring <[email protected]>

There's no real need to expose this and it can be removed by using a static allocation.

---
 common/cmd_ext4.c       |   10 +++-------
 common/cmd_ext_common.c |   19 +++----------------
 fs/ext4/ext4fs.c        |   36 ++----------------------------------
 include/ext4fs.h        |    2 --
 include/zfs_common.h    |    2 --
 5 files changed, 8 insertions(+), 61 deletions(-)
common/cmd_ext4.c +3 -7
index 77094c4..e92c02f 100644
@@ -151,8 +151,6 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 		printf("Block device %s %d not supported\n", argv[1], dev);
 		return 1;
 	}
-	if (init_fs(ext4_dev_desc))
-		return 1;
 
 	fs = get_fs();
 	if (*ep) {
@@ -173,21 +171,21 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 	file_size = simple_strtoul(argv[5], NULL, 10);
 
 	/* set the device as block device */
-	part_length = ext4fs_set_blk_dev(fs->dev_desc, part);
+	part_length = ext4fs_set_blk_dev(ext4_dev_desc, part);
 	if (part_length == 0) {
 		printf("Bad partition - %s %d:%lu\n", argv[1], dev, part);
 		goto fail;
 	}
 
 	/* register the device and partition */
-	if (ext4_register_device(fs->dev_desc, part) != 0) {
+	if (ext4_register_device(ext4_dev_desc, part) != 0) {
 		printf("Unable to use %s %d:%lu for fattable\n",
 		       argv[1], dev, part);
 		goto fail;
 	}
 
 	/* get the partition information */
-	if (!get_partition_info(fs->dev_desc, part, &info)) {
+	if (!get_partition_info(ext4_dev_desc, part, &info)) {
 		total_sector = (info.size * info.blksz) / SECTOR_SIZE;
 		fs->total_sect = total_sector;
 	} else {
@@ -207,13 +205,11 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 		goto fail;
 	}
 	ext4fs_close();
-	deinit_fs(fs->dev_desc);
 
 	return 0;
 
 fail:
 	ext4fs_close();
-	deinit_fs(fs->dev_desc);
 
 	return 1;
 }
common/cmd_ext_common.c +3 -16
index 56ee9a5..8972ccc 100644
@@ -75,7 +75,6 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
 	ulong part_length;
 	int filelen;
 	disk_partition_t info;
-	struct ext_filesystem *fs;
 	char buf[12];
 	unsigned long count;
 	const char *addr_str;
@@ -117,10 +116,7 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
 		printf("** Block device %s %d not supported\n", argv[1], dev);
 		return 1;
 	}
-	if (init_fs(ext4_dev_desc))
-		return 1;
 
-	fs = get_fs();
 	if (*ep) {
 		if (*ep != ':') {
 			puts("** Invalid boot device, use `dev[:part]' **\n");
@@ -130,7 +126,7 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
 	}
 
 	if (part != 0) {
-		if (get_partition_info(fs->dev_desc, part, &info)) {
+		if (get_partition_info(ext4_dev_desc, part, &info)) {
 			printf("** Bad partition %lu **\n", part);
 			goto fail;
 		}
@@ -149,7 +145,7 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
 		       filename, argv[1], dev);
 	}
 
-	part_length = ext4fs_set_blk_dev(fs->dev_desc, part);
+	part_length = ext4fs_set_blk_dev(ext4_dev_desc, part);
 	if (part_length == 0) {
 		printf("**Bad partition - %s %d:%lu **\n", argv[1], dev, part);
 		ext4fs_close();
@@ -180,7 +176,6 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
 	}
 
 	ext4fs_close();
-	deinit_fs(fs->dev_desc);
 	/* Loading ok, update default load address */
 	load_addr = addr;
 
@@ -190,7 +185,6 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	return 0;
 fail:
-	deinit_fs(fs->dev_desc);
 	return 1;
 }
 
@@ -200,7 +194,6 @@ int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	int dev;
 	unsigned long part = 1;
 	char *ep;
-	struct ext_filesystem *fs;
 	int part_length;
 	if (argc < 3)
 		return cmd_usage(cmdtp);
@@ -214,10 +207,6 @@ int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		return 1;
 	}
 
-	if (init_fs(ext4_dev_desc))
-		return 1;
-
-	fs = get_fs();
 	if (*ep) {
 		if (*ep != ':') {
 			puts("\n** Invalid boot device, use `dev[:part]' **\n");
@@ -229,7 +218,7 @@ int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	if (argc == 4)
 		filename = argv[3];
 
-	part_length = ext4fs_set_blk_dev(fs->dev_desc, part);
+	part_length = ext4fs_set_blk_dev(ext4_dev_desc, part);
 	if (part_length == 0) {
 		printf("** Bad partition - %s %d:%lu **\n", argv[1], dev, part);
 		ext4fs_close();
@@ -250,10 +239,8 @@ int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	};
 
 	ext4fs_close();
-	deinit_fs(fs->dev_desc);
 	return 0;
 
 fail:
-	deinit_fs(fs->dev_desc);
 	return 1;
 }
fs/ext4/ext4fs.c +2 -34
index 114c2a2..c366e6f 100644
@@ -44,43 +44,11 @@
 
 int ext4fs_symlinknest;
 block_dev_desc_t *ext4_dev_desc;
+struct ext_filesystem ext_fs;
 
 struct ext_filesystem *get_fs(void)
 {
-	if (ext4_dev_desc == NULL || ext4_dev_desc->priv == NULL)
-		printf("Invalid Input Arguments %s\n", __func__);
-
-	return ext4_dev_desc->priv;
-}
-
-int init_fs(block_dev_desc_t *dev_desc)
-{
-	struct ext_filesystem *fs;
-	if (dev_desc == NULL) {
-		printf("Invalid Input Arguments %s\n", __func__);
-		return -EINVAL;
-	}
-
-	fs = zalloc(sizeof(struct ext_filesystem));
-	if (fs == NULL) {
-		printf("malloc failed: %s\n", __func__);
-		return -ENOMEM;
-	}
-
-	fs->dev_desc = dev_desc;
-	dev_desc->priv = fs;
-
-	return 0;
-}
-
-void deinit_fs(block_dev_desc_t *dev_desc)
-{
-	if (dev_desc == NULL) {
-		printf("Invalid Input Arguments %s\n", __func__);
-		return;
-	}
-	free(dev_desc->priv);
-	dev_desc->priv = NULL;
+	return &ext_fs;
 }
 
 void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot)
include/ext4fs.h +0 -2
index ab2983c..6ad008c 100644
@@ -130,8 +130,6 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 #endif
 
 struct ext_filesystem *get_fs(void);
-int init_fs(block_dev_desc_t *dev_desc);
-void deinit_fs(block_dev_desc_t *dev_desc);
 int ext4fs_open(const char *filename);
 int ext4fs_read(char *buf, unsigned len);
 int ext4fs_mount(unsigned part_length);
include/zfs_common.h +0 -2
index 04e73d0..628231e 100644
@@ -94,8 +94,6 @@ struct zfs_dirhook_info {
 
 
 struct zfs_filesystem *zfsget_fs(void);
-int init_fs(block_dev_desc_t *dev_desc);
-void deinit_fs(block_dev_desc_t *dev_desc);
 int zfs_open(zfs_file_t, const char *filename);
 uint64_t zfs_read(zfs_file_t, char *buf, uint64_t len);
 struct zfs_data *zfs_mount(device_t);
Signature · 1 line
1.7.9.5
#15 lore raw

PATCH 5/9 cmd_extX: use common get_device_and_partition function

In reply to Rob Herring

From: Rob Herring <[email protected]>

Convert ext2/4 load, ls, and write functions to use common device and partition parsing function. With the common function "dev:part" can come from the environment and a '-' can be used in that case.

---
 common/cmd_ext4.c       |  102 ++++-------------------------------------------
 common/cmd_ext_common.c |   95 +++++++++++--------------------------------
 fs/ext4/dev.c           |   32 ++++++---------
 fs/ext4/ext4_common.h   |    1 -
 fs/ext4/ext4fs.c        |    1 -
 include/ext4fs.h        |    3 +-
 include/ext_common.h    |    2 +
 7 files changed, 46 insertions(+), 190 deletions(-)
common/cmd_ext4.c +7 -95
index e92c02f..48f9ba3 100644
@@ -56,21 +56,6 @@
 #include <usb.h>
 #endif
 
-#if !defined(CONFIG_DOS_PARTITION) && !defined(CONFIG_EFI_PARTITION)
-#error DOS or EFI partition support must be selected
-#endif
-
-uint64_t total_sector;
-uint64_t part_offset;
-#if defined(CONFIG_CMD_EXT4_WRITE)
-static uint64_t part_size;
-static uint16_t cur_part = 1;
-#endif
-
-#define DOS_PART_MAGIC_OFFSET		0x1fe
-#define DOS_FS_TYPE_OFFSET		0x36
-#define DOS_FS32_TYPE_OFFSET		0x52
-
 int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
 						char *const argv[])
 {
@@ -89,77 +74,24 @@ int do_ext4_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 }
 
 #if defined(CONFIG_CMD_EXT4_WRITE)
-static int ext4_register_device(block_dev_desc_t *dev_desc, int part_no)
-{
-	unsigned char buffer[SECTOR_SIZE];
-	disk_partition_t info;
-
-	if (!dev_desc->block_read)
-		return -1;
-
-	/* check if we have a MBR (on floppies we have only a PBR) */
-	if (dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) {
-		printf("** Can't read from device %d **\n", dev_desc->dev);
-		return -1;
-	}
-	if (buffer[DOS_PART_MAGIC_OFFSET] != 0x55 ||
-	    buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) {
-		/* no signature found */
-		return -1;
-	}
-
-	/* First we assume there is a MBR */
-	if (!get_partition_info(dev_desc, part_no, &info)) {
-		part_offset = info.start;
-		cur_part = part_no;
-		part_size = info.size;
-	} else if ((strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET],
-			    "FAT", 3) == 0) || (strncmp((char *)&buffer
-							[DOS_FS32_TYPE_OFFSET],
-							"FAT32", 5) == 0)) {
-		/* ok, we assume we are on a PBR only */
-		cur_part = 1;
-		part_offset = 0;
-	} else {
-		printf("** Partition %d not valid on device %d **\n",
-		       part_no, dev_desc->dev);
-		return -1;
-	}
-
-	return 0;
-}
-
 int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 				char *const argv[])
 {
 	const char *filename = "/";
-	int part_length;
-	unsigned long part = 1;
-	int dev;
-	char *ep;
+	int dev, part;
 	unsigned long ram_address;
 	unsigned long file_size;
 	disk_partition_t info;
-	struct ext_filesystem *fs;
+	block_dev_desc_t *dev_desc;
 
 	if (argc < 6)
 		return cmd_usage(cmdtp);
 
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	ext4_dev_desc = get_dev(argv[1], dev);
-	if (ext4_dev_desc == NULL) {
-		printf("Block device %s %d not supported\n", argv[1], dev);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
 
-	fs = get_fs();
-	if (*ep) {
-		if (*ep != ':') {
-			puts("Invalid boot device, use `dev[:part]'\n");
-			goto fail;
-		}
-		part = simple_strtoul(++ep, NULL, 16);
-	}
+	dev = dev_desc->dev;
 
 	/* get the filename */
 	filename = argv[3];
@@ -171,30 +103,10 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 	file_size = simple_strtoul(argv[5], NULL, 10);
 
 	/* set the device as block device */
-	part_length = ext4fs_set_blk_dev(ext4_dev_desc, part);
-	if (part_length == 0) {
-		printf("Bad partition - %s %d:%lu\n", argv[1], dev, part);
-		goto fail;
-	}
-
-	/* register the device and partition */
-	if (ext4_register_device(ext4_dev_desc, part) != 0) {
-		printf("Unable to use %s %d:%lu for fattable\n",
-		       argv[1], dev, part);
-		goto fail;
-	}
-
-	/* get the partition information */
-	if (!get_partition_info(ext4_dev_desc, part, &info)) {
-		total_sector = (info.size * info.blksz) / SECTOR_SIZE;
-		fs->total_sect = total_sector;
-	} else {
-		printf("error : get partition info\n");
-		goto fail;
-	}
+	ext4fs_set_blk_dev(dev_desc, &info);
 
 	/* mount the filesystem */
-	if (!ext4fs_mount(part_length)) {
+	if (!ext4fs_mount(info.size)) {
 		printf("Bad ext4 partition %s %d:%lu\n", argv[1], dev, part);
 		goto fail;
 	}
common/cmd_ext_common.c +23 -72

134 lines not on this page · raw message

fs/ext4/dev.c +13 -19
index fb62f24..5a3fbef 100644
@@ -38,26 +38,20 @@
 
 #include <common.h>
 #include <config.h>
+#include <ext4fs.h>
 #include <ext_common.h>
 
+unsigned long part_offset;
+
 static block_dev_desc_t *ext4fs_block_dev_desc;
-static disk_partition_t part_info;
+static disk_partition_t *part_info;
 
-int ext4fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
+void ext4fs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info)
 {
 	ext4fs_block_dev_desc = rbdd;
-
-	if (part == 0) {
-		/* disk doesn't use partition table */
-		part_info.start = 0;
-		part_info.size = rbdd->lba;
-		part_info.blksz = rbdd->blksz;
-	} else {
-		if (get_partition_info(ext4fs_block_dev_desc,
-					part, &part_info))
-			return 0;
-	}
-	return part_info.size;
+	part_info = info;
+	part_offset = info->start;
+	get_fs()->total_sect = (info->size * info->blksz) / SECTOR_SIZE;
 }
 
 int ext4fs_devread(int sector, int byte_offset, int byte_len, char *buf)
@@ -68,7 +62,7 @@ int ext4fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 	/* Check partition boundaries */
 	if ((sector < 0)
 	    || ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >=
-		part_info.size)) {
+		part_info->size)) {
 		printf("%s read outside partition %d\n", __func__, sector);
 		return 0;
 	}
@@ -88,7 +82,7 @@ int ext4fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 		/* read first part which isn't aligned with start of sector */
 		if (ext4fs_block_dev_desc->
 		    block_read(ext4fs_block_dev_desc->dev,
-				part_info.start + sector, 1,
+				part_info->start + sector, 1,
 				(unsigned long *) sec_buf) != 1) {
 			printf(" ** ext2fs_devread() read error **\n");
 			return 0;
@@ -111,14 +105,14 @@ int ext4fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 
 		block_len = SECTOR_SIZE;
 		ext4fs_block_dev_desc->block_read(ext4fs_block_dev_desc->dev,
-						  part_info.start + sector,
+						  part_info->start + sector,
 						  1, (unsigned long *)p);
 		memcpy(buf, p, byte_len);
 		return 1;
 	}
 
 	if (ext4fs_block_dev_desc->block_read(ext4fs_block_dev_desc->dev,
-					       part_info.start + sector,
+					       part_info->start + sector,
 					       block_len / SECTOR_SIZE,
 					       (unsigned long *) buf) !=
 					       block_len / SECTOR_SIZE) {
@@ -134,7 +128,7 @@ int ext4fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 		/* read rest of data which are not in whole sector */
 		if (ext4fs_block_dev_desc->
 		    block_read(ext4fs_block_dev_desc->dev,
-				part_info.start + sector, 1,
+				part_info->start + sector, 1,
 				(unsigned long *) sec_buf) != 1) {
 			printf("* %s read error - last part\n", __func__);
 			return 0;
fs/ext4/ext4_common.h +0 -1
index 801b8b8..7489397 100644
@@ -57,7 +57,6 @@
 
 #define zalloc(size) calloc(1, size)
 
-extern unsigned long part_offset;
 int ext4fs_read_inode(struct ext2_data *data, int ino,
 		      struct ext2_inode *inode);
 int ext4fs_read_file(struct ext2fs_node *node, int pos,
fs/ext4/ext4fs.c +0 -1
index c366e6f..93dcb7e 100644
@@ -43,7 +43,6 @@
 #include "ext4_common.h"
 
 int ext4fs_symlinknest;
-block_dev_desc_t *ext4_dev_desc;
 struct ext_filesystem ext_fs;
 
 struct ext_filesystem *get_fs(void)
include/ext4fs.h +1 -2
index 6ad008c..b6eedde 100644
@@ -113,7 +113,6 @@ struct ext_filesystem {
 	block_dev_desc_t *dev_desc;
 };
 
-extern block_dev_desc_t *ext4_dev_desc;
 extern struct ext2_data *ext4fs_root;
 extern struct ext2fs_node *ext4fs_file;
 
@@ -137,6 +136,6 @@ void ext4fs_close(void);
 int ext4fs_ls(const char *dirname);
 void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot);
 int ext4fs_devread(int sector, int byte_offset, int byte_len, char *buf);
-int ext4fs_set_blk_dev(block_dev_desc_t *rbdd, int part);
+void ext4fs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);
 long int read_allocated_block(struct ext2_inode *inode, int fileblock);
 #endif
include/ext_common.h +2 -0
index 9b97522..ce73857 100644
@@ -186,6 +186,8 @@ struct ext2_data {
 	struct ext2fs_node diropen;
 };
 
+extern unsigned long part_offset;
+
 int do_ext2ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ext2load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
Signature · 1 line
1.7.9.5
#16 lore raw

PATCH 6/9 cmd_fat: use common get_device_and_partition function

In reply to Rob Herring

From: Rob Herring <[email protected]>

Convert fatload, fatls, and fatinfo to use common device and partition parsing function. With the common function "dev:part" can come from the environment and a '-' can be used in that case.

---
 common/cmd_fat.c |  100 +++++++++++++++++++-----------------------------------
 1 file changed, 34 insertions(+), 66 deletions(-)
common/cmd_fat.c +34 -66
index 559a16d..90412d6 100644
@@ -40,29 +40,20 @@ int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	unsigned long count;
 	char buf [12];
 	block_dev_desc_t *dev_desc=NULL;
-	int dev=0;
-	int part=1;
-	char *ep;
+	disk_partition_t info;
+	int part, dev;
 
 	if (argc < 5) {
-		printf( "usage: fatload <interface> <dev[:part]> "
+		printf("usage: fatload <interface> [<dev[:part]>] "
 			"<addr> <filename> [bytes]\n");
 		return 1;
 	}
 
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	dev_desc = get_dev(argv[1],dev);
-	if (dev_desc == NULL) {
-		puts("\n** Invalid boot device **\n");
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
-	if (*ep) {
-		if (*ep != ':') {
-			puts("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = (int)simple_strtoul(++ep, NULL, 16);
-	}
+
+	dev = dev_desc->dev;
 	if (fat_register_device(dev_desc,part)!=0) {
 		printf("\n** Unable to use %s %d:%d for fatload **\n",
 			argv[1], dev, part);
@@ -93,7 +84,7 @@ int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(
 	fatload,	6,	0,	do_fat_fsload,
 	"load binary file from a dos filesystem",
-	"<interface> <dev[:part]>  <addr> <filename> [bytes]\n"
+	"<interface> [<dev[:part]>]  <addr> <filename> [bytes]\n"
 	"    - load binary file 'filename' from 'dev' on 'interface'\n"
 	"      to address 'addr' from dos filesystem"
 );
@@ -101,29 +92,20 @@ U_BOOT_CMD(
 int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char *filename = "/";
-	int ret;
-	int dev=0;
-	int part=1;
-	char *ep;
+	int ret, dev, part;
 	block_dev_desc_t *dev_desc=NULL;
+	disk_partition_t info;
 
-	if (argc < 3) {
-		printf("usage: fatls <interface> <dev[:part]> [directory]\n");
+	if (argc < 2) {
+		printf("usage: fatls <interface> [<dev[:part]>] [directory]\n");
 		return 0;
 	}
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	dev_desc = get_dev(argv[1],dev);
-	if (dev_desc == NULL) {
-		puts("\n** Invalid boot device **\n");
+
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
-	if (*ep) {
-		if (*ep != ':') {
-			puts("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = (int)simple_strtoul(++ep, NULL, 16);
-	}
+
+	dev = dev_desc->dev;
 	if (fat_register_device(dev_desc,part)!=0) {
 		printf("\n** Unable to use %s %d:%d for fatls **\n",
 			argv[1], dev, part);
@@ -142,34 +124,26 @@ int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(
 	fatls,	4,	1,	do_fat_ls,
 	"list files in a directory (default /)",
-	"<interface> <dev[:part]> [directory]\n"
+	"<interface> [<dev[:part]>] [directory]\n"
 	"    - list files from 'dev' on 'interface' in a 'directory'"
 );
 
 int do_fat_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	int dev=0;
-	int part=1;
-	char *ep;
-	block_dev_desc_t *dev_desc=NULL;
+	int dev, part;
+	block_dev_desc_t *dev_desc;
+	disk_partition_t info;
 
 	if (argc < 2) {
-		printf("usage: fatinfo <interface> <dev[:part]>\n");
+		printf("usage: fatinfo <interface> [<dev[:part]>]\n");
 		return 0;
 	}
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	dev_desc = get_dev(argv[1],dev);
-	if (dev_desc == NULL) {
-		puts("\n** Invalid boot device **\n");
+
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
-	if (*ep) {
-		if (*ep != ':') {
-			puts("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = (int)simple_strtoul(++ep, NULL, 16);
-	}
+
+	dev = dev_desc->dev;
 	if (fat_register_device(dev_desc,part)!=0) {
 		printf("\n** Unable to use %s %d:%d for fatinfo **\n",
 			argv[1], dev, part);
@@ -181,7 +155,7 @@ int do_fat_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(
 	fatinfo,	3,	1,	do_fat_fsinfo,
 	"print information about filesystem",
-	"<interface> <dev[:part]>\n"
+	"<interface> [<dev[:part]>]\n"
 	"    - print information about filesystem from 'dev' on 'interface'"
 );
 
@@ -193,6 +167,7 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
 	unsigned long addr;
 	unsigned long count;
 	block_dev_desc_t *dev_desc = NULL;
+	disk_partition_t info;
 	int dev = 0;
 	int part = 1;
 	char *ep;
@@ -200,19 +175,12 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
 	if (argc < 5)
 		return cmd_usage(cmdtp);
 
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	dev_desc = get_dev(argv[1], dev);
-	if (dev_desc == NULL) {
-		puts("\n** Invalid boot device **\n");
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
-	if (*ep) {
-		if (*ep != ':') {
-			puts("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = (int)simple_strtoul(++ep, NULL, 16);
-	}
+
+	dev = dev_desc->dev;
+
 	if (fat_register_device(dev_desc, part) != 0) {
 		printf("\n** Unable to use %s %d:%d for fatwrite **\n",
 			argv[1], dev, part);
Signature · 1 line
1.7.9.5
#17 lore raw

PATCH 7/9 cmd_disk: use common get_device_and_partition function

In reply to Rob Herring

From: Rob Herring <[email protected]>

---
 common/cmd_disk.c |   77 +++++++++++++----------------------------------------
 1 file changed, 19 insertions(+), 58 deletions(-)
common/cmd_disk.c +19 -58
index 38420dc..e6676b0 100644
@@ -1,13 +1,15 @@
 #include <common.h>
 #include <command.h>
+#include <part.h>
 
+#if defined(CONFIG_CMD_IDE) || defined(CONFIG_CMD_SCSI) || \
+	defined(CONFIG_USB_STORAGE)
 int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc, 
 		    char *const argv[])
 {
-	char *boot_device = NULL;
-	char *ep;
-	int dev, part = 0;
-	ulong addr, cnt;
+	int dev, part;
+	ulong addr = CONFIG_SYS_LOAD_ADDR;
+	ulong cnt;
 	disk_partition_t info;
 	image_header_t *hdr;
 	block_dev_desc_t *dev_desc;
@@ -17,72 +19,30 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
 #endif
 
 	bootstage_mark(BOOTSTAGE_ID_IDE_START);
-	switch (argc) {
-	case 1:
-		addr = CONFIG_SYS_LOAD_ADDR;
-		boot_device = getenv("bootdevice");
-		break;
-	case 2:
-		addr = simple_strtoul(argv[1], NULL, 16);
-		boot_device = getenv("bootdevice");
-		break;
-	case 3:
-		addr = simple_strtoul(argv[1], NULL, 16);
-		boot_device = argv[2];
-		break;
-	default:
+	if (argc > 3) {
 		bootstage_error(BOOTSTAGE_ID_IDE_ADDR);
 		return CMD_RET_USAGE;
 	}
 	bootstage_mark(BOOTSTAGE_ID_IDE_ADDR);
 
-	if (!boot_device) {
-		puts("\n** No boot device **\n");
-		bootstage_error(BOOTSTAGE_ID_IDE_BOOT_DEVICE);
-		return 1;
-	}
-	bootstage_mark(BOOTSTAGE_ID_IDE_BOOT_DEVICE);
+	if (argc > 1)
+		addr = simple_strtoul(argv[1], NULL, 16);
 
-	dev = simple_strtoul(boot_device, &ep, 16);
+	bootstage_mark(BOOTSTAGE_ID_IDE_BOOT_DEVICE);
 
-	dev_desc = get_dev(intf, dev);
-	if (dev_desc->type == DEV_TYPE_UNKNOWN) {
-		printf("\n** Device %d not available\n", dev);
+	part = get_device_and_partition(intf, (argc == 3) ? argv[2] : NULL,
+					&dev_desc, &info);
+	if (part < 0) {
 		bootstage_error(BOOTSTAGE_ID_IDE_TYPE);
 		return 1;
 	}
-	bootstage_mark(BOOTSTAGE_ID_IDE_TYPE);
-
-	if (*ep) {
-		if (*ep != ':') {
-			puts("\n** Invalid boot device, use `dev[:part]' **\n");
-			bootstage_error(BOOTSTAGE_ID_IDE_PART);
-			return 1;
-		}
-		part = simple_strtoul(++ep, NULL, 16);
-	}
-	bootstage_mark(BOOTSTAGE_ID_IDE_PART);
 
-	if (get_partition_info(dev_desc, part, &info)) {
-		bootstage_error(BOOTSTAGE_ID_IDE_PART_INFO);
-		return 1;
-	}
-	bootstage_mark(BOOTSTAGE_ID_IDE_PART_INFO);
-
-	if ((strncmp((char *)info.type, BOOT_PART_TYPE, sizeof(info.type)) != 0)
-	    &&
-	    (strncmp((char *)info.type, BOOT_PART_COMP, sizeof(info.type)) != 0)
-	   ) {
-		printf("\n** Invalid partition type \"%.32s\"" " (expect \""
-			BOOT_PART_TYPE "\")\n",
-			info.type);
-		bootstage_error(BOOTSTAGE_ID_IDE_PART_TYPE);
-		return 1;
-	}
-	bootstage_mark(BOOTSTAGE_ID_IDE_PART_TYPE);
+	dev = dev_desc->dev;
+	bootstage_mark(BOOTSTAGE_ID_IDE_TYPE);
 
-	printf("\nLoading from disk device %d, partition %d: "
-	       "Name: %.32s  Type: %.32s\n", dev, part, info.name, info.type);
+	printf("\nLoading from %s device %d, partition %d: "
+	       "Name: %.32s  Type: %.32s\n", intf, dev, part, info.name,
+	       info.type);
 
 	debug("First Block: %ld,  # of blocks: %ld, Block Size: %ld\n",
 	      info.start, info.size, info.blksz);
@@ -158,4 +118,5 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
 
 	return bootm_maybe_autostart(cmdtp, argv[0]);
 }
+#endif
 
Signature · 1 line
1.7.9.5
#18 lore raw

PATCH 8/9 cmd_zfs: use common get_device_and_partition function

In reply to Rob Herring

From: Rob Herring <[email protected]>

Convert zfsload and zfsls to use common device and partition parsing function. With the common function "dev:part" can come from the environment and a '-' can be used in that case.

---
 common/cmd_zfs.c     |   88 +++++++++++---------------------------------------
 fs/zfs/dev.c         |   35 +++++++-------------
 include/zfs_common.h |    5 +--
 3 files changed, 31 insertions(+), 97 deletions(-)
common/cmd_zfs.c +19 -69
index a6ea2c0..27f8856 100644
@@ -49,12 +49,11 @@
 static int do_zfs_load(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	char *filename = NULL;
-	char *ep;
 	int dev;
-	unsigned long part = 1;
+	int part;
 	ulong addr = 0;
-	ulong part_length;
 	disk_partition_t info;
+	block_dev_desc_t *dev_desc;
 	char buf[12];
 	unsigned long count;
 	const char *addr_str;
@@ -95,48 +94,17 @@ static int do_zfs_load(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 		return 1;
 	}
 
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	zfs_dev_desc = get_dev(argv[1], dev);
-	if (zfs_dev_desc == NULL) {
-		printf("** Block device %s %d not supported\n", argv[1], dev);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
 
-	if (*ep) {
-		if (*ep != ':') {
-			puts("** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = simple_strtoul(++ep, NULL, 16);
-	}
+	dev = dev_desc->dev;
+	printf("Loading file \"%s\" from %s device %d%c%c\n",
+		filename, argv[1], dev,
+		part ? ':' : ' ', part ? part + '0' : ' ');
 
-	if (part != 0) {
-		if (get_partition_info(zfs_dev_desc, part, &info)) {
-			printf("** Bad partition %lu **\n", part);
-			return 1;
-		}
-
-		if (strncmp((char *)info.type, BOOT_PART_TYPE,
-					strlen(BOOT_PART_TYPE)) != 0) {
-			printf("** Invalid partition type \"%s\" (expect \"" BOOT_PART_TYPE "\")\n",
-				   info.type);
-			return 1;
-		}
-		printf("Loading file \"%s\" "
-			   "from %s device %d:%lu %s\n",
-			   filename, argv[1], dev, part, info.name);
-	} else {
-		printf("Loading file \"%s\" from %s device %d\n",
-			   filename, argv[1], dev);
-	}
-
-	part_length = zfs_set_blk_dev(zfs_dev_desc, part);
-	if (part_length == 0) {
-		printf("**Bad partition - %s %d:%lu **\n", argv[1], dev, part);
-		return 1;
-	}
-
-	vdev.part_length = part_length;
+	zfs_set_blk_dev(dev_desc, &info);
+	vdev.part_length = info.size;
 
 	memset(&zfile, 0, sizeof(zfile));
 	zfile.device = &vdev;
@@ -149,7 +117,7 @@ static int do_zfs_load(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 		zfile.size = (uint64_t)count;
 
 	if (zfs_read(&zfile, (char *)addr, zfile.size) != zfile.size) {
-		printf("** Unable to read \"%s\" from %s %d:%lu **\n",
+		printf("** Unable to read \"%s\" from %s %d:%d **\n",
 			   filename, argv[1], dev, part);
 		zfs_close(&zfile);
 		return 1;
@@ -181,41 +149,23 @@ int zfs_print(const char *entry, const struct zfs_dirhook_info *data)
 static int do_zfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	const char *filename = "/";
-	int dev;
-	unsigned long part = 1;
-	char *ep;
-	int part_length;
+	int part;
+	block_dev_desc_t *dev_desc;
+	disk_partition_t info;
 	struct device_s vdev;
 
-	if (argc < 3)
+	if (argc < 2)
 		return cmd_usage(cmdtp);
 
-	dev = (int)simple_strtoul(argv[2], &ep, 16);
-	zfs_dev_desc = get_dev(argv[1], dev);
-
-	if (zfs_dev_desc == NULL) {
-		printf("\n** Block device %s %d not supported\n", argv[1], dev);
-		return 1;
-	}
-
-	if (*ep) {
-		if (*ep != ':') {
-			puts("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = simple_strtoul(++ep, NULL, 16);
-	}
-
 	if (argc == 4)
 		filename = argv[3];
 
-	part_length = zfs_set_blk_dev(zfs_dev_desc, part);
-	if (part_length == 0) {
-		printf("** Bad partition - %s %d:%lu **\n", argv[1], dev, part);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
 
-	vdev.part_length = part_length;
+	zfs_set_blk_dev(dev_desc, &info);
+	vdev.part_length = info.size;
 
 	zfs_ls(&vdev, filename,
 		   zfs_print);
fs/zfs/dev.c +11 -24
index d68372c..36be8f5 100644
@@ -26,23 +26,12 @@
 #include <zfs_common.h>
 
 static block_dev_desc_t *zfs_block_dev_desc;
-static disk_partition_t part_info;
+static disk_partition_t *part_info;
 
-int zfs_set_blk_dev(block_dev_desc_t *rbdd, int part)
+void zfs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info)
 {
 	zfs_block_dev_desc = rbdd;
-
-	if (part == 0) {
-		/* disk doesn't use partition table */
-		part_info.start = 0;
-		part_info.size = rbdd->lba;
-		part_info.blksz = rbdd->blksz;
-	} else {
-		if (get_partition_info(zfs_block_dev_desc, part, &part_info))
-			return 0;
-	}
-
-	return part_info.size;
+	part_info = info;
 }
 
 /* err */
@@ -57,7 +46,7 @@ int zfs_devread(int sector, int byte_offset, int byte_len, char *buf)
 	 */
 	if ((sector < 0) ||
 		((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >=
-		 part_info.size)) {
+		 part_info->size)) {
 		/*		errnum = ERR_OUTSIDE_PART; */
 		printf(" ** zfs_devread() read outside partition sector %d\n", sector);
 		return 1;
@@ -79,8 +68,8 @@ int zfs_devread(int sector, int byte_offset, int byte_len, char *buf)
 	if (byte_offset != 0) {
 		/* read first part which isn't aligned with start of sector */
 		if (zfs_block_dev_desc->block_read(zfs_block_dev_desc->dev,
-										   part_info.start + sector, 1,
-										   (unsigned long *) sec_buf) != 1) {
+			part_info->start + sector, 1,
+			(unsigned long *)sec_buf) != 1) {
 			printf(" ** zfs_devread() read error **\n");
 			return 1;
 		}
@@ -102,17 +91,15 @@ int zfs_devread(int sector, int byte_offset, int byte_len, char *buf)
 
 		block_len = SECTOR_SIZE;
 		zfs_block_dev_desc->block_read(zfs_block_dev_desc->dev,
-									   part_info.start + sector,
-									   1, (unsigned long *)p);
+			part_info->start + sector,
+			1, (unsigned long *)p);
 		memcpy(buf, p, byte_len);
 		return 0;
 	}
 
 	if (zfs_block_dev_desc->block_read(zfs_block_dev_desc->dev,
-									   part_info.start + sector,
-									   block_len / SECTOR_SIZE,
-									   (unsigned long *) buf) !=
-		block_len / SECTOR_SIZE) {
+		part_info->start + sector, block_len / SECTOR_SIZE,
+		(unsigned long *) buf) != block_len / SECTOR_SIZE) {
 		printf(" ** zfs_devread() read error - block\n");
 		return 1;
 	}
@@ -126,7 +113,7 @@ int zfs_devread(int sector, int byte_offset, int byte_len, char *buf)
 		/* read rest of data which are not in whole sector */
 		if (zfs_block_dev_desc->
 			block_read(zfs_block_dev_desc->dev,
-					   part_info.start + sector, 1,
+					   part_info->start + sector, 1,
 					   (unsigned long *) sec_buf) != 1) {
 			printf(" ** zfs_devread() read error - last part\n");
 			return 1;
include/zfs_common.h +1 -4
index 628231e..3bd575e 100644
@@ -66,9 +66,6 @@ struct zfs_filesystem {
 	block_dev_desc_t *dev_desc;
 };
 
-
-extern block_dev_desc_t *zfs_dev_desc;
-
 struct device_s {
 	uint64_t part_length;
 };
@@ -101,7 +98,7 @@ int zfs_close(zfs_file_t);
 int zfs_ls(device_t dev, const char *path,
 		   int (*hook) (const char *, const struct zfs_dirhook_info *));
 int zfs_devread(int sector, int byte_offset, int byte_len, char *buf);
-int zfs_set_blk_dev(block_dev_desc_t *rbdd, int part);
+void zfs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);
 void zfs_unmount(struct zfs_data *data);
 int lzjb_decompress(void *, void *, uint32_t, uint32_t);
 #endif
Signature · 1 line
1.7.9.5
#19 lore raw

PATCH 9/9 cmd_reiser: use common get_device_and_partition function

In reply to Rob Herring

From: Rob Herring <[email protected]>

Convert reiserload and reiserls to use common device and partition parsing function. With the common function "dev:part" can come from the environment and a '-' can be used in that case.

---
 common/cmd_reiser.c |   81 +++++++++++----------------------------------------
 fs/reiserfs/dev.c   |   29 +++++++-----------
 include/reiserfs.h  |    2 +-
 3 files changed, 28 insertions(+), 84 deletions(-)
common/cmd_reiser.c +17 -64
index fbb9484..2865014 100644
@@ -50,43 +50,27 @@
 int do_reiserls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char *filename = "/";
-	int dev=0;
-	int part=1;
-	char *ep;
+	int dev, part;
 	block_dev_desc_t *dev_desc=NULL;
-	int part_length;
+	disk_partition_t info;
 
 	if (argc < 3)
 		return CMD_RET_USAGE;
 
-	dev = (int)simple_strtoul (argv[2], &ep, 16);
-	dev_desc = get_dev(argv[1],dev);
-
-	if (dev_desc == NULL) {
-		printf ("\n** Block device %s %d not supported\n", argv[1], dev);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
-
-	if (*ep) {
-		if (*ep != ':') {
-			puts ("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = (int)simple_strtoul(++ep, NULL, 16);
-	}
 
 	if (argc == 4) {
 	    filename = argv[3];
 	}
 
+	dev = dev_desc->dev;
 	PRINTF("Using device %s %d:%d, directory: %s\n", argv[1], dev, part, filename);
 
-	if ((part_length = reiserfs_set_blk_dev(dev_desc, part)) == 0) {
-		printf ("** Bad partition - %s %d:%d **\n",  argv[1], dev, part);
-		return 1;
-	}
+	reiserfs_set_blk_dev(dev_desc, &info);
 
-	if (!reiserfs_mount(part_length)) {
+	if (!reiserfs_mount(info.size)) {
 		printf ("** Bad Reiserfs partition or disk - %s %d:%d **\n",  argv[1], dev, part);
 		return 1;
 	}
@@ -112,9 +96,8 @@ U_BOOT_CMD(
 int do_reiserload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char *filename = NULL;
-	char *ep;
-	int dev, part = 0;
-	ulong addr = 0, part_length, filelen;
+	int dev, part;
+	ulong addr = 0, filelen;
 	disk_partition_t info;
 	block_dev_desc_t *dev_desc = NULL;
 	char buf [12];
@@ -157,49 +140,19 @@ int do_reiserload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	dev = (int)simple_strtoul (argv[2], &ep, 16);
-	dev_desc = get_dev(argv[1],dev);
-	if (dev_desc==NULL) {
-		printf ("\n** Block device %s %d not supported\n", argv[1], dev);
+	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+	if (part < 0)
 		return 1;
-	}
-	if (*ep) {
-		if (*ep != ':') {
-			puts ("\n** Invalid boot device, use `dev[:part]' **\n");
-			return 1;
-		}
-		part = (int)simple_strtoul(++ep, NULL, 16);
-	}
 
-	PRINTF("Using device %s%d, partition %d\n", argv[1], dev, part);
+	dev = dev_desc->dev;
 
-	if (part != 0) {
-		if (get_partition_info (dev_desc, part, &info)) {
-			printf ("** Bad partition %d **\n", part);
-			return 1;
-		}
+	printf("Loading file \"%s\" from %s device %d%c%c\n",
+		filename, argv[1], dev,
+		part ? ':' : ' ', part ? part + '0' : ' ');
 
-		if (strncmp((char *)info.type, BOOT_PART_TYPE, sizeof(info.type)) != 0) {
-			printf ("\n** Invalid partition type \"%.32s\""
-				" (expect \"" BOOT_PART_TYPE "\")\n",
-				info.type);
-			return 1;
-		}
-		PRINTF ("\nLoading from block device %s device %d, partition %d: "
-			"Name: %.32s  Type: %.32s  File:%s\n",
-			argv[1], dev, part, info.name, info.type, filename);
-	} else {
-		PRINTF ("\nLoading from block device %s device %d, File:%s\n",
-			argv[1], dev, filename);
-	}
-
-
-	if ((part_length = reiserfs_set_blk_dev(dev_desc, part)) == 0) {
-		printf ("** Bad partition - %s %d:%d **\n",  argv[1], dev, part);
-		return 1;
-	}
+	reiserfs_set_blk_dev(dev_desc, &info);
 
-	if (!reiserfs_mount(part_length)) {
+	if (!reiserfs_mount(info.size)) {
 		printf ("** Bad Reiserfs partition or disk - %s %d:%d **\n",  argv[1], dev, part);
 		return 1;
 	}
fs/reiserfs/dev.c +10 -19
index 1facfaf..cb288d6 100644
@@ -25,24 +25,13 @@
 #include "reiserfs_private.h"
 
 static block_dev_desc_t *reiserfs_block_dev_desc;
-static disk_partition_t part_info;
+static disk_partition_t *part_info;
 
 
-int reiserfs_set_blk_dev(block_dev_desc_t *rbdd, int part)
+void reiserfs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info)
 {
 	reiserfs_block_dev_desc = rbdd;
-
-	if (part == 0) {
-		/* disk doesn't use partition table */
-		part_info.start = 0;
-		part_info.size = rbdd->lba;
-		part_info.blksz = rbdd->blksz;
-	} else {
-		if (get_partition_info (reiserfs_block_dev_desc, part, &part_info)) {
-			return 0;
-		}
-	}
-	return (part_info.size);
+	part_info = info;
 }
 
 
@@ -59,7 +48,7 @@ int reiserfs_devread (int sector, int byte_offset, int byte_len, char *buf)
 	*/
 	if (sector < 0
 	    || ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS))
-	    >= part_info.size)) {
+	    >= part_info->size)) {
 /*		errnum = ERR_OUTSIDE_PART; */
 		printf (" ** reiserfs_devread() read outside partition\n");
 		return 0;
@@ -83,7 +72,8 @@ int reiserfs_devread (int sector, int byte_offset, int byte_len, char *buf)
 	if (byte_offset != 0) {
 		/* read first part which isn't aligned with start of sector */
 		if (reiserfs_block_dev_desc->block_read(reiserfs_block_dev_desc->dev,
-		    part_info.start+sector, 1, (unsigned long *)sec_buf) != 1) {
+		    part_info->start + sector, 1,
+		    (unsigned long *)sec_buf) != 1) {
 			printf (" ** reiserfs_devread() read error\n");
 			return 0;
 		}
@@ -96,8 +86,8 @@ int reiserfs_devread (int sector, int byte_offset, int byte_len, char *buf)
 	/* read sector aligned part */
 	block_len = byte_len & ~(SECTOR_SIZE-1);
 	if (reiserfs_block_dev_desc->block_read(reiserfs_block_dev_desc->dev,
-	    part_info.start+sector, block_len/SECTOR_SIZE, (unsigned long *)buf) !=
-	    block_len/SECTOR_SIZE) {
+	    part_info->start + sector, block_len/SECTOR_SIZE,
+	    (unsigned long *)buf) != block_len/SECTOR_SIZE) {
 		printf (" ** reiserfs_devread() read error - block\n");
 		return 0;
 	}
@@ -108,7 +98,8 @@ int reiserfs_devread (int sector, int byte_offset, int byte_len, char *buf)
 	if ( byte_len != 0 ) {
 		/* read rest of data which are not in whole sector */
 		if (reiserfs_block_dev_desc->block_read(reiserfs_block_dev_desc->dev,
-		    part_info.start+sector, 1, (unsigned long *)sec_buf) != 1) {
+		    part_info->start + sector, 1,
+		    (unsigned long *)sec_buf) != 1) {
 			printf (" ** reiserfs_devread() read error - last part\n");
 			return 0;
 		}
include/reiserfs.h +1 -1
index c465b3c..dc89342 100644
@@ -75,7 +75,7 @@ typedef enum
 } reiserfs_error_t;
 
 
-extern int reiserfs_set_blk_dev(block_dev_desc_t *rbdd, int part);
+extern void reiserfs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);
 extern int reiserfs_ls (char *dirname);
 extern int reiserfs_open (char *filename);
 extern int reiserfs_read (char *buf, unsigned len);
Signature · 1 line
1.7.9.5