Skip to content

Fix G4 SPI clock being double what it should be #14207

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

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

SteveCEvans
Copy link
Member

@SteveCEvans SteveCEvans commented Jan 26, 2025

G4 SPI clock was observed to be double what it should be.

Using a check for a number of frequencies using this temporary patch to the BMI270 driver:

image

We get the following expected frequencies, but we actually see double that.

image

As can be seen below:

image

After this fix, we see the following:

image

Previously the divisor values were half of what they should be, and the frequency was consequently double.

For FC's using the BMI270 gyro the consequence had been that the gyro was being clocked at 10.5MHz (one 8th of 84MHz), only just above its max permissible value of 10MHz. A consequence of this PR is that the frequency is now correctly set below 10MHz, to 5.25MHz, thus, but it's still plenty fast enough:

image

Copy link

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #14207 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@haslinghuis haslinghuis added this to the 4.6 milestone Jan 26, 2025
@crteensy
Copy link
Contributor

I can confirm that my prototype FC now has expected SCK frequency:
make EXTRA_FLAGS='-DUSE_CLOCK_SOURCE_HSI -DICM426XX_CLOCK=24000000' YOLOSOLOG4ELRS
results in 21 MHz SCK (scoped)

@haslinghuis
Copy link
Member

haslinghuis commented Jan 26, 2025

@ledvinap @blckmn @nerdCopter we still have the git submodule present.

https://superuser.com/questions/1301581/get-git-commit-a-to-ignore-submodules

@SteveCEvans
Copy link
Member Author

I noticed src/config was being included which seems like a bad thing, so I've removed it here, but I'll obviously align to the consensus on what should be happening.

@SteveCEvans
Copy link
Member Author

Not sure what's going on with src/config, but this PR now only includes the desired delta from master.

@SteveCEvans
Copy link
Member Author

On a related topic, as a check, I've confirmed the I2C clock speed to be correct. It's configured to be 800kHz, as per

#define I2C1_CLOCKSPEED 800
, thus:

image

And an access to the baro shows this to be so.

image

uint32_t spiClk = SystemCoreClock / 2;
#elif defined(STM32H7)
uint32_t spiClk = 100000000;
#elif defined(STM32G4)
uint32_t spiClk = SystemCoreClock;
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, this should be HAL_RCC_GetPCLK1Freq or similar driver function. It will probably work for other processors too (checked F7), but it may be slightly complicated by SPI using both APB1 and APB2 (and thus PCLK1 / PCLK2), but that can be resolved using peripheral address. This is already implemented for timer clocks.

This special-casing will work, but it is mixing a lot of assumptions. But the simplicity may be worth it.

@blckmn
Copy link
Member

blckmn commented Jan 26, 2025

@ledvinap @blckmn @nerdCopter we still have the git submodule present.

https://superuser.com/questions/1301581/get-git-commit-a-to-ignore-submodules

It is possible this is based on a master that does not include the dirty check on submodules.

@blckmn
Copy link
Member

blckmn commented Jan 26, 2025

@SteveCEvans you could try: git config submodule.<your module path>.ignore all

@haslinghuis haslinghuis merged commit 3d9dcbc into betaflight:master Jan 27, 2025
30 checks passed
@haslinghuis
Copy link
Member

@SteveCEvans this needs a back port ?

@SteveCEvans
Copy link
Member Author

@SteveCEvans this needs a back port ?

Yes, will do. Spent the evening making cables to debug GPS issues.

@haslinghuis
Copy link
Member

haslinghuis commented Jan 27, 2025

Using breadboard as bridge with dupont on both sides :)
But BF connector standard should make it plug and play

IMG_20250128_000704_HDR

@SteveCEvans
Copy link
Member Author

SteveCEvans commented Jan 28, 2025

BF standard JST-SH GPS connector to logic analyser plug with a six way DuPont connector to JST-SH that can be easily reconfigured to suit alternate flight controllers.

IMG_1097

SteveCEvans added a commit to SteveCEvans/betaflight that referenced this pull request Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

6 participants