mips: clean up pic32 makefiles

[neiljay]

Make pic32m* depend on pic32_common which depends on mips32r2_common.
Remove some duplication.

This addresses some issues in #8052.

[cladmi]

The changes basically ends up being only moving the include $(RIOTCPU)/mips32r2_common things but has a meaning regarding cpu dependency.

Travis and Murdock also complains with whitespaces errors, github shows them too.

Another thing I wanted to do last time, was to move the include $(RIOTCPU)/mips_... and $(RIOTMAKE)/arch/mips in more specific to more generic order to the bottom of the file, this way, the local CPU includes will be before the common ones.

It would then match https:/RIOT-OS/RIOT/blob/master/cpu/stm32f1/Makefile.include and https:/RIOT-OS/RIOT/blob/master/cpu/samd21/Makefile.include

Quickly look through the files, I think it would not break anything. But not sure.
If it does, the solution is to lazy define variables in the common makefiles that could be set in the specific file.

What do you think ?

[cladmi]

I think these OFLAGS should stay in the mips_pic32mx/z Makefile.include as they are specific to the ldscript used there.
This way all the --change-section-lma for both boards are also not merged here.

I do not really understand why they are needed and not automatically calculated from the elf file, as in other cpus. But I may be missing something.

[neiljay]

I do not really understand why they are needed and not automatically calculated from the elf file,
as in other cpus. But I may be missing something.

Its a weird quirk of the Microchip MPLAB programmer. Other programming tools or elf loaders don't require this.

[cladmi]

That is the part I do not understand technically why :) But its completely fine to have it has its needed.

As I said however, I would still keep it in the specific CPU as its specific to the ldscript.

[neiljay]

ok fine to do that, but there were complaints about duplication in the makefiles, it works this way you just get a few more warnings about missing section names (you will always get one warning unless compiling C++ code).

[cladmi]

I think making mips32r2_common and mips32r2_generic solved almost all of them, like what is now in mips32r2_common/Makefile.include was duplicated as not included.

I also thought that making mips_pic32_common include mips32r2_common would remove more duplication but in practice not that much.
I did not check enough and these two OFLAGS lines where indeed different for the board.

However, now the fact that pic32_common are indeed mips32r2_common too is explicit.

[PeterKietzmann]

Now that you clean up the Makefiles what is this line supposed to do? If this is a dependency it should go into a Makefile.dep. If it means that the hardware has an hwrng on board, it should be FEATURES_PROVIDED += periph_hwrng right?

[neiljay]

I have no Idea how RIOT's make system is supposed to actually work, Its not really documented very well, I mean what is the difference between 'USEMODULE' and 'FEATURES_PROVIDED' and does it matter which you use ?

In answer to your question (on another thread) yes PIC32 devices support HWRNG and a driver is implemented.

[jnohlgard]

The FEATURES_PROVIDED list the features that a particular board can offer, while the USEMODULE explicitly tells the build system to build that support. FEATURES_PROVIDED is set by the board and CPU makefiles, while the application and its dependencies decide whether it should be compiled in or not. This is done in Makefile.dep for implicit dependencies and additional application specific dependencies are listed in the main Makefile for the application.

[jnohlgard]

Additionally, FEATURES_REQUIRED is used by an application to state that it needs a particular set of hardware/driver features in order to run properly. See the Makefiles in tests/periph_xxx for an example of this.

[cladmi]

The features in Makefile.features for all cpu of the mips family have been added by 28a2028
And may not be complete.

I think in this case, it should just also be added to the mips_pic32_common/Makefile.features:

FEATURES_PROVIDED += periph_hwrng

Is that right @gebart ?

But this could be a different PR, "update features for mips cpus".

[neiljay]

@cladmi, are you happy now ? Can you sign off on the review.

[cladmi]

Sorry I was not there a few days and took me some time to review.

I would not remove hwrng_init right now as it would break hwrng support for mips_pic32mz.
As you say it will be done by periph_common/init.c generic periph_init function in a further PR so would rather replace it in the upcoming one.

[cladmi]

Sorry I did not see that you updated the commit. Just say "done" below my answer to trigger me next time :)