Skip to content

Conversation

@rogerlz
Copy link
Contributor

@rogerlz rogerlz commented Jan 3, 2026

This PR reverts #701 and cherry-picks Klipper PR #7091, as both provide similar functionality. Aligning with #7091 makes it significantly easier to stay in sync with upstream Klipper going forward.

It also includes Klipper PR #7158 and commit 7377da63b7b1ca4b62fb7d843b57224f332b37c7.

Klipper PR #7156 was intentionally skipped, as it is superseded by PR #7158.

rogerlz and others added 9 commits January 3, 2026 23:05
Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
To support the cartographer, it is required to output 24 MHz.
With current defaults max output frequency is:
48 MHz/256 = 187.5 KHz
Adjusting the PWM scale allows for ramping up the frequency.

To not mess up with existing PWM users,
define the STM32-specific command.

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
@rogerlz rogerlz changed the title Klipper pr 7091 Sync PWM implementation with upstream Klipper Jan 3, 2026
@garethky
Copy link
Contributor

garethky commented Jan 5, 2026

I tried this on the STM32F072 chip on the ALPS board. It needs PA3 to generate an 8MHz clock signal. With the config:

[static_pwm_clock alps_clock]
pin: alps-mcu:PA3
frequency: 8000000

Results in the error:

MCU 'alps-mcu' error during config: PWM already programmed with different pulse duration
Once the underlying issue is corrected, use the
"FIRMWARE_RESTART" command to reset the firmware, reload the
config, and restart the host software.
Error configuring printer

This is because the default clock for PA3 is TIM2, which is klipper's own reference clock. This gets set up before any gpio pins and so its prescaler is already configured. This used to be OK because you couldn't reprogram the prescaler, and it still is OK for some PWM frequencies.

Fixing this in this case means using a timer that's not going to conflict with klipper's main clock. In this case changing this line from:

    {TIM2, GPIO('A', 3), 4, GPIO_FUNCTION(2)},

to:

    {TIM15, GPIO('A', 3), 4, GPIO_FUNCTION(2)},

This is not a hardware capability problem, this is just a software limitation. So how to go about more robustly solving that. Should the software know about alternate timers on a pin and automatically use them when there is a conflict with the primary timer? I.e. have a list of timers and try them in a loop:

    {[TIM2, TIM15], GPIO('A', 3), 4, GPIO_FUNCTION(2)},

Then it would only error out when all of the timers on the pin have been programmed.

FWIW @nefelim4ag

@nefelim4ag
Copy link
Contributor

nefelim4ag commented Jan 5, 2026

As I mentioned in the discord.
Those pins are MCU specific.
So to my taste, TIM2 is here by accident.
As you mentioned it may work or may not, depends on the frequency.

Generally, I think the rule of thumb we care about practical application.
This pin is practical.
So, for me, it seems, it is correct to do PR with TIM15.

Cheers,
-Timofey

@garethky
Copy link
Contributor

garethky commented Jan 5, 2026

Doing this on an ad-hock basis for each board we try to adopt could work, but its tedious, error prone & prone to accidental reverts. Having a list of alternate timers would make things a lot more predictable and not require any edits in the future. It would restore control to the user via the printer.cfg file.

On the negatives side:

  • Structs with arrays in them are something of a "no no" in C.
  • Its a lot of work to go through all of the chip documentation and find the alternate timers for every pin.

I can do the one off PR but I don't love that as a generalized answer.

@garethky
Copy link
Contributor

garethky commented Jan 5, 2026

Either way, ship this change! 🚢

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.

5 participants