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

Add support for the Arduino nano 33 BLE sense #29097

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

saltyJeff
Copy link
Contributor

This PR adds board support for the Arduino Nano 33 BLE and its Sense variant.

@saltyJeff saltyJeff force-pushed the master branch 5 times, most recently from 279c505 to 05fd708 Compare October 11, 2020 05:55
@MaureenHelm MaureenHelm requested a review from ioannisg October 12, 2020 21:23
@ioannisg ioannisg added the Release Notes To be mentioned in the release notes label Oct 26, 2020
@ioannisg ioannisg added this to the v2.5.0 milestone Oct 26, 2020
};

&flash0 {
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove this multi-line comment? The link does not exist any more

* these bindings are irrelevant. See the LD script
*/
partitions {
compatible = "fixed-partitions";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you're defining only the code-partition here, nothing about bootloader or storage (see other ARM boards for reference).

Since the code-partition does not start at 0x0, maybe you could define the area from 0x0 to 0x10000 as the bootloader partition? Otherwise, how would the image, loaded in the code-partition be booted?

I am also wondering what happens to the flash area above the code partition - why is it not included in the code-partition - is it used by sthg else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Arduino has their own BOSSA-adjacent bootloader in the region from 0-0x10000. If I tag this region as bootloader do I need to do anything to make sure that MCUBoot doesn't get written over the Arduino-supplied bootloader (it's a pain to flash the original back).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see that you are using a custom linker script with the following snippet:

MEMORY
{
  FLASH (rx) : ORIGIN = 0x10000, LENGTH = 0xf0000
  RAM_NVIC (rwx) : ORIGIN = 0x20000000, LENGTH = 0x100
  RAM_CRASH_DATA (rwx) : ORIGIN = (0x20000000 + 0x100), LENGTH = 0x100
  RAM (rwx) : ORIGIN = ((0x20000000 + 0x100) + 0x100), LENGTH = (0x40000 - (0x100 + 0x100))
}

This one seems to overwrite the common ARM linker script. So it basically overrides all the DeviceTree / Kconfig infrastructure for defining flash and ram areas.

Could you get rid of this, and find a way that you can use the common ARM linker script "as is"? At least for FLASH and SRAM.

For example, I see that you would like to have your code area starting from 0x10000 and with size 0xf0000. Should that always be the case? If so, you can define your code partition as follows:

code_partition: partition@10000 {
			label = "code";
			reg = <0x10000 0xf0000>;
			read-only;
};

And (as it is already the case in the .dts) choose it:

zephyr,flash = &flash0;
zephyr,code-partition = &code_partition;

For this to be enforced by the linker, you need to enable the CONFIG_USE_DT_CODE_PARTITION in your board definition.

This has nothing to do with MCUboot.
And, by the way, MCUboot requires a partition named "boot_partition", but you do not define such thing here. So, MCUboot won't work. If you do not need MCUboot, you can ignore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @saltyJeff , you should follow what @ioannisg recommended. I complement with, you may need look at #29312 and #29874. to be aware. The have BOSSAC fully integrated with Zephyr west tool, you need SDK 0.12.0 (https://github.com/zephyrproject-rtos/sdk-ng/releases/tag/v0.12.0-beta-1) to get support to SAM-BA compatible bootloaders and 29874.

As @ioannisg said, you need CONFIG_USE_DT_CODE_PARTITION . However, if you use BOSSAC Arduino/Adafruit UF2 compatible you need define at <board>_defconfig
CONFIG_BOOTLOADER_BOSSA=y
CONFIG_BOOTLOADER_BOSSA_ARDUINO=y or CONFIG_BOOTLOADER_BOSSA_ADAFRUIT_UF2=y
This will auto select CONFIG_USE_DT_CODE_PARTITION and enables the board to reset and enter on bootloader mode too.

You can look at arduino_nano_33_iot or adafruit_itsybitsy_m4_express as reference.

CONFIG_BOARD_ARDUINO_NANO_33_BLE=y

# Enable MPU
CONFIG_ARM_MPU=y
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you also enable HW_STACK_PROTECTION by default (it's now a recommendation for ARM boards)

Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments around flash partition definition and default Configs.

@ioannisg ioannisg requested a review from anangl November 10, 2020 18:36
@ioannisg ioannisg added the platform: nRF Nordic nRFx label Nov 11, 2020
@saltyJeff
Copy link
Contributor Author

Thanks for all the advice everyone! Just a heads up midterms are getting a little spicy, I'll start work on a new revision around Thanksgiving break, so sorry if it seems like I'm ghosting this PR.

@saltyJeff
Copy link
Contributor Author

saltyJeff commented Nov 26, 2020

I've updated devicetree and defconfig. I did run into some hiccups:

EDIT found it: https://github.com/arduino/BOSSA

@nandojve
Copy link
Member

Hi @saltyJeff ,

I've updated devicetree and defconfig. I did run into some hiccups:

depends on SOC_FAMILY_SAM0 || SOC_SERIES_NRF52X

EDIT found it: https://github.com/arduino/BOSSA

Zephyr will have a new SDK that supports bossac 1,9.1+ SDK 0.12.0-beta-2 . There are some issues and you need apply this #29874 .

Would you mind run a test to make sure if nRF patch is already in there?

If it fails, you can point the bossac version that works for you until everything be ready:

at board.cmake
board_runner_args(bossac "--bossac=<you bossac path>")

CC: @galak

@saltyJeff
Copy link
Contributor Author

saltyJeff commented Nov 27, 2020

Hi @nandojve, I did some more digging and this is what I found:

CONFIG_BOSSA_BOOTLOADER
depends on SOC_FAMILY_SAM0 || SOC_SERIES_NRF52X

The flag is actually for building the BOSSA bootloader itself: https://github.com/zephyrproject-rtos/zephyr/blob/045cc2eb2a7808e18d9859b57b545fd6bf0c21f2/soc/arm/atmel_sam0/common/CMakeLists.txt.

Since the board comes pre-flashed with a bootloader, I'll just leave this option off.

Zephyr will have a new SDK that supports bossac 1,9.1+ SDK 0.12.0-beta-2 . There are some issues and you need apply this #29874 .

Would you mind run a test to make sure if nRF patch is already in there?

It looks like the Arduino version of BOSSA is going to be a hard fork, so NRF support will never be there. Mainline BOSSA was designed for the SAM* products only, for some reason the Arduino team hacked a SAM* bootloader into the NRF line. I've added notes to the board's documentation on the need for a different version of bossac.

If it fails, you can point the bossac version that works for you until everything be ready:

at board.cmake
board_runner_args(bossac "--bossac=<you bossac path>")

I added notes to the documentations about this flag.

@nandojve
Copy link
Member

Hi @nandojve, I did some more digging and this is what I found:

CONFIG_BOSSA_BOOTLOADER
depends on SOC_FAMILY_SAM0 || SOC_SERIES_NRF52X

The flag is actually for building the BOSSA bootloader itself: https://github.com/zephyrproject-rtos/zephyr/blob/045cc2eb2a7808e18d9859b57b545fd6bf0c21f2/soc/arm/atmel_sam0/common/CMakeLists.txt.

Since the board comes pre-flashed with a bootloader, I'll just leave this option off.

The code that you select have the function to reset the board into bootloader mode, it is not the bootloader. That code is dedicated to SAM0 SoC and can be used as reference to make a similar one for nRF. It is not mandatory to have that code implemented for nRF right now. The draw back is, you need put your own board into bootloader mode before call west flash. I think it will be better you work on it after.

Zephyr will have a new SDK that supports bossac 1,9.1+ SDK 0.12.0-beta-2 . There are some issues and you need apply this #29874 .
Would you mind run a test to make sure if nRF patch is already in there?

It looks like the Arduino version of BOSSA is going to be a hard fork, so NRF support will never be there. Mainline BOSSA was designed for the SAM* products only, for some reason the Arduino team hacked a SAM* bootloader into the NRF line. I've added notes to the board's documentation on the need for a different version of bossac.

If it fails, you can point the bossac version that works for you until everything be ready:

at board.cmake
board_runner_args(bossac "--bossac=<you bossac path>")

I added notes to the documentations about this flag.

If you want use west flash you need add support to external BOSSA, since the current one don't have it. It will be important have all documented, including the procedure to put board into bootloader mode before call west flash.

The PR #29874 fix support to BOSSA bootloader and handle the --offset parameter for you. You will need keep the entries at board defconfig file.

CONFIG_BOOTLOADER_BOSSA
CONFIG_BOOTLOADER_BOSSA_ARDUINO or CONFIG_BOOTLOADER_BOSSA_ADAFRUIT_UF2

If the bootloader mode is not Arduino or Adafruit_UF2, it will be necessary add the correspondent one at Kconfig.zephyr and scritps/west_commands/runners/bossac.py. The last one after #29874 merge.

@saltyJeff
Copy link
Contributor Author

The west flash --bossac command has already been added:

parser.add_argument('--bossac', default='bossac',

The procedure to enter bootloader mode is summarized in my docs here: https://github.com/saltyJeff/zephyr/blob/master/boards/arm/arduino_nano_33_ble/doc/index.rst#programming-and-debugging

The Arduino bootloader is only distributed in binary/ELF form. I'll spend some time objdumping to try and figure out what the magic number is.


&flash0 {
/*
* For more information, see:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saltyJeff this link does not seem to exist any more, please remove the comment.

Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this looks OK to go (flash partitioning can be improved later as @nandojve suggested, but at least now .dts defines and selects a code partition to be used).

@saltyJeff you need to address the compliance issues found by CI.

@nandojve
Copy link
Member

nandojve commented Dec 7, 2020

The #29874 got merged. @saltyJeff can you confirm that bossac runner is working OK for you?

@saltyJeff
Copy link
Contributor Author

It's currently week 10 for us, I'll try to clean up all the compliance issues and test bossac during winter break.

@WilliamGFish
Copy link
Collaborator

Good job Salty boy. @saltyJeff
It compiles and runs, I did need to manually flash the Nano. Bossa doesn't work in Windows with west; undoubtedly due to the \ vs / issues (escape chars).

I have it running within a self provisioning ble mesh and getting basic sensor readings, not checked 'em in detail.

The only things that are bugging me:

  • Lack of console

  • USB ID Settings

  CONFIG_USB_DEVICE_VID has default value 0x2FE3.

  This value is only for testing and MUST be configured for USB products.

I'll have a better look in the coming days.

Billy..

tx-pin = <35>; //P1.03
rx-pin = <42>; //P1.10
};
&i2c0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the schematics you may want to swap these as the sensors are connected to pins 14/15.

@WilliamGFish
Copy link
Collaborator

@saltyJeff
I've had another play and found that zephyr can not see any of the sensors on the i2c bus. Without a USB console I can't offer much more help.

label = "Red LED";
};
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Nano 33 BLE I have, red is pin 22, green 23 and blue 24 for the RGB LED. The power LED is 25 and the Arduino "built in" LED is 13.

@saltyJeff saltyJeff force-pushed the master branch 4 times, most recently from 4a4bf17 to a064b23 Compare January 3, 2021 10:41
@saltyJeff
Copy link
Contributor Author

@WilliamGFish I've reverified my I2C capability, it's working on my side.

The program I ran (no proj.conf or anything besides the default CMakeLists.txt was used): https://gist.github.com/saltyJeff/c1a0a6f1a6f53fb007e31c4b5951928f

Waveforms:
image

@astavale I've also verified my pin numbers off the online schematics. The R channel of the RGB LED connects to P0.24.

I think you may be confused on how the pins are numbered, the Arduino pin numbers mean nothing. In the DTS, there's a guide on how to write in a pin of the form PX.Y which becomes 32*X + Y in the DTS.

Another reason is that I bought a late-model of the board, and I note that the board has a V2 appended to the schematic. You may be holding an early proto of the board.

@nandojve I tried the new SDK's build tools and the new bossac still doesn't play nice with the board. The Arduino hard fork version is the only one that supports this board.

As for forcing the device into bootloader mode, I've disassembled the bootloader from the arduino repo.

The results are here: https://gist.github.com/saltyJeff/65e5033703de757ad99bcbd57664cec1

I tried to grep for both magic numbers in bossac.c. I did find the CONFIG_BOOTLOADER_BOSSA_ARDUINO magic number (0x07738135) just hanging around at the end of the main function. Because the SAM version involves moving this value to a constant address, I assumed I would find the same in the bootloader disassembly but I couldn't find it anywhere.

@nandojve
Copy link
Member

nandojve commented Jan 3, 2021

Hi @saltyJeff

Small recap to sync all info about BOSSA:

  1. bootloader: OK, nRF version
  2. put device into bootloader mode: Ok, it is manual - double tap on the switch.
  3. bossac: OK, need to use nRF version
  4. Arduino nano 33 Zephyr config: OK. It create a valid image. The image can be loaded into device invoking [3] from command line, for instance.
  5. west (bossac runner): OK using west flash --bossac=xxx.

[2] I think that put device into bootloader mode automatically should be a future step. This way we can move PR forward.
[5] Since #29874 got merged, can you confirm that you are using Zephyr mainline version that have that changes ?

@saltyJeff
Copy link
Contributor Author

saltyJeff commented Jan 4, 2021

Hi @saltyJeff

Small recap to sync all info about BOSSA:

1. bootloader: OK, nRF version

2. put device into bootloader mode: Ok, it is manual - double tap on the switch.

3. bossac: OK, need to use [nRF version](https://github.com/arduino/BOSSA/tree/nrf)

4. Arduino nano 33 Zephyr config: OK. It create a valid image. The image can be loaded into device invoking [3] from command line, for instance.

5. west (bossac runner): OK using **west flash --bossac=xxx**.

This is an accurate summary

[2] I think that put device into bootloader mode automatically should be a future step. This way we can move PR forward.

I agree, hopefully the Arduino ppl will eventually release the source code

[5] Since #29874 got merged, can you confirm that you are using Zephyr mainline version that have that changes ?

I'm having issues building the project after merging my changes into the Zephyr master. I believe I've come upon a regression:

what I tried

I ran west build --board=arduino_nano_33_ble samples/basic/blinky:

My CMake build trace:

In file included from ../include/kernel_includes.h:46,
                 from ../include/kernel.h:17,
                 from /home/jeffe/zephyrproject/zephyr/arch/arm/core/offsets/offsets_aarch32.c:28,
                 from /home/jeffe/zephyrproject/zephyr/arch/arm/core/offsets/offsets.c:12:
../include/sys/kobject.h:317:10: fatal error: syscalls/kobject.h: No such file or directory
  317 | #include <syscalls/kobject.h>
      |          ^~~~~~~~~~~~~~~~~~~~

The file in question:

#include <syscalls/kobject.h>

I can't find the requested header anywhere in the source tree.

jeffe@jeffe-desktop:~/zephyrproject/zephyr$ find . -name kobject.h
./subsys/testsuite/ztest/include/syscalls/kobject.h
./include/sys/kobject.h

I've run west update and downloaded v0.11.4 of the SDK.

There appears to be a stub header that matches the path inside the testing path: https://docs.zephyrproject.org/apidoc/latest/subsys_2testsuite_2ztest_2include_2syscalls_2kobject_8h_source.html, so I just ran mkdir include/syscalls && touch include/syscalls/kobject.h.

result

I was able to build and flash after adding the dummy file, but the light did not blink, I assume because of some internal fault.

regression check

I ran the following to rebase my fork off of v2.4.0

# reset to Zephyr v2.4.0
git checkout v2.4.0-branch
# apply the bossac runner changes manually ( git is hard )
wget -O scripts/west_commands/runners/bossac.py https://raw.githubusercontent.com/nandojve/zephyr/21003324e4b638e88690a20172243c7633987b10/scripts/west_commands/runners/bossac.py
# apply the new board definition from my fork
git cherry-pick a064b23

I was able to flash and the light blinks too. Have there been any other reports of the missing header after the refactor?

@nandojve
Copy link
Member

nandojve commented Jan 4, 2021

Hi @saltyJeff ,

I'm having issues building the project after merging my changes into the Zephyr master. I believe I've come upon a regression:

Your PR is passing buildkite but you are having regression issues. I noted that you not added an arduino_nano_33_ble.yaml file and this can explain why you are not seen regression here. Make sure you added all files as described at create-your-board-directory. If you look at buildkite #17002 you can confirm that CI is running 0 tests.

INFO    - Running only a subset: 1/40
--
  | INFO    - Zephyr version: zephyr-v2.4.0-2711-g5cba90ad8d
  | INFO    - JOBS: 2
  | INFO    - 1056 test suites (0 configurations) selected, 0 configurations discarded due to filters.
  | INFO    - Adding tasks to the queue...
  | INFO    - Added initial list of jobs to queue
  |  
  | INFO    - 0 of 0 test configurations passed (0.00%), 0 failed, 0 skipped with 0 warnings in 2.04 seconds
  | INFO    - In total 0 test cases were executed, 0 skipped on 0 out of total 318 platforms (0.00%)
  | INFO    - 0 test configurations executed on platforms, 0 test configurations were only built.
  | INFO    - Run completed

I was able to flash and the light blinks too.

Nice! This means that west bossac runner is ok for you user case.

Now, you need add that missing file and have you PR in sync locally. Run scripts/twister -p <you board> before push the PR, just to make sure twister is running > 0 tests and 0 fail.

@WilliamGFish
Copy link
Collaborator

@WilliamGFish I've reverified my I2C capability, it's working on my side.

The program I ran (no proj.conf or anything besides the default CMakeLists.txt was used): https://gist.github.com/saltyJeff/c1a0a6f1a6f53fb007e31c4b5951928f

@saltyJeff , I've no doubt you're right about i2c working.
As we are back in lockdown again,I'll have a closer look. Might be a simple issues with driver implementation.

Billy..

Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

Your PR is passing buildkite but you are having regression issues. I noted that you not added an arduino_nano_33_ble.yaml file and this can explain why you are not seen regression here. Make sure you added all files as described at create-your-board-directory. If you look at buildkite #17002 you can confirm that CI is running 0 tests.

INFO    - Running only a subset: 1/40
--
  | INFO    - Zephyr version: zephyr-v2.4.0-2711-g5cba90ad8d
  | INFO    - JOBS: 2
  | INFO    - 1056 test suites (0 configurations) selected, 0 configurations discarded due to filters.
  | INFO    - Adding tasks to the queue...
  | INFO    - Added initial list of jobs to queue
  |  
  | INFO    - 0 of 0 test configurations passed (0.00%), 0 failed, 0 skipped with 0 warnings in 2.04 seconds
  | INFO    - In total 0 test cases were executed, 0 skipped on 0 out of total 318 platforms (0.00%)
  | INFO    - 0 test configurations executed on platforms, 0 test configurations were only built.
  | INFO    - Run completed

Good catch, @nandojve . We really need the yaml file added to make sure CI builds for this board.

This commit adds the board definition files
needed to support the Arduino Nano BLE 33.
Tested: the following have been verified with
my logic analyzer.
* Serial peripherals (UART, I2C, SPI)
* USB
* RTC

Untested:
* PWM. In theory it should work but I don't
have a good enough logic analyzer to test this

* RTC's. The board doesn't have a backup battery.
The peripherals are enabled for modding another
battery in the device tree.

Signed-off-by: Jefferson Lee <jeffersonlee2000@gmail.com>
@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Jan 7, 2021
@saltyJeff
Copy link
Contributor Author

YAML file is in

Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BOSSAC LGTM!

@MaureenHelm MaureenHelm merged commit 35c5fbe into zephyrproject-rtos:master Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards area: Devicetree area: Documentation area: Tests Issues related to a particular existing or missing test platform: nRF Nordic nRFx Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants