Skip to content
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

Simplify msp430 headers #408

Merged
merged 6 commits into from
Dec 19, 2013
Merged

Simplify msp430 headers #408

merged 6 commits into from
Dec 19, 2013

Conversation

rousselk
Copy link
Contributor

@rousselk rousselk commented Dec 9, 2013

Modification of the way standard MSP430 header files are included in RIOT.
This new scheme should reduce the use of #ifdefs in RIOT source code, and allow for easy integration of any other MSP430-based platform.

@LudwigKnuepfer
Copy link
Member

Breaks build with BOARD=chronos:

make[3]: Entering directory '/home/lo/native/RIOT/cpu/msp430-common'
flashrom.c: In function 'prepare':
flashrom.c:59:5: error: 'IFG1' undeclared (first use in this function)
flashrom.c:59:5: note: each undeclared identifier is reported only once for each function it appears in
flashrom.c:63:5: error: 'FCTL2' undeclared (first use in this function)
flashrom.c:63:21: error: 'FSSEL_3' undeclared (first use in this function)
flashrom.c:63:31: error: 'FN2' undeclared (first use in this function)
flashrom.c:63:37: error: 'FN0' undeclared (first use in this function)
flashrom.c:70:11: error: 'IE1' undeclared (first use in this function)
flashrom.c:71:11: error: 'IE2' undeclared (first use in this function)
flashrom.c: In function 'finish':
flashrom.c:80:5: error: 'IE1' undeclared (first use in this function)
flashrom.c:81:5: error: 'IE2' undeclared (first use in this function)
/home/lo/native/projects/hello-world/../../RIOT/Makefile.base:37: recipe for target '/home/lo/native/projects/hello-world/../../RIOT/bin/flashrom.o' failed
make[3]: *** [/home/lo/native/projects/hello-world/../../RIOT/bin/flashrom.o] Error 1
make[3]: Leaving directory '/home/lo/native/RIOT/cpu/msp430-common'
Makefile:27: recipe for target 'msp430-common' failed
make[2]: *** [msp430-common] Error 2

@LudwigKnuepfer
Copy link
Member

Please rebase on current master, #310 has been merged =)

@rousselk
Copy link
Contributor Author

rebasing done; I'll look into the flashrom issue ASAP.

@LudwigKnuepfer
Copy link
Member

BTW: this is with msp430-gcc (mspgcc_20120406) 4.6.3 20120301 (mspgcc LTS 20120406 unpatched)

@rousselk
Copy link
Contributor Author

After consulting the "TI MSP x5xxx and x6xxx family user guide" (a.k.a. SLAU208M), it seems that the Flash module on these MCU works completely differently than on the x1xxx and x2xxx family. (Among others: there is no FCTL2 register on CC430xxxx devices, and the addresses are totally different from what we have in F1611/F1612/F2617).

I now get the impression that the current 'flashrom.c' file should be moved from the 'msp430-common' folder to the 'msp430x16x' folder, and that a new 'flashrom.c' file should be created in the 'cc430' folder for these peculiar families...

@OlegHahm
Copy link
Member

That's maybe the reason why flashrom.c exists as well in msp430-common as in msp430x16x. Probably someone (might be me) wanted to move this file from common to 16x but didn't removed the copy in common...

Hence, I agree in removing this file from msp430-common. It is rarely used anyway as far as I can tell.

@rousselk
Copy link
Contributor Author

Plus, I think it just wouldn't actually work on CC430: as I said, the SLAU208M manual says there is no FCTL2 register in these chips; however, the current code writes in this register during setup...

@LudwigKnuepfer
Copy link
Member

Would you do that along with this PR?

@rousselk
Copy link
Contributor Author

Here you go.

Also added a "dummy" flashrom.c file in the 'cc430' directory: people using this kind of MCUs may implement it if they need/want to update Flash memory at runtime.

@LudwigKnuepfer
Copy link
Member

ACK

@thomaseichinger
Copy link
Member

please rebase, #251 was merged

@rousselk
Copy link
Contributor Author

done.

@LudwigKnuepfer
Copy link
Member

I don't know if this happened before the rebase (or if telosb built before the rebase at all), but at least now the build for telosb fails here while it does not in master:

In file included from /home/lo/native/projects/default/../../RIOT/core/include/kernel.h:26:0,
                 from /home/lo/native/projects/default/../../RIOT/core/include/thread.h:26,
                 from main.c:13:
/home/lo/native/projects/default/../../RIOT/cpu/msp430-common/include/cpu.h:30:19: fatal error: board.h: No such file or directory
compilation terminated.

@rousselk
Copy link
Contributor Author

Indeed: the problem is that the Makefiles of both the project ('default') and the 'RIOT/cpu/msp430x16x' folder don't include the board's header files...

Since RIOT is always built for a given platform (a.k.a. "board"), it seems to me that Makefiles (at least all the ones in cpu-specific directories) should always have a line like:
INCLUDE += -I${RIOTBOARD}/${BOARD}/include/

Do you agree, people?

@LudwigKnuepfer
Copy link
Member

The problem is a missing section in boards/Makefile.base. I don't see why one should have to specify this by hand.

@rousselk
Copy link
Contributor Author

Whoops, I missed this file totally. My bad.

Shall I add the needed section in that file, or should it be made in another PR (since it is not strictly directly related to the purpose of the current one)?

@LudwigKnuepfer
Copy link
Member

I'd just add it here as a single commit..

@LudwigKnuepfer
Copy link
Member

.. and I guess it's the make structure which is to blame, not you ;-)

@rousselk
Copy link
Contributor Author

Update done; maybe does it explain (and solve) some problems Oleg experienced with the telosb port?...

@LudwigKnuepfer
Copy link
Member

ACK

#include <stddef.h>
#include <stdint.h>
#include <cpu.h>
#include <irq.h>
Copy link
Member

Choose a reason for hiding this comment

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

Use quotes for include directives of internal headers (#437).

@OlegHahm
Copy link
Member

Apart from the small, pureley cosmetic changes: ACK.

@rousselk
Copy link
Contributor Author

Changes done (indeed, it's a recommended practice to quote custom headers and to bracket the standard ones).

@LudwigKnuepfer
Copy link
Member

ACK ACK

@ghost ghost assigned OlegHahm Dec 19, 2013
@OlegHahm
Copy link
Member

Hooray!

OlegHahm added a commit that referenced this pull request Dec 19, 2013
@OlegHahm OlegHahm merged commit 37a7393 into RIOT-OS:master Dec 19, 2013
@rousselk rousselk deleted the simplify-msp430-headers branch February 12, 2014 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants