issue #1600 Bootloader enhancements #1614
Conversation
mcuee
changed the title
|
The underlying trick is vvv cool, but the implementation looks too ad-hoc and specific. We should think bigger:
So, yes, it is a bigger piece of work for this PR (or another one with the same goal) to become truly useful. Also, the suppression of the warning looks like doctoring the symptoms, rather than the underlying cause: I suspect that a programmer for UPDI parts piggy-backs on |
Upon closer inspection, none of the variants of "optiboot_x" handle Cmnd_STK_SET_DEVICE and simply return Resp_STK_OK. So this can be handled more concisely. if ((rc = stk500_getparm(pgm, Parm_STK_SW_MAJOR, &maj)) < 0
|| (rc = stk500_getparm(pgm, Parm_STK_SW_MINOR, &min)) < 0 ) {
pmsg_error("cannot obtain SW version\n");
return rc;
}
+ // For "optiboot_x" variant
+ // The responses after this are dummy and unnecessary.
+ if (str_eq(pgmid, "arduino") && p->prog_modes & PM_UPDI)
+ return pgm->program_enable(pgm, p);
+
// MIB510 does not need extparams
if (str_eq(pgmid, "mib510"))
n_extparms = 0;This looks like even more shoddy code. Therefore, it seems good to extend the "modernAVR" mode with the "-x" option...
This should have been the best, at least when only tinyAVR and megaAVR existed. Due to its open design, his STK500v1 can be used with a serial port, data length and address can be specified in the request, and there is no checksum in the response. It's very simple. Other products, such as the AVR109, directly packetize SPI instructions, so they are not suitable for Microchip-era AVRs with linear memory.
Even if everything is integrated into urlock in the future, the STK500 code is confusing and outdated at first glance, but its purpose and implementation is clear and easy to add to and modify. Suitable for repeating various experiments and verifications. I think that's why code has historically been extended repeatedly to become larger code. |
|
Adopted There used to be an option that was not in the manual, but this was also fixed at the same time. Previous UsageIf you do not use Memory optiboot_x read test summary without -xem optionUsing Memory optiboot_x read test summary with -xem option
If you are using
If
If you are using
Other bootloader variants that use Other implementations that share STK500 code include stk500, mib510, and avrisp, but none of them handle UPDI, so there should be no conflicts. I'm sure that's the case. Something I found useful while testing it myself is that FUSE_BOOTEND is now readable. This is a clear representation of the bootloader size without any guesswork involved. I think it was worth the effort just to be able to do this. |
|
Adoption of -xrtsdtr option and review of arduino_open() Comparing arduino_open() and stk500_getsync(), they do the same thing inside a "attempt" loop. The difference is the content of the first loop and subsequent loops. This is probably because stk500_getsync() was changed later. Since this is an opportunity for maintenance, I decided to organize both of them. (AVR now starts programming about 1 second earlier) In addition to that, we added the |
I forgot one thing. The reason I originally wanted this feature was because of the presence of a serial port switch that allows flash programs to coexist with the Arduino IDE's serial monitor/plotter. The IDE has the property of keeping RTS/DTR low while running a serial monitor/plotter. By designing a two-circuit bus switch that connects the -EN side to the console's serial port and the +EN side to serialupdi, two types of serial port devices can coexist without changing the physical ports. Sending "-carduino -xrtsdtr=low" to the Serialupdi board also allows writing with the normal bootloader. (Auto-reset support is also required) Actually, I am currently using this board as well. |
|
What I thought after trying out some uniquely designed serial adapters. When the Arduino bootloader goes out of sync, it often fails to resync and reverts to normal running behavior. One reason is the get_sync loop implementation. In the STK500 implementation, get_sync sends 2 bytes and reads 2 bytes. I have code in case the first byte of the returned set of 2 bytes is lost, and I'll catch that case and resynchronize. In Arduino mode, an exception condition is executed and a reset signal is always sent in the retry loop after two rounds. Therefore, resynchronization will only succeed in the first round. On a retry, the delay after a reset is not the same as the first time, so the next reset is sent before the synchronization is successful. After all, synchronization is less likely to be successful after the second round. The reason it is almost impractical to install a bootloader on a chip that cannot normally be reset, such as tinyAVR, is that the chip must be powered on or connected to a USB port after avrdude boots. Current avrdude does not allow that. If you want to support tinyAVR's bootloader operations, you will need a get_sync retry loop that involves reopening the serial port and sending a reset. It's clear that you can't reuse STK500's get_sync to implement this. As a challenge, I would like to consider starting the bootloader of tinyAVR (powering on after starting avrdude). This behavior differs from previous behavior, so we will also consider additional options. |
|
Thanks @askn37! This needs careful checking; it's a new feature request, so likely to be looked at after v7.3 release |
|
There's no need to rush. And it looks like the idea around get_sync needs more work. I have no objection to this PR being rejected. (And continues with my fork.) |
|
Adoption of Previously, when starting avrdude, it would exit immediately if the port was invalid. This is by design.
Similarly, if the port is opened by another application, it will be retried. The bootloader gets a chance to start even if you close other applications. |
|
@askn37 |
Correct, I have never seen @askn37 Have a look at the @mcuee and @askn37 This PR is generally problematic insofar as it focuses on |
@askn37 |
|
Just an example how |
|
Another more extreme example, using urboot stk500v1 autobaud firmware. Look at the 167.72 s number. But at least
|
|
I think you are absolutely right that there is no point in further improving the Arduino protocol. In particular, it doesn't have enough power to cover his UPDI generation of hardware, which I'm eyeing. More than 97 models have already been released in this category, which is more than 1/4 of all 8bit-AVR models. The problem is that using the urboot protocol doesn't seem to take advantage of the performance of the hardware you have. In particular, we do not believe that the following hardware UART features will be supported:
My personal challenge will ultimately involve designing a new open source protocol. This effort is very important, but if AVR-DU can be used as expected (protocol conversion from AVRISP/JTAGmkII becomes easier, etc.), the difficulty will be significantly reduced. We're still halfway through. |
Thanks a lot for the contributions. In this case, I will close this PR as not planned. I will move the other part about urboot and modern UPDI AVRs into a discussion. |
This is the memory read enhancement patch for the Adruino compatible bootloader mentioned in issue #1600.
Allows the following memory reads similar to EEPROM only for PM_SPM + PM_UPDI parts.
This trick works for all bootloaders that support READ memchr == 'E'.
-U : userrow usersig bootrow sigrow prodsig and signature
-T dump : userrow and bootrow
This trick is useful if OCD or UPDI control becomes unavailable for some reason (damaged pins due to high voltage, unintentional FUSE rewrite, etc.) but the bootloader is still working. This is important for debugging and verification tasks because BOOTROW can only be read by boot code.
The good thing about bootloaders is that there is no risk of further damage to the chip.
There are some things that can be read with -U but not with -T dump. This is probably because it is currently prohibited by
avr_has_paged_access(pgm, mem). Unfortunately, I can't read LOCK and FUSE as a result.A similar implementation is possible in urclock.c, but it is not as easily extended as stk500.c. I tried it, but this time I gave up. It seems like there are too many things that need to be redone.
The ability to write USERROW and BOOTROW is not integrated at this time as it requires new BIGBOOT development. At least someone has to decide on memory types other than "F" and "E".
(Example: "X". For this he needs an absolute address in 64KiB space. Using a relative address like 'E' will make the boot code obese)
P.S.1 : I don't think this patch will be rushed to 7.3. Because that may still be up for debate.
P.S.2 : There is one place in the code to suppress extra warnings that are not related to the PM_UPDI part.