Skip to content

Overhaul of RCC, GPIO, Serial. Add SPI, Delay #2

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 5 commits into from
Jun 13, 2019

Conversation

richardeoin
Copy link
Member

  • Working support for RCC configuration, with example
  • Working support for SPI, with example
  • Working support for Serial, with example
  • Working support for Delay, with example
  • Add memory.x
  • Add rustfmt.toml
  • Added myself as an author

The RCC peripheral returns a "Core Clock Distribution and Reset"
structure, which represents that the core clock configuration is
frozen. However we retain the right to switch most PKSUs on the fly,
to fine-tune PLL frequencies, and to enable / reset peripherals.

For now, use a local build of the stm32h7 crate. This keeps us
up-to-date without Adam having to issue patch releases all the
time. Hopefully the stm32h7 crate will stabilise and can be reverted
soon. This is documented in the "hacking" section of the README.

Tested on STM32H743 Rev Y

stm32h7xx-hal PR #1

@richardeoin richardeoin force-pushed the gpio-rcc--serial-spi-delay branch from d67c8d6 to c96c2b9 Compare May 27, 2019 18:57
richardeoin referenced this pull request in richardeoin/stm32h7xx-hal May 27, 2019
* Working support for RCC configuration, with example
* Working support for SPI, with example
* Working support for Serial, with example
* Working support for Delay, with example
* Add memory.x
* Add rustfmt.toml
* Added myself as an author

The RCC peripheral returns a "Core Clock Distribution and Reset"
structure, which represents that the core clock configuration is
frozen. However we retain the right to switch most PKSUs on the fly,
to fine-tune PLL frequencies, and to enable / reset peripherals.

For now, use a local build of the stm32h7 crate. This keeps us
up-to-date without Adam having to issue patch releases all the
time. Hopefully this will stabilise and can be reverted soon. This is
documented in the "hacking" section of the README

Tested on STM32H743 Rev Y

stm32h7xx-hal PR #2

See PR #1 for context
richardeoin referenced this pull request in richardeoin/stm32h7xx-hal May 27, 2019
* Working support for RCC configuration, with example
* Working support for SPI, with example
* Working support for Serial, with example
* Working support for Delay, with example
* Add memory.x
* Add rustfmt.toml
* Added myself as an author

The RCC peripheral returns a "Core Clock Distribution and Reset"
structure, which represents that the core clock configuration is
frozen. However we retain the right to switch most PKSUs on the fly,
to fine-tune PLL frequencies, and to enable / reset peripherals.

For now, use a local build of the stm32h7 crate. This keeps us
up-to-date without Adam having to issue patch releases all the
time. Hopefully this will stabilise and can be reverted soon. This is
documented in the "hacking" section of the README

Tested on STM32H743 Rev Y

stm32h7xx-hal PR #2

See PR #1 for context
@astraw
Copy link
Member

astraw commented May 27, 2019

Wow, this is a lot, thanks for the PR. At first glance, it looks like you've done the hard work of getting quite some things working that I hadn't managed yet, so this is really welcome.

It will take me a little while to go over this but I have a question already. Do you think it makes sense to keep the examples in a separate crate? For me, this seems to make sense since probably most examples will depend not just on a h7xx chip, but also on a particular board. I had started the nucleo-h743zi crate for just this. Do you have a nucleo-h743zi? How does this idea sound to you?

Thanks again and I will try to test test in the next couple of days. @jordens may also be interested.

@astraw
Copy link
Member

astraw commented May 28, 2019

In addition to the above points, which are still open, I now have a few more comments and questions. I was able to get an LED blinking and some bits being toggled using your blinky and spi examples respectively - great!

In Cargo.toml, I had to make this change to get compilation with nightly to work:

[dev-dependencies]
panic-itm = "0.4"
cortex-m-log = {version="0.6", features=["itm"]}

It looks like nightly is needed in your PR for the never_type (!) and type_alias_enum_variants. I would prefer to keep this compiling on stable. ! could be substituted by void::Void I think, although I have not used it myself. I do not know how difficult it would be to find an alternate solution to type_alias_enum_variants.

There is quite some ITM logging in the examples. I have never been able to get ITM working when I wasn't connected with a debugger but rather the chip seems to lock up early. Is that the case for you? If so, I think it would be good to put the ITM logging behind a feature gate so that the examples can also be run without a debugger.

In README.md, I suggest adding this:

## Building

Prerequisites:

    rustup component add llvm-tools-preview --toolchain nightly

To build:

    cargo +nightly build --examples --features "stm32h743" --release

    cargo +nightly objcopy --release --features stm32h743 --example blinky -- -O binary target/thumbv7em-none-eabihf/release/examples/blinky.bin

@richardeoin
Copy link
Member Author

That sounds great! I'll answer some of your questions

Do you think it makes sense to keep the examples in a separate crate?

[dev-dependencies]

Cargo has first-class support for examples through cargo build --examples, so I think we should use that. Looks like you've got it working already! I'll add the [dev-dependencies] and commands in README.md

I had started the nucleo-h743zi crate for just this. Do you have a nucleo-h743zi?

A board support crate sounds great! I don't have a nucleo board, and it looks like distributors are out of h743zi boards whilst not yet having the h743zi2 version. When I can I should pick one up.

It looks like nightly is needed in your PR

I couldn't think of an obvious solution to type_alias_enum_variants either. The stm32h7 aliases its enums, and accessing anything other than the variants of that aliased enum would be very confusing to read. I figured anyone this bleeding edge would be on nightly, but agree that stable would be desirable.

I have never been able to get ITM working when I wasn't connected with a debugger but rather the chip seems to lock up early. Is that the case for you?

No, my blinky keeps blinking on external power! There's a useful discussion here perhaps. But debugger misconfigurations and so on are common, so a feature gating the examples would be fine.

richardeoin referenced this pull request in richardeoin/stm32h7xx-hal May 28, 2019
* Working support for RCC configuration, with example
* Working support for SPI, with example
* Working support for Serial, with example
* Working support for Delay, with example
* Add memory.x
* Add rustfmt.toml
* Document codegen options in Cargo.toml
* Added myself as an author

The RCC peripheral returns a "Core Clock Distribution and Reset"
structure, which represents that the core clock configuration is
frozen. However we retain the right to switch most PKSUs on the fly,
to fine-tune PLL frequencies, and to enable / reset peripherals.

For now, use a local build of the stm32h7 crate. This keeps us
up-to-date without Adam having to issue patch releases all the
time. Hopefully this will stabilise and can be reverted soon. This is
documented in the "hacking" section of the README

Tested on STM32H743 Rev Y

stm32h7xx-hal PR #2

See PR #1 for context
@richardeoin richardeoin force-pushed the gpio-rcc--serial-spi-delay branch from c96c2b9 to 9ee7566 Compare May 28, 2019 20:50
@richardeoin
Copy link
Member Author

I'm keeping richardeoin/master up-to-date with this PR, so you can view a rendered README there.

@richardeoin
Copy link
Member Author

Looks like cargo objcopy is a non-standard extension to cargo. For those that want a separate bin file, do we need to say something like:

$ cargo install cargo-binutils

$ rustup component add llvm-tools-preview

$ cargo objcopy --release --features stm32h743,rt --example blinky -- -O binary target/thumbv7em-none-eabihf/release/examples/blinky.bin

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

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

Impressive.
I'd really like to find some time to see how I can shoehorn my project into using this in practice and figure out whether using the hal would constrain me.

  • Since we have quite some documentation on the register field variants now (e.g. RCC and SPI), I think those should be used. It would make several comments redundant.
  • I think the IO compensation cell (syscfg.cccsr) needs to be configured as well since you seem to cover all frequency ranges supported.

src/rcc.rs Outdated
let ref1_ck = srcclk / pll1_m;
assert!(ref1_ck >= 1_000_000 && ref1_ck <= 2_000_000);

// VCO output frequency. Choose the lowest VCO frequency
Copy link
Member

Choose a reason for hiding this comment

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

Is this obvious? The higher VCO frequency would give less noise but higher power consumption.
Same for the PFD frequency. Higher is better in terms of noise but worse in terms of power.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just picked something here to get it working. I think I agree on the default - it should pick the highest VCO frequency. I dread to think how low the Q of the VCO's planar L is at 150MHz! It already chooses the highest PFD frequency, so that can stay.

Perhaps ultimately we should offer a choice in the config that meets almost all use cases - something like:

pub enum VcoStrategy {
    HighPerformance, // VCOH, highest PFD frequency, highest VCO frequency
    Normal,          // VCOL, highest PFD frequency, highest VCO frequency
    LowestPower,     // VCOL, lowest PFD frequency, lowest VCO frequency
}

* Working support for RCC configuration, with example
* Working support for SPI, with example
* Working support for Serial, with example
* Working support for Delay, with example
* Add memory.x
* Add rustfmt.toml
* Document codegen options in Cargo.toml
* Added myself as an author

The RCC peripheral returns a "Core Clock Distribution and Reset"
structure, which represents that the core clock configuration is
frozen. However we retain the right to switch most PKSUs on the fly,
to fine-tune PLL frequencies, and to enable / reset peripherals.

For now, use a local build of the stm32h7 crate. This keeps us
up-to-date without Adam having to issue patch releases all the
time. Hopefully this will stabilise and can be reverted soon. This is
documented in the "hacking" section of the README

Tested on STM32H743 Rev Y

stm32h7xx-hal PR #2

See PR #1 for context
Lower noise, more power consumption. Still using VCOL
@richardeoin richardeoin force-pushed the gpio-rcc--serial-spi-delay branch from 9ee7566 to 63586ac Compare May 29, 2019 17:14
@richardeoin
Copy link
Member Author

Thanks for the comments @jordens . I'll follow up on your two points over the next few days

* Assert limits based on voltage scale
* Set flash wait states based on voltage scale
* Use more register field variants from the PAC
* Configure the IO compensation cell in SYSCFG
So crate can be returned to stable once
`feature(type_alias_enum_variants)` is stabilised. Document in README
@richardeoin
Copy link
Member Author

Followed up with the following changes:

  • Use more register field variants from the PAC, although there's probably more to add.
  • Configure IO compensation cell (syscfg.cccsr) at the end of the RCC freeze routine.
  • Remove requirement on feature(never_type), so the crate can be built on stable as soon as type_alias_enum_variants lands (soon).

Almost ready to merge I think, unless there's further comments. Remaining open points:

  • Feature gates on ITM in examples.
  • Example of cargo objcopy in README.

@jordens
Copy link
Member

jordens commented Jun 6, 2019

Should we push for a new stm32-rs release now to first land the stm32h7 changes?

@hargoniX
Copy link
Member

Just adding as a note here from #3 once this PR is merged I'd add my implementations from my stm32h7x3-hal project

@astraw
Copy link
Member

astraw commented Jun 13, 2019

(Sorry for my delay, things have been a bit crazy lately.) Shall I just merge this and we sort out any remaining issues later? I think the crate is only barely useable now and I'm interested in getting everyone onboard as soon as possible.

I was thinking about a possible solution to the demo code: rather than putting it in examples/, we make sub-crates in this repo which would be e.g. nucleo-h743-examples with its own Cargo.toml. The base directory in this repo would become the root of a cargo workspace. @richardeoin what board are you testing your h743 on? We could also make a xyz-h743-examples crate for your examples.

I agree it doesn't make sense to keep this on stable while waiting for type_alias_enum_variants to land. We can shift to nightly for a brief period until that moves to stable.

@adamgreig @therealprof can you add @jordens @richardeoin and @hargoniX as committers to this repo? I think they are all equally (if not more) qualified than me in this regard.

@richardeoin
Copy link
Member Author

Since this is blocking @hargoniX, we should probably go with a merge now. I'm happy to retain the open issues

  • Examples structure
  • Set stm32h7 version on the next stm32-rs release.
  • Eventual return to stable (Rust 1.37.0)
  • Feature gates on ITM in examples.
  • Example of cargo objcopy in README.

@astraw astraw merged commit 41b7a04 into stm32-rs:master Jun 13, 2019
@adamgreig
Copy link
Member

@adamgreig @therealprof can you add @jordens @richardeoin and @hargoniX as committers to this repo? I think they are all equally (if not more) qualified than me in this regard.

Done; if those people want to also join stm32-rs just shout. @astraw, I've also set you as repo admin (as you should have been from the beginning!) so you can manage collaborators (and other admins etc) yourself now.

@richardeoin richardeoin deleted the gpio-rcc--serial-spi-delay branch June 13, 2019 19:37
@hargoniX
Copy link
Member

@adamgreig I'd love to join the stm32-rs org, I got lots of free time in the next months and was planning to work on projects in embedded rust during that time anyways.

mtthw-meyer pushed a commit to mtthw-meyer/stm32h7xx-hal that referenced this pull request Jul 9, 2020
…elay

Overhaul of RCC, GPIO, Serial. Add SPI, Delay
seisyuu-hantatsushi pushed a commit to seisyuu-hantatsushi/stm32h7xx-hal that referenced this pull request Jan 27, 2024
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.

5 participants