[U-Boot] [PATCH] mmc: fix OCR Polling

[Pantelis Antoniou]

Hi Andrew, Peng,

> On Mar 23, 2015, at 01:23 , Andrew Gabbasov <andrew_gabbasov at mentor.com> wrote:
> 
> Hi Peng,
> 
>> From: Peng Fan [mailto:Peng.Fan at freescale.com]
>> Sent: Saturday, March 21, 2015 1:34 PM
>> To: Gabbasov, Andrew; u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH] mmc: fix OCR Polling
>> 
>> [skipped]
>> 
>>> After this patch, if the busy flag is indeed already set (so that the
>>> loop body is not executed), and it is not an SPI case
>>> (mmc_host_is_spi(mmc) is false), the "cmd" structure (which is local
>>> to mmc_complete_op_cond() function) is left uninitialized, and using
>>> cmd.response[0] later in the function becomes incorrect. And the OCR
>>> register value and the high capacity flag may be set incorrectly.
>> Yeah. you are right.
>> Maybe the following piece of code should be added to replace mmc->ocr =
>> cmd.response[0]:
>> "
>> if (mmc_host_is_spi(mmc))
>>     mmc->ocr = cmd.response[0];
>> else
>>     mmc->ocr = mmc->op_cond_response;
>> "
>> Thanks for correcting me.
> 
> Well, there can be several ways to correct that. The easiest would be
> something
> similar to what you propose, but, just to avoid extra "if", we could add
>    mmc->op_cond_response = cmd.response[0];
> to the end of existing "if(mmc_host_is_spi(mmc))" and change
>    mmc->ocr = cmd.response[0];
> to
>    mmc->ocr = mmc->op_cond_response;
> at the end of function. Since op_cond_response should be already set from
> the function beginning, this can be used immediately.
> 
> And, going further, since op_cond_response is actually the same contents
> as mmc->ocr, we could combine them and use mmc->ocr at once, from
> the beginning of polling loops. This is a little more complex, but makes
> the code cleaner. This is what is done in one of other patches in my series
> ;-)
> 

This does seem like a case where a simple accessor structure would help until
we figure out how to process.

Something like mmc_get_ocr() as a private API perhaps?

> Thanks.
> 
> Best regards,
> Andrew
> 

Regards

— Pantelis

> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot