[Bug] spi_start() in ./platforms/avr/drivers/spi_master.c fails to always write the SPI2X bit in SPSR.

Hi,

this is just a complaint about the logic in the code. It is likely that right now this does not have any real-world consequences:

Looking at the avr/drivers/spi_master.c spi_start() function for AVR, it appears that the SPI2X bit of SPSR is only ever set 0->1 and never cleared (1->0) if the SPI divisor requires that.

That made me concerned at first, but it seems that the surrounding logic prevents that from becoming a problem with the usual use-case: SPI2X is always cleared back to 0 in spi_stop(). And spi_start() appears to ensure that spi_start() and spi_stop() always pair (by checking for currentSlavePin == NO_PIN). Also at least for Atmega32U4, SPSR defaults to 0x00 after power-on.

It will still fail, if code outside spi_master.c writes the SPSR bit, and grepping through the QMK source-base, there appear to be files directly accessing SPSR (though maybe these files don't intersect with use of spi_master.c).

Having the correctness of this code hinging on these undocumented preconditions may still warrant a fix?

Similar issues appear to exist WRT SPCR register use in spi_start(), however SPCR is at least initialized to a well-defined value in spi_init(), whereas SPSS is not. Maybe initializing SPSS in spi_init() is what the originial author of the code intended but forgot to implement?

cheers,

David

Keyboard Used

No response

Link to product page (if applicable)

No response

Operating System

No response

qmk doctor Output

No response

Is AutoHotKey / Karabiner installed

• AutoHotKey (Windows)
• Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

No response