zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.71k stars 6.54k forks source link

Bossac runner flashes at an incorrect offset #33523

Closed eHammarstrom closed 3 years ago

eHammarstrom commented 3 years ago

Describe the bug When flashing the arduino-nano-33-ble-sense using west the bossac runner passes --offset even though bossac seem to account for the elf offset. This results in a "double" offset of 128kB instead of the 64kB offset where the bootloader starts execution.

To workaround this issue I can flash the device using only the bossac tool, without passing the --offset flag.

The result of the west flash is that the first 64kB of the old image [0x10000, 0x20000] still remain.

To Reproduce

west build -p -b arduino_nano_33_ble zephyr/samples/hello_world
west flash --bossac=/home/estrom/.arduino15/packages/arduino/tools/bossac/1.9.1-arduino2/bossac

If your binaries are sub 64kB then the previous commands result in a "NOP", if you dump flash you can see the old program at 0x10000 and the new program with a 0x20000 offset.

Expected behavior Hello world UART output on the TX and RX pins.

Impact Unable to flash board using west

Logs and console output The bare bossac flash without --offset passed to it,

017C90  3C 01 40 01 44 01 48 01 4C 01 50 01 61 72 64 75  |<.@.D.H.L.P.ardu|
017CA0  69 6E 6F 5F 6E 61 6E 6F 5F 33 33 5F 62 6C 65 00  |ino_nano_33_ble.|
017CB0  48 65 6C 6C 6F 20 57 6F 72 6C 64 21 20 25 73 0A  |Hello World! %s.|

The west bossac flash, which is passed -o 65536,

027C90  3C 01 40 01 44 01 48 01 4C 01 50 01 61 72 64 75  |<.@.D.H.L.P.ardu|
027CA0  69 6E 6F 5F 6E 61 6E 6F 5F 33 33 5F 62 6C 65 00  |ino_nano_33_ble.|
027CB0  48 65 6C 6C 6F 20 42 61 6A 73 74 69 61 6E 21 20  |Hello Bajstian! |

Environment (please complete the following information): Linux, Debian Testing zephyr-sdk 0.12.3 bossac "1.9.1-arduino2" shipped with the arduino IDE zephyr v2.5-branch

nandojve commented 3 years ago

This board was introduced at #29097. At that time, the bossac used was nRF version as mention at https://github.com/zephyrproject-rtos/zephyr/pull/29097#issuecomment-753635609

@saltyJeff did you know if there is difference between the two versions: nRF vs Arduino?

saltyJeff commented 3 years ago

I couldn't install the arduino IDE on my linux distro for some reason, so I cloned the mainline repo and used it:

It was the "nRF" branch of the "Arduino" main repository that I used to flash: https://github.com/arduino/BOSSA

I assumed they were the same but something may have changed in the past few months. Can you check the hashes for the Arduino copy and the in-tree build?

saltyJeff commented 3 years ago

Also found this: @29312

At same way, we need harmonize newer SDK with old ones that only support BOSSA 1.7 and don't have offset parameter. On the newer SDKs if user don't define CONFIG_USE_DT_CODE_PARTITION he will get offset=0 all times and there are dead codes on west. With old SDKs bossac aborts because there is no --offset parameter.

This is listed as an undesired behavior but I believe we've hit an edge case.

nandojve commented 3 years ago

Also found this: @29312

At same way, we need harmonize newer SDK with old ones that only support BOSSA 1.7 and don't have offset parameter. On the newer SDKs if user don't define CONFIG_USE_DT_CODE_PARTITION he will get offset=0 all times and there are dead codes on west. With old SDKs bossac aborts because there is no --offset parameter.

Fixed at #29874.

This is listed as an undesired behavior but I believe we've hit an edge case.

The #29874 introduced a lot of tests that cover many scenarios. I was wondering if someone of them not cover this. In this case, it will only require add/rem/change board definitions and it will be an easy fix.

@saltyJeff , could you make me a favor? build/flash mainline using your setup. This will help us to guarantee that current mainline not changed some internal behavior around flash variables, ok?

saltyJeff commented 3 years ago

I'm swamped this week but if @eHammarstrom wants to try, I believe the steps are:

If not, I'll probably get around to it next week.

eugmes commented 3 years ago

I've tried to use bossac from current Arduino (Version 1.9.1-17-g89f3556) with the current master (f6f951cc1709a23fa0a6e4d74337667f90a03af1) on MacOS and the application keeps rebooting. Removing -o parameter to bossac helps.

nandojve commented 3 years ago

Hello @eHammarstrom ,

As comment at BOSSAC RFC, offset indicate where erase starts. You should ensure to remove knowledge of the bootloader size from BOSSAC, see https://github.com/shumatech/BOSSA/commit/dbdd088909c57bdf9aee191720bf6c6159217e22.

As mentioned

To workaround this issue I can flash the device using only the bossac tool, without passing the --offset flag.

This means that your bootloader/tool is not following what was defined by shumatech/BOSSA. I'm fine with requiring that it be explicitly specified for non-ROM bootloaders like the SAMD's. Originally posted by @shumatech in https://github.com/shumatech/BOSSA/issues/79#issuecomment-415996880

The Zephyr west bossac follows https://github.com/shumatech/BOSSA/issues/79. At end, west bossac should pass --offset for all non-ROM bootloaders when --offset is available.

If there are something I miss, help me identify what is the proper use case to add at west bossac tests.

eHammarstrom commented 3 years ago

@nandojve Thanks for clarifying this, I was not aware that the binaries differed in the --offset behavior.

I wonder if it would be beneficial for West's bossac runner to output a warning if a non-zephyr bossac binary is supplied?

If not, you may close this ticket.

eugmes commented 3 years ago

@nandojve: The documentation here https://github.com/zephyrproject-rtos/zephyr/blob/master/boards/arm/arduino_nano_33_ble/doc/index.rst#programming-and-debugging says to use bossac from Arduino IDE. Is there a special Zephyr version of bossac that should be used instead?

nandojve commented 3 years ago

@eHammarstrom ,

I wonder if it would be beneficial for West's bossac runner to output a warning if a non-zephyr bossac binary is supplied?

I'm not sure if it is possible detect.

@eugmes , as far I know, @saltyJeff used a special nRF branch, see his comments here: https://github.com/zephyrproject-rtos/zephyr/issues/33523#issuecomment-804392269

Stefan-Schmidt commented 3 years ago

Just wanted to confirm what @eugmes wrote at https://github.com/zephyrproject-rtos/zephyr/issues/33523#issuecomment-817208257 I was hitting the same issue when using the recommended bossac version from the documentation (arduino IDE or compiled manually).

Removing the offset adding in scripts/west_commands/runners/bossac.py line 167&168 works around this for me, but there is clearly a gap between what is documented and working in Zephyr mainline.

nandojve commented 3 years ago

Can someone test the below devicetree configuration. I changed from reg = <0x10000 0xf0000>; to reg = <0x0 0xf0000>;

&flash0 {
    partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        code_partition: partition@0 {
            label = "code";
            reg = <0x0 0xf0000>;
            read-only;
        };
    };
};
Stefan-Schmidt commented 3 years ago

@nandojve Just checked and it does not solve the problem. Maybe the offset is still somehow being applied from west flash bossac.py runner?

nandojve commented 3 years ago

Could you execute west flash using -v option and print here the command line? west -v flash

This change on the devicetree should avoid --offset parameter because start address is 0.

nandojve commented 3 years ago

There are two solutions to this board. I think add a compatibility mode is better than change flash layout. I'm waiting feedback about #34947 to finish the work.

Stefan-Schmidt commented 3 years ago

Just tried with your PR and I can confirm it fixes the issue for me. I assume that means you do not want any verbose flash output anymore. If you still need any logs or such let me know.

Thanks for working on this!

nandojve commented 3 years ago

Hi @Stefan-Schmidt , No need for verbose output on previous case. There are two solution and I believe that #34947 is the most recommended solution.

I'll finish PR and open to review soon. Thank you all for bring this to us.

WilliamGFish commented 3 years ago

Part of the issue is there are two versions of BOSSA and the Arduino team have forked from the original shumatech one. The one mentioned in the nano 33 ble info is that forked version which has been redeveloped to work specifically with the nRF based boards. I expect that there will be further improvements made this version needs to be included in the west repository.

nandojve commented 3 years ago

Hi @WilliamGFish ,

Did you checked #34947 ? Is it working for you?

WilliamGFish commented 3 years ago

Hi @WilliamGFish ,

Did you checked #34947 ? Is it working for you?

I have applied the PR and it appears to work. The USB port on the nano 33 still doesn't seem to function. I have used both versions of 1.9.1 the shumatech.com and Arduino IDE version, they both flash successfully.

I have changed some of the switches to provide better feedback in the bossac west runner script: (scripts\west_commands\runners\bossac.py - ln 159)

        cmd_flash = [self.bossac, '-d', '-i', '-U', '-p', self.port, '-R', '-e', '-w', '-v',
                      self.cfg.bin_file]

and amended the Windows error check: (ln 194)

        if platform.system() == 'Windows' and self.port == DEFAULT_BOSSAC_PORT:
            msg = 'CAUTION: BOSSAC runner on Windows requires --bossac-port defined, ie COM7!'
            raise RuntimeError(msg)

To get west to flash to flash to boards in Windows I am using: west -v flash --bossac="bossac.exe" --bossac-port="COM7"

You want to roll these amendments in your PR (and give me a mention)