espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.48k stars 7.26k forks source link

i2s_ll_enable_builtin_dac setting tx_right_first with enable parameter (IDFGH-11625) #12741

Closed PilnyTomas closed 10 months ago

PilnyTomas commented 10 months ago

Answers checklist.

General issue report

This is a minor issue. No need to rush. branch: master, release/v5.2, release/v5.1, release/v5.0 file: components/hal/esp32/include/hal/i2s_ll.h function: i2s_ll_enable_builtin_dac link: https://github.com/espressif/esp-idf/blob/30870c819f8bf1be1c8a3b4360be8174a280b40c/components/hal/esp32/include/hal/i2s_ll.h#L1161 Description The parameter enable is described as Set true to enable build in DAC The reg field tx_right_first is being set by the parameter enable. According to the TRM: Set this bit to transmit right-channel data first. This reg field is already controlled by function i2s_ll_tx_enable_right_first Conclusion: I suggested changing the line hw->conf.tx_right_first = enable; to hw->conf.tx_right_first = 0;

L-KAYA commented 10 months ago

tx_right_first is set by i2s_ll_tx_enable_right_first in dac_dma.c, as well as tx_msb_shift and tx_short_sync, so these redundant configuration in i2s_ll_enable_builtin_dac will be removed.

Thanks for your suggestion! @PilnyTomas