Marlin: [BUG 2.0.x] (TMC 5160 Stealthchop very bad performance (incorrect PWMCONF_t))

[Silverpumax]

Hi all,
Thanks in advance for your great (and probably hard) work.
Here is my issue

MKS GEN L with MKS Mini LCD 12864
TMC5160 using SPI on X
Marlin bugfix 2.0.x from Jun, 19
TMCStepper version=0.3.5

Description

I am dealing with the very bad performance of a TMC 5160 in stealthchop.
Therefore i measured the motor current with a current clamp and a oscilloscope.

The evaluation ,see attched file, give (among other things) misconfigured pwm_grad parameter.
This is reflected by a current drop after a move of 2-3 mm tested with a speed of 4000 mm/min (80 fullsteps/s).
The current drops from about 800 mA RMS to about 80 mA RMS.
For faster movements the motor stalls completely.

In comparison with TMC 2208 Standalone (stealthchop only) the 2208 runs up to 20000 mm/min (340 fullsteps/s) with no current drop !

Thus i have do a look in to the code for evt. patch the pwm_grad start parameter.
The patch is not possible because you use the wrong PWMCONF_t structur.

Seen in file _stepper_indirection.cpp_
#if HAS_DRIVER(TMC5160)
the pwmconf.pwm_ampl = 180; is a non existing member in PWMCONF_t of a TMC 5160
Seen in file _TMC5160_bitfields.h_ on the TMCStepper library.
Also any modifications to PWMCONF_t in TMC5160_bitfields.h has no effect (and no errors) at compile time.
Also the M122 V readout schows the erroneous PWMCONF_t register for a TMC5160.

Until now i do not find the reason for this behaviour.
I am not very experienced in using templates or namespace in a multi ifdef enviroment.

Among other things i do not understand:
See file _TMC2130_bitfields.h_
struct CHOPCONF_t
The Union "TMC5160" has 35 Bits for a 32 Bit Registerstructur !?

Additional Information

image 1: TMC 5160 in stealthchop with misconfigured pwm_grad 12V
image 2: TMC 2208 standalone 12V (same move)
image 3: TMC 2208 fastmove at 24 V

StepperCurrent.pdf

[teemuatlut]

The TMC5160 should absolutely use the PWM_CONF register struct from the correct namespace!

Also fixing the CHOPCONF struct as well.

[Silverpumax]

Ok, we're going one step into the correct direction.
I get the branch and the latest TMCStepper version (v0.4.0) and i try it out.

But the behavior is still existent !
To adapt the current drop i need modify then pwm_reg entry of PWMCONF for setting a new gradient to the PWM regulator.
But setting any value to pwm_reg has no effect !
Also the M122 V PWMCONF value shows 0x0 for pwm_reg !

Description

Therefore i measured on the SPI bus to see which value are moved to the chip.
The evaluation ,see attched file, give a pwm_reg and pwm_lim parameter of 0x0.
The upper 8 bit are always 0x0.
The lower 24 Bit shows always the correct values !

Something or other masked out the upper 8 bits in software !
Until now i do not find the reason for this behaviour.

Additional Information

The images shows the init sequence for a TMC 5160.
0xF0 is the write access to PWMCONF (0x70)
Packet 18 is the PWMCONF register.

TMC_5160_Initsequence.pdf

[teemuatlut]

Post of a diff of your changes.

[Silverpumax]

Myself have nothing changed.
I get ,via the RAW Button of your branch, the new file stepper_indirection.cpp to replace the old one.
Also i get the new version of TMCStepper lib and changed nothing !
Also i changed nothing in marlin exept the two configfiles !

[Silverpumax]

_I have potentially a hint for you._

The SPI write routine in TMC2130Stepper.cpp uses the PWMCONV_t uint32_t sr:22; register struct of TMC2130_bitfields.h for writing ! (Not the PWMCONV_t `uint32_t sr; struct derived from 2208/2160)
Therefore the upper bits are masked !
This means the stepper_indirection.cpp initialize the correct bits but the write routine uses the wrong union sr:22 !

If i change this struct to uint32_t sr; all is useful.

• Now i can see the correct upper bits in the SPI datastream
• The M122 V gives the correct answer.
• Also the TMC5160 driver runs now without the current drop at max. speed of 100 Rpm (340 Fullsteps/s) (20000 mm/min)

struct PWMCONF_t {
  constexpr static uint8_t address = 0x70;
  union {
    uint32_t sr; //  : 22;    This masks the register !
    struct {
      uint8_t pwm_ampl : 8,
              pwm_grad : 8,
              pwm_freq : 2;
      bool pwm_autoscale : 1,
           pwm_symmetric : 1;
      uint8_t freewheel : 2;
    };
  };
};

best regards --> now in german it is 01:59 in the night --> my bed calls me !

[teemuatlut]

https://github.com/teemuatlut/TMCStepper/releases/tag/v0.4.1

[boelle]

Configurations, please
Please ZIP up your Configuration.h and Configuration_adv.h files (as
requested in the Issue template) and drop them into your next reply.
We'll check them over and see if anything is amiss.

[boelle]

@Silverpumax is the issue still there? and if not also remember to attach your config files

[boelle]

Lack of Activity
This issue is being closed due to lack of activity. If you have solved the
issue, please let us know how you solved it. If you haven't, please tell us
what else you've tried in the meantime, and possibly this issue will be
reopened.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.