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

Fan PWM Troubleshooting / Timer Investigation #26

Open
chrissbarr opened this issue Jun 6, 2020 · 2 comments
Open

Fan PWM Troubleshooting / Timer Investigation #26

chrissbarr opened this issue Jun 6, 2020 · 2 comments

Comments

@chrissbarr
Copy link
Contributor

chrissbarr commented Jun 6, 2020

Collecting information for troubleshooting why PWM on FAN (PC9) isn't outputting the expected values when running Marlin bugfix-2.0.x.

Suspect there is a conflict somewhere / the same timer is being used in multiple places or something to that effect.

Timers in use

Checking everywhere timers are configured/selected.

STM32 Arduino Core

RUMBA32_F446VE/variant.h:

// Timer Definitions
// Use TIM6/TIM7 when possible as servo and tone don't need GPIO output pin
#define TIMER_TONE              TIM6
#define TIMER_SERVO             TIM7

SoftwareSerial/src/SoftwareSerial.cpp

// It's best to define TIMER_SERIAL in variant.h. If not defined, we choose one here
// The order is based on (lack of) features and compare channels, we choose the simplest available
// because we only need an update interrupt
#if !defined(TIMER_SERIAL)
  #if defined (TIM18_BASE)
    #define TIMER_SERIAL TIM18
  #elif defined (TIM7_BASE)
    #define TIMER_SERIAL TIM7
  #elif defined (TIM6_BASE)
    #define TIMER_SERIAL TIM6
  #elif defined (TIM22_BASE)
    #define TIMER_SERIAL TIM22

RUMBA32_F446VE/PeripheralPins.c (enabled pins only):

WEAK const PinMap PinMap_PWM[] = {
  {PA_0,  TIM2,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM2, 1, 0)},  // TIM2_CH1
  {PA_1,  TIM5,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM5, 2, 0)},  // TIM5_CH2
  {PA_5,  TIM2,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM2, 1, 0)},  // TIM2_CH1
  {PA_6,  TIM13,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF9_TIM13, 1, 0)}, // TIM13_CH1
  {PA_7,  TIM14,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF9_TIM14, 1, 0)}, // TIM14_CH1
  {PA_8,  TIM1,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM1, 1, 0)},  // TIM1_CH1
  {PA_9,  TIM1,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM1, 2, 0)},  // TIM1_CH2
  {PA_10, TIM1,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM1, 3, 0)},  // TIM1_CH3
  {PA_11, TIM1,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM1, 4, 0)},  // TIM1_CH4
  {PA_15, TIM2,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM2, 1, 0)},  // TIM2_CH1
  {PB_0,  TIM8,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF3_TIM8, 2, 1)},  // TIM8_CH2N
  {PB_1,  TIM8,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF3_TIM8, 3, 1)},  // TIM8_CH3N
  {PB_2,  TIM2,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM2, 4, 0)},  // TIM2_CH4
  {PB_3,  TIM2,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM2, 2, 0)},  // TIM2_CH2
  {PB_4,  TIM3,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM3, 1, 0)},  // TIM3_CH1
  {PB_5,  TIM3,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM3, 2, 0)},  // TIM3_CH2
  {PB_6,  TIM4,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 1, 0)},  // TIM4_CH1
  {PB_7,  TIM4,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 2, 0)},  // TIM4_CH2
  {PB_8,  TIM4,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 3, 0)},  // TIM4_CH3
  {PB_9,  TIM11,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF3_TIM11, 1, 0)}, // TIM11_CH1
  {PB_10, TIM2,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM2, 3, 0)},  // TIM2_CH3
  {PB_13, TIM1,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF1_TIM1, 1, 1)},  // TIM1_CH1N
  {PB_14, TIM12,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF9_TIM12, 1, 0)}, // TIM12_CH1
  {PB_15, TIM12,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF9_TIM12, 2, 0)}, // TIM12_CH2
  {PC_6,  TIM3,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM3, 1, 0)},  // TIM3_CH1
  {PC_7,  TIM8,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF3_TIM8, 2, 0)},  // TIM8_CH2
  {PC_8,  TIM3,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM3, 3, 0)},  // TIM3_CH3
  {PC_9,  TIM8,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF3_TIM8, 4, 0)},  // TIM8_CH4
  {NC,    NP,    0}
};

Marlin

STM32/timers.cpp:

#elif defined(STM32F401xC) || defined(STM32F401xE)
  #define HAL_TIMER_RATE (F_CPU / 2)
  #define MCU_STEP_TIMER  9
  #define MCU_TEMP_TIMER 10
#elif defined(STM32F4xx) || defined(STM32F7xx)
  #define HAL_TIMER_RATE (F_CPU / 2)
  #define MCU_STEP_TIMER  6           // STM32F401 has no TIM6, TIM7, or TIM8
  #define MCU_TEMP_TIMER 14           // TIM7 is consumed by Software Serial if used.
#endif

Timer Reference

image

Timer Mapping

Current Mapping

Timer PWM Pins Tone Servo Soft Serial Step Timer Temp Timer Comment
TIM1 PA8 (CH1) PA9 (CH2) PA10 (CH3) PA11 (CH4)
TIM2 PA0 (CH1) PA5 (CH1) PA15 (CH1) PB2 (CH4) PB3 (CH2) PB10 (CH3) Conflict with multiple pins using CH1
TIM3 PB4 (CH1) PB5 (CH2) PC6 (CH1) PC8 (CH3) Conflict with multiple pins using CH1
TIM4 PB6 (CH1) PB7 (CH2) PB8 (CH3)
TIM5 PA1 (CH2)
TIM6 X X Conflict with Tone and Step
TIM7 X X Conflict with Servo and SoftSerial
TIM8 PB0 (CH2) PB1 (CH3) PC7 (CH2) PC9 (CH4) Conflict with multiple pins using CH2
TIM9
TIM10
TIM11 PB9 (CH1)
TIM12 PB14 (CH1) PB15 (CH2)
TIM13 PA5 (CH1)
TIM14 PA7 (CH1) X Conflict with PWM and Temp

As noted above, there are several conflicts.

Many of the pin PWM conflicts likely do not arise / are not noticed because PWM is actually only enabled and used on a few pins. Could cut back PWM config to only required pins which would remove all conflicts and free up a few timers. Nice to have PWM available on all pins possible, so should trim this back with the goal of keeping as many PWM pins as possible.

Servo and SoftwareSerial conflict is mentioned elsewhere. Likely not always noticed as SoftwareSerial is only used if enabled (for example, TMC2208/2209 drivers). Servo is probably not always used/enabled either, most common application is probably BLTouch sensors. Should remap one of these.

Tone is probably overridden by Marlin step timer setup and doesn't work after that. Probably not used often / not noticed. Should remap one of these.

Interestingly, there is no obvious conflict with the fan PWM / PC9. It may be behaving oddly because of the other conflicted channel mapping on TIM8. Will have to check if resolving all above solves problem.

Proposed Changes

PWM pin allocation can only be changed by updating the STM32Duino Core.
Timers for Tone and Servo can only be changed by updating the STM32Duino Core.
Timers for SoftSerial, Step and Temp can be changed in Marlin (for instance in RUMBA32 pins file) or in STM32DuinoCore.

Updating Marlin will be faster, assuming changes get pulled into bugfix-2.0.x branch. STM32Duino Core changes will only take effect for most users on next release / when PlatformIO incorporates next release.

Suggest moving Step Timer to currently available TIM10. Should be done in Marlin pins_RUMBA32.h file to fix ASAP.
Suggest moving SoftSerial to currently available TIM9. Should be done in Marlin pins_RUMBA32.h file to fix ASAP. Should also be addressed in STM32Duino variant.h so non-Marlin projects do not have to deal with same problem.

PA5 and PA7 are SPI1_SCK and SPI1_MOSI. These do not need PWM. Removing these frees TIM13 and fixes conflict on TIM14. Other PWM pins should be checked for conflicts and reassigned or removed as necessary. Needs to be done in STM32Duino Core.

So, should result in a PR for both Marlin and STM32Duino Core.

Timer PWM Pins Tone Servo Soft Serial Step Timer Temp Timer Comment
TIM1 PA8 (CH1) PA9 (CH2) PA10 (CH3) PE14 (CH4)
TIM2 PA15 (CH1) PB2 (CH4) PB3 (CH2) PB10 (CH3)
TIM3 PC6 (CH1) PC7 (CH2) PC8 (CH3) PC9 (CH4)
TIM4 PD12 (CH1) PD13 (CH2) PD14 (CH3) PD15 (CH4)
TIM5 PA0 (CH1) PA1 (CH2) PA2 (CH3) PA3 (CH4)
TIM6 X
TIM7 X
TIM8
TIM9 X
TIM10 X
TIM11
TIM12 PB14 (CH1) PB15 (CH2)
TIM13
TIM14 X

Other References

Issue about softwareserial / servo conflict here: stm32duino/Arduino_Core_STM32#731

@phongshader
Copy link

I don't know if the issue I'm having is connected to this but it sounds like it is. If I run a m106 command FAN0 does not turn on however FAN0 led does light up. The voltage at the fan terminal after a m106 s255 is 1.39 and s128 is .93, s0 is .10

@chrissbarr
Copy link
Contributor Author

Hi @phongshader,

The problem you're describing is the reason I started looking into this. However, I've found the fan issue is not related to any of the above, but is an issue with the PWM frequency+resolution in 1.7.0 of the Arduino Core (which is the version ProjectIO + Marlin currently uses). When ProjectIO and Marlin update to use v1.9 this issue should be fixed.

In the meantime, the best way to get fans running correctly is to use Marlin's #define FAN_SOFT_PWM feature - all fans should work when that is enabled.

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

No branches or pull requests

2 participants