search

ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.66k stars 2.97k forks source link

The ISR register is never re-evaluated in HAL_DMA_PollForTransfer for STM32F4 #3272

Closed andreaslarssonublox closed 7 years ago

andreaslarssonublox commented 7 years ago

This will cause the function to unnecessarily wait for timeout in some circumstances. Putting 'tmpisr = regs->ISR;' at the end of the while loop solves it for the ODIN driver. See https://github.com/andreaslarssonublox/mbed/commit/df192f81334d6ddd48a98f708b86307084bde2e4

The HAL_DMA_PollForTransfer function is here: mbed-os\targets\TARGET_STM\TARGET_STM32F4\device\stm32f4xx_hal_dma.c

Description

Bug

Target UBLOX_EVK_ODIN_W2 All that are based on TARGET_STM32F4\device\stm32f4xx_hal_dma.c Looks like TARGET_STM32F7\device\stm32f7xx_hal_dma.c has the same problem.

Toolchain: GCC_ARM|ARM|IAR

Toolchain version: GCC - 4.8 2013q4

mbed-cli version: 0.9.10

meed-os sha: 9d8ec61 Merge pull request #3258 from sarahmarshy/build_test_err

DAPLink version: 0221

Expected behavior The function returns when the DMA transfer is completed.

Actual behavior The function returns only after the timeout has passed which makes the system very slow.

Steps to reproduce This has been reproduced with the private repo of the u-blox ODIN-drivers and the fix can therefore only be verified by u-blox for the UBLOX_EVK_ODIN_W2 target.

andreaslarssonublox commented 7 years ago

cc @LMESTM @bcostm

bcostm commented 7 years ago

Thanks for the information.

LMESTM commented 7 years ago

@andreaslarssonublox I suggest that you post the PR from your branch. We'll X-check with HAL DMA owners and then confirm if it's ok to merge.

0xc0170 commented 7 years ago

@andreaslarssonublox Any update for this issue ?

andreaslarssonublox commented 7 years ago

@0xc0170 I will create a pull request today for the TARGET_STM32F4\device\stm32f4xx_hal_dma.c I don't have any STM32F7 board to test with so will not include that one. I will close this one once the PR has been created.

LMESTM commented 7 years ago

@andreaslarssonublox - thank you ! We'll take care and check if the same applies to other families.

andreaslarssonublox commented 7 years ago

Created the PR: https://github.com/ARMmbed/mbed-os/pull/3393

0xc0170 commented 7 years ago

An issue should be closed one the fix is integrated (reviewed and accepted).

Thanks @andreaslarssonublox for providing a fix !

andreaslarssonublox commented 7 years ago

Ok, @0xc0170 maybe I closed it a bit too early. Thanks for the clarification.