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.

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

Add support for I2S MCLK. #594

Merged
merged 7 commits into from
Jan 4, 2023
Merged

Add support for I2S MCLK. #594

merged 7 commits into from
Jan 4, 2023

Conversation

DatanoiseTV
Copy link
Contributor

This commit implements support for generating basic 256*fs MCLK using the APLL.

@DatanoiseTV DatanoiseTV mentioned this pull request Dec 24, 2022
Closed
earlephilhower
earlephilhower requested changes Dec 26, 2022
View reviewed changes
src/AudioOutputI2S.cpp
@@ -104,6 +104,20 @@ bool AudioOutputI2S::SetPinout(int bclk, int wclk, int dout)

return true;
}

bool AudioOutputI2S::SetPinout(int bclk, int wclk, int dout, int mclk)
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please add (void) mclk in the non-ESP32 path? OTW we get fatal warning/errors.

src/AudioOutputI2S.cpp
@@ -147,6 +161,16 @@ bool AudioOutputI2S::SetLsbJustified(bool lsbJustified)
return true;
}

bool AudioOutputI2S::SetMclk(bool enabled){
Copy link
Owner

Choose a reason for hiding this comment

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

Same as other comment, for enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@earlephilhower
Copy link
Owner

Something got weird on the commit/pushes and your (void) marker seems to have been lost (see the 8266 failures). Can you take a look again, please?

@DatanoiseTV
Copy link
Contributor Author

Something got weird on the commit/pushes and your (void) marker seems to have been lost (see the 8266 failures). Can you take a look again, please?

I think now it is right.

@earlephilhower
Copy link
Owner

OK, we're almost there. The change in the 8266 needs to applied to the RP2040 branch and we should all be OK. The joys of supporting 3 different (actually w/the ESP32-XXs maybe 6?) boards.

@earlephilhower
Copy link
Owner

@DatanoiseTV can you pull in the latest master? The ESP8266 internal test scripts changed over in that repo, so I needed to hack the tests here that used it. It should now work and we should be able to hopefully get a complete run.

@earlephilhower

This comment was marked as outdated.

Sign in to view
@earlephilhower earlephilhower merged commit 4ba7df1 into earlephilhower:master Jan 4, 2023
@FREEWING-JP
Copy link

FREEWING-JP commented Mar 11, 2023 •
edited
Loading

I tested CS4344 I2S DAC with ESP32-D0WD-V3 (revision v3.0) .
( CS4344 need MCLK)
It works Fine !
Thank you !

https://github.com/FREEWING-JP/ESP32_BadApple_MP3

https://youtu.be/4GXgd6Nbv94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@earlephilhower earlephilhower earlephilhower approved these changes

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
@DatanoiseTV @earlephilhower @FREEWING-JP