[U-Boot] [PATCH] mmc and fat bug fixes

[Ruud Commandeur]

> -----Oorspronkelijk bericht-----
> Van: Andy Fleming [mailto:afleming at 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 
> <RCommandeur at 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
>