Dual flash and Hex file support on NCS36510 #4397

pradeep-gr · 2017-05-26T11:44:01Z

Description

Dual flash and Hex file support added

Status

READY

Related PRs

https://github.com/maclobdell/FlashAlgo/pull/2
https://github.com/maclobdell/DAPLink/pull/9

Deploy notes

Build output will be hex file. Post build script will insert FIB, User trim and trims gap.
Post script generates hex and binary file. Binary file required if user uses boot loader to download.

Steps to test or reproduce

Build using 'mbed test --compile'

theotherjimmy · 2017-05-26T18:57:14Z

targets/targets.json

@@ -2801,6 +2801,7 @@
                "value": "0xFFFFFFFF"
            }
        },
+        "OUTPUT_EXT": "bin",


This should be the default...

to switch support to hex file output, this needs to be changed to "OUTPUT_EXT": "hex". even if the post build script takes the bin and converts it to hex, you still need to make this setting so that the online compiler will know to download the hex to the target and not the bin. this setting does require changes to the post build script.

theotherjimmy

Tools changes look fine.

maclobdell · 2017-05-31T14:05:34Z

@studavekar can you check the CI result and re-run if necessary. From the logs, it looks like the failures are unrelated to the code changes made in this PR.

theotherjimmy · 2017-05-31T14:10:20Z

targets/targets.json

@@ -2846,4 +2847,4 @@
        "inherits": ["SARA_NBIOT"],
        "extra_labels": ["ublox", "HI2110", "SARA_NBIOT"]
    }
-}
+}


Can you remove this unrelated change?

I have not done this change.

Huh. Well then. I'll reapprove this PR.

theotherjimmy · 2017-05-31T14:13:03Z

We use the pull request titles in the release notes, which are a markdown document. Could you change the title to avoid use of [ and ]? For example "Support dual flash banks on the NCS36510" Would work.

pradeep-gr · 2017-06-01T07:25:47Z

@theotherjimmy Done

theotherjimmy · 2017-06-01T16:46:08Z

Thanks!

maclobdell · 2017-06-08T15:51:07Z

I checked the build logs and there is no clear failure indicated. I suspect the build test needs to run again.

sg- · 2017-06-08T20:01:39Z

@pradeep-gr It seems that your editor strips newlines. Can you change the setting and reinstate the newlines? This will cause warning and linting to fail

pradeep-gr · 2017-06-09T05:36:52Z

@sg- Done. Can you try now?

0xc0170 · 2017-06-26T07:02:58Z

/morph test

mbed-bot · 2017-06-26T07:48:00Z

Result: FAILURE

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

/morph test

Output

mbed Build Number: 631

Build failed!

0xc0170 · 2017-06-26T08:07:34Z

@pradeep-gr The Ci failed to build some of the tests for this platform, you can find build log here http://mbedosci.cloudapp.net/results/pr/4397/631

pradeep-gr · 2017-06-27T06:40:23Z

cc @danish-iftikhar @danclement @jacobjohnson-ON

sg- · 2017-06-29T17:43:58Z

/morph test

maclobdell · 2017-06-29T21:15:43Z

tools/targets/NCS.py

@@ -211,5 +238,4 @@ def add_fib_at_start(arginput):
    output_hex_file.merge(input_hex_file, overlap='error')

    # Write out file(s)
-    output_hex_file.tofile(file_name_hex, 'hex')
-    output_hex_file.tofile(file_name_bin, 'bin')
+output_hex_file.tofile(file_name_hex, 'hex')


@pradeep-gr
There is a merge error here that is causing everything to fail. Nothing builds until making a change here. Please add 4 spaces in front of this line to align it with the rest of the function.

output_hex_file.tofile(file_name_hex, 'hex')

mbed-bot · 2017-06-30T04:09:08Z

Result: FAILURE

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

/morph test

Output

mbed Build Number: 702

Build failed!

pradeep-gr · 2017-06-30T05:37:56Z

@maclobdell @sg- @danclement @danish-iftikhar I am able to build successfully @ commit 9b082ff for GCC_ARM and ARM. But build not started for IAR.

0xc0170 · 2017-06-30T08:18:19Z

But build not started for IAR.

What does ti mean "not started" ?

pradeep-gr · 2017-06-30T08:49:38Z

@0xc0170
Find attached IAR build result. It stops at first compile and does not proceed further
IAR_Build_Result.txt

maclobdell · 2017-07-03T20:41:55Z

@pradeep-gr The behavior when using IAR compiler sounds similar to what happens to me when it can't access our network license server. It just stops on the first file with no error message. Normally, when I'm connected on our network it can find the server and it compiles. Perhaps you are having a similar issue. I just did a quick test and it is working for me.

Please commit the change to the post build script to your branch referenced in this pull request (master branch of https://github.com/pradeep-gr/mbed-os5-onsemi.git). I definitely recommend you run all the tests and confirm it is all working.

maclobdell · 2017-07-04T03:48:58Z

I ran tests manually using IAR compiler on the branch for the PR (with the change previously mentioned above), plus the master branch. I get 17 errors/fails on this PR branch, but only 1 on master (unrelated issue).
This PR only touches the IAR linker file and post build script + target description for hex file output. The post build script with hex file output are clearly working.

There seems to be a fundamental issue in the IAR linker file that is causing these failures.

The only substantive differences that I see with the new linker file are the following:

the heap size was increased from 0x3000 to 0x4000. But changing it back doesn't help.
the cstack placement is not explicitly set to the end of ram.

previously it was:

place at end of RAM_ALL    { block CSTACK };

now:

place in RAM_region { readwrite, block HEAP, section XHEAP, readonly code section EXECINRAM, overlay MIBOVERLAY, readwrite section MIBENDSECTION, block CSTACK};

@pradeep-gr Can you investigate the map files before and after this change and see if the placement of the heap and stack is the problem?

here are the tests that are failing. I know that many of these are very dependent on RAM allocation.

 NCS36510-IAR 	 Testing read write random blocks                       	 ERROR  
 NCS36510-IAR 	 Testing formatting of master boot record               	 ERROR  
 NCS36510-IAR 	 Testing slicing of a block device                      	 ERROR  
 NCS36510-IAR 	 rtos-rtx5-target_cortex_m-tests-memory-heap_and_stack  	 ERROR  
 NCS36510-IAR 	 Testing calls with 5 args                              	 FAIL   
 NCS36510-IAR 	 tests-mbed_drivers-dev_null                            	 ERROR  
 NCS36510-IAR 	 function init race                                     	 ERROR  
 NCS36510-IAR 	 RTC strftime                                           	 OK     
 NCS36510-IAR 	 STL std::equal                                         	 ERROR  
 NCS36510-IAR 	 tests-mbed_drivers-ticker                              	 FAIL   
 NCS36510-IAR 	 mk time                                                	 ERROR  
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-basic                        	 FAIL   
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-isr                          	 ERROR  
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-mail                         	 ERROR  
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-malloc                       	 ERROR  
 NCS36510-IAR 	 Test dual thread lock locked                           	 ERROR  
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-queue                        	 ERROR  
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-semaphore                    	 ERROR  
 NCS36510-IAR 	 tests-mbedmicro-rtos-mbed-signals                      	 ERROR  
 NCS36510-IAR 	 Testing single thread                                  	 ERROR  
 NCS36510-IAR 	 mbedtls_sha256_self_test                               	 ERROR

theotherjimmy · 2017-07-10T15:29:14Z

@pradeep-gr Can you investigate the map files before and after this change and see if the placement of the heap and stack is the problem?

Have you found anything?

pradeep-gr · 2017-07-11T04:53:17Z

CC @danclement @danish-iftikhar

maclobdell

Here is a summary of a few critical fixes we found. This PR should be good to go once these fixes are made.

maclobdell · 2017-07-13T23:43:33Z

targets/TARGET_ONSEMI/TARGET_NCS36510/device/TOOLCHAIN_IAR/NCS36510.icf


-/* Handle initialization */
-do not initialize { section .noinit };
+define region RAM_region    =   mem:[from __ICFEDIT_region_IRAM1_start__ to __ICFEDIT_region_IRAM1_end__]


There seem to be two critical problems that are causing all the tests to fail. The first is that the stack is not getting placed at the end of RAM. The second issue is that the RAM Vector area is not getting allocated, and the heap is clobbering it.

Creating regions for these sections fixes this. There might be a more elegant solution, but this works.

define region RAM_VECTOR_region = mem:[from __ICFEDIT_region_IRAM3_start__ to __ICFEDIT_region_IRAM3_start__ + 0x90 - 1]; define region RAM_region = mem:[from __ICFEDIT_region_IRAM3_start__ + 0x90 to __ICFEDIT_region_IRAM3_end__] | mem:[from __ICFEDIT_region_IRAM2_start__ to __ICFEDIT_region_IRAM2_end__] | mem:[from __ICFEDIT_region_IRAM1_start__ to __ICFEDIT_region_IRAM1_end__ - __ICFEDIT_size_cstack__ ]; define region CSTACK_region = mem:[from __ICFEDIT_region_IRAM1_end__ - __ICFEDIT_size_cstack__ + 1 to __ICFEDIT_region_IRAM1_end__];

maclobdell · 2017-07-13T23:44:56Z

targets/TARGET_ONSEMI/TARGET_NCS36510/device/TOOLCHAIN_IAR/NCS36510.icf

+define overlay MIBOVERLAY { section MIBSECTION };
+
+define block CSTACK    with alignment = 8, size = __ICFEDIT_size_cstack__   { };
+define block HEAP      with alignment = 8, size = __ICFEDIT_size_heap__     { };



To allocate room for the RAM_VECTORS, define a block similar to CSTACK and HEAP.

define block RAM_VECTORS with alignment = 8, size = 0x90 { };

maclobdell · 2017-07-13T23:45:58Z

targets/TARGET_ONSEMI/TARGET_NCS36510/device/TOOLCHAIN_IAR/NCS36510.icf


-place in RAM_ALL           { readwrite };
-place in RAM_ALL           { block HEAP };
+place in RAM_region { readwrite, block HEAP, section XHEAP, readonly code section EXECINRAM, overlay MIBOVERLAY, readwrite section MIBENDSECTION, block CSTACK};


Add the RAM_VECTORS and CSTACK regions...

place at start of RAM_VECTOR_region {block RAM_VECTORS}; place in RAM_region { readwrite, block HEAP, section XHEAP, readonly code section EXECINRAM, overlay MIBOVERLAY, readwrite section MIBENDSECTION}; place at end of CSTACK_region { block CSTACK };

danclement · 2017-07-17T23:08:06Z

@maclobdell, does using the IAR 7.80.2 version fix these linker script problems? We had a bad version go out and you have to be at 7.80.2 or newer to avoid it. I sent you a quick phone email on this but didn't have a chance to follow-up until now.

If it's more than this, let me know. Jake is back in the office now so we can pick up our progress on mbed.

maclobdell · 2017-07-18T02:14:47Z

@danclement The linker file used to build mbed OS for this target is within mbed os itself. So the changes I listed are needed. It is not related to the IAR IDE version, however you might also supply the linker file with your support files with IAR too.

theotherjimmy · 2017-07-24T15:25:13Z

@pradeep-gr Could you respond to @maclobdell's request for changes?

jacobjohnson-ON · 2017-07-26T00:02:08Z

Generated separate pull request to hasten the requested changes. #4813 should replace this.

binary output file when more than 316K binary is generated. Need updated daplink bootloader to flash >316K binary file by drag n drop.Refer pull request generated for flashalgo and daplink repo for more info.IAR lnker script modified for flash merge-640K and format changed as per IAR requirement.

theotherjimmy · 2017-07-26T15:43:13Z

superseded by #4813, so closing this one.

0xc0170 requested review from theotherjimmy and c1728p9 May 26, 2017 12:02

0xc0170 added the needs: review label May 26, 2017

pradeep-gr changed the title ~~Dual flash and Hex file support added~~ [NCS36510] Dual flash and Hex file support added May 26, 2017

theotherjimmy reviewed May 26, 2017

View reviewed changes

theotherjimmy approved these changes May 26, 2017

View reviewed changes

theotherjimmy suggested changes May 31, 2017

View reviewed changes

pradeep-gr changed the title ~~[NCS36510] Dual flash and Hex file support added~~ Dual flash and Hex file support on NCS36510 Jun 1, 2017

theotherjimmy approved these changes Jun 1, 2017

View reviewed changes

maclobdell changed the title ~~Dual flash and Hex file support on NCS36510~~ Support dual flash banks and Hex file output on NCS36510 Jun 8, 2017

maclobdell changed the title ~~Support dual flash banks and Hex file output on NCS36510~~ Dual flash and Hex file support on NCS36510 Jun 8, 2017

pradeep-gr force-pushed the master branch from 77bd2b3 to 60c5588 Compare June 9, 2017 05:35

0xc0170 approved these changes Jun 12, 2017

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Jun 12, 2017

0xc0170 added the release-version: 5.5.2 label Jun 21, 2017

0xc0170 added needs: work and removed needs: CI labels Jun 26, 2017

maclobdell reviewed Jun 29, 2017

View reviewed changes

0xc0170 removed the release-version: 5.5.2 label Jun 30, 2017

maclobdell suggested changes Jul 13, 2017

View reviewed changes

maclobdell closed this Jul 18, 2017

sg- removed the needs: work label Jul 18, 2017

maclobdell reopened this Jul 18, 2017

theotherjimmy added the needs: work label Jul 24, 2017

jacobjohnson-ON mentioned this pull request Jul 26, 2017

Dual Flash and Hex file support on NCS36510 #4813

Merged

pradeep-gr added 5 commits July 26, 2017 12:18

Hex file support added in python script. Output_ext changed to binary

ee4a3a9

Reverted file to dual flash

ea20b14

OUTPUT_EXT to hex and supporting script added

385d458

Added newline at EOF

576d096

pradeep-gr force-pushed the master branch from 60c5588 to 576d096 Compare July 26, 2017 06:52

theotherjimmy closed this Jul 26, 2017

theotherjimmy removed the needs: work label Jul 26, 2017

Dual flash and Hex file support on NCS36510 #4397

Dual flash and Hex file support on NCS36510 #4397

Uh oh!

Conversation

pradeep-gr commented May 26, 2017

Description

Status

Related PRs

Deploy notes

Steps to test or reproduce

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

maclobdell commented May 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented May 31, 2017

Uh oh!

pradeep-gr commented Jun 1, 2017

Uh oh!

theotherjimmy commented Jun 1, 2017

Uh oh!

maclobdell commented Jun 8, 2017

Uh oh!

sg- commented Jun 8, 2017

Uh oh!

pradeep-gr commented Jun 9, 2017

Uh oh!

0xc0170 commented Jun 26, 2017

Uh oh!

mbed-bot commented Jun 26, 2017

Output

Uh oh!

0xc0170 commented Jun 26, 2017

Uh oh!

pradeep-gr commented Jun 27, 2017

Uh oh!

sg- commented Jun 29, 2017

Uh oh!

maclobdell Jun 29, 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!

mbed-bot commented Jun 30, 2017

Output

Uh oh!

pradeep-gr commented Jun 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Jun 30, 2017

Uh oh!

pradeep-gr commented Jun 30, 2017

Uh oh!

maclobdell commented Jul 3, 2017

Uh oh!

maclobdell commented Jul 4, 2017

Uh oh!

theotherjimmy commented Jul 10, 2017

Uh oh!

pradeep-gr commented Jul 11, 2017

Uh oh!

maclobdell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maclobdell Jun 29, 2017 •

edited

Loading

pradeep-gr commented Jun 30, 2017 •

edited

Loading

maclobdell commented Jul 18, 2017 •

edited

Loading