Skip to content
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

Added fake EFUSE to atmega8 part. #981

Closed
wants to merge 1 commit into from

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented May 31, 2022

This allows to have a common platform.txt recipe for all the ATMegaXX parts
in the Arduino IDE.

See:
arduino/Arduino#2075 (comment)
arduino/Arduino#2075

Co-authored-by: Martino Facchin m.facchin@arduino.cc

This allows to have a common platform.txt recipe for all the ATMegaXX parts
in the Arduino IDE.

See:
arduino/Arduino#2075 (comment)
arduino/Arduino#2075

Co-authored-by: Martino Facchin <m.facchin@arduino.cc>
@MCUdude
Copy link
Collaborator

MCUdude commented May 31, 2022

Thank you for the PR Umberto!

I've incorporated the same fix to my own 3rd party Arduino cores through their own avrdude.conf files, as it is a neat hack that works well with Arduino IDE. However, Avrdude is a standalone program that happens to be bundled with Arduino IDE, and adding a "fake" fuse that will show up in the "Memory type table" when using Avrdude in verbose mode:

# Table printed when connected to an ATmega8.
# Note that efuse appears, even though it's not really present in the hardware.

                                           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
           efuse                   0     0     0    0 no          0    1      0     0     0 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

So, in my opinion, I think it's best to leave this to forks and custom avrdude.conf files, like the one I use with my 3rd party Arduino cores. If not, we'll have to add a fake efuse to every target that only has a low and a high fuse, for consistency.
But I'm not the one to decide this, @dl8dtl is the all mighty one!

But I'm very grateful that you're taking your time to submit these PRs!

@dl8dtl
Copy link
Contributor

dl8dtl commented Jun 15, 2022

Since all newer AVRs don't fit into the classic lfuse/hfuse/efuse scheme anyway (let alone the very first AVRs ;-), I think there ought to be a better solution.

@MCUdude
Copy link
Collaborator

MCUdude commented Jun 19, 2022

I think there ought to be a better solution.

This PR only makes it possible to pass -Uefuse to Avrdude without it returning an error, in order to fit into the Arduino "ClassicAVR" build system. Avrdude is supposed to return an error if an invalid memory name is passed. Do you suggest we downgrade it to a warning, and just ignores the read/write request for memory sections that's not present?

@MCUdude
Copy link
Collaborator

MCUdude commented Jun 19, 2022

Do you suggest we downgrade it to a warning, and just ignores the read/write request for memory sections that's not present?

When thinking a bit more, I don't think this is such a terrible idea after all. It's a very simple fix (see below), and it certainly makes this PR obsolete. @dl8dtl would such a PR?

$ ./avrdude -patmega2560 -cusbasp -Ufakefuse:r:-:h -Uefuse:r:-:h

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e9801 (probably m2560)
avrdude: warning: "fakefuse" memory type not defined for part "ATmega2560"

avrdude: reading efuse memory:

Reading | ################################################## | 100% 0.00s

avrdude: writing output file "<stdout>"
0xfd

avrdude done.  Thank you.
diff --git a/src/update.c b/src/update.c
index 1500254..7cb1b99 100644
--- a/src/update.c
+++ b/src/update.c
@@ -222,9 +222,9 @@ int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags f
 
   mem = avr_locate_mem(p, upd->memtype);
   if (mem == NULL) {
-    avrdude_message(MSG_INFO, "\"%s\" memory type not defined for part \"%s\"\n",
-            upd->memtype, p->desc);
-    return -1;
+    avrdude_message(MSG_INFO, "%s: warning: \"%s\" memory type not defined for part \"%s\"\n\n",
+            progname, upd->memtype, p->desc);
+    return 0;
   }

@dl8dtl
Copy link
Contributor

dl8dtl commented Jun 19, 2022

I'm still not convinced at all why to hack up AVRDUDE for that.

@mcuee mcuee added the enhancement New feature or request label Jun 20, 2022
@MCUdude
Copy link
Collaborator

MCUdude commented Jun 20, 2022

I'm still not convinced at all why to hack up AVRDUDE for that.

Well, it would make Avrdude a little more permissive. But it's not for me to decide whether this is a good thing or not.

@umbynos
Copy link
Contributor Author

umbynos commented Jun 20, 2022

IMHO it's not a bad idea

@MCUdude
Copy link
Collaborator

MCUdude commented Jun 20, 2022

Here is a slightly different version that prevents Avrdude from trying to verify a non-existent memory and thus printing the warning twice.

main.c

   for (ln=lfirst(updates); ln; ln=lnext(ln)) {
+    UPDATE * prev = upd;
     upd = ldata(ln);
-    rc = do_op(pgm, p, upd, uflags);
+
+    if (strcmp(prev->memtype, upd->memtype) != 0 || avr_locate_mem(p, upd->memtype) != NULL)
+      rc = do_op(pgm, p, upd, uflags);
+
     if (rc) {
       exitrc = 1;
       break;
    }
  }

update.c

int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags flags)
{
  struct avrpart * v;
  AVRMEM * mem;
  int size, vsize;
  int rc;

   mem = avr_locate_mem(p, upd->memtype);
   if (mem == NULL) {
-    avrdude_message(MSG_INFO, "\"%s\" memory type not defined for part \"%s\"\n",
-            upd->memtype, p->desc);
-    return -1;
+    avrdude_message(MSG_INFO, "%s: warning: \"%s\" memory type not defined for part \"%s\"\n\n",
+            progname, upd->memtype, p->desc);
+    return 0;
   }

Output:

$ ./avrdude -cusbasp -patmega128 -Ufakefuse:w:0x00:m -Uefuse:w:0xff:m

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e9702 (probably m128)
avrdude: warning: "fakefuse" memory type not defined for part "ATmega128"

avrdude: reading input file "0xff"
avrdude: writing efuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of efuse written
avrdude: verifying efuse memory against 0xff:

Reading | ################################################## | 100% 0.00s

avrdude: 1 bytes of efuse verified

avrdude done.  Thank you.

@mcuee
Copy link
Collaborator

mcuee commented Jul 14, 2022

@MCUdude
I think your idea is better and you may want to create a new PR to replace this PR.

@mcuee
Copy link
Collaborator

mcuee commented Jul 20, 2022

I am in favor of PR #1036 than this PR. It is a generic fix for such issues.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1036.exe -c usbasp -qp m8a  -D
 -U efuse:r:-:h -U lfuse:r:-:h -U hfuse:r:-:h

avrdude_pr1036.exe: AVR device initialized and ready to accept instructions
avrdude_pr1036.exe: Device signature = 0x1e9307 (probably m8a)
avrdude_pr1036.exe: warning: "efuse" memory type not defined for part "ATmega8A"

avrdude_pr1036.exe: reading lfuse memory:
avrdude_pr1036.exe: writing output file "<stdout>"
0xef
avrdude_pr1036.exe: reading hfuse memory:
avrdude_pr1036.exe: writing output file "<stdout>"
0xd9

avrdude_pr1036.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git.exe -c usbasp -qp m8a  -D 
-U efuse:r:-:h -U lfuse:r:-:h -U hfuse:r:-:h

avrdude_git.exe: AVR device initialized and ready to accept instructions
avrdude_git.exe: Device signature = 0x1e9307 (probably m8a)
"efuse" memory type not defined for part "ATmega8A"

avrdude_git.exe done.  Thank you.

@dl8dtl
Copy link
Contributor

dl8dtl commented Jul 21, 2022

I agree

@umbynos
Copy link
Contributor Author

umbynos commented Aug 5, 2022

@stefanrueger So this one should be superseded by #1053 right?

@stefanrueger
Copy link
Collaborator

@umbynos Correct. Try it out. In -U, using a known memory (ie, for which there is a part that has such a memory) that happens not to be defined for the particular part will just skip that one -U bit.

$ avrdude -U efuse:w:0xff:m -qqp ATmega8 -c usb-bub-ii && echo OK
avrdude: skipping -U efuse:... as memory not defined for part ATmega8
OK

Specifically implemented that way, so that you don't need to add fake fuses (though that would still work if you wanted to do that in your private ~/.avrduderc). Note that the permissiveness stops for memory names that are unknown to AVRDUDE:

$ avrdude -U typo:r:-:h -p m328p -c ...
avrdude: unknown memory type typo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants