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

[Rob Herring]

On 08/23/2012 05:36 PM, Stephen Warren wrote:
> On 08/23/2012 03:31 PM, Rob Herring wrote:
>> From: Rob Herring <rob.herring at 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

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.

> 
> 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.

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.

> 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?

> 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?

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

> 
> (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.
>