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

n3xx: Support clock_source=external with time_source=gpsdo #570

Conversation

draeman-synoptic
Copy link
Contributor

Update list of valid sync sources on N320 so that external refclk can be used in conjunction with internal GPSDO PPS time source.

Pull Request Details

Description

We have a use case that requires using an external refclk to get very low phase noise in the receive chain, but we wanted to use the internal GPSDO PPS to provide coarse time synchronization across distributed radios. A quick review of the schematic and FPGA suggest this case should work fine, but the combination was not in the list of valid sync sources for the radio.

Related Issue

Which devices/areas does this affect?

Impacts supported set of sync sources on N3xx radios.

Testing Done

I've done rudimentary testing on an N320 to confirm the clock locks and PPS is arriving as expected, and more thorough validation across several distributed N320s is underway.

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project. See CODING.md.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes, and all previous tests pass.
  • I have checked all compat numbers if they need updating (FPGA compat,
    MPM compat, noc_shell, specific RFNoC block, ...)

Previously, the GPSDO PPS could only be selected if GPSDO was also
providing refclk. This change allows a use case of externally provided
refclk with the internal GPSDO PPS. This is useful, for example, when
wanting to use a clock with lower phase noise for sampling, but wanting
to use GPSDO PPS for coarse time sync across distribued radios. Note the
HW and FPGA designs already support this combination, and it was just be
filtered as invalid by software.
@mbr0wn
Copy link
Contributor

mbr0wn commented Feb 28, 2022

@draeman-synoptic We deliberately do not support this on N310. The difference between external/external and external/gpsdo is that for the former case, we assume people are providing matching PPS and 10 MHz. Mismatched PPS and 10 MHz is one of those issues that come up a lot with customer support etc. and is a common mistake for setting up synchronized system. One of our engineers spoke about this at GRCon '19: https://youtu.be/MZe-hh0NRiY

external/gpsdo is guaranteed to not have PPS and 10 MHz matched, so we are likely to fail setup&hold times. Now it's not like your USRP will explode if you do that, and maybe your application is resilient enough to deal with bad synchronization (what's a few samples give or take), but we're hesitant to simply enable this feature given the risk of folks de-syncing their devices.

@draeman-synoptic
Copy link
Contributor Author

draeman-synoptic commented Feb 28, 2022

@mbr0wn Thanks for the feedback and the link's to Daniel's talk, which I just watched in its entirety. I can understand how supporting unmatched PPS/10MHz causes a lot of confusion for users trying to setup synchronized sampling. My use case is more of a niche - there are a few N320s that are separated by a few miles and that do not have access to wired networking. I don't need synchronous sampling, I just need to latch a "roughly common" system time across the distributed radios (plus or minus a handful of microsecond is fine). I'm also not concerned with other common pitfalls such as the fact that they'll slowly drift apart due to the oscillator not being GPS-disciplined.

I appreciate the info and understand why this causes more trouble than it's worth across a broad userbase, especially when my use case is rather uncommon. We'll continue to apply a patch like this out-of-tree as needed for our experiments.

@mbr0wn
Copy link
Contributor

mbr0wn commented Feb 16, 2024

Just to close this out: We are not comfortable making this feature available to the wider public, so we'll be closing this PR.

@mbr0wn mbr0wn closed this Feb 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants