> On Wed, May 9, 2012 at 6:47 PM, <kdorfman@xxxxxxxxxxxxxx> wrote: >>> @@ -403,6 +403,7 @@ int mmc_interrupt_hpi(struct mmc_card *card) >>> { >>> int err; >>> u32 status; >>> + unsigned long starttime; >>> >>> BUG_ON(!card); >>> >>> @@ -421,27 +422,31 @@ int mmc_interrupt_hpi(struct mmc_card *card) >>> /* >>> * 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); >>> + if (R1_CURRENT_STATE(status) != R1_STATE_PRG) { >>> + pr_debug("%s: Can't send HPI: Card state=%d\n", >>> + mmc_hostname(card->host), >>> R1_CURRENT_STATE(status)); >>> + err = -EINVAL; >>> + goto out; >>> + } >> You can't return error here, because the card status may be: >> 0 = Idle 1 = Ready 2 = Ident 3 = Stby 4 = Tran 5 = Data 6 = Rcv 7 = Prg >> 8 >> = Dis 9 = Btst 10 >> Not every value is error here: the card state may be Idle/Ready and then >> you do not need to return error. >> > Depends. When the intent is to send HPI command, and the current state > is such that it can't be sent, > then a error has to be returned, right ? > OTOH, if the API was "mmc_get_to_transfer_state()", then it would make > sense to not return an error. I'm working on HPI commmand that stops running BKOPS, when runtime suspend flow is triggered. Right now, the code is not correct and CMD5 (sleep) sent to the card without any check for running BKOPS. In this case card may be in any state. Do you think we need to wait/check state before calling mmc_interrupt_hpi() function? > > >>> + >>> + starttime = jiffies; >>> + err = mmc_send_hpi_cmd(card, &status); >>> + if (err) { >>> + pr_debug("%s:HPI could not be sent.err=%d)\n", >>> + mmc_hostname(card->host), err); >>> + goto out; >>> + } >>> + >>> + do { >>> + err = mmc_send_status(card, &status); >>> + >>> + if (!err && R1_CURRENT_STATE(status) == R1_STATE_TRAN) >>> + goto out; >>> + if (jiffies_to_msecs(jiffies - starttime) > >>> + card->ext_csd.out_of_int_time) I think we need to add some margin time to the out_of_int_time here. Thanks, Kostya Consultant for Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum -- 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