AT89C51program runs using Keil C but not SDCC

  • don89c

    don89c - 2007-08-09

    I've been working on converting a simple C program and compiling it using SDCC instead of Keil C.  The program uses the AT89C51 watchdog timer, the serial port, and blinks an LED.  When compiled using Keil, the program works perfectly.  The program simply outputs 21 exes ('X') in response to receiving a 'D' from a notebook PC.  To compile the program using SDCC, I first converted the AT89C51xDS header file from Atmel so that sfr and sbit lines had the correct syntax.  I also toggled the the LED bit using ! rather than ~.  Finally, I used packihx to create the hex file.

    The program responds to receiving a 'D' from the notebook PC, but it only transmits 8 characters instead of 21.

    Any suggestions?  Thanks for your help in advance!!

    With apologies for the clutter, I've attached the code below:

    #define UART_TXD_DELAY_COUNT 10 // transmit delay cnt => 40*6/30MHz = 8mics/cnt for 115200 bps
    #define UNIT_TIMER_RELOAD 0x06 //250 mac. cycles/super timer cnt or 50mics res. at 30MHz

    #define MAX_WAIT 4400  //4400*50mics=220ms conversion time at 30 mHz

    #define LED0_PORT_BIT P2_0 // setup/power and while(1) loop cycle flip-flop LED

    #define WATCHDOG_ON // comment this line out to turn off watchdog timer for debugging

    #include "AT89C51xD2SDCC.h"  // From Atmel: definition for the register names of the AT89C51xD2 chips

    unsigned long data g_ul_clock_count=0;

    unsigned long data main_clock_count=0;

    // Interrupt Vectors and Interrupt Support Functions

    // timer 0 super clock count interrupt routine
    void timer_0_interrupt(void) interrupt 1 using 3
    TF0=0; // reset interrupt flag
    if(!(~(((unsigned char data *)(&g_ul_clock_count))[3])))
      if(!(~(((unsigned char data *)(&g_ul_clock_count))[2])))
       if(!(~(((unsigned char data *)(&g_ul_clock_count))[1])))
        if(!(~(((unsigned char data *)(&g_ul_clock_count))[0]))) g_ul_clock_count=0;

    // UART Serial Port Interrupt 4 Function (input: RxD)(output: TxD)
    //  with SBUF (serial port) array output (256 bytes max) for interrupt 4
    void UART_interrupt(void) interrupt 4 using 2
    unsigned char UART_data=0;
    unsigned char array_index=0;
    unsigned long delay_index=0;

    if(RI==1) // if reception occurs
      RI=0; // clear reception flag for next reception
      UART_data=SBUF; // read receive data
       for(array_index=0; array_index<=20; array_index++)
        SBUF='X'; // send each byte to UART
        while(delay_index<UART_TXD_DELAY_COUNT) delay_index++;
    } else TI=0; // if emission occurs, clear emission flag for next emission
    #ifdef WATCHDOG_ON
    WDTRST=0x1E; // reset watchdog sequence
    WDTRST=0xE1; // reset watchdog sequence

    // Initialize relevant system devices and defaults
    void setup_ThermoMIC(void)
    // CPU Clock in X2 mode (peripherals are also at X2 or 6 clocks per mac. cyc.)

    // UART settings using the internal baud rate generator
    PCON|=0x80; // SMOD1=1, selects double baud rate mode for UART in mode 1,2,3
    SCON=0x50; // UART in mode 1 (8-bit), REN=1
    BDRCON&=0xEC; // BRR=0, SRC=0
    BDRCON|=0x0E; // TBCK=1, RBCK=1, SPD=1
    BRL=0xF0; // 115200 bps at 30MHz in X2 mode

    // Watchdog time-out setup
    // For mode X1: 12 clock periods per machine cycle
    // For mode X2:  6 clock periods per machine cycle
    WDTPRG|=0x07; // (2^(14+0x07)-1) machine cycles [0x07=MAX] or 0.419s @ Fosc=30MHz & X2

    // setup timer 0 in mode 2 with auto-reload & software gate
    //  to create a 100 microsec unit time count
    TMOD&=0xF0; // timer 0 mode 2 with software gate
    TMOD|=0x02; // GATE0=0, C/T0#=0, M10=1, M00=0
    TH0=UNIT_TIMER_RELOAD; // reload value
    TL0=UNIT_TIMER_RELOAD; // init value
    IPL0|=0x02; // set timer 0 interrupt priority to 3 (highest)
    IPH0|=0x02; // set timer 0 interrupt priority to 3 (highest)

    // after setups are done then enable interrupts and start peripherals
    ES=1; // enable UART serial port interrupt
    ET0=1; // enable timer 0 interrupt
    EA=1; // enable all interrupts (global mask)
    TR0=1; // run timer 0
    SPCON|=0x40; // run SPI
    BDRCON|=0x10; // run internal baud rate generator

    // watchdog restart sequence
    void restart_watchdog(void)
    #ifdef WATCHDOG_ON

    // superclock count reset
    void reset_superclock(void)
    ET0=0; // disable timer 0 interrupt
    ET0=1; // enable timer 0 interrupt

    // reset program clock count vars
    void reset_clock_counts(void)

    // Main entry point of looping controller program

    void main(void)
    unsigned char indexer=0;
    unsigned char check_flag=0;

    // setup functions

    // primary loop
    while(g_ul_clock_count<main_clock_count) { restart_watchdog(); } // wait if early


      while(g_ul_clock_count<main_clock_count) { restart_watchdog(); } // wait if early

    • Jesus Calvino-Fraga

      ISRs in sdcc are non-reentrant!  To see exactly what is going on check the generated asm code.  In your code the variables UART_data, array_index, and  delay_index are reset to zero everytime the ISR is activated.  So, when a byte is transmited inside the loops of the ISR, a new interrupt is generated and the variables controling the loop are reset to zero messing up everything!   Either make the variables static, or even better don't initialize the variables at all.  I tested your program with actual hardware with an AT89C51ED2 running at 22 MHZ. It sends 21 'X' everytime I press 'D'.  Some changes I made to your program:

      1) Used SDCC's <at89c51ed2.h> after fixing a typo in the declaration of CKCON0.  I'll be updating the subversion repository shortly.

      2) BRL=256-12; // 115200 bps at 22MHz in X2 mode

      3) #define UART_TXD_DELAY_COUNT 20 //Because of slower clock

      4) Commented out //#define WATCHDOG_ON

      5) The most important, I think:

      void UART_interrupt(void) interrupt 4 using 2
      unsigned char UART_data; //Do not initialize!!!
      unsigned char array_index; //Do not initialize!!!
      unsigned long delay_index; //Do not initialize!!!

      [your code here...]

      (By the way, I don't know exactly what do you want to do with your code, but the ISR looks ugly and dangerous!)


    • don89c

      don89c - 2007-08-10


      Thanks for your suggestions.  I'll try them out.

      The ISR is not mine.  I'm attempting to use someone else's code.  I'll check out some examples and attempt to clean up the code.

      Thanks for the help!


    • Maarten Brock

      Maarten Brock - 2007-08-12

      Jesus said:
      Either make the variables static, or even better don't initialize the variables at all.

      I don't agree at all. Not initializing makes for bad programming practices. They should be static. That's what the keyword was invented for.

      Even if all variables in SDCC are already sort-of static with the non-reentrant default behaviour, they can still be overlayed! If you just don't initialize them you still risk losing the contents. You were lucky SDCC knows interrupt local variables should not be overlayed, but using it inside a function called from the ISR could easily put you in trouble.


    • don89c

      don89c - 2007-09-06


      I've just now had a chance to try your suggestions.  But the program still does not work for me.

      You mentioned that you used SDCC's <at89c51ed2.h> after fixing a type in the declaration of CKCON0.  Where can I find SDCC's version <at89c51ed2.h>, and what is the typo that you fixed in the declaration of CKCON0?


    • don89c

      don89c - 2007-09-06


      Thanks.  I downloaded at89c51ed2.h and modified the #include line in my c source file to include this file name.  I also added the statement  P2_1=!P2_1;  to the interrupt.  Every time I transmit a "D" to the 89c51, the LED connected to P2_1 toggles on or off.  However, I still get only 8 characters back, and none of them are "X".

      I'm sure I'm doing something stupid, but I'm not sure what it is.


    • don89c

      don89c - 2007-09-06


      The program works fine if I change the line #define UART_TXD_DELAY_COUNT 10 to #define UART_TXD_DELAY_COUNT 40 to allow more time between writing each byte to the UART.  For some reason, a greater delay between writing bytes is necessary when using the SDCC compiler versus the Keil compiler.  Not sure why this is the case!



Log in to post a comment.