search

microchip-pic-avr-tools / pymcuprog

a Python utility for programming various Microchip MCU devices using Microchip CMSIS-DAP based debuggers
MIT License
104 stars 22 forks source link

Corrupted flash memory writes for Atmega328p with ISP interface #44

Open araqioui opened 7 months ago

araqioui commented 7 months ago

While trying to execute :

pymcuprog write  -f executable.hex --erase --verify -t atmelice -d atmega328p -i isp

The command always fails at flash memory verification step:

Connecting to any atmelice
Connected to Atmel-ICE CMSIS-DAP from ATMEL (serial number J42700040265)
Debugger firmware version 1.45.7
Debugger hardware revision 0
pymcuprog.nvm - WARNING - 
pymcuprog.nvm - WARNING - AVR-ISP/SPI stack is in Alpha state
pymcuprog.nvm - WARNING - Expect some features to be missing
pymcuprog.nvm - WARNING - 
Pinging device...
Ping response: 1E950F
Erasing device before writing from hex file...
Writing from hex file...
Writing flash...
Verifying flash...
pymcuprog.programmer - ERROR - Verify failed for flash memory:
pymcuprog.programmer - ERROR - Verify mismatch starting at location 0x000000: 0x0C vs 0xFF (is the memory section erased?)
Verification failed!

The mismatch location address is not always the same.

On the other hand, while trying to flash using MPLAB IPE with the same HW setup, and the same hex file, it always passes.

xedbg commented 7 months ago

Logged internally as DSG-7318. Thanks for reporting this and providing a fix! We currently don't do pull-requests directly on github but the patch can be made on our git repo and the fix will be in place in the next release. We have just recently completed a release, so can't yet comment on when this might be done.

araqioui commented 7 months ago

@xedbg Thank you for taking the time to look at this You may have a look at this #PR for proposed fix. Have a great day !

xedbg commented 7 months ago

I think your fix works... interesting though, the tool protocol also has a delay argument which is not correctly used. This fix in pyedbglib's avrispprotocol.py could arguably be more "correct" when seen from the tool firmware:

Add parameter to write_flash_page:

# def write_flash_page(self, byte_address, data):
def write_flash_page(self, byte_address, data, delay_ms=5):

Then send in the delay to the tool firmware and set bit 6 to make use of it:

# command.extend([0x81])  # Page mode
# command.extend([0])  # Not used
command.extend([0x91])  # Write page with timed delay
command.extend([delay_ms])
araqioui commented 7 months ago

@xedbg Thanks for the review and the proposed fix. I can't help but agree with you on your suggested fix... Tested and worked just perfectly :100: I opened a #PR to bring your suggested fix to life in the next release hopefully :smiley: