Update pmw3360 comments to match the datasheet better, fix delays. (#15682)

drivers/sensors/pmw3360.c

@@ -78,8 +78,12 @@
 #define CPI_STEP          100
 // clang-format on
 
+// limits to 0--119, resulting in a CPI range of 100 -- 12000 (as only steps of 100 are possible).
+// Note that for the PMW3389DM chip, the step size is 50 and supported range is
+// up to 16000. The datasheet does not indicate the minimum CPI though, neither
+// whether this uses 2 bytes (as 16000/50 == 320)
 #ifndef MAX_CPI
-#    define MAX_CPI 0x77  // limits to 0--119, should be max cpi/100
+#    define MAX_CPI 0x77
 #endif
 
 bool _inBurst = false;
@@ -91,6 +95,7 @@ void print_byte(uint8_t byte) { dprintf("%c%c%c%c%c%c%c%c|", (byte & 0x80 ? '1'
 
 bool pmw3360_spi_start(void) {
     bool status = spi_start(PMW3360_CS_PIN, PMW3360_SPI_LSBFIRST, PMW3360_SPI_MODE, PMW3360_SPI_DIVISOR);
+    // tNCS-SCLK, 120ns
     wait_us(1);
     return status;
 }
@@ -106,12 +111,12 @@ spi_status_t pmw3360_write(uint8_t reg_addr, uint8_t data) {
     spi_status_t status = spi_write(reg_addr | 0x80);
     status              = spi_write(data);
 
-    // tSCLK-NCS for write operation
+    // tSCLK-NCS for write operation is 35us
     wait_us(35);
+    spi_stop();
 
     // tSWW/tSWR (=180us) minus tSCLK-NCS. Could be shortened, but is looks like a safe lower bound
     wait_us(145);
-    spi_stop();
     return status;
 }
 
@@ -119,15 +124,16 @@ 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);
+    // tSRAD (=160us)
+    wait_us(160);
     uint8_t data = spi_read();
 
     // tSCLK-NCS for read operation is 120ns
     wait_us(1);
+    spi_stop();
 
     //  tSRW/tSRR (=20us) minus tSCLK-NCS
     wait_us(19);
-
-    spi_stop();
     return data;
 }
 
@@ -149,7 +155,7 @@ bool pmw3360_init(void) {
     spi_stop();
     wait_us(40);
 
-    // reboot
+    // power up, need to first drive NCS high then low, see above.
     pmw3360_write(REG_Power_Up_Reset, 0x5a);
     wait_ms(50);
 
@@ -190,6 +196,9 @@ bool pmw3360_init(void) {
 }
 
 void pmw3360_upload_firmware(void) {
+    // Datasheet claims we need to disable REST mode first, but during startup
+    // it's already disabled and we're not turning it on ...
+    //pmw3360_write(REG_Config2, 0x00);  // disable REST mode
     pmw3360_write(REG_SROM_Enable, 0x1d);
 
     wait_ms(10);
@@ -242,15 +251,20 @@ report_pmw3360_t pmw3360_read_burst(void) {
 
     pmw3360_spi_start();
     spi_write(REG_Motion_Burst);
-    wait_us(35);  // waits for tSRAD
+    wait_us(35);  // waits for tSRAD_MOTBR
 
     report.motion = spi_read();
-    spi_write(0x00);  // skip Observation
+    spi_read();  // skip Observation
+    // delta registers
     report.dx  = spi_read();
     report.mdx = spi_read();
     report.dy  = spi_read();
     report.mdy = spi_read();
 
+    if (report.motion & 0b111) {  // panic recovery, sometimes burst mode works weird.
+        _inBurst = false;
+    }
+
     spi_stop();
 
 #ifdef CONSOLE_ENABLE
@@ -271,9 +285,5 @@ report_pmw3360_t pmw3360_read_burst(void) {
     report.dy |= (report.mdy << 8);
     report.dy = report.dy * -1;
 
-    if (report.motion & 0b111) {  // panic recovery, sometimes burst mode works weird.
-        _inBurst = false;
-    }
-
     return report;
 }

keyboards/ploopyco/mouse/readme.md

@@ -39,7 +39,7 @@ This should allow you to more heavily customize the behavior.
 
 Alternatively, the `process_wheel` and `process_mouse` functions can both be replaced too, to allow for even more functionality.
 
-Additionally, you can change the DPI/CPI or speed of the krackball by calling `pmw_set_cpi` at any time. Additionally, there is a `DPI_CONFIG` macro that will cycle through an array of options for the DPI.  This is set to 1200, 1600, and 2400, but can be changed.  1600 is also set to the default. 
+Additionally, you can change the DPI/CPI or speed of the trackball by calling `pointing_device_set_cpi` at any time. Additionally, there is a `DPI_CONFIG` macro that will cycle through an array of options for the DPI.  This is set to 1200, 1600, and 2400, but can be changed.  1600 is also set to the default.
 
 To configure/set your own array, there are two defines to use, `PLOOPY_DPI_OPTIONS` to set the array, and `PLOOPY_DPI_DEFAULT`. 
 

keyboards/ploopyco/trackball/readme.md

@@ -19,7 +19,7 @@ See the [build environment setup](https://docs.qmk.fm/#/getting_started_build_to
 
 ## Revisions
 
-There are two main revisions for the PloopyCo Tracball, everything up to 1.004, and 1.005-1.006.
+There are two main revisions for the PloopyCo Trackball, everything up to 1.004, and 1.005-1.006.
 
 In the 1.005 revision, button for was changed from pin B5 to B6, and the debug LED pin was changed from F7 to B5. 
 
@@ -47,7 +47,7 @@ This should allow you to more heavily customize the behavior.
 
 Alternatively, the `process_wheel` and `process_mouse` functions can both be replaced too, to allow for even more functionality.
 
-Additionally, you can change the DPI/CPI or speed of the trackball by calling `pmw_set_cpi` at any time. Additionally, there is a `DPI_CONFIG` macro that will cycle through an array of options for the DPI.  This is set to 1200, 1600, and 2400, but can be changed.  1600 is also set to the default. 
+Additionally, you can change the DPI/CPI or speed of the trackball by calling `pointing_device_set_cpi` at any time. Additionally, there is a `DPI_CONFIG` macro that will cycle through an array of options for the DPI.  This is set to 1200, 1600, and 2400, but can be changed.  1600 is also set to the default.
 
 To configure/set your own array, there are two defines to use, `PLOOPY_DPI_OPTIONS` to set the array, and `PLOOPY_DPI_DEFAULT`. 
 

keyboards/ploopyco/trackball/trackball.c

@@ -218,7 +218,7 @@ void keyboard_pre_init_kb(void) {
 }
 
 void pointing_device_init_kb(void) {
-    pmw3360_set_cpi(dpi_array[keyboard_config.dpi_config]);
+    pointing_device_set_cpi(dpi_array[keyboard_config.dpi_config]);
     // initialize the scroll wheel's optical encoder
     opt_encoder_init();
 }