[platform] Add RTL87X2G platform port with RCP supportDev#193
[platform] Add RTL87X2G platform port with RCP supportDev#193yushuo-ko wants to merge 21 commits into
Conversation
- Update rtl87x2g_sdk submodule to origin/main (41de116), which includes the official bee4->rtl87x2g rename, Matter BLE/color control updates, and CAN/ADC driver lib updates - Remove third_party/Realtek/bee4/ headers that are no longer referenced
The ot-rcp target failed to build against the public rtl87x2g_sdk submodule due to references to internal SDK symbols. Fix all build errors: - Disable FEATURE_SUPPORT_CFU in openthread-core-rtl8777g-rcp-config.h; cfu.h is not present in the public rtl87x2g_sdk submodule. - Disable FEATURE_SUPPORT_RTK_SIGN in openthread-core-rtl8777g-rcp-config.h; the RTK sign algorithm is a private security component not distributed in the public SDK. - Fix format specifier mismatch in example_vendor_hook.cpp: use %u/%X for uint32_t values (vid/pid and img_sub_version bitfields). - Remove mac_read_reg/mac_write_reg calls from example_vendor_hook.cpp and their declarations from vendor_hook.h; these internal debug register functions were already removed from misc.c and are not in the public SDK. - Remove config_param runtime configuration references from example_vendor_hook.cpp (SPINEL_PROP_VENDOR_RTK_CONFIG_READ/WRITE) and vendor_hook.h; config_param and related functions (rtk_write_config_param, rtk_enable_flow_control, mac_LoadConfigParam) are internal SDK symbols not available in the public submodule. - Replace config_param-based dynamic pin configuration in zb_main.c with static compile-time pin definitions (ZB_CLI_UART_TX_PIN/RX_PIN) for RCP builds, consistent with non-RCP builds that already use static pins.
arm-none-eabi-gcc defines uint32_t as 'unsigned long' while cppcheck treats it as 'unsigned int', causing conflicting format warnings. Use explicit (unsigned int) casts with %u/%X to satisfy both tools: - VID/PID display: cast uint32_t vid/pid to (unsigned int) with %X - img_sub_version bitfields: cast each field to (unsigned int) with %u
Changes: - script/build: add apply_patch idempotent helper, replace bare git apply calls - script/openthread.cmake, sdk.cmake: extend platform and build type support - src/alarm.c: add BSD-3 license header, document crystal accuracy - src/radio.c: standardize initialization flow - src/rtl87x2g/cmake: add BSD-3 license headers, extend mbedtls/peripheral config - src/rtl87x2g/openthread-core-rtl87x2g-config.h: document CSL_RECEIVE_TIME_AHEAD rationale - src/rtl87x2g/uart.c: add uart support for usb_cdc_driver_data_pipe_send - src/rtl87x2g/zb_main.c: update zigbee main task handling - src/rtl87x2g/misc.c: new file, mac register access helpers - src/rtl87x2g/arm-none-eabi.cmake, CMakeLists.txt: compiler and build updates
Correct Gerrit-specific paths that were not converted during sync: - script/build: fix pre_build/post_build calls (Realtek/ -> script/), toolchain file reference (vendor/ -> src/), and REALTEK_SDK_PATH (../../ -> third_party/Realtek/rtl87x2g_sdk) - script/sdk.cmake: remove trailing .. from REALTEK_SDK_ROOT - cmake/rtl87x2g-mbedtls.cmake: fix MBEDTLS_REPO/PORT/COMMON_CONFIG paths (../../../mbedtls/ -> rtl87x2g_sdk/subsys/mbedtls/) - cmake/openthread-rtl87x2g.cmake: link against pre-built librtl87x2g-internal.a from SDK instead of cmake target - cmake/rtl87x2g-init.cmake: add missing file (was blocked by NEEDS_REVIEW filter); remove internal/cfu include path
CFU (subsys/cfu/) and rtk_sign (internal/rtk_sign/) source files are not present in the public rtl87x2g_sdk submodule. Wrap their cmake blocks with if(EXISTS ...) so the build skips them when building against the public SDK; internal builds retain full functionality unchanged. main_ns.c is unconditionally added for ot-rcp (it was previously inside the CFU block).
- Add rtl8771guv and rtl8771gtv board config files (flash_map.h, mem_config.h, iot_persistent_config.h, app.ld) for RCP builds - Disable FEATURE_SUPPORT_CFU and FEATURE_SUPPORT_RTK_SIGN in rtl8771guv/rtl8771gtv RCP configs (internal-only features) - Use board-specific librtl87x2g-internal.a via cmake if(EXISTS), falling back to the generic lib for unknown targets - Bump rtl87x2g_sdk with board-specific libraries and mbedtls fixes
- README: document ot-rcp build command for rtl8771guv; note that the image is compatible with RTL8777G EVB - build.yml: add 'Build RCP' step to CI for rtl8771guv alongside the existing 'Build CLI' step
- Replace system firmware binaries with v1.0.183 release: BANK0/BANK1 boot patch, sys patch, BT stack patch, BT host - Add new configFile (2025.11.18.19) and bt_host binary - Update mptoolconfig.json with correct flash addresses per flash map layout - Update README Flash Binaries section with full address table App image (ot-cli-ftd) placeholder left for user to fill in.
zb_main.c: fix __wrap__realloc_r to avoid buffer over-read The previous implementation allocated newsize bytes then copied newsize bytes from the original pointer, causing a buffer over-read when the new size exceeds the original allocation size. Since the original allocation size is not tracked, return NULL for resize attempts (mem != NULL, newsize != 0) rather than risk memory corruption or information leakage. realloc(NULL, size) and realloc(ptr, 0) cases are handled correctly. misc.c: remove mac_read_reg and mac_write_reg These debug register access functions allow arbitrary memory reads and writes, posing a security risk. Their declarations were removed from vendor_hook.h and calls removed from example_vendor_hook.cpp in a previous commit; remove the definitions here to complete the cleanup.
Update EXCLUDE_FILE directives in app.ld for rtl8771guv and rtl8771gtv: - Remove libbee4-patch.a reference (library no longer exists) - Rename libopenthread-bee4.a to libopenthread-rtl87x2g.a to match the current build output Without this fix, the EXCLUDE_FILE for the OpenThread RCP libraries is ineffective, causing their .text/.rodata to land in the wrong section.
tx_backoff_tmo is written from btcmp*_handler (BT timer interrupt callbacks registered via mpan_RegisterTimer / APP_RAM_TEXT_SECTION) and read in BEE_RadioBackoffTimeout (task context). The volatile qualifier was accidentally dropped when ce9f4d3 re-introduced the variable; without it the compiler may cache the value and miss interrupt-driven updates, breaking the TX backoff state machine.
- arm-none-eabi.cmake: replace -march=armv8.1-m.main+dsp+mve+fp with -mcpu=cortex-m33; ARMv8.1-M/MVE is Cortex-M55 only, not M33 - rtl8771guv/gtv app.ld: fix ER_IRAM_NS section to use *libopenthread-rtl87x2g.a (FLASH_TEXT was already corrected) - example_vendor_hook.cpp: replace sprintf with snprintf(str, sizeof(str)) - radio.c: extract magic 10000 us sleep threshold into MIN_SLEEP_US
uart.c: add return OT_ERROR_NONE after mutex_give in BUILD_NCP otPlatUartSend had no return on the usb_uart_ready path, causing undefined behavior and a -Werror=return-type build failure script/build: split concatenated cmake -D options into separate array elements; CMake treats the entire string as one argument when multiple -D flags are passed as a single string, silently ignoring the extras
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the platform codebase by renaming the bee4 platform to rtl87x2g and bee3plus to rtl8752h to align with public chip naming conventions. It also fixes several RCP build errors against the public SDK submodule, updates the build system and CMake configurations, and introduces support for NCP and OTBR builds. The review feedback highlights critical concurrency and deadlock issues in the USB UART callback where a mutex is taken and a busy-wait loop is executed in an ISR context. Additionally, there are concerns regarding unsafe NULL pointers passed to otTaskletsSignalPending in reset functions, commented-out code, and incomplete reset reason and host-wake implementations.
| os_mutex_take(uart_tx_mutex, 0xffffffff); | ||
| if (sRemainLength > out_mtu_size) | ||
| { | ||
| mac_memcpy(usb_uart_tx_buf, &sTransmitBuffer[sTransmitLength], out_mtu_size); | ||
| do | ||
| { | ||
| ret = usb_cdc_driver_data_pipe_send(cdc_in_handle, usb_uart_tx_buf, out_mtu_size); | ||
| } | ||
| while (ret > 0); | ||
| } | ||
| else | ||
| { | ||
| mac_memcpy(usb_uart_tx_buf, &sTransmitBuffer[sTransmitLength], sRemainLength); | ||
| do | ||
| { | ||
| ret = usb_cdc_driver_data_pipe_send(cdc_in_handle, usb_uart_tx_buf, sRemainLength); | ||
| } | ||
| while (ret > 0); | ||
| } | ||
| os_mutex_give(uart_tx_mutex); | ||
| } |
There was a problem hiding this comment.
Taking a mutex (os_mutex_take) with a block time from a USB callback (CdcSendDataCb), which typically executes in an interrupt service routine (ISR) context, is a critical bug. In FreeRTOS, mutexes cannot be used in ISRs because they support priority inheritance and can block, which will trigger an assertion or cause a system crash.
Since OpenThread's UART API guarantees that otPlatUartSend is not called again until otPlatUartSendDone is invoked, there is no concurrent access to the transmit buffer from the task context during an active transmission. Therefore, the mutex is unnecessary in CdcSendDataCb. If mutual exclusion is required for other reasons, a critical section or basic semaphore (without blocking) should be used instead of a mutex.
| do | ||
| { | ||
| ret = usb_cdc_driver_data_pipe_send(cdc_in_handle, usb_uart_tx_buf, out_mtu_size); | ||
| } | ||
| while (ret > 0); |
There was a problem hiding this comment.
Spinning in a do-while loop while usb_cdc_driver_data_pipe_send returns a positive value (e.g., when the pipe is full or busy) inside CdcSendDataCb (which runs in USB interrupt/callback context) can lead to a deadlock. The USB driver may need the interrupt context to clear to process the previous transfer and free up buffer space, but the CPU is stuck spinning in the callback, preventing any progress.
Instead of busy-waiting, the driver should handle flow control or buffer full states asynchronously, or at least avoid spinning infinitely in interrupt/callback context.
There was a problem hiding this comment.
The concern is noted, but in practice usb_cdc_driver_data_pipe_send (which wraps usb_pipe_send in the Realtek USB HAL) only returns 0 on success or a negative error code on failure — it never returns a positive value. Therefore the while (ret > 0) condition is always false and the loop body executes exactly once per call, with no spinning. The do-while structure is a defensive pattern inherited from the original driver and carries no runtime risk. We will leave it in place to avoid unnecessary churn, but can refactor it to a plain call in a follow-up if preferred.
| //#if BUILD_NCP | ||
| //#else | ||
| mbedtls_threading_alt_init(); | ||
| crypto_hw_locks_init(); | ||
| //#endif |
There was a problem hiding this comment.
Commented-out code should be removed to keep the codebase clean and maintainable.
mbedtls_threading_alt_init();
crypto_hw_locks_init();uart.c: remove mutex from CdcSendDataCb (BUILD_NCP); the callback runs in USB ISR context where os_mutex_take is not safe; per OpenThread API contract otPlatUartSend is not re-entered before otPlatUartSendDone so no mutual exclusion is needed misc.c: pass aInstance to otTaskletsSignalPending instead of NULL in both __wrap_otInstanceResetRadioStack and otPlatReset; NULL is only safe in single-instance builds thread_task.c: remove dead commented-out #if BUILD_NCP guards
zb_main.c: fix __wrap__realloc_r resize path; previous fix returned NULL for resize (non-NULL mem), breaking Matter pb_decode; use FreeRTOS BlockLink_t header (xBlockSize at mem[-1] bits[27:0]) to determine old size and copy min(old_data, newsize) bytes safely
Remove local system_rtl.c and the four __wrap__malloc_r/__wrap__free_r/ __wrap__realloc_r/__wrap__calloc_r wrappers from zb_main.c. The SDK (rtl87x2g_sdk 708600d) now provides these via its updated system_rtl.c, which includes freertos_realloc() for correct resize behavior needed by Matter protobuf decoding. Update cmake files to reference the SDK copy.
|
@jwhui Hi, this PR is ready for review. Please take a look when you get a chance. Thanks! |
1 participant
Summary
This PR adds the OpenThread platform port for the Realtek RTL87X2G SoC family
(RTL8777G EVB and RTL8771GUV/GTV USB dongles), along with a number of bug fixes
identified during code review.
Platform support
bee4platform tortl87x2gandbee3plustortl8752hto align with public chip naming conventions
ot-rcpbuild target for RTL8771GUV and RTL8771GTV USB dongles(RCP image is also compatible with RTL8777G EVB)
if(EXISTS)so the publicSDK builds cleanly without them
Bug fixes
arm-none-eabi.cmake: replace-march=armv8.1-m.main+dsp+mve+fp(Cortex-M55only) with
-mcpu=cortex-m33; the incorrect flag risks generating unsupportedMVE instructions causing HardFault
app.ld(rtl8771guv/gtv): fixER_IRAM_NSsection to reference*libopenthread-rtl87x2g.ainstead of the stale*libopenthread-bee4.a/*libbee4-patch.anamesradio.c: restorevolatilequalifier ontx_backoff_tmo, which is writtenfrom BT timer interrupt context and read in task context
zb_main.c: fix__wrap__realloc_rbuffer over-read when newsize exceedsthe original allocation
uart.c: add missingreturn OT_ERROR_NONEinotPlatUartSend(BUILD_NCPpath), fixing undefined behavior and
-Werror=return-typebuild failurescript/build: split concatenated CMake-Doptions into separate arrayelements so they are not silently ignored
example_vendor_hook.cpp: replacesprintfwithsnprintfBuild
vendor/→src/,Realtek/→script/)rtl87x2g_sdksubmodule with board-specific libraries forRTL8777G, RTL8771GUV, and RTL8771GTV