Re: [PATCH v2] mmc: core: Fix the HPI execution sequence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 18, 2012 at 10:15 AM, Namjae Jeon <linkinjeon@xxxxxxxxx> wrote:
> 2012/4/18 Jaehoon Chung <jh80.chung@xxxxxxxxxxx>:
>> On 04/18/2012 09:20 AM, Namjae Jeon wrote:
>>
>>> 2012/4/17 Venkatraman S <svenkatr@xxxxxx>:
>>>> mmc_execute_hpi should send the HPI command only
>>>> once, 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.
>>> Hi. Venkatraman.
>>> I can not find this words. " the command's completion time is not
>>> dependent on OUT_OF_INTERRUPT_TIME" .
>>> Would you inform me which page and which part you checked in specification ?
>>
>> Well, i know that timeout value is used the OUT_OF_INTERRUPT_TIME, when interrupted by HPI.
>> It's my misunderstanding?
> I agree, I also understand with you. But we should hear Venkatraman's
> opinion from next reply.
> see the spec.

That particular line was explicit, *emphasis* mine..
In Section 6.8.2 "OUT_OF_INTERRUPT_TIME defines the maximum time
between the *end* bit of CMD12/13, arg[0]=1 to the DAT0 release of the
device."

Which essentially means the timer should start after the HPI command
has been exchanged, and should normally end when the DAT0 line is
released (in other words, move out of PRG state).
You can see the same definition in Section 7.4.33

The definition in 6.6.23, is partly confusing, for one it uses
OUT_OF_INTERRUPT_BUSY_TIME and not OUT_OF_INTERRUPT_TIME, and other,
it refers to the command being *interrupted by* HPI, not the HPI
command itself.

Let me know if this explains it a bit better.

Best regards,
Venkat.
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> HPI command is accepted as a legal command in prg-state. However, only
> some commands may be interrupted by HPI. If HPI is received during
> commands which are not interruptible, a response is sent but the HPI
> command has no effect and the original command is completed normally,
> possibly exceeding the OUT_OF_INTERRUPT_TIME timeout.
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> we can know OUT_OF_INTERRUPT_TIME timeout can be exceeded when
> commands is not interrupted by HPI.
> Also I think that timeout should be operated by host driver as current code.
> But I have a question while checking spec.
> Currently, when sending HPI command, MMC_RSP_R1 is set to flags.
> When setting MMC_RSP_R1B, does host driver set timeout value from mmc core  ?
>
> when I check stop transmission in spec, R1B response should be set in
> write cases.
> So MMC_RSP_R1B should be set to flags in HPI stop transmission instead
> of MMC_RSP_R1 ?
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> Abbreviation : STOP_ TRANSMISSION
> response : R1/ R1b4
> NOTE 4 R1 for read cases and R1b for write cases.
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> Thanks.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Thanks.
>>>>
>>>> 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.
>>>>
>>>> Reported-by: Alex Lemberg <Alex.Lemberg@xxxxxxxxxxx>
>>>> Signed-off-by: Venkatraman S <svenkatr@xxxxxx>
>>>> CC: Namjae Jeon <linkinjeon@xxxxxxxxx>
>>>> CC: Jae hoon Chung <jh80.chung@xxxxxxxxx>
>>>> CC: Chris Ball <cjb@xxxxxxxxxx>
>>>> ---
>>>> Changes since v1:
>>>>        Skip looping over send_status indefinitely
>>>>        Correct the interpretation of OUT_OF_INTERRUPT_TIME
>>>>
>>>>  drivers/mmc/core/core.c    |   45 ++++++++++++++++++++++++--------------------
>>>>  drivers/mmc/core/mmc_ops.c |    2 +-
>>>>  2 files changed, 26 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index e541efb..027c579 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -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;
>>>> +       }
>>>> +
>>>> +       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)
>>>> +                       err = -ETIMEDOUT;
>>>> +       } while (!err);
>>>>
>>>> -                       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));
>>>>
>>>>  out:
>>>>        mmc_release_host(card->host);
>>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>>> index 69370f4..f2235d6 100644
>>>> --- a/drivers/mmc/core/mmc_ops.c
>>>> +++ b/drivers/mmc/core/mmc_ops.c
>>>> @@ -569,7 +569,7 @@ 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;
>>>> +       cmd.cmd_timeout_ms = 0;
>>>>
>>>>        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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux