Skip to content

Fix tmc2240 sensorless homing #27907

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

Conversation

dbuezas
Copy link
Contributor

@dbuezas dbuezas commented Jun 2, 2025

The 2240 uses open collector output on diag0 by default, which is active low.

This change sets it up as push pull active high which should make sensorless homing work

Description

Requirements

Benefits

Configurations

Related Issues

(cherry picked from commit 385ddb9)
@dbuezas dbuezas changed the title enable diag0 push-pull Fix tmc2240 sensorless homing Jun 2, 2025
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 3 times, most recently from 3791e7d to 6ea4a16 Compare June 2, 2025 21:51
@thinkyhead
Copy link
Member

Currently the code is requiring active-HIGH versus active-LOW like so:

    // Stall detection DIAG = HIGH : TMC2209/2240
    // Stall detection DIAG = LOW  : TMC2130/2160/2660/5130/5160

It's not entirely clear why we made TMC2209 the exception and why we want to follow the same pattern with 2240. Which active state is more advantageous in your opinion?

@thinkyhead thinkyhead merged commit 098e096 into MarlinFirmware:bugfix-2.1.x Jun 2, 2025
66 checks passed
@dbuezas
Copy link
Contributor Author

dbuezas commented Jun 2, 2025

Marlin didn't do an exception, the 2209 can only do push pull (with active low) in diag0.

The 2240 (and 5160 and maybe others) can do both open collector (with active low) and push pull (with active high)

The advantage of configuring the 2240 as push pull is that is the same as the 2209. If some board has a pull down (or an mcu doesn't have an internal pullup) then this would potentially be safer.

Bot mostly because it is the smallest change

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.

2 participants