Skip to content

New project AD916x & dac_fmc_ebz update #1617

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cristianmihaipopa
Copy link
Collaborator

@cristianmihaipopa cristianmihaipopa commented Mar 19, 2025

PR Description

AD916x (61-64) is a family of DACs which was previously part of dac_fmc_ebz project. It was migrated out and made a separate project. Due to the migration, the following changes were made:

  • new separate project for AD916x, on ZCU102
  • new documentation for AD916x
  • reverted back the changes and removed all the references for AD916x from dac_fmc_ebz
  • updated the documentation for dac_fmc_ebz
  • included cache coherency support

All the devicetrees for each device were merged and support cache coherency. Also, the driver has been updated to fix some issues. The project was tested in hardware.

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)
  • Documentation

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

- AD916x is a family of DACs
- This JESD project was migrated completly
from dac_fmc_ebz/project
- All the files have been trimmed accordignly,
leaving only the relevant code for AD916x
(removed unsed contraints, signals etc.)
- A documentation page was also created for
this project

Signed-off-by: Cristian Mihai Popa <[email protected]>
- Removed AD916x and reverted the files affected
by the adittion of the DAC family back to the
original state
- Due to the migration of AD916x out of dac_fmc_ebz,
a dedicated doc. page has been created, thus removing
all mentions of AD916x from dac_fmc_ebz doc.

Signed-off-by: Cristian Mihai Popa <[email protected]>
Signed-off-by: Cristian Mihai Popa <[email protected]>
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.

Split into 5 commits:

  1. bug fix on dac_fmc_ebz (with explanation in the commit body that it reverts another commit and give the ID)
  2. addition of the ad916x project
  3. addition of the ad916x documentation
  4. deletion of the ad916x devices from the dac_fmc_ebz project
  5. changes for dac_fmc_ebz documentation

After doing all these changes, please retrigger the CI, and wait for the build on all dac_fmc_ebz projects and ad916x to analyze eventual warnings/critical warnings.

source ../common/dac_fmc_ebz_bd.tcl

if {[info exists ::env(ADI_LANE_RATE)]} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the ADI_LANE_RATE just for the AD916x family? Otherwise, why was it removed?
+ Keep the same alignment of the values as it was before, because this way it is easier to read them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it was. Will fix the alignment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove the source of dac_fmc_ebz_bd.tcl?

@IuliaCMoldovan
Copy link
Contributor

Add the project in the CODEOWNERS file, in the commit with the AD916x project addition. The first one should be you and then Paul.

Signed-off-by: Cristian Mihai Popa <[email protected]>
@cristianmihaipopa
Copy link
Collaborator Author

V1: Did all the requested changes. If everything is fine, and no more changes are needed, will structure the commits in 4 big commits as mentioned above.

Signed-off-by: Cristian Mihai Popa <[email protected]>
Signed-off-by: Cristian Mihai Popa <[email protected]>
source ../common/dac_fmc_ebz_bd.tcl

if {[info exists ::env(ADI_LANE_RATE)]} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove the source of dac_fmc_ebz_bd.tcl?

Signed-off-by: Cristian Mihai Popa <[email protected]>
@cristianmihaipopa
Copy link
Collaborator Author

V2: Did the requested changes

Comment on lines +155 to +156
dac_jesd204_xcvr 0x84A6_0000
dac_jesd204_transport 0x84A0_4000
Copy link
Contributor

Choose a reason for hiding this comment

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

Swap line 155 with 156


### Example configurations

#### AD9162, TX mode 08
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### AD9162, TX mode 08
#### AD9162, TX mode 08 (default)

Comment on lines +116 to +124
/*
* Control signals for the FMC boards:
*
* dac_ctrl FMC 916x family
*
* fmc_txen_0 0 H13 FMC_TXEN_0
* fmc_hmc849vctrl 1 D15 FMC_HMC849VCTL
*/

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/*
* Control signals for the FMC boards:
*
* dac_ctrl FMC 916x family
*
* fmc_txen_0 0 H13 FMC_TXEN_0
* fmc_hmc849vctrl 1 D15 FMC_HMC849VCTL
*/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants