Issue #438: Fixes for Arduino Nano 33 BLE.

[chuckstanski]

This PR removes code that was intended to support the Arduino Giga R1 boards; however, its integration into the codebase broke the Arduino Nana 33 BLE devices and possibly other ARM M4 device. The problematic macro was the use of defined(TARGET_M4) which unintentionally assumed all ARM M4 chips are using a 240MHz clock. The Nano 33 BLE boards have a ARM M4 at their core but run at a much lower clock rate. In addition the Nano 33 BLE boards use a different mechanism to control the GPIO (see NRF52 code in Adafruit_NeoPixel.cpp starting at line 2169) which was not used due to the mentioned macro.

Arduino Giga R1 support is already in the codebase (see Adafruit_NeoPixel.cpp line 3136 and other lines) which the macro defined(TARGET_GIGA) || defined(TARGET_M4) bypassed due to its placement in the file as well.

With the proposed changes, the Arduino Nano 33 BLE board has been tested. The Arduino Giga R1 code has been verified to compile correctly and include the correct code (i.e. defined(ARDUINO_ARCH_MBED_GIGA)); however, has not been verified on target due to lack of a Giga R1 board.

[dhalbert]

TARGET_M4 would seem to me to cover a large number of boards, such as SAMD51. It was not previously present. I agree that seems possibly wrong here.

If you just change #if defined(TARGET_GIGA) || defined(TARGET_M4) to #if defined(TARGET_GIGA), does that make the Nano 33 BLE work? That would be a less invasive change.

There have been at least two Giga support PR's. I tested one of them a while ago.

There needs to be some clean-up in the long run.

[chuckstanski]

Yes, that will make the Nano 33 work but I'm not sure about the Giga R1. The Giga R1 is a dual core SoC. The primary core is an Arm M7 and the secondary core is an Arm M4. From the Arduino board / variants files (includes.txt), the TARGET_GIGA is defined for the M7 core but not the M4 core. This is probably why the original author used 2 macros (TARGET_GIGA) and (TARGET_M4) to catch both processors.

If just #if defined(TARGET_GIGA) is used, the code will catch the M7 core for a Giga R1 build. The M4 code will probably use the code starting at line 3136 (.cpp) due to the macro #if ..... || defined(ARDUINO_ARCH_MBED_GIGA). The code defined by ARDUINO_ARCH_MBED_GIGA looks well written and seems to support both the M7 and M4 cores for the Giga R1 so there is no need to have the code defined by #if defined(TARGET_GIGA) || defined(TARGET_M4).

I agree with you that the defined(TARGET_M4) would catch more boards but the code as written hard-codes the clock to 240MHz which may not be true for all ARM M4 processors. The code I'm proposing to remove probably should have been removed with this checkin: #428

[dhalbert]

I understand. I do have a Giga R1 and can test that it still works with this PR.

[chuckstanski]

Great. Thank you.

[chuckstanski]

dhalbert, I was able to get a hold of a Arduino GIGA R1 board. There were some issues with running the code on the M4 core. My latest code has been tested and works on the Arduino Nano BLE 33 as well as the Arduino GIGA R1 (both the M7 and M4 cores).

[ladyada]

thanks!