[drivers][power] add CLI helpers and current control support#11301
[drivers][power] add CLI helpers and current control support#11301Evlers wants to merge 5 commits intoRT-Thread:masterfrom
Conversation
- add msh commands to inspect/operate power supplies and regulators - expose snapshot/name helpers so shells and daemons can enumerate nodes safely - add current support for regulator - relax DM-only constraints in power/regulator stacks so basic builds work without DM/OFW - solve the problem of enabling counting for regulator
|
👋 感谢您对 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: 2026-03-29 15:16 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull request overview
This PR extends the RT-Thread power/regulator driver stack with (1) MSH/CLI helpers for introspection and basic operations, (2) current (uA) control support for regulators, and (3) relaxed build-time dependencies so parts of the stack can build without DM/OFW.
Changes:
- Add regulator current control APIs (get/set/support checks) plus notifier messages, and add regulator/power-supply snapshot helpers for safe enumeration.
- Introduce new MSH commands:
regulator(list/info/on/off/getv/setv/getc/setc) andpower_supply(list/show/get). - Relax Kconfig constraints so base regulator/power-supply frameworks can build without DM; add OFW guards/stubs accordingly.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| components/drivers/regulator/regulator_dm.h | Stub regulator_ofw_parse() when OFW is disabled. |
| components/drivers/regulator/regulator_cmd.c | New regulator MSH command for listing/inspecting/controlling regulators. |
| components/drivers/regulator/regulator.c | Add current support, enable-count tweaks, non-OFW name lookup via records, and regulator snapshot APIs. |
| components/drivers/regulator/SConscript | Build the new regulator CLI source. |
| components/drivers/regulator/Kconfig | Remove hard DM dependency for core regulator framework; keep DM/OFW deps for specific regulator implementations. |
| components/drivers/power/supply/supply_cmd.c | New power_supply MSH command using snapshot/name helpers. |
| components/drivers/power/supply/supply.c | Add public name getter + snapshot APIs; add OFW/DM guards for fwdata and naming. |
| components/drivers/power/supply/supply-daemon.c | Use the new public name getter for logging. |
| components/drivers/power/supply/emu-power.c | Adjust device naming for non-DM builds. |
| components/drivers/power/supply/SConscript | Build the new power-supply CLI source. |
| components/drivers/power/supply/Kconfig | Remove hard DM dependency for core power-supply framework; keep DM/OFW deps for specific drivers. |
| components/drivers/include/rtdevice.h | Reorder header includes so regulator/power-supply headers are not gated by DM. |
| components/drivers/include/drivers/regulator.h | Add regulator current APIs + notifier messages + snapshot helper declarations. |
| components/drivers/include/drivers/power_supply.h | Add public name getter + snapshot helper declarations. |
| } | ||
| } | ||
|
|
||
| rt_regulator_nodes_snapshot_free(nodes, count); |
There was a problem hiding this comment.
[bug/缺陷]: Use-after-free in regulator_find_node_by_name()
English: regulator_find_node_by_name() returns a node pointer but releases the snapshot (rt_regulator_nodes_snapshot_free) before returning. Since snapshot_free drops the refcounts, the returned node can become invalid immediately, leading to use-after-free in callers (e.g., regulator_msh_info / regulator_msh_switch). Please keep the snapshot alive while using the node, or redesign the helper to not return a node after snapshot_free (e.g., do the lookup + printing within the snapshot scope).
中文:regulator_find_node_by_name() 在返回 node 指针前先调用了 rt_regulator_nodes_snapshot_free() 释放快照;snapshot_free 会减少引用计数,因此返回的 node 可能立刻失效,调用方(如 regulator_msh_info / regulator_msh_switch)会发生释放后使用(UAF)。建议保持 snapshot 生命周期覆盖 node 的使用范围,或重构为在 snapshot 作用域内完成查找与打印,避免在 snapshot_free 后返回 node 指针。
| rt_regulator_nodes_snapshot_free(nodes, count); | |
| /* NOTE: Do not free the snapshot here to avoid dropping node references | |
| * before the caller finishes using the returned node. The snapshot | |
| * lifetime must be managed in a way that keeps the node valid. */ |
| rt_hw_spin_lock(&_regulator_lock.lock); | ||
|
|
||
| if (regulator_find_record_by_name(reg_np->supply_name)) | ||
| { | ||
| rt_hw_spin_unlock(&_regulator_lock.lock); | ||
| return -RT_EBUSY; | ||
| } |
There was a problem hiding this comment.
[bug/缺陷]: supply_name can be NULL but is used as a required key
English: rt_regulator_register() calls regulator_find_record_by_name(reg_np->supply_name) and the lookup does rt_strcmp(record->reg_np->supply_name, name) without NULL checks. If any regulator node is registered with supply_name == NULL, this will crash. Please validate reg_np->supply_name (and ideally record->reg_np->supply_name) before comparing, and return -RT_EINVAL if supply_name is missing.
中文:rt_regulator_register() 使用 reg_np->supply_name 作为键去 regulator_find_record_by_name() 查重,而查找内部直接 rt_strcmp(record->reg_np->supply_name, name) 且没有做 NULL 检查。如果某个 regulator 节点的 supply_name 为空,会直接崩溃。建议在注册时强制校验 reg_np->supply_name(并在比较时也防御性检查),缺失时返回 -RT_EINVAL。
| struct rt_device *dev = psy ? psy->dev : RT_NULL; | ||
|
|
||
| #ifdef RT_USING_DM | ||
| return rt_dm_dev_get_name(dev); |
There was a problem hiding this comment.
[bug/缺陷]: rt_power_supply_name() may assert in DM builds when psy/dev is NULL
English: In RT_USING_DM builds, rt_power_supply_name() unconditionally calls rt_dm_dev_get_name(dev), which RT_ASSERTs that dev != NULL (see components/drivers/core/dm.c). Since this helper accepts psy and derives dev from it, it should guard against NULL and return a fallback string instead of asserting.
中文:在开启 RT_USING_DM 时,rt_power_supply_name() 直接调用 rt_dm_dev_get_name(dev),而该函数会 RT_ASSERT(dev != NULL)(见 components/drivers/core/dm.c)。由于该 helper 允许传入 psy 并从中取 dev,建议对 NULL 做保护并返回兜底字符串,避免触发断言。
| return rt_dm_dev_get_name(dev); | |
| if (dev) | |
| { | |
| return rt_dm_dev_get_name(dev); | |
| } | |
| return "<no-dev>"; |
| psy->thermal_dev->parent.parent.name = rt_power_supply_name(psy); | ||
| #endif |
There was a problem hiding this comment.
[bug/缺陷]: Assigning to rt_object.name is invalid when RT_NAME_MAX > 0
English: In the non-DM path, psy->thermal_dev->parent.parent.name = rt_power_supply_name(psy); attempts to assign to struct rt_object::name. When RT_NAME_MAX > 0, name is a fixed-size char array (include/rtdef.h), so this won’t compile. Please copy into the array (e.g., rt_strncpy/rt_snprintf) instead of assigning a pointer.
中文:在未启用 DM 的分支中,psy->thermal_dev->parent.parent.name = rt_power_supply_name(psy); 试图给 struct rt_object 的 name 成员赋值。RT_NAME_MAX > 0 时,name 是固定长度 char 数组(见 include/rtdef.h),因此这里会编译失败。应改为把字符串复制到数组中(例如 rt_strncpy/rt_snprintf),而不是指针赋值。
| psy->thermal_dev->parent.parent.name = rt_power_supply_name(psy); | |
| #endif | |
| { | |
| const char *name = rt_power_supply_name(psy); | |
| #if (RT_NAME_MAX > 0) | |
| rt_strncpy(psy->thermal_dev->parent.parent.name, name, RT_NAME_MAX); | |
| psy->thermal_dev->parent.parent.name[RT_NAME_MAX - 1] = '\0'; | |
| #else | |
| psy->thermal_dev->parent.parent.name = (char *)name; | |
| #endif | |
| } | |
| #endif |
| rt_spin_lock(&nodes_lock); | ||
|
|
||
| rt_list_for_each_entry_safe(psy, psy_next, &power_supply_nodes, list) | ||
| { | ||
| rt_spin_unlock(&nodes_lock); | ||
|
|
||
| rt_kprintf("%s %s\n", rt_dm_dev_get_name(psy->dev), type_str[psy->type]); | ||
|
|
||
| rt_power_supply_get_property(psy, RT_POWER_SUPPLY_PROP_STATUS, &propval); | ||
| rt_kprintf("status: %s\n", status_str[propval.intval]), propval.intval = 0; | ||
| total++; | ||
| } | ||
| rt_spin_unlock(&nodes_lock); | ||
|
|
||
| rt_power_supply_get_property(psy, RT_POWER_SUPPLY_PROP_CHARGE_TYPE, &propval); | ||
| rt_kprintf("charge type: %s\n", charge_type_str[propval.intval]), propval.intval = 0; | ||
| if (!total) | ||
| { | ||
| return RT_NULL; | ||
| } | ||
|
|
||
| rt_power_supply_get_property(psy, RT_POWER_SUPPLY_PROP_HEALTH, &propval); | ||
| rt_kprintf("health: %s\n", health_str[propval.intval]), propval.intval = 0; | ||
| nodes = rt_calloc(total, sizeof(*nodes)); | ||
| if (!nodes) | ||
| { | ||
| return RT_NULL; | ||
| } | ||
|
|
||
| if (psy->battery_info) | ||
| { | ||
| struct rt_power_supply_battery_info *info = psy->battery_info; | ||
|
|
||
| rt_power_supply_get_property(psy, RT_POWER_SUPPLY_PROP_CAPACITY, &propval); | ||
| rt_kprintf("capacity: %d%%\n", propval.intval), propval.intval = 0; | ||
|
|
||
| rt_kprintf("technology: %s\n", tech_str[info->technology]); | ||
| rt_kprintf("energy full design: %u uWh\n", info->energy_full_design_uwh); | ||
| rt_kprintf("charge full design: %u uAh\n", info->charge_full_design_uah); | ||
| rt_kprintf("voltage design range: %u~%u uV\n", info->voltage_min_design_uv, info->voltage_max_design_uv); | ||
| rt_kprintf("precharge current: %u uA\n", info->precharge_current_ua); | ||
| rt_kprintf("charge term current: %u uA\n", info->charge_term_current_ua); | ||
| rt_kprintf("charge restart voltage: %u uV\n", info->charge_restart_voltage_uv); | ||
| rt_kprintf("constant charge current max: %u uA\n", info->constant_charge_current_max_ua); | ||
| rt_kprintf("constant charge voltage max: %u uV\n", info->constant_charge_voltage_max_uv); | ||
| rt_kprintf("temp ambient alert range: %+d.%u~%+d.%u C\n", | ||
| info->temp_ambient_alert_min / 1000, rt_abs(info->temp_ambient_alert_min) % 1000, | ||
| info->temp_ambient_alert_max / 1000, rt_abs(info->temp_ambient_alert_max) % 1000); | ||
| rt_kprintf("temp alert range: %+d.%u~%+d.%u C\n", | ||
| info->temp_alert_min / 1000, rt_abs(info->temp_alert_min) % 1000, | ||
| info->temp_alert_max / 1000, rt_abs(info->temp_alert_max) % 1000); | ||
| rt_kprintf("temp range: %+d.%u~%+d.%u C\n", | ||
| info->temp_min / 1000, rt_abs(info->temp_min) % 1000, | ||
| info->temp_max / 1000, rt_abs(info->temp_max) % 1000); | ||
| } | ||
| rt_spin_lock(&nodes_lock); | ||
| rt_list_for_each_entry_safe(psy, psy_next, &power_supply_nodes, list) | ||
| { | ||
| nodes[idx] = psy; | ||
| rt_ref_get(&psy->ref); | ||
| idx++; | ||
| } | ||
| rt_spin_unlock(&nodes_lock); |
There was a problem hiding this comment.
[concurrency/并发]: Snapshot count/fill race can overflow the allocated array
English: rt_power_supply_snapshot() counts power_supply_nodes under the lock, unlocks, allocates nodes, then locks again and fills. If a power supply is registered between the two lock regions, the second pass can append more than total entries and write past the allocated array. Please use a retry loop (recount after re-lock, or verify idx < total and retry) to avoid potential memory corruption.
中文:rt_power_supply_snapshot() 先在加锁下统计 power_supply_nodes 的数量,解锁后分配 nodes,再次加锁填充。如果在两次加锁之间有新的 power supply 注册,第二次遍历可能写入超过 total 的条目,导致数组越界。建议采用重试机制(再次加锁后复核数量或校验 idx < total 并重试)来避免潜在的内存破坏。
| #ifdef RT_USING_DM | ||
| rt_dm_dev_set_name(&ep->parent, "emu-power"); | ||
| #else | ||
| ep->parent.parent.name = "emu-power"; |
There was a problem hiding this comment.
[bug/缺陷]: Assigning to rt_object.name is invalid when RT_NAME_MAX > 0
English: In the non-DM path, ep->parent.parent.name = "emu-power"; assigns to struct rt_object::name. When RT_NAME_MAX > 0, name is a char array (include/rtdef.h), so this won’t compile. Please copy the string into the name buffer (e.g., rt_strncpy/rt_snprintf) instead.
中文:在未启用 DM 的分支中,ep->parent.parent.name = "emu-power"; 给 struct rt_object 的 name 成员做了赋值。RT_NAME_MAX > 0 时 name 是 char 数组(见 include/rtdef.h),这里会编译失败。建议改为把字符串拷贝到 name 缓冲区中(例如 rt_strncpy/rt_snprintf)。
| ep->parent.parent.name = "emu-power"; | |
| rt_snprintf(ep->parent.parent.name, | |
| sizeof(ep->parent.parent.name), | |
| "emu-power"); |
| rt_hw_spin_lock(&_regulator_lock.lock); | ||
|
|
||
| if (regulator_find_record_by_name(reg_np->supply_name)) | ||
| { | ||
| rt_hw_spin_unlock(&_regulator_lock.lock); | ||
| return -RT_EBUSY; | ||
| } | ||
|
|
||
| record = rt_calloc(1, sizeof(*record)); | ||
|
|
||
| if (!record) | ||
| { | ||
| rt_hw_spin_unlock(&_regulator_lock.lock); | ||
| return -RT_ENOMEM; | ||
| } | ||
|
|
There was a problem hiding this comment.
[concurrency/并发]: Allocating memory while holding rt_hw_spin_lock (IRQs disabled)
English: rt_regulator_register() calls rt_calloc() while holding rt_hw_spin_lock(&_regulator_lock.lock). On non-SMP builds rt_hw_spin_lock disables interrupts (see include/rthw.h), so heap allocation here can be unsafe and may deadlock or introduce long IRQ-off latency. Allocate record before taking the lock, then re-check for duplicates under the lock and insert/free accordingly.
中文:rt_regulator_register() 在持有 rt_hw_spin_lock(&_regulator_lock.lock) 期间调用 rt_calloc()。在非 SMP 配置下 rt_hw_spin_lock 会关中断(见 include/rthw.h),在关中断区进行堆内存分配可能导致死锁或过长的关中断延迟。建议先在锁外分配 record,然后加锁后二次检查重名并插入;如发现冲突再释放 record。
| rt_hw_spin_lock(&_regulator_lock.lock); | |
| if (regulator_find_record_by_name(reg_np->supply_name)) | |
| { | |
| rt_hw_spin_unlock(&_regulator_lock.lock); | |
| return -RT_EBUSY; | |
| } | |
| record = rt_calloc(1, sizeof(*record)); | |
| if (!record) | |
| { | |
| rt_hw_spin_unlock(&_regulator_lock.lock); | |
| return -RT_ENOMEM; | |
| } | |
| /* allocate record outside of spinlock to avoid heap allocation with IRQs disabled */ | |
| record = rt_calloc(1, sizeof(*record)); | |
| if (!record) | |
| { | |
| return -RT_ENOMEM; | |
| } | |
| rt_hw_spin_lock(&_regulator_lock.lock); | |
| if (regulator_find_record_by_name(reg_np->supply_name)) | |
| { | |
| rt_hw_spin_unlock(&_regulator_lock.lock); | |
| rt_free(record); | |
| return -RT_EBUSY; | |
| } |
| rt_hw_spin_lock(&_regulator_lock.lock); | ||
| rt_list_for_each_entry(record, &_regulator_records, list) | ||
| { | ||
| nodes[idx] = record->reg_np; | ||
| rt_ref_get(&record->reg_np->ref); | ||
| idx++; | ||
| } | ||
| rt_hw_spin_unlock(&_regulator_lock.lock); | ||
|
|
||
| *count = total; |
There was a problem hiding this comment.
[concurrency/并发]: Snapshot count/fill race can overflow the allocated array
English: rt_regulator_nodes_snapshot() counts _regulator_records under the lock, unlocks, allocates nodes, then re-locks and fills. If a regulator is registered between the two lock regions, the second iteration can see more entries than total, causing out-of-bounds writes (memory corruption). Please use a retry loop (count -> alloc -> lock+fill+verify count unchanged), or another approach that guarantees the filled count never exceeds the allocated size.
中文:rt_regulator_nodes_snapshot() 先在加锁下统计数量,解锁后分配 nodes,再次加锁填充。如果在两次加锁之间有新的 regulator 注册,第二次遍历可能看到比 total 更多的条目,导致写越界(内存破坏)。建议使用重试机制(统计->分配->加锁填充并校验数量未变化),或采用能保证填充数量不超过分配大小的实现方式。
| rt_hw_spin_lock(&_regulator_lock.lock); | |
| rt_list_for_each_entry(record, &_regulator_records, list) | |
| { | |
| nodes[idx] = record->reg_np; | |
| rt_ref_get(&record->reg_np->ref); | |
| idx++; | |
| } | |
| rt_hw_spin_unlock(&_regulator_lock.lock); | |
| *count = total; | |
| idx = 0; | |
| rt_hw_spin_lock(&_regulator_lock.lock); | |
| rt_list_for_each_entry(record, &_regulator_records, list) | |
| { | |
| if (idx >= total) | |
| { | |
| break; | |
| } | |
| nodes[idx] = record->reg_np; | |
| rt_ref_get(&record->reg_np->ref); | |
| idx++; | |
| } | |
| rt_hw_spin_unlock(&_regulator_lock.lock); | |
| *count = idx; |
| menuconfig RT_USING_REGULATOR | ||
| bool "Using Voltage and Current Regulator" | ||
| select RT_USING_ADT | ||
| select RT_USING_ADT_REF | ||
| depends on RT_USING_DM | ||
| default n | ||
|
|
There was a problem hiding this comment.
[documentation/文档]: PR description template is still unfilled (missing What/Why/How + test info)
English: The PR description currently still contains the unfilled template block (the content inside [ ... ] is empty). Please update the PR description to clearly state (1) what changed, (2) why it’s needed, (3) which files/modules are affected, and (4) which BSP/configs have been tested (or how it was verified).
中文:当前 PR 描述仍保留了未填写的模板块([ ... ] 中的必填内容为空)。请补全 PR 描述,明确说明:(1) 做了哪些修改,(2) 为什么需要,(3) 影响了哪些文件/模块,(4) 已验证的 BSP / 配置(或验证方式)。
拉取/合并请求描述:(PR description)
[
为什么提交这份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