Sai i2s v0.2.x 1

[mtthw-meyer]

Implemented I2S for SAI1-4.
Added traits to split DMA channels.

Example runs on daisy seed board successfully.

Closes #90

[mtthw-meyer]

I'm done with this if you think it's ready for review.

[richardeoin]

Many thanks for contributing this @mtthw-meyer! I don't have any comments on the DMA impl, it looks fine for what it is but there's likely more to be learnt by using it.

Don't forget to add yourself to the authors list

[richardeoin]

I finally got round to trying this with the WM8994. It actually requires a huge amount of configuration over I2C to get it to do anything, but so far I've been using it as an ADC.

The "I2S" interface from the datasheet:

The main point is that if configured to use the actual "I2S" interface it follows the Phillips I2S specification where the DACDAT/ADCDAT are available one cycle later. That means that FRCR->FSOFF needs to be set. The first bit offset in SLOTR isn't quite what's needed - that masks the bit ahead of the MSB but the LSB is still lost.

Could FSOFF be added to I2SChanConfig with some sensible name?

Also the WM8994 defaults to having 16-bit audio words but 64-bit frames. It does work with this implementation by changing the AIF1 BCLK in Register 771, then it has 16-bit words and 32-bit frames. But I wonder if it would make more sense for I2SChanConfig::new to specify the frame length and just assume the number of slots is 2 for now?

The current code doesn't make much sense for slots != 2. slots = {0,1} would go very badly (frame_length = 0) whilst my understanding is that > 2 slots is only practically useful where some of them are disabled, so other devices on the bus can insert data. We could add support for slots != 2 at some point in the future, but I think those are quite specialised use cases.

[mtthw-meyer]

I'll look into those changes. There are ALOT of settings and it's not clear which ones are useful to be accessible.

[richardeoin]

Thanks for those updates! Indeed managing all the configuration options is difficult, it's really only by experimenting that you get an idea of which ones are useful.

A nice method of managing all the configuration options is the 'builder pattern', there's a good example in the spi config @ryan-summers added recently. The new function can still take arguments, but things that have obvious defaults / are optional can configured by a builder method instead.

[mtthw-meyer]

Builder is in place now with most of the configuration exposed. I cleaned up the listen/unlisten methods to work by channel since they were locked into channel A and exposed some more controls at the sai level.

[richardeoin]

Nice improvements, thanks!

What do you think of allowing the user to specify the frame size? It can be implemented with an Option<T> in the I2SChanConfig with default None; then in i2s_config_channel you can use unwrap_or to use the calculated value if it was never set by the user.

[mtthw-meyer]

I can expose that as well, I'm not sure anyone will need it but might as well.

[mtthw-meyer]

Closes #90

[richardeoin]

Looks good to me! Now this is only waiting on rust-embedded/embedded-hal#204 for the traits.

[eldruin]

Alright, I'll mark this implementation as finished in the issue. Thank you for all your work!

[richardeoin]

I'd like to see this merged for the next stm32h7xx-hal release. Until embedded-hal catches up, we can do by copying the required traits into the traits directory.

@mtthw-meyer does that sound like a reasonable plan to you?

[mtthw-meyer]

Sounds fine to me.

[richardeoin]

Done and rebased onto master

[richardeoin]

bors r+

[bors]

Build succeeded:

• ci (1.43.0, stm32h743)
• ci (1.43.0, stm32h743v)
• ci (1.43.0, stm32h747cm7)
• ci (1.43.0, stm32h753)
• ci (1.43.0, stm32h753v)
• ci (stable, stm32h743)
• ci (stable, stm32h743v)
• ci (stable, stm32h747cm7)
• ci (stable, stm32h753)
• ci (stable, stm32h753v)
• Rustfmt