[DM][REGULATOR] Update regulator #11009
Conversation
Just not regulator really, user can work ok, so remove the error no. Signed-off-by: GuEe-GUI <2991707448@qq.com>
Signed-off-by: GuEe-GUI <2991707448@qq.com>
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-12-02 16:19 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the RT-Thread regulator subsystem by adding support for voltage settling time configuration and fixing a critical bug in the regulator lookup function. The changes introduce new device tree properties for settling time delays and adjust delay calculation logic to support operator-defined ramp delays.
- Adds settling time configuration support (settling_time, settling_time_up, settling_time_down)
- Fixes regulator lookup error handling when regulator node is not found
- Implements voltage change delay calculation based on settling time parameters
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| components/drivers/include/drivers/regulator.h | Adds three new settling time fields and ramp_disable flag to rt_regulator_param structure |
| components/drivers/regulator/regulator_dm.c | Parses new device tree properties for settling time configuration and sets ramp_disable flag when ramp-delay is absent |
| components/drivers/regulator/regulator.c | Adds regulator_set_voltage_time function, calls set_ramp_delay during registration, fixes enable delay timing, applies voltage settling delay, and updates comment formatting |
| else if (reg_np->param->ramp_delay) | ||
| { | ||
| ramp_delay = reg_np->param->ramp_delay; |
There was a problem hiding this comment.
🔴 [Bug/错误]: Duplicate condition check - both conditions check the same variable reg_np->param->ramp_delay
English: Lines 203-210 contain a logic error. The first if at line 203 checks reg_np->param->ramp_delay, and the else if at line 207 checks the exact same condition again, making it unreachable dead code. Based on the context and PR description about supporting operator delay, this second condition should likely check a different field, possibly reg_np->ops->get_voltage_time or another delay-related parameter.
中文: 第 203-210 行存在逻辑错误。第 203 行的 if 检查 reg_np->param->ramp_delay，而第 207 行的 else if 检查完全相同的条件，使其成为永远无法到达的死代码。根据上下文和关于支持操作延迟的 PR 描述，第二个条件应该检查不同的字段，可能是 reg_np->ops->get_voltage_time 或其他延迟相关参数。
Suggested fix/建议修复:
if (reg_np->param->ramp_delay)
{
ramp_delay = reg_np->param->ramp_delay;
}
else if (reg_np->ops->get_voltage_time) /* Check ops function instead */
{
ramp_delay = reg_np->ops->get_voltage_time(reg_np, old_uvolt, new_uvolt);
}
else if (reg_np->param->settling_time)
{
return reg_np->param->settling_time;
}| else if (reg_np->param->ramp_delay) | |
| { | |
| ramp_delay = reg_np->param->ramp_delay; | |
| else if (reg_np->ops->get_voltage_time) | |
| { | |
| ramp_delay = reg_np->ops->get_voltage_time(reg_np, old_uvolt, new_uvolt); |
| #endif /* RT_USING_OFW */ | ||
|
|
||
| if (!reg_np) | ||
| { |
There was a problem hiding this comment.
🟠 [Bug/错误]: Missing error code assignment when regulator node is not found
English: When reg_np is NULL (line 642), the function jumps to _end without setting an error code in reg. The removed line at 643 previously set reg = rt_err_ptr(-RT_ENOSYS) to indicate the regulator was not found. Without this assignment, the function returns NULL instead of an error pointer, which breaks the error handling contract where callers expect error pointers (as seen in lines 608, 628, 657).
中文: 当 reg_np 为 NULL（第 642 行）时，函数跳转到 _end 而没有在 reg 中设置错误代码。之前第 643 行的代码设置了 reg = rt_err_ptr(-RT_ENOSYS) 来表示未找到调节器。如果没有这个赋值，函数返回 NULL 而不是错误指针，这破坏了错误处理契约，调用者期望错误指针（如第 608、628、657 行所示）。
Suggested fix/建议修复:
if (!reg_np)
{
reg = rt_err_ptr(-RT_ENOSYS);
goto _end;
}| { | |
| { | |
| reg = rt_err_ptr(-RT_ENOSYS); |
| int settling_time; | ||
| int settling_time_up; | ||
| int settling_time_down; |
There was a problem hiding this comment.
🟢 [Documentation/文档]: Missing unit comments for new settling time fields
English: The new fields settling_time, settling_time_up, and settling_time_down (lines 33-35) lack unit specification comments. Following the existing convention in the struct where other time-related fields have "/* In usec /" or "/ In uV/usec */" comments, these new fields should also document their units for consistency and clarity.
中文: 新字段 settling_time、settling_time_up 和 settling_time_down（第 33-35 行）缺少单位说明注释。遵循结构体中现有的约定，其他与时间相关的字段都有 "/* In usec /" 或 "/ In uV/usec */" 注释，这些新字段也应该记录其单位以保持一致性和清晰度。
Suggested fix/建议修复:
int settling_time; /* In usec */
int settling_time_up; /* In usec */
int settling_time_down; /* In usec */| int settling_time; | |
| int settling_time_up; | |
| int settling_time_down; | |
| int settling_time; /* In usec */ | |
| int settling_time_up; /* In usec */ | |
| int settling_time_down; /* In usec */ |
2 participants
拉取/合并请求描述：(PR description)
[
]
当前拉取/合并请求的状态 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