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

Update stm32h7.c #148

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Conversation

calithameridi
Copy link
Contributor

This fixes https://github.com/DangerKlippers/danger-klipper/issues/147 where pll_base is hard-coded to a value of 5 MHz, which works nominally for boards with a 25 MHz crystal on HSE but fails for 12 MHz and other values that do not evenly divide into 5 MHz when configuring the DIVM1 prescaler.

Thanks @dalegaard for the help.

May want to test against a board with a 25 MHz crystal before merging, and we may also want to check if this bug exists for other stm32 clock configs

fixes clock configuration error with non-25 MHz crystals
@lraithel15133
Copy link
Contributor

I won't touch this PR because I know absolutely nothing about this, but just wanted to say good work y'all :)

@bwnance
Copy link
Contributor

bwnance commented Feb 7, 2024

@calithameridi have we validated this change for regression on an STM32H7 with a 25mhz crystal as well?

@calithameridi
Copy link
Contributor Author

@calithameridi have we validated this change for regression on an STM32H7 with a 25mhz crystal as well?

No I have not, don't see any particular reason why it won't work but I don't have any 25 MHz boards on hand hence my earlier comment on testing with an OctoPro or such.

@bwnance
Copy link
Contributor

bwnance commented Feb 8, 2024

@calithameridi have we validated this change for regression on an STM32H7 with a 25mhz crystal as well?

No I have not, don't see any particular reason why it won't work but I don't have any 25 MHz boards on hand hence my earlier comment on testing with an OctoPro or such.

Yeah, I gotcha - but I'd like to make sure there's not some bizarre behavior - once you can get someone to test it on an octopro or similar 25mhz crystal chip I'll approve and merge

@bwnance bwnance merged commit 288107c into KalicoCrew:master Feb 9, 2024
2 checks passed
@calithameridi
Copy link
Contributor Author

Again thank you for reviewing, dalegaard for work on the fix, csmonate on Armchair discord for verifying fix with 25 MHz crystal, and derpimus for emotional support.

Would want to get this added to mainline at some point and I anticipate that would be fairly non-difficult since it's a bug and we have the fix.

Might want to see if there are more instances of this behavior in the rest of the stm32 codebase, but that for later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stm32h7.c hardcodes PLL_BASE to 5 MHz causing timing errors
3 participants