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

cpu/sam0_common/periph_adc: add work around for errata 2.1.6 #20994

Merged

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 14, 2024

Contribution description

This adds a delay between enabling the ADC and starting to sample on the SAMD5x MCUs when the internal bandgap reference is used.

image

Testing procedure

Before, adc_sample() on one of the SAMD5x MCUs I have at hands got stuck when the internal bandgap reference was used by never leaving this loop:

0x00001660 in _sample (line=line@entry=0)
    at /home/[email protected]/Repos/software/ML-PA-RIOT/cpu/sam0_common/periph/adc.c:489
489     while (!(dev->INTFLAG.reg & ADC_INTFLAG_RESRDY)) {}

Now, it works also on the "special" MCU that got stuck before:

> saul read 0
2024-11-14 22:14:45,179 # saul read 0
2024-11-14 22:14:45,180 # Reading from #0 (vol|SENSE_ANALOG)
2024-11-14 22:14:45,180 # Data:	          2122 
> saul read 1
2024-11-14 22:14:47,434 # saul read 1
2024-11-14 22:14:47,451 # Reading from #1 (cur|SENSE_ANALOG)
2024-11-14 22:14:47,452 # Data:	            18 

Issues/PRs references

None

@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 14, 2024
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Nov 14, 2024
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Quick review from the phone.
Can't test right now but this looks good.

cpu/sam0_common/periph/adc.c Outdated Show resolved Hide resolved
ztimer_sleep(ZTIMER_USEC, 1);
# else
/* let's rather be safe than sorry */
busy_wait_us(100);
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to wait 100 us here but 40 us with ztimer_usec ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The busy_wait_us() is just counting CPU cycles and not that precise. I make the comment above a bit more specific.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also reduced it two twice the specified amount. That turns out to work reliably on my "bad MCU" affected by the bug and there should be enough safety margin.

This adds a delay between enabling the ADC and starting to sample
on the SAMD5x MCUs when the internal bandgap reference is used.

Co-authored-by: Dylan Laduranty <[email protected]>
@maribu maribu force-pushed the cpu/sam0_common/periph_adc/errata-2.1.6 branch from 51329dd to a89c924 Compare November 14, 2024 21:44
@riot-ci
Copy link

riot-ci commented Nov 15, 2024

Murdock results

✔️ PASSED

a89c924 cpu/sam0_common/periph_adc: add work around for errata 2.1.6

Success Failures Total Runtime
10250 0 10251 16m:12s

Artifacts

@maribu maribu enabled auto-merge November 15, 2024 06:46
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

ACK.
Tested on my same54-xpro with tests/periph/adc which is stuck if
#define ADC_REF_DEFAULT ADC_REFCTRL_REFSEL_INTREF
is used.
With this PR, it works as expected.

@maribu maribu added this pull request to the merge queue Nov 17, 2024
Merged via the queue into RIOT-OS:master with commit f5f6b41 Nov 17, 2024
27 checks passed
@maribu maribu deleted the cpu/sam0_common/periph_adc/errata-2.1.6 branch November 17, 2024 20:26
@maribu
Copy link
Member Author

maribu commented Nov 17, 2024

Thx :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants