On Wed, Jun 20, 2012 at 11:29 PM, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> wrote: > Hi Venkatraman, > >> -----Original Message----- >> From: linux-mmc-owner@xxxxxxxxxxxxxxx [mailto:linux-mmc- >> owner@xxxxxxxxxxxxxxx] On Behalf Of Venkatraman S >> Sent: Wednesday, June 20, 2012 3:04 PM >> To: cjb@xxxxxxxxxx >> Cc: linux-mmc@xxxxxxxxxxxxxxx; linkinjeon@xxxxxxxxx; >> jh80.chung@xxxxxxxxxxx; alex.lemberg@xxxxxxxxxxx; Venkatraman S; Kostya >> Subject: [PATCH v3] mmc: core: Fix the HPI execution sequence >> >> mmc_execute_hpi should send the HPI command only once, and only if the >> card is in PRG state. >> >> According to eMMC spec, the command's completion time is not dependent on >> OUT_OF_INTERRUPT_TIME. Only the transition out of PRG STATE is guarded by >> OUT_OF_INTERRUPT_TIME - which is defined to begin at the end of sending >> the command itself. >> >> Specify the default timeout for the actual sending of HPI command, and > then >> use OUT_OF_INTERRUPT_TIME to wait for the transition out of PRG state. > > I guess this is not the correct interpretation. cmd.cmd_timeout_ms should > specify for how much time host driver should wait for command + busy line > deassertion. So why are you keeping the timeout calculation after the > command is completed? > If what you stay is correct, the card should be in PRG state after mmc_send_hpi_cmd() returns. I understood and observed it as opposite. HPI cmd returns immediately, and the device takes OUT_OF_INTERRUPT_TIME time to come out of PRG state. My experiments / tests confirm that behavior. It takes a few ms (and several SEND_STATUS messages later), after HPI, for the card to be out of PRG state. > mmc_send_hpi_cmd() returns means the host controller driver have ensured > that BUSY indication is removed so after mmc_send_hpi_cmd(), if you want to > check if the card is still in PROGRAMMING state, it should be unconditional > check in loop (rather than timeout based) until card is out of programming > state. > > To me, mmc_send_hpi_cmd() implementation should not be anything different > than mmc_switch(). In mmc_switch() also, after mmc_wait_for_cmd() returns it > continuously keeps checking if card is programming or not. > > Regards, > Subhash > >> >> Reported-by: Alex Lemberg <Alex.Lemberg@xxxxxxxxxxx> >> Signed-off-by: Venkatraman S <svenkatr@xxxxxx> >> Reviewed-by: Namjae Jeon <linkinjeon@xxxxxxxxx> >> Acked-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx> >> CC: Kostya <kdorfman@xxxxxxxxxxxxxx> >> --- >> v2 discussions: http://marc.info/?l=linux-mmc&m=133657255616390&w=2 >> >> v2->v3: >> Return gracefully for card idle states (suggested by kdorfman) >> >> drivers/mmc/core/core.c | 57 > ++++++++++++++++++++++++++----------------- >> - >> drivers/mmc/core/mmc_ops.c | 1 - >> 2 files changed, 34 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index >> 80ec427..08ed144 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -404,6 +404,7 @@ int mmc_interrupt_hpi(struct mmc_card *card) { >> int err; >> u32 status; >> + unsigned long starttime; >> >> BUG_ON(!card); >> >> @@ -419,30 +420,40 @@ int mmc_interrupt_hpi(struct mmc_card *card) >> goto out; >> } >> >> - /* >> - * If the card status is in PRG-state, we can send the HPI command. >> - */ >> - if (R1_CURRENT_STATE(status) == R1_STATE_PRG) { >> - do { >> - /* >> - * We don't know when the HPI command will finish >> - * processing, so we need to resend HPI until out >> - * of prg-state, and keep checking the card status >> - * with SEND_STATUS. If a timeout error occurs when >> - * sending the HPI command, we are already out of >> - * prg-state. >> - */ >> - err = mmc_send_hpi_cmd(card, &status); >> - if (err) >> - pr_debug("%s: abort HPI (%d error)\n", >> - mmc_hostname(card->host), err); >> + switch (R1_CURRENT_STATE(status)) { >> >> - err = mmc_send_status(card, &status); >> - if (err) >> - break; >> - } while (R1_CURRENT_STATE(status) == R1_STATE_PRG); >> - } else >> - pr_debug("%s: Left prg-state\n", mmc_hostname(card->host)); >> + case R1_STATE_IDLE: >> + case R1_STATE_READY: >> + case R1_STATE_STBY: /* intentional fall through */ >> + /* In idle states, HPI is not needed and the caller >> + can issue the next intended command immediately */ >> + goto out; >> + break; >> + case R1_STATE_PRG: >> + break; >> + default: >> + /* In all other states, it's illegal to issue HPI */ >> + pr_debug("%s: HPI cannot be sent. Card state=%d\n", >> + mmc_hostname(card->host), >> R1_CURRENT_STATE(status)); >> + err = -EINVAL; >> + goto out; >> + break; >> + } >> + >> + starttime = jiffies; >> + err = mmc_send_hpi_cmd(card, &status); >> + if (err) >> + goto out; >> + >> + do { >> + err = mmc_send_status(card, &status); >> + >> + if (!err && R1_CURRENT_STATE(status) == R1_STATE_TRAN) >> + break; >> + if (jiffies_to_msecs(jiffies - starttime) >= >> + card->ext_csd.out_of_int_time) >> + err = -ETIMEDOUT; >> + } while (!err); >> >> out: >> mmc_release_host(card->host); >> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >> index 69370f4..0ed2cc5 100644 >> --- a/drivers/mmc/core/mmc_ops.c >> +++ b/drivers/mmc/core/mmc_ops.c >> @@ -569,7 +569,6 @@ int mmc_send_hpi_cmd(struct mmc_card *card, u32 >> *status) >> >> cmd.opcode = opcode; >> cmd.arg = card->rca << 16 | 1; >> - cmd.cmd_timeout_ms = card->ext_csd.out_of_int_time; >> >> err = mmc_wait_for_cmd(card->host, &cmd, 0); >> if (err) { >> -- >> 1.7.10.rc2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body >> of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at >> http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
