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

Refactor Makefiles across the repository #103

Merged
merged 32 commits into from
May 17, 2024
Merged

Refactor Makefiles across the repository #103

merged 32 commits into from
May 17, 2024

Conversation

wom-bat
Copy link
Contributor

@wom-bat wom-bat commented May 5, 2024

The aim is to have local knowledge of how to build a component confined to that component's Makefiles, rather than having to work it out in a per-project Makefile.

Each component and driver has a componentname.mk snippet designed to be included into a per-project makefile invoked in the build directory.

Thus, the overall Makefile for a project will create the build directory (if it doesn't already exist), add a Makefile into that directory, and invoke make in the build directory. The examples show what I mean.

The Makefile in the build directory just does
include componentname.mk
for each component it uses, and has a single line invoking the microkit tool that depends on all the elf files that it needs.

Still to do is working out how to have more than one variant (e.g., for the i2c host).

@Ivan-Velickovic
Copy link
Collaborator

For consistency we should use MICROKIT_SDK everywhere instead of MK_SDK. We never refer to Microkit as mk or MK so it will be confusing for people.

@JE-Archer
Copy link
Contributor

Nitpicks:

  • between the driver make fragments, the timer drivers define TIMER_DIR and TIMER_OBJ while the ethernet and I2C drivers don't define the *_OBJ variable;
  • the timer and I2C drivers define TIMER_DIR and I2C_DRIVER_DIR, while the ethernet drivers define ETHERNET_DRIVER without the _DIR;
  • the timer driver is referred to in the variables as just timer, while the ethernet and I2C are referred to as *_driver;
  • the ELF files are called timer.elf, eth.elf, uart_driver.elf and i2c_driver.elf—I suggest timer_driver.elf, ethernet_driver.elf`;
  • the .mk files themselves are called timer.mk, uart.mk, ethdriver.mk and i2c_driver.mk—I suggest timer_driver.mk, uart_driver.mk, ethernet_driver.mk and i2c_driver.mk;
  • the meson ethernet driver has a Makefile as well as a .mk;
  • some driver make fragments have notes about what is required in the system description while others don't;
  • the timer drivers get the directory via $(dir $(lastword $(MAKEFILE_LIST))), while the others just hardcode it;
  • driver as abbreviated sometimes as drv, other times as driv.

Questions:

  • given we are moving to make fragments to be included by third parties, might it be better to add a prefix to identifiers?
  • if all the meat of the building is in the fragments, could it be better to have a global Makefile (either in root or in examples/) instead of one per example system (one would still be able to build a specific system by specifying the target)?

@Ivan-Velickovic
Copy link
Collaborator

if all the meat of the building is in the fragments, could it be better to have a global Makefile (either in root or in examples/) instead of one per example system (one would still be able to build a specific system by specifying the target)?

The point of the examples is to be isolated so someone can copy and paste and use it as a start for experimentation/new system.

@wom-bat
Copy link
Contributor Author

wom-bat commented May 16, 2024

Nitpicks:

* between the driver make fragments, the timer drivers define `TIMER_DIR` and `TIMER_OBJ` while the ethernet and I2C drivers don't define the *_OBJ variable;

I wanted to give a variety of styles to show different ways the makefile fragment could work. None of these is better or worse than another.

* the timer and I2C drivers define `TIMER_DIR` and `I2C_DRIVER_DIR`, while the ethernet drivers define `ETHERNET_DRIVER` without the _DIR;

Should all probably be without the DIR. One thing I'm concerned about, and should probably fix, is the possibility that more than one different I2C or timer driver may be in the system at the same time: I should probably give the variables more constrained names, so they don't collide in that case, as were running in a global namespace.

* the timer driver is referred to in the variables as just timer, while the ethernet and I2C are referred to as *_driver;

* the ELF files are called `timer.elf`, `eth.elf`, `uart_driver.elf` and `i2c_driver.elf`—I suggest `timer_driver.elf`, ethernet_driver.elf`;

Usually I kept the names they already had. The one exception was the network vitualisers that I added 'network_' a a prefix for so they could coexist with the serial virtualisers.

* the .mk files themselves are called `timer.mk`, `uart.mk`, `ethdriver.mk` and `i2c_driver.mk`—I suggest `timer_driver.mk`, `uart_driver.mk`, `ethernet_driver.mk` and `i2c_driver.mk`;

OK. Although the directory they're in makes it clear they're drivers.

* the meson ethernet driver has a Makefile as well as a .mk;

The Makefile pre-existed; I didn't want to delete it in the patch until I know what's using it.

* some driver make fragments have notes about what is required in the system description while others don't;

That needs fixing.

* the timer drivers get the directory via `$(dir $(lastword $(MAKEFILE_LIST)))`, while the others just hardcode it;

Should probably use this mechanism everywhere. It's a GnuMAKE only thing though.

* driver as abbreviated sometimes as drv, other times as driv.

Meant to be drv everywhere, I'll try to search+replace

Questions:

* given we are moving to make fragments to be included by third parties, might it be better to add a prefix to identifiers?

Yes!

* if all the meat of the building is in the fragments, could it be better to have a global Makefile (either in root or in examples/) instead of one per example system (one would still be able to build a specific system by specifying the target)?

No --- the examples are meant to be standalone, and include only the fragments needed.

wom-bat added 21 commits May 17, 2024 10:11
Add snippets to most sDDF components
Alter the echo_server Makefile to match,

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
-- Update system files for rename virt_[rt]x.elf ->
   network_virt_[rt]x.elf
-- Make sure things get rebuilt if the board name is changed

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Add timer.mk and benchmark.mk files, and use them in the echo example.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Add more Make snippets; create i2c.mk to use them.

There's a little renaming to avoid namespace clashes.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Also add IMX8MM_evk target

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Also fix echoserver Makefile snippet to use newlib.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
It doesn't make sense to use the serial version if we're in a UART
driver anyway

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
IT didn't use to remove the CFLAGS encoding.

Also rename IMAGES variable as it's too generic, and can clash with
other Makefiles.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Add notes as to what's generated in each non-driver makefile snippet.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Fix serial->network copy/paste error

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
'unimplemented' stubs are in  libsddf_util_debug.a
so include that _after_ libc

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Use MICROKIT_SDK instead of MK_SDK

This means we need to use override to reset the MICROKIT_SDK variable
to its absolute path

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
It's not necessary, and confuses make on MacOSX.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
MacOS archives ned ranlib to be run on them.
MacOS doesn't have md5sum, so use shasum instead.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
On Linux they produce a trailing filename that needs to be deleted.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Include libsddf_util_debug.a in the libraries for every  build, so the
stubs for unimplemented system calls can be found
This removes the need for explicitly including it where it was
included before.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
wom-bat added 11 commits May 17, 2024 10:11
Signed-off-by: Peter Chubb <peter.chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Mainline fix removed the ARP component.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
So that the constant can be global, as other virtualisers may have
more than one client, we use a more specific name for the
virtualiser's NUM_CLIENTS constant.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Without this, on subsequent rebuilds, all the dependencies including
header files are handed to ld which (rightly) complains.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
This forces rebuild when MICROKIT_BOARD (etc) change.

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Make all driver files be named *_driver.elf
Use the GnuMAKE feature for getting directory instead of hardcoding
relative to ${SDDF}
Add NOTES to each driver snippet as to the variables expected in the
SDF (this doesn't include the SDDF communications interfaces which are
common to all drivers of a class, only the hardware registers
addresses)

Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Signed-off-by: Peter Chubb <Peter.Chubb@unsw.edu.au>
Copy link
Collaborator

Choose a reason for hiding this comment

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

None of the other network drivers have both a Makefile and a .mk, is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was there before I started; I don't want to delete it until I know why it's there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's there since we started to do Makefile snippets. It should be deleted.

@wom-bat wom-bat merged commit e2f87a9 into au-ts:main May 17, 2024
4 checks passed
@wom-bat wom-bat deleted the fixes branch May 17, 2024 02:10
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.

3 participants