arch/xtensa: Add common I2S support for esp32[- |-s2|-s3]

[eren-terzioglu]

Summary

I2S driver changed to a common layer approach to fix RX/TX concurrency working problems on esp32[-|-s2] and fixing bugs easier with using one/common implementation instead of dealing to work on every chip register by register. This approach helps to increase testability with using same configuration values and minimizing potentially missing bugs for each chip. Also we had more capability to implement different modes (e.g. STD, PDM) if supported by hardware.

• arch/xtensa/esp32[s3]: Add more dma functions

• arch/esp32[s2|s3]: Add common I2S arch layer support

• boards/esp32[s2|s3]: Add common I2S board layer support

• arch/esp[s2|s3]: Update common layer version

• Documentation/esp32s3: Add I2S defconfig documentation

• boards/xtensa: Remove legacy I2S implementation for esp32[-|-s2|s3]

• arch/xtensa: Remove legacy I2S implementation for esp32[-|-s2|s3]

Impact

Impact on user: No, they can use their defconfigs.

Impact on build: No, Current I2S can be used without any issue

Impact on hardware: Yes, but new I2S interface can work with old defconfigs

Impact on documentation: Yes, missing i2schar defconfigs doc added for esp32s3

Impact on security: No

Impact on compatibility: No, older defconfigs should work fine

Testing

Building

Build command for esp32

make -j distclean && ./tools/configure.sh esp32-devkitc:i2schar
&& kconfig-tweak -d CONFIG_ESP32_I2S0 && kconfig-tweak -d CONFIG_ESP32_I2S1 && kconfig-tweak -e CONFIG_ESPRESSIF_I2S && kconfig-tweak -e CONFIG_ESPRESSIF_I2S0 && kconfig-tweak -e CONFIG_ESPRESSIF_I2S_MCLK && kconfig-tweak -e CONFIG_ESPRESSIF_I2S0_MCLK && make olddefconfig && make flash EXTRAFLAGS="-Wno-cpp -Werror" ESPTOOL_BINDIR=./ ESPTOOL_PORT=/dev/ttyUSB0 -s -j$(nproc) && minicom

Build command for esp32s2

make -j distclean && ./tools/configure.sh esp32s2-saola-1:i2schar && kconfig-tweak -d CONFIG_ESP32S2_I2S0 && kconfig-tweak -d CONFIG_ESP32S2_I2S1 && kconfig-tweak -e CONFIG_ESPRESSIF_I2S && kconfig-tweak -e CONFIG_ESPRESSIF_I2S0 && kconfig-tweak -e CONFIG_ESPRESSIF_I2S_MCLK && kconfig-tweak -e CONFIG_ESPRESSIF_I2S0_MCLK && make olddefconfig && make flash EXTRAFLAGS="-Wno-cpp -Werror" ESPTOOL_BINDIR=./ ESPTOOL_PORT=/dev/ttyUSB0 -s -j$(nproc) && minicom

Build command for esp32s3

make -j distclean && ./tools/configure.sh esp32s3-devkit:nsh
&& kconfig-tweak -e CONFIG_ESPRESSIF_I2S && kconfig-tweak -e CONFIG_ESPRESSIF_I2S0 && kconfig-tweak -e CONFIG_ESPRESSIF_I2S_MCLK && kconfig-tweak -e CONFIG_ESPRESSIF_I2S0_MCLK && kconfig-tweak -e CONFIG_AUDIO && kconfig-tweak -e CONFIG_AUDIO_I2S && kconfig-tweak -e CONFIG_AUDIO_I2SCHAR && kconfig-tweak -e CONFIG_DRIVERS_AUDIO && kconfig-tweak -e CONFIG_EXAMPLES_I2SCHAR && kconfig-tweak -e CONFIG_EXAMPLES_I2SCHAR_RX && kconfig-tweak -e CONFIG_EXAMPLES_I2SCHAR_TX && make olddefconfig && make flash EXTRAFLAGS="-Wno-cpp -Werror" ESPTOOL_BINDIR=./ ESPTOOL_PORT=/dev/ttyUSB0 -s -j$(nproc) && minicom

Running

After build step Dout (esp32/esp32s3: 18, esp32s2: 36 default) and Din (esp32/esp32s3: 19, esp32s2: 37 default) pins connected together via wire (or pointed to the same pin in menuconfig). After that i2schar command used to test it.

Results

Expected output should be like this:

nsh> i2schar
i2schar_main: Start receiver thread
i2schar_read: buffer=0x40815678 buflen=57
i2s_receive: Prepared 5 bytes to receive DMA buffers
Recieved Audio pipeline buffer: (0x408156ac):
i2schar_read: buffer=0x408156b8 buflen=57
i2s_receive: Prepared 5 bytes to receive DMA buffers
Recieved Audio pipeline buffer: (0x408156ec):
i2schar_write: buffer=0x408134b8 buflen=57
i2s_tx_worker: tx.act.head=0 tx.done.head=0x4080bce0
i2schar_txcallback: apb=0x408134b8 nbytes=5 result=0
i2schar_txcallback: Freeing apb=0x408134b8 crefs=2
i2s_send: Queued 5 bytes into DMA buffers
Audio pipeline buffer: (0x408134ec):
0000  00 01 02 03 04                                   .....           
i2schar_write: buffer=0x408134f8 buflen=57
i2s_tx_worker: tx.act.head=0 tx.done.head=0x4080bce0
i2schar_txcallback: apb=0x408134f8 nbytes=5 result=0
i2schar_txcallback: Freeing apb=0x408134f8 crefs=2
i2s_send: Queued 5 bytes into DMA buffers
Audio pipeline buffer: (0x4081352c):
0000  05 06 07 08 09                                   .....           
i2schar_main: Start transmitter thread
i2schar_receiver: Received buffer 1
i2schar_receiver: Received buffer 2
i2schar_main: Waiting for the transmitter thread
i2schar_transmitter: Send buffer 1
i2schar_transmitter: Send buffer 2
i2schar_main: Waiting for the receiver thread

Note: Default pins for devices:

Device/Pin	MCLK	BCLK	WS	DIN	DOUT
ESP32	0	4	5	19	18
ESP32-S2	33	35	34	37	36
ESP32-S3	0	4	5	19	18

[nuttxpr]

[Experimental Bot, please feedback here]

This PR description mostly meets the NuttX requirements but could be improved in a few areas.

Strengths:

• Clear summary of the "why": The rationale for refactoring the I2S driver is well-explained, focusing on maintainability, testability, and potential for future feature expansion.
• Good overview of changed components: The list of affected files/directories gives a good sense of the scope of the changes.
• Testing information is detailed: Build commands, connection instructions, expected output, and default pin assignments are provided, making it easier for reviewers to reproduce the tests.

Weaknesses & Suggestions:

• Summary - Lack of concise "what": While the "why" is clear, the "what" could be more concise. Instead of listing changed files, briefly summarize the core change: "Refactors the ESP32, ESP32-S2, and ESP32-S3 I2S drivers to use a common architecture layer."
• Summary - "How" is too high-level: The description of how the change works focuses on benefits rather than the technical implementation. Briefly mention the key technical aspects of the common layer approach. For example, "Abstracts hardware-specific register access into a unified API."
• Impact - Be more specific: Avoid simply stating "No" or "Yes." Provide brief explanations even if the impact is minimal. Examples:
  • Impact on user: "No, existing defconfigs will continue to work as expected."
  • Impact on hardware: "Yes, the I2S interface is refactored, but backward compatibility is maintained with existing defconfigs."
  • Impact on documentation: "Yes, added documentation for the i2schar defconfig for ESP32-S3."
• Testing - Logs before/after change are missing: The template specifically requests logs before and after the change. Include these to demonstrate the issue being fixed and the improvement achieved by the change. Even simple logs demonstrating the previous buggy behavior and the corrected behavior are valuable.
• Testing - Host Information Missing: Provide details about the build host environment used for testing (OS, CPU architecture, compiler version).

Revised Example (incorporating suggestions):

Summary

Refactors the ESP32, ESP32-S2, and ESP32-S3 I2S drivers to use a common architecture layer to address RX/TX concurrency issues and improve maintainability and testability. This common layer abstracts hardware-specific register access into a unified API, enabling easier bug fixing and future feature expansion (e.g., supporting different I2S modes).

Related NuttX Issue (If applicable)

Impact

• Impact on user: No, existing defconfigs will continue to work as expected.
• Impact on build: No, the build process remains unchanged for existing I2S configurations.
• Impact on hardware: Yes, the I2S interface is refactored, but backward compatibility is maintained with existing defconfigs.
• Impact on documentation: Yes, added documentation for the i2schar defconfig for ESP32-S3.
• Impact on security: No, this change does not have security implications.
• Impact on compatibility: No, backward compatibility with older defconfigs is maintained.

Testing

I confirm that changes are verified on local setup and works as intended:

• Build Host(s): Ubuntu 20.04, x86_64, GCC 9.4.0
• Target(s): ESP32-DevKitC, ESP32-S2-Saola-1, ESP32-S3-DevKit

Testing logs before change:

(Logs demonstrating the RX/TX concurrency issue, e.g., garbled output, dropped data)

Testing logs after change:

(Logs from the "Results" section showing correct operation)

(Include Build commands and Running instructions as before)

By addressing these points, your PR will be clearer, easier to review, and more likely to be accepted. Remember to replace the placeholders (e.g., issue number, before/after logs) with the actual information.

[lupyuen]

@eren-terzioglu Please check the conflict. Thanks :-)