drivers/periph: reworked the ADC driver by haukepetersen · Pull Request #4430 · RIOT-OS/RIOT

haukepetersen · 2015-12-07T20:48:21Z

This is a proposal for a reworked and simplified ADC driver interface, introducing the concept of logical ADC lines. This should separate the former used logical channels and the CPU internal channels now clearly from each other, leading to less confusion when implementing this interface.

This remodeled interface has exactly the same feature set than the former interface, so only support for non-sequential, one-shot conversions for now.

Enhancements are:

plain ADC lines, no dev/chan combinations anymore
resolution is now specified for each conversion
removed the power_xx functions from interface (made them implicit now)
better documentation...

A sample implementation for the stm32f4(discovery) is attached, also the test application is already adapted.

DipSwitch · 2015-12-08T14:32:58Z

cpu/stm32f4/periph/adc.c

Please note that if you disable the ADC clock you will reset it's configuration to reset state. This means the ADC will be unconfigured next time you call adc_sample. Don't know if that was intended?

are you sure about this? As far as I know does the configuration stay if you disable the clock

Maybe you are right and am I confused with the RCC RST register.

DipSwitch · 2015-12-08T15:09:15Z

Clean solution :) This would also work for the L1, F0, F1 and maybe the F3 (don't have that one)

haukepetersen · 2015-12-09T14:48:33Z

backported the gpio_init_analog from #4431.

@DipSwitch, @PeterKietzmann: are you in line with these changes?

DipSwitch · 2015-12-09T15:01:05Z

ACK

PeterKietzmann · 2015-12-09T15:40:55Z

cpu/stm32f4/periph/adc.c

Just asking: Wasn't there also APB1ENR for some ports or pins?

just re-checked: all F4s supported by RIOT currently have the ADC periphs connected to the APB1 bus

For the F(1|3)'s this is also the case, and probably also for the F0 and L series it's some sort of a business model :p

PeterKietzmann · 2015-12-09T15:47:27Z

@haukepetersen the change looks reasonable and "similar" to other periph driver optimizations. I did not review or test in detail right now.

DipSwitch · 2015-12-13T03:28:47Z

cpu/stm32f4/periph/adc.c

Maybe you would also like to calculate the maximum allowed clock speed compile time.

/* calculate the maximum allowed ADC clock speed (12Mhz) */ uint32_t devider = 2; for (; devider < 8; devider += 2) { if ((CLOCK_CORECLOCK / devider) <= 12000000) { break; } } devider = ((devider / 2) - 1) << 16; /* set clock prescaler */ RCC->CCR &= ~(ADC_CCR_ADCPRE); RCC->CCR |= devider;

Hm, I would rather configure the preferred divider in the periph_conf.h to save code size

Since CLOCK_CORECLOCK is a constant, the compiler optimized the for loop away and replaces it with a constant ;)

sure about that? Recently I started to doubt the compile a little bit when doing some code size analyses... But there is only one way to find out - I will give this a try. If optimization works as expected, this patch would definitely make sense

Jep, you were right, added this to the code.

DipSwitch · 2015-12-13T03:45:24Z

gpio_init_analog() is never called.
Maybe it would be nice to automatically add channel 16, 17 and 18 to the line configuration:

Temperature
Internal Vref
Battery Charge

To enable 16 and 17 you need to set ADC_CCR_TSVREFE to ADC->CCR and for channel 18 you need to set ADC_CCR_VBATE to ADC->CCR

Maybe increase the default sample time a bit to reduce measurement noise. ADC->SMPR[12]

DipSwitch · 2015-12-13T04:05:38Z

cpu/stm32f4/periph/adc.c

/* check if the linenel is valid */ if (line >= ADC_NUMOF) { return -1; }

nope. The paradigm for the periph drivers is: check the device for correctnes in the init function and assume it to be correct in all other functions. This saves quite some instructions and memory...

haukepetersen · 2015-12-14T14:32:15Z

About the internal ADC values and sample time: Let's discuss this in another PR...

haukepetersen · 2016-03-14T12:06:40Z

squashed

DipSwitch · 2016-03-14T12:09:50Z

Sorry missed that, killed the previous travis build (which is still building) and make it build the new commit :)

DipSwitch · 2016-03-14T12:12:40Z

Nope, there are still some s MSG commits, I think you want to squash those as well?

haukepetersen · 2016-03-14T12:14:10Z

I can't see them, did you update your view?

DipSwitch · 2016-03-14T12:16:06Z

I did, but probably there was a delay in github, they are gone now and travis builds the latest version. ^_^

DipSwitch · 2016-03-14T12:46:50Z

Murdock agree's, do we wait for Travis?

haukepetersen · 2016-03-14T13:35:45Z

As decided this morning -> if Murdock is good, go...

drivers/periph: reworked the ADC driver

miri64 · 2016-03-14T13:46:28Z

(I took @DipSwitch's comment as implicit ACK if not, I ack'd it implicitly by merging.)

haukepetersen assigned DipSwitch Dec 7, 2015

haukepetersen added this to the Release 2016.03 milestone Dec 7, 2015

This was referenced Dec 7, 2015

cpu/stm32f1/periph/adc: ADC implementation for STM32F1 #4221

Merged

drivers/periph: updated DAC driver interface #4431

Merged

DipSwitch reviewed Dec 8, 2015
View reviewed changes

haukepetersen force-pushed the opt_periph_adc branch from 6909026 to 5cc8b94 Compare December 9, 2015 14:46

PeterKietzmann reviewed Dec 9, 2015
View reviewed changes

DipSwitch reviewed Dec 13, 2015
View reviewed changes

DipSwitch added a commit to DipSwitch/RIOT that referenced this pull request Dec 13, 2015

update to interface proposal RIOT-OS#4430

7088f0b

haukepetersen mentioned this pull request Dec 15, 2015

drivers/periph: enhance low-level peripheral driver doc #4479

Closed

OlegHahm unassigned DipSwitch Dec 15, 2015

jnohlgard mentioned this pull request Jan 14, 2016

drivers: Optimized periph PWM interfaces [adopted] #4638

Merged

PeterKietzmann mentioned this pull request Jan 26, 2016

SAMR21 [ADC] #4162

Closed

11 tasks

miri64 assigned DipSwitch Jan 26, 2016

haukepetersen added 15 commits March 14, 2016 13:06

boards/frdm-k64f: updated ADC configuration

4f157c6

boards/pba-d-01: updated ADC configuration

7dc8aa5

cpu/k64f: moved include of mcg.h to cpu.c

58e2b93

cpu/kw2x: moved include of mcg.h to cpu.c

56c8e49

drivers/mq3: adapted to ADC driver changes

9bcfec9

tests/driver_mq3: adapted to ADC driver changes

c46d8ee

tests/periph_dac: adapted to ADC changes

1c03639

boards/f4vi1: added dummy ADC config

87f8772

boards/nrf51dongle: added dummy ADC config

7dd4c53

boards/nrf6310: added dummy ADC config

164ae7e

boards/nucleo-f091: added dummy ADC config

7a76396

boards/nucleo-f401: added dummy ADC config

838683d

boards/pca10000: added dummy ADC config

b122803

baords/ek-lm4f120xl: adapted ADC configuration

6558016

cpu/lm4f120: reworked ADC implementation

432f033

haukepetersen force-pushed the opt_periph_adc branch from 445ec7d to 432f033 Compare March 14, 2016 12:06

haukepetersen added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 14, 2016

DipSwitch mentioned this pull request Mar 14, 2016

cpu/lm4f120: ADC support #4978

Merged

miri64 added a commit that referenced this pull request Mar 14, 2016

Merge pull request #4430 from haukepetersen/opt_periph_adc

d66625b

drivers/periph: reworked the ADC driver

miri64 merged commit d66625b into RIOT-OS:master Mar 14, 2016

haukepetersen deleted the opt_periph_adc branch March 14, 2016 13:54

Conversation

haukepetersen commented Dec 7, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DipSwitch commented Dec 8, 2015

Uh oh!

haukepetersen commented Dec 9, 2015

Uh oh!

DipSwitch commented Dec 9, 2015 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterKietzmann commented Dec 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DipSwitch commented Dec 13, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Dec 14, 2015

Uh oh!

haukepetersen commented Mar 14, 2016

Uh oh!

DipSwitch commented Mar 14, 2016

Uh oh!

DipSwitch commented Mar 14, 2016

Uh oh!

haukepetersen commented Mar 14, 2016

Uh oh!

DipSwitch commented Mar 14, 2016

Uh oh!

DipSwitch commented Mar 14, 2016

Uh oh!

haukepetersen commented Mar 14, 2016

Uh oh!

miri64 commented Mar 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants