New Platform: Msp432 #1911

lebakassemmerl · 2020-06-05T16:20:37Z

New Platform Support for the MSP-EXP432P401R Eval-Board

This pull-request adds support for MSP432P401R chip from Texas Instruments.
The implementation is currently very, very basic, only the GPIO-module is properly implemented in order to create a panic-handler which blinks an LED and to be able to test a simple user-space application.

If this PR will be accepted, the next step would be to implement the UART and timer modules for supporting debug-output and delays in userspace.

Adding a Platform to Tock Repository

The hardware must be widely available. Generally that means the hardware platform can be purchased online.
The port of Tock to the platform must include at least:
- Console support so that debug!() and printf() work.
- Timer support.
- GPIO support with interrupt functionality.
The contributor must be willing to maintain the platform, at least initially, and help test the platform for future releases.

alistair23 · 2020-06-05T16:32:01Z

Thanks for the PR!!!

Would you mind rebasing on master? It looks like GitHub is confused and has included other patches in the PR. It makes it hard to review your changes.

Also the Tock porting guide gives some hints for porting: https://github.com/tock/tock/blob/master/doc/Porting.md#adding-a-platform-to-tock-repository
It also specifies that you need a working console (and timer but that might be less important) before the board is merged.

alistair23 · 2020-06-05T16:33:19Z

It also looks like the tests are failing. Make sure to run at least make ci-nosetup locally. For new boards the CI check can be helpful in finding bugs (it checks all of your register offsets).

lebakassemmerl · 2020-06-05T17:15:48Z

Hi!
Yes, of course. I will rebase it on master. Also I will make sure that the test won't fail.

Concerning the console and timer: I read this guide and somewhere at the beginning it said that after GPIO is working it's a good time to open a pull request. So should I close/delete this PR, fix my tests, get the console working and open a new one?

alistair23 · 2020-06-05T17:39:08Z

Great! Thanks.

I think opening a PR makes sense. It lets everyone know what you are working on (so we don't duplicate efforts). It is also a great way to get feedback.

It might make sense to convert it to a draft PR until you have the tests and UART working though.

bradjc · 2020-06-05T20:52:18Z

Very cool!

Can I ask what attracted you to this board/MCU?

lebakassemmerl · 2020-06-07T07:18:36Z

Yeah, sure!

I'm doing my master thesis in porting Tock to a new platform, and my only 'requirement' was to choose a platform which is of course not supported yet and where a cheap evaluation board exists. In our university (FH Joanneum) we work a lot with all forms of TI products, so I had a look at their microcontroller boards. I chose this particular one because openOCD had good support for it, it contains a quite new Cortex-M4-based chip and the board costs only 20$.

lebakassemmerl · 2020-06-19T15:33:22Z

Hi!

So far I managed it to get the UART in TX direction working in order to print panic- and debug-messages. Is it necessary for a first pull into the main-repo to get also the RX-functionality working?

Also the GPIO implementation doesn't support interrupts at the moment. Is this also necessary to get merged?

alistair23 · 2020-06-19T18:02:25Z

You don't need to have RX working, so that's fine. For GPIO I think that's also ok.

alistair23

Looks REALLY good! :)

I have added some comments, they should all be pretty small. Overall this looks really good. I didn't compare it to the datasheet but everything looks reasonable.

Do you mind addressesing the comments and then I'll have another look?

.vscode/launch.json

boards/msp_exp432p401r/Makefile

boards/msp_exp432p401r/README.md

boards/msp_exp432p401r/src/io.rs

chips/msp432/src/pcm.rs

chips/msp432/src/wdt.rs

chips/msp432/src/sysctl.rs

lebakassemmerl · 2020-06-25T07:44:45Z

Looks REALLY good! :)

I have added some comments, they should all be pretty small. Overall this looks really good. I didn't compare it to the datasheet but everything looks reasonable.

Do you mind addressesing the comments and then I'll have another look?

I think I have addressed all your comments with the latest commits.
So, when you find the time, please have another look (:

ppannuto

Overall, looks great!

boards/msp_exp432p401r/Makefile

ppannuto · 2020-06-30T11:59:38Z

boards/msp_exp432p401r/src/main.rs

+#![no_std]
+#![cfg_attr(not(doc), no_main)]
+#![feature(asm, core_intrinsics)]
+// #![deny(missing_docs)]


Suggested change

// #![deny(missing_docs)]

#![deny(missing_docs)]

This still needs to be addressed. Should just be the listed change here.

boards/msp_exp432p401r/src/main.rs

chips/msp432/src/cs.rs

chips/msp432/src/flctl.rs

chips/msp432/src/pcm.rs

chips/msp432/src/lib.rs

chips/msp432/src/uart.rs

alistair23 · 2020-07-08T19:22:39Z

When you address a comment can you mark it as resolved?

lebakassemmerl · 2020-07-09T08:24:01Z

When you address a comment can you mark it as resolved?

Sure! I didn't know that I was supposed to mark the comments as resolved.

ppannuto

Take a look at the SAM4L spi implementation for an example of PeripheralManager use. The idea is that the manager encapsulates the registers, so all register access must go through the manager. That way, hardware is provably always locked/unlocked at compile time.

e.g. when accessing registers, first create an instance of the manager: https://github.com/tock/tock/blob/master/chips/sam4l/src/spi.rs#L264
helper functions take a reference to the manager: https://github.com/tock/tock/blob/master/chips/sam4l/src/spi.rs#L241

chips/msp432/src/cs.rs

ppannuto

I think this is looking pretty much ready! Very exciting!

ppannuto · 2020-07-13T14:02:38Z

I think you can move this from a Draft PR to a full-fledged one at the point :)

chips/msp432/src/uart.rs

lebakassemmerl · 2020-07-16T07:23:50Z

I implemented no a very first version for handling GPIO interrupts. However, there is a problem with this controller, it does NOT support detecting interrupts at the falling edge and at the rising edge simultaneously. Just for testing the implementation with the button-capsule I detect only falling edges where actually both should be detected.

How should I solve this problem? Just 'ignore' the fact that both should be detected? Maybe panic!()? Or maybe any kind of software implementation (However I have no idea how this could be done within Tock)?

hudson-ayers · 2020-07-16T12:20:59Z

I implemented no a very first version for handling GPIO interrupts. However, there is a problem with this controller, it does NOT support detecting interrupts at the falling edge and at the rising edge simultaneously. Just for testing the implementation with the button-capsule I detect only falling edges where actually both should be detected.

How should I solve this problem? Just 'ignore' the fact that both should be detected? Maybe panic!()? Or maybe any kind of software implementation (However I have no idea how this could be done within Tock)?

Regarding a software implementation, I think it would basically involve re-configuring a pin to check for the other edge type each time an interrupt arrived for that pin. This would just be done in the chip GPIO driver -- basically when a falling edge interrupt arrives, check that the pin is still low, and if so configure that pin to look for a rising edge interrupt, and vice-versa. Unfortunately it would have to take place in the bottom half interrupt handler, which means there is the possibility for large enough delays that interrupts could be missed, but I think this is still a better solution than only handling interrupts in one direction.

lebakassemmerl · 2020-07-16T14:41:59Z

I implemented no a very first version for handling GPIO interrupts. However, there is a problem with this controller, it does NOT support detecting interrupts at the falling edge and at the rising edge simultaneously. Just for testing the implementation with the button-capsule I detect only falling edges where actually both should be detected.
How should I solve this problem? Just 'ignore' the fact that both should be detected? Maybe panic!()? Or maybe any kind of software implementation (However I have no idea how this could be done within Tock)?

Regarding a software implementation, I think it would basically involve re-configuring a pin to check for the other edge type each time an interrupt arrived for that pin. This would just be done in the chip GPIO driver -- basically when a falling edge interrupt arrives, check that the pin is still low, and if so configure that pin to look for a rising edge interrupt, and vice-versa. Unfortunately it would have to take place in the bottom half interrupt handler, which means there is the possibility for large enough delays that interrupts could be missed, but I think this is still a better solution than only handling interrupts in one direction.

Ok, thanks!
Then I will try to implement a first software based approach for detecting both edges simultaneously.

lebakassemmerl · 2020-07-17T10:59:55Z

I have now implemented a first approach for detecting both interrupts by software. Can somebody please have a look on it if the implementation makes sense?
I tested it within the debugger, the basic functionality seems to work, after a rising edge it detects a falling one and vice versa. However I am not sure if I didn't make any subtle mistakes in this implementation.

…(and vice versa)

…rom the review

…sing registers

…th edges

…he Msp432 struct

lebakassemmerl · 2020-07-23T09:34:35Z

With the latest commit I added some missing lifetime specifiers in the GPIO module. However, since the struct PinJ basically doesn't need any lifetime (because it doesn't support interrupts), the macro I created doesn't work any more. For this reason I added a PhantomData<&'a ()> member to PinJ which makes it behave as it would have a lifetime. Is this the intended way to go or is there a better one?

bradjc · 2020-07-24T20:51:40Z

bors r+

bors · 2020-07-24T21:00:18Z

Build succeeded:

lebakassemmerl changed the title ~~Msp432~~ New Platform: Msp432 Jun 5, 2020

lebakassemmerl force-pushed the msp432 branch from 062e61e to 82ed290 Compare June 5, 2020 18:00

lebakassemmerl marked this pull request as draft June 5, 2020 18:25

lebakassemmerl force-pushed the msp432 branch from c8ac478 to a04e2c0 Compare June 15, 2020 21:43

alistair23 suggested changes Jun 19, 2020

View reviewed changes

ppannuto requested changes Jun 30, 2020

View reviewed changes

lebakassemmerl force-pushed the msp432 branch from 22117df to 8706cd4 Compare July 6, 2020 13:01

lebakassemmerl requested a review from ppannuto July 10, 2020 09:52

ppannuto requested changes Jul 10, 2020

View reviewed changes

chips/msp432/src/cs.rs Outdated Show resolved Hide resolved

chips/msp432/src/cs.rs Outdated Show resolved Hide resolved

ppannuto previously approved these changes Jul 13, 2020

View reviewed changes

lebakassemmerl marked this pull request as ready for review July 13, 2020 17:00

alistair23 reviewed Jul 14, 2020

View reviewed changes

chips/msp432/src/uart.rs Outdated Show resolved Hide resolved

lebakassemmerl dismissed ppannuto’s stale review via bb7d888 July 16, 2020 07:13

lebakassemmerl force-pushed the msp432 branch from bb7d888 to 87bd845 Compare July 17, 2020 10:34

lebakassemmerl added 21 commits July 23, 2020 10:35

msp432: uart: fixed bug in baudrate-calculation

bda2f2e

msp432: converted register-structs to register_structs!() macro

ba85f04

msp432: apps are now flashed via openOCD without touching the kernel …

c2dc8fa

…(and vice versa)

msp432: removed in_band_lifetimes

2102a40

msp432: added/changed a lot of comments and applied recommendations f…

78b9998

…rom the review

msp432: vscode: changed debug-config from gdb to cortex-debug addon

12e34c8

msp432: applied another set of recommendations from the review

a45eb4d

msp432: updated APP_MEMORY to new model

8656f99

msp432: cs: implemented PeripheralManagement trait and documented mis…

bf54cce

…sing registers

msp432: cs: use PeripheralManager correctly

c081f79

msp432: gpio: first implementation of interrupt-functionality

4000f21

msp432: uart: removed currently unused rx-interrupt

29a6a31

msp432: gpio: implemented 1st software-based version for detecting bo…

f2afddf

…th edges

msp432: wdt: adopted to the new watchdog interface

05ce378

msp32: chip: adopted to the new chip interface

647c5ef

msp432: wdt: renamed function to be less ambiguous

f69b16a

msp432: cs: configured missing HSMCLK and ACLK

3f7eeaa

msp432: timer: implemented 1st verion of an alarm timer

88e949c

msp432: wdt: changed config to new SMCLK frequency

8016819

msp432: removed global watchdog instance and made a field for it in t…

aeafb15

…he Msp432 struct

msp432: gpio: added missing lifetime specifiers

578d70d

lebakassemmerl force-pushed the msp432 branch from 5122dec to 578d70d Compare July 23, 2020 09:21

alistair23 approved these changes Jul 24, 2020

View reviewed changes

bradjc approved these changes Jul 24, 2020

View reviewed changes

ppannuto approved these changes Jul 24, 2020

View reviewed changes

ppannuto added the last-call Final review period for a pull request. label Jul 24, 2020

bors bot merged commit 8bb0709 into tock:master Jul 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Platform: Msp432 #1911

New Platform: Msp432 #1911

lebakassemmerl commented Jun 5, 2020 •

edited

alistair23 commented Jun 5, 2020

alistair23 commented Jun 5, 2020

lebakassemmerl commented Jun 5, 2020

alistair23 commented Jun 5, 2020

bradjc commented Jun 5, 2020

lebakassemmerl commented Jun 7, 2020

lebakassemmerl commented Jun 19, 2020

alistair23 commented Jun 19, 2020

alistair23 left a comment

lebakassemmerl commented Jun 25, 2020

ppannuto left a comment

ppannuto Jun 30, 2020

alistair23 commented Jul 8, 2020

lebakassemmerl commented Jul 9, 2020

ppannuto left a comment

ppannuto left a comment

ppannuto commented Jul 13, 2020

lebakassemmerl commented Jul 16, 2020

hudson-ayers commented Jul 16, 2020

lebakassemmerl commented Jul 16, 2020

lebakassemmerl commented Jul 17, 2020

lebakassemmerl commented Jul 23, 2020 •

edited

bradjc commented Jul 24, 2020

bors bot commented Jul 24, 2020

New Platform: Msp432 #1911

New Platform: Msp432 #1911

Conversation

lebakassemmerl commented Jun 5, 2020 • edited

New Platform Support for the MSP-EXP432P401R Eval-Board

Adding a Platform to Tock Repository

alistair23 commented Jun 5, 2020

alistair23 commented Jun 5, 2020

lebakassemmerl commented Jun 5, 2020

alistair23 commented Jun 5, 2020

bradjc commented Jun 5, 2020

lebakassemmerl commented Jun 7, 2020

lebakassemmerl commented Jun 19, 2020

alistair23 commented Jun 19, 2020

alistair23 left a comment

Choose a reason for hiding this comment

lebakassemmerl commented Jun 25, 2020

ppannuto left a comment

Choose a reason for hiding this comment

ppannuto Jun 30, 2020

Choose a reason for hiding this comment

alistair23 commented Jul 8, 2020

lebakassemmerl commented Jul 9, 2020

ppannuto left a comment

Choose a reason for hiding this comment

ppannuto left a comment

Choose a reason for hiding this comment

ppannuto commented Jul 13, 2020

lebakassemmerl commented Jul 16, 2020

hudson-ayers commented Jul 16, 2020

lebakassemmerl commented Jul 16, 2020

lebakassemmerl commented Jul 17, 2020

lebakassemmerl commented Jul 23, 2020 • edited

bradjc commented Jul 24, 2020

bors bot commented Jul 24, 2020

lebakassemmerl commented Jun 5, 2020 •

edited

lebakassemmerl commented Jul 23, 2020 •

edited