Skip to content

Conversation

Martinhoff-maker
Copy link
Member

@Martinhoff-maker Martinhoff-maker commented Aug 8, 2025

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:

  • slwrb4180a (xg21 SoC)
  • sltb010a (xg22 SoC)
  • xg23_rb4210a (xg23 SoC)
  • xg24_rb4187c (xg24 SoC)
  • xg27_dk2602a (xg27 SoC)
  • xg29_rb4412a (xg29 SoC)

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:

  • Implement runtime calibration (described in Silicon Labs reference manual)
  • Implement internal IADC timer instead of using the kernel timer
  • Add more possibilities for the acquisition time (which is different from interval between samplings)

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?

@Martinhoff-maker Martinhoff-maker force-pushed the adc_series_2 branch 2 times, most recently from 65ca9a2 to ed1888b Compare August 8, 2025 13:03
Copy link

sonarqubecloud bot commented Aug 8, 2025

@Martinhoff-maker Martinhoff-maker force-pushed the adc_series_2 branch 3 times, most recently from d9e92b0 to 5a91f78 Compare September 10, 2025 14:54
@Martinhoff-maker Martinhoff-maker marked this pull request as ready for review September 10, 2025 15:51
@zephyrbot zephyrbot added Release Notes To be mentioned in the release notes area: Samples Samples platform: Silabs Silicon Labs area: ADC Analog-to-Digital Converter (ADC) labels Sep 10, 2025
@Martinhoff-maker Martinhoff-maker force-pushed the adc_series_2 branch 4 times, most recently from 396684b to 474f69c Compare September 19, 2025 15:05
@jhedberg
Copy link
Member

jhedberg commented Oct 3, 2025

@JarmouniA the points you raise about platform_allow require broader discussion within the project. PR review comments don't give the issue the attention it deserves, IMO. This was also a topic that was raised at the recent TSC F2F meeting a month ago. I believe one reason why these keep getting added is downstream on-HW CI, since not having the platforms listed means you have to explicitly force twister to use a given platform for testing, rather than e.g. requesting it to "test on all Silabs platforms".

@JarmouniA
Copy link
Contributor

JarmouniA commented Oct 3, 2025

I believe one reason why these keep getting added is downstream on-HW CI, since not having the platforms listed means you have to explicitly force twister to use a given platform for testing, rather than e.g. requesting it to "test on all Silabs platforms".

Using --force-platform allows to override filtering caused by platform_allow, platform_exclude, arch_allow and arch_exclude keys in test configuration files.
https://docs.zephyrproject.org/latest/develop/test/twister.html#selecting-platform-scope

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.

@jhedberg
Copy link
Member

jhedberg commented Oct 3, 2025

Using --force-platform allows to override filtering caused by platform_allow, platform_exclude, arch_allow and arch_exclude keys in test configuration files. https://docs.zephyrproject.org/latest/develop/test/twister.html#selecting-platform-scope

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 --vendor VENDOR switch to cover all platforms for a given vendor.

It's also worth noting the definition of platform_allow in the above documentation, which is currently pretty clearly being violated in Zephyr's use of it:

"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."

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.

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.

@Martinhoff-maker
Copy link
Member Author

V5:

  • In order to be compliant with futur PR of @JarmouniA, remove all new sample overlays and instead introduce io-channels in some boards.dts
  • Still a little bit confused about how we handle tests overlays since we have "differences" between SoC even if it's the "same" IP as Johan said.

@JarmouniA
Copy link
Contributor

  • Still a little bit confused about how we handle tests overlays since we have "differences" between SoC even if it's the "same" IP as Johan said

Different revisions of an IP should have different compatibles, see STM32 ADC for ex.

JarmouniA
JarmouniA previously approved these changes Oct 3, 2025
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]>
This commit introduces a new driver for the silabs Incremental ADC (IADC).

Signed-off-by: Martin Hoff <[email protected]>
@Martinhoff-maker
Copy link
Member Author

Rebased for merge conflict with migration guide.

asmellby
asmellby previously approved these changes Oct 8, 2025
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
Copy link
Member Author

V6: Add the deprecation of the old IADC driver

depends on DT_HAS_SILABS_GECKO_IADC_ENABLED
select SOC_GECKO_IADC
select ADC_CONFIGURABLE_INPUTS
select DEPRECATED
Copy link
Contributor

Choose a reason for hiding this comment

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

Add [deprecated] in the prompt as well, and direct the user in the help to the driver that shall be used.
And the driver should be disabled by default.

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 think it could stay on enabled by default because if a user don't explicitly return to the old bindings, the symbol will not be selected.


adc0: adc@5a004000 {
compatible = "silabs,gecko-iadc";
compatible = "silabs,iadc";
Copy link
Contributor

@JarmouniA JarmouniA Oct 8, 2025

Choose a reason for hiding this comment

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

Since all instances of silabs,gecko-iadc have been replaced, shouldn't the new driver be enabled by default for nodes with silabs,gecko-iadc compatible (downstream) ?

Copy link
Member Author

@Martinhoff-maker Martinhoff-maker Oct 8, 2025

Choose a reason for hiding this comment

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

I want to get rid of the "gecko" naming so I want to have silabs,iadc, but yeah, no soc will use the old driver now. But i would like to let the possibility to the user to return back to the old driver for 1 release cycle before completely removed the old driver and compatible associated.

Copy link
Contributor

Choose a reason for hiding this comment

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

The driver can stay, it just will not be enabled by default, the newer one will instead for all nodes with new or old compatible.

Copy link
Member Author

Choose a reason for hiding this comment

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

So if I understand right, removed default=y in the old Kconfig and add:
depends on (DT_HAS_SILABS_IADC_ENABLED || DT_HAS_SILABS_GECKO_IADC_ENABLED) in the the new Kconfig ?
With that conf, the old binding will use the new driver but how can a user decide to go back to the old ? selecting the old Kconfig symbol will create a conflict. Or will the user needs to do:

CONFIG_ADC_SILABS_IADC=n
CONFIG_ADC_GECKO_IADC=y

and change the binding ?
It makes me thinking that I might prefer to add a binding than replace it like:
compatible = "silabs,iadc", "silabs,gecko-iadc";

Copy link
Contributor

Choose a reason for hiding this comment

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

So if I understand right, removed default=y in the old Kconfig and add: depends on (DT_HAS_SILABS_IADC_ENABLED || DT_HAS_SILABS_GECKO_IADC_ENABLED) in the the new Kconfig ?

Yes, but you will have to add

#undef DT_DRV_COMPAT
#define DT_DRV_COMPAT silabs_gecko_iadc
DT_INST_FOREACH_STATUS_OKAY(IADC_INIT) 

at the end of the new driver as well.

With that conf, the old binding will use the new driver but how can a user decide to go back to the old ? selecting the old Kconfig symbol will create a conflict. Or will the user needs to do:

CONFIG_ADC_SILABS_IADC=n
CONFIG_ADC_GECKO_IADC=y

Yes, and ADC_GECKO_IADC should depend on ADC_SILABS_IADC=n

and change the binding ?

If you mean the compatible in device node? If they have the new compatible alone, yes they will have to put the old one (either alone, or with the new one)

It makes me thinking that I might prefer to add a binding than replace it like: compatible = "silabs,iadc", "silabs,gecko-iadc";

That's also a great option, this way switching drivers can be done with just

CONFIG_ADC_SILABS_IADC=n
CONFIG_ADC_GECKO_IADC=y

Copy link

sonarqubecloud bot commented Oct 8, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) area: Boards/SoCs area: Devicetree Bindings area: Samples Samples area: Tests Issues related to a particular existing or missing test platform: Silabs Silicon Labs Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants