On Wed, May 9, 2012 at 7:36 PM, <kdorfman@xxxxxxxxxxxxxx> wrote:
>> 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?
>
It's a completely different issue that I had informed Saugata, who is
doing the rework on
power off notify and sleep command sequence.
Essentially, the card shouldn't be put to sleep once BKOPS_START has
been issued,
and we should need to interrupt with HPI only when a higher layer
request needs to be serviced.
I don't know whether you are interested in handling only urgent BKOPS
or periodic BKOPS,
but we can discuss the design on a separate thread.
>>
>>
>>>> +
>>>> + 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
