kinetis/fcfield-check*: merge into single file and support binary flash by fjmolinas · Pull Request #11589 · RIOT-OS/RIOT

fjmolinas · 2019-05-27T14:55:19Z

Contribution description

This PR merges the two existing scripts to check for the fcfield contents on kinetis boards. It also adds to the merged scipt the capacity of checking for bin files.

Regarding flashing at an offset, since the fcfield content is only read on reset, we don't care for its content when flashing an Image after 0x410 or 1040, therefore we won't check its content.

Testing procedure

Modify any value in the fcfield:

RIOT/cpu/kinetis/fcfield.c

Lines 55 to 73 in 8fe12bc

    
           __attribute__((weak, used, section(".fcfield"))) 
        
           const uint8_t flash_configuration_field[] = { 
        
               0xff,    /* backdoor comparison key 3., offset: 0x0 */ 
        
               0xff,    /* backdoor comparison key 2., offset: 0x1 */ 
        
               0xff,    /* backdoor comparison key 1., offset: 0x2 */ 
        
               0xff,    /* backdoor comparison key 0., offset: 0x3 */ 
        
               0xff,    /* backdoor comparison key 7., offset: 0x4 */ 
        
               0xff,    /* backdoor comparison key 6., offset: 0x5 */ 
        
               0xff,    /* backdoor comparison key 5., offset: 0x6 */ 
        
               0xff,    /* backdoor comparison key 4., offset: 0x7 */ 
        
               0xff,    /* non-volatile p-flash protection 1 - low register, offset: 0x8 */ 
        
               0xff,    /* non-volatile p-flash protection 1 - high register, offset: 0x9 */ 
        
               0xff,    /* non-volatile p-flash protection 0 - low register, offset: 0xa */ 
        
               0xff,    /* non-volatile p-flash protection 0 - high register, offset: 0xb */ 
        
               0xfe,    /* non-volatile flash security register, offset: 0xc */ 
        
               KINETIS_FOPT, /* FOPT non-volatile flash option register, offset: 0xd */ 
        
               0xff,    /* non-volatile eeram protection register, offset: 0xe */ 
        
               0xff,    /* non-volatile d-flash protection register, offset: 0xf */ 
        
           };

Flash an elf, hex and bin file and see that it is properly checked:

make -C examples/hello-world/ BOARD=frdm-kw41z flash

### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/francisco/workspace/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf:

/home/francisco/workspace/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf:     file format elf32-littlearm

Contents of section .fcfield:
 0400 ffffffff ffffffff aaffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1

FFLAGS='flash $(HEXFILE)' make -C examples/hello-world/ BOARD=frdm-kw41z flash

### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/francisco/workspace/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.hex:

/home/francisco/workspace/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.hex:     file format ihex

Contents of section .sec1:
 0400 ffffffff ffffffff aaffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1

FFLAGS='flash $(BINFILE)' make -C examples/hello-world/ BOARD=frdm-kw41z binfile flash

Flashing Target

Danger of bricking the device during flash!
Flash configuration field of /home/francisco/workspace/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:

/home/francisco/workspace/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:     file format binary

Contents of section .data:
 0400 ffffffff ffffffff aaffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1

Remove the changes to fcfield, and repeat the above procedure, no errors will be encountered.

Issues/PRs references

Used by #11562

cpu/kinetis/dist/check-fcfield.sh

cladmi · 2019-05-27T14:56:54Z

cpu/kinetis/dist/check-fcfield.sh

+if [ ${FLASHFILE##*.} = bin ]; then
+    OBJDUMP=$(arm-none-eabi-objdump --start-address=${FCFIELD_START} --stop-address=${FCFIELD_END} -bbinary -marm ${FLASHFILE} -s)
+elif [ ${FLASHFILE##*.} = elf ]; then
+    OBJDUMP=$(arm-none-eabi-objdump -j.fcfield -s "${ELFFILE}")


Here should be FLASHFILE.

cpu/kinetis/dist/check-fcfield.sh

fjmolinas · 2019-05-27T15:52:42Z

@cladmi I have addressed your comments. I split merging the scripts and adding binary check in separate commits. Also refactored to use functions and avoid assigning command output to variables, Hopefully it looks better now :) ! (And thanks for the feedback)

cladmi · 2019-05-27T16:45:07Z

I get the correct output without offset:

RIOT_CI_BUILD=1 BOARD=frdm-kw41z make -C examples/hello-world/ flash
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
Building application "hello-world" for "frdm-kw41z" with MCU "kinetis".

   text    data     bss     dec     hex filename
   9084     116    2544   11744    2de0 /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf
/home/harter/work/git/RIOT/dist/tools/openocd/openocd.sh flash /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf
### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf:

/home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf:     file format elf32-littlearm

Contents of section .fcfield:
 0400 00ffffff ffffffff ffffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:538: recipe for target 'flash' failed
make: *** [flash] Error 1
make: Leaving directory '/home/harter/work/git/RIOT/examples/hello-world'

FLASHFILE='$(HEXFILE)' RIOT_CI_BUILD=1 BOARD=frdm-kw41z make -C examples/hello-world/ flash
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
Building application "hello-world" for "frdm-kw41z" with MCU "kinetis".

   text    data     bss     dec     hex filename
   9084     116    2544   11744    2de0 /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf
/home/harter/work/git/RIOT/dist/tools/openocd/openocd.sh flash /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.hex
### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.hex:

/home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.hex:     file format ihex

Contents of section .sec1:
 0400 00ffffff ffffffff ffffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:538: recipe for target 'flash' failed
make: *** [flash] Error 1
make: Leaving directory '/home/harter/work/git/RIOT/examples/hello-world'

FLASHFILE='$(BINFILE)' RIOT_CI_BUILD=1 BOARD=frdm-kw41z make -C examples/hello-world/ flash
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
Building application "hello-world" for "frdm-kw41z" with MCU "kinetis".

   text    data     bss     dec     hex filename
   9084     116    2544   11744    2de0 /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf
/home/harter/work/git/RIOT/dist/tools/openocd/openocd.sh flash /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin
### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:

/home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:     file format binary

Contents of section .data:
 0400 00ffffff ffffffff ffffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:538: recipe for target 'flash' failed
make: *** [flash] Error 1
make: Leaving directory '/home/harter/work/git/RIOT/examples/hello-world'

cladmi

Without offset it works as expected (see comment)

The values with an offset have some issues:

If ${IMAGE_OFFSET} < 0x410 it is not taken into account and the checked memory is the one at offset 0.
No need to handle it properly, failing with "currently not handled" is a good solution.

IMAGE_OFFSET=0x400 FLASHFILE='$(BINFILE)' RIOT_CI_BUILD=1 BOARD=frdm-kw41z make -C examples/hello-world/ flash-only
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
/home/harter/work/git/RIOT/dist/tools/openocd/openocd.sh flash /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin
### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:

/home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:     file format binary

Contents of section .data:
 0400 00ffffff ffffffff ffffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:541: recipe for target 'flash-only' failed
make: *** [flash-only] Error 1
make: Leaving directory '/home/harter/work/git/RIOT/examples/hello-world'

IMAGE_OFFSET==0x410 still triggers the check when it should not as the field is 16bytes long.

IMAGE_OFFSET=0x410 FLASHFILE='$(BINFILE)' RIOT_CI_BUILD=1 BOARD=frdm-kw41z make -C examples/hello-world/ flash
make: Entering directory '/home/harter/work/git/RIOT/examples/hello-world'
Building application "hello-world" for "frdm-kw41z" with MCU "kinetis".

   text    data     bss     dec     hex filename
   9084     116    2544   11744    2de0 /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.elf
/home/harter/work/git/RIOT/dist/tools/openocd/openocd.sh flash /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin
### Flashing Target ###
Danger of bricking the device during flash!
Flash configuration field of /home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:

/home/harter/work/git/RIOT/examples/hello-world/bin/frdm-kw41z/hello-world.bin:     file format binary

Contents of section .data:
 0400 00ffffff ffffffff ffffffff feffffff  ................
Abort flash procedure!
pre-flash checks failed, status=1
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:538: recipe for target 'flash' failed
make: *** [flash] Error 1
make: Leaving directory '/home/harter/work/git/RIOT/examples/hello-world'

cladmi · 2019-05-27T16:52:45Z

cpu/kinetis/dist/check-fcfield.sh

+fi
+
+if [ $(printf '%d' "${IMAGE_OFFSET}") -gt $(printf '%d' "${FCFIELD_END}") ] ; then
+    echo "WARN: we don't check the fcfield value if flashing at \$IMAGE_OFFSET > 0x410"


No need to warn here I think. It is a normal case.

fjmolinas · 2019-05-28T07:55:19Z

@cladmi Addressed comments!

cladmi · 2019-05-28T08:10:26Z

Tests from my previous comments now work as I expect.
I will finish the review.

cladmi · 2019-05-28T08:13:03Z

Please squash.

For the commit in fcfield you can precise in the comment that it is placed at the address 0x400 in memory, so not relative to the .vector position.

- fcfield is located in memory at 0x400-0x40f. Its content is only read upon reset, therefore if in presence of a bootloader and multiple applications, the fcfield will only be read when the bootloader is loaded. As long as we flash at IMAGE_OFFSET > 0x410 we do not care about the fcfield content since it won't get overwritten.

cladmi · 2019-05-28T09:11:20Z

I could correctly use this one with #11562
I am re-running the test suit for good measure.

EDIT: running the test suit still requires #10870 change to the reset_config for boards/frdm to correctly flash all test. But it was the case in master

cladmi

The described tests work as expected, same for my additional ones in the comments.
The RIOT test suite works with similar results to master.

I agree with the change and the way they are done.

fjmolinas added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels May 27, 2019

fjmolinas requested a review from cladmi May 27, 2019 14:55

fjmolinas mentioned this pull request May 27, 2019

boards/frdm-kw41z-k64f: add riotboot #11562

Merged

3 tasks

cladmi suggested changes May 27, 2019

View reviewed changes

fjmolinas added 2 commits May 27, 2019 17:38

kinetis/check-fcfield: merge hex and elf script

963bbe7

kinetis/check-fcfield: add fcfield verirication for bin files

7156977

fjmolinas force-pushed the pr_kinetis_merge_fcfield_check branch 2 times, most recently from 7413651 to 25a0cca Compare May 27, 2019 15:50

cladmi suggested changes May 27, 2019

View reviewed changes

fjmolinas force-pushed the pr_kinetis_merge_fcfield_check branch from ed9d888 to c219fdc Compare May 28, 2019 08:19

cladmi added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 28, 2019

cladmi changed the title ~~kinetis/fcfield-check*: merge into single file~~ kinetis/fcfield-check*: merge into single file and support binary flash May 28, 2019

cladmi approved these changes May 28, 2019

View reviewed changes

cladmi merged commit 0f8372f into RIOT-OS:master May 28, 2019

cladmi mentioned this pull request May 28, 2019

cpu/kinetis: allow flashing without toolchain #11545

Merged

fjmolinas deleted the pr_kinetis_merge_fcfield_check branch May 28, 2019 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kinetis/fcfield-check*: merge into single file and support binary flash#11589

kinetis/fcfield-check*: merge into single file and support binary flash#11589
cladmi merged 3 commits intoRIOT-OS:masterfrom
fjmolinas:pr_kinetis_merge_fcfield_check

fjmolinas commented May 27, 2019

Uh oh!

Uh oh!

cladmi May 27, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented May 27, 2019

Uh oh!

cladmi commented May 27, 2019 •

edited

Loading

Uh oh!

cladmi left a comment

Uh oh!

cladmi May 27, 2019

Uh oh!

fjmolinas commented May 28, 2019

Uh oh!

cladmi commented May 28, 2019

Uh oh!

cladmi commented May 28, 2019

Uh oh!

cladmi commented May 28, 2019 •

edited

Loading

Uh oh!

cladmi left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	__attribute__((weak, used, section(".fcfield")))
	const uint8_t flash_configuration_field[] = {
	0xff, /* backdoor comparison key 3., offset: 0x0 */
	0xff, /* backdoor comparison key 2., offset: 0x1 */
	0xff, /* backdoor comparison key 1., offset: 0x2 */
	0xff, /* backdoor comparison key 0., offset: 0x3 */
	0xff, /* backdoor comparison key 7., offset: 0x4 */
	0xff, /* backdoor comparison key 6., offset: 0x5 */
	0xff, /* backdoor comparison key 5., offset: 0x6 */
	0xff, /* backdoor comparison key 4., offset: 0x7 */
	0xff, /* non-volatile p-flash protection 1 - low register, offset: 0x8 */
	0xff, /* non-volatile p-flash protection 1 - high register, offset: 0x9 */
	0xff, /* non-volatile p-flash protection 0 - low register, offset: 0xa */
	0xff, /* non-volatile p-flash protection 0 - high register, offset: 0xb */
	0xfe, /* non-volatile flash security register, offset: 0xc */
	KINETIS_FOPT, /* FOPT non-volatile flash option register, offset: 0xd */
	0xff, /* non-volatile eeram protection register, offset: 0xe */
	0xff, /* non-volatile d-flash protection register, offset: 0xf */
	};

Conversation

fjmolinas commented May 27, 2019

Contribution description

Testing procedure

Flashing Target

Issues/PRs references

Uh oh!

Uh oh!

cladmi May 27, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented May 27, 2019

Uh oh!

cladmi commented May 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

cladmi May 27, 2019

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented May 28, 2019

Uh oh!

cladmi commented May 28, 2019

Uh oh!

cladmi commented May 28, 2019

Uh oh!

cladmi commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cladmi commented May 27, 2019 •

edited

Loading

cladmi commented May 28, 2019 •

edited

Loading