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] mmc: fix OCR Polling
Date: Mon, 23 Mar 2015 11:23:25 +0300	[thread overview]
Message-ID: <000101d06542$a2a45530$e7ecff90$@mentor.com> (raw)
In-Reply-To: <550D48FE.1040205@freescale.com>

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
;-)

Thanks.

Best regards,
Andrew

  reply	other threads:[~2015-03-23  8:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-20  7:19 [U-Boot] [PATCH] mmc: fix OCR Polling Andrew Gabbasov
2015-03-21 10:33 ` Peng Fan
2015-03-23  8:23   ` Andrew Gabbasov [this message]
2015-03-27 18:31     ` Pantelis Antoniou
2015-03-30 11:15       ` Andrew Gabbasov
  -- strict thread matches above, loose matches on Subject: below --
2015-03-19  8:22 Peng Fan
2015-05-05  9:00 ` 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='000101d06542$a2a45530$e7ecff90$@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