[U-Boot] [PATCH 1/3] mmc: limit block count for multi-block write commands

Wolfgang Denk wd at denx.de
Tue Sep 7 15:57:58 CEST 2010


Dear Lei Wen,

In message <1283862729-17045-1-git-send-email-leiwen at marvell.com> you wrote:
> Signed-off-by: Lei Wen <leiwen at marvell.com>
> ---
>  drivers/mmc/mmc.c |   59 ++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 38 insertions(+), 21 deletions(-)

> +	BUG_ON(blklen * blocknum > 524288);
> +	BUG_ON(blocknum > 65535);

Please add a comment that explains where these magic numbers are
coming from.

And are you sure that these are conditions where the system should
hang? I think this is not an approrpiate way to handle such errors.
Just failing the command should be enough.

> +	if (blocknum > 1)

I already complained that I consider "blocknum" to be inappropriately
named. It is not a block number (= number of a block), but a block
count.

> -	data.blocks = blkcnt;
> +	data.blocks = blocknum;

The previously used name was way better!

>  	data.blocksize = blklen;
>  	data.flags = MMC_DATA_WRITE;
>  
> @@ -124,7 +109,7 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
>  		return err;
>  	}
>  
> -	if (blkcnt > 1) {
> +	if (blocknum > 1) {
>  		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
>  		cmd.cmdarg = 0;
>  		cmd.resp_type = MMC_RSP_R1b;
> @@ -132,6 +117,38 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
>  		stoperr = mmc_send_cmd(mmc, &cmd, NULL);
>  	}
> +	max = 524288 / mmc->write_bl_len;

Please explain where these magic number is coming from. Eventually use
#defines.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Commitment, n.:      Commitment can be illustrated by a breakfast
of ham and eggs. The chicken was involved, the pig was committed.


More information about the U-Boot mailing list