Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful

["S, Venkatraman" <svenkatr@ti.com>]

On Sun, Apr 15, 2012 at 8:40 PM, Jae hoon Chung <jh80.chung@gmail.com> wrote:
> Hi Venkatraman
>
> 2012/4/14 Namjae Jeon <linkinjeon@gmail.com>:
>> 2012/4/14 S, Venkatraman <svenkatr@ti.com>:
>>> On Fri, Apr 13, 2012 at 8:09 PM, Namjae Jeon <linkinjeon@gmail.com> wrote:
>>>> Hi. Venkatraman.
>>>>
>>>> You fixed 10 times. why is it 10 times ?
>>> There's no right number - I haven't seen it fail much in my tests but
>>> should allow a few retries.
>> Is there the reason should send HPI a few retries ?
>>>
>>>> and checking err from mmc_send_status is not needed ? is it  also
>>>> infinite case ?
>>> Check again - the loop will break if the send_status returns an error
>>> (even once), or if the card comes out of PRG state.
> Well, if hpi command is successful, that loop should be break.
> Already out-of prg-state.
But that would happen anyway if the card is out of prg state.

> But if hpi command is failed, just waiting for out-of prg-state?
> why wait for out-of prg-state?...if state is prg-state, hpi-cmd is
> already failed..?
> then...i think that need not to wait for out-of prg-state..just return failed...
It's already happening with the current patch.

But from what I understand from NJ and you, it doesn't make sense to
retry sending HPI command multiple times. I'll update the patch to send
it only once.


>
> Best Regards,
> Jaehoon Chung
>> okay, but this patch print "abort HPI" although card_status is fail.
>> if print abort HPI fail, Can we know which error occurred in either
>> send hpi cmd error and send status error ?
>> Thanks.
>>>
>>>>
>>>> Thanks.
>>>>
>>>> 2012/4/13 Venkatraman S <svenkatr@ti.com>:
>>>>> Try to send HPI only a fixed number of times till it is
>>>>> successful. One successful transfer is enough - but wait
>>>>> till the card comes out of transfer state.
>>>>> Return an error if the card was not in programming state to
>>>>> begin with - so that the caller knows that HPI was not sent.
>>>>>
>>>>> Reported-by: Alex Lemberg <Alex.Lemberg@sandisk.com>
>>>>> Signed-off-by: Venkatraman S <svenkatr@ti.com>
>>>>> ---
>>>>>  drivers/mmc/core/core.c |   34 ++++++++++++++++++----------------
>>>>>  1 file changed, 18 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index e541efb..ceabef5 100644
>>>>> --- a/drivers/mmc/core/core.c
>>>>> +++ b/drivers/mmc/core/core.c
>>>>> @@ -401,7 +401,7 @@ EXPORT_SYMBOL(mmc_wait_for_req);
>>>>>  */
>>>>>  int mmc_interrupt_hpi(struct mmc_card *card)
>>>>>  {
>>>>> -       int err;
>>>>> +       int err, i;
>>>>>        u32 status;
>>>>>
>>>>>        BUG_ON(!card);
>>>>> @@ -421,27 +421,29 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>>>>>        /*
>>>>>         * If the card status is in PRG-state, we can send the HPI command.
>>>>>         */
>>>>> +       err = -EINVAL;
>>>>>        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.
>>>>> -                        */
>>>>> +               /* To prevent an infinite lockout, try to send HPI
>>>>> +                * a fixed number of times and bailout if it can't
>>>>> +                * succeed.
>>>>> +                */
>>>>> +               for (i = 0; i < 10 && err != 0 ; i++)
>>>>>                        err = mmc_send_hpi_cmd(card, &status);
>>>>> -                       if (err)
>>>>> +               /* Once HPI was sent successfully, the card is
>>>>> +                * supposed to be back to transfer state.
>>>>> +                */
>>>>> +               do {
>>>>> +                       if (err) {
>>>>>                                pr_debug("%s: abort HPI (%d error)\n",
>>>>>                                         mmc_hostname(card->host), err);
>>>>> -
>>>>> -                       err = mmc_send_status(card, &status);
>>>>> -                       if (err)
>>>>>                                break;
>>>>> +                       }
>>>>> +                       err = mmc_send_status(card, &status);
>>>>>                } while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
>>>>> -       } else
>>>>> -               pr_debug("%s: Left prg-state\n", mmc_hostname(card->host));
>>>>> +       } else {
>>>>> +               pr_debug("%s: Can't send HPI. State=%d\n",
>>>>> +                       mmc_hostname(card->host), R1_CURRENT_STATE(status));
>>>>> +       }
>>>>>
>>>>>  out:
>>>>>        mmc_release_host(card->host);
>>>>> --
>>>>> 1.7.10.rc2
>>>>>