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