public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for OCR only if it is still not ready
Date: Tue, 24 Mar 2015 19:33:58 +0300	[thread overview]
Message-ID: <000001d06650$54901550$fdb03ff0$@mentor.com> (raw)
In-Reply-To: <551072D8.4090802@boundarydevices.com>

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

  reply	other threads:[~2015-03-24 16:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-23  7:38 [U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for OCR only if it is still not ready Andrew Gabbasov
2015-03-23 20:08 ` Troy Kisky
2015-03-24 16:33   ` Andrew Gabbasov [this message]
2015-03-24 18:03     ` Troy Kisky
2015-03-24 19:02       ` Andrew Gabbasov
2015-05-05  9:05         ` Pantelis Antoniou
  -- strict thread matches above, loose matches on Subject: below --
2015-03-20  7:19 Andrew Gabbasov
2015-03-19 12:44 [U-Boot] [PATCH 0/6] mmc: Fix OCR polling and splitted initialization Andrew Gabbasov
2015-03-19 12:44 ` [U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for OCR only if it is still not ready Andrew Gabbasov
2015-03-20  1:51   ` Peng.Fan at freescale.com
2015-03-20 18:38     ` Troy Kisky
2015-05-05  9:09   ` Pantelis Antoniou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='000001d06650$54901550$fdb03ff0$@mentor.com' \
    --to=andrew_gabbasov@mentor.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox