Skip to content

cpu/mips: make mips32r2_common only a common cpu and remove cpu/periph module duplicate names.#8032

Merged
neiljay merged 9 commits intoRIOT-OS:masterfrom
cladmi:pr/cpu/mips_cleanup_modules
Nov 28, 2017
Merged

cpu/mips: make mips32r2_common only a common cpu and remove cpu/periph module duplicate names.#8032
neiljay merged 9 commits intoRIOT-OS:masterfrom
cladmi:pr/cpu/mips_cleanup_modules

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Nov 14, 2017

This fixes #8019

mips32r2_common is both a standalone cpu and a common cpu used by mips_pic32mx/mz.
This makes that mips_pic32mx and mips_pic32mz compile both the cpu and periph module from mips_pic32mx/z and mips32r2_common

To make mips32r2_common a real common cpu with different cpu/periph module names, I added mips32r2_generic to separate the standalone cpu part from the common part and namespaced the common one.

Now mips_pic32mx and mips_pic32mz correctly depends on mip32r2_common namespaced cpu and periph module.

Steps:

  • Add a mips32r2_generic cpu using mips32r2_common
  • Cleanup mips32r2_common
  • Namespace mips32r2_common and mips_pic32_common to remove conflicting module names.

Needs additional review for fixes of things that were here before.
I could do different PRs if needed.

  • Add periph_timer as a dependency of mips32r2_common. This adds it to mips_pic32mx/mz.
    I am not sure, but if mips32r2_common/periph needs it, it may have been required before too.
  • Include mips32r2_common/Makefile.features in mips_pic32mx/mz, as they are using the sames periph it makes sense to me.

Going further:

  • Update the CPUs documentation, I patched a bit mips32r2_generic headers but it may not be enough or wrong.
  • Add some documentation on what are these different CPUs directories and why not only one.
  • Is mips32r2_common a dependency of mips_pic32_common? Because right now mips_pic32mx/mz depend on two common cpus.

@cladmi cladmi added Area: build system Area: Build system Platform: MIPS Platform: This PR/issue effects MIPS-based platforms labels Nov 14, 2017
@cladmi cladmi requested a review from kaspar030 November 14, 2017 13:01
@cladmi cladmi force-pushed the pr/cpu/mips_cleanup_modules branch from a3653de to 3c4d949 Compare November 14, 2017 13:05
@cladmi cladmi mentioned this pull request Nov 16, 2017
Closed
@neiljay neiljay mentioned this pull request Nov 16, 2017
Closed
4 tasks
@cladmi cladmi added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 21, 2017
neiljay
neiljay approved these changes Nov 24, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@neiljay neiljay left a comment

Choose a reason for hiding this comment

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

All looks good to me, thanks for sorting this out. I've tested on all 3 board types.

@neiljay neiljay mentioned this pull request Nov 24, 2017
Closed
@neiljay
Copy link
Copy Markdown
Contributor

neiljay commented Nov 24, 2017

@cladmi I'm happy to merge this now if you like ?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 24, 2017

I think @cladmi should squash this PR first down a little bit :-).

@miri64 miri64 added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 24, 2017
@neiljay
Copy link
Copy Markdown
Contributor

neiljay commented Nov 24, 2017

Ah yes, LOL I forgot about the 'REVIEW ME' commit messages normally murdock fails when there is 'SQUASH' but clearly not on 'REVIEW'

@cladmi
cpu/mips32r2_generic: add new cpu that depends on mips32r2_common
062fbd8 
Prepare to make `mips32r2_common` only a common cpu and not a standalone one.
@cladmi
cpu/mips32r2_common: Use common 'Makefile.include' for cpus
d864e79 
Use mips32r2_common 'Makefile.include' to simplify depending cpus:

'mips_pic32mx', 'mips_pic32mz' and 'mips32r2_generic'.
@cladmi
cpu/mips32r2_common: mips32r2_common/include INCLUDES path.
1a8bb14 
Add include path for 'eic_irq.h' file and fix c files relative include.

Also remove wrong include to it in 'mips_pic32_common'.
@cladmi
cpu/mips32r2_common: only build newlib_syscalls_mips_uhi when needed
c835ddf
@cladmi
cpu/mips32r2_common: prefix modules with mips32r2_common namespace
383a69d 
This prevents clash between modules names cpu and periph with main cpus.

Add dependency management in Makefile.include to keep things contained.
@cladmi
cpu/mips_pic32_common: prefix modules with mips_pic32_common namespace
71b6fce 
This prevents clash between modules names cpu and periph with main cpus.

Add dependency management in Makefile.include to keep things contained.
@cladmi
cpu/mips_pic32mx/mz: remove unneeded USEMODULE in Makefile
6894524 
USEMODULE in Makefile is not used by the build system and so does nothing.
@cladmi
cpu/mips_pic32mx/mz: add missing mips32r2_common/Makefile.features
2d8c968
@cladmi cladmi force-pushed the pr/cpu/mips_cleanup_modules branch from 3c4d949 to f1d2895 Compare November 24, 2017 15:49
@cladmi cladmi removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Nov 24, 2017
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 24, 2017

Removed the "REVIEW ME" part of commits messages, how can we retrigger build ?

@neiljay neiljay added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 24, 2017
@neiljay
Copy link
Copy Markdown
Contributor

neiljay commented Nov 24, 2017

@cladmi You have to remove then re-add the 'Ready for CI build' label, but you need to be a maintainer to do that.

@neiljay
Copy link
Copy Markdown
Contributor

neiljay commented Nov 24, 2017

@cladmi, maybe squash the final 'BUG' commit.

@cladmi cladmi force-pushed the pr/cpu/mips_cleanup_modules branch from f1d2895 to 2375fcd Compare November 24, 2017 16:17
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 24, 2017

Just amended the last commit and pushed --force --force-with-lease.

@neiljay
Copy link
Copy Markdown
Contributor

neiljay commented Nov 24, 2017

@ cladmi, its still not squashed though.
use git rebase -i and squash/fixup #2375fcd into #062fbd8 or whichever is most appropriate

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 24, 2017

The final "BUG" commit is a fix to a problem that was here before. The pic32 boards did not have this dependency which I think was required.
But maybe the message is not clear enough.

@neiljay neiljay merged commit 5ccfe68 into RIOT-OS:master Nov 28, 2017
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 28, 2017

Thank you for reviewing and merging.

@cladmi cladmi deleted the pr/cpu/mips_cleanup_modules branch November 28, 2017 14:19
@cladmi cladmi mentioned this pull request Nov 30, 2017
Closed
6 tasks
@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaspar030 kaspar030 Awaiting requested review from kaspar030

1 more reviewer

@neiljay neiljay neiljay approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@neiljay neiljay

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: MIPS Platform: This PR/issue effects MIPS-based platforms

Projects

None yet

Milestone

Release 2018.01

Development

Successfully merging this pull request may close these issues.

Module 'cpu' name conflict for mips_pic32mx and mips_pic32mz

4 participants

@cladmi @neiljay @miri64 @aabadie