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 cn0363 spi engine #1183

Merged
merged 5 commits into from
Oct 25, 2023
Merged

Update cn0363 spi engine #1183

merged 5 commits into from
Oct 25, 2023

Conversation

PIoandan
Copy link
Collaborator

@PIoandan PIoandan commented Sep 27, 2023

PR Description

I replaced the SPI Engine connections in the cn0363_bd.tcl with the
spi_engine_create procedure found in the spi_engine.tcl script. Through
these changes, a more generic instantiation for the spi_engine can be
achieved. I updated the system_constr.xdc and makefile files and created the cn0363_pmod.txt file.
The linux part works fine.

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

set_property PACKAGE_PIN Y11 [get_ports gain0_o]
set_property IOSTANDARD LVCMOS33 [get_ports gain0_o]
set_property PACKAGE_PIN AA11 [get_ports gain1_o]
set_property PACKAGE_PIN Y11 [get_ports gain0_o] ; ## PMOD [2] PMOD JA1 IO_L10P_T1
Copy link
Contributor

Choose a reason for hiding this comment

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

How about also reordering those for clarity? package_pin first, then iostandard?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@IuliaCMoldovan IuliaCMoldovan left a comment

Choose a reason for hiding this comment

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

The PR description and the commit message should be modified to also state why these changes were done. After the PR is approved, the commits must be squashed

I replaced the SPI Engine connections in the cn0363_bd.tcl with the
spi_engine_create procedure found in the spi_engine.tcl script. Through
these changes, a more generic instantiation for the spi_engine can be
achieved. I updated the system_constr.xdc file and
created the cn0363_pmod.txt file.

Signed-off-by: Ioan-daniel Pop <[email protected]>
I modified the Readme.md file.

Signed-off-by: Ioan-daniel Pop <[email protected]>

ad_mem_hp2_interconnect $sys_cpu_clk sys_ps7/S_AXI_HP2
ad_mem_hp2_interconnect $sys_cpu_clk axi_dma/m_dest_axi
ad_mem_hp2_interconnect $sys_cpu_clk axi_dma/m_dest_axi
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add an empty line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

set_property -dict {PACKAGE_PIN AB11 IOSTANDARD LVCMOS33} [get_ports {spi_cs[0]}] ; ## PMOD [1] PMOD JA7 IO_L8N_T1
set_property -dict {PACKAGE_PIN AB10 IOSTANDARD LVCMOS33 PULLUP true} [get_ports spi_sdo] ; ## PMOD [3] PMOD JA8 IO_L9P_T1_DQS
set_property -dict {PACKAGE_PIN AB9 IOSTANDARD LVCMOS33 PULLUP true} [get_ports spi_sdi] ; ## PMOD [5] PMOD JA9 IO_L9N_T1_DQS
set_property -dict {PACKAGE_PIN AA8 IOSTANDARD LVCMOS33} [get_ports spi_sclk] ; ## PMOD [7] PMOD JA10 IO_L11N_T1_SRCC
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add an empty line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Added an empty line at the end of the cn0363_bd.tcl and
system_constr.xdc files.

Signed-off-by: Ioan-daniel Pop <[email protected]>
I edited the cn0363_pmod.txt file.

Signed-off-by: Ioan-daniel Pop <[email protected]>
gastmaier
gastmaier previously approved these changes Oct 10, 2023

# cn0363

PMOD [2] PMOD JA1 GAIN0 gain0_o LVCMOS33 #N/A
Copy link
Contributor

Choose a reason for hiding this comment

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

The ports and pins should be updated similarly to the pulsar project.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

set_property PACKAGE_PIN AA8 [get_ports spi_sclk]
set_property IOSTANDARD LVCMOS33 [get_ports spi_sclk]

set_property -dict {PACKAGE_PIN Y11 IOSTANDARD LVCMOS33} [get_ports gain0_o] ; ## PMOD [2] PMOD JA1 IO_L10P_T1
Copy link
Contributor

Choose a reason for hiding this comment

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

The comments should be revised because they refer to ports that are routed to the FMC connector.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Updated the pin and port name in cn0363_pmod.txt and system_constr.xdc files.

Signed-off-by: Ioan-daniel Pop <[email protected]>
@PIoandan PIoandan merged commit 8621695 into master Oct 25, 2023
1 of 2 checks passed
@PIoandan PIoandan deleted the update_cn0363_spi_engine branch October 25, 2023 12:26
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.

5 participants