pwm3360 driver cleanup and diff reduction to adns9800 (#15559)

* Diff reduction between ADNS9800 and PMW3360 drivers.

They are very similar devices. This (somewhat) unreadable diff is
essentially a no-op, but it makes a `vimdiff` between the 2 drivers much
more readable.

* Cleanup pwm3360 driver some more.

Remove redundant calls to spi_start() and spi_stop(), as pmw3360_write()
will already call these.

3 files changed, 131 insertions, 143 deletions

diff --git a/drivers/sensors/adns9800.c b/drivers/sensors/adns9800.c
index c52f991804..995c9e8614 100644
--- a/drivers/sensors/adns9800.c
+++ b/drivers/sensors/adns9800.c
@@ -77,7 +77,9 @@
 #define MSB1              0x80
 // clang-format on
 
-void adns9800_spi_start(void) { spi_start(ADNS9800_CS_PIN, false, ADNS9800_SPI_MODE, ADNS9800_SPI_DIVISOR); }
+void adns9800_spi_start(void) {
+    spi_start(ADNS9800_CS_PIN, false, ADNS9800_SPI_MODE, ADNS9800_SPI_DIVISOR);
+}
 
 void adns9800_write(uint8_t reg_addr, uint8_t data) {
     adns9800_spi_start();
@@ -154,8 +156,8 @@ void adns9800_init() {
 }
 
 config_adns9800_t adns9800_get_config(void) {
-    uint8_t config_1 = adns9800_read(REG_Configuration_I);
-    return (config_adns9800_t){(config_1 & 0xFF) * CPI_STEP};
+    uint8_t cpival = adns9800_read(REG_Configuration_I);
+    return (config_adns9800_t){(cpival & 0xFF) * CPI_STEP};
 }
 
 void adns9800_set_config(config_adns9800_t config) {
@@ -164,8 +166,8 @@ void adns9800_set_config(config_adns9800_t config) {
 }
 
 uint16_t adns9800_get_cpi(void) {
-    uint8_t config_1 = adns9800_read(REG_Configuration_I);
-    return (uint16_t){(config_1 & 0xFF) * CPI_STEP};
+    uint8_t cpival = adns9800_read(REG_Configuration_I);
+    return (uint16_t)(cpival & 0xFF) * CPI_STEP;
 }
 
 void adns9800_set_cpi(uint16_t cpi) {
@@ -184,7 +186,7 @@ static int16_t convertDeltaToInt(uint8_t high, uint8_t low) {
 }
 
 report_adns9800_t adns9800_get_report(void) {
-    report_adns9800_t report = {0, 0};
+    report_adns9800_t report = {0};
 
     adns9800_spi_start();
 
diff --git a/drivers/sensors/pmw3360.c b/drivers/sensors/pmw3360.c
index cebf4f25d8..4854ba5f47 100644
--- a/drivers/sensors/pmw3360.c
+++ b/drivers/sensors/pmw3360.c
@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "spi_master.h"
 #include "pmw3360.h"
 #include "wait.h"
 #include "debug.h"
@@ -24,55 +25,57 @@
 
 // Registers
 // clang-format off
-#define REG_Product_ID                 0x00
-#define REG_Revision_ID                0x01
-#define REG_Motion                     0x02
-#define REG_Delta_X_L                  0x03
-#define REG_Delta_X_H                  0x04
-#define REG_Delta_Y_L                  0x05
-#define REG_Delta_Y_H                  0x06
-#define REG_SQUAL                      0x07
-#define REG_Raw_Data_Sum               0x08
-#define REG_Maximum_Raw_data           0x09
-#define REG_Minimum_Raw_data           0x0A
-#define REG_Shutter_Lower              0x0B
-#define REG_Shutter_Upper              0x0C
-#define REG_Control                    0x0D
-#define REG_Config1                    0x0F
-#define REG_Config2                    0x10
-#define REG_Angle_Tune                 0x11
-#define REG_Frame_Capture              0x12
-#define REG_SROM_Enable                0x13
-#define REG_Run_Downshift              0x14
-#define REG_Rest1_Rate_Lower           0x15
-#define REG_Rest1_Rate_Upper           0x16
-#define REG_Rest1_Downshift            0x17
-#define REG_Rest2_Rate_Lower           0x18
-#define REG_Rest2_Rate_Upper           0x19
-#define REG_Rest2_Downshift            0x1A
-#define REG_Rest3_Rate_Lower           0x1B
-#define REG_Rest3_Rate_Upper           0x1C
-#define REG_Observation                0x24
-#define REG_Data_Out_Lower             0x25
-#define REG_Data_Out_Upper             0x26
-#define REG_Raw_Data_Dump              0x29
-#define REG_SROM_ID                    0x2A
-#define REG_Min_SQ_Run                 0x2B
-#define REG_Raw_Data_Threshold         0x2C
-#define REG_Config5                    0x2F
-#define REG_Power_Up_Reset             0x3A
-#define REG_Shutdown                   0x3B
-#define REG_Inverse_Product_ID         0x3F
-#define REG_LiftCutoff_Tune3           0x41
-#define REG_Angle_Snap                 0x42
-#define REG_LiftCutoff_Tune1           0x4A
-#define REG_Motion_Burst               0x50
-#define REG_LiftCutoff_Tune_Timeout    0x58
-#define REG_LiftCutoff_Tune_Min_Length 0x5A
-#define REG_SROM_Load_Burst            0x62
-#define REG_Lift_Config                0x63
-#define REG_Raw_Data_Burst             0x64
-#define REG_LiftCutoff_Tune2           0x65
+#define REG_Product_ID                   0x00
+#define REG_Revision_ID                  0x01
+#define REG_Motion                       0x02
+#define REG_Delta_X_L                    0x03
+#define REG_Delta_X_H                    0x04
+#define REG_Delta_Y_L                    0x05
+#define REG_Delta_Y_H                    0x06
+#define REG_SQUAL                        0x07
+#define REG_Raw_Data_Sum                 0x08
+#define REG_Maximum_Raw_data             0x09
+#define REG_Minimum_Raw_data             0x0a
+#define REG_Shutter_Lower                0x0b
+#define REG_Shutter_Upper                0x0c
+#define REG_Control                      0x0d
+#define REG_Config1                      0x0f
+#define REG_Config2                      0x10
+#define REG_Angle_Tune                   0x11
+#define REG_Frame_Capture                0x12
+#define REG_SROM_Enable                  0x13
+#define REG_Run_Downshift                0x14
+#define REG_Rest1_Rate_Lower             0x15
+#define REG_Rest1_Rate_Upper             0x16
+#define REG_Rest1_Downshift              0x17
+#define REG_Rest2_Rate_Lower             0x18
+#define REG_Rest2_Rate_Upper             0x19
+#define REG_Rest2_Downshift              0x1a
+#define REG_Rest3_Rate_Lower             0x1b
+#define REG_Rest3_Rate_Upper             0x1c
+#define REG_Observation                  0x24
+#define REG_Data_Out_Lower               0x25
+#define REG_Data_Out_Upper               0x26
+#define REG_Raw_Data_Dump                0x29
+#define REG_SROM_ID                      0x2a
+#define REG_Min_SQ_Run                   0x2b
+#define REG_Raw_Data_Threshold           0x2c
+#define REG_Config5                      0x2f
+#define REG_Power_Up_Reset               0x3a
+#define REG_Shutdown                     0x3b
+#define REG_Inverse_Product_ID           0x3f
+#define REG_LiftCutoff_Tune3             0x41
+#define REG_Angle_Snap                   0x42
+#define REG_LiftCutoff_Tune1             0x4a
+#define REG_Motion_Burst                 0x50
+#define REG_LiftCutoff_Tune_Timeout      0x58
+#define REG_LiftCutoff_Tune_Min_Length   0x5a
+#define REG_SROM_Load_Burst              0x62
+#define REG_Lift_Config                  0x63
+#define REG_Raw_Data_Burst               0x64
+#define REG_LiftCutoff_Tune2             0x65
+
+#define CPI_STEP          100
 // clang-format on
 
 #ifndef MAX_CPI
@@ -86,23 +89,19 @@ void print_byte(uint8_t byte) { dprintf("%c%c%c%c%c%c%c%c|", (byte & 0x80 ? '1'
 #endif
 #define constrain(amt, low, high) ((amt) < (low) ? (low) : ((amt) > (high) ? (high) : (amt)))
 
-bool spi_start_adv(void) {
+bool pmw3360_spi_start(void) {
     bool status = spi_start(PMW3360_CS_PIN, PMW3360_SPI_LSBFIRST, PMW3360_SPI_MODE, PMW3360_SPI_DIVISOR);
     wait_us(1);
     return status;
 }
 
-void spi_stop_adv(void) {
-    wait_us(1);
-    spi_stop();
-}
+spi_status_t pmw3360_write(uint8_t reg_addr, uint8_t data) {
+    pmw3360_spi_start();
 
-spi_status_t spi_write_adv(uint8_t reg_addr, uint8_t data) {
     if (reg_addr != REG_Motion_Burst) {
         _inBurst = false;
     }
 
-    spi_start_adv();
     // send address of the register, with MSBit = 1 to indicate it's a write
     spi_status_t status = spi_write(reg_addr | 0x80);
     status              = spi_write(data);
@@ -116,11 +115,10 @@ spi_status_t spi_write_adv(uint8_t reg_addr, uint8_t data) {
     return status;
 }
 
-uint8_t spi_read_adv(uint8_t reg_addr) {
-    spi_start_adv();
+uint8_t pmw3360_read(uint8_t reg_addr) {
+    pmw3360_spi_start();
     // send adress of the register, with MSBit = 0 to indicate it's a read
     spi_write(reg_addr & 0x7f);
-
     uint8_t data = spi_read();
 
     // tSCLK-NCS for read operation is 120ns
@@ -133,19 +131,6 @@ uint8_t spi_read_adv(uint8_t reg_addr) {
     return data;
 }
 
-void pmw3360_set_cpi(uint16_t cpi) {
-    uint8_t cpival = constrain((cpi / 100) - 1, 0, MAX_CPI);
-
-    spi_start_adv();
-    spi_write_adv(REG_Config1, cpival);
-    spi_stop();
-}
-
-uint16_t pmw3360_get_cpi(void) {
-    uint8_t cpival = spi_read_adv(REG_Config1);
-    return (uint16_t)((cpival + 1) & 0xFF) * 100;
-}
-
 bool pmw3360_init(void) {
     setPinOutput(PMW3360_CS_PIN);
 
@@ -153,42 +138,51 @@ bool pmw3360_init(void) {
     _inBurst = false;
 
     spi_stop();
-    spi_start_adv();
+    pmw3360_spi_start();
     spi_stop();
 
-    spi_write_adv(REG_Shutdown, 0xb6);  // Shutdown first
+    pmw3360_write(REG_Shutdown, 0xb6);  // Shutdown first
     wait_ms(300);
 
-    spi_start_adv();
+    pmw3360_spi_start();
     wait_us(40);
-    spi_stop_adv();
+    spi_stop();
     wait_us(40);
 
-    spi_write_adv(REG_Power_Up_Reset, 0x5a);
+    // reboot
+    pmw3360_write(REG_Power_Up_Reset, 0x5a);
     wait_ms(50);
 
-    spi_read_adv(REG_Motion);
-    spi_read_adv(REG_Delta_X_L);
-    spi_read_adv(REG_Delta_X_H);
-    spi_read_adv(REG_Delta_Y_L);
-    spi_read_adv(REG_Delta_Y_H);
+    // read registers and discard
+    pmw3360_read(REG_Motion);
+    pmw3360_read(REG_Delta_X_L);
+    pmw3360_read(REG_Delta_X_H);
+    pmw3360_read(REG_Delta_Y_L);
+    pmw3360_read(REG_Delta_Y_H);
 
     pmw3360_upload_firmware();
 
-    spi_stop_adv();
+    spi_stop();
 
     wait_ms(10);
     pmw3360_set_cpi(PMW3360_CPI);
 
     wait_ms(1);
 
-    spi_write_adv(REG_Config2, 0x00);
+    pmw3360_write(REG_Config2, 0x00);
 
-    spi_write_adv(REG_Angle_Tune, constrain(ROTATIONAL_TRANSFORM_ANGLE, -127, 127));
+    pmw3360_write(REG_Angle_Tune, constrain(ROTATIONAL_TRANSFORM_ANGLE, -127, 127));
 
-    spi_write_adv(REG_Lift_Config, PMW3360_LIFTOFF_DISTANCE);
+    pmw3360_write(REG_Lift_Config, PMW3360_LIFTOFF_DISTANCE);
 
     bool init_success = pmw3360_check_signature();
+#ifdef CONSOLE_ENABLE
+    if (init_success) {
+        dprintf("pmw3360 signature verified");
+    } else {
+        dprintf("pmw3360 signature verification failed!");
+    }
+#endif
 
     writePinLow(PMW3360_CS_PIN);
 
@@ -196,13 +190,13 @@ bool pmw3360_init(void) {
 }
 
 void pmw3360_upload_firmware(void) {
-    spi_write_adv(REG_SROM_Enable, 0x1d);
+    pmw3360_write(REG_SROM_Enable, 0x1d);
 
     wait_ms(10);
 
-    spi_write_adv(REG_SROM_Enable, 0x18);
+    pmw3360_write(REG_SROM_Enable, 0x18);
 
-    spi_start_adv();
+    pmw3360_spi_start();
     spi_write(REG_SROM_Load_Burst | 0x80);
     wait_us(15);
 
@@ -214,68 +208,72 @@ void pmw3360_upload_firmware(void) {
     }
     wait_us(200);
 
-    spi_read_adv(REG_SROM_ID);
-
-    spi_write_adv(REG_Config2, 0x00);
-
-    spi_stop();
-    wait_ms(10);
+    pmw3360_read(REG_SROM_ID);
+    pmw3360_write(REG_Config2, 0x00);
 }
 
 bool pmw3360_check_signature(void) {
-    uint8_t pid      = spi_read_adv(REG_Product_ID);
-    uint8_t iv_pid   = spi_read_adv(REG_Inverse_Product_ID);
-    uint8_t SROM_ver = spi_read_adv(REG_SROM_ID);
+    uint8_t pid      = pmw3360_read(REG_Product_ID);
+    uint8_t iv_pid   = pmw3360_read(REG_Inverse_Product_ID);
+    uint8_t SROM_ver = pmw3360_read(REG_SROM_ID);
     return (pid == firmware_signature[0] && iv_pid == firmware_signature[1] && SROM_ver == firmware_signature[2]);  // signature for SROM 0x04
 }
 
+uint16_t pmw3360_get_cpi(void) {
+    uint8_t cpival = pmw3360_read(REG_Config1);
+    return (uint16_t)((cpival + 1) & 0xFF) * CPI_STEP;
+}
+
+void pmw3360_set_cpi(uint16_t cpi) {
+    uint8_t cpival = constrain((cpi / CPI_STEP) - 1, 0, MAX_CPI);
+    pmw3360_write(REG_Config1, cpival);
+}
+
 report_pmw3360_t pmw3360_read_burst(void) {
+    report_pmw3360_t report = {0};
+
     if (!_inBurst) {
 #ifdef CONSOLE_ENABLE
         dprintf("burst on");
 #endif
-        spi_write_adv(REG_Motion_Burst, 0x00);
+        pmw3360_write(REG_Motion_Burst, 0x00);
         _inBurst = true;
     }
 
-    spi_start_adv();
+    pmw3360_spi_start();
     spi_write(REG_Motion_Burst);
     wait_us(35);  // waits for tSRAD
 
-    report_pmw3360_t data = {0};
-
-    data.motion = spi_read();
+    report.motion = spi_read();
     spi_write(0x00);  // skip Observation
-    data.dx  = spi_read();
-    data.mdx = spi_read();
-    data.dy  = spi_read();
-    data.mdy = spi_read();
+    report.dx  = spi_read();
+    report.mdx = spi_read();
+    report.dy  = spi_read();
+    report.mdy = spi_read();
 
     spi_stop();
 
 #ifdef CONSOLE_ENABLE
     if (debug_mouse) {
-        print_byte(data.motion);
-        print_byte(data.dx);
-        print_byte(data.mdx);
-        print_byte(data.dy);
-        print_byte(data.mdy);
+        print_byte(report.motion);
+        print_byte(report.dx);
+        print_byte(report.mdx);
+        print_byte(report.dy);
+        print_byte(report.mdy);
         dprintf("\n");
     }
 #endif
 
-    data.isMotion    = (data.motion & 0x80) != 0;
-    data.isOnSurface = (data.motion & 0x08) == 0;
-    data.dx |= (data.mdx << 8);
-    data.dx = data.dx * -1;
-    data.dy |= (data.mdy << 8);
-    data.dy = data.dy * -1;
-
-    spi_stop();
+    report.isMotion    = (report.motion & 0x80) != 0;
+    report.isOnSurface = (report.motion & 0x08) == 0;
+    report.dx |= (report.mdx << 8);
+    report.dx = report.dx * -1;
+    report.dy |= (report.mdy << 8);
+    report.dy = report.dy * -1;
 
-    if (data.motion & 0b111) {  // panic recovery, sometimes burst mode works weird.
+    if (report.motion & 0b111) {  // panic recovery, sometimes burst mode works weird.
         _inBurst = false;
     }
 
-    return data;
+    return report;
 }
diff --git a/drivers/sensors/pmw3360.h b/drivers/sensors/pmw3360.h
index 9aa8e13f8e..df0c10d643 100644
--- a/drivers/sensors/pmw3360.h
+++ b/drivers/sensors/pmw3360.h
@@ -19,8 +19,6 @@
 #pragma once
 
 #include <stdint.h>
-#include "report.h"
-#include "spi_master.h"
 
 #ifndef PMW3360_CPI
 #    define PMW3360_CPI 1600
@@ -69,10 +67,6 @@ This should work for the 3390 and 3391 too, in theory.
 #    define PMW3360_FIRMWARE_H "pmw3360_firmware.h"
 #endif
 
-#ifdef CONSOLE_ENABLE
-void print_byte(uint8_t byte);
-#endif
-
 typedef struct {
     int8_t  motion;
     bool    isMotion;     // True if a motion is detected.
@@ -83,16 +77,10 @@ typedef struct {
     int8_t  mdy;
 } report_pmw3360_t;
 
-bool             spi_start_adv(void);
-void             spi_stop_adv(void);
-spi_status_t     spi_write_adv(uint8_t reg_addr, uint8_t data);
-uint8_t          spi_read_adv(uint8_t reg_addr);
 bool             pmw3360_init(void);
-void             pmw3360_set_cpi(uint16_t cpi);
-uint16_t         pmw3360_get_cpi(void);
 void             pmw3360_upload_firmware(void);
 bool             pmw3360_check_signature(void);
+uint16_t         pmw3360_get_cpi(void);
+void             pmw3360_set_cpi(uint16_t cpi);
+/* Reads and clears the current delta values on the sensor */
 report_pmw3360_t pmw3360_read_burst(void);
-
-#define degToRad(angleInDegrees) ((angleInDegrees)*M_PI / 180.0)
-#define radToDeg(angleInRadians) ((angleInRadians)*180.0 / M_PI)