Add silabs series 2 IADC driver with scan conversion and DMA support. #94264
Conversation
Martinhoff-maker
force-pushed
the
adc_series_2
branch
2 times, most recently
from
65ca9a2 to
ed1888b
Compare
|
Martinhoff-maker
force-pushed
the
adc_series_2
branch
3 times, most recently
from
d9e92b0 to
5a91f78
Compare
zephyrbot
added
Release Notes
zephyrbot
requested review from
anangl,
asmellby,
cfriedt,
JarmouniA,
jerome-pouiller,
jhedberg,
kartben,
mnkp,
nashif,
rettichschnidi and
yonsch
Martinhoff-maker
force-pushed
the
adc_series_2
branch
4 times, most recently
from
396684b to
474f69c
Compare
zephyrbot
added
area: Boards/SoCs
area: Tests
Martinhoff-maker
force-pushed
the
adc_series_2
branch
from
5ec41b0 to
d4e6654
Compare
|
V4:
|
Martinhoff-maker
force-pushed
the
adc_series_2
branch
from
d4e6654 to
bc0d950
Compare
|
Need to wait for #96859 to be merged to have the CI green after a rebase. |
There was a problem hiding this comment.
Please take a look at the discussion in #94585 (comment) regarding all the samples' overlays/conf files added in this PR, it's not scalable at all and they have little added value of any.
Non-blocking
| extra_configs: | ||
| - CONFIG_SEQUENCE_RESOLUTION=20 | ||
| - CONFIG_SEQUENCE_OVERSAMPLING=10 | ||
| - CONFIG_SEQUENCE_32BITS_REGISTERS=y |
There was a problem hiding this comment.
these should be in board .conf to be usable by users outside Twister.
| - xg24_rb4187c | ||
| - xg27_dk2602a | ||
| - bg29_rb4420a | ||
| - xg29_rb4412a |
There was a problem hiding this comment.
What is the value of building/testing the exact same IP/driver with the same configuration on multiple targets!??
There was a problem hiding this comment.
same answer as for the samples question. And moreover, the IP can have little differences between two boards.
There was a problem hiding this comment.
Note there are several revision of the IP (4 revisions I remember right). I agree it may not make sense to maintain one sample instance for each board. But I believe we need to cover all the boards in the tests (especially if you consider some CI are required to run these tests for the compliance with quality standards)
Don't misunderstand me, I agree with you. I would like to see the tests work out of the box without all the .overlay
There was a problem hiding this comment.
Note there are several revision of the IP (4 revisions I remember right).
When I say IP, I mean compatible, if there is a different revision, it should have its own compatible.
I agree it may not make sense to maintain one sample instance for each board. But I believe we need to cover all the boards in the tests (especially if you consider some CI are required to run these tests for the compliance with quality standards)
With this test, we are covering the ADC driver, and the Zephyr ADC API layer. Boards' coverage is done with tests that do not need any special overlay/conf, like kernel tests.
Don't misunderstand me, I agree with you. I would like to see the tests work out of the box without all the .overlay
No, that's my point, that should be the case for kernel tests for ex. but not drivers, an ADC test is no supposed to run on any board out-of-the-box, makes no sense, we should only care about targets that are intentionally selected to cover a certain ADC IP/driver.
|
@JarmouniA the points you raise about |
Also, it does not make sense for downstream on-HW CI to test the same IP/driver with the same testcases on a ton of targets. This is for tests, samples are completely different, they are demos, not supposed to be built for every single target in CI, and any added overlay/conf files should provide the user with extra info about configuring a certain IP/driver/subsystem... not already provided in another overlay/conf, otherwise we will end up with 8xx overlay/conf files in every sample. Anyway, my comments above are non-blocking, but this will have to be cleaned up at some point. |
I'm familiar with that. I still think it's rather clumsy to have to manually override each individual platform, instead of being able to use something like the It's also worth noting the definition of "Do not use this option to limit testing or building in CI due to time or resource constraints, this option should only be used if the test or sample can only be run on the allowed platform and nothing else."
Even though the IP is the same, details such as device tree properties may be different (e.g. base register address) and have potential bugs. So even though all tests may be overkill I don't agree that there shouldn't be any testing at all of the same IP on multiple different targets. |
This commit updates the adc_raw_to_millivolts function to use 64-bit arithmetic. It is necessary when the intermediate result is higher than the maximum value of uint32_t. It also adds a range check to ensure the conversion result does not exceed the limits of int32_t, providing an assertion message for out-of-range values. Signed-off-by: Martin Hoff <[email protected]>
Martinhoff-maker
force-pushed
the
adc_series_2
branch
from
bc0d950 to
3dced68
Compare
|
V5:
|
|
This commit introduces a new driver for the silabs Incremental ADC (IADC). Signed-off-by: Martin Hoff <[email protected]>
Update compatibility from "silabs,gecko-iadc" to "silabs,iadc". It allows to use the new driver which are more custom to series 2 boards. Signed-off-by: Martin Hoff <[email protected]>
The goal to add the zephyr,user io-channels is to be able to run the ADC samples without having an overlays for each boards. Signed-off-by: Martin Hoff <[email protected]>
This commit introduces a new Kconfig option for oversampling in the ADC sequence sample. It allows to test oversampling for IADC sequences. Signed-off-by: Martin Hoff <[email protected]>
This commit updates the ADC sequence sample to improve handling of differential mode readings. It modifies the conversion logic to correctly interpret 16-bit/32-bit signed values and adjusts the resolution accordingly. This ensures accurate millivolt conversion for both differential and single-ended configurations. Signed-off-by: Martin Hoff <[email protected]>
This commit introduces/updates overlays for multiple silabs boards for ADC samples and ADC tests. Signed-off-by: Martin Hoff <[email protected]>
This commit introduces DMA support for the Silabs IADC driver. A new Kconfig option is added to enable DMA support, ensuring compatibility with the existing ADC configuration. DMA can be used for synch/asynch operation. Signed-off-by: Martin Hoff <[email protected]>
This commit introduces a new configuration file for DMA support specific to the Silabs IADC, along with updates to the test case YAML and device tree overlay for the xg29_rb4412a board. The changes ensure that the ADC driver can utilize DMA operations effectively. Signed-off-by: Martin Hoff <[email protected]>
Martinhoff-maker
force-pushed
the
adc_series_2
branch
from
3dced68 to
a279105
Compare
Different revisions of an IP should have different compatibles, see STM32 ADC for ex. |
This pull request introduces a new Silicon Labs IADC (Incremental ADC) driver for Series 2 SoCs, replaces the old device tree compatible string, and adds support for DMA-based ADC conversions. It also updates board overlays, device tree bindings, and test configurations to support and validate the new driver on multiple series 2 SoCs.
Tested all these tests and samples: adc_dt, adc_sequence, adc_api (normal, with DMA, with DMA async), adc_accuracy, adc_error_case
on all these boards:
Also tested the adc_sequence sample with PM over xg29_rb4412 and we got an average current consumption of 13 uA.
A few ideas for improvement:
FYI:
The resolution property for an ADC channel node is not mandatory. When it is not present, we set the property to 0 in the channel struct.
I initially used this "0" value to let the driver set the default resolution value. However, in the adc_error_case test case, a resolution of "0" is considered an error.
Shouldn't we set the resolution as a mandatory property since 0 is not a valid resolution value?