Skip to content

Initial nrf52811 support #14008

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

Merged
merged 1 commit into from
May 10, 2020
Merged

Initial nrf52811 support #14008

merged 1 commit into from
May 10, 2020

Conversation

Citrullin
Copy link
Contributor

@Citrullin Citrullin commented May 3, 2020

Contribution description

Initial support for the nrf52811 MCU. Will, at some point, add the board configuration for the nrf52811 mdk as well.
Restructuring files etc. makes probably sense at this point.

Testing procedure

Only tested hello world on the nrf52811mdk.

Issues/PRs references

Intitial support the nrf52811 as mentioned in #13054.

@Citrullin Citrullin changed the title Add initial nrf52811 support Initial nrf52811 support May 3, 2020
@benpicco benpicco added Area: cpu Area: CPU/MCU ports Type: new feature The issue requests / The PR implemements a new feature for RIOT labels May 3, 2020
/**
* @brief Structure for UART configuration data
*/
typedef struct {
NRF_UARTE_Type *dev; /**< UART with EasyDMA device base register address */
NRF_UARTE_Type *dev; /**< UART with EasyDMA device base
* register address */
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for that line break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise the line is too long. It was a warning in Travis.

@benpicco
Copy link
Contributor

benpicco commented May 4, 2020

This looks good, feel free to squash.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 5, 2020
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Looks good to me - there should be no potential for regressions.
Looking forward to the PR for the nrf52811mdk.

@benpicco
Copy link
Contributor

benpicco commented May 7, 2020

Commit message should be somethinkg like cpu/nrf52: add support for nrf52811

@Citrullin
Copy link
Contributor Author

Commit message should be somethinkg like cpu/nrf52: add support for nrf52811

Done

@benpicco benpicco merged commit 63a4d12 into RIOT-OS:master May 10, 2020
@benpicco benpicco mentioned this pull request May 10, 2020
Comment on lines +107 to +108
isr_spi1_twi1, /* spi1_twi1 */
isr_spi0, /* spi 0 */
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm now I wonder - are you sure those are not just

    isr_spi0_twi0,         /* spi0_twi0 */
    isr_spi1_twi1,         /* spi1_twi1 */

?

Copy link
Contributor Author

@Citrullin Citrullin May 12, 2020

Choose a reason for hiding this comment

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

No, I am not sure. I just copied it from the datasheet. Don't know how it works in detail. But even though, it is wrong. I just looked into it again. It should be spi1_twi0. Just compiled some examples for the nrf52811dk so far. Want to try a SPI example in the near future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benpicco Would be great, if you can tell where these variables are coming from or what happens to them? Where do they get the addresses from?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the interrupt vector table.
When an interrupt occurs the CPU uses the ID of the interrupt source as an index to decide which function to call to handle the interrupt.

So interrupts are enabled for the SPI peripheral, those functions will get called.
They are currently unused, but #14057 will change this.

Copy link
Contributor Author

@Citrullin Citrullin May 12, 2020

Choose a reason for hiding this comment

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

@benpicco Yes, I understood this. Thanks for the information that they are not being used. This explains why I couldn't find it :D

Copy link
Contributor

Choose a reason for hiding this comment

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

On which page of the data sheet is it?
I couldn't find it yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, tricky

There is a direct relationship between peripheral ID and base address. For example, a peripheral with base
address 0x40000000 is assigned ID=0, a peripheral with base address 0x40001000 is assigned ID=1, and a
peripheral with base address 0x4001F000 is assigned ID=31.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, tricky

There is a direct relationship between peripheral ID and base address. For example, a peripheral with base
address 0x40000000 is assigned ID=0, a peripheral with base address 0x40001000 is assigned ID=1, and a
peripheral with base address 0x4001F000 is assigned ID=31.

image

Yep, exactly the part I messed up. It would make sense to have spi0 and twi0 together, but it isn't ^^
Yes, this is why I thought the order matters. I wasn't sure about the deprecated things so. I kept it like it currently is. But spi should be spim or spis.
This is probably also a bit connected to #10587 then.

@@ -34,6 +35,7 @@ void dummy_handler(void) {
WEAK_DEFAULT void isr_power_clock(void);
WEAK_DEFAULT void isr_radio(void);
WEAK_DEFAULT void isr_uart0(void);
WEAK_DEFAULT void isr_spi0(void);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this CPU really have a new interrupt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the other isr.

@benpicco
Copy link
Contributor

benpicco commented May 12, 2020

Is there no nrf52811.h vendor file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants