bugAVR Downloader/UploaDEr - Bugs: bug #42267, jtag3isp fails to read lock and...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #42267: jtag3isp fails to read lock and fuse bytes directly after changing lock byte

Submitter:  Matthijs Kooijman <matthijs>
Submitted:  Fri 02 May 2014 09:43:30 AM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Privacy:  Public
Assigned to:  joerg_wunsch Originator Name:  Matthijs Kooijman
Open/Closed:  Closed Release:  None
Programmer hardware:  Device type: 

Jump to the original submission

Tue 29 Dec 2020 02:30:17 PM UTC, comment #10: 

For future reference, the workaround was added in this commit: http://svn.savannah.gnu.org/viewvc/avrdude/trunk/avrdude/stk500v2.c?r1=1336&r2=1337

Matthijs Kooijman <matthijs>
Tue 28 Oct 2014 10:27:35 AM UTC, comment #9: 

After a bit more prodding, Atmel writes:

> Yes, adding a delay is the proper way to solve this problem. Atmel
> Studio adds a fixed delay of 20ms after writing the fuse and lock bits
> using ISP interface.


So it seems that a delay is the way to go, it should perhaps even be extended to 20ms.

Matthijs Kooijman <matthijs>
Tue 14 Oct 2014 06:14:03 PM UTC, comment #8: 

Hmm, I just noticed min_write_delay and max_write_delay in avrdude.conf. Aren't these related? I couldn't quite figure out what they're used for exactly, though.

Matthijs Kooijman <matthijs>
Tue 14 Oct 2014 05:48:33 PM UTC, comment #7: 

Just compiled svn trunk and did a quick test, seems that the delay
solves this problem for me as well.

Atmel has replied to my case with:

> I understood from the bug report given in the following link that in
> AVRdude there is no proper delay after the write routine which makes the
> read operation failed. Hence, there might be no error with the JTAGICE3
> firmware. Please try the workaround which suggests adding some delay
> after the write routine in AVRdude and verify whether the problem gets
> sloved.
> https://savannah.nongnu.org/bugs/?42267
>
> I tried to program the device using Atmel Studio 6.2 using JTAGICE3 tool
> and I am able to write and read the lock bits successfully. As the read
> /write operation works fine with Atmel Studio I thing that there might
> be no error with the JTAGICE3 firmware.
>
> As AVRdude is a third party tool we are unable to comment on it.


Which doesn't really tell me if this behaviour is expected and/or if they
also consider the delay a workaround. I haven't replied yet, since I
discovered a bit more below.

Looking more closely at how things are implemented, I noticed that
avrdude pretty much sends raw ICSP commands to the JTAGICE3, which get
sent as-is over the SPI bus. This made me wonder if the firmware is
involved at all, or if it is the hardware messing this up directly.

I connected a logic analyizer to the ICSP lines and found out that the
incorrect 0xff reading comes directly from the target - the firmware is
just passing it through. Perhaps the older firmwares that do work
without the workaround have a delay themselves, either explicitly to
fix this, or implicitly because of slowness in the implementation?

Looking at the code, I also noticed that there was already a workaround
for the fuse bytes, where program mode was exited and re-entered after
programming a fuse byte. You replaced that workaround with the delay,
which seems to work as well. Removing both workarounds shows that fuse
bytes also return 0xff shortly after writing them, making it likely that
this problem is the same for both fuse and lock bytes.

I suspect the old workaround was based on this (from the 328 datasheet):

> The fuse values are latched when the device enters programming mode and
> changes of the fuse values will have no effect until the part leaves
> Programming mode.  This does not apply to the EESAVE Fuse which will take
> effect once it is programmed. The fuses are also latched on Power-up in
> Normal mode.


However, that doesn't say reading the fuses needs re-entering programming mode,
just that they won't take effect until you leave programming mode (so not
leaving programming mode is probably a good idea, in case you change e.g. the
clock speed).

Having said all that, it seems that adding a small delay like now is
probably a reasonable way to make sure things work. I'll ask Atmel if
they can confirm that such a delay is necessary, since it doesn't seem to be
documented anywhere I could find.

For the record, here's the SPI dumps with and without the fix. Everything is
identical, except for the timings and the last byte.

With fix                                          Without fix
Time [s],Packet ID,MOSI,MISO                      Time [s],Packet ID,MOSI,MISO
0.056597700000000,,0xAC,0x00                      0.056595600000000,,0xAC,0x00
0.057665400000000,,0x53,0x00                      0.057663300000000,,0x53,0x00
0.058733100000000,,0x00,0x53                      0.058731000000000,,0x00,0x53
0.059800800000000,,0x00,0x00                      0.059798700000000,,0x00,0x00
0.276508800000000,,0x30,0x00                      0.271037000000000,,0x30,0x00
0.277577000000000,,0x00,0x30                      0.272105800000000,,0x00,0x30
0.278645800000000,,0x00,0x00                      0.273174600000000,,0x00,0x00
0.279714500000000,,0x00,0x1E                      0.274243300000000,,0x00,0x1E
0.281239800000000,,0x30,0x00                      0.275737700000000,,0x30,0x00
0.282308600000000,,0x00,0x30                      0.276806400000000,,0x00,0x30
0.283377300000000,,0x01,0x00                      0.277882100000000,,0x01,0x00
0.284446100000000,,0x00,0xA8                      0.278950900000000,,0x00,0xA8
0.285893000000000,,0x30,0x00                      0.280338600000000,,0x30,0x00
0.286961200000000,,0x00,0x30                      0.281406800000000,,0x00,0x30
0.288030000000000,,0x02,0x00                      0.282475600000000,,0x02,0x00
0.289098700000000,,0x00,0x02                      0.283544300000000,,0x00,0x02
0.290716300000000,,0x58,0x00                      0.285061600000000,,0x58,0x00
0.291784500000000,,0x00,0x58                      0.286129800000000,,0x00,0x58
0.292853300000000,,0x00,0x00                      0.287198600000000,,0x00,0x00
0.293921000000000,,0x00,0xCF                      0.288267400000000,,0x00,0xCF
0.295528900000000,,0xAC,0x00                      0.289807600000000,,0xAC,0x00
0.296597100000000,,0xE0,0xAC                      0.290881700000000,,0xE0,0xAC
0.297665900000000,,0x00,0xE0                      0.291950400000000,,0x00,0xE0
0.298734600000000,,0xCF,0x00                      0.293018700000000,,0xCF,0x00
0.310333700000000,,0x58,0xCF                      0.294517300000000,,0x58,0xCF
0.311401900000000,,0x00,0x58                      0.295585000000000,,0x00,0x58
0.312470200000000,,0x00,0x00                      0.296653700000000,,0x00,0x00
0.313538900000000,,0x00,0xCF                      0.297722500000000,,0x00,0xFF

Matthijs Kooijman <matthijs>
Tue 07 Oct 2014 03:31:38 PM UTC, comment #6: 

Getting more info sounds sane :-)

> As a first step, please report this to Atmel, so they open a

ticket for it.

I opened up case #00005754 through "myAtmel" (though it seems those cases are not public, so can't provide a link). I'll keep you updated on any progress.

Thanks for adding a workaround. I'll try compiling avrude this week to see if it works for me too.

Matthijs Kooijman <matthijs>
Tue 07 Oct 2014 03:22:55 PM UTC, comment #5: 

I added 10 ms of delay before exiting from stk500isp_write_byte().
This also seems to "fix" other strange obervations made before,
where fuse updates didn't propagate back immediately (and where
there used to be a completely different workaround that could
be dropped now).

Oh well, let's hope 10 ms will be good for just everyone …

Joerg Wunsch <joerg_wunsch>
Group administrator
Tue 07 Oct 2014 03:06:29 PM UTC, comment #4: 


> What's the problem with adding a workaround in avrdude?


Hacks like these are always evil.  As we don't have an idea
what happens in the firmware here, how do we know a safe
value to delay?  While 10 ms seem to work for me, how could
we be sure they'll work for anyone?

> Hmm, I see. However, even though this might be the firmware's
> fault, it seems unlikely that this will be fixed on that end


As a first step, please report this to Atmel, so they open a
ticket for it.

> Closing this as invalid and ignoring the problem doesn't
> seem like a great idea


That's why I didn't do it immediately …

Joerg Wunsch <joerg_wunsch>
Group administrator
Tue 07 Oct 2014 02:23:23 PM UTC, comment #3: 

Hmm, I see. However, even though this might be the firmware's fault, it seems unlikely that this will be fixed on that end (especially for the older hardware, I'd expect). Closing this as invalid and ignoring the problem doesn't seem like a great idea, since then things will still be broken for anyone using avrdude with this hardware. When manually running avrdude, the obvious workaround is of course to not verify the lockbits, but for example running inside the Arduino IDE, I don't think this easy (and probably not possible without also disabling verification of the full binary). Having to disable verification isn't so nice anyway...

What's the problem with adding a workaround in avrdude? Sounds like it could just be added for the affected hardware (if the code structures allows this, not sure). Alternatively, adding a small delay for all hardware doesn't seem like a big burden?

Ideally, you'd not add workarounds for broken hardware or firmware to any software, but that's unfortunately often impossible. If the Linux devs would have used this filosophy, I don't think the Linux kernel would still exist today ;-p

Matthijs Kooijman <matthijs>
Tue 07 Oct 2014 02:02:06 PM UTC, comment #2: 

This appears to be an issue with the JTAGICE3 firmware itself.

You are right in that it's certainly timing-related: even
adding -vvvv makes AVRDUDE slow enough to no longer trigger
the issue.  If you sniff the USB traffic, you can see that
the wrong answer comes straight from the device, so AVRDUDE
itself is not at fault.

Btw., this happens for all the Atmel tools, ranging from
the AVRISPmkII through STK600, JTAGICE3 to the new Atmel-ICE.
Quite likely, they all use the same piece of firmware to
implement the ISP protocol.  The only tool being different is
the JTAGICEmkII which works correctly.

So far, this used to have been masked off by the stk500v2
code using the old SPI_MULTI command for single-byte updates.
Starting with the JTAGICE3, this command is no longer supported,
so it had to be replaced using standard read/write commands in
AVRDUDE.  Meanwhile, the code has been modified to always use
this (more modern) implementation, so this firmware bug becomes
apparent for all the tools.  (Besides, that change "fixes" the
effect described in bug #21954.)

I'd tend to close the bug report as being "invalid" as it's
clearly a firmware issue with those tools.  Adding arbitrary
delays just to accomodate for broken tools seems like a crock
to me, albeit it might be possible to do that.

Joerg Wunsch <joerg_wunsch>
Group administrator
Tue 06 May 2014 12:40:18 PM UTC, comment #1: 

I just did some quick testing with avrdude 6.1, which also shows the same problem.

Matthijs Kooijman <matthijs>
Fri 02 May 2014 09:43:30 AM UTC, original submission:  

I'm having some trouble reading and verifying the lock bits with jtag3isp. jtag3 works as expected. I'm using Avrdude 6.0.1.

It seems that (shortly) after writing the lock byte, the fuse and lock bytes no longer read correctly. When the clock is slower, only the first few reads after the write fail, suggesting that this is somehow a timing problem.

The problem did not occur when writing the fuse bytes, only when writing the lock byte.

See the attachment for an extensive log of commands and output, along with inline comments by me showing the problem.

I haven't tried actually sniffing the SPI bus to see what values are actually returned by the device being programmed. I might try that if you think it helps (and my bus pirate is fast enough ;-p).

Matthijs Kooijman <matthijs>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #31298:  log.txt added by matthijs (44KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by joerg_wunsch (Posted a comment)
  • -email is unavailable- added by matthijs (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-10-07 joerg_wunsch StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2014-10-07 joerg_wunsch StatusNone Confirmed
        Assigned toNone joerg_wunsch
    2014-05-02 matthijs Attached File- Added log.txt, #31298

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code