stefanrueger / urboot

Small AVR bootloader using urprotocol
GNU General Public License v3.0
55 stars 8 forks source link

Bootloader not running (request help with troubleshooting) #25

Closed DRSDavidSoft closed 9 months ago

DRSDavidSoft commented 9 months ago

Hi again,

Sorry to bother you, I've ran into an issue where urboot is not blinking the LED after it is burned on the chip. I'm using an ATmega32 SMD chip, which I know is not officially supported, but I would appreciate it if you could please take a quick look at my process and let me know if I'm doing something wrong here.

I'm trying to run urboot on my custom PCB now, but the bootloader doesn't seem to run as the LED is not blinking upon RESET. I've verified that Optiboot works correctly and blinks the LED fine, and I've even uploaded a couple of sketches to make sure the electrical side of things are properly working.

The pre-built ones that I tried are as follows:

I also tried to build one myself using the following make flags:

$ make MCU=atmega32 URPROTOCOL=0 SFMCS=AtmelPB4 LED=AtmelPB0 DUAL=1 FRILLS=7 NAME=mightycore32 F_CPU=16000000L AUTOBAUD=1 EEPROM=1 WDTO=1S PROTECTME=1 UARTNUM=0 RX=AtmelPD0 TX=AtmelPD1 VBL=0 PGMWRITEPAGE=1 CHIP_ERASE=1 BLINK=2

Now, I have verified the following:

The avr-gcc builds with:

./avr-toolchain/5.4.0/bin/avr-gcc -DSTART=0x7c00UL -DRJMPWP=0xcf07 -Wl,--section-start=.text=0x7c00 -Wl,--section-start=.version=0x7ffa -D_urboot_AVAILABLE=434 -g -Wundef -Wall -Os -fno-split-wide-types -mrelax -mmcu=atmega32 -DF_CPU=16000000L -Wno-clobbered -DWDTO=1S -DAUTOBAUD=1 -DLED=AtmelPB0 -DBLINK=2 -DDUAL=1 -DSFMCS=AtmelPB4 -DEEPROM=1 -DURPROTOCOL=0 -DFRILLS=7 -DVBL=0 -DCHIP_ERASE=1 -DUARTNUM=0 -DTX=AtmelPD1 -DRX=AtmelPD0 -DPGMWRITEPAGE=1 -Wl,--relax -nostartfiles -nostdlib -o mightycore32.elf urboot.c
./avr-toolchain/5.4.0/bin/avr-objcopy -j .text -j .data -j .version --set-section-flags .version=alloc,load -O ihex mightycore32.elf mightycore32.hex
./avr-toolchain/5.4.0/bin/avr-objdump -h -S mightycore32.elf > mightycore32.lst
-- 590 1024 u7.7 wesdhprac mightycore32.hex

The raw content of the built HEX file is:

(Click to expand...)
:107C000011248FE598E09EBF8DBFC4B72C2E14BE03
:107C100080E0EBD0C1FD3FC0C3FF3CC080E588BB26
:107C200090EB97BB8DB9E0E0F0E0C8EFC49883E03B
:107C3000C7D080E0C5D08F2FC3D08E2FC1D000E633
:107C400010E080E0BDD0D8018D938D01A03EB1053C
:107C5000C1F7C49A309739F480916100843919F0E2
:107C6000807F803CA1F4D5D0E058FF4FC150F1F6A1
:107C7000C49886E0A5D0C49AC49804E080E2A0D05D
:107C800080E00150E1F7C49A8EE0AFD0FFCF1DB87D
:107C900017BA18BAB5C18EE0A8D0AFE7BFEF809988
:107CA000FECF9096809BFDCFB9B91197F1F782E096
:107CB0008BB988E18AB98ED0813429F48BD081E0E8
:107CC00098D087E011C0823411F485E175C0853405
:107CD00011F486E071C0853749F481E08AD08EE1E5
:107CE00074D085E972D082E070D067C08635C1F467
:107CF00071D0D82F6FD0C82F6DD06CD081E079D0E3
:107D0000DC3A11F08FEFF0CFC038E1F7E0E0FCE7AC
:107D1000E058F109A89583E090D03097C9F7F2CFE9
:107D2000853539F457D0E82F55D0F82FEE0FFF1FC7
:107D300042C0843611F54DD0C82F4CD0F82EDC2F20
:107D400000E610E047D0D8018D938D01D150D1F7D6
:107D500081E04FD0B6E4FB1671F0A0E6B0E0E19907
:107D6000FECFFFBBEEBB8D918DBBE29AE19A3196BF
:107D7000C150A9F722C04DD020C08437C1F429D00A
:107D8000C82F28D0D82F81E034D0D63459F0E199CB
:107D9000FECFFFBBEEBBE09A8DB317D03196C1503A
:107DA000B1F70BC0859111D0C150E1F706C0813504
:107DB00011F488E01AD081E01CD080E106D07BCF9E
:107DC0008FB9779BFECF8FB108955D9BFECF8CB9A5
:107DD000089500D0C09AB89A8BB187FFFDCF84FD7B
:107DE0000FC0A8958CB1C098089598E191BD81BD50
:107DF0000895CF93C82FEEDFC150E9F7803219F014
:107E000088E0F3DFFFCF84E1CF91DFCFFB01DC011E
:107E100002C0A0E6B0E060E86A0F9F01FC3790F472
:107E200083E00BD081E00D901D9007D03296A61311
:107E3000FACFF90185E001D081E187BFE89507B667
:087E400000FCFDCF11240895A0
:067FFA00080007CFD13F93
:0400000300007C007D
:00000001FF

Since the built file is 590 bytes, it requires the 1024 BOOT SZ (starting at $3C00), so I'm using 0xC2 as High Fuse, and also 0x3F as Low Fuse.

Putting these into avrdude 6.x and 7.x (under Windows) I get the following command:

/bin/avrdude -C /etc/avrdude.conf -v -p atmega32 -c usbasp -U flash:w:mightycore32_urboot.hex:i -U lfuse:w:0x3F:m -U hfuse:w:0xC2:m -U efuse:w:0xFF:m   

(Update: I also tried toggling FUSE_BOOTRST which made the hfuse to be C3 instead of C2 but this also didn't work)

This successfully programs the ATmega32 using USPasp, and I get the following:

(Click to expand...)
avrdude: Version 7.1-arduino.1
         Copyright the AVRDUDE authors;
         see https://github.com/avrdudes/avrdude/blob/main/AUTHORS

         System wide configuration file is MightyCore\hardware\avr\2.2.2\avrdude.conf

         Using Port                    : usb
         Using Programmer              : usbasp
         AVR Part                      : ATmega32
         Chip Erase delay              : 9000 us
         PAGEL                         : PD7
         BS2                           : PA0
         RESET disposition             : possible i/o
         RETRY pulse                   : SCK
         Serial program mode           : yes
         Parallel program mode         : yes
         Timeout                       : 200
         StabDelay                     : 100
         CmdexeDelay                   : 25
         SyncLoops                     : 32
         PollIndex                     : 3
         PollValue                     : 0x53
         Memory Detail                 :

                                           Block Poll               Page                       Polled
           Memory Type Alias    Mode Delay Size  Indx Paged  Size   Size #Pages MinW  MaxW   ReadBack
           ----------- -------- ---- ----- ----- ---- ------ ------ ---- ------ ----- ----- ---------
           eeprom                  4    10    64    0 no       1024    4      0  9000  9000 0xff 0xff
           flash                  33     6    64    0 yes     32768  128    256  4500  4500 0xff 0xff
           lfuse                   0     0     0    0 no          1    1      0  2000  2000 0x00 0x00
           hfuse                   0     0     0    0 no          1    1      0  2000  2000 0x00 0x00
           lock                    0     0     0    0 no          1    1      0  2000  2000 0x00 0x00
           signature               0     0     0    0 no          3    1      0     0     0 0x00 0x00
           calibration             0     0     0    0 no          4    1      0     0     0 0x00 0x00

         Programmer Type : usbasp
         Description     : USBasp, http://www.fischl.de/usbasp/

avrdude: auto set sck period (because given equals null)
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9502 (probably m32)
avrdude: Note: flash memory has been specified, an erase cycle will be performed.
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: auto set sck period (because given equals null)
avrdude: reading input file mightycore32_urboot.hex for flash
         with 590 bytes in 2 sections within [0x7c00, 0x7fff]
         using 6 pages and 178 pad bytes
avrdude: writing 590 bytes flash ...

Writing | ################################################## | 100% 0.10 s

avrdude: 590 bytes of flash written
avrdude: verifying flash memory against mightycore32_urboot.hex

Reading | ################################################## | 100% 0.03 s

avrdude: 590 bytes of flash verified
avrdude: reading input file 0xcf for lock
         with 1 byte in 1 section within [0, 0]
avrdude: writing 1 byte lock ...
avrdude: 1 byte of lock written
avrdude: verifying lock memory against 0xcf
avrdude: 1 byte of lock verified
avrdude: reading input file 0xC2 for hfuse
         with 1 byte in 1 section within [0, 0]
avrdude: writing 1 byte hfuse ...
avrdude: 1 byte of hfuse written
avrdude: verifying hfuse memory against 0xC2
avrdude: 1 byte of hfuse verified
avrdude: reading input file 0x3F for lfuse
         with 1 byte in 1 section within [0, 0]
avrdude: writing 1 byte lfuse ...
avrdude: 1 byte of lfuse written
avrdude: verifying lfuse memory against 0x3F
avrdude: 1 byte of lfuse verified

avrdude done.  Thank you.

Which means that the hfuse, lfuse, lockbit and the flash are all successfully written and verified. However, the LED doesn't blink at all upon reset :( Additionally, I didn't try the serial uploading since the LED didn't work.

I'm really sad that I couldn't get this to work. Can you please help me find the problem, and potentially fix it? I'd appreciate it very much! 😄

Thank you for your help regarding building and burning urboot!

P.S. Sorry for the rather lengthy post, I tried to cover each and every case that I could think of as the issue.

mcuee commented 9 months ago

I will suggest you try the other urboot hex files before to see if they work (not STK500v1).

DRSDavidSoft commented 9 months ago

Thanks @mcuee I did, they didn't work either (URPROTOCOL=1). I strongly suspect the problem is with my fuses.

stefanrueger commented 9 months ago

What is your Vcc? The BOD is set at 4V, so if Vcc = 3.3 V then you'd have a constant brownout reset.

$  avrdude -qqc dryrun -U lfuse:w:0x3F:m -U hfuse:w:0xC2:m -p m32 -t
avrdude> config
config sut_cksel=exthifxtalres_16kck_64ms # 63
config boden=bod_enabled # 0
config bodlevel=bod_4v0 # 0
config bootrst=boot_section # 0
config bootsz=bs_1024w # 1
config eesave=ee_preserved # 0
config ckopt=full_railtorail # 0
config spien=isp_enabled # 0
config jtagen=jtag_disabled # 1
config ocden=ocd_disabled # 1
config lb=no_lock # 3
config blb0=no_lock_in_app # 3
config blb1=no_lock_in_boot # 3
avrdude> 

The configuration values look sane otherwise. Your process also looks good.

stefanrueger commented 9 months ago

LED is not blinking upon RESET

Note that urboot blinks the LED in a different way to optiboot: For each serial character that the bootloader reads the LED is switched on and off. Optiboot in contrast blinks the LED for a few times on reset, but the code for this is a lot more involved. So I suggest you try uploading a program which triggers flickering of the LED as long as there is serial traffic.

I need STK500v1 functionality

There should be no need for that. Use avrdude -c urclock and you'll be fine without STK500v1 functionality (which is quite wasteful in terms of code size).

DRSDavidSoft commented 9 months ago

Thanks for the help! The VCC is indeed 5V, so no problem there.

I'm sorry it didn't occur to me to actually test uploading something to the board, it seems that Urboot is doing something. It definitely blinks the LED while the code is being uploaded, although I preferred the blink of Optiboot as PB0 is connected to both an LED and a buzzer in this board.

The problem now is that avrdude uploads successfully, but the code never runs (a simple blink sketch I wrote). Additionally, the bootloader will not accept another firmware after the first upload. avrdude says "not in sync" after the first seemingly successful upload.

Also, the way that Urboot handles the LED is also interesting! Definitely very useful on tiny flash MCUs such as the ATtiny series. Personally I don't mind the huge 1024 bytes bootloader as the app size would reach up to 24K, so the more "frills" that are baked into the bootloader, the better! (I was even planning to drive a LCD in the bootloader mode to provide some feedback).

Lastly, I made all attempts using the avrdude -c urclock mode, as the -c arduino mode doesn't work, even though URPROTOCOL=0. I kind of like and need the backwards compatibility for this board, although I wouldn't mind upgrading the remote host to avrdude 7 so that the urclock is present on those machines as well.

One very minor nitpick, would you please be open to implementing the Optiboot's way of the LED with some "frills" number, please? It's super useful if we already have the 1024 bytes of space to fill it with good & cool functionality! 😅

I'm now going through the documentation again to see if I forgot some important step for Urboot to work. I can't figure out why it behaves like this on the board, it accepts the application firmware but never runs it 😢

Maybe the issue is with the sketch somehow overwriting the bootloader?

Update 1: Hmm... so BLINK=2 was never a valid option according to the documentation, I mistakenly thought it represented how many times should the LED blink, but it only accepts a 0 or 1, which would either make the LED blink during serial communications or not. Interesting way to reduce memory usage!

Update 2: The value of C3 for hfuse works, but C2 doesn't, so if VBL=0 then the bootrst bit must be set. Still can't figure out why I can't upload again past the initial upload, trying to figure it out!

P.S. Sorry for not correctly reading the docs, I should've noticed the difference between Urboot and Optiboot's LED implementation

stefanrueger commented 9 months ago

VBL=0 then the bootrst bit must be set

Correct, ... and the correct BOOTSZ must be set to the correct size, too, in this case.

can't figure out why I can't upload again past the initial upload

This is a conundrum, indeed. If the fuses are set to point reset straight to the bootloader of the correct size, then, well, reset should jump there and the bootloader should be able to upload other programs, again and again. hexls bootloader.hex should show 'h' as a feature (hardware-supported bootloader). Not being able to upload a second time can in theory happen with wrong fuses and vector bootloaders. Let me know how you progress with this one.

-c arduino mode doesn't work, even though URPROTOCOL=0

Can happen, usually if the WDTO is smaller than one second. -c arduino waits for a long time before establishing comms. Backwards compatibility of -c urclock is similarly compromised, as various initial blinking sessions of various bootloaders make it unknown when the bootloader is ready to receive a command. Of course, fuse settings for initial delays to allow the quartz time to spin up into a reliable oscillation pattern also play its part. For this case -c urclock has -xdelay=[ms] (which can be negative) and -xstrict parameters that you can play with so that a particular board works.

5 V

Good. I just noted that the board has a W25Q32 SPI flash, which I thought required 3.3 V.

DUAL=1

I note you are trying bootloaders with and without dual boot feature. I would start playing around without dual boot and, when all works, and you are comfortable with your board and urboot bootloaders then check out dual boot.

DRSDavidSoft commented 9 months ago

Thank you again for the help; indeed I should firstly focus on making this work without the dual boot feature. Regarding the W25Q32 SPI flash, you are correct that it requires 3.3V, and I embarrassingly didn't check this beforehand so I didn't make use of a level shifter.

(I should either remove the chip or provide 3.3V as VCC and adjust BOD to suitable level)

In any case, I'll also forgo the -c arduino functionality for now as well due to the WDTO amount needed, I've set it to 1S which is less than the safe required value it appears.

Can you please explain the -xstrict parameter? I couldn't find any documentation for it aside from this in the code. Sidenote from reading your comments, as mentioned, I kind of like the entertaining 300ms blink of the LED :)

I'll try with this changes, and I hope I can get the Urboot + avrdude to work consistently and reliablely.

Once again, thanks for the help and the great advice!

stefanrueger commented 9 months ago

I kind of like the entertaining 300ms blink of the LED

There is a DEBUG_FREQ option where the bootloader blinks 5 times with the defined frequency after reset. This to enable measurement of the blink frequency and establish how good the onboard resonator/oscillator/quartz is (or see whether a fuse divides the system frequency by 8 etc). Not meant for production, though. And WDTO probably needs to be greater than the time it takes for 5 blinks (so, eg, at 10 Hz DEBUG_FREQ WDTO should be at least 1 s).

-xstrict

If you use urboot bootloaders together with -c urclock then -xstrict is normally not needed; it might be useful to get certain bootloaders (different from urboot/optiboot) to run with the autobaud feature (which eats up the first byte sent from the host).

stefanrueger commented 9 months ago

blinks 5 times

I should add that the ATmega32 doesn't have the PIN toggle functionality and DEBUG_FREQ is untested on that particular chip.

DRSDavidSoft commented 9 months ago

Thanks for the note, good point. If I'm able to figure out the urboot.c code I might change it so the PORT register is written to. Currently I'm trying to understand the code to see if I could change the number of times and the duration for each pulse of the blink, ideally I'd like to receive 2 pulses.

DRSDavidSoft commented 9 months ago

Quick question, for the lockbit fuses (-U lock:w:0xhh:m) should it be 0XFF or 0XCF? It doesn't seem to make a difference at the moment to me.

DRSDavidSoft commented 9 months ago

@stefanrueger Shouldn't Urboot stay in bootloader mode for 1S after reset? I believe the cause of the "not in sync" messages now is that after avrdude opens the port (which causes a reset), immediately the application is ran instead of the bootloader. If it wouldn't jump to application, then the upload should've worked correctly.

stefanrueger commented 9 months ago

Shouldn't Urboot stay in bootloader mode for 1S after reset?

After external reset, yes (but not necessarily after WD reset, power-up reset, BOD reset ...). Does it not? It's programmed to check I/O after external rest and if there is no activity/known command for 1 s (or WDTO) then the board resets via a WD reset and jumps to the application. If the host baud rate is incompatible with the board's (that can happen with a terrible resonator, particularly with 115 kBaud and 16 MHz as that is technically out of specs) then commands that the bootloader sees are garbled and it might decide to go to the application much earlier. So, perhaps a good idea to test the board for stable oscillation and check serial I/O with a sketch that you upload with your usbasp programmer.

stefanrueger commented 9 months ago

lockbit fuses (-U lock:w:0xhh:m) should it be 0XFF or 0XCF? It doesn't seem to make a difference

Correct. Only the low 6 bits are relevant. However, the factory initial value is 0xff so I'd set the top two (unused) bits to be compatible with that:

$ avrdude -pm32/st | grep "\slock\s.*0x"
.ptmm   ATmega32    lock    initval 0xff
.ptmm   ATmega32    lock    bitmask 0x3f
DRSDavidSoft commented 9 months ago

I built a simple bootloader using make MCU=atmega32 NAME=test F_CPU=16000000L BAUD_RATE=9600 WDTO=2S (hexls outputs -- 358 512 u7.7 wes-hpr-- test.hex) to make sure the baud rate is slow and WDTO is 2S. After the first upload using -c urclock, the issue still persists, using an external reset the board is not in bootloader more for 2 seconds, and the application is ran instead (even tried grounding RESET instead of using avrdude and DTR pin to make sure no I/O is happening).

Quick question, what is the default behavior when unexpected data is received by Urboot, will it ignore it for the duration of the WDTO, or will it jump straight to the application? Although I believe we can customize it using the Makefile parameters, if I'm not mistaken.

In any case, now I suspect I'm overwriting the bootloader using avrdude and -c urclock. I say this because the same issue occurs with Optiboot too, when I use -c urclock with Optiboot, the board appears to not have retained the bootloader as it jumps to application upon RESET.

I'm sorry for taking up your time like this, but the help and information you've provided are substantially valuable, so I appreciate your help!

Another side note question, I know bootloaders in general shouldn't respond to WDT resets as it might cause bootloops. Optiboot has some information about what conditions it expects on device boot (no interrupts, timer 1 register reset, etc). Does Urboot also rely on some initial configuration that I need to apply before I can enter it without a hardware reset? This is super useful in case I need to free up one of the GPIO output pins and avoid resetting the chip by tying the RESET pin to the said GPIO pin. Currently, only the DTR pin goes to the RESET pin using the 100nF capacitor.

Thanks for the help!


Update: The issue is with fuses I believe, not being overwritten. The Optiboot issue is solved when correct values for the board are used, C6 for hfuse and BF for lfuse (the difference for lfuse appears to only be BOD related, though). It appears that avrdude writes 0xFF for lock bits before burning bootloader, and then 0xCF right after burning Optiboot.

I hope I'll be able to figure out what I'm doing wrong here.

DRSDavidSoft commented 9 months ago

🎉 Hooray! It works now!

It appears the correct hfuse and lfuse values that I mentioned in the last post (C6 and 3F) works great with Urclock the same way as Optiboot! Out of desperation I tried copying the same values, which turned out to be the key.

So what's the difference? Firstly, BOOTRST should be set as I forgot again (sorry...), but also the BOOTSZ was set from 512 to 256... I'm at a loss. hexls showed a value of 512 for this particular build. I now need to test all the possible permutations of the configuration in order to achieve the best results.

stefanrueger commented 9 months ago

Yes, I just noted that you had set the fuse to 1024 word bootloader size for 1024 bytes. Now you need 256 words for 512 bytes size

DRSDavidSoft commented 9 months ago

My mistake, apologies

stefanrueger commented 9 months ago

All good. I hope you enjoy urboot. And I am sure after a while you will get used to the LED code that only takes 6 bytes of space. In general, -c urclock is the correct uploader for urboot bootloaders. There is no value in stk500v1 compatibility, really once you have access to avrdude -c urclock.

DRSDavidSoft commented 9 months ago

Thank you very much!

stefanrueger commented 9 months ago

Thanks for reaching out @DRSDavidSoft; I have updated the trouble shooting section of the readme file for this case:

stefanrueger commented 9 months ago

It appears that avrdude writes 0xFF for lock bits before burning bootloader, and then 0xCF right after burning Optiboot

This protects the bootloader from being overwritten by hardware. Urboot protects itself from being overwritten by software (but you can also set the lock bits to lock the bootloader region for hardware supported bootloaders)

DRSDavidSoft commented 9 months ago

@stefanrueger Thanks again! Another quick question, is it possible to make use of Timer2 interrupt when we're in Urboot?

stefanrueger commented 9 months ago

Timer2 interrupt when we're in Urboot?

No, far too much trouble

DRSDavidSoft commented 9 months ago

I did some tests without use of timers, and using 32 additional bytes (instead of 6), the old behavior can be achieved:

led_setup();

uint8_t r = 3;

do {
        led_toggle();
        _delay_ms(70);
} while (r--);

Assuming this code comes before "Start of bootloader", and the following macro is used to toggle the pin on ATmega32:

#define led_toggle()    (UR_PORT(LED) ^=  UR_BV(LED))

(Obviously, the led_on() and led_off() calls must be removed in the getch*() functions).

The only downside that I see with this approach (aside from 32 bytes instead of 6) is that while using _delay_ms(), the mcu is unresponsive, thus necessitating the use of -delay=210 (based on the values I chose) or -xstrict switches in avrdude to work. And yes, it's slower by the amount of 200ms to get the thing started :-)

Anyways, in this particular board, the "LED" pin is simultaneously connected to an LED and a buzzer, since I wanted to hear the "double beep" sound at each power on/RESET. This modification achieves just that, and fits in the available space that would otherwise go to waste, and I believe the 200ms of unresponsiveness is totally worth the change - in my case.

If I remove the buzzer, then the 6-bytes official LED behavior is also good, so I'll keep that in mind for future use cases.

Again, thank you @stefanrueger, and sorry for taking up your time!

I'm now going to focus on the timers and interrupts and see if I can make my change non-blocking. I wonder why bootloaders avoid the use of interrupts, maybe other than the space that it takes (e.g. since Urboot has a custom WDT header file to avoid using interrupts during the bootload time). Fascinating stuff!

stefanrueger commented 9 months ago

I don't see initial blinking as bootloader functionality and am not in favour of implementing a FRILL level for this. One reason is waste of code space and another a waste of time-to-reply as you found with the 210 ms delay. urboot bootloaders can tell avrdude meta-data (sketch size, unused flash size that the application might use for storage etc). Try, eg, avrdude -c urclock -xshowall. I consider a zippy response without blinking or beeping very much desirable.

In your own, private, implementation you get almost the same code if you use DEBUG_FREQ=7.1428 apart from that there are 5 blink cycles (rn = 10) instead of 1.5 cycles (that would be your rn =3) 2 cycles (rn = 4). Note that the DEBUG_PIN can be a different pin to the LED. The urboot code is possibly shorter (at the expense of generating the 70 ms with a granularity of 5 CPU cycles instead of 1 CPU cycle). Also note that - owing to the odd number of half cycles - the LED is left on during upload/download if the LED is active-high.

change non-blocking

The code could sense the rx line after each 70 ms delay (I believe optiboot does something similar) and exit the loop if the host starts talking. Or sense rx in the inner counting-down line that _delay_us() resolves to (this changes the timing, though). Needs careful managing of what the state of the LED should be after exiting the loop this way.

bootloaders avoid the use of interrupts

Consider the ramifications: applications want their own use of interrupts, so how should this work? The code can still use the timer hardware but needs to poll the timer flags; I suspect setting up the timer and polling will not give shorter code.

stefanrueger commented 9 months ago

I wanted to hear the "double beep" sound at each power on/RESET

BTW, this happens at each external reset, but not at power-on reset, or?

DRSDavidSoft commented 9 months ago

Thanks for the insights, also fair enough regarding not implementing this. I agree that the additional overhead makes the response not feel as snappy as not including the additional 210 ms delay. Also, if vectors are used instead of hardware bootloaders, it makes sense to keep the amount of the space used to a bare minimum, so the 32 additional bytes doesn't make sense there. Personally, I'm using hardware bootloading in this case.

I'm interested regardign the 7.1428 Hz frequency for the debug pin, why such an odd number? I couldn't quite figure out how the debug frequency generation work by reading the source code. It seems to me there isn't any traditional delay between each pulse, is it only taking CPU cycles to achieve this? Additionally, would you be open to make the rn value also configurable using make and set 5 to be the default value? I know it's used for "debugging" purposes, especially for the oscilator/resonator, but the functionality is also useful for this case.

owing to the odd number of half cycles - the LED is left on during upload/download if the LED is active-high

That's odd! The led is active high, but using a r = 3 the led doesn't stay on during upload/download. I think this is due to the do-while loop and using the r-- operation instead of --r operation, there is actually four iterations instead of three.

I used a WDTO of 2S and delay of 150ms just to be sure and tested it out again.

The code could sense the rx line after each 70 ms delay

This is quite an interesting way! I also noticed that Optiboot only blinks two times if there is no I/O activity, but if there is, it only blinks one time. Must be the exact same thing, or something similar at least.

Needs careful managing of what the state of the LED should be after exiting the loop this way.

Of course, a simple led_off() call after breaking from the outer loop would fix this. (And costs 2 bytes)

applications want their own use of interrupts, so how should this work? The code can still use the timer hardware but needs to poll the timer flags

Can you please explain this some more? The bootloader code needs to poll timer flags for what reason?

This is the code that I was trying to set up to blink the led:

#include <avr/io.h>
#include <avr/interrupt.h>

volatile uint8_t ledTimer = 0, ledCount = 0;

void main() {
    // ...
    TIMSK = (1 << OCIE2); // Enable Timer2 compare interrupt
    TCCR2 = (1 << CS22) | (1 << CS20); // Start Timer2 with prescaler 128
    // ...
}

ISR(TIMER2_COMP_vect) {
    if (++ledTimer == 50)
    {
        toggle_led();

        if (++ledCount == 4)
        {
            TCCR2 = 0; // Disable timer 2
            TIMSK = 0;
        }

        ledTimer = 0;
    }
}

After the last blink, the timer registers are reset, ready for application usage.

BTW, this happens at each external reset, but not at power-on reset, or?

It happens only on external reset, not on power-on reset, I just tested it out.

Good point, I like to also have this on power-on reset as well, so it shouldn't be at the "start of the bootloader", other wise it only does this if the board enters bootloader mode.

I know this isn't a job of the bootloader, as you put it, but since I'm using a WDTO of 1S or 2S, I prefer the board to blink as soon as it gets power or is RESET, and not wait for the watchdog to leave bootloader and enter application.

stefanrueger commented 9 months ago

ISR(TIMER2_COMP_vect)

Interrupts in the bootloader require it to keep a dedicated bootloader ISR table, switch to that ISR table in its code (see data sheet below) before enabling interrupts; later after disabling interrupts and before switching to the application, the code needs to switch back to the application ISR table. That is a lot of code and effort to be able to blink twice in the bootloader using interrupts. Optiboot uses a timer for blinking but instead of interrupts polls the timer bits to see when it is finished. Urboot DEBUG_FREQ blinking uses a time-delay loop, basically _delay_us(), just normally shorter code.

ivsel

7.1428 Hz

... is the frequency that has a half-period of 70 ms.

actually four iterations

You are right (and this implies a 280 ms delay).

make the rn value also configurable

Why not; I've just pushed a commit doing this. Please try out DEBUG_FREQ=7.1428 DEBUG_CYCLES=2

stefanrueger commented 9 months ago

it shouldn't be at the "start of the bootloader"

Exactly! This kind of thing is best left to the application!