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

[Tom Rini]

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

-- 
Tom