From a8a47a61f5469ef201e2e23bb27054780a89b353 Mon Sep 17 00:00:00 2001 From: Armando Date: Fri, 1 Apr 2022 16:53:40 +0800 Subject: [PATCH] spi_flash: move buffer check from hal layer to driver layer Prior to this change, `spi_flash_hal_supports_direct_write` and `spi_flash_hal_supports_direct_read` will check the buffer pointer place, which should be done in driver layer, instead of HAL layer. --- components/hal/include/hal/spi_flash_hal.h | 1 - components/hal/include/hal/spi_flash_types.h | 30 +++++++++---------- components/hal/spi_flash_hal.c | 31 +++++++------------- components/spi_flash/esp_flash_api.c | 13 ++++++-- components/spi_flash/memspi_host_driver.c | 23 +++++++-------- tools/ci/check_copyright_ignore.txt | 3 -- 6 files changed, 46 insertions(+), 55 deletions(-) diff --git a/components/hal/include/hal/spi_flash_hal.h b/components/hal/include/hal/spi_flash_hal.h index 4196516fbf..c36d5fa541 100644 --- a/components/hal/include/hal/spi_flash_hal.h +++ b/components/hal/include/hal/spi_flash_hal.h @@ -17,7 +17,6 @@ #include "hal/spi_flash_ll.h" #include "hal/spi_types.h" #include "hal/spi_flash_types.h" -#include "esp_memory_utils.h" /* Hardware host-specific constants */ #define SPI_FLASH_HAL_MAX_WRITE_BYTES 64 diff --git a/components/hal/include/hal/spi_flash_types.h b/components/hal/include/hal/spi_flash_types.h index c9d5035e60..98426a2de8 100644 --- a/components/hal/include/hal/spi_flash_types.h +++ b/components/hal/include/hal/spi_flash_types.h @@ -1,16 +1,8 @@ -// Copyright 2010-2019 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2010-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #pragma once @@ -180,7 +172,11 @@ struct spi_flash_host_driver_s { * Program a page of the flash. Check ``max_write_bytes`` for the maximum allowed writing length. */ void (*program_page)(spi_flash_host_inst_t *host, const void *buffer, uint32_t address, uint32_t length); - /** Check whether given buffer can be directly used to write */ + /** + * @brief Check whether the SPI host supports direct write + * + * When cache is disabled, SPI1 doesn't support directly write when buffer isn't internal. + */ bool (*supports_direct_write)(spi_flash_host_inst_t *host, const void *p); /** * Slicer for write data. The `program_page` should be called iteratively with the return value @@ -198,7 +194,11 @@ struct spi_flash_host_driver_s { * Read data from the flash. Check ``max_read_bytes`` for the maximum allowed reading length. */ esp_err_t (*read)(spi_flash_host_inst_t *host, void *buffer, uint32_t address, uint32_t read_len); - /** Check whether given buffer can be directly used to read */ + /** + * @brief Check whether the SPI host supports direct read + * + * When cache is disabled, SPI1 doesn't support directly read when the given buffer isn't internal. + */ bool (*supports_direct_read)(spi_flash_host_inst_t *host, const void *p); /** * Slicer for read data. The `read` should be called iteratively with the return value diff --git a/components/hal/spi_flash_hal.c b/components/hal/spi_flash_hal.c index d417eccf20..bce76ad3b4 100644 --- a/components/hal/spi_flash_hal.c +++ b/components/hal/spi_flash_hal.c @@ -1,16 +1,8 @@ -// Copyright 2015-2018 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ // HAL for SPI Flash (non-IRAM part) // The IRAM part is in spi_flash_hal_iram.c, spi_flash_hal_gpspi.c, spi_flash_hal_common.inc. @@ -93,9 +85,6 @@ static inline int extra_dummy_under_timing_tuning(const spi_flash_hal_config_t * esp_err_t spi_flash_hal_init(spi_flash_hal_context_t *data_out, const spi_flash_hal_config_t *cfg) { - if (!esp_ptr_internal(data_out) && cfg->host_id == SPI1_HOST) { - return ESP_ERR_INVALID_ARG; - } if (cfg->cs_num >= SOC_SPI_PERIPH_CS_NUM(cfg->host_id)) { return ESP_ERR_INVALID_ARG; } @@ -144,16 +133,16 @@ esp_err_t spi_flash_hal_init(spi_flash_hal_context_t *data_out, const spi_flash_ bool spi_flash_hal_supports_direct_write(spi_flash_host_inst_t *host, const void *p) { - bool direct_write = ( ((spi_flash_hal_context_t *)host)->spi != spi_flash_ll_get_hw(SPI1_HOST) - || esp_ptr_in_dram(p) ); + (void)p; + bool direct_write = (((spi_flash_hal_context_t *)host)->spi != spi_flash_ll_get_hw(SPI1_HOST)); return direct_write; } bool spi_flash_hal_supports_direct_read(spi_flash_host_inst_t *host, const void *p) { - //currently the host doesn't support to read through dma, no word-aligned requirements - bool direct_read = ( ((spi_flash_hal_context_t *)host)->spi != spi_flash_ll_get_hw(SPI1_HOST) - || esp_ptr_in_dram(p) ); + (void)p; + //currently the host doesn't support to read through dma, no word-aligned requirements + bool direct_read = ( ((spi_flash_hal_context_t *)host)->spi != spi_flash_ll_get_hw(SPI1_HOST)); return direct_read; } diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index a432e1267a..8787d20dad 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -9,6 +9,7 @@ #include #include +#include "esp_memory_utils.h" #include "spi_flash_chip_driver.h" #include "memspi_host_driver.h" #include "esp_log.h" @@ -782,7 +783,11 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add } //when the cache is disabled, only the DRAM can be read, check whether we need to receive in another buffer in DRAM. - bool direct_read = chip->host->driver->supports_direct_read(chip->host, buffer); + bool direct_read = false; + //If the buffer is internal already, it's ok to use it directly + direct_read |= esp_ptr_in_dram(buffer); + //If not, we need to check if the HW support direct write + direct_read |= chip->host->driver->supports_direct_read(chip->host, buffer); uint8_t* temp_buffer = NULL; //each time, we at most read this length @@ -850,7 +855,11 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3 } //when the cache is disabled, only the DRAM can be read, check whether we need to copy the data first - bool direct_write = chip->host->driver->supports_direct_write(chip->host, buffer); + bool direct_write = false; + //If the buffer is internal already, it's ok to write it directly + direct_write |= esp_ptr_in_dram(buffer); + //If not, we need to check if the HW support direct write + direct_write |= chip->host->driver->supports_direct_write(chip->host, buffer); // Indicate whether the bus is acquired by the driver, needs to be released before return bool bus_acquired = false; diff --git a/components/spi_flash/memspi_host_driver.c b/components/spi_flash/memspi_host_driver.c index b242c9512b..655be0162f 100644 --- a/components/spi_flash/memspi_host_driver.c +++ b/components/spi_flash/memspi_host_driver.c @@ -1,16 +1,8 @@ -// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include "soc/soc_caps.h" #include "spi_flash_defs.h" @@ -19,6 +11,7 @@ #include "esp_log.h" #include "cache_utils.h" #include "esp_flash_partitions.h" +#include "esp_memory_utils.h" #define SPI_FLASH_HAL_MAX_WRITE_BYTES 64 @@ -69,6 +62,10 @@ static const spi_flash_host_driver_t esp_flash_gpspi_host = { esp_err_t memspi_host_init_pointers(memspi_host_inst_t *host, const memspi_host_config_t *cfg) { + if (!esp_ptr_internal(host) && cfg->host_id == SPI1_HOST) { + return ESP_ERR_INVALID_ARG; + } + #if SOC_MEMSPI_IS_INDEPENDENT if (cfg->host_id == SPI1_HOST) host->inst.driver = &esp_flash_default_host; diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index 3f3e94b883..d5b170d7ab 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -926,7 +926,6 @@ components/hal/include/hal/sha_hal.h components/hal/include/hal/sigmadelta_hal.h components/hal/include/hal/soc_hal.h components/hal/include/hal/spi_flash_encrypt_hal.h -components/hal/include/hal/spi_flash_types.h components/hal/include/hal/spi_slave_hal.h components/hal/include/hal/spi_slave_hd_hal.h components/hal/include/hal/systimer_hal.h @@ -954,7 +953,6 @@ components/hal/sha_hal.c components/hal/sigmadelta_hal.c components/hal/soc_hal.c components/hal/spi_flash_encrypt_hal_iram.c -components/hal/spi_flash_hal.c components/hal/spi_flash_hal_gpspi.c components/hal/spi_slave_hal.c components/hal/spi_slave_hal_iram.c @@ -1629,7 +1627,6 @@ components/spi_flash/include/spi_flash_chip_generic.h components/spi_flash/include/spi_flash_chip_issi.h components/spi_flash/include/spi_flash_chip_mxic.h components/spi_flash/include/spi_flash_chip_winbond.h -components/spi_flash/memspi_host_driver.c components/spi_flash/sim/SpiFlash.cpp components/spi_flash/sim/flash_mock.cpp components/spi_flash/sim/flash_mock_util.c