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

drivers: frequency: add support for ADMFM2000 #2416

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ranechita
Copy link
Contributor

The ADMFM2000 is a dual-channel microwave downconverter, with input RF and local oscillator (LO) frequency ranges covering 5 GHz to 32 GHz, with an output intermediate frequency (IF) frequency range from 0.5 GHz to 8 GHz. Added driver and iio_support.

Pull Request Description

Please replace this with a detailed description and motivation of the changes.
You can tick the checkboxes below with an 'x' between square brackets or just check them after publishing the PR.
If this PR contains a breaking change, list dependent PRs and try to push all related PRs at the same time.

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 Coding style guidelines
  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe etc), if applies

@ranechita ranechita requested a review from a team January 16, 2025 14:22
@ranechita ranechita self-assigned this Jan 16, 2025
drivers/frequency/admfm2000/admfm2000.h Outdated Show resolved Hide resolved
drivers/frequency/admfm2000/admfm2000.c Outdated Show resolved Hide resolved
drivers/frequency/admfm2000/admfm2000.c Outdated Show resolved Hide resolved
drivers/frequency/admfm2000/admfm2000.c Outdated Show resolved Hide resolved
@ranechita
Copy link
Contributor Author

v2:

  • dropped code separators
  • switched to 2d array for DSA and switch gpios & init params

The ADMFM2000 is a dual-channel microwave downconverter, with
input RF and local oscillator (LO) frequency ranges covering 5 GHz
to 32 GHz, with an output intermediate frequency (IF) frequency
range from 0.5 GHz to 8 GHz. Added driver and iio_support.

Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@analog.com>
@ranechita
Copy link
Contributor Author

v3:

  • shortened param initialization in README

@CiprianRegus
Copy link
Contributor

There seems to be an error regarding the documentation:

Missing admfm2000.rst file at doc/sphinx/source/drivers
Missing drivers/admfm2000 link inside doc/sphinx/source/drivers_doc.rst

Copy link
Contributor

@amiclaus amiclaus left a comment

Choose a reason for hiding this comment

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

please split the code in multiple commits: one for driver, one for iio driver, one for documentation. there are multiple recent examples for this.

*/
int32_t admfm2000_set_channel_mode(struct admfm2000_dev *dev, uint8_t config);

#endif /* SRC_ADMFM2000_H_ */
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: new line


/**
* @brief Initialize the admfm2000 device.
* @param device - The device structure.
Copy link
Contributor

Choose a reason for hiding this comment

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

all these header comments should go in the .c file as you did in the iio_admfm200.c

* @brief Header file for admfm2000 Driver.
* @author Ramona Nechita (ramona.nechita@analog.com)
********************************************************************************
* Copyright 2024(c) Analog Devices, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

* @brief Implementation of admfm2000 Driver.
* @author Ramona Nechita (ramona.nechita@analog.com)
********************************************************************************
* Copyright 2024(c) Analog Devices, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

* @brief Implementation of admfm2000 IIO Driver.
* @author Ramona Nechita (ramona.nechita@analog.com)
********************************************************************************
* Copyright 2024(c) Analog Devices, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

* @brief Header file for admfm2000 IIO Driver.
* @author Ramona Nechita (ramona.nechita@analog.com)
********************************************************************************
* Copyright 2024(c) Analog Devices, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

#define ADMFM2000_MIN_GAIN_RAW 0
#define ADMFM2000_DEFAULT_GAIN -0x20
#define ADMFM2000_NUM_CHANNELS 2

Copy link
Contributor

Choose a reason for hiding this comment

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

drop extra line.

#include "no_os_print_log.h"


int32_t admfm2000_get_gain(struct admfm2000_dev *dev, uint8_t chan,
Copy link
Contributor

Choose a reason for hiding this comment

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

all these functions should return int



int32_t admfm2000_get_gain(struct admfm2000_dev *dev, uint8_t chan,
int32_t *gain)
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you store in the gain variable? doesn't seem to be a gain, rather a raw value after reading gpios.

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

Successfully merging this pull request may close these issues.

4 participants