| Message ID | 1527229031-216838-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25 May 2018 at 08:17, Shawn Lin <shawn.lin@rock-chips.com> wrote: > In preparation for reusing mmc_poll_for_busy() to avoid duplication > of code for polling busy. > > No functional change intended. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > > drivers/mmc/core/mmc_ops.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index 88f34fd..4d73db4 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -447,7 +447,8 @@ int mmc_switch_status(struct mmc_card *card) > } > > static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > - bool send_status, bool retry_crc_err, bool use_r1b_resp) > + bool send_status, bool retry_crc_err, bool use_r1b_resp, > + unsigned int retries) What are the use-case for actually being able to use different amount of retries for polling? I am thinking that, we can possibly change the callers of mmc_poll_for_busy() to use the same amount of retries instead, don't you think? > { > struct mmc_host *host = card->host; > int err; > @@ -486,7 +487,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > if (host->ops->card_busy) { > busy = host->ops->card_busy(host); > } else { > - err = mmc_send_status(card, &status); > + err = __mmc_send_status(card, &status, retries); > if (retry_crc_err && err == -EILSEQ) { > busy = true; > } else if (err) { > @@ -578,7 +579,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, > > /* Let's try to poll to find out when the command is completed. */ > err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err, > - use_r1b_resp); > + use_r1b_resp, MMC_CMD_RETRIES); > if (err) > goto out; > > -- > 1.9.1 > > > -- > 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 Kind regards Uffe -- 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
On 2018/5/25 16:42, Ulf Hansson wrote: > On 25 May 2018 at 08:17, Shawn Lin <shawn.lin@rock-chips.com> wrote: >> In preparation for reusing mmc_poll_for_busy() to avoid duplication >> of code for polling busy. >> >> No functional change intended. >> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >> --- >> >> drivers/mmc/core/mmc_ops.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >> index 88f34fd..4d73db4 100644 >> --- a/drivers/mmc/core/mmc_ops.c >> +++ b/drivers/mmc/core/mmc_ops.c >> @@ -447,7 +447,8 @@ int mmc_switch_status(struct mmc_card *card) >> } >> >> static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, >> - bool send_status, bool retry_crc_err, bool use_r1b_resp) >> + bool send_status, bool retry_crc_err, bool use_r1b_resp, >> + unsigned int retries) > > What are the use-case for actually being able to use different amount > of retries for polling? ioctl_rpmb_card_status_poll() retry 5 times but mmc_do_erase() wants zero. The comment seems to imply it wants the first send_status get back the device status related to the last command prior to it. > > I am thinking that, we can possibly change the callers of > mmc_poll_for_busy() to use the same amount of retries instead, don't > you think? > >> { >> struct mmc_host *host = card->host; >> int err; >> @@ -486,7 +487,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, >> if (host->ops->card_busy) { >> busy = host->ops->card_busy(host); >> } else { >> - err = mmc_send_status(card, &status); >> + err = __mmc_send_status(card, &status, retries); >> if (retry_crc_err && err == -EILSEQ) { >> busy = true; >> } else if (err) { >> @@ -578,7 +579,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, >> >> /* Let's try to poll to find out when the command is completed. */ >> err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err, >> - use_r1b_resp); >> + use_r1b_resp, MMC_CMD_RETRIES); >> if (err) >> goto out; >> >> -- >> 1.9.1 >> >> >> -- >> 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 > > Kind regards > Uffe > > > -- 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
On 25 May 2018 at 10:59, Shawn Lin <shawn.lin@rock-chips.com> wrote: > On 2018/5/25 16:42, Ulf Hansson wrote: >> >> On 25 May 2018 at 08:17, Shawn Lin <shawn.lin@rock-chips.com> wrote: >>> >>> In preparation for reusing mmc_poll_for_busy() to avoid duplication >>> of code for polling busy. >>> >>> No functional change intended. >>> >>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >>> --- >>> >>> drivers/mmc/core/mmc_ops.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >>> index 88f34fd..4d73db4 100644 >>> --- a/drivers/mmc/core/mmc_ops.c >>> +++ b/drivers/mmc/core/mmc_ops.c >>> @@ -447,7 +447,8 @@ int mmc_switch_status(struct mmc_card *card) >>> } >>> >>> static int mmc_poll_for_busy(struct mmc_card *card, unsigned int >>> timeout_ms, >>> - bool send_status, bool retry_crc_err, bool >>> use_r1b_resp) >>> + bool send_status, bool retry_crc_err, bool >>> use_r1b_resp, >>> + unsigned int retries) >> >> >> What are the use-case for actually being able to use different amount >> of retries for polling? > > > ioctl_rpmb_card_status_poll() retry 5 times but mmc_do_erase() wants > zero. The comment seems to imply it wants the first send_status get back > the device status related to the last command prior to it. I am guessing you refer to "Do not retry else we can't see errors". Actually, the same thing applies to switch commands. Only the first CMD13 will give the switch status, anyway we are trying 5 times... My point is, I think we shall use the kind of policy, unless there are good reasons not to. [...] Kind regards Uffe -- 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
On 25 May 2018 at 11:12, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 25 May 2018 at 10:59, Shawn Lin <shawn.lin@rock-chips.com> wrote: >> On 2018/5/25 16:42, Ulf Hansson wrote: >>> >>> On 25 May 2018 at 08:17, Shawn Lin <shawn.lin@rock-chips.com> wrote: >>>> >>>> In preparation for reusing mmc_poll_for_busy() to avoid duplication >>>> of code for polling busy. >>>> >>>> No functional change intended. >>>> >>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >>>> --- >>>> >>>> drivers/mmc/core/mmc_ops.c | 7 ++++--- >>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >>>> index 88f34fd..4d73db4 100644 >>>> --- a/drivers/mmc/core/mmc_ops.c >>>> +++ b/drivers/mmc/core/mmc_ops.c >>>> @@ -447,7 +447,8 @@ int mmc_switch_status(struct mmc_card *card) >>>> } >>>> >>>> static int mmc_poll_for_busy(struct mmc_card *card, unsigned int >>>> timeout_ms, >>>> - bool send_status, bool retry_crc_err, bool >>>> use_r1b_resp) >>>> + bool send_status, bool retry_crc_err, bool >>>> use_r1b_resp, >>>> + unsigned int retries) >>> >>> >>> What are the use-case for actually being able to use different amount >>> of retries for polling? >> >> >> ioctl_rpmb_card_status_poll() retry 5 times but mmc_do_erase() wants >> zero. The comment seems to imply it wants the first send_status get back >> the device status related to the last command prior to it. > > I am guessing you refer to "Do not retry else we can't see errors". > > Actually, the same thing applies to switch commands. Only the first > CMD13 will give the switch status, anyway we are trying 5 times... > > My point is, I think we shall use the kind of policy, unless there are /s/kind/same kind > good reasons not to. > > [...] > > Kind regards > Uffe -- 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
On 2018/5/25 17:12, Ulf Hansson wrote: > On 25 May 2018 at 10:59, Shawn Lin <shawn.lin@rock-chips.com> wrote: >> On 2018/5/25 16:42, Ulf Hansson wrote: >>> >>> On 25 May 2018 at 08:17, Shawn Lin <shawn.lin@rock-chips.com> wrote: >>>> >>>> In preparation for reusing mmc_poll_for_busy() to avoid duplication >>>> of code for polling busy. >>>> >>>> No functional change intended. >>>> >>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >>>> --- >>>> >>>> drivers/mmc/core/mmc_ops.c | 7 ++++--- >>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >>>> index 88f34fd..4d73db4 100644 >>>> --- a/drivers/mmc/core/mmc_ops.c >>>> +++ b/drivers/mmc/core/mmc_ops.c >>>> @@ -447,7 +447,8 @@ int mmc_switch_status(struct mmc_card *card) >>>> } >>>> >>>> static int mmc_poll_for_busy(struct mmc_card *card, unsigned int >>>> timeout_ms, >>>> - bool send_status, bool retry_crc_err, bool >>>> use_r1b_resp) >>>> + bool send_status, bool retry_crc_err, bool >>>> use_r1b_resp, >>>> + unsigned int retries) >>> >>> >>> What are the use-case for actually being able to use different amount >>> of retries for polling? >> >> >> ioctl_rpmb_card_status_poll() retry 5 times but mmc_do_erase() wants >> zero. The comment seems to imply it wants the first send_status get back >> the device status related to the last command prior to it. > > I am guessing you refer to "Do not retry else we can't see errors". > yes. > Actually, the same thing applies to switch commands. Only the first > CMD13 will give the switch status, anyway we are trying 5 times... > > My point is, I think we shall use the kind of policy, unless there are > good reasons not to. Ok, I got it. Will rework it. :) > > [...] > > Kind regards > Uffe > -- > 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
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index 88f34fd..4d73db4 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -447,7 +447,8 @@ int mmc_switch_status(struct mmc_card *card) } static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, - bool send_status, bool retry_crc_err, bool use_r1b_resp) + bool send_status, bool retry_crc_err, bool use_r1b_resp, + unsigned int retries) { struct mmc_host *host = card->host; int err; @@ -486,7 +487,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, if (host->ops->card_busy) { busy = host->ops->card_busy(host); } else { - err = mmc_send_status(card, &status); + err = __mmc_send_status(card, &status, retries); if (retry_crc_err && err == -EILSEQ) { busy = true; } else if (err) { @@ -578,7 +579,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, /* Let's try to poll to find out when the command is completed. */ err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err, - use_r1b_resp); + use_r1b_resp, MMC_CMD_RETRIES); if (err) goto out;
In preparation for reusing mmc_poll_for_busy() to avoid duplication of code for polling busy. No functional change intended. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- drivers/mmc/core/mmc_ops.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)