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)? Isn't it only created once (effectively the first time "mmc_init" is called on that device)? 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...)
mmc->block_dev.block_read(0, 0, 1, buf); /* read first block (from Boot1 partition) */
Yes.
mmc->block_dev.specified_hw_part = 0; /* specify User partition */ mmc->block_dev.block_read(0, 0, 1, buf); /* read first block (from User partition) */
I don't think this is a good idea...
Oh, but it is:-)