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

SPRACINGH7RF Failure to boot caused by re-use of SPI pins. #11157

Closed
hydra opened this issue Dec 24, 2021 · 7 comments
Closed

SPRACINGH7RF Failure to boot caused by re-use of SPI pins. #11157

hydra opened this issue Dec 24, 2021 · 7 comments
Labels
BUG Bugs are excluded from automatically being marked as stale
Milestone

Comments

@hydra
Copy link
Contributor

hydra commented Dec 24, 2021

Describe the bug

Firmware fails to boot when a later unused SPI port uses the same pins as a used SPI port.

To Reproduce

Configure any H7 target to use PB3/PB4/PB5 for SPI3 and allocate the same pins for SPI6.
Likely also using SPI1 and SPI3 would also exhibit the same issue.

image

resource SPI_SCK 3 B03
resource SPI_MISO 3 B04
resource SPI_MOSI 3 B05
set gyro_1_spibus = 3

after disabling the gyro autodection code that fails, resource show still says:

B03: SPI_SCK 6
B04: SPI_MISO 6
B05: SPI_MOSI 6

resource shows:

resource SPI_SCK 2 D03
resource SPI_SCK 3 B03
resource SPI_MISO 2 B14
resource SPI_MISO 3 B04
resource SPI_MOSI 2 B15
resource SPI_MOSI 3 B05

image

after it calls spiInit(SPIDEV_3) the AF's are correct.

image

after the call to spiInit(SPIDEV_6) the AF's are incorrect.

image

The AF's and pins are correct for both devices, but attempting to use SPI3 for the Gyro results in a lock-up as the SPI read never completes as the GPIO pins have the wrong AF now.

Likely the SPI device should not be initialised if the pins are used by another SPI device as per the resource configuration.

The default resource configuration in the target appears as follows:

resource SPI_SCK 2 D03
resource SPI_SCK 3 B03
resource SPI_SCK 6 B03
resource SPI_MISO 2 B14
resource SPI_MISO 3 B04
resource SPI_MISO 6 B04
resource SPI_MOSI 2 B15
resource SPI_MOSI 3 B05
resource SPI_MOSI 6 B05

The target.h file has this:

// SPI3 and SPI6 use the same pins however SPI6 supports independent clock configuration, thus SPI6 used to allow separation between SX1280 and Gyro SPI clocks.
#define USE_SPI_DEVICE_6
#define SPI6_SCK_PIN            PB3
#define SPI6_MISO_PIN           PB4
#define SPI6_MOSI_PIN           PB5
#define SPI6_NSS_PIN            PA15

#define USE_SPI_DEVICE_3
#define SPI3_SCK_PIN            PB3
#define SPI3_MISO_PIN           PB4
#define SPI3_MOSI_PIN           PB5
#define SPI3_NSS_PIN            PA15

Expected behavior

FC shouldn't lockup, SPI3 should work.

However, I'm not 100% certain if it's intended that target definitions can define the same pin for multiple ports.

Flight controller configuration

# version
# Betaflight / SPRACINGH7RF (SP7R) 4.3.0 Dec 24 2021 / 15:02:17 (norevision) MSP API: 1.44

# start the command batch
batch start

board_name SPRACINGH7RF

profile 0

rateprofile 0

# end the command batch
batch end
# resource show all
Currently active IO resource assignments:
(reboot to update)
--------------------
A00: FREE
A01: FREE
A02: FREE
A03: FREE
A04: FREE
A05: FREE
A06: FREE
A07: FREE
A08: FREE
A09: FREE
A10: FREE
A11: USB
A12: USB
A13: SWD
A14: SWD
A15: FREE
B00: FREE
B01: FREE
B03: SPI_SCK 6
B04: SPI_MISO 6
B05: SPI_MOSI 6
B07: LED_STRIP
B08: I2C_SCL 1
B09: I2C_SDA 1
B10: I2C_SCL 2
B11: I2C_SDA 2
B12: RX_SPI_CS
B13: FREE
B14: SPI_MISO 2
B15: SPI_MOSI 2
C00: ADC_CURR
C01: FREE
C02: ADC_EXT
C03: ADC_BATT
C04: FREE
C05: FREE
C06: RX_SPI_EXTI
C07: RX_SPI_EXPRESSLRS_BUSY
C08: SDIO_D0
C09: SDIO_D1
C10: SDIO_D2
C11: SDIO_D3
C12: SDIO_CK
C13: SDCARD_DETECT
C14: SYSTEM
C15: PINIO 1
D00: FREE
D01: FREE
D02: SDIO_CMD
D03: SPI_SCK 2
D04: FREE
D05: SERIAL_TX 2
D06: SERIAL_RX 2
D07: FREE
D08: SYSTEM
D09: SYSTEM
D10: RX_SPI_EXPRESSLRS_RESET
D14: FREE
D15: FREE
E00: FREE
E01: FREE
E03: FREE
E04: BEEPER
E05: OSD
E06: OSD
E11: OSD
E12: OSD
E13: OSD
E14: OSD
E15: OSD
F00: FREE
F01: FREE
F02: FREE
F03: FREE
F04: FREE
F05: FREE
F06: FREE
F07: FREE
F08: FREE
F09: FREE
F10: FREE
F11: FREE
F12: FREE
F13: FREE
F14: FREE
F15: FREE
G00: FREE
G01: FREE
G02: FREE
G03: FREE
G04: FREE
G05: FREE
G06: FREE
G07: FREE
G08: FREE
G09: FREE
G10: FREE
G11: FREE
G12: FREE
G13: FREE
G14: FREE
G15: FREE
H00: FREE
H01: FREE
H02: FREE
H03: FREE
H04: FREE
H05: FREE
H06: FREE
H07: FREE
H08: FREE
H09: FREE
H10: FREE
H11: FREE
H12: FREE
H13: FREE
H14: FREE
H15: FREE

Currently active Timers:
-----------------------
TIM1: FREE
TIM2: FREE
TIM3: FREE
TIM4: FREE
TIM5: FREE
TIM6: FREE
TIM7: FREE
TIM8: FREE
TIM12: FREE
TIM13: FREE
TIM14: FREE
TIM15: FREE
TIM16: FREE
TIM17:
    CH1N: LED_STRIP

Currently active DMA:
--------------------
DMA1 Stream 0: SPI_MOSI 2
DMA1 Stream 1: SPI_MISO 2
DMA1 Stream 2: FREE
DMA1 Stream 3: FREE
DMA1 Stream 4: FREE
DMA1 Stream 5: FREE
DMA1 Stream 6: FREE
DMA1 Stream 7: FREE
DMA2 Stream 0: FREE
DMA2 Stream 1: FREE
DMA2 Stream 2: ADC 3
DMA2 Stream 3: FREE
DMA2 Stream 4: LED_STRIP
DMA2 Stream 5: OSD
DMA2 Stream 6: OSD
DMA2 Stream 7: OSD

Setup / Versions

  • Flight controller: SPRacingH7RF
  • Other components: N/A
  • How are the different components wired up: N/A

Code use for testing is from #11110 (comment), which is based on a recent master which doesn't have any resource allocation / spi initialisation changes that I'm aware of. I verified that by comparing the source tree and looking at SPI related/resource files like pg/bus_spi.c, drivers/bus_spi*, drivers/io.c, etc.

@hydra hydra added the Template: Bug Set by auto_close_issue. label Dec 24, 2021
@github-actions github-actions bot added the Not following template Set by auto_close_issue. label Dec 24, 2021
@github-actions
Copy link

@hydra: This issue is being automatically closed because it does not follow the template.

When you open an issue or feature request you are presented with a template. Follow the guidelines.

You can edit your message to fix this and the issue will be automatically reopened.

@hydra
Copy link
Contributor Author

hydra commented Dec 24, 2021

This sequence also results in the same issue:

defaults
resource SPI_SCK 3 NONE
resource SPI_MISO 3 NONE
resource SPI_MOSI 3 NONE
resource SPI_SCK 6 NONE
resource SPI_MISO 6 NONE
resource SPI_MOSI 6 NONE

diff

# version
# Betaflight / SPRACINGH7RF (SP7R) 4.3.0 Dec 24 2021 / 15:02:17 (norevision) MSP API: 1.44

# start the command batch
batch start

board_name SPRACINGH7RF

# resources
resource SPI_SCK 3 NONE
resource SPI_SCK 6 NONE
resource SPI_MISO 3 NONE
resource SPI_MISO 6 NONE
resource SPI_MOSI 3 NONE
resource SPI_MOSI 6 NONE

profile 0

rateprofile 0

# end the command batch
batch end

save

FC reboots, enter CLI:

resource SPI_SCK 3 B03
resource SPI_MISO 3 B04
resource SPI_MOSI 3 B05
set gyro_1_spibus = 3
save

resource show:

B03: SPI_SCK 6
B04: SPI_MISO 6
B05: SPI_MOSI 6

^ incorrect, should be as follows:

B03: SPI_SCK 3
B04: SPI_MISO 3
B05: SPI_MOSI 3

@github-actions github-actions bot removed the Not following template Set by auto_close_issue. label Dec 24, 2021
@github-actions github-actions bot reopened this Dec 24, 2021
@hydra
Copy link
Contributor Author

hydra commented Dec 24, 2021

On further investigation it seems this issue occurs because the target also uses the same initialisation sequence that CONFIG_IN_EXTERNAL_FLASH uses which results in a call to configureSPIBusses before initEEPROM.

When using (CONFIG_IN_EXTERNAL_FLASH) or (CONFIG_IN_SDCARD and USE_SDCARD_SPI).
Two solutions present themselves:

  1. don't specify the same pins for 2 SPI devices.
  2. after config is loaded, reset the pins uses for the device that loaded the config to the defaults, ensure those pins aren't used by another device, and if they are remove them from the other device.

In the case of the H7RF, SPI isn't used to load the config, OctoSPI is, so changing the SPI pins should be allowed.

When work more on #11110 I'll investigate this further.

@hydra
Copy link
Contributor Author

hydra commented Dec 24, 2021

This can be rescheduled for 4.4.

In the mean time I developed a fix for the H7RF, here:

spracing@911c040

And a release for the above code:

https://github.com/spracing/betaflight/releases/tag/spracingh7-20211224-1642

# diff

# version
# Betaflight / SPRACINGH7RF (SP7R) 4.3.0 Dec 24 2021 / 16:35:01 (911c0401a2) MSP API: 1.44

# start the command batch
batch start

board_name SPRACINGH7RF

# resources
resource SPI_SCK 6 NONE
resource SPI_MISO 6 NONE
resource SPI_MOSI 6 NONE

# master
set gyro_1_spibus = 3

profile 0

rateprofile 0

# end the command batch
batch end
# resource show
...
B03: SPI_SCK 3
B04: SPI_MISO 3
B05: SPI_MOSI 3

@haslinghuis haslinghuis added this to the 4.4 milestone Dec 26, 2021
@haslinghuis haslinghuis added BUG Bugs are excluded from automatically being marked as stale and removed Template: Bug Set by auto_close_issue. labels Dec 26, 2021
@SteveCEvans
Copy link
Member

@hydra Could you please explain why you define the same pins for bus 3 and bus 6.

@hydra
Copy link
Contributor Author

hydra commented Dec 27, 2021

@hydra Could you please explain why you define the same pins for bus 3 and bus 6.

So that you can switch between them using just the gyro_1_spibus and there are appropriate defaults for the pins, I may need to refresh my memory on whether that was intended use of the resource management system.

Additional background:

Different DMA controllers are used for SPI1/3 and SPI6 so it's useful to be able to balance the DMA resources and clock-speed requirements. On the H7RF SPI6 can be clocked at exactly 20Mhz because it has a different clock mux than SPI1/3 and uses a different DMA channel, so for the best utilization of the DMA controllers and the lowest latency gyro spi transfers SPI6 should be used for the Gyro, but currently there's no DMA support for SPI6.

@ctzsnooze ctzsnooze changed the title Failure to boot caused by re-use of SPI pins. SPRACINGH7RF Failure to boot caused by re-use of SPI pins. Apr 15, 2022
@haslinghuis haslinghuis added this to Bug Tracker in Finalizing Firmware 4.3 Release via automation Apr 15, 2022
@haslinghuis haslinghuis modified the milestones: 4.4, 4.3 Apr 15, 2022
@hydra
Copy link
Contributor Author

hydra commented Jun 17, 2022

Gonna close this as my expectations don't match the intended design.

@hydra hydra closed this as completed Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Bugs are excluded from automatically being marked as stale
Projects
None yet
Development

No branches or pull requests

3 participants