energia / Energia

Fork of Arduino for the Texas Instruments LaunchPad's
http://energia.nu
Other
795 stars 671 forks source link

SPI SRAM Strange Findings — Max Speed = 4 MHz on LM4F! #164

Closed rei-vilo closed 11 years ago

rei-vilo commented 11 years ago

Code available at branch issue_164_SPI_SRAM/examples/4.Communication/SRAM_SPI

A couple of strange things:

before starting to read the right address.

The 23K640 is rated at 20 MHz.

Generally speaking, SPI seems more robust on the LM4F.

rei-vilo commented 11 years ago

SPI_CLOCK_DIV1 isn't declared for the StellarPad.

rei-vilo commented 11 years ago

Why line 119 of Energia / hardware / lm4f / libraries / SPI / SPI.cpp is limited to 4 MHz?

ROM_SSIConfigSetExpClk(SSIBASE, SysCtlClockGet(), SSI_FRF_MOTO_MODE_0,
      SSI_MODE_MASTER, 4000000, 8);
rei-vilo commented 11 years ago

The extra SPI.transfer(0) on the MSP430G2553 has been confirmed by B@tto

Oh yes ! I was trying to play with NRF24L01 and I had this bug which was getting me crazy ... Thank you very much !

at http://forum.43oh.com/topic/3083-energia-library-launchpad-stellarpad-—-23k640-spi-sram/?p=26984

RickKimball commented 11 years ago

It would be nice to see some logic analyzer output with simple test cases to confirm this. I don't have either of these devices to test with.

RickKimball commented 11 years ago

Maybe you could try commenting out lines 72,73 in https://github.com/energia/Energia/blob/master/hardware/msp430/libraries/SPI/utility/usci_spi.cpp

-rick

rei-vilo commented 11 years ago

It would be nice to see some logic analyzer output with simple test cases to confirm this. I don't have either of these devices to test with.

I've tried, but the signal on the MSP430G2553 is below the threshold of my logic analyser which doesn't feature adjustable threshold.

On the LM4F and on the MSP430G2553 with the initial mock reading data[0] = SPI.transfer(0);

*** start
write >abcdefghijklmno
buffer=---------------
read  <abcdefghijklmno
*** end

On the MSP430G2553 without the initial mock reading data[0] = SPI.transfer(0);

*** start
write  >abcdefghijklmno
buffer =---------------
read   <�abcdefghijklmn
*** end

The first reading returns .

Commenting lines 72 and 73 on usci_spi.cpp as suggested has no impact.


Attached minimal code with unchanged SRAM library SRAM.cpp and SRAM.h

#include "Energia.h"
#include "SPI.h"
#include "SRAM.h"

// Define variables and constants
#if defined(__MSP430G2553__)
SRAM mySRAM(P1_4); // chip select on pin P1_4
#elif defined(__LM4F120H5QR__)
SRAM mySRAM(PE_5); // chip select on pin PE_5
#else
#error Board not supported
#endif

const uint16_t MAX = 0x0f;
char buffer[MAX];

void setup (void)
{
    SPI.begin();
    SPI.setClockDivider(SPI_CLOCK_DIV8);
    mySRAM.begin();

    Serial.begin (9600);
    Serial.println();

    Serial.println("*** start");

    Serial.print("write  >");
    for (uint8_t j=0; j<MAX; j++) {
        buffer[j]='a' + j;
        Serial.print((char)buffer[j]);
    }
    Serial.println();

    mySRAM.write(300, (uint8_t *) buffer, sizeof buffer);

    Serial.print("buffer =");

    for (uint8_t j=0; j<MAX; j++) {
        buffer[j]='-';
        Serial.print((char)buffer[j]);
    }
    Serial.println();

    Serial.print("read   <");

    mySRAM.read(300, (uint8_t *) buffer, MAX);

    for (uint8_t j=0; j<MAX; j++) {
        Serial.print((char)buffer[j]);
    }
    Serial.println();

    Serial.println("*** end");
    Serial.end();
    while(true);
}

void loop (void)
{
}
RickKimball commented 11 years ago
/**
 * spi_send() - send a byte and recv response
 */
uint8_t spi_send(const uint8_t _data)
{
    UCB0TXBUF = _data; // setting TXBUF clears the TXIFG flag

    while (UCB0STAT & UCBUSY)
        ; // wait for SPI TX/RX to finish

    return UCB0RXBUF; // reading clears RXIFG flag
}
rei-vilo commented 11 years ago

Thanks!

It works fine and stability is met up to SPI.setClockDivider(SPI_CLOCK_DIV2); for MSP430G2553.

Are other usi_spi and eusci_spi concerned by the new code?

For the LM4F120H5QR, I suggest changing line 119 from

ROM_SSIConfigSetExpClk(SSIBASE, SysCtlClockGet(), SSI_FRF_MOTO_MODE_0,
      SSI_MODE_MASTER, 4000000, 8);

to

ROM_SSIConfigSetExpClk(SSIBASE, SysCtlClockGet(), SSI_FRF_MOTO_MODE_0,
      SSI_MODE_MASTER, 8000000, 8);
RickKimball commented 11 years ago

@rei-vilo "Are other usi_spi and eusci_spi concerned by the new code?"

Not that I know of. ... actually the eusci_spi.cpp may need the same change. I'll test it when I get a chance.

calinp commented 11 years ago

Hi Rei, I made some checks on the SPI.setClockDivider function and it appears that it changes only the system clock prescaler. I changed the original code to

//value must be even //HWREG(SSIBASE + SSI_O_CPSR) = divider;

HWREG(SSIBASE + SSI_O_CPSR) = 4; //predivider 4 => 80 / 4 = 20MHz SSIclk HWREG(SSIBASE + SSI_O_CR0) |= (uint16_t)((divider)<<8); //divide 20MHz by 'divider' for desired baudrate

and it seems that the clock changes.

If you have time, maybe you can make some checks on the scope. I expect that setting SPI_CLOCK_DIV1 will result in 20MHz SPI clock, SPI_CLOCK_DIV2 in 10MHz and so on.

Calin

rei-vilo commented 11 years ago

Here are the results for the solution suggested by calinp:

HWREG(SSIBASE + SSI_O_CPSR) = divider;

with initial (line 120)

ROM_SSIConfigSetExpClk(SSIBASE, SysCtlClockGet(), SSI_FRF_MOTO_MODE_0,
      SSI_MODE_MASTER, 4000000, 8);
HWREG(SSIBASE + SSI_O_CPSR) = 4; //predivider 4 => 80 / 4 = 20MHz SSIclk
HWREG(SSIBASE + SSI_O_CR0) |= (uint16_t)((divider)<<8); //divide 20MHz by 'divider' for desired baudrate

SPI_results

New config=4000000, pre-divider=4, divider=DIV2... new_div2 ...compared to old config=4000000, divider=DIV2: old_div2

The SRAM can't cope with speeds higher than 8 MHz... SPI_nok ...instead of SPI_ok

rei-vilo commented 11 years ago

Closed by error.

Thank you calinp for the suggestion!

Unfortunately, the issue is still there!

For the moment, I'm sticking with my patch on line 120

ROM_SSIConfigSetExpClk(SSIBASE, SysCtlClockGet(), SSI_FRF_MOTO_MODE_0,
      SSI_MODE_MASTER, 8000000, 8);
rei-vilo commented 11 years ago

See also commit 05adb08