-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Extend SPI support to fully support all SPI devices on STM32F429. #1694
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
Conversation
spi_clock = HAL_RCC_GetPCLK2Freq(); | ||
} else { | ||
if ( (self->spi->Instance == SPI2) || | ||
(self->spi->Instance == SPI3) ) { | ||
// SPI2 and SPI3 are on APB1 |
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.
Reorder avoids ugly #ifdef(SPI4) constructs as below.
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.
Yes, good.
4fcf700
to
4e7d051
Compare
Thanks for this! I see that you only enabled SPI5 for the F429DISCO... does it have SPI4 and SPI6? Regarding the macros (HW_ENABLE_SPIx), we should change them to be the same as how I2C is done. Ie, you define the exact pins you want to use in mpconfigboard.h, and if they're not defined then the bus is not enabled. I'll do this as a separate patch and then you can rebase against that. |
Also, the example python script should be a separate PR (probably it should go in drivers/ directory). |
Ok, I adjusted the macros for SPI, see f7c4f9a. It should now be simpler to add SPI4/5/6. |
4e7d051
to
ee4a360
Compare
Thank you for your feedback. The example python script for STM32F4DISC (staccel.py) is located in the boards subfolder as well. I prop 8000 ose to move this script as well to the proposed location. I will send my script in a separate PR. I have merged your propositions to this PR. |
ee4a360
to
b4fa1ca
Compare
//#define MICROPY_HW_SPI1_NSS (pin_A4) | ||
//#define MICROPY_HW_SPI1_SCK (pin_A5) | ||
//#define MICROPY_HW_SPI1_MISO (pin_A6) | ||
//#define MICROPY_HW_SPI1_MOSI (pin_A7) |
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.
Definition of SPI2 collides with usage of USE_USB_HS_IN_FS. Therfore I do not add it here.
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'd recommend that you add it, but do something like this:
#if USE_USB_HS_IN_FS
// The HS USB uses B14 & B15 for D- and D+
#else
#define MICROPY_HW_SPI2_NSS (pin_B12)
#define MICROPY_HW_SPI2_SCK (pin_B13)
#define MICROPY_HW_SPI2_MISO (pin_B14)
#define MICROPY_HW_SPI2_MOSI (pin_B15)
#endif
b4fa1ca
to
3a8ca24
Compare
@@ -1,3 +1,5 @@ | |||
#include STM32_HAL_H |
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.
Must be included to get the definition of USE_USB_HS_IN_FS.
Added inputs of dhylands and repushed it. |
3a8ca24
to
2573187
Compare
Thanks! Merged in c5d8ffe. |
Some remarks:
Tests done on the original pull request:
Attached BusPirate to the Output pins for SPI4, SPI5 and SPI6 where I could detect the read_id call from the python script. For SPI5 the MEMS connected on STM32F429-DISCO gives sensible values.
Please change commit comment to "Extend SPI support to fully support all SPI devices on STM32F429." and do not mention the example script. This will be part of another PR.