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

Update cn0540 spi engine #1207

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Update cn0540 spi engine #1207

merged 1 commit into from
Feb 13, 2024
+69 −83

Conversation

PIoandan
Copy link
Collaborator

@PIoandan PIoandan commented Nov 1, 2023 •
edited by IuliaCMoldovan
Loading

PR Description

Replaced the SPI Engine connections in the cn0540_bd.tcl with the spi_engine_create procedure found in the spi_engine.tcl.
Through these changes, a more generic instantiation for the spi_engine can be achieved.
I also created the cn0540_ardz.txt file.
It was tested with Linux and it worked well.

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

@PIoandan PIoandan requested a review from PopPaul2021 November 1, 2023 14:36
@gastmaier gastmaier self-requested a review January 11, 2024 17:19
gastmaier
gastmaier previously approved these changes Jan 12, 2024
View reviewed changes
Copy link
Contributor

@gastmaier gastmaier 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.
Synthesized on my side also.
Just one question not really related to the changes:
How does the offload trigger adc_data_ready behave?
Does it ticks periodically or stays high during bursts?
If the second, #1249 might break this project.

@LBFFilho
Copy link
Contributor

@gastmaier I believe adc_data_ready comes from the DRDY signal from a ad7768-1. This is a periodic signal indicating the end of each conversion, so I think this is not going to be affected.

PopPaul2021
PopPaul2021 requested changes Feb 6, 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.

During the build process for CoraZ7s, we need to verify the create_generated_clock -name SCLK_clk constraint due to a warning indicating that it cannot be evaluated.

ladace
ladace previously approved these changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@ladace ladace left a comment

Choose a reason for hiding this comment

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

Please update the copyright to - 2024 in changed files :)

@PIoandan
Copy link
Collaborator Author

PIoandan commented Feb 7, 2024

During the build process for CoraZ7s, we need to verify the create_generated_clock -name SCLK_clk constraint due to a warning indicating that it cannot be evaluated.

The creation of the generated clock will be evaluated in the implementation phase and the error appears during the synthesis phase.

@PIoandan PIoandan dismissed stale reviews from ladace and gastmaier via ac4f34a February 7, 2024 14:55
PopPaul2021
PopPaul2021 approved these changes Feb 7, 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!

@PIoandan
Copy link
Collaborator Author

PIoandan commented Feb 7, 2024

Please update the copyright to - 2024 in changed files :)

Done in V2.

@PIoandan PIoandan requested review from ladace and gastmaier February 7, 2024 14:59
ladace
ladace requested changes Feb 13, 2024
View reviewed changes
Copy link
Contributor

@ladace ladace left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't be better to squash the two commits in only one, in rest looks fine to me

@PIoandan
Update cn0540 spi engine
9bc2790 
I replaced the SPI Engine connections in the cn0540_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 also created the cn0540_ardz.txt file. It was tested on linux
and worked well.
Update the Copyright.

Signed-off-by: Ioan-daniel Pop <Pop.Ioan-daniel@analog.com>
@PIoandan PIoandan force-pushed the update_cn0540_spi_engine branch from ac4f34a to 9bc2790 Compare February 13, 2024 14:01
@PIoandan
Copy link
Collaborator Author

Shouldn't be better to squash the two commits in only one, in rest looks fine to me

Done.

ladace
ladace approved these changes Feb 13, 2024
View reviewed changes
Copy link
Contributor

@ladace ladace left a comment

Choose a reason for hiding this comment

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

Perfect!

@PIoandan PIoandan merged commit 2954460 into main Feb 13, 2024
1 of 2 checks passed
@PIoandan PIoandan deleted the update_cn0540_spi_engine branch February 13, 2024 14:15
@gastmaier gastmaier mentioned this pull request Feb 26, 2024
Merged
12 tasks
bia1708 pushed a commit to bia1708/hdl that referenced this pull request Apr 5, 2024
@PIoandan @bia1708
Update cn0540 spi engine (analogdevicesinc#1207)
92f8b30 
Signed-off-by: Ioan-daniel Pop <Pop.Ioan-daniel@analog.com>
ladace pushed a commit that referenced this pull request Jun 13, 2024
@PIoandan @ladace
Update cn0540 spi engine (#1207)
dc3ea10 
Signed-off-by: Ioan-daniel Pop <Pop.Ioan-daniel@analog.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ladace ladace ladace approved these changes

@PopPaul2021 PopPaul2021 PopPaul2021 approved these changes

@acostina acostina Awaiting requested review from acostina

@gastmaier gastmaier Awaiting requested review from gastmaier

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

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