Skip to content

Conversation

@gmarull
Copy link
Member

@gmarull gmarull commented Apr 29, 2020

  • Add support for ADC on H7 series. Note that ADC1 and ADC2 share the same register set, so it is added as "adc1_2"
  • Enables ADC on Nucleo H743ZI board (ADC12 channel 15).

Performed quick tests on Nucleo H743ZI board on ADC12 CH15 and ADC3 CH6 using a signal generator as an analog test signal.

NOTE: I think that current STM32 ADC driver needs a rework, as it's in a kind of "ifdef hell" state.

@gmarull gmarull requested review from FRASTM and erwango April 29, 2020 11:10
@gmarull gmarull added area: ADC Analog-to-Digital Converter (ADC) platform: STM32 ST Micro STM32 labels Apr 29, 2020
@zephyrbot zephyrbot added area: Devicetree area: Boards area: Tests Issues related to a particular existing or missing test labels Apr 29, 2020
@erwango
Copy link
Member

erwango commented Apr 30, 2020

^^@ABOSTM

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Regarding the ADC1/2 common node, impact I have in mind in having one node for both instances is that you can't invoke them separately in an application. Looking to reference manual, it seems though that ADC1 and ADC2 doesn't share all resources and ADC1 could be used with ADC2 in a master/slave mode. Running these instances in parallel is not possible today due to driver limitation, so you're not breaking anything. Also I'm not enough familiar with ADC to say if this mode is really something that is required in a near future.

So this is fine for now, but maybe we'll have to come back on this later.

Anyway, please use ADC1_2 for disambiguation.

@gmarull
Copy link
Member Author

gmarull commented Apr 30, 2020

@erwango I'll adjust node label to ADC1_2. Regarding adc12 vs. adc1_2 I'm fine with both approaches, so just let me know. I think it will be difficult to add support for ADC1/2 Master/Slave mode in a general purpose driver, I think it is designed for some specific applications (e.g. motor control where you want to sample phases in synchronization). Some details: https://www.st.com/resource/en/application_note/cd00258017-stm32s-adc-modes-and-their-applications-stmicroelectronics.pdf

@gmarull gmarull requested a review from erwango April 30, 2020 11:14
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Let's go with adc1_2

@gmarull gmarull requested a review from erwango April 30, 2020 17:10
@zephyrbot
Copy link

zephyrbot commented May 3, 2020

All checks passed.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@gmarull
Copy link
Member Author

gmarull commented May 3, 2020

@erwango updated number of channels in the driver to 20, as H7 has 20 valid external channels (F7 or F4 have 19, so the number was actually wrong).

@stephanosio
Copy link
Member

-:70: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#70: FILE: drivers/adc/adc_stm32.c:7:

  • SPDX-License-Identifier: Apache-2.0

Please ignore these warnings (see #24921)

@galak
Copy link
Contributor

galak commented May 5, 2020

can you rebase to resolve merge conflict.

@gmarull
Copy link
Member Author

gmarull commented May 5, 2020

@galak done

gmarull added 2 commits May 6, 2020 12:34
Add support for ADC on H7 series. Note that ADC1 and ADC2 share the same
register set, so it is added as "adc1_2".

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Enable ADC on Nucleo H743ZI board (ADC12 channel 15).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@galak galak merged commit 5c454e9 into zephyrproject-rtos:master May 8, 2020
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 area: Devicetree area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants