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

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

Closed
3 tasks done
PilnyTomas opened this issue Dec 6, 2023 · 1 comment
Closed
3 tasks done
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally

Comments

@PilnyTomas
Copy link

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

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:

hw->conf.tx_right_first = enable;

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;

@PilnyTomas PilnyTomas changed the title i2s_ll_enable_builtin_dac setting tx_right_first with enable aprameter i2s_ll_enable_builtin_dac setting tx_right_first with enable parameter Dec 6, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Dec 6, 2023
@github-actions github-actions bot changed the title i2s_ll_enable_builtin_dac setting tx_right_first with enable parameter i2s_ll_enable_builtin_dac setting tx_right_first with enable parameter (IDFGH-11625) Dec 6, 2023
@L-KAYA
Copy link
Collaborator

L-KAYA commented Dec 7, 2023

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

@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Opened Issue is new Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed labels Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

3 participants