cpu/efm32: add periph_adc support for Gecko Series 2 #18933
Conversation
jue89
added
Type: new feature
jue89
requested review from
leandrolanzieri,
jia200x,
MrKevinWeiss and
basilfx
as code owners
github-actions
bot
added
Area: boards
jue89
force-pushed
the
feature/cpu_efm32_series_2_adc
branch
from
77af3f2 to
d759701
Compare
|
|
||
| /* wait for the conservation to provide the result in the fifo */ | ||
| while ((IADC_getStatus(adc_config[dev].dev) & IADC_STATUS_SINGLEFIFODV) == 0); | ||
| uint32_t result = IADC_pullSingleFifoData(adc_config[dev].dev); |
There was a problem hiding this comment.
If you set a negative input for differential mode, can't the measured voltage be negative?
You'd want a int32_t result then.
There was a problem hiding this comment.
RIOT/cpu/efm32/periph/adc_series2.c
Line 68 in d759701
This line makes sure that the result is never negative.
I took this route to be consistent with the adc_sample() interface which uses negative values to indicate errors.
There was a problem hiding this comment.
I took this route to be consistent with the
adc_sample()interface which uses negative values to indicate errors.
on sam0 I just ignored that interface convention - if there is a wrong config the function should just throw an assert() failure.
adc_sample() just doesn't do this because the test does use this to query supported resolutions, but I think no real-world application will do that.
There should just be a separate interface to check if a resolution is valid (adc_ng does this).
There was a problem hiding this comment.
Hmm, I don't want to mess around with the defined (and of course very limited) ADC interface.
Otherwise the user can't tell whether something wrong or the ADC just measured -1. Instead, I'd would wait for an ADC interface that's capable of reporting negative values ...
There was a problem hiding this comment.
What does iadcCfgTwosCompUnipolar mean?
Will the application have to deal with this then?
There was a problem hiding this comment.
It clamps the value to 0 if it would become negative. cf. https://docs.silabs.com/gecko-platform/3.0/emlib/api/efr32xg22/group-iadc#gadaad7387cbc0b5a22c631d54fa01d70c
There was a problem hiding this comment.
We should add a adc_check_res(adc_t line, adc_res_t res) API so that adc_sample() will never need to return error.
There was a problem hiding this comment.
Huh? There's already an API for that?
There was a problem hiding this comment.
I only see adc_init(adc_t line) and adc_sample(adc_t line, adc_res_t res) in periph/adc.h
There was a problem hiding this comment.
Ah, it's an idea for a follow-up? We should have a deeper look into adc_ng - maybe that's a good abstraction to fit this case. Otherwise we have the go through the API change process ...
jue89
force-pushed
the
feature/cpu_efm32_series_2_adc
branch
from
7f0728e to
baed961
Compare
jue89
added
CI: ready for build
Series 2 features IADCs instead of ADCs.
jue89
force-pushed
the
feature/cpu_efm32_series_2_adc
branch
from
df60c54 to
ab86198
Compare
aabadie
removed
the
CI: ready for build
|
Huh? Where's the |
jue89
added
the
CI: ready for build
Contribution description
This is a follow-up of #18780 and adds support for
periph_adcTesting procedure
I added two ADC lines to the
xg23-pl6068aboard and tested the ADC usingtests/periph_adc.ADC_LINE(0)is a single-ended input onPA10. I connected an almost-empty battery to PA10 and GND.The battery has a voltage of 147mV. It's multiplied with a gain of 0.5 and compared to the internal bandgap of 1.21V. The test sets a resolution of 10bit.
Expected result:
(147mV * 0.5 / 1,210mV) * (2^10 - 1) = 62.14Measured result:
62.ADC_LINE(1)is a differential input betweenPA5andPA0. I connected the same battery to these inputs.Expected result: The same is true like described for
ADC_LINE(0)Measured result:
62.Issues/PRs references
Follow-up of #18780