[PATCH 0/4] mmc: core and queue cleanups

[PATCH 0/4] mmc: core and queue cleanups

[Venkatraman S]

	The first 3 are straight forward / trivial fixes.
The last one is a functional change on how HPI should be invoked
on the card. This is required for my future foreground HPI series
that'll follow.

Also available at git://github.com/svenkatr/linux.git my/mmc/cleanups-v2

Venkatraman S (4):
  mmc: queue: rename mmc_request function
  mmc: include cd-gpio.h in the source file
  mmc: queue: remove redundant memsets
  mmc: core: Send HPI only till it is successful

 drivers/mmc/card/queue.c   |    6 ++----
 drivers/mmc/core/cd-gpio.c |    1 +
 drivers/mmc/core/core.c    |   34 ++++++++++++++++++----------------
 3 files changed, 21 insertions(+), 20 deletions(-)

-- 
1.7.10.rc2

[PATCH 1/4] mmc: queue: rename mmc_request function

[Venkatraman S]

The name mmc_request is used for both the issue function
and a data structure, which creates conflicts in symbol lookups
in editors. Rename the function to mmc_request_fn

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/card/queue.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 996f8e3..49af43c 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -96,7 +96,7 @@ static int mmc_queue_thread(void *d)
  * on any queue on this host, and attempt to issue it.  This may
  * not be the queue we were asked to process.
  */
-static void mmc_request(struct request_queue *q)
+static void mmc_request_fn(struct request_queue *q)
 {
 	struct mmc_queue *mq = q->queuedata;
 	struct request *req;
@@ -171,7 +171,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
 		limit = *mmc_dev(host)->dma_mask;
 
 	mq->card = card;
-	mq->queue = blk_init_queue(mmc_request, lock);
+	mq->queue = blk_init_queue(mmc_request_fn, lock);
 	if (!mq->queue)
 		return -ENOMEM;
 
-- 
1.7.10.rc2

[PATCH 2/4] mmc: include cd-gpio.h in the source file

[Venkatraman S]

Include the corresponding header file in cd-gpio.c
for prototype consistency. This gets rid of some
sparse warnings.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---

 drivers/mmc/core/cd-gpio.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/cd-gpio.c b/drivers/mmc/core/cd-gpio.c
index 29de31e..da7c3f9 100644
--- a/drivers/mmc/core/cd-gpio.c
+++ b/drivers/mmc/core/cd-gpio.c
@@ -13,6 +13,7 @@
 #include <linux/interrupt.h>
 #include <linux/jiffies.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/cd-gpio.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
-- 
1.7.10.rc2

[PATCH 3/4] mmc: queue: remove redundant memsets

[Venkatraman S]

Not needed to memset, as they are pointers and are assigned
to proper values in the next line anyway.

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/card/queue.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 49af43c..e360a97 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -175,8 +175,6 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
 	if (!mq->queue)
 		return -ENOMEM;
 
-	memset(&mq->mqrq_cur, 0, sizeof(mq->mqrq_cur));
-	memset(&mq->mqrq_prev, 0, sizeof(mq->mqrq_prev));
 	mq->mqrq_cur = mqrq_cur;
 	mq->mqrq_prev = mqrq_prev;
 	mq->queue->queuedata = mq;
-- 
1.7.10.rc2

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

[Venkatraman S]

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

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

[Namjae Jeon]

Hi. Venkatraman.

You fixed 10 times. why is it 10 times ?
and checking err from mmc_send_status is not needed ? is it  also
infinite case ?

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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[S, Venkatraman]

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.

> 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.

>
> 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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Namjae Jeon]

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.
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
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jae hoon Chung]

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 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...

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
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[S, Venkatraman]

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
>>>>>

Re: [PATCH 3/4] mmc: queue: remove redundant memsets

[Namjae Jeon]

2012/4/13 Venkatraman S <svenkatr@ti.com>:
> Not needed to memset, as they are pointers and are assigned
> to proper values in the next line anyway.
>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
Looks good!!
Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>
Thanks.

Re: [PATCH 1/4] mmc: queue: rename mmc_request function

[Namjae Jeon]

2012/4/13 Venkatraman S <svenkatr@ti.com>:
> The name mmc_request is used for both the issue function
> and a data structure, which creates conflicts in symbol lookups
> in editors. Rename the function to mmc_request_fn
>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
Looks reasonable to me~
Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>
Thanks.

> ---

Re: [PATCH 2/4] mmc: include cd-gpio.h in the source file

[Namjae Jeon]

2012/4/13 Venkatraman S <svenkatr@ti.com>:
> Include the corresponding header file in cd-gpio.c
> for prototype consistency. This gets rid of some
> sparse warnings.
>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
This patch is same with H Hartley Sweeten's patch ([PATCH v2] mmc:
cd-gpio.c: Include header to pickup exported symbol prototypes)
Thanks.

> ---
>

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

[Chris Ball]

Hi,

On Fri, Apr 13 2012, Venkatraman S wrote:
> 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.
> +		 */

Minor nit: for consistency, please always use:

                /*
                 * Kernel-style comment blocks.
                 */

.. even though they waste the first line.  :)

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

[S, Venkatraman]

On Thu, Apr 19, 2012 at 9:59 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Fri, Apr 13 2012, Venkatraman S wrote:
>> 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.
>> +              */
>
> Minor nit: for consistency, please always use:
>
>                /*
>                 * Kernel-style comment blocks.
>                 */
>
> .. even though they waste the first line.  :)
>
Thanks - I'll be more careful - but that comment is not present on the
last version I posted.

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

[kdorfman]

Hi,
I see 2 fixes for existing mmc_interrupt_hpi() function:
-  [PATCH 4/4] mmc: core: Send HPI only till it is successful"
-  [PATCH v2] mmc: core: Fix the HPI execution sequence

They are exclusive. Can you explain how final solution will look like?
Thanks,
Kostya

> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[S, Venkatraman]

On Wed, May 9, 2012 at 6:52 PM,  <kdorfman@codeaurora.org> wrote:
> Hi,
> I see 2 fixes for existing mmc_interrupt_hpi() function:
> -  [PATCH 4/4] mmc: core: Send HPI only till it is successful"
> -  [PATCH v2] mmc: core: Fix the HPI execution sequence
>
> They are exclusive. Can you explain how final solution will look like?
This one is old. After some review, I did fix the subject to reflect
the changes.
IOW, the latest one is the "mmc: core: Fix the HPI execution sequence" patch.

Regards,
Venkat.

> Thanks,
> Kostya
>
>> 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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
>
>
>

Re: [PATCH 0/4] mmc: core and queue cleanups

[Chris Ball]

Hi Venkat,

On Fri, Apr 13 2012, Venkatraman S wrote:
> 	The first 3 are straight forward / trivial fixes.
> The last one is a functional change on how HPI should be invoked
> on the card. This is required for my future foreground HPI series
> that'll follow.
>
> Also available at git://github.com/svenkatr/linux.git my/mmc/cleanups-v2
>
> Venkatraman S (4):
>   mmc: queue: rename mmc_request function
>   mmc: include cd-gpio.h in the source file
>   mmc: queue: remove redundant memsets
>   mmc: core: Send HPI only till it is successful

I've pushed patches 1 and 3 to mmc-next for 3.5; 2 is already applied
via H Hartley Sweeten, and I'm waiting on a final resend of 4.

Thanks!

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 0/4] mmc: core and queue cleanups

[S, Venkatraman]

On Wed, May 9, 2012 at 7:41 PM, Chris Ball <cjb@laptop.org> wrote:
> Hi Venkat,
>
> On Fri, Apr 13 2012, Venkatraman S wrote:
>>       The first 3 are straight forward / trivial fixes.
>> The last one is a functional change on how HPI should be invoked
>> on the card. This is required for my future foreground HPI series
>> that'll follow.
>>
>> Also available at git://github.com/svenkatr/linux.git my/mmc/cleanups-v2
>>
>> Venkatraman S (4):
>>   mmc: queue: rename mmc_request function
>>   mmc: include cd-gpio.h in the source file
>>   mmc: queue: remove redundant memsets
>>   mmc: core: Send HPI only till it is successful
>
> I've pushed patches 1 and 3 to mmc-next for 3.5; 2 is already applied
> via H Hartley Sweeten, and I'm waiting on a final resend of 4.
>

Thanks Chris !! Actually 4 is superseded by another patch with a
different subject -
"mmc: core: fix HPI execution sequence". I had sent the most correct
version to you - but meanwhile
it is receiving further reviews as I type this. I'll check and send a
final version after answering the
questions.

~Venkat.

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

[Philip Rakity]

On Apr 13, 2012, at 7:39 AM, Namjae Jeon wrote:

> Hi. Venkatraman.
> 
> You fixed 10 times. why is it 10 times ?
> and checking err from mmc_send_status is not needed ? is it  also
> infinite case ?
> 
> 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++)


would it make more sense to check time as well as retry count ?
worry that count may be too short or behavior is dependent on card manufacturer.


>>                        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
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{��g"��^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�m�

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

[S, Venkatraman]

On Wed, May 9, 2012 at 8:20 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> On Apr 13, 2012, at 7:39 AM, Namjae Jeon wrote:
>
>> Hi. Venkatraman.
>>
>> You fixed 10 times. why is it 10 times ?
>> and checking err from mmc_send_status is not needed ? is it  also
>> infinite case ?
>>
>> 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++)
>
>
> would it make more sense to check time as well as retry count ?
> worry that count may be too short or behavior is dependent on card manufacturer.
>
This patch is discarded. Let me know if you have some comments on the
follow up patch with
the subject "mmc: fix HPI execution sequence"

>
>>>                        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
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html