Skip to content

Conversation

hudson-ayers
Copy link
Contributor

Pull Request Overview

This pull request ports the MSP432 chip/board to the new peripheral instantiation approach that does not rely on global variables (first proposed in #2069).

This chip was relatively easy to change compared to some of the others, but @lebakassemmerl I would still appreciate a quick look over the changes or a test of a couple apps to ensure I haven't done anything stupid.

This is the last remaining chip before all upstream chips/boards have been ported!

Testing Strategy

This pull request was tested by compiling, hardware testing would be nice.

TODO or Help Wanted

N/A

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

@lebakassemmerl
Copy link
Contributor

Sure, no problem!
I'm currently not at home but I will run the tests on the hardware in the evening today or tomorrow morning (UTC +1).

@lebakassemmerl
Copy link
Contributor

I ran now a couple of test programs (blink, button, helloworld, ipc, etc.) , and everything seems to work fine. However, I stumbled across a different problem which might be a bug in the linking-stage or within the kernel-linker-script.

When I run make flash everything works fine, and the binary fits into the reserved flash-space.
Running make flash-debug creates a binary which is slightly too big for the reserved flash. And then I realized that that the .bss section is copied into the actual hex-file. I get the following output:

   text    data     bss     dec     hex 
  81921       0   16384   98305   18001

Initially I thought maybe .bss and .data are interchanged, but when I make the following changes to main.rs:

pub static mut TEST: usize = 334;

unsafe fn startup_intilialisation() {
    msp432::init();

    TEST += 1;
    ...
}

I get the following output:

   text    data     bss     dec     hex 
 114689       4   16128  130821   1ff05

So obviously .bss and .data are not interchanged.
And as far as I know the .bss contains only uninitialized data which gets zeroed after startup. Should this really be in the binary?
And if this is a bug, should I open an issue?

@hudson-ayers
Copy link
Contributor Author

If by "end up in the binary" you mean "ends up in flash" I don't think that is happening. The GNU-size utility used to print the size information says the following:

The Berkeley style output counts read only data in the text column, not in the data column, the dec and hex columns both display the sum of the text, data, and bss columns in decimal and hexadecimal respectively.

So I think we are seeing exactly what we should expect.

bradjc
bradjc previously approved these changes Nov 13, 2020
@lebakassemmerl
Copy link
Contributor

If by "end up in the binary" you mean "ends up in flash" I don't think that is happening. The GNU-size utility used to print the size information says the following:

The Berkeley style output counts read only data in the text column, not in the data column, the dec and hex columns both display the sum of the text, data, and bss columns in decimal and hexadecimal respectively.

So I think we are seeing exactly what we should expect.

Ok, thanks for the answer!

I am still a bit confused about this because when flashing the binary with openocd it always prints the amount of flashed bytes. And this number equals exactly the decimal or hexadecimal column. But it's not a big problem, I'll try to find the problem when I have time (:

@ppannuto
Copy link
Member

bors d+

@bors
Copy link
Contributor

bors bot commented Nov 14, 2020

✌️ hudson-ayers can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@hudson-ayers
Copy link
Contributor Author

bors r+

@hudson-ayers
Copy link
Contributor Author

rebased to resolve conflicts

@lebakassemmerl
Copy link
Contributor

Will this PR be merged in the near future? Because I have some fixes pending for this platform but I thought it doesn't make sense to open a PR before this one isn't merged.

Copy link
Member

@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 1, 2020

Timed out.

@hudson-ayers
Copy link
Contributor Author

bors retry

@bors
Copy link
Contributor

bors bot commented Dec 1, 2020

Timed out.

@hudson-ayers
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 2, 2020

@bors bors bot merged commit 9ae41f8 into tock:master Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants