[U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for OCR only if it is still not ready

[Troy Kisky]

On 3/24/2015 9:33 AM, Andrew Gabbasov wrote:
> Hi Troy,
> 
>> From: Troy Kisky [mailto:troy.kisky at boundarydevices.com]
>> Sent: Monday, March 23, 2015 11:09 PM
>> To: Gabbasov, Andrew; Peng.Fan at freescale.com; u-boot at lists.denx.de
>> Cc: Eric Nelson
>> Subject: Re: [U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for OCR
>> only if it is still not ready
>>
>> On 3/23/2015 12:38 AM, Andrew Gabbasov wrote:
>>> Hi Troy,
>>>
>>>> From: Troy Kisky [mailto:troy.kisky at boundarydevices.com]
>>>> Sent: Friday, March 20, 2015 9:39 PM
>>>> To: Peng.Fan at freescale.com; Gabbasov, Andrew; u-boot at lists.denx.de
>>>> Cc: Eric Nelson
>>>> Subject: Re: [U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for
>>>> OCR only if it is still not ready
>>>>
>>>> [skipped]
>>>>
>>>> Here's another patch that solves the problem a little earlier. It has
>>>> this disadvantage of being slightly bigger, though it makes the code
>>>> look
>>> better.
>>>>
>>>> https://github.com/boundarydevices/u-boot-imx6/commit/c0260ca
>>>>
>>>
>>> I have a couple of doubts regarding that patch.
>>>
>>> First, my personal taste objects to such duplicating of the code (I
>>> mean setting of version, ocr, rca, etc fields of mmc structure).
>>> If we'll have to change or add anything to these settings, we'll have
>>> to make the same change in 2 different place, which is error-prone and
>>> extremely inconvenient from maintenance point of view.
>>>
>>> Second, what about SPI mode? Doesn't this patch skip retrieving of OCR
>>> register with a special command for SPI host case (thus setting ocr
>>> field incorrectly), if the card comes to ready state with the first
>>> attempt?
>>
>> That's a good argument for a subroutine to be doing that work instead of
> in two
>> places.
> 
> In some sense the second function of these two (mmc_complete_op_cond())
> is exactly such subroutine ;-) It is doing this work of final settings and
> actions
> after making OCR polling. Although completing the polling itself
> in some cases too.
> Actually, it seems that introducing of one more service function would
> make the code a little too "chopped" into too small pieces, and also
> further less similar to SD (as opposed to MMC) case.
> 
> Thanks.
> 


I've already said that I'm fine with any patch that fixes the problem, so there is
no need to convince me of anything. I just wanted to show that setting the pending
flag, when the command is actually complete, does not make a lot of sense.

Thanks
Troy