Re: [PATCH v4] mmc: Add quirk MMC_QUIRK_BROKEN_CACHE_FLUSH for Micron eMMC Q2J54A

[Bean Huo <beanhuo@iokpp.de>]

Hi Ulf,

thanks for your comments, I didn't quite get your points:

On Tue, 2023-10-10 at 16:20 +0200, Ulf Hansson wrote:
> > @@ -2381,8 +2381,11 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct
> > mmc_queue *mq, struct request *req)
> >                          }
> >                          ret = mmc_blk_cqe_issue_flush(mq, req);
> >                          break;
> > -               case REQ_OP_READ:
> >                  case REQ_OP_WRITE:
> > +                       if (mmc_card_broken_cache_flush(card) &&
> > !card->written_flag)
> 
> It looks superfluous to me to check mmc_card_broken_cache_flush() and
> !card->written_flag. Just set the card->written_flag unconditionally.

what did you mean "Just set the card->written_flag unconditionally."?
This means I just need to check card->written_flag and set card-
>written_flag to true and false in the case
MMC_QUIRK_BROKEN_CACHE_FLUSH? or don't need to call
mmc_card_broken_cache_flush()?

> 
> > +                               card->written_flag = true;
> > +                       fallthrough;
> > +               case REQ_OP_READ:
> >                          if (host->cqe_enabled)
> >                                  ret = mmc_blk_cqe_issue_rw_rq(mq,
> > req);
> >                          else
> > diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> > index 4edf9057fa79..b7754a1b8d97 100644
> > --- a/drivers/mmc/core/card.h
> > +++ b/drivers/mmc/core/card.h
> > @@ -280,4 +280,8 @@ static inline int
> > mmc_card_broken_sd_cache(const struct mmc_card *c)
> >          return c->quirks & MMC_QUIRK_BROKEN_SD_CACHE;
> >   }
> > 
> > +static inline int mmc_card_broken_cache_flush(const struct
> > mmc_card *c)
> > +{
> > +       return c->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH;
> > +}
> >   #endif
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index 89cd48fcec79..47896c32086e 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;
> > +
> 
> How about doing this after a successful flush operation instead? In
> other words in _mmc_flush_cache().

Here initializes flag and the patch is intenting to eliminate the cache
flush command before writing. what do you mean adding in
mmc_flush_cache()?


Kind regards,
Bean