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

Fix SKR/GTR PeripheralPins.c #17937

Merged
merged 2 commits into from
May 12, 2020

Conversation

thinkyhead
Copy link
Member

Suggested by @GhostlyCrowd in #17918

Redundant PWM pin definitions are breaking SKR / GTR timers and functionality. This PR removes the redundant definitions.

@GhostlyCrowd
Copy link
Contributor

I know that this will fix the SKR Pro for sure, I'm not sure if it will cause a regression for the GTR since they use the same variant shared files. The person who added GTR support enabled these Pins (in error possibly?) or on purpose for the GTR, if the latter is the case then the solution would b to split the boards in the variants? Or fixing one will break the other if that's the case.

@randellhodges
Copy link
Contributor

randellhodges commented May 10, 2020

What's interesting is that, you don't even need the value there for PA_1 or PA_2 with a bltouch. The servo library is only using TIM5 to use a sort of software PWM for the server/bltouch.

You can completely comment out

{PA_1,  TIM5,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM5, 2, 0)}, // TIM5_CH2    BL-TOUCH-SERVO

and the servo will work.

What I'm curious, because I don't have the knowledge of how this stuff works, is, does the hardware PWM that is defined here use the TIM5 timer? Could that somehow interfere with bltouch?

There is a note in the variant.h that says

//Do not use timer used by PWM pins when possible. See PinMap_PWM in PeripheralPins.c

but TIM5 is a timer used by PWM pins.

@thinkyhead
Copy link
Member Author

I know that this will fix the SKR Pro for sure, I'm not sure if it will cause a regression for the GTR since they use the same variant shared files.

Well that is easy enough to remedy. Let me take that approach first, and then maybe someone will be able to tell us what can be kept in the new file(s) and what can go.

@thinkyhead thinkyhead force-pushed the bf2_skr_gtr_fix_PR branch from 07bc8c4 to b777208 Compare May 10, 2020 23:53
@thinkyhead thinkyhead force-pushed the bf2_skr_gtr_fix_PR branch from b777208 to fabb156 Compare May 11, 2020 00:31
@thinkyhead
Copy link
Member Author

One thing I wasn't too sure about was the MOSI pin change that came along with the GTR submission from @bigtreetech. I also noticed that the chip model numbers are slightly different between SKR and GTR, so maybe there should be other small differences in the variants.

@thinkyhead thinkyhead merged commit a06a0c5 into MarlinFirmware:bugfix-2.0.x May 12, 2020
@thinkyhead thinkyhead deleted the bf2_skr_gtr_fix_PR branch May 12, 2020 04:56
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request May 29, 2020
* Add a separate GTR board/variant.
* Revert SKR Pro MOSI (before 248b7df).
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
* Add a separate GTR board/variant.
* Revert SKR Pro MOSI (before 248b7df).
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
* Add a separate GTR board/variant.
* Revert SKR Pro MOSI (before 248b7df).
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
* Add a separate GTR board/variant.
* Revert SKR Pro MOSI (before 248b7df).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants