ARMmbed / DAPLink

https://daplink.io
Apache License 2.0
2.28k stars 967 forks source link

LPC546XX fails programming when automation mode is turned on #346

Closed studavekar closed 5 years ago

studavekar commented 6 years ago

Failing Daplink version https://github.com/ARMmbed/DAPLink/releases/tag/0244

Binary in mbed-os are build without CRC appended for this device , i updated target.json to build with CRC enabled still we see failure.

# DAPLink Firmware - see https://mbed.com/daplink
Unique ID: 1056000005b8ec7700000000000000000000000097969905
HIC ID: 97969905
Auto Reset: 0
Automation allowed: 1
Overflow detection: 1
Daplink Mode: Interface
Interface Version: 0244
Bootloader Version: 0243
Git SHA: 392f85aa88a41125dec0b963ce73c6795b8bdd0d
Local Mods: 0
USB Interfaces: MSD, CDC, HID
Bootloader CRC: 0x44e8e5be
Interface CRC: 0x629ef788
Remount count: 1

FAIL.TXT

Flash algorithm write command FAILUR
c1728p9 commented 6 years ago

CC @mmahadevan108

mmahadevan108 commented 6 years ago

@c1728p9 It seems like the failure is occurring when running the below code:


            if (!swd_read_memory(addr, rb_buf, verify_size)) {
                return ERROR_ALGO_DATA_SEQ;
            }
            if (memcmp(buf, rb_buf, verify_size) != 0) {
                return ERROR_WRITE;
            }

Is the above code trying to read from flash in order to do the compare?

c1728p9 commented 6 years ago

Hi @mmahadevan108, yep this code is reading from the flash to do the compare.

mmahadevan108 commented 6 years ago

@c1728p9 I have fixed an issue in the Flash Algo for LPC54114 and LPC54608. I don't see the failure on LPC54608. However when testing on LPC54114 I am seeing a strange behavior where the value inside addr variable while flash programming is 0x0 but when reading from flash the value inside addr is changed to 0x10. Hence there is a mismatch during compare. Any ideas? Below is the code snippet:

    // Run flash programming
    if (!swd_flash_syscall_exec(&flash->sys_call_s,
                                flash->program_page,
                                addr,
                                flash->program_buffer_size,
                                flash->program_buffer,
                                0)) {
        return ERROR_WRITE;
    }
    // Added for debugging by Mahesh
    swd_write_memory(flash->program_buffer + 0x200, (uint8_t *)&addr, 4);
    if (config_get_automation_allowed()) {
        // Verify data flashed if in automation mode
        while (write_size > 0) {
            uint8_t rb_buf[16];
            uint32_t verify_size = MIN(write_size, sizeof(rb_buf));
            if (!swd_read_memory(addr, rb_buf, verify_size)) {
                return ERROR_ALGO_DATA_SEQ;
            }
            // Added for debugging by Mahesh
            swd_write_memory(flash->program_buffer + 0x100, rb_buf, verify_size);
            swd_write_memory(flash->program_buffer + 0x210, (uint8_t *)&addr, 4);
c1728p9 commented 6 years ago

Hi @mmahadevan108 I tried the LPC54114 locally, but I couldn't reproduce this problem. Flashing went through successfully. Are you still seeing this?

c1728p9 commented 6 years ago

@studavekar this issue should be resolved, so feel free to close when you are happy with it.

mmahadevan108 commented 6 years ago

Yes, I am still seeing it. Below are my details:

DAPLink Firmware - see https://mbed.com/daplink

Unique ID: 105400001b4ee77600000000000000000000000097969905 HIC ID: 97969905 Auto Reset: 0 Automation allowed: 1 Overflow detection: 0 Daplink Mode: Interface Interface Version: 0244 Bootloader Version: 0243 Git SHA: 574979329ed9e323b5c3ea47dabed96edd864dd8 Local Mods: 0 USB Interfaces: MSD, CDC, HID Bootloader CRC: 0x26bbb78c Interface CRC: 0x1b9951a0 Remount count: 0

studavekar commented 6 years ago

We are still seeing the issue on https://os.mbed.com/platforms/LPCXpresso54114/ with the latest revision of DAPLINK https://github.com/ARMmbed/DAPLink/releases/tag/0246

maclobdell commented 6 years ago

@mmahadevan108 - I just tested daplink version 0249 on LPC54608 and this issue is still present. It indicates that it is failing to read after programming the flash.

maclobdell commented 5 years ago

I think this issue is caused by a checksum that gets inserted by the flashalgo at address 0x1C. This causes a mismatch in the data for a single long word. Is it possible to have the verification skip this longword, or can the checksum be inserted into the binary in a post build script instead of being added by the flash algo?

From The reference manual: At the reserved ARM interrupt vector location (0x0000 001C), place the 2's complement of the sum of the first 7 vectors. This causes the checksum of all of the first 8 vectors together to be 0.

lpc546xx_crc

flit commented 5 years ago

@maclobdell Yeah, I'm surprised the flash algo computes the checksum for you. The LPC54114 flash algo does the same. The checksum should be handled by a post-build script.

@mmahadevan108 What was the reason for computing the checksum in the flash algo?

mmahadevan108 commented 5 years ago

This is per the RM. It is also done in the FlashAlgo for other LPC targets. At the reserved ARM interrupt vector location (0x0000 001C), place the 2's complement of the sum of the first 7 vectors. This causes the checksum of all of the first 8 vectors together to be 0.

flit commented 5 years ago

Hi @mmahadevan108, I understand why the checksum needs to be present, but is there a specific reason you can't rely on build tool to insert it? Did you have a specific use case where it didn't work, and the solution was to compute the checksum in the flash algo?

For example, does MCUXpresso correctly insert the checksum? Does Mbed CLI insert the checksum when it builds? If both of those are true, then we should be able to remove the checksum computation from the flash algo. We should be assuming the build tools produce a valid binary.

mmahadevan108 commented 5 years ago

mbed-cli does not insert the checksum. Same with tools like Keil. Also the reference binaries included in the SDK release does not have the checksum.

MCUXpresso seems to be adding this into the binary.

brianesquilona commented 5 years ago

@c1728p9 closing this issue, I believe that the build in mbed-os handled this.

c1728p9 commented 5 years ago

The mbed-os PR which made this change is https://github.com/ARMmbed/mbed-os/pull/8319

brianesquilona commented 5 years ago

@c1728p9 Thanks Russ!