View previous topic :: View next topic |
Author |
Message |
jecottrell
Joined: 16 Jan 2005 Posts: 559 Location: Tucson, AZ
|
Inconsistent FRAM Read Results |
Posted: Wed Apr 04, 2007 2:07 pm |
|
|
18F6627 40MHz
FM25256 SPI FRAM
3.236
Hello All,
Battling a new one over the last few hours. If use sequential reads and writes everything works ok (or so it appears.) If I use individual reads and writes I get errors after writing more than 64 values in a FOR loop. And it appears that mixing a sequential write with a individual read causes problems.
Here is some code:
Code: |
#define FRAM_SIZE 32768
void init_fram(void)
{
delay_ms(15); //FRAM is not available until 10ms after power-up
setup_spi(SPI_MASTER | SPI_MODE_0_0 | SPI_CLK_DIV_4);
}
void write_int16_fram(int16 address, int16 value)
{
output_low(FRAM_CS); //select FRAM
spi_write(WREN); //enable writes, required prior to WRITE cmd
output_high(FRAM_CS); //de-select FRAM, required between each op-code
delay_cycles(1); //ensure compiler doesn't remove the pin toggle
output_low(FRAM_CS); //re-select the FRAM
spi_write(WRITE); // send the WRITE op-code
spi_write(address); //send the low byte of the address
spi_write(address >> 8); //send the high byte of the address
spi_write(value);
spi_write(value >> 8);
output_high(FRAM_CS); //de-select FRAM
}
int16 read_int16_fram(int16 address)
{
int8 value_low;
int8 value_hi;
int16 value;
output_low(FRAM_CS); //re-select the FRAM
spi_write(READ); // send the WRITE op-code
spi_write(address); //send the low byte of the address
spi_write(address >> 8); //send the high byte of the address
value_low = spi_read(0);
value_hi = spi_read(0);
output_high(FRAM_CS); //de-select FRAM
value = make16(value_hi, value_low);
return(value);
}
void write_int16_seq_fram(int16 start_addr, int16 *pntr, int16 count)
{
if(start_addr + (count * 2) <= FRAM_SIZE)
{
//setup_spi(SPI_MASTER | SPI_MODE_0_0 | SPI_CLK_DIV_4);
output_low(FRAM_CS); //select FRAM
spi_write(WREN); //enable writes, required prior to WRITE cmd
output_high(FRAM_CS); //de-select FRAM, required between each op-code
delay_cycles(1); //ensure compiler doesn't remove the pin toggle
output_low(FRAM_CS); //re-select the FRAM
spi_write(WRITE); // send the WRITE op-code
spi_write(start_addr); //send the low byte of the address
spi_write(start_addr>>8); //send the high byte of the address
do
{
//restart_wdt(); //uncomment if slow speeds
spi_write(*pntr++);
spi_write(*pntr++);
} while(--count);
output_high(FRAM_CS); //de-select FRAM
}
}
void read_int16_seq_fram(int16 start_addr, int16 *pntr, int16 count)
{
if(start_addr + (count * 2) <FRAM_SIZE>>8); //send the high byte of the address
do
{
//restart_wdt(); //uncomment if slow speeds
*pntr++ = spi_read(0);
*pntr++ = spi_read(0);
} while(--count);
output_high(FRAM_CS); //de-select FRAM
}
}
|
I'm to a point where I'm not getting any sort of results that point me in a direction to look.
Any help would be appreciated,
John
EDIT:
I'm starting to think it may be a difference in the addressing between sequential and individual reads/writes?
Last edited by jecottrell on Wed Apr 04, 2007 5:28 pm; edited 1 time in total |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Wed Apr 04, 2007 2:16 pm |
|
|
Quote: | void write_int16_seq_fram(int16 start_addr, int16 *pntr, int16 count)
spi_write(*pntr++);
spi_write(*pntr++); |
Your pointer is pointing to a 16-bit object. When you increment it,
it will increment by 2 bytes each time. Suppose pntr is initially set to 0.
After your first spi_write() statement, it will now point to address 2.
The same problem exists in the read routine. |
|
|
jecottrell
Joined: 16 Jan 2005 Posts: 559 Location: Tucson, AZ
|
|
Posted: Wed Apr 04, 2007 2:22 pm |
|
|
THANK-YOU!
I'll read up on it and try to clean it up this evening and test it out.
John |
|
|
jecottrell
Joined: 16 Jan 2005 Posts: 559 Location: Tucson, AZ
|
|
Posted: Wed Apr 04, 2007 5:38 pm |
|
|
OK. I'm going to ask for some spoon feeding.... (or a good tutorial on pointers). I only brought one C book into the office this evening and it doesn't cover this stuff in very good detail. (O'Reilly Practical C)
I can think of a couple of ways to kludge something together, but I'm sure they're probably not the recommended ways... or probably won't work for some subtle reason.
Code: | void write_int16_seq_fram(int16 start_addr, int8 *pntr, int16 count) |
or maybe...
Code: | void write_int16_seq_fram(int16 start_addr, int16 *pntr, int16 count)
spi_write(*pntr++);
spi_write(*(pntr-1)); |
Thanks,
John
EDIT:
After a quick bit of reading and even smaller bit of thinking... it looks like the second approach is 'cleaner'.... but probably executed wrong...
EDIT2:
Wait. I just realized something new that confuses me even more... is *pntr a int16 or int8? The spi_write is only going send a byte at a time...
ahhhh crap. I give up.... |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Wed Apr 04, 2007 6:00 pm |
|
|
You have a pointer to a 16-bit word. All you want to do is extract
the low and high bytes. |
|
|
jecottrell
Joined: 16 Jan 2005 Posts: 559 Location: Tucson, AZ
|
|
Posted: Wed Apr 04, 2007 6:02 pm |
|
|
I was just going to add another edit with that conclusion.... it just hit me.
Thanks.
John |
|
|
jecottrell
Joined: 16 Jan 2005 Posts: 559 Location: Tucson, AZ
|
|
Posted: Wed Apr 04, 2007 10:22 pm |
|
|
PCM,
Can you grade my work? I'm not at PC where I can test my code, but I'd still like to get some feed back on my approach. Specifically, the order of operations/parenthesis grouping in the increment/shift operation and is there a cleaner way to read and combine the two int8s into an int16?
Thanks,
John
Code: | void write_int16_seq_fram(int16 start_addr, int16 *pntr, int16 count)
{
if(start_addr + (count * 2) <= FRAM_SIZE)
{
//setup_spi(SPI_MASTER | SPI_MODE_0_0 | SPI_CLK_DIV_4);
output_low(FRAM_CS); //select FRAM
spi_write(WREN); //enable writes, required prior to WRITE cmd
output_high(FRAM_CS); //de-select FRAM, required between each op-code
delay_cycles(1); //ensure compiler doesn't remove the pin toggle
output_low(FRAM_CS); //re-select the FRAM
spi_write(WRITE); // send the WRITE op-code
spi_write(start_addr); //send the low byte of the address
spi_write(start_addr>>8); //send the high byte of the address
do
{
//restart_wdt(); //uncomment if slow speeds
spi_write(*pntr);
spi_write((*pntr++) >> 8);
} while(--count);
output_high(FRAM_CS); //de-select FRAM
}
}
void read_int16_seq_fram(int16 start_addr, int16 *pntr, int16 count)
{
int8 value_low;
int8 value_hi;
if(start_addr + (count * 2) <= FRAM_SIZE)
{
output_low(FRAM_CS); //re-select the FRAM
spi_write(READ); // send the WRITE op-code
spi_write(start_addr); //send the low byte of the address
spi_write(start_addr>>8); //send the high byte of the address
do
{
//restart_wdt(); //uncomment if slow speeds
value_low = spi_read(0);
value_hi = spi_read(0);
*pntr++ = make16(value_hi, value_low);;
} while(--count);
output_high(FRAM_CS); //de-select FRAM
}
} |
EDIT:
OK. Following 'Practical C's suggestion, I'll stick with clear coding as opposed to trying to be slick (which I'm not....)
Code: |
spi_write(*pntr);
spi_write(*pntr >> 8);
pntr++;
value_low = spi_read(0);
value_hi = spi_read(0);
*pntr = make16(value_hi, value_low);
pntr++;
|
|
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Wed Apr 04, 2007 11:41 pm |
|
|
Quote: |
spi_write(READ); // send the WRITE op-code
spi_write(start_addr); //send the low byte of the address
spi_write(start_addr>>8); //send the high byte of the address
|
Quote: |
spi_write(WRITE); // send the WRITE op-code
spi_write(start_addr); //send the low byte of the address
spi_write(start_addr>>8); //send the high byte of the address
|
In both your read and write routines above, you're sending the LSB
of the address first. Figures 9 and 10 of the data sheet (on page 8)
show that the MSB of the address should be sent first, and then the LSB.
http://www.ramtron.com/lib/literature/datasheets/Errata004_25x256.pdf |
|
|
|