Skip to content

Commit

Permalink
Merge branch 'fix/fix_p4_sdspi_v5.3' into 'release/v5.3'
Browse files Browse the repository at this point in the history
sdspi: fix p4 sdspi (v5.3)

See merge request espressif/esp-idf!33189
  • Loading branch information
suda-morris committed Sep 4, 2024
2 parents 20975fb + fa13b31 commit cea789d
Show file tree
Hide file tree
Showing 18 changed files with 140 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ menu "SDMMC Test Board Configuration"
bool "ESP32-P4 Function EV Board"
depends on IDF_TARGET_ESP32P4

config SDMMC_BOARD_ESP32P4_EV_BOARD_WITH_SDSPI
bool "ESP32-P4 Function EV Board with SDSPI breakout"
depends on IDF_TARGET_ESP32P4

config SDMMC_BOARD_CUSTOM_SD
depends on SOC_SDMMC_HOST_SUPPORTED
bool "Custom SD (choose pins)"
Expand Down Expand Up @@ -133,6 +137,8 @@ menu "SDMMC Test Board Configuration"

config SDMMC_BOARD_CUSTOM_UNUSED
int "GPIO not routed on the board"
default 34 if IDF_TARGET_ESP32P4
default 8 if IDF_TARGET_ESP32C5
default -1

endmenu
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,34 @@ static const sdmmc_test_board_info_t s_board_info = {
},
};

#elif CONFIG_SDMMC_BOARD_ESP32P4_EV_BOARD_WITH_SDSPI

static const sdmmc_test_board_info_t s_board_info = {
.name = "ESP32-P4 Function EV Board with SDSPI breakout",
.slot = {
{
.slot_exists = false
},
{
.slot_exists = true,
.bus_width = 1,
.clk = 53,
.cmd_mosi = 36,
.d0_miso = 47,
.d1 = GPIO_NUM_NC,
.d2 = GPIO_NUM_NC,
.d3_cs = 33,
.d4 = GPIO_NUM_NC,
.d5 = GPIO_NUM_NC,
.d6 = GPIO_NUM_NC,
.d7 = GPIO_NUM_NC,
.cd = CONFIG_SDMMC_BOARD_CUSTOM_CD,
.wp = CONFIG_SDMMC_BOARD_CUSTOM_WP,
.unused_pin = CONFIG_SDMMC_BOARD_CUSTOM_UNUSED,
}
},
};

#elif CONFIG_SDMMC_BOARD_CUSTOM_SD

static const sdmmc_test_board_info_t s_board_info = {
Expand Down
5 changes: 2 additions & 3 deletions components/esp_driver_sdspi/test_apps/.build-test-rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ components/esp_driver_sdspi/test_apps/sdspi:
disable:
- if: SOC_GPSPI_SUPPORTED != 1
disable_test:
- if: SOC_GPSPI_SUPPORTED == 1
temporary: true
reason: will add runners later # TODO: IDF-8747
- if: IDF_TARGET not in ["esp32", "esp32s3", "esp32c3", "esp32p4"]
reason: needs special runner, select few typical targets for testing
depends_components:
- sdmmc
- esp_driver_sdspi
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -11,6 +11,8 @@
#include "sdmmc_test_board.h"
#include "sdmmc_test_begin_end_spi.h"
#include "sdmmc_test_cd_wp_common.h"
#include "sd_pwr_ctrl.h"
#include "sd_pwr_ctrl_by_on_chip_ldo.h"

TEST_CASE("CD input works in SPI mode", "[sdspi]")
{
Expand All @@ -22,6 +24,18 @@ TEST_CASE("CD input works in SPI mode", "[sdspi]")
sdmmc_test_board_get_config_sdspi(slot, &config, &bus_config, &dev_config);
const int test_gpio = sdmmc_test_board_get_slot_info(slot)->unused_pin;
dev_config.gpio_cd = test_gpio;

#if SOC_SDMMC_IO_POWER_EXTERNAL
#define SDMMC_PWR_LDO_CHANNEL 4
sd_pwr_ctrl_ldo_config_t ldo_config = {
.ldo_chan_id = SDMMC_PWR_LDO_CHANNEL,
};
sd_pwr_ctrl_handle_t pwr_ctrl_handle = NULL;

TEST_ESP_OK(sd_pwr_ctrl_new_on_chip_ldo(&ldo_config, &pwr_ctrl_handle));
config.pwr_ctrl_handle = pwr_ctrl_handle;
#endif

sdmmc_test_board_card_power_set(true);
TEST_ESP_OK(spi_bus_initialize(dev_config.host_id, &bus_config, SPI_DMA_CH_AUTO));
TEST_ESP_OK(sdspi_host_init());
Expand All @@ -34,6 +48,9 @@ TEST_CASE("CD input works in SPI mode", "[sdspi]")
TEST_ESP_OK(sdspi_host_deinit());
TEST_ESP_OK(spi_bus_free(SDSPI_DEFAULT_HOST));
sdmmc_test_board_card_power_set(false);
#if SOC_SDMMC_IO_POWER_EXTERNAL
TEST_ESP_OK(sd_pwr_ctrl_del_on_chip_ldo(pwr_ctrl_handle));
#endif
}

TEST_CASE("WP input works in SPI mode", "[sdspi]")
Expand All @@ -48,6 +65,16 @@ TEST_CASE("WP input works in SPI mode", "[sdspi]")
dev_config.gpio_wp = test_gpio;
sdmmc_test_board_card_power_set(true);
TEST_ESP_OK(spi_bus_initialize(dev_config.host_id, &bus_config, SPI_DMA_CH_AUTO));
#if SOC_SDMMC_IO_POWER_EXTERNAL
#define SDMMC_PWR_LDO_CHANNEL 4
sd_pwr_ctrl_ldo_config_t ldo_config = {
.ldo_chan_id = SDMMC_PWR_LDO_CHANNEL,
};
sd_pwr_ctrl_handle_t pwr_ctrl_handle = NULL;

TEST_ESP_OK(sd_pwr_ctrl_new_on_chip_ldo(&ldo_config, &pwr_ctrl_handle));
config.pwr_ctrl_handle = pwr_ctrl_handle;
#endif
TEST_ESP_OK(sdspi_host_init());
TEST_ESP_OK(sdspi_host_init_device(&dev_config, &handle));

Expand All @@ -58,4 +85,7 @@ TEST_CASE("WP input works in SPI mode", "[sdspi]")
TEST_ESP_OK(sdspi_host_deinit());
TEST_ESP_OK(spi_bus_free(SDSPI_DEFAULT_HOST));
sdmmc_test_board_card_power_set(false);
#if SOC_SDMMC_IO_POWER_EXTERNAL
TEST_ESP_OK(sd_pwr_ctrl_del_on_chip_ldo(pwr_ctrl_handle));
#endif
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -18,6 +18,7 @@ static void do_one_sdspi_probe(int slot, int freq_khz)
sdmmc_card_print_info(stdout, &card);
uint8_t* buffer = heap_caps_calloc(512, 1, MALLOC_CAP_DMA);
TEST_ESP_OK(sdmmc_read_sectors(&card, buffer, 0, 1));
free(buffer);
sdmmc_test_spi_end(slot, &card);
}

Expand All @@ -32,21 +33,16 @@ TEST_CASE("sdspi probe, slot 0, HS", "[sdspi]")
do_one_sdspi_probe(SLOT_0, SDMMC_FREQ_HIGHSPEED);
}

#if !CONFIG_IDF_TARGET_ESP32 && !CONFIG_IDF_TARGET_ESP32S3
//TODO: IDF-8750. Leaks too much memory, needs check
TEST_CASE("sdspi probe, slot 1", "[sdspi]")
{
do_one_sdspi_probe(SLOT_1, SDMMC_FREQ_PROBING);
do_one_sdspi_probe(SLOT_1, SDMMC_FREQ_DEFAULT);
do_one_sdspi_probe(SLOT_1, SDMMC_FREQ_CUSTOM_10M);
}
#endif

#if !CONFIG_IDF_TARGET_ESP32 && !CONFIG_IDF_TARGET_ESP32S3
//TODO: IDF-8749
//here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
TEST_CASE("sdspi probe, slot 1, HS", "[sdspi]")
{
//TODO: IDF-8749
//here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
do_one_sdspi_probe(SLOT_1, SDMMC_FREQ_DEFAULT);
}
#endif
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -26,14 +26,12 @@ TEST_CASE("sdspi read/write performance, slot 0", "[sdspi]")
do_one_sdspi_perf_test(SLOT_0, SDMMC_FREQ_HIGHSPEED);
}

#if !CONFIG_IDF_TARGET_ESP32 && !CONFIG_IDF_TARGET_ESP32S3
//TODO: IDF-8749
//here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
TEST_CASE("sdspi read/write performance, slot 1", "[sdspi]")
{
//TODO: IDF-8749
//here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
do_one_sdspi_perf_test(SLOT_1, SDMMC_FREQ_DEFAULT);
}
#endif

/* ========== Read/write tests with offset, SPI ========== */

Expand All @@ -52,14 +50,12 @@ TEST_CASE("sdspi read/write performance with offset, slot 0", "[sdspi]")
do_one_sdspi_rw_test_with_offset(SLOT_0, SDMMC_FREQ_HIGHSPEED);
}

#if !CONFIG_IDF_TARGET_ESP32 && !CONFIG_IDF_TARGET_ESP32S3
//TODO: IDF-8749
//here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
TEST_CASE("sdspi read/write performance with offset, slot 1", "[sdspi]")
{
//TODO: IDF-8749
//here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
do_one_sdspi_rw_test_with_offset(SLOT_1, SDMMC_FREQ_DEFAULT);
}
#endif

/* ========== Read/write tests with unaligned source/destination buffer, SPI ========== */

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@

void setUp(void)
{
printf("%s", ""); /* sneakily lazy-allocate the reent structure for this test task */
unity_utils_record_free_mem();
}

void tearDown(void)
{
esp_reent_cleanup();
unity_utils_evaluate_leaks_direct(TEST_MEMORY_LEAK_THRESHOLD);
}

Expand Down
11 changes: 8 additions & 3 deletions components/esp_driver_sdspi/test_apps/sdspi/pytest_sdspi.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
# SPDX-License-Identifier: CC0-1.0

import pytest
from pytest_embedded_idf import IdfDut


@pytest.mark.esp32
@pytest.mark.esp32c3
@pytest.mark.esp32s3
@pytest.mark.esp32p4
@pytest.mark.sdcard_spimode
def test_sdspi(dut: IdfDut) -> None:
dut.run_all_single_board_cases()
dut.run_all_single_board_cases(reset=True)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CONFIG_SDMMC_BOARD_ESP32P4_EV_BOARD_WITH_SDSPI=y
CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG=y
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CONFIG_SDMMC_BOARD_ESP32S3_EMMC_TEST=y
CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG=y
23 changes: 15 additions & 8 deletions components/esp_driver_spi/src/gpspi/spi_master.c
Original file line number Diff line number Diff line change
Expand Up @@ -1149,15 +1149,22 @@ static SPI_MASTER_ISR_ATTR esp_err_t setup_priv_desc(spi_host_t *host, spi_trans
#endif
}

if (rcv_ptr && bus_attr->dma_enabled && (!esp_ptr_dma_capable(rcv_ptr) || rx_unaligned)) {
ESP_RETURN_ON_FALSE(!(trans_desc->flags & SPI_TRANS_DMA_BUFFER_ALIGN_MANUAL), ESP_ERR_INVALID_ARG, SPI_TAG, "Set flag SPI_TRANS_DMA_BUFFER_ALIGN_MANUAL but RX buffer addr&len not align to %d, or not dma_capable", alignment);
//if rxbuf in the desc not DMA-capable, or not aligned to alignment, malloc a new one
ESP_EARLY_LOGD(SPI_TAG, "Allocate RX buffer for DMA");
rx_byte_len = (rx_byte_len + alignment - 1) & (~(alignment - 1)); // up align alignment
rcv_ptr = heap_caps_aligned_alloc(alignment, rx_byte_len, MALLOC_CAP_DMA);
if (rcv_ptr == NULL) {
goto clean_up;
if (rcv_ptr && bus_attr->dma_enabled) {
if ((!esp_ptr_dma_capable(rcv_ptr) || rx_unaligned)) {
ESP_RETURN_ON_FALSE(!(trans_desc->flags & SPI_TRANS_DMA_BUFFER_ALIGN_MANUAL), ESP_ERR_INVALID_ARG, SPI_TAG, "Set flag SPI_TRANS_DMA_BUFFER_ALIGN_MANUAL but RX buffer addr&len not align to %d, or not dma_capable", alignment);
//if rxbuf in the desc not DMA-capable, or not aligned to alignment, malloc a new one
ESP_EARLY_LOGD(SPI_TAG, "Allocate RX buffer for DMA");
rx_byte_len = (rx_byte_len + alignment - 1) & (~(alignment - 1)); // up align alignment
rcv_ptr = heap_caps_aligned_alloc(alignment, rx_byte_len, MALLOC_CAP_DMA);
if (rcv_ptr == NULL) {
goto clean_up;
}
}
#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
// do invalid here to hold on cache status to avoid hardware auto write back during dma transaction
esp_err_t ret = esp_cache_msync((void *)rcv_ptr, rx_byte_len, ESP_CACHE_MSYNC_FLAG_DIR_M2C);
assert(ret == ESP_OK);
#endif
}
priv_desc->buffer_to_send = send_ptr;
priv_desc->buffer_to_rcv = rcv_ptr;
Expand Down
22 changes: 13 additions & 9 deletions components/esp_driver_spi/src/gpspi/spi_slave.c
Original file line number Diff line number Diff line change
Expand Up @@ -379,16 +379,20 @@ static esp_err_t SPI_SLAVE_ISR_ATTR spi_slave_setup_priv_trans(spi_host_device_t
esp_err_t ret = esp_cache_msync((void *)priv_trans->tx_buffer, buffer_byte_len, ESP_CACHE_MSYNC_FLAG_DIR_C2M);
ESP_RETURN_ON_FALSE_ISR(ESP_OK == ret, ESP_ERR_INVALID_STATE, SPI_TAG, "mem sync c2m(writeback) fail");
}
if (spihost[host]->dma_enabled && trans->rx_buffer && (!esp_ptr_dma_capable(trans->rx_buffer) || ((((uint32_t)trans->rx_buffer) | (trans->length + 7) / 8) & (alignment - 1)))) {
ESP_RETURN_ON_FALSE_ISR(trans->flags & SPI_SLAVE_TRANS_DMA_BUFFER_ALIGN_AUTO, ESP_ERR_INVALID_ARG, SPI_TAG, "RX buffer addr&len not align to %d, or not dma_capable", alignment);
//if rxbuf in the desc not DMA-capable, or not align to "alignment", malloc a new one
ESP_EARLY_LOGD(SPI_TAG, "Allocate RX buffer for DMA");
buffer_byte_len = (buffer_byte_len + alignment - 1) & (~(alignment - 1)); // up align to "alignment"
priv_trans->rx_buffer = heap_caps_aligned_alloc(alignment, buffer_byte_len, MALLOC_CAP_DMA);
if (priv_trans->rx_buffer == NULL) {
free(priv_trans->tx_buffer);
return ESP_ERR_NO_MEM;
if (spihost[host]->dma_enabled && trans->rx_buffer) {
if ((!esp_ptr_dma_capable(trans->rx_buffer) || ((((uint32_t)trans->rx_buffer) | (trans->length + 7) / 8) & (alignment - 1)))) {
ESP_RETURN_ON_FALSE_ISR(trans->flags & SPI_SLAVE_TRANS_DMA_BUFFER_ALIGN_AUTO, ESP_ERR_INVALID_ARG, SPI_TAG, "RX buffer addr&len not align to %d, or not dma_capable", alignment);
//if rxbuf in the desc not DMA-capable, or not align to "alignment", malloc a new one
ESP_EARLY_LOGD(SPI_TAG, "Allocate RX buffer for DMA");
buffer_byte_len = (buffer_byte_len + alignment - 1) & (~(alignment - 1)); // up align to "alignment"
priv_trans->rx_buffer = heap_caps_aligned_alloc(alignment, buffer_byte_len, MALLOC_CAP_DMA);
if (priv_trans->rx_buffer == NULL) {
free(priv_trans->tx_buffer);
return ESP_ERR_NO_MEM;
}
}
esp_err_t ret = esp_cache_msync((void *)priv_trans->rx_buffer, buffer_byte_len, ESP_CACHE_MSYNC_FLAG_DIR_M2C);
ESP_RETURN_ON_FALSE_ISR(ESP_OK == ret, ESP_ERR_INVALID_STATE, SPI_TAG, "mem sync m2c(invalid) fail");
}
#endif //SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
return ESP_OK;
Expand Down
3 changes: 3 additions & 0 deletions components/esp_driver_spi/src/gpspi/spi_slave_hd.c
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,9 @@ static esp_err_t s_spi_slave_hd_setup_priv_trans(spi_host_device_t host, spi_sla
memcpy(priv_trans->aligned_buffer, orig_trans->data, orig_trans->len);
esp_err_t ret = esp_cache_msync((void *)priv_trans->aligned_buffer, byte_len, ESP_CACHE_MSYNC_FLAG_DIR_C2M);
ESP_RETURN_ON_FALSE(ESP_OK == ret, ESP_ERR_INVALID_STATE, TAG, "mem sync c2m(writeback) fail");
} else {
esp_err_t ret = esp_cache_msync((void *)priv_trans->aligned_buffer, byte_len, ESP_CACHE_MSYNC_FLAG_DIR_M2C);
ESP_RETURN_ON_FALSE(ESP_OK == ret, ESP_ERR_INVALID_STATE, TAG, "mem sync m2c(invalid) fail");
}
#endif //SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
return ESP_OK;
Expand Down
4 changes: 2 additions & 2 deletions components/hal/include/hal/spi_hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ typedef struct {
typedef struct {
spi_ll_clock_val_t clock_reg; ///< Register value used by the LL layer
spi_clock_source_t clock_source; ///< Clock source of each device used by LL layer
uint32_t source_pre_div; ///< Pre divider befor enter SPI peripheral
uint32_t source_pre_div; ///< Pre divider before enter SPI peripheral
int real_freq; ///< Output of the actual frequency
int timing_dummy; ///< Extra dummy needed to compensate the timing
int timing_miso_delay; ///< Extra miso delay clocks to compensate the timing
Expand Down Expand Up @@ -129,7 +129,7 @@ typedef struct {
#if SOC_SPI_AS_CS_SUPPORTED
uint32_t as_cs : 1; ///< Whether to toggle the CS while the clock toggles, device specific
#endif
uint32_t positive_cs : 1; ///< Whether the postive CS feature is abled, device specific
uint32_t positive_cs : 1; ///< Whether the positive CS feature is abled, device specific
};//boolean configurations
} spi_hal_dev_config_t;

Expand Down
5 changes: 2 additions & 3 deletions examples/storage/.build-test-rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,8 @@ examples/storage/sd_card/sdspi:
disable:
- if: SOC_GPSPI_SUPPORTED != 1
disable_test:
- if: IDF_TARGET not in ["esp32", "esp32c3"]
temporary: true
reason: lack of runners
- if: IDF_TARGET not in ["esp32", "esp32s3", "esp32c3", "esp32p4"]
reason: needs special runner, select few typical targets for testing

examples/storage/semihost_vfs:
depends_components:
Expand Down
16 changes: 8 additions & 8 deletions examples/storage/sd_card/sdspi/main/Kconfig.projbuild
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,35 @@ menu "SD SPI Example Configuration"
int "MOSI GPIO number"
default 15 if IDF_TARGET_ESP32
default 35 if IDF_TARGET_ESP32S2
default 35 if IDF_TARGET_ESP32S3
default 4 if IDF_TARGET_ESP32S3
default 5 if IDF_TARGET_ESP32H2
default 11 if IDF_TARGET_ESP32P4
default 36 if IDF_TARGET_ESP32P4
default 4 # C3 and others

config EXAMPLE_PIN_MISO
int "MISO GPIO number"
default 2 if IDF_TARGET_ESP32
default 37 if IDF_TARGET_ESP32S2
default 37 if IDF_TARGET_ESP32S3
default 5 if IDF_TARGET_ESP32S3
default 0 if IDF_TARGET_ESP32H2
default 13 if IDF_TARGET_ESP32P4
default 47 if IDF_TARGET_ESP32P4
default 6 # C3 and others

config EXAMPLE_PIN_CLK
int "CLK GPIO number"
default 14 if IDF_TARGET_ESP32
default 36 if IDF_TARGET_ESP32S2
default 36 if IDF_TARGET_ESP32S3
default 2 if IDF_TARGET_ESP32S3
default 4 if IDF_TARGET_ESP32H2
default 12 if IDF_TARGET_ESP32P4
default 53 if IDF_TARGET_ESP32P4
default 5 # C3 and others

config EXAMPLE_PIN_CS
int "CS GPIO number"
default 13 if IDF_TARGET_ESP32
default 34 if IDF_TARGET_ESP32S2
default 34 if IDF_TARGET_ESP32S3
default 10 if IDF_TARGET_ESP32P4
default 8 if IDF_TARGET_ESP32S3
default 33 if IDF_TARGET_ESP32P4
default 1 # C3 and others

config EXAMPLE_DEBUG_PIN_CONNECTIONS
Expand Down
Loading

0 comments on commit cea789d

Please sign in to comment.