openocd.sh: allow flashing binary files without configuration

[cladmi]

Contribution description

This adds support for flashing binary files using openocd and not configuring anything.

When flashing binaries, openocd needs the absolute address for flashing.
There was already IMAGE_OFFSET but it required configuration for flashing by default for boards that do not start at 0x0, like iotlab-m3.

The rom base address is now queried from openocd and IMAGE_OFFSET is added to that.

• API change to IMAGE_OFFSET with binary files as it should now be an offset to the base address
  • This makes it more compatible with edbg behavior too, which as an --offset variable that takes into account the base address.
• FIX verify image that is not using IMAGE_OFFSET when it should
• Use openocd flash list to extract the first bank address
  • I did not make this bank number configurable for now. It could be addressed later if necessary.

API change

This is an API change but would still consider it minor as it only affects openocd when flashing binary files and was also not working as IMAGE_OFFSET was not set for verify_image. So changing an API of something that did not work.

Testing

I tested with iotlab-m3 but using any board using openocd should do. Even better if the board start address is not zero, and if it has with multiple rom banks.

For easier eye debugging, openocd can be run with sh -xc instead of sh -c here:

RIOT/dist/tools/openocd/openocd.sh

Line 163 in 31aba49

sh -c "${OPENOCD} \

Flash an elf with offset

make -C examples/hello-world BOARD=iotlab-m3 flash IMAGE_OFFSET=0x1000
# With debug output there is the correct address
-c 'flash write_image erase "/RIOT/examples/hello-world/bin/iotlab-m3/hello-world.elf" 0x1000 '

# In master it generated
# Error: checksum mismatch - attempting binary compare
# diff 0 address 0x08000000. Was 0x7f instead of 0x00
# diff 1 address 0x08000001. Was 0x45 instead of 0x02
# diff 2 address 0x08000002. Was 0x4c instead of 0x00

Flash a simple binary without configuration

make -C examples/hello-world BOARD=iotlab-m3 IMAGE_FILE=bin/iotlab-m3/hello-world.bin binfile flash-only
# In master it generated
# Warn : no flash bank found for address 0            
# wrote 0 bytes from file bin/iotlab-m3/hello-world.bin in 0.001894s (0.000 KiB/s)

Flash a binary with offset

make -C examples/hello-world BOARD=iotlab-m3 IMAGE_FILE=bin/iotlab-m3/hello-world.bin binfile flash-only IMAGE_OFFSET=0x100
# With debug output there is the correct address
-c 'flash write_image erase "bin/iotlab-m3/hello-world.bin" 0x8000100 '

Issues/PRs references

This is part of the OTA implementation required features in #9342

[kYc0o]

What do you call bank here?

[cladmi]

I used the term from openocd documentation: http://www.openocd.org/doc/html/Flash-Commands.html and here http://repo.or.cz/openocd.git/blob_plain/HEAD:/tcl/target/stm32f1x.cfg (flash bank).

From what I understood, it is a contiguous area of ROM.

By setting a 1 here, I assume that, if the board has multiple memory banks, the first one is used.

[kYc0o]

I reviewed the code and it makes sense to me. However I want to test it... Would it be possible to add a flash-bin target to use it? This means that we need systematically a bin file generated, but it can come as a dependency of the flash-bin target, so it's only generated when this recipe is used.

[cladmi]

To have a generic flash-bin it would require #8838
I could have a target for openocd but it can also be tested without.

You can test with the examples in the PR description:

make -C examples/hello-world BOARD=iotlab-m3 IMAGE_FILE=bin/iotlab-m3/hello-world.bin binfile flash-only
make -C examples/hello-world BOARD=iotlab-m3 IMAGE_FILE=bin/iotlab-m3/hello-world.bin binfile flash-only IMAGE_OFFSET=0x100

I am also thinking about adding a test for a two step flashing but would currently require specific handling for edbg and openocd.

[kYc0o]

I've tested it successfully with your instructions, thanks!

However, I find that it would be useful to warn while flashing that openocd is writing with an offset, something like writing file_name with offset 0x100 for instance.

The file name is also different from what is today on master. It gives the full path while flashbin gives only the relative path. Can we normalise this into showing the full path?

[cladmi]

It is because I gave a relative file in the command line. This gives the full path:

make -C examples/hello-world BOARD=iotlab-m3 IMAGE_FILE=${PWD}/bin/iotlab-m3/hello-world.bin binfile flash-only

I did not added the offset flashing, it was already in master.

RIOT/dist/tools/openocd/openocd.sh

Lines 90 to 95 in 2d27b32

	# This is an optional offset to the base address that can be used to flash an
	# image in a different location than it is linked at. This feature can be useful
	# when flashing images for firmware swapping/remapping boot loaders.
	# Default offset is 0, meaning the image will be flashed at the address that it
	# was linked at.
	: ${IMAGE_OFFSET:=0}

I only fixed that the 'verify' was not using the offset.

What I did, is that now if you provide a binary file, I add the ROM_BASE_ADDR.
I could add an info for this.

[kYc0o]

it is because I gave a relative file in the command line. This gives the full path:

That's what I meant with adding a new target flash-bin which depends on binfile to generate the file and it can be passed directly with its full path. Maybe that's easier isn't it?

I did not added the offset flashing, it was already in master.

That was not my question, sorry for not being clear. What I mean that I'd like to know if the flash script is flashing using an offset printing in the script something like Flashing at address 0x... which can only happen when flashing binfiles.

What I did, is that now if you provide a binary file, I add the ROM_BASE_ADDR.
I could add an info for this.

I think by printing the direct flashing address this is included.

[kYc0o]

For the changes I'm asking I think it's better to wait for FLASHFILE so everything would fit better.

ACK.

[kYc0o]

@cladmi ok to merge?

[cladmi]

There is not conflicts so I would say yes.

[cladmi]

The test for a binary file is wrong. I tried flashing a text based binary so was not recognized as binary.

I could replace it by testing the extension and an env variable to force it. Any preference on this ?

After patching it, I tested in tests/periph_flashpage:

make BOARD=iotlab-m3 IMAGE_FILE=${PWD}/bin/iotlab-m3/tests_periph_flashpage.bin binfile flash-only
make BOARD=iotlab-m3 IMAGE_FILE=hello_riot.bin IMAGE_OFFSET=0x7f800 flash-only  QUIET=0
make BOARD=iotlab-m3 term
dump 255
> dump 255                                                                                                                                                                                                        
2018-08-28 18:25:22,259 - INFO #  dump 255                                                                                                                                                                        
2018-08-28 18:25:22,259 - INFO # Flash page 255 at address 0x807f800                                                                                                                                              
2018-08-28 18:25:22,260 - INFO # 0x48 0x65 0x6c 0x6c 0x6f 0x20 0x57 0x6f 0x72 0x6c 0x64 0x20 0x52 0x49 0x4f 0x54                                                                                                  
2018-08-28 18:25:22,261 - INFO #   H    e    l    l    o         W    o    r    l    d         R    I    O    T                                                                                                   
2018-08-28 18:25:22,262 - INFO # 0x21 0x21 0x21 0x21 0x0a 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff                                                                                                  
2018-08-28 18:25:22,262 - INFO #   !    !    !    !    ??   ??   ??   ??   ??   ??   ??   ??   ??   ??   ??   ??                                                                                                  
2018-08-28 18:25:22,263 - INFO # 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 

[cladmi]

I cannot block my own PR…

[kYc0o]

I cannot block my own PR…

UnACK.

[cladmi]

I added debug output for IMAGE_OFFSET and the FLASH_ADDR used for binary files.

I changed the test to check the file extension and also use IMAGE_TYPE existing variable to force it.

IMAGE_TYPE:

• Not a binfile
  • IMAGE_FILE=abc.bin IMAGE_TYPE=elf
  • IMAGE_FILE=abc.elf
• binfile:
  • IMAGE_FILE=abc.bin
  • IMAGE_FILE=abc IMAGE_TYPE=bin

The periph_flashpage test also works.

[cladmi]

@kYc0o for your expr not found problem mentioned IRL, I will change the script to use bash #! /bin/bash as a pre-dependency and rebase on it.
I will then update my commits accordingly with fixups too as I currently limited myself to /bin/sh features.

[cladmi]

I redid the periph_flashpage test and the binfile detection:

touch hello
touch hello.bin
touch hello_notabin

# Binfile detected
make flash-only BOARD=iotlab-m3 IMAGE_FILE=hello IMAGE_TYPE=bin 2>&1 | grep -m 1 'Binfile detected'
# Binfile detected, adding ROM base address: 0x08000000
make flash-only BOARD=iotlab-m3 IMAGE_FILE=hello.bin 2>&1 | grep -m 1 'Binfile detected'
# Binfile detected, adding ROM base address: 0x08000000

# Not binfile
make flash-only BOARD=iotlab-m3 IMAGE_FILE=hello 2>&1 | grep -m 1 'Binfile detected'
make flash-only BOARD=iotlab-m3 IMAGE_FILE=hello_notabin 2>&1 | grep -m 1 'Binfile detected'
make flash-only BOARD=iotlab-m3 IMAGE_FILE=hello.bin IMAGE_TYPE=elf 2>&1 | grep -m 1 'Binfile detected'

@kYc0o can you retry with mac ?

[kYc0o]

Tested with success on a nucleo-l152re on OSX. Squash and re-ACK.

[cladmi]

Rebased with fix included to print as %08x to be coherent with flash_addr.

diff --git a/dist/tools/openocd/openocd.sh b/dist/tools/openocd/openocd.sh
index 04faaaa67..a4c72dd38 100755
--- a/dist/tools/openocd/openocd.sh
+++ b/dist/tools/openocd/openocd.sh
@@ -204,7 +204,7 @@ do_flash() {
         FLASH_ADDR=$(_flash_address 1)
         echo "Binfile detected, adding ROM base address: ${FLASH_ADDR}"
         IMAGE_TYPE=bin
-        IMAGE_OFFSET=$(printf "0x%x\n" "$((${IMAGE_OFFSET} + ${FLASH_ADDR}))")
+        IMAGE_OFFSET=$(printf "0x%08x\n" "$((${IMAGE_OFFSET} + ${FLASH_ADDR}))")
     fi

     if [ "${IMAGE_OFFSET}" != "0" ]; then

[kYc0o]

That's ok, just a beautifier. Go!