[U-Boot] [PATCH 3/9] disk/part: introduce get_device_and_partition

Stephen Warren
2012-08-23T22:36:15Z

On 08/23/2012 03:31 PM, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> 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.

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.