> -----Oorspronkelijk bericht-----
> Van: Andy Fleming [mailto:aflem...@gmail.com]
> Verzonden: donderdag 16 mei 2013 0:15
> Aan: Ruud Commandeur
> CC: U-Boot list; Tom Rini; Mats K?rrman
> Onderwerp: Re: [U-Boot] [PATCH] mmc and fat bug fixes
>
>
>
>
> On Wed, May 15, 2013 at 9:23 AM, Ruud Commandeur
> <rcommand...@clb.nl> wrote:
>
>
> This patch fixes a number of mmc and fat-related bugs:
>
> > Added a check for blkcnt > 0 in mmc_write_blocks
> (drivers/mmc.c) to prevent a hangup for further mmc commands.
>
>
>
>
> You need more information than that. Why is some code
> requesting 0-byte data commands?
>
I have discussed this issue in an earlier thread. But I agree that it
would make
sense to add some of this earlier comments here.
> As others mentioned, you need to break up patches so each
> change is one patch.
>
Yep :-)
>
>
> Index: drivers/mmc/mmc.c
>
> ===================================================================
> --- drivers/mmc/mmc.c (revision 9)
> +++ drivers/mmc/mmc.c (working copy)
> @@ -282,8 +282,9 @@
>
> if (blkcnt > 1)
> cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
> - else
> + else if (blkcnt > 0)
> cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
> + else return 0; //Called with blkcnt = 0
>
>
>
>
> Assuming this is necessary, I think it then might be time to
> reorder this:
>
> if (!blkcnt) <-- possibly at the very start of the function.
> return 0;
>
> if (blkcnt == 1)
> cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
> else
> cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
>
>
> While technically correct, checking >1, then >0 creates an
> odd dissonance in my mind, and makes me have to think about
> when that if clause will evaluate to true, and I hate having
> to think. :)
>
You're right. That was the reason for adding my (wrong styled) comments.
So I can either reorder his to:
if (blkcnt == 0)
return 0;
else if (blkcnt == 1)
cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
else
cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
Or add the test for 0 at the very beginning as you suggested.
Any preference?
> Andy
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
