[U-Boot,3/7] mmc: Enhance erase handling procedure

Erass sequence:
1. check if erase command is support by card. If not return.
2. Check the erase range to see if it was aligned. The min erase size
should be one erase group. SD card it was one block(512), mmc card
it should be one erase group.
3. If not, aligned the erase rang according to the erase group size.
4. Send erase command to erase card once one erase group.
5. If it was SD card, erase with arg 0x00000000 should be send.
   else if support secure feature, erase with arg 0x80000000(Spec eMMC 4.41).
   else erase with arg 0x00000000.(Trim and discard is ingnored here)
6. Check card status after erase.

Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
---
 drivers/mmc/mmc_write.c | 70 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 10 deletions(-)

Hi Haijun,

On Nov 5, 2013, at 8:23 AM, Haijun Zhang wrote:

> Erass sequence:
> 1. check if erase command is support by card. If not return.
> 2. Check the erase range to see if it was aligned. The min erase size
> should be one erase group. SD card it was one block(512), mmc card
> it should be one erase group.
> 3. If not, aligned the erase rang according to the erase group size.
> 4. Send erase command to erase card once one erase group.
> 5. If it was SD card, erase with arg 0x00000000 should be send.
>   else if support secure feature, erase with arg 0x80000000(Spec eMMC 4.41).
>   else erase with arg 0x00000000.(Trim and discard is ingnored here)
> 6. Check card status after erase.
> 

Can I get a few lines about what this patch implements before explaining
the algorithm?

Something like

	"This patch enhances the currently implemented erase procedure in u-boot,
	which has the following problems/missing features..."

> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> ---
> drivers/mmc/mmc_write.c | 70 ++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> index aa2fdef..f2e9baf 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -17,6 +17,10 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> 	struct mmc_cmd cmd;
> 	ulong end;
> 	int err, start_cmd, end_cmd;
> +	uint arg = 0;
> +
> +	if (!(mmc->cmdclass & CCC_ERASE))
> +		return NOT_SUPPORT;
> 
> 	if (mmc->high_capacity) {
> 		end = start + blkcnt - 1;
> @@ -48,8 +52,17 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> 	if (err)
> 		goto err_out;
> 
> +	arg = MMC_ERASE_ARG;
> +
> +	if (mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)
> +		arg = MMC_SECURE_ERASE_ARG;
> +
> +	/* SD card only support %MMC_ERASE_ARG */
> +	if (IS_SD(mmc))
> +		arg = MMC_ERASE_ARG;
> +

Shouldn't that be:

	/* SD card only support %MMC_ERASE_ARG */
	if (!IS_SD(mmc) && 
		(mmc->sec_feature_support & EXT_CSD_SEC_ER_EN))
		arg = MMC_SECURE_ERASE_ARG;
	else
		arg = MMC_ERASE_ARG;

> 	cmd.cmdidx = MMC_CMD_ERASE;
> -	cmd.cmdarg = SECURE_ERASE;
> +	cmd.cmdarg = arg;
> 	cmd.resp_type = MMC_RSP_R1b;
> 
> 	err = mmc_send_cmd(mmc, &cmd, NULL);
> @@ -69,24 +82,61 @@ unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> 	struct mmc *mmc = find_mmc_device(dev_num);
> 	lbaint_t blk = 0, blk_r = 0;
> 	int timeout = 1000;
> +	int res;
> +	bool align = false;
> 
> 	if (!mmc)
> 		return -1;
> 
> +	if (!(mmc->cmdclass & CCC_ERASE)) {
> +		printf("\nErase operation is not support by card\n");
> +		return NOT_SUPPORT;
> +	}
> +
> 	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
> +		align = true;
> +

^ The use of align here is confusing.

AFAIKT the meaning is reversed.

align actually means not_aligned.
 
> +	res = start % mmc->erase_grp_size;
> +	if (res) {
> +		res = mmc->erase_grp_size - res;
> +		start += res;
> +		if (blkcnt > res)
> +			blkcnt -= res;
> +		else {
> +			printf("\nErase size smaller than Erase group "
> +				"size [0x%x] is not support by the device.\n",
> +				mmc->erase_grp_size);
> +			return NOT_SUPPORT;
> +		}
> +	}
> +
> +	res = blkcnt % mmc->erase_grp_size;
> +	if (res)
> +		blkcnt -= res;
> +
> +	if (!blkcnt) {
> +		printf("\nErase size smaller than Erase group size [0x%x] "
> +			"is not support by the device.\n",
> +			mmc->erase_grp_size);
> +		return NOT_SUPPORT;
> +	}
> +
> +	if (align)
> 		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
> -		       "The erase range would be change to "
> -		       "0x" LBAF "~0x" LBAF "\n\n",
> -		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
> -		       ((start + blkcnt + mmc->erase_grp_size)
> -		       & ~(mmc->erase_grp_size - 1)) - 1);
> +			"The erase range would be change to "
> +			"0x" LBAF "~0x" LBAF "\n\n",
> +			mmc->erase_grp_size, start, start + blkcnt);
> +
> 
> 	while (blk < blkcnt) {
> -		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
> -			mmc->erase_grp_size : (blkcnt - blk);
> +		if ((blkcnt - blk) >= mmc->erase_grp_size)
> +			blk_r = mmc->erase_grp_size;
> 		err = mmc_erase_t(mmc, start + blk, blk_r);
> -		if (err)
> -			break;
> +		if (err) {
> +			printf("\nErase range from 0x" LBAF "~0x" LBAF
> +				" Failed\n", start + blk, blkcnt);
> +			return COMM_ERR;
> +		}
> 
> 		blk += blk_r;
> 
> -- 
> 1.8.4
> 
> 

Regards

-- Pantelis