On Thu, Aug 23, 2012 at 08:51:50PM -0600, Stephen Warren wrote:
On 08/23/2012 07:57 PM, Rob Herring wrote:
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@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.
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]
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.
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.