From aa4532395ae9541fd3f8a4023df7a11e5a0a951c Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 30 Jun 2026 14:26:21 +0800 Subject: [PATCH] fix(esp_crc): clarify CRC helper usage documentation Document the implicit bitwise inversion behavior in the CRC ROM wrappers and add regression tests covering continuous-buffer examples. Closes https://github.com/espressif/esp-idf/issues/18715 --- .../test_apps/dma/main/test_async_crc.c | 75 +++++++++++++++ components/esp_hw_support/include/esp_crc.h | 32 +++++-- .../main/test_hw_support_linux.c | 27 ++++++ components/esp_rom/include/esp_rom_crc.h | 96 +++++++++---------- components/esp_rom/patches/esp_rom_crc.c | 18 +--- tools/ci/check_copyright_ignore.txt | 2 - 6 files changed, 180 insertions(+), 70 deletions(-) diff --git a/components/esp_driver_dma/test_apps/dma/main/test_async_crc.c b/components/esp_driver_dma/test_apps/dma/main/test_async_crc.c index 7ac2a326df0..20e7ab37eb5 100644 --- a/components/esp_driver_dma/test_apps/dma/main/test_async_crc.c +++ b/components/esp_driver_dma/test_apps/dma/main/test_async_crc.c @@ -210,3 +210,78 @@ TEST_CASE("async_crc multiple requests", "[async_crc]") TEST_ESP_OK(esp_async_crc_uninstall(driver)); #endif } + +static uint32_t test_rom_crc32_le_reference(const uint8_t *data, size_t len) +{ + uint32_t crc = (uint32_t)~UINT32_MAX; + + crc = esp_crc32_le(crc, data, len); + + return ~crc ^ UINT32_MAX; +} + +static uint16_t test_rom_crc16_xmodem_reference(const uint8_t *data, size_t len) +{ + uint16_t crc = (uint16_t)~0x0000; + + crc = esp_crc16_be(crc, data, len); + + return (uint16_t)(~crc ^ 0x0000); +} + +static void test_async_crc_matches_rom_helper(async_crc_handle_t driver, bool supports_crc32) +{ + static const char test_input_string[] __attribute__((aligned(16))) = "GDMACRC::TEST::LONGSTRING::REPEAT::GDMACRC::TEST::LONGSTRING::REPEAT::GDMACRC::TEST::LONGSTRING::REPEAT::GDMACRC::TEST::LONGSTRING::REPEAT::END!"; + size_t input_len = strlen(test_input_string); + uint32_t async_result = 0; + + if (supports_crc32) { + async_crc_params_t crc32_params = { + .width = 32, + .polynomial = 0x04C11DB7, + .init_value = UINT32_MAX, + .final_xor_value = UINT32_MAX, + .reverse_input = true, + .reverse_output = true, + }; + uint32_t crc32_rom_result = test_rom_crc32_le_reference((const uint8_t *)test_input_string, input_len); + TEST_ESP_OK(esp_crc_calc_blocking(driver, test_input_string, input_len, &crc32_params, -1, &async_result)); + printf("CRC-32 async result: 0x%"PRIx32", ROM helper result: 0x%"PRIx32"\r\n", async_result, crc32_rom_result); + TEST_ASSERT_EQUAL_HEX32(crc32_rom_result, async_result); + } + + async_crc_params_t crc16_params = { + .width = 16, + .polynomial = 0x1021, + .init_value = 0x0000, + .final_xor_value = 0x0000, + .reverse_input = false, + .reverse_output = false, + }; + uint16_t crc16_rom_result = test_rom_crc16_xmodem_reference((const uint8_t *)test_input_string, input_len); + TEST_ESP_OK(esp_crc_calc_blocking(driver, test_input_string, input_len, &crc16_params, -1, &async_result)); + printf("CRC-16/XMODEM async result: 0x%"PRIx32", ROM helper result: 0x%04x\r\n", async_result, crc16_rom_result); + TEST_ASSERT_EQUAL_HEX16(crc16_rom_result, async_result); +} + +TEST_CASE("async_crc matches ROM helper results for standard CRC flavors", "[async_crc]") +{ + async_crc_config_t config = { + .backlog = 1, + .dma_burst_size = 16, + }; + async_crc_handle_t driver = NULL; +#if SOC_HAS(AHB_GDMA) + printf("Testing async CRC against ROM helper by AHB GDMA\r\n"); + TEST_ESP_OK(esp_async_crc_install_gdma_ahb(&config, &driver)); + test_async_crc_matches_rom_helper(driver, true); + TEST_ESP_OK(esp_async_crc_uninstall(driver)); +#endif + +#if SOC_HAS(AXI_GDMA) + printf("Testing async CRC against ROM helper by AXI GDMA\r\n"); + TEST_ESP_OK(esp_async_crc_install_gdma_axi(&config, &driver)); + test_async_crc_matches_rom_helper(driver, false); + TEST_ESP_OK(esp_async_crc_uninstall(driver)); +#endif +} diff --git a/components/esp_hw_support/include/esp_crc.h b/components/esp_hw_support/include/esp_crc.h index f12dcf767dd..fd779335b21 100644 --- a/components/esp_hw_support/include/esp_crc.h +++ b/components/esp_hw_support/include/esp_crc.h @@ -14,10 +14,25 @@ extern "C" { // This header is only a wrapper on ROM CRC API #include "esp_rom_crc.h" +/** +* @brief Convenience wrappers for the ROM CRC helpers. +* +* These wrappers keep the ROM helper behavior unchanged: each call bitwise +* inverts the CRC value before and after processing. To start a calculation, +* pass the bitwise inverse of the CRC flavor's initial value. To continue over +* another buffer, pass the previous return value. After the last chunk, bitwise +* invert the return value again and then apply the CRC flavor's final XOR. +* +* The correct initial value and final XOR depend on the CRC flavor that you are +* implementing, so there is no single universal `INITIAL` or `FINAL_XOR` +* constant for a given helper. +*/ + /** * @brief CRC32 value in little endian. * -* @param crc: Initial CRC value (result of last calculation or 0 for the first time) +* @param crc: Bitwise inverse of the CRC variant's initial value on the first +* call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC32 value @@ -30,7 +45,8 @@ static inline uint32_t esp_crc32_le(uint32_t crc, uint8_t const *buf, uint32_t l /** * @brief CRC32 value in big endian. * -* @param crc: Initial CRC value (result of last calculation or 0 for the first time) +* @param crc: Bitwise inverse of the CRC variant's initial value on the first +* call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC32 value @@ -43,7 +59,8 @@ static inline uint32_t esp_crc32_be(uint32_t crc, uint8_t const *buf, uint32_t l /** * @brief CRC16 value in little endian. * -* @param crc: Initial CRC value (result of last calculation or 0 for the first time) +* @param crc: Bitwise inverse of the CRC variant's initial value on the first +* call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC16 value @@ -56,7 +73,8 @@ static inline uint16_t esp_crc16_le(uint16_t crc, uint8_t const *buf, uint32_t l /** * @brief CRC16 value in big endian. * -* @param crc: Initial CRC value (result of last calculation or 0 for the first time) +* @param crc: Bitwise inverse of the CRC variant's initial value on the first +* call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC16 value @@ -69,7 +87,8 @@ static inline uint16_t esp_crc16_be(uint16_t crc, uint8_t const *buf, uint32_t l /** * @brief CRC8 value in little endian. * -* @param crc: Initial CRC value (result of last calculation or 0 for the first time) +* @param crc: Bitwise inverse of the CRC variant's initial value on the first +* call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC8 value @@ -82,7 +101,8 @@ static inline uint8_t esp_crc8_le(uint8_t crc, uint8_t const *buf, uint32_t len) /** * @brief CRC8 value in big endian. * -* @param crc: Initial CRC value (result of last calculation or 0 for the first time) +* @param crc: Bitwise inverse of the CRC variant's initial value on the first +* call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC8 value diff --git a/components/esp_hw_support/test_apps/host_test_linux/main/test_hw_support_linux.c b/components/esp_hw_support/test_apps/host_test_linux/main/test_hw_support_linux.c index 26839c069ae..396f529a118 100644 --- a/components/esp_hw_support/test_apps/host_test_linux/main/test_hw_support_linux.c +++ b/components/esp_hw_support/test_apps/host_test_linux/main/test_hw_support_linux.c @@ -6,6 +6,7 @@ #include #include #include "unity.h" +#include "esp_crc.h" #include "esp_random.h" /* Note: these are just sanity tests, the implementation of esp_random() relies on getentropy() on Linux. @@ -123,6 +124,32 @@ TEST_CASE("esp_fill_random() fills exactly 257 bytes", "[random]") TEST_ASSERT_GREATER_THAN(0, one_buf[0]); } +TEST_CASE("esp_crc32_le supports continuous buffers with standard CRC-32 parameters", "[crc]") +{ + static const uint8_t buf0[] = "1234"; + static const uint8_t buf1[] = "56789"; + uint32_t crc = (uint32_t)~UINT32_MAX; + + crc = esp_crc32_le(crc, buf0, sizeof(buf0) - 1); + crc = esp_crc32_le(crc, buf1, sizeof(buf1) - 1); + crc = ~crc ^ UINT32_MAX; + + TEST_ASSERT_EQUAL_HEX32(0xCBF43926, crc); +} + +TEST_CASE("esp_crc16_be supports continuous buffers with CRC-16/XMODEM parameters", "[crc]") +{ + static const uint8_t buf0[] = "1234"; + static const uint8_t buf1[] = "56789"; + uint16_t crc = (uint16_t)~0x0000; + + crc = esp_crc16_be(crc, buf0, sizeof(buf0) - 1); + crc = esp_crc16_be(crc, buf1, sizeof(buf1) - 1); + crc = (uint16_t)(~crc ^ 0x0000); + + TEST_ASSERT_EQUAL_HEX16(0x31C3, crc); +} + void app_main(void) { printf("Running hw support linux API host test app"); diff --git a/components/esp_rom/include/esp_rom_crc.h b/components/esp_rom/include/esp_rom_crc.h index 39787ed1402..7cf1a6713ed 100644 --- a/components/esp_rom/include/esp_rom_crc.h +++ b/components/esp_rom/include/esp_rom_crc.h @@ -1,16 +1,8 @@ -// Copyright 2010-2020 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-2026 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #pragma once @@ -21,48 +13,49 @@ extern "C" { #include /** Notes about CRC API - * The ESP32 ROM include some CRC tables and CRC APIs to speed up CRC calculation. - * The CRC APIs include CRC8, CRC16, CRC32 algorithms for both little endian and big endian modes. - * Here are the polynomials for the algorithms: + * The ROM CRC helpers accelerate CRC-8, CRC-16, and CRC-32 calculations for + * the following base polynomials: * CRC-8 x8+x2+x1+1 0x07 * CRC16-CCITT x16+x12+x5+1 0x1021 * CRC32 x32+x26+x23+x22+x16+x12+x11+x10+x8+x7+x5+x4+x2+x1+1 0x04c11db7 * - * These group of CRC APIs are designed to calculate the data in buffers either continuous or not. - * To make it easy, we had added a `~` at the beginning and the end of the functions. - * To calculate non-continuous buffers, we can write the code like this: - * init = ~init; - * crc = crc32_le(init, buf0, length0); + * The `_le` and `_be` suffixes describe the bit processing order: + * - use the little-endian helper when `refin == true` and `refout == true` + * - use the big-endian helper when `refin == false` and `refout == false` + * + * These helpers invert the CRC register before and after processing each call. + * As a result: + * - the first call must receive the bitwise inverse of the CRC variant's + * initial value + * - each later call must receive the previous helper's return value so + * non-contiguous buffers can be chained + * - the final CRC value must be bitwise inverted again before applying the + * CRC variant's `xorout` + * + * The initial value and final XOR depend on the CRC flavor that you are + * implementing, so these helpers do not define a single universal + * INITIAL/FINAL_XOR pair. + * + * Example for a CRC-32 flavor with `init = 0xffffffff`, `refin = true`, + * `refout = true`, `xorout = 0xffffffff`: + * uint32_t crc = (uint32_t)~0xffffffff; + * crc = crc32_le(crc, buf0, length0); * crc = crc32_le(crc, buf1, length1); - * crc = ~crc; - * - * However, it is not easy to select which API to use and give the correct parameters. - * A specific CRC algorithm will include this parameters: width, polynomials, init, refin, refout, xorout - * refin and refout show the endian of the algorithm: - * if both of them are true, please use the little endian API. - * if both of them are false, please use the big endian API. - * xorout is the value which you need to be xored to the raw result. - * However, these group of APIs need one '~' before and after the APIs. - * - * Here are some examples for CRC16: - * CRC-16/CCITT, poly = 0x1021, init = 0x0000, refin = true, refout = true, xorout = 0x0000 - * crc = ~crc16_le((uint16_t)~0x0000, buf, length); - * - * CRC-16/CCITT-FALSE, poly = 0x1021, init = 0xffff, refin = false, refout = false, xorout = 0x0000 - * crc = ~crc16_be((uint16_t)~0xffff, buf, length); - * - * CRC-16/X25, poly = 0x1021, init = 0xffff, refin = true, refout = true, xorout = 0xffff - * crc = (~crc16_le((uint16_t)~(0xffff), buf, length))^0xffff; - * - * CRC-16/XMODEM, poly= 0x1021, init = 0x0000, refin = false, refout = false, xorout = 0x0000 - * crc = ~crc16_be((uint16_t)~0x0000, buf, length); + * crc = ~crc ^ 0xffffffff; * + * Example for CRC-16/XMODEM with `init = 0x0000`, `refin = false`, + * `refout = false`, `xorout = 0x0000`: + * uint16_t crc = (uint16_t)~0x0000; + * crc = crc16_be(crc, buf0, length0); + * crc = crc16_be(crc, buf1, length1); + * crc = (uint16_t)(~crc ^ 0x0000); */ /** * @brief CRC32 value in little endian. * - * @param crc: Initial CRC value (result of last calculation or 0 for the first time) + * @param crc: Bitwise inverse of the CRC variant's initial value on the first + * call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC32 value @@ -72,7 +65,8 @@ uint32_t esp_rom_crc32_le(uint32_t crc, uint8_t const *buf, uint32_t len); /** * @brief CRC32 value in big endian. * - * @param crc: Initial CRC value (result of last calculation or 0 for the first time) + * @param crc: Bitwise inverse of the CRC variant's initial value on the first + * call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC32 value @@ -82,7 +76,8 @@ uint32_t esp_rom_crc32_be(uint32_t crc, uint8_t const *buf, uint32_t len); /** * @brief CRC16 value in little endian. * - * @param crc: Initial CRC value (result of last calculation or 0 for the first time) + * @param crc: Bitwise inverse of the CRC variant's initial value on the first + * call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC16 value @@ -92,7 +87,8 @@ uint16_t esp_rom_crc16_le(uint16_t crc, uint8_t const *buf, uint32_t len); /** * @brief CRC16 value in big endian. * - * @param crc: Initial CRC value (result of last calculation or 0 for the first time) + * @param crc: Bitwise inverse of the CRC variant's initial value on the first + * call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC16 value @@ -102,7 +98,8 @@ uint16_t esp_rom_crc16_be(uint16_t crc, uint8_t const *buf, uint32_t len); /** * @brief CRC8 value in little endian. * - * @param crc: Initial CRC value (result of last calculation or 0 for the first time) + * @param crc: Bitwise inverse of the CRC variant's initial value on the first + * call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC8 value @@ -112,7 +109,8 @@ uint8_t esp_rom_crc8_le(uint8_t crc, uint8_t const *buf, uint32_t len); /** * @brief CRC8 value in big endian. * - * @param crc: Initial CRC value (result of last calculation or 0 for the first time) + * @param crc: Bitwise inverse of the CRC variant's initial value on the first + * call, or the previous return value to continue a calculation * @param buf: Data buffer that used to calculate the CRC value * @param len: Length of the data buffer * @return CRC8 value diff --git a/components/esp_rom/patches/esp_rom_crc.c b/components/esp_rom/patches/esp_rom_crc.c index 07d9db2c9c6..dab407755f0 100644 --- a/components/esp_rom/patches/esp_rom_crc.c +++ b/components/esp_rom/patches/esp_rom_crc.c @@ -1,16 +1,8 @@ -// Copyright 2010-2020 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-2026 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include #include "esp_rom_caps.h" diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index bbcf995f910..4b40dcc906c 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -409,10 +409,8 @@ components/esp_rom/esp32s3/ld/esp32s3.rom.ld components/esp_rom/esp32s3/ld/esp32s3.rom.libgcc.ld components/esp_rom/esp32s3/ld/esp32s3.rom.newlib-nano.ld components/esp_rom/esp32s3/ld/esp32s3.rom.version.ld -components/esp_rom/include/esp_rom_crc.h components/esp_rom/linux/esp_rom_crc.c components/esp_rom/linux/esp_rom_md5.c -components/esp_rom/patches/esp_rom_crc.c components/esp_rom/patches/esp_rom_longjmp.S components/esp_system/ubsan.c components/esp_wifi/src/mesh_event.c