Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions components/utilities/rt-link/src/rtlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
#define RT_LINK_THREAD_NAME "rtlink"
#define RT_LINK_THREAD_TICK 20
#define RT_LINK_THREAD_PRIORITY 25
#define RT_LINK_THREAD_STACK_SIZE 1024 /* 32 bytes aligned */
#define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* 32 bytes aligned */
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Maintainability/可维护性] Stack size increase needs justification / 栈大小增加需要说明理由

English: The stack size is increased from 1024 to 4096 bytes (4x increase). While this may be necessary for 64-bit architectures due to larger pointer sizes and stack frame alignment requirements, the change lacks documentation explaining:

  1. Why this specific size (4KB) was chosen
  2. Whether this has been tested on resource-constrained devices
  3. The actual stack usage observed on 64-bit platforms

Consider adding a comment explaining the rationale, or using conditional compilation to set different stack sizes for 32-bit vs 64-bit architectures:

#ifdef ARCH_CPU_64BIT
#define RT_LINK_THREAD_STACK_SIZE   (4 * 1024) /* Larger stack for 64-bit */
#else
#define RT_LINK_THREAD_STACK_SIZE   1024       /* Original size for 32-bit */
#endif

中文:栈大小从 1024 字节增加到 4096 字节(增加了 4 倍)。虽然这对于 64 位架构可能是必要的(由于更大的指针大小和栈帧对齐要求),但此更改缺少文档说明:

  1. 为什么选择这个特定大小(4KB)
  2. 是否在资源受限的设备上进行了测试
  3. 在 64 位平台上观察到的实际栈使用情况

建议添加注释说明理由,或使用条件编译为 32 位和 64 位架构设置不同的栈大小(见示例代码)。

Suggested change
#define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* 32 bytes aligned */
/*
* RT_LINK_THREAD_STACK_SIZE:
* For 64-bit architectures, stack frames and pointers are larger, requiring more stack space.
* For 32-bit architectures, 1024 bytes is sufficient for typical usage.
* If you are running on a resource-constrained device, consider profiling actual stack usage.
*/
#ifdef ARCH_CPU_64BIT
#define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* Larger stack for 64-bit arch */
#else
#define RT_LINK_THREAD_STACK_SIZE 1024 /* Original size for 32-bit arch */
#endif

Copilot uses AI. Check for mistakes.

#define RT_LINK_FRAME_SENT 1
#define RT_LINK_FRAME_NOSEND 0
Expand Down Expand Up @@ -905,8 +905,8 @@ static void rt_link_frame_recv_timeout(void)

static void rt_link_send_timeout(void)
{
LOG_D("send count(%d)", (rt_uint32_t)rt_link_scb->sendtimer.parameter);
if ((rt_uint32_t)rt_link_scb->sendtimer.parameter >= 5)
LOG_D("send count(%d)", (rt_ubase_t)rt_link_scb->sendtimer.parameter);
if ((rt_ubase_t)rt_link_scb->sendtimer.parameter >= 5)
{
rt_timer_stop(&rt_link_scb->sendtimer);
LOG_W("Send timeout, please check the link status!");
Expand All @@ -929,7 +929,7 @@ static void rt_link_send_timeout(void)

static void rt_link_long_recv_timeout(void)
{
if ((rt_uint32_t)rt_link_scb->longframetimer.parameter >= 5)
if ((rt_ubase_t)rt_link_scb->longframetimer.parameter >= 5)
{
LOG_W("long package receive timeout");
rt_link_scb->longframetimer.parameter = 0x00;
Expand Down Expand Up @@ -996,7 +996,7 @@ void rt_link_thread(void *parameter)

static void rt_link_sendtimer_callback(void *parameter)
{
rt_uint32_t count = (rt_uint32_t)rt_link_scb->sendtimer.parameter + 1;
rt_ubase_t count = (rt_ubase_t)rt_link_scb->sendtimer.parameter + 1;
rt_link_scb->sendtimer.parameter = (void *)count;
rt_event_send(&rt_link_scb->event, RT_LINK_SEND_TIMEOUT_EVENT);
}
Expand All @@ -1008,7 +1008,7 @@ static void rt_link_recvtimer_callback(void *parameter)

static void rt_link_receive_long_frame_callback(void *parameter)
{
rt_uint32_t count = (rt_uint32_t)rt_link_scb->longframetimer.parameter + 1;
rt_ubase_t count = (rt_ubase_t)rt_link_scb->longframetimer.parameter + 1;
rt_link_scb->longframetimer.parameter = (void *)count;
rt_event_send(&rt_link_scb->event, RT_LINK_RECV_TIMEOUT_LONG_EVENT);
}
Expand Down Expand Up @@ -1174,8 +1174,8 @@ MSH_CMD_EXPORT(rtlink_status, Display RTLINK status);
* */
rt_err_t rt_link_deinit(void)
{
rt_enter_critical();
rt_link_hw_deinit();
rt_enter_critical();
Comment on lines 1177 to +1178
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Concurrency/并发] Potential race condition in critical section reordering / 临界区重排可能引入竞争条件

English: Moving rt_link_hw_deinit() before rt_enter_critical() creates a potential race condition. If another thread accesses rt_link_scb between rt_link_hw_deinit() and rt_enter_critical(), it may observe inconsistent state.

Consider this ordering instead:

  1. Enter critical section first
  2. Save a local copy of rt_link_scb
  3. Set rt_link_scb = RT_NULL
  4. Exit critical section
  5. Call rt_link_hw_deinit() and other cleanup using the local copy

中文:将 rt_link_hw_deinit() 移到 rt_enter_critical() 之前可能引入竞争条件。如果其他线程在 rt_link_hw_deinit()rt_enter_critical() 之间访问 rt_link_scb,可能会观察到不一致的状态。

建议采用以下顺序:

  1. 先进入临界区
  2. 保存 rt_link_scb 的本地副本
  3. 设置 rt_link_scb = RT_NULL
  4. 退出临界区
  5. 使用本地副本调用 rt_link_hw_deinit() 和其他清理操作

Copilot uses AI. Check for mistakes.
if (rt_link_scb)
{
rt_timer_detach(&rt_link_scb->longframetimer);
Expand Down Expand Up @@ -1235,6 +1235,13 @@ int rt_link_init(void)
rt_slist_init(&rt_link_scb->tx_data_slist);
rt_link_scb->tx_seq = RT_LINK_INIT_FRAME_SEQENCE;

if (RT_EOK != rt_link_hw_init())
{
LOG_E("rtlink hw init failed.");
result = -RT_ERROR;
goto __exit;
}

/* create rtlink core work thread */
thread = rt_thread_create(RT_LINK_THREAD_NAME,
rt_link_thread,
Expand Down
10 changes: 6 additions & 4 deletions components/utilities/rt-link/src/rtlink_dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <sys/stat.h>
#include <sys/statfs.h>
#include <poll.h>
#include <dfs_file.h>
#include <fcntl.h>

int rtlink_fops_open(struct dfs_file *fd)
{
Expand Down Expand Up @@ -82,9 +84,9 @@ int rtlink_fops_ioctl(struct dfs_file *fd, int cmd, void *args)
}
}

int rtlink_fops_read(struct dfs_file *fd, void *buf, size_t count)
ssize_t rtlink_fops_read(struct dfs_file *fd, void *buf, size_t count)
{
int size = 0;
ssize_t size = 0;
rt_device_t device;
device = (rt_device_t)fd->vnode->data;

Expand All @@ -96,9 +98,9 @@ int rtlink_fops_read(struct dfs_file *fd, void *buf, size_t count)
return size;
}

int rtlink_fops_write(struct dfs_file *fd, const void *buf, size_t count)
ssize_t rtlink_fops_write(struct dfs_file *fd, const void *buf, size_t count)
{
int size = 0;
ssize_t size = 0;
rt_device_t device;
device = (rt_device_t)fd->vnode->data;

Expand Down