[U-Boot] mmc: add bkops-enable command

[Jaehoon Chung]

On 11/16/2016 10:12 PM, Tomas Melin wrote:
> Hi,
> 
> On 11/16/2016 02:05 PM, Jaehoon Chung wrote:
>>
>> Why needs to set bkops on bootloader? Is there special reason?
>> And Linux kernel has already discussed about this.
>>
> It is beneficial to be able to do all required eMMC settings without being dependent on Linux booting.
> It saves time in production to do initial eMMC setup directly from bootloader.
> 
>> I don't want to provide this command on u-boot side.
>> Don't handle Onetime programmable register on u-boot. So NACK.
> 
> U-Boot already provides One-time programmable commands such as hwpartition and rst-function.
> This adds to that same palette of commands that are needed when properly configuring an eMMC device. 

hwpartition and rst-function didn't affect the performance. And it's not critical thing.
But BKOPS needs to consider too many things. 
Just even set to enable bit..it's not working.

Did you check the periodic bkops and auto bkops? And is it working correct?
How to control suspend and Idle state? LEVEL2/3?

When i had tested the bkops, it has affected IO performance...Of course, it should be maintained average.
Because it should be doing GC with bkops. That is not reason that has to enable on bootloader.

I will not apply on u-boot-mmc for bkops. There is no benefit.

If you set to enable bkops, then you did to pass the other controlling responsibility to Kernel. 

Best Regards,
Jaehoon Chung

> 
> One possible option going forward could be putting all eMMC-configure related commands behind a common CONFIG_ option.
> 
> BR,
> Tomas
> 
> 
>>
>>>
>>> Signed-off-by: Tomas Melin <tomas.melin at vaisala.com>
>>> ---
>>>  cmd/mmc.c         | 26 ++++++++++++++++++++++++++
>>>  drivers/mmc/mmc.c | 30 ++++++++++++++++++++++++++++++
>>>  include/mmc.h     |  4 ++++
>>>  3 files changed, 60 insertions(+)
>>>
>>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>>> index b2761e9..3ae9682 100644
>>> --- a/cmd/mmc.c
>>> +++ b/cmd/mmc.c
>>> @@ -729,6 +729,29 @@ static int do_mmc_setdsr(cmd_tbl_t *cmdtp, int flag,
>>>  	return ret;
>>>  }
>>>  
>>> +static int do_mmc_bkops_enable(cmd_tbl_t *cmdtp, int flag,
>>> +				   int argc, char * const argv[])
>>> +{
>>> +	int dev;
>>> +	struct mmc *mmc;
>>> +
>>> +	if (argc != 2)
>>> +		return CMD_RET_USAGE;
>>> +
>>> +	dev = simple_strtoul(argv[1], NULL, 10);
>>> +
>>> +	mmc = init_mmc_device(dev, false);
>>> +	if (!mmc)
>>> +		return CMD_RET_FAILURE;
>>> +
>>> +	if (IS_SD(mmc)) {
>>> +		puts("BKOPS_EN only exists on eMMC\n");
>>> +		return CMD_RET_FAILURE;
>>> +	}
>>> +
>>> +	return mmc_set_bkops_enable(mmc);
>>> +}
>>> +
>>>  static cmd_tbl_t cmd_mmc[] = {
>>>  	U_BOOT_CMD_MKENT(info, 1, 0, do_mmcinfo, "", ""),
>>>  	U_BOOT_CMD_MKENT(read, 4, 1, do_mmc_read, "", ""),
>>> @@ -749,6 +772,7 @@ static cmd_tbl_t cmd_mmc[] = {
>>>  	U_BOOT_CMD_MKENT(rpmb, CONFIG_SYS_MAXARGS, 1, do_mmcrpmb, "", ""),
>>>  #endif
>>>  	U_BOOT_CMD_MKENT(setdsr, 2, 0, do_mmc_setdsr, "", ""),
>>> +	U_BOOT_CMD_MKENT(bkops-enable, 2, 0, do_mmc_bkops_enable, "", ""),
>>>  };
>>>  
>>>  static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> @@ -813,6 +837,8 @@ U_BOOT_CMD(
>>>  	"mmc rpmb counter - read the value of the write counter\n"
>>>  #endif
>>>  	"mmc setdsr <value> - set DSR register value\n"
>>> +	"mmc bkops-enable <dev> - enable background operations handshake on device\n"
>>> +	"   WARNING: This is a write-once setting.\n"
>>>  	);
>>>  
>>>  /* Old command kept for compatibility. Same as 'mmc info' */
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 0cec02c..d6f40cc 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -1810,3 +1810,33 @@ int mmc_initialize(bd_t *bis)
>>>  	mmc_do_preinit();
>>>  	return 0;
>>>  }
>>> +
>>> +int mmc_set_bkops_enable(struct mmc *mmc)
>>> +{
>>> +	int err;
>>> +	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
>>> +
>>> +	err = mmc_send_ext_csd(mmc, ext_csd);
>>> +	if (err) {
>>> +		puts("Could not get ext_csd register values\n");
>>> +		return err;
>>> +	}
>>> +
>>> +	if (!(ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)) {
>>> +		puts("Background operations not supported on device\n");
>>> +		return -EMEDIUMTYPE;
>>> +	}
>>> +
>>> +	if (ext_csd[EXT_CSD_BKOPS_EN] & 0x1) {
>>> +		puts("Background operations already enabled\n");
>>> +		return 0;
>>> +	}
>>> +
>>> +	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BKOPS_EN, 1);
>>> +	if (err) {
>>> +		puts("Failed to enable background operations\n");
>>> +		return err;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> diff --git a/include/mmc.h b/include/mmc.h
>>> index 5ef37d3..0772d53 100644
>>> --- a/include/mmc.h
>>> +++ b/include/mmc.h
>>> @@ -175,6 +175,7 @@
>>>  #define EXT_CSD_MAX_ENH_SIZE_MULT	157	/* R */
>>>  #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
>>>  #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
>>> +#define EXT_CSD_BKOPS_EN		163	/* R/W */
>>>  #define EXT_CSD_WR_REL_PARAM		166	/* R */
>>>  #define EXT_CSD_WR_REL_SET		167	/* R/W */
>>>  #define EXT_CSD_RPMB_MULT		168	/* RO */
>>> @@ -189,6 +190,7 @@
>>>  #define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
>>>  #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
>>>  #define EXT_CSD_BOOT_MULT		226	/* RO */
>>> +#define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
>>>  
>>>  /*
>>>   * EXT_CSD field definitions
>>> @@ -541,6 +543,8 @@ int mmc_rpmb_read(struct mmc *mmc, void *addr, unsigned short blk,
>>>  		  unsigned short cnt, unsigned char *key);
>>>  int mmc_rpmb_write(struct mmc *mmc, void *addr, unsigned short blk,
>>>  		   unsigned short cnt, unsigned char *key);
>>> +int mmc_set_bkops_enable(struct mmc *mmc);
>>> +
>>>  /**
>>>   * Start device initialization and return immediately; it does not block on
>>>   * polling OCR (operation condition register) status.  Then you should call
>>>
>>>