Dear Lei Wen,

In message <1283862729-17045-1-git-send-email-lei...@marvell.com> you wrote:
> Signed-off-by: Lei Wen <lei...@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: w...@denx.de
Commitment, n.:      Commitment can be illustrated by a breakfast
of ham and eggs. The chicken was involved, the pig was committed.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to