| Message ID | 1495438722-22091-1-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 22/05/17 10:38, Ulf Hansson wrote: > There are no reason to why the mmc block device driver needs to implements > its own version of how to get the status of the card. Current it do so via > the static function get_card_status(). Let's instead drop that function and > convert to use mmc_send_status() as that is what everybody else is using. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/block.c | 22 +++------------------- > drivers/mmc/core/mmc_ops.c | 1 + > 2 files changed, 4 insertions(+), 19 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index e973798..23a3254 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -127,7 +127,6 @@ MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device"); > > static inline int mmc_blk_part_switch(struct mmc_card *card, > struct mmc_blk_data *md); > -static int get_card_status(struct mmc_card *card, u32 *status, int retries); > > static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) > { > @@ -385,7 +384,7 @@ static int ioctl_rpmb_card_status_poll(struct mmc_card *card, u32 *status, > return -EINVAL; > > do { > - err = get_card_status(card, status, 5); > + err = mmc_send_status(card, status); > if (err) > break; > > @@ -885,21 +884,6 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks) > return 0; > } > > -static int get_card_status(struct mmc_card *card, u32 *status, int retries) > -{ > - struct mmc_command cmd = {}; > - int err; > - > - cmd.opcode = MMC_SEND_STATUS; > - if (!mmc_host_is_spi(card->host)) > - cmd.arg = card->rca << 16; > - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC; > - err = mmc_wait_for_cmd(card->host, &cmd, retries); > - if (err == 0) > - *status = cmd.resp[0]; > - return err; > -} > - > static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms, > bool hw_busy_detect, struct request *req, bool *gen_err) > { > @@ -908,7 +892,7 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms, > u32 status; > > do { > - err = get_card_status(card, &status, 5); > + err = mmc_send_status(card, &status); > if (err) { > pr_err("%s: error %d requesting status\n", > req->rq_disk->disk_name, err); > @@ -1076,7 +1060,7 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, struct request *req, > * we can't be sure the returned status is for the r/w command. > */ > for (retry = 2; retry >= 0; retry--) { > - err = get_card_status(card, &status, 0); > + err = mmc_send_status(card, &status); That is not quite the same because mmc_send_status() does MMC_CMD_RETRIES retries. That changes the value and meaning of prev_cmd_status_valid. i.e. the logic is considering whether the error bits in the status might have been cleared by the retry. > if (!err) > break; > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index 78f75f0..ac4b694 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -76,6 +76,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status) > > return 0; > } > +EXPORT_SYMBOL_GPL(mmc_send_status); > > static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card) > { > -- 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 22 May 2017 at 09:43, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 22/05/17 10:38, Ulf Hansson wrote: >> There are no reason to why the mmc block device driver needs to implements >> its own version of how to get the status of the card. Current it do so via >> the static function get_card_status(). Let's instead drop that function and >> convert to use mmc_send_status() as that is what everybody else is using. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> drivers/mmc/core/block.c | 22 +++------------------- >> drivers/mmc/core/mmc_ops.c | 1 + >> 2 files changed, 4 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c >> index e973798..23a3254 100644 >> --- a/drivers/mmc/core/block.c >> +++ b/drivers/mmc/core/block.c >> @@ -127,7 +127,6 @@ MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device"); >> >> static inline int mmc_blk_part_switch(struct mmc_card *card, >> struct mmc_blk_data *md); >> -static int get_card_status(struct mmc_card *card, u32 *status, int retries); >> >> static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) >> { >> @@ -385,7 +384,7 @@ static int ioctl_rpmb_card_status_poll(struct mmc_card *card, u32 *status, >> return -EINVAL; >> >> do { >> - err = get_card_status(card, status, 5); >> + err = mmc_send_status(card, status); >> if (err) >> break; >> >> @@ -885,21 +884,6 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks) >> return 0; >> } >> >> -static int get_card_status(struct mmc_card *card, u32 *status, int retries) >> -{ >> - struct mmc_command cmd = {}; >> - int err; >> - >> - cmd.opcode = MMC_SEND_STATUS; >> - if (!mmc_host_is_spi(card->host)) >> - cmd.arg = card->rca << 16; >> - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC; >> - err = mmc_wait_for_cmd(card->host, &cmd, retries); >> - if (err == 0) >> - *status = cmd.resp[0]; >> - return err; >> -} >> - >> static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms, >> bool hw_busy_detect, struct request *req, bool *gen_err) >> { >> @@ -908,7 +892,7 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms, >> u32 status; >> >> do { >> - err = get_card_status(card, &status, 5); >> + err = mmc_send_status(card, &status); >> if (err) { >> pr_err("%s: error %d requesting status\n", >> req->rq_disk->disk_name, err); >> @@ -1076,7 +1060,7 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, struct request *req, >> * we can't be sure the returned status is for the r/w command. >> */ >> for (retry = 2; retry >= 0; retry--) { >> - err = get_card_status(card, &status, 0); >> + err = mmc_send_status(card, &status); > > That is not quite the same because mmc_send_status() does MMC_CMD_RETRIES > retries. That changes the value and meaning of prev_cmd_status_valid. i.e. > the logic is considering whether the error bits in the status might have > been cleared by the retry. Right, I didn't really think that was of importance. Perhaps I was comparing to earlier experiences we have had for mmc_switch(), as there have always been re-tries in some paths but which never made any sense. Anyway, let me re-spin and export a function which allows the caller to specify retries then. > >> if (!err) >> break; >> >> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >> index 78f75f0..ac4b694 100644 >> --- a/drivers/mmc/core/mmc_ops.c >> +++ b/drivers/mmc/core/mmc_ops.c >> @@ -76,6 +76,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status) >> >> return 0; >> } >> +EXPORT_SYMBOL_GPL(mmc_send_status); >> >> static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card) >> { >> > Thanks for reviewing. 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/block.c b/drivers/mmc/core/block.c index e973798..23a3254 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -127,7 +127,6 @@ MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device"); static inline int mmc_blk_part_switch(struct mmc_card *card, struct mmc_blk_data *md); -static int get_card_status(struct mmc_card *card, u32 *status, int retries); static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) { @@ -385,7 +384,7 @@ static int ioctl_rpmb_card_status_poll(struct mmc_card *card, u32 *status, return -EINVAL; do { - err = get_card_status(card, status, 5); + err = mmc_send_status(card, status); if (err) break; @@ -885,21 +884,6 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks) return 0; } -static int get_card_status(struct mmc_card *card, u32 *status, int retries) -{ - struct mmc_command cmd = {}; - int err; - - cmd.opcode = MMC_SEND_STATUS; - if (!mmc_host_is_spi(card->host)) - cmd.arg = card->rca << 16; - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC; - err = mmc_wait_for_cmd(card->host, &cmd, retries); - if (err == 0) - *status = cmd.resp[0]; - return err; -} - static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms, bool hw_busy_detect, struct request *req, bool *gen_err) { @@ -908,7 +892,7 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms, u32 status; do { - err = get_card_status(card, &status, 5); + err = mmc_send_status(card, &status); if (err) { pr_err("%s: error %d requesting status\n", req->rq_disk->disk_name, err); @@ -1076,7 +1060,7 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, struct request *req, * we can't be sure the returned status is for the r/w command. */ for (retry = 2; retry >= 0; retry--) { - err = get_card_status(card, &status, 0); + err = mmc_send_status(card, &status); if (!err) break; diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index 78f75f0..ac4b694 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -76,6 +76,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status) return 0; } +EXPORT_SYMBOL_GPL(mmc_send_status); static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card) {
There are no reason to why the mmc block device driver needs to implements its own version of how to get the status of the card. Current it do so via the static function get_card_status(). Let's instead drop that function and convert to use mmc_send_status() as that is what everybody else is using. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/mmc/core/block.c | 22 +++------------------- drivers/mmc/core/mmc_ops.c | 1 + 2 files changed, 4 insertions(+), 19 deletions(-) -- 2.7.4