Skip to content

cpu/mips: deprecate mips cpu#17304

Merged
fjmolinas merged 1 commit intoRIOT-OS:masterfrom
MrKevinWeiss:pr/depmips
Dec 9, 2021
Merged

cpu/mips: deprecate mips cpu#17304
fjmolinas merged 1 commit intoRIOT-OS:masterfrom
MrKevinWeiss:pr/depmips

Conversation

@MrKevinWeiss
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss commented Dec 1, 2021

Contribution description

Add deprecate note in the mips cpu.
Add warning in make when any mips based board is being used (6lowpan-clicker, pic32-wifire).

After some discussion on the forum and between some maintainers we have decided to drop mips cpu as nobody is willing to continue to maintain it.

Testing procedure

Read the docs and run:

BOARD=6lowpan-clicker make all -C examples/hello-world/ && BOARD=pic32-wifire make all -C examples/hello-world/

then check for the error messages.

Issues/PRs references

Related to #11788
[Forum post discussing dropping mips] (https://forum.riot-os.org/t/dropping-support-of-outdated-poorly-supported-boards-and-modules/3343/19)

@MrKevinWeiss MrKevinWeiss added the Process: deprecation Integration Process: The PR is deprecating a feature or API label Dec 1, 2021
@github-actions github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Platform: MIPS Platform: This PR/issue effects MIPS-based platforms labels Dec 1, 2021
kaspar030
kaspar030 reviewed Dec 1, 2021
View reviewed changes
makefiles/dependency_resolution.inc.mk Outdated
$(shell $(COLOR_ECHO) "$(COLOR_RED)Deprecated modules are in use:$(COLOR_RESET)"\
"$(DEPRECATED_MODULES_USED)" 1>&2)
endif
# Warn about used deprecated boards
Copy link
Contributor

@kaspar030 kaspar030 Dec 1, 2021

Choose a reason for hiding this comment

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

I'm not sure this isn't going to cause trouble. does it print when doing, e.g., "BOARD=6lowpan-clicker make info-modules"?

Copy link
Contributor Author

@MrKevinWeiss MrKevinWeiss Dec 2, 2021

Choose a reason for hiding this comment

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

True, I guarded it with ifneq ($(RIOT_CI_BUILD),1), which should help against automated checks. For the developer just looking at it I don't think the warnings are a problem.

Copy link
Contributor

@fjmolinas fjmolinas Dec 3, 2021

Choose a reason for hiding this comment

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

Yep, same thing is done for BOARDSDIR already this makes sense

@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 2, 2021
@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Dec 3, 2021

murdock passes... may I squash?

fjmolinas
fjmolinas reviewed Dec 3, 2021
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

~~Please suqash!~~talked too fast

fjmolinas
fjmolinas reviewed Dec 3, 2021
View reviewed changes
fjmolinas
fjmolinas reviewed Dec 3, 2021
View reviewed changes
makefiles/dependency_resolution.inc.mk Outdated
"$(DEPRECATED_MODULES_USED)" 1>&2)
endif
# Warn about used deprecated boards
include $(RIOTMAKE)/deprecated_boards.inc.mk
Copy link
Contributor

@fjmolinas fjmolinas Dec 3, 2021

Choose a reason for hiding this comment

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

Does this need to run here? Couldn't it be done in Makefile.include? Also can we make it only show up once with:

  # Do not warn when set from sub-make
  ifeq ($(MAKELEVEL),0)
    ...
  endif

Copy link
Contributor Author

@MrKevinWeiss MrKevinWeiss Dec 6, 2021

Choose a reason for hiding this comment

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

makes sense

@fjmolinas
Copy link
Contributor

fjmolinas commented Dec 6, 2021

I see the call twice when building in docker, but I think that's fine otherwise I get the warning with the shinny red color, and no warning with RIOT_CI_BUILD=1

@fjmolinas
Copy link
Contributor

fjmolinas commented Dec 6, 2021

LGTM Ok to squash on my side, @kaspar030?

@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Dec 6, 2021

I see the call twice when building in docker, but I think that's fine otherwise I get the warning with the shinny red color, and no warning with RIOT_CI_BUILD=1

Hmm I guess the MAKELEVEL doesn't get passed through the docker make... it would be nice to use this a bit more. I know that kconfig warning also output several times.

@fjmolinas
Copy link
Contributor

fjmolinas commented Dec 8, 2021

Hmm I guess the MAKELEVEL doesn't get passed through the docker make... it would be nice to use this a bit more. I know that kconfig warning also output several times.

Yes but improove later!

@fjmolinas
Copy link
Contributor

fjmolinas commented Dec 8, 2021

Please squash!

fjmolinas
fjmolinas approved these changes Dec 8, 2021
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Dec 8, 2021

Done, thanks for the review.

leandrolanzieri
leandrolanzieri reviewed Dec 8, 2021
View reviewed changes
leandrolanzieri
leandrolanzieri reviewed Dec 9, 2021
View reviewed changes
@github-actions github-actions bot added the Area: boards Area: Board ports label Dec 9, 2021
@MrKevinWeiss MrKevinWeiss force-pushed the pr/depmips branch 2 times, most recently from 9003d91 to 3b2f324 Compare December 9, 2021 09:10
@MrKevinWeiss
cpu/mips: deprecate mips cpu
cf74b26 
Add deprecate note in the mips cpu.
Add warning in make when any mips based board is being used (6lowpan-clicker, pic32-wifire).
@leandrolanzieri leandrolanzieri added this to the Release 2022.01 milestone Dec 9, 2021
@fjmolinas fjmolinas enabled auto-merge December 9, 2021 09:49
@fjmolinas
Copy link
Contributor

fjmolinas commented Dec 9, 2021

Enabled auto-merge, go!

@fjmolinas fjmolinas merged commit 622fca0 into RIOT-OS:master Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaspar030 kaspar030 kaspar030 left review comments

@fjmolinas fjmolinas fjmolinas approved these changes

@maribu maribu maribu approved these changes

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation 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 Process: deprecation Integration Process: The PR is deprecating a feature or API

Projects

None yet

Milestone

Release 2022.01

Development

Successfully merging this pull request may close these issues.

5 participants

@MrKevinWeiss @fjmolinas @maribu @kaspar030 @leandrolanzieri