Skip to content

Conversation

xf-cai
Copy link
Collaborator

@xf-cai xf-cai commented May 21, 2025

1、适配arcs芯片烧录;
2、支持 arcs 6M波特率烧录;
3、支持 emmc 烧录。

Copy link

@xingrz xingrz left a comment

Choose a reason for hiding this comment

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

支持arcs、支持emmc、支持lock/unlock是三个feature,应该分成三个commit(但还是在同一个PR)提交。

其它见review内容。

Copy link

@xingrz xingrz left a comment

Choose a reason for hiding this comment

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

  1. 如review所示
  2. 对PR中的commit的修正应该squash到原本的commit里,避免用一个新的commit反复改动前一个commit的东西

CHIP_TYPE_6 = 6,
CHIP_TYPE_ARCS,
CHIP_TYPE_UNKNOWN = 0xFF,
} chip_type_t;
Copy link

Choose a reason for hiding this comment

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

我感觉叫 cskburn_serial_chip_t 更能保持一致,并且应该放在 cskburn_serial_target_t 上面;另外,core.h和cskburn_serial_open里的uint32_t也还没改

Comment on lines 35 to 40
#define CHIP_TO_STR(chip) \
((chip) == CHIP_TYPE_3 ? "3" \
: (chip) == CHIP_TYPE_4 ? "4" \
: (chip) == CHIP_TYPE_6 ? "6" \
: (chip) == CHIP_TYPE_ARCS ? "arcs" \
: "unknown")
Copy link

Choose a reason for hiding this comment

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

感觉用 static const char数组更方便后续扩展

#include "time_monotonic.h"

#define BAUD_RATE_INIT 115200
#define MAX_ROM_BAUD_RATE 3000000
Copy link

Choose a reason for hiding this comment

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

BAUD_RATE_MAX

}

if (chip == CHIP_TYPE_ARCS) {
ret = serial_config_rts_state(serial, SERIAL_RTS_OFF); // enable RTS
Copy link

Choose a reason for hiding this comment

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

为什么传参是OFF,注释反而是enable?需要在注释里简略说明这个调用的作用(会改变什么行为,为什么需要这么做)

if (burner != NULL && len > 0) {
uint64_t t1 = time_monotonic();

// For CSK6, CMD_CHANGE_BAUD is supported by the ROM, so take advantage
Copy link

Choose a reason for hiding this comment

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

For CSK6 and ARCS...

return ret;
}
}

Copy link

Choose a reason for hiding this comment

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

多删了一行

msleep(500);

uint64_t t2 = time_monotonic();
print_time_spent("writing ram loader", t1, t2);
Copy link

Choose a reason for hiding this comment

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

Writing RAM loader

@xingrz xingrz requested a review from Copilot May 23, 2025 09:06
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements support for ARCS chip burning by introducing new APIs for configuring RTS/DTR signals, adjusting baud rate limits and memory read sizes, and adding eMMC-specific commands. Key changes include:

  • Replacing direct port configuration in the Windows serial module with explicit DCB manipulation.
  • Adding new functions and enums for configuring RTS/DTR states across Win32 and POSIX platforms.
  • Extending the burner application to support ARCS chips and eMMC targets with associated command updates and option parsing adjustments.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
libserial/src/serial_win32.c Updated serial_set_speed and read logic using explicit DCB configuration.
libserial/src/serial_posix.c Introduced serial_config_rts_state and serial_config_dtr_state implementations.
libserial/include/serial.h Added enums and APIs for RTS/DTR configuration.
libcskburn_serial/src/core.{h,c} Modified chip type usage and burning logic to support ARCS chips.
libcskburn_serial/src/cmd.{h,c} Increased FLASH_READ_SIZE and added eMMC command implementations.
libcskburn_serial/include/cskburn_serial.h Updated chip type enums and added new locking/unlocking APIs.
libcskburn_serial/CMakeLists.txt Included burner_arcs_img.c in the build.
cskburn/src/utils.{h,c} Added utility function for address alignment.
cskburn/src/main.c Enhanced chip type mapping and option parsing to support ARCS and eMMC.

ret = serial_do_read(dev, buf, count, timeout);
if (ret < 0) {
return ret;
DWORD wait_time = min(10, (DWORD)(timeout - elapsed));
Copy link

Copilot AI May 23, 2025

Choose a reason for hiding this comment

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

Ensure that the 'min' macro is defined on all targeted Windows platforms; if not, consider defining it or using a more portable alternative to avoid potential compilation issues.

Copilot uses AI. Check for mistakes.

serial_set_dtr(dev->serial, SERIAL_HIGH); // RESET=HIGH
if (dev->chip == CHIP_TYPE_ARCS) {
if (reset_delay > 0) {
serial_set_dtr(dev->serial, !rts_active); // UPDATE=HIGH
Copy link

Copilot AI May 23, 2025

Choose a reason for hiding this comment

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

The use of 'SERIAL_HIGH'/'SERIAL_LOW' with serial_set_rts/dtr seems inconsistent with the boolean APIs expected in some modules; consider unifying the API to avoid potential misuse or confusion when managing signal states.

Copilot uses AI. Check for mistakes.

#define RAM_BLOCK_SIZE (2 * 1024)
#define FLASH_BLOCK_SIZE (4 * 1024)
#define FLASH_READ_SIZE (64)
#define FLASH_READ_SIZE (4096)
Copy link

Copilot AI May 23, 2025

Choose a reason for hiding this comment

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

Add a comment explaining the rationale for increasing FLASH_READ_SIZE from 64 to 4096 to improve read performance, ensuring future maintainers understand the memory and performance trade-offs.

Copilot uses AI. Check for mistakes.

Comment on lines +14 to +21
#ifndef min
#define min(a, b) ((a) < (b) ? (a) : (b))
#endif

#ifndef max
#define max(a, b) ((a) > (b) ? (a) : (b))
#endif

Copy link

Choose a reason for hiding this comment

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

这两个是在支持ARCS的那条提交里用到的,应该放到那条提交里

@xf-cai xf-cai force-pushed the feat/arcs branch 2 times, most recently from 6a1ac44 to 1fc863e Compare May 27, 2025 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants