Skip to content

Conversation

vangao-gg
Copy link
Contributor

@vangao-gg vangao-gg commented Aug 25, 2025

Resolves #10635

拉取/合并请求描述:(PR description)

[
本PR增强了STM32硬件I2C驱动(drv_hard_i2c),支持在运行时通过控制接口动态调整I2C速率,并在开启DMA的情况下安全地重建DMA与中断配置,避免通信异常。同时清理了无效/未实现的占空比设置项,统一英文注释,并将控制宏移动到头文件,整体风格与现有代码保持一致。

为什么提交这份PR (why to submit this PR)

  • 现有驱动在初始化时固定速率,运行期无法调整;

你的解决方案是什么 (what is your solution)

  • 新增运行时速率设置路径:通过 i2c_bus->ops->i2c_bus_control(bus, I2C_CTRL_SET_SPEED, &speed) 调整速率。
  • HAL_I2C_Init 之后,若启用DMA,按系统启动流程完整重建 DMA RX/TX 句柄、__HAL_LINKDMA、DMA NVIC 以及 I2C 事件中断 NVIC,保证DMA场景稳定。
  • I2C_CTRL_SET_SPEED 宏迁移至头文件 drv_hard_i2c.h,便于外部调用与统一管理。
  • 将新增、修改处的注释改为英文,并按文件原有风格格式化。

主要变更点:

  • drv_hard_i2c.h
    • 新增宏:#define I2C_CTRL_SET_SPEED (0x01U)
  • drv_hard_i2c.c
    • 新增函数:stm32_i2c_set_speed()(含DMA/IRQ重配置)
    • 控制接口:stm32_i2c_control() 仅支持 I2C_CTRL_SET_SPEED
    • 删除占空比控制分支与相关注释(未实现路径)
    • 注释英文化、代码风格统一

请提供验证的bsp和config (provide the config and bsp)

  • BSP:

    • [请填写实际验证BSP路径,如] bsp/stm32/stm32f411-weact 或 bsp/stm32/stm32f429-st-discovery
  • .config:

    • CONFIG_BSP_USING_I2C=y
    • CONFIG_BSP_USING_HARD_I2C1=y
    • CONFIG_BSP_I2C1_SPEED=400000
    • [如启用DMA则] CONFIG_BSP_I2C1_RX_USING_DMA=y、CONFIG_BSP_I2C1_TX_USING_DMA=y
  • action:

    • [请填写你仓库该PR分支的CI编译链接]
      ]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting等工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到 .github/ALL_BSP_COMPILE.json

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions bot added BSP: STM32 BSP related with ST/STM32 BSP labels Aug 25, 2025
Copy link

github-actions bot commented Aug 25, 2025

📌 Code Review Assignment

🏷️ Tag: bsp_stm32

Reviewers: Liang1795 hamburger-os wdfk-prog

Changed Files (Click to expand)
  • bsp/stm32/libraries/HAL_Drivers/drivers/drv_hard_i2c.c
  • bsp/stm32/libraries/HAL_Drivers/drivers/drv_hard_i2c.h

📊 Current Review Status (Last Updated: 2025-08-25 18:28 CST)

  • Liang1795 Pending Review
  • hamburger-os Pending Review
  • wdfk-prog Pending Review

📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@@ -22,6 +22,9 @@ extern "C"
{
#endif

/* I2C bus control command ids */
#define I2C_CTRL_SET_SPEED (0x01U)
Copy link
Contributor

Choose a reason for hiding this comment

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

这个标志位可以向上提到这里

#define RT_I2C_WR 0x0000 /*!< i2c wirte flag */
#define RT_I2C_RD (1u << 0) /*!< i2c read flag */
#define RT_I2C_ADDR_10BIT (1u << 2) /*!< this is a ten bit chip address */
#define RT_I2C_NO_START (1u << 4) /*!< do not generate START condition */
#define RT_I2C_IGNORE_NACK (1u << 5) /*!< ignore NACK from slave */
#define RT_I2C_NO_READ_ACK (1u << 6) /* when I2C reading, we do not ACK */
#define RT_I2C_NO_STOP (1u << 7) /*!< do not generate STOP condition */
#define RT_I2C_DEV_CTRL_10BIT (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x01)
#define RT_I2C_DEV_CTRL_ADDR (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x02)
#define RT_I2C_DEV_CTRL_TIMEOUT (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x03)
#define RT_I2C_DEV_CTRL_RW (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x04)
#define RT_I2C_DEV_CTRL_CLK (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x05)
#define RT_I2C_DEV_CTRL_UNLOCK (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x06)
#define RT_I2C_DEV_CTRL_GET_STATE (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x07)
#define RT_I2C_DEV_CTRL_GET_MODE (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x08)
#define RT_I2C_DEV_CTRL_GET_ERROR (RT_DEVICE_CTRL_BASE(I2CBUS) + 0x09)

Comment on lines +52 to +165
}

/* Reconfigure base parameters */
rt_memset(i2c_handle, 0, sizeof(I2C_HandleTypeDef));
i2c_handle->Instance = i2c_drv->config->Instance;

#if defined(SOC_SERIES_STM32H7)
/* H7 uses Timing field */
i2c_handle->Init.Timing = i2c_drv->config->timing;
#elif defined(SOC_SERIES_STM32F1) || defined(SOC_SERIES_STM32F4)
/* F1/F4 uses ClockSpeed field */
i2c_handle->Init.ClockSpeed = speed;
i2c_handle->Init.DutyCycle = I2C_DUTYCYCLE_2;
#endif

i2c_handle->Init.OwnAddress1 = 0;
i2c_handle->Init.AddressingMode = I2C_ADDRESSINGMODE_7BIT;
i2c_handle->Init.DualAddressMode = I2C_DUALADDRESS_DISABLE;
i2c_handle->Init.OwnAddress2 = 0;
i2c_handle->Init.GeneralCallMode = I2C_GENERALCALL_DISABLE;
i2c_handle->Init.NoStretchMode = I2C_NOSTRETCH_DISABLE;

if (HAL_I2C_Init(i2c_handle) != HAL_OK)
{
LOG_E("Failed to reinit I2C with new speed");
return -RT_ERROR;
}

#if defined(SOC_SERIES_STM32H7)
/* Optional analog/digital filter settings */
if (HAL_I2CEx_ConfigAnalogFilter(i2c_handle, I2C_ANALOGFILTER_ENABLE) != HAL_OK)
{
LOG_E("Failed to configure analog filter");
ret = -RT_ERROR;
}

if (HAL_I2CEx_ConfigDigitalFilter(i2c_handle, 0) != HAL_OK)
{
LOG_E("Failed to configure digital filter");
ret = -RT_ERROR;
}
#endif

/* If DMA is enabled, reconfigure DMA and NVIC similar to boot init path */
if (i2c_drv->i2c_dma_flag & I2C_USING_RX_DMA_FLAG)
{
i2c_drv->dma.handle_rx.Instance = i2c_drv->config->dma_rx->Instance;
#if defined(SOC_SERIES_STM32F2) || defined(SOC_SERIES_STM32F4) || defined(SOC_SERIES_STM32F7)
i2c_drv->dma.handle_rx.Init.Channel = i2c_drv->config->dma_rx->channel;
#elif defined(SOC_SERIES_STM32L4) || defined(SOC_SERIES_STM32G0) || defined(SOC_SERIES_STM32MP1) || defined(SOC_SERIES_STM32WB) || defined(SOC_SERIES_STM32H7)
i2c_drv->dma.handle_rx.Init.Request = i2c_drv->config->dma_rx->request;
#endif
#ifndef SOC_SERIES_STM32U5
i2c_drv->dma.handle_rx.Init.Direction = DMA_PERIPH_TO_MEMORY;
i2c_drv->dma.handle_rx.Init.PeriphInc = DMA_PINC_DISABLE;
i2c_drv->dma.handle_rx.Init.MemInc = DMA_MINC_ENABLE;
i2c_drv->dma.handle_rx.Init.PeriphDataAlignment = DMA_PDATAALIGN_BYTE;
i2c_drv->dma.handle_rx.Init.MemDataAlignment = DMA_MDATAALIGN_BYTE;
i2c_drv->dma.handle_rx.Init.Mode = DMA_NORMAL;
i2c_drv->dma.handle_rx.Init.Priority = DMA_PRIORITY_LOW;
#endif
#if defined(SOC_SERIES_STM32F2) || defined(SOC_SERIES_STM32F4) || defined(SOC_SERIES_STM32F7) || defined(SOC_SERIES_STM32MP1) || defined(SOC_SERIES_STM32H7)
i2c_drv->dma.handle_rx.Init.FIFOMode = DMA_FIFOMODE_DISABLE;
#endif
HAL_DMA_DeInit(&i2c_drv->dma.handle_rx);
HAL_DMA_Init(&i2c_drv->dma.handle_rx);
__HAL_LINKDMA(&i2c_drv->handle, hdmarx, i2c_drv->dma.handle_rx);
HAL_NVIC_SetPriority(i2c_drv->config->dma_rx->dma_irq, 0, 0);
HAL_NVIC_EnableIRQ(i2c_drv->config->dma_rx->dma_irq);
}

if (i2c_drv->i2c_dma_flag & I2C_USING_TX_DMA_FLAG)
{
i2c_drv->dma.handle_tx.Instance = i2c_drv->config->dma_tx->Instance;
#if defined(SOC_SERIES_STM32F2) || defined(SOC_SERIES_STM32F4) || defined(SOC_SERIES_STM32F7)
i2c_drv->dma.handle_tx.Init.Channel = i2c_drv->config->dma_tx->channel;
#elif defined(SOC_SERIES_STM32L4) || defined(SOC_SERIES_STM32G0) || defined(SOC_SERIES_STM32MP1) || defined(SOC_SERIES_STM32WB) || defined(SOC_SERIES_STM32H7)
i2c_drv->dma.handle_tx.Init.Request = i2c_drv->config->dma_tx->request;
#endif
#ifndef SOC_SERIES_STM32U5
i2c_drv->dma.handle_tx.Init.Direction = DMA_MEMORY_TO_PERIPH;
i2c_drv->dma.handle_tx.Init.PeriphInc = DMA_PINC_DISABLE;
i2c_drv->dma.handle_tx.Init.MemInc = DMA_MINC_ENABLE;
i2c_drv->dma.handle_tx.Init.PeriphDataAlignment = DMA_PDATAALIGN_BYTE;
i2c_drv->dma.handle_tx.Init.MemDataAlignment = DMA_MDATAALIGN_BYTE;
i2c_drv->dma.handle_tx.Init.Mode = DMA_NORMAL;
i2c_drv->dma.handle_tx.Init.Priority = DMA_PRIORITY_LOW;
#endif
#if defined(SOC_SERIES_STM32F2) || defined(SOC_SERIES_STM32F4) || defined(SOC_SERIES_STM32F7) || defined(SOC_SERIES_STM32MP1) || defined(SOC_SERIES_STM32H7)
i2c_drv->dma.handle_tx.Init.FIFOMode = DMA_FIFOMODE_DISABLE;
i2c_drv->dma.handle_tx.Init.FIFOThreshold = DMA_FIFO_THRESHOLD_FULL;
i2c_drv->dma.handle_tx.Init.MemBurst = DMA_MBURST_INC4;
i2c_drv->dma.handle_tx.Init.PeriphBurst = DMA_PBURST_INC4;
#endif
HAL_DMA_DeInit(&i2c_drv->dma.handle_tx);
HAL_DMA_Init(&i2c_drv->dma.handle_tx);
__HAL_LINKDMA(&i2c_drv->handle, hdmatx, i2c_drv->dma.handle_tx);
HAL_NVIC_SetPriority(i2c_drv->config->dma_tx->dma_irq, 1, 0);
HAL_NVIC_EnableIRQ(i2c_drv->config->dma_tx->dma_irq);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这段代码跟stm32_i2c_init重复了吧?
修改stm32_i2c_init名称为stm32_i2c_config,然后进行调用吧?


if (ret == RT_EOK)
{
LOG_D("I2C speed changed to %d Hz successfully (DMA/IRQ reconfigured)", speed);
Copy link
Contributor

Choose a reason for hiding this comment

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

放到control做返回值判断后打印日志更好

Comment on lines +183 to +196
switch (cmd)
{
case I2C_CTRL_SET_SPEED:
if (args != RT_NULL)
{
uint32_t speed = *(uint32_t*)args;
return stm32_i2c_set_speed(bus, speed);
}
return -RT_EINVAL;

default:
return -RT_ENOSYS;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

static rt_err_t i2c_bus_device_control(rt_device_t dev,
int cmd,
void *args)
{
rt_err_t ret;
struct rt_i2c_priv_data *priv_data;
struct rt_i2c_bus_device *bus = (struct rt_i2c_bus_device *)dev->user_data;
RT_ASSERT(bus != RT_NULL);
switch (cmd)
{
/* set 10-bit addr mode */
case RT_I2C_DEV_CTRL_10BIT:
bus->flags |= RT_I2C_ADDR_10BIT;
break;
case RT_I2C_DEV_CTRL_TIMEOUT:
bus->timeout = *(rt_uint32_t *)args;
break;
case RT_I2C_DEV_CTRL_RW:
priv_data = (struct rt_i2c_priv_data *)args;
ret = rt_i2c_transfer(bus, priv_data->msgs, priv_data->number);
if (ret < 0)
{
return -RT_EIO;
}
break;
default:
return rt_i2c_control(bus, cmd, args);
}
return RT_EOK;
}

  • 建议参考这里的写法

RT_NULL
};
static const struct rt_i2c_bus_device_ops stm32_i2c_ops = {
.master_xfer = stm32_i2c_master_xfer, .slave_xfer = RT_NULL, .i2c_bus_control = stm32_i2c_control};
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 按照原来的格式排版^__^

@wdfk-prog
Copy link
Contributor

struct stm32_i2c_config *cfg = i2c_drv->config;

  • 这里可以加一个断言判断是否空指针

  • 这个格式好像没对齐,可以帮忙对齐一下

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BSP: STM32 BSP related with ST/STM32 BSP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] STM32的drv_hard_i2c缺少一个设置I2c速度的接口
4 participants