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

Adrv9009 #149

Merged
merged 30 commits into from
Sep 27, 2024
Merged

Adrv9009 #149

merged 30 commits into from
Sep 27, 2024

Conversation

liambeguin
Copy link
Contributor

No description provided.

@liambeguin
tests: clocks: fix and use solver fixture
6bbf5eb 
Fix naming typo that remained undetected because the solver was never
used.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
tests: clocks: drop duplicate test case
cf1bc6d 
Now that the fixture is used, this test case is duplicated.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
tests: adrv9009: factorize test cases
a51246c 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
tests: clocks: factorize ad9523_1_daq2_validate test case
e1065d8 
Use fixtures to drop duplicated code.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
clocks: ad9528: fix typo in divider name
03c9303 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
clocks: ad9528: fix N2 range
238a776 
datasheet says the available range for N2 is 1 to 255.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
clocks: ad9528: add sysref support
ff2bb54 
Add SYSREF support, this makes sysref optional optional.
By default, K will not be computed.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
clocks: ad9528: add vco and vcxo to configuration output
e8cdff3 
This can be useful to users, and to validate other parameters in test
cases.

These are also available in the ad9523_1 output.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
clocks: ad9528: allow for duplicate output frequencies
ea82ec0 
Allow users to defines all output frequencies even if they have the same
value.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
tests: clocks: add ad9528 test case
7bf6aa6 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
tests: clocks: add sysref test case for the ad9528
e4ff1ab 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: adrv9009: add explicit converter names
c98be20 
In preparation for adrv9009 test cases, add explicit names to each
nested converter.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: ad9081: drop unused _model_type
6adeed2 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: ad9081: make use of _add_intermediate
0f99480 
Simplify code by making use of gekko_trans.py

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: adrv9009: fix nested support
d267a9f 
Update nested support to allow users to run something like:

	sys = adijif.system("adrv9009", "ad9528", "xilinx", vcxo, solver=solver)

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
tests: adrv9009: mark gekko tests as expected failures
bed5263 
The adrv9009 class currently doesn't support the gekko solver.
Mark test cases as expected failures until this is implemented.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
jesd: fix copy-paste error in docstring
4869e76 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
jesd: use f-strings
cd2f84f 
Add missing f-string marker on Exceptions, and drop .format(), when not
exceeding the characters per line limit.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: adrv9009: derive adrv9009_core from converter
78358bf 
Parts of the codebase rely on the fact that converters are derived form
the converter class (and thus jesd class). Make sure the ADRV9009
follows that assumption before editing the rest of the code.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
system: fix type hint on converter input
eb9658e 
The codebase already allows for passing a list of strings.
Make sure the type hint reflects that option.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
system: use proper L parameter for nested devices
a530ccd 
Nested converters still inherit from the jesd class, and have their own
default lane count parameter (L). This value isn't really used and
should be the sum of nested converters.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
docs: update list of supported parts
99ede21 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
Copy link
Contributor Author

Hi @tfcollins,

I'm not sure this is complete, but everything seems to work so far.

I wonder if we should put more guards in place to prevent users from setting parameters in the wrong place when using nested converters. For example since adrv9009, adrv9009_rx, and adrv9009_tx all inherit from the converter class you can set adrv9009.sample_rate which won't work.

@liambeguin liambeguin marked this pull request as ready for review September 28, 2023 17:11
@tfcollins
Copy link
Collaborator

@liambeguin thanks for the contribution. I'll take a deeper look tomorrow through things, but can you check the CI errors first

@liambeguin
utils: rename jesd_mode to jesd_class
f0d48f8 
Rename key in get_jesd_mode_from_params' return list, allowing users
directly expand an item into set_quick_configuration_mode().

This also avoids further confusion with jesd_mode in the rest of the
codebase which usually refers to the mode number describing the jesd204
set of parameters and not the jesd204 class (jesd204, jesd204a,
jesd204b, jesd204c).

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: adrv9009: update interpolation/decimation parameters
4d94b30 
Update the list of available parameters for the interpolation and
decimation stages of the ADRV9009.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: adrv9009: adjust converter_clock range
3ff7ce6 
Update converter_clock_(min|max) based on
https://www.analog.com/media/en/technical-documentation/data-sheets/ADRV9009.pdf

Table 1:
	Data Rate per Channel (NRZ)
		min=2135 Mbps
		max=12288 Mbps

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
examples: add adrv9009-pcbz sample
a088a2e 
Add an example based on the default configuration of the adrv9009-pcbz.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
converters: adrv9009: fix jesd_mode config output
9d14f95 
jesd_mode in sys.get_config() always returns None.
Make sure to pass down the parent class' data.

Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
docs: remove duplicate SYSREF definition
07f0a8b 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
docs: fpga_internal: fix minor typos and formatting
dba090c 
Signed-off-by: Liam Beguin <[email protected]>
@liambeguin
Copy link
Contributor Author

I went over the CI errors, and sent a PR to pyadi-dt for the import errors: analogdevicesinc/pyadi-dt#23

I have a few more comments/questions:

  • Would it make sense to replace fpga.force_(cpll|qpll|qpll1) by fpga.sys_clk_select =(XCVR_CPLL|XCVR_QPLL0|XCVR_QPLL1) we already use that format for out_clk_select, it would also take care of having only one active at a time, and would match the output data.
  • requires_separate_link_layer_out_clock seems unused. should it be removed? I noticed that because I can't seem to be able to generate a zcu102+adrv9009 configuration that has ref_clk == link_out_clk. Maybe I missed something in the example?
  • I was trying to add an equation to resolve the value of HS Div for the ADRV9009, but I'm not sure how valuable that is.
  • would it make sense to rename {conv.name}_fpga_link_out_clk to {conv.name}_fpga_line_rate to match the rest of the documentation? Going through the code documentation helped a lot with figuring out what names are equivalent.
  • I added a separate sysref to the ad9528 but that hasn't been hooked up at the system level, not sure what the best way to do that would be.
  • I know there's also pyadi-dt but did you have something in mind to extract parameters from an active system? or something to validate parameters in an existing vivado project?

@tfcollins tfcollins self-requested a review October 26, 2023 17:53
@tfcollins tfcollins merged commit 4eafcef into analogdevicesinc:main Sep 27, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfcollins tfcollins Awaiting requested review from tfcollins

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

Successfully merging this pull request may close these issues.
2 participants
@liambeguin @tfcollins