On 14-05-30 08:58 AM, Stephen Warren wrote:
On 05/29/2014 04:03 PM, Steve Rae wrote:


On 14-05-29 01:30 PM, Stephen Warren wrote:
On 05/29/2014 01:44 PM, Steve Rae wrote:


On 14-05-29 11:51 AM, Stephen Warren wrote:
On 05/29/2014 11:58 AM, Steve Rae wrote:
Hi, Stephen

On 14-05-29 09:25 AM, Stephen Warren wrote:
On 05/28/2014 04:15 PM, Steve Rae wrote:
Each wrapper function:
- switches to the specified physical partition, then
- performs the original function, and then
- switches back to the original physical partition
where the physical partition (aka HW partition) is
      0=User, 1=Boot1, 2=Boot2, etc.

This feels wrong; why wouldn't mmc_get_dev() return a
block_dev_desc_t
containing block_read/block_write functions that do the HW partition
switching. That way, this is all completely hidden, and all client
code
only knows about block devices, rather than having to know about
MMC-specific mmc_block_read/write/erase_hwpart() functions.

This goes back to the initial discussion on this mailing list
(which was
never resolved):
     http://lists.denx.de/pipermail/u-boot/2014-April/178171.html
This issue is that the three callback functions defined in
'block_desc_t' do not accept the "partition number" as an argument.
It was suggested that we could overwrite those functions; but the
"partition number" still needs to be passed in by:
(1) overloading the "int dev_num" argument, or
(2) adding another argument to the callback functions
I assumed that neither of these was acceptable, so I have proposed
these
wrappers...

Can't the data simply be stored in the block_desc_t itself?

If I understand this suggestion, are you proposing:
- add an "unsigned int specified_hw_part" to the block_desc_t

Yes.

Then the usage would become:
mmc->block_dev.specified_hw_part = 1;      /* specify Boot1 partition */

The only code that would need to assign that field is
disk/part.c:get_dev() or something called from it. that is the function
that's responsible for looking up or creating the block_dev_desc_t
"handle" for a user-specified storage device, so it's exactly the place
for this kind of object "constructor" code to execute.

Sorry, but now I am totally confused...
Doesn't the "block_dev_desc_t" contain the "device" information (not the
"partition" information)?

The eMMC HW partitions are separate block devices. So,
get_device_and_partition() returns a block device that represents one of:

a) eMMC "boot0" HW partition
b) eMMC "boot1" HW partition
c) eMMC "main data/user area" HW partition

These HW partitions are entirely separate from (MBR/GPT) SW partitions,
even though both are referred to as "partitions". That's why I call the
former "HW partitions" rather than "partitions".

Agree -- and sometimes called "physical partitions"

Isn't it only created once (effectively the first time "mmc_init" is
called on that device)?

The block_dev_desc_t initialization/creation does call mmc_init, yes...

So when I'm doing a block_read from the Boot1 partition, followed by a
block_read from the User partition, I don't expect to see a
"constructor" being executed (from a get_dev() or anything else...)

Most U-Boot commands take a single device name (e.g. "mmc 0") and act
just on that. If you want to do something on different block devices,
you'd need to run separate commands, or perhaps have one command take a
list of devices, and initialize each one in turn.
Agree - and can switch to different HW partitions with the existing "mmc dev 0 1" command

 What code are you
looking at that handles multiple devices sequentially under program
control rather than user command control?

Cannot go into too much detail here (yet) -- but imagine the situation where:
- lookup the GPT partition name (in User, Boot1, Boot2)
- do a block_write to the desired location...

So after discussing with a colleague, we would propose the following. Does this implement what you were proposing?:


Usage (example):

mmc->part_num_next_block_op = 1;          /* specify Boot1 partition */
mmc->block_dev.block_read(0, 0, 1, buf); /* read first block from Boot1 partition */
mmc->part_num_next_block_op = 0;          /* specify User partition */
mmc->block_dev.block_read(0, 0, 1, buf); /* read first block from User partition */

Implementation:
(1) The mmc->part_num_next_block_op needs to be initialized to -1.
(2) Each existing mmc_{bread,bwrite,berase} function needs modification:
if 0 <= mmc->part_num_next_block_op && mmc->part_num != mmc->part_num_next_block_op
    switch
    if switch failed
        mmc->part_num_next_block_op = -1
        return error
[... the original code ...]
if 0 <= mmc->part_num_next_block_op && mmc->part_num != mmc->part_num_next_block_op
    switch_back
    if switch_back failed
        mmc->part_num = mmc->part_num_next_block_op
    mmc->part_num_next_block_op = -1


Many Thanks, Steve
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to