| Message ID | 1527646312-133348-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30 May 2018 at 04:11, 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> > --- > > Changes in v3: None > Changes in v2: None > > drivers/mmc/core/mmc_ops.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index ee5f5ea..7e8be97 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, > + u32 *resp_status, bool check_busy(u32 device_status)) Okay, so I like the idea that the caller should be give the responsibility of checking error/status codes via a callback. An option, could be to add that callback to the card struct. So, instead of providing it as a parameter to a function the caller will have to set the callback prior invoking mmc_poll_for_busy(). I think the code may become a bit nicer, but I guess it depends on what one prefer. :-) > { > struct mmc_host *host = card->host; > int err; > @@ -461,6 +462,9 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > mmc_host_is_spi(host)) > return 0; > > + if (WARN_ON(!check_busy)) > + return 0; > + > /* We have an unspecified cmd timeout, use the fallback value. */ > if (!timeout_ms) > timeout_ms = MMC_OPS_TIMEOUT_MS; > @@ -487,6 +491,9 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > busy = host->ops->card_busy(host); > } else { > err = mmc_send_status(card, &status); > + /* Accumulate any response error bits seen */ > + if (resp_status) > + *resp_status |= status; This makes sense, particular if we expect the caller to care about several status bits, which may be set while iterating through the loops. I assume that's the intent, right? > if (retry_crc_err && err == -EILSEQ) { > busy = true; > } else if (err) { > @@ -495,7 +502,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > err = mmc_switch_status_error(host, status); > if (err) > return err; > - busy = R1_CURRENT_STATE(status) == R1_STATE_PRG; > + busy = check_busy(status); I think this can be made more flexible, and also with the benefit of keeping mmc_poll_for_busy() as simple as possible. Instead of making the callback to check for the card status only, wouldn't it be better if had the responsibility to check the entire error/busy thingy? In principle it should mean we get lesser in-parameters to mmc_poll_for_busy() rather than more. Thoughts? > } > } > > @@ -510,6 +517,11 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > return 0; > } > > +static inline bool mmc_switch_in_prg_state(u32 status) > +{ > + return R1_CURRENT_STATE(status) == R1_STATE_PRG; > +} > + > /** > * __mmc_switch - modify EXT_CSD register > * @card: the MMC card associated with the data transfer > @@ -577,7 +589,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, NULL, &mmc_switch_in_prg_state); > if (err) > goto out; > 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
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index ee5f5ea..7e8be97 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, + u32 *resp_status, bool check_busy(u32 device_status)) { struct mmc_host *host = card->host; int err; @@ -461,6 +462,9 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, mmc_host_is_spi(host)) return 0; + if (WARN_ON(!check_busy)) + return 0; + /* We have an unspecified cmd timeout, use the fallback value. */ if (!timeout_ms) timeout_ms = MMC_OPS_TIMEOUT_MS; @@ -487,6 +491,9 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, busy = host->ops->card_busy(host); } else { err = mmc_send_status(card, &status); + /* Accumulate any response error bits seen */ + if (resp_status) + *resp_status |= status; if (retry_crc_err && err == -EILSEQ) { busy = true; } else if (err) { @@ -495,7 +502,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, err = mmc_switch_status_error(host, status); if (err) return err; - busy = R1_CURRENT_STATE(status) == R1_STATE_PRG; + busy = check_busy(status); } } @@ -510,6 +517,11 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, return 0; } +static inline bool mmc_switch_in_prg_state(u32 status) +{ + return R1_CURRENT_STATE(status) == R1_STATE_PRG; +} + /** * __mmc_switch - modify EXT_CSD register * @card: the MMC card associated with the data transfer @@ -577,7 +589,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, NULL, &mmc_switch_in_prg_state); 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> --- Changes in v3: None Changes in v2: None drivers/mmc/core/mmc_ops.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)