bsp: k230: add i2c driver #10851
bsp: k230: add i2c driver #10851
Conversation
📌 Code Review Assignment🏷️ Tag: bsp_k230Reviewers: unicornx Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-10-27 16:24 CST)
📝 Review Instructions
|
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
Ze-Hou
force-pushed
the
master
branch
3 times, most recently
from
5548acf to
4784db2
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds I2C driver support for the K230 platform, enabling both master and slave modes. The implementation includes hardware register definitions, core driver functionality, and comprehensive test cases.
Key Changes
- Added I2C hardware register definitions and driver implementation supporting both master and slave modes
- Implemented slave mode with EEPROM emulation accessible via device file system
- Added comprehensive unit tests for both master and slave modes with hardware validation
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| bsp/k230/rtconfig.h | Enabled I2C configuration options |
| bsp/k230/drivers/utest/test_i2c.c | Added I2C driver test cases for master/slave modes |
| bsp/k230/drivers/utest/SConscript | Updated build script to include I2C tests |
| bsp/k230/drivers/interdrv/i2c/drv_i2c.h | Added hardware register structure definitions |
| bsp/k230/drivers/interdrv/i2c/drv_i2c.c | Implemented I2C driver core functionality |
| bsp/k230/drivers/interdrv/i2c/SConscript | Added I2C driver build configuration |
| bsp/k230/board/Kconfig | Added Kconfig options for I2C device configuration |
| bsp/k230/.config | Updated configuration to enable I2C support |
bsp/k230/drivers/utest/test_i2c.c
Outdated
| if (pfd.revents & POLLIN) | ||
| { | ||
| int n = read(fd, buffer, TEST_BUFFER_SIZE); | ||
| LOG_I("slave buffer data:\n", n); |
There was a problem hiding this comment.
The LOG_I format string contains a placeholder (\n) but an unused argument 'n' is passed. Either remove the argument 'n' or add a proper format specifier like 'Received %d bytes:\n'.
English: The format string and arguments don't match - 'n' is passed but never used in the format string.
中文：格式字符串和参数不匹配 - 传递了参数 'n' 但在格式字符串中未使用。
| LOG_I("slave buffer data:\n", n); | |
| LOG_I("slave buffer data: received %d bytes\n", n); |
|
|
||
| static ssize_t eeprom_write(struct dfs_file *file, const void *buffer, size_t size, off_t *pos) | ||
| { | ||
| if (*pos>= file->vnode->size) |
There was a problem hiding this comment.
Missing space before the comparison operator. Should be '*pos >= file->vnode->size' for consistent formatting.
English: Spacing issue - there should be a space before '>=' operator.
中文：空格问题 - '>=' 运算符前应该有空格。
| if (*pos>= file->vnode->size) | |
| if (*pos >= file->vnode->size) |
| static off_t eeprom_lseek(struct dfs_file *file, off_t offset, int wherece) | ||
| { | ||
| (void)wherece; |
There was a problem hiding this comment.
Parameter name 'wherece' appears to be a typo. Should likely be 'whence' (standard POSIX lseek parameter name).
English: Corrected spelling of 'wherece' to 'whence'.
中文：将 'wherece' 的拼写更正为 'whence'。
| static off_t eeprom_lseek(struct dfs_file *file, off_t offset, int wherece) | |
| { | |
| (void)wherece; | |
| static off_t eeprom_lseek(struct dfs_file *file, off_t offset, int whence) | |
| { | |
| (void)whence; |
|
|
||
| case I2C_SLAVE_READ_PROCESSED: | ||
| /* The previous byte made it to the bus, get next one */ | ||
| _i2c_slave_read_add_increment(eeprom, val); |
There was a problem hiding this comment.
Missing 'break' statement between case statements. Case I2C_SLAVE_READ_PROCESSED will fall through to I2C_SLAVE_READ_REQUESTED, which may be intentional but should be documented with a /* fall through */ comment if so.
English: Missing break statement causes fall-through behavior that should be explicitly documented.
中文：缺少 break 语句导致向下贯穿行为，如果这是有意的应明确注释。
| _i2c_slave_read_add_increment(eeprom, val); | |
| _i2c_slave_read_add_increment(eeprom, val); | |
| /* fall through */ |
| if(eeprom->poll_event & POLLIN) | ||
| { | ||
| rt_wqueue_wakeup(&eeprom->device.wait_queue, (void*)POLLIN); | ||
| } |
There was a problem hiding this comment.
Missing 'break' statement between case statements. Case I2C_SLAVE_STOP will fall through to I2C_SLAVE_WRITE_REQUESTED, which may cause unintended behavior.
English: Missing break statement causes fall-through from I2C_SLAVE_STOP to I2C_SLAVE_WRITE_REQUESTED.
中文：缺少 break 语句导致从 I2C_SLAVE_STOP 向下贯穿到 I2C_SLAVE_WRITE_REQUESTED。
| } | |
| } | |
| break; |
| @@ -0,0 +1,11 @@ | |||
| # RT-Thread building script for RTC component | |||
There was a problem hiding this comment.
The comment incorrectly states 'RTC component' but this is the I2C component build script.
English: Comment should reference I2C component, not RTC component.
中文：注释应该引用 I2C 组件，而不是 RTC 组件。
| # RT-Thread building script for RTC component | |
| # RT-Thread building script for I2C component |
K230 好像还没有attachconfig |
https://club.rt-thread.org/ask/article/5c41835bb8ff9b41.html 可以加一下很简单。 |
好的 |
这个改动看上去和 i2c 驱动开发没有直接关系啊，我建议另外起一个 pr 改动 我已经新提了一个 issue #10857 来 track 这个事情。本 PR 先确保 i2c 驱动的工作。 |
有关系的，可以只加一个i2c的driver的配置，加了Kconfig之后，对应的配置如何配置，让ci知道，从而ci帮忙编译到i2c的driver，这是可以让ci进行编译检查，从而防止编译不过的代码流入到上游代码中，也防止遗漏文件。 |
要不这样，两位老师，我先把上述格式问题修改了，然后提交了pr，再通过#10857 来增加，i2c的编译本地已经测试过的，这样能较好的区分问题 |
|
这样，就加下面这个文件就行了。我帮你写了。 bsp/k230/.ci/attachconfig/ci.attachconfig.yml |
| * | ||
| * Redistribution and use in source and binary forms, with or without | ||
| * modification, are permitted provided that the following conditions are met: | ||
| * 1. Redistributions of source code must retain the above copyright |
There was a problem hiding this comment.
这个license？哪里的？是符合你的需求吗
许可是，从k230的其他driver搬的
好的，这次我先把i2c的加上 |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
bsp/k230/drivers/utest/test_i2c.c:1
- English: The comments "write data" and "recv addr byte" use inconsistent comment styles. According to RT-Thread coding standards, use
/* */style comments instead of//for code comments.
中文：注释 "write data" 和 "recv addr byte" 使用了不一致的注释风格。根据 RT-Thread 编码标准，应使用/* */风格的注释，而不是//。
/* Copyright (c) 2023, Canaan Bright Sight Co., Ltd
bsp/k230/drivers/utest/test_i2c.c:1
- English: The comments "write data" and "recv addr byte" use inconsistent comment styles. According to RT-Thread coding standards, use
/* */style comments instead of//for code comments.
中文：注释 "write data" 和 "recv addr byte" 使用了不一致的注释风格。根据 RT-Thread 编码标准，应使用/* */风格的注释，而不是//。
/* Copyright (c) 2023, Canaan Bright Sight Co., Ltd
| LOG_E("%s: rt_device_register error %d\n", __func__, ret); | ||
| return -RT_ERROR; | ||
| } | ||
| device->fops = &eeprom_fops; |
There was a problem hiding this comment.
English: The device->fops field is assigned twice (lines 581 and 588). The first assignment is redundant and should be removed.
中文：device->fops 字段被赋值两次（第 581 行和第 588 行）。第一次赋值是冗余的，应该删除。
| * spklen = 0~50ns | ||
| */ | ||
|
|
||
| spklen = 10 / (1e9 / dev->clock); |
There was a problem hiding this comment.
English: The calculation 10 / (1e9 / dev->clock) involves floating-point division that could be optimized to dev->clock * 10 / 1e9 to avoid two division operations and improve performance.
中文：计算 10 / (1e9 / dev->clock) 涉及浮点除法，可以优化为 dev->clock * 10 / 1e9 以避免两次除法操作并提高性能。
| spklen = 10 / (1e9 / dev->clock); | |
| spklen = dev->clock * 10 / 1e9; |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
bsp/k230/drivers/utest/test_i2c.c:1
- English: C++ style comments (//) should be replaced with C-style comments (/* /) to maintain consistency with RT-Thread coding standards and the rest of the codebase which uses C89/C90 style comments.
中文：应使用 C 风格注释（/ */）替换 C++ 风格注释（//），以保持与 RT-Thread 编码标准和代码库其余部分（使用 C89/C90 风格注释）的一致性。
/* Copyright (c) 2023, Canaan Bright Sight Co., Ltd
bsp/k230/drivers/utest/test_i2c.c:1
- English: C++ style comments (//) should be replaced with C-style comments (/* /) to maintain consistency with RT-Thread coding standards and the rest of the codebase which uses C89/C90 style comments.
中文：应使用 C 风格注释（/ */）替换 C++ 风格注释（//），以保持与 RT-Thread 编码标准和代码库其余部分（使用 C89/C90 风格注释）的一致性。
/* Copyright (c) 2023, Canaan Bright Sight Co., Ltd
| help | ||
| Select the I2C0 device mode to be used. |
There was a problem hiding this comment.
English: The help text should be bilingual (English and Chinese) to maintain consistency with RT-Thread's internationalization practices, similar to other BSP configurations in the project.
中文：帮助文本应使用双语（英文和中文）以保持与 RT-Thread 国际化实践的一致性，类似于项目中其他 BSP 配置。
| case I2C_SLAVE_READ_PROCESSED: | ||
| /* The previous byte made it to the bus, get next one */ | ||
| _i2c_slave_read_add_increment(eeprom, val); | ||
| /* fall through */ |
There was a problem hiding this comment.
[nitpick] English: Consider using attribute((fallthrough)) or the C23 [[fallthrough]] attribute for explicit fallthrough annotation in switch statements, which is more modern and compiler-recognized than comments.
中文：考虑使用 attribute((fallthrough)) 或 C23 [[fallthrough]] 属性来显式标注 switch 语句中的穿透，这比注释更现代且能被编译器识别。
| { | ||
| rt_wqueue_wakeup(&eeprom->device.wait_queue, (void*)POLLIN); | ||
| } | ||
| /* fall through */ |
There was a problem hiding this comment.
[nitpick] English: Consider using attribute((fallthrough)) or the C23 [[fallthrough]] attribute for explicit fallthrough annotation in switch statements, which is more modern and compiler-recognized than comments.
中文：考虑使用 attribute((fallthrough)) 或 C23 [[fallthrough]] 属性来显式标注 switch 语句中的穿透，这比注释更现代且能被编译器识别。
| /* fall through */ | |
| __attribute__((fallthrough)); |
Requirement: The BSP for the k230 platform in the RT-Thread repository
does not yet have an I2C driver.
Solution: Provide I2C drivers for the k230 platform in the RT-Thread
repository.
1. support i2c master mode
2. support i2c slave mode
Signed-off-by: Ze-Hou <[email protected]>
| // write data | ||
| _i2c_slave_write(eeprom, val); | ||
| } | ||
| else | ||
| { | ||
| // recv addr byte |
There was a problem hiding this comment.
English: C++-style comments (//) are used instead of C-style comments (/* /). For consistency with RT-Thread coding standards and the rest of the codebase, use C-style comments.
中文：使用了 C++ 风格的注释（//）而不是 C 风格的注释（/ */）。为了与 RT-Thread 编码标准和代码库的其余部分保持一致，请使用 C 风格的注释。
| // write data | |
| _i2c_slave_write(eeprom, val); | |
| } | |
| else | |
| { | |
| // recv addr byte | |
| /* write data */ | |
| _i2c_slave_write(eeprom, val); | |
| } | |
| else | |
| { | |
| /* recv addr byte */ |
| // write data | ||
| _i2c_slave_write(eeprom, val); | ||
| } | ||
| else | ||
| { | ||
| // recv addr byte |
There was a problem hiding this comment.
English: C++-style comments (//) are used instead of C-style comments (/* /). For consistency with RT-Thread coding standards and the rest of the codebase, use C-style comments.
中文：使用了 C++ 风格的注释（//）而不是 C 风格的注释（/ */）。为了与 RT-Thread 编码标准和代码库的其余部分保持一致，请使用 C 风格的注释。
| // write data | |
| _i2c_slave_write(eeprom, val); | |
| } | |
| else | |
| { | |
| // recv addr byte | |
| /* write data */ | |
| _i2c_slave_write(eeprom, val); | |
| } | |
| else | |
| { | |
| /* recv addr byte */ |
3 participants
拉取/合并请求描述：(PR description)
[
庐山派，测试 k230 i2c 从机使用CH341T模块进行通信测试，k230 i2c 主机使用AT24C08作为从机进行通信测试。
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up