search

nxp-mcuxpresso / mcux-sdk

MCUXpresso SDK
BSD 3-Clause "New" or "Revised" License
323 stars 142 forks source link

fsl_wm8960.c: Wrong argument order, #70

Closed robert-hh closed 2 years ago

robert-hh commented 2 years ago

https://github.com/NXPmicro/mcux-sdk/blob/7b545788e3f6e0c1a8decae21e428ed8d5b5a8e7/components/codec/wm8960/fsl_wm8960.c#L201

The argument order or the call is wrong. It must me:

        WM8960_CHECK_RET(WM8960_SetInternalPllConfig(handle, config->masterClock.sysclkFreq, sysclk,
                                                     config->format.sampleRate, config->format.bitWidth),
mcuxcc commented 2 years ago

Hi @robert-hh ,

Thank you for your feedback.

I am sorry for late reply.

Let us discuss the issue you report, the API WM8960_SetInternalPllConfig prototype is

*WM8960_SetInternalPllConfig( wm8960_handle_t handle, uint32_t inputMclk, uint32_t outputClk, uint32_t sampleRate, uint32_t bitWidth)**

The second parameter the is source clock of the PLL, the source is the external input from MCLK pin, it is corresponding to the member mclk_HZ of the wm8960_audio_format_t. However the member sysclkFreq of the wm8960_master_sysclk_config_t is the output clock frequency of PLL, so i think there is no issue here.

How do you think?

robert-hh commented 2 years ago

I pretty agree on on the intention of the function, and if used that way, it works. So my concern may stem from the way, the values are set in the SAI example code in the driver library.

mcuxcc commented 2 years ago

so you have concern about the member sysclkFreq is setting in application? Do you better suggestions? image

robert-hh commented 2 years ago

Indeed, here it looks fine. So maybe I got lost in translation. FInally, I use it that way, but I translated that driver for my application into Python. Still with MIMXRT boards, but with MicroPython as platform.

mcuxcc commented 2 years ago

Excited to hear your are using using RT with MicroPython. Looking forward more feedback comes from you.

robert-hh commented 2 years ago

It is part now of the official MicroPython project at https://github.com/micropython/micropython. Together with two other people I started to bring that port forward and keep it up-to-date with MicroPython. We try to test it with all available board. But we have no MIMXRT1060 and -1064 dev boards. I did not like to spend more money just for short tests. So if you have access to these boards, you could give it a try. According to Murphy's law, most probably they will not work. Pre-built binaries are at https://micropython.org/download/?port=mimxrt. Not listed there, but already working are ports for MIMXRT1015_DEV, MIMXRT1176_DEV and Olimex RT1010Py.

robert-hh commented 2 years ago

The Python WM8960 driver is at: https://github.com/robert-hh/wm8960