Sunday, August 2, 2015

Ralph's rant: non-portable AVR code

One thing I like about AVR MCUs is that in addition to instruction-set, a number of them have some degree of I/O register-level compatibility.  For example, both the ATtiny85  and ATtiny84a have PORTB at I/O address 0x18.  Because of this, I was able to write my 64-byte picoboot bootloader, which uses a soft UART on PB1, so that a single binary works on both the tiny85 and tiny84.

I recently though I could take advantage of the register-level compatibility between the ATmega328p and ATmega168p in my arduino compatible picoboot bootloader.  The source is already identical, and the only difference in the binary files is the flash start address and the signature bytes reported.  My idea was to build a version which returned the signature bytes of the 328p, but that loaded on the 168p.  When flashed to a 168p, it would look like a Uno to the Arduino IDE, so people could switch between a 328p board and a 168p board without having to modify the boards.txt file.  Obviously projects with a code size larger than 16Kb wouldn't work, but for everything else, I thought it was a great idea.  But it didn't work.

The bootloader would initially work OK; clicking Upload in the Arduino IDE would seem to upload the code to the 168p board when Uno was selected as the target, but the uploaded code wouldn't work.  I double-checked the fuse settings for the 168p.  I flashed the board back to the regular 168p bootloader, selected my modified pro mini 168 target in the boards menu, and uploaded.  Everything worked fine, so there was nothing wrong with the board.  I compared the disassembly of the normal 168p bootloader and my 168p masquerade bootloader as I was calling it; the only difference was the signature bytes reported.  I even reviewed the 168p/328p datasheet in case I missed an important difference - and found nothing.

I then decided to verify that the bootloader was properly flashing the uploaded code and hadn't somehow corrupted the flash.  I uploaded a basic blink program using the 168p masquerade bootloader, then connected a USBasp to read back the full contents of the flash, including the bootloader:
avrdude -c usbasp -C /etc/avrdude.conf -p m168p -U flash:r:flash168masq.hex:i

Then I used avr-objcopy to convert the hex file to elf:
avr-objcopy -I ihex flash168masq.hex -O elf32-avr flash168.elf

Finally, I used avr-objdump to disassemble the elf file:
avr-objdump -D flash168.elf

The reset vector was a jump to 0x00ae:
       0:       0c 94 57 00     jmp     0xae    ;  0xae
      ae:       11 24           eor     r1, r1
      b0:       1f be           out     0x3f, r1        ; 63
      b2:       cf ef           ldi     r28, 0xFF       ; 255
      b4:       d8 e0           ldi     r29, 0x08       ; 8
      b6:       de bf           out     0x3e, r29       ; 62

      b8:       cd bf           out     0x3d, r28       ; 61

The code at 0x00ae first clears the zero register (r1), then clears SREG(0x3f).  Clearing SREG is redundant since section 7.3.1 of the datahsheet shows that SREG is always cleared after reset.  Clearing it again wasn't going to cause any problems though.  The next four instructions initialize the stack (SPL and SPH).  I immediately recognized this as the problem.  I described how this was redundant in Trimming the fat from avr-gcc code.  In this case it wasn't redundant, it was wrong!  Since avr-gcc thought it was generating code for a m328p, it included the (normally just redundant) code to initialize the stack to 0x08FF.  But on the m168p, the end of RAM, and therefore the reset value of the stack pointer, is 0x04FF.  With an improperly initialized stack, it was obvious why programs uploaded to the 168p masquerading as a 328p weren't working.

So the superfluous code emitted by avr-gcc not only wastes space, it interferes with releasing binary code that runs on a number of different AVR MCUs.  I think it also demonstrates the dangers of developers writing code with a "it shouldn't hurt," attitude rather than a "is it necessary?" attitude.  I don't know who first said it, but it was a wise man that recognized when building a project you should include everything necessary but nothing more.

No comments:

Post a Comment