Skip to content

Conversation

@ljq-ctrl
Copy link

@ljq-ctrl ljq-ctrl commented Nov 1, 2025

…m #97906

audio playback and capure, extend API in codec.h

@ljq-ctrl
Copy link
Author

ljq-ctrl commented Nov 1, 2025

download code from github very very slow.
Encountered a git problem that cannot be solved
I close the following PR and recommit it refrence
https://docs.zephyrproject.org/latest/contribute/guidelines.html#contribution-workflow

the old PR is:
driver:audio: sf32lb52x:Add onchip codec driver for sf32lb52x platform #97906

@ljq-ctrl ljq-ctrl changed the title driver:audio: sf32lb52x:Add onchip codec driver for sf32lb52x platfor… drivers: audio: sf32lb52x: audio codec driver Nov 1, 2025
- uart
- gpio
- watchdog
- audio-codec
Copy link
Contributor

@JarmouniA JarmouniA Nov 1, 2025

Choose a reason for hiding this comment

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

Suggested change
- audio-codec
- audio

it should be used as a tag for some test/sample, otherwise has no utility.
https://docs.zephyrproject.org/latest/develop/test/twister.html#board-configuration

Speaking of which, this driver has to be tested (or at least built) in CI.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Please make sure to not make unrelated changes into a big commit, split changes into chunks as needed. Also, make sure CI passes and do not open new PRs.

@ljq-ctrl ljq-ctrl force-pushed the codec branch 8 times, most recently from 9625992 to 4f8250e Compare November 4, 2025 04:10
@ljq-ctrl ljq-ctrl requested a review from gmarull November 4, 2025 05:17
@ljq-ctrl ljq-ctrl force-pushed the codec branch 2 times, most recently from 7b5e71e to 6953241 Compare November 18, 2025 02:43
@gangheivt gangheivt force-pushed the codec branch 3 times, most recently from 2ed7f60 to a00b251 Compare November 24, 2025 06:33
@zephyrbot zephyrbot added the area: Samples Samples label Nov 24, 2025
@zephyrbot zephyrbot requested review from kartben and nashif November 24, 2025 06:35
@gangheivt gangheivt force-pushed the codec branch 5 times, most recently from a3b597d to c7eb930 Compare November 24, 2025 07:19
Gang He added 3 commits November 27, 2025 11:14
Add more callback in DMA processing, Fix DMA size bug.

Signed-off-by: Gang He <[email protected]>
Add audio codec device tree information for sf32lb52x.dtsi

Signed-off-by: Gang He <[email protected]>
SF32LB52X audio HAL driver source code is not need in Zephyr driver.

Signed-off-by: Gang He <[email protected]>
Support audio device that has both input and output function.

Signed-off-by: Gang He <[email protected]>
@@ -0,0 +1,22 @@
# Copyright (c) 2025 Qingsong Gou <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

why my name :)

Choose a reason for hiding this comment

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

Fixed.


pll_calibration(cfg->reg);

LOG_INF("%s done", __func__);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LOG_INF("%s done", __func__);

struct sf32lb_audcodec_data *data = dev->data;
struct sf32lb_codec_hw_config *hw_cfg = &data->hw_config;

memset(data, 0, sizeof(struct sf32lb_audcodec_data));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
memset(data, 0, sizeof(struct sf32lb_audcodec_data));

#include <zephyr/logging/log.h>

#include <register.h>
#include <bf0_hal_pmu.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include <bf0_hal_pmu.h>

Copy link
Author

@ljq-ctrl ljq-ctrl Nov 28, 2025

Choose a reason for hiding this comment

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

HAL_PMU_EnableAudio() need it, this func under $root_dir/module/hal/sifli

Copy link
Contributor

Choose a reason for hiding this comment

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

check #100138 for pmu reigster access

Copy link
Author

@ljq-ctrl ljq-ctrl Nov 28, 2025

Choose a reason for hiding this comment

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

here is this PMU functuon under module/hal/sifli/

HAL_StatusTypeDef HAL_PMU_EnableAudio(int enable)
{
    if (enable)
        hwp_pmuc->HXT_CR1 |= PMUC_HXT_CR1_BUF_AUD_EN;
    else
        hwp_pmuc->HXT_CR1 &= (~PMUC_HXT_CR1_BUF_AUD_EN);
    return HAL_OK;
}

move this function to this file and use sys_write32() ? this will access hwp_pmuc directly
it same that, it better to add audio clock in clock device tree and access PMUC_HXT_CR1_BUF_AUD_EN in clocl_control_sf32lb_rcc.c. codec driver only need to enable the clock by DT

#include <register.h>
#include <bf0_hal_pmu.h>

LOG_MODULE_REGISTER(siflicodec, CONFIG_AUDIO_CODEC_LOG_LEVEL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LOG_MODULE_REGISTER(siflicodec, CONFIG_AUDIO_CODEC_LOG_LEVEL);
LOG_MODULE_REGISTER(sifli_codec, CONFIG_AUDIO_CODEC_LOG_LEVEL);


static const struct device *codec_dev;

static int config_dac_path(AUDCODEC_TypeDef *reg, uint16_t bypass)
Copy link
Contributor

Choose a reason for hiding this comment

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

change param coz register access is changed with upper comments

Copy link
Author

Choose a reason for hiding this comment

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

AUDCODEC_TypeDef was defined in $root_dir/module/hal/sifli

Copy link
Author

@ljq-ctrl ljq-ctrl Nov 28, 2025

Choose a reason for hiding this comment

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

a previous version redefined a struct for all register, but it is recommended to use register.h,. so many registers for audio, unless redefined all register as dma_ sf32lb. c, which is prone to errors

switch (channel) {
case 0:
reg->DAC_CH0_CFG = (1 << AUDCODEC_DAC_CH0_CFG_ENABLE_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DOUT_MUTE_Pos)
Copy link
Contributor

Choose a reason for hiding this comment

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

using FIELD_PREP instead

Comment on lines +381 to +421
| (2 << AUDCODEC_DAC_CH0_CFG_DEM_MODE_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DMA_EN_Pos)
| (6 << AUDCODEC_DAC_CH0_CFG_ROUGH_VOL_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_FINE_VOL_Pos)
| (1 << AUDCODEC_DAC_CH0_CFG_DATA_FORMAT_Pos)
| (dac_clk->sinc_gain << AUDCODEC_DAC_CH0_CFG_SINC_GAIN_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DITHER_GAIN_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DITHER_EN_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_CLK_ANA_POL_Pos);

reg->DAC_CH0_CFG_EXT = (1 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_EN_Pos)
| (1 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_MODE_Pos)
| (1 << AUDCODEC_DAC_CH0_CFG_EXT_ZERO_ADJUST_EN_Pos)
| (2 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_INTERVAL_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_STAT_Pos);

reg->DAC_CH0_DEBUG = (0 << AUDCODEC_DAC_CH0_DEBUG_BYPASS_Pos)
| (0xFF << AUDCODEC_DAC_CH0_DEBUG_DATA_OUT_Pos);

break;
case 1:
reg->DAC_CH1_CFG = (1 << AUDCODEC_DAC_CH0_CFG_ENABLE_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DOUT_MUTE_Pos)
| (2 << AUDCODEC_DAC_CH0_CFG_DEM_MODE_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DMA_EN_Pos)
| (6 << AUDCODEC_DAC_CH0_CFG_ROUGH_VOL_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_FINE_VOL_Pos)
| (1 << AUDCODEC_DAC_CH0_CFG_DATA_FORMAT_Pos)
| (dac_clk->sinc_gain << AUDCODEC_DAC_CH0_CFG_SINC_GAIN_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DITHER_GAIN_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_DITHER_EN_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_CLK_ANA_POL_Pos);

reg->DAC_CH1_CFG_EXT = (1 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_EN_Pos)
| (1 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_MODE_Pos)
| (1 << AUDCODEC_DAC_CH0_CFG_EXT_ZERO_ADJUST_EN_Pos)
| (2 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_INTERVAL_Pos)
| (0 << AUDCODEC_DAC_CH0_CFG_EXT_RAMP_STAT_Pos);

reg->DAC_CH1_DEBUG = (0 << AUDCODEC_DAC_CH0_DEBUG_BYPASS_Pos)
| (0xFF << AUDCODEC_DAC_CH0_DEBUG_DATA_OUT_Pos);
Copy link
Contributor

Choose a reason for hiding this comment

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

same, using FIELD_PREP instead

Copy link
Author

@ljq-ctrl ljq-ctrl Nov 28, 2025

Choose a reason for hiding this comment

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

these was ported from other platform, careful and meticulous modifications are required

} else {
LOG_ERR("dma err");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

empty line, run clang format

Comment on lines +1508 to +1509
static struct sf32lb_audcodec_data data##n = { \
}; \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static struct sf32lb_audcodec_data data##n = { \
}; \
static struct sf32lb_audcodec_data data##n; \

ljq-ctrl and others added 2 commits November 29, 2025 09:28
audio playback and capure, extend API in codec.h

Signed-off-by: ljq-ctrl <[email protected]>
Add audio codec in board device tree, including PA control pin.

Signed-off-by: Gang He <[email protected]>
Add audio loop back sample

Signed-off-by: ljq-ctrl <[email protected]>
@sonarqubecloud
Copy link

@@ -0,0 +1,30 @@
.. zephyr:code-sample:: codec
:name: codec
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:name: codec
:name: audio codec

sample:
description: onchip audio codec example
application
name: codec
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: codec
name: audio codec

@JarmouniA JarmouniA dismissed their stale review November 29, 2025 10:32

Addressed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants