[U-Boot] [PATCH v2] mmc: Split device init to decouple OCR-polling delay

[Jaehoon Chung]

Hi Simon,

It looks good to me.

Acked-by: Jaehoon Chung <jh80.chung at samsung.com>

On 02/09/2013 01:48 AM, Simon Glass wrote:
> Hi,
> 
> On Sun, Dec 16, 2012 at 7:44 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi,
>>
>> On Sun, Dec 16, 2012 at 6:12 PM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>>> On 12/16/2012 02:18 AM, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On Fri, Nov 30, 2012 at 3:13 PM, Simon Glass <sjg at chromium.org> wrote:
>>>>> Hi Jaehoon,
>>>>>
>>>>> On Fri, Nov 30, 2012 at 12:25 AM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This concept is very good.
>>>>>> But I have one question. I think need to call mmc_init() one more, right?
>>>>>> how did you save the boot time(200ms)?
>>>>>>
>>>>>> On 11/29/2012 10:21 AM, Simon Glass wrote:
>>>>>>> From: Che-Liang Chiou <clchiou at chromium.org>
>>>>>>>
>>>>>>> Most of time that MMC driver spends on initializing a device is polling
>>>>>>> OCR (operation conditions register).  To decouple this polling loop,
>>>>>>> device init is split into two parts: The first part fires the OCR query
>>>>>>> command, and the second part polls the result.  So the caller is now no
>>>>>>> longer bound to the OCR-polling delay; he may fire the query, go
>>>>>>> somewhere and then come back later for the result.
>>>>>>>
>>>>>>> To use this, call mmc_set_preinit() on any device which needs this.
>>>>>>>
>>>>>>> This can save significant amounts of time on boot (e.g. 200ms) by
>>>>>>> hiding the MMC init time behind other init.
>>>>>> snip..
>>>>>>> +int mmc_init(struct mmc *mmc)
>>>>>>> +{
>>>>>>> +     int err = IN_PROGRESS;
>>>>>>> +     unsigned start = get_timer(0);
>>>>>>> +
>>>>>>> +     if (mmc->has_init)
>>>>>>> +             return 0;
>>>>>>> +     if (!mmc->init_in_progress)
>>>>>>> +             err = mmc_start_init(mmc);
>>>>>> It need not to return? if err is IN_PROGRESS, next condition is immediately run.
>>>>>> Then i think we didn't save the time before adjust this patch.
>>>>>
>>>>> It's a little confusing, but the way it works is that mmc_preinit()
>>>>> calls mmc_start_init() early in boot. Then when mmc_init() finally
>>>>> gets called (later) it finishes off the init. We still need mmc_init()
>>>>> to actually fully complete the init. If it were to return before
>>>>> completing the init then we would be unable to use the MMC.
>>>>>
>>>>>>> +
>>>>>>> +     if (!err || err == IN_PROGRESS)
>>>>>>> +             err = mmc_complete_init(mmc);
>>>>>>> +     debug("%s: %d, time %lu\n", __func__, err, get_timer(start));
>>>>>>>       return err;
>>>>>>>  }
>>>>>
>>>>
>>>> Does this patch look good now? I am wondering if it will be including
>>>> in release, or in next?
>>> Well, concept is very good. But i didn't see the any benefit yet.
>>> I will test more...and share the result.
>>
>> OK thanks. Assuming that make sure that the pre-init is definitely
>> done, I suggest you then make U-Boot go and do something else for
>> 200ms, then do the full mmc init after that. It should complete
>> immediately.
>>
>> So there is no time saving in mmc init, but you can do something else
>> while waiting for the MMC init to complete (it takes 200ms or so for
>> me).
>>
>> Another thing to note with eMMC is that we probe for SD first. This
>> takes a little bit of time. We could perhaps use device tree to
>> provide information about the type of attached memory, and thus avoid
>> that probe.
> 
> Does this patch look suitable for merging at this point?
> 
> Regards,
> Simon
>