AVR128DA bootloader issues...

Log in or register to post comments
Go To Last Post
9 posts / 0 new
Author
Message
Sothis
Sothis's picture
Level: Hangaround
Joined: Thu. Oct 3, 2013
Posts: 140 View posts
#1
Posted by Sothis: Wed. Oct 14, 2020 - 05:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi all, I'm currently porting a bootloader from the Atmega644 to the AVR128DA using the 'AN3341: Basic Bootloader for the AVR DA MCU Family' as a guide. I get two amazingly descript errors: "confused by earlier errors, bailing out" & "local fram unavailable (naked function?), both of which point to an I2C read function in the main bootloader code. I'm starting the boot code in the ".ctor" section as described in the app note and I think I just don't know how to code this properly. The little I've seen on what these errors mean it could be the c code in the 'naked' function, which the Atmel bootloader code gets away with, or functions not being inlined properly (although I'm not sure that applies here without inline functions). Here's what I've got so far:
 

__attribute__((naked)) __attribute__((section(".ctors"))) void boot(void)
{
	// Initialize system for AVR GCC support, expects r1 = 0
	asm volatile("clr r1");

	fram_init();		// Initializes FRAM

	// Read AVR EEPROM for start condition
	bootStatus = fram_readByte(&FRAMBoot.bootStatus);

	// Check start normal or finalizing
	if (bootStatus == START_NORMAL)
	{
		// Enable Boot Section Lock
		NVMCTRL.CTRLB = NVMCTRL_BOOTRP_bm;

		// Jump to OS
		pgm_jmp_far (APPCODE_START / sizeof(uint16_t));
	}

	// Execute bootloader
	bootloader();
}

This code above just checks FRAM for a byte that says to start the bootloader or normally. The boot loader itself is:
 

void bootloader (void)
{
	//#define BOOT_READY 10
	uint8_t handshakeStatus = USB_HS_NONE;
	uint8_t command;
	uint8_t statusCmd = CMD_SEND_FW_PACKET;

	cli();				// Disable Interrupts
	bootInit();			// Initialize bootloader (Pins)
	i2c_initMaster();	// Initializes the i2c bus

	setOutPinHigh();	// Tells PC we are ready after restart

	for(;;)
	{
		// Listen for USB handshake
		if((~USB_PIN_PORT.IN & USB_IN_PIN_MSK) && (handshakeStatus == USB_HS_RESET))
		{
			handshakeStatus = usb_HandshakeRespond();
		}

		// Get Task
		if(handshakeStatus == USB_HS_HANDSHAKE)
		{
			handshakeStatus = USB_HS_TASK;
			USBReadMessage(FTDI_7BIT_ADDR, &command, SIZE_GET_USB_CMD); <- Problem is here

			switch(command)
			{
				case CMD_GET_FW_VER:
					usb_GetFirmwareVersionCommand();
					break;

				case CMD_GET_DATE_UPDATED:
					usb_GetDateLastUpdated();
					break;

				...

The problem occurs at the USBReadMessage function and if I comment that out it compiles. The USBReadMessage is just an I2C read assembled from the Fleury driver that I ported to the DA series TWI.

 

Any clue what is going on? And, do I even have to start the bootloader this way or can it be done normally with main and not omitting the function prologue and epilogue as shown? Seems like that would be a bit simpler.

This topic has a solution.

Tags:

AVR Microcontrollers, AVR and XMEGA, solved
Last Edited: Thu. Oct 15, 2020 - 09:28 PM
Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 3071 View posts
Posted by curtvm: Thu. Oct 15, 2020 - 12:52 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I would just start with a new project like you are coding any other normal project. Set the BOOTSIZE fuse to something that gives you plenty of room, then write your boot app as you wish. Eventually you figure out the size required and adjust the BOOTSIZE accordingly. If getting another free page of code space is important (512 bytes), then you can tinker with it like they do and eliminate the vectors (and startup code).

 

Not sure why -they- jump right into squeezing every last byte out of the bootloader, when that can wait until needed. What also is missing when you do the -nostartfiles is the data initialization startup functions, which is fine until you forget that none of your data is initialized like a normal app.

Last Edited: Thu. Oct 15, 2020 - 01:17 AM
Top
Sothis
Sothis's picture
Level: Hangaround
Joined: Thu. Oct 3, 2013
Posts: 140 View posts
Posted by Sothis: Thu. Oct 15, 2020 - 01:18 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Been messing around with this all day and found some interesting things:
 

a. I can get the above code to compile by getting rid of naked function and putting the boot code in main instead of bootloader(). However, this same code compiles much smaller on the Atmega644.

b. If I comment out the offending function call, the code will compile and is around the same code size as it was on the old Atmega. Thus, the naked approach appears to save around 1kB in program memory.
c. Downloaded the Atmel bootloader code and began adding my TWI and FRAM libraries and had no problems compiling, even when adding the fram_init and readByte to the naked function. And the USBReadMessage() call that seemed to be causing the problem compiles when added to the bootloader function like in my code above.

 

I'd think this is down to something small but I don't see anything obvious!

 

Top
Sothis
Sothis's picture
Level: Hangaround
Joined: Thu. Oct 3, 2013
Posts: 140 View posts
Posted by Sothis: Thu. Oct 15, 2020 - 01:29 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm wrote:
I would just start with a new project like you are coding any other normal project. Set the BOOTSIZE fuse to something that gives you plenty of room, then write your boot app as you wish. Eventually you figure out the size required and adjust the BOOTSIZE accordingly. If getting another free page of code space is important (512 bytes), then you can tinker with it like they do and eliminate the vectors (and startup code).

Thank you curtvm! Ya, you're right... I kind of get in that mindset where if I'm not being as efficient as I can then it's bad code. This is afterall a 128 for a project that just barely required the 644...there's plenty of space.

 

curtvm wrote:
Not sure why -they- jump right into squeezing every last byte out of the bootloader, when that can wait until needed. What also is missing when you do the -nostartfiles is the data initialization startup functions, which is fine until you forget that none of your data is initialized like a normal app.

I have no experience messing with this kind of stuff or naked functions and whatnot. It seems like getting rid of the interrupt vectors would be nice since my code doesn't use them for the bootloader. I'm not sure what, but something in that startup code seems to take up quite a bit of program memory.

Top
westfw
westfw's picture
Level: Raving Lunatic
Joined: Wed. Jun 19, 2002
Posts: 4641 View posts
Location: SF Bay area
This reply has been marked as the solution. 
Posted by westfw: Thu. Oct 15, 2020 - 05:30 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

bailing out" & "local fram unavailable (naked function?)

__attribute__((naked)) __attribute__((section(".ctors"))) void boot(void) {

	// Execute bootloader
	bootloader();
}

 

void bootloader (void) {
	uint8_t command;
	 :
			USBReadMessage(FTDI_7BIT_ADDR, &command, SIZE_GET_USB_CMD); <- Problem is here

 

I would guess that:

  1. the compiler "obviously" in-lines the tail-chained bootloader() into boot()
  2. bootloader() then becomes "naked" as well.
  3. So it doesn't have a stack frame to use for local variables.
  4.  that would be fine since all the local variables fit in registers, anyway.
  5.  EXCEPT you can't do &variable if variable is in a register (certainly not on DA series!  maybe not ever?)

 

IF I'm right, there are a couple of ways to fix this:

  1. make cmd a global (or static) variable instead of local.
  2. prevent bootloader() from being inlined with an  __attribute__((noinline)) declaration.

 

 

the naked approach appears to save around 1kB in program memory.

 That SOUNDS like your original approach is somehow preventing the inclusion of standard libraries and startup code.  I didn't know that was possible, just by putting stuff in the .ctors section.

 

Normally, that would be done at link time with "-nostartfiles -nostdlib"

Optiboot uses section .init8 (plus the linker commands.)

 

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 3071 View posts
Posted by curtvm: Thu. Oct 15, 2020 - 12:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>What also is missing when you do the -nostartfiles is the data initialization startup functions

 

I did get that wrong, the vectors and the simple stack init/clear r1 come from the crt.o file, and libgcc has the data init. With the way the crt.o file now has some linker info about memory it also gets a little more difficult than it used to be to discard the crt.o file via nostartfiles.

 

 

Top
Sothis
Sothis's picture
Level: Hangaround
Joined: Thu. Oct 3, 2013
Posts: 140 View posts
Posted by Sothis: Thu. Oct 15, 2020 - 03:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

westfw wrote:

IF I'm right, there are a couple of ways to fix this:

  1. make cmd a global (or static) variable instead of local.
  2. prevent bootloader() from being inlined with an  __attribute__((noinline)) declaration.

Thank you westfw for the amazing response and that seemed like it could be it! I tried both and actually the ((noinline)) get rid of the two errors but instead we got a "ld returned 1 exit status" error. Moving command to be a global variable did not help. Odd.

 

I think what I might do is just take the Atmel project and rename it, then slowly add my code to until I either find the offending code via an error or the whole thing compiles. Nothing is really jumping out comparing their code to mine, so I wonder if its not something about the way the project is set up or something. I'm just not advanced enough a programmer to know what else to do.

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 3071 View posts
Posted by curtvm: Thu. Oct 15, 2020 - 06:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I tried the AN3341 bootloader project they have on Github, imported into MPLABX. It takes some hoop-jumping to make it work there, at least.

 

I renamed boot to main because it did not like the missing main, I moved the utils/ header files to the project folder so they could be found, and had to modify the specs file because the linker did not have the __DATA_REGION_ORIGIN__ value from the now missing crt.o file, and complained about the data memory origin.

 

I did get it to compile (a DA48), and the resulting size was 894 bytes. Without -nostartfiles it was 1158 bytes so in this case makes it a 3 page bootloader from a 2 page. Since -nodefaultlibs was not used, you still get the data/bss init code (not used) and the jump to main (which is why the complaint for the missing main). With also using -nodefaultlibs the byte count was 878.

 

Why this is all worth the saved 264 bytes, I don't know. Like all byte counting, it only matters when it matters, and it matters not very often.

Top
Sothis
Sothis's picture
Level: Hangaround
Joined: Thu. Oct 3, 2013
Posts: 140 View posts
Posted by Sothis: Thu. Oct 15, 2020 - 09:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

westfw wrote:
prevent bootloader() from being inlined with an  __attribute__((noinline)) declaration.

Actually I tried a bunch of stuff today and even replacing all of the Atmel code worked until the last little bit caused the same errors. I finally went back and tried this again and it compiled! I'm not sure what the hell else I did that made it work. Thank you sooooooooo much, west fw!!

 

curtvm wrote:
Why this is all worth the saved 264 bytes, I don't know. Like all byte counting, it only matters when it matters, and it matters not very often.

Ya, now that it compiles I'm getting only about this amount of savings over when it was working without the ((naked)) function. Definitely not worth it aside from the experience.

 

I wonder, why would they go that route given the AVR gcc docs on naked attributes state: " While using extended asm or a mixture of basic asm and C code may appear to work, they cannot be depended upon to work reliably and are not supported. "?? That rule gets broken in line 2 of their code and it seems like based on your and my experiences it's going to cause a lot of people headaches.

 

 

Top