All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mmc: support BKOPS feature for eMMC
@ 2011-11-02 10:28 Jaehoon Chung
  2011-11-04 18:06 ` Per Forlin
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Jaehoon Chung @ 2011-11-02 10:28 UTC (permalink / raw)
  To: linux-mmc
  Cc: Chris Ball, Kyungmin Park, Hanumath Prasad, Sebastian Rasmussen,
	Per Forlin, svenkatr, chuanxiao.dong

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>

---
Changelog V2:
	- Use EXCEPTION_STATUS instead of URGENT_BKOPS
	- Add function to check Exception_status(for eMMC4.5)
	- remove unnecessary code.

 drivers/mmc/card/block.c   |    9 +++++
 drivers/mmc/card/queue.c   |    4 ++
 drivers/mmc/core/core.c    |   87 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/core/mmc.c     |    9 ++++-
 drivers/mmc/core/mmc_ops.c |    4 ++
 include/linux/mmc/card.h   |   12 ++++++
 include/linux/mmc/core.h   |    3 ++
 include/linux/mmc/host.h   |    1 +
 include/linux/mmc/mmc.h    |   14 +++++++
 9 files changed, 142 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index a1cb21f..fbfb405 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 		case MMC_BLK_SUCCESS:
 		case MMC_BLK_PARTIAL:
 			/*
+			 * Check BKOPS urgency from each R1 response
+			 */
+			if (mmc_card_mmc(card) &&
+				(brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
+				if (mmc_is_exception_event(card,
+						EXT_CSD_URGENT_BKOPS))
+					mmc_card_set_need_bkops(card);
+			}
+			/*
 			 * A block was successfully transferred.
 			 */
 			mmc_blk_reset_success(md, type);
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index dcad59c..20bb4a5 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
 		spin_unlock_irq(q->queue_lock);
 
 		if (req || mq->mqrq_prev->req) {
+			if (mmc_card_doing_bkops(mq->card))
+				mmc_interrupt_bkops(mq->card);
+
 			set_current_state(TASK_RUNNING);
 			mq->issue_fn(mq, req);
 		} else {
@@ -68,6 +71,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..41ea923 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;
+
+	card->host->caps2 |= MMC_CAP2_BKOPS;
+	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,49 @@ 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 = 0;
+	unsigned long flags;
+
+	BUG_ON(!card);
+
+	err = mmc_interrupt_hpi(card);
+
+	spin_lock_irqsave(&card->host->lock, flags);
+	mmc_card_clr_doing_bkops(card);
+	spin_unlock_irqrestore(&card->host->lock, flags);
+
+	return err;
+}
+EXPORT_SYMBOL(mmc_interrupt_bkops);
+
+int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
+{
+	int err;
+	u8 ext_csd[512];
+
+	/* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
+	if (card->ext_csd.rev == 5)
+		return 1;
+
+	err = mmc_send_ext_csd(card, ext_csd);
+	if (err)
+		return err;
+
+	return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
+}
+EXPORT_SYMBOL(mmc_is_exception_event);
+
+/**
  *	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..af40fbb 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -448,6 +448,14 @@ 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;
+			card->ext_csd.bkops_en = 1;
+			card->ext_csd.raw_bkops_status =
+				ext_csd[EXT_CSD_BKOPS_STATUS];
+		}
+
 		/* check whether the eMMC card supports HPI */
 		if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
 			card->ext_csd.hpi = 1;
@@ -498,7 +506,6 @@ static inline void mmc_free_ext_csd(u8 *ext_csd)
 	kfree(ext_csd);
 }
 
-
 static int mmc_compare_ext_csds(struct mmc_card *card, unsigned bus_width)
 {
 	u8 *bw_ext_csd;
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 415f2db..34740c5 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 */
@@ -84,6 +86,7 @@ struct mmc_ext_csd {
 	u8			raw_sec_erase_mult;	/* 230 */
 	u8			raw_sec_feature_support;/* 231 */
 	u8			raw_trim_mult;		/* 232 */
+	u8			raw_bkops_status;	/* 246 */
 	u8			raw_sectors[4];		/* 212 - 4 bytes */
 
 	unsigned int            feature_support;
@@ -206,6 +209,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 +370,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 +380,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..f6b997a 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -134,6 +134,8 @@ struct mmc_host;
 struct mmc_card;
 struct mmc_async_req;
 
+extern int mmc_interrupt_bkops(struct mmc_card *);
+extern int mmc_is_exception_event(struct mmc_card *, unsigned int);
 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 +165,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..20b1420 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_EXCEPTION_EVENT	(1 << 6)	/* sx, a */
 #define R1_APP_CMD		(1 << 5)	/* sr, c */
 
 #define R1_STATE_IDLE	0
@@ -273,11 +274,14 @@ struct _mmc_csd {
 #define EXT_CSD_FLUSH_CACHE		32      /* W */
 #define EXT_CSD_CACHE_CTRL		33      /* R/W */
 #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */
+#define EXT_CSD_EXCEPTION_STATUS	54	/* RO */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #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 */
@@ -310,9 +314,11 @@ struct _mmc_csd {
 #define EXT_CSD_PWR_CL_200_360		237	/* RO */
 #define EXT_CSD_PWR_CL_DDR_52_195	238	/* RO */
 #define EXT_CSD_PWR_CL_DDR_52_360	239	/* RO */
+#define EXT_CSD_BKOPS_STATUS		246	/* RO */
 #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 */
 
 /*
@@ -373,4 +379,12 @@ struct _mmc_csd {
 #define MMC_SWITCH_MODE_CLEAR_BITS	0x02	/* Clear bits which are 1 in value */
 #define MMC_SWITCH_MODE_WRITE_BYTE	0x03	/* Set target to value */
 
+/*
+ * EXCEPTION_EVENT_STATUS field (eMMC4.5)
+ */
+#define EXT_CSD_URGENT_BKOPS		BIT(0)
+#define EXT_CSD_DYNCAP_NEEDED		BIT(1)
+#define EXT_CSD_SYSPOOL_EXHAUSTED	BIT(2)
+#define EXT_CSD_PACKED_FAILURE		BIT(3)
+
 #endif /* LINUX_MMC_MMC_H */

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-02 10:28 [PATCH v2] mmc: support BKOPS feature for eMMC Jaehoon Chung
@ 2011-11-04 18:06 ` Per Forlin
  2011-11-08 12:43   ` Jae hoon Chung
  2011-11-10 22:01 ` Per Forlin
  2011-11-11  6:51 ` Dong, Chuanxiao
  2 siblings, 1 reply; 20+ messages in thread
From: Per Forlin @ 2011-11-04 18:06 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad,
	Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Jaehoon,

On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 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>
>
> ---
> Changelog V2:
>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>        - Add function to check Exception_status(for eMMC4.5)
>        - remove unnecessary code.
>
>  drivers/mmc/card/block.c   |    9 +++++
>  drivers/mmc/card/queue.c   |    4 ++
>  drivers/mmc/core/core.c    |   87 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/core/mmc.c     |    9 ++++-
>  drivers/mmc/core/mmc_ops.c |    4 ++
>  include/linux/mmc/card.h   |   12 ++++++
>  include/linux/mmc/core.h   |    3 ++
>  include/linux/mmc/host.h   |    1 +
>  include/linux/mmc/mmc.h    |   14 +++++++
>  9 files changed, 142 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index a1cb21f..fbfb405 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>                case MMC_BLK_SUCCESS:
>                case MMC_BLK_PARTIAL:
>                        /*
> +                        * Check BKOPS urgency from each R1 response
> +                        */
> +                       if (mmc_card_mmc(card) &&
> +                               (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
> +                               if (mmc_is_exception_event(card,
> +                                               EXT_CSD_URGENT_BKOPS))
> +                                       mmc_card_set_need_bkops(card);
> +                       }
> +                       /*
>                         * A block was successfully transferred.
>                         */
>                        mmc_blk_reset_success(md, type);
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index dcad59c..20bb4a5 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
>                spin_unlock_irq(q->queue_lock);
>
>                if (req || mq->mqrq_prev->req) {
> +                       if (mmc_card_doing_bkops(mq->card))
> +                               mmc_interrupt_bkops(mq->card);
> +
>                        set_current_state(TASK_RUNNING);
>                        mq->issue_fn(mq, req);
>                } else {
> @@ -68,6 +71,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..41ea923 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;
> +

> +       card->host->caps2 |= MMC_CAP2_BKOPS;
I guess this should be set by the host driver?

> +       if ((!card || !card->ext_csd.bkops_en) &&
> +                       !(card->host->caps2 & MMC_CAP2_BKOPS))
I don't really understand this. If card is NULL it will seg-fault.

Do you mean:
if (!card)
  return

if (!card->ext_csd.bkops_en ||
   !(card->host->caps2 & MMC_CAP2_BKOPS))
    return

Do you really need to check card != NULL? Consider BUG_ON

> +               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,49 @@ 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 = 0;
> +       unsigned long flags;
> +
> +       BUG_ON(!card);
> +
> +       err = mmc_interrupt_hpi(card);
> +
> +       spin_lock_irqsave(&card->host->lock, flags);
> +       mmc_card_clr_doing_bkops(card);
> +       spin_unlock_irqrestore(&card->host->lock, flags);
> +
> +       return err;
> +}
> +EXPORT_SYMBOL(mmc_interrupt_bkops);
> +
> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
> +{
> +       int err;
> +       u8 ext_csd[512];
> +
> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
> +       if (card->ext_csd.rev == 5)
> +               return 1;
> +
> +       err = mmc_send_ext_csd(card, ext_csd);
> +       if (err)
> +               return err;
> +
> +       return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
> +}
> +EXPORT_SYMBOL(mmc_is_exception_event);
> +
> +/**
>  *     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..af40fbb 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -448,6 +448,14 @@ 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;
> +                       card->ext_csd.bkops_en = 1;
What if the card has not enabled bkops? This bit could be unset.
card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];

> +                       card->ext_csd.raw_bkops_status =
> +                               ext_csd[EXT_CSD_BKOPS_STATUS];
> +               }
> +
>                /* check whether the eMMC card supports HPI */
>                if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>                        card->ext_csd.hpi = 1;
> @@ -498,7 +506,6 @@ static inline void mmc_free_ext_csd(u8 *ext_csd)
>        kfree(ext_csd);
>  }
>
> -
>  static int mmc_compare_ext_csds(struct mmc_card *card, unsigned bus_width)
>  {
>        u8 *bw_ext_csd;
> 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 415f2db..34740c5 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 */
> @@ -84,6 +86,7 @@ struct mmc_ext_csd {
>        u8                      raw_sec_erase_mult;     /* 230 */
>        u8                      raw_sec_feature_support;/* 231 */
>        u8                      raw_trim_mult;          /* 232 */
> +       u8                      raw_bkops_status;       /* 246 */
>        u8                      raw_sectors[4];         /* 212 - 4 bytes */
>
>        unsigned int            feature_support;
> @@ -206,6 +209,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 +370,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 +380,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..f6b997a 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -134,6 +134,8 @@ struct mmc_host;
>  struct mmc_card;
>  struct mmc_async_req;
>
> +extern int mmc_interrupt_bkops(struct mmc_card *);
> +extern int mmc_is_exception_event(struct mmc_card *, unsigned int);
>  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 +165,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..20b1420 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_EXCEPTION_EVENT     (1 << 6)        /* sx, a */
>  #define R1_APP_CMD             (1 << 5)        /* sr, c */
>
>  #define R1_STATE_IDLE  0
> @@ -273,11 +274,14 @@ struct _mmc_csd {
>  #define EXT_CSD_FLUSH_CACHE            32      /* W */
>  #define EXT_CSD_CACHE_CTRL             33      /* R/W */
>  #define EXT_CSD_POWER_OFF_NOTIFICATION 34      /* R/W */
> +#define EXT_CSD_EXCEPTION_STATUS       54      /* RO */
>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>  #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 */
> @@ -310,9 +314,11 @@ struct _mmc_csd {
>  #define EXT_CSD_PWR_CL_200_360         237     /* RO */
>  #define EXT_CSD_PWR_CL_DDR_52_195      238     /* RO */
>  #define EXT_CSD_PWR_CL_DDR_52_360      239     /* RO */
> +#define EXT_CSD_BKOPS_STATUS           246     /* RO */
>  #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 */
>
>  /*
> @@ -373,4 +379,12 @@ struct _mmc_csd {
>  #define MMC_SWITCH_MODE_CLEAR_BITS     0x02    /* Clear bits which are 1 in value */
>  #define MMC_SWITCH_MODE_WRITE_BYTE     0x03    /* Set target to value */
>
> +/*
> + * EXCEPTION_EVENT_STATUS field (eMMC4.5)
> + */
> +#define EXT_CSD_URGENT_BKOPS           BIT(0)
> +#define EXT_CSD_DYNCAP_NEEDED          BIT(1)
> +#define EXT_CSD_SYSPOOL_EXHAUSTED      BIT(2)
> +#define EXT_CSD_PACKED_FAILURE         BIT(3)
> +
>  #endif /* LINUX_MMC_MMC_H */
> --
> 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
>

Regards,
Per

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-04 18:06 ` Per Forlin
@ 2011-11-08 12:43   ` Jae hoon Chung
  2011-11-08 13:53     ` Per Forlin
  0 siblings, 1 reply; 20+ messages in thread
From: Jae hoon Chung @ 2011-11-08 12:43 UTC (permalink / raw)
  To: Per Forlin
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Per.

2011/11/5 Per Forlin <per.lkml@gmail.com>:
> Hi Jaehoon,
>
> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> 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>
>>
>> ---
>> Changelog V2:
>>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>        - Add function to check Exception_status(for eMMC4.5)
>>        - remove unnecessary code.
>>
>>  drivers/mmc/card/block.c   |    9 +++++
>>  drivers/mmc/card/queue.c   |    4 ++
>>  drivers/mmc/core/core.c    |   87 ++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>  include/linux/mmc/card.h   |   12 ++++++
>>  include/linux/mmc/core.h   |    3 ++
>>  include/linux/mmc/host.h   |    1 +
>>  include/linux/mmc/mmc.h    |   14 +++++++
>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index a1cb21f..fbfb405 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>>                case MMC_BLK_SUCCESS:
>>                case MMC_BLK_PARTIAL:
>>                        /*
>> +                        * Check BKOPS urgency from each R1 response
>> +                        */
>> +                       if (mmc_card_mmc(card) &&
>> +                               (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>> +                               if (mmc_is_exception_event(card,
>> +                                               EXT_CSD_URGENT_BKOPS))
>> +                                       mmc_card_set_need_bkops(card);
>> +                       }
>> +                       /*
>>                         * A block was successfully transferred.
>>                         */
>>                        mmc_blk_reset_success(md, type);
>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>> index dcad59c..20bb4a5 100644
>> --- a/drivers/mmc/card/queue.c
>> +++ b/drivers/mmc/card/queue.c
>> @@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
>>                spin_unlock_irq(q->queue_lock);
>>
>>                if (req || mq->mqrq_prev->req) {
>> +                       if (mmc_card_doing_bkops(mq->card))
>> +                               mmc_interrupt_bkops(mq->card);
>> +
>>                        set_current_state(TASK_RUNNING);
>>                        mq->issue_fn(mq, req);
>>                } else {
>> @@ -68,6 +71,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..41ea923 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;
>> +
>
>> +       card->host->caps2 |= MMC_CAP2_BKOPS;
> I guess this should be set by the host driver?
You're right...it's my mistake...i will remove this..
>
>> +       if ((!card || !card->ext_csd.bkops_en) &&
>> +                       !(card->host->caps2 & MMC_CAP2_BKOPS))
> I don't really understand this. If card is NULL it will seg-fault.
>
> Do you mean:
> if (!card)
>  return
>
> if (!card->ext_csd.bkops_en ||
>   !(card->host->caps2 & MMC_CAP2_BKOPS))
>    return
>
> Do you really need to check card != NULL? Consider BUG_ON
Maybe if (!card && !card->ext_csd.bkops_en) is correctly...i think...
>
>> +               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,49 @@ 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 = 0;
>> +       unsigned long flags;
>> +
>> +       BUG_ON(!card);
>> +
>> +       err = mmc_interrupt_hpi(card);
>> +
>> +       spin_lock_irqsave(&card->host->lock, flags);
>> +       mmc_card_clr_doing_bkops(card);
>> +       spin_unlock_irqrestore(&card->host->lock, flags);
>> +
>> +       return err;
>> +}
>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>> +
>> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
>> +{
>> +       int err;
>> +       u8 ext_csd[512];
>> +
>> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
>> +       if (card->ext_csd.rev == 5)
>> +               return 1;
>> +
>> +       err = mmc_send_ext_csd(card, ext_csd);
>> +       if (err)
>> +               return err;
>> +
>> +       return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
>> +}
>> +EXPORT_SYMBOL(mmc_is_exception_event);
>> +
>> +/**
>>  *     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..af40fbb 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -448,6 +448,14 @@ 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;
>> +                       card->ext_csd.bkops_en = 1;
> What if the card has not enabled bkops? This bit could be unset.
> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
When card is supported the bkops, bkops_en bit is set by default...

>
>> +                       card->ext_csd.raw_bkops_status =
>> +                               ext_csd[EXT_CSD_BKOPS_STATUS];
>> +               }
>> +
>>                /* check whether the eMMC card supports HPI */
>>                if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>>                        card->ext_csd.hpi = 1;
>> @@ -498,7 +506,6 @@ static inline void mmc_free_ext_csd(u8 *ext_csd)
>>        kfree(ext_csd);
>>  }
>>
>> -
>>  static int mmc_compare_ext_csds(struct mmc_card *card, unsigned bus_width)
>>  {
>>        u8 *bw_ext_csd;
>> 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 415f2db..34740c5 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 */
>> @@ -84,6 +86,7 @@ struct mmc_ext_csd {
>>        u8                      raw_sec_erase_mult;     /* 230 */
>>        u8                      raw_sec_feature_support;/* 231 */
>>        u8                      raw_trim_mult;          /* 232 */
>> +       u8                      raw_bkops_status;       /* 246 */
>>        u8                      raw_sectors[4];         /* 212 - 4 bytes */
>>
>>        unsigned int            feature_support;
>> @@ -206,6 +209,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 +370,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 +380,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..f6b997a 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -134,6 +134,8 @@ struct mmc_host;
>>  struct mmc_card;
>>  struct mmc_async_req;
>>
>> +extern int mmc_interrupt_bkops(struct mmc_card *);
>> +extern int mmc_is_exception_event(struct mmc_card *, unsigned int);
>>  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 +165,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..20b1420 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_EXCEPTION_EVENT     (1 << 6)        /* sx, a */
>>  #define R1_APP_CMD             (1 << 5)        /* sr, c */
>>
>>  #define R1_STATE_IDLE  0
>> @@ -273,11 +274,14 @@ struct _mmc_csd {
>>  #define EXT_CSD_FLUSH_CACHE            32      /* W */
>>  #define EXT_CSD_CACHE_CTRL             33      /* R/W */
>>  #define EXT_CSD_POWER_OFF_NOTIFICATION 34      /* R/W */
>> +#define EXT_CSD_EXCEPTION_STATUS       54      /* RO */
>>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
>>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>>  #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 */
>> @@ -310,9 +314,11 @@ struct _mmc_csd {
>>  #define EXT_CSD_PWR_CL_200_360         237     /* RO */
>>  #define EXT_CSD_PWR_CL_DDR_52_195      238     /* RO */
>>  #define EXT_CSD_PWR_CL_DDR_52_360      239     /* RO */
>> +#define EXT_CSD_BKOPS_STATUS           246     /* RO */
>>  #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 */
>>
>>  /*
>> @@ -373,4 +379,12 @@ struct _mmc_csd {
>>  #define MMC_SWITCH_MODE_CLEAR_BITS     0x02    /* Clear bits which are 1 in value */
>>  #define MMC_SWITCH_MODE_WRITE_BYTE     0x03    /* Set target to value */
>>
>> +/*
>> + * EXCEPTION_EVENT_STATUS field (eMMC4.5)
>> + */
>> +#define EXT_CSD_URGENT_BKOPS           BIT(0)
>> +#define EXT_CSD_DYNCAP_NEEDED          BIT(1)
>> +#define EXT_CSD_SYSPOOL_EXHAUSTED      BIT(2)
>> +#define EXT_CSD_PACKED_FAILURE         BIT(3)
>> +
>>  #endif /* LINUX_MMC_MMC_H */
>> --
>> 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
>>
>
> 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] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-08 12:43   ` Jae hoon Chung
@ 2011-11-08 13:53     ` Per Forlin
  2011-11-08 18:37       ` Per Forlin
  2011-11-11  6:24       ` Jae hoon Chung
  0 siblings, 2 replies; 20+ messages in thread
From: Per Forlin @ 2011-11-08 13:53 UTC (permalink / raw)
  To: Jae hoon Chung
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Jaehoon,

On Tue, Nov 8, 2011 at 1:43 PM, Jae hoon Chung <jh80.chung@gmail.com> wrote:
> Hi Per.
>
> 2011/11/5 Per Forlin <per.lkml@gmail.com>:
>> Hi Jaehoon,
>>
>> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> 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>
>>>
>>> ---
>>> Changelog V2:
>>>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>>        - Add function to check Exception_status(for eMMC4.5)
>>>        - remove unnecessary code.
>>>
>>>  drivers/mmc/card/block.c   |    9 +++++
>>>  drivers/mmc/card/queue.c   |    4 ++
>>>  drivers/mmc/core/core.c    |   87 ++++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>>  include/linux/mmc/card.h   |   12 ++++++
>>>  include/linux/mmc/core.h   |    3 ++
>>>  include/linux/mmc/host.h   |    1 +
>>>  include/linux/mmc/mmc.h    |   14 +++++++
>>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index a1cb21f..fbfb405 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>>>                case MMC_BLK_SUCCESS:
>>>                case MMC_BLK_PARTIAL:
>>>                        /*
>>> +                        * Check BKOPS urgency from each R1 response
>>> +                        */
>>> +                       if (mmc_card_mmc(card) &&
>>> +                               (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>>> +                               if (mmc_is_exception_event(card,
>>> +                                               EXT_CSD_URGENT_BKOPS))
>>> +                                       mmc_card_set_need_bkops(card);
>>> +                       }
>>> +                       /*
>>>                         * A block was successfully transferred.
>>>                         */
>>>                        mmc_blk_reset_success(md, type);
>>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>>> index dcad59c..20bb4a5 100644
>>> --- a/drivers/mmc/card/queue.c
>>> +++ b/drivers/mmc/card/queue.c
>>> @@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
>>>                spin_unlock_irq(q->queue_lock);
>>>
>>>                if (req || mq->mqrq_prev->req) {
>>> +                       if (mmc_card_doing_bkops(mq->card))
>>> +                               mmc_interrupt_bkops(mq->card);
>>> +
>>>                        set_current_state(TASK_RUNNING);
>>>                        mq->issue_fn(mq, req);
>>>                } else {
>>> @@ -68,6 +71,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..41ea923 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;
>>> +
>>
>>> +       card->host->caps2 |= MMC_CAP2_BKOPS;
>> I guess this should be set by the host driver?
> You're right...it's my mistake...i will remove this..
>>
>>> +       if ((!card || !card->ext_csd.bkops_en) &&
>>> +                       !(card->host->caps2 & MMC_CAP2_BKOPS))
>> I don't really understand this. If card is NULL it will seg-fault.
>>
>> Do you mean:
>> if (!card)
>>  return
>>
>> if (!card->ext_csd.bkops_en ||
>>   !(card->host->caps2 & MMC_CAP2_BKOPS))
>>    return
>>
>> Do you really need to check card != NULL? Consider BUG_ON
> Maybe if (!card && !card->ext_csd.bkops_en) is correctly...i think...
If card is NULL it will seg-fault.
Did you mean: if (card && !card->ext_csd.bkops_en)

I would suggest.
# return if card is NULL
if (!card)
  return

# if card is set, check the values.
if (!card->ext_csd.bkops_en ||
   !(card->host->caps2 & MMC_CAP2_BKOPS))
   return


>>
>>> +               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,49 @@ 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 = 0;
>>> +       unsigned long flags;
>>> +
>>> +       BUG_ON(!card);
>>> +
>>> +       err = mmc_interrupt_hpi(card);
>>> +
>>> +       spin_lock_irqsave(&card->host->lock, flags);
>>> +       mmc_card_clr_doing_bkops(card);
>>> +       spin_unlock_irqrestore(&card->host->lock, flags);
>>> +
>>> +       return err;
>>> +}
>>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>>> +
>>> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
>>> +{
>>> +       int err;
>>> +       u8 ext_csd[512];
>>> +
>>> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
>>> +       if (card->ext_csd.rev == 5)
>>> +               return 1;
>>> +
>>> +       err = mmc_send_ext_csd(card, ext_csd);
>>> +       if (err)
>>> +               return err;
>>> +
>>> +       return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
>>> +}
>>> +EXPORT_SYMBOL(mmc_is_exception_event);
>>> +
>>> +/**
>>>  *     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..af40fbb 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -448,6 +448,14 @@ 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;
>>> +                       card->ext_csd.bkops_en = 1;
>> What if the card has not enabled bkops? This bit could be unset.
>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
> When card is supported the bkops, bkops_en bit is set by default...
>
How do you know if the bkops_en is really enabled in th ext_csd?
It could be unset in the ext_csd, right?

>>
>>> +                       card->ext_csd.raw_bkops_status =
>>> +                               ext_csd[EXT_CSD_BKOPS_STATUS];
>>> +               }
>>> +
>>>                /* check whether the eMMC card supports HPI */
>>>                if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>>>                        card->ext_csd.hpi = 1;
>>> @@ -498,7 +506,6 @@ static inline void mmc_free_ext_csd(u8 *ext_csd)
>>>        kfree(ext_csd);
>>>  }
>>>
>>> -
>>>  static int mmc_compare_ext_csds(struct mmc_card *card, unsigned bus_width)
>>>  {
>>>        u8 *bw_ext_csd;
>>> 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 415f2db..34740c5 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 */
>>> @@ -84,6 +86,7 @@ struct mmc_ext_csd {
>>>        u8                      raw_sec_erase_mult;     /* 230 */
>>>        u8                      raw_sec_feature_support;/* 231 */
>>>        u8                      raw_trim_mult;          /* 232 */
>>> +       u8                      raw_bkops_status;       /* 246 */
>>>        u8                      raw_sectors[4];         /* 212 - 4 bytes */
>>>
>>>        unsigned int            feature_support;
>>> @@ -206,6 +209,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 +370,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 +380,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..f6b997a 100644
>>> --- a/include/linux/mmc/core.h
>>> +++ b/include/linux/mmc/core.h
>>> @@ -134,6 +134,8 @@ struct mmc_host;
>>>  struct mmc_card;
>>>  struct mmc_async_req;
>>>
>>> +extern int mmc_interrupt_bkops(struct mmc_card *);
>>> +extern int mmc_is_exception_event(struct mmc_card *, unsigned int);
>>>  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 +165,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..20b1420 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_EXCEPTION_EVENT     (1 << 6)        /* sx, a */
>>>  #define R1_APP_CMD             (1 << 5)        /* sr, c */
>>>
>>>  #define R1_STATE_IDLE  0
>>> @@ -273,11 +274,14 @@ struct _mmc_csd {
>>>  #define EXT_CSD_FLUSH_CACHE            32      /* W */
>>>  #define EXT_CSD_CACHE_CTRL             33      /* R/W */
>>>  #define EXT_CSD_POWER_OFF_NOTIFICATION 34      /* R/W */
>>> +#define EXT_CSD_EXCEPTION_STATUS       54      /* RO */
>>>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
>>>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>>>  #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 */
>>> @@ -310,9 +314,11 @@ struct _mmc_csd {
>>>  #define EXT_CSD_PWR_CL_200_360         237     /* RO */
>>>  #define EXT_CSD_PWR_CL_DDR_52_195      238     /* RO */
>>>  #define EXT_CSD_PWR_CL_DDR_52_360      239     /* RO */
>>> +#define EXT_CSD_BKOPS_STATUS           246     /* RO */
>>>  #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 */
>>>
>>>  /*
>>> @@ -373,4 +379,12 @@ struct _mmc_csd {
>>>  #define MMC_SWITCH_MODE_CLEAR_BITS     0x02    /* Clear bits which are 1 in value */
>>>  #define MMC_SWITCH_MODE_WRITE_BYTE     0x03    /* Set target to value */
>>>
>>> +/*
>>> + * EXCEPTION_EVENT_STATUS field (eMMC4.5)
>>> + */
>>> +#define EXT_CSD_URGENT_BKOPS           BIT(0)
>>> +#define EXT_CSD_DYNCAP_NEEDED          BIT(1)
>>> +#define EXT_CSD_SYSPOOL_EXHAUSTED      BIT(2)
>>> +#define EXT_CSD_PACKED_FAILURE         BIT(3)
>>> +
>>>  #endif /* LINUX_MMC_MMC_H */
>>> --
>>> 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
>>>
>>
>> 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] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-08 13:53     ` Per Forlin
@ 2011-11-08 18:37       ` Per Forlin
  2011-11-11  6:24       ` Jae hoon Chung
  1 sibling, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-11-08 18:37 UTC (permalink / raw)
  To: Jae hoon Chung
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

On Tue, Nov 8, 2011 at 2:53 PM, Per Forlin <per.lkml@gmail.com> wrote:
> Hi Jaehoon,
>
> On Tue, Nov 8, 2011 at 1:43 PM, Jae hoon Chung <jh80.chung@gmail.com> wrote:
>> Hi Per.
>>
>> 2011/11/5 Per Forlin <per.lkml@gmail.com>:
>>> Hi Jaehoon,
>>>
>>> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> 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.
>>>>
I'm thinking if it's good to add something in this commit message
about what has been proposed in this thread. The scope of this patch
is good and any additional optimizations could be added separately
later on.

Future considerations
 * Check BKOPS_LEVEL=1 and start BKOPS in a preventive manner.
 * Interrupt ongoing BKOPS before powering off the card.

Regards,
Per

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-02 10:28 [PATCH v2] mmc: support BKOPS feature for eMMC Jaehoon Chung
  2011-11-04 18:06 ` Per Forlin
@ 2011-11-10 22:01 ` Per Forlin
  2011-11-11  4:28   ` Jae hoon Chung
  2011-11-11  6:51 ` Dong, Chuanxiao
  2 siblings, 1 reply; 20+ messages in thread
From: Per Forlin @ 2011-11-10 22:01 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad,
	Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Jaehoon,

On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 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>
>
> ---
> Changelog V2:
>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>        - Add function to check Exception_status(for eMMC4.5)
>        - remove unnecessary code.
Will there be a V3 soon?

I have tested this patch with my minor changes on top and as far as I
can tell it looks ok.
I'm ready to accept this patch if those minor updates are made in V3.

Thanks,
Per

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-10 22:01 ` Per Forlin
@ 2011-11-11  4:28   ` Jae hoon Chung
  0 siblings, 0 replies; 20+ messages in thread
From: Jae hoon Chung @ 2011-11-11  4:28 UTC (permalink / raw)
  To: Per Forlin
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Per...

I wll send the patch v3 about next week.

Actually i didn't find how control about the level-1.
In future,,we need to add this control at this patch.

Best regards,
Jaehoon Chung

2011/11/11 Per Forlin <per.lkml@gmail.com>:
> Hi Jaehoon,
>
> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> 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>
>>
>> ---
>> Changelog V2:
>>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>        - Add function to check Exception_status(for eMMC4.5)
>>        - remove unnecessary code.
> Will there be a V3 soon?
>
> I have tested this patch with my minor changes on top and as far as I
> can tell it looks ok.
> I'm ready to accept this patch if those minor updates are made in V3.
>
> Thanks,
> 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] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-08 13:53     ` Per Forlin
  2011-11-08 18:37       ` Per Forlin
@ 2011-11-11  6:24       ` Jae hoon Chung
  2011-11-11  7:55         ` Per Forlin
  1 sibling, 1 reply; 20+ messages in thread
From: Jae hoon Chung @ 2011-11-11  6:24 UTC (permalink / raw)
  To: Per Forlin
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi per.

2011/11/8 Per Forlin <per.lkml@gmail.com>:
> Hi Jaehoon,
>
> On Tue, Nov 8, 2011 at 1:43 PM, Jae hoon Chung <jh80.chung@gmail.com> wrote:
>> Hi Per.
>>
>> 2011/11/5 Per Forlin <per.lkml@gmail.com>:
>>> Hi Jaehoon,
>>>
>>> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> 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>
>>>>
>>>> ---
>>>> Changelog V2:
>>>>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>>>        - Add function to check Exception_status(for eMMC4.5)
>>>>        - remove unnecessary code.
>>>>
>>>>  drivers/mmc/card/block.c   |    9 +++++
>>>>  drivers/mmc/card/queue.c   |    4 ++
>>>>  drivers/mmc/core/core.c    |   87 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>>>  include/linux/mmc/card.h   |   12 ++++++
>>>>  include/linux/mmc/core.h   |    3 ++
>>>>  include/linux/mmc/host.h   |    1 +
>>>>  include/linux/mmc/mmc.h    |   14 +++++++
>>>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index a1cb21f..fbfb405 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>>>>                case MMC_BLK_SUCCESS:
>>>>                case MMC_BLK_PARTIAL:
>>>>                        /*
>>>> +                        * Check BKOPS urgency from each R1 response
>>>> +                        */
>>>> +                       if (mmc_card_mmc(card) &&
>>>> +                               (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>>>> +                               if (mmc_is_exception_event(card,
>>>> +                                               EXT_CSD_URGENT_BKOPS))
>>>> +                                       mmc_card_set_need_bkops(card);
>>>> +                       }
>>>> +                       /*
>>>>                         * A block was successfully transferred.
>>>>                         */
>>>>                        mmc_blk_reset_success(md, type);
>>>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>>>> index dcad59c..20bb4a5 100644
>>>> --- a/drivers/mmc/card/queue.c
>>>> +++ b/drivers/mmc/card/queue.c
>>>> @@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
>>>>                spin_unlock_irq(q->queue_lock);
>>>>
>>>>                if (req || mq->mqrq_prev->req) {
>>>> +                       if (mmc_card_doing_bkops(mq->card))
>>>> +                               mmc_interrupt_bkops(mq->card);
>>>> +
>>>>                        set_current_state(TASK_RUNNING);
>>>>                        mq->issue_fn(mq, req);
>>>>                } else {
>>>> @@ -68,6 +71,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..41ea923 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;
>>>> +
>>>
>>>> +       card->host->caps2 |= MMC_CAP2_BKOPS;
>>> I guess this should be set by the host driver?
>> You're right...it's my mistake...i will remove this..
>>>
>>>> +       if ((!card || !card->ext_csd.bkops_en) &&
>>>> +                       !(card->host->caps2 & MMC_CAP2_BKOPS))
>>> I don't really understand this. If card is NULL it will seg-fault.
>>>
>>> Do you mean:
>>> if (!card)
>>>  return
>>>
>>> if (!card->ext_csd.bkops_en ||
>>>   !(card->host->caps2 & MMC_CAP2_BKOPS))
>>>    return
>>>
>>> Do you really need to check card != NULL? Consider BUG_ON
>> Maybe if (!card && !card->ext_csd.bkops_en) is correctly...i think...
> If card is NULL it will seg-fault.
> Did you mean: if (card && !card->ext_csd.bkops_en)
>
> I would suggest.
> # return if card is NULL
> if (!card)
>  return
>
> # if card is set, check the values.
> if (!card->ext_csd.bkops_en ||
>   !(card->host->caps2 & MMC_CAP2_BKOPS))
>   return
>
You're right...i will use BUGON()...and modify for ext_bkops_en checking
>
>>>
>>>> +               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,49 @@ 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 = 0;
>>>> +       unsigned long flags;
>>>> +
>>>> +       BUG_ON(!card);
>>>> +
>>>> +       err = mmc_interrupt_hpi(card);
>>>> +
>>>> +       spin_lock_irqsave(&card->host->lock, flags);
>>>> +       mmc_card_clr_doing_bkops(card);
>>>> +       spin_unlock_irqrestore(&card->host->lock, flags);
>>>> +
>>>> +       return err;
>>>> +}
>>>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>>>> +
>>>> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
>>>> +{
>>>> +       int err;
>>>> +       u8 ext_csd[512];
>>>> +
>>>> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
>>>> +       if (card->ext_csd.rev == 5)
>>>> +               return 1;
>>>> +
>>>> +       err = mmc_send_ext_csd(card, ext_csd);
>>>> +       if (err)
>>>> +               return err;
>>>> +
>>>> +       return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
>>>> +}
>>>> +EXPORT_SYMBOL(mmc_is_exception_event);
>>>> +
>>>> +/**
>>>>  *     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..af40fbb 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -448,6 +448,14 @@ 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;
>>>> +                       card->ext_csd.bkops_en = 1;
>>> What if the card has not enabled bkops? This bit could be unset.
>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>> When card is supported the bkops, bkops_en bit is set by default...
>>
> How do you know if the bkops_en is really enabled in th ext_csd?
> It could be unset in the ext_csd, right?

In emmc4.5 spec, if card is supported the bkops, bkops_en is set by default.
So i didn't assign ext_csd[EXT_CSD_BKOPS_EN], because maybe should be
set by defalut
when is supported the bkops. if i think wrong, plz let me know.

Best Regards,
Jaehoon Chung
>
>>>
>>>> +                       card->ext_csd.raw_bkops_status =
>>>> +                               ext_csd[EXT_CSD_BKOPS_STATUS];
>>>> +               }
>>>> +
>>>>                /* check whether the eMMC card supports HPI */
>>>>                if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>>>>                        card->ext_csd.hpi = 1;
>>>> @@ -498,7 +506,6 @@ static inline void mmc_free_ext_csd(u8 *ext_csd)
>>>>        kfree(ext_csd);
>>>>  }
>>>>
>>>> -
>>>>  static int mmc_compare_ext_csds(struct mmc_card *card, unsigned bus_width)
>>>>  {
>>>>        u8 *bw_ext_csd;
>>>> 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 415f2db..34740c5 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 */
>>>> @@ -84,6 +86,7 @@ struct mmc_ext_csd {
>>>>        u8                      raw_sec_erase_mult;     /* 230 */
>>>>        u8                      raw_sec_feature_support;/* 231 */
>>>>        u8                      raw_trim_mult;          /* 232 */
>>>> +       u8                      raw_bkops_status;       /* 246 */
>>>>        u8                      raw_sectors[4];         /* 212 - 4 bytes */
>>>>
>>>>        unsigned int            feature_support;
>>>> @@ -206,6 +209,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 +370,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 +380,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..f6b997a 100644
>>>> --- a/include/linux/mmc/core.h
>>>> +++ b/include/linux/mmc/core.h
>>>> @@ -134,6 +134,8 @@ struct mmc_host;
>>>>  struct mmc_card;
>>>>  struct mmc_async_req;
>>>>
>>>> +extern int mmc_interrupt_bkops(struct mmc_card *);
>>>> +extern int mmc_is_exception_event(struct mmc_card *, unsigned int);
>>>>  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 +165,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..20b1420 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_EXCEPTION_EVENT     (1 << 6)        /* sx, a */
>>>>  #define R1_APP_CMD             (1 << 5)        /* sr, c */
>>>>
>>>>  #define R1_STATE_IDLE  0
>>>> @@ -273,11 +274,14 @@ struct _mmc_csd {
>>>>  #define EXT_CSD_FLUSH_CACHE            32      /* W */
>>>>  #define EXT_CSD_CACHE_CTRL             33      /* R/W */
>>>>  #define EXT_CSD_POWER_OFF_NOTIFICATION 34      /* R/W */
>>>> +#define EXT_CSD_EXCEPTION_STATUS       54      /* RO */
>>>>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
>>>>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>>>>  #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 */
>>>> @@ -310,9 +314,11 @@ struct _mmc_csd {
>>>>  #define EXT_CSD_PWR_CL_200_360         237     /* RO */
>>>>  #define EXT_CSD_PWR_CL_DDR_52_195      238     /* RO */
>>>>  #define EXT_CSD_PWR_CL_DDR_52_360      239     /* RO */
>>>> +#define EXT_CSD_BKOPS_STATUS           246     /* RO */
>>>>  #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 */
>>>>
>>>>  /*
>>>> @@ -373,4 +379,12 @@ struct _mmc_csd {
>>>>  #define MMC_SWITCH_MODE_CLEAR_BITS     0x02    /* Clear bits which are 1 in value */
>>>>  #define MMC_SWITCH_MODE_WRITE_BYTE     0x03    /* Set target to value */
>>>>
>>>> +/*
>>>> + * EXCEPTION_EVENT_STATUS field (eMMC4.5)
>>>> + */
>>>> +#define EXT_CSD_URGENT_BKOPS           BIT(0)
>>>> +#define EXT_CSD_DYNCAP_NEEDED          BIT(1)
>>>> +#define EXT_CSD_SYSPOOL_EXHAUSTED      BIT(2)
>>>> +#define EXT_CSD_PACKED_FAILURE         BIT(3)
>>>> +
>>>>  #endif /* LINUX_MMC_MMC_H */
>>>> --
>>>> 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
>>>>
>>>
>>> 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] 20+ messages in thread

* RE: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-02 10:28 [PATCH v2] mmc: support BKOPS feature for eMMC Jaehoon Chung
  2011-11-04 18:06 ` Per Forlin
  2011-11-10 22:01 ` Per Forlin
@ 2011-11-11  6:51 ` Dong, Chuanxiao
  2011-11-11  7:48   ` Per Forlin
  2011-11-16  8:36   ` Jaehoon Chung
  2 siblings, 2 replies; 20+ messages in thread
From: Dong, Chuanxiao @ 2011-11-11  6:51 UTC (permalink / raw)
  To: Jaehoon Chung, linux-mmc, Jae hoon Chung
  Cc: Chris Ball, Kyungmin Park, Hanumath Prasad, Sebastian Rasmussen,
	Per Forlin, svenkatr

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Wednesday, November 02, 2011 6:29 PM
> To: linux-mmc
> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad; Sebastian Rasmussen; Per Forlin;
> svenkatr@ti.com; Dong, Chuanxiao
> Subject: [PATCH v2] mmc: support BKOPS feature for eMMC
> 
> 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>
> 
> ---
> Changelog V2:
> 	- Use EXCEPTION_STATUS instead of URGENT_BKOPS
> 	- Add function to check Exception_status(for eMMC4.5)
> 	- remove unnecessary code.
> 
>  drivers/mmc/card/block.c   |    9 +++++
>  drivers/mmc/card/queue.c   |    4 ++
>  drivers/mmc/core/core.c    |   87
> ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/core/mmc.c     |    9 ++++-
>  drivers/mmc/core/mmc_ops.c |    4 ++
>  include/linux/mmc/card.h   |   12 ++++++
>  include/linux/mmc/core.h   |    3 ++
>  include/linux/mmc/host.h   |    1 +
>  include/linux/mmc/mmc.h    |   14 +++++++
>  9 files changed, 142 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index a1cb21f..fbfb405 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
> *mq, struct request *rqc)
>  		case MMC_BLK_SUCCESS:
>  		case MMC_BLK_PARTIAL:
>  			/*
> +			 * Check BKOPS urgency from each R1 response
> +			 */
> +			if (mmc_card_mmc(card) &&
> +				(brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
> +				if (mmc_is_exception_event(card,
> +						EXT_CSD_URGENT_BKOPS))
> +					mmc_card_set_need_bkops(card);
> +			}
> +			/*
Have you thought about moving this into mmc_wait_for_req_done()? We can check if the command is a R1 or R1B or not, and set BKOPS bit in there if needed. I was just thinking if we put such code here, we may only cover the successful scenario but not for the failed cases. Putting in mmc_wait_for_req_done can cover all cases.

Thanks
Chuanxiao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  6:51 ` Dong, Chuanxiao
@ 2011-11-11  7:48   ` Per Forlin
  2011-11-11  8:32     ` Per Forlin
  2011-11-16  8:36   ` Jaehoon Chung
  1 sibling, 1 reply; 20+ messages in thread
From: Per Forlin @ 2011-11-11  7:48 UTC (permalink / raw)
  To: Dong, Chuanxiao
  Cc: Jaehoon Chung, linux-mmc, Jae hoon Chung, Chris Ball,
	Kyungmin Park, Hanumath Prasad, Sebastian Rasmussen, svenkatr

On Fri, Nov 11, 2011 at 7:51 AM, Dong, Chuanxiao
<chuanxiao.dong@intel.com> wrote:
> Hi Jaehoon,
>
>> -----Original Message-----
>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>> Sent: Wednesday, November 02, 2011 6:29 PM
>> To: linux-mmc
>> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad; Sebastian Rasmussen; Per Forlin;
>> svenkatr@ti.com; Dong, Chuanxiao
>> Subject: [PATCH v2] mmc: support BKOPS feature for eMMC
>>
>> 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>
>>
>> ---
>> Changelog V2:
>>       - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>       - Add function to check Exception_status(for eMMC4.5)
>>       - remove unnecessary code.
>>
>>  drivers/mmc/card/block.c   |    9 +++++
>>  drivers/mmc/card/queue.c   |    4 ++
>>  drivers/mmc/core/core.c    |   87
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>  include/linux/mmc/card.h   |   12 ++++++
>>  include/linux/mmc/core.h   |    3 ++
>>  include/linux/mmc/host.h   |    1 +
>>  include/linux/mmc/mmc.h    |   14 +++++++
>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index a1cb21f..fbfb405 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>> *mq, struct request *rqc)
>>               case MMC_BLK_SUCCESS:
>>               case MMC_BLK_PARTIAL:
>>                       /*
>> +                      * Check BKOPS urgency from each R1 response
>> +                      */
>> +                     if (mmc_card_mmc(card) &&
>> +                             (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>> +                             if (mmc_is_exception_event(card,
>> +                                             EXT_CSD_URGENT_BKOPS))
>> +                                     mmc_card_set_need_bkops(card);
>> +                     }
>> +                     /*
> Have you thought about moving this into mmc_wait_for_req_done()? We can check if the command is a R1 or R1B or not, and set BKOPS bit in there if needed. I was just thinking if we put such code here, we may only cover the successful scenario but not for the failed cases. Putting in mmc_wait_for_req_done can cover all cases.
>
Good point! I'm in favor of this change.

Thanks,
Per

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  6:24       ` Jae hoon Chung
@ 2011-11-11  7:55         ` Per Forlin
  2011-11-14  4:18           ` Jaehoon Chung
  0 siblings, 1 reply; 20+ messages in thread
From: Per Forlin @ 2011-11-11  7:55 UTC (permalink / raw)
  To: Jae hoon Chung
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

On Fri, Nov 11, 2011 at 7:24 AM, Jae hoon Chung <jh80.chung@gmail.com> wrote:
> Hi per.
>
> 2011/11/8 Per Forlin <per.lkml@gmail.com>:
>> Hi Jaehoon,
>>
>> On Tue, Nov 8, 2011 at 1:43 PM, Jae hoon Chung <jh80.chung@gmail.com> wrote:
>>> Hi Per.
>>>
>>> 2011/11/5 Per Forlin <per.lkml@gmail.com>:
>>>> Hi Jaehoon,
>>>>
>>>> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>> 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>
>>>>>
>>>>> ---
>>>>> Changelog V2:
>>>>>        - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>>>>        - Add function to check Exception_status(for eMMC4.5)
>>>>>        - remove unnecessary code.
>>>>>
>>>>>  drivers/mmc/card/block.c   |    9 +++++
>>>>>  drivers/mmc/card/queue.c   |    4 ++
>>>>>  drivers/mmc/core/core.c    |   87 ++++++++++++++++++++++++++++++++++++++++++++
>>>>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>>>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>>>>  include/linux/mmc/card.h   |   12 ++++++
>>>>>  include/linux/mmc/core.h   |    3 ++
>>>>>  include/linux/mmc/host.h   |    1 +
>>>>>  include/linux/mmc/mmc.h    |   14 +++++++
>>>>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>> index a1cb21f..fbfb405 100644
>>>>> --- a/drivers/mmc/card/block.c
>>>>> +++ b/drivers/mmc/card/block.c
>>>>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>>>>>                case MMC_BLK_SUCCESS:
>>>>>                case MMC_BLK_PARTIAL:
>>>>>                        /*
>>>>> +                        * Check BKOPS urgency from each R1 response
>>>>> +                        */
>>>>> +                       if (mmc_card_mmc(card) &&
>>>>> +                               (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>>>>> +                               if (mmc_is_exception_event(card,
>>>>> +                                               EXT_CSD_URGENT_BKOPS))
>>>>> +                                       mmc_card_set_need_bkops(card);
>>>>> +                       }
>>>>> +                       /*
>>>>>                         * A block was successfully transferred.
>>>>>                         */
>>>>>                        mmc_blk_reset_success(md, type);
>>>>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>>>>> index dcad59c..20bb4a5 100644
>>>>> --- a/drivers/mmc/card/queue.c
>>>>> +++ b/drivers/mmc/card/queue.c
>>>>> @@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
>>>>>                spin_unlock_irq(q->queue_lock);
>>>>>
>>>>>                if (req || mq->mqrq_prev->req) {
>>>>> +                       if (mmc_card_doing_bkops(mq->card))
>>>>> +                               mmc_interrupt_bkops(mq->card);
>>>>> +
>>>>>                        set_current_state(TASK_RUNNING);
>>>>>                        mq->issue_fn(mq, req);
>>>>>                } else {
>>>>> @@ -68,6 +71,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..41ea923 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;
>>>>> +
>>>>
>>>>> +       card->host->caps2 |= MMC_CAP2_BKOPS;
>>>> I guess this should be set by the host driver?
>>> You're right...it's my mistake...i will remove this..
>>>>
>>>>> +       if ((!card || !card->ext_csd.bkops_en) &&
>>>>> +                       !(card->host->caps2 & MMC_CAP2_BKOPS))
>>>> I don't really understand this. If card is NULL it will seg-fault.
>>>>
>>>> Do you mean:
>>>> if (!card)
>>>>  return
>>>>
>>>> if (!card->ext_csd.bkops_en ||
>>>>   !(card->host->caps2 & MMC_CAP2_BKOPS))
>>>>    return
>>>>
>>>> Do you really need to check card != NULL? Consider BUG_ON
>>> Maybe if (!card && !card->ext_csd.bkops_en) is correctly...i think...
>> If card is NULL it will seg-fault.
>> Did you mean: if (card && !card->ext_csd.bkops_en)
>>
>> I would suggest.
>> # return if card is NULL
>> if (!card)
>>  return
>>
>> # if card is set, check the values.
>> if (!card->ext_csd.bkops_en ||
>>   !(card->host->caps2 & MMC_CAP2_BKOPS))
>>   return
>>
> You're right...i will use BUGON()...and modify for ext_bkops_en checking
>>
>>>>
>>>>> +               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,49 @@ 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 = 0;
>>>>> +       unsigned long flags;
>>>>> +
>>>>> +       BUG_ON(!card);
>>>>> +
>>>>> +       err = mmc_interrupt_hpi(card);
>>>>> +
>>>>> +       spin_lock_irqsave(&card->host->lock, flags);
>>>>> +       mmc_card_clr_doing_bkops(card);
>>>>> +       spin_unlock_irqrestore(&card->host->lock, flags);
>>>>> +
>>>>> +       return err;
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>>>>> +
>>>>> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
>>>>> +{
>>>>> +       int err;
>>>>> +       u8 ext_csd[512];
>>>>> +
>>>>> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
>>>>> +       if (card->ext_csd.rev == 5)
>>>>> +               return 1;
>>>>> +
>>>>> +       err = mmc_send_ext_csd(card, ext_csd);
>>>>> +       if (err)
>>>>> +               return err;
>>>>> +
>>>>> +       return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_is_exception_event);
>>>>> +
>>>>> +/**
>>>>>  *     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..af40fbb 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -448,6 +448,14 @@ 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;
>>>>> +                       card->ext_csd.bkops_en = 1;
>>>> What if the card has not enabled bkops? This bit could be unset.
>>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>>> When card is supported the bkops, bkops_en bit is set by default...
>>>
>> How do you know if the bkops_en is really enabled in th ext_csd?
>> It could be unset in the ext_csd, right?
>
> In emmc4.5 spec, if card is supported the bkops, bkops_en is set by default.
If bkops_en is set by default it should be in the the ext_csd too, right?
It should be safer to trust the value of ext_csd[EXT_CSD_BKOPS_EN] in
this case I think.

Regards,
Per

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  7:48   ` Per Forlin
@ 2011-11-11  8:32     ` Per Forlin
  2011-11-11  8:42       ` Dong, Chuanxiao
  0 siblings, 1 reply; 20+ messages in thread
From: Per Forlin @ 2011-11-11  8:32 UTC (permalink / raw)
  To: Dong, Chuanxiao
  Cc: Jaehoon Chung, linux-mmc, Jae hoon Chung, Chris Ball,
	Kyungmin Park, Hanumath Prasad, Sebastian Rasmussen, svenkatr

On Fri, Nov 11, 2011 at 8:48 AM, Per Forlin <per.lkml@gmail.com> wrote:
> On Fri, Nov 11, 2011 at 7:51 AM, Dong, Chuanxiao
> <chuanxiao.dong@intel.com> wrote:
>> Hi Jaehoon,
>>
>>> -----Original Message-----
>>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>>> Sent: Wednesday, November 02, 2011 6:29 PM
>>> To: linux-mmc
>>> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad; Sebastian Rasmussen; Per Forlin;
>>> svenkatr@ti.com; Dong, Chuanxiao
>>> Subject: [PATCH v2] mmc: support BKOPS feature for eMMC
>>>
>>> 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>
>>>
>>> ---
>>> Changelog V2:
>>>       - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>>       - Add function to check Exception_status(for eMMC4.5)
>>>       - remove unnecessary code.
>>>
>>>  drivers/mmc/card/block.c   |    9 +++++
>>>  drivers/mmc/card/queue.c   |    4 ++
>>>  drivers/mmc/core/core.c    |   87
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>>  include/linux/mmc/card.h   |   12 ++++++
>>>  include/linux/mmc/core.h   |    3 ++
>>>  include/linux/mmc/host.h   |    1 +
>>>  include/linux/mmc/mmc.h    |   14 +++++++
>>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index a1cb21f..fbfb405 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>> *mq, struct request *rqc)
>>>               case MMC_BLK_SUCCESS:
>>>               case MMC_BLK_PARTIAL:
>>>                       /*
>>> +                      * Check BKOPS urgency from each R1 response
>>> +                      */
>>> +                     if (mmc_card_mmc(card) &&
>>> +                             (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>>> +                             if (mmc_is_exception_event(card,
>>> +                                             EXT_CSD_URGENT_BKOPS))
>>> +                                     mmc_card_set_need_bkops(card);
>>> +                     }
>>> +                     /*
>> Have you thought about moving this into mmc_wait_for_req_done()? We can check if the command is a R1 or R1B or not, and set BKOPS bit in there if needed. I was just thinking if we put such code here, we may only cover the successful scenario but not for the failed cases. Putting in mmc_wait_for_req_done can cover all cases.
>>
> Good point! I'm in favor of this change.
Or, put the check before the switch-case.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  8:32     ` Per Forlin
@ 2011-11-11  8:42       ` Dong, Chuanxiao
  2011-11-11 11:13         ` Per Forlin
  0 siblings, 1 reply; 20+ messages in thread
From: Dong, Chuanxiao @ 2011-11-11  8:42 UTC (permalink / raw)
  To: Per Forlin
  Cc: Jaehoon Chung, linux-mmc, Jae hoon Chung, Chris Ball,
	Kyungmin Park, Hanumath Prasad, Sebastian Rasmussen, svenkatr



> -----Original Message-----
> From: Per Forlin [mailto:per.lkml@gmail.com]
> Sent: Friday, November 11, 2011 4:32 PM
> To: Dong, Chuanxiao
> Cc: Jaehoon Chung; linux-mmc; Jae hoon Chung; Chris Ball; Kyungmin Park;
> Hanumath Prasad; Sebastian Rasmussen; svenkatr@ti.com
> Subject: Re: [PATCH v2] mmc: support BKOPS feature for eMMC
> 
> On Fri, Nov 11, 2011 at 8:48 AM, Per Forlin <per.lkml@gmail.com> wrote:
> > On Fri, Nov 11, 2011 at 7:51 AM, Dong, Chuanxiao
> > <chuanxiao.dong@intel.com> wrote:
> >> Hi Jaehoon,
> >>
> >>> -----Original Message-----
> >>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> >>> Sent: Wednesday, November 02, 2011 6:29 PM
> >>> To: linux-mmc
> >>> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad; Sebastian Rasmussen; Per
> Forlin;
> >>> svenkatr@ti.com; Dong, Chuanxiao
> >>> Subject: [PATCH v2] mmc: support BKOPS feature for eMMC
> >>>
> >>> 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>
> >>>
> >>> ---
> >>> Changelog V2:
> >>>       - Use EXCEPTION_STATUS instead of URGENT_BKOPS
> >>>       - Add function to check Exception_status(for eMMC4.5)
> >>>       - remove unnecessary code.
> >>>
> >>>  drivers/mmc/card/block.c   |    9 +++++
> >>>  drivers/mmc/card/queue.c   |    4 ++
> >>>  drivers/mmc/core/core.c    |   87
> >>> ++++++++++++++++++++++++++++++++++++++++++++
> >>>  drivers/mmc/core/mmc.c     |    9 ++++-
> >>>  drivers/mmc/core/mmc_ops.c |    4 ++
> >>>  include/linux/mmc/card.h   |   12 ++++++
> >>>  include/linux/mmc/core.h   |    3 ++
> >>>  include/linux/mmc/host.h   |    1 +
> >>>  include/linux/mmc/mmc.h    |   14 +++++++
> >>>  9 files changed, 142 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> >>> index a1cb21f..fbfb405 100644
> >>> --- a/drivers/mmc/card/block.c
> >>> +++ b/drivers/mmc/card/block.c
> >>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct
> mmc_queue
> >>> *mq, struct request *rqc)
> >>>               case MMC_BLK_SUCCESS:
> >>>               case MMC_BLK_PARTIAL:
> >>>                       /*
> >>> +                      * Check BKOPS urgency from each R1 response
> >>> +                      */
> >>> +                     if (mmc_card_mmc(card) &&
> >>> +                             (brq->cmd.resp[0] &
> R1_EXCEPTION_EVENT)) {
> >>> +                             if (mmc_is_exception_event(card,
> >>>
> +                                             EXT_CSD_URGENT_BK
> OPS))
> >>>
> +                                     mmc_card_set_need_bkops(card
> );
> >>> +                     }
> >>> +                     /*
> >> Have you thought about moving this into mmc_wait_for_req_done()? We can
> check if the command is a R1 or R1B or not, and set BKOPS bit in there if needed. I
> was just thinking if we put such code here, we may only cover the successful
> scenario but not for the failed cases. Putting in mmc_wait_for_req_done can cover
> all cases.
> >>
> > Good point! I'm in favor of this change.
> Or, put the check before the switch-case.
If we put in mmc_wait_for_req_done, then we can catch all R1 response command result not only for the READ/WRITE command, right? ERASE command is also using R1 response. However, I am not sure whether ERASE command can set the BKOPS Urgent bit for us. Anyway, I agree putting before switch-case is another good point.

Thanks
Chuanxiao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  8:42       ` Dong, Chuanxiao
@ 2011-11-11 11:13         ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-11-11 11:13 UTC (permalink / raw)
  To: Dong, Chuanxiao
  Cc: Jaehoon Chung, linux-mmc, Jae hoon Chung, Chris Ball,
	Kyungmin Park, Hanumath Prasad, Sebastian Rasmussen, svenkatr

On Fri, Nov 11, 2011 at 9:42 AM, Dong, Chuanxiao
<chuanxiao.dong@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Per Forlin [mailto:per.lkml@gmail.com]
>> Sent: Friday, November 11, 2011 4:32 PM
>> To: Dong, Chuanxiao
>> Cc: Jaehoon Chung; linux-mmc; Jae hoon Chung; Chris Ball; Kyungmin Park;
>> Hanumath Prasad; Sebastian Rasmussen; svenkatr@ti.com
>> Subject: Re: [PATCH v2] mmc: support BKOPS feature for eMMC
>>
>> On Fri, Nov 11, 2011 at 8:48 AM, Per Forlin <per.lkml@gmail.com> wrote:
>> > On Fri, Nov 11, 2011 at 7:51 AM, Dong, Chuanxiao
>> > <chuanxiao.dong@intel.com> wrote:
>> >> Hi Jaehoon,
>> >>
>> >>> -----Original Message-----
>> >>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>> >>> Sent: Wednesday, November 02, 2011 6:29 PM
>> >>> To: linux-mmc
>> >>> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad; Sebastian Rasmussen; Per
>> Forlin;
>> >>> svenkatr@ti.com; Dong, Chuanxiao
>> >>> Subject: [PATCH v2] mmc: support BKOPS feature for eMMC
>> >>>
>> >>> 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>
>> >>>
>> >>> ---
>> >>> Changelog V2:
>> >>>       - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>> >>>       - Add function to check Exception_status(for eMMC4.5)
>> >>>       - remove unnecessary code.
>> >>>
>> >>>  drivers/mmc/card/block.c   |    9 +++++
>> >>>  drivers/mmc/card/queue.c   |    4 ++
>> >>>  drivers/mmc/core/core.c    |   87
>> >>> ++++++++++++++++++++++++++++++++++++++++++++
>> >>>  drivers/mmc/core/mmc.c     |    9 ++++-
>> >>>  drivers/mmc/core/mmc_ops.c |    4 ++
>> >>>  include/linux/mmc/card.h   |   12 ++++++
>> >>>  include/linux/mmc/core.h   |    3 ++
>> >>>  include/linux/mmc/host.h   |    1 +
>> >>>  include/linux/mmc/mmc.h    |   14 +++++++
>> >>>  9 files changed, 142 insertions(+), 1 deletions(-)
>> >>>
>> >>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> >>> index a1cb21f..fbfb405 100644
>> >>> --- a/drivers/mmc/card/block.c
>> >>> +++ b/drivers/mmc/card/block.c
>> >>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct
>> mmc_queue
>> >>> *mq, struct request *rqc)
>> >>>               case MMC_BLK_SUCCESS:
>> >>>               case MMC_BLK_PARTIAL:
>> >>>                       /*
>> >>> +                      * Check BKOPS urgency from each R1 response
>> >>> +                      */
>> >>> +                     if (mmc_card_mmc(card) &&
>> >>> +                             (brq->cmd.resp[0] &
>> R1_EXCEPTION_EVENT)) {
>> >>> +                             if (mmc_is_exception_event(card,
>> >>>
>> +                                             EXT_CSD_URGENT_BK
>> OPS))
>> >>>
>> +                                     mmc_card_set_need_bkops(card
>> );
>> >>> +                     }
>> >>> +                     /*
>> >> Have you thought about moving this into mmc_wait_for_req_done()? We can
>> check if the command is a R1 or R1B or not, and set BKOPS bit in there if needed. I
>> was just thinking if we put such code here, we may only cover the successful
>> scenario but not for the failed cases. Putting in mmc_wait_for_req_done can cover
>> all cases.
>> >>
>> > Good point! I'm in favor of this change.
>> Or, put the check before the switch-case.
> If we put in mmc_wait_for_req_done, then we can catch all R1 response command result not only for the READ/WRITE command, right?
Yes I agree. The only reason for _not_ moving it to
wait_for_req_done() would be if this response (URGENT_BKOPS) is
directly connected to request handled by mmc_blk_issue_rw_rq() and no
one else. I don't know the answer.

>ERASE command is also using R1 response.
I wasn't aware of this. Reading the R1 response should be done in
wait_for_req_done().
Checking if the  response is (URGENT_BKOPS) could still be done in
mmc_blk_issue_rw_rq(), if this particular response only happens here.
Otherwise that needs to be done in wait_for_req_done() too.

Thanks,
Per

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  7:55         ` Per Forlin
@ 2011-11-14  4:18           ` Jaehoon Chung
  2011-11-28  9:39             ` Konstantin Dorfman
  0 siblings, 1 reply; 20+ messages in thread
From: Jaehoon Chung @ 2011-11-14  4:18 UTC (permalink / raw)
  To: Per Forlin
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Per

>>>>>>
>>>>>>        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;
>>>>>> +                       card->ext_csd.bkops_en = 1;
>>>>> What if the card has not enabled bkops? This bit could be unset.
>>>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>>>> When card is supported the bkops, bkops_en bit is set by default...
>>>>
>>> How do you know if the bkops_en is really enabled in th ext_csd?
>>> It could be unset in the ext_csd, right?
>>
>> In emmc4.5 spec, if card is supported the bkops, bkops_en is set by default.
> If bkops_en is set by default it should be in the the ext_csd too, right?
> It should be safer to trust the value of ext_csd[EXT_CSD_BKOPS_EN] in
> this case I think.

If we want to ensure whether bkops_en bit is set or not, your approach is right.

Best Regards,
Jaehoon Chung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-11  6:51 ` Dong, Chuanxiao
  2011-11-11  7:48   ` Per Forlin
@ 2011-11-16  8:36   ` Jaehoon Chung
  1 sibling, 0 replies; 20+ messages in thread
From: Jaehoon Chung @ 2011-11-16  8:36 UTC (permalink / raw)
  To: Dong, Chuanxiao
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, Per Forlin, svenkatr

Hi Chuanxiao,

On 11/11/2011 03:51 PM, Dong, Chuanxiao wrote:

> Hi Jaehoon,
> 
>> -----Original Message-----
>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>> Sent: Wednesday, November 02, 2011 6:29 PM
>> To: linux-mmc
>> Cc: Chris Ball; Kyungmin Park; Hanumath Prasad; Sebastian Rasmussen; Per Forlin;
>> svenkatr@ti.com; Dong, Chuanxiao
>> Subject: [PATCH v2] mmc: support BKOPS feature for eMMC
>>
>> 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>
>>
>> ---
>> Changelog V2:
>> 	- Use EXCEPTION_STATUS instead of URGENT_BKOPS
>> 	- Add function to check Exception_status(for eMMC4.5)
>> 	- remove unnecessary code.
>>
>>  drivers/mmc/card/block.c   |    9 +++++
>>  drivers/mmc/card/queue.c   |    4 ++
>>  drivers/mmc/core/core.c    |   87
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mmc/core/mmc.c     |    9 ++++-
>>  drivers/mmc/core/mmc_ops.c |    4 ++
>>  include/linux/mmc/card.h   |   12 ++++++
>>  include/linux/mmc/core.h   |    3 ++
>>  include/linux/mmc/host.h   |    1 +
>>  include/linux/mmc/mmc.h    |   14 +++++++
>>  9 files changed, 142 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index a1cb21f..fbfb405 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>> *mq, struct request *rqc)
>>  		case MMC_BLK_SUCCESS:
>>  		case MMC_BLK_PARTIAL:
>>  			/*
>> +			 * Check BKOPS urgency from each R1 response
>> +			 */
>> +			if (mmc_card_mmc(card) &&
>> +				(brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>> +				if (mmc_is_exception_event(card,
>> +						EXT_CSD_URGENT_BKOPS))
>> +					mmc_card_set_need_bkops(card);
>> +			}
>> +			/*

> Have you thought about moving this into mmc_wait_for_req_done()? We can check if the command is a R1 or R1B or not, and set BKOPS bit in there if needed. I was just thinking if we put such code here, we may only cover the successful scenario but not for the failed cases. Putting in mmc_wait_for_req_done can cover all cases.


Thanks for comment..I will move this into mmc_wait_for_req_done..and i will test.
Then i will modify and resend the patch with your opinion

Best Regards,
Jaehoon Chung

> 
> Thanks
> Chuanxiao
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-14  4:18           ` Jaehoon Chung
@ 2011-11-28  9:39             ` Konstantin Dorfman
  2011-11-28 11:59               ` Jaehoon Chung
  0 siblings, 1 reply; 20+ messages in thread
From: Konstantin Dorfman @ 2011-11-28  9:39 UTC (permalink / raw)
  Cc: Per Forlin, Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hello Jaehoon, Per

I have experimented with eMMC v4.41 card and BKOPS feature and want to add
info on EXT_CSD_BKOPS_EN[163] field. See below.

> Hi Per
>
>>>>>>>
>>>>>>>        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;
>>>>>>> +                       card->ext_csd.bkops_en = 1;
>>>>>> What if the card has not enabled bkops? This bit could be unset.
>>>>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>>>>> When card is supported the bkops, bkops_en bit is set by default...
>>>>>
>>>> How do you know if the bkops_en is really enabled in th ext_csd?
>>>> It could be unset in the ext_csd, right?
>>>
>>> In emmc4.5 spec, if card is supported the bkops, bkops_en is set by
>>> default.
>> If bkops_en is set by default it should be in the the ext_csd too,
>> right?
>> It should be safer to trust the value of ext_csd[EXT_CSD_BKOPS_EN] in
>> this case I think.
>
> If we want to ensure whether bkops_en bit is set or not, your approach is
> right.
>
> Best Regards,
> Jaehoon Chung

First of all protocol state:
------------------------------
7.4.57 BKOPS_EN [163]
This field allows the _host_ to indicate to the device if it is expected
to periodically manually start background operations by writing to the
BKOPS_START field.

...In order for the device to know if host is going to periodically start
background operations, host shall set bit 0 of BKOPS_EN (EXT_CSD byte
[163]) to indicate that it is going to write to BKOPS_START periodically.
The device may then delay some of its maintenance operations to when host
writes to BKOPS_START.
------------------------------

My card supports BKOPS (by reading Bit 0 of BKOPS_SUPPORT (EXT_CSD byte
[502]), but BKOPS_EN[163] is not set. So I suppose, original code to set
BKOPS_EN[163] is needed, otherwise card will never delay its maintenance
operations.

Thanks,
Kostya





^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-28  9:39             ` Konstantin Dorfman
@ 2011-11-28 11:59               ` Jaehoon Chung
  2011-11-28 12:10                 ` Kyungmin Park
  0 siblings, 1 reply; 20+ messages in thread
From: Jaehoon Chung @ 2011-11-28 11:59 UTC (permalink / raw)
  To: Konstantin Dorfman
  Cc: Jaehoon Chung, Per Forlin, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

Hi Kostya

On 11/28/2011 06:39 PM, Konstantin Dorfman wrote:

> Hello Jaehoon, Per
> 
> I have experimented with eMMC v4.41 card and BKOPS feature and want to add
> info on EXT_CSD_BKOPS_EN[163] field. See below.
> 
>> Hi Per
>>
>>>>>>>>
>>>>>>>>        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;
>>>>>>>> +                       card->ext_csd.bkops_en = 1;
>>>>>>> What if the card has not enabled bkops? This bit could be unset.
>>>>>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>>>>>> When card is supported the bkops, bkops_en bit is set by default...
>>>>>>
>>>>> How do you know if the bkops_en is really enabled in th ext_csd?
>>>>> It could be unset in the ext_csd, right?
>>>>
>>>> In emmc4.5 spec, if card is supported the bkops, bkops_en is set by
>>>> default.
>>> If bkops_en is set by default it should be in the the ext_csd too,
>>> right?
>>> It should be safer to trust the value of ext_csd[EXT_CSD_BKOPS_EN] in
>>> this case I think.
>>
>> If we want to ensure whether bkops_en bit is set or not, your approach is
>> right.
>>
>> Best Regards,
>> Jaehoon Chung
> 
> First of all protocol state:
> ------------------------------
> 7.4.57 BKOPS_EN [163]
> This field allows the _host_ to indicate to the device if it is expected
> to periodically manually start background operations by writing to the
> BKOPS_START field.
> 
> ...In order for the device to know if host is going to periodically start
> background operations, host shall set bit 0 of BKOPS_EN (EXT_CSD byte
> [163]) to indicate that it is going to write to BKOPS_START periodically.
> The device may then delay some of its maintenance operations to when host
> writes to BKOPS_START.
> ------------------------------
> 
> My card supports BKOPS (by reading Bit 0 of BKOPS_SUPPORT (EXT_CSD byte
> [502]), but BKOPS_EN[163] is not set. So I suppose, original code to set
> BKOPS_EN[163] is needed, otherwise card will never delay its maintenance
> operations.


Ok...i will add the code for setting BKOPS_EN bit in patch-v4.
Thanks,
Jaehoon Chung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-28 11:59               ` Jaehoon Chung
@ 2011-11-28 12:10                 ` Kyungmin Park
  2011-11-29 11:47                   ` Konstantin Dorfman
  0 siblings, 1 reply; 20+ messages in thread
From: Kyungmin Park @ 2011-11-28 12:10 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: Konstantin Dorfman, Per Forlin, linux-mmc, Chris Ball,
	Hanumath Prasad, Sebastian Rasmussen, svenkatr, chuanxiao.dong

On 11/28/11, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Kostya
>
> On 11/28/2011 06:39 PM, Konstantin Dorfman wrote:
>
>> Hello Jaehoon, Per
>>
>> I have experimented with eMMC v4.41 card and BKOPS feature and want to add
>> info on EXT_CSD_BKOPS_EN[163] field. See below.
>>
>>> Hi Per
>>>
>>>>>>>>>
>>>>>>>>>        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;
>>>>>>>>> +                       card->ext_csd.bkops_en = 1;
>>>>>>>> What if the card has not enabled bkops? This bit could be unset.
>>>>>>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>>>>>>> When card is supported the bkops, bkops_en bit is set by default...
>>>>>>>
>>>>>> How do you know if the bkops_en is really enabled in th ext_csd?
>>>>>> It could be unset in the ext_csd, right?
>>>>>
>>>>> In emmc4.5 spec, if card is supported the bkops, bkops_en is set by
>>>>> default.
>>>> If bkops_en is set by default it should be in the the ext_csd too,
>>>> right?
>>>> It should be safer to trust the value of ext_csd[EXT_CSD_BKOPS_EN] in
>>>> this case I think.
>>>
>>> If we want to ensure whether bkops_en bit is set or not, your approach is
>>> right.
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>
>> First of all protocol state:
>> ------------------------------
>> 7.4.57 BKOPS_EN [163]
>> This field allows the _host_ to indicate to the device if it is expected
>> to periodically manually start background operations by writing to the
>> BKOPS_START field.
>>
>> ...In order for the device to know if host is going to periodically start
>> background operations, host shall set bit 0 of BKOPS_EN (EXT_CSD byte
>> [163]) to indicate that it is going to write to BKOPS_START periodically.
>> The device may then delay some of its maintenance operations to when host
>> writes to BKOPS_START.
>> ------------------------------
>>
>> My card supports BKOPS (by reading Bit 0 of BKOPS_SUPPORT (EXT_CSD byte
>> [502]), but BKOPS_EN[163] is not set. So I suppose, original code to set
>> BKOPS_EN[163] is needed, otherwise card will never delay its maintenance
>> operations.
>
>
> Ok...i will add the code for setting BKOPS_EN bit in patch-v4.
I hope this features are provided from platform data. since it's not
mandatory for all devices

Thank you,
Kyungmin Park
> Thanks,
> Jaehoon Chung
> --
> 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] 20+ messages in thread

* Re: [PATCH v2] mmc: support BKOPS feature for eMMC
  2011-11-28 12:10                 ` Kyungmin Park
@ 2011-11-29 11:47                   ` Konstantin Dorfman
  0 siblings, 0 replies; 20+ messages in thread
From: Konstantin Dorfman @ 2011-11-29 11:47 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Jaehoon Chung, Konstantin Dorfman, Per Forlin, linux-mmc,
	Chris Ball, Hanumath Prasad, Sebastian Rasmussen, svenkatr,
	chuanxiao.dong

>>>
>>> First of all protocol state:
>>> ------------------------------
>>> 7.4.57 BKOPS_EN [163]
>>> This field allows the _host_ to indicate to the device if it is
>>> expected
>>> to periodically manually start background operations by writing to the
>>> BKOPS_START field.
>>>
>>> ...In order for the device to know if host is going to periodically
>>> start
>>> background operations, host shall set bit 0 of BKOPS_EN (EXT_CSD byte
>>> [163]) to indicate that it is going to write to BKOPS_START
>>> periodically.
>>> The device may then delay some of its maintenance operations to when
>>> host
>>> writes to BKOPS_START.
>>> ------------------------------
>>>
>>> My card supports BKOPS (by reading Bit 0 of BKOPS_SUPPORT (EXT_CSD byte
>>> [502]), but BKOPS_EN[163] is not set. So I suppose, original code to
>>> set
>>> BKOPS_EN[163] is needed, otherwise card will never delay its
>>> maintenance
>>> operations.
>>
>>
>> Ok...i will add the code for setting BKOPS_EN bit in patch-v4.
> I hope this features are provided from platform data. since it's not
> mandatory for all devices
This feature is mandatory for eMMC v4.5, but it may be enabled only when
card supports BKOPS_SUPPORT[502] is set (and this one depend on device)
Thanks,
Kostya


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-11-29 11:47 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-02 10:28 [PATCH v2] mmc: support BKOPS feature for eMMC Jaehoon Chung
2011-11-04 18:06 ` Per Forlin
2011-11-08 12:43   ` Jae hoon Chung
2011-11-08 13:53     ` Per Forlin
2011-11-08 18:37       ` Per Forlin
2011-11-11  6:24       ` Jae hoon Chung
2011-11-11  7:55         ` Per Forlin
2011-11-14  4:18           ` Jaehoon Chung
2011-11-28  9:39             ` Konstantin Dorfman
2011-11-28 11:59               ` Jaehoon Chung
2011-11-28 12:10                 ` Kyungmin Park
2011-11-29 11:47                   ` Konstantin Dorfman
2011-11-10 22:01 ` Per Forlin
2011-11-11  4:28   ` Jae hoon Chung
2011-11-11  6:51 ` Dong, Chuanxiao
2011-11-11  7:48   ` Per Forlin
2011-11-11  8:32     ` Per Forlin
2011-11-11  8:42       ` Dong, Chuanxiao
2011-11-11 11:13         ` Per Forlin
2011-11-16  8:36   ` 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.