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.

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

Patch in WM8960 support #133

Merged
merged 8 commits into from
Apr 23, 2022
Merged

Patch in WM8960 support #133

merged 8 commits into from
Apr 23, 2022

Conversation

probonopd
Copy link
Owner

@probonopd probonopd commented Apr 21, 2022

Temporary patch to add WM8960 support until rsta2/circle#286 is integrated into Circle

Thanks @matemaciek

@probonopd
Copy link
Owner Author

Tested with MiniDexed and a WM8960 DAC, works for me! Both the headphone jack on the WM8960 board and also the speakers connected to the built in amplifier +1

Here is the build:
https://github.com/probonopd/MiniDexed/suites/6210021520/artifacts/219385493

Now it would be good if someone with a PCM5102A or PCM5122 based DAC could also test this in order to ensure that it doesn't break those.

@probonopd
Copy link
Owner Author

@matemaciek I am mirroring and locally applying your patch in MiniDexed for now so that we can enjoy our WM8960s while an architecturally cleaner solution is being worked out in Circle.

@probonopd
Copy link
Owner Author

probonopd commented Apr 21, 2022

It would be good if someone with access to a PCM5122 DAC could test the build from this branch (linked above) to ensure this doesn't break the PCM5122 which also uses i2c. @rsta2 do you have one?

Also, since this is auto-detecting i2c addresses, should we remove the related code from MiniDexed?

@ChKK1963
Copy link

It would be good if someone with access to a PCM5122 DAC could test the build from this branch (linked above) to ensure this doesn't break the PCM5122 which also uses i2c

Just tested the build with my new Raspiaudio+ V2 DAC which has a PCM5102A DAC. Seems to work fine.

@probonopd
Copy link
Owner Author

Thanks for testing @ChKK1963. We still need to find someone with a PCM5122 DAC though, as that one has i2c while the PCM5102A doesn't.

@matemaciek
Copy link

Also, since this is auto-detecting i2c addresses, should we remove the related code from MiniDexed?
Is there any? I only see passing of optional I2C address, and it still makes sense - if user provides specific address, auto-detecting is skipped.

@rsta2
Copy link
Contributor

rsta2 commented Apr 22, 2022

I tried a PiFi DAC+ v2.0 HAT (uses a PCM5122 DAC) attached to a RPi Zero 2 W using this build:

https://github.com/probonopd/MiniDexed/suites/6210021520/artifacts/219385493

First it initialized OK, but after some seconds the sound was strange and after a restart, the I2C initialization failed (-2 from all probed addresses). I aborted the test, because I don't want to destroy something.

With a kernel image, built from the main branch, everything works OK.

@matemaciek
Copy link

I've ordered PiFi, I'll look into it. Previous version was just assuming PCM5122 is present and was blindly sending its init sequence on its two known addresses. Can you try setting DACI2CAddress=76 and if that doesn't work DACI2CAddress=77 in minidexed.ini?

@rsta2
Copy link
Contributor

rsta2 commented Apr 22, 2022

@matemaciek I guess the i2cdetect test, you have implemented, does not work reliable with some devices. I will wait for the license information and then merge your PR, maybe with some modifications. Then I will test again.

@matemaciek
Copy link

@rsta2 They've just added GPLv3 (-:

@probonopd
Copy link
Owner Author

WM8960 works. If PCM5xxx also still works, we might want to consider merging.

@probonopd
Copy link
Owner Author

PCM5xxx confirmed, hence merging.

@probonopd probonopd merged commit addbd45 into main Apr 23, 2022
@probonopd probonopd deleted the wm8960 branch April 23, 2022 09:49
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.

4 participants