Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bossac runner flashes at an incorrect offset #33523

Closed
eHammarstrom opened this issue Mar 20, 2021 · 20 comments · Fixed by #34947
Closed

Bossac runner flashes at an incorrect offset #33523

eHammarstrom opened this issue Mar 20, 2021 · 20 comments · Fixed by #34947
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Milestone

Comments

@eHammarstrom
Copy link
Contributor

eHammarstrom commented Mar 20, 2021

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

@eHammarstrom eHammarstrom added the bug The issue is a bug, or the PR is fixing a bug label Mar 20, 2021
@galak galak added the priority: low Low impact/importance bug label Mar 22, 2021
@nandojve
Copy link
Member

This board was introduced at #29097. At that time, the bossac used was nRF version as mention at #29097 (comment)

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

@saltyJeff
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

nandojve commented Mar 22, 2021

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
Copy link
Contributor

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
Copy link
Contributor

eugmes commented Apr 10, 2021

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

@nandojve
Copy link
Member

nandojve commented May 2, 2021

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 shumatech/BOSSA@dbdd088.

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 shumatech/BOSSA#79 (comment)

The Zephyr west bossac follows shumatech/BOSSA#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
Copy link
Contributor Author

@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
Copy link
Contributor

eugmes commented May 2, 2021

@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
Copy link
Member

nandojve commented May 2, 2021

@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: #33523 (comment)

@Stefan-Schmidt
Copy link
Contributor

Just wanted to confirm what @eugmes wrote at #33523 (comment) 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
Copy link
Member

nandojve commented May 5, 2021

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
Copy link
Contributor

@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
Copy link
Member

nandojve commented May 6, 2021

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
Copy link
Member

nandojve commented May 7, 2021

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
Copy link
Contributor

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
Copy link
Member

nandojve commented May 7, 2021

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
Copy link
Collaborator

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
Copy link
Member

Hi @WilliamGFish ,

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

@WilliamGFish
Copy link
Collaborator

WilliamGFish commented May 10, 2021

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)

@galak galak added this to the v2.6.0 milestone May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants