* [PATCH] mmc: support BKOPS feature for eMMC
@ 2011-10-27 11:10 Jaehoon Chung
2011-10-27 11:37 ` Dong, Chuanxiao
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Jaehoon Chung @ 2011-10-27 11:10 UTC (permalink / raw)
To: linux-mmc; +Cc: Chris Ball, Kyungmin Park, Hanumath Prasad
Background operation(BKOPS) is one of eMMC's features.
If set the URGENT_BKOPS in response, we can notify that card nedd the BKOPS.
And all I/O request is done, then run background operation.
If request read/write operation during BKOPS, issue HPI interrupt.
If you want to use this feature, should be set MMC_CAP2_BKOPS.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Hanumath Prasad <hanumath.prasad@stericsson.com>
---
drivers/mmc/card/block.c | 13 +++++++-
drivers/mmc/card/queue.c | 1 +
drivers/mmc/core/core.c | 77 ++++++++++++++++++++++++++++++++++++++++++++
drivers/mmc/core/mmc.c | 21 ++++++++++++
drivers/mmc/core/mmc_ops.c | 4 ++
include/linux/mmc/card.h | 11 ++++++
include/linux/mmc/core.h | 2 +
include/linux/mmc/host.h | 1 +
include/linux/mmc/mmc.h | 4 ++
9 files changed, 133 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 4fd5723..edbed7a 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1185,14 +1185,25 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
type = rq_data_dir(req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
mmc_queue_bounce_post(mq_rq);
+ if (mmc_card_doing_bkops(card)) {
+ if (mmc_interrupt_bkops(card))
+ goto cmd_abort;
+ }
+
switch (status) {
case MMC_BLK_SUCCESS:
case MMC_BLK_PARTIAL:
+ spin_lock_irq(&md->lock);
+ /*
+ * Check BKOPS urgency from each R1 response
+ */
+ if (mmc_card_mmc(card) &&
+ (brq->cmd.resp[0] & R1_URGENT_BKOPS))
+ mmc_card_set_need_bkops(card);
/*
* A block was successfully transferred.
*/
mmc_blk_reset_success(md, type);
- spin_lock_irq(&md->lock);
ret = __blk_end_request(req, 0,
brq->data.bytes_xfered);
spin_unlock_irq(&md->lock);
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..31e5eca 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
set_current_state(TASK_RUNNING);
break;
}
+ mmc_start_bkops(mq->card);
up(&mq->thread_sem);
schedule();
down(&mq->thread_sem);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5278ffb..7ef270d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
host->ops->request(host, mrq);
}
+/**
+ * mmc_start_bkops - start BKOPS for supported cards
+ * @card: MMC card to start BKOPS
+ *
+ * Start background operations whenever requested.
+ * when the urgent BKOPS bit is set in a R1 command response
+ * then background operations should be started immediately.
+*/
+void mmc_start_bkops(struct mmc_card *card)
+{
+ int err;
+ unsigned long flags;
+
+ if ((!card || !card->ext_csd.bkops_en) &&
+ !(card->host->caps2 & MMC_CAP2_BKOPS))
+ return;
+
+ /*
+ * If card is already doing bkops or need for
+ * bkops flag is not set, then do nothing just
+ * return
+ */
+ if (mmc_card_doing_bkops(card)
+ || !mmc_card_need_bkops(card))
+ return;
+
+ mmc_claim_host(card->host);
+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_BKOPS_START, 1, 0);
+ if (err) {
+ pr_warning("error %d starting bkops\n", err);
+ mmc_card_clr_need_bkops(card);
+ goto out;
+ }
+ spin_lock_irqsave(&card->host->lock, flags);
+ mmc_card_clr_need_bkops(card);
+ mmc_card_set_doing_bkops(card);
+ spin_unlock_irqrestore(&card->host->lock, flags);
+out:
+ mmc_release_host(card->host);
+}
+EXPORT_SYMBOL(mmc_start_bkops);
+
+
static void mmc_wait_done(struct mmc_request *mrq)
{
complete(&mrq->completion);
@@ -466,6 +510,39 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
EXPORT_SYMBOL(mmc_wait_for_cmd);
/**
+ * mmc_interrupt_bkops - interrupt ongoing BKOPS
+ * @card: MMC card to check BKOPS
+ *
+ * Send HPI command to interrupt ongoing background operations,
+ * to allow rapid servicing of foreground operations,e.g. read/
+ * writes. Wait until the card comes out of the programming state
+ * to avoid errors in servicing read/write requests.
+ */
+int mmc_interrupt_bkops(struct mmc_card *card)
+{
+ int err;
+ unsigned long flags;
+ u32 status;
+
+ BUG_ON(!card);
+
+ /* send HPI to interrupt BKOPS. */
+ err = mmc_send_hpi_cmd(card, &status);
+ if (err || R1_CURRENT_STATE(status) == 7) {
+ do {
+ err = mmc_send_status(card, &status);
+ if (err)
+ return err;
+ } while (R1_CURRENT_STATE(status) == 7);
+ }
+ spin_lock_irqsave(&card->host->lock, flags);
+ mmc_card_clr_doing_bkops(card);
+ spin_unlock_irqrestore(&card->host->lock, flags);
+ return 0;
+}
+EXPORT_SYMBOL(mmc_interrupt_bkops);
+
+/**
* mmc_set_data_timeout - set the timeout for a data command
* @data: data phase for command
* @card: the MMC card associated with the data transfer
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 3627044..b16db45 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -448,6 +448,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
}
if (card->ext_csd.rev >= 5) {
+ /* check whether the eMMC card support BKOPS */
+ if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)
+ card->ext_csd.bkops = 1;
+
/* check whether the eMMC card supports HPI */
if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
card->ext_csd.hpi = 1;
@@ -909,6 +913,23 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
}
/*
+ * enable BKOPS feature (if supported)
+ */
+ if (card->ext_csd.bkops) {
+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_BKOPS_EN, 1, 0);
+ if (err && err != -EBADMSG)
+ goto free_card;
+
+ if (err) {
+ pr_warning("%s: Enabling BKOPS failed\n",
+ mmc_hostname(card->host));
+ err = 0;
+ } else
+ card->ext_csd.bkops_en = 1;
+ }
+
+ /*
* Enable HPI feature (if supported)
*/
if (card->ext_csd.hpi) {
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 007863e..7653e33 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -398,6 +398,10 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
if (err)
return err;
+ /* No need to check card status in case of BKOPS switch*/
+ if (index == EXT_CSD_BKOPS_START)
+ return 0;
+
/* Must check status to be sure of no errors */
do {
err = mmc_send_status(card, &status);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 6e04e10..f4abb7c 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -71,6 +71,8 @@ struct mmc_ext_csd {
bool hpi_en; /* HPI enablebit */
bool hpi; /* HPI support bit */
unsigned int hpi_cmd; /* cmd used as HPI */
+ bool bkops; /* background support bit */
+ bool bkops_en; /* background enable bit */
u8 raw_partition_support; /* 160 */
u8 raw_erased_mem_count; /* 181 */
u8 raw_ext_csd_structure; /* 194 */
@@ -206,6 +208,8 @@ struct mmc_card {
#define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode */
#define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in ultra high speed mode */
#define MMC_CARD_SDXC (1<<6) /* card is SDXC */
+#define MMC_STATE_NEED_BKOPS (1<<7) /* card need to do BKOPS */
+#define MMC_STATE_DOING_BKOPS (1<<8) /* card is doing BKOPS */
unsigned int quirks; /* card quirks */
#define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */
#define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */
@@ -365,6 +369,8 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
#define mmc_card_ddr_mode(c) ((c)->state & MMC_STATE_HIGHSPEED_DDR)
#define mmc_sd_card_uhs(c) ((c)->state & MMC_STATE_ULTRAHIGHSPEED)
#define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
+#define mmc_card_need_bkops(c) ((c)->state & MMC_STATE_NEED_BKOPS)
+#define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
#define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
@@ -373,6 +379,11 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
#define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR)
#define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
#define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
+#define mmc_card_set_need_bkops(c) ((c)->state |= MMC_STATE_NEED_BKOPS)
+#define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS)
+
+#define mmc_card_clr_need_bkops(c) ((c)->state &= ~MMC_STATE_NEED_BKOPS)
+#define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS)
/*
* Quirk add/remove for MMC products.
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 174a844..600a177 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -134,6 +134,7 @@ struct mmc_host;
struct mmc_card;
struct mmc_async_req;
+extern int mmc_interrupt_bkops(struct mmc_card *);
extern struct mmc_async_req *mmc_start_req(struct mmc_host *,
struct mmc_async_req *, int *);
extern int mmc_interrupt_hpi(struct mmc_card *);
@@ -163,6 +164,7 @@ extern int mmc_can_sanitize(struct mmc_card *card);
extern int mmc_can_secure_erase_trim(struct mmc_card *card);
extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
unsigned int nr);
+extern void mmc_start_bkops(struct mmc_card *card);
extern unsigned int mmc_calc_max_discard(struct mmc_card *card);
extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index a3ac9c4..692e5f6 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -242,6 +242,7 @@ struct mmc_host {
#define MMC_CAP2_CACHE_CTRL (1 << 1) /* Allow cache control */
#define MMC_CAP2_POWEROFF_NOTIFY (1 << 2) /* Notify poweroff supported */
#define MMC_CAP2_NO_MULTI_READ (1 << 3) /* Multiblock reads don't work */
+#define MMC_CAP2_BKOPS (1 << 4) /* BKOPS supported */
mmc_pm_flag_t pm_caps; /* supported pm features */
unsigned int power_notify_type;
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 0e71356..283b848 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -138,6 +138,7 @@ static inline bool mmc_op_multi(u32 opcode)
#define R1_CURRENT_STATE(x) ((x & 0x00001E00) >> 9) /* sx, b (4 bits) */
#define R1_READY_FOR_DATA (1 << 8) /* sx, a */
#define R1_SWITCH_ERROR (1 << 7) /* sx, c */
+#define R1_URGENT_BKOPS (1 << 6) /* sx, a */
#define R1_APP_CMD (1 << 5) /* sr, c */
#define R1_STATE_IDLE 0
@@ -278,6 +279,8 @@ struct _mmc_csd {
#define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
#define EXT_CSD_HPI_MGMT 161 /* R/W */
#define EXT_CSD_RST_N_FUNCTION 162 /* R/W */
+#define EXT_CSD_BKOPS_EN 163 /* R/W */
+#define EXT_CSD_BKOPS_START 164 /* W */
#define EXT_CSD_SANITIZE_START 165 /* W */
#define EXT_CSD_WR_REL_PARAM 166 /* RO */
#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
@@ -313,6 +316,7 @@ struct _mmc_csd {
#define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
#define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
+#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
#define EXT_CSD_HPI_FEATURES 503 /* RO */
/*
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 11:10 [PATCH] mmc: support BKOPS feature for eMMC Jaehoon Chung
@ 2011-10-27 11:37 ` Dong, Chuanxiao
2011-10-27 12:56 ` Kyungmin Park
2011-10-27 20:31 ` Sebastian Rasmussen
2011-10-27 19:26 ` Sebastian Rasmussen
2011-10-27 19:35 ` Per Forlin
2 siblings, 2 replies; 17+ messages in thread
From: Dong, Chuanxiao @ 2011-10-27 11:37 UTC (permalink / raw)
To: Jaehoon Chung, linux-mmc; +Cc: Chris Ball, Kyungmin Park, Hanumath Prasad
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Jaehoon Chung
> Sent: Thursday, October 27, 2011 7:10 PM
> To: linux-mmc
> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad
> Subject: [PATCH] mmc: support BKOPS feature for eMMC
>
> Background operation(BKOPS) is one of eMMC's features.
>
> If set the URGENT_BKOPS in response, we can notify that card nedd the BKOPS.
> And all I/O request is done, then run background operation.
> If request read/write operation during BKOPS, issue HPI interrupt.
>
> If you want to use this feature, should be set MMC_CAP2_BKOPS.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Hanumath Prasad <hanumath.prasad@stericsson.com>
> ---
> drivers/mmc/card/block.c | 13 +++++++-
> drivers/mmc/card/queue.c | 1 +
> drivers/mmc/core/core.c | 77
> ++++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/core/mmc.c | 21 ++++++++++++
> drivers/mmc/core/mmc_ops.c | 4 ++
> include/linux/mmc/card.h | 11 ++++++
> include/linux/mmc/core.h | 2 +
> include/linux/mmc/host.h | 1 +
> include/linux/mmc/mmc.h | 4 ++
> 9 files changed, 133 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 4fd5723..edbed7a 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1185,14 +1185,25 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
> *mq, struct request *rqc)
> type = rq_data_dir(req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
> mmc_queue_bounce_post(mq_rq);
>
> + if (mmc_card_doing_bkops(card)) {
> + if (mmc_interrupt_bkops(card))
> + goto cmd_abort;
> + }
> +
> switch (status) {
> case MMC_BLK_SUCCESS:
> case MMC_BLK_PARTIAL:
> + spin_lock_irq(&md->lock);
> + /*
> + * Check BKOPS urgency from each R1 response
> + */
> + if (mmc_card_mmc(card) &&
> + (brq->cmd.resp[0] & R1_URGENT_BKOPS))
> + mmc_card_set_need_bkops(card);
> /*
> * A block was successfully transferred.
> */
> mmc_blk_reset_success(md, type);
> - spin_lock_irq(&md->lock);
> ret = __blk_end_request(req, 0,
> brq->data.bytes_xfered);
> spin_unlock_irq(&md->lock);
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index dcad59c..31e5eca 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
> set_current_state(TASK_RUNNING);
> break;
> }
> + mmc_start_bkops(mq->card);
> up(&mq->thread_sem);
> schedule();
> down(&mq->thread_sem);
Maybe put mmc_interrupt_bkops(card) here is better, I think.
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 5278ffb..7ef270d 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct
> mmc_request *mrq)
> host->ops->request(host, mrq);
> }
>
> +/**
> + * mmc_start_bkops - start BKOPS for supported cards
> + * @card: MMC card to start BKOPS
> + *
> + * Start background operations whenever requested.
> + * when the urgent BKOPS bit is set in a R1 command response
> + * then background operations should be started immediately.
> +*/
> +void mmc_start_bkops(struct mmc_card *card)
> +{
> + int err;
> + unsigned long flags;
> +
> + if ((!card || !card->ext_csd.bkops_en) &&
> + !(card->host->caps2 & MMC_CAP2_BKOPS))
> + return;
> +
> + /*
> + * If card is already doing bkops or need for
> + * bkops flag is not set, then do nothing just
> + * return
> + */
> + if (mmc_card_doing_bkops(card)
> + || !mmc_card_need_bkops(card))
> + return;
> +
> + mmc_claim_host(card->host);
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_BKOPS_START, 1, 0);
> + if (err) {
> + pr_warning("error %d starting bkops\n", err);
> + mmc_card_clr_need_bkops(card);
> + goto out;
> + }
> + spin_lock_irqsave(&card->host->lock, flags);
> + mmc_card_clr_need_bkops(card);
> + mmc_card_set_doing_bkops(card);
> + spin_unlock_irqrestore(&card->host->lock, flags);
> +out:
> + mmc_release_host(card->host);
> +}
> +EXPORT_SYMBOL(mmc_start_bkops);
> +
> +
> static void mmc_wait_done(struct mmc_request *mrq)
> {
> complete(&mrq->completion);
> @@ -466,6 +510,39 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct
> mmc_command *cmd, int retries
> EXPORT_SYMBOL(mmc_wait_for_cmd);
>
> /**
> + * mmc_interrupt_bkops - interrupt ongoing BKOPS
> + * @card: MMC card to check BKOPS
> + *
> + * Send HPI command to interrupt ongoing background operations,
> + * to allow rapid servicing of foreground operations,e.g. read/
> + * writes. Wait until the card comes out of the programming state
> + * to avoid errors in servicing read/write requests.
> + */
> +int mmc_interrupt_bkops(struct mmc_card *card)
> +{
> + int err;
> + unsigned long flags;
> + u32 status;
> +
> + BUG_ON(!card);
> +
> + /* send HPI to interrupt BKOPS. */
> + err = mmc_send_hpi_cmd(card, &status);
What will be happened if eMMC card only support BKOPS but not support HPI?
> + if (err || R1_CURRENT_STATE(status) == 7) {
> + do {
> + err = mmc_send_status(card, &status);
> + if (err)
> + return err;
> + } while (R1_CURRENT_STATE(status) == 7);
> + }
> + spin_lock_irqsave(&card->host->lock, flags);
> + mmc_card_clr_doing_bkops(card);
> + spin_unlock_irqrestore(&card->host->lock, flags);
> + return 0;
> +}
> +EXPORT_SYMBOL(mmc_interrupt_bkops);
> +
> +/**
> * mmc_set_data_timeout - set the timeout for a data command
> * @data: data phase for command
> * @card: the MMC card associated with the data transfer
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3627044..b16db45 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -448,6 +448,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8
> *ext_csd)
> }
>
> if (card->ext_csd.rev >= 5) {
> + /* check whether the eMMC card support BKOPS */
> + if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)
> + card->ext_csd.bkops = 1;
> +
> /* check whether the eMMC card supports HPI */
> if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
> card->ext_csd.hpi = 1;
> @@ -909,6 +913,23 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> }
>
> /*
> + * enable BKOPS feature (if supported)
> + */
> + if (card->ext_csd.bkops) {
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_BKOPS_EN, 1, 0);
> + if (err && err != -EBADMSG)
> + goto free_card;
> +
> + if (err) {
> + pr_warning("%s: Enabling BKOPS failed\n",
> + mmc_hostname(card->host));
> + err = 0;
> + } else
> + card->ext_csd.bkops_en = 1;
> + }
Just found this bit is one time programmable, so that is to say this bit is not suitable for driver to change.
> +
> + /*
> * Enable HPI feature (if supported)
> */
> if (card->ext_csd.hpi) {
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 007863e..7653e33 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -398,6 +398,10 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index,
> u8 value,
> if (err)
> return err;
>
> + /* No need to check card status in case of BKOPS switch*/
> + if (index == EXT_CSD_BKOPS_START)
> + return 0;
> +
> /* Must check status to be sure of no errors */
> do {
> err = mmc_send_status(card, &status);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 6e04e10..f4abb7c 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -71,6 +71,8 @@ struct mmc_ext_csd {
> bool hpi_en; /* HPI enablebit */
> bool hpi; /* HPI support bit */
> unsigned int hpi_cmd; /* cmd used as HPI */
> + bool bkops; /* background support bit */
> + bool bkops_en; /* background enable bit */
> u8 raw_partition_support; /* 160 */
> u8 raw_erased_mem_count; /* 181 */
> u8 raw_ext_csd_structure; /* 194 */
> @@ -206,6 +208,8 @@ struct mmc_card {
> #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode
> */
> #define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in ultra high speed
> mode */
> #define MMC_CARD_SDXC (1<<6) /* card is SDXC */
> +#define MMC_STATE_NEED_BKOPS (1<<7) /* card need to do BKOPS */
> +#define MMC_STATE_DOING_BKOPS (1<<8) /* card is doing BKOPS */
> unsigned int quirks; /* card quirks */
> #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes
> outside of the VS CCCR range */
> #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize
> */
> @@ -365,6 +369,8 @@ static inline void __maybe_unused remove_quirk(struct
> mmc_card *card, int data)
> #define mmc_card_ddr_mode(c) ((c)->state & MMC_STATE_HIGHSPEED_DDR)
> #define mmc_sd_card_uhs(c) ((c)->state & MMC_STATE_ULTRAHIGHSPEED)
> #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
> +#define mmc_card_need_bkops(c) ((c)->state & MMC_STATE_NEED_BKOPS)
> +#define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
>
> #define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
> #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
> @@ -373,6 +379,11 @@ static inline void __maybe_unused remove_quirk(struct
> mmc_card *card, int data)
> #define mmc_card_set_ddr_mode(c) ((c)->state |=
> MMC_STATE_HIGHSPEED_DDR)
> #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
> #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
> +#define mmc_card_set_need_bkops(c) ((c)->state |=
> MMC_STATE_NEED_BKOPS)
> +#define mmc_card_set_doing_bkops(c) ((c)->state |=
> MMC_STATE_DOING_BKOPS)
> +
> +#define mmc_card_clr_need_bkops(c) ((c)->state &= ~MMC_STATE_NEED_BKOPS)
> +#define mmc_card_clr_doing_bkops(c) ((c)->state &=
> ~MMC_STATE_DOING_BKOPS)
>
> /*
> * Quirk add/remove for MMC products.
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 174a844..600a177 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -134,6 +134,7 @@ struct mmc_host;
> struct mmc_card;
> struct mmc_async_req;
>
> +extern int mmc_interrupt_bkops(struct mmc_card *);
> extern struct mmc_async_req *mmc_start_req(struct mmc_host *,
> struct mmc_async_req *, int *);
> extern int mmc_interrupt_hpi(struct mmc_card *);
> @@ -163,6 +164,7 @@ extern int mmc_can_sanitize(struct mmc_card *card);
> extern int mmc_can_secure_erase_trim(struct mmc_card *card);
> extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
> unsigned int nr);
> +extern void mmc_start_bkops(struct mmc_card *card);
> extern unsigned int mmc_calc_max_discard(struct mmc_card *card);
>
> extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index a3ac9c4..692e5f6 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -242,6 +242,7 @@ struct mmc_host {
> #define MMC_CAP2_CACHE_CTRL (1 << 1) /* Allow cache control */
> #define MMC_CAP2_POWEROFF_NOTIFY (1 << 2) /* Notify poweroff supported
> */
> #define MMC_CAP2_NO_MULTI_READ (1 << 3) /* Multiblock reads don't
> work */
> +#define MMC_CAP2_BKOPS (1 << 4) /* BKOPS supported */
>
> mmc_pm_flag_t pm_caps; /* supported pm features */
> unsigned int power_notify_type;
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 0e71356..283b848 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -138,6 +138,7 @@ static inline bool mmc_op_multi(u32 opcode)
> #define R1_CURRENT_STATE(x) ((x & 0x00001E00) >> 9) /* sx, b (4 bits) */
> #define R1_READY_FOR_DATA (1 << 8) /* sx, a */
> #define R1_SWITCH_ERROR (1 << 7) /* sx, c */
> +#define R1_URGENT_BKOPS (1 << 6) /* sx, a */
> #define R1_APP_CMD (1 << 5) /* sr, c */
>
> #define R1_STATE_IDLE 0
> @@ -278,6 +279,8 @@ struct _mmc_csd {
> #define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
> #define EXT_CSD_HPI_MGMT 161 /* R/W */
> #define EXT_CSD_RST_N_FUNCTION 162 /* R/W */
> +#define EXT_CSD_BKOPS_EN 163 /* R/W */
> +#define EXT_CSD_BKOPS_START 164 /* W */
> #define EXT_CSD_SANITIZE_START 165 /* W */
> #define EXT_CSD_WR_REL_PARAM 166 /* RO */
> #define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
> @@ -313,6 +316,7 @@ struct _mmc_csd {
> #define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
> #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
> #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
> +#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
> #define EXT_CSD_HPI_FEATURES 503 /* RO */
>
> /*
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 11:37 ` Dong, Chuanxiao
@ 2011-10-27 12:56 ` Kyungmin Park
2011-10-27 20:33 ` Sebastian Rasmussen
2011-10-27 20:31 ` Sebastian Rasmussen
1 sibling, 1 reply; 17+ messages in thread
From: Kyungmin Park @ 2011-10-27 12:56 UTC (permalink / raw)
To: Dong, Chuanxiao; +Cc: Jaehoon Chung, linux-mmc, Chris Ball, Hanumath Prasad
Hi,
On Thu, Oct 27, 2011 at 8:37 PM, Dong, Chuanxiao
<chuanxiao.dong@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org
>> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Jaehoon Chung
>> Sent: Thursday, October 27, 2011 7:10 PM
>> To: linux-mmc
>> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad
>> Subject: [PATCH] mmc: support BKOPS feature for eMMC
>>
>> Background operation(BKOPS) is one of eMMC's features.
>>
>> If set the URGENT_BKOPS in response, we can notify that card nedd the BKOPS.
>> And all I/O request is done, then run background operation.
>> If request read/write operation during BKOPS, issue HPI interrupt.
>>
>> If you want to use this feature, should be set MMC_CAP2_BKOPS.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> CC: Hanumath Prasad <hanumath.prasad@stericsson.com>
>> ---
>> drivers/mmc/card/block.c | 13 +++++++-
>> drivers/mmc/card/queue.c | 1 +
>> drivers/mmc/core/core.c | 77
>> ++++++++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/core/mmc.c | 21 ++++++++++++
>> drivers/mmc/core/mmc_ops.c | 4 ++
>> include/linux/mmc/card.h | 11 ++++++
>> include/linux/mmc/core.h | 2 +
>> include/linux/mmc/host.h | 1 +
>> include/linux/mmc/mmc.h | 4 ++
>> 9 files changed, 133 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index 4fd5723..edbed7a 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1185,14 +1185,25 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>> *mq, struct request *rqc)
>> type = rq_data_dir(req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
>> mmc_queue_bounce_post(mq_rq);
>>
>> + if (mmc_card_doing_bkops(card)) {
>> + if (mmc_interrupt_bkops(card))
>> + goto cmd_abort;
>> + }
>> +
>> switch (status) {
>> case MMC_BLK_SUCCESS:
>> case MMC_BLK_PARTIAL:
>> + spin_lock_irq(&md->lock);
>> + /*
>> + * Check BKOPS urgency from each R1 response
>> + */
>> + if (mmc_card_mmc(card) &&
>> + (brq->cmd.resp[0] & R1_URGENT_BKOPS))
>> + mmc_card_set_need_bkops(card);
>> /*
>> * A block was successfully transferred.
>> */
>> mmc_blk_reset_success(md, type);
>> - spin_lock_irq(&md->lock);
>> ret = __blk_end_request(req, 0,
>> brq->data.bytes_xfered);
>> spin_unlock_irq(&md->lock);
>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>> index dcad59c..31e5eca 100644
>> --- a/drivers/mmc/card/queue.c
>> +++ b/drivers/mmc/card/queue.c
>> @@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
>> set_current_state(TASK_RUNNING);
>> break;
>> }
>> + mmc_start_bkops(mq->card);
>> up(&mq->thread_sem);
>> schedule();
>> down(&mq->thread_sem);
> Maybe put mmc_interrupt_bkops(card) here is better, I think.
>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 5278ffb..7ef270d 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct
>> mmc_request *mrq)
>> host->ops->request(host, mrq);
>> }
>>
>> +/**
>> + * mmc_start_bkops - start BKOPS for supported cards
>> + * @card: MMC card to start BKOPS
>> + *
>> + * Start background operations whenever requested.
>> + * when the urgent BKOPS bit is set in a R1 command response
>> + * then background operations should be started immediately.
>> +*/
>> +void mmc_start_bkops(struct mmc_card *card)
>> +{
>> + int err;
>> + unsigned long flags;
>> +
>> + if ((!card || !card->ext_csd.bkops_en) &&
>> + !(card->host->caps2 & MMC_CAP2_BKOPS))
>> + return;
>> +
>> + /*
>> + * If card is already doing bkops or need for
>> + * bkops flag is not set, then do nothing just
>> + * return
>> + */
>> + if (mmc_card_doing_bkops(card)
>> + || !mmc_card_need_bkops(card))
>> + return;
>> +
>> + mmc_claim_host(card->host);
>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> + EXT_CSD_BKOPS_START, 1, 0);
>> + if (err) {
>> + pr_warning("error %d starting bkops\n", err);
>> + mmc_card_clr_need_bkops(card);
>> + goto out;
>> + }
>> + spin_lock_irqsave(&card->host->lock, flags);
>> + mmc_card_clr_need_bkops(card);
>> + mmc_card_set_doing_bkops(card);
>> + spin_unlock_irqrestore(&card->host->lock, flags);
>> +out:
>> + mmc_release_host(card->host);
>> +}
>> +EXPORT_SYMBOL(mmc_start_bkops);
>> +
>> +
>> static void mmc_wait_done(struct mmc_request *mrq)
>> {
>> complete(&mrq->completion);
>> @@ -466,6 +510,39 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct
>> mmc_command *cmd, int retries
>> EXPORT_SYMBOL(mmc_wait_for_cmd);
>>
>> /**
>> + * mmc_interrupt_bkops - interrupt ongoing BKOPS
>> + * @card: MMC card to check BKOPS
>> + *
>> + * Send HPI command to interrupt ongoing background operations,
>> + * to allow rapid servicing of foreground operations,e.g. read/
>> + * writes. Wait until the card comes out of the programming state
>> + * to avoid errors in servicing read/write requests.
>> + */
>> +int mmc_interrupt_bkops(struct mmc_card *card)
>> +{
>> + int err;
>> + unsigned long flags;
>> + u32 status;
>> +
>> + BUG_ON(!card);
>> +
>> + /* send HPI to interrupt BKOPS. */
>> + err = mmc_send_hpi_cmd(card, &status);
> What will be happened if eMMC card only support BKOPS but not support HPI?
Basically v4.41 and v4.5 doesn't mentioned the BKOPS timeout so if
there's no HPI command, it should wait until BKOPS is done. It seems
to be shown system hang.
>
>> + if (err || R1_CURRENT_STATE(status) == 7) {
>> + do {
>> + err = mmc_send_status(card, &status);
>> + if (err)
>> + return err;
>> + } while (R1_CURRENT_STATE(status) == 7);
>> + }
>> + spin_lock_irqsave(&card->host->lock, flags);
>> + mmc_card_clr_doing_bkops(card);
>> + spin_unlock_irqrestore(&card->host->lock, flags);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>> +
>> +/**
>> * mmc_set_data_timeout - set the timeout for a data command
>> * @data: data phase for command
>> * @card: the MMC card associated with the data transfer
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 3627044..b16db45 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -448,6 +448,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8
>> *ext_csd)
>> }
>>
>> if (card->ext_csd.rev >= 5) {
>> + /* check whether the eMMC card support BKOPS */
>> + if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)
>> + card->ext_csd.bkops = 1;
>> +
>> /* check whether the eMMC card supports HPI */
>> if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>> card->ext_csd.hpi = 1;
>> @@ -909,6 +913,23 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>> }
>>
>> /*
>> + * enable BKOPS feature (if supported)
>> + */
>> + if (card->ext_csd.bkops) {
>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> + EXT_CSD_BKOPS_EN, 1, 0);
>> + if (err && err != -EBADMSG)
>> + goto free_card;
>> +
>> + if (err) {
>> + pr_warning("%s: Enabling BKOPS failed\n",
>> + mmc_hostname(card->host));
>> + err = 0;
>> + } else
>> + card->ext_csd.bkops_en = 1;
>> + }
> Just found this bit is one time programmable, so that is to say this bit is not suitable for driver to change.
You mean require the BKOPS_EN_FORCE config if user want to use at kernel?
>
>> +
>> + /*
>> * Enable HPI feature (if supported)
>> */
>> if (card->ext_csd.hpi) {
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 007863e..7653e33 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -398,6 +398,10 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index,
>> u8 value,
>> if (err)
>> return err;
>>
>> + /* No need to check card status in case of BKOPS switch*/
>> + if (index == EXT_CSD_BKOPS_START)
>> + return 0;
>> +
>> /* Must check status to be sure of no errors */
>> do {
>> err = mmc_send_status(card, &status);
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index 6e04e10..f4abb7c 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -71,6 +71,8 @@ struct mmc_ext_csd {
>> bool hpi_en; /* HPI enablebit */
>> bool hpi; /* HPI support bit */
>> unsigned int hpi_cmd; /* cmd used as HPI */
>> + bool bkops; /* background support bit */
>> + bool bkops_en; /* background enable bit */
>> u8 raw_partition_support; /* 160 */
>> u8 raw_erased_mem_count; /* 181 */
>> u8 raw_ext_csd_structure; /* 194 */
>> @@ -206,6 +208,8 @@ struct mmc_card {
>> #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode
>> */
>> #define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in ultra high speed
>> mode */
>> #define MMC_CARD_SDXC (1<<6) /* card is SDXC */
>> +#define MMC_STATE_NEED_BKOPS (1<<7) /* card need to do BKOPS */
>> +#define MMC_STATE_DOING_BKOPS (1<<8) /* card is doing BKOPS */
>> unsigned int quirks; /* card quirks */
>> #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes
>> outside of the VS CCCR range */
>> #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize
>> */
>> @@ -365,6 +369,8 @@ static inline void __maybe_unused remove_quirk(struct
>> mmc_card *card, int data)
>> #define mmc_card_ddr_mode(c) ((c)->state & MMC_STATE_HIGHSPEED_DDR)
>> #define mmc_sd_card_uhs(c) ((c)->state & MMC_STATE_ULTRAHIGHSPEED)
>> #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>> +#define mmc_card_need_bkops(c) ((c)->state & MMC_STATE_NEED_BKOPS)
>> +#define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
>>
>> #define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
>> #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
>> @@ -373,6 +379,11 @@ static inline void __maybe_unused remove_quirk(struct
>> mmc_card *card, int data)
>> #define mmc_card_set_ddr_mode(c) ((c)->state |=
>> MMC_STATE_HIGHSPEED_DDR)
>> #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
>> #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
>> +#define mmc_card_set_need_bkops(c) ((c)->state |=
>> MMC_STATE_NEED_BKOPS)
>> +#define mmc_card_set_doing_bkops(c) ((c)->state |=
>> MMC_STATE_DOING_BKOPS)
>> +
>> +#define mmc_card_clr_need_bkops(c) ((c)->state &= ~MMC_STATE_NEED_BKOPS)
>> +#define mmc_card_clr_doing_bkops(c) ((c)->state &=
>> ~MMC_STATE_DOING_BKOPS)
>>
>> /*
>> * Quirk add/remove for MMC products.
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index 174a844..600a177 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -134,6 +134,7 @@ struct mmc_host;
>> struct mmc_card;
>> struct mmc_async_req;
>>
>> +extern int mmc_interrupt_bkops(struct mmc_card *);
>> extern struct mmc_async_req *mmc_start_req(struct mmc_host *,
>> struct mmc_async_req *, int *);
>> extern int mmc_interrupt_hpi(struct mmc_card *);
>> @@ -163,6 +164,7 @@ extern int mmc_can_sanitize(struct mmc_card *card);
>> extern int mmc_can_secure_erase_trim(struct mmc_card *card);
>> extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
>> unsigned int nr);
>> +extern void mmc_start_bkops(struct mmc_card *card);
>> extern unsigned int mmc_calc_max_discard(struct mmc_card *card);
>>
>> extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index a3ac9c4..692e5f6 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -242,6 +242,7 @@ struct mmc_host {
>> #define MMC_CAP2_CACHE_CTRL (1 << 1) /* Allow cache control */
>> #define MMC_CAP2_POWEROFF_NOTIFY (1 << 2) /* Notify poweroff supported
>> */
>> #define MMC_CAP2_NO_MULTI_READ (1 << 3) /* Multiblock reads don't
>> work */
>> +#define MMC_CAP2_BKOPS (1 << 4) /* BKOPS supported */
>>
>> mmc_pm_flag_t pm_caps; /* supported pm features */
>> unsigned int power_notify_type;
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 0e71356..283b848 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -138,6 +138,7 @@ static inline bool mmc_op_multi(u32 opcode)
>> #define R1_CURRENT_STATE(x) ((x & 0x00001E00) >> 9) /* sx, b (4 bits) */
>> #define R1_READY_FOR_DATA (1 << 8) /* sx, a */
>> #define R1_SWITCH_ERROR (1 << 7) /* sx, c */
>> +#define R1_URGENT_BKOPS (1 << 6) /* sx, a */
>> #define R1_APP_CMD (1 << 5) /* sr, c */
>>
>> #define R1_STATE_IDLE 0
>> @@ -278,6 +279,8 @@ struct _mmc_csd {
>> #define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
>> #define EXT_CSD_HPI_MGMT 161 /* R/W */
>> #define EXT_CSD_RST_N_FUNCTION 162 /* R/W */
>> +#define EXT_CSD_BKOPS_EN 163 /* R/W */
>> +#define EXT_CSD_BKOPS_START 164 /* W */
>> #define EXT_CSD_SANITIZE_START 165 /* W */
>> #define EXT_CSD_WR_REL_PARAM 166 /* RO */
>> #define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
>> @@ -313,6 +316,7 @@ struct _mmc_csd {
>> #define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
>> #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
>> #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
>> +#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
>> #define EXT_CSD_HPI_FEATURES 503 /* RO */
>>
>> /*
>> --
>> 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
> --
> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 11:10 [PATCH] mmc: support BKOPS feature for eMMC Jaehoon Chung
2011-10-27 11:37 ` Dong, Chuanxiao
@ 2011-10-27 19:26 ` Sebastian Rasmussen
2011-10-28 7:25 ` Jaehoon Chung
2011-10-27 19:35 ` Per Forlin
2 siblings, 1 reply; 17+ messages in thread
From: Sebastian Rasmussen @ 2011-10-27 19:26 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad
> Background operation(BKOPS) is one of eMMC's features.
>
> If set the URGENT_BKOPS in response, we can notify that card nedd the BKOPS.
> And all I/O request is done, then run background operation.
> If request read/write operation during BKOPS, issue HPI interrupt.
>
> If you want to use this feature, should be set MMC_CAP2_BKOPS.
Enable eMMC background operations (BKOPS) feature.
If URGENT_BKOPS is set after a response, note that BKOPS
are required. After all I/O requests are finished, run
BKOPS if required. Should read/write operations be requested
during BKOPS, first issue HPI to interrupt the ongoing BKOPS
and then service the request.
If you want to enable this feature, set MMC_CAP2_BKOPS.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Hanumath Prasad <hanumath.prasad@stericsson.com>
> ---
> drivers/mmc/card/block.c | 13 +++++++-
> drivers/mmc/card/queue.c | 1 +
> drivers/mmc/core/core.c | 77 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/core/mmc.c | 21 ++++++++++++
> drivers/mmc/core/mmc_ops.c | 4 ++
> include/linux/mmc/card.h | 11 ++++++
> include/linux/mmc/core.h | 2 +
> include/linux/mmc/host.h | 1 +
> include/linux/mmc/mmc.h | 4 ++
> 9 files changed, 133 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 4fd5723..edbed7a 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1185,14 +1185,25 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> type = rq_data_dir(req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
> mmc_queue_bounce_post(mq_rq);
>
> + if (mmc_card_doing_bkops(card)) {
> + if (mmc_interrupt_bkops(card))
> + goto cmd_abort;
> + }
> +
> switch (status) {
> case MMC_BLK_SUCCESS:
> case MMC_BLK_PARTIAL:
> + spin_lock_irq(&md->lock);
> + /*
> + * Check BKOPS urgency from each R1 response
> + */
> + if (mmc_card_mmc(card) &&
> + (brq->cmd.resp[0] & R1_URGENT_BKOPS))
> + mmc_card_set_need_bkops(card);
First of all, R1_URGENT_BKOPS is fine to check in cmd->resp[0], but it
is only valid if cmd->resp contains an R1 or an R1b response. Granted,
commands that do not have R1/R1b responses are few, but I still
believe you need to check that with mmc_resp_type().
Moreover in the eMMC 4.41 spec it was ok to check bit 6 of the status
(which is what you do above) as that bit corresponded to URGENT_BKOPS.
However for eMMC 4.5 this bit has been combined with other kinds of of
exceptional situations. The bit has been renamed EXCEPTION_EVENT and
if it is set you then need to read (using CMD8) bytes 55:54 in EXT_CSD
named EXCEPTION_EVENTS_STATUS where bit 0 indicates URGENT_BKOPS.
So I you need to check card->ext_csd.rev here to know what method you
need to apply to determine if BKOPS are urgent or not.
> /*
> * A block was successfully transferred.
> */
> mmc_blk_reset_success(md, type);
> - spin_lock_irq(&md->lock);
> ret = __blk_end_request(req, 0,
> brq->data.bytes_xfered);
> spin_unlock_irq(&md->lock);
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index dcad59c..31e5eca 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
> set_current_state(TASK_RUNNING);
> break;
> }
> + mmc_start_bkops(mq->card);
> up(&mq->thread_sem);
> schedule();
> down(&mq->thread_sem);
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 5278ffb..7ef270d 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
> host->ops->request(host, mrq);
> }
>
> +/**
> + * mmc_start_bkops - start BKOPS for supported cards
> + * @card: MMC card to start BKOPS
> + *
> + * Start background operations whenever requested.
> + * when the urgent BKOPS bit is set in a R1 command response
> + * then background operations should be started immediately.
> +*/
> +void mmc_start_bkops(struct mmc_card *card)
> +{
> + int err;
> + unsigned long flags;
> +
> + if ((!card || !card->ext_csd.bkops_en) &&
> + !(card->host->caps2 & MMC_CAP2_BKOPS))
> + return;
> +
> + /*
> + * If card is already doing bkops or need for
> + * bkops flag is not set, then do nothing just
> + * return
> + */
> + if (mmc_card_doing_bkops(card)
> + || !mmc_card_need_bkops(card))
> + return;
> +
> + mmc_claim_host(card->host);
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_BKOPS_START, 1, 0);
> + if (err) {
> + pr_warning("error %d starting bkops\n", err);
> + mmc_card_clr_need_bkops(card);
> + goto out;
> + }
> + spin_lock_irqsave(&card->host->lock, flags);
> + mmc_card_clr_need_bkops(card);
> + mmc_card_set_doing_bkops(card);
> + spin_unlock_irqrestore(&card->host->lock, flags);
> +out:
> + mmc_release_host(card->host);
> +}
> +EXPORT_SYMBOL(mmc_start_bkops);
> +
> +
> static void mmc_wait_done(struct mmc_request *mrq)
> {
> complete(&mrq->completion);
> @@ -466,6 +510,39 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
> EXPORT_SYMBOL(mmc_wait_for_cmd);
>
> /**
> + * mmc_interrupt_bkops - interrupt ongoing BKOPS
> + * @card: MMC card to check BKOPS
> + *
> + * Send HPI command to interrupt ongoing background operations,
> + * to allow rapid servicing of foreground operations,e.g. read/
> + * writes. Wait until the card comes out of the programming state
> + * to avoid errors in servicing read/write requests.
> + */
> +int mmc_interrupt_bkops(struct mmc_card *card)
> +{
> + int err;
> + unsigned long flags;
> + u32 status;
> +
> + BUG_ON(!card);
> +
> + /* send HPI to interrupt BKOPS. */
> + err = mmc_send_hpi_cmd(card, &status);
> + if (err || R1_CURRENT_STATE(status) == 7) {
> + do {
> + err = mmc_send_status(card, &status);
> + if (err)
> + return err;
> + } while (R1_CURRENT_STATE(status) == 7);
> + }
> + spin_lock_irqsave(&card->host->lock, flags);
> + mmc_card_clr_doing_bkops(card);
> + spin_unlock_irqrestore(&card->host->lock, flags);
> + return 0;
> +}
> +EXPORT_SYMBOL(mmc_interrupt_bkops);
> +
> +/**
> * mmc_set_data_timeout - set the timeout for a data command
> * @data: data phase for command
> * @card: the MMC card associated with the data transfer
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3627044..b16db45 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -448,6 +448,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
> }
>
> if (card->ext_csd.rev >= 5) {
> + /* check whether the eMMC card support BKOPS */
> + if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)
> + card->ext_csd.bkops = 1;
How about this to include/linux/mmc/mmc.h and using it here?
#define EXT_CSD_BKOPS_SUPPORTED BIT(0)
BTW, this is required starting from eMMC 4.5.
> +
> /* check whether the eMMC card supports HPI */
> if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
> card->ext_csd.hpi = 1;
> @@ -909,6 +913,23 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> }
>
> /*
> + * enable BKOPS feature (if supported)
> + */
> + if (card->ext_csd.bkops) {
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_BKOPS_EN, 1, 0);
The BKOPS_EN property in EXT_CSD is labelled R/W which means that it
is one time programmable. Therefore it makes no sense to write this
every time you are initializing an eMMC. What you could do is to read
BKOPS_EN to determine if BKOPS should, according to the eMMC, be
enabled in the MMC framework.
According to other discussions here it seems to me as if Chris and
others are wary of having sysfs-files enable users to write to
one-time programmable registers. So therefore I guess the only option
is to use the generic MMC command passthrough in mmc_blk_ioctl() to do
such things.
> + if (err && err != -EBADMSG)
> + goto free_card;
> +
> + if (err) {
> + pr_warning("%s: Enabling BKOPS failed\n",
> + mmc_hostname(card->host));
> + err = 0;
> + } else
> + card->ext_csd.bkops_en = 1;
> + }
> +
> + /*
> * Enable HPI feature (if supported)
> */
> if (card->ext_csd.hpi) {
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 007863e..7653e33 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -398,6 +398,10 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> if (err)
> return err;
>
> + /* No need to check card status in case of BKOPS switch*/
> + if (index == EXT_CSD_BKOPS_START)
> + return 0;
Why is this not needed? I can see why you wouldn't want to wait until
the card is no longer busy (as it will stay busy until BKOPS are no
longer needed), but the bits indicating errors must surely be
interesting to check?
> +
> /* Must check status to be sure of no errors */
> do {
> err = mmc_send_status(card, &status);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 6e04e10..f4abb7c 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -71,6 +71,8 @@ struct mmc_ext_csd {
> bool hpi_en; /* HPI enablebit */
> bool hpi; /* HPI support bit */
> unsigned int hpi_cmd; /* cmd used as HPI */
> + bool bkops; /* background support bit */
> + bool bkops_en; /* background enable bit */
> u8 raw_partition_support; /* 160 */
> u8 raw_erased_mem_count; /* 181 */
> u8 raw_ext_csd_structure; /* 194 */
> @@ -206,6 +208,8 @@ struct mmc_card {
> #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode */
> #define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in ultra high speed mode */
> #define MMC_CARD_SDXC (1<<6) /* card is SDXC */
> +#define MMC_STATE_NEED_BKOPS (1<<7) /* card need to do BKOPS */
> +#define MMC_STATE_DOING_BKOPS (1<<8) /* card is doing BKOPS */
> unsigned int quirks; /* card quirks */
> #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */
> #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */
> @@ -365,6 +369,8 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
> #define mmc_card_ddr_mode(c) ((c)->state & MMC_STATE_HIGHSPEED_DDR)
> #define mmc_sd_card_uhs(c) ((c)->state & MMC_STATE_ULTRAHIGHSPEED)
> #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
> +#define mmc_card_need_bkops(c) ((c)->state & MMC_STATE_NEED_BKOPS)
> +#define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
>
> #define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
> #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
> @@ -373,6 +379,11 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
> #define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR)
> #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
> #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
> +#define mmc_card_set_need_bkops(c) ((c)->state |= MMC_STATE_NEED_BKOPS)
> +#define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS)
> +
> +#define mmc_card_clr_need_bkops(c) ((c)->state &= ~MMC_STATE_NEED_BKOPS)
> +#define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS)
>
> /*
> * Quirk add/remove for MMC products.
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 174a844..600a177 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -134,6 +134,7 @@ struct mmc_host;
> struct mmc_card;
> struct mmc_async_req;
>
> +extern int mmc_interrupt_bkops(struct mmc_card *);
> extern struct mmc_async_req *mmc_start_req(struct mmc_host *,
> struct mmc_async_req *, int *);
> extern int mmc_interrupt_hpi(struct mmc_card *);
> @@ -163,6 +164,7 @@ extern int mmc_can_sanitize(struct mmc_card *card);
> extern int mmc_can_secure_erase_trim(struct mmc_card *card);
> extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
> unsigned int nr);
> +extern void mmc_start_bkops(struct mmc_card *card);
> extern unsigned int mmc_calc_max_discard(struct mmc_card *card);
>
> extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index a3ac9c4..692e5f6 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -242,6 +242,7 @@ struct mmc_host {
> #define MMC_CAP2_CACHE_CTRL (1 << 1) /* Allow cache control */
> #define MMC_CAP2_POWEROFF_NOTIFY (1 << 2) /* Notify poweroff supported */
> #define MMC_CAP2_NO_MULTI_READ (1 << 3) /* Multiblock reads don't work */
> +#define MMC_CAP2_BKOPS (1 << 4) /* BKOPS supported */
>
> mmc_pm_flag_t pm_caps; /* supported pm features */
> unsigned int power_notify_type;
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 0e71356..283b848 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -138,6 +138,7 @@ static inline bool mmc_op_multi(u32 opcode)
> #define R1_CURRENT_STATE(x) ((x & 0x00001E00) >> 9) /* sx, b (4 bits) */
> #define R1_READY_FOR_DATA (1 << 8) /* sx, a */
> #define R1_SWITCH_ERROR (1 << 7) /* sx, c */
> +#define R1_URGENT_BKOPS (1 << 6) /* sx, a */
> #define R1_APP_CMD (1 << 5) /* sr, c */
>
> #define R1_STATE_IDLE 0
> @@ -278,6 +279,8 @@ struct _mmc_csd {
> #define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
> #define EXT_CSD_HPI_MGMT 161 /* R/W */
> #define EXT_CSD_RST_N_FUNCTION 162 /* R/W */
> +#define EXT_CSD_BKOPS_EN 163 /* R/W */
> +#define EXT_CSD_BKOPS_START 164 /* W */
> #define EXT_CSD_SANITIZE_START 165 /* W */
> #define EXT_CSD_WR_REL_PARAM 166 /* RO */
> #define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
> @@ -313,6 +316,7 @@ struct _mmc_csd {
> #define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
> #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
> #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
> +#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
> #define EXT_CSD_HPI_FEATURES 503 /* RO */
>
> /*
> --
> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 11:10 [PATCH] mmc: support BKOPS feature for eMMC Jaehoon Chung
2011-10-27 11:37 ` Dong, Chuanxiao
2011-10-27 19:26 ` Sebastian Rasmussen
@ 2011-10-27 19:35 ` Per Forlin
2011-10-27 20:51 ` Sebastian Rasmussen
2011-10-28 5:14 ` Jaehoon Chung
2 siblings, 2 replies; 17+ messages in thread
From: Per Forlin @ 2011-10-27 19:35 UTC (permalink / raw)
To: linux-mmc
Jaehoon Chung <jh80.chung <at> samsung.com> writes:
> +++ b/drivers/mmc/core/core.c
> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct
mmc_request *mrq)
> host->ops->request(host, mrq);
> }
>
> +/**
> + * mmc_start_bkops - start BKOPS for supported cards
> + * @card: MMC card to start BKOPS
> + *
> + * Start background operations whenever requested.
> + * when the urgent BKOPS bit is set in a R1 command response
> + * then background operations should be started immediately.
> +*/
This patch only starts BKOPS if it's urgent or critical. I would be preferable
to run bkops periodically and only when the card is idle to minimize the risk of
reaching URGENT.
The specs says:
-----
Hosts shall still read the full status from the BKOPS_STATUS byte periodically
and start background operations as needed.
-----
I'm thinking of checking BKOPS_STATUS when the card is idle and then run bkops
even if level is only 1 (Operations outstanding – non critical). Would this make
sense?
Regards,
Per
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 11:37 ` Dong, Chuanxiao
2011-10-27 12:56 ` Kyungmin Park
@ 2011-10-27 20:31 ` Sebastian Rasmussen
2011-10-28 3:10 ` Dong, Chuanxiao
1 sibling, 1 reply; 17+ messages in thread
From: Sebastian Rasmussen @ 2011-10-27 20:31 UTC (permalink / raw)
To: Dong, Chuanxiao
Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad
>> @@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
>> set_current_state(TASK_RUNNING);
>> break;
>> }
>> + mmc_start_bkops(mq->card);
>> up(&mq->thread_sem);
>> schedule();
>> down(&mq->thread_sem);
> Maybe put mmc_interrupt_bkops(card) here is better, I think.
And the reason being that one already knows at this point that there
is a new request to be processed and so BKOPS may just as well be
interrupted at this point as later inside mq->issue_fn()?
>> + /* send HPI to interrupt BKOPS. */
>> + err = mmc_send_hpi_cmd(card, &status);
> What will be happened if eMMC card only support BKOPS but not support HPI?
For eMMC 4.41 this is actually a valid, but possibly unlikely, scenario.
For eMMC 4.5 both BKOPS and HPI are mandatory as I read the spec.
As far as I can understand is that card->ext_csd.hpi_cmd == 0 (as
card->ext_csd.hpi == 0 due toe EXT_CSD_HPI_FEATURES being 0). Inside
mmc_send_hpi_cmd() this means that cmd.opcode not going to be either
MMC_STOP_TRANSMISSION or MMC_SEND_STATUS but the command is still
sent. As far as I understand the card will is that case not responde,
but only set it's ILLEGAL_COMMAND bit in the status register and
mmc_send_hpi_cmd() will be stuck in mmc_wait_for_cmd().
Basically you are trying to say that BKOPS should only be enabled if
HPI is also supported and enabled. Unfortunately it may be the case
that someone has enabled BKOPS through the one-time programmable
BKOPS_EN for an eMMC which does not support HPI. If the MMC framework
discovers such a card I suggest bailing out early, because such a card
is obviously misconfigured.
>> + * enable BKOPS feature (if supported)
>> + */
>> + if (card->ext_csd.bkops) {
>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> + EXT_CSD_BKOPS_EN, 1, 0);
> Just found this bit is one time programmable, so that is to say this bit is not suitable for driver to change.
Agreed, my suggestion was to read BKOPS_EN and if BKOPS then use that
to determine if the framework should call mmc_start_bkops().
/ Sebastian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 12:56 ` Kyungmin Park
@ 2011-10-27 20:33 ` Sebastian Rasmussen
0 siblings, 0 replies; 17+ messages in thread
From: Sebastian Rasmussen @ 2011-10-27 20:33 UTC (permalink / raw)
To: Kyungmin Park
Cc: Dong, Chuanxiao, Jaehoon Chung, linux-mmc, Chris Ball, Hanumath Prasad
> > Just found this bit is one time programmable, so that is to
> > say this bit is not suitable for driver to change.
> You mean require the BKOPS_EN_FORCE config if user want to use at kernel?
What do you refer to when you write BKOPS_EN_FORCE config? It doesn't
seem like the OPs patch contains such a config.
/ Sebastian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 19:35 ` Per Forlin
@ 2011-10-27 20:51 ` Sebastian Rasmussen
2011-10-27 21:47 ` Per Forlin
2011-10-28 11:01 ` Jaehoon Chung
2011-10-28 5:14 ` Jaehoon Chung
1 sibling, 2 replies; 17+ messages in thread
From: Sebastian Rasmussen @ 2011-10-27 20:51 UTC (permalink / raw)
To: Per Forlin; +Cc: linux-mmc
Hi!
> This patch only starts BKOPS if it's urgent or critical.
Almost, it starts BKOPS when it is urgent, which per spec means level
2 or 3, i.e. when performance is impacted or when it is critical.
Better use the specs terminology as far as possible to relieve
everyone of confusion.
> I would be preferable to run bkops periodically and only when
> the card is idle to minimize the risk of reaching URGENT.
Well, you kind of need both. If the eMMC is kept busy to such an
extent that the block device is never idling then you would definitely
require this patch, right? Otherwise you may end up wasting the time
between the last command sent and when the device has been deemed to
be idle for long enough.
So basically the OP's patch fixes the case where, e.g. sustained
(re-)writing, keeps the block device busy until and after it reaches
the critical BKOPS level, while your proposal takes care of the other
case where the block device is not accessed all the time.
> The specs says:
> -----
> Hosts shall still read the full status from the BKOPS_STATUS byte periodically
> and start background operations as needed.
> -----
Sure, if there is idle time to do it, then why not.
If there is no idle time and URGENT_BKOPS is asserted, then the
framework can not wait until the next idle period, but should issue
BKOPS as soon as possible after the current command is finished.
> I'm thinking of checking BKOPS_STATUS when the card is idle and then run bkops
> even if level is only 1 (Operations outstanding – non critical). Would this make
> sense?
I guess this boils down to how you define idle..? Does it mean
immediately after the current command has been fully serviced, or does
it mean some arbitrary time after the last sent command has been fully
serviced? My assumption is that you are refering to the latter, in
which case certain access patterns may cause problems. So, how do
_you_ define idle? :)
/ Sebastian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 20:51 ` Sebastian Rasmussen
@ 2011-10-27 21:47 ` Per Forlin
2011-10-27 22:25 ` Sebastian Rasmussen
2011-10-28 11:01 ` Jaehoon Chung
1 sibling, 1 reply; 17+ messages in thread
From: Per Forlin @ 2011-10-27 21:47 UTC (permalink / raw)
To: Sebastian Rasmussen; +Cc: linux-mmc
Hi Sebastian,
On Thu, Oct 27, 2011 at 10:51 PM, Sebastian Rasmussen <sebras@gmail.com> wrote:
> Hi!
>
>> This patch only starts BKOPS if it's urgent or critical.
>
> Almost, it starts BKOPS when it is urgent, which per spec means level
> 2 or 3, i.e. when performance is impacted or when it is critical.
> Better use the specs terminology as far as possible to relieve
> everyone of confusion.
>
>> I would be preferable to run bkops periodically and only when
>> the card is idle to minimize the risk of reaching URGENT.
>
> Well, you kind of need both.
Yes, this is what I mean. If the URGENT_BKOPS is set meaning (status 2
or 3) there is no escape, current solution will do fine.
Periodical check is a complement, not a replacement. Must BKOPS always
be deferred until performance is impacted?
About starvation.
What happens if the BKOPS never have time to finish because new writes
are coming in all the time. Is it possible to starve the
BKOPS-operation?
Will it come to a point when BKOPS must run without interruption?
> extent that the block device is never idling then you would definitely
> require this patch, right? Otherwise you may end up wasting the time
> between the last command sent and when the device has been deemed to
> be idle for long enough.
>
> So basically the OP's patch fixes the case where, e.g. sustained
> (re-)writing, keeps the block device busy until and after it reaches
> the critical BKOPS level, while your proposal takes care of the other
> case where the block device is not accessed all the time.
>
>> The specs says:
>> -----
>> Hosts shall still read the full status from the BKOPS_STATUS byte periodically
>> and start background operations as needed.
>> -----
>
> Sure, if there is idle time to do it, then why not.
> If there is no idle time and URGENT_BKOPS is asserted, then the
> framework can not wait until the next idle period, but should issue
> BKOPS as soon as possible after the current command is finished.
>
>> I'm thinking of checking BKOPS_STATUS when the card is idle and then run bkops
>> even if level is only 1 (Operations outstanding – non critical). Would this make
>> sense?
>
> I guess this boils down to how you define idle..? Does it mean
> immediately after the current command has been fully serviced, or does
> it mean some arbitrary time after the last sent command has been fully
> serviced? My assumption is that you are refering to the latter, in
> which case certain access patterns may cause problems. So, how do
> _you_ define idle? :)
My first point was just to raise my concern and start a discussion
about this. If this turns out to be a good idea I be happy to look
more into the details.
One question is:
Is it worth running BKOPS if the BKOPS_STATUS is only 1? I could run
some tests comparing write performance when status is 0,1,2.
I don't know what the best place would be for a BKOPS_STATUS check.
What comes to my mind is to use the same credentials that are used for
power save. I need to look more closely into this in order to propose
a patch.
Suspend? if suspend is requested one might check BKOPS_STATUS and
return -EBUSY if BKOPS needs to be started.
Regards,
Per
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 21:47 ` Per Forlin
@ 2011-10-27 22:25 ` Sebastian Rasmussen
2011-11-04 15:17 ` Per Forlin
0 siblings, 1 reply; 17+ messages in thread
From: Sebastian Rasmussen @ 2011-10-27 22:25 UTC (permalink / raw)
To: Per Forlin, Jaehoon Chung; +Cc: linux-mmc
Hi!
>> Well, you kind of need both.
> Periodical check is a complement, not a replacement.
Then we are indeed in agreement.
> Must BKOPS always be deferred until performance is impacted?
No, of course not. As you say doing BKOPS too late is not good, but
also doing them too often is probably not wise either (will cause
latency for interrupting BKOPS for too many requests in that case).
> About starvation.
> What happens if the BKOPS never have time to finish because new writes
> are coming in all the time. Is it possible to starve the
> BKOPS-operation?
> Will it come to a point when BKOPS must run without interruption?
The 4.5 spec says that if the level is at critical then some
operations may extend beyond their original timeouts due to
undelayable maintenance operations. So I can not forsee that an eMMC
might stop working because the level reached critical and the host did
not start BKOPS periodically. So as far as I understand it you may HPI
interrupt BKOPS at critical level in order to issue CMD25
(WRITE_MULTIPLE_BLOCK), but there is a good chance that this write
will take a long time to complete.
> One question is:
> Is it worth running BKOPS if the BKOPS_STATUS is only 1? I could run
> some tests comparing write performance when status is 0,1,2.
The spec is likely intentionally arbitrary about what these levels
mean in order to allow vendors to implement those differently.
> I don't know what the best place would be for a BKOPS_STATUS check.
> What comes to my mind is to use the same credentials that are used for
> power save.
That seems like a good option, yes.
> Suspend? if suspend is requested one might check BKOPS_STATUS and
> return -EBUSY if BKOPS needs to be started.
Maybe, one could also choose to do this based on the level of course.
However now that I'm thinking about it, I don't think Jaehoon has
taken care of the case where suspend happens while BKOPS are running
in the background. I guess one has to issue HPI in that case to stop
the BKOPS before going in to suspend, otherwise one risks cutting
power to the card while BKOPS are running.
/ Sebastian
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 20:31 ` Sebastian Rasmussen
@ 2011-10-28 3:10 ` Dong, Chuanxiao
0 siblings, 0 replies; 17+ messages in thread
From: Dong, Chuanxiao @ 2011-10-28 3:10 UTC (permalink / raw)
To: Sebastian Rasmussen
Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad
> -----Original Message-----
> From: Sebastian Rasmussen [mailto:sebras@gmail.com]
> Sent: Friday, October 28, 2011 4:31 AM
> To: Dong, Chuanxiao
> Cc: Jaehoon Chung; linux-mmc; Chris Ball; Kyungmin Park; Hanumath Prasad
> Subject: Re: [PATCH] mmc: support BKOPS feature for eMMC
>
> >> @@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
> >> set_current_state(TASK_RUNNING);
> >> break;
> >> }
> >> + mmc_start_bkops(mq->card);
> >> up(&mq->thread_sem);
> >> schedule();
> >> down(&mq->thread_sem);
> > Maybe put mmc_interrupt_bkops(card) here is better, I think.
>
> And the reason being that one already knows at this point that there
> is a new request to be processed and so BKOPS may just as well be
> interrupted at this point as later inside mq->issue_fn()?
Yes. To me, that is the reason. Since once schedule back here, means block driver will handle mmc request then. No matter what kind of request, that will need to use eMMC card. So I think here is an proper position to interrupt background operations.
>
> >> + /* send HPI to interrupt BKOPS. */
> >> + err = mmc_send_hpi_cmd(card, &status);
> > What will be happened if eMMC card only support BKOPS but not support HPI?
>
> For eMMC 4.41 this is actually a valid, but possibly unlikely, scenario.
> For eMMC 4.5 both BKOPS and HPI are mandatory as I read the spec.
>
> As far as I can understand is that card->ext_csd.hpi_cmd == 0 (as
> card->ext_csd.hpi == 0 due toe EXT_CSD_HPI_FEATURES being 0). Inside
> mmc_send_hpi_cmd() this means that cmd.opcode not going to be either
> MMC_STOP_TRANSMISSION or MMC_SEND_STATUS but the command is still
> sent. As far as I understand the card will is that case not responde,
> but only set it's ILLEGAL_COMMAND bit in the status register and
> mmc_send_hpi_cmd() will be stuck in mmc_wait_for_cmd().
>
> Basically you are trying to say that BKOPS should only be enabled if
> HPI is also supported and enabled. Unfortunately it may be the case
> that someone has enabled BKOPS through the one-time programmable
> BKOPS_EN for an eMMC which does not support HPI. If the MMC framework
> discovers such a card I suggest bailing out early, because such a card
> is obviously misconfigured.
Yes, basically that is what I mean. Seems you know a lot of about me ;).
Maybe if the eMMC card only support BKOPS, then mmc_interrupt_bkops can send CMD13 to poll the card status instead of sending HPI command directly.
Just as we mentioned below, BKOPS_EN is one time programmable, thus this bit should be programmed by eMMC device vendor or some product provider. They can choose whether to enable this from a device performance point of view. If there device have a performance improvement even without a HPI support, then go forward. If not, just leave this bit along.
So what driver does is only to do the right things. If he has HPI, using HPI to stop BKOPS. If not, he has to poll the card status.
Thanks
Chuanxiao
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 19:35 ` Per Forlin
2011-10-27 20:51 ` Sebastian Rasmussen
@ 2011-10-28 5:14 ` Jaehoon Chung
1 sibling, 0 replies; 17+ messages in thread
From: Jaehoon Chung @ 2011-10-28 5:14 UTC (permalink / raw)
To: Per Forlin; +Cc: linux-mmc
On 10/28/2011 04:35 AM, Per Forlin wrote:
> Jaehoon Chung <jh80.chung <at> samsung.com> writes:
>
>> +++ b/drivers/mmc/core/core.c
>> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct
> mmc_request *mrq)
>> host->ops->request(host, mrq);
>> }
>>
>> +/**
>> + * mmc_start_bkops - start BKOPS for supported cards
>> + * @card: MMC card to start BKOPS
>> + *
>> + * Start background operations whenever requested.
>> + * when the urgent BKOPS bit is set in a R1 command response
>> + * then background operations should be started immediately.
>> +*/
> This patch only starts BKOPS if it's urgent or critical. I would be preferable
> to run bkops periodically and only when the card is idle to minimize the risk of
> reaching URGENT.
>
> The specs says:
> -----
> Hosts shall still read the full status from the BKOPS_STATUS byte periodically
> and start background operations as needed.
> -----
>
> I'm thinking of checking BKOPS_STATUS when the card is idle and then run bkops
> even if level is only 1 (Operations outstanding – non critical). Would this make
> sense?
>
host shall check the BKOPS_STATUS periodically..i will add this point..
(i think that need to check EXT_CSD register periodically.)
Thanks,
Jaehoon Chung
> Regards,
> Per
>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 19:26 ` Sebastian Rasmussen
@ 2011-10-28 7:25 ` Jaehoon Chung
0 siblings, 0 replies; 17+ messages in thread
From: Jaehoon Chung @ 2011-10-28 7:25 UTC (permalink / raw)
To: Sebastian Rasmussen
Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad
Hi Sebastian.
On 10/28/2011 04:26 AM, Sebastian Rasmussen wrote:
>> Background operation(BKOPS) is one of eMMC's features.
>>
>> If set the URGENT_BKOPS in response, we can notify that card nedd the BKOPS.
>> And all I/O request is done, then run background operation.
>> If request read/write operation during BKOPS, issue HPI interrupt.
>>
>> If you want to use this feature, should be set MMC_CAP2_BKOPS.
>
> Enable eMMC background operations (BKOPS) feature.
>
> If URGENT_BKOPS is set after a response, note that BKOPS
> are required. After all I/O requests are finished, run
> BKOPS if required. Should read/write operations be requested
> during BKOPS, first issue HPI to interrupt the ongoing BKOPS
> and then service the request.
>
> If you want to enable this feature, set MMC_CAP2_BKOPS.
>
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> CC: Hanumath Prasad <hanumath.prasad@stericsson.com>
>> ---
>> drivers/mmc/card/block.c | 13 +++++++-
>> drivers/mmc/card/queue.c | 1 +
>> drivers/mmc/core/core.c | 77 ++++++++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/core/mmc.c | 21 ++++++++++++
>> drivers/mmc/core/mmc_ops.c | 4 ++
>> include/linux/mmc/card.h | 11 ++++++
>> include/linux/mmc/core.h | 2 +
>> include/linux/mmc/host.h | 1 +
>> include/linux/mmc/mmc.h | 4 ++
>> 9 files changed, 133 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index 4fd5723..edbed7a 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1185,14 +1185,25 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>> type = rq_data_dir(req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
>> mmc_queue_bounce_post(mq_rq);
>>
>> + if (mmc_card_doing_bkops(card)) {
>> + if (mmc_interrupt_bkops(card))
>> + goto cmd_abort;
>> + }
>> +
>> switch (status) {
>> case MMC_BLK_SUCCESS:
>> case MMC_BLK_PARTIAL:
>> + spin_lock_irq(&md->lock);
>> + /*
>> + * Check BKOPS urgency from each R1 response
>> + */
>> + if (mmc_card_mmc(card) &&
>> + (brq->cmd.resp[0] & R1_URGENT_BKOPS))
>> + mmc_card_set_need_bkops(card);
>
> First of all, R1_URGENT_BKOPS is fine to check in cmd->resp[0], but it
> is only valid if cmd->resp contains an R1 or an R1b response. Granted,
> commands that do not have R1/R1b responses are few, but I still
> believe you need to check that with mmc_resp_type().
Ok...i will add to check for mmc_resp_type().
>
> Moreover in the eMMC 4.41 spec it was ok to check bit 6 of the status
> (which is what you do above) as that bit corresponded to URGENT_BKOPS.
> However for eMMC 4.5 this bit has been combined with other kinds of of
> exceptional situations. The bit has been renamed EXCEPTION_EVENT and
> if it is set you then need to read (using CMD8) bytes 55:54 in EXT_CSD
> named EXCEPTION_EVENTS_STATUS where bit 0 indicates URGENT_BKOPS.
Right..eMMC4.5 is modified them. So host need to check EXCEPTION_EVENTS_STATUS and
whenever need to check, host should read the ext_csd register. right?
I will modify also this point..(actually..this patch is based on eMMC4.41)
>
> So I you need to check card->ext_csd.rev here to know what method you
> need to apply to determine if BKOPS are urgent or not.
>
>> /*
>> * A block was successfully transferred.
>> */
>> mmc_blk_reset_success(md, type);
>> - spin_lock_irq(&md->lock);
>> ret = __blk_end_request(req, 0,
>> brq->data.bytes_xfered);
>> spin_unlock_irq(&md->lock);
>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>> index dcad59c..31e5eca 100644
>> --- a/drivers/mmc/card/queue.c
>> +++ b/drivers/mmc/card/queue.c
>> @@ -68,6 +68,7 @@ static int mmc_queue_thread(void *d)
>> set_current_state(TASK_RUNNING);
>> break;
>> }
>> + mmc_start_bkops(mq->card);
>> up(&mq->thread_sem);
>> schedule();
>> down(&mq->thread_sem);
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 5278ffb..7ef270d 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>> host->ops->request(host, mrq);
>> }
>>
>> +/**
>> + * mmc_start_bkops - start BKOPS for supported cards
>> + * @card: MMC card to start BKOPS
>> + *
>> + * Start background operations whenever requested.
>> + * when the urgent BKOPS bit is set in a R1 command response
>> + * then background operations should be started immediately.
>> +*/
>> +void mmc_start_bkops(struct mmc_card *card)
>> +{
>> + int err;
>> + unsigned long flags;
>> +
>> + if ((!card || !card->ext_csd.bkops_en) &&
>> + !(card->host->caps2 & MMC_CAP2_BKOPS))
>> + return;
>> +
>> + /*
>> + * If card is already doing bkops or need for
>> + * bkops flag is not set, then do nothing just
>> + * return
>> + */
>> + if (mmc_card_doing_bkops(card)
>> + || !mmc_card_need_bkops(card))
>> + return;
>> +
>> + mmc_claim_host(card->host);
>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> + EXT_CSD_BKOPS_START, 1, 0);
>> + if (err) {
>> + pr_warning("error %d starting bkops\n", err);
>> + mmc_card_clr_need_bkops(card);
>> + goto out;
>> + }
>> + spin_lock_irqsave(&card->host->lock, flags);
>> + mmc_card_clr_need_bkops(card);
>> + mmc_card_set_doing_bkops(card);
>> + spin_unlock_irqrestore(&card->host->lock, flags);
>> +out:
>> + mmc_release_host(card->host);
>> +}
>> +EXPORT_SYMBOL(mmc_start_bkops);
>> +
>> +
>> static void mmc_wait_done(struct mmc_request *mrq)
>> {
>> complete(&mrq->completion);
>> @@ -466,6 +510,39 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
>> EXPORT_SYMBOL(mmc_wait_for_cmd);
>>
>> /**
>> + * mmc_interrupt_bkops - interrupt ongoing BKOPS
>> + * @card: MMC card to check BKOPS
>> + *
>> + * Send HPI command to interrupt ongoing background operations,
>> + * to allow rapid servicing of foreground operations,e.g. read/
>> + * writes. Wait until the card comes out of the programming state
>> + * to avoid errors in servicing read/write requests.
>> + */
>> +int mmc_interrupt_bkops(struct mmc_card *card)
>> +{
>> + int err;
>> + unsigned long flags;
>> + u32 status;
>> +
>> + BUG_ON(!card);
>> +
>> + /* send HPI to interrupt BKOPS. */
>> + err = mmc_send_hpi_cmd(card, &status);
>> + if (err || R1_CURRENT_STATE(status) == 7) {
>> + do {
>> + err = mmc_send_status(card, &status);
>> + if (err)
>> + return err;
>> + } while (R1_CURRENT_STATE(status) == 7);
>> + }
>> + spin_lock_irqsave(&card->host->lock, flags);
>> + mmc_card_clr_doing_bkops(card);
>> + spin_unlock_irqrestore(&card->host->lock, flags);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>> +
>> +/**
>> * mmc_set_data_timeout - set the timeout for a data command
>> * @data: data phase for command
>> * @card: the MMC card associated with the data transfer
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 3627044..b16db45 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -448,6 +448,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>> }
>>
>> if (card->ext_csd.rev >= 5) {
>> + /* check whether the eMMC card support BKOPS */
>> + if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)
>> + card->ext_csd.bkops = 1;
>
> How about this to include/linux/mmc/mmc.h and using it here?
>
> #define EXT_CSD_BKOPS_SUPPORTED BIT(0)
>
> BTW, this is required starting from eMMC 4.5.
Consider your opinion..:)
>
>> +
>> /* check whether the eMMC card supports HPI */
>> if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>> card->ext_csd.hpi = 1;
>> @@ -909,6 +913,23 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>> }
>>
>> /*
>> + * enable BKOPS feature (if supported)
>> + */
>> + if (card->ext_csd.bkops) {
>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> + EXT_CSD_BKOPS_EN, 1, 0);
>
> The BKOPS_EN property in EXT_CSD is labelled R/W which means that it
> is one time programmable. Therefore it makes no sense to write this
> every time you are initializing an eMMC. What you could do is to read
> BKOPS_EN to determine if BKOPS should, according to the eMMC, be
> enabled in the MMC framework.
Then this code is unnecessary..because card supported bkops, EXT_CSD_BKOPS_EN is set by default.
And if user want to use BKOPS,user can use IOCTL like you mentioned.
>
> According to other discussions here it seems to me as if Chris and
> others are wary of having sysfs-files enable users to write to
> one-time programmable registers. So therefore I guess the only option
> is to use the generic MMC command passthrough in mmc_blk_ioctl() to do
> such things.
>
>> + if (err && err != -EBADMSG)
>> + goto free_card;
>> +
>> + if (err) {
>> + pr_warning("%s: Enabling BKOPS failed\n",
>> + mmc_hostname(card->host));
>> + err = 0;
>> + } else
>> + card->ext_csd.bkops_en = 1;
>> + }
>> +
>> + /*
>> * Enable HPI feature (if supported)
>> */
>> if (card->ext_csd.hpi) {
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 007863e..7653e33 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -398,6 +398,10 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>> if (err)
>> return err;
>>
>> + /* No need to check card status in case of BKOPS switch*/
>> + if (index == EXT_CSD_BKOPS_START)
>> + return 0;
>
> Why is this not needed? I can see why you wouldn't want to wait until
> the card is no longer busy (as it will stay busy until BKOPS are no
> longer needed), but the bits indicating errors must surely be
> interesting to check?
I think that didn't need to check because just write any value in EXT_CSD_BKOPS_START.
>
>> +
>> /* Must check status to be sure of no errors */
>> do {
>> err = mmc_send_status(card, &status);
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index 6e04e10..f4abb7c 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -71,6 +71,8 @@ struct mmc_ext_csd {
>> bool hpi_en; /* HPI enablebit */
>> bool hpi; /* HPI support bit */
>> unsigned int hpi_cmd; /* cmd used as HPI */
>> + bool bkops; /* background support bit */
>> + bool bkops_en; /* background enable bit */
>> u8 raw_partition_support; /* 160 */
>> u8 raw_erased_mem_count; /* 181 */
>> u8 raw_ext_csd_structure; /* 194 */
>> @@ -206,6 +208,8 @@ struct mmc_card {
>> #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed mode */
>> #define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in ultra high speed mode */
>> #define MMC_CARD_SDXC (1<<6) /* card is SDXC */
>> +#define MMC_STATE_NEED_BKOPS (1<<7) /* card need to do BKOPS */
>> +#define MMC_STATE_DOING_BKOPS (1<<8) /* card is doing BKOPS */
>> unsigned int quirks; /* card quirks */
>> #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */
>> #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */
>> @@ -365,6 +369,8 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
>> #define mmc_card_ddr_mode(c) ((c)->state & MMC_STATE_HIGHSPEED_DDR)
>> #define mmc_sd_card_uhs(c) ((c)->state & MMC_STATE_ULTRAHIGHSPEED)
>> #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>> +#define mmc_card_need_bkops(c) ((c)->state & MMC_STATE_NEED_BKOPS)
>> +#define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
>>
>> #define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
>> #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
>> @@ -373,6 +379,11 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
>> #define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR)
>> #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
>> #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
>> +#define mmc_card_set_need_bkops(c) ((c)->state |= MMC_STATE_NEED_BKOPS)
>> +#define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS)
>> +
>> +#define mmc_card_clr_need_bkops(c) ((c)->state &= ~MMC_STATE_NEED_BKOPS)
>> +#define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS)
>>
>> /*
>> * Quirk add/remove for MMC products.
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index 174a844..600a177 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -134,6 +134,7 @@ struct mmc_host;
>> struct mmc_card;
>> struct mmc_async_req;
>>
>> +extern int mmc_interrupt_bkops(struct mmc_card *);
>> extern struct mmc_async_req *mmc_start_req(struct mmc_host *,
>> struct mmc_async_req *, int *);
>> extern int mmc_interrupt_hpi(struct mmc_card *);
>> @@ -163,6 +164,7 @@ extern int mmc_can_sanitize(struct mmc_card *card);
>> extern int mmc_can_secure_erase_trim(struct mmc_card *card);
>> extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
>> unsigned int nr);
>> +extern void mmc_start_bkops(struct mmc_card *card);
>> extern unsigned int mmc_calc_max_discard(struct mmc_card *card);
>>
>> extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index a3ac9c4..692e5f6 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -242,6 +242,7 @@ struct mmc_host {
>> #define MMC_CAP2_CACHE_CTRL (1 << 1) /* Allow cache control */
>> #define MMC_CAP2_POWEROFF_NOTIFY (1 << 2) /* Notify poweroff supported */
>> #define MMC_CAP2_NO_MULTI_READ (1 << 3) /* Multiblock reads don't work */
>> +#define MMC_CAP2_BKOPS (1 << 4) /* BKOPS supported */
>>
>> mmc_pm_flag_t pm_caps; /* supported pm features */
>> unsigned int power_notify_type;
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 0e71356..283b848 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -138,6 +138,7 @@ static inline bool mmc_op_multi(u32 opcode)
>> #define R1_CURRENT_STATE(x) ((x & 0x00001E00) >> 9) /* sx, b (4 bits) */
>> #define R1_READY_FOR_DATA (1 << 8) /* sx, a */
>> #define R1_SWITCH_ERROR (1 << 7) /* sx, c */
>> +#define R1_URGENT_BKOPS (1 << 6) /* sx, a */
>> #define R1_APP_CMD (1 << 5) /* sr, c */
>>
>> #define R1_STATE_IDLE 0
>> @@ -278,6 +279,8 @@ struct _mmc_csd {
>> #define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
>> #define EXT_CSD_HPI_MGMT 161 /* R/W */
>> #define EXT_CSD_RST_N_FUNCTION 162 /* R/W */
>> +#define EXT_CSD_BKOPS_EN 163 /* R/W */
>> +#define EXT_CSD_BKOPS_START 164 /* W */
>> #define EXT_CSD_SANITIZE_START 165 /* W */
>> #define EXT_CSD_WR_REL_PARAM 166 /* RO */
>> #define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
>> @@ -313,6 +316,7 @@ struct _mmc_csd {
>> #define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
>> #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
>> #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
>> +#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
>> #define EXT_CSD_HPI_FEATURES 503 /* RO */
>>
>> /*
>> --
>> 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
>>
> --
> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 20:51 ` Sebastian Rasmussen
2011-10-27 21:47 ` Per Forlin
@ 2011-10-28 11:01 ` Jaehoon Chung
2011-10-28 14:42 ` Sebastian Rasmussen
1 sibling, 1 reply; 17+ messages in thread
From: Jaehoon Chung @ 2011-10-28 11:01 UTC (permalink / raw)
To: Sebastian Rasmussen; +Cc: Per Forlin, linux-mmc
Hi Sebastian.
I have some question..
Maybe i think that my-patch should be worked only level-2/3..right?
Because URGENT_BKOPS bit in the EXCEPTION_EVENTS_STATUS is set whenever is either 2 or 3.
In this case, host control with R1-type response.
But in level-0/1 case, need to check BKOPS_STATUS periodically.
How did you understand that "periodically" means?
If host need to get BKOPS_STATUS periodically, maybe send CMD8 periodically.
How periodically?
Best Regards,
Jaehoon Chung
On 10/28/2011 05:51 AM, Sebastian Rasmussen wrote:
> Hi!
>
>> This patch only starts BKOPS if it's urgent or critical.
>
> Almost, it starts BKOPS when it is urgent, which per spec means level
> 2 or 3, i.e. when performance is impacted or when it is critical.
> Better use the specs terminology as far as possible to relieve
> everyone of confusion.
>
>> I would be preferable to run bkops periodically and only when
>> the card is idle to minimize the risk of reaching URGENT.
>
> Well, you kind of need both. If the eMMC is kept busy to such an
> extent that the block device is never idling then you would definitely
> require this patch, right? Otherwise you may end up wasting the time
> between the last command sent and when the device has been deemed to
> be idle for long enough.
>
> So basically the OP's patch fixes the case where, e.g. sustained
> (re-)writing, keeps the block device busy until and after it reaches
> the critical BKOPS level, while your proposal takes care of the other
> case where the block device is not accessed all the time.
>
>> The specs says:
>> -----
>> Hosts shall still read the full status from the BKOPS_STATUS byte periodically
>> and start background operations as needed.
>> -----
>
> Sure, if there is idle time to do it, then why not.
> If there is no idle time and URGENT_BKOPS is asserted, then the
> framework can not wait until the next idle period, but should issue
> BKOPS as soon as possible after the current command is finished.
>
>> I'm thinking of checking BKOPS_STATUS when the card is idle and then run bkops
>> even if level is only 1 (Operations outstanding – non critical). Would this make
>> sense?
>
> I guess this boils down to how you define idle..? Does it mean
> immediately after the current command has been fully serviced, or does
> it mean some arbitrary time after the last sent command has been fully
> serviced? My assumption is that you are refering to the latter, in
> which case certain access patterns may cause problems. So, how do
> _you_ define idle? :)
>
> / Sebastian
> --
> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-28 11:01 ` Jaehoon Chung
@ 2011-10-28 14:42 ` Sebastian Rasmussen
2011-10-28 17:13 ` S, Venkatraman
0 siblings, 1 reply; 17+ messages in thread
From: Sebastian Rasmussen @ 2011-10-28 14:42 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Per Forlin, linux-mmc
> Maybe i think that my-patch should be worked only level-2/3..right?
> Because URGENT_BKOPS bit in the EXCEPTION_EVENTS_STATUS is set whenever is either 2 or 3.
> In this case, host control with R1-type response.
For 4.41 your patch is fine. For 4.5 I think it starts BKOPS too often.
For 4.5 devices the R1 response indicates that there is an exception
event which may be have several different causes: PACKED_FAILURE,
SYSPOOL_EXHAUSTED, DYNCAP_NEEDED or URGENT_BKOPS. But if the R1
response indicates an exception event it may be becuase of a cause
which is not URGENT_BKOPS. So for 4.5 you must read
EXCEPTION_EVENTS_STATUS to determine what the cause was.
> But in level-0/1 case, need to check BKOPS_STATUS periodically.
> How did you understand that "periodically" means?
> If host need to get BKOPS_STATUS periodically, maybe send CMD8 periodically.
> How periodically?
Good question, I don't know yet. Problem this should be done when the
system is otherwise idling, to not interfere with normal accesses (if
you do I would expect you to get worse performance).
/ Sebastian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-28 14:42 ` Sebastian Rasmussen
@ 2011-10-28 17:13 ` S, Venkatraman
0 siblings, 0 replies; 17+ messages in thread
From: S, Venkatraman @ 2011-10-28 17:13 UTC (permalink / raw)
To: Sebastian Rasmussen; +Cc: Jaehoon Chung, Per Forlin, linux-mmc
On Fri, Oct 28, 2011 at 8:12 PM, Sebastian Rasmussen <sebras@gmail.com> wrote:
>> Maybe i think that my-patch should be worked only level-2/3..right?
>> Because URGENT_BKOPS bit in the EXCEPTION_EVENTS_STATUS is set whenever is either 2 or 3.
>> In this case, host control with R1-type response.
>
> For 4.41 your patch is fine. For 4.5 I think it starts BKOPS too often.
>
> For 4.5 devices the R1 response indicates that there is an exception
> event which may be have several different causes: PACKED_FAILURE,
> SYSPOOL_EXHAUSTED, DYNCAP_NEEDED or URGENT_BKOPS. But if the R1
> response indicates an exception event it may be becuase of a cause
> which is not URGENT_BKOPS. So for 4.5 you must read
> EXCEPTION_EVENTS_STATUS to determine what the cause was.
>
>> But in level-0/1 case, need to check BKOPS_STATUS periodically.
>> How did you understand that "periodically" means?
>> If host need to get BKOPS_STATUS periodically, maybe send CMD8 periodically.
>> How periodically?
>
> Good question, I don't know yet. Problem this should be done when the
> system is otherwise idling, to not interfere with normal accesses (if
> you do I would expect you to get worse performance).
>
I think there is a need for a 'mmc_maintenance' timer of some sort, which should
run while idle. When it expires, all pending maintenance operations can be
executed.. (Sanitize, Cache flush, BKOPS and RealTime Clock). This
aligns also with
the update frequency requirements for RTC.
This would reduce the need for running such plumbing operations on the hot path.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: support BKOPS feature for eMMC
2011-10-27 22:25 ` Sebastian Rasmussen
@ 2011-11-04 15:17 ` Per Forlin
0 siblings, 0 replies; 17+ messages in thread
From: Per Forlin @ 2011-11-04 15:17 UTC (permalink / raw)
To: Sebastian Rasmussen; +Cc: Jaehoon Chung, linux-mmc
On Fri, Oct 28, 2011 at 12:25 AM, Sebastian Rasmussen <sebras@gmail.com> wrote:
> Hi!
>
>>> Well, you kind of need both.
>> Periodical check is a complement, not a replacement.
>
> Then we are indeed in agreement.
>
>> Must BKOPS always be deferred until performance is impacted?
>
> No, of course not. As you say doing BKOPS too late is not good, but
> also doing them too often is probably not wise either (will cause
> latency for interrupting BKOPS for too many requests in that case).
>
>> About starvation.
>> What happens if the BKOPS never have time to finish because new writes
>> are coming in all the time. Is it possible to starve the
>> BKOPS-operation?
>> Will it come to a point when BKOPS must run without interruption?
>
> The 4.5 spec says that if the level is at critical then some
> operations may extend beyond their original timeouts due to
> undelayable maintenance operations. So I can not forsee that an eMMC
> might stop working because the level reached critical and the host did
> not start BKOPS periodically. So as far as I understand it you may HPI
> interrupt BKOPS at critical level in order to issue CMD25
> (WRITE_MULTIPLE_BLOCK), but there is a good chance that this write
> will take a long time to complete.
>
>> One question is:
>> Is it worth running BKOPS if the BKOPS_STATUS is only 1? I could run
>> some tests comparing write performance when status is 0,1,2.
>
> The spec is likely intentionally arbitrary about what these levels
> mean in order to allow vendors to implement those differently.
>
>> I don't know what the best place would be for a BKOPS_STATUS check.
>> What comes to my mind is to use the same credentials that are used for
>> power save.
>
> That seems like a good option, yes.
>
>> Suspend? if suspend is requested one might check BKOPS_STATUS and
>> return -EBUSY if BKOPS needs to be started.
>
> Maybe, one could also choose to do this based on the level of course.
>
> However now that I'm thinking about it, I don't think Jaehoon has
> taken care of the case where suspend happens while BKOPS are running
> in the background. I guess one has to issue HPI in that case to stop
> the BKOPS before going in to suspend, otherwise one risks cutting
> power to the card while BKOPS are running.
Sebastian,
Would you recommend to _not_ cut power while BKOPS? Is this documented anywhere?
Or is it so that the BKOPS will resume when the card is powered up again?
Thanks,
Per
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-11-04 15:17 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-27 11:10 [PATCH] mmc: support BKOPS feature for eMMC Jaehoon Chung
2011-10-27 11:37 ` Dong, Chuanxiao
2011-10-27 12:56 ` Kyungmin Park
2011-10-27 20:33 ` Sebastian Rasmussen
2011-10-27 20:31 ` Sebastian Rasmussen
2011-10-28 3:10 ` Dong, Chuanxiao
2011-10-27 19:26 ` Sebastian Rasmussen
2011-10-28 7:25 ` Jaehoon Chung
2011-10-27 19:35 ` Per Forlin
2011-10-27 20:51 ` Sebastian Rasmussen
2011-10-27 21:47 ` Per Forlin
2011-10-27 22:25 ` Sebastian Rasmussen
2011-11-04 15:17 ` Per Forlin
2011-10-28 11:01 ` Jaehoon Chung
2011-10-28 14:42 ` Sebastian Rasmussen
2011-10-28 17:13 ` S, Venkatraman
2011-10-28 5:14 ` Jaehoon Chung
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.