This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

changing buffer from data to xdata

The following code fragment has been running for three years on an Atmel 89c55 with no problems. Now that I have started a project using the Cypress EZ-USB FX chip I would like to move the fifo buffer from the default generic pointer in data space.

#define FIFO0SIZE 10
BYTE *fifo0[FIFO0SIZE];

to xdata and increase the size using

#define FIFO0SIZE 100
xdata BYTE *fifo0[FIFO0SIZE];

It compiles but continualy sends garbage out the serial port.
I have tried pointer casting and changing to the large memory modle and a few other things, all with negative results.

Can anyone shed some light on this problem.

#define FIFO0FULL FIFO0SIZE-2
BYTE Que(new_ptr)
BYTE *new_ptr;
{
BYTE idata Pass,i;
Pass=0xff;
for(i=0;i<FIFO0SIZE-1;i++){
// add new message to end of buffer
if(!fifo0[i]){
// current position is empty go ahead and use it
if(i>FIFO0FULL) SBuff0_Full=1;
Pass=0;
fifo0[i]=(BYTE xdata *) new_ptr;
fifo0[i+1]=0;
if(!out0_busy) TI=1; // kick start the uart
break;
}
}
return Pass;
}

void Serial_Port0(void) interrupt COM0_VECT
{
BYTE idata i;
static BYTE data x=0;

if(RI) {
RI=0; // extract recieved byte
Inkey(SBUF0);
}

if(TI) {
TI=0;
if(fifo0[0][x]) {
SBUF0=(fifo0[0][x]);
x++;
out0_busy=1;
}
else out0_busy=0; // no character to send
if(!fifo0[0][x]) { // end of string last character is being sent
if(out0_busy) { // last character is being sent
for(i=0;i<FIFO0SIZE-1;i++) { // rotate buffer
if(fifo0[i]) { // if = 1 = buffer not empty
fifo0[i]=fifo0[i+1];
x=0;
} // if fifo0 buffer not empty
else {
if(i<2) SBuff0_Full=0;
break; // end of buffer
}
}
}
}
}
}

Parents
  • I agree that the assembly code fragments look like they should work. So that kinda makes me want to look elsewhere. BTW, did you ever have your "original" version (before moving to xdata) working on the Cypress part?

    That Cypress part is a speedy little bugger. It looks like CLK24 or CLK48 feeds the timers you'd be using for baud rate generation. Can we eliminate UART and BRG initialization differences factoring into the equation? I guess back to the first question -- had code running on the Cypress part ever output non-garbage via the UART (even the simplest single character non-interrupt driven output)?

    --Dan Henry

Reply
  • I agree that the assembly code fragments look like they should work. So that kinda makes me want to look elsewhere. BTW, did you ever have your "original" version (before moving to xdata) working on the Cypress part?

    That Cypress part is a speedy little bugger. It looks like CLK24 or CLK48 feeds the timers you'd be using for baud rate generation. Can we eliminate UART and BRG initialization differences factoring into the equation? I guess back to the first question -- had code running on the Cypress part ever output non-garbage via the UART (even the simplest single character non-interrupt driven output)?

    --Dan Henry

Children
  • Yes, the NON xdata (original) version works fine on the EZ-USB FX. The reason I MUST change to xdata is the USB code uses more of the internal RAM than my original code can spare.
    I'm sure the problem is very simple. Once I figure it out I will be embarrassed that I was stumped for so long. My problem is I have been using the interrupt driven Queuing scheme for so long I may be making fasle assumptions.
    My eventual goal is to convert the pointer to pointer buffer to a rotary buffer of the actual characters to be sent out the serial port. With the old system I was limited to internal memory. Now with the Cypress EZ-USB FX I can make this move.
    But my Que will still be receiving messages from DATA, IDATA and CODE. If this is the problem I am running into i.e. Generic pointers from different MSpaces. I may be fighting the same problem with the added confustion of all new code.

    Do you know of a good xdata rotary buffer example I could look at.

    Thank you very much for your input.
    Bob Dillon

  • To satisfy my curiosity, I tried simulating your code in its "xdata" version. I ran into a couple of problems, one of which could have been due to the simulation environment. Quoting your original code excerpts for reference:

    BYTE Que(new_ptr)
        BYTE *new_ptr;
    {
        BYTE idata Pass,i;
        Pass=0xff;
        for(i=0;i<FIFO0SIZE-1;i++){
            // add new message to end of buffer
            if(!fifo0[i]){
                // current position is empty go ahead and use it
                if(i>FIFO0FULL) SBuff0_Full=1;
                Pass=0;
                fifo0[i]= (BYTE xdata *) new_ptr; // <-- ?1?
                fifo0[i+1]=0;
                if(!out0_busy) TI=1; // kick start the uart
                break;
            }
        }
        return Pass;
    }
    
    void Serial_Port0(void) interrupt COM0_VECT
    {
        BYTE idata i;
        static BYTE data x=0;
    
        if(RI) {
            RI=0; // extract recieved byte
            Inkey(SBUF0);
        }
    
        if(TI) {
            TI=0;
            if(fifo0[0][x]) {
                SBUF0=(fifo0[0][x]);
                x++;
                out0_busy=1;
            }
            else out0_busy=0; // no character to send
            if(!fifo0[0][x]) { // end of string last character is being sent
                if(out0_busy) { // last character is being sent
                    for(i=0;i<FIFO0SIZE-1;i++) { // rotate buffer
                        if(fifo0[i]) { // if = 1 = buffer not empty
                            fifo0[i]=fifo0[i+1];
                            x=0; // <-- ?2?
                        } // if fifo0 buffer not empty
                        else {
                            if(i<2) SBuff0_Full=0;
                            break; // end of buffer
                        }
                    }
                }
            }
        }
    }
    The first problem, referring to "?1?" above, would be that a generic pointer gets stored in the FIFO as a memory-specific pointer, thus losing the data or code space-qualifiers in the process.

    The second problem, which may have been introduced by the simulation environment (speed differences versus actual hardware) is that "x" wasn't always getting reset to zero.

    A possible third problem is that I didn't see any obvious means to protect fifo0[] manipulation in Que() from being corrupted by the "rotation" in the ISR. Again, this could have been induced by simulation vs. hardware speed.

    Rather that proceed further, I just made a change (still keeping your "array of message pointer" FIFO approach) to avoid "rotating" the buffer by actually "shifting" fifo0 elements and instead use "head" and "tail" pointers and another pointer for scanning the bytes within a message. With this change, simulation runs fine with the FIFO in xdata, outputting messages from data, idata, xdata, and code memory spaces.

    Changed code below:

    BYTE  *byte_ptr;            /* Advances through message      */
    BYTE **tail_ptr = fifo0;    /* Where to enqueue next msg ptr */
    BYTE **head_ptr = fifo0;    /* Where to dequeue next msg ptr */
    BYTE   queued_ctr;          /* Count of msg's queued in FIFO */
    
    BYTE Que(new_ptr)
        BYTE *new_ptr;
    {
        BYTE Pass=0xff;
    
        /*  Don't enqueue NUL messages.
         */
        if (*new_ptr != 0)
        {
            /*  If the FIFO queue has room, enqueue new_ptr at
             *  the tail.  Disabe serial interrupts while
             *  accessing FIFO queue controls.
             */
            ES0 = 0;
    
            if (queued_ctr != FIFO0SIZE)
            {
                queued_ctr++;
                *tail_ptr = new_ptr;
    
                /*  Advance the tail, wrapping to the start of
                 *  the FIFO array if necessary.
                 */
                if (++tail_ptr == &fifo0[FIFO0SIZE])
                    tail_ptr = fifo0;
    
                Pass = 0;
            }
    
            ES0 = 1;
    
            /*  out0_busy is FALSE only when the transmit ISR
             *  has nothing to transmit (e.g., after either
             *  init or transmitting the last message's last
             *  non-NUL byte).
             */
            if (!out0_busy)
            {
                out0_busy = 1;          /* Will be busy after TI is set. */
                byte_ptr = *head_ptr;
                TI = 1;
            }
        }
    
        return Pass;
    }
    
    void Serial_Port0(void) interrupt COM0_VECT
    {
        BYTE b;
    
        if(RI) {
            RI=0; // extract recieved byte
            Inkey(SBUF0);
        }
    
        if (TI)
        {
            TI = 0;
            b = *byte_ptr++;
    
            if (b)
                SBUF0 = b;
            else
            {
                /*  At end of message.  If more messages in the
                 *  FIFO, advance the head to the next one
                 *  (wrapping if necessary) and start
                 *  transmitting from there.  Otherwise,
                 *  indicate that the transmit ISR has shut
                 *  down.
                 */
                if (--queued_ctr)
                {
                    if (++head_ptr == &fifo0[FIFO0SIZE])
                        head_ptr = fifo0;
    
                    byte_ptr = *head_ptr;
                    SBUF0 = *byte_ptr++;
                }
                else
                    out0_busy = 0;
            }
        }
    }
    Hope this helps.

    --Dan Henry

  • 1.
    If someday some code should be added between

    ES0 = 1;
    and
    if (!out0_busy) 
    and before entry the
    if (!out0_busy) 
    block, all the data in the buffer were send out, and the out0_busy was cleared, so the program will entry the block code and sent the old message again.

    I think change the line's
    ES0 = 1;
    position like the followings
            if (!out0_busy)
            {
                out0_busy = 1;          /* Will be busy after TI is set. */
                byte_ptr = *head_ptr;
                TI = 1;
            }
            ES0 = 1;
    It will be safe enough.
    However it is also Ok for the original code, just an unnecessary suggestion.
    2.
    I think the followings code
                if (--queued_ctr)
                {
                    if (++head_ptr == &fifo0[FIFO0SIZE])
                        head_ptr = fifo0;
    
                    byte_ptr = *head_ptr;
                    SBUF0 = *byte_ptr++;
                }
                else
                    out0_busy = 0;
    
    should be changed like the followings
                if (++head_ptr == &fifo0[FIFO0SIZE])
                    head_ptr = fifo0;
                if (--queued_ctr)
                {
                    byte_ptr = *head_ptr;
                    SBUF0 = *byte_ptr++;
                }
                else
                    out0_busy = 0;
    
    In this way the head_ptr pointer will be point to the next new message.
    In another hand you can adjust the code in the que function.
    Sorry for my cool English.

  • Thank you all for your help, Andrew Neil, Dan Henry and Bob Mu. I do appreciate the time and effort you spent looking at my code and making suggestions. Dan, you are correct about ?1? the mspace casting and the loss of the Generic Pointers. It was an artifact from some of the code changes I was making while I was desperately trying to figure this out. As to ?2? the variable x is properly maintained in the hardware version.

    Note: This did inspire me to go back to my original system that this Serial routine was taken from and change the uC from an Atmel 89c55 to a Philips 89c51rd+ which has additional internal RAM accessed with a register bit and xdata/movx. My original code on the original hardware does work with both data and xdata buffers. Dan may be right, my problem may be related to the EZ-USB FX.

    Another Note. I do agree that a circular buffer is the traditional way to go. My problem was I was out of RAM. A circular buffer requires the Head and Tail pointer variable in addition to the buffer. If I used one more byte of RAM the code crashes due to Stack conflict. The original system communicated through the serial port at 300 baud to a system that did not use a UART. Instead it used a pin on a uP. Therefore not only was it a low baud rate but I had to put a delay between messages while it processed the previous message. The initial message is so long it takes about 3 minutes at 300 baud to complete the down load. All of these requirements led me to this style of serial communications.

    Bob you are correct about ES0 = 1. I had already made the change you first suggested and narrowed the last problem down to the if(--queued_ctr) . The problem was as long as you continue to stuff the buffer and never let it run out, the messages continue to flow. But as soon as you let queued_ctr run down to zero you never get the last message. Particularly troublesome is when you place a short message in the buffer and let a message finish before you try to send another short message you will get the old message out over and over until you stuff the buffer faster than it can be depleted.

    Here is the final version of the circular buffer.
    Note: There are a few global variables I need to maintain for other functions to use.

    #define FIFOSIZE 100
    xdata BYTE *fifo[FIFOSIZE];
    BYTE  *byte_ptr;           /* Advances through message      */
    BYTE **tail_ptr = fifo;    /* Where to enqueue next msg ptr */
    BYTE **head_ptr = fifo;    /* Where to dequeue next msg ptr */
    BYTE   queued_ctr=0;       /* Count of msg's queued in FIFO */
    
    BYTE Que(new_ptr)
    BYTE *new_ptr;
    {
    BYTE idata Pass;
    bit bdata EAstat, ESstat;
    EAstat=EA;
    EA=0;  /* disable all int */
    ESstat=ES;
    ES=0; /* turn off serial int */
    Pass=0xff;
    
        if (*new_ptr != 0) {
            /*  If the FIFO queue has room, enqueue new_ptr at
             *  the tail.  Disable serial interrupts while
             *  accessing FIFO queue controls. */
            if (queued_ctr++ < FIFOSIZE)
            {
                *tail_ptr = new_ptr;
    
                /*  Advance the tail, wrapping to the start of
                 *  the FIFO array if necessary. */
                if (++tail_ptr == &fifo[FIFOSIZE]) tail_ptr = fifo;
    	    if(queued_ctr>=FIFOSIZE) SBuff_Full=1;
                Pass = 0;
            }
            /*  out0_busy is FALSE only when the transmit ISR
             *  has nothing to transmit (e.g., after either
             *  init or transmitting the last message's last
             *  non-NUL byte). */
            if (!out_busy)
            {
                out_busy = 1;          /* Will be busy after TI is set. */
                byte_ptr = *head_ptr;
                TI = 1;
            }
        }
    ES=ESstat;
    EA=EAstat;
    return Pass;
    }
    
    /* **************** UART I/O *********************** */
    void Serial_Port0(void) interrupt 4
    {
        BYTE b;
    
        if(RI) {
            RI=0; // extract received byte
            Inkey(SBUF);
        }
    
        if (TI) {
            TI = 0;
            b = *byte_ptr++;
            if((*byte_ptr==0)) {
    	    ES=0;
    	    Throttle(25);	/* delay = 25 x DelayTick*/
    			        /* 1/4 Sec = 25 x 10mSec */
    	    TR0=1; /* Turn on timer 0 */
    	    ET0=1; /* Enable Timer 0 interrupts */
    	}
            if (b) {
                SBUF = b;
    	    out_busy=1;
    	}
            else
            {
                /*  At end of message. If more messages in the
                 *  FIFO, advance the head to the next one
                 *  (wrapping if necessary) and start
                 *  transmitting from there. Otherwise,
                 *  indicate that the transmit ISR has shut
                 *  down. */
    	    out_busy = 0;
                if (++head_ptr == &fifo[FIFOSIZE]) head_ptr = fifo;
                if (--queued_ctr) {
                    byte_ptr = *head_ptr;
                    SBUF = *byte_ptr++;
    		out_busy = 1;
                }
            }
        }
    }
    
    Now I need to get back to the EZ-USB FX problem.
    Thanks once again for your help.

    Bob Dillon