Lpc2000

Perhaps someone who is better versed in MMC SPI stuff should take a good 
look at Philips AN10406.  There appears to be some problems with the 
example code it contains.  Two issues jump right out in the file: 
spi_sd.c...

In mmc_init(), look at the _second_ for() loop:
======== begin mmc_init() ==========
int mmc_init( void )
{
   DWORD i;
   /* Write data pattern for testing */
   for(i=0;i<MMC_DATA_SIZE;i++) {
      MMCWRData[i] = i;
   }
   SDStatus = 0;
#if !USE_CS
   IOCLR0 = 0x00100000; /* clear SPI SSEL */
#endif
   /* Initializes the card into SPI mode by sending 80 clocks */
   for(i=0; i<10; i++) {
      MMCCmd[i] = 0xFF;
   }
   SPI_Send( MMCCmd, 10 );
======== snip =================

The sizeof(MMCCmd) is only 8 bytes, not 10.

While that is not too bad, it gets more interesting as you read the 
mmc_block_write().  During mmc_init(), CMD16 has set the blocksize to 
0x200 (512 bytes), okay?  But, when they write the data during 
mmc_block_write(), they only write 8 bytes.  Then they proceed to hold 
the chipselect down and poll 255 times for a completion status code!  Of 
course, it dies, while the MMC gets deadlocked waiting for the rest of 
the data...

Another issue is that the mmc_write_block() does not look at BUSY from 
the MMC, but assumes a delay loop is an answer to when a write has 
completed!!!

With my understanding of MMC and the reading of the code in AN10406, it 
looks as if mmc_block_write() is never going to write a block to the 
MMC?  After compiling and debugging the code for a few hours, I'm 
satisfied that there are problems with it (appnote code).

The night was not a total loss, working with the app note had made me go 
back and take a critical look at the MMC spec + my code.  Heh, my code 
is working flawlessly now!

IMHO, I strongly urge Philips to remove AN10406 from their website until 
a code review has been performed.


Regards,

TomW




-- 
Tom Walsh - WN3L - Embedded Systems Consultant
http://openhardware.net, http://cyberiansoftware.com
"Windows? No thanks, I have work to do..."
----------------------------------------------------

Tom,
    Thanks for the comments. We are going to review it ASAP.
There are some issues in the sample source for sure, as you
pointed out in 80 clocks in mmc_init(). But, in mmc_write_block(), 
the first 8 bytes is not the actual data, but some dummy 
clocks before sending the actual command, followed by 512 bytes
of data. 
 
Regardless, I am going to look into that again and give you
all a satisfying answer.

Tom
Philips Apps.


--- In lpc2000@yahoogroups.com, Tom Walsh <tom@o...> wrote:
>
> Perhaps someone who is better versed in MMC SPI stuff should take 
a good 
> look at Philips AN10406.  There appears to be some problems with 
the 
> example code it contains.  Two issues jump right out in the file: 
> spi_sd.c...
> 
> In mmc_init(), look at the _second_ for() loop:
> ======== begin mmc_init() ==========
> int mmc_init( void )
> {
>    DWORD i;
>    /* Write data pattern for testing */
>    for(i=0;i<MMC_DATA_SIZE;i++) {
>       MMCWRData[i] = i;
>    }
>    SDStatus = 0;
> #if !USE_CS
>    IOCLR0 = 0x00100000; /* clear SPI SSEL */
> #endif
>    /* Initializes the card into SPI mode by sending 80 clocks */
>    for(i=0; i<10; i++) {
>       MMCCmd[i] = 0xFF;
>    }
>    SPI_Send( MMCCmd, 10 );
> ======== snip =================
> 
> The sizeof(MMCCmd) is only 8 bytes, not 10.
> 
> While that is not too bad, it gets more interesting as you read 
the 
> mmc_block_write().  During mmc_init(), CMD16 has set the blocksize 
to 
> 0x200 (512 bytes), okay?  But, when they write the data during 
> mmc_block_write(), they only write 8 bytes.  Then they proceed to 
hold 
> the chipselect down and poll 255 times for a completion status 
code!  Of 
> course, it dies, while the MMC gets deadlocked waiting for the 
rest of 
> the data...
> 
> Another issue is that the mmc_write_block() does not look at BUSY 
from 
> the MMC, but assumes a delay loop is an answer to when a write has 
> completed!!!
> 
> With my understanding of MMC and the reading of the code in 
AN10406, it 
> looks as if mmc_block_write() is never going to write a block to 
the 
> MMC?  After compiling and debugging the code for a few hours, I'm 
> satisfied that there are problems with it (appnote code).
> 
> The night was not a total loss, working with the app note had made 
me go 
> back and take a critical look at the MMC spec + my code.  Heh, my 
code 
> is working flawlessly now!
> 
> IMHO, I strongly urge Philips to remove AN10406 from their website 
until 

> a code review has been performed.
> 
> 
> Regards,
> 
> TomW
> 
> 
> 
> 
> -- 
> Tom Walsh - WN3L - Embedded Systems Consultant
> http://openhardware.net, http://cyberiansoftware.com
> "Windows? No thanks, I have work to do..."
> ----------------------------------------------------
>

philips_apps wrote:

>Tom,
>    Thanks for the comments. We are going to review it ASAP.
>There are some issues in the sample source for sure, as you
>pointed out in 80 clocks in mmc_init(). But, in mmc_write_block(), 
>the first 8 bytes is not the actual data, but some dummy 
>clocks before sending the actual command, followed by 512 bytes
>of data. 
> 
>Regardless, I am going to look into that again and give you
>all a satisfying answer.
>
>  
>
It may have been a simple mistake on the part of the programmer that 
they submitted some early test code for the appnote (experimental).  
They may have not submitted the _real_, final, code.

Anyway, thank you for looking at it.

Regards,

TomW




>Tom
>Philips Apps.
>
>
>--- In lpc2000@yahoogroups.com, Tom Walsh <tom@o...> wrote:
>  
>
>>Perhaps someone who is better versed in MMC SPI stuff should take 
>>    
>>
>a good 
>  
>
>>look at Philips AN10406.  There appears to be some problems with 
>>    
>>
>the 
>  
>
>>example code it contains.  Two issues jump right out in the file: 
>>spi_sd.c...
>>
>>In mmc_init(), look at the _second_ for() loop:
>>======== begin mmc_init() ==========
>>int mmc_init( void )
>>{
>>   DWORD i;
>>   /* Write data pattern for testing */
>>   for(i=0;i<MMC_DATA_SIZE;i++) {
>>      MMCWRData[i] = i;
>>   }
>>   SDStatus = 0;
>>#if !USE_CS
>>   IOCLR0 = 0x00100000; /* clear SPI SSEL */
>>#endif
>>   /* Initializes the card into SPI mode by sending 80 clocks */
>>   for(i=0; i<10; i++) {
>>      MMCCmd[i] = 0xFF;
>>   }
>>   SPI_Send( MMCCmd, 10 );
>>======== snip =================
>>
>>The sizeof(MMCCmd) is only 8 bytes, not 10.
>>
>>While that is not too bad, it gets more interesting as you read 
>>    
>>
>the 
>  
>
>>mmc_block_write().  During mmc_init(), CMD16 has set the blocksize 
>>    
>>
>to 
>  
>
>>0x200 (512 bytes), okay?  But, when they write the data during 
>>mmc_block_write(), they only write 8 bytes.  Then they proceed to 
>>    
>>
>hold 
>  
>
>>the chipselect down and poll 255 times for a completion status 
>>    
>>
>code!  Of 
>  
>
>>course, it dies, while the MMC gets deadlocked waiting for the 
>>    
>>
>rest of 
>  
>
>>the data...
>>
>>Another issue is that the mmc_write_block() does not look at BUSY 
>>    
>>
>from 
>  
>
>>the MMC, but assumes a delay loop is an answer to when a write has 
>>completed!!!
>>
>>With my understanding of MMC and the reading of the code in 
>>    
>>
>AN10406, it 
>  
>
>>looks as if mmc_block_write() is never going to write a block to 
>>    
>>
>the 
>  
>
>>MMC?  After compiling and debugging the code for a few hours, I'm 
>>satisfied that there are problems with it (appnote code).
>>
>>The night was not a total loss, working with the app note had made 
>>    
>>
>me go 
>  
>
>>back and take a critical look at the MMC spec + my code.  Heh, my 
>>    
>>
>code 
>  
>
>>is working flawlessly now!
>>
>>IMHO, I strongly urge Philips to remove AN10406 from their website 
>>    
>>
>until 
>  
>
>>a code review has been performed.
>>
>>
>>Regards,
>>
>>TomW
>>
>>
>>
>>
>>-- 
>>Tom Walsh - WN3L - Embedded Systems Consultant
>>http://openhardware.net, http://cyberiansoftware.com
>>"Windows? No thanks, I have work to do..."
>>----------------------------------------------------
>>
>>    
>>
>
>
>
>
>
>
>
> 
>Yahoo! Groups Links
>
>
>
> 
>
>
>  
>


-- 
Tom Walsh - WN3L - Embedded Systems Consultant
http://openhardware.net, http://cyberiansoftware.com
"Windows? No thanks, I have work to do..."
----------------------------------------------------