* [PATCH 0/4] mmc: core and queue cleanups
@ 2012-04-13 12:24 Venkatraman S
2012-04-13 12:24 ` [PATCH 1/4] mmc: queue: rename mmc_request function Venkatraman S
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Venkatraman S @ 2012-04-13 12:24 UTC (permalink / raw)
To: cjb, linux-mmc; +Cc: 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/4] mmc: queue: rename mmc_request function
2012-04-13 12:24 [PATCH 0/4] mmc: core and queue cleanups Venkatraman S
@ 2012-04-13 12:24 ` Venkatraman S
2012-04-18 23:04 ` Namjae Jeon
2012-04-13 12:24 ` [PATCH 2/4] mmc: include cd-gpio.h in the source file Venkatraman S
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Venkatraman S @ 2012-04-13 12:24 UTC (permalink / raw)
To: cjb, linux-mmc; +Cc: 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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/4] mmc: include cd-gpio.h in the source file
2012-04-13 12:24 [PATCH 0/4] mmc: core and queue cleanups Venkatraman S
2012-04-13 12:24 ` [PATCH 1/4] mmc: queue: rename mmc_request function Venkatraman S
@ 2012-04-13 12:24 ` Venkatraman S
2012-04-18 23:08 ` Namjae Jeon
2012-04-13 12:24 ` [PATCH 3/4] mmc: queue: remove redundant memsets Venkatraman S
` (2 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Venkatraman S @ 2012-04-13 12:24 UTC (permalink / raw)
To: cjb, linux-mmc; +Cc: 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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] mmc: queue: remove redundant memsets
2012-04-13 12:24 [PATCH 0/4] mmc: core and queue cleanups Venkatraman S
2012-04-13 12:24 ` [PATCH 1/4] mmc: queue: rename mmc_request function Venkatraman S
2012-04-13 12:24 ` [PATCH 2/4] mmc: include cd-gpio.h in the source file Venkatraman S
@ 2012-04-13 12:24 ` Venkatraman S
2012-04-18 23:02 ` Namjae Jeon
2012-04-13 12:24 ` [PATCH 4/4] mmc: core: Send HPI only till it is successful Venkatraman S
2012-05-09 14:11 ` [PATCH 0/4] mmc: core and queue cleanups Chris Ball
4 siblings, 1 reply; 21+ messages in thread
From: Venkatraman S @ 2012-04-13 12:24 UTC (permalink / raw)
To: cjb, linux-mmc; +Cc: 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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 12:24 [PATCH 0/4] mmc: core and queue cleanups Venkatraman S
` (2 preceding siblings ...)
2012-04-13 12:24 ` [PATCH 3/4] mmc: queue: remove redundant memsets Venkatraman S
@ 2012-04-13 12:24 ` Venkatraman S
2012-04-13 14:39 ` Namjae Jeon
` (2 more replies)
2012-05-09 14:11 ` [PATCH 0/4] mmc: core and queue cleanups Chris Ball
4 siblings, 3 replies; 21+ messages in thread
From: Venkatraman S @ 2012-04-13 12:24 UTC (permalink / raw)
To: cjb, linux-mmc; +Cc: 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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 12:24 ` [PATCH 4/4] mmc: core: Send HPI only till it is successful Venkatraman S
@ 2012-04-13 14:39 ` Namjae Jeon
2012-04-13 15:58 ` S, Venkatraman
2012-05-09 14:50 ` Philip Rakity
2012-04-19 4:29 ` Chris Ball
2012-05-09 13:22 ` kdorfman
2 siblings, 2 replies; 21+ messages in thread
From: Namjae Jeon @ 2012-04-13 14:39 UTC (permalink / raw)
To: Venkatraman S; +Cc: cjb, linux-mmc
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 14:39 ` Namjae Jeon
@ 2012-04-13 15:58 ` S, Venkatraman
2012-04-14 0:35 ` Namjae Jeon
2012-05-09 14:50 ` Philip Rakity
1 sibling, 1 reply; 21+ messages in thread
From: S, Venkatraman @ 2012-04-13 15:58 UTC (permalink / raw)
To: Namjae Jeon; +Cc: cjb, linux-mmc
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 15:58 ` S, Venkatraman
@ 2012-04-14 0:35 ` Namjae Jeon
2012-04-15 15:10 ` Jae hoon Chung
0 siblings, 1 reply; 21+ messages in thread
From: Namjae Jeon @ 2012-04-14 0:35 UTC (permalink / raw)
To: S, Venkatraman; +Cc: cjb, linux-mmc
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-14 0:35 ` Namjae Jeon
@ 2012-04-15 15:10 ` Jae hoon Chung
2012-04-17 12:06 ` S, Venkatraman
0 siblings, 1 reply; 21+ messages in thread
From: Jae hoon Chung @ 2012-04-15 15:10 UTC (permalink / raw)
To: Namjae Jeon; +Cc: S, Venkatraman, cjb, linux-mmc, Jaehoon 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-15 15:10 ` Jae hoon Chung
@ 2012-04-17 12:06 ` S, Venkatraman
0 siblings, 0 replies; 21+ messages in thread
From: S, Venkatraman @ 2012-04-17 12:06 UTC (permalink / raw)
To: Jae hoon Chung; +Cc: Namjae Jeon, cjb, linux-mmc, Jaehoon Chung
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
>>>>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] mmc: queue: remove redundant memsets
2012-04-13 12:24 ` [PATCH 3/4] mmc: queue: remove redundant memsets Venkatraman S
@ 2012-04-18 23:02 ` Namjae Jeon
0 siblings, 0 replies; 21+ messages in thread
From: Namjae Jeon @ 2012-04-18 23:02 UTC (permalink / raw)
To: Venkatraman S; +Cc: cjb, linux-mmc
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] mmc: queue: rename mmc_request function
2012-04-13 12:24 ` [PATCH 1/4] mmc: queue: rename mmc_request function Venkatraman S
@ 2012-04-18 23:04 ` Namjae Jeon
0 siblings, 0 replies; 21+ messages in thread
From: Namjae Jeon @ 2012-04-18 23:04 UTC (permalink / raw)
To: Venkatraman S; +Cc: cjb, linux-mmc
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.
> ---
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] mmc: include cd-gpio.h in the source file
2012-04-13 12:24 ` [PATCH 2/4] mmc: include cd-gpio.h in the source file Venkatraman S
@ 2012-04-18 23:08 ` Namjae Jeon
0 siblings, 0 replies; 21+ messages in thread
From: Namjae Jeon @ 2012-04-18 23:08 UTC (permalink / raw)
To: Venkatraman S; +Cc: cjb, linux-mmc
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.
> ---
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 12:24 ` [PATCH 4/4] mmc: core: Send HPI only till it is successful Venkatraman S
2012-04-13 14:39 ` Namjae Jeon
@ 2012-04-19 4:29 ` Chris Ball
2012-04-19 4:33 ` S, Venkatraman
2012-05-09 13:22 ` kdorfman
2 siblings, 1 reply; 21+ messages in thread
From: Chris Ball @ 2012-04-19 4:29 UTC (permalink / raw)
To: Venkatraman S; +Cc: linux-mmc
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-19 4:29 ` Chris Ball
@ 2012-04-19 4:33 ` S, Venkatraman
0 siblings, 0 replies; 21+ messages in thread
From: S, Venkatraman @ 2012-04-19 4:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 12:24 ` [PATCH 4/4] mmc: core: Send HPI only till it is successful Venkatraman S
2012-04-13 14:39 ` Namjae Jeon
2012-04-19 4:29 ` Chris Ball
@ 2012-05-09 13:22 ` kdorfman
2012-05-09 13:43 ` S, Venkatraman
2 siblings, 1 reply; 21+ messages in thread
From: kdorfman @ 2012-05-09 13:22 UTC (permalink / raw)
Cc: cjb, linux-mmc, Venkatraman S
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-05-09 13:22 ` kdorfman
@ 2012-05-09 13:43 ` S, Venkatraman
0 siblings, 0 replies; 21+ messages in thread
From: S, Venkatraman @ 2012-05-09 13:43 UTC (permalink / raw)
To: kdorfman; +Cc: cjb, linux-mmc
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
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] mmc: core and queue cleanups
2012-04-13 12:24 [PATCH 0/4] mmc: core and queue cleanups Venkatraman S
` (3 preceding siblings ...)
2012-04-13 12:24 ` [PATCH 4/4] mmc: core: Send HPI only till it is successful Venkatraman S
@ 2012-05-09 14:11 ` Chris Ball
2012-05-09 14:15 ` S, Venkatraman
4 siblings, 1 reply; 21+ messages in thread
From: Chris Ball @ 2012-05-09 14:11 UTC (permalink / raw)
To: Venkatraman S; +Cc: linux-mmc
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] mmc: core and queue cleanups
2012-05-09 14:11 ` [PATCH 0/4] mmc: core and queue cleanups Chris Ball
@ 2012-05-09 14:15 ` S, Venkatraman
0 siblings, 0 replies; 21+ messages in thread
From: S, Venkatraman @ 2012-05-09 14:15 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-04-13 14:39 ` Namjae Jeon
2012-04-13 15:58 ` S, Venkatraman
@ 2012-05-09 14:50 ` Philip Rakity
2012-05-11 10:07 ` S, Venkatraman
1 sibling, 1 reply; 21+ messages in thread
From: Philip Rakity @ 2012-05-09 14:50 UTC (permalink / raw)
To: Namjae Jeon; +Cc: Venkatraman S, cjb, linux-mmc
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�
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] mmc: core: Send HPI only till it is successful
2012-05-09 14:50 ` Philip Rakity
@ 2012-05-11 10:07 ` S, Venkatraman
0 siblings, 0 replies; 21+ messages in thread
From: S, Venkatraman @ 2012-05-11 10:07 UTC (permalink / raw)
To: Philip Rakity; +Cc: Namjae Jeon, cjb, linux-mmc
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
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2012-05-11 10:08 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-13 12:24 [PATCH 0/4] mmc: core and queue cleanups Venkatraman S
2012-04-13 12:24 ` [PATCH 1/4] mmc: queue: rename mmc_request function Venkatraman S
2012-04-18 23:04 ` Namjae Jeon
2012-04-13 12:24 ` [PATCH 2/4] mmc: include cd-gpio.h in the source file Venkatraman S
2012-04-18 23:08 ` Namjae Jeon
2012-04-13 12:24 ` [PATCH 3/4] mmc: queue: remove redundant memsets Venkatraman S
2012-04-18 23:02 ` Namjae Jeon
2012-04-13 12:24 ` [PATCH 4/4] mmc: core: Send HPI only till it is successful Venkatraman S
2012-04-13 14:39 ` Namjae Jeon
2012-04-13 15:58 ` S, Venkatraman
2012-04-14 0:35 ` Namjae Jeon
2012-04-15 15:10 ` Jae hoon Chung
2012-04-17 12:06 ` S, Venkatraman
2012-05-09 14:50 ` Philip Rakity
2012-05-11 10:07 ` S, Venkatraman
2012-04-19 4:29 ` Chris Ball
2012-04-19 4:33 ` S, Venkatraman
2012-05-09 13:22 ` kdorfman
2012-05-09 13:43 ` S, Venkatraman
2012-05-09 14:11 ` [PATCH 0/4] mmc: core and queue cleanups Chris Ball
2012-05-09 14:15 ` S, Venkatraman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.