Pages: 1 2 [3]   Go Down
Author Topic: Watchdog ISR in assembly language  (Read 8003 times)
0 Members and 1 Guest are viewing this topic.
nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 129
Posts: 8530
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

This thread is dead but I'll summarise for future generations.

Yes it's easy to include an ASM file in an ISR using the following

ISR (WDT_vect, ISR_NAKED) {
    #include "my_asm_file.s"
}

The included file must have every line wrapped in asm(""); and there is a Python script to do that written by robtillaart.

To do the same job (SPI bit bang 4 bytes) an ASM version is 10x faster than using the Arduino abstraction layer and quite a lot faster than using direct port manipulation (26uS vs 45uS).

That's it in a nutshell.

Thanks guys.

______
Rob


« Last Edit: February 17, 2011, 10:12:33 pm by Graynomad » Logged

Rob Gray aka the GRAYnomad www.robgray.com

SF Bay Area (USA)
Online Online
Tesla Member
***
Karma: 134
Posts: 6763
Strongly opinionated, but not official!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

It seems to be pretty common for C compilers to be "not very good" at dealing with this sort of "shift a single bit from an IO register into a variable" sort of operation; they're just not very happy dealing with any quantity smaller than the smallest register (eg 8 bits on almost every architecture still in existence.)  However:

Code:
  for (i = 0; i < 8; i++) {
    PORTB |= (1<<PB5);
    cmd |= (PINB >> PB4) & 1;
    cmd <<= 1;
    PORTB &= ~(1<<PB5);
  }
It turns out that this would be significantly smaller and faster written as:
Code:
  for (i = 0; i < 8; i++) {
    PORTB |= (1<<PB5);
    if (PINB & 1<<PB4) {
      cmd |= 1;
    }
    cmd <<= 1;
    PORTB &= ~(1<<PB5);
  }
Apparently the compile isn't able to recognize the first as a bit isolation technique, and actually goes through with shifting the input value however many times, while the latter manages to become a "skip if bit in io register clear" instruction.
Logged

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 129
Posts: 8530
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Very interesting westfw, I did some timing from start of first clock pulse to end of last.

First version of the code 18.4uS
Second version of the code 8.5uS

Just a slight improvement smiley

BTW, there's a bug in my example code, the "cmd <<= 1;" should be moved to the top of the loop.

Code:
for (i = 0; i < 8; i++) {
   cmd <<= 1;
   PORTB |= (1<<PB5);
    if (PINB & 1<<PB4) {
      cmd |= 1;
    }
 //   cmd <<= 1; // not here
    PORTB &= ~(1<<PB5);
  }

Or there is a shift done after the last bit and the value received is 2x that sent. (I remember now had the same bug with my ASM version but I'm a slow learner).

Thanks for delving into that westfw, it looks like we've got a nice tight shiftIn() function (albeit hardcoded) for others to use as well.

PS, I've modified my summary above to reflect this.

______
Rob
« Last Edit: February 17, 2011, 09:59:50 pm by Graynomad » Logged

Rob Gray aka the GRAYnomad www.robgray.com

SF Bay Area (USA)
Online Online
Tesla Member
***
Karma: 134
Posts: 6763
Strongly opinionated, but not official!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Could you post the full code for the final version for comparison purposes?
Logged

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 129
Posts: 8530
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

It's still a work in progess and a bit rough

Code:
//
// Acts as an SPI master
// Receives 3 bytes, CMD, ADDR_LO, ADDR_HI
// performs an action based on the CMD
// returns the results as a fourth byte
//
// Still very rough, a work in progress.
//

//#define RUNNING_ON_328
#define RUNNING_ON_2560

//#define  VERSION_ASM
#define  VERSION_C


#ifdef RUNNING_ON_328
#define ATMON_PORT_PIN_MOSI  3
#define ATMON_PORT_PIN_MISO  4
#define ATMON_PORT_PIN_SCK   5

#define ATMON_ARD_PIN_MOSI  11
#define ATMON_ARD_PIN_MISO  12
#define ATMON_ARD_PIN_SCK   13
#define ATMON_ARD_PIN_DEBUG 8
#endif

#ifdef RUNNING_ON_2560
#define ATMON_PORT_PIN_MOSI  2
#define ATMON_PORT_PIN_MISO  3
#define ATMON_PORT_PIN_SCK   1

#define ATMON_ARD_PIN_MOSI  51
#define ATMON_ARD_PIN_MISO  50
#define ATMON_ARD_PIN_SCK   52
#define ATMON_ARD_PIN_DEBUG 53
#endif

#define    CMD_POLL  1
#define    CMD_RD    2
#define    CMD_WR    3

#define  ACK    6
#define  NAK    15

// known location to read from
byte  test_byte = 0x23;


#ifdef VERSION_ASM
ISR (WDT_vect, ISR_NAKED) {
  #include "d:\work\atmon\atmon\atmon_isr.s"
}
#endif

#ifdef VERSION_C
ISR (WDT_vect) {
  volatile byte cmd = 0;
  volatile byte addr_hi = 0;
  volatile byte addr_lo = 0;
  byte response;
  byte *address;
  int i;

  asm ("wdr");
 
 
  // get the command byte from other Arduino
 for (i = 0; i < 8; i++) {
    cmd <<= 1;
    PORTB |= (1 << ATMON_PORT_PIN_SCK);     
    if (PINB & 1 << ATMON_PORT_PIN_MISO) {
      cmd |= 1;
    }
    PORTB &= ~(1 << ATMON_PORT_PIN_SCK);
  }
  delayMicroseconds (5); // needed to allow the other Arduino some time to load another byte
 
  // get the low address byte from other Arduino
  for (i = 0; i < 8; i++) {
    addr_lo <<= 1;
    PORTB |= (1 << ATMON_PORT_PIN_SCK);
    if (PINB & 1 << ATMON_PORT_PIN_MISO) {
      addr_lo |= 1;
    }
    PORTB &= ~(1 << ATMON_PORT_PIN_SCK);
  }
  delayMicroseconds (5);
 
  // get the high address byte from other Arduino
  for (i = 0; i < 8; i++) {
    addr_hi <<= 1;
    PORTB |= (1 << ATMON_PORT_PIN_SCK);
    if (PINB & 1 << ATMON_PORT_PIN_MISO) {
      addr_hi |= 1;
    }
    PORTB &= ~(1 << ATMON_PORT_PIN_SCK);
  }
 
  address = (byte*)((addr_hi << 8) + addr_lo);
 
  // action the command
  switch (cmd) {
    case  CMD_POLL:
      response = ACK;
      break;
 
    case  CMD_RD:
      response = *address;
      break;
 
    case  CMD_WR:
      response = ACK; // not implemented yet
      break; 
     
    default:
      response = NAK;
      break; 
  }

   // send response byte to other Arduino 
   for (i = 0; i < 8; i++) {
     if ((response & 0x80) == 0)
        PORTB &= ~(1 << ATMON_PORT_PIN_MOSI);
     else
        PORTB |= (1 << ATMON_PORT_PIN_MOSI);
 
    PORTB |= (1 << ATMON_PORT_PIN_SCK);
    response <<= 1;
    PORTB &= ~(1 << ATMON_PORT_PIN_SCK);
  }
}
#endif

void setup () {
 
  WDTCSR = (1<<WDIE);
 
  Serial.begin (115200);
 
  pinMode(ATMON_ARD_PIN_MOSI, OUTPUT);
  pinMode(ATMON_ARD_PIN_MISO, INPUT);
  pinMode(ATMON_ARD_PIN_SCK, OUTPUT);
  pinMode(ATMON_ARD_PIN_DEBUG,OUTPUT);
 
};

void loop () {
  // pulse to allow measuring of the timeout from main code
   PORTB |= (1);
   PORTB &= ~(1);
};

I cleaned it up a bit, hopefully I didn't break anything while doing so.

______
Rob
Logged

Rob Gray aka the GRAYnomad www.robgray.com

Pages: 1 2 [3]   Go Up
Jump to: