-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Corundum 10 Gbps QSFP+ support for ADRV9009ZU11EG #1971
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
base: main
Are you sure you want to change the base?
Conversation
projects/adrv9009zu11eg/adrv2crr_fmc/system_constr_corundum.xdc
Outdated
Show resolved
Hide resolved
projects/adrv9009zu11eg/adrv2crr_fmc/system_constr_corundum.xdc
Outdated
Show resolved
Hide resolved
IstvanZsSzekely
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase the PR on the current main branch and solve the conflicts.
d622b03 to
066733e
Compare
|
V1: Did the requested changes. The Guideline checker is failing because of the Copyright in /library/corundum/ethernet/adrv9009zu11eg/ethernet_adrv9009zu11eg.v, but it's similar to the other .v files from /library/corundum/ethernet. |
| be cloned alongside HDL repository. Do a git checkout to the latest tested | ||
| version (commit - 37f2607). When the 10G-based implementation (e.g., in | ||
| this project) is used, apply the indicated patch. Then navigate back to the | ||
| location of the project, and build the project using the enviromental variable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: environment variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo.
7c00bdb to
d11d2f1
Compare
|
V2: Did the requested changes. |
d11d2f1 to
8411aa1
Compare
8411aa1 to
bd0b0cd
Compare
PopPaul2021
left a comment
There was a problem hiding this 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!
bd0b0cd to
4469f44
Compare
|
V3: In corundum.tcl we have a common if for ptp signals for K26 and ADRV9009ZU11EG. |
PopPaul2021
left a comment
There was a problem hiding this 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!
| be cloned alongside HDL repository. Do a git checkout to the latest tested | ||
| version (commit - 37f2607). When the 10G-based implementation (e.g., in | ||
| this project) is used, apply the indicated patch. Then navigate back to the | ||
| location of the project, and build the project using the enviromental variable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo.
library/corundum/ethernet/adrv9009zu11eg/ethernet_adrv9009zu11eg_ip.tcl
Outdated
Show resolved
Hide resolved
- Initial Corundum support, use mode is QSFP+ 10Gbps variant, for ADRV9009ZU11EG - Added the configuration, scripts and ethernet architecure for support on ADRV9009ZU11EG - In corundum.tcl, changed the last if case to be more explicit, for MicroBlaze case Signed-off-by: Cristian Mihai Popa <cristianmihai.popa@analog.com>
- Corundum support is ONLY available for the ADRV9009ZU11EG/ADRV2CRR_FMC variant of the project - Created a new constraints file, alongside with a new top file, for routing the QSFP signals - Added a separate bd.tcl script, to handle the necessary exports, configurations and connections for Corundum features - Added a new build mode for Corundum, by using the new parameter called CORUNDUM (make CORUNDUM=1) - All the Corundum dependecies are contraint by this variable; if the variable it's not specied when using make, the default variant of the project will be built - IMPORTANT: this implementaion of the Corundum IP, bypasses the Corundum Reset Generator (as seen in the other implementations), connects the Corundum Hierarchy reset and clock directly to the PS Signed-off-by: Cristian Mihai Popa <cristianmihai.popa@analog.com>
- Updated the projects/adrv9009zu11eg/adrv2crr_fmc/README.md - Updated ADRV9009ZU11EG documentation Signed-off-by: Cristian Mihai Popa <cristianmihai.popa@analog.com>
2837b7c
4469f44 to
2837b7c
Compare
| 0'b0, // 11 | ||
| 0'b0, // 10 | ||
| 0'b0, // 9 | ||
| 0'b0, // 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vivado critical warnings: multi-driven nets and illegal expression in output port connections.
These signals are inout ports, which mean that they can be both input and output. Having these values hardcoded to any value means that when in input mode, the input signal is driven by the output value of the FPGA and the hardcoded value, thus resulting in multi-driven net.
Please remove these GPIO pins from the IOB instantiation.
| .gpio_i (gpio_i), | ||
| .gpio_o (gpio_o), | ||
| .gpio_t (gpio_t), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Port width mismatch between system_top and system_wrapper.
| ad_connect smartconnect_corundum/ARESETN sys_ps8/pl_resetn1 | ||
| ad_connect smartconnect_corundum/S00_ARESETN sys_ps8/pl_resetn1 | ||
| ad_connect smartconnect_corundum/M00_ARESETN sys_ps8/pl_resetn1 | ||
| ad_connect smartconnect_corundum/M01_ARESETN sys_ps8/pl_resetn1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vivado critical warning: reset pin is asynchronous to the input clock.
Please synchronize the reset to the clock source.
| /* | ||
| * Ethernet: QSFP28 | ||
| */ | ||
| output wire qsfp_resetl, | ||
| input wire qsfp_modprsl, | ||
| input wire qsfp_intl, | ||
| output wire qsfp_lpmode, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qsfp_modsell is not connected in the top module. Is this intentional?
| assign qsfp_drp_rst = rst_125mhz_int; | ||
|
|
||
| wire qsfp_rx_block_lock; | ||
| wire qsfp_gtpowergood; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qsfp_gtpowergood is already declared in the input port list.
| wire qsfp_modprsl_int; | ||
| wire qsfp_intl_int; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These signals are used before they are declared.
| reg qsfp_reset_reg = 1'b0; | ||
| reg qsfp_lpmode_reg = 1'b0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These signals are used before they are declared.
PR Description
Added support for Corundum Network Stack on ADRV9009ZU11EG. This feature is only available for ADRV9009ZU11EG/ADRV2CRR-FMC variant of the project. Currently, this Corundum configuration supports 10 Gbps on QSFP+ connection. In this PR, the main changes are:
In order to build the project and have the Corundum stack integrated, the user needs to use the new parameter called CORUNDUM and set it to 1 (default it's 0).
IMPORTANT: The Corundum stack was integrated according to our already present Corundum infrastructure and it's very similar to the other projects which support Corundum, besides one thing. In this project, the Corundum Reset Generator, it's not used, it's bypassed, the Corundum hierarchy is directly connected to the PS clock (of 250 MHz) and directly connected to the PS aresetn.
PR Type
PR Checklist