[U-Boot] [PATCH] mmc: add wrappers for MMC block_{read, write, erase}

[Stephen Warren <swarren@wwwdotorg.org>]

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

> 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. What code are you
looking at that handles multiple devices sequentially under program
control rather than user command control?