Adrian Hunter wrote:
> On 06/10/11 07:38, Seungwon Jeon wrote:
> > Adrian Hunter wrote:
> >> On 08/09/11 10:29, Seungwon Jeon wrote:
> >>> This patch adds cache feature of eMMC4.5 Spec.
> >>> If device supports cache capability, host can utilize some specific
> >>> operations.
> >>>
> >>> Signed-off-by: Seungwon Jeon<[email protected]>
> >>> ---
> >>> This patch is base on [PATCH v3] mmc: core: Add default timeout value
> >> for CMD6
> >>>
> >>>    drivers/mmc/card/block.c |   14 ++++++----
> >>>    drivers/mmc/core/core.c  |   62
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >>>    drivers/mmc/core/mmc.c   |   23 +++++++++++++++++
> >>>    include/linux/mmc/card.h |    2 +
> >>>    include/linux/mmc/core.h |    2 +
> >>>    include/linux/mmc/host.h |    3 ++
> >>>    include/linux/mmc/mmc.h  |    3 ++
> >>>    7 files changed, 103 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> >>> index e702c61..84fa4bb 100644
> >>> --- a/drivers/mmc/card/block.c
> >>> +++ b/drivers/mmc/card/block.c
> >>> @@ -779,16 +779,18 @@ out:
> >>>    static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request
> >> *req)
> >>>    {
> >>>           struct mmc_blk_data *md = mq->data;
> >>> + struct mmc_card *card = md->queue.card;
> >>> + int ret = 0;
> >>> +
> >>> + ret = mmc_flush_cache(card);
> >>> + if (ret)
> >>> +         ret = -EIO;
> >>>
> >>> - /*
> >>> -  * No-op, only service this because we need REQ_FUA for reliable
> >>> -  * writes.
> >>> -  */
> >>>           spin_lock_irq(&md->lock);
> >>> - __blk_end_request_all(req, 0);
> >>> + __blk_end_request_all(req, ret);
> >>>           spin_unlock_irq(&md->lock);
> >>>
> >>> - return 1;
> >>> + return ret ? 0 : 1;
> >>>    }
> >>>
> >>>    /*
> >>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >>> index 557856b..14c2431 100644
> >>> --- a/drivers/mmc/core/core.c
> >>> +++ b/drivers/mmc/core/core.c
> >>> @@ -1987,6 +1987,64 @@ int mmc_card_can_sleep(struct mmc_host *host)
> >>>    }
> >>>    EXPORT_SYMBOL(mmc_card_can_sleep);
> >>>
> >>> +/*
> >>> + * Flush the cache to the non-volatile storage.
> >>> + */
> >>> +int mmc_flush_cache(struct mmc_card *card)
> >>> +{
> >>> + struct mmc_host *host = card->host;
> >>> + int err = 0;
> >>> +
> >>> + if (!(host->caps&   MMC_CAP_CACHE_CTRL))
> >>> +         return err;
> >>> +
> >>> + if (mmc_card_mmc(card)&&
> >>> +                 (card->ext_csd.cache_size>   0)&&
> >>> +                 (card->ext_csd.cache_ctrl&   1)) {
> >>> +         err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >>> +                         EXT_CSD_FLUSH_CACHE, 1, 0);
> >>> +         if (err)
> >>> +                 pr_err("%s: cache flush error %d\n",
> >>> +                                 mmc_hostname(card->host), err);
> >>> + }
> >>> +
> >>> + return err;
> >>> +}
> >>> +EXPORT_SYMBOL(mmc_flush_cache);
> >>> +
> >>> +/*
> >>> + * Turn the cache ON/OFF.
> >>> + * Turning the cache OFF shall trigger flushing of the data
> >>> + * to the non-volatile storage.
> >>> + */
> >>> +int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
> >>> +{
> >>> + struct mmc_card *card = host->card;
> >>> + int err = 0;
> >>> +
> >>> + if (!(host->caps&   MMC_CAP_CACHE_CTRL))
> >>> +         return err;
> >>> +
> >>
> >> This patch does not cover code paths for removable cards.
> >> For clarity and in case a platform does not set non-removable
> >> flags for eMMC, a check is needed here e.g.
> >>
> >>    if (mmc_card_is_removable(host))
> >>            return 0;
> >>
> > Is it worry about normal MMC card type and?
> > Then, "card->ext_csd.cache_size" is a good condition for deciding cache
> control.
> > Or even though eMMC should be non-removable type, platform does not set
> non-removable for eMMC type mistakenly?
> 
> Yes.  Looking at mmc_pm_notify() and mmc_attach_bus_ops() it seems that an
> eMMC
> will get powered off before suspend if the MMC_CAP_NONREMOVABLE is not set.
> That was what I meant about not covering code paths for removable cards.
> 
> This code does not cover cards that are not specified as non-removable, so:
> 
>       if (mmc_card_is_removable(host))
>               return 0;
Yes, I found it.
This case is undesirable but will affect flushing cache.
Thank you for review.
I will resend including this.
> 
> > I can't catch the meaning exactly.
> > Please let me know.
> >
> > Thanks.
> > Beset regards,
> > Seungwon Jeon.
> >
> >>> + if (card&&   mmc_card_mmc(card)&&
> >>> +                 (card->ext_csd.cache_size>   0)) {
> >>> +         enable = !!enable;
> >>> +
> >>> +         if (card->ext_csd.cache_ctrl ^ enable)
> >>> +                 err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >>> +                                 EXT_CSD_CACHE_CTRL, enable, 0);
> >>> +         if (err)
> >>> +                 pr_err("%s: cache %s error %d\n",
> >>> +                                 mmc_hostname(card->host),
> >>> +                                 enable ? "on" : "off",
> >>> +                                 err);
> >>> +         else
> >>> +                 card->ext_csd.cache_ctrl = enable;
> >>> + }
> >>> +
> >>> + return err;
> >>> +}
> >>> +EXPORT_SYMBOL(mmc_cache_ctrl);
> >>> +
> >>>    #ifdef CONFIG_PM
> >>>
> >>>    /**
> >>> @@ -2001,6 +2059,9 @@ int mmc_suspend_host(struct mmc_host *host)
> >>>                   cancel_delayed_work(&host->disable);
> >>>           cancel_delayed_work(&host->detect);
> >>>           mmc_flush_scheduled_work();
> >>> + err = mmc_cache_ctrl(host, 0);
> >>> + if (err)
> >>> +         goto out;
> >>>
> >>>           mmc_bus_get(host);
> >>>           if (host->bus_ops&&   !host->bus_dead) {
> >>> @@ -2025,6 +2086,7 @@ int mmc_suspend_host(struct mmc_host *host)
> >>>           if (!err&&   !mmc_card_keep_power(host))
> >>>                   mmc_power_off(host);
> >>>
> >>> +out:
> >>>           return err;
> >>>    }
> >>>
> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >>> index ea58a0c..035112b 100644
> >>> --- a/drivers/mmc/core/mmc.c
> >>> +++ b/drivers/mmc/core/mmc.c
> >>> @@ -419,6 +419,12 @@ static int mmc_read_ext_csd(struct mmc_card *card,
> >> u8 *ext_csd)
> >>>                   */
> >>>                   card->ext_csd.generic_cmd6_time = 0;
> >>>
> >>> + card->ext_csd.cache_size =
> >>> +         ext_csd[EXT_CSD_CACHE_SIZE + 0]<<   0 |
> >>> +         ext_csd[EXT_CSD_CACHE_SIZE + 1]<<   8 |
> >>> +         ext_csd[EXT_CSD_CACHE_SIZE + 2]<<   16 |
> >>> +         ext_csd[EXT_CSD_CACHE_SIZE + 3]<<   24;
> >>> +
> >>>    out:
> >>>           return err;
> >>>    }
> >>> @@ -851,6 +857,23 @@ static int mmc_init_card(struct mmc_host *host,
> u32
> >> ocr,
> >>>                   }
> >>>           }
> >>>
> >>> + /*
> >>> +  * If cache size is higher than 0, this indicates
> >>> +  * the the existence of cache and it can be turned on.
> >>> +  */
> >>> + if ((host->caps&   MMC_CAP_CACHE_CTRL)&&
> >>> +                 card->ext_csd.cache_size>   0) {
> >>> +         err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >>> +                         EXT_CSD_CACHE_CTRL, 1, 0);
> >>> +         if (err&&   err != -EBADMSG)
> >>> +                 goto free_card;
> >>> +
> >>> +         /*
> >>> +          * Only if no error, cache is turned on successfully.
> >>> +          */
> >>> +         card->ext_csd.cache_ctrl = err ? 0 : 1;
> >>> + }
> >>> +
> >>>           if (!oldcard)
> >>>                   host->card = card;
> >>>
> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> >>> index b713abf..519660b 100644
> >>> --- a/include/linux/mmc/card.h
> >>> +++ b/include/linux/mmc/card.h
> >>> @@ -50,6 +50,7 @@ struct mmc_ext_csd {
> >>>           u8                      rel_sectors;
> >>>           u8                      rel_param;
> >>>           u8                      part_config;
> >>> + u8                      cache_ctrl;
> >>>           unsigned int            part_time;              /* Units: ms */
> >>>           unsigned int            sa_timeout;             /* Units: 100ns 
> >>> */
> >>>           unsigned int            generic_cmd6_time;      /* Units: 10ms 
> >>> */
> >>> @@ -65,6 +66,7 @@ struct mmc_ext_csd {
> >>>           unsigned long long      enhanced_area_offset;   /* Units: Byte 
> >>> */
> >>>           unsigned int            enhanced_area_size;     /* Units: KB */
> >>>           unsigned int            boot_size;              /* in bytes */
> >>> + unsigned int            cache_size;             /* Units: KB */
> >>>           u8                      raw_partition_support;  /* 160 */
> >>>           u8                      raw_erased_mem_count;   /* 181 */
> >>>           u8                      raw_ext_csd_structure;  /* 194 */
> >>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> >>> index b8b1b7a..45b4acf 100644
> >>> --- a/include/linux/mmc/core.h
> >>> +++ b/include/linux/mmc/core.h
> >>> @@ -171,6 +171,8 @@ extern void mmc_release_host(struct mmc_host
> *host);
> >>>    extern void mmc_do_release_host(struct mmc_host *host);
> >>>    extern int mmc_try_claim_host(struct mmc_host *host);
> >>>
> >>> +extern int mmc_flush_cache(struct mmc_card *);
> >>> +
> >>>    /**
> >>>     *     mmc_claim_host - exclusively claim a host
> >>>     *     @host: mmc host to claim
> >>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> >>> index 4c4bddf..6d0006d 100644
> >>> --- a/include/linux/mmc/host.h
> >>> +++ b/include/linux/mmc/host.h
> >>> @@ -230,6 +230,7 @@ struct mmc_host {
> >>>    #define MMC_CAP_MAX_CURRENT_600        (1<<   28)      /* Host max 
> >>> current
> >> limit is 600mA */
> >>>    #define MMC_CAP_MAX_CURRENT_800        (1<<   29)      /* Host max 
> >>> current
> >> limit is 800mA */
> >>>    #define MMC_CAP_CMD23          (1<<   30)      /* CMD23 supported.
> */
> >>> +#define MMC_CAP_CACHE_CTRL               (1<<   31)      /* Allow cache
> >> control. */
> >>>
> >>>           mmc_pm_flag_t           pm_caps;        /* supported pm 
> >>> features */
> >>>
> >>> @@ -335,6 +336,8 @@ extern int mmc_power_restore_host(struct mmc_host
> >> *host);
> >>>    extern void mmc_detect_change(struct mmc_host *, unsigned long
> delay);
> >>>    extern void mmc_request_done(struct mmc_host *, struct mmc_request
> *);
> >>>
> >>> +extern int mmc_cache_ctrl(struct mmc_host *, u8);
> >>> +
> >>>    static inline void mmc_signal_sdio_irq(struct mmc_host *host)
> >>>    {
> >>>           host->ops->enable_sdio_irq(host, 0);
> >>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> >>> index e869f00..b7fa9f7 100644
> >>> --- a/include/linux/mmc/mmc.h
> >>> +++ b/include/linux/mmc/mmc.h
> >>> @@ -270,6 +270,8 @@ struct _mmc_csd {
> >>>     * EXT_CSD fields
> >>>     */
> >>>
> >>> +#define EXT_CSD_FLUSH_CACHE              32      /* W */
> >>> +#define EXT_CSD_CACHE_CTRL               33      /* R/W */
> >>>    #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
> >>>    #define EXT_CSD_PARTITION_SUPPORT      160     /* RO */
> >>>    #define EXT_CSD_WR_REL_PARAM           166     /* RO */
> >>> @@ -294,6 +296,7 @@ struct _mmc_csd {
> >>>    #define EXT_CSD_SEC_FEATURE_SUPPORT    231     /* RO */
> >>>    #define EXT_CSD_TRIM_MULT              232     /* RO */
> >>>    #define EXT_CSD_GENERIC_CMD6_TIME      248     /* RO */
> >>> +#define EXT_CSD_CACHE_SIZE               249     /* RO, 4 bytes */
> >>>
> >>>    /*
> >>>     * EXT_CSD field definitions
> >>> --
> >>> 1.7.0.4
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
> in
> >>> the body of a message to [email protected]
> >>> 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 [email protected]
> >> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to