Add support for Nordic's nRF52810 IC #7915
Conversation
carlescufi
requested review from
anangl,
dbkinder,
galak and
MaureenHelm
as code owners
| @@ -155,6 +155,8 @@ struct arm_mpu { | |||
| #define REGION_1G (0x1D << 1) | |||
| #define REGION_2G (0x1E << 1) | |||
| #define REGION_4G (0x1F << 1) | |||
| #define REGION_24K (0x20 << 1) | |||
There was a problem hiding this comment.
This is not correct. These macros are used to encode the region size in the MPU register, when configuring MPU regions, and AFAIK, we only have powers of two, there.
Check table B3-43 in ARMv7-M technical reference manual.
|
CC @junqingzou @jarz-nordic @kl-cruz |
Codecov Report
@@ Coverage Diff @@
## master #7915 +/- ##
==========================================
- Coverage 63.8% 63.78% -0.02%
==========================================
Files 426 426
Lines 40890 40890
Branches 6921 6921
==========================================
- Hits 26088 26082 -6
- Misses 11666 11672 +6
Partials 3136 3136
Continue to review full report at Codecov.
|
| @@ -0,0 +1,20 @@ | |||
| # Kconfig.defconfig.nrf52810 - Nordic Semiconductor nRF52832 MCU | |||
| @@ -0,0 +1,9 @@ | |||
| # Kconfig - nRF52810 PCA10040 board configuration | |||
There was a problem hiding this comment.
pca10068 - also correct folder name
There was a problem hiding this comment.
No, see commit messages for explanation
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| if BOARD_NRF52810_PCA10040 |
There was a problem hiding this comment.
By the way @carlescufi do we really need these empty Kconfig files?
arch/arm/soc/nordic_nrf/nrf52/soc.c
Outdated
| @@ -381,7 +381,7 @@ static int nordicsemi_nrf52_init(struct device *arg) | |||
|
|
|||
| key = irq_lock(); | |||
|
|
|||
| #ifdef CONFIG_SOC_NRF52832 | |||
| #if defined(CONFIG_SOC_NRF52832) || defined(CONFIG_SOC_NRF52810) | |||
| nordicsemi_nrf52832_init(); | |||
There was a problem hiding this comment.
I think that both chips shall have different init functions. Most of PANs were fixed in nrf52810
| "nordic,nrf52810"; | ||
|
|
||
| chosen { | ||
| zephyr,console = &uart0; |
There was a problem hiding this comment.
nrf52810 does not have an UART0, only UARTE0
There was a problem hiding this comment.
here UART I think means "serial device"? not the type of hardware, that's defined in the "compatible" tag I believe?
There was a problem hiding this comment.
While @carlescufi comment is correct, if its the case that UARTE0 can only be the 'E' form, than the compat in the SOC dts should just be 'nordic,nrf-uarte' and we don't need to update the compat in the board.dts
There was a problem hiding this comment.
It's already the case that compat in the SoC dts is only nrf-uarte
dts/arm/nordic/nrf52810.dtsi
Outdated
| label = "I2C_0"; | ||
| }; | ||
|
|
||
| i2c1: i2c@40004000 { |
There was a problem hiding this comment.
nrf52810 does not have TWIM1.
It has only TWIM0 at address 40003000
| label = "image-1"; | ||
| reg = <0x00016000 0xa000>; | ||
| }; | ||
| scratch_partition: partition@20000 { |
There was a problem hiding this comment.
Why the scratch partition is of the image slot size? The scratch size of 0x3000 will be sufficient here, Then slots size can be increased to size 0xd000 each. So scratch_size/slot_size ~ 0.23, which is near to ratio of nrf52832_pac10040 (0.2).
| @@ -8,7 +8,7 @@ | |||
| config SOC_SERIES_NRF52X | |||
| bool "Nordic Semiconductor nRF52 series MCU" | |||
| select CPU_CORTEX_M4 | |||
| select CPU_HAS_FPU | |||
| select CPU_HAS_FPU if !SOC_NRF52810 | |||
There was a problem hiding this comment.
At this point, seems like move the 'select CPU_HAS_FPU' over to the SOC_NRF52... would be better.
boards/arm/nrf52810_pca10040/board.h
Outdated
| #include <soc.h> | ||
|
|
||
| /* Push button switch 0 */ | ||
| #define SW0_GPIO_PIN 13 |
There was a problem hiding this comment.
Can we do this via DTS instead since this is a new board.
There was a problem hiding this comment.
I would love to, but I think I would need some help here. Can you give me some pointers?
There was a problem hiding this comment.
Look at the following commit:
commit c627de1
Author: Maureen Helm maureen.helm@nxp.com
Date: Thu Apr 19 12:57:18 2018 -0500
boards: Move led and button definitions to dts for kinetis boards
There was a problem hiding this comment.
You'll also need to pull in some changes from PR #7444 (meaning adding the dt nodes for GPIO etc).
There was a problem hiding this comment.
Very well. In that case I will wait until #7444 is merged, since this should not be merged before 1.12 is released anyway. Thanks for the pointers.
There was a problem hiding this comment.
@galak this will be fixed as you propose. Only aliases will be kept in the board.h, in compliance with what other boards do.
| "nordic,nrf52810"; | ||
|
|
||
| chosen { | ||
| zephyr,console = &uart0; |
There was a problem hiding this comment.
While @carlescufi comment is correct, if its the case that UARTE0 can only be the 'E' form, than the compat in the SOC dts should just be 'nordic,nrf-uarte' and we don't need to update the compat in the board.dts
|
|
||
| &uart0 { | ||
| status = "ok"; | ||
| compatible = "nordic,nrf-uarte"; |
There was a problem hiding this comment.
See above comment if this compat should just be in the SoC dtsi
There was a problem hiding this comment.
I removed this line from the board dts file, since it's already in the dtsi
carlescufi
force-pushed
the
gluon
branch
4 times, most recently
from
7e648ac
to
cc2c3e9
Compare
carlescufi
requested review from
andrewboie,
andyross,
lpereira and
spoorthik
as code owners
|
|
||
| config SOC | ||
| string | ||
| default nRF52810_QFAA |
There was a problem hiding this comment.
Make this default "nRF52810_QFAA" to avoid generating a warning once this gets in.
| // <e> NRFX_SPI_ENABLED - nrfx_spi - SPI peripheral driver | ||
| //========================================================== | ||
| #ifdef CONFIG_NRFX_SPI | ||
| #define NRFX_SPI_ENABLED 1 |
There was a problem hiding this comment.
nRF52810 is not equipped with SPI. There are SPIM and SPIS
There was a problem hiding this comment.
I pushed a new file, please re-review
|
|
||
|
|
||
| #ifndef NRFX_TIMER3_ENABLED | ||
| #define NRFX_TIMER3_ENABLED 0 |
There was a problem hiding this comment.
There is no TIMER3 in nRF52810
There was a problem hiding this comment.
I pushed a new file, please re-review
|
|
||
|
|
||
| #ifndef NRFX_TIMER4_ENABLED | ||
| #define NRFX_TIMER4_ENABLED 0 |
There was a problem hiding this comment.
There is no TIMER4 in nRF52810
| // <e> NRFX_I2S_ENABLED - nrfx_i2s - I2S peripheral driver | ||
| //========================================================== | ||
| #ifndef NRFX_I2S_ENABLED | ||
| #define NRFX_I2S_ENABLED 0 |
| // <e> NRFX_LPCOMP_ENABLED - nrfx_lpcomp - LPCOMP peripheral driver | ||
| //========================================================== | ||
| #ifndef NRFX_LPCOMP_ENABLED | ||
| #define NRFX_LPCOMP_ENABLED 0 |
|
|
||
|
|
||
| #ifndef NRFX_PWM1_ENABLED | ||
| #define NRFX_PWM1_ENABLED 0 |
|
|
||
|
|
||
| #ifdef CONFIG_SPI_1_NRF_SPIM | ||
| #define NRFX_SPIM1_ENABLED 1 |
|
|
||
|
|
||
| #ifndef NRFX_TWIM1_ENABLED | ||
| #define NRFX_TWIM1_ENABLED 0 |
| // <e> NRFX_TWI_ENABLED - nrfx_twi - TWI peripheral driver | ||
| //========================================================== | ||
| #ifndef NRFX_TWI_ENABLED | ||
| #define NRFX_TWI_ENABLED 0 |
There was a problem hiding this comment.
no TWI in nRF52810.
This config file needs to be changed
| @@ -5,18 +5,29 @@ | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
| # | |||
|
|
|||
| config SOC_NRF52810 | |||
There was a problem hiding this comment.
There is one more difference between nRF52810 and other members of the nRF52 family: The nRF52810 does not have instruction cache, which should be enabled on other SoCs.
At the moment it looks that we are not enabling cache at all. IMHO we should fix that asap as without cache performance is significantly degraded. Please fix that in this PR or create new issue.
There was a problem hiding this comment.
Done, updated the patches to reflect this
| if BOARD_NRF52810_PCA10040 | ||
|
|
||
| config BOARD | ||
| default nrf52810_pca10040 |
There was a problem hiding this comment.
Still missing quotes here.
In order to avoid auto-selection by Kconfig, the board needs to define the specific IC that it contains, in this case an nRF52832. Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
|
@anangl addressed, thank you |
|
recheck |
1 similar comment
|
recheck |
nrfx requires specific chip configuration for each IC. Those are not upstream in the nrfx repository, but rather part of the Zephyr nrfx port. Add a configuration for the nRF52810. Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no> Signed-off-by: Ioannis Glaropoulos <ioannis.glaropoulos@nordicsemi.no>
Since Nordic does not offer an nRF52810 Development Kit and instead recommends using the nRF52 DK (nRF52832 based), add a board that enforces the limitations of the nRF52810 but is designed to be flashed onto the actual nRF52 DK hardware. Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no> Signed-off-by: Ioannis Glaropoulos <ioannisg.glaropoulos@nordicsemi.no>
When adding the nRF52810, which has 24KB of RAM, some of the tests don't compile anymore due to lack of SRAM. Address this by either filtering the test out or reducing the amount of memory allocation. Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
The nRF52810 is a low-cost variant of the nRF52832, with a reduced set of peripherals and memory. This commit adds basic support for it in the arch SoC and dts folders. Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no> Signed-off-by: Ioannis Glaropoulos <ioannis.glaropoulos@nordicsemi.no>
This commit adds a compile-time directive for the nrf52810_pca10040 board that instruct nRFx to perform certain board initialization. The directive is needed as nrf52810_pca10040 mirrors the nrf52832 DK hardware. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit moves the definitions for the LED and Buttons supported in nrf52810_pca10040 DK in DTS from board.h. Aliases are kept in board.h to make basic examples pass. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
The commit modifies the I2C-related defines in nrfx configuration to comply with the updates in (57e3644). It also modifies the i2c configuration in DTS for board nrf52810_pca10040 to reflect the nRF52810 capabilities. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit selects HAS_DTS_I2C in nRF8232 DK board definition. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
|
@dbkinder can you revisit this one? |
|
looks good to me |
|
thanks @dbkinder! |
Adds support for the IC itself and a new board which enforces the 52810 limitations but mirrors the nRF52 DK hardware, since there is no development kit specific to the 52810.
Tested with the RTT console since UART is not functional yet.
Note that BLE is not functional yet either. We require a new version of nrfx with the appropriate radio bindings and then @cvinayak will be able to add a layer for it in the bluetooth hal folder.