cpu/stm32: unified ADC driver implementations by haukepetersen · Pull Request #7277 · RIOT-OS/RIOT

haukepetersen · 2017-06-29T13:58:33Z

EDIT
~~This PR is still work in progress, just wanted to put it already out there to prevent multiple people working on this...~~

I have not test all of the STM CPUs, yet. ~~Especially I doubt the L1 and L4's to work, as it seems they have subtle differences in their ADC registers...~~

L1 works, L4 was not implemented before anyway...

aabadie · 2017-06-29T14:01:21Z

just wanted to put it already out there to prevent multiple people working on this...

good idea, I was thinking of this some times ago :)

aabadie · 2017-06-29T14:02:02Z

this already needs rebase btw...

haukepetersen · 2017-06-29T14:06:50Z

rebased

aabadie

Couldn't resist in having a look at this one. There 2 things in this PR:

unified ADC
common configuration for some nucleo boards (as suggested in a comment that I couldn't find)

I would prefer to provide the second point in dedicated PRs (even if I like the general idea).

aabadie · 2017-06-29T14:08:35Z

boards/msbiot/include/periph_conf.h

-    {GPIO_PIN(PORT_B, 1), 0, 9}  \
-}
+static const adc_conf_t adc_config[] = {
+    { .pin = GPIO_PIN(PORT_B, 0), .dev = ADC_1, .chan =  8 },


extra space before 8 (and 9 the line below)

there is still this one

aabadie · 2017-06-29T14:14:39Z

boards/nucleo32-common/include/periph_conf_nucleo32.h

+    { .pin = GPIO_PIN(PORT_A, 5), .dev = ADC_1, .chan = 5 },
+    { .pin = GPIO_PIN(PORT_A, 6), .dev = ADC_1, .chan = 6 },
+    { .pin = GPIO_PIN(PORT_A, 7), .dev = ADC_1, .chan = 7 },
+    { .pin = GPIO_PIN(PORT_A, 2), .dev = ADC_1, .chan = 2 }


Maybe move this line at third position

nope, the way I put them here is so that the Ardunio Ax lines match RIOTs ADC_LINE(x) configuration. ST simply mapped their ADCs like this...

aabadie · 2017-06-29T14:15:44Z

boards/nucleo32-f031/include/periph_conf.h

- * @name   ADC configuration
- * @{
- */
-#define ADC_CONFIG {            \


This seems very different from the common part.

I don't understand, what do you mean with common part?

I guess @aabadie means, currently there are 5 ADC lines defined here, but in your common definition in periph_conf_nucleo32.h above you define 8 ADC lines.

yes, but as I see it, the existing config was incomplete and this PR fixes that...

might be related to the fact that SPI1 and some PWM channels are on some of the ADC lines, too. But even the original ADC config does not ignore those pins and would hence collide with those, too.

haukepetersen · 2017-07-10T11:01:16Z

I would prefer to provide the second point in dedicated PRs (even if I like the general idea).

I don't like to make this a separate PR -> IMHO it is good practice to start the config centralization in a concrete PR (as in this one), so we have already some 'meat' to put in the common files and it is clear how the centralization is intended to work...

smlng · 2017-07-19T09:40:50Z

boards/opencm904/include/periph_conf.h

-/**
- * @name ADC configuration
- * @{
- */


nope, related: the opencm904 uses a stm cpu and the reworked adc driver does not requite 'empty' numof defs anymore... So related cleanup.

smlng · 2017-07-19T09:41:09Z

boards/spark-core/include/periph_conf.h

-/**
- * @name ADC configuration
- * @{
- */


dito, seems unrelated

smlng · 2017-07-19T15:07:44Z

@haukepetersen what's your ETA on this one? Further, I think L1-L4 should be adaptable to this, too - maybe a bit more ifdef, but as there is already that much ...

haukepetersen · 2017-07-20T09:24:42Z

ETA: as soon as possible, but not before beginning of August, as there is the Dagstuhl workshop next week...

haukepetersen · 2017-08-15T17:01:12Z

So, finally got to this PR again - sorry for the delay.

The PR should now be ready for review/testing. All boards that had the ADC feature before should still work. Just tested with nucleo-f072, nucleo-f411, and nucleo-l152.

ADC drivers are missing for stm32l0, stm32l4, and stm32f7, but these never existed so far and merging this into the unified driver is work for another day...

haukepetersen · 2017-08-29T13:37:50Z

Picked up work on this again, update will follow soon

aabadie · 2017-11-06T16:37:27Z

cpu/stm32_common/periph/adc.c

+#endif
+
+/* find out which bus the ADC is connected to */
+#if defined()


Looks like there are things missing here

haukepetersen · 2017-11-08T12:40:17Z

yes, its still work in progress. Run into trouble with some STM CPUs and never found the time to continue debugging... But I havn't forgotten about this PR :-)

aabadie · 2017-11-08T14:13:05Z

#7815 has been merged so maybe this PR could be adapted with it ?

haukepetersen · 2017-11-08T17:00:20Z

addressed comment
rebased

aabadie · 2017-11-09T09:55:49Z

boards/nucleo144-f207/include/periph_conf.h

+    { .pin = GPIO_PIN(PORT_A,  3), .dev = ADC_1, .chan =  3 },
+    { .pin = GPIO_PIN(PORT_C,  0), .dev = ADC_1, .chan = 10 },
+    { .pin = GPIO_PIN(PORT_C,  3), .dev = ADC_1, .chan = 13 },
+    { .pin = GPIO_PIN(PORT_F,  3), .dev = ADC_3, .chan =  9 },


The build fails for me because of the use of ADC_3. Even if it's compliant with the board datasheet.

aabadie · 2017-11-09T09:57:15Z

cpu/stm32_common/include/periph_cpu_common.h

+#endif
+#if defined(CPU_MODEL_STM32F407VG) || defined(CPU_MODEL_STM32F415RG) \
+    || defined(CPU_MODEL_STM32F446RE) || defined(CPU_MODEL_STM32F429ZI)
+    ADC_2 = 1,          /**< ADC2 */


ADC_3 is not defined for CPU_FAM_STM32F2 but is used in nucleo144-f207: the build fails. I tried to enable it for f207 but the application hangs when initializing ADC3...

aabadie · 2017-11-09T10:03:50Z

boards/nucleo144-f429/include/periph_conf.h

+    { .pin = GPIO_PIN(PORT_A,  3), .dev = ADC_1, .chan =  3 },
+    { .pin = GPIO_PIN(PORT_C,  0), .dev = ADC_1, .chan = 10 },
+    { .pin = GPIO_PIN(PORT_C,  3), .dev = ADC_1, .chan = 13 },
+    { .pin = GPIO_PIN(PORT_F,  3), .dev = ADC_1, .chan =  9 },


According to the board datasheet, the device should be ADC_3. I tried to change that but then the application hangs when initializing it (similar issue with f207).

haukepetersen · 2017-11-09T10:33:24Z

Hmm, seems like I did not compile-test my last fix. I don't have any of these boards here, so I couldnt test on hardware...

Will take a look again at the ADC3 situation, shouldn't be too hard to find :-)

stale · 2019-08-10T06:07:53Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

fjmolinas · 2021-11-18T08:30:08Z

@aabadie do you think this one is still relevant with the current stm32 structure?

aabadie · 2021-11-18T08:48:38Z

Having everything in a single adc.c file is not the way to go IMHO, we should take inspiration of what was done for i2c. The shared ADC config for some nucleo64 boards is something that could be kept.
In the current state, this PR is too much out-of-date, so better split-out parts of it in new PRs.

haukepetersen · 2021-11-19T07:59:27Z

I will not have the time to work on this any further... Please feel free to close/fork/take this PR or whatever might be useful of this :-)

Teufelchen1 · 2024-01-23T10:02:47Z

Closed as out-of-date.

haukepetersen added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 29, 2017

aabadie added this to the Release 2017.10 milestone Jun 29, 2017

aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jun 29, 2017

aabadie self-assigned this Jun 29, 2017

haukepetersen force-pushed the opt_stm32_adc branch from 00ce143 to 51e0ad4 Compare June 29, 2017 14:06

aabadie changed the title ~~cpu/stm32: unified ADC driver implementaions~~ cpu/stm32: unified ADC driver implementations Jun 29, 2017

aabadie requested changes Jun 29, 2017

View reviewed changes

smlng reviewed Jul 19, 2017

View reviewed changes

haukepetersen force-pushed the opt_stm32_adc branch from ce3c701 to 8456356 Compare August 15, 2017 16:58

haukepetersen added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Aug 15, 2017

aabadie mentioned this pull request Aug 16, 2017

cpu/stm32l0: add i2c channel 2 & 3 #7467

Merged

haukepetersen added Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 29, 2017

haukepetersen force-pushed the opt_stm32_adc branch from 8456356 to ac3597a Compare August 29, 2017 13:18

aabadie reviewed Nov 6, 2017

View reviewed changes

aabadie mentioned this pull request Nov 8, 2017

cpu/stm32l0 : implementation of ADC driver #7815

Merged

haukepetersen added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 8, 2017

haukepetersen added 4 commits November 8, 2017 17:59

fixup! nucleo144-f446: fixed ADC configuration

9e36029

fixup! nucleo144-f207: fixed ADC config again

0f1e7d1

fixup! random fixes

5c09eff

fixup! board/msbiot: fixed extra spaces

817d14e

haukepetersen force-pushed the opt_stm32_adc branch from 644dd46 to 817d14e Compare November 8, 2017 16:59

aabadie reviewed Nov 9, 2017

View reviewed changes

aabadie mentioned this pull request Nov 15, 2017

boards/nucleo: concept for vastly improving code re-usage [WIP] #8044

Closed

smlng modified the milestones: Release 2017.10, Release 2018.01 Nov 16, 2017

smlng modified the milestones: Release 2018.01, Release 2018.04 Jan 18, 2018

MichelRottleuthner mentioned this pull request Mar 20, 2018

cpu/stm32l4: add adc implementation #8804

Merged

kaspar030 removed this from the Release 2018.04 milestone Apr 11, 2018

kYc0o removed the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Aug 28, 2018

haukepetersen mentioned this pull request Dec 13, 2018

periph: API change of macros ADC_LINE and DAC_LINE #10504

Closed

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

haukepetersen added the State: don't stale State: Tell state-bot to ignore this issue label Aug 12, 2019

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 12, 2019

MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

Teufelchen1 closed this Jan 23, 2024

Conversation

haukepetersen commented Jun 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabadie commented Jun 29, 2017

Uh oh!

aabadie commented Jun 29, 2017

Uh oh!

haukepetersen commented Jun 29, 2017

Uh oh!

aabadie left a comment

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!

aabadie Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

haukepetersen commented Jul 10, 2017

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!

smlng commented Jul 19, 2017

Uh oh!

haukepetersen commented Jul 20, 2017

Uh oh!

haukepetersen commented Aug 15, 2017

Uh oh!

haukepetersen commented Aug 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Nov 8, 2017

Uh oh!

aabadie commented Nov 8, 2017

Uh oh!

haukepetersen commented Nov 8, 2017

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!

haukepetersen commented Nov 9, 2017

Uh oh!

stale bot commented Aug 10, 2019

Uh oh!

fjmolinas commented Nov 18, 2021

Uh oh!

aabadie commented Nov 18, 2021

Uh oh!

haukepetersen commented Nov 19, 2021

Uh oh!

Teufelchen1 commented Jan 23, 2024

Uh oh!

Reviewers

haukepetersen commented Jun 29, 2017 •

edited

Loading

aabadie Nov 8, 2017 •

edited

Loading