Skip to content
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

RFC: add support for ADC #51

Merged
merged 4 commits into from
Mar 28, 2019
Merged

RFC: add support for ADC #51

merged 4 commits into from
Mar 28, 2019

Conversation

geomatsi
Copy link
Contributor

Summary

This PR adds the following bits:

  • implement ADC clock configuration in rcc module
  • implement ADC support, including multiple ADC blocks
  • add ADC example

This PR is based on #19 by @TheZoq2 . As per request from @therealprof , it has been reworked following stm32f0x ADC implementation. Besides, some implementation details follow stm32f4x ADC implementation.

Limitations

  1. Basic one-shot support only: similar to what we have in stm32f0x, but not in stm32f4xx
  2. No dual ADC mode support when 2 ADC blocks are available
  3. No fool proof against using the same channel in two ADC blocks.

Implementation notes

  1. Split of ADC implementation to simplify review
    To simplify review, ADC implementation is split into the following two parts:
    • 2nd commit: implement one-shot support for ADC1 only
    • 3rd commit: use rust macros to support multiple ADC blocks (including gpio channel assignment) w/o functional changes to ADC handling code
  2. Hardware support
    Current implementation enables the following ADC blocks:
    • the only available ADC1 for both stm32f100 and stm32f101
    • both ADC1 and ADC2 available on medium-density stm32f103 chips
  3. Support for a more capable stm32f10x chips
    Note that XL-density stm32f103 chips support ADC3 as well. Besides, gpio-to-channel mapping is different on those chips. Meanwhile it can be also embraced by suggested implementation. For instance, something like the following should work:
#[cfg(feature = "stm32f103XL")]
adc_pins!(ADC1,
    gpioa::PA0<Analog> => 0_u8,
    gpioa::PA1<Analog> => 1_u8,
    gpioa::PA2<Analog> => 2_u8,
    gpioa::PA3<Analog> => 3_u8,
    gpioa::PA4<Analog> => 4_u8,
    gpioa::PA5<Analog> => 5_u8,
    gpioa::PA6<Analog> => 6_u8,
    gpioa::PA7<Analog> => 7_u8,
    gpiob::PB0<Analog> => 8_u8,
    gpiob::PB1<Analog> => 9_u8,
    gpioc::PC0<Analog> => 10_u8,
    gpioc::PC1<Analog> => 11_u8,
    gpioc::PC2<Analog> => 12_u8,
    gpioc::PC3<Analog> => 13_u8,
    gpioc::PC4<Analog> => 14_u8,
    gpioc::PC5<Analog> => 15_u8,
);

#[cfg(feature = "stm32f103XL")]
adc_pins!(ADC3,
    gpioa::PA0<Analog> => 0_u8,
    gpioa::PA1<Analog> => 1_u8,
    gpioa::PA2<Analog> => 2_u8,
    gpioa::PA3<Analog> => 3_u8,
    gpioa::PA4<Analog> => 4_u8,
    gpioa::PA5<Analog> => 5_u8,
    gpioa::PA6<Analog> => 6_u8,
    gpioa::PA7<Analog> => 7_u8,
    gpiob::PB0<Analog> => 8_u8,
    gpiob::PB1<Analog> => 9_u8,
    gpioc::PC0<Analog> => 10_u8,
    gpioc::PC1<Analog> => 11_u8,
    gpioc::PC2<Analog> => 12_u8,
    gpioc::PC3<Analog> => 13_u8,
    gpioc::PC4<Analog> => 14_u8,
    gpioc::PC5<Analog> => 15_u8,
);

#[cfg(feature = "stm32f103XL")]
adc_pins!(ADC3,
    gpioa::PA0<Analog> => 0_u8,
    gpioa::PA1<Analog> => 1_u8,
    gpioa::PA2<Analog> => 2_u8,
    gpioa::PA3<Analog> => 3_u8,
    gpiof::PF6<Analog> => 4_u8,
    gpiof::PF7<Analog> => 5_u8,
    gpiof::PF8<Analog> => 6_u8,
    gpiof::PF9<Analog> => 7_u8,
    gpiof::PF10<Analog> => 8_u8,
    gpioc::PC0<Analog> => 10_u8,
    gpioc::PC1<Analog> => 11_u8,
    gpioc::PC2<Analog> => 12_u8,
    gpioc::PC3<Analog> => 13_u8,
);

#[cfg(feature = "stm32f103XL")]
adc_hal! {
    ADC1: (
        adc1,
        adc1en,
        adc1rst
    ),
    ADC2: (
        adc2,
        adc2en,
        adc2rst
    ),
    ADC3: (
        adc3,
        adc3en,
        adc3rst
    ),
}

Copy link
Member

@TheZoq2 TheZoq2 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, most of it looks great!

I added some comments that are mostly nitpicks but would be nice if you could address :)

src/adc.rs Outdated Show resolved Hide resolved
src/adc.rs Show resolved Hide resolved
src/rcc.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
examples/adc.rs Show resolved Hide resolved
@geomatsi
Copy link
Contributor Author

Thanks for review, I will address comments and re-submit. Let me know if you are OK with multiple force-push updates for the same PR.

@therealprof
Copy link
Member

@geomatsi Feel free to force-push away.

Default value is the slowest possible ADC clock PCLK2 / 8. This is to make sure
that 14MHz threshold is not exceeded. Meanwhile ADC clock is configurable, so its
frequency may be tweaked to meet certain practical needs, e.g. speed vs power
saving choice. User specified value is approximated using supported prescaler
values 2/4/6/8. Invalid specified frequency (e.g. too high) will result in
the slowest possible ADC clock PCLK2 / 8.

Example:
  let p = stm32::Peripherals::take().unwrap();
  let mut flash = p.FLASH.constrain();
  let mut rcc = p.RCC.constrain();
  let clocks = rcc.cfgr.adcclk(2.mhz()).freeze(&mut flash.acr);

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Implement adc embedded_hal traits Channel and OneShot
for ADC1 on stm32f10x chips.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Some stm32f10x chips support multiple ADC blocks. For instance,
stm32f103 medium-density chips has two ADC blocks. This commit
makes use of rust macros to provide framework that supports
description of multiple ADC blocks.

The following ADC blocks are enabled for now:
- stm32f100: ADC1
- stm32f101: ADC1
- stm32f103: ADC1, ADC2

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Add ADC example that covers the following functionality:
- ADC clock configuration
- making use of the only available ADC block on stm32f10[01]
- making use of both available ADC blocks on stm32f103

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
@geomatsi
Copy link
Contributor Author

Hi all,

This PR has been updated. Here is a brief summary of changes.
v1 -> v2: introduce changes according to @TheZoq2 comments

  • ADC1 is available on all the pins, so remove cfg(any(..)) for it
  • fix comment in rcc.rs: adc frequency, not core frequency
  • use allow(non_camel_case_types) only for AdcSampleTime enum, do not make it global
  • add detailed comment to adc example: why and how to configure adc clocks
  • update adc clocks commit message: why and how to configure adc clocks

Regards,
Sergey

@geomatsi
Copy link
Contributor Author

Hi @therealprof , @TheZoq2

Ok, then lets start with blocking version and merge current solution.

I will experiment with multiple channel readings while working on vacuum cleaner firmware. So I will check out non-blocking implementations including #19 as well as mentioned in your comments.

Regards,
Sergey

Copy link
Member

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM

@TheZoq2
Copy link
Member

TheZoq2 commented Mar 28, 2019

Looks good to me as well, thanks!

@TheZoq2 TheZoq2 merged commit 3b78e6b into stm32-rs:master Mar 28, 2019
@geomatsi geomatsi deleted the adc-wip branch March 28, 2019 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants