Skip to content

[DM][REGULATOR] Update regulator #11009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Rbb666 merged 2 commits into from
Dec 2, 2025
Merged

[DM][REGULATOR] Update regulator #11009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da440ba
[DM][REGULATOR] Fixup regulator find error
GuEe-GUI Dec 2, 2025
7051df8
[DM][REGULATOR] Support operator delay
GuEe-GUI Dec 2, 2025
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
4 changes: 4 additions & 0 deletions components/drivers/include/drivers/regulator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,17 @@ struct rt_regulator_param
int ramp_delay; /* In uV/usec */
int enable_delay; /* In usec */
int off_on_delay; /* In usec */
int settling_time;
int settling_time_up;
int settling_time_down;
Comment on lines +33 to +35
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

🟢 [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 */
Suggested change
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 */

Copilot uses AI. Check for mistakes.

rt_uint32_t enable_active_high:1;
rt_uint32_t boot_on:1; /* Is enabled on boot */
rt_uint32_t always_on:1; /* Must be enabled */
rt_uint32_t soft_start:1; /* Ramp voltage slowly */
rt_uint32_t pull_down:1; /* Pull down resistor when regulator off */
rt_uint32_t over_current_protection:1; /* Auto disable on over current */
rt_uint32_t ramp_disable:1; /* Disable ramp delay */
};

struct rt_regulator_ops;
Expand Down
65 changes: 60 additions & 5 deletions components/drivers/regulator/regulator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ static rt_err_t regulator_disable(struct rt_regulator_node *reg_np);

rt_err_t rt_regulator_register(struct rt_regulator_node *reg_np)
{
rt_err_t err;
const struct rt_regulator_param *param;

if (!reg_np || !reg_np->dev || !reg_np->param || !reg_np->ops)
Expand All @@ -48,6 +49,16 @@ rt_err_t rt_regulator_register(struct rt_regulator_node *reg_np)

reg_np->parent = RT_NULL;

if ((param->ramp_delay || param->ramp_disable) &&
reg_np->ops->set_ramp_delay)
{
if ((err = reg_np->ops->set_ramp_delay(reg_np, param->ramp_delay)))
{
LOG_E("Set ramp error = %s\n", rt_strerror(err));
return err;
}
}

#ifdef RT_USING_OFW
if (reg_np->dev->ofw_node)
{
Expand Down Expand Up @@ -184,6 +195,40 @@ static rt_uint32_t regulator_get_enable_time(struct rt_regulator_node *reg_np)
return 0;
}

static rt_uint32_t regulator_set_voltage_time(struct rt_regulator_node *reg_np,
int old_uvolt, int new_uvolt)
{
unsigned int ramp_delay = 0;

if (reg_np->param->ramp_delay)
{
ramp_delay = reg_np->param->ramp_delay;
}
else if (reg_np->param->ramp_delay)
{
ramp_delay = reg_np->param->ramp_delay;
Comment on lines +207 to +209
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

🔴 [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;
}
Suggested change
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);

Copilot uses AI. Check for mistakes.
}
else if (reg_np->param->settling_time)
{
return reg_np->param->settling_time;
}
else if (reg_np->param->settling_time_up && new_uvolt > old_uvolt)
{
return reg_np->param->settling_time_up;
}
else if (reg_np->param->settling_time_down && new_uvolt < old_uvolt)
{
return reg_np->param->settling_time_down;
}

if (ramp_delay == 0)
{
return 0;
}

return RT_DIV_ROUND_UP(rt_abs(new_uvolt - old_uvolt), ramp_delay);
}

static void regulator_delay(rt_uint32_t delay)
{
rt_uint32_t ms = delay / 1000;
Expand Down Expand Up @@ -227,14 +272,15 @@ static void regulator_delay(rt_uint32_t delay)
static rt_err_t regulator_enable(struct rt_regulator_node *reg_np)
{
rt_err_t err = RT_EOK;
rt_uint32_t enable_delay = regulator_get_enable_time(reg_np);

if (reg_np->ops->enable)
{
err = reg_np->ops->enable(reg_np);

if (!err)
{
rt_uint32_t enable_delay = regulator_get_enable_time(reg_np);

if (enable_delay)
{
regulator_delay(enable_delay);
Expand Down Expand Up @@ -369,7 +415,17 @@ static rt_err_t regulator_set_voltage(struct rt_regulator_node *reg_np, int min_
err = reg_np->ops->set_voltage(reg_np, min_uvolt, max_uvolt);
}

if (err)
if (!err)
{
rt_uint32_t delay = regulator_set_voltage_time(reg_np,
args.old_uvolt, reg_np->ops->get_voltage(reg_np));

if (delay)
{
regulator_delay(delay);
}
}
else
{
regulator_notifier_call_chain(reg_np, RT_REGULATOR_MSG_VOLTAGE_CHANGE_ERR,
(void *)(rt_base_t)args.old_uvolt);
Expand Down Expand Up @@ -538,7 +594,7 @@ static void regulator_check_parent(struct rt_regulator_node *reg_np)
rt_list_insert_after(&reg_np->parent->children_nodes, &reg_np->list);
rt_ofw_node_put(np);
}
#endif
#endif /* RT_USING_OFW */
}
}

Expand Down Expand Up @@ -581,11 +637,10 @@ struct rt_regulator *rt_regulator_get(struct rt_device *dev, const char *id)
reg_np = rt_ofw_data(np);
rt_ofw_node_put(np);
}
#endif
#endif /* RT_USING_OFW */

if (!reg_np)
{
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

🟠 [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;
}
Suggested change
{
{
reg = rt_err_ptr(-RT_ENOSYS);

Copilot uses AI. Check for mistakes.
reg = rt_err_ptr(-RT_ENOSYS);
goto _end;
}

Expand Down
19 changes: 19 additions & 0 deletions components/drivers/regulator/regulator_dm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,31 @@ rt_err_t regulator_ofw_parse(struct rt_ofw_node *np, struct rt_regulator_param *
{
param->ramp_delay = pval;
}
else
{
param->ramp_disable = RT_TRUE;
}

if (!rt_ofw_prop_read_u32(np, "regulator-enable-ramp-delay", &pval))
{
param->enable_delay = pval;
}

if (!rt_ofw_prop_read_u32(np, "regulator-settling-time-us", &pval))
{
param->settling_time = pval;
}

if (!rt_ofw_prop_read_u32(np, "regulator-settling-time-up-us", &pval))
{
param->settling_time_up = pval;
}

if (!rt_ofw_prop_read_u32(np, "regulator-settling-time-down-us", &pval))
{
param->settling_time_down = pval;
}

param->enable_active_high = rt_ofw_prop_read_bool(np, "enable-active-high");
param->boot_on = rt_ofw_prop_read_bool(np, "regulator-boot-on");
param->always_on = rt_ofw_prop_read_bool(np, "regulator-always-on");
Expand Down