cpu/qn908x: Initial minimal support for NXP QN908x CPUs.

[iosabi]

Contribution description

The NXP QN908x CPU family is a Cortex-M4F CPU with integrated USB,
Bluetooth Low Energy and in some variants NFC. This patch implements the
first steps for having support for this CPU.

While the QN908x can be considered the successor of similar chips from
NXP like the KW41Z when looking at the feature set, the internal
architecture, boot image format and CPU peripherals don't match those
in the Kinetis line. Therefore, this patch creates a new directory for
just the QN908x chip under cpu/qn908x.

The minimal set of peripherals are implemented in this patch to allow
the device to boot and enable a GPIO: the gpio and wdt peripheral
modules only.

The wdt driver is required to boot and disable the wdt. On reset, the
wdt is disabled by the chip, however the QN908x bootloader stored in
the internal ROM enables the wdt and sets a timer to reboot after 10
seconds, therefore it is needed to disable the wdt in RIOT OS soon
after booting. This patch sets it up such that when no periph_wdt module
is used the Watchdog is disabled, but if the periph_wdt is used it must
be configured (initialized) within the first 10 seconds.

The header files under the vendor/ directories (both
cpu/qn908x/include/vendor/ and cpu/qn908x/vendor/) are part of NXP's
SDK for the QN908x family available for download from:
https://mcuxpresso.nxp.com/en/builder
The files included in this vendor/ directory are released by NXP under
an Open Source license as described in each file, but only the files
used by this patch are included here.

Testing procedure

Defined a custom board for this CPU and compiled a simple application
that blinks some LEDs. Manually tested with periph_wdt and with
periph_wdt_cb as well.

Issues/PRs references

This is part of the FR #13852 . This patch depends on PR #13851 (among other things) to create a valid image but they can be merged independently.

[benpicco]

The code looks good.
Is there an evaluation board with this CPU or can you add the board you are using?

It should be possible for CI to compile-test this code.

[iosabi]

There is an evaluation board from NXP, the QN9080DK board, however I didn't buy that board because its selection of peripherals didn't match want I needed. I'm testing with a custom board I made with a QN9080. This is not really an issue since there is a lot of support needed in the cpu module for things that are independent of the board (like the uart, i2c, wdt, timers, etc). I plan to send commits for a "common" board with some things you need regardless of your board layout that I think need to go in the board module and after that I could create a qn9080dk board module with the board definition of the developer board since the schematics of the evaluation board are public, but I wouldn't be able to test that part on hardware. However in the interest of sending small and easy to review commits I'm only including the initial support here. Should I send more commits together instead?

I'll push an update to this patch to fix an issue with the gpio irq (I forgot to enable it in the NVIC).

[benpicco]

Should I send more commits together instead?

I'm all for small PRs - but I'd like to have a simple board so the code can run through CI.
Even a dummy board would be fine (I see that QN9080 is also sold a module, so that would probably even be useful).

[benpicco]

@iosabi I'd still like to have this in - but we need a board (of any kind) in order to compile this, so it doesn't bit-rot away.

[iosabi]

@benpicco Hey, sorry for being unresponsive with this for so long. The current situation didn't leave me much time to work on this but I'll dedicate more time to this now.

I updated the patches to work on top of master and added a Kconfig. This pull request now has 3 commits:

• one for the cpu/qn908x
• one for the boards/common/qn908x (there's a fair amount of things to deal with at the board level regardless of the actual board you use)
• a new one for boards/qn9080dk for the base board of the development kit.

This being the minimum possible to make things compile/work together it only implements GPIO and WDT. I have more commits in the pipeline to add UART, I2C and SPI, but I will add those as follow up commits since this is already large.

Would this be enough for the CI to run? I tested it with:

make BOARD=qn9080dk -C examples/hello-world/

(and the examples/saul too)

Regarding the choice of board, there's a "QN9080-001-M17Z" "module" (it looks like epoxied together so somewhere between a board and stacked chip) but that one is for the QN9080SIP which includes NFC. I don't have one of those and I'm not sure if it makes more sense to handle that little board as a cpu_model or as a board. Anyway, the qn9080dk fits better the description of a board and would be a better example if you want to make your own board based on this or stack a module on top of the DK GPIOs.

Finally, I'm getting this error in the make static-test but I don't know how to solve it, other cortex MCUs have the same code:

Running "./dist/tools/cppcheck/check.sh" x
Command output:

	cpu/qn908x/vectors.c:30: error (unknownMacro): There is an unknown macro here somewhere. Configuration is required. If ISR_VECTOR is a macro then please configure it.

[benpicco]

Finally, I'm getting this error in the make static-test but I don't know how to solve it, other cortex MCUs have the same code:

Running "./dist/tools/cppcheck/check.sh" x
Command output:

	cpu/qn908x/vectors.c:30: error (unknownMacro): There is an unknown macro here somewhere. Configuration is required. If ISR_VECTOR is a macro then please configure it.

Not sure how you got those, CI seems to be happy with the static tests.
Only tests/kconfig_features fails which means there is a discrepancy between the options selected by Kconfig and the old build system.

[iosabi]

I'm looking at the tests/kconfig_features failure, I'll update this with a fix.

[benpicco]

looks pretty good.
Please make a separate commit for adding the vendor files, that makes the review easier.

[benpicco]

gpio_init_mux() would be more conventional name for this function :)
That can also live in gpio.c.

[iosabi]

The issue with this is that is not gpio related only. The iocon is used to select the function of the pin, which "GPIO" is one of those functions. I'll use this from other drivers to set the I/O pin to the right function.

[benpicco]

Yes that's the case for the other implementations as well.
Don't worry about gpio.c being compiled even though most functions are unused - we enable -ffunction-sections and --gc-sections so unused functions are thrown out by the linker.

[iosabi]

I see that other CPUs have "gpio_init_mux()" declared in the periph_cpu.h. I renamed this to "gpio_init_mux" and renamed this header to gpio_mux.h, but the implementation is not in gpio.c. I don't think it is a good idea to have the prototype in one place and the definition in another file, nor I think we should expose this to anybody importing periph_cpu.h.

Conceptually this is a different block in the MCU, it doesn't need the GPIO clock to be enabled (yeah, the GPIO block has a clock enable bit, don't ask me why but it is needed for the gpio to work, I guess you can save \epsilon power there if you never run gpio_init()), this is about multiplexing physical "pins", all of which support in particular GPIO.

[iosabi]

I updated the PR, split the vendor files to a new commit (first in the stack) and added new Kconfig with the clock configuration. Please take a look again, thanks!

[leandrolanzieri]

@iosabi thanks for adding the clock configurations with Kconfig, I left some comments on that.

[benpicco]

This looks pretty good! Btw, if you have a debugger that supports it, you can also try stdio_semihosting to get some output from the board without UART.

[benpicco]

I hate modifying vendor files just as much as the next guy, but those need a

#ifdef __cplusplus
extern "C" {
#endif

…

#ifdef __cplusplus
}
#endif

header guard.
CI is complaining - usually vendor files are excluded from CI checks, but I see the point since they might transitively end up getting included in a global header file.

It's only 5 files, so I guess this is the easiest way.

[iosabi]

Well, the thing is that the files that need this have it around the functions already. If you only have #define or typedef you don't need this extern "C" header guard, which is the case for those files. I can add them but it would be just to calm down externc/check.sh. I can deal with providing some tooling to update the vendor files that patch this in when needed. Vendor SDK updates are not that common anyway.

[iosabi]

Vendor files also use "#if defined(__cplusplus)" but the check expects "#ifdef __cplusplus"

[benpicco]

Vendor files also use "#if defined(__cplusplus)" but the check expects "#ifdef __cplusplus"

urgh, yea dist/tools/externc/check.sh doesn't check for this - but this is a bug in the script.
If you could fix that, that would be great!
Not sure if there is a good way to determine if a header file only contains defines.

[iosabi]

This looks pretty good! Btw, if you have a debugger that supports it, you can also try stdio_semihosting to get some output from the board without UART.

I have already a UART driver for this using the FLEXCOMM modules which works fine, I'll send that patch once this is merged. The stdio_null was the easiest way for me to break the dependency on stdio and make examples compile. I've tested a larger stack but I think it is better to split things up for review.

Let me know if there's anything else to do here.

[benpicco]

CI is how happy too - looking forward to the follow ups :)

[benpicco]

If you have UART you can also run auto_test from tests/periph_gpio to see if interrupts are working as expected.

You'll need a timer, until you have a periph timer implemented #14553 would also do.

[iosabi]

If you have UART you can also run auto_test from tests/periph_gpio to see if interrupts are working as expected.

You'll need a timer, until you have a periph timer implemented #14553 would also do.

I have a timer in the pipeline... but timers need a bit more work because this cpu exposes a SCTimer/PWM combined block which you can in theory use for both timers and PWM, as well as a CTIMER block (four of them) which you can use for timer. I'll have xtimer working soon, but first I'll send you the RTC module which is simpler.

Regarding the GPIO interrupts I tested them manually just poking them :-)