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

[Andrew Gabbasov]

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.

Best regards,
Andrew