bugAVR Downloader/UploaDEr - Bugs: bug #21954, verify fails for masked ('x') bits

 
 

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

bug #21954: verify fails for masked ('x') bits

Submitter:  Emil <zelea2>
Submitted:  Sat 05 Jan 2008 03:16:08 AM UTC
Votes: 20
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Privacy:  Public
Assigned to:  None Originator Name:  Emil
Open/Closed:  Closed Release:  6.3
Programmer hardware:  Device type: 

Tue 02 Aug 2016 06:42:02 PM UTC, comment #5: 

My version of the problem is solved in 6.3 indeed.

Matthijs Kooijman <matthijs>
Sun 08 May 2016 07:06:18 PM UTC, comment #4: 

It seems the original problem of this bug report got fixed with commit r1336. See also bug#46759.

Milan Kupcevic <miki>
Wed 15 Oct 2014 12:44:20 PM UTC, comment #3: 

I suggest you start digging through the code, and have a look
at how you would integrate a real fix for it, without
breaking anything else.

I recently made a sweep through avrdude.conf, and unified the
fuse bit settings for all except archaic devices (which have
very odd fuse read/write commands, compared to today), so there's
a consistent state now.  Note that this consistent state means
the definitions for read and write commands differ: while the
read command always reads full bytes (8 x 'o') so the users
get the '1' bits in the unused bit positions returned, the
write command only has 'i' bits for those bits that are marked
writable by the datasheet.

To make things worse, all recent AVRs have defined their unused
upper two lock bits as '1' (rather than 'o') in the read command.
See bug #43421 for the symptoms.

If you can come up with a patch that can consider all the
ramifications, without adding too ugly hacks into the generic
memory layer, you're welcome.

Joerg Wunsch <joerg_wunsch>
Group administrator
Tue 14 Oct 2014 08:38:30 PM UTC, comment #2: 

Any comment on this issue? I'd like to get this fixed. I'm happy to help out a bit if we can figure out how to fix it.

As for how things should work, I'd propose this:
 - For every single-byte memory (fuses, lock bytes), avrdude should know what bits are used and which aren't.
 - When writing these bytes, any unused bits in the input are ignored and fixed to 1 when the byte is sent to the programmer.
 - If any of the unused bits are specified as 0 in the input, avrdude should show a warning recommending to set them to 1.
 - When reading these bytes, they should probably be displayed as returned by the hardware.
 - When verifying these bytes, only the used bits should be compared, the unused bits ignored.


The tricky part of implementing this, is how to tell avrdude what bits are and aren't used. In theory, the information is already present as part of the commands in avrdude.conf, but as #22248 suggests they aren't perfectly consistent yet (right?).

If the commands can be made consistent it should be possible to scan the commands to find out what bits are and aren't used (though I'm not 100% sure if this applies to the more exotic hardware as well?).

Alternatively, the commands could be changed to always read and write full bytes, and we could add an explicit mask byte in the config, which might be a bit more straightforward?

Matthijs Kooijman <matthijs>
Fri 02 May 2014 07:43:50 AM UTC, comment #1: 

I'm running into a similar problem: Using my STK500 ISP programmer, the unused bits get written as 1, but read as 0, so I must use e.g. 0x3f when writing the lock byte to prevent a verify error.

However, on my JTAG3ICE in ISP or JTAG mode, the unused bits get written and read without any extra processing. Since the hardware fixes thes bits to 1, and they should be written to 1, I must write 0xff to prevent verification errors.

In general, this makes the value to use for the lock byte dependent on the programmer to use, which is a pain when multiple boards are involved too (like in the Arduino environment).

I've had a look at the code, here's what I found:

 - STK500 uses the avr_read_byte_default(), which uses the configured read ops from avrdude.conf and avr_get_output() to just set values for the used lock bits in the value read. Since the value starts out as 0, any unused bits remain 0 (even though they're 1 in the hardware). The read op for m2560 for example, looks like this:

        read            = "0 1 0 1  1 0 0 0   0 0 0 0  0 0 0 0",
                          "x x x x  x x x x   x x o o  o o o o";

e.g., only 6 bits are used, the upper two are ignored.

 - The write op for m2560 looks like this:
        write           = "1 0 1 0  1 1 0 0   1 1 1 x  x x x x",
                          "x x x x  x x x x   1 1 i i  i i i i";

   e.g., the upper bits are always written as ones.

 - jtag3 uses its own read byte function, that does not use the read and write ops from avrdude.conf. Instead, it just sends a fixed "CMD3_READ_MEMORY MTYPE_LOCK_BITS" command, which returns the full lock byte. It then returns it without further processing.
 - jtag3isp only uses the read op from avrdude.conf partly: It constructs the command to send through the STK500 protocol with it, but then the command result is just a single byte, not the full ISP response.

An obvious solution would be to either not do any masking at all (just read and write full bytes, it seems the read command actually returns the full lock byte, with 1s for the unused bits), or default unused bytes to 1 to match the hardware. However, this is bad for backward compatibility - Anyone using e.g. 0x3f with the STK500 will suddenly see his verifies breaking. On the plus side - it's probably a good idea to set the unused bits to 1 on the cmdline as well, in case you ever change parts and a previously unused lock bit is now used, at least it will remain unlocked.

A better approach is probably to explicitely mask out the bits during verification, so it doesn't matter if you specify them as 0 or 1 on the commandline. This keeps compatibility with either current approach, and makes things less error-prone. Figuring out which bits are unused can probably happen from the read or write ops, or perhaps an explicit mask would be nicer?

Matthijs Kooijman <matthijs>
Sat 05 Jan 2008 03:16:08 AM UTC, original submission:  

AVR_CMDBIT_IGNORE is not used properly when reading bits.
The current workaround is using 'x' for both read and write
operations then the verify won't fail. This is however not
following Atmel's documentation which says reserved bits should
be programmed to 1 because they might get used in the future.
The bit value for IGNORED bits is fixed to 0 so 'x' bits get
actually programmed to 0. A proper implementation would mask
the programmed and verified values before the comparisson.

Example:

$ avrdude -p m88p -c stk500v2 -P /dev/ttyS0 -u -U lock:w:0xff:m

avrdude: AVR device initialized and ready to accept instructions

avrdude: Device signature = 0x1e930f
avrdude: reading input file "0xff"
avrdude: writing lock (1 bytes):

Writing |                                                    | 0% 0.00s ***failed;

avrdude: 1 bytes of lock written
avrdude: verifying lock memory against 0xff:
avrdude: load data lock data from input file 0xff:
avrdude: input file 0xff contains 1 bytes
avrdude: reading on-chip lock data:

avrdude: verifying ...
avrdude: verification error, first mismatch at byte 0x0000
         0xff != 0x3f
avrdude: verification error; content mismatch


Emil <zelea2>

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by miki (Posted a comment)
  • -email is unavailable- added by joerg_wunsch (Posted a comment)
  • -email is unavailable- added by matthijs (Posted a comment)
  • -email is unavailable- added by sjackman (Voted in favor of this item)
  • -email is unavailable- added by zelea2 (Submitted the item)
  •  

    There are 20 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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-11-27 joerg_wunsch StatusNone Fixed
        Open/ClosedOpen Closed
        ReleaseNone 6.3
    2008-01-10 sjackman Carbon-Copy- Added sjackman

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code