i2s support for esp32 and esp32s2 #83710

wmrsouza · 2025-01-08T19:52:03Z

This PR adds i2s support to those SoCs lacking GDMA:

ESP32
ESP32-S2

zephyrbot · 2025-02-12T13:47:56Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_espressif	zephyrproject-rtos/hal_espressif@`a459b40`	zephyrproject-rtos/hal_espressif@`d64439e` (`zephyr`)	zephyrproject-rtos/[email protected]

✅ All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

sylvioalves · 2025-02-13T10:28:47Z

drivers/i2s/i2s_esp32.c


 #include <zephyr/logging/log.h>
 #include <zephyr/irq.h>

 #include <esp_clk_tree.h>
 #include <hal/i2s_hal.h>

+#if !SOC_GDMA_SUPPORTED
+#include <soc/lldesc.h>


Can you move this to line 20 to use a single #if !SOC_GDMA_SUPPORTED?

sylvioalves · 2025-02-13T10:29:04Z

drivers/i2s/i2s_esp32.c

+#if !SOC_GDMA_SUPPORTED
+#include <soc/lldesc.h>
+#endif /* !SOC_GDMA_SUPPORTED */
+
 LOG_MODULE_REGISTER(i2s_esp32, CONFIG_I2S_LOG_LEVEL);

 #if !SOC_GDMA_SUPPORTED


Perhaps this #ifdef is not necessary, just leave the definition in there.

sylvioalves · 2025-02-13T10:32:47Z

drivers/i2s/i2s_esp32.c

@@ -12,18 +12,28 @@
 #include <soc.h>
 #include <zephyr/drivers/clock_control.h>
 #include <zephyr/drivers/pinctrl.h>
+#include <zephyr/devicetree.h>
+
+#if !SOC_GDMA_SUPPORTED


What is SOC_GDMA_SUPPORTED is set per SoC but dma is not enabled, i.e, app is using descriptors instead?

This PR does not implement dma transfers using descriptor on SoCs that support GDMA. It is only made for those SoCs that do not support GDMA. Since a dma instance is required to perform i2s transfers on SoCs that support GDMA, the build process will fail. I will provide an preprocessor error conditional on SOC_GDMA_SUPPORTED and !DT_HAS_COMPAT_STATUS_OKAY(espressif_esp32_gdma) for the cases where dma has been disable:

#if SOC_GDMA_SUPPORTED && !DT_HAS_COMPAT_STATUS_OKAY(espressif_esp32_gdma) #error "DMA peripheral is not enabled!" #endif /* SOC_GDMA_SUPPORTED */

Even so I bilieve the failure will continue to occur despite the error message being issued

sylvioalves · 2025-02-13T10:38:34Z

dts/xtensa/espressif/esp32s3/esp32s3_common.dtsi

@@ -310,8 +310,6 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0x6000f000 0x1000>;
-			interrupts = <I2S0_INTR_SOURCE IRQ_DEFAULT_PRIORITY 0>;


Why was this removed from ESP32S3 and C3?

Initially I thought it was necessary, but now I see it's a mistake.
I'll revert it

sylvioalves · 2025-02-13T10:40:02Z

dts/xtensa/espressif/esp32/esp32_common.dtsi

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 Intel Corporation.
+ * Copyright (c) 2019-2025 Intel Corporation.


Instead of extending the copyright of the original creator, append a new line adding Espressif's. Do the same for all files that copyright was changed.

sylvioalves · 2025-02-13T10:41:03Z

dts/bindings/i2s/espressif,esp32-i2s.yaml

@@ -17,18 +17,6 @@ properties:
  clocks:
    required: true

-  interrupts:


interrupts really need to go?

interrupts along with interrupt-names was the way I found to inform to the driver during build time which i2s channels are enabled so those resources that are not being used due to a not used i2s channel can be not allocated or removed by optimization process. The same stuff is achieved by using dmas along with dma-names for the GDMA SoCs .

sylvioalves · 2025-02-13T10:51:52Z

samples/drivers/i2s/echo/boards/esp32_devkitc_wrover_procpu.overlay

+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+


remove extra line

same for the other files.

sylvioalves · 2025-02-13T10:52:59Z

samples/drivers/i2s/echo/boards/esp32_devkitc_wrover_procpu.overlay

+i2s_rxtx: &i2s0 {
+	status = "okay";
+
+	interrupts = <I2S0_INTR_SOURCE IRQ_DEFAULT_PRIORITY 0>,


interrupts are already defined in .dtsi. Is it needed again here?

same for the other files

interrupts along with interrupt-names was the way I found to inform to the driver during build time which i2s channels are enabled so those resources that are not being used due to a not used i2s channel can be not allocated or removed by optimization process.

sylvioalves · 2025-02-13T10:57:28Z

samples/drivers/i2s/echo/Kconfig

@@ -17,3 +17,6 @@ config STOP_START_STREAMS_SW1
 	depends on $(dt_alias_enabled,sw1)
 	select GPIO
 	default y
+
+config DOESNT_SUPPORT_I2S_DIR_BOTH


Suggest I2S_DIR_BOTH_NOT_SUPPORTED

Are you sure we can't have I2S_DIR_BOTH implemented in the I2S driver?

I'll rework the i2s_esp32_trigger() and the ì2s_esp32_configure() functions to accept I2S_DIR_BOTH and revert the changes made on echo sample

raffarost · 2025-02-19T12:27:58Z

boards/espressif/esp32_devkitc_wroom/esp32_devkitc_wroom-pinctrl.dtsi

+				 <I2S0_O_SD_GPIO7>,
+				 <I2S0_I_WS_GPIO15>,
+				 <I2S0_I_BCK_GPIO16>;
+			output-enable;


are these the right flags? (usually used for loopback)

I think they are correct

Shouldn't the input pins be placed in group2?

In fact no. In this configuration they are output pins connected to the input/rx path. In other configuration they could be driven by output sources, but in this conf they will provide output signals.

raffarost · 2025-02-19T13:31:51Z

drivers/i2s/i2s_esp32.c

+	const struct i2s_esp32_stream *stream;
+	int err;
+
+	switch (dir) {


if you use a fallthrough between RX/TX (and the appropriate checks for break instructions), it's possible to avoid repeated code (suggestion only), for I2S_DIR_BOTH

raffarost · 2025-02-19T13:37:12Z

dts/xtensa/espressif/esp32/esp32_common.dtsi

+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x3ff4f000 0x1000>;
+			interrupts = <I2S0_INTR_SOURCE IRQ_DEFAULT_PRIORITY 0>,


is it necessary to declare the same source twice (could it be avoided)?

that was the way I found to save resources (throughout interrup-names and dma-names)
as the interrupt source is the same for TX and RX I had to declare the same source for each channel.

Code and data refactoring to prepare for adding support for non-gdma SoCs Signed-off-by: Marcio Ribeiro <[email protected]>

hal_espressif revision update to add i2s support for esp32 and esp32s2 Signed-off-by: Marcio Ribeiro <[email protected]>

Adds support for: - esp32 - esp32s2 Signed-off-by: Marcio Ribeiro <[email protected]>

Adds i2s support for boards based on: - esp32 - esp32s2 Signed-off-by: Marcio Ribeiro <[email protected]>

Adds esp32 board overlay files to i2s samples Signed-off-by: Marcio Ribeiro <[email protected]>

wmrsouza force-pushed the feature/i2s_no_gdma branch from af5e945 to 60d7026 Compare January 11, 2025 10:41

wmrsouza force-pushed the feature/i2s_no_gdma branch 2 times, most recently from 4431669 to 045619f Compare January 21, 2025 15:39

wmrsouza force-pushed the feature/i2s_no_gdma branch from 045619f to 652e2aa Compare February 12, 2025 13:46

zephyrbot added the manifest label Feb 12, 2025

wmrsouza marked this pull request as ready for review February 12, 2025 13:47

zephyrbot added manifest-hal_espressif DNM (manifest) This PR should not be merged (controlled by action-manifest) area: I2S area: Xtensa Xtensa Architecture area: RISCV RISCV Architecture (32-bit & 64-bit) platform: ESP32 Espressif ESP32 area: Pinctrl labels Feb 12, 2025

zephyrbot requested review from anangl, carlocaione, edersondisouza, fkokosinski, gmarull, katsuster, kgugala, LucasTambor, marekmatej, mgielda, npitre, raffarost, sylvioalves, tgorochowik and uLipe February 12, 2025 13:48

zephyrbot assigned gmarull Feb 12, 2025

sylvioalves reviewed Feb 13, 2025

View reviewed changes

wmrsouza force-pushed the feature/i2s_no_gdma branch 2 times, most recently from d27f92c to 33d75c2 Compare February 14, 2025 02:41

wmrsouza requested a review from sylvioalves February 14, 2025 02:44

wmrsouza force-pushed the feature/i2s_no_gdma branch from 33d75c2 to e11d6dc Compare February 14, 2025 04:37

gmarull assigned anangl and unassigned gmarull Feb 17, 2025

gmarull removed their request for review February 17, 2025 09:03

wmrsouza force-pushed the feature/i2s_no_gdma branch 3 times, most recently from 8bcbe65 to 17e904c Compare February 19, 2025 05:09

raffarost reviewed Feb 19, 2025

View reviewed changes

wmrsouza added 3 commits February 20, 2025 09:35

drivers: i2s: esp32: code and data refactoring

b9bd48b

Code and data refactoring to prepare for adding support for non-gdma SoCs Signed-off-by: Marcio Ribeiro <[email protected]>

west.yml: esp32: hal_espressif revision update

6404018

hal_espressif revision update to add i2s support for esp32 and esp32s2 Signed-off-by: Marcio Ribeiro <[email protected]>

drivers: i2s: esp32: add support for non-gdma SoCs

ab8ab76

Adds support for: - esp32 - esp32s2 Signed-off-by: Marcio Ribeiro <[email protected]>

wmrsouza force-pushed the feature/i2s_no_gdma branch from 17e904c to 73f2fd1 Compare February 20, 2025 12:39

zephyrbot removed the DNM (manifest) This PR should not be merged (controlled by action-manifest) label Feb 20, 2025

wmrsouza added 2 commits February 20, 2025 13:46

boards: esp32: add i2s for non-gdma SoC boards

ab703b8

Adds i2s support for boards based on: - esp32 - esp32s2 Signed-off-by: Marcio Ribeiro <[email protected]>

samples: i2s: add esp32 board overlay files

97c1a72

Adds esp32 board overlay files to i2s samples Signed-off-by: Marcio Ribeiro <[email protected]>

wmrsouza force-pushed the feature/i2s_no_gdma branch from 73f2fd1 to 97c1a72 Compare February 20, 2025 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

i2s support for esp32 and esp32s2 #83710

i2s support for esp32 and esp32s2 #83710

wmrsouza commented Jan 8, 2025 •

edited

Loading

zephyrbot commented Feb 12, 2025 •

edited

Loading

sylvioalves Feb 13, 2025

sylvioalves Feb 13, 2025 •

edited

Loading

sylvioalves Feb 13, 2025

wmrsouza Feb 13, 2025

sylvioalves Feb 13, 2025

wmrsouza Feb 13, 2025

sylvioalves Feb 13, 2025

sylvioalves Feb 13, 2025

wmrsouza Feb 13, 2025

sylvioalves Feb 13, 2025

sylvioalves Feb 13, 2025

sylvioalves Feb 13, 2025

sylvioalves Feb 13, 2025

wmrsouza Feb 13, 2025

sylvioalves Feb 13, 2025

sylvioalves Feb 13, 2025

wmrsouza Feb 13, 2025 •

edited

Loading

raffarost Feb 19, 2025

wmrsouza Feb 19, 2025

LucasTambor Feb 19, 2025

wmrsouza Feb 19, 2025

raffarost Feb 19, 2025 •

edited

Loading

raffarost Feb 19, 2025

wmrsouza Feb 19, 2025

i2s support for esp32 and esp32s2 #83710

Are you sure you want to change the base?

i2s support for esp32 and esp32s2 #83710

Conversation

wmrsouza commented Jan 8, 2025 • edited Loading

zephyrbot commented Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

sylvioalves Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wmrsouza Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raffarost Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wmrsouza commented Jan 8, 2025 •

edited

Loading

zephyrbot commented Feb 12, 2025 •

edited

Loading

sylvioalves Feb 13, 2025 •

edited

Loading

wmrsouza Feb 13, 2025 •

edited

Loading

raffarost Feb 19, 2025 •

edited

Loading