driver:audio: sf32lb52x:Add onchip codec driver for sf32lb52x platform #97905 #97906
Conversation
zephyrbot
requested review from
JarmouniA,
TomasBarakNXP,
cameled,
ck-telecom,
gmarull,
kartben,
kv2019i,
lgirdwood,
lyakh and
nashif
drivers/audio/Kconfig
Outdated
| # Audio Codec options | ||
| # | ||
|
|
||
| source "drivers/audio/Kconfig.sf32lb" |
There was a problem hiding this comment.
This is not the correct position, it should be placed here.
There was a problem hiding this comment.
not depend audio codec, not use codec.h API, so the menu place out ot the audio codec menu
|
|
||
| config AUDIO_CODEC_SF32LB | ||
| bool "sf32lb codec driver support" | ||
| select USE_SIFLI_HAL |
There was a problem hiding this comment.
Should the HAL be selected at the SoC level?
There was a problem hiding this comment.
USE_SIFLI_HAL is sco low chip driver
There was a problem hiding this comment.
USE_SIFLI_HAL will using \zephyrproject\modules\hal\sifli,
remote url is https://github.com/zephyrproject-rtos/hal_sifli
| extern void HAL_TURN_ON_PLL(); | ||
| extern void HAL_TURN_OFF_PLL(); | ||
|
|
||
|
|
| .start = sf32lb_codec_start, | ||
| .stop = sf32lb_codec_stop, | ||
| .set_dac_volume = sf32lb_codec_set_dac_volume, | ||
| .set_dac_mute = sf32lb_codec_set_dac_mute, |
There was a problem hiding this comment.
Is there a driver API named set_dac_volume?
There was a problem hiding this comment.
self defined API in sf32lb_codec.h
| @@ -0,0 +1,189 @@ | |||
| /* | |||
There was a problem hiding this comment.
you are not using zephyr codec device driver model?
There was a problem hiding this comment.
The Zephyr audio codec interface cannot support some functions. For example, during HFP calls, it cannot strictly control the order of opening the mic and speaker. Both the mic and speaker may be opened simultaneously. After sending data, there is no event callback, so the app does not know when it can send the next frame of data. ...
There was a problem hiding this comment.
it's better to open mic & speaker simultaneously for Echo cancellation using WEBRTC or other algorithm
There was a problem hiding this comment.
Then you probably need to extend the codec driver API
There was a problem hiding this comment.
ok, now codec only has AUDIO_DAI_TYPE_I2S, only has one config for i2s.
typedef union {
struct i2s_config i2s; /**< I2S configuration /
/ Other DAI types go here */
} audio_dai_cfg_t;
i'll add new DAI type AUDIO_DAI_TYPE_PCM for pcm playback and capture using onchip codec
and add playback and capture data coming callback interface.
for i2s capture, I not see a sample can notify app to read data or callback to app with incoming data.
I'll add these in new DAI type config.
|
Please take a look at https://docs.zephyrproject.org/latest/contribute/guidelines.html, your commit message does not comply with Zephyr conventions. |
There was a problem hiding this comment.
This PR has some fundamental issues, see my comments. Also please, do not introduce platform specific samples for drivers that are supposed to offer a vendor-agnostic interface. Only use these samples if you have to extend standard interface with vendor-specific extra APIs.
drivers/audio/sf32lb_codec.c
Outdated
| extern void HAL_TURN_ON_PLL(); | ||
| extern void HAL_TURN_OFF_PLL(); |
There was a problem hiding this comment.
definitely -1, we need clock control driver to be extended.
| #define AUDCODEC_ADC0_DMA_IRQ DMAC1_CH4_IRQn | ||
| #define AUDCODEC_ADC0_DMA_IRQ_PRIO 0 | ||
| #define AUDCODEC_ADC0_DMA_INSTANCE DMA1_Channel4 | ||
| #define AUDCODEC_ADC0_DMA_REQUEST 39 | ||
|
|
||
| #define AUDCODEC_DAC0_DMA_IRQ DMAC1_CH1_IRQn | ||
| #define AUDCODEC_DAC0_DMA_IRQ_PRIO 0 | ||
| #define AUDCODEC_DAC0_DMA_INSTANCE DMA1_Channel1 | ||
| #define AUDCODEC_DAC0_DMA_REQUEST 41 |
There was a problem hiding this comment.
-1, this needs to be properly described in devicetree, use DMA driver
drivers/audio/sf32lb_codec.c
Outdated
| #undef LOG_INF | ||
| #define LOG_INF printf | ||
| #if (LOG_LEVEL >= LOG_LEVEL_DEBUG) | ||
| #else | ||
| #endif |
| HAL_PMU_EnableAudio(1); | ||
| HAL_RCC_EnableModule(RCC_MOD_AUDCODEC_HP); | ||
| HAL_RCC_EnableModule(RCC_MOD_AUDCODEC_LP); |
There was a problem hiding this comment.
there's clock control driver for this
|
4 participants
No description provided.