| Message ID | 20230921203426.638262-1-beanhuo@iokpp.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] mmc: Add quirk MMC_QUIRK_BROKEN_CACHE_FLUSH for Micron eMMC Q2J54A | expand |
On Thu, Sep 21, 2023 at 10:34:26PM +0200, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > Micron MTFC4GACAJCN eMMC supports cache but requires that flush cache > operation be allowed only after a write has occurred. Otherwise, the > cache flush command or subsequent commands will time out. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > Co-developed-by: Rafael Beims <rafael.beims@toradex.com> Then Rafael also has to sign off on this, right?
On Thu, Sep 21, 2023 at 10:34:26PM +0200, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > Micron MTFC4GACAJCN eMMC supports cache but requires that flush cache > operation be allowed only after a write has occurred. Otherwise, the > cache flush command or subsequent commands will time out. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > Co-developed-by: Rafael Beims <rafael.beims@toradex.com> > Cc: stable@vger.kernel.org > --- > Changelog: > > v2--v3: > 1. Set card->written_flag in mmc_blk_mq_issue_rq(). > v1--v2: > 1. Add Rafael's test-tag, and Co-developed-by. > 2. Check host->card whether NULL or not in __mmc_start_request() before asserting host->card->->quirks > --- > drivers/mmc/core/block.c | 4 ++++ > drivers/mmc/core/mmc.c | 5 +++++ > drivers/mmc/core/quirks.h | 7 ++++--- > include/linux/mmc/card.h | 2 ++ > 4 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index 3a8f27c3e310..14d0dc7942de 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -2387,6 +2387,10 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req) > ret = mmc_blk_cqe_issue_rw_rq(mq, req); > else > ret = mmc_blk_mq_issue_rw_rq(mq, req); > + > + if (host->card->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH && > + !host->card->written_flag && !ret) > + host->card->written_flag = true; From what I can see this branch is followed for both REQ_OP_READ and REQ_OP_WRITE, and I would say we want to set this flag only for REQ_OP_WRITE. Am I wrong? Francesco
On 22/09/2023 06:29, Greg KH wrote: > On Thu, Sep 21, 2023 at 10:34:26PM +0200, Bean Huo wrote: >> From: Bean Huo <beanhuo@micron.com> >> >> Micron MTFC4GACAJCN eMMC supports cache but requires that flush cache >> operation be allowed only after a write has occurred. Otherwise, the >> cache flush command or subsequent commands will time out. >> >> Signed-off-by: Bean Huo <beanhuo@micron.com> >> Co-developed-by: Rafael Beims <rafael.beims@toradex.com> > Then Rafael also has to sign off on this, right? > Correct, Signed-off-by: Rafael Beims <rafael.beims@toradex.com> Thanks, Rafael
On Thu, 21 Sept 2023 at 22:35, Bean Huo <beanhuo@iokpp.de> wrote: > > From: Bean Huo <beanhuo@micron.com> > > Micron MTFC4GACAJCN eMMC supports cache but requires that flush cache > operation be allowed only after a write has occurred. Otherwise, the > cache flush command or subsequent commands will time out. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > Co-developed-by: Rafael Beims <rafael.beims@toradex.com> > Cc: stable@vger.kernel.org > --- > Changelog: > > v2--v3: > 1. Set card->written_flag in mmc_blk_mq_issue_rq(). > v1--v2: > 1. Add Rafael's test-tag, and Co-developed-by. > 2. Check host->card whether NULL or not in __mmc_start_request() before asserting host->card->->quirks > --- > drivers/mmc/core/block.c | 4 ++++ > drivers/mmc/core/mmc.c | 5 +++++ > drivers/mmc/core/quirks.h | 7 ++++--- > include/linux/mmc/card.h | 2 ++ > 4 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index 3a8f27c3e310..14d0dc7942de 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -2387,6 +2387,10 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req) > ret = mmc_blk_cqe_issue_rw_rq(mq, req); > else > ret = mmc_blk_mq_issue_rw_rq(mq, req); > + > + if (host->card->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH && Please add an inline helper function for this in drivers/mmc/core/card.h, along with the others. [...] Kind regards Uffe
On Fri, 2023-09-22 at 19:01 +0200, Francesco Dolcini wrote: > > + > > + if (host->card->quirks & > > MMC_QUIRK_BROKEN_CACHE_FLUSH && > > + !host->card->written_flag && !ret) > > + host->card->written_flag = true; > > From what I can see this branch is followed for both REQ_OP_READ and > REQ_OP_WRITE, and I would say we want to set this flag only for > REQ_OP_WRITE. > > Am I wrong? > > Francesco you are right, we only care about write, I will update it in the next version, thanks for your review! Kind regards, Bean
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 3a8f27c3e310..14d0dc7942de 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -2387,6 +2387,10 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req) ret = mmc_blk_cqe_issue_rw_rq(mq, req); else ret = mmc_blk_mq_issue_rw_rq(mq, req); + + if (host->card->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH && + !host->card->written_flag && !ret) + host->card->written_flag = true; break; default: WARN_ON_ONCE(1); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 89cd48fcec79..a2edd065fa1b 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1929,6 +1929,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, if (!oldcard) host->card = card; + card->written_flag = false; + return 0; free_card: @@ -2081,6 +2083,9 @@ static int _mmc_flush_cache(struct mmc_host *host) { int err = 0; + if (host->card->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH && !host->card->written_flag) + return err; + if (_mmc_cache_enabled(host)) { err = mmc_switch(host->card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_FLUSH_CACHE, 1, diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index 32b64b564fb1..5e68c8b4cdca 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -110,11 +110,12 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = { MMC_QUIRK_TRIM_BROKEN), /* - * Micron MTFC4GACAJCN-1M advertises TRIM but it does not seems to - * support being used to offload WRITE_ZEROES. + * Micron MTFC4GACAJCN-1M supports TRIM but does not appear to suppor + * WRITE_ZEROES offloading. It also supports caching, but the cache can + * only be flushed after a write has occurred. */ MMC_FIXUP("Q2J54A", CID_MANFID_MICRON, 0x014e, add_quirk_mmc, - MMC_QUIRK_TRIM_BROKEN), + MMC_QUIRK_TRIM_BROKEN | MMC_QUIRK_BROKEN_CACHE_FLUSH), /* * Kingston EMMC04G-M627 advertises TRIM but it does not seems to diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index daa2f40d9ce6..7b12eebc5586 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -295,7 +295,9 @@ struct mmc_card { #define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */ #define MMC_QUIRK_BROKEN_SD_DISCARD (1<<14) /* Disable broken SD discard support */ #define MMC_QUIRK_BROKEN_SD_CACHE (1<<15) /* Disable broken SD cache support */ +#define MMC_QUIRK_BROKEN_CACHE_FLUSH (1<<16) /* Don't flush cache until the write has occurred */ + bool written_flag; /* Indicates eMMC has been written since power on */ bool reenable_cmdq; /* Re-enable Command Queue */ unsigned int erase_size; /* erase size in sectors */