Skip to content

Draft: I2S HAL implementation (for embedded-hal v0.2.x) #212

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Draft: I2S HAL implementation (for embedded-hal v0.2.x) #212

wants to merge 2 commits into from

Conversation

maxekman
Copy link

This is a WIP and possible reference implementation for the embedded-hal proposal for a I2S interface: rust-embedded/embedded-hal#204

Not ready for review, left here in the open for comments and opinions.

  • Fully support all devices with the macro system.
  • Support possible second I2SPPL clock source in some devices (I think).
  • Calculate viable audio clock frequencies. A bit tricky because the calculation is spread out between the RCC and I2S setup, they both have mult and div settings that have to play together to achieve the correct final MCKL out.
  • Provide example, can be copied from the stm32f407-disc crate when done.
  • Configurable I2S standard (Philips, right, left etc) and bit depth.

@maxekman maxekman marked this pull request as draft September 14, 2020 19:02
@YruamaLairba
Copy link
Contributor

ooops, i started to work on i2s without seeing there was already a work in progress. I will look at it.

I'm a bit skeptical about calculating i2s clock and audio frequency. As you said it's a nightmare to calculate it directly and i don't see how to design an appropriate HAL interface for that since the calculation is splitted between RCC and IS2 peripherals. Adding to this, such interface tend to hide differences between real and expected clock.
Instead, it's easy for an user to do scripts to try all valid combination to find the ones that fulfill their requirement, So why not to let use prescaler value ?

@mgottschlag mgottschlag mentioned this pull request Jan 2, 2021
Merged
6 tasks
bors bot added a commit that referenced this pull request Jan 10, 2021
@bors @mgottschlag
Merge #247
2dc4277 
247: rcc: I2S and SAI clocks r=therealprof a=mgottschlag

# Preface

Some of this code is probably pretty ugly. In parts, the code looks pretty bad, and it imposes pretty arbitrary limitations (see below) in terms of functionality. I propose merging something like this on the basis that it is strictly more powerful than the previous implementation - in cases where no I2S or SAI clocks are requested, the old code is still used providing identical performance and identical results.

# Situation

The current RCC code only provides the sysclk, but no I2S and SAI clocks. Future pull requests (#212? I am currently working on support for SAI) require these clocks. Different MCUs have wildly differing clock trees, though. Some microcontrollers (e.g. STM32F411/446) have separate I2SPLLM dividers, others do not. F413/423 do not have an SAI PLL even though SAI is supported. F410 does not even have an I2S PLL and the I2S clock is generated by the main PLL. The different amount of SAIs and I2S instances causes differences in the clock selection code. See #240 for a table of all those differences. Note: The table does not include that F413/423 have a different SAI divider range compared to the other models.

# Approach

This wide range of different configurations and the flexible, yet very model-specific connections (e.g., SAI clocks can usually be generated by the I2S PLL and vice versa) make writing a completely flexible solution difficult. The computational complexity of such a solution to generate the ideal results might very likely be prohibitive for any runtime implementation.

I therefore decided on an approach with some limitations to keep both implementation and computational complexity down. In particular, I2S clocks, if required, are always generated by the I2S PLL, and likewise SAI clocks are always generated by the SAI PLL, with special cases for the models where these PLLs do not exist. This means that there can be only one I2S frequency (and likewise SAI frequency) that does not match the provided I2S_CKIN external frequency. This should be enough for all applications except for applications which implement resampling between e.g. 44100 and 48000 Hz audio and have to implement an I2S master for both.

The code decouples clock selection (the bottom half of the table in #240) from PLL optimization to reduce the numbers of special cases required in each part - clock selection is implemented in the form of I2sClocks and SaiClocks types in rcc.rs.

# Alternatives

I believe the restrictions described above are valid for now. If, eventually, we want a more complete and more flexible interface and do not care about API backwards compatibility, we can implement clock tree configuration in the form of an external compile-time tool - either via procedural macros or via a library that can be used within build.rs. In this case, the RCC library should probably be changed to provide two functions "RCC.apply_static_config()" or "RCC.apply_runtime_config()", where the former just applies a set of precalculated config options and the latter executes the current cheap yet limited runtime configuration algorithms.

# Status/Testing

At the moment, most of the code is still completely untested and I only have a STM32F429 discovery board for testing:

- [x] Main PLL (most models): The old main PLL code to generate sysclk has been tested with a blinky example on a STM32F429 board. Most models should behave identically.
- [ ] Main PLL (STM32F410) when an I2S clock has been requested: TODO, does anybody have an STM32F410 and can run any blinky example modified to request ".i2s_clk(64.mhz())"?
- [ ] I2S PLL (STM32F405/407/415/419/427/429/437/439): WIP. If anyone working on I2S wants to spend some time on debugging, feel free ;-)
- [x] SAI PLL (STM32F427-429, 469, 479): The code seems to generate the correct MCK signal.
- [ ] I2S PLL (STM32F411-413, 423, 447): I do not have the required MCU. Most of the code is identical to the other models, though. Let me test on the 429 first before you give it a go.
- [x] SAI PLL (STM32F446): I do not have the required MCU. The code path should be identical to the STM32F429 without main PLL though, and that works.

Once the tests on the F429 are done, I need help with the other models - and I could use help with the F410 right away. Do not expect the code to work as-is, though.

Co-authored-by: Mathias Gottschlag <[email protected]>
Co-authored-by: Mathias Gottschlag <[email protected]>
@samcrow
Copy link
Contributor

samcrow commented Feb 5, 2021

FYI: I recently made another I2S driver similar to this one.

Driver library: https://github.com/samcrow/stm32_i2s/
i2s module in a fork of stm32f4xx-hal: https://github.com/samcrow/stm32f4xx-hal/blob/i2s/src/i2s.rs

Some properties:

  • Supports all four master/slave and transmit/receive modes
  • Supports all for data formats (16 bits, 16 bits in 32-bit frames, 24 bits in 32-bit frames, 32 bits)
  • Automatic clock division calculation based on the input frequency and desired sample rate (or the division can be specified manually)
  • Not compatible with embedded-hal yet
  • I have tested master transmit mode fairly thoroughly, but have not tested the other modes

I don't want to steal your thunder or invalidate the work you've already done. If you think any parts of my code are good, you're welcome to copy them.

@maxekman
Copy link
Author

maxekman commented Feb 5, 2021

@samcrow I'm glad it’s being worked on! Haven’t had time myself. Please also take anything you want from my work and submit a PR if you have something that is more complete. Maybe I’ll have time to revisit this some day, but not right now. Thanks for being so considerate!

@samcrow samcrow mentioned this pull request Feb 7, 2021
Merged
bors bot added a commit that referenced this pull request Apr 15, 2021
@bors @samcrow
Merge #265
98d7bc1 
265: Add I2S communication using SPI peripherals r=therealprof a=samcrow

# Introduction

Welcome to my first large pull request, which adds support for I2S audio communication using supported SPI peripherals. Like the way we support CAN with the bxcan library, I am proposing to support I2S using my [stm32_i2s_v12x](https://crates.io/crates/stm32_i2s_v12x) library.

Although stm32_i2s_v12x is in a separate repository, we can also talk about it here.

# Notes

* The I2S module is available if the `i2s` feature is enabled, in order to not increase compile time for applications that don't use I2S.
* All four modes are supported: master transmit, master receive, slave transmit, and slave receive.
* Basic support for DMA, interrupts, and error detection is provided.
* All STM32F4 models are supported.
* I added two examples that run on the STM32F411E-DISCO board and send audio to the on-board DAC. One of them uses DMA.

These changes are not perfect, so criticism and suggestions are welcome.

# Limitations

* No support for full-duplex communication
* I have tested master transmit mode fairly thoroughly, but have not tested the other modes.
* No support for embedded-hal I2S traits
  * [The I2S pull request](rust-embedded/embedded-hal#204) is still under review. The proposed traits look compatible with this driver code. Until that pull request gets merged and released, people can still use this driver code directly.

# Related work

* SAI driver pull request in this repository: #248
* Another, less complete, pull request for I2S in this repository: #212
  * also #145
* embedded-hal pull request for I2S traits: rust-embedded/embedded-hal#204 

Co-authored-by: Sam Crow <[email protected]>
Co-authored-by: Sam Crow <[email protected]>
@maxekman
Copy link
Author

I'll close this as @samcrow has merged I2S support. Thanks for finishing it!

@maxekman maxekman closed this May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maxekman @YruamaLairba @samcrow