Skip to content

tests: drivers: adc: adc_dma: increase sampling interval for MCUX ADC16 #58547

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

Merged
merged 1 commit into from
Jun 2, 2023
Merged

tests: drivers: adc: adc_dma: increase sampling interval for MCUX ADC16 #58547

merged 1 commit into from
Jun 2, 2023

Conversation

danieldegrasse
Copy link
Collaborator

@danieldegrasse danieldegrasse commented May 31, 2023

tests: drivers: adc: adc_dma: increase sampling interval for MCUX ADC16

Increase hardware sampling interval to 30ms for MCUX ADC16. The ADC
callback in the repeated_samplings test takes roughly 27ms to execute,
so a sampling frequency of this interval still allows the ADC test to
verify the ADC is being sampled at a given interval, while avoiding the
pitfalls that result from a kernel timer shorter than the duration the
ADC callback takes to run.

Fixes #58467

@danieldegrasse danieldegrasse requested a review from nashif as a code owner May 31, 2023 21:23
@zephyrbot zephyrbot added the area: ADC Analog-to-Digital Converter (ADC) label May 31, 2023
@zephyrbot zephyrbot requested a review from anangl May 31, 2023 21:23
@danieldegrasse
Copy link
Collaborator Author

@heinwessels I'd like to understand the changes in #52965 a bit better- particularly the change to use SAMPLE_INTERVAL_US here: https://github.com/zephyrproject-rtos/zephyr/pull/56104/files#diff-0f2bf1897d5421331070f482911ef31bdf0fe065480deeeff59f0065c5ed6554R294. Using SAMPLE_INTERVAL_US, particularly with an interval shorter than (1/CONFIG_SYS_CLOCK_TICKS_PER_SEC), will result in a kernel timer that fires sporadically.

Moreover, on the FRDM-K64F the repeated_samplings_callback in the ADC test takes ~27ms to execute. So for any kernel timer interval less than 27ms, the timer may fire before the callback completes. This will result in the EBUSY error for the adc_read call, as the ctx->sampling_requested variable the ADC timer expiration handler checks against is not decremented until the user's ADC callback completes.

However, if the sampling interval is set to 0 (as you did in your initial PR #52965, the ADC context subsystem will correctly attempt to sample as soon as the ADC driver indicates a previous sample has completed, and will not start a kernel timer.

Is there a particular reason you changed the interval back to SAMPLE_INTERVAL_US?

@danieldegrasse danieldegrasse changed the title tests: drivers: adc: adc_dma: increase sampling interval for mcux ADC16 tests: drivers: adc: adc_dma: remove sampling interval for MCUX ADC16 May 31, 2023
@heinwessels
Copy link
Contributor

@heinwessels I'd like to understand the changes in #52965 a bit better- particularly the change to use SAMPLE_INTERVAL_US here: https://github.com/zephyrproject-rtos/zephyr/pull/56104/files#diff-0f2bf1897d5421331070f482911ef31bdf0fe065480deeeff59f0065c5ed6554R294. Using SAMPLE_INTERVAL_US, particularly with an interval shorter than (1/CONFIG_SYS_CLOCK_TICKS_PER_SEC), will result in a kernel timer that fires sporadically.

Moreover, on the FRDM-K64F the repeated_samplings_callback in the ADC test takes ~27ms to execute. So for any kernel timer interval less than 27ms, the timer may fire before the callback completes. This will result in the EBUSY error for the adc_read call, as the ctx->sampling_requested variable the ADC timer expiration handler checks against is not decremented until the user's ADC callback completes.

However, if the sampling interval is set to 0 (as you did in your initial PR #52965, the ADC context subsystem will correctly attempt to sample as soon as the ADC driver indicates a previous sample has completed, and will not start a kernel timer.

Is there a particular reason you changed the interval back to SAMPLE_INTERVAL_US?

As mentioned in the commit message, I changed it back due to #56070. The reason is that I will simply reverting things back to how I found it, because it apparently caused issues. I don't work with NXP, nor have devices available, so I'm not really sure which the right approach is. Even though I do believe the sampling_interval_us should be 0, as this PR is doing.

Increase hardware sampling interval to 30ms for MCUX ADC16. The ADC
callback in the repeated_samplings test takes roughly 27ms to execute,
so a sampling frequency of this interval still allows the ADC test to
verify the ADC is being sampled at a given interval, while avoiding the
pitfalls that result from a kernel timer shorter than the duration the
ADC callback takes to run.

Fixes #58467

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse danieldegrasse changed the title tests: drivers: adc: adc_dma: remove sampling interval for MCUX ADC16 tests: drivers: adc: adc_dma: increase sampling interval for MCUX ADC16 Jun 2, 2023
@danieldegrasse
Copy link
Collaborator Author

After discussing with @hakehuang, I have increased the sampling interval to 30ms. This way the test still verifies that the ADC hardware sampling is triggered on the timer interval, but we do not run into the pitfall of using a kernel timer shorter in duration than the time the ADC callback takes to execute (27ms on the FRDM-K64F)

@danieldegrasse danieldegrasse requested a review from hakehuang June 2, 2023 14:58
@decsny decsny self-requested a review June 2, 2023 16:28
@nashif nashif merged commit d5792ab into zephyrproject-rtos:main Jun 2, 2023
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI:adc_dma:frdm_k64f: adc_read return EBUSY in repeat_sample test
6 participants