All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
@ 2012-07-17 19:34 merez
  2012-07-18  6:42 ` Jaehoon Chung
  0 siblings, 1 reply; 10+ messages in thread
From: merez @ 2012-07-17 19:34 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad,
	Per FORLIN, Sebastian Rasmussen, Dong, Chuanxiao, svenkatr,
	Saugata Das, Konstantin Dorfman, Adrian Hunter, Maya Erez,
	Ulf Hansson

See my comments below.

> +/**
> + *	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;
> +	int timeout;
> +	u8 use_busy_signal;
> +
> +	BUG_ON(!card);
> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))

Can you please explain why we need to have MMC_CAP2_BKOPS in addition to
card->ext_csd.bkops_en?

+		return;
> +
> +	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
> +		if (card->ext_csd.raw_bkops_status)
> +			mmc_card_set_need_bkops(card);

I think we can remove the bkops need flag. You can just return here in
case it is not needed instead of updating the flag and checking it in the
next line.


> @@ -354,6 +422,20 @@ struct mmc_async_req *mmc_start_req(struct mmc_host
*host,
>  	if (host->areq) {
>  		mmc_wait_for_req_done(host, host->areq->mrq);
>  		err = host->areq->err_check(host->card, host->areq);
> +		/*
> +		 * Check BKOPS urgency from each R1 response
> +		 */
> +		if (host->card && mmc_card_mmc(host->card) &&
> +		((mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1) ||
> +		 (mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1B)) &&
> +		(host->areq->mrq->cmd->resp[0] & R1_EXP_EVENT))
> +			mmc_start_bkops(host->card);
> +		/*
> +		 * If areq exsit,
> +		 * we stop the bkops for foreground operation
> +		 */
> +		if (areq && mmc_card_doing_bkops(host->card))
> +			err = mmc_stop_bkops(host->card);

I think that mmc_start_bkops should handle only level 2 and 3 for now.
Since it is not likely to have an exception with level 1 on, the best way
to deal with it is inside mmc_start_bkops (starting the BKOPs for levels 2
and 3).
When the periodiv BKOPs will be added, then level 1 will be fully
supported. In such a case mmc_start_bkops should know to start BKOPs on
level 1 only if it is called due to the periodic delayed work (for
example, by adding a flag to mmc_start_bkops)

> +int mmc_stop_bkops(struct mmc_card *card)
> +{
> +	int err = 0;
> +
> +	BUG_ON(!card);
> +	err = mmc_interrupt_hpi(card);
> +
> +	/*
> +	 * if err is EINVAL, it's status that can't issue HPI.
> +	 * Maybe it should be completed the BKOPS.

should complete

> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 4ad994a..baf90e0 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -368,18 +368,19 @@ int mmc_spi_set_crc(struct mmc_host *host, int
use_crc)
>  }
>
>  /**
> - *	mmc_switch - modify EXT_CSD register
> + *	__mmc_switch - modify EXT_CSD register
>   *	@card: the MMC card associated with the data transfer
>   *	@set: cmd set values
>   *	@index: EXT_CSD register index
>   *	@value: value to program into EXT_CSD register
>   *	@timeout_ms: timeout (ms) for operation performed by register write,
*                   timeout of zero implies maximum possible timeout
> + *                   @wait_for_prod_done: is waiting for program done

Change the comment for the new parameter: use_busy_signal

>   *
>   *	Modifies the EXT_CSD register for selected card.
>   */
> -int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, -
   unsigned int timeout_ms)
> +int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, +
     unsigned int timeout_ms, u8 use_busy_signal)

Use bool instead of u8

>  {
>  	int err;
>  	struct mmc_command cmd = {0};
> @@ -393,13 +394,24 @@ int mmc_switch(struct mmc_card *card, u8 set, u8
index, u8 value,
>  		  (index << 16) |
>  		  (value << 8) |
>  		  set;
> -	cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
> +
> +	cmd.flags = MMC_CMD_AC;
> +	if (use_busy_signal)
> +		cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
> +	else
> +		cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
> +
>  	cmd.cmd_timeout_ms = timeout_ms;
>
>  	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
>  	if (err)
>  		return err;
>
> +	/* No need to check card status in case of BKOPS LEVEL2 switch*/

have a space before the */

> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
cdfbc3c..b9e11aa 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -79,10 +79,13 @@ 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 */
>  	unsigned int            data_sector_size;       /* 512 bytes or 4KB */
unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
>  	unsigned int		boot_ro_lock;		/* ro lock support */
>  	bool			boot_ro_lockable;
> +	u8			raw_exception_status;	/* 53 */
>  	u8			raw_partition_support;	/* 160 */
>  	u8			raw_erased_mem_count;	/* 181 */
>  	u8			raw_ext_csd_structure;	/* 194 */
> @@ -96,6 +99,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;
> @@ -229,6 +233,9 @@ struct mmc_card {
>  #define MMC_CARD_REMOVED	(1<<7)		/* card has been removed */
>  #define MMC_STATE_HIGHSPEED_200	(1<<8)		/* card is in HS200 mode */
#define MMC_STATE_SLEEP		(1<<9)		/* card is in sleep state */
> +#define MMC_STATE_NEED_BKOPS	(1<<10)		/* card need to do BKOPS */
+#define MMC_STATE_DOING_BKOPS	(1<<11)		/* card is doing BKOPS */
+#define MMC_STATE_CHECK_BKOPS	(1<<12)		/* card need to check BKOPS */

MMC_STATE_CHECK_BKOPS is not used, can be removed

> @@ -400,6 +407,9 @@ static inline void __maybe_unused
remove_quirk(struct
> mmc_card *card, int data)
>  #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>  #define mmc_card_removed(c)	((c) && ((c)->state & MMC_CARD_REMOVED))
#define mmc_card_is_sleep(c)	((c)->state & MMC_STATE_SLEEP)
> +#define mmc_card_need_bkops(c)	((c)->state & MMC_STATE_NEED_BKOPS)
This flag can also be removed
+#define mmc_card_doing_bkops(c)	((c)->state & MMC_STATE_DOING_BKOPS)
+#define mmc_card_check_bkops(c) ((c)->state & MMC_STATE_CHECK_BKOPS)

mmc_card_check_bkops is not used, can be removed

>
>  #define mmc_card_set_present(c)	((c)->state |= MMC_STATE_PRESENT)
#define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
> @@ -412,7 +422,13 @@ static inline void __maybe_unused
remove_quirk(struct
> mmc_card *card, int data)
>  #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
#define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
#define mmc_card_set_sleep(c)	((c)->state |= MMC_STATE_SLEEP)
> +#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_set_check_bkops(c) ((c)->state |=
MMC_STATE_CHECK_BKOPS)

mmc_card_set_check_bkops can be removed

>
> +#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)
> +#define mmc_card_clr_check_bkops(c) ((c)->state &=
> ~MMC_STATE_CHECK_BKOPS)

mmc_card_clr_check_bkops can be removed

Thanks,
Maya
-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum









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

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-17 19:34 [PATCH v10] mmc: support BKOPS feature for eMMC merez
@ 2012-07-18  6:42 ` Jaehoon Chung
  2012-07-18 17:13   ` merez
  0 siblings, 1 reply; 10+ messages in thread
From: Jaehoon Chung @ 2012-07-18  6:42 UTC (permalink / raw)
  To: merez
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Per FORLIN, Sebastian Rasmussen, Dong,
	Chuanxiao, svenkatr, Saugata Das, Konstantin Dorfman,
	Adrian Hunter, Ulf Hansson

On 07/18/2012 04:34 AM, merez@codeaurora.org wrote:
> See my comments below.
> 
>> +/**
>> + *	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;
>> +	int timeout;
>> +	u8 use_busy_signal;
>> +
>> +	BUG_ON(!card);
>> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
> 
> Can you please explain why we need to have MMC_CAP2_BKOPS in addition to
> card->ext_csd.bkops_en?
We can remove the MMC_CAP2_BKOPS. but if someone didn't want to use the bkops feature,
then can control with that capability.
> 
> +		return;
>> +
>> +	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
>> +		if (card->ext_csd.raw_bkops_status)
>> +			mmc_card_set_need_bkops(card);
> 
> I think we can remove the bkops need flag. You can just return here in
> case it is not needed instead of updating the flag and checking it in the
> next line.
Right..it can remove..i will remove.
> 
> 
>> @@ -354,6 +422,20 @@ struct mmc_async_req *mmc_start_req(struct mmc_host
> *host,
>>  	if (host->areq) {
>>  		mmc_wait_for_req_done(host, host->areq->mrq);
>>  		err = host->areq->err_check(host->card, host->areq);
>> +		/*
>> +		 * Check BKOPS urgency from each R1 response
>> +		 */
>> +		if (host->card && mmc_card_mmc(host->card) &&
>> +		((mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1) ||
>> +		 (mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1B)) &&
>> +		(host->areq->mrq->cmd->resp[0] & R1_EXP_EVENT))
>> +			mmc_start_bkops(host->card);
>> +		/*
>> +		 * If areq exsit,
>> +		 * we stop the bkops for foreground operation
>> +		 */
>> +		if (areq && mmc_card_doing_bkops(host->card))
>> +			err = mmc_stop_bkops(host->card);
> 
> I think that mmc_start_bkops should handle only level 2 and 3 for now.
> Since it is not likely to have an exception with level 1 on, the best way
> to deal with it is inside mmc_start_bkops (starting the BKOPs for levels 2
> and 3).
> When the periodiv BKOPs will be added, then level 1 will be fully
> supported. In such a case mmc_start_bkops should know to start BKOPs on
> level 1 only if it is called due to the periodic delayed work (for
> example, by adding a flag to mmc_start_bkops)
> 
>> +int mmc_stop_bkops(struct mmc_card *card)
>> +{
>> +	int err = 0;
>> +
>> +	BUG_ON(!card);
>> +	err = mmc_interrupt_hpi(card);
>> +
>> +	/*
>> +	 * if err is EINVAL, it's status that can't issue HPI.
>> +	 * Maybe it should be completed the BKOPS.
> 
> should complete
> 
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 4ad994a..baf90e0 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -368,18 +368,19 @@ int mmc_spi_set_crc(struct mmc_host *host, int
> use_crc)
>>  }
>>
>>  /**
>> - *	mmc_switch - modify EXT_CSD register
>> + *	__mmc_switch - modify EXT_CSD register
>>   *	@card: the MMC card associated with the data transfer
>>   *	@set: cmd set values
>>   *	@index: EXT_CSD register index
>>   *	@value: value to program into EXT_CSD register
>>   *	@timeout_ms: timeout (ms) for operation performed by register write,
> *                   timeout of zero implies maximum possible timeout
>> + *                   @wait_for_prod_done: is waiting for program done
> 
> Change the comment for the new parameter: use_busy_signal
Will fix
> 
>>   *
>>   *	Modifies the EXT_CSD register for selected card.
>>   */
>> -int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, -
>    unsigned int timeout_ms)
>> +int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, +
>      unsigned int timeout_ms, u8 use_busy_signal)
> 
> Use bool instead of u8
Will change
> 
>>  {
>>  	int err;
>>  	struct mmc_command cmd = {0};
>> @@ -393,13 +394,24 @@ int mmc_switch(struct mmc_card *card, u8 set, u8
> index, u8 value,
>>  		  (index << 16) |
>>  		  (value << 8) |
>>  		  set;
>> -	cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
>> +
>> +	cmd.flags = MMC_CMD_AC;
>> +	if (use_busy_signal)
>> +		cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
>> +	else
>> +		cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
>> +
>>  	cmd.cmd_timeout_ms = timeout_ms;
>>
>>  	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
>>  	if (err)
>>  		return err;
>>
>> +	/* No need to check card status in case of BKOPS LEVEL2 switch*/
> 
> have a space before the */
> 
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
> cdfbc3c..b9e11aa 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -79,10 +79,13 @@ 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 */
>>  	unsigned int            data_sector_size;       /* 512 bytes or 4KB */
> unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
>>  	unsigned int		boot_ro_lock;		/* ro lock support */
>>  	bool			boot_ro_lockable;
>> +	u8			raw_exception_status;	/* 53 */
>>  	u8			raw_partition_support;	/* 160 */
>>  	u8			raw_erased_mem_count;	/* 181 */
>>  	u8			raw_ext_csd_structure;	/* 194 */
>> @@ -96,6 +99,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;
>> @@ -229,6 +233,9 @@ struct mmc_card {
>>  #define MMC_CARD_REMOVED	(1<<7)		/* card has been removed */
>>  #define MMC_STATE_HIGHSPEED_200	(1<<8)		/* card is in HS200 mode */
> #define MMC_STATE_SLEEP		(1<<9)		/* card is in sleep state */
>> +#define MMC_STATE_NEED_BKOPS	(1<<10)		/* card need to do BKOPS */
> +#define MMC_STATE_DOING_BKOPS	(1<<11)		/* card is doing BKOPS */
> +#define MMC_STATE_CHECK_BKOPS	(1<<12)		/* card need to check BKOPS */
> 
> MMC_STATE_CHECK_BKOPS is not used, can be removed
> 
>> @@ -400,6 +407,9 @@ static inline void __maybe_unused
> remove_quirk(struct
>> mmc_card *card, int data)
>>  #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>>  #define mmc_card_removed(c)	((c) && ((c)->state & MMC_CARD_REMOVED))
> #define mmc_card_is_sleep(c)	((c)->state & MMC_STATE_SLEEP)
>> +#define mmc_card_need_bkops(c)	((c)->state & MMC_STATE_NEED_BKOPS)
> This flag can also be removed
> +#define mmc_card_doing_bkops(c)	((c)->state & MMC_STATE_DOING_BKOPS)
> +#define mmc_card_check_bkops(c) ((c)->state & MMC_STATE_CHECK_BKOPS)
> 
> mmc_card_check_bkops is not used, can be removed
> 
>>
>>  #define mmc_card_set_present(c)	((c)->state |= MMC_STATE_PRESENT)
> #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
>> @@ -412,7 +422,13 @@ static inline void __maybe_unused
> remove_quirk(struct
>> mmc_card *card, int data)
>>  #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
> #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
> #define mmc_card_set_sleep(c)	((c)->state |= MMC_STATE_SLEEP)
>> +#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_set_check_bkops(c) ((c)->state |=
> MMC_STATE_CHECK_BKOPS)
> 
> mmc_card_set_check_bkops can be removed
> 
>>
>> +#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)
>> +#define mmc_card_clr_check_bkops(c) ((c)->state &=
>> ~MMC_STATE_CHECK_BKOPS)
> 
> mmc_card_clr_check_bkops can be removed
> 
> Thanks,
> Maya
> 



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

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-18  6:42 ` Jaehoon Chung
@ 2012-07-18 17:13   ` merez
  0 siblings, 0 replies; 10+ messages in thread
From: merez @ 2012-07-18 17:13 UTC (permalink / raw)
  Cc: merez, Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Per FORLIN, Sebastian Rasmussen, Dong,
	Chuanxiao, svenkatr, Saugata Das, Konstantin Dorfman,
	Adrian Hunter, Ulf Hansson


>>> +void mmc_start_bkops(struct mmc_card *card)
>>> +{
>>> +	int err;
>>> +	int timeout;
>>> +	u8 use_busy_signal;
>>> +
>>> +	BUG_ON(!card);
>>> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
>>
>> Can you please explain why we need to have MMC_CAP2_BKOPS in addition to
>> card->ext_csd.bkops_en?
> We can remove the MMC_CAP2_BKOPS. but if someone didn't want to use the
> bkops feature,
> then can control with that capability.

I don't think that the host can decide not to support BKOPs if it is
enabled on the card. In such a case we might run into performance
degradation or even non functionality of the card if it needs to perform
BKOPs.

Thanks,
Maya
-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-18  6:16     ` Adrian Hunter
@ 2012-07-18  6:31       ` merez
  0 siblings, 0 replies; 10+ messages in thread
From: merez @ 2012-07-18  6:31 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Per FORLIN, Sebastian Rasmussen, Dong,
	Chuanxiao, svenkatr, Saugata Das, Konstantin Dorfman, Maya Erez,
	Ulf Hansson


On Tue, July 17, 2012 11:16 pm, Adrian Hunter wrote:
> On 17/07/12 15:58, Jaehoon Chung wrote:
>> On 07/17/2012 09:30 PM, Adrian Hunter wrote:
>>> On 17/07/12 05:44, Jaehoon Chung wrote:
>>>
>>> <snip>
>>>
>>>> +/**
>>>> + *	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;
>>>> +	int timeout;
>>>> +	u8 use_busy_signal;
>>>> +
>>>> +	BUG_ON(!card);
>>>> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 &
>>>> MMC_CAP2_BKOPS))
>>>> +		return;
>>>> +
>>>> +	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
>>>> +		if (card->ext_csd.raw_bkops_status)
>>>> +			mmc_card_set_need_bkops(card);
>>>> +
>>>> +	/*
>>>> +	 * 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);
>>>> +	if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
>>>> +		timeout = MMC_BKOPS_MAX_TIMEOUT;
>>>> +		use_busy_signal = 0;
>>>> +	} else {
>>>> +		timeout = 0;
>>>> +		use_busy_signal = 1;
>>>> +	}
>>>
>>> Is this the right way around?
>>
>> use the mmc_switch() with R1B..
>> this case is no problem, because after sending bkops, repeat the
>> checking card status until prg-done.
>> But, at sdhci controller, be occurred data timeout error.
>> Because response type is r1b, and timeout value is too large.
>> (Actually, i think that is controller's problem..but just its my test
>> environment.)
>>
>> if other sdhci controller working well with R1b, use_busy_signal need
>> not.
>> Just use the mmc_switch().
>>
>> But When running bkops level2/3 use with R1 type, also no problem.
>> Because the also checking status in mmc_switch() until prg-done.
>
> You added:
>
> +	/*
> +	 * For urgent bkops status (LEVEL_2 and more)
> +	 * bkops executed synchronously, otherwise
> +	 * the operation is in progress
> +	 */
> +	if (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2)
> +		mmc_card_set_doing_bkops(card);
>
> But:
> 	status < 2		=> use_busy_signal = 1
> 	use_busy_signal = 1	=> waiting
> 	waiting			=> the operation is NOT in progress
>
> Hence my question: Is this the right way around?

Good catch.
in mmc_start_bkops, use_busy_signal should be set to 1 in case
card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2

>
> Also this change is needed for the addition of 'use_busy_signal' to
> mmc_switch:
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index baf90e0..e202a5e 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -417,7 +417,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8
> index, u8 value,
>                 err = mmc_send_status(card, &status);
>                 if (err)
>                         return err;
> -               if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
> +               if (use_busy_signal && (card->host->caps &
> MMC_CAP_WAIT_WHILE_BUSY))
>                         break;
>                 if (mmc_host_is_spi(card->host))
>                         break;
>
>


-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-17 12:58   ` Jaehoon Chung
@ 2012-07-18  6:16     ` Adrian Hunter
  2012-07-18  6:31       ` merez
  0 siblings, 1 reply; 10+ messages in thread
From: Adrian Hunter @ 2012-07-18  6:16 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad,
	Per FORLIN, Sebastian Rasmussen, Dong, Chuanxiao, svenkatr,
	Saugata Das, Konstantin Dorfman, Maya Erez, Ulf Hansson

On 17/07/12 15:58, Jaehoon Chung wrote:
> On 07/17/2012 09:30 PM, Adrian Hunter wrote:
>> On 17/07/12 05:44, Jaehoon Chung wrote:
>>
>> <snip>
>>
>>> +/**
>>> + *	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;
>>> +	int timeout;
>>> +	u8 use_busy_signal;
>>> +
>>> +	BUG_ON(!card);
>>> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
>>> +		return;
>>> +
>>> +	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
>>> +		if (card->ext_csd.raw_bkops_status)
>>> +			mmc_card_set_need_bkops(card);
>>> +
>>> +	/*
>>> +	 * 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);
>>> +	if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
>>> +		timeout = MMC_BKOPS_MAX_TIMEOUT;
>>> +		use_busy_signal = 0;
>>> +	} else {
>>> +		timeout = 0;
>>> +		use_busy_signal = 1;
>>> +	}
>>
>> Is this the right way around?
> 
> use the mmc_switch() with R1B..
> this case is no problem, because after sending bkops, repeat the checking card status until prg-done.
> But, at sdhci controller, be occurred data timeout error.
> Because response type is r1b, and timeout value is too large.
> (Actually, i think that is controller's problem..but just its my test environment.)
> 
> if other sdhci controller working well with R1b, use_busy_signal need not.
> Just use the mmc_switch().
> 
> But When running bkops level2/3 use with R1 type, also no problem.
> Because the also checking status in mmc_switch() until prg-done.

You added:

+	/*
+	 * For urgent bkops status (LEVEL_2 and more)
+	 * bkops executed synchronously, otherwise
+	 * the operation is in progress
+	 */
+	if (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2)
+		mmc_card_set_doing_bkops(card);

But:
	status < 2		=> use_busy_signal = 1
	use_busy_signal = 1	=> waiting
	waiting			=> the operation is NOT in progress

Hence my question: Is this the right way around?

Also this change is needed for the addition of 'use_busy_signal' to
mmc_switch:

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index baf90e0..e202a5e 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -417,7 +417,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
                err = mmc_send_status(card, &status);
                if (err)
                        return err;
-               if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
+               if (use_busy_signal && (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY))
                        break;
                if (mmc_host_is_spi(card->host))
                        break;


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

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-17 12:30 ` Adrian Hunter
@ 2012-07-17 12:58   ` Jaehoon Chung
  2012-07-18  6:16     ` Adrian Hunter
  0 siblings, 1 reply; 10+ messages in thread
From: Jaehoon Chung @ 2012-07-17 12:58 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Per FORLIN, Sebastian Rasmussen, Dong,
	Chuanxiao, svenkatr, Saugata Das, Konstantin Dorfman, Maya Erez,
	Ulf Hansson

On 07/17/2012 09:30 PM, Adrian Hunter wrote:
> On 17/07/12 05:44, Jaehoon Chung wrote:
> 
> <snip>
> 
>> +/**
>> + *	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;
>> +	int timeout;
>> +	u8 use_busy_signal;
>> +
>> +	BUG_ON(!card);
>> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
>> +		return;
>> +
>> +	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
>> +		if (card->ext_csd.raw_bkops_status)
>> +			mmc_card_set_need_bkops(card);
>> +
>> +	/*
>> +	 * 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);
>> +	if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
>> +		timeout = MMC_BKOPS_MAX_TIMEOUT;
>> +		use_busy_signal = 0;
>> +	} else {
>> +		timeout = 0;
>> +		use_busy_signal = 1;
>> +	}
> 
> Is this the right way around?

use the mmc_switch() with R1B..
this case is no problem, because after sending bkops, repeat the checking card status until prg-done.
But, at sdhci controller, be occurred data timeout error.
Because response type is r1b, and timeout value is too large.
(Actually, i think that is controller's problem..but just its my test environment.)

if other sdhci controller working well with R1b, use_busy_signal need not.
Just use the mmc_switch().

But When running bkops level2/3 use with R1 type, also no problem.
Because the also checking status in mmc_switch() until prg-done.

Best Regards,
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] 10+ messages in thread

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-17  2:44 Jaehoon Chung
  2012-07-17  9:25 ` S, Venkatraman
@ 2012-07-17 12:30 ` Adrian Hunter
  2012-07-17 12:58   ` Jaehoon Chung
  1 sibling, 1 reply; 10+ messages in thread
From: Adrian Hunter @ 2012-07-17 12:30 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad,
	Per FORLIN, Sebastian Rasmussen, Dong, Chuanxiao, svenkatr,
	Saugata Das, Konstantin Dorfman, Maya Erez, Ulf Hansson

On 17/07/12 05:44, Jaehoon Chung wrote:

<snip>

> +/**
> + *	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;
> +	int timeout;
> +	u8 use_busy_signal;
> +
> +	BUG_ON(!card);
> +	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
> +		return;
> +
> +	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
> +		if (card->ext_csd.raw_bkops_status)
> +			mmc_card_set_need_bkops(card);
> +
> +	/*
> +	 * 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);
> +	if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
> +		timeout = MMC_BKOPS_MAX_TIMEOUT;
> +		use_busy_signal = 0;
> +	} else {
> +		timeout = 0;
> +		use_busy_signal = 1;
> +	}

Is this the right way around?

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

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-17  9:25 ` S, Venkatraman
@ 2012-07-17 10:17   ` Jaehoon Chung
  0 siblings, 0 replies; 10+ messages in thread
From: Jaehoon Chung @ 2012-07-17 10:17 UTC (permalink / raw)
  To: S, Venkatraman
  Cc: Jaehoon Chung, linux-mmc, Chris Ball, Kyungmin Park,
	Hanumath Prasad, Per FORLIN, Sebastian Rasmussen, Dong,
	Chuanxiao, Saugata Das, Konstantin Dorfman, Adrian Hunter,
	Maya Erez, Ulf Hansson

Hi

I will fix your comment and resend the patch.
Thank you.

Best Regards,
Jaehoon Chung

On 07/17/2012 06:25 PM, S, Venkatraman wrote:
> Some minor nits..
> 
> On Tue, Jul 17, 2012 at 8:14 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 BKOPS-STATUS is upper than LEVEL2, need to check until clear
>> the BKOPS-STATUS vaule.
>>
>> If you want to enable this feature, set MMC_CAP2_BKOPS.
>>
>> I tested with dw-mmc and sdhci controller.
>> When repeating the writing 1GB data, at a certain time, performance is decreased.
>> At that time, card is also triggered the Level-3 or Level-2.
>> After running bkops, performance is recovered.
>>
>> Future considerations
>>  * Check BKOPS_LEVEL=1 and start BKOPS in a preventive manner.
>>  * Interrupt ongoing BKOPS before powering off the card.
>>  * How get BKOPS_STATUS value.(periodically send ext_csd command?)
>>  * If use periodic bkops, also consider runtime_pm control.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
>> Signed-off-by: Maya Erez <merez@codeaurora.org>
>> ---
>> Changelog v10:
>>         - Based on latest mmc-next
>>         - Only control the level-2/3.
>>                 : If triggered upper than level2, immediately start bkops.
>>         - Revmoe the BKOPS_CAP2_INIT_BKOPS (move into mmc-util)
>>         - change use_busy_signal instead of waiting_for_prog_done for __mmc_switch
>> Changelog V9:
>>         - Rebased on patch-v7.
>>         - Added Konstantin and Maya's patch
>>                 : mmc:core: Define synchronous BKOPS timeout
>>                 : mmc:core: eMMC4.5 BKOPS fixes
>>         - Removed periodic bkops
>>                 : This feature will do in future work
>>         - Add __mmc_switch() for waiting_for_prod_done.
>>
>> Changelog V8:
>>         - Remove host->lock spin lock reviewed by Adrian
>>         - Support periodic start bkops
>>         - when bkops_status is level-3, if timeout is set to 0, send hpi.
>>         - Move the start-bkops point
>> Changelog V7:
>>         - Use HPI command when issued URGENT_BKOPS
>>         - Recheck until clearing the bkops-status bit.
>> Changelog V6:
>>         - Add the flag of check-bkops-status.
>>           (For fixing async_req problem)
>>         - Add the capability for MMC_CAP2_INIT_BKOPS.
>>           (When unset the bkops_en bit in ext_csd register)
>>         - modify the wrong condition.
>> Changelog V5:
>>         - Rebase based on the latest mmc-next.
>>         - modify codes based-on Chris's comment
>> Changelog V4:
>>         - Add mmc_read_bkops_status
>>         - When URGENT_BKOPS(level2-3), didn't use HPI command.
>>         - In mmc_switch(), use R1B/R1 according to level.
>> Changelog V3:
>>         - move the bkops setting's location in mmc_blk_issue_rw_rq
>>         - modify condition checking
>>         - bkops_en is assigned ext_csd[EXT_CSD_BKOPS_EN] instead of "1"
>>         - remove the unused code
>>         - change pr_debug instead of pr_warn in mmc_send_hpi_cmd
>>         - Add the Future consideration suggested by Per
>> Changelog V2:
>>         - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>         - Add function to check Exception_status(for eMMC4.5)
>> ---
>>  drivers/mmc/core/core.c    |  161 +++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/mmc/core/mmc.c     |   11 +++
>>  drivers/mmc/core/mmc_ops.c |   27 ++++++-
>>  include/linux/mmc/card.h   |   16 +++++
>>  include/linux/mmc/core.h   |    5 ++
>>  include/linux/mmc/host.h   |    2 +-
>>  include/linux/mmc/mmc.h    |   19 +++++-
>>  7 files changed, 233 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 9503cab..52bdd33 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -41,6 +41,12 @@
>>  #include "sd_ops.h"
>>  #include "sdio_ops.h"
>>
>> +/*
>> + * The Background operation can take a long time, depends on the house keeping
>> + * operations the card has to perform
>> + */
>> +#define MMC_BKOPS_MAX_TIMEOUT  (4 * 60 * 1000) /* max time to wait in ms */
>> +
>>  static struct workqueue_struct *workqueue;
>>  static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
>>
>> @@ -245,6 +251,68 @@ 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;
>> +       int timeout;
>> +       u8 use_busy_signal;
>> +
>> +       BUG_ON(!card);
>> +       if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
>> +               return;
>> +
>> +       if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
>> +               if (card->ext_csd.raw_bkops_status)
>> +                       mmc_card_set_need_bkops(card);
>> +
>> +       /*
>> +        * If card is already doing bkops or need for
>> +        * bkops flag is not set, then do nothing just
>> +        * return
>> +        */
> Redundant comments. It is obvious from the code.
> 
>> +       if (mmc_card_doing_bkops(card) || !mmc_card_need_bkops(card))
> This check can be split into two. The mmc_card_doing_bkops() part
> should go above the _is_exception_event() check, and can save an extra
> EXT_CSD read.
> 
> The second part can be folded as an else part of the
> ""
>  +               if (card->ext_csd.raw_bkops_status)
>  +                       mmc_card_set_need_bkops(card);
> ""
> above.. and return.
> 
>> +               return;
>> +
>> +       mmc_claim_host(card->host);
>> +       if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
>> +               timeout = MMC_BKOPS_MAX_TIMEOUT;
>> +               use_busy_signal = 0;
>> +       } else {
>> +               timeout = 0;
>> +               use_busy_signal = 1;
>> +       }
>> +
>> +       err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +                       EXT_CSD_BKOPS_START, 1, timeout, use_busy_signal);
>> +       if (err) {
>> +               pr_warn("%s: error %d starting bkops\n",
>> +                          mmc_hostname(card->host), err);
>> +               mmc_card_clr_need_bkops(card);
>> +               goto out;
>> +       }
>> +
>> +       mmc_card_clr_need_bkops(card);
>> +
>> +       /*
>> +        * For urgent bkops status (LEVEL_2 and more)
>> +        * bkops executed synchronously, otherwise
>> +        * the operation is in progress
>> +        */
>> +       if (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2)
> This line can be replaced with
>  if (use_busy_signal)
> for consistency.
> 
>> +               mmc_card_set_doing_bkops(card);
>> +out:
>> +       mmc_release_host(card->host);
>> +}
>> +EXPORT_SYMBOL(mmc_start_bkops);
>> +
>>  static void mmc_wait_done(struct mmc_request *mrq)
>>  {
>>         complete(&mrq->completion);
>> @@ -354,6 +422,20 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>>         if (host->areq) {
>>                 mmc_wait_for_req_done(host, host->areq->mrq);
>>                 err = host->areq->err_check(host->card, host->areq);
>> +               /*
>> +                * Check BKOPS urgency from each R1 response
> s/from/for
> 
>> +                */
>> +               if (host->card && mmc_card_mmc(host->card) &&
>> +               ((mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1) ||
>> +                (mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1B)) &&
>> +               (host->areq->mrq->cmd->resp[0] & R1_EXP_EVENT))
>> +                       mmc_start_bkops(host->card);
>> +               /*
>> +                * If areq exsit,
> 
> Typo s/exsit/exist
> 
>> +                * we stop the bkops for foreground operation
>> +                */
>> +               if (areq && mmc_card_doing_bkops(host->card))
>> +                       err = mmc_stop_bkops(host->card);
>>         }
>>
>>         if (!err && areq)
>> @@ -488,6 +570,72 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
>>
>>  EXPORT_SYMBOL(mmc_wait_for_cmd);
>>
>> +/*
>> + *     mmc_stop_bkops - stop ongoing BKOPS
>> + *     @card: MMC card to check BKOPS
>> + *
>> + *     Send HPI command to stop 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_stop_bkops(struct mmc_card *card)
>> +{
>> +       int err = 0;
>> +
>> +       BUG_ON(!card);
>> +       err = mmc_interrupt_hpi(card);
>> +
>> +       /*
>> +        * if err is EINVAL, it's status that can't issue HPI.
>> +        * Maybe it should be completed the BKOPS.
>> +        */
>> +       if (!err || (err == -EINVAL)) {
>> +               mmc_card_clr_doing_bkops(card);
>> +               err = 0;
>> +       }
>> +
>> +       return err;
>> +}
>> +EXPORT_SYMBOL(mmc_stop_bkops);
>> +
>> +int mmc_read_bkops_status(struct mmc_card *card)
>> +{
>> +       int err;
>> +       u8 ext_csd[512];
>> +
>> +       mmc_claim_host(card->host);
>> +       err = mmc_send_ext_csd(card, ext_csd);
>> +       mmc_release_host(card->host);
>> +       if (err)
>> +               return err;
>> +
>> +       card->ext_csd.raw_bkops_status = ext_csd[EXT_CSD_BKOPS_STATUS];
>> +       card->ext_csd.raw_exception_status = ext_csd[EXT_CSD_EXP_EVENTS_STATUS];
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL(mmc_read_bkops_status);
>> +
>> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
>> +{
>> +       int err;
>> +
>> +       err = mmc_read_bkops_status(card);
>> +       if (err) {
>> +               pr_err("%s: Didn't read bkops status : %d\n",
>> +                      mmc_hostname(card->host), err);
>> +               return 0;
>> +       }
>> +
>> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
>> +       if (card->ext_csd.rev == 5)
>> +               return 1;
>> +
>> +       return (card->ext_csd.raw_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
>> @@ -2328,8 +2476,14 @@ int mmc_suspend_host(struct mmc_host *host)
>>         mmc_bus_get(host);
>>         if (host->bus_ops && !host->bus_dead) {
>>
>> -               if (host->bus_ops->suspend)
>> +               if (host->bus_ops->suspend) {
>> +                       if (mmc_card_doing_bkops(host->card)) {
>> +                               err = mmc_stop_bkops(host->card);
>> +                               if (err)
>> +                                       goto out;
>> +                       }
>>                         err = host->bus_ops->suspend(host);
>> +               }
>>
>>                 if (err == -ENOSYS || !host->bus_ops->resume) {
>>                         /*
>> @@ -2416,6 +2570,11 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>         switch (mode) {
>>         case PM_HIBERNATION_PREPARE:
>>         case PM_SUSPEND_PREPARE:
>> +               if (host->card && mmc_card_mmc(host->card) &&
>> +                               mmc_card_doing_bkops(host->card)) {
>> +                       mmc_interrupt_hpi(host->card);
> 
> Better to use mmc_stop_bkops() instead of mmc_interrupt_hpi() ?
> 
>> +                       mmc_card_clr_doing_bkops(host->card);
>> +               }
>>
>>                 spin_lock_irqsave(&host->lock, flags);
>>                 host->rescan_disable = 1;
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 7150f02..6fbffb6 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -463,6 +463,17 @@ 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 = ext_csd[EXT_CSD_BKOPS_EN];
>> +                       card->ext_csd.raw_bkops_status =
>> +                               ext_csd[EXT_CSD_BKOPS_STATUS];
>> +                       if (!card->ext_csd.bkops_en)
>> +                               pr_info("%s: Didn't set BKOPS_EN bit!\n",
>> +                                               mmc_hostname(card->host));
>> +               }
>> +
>>                 /* check whether the eMMC card supports HPI */
>>                 if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>>                         card->ext_csd.hpi = 1;
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 4ad994a..baf90e0 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -368,18 +368,19 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
>>  }
>>
>>  /**
>> - *     mmc_switch - modify EXT_CSD register
>> + *     __mmc_switch - modify EXT_CSD register
>>   *     @card: the MMC card associated with the data transfer
>>   *     @set: cmd set values
>>   *     @index: EXT_CSD register index
>>   *     @value: value to program into EXT_CSD register
>>   *     @timeout_ms: timeout (ms) for operation performed by register write,
>>   *                   timeout of zero implies maximum possible timeout
>> + *                   @wait_for_prod_done: is waiting for program done
>>   *
>>   *     Modifies the EXT_CSD register for selected card.
>>   */
>> -int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>> -              unsigned int timeout_ms)
>> +int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>> +              unsigned int timeout_ms, u8 use_busy_signal)
>>  {
>>         int err;
>>         struct mmc_command cmd = {0};
>> @@ -393,13 +394,24 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>>                   (index << 16) |
>>                   (value << 8) |
>>                   set;
>> -       cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
>> +
>> +       cmd.flags = MMC_CMD_AC;
>> +       if (use_busy_signal)
>> +               cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
>> +       else
>> +               cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
>> +
>>         cmd.cmd_timeout_ms = timeout_ms;
>>
>>         err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
>>         if (err)
>>                 return err;
>>
>> +       /* No need to check card status in case of BKOPS LEVEL2 switch*/
>> +       if ((index == EXT_CSD_BKOPS_START) &&
>> +               (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2))
> Please don't mix policy into utility functions. May be replace with
> use_busy_signal check instead.
> 
>> +               return 0;
>> +
>>         /* Must check status to be sure of no errors */
>>         do {
>>                 err = mmc_send_status(card, &status);
>> @@ -424,6 +436,13 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>>
>>         return 0;
>>  }
>> +EXPORT_SYMBOL_GPL(__mmc_switch);
>> +
>> +int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>> +               unsigned int timeout_ms)
>> +{
>> +       return __mmc_switch(card, set, index, value, timeout_ms, 1);
>> +}
>>  EXPORT_SYMBOL_GPL(mmc_switch);
>>
>>  int mmc_send_status(struct mmc_card *card, u32 *status)
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index cdfbc3c..b9e11aa 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -79,10 +79,13 @@ 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 */
>>         unsigned int            data_sector_size;       /* 512 bytes or 4KB */
>>         unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
>>         unsigned int            boot_ro_lock;           /* ro lock support */
>>         bool                    boot_ro_lockable;
>> +       u8                      raw_exception_status;   /* 53 */
>>         u8                      raw_partition_support;  /* 160 */
>>         u8                      raw_erased_mem_count;   /* 181 */
>>         u8                      raw_ext_csd_structure;  /* 194 */
>> @@ -96,6 +99,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;
>> @@ -229,6 +233,9 @@ struct mmc_card {
>>  #define MMC_CARD_REMOVED       (1<<7)          /* card has been removed */
>>  #define MMC_STATE_HIGHSPEED_200        (1<<8)          /* card is in HS200 mode */
>>  #define MMC_STATE_SLEEP                (1<<9)          /* card is in sleep state */
>> +#define MMC_STATE_NEED_BKOPS   (1<<10)         /* card need to do BKOPS */
>> +#define MMC_STATE_DOING_BKOPS  (1<<11)         /* card is doing BKOPS */
>> +#define MMC_STATE_CHECK_BKOPS  (1<<12)         /* card need to check 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 */
>> @@ -400,6 +407,9 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
>>  #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>>  #define mmc_card_removed(c)    ((c) && ((c)->state & MMC_CARD_REMOVED))
>>  #define mmc_card_is_sleep(c)   ((c)->state & MMC_STATE_SLEEP)
>> +#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_check_bkops(c) ((c)->state & MMC_STATE_CHECK_BKOPS)
>>
>>  #define mmc_card_set_present(c)        ((c)->state |= MMC_STATE_PRESENT)
>>  #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
>> @@ -412,7 +422,13 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
>>  #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
>>  #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
>>  #define mmc_card_set_sleep(c)  ((c)->state |= MMC_STATE_SLEEP)
>> +#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_set_check_bkops(c) ((c)->state |= MMC_STATE_CHECK_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)
>> +#define mmc_card_clr_check_bkops(c) ((c)->state &= ~MMC_STATE_CHECK_BKOPS)
>>  #define mmc_card_clr_sleep(c)  ((c)->state &= ~MMC_STATE_SLEEP)
>>  /*
>>   * Quirk add/remove for MMC products.
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index d787037..fa5641d 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -137,6 +137,9 @@ struct mmc_host;
>>  struct mmc_card;
>>  struct mmc_async_req;
>>
>> +extern int mmc_stop_bkops(struct mmc_card *);
>> +extern int mmc_read_bkops_status(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 *);
>> @@ -145,6 +148,8 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int);
>>  extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *);
>>  extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
>>         struct mmc_command *, int);
>> +extern void mmc_start_bkops(struct mmc_card *card);
>> +extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, u8);
>>  extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
>>  extern int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd);
>>
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 79d8921..35b7ce8 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -273,7 +273,7 @@ struct mmc_host {
>>  #define MMC_CAP2_PACKED_WR         (1 << 21)   /* Allow packed write */
>>  #define MMC_CAP2_PACKED_CMD    (MMC_CAP2_PACKED_RD | \
>>                                  MMC_CAP2_PACKED_WR) /* Allow packed commands */
>> -
>> +#define MMC_CAP2_BKOPS         (1 << 22)       /* BKOPS supported */
>>         mmc_pm_flag_t           pm_caps;        /* supported pm features */
>>         unsigned int        power_notify_type;
>>  #define MMC_HOST_PW_NOTIFY_NONE                0
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 254901a..e7396b4 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -285,6 +285,8 @@ struct _mmc_csd {
>>  #define EXT_CSD_PARTITION_SUPPORT      160     /* RO */
>>  #define EXT_CSD_HPI_MGMT               161     /* R/W */
>>  #define EXT_CSD_RST_N_FUNCTION         162     /* R/W */
>> +#define EXT_CSD_BKOPS_EN               163     /* R/W */
>> +#define EXT_CSD_BKOPS_START            164     /* W */
>>  #define EXT_CSD_SANITIZE_START         165     /* W */
>>  #define EXT_CSD_WR_REL_PARAM           166     /* RO */
>>  #define EXT_CSD_BOOT_WP                        173     /* R/W */
>> @@ -318,6 +320,7 @@ 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 */
>> @@ -325,6 +328,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_DATA_TAG_SUPPORT       499     /* RO */
>>  #define EXT_CSD_MAX_PACKED_WRITES      500     /* RO */
>>  #define EXT_CSD_MAX_PACKED_READS       501     /* RO */
>> +#define EXT_CSD_BKOPS_SUPPORT          502     /* RO */
>>  #define EXT_CSD_HPI_FEATURES           503     /* RO */
>>
>>  /*
>> @@ -387,12 +391,23 @@ struct _mmc_csd {
>>
>>  #define EXT_CSD_PACKED_EVENT_EN        (1 << 3)
>>
>> -#define EXT_CSD_PACKED_FAILURE (1 << 3)
>> -
>>  #define EXT_CSD_PACKED_GENERIC_ERROR   (1 << 0)
>>  #define EXT_CSD_PACKED_INDEXED_ERROR   (1 << 1)
>>
>>  /*
>> + * 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)
>> +
>> +/*
>> + * BKOPS status level
>> + */
>> +#define EXT_CSD_BKOPS_LEVEL_2          0x2
>> +
>> +/*
>>   * MMC_SWITCH access modes
>>   */
>>
>> --
>> 1.7.4.1
> --
> 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] 10+ messages in thread

* Re: [PATCH v10] mmc: support BKOPS feature for eMMC
  2012-07-17  2:44 Jaehoon Chung
@ 2012-07-17  9:25 ` S, Venkatraman
  2012-07-17 10:17   ` Jaehoon Chung
  2012-07-17 12:30 ` Adrian Hunter
  1 sibling, 1 reply; 10+ messages in thread
From: S, Venkatraman @ 2012-07-17  9:25 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Chris Ball, Kyungmin Park, Hanumath Prasad,
	Per FORLIN, Sebastian Rasmussen, Dong, Chuanxiao, Saugata Das,
	Konstantin Dorfman, Adrian Hunter, Maya Erez, Ulf Hansson

Some minor nits..

On Tue, Jul 17, 2012 at 8:14 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 BKOPS-STATUS is upper than LEVEL2, need to check until clear
> the BKOPS-STATUS vaule.
>
> If you want to enable this feature, set MMC_CAP2_BKOPS.
>
> I tested with dw-mmc and sdhci controller.
> When repeating the writing 1GB data, at a certain time, performance is decreased.
> At that time, card is also triggered the Level-3 or Level-2.
> After running bkops, performance is recovered.
>
> Future considerations
>  * Check BKOPS_LEVEL=1 and start BKOPS in a preventive manner.
>  * Interrupt ongoing BKOPS before powering off the card.
>  * How get BKOPS_STATUS value.(periodically send ext_csd command?)
>  * If use periodic bkops, also consider runtime_pm control.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
> Signed-off-by: Maya Erez <merez@codeaurora.org>
> ---
> Changelog v10:
>         - Based on latest mmc-next
>         - Only control the level-2/3.
>                 : If triggered upper than level2, immediately start bkops.
>         - Revmoe the BKOPS_CAP2_INIT_BKOPS (move into mmc-util)
>         - change use_busy_signal instead of waiting_for_prog_done for __mmc_switch
> Changelog V9:
>         - Rebased on patch-v7.
>         - Added Konstantin and Maya's patch
>                 : mmc:core: Define synchronous BKOPS timeout
>                 : mmc:core: eMMC4.5 BKOPS fixes
>         - Removed periodic bkops
>                 : This feature will do in future work
>         - Add __mmc_switch() for waiting_for_prod_done.
>
> Changelog V8:
>         - Remove host->lock spin lock reviewed by Adrian
>         - Support periodic start bkops
>         - when bkops_status is level-3, if timeout is set to 0, send hpi.
>         - Move the start-bkops point
> Changelog V7:
>         - Use HPI command when issued URGENT_BKOPS
>         - Recheck until clearing the bkops-status bit.
> Changelog V6:
>         - Add the flag of check-bkops-status.
>           (For fixing async_req problem)
>         - Add the capability for MMC_CAP2_INIT_BKOPS.
>           (When unset the bkops_en bit in ext_csd register)
>         - modify the wrong condition.
> Changelog V5:
>         - Rebase based on the latest mmc-next.
>         - modify codes based-on Chris's comment
> Changelog V4:
>         - Add mmc_read_bkops_status
>         - When URGENT_BKOPS(level2-3), didn't use HPI command.
>         - In mmc_switch(), use R1B/R1 according to level.
> Changelog V3:
>         - move the bkops setting's location in mmc_blk_issue_rw_rq
>         - modify condition checking
>         - bkops_en is assigned ext_csd[EXT_CSD_BKOPS_EN] instead of "1"
>         - remove the unused code
>         - change pr_debug instead of pr_warn in mmc_send_hpi_cmd
>         - Add the Future consideration suggested by Per
> Changelog V2:
>         - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>         - Add function to check Exception_status(for eMMC4.5)
> ---
>  drivers/mmc/core/core.c    |  161 +++++++++++++++++++++++++++++++++++++++++++-
>  drivers/mmc/core/mmc.c     |   11 +++
>  drivers/mmc/core/mmc_ops.c |   27 ++++++-
>  include/linux/mmc/card.h   |   16 +++++
>  include/linux/mmc/core.h   |    5 ++
>  include/linux/mmc/host.h   |    2 +-
>  include/linux/mmc/mmc.h    |   19 +++++-
>  7 files changed, 233 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 9503cab..52bdd33 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -41,6 +41,12 @@
>  #include "sd_ops.h"
>  #include "sdio_ops.h"
>
> +/*
> + * The Background operation can take a long time, depends on the house keeping
> + * operations the card has to perform
> + */
> +#define MMC_BKOPS_MAX_TIMEOUT  (4 * 60 * 1000) /* max time to wait in ms */
> +
>  static struct workqueue_struct *workqueue;
>  static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
>
> @@ -245,6 +251,68 @@ 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;
> +       int timeout;
> +       u8 use_busy_signal;
> +
> +       BUG_ON(!card);
> +       if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
> +               return;
> +
> +       if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
> +               if (card->ext_csd.raw_bkops_status)
> +                       mmc_card_set_need_bkops(card);
> +
> +       /*
> +        * If card is already doing bkops or need for
> +        * bkops flag is not set, then do nothing just
> +        * return
> +        */
Redundant comments. It is obvious from the code.

> +       if (mmc_card_doing_bkops(card) || !mmc_card_need_bkops(card))
This check can be split into two. The mmc_card_doing_bkops() part
should go above the _is_exception_event() check, and can save an extra
EXT_CSD read.

The second part can be folded as an else part of the
""
 +               if (card->ext_csd.raw_bkops_status)
 +                       mmc_card_set_need_bkops(card);
""
above.. and return.

> +               return;
> +
> +       mmc_claim_host(card->host);
> +       if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
> +               timeout = MMC_BKOPS_MAX_TIMEOUT;
> +               use_busy_signal = 0;
> +       } else {
> +               timeout = 0;
> +               use_busy_signal = 1;
> +       }
> +
> +       err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +                       EXT_CSD_BKOPS_START, 1, timeout, use_busy_signal);
> +       if (err) {
> +               pr_warn("%s: error %d starting bkops\n",
> +                          mmc_hostname(card->host), err);
> +               mmc_card_clr_need_bkops(card);
> +               goto out;
> +       }
> +
> +       mmc_card_clr_need_bkops(card);
> +
> +       /*
> +        * For urgent bkops status (LEVEL_2 and more)
> +        * bkops executed synchronously, otherwise
> +        * the operation is in progress
> +        */
> +       if (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2)
This line can be replaced with
 if (use_busy_signal)
for consistency.

> +               mmc_card_set_doing_bkops(card);
> +out:
> +       mmc_release_host(card->host);
> +}
> +EXPORT_SYMBOL(mmc_start_bkops);
> +
>  static void mmc_wait_done(struct mmc_request *mrq)
>  {
>         complete(&mrq->completion);
> @@ -354,6 +422,20 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>         if (host->areq) {
>                 mmc_wait_for_req_done(host, host->areq->mrq);
>                 err = host->areq->err_check(host->card, host->areq);
> +               /*
> +                * Check BKOPS urgency from each R1 response
s/from/for

> +                */
> +               if (host->card && mmc_card_mmc(host->card) &&
> +               ((mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1) ||
> +                (mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1B)) &&
> +               (host->areq->mrq->cmd->resp[0] & R1_EXP_EVENT))
> +                       mmc_start_bkops(host->card);
> +               /*
> +                * If areq exsit,

Typo s/exsit/exist

> +                * we stop the bkops for foreground operation
> +                */
> +               if (areq && mmc_card_doing_bkops(host->card))
> +                       err = mmc_stop_bkops(host->card);
>         }
>
>         if (!err && areq)
> @@ -488,6 +570,72 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
>
>  EXPORT_SYMBOL(mmc_wait_for_cmd);
>
> +/*
> + *     mmc_stop_bkops - stop ongoing BKOPS
> + *     @card: MMC card to check BKOPS
> + *
> + *     Send HPI command to stop 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_stop_bkops(struct mmc_card *card)
> +{
> +       int err = 0;
> +
> +       BUG_ON(!card);
> +       err = mmc_interrupt_hpi(card);
> +
> +       /*
> +        * if err is EINVAL, it's status that can't issue HPI.
> +        * Maybe it should be completed the BKOPS.
> +        */
> +       if (!err || (err == -EINVAL)) {
> +               mmc_card_clr_doing_bkops(card);
> +               err = 0;
> +       }
> +
> +       return err;
> +}
> +EXPORT_SYMBOL(mmc_stop_bkops);
> +
> +int mmc_read_bkops_status(struct mmc_card *card)
> +{
> +       int err;
> +       u8 ext_csd[512];
> +
> +       mmc_claim_host(card->host);
> +       err = mmc_send_ext_csd(card, ext_csd);
> +       mmc_release_host(card->host);
> +       if (err)
> +               return err;
> +
> +       card->ext_csd.raw_bkops_status = ext_csd[EXT_CSD_BKOPS_STATUS];
> +       card->ext_csd.raw_exception_status = ext_csd[EXT_CSD_EXP_EVENTS_STATUS];
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(mmc_read_bkops_status);
> +
> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
> +{
> +       int err;
> +
> +       err = mmc_read_bkops_status(card);
> +       if (err) {
> +               pr_err("%s: Didn't read bkops status : %d\n",
> +                      mmc_hostname(card->host), err);
> +               return 0;
> +       }
> +
> +       /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
> +       if (card->ext_csd.rev == 5)
> +               return 1;
> +
> +       return (card->ext_csd.raw_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
> @@ -2328,8 +2476,14 @@ int mmc_suspend_host(struct mmc_host *host)
>         mmc_bus_get(host);
>         if (host->bus_ops && !host->bus_dead) {
>
> -               if (host->bus_ops->suspend)
> +               if (host->bus_ops->suspend) {
> +                       if (mmc_card_doing_bkops(host->card)) {
> +                               err = mmc_stop_bkops(host->card);
> +                               if (err)
> +                                       goto out;
> +                       }
>                         err = host->bus_ops->suspend(host);
> +               }
>
>                 if (err == -ENOSYS || !host->bus_ops->resume) {
>                         /*
> @@ -2416,6 +2570,11 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>         switch (mode) {
>         case PM_HIBERNATION_PREPARE:
>         case PM_SUSPEND_PREPARE:
> +               if (host->card && mmc_card_mmc(host->card) &&
> +                               mmc_card_doing_bkops(host->card)) {
> +                       mmc_interrupt_hpi(host->card);

Better to use mmc_stop_bkops() instead of mmc_interrupt_hpi() ?

> +                       mmc_card_clr_doing_bkops(host->card);
> +               }
>
>                 spin_lock_irqsave(&host->lock, flags);
>                 host->rescan_disable = 1;
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 7150f02..6fbffb6 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -463,6 +463,17 @@ 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 = ext_csd[EXT_CSD_BKOPS_EN];
> +                       card->ext_csd.raw_bkops_status =
> +                               ext_csd[EXT_CSD_BKOPS_STATUS];
> +                       if (!card->ext_csd.bkops_en)
> +                               pr_info("%s: Didn't set BKOPS_EN bit!\n",
> +                                               mmc_hostname(card->host));
> +               }
> +
>                 /* check whether the eMMC card supports HPI */
>                 if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
>                         card->ext_csd.hpi = 1;
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 4ad994a..baf90e0 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -368,18 +368,19 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
>  }
>
>  /**
> - *     mmc_switch - modify EXT_CSD register
> + *     __mmc_switch - modify EXT_CSD register
>   *     @card: the MMC card associated with the data transfer
>   *     @set: cmd set values
>   *     @index: EXT_CSD register index
>   *     @value: value to program into EXT_CSD register
>   *     @timeout_ms: timeout (ms) for operation performed by register write,
>   *                   timeout of zero implies maximum possible timeout
> + *                   @wait_for_prod_done: is waiting for program done
>   *
>   *     Modifies the EXT_CSD register for selected card.
>   */
> -int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> -              unsigned int timeout_ms)
> +int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> +              unsigned int timeout_ms, u8 use_busy_signal)
>  {
>         int err;
>         struct mmc_command cmd = {0};
> @@ -393,13 +394,24 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>                   (index << 16) |
>                   (value << 8) |
>                   set;
> -       cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
> +
> +       cmd.flags = MMC_CMD_AC;
> +       if (use_busy_signal)
> +               cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
> +       else
> +               cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
> +
>         cmd.cmd_timeout_ms = timeout_ms;
>
>         err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
>         if (err)
>                 return err;
>
> +       /* No need to check card status in case of BKOPS LEVEL2 switch*/
> +       if ((index == EXT_CSD_BKOPS_START) &&
> +               (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2))
Please don't mix policy into utility functions. May be replace with
use_busy_signal check instead.

> +               return 0;
> +
>         /* Must check status to be sure of no errors */
>         do {
>                 err = mmc_send_status(card, &status);
> @@ -424,6 +436,13 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(__mmc_switch);
> +
> +int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> +               unsigned int timeout_ms)
> +{
> +       return __mmc_switch(card, set, index, value, timeout_ms, 1);
> +}
>  EXPORT_SYMBOL_GPL(mmc_switch);
>
>  int mmc_send_status(struct mmc_card *card, u32 *status)
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index cdfbc3c..b9e11aa 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -79,10 +79,13 @@ 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 */
>         unsigned int            data_sector_size;       /* 512 bytes or 4KB */
>         unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
>         unsigned int            boot_ro_lock;           /* ro lock support */
>         bool                    boot_ro_lockable;
> +       u8                      raw_exception_status;   /* 53 */
>         u8                      raw_partition_support;  /* 160 */
>         u8                      raw_erased_mem_count;   /* 181 */
>         u8                      raw_ext_csd_structure;  /* 194 */
> @@ -96,6 +99,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;
> @@ -229,6 +233,9 @@ struct mmc_card {
>  #define MMC_CARD_REMOVED       (1<<7)          /* card has been removed */
>  #define MMC_STATE_HIGHSPEED_200        (1<<8)          /* card is in HS200 mode */
>  #define MMC_STATE_SLEEP                (1<<9)          /* card is in sleep state */
> +#define MMC_STATE_NEED_BKOPS   (1<<10)         /* card need to do BKOPS */
> +#define MMC_STATE_DOING_BKOPS  (1<<11)         /* card is doing BKOPS */
> +#define MMC_STATE_CHECK_BKOPS  (1<<12)         /* card need to check 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 */
> @@ -400,6 +407,9 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
>  #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>  #define mmc_card_removed(c)    ((c) && ((c)->state & MMC_CARD_REMOVED))
>  #define mmc_card_is_sleep(c)   ((c)->state & MMC_STATE_SLEEP)
> +#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_check_bkops(c) ((c)->state & MMC_STATE_CHECK_BKOPS)
>
>  #define mmc_card_set_present(c)        ((c)->state |= MMC_STATE_PRESENT)
>  #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
> @@ -412,7 +422,13 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
>  #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
>  #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
>  #define mmc_card_set_sleep(c)  ((c)->state |= MMC_STATE_SLEEP)
> +#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_set_check_bkops(c) ((c)->state |= MMC_STATE_CHECK_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)
> +#define mmc_card_clr_check_bkops(c) ((c)->state &= ~MMC_STATE_CHECK_BKOPS)
>  #define mmc_card_clr_sleep(c)  ((c)->state &= ~MMC_STATE_SLEEP)
>  /*
>   * Quirk add/remove for MMC products.
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index d787037..fa5641d 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -137,6 +137,9 @@ struct mmc_host;
>  struct mmc_card;
>  struct mmc_async_req;
>
> +extern int mmc_stop_bkops(struct mmc_card *);
> +extern int mmc_read_bkops_status(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 *);
> @@ -145,6 +148,8 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int);
>  extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *);
>  extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
>         struct mmc_command *, int);
> +extern void mmc_start_bkops(struct mmc_card *card);
> +extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, u8);
>  extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
>  extern int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd);
>
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 79d8921..35b7ce8 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -273,7 +273,7 @@ struct mmc_host {
>  #define MMC_CAP2_PACKED_WR         (1 << 21)   /* Allow packed write */
>  #define MMC_CAP2_PACKED_CMD    (MMC_CAP2_PACKED_RD | \
>                                  MMC_CAP2_PACKED_WR) /* Allow packed commands */
> -
> +#define MMC_CAP2_BKOPS         (1 << 22)       /* BKOPS supported */
>         mmc_pm_flag_t           pm_caps;        /* supported pm features */
>         unsigned int        power_notify_type;
>  #define MMC_HOST_PW_NOTIFY_NONE                0
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 254901a..e7396b4 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -285,6 +285,8 @@ struct _mmc_csd {
>  #define EXT_CSD_PARTITION_SUPPORT      160     /* RO */
>  #define EXT_CSD_HPI_MGMT               161     /* R/W */
>  #define EXT_CSD_RST_N_FUNCTION         162     /* R/W */
> +#define EXT_CSD_BKOPS_EN               163     /* R/W */
> +#define EXT_CSD_BKOPS_START            164     /* W */
>  #define EXT_CSD_SANITIZE_START         165     /* W */
>  #define EXT_CSD_WR_REL_PARAM           166     /* RO */
>  #define EXT_CSD_BOOT_WP                        173     /* R/W */
> @@ -318,6 +320,7 @@ 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 */
> @@ -325,6 +328,7 @@ struct _mmc_csd {
>  #define EXT_CSD_DATA_TAG_SUPPORT       499     /* RO */
>  #define EXT_CSD_MAX_PACKED_WRITES      500     /* RO */
>  #define EXT_CSD_MAX_PACKED_READS       501     /* RO */
> +#define EXT_CSD_BKOPS_SUPPORT          502     /* RO */
>  #define EXT_CSD_HPI_FEATURES           503     /* RO */
>
>  /*
> @@ -387,12 +391,23 @@ struct _mmc_csd {
>
>  #define EXT_CSD_PACKED_EVENT_EN        (1 << 3)
>
> -#define EXT_CSD_PACKED_FAILURE (1 << 3)
> -
>  #define EXT_CSD_PACKED_GENERIC_ERROR   (1 << 0)
>  #define EXT_CSD_PACKED_INDEXED_ERROR   (1 << 1)
>
>  /*
> + * 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)
> +
> +/*
> + * BKOPS status level
> + */
> +#define EXT_CSD_BKOPS_LEVEL_2          0x2
> +
> +/*
>   * MMC_SWITCH access modes
>   */
>
> --
> 1.7.4.1

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

* [PATCH v10] mmc: support BKOPS feature for eMMC
@ 2012-07-17  2:44 Jaehoon Chung
  2012-07-17  9:25 ` S, Venkatraman
  2012-07-17 12:30 ` Adrian Hunter
  0 siblings, 2 replies; 10+ messages in thread
From: Jaehoon Chung @ 2012-07-17  2:44 UTC (permalink / raw)
  To: linux-mmc
  Cc: Chris Ball, Kyungmin Park, Hanumath Prasad, Per FORLIN,
	Sebastian Rasmussen, Dong, Chuanxiao, svenkatr, Saugata Das,
	Konstantin Dorfman, Adrian Hunter, Maya Erez, Ulf Hansson

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 BKOPS-STATUS is upper than LEVEL2, need to check until clear
the BKOPS-STATUS vaule. 

If you want to enable this feature, set MMC_CAP2_BKOPS.

I tested with dw-mmc and sdhci controller.
When repeating the writing 1GB data, at a certain time, performance is decreased.
At that time, card is also triggered the Level-3 or Level-2.
After running bkops, performance is recovered.

Future considerations
 * Check BKOPS_LEVEL=1 and start BKOPS in a preventive manner.
 * Interrupt ongoing BKOPS before powering off the card.
 * How get BKOPS_STATUS value.(periodically send ext_csd command?)
 * If use periodic bkops, also consider runtime_pm control.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
Changelog v10:
	- Based on latest mmc-next
	- Only control the level-2/3.
		: If triggered upper than level2, immediately start bkops.
	- Revmoe the BKOPS_CAP2_INIT_BKOPS (move into mmc-util)
	- change use_busy_signal instead of waiting_for_prog_done for __mmc_switch
Changelog V9:
	- Rebased on patch-v7.
	- Added Konstantin and Maya's patch
		: mmc:core: Define synchronous BKOPS timeout
		: mmc:core: eMMC4.5 BKOPS fixes
	- Removed periodic bkops
		: This feature will do in future work
	- Add __mmc_switch() for waiting_for_prod_done.
	
Changelog V8:
	- Remove host->lock spin lock reviewed by Adrian
	- Support periodic start bkops
	- when bkops_status is level-3, if timeout is set to 0, send hpi.
	- Move the start-bkops point
Changelog V7:
	- Use HPI command when issued URGENT_BKOPS
	- Recheck until clearing the bkops-status bit.
Changelog V6:
	- Add the flag of check-bkops-status.
	  (For fixing async_req problem)
	- Add the capability for MMC_CAP2_INIT_BKOPS.
	  (When unset the bkops_en bit in ext_csd register)
	- modify the wrong condition.
Changelog V5:
	- Rebase based on the latest mmc-next.
	- modify codes based-on Chris's comment
Changelog V4:
	- Add mmc_read_bkops_status
	- When URGENT_BKOPS(level2-3), didn't use HPI command.
	- In mmc_switch(), use R1B/R1 according to level.
Changelog V3:
	- move the bkops setting's location in mmc_blk_issue_rw_rq
	- modify condition checking
	- bkops_en is assigned ext_csd[EXT_CSD_BKOPS_EN] instead of "1"
	- remove the unused code
	- change pr_debug instead of pr_warn in mmc_send_hpi_cmd
	- Add the Future consideration suggested by Per
Changelog V2:
	- Use EXCEPTION_STATUS instead of URGENT_BKOPS
	- Add function to check Exception_status(for eMMC4.5)
---
 drivers/mmc/core/core.c    |  161 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/core/mmc.c     |   11 +++
 drivers/mmc/core/mmc_ops.c |   27 ++++++-
 include/linux/mmc/card.h   |   16 +++++
 include/linux/mmc/core.h   |    5 ++
 include/linux/mmc/host.h   |    2 +-
 include/linux/mmc/mmc.h    |   19 +++++-
 7 files changed, 233 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 9503cab..52bdd33 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -41,6 +41,12 @@
 #include "sd_ops.h"
 #include "sdio_ops.h"
 
+/*
+ * The Background operation can take a long time, depends on the house keeping
+ * operations the card has to perform
+ */
+#define MMC_BKOPS_MAX_TIMEOUT	(4 * 60 * 1000) /* max time to wait in ms */
+
 static struct workqueue_struct *workqueue;
 static const unsigned freqs[] = { 400000, 300000, 200000, 100000 };
 
@@ -245,6 +251,68 @@ 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;
+	int timeout;
+	u8 use_busy_signal;
+
+	BUG_ON(!card);
+	if (!card->ext_csd.bkops_en || !(card->host->caps2 & MMC_CAP2_BKOPS))
+		return;
+
+	if (mmc_is_exception_event(card, EXT_CSD_URGENT_BKOPS))
+		if (card->ext_csd.raw_bkops_status)
+			mmc_card_set_need_bkops(card);
+
+	/*
+	 * 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);
+	if (card->ext_csd.raw_bkops_status >= EXT_CSD_BKOPS_LEVEL_2) {
+		timeout = MMC_BKOPS_MAX_TIMEOUT;
+		use_busy_signal = 0;
+	} else {
+		timeout = 0;
+		use_busy_signal = 1;
+	}
+
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			EXT_CSD_BKOPS_START, 1, timeout, use_busy_signal);
+	if (err) {
+		pr_warn("%s: error %d starting bkops\n",
+			   mmc_hostname(card->host), err);
+		mmc_card_clr_need_bkops(card);
+		goto out;
+	}
+
+	mmc_card_clr_need_bkops(card);
+
+	/*
+	 * For urgent bkops status (LEVEL_2 and more)
+	 * bkops executed synchronously, otherwise
+	 * the operation is in progress
+	 */
+	if (card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2)
+		mmc_card_set_doing_bkops(card);
+out:
+	mmc_release_host(card->host);
+}
+EXPORT_SYMBOL(mmc_start_bkops);
+
 static void mmc_wait_done(struct mmc_request *mrq)
 {
 	complete(&mrq->completion);
@@ -354,6 +422,20 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
 	if (host->areq) {
 		mmc_wait_for_req_done(host, host->areq->mrq);
 		err = host->areq->err_check(host->card, host->areq);
+		/*
+		 * Check BKOPS urgency from each R1 response
+		 */
+		if (host->card && mmc_card_mmc(host->card) &&
+		((mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1) ||
+		 (mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1B)) &&
+		(host->areq->mrq->cmd->resp[0] & R1_EXP_EVENT))
+			mmc_start_bkops(host->card);
+		/*
+		 * If areq exsit,
+		 * we stop the bkops for foreground operation
+		 */
+		if (areq && mmc_card_doing_bkops(host->card))
+			err = mmc_stop_bkops(host->card);
 	}
 
 	if (!err && areq)
@@ -488,6 +570,72 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
 
 EXPORT_SYMBOL(mmc_wait_for_cmd);
 
+/*
+ *	mmc_stop_bkops - stop ongoing BKOPS
+ *	@card: MMC card to check BKOPS
+ *
+ *	Send HPI command to stop 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_stop_bkops(struct mmc_card *card)
+{
+	int err = 0;
+
+	BUG_ON(!card);
+	err = mmc_interrupt_hpi(card);
+
+	/*
+	 * if err is EINVAL, it's status that can't issue HPI.
+	 * Maybe it should be completed the BKOPS.
+	 */
+	if (!err || (err == -EINVAL)) {
+		mmc_card_clr_doing_bkops(card);
+		err = 0;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL(mmc_stop_bkops);
+
+int mmc_read_bkops_status(struct mmc_card *card)
+{
+	int err;
+	u8 ext_csd[512];
+
+	mmc_claim_host(card->host);
+	err = mmc_send_ext_csd(card, ext_csd);
+	mmc_release_host(card->host);
+	if (err)
+		return err;
+
+	card->ext_csd.raw_bkops_status = ext_csd[EXT_CSD_BKOPS_STATUS];
+	card->ext_csd.raw_exception_status = ext_csd[EXT_CSD_EXP_EVENTS_STATUS];
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_read_bkops_status);
+
+int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
+{
+	int err;
+
+	err = mmc_read_bkops_status(card);
+	if (err) {
+		pr_err("%s: Didn't read bkops status : %d\n",
+		       mmc_hostname(card->host), err);
+		return 0;
+	}
+
+	/* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
+	if (card->ext_csd.rev == 5)
+		return 1;
+
+	return (card->ext_csd.raw_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
@@ -2328,8 +2476,14 @@ int mmc_suspend_host(struct mmc_host *host)
 	mmc_bus_get(host);
 	if (host->bus_ops && !host->bus_dead) {
 
-		if (host->bus_ops->suspend)
+		if (host->bus_ops->suspend) {
+			if (mmc_card_doing_bkops(host->card)) {
+				err = mmc_stop_bkops(host->card);
+				if (err)
+					goto out;
+			}
 			err = host->bus_ops->suspend(host);
+		}
 
 		if (err == -ENOSYS || !host->bus_ops->resume) {
 			/*
@@ -2416,6 +2570,11 @@ int mmc_pm_notify(struct notifier_block *notify_block,
 	switch (mode) {
 	case PM_HIBERNATION_PREPARE:
 	case PM_SUSPEND_PREPARE:
+		if (host->card && mmc_card_mmc(host->card) &&
+				mmc_card_doing_bkops(host->card)) {
+			mmc_interrupt_hpi(host->card);
+			mmc_card_clr_doing_bkops(host->card);
+		}
 
 		spin_lock_irqsave(&host->lock, flags);
 		host->rescan_disable = 1;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 7150f02..6fbffb6 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -463,6 +463,17 @@ 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 = ext_csd[EXT_CSD_BKOPS_EN];
+			card->ext_csd.raw_bkops_status =
+				ext_csd[EXT_CSD_BKOPS_STATUS];
+			if (!card->ext_csd.bkops_en)
+				pr_info("%s: Didn't set BKOPS_EN bit!\n",
+						mmc_hostname(card->host));
+		}
+
 		/* check whether the eMMC card supports HPI */
 		if (ext_csd[EXT_CSD_HPI_FEATURES] & 0x1) {
 			card->ext_csd.hpi = 1;
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 4ad994a..baf90e0 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -368,18 +368,19 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
 }
 
 /**
- *	mmc_switch - modify EXT_CSD register
+ *	__mmc_switch - modify EXT_CSD register
  *	@card: the MMC card associated with the data transfer
  *	@set: cmd set values
  *	@index: EXT_CSD register index
  *	@value: value to program into EXT_CSD register
  *	@timeout_ms: timeout (ms) for operation performed by register write,
  *                   timeout of zero implies maximum possible timeout
+ *                   @wait_for_prod_done: is waiting for program done
  *
  *	Modifies the EXT_CSD register for selected card.
  */
-int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
-	       unsigned int timeout_ms)
+int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
+	       unsigned int timeout_ms, u8 use_busy_signal)
 {
 	int err;
 	struct mmc_command cmd = {0};
@@ -393,13 +394,24 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		  (index << 16) |
 		  (value << 8) |
 		  set;
-	cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
+
+	cmd.flags = MMC_CMD_AC;
+	if (use_busy_signal)
+		cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
+	else
+		cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
+
 	cmd.cmd_timeout_ms = timeout_ms;
 
 	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
 	if (err)
 		return err;
 
+	/* No need to check card status in case of BKOPS LEVEL2 switch*/
+	if ((index == EXT_CSD_BKOPS_START) &&
+		(card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2))
+		return 0;
+
 	/* Must check status to be sure of no errors */
 	do {
 		err = mmc_send_status(card, &status);
@@ -424,6 +436,13 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(__mmc_switch);
+
+int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
+		unsigned int timeout_ms)
+{
+	return __mmc_switch(card, set, index, value, timeout_ms, 1);
+}
 EXPORT_SYMBOL_GPL(mmc_switch);
 
 int mmc_send_status(struct mmc_card *card, u32 *status)
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index cdfbc3c..b9e11aa 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -79,10 +79,13 @@ 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 */
 	unsigned int            data_sector_size;       /* 512 bytes or 4KB */
 	unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
 	unsigned int		boot_ro_lock;		/* ro lock support */
 	bool			boot_ro_lockable;
+	u8			raw_exception_status;	/* 53 */
 	u8			raw_partition_support;	/* 160 */
 	u8			raw_erased_mem_count;	/* 181 */
 	u8			raw_ext_csd_structure;	/* 194 */
@@ -96,6 +99,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;
@@ -229,6 +233,9 @@ struct mmc_card {
 #define MMC_CARD_REMOVED	(1<<7)		/* card has been removed */
 #define MMC_STATE_HIGHSPEED_200	(1<<8)		/* card is in HS200 mode */
 #define MMC_STATE_SLEEP		(1<<9)		/* card is in sleep state */
+#define MMC_STATE_NEED_BKOPS	(1<<10)		/* card need to do BKOPS */
+#define MMC_STATE_DOING_BKOPS	(1<<11)		/* card is doing BKOPS */
+#define MMC_STATE_CHECK_BKOPS	(1<<12)		/* card need to check 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 */
@@ -400,6 +407,9 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
 #define mmc_card_removed(c)	((c) && ((c)->state & MMC_CARD_REMOVED))
 #define mmc_card_is_sleep(c)	((c)->state & MMC_STATE_SLEEP)
+#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_check_bkops(c) ((c)->state & MMC_STATE_CHECK_BKOPS)
 
 #define mmc_card_set_present(c)	((c)->state |= MMC_STATE_PRESENT)
 #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
@@ -412,7 +422,13 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
 #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
 #define mmc_card_set_sleep(c)	((c)->state |= MMC_STATE_SLEEP)
+#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_set_check_bkops(c) ((c)->state |= MMC_STATE_CHECK_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)
+#define mmc_card_clr_check_bkops(c) ((c)->state &= ~MMC_STATE_CHECK_BKOPS)
 #define mmc_card_clr_sleep(c)	((c)->state &= ~MMC_STATE_SLEEP)
 /*
  * Quirk add/remove for MMC products.
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index d787037..fa5641d 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -137,6 +137,9 @@ struct mmc_host;
 struct mmc_card;
 struct mmc_async_req;
 
+extern int mmc_stop_bkops(struct mmc_card *);
+extern int mmc_read_bkops_status(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 *);
@@ -145,6 +148,8 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int);
 extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *);
 extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
 	struct mmc_command *, int);
+extern void mmc_start_bkops(struct mmc_card *card);
+extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, u8);
 extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
 extern int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd);
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 79d8921..35b7ce8 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -273,7 +273,7 @@ struct mmc_host {
 #define MMC_CAP2_PACKED_WR	    (1 << 21)	/* Allow packed write */
 #define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \
 				 MMC_CAP2_PACKED_WR) /* Allow packed commands */
-
+#define MMC_CAP2_BKOPS		(1 << 22)	/* BKOPS supported */
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
 #define MMC_HOST_PW_NOTIFY_NONE		0
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 254901a..e7396b4 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -285,6 +285,8 @@ struct _mmc_csd {
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
 #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
+#define EXT_CSD_BKOPS_EN		163	/* R/W */
+#define EXT_CSD_BKOPS_START		164	/* W */
 #define EXT_CSD_SANITIZE_START		165     /* W */
 #define EXT_CSD_WR_REL_PARAM		166	/* RO */
 #define EXT_CSD_BOOT_WP			173	/* R/W */
@@ -318,6 +320,7 @@ 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 */
@@ -325,6 +328,7 @@ struct _mmc_csd {
 #define EXT_CSD_DATA_TAG_SUPPORT	499	/* RO */
 #define EXT_CSD_MAX_PACKED_WRITES	500	/* RO */
 #define EXT_CSD_MAX_PACKED_READS	501	/* RO */
+#define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
 #define EXT_CSD_HPI_FEATURES		503	/* RO */
 
 /*
@@ -387,12 +391,23 @@ struct _mmc_csd {
 
 #define EXT_CSD_PACKED_EVENT_EN	(1 << 3)
 
-#define EXT_CSD_PACKED_FAILURE	(1 << 3)
-
 #define EXT_CSD_PACKED_GENERIC_ERROR	(1 << 0)
 #define EXT_CSD_PACKED_INDEXED_ERROR	(1 << 1)
 
 /*
+ * 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)
+
+/*
+ * BKOPS status level
+ */
+#define EXT_CSD_BKOPS_LEVEL_2		0x2
+
+/*
  * MMC_SWITCH access modes
  */
 
-- 
1.7.4.1

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

end of thread, other threads:[~2012-07-18 17:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 19:34 [PATCH v10] mmc: support BKOPS feature for eMMC merez
2012-07-18  6:42 ` Jaehoon Chung
2012-07-18 17:13   ` merez
  -- strict thread matches above, loose matches on Subject: below --
2012-07-17  2:44 Jaehoon Chung
2012-07-17  9:25 ` S, Venkatraman
2012-07-17 10:17   ` Jaehoon Chung
2012-07-17 12:30 ` Adrian Hunter
2012-07-17 12:58   ` Jaehoon Chung
2012-07-18  6:16     ` Adrian Hunter
2012-07-18  6:31       ` merez

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.