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

cpu/stm32_common: add DMA support for all stm32 #9171

Merged
merged 11 commits into from
Jan 8, 2019

Conversation

vincent-d
Copy link
Member

Contribution description

This adds DMA support for all stm32.
STDIO UART works fine on nucleo-f091 and on b-l072z-lrwan1. But SPI doesn't work on b-l072z-lrwan1, not tested on nucleo-f091.
I'm not sure I'll have a lot of time to continue this soon.

Issues/PRs references

Follow-up #7658

@vincent-d vincent-d added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: drivers Area: Device drivers labels May 23, 2018
@vincent-d vincent-d requested a review from aabadie May 23, 2018 17:16
@aabadie
Copy link
Contributor

aabadie commented Aug 29, 2018

I'm not sure I'll have a lot of time to continue this soon.

@vincent-d, is it still the case ?

@vincent-d
Copy link
Member Author

@vincent-d, is it still the case ?

Unfortunately, yes. But if you wish, you can take this over. It should be close to work.

@aabadie
Copy link
Contributor

aabadie commented Dec 21, 2018

This PR needs a rebase.

I can confirm that DMA is not working with SPI on b-l072z-lrwan1. I didn't try either the f091.
I briefly looked at the code but have no idea what's wrong. It will require more investigation.

Otherwise, while looking at the code (correct me if I'm wrong), I have the impression that once periph_dma is required, all UART and SPI devices have to provide a configuration for it.
Would it be more simple/flexible that to make use of DMA in the periph implementation (spi.c/uart.c) only if a define (UART_USE_DMA, SPI_USE_DMA) is set in the board peripheral configuration.
This way it will be possible to keep DMA for UART on b-l072z-lrwan1 but not for SPI (since it doesn't work).

What do you think ?

@vincent-d
Copy link
Member Author

Otherwise, while looking at the code (correct me if I'm wrong), I have the impression that once periph_dma is required, all UART and SPI devices have to provide a configuration for it.

I haven't re-checked it, but as far as I remember, it should be possible to disable dma per uart or spi, in that case, the dma field should be set to DMA_STREAM_UNDEF.

@vincent-d
Copy link
Member Author

Rebased.

I think I found why it was not working on b-l027z-lrwan1: the sx127x driver disables interrupt before using spi functions (

cpsr = irq_disable();
) which breaks dma. I quickly tested with those lines commented and it does not stay stuck anymore, though I get stuck when trying ifconfig (with examples/default) but I don't know if it's related.

So it should be ready for review and merge.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Small changes are required, see below.

the sx127x driver disables interrupt before using spi functions which breaks dma

I made the same analysis last week but I was unsure. We should fix this driver (and maybe others?) before merging this PR.
Normally using ifconfig with examples/default is not supposed to hang.

boards/b-l072z-lrwan1/Makefile.features Outdated Show resolved Hide resolved
boards/nucleo-f091rc/Makefile.features Outdated Show resolved Hide resolved
@vincent-d
Copy link
Member Author

We should fix this driver (and maybe others?) before merging this PR.

I don't know why it has been implemented like that in the first place. I can provide a quick fix though.

@vincent-d
Copy link
Member Author

A quick grep shows that cc110x and sc127x use irq_disable/irq_restore around spi transfers. So they should be fixed.

@vincent-d
Copy link
Member Author

I finally found the remaining issue!

It was a stupid mistake with the channel selection when 2 streams were used at the same time.

It finally works with b-l072z-lrwan1!

I guess I should split the drivers (cc110x and sx127x) parts in one (or 2) PR?

@aabadie
Copy link
Contributor

aabadie commented Dec 27, 2018

I guess I should split the drivers (cc110x and sx127x) parts in one (or 2) PR?

One PR for each would be better. Great that you could fix all issues.

@vincent-d
Copy link
Member Author

See #10669 and #10670

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Tested in nucleo-f091rc: UART and SPI work. Also 313eb03 is merged now.

Now it would be great to provide a DMA configuration for at least one (nucleo) board of each STM32 remaining families (f0, f1, f2, f7, l4). We can share the work here if you don't have the time.

Also a minor nit in dma.c ;)

And we still need #10670 or #10340 to be merged first.

cpu/stm32_common/periph/dma.c Outdated Show resolved Hide resolved
@vincent-d
Copy link
Member Author

Cherry-picked some commits from @aabadie and updated config for b-l475-iot01a, STDIO UART works fine, SPI and second UART are not tested.

@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 7, 2019
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

For me this PR is complete: all STM32s have at least one board with a DMA configuration. The code changes are ok.
I tested the implementation on all boards and all worked.

#10670 is now merged. So please rebase and squash. I triggered Murdock to check if there are not any build issue but I think it will be fine.

ACK

@aabadie
Copy link
Contributor

aabadie commented Jan 7, 2019

I triggered Murdock to check if there are not any build issue but I think it will be fine.

So Murdock found a remaining issue with C++ and the UART configuration struct, the dma related attributes were not at the right place for b-l072z-lrwan1 and b-l475e-iot01a boards. I (force) pushed
2d79213 to fix that.

Now everything is fine. You can squash at any time (be careful the branches have diverged).

@vincent-d
Copy link
Member Author

Rebased and squashed

@aabadie
Copy link
Contributor

aabadie commented Jan 8, 2019

CI is all green, let's go!

@aabadie aabadie merged commit c1524ba into RIOT-OS:master Jan 8, 2019
@vincent-d vincent-d deleted the pr/stm32_dma_all branch January 8, 2019 09:11
@vincent-d
Copy link
Member Author

Thanks @aabadie !

@cladmi
Copy link
Contributor

cladmi commented Jan 17, 2019

A git bisect pointed me to the commit 0b6bccb as breaking the thread tests for iotlab-m3.

I used this test to find the commit:

BOARD=iotlab-m3 make -C tests/thread_basic/ all flash test

Also these other ones are failing but did not bisect them.

- [tests/cond_order](tests/cond_order/test.failed)
- [tests/isr_yield_higher](tests/isr_yield_higher/test.failed)
- [tests/msg_try_receive](tests/msg_try_receive/test.failed)
- [tests/mutex_order](tests/mutex_order/test.failed)
- [tests/pipe](tests/pipe/test.failed)
- [tests/posix_semaphore](tests/posix_semaphore/test.failed)
- [tests/ps_schedstatistics](tests/ps_schedstatistics/test.failed)
- [tests/pthread_barrier](tests/pthread_barrier/test.failed)
- [tests/pthread_cooperation](tests/pthread_cooperation/test.failed)
- [tests/pthread_rwlock](tests/pthread_rwlock/test.failed)
- [tests/rmutex](tests/rmutex/test.failed)
- [tests/thread_basic](tests/thread_basic/test.failed)
- [tests/thread_exit](tests/thread_exit/test.failed)
- [tests/thread_flags](tests/thread_flags/test.failed)
- [tests/thread_msg](tests/thread_msg/test.failed)
- [tests/thread_msg_seq](tests/thread_msg_seq/test.failed)

It also showed me that the commit 08a85fb cannot compile as I guess required changes are introduced later.

/home/harter/work/git/RIOT/cpu/stm32_common/periph/uart.c:207:35: error: 'USART_ISR_TXE' undeclared (first use in this function); did you mean 'USART_SR_TXE'?
     while (!(dev(uart)->ISR_REG & USART_ISR_TXE)) {}
                                   ^~~~~~~~~~~~~
                                   USART_SR_TXE
/home/harter/work/git/RIOT/cpu/stm32_common/periph/uart.c:207:35: note: each undeclared identifier is reported only once for each function it appears in

crest42 added a commit to crest42/RIOT that referenced this pull request Feb 13, 2019
Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.
crest42 added a commit to crest42/RIOT that referenced this pull request Feb 13, 2019
Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.
crest42 added a commit to crest42/RIOT that referenced this pull request Feb 14, 2019
Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.
crest42 added a commit to crest42/RIOT that referenced this pull request Apr 14, 2019
Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.
crest42 added a commit to crest42/RIOT that referenced this pull request Apr 14, 2019
Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.
aabadie pushed a commit to crest42/RIOT that referenced this pull request May 21, 2019
Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.
crest42 added a commit to crest42/RIOT that referenced this pull request Jun 27, 2019
stm32eth: Move to stm32_common periph
cpu/stm32_periph_eth: Rebase to current master branch

- Update DMA to use new vendor headers
- Update send to use iolist. It looks like the packet headers are now transfered as seperate iolist entries which results in the eth periph sending each header as own packet. To fix this a rather ugly workaround is used where the whole iolist content is first copied to a static buffer. This will be fixed soon in another commit
- If MAC is set to zero use luid to generate one
- Small code style fixes

cpu/stm312f7: Add periph config for on-board ethernet
boards/nucleo-f767zi: Add config for on board ethernet
tests/stm32_eth_lwip: Remove board restriction
boards/common/nucleo: Add luid module if stm32 ethernet is used
tests/stm32_eth_gnrc: Add Testcase for gnrc using the stm32 eth periph
stm32_eth: Rework netdev driver layour
tests/stm32_eth_*: Use netdev driver header file for prototypes
stm32_eth: Add auto init for stm32 eth netdev driver
boards/stm32: Enable ethernet conf for nucleo boards
stm32_eth_auto_init: Add dont be pendantic flag
stm32_eth: Remove dma specific stuff from periph_cpu.h

Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.

stm32_eth: Remove eth feature from stm32f4discovery boards
stm32_eth: Migrate to stm32 DMA API
stm32_eth: Add iolist to module deps
stm32_eth: Rework send function to use iolist
stm32_eth: Fix ci build warnings
stm32_eth: Fix bug introduced with iolist usage
stm32_eth: Remove redundant static buffer
stm32_eth: Fix feature dependencies
stm32_eth: Fix wrong header guard name
stm32_eth: Implement correct l2 netstats interface
stm32_eth: Rename public functions to stm32_eth_*
stm32_eth: Fix doccheck
stm32_eth: Move register DEFINE to appropriate header file
stm32_eth: remove untested configuration for f446ze boards
stm32_eth: Move periph configuration struct to stm32_common
stm32_eth: Fix naming of eth_phy_read and eth_phy_write
stm32_eth: Remove obsolete test applications
crest42 added a commit to crest42/RIOT that referenced this pull request Jul 4, 2019
stm32eth: Move to stm32_common periph
cpu/stm32_periph_eth: Rebase to current master branch

- Update DMA to use new vendor headers
- Update send to use iolist. It looks like the packet headers are now transfered as seperate iolist entries which results in the eth periph sending each header as own packet. To fix this a rather ugly workaround is used where the whole iolist content is first copied to a static buffer. This will be fixed soon in another commit
- If MAC is set to zero use luid to generate one
- Small code style fixes

cpu/stm312f7: Add periph config for on-board ethernet
boards/nucleo-f767zi: Add config for on board ethernet
tests/stm32_eth_lwip: Remove board restriction
boards/common/nucleo: Add luid module if stm32 ethernet is used
tests/stm32_eth_gnrc: Add Testcase for gnrc using the stm32 eth periph
stm32_eth: Rework netdev driver layour
tests/stm32_eth_*: Use netdev driver header file for prototypes
stm32_eth: Add auto init for stm32 eth netdev driver
boards/stm32: Enable ethernet conf for nucleo boards
stm32_eth_auto_init: Add dont be pendantic flag
stm32_eth: Remove dma specific stuff from periph_cpu.h

Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.

stm32_eth: Remove eth feature from stm32f4discovery boards
stm32_eth: Migrate to stm32 DMA API
stm32_eth: Add iolist to module deps
stm32_eth: Rework send function to use iolist
stm32_eth: Fix ci build warnings
stm32_eth: Fix bug introduced with iolist usage
stm32_eth: Remove redundant static buffer
stm32_eth: Fix feature dependencies
stm32_eth: Fix wrong header guard name
stm32_eth: Implement correct l2 netstats interface
stm32_eth: Rename public functions to stm32_eth_*
stm32_eth: Fix doccheck
stm32_eth: Move register DEFINE to appropriate header file
stm32_eth: remove untested configuration for f446ze boards
stm32_eth: Move periph configuration struct to stm32_common
stm32_eth: Fix naming of eth_phy_read and eth_phy_write
stm32_eth: Remove obsolete test applications
aabadie pushed a commit to crest42/RIOT that referenced this pull request Jul 4, 2019
stm32eth: Move to stm32_common periph
cpu/stm32_periph_eth: Rebase to current master branch

- Update DMA to use new vendor headers
- Update send to use iolist. It looks like the packet headers are now transfered as seperate iolist entries which results in the eth periph sending each header as own packet. To fix this a rather ugly workaround is used where the whole iolist content is first copied to a static buffer. This will be fixed soon in another commit
- If MAC is set to zero use luid to generate one
- Small code style fixes

cpu/stm312f7: Add periph config for on-board ethernet
boards/nucleo-f767zi: Add config for on board ethernet
tests/stm32_eth_lwip: Remove board restriction
boards/common/nucleo: Add luid module if stm32 ethernet is used
tests/stm32_eth_gnrc: Add Testcase for gnrc using the stm32 eth periph
stm32_eth: Rework netdev driver layour
tests/stm32_eth_*: Use netdev driver header file for prototypes
stm32_eth: Add auto init for stm32 eth netdev driver
boards/stm32: Enable ethernet conf for nucleo boards
stm32_eth_auto_init: Add dont be pendantic flag
stm32_eth: Remove dma specific stuff from periph_cpu.h

Looks like @vincent-d and @aabadie implemented this in PR RIOT-OS#9171 and 021697a with the same interface.

stm32_eth: Remove eth feature from stm32f4discovery boards
stm32_eth: Migrate to stm32 DMA API
stm32_eth: Add iolist to module deps
stm32_eth: Rework send function to use iolist
stm32_eth: Fix ci build warnings
stm32_eth: Fix bug introduced with iolist usage
stm32_eth: Remove redundant static buffer
stm32_eth: Fix feature dependencies
stm32_eth: Fix wrong header guard name
stm32_eth: Implement correct l2 netstats interface
stm32_eth: Rename public functions to stm32_eth_*
stm32_eth: Fix doccheck
stm32_eth: Move register DEFINE to appropriate header file
stm32_eth: remove untested configuration for f446ze boards
stm32_eth: Move periph configuration struct to stm32_common
stm32_eth: Fix naming of eth_phy_read and eth_phy_write
stm32_eth: Remove obsolete test applications
aabadie pushed a commit to crest42/RIOT that referenced this pull request Jul 4, 2019
stm32eth: Move to stm32_common periph
cpu/stm32_periph_eth: Rebase to current master branch

- Update DMA to use new vendor headers
- Update send to use iolist. It looks like the packet headers are now transfered as seperate iolist entries which results in the eth periph sending each header as own packet. To fix this a rather ugly workaround is used where the whole iolist content is first copied to a static buffer. This will be fixed soon in another commit
- If MAC is set to zero use luid to generate one
- Small code style fixes

cpu/stm312f7: Add periph config for on-board ethernet
boards/nucleo-f767zi: Add config for on board ethernet
tests/stm32_eth_lwip: Remove board restriction
boards/common/nucleo: Add luid module if stm32 ethernet is used
tests/stm32_eth_gnrc: Add Testcase for gnrc using the stm32 eth periph
stm32_eth: Rework netdev driver layour
tests/stm32_eth_*: Use netdev driver header file for prototypes
stm32_eth: Add auto init for stm32 eth netdev driver
boards/stm32: Enable ethernet conf for nucleo boards
stm32_eth_auto_init: Add dont be pendantic flag
stm32_eth: Remove dma specific stuff from periph_cpu.h

Looks like this was implemented in PR RIOT-OS#9171 and 021697a with the same interface.

stm32_eth: Remove eth feature from stm32f4discovery boards
stm32_eth: Migrate to stm32 DMA API
stm32_eth: Add iolist to module deps
stm32_eth: Rework send function to use iolist
stm32_eth: Fix ci build warnings
stm32_eth: Fix bug introduced with iolist usage
stm32_eth: Remove redundant static buffer
stm32_eth: Fix feature dependencies
stm32_eth: Fix wrong header guard name
stm32_eth: Implement correct l2 netstats interface
stm32_eth: Rename public functions to stm32_eth_*
stm32_eth: Fix doccheck
stm32_eth: Move register DEFINE to appropriate header file
stm32_eth: remove untested configuration for f446ze boards
stm32_eth: Move periph configuration struct to stm32_common
stm32_eth: Fix naming of eth_phy_read and eth_phy_write
stm32_eth: Remove obsolete test applications
fjmolinas pushed a commit to fjmolinas/RIOT that referenced this pull request Aug 4, 2019
stm32eth: Move to stm32_common periph
cpu/stm32_periph_eth: Rebase to current master branch

- Update DMA to use new vendor headers
- Update send to use iolist. It looks like the packet headers are now transfered as seperate iolist entries which results in the eth periph sending each header as own packet. To fix this a rather ugly workaround is used where the whole iolist content is first copied to a static buffer. This will be fixed soon in another commit
- If MAC is set to zero use luid to generate one
- Small code style fixes

cpu/stm312f7: Add periph config for on-board ethernet
boards/nucleo-f767zi: Add config for on board ethernet
tests/stm32_eth_lwip: Remove board restriction
boards/common/nucleo: Add luid module if stm32 ethernet is used
tests/stm32_eth_gnrc: Add Testcase for gnrc using the stm32 eth periph
stm32_eth: Rework netdev driver layour
tests/stm32_eth_*: Use netdev driver header file for prototypes
stm32_eth: Add auto init for stm32 eth netdev driver
boards/stm32: Enable ethernet conf for nucleo boards
stm32_eth_auto_init: Add dont be pendantic flag
stm32_eth: Remove dma specific stuff from periph_cpu.h

Looks like this was implemented in PR RIOT-OS#9171 and 021697a with the same interface.

stm32_eth: Remove eth feature from stm32f4discovery boards
stm32_eth: Migrate to stm32 DMA API
stm32_eth: Add iolist to module deps
stm32_eth: Rework send function to use iolist
stm32_eth: Fix ci build warnings
stm32_eth: Fix bug introduced with iolist usage
stm32_eth: Remove redundant static buffer
stm32_eth: Fix feature dependencies
stm32_eth: Fix wrong header guard name
stm32_eth: Implement correct l2 netstats interface
stm32_eth: Rename public functions to stm32_eth_*
stm32_eth: Fix doccheck
stm32_eth: Move register DEFINE to appropriate header file
stm32_eth: remove untested configuration for f446ze boards
stm32_eth: Move periph configuration struct to stm32_common
stm32_eth: Fix naming of eth_phy_read and eth_phy_write
stm32_eth: Remove obsolete test applications
olegart pushed a commit to unwireddevices/RIOT that referenced this pull request Sep 10, 2019
stm32eth: Move to stm32_common periph
cpu/stm32_periph_eth: Rebase to current master branch

- Update DMA to use new vendor headers
- Update send to use iolist. It looks like the packet headers are now transfered as seperate iolist entries which results in the eth periph sending each header as own packet. To fix this a rather ugly workaround is used where the whole iolist content is first copied to a static buffer. This will be fixed soon in another commit
- If MAC is set to zero use luid to generate one
- Small code style fixes

cpu/stm312f7: Add periph config for on-board ethernet
boards/nucleo-f767zi: Add config for on board ethernet
tests/stm32_eth_lwip: Remove board restriction
boards/common/nucleo: Add luid module if stm32 ethernet is used
tests/stm32_eth_gnrc: Add Testcase for gnrc using the stm32 eth periph
stm32_eth: Rework netdev driver layour
tests/stm32_eth_*: Use netdev driver header file for prototypes
stm32_eth: Add auto init for stm32 eth netdev driver
boards/stm32: Enable ethernet conf for nucleo boards
stm32_eth_auto_init: Add dont be pendantic flag
stm32_eth: Remove dma specific stuff from periph_cpu.h

Looks like this was implemented in PR RIOT-OS#9171 and 021697a with the same interface.

stm32_eth: Remove eth feature from stm32f4discovery boards
stm32_eth: Migrate to stm32 DMA API
stm32_eth: Add iolist to module deps
stm32_eth: Rework send function to use iolist
stm32_eth: Fix ci build warnings
stm32_eth: Fix bug introduced with iolist usage
stm32_eth: Remove redundant static buffer
stm32_eth: Fix feature dependencies
stm32_eth: Fix wrong header guard name
stm32_eth: Implement correct l2 netstats interface
stm32_eth: Rename public functions to stm32_eth_*
stm32_eth: Fix doccheck
stm32_eth: Move register DEFINE to appropriate header file
stm32_eth: remove untested configuration for f446ze boards
stm32_eth: Move periph configuration struct to stm32_common
stm32_eth: Fix naming of eth_phy_read and eth_phy_write
stm32_eth: Remove obsolete test applications
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants