MCF5272 QSPI

[rtripodi]

Good morning to all,

i need your help about QSPI implementation. I'm a rockie on firmware implementation and now i must implement a QSPI to connect a single device. Now i haven't the device, and to test my code i've applied a jumper to MOSI and MISO pin.

My problem is after the first transmition, i call my QSPI_Transfer function 4 times with 4 diffferent values, after the first transmition i call QSPI_Transfer with another value, it was transmitted, but when i try to read the receive data i read the data of my first transmition.This happens for every transmition, i receive data of my previous transmittion.For example:

transmit data :

0x01

0x02

0x03

0x04

receive data:

0x01

0x01

0x02

0x03

To resolve this problem i have added a for cycle after transmition and i read the correct data.This solution can be right?
what i wrong in my code? what is the problem?

I've implemented this code:

in main:

main(){

BYTE data,Receive;

QSPI_Init();

.

.

.

data=0x01;

Receive=QSPI_Transfer(data);

data=0x02;

Receive=QSPI_Transfer(data);

data=0x03;

Receive=QSPI_Transfer(data);

data=0x04;

Receive=QSPI_Transfer(data);

}

void QSPI_Init(void){

// I call this function only 1 a time
WORD config = 41215;   
MCF5272_WR_QSPI_QMR(config);  //set the QMR register
}


//For my purpose i call this function 4 times
BYTE QSPI_Transfer(BYTE Data){

BYTE Ricezione;
BYTE i;
WORD QMR;
//Set data
MCF5272_WR_QSPI_QAR(QSPI_RAM_TX);     // QSPI_RAM_TX is 0x00
MCF5272_WR_QSPI_QDR(Data);      //data is 0x05

//Set Wrap register
MCF5272_WR_QSPI_QWR(QSPI_TX_QWR);  //set wrap to 0

//set command
MCF5272_WR_QSPI_QAR(QSPI_RAM_CMD); // QSPI_RAM_CMD is 0x20
MCF5272_WR_QSPI_QDR(QSPI_CMD);   //command is 16384

//Start transfer
MCF5272_WR_QSPI_QDLYR(QSPI_START);   //QSPI_START is 32768

while(!MCF5272_RD_QSPI_QIR){}   //Wait untill SPIF is set to 1

MCF5272_WR_QSPI_QIR(0x00);   //Reset SPIF

MCF5272_WR_QSPI_QAR(QSPI_RAM_RX);   //QSPI_RAM_RX is 0x10

for(i=0;i<200;i++){}    //wait the update of data

Ricezione=MCF5272_RD_QSPI_QDR;   //read transmetted data

return Ricezione;
}

Thank you for your help.

[TomE]

First of all, please don't do things like loading "41215" into QMR. I shouldn't have to pick up a hex calculator to convert that back into bits to see how you're programming the chip. It is almost as if you are trying to make your code as hard to read as possible. You don't need to use raw hex either (like you do elsewhere) as the register bit meanings should already be defined in one of your system header files. Your code will be a lot easier for you and everyone else to read if you write it this way.

Here's an example of poor code in our system and how it should have been written:

    uint16_t qmr = 0x8004 | ((is_words?0:8)<<10); // 10000010 00000100 - 10MHz    uint16_t qwr = 0x1000 | ((p->num-1)<<8); // 0001 nnnn 0000 0000     uint16_t qdlr = 0x8001; // go with  QCD=0 and DTL=1    if ( p->options & QSPI_DELAY )    {        /* Doesn't set DTL=14, but sets to 15 unintentionally... */        qdlr |= 0x0e; // hack which is the right t_conv for ADC    }            MCF_QSPI_QMR = qmr;    MCF_QSPI_QWR = qwr;    MCF_QSPI_QDLYR = qdlr;===================  The first bit should have been written like ========    uint16_t qmr = MCF_QSPI_QMR_MSTR | MCF_QSPI_QMR_BAUD(4) |                   MCF_QSPI_QMR_BITS(is_words?0:8);    uint16_t qwr = MCF_QSPI_QWR_CSIV | MCF_QSPI_QWR_ENDQP(p->num - 1);     uint16_t qdlr = MCF_QSPI_QDLYR_SPE | MCF_QSPI_QDLYR_QCD(0) |                    MCF_QSPI_QDLYR_DTL(1);    if ( p->options & QSPI_DELAY )    {        qdlr = MCF_QSPI_QDLYR_SPE | MCF_QSPI_QDLYR_QCD(0) | MCF_QSPI_QDLYR_DTL(QSPI_DTL_ADC);    }

Please use the "[C]" button on this forum to insert code like I did for the above section. It makes it easier to read.

Search this forum for "QSPI" and read through all the previous problems. Read through this one:

https://community.freescale.com/message/50239#50239

You should follow the links to Rich T's QSPI drivers and use them instead of trying to write your own.

Specifically the remarks that hint at the cause of your problem: "If DT = 1, you can shrink or grow the delay by changing the value of QDLYR[DTL].  If DT = 0, a standard delay period is used.  Time is needed to ensure the transfer RAM is loaded."

If you're finding that you need this delay, you should not add it with a "for loop". That is dangerous for a whole lot of reasons. The QSPI has programmable delays after transfers. Read up on how to set these. If you need a longer delay then set "DT" and "QDLYR[DTL]."

In my manual (MCF5329, but same QSPI pretty much), the "Transfer Delays" section says:

Adequate delay between transfers must be specified for long data streams becausethe QSPI module requires time to load a transmit RAM entry for transfer. Receivingdevices need at least the standard delay between successive transfers. If theinternal bus clock is operating at a slower rate, the delay between transfersmust be increased proportionately.

I don't know what they mean by the last sentence either, but it must mean something...

The "delay" might not be your problem though. There are two obvious bugs in your code I've just noticed:

while(!MCF5272_RD_QSPI_QIR){}   //Wait untill SPIF is set to 1MCF5272_WR_QSPI_QIR(0x00);   //Reset SPIF

The first line isn't waiting until SPIF is one - it is waiting for ANY bit to be set in this register. So if you're getting error bits set you won't see them. You should explicitly test the bit you're waiting for for when you do get an error. And test for error bits and print diagnostics if they get set. That will save you some time.

The second line won't "Reset SPIF". It won't do anything at all. Read the manual and read what "W1C" means. The SPIF bit description states "Writing a 1 to this bit (w1c) clears it and writing 0 has no effect."

So you're never clearing SPIF and since that's the bit you're waiting on it should explain your problem.

Tom

[rtripodi]

I give you my apologize for my post presentation, that is my first post, now i know how i must do a post.

My problem was to clear SPIF bit, it didn't reset. i've corrected my bugs, here my code:

while(!(MCF5272_RD_QSPI_QIR & QSPI_QIR_SPIF)){} //until QIR[SPIF] is 1MCF5272_WR_QSPI_QIR(QSPI_QIR_SPIF); //set QIR 0000000000000001

I've seen the post about QSPI delay, and i've a question about it. I must control via QSPI my slave divice, i must send read command and wait until the end of reading.My device put that data into his slave transfer register. After reading i must use my QSPi in  MISO mode.  But to do this my device need time. Is correct to use QSPI delay after transfer for this purpose?

Thank you for your help Tom.

[TomE]

> After reading i must use my QSPi in  MISO mode.

No. The QSPI is always the master device on the SPI bus. It isn't like you send a command to the chip in Master mode and then it switches itself to Master mode in order to "write the response back". That's not the way SPI is meant to work.

A single SPI exchange shifts 8 (or 12 or 16) bits out and shifts the same number of bits in at the same time.

A "command" to a peripheral consists of two SPI sequences. In the first one you shift out the command (to make the peripheral read something) and ignore the data read in that cycle. In the second you shift out something that won't do any damage and get the response to the previous command back. In the second cycle you might be able to shift out a new command (while reading the previous response) and then read that the next time. With some memory chips if you keep cycling it keeps providing successive data bytes..

With a simple SPI you would send single commands like that. That might be all your application needs.

But since this is a QSPI you can get it to execute a QUEUE of up to 16 commands.

We have an ADC on the SPI bus. It has 11 inputs. It takes 16-bit SPI commands and returns 16 bit responses. We load the QSPI with sequential commands to read channels 1, 2, 3 ... 9 10 11. It also reads the results to 11, 1, 2, 3 ... 8, 9, 10. The first read returns the results for channel 11 the PREVIOUS time we sent this burst.

Since the ADC needs 5us to perform the convert between the command and the next cycle, we have to use the "Delay" command to separate the SPI cycles by about 5.5us. The QSPI reads 11 ADC channels over about 80us without any CPU intervention required. It interrupts and all the data is there.

If your chip needs a delay between the SPI cycles (like our ADC does) then that's what the delay is for. You might have a slow memory chip that needs a while to read data before returning it.

I strongly recommend you use someone else's code rather than write your own. That should be a far better use of your time than reinventing this wheel.

Tom

[rtripodi]

 > After reading i must use my QSPi in  MISO mode.
You have misunderstood my phase. I've write "use my QSPI in MISO mode" to simplify the explanation of the process of receiving. I know is always the master the coordinator of transmission and reception, how you have described above.Sorry for this misunderstanding.

I didn't know how work QSPI delay, thank you for your explanation, but this doesn't resolve my problem.

My Problem is the time. I send to slave via SPI the command Byte. This Byte indicate,start reading.I must send this byte otherwise my device doesn't work. The reading operation need 2-3 second to read data. I must wait this 2-3 second before start a new transmission and get data.
I've think this solution but i don't know if is correct, because i use a loop :

BYTE cont=0;          //inizialize counterQSPI_Transfer(Command);  //send command readwhile(cont++<1000){};  //wait cont=0;data=QSPI_Receive();   //get data

It can be a solution? otherwise how i can put my system in standby for 2-3 second before reading data?

Thank you for your support.

[TomE]

> I must wait this 2-3 second before start a new transmission and get data.

> I've think this solution but i don't know if is correct, because i use a loop :

> BYTE cont=0;

> while(cont++<1000){}; //wait

You're aiming for a record number of bugs in the one bit of code there.

A byte can't ever count beyond 255, so it is going to take a very long time to get to 1000!

Assuming you used a variable the right width, your 66MHz CPU is going to count to 1000 in about 4 clocks/count, or about 60us. That's 5500 times faster than you need.

The compiler is also likely to notice that loop does nothing, and is within its rights to remove the loop completely. That's why you should never ever use a "wait loop". Unless you've coded it in assembly you have no idea how long it will take, and the time might very well change when you recompile it. Even in Assembly it will change if you change the memory timing or cache programming.

> how i can put my system in standby for 2-3 second before reading data?

Hasn't your computer got something better to do while it is waiting for this data?

If you were running a multithreaded operating system you'd call the OS function to suspend the thread.

If you were running a "task based" OS you'd set up a timer to run the thread again a few seconds later.

If you were running an "event loop" system with fixed timing you'd code a very small state machine and have it count time or calls. Something in your system should be set up to return the current time in milliseconds, microseconds or "struct timeval".

You could even program one of the four timers (TMR0 - TMR3) to generate an interrupt and have that run the next part of your code.

The first task is to get some sort of "operating system" running so it can do these things for you.

If you must wait in a "dead loop", then program up one of the Timers. They have a "maximum period of 4 seconds" according to the manual, so set one up like that and wait for it to count the "real time" for you.

I missed the best solution. When your peripheral has completed its task it should INTERRUPT your main CPU to then read the data. Then your main CPU doesn't have to always "know" how long the peripheral takes - that's a magic number in two different peices of hardware that can change and break the system. Alternately, couldn't the main CPU send some sort of command to the peripheral to poll and ask if it is ready and then read back when it is?

Tom

[FridgeFreezer]

The system we use to keep time in our system (many different things which must be done on a certain time-peroid) is this;

Set a hardware interrupt timer at 1KHz (1ms) interval. The interrupt service routine can contain only one line - incrementing a global TIME counter (32-bit number) by 1.

Then you store a "timeout" value (32-bit) for what you want to happen, and when you go past it you then do the action - for example:

TIME == 45678To set a timeout for 2sec:timeout = (TIME + 2000);in main():if(TIME > timeout){  action();timeout = (TIME + 2000); // reset timer}

 If you only want to do the action once you can set a flag, like:

if(FLAG == TRUE)
{
if(TIME > timeout)
{
action();
FLAG = FALSE;
}}

We have some stuff that happens exery 1sec, every 1min, etc. and some stuff which may only happen once on demand.

There is extra code (I have simplified here to show the concept) that checks for wraparound of the time/timeout values.

This is sort-of an alternative to using an RTOS to schedule stuff, you can just keep polling the timers in main(), if there are ones you know are "long-period" (EG minute, hour) you can nest them so you only check once per second if the minute-timer has expired for example.

Some micros you may be able to leave the interrupt timer free-running and just read the count directly from the registers.

[TomE]

> Set a hardware interrupt timer at 1KHz (1ms) interval.

Our code does exactly the same. An IPL6 interrupt (that can interrupt all other interrupts) incrementing a 32-bit millisecond counter.

The example code in the previous post doesn't execute every 2000ms, but every 2001ms after "action()" has finished. This may be exactly what is required in this case, but shouldn't be copied for something that is intended to be strictly periodic

For periodic tasks we usually code like the following. The forced use of unsigned subtraction and comparison also automatically handles when TIME rolls over 31 bits (sign rollover) and 32 bits (rolls back thtough zero):

extern volatile uint32_t TIME;
uint32_t nTimeStart = TIME;
...
if ((TIME - nTimeStart) >= 2000UL)
{
  action();
  nTimeStart += 2000UL; /* Not 2000 from NOW but 2000 from last trigger */
}

With this sort of timer code you have to be VERY careful about what is signed, what is unsigned, and what sort of comparisons are being done. Get that wrong and your code will fail every 28 days 20 hours when 32 bits of milliseconds goes negative, or at 49 days 17 hours  when it rolls back past zero. Or every 35/71 minutes if you're using a hardware microsecond counter.

Here's a good discussion and a general solution:

http://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution

That's why all timer code like this should be in a well defined and debugged LIBRARY of some sort. Our code base has 153 references to our equivalent of TIME above, and 267 references to the hardware microsecond counter we use.

In the above example, all I have to get wrong is to declare something "int32_t" instead of "uint32_t" once out of 153 times (or miss a cast) and I've got a hidden bug that won't be found unless regression testing forces TIME through the rollover points.

Tom

[rtripodi · Accepted solution]

Thank you for your answers.