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

Add ad9375 support #474

Merged
merged 7 commits into from
Nov 29, 2023
Merged

Add ad9375 support #474

merged 7 commits into from
Nov 29, 2023

Conversation

trishaange01
Copy link
Contributor

@trishaange01 trishaange01 commented Sep 3, 2023 •
edited
Loading

Description

Added support for ad9375 under ad937x.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this been tested?

The class was tested on the ADRV9375 connected to ZC706.

Test Configuration:

  • Hardware: ZC706-ADRV9375
  • OS: Windows

Documentation

If this is a new feature or example please mention or link any documentation. All new hardware interface classes require documentation.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have signed off all commits and they contain "Signed-off by: "
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@trishaange01 trishaange01 reopened this Sep 3, 2023
@trishaange01 trishaange01 marked this pull request as draft September 3, 2023 16:04
@trishaange01
Copy link
Contributor Author

Hi @tfcollins! I just have a few concerns about this support:

  1. Differences in attribute value displayed in iio osc and value read in property:
  • CLGC: desired gain, orx rms, tx rms, tx gain
  • DPD: external path delay
  • VSWR: forward gain, forward orx, forward tx, reflected gain, reflected orx, reflected tx
  1. DPD model error: 100% is displayed in iio osc and 1000 is read in property
  2. DPD status: Error:ORx signal too low is displayed in iio osc and 5 is read in property
  3. How should DPD reset (button) be handled?

Is there a documentation for this?
Thank you.

@tfcollins
Copy link
Collaborator

tfcollins commented Sep 13, 2023 •
edited
Loading

Hi @tfcollins! I just have a few concerns about this support:

  1. Differences in attribute value displayed in iio osc and value read in property:
  • CLGC: desired gain, orx rms, tx rms, tx gain
  • DPD: external path delay
  • VSWR: forward gain, forward orx, forward tx, reflected gain, reflected orx, reflected tx

These probably get translated to specific units (dB, ms, ...). Do they have related scaling properties?

  1. DPD model error: 100% is displayed in iio osc and 1000 is read in property

This is probably just scaled by 1/10. I would just add the same to the property.

  1. DPD status: Error:ORx signal too low is displayed in iio osc and 5 is read in property

This is just a lookup from here: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-transceiver/ad9371#dpd_status Can you add the table to the property

  1. How should DPD reset (button) be handled?

This should probably be just a generic method (not a property) and it performs the toggle: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-transceiver/ad9371#dpd_reset_control

Is there a documentation for this? Thank you.

Driver doc: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-transceiver/ad9371

@trishaange01
Copy link
Contributor Author

trishaange01 commented Sep 14, 2023 •
edited
Loading

Hi @tfcollins! I just have a few concerns about this support:

  1. Differences in attribute value displayed in iio osc and value read in property:
  • CLGC: desired gain, orx rms, tx rms, tx gain
  • DPD: external path delay
  • VSWR: forward gain, forward orx, forward tx, reflected gain, reflected orx, reflected tx

These probably get translated to specific units (dB, ms, ...). Do they have related scaling properties?

Yes, they have related scaling properties. I already found the conversion in the link you sent.

  1. DPD model error: 100% is displayed in iio osc and 1000 is read in property

This is probably just scaled by 1/10. I would just add the same to the property.

Okay. I'll just add the same.

  1. DPD status: Error:ORx signal too low is displayed in iio osc and 5 is read in property

This is just a lookup from here: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-transceiver/ad9371#dpd_status Can you add the table to the property

Do you mean add the table only as comment or should the raw value be translated?

  1. How should DPD reset (button) be handled?

This should probably be just a generic method (not a property) and it performs the toggle: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-transceiver/ad9371#dpd_reset_control

Is there a documentation for this? Thank you.

Driver doc: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-transceiver/ad9371

Do I need to add comments to the properties for documentation?

I'll just read through the documentation. Thank you!

@tfcollins
Copy link
Collaborator

Add a lookup mechanism for the strings so its easier to user.

Property should just have basic docstrings. I would put a link to the driver doc in the class docstring

@trishaange01 trishaange01 force-pushed the ad9375_support branch 2 times, most recently from 9ff2758 to 5b3a0f6 Compare October 5, 2023 06:17
@trishaange01 trishaange01 marked this pull request as ready for review October 5, 2023 06:18
@trishaange01
Copy link
Contributor Author

Hi @tfcollins. This PR is now ready for review. Thanks! :)

@dlech dlech mentioned this pull request Oct 5, 2023
Merged
10 tasks
@trishaange01 trishaange01 force-pushed the ad9375_support branch 2 times, most recently from aa4ff21 to 684e65e Compare October 23, 2023 02:22
@trishaange01
Copy link
Contributor Author

Hello @tfcollins! I think this PR is ready for review. Thanks!

@tfcollins tfcollins self-requested a review October 25, 2023 03:09
@tfcollins tfcollins changed the base branch from master to main October 26, 2023 17:13
tfcollins
tfcollins reviewed Nov 17, 2023
View reviewed changes
supported_parts.md Outdated
@@ -96,8 +96,7 @@
- AD9250
- AD9265
- AD936X (Pluto, FMComms2/3/4/5, ADRV936X)
- AD9371
- AD9375
- AD937X (AD9371, AD9375, ADRV937X)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be just AD9371 and AD9375 on separate lines. ADRV937X doesnt exist

@tfcollins tfcollins merged commit f7ceaff into analogdevicesinc:main Nov 29, 2023
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfcollins tfcollins tfcollins approved these changes

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
@trishaange01 @tfcollins