Electronic – 8051 serial routine returns all 1s despite apparently valid data being on the bus

8051debuggingmicrocontrollerrtcuart

So, I am trying to receive synchronous serial data from a DS1306 in 3-wire mode using mode 0 of my AT89LP51ED2's USART. However, my serial read routine is returning all 1s instead of valid data, even though valid data appears to be on the serial bus according to what I see on my oscilloscope, the 32kHz oscillator is going normally, and Vdd and Vbat are both present. Writing appears to be OK, though, although it's pretty hard to tell with this read issue — there is also a NHD-0216K3Z-FL-GBW LCD module on the bus set to 3-wire/SPI mode. It pulls the serial line up with 10kΩ, and writing to it works as well, using an independent chip select signal.

Why is this happening, and how do I fix it?

schematic

Serial frame (we are reading the time/date regs from 0x0 through 0x6):
overall shot of serial frame

Address byte:
address byte

First data byte:
data byte 0

Second data byte:
data byte 1

Third data byte:
data byte 2

Fourth data byte:
data byte 3

Fifth data byte:
data byte 4

Sixth data byte:
data byte 5

Seventh data byte:
data byte 6

Representative leading edge closeup (taken from the first byte):
representative data/clock leading edge closeup

Representative trailing edge closeup (also taken from the first byte):
representative data/clock trailing edge closeup

Code, using SDCC for the 8051 (the main() is a highly simplified MWE main(), the rest of it is direct from the firmware with a few irrelevant pieces stripped out and comments added in):

#include <stdint.h>
#include <stdio.h>
#include <ctype.h>
#include <stdnoreturn.h>
#include <stdbool.h>
#include <string.h>

void gpio_init();
void serial_init();
void lcd_init();
void read_and_print_rtc();

// Peripheral stuff
#include <at89c51ed2.h>
__sfr __at (0xE6) P0M0;
__sfr __at (0xE7) P0M1;
__sfr __at (0xD6) P1M0;
__sfr __at (0xD7) P1M1;
__sfr __at (0xCE) P2M0;
__sfr __at (0xCF) P2M1;
__sfr __at (0xC6) P3M0;
__sfr __at (0xC7) P3M1;
__sfr __at (0xBE) P4M0;
__sfr __at (0xBF) P4M1;

__sfr __at (0x87) PCON;
    #define SMOD1 0x80
    #define SMOD0 0x40

noreturn int main()
{
  gpio_init();
  serial_init();
  lcd_init();

  while (true)
    read_and_print_rtc();
}

typedef union 
{
    uint8_t buf[7];
    struct
    {
        uint8_t sc;
        uint8_t mi;
        uint8_t hr;
        uint8_t wd;
        uint8_t dy;
        uint8_t mo;
        uint8_t yr;
    } tm;
} rw_time_t;

rw_time_t tb;

void rtc_input(uint8_t addr, uint8_t* chars, int n);

void read_and_print_rtc(void)
{
    memset(tb.buf, 0, sizeof(tb));
    // Read the time & date in one go to avoid rollover woes
    rtc_input(0x00, tb.buf, 7);
    // debug dump output
    printf("\f%x%x%x", tb.buf[0], tb.buf[1], tb.buf[2]);
    printf("\n%x%x%x", tb.buf[4], tb.buf[5], tb.buf[6]);
}

void gpio_init(void) __reentrant
{
    // We set up the ports as follows:
    // P0: quasi-bidir, default FF
    // P1: quasi-bidir, default FF
    // P2: totem pole, default 0
    // P3.0: quasi-bidir, default 1
    // P3.1: totem pole, default 1
    // P3.2: input only
    // P3.3-P3.5: totem pole, default 0
    // P3.6-P3.7: totem pole, default 1
    // P4.4-P4.7: totem pole, default 1

    // Everything starts up in quasi-bidir as TRIPORT is cleared
    // Our port latches also start up as 0xFF as well
    P0 = 0xFF;
    P1 = 0xFF;
    P2 = 0x0;
    P3 = 0xC7;
    P4 = 0xFF;
    P0M0 = 0x0;
    P0M1 = 0x0;
    P1M0 = 0x0;
    P1M1 = 0x0;
    P2M0 = 0x0;
    P2M1 = 0xFF;
    P3M0 = 0x04;
    P3M1 = 0xFA;
    P4M0 = 0x0;
    P4M1 = 0xFF;
}

void serial_init(void)
{
    // We put the 8051 USART in Mode 0 for this as we are talking to
    // SPI-ish devices (DS1306, SPI-ish LCD) -- SMOD0 in PCON is off
    // by default, which is good, because we don't need FE anyway; 
    // however, we need to set SMOD1 which not only means the BRG runs 
    // at 2x speed but also makes the Rx sample on the rising edge of 
    // the serial clock when SM2=1 for driving the RTC
    PCON |= SMOD1;

    // SM2 is also set to 0 by default, which means the clock idles HIGH
    // This is what the LCD expects, but the RTC expects idle LOW -- 
    // writing to the USART requires a CPOL (SM2) flip before driving SS

    // We set up the BRG for an 83kHz clock rate -- its the fastest 
    // rate we can get from the BRG that meets the 100kHz max on the LCD
    BRL = 196;
    BDRCON = BRR | TBCK | RBCK | SPD | SRC;
}

const int MAXCOLS = 16;
const int MAXROWS = 2;

void _move_cursor(int r, int c);
void _visual_bell(void);

void lcd_output(uint8_t* chars, int n);

#define CONST const __code

void putchar(char c)
{
    // Assume that the LCD has been init'ed with the screen cleared 
    // and the cursor homed the first time through
    static int row = 0, col = 0;
    // The dummy bytes are there to enforce a time delay
    CONST uint8_t cls_cmd[] = {0xFE, 0x51, 0x20, 0x20, 0x20, 0x20, 0x20,
        0x20, 0x20, 0x20, 0x20};
    // 16 spaces to clear a line
    CONST uint8_t clear_line_cmd[] = {
        0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
        0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
    };
    CONST uint8_t bksp_cmd[] = {0xFE, 0x4E};
    switch (c)
    {
    // Alarm bell -- we need some sort of visual bell here
    case '\a':
        _visual_bell();
        break;
    // Backspace -- this does what you think it does, although backspacing
    // from column 0 needs to be handled specially
    case '\b':
        if (col > 0)
        {
            --col;
            lcd_output(bksp_cmd, sizeof(bksp_cmd)/sizeof(bksp_cmd[0]));
        }
        else
        {
            if (row > 0)
            {
                // Move the cursor to the end of the previous line
                _move_cursor(--row, col = MAXCOLS);
                --col;
                lcd_output(bksp_cmd, sizeof(bksp_cmd)/sizeof(bksp_cmd[0]));
            }
            else
            {
                // Backspace from 0,0 is no good -- visual bell instead
                _visual_bell();
            }
        }
        break;
    // Form feed -- clears the screen and homes the cursor
    case '\f':
        row = 0, col = 0;
        lcd_output(cls_cmd, sizeof(cls_cmd)/sizeof(cls_cmd[0]));
        // Backspace out an extra space left over from adjusting the 
        // timing on the clear screen command
        // This is a kludge -- but we don't know the exact timing on 
        // clearing the screen on this module, so it's the best we can do
        lcd_output(bksp_cmd, sizeof(bksp_cmd)/sizeof(bksp_cmd[0]));
        break;
    // Newline -- we follow the UNIX convention here as moving to the
    // next line without going back to col 0 is pretty useless and lame
    case '\n':
        row = (row + 1) % MAXROWS;
        col = 0;
        _move_cursor(row, col);
        lcd_output(clear_line_cmd,
            sizeof(clear_line_cmd)/sizeof(clear_line_cmd[0]));
        _move_cursor(row, col);
        break;
    // Carriage return -- go back to col 0 without moving lines
    case '\r':
        col = 0;
        _move_cursor(row, col);
        break;
    // VT and HT aren't supported as I have no idea what to do with them
    // Someone tried to sneak a LCD command escape through STDIO -- 
    // that's a no-no, so eat it as we can't return EOF because SDCC's
    // putchar decl is silly and non-conformant
    case '\xfe':
        return;
    // Not a character we handle, so just fling it out to the LCD
    default:
        if (col >= MAXCOLS) // wrap to the next line
        {
            row = (row + 1) % MAXROWS;
            col = 0;
            _move_cursor(row, col);
            lcd_output(clear_line_cmd,
                sizeof(clear_line_cmd)/sizeof(clear_line_cmd[0]));
            _move_cursor(row, col);
        }
        lcd_output(&c, 1);
        ++col;
        break;
    }
    return;
}

void _visual_bell(void)
{
}

void _move_cursor(int r, int c)
{
    // note, this can't live in code space or be a const as we scribble
    // on it to prepare the actual command
    uint8_t move_cmd[] = {0xFE, 0x45, 0x00};
    if (r >= MAXROWS)
        r = MAXROWS;
    if (c >= MAXCOLS)
        c = MAXCOLS;
    move_cmd[2] = ((r & 0x03) << 6) | (c & 0x3F);
    lcd_output(move_cmd, sizeof(move_cmd)/sizeof(move_cmd[0]));
}

void lcd_init(void)
{
    // the display defaults ON already, but force it ON because this may
    // be called with the display OFF in a warm restart situation
    // we also clear the screen here
    CONST uint8_t cmd[] = {0xFE, 0x41, 0xFE, 0x51, 0x20, 0x20, 0x20, 0x20,
        0x20, 0x20, 0x20, 0x20};
    lcd_output(cmd, sizeof(cmd)/sizeof(cmd[0]));
}

uint8_t rev_bits(uint8_t c);
// Write a blob of data to the LCD -- this doesn't care about control
// characters etal
void lcd_output(uint8_t* chars, int n)
{
    int i = 0;
    // make sure SM2 is cleared FIRST (CPOL=idle HIGH)
    SM2 = 0;
    // drive the slave select for the LCD
    P4_6 = 0;
    for (; i < n; ++i)
    {
        // transmit a byte -- swap the bits in it as the LCD wants data
        // MSB-first, but the 8051 USART sends LSB-first only
        SBUF = rev_bits(chars[i]);
        // wait for it to fly out the door as we don't really have
        // anything better to do
        while (!TI);
        TI = 0;
    }
    // turn off the slave select
    P4_6 = 1;
}

// Reverse the bits in a byte using some clever bit fiddling
// Swaps nybbles first, then two-bit chunks, then individual adjacent bits
uint8_t rev_bits(uint8_t c) 
{
    c = (c & 0xF0) >> 4 | (c & 0x0F) << 4;
    c = (c & 0xCC) >> 2 | (c & 0x33) << 2;
    c = (c & 0xAA) >> 1 | (c & 0x55) << 1;
    return c;
}

// Note that I've tried having this code set the port pin as an input-only pin for the duration of reception from the RTC, but to no avail
// Read a blob of data from the RTC chip -- takes care of the address too
void rtc_input(uint8_t addr, uint8_t* chars, int n)
{
    int i = 0;
    // make sure SM2 is set FIRST (CPOL = idle LOW)
    SM2 = 1;
    // drive the slave select for the RTC chip
    P4_7 = 0;
    // transmit the address byte with the MSB forced to 0
    SBUF = addr & 0x7F;
    // wait for it to finish
    while (!TI);
    TI = 0;
    REN = 1;
    RI = 0;
    for (; i < n; ++i)
    {
        // wait for a byte to come in -- the RTC sends LSB-first, too
        // an interrupt driven serial routine would add complexity for
        // very little gain here -- the keyboard interrupt is the only
        // thing we have to remain responsive to
        while (!RI);
        RI = 0;
        chars[i] = SBUF;
    }
    REN = 0;
    // turn off the slave select
    P4_7 = 1;
}

Extra credit: why is there a runt pulse at the end of the address byte, and how can I get rid of it?

runt pulse at full height

runt pulse closeup

Best Answer

And this is why SPI is but only a convention...

The AT89LP51ED2 USART, for all it has up its sleeve, simply cannot be convinced to sample the serial data line mid-cycle (i.e. while SCK is solidly HIGH), which is what it takes to receive correct data from the DS1306's 3-wire mode. The only solution I was able to find, after hours of dogged work with the Bus Pirate and the MWE from the above example, was to bitbang the serial receive, as in the code below, which is a modified version of the MWE above. (Sampling with SMOD0 = 0 produced results that were off by a bit or two, as did trying to edge-sample while bitbanging.) Of course, neither the DS1306 datasheet nor Maxim appnote 3510 say this -- in fact, the code listing in the appnote edge-samples!

#include <stdint.h>
#include <stdio.h>
#include <ctype.h>
#include <stdnoreturn.h>
#include <stdbool.h>
#include <string.h>


// Peripheral stuff
#include <at89c51ed2.h>
__sfr __at (0xE6) P0M0;
__sfr __at (0xE7) P0M1;
__sfr __at (0xD6) P1M0;
__sfr __at (0xD7) P1M1;
__sfr __at (0xCE) P2M0;
__sfr __at (0xCF) P2M1;
__sfr __at (0xC6) P3M0;
__sfr __at (0xC7) P3M1;
__sfr __at (0xBE) P4M0;
__sfr __at (0xBF) P4M1;

__sfr __at (0x87) PCON;
    #define SMOD1 0x80
    #define SMOD0 0x40

void gpio_init();
void serial_init();
void lcd_init();
void read_and_print_rtc();

noreturn int main()
{
  uint16_t i;
  gpio_init();
  serial_init();
  lcd_init();

  while (true)
  {
    read_and_print_rtc();
    for (i = 1; i; ++i);
  }
}

typedef union 
{
    uint8_t buf[7];
    struct
    {
        uint8_t sc;
        uint8_t mi;
        uint8_t hr;
        uint8_t wd;
        uint8_t dy;
        uint8_t mo;
        uint8_t yr;
    } tm;
} rw_time_t;

rw_time_t tb;

void rtc_input(uint8_t addr, uint8_t* chars, int n);

void read_and_print_rtc(void)
{
    memset(tb.buf, 0, sizeof(tb));
    // Read the time & date in one go to avoid rollover woes
    rtc_input(0x00, tb.buf, 7);
    // debug dump output
    printf("\f %2x%2x%2x", tb.buf[0], tb.buf[1], tb.buf[2]);
    printf("%2x%2x%2x", tb.buf[4], tb.buf[5], tb.buf[6]);
}

void gpio_init(void) __reentrant
{
    // We set up the ports as follows:
    // P0: quasi-bidir, default FF
    // P1: quasi-bidir, default FF
    // P2: totem pole, default 0
    // P3.0: quasi-bidir, default 1
    // P3.1: totem pole, default 1
    // P3.2: input only
    // P3.3-P3.5: totem pole, default 0
    // P3.6-P3.7: totem pole, default 1
    // P4.4-P4.7: totem pole, default 1

    // Everything starts up in quasi-bidir as TRIPORT is cleared
    // Our port latches also start up as 0xFF as well
    P0 = 0xFF;
    P1 = 0xFF;
    P2 = 0x0;
    P3 = 0xC7;
    P4 = 0xFF;
    P0M0 = 0x0;
    P0M1 = 0x0;
    P1M0 = 0x0;
    P1M1 = 0x0;
    P2M0 = 0x0;
    P2M1 = 0xFF;
    P3M0 = 0x04;
    P3M1 = 0xFA;
    P4M0 = 0x0;
    P4M1 = 0xFF;
}

void serial_init(void)
{
    // We put the 8051 USART in Mode 0 for this as we are talking to
    // SPI-ish devices (DS1306, SPI-ish LCD) -- SMOD0 in PCON is off
    // by default, which is good, because we don't need FE anyway; 
    // however, we need to set SMOD1 which not only means the BRG runs 
    // at 2x speed but also makes the Rx sample on the rising edge of 
    // the serial clock when SM2=1 for driving the RTC
    PCON |= SMOD1;

    // SM2 is also set to 0 by default, which means the clock idles HIGH
    // This is what the LCD expects, but the RTC expects idle LOW -- 
    // writing to the USART requires a CPOL (SM2) flip before driving SS

    // We set up the BRG for an 83kHz clock rate -- its the fastest 
    // rate we can get from the BRG that meets the 100kHz max on the LCD
    BRL = 196;
    BDRCON = BRR | TBCK | RBCK | SPD | SRC;
}

const int MAXCOLS = 16;
const int MAXROWS = 2;

void _move_cursor(int r, int c);
void _visual_bell(void);

void lcd_output(uint8_t* chars, int n);

#define CONST const __code

void putchar(char c)
{
    // Assume that the LCD has been init'ed with the screen cleared 
    // and the cursor homed the first time through
    static int row = 0, col = 0;
    // The dummy bytes are there to enforce a time delay
    CONST uint8_t cls_cmd[] = {0xFE, 0x51, 0x20, 0x20, 0x20, 0x20, 0x20,
        0x20, 0x20, 0x20, 0x20};
    // 16 spaces to clear a line
    CONST uint8_t clear_line_cmd[] = {
        0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
        0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
    };
    CONST uint8_t bksp_cmd[] = {0xFE, 0x4E};
    switch (c)
    {
    // Alarm bell -- we need some sort of visual bell here
    case '\a':
        _visual_bell();
        break;
    // Backspace -- this does what you think it does, although backspacing
    // from column 0 needs to be handled specially
    case '\b':
        if (col > 0)
        {
            --col;
            lcd_output(bksp_cmd, sizeof(bksp_cmd)/sizeof(bksp_cmd[0]));
        }
        else
        {
            if (row > 0)
            {
                // Move the cursor to the end of the previous line
                _move_cursor(--row, col = MAXCOLS);
                --col;
                lcd_output(bksp_cmd, sizeof(bksp_cmd)/sizeof(bksp_cmd[0]));
            }
            else
            {
                // Backspace from 0,0 is no good -- visual bell instead
                _visual_bell();
            }
        }
        break;
    // Form feed -- clears the screen and homes the cursor
    case '\f':
        row = 0, col = 0;
        lcd_output(cls_cmd, sizeof(cls_cmd)/sizeof(cls_cmd[0]));
        // Backspace out an extra space left over from adjusting the 
        // timing on the clear screen command
        // This is a kludge -- but we don't know the exact timing on 
        // clearing the screen on this module, so it's the best we can do
        lcd_output(bksp_cmd, sizeof(bksp_cmd)/sizeof(bksp_cmd[0]));
        break;
    // Newline -- we follow the UNIX convention here as moving to the
    // next line without going back to col 0 is pretty useless and lame
    case '\n':
        row = (row + 1) % MAXROWS;
        col = 0;
        _move_cursor(row, col);
        lcd_output(clear_line_cmd,
            sizeof(clear_line_cmd)/sizeof(clear_line_cmd[0]));
        _move_cursor(row, col);
        break;
    // Carriage return -- go back to col 0 without moving lines
    case '\r':
        col = 0;
        _move_cursor(row, col);
        break;
    // VT and HT aren't supported as I have no idea what to do with them
    // Someone tried to sneak a LCD command escape through STDIO -- 
    // that's a no-no, so eat it as we can't return EOF because SDCC's
    // putchar decl is silly and non-conformant
    case '\xfe':
        return;
    // Not a character we handle, so just fling it out to the LCD
    default:
        if (col >= MAXCOLS) // wrap to the next line
        {
            row = (row + 1) % MAXROWS;
            col = 0;
            _move_cursor(row, col);
            lcd_output(clear_line_cmd,
                sizeof(clear_line_cmd)/sizeof(clear_line_cmd[0]));
            _move_cursor(row, col);
        }
        lcd_output(&c, 1);
        ++col;
        break;
    }
    return;
}

void _visual_bell(void)
{
}

void _move_cursor(int r, int c)
{
    // note, this can't live in code space or be a const as we scribble
    // on it to prepare the actual command
    uint8_t move_cmd[] = {0xFE, 0x45, 0x00};
    if (r >= MAXROWS)
        r = MAXROWS;
    if (c >= MAXCOLS)
        c = MAXCOLS;
    move_cmd[2] = ((r & 0x03) << 6) | (c & 0x3F);
    lcd_output(move_cmd, sizeof(move_cmd)/sizeof(move_cmd[0]));
}

void lcd_init(void)
{
    // the display defaults ON already, but force it ON because this may
    // be called with the display OFF in a warm restart situation
    // we also clear the screen here
    CONST uint8_t cmd[] = {0xFE, 0x41, 0xFE, 0x51, 0x20, 0x20, 0x20, 0x20,
        0x20, 0x20, 0x20, 0x20};
    lcd_output(cmd, sizeof(cmd)/sizeof(cmd[0]));
}

uint8_t rev_bits(uint8_t c);
// Write a blob of data to the LCD -- this doesn't care about control
// characters etal
void lcd_output(uint8_t* chars, int n)
{
    int i = 0;
    // make sure SM2 is cleared FIRST (CPOL=idle HIGH)
    SM2 = 0;
    // drive the slave select for the LCD
    P4_6 = 0;
    for (; i < n; ++i)
    {
        // transmit a byte -- swap the bits in it as the LCD wants data
        // MSB-first, but the 8051 USART sends LSB-first only
        SBUF = rev_bits(chars[i]);
        // wait for it to fly out the door as we don't really have
        // anything better to do
        while (!TI);
        TI = 0;
    }
    // turn off the slave select
    P4_6 = 1;
}

// Reverse the bits in a byte using some clever bit fiddling
// Swaps nybbles first, then two-bit chunks, then individual adjacent bits
uint8_t rev_bits(uint8_t c) 
{
    c = (c & 0xF0) >> 4 | (c & 0x0F) << 4;
    c = (c & 0xCC) >> 2 | (c & 0x33) << 2;
    c = (c & 0xAA) >> 1 | (c & 0x55) << 1;
    return c;
}

// Read a blob of data from the RTC chip -- takes care of the address too
void rtc_input(uint8_t addr, uint8_t* chars, int n)
{
    int i = 0, j;
    uint8_t r = 0, t;
    // drive the slave select for the RTC chip
    P3_1 = 0;
    P4_7 = 0;
    // transmit the address byte with the MSB forced to 0
    addr &= 0x7F;
    for (j = 0; j < 8; ++j)
    {
        P3_0 = 0;
        if (addr & 0x1)
            P3_0 = 1;
        P3_1 = 1;
        P3_1 = 0;
        addr >>= 1;
    }
    for (; i < n; ++i)
    {
        // Bitbang a byte in, we can't use the hardware serial support
        // for this because there's no way to make the USART sample in
        // the middle of a SCK cycle
        P3_0 = 1;
        for (j = 0; j < 8; ++j)
        {
            P3_1 = 1;
            t = (uint8_t) P3_0;
            t <<= 7;
            r >>= 1;
            r |= t;
            P3_1 = 0;
        }
        chars[i] = r;
        r = 0;
    }
    // turn off the slave select
    P4_7 = 1;
    P3_1 = 1;
}
Related Topic