On 07.09.2010 15:57, Wolfgang Denk wrote:
> 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.
Besides that my remarks to yesterdays patch of yours are still valid:
Those "magic numbers" are due to a specific hardware controller/limitation
and not any SD/MMC card limitation.
And which hardware driver are you using this with?
Maybe some context would help.
Maybe the splitting could be done in the hardware driver and not in this
generic part?
Reinhard
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
