arduino / ArduinoCore-samd

Arduino Core for SAMD21 CPU
GNU Lesser General Public License v2.1
470 stars 718 forks source link

I2S Library clocking improvements #189

Open Mike-Dellisanti-Bose opened 7 years ago

Mike-Dellisanti-Bose commented 7 years ago

Nice to see the recent edition of the I2S library -- it's a neat little feature of the D21 and is more flexible then what's included on the SAM3 in the DUE. The example project connects to a device that doesn't require an MCLK (cool!), but it would be great to have a little more flexibility in the clocking to allow for connection to other devices.

The first change I'd like to make is adding an MCLK pin option. Many codecs need this to operate. The D21 includes a fractional PLL which can generate the MCLK for standard audio sampling frequencies. Configuration of the PLL is probably beyond the scope of the I2S lib, but small mods could be made to the existing lib to allow this to be the source clock (rather than the system clock), which would allow the part to output the proper MCLK clock to the external codec.

The second change is adding an additional overloaded begin() method that would take a new clock config argument. The clock config argument could be something like the SPISettings() class that would offer more options, but provide a basic default for an simple use cases. This could be used to set clock dividers for more granular control (and could help setup the mclk stuff from above).

All in all, I think the code changes would be fairly minimal. I don't mind putting out a PR for this, but since this is a new module there may be additional changes in the works that I am not aware of (or perhaps this is out of scope). If this is interesting/useful, I'll go ahead and code something up and submit for review.

sandeepmistry commented 7 years ago

Hi @bose-mdellisanti,

There's a lengthy discussion on the developer mailing list regarding I2S: https://groups.google.com/a/arduino.cc/forum/#!topic/developers/pER4iiz7WZM

We're definitely open to adding support for MCK output. While working on the I2S library, the 2 devices we tested with (Adafruit I2S 3W Class D Amplifier Breakout - MAX98357A and ICS43432 I2S Digital Microphone) did not require a MCK signal, so we left it out of the initial release.

Do you have any recommended I2S devices that require a MCK signal that we can test with?

In order to move forward the the MCK change, maybe you can submit a pull request to get the discussion going? Then we can tie it into the mailing list.

PaulStoffregen commented 7 years ago

Do you have any recommended I2S devices that require a MCK signal that we can test with?

I'd be happy to send you an audio shield we use with Teensy. It has the SGTL5000 codec chip, which requires MCLK.

Please email me directly (paul at pjrc dot com) with your mailing address.

Mike-Dellisanti-Bose commented 7 years ago

I took a quick look through the post over on the forum and certainly think there is some common ground there. I'll need a little more time to digest it all before putting the code together.

Finding an off-the-shelf breakout board for a codec is a bit of a challenge. I am currently using a TI Codec/DSP (AIC3256) with the Zero on a custom board. A quick search found this board: http://www.digikey.com/product-detail/en/microchip-technology/AC320100/AC320100-ND/5401224, which looks like it might be useful, but I haven't used it.

I could send one of our custom zero boards with the TI codec, but this also requires our board support package which might not exactly be what you want. Let me know if that's helpful.

In the meantime I'll try to pull a proposal together on what some of the changes might look like to keep the discussion going.

sandeepmistry commented 7 years ago

I'd be happy to send you an audio shield we use with Teensy. It has the SGTL5000 codec chip, which requires MCLK.

@PaulStoffregen thanks, I've sent you an email!

I took a quick look through the post over on the forum and certainly think there is some common ground there. I'll need a little more time to digest it all before putting the code together.

@bose-mdellisanti great, we don't have to cover all the topics in the dev mailing discussion at once. Your initial MCK suggestion would be a great start.

I could send one of our custom zero boards with the TI codec, but this also requires our board support package which might not exactly be what you want. Let me know if that's helpful.

In the meantime I'll try to pull a proposal together on what some of the changes might look like to keep the discussion going.

Thanks for the offer, for now we'll plan to test your proposal with the Teensy Audio Shield Paul mentioned.

Your custom zero board sounds like a good candidate for the At Heart program ...

Taburn commented 2 weeks ago

Hello. I'm trying to use the Arduino MKR Zero to send I2S to a PCM1754 ADC, which requires an MCLK signal. Any chance this library could be modified to include an MCLK output?