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

QMTech boards: make Artix7 bitstream/platform commands conditional on toolchain #541

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

hansfbaier
Copy link
Contributor

I also removed setting .f4pga_device since it is identical to platform.device

@enjoy-digital
Copy link
Member

Thanks @hansfbaier. In the past, I tried to avoid specializing platforms/target to the requirements of the different toolchain. We agree to allow it on the Digilent Arty to still allow fast experimentation/developments but with the aim in the long term to remove it. I think the different toolchain will try to align with Vivado commands in the long terms, so it would probably be better to handle there unsupported commands directly in the LiteX build backend for the toolchain. Ex have the build backend catch a "set_property INTERNAL_VREF" command and generate a Warning since not supported but also not inserting it in the generated files. This way, as the toolchain support improve, you'll just be able to modify the build backend and progressively remove the modifications, without requiring modifying the different platforms.

@hansfbaier
Copy link
Contributor Author

Yes that totally makes sense. Can you point me to the code location where that would be?

@enjoy-digital
Copy link
Member

I think we could overload add_platform_command in LiteX for the concerned build back-ends and catch the set_property XXYY to threat them differently: Add a warning and avoid adding the command.

@hansfbaier
Copy link
Contributor Author

OK, implemented the requested changes here.

@enjoy-digital enjoy-digital merged commit 554cdaa into litex-hub:master Oct 24, 2023
1 check passed
@enjoy-digital
Copy link
Member

Thanks @hansfbaier.

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.

2 participants