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

update examples build system #186

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

update examples build system #186

wants to merge 3 commits into from

Conversation

playduck
Copy link
Contributor

This PR aims to improve the makefiles of the example projects.
The goal is to make the examples easier to understand, use and maintain.

Changes:

  • Created a tools.mk file to specify either local or docker tools
  • Created a common.mk file which includes all targets for ICE and ECP5
  • Gave all examples makefiles which include the aforementioned files
  • Added a README to /examples

Every project has it's own makefile.
There it can define its own properties (which source files, what package, etc.) and it implements its own targets if required.
The common makefile includes all different targets to call yosys, nextpnr and others.
The source of the tools can either be the local tools (default behaviour) or the docker tools by specifying make USE=DOCKER=1.
Adding a new example can usually rely on the common.mk makefile.
All projects successfully generate a bitstream.

Current Issues:

  • As I don't have the hardware I couldn't test any generated bitstream
  • Docker is untested (Docker image is only available as arm64, i'm on aarch64)
  • I don't know where or if icetime and iceprog can be accessed in the docker image. For now I've left their defintion in tools.mk blank
  • common.mk may be overwhelming to new users making the build process seem more complicated. Perhaps splitting these up into a makefile for Ice and one for ecp5?

@tgingold
Copy link
Member

Ok for me.
@umarcor, any comment ?

OPENOCD = $(DOCKER) $(DOCKERARGS) --device /dev/bus/usb ghdl/synth:prog openocd

ICEPORG = # ?
ICETIME = # ?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before merging, could someone check if iceprog and icetime exist in the docker image?
And if they do, how would they be included and would they need any flags like openocd does?

@umarcor
Copy link
Member

umarcor commented Apr 20, 2023

Overall, the proposal is correct and adequate. Thanks for the PR!

There are at least two similar solutions which I am aware of (because I did implement them either partially or totally):

You can see that those solutions "split common.mk into multiple makefiles", as @playduck commented above.

Hence, please let me have the weekend to have a better look at it and hopefully catch some simple envvar renames that can make this more consistent with those implementations.

Eventually, we might download and reuse the makefiles in this repo (e.g. https://github.com/stnolting/neorv32-setups/blob/main/osflow/tools.mk), instead of duplicating them. Nevertheless, I will try to review this PR to have it merged ASAP, we don't want to overcomplicate it now, because it does look good enough already.

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