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

driver/at86rf215: add functions to configure trim & clock output at run-time #15120

Merged
merged 4 commits into from Dec 2, 2020

Conversation

benpicco
Copy link
Contributor

Contribution description

To calibrate the at86rf215 radio, trim value has to be set at run-time during board production.
Add two helper functions to control the trim value and clock output register.

This also adds a gnrc_netif_get_netdev() netdev function to get a pointer to a previously registered netdev from netif.

Testing procedure

tests/driver_at86rf215 has gained three new functions:

  • set_trim set the trim value
  • set_clko enable clock output
  • get_random get numbers from the radio's random number generator (previously untested)

If you connect a frequency counter to the clock output pin, you should be able to measure the set frequency and see it change when adjusting the trim value.

Issues/PRs references

@benpicco benpicco added Area: drivers Area: Device drivers Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Sep 29, 2020
@benpicco benpicco added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 19, 2020
@benpicco benpicco removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 30, 2020
@benpicco benpicco requested a review from maribu November 30, 2020 16:49
@benpicco
Copy link
Contributor Author

benpicco commented Dec 1, 2020

rebased to solve the merge conflict

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 1, 2020
@benpicco benpicco added this to Backlog / Proposals in RIOT Sprint Day via automation Dec 1, 2020
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

And again, I forget to submit a review. Let's see if the comments are still valid

drivers/include/at86rf215.h Outdated Show resolved Hide resolved
drivers/include/at86rf215.h Outdated Show resolved Hide resolved
drivers/include/at86rf215.h Outdated Show resolved Hide resolved
sys/net/gnrc/netif/gnrc_netif.c Outdated Show resolved Hide resolved
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Some comments for the test

tests/driver_at86rf215/main.c Show resolved Hide resolved
tests/driver_at86rf215/main.c Show resolved Hide resolved
tests/driver_at86rf215/main.c Outdated Show resolved Hide resolved
tests/driver_at86rf215/main.c Outdated Show resolved Hide resolved
tests/driver_at86rf215/main.c Outdated Show resolved Hide resolved
@maribu maribu added the Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines label Dec 2, 2020
@maribu maribu added Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Dec 2, 2020
@maribu
Copy link
Member

maribu commented Dec 2, 2020

@benpicco: Code looks good to me. I cannot test this with my hardware, as

  1. I cannot access the clock output due to missing testing point (will be fixed by next revision)
  2. I'm using a TCXO instead of a crystal as clock source, but trimming only makes sense for crystals.

Maybe you could give the test another spin after squashing and document your result here?

@benpicco
Copy link
Contributor Author

benpicco commented Dec 2, 2020

2020-12-02 12:44:51,311 #  set_clko 26
2020-12-02 12:44:51,314 # Clock output set to 26 MHz

IMG_0305

2020-12-02 12:46:09,985 #  set_trim 15
2020-12-02 12:46:09,986 # setting trim to 4500 fF

IMG_0306

2020-12-02 12:46:38,560 # set_clko 32
2020-12-02 12:46:38,563 # Clock output set to 32 MHz

IMG_0310

2020-12-02 12:47:16,320 #  set_clko 8
2020-12-02 12:47:16,322 # Clock output set to 8 MHz

IMG_0311

@maribu maribu added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Dec 2, 2020
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. @jia200x, any objections to merging?

RIOT Sprint Day automation moved this from Backlog / Proposals to Waiting For Murdock Dec 2, 2020
Add a function to retrieve a netdev from the list of interfaces.
To calibrate the at86rf215 radio, trim value has to be set at run-time
during board production.
Add two helper functions to control the trim value and clock output register.
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

my ACK remains valid

@jia200x
Copy link
Member

jia200x commented Dec 2, 2020

I'm fine with it!

My only comment is that I think we should resolve in the mid term how to access device specific stuff + how to represent a device within the OS. Accessing the device descriptor directly when there are network interfaces on top seems to be sensible, since changing any internal state might affect what the network interface/stack expects from the radio.

IMO this should be done through a generic netif_t (not gnrc_netif_t). But this is completely independent of this PR :)

@benpicco benpicco merged commit 665c07e into RIOT-OS:master Dec 2, 2020
RIOT Sprint Day automation moved this from Waiting For Murdock to Done Dec 2, 2020
@benpicco benpicco deleted the driver/at86rf215-trim branch December 2, 2020 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants