Sam Bucca
Sam Bucca

Reputation: 27

C Memory Buffer corrupting micro-controller memory allocation - Atmel ATxmega

I am using the ATxmega128A1 8-bit microcontroller, and I am trying to figure out why my memory buffer is behaving strangely.

I have a FIFO buffer of the following structure:

typedef struct
{
  FIFOid ID;                        //ID value to determine the type of FIFO being used
  BOOL LOCK;                        //A lock to prevent multiple FIFO accessing (i.e. data corruption)
  UINT16 Start, End;                //Value of the current start and end FIFO index
  UINT16 volatile NbBytes;          //Number of bytes currently inside the FIFO
  UINT8 Buffer[FIFO_SIZE];          //Memory buffer for the FIFO buffer
} TFIFO;

And I define 2 FIFOs:

//FIFO Buffers for USART
TFIFO RxFIFO;           //Creates the Receive FIFO
TFIFO TxFIFO;           //Creates the Transmit FIFO

Where FIFO_SIZE is

#define FIFO_SIZE 256       

When I want to get a byte from the Buffer[] I pass a pointer to the FIFO through this function:

BOOL FIFO_Get(TFIFO * const FIFO, UINT8 * const dataPtr);

This works fine, however, once I increase my FIFO_SIZE above ~510 bytes - Passing this FIFO pointer through this function causes another one of my FIFOs' buffer to be altered.

I have seen on my debugger, there is no problem at this line:

if(FIFO_Get(&TxFIFO, &data)){       //Get a byte from TxFIFO

And once I step over, and it is passed through to the function, the RxFIFO->Buffer[] array is modified, namely the larger address values. FIFO_Get function:

// ----------------------------------------
// FIFO_Get
// ----------------------------------------
// Remove one character from the FIFO
// Input: 
//   FIFO is a  pointer to a FIFO struct with data to be retrieved
//   dataPtr is a pointer to a memory location to place the retrieved byte
// Output:
//   TRUE if the operation was successful and the data is valid
// Conditions:
//   Assumes that FIFO_Init has been called

BOOL FIFO_Get(TFIFO * const FIFO, UINT8 * const dataPtr)
{
    BOOL FIFOGetSuccess;
    FIFOGetSuccess = bFALSE;

    //disable interrupts
    //if we're GETTING from the USART receive FIFO
    //we need to disable the receive interrupt so
    //there is no data being 'Put' during a 'Get'
    if(FIFO->ID == URX){
        USARTD0.CTRLA = USART_RXCINTLVL_OFF_gc; //Rx interrupts off
        USARTD1.CTRLA = USART_RXCINTLVL_OFF_gc; //Rx interrupts off
    }

    //Attempt to get a byte from the FIFO
    if(FIFO->LOCK == bFALSE){                           //Check that the FIFO is not locked
        FIFO->LOCK = bTRUE;                             //lock the FIFO
        if (FIFO->NbBytes == 0){                        //Checks whether FIFO is empty; number of bytes in FIFO = 0
            FIFOGetSuccess = bFALSE;                    //If empty, false is Returned
        }else{
            *dataPtr = FIFO->Buffer[FIFO->Start];       //Reads the byte in the FIFO start position, and saves it to the location of the data pointer
            if (FIFO->Start == (FIFO_SIZE - 1)){        //Checks whether the start position is equal to FIFO size, the last valid 8 bit number
                FIFO->Start = 0;                        //If so, the position is set to 0
            }else{                                      //If not fifo size;
                FIFO->Start++;                          //Increments the start position by 1
            }
            FIFO->NbBytes--;                            //Decrements the number of bytes in the FIFO by 1
            FIFOGetSuccess =  bTRUE;                    //Returns true if this is successful
        }
        FIFO->LOCK = bFALSE;                            //unlock the FIFO
    }

    //re-enable interrupts
    if(FIFO->ID == URX){
        USARTD0.CTRLA = USART_RXCINTLVL_LO_gc;  //low level interrupts on Rx
        USARTD1.CTRLA = USART_RXCINTLVL_LO_gc;  //low level interrupts on Rx        
    }

    return FIFOGetSuccess;
}

My guess is that i am not safely allocating memory in the microcontroller, however there seems to be plenty of memory left in the chip, after build:

        Program Memory Usage    :   21460 bytes   15.4 % Full
        Data Memory Usage       :   6967 bytes   12.1 % Full

How do I go about fixing this & what am I doing wrong?

EDIT:

I have found that increasing the FIFO SIZE maps the FIFO buffer over the SRAM address range (the SRAM of the ATxmega128A1 goes from 0x2000 to 0x3FFF)

FIFO address mapping

The addresses 0x3FF7 to 0x3FFF are changing when the other FIFO is passed through a function

EDIT 2: Elf Files

Upvotes: 0

Views: 185

Answers (2)

OlivierM
OlivierM

Reputation: 3212

From the ELF file, I can see this memory map:

00802000 B __bss_start
00802000 D __data_end
00802000 D __data_start
00802000 D _edata
00802000 00000002 b n.4418
00802002 00000001 b packetEnd.1686
00802003 00000002 b byteCount.1684
00802005 00000002 b StepsCounted.4253
00802007 00000001 b ErrorCode.4252
00802008 00000001 b SuccessNb.4251
00802009 00000001 b ReturnNb.4250
0080200a 00000004 b stepsOutOpto.4211
0080200e 00000004 b HomingError.4212
00802012 00000008 b stepscounted.4210
0080201a 00000004 b CntHomeState.4209
0080201e 00000001 b byteCount.4166
0080201f 00000002 B EjectMsgID
00802021 00000001 B VersionMinor
00802022 00000002 B QueueEnd
00802024 00000001 B VersionMonth
00802025 00000001 B ACC1_IN
00802026 0000001f B PacketQRx
00802045 0000001f B PacketTx
00802064 00000001 B VersionMajor
00802065 00000001 B DeviceTypeId
00802066 00000001 B debugCycler_bytes
00802067 00000001 B debugAXIS_ACK
00802068 000000d4 B AXIS
0080213c 00000001 B LVL_SENSE_IN
0080213d 00000001 B LID_DETECTION_ON
0080213e 00000001 B readyimmediateTx
0080213f 0000001f B PacketQTx
0080215e 00000004 B RobotSerialNumber
00802162 00000001 B USBIN
00802163 00000001 B startupComplete
00802164 000007c0 B PacketQueue
00802924 0000001f B PacketRx
00802943 00000001 B ACC2_IN
00802944 00000002 B QueueNb
00802946 00000001 B QueuedCommandCalled
00802947 0000001f B dummyPkt
00802966 00000001 B CYCLER_IN
00802967 00000002 B QueueStart
00802969 00000001 B HardwareRevision
0080296a 00000001 B VersionYear
0080296b 00000980 B AXISRxFIFO
008032eb 00000004 B RTS
008032ef 00000980 B AXISTxFIFO
00803c6f 00000068 B AXIS_SPI_Tx_Packet
00803cd7 00000004 B ReturnPacketMissed
00803cdb 00000068 B AXIS_SPI_Rx_Packet
00803d43 00000004 B NbPacketsInAxisFIFO
00803d47 00000100 B CyclerEEPROM
00803e47 00000260 B TxFIFO
008040a7 00000260 B RxFIFO
00804307 B __bss_end

Note: the way I had this map was by using nm with the command nm -S -n MotorBoard\ Xmega\ Large\ FIFO.elf. The first column is the address and the second column is the size of the variables.

ATMEL ATxmega128A1 Datasheet says you have 8KB of SRAM between 0x2000 and 0x3FFF. As you can see both of your FIFO buffer overlap the 0x3FFF limit (0x3E47 for TxFIFO and 0x40A7 for RxFIFO).

One way to fix your issue is to move some of your variable into ROM by declaring them as const. For instance, I can see VersionMonth, VersionMajor that are probably constant.

Upvotes: 1

unalignedmemoryaccess
unalignedmemoryaccess

Reputation: 7441

No. Just no.

Your concept is wrong. With FIFO buffer, you should use only one write and only one read point in your program. In you case, write would be inside UART RX interrupt and read would be for example in while (1) loop of your program.

Sufficient structure info is this:

#define FIFO_SIZE       100
typedef struct {
    uint16_t Write, Read;
    uint8_t Buffer[FIFO_SIZE];
} FIFO;

And then you need 2 (better 3) functions:

  • FIFO_Write(FIFO* f, uint8_t ch)
  • FIFO_Get(FIFO* f, uint8_t* ch)
  • FIFO_Init(FIFO* f);

And the implementation something similar as:

void FIFO_Init(FIFO* f) {
    f->Write = 0;
    f->Read = 0;
}

//This function needs check if buffer is full, will update it.
void FIFO_Write(FIFO* f, uint8_t ch) {
    //Check here if buffer is full first
    f->Buffer[f->Write++] = ch;
    if (f->Write >= FIFO_SIZE) {
        f->Write = 0;
    }
}

uint8_t FIFO_Read(FIFO* f, uint8_t* ch) {
    volatile uint16_t write = f->Write, read = f->Read;
    if (write == read) {
        return 0;  //FIFO empty, return 0
    }
    *ch = f->Buffer[f->Read++];
    if (f->Read >= FIFO_SIZE) {
        f->Read = 0;
    }
    return 1;      //character is valid
}

Functions for writing and reading are now safe of each other. You can write from IRQ or everywhere and read at the same time without worrying at all.

You can use FIFO_Write inside interrupt easily and when you read from FIFO, you don't need to disable interrupts. That's the purpose of FIFO. Single write point, single read point.

Hope it helps.

Upvotes: 0

Related Questions