-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change condition for using paged access in avr_read() and avr_write() #1110
Conversation
This seems to be a simple and good fix for #970. But I still do not quite understand the internal working of avrdude very well. Two questions from my side.
|
@stefanrueger The following is tested with the bootloader in #940 (https://github.com/avrdudes/avrdude/files/8882667/atmega328p.txt) Previously writing EEPROM will fail if simulating the ATmega161 using ATmega328P.
Now it seems to be able to write the EEPROM but the results are not correct.
|
It's a change that will affect all parts, so we should test it on some actual usual suspect hardware. And perhaps @dl8dtl should have a look as well? I can test with an ATmega8535 later today, both in ISP and bootloader mode. |
It will be good to test bootloader for AVR8X parts as well. Unfortunately I have not figured how to get a bootloader working yet with them. I have the official ATmega4809 Arduino Nano Every and two ATmega4808 Nano clones. |
When I use the wiring bootloader, it seems to work fine, with or without this patch.
|
I have a few custom-designed boards I can use to test 32 and 48-pin AVR8X chips with, with, and without (Optiboot) bootloader. I'll see if I can test with an AVR128Dx too. |
So I tested using the optiboot bootloader from @MCUdude's Arduino MegaCore (which supports EEPROM). It has the same results. Both reading and writing are not correct.
|
Just figured out how to burn the Optiboot-DX bootloader from DXCore for my AVR128DB48 Curiosity Nano board (need to use UART3). Results seem to be the same. It is better than git main but still both writing and reading will fail. So all in all, it does not work with optiboot as of now.
|
I am wondering if you can help to figure out how to get optiboot working for ATtiny817 Xplained Mini. I have multiple issues with burning the megaTinyCore optiboot bootloader onto ATtiny817 Xplained Mini. |
Can't you just use the upstream version of Avrdude and upload the bootloader file directly? |
I tried but it does not work -- I am not so sure how to enter the bootloader. It seems to be tricky. As of now, I fail to understand the following. It seems to me there is no way to get it working out of the box (autoreset is not possible as I need updi, PB4 is not possible). The only way seems to use firmware which seems to be tricky as well. I will just give up for now. https://github.com/SpenceKonde/megaTinyCore#bootloader-optiboot-support
|
The "proper" solution would be to turn the UPDI pin into the reset pin. However, you can't recover the chip unless you have a HV UPDI programmer. I believe megaTinyCore has a bootloader build with an 8-second application start delay. This means the chip would stay in bootloader mode for 8 seconds before timing out and starting the user application. |
Basically, git mainline uses For eeprom the former is probably wrong for all UPDI parts and some classic parts (amongst which ATmega161 ATmega163 ATmega8515 ATmega8535). I now realise that for flash the PR definition is probably wrong about the following older parts
Sooooo, I have changed the
|
@mcuee Your questions:
@MCUdude and @mcuee Thanks for testing! Good find re optiboot: I thought about this and now realise that optiboot will never be able to treat eeprom with page size of 1 (no matter how hard we try) owing to the fact that the stk500v1 implementation sends a word address for eeprom (we had a long discussion re this); this is why you see only every other byte written. That is unrelated to this PR and cannot be helped now that word addresses were (erronously) used for a decade in this vvv old STK500v1 protocol implementation. Other bootloaders may benefit from being called with paged routines; good to know that wiring can manage without this PR (but then it is large, so can afford to implement both byte-wise read/write and paged read/write, most bootloaders only implement paged r/w!); I know that my (soon to be included) It really is about when to call paged r/w routines. I realise that there is |
Here's the result with an ATmega8535. Writing to EEPROM using the bootloader doesn't work properly... USBasp read/write EEPROM:
Optiboot (Arduino) bootloader
|
Thanks for the explanations. Yes it is clear to me why this PR does not work with the optiboot. |
@MCUdude Thanks for testing, yes writing EEPROM with the optiboot bootloader and a page_size of 1 doesn't work as |
@stefanrueger |
@stefanrueger
Or shall we just extend |
@stefanrueger |
I was worried about whether this PR would cause regressions for optiboot. It seems to me there is no regression.
|
Hmm, I do see a potential problem. Please take note the extra warning I've tried multiple times and the result is the same. Edit: please ignore this one. It seems to be random and I think this is not the problem of this PR.
|
@stefanrueger Line 320 in d74b17b
|
If we can differentiate with the different optiboot bootlaoders and use With current code, the optiboot_dx will fail.
But with
Edit: sorry but I can not reproduce the good write test results any more. It writes nothing now.
|
Thanks all round for testing. Given the potential for regressions that has been uncovered and the limited benefit that this PR shows, I now think it is better to close this PR and the issue it was supposed to solve. |
@stefanrueger |
Supposed to fix #970.
The fundamental avr_read() and avr_write() functions that carry out
-U
useif(pgm->paged_... && mem->page_size > 1)
to decide whether to use paged access. I believe the conditionmem->page_size > 1
serves as a proxy for whether the memory is of flash or eeprom type (application, apptable, ...).This has disadvantages when it comes to bootloaders and EEPROM for which the newer parts(UPDI / AVR8X) and some older parts (ATmega161 ATmega163 ATmega8515 ATmega8535) declare a page_size of 1. The problem is that bootloaders often only implement paged access for EEPROM and flash, so are bound to fail. See Issue #970
I suggest using a more comprehensive condition of when to use paged access functions:
avrdude/src/avrcache.c
Lines 105 to 117 in 44fe5be
As this is a change for the fundamental strategy how all
-U
operations are carried out, this requires careful consideration. Comments? Ramifications I overlooked?