SPI v3 implementation for stm32h7 #1468
Conversation
| void spi_enable_rx_dma(uint32_t spi); | ||
| void spi_disable_rx_dma(uint32_t spi); | ||
| void spi_set_standard_mode(uint32_t spi, uint8_t mode); | ||
| void spi_reset(uint32_t spi_peripheral); |
There was a problem hiding this comment.
This is the only common function for all three spi !
| @@ -0,0 +1,390 @@ | |||
| /** @addtogroup spi_defines | |||
| * | |||
| * @author @htmlonly © @endhtmlonly 2009 Uwe Hermann <uwe@hermann-uwe.de> | |||
There was a problem hiding this comment.
Did not change this because I copied the file, but the contents is not written by me.
| /* | ||
| * This file is part of the libopencm3 project. | ||
| * | ||
| * Copyright (C) 2023 Sebastien Lorquet <sebastien@lorquet.fr> |
There was a problem hiding this comment.
This is not fully correct. We need to include other contributors who almost got this correct.
lib/stm32/common/spi_common_v3.c
Outdated
| /* | ||
| * This file is part of the libopencm3 project. | ||
| * | ||
| * Copyright (C) 2009 Uwe Hermann <uwe@hermann-uwe.de> |
There was a problem hiding this comment.
Also not correct...
| */ | ||
|
|
||
| int spi_init_master(uint32_t spi, uint8_t clkdiv, bool cpol, bool cpha, | ||
| bool lsbfirst, bool keepgpio) |
There was a problem hiding this comment.
at least do we agree on this definition? should keepgpio be predefined here and modifiable by a pair of dedicated functions?
| SPI_CFG2(spi) |= SPI_CFG2_LSBFRST; | ||
| } else { | ||
| SPI_CFG2(spi) &= ~SPI_CFG2_LSBFRST; | ||
| } |
There was a problem hiding this comment.
BTW why are these spi_init_master functions in all spi versions not calling the individual access routines?
|
I have fixed tab and space issues. |
|
Please review again. |
|
Wow, this pull request has become so cold it will start to freeze. |
|
What about a first shot only introducing Before: After: In practice, given as you said,there is only The the patch would be very small: just addition of the extra code, maintaining the v1/v2 unchanged, easier to review, and then merging (the difficult part) could be done separately? Hope that this helps in deciphering why these changes would get blocked from merging in silence. |
|
first shoot or not this project is dead. |
|
If only I had the time to...
|
|
This would be all about "breaking everything" while refactoring without the risk of breaking the projects relying on libopencm3, much alive themself with commits every weeks. |
|
note: project still dead a year or so later. |
Including reorganization of existing stm32 spi cores.
Rationale:
The old SPI code hierarchy was:
spi_common_v1 -> spi_common_all
spi_common_v2 -> spi_common_all
But the spiv3 changes a lot of small details, so most of the spi_common_all file cannot be reused without conditional compilation (which is not welcome if I understand correctly) : Many functions work with the same logic, but use same (or different) bits in different registers.
So only the really common code was kept in spi_common_all, and everything that is common to spiv1 and spiv2 was moved to a new file spi_v12, resulting in the new hierarchy:
spi_common_v1 -> spi_common_v12 -> spi_common_all
spi_common_v2 -> spi_common_v12 -> spi_common_all
spi_common_v3 -> spi_common_all
I tried to avoid this but it cannot be avoided because most of the bits that were in common regs in v1 and v2 are now in new registers.
I know that the copyright of the files is not correct and I have no idea what else to put there
I did my best not to add dumb mistakes but I am ready to change anything required to bring this upstream.
Thank you for your review.