From b61836dd57f3287fb321728e014ac99571993dba Mon Sep 17 00:00:00 2001
From: "D.R.racer" <drracer@drracer.eu>
Date: Thu, 29 Jul 2021 08:52:56 +0200
Subject: [PATCH] Remove shifting in config for TMC + add compile-time checks

---
 src/config/config.h     | 24 +++++++++++++++++-------
 src/hal/avr/tmc2130.cpp | 10 +++++++---
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/config/config.h b/src/config/config.h
index 5732ec0..416dadc 100644
--- a/src/config/config.h
+++ b/src/config/config.h
@@ -108,12 +108,22 @@ static constexpr IdlerLimits idlerLimits = {
 
 // TMC2130 setup
 
-static constexpr uint8_t tmc2130_sg_thrs = 3; // @todo 7bit two's complement for the sg_thrs
-static constexpr uint32_t tmc2130_coolConf = (((uint32_t)tmc2130_sg_thrs) << 16U);
-static constexpr uint16_t tmc2130_coolStepThreshold = 400;
-static constexpr uint32_t tmc2130_PWM_AMPL = (uint32_t)(240U & 0xFFU) << 0U;
-static constexpr uint32_t tmc2130_PWM_GRAD = (uint32_t)(4U & 0xFFU) << 8U;
-static constexpr uint32_t tmc2130_PWM_FREQ = (uint32_t)(2U & 0x03U) << 16U;
-static constexpr uint32_t tmc2130_PWM_AUTOSCALE = (uint32_t)(1U & 0x01U) << 18U;
+static constexpr int8_t tmc2130_sg_thrs = 3; // @todo 7bit two's complement for the sg_thrs
+static_assert(tmc2130_sg_thrs >= -64 && tmc2130_sg_thrs <= 63, "tmc2130_sg_thrs out of range");
+
+static constexpr uint32_t tmc2130_coolStepThreshold = 400; ///< step-based 20bit uint
+static_assert(tmc2130_coolStepThreshold <= 0x3ffff, "tmc2130_coolStepThreshold out of range");
+
+static constexpr uint32_t tmc2130_PWM_AMPL = 240;
+static_assert(tmc2130_PWM_AMPL <= 255, "tmc2130_PWM_AMPL out of range");
+
+static constexpr uint32_t tmc2130_PWM_GRAD = 4;
+static_assert(tmc2130_PWM_GRAD <= 255, "tmc2130_PWM_GRAD out of range");
+
+static constexpr uint32_t tmc2130_PWM_FREQ = 2;
+static_assert(tmc2130_PWM_FREQ <= 3, "tmc2130_PWM_GRAD out of range");
+
+static constexpr uint32_t tmc2130_PWM_AUTOSCALE = 1;
+static_assert(tmc2130_PWM_AUTOSCALE <= 1, "tmc2130_PWM_AUTOSCALE out of range");
 
 } // namespace config
diff --git a/src/hal/avr/tmc2130.cpp b/src/hal/avr/tmc2130.cpp
index fc185c5..20c36b6 100644
--- a/src/hal/avr/tmc2130.cpp
+++ b/src/hal/avr/tmc2130.cpp
@@ -19,7 +19,7 @@ bool TMC2130::Init(const MotorParams &params) {
     ///check for compatible tmc driver (IOIN version field)
     uint32_t IOIN = ReadRegister(params, Registers::IOIN);
 
-    // if the version is incorrect or an always 1st bit is 0
+    // if the version is incorrect or a bit always set to 1 is suddenly 0
     // (the supposed SD_MODE pin that doesn't exist on this driver variant)
     if (((IOIN >> 24U) != 0x11) | !(IOIN & (1U << 6U)))
         return true; // @todo return some kind of failure
@@ -45,7 +45,8 @@ bool TMC2130::Init(const MotorParams &params) {
     WriteRegister(params, Registers::TPOWERDOWN, 0);
 
     ///Stallguard parameters
-    WriteRegister(params, Registers::COOLCONF, config::tmc2130_coolConf);
+    static constexpr uint32_t tmc2130_coolConf = (((uint32_t)config::tmc2130_sg_thrs) << 16U);
+    WriteRegister(params, Registers::COOLCONF, tmc2130_coolConf);
     WriteRegister(params, Registers::TCOOLTHRS, config::tmc2130_coolStepThreshold);
 
     ///Write stealth mode config and setup diag0 output
@@ -54,7 +55,10 @@ bool TMC2130::Init(const MotorParams &params) {
     WriteRegister(params, Registers::GCONF, gconf);
 
     ///stealthChop parameters
-    constexpr uint32_t pwmconf = config::tmc2130_PWM_AMPL | config::tmc2130_PWM_GRAD | config::tmc2130_PWM_FREQ | config::tmc2130_PWM_AUTOSCALE;
+    constexpr uint32_t pwmconf = ((uint32_t)(config::tmc2130_PWM_AMPL) << 0U)
+        | ((uint32_t)(config::tmc2130_PWM_GRAD) << 8U)
+        | ((uint32_t)(config::tmc2130_PWM_FREQ) << 16U)
+        | ((uint32_t)(config::tmc2130_PWM_AUTOSCALE & 0x01U) << 18U);
     WriteRegister(params, Registers::PWMCONF, pwmconf);
 
     /// TPWMTHRS: switching velocity between stealthChop and spreadCycle.