Skip to content
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.

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

[bsp/stm32] 简化drv_usart中的DMA接收逻辑 #6357

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

BreederBai
Copy link
Contributor

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

[
原有的DMA接收逻辑不是很清晰,我按照V2的代码进行了简化。已经在stm32427和405上进行了测试,功能正常。
]

以下的内容不应该在提交PR时的message修改,修改下述message,PR会被直接关闭。请在提交PR后,浏览器查看PR并对以下检查项逐项check,没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.

当前拉取/合并请求的状态 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

@armink
Copy link
Member

armink commented Aug 31, 2022

有没有做过擦写片内 Flash 同时 DMA 接收数据的测试?

我记得 STM32 在擦写片内 Flash 时,CPU 是没法响应中断的,此时 DMA 依然可以持续的接收数据。等擦写 Flash 完成后,中断 ISR 才会执行,此时可能串口空闲中断晚于串口 DMA 缓存区满中断。所以为了考虑这个情况,专门增加了一些额外逻辑。可以留意下当时的这个 PR #739

@BreederBai
Copy link
Contributor Author

有没有做过擦写片内 Flash 同时 DMA 接收数据的测试?

我记得 STM32 在擦写片内 Flash 时,CPU 是没法响应中断的,此时 DMA 依然可以持续的接收数据。等擦写 Flash 完成后,中断 ISR 才会执行,此时可能串口空闲中断晚于串口 DMA 缓存区满中断。所以为了考虑这个情况,专门增加了一些额外逻辑。可以留意下当时的这个 PR #739

刚测试了下,在DMA 接收数据时,擦内部flash,没有发现有什么异常

@mysterywolf mysterywolf added the BSP: STM32 BSP related with ST/STM32 label Sep 1, 2022
@Guozhanxin Guozhanxin added the +1 Agree +1 label Sep 2, 2022
@mysterywolf mysterywolf added the +2 Agree +2 label Sep 2, 2022
@armink
Copy link
Member

armink commented Sep 2, 2022

有没有做过擦写片内 Flash 同时 DMA 接收数据的测试?
我记得 STM32 在擦写片内 Flash 时,CPU 是没法响应中断的,此时 DMA 依然可以持续的接收数据。等擦写 Flash 完成后,中断 ISR 才会执行,此时可能串口空闲中断晚于串口 DMA 缓存区满中断。所以为了考虑这个情况,专门增加了一些额外逻辑。可以留意下当时的这个 PR #739

刚测试了下,在DMA 接收数据时,擦内部flash,没有发现有什么异常

测试时,在内部 Flash 擦写过程中,是有确认触发 DMA 缓冲区满中断的吗?

@BreederBai
Copy link
Contributor Author

有没有做过擦写片内 Flash 同时 DMA 接收数据的测试?
我记得 STM32 在擦写片内 Flash 时,CPU 是没法响应中断的,此时 DMA 依然可以持续的接收数据。等擦写 Flash 完成后,中断 ISR 才会执行,此时可能串口空闲中断晚于串口 DMA 缓存区满中断。所以为了考虑这个情况,专门增加了一些额外逻辑。可以留意下当时的这个 PR #739

刚测试了下,在DMA 接收数据时,擦内部flash,没有发现有什么异常

测试时,在内部 Flash 擦写过程中,是有确认触发 DMA 缓冲区满中断的吗?

这个不确定,我不是在调试下测试的

@mysterywolf mysterywolf added the in progress PR/issue in progress. label Sep 2, 2022
@BreederBai
Copy link
Contributor Author

测试了下,擦flash的时候还会响应中断,下面是我打断点的地方。但我不清楚打断点的地方对不对。
HAL_FLASHEx_Erase函数中
image

drv_usart.c里dma_recv_isr函数中
image

@armink
Copy link
Member

armink commented Sep 5, 2022

擦写 Flash 的过程中,是不会执行代码的,也包括 ISR ,擦写完成才会触发 ISR。可以选择 F4 芯片,它有个 128KB 扇区,擦除时间较长,利于测试。

@BreederBai
Copy link
Contributor Author

擦写 Flash 的过程中,是不会执行代码的,也包括 ISR ,擦写完成才会触发 ISR。可以选择 F4 芯片,它有个 128KB 扇区,擦除时间较长,利于测试。

哦哦,原来是这样,如果原理上是不会执行的,那我测试的应该有些问题。我还有个疑问,我目前的修改只是对之前代码的整理,如果没有考虑擦flash的情况,那之前的代码也没有考虑。我之所以改这个地方,是因为最近一次关于计算DMA接收长度的更改,导致我的代码接收数据出了问题,那个改法应该是有BUG。

@BreederBai
Copy link
Contributor Author

擦写 Flash 的过程中,是不会执行代码的,也包括 ISR ,擦写完成才会触发 ISR。可以选择 F4 芯片,它有个 128KB 扇区,擦除时间较长,利于测试。

我的设想是在擦除flash前后打两个断点,在DMA中断函数里打一个断点,然后去看是不是擦flash的时候,只有走完擦flash的第二个断点后,才会进中断。按我发的图中那样,在擦flash第二个断点前,就进DMA中断了。
请问,按照我这个思路去测试,该把断点打到哪个地方呢

@BreederBai
Copy link
Contributor Author

@armink 您提到的因为考虑到擦flash的情况,增加的额外逻辑,我使用的这个代码里似乎是有的。

@armink
Copy link
Member

armink commented Sep 6, 2022

建议这块最好增加一个测试用例,单片机接收测试时候,增加数据有效性校验检查,保证即便有 Flash 擦写条件下也不影响正常接收数据的有效性就好了

@thewon86
Copy link
Contributor

thewon86 commented Sep 6, 2022

没看出来哪儿简化了。看看 serialX 里的处理,那个才叫简化
dma_cnt = RT_SERIAL_DMA_BUFSZ - __HAL_DMA_GET_COUNTER(&(uart->dma_rx.handle));
三种中断,同一个公式搞定。

@BreederBai
Copy link
Contributor Author

建议这块最好增加一个测试用例,单片机接收测试时候,增加数据有效性校验检查,保证即便有 Flash 擦写条件下也不影响正常接收数据的有效性就好了

能具体下如何测试吗?我不太清楚该怎么测试。

@BreederBai
Copy link
Contributor Author

没看出来哪儿简化了。看看 serialX 里的处理,那个才叫简化 dma_cnt = RT_SERIAL_DMA_BUFSZ - __HAL_DMA_GET_COUNTER(&(uart->dma_rx.handle)); 三种中断,同一个公式搞定。

如果能用一个公式最好了,没注意serialX中的处理,目前只是按照serialv2中的方法,做了统一。改这个地方也是因为之前的逻辑,接收有些问题。serialX框架中,USB虚拟串口可以使用吗?我目前测试serialv2框架的USB虚拟串口接收不了数据

@thewon86
Copy link
Contributor

thewon86 commented Sep 6, 2022

你说的 “USB虚拟串口” 是 cdc_vcom.c 文件实现的功能吗?
看看这个文件源码就知道了,这个东西跟串口驱动没一点儿关系,借用了 rt_hw_serial_isr 这个函数和 struct rt_uart_ops 这个结构体,很奇葩的想法。

@BreederBai
Copy link
Contributor Author

你说的 “USB虚拟串口” 是 cdc_vcom.c 文件实现的功能吗? 看看这个文件源码就知道了,这个东西跟串口驱动没一点儿关系,借用了 rt_hw_serial_isr 这个函数和 struct rt_uart_ops 这个结构体,很奇葩的想法。

对,是cdc_vcom.c,这个用到了rt_hw_serial_isr,而且和serial有些耦合,所以导致串口架构会影响USB虚拟串口,这个地方做的是有些问题,,,不知道serialX,有没有对cdc_vcom.c做适配

@thewon86
Copy link
Contributor

thewon86 commented Sep 7, 2022

你说的 “USB虚拟串口” 是 cdc_vcom.c 文件实现的功能吗? 看看这个文件源码就知道了,这个东西跟串口驱动没一点儿关系,借用了 rt_hw_serial_isr 这个函数和 struct rt_uart_ops 这个结构体,很奇葩的想法。

对,是cdc_vcom.c,这个用到了rt_hw_serial_isr,而且和serial有些耦合,所以导致串口架构会影响USB虚拟串口,这个地方做的是有些问题,,,不知道serialX,有没有对cdc_vcom.c做适配

上面说了,vcom 和 串口没半毛钱关系,没兴趣去做适配。
自己重写 vcom 驱动吧,把 serial 相关的东西全删掉。

@BreederBai
Copy link
Contributor Author

你说的 “USB虚拟串口” 是 cdc_vcom.c 文件实现的功能吗? 看看这个文件源码就知道了,这个东西跟串口驱动没一点儿关系,借用了 rt_hw_serial_isr 这个函数和 struct rt_uart_ops 这个结构体,很奇葩的想法。

对,是cdc_vcom.c,这个用到了rt_hw_serial_isr,而且和serial有些耦合,所以导致串口架构会影响USB虚拟串口,这个地方做的是有些问题,,,不知道serialX,有没有对cdc_vcom.c做适配

上面说了,vcom 和 串口没半毛钱关系,没兴趣去做适配。 自己重写 vcom 驱动吧,把 serial 相关的东西全删掉。

如果是这样,那感觉有些问题,新功能的引用,不应该破坏原有的功能

@Guozhanxin
Copy link
Member

你说的 “USB虚拟串口” 是 cdc_vcom.c 文件实现的功能吗? 看看这个文件源码就知道了,这个东西跟串口驱动没一点儿关系,借用了 rt_hw_serial_isr 这个函数和 struct rt_uart_ops 这个结构体,很奇葩的想法。

对,是cdc_vcom.c,这个用到了rt_hw_serial_isr,而且和serial有些耦合,所以导致串口架构会影响USB虚拟串口,这个地方做的是有些问题,,,不知道serialX,有没有对cdc_vcom.c做适配

cdc_vcom里的串口代码是将和pc通信的通道注册为了一个串口设备。这样应用层直接打开这个vcom串口就可以和PC通信了。

@BreederBai
Copy link
Contributor Author

你说的 “USB虚拟串口” 是 cdc_vcom.c 文件实现的功能吗? 看看这个文件源码就知道了,这个东西跟串口驱动没一点儿关系,借用了 rt_hw_serial_isr 这个函数和 struct rt_uart_ops 这个结构体,很奇葩的想法。

对,是cdc_vcom.c,这个用到了rt_hw_serial_isr,而且和serial有些耦合,所以导致串口架构会影响USB虚拟串口,这个地方做的是有些问题,,,不知道serialX,有没有对cdc_vcom.c做适配

cdc_vcom里的串口代码是将和pc通信的通道注册为了一个串口设备。这样应用层直接打开这个vcom串口就可以和PC通信了。

serial V2中的虚拟串口似乎有问题,目前有其他人提这个问题吗?有计划多久解决这个问题吗

@BernardXiong BernardXiong merged commit 61e1e31 into RT-Thread:master Sep 22, 2022
wosayttn pushed a commit to OpenNuvoton/rt-thread that referenced this pull request Oct 3, 2022
wdfk-prog added a commit to wdfk-prog/canopen-switches-modbus-gateway that referenced this pull request Nov 4, 2022
wdfk-prog added a commit to wdfk-prog/canopen-switches-modbus-gateway that referenced this pull request Nov 4, 2022
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 in progress PR/issue in progress. +1 Agree +1 +2 Agree +2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants