Go Down

Topic: Watchdog ISR in assembly language (Read 8822 times) previous topic - next topic

Graynomad

#30
Feb 17, 2011, 09:34 am Last Edit: Feb 18, 2011, 04:12 am by Graynomad Reason: 1
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


Rob Gray aka the GRAYnomad www.robgray.com

westfw

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: [Select]
  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: [Select]
  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.

Graynomad

#32
Feb 18, 2011, 03:45 am Last Edit: Feb 18, 2011, 03:59 am by Graynomad Reason: 1
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 :)

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

Code: [Select]
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
Rob Gray aka the GRAYnomad www.robgray.com

westfw

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

Graynomad

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

Code: [Select]

//
// 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
Rob Gray aka the GRAYnomad www.robgray.com

Go Up