LPC1768 IAP Fix #4993

chrissnow · 2017-08-30T12:58:33Z

Description

Reinstatates LPC1768 IAP by using HAL routines rather than CMSIS Algo.

Status

READY

+-------------+---------------+------------------------------+-------------------------------+--------+--------+--------+--------------------+
| target      | platform_name | test suite                   | test case                     | passed | failed | result | elapsed_time (sec) |
+-------------+---------------+------------------------------+-------------------------------+--------+--------+--------+--------------------+
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - buffer alignment test | 1      | 0      | OK     | 0.17               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - clock and cache test  | 1      | 0      | OK     | 0.06               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - erase sector          | 1      | 0      | OK     | 0.14               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - init                  | 1      | 0      | OK     | 0.06               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - mapping alignment     | 1      | 0      | OK     | 0.06               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - program page          | 1      | 0      | OK     | 0.26               |
+-------------+---------------+------------------------------+-------------------------------+--------+--------+--------+--------------------+

+-------------+---------------+-------------------------------------+---------------------------+--------+--------+--------+--------------------+
| target      | platform_name | test suite                          | test case                 | passed | failed | result | elapsed_time (sec) |
+-------------+---------------+-------------------------------------+---------------------------+--------+--------+--------+--------------------+
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_drivers-flashiap | FlashIAP - init           | 1      | 0      | OK     | 0.04               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_drivers-flashiap | FlashIAP - program        | 1      | 0      | OK     | 0.28               |
| LPC1768-ARM | LPC1768       | mbed-os-tests-mbed_drivers-flashiap | FlashIAP - program errors | 1      | 0      | OK     | 0.06               |
+-------------+---------------+-------------------------------------+---------------------------+--------+--------+--------+--------------------+

…ed correctly. Tidy up

chrissnow · 2017-08-30T13:07:41Z

GCC also passes.

+-----------------+---------------+-------------------------------------+---------------------------+--------+--------+--------+--------------------+
| target          | platform_name | test suite                          | test case                 | passed | failed | result | elapsed_time (sec) |
+-----------------+---------------+-------------------------------------+---------------------------+--------+--------+--------+--------------------+
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_drivers-flashiap | FlashIAP - init           | 1      | 0      | OK     | 0.04               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_drivers-flashiap | FlashIAP - program        | 1      | 0      | OK     | 0.28               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_drivers-flashiap | FlashIAP - program errors | 1      | 0      | OK     | 0.07               |
+-----------------+---------------+-------------------------------------+---------------------------+--------+--------+--------+--------------------+

+-----------------+---------------+------------------------------+-------------------------------+--------+--------+--------+--------------------+
| target          | platform_name | test suite                   | test case                     | passed | failed | result | elapsed_time (sec) |
+-----------------+---------------+------------------------------+-------------------------------+--------+--------+--------+--------------------+
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - buffer alignment test | 1      | 0      | OK     | 0.18               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - clock and cache test  | 1      | 0      | OK     | 0.08               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - erase sector          | 1      | 0      | OK     | 0.14               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - init                  | 1      | 0      | OK     | 0.07               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - mapping alignment     | 1      | 0      | OK     | 0.05               |
| LPC1768-GCC_ARM | LPC1768       | mbed-os-tests-mbed_hal-flash | Flash - program page          | 1      | 0      | OK     | 0.25               |
+-----------------+---------------+------------------------------+-------------------------------+--------+--------+--------+--------------------+

0xc0170 · 2017-08-31T07:01:26Z

targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c

+ */
+
+unsigned long GetSecNum (unsigned long address) {
+  unsigned long n;


{ on t he new line, please leave one line space between functions (line 70)

0xc0170 · 2017-08-31T07:01:55Z

targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c

+  uint8_t *data;
+  unsigned long n;
+
+  if ((unsigned long)datain%4==0)//word boundary


https://docs.mbed.com/docs/mbed-os-handbook/en/latest/cont/code_style/

2 spaces instead of 4? Please read the above coding style and align this code

0xc0170

Code style alingment

0xc0170 · 2017-08-31T07:03:00Z

Thanks for fixing the previous bug in the implementation !

0xc0170 · 2017-09-04T09:36:32Z

@chrissnow bump

chrissnow · 2017-09-04T11:30:54Z

Code tidied but I still have a warning to sort out

mbed-os\targets\TARGET_NXP\TARGET_LPC176X\device\flash_api.c(117): warning:  #513-D: a value of type "const uint8_t *" cannot be assigned to an entity of type "uint8_t *"

I also want to understand SET_VALID_CODE and what we want it to be set to.

chrissnow · 2017-09-04T12:53:51Z

@0xc0170 This should now be complete.

0xc0170

Can you align if else code aligment , as shared previously to the mbed coding style please?

chrissnow · 2017-09-04T14:08:08Z

Apologies, I think that matches now.

0xc0170 · 2017-09-04T14:19:07Z

targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c

+        free(alignedData);
+    }
+
+    if (IAP.stat) return (1);// Command Failed


Should be

if (IAP.stat) { return (1); }

0xc0170 · 2017-09-04T14:19:22Z

targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c

+
+uint32_t flash_get_sector_size(const flash_t *obj, uint32_t address)
+{
+    if(address < flash_get_start_address(obj) || address >= flash_get_start_address(obj) +flash_get_size(obj)){


if (address.....) (space after if)

0xc0170 · 2017-09-04T14:19:31Z

targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c

+ *    Return Value:   Sector Number
+ */
+
+unsigned long GetSecNum (unsigned long address) {


{ new line

chrissnow · 2017-09-04T14:57:09Z

I think I now have eclipse set up to format to mbed style.

0xc0170

We will squash this as it comes as one fix (12 commits are mostly devel commits in here)

theotherjimmy · 2017-09-21T14:16:26Z

/morph test

mbed-bot · 2017-09-22T08:04:19Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1359

All builds and test passed!

0xc0170 · 2017-09-25T09:43:06Z

@chrissnow we are looking at this again, seems like IAR fails from time to time for the flashiap test. Have you noticed any sporadic failures with program() ?

chrissnow · 2017-09-25T10:05:54Z

@0xc0170 I'm wondering if malloc isn't always assigning an aligned location like I expected.
C99 7.20.3

The pointer returned if the allocation
succeeds is suitably aligned so that it may be assigned to a pointer to any type of object

I didn't spend much time with IAR other than to get the test results a couple of times.

I haven't had any issues with ARM with my project though.

Do you know if its hard faulting? that's what will happen if it isn't aligned and I guess timeout the test.

0xc0170 · 2017-09-25T10:12:19Z

Do you know if its hard faulting? that's what will happen if it isn't aligned and I guess timeout the test.

I've attached debugger, it's hard faulting, I am investigating where it writes and how to get into the hardfault. Or maybe not hardfault at al,, will provide an update

chrissnow · 2017-09-25T10:48:34Z

I may have figured it out,
the IAR linker is missing the ram reservation for IAP

mbed-os/targets/TARGET_NXP/TARGET_LPC176X/device/TOOLCHAIN_IAR/LPC17xx.icf

Line 12 in c6f655c

define symbol __ICFEDIT_region_RAM_end__ = 0x10007FDF;

I think that needs to be 0x10007FE0
see

mbed-os/targets/TARGET_NXP/TARGET_LPC176X/device/TOOLCHAIN_ARM_STD/LPC1768.sct

Line 9 in c6f655c

    
           ; 32KB (RAM size) - 0xC8 (NIVT) - 32 (topmost 32 bytes used by IAP functions) = 0x7F18

0xc0170 · 2017-09-25T11:02:23Z

Looks correct that RAM end is not defined as it should, I am however also running test with this fix (RAM end should be 0x10007EF8).

It fails sporadically. As it is, it gets to hardfault. With the fixed RAM end, debugging crashes under IAR (can't do anything just wait until I receive an error a target cant be stopped).
It happens during programming, at 2 addresses (I so far got 2 different addresses). Stacked PC counter indicates that is SystickHandler and fault register shows invalid instruction fault.

Some logs


Erasing: address 491520, sector size: 32768
Programming
Programming: page_addr 491520, page_size: 1024
Programming: page_addr 492544, page_size: 1024
Programming: page_addr 493568, page_size: 1024
Programming: page_addr 494592, page_size: 1024
Programming: page_addr 495616, page_size: 1024
Programming: page_addr 496640, page_size: 1024
Programming: page_addr 497664, page_size: 1024
Programming: page_addr 498688, page_size: 1024
Programming: page_addr 499712, page_size: 1024
Programming: page_addr 500736, page_size: 1024
Programming: page_addr 501760, page_size: 1024
Programming: page_addr 502784, page_size: 1024
Programming: page_addr 503808, page_size: 1024
Programming: page_addr 504832, page_size: 1024
Programming: page_addr 505856, page_size: 1024
Programming: page_addr 506880, page_size: 1024
Programming: page_addr 507904, page_size: 1024
Programming: page_addr 508928, page_size: 1024
Programming: page_addr 509952, page_size: 1024
Programming: page_addr 510976, page_size: 1024
Programming: page_addr 512000, page_size: 1024
Programming: page_addr 513024, page_size: 1024
Programming: page_addr 514048, page_size: 1024
Programming: page_addr 515072, page_size: 1024
Programming: page_addr 516096, page_size: 1024
Programming: page_addr 517120, page_size: 1024
Programming: page_addr 518144, page_size: 1024
Programming: page_addr 519168, page_size: 1024
Programming: page_addr 520192, page_size: 1024
Programming: page_addr 521216, page_size: 1024
Programming: page_addr 522240, page_size: 1024
Programming: page_addr 523264, page_size: 1024
Reading
Deinit
TEST DONE Done


Hard fault

Erasing: address 491520, sector size: 32768
Programming
Programming: page_addr 491520, page_size: 1024
Programming: page_addr 492544, page_size: 1024
Programming: page_addr 493568, page_size: 1024
Programming: page_addr 494592, page_size: 1024
Programming: page_addr 495616, page_size: 1024
Programming: page_addr 496640, page_size: 1024
Programming: page_addr 497664, page_size: 1024
Programming: page_addr 498688, page_size: 1024
Programming: page_addr 499712, page_size: 1024
Programming: page_addr 500736, page_size: 1024
Programming: page_addr 501760, page_size: 1024
Programming: page_addr 502784, page_size: 1024
Programming: page_addr 503808, page_size: 1024
Programming: page_addr 504832, page_size: 1024
Programming: page_addr 505856, page_size: 1024
Programming: page_addr 506880, page_size: 1024
Programming: page_addr 507904, page_size: 1024
Programming: page_addr 508928, page_size: 1024
Programming: page_addr 509952, page_size: 1024
Programming: page_addr 510976, page_size: 1024
Programming: page_addr 512000, page_size: 1024
Programming: page_addr 513024, page_size: 1024
Programming: page_addr 514048, page_size: 1024
Programming: page_addr 515072, page_size: 1024
Programming: page_addr 516096, page_size: 1024

Another hard fault

Erasing: address 491520, sector size: 32768
Programming
Programming: page_addr 491520, page_size: 1024
Programming: page_addr 492544, page_size: 1024
Programming: page_addr 493568, page_size: 1024
Programming: page_addr 494592, page_size: 1024
Programming: page_addr 495616, page_size: 1024
Programming: page_addr 496640, page_size: 1024
Programming: page_addr 497664, page_size: 1024
Programming: page_addr 498688, page_size: 1024
Programming: page_addr 499712, page_size: 1024
Programming: page_addr 500736, page_size: 1024
Programming: page_addr 501760, page_size: 1024

0xc0170 · 2017-09-25T12:39:52Z

@chrissnow Besides that RAM end that I'll fix, it seems that systick is ongoing (hardfault points to systick handler causing undefinstr error). If I suspend kernel (not needed for the test), all is running OK. I've run it at least 10x so far , all OK with my own test case.

Hints?

I'll rebuild everything, clean, and retest changes for RAM end again, to be certain that does not fix.

chrissnow · 2017-09-25T12:53:00Z

Having done a bit of reading I think IAP blocks flash access during operations, which isn't going to end well with interrupts.

Perhaps disable interrupts during IAP operations?

https://os.mbed.com/questions/54146/LPC824-RTOS-and-IAP/

IAP is indeed a ROM call on LPC devices, but I don't know if the ROM code disables interrupts. For example I know for a fact that all flash access during programming/erasing of any sector results in the MCU crashing. It can handle a while(IAP_read) loop, most likely since that is prefetched from flash, for the LPC this is not relevant however since it is fetched from ROM. However if interrupts would not be disabled, it would jump to the interrupt vector where it would need to use the flash.

chrissnow · 2017-09-25T12:56:10Z

And from the LPC1768 user manual

32.3.2.6 Interrupts during IAP
The on-chip flash memory is not accessible during erase/write operations. When the user
application code starts executing the interrupt vectors from the user flash area are active.
The user should either disable interrupts, or ensure that user interrupt vectors are active in
RAM and that the interrupt handlers reside in RAM, before making a flash erase/write IAP
call. The IAP code does not use or disable interrupts.

0xc0170 · 2017-09-25T12:57:23Z

Run from new branch, all seems fine , sent a fix - #5189

0xc0170 · 2017-09-25T13:09:11Z

@chrissnow Thanks, that is what I suspected. We shall add critical section then, lets take it to the fix, I can add one more

chrissnow added 6 commits August 29, 2017 22:16

initial IAP routine

6e94989

remove CMSIS algorithm and enable bootloader

cc61e3a

missing CR

17ac327

Ensure IAP uses word boundary aligned data.

a7983a6

Fix flash_mapping_alignment_test so that invalid addresses are report…

fe9c20a

…ed correctly. Tidy up

Disable bootloader's for now

6bd0ec1

This was referenced Aug 30, 2017

UBLOX_C027 - 'Flash - buffer alignment test' is failing #4970

Closed

ARCH_PRO - Flash - buffer alignment test is failing #4288

Closed

0xc0170 reviewed Aug 31, 2017

View reviewed changes

0xc0170 requested changes Aug 31, 2017

View reviewed changes

0xc0170 added the needs: work label Aug 31, 2017

Code tidy.

2f15725

chrissnow added 2 commits September 4, 2017 13:17

Remove SET_VALID_CODE

44713ce

Resolve warning

5eea5de

0xc0170 requested changes Sep 4, 2017

View reviewed changes

Tidy if else code aligment.

3fabf8f

0xc0170 reviewed Sep 4, 2017

View reviewed changes

Code tidy.

58800b0

Code tidy

f352920

0xc0170 approved these changes Sep 4, 2017

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Sep 4, 2017

0xc0170 added ready for merge release-version: 5.6.1 and removed needs: CI labels Sep 22, 2017

0xc0170 merged commit e2c42bb into ARMmbed:master Sep 22, 2017

0xc0170 removed the ready for merge label Sep 22, 2017

LPC1768 IAP Fix #4993

LPC1768 IAP Fix #4993

Uh oh!

Conversation

chrissnow commented Aug 30, 2017

Description

Status

Uh oh!

chrissnow commented Aug 30, 2017

Uh oh!

0xc0170 Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

0xc0170 Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Aug 31, 2017

Uh oh!

0xc0170 commented Sep 4, 2017

Uh oh!

chrissnow commented Sep 4, 2017

Uh oh!

chrissnow commented Sep 4, 2017

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

chrissnow commented Sep 4, 2017

Uh oh!

0xc0170 Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

0xc0170 Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

0xc0170 Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

chrissnow commented Sep 4, 2017

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Sep 21, 2017

Uh oh!

mbed-bot commented Sep 22, 2017

Result: SUCCESS

Output

Uh oh!

0xc0170 commented Sep 25, 2017

Uh oh!

chrissnow commented Sep 25, 2017

Uh oh!

0xc0170 commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrissnow commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrissnow commented Sep 25, 2017

Uh oh!

chrissnow commented Sep 25, 2017

Uh oh!

0xc0170 commented Sep 25, 2017

Uh oh!

0xc0170 commented Sep 25, 2017

Uh oh!

Uh oh!

0xc0170 Aug 31, 2017 •

edited

Loading

0xc0170 commented Sep 25, 2017 •

edited

Loading

chrissnow commented Sep 25, 2017 •

edited

Loading

0xc0170 commented Sep 25, 2017 •

edited

Loading

0xc0170 commented Sep 25, 2017 •

edited

Loading