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.

Sign up for GitHub

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

Jump to bottom

spi_engine: Create interface_ip.tcl #1251

Merged
merged 4 commits into from
Feb 28, 2024
Merged

spi_engine: Create interface_ip.tcl #1251

merged 4 commits into from
Feb 28, 2024
+243 −642

Conversation

gastmaier
Copy link
Contributor

@gastmaier gastmaier commented Jan 18, 2024 •
edited by IuliaCMoldovan
Loading

PR Description

Use tcl script instead of static xmls for the interface.
Easier to mantain and are not gitignored.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

@gastmaier gastmaier marked this pull request as ready for review January 25, 2024 22:24
IuliaCMoldovan
IuliaCMoldovan reviewed Jan 30, 2024
View reviewed changes
library/spi_engine/interfaces/Makefile Outdated
@@ -0,0 +1,40 @@
####################################################################################
####################################################################################
## Copyright (c) 2024 Analog Devices, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should follow the pattern of the other Makefiles, like the one from library/interfaces/Makefile.
So with 2018-2024 range since these are autogenerated.

@gastmaier gastmaier force-pushed the spi-engine-interface branch from fbb381a to 10da3e7 Compare January 30, 2024 18:46
@gastmaier
Copy link
Contributor Author

gastmaier commented Jan 30, 2024 •
edited
Loading

This pr had the unfortunate side effect of requiring renaming spi_master to spi_engine because dynamic interfaces need to be prefixed by the IP name.
Alternatively, I could have renamed to spi_engine_master instead of spi_engine, however for the other framework, I'm using the IP name as the interface, and for consistence I named the interface the IP name.

@IuliaCMoldovan
Copy link
Contributor

This pr had the unfortunate side effect of requiring renaming spi_master to spi_engine

And unfortunately the copyright year for all the edited files should be updated to contain 2024 as well... so for example if it was "2017, 2019, 2022", you would have to add ", 2024" and not a range.

@gastmaier gastmaier force-pushed the spi-engine-interface branch 2 times, most recently from ad22f28 to 8ddff1d Compare January 31, 2024 12:52
@gastmaier
Copy link
Contributor Author

Done, all projects/libraries were edited on 2023, so I extended the range.
I will squash the commits on merge.

LBFFilho
LBFFilho previously approved these changes Feb 8, 2024
View reviewed changes
PopPaul2021
PopPaul2021 previously approved these changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@PopPaul2021 PopPaul2021 left a comment

Choose a reason for hiding this comment

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

Looks good to me! I've builded the CN0540 on Coraz7s and on DE10Nano just to make sure that it doesn't break the Intel projects.

@sarpadi
Copy link
Contributor

sarpadi commented Feb 26, 2024

Please fix conflicts

@gastmaier
spi_engine: Create interface_ip.tcl
07e080f 
Use tcl script instead of static xmls for the interface.
Easier to mantain and are not gitignored.

Signed-off-by: Jorge Marques <[email protected]>
@gastmaier
spi_engine: Rename spi_master to spi_engine
6fb32fd 
Every interface should be prefixed by the IP name.
In this case, spi_engine.
spi_engine_master would also be valid, but redundant.

Signed-off-by: Jorge Marques <[email protected]>
@gastmaier
spi_engine: Update IPs using spi_engine
059eabb 
Signed-off-by: Jorge Marques <[email protected]>
@gastmaier gastmaier dismissed stale reviews from PopPaul2021 and LBFFilho via 059eabb February 26, 2024 14:22
@gastmaier gastmaier force-pushed the spi-engine-interface branch from 8091c6f to 059eabb Compare February 26, 2024 14:22
@gastmaier
Copy link
Contributor Author

Force pushed to update cn0540 common db, that on pr #1207 changed to use the spi_engine script.
Now the only change on this file is the interface itself

@sarpadi
Copy link
Contributor

sarpadi commented Feb 27, 2024 •
edited
Loading

running "make clean" inside "/hdl/library/spi_engine/interfaces" does not remove .sv files

@gastmaier
library: Ignore **/interfaces/*.sv and clean them
7539df7 
Remove interface/*.sv files on make clean and git ignore them.

Signed-off-by: Jorge Marques <[email protected]>
@gastmaier gastmaier requested a review from podgori as a code owner February 27, 2024 20:40
@gastmaier
Copy link
Contributor Author

I added to the removal list for every /**/interfaces/ and updated the .gitignore on 7539df7.

@gastmaier gastmaier merged commit e2ca5a9 into main Feb 28, 2024
1 of 3 checks passed
@gastmaier gastmaier deleted the spi-engine-interface branch February 28, 2024 13:31
bia1708 pushed a commit to bia1708/hdl that referenced this pull request Apr 5, 2024
@gastmaier @bia1708
spi_engine: Create interface_ip.tcl (analogdevicesinc#1251)
8343d8e 
Use tcl script instead of static xmls for the interface.
Easier to maintain and are not gitignored.
Rename spi_master to spi_engine because every interface should be
prefixed by the IP name; in this case, spi_engine.
Also, remove interface/*.sv files on make clean and git ignore them.

Signed-off-by: Jorge Marques <[email protected]>
@gastmaier gastmaier mentioned this pull request Jun 10, 2024
Merged
11 tasks
ladace pushed a commit that referenced this pull request Jun 13, 2024
@gastmaier @ladace
spi_engine: Create interface_ip.tcl (#1251)
a932019 
Use tcl script instead of static xmls for the interface.
Easier to maintain and are not gitignored.
Rename spi_master to spi_engine because every interface should be
prefixed by the IP name; in this case, spi_engine.
Also, remove interface/*.sv files on make clean and git ignore them.

Signed-off-by: Jorge Marques <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acostina acostina acostina approved these changes

@sarpadi sarpadi sarpadi approved these changes

@PopPaul2021 PopPaul2021 PopPaul2021 approved these changes

@AndreiGrozav AndreiGrozav Awaiting requested review from AndreiGrozav AndreiGrozav is a code owner

@StancaPop StancaPop Awaiting requested review from StancaPop StancaPop is a code owner

@laurent-19 laurent-19 Awaiting requested review from laurent-19

@ladace ladace Awaiting requested review from ladace ladace is a code owner

@LBFFilho LBFFilho Awaiting requested review from LBFFilho LBFFilho is a code owner

@podgori podgori Awaiting requested review from podgori podgori is a code owner

@IuliaCMoldovan IuliaCMoldovan Awaiting requested review from IuliaCMoldovan IuliaCMoldovan is a code owner

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gastmaier @IuliaCMoldovan @sarpadi @acostina @LBFFilho @PopPaul2021