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
stm32/qspi: Update qspi settings and set pin speed to very-high. #4572
Conversation
Thanks, it looks good. The ST examples do configure the QSPI GPIO to be in very-high-freq mode, so that looks like the right thing to do. I do eventually want to make the clock speed automatically configured based on HCLK (and adjusted automatically if machine.freq() is called) but that can come later. |
Ah yes, some automated calcs of speed would be useful. The speed will still need to be adjustable in the port setting based on chip capabilities. Similarly, it would be nice to set the flash size in a more user friendly format, however the mechanism used in the hal example appears to use runtime calculation which seems entirely unnecessary: https://github.com/micropython/micropython/pull/4563/files#diff-6940483eebdd547e1e00f6a9845b1f5eR325 |
ports/stm32/qspi.c
Outdated
#endif | ||
|
||
#ifndef MICROPY_HW_QSPI_CS_HIGH_TIME | ||
#define MICROPY_HW_QSPI_CS_HIGH_TIME QSPI_CS_HIGH_TIME_2_CYCLE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you're using the HAL constant here... this is a bit inconsistent because the other 3 config values above just use integers. So it might be better to just specify this as an integer like MICROPY_HW_QSPI_PRESCALER, then subtract 1 when it's used below so the integer does represent the number of high cycles.
Note that it doesn't actually build when using the HAL constant, at least not on STM32F769DISC (due to missing include of this HAL header).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I first looked at those defines I thought they were more complex than just (n-1) << offset
, I should have looked closer...
ports/stm32/qspi.c
Outdated
|
||
// Bring up the QSPI peripheral | ||
|
||
__HAL_RCC_QSPI_CLK_ENABLE(); | ||
|
||
QUADSPI->CR = | ||
2 << QUADSPI_CR_PRESCALER_Pos // F_CLK = F_AHB/3 (72MHz when CPU is 216MHz) | ||
(MICROPY_HW_QSPI_PRESCALER-1) << QUADSPI_CR_PRESCALER_Pos // F_CLK = F_AHB/3 (72MHz when CPU is 216MHz) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on this line can go (it exists above).
ports/stm32/qspi.c
Outdated
| 1 << QUADSPI_CR_EN_Pos // enable the peripheral | ||
; | ||
|
||
QUADSPI->DCR = | ||
(MICROPY_HW_QSPIFLASH_SIZE_BITS_LOG2 - 3 - 1) << QUADSPI_DCR_FSIZE_Pos | ||
| 1 << QUADSPI_DCR_CSHT_Pos // nCS stays high for 2 cycles | ||
| MICROPY_HW_QSPI_CS_HIGH_TIME // nCS stays high for 2 cycles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment can go.
The qspi peripheral settings have been updated based on testing on stm32f765 The default speed of the qspi interface is 72Mhz whereas the standard AF pin speed (high) is only rated to 50Mhz
Thanks for updating, merged in 0c60cb1 |
extmod/re1.5: Check and report byte overflow errors in _compilecode.
The qspi pin speeds have been increased as the default speed of the qspi interface is 72Mhz whereas the standard AF pin speed (high) is only rated to 50Mhz
Some of the qspi settings which can be hardware dependent have been made
#define
configurable:This does not change their values directly but allows a port config to override them if needed.
In particular I'd like the clock speed to be configurable, however the others seem like they could be useful too based on recent conversations.